Add source_task_id sidecar; keep User as a strict unit variant#15
Merged
Conversation
Decorate `SessionSourceType::User` with an `Option<String> task_id`
field so manually-shared local sessions can plumb the conversation's
server-side `ai_tasks` row id through to orchestration discovery, just
as cloud-spawned `AmbientAgent` sessions do today.
Wire compatibility is preserved in both directions:
* The custom `Deserialize` accepts all three shapes: bare `"User"`
(legacy), `{"User":{"task_id":"..."}}` (new), and
`{"AmbientAgent":{"task_id":"..."}}` — mirroring the existing
`AmbientAgent` handling.
* A new manual `Serialize` impl emits the bare legacy form when
`task_id.is_none()` and the struct form otherwise, so older readers
that only understand the unit-variant shape stay forward-compatible
until they pick up the new deserializer.
* `From<&SessionSourceType> for LegacySessionSourceType` now matches
`User { .. }` instead of the unit variant, so the legacy
`JoinedSuccessfully.source_type` field continues to round-trip.
Adds a `SessionSourceType::orchestrator_task_id()` helper that returns
the `task_id` regardless of variant, so downstream orchestration sites
can key off task-id presence rather than the variant discriminant.
Includes unit tests covering legacy + new wire shapes in both
directions, default value, the helper, and the `From` mapping for both
variants.
Co-Authored-By: Oz <oz-agent@warp.dev>
Mirror the existing deserialize_new_user_with_null_task_id test for the AmbientAgent variant so we keep wire compatibility with any already-persisted Redis SessionManifest rows that were written before the manual Serialize impl collapsed the None case to the bare unit-variant form. Co-Authored-By: Oz <oz-agent@warp.dev>
Trim the multi-line doc comments left by the previous cleanup: - SessionSourceType::User: drop the orchestrator-discovery paragraph and fold the `task_id` description into a single sentence. - Default impl: collapse the 4-line "default matches previous behavior" note to a 2-line stable-Rust workaround note. - orchestrator_task_id: drop the "Used to drive orchestration discovery" postscript — the function name carries it. - Serialize impl doc: collapse the 6-line variant explanation to 3 lines. - session_source_type_tests module doc: drop the multi-paragraph migration narrative and keep the one-line summary. - deserialize_new_ambient_agent_with_null_task_id: shorten the inline rationale to one line. Co-Authored-By: Oz <oz-agent@warp.dev>
1 task
Reverts SessionSourceType::User back to a strict unit variant and adds a new optional source_task_id sidecar to InitPayload (sharer) and JoinedSuccessfully (viewer). Restoring the unit form keeps the wire shape compatible with pre-QUALITY-726 viewers that only understood the bare 'User' string; new code threads orchestrator task ids through the sidecar instead. AmbientAgent is left unchanged from main (struct variant with task_id) so existing AmbientAgent producers and viewers continue to roundtrip. The User-variant deserialization, custom Serialize impl, and the NewUser wire branch are all removed; tests are pared down to the reverted SessionSourceType surface and a new init_payload_tests module that exercises the sidecar field for back-compat and roundtrip. Co-Authored-By: Oz <oz-agent@warp.dev>
The new InitPayload::source_task_id sidecar pushed InitPayload across the default clippy threshold for variant size. Boxing the variant would be wire-compatible but ripples through every call site; suppress the lint instead, matching the same scoped allow the vendored protocol copy in session-sharing-server uses. Co-Authored-By: Oz <oz-agent@warp.dev>
cephalonaut
changed the title
szgupta
approved these changes
May 21, 2026
Address review feedback on PR #15 ("tests seem unnecessary for a protocol"): keep only the cases that exercise hand-written logic. The 5 SessionSourceType deserialize tests pin the custom Deserialize impl that bridges the legacy bare-string form and the new struct-variant form, and the InitPayload sidecar test pins the #[serde(default)] contract that lets old clients omit source_task_id. Drop the derive snapshots, roundtrips, trivial From/Default tests, and the Some-case sidecar roundtrip. Co-Authored-By: Oz <oz-agent@warp.dev>
Per review on PR #15: the 5 SessionSourceType deserialize tests covered code that already exists on main (the custom Deserialize bridge predates this PR), and the InitPayload source_task_id default test exercised standard #[serde(default)] behavior that is documented by the attribute itself. Co-Authored-By: Oz <oz-agent@warp.dev>
cephalonaut
added a commit
to warpdotdev/warp
that referenced
this pull request
May 22, 2026
## 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.md` and `specs/QUALITY-726/TECH.md` for the full design. User-visible changes: - A manually shared local orchestrator now drives orchestration discovery in its viewer's pill bar (previously the gate matched only `AmbientAgent`, so manually-shared local orchestrators got no pill bar). - Local children of a manually-shared local orchestrator inherit the share automatically, and the share's children pane shows them. Children that existed before the share started are caught up via a one-shot cascade. Stopping the host's share cascades to those children. - Local-to-driver children of a remote orchestrator now surface in the parent's pill bar — the local client reports the freshly-minted `session_id` back to warp-server on share so the viewer's REST descendant fetch can locate it. - Agent-id references inside conversation bodies resolve to the correct display name on first poll (previously the viewer-side index was never populated, so references rendered as "Unknown"). - Non-orchestrator user shares stop polling after their first empty descendant fetch and only resume on the next `AppendedExchange` on the orchestrator. Wire-format notes: - `SessionSourceType::User` stays a strict unit variant on the wire. A new `source_task_id: Option<String>` sidecar travels alongside `source_type` on `InitPayload` and `JoinedSuccessfully` (and the server's `SessionManifest`), carrying the conversation's `ai_tasks` row id when the user shares an orchestrator. This avoids the wire-incompatibility an earlier iteration introduced when it turned `User` into a struct variant. - A new `BlocklistAIHistoryEvent::LocalSharedSessionEstablished` lifecycle event drives a sidecar model that links the local share's session id to its `ai_tasks` row via `update_agent_task`. ## Demos - local → local: https://www.loom.com/share/7cab1a115706404c9869a41d28dc1efd - local → remote: https://www.loom.com/share/e06c7d4da56f4923b9304f94e4982a29 - remote → local: https://www.loom.com/share/57ae334e3d5f494393709ac87606755b - remote → remote: https://www.loom.com/share/b66e778549a14108be6d9d440be1b3cf ## Known issues / follow-up - **Child-link sibling resolution deferred.** Opening a *child* agent's share link directly does not yet resolve sibling agent IDs in the child transcript — they fall back to the existing "Unknown agent" placeholder. The original design fetched siblings on join and registered them via `start_new_conversation`, which polluted `live_conversation_ids_for_terminal_view`, emitted `StartedNewConversation` events 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 on `BlocklistAIHistoryModel` that doesn't go through `start_new_conversation`. - **Pre-`StreamInit` share with concurrent viewer join.** If the user shares a brand-new local conversation before the first response (so `task_id` is still `None`) and a viewer joins in that window, the sharer's later `StreamInit` upgrade mutates only the local `TerminalModel.shared_session_source.source_task_id`. Already-joined viewers received `source_task_id: None` on `JoinedSuccessfully` and have no push channel for the upgraded value, so their `OrchestrationViewerModel` never 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→viewer `DownstreamMessage::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. - **Catch-up cascade dispatch test coverage.** The `inherit_share_for_local_child` rule itself has tests; the pane-group dispatch loop does not yet. ## Related PRs (must land together) - session-sharing-protocol wire-format change: warpdotdev/session-sharing-protocol#15 - session-sharing-server vendored-protocol mirror: warpdotdev/session-sharing-server#449 - warp-server session-link gate relaxation: warpdotdev/warp-server#11372 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 on `main`. ### Agent Mode - [x] This PR was created by Warp Agent Mode Co-Authored-By: Oz <oz-agent@warp.dev> --------- 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
Adds a
source_task_id: Option<String>sidecar field tosharer::InitPayloadandviewer::DownstreamMessage::JoinedSuccessfully.SessionSourceType::Userstays a strict unit variant (no struct fields), matchingmain.AmbientAgent { task_id }is unchanged.Both new fields are gated on
#[serde(default)]so older clients and viewers ignore them silently. The sidecar carries the conversation's server-sideai_tasksrow id so downstream orchestration discovery can enumerate descendants without re-shaping the variant.Why
Required by the warp client to support session sharing of manually-shared local orchestrator conversations.
Related
Co-Authored-By: Oz oz-agent@warp.dev