QUALITY-726: session sharing for orchestrated agent sessions#11465
Open
cephalonaut wants to merge 23 commits into
Open
QUALITY-726: session sharing for orchestrated agent sessions#11465cephalonaut wants to merge 23 commits into
cephalonaut wants to merge 23 commits into
Conversation
…ad B) Implements Thread B of QUALITY-726 (orchestrated agent session sharing) on the shared-session viewer side, so transcript references to sibling agents resolve to display names instead of "Unknown agent" / "Orchestrator". B1 - Populate `agent_id_to_conversation_id` for viewer-created children: `OrchestrationViewerModel::apply_children_fetch` now calls `BlocklistAIHistoryModel::assign_run_id_for_conversation(child_id, run_id, Some(task_id), terminal_view_id, ctx)` after creating each child conversation. Replaces the prior `set_task_id` call (which never updated the reverse index), so `BlocklistAIHistoryModel::conversation_id_for_agent_id` now resolves child `run_id`s back to their local conversation on the first poll. B2 - Backfill `parent_agent_id` on viewer-created children: subscribe to `BlocklistAIHistoryEvent::ConversationServerTokenAssigned` from `OrchestrationViewerModel::new`. When the event fires for the orchestrator conversation tracked by this model, iterate every viewer-created child whose `parent_agent_id` is unset and stamp it with the orchestrator's `orchestration_agent_id()`. Existing `parent_agent_id` values are not overwritten. This restores `parent_conversation_id` resolution via the `parent_agent_id` fallback when the orchestrator's run id arrives after the first children fetch. B3 - Audit of conversation-body resolution paths (no fixes, as scoped): - `orchestration_conversation_links::parent_conversation_id` (lines 120-129) correctly falls back to `conversation_id_for_agent_id` when `parent_conversation_id` is unset; with B1+B2 in place, it resolves consistently. - `agent_view/child_agent_status_card.rs:197` defaults to "Agent" not "Orchestrator" for missing names. - Latent audit findings documented in the PR description rather than fixed: - `block/view_impl/orchestration.rs:117-123` (`participant_for_conversation`) treats a conversation with no `parent_conversation_id` and a `None` `orchestrator_agent_id` as the orchestrator. With B1+B2 done, the only surface that can still trip this is a child-link view where the child's own `run_id` is missing, which is itself a separate edge case. - `block/view_impl/orchestration.rs:148-164` (`participant_for_current_conversation`) returns the orchestrator avatar as the fallback when the current conversation cannot be resolved. B4 - Child-link sibling preload: add `OrchestrationViewerModel::new_for_child_link(child_task_id, terminal_view_id, terminal_view, ctx)`, which issues the two-call REST chain `get_ambient_agent_task(child_task_id)` -> `parent_run_id`, then `list_ambient_agent_tasks(ancestor_run_id = parent_run_id)`, and indexes each sibling via `assign_run_id_for_conversation`. Sibling placeholder conversations are created with `is_viewing_shared_session = true`; no `EnsureSharedSessionViewerChildPane` events are emitted. Already-indexed siblings (re-joins, restored conversations) are skipped to avoid duplicate placeholders. The constructor is gated by callers on `FeatureFlag::OrchestrationViewerPillBar`. Construction wiring deferred: `new_for_child_link` is not yet called from `shared_session/viewer/terminal_manager.rs:798-816`. That construction site is owned by Thread A's gate restructure, so the integration is left for Thread A. The new helper is marked `#[allow(dead_code)]` until then. Callers should construct it in the existing `FeatureFlag::OrchestrationViewerPillBar` branch when `enable_orchestration_polling == false`. Tests added: - `b1_populates_agent_id_to_conversation_id_for_new_child` - verifies `conversation_id_for_agent_id(run_id)` resolves after the fetch. - `b2_backfills_parent_agent_id_on_orchestrator_token_assigned` - verifies backfill stamps every previously-unset child. - `b2_does_not_overwrite_existing_parent_agent_id` - verifies idempotence. - `b2_ignores_token_assigned_for_unrelated_conversation` - verifies the parent-mismatch and missing-id short-circuits. - `b4_apply_sibling_preload_indexes_each_sibling` - verifies siblings are indexed, joined child is skipped, no pill-bar children are materialized. - `b4_apply_sibling_preload_skips_already_indexed_siblings` - verifies existing index entries are not rebound. - `b4_apply_sibling_preload_handles_empty_siblings` - edge case. All tests use `FeatureFlag::OrchestrationV2.override_enabled(true)` so the `agent_id_to_conversation_id` index is keyed by `run_id`, matching the v2 spec semantics. Co-Authored-By: Oz <oz-agent@warp.dev>
…ocal orchestrators This change implements the warp-client side of QUALITY-726 Thread A: when the user manually shares a local orchestrator session, child agents spawned via `run_agents(local)` inherit the share and are surfaced in the parent-scoped pill bar without enabling cloud-only UI side effects. Key changes: - Source-type stamping: `InputEvent::StartRemoteControl`, `UseAgentToolbarEvent::StartRemoteControl`, and `ShareSessionModalEvent::StartSharing` now stamp the active conversation's `task_id` onto `SessionSourceType::User`. Added a new `TerminalView::active_conversation_task_id` helper that resolves through the agent view controller and falls back to the AI context model. - Cascade-to-local-children: `inherit_share_for_local_child` now cascades for any host source type carrying an `orchestrator_task_id()`, preserving the host variant kind (User -> User, AmbientAgent -> AmbientAgent) with the child's own task_id. - Stop-share cascade: `PaneGroup` tracks cascaded child pane ids in `cascaded_child_panes`; `stop_cascaded_child_shares` stops cascaded children when the host's manual share stops. Wired through a new `Event::StopSharingCurrentSession` handler in `terminal_pane.rs`. - Viewer-side gate restructure: lifted `task_id` parsing out of the `AmbientAgent`-only match in `viewer/terminal_manager.rs` to use `source_type.orchestrator_task_id()`. The orchestration discovery model now spins up for both `User` and `AmbientAgent` shares; cloud-only side effects (`mark_terminal_view_as_ambient_agent_session_view`, `register_ambient_session`, conversation details auto-open, viewer-driven sizing skip) remain strictly `AmbientAgent`-gated with explanatory comments at each check site. - Pre-StreamInit upgrade: extended the existing `BlocklistAIHistoryModel` subscription in `local_tty/terminal_manager.rs` with a `ConversationServerTokenAssigned` arm that upgrades `User { task_id: None }` to `User { task_id: Some(_) }` on the host model once the conversation receives its server-side `task_id`. - Test-only migrations: 9 `SessionSourceType::default()` call sites migrated to `SessionSourceType::User { task_id: None }`. - New tests: 6 unit tests for `inherit_share_for_local_child` covering feature-flag gate, None host, User/AmbientAgent variants with and without `task_id`. Deferred (documented in viewer-side comment): - No re-emit of upgraded source type to existing viewers. The session-sharing protocol (Thread A0) only added `task_id` to the `User`/`AmbientAgent` variants and has no `UpdateSourceType` upstream message. Viewers connected before the upgrade keep the original `task_id: None` until they reconnect; viewers joining after the upgrade see the new task id in `StreamInit`. Validation: - `cargo build -p warp --features local_fs` succeeds. - `cargo fmt --all -- --check` clean. - 9 directly-related tests pass (`test_start_shared_session_from_modal`, `test_stop_shared_session`, `test_stop_sharing_session`, `test_stop_sharing_all_sessions_in_tab`, viewer-impl tests, plus 6 new `inherit_share_for_local_child` cases). - Broader shared-session test bucket: 143 tests pass. Notes: - `Cargo.toml` pins `session-sharing-protocol` to commit ae7ef53 and adds a local-validation `[patch]` table pointing to `../session-sharing-protocol`. The patch is temporary and must be removed once A0 lands on GitHub. Co-Authored-By: Oz <oz-agent@warp.dev>
…ildren Thread D of the QUALITY-726 session-sharing-for-orchestrated-agents work. Adds a new `BlocklistAIHistoryEvent::LocalSharedSessionEstablished` lifecycle event that fires from the sharer-side network handler the moment the local shared session is established, and a subscriber in `TaskStatusSyncModel` that links the conversation's server-side `ai_tasks` row to the freshly-minted `session_id` via `update_agent_task(task_id, None, Some(session_id), None, None)`. The subscriber applies the spec-required guards (skip viewers via `is_viewing_shared_session()`, skip remote-child placeholders via `is_remote_child()`, skip when `task_id` or the conversation lookup is missing) and dedupes on the `(task_id, session_id)` pair so reconnects don't re-fire the RPC. This is what surfaces local-to-driver children inside a remote orchestrator's pill bar — the existing `EnsureSharedSessionViewerChildPane` materialization waits on `AmbientAgentTask.session_id`, which is now populated by the driver. Files changed (Thread D-owned): - app/src/ai/blocklist/history_model.rs - Adds `LocalSharedSessionEstablished { conversation_id, session_id }` variant and a `terminal_view_id()` arm returning `None` (conversation-scoped lifecycle event). - app/src/terminal/local_tty/terminal_manager.rs - Emits the new event from the `SharedSessionCreatedSuccessfully` handler alongside `Manager::started_share(...)`, using the `selected_conversation_id` already captured at share-init time. - app/src/ai/blocklist/task_status_sync_model.rs - Adds `linked_local_shared_sessions: HashSet<(TaskId, SessionId)>` for dedupe. - Adds `on_local_shared_session_established` + `fire_session_link` fire-and-forget pattern that mirrors `fire_update`. - Factors construction through `new_with_ai_client`/a test-only `new_with_ai_client_for_test` so `MockAIClient` can be injected. - app/src/ai/blocklist/task_status_sync_model_tests.rs - 8 new tests covering: positive case fires exactly one RPC; argument order is `(task_id, None, Some(session_id), None, None)`; dedupe blocks a second identical event; distinct session_ids on the same task fire separately; viewer guard skips; remote-child guard skips; missing task_id skips; unknown conversation skips. Files changed (mechanical match-arm additions for the new enum variant): - app/src/ai/agent_conversations_model.rs - app/src/ai/agent_sdk/driver.rs - app/src/ai/blocklist/action_model/execute/start_agent.rs - app/src/ai/blocklist/orchestration_event_streamer.rs - app/src/pane_group/pane/terminal_pane.rs - app/src/terminal/view.rs Each picks up the variant in its existing no-op match arm; no behavior change in those files. Validation: - cargo check -p warp: clean. - cargo test -p warp --lib ai::blocklist::task_status_sync_model: 19/19 passed (11 pre-existing + 8 new). Spec note: TECH.md Thread D names the trigger as `manager.joined_share(...)` inside `JoinedSuccessfully` handling, but those symbols only exist in the viewer-side `shared_session/viewer/terminal_manager.rs`. The owned-file list and the sharer-side semantics ("driver shares the local child" → sharer fires the link) point at the analogous sharer site `manager.started_share(...)` inside `SharedSessionCreatedSuccessfully` handling in `local_tty/terminal_manager.rs`. Emitting from the viewer side would also be neutered by the guards (a viewer's conversation has `is_viewing_shared_session() = true`), so the sharer-side emission point is the only one that ever produces an RPC. Flagged here for integration review. Co-Authored-By: Oz <oz-agent@warp.dev>
…share viewers
Adds an `idle_due_to_no_children` flag to `OrchestrationViewerModel` so that
viewers whose orchestrator has no descendants spend zero CPU / network after
the initial descendant fetch. This is the polling-cost mitigation required by
Thread A's always-stamp `task_id` policy, which spins up the viewer model for
every shared session that carries an `ai_tasks` row (orchestrators and
non-orchestrators alike).
State-machine changes (`orchestration_viewer_model.rs`):
* `apply_children_fetch` sets `idle_due_to_no_children = true` and aborts
the polling handle when the tracked `children` map is empty after the
fetch; clears the flag otherwise.
* `schedule_next_poll` honours the flag by aborting any prior handle and
returning without spawning a new timer. The model waits for an
`AppendedExchange` on the orchestrator to resume.
* `maybe_kick_polling` gains an idle-resume branch in front of the existing
active-cadence guards. The `polling_handle.is_none()` guard alone
conflates "fetch in flight" with "stopped because no children"; the new
branch disambiguates and only resumes on an orchestrator-scoped exchange.
Tests (`orchestration_viewer_model_tests.rs`):
* `c_empty_descendant_fetch_sets_idle_flag_and_aborts_polling` — pre-populates
a polling handle and asserts it is cleared after an empty fetch.
* `c_appended_exchange_on_orchestrator_resumes_from_idle` — observes
`fetch_children` indirectly via `fetch_generation` to confirm resume.
* `c_non_empty_fetch_clears_idle_flag_and_resumes_polling` — exercises the
reverse transition and verifies `schedule_next_poll` re-spawns a timer.
* `c_appended_exchange_on_non_orchestrator_does_not_resume_idle` — confirms
unrelated-conversation exchanges do not pull the model out of idle.
Render-gate audit (`pane_impl.rs:497-552`, no changes): `maybe_add_parent_navigation_card`
still gates on `OrchestrationPillBar || OrchestrationViewerPillBar` plus
`AgentView` fullscreen, and `OrchestrationPillBar::pill_specs` collapses to
`Empty` when there are no descendants. With Thread A's `orchestrator_task_id()`
helper the pill bar surfaces for `User { task_id: Some(_) }` shares as soon as
children are discovered.
Co-Authored-By: Oz <oz-agent@warp.dev>
Thread D's event-based LocalSharedSessionEstablished path (via TaskStatusSyncModel) supersedes the inline update_agent_task call that was added in PR #9516 at the SharedSessionCreatedSuccessfully handler in local_tty/terminal_manager.rs. The two paths fired the same RPC with the same arguments, so removing the inline call avoids double-firing while preserving the same end state. Thread D's path adds (task_id, session_id) dedupe on top. Also removes the now-unused ServerApiProvider import in this file. Deferred: wiring B's OrchestrationViewerModel::new_for_child_link helper into A's viewer-side gate restructure (viewer/terminal_manager.rs:778-832). The helper is annotated #[allow(dead_code)] for now. Wiring it requires a scope-detection REST call before construction, which is a non-trivial restructure best handled as follow-up. Co-Authored-By: Oz <oz-agent@warp.dev>
Reformat Thread D's task_status_sync_model.rs and _tests.rs to match the rustfmt config in this branch. No behavior change. Co-Authored-By: Oz <oz-agent@warp.dev>
The regular cascade in `terminal_pane::inherit_share_for_local_child` only fires at child-pane creation time, so children that were spawned before the parent started sharing stay unshared. The parent viewer's pill bar lists them (warp-server still has their task rows) but the materialization gate in `OrchestrationViewerModel` never trips because `session_id` is never populated on the child task rows. Add a one-time catch-up pass. `PaneGroup` subscribes to `BlocklistAIHistoryEvent::LocalSharedSessionEstablished` (the event Thread D emits from the sharer's `SharedSessionCreatedSuccessfully` handler) and, when a parent's local share goes active, iterates any direct local child agent panes that already exist and dispatches a share for each via `attempt_to_share_session`. Children are recorded in `cascaded_child_panes` so the host's stop-share cascade applies to them as well. Grandchildren are picked up transitively: each newly-shared child's `LocalSharedSessionEstablished` event re-enters the subscriber. Visibility-only change to two terminal_pane helpers (`host_terminal_shared_session_source_type` and `inherit_share_for_local_child`) so they're reachable from `pane_group/mod.rs`. Co-Authored-By: Oz <oz-agent@warp.dev>
Co-Authored-By: Oz <oz-agent@warp.dev>
Move the session-link subscriber out of TaskStatusSyncModel (which is about status sync) and into a new sibling LocalSharedSessionLinkModel. The two concerns now live in separate files with the same singleton lifetime, registered alongside each other in lib.rs. Co-Authored-By: Oz <oz-agent@warp.dev>
Renames the field, method, and helper from `cascade`/`cascaded` to `transitively_shared` to better describe what the structure tracks: the set of child panes whose share was inherited transitively from a host. The per-child-spawn rule (`inherit_share_for_local_child`) is left unchanged because it describes the inheritance rule itself, not the resulting tracking state. * `cascaded_child_panes` -> `transitively_shared_child_panes` * `cascade_share_to_existing_local_children` -> `transitively_share_existing_local_children` * `stop_cascaded_child_shares` -> `stop_transitively_shared_child_shares` * `forget_cascaded_pane` -> `forget_transitively_shared_pane` Co-Authored-By: Oz <oz-agent@warp.dev>
Extracts the inherit_share_for_local_child unit tests from the inline `mod tests` block at the bottom of terminal_pane.rs into a sibling terminal_pane_tests.rs file, included via the `#[cfg(test)] #[path = ...] mod tests;` convention used elsewhere in the codebase. Co-Authored-By: Oz <oz-agent@warp.dev>
Strip references to QUALITY-726, Thread A/B/C/D, spec paths, and \"Handled by TaskStatusSyncModel\" placeholder comments across the shared-session viewer, agent view, pane group, and related test files so the code reads as if it was always this way. Doc comments and \"why\" comments are retained; only dev-artifact references are removed or rewritten to describe behavior without naming the original work threads. Co-Authored-By: Oz <oz-agent@warp.dev>
When UndoClosedPanes is disabled (or the close path otherwise bypasses the undo stack), close_pane previously detached the pane and removed it from pane_contents/panes, but never invoked forget_transitively_shared_pane. That helper is only reached via cleanup_closed_pane on the undo branch, so a closed host with cascaded transitively-shared children leaked stale entries in transitively_shared_child_panes. Mirror cleanup_closed_pane's cleanup by calling forget_transitively_shared_pane in the non-undo branch as well, and add a regression test that exercises that branch via FeatureFlag::UndoClosedPanes.override_enabled(false). Co-Authored-By: Oz <oz-agent@warp.dev>
The patch table targets session-sharing-protocol while its commit lands on main; the QUALITY-726/Thread A0 origin is a dev artifact and doesn't belong in the comment. Rephrase to describe the patch in terms of the upstream PR, without referencing the internal tracking id. The empty LocalSharedSessionLinkModelEvent enum is intentionally left as-is: TaskStatusSyncModel (the closest non-emitting sibling) uses the same empty-enum pattern, and the codebase has no `type Event = ();` uses to follow. Co-Authored-By: Oz <oz-agent@warp.dev>
Trim the multi-paragraph doc and inline comments introduced by the QUALITY-726 commits so the changed surface reads consistently. Key hot spots: - orchestration_viewer_model.rs: collapse the module doc's child-link/ scope-detection narrative, the multi-line struct field docs (parent_task_id, joined_child_task_id, idle_due_to_no_children), and the verbose docs on new_for_child_link / fetch_siblings_for_child_link / apply_sibling_preload / schedule_next_poll / maybe_kick_polling / maybe_backfill_parent_agent_ids / apply_children_fetch. - local_shared_session_link_model.rs: shorten the model-level doc, linked_sessions field doc, new_with_ai_client doc, the skip-conditions doc on on_local_shared_session_established, and fire_session_link. - viewer/terminal_manager.rs: collapse the 7- and 23-line orchestration discovery comments around the join handshake. - pane_group/mod.rs: trim transitively_shared_child_panes field doc, the catch-up share subscribe comment, insert_terminal_pane_hidden_for_child_agent doc, and the new transitively_share_existing_local_children doc. - terminal_pane.rs: collapse the verbose inherit_share_for_local_child doc into a single 8-line block. - local_tty/terminal_manager.rs: shorten the 14-line ConversationServerTokenAssigned upgrade comment plus the two lifecycle-event comments in SharedSessionCreatedSuccessfully. - view.rs / view_impl.rs / use_agent_footer/mod.rs / history_model.rs: trim short multi-line comments to one-or-two-line summaries. - task_status_sync_model.rs: trim the new_with_ai_client doc. Validated: cargo fmt, cargo check -p warp --features local_fs, cargo clippy -p warp --features local_fs --tests -- -D warnings, cargo nextest run -p warp (touched modules). Co-Authored-By: Oz <oz-agent@warp.dev>
Co-Authored-By: Oz <oz-agent@warp.dev>
Contributor
|
I'm starting a first review of this pull request. You can view the conversation on Warp. I completed the review and no human review was requested for this pull request. Comment Powered by Oz |
![oz-for-oss[bot]](https://avatars.githubusercontent.com/in/3450139?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Overview
This PR extends shared-session orchestration discovery across local/remote parent and child topologies, adds server-side session-id linking for local shared sessions, and adds specs/tests for the new behavior.
Concerns
- The committed
Cargo.tomllocal path patch makes the branch unreproducible outside a sibling checkout and overrides the pinned protocol revision. - A manual share that starts before the conversation task id exists only upgrades the local model source type; existing viewers remain on
User { task_id: None }and never start orchestration discovery. - Child-link sibling preload creates normal live conversations for siblings, which can expose sibling navigation/metadata in a view that is supposed to stay child-scoped.
Verdict
Found: 0 critical, 3 important, 0 suggestions
Request changes
Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).
Powered by Oz
| # on `main`; patch to the sibling local worktree until the upstream PR lands. | ||
| # Remove this table once that happens. | ||
| [patch."https://github.com/warpdotdev/session-sharing-protocol"] | ||
| session-sharing-protocol = { path = "../session-sharing-protocol" } |
Contributor
There was a problem hiding this comment.
../session-sharing-protocol, and it overrides the pinned git rev. Remove the patch and commit the upstream protocol rev once the protocol PR lands before merging.
Contributor
Author
There was a problem hiding this comment.
Will fix when protocol PR lands
| return; | ||
| }; | ||
|
|
||
| model |
Contributor
There was a problem hiding this comment.
TerminalModel leaves already-joined viewers with User { task_id: None }, so a share opened before StreamInit never constructs OrchestrationViewerModel after the task id arrives. Propagate this source-type upgrade to active viewers or avoid advertising the pre-StreamInit path as parent-scoped until reconnect.
Contributor
Author
There was a problem hiding this comment.
Narrow blast radius - will follow up.
| // Standalone conversation: there's no local parent on a | ||
| // child-link viewer, so we don't try to wire one up. The | ||
| // conversation is purely a name-resolution placeholder. | ||
| let conversation_id = history.start_new_conversation( |
Contributor
There was a problem hiding this comment.
start_new_conversation registers each sibling in live_conversation_ids_for_terminal_view and emits StartedNewConversation, so a child-link viewer can gain real sibling conversations even though child links must stay scoped to the joined child. Populate the agent-id/name index with metadata-only placeholders or otherwise keep these sibling entries non-navigable.
Contributor
Author
There was a problem hiding this comment.
fixed - by removing the sibling related code for now
The protocol revert in session-sharing-protocol@30bb415 restored `SessionSourceType::User` to a strict unit variant (matching `main`) so pre-QUALITY-726 viewers don't choke on the new struct variant. The orchestrator `task_id` previously carried inside `User { task_id }` now rides alongside as a NEW out-of-band field on the `InitPayload` and `viewer::DownstreamMessage::JoinedSuccessfully`: `source_task_id: Option<String>` (serde `#[serde(default)]`). Threading changes in the integration crate: * `IsSharedSessionCreator::Yes` and `SharedSessionStatus::SharePendingPreBootstrap` pair `source_type` with `source_task_id: Option<String>`. * `TerminalModel` gains `shared_session_source_task_id` field + accessors; `ambient_agent_task_id` falls back to the sidecar so manually-shared local orchestrators (`User` variant) still surface their task id to the viewer-side discovery path. * `Event::StartSharingCurrentSession`, `attempt_to_share_session`, `on_session_share_started`, and `local_tty::TerminalManager:: start_sharing_session` all accept a `source_task_id` argument and thread it into `sharer::Network::new`'s `InitPayload`. * `NetworkEvent::JoinedSuccessfully` and the viewer terminal-manager handler carry `source_task_id`; the viewer writes it to the model sidecar and builds `ambient_task_id` from it (with a fallback to `AmbientAgent.task_id` for cloud-executed shares). * `inherit_share_for_local_child` accepts `host_source_task_id: Option<&str>` and emits the paired `IsSharedSessionCreator::Yes` shape; cascade eligibility accepts EITHER the sidecar OR `AmbientAgent.task_id` so both share kinds cascade as before. * `ConversationServerTokenAssigned` upgrade path mutates the sidecar instead of the (now-unit) `User` variant. * `driver/terminal.rs` mirrors the ambient agent `task_id` into the sidecar so viewer-side discovery can read one field for both variants. Test updates: * `terminal_pane_tests.rs` cascade tests rewritten for the new signature; `User` cascade now asserts on the sidecar; `AmbientAgent` cascade asserts on both the variant field and the sidecar mirror. * All test fixtures that previously constructed `SessionSourceType::User { task_id: ... }` now use the bare unit variant; `attempt_to_share_session` callsites adopt the new `source_task_id: None` argument. Co-Authored-By: Oz <oz-agent@warp.dev>
Rewrites the Thread A design section to describe the sidecar
source_task_id approach that replaced the User struct-variant
iteration:
- SessionSourceType::User stays strictly unit on the wire.
- New source_task_id: Option<String> sidecar on InitPayload,
JoinedSuccessfully, and SessionManifest carries the orchestrator
task_id when source is User.
- AmbientAgent { task_id } is unchanged from main; new sharer code
mirrors the value into source_task_id so viewers can read one
field for both variants.
- Wire-compat section trimmed: no custom Serialize/Deserialize
needed for User; the sidecar is additive #[serde(default)].
- Cascade rule, share-time stamping, non-orchestrator share, test
list, and risks updated to reference the sidecar instead of the
defunct User struct variant.
Co-Authored-By: Oz <oz-agent@warp.dev>
The sharer Network only emits SharedSessionCreatedSuccessfully once per share lifecycle. Reconnects flow through SessionReconnected (not SessionInitialized) and do not re-fire this event, so we never see the same (task_id, session_id) pair twice in practice. Stop-and-reshare mints a fresh session_id from the server, which is a genuinely new pair that should fire its own RPC. The HashSet was guarding against a scenario that doesn't exist; the server is idempotent on update_agent_task, so even if a duplicate did slip through it would be harmless extra traffic. Remove the field, the &mut self plumbing, and the two tests that existed only to exercise the dedupe contract. Co-Authored-By: Oz <oz-agent@warp.dev>
…urce
Replace the historical pair of (SessionSourceType, Option<String>) call-site arguments with a single bundled `SharedSessionSource` carrying both fields. `SharedSessionSource::user(...)` and `SharedSessionSource::ambient_agent(...)` constructors make intent obvious at call sites.
TerminalModel now exposes `shared_session_source()/set_shared_session_source()` instead of separate `set_shared_session_source_type()`/`shared_session_source_task_id()` getters/setters. `IsSharedSessionCreator::Yes { source }` and `SharedSessionStatus::SharePendingPreBootstrap { source }` follow the same single-field shape. View, sharer/viewer network, terminal manager, agent SDK driver, and cascade helpers all updated to thread the bundled source through unchanged.
Test fixtures updated to the new API across pane_group, workspace, terminal view, use_agent_footer, view_impl, and shared session test utilities. Targeted nextest filter (164 tests) passes.
Co-Authored-By: Oz <oz-agent@warp.dev>
B4's `apply_sibling_preload` registered sibling conversations on the child-link viewer's `terminal_view_id`, polluting `live_conversation_ids_for_terminal_view`, emitting `StartedNewConversation` events that cascaded into session-restoration persistence, and exposing siblings as navigable conversations in pickers. Removed the feature to ship a clean QUALITY-726; tracked the follow-up via a TECH.md note. Co-Authored-By: Oz <oz-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
2 participants
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.
What
Makes the orchestration pill bar work for shared-session viewers across every orchestrator/child topology (local-local, local-remote, remote-local, remote-remote × share-parent / share-child). See
specs/QUALITY-726/PRODUCT.mdandspecs/QUALITY-726/TECH.mdfor the full design.User-visible changes:
AmbientAgent, so manually-shared local orchestrators got no pill bar).session_idback to warp-server on share so the viewer's REST descendant fetch can locate it.AppendedExchangeon the orchestrator.Wire-format notes:
SessionSourceType::Userstays a strict unit variant on the wire. A newsource_task_id: Option<String>sidecar travels alongsidesource_typeonInitPayloadandJoinedSuccessfully(and the server'sSessionManifest), carrying the conversation'sai_tasksrow id when the user shares an orchestrator. This avoids the wire-incompatibility an earlier iteration introduced when it turnedUserinto a struct variant.BlocklistAIHistoryEvent::LocalSharedSessionEstablishedlifecycle event drives a sidecar model that links the local share's session id to itsai_tasksrow viaupdate_agent_task.Demos
Known issues / follow-up
start_new_conversation, which pollutedlive_conversation_ids_for_terminal_view, emittedStartedNewConversationevents that cascaded into session-restoration persistence, and exposed siblings as navigable conversations in pickers. Stripped from this PR; a follow-up should add a name-only resolution path onBlocklistAIHistoryModelthat doesn't go throughstart_new_conversation.StreamInitshare with concurrent viewer join. If the user shares a brand-new local conversation before the first response (sotask_idis stillNone) and a viewer joins in that window, the sharer's laterStreamInitupgrade mutates only the localTerminalModel.shared_session_source.source_task_id. Already-joined viewers receivedsource_task_id: NoneonJoinedSuccessfullyand have no push channel for the upgraded value, so theirOrchestrationViewerModelnever constructs and they never see the pill bar for that share. Workaround: the affected viewer can rejoin to pick up the upgraded value. Follow-up: add a sharer→viewerDownstreamMessage::UpdateSourceTaskId(or piggyback on an existing channel) and re-run viewer-side orchestration-discovery construction on receipt. Narrow blast radius — common-case shares and late-joining viewers are unaffected.inherit_share_for_local_childrule itself has tests; the pane-group dispatch loop does not yet.Related PRs (must land together)
The
Cargo.toml[patch]override in this PR currently points the protocol crate at a local sibling worktree; remove the override and bump the rev pin once warpdotdev/session-sharing-protocol#15 lands onmain.Agent Mode
Co-Authored-By: Oz oz-agent@warp.dev