Stage child-run deep linking in the web session viewer (Phase 0 + spec) - #15317
Draft
warp-agent-staging[bot] wants to merge 6 commits into
Draft
Stage child-run deep linking in the web session viewer (Phase 0 + spec)#15317warp-agent-staging[bot] wants to merge 6 commits into
warp-agent-staging[bot] wants to merge 6 commits into
Conversation
…LITY-1764) Clicking a child agent's pill in the web session viewer swapped the child's pane into focus, which then unconditionally recomputed the browser URL from that focused pane's own shareable link. This silently rewrote the orchestrator's ConversationView/SessionView URL to the child's, so a refresh or copied link landed on the child instead of the orchestrator. update_browser_url() already special-cased this for one path: when a focused pane has no shareable link (ShareableLink::Base), it falls back to the current URL instead of resetting to /app if that URL is a ConversationView/SessionView. Extend that same guard to the ShareableLink::Pane case, so a focused pane's own link never replaces the viewer's entry URL either. Both known write paths (PaneGroup::focus's own update_browser_url, and the JoinedSession handler in handle_pane_link_updated) funnel through this single function, so guarding it there covers both without a second fix. Move the ConversationView/SessionView predicate onto WebIntent in web_intent_parser.rs (host-testable, unlike the wasm-gated browser_url_handler.rs) and add unit tests for it.
Contributor
Author
|
This PR was generated with Warp. Comment   |
The predicate tests alone don't observe update_browser_url()'s guard: they still pass if the guard is deleted or its wiring regresses. Extract the actual URL-resolution decision (which URL to commit given the current URL, the requested URL, and force_redirect) into a pure resolve_browser_url() function in a new browser_url_resolution.rs, gated to wasm and test builds since those are its only real callers. update_browser_url() becomes a thin wrapper around it plus the existing DOM-writing code. Add tests that exercise resolve_browser_url directly and fail if the guard regresses: keeping the parent's ConversationView/SessionView URL when a child pane requests its own link (both directions), keeping it when the focused pane has no link at all, using the requested URL and falling back to the base app URL outside the viewer, and bypassing the guard when force_redirect is set.
warp-agent-staging
Bot
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
This PR combines the approved child-run deep-link specification with the narrow URL-preservation fix as Phase 0.
When a child pill swaps the child pane into focus, both pane focus and a later
ManagerEvent::JoinedSessioncan ask the shared browser URL handler to commit the child's own link. Before Phase 0, that replaced the orchestrator route. Refreshing or copying the address bar then opened the child without its orchestration context.Phase 0 adds the pure
resolve_browser_url()decision and preserves a current/conversation/:idor/session/:idroute against non-forced pane URL requests. Both known write paths funnel through this decision. Forced login and signup redirects continue to bypass it.The approved final design lives in
specs/QUALITY-1764/PRODUCT.mdandspecs/QUALITY-1764/TECH.md:?view=standalone.#child=<run-id>, refresh restoration, and Back/Forward selection history.Phase 0 is deliberately temporary. It fixes the reported bug now, but refreshes and copied links return to the orchestrator with no child selected. Its blanket viewer-route guard suppresses the explicit fragment write Phase 2 needs. Phase 2 must remove or invert that guard before it adds anchor navigation; layering the anchor writer behind the current guard would silently prevent the fragment from appearing.
Linked Issue
Linear: QUALITY-1764
Testing
app/src/uri/uri_tests.rshas direct regression coverage for Phase 0's guard throughresolve_browser_url():/conversation/...URL when a child pane requests/session/..., and the reverseforce_redirectis setWebIntent::is_conversation_or_session_viewpredicate testscargo nextest run -p warp -E 'test(/^uri::/)'— 78/78 passed.cargo check -p warp --libandcargo clippy -p warp --all-targets --tests -- -D warnings— clean, no warnings../script/wasm/bundle --check-only(cargo check --target wasm32-unknown-unknown) — thewarplibrary, including the WASM-gated URL resolver, type-checks cleanly under WASM.cargo fmt -- --checkon the touched files — clean.git diff --check— clean; no additional code changed.Visual verification: blocked, not skipped. A dedicated attempt stood up the full local stack needed to exercise the real flow: a local
warp-serverwith Postgres, Redis, and Temporal; non-destructive fixture seeding; the Warp client built for WASM withwith_local_server; and the classic web shell serving/appand/conversation/:id. Opening it in a real browser reached Warp's real sign-in screen. The environment had no guest/anonymous browser path, real Warp test-account credentials, or supported non-UI way to mint a valid local browser session. Therefore the address-bar and refresh behavior is verified by directresolve_browser_urlunit tests and call-graph tracing, not by a live capture.Next step to unblock visual verification: provide a real test-account login or a supported non-UI way to mint a valid browser session for the local server. The rest of the local pipeline is ready to resume.
./script/runAgent Mode
CHANGELOG-BUG-FIX: Fixed the web session viewer rewriting the browser URL to a revealed child agent's URL, which meant refreshing or copying the link landed on the child instead of the orchestrator.