Keep shared-session viewer panes on their own conversation (QUALITY-1676) - #15176
Draft
warp-agent-staging[bot] wants to merge 2 commits into
Draft
Keep shared-session viewer panes on their own conversation (QUALITY-1676)#15176warp-agent-staging[bot] wants to merge 2 commits into
warp-agent-staging[bot] wants to merge 2 commits into
Conversation
) A shared-session viewer pane could be repointed away from the conversation it was replaying. For every `Init` event, `on_shared_init` resolved the event's server conversation token and then unconditionally rebound the pane's active conversation to the result. On a token miss mid-replay the pane's own conversation is no longer empty, so control fell through to `start_new_conversation` and the pane ended up bound to a brand-new empty conversation: the transcript disappeared, the raw terminal blocklist underneath was left on screen, and the `InProgress` status kept the spinner running. Two halves of one invariant: a viewer pane must never repoint its active conversation away from the session's own conversation, and the sharer must never relay a sibling conversation's stream into that session. - `on_shared_init` drops an `Init` with an empty conversation token, and no longer binds the pane to a conversation it had to mint for an unmatched token. The stream is still recorded, so a later `SelectedConversation` update can navigate to it; pane selection stays on its own channel. - The sharer no longer forwards response events for remote-child placeholders. Those live on the parent's terminal surface but mirror another run's conversation, and carry a token no viewer can match. - Historical replay skips conversations with neither a server token nor a forked-from token instead of emitting `conversation_id: ""`, and excludes remote-child placeholders, mirroring the live-forwarding scope. - `link_forked_conversation_token` refuses to write an empty token, which would otherwise unmatch every later `Init` for that conversation. - Refreshes the stale `?ancestor_run_id=` comment: the endpoint is implemented server-side as `parent_run_id = $n` (direct children only). Co-Authored-By: Warp <agent@warp.dev>
Contributor
Author
|
This PR was generated with Warp. |
The first guard keyed off "did this Init force us to mint a conversation", which only held for the first event. `initialize_output_for_response_stream` writes the incoming token onto the minted conversation and indexes it, and replay emits one `Init` per exchange, so the second `Init` for the same foreign conversation resolved through the token index and rebound the pane exactly as before the fix. Test the pane's identity instead: bind unless doing so would displace a shared-session conversation that already holds content. The regression test now sends a second `Init` for the same foreign conversation, which fails against the previous guard. Nothing legitimate loses its rebind: selection travels its own channel (`apply_selected_conversation_update` -> `try_enter_agent_view`, which sets the active conversation itself), a fork re-points the pane conversation's own token first so its `Init` matches by identity, and the initial bind on a pane with nothing established is unaffected. Also from review: - Drop the live `is_remote_child` forward gate. No producer of a local response stream on a remote-child placeholder was found, so it was dead code; the reachable sharer-side leak is the replay path, which is filtered. - Move the replay policy into one tested predicate, `is_replayable_for_shared_session`, covering both tokenless conversations and remote-child placeholders, and log a warning naming the conversation and its exchange count when one is dropped. The terminal-view adaptor no longer duplicates (and mis-describes) that policy. - Cover the remote-child replay filter with a unit test. Co-Authored-By: Warp <agent@warp.dev>
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
Fixes QUALITY-1676. Partway through replaying an orchestrator's transcript in a shared cloud session, the pane stopped showing the transcript and rendered the terminal blocklist underneath it — the environment bootstrap
git cloneblock — with the spinner stuck onWarping....BlocklistAIController::on_shared_initresolved eachInitevent's server conversation token and then rebound the pane's active conversation to whatever it resolved to. When the token belonged to some other conversation the pane ended up bound to it (or, on a miss, to a brand-new empty one), so the transcript vanished and theInProgressstatus kept the spinner running.This change establishes one invariant: a shared-session viewer pane never repoints its active conversation away from the conversation it is already showing.
on_shared_initbinds the pane only when the resolved conversation is the one the pane already shows, or when the pane has no established shared-session conversation yet (the join path). The test is identity, not novelty: the firstInitfor a foreign conversation binds its token to a locally minted conversation, so laterInits for that same conversation resolve normally and would otherwise rebind. Foreign streams are still recorded, so a later selection update can navigate to them; pane selection continues to travel its own channel (apply_selected_conversation_update→try_enter_agent_view, which sets the active conversation itself).on_shared_initdrops anInitwhoseconversation_idis empty. It identifies no conversation, so it can only ever mint one.is_replayable_for_shared_session: conversations with no token at all (previously replayed asconversation_id: "", which matches nothing on the viewer) and remote-child placeholders (they live on the parent's surface but mirror another run, which the viewer materializes from that run's own session) are skipped, with a warning naming the conversation.link_forked_conversation_tokenrefuses an empty incoming token, which would otherwise blank a live conversation's token and unmatch every laterInitfor it.orchestration_viewer_model.rs:?ancestor_run_id=is implemented server-side asparent_run_id(direct children only). Comment only.Not included: a live-forwarding gate on the sharer. No producer of a local response stream on a remote-child placeholder exists today, so that gate would be dead code — the reachable sharer-side leak is the replay path above. Root-cause analysis, the regression window, and the
OrchestrationUnifiedStackquestion are recorded on QUALITY-1676.Linked Issue
QUALITY-1676 (Linear).
Testing
New regression tests, each traced against
masterto confirm it fails there:view_impl_tests.rs— after the pane replays its own conversation,Initevents for a conversation the viewer does not hold must not repoint it and must not cost it its transcript. Two events are sent, because the second is the one that resolves through the token index. Asserted withOrchestrationUnifiedStackboth on and off, since the invariant is not flag-gated.view_impl_tests.rs— anInitwith an emptyconversation_idneither repoints the pane nor creates a conversation.view_impl_tests.rs—link_forked_conversation_tokenignores an empty incoming token.replay_agent_conversations_tests.rs(new) — replay skips tokenless conversations and remote-child placeholders, and still uses the forked-from token when that is the only one.Validation:
./script/formatreports no changes. This repo skips CI on draft PRs (ci.yml:46-53), so the full workflow was dispatched manually against this branch (run 31860632569, all green) — fmt/clippy on Linux, macOS, Windows and wasm; release compilation on all four;Verify compilation with eval features; and the workspace test suites on Linux, macOS and Windows.No local
cargo check/nextestrun: the authoring sandbox has ~4 GB RAM, 2 cores and no swap, and every attempt was OOM-killed partway through thewarpcrate.Verification gap
This restores a visibly broken screen, and it has not been verified visually. Reproducing it needs a two-party shared session — a sharer relaying a foreign
Initto a viewer — driven from a client built off this branch.crates/integrationhas no shared-session support at all today, so an automated visual check would mean building new harness capability, and a live repro additionally needs a depth-2 cloud run on staging. The unit tests above cover the exact event sequences that produced the screenshot, at the model layer../script/runAgent Mode