public/galaxy

Fork 0

Files

T

Ryan Ward 4770ac06b5 first pass of merging in warp (doesn't build)

2026-07-01 16:08:58 -05:00

40 KiB

Raw Permalink Blame History

Orchestration viewer polling — Tech Spec

Context

OrchestrationViewerModel (app/src/terminal/shared_session/viewer/orchestration_viewer_model.rs) drives the orchestration pill bar in the shared-session viewer. It is the only steady-cadence REST poller in the orchestration code paths and the only orchestration delivery path that has not migrated to SSE.

How it works today

Created lazily on the root viewer pane's NetworkEvent::JoinedSuccessfully (viewer/terminal_manager.rs:797-816). Per-child viewer panes are constructed with enable_orchestration_polling = false, so the model is not nested.
Endpoint: GET /agent/runs?ancestor_run_id={parent_task_id} with CHILD_DISCOVERY_FETCH_LIMIT = 100. Despite the parameter name, the server filter is WHERE parent_run_id = $1 (warp-server/model/ai_tasks.go:1814-1818, confirmed by test/integration/public_api_run_parent_run_id_test.go:298-321) — direct children only. The response is a snapshot of those children's task records.
Cadence: STATUS_POLL_INTERVAL = 5s while at least one tracked child is non-terminal, STATUS_POLL_INTERVAL_IDLE = 30s once all are terminal. Idle polling never stops.
Stale-fetch guard: fetch_generation is bumped before each dispatch; older callbacks are dropped.
Kick: subscribes to BlocklistAIHistoryEvent::AppendedExchange and, only during the idle phase, issues an immediate fetch when an exchange lands on the orchestrator or a tracked child.
Teardown: stop_orchestration_polling drops the model on SessionEnded (non-owner), ViewerRemoved, FailedToReconnect, or DetachType::Closed (via TerminalManager::on_view_detached, covering tab-close and split-pane-close). Spawn continuations become no-ops on entity drop; the in-flight HTTP request is not aborted explicitly.

Why it needs to change

For one viewer pane on a remote orchestration with N children: 12 polls/min while anything is non-terminal, 2 polls/min forever after everything's done. N affects payload size, not request count. Multiple viewer panes of the same orchestrator each poll independently. Specific defects:

Idle polling has no upper bound — finished orchestrations keep hitting the endpoint every 30s forever.
No transient/permanent error backoff; failed responses reschedule at the same cadence.
The kick guard does not dedupe in-flight fetches. polling_handle tracks the timer handle, not the fetch handle. After the timer fires and dispatches fetch_children, the slot still holds the (now-completed) timer handle until the response callback runs schedule_next_poll. An AppendedExchange in that window slips past the is_none() check and dispatches a second fetch; the first fetch's response is then discarded by the generation guard. Net effect: every qualifying AppendedExchange issues a redundant HTTP request whose response is thrown away.
The kick is gated on all_terminal and so does nothing while children are active — exactly when new children are most likely to appear.
No visibility/focus gating: the model polls at the same rate whether the viewer pane is visible or hidden behind another tab.
No coalescing across panes: two viewer panes of the same orchestrator each spin their own loop.
Full descendant snapshot every cycle — no since cursor on the endpoint.

Other orchestration delivery for reference

OrchestrationEventStreamer (app/src/ai/blocklist/orchestration_event_streamer.rs) is SSE-based for the orchestrator-owner side. It currently excludes is_viewing_shared_session() and is_remote_child() conversations via is_remote_run_view, so viewer placeholders are not served. Reconnect, cursor, proactive recycle, and consumer registration are well-tested there.
AgentConversationsModel has a 30s POLLING_INTERVAL for the agent-management view, disabled when AmbientAgentsRTC is enabled. Under RTC it refreshes via UpdateManagerEvent::AmbientTaskUpdated pushed through Warp Drive RTC, throttled at 5s (see specs/ambient-agent-rtc-refresh-throttle/TECH.md). The RTC push is owner-scoped on the server (warp-server/pubsub/subscriber/cloud_drive_subscriber_v2.go:581-596), so it does not cover viewers of user-owned orchestrations.

Goals

Replace OrchestrationViewerModel's REST polling loop with event-driven delivery.
Real-time child discovery and per-child ConversationStatus updates.
Cross-pane dedupe of the SSE — multiple viewer panes on the same orchestrator share one connection (placeholders remain per-pane).
Consolidate orchestration delivery in OrchestrationEventStreamer rather than spinning a parallel SSE manager.
Auth model covers the same viewers the current REST poll does (any principal with ViewAction on the parent task), including teammates of a user-owned orchestrator.

Non-goals

Re-platforming the orchestration pill bar UI.
Changes to how the parent's shared session is delivered (PTY / WebSocket).
Migrating orchestrator-owner-side delivery to the new ancestor endpoint in this work. The existing per-run-ids SSE serves owner-side correctly today; migrating owner-side to subscribe via ancestor_run_id (one SSE instead of one-per-run-ids set, no reconnect-on-new-child) is plausible and probably desirable as a future-extension once viewer-side is stable and the endpoint has bedded down. Out of scope here to keep this PR series focused.
Solving the external link-shared viewer case (a viewer with neither team membership nor direct ViewAction). That gap exists today on the REST path and is out of scope here.
Transitive descendant scope (i.e. grandchildren). Today the orchestration model is single-level — children are spawned only by the top-level orchestrator. The new design matches that scope. Extending to transitive ancestry is a future extension if/when nested orchestration ships.

Approach

Server adds GET /api/v1/agent/events/stream?ancestor_run_id={id}&since={cursor} that streams events whose event.run_id's parent_run_id equals the given run — direct children only, matching the existing REST endpoint's scope on the server. The client extends OrchestrationEventStreamer to consume this stream in a new "viewer mode" that emits broadcast events keyed on {parent_task_id, run_id, …}. OrchestrationViewerModel becomes a thin consumer that subscribes to those events, maintains its own run_id → conversation_id map, and applies the resulting status writes locally via BlocklistAIHistoryModel::update_conversation_status. Net steady-state cost for one viewer pane on an orchestrator with N children: one long-lived SSE per orchestrator, one REST snapshot on viewer open (to seed the child set and the SSE cursor), one REST fetch per newly-discovered child after open (for pill metadata), and no periodic polling. Cross-pane dedupe falls out automatically because the streamer is a singleton: multiple viewer panes on the same parent_task_id share the same SSE. Each viewer pane still maintains its own placeholder conversations and run_id → conversation_id map — they can't be shared because each pane has its own BlocklistAIController and conversation tree.

Server design

Sketched briefly; a paired warp-server/specs/... should land first with the full server-side details.

Endpoint

GET /api/v1/agent/events/stream?ancestor_run_id={id}&since={cursor} served by warp-server-rtc, sibling to the existing ?run_ids[]= stream. The parameter name ancestor_run_id mirrors the existing REST endpoint and is kept for naming consistency; under today's single-level orchestration model, the semantic scope is "direct children only" (same as the REST endpoint).

Filter semantics

An event is included iff event.run_id's parent_run_id equals the given run. Direct children only. Matches the existing ?ancestor_run_id= REST endpoint's scope on the server; no new recursive-descendant infrastructure required.
The endpoint emits the same event types as the existing per-run stream — lifecycle (run_in_progress, run_succeeded, run_failed, run_errored, run_cancelled, run_blocked) and new_message — applied across the child set.
The first lifecycle event for a previously-unknown child IS the spawn signal. No separate child_spawned event type on the wire.
For new children spawned after the SSE opens, the server fan-out must add them to the active filter without requiring a re-subscription. Simplest implementation: on task insert, look up the parent and push the new run to any open ancestor-stream subscriber for that parent.

Cursor

The since parameter uses the same monotonic sequence field the existing streams use. The cursor is scoped to the parent's child set (i.e. the maximum sequence the client has seen across any direct child of this parent).
On reconnect, the client sends the last sequence it persisted; the server replays all events with sequence > since for the child set.
The server is the source of truth for sequencing; the client never invents sequence numbers.

Auth

ViewAction on the parent task, evaluated at subscription time. This matches the existing REST ?ancestor_run_id= semantics. Teammates of a user-owned orchestrator are covered; external link-shared viewers without ViewAction are not (same as today).

Operational

Cloud Run streaming timeout applies (~15–20m depending on configuration); reuse the existing DEFAULT_AGENT_EVENT_PROACTIVE_RECONNECT = 14m recycle which is safely under either bound.
Same backoff schedule applies on errors (AgentEventDriverConfig::retry_forever).
Server emits the same delivery counter / latency metric scheme as the existing endpoint for parity.

Client design

Two implementation PRs plus a cleanup PR, all behind a feature flag so the old polling path stays alive until rollout. PR 1 is pure additive wiring (no behaviour change with the flag on or off); PR 2 turns the feature on by adding the SSE consumer and replacing the viewer's polling loop; PR 3 is post-rollout cleanup.

Streamer ↔ viewer-model contract

The streamer broadcasts events for viewer-mode consumers; viewer models subscribe and translate to local placeholder conversations. Both events are parent-task-id-keyed so they can be delivered to all viewer-pane subscribers of the same orchestrator without per-pane streamer state. Each subscriber filters on parent_task_id == self.parent_task_id and maintains its own run_id → conversation_id map.

pub enum OrchestrationEventStreamerEvent {
    // existing variants...
    ChildSpawned {
        parent_task_id: AmbientAgentTaskId,
        run_id: String,
    },
    ChildStatusChanged {
        parent_task_id: AmbientAgentTaskId,
        run_id: String,
        status: ConversationStatus,
    },
}

This is the "broadcast" dispatch shape: the streamer holds a minimal known-child set per orchestrator (to dedupe ChildSpawned) and otherwise broadcasts events; viewer models do all conversation-level translation. Chosen over the alternative where the streamer holds per-pane placeholder maps and writes statuses directly. It is symmetric to existing ctx.subscribe_to_model patterns elsewhere in the codebase (ActiveAgentViewsModel, OrchestrationPillBarModel). new_message events on the ancestor stream are dropped in viewer mode — viewers do not surface inter-agent messages, so the streamer's existing dispatch into OrchestrationEventService is suppressed for viewer-mode parent_task_ids.

PR 1 — Streamer viewer-mode plumbing

Pure additive; the new code paths are unreachable until PR 2 wires them up. Shipped as commit 24477f400 on matthew/orch-polling-pr2, bundled with PR 2 in the same GitHub PR (#11408) since the two pieces interlock too tightly to land in separate review cycles.

Add a per-orchestrator entry inside OrchestrationEventStreamer, keyed on parent_task_id, stored on OrchestrationEventStreamer::viewer_mode_orchestrators: HashMap<AmbientAgentTaskId, OrchestratorStreamState>. Reference-counted across viewer-mode consumer registrations. Entry point: register_viewer_mode_consumer(parent_task_id: AmbientAgentTaskId, orchestrator_placeholder_conv_id: AIConversationId, consumer_id: EntityId). The existing register_consumer(conversation_id, consumer_id) is keyed on conversation; the viewer-mode entry is keyed on parent_task_id so multiple panes' orchestrator placeholders can share the same ancestor consumer. Pair with unregister_viewer_mode_consumer(parent_task_id, consumer_id) for Drop.
Relax is_remote_run_view for is_viewing_shared_session() conversations only when the new feature flag is on. Keep is_remote_child() excluded — owner-side remote children still receive events through their parent's existing per-run-ids SSE.
Add a private lifecycle_event_type_from_wire(&str) -> Option<api::LifecycleEventType> helper that owns the wire-string mapping (including deprecated legacy variants). Shared by drain_ancestor_events and convert_lifecycle_events so both paths agree on which event-type strings are recognised. Pair it with a conversation_status_from_lifecycle_event_type mapping helper (LifecycleEventType → ConversationStatus). Mostly 1:1 with the existing conversation_status_from_state(AmbientAgentTaskState) collapsing rules; the only call-out is Blocked, which today is emitted without a blocked_action payload (we accept empty, matching the REST path).
Add the ChildSpawned / ChildStatusChanged event variants on OrchestrationEventStreamerEvent.
Add an is_known_child(parent_task_id, run_id) helper inside the streamer's per-orchestrator entry so it can emit ChildSpawned once on first observation of a new run_id and only ChildStatusChanged thereafter.

PR 2 — Ancestor SSE consumer + viewer-model migration

This is the PR that actually turns the feature on (still gated by the flag). It combines the streamer-side SSE wiring with the viewer-model rewrite — they were originally split as separate PRs, but without the viewer-model migration the streamer's emitted events have no consumer, and without the streamer the viewer model has no source of events, so the two land together. Shipped as commit b27d694cb on matthew/orch-polling-pr2, alongside the PR 1 commit in GitHub PR #11408. Streamer side:

Add ancestor SSE driver state on the per-orchestrator entry (OrchestratorStreamState::sse_connection: Option<AncestorSseConnectionState>, paired with AncestorForwardingConsumer and AncestorSseStreamItem), ref-counted across viewer-mode registrations. Opens when the first viewer registers; tears down when the last unregisters.
Pick an AgentEventSource trait shape for the new endpoint. Recommended: extend the trait with a filter: AgentEventFilter::AncestorRunId(String) | RunIds(Vec<String>) enum so future endpoints drop in naturally. Alternative: a sibling ServerApiAgentEventSource::open_ancestor_stream(...) method.
The existing run_agent_event_driver::retry_forever machinery is reused for reconnect, transient/permanent backoff, and proactive recycle. The driver itself doesn't need substantive new code, though the AgentEventSource::open_stream(&[String], i64) call site changes to accept the filter enum if that's the chosen trait shape — a small mechanical change that propagates from the trait.
Seed the consumer from a one-shot REST ?ancestor_run_id= snapshot on first open (see "Cold-start seeding" below). The snapshot is used to seed the consumer's known-child set and the SSE's since cursor so already-terminal children don't generate spurious ChildSpawned events.
On each lifecycle or new_message event whose run_id is not yet known to the consumer, emit ChildSpawned { parent_task_id, run_id } once, then proceed to status dispatch.
On every lifecycle event, emit ChildStatusChanged { parent_task_id, run_id, status } after applying the mapping.
Persist the cursor on cursor-advance to the orchestrator's viewer placeholder conversation (not the child placeholders) via the existing persist_event_cursor machinery, with the server-update side disabled for viewer-mode conversations (see "Cursor consistency" below). One cursor per orchestrator, advanced whenever any child's event sequence advances. Viewer-model side:
Delete polling_handle, fetch_generation, fetch_children, schedule_next_poll, maybe_kick_polling, the AppendedExchange subscription, and the constants STATUS_POLL_INTERVAL / STATUS_POLL_INTERVAL_IDLE / CHILD_DISCOVERY_FETCH_LIMIT (unless still referenced by the seed fetch).
On construction, call OrchestrationEventStreamer::register_viewer_mode_consumer(parent_task_id, orchestrator_placeholder_conv_id, terminal_view_id) so the streamer can route ancestor events to this pane's orchestrator placeholder.
Maintain a local run_id → AIConversationId map.
Subscribe to OrchestrationEventStreamerEvent::ChildSpawned and ChildStatusChanged. Filter both on parent_task_id == self.parent_task_id.
- On ChildSpawned: spawn a get_ambient_agent_task(run_id) to fetch pill metadata; create the child placeholder conversation (existing start_new_child_conversation + set_viewing_shared_session_for_conversation(true) + set_task_id flow); record run_id → conversation_id in the local map; emit EnsureSharedSessionViewerChildPane once session_id is known.
- On ChildStatusChanged: look up the placeholder via the local map; call BlocklistAIHistoryModel::update_conversation_status directly.
Subscribe to BlocklistAIHistoryEvent for ConversationServerTokenAssigned and call maybe_backfill_parent_agent_ids(event, ctx) from handle_history_event. The legacy polling path performed this stamping via the maybe_kick_polling subscription pair; the streamer-driven path must keep the parent-agent-id backfill or downstream pill metadata that joins on parent_agent_id would silently miss late-arriving server tokens.
On drop, unregister from the streamer's ref-counted consumer set.
Fix the misleading comment at orchestration_viewer_model.rs:243-245 while editing the file ("ancestor_run_id returns every descendant" — it does not).
Rewrite orchestration_viewer_model_tests.rs. The polling-cadence fixtures don't apply; the apply_children_fetch semantics map almost directly to "apply spawn/status events." Pure-function tests for conversation_status_from_state are unchanged. After this PR (flag on), the viewer model is a ~100-line event router with no timers and no REST loop. Flag off, the old code path stays.

PR 3 — Cleanup

After broad rollout:

Delete the feature flag and the old code path.
Update specs/review-sse-connection-strategy/TECH.md. The "passive views never open their own SSE" invariant becomes "passive viewer-mode views open ancestor-scoped SSEs." Add a debug snapshot for the streamer's viewer-mode entries (mirroring the follow-up that spec already records).

Cold-start seeding

Replay-from-zero on the SSE is impractical for long-lived orchestrations (hundreds to thousands of events for an active multi-child run). Instead, on first open of an orchestrator viewer:

One-shot REST ?ancestor_run_id={parent} fetch — same endpoint the current poller uses, just once.
Read each direct child's last_event_sequence: Option<i64> (already on AmbientAgentTask, populated by the server today).
Compute seed = max(last_event_sequence across children, locally persisted cursor). Locally persisted cursor wins if it's higher (catches the restore-from-disk case). Why max(across children) is safe: each child's last_event_sequence is the highest sequence the server has recorded for that child, so lower-sequence events for a different child don't exist yet at snapshot time. Anything the SSE would skip by starting at max has by definition already been incorporated into one of the child snapshots.
Open the ancestor SSE with since=seed. Only events after that point stream.
The snapshot also pre-populates the consumer's known-child set, so already-terminal children don't generate spurious ChildSpawned events when their first lifecycle event replays. This matches the existing owner-side restore flow in OrchestrationEventStreamer::on_restored_conversations (get_ambient_agent_task + max(sqlite_cursor, server_seq)), just with a list-by-parent variant.

Cursor consistency

The cursor is the only client-side state that has to survive across reconnects and across client restarts. Five points:

Where it lives. Reuse AIConversation::last_event_sequence on the viewer's local orchestrator-placeholder conversation. The streamer's existing persist_event_cursor writes to this field; the new viewer-mode code path uses the same plumbing. No new SQLite column needed. Each viewer pane keeps its own local copy — the cursor is per-pane local state, not server-observable.
Server-side update is suppressed. The existing persist_event_cursor ALSO pushes the cursor to the server via update_event_sequence_on_server so other clients of the same agent run can resume from the same point. For viewer-mode conversations this push must be suppressed: the viewer does not own the task, and writing to the server's task cursor would interfere with the orchestrator-owner's process resuming its own SSE. Gate the server-update branch on !is_viewing_shared_session() and early-return before touching the owner-side self.streams map so viewer-mode placeholders never leave a spurious owner-side ConversationStreamState entry behind.
Monotonicity enforced at the call site. BlocklistAIHistoryModel::update_event_sequence and the server-side update_event_sequence_on_server are both set-not-max. persist_event_cursor therefore folds every known prior value into one effective sequence at the top of the function: the incoming sequence, the in-memory streams[conversation_id].event_cursor (read without inserting, so viewer-mode placeholders stay out of the owner-side map), and the persisted conversation.last_event_sequence(). The effective sequence is then written to SQLite, written to the in-memory map (owner-side only), and pushed to the server (owner-side only). Without this fold a stale drain delivering an older sequence would clobber a higher persisted value.
on_restored_conversations already early-returns for viewer-side conversations (is_remote_view check before any cursor write). So no additional gating is needed there — the changes in (2) and (3) are the only places that needed new gating.
Cursor scope. The cursor on a viewer-mode conversation represents "highest sequence seen across the parent's direct child set." This is a different scope from the owner-side per-conversation cursor on the same orchestrator (which represents "highest sequence on watched_run_ids"). Both numbers come from the same server event log so they're comparable, but they live on different conversation rows (viewer-side placeholder vs. owner-side conversation), so they don't collide. Document this on the field via a doc comment.

Hotfix candidate

Obsolete (PR 2 shipped without a separate hotfix). The kick-guard bug lived on the REST polling path that PR 2 deletes entirely, so patching maybe_kick_polling would have been thrown away within the same review cycle. Retained here for design context.

Rejected alternatives

A — tighten the existing poller (kick guard fix + error backoff + idle backoff + visibility gating). Local fixes inside OrchestrationViewerModel only. Doesn't change the fact that we're REST-polling, and most of the logic gets thrown away when the SSE path lands. Worth doing as a hotfix for the kick-guard bug if we cannot ship the new design immediately.
B — drive discovery from parent history events (AppendedExchange + safety-net timer). Eliminates idle polling but still REST-fetches on each kick. Obsoleted by the ancestor SSE.
C — viewer subscribes to the parent's per-run_id SSE directly. Discovers via parent-bound new_message events; still needs a REST snapshot for child metadata; doesn't catch children that fail before sending the parent anything; introduces a second SSE manager next to OrchestrationEventStreamer. Strictly weaker than the chosen design.
D — relax the streamer's is_remote_run_view exclusion and open one SSE per child placeholder. Per-child SSEs without parent-scope leave discovery unsolved. The chosen design uses one parent-scoped SSE that delivers all child events together.
E (standalone) — client subscribes to the new ancestor endpoint without consolidating in the streamer. Same wire shape but creates a parallel SSE manager. Worse for reconnect / cursor / proactive-recycle consistency.
F.cheap — REST ancestor watcher inside the streamer. Useful only if the server endpoint can't ship first. Skipped because the server endpoint can ship first.
G — use AgentConversationsModel's RTC pushes. Owner-scoped on the server (getUsersToNotifyForTask returns task owner or team members), so teammate viewers of user-owned orchestrators never receive the push. Even if scope were relaxed, the dispatch-path question would still need answering. The chosen design subsumes it.
Dispatch via streamer-owned per-pane placeholder maps. The streamer would hold per-orchestrator Vec<ViewerRegistration> and per-viewer run_id → conversation_id maps; handle_event_batch would iterate registrations and call update_conversation_status directly. Picked the broadcast-event option instead: streamer emits parent-task-id-keyed events, viewer models translate to local placeholder conversations. Cleaner separation, symmetric to existing event-subscription patterns, and the "more code per viewer" cost is small.
Transitive descendant scope. Today the orchestration model is single-level. Designing for transitive ancestry now would require either a recursive CTE filter (expensive) or a denormalised root_orchestrator_id column (server schema migration). Out of scope; future extension if nested orchestration ships.

Testing and validation

Unit tests

Streamer ancestor consumer (PR 1 + PR 2). Lifecycle event for a known child → ChildStatusChanged emitted with the mapped status (one test per LifecycleEventType). Lifecycle event for an unknown child → ChildSpawned emitted exactly once, then ChildStatusChanged. New-message events do NOT route into OrchestrationEventService for viewer-mode parent_task_ids.
Ancestor consumer ref-counting (PR 2). Two viewer-mode registrations for the same parent_task_id open one SSE; second unregister tears down; reconnect on flap. Stale cursor on registration is honoured.
Cold-start seeding (PR 2). One-shot REST snapshot populates the known-child set; SSE opens with since=max(child.last_event_sequence, local_cursor); replayed events for already-known children do NOT re-emit ChildSpawned.
Cursor scope (PR 2). Server-update path is suppressed for viewer-mode conversations. Local SQLite cursor advances on each event applied. Restart-from-disk path uses the local cursor and does not double-deliver.
OrchestrationViewerModel as consumer (PR 2). ChildSpawned event → child placeholder conversation created with correct agent_name / set_task_id / is_viewing_shared_session=true. Subsequent ChildStatusChanged for the same run_id updates the pill via the local map. EnsureSharedSessionViewerChildPane is emitted once per child, after session_id becomes known. Events for other parent_task_ids are ignored.
Two-pane fixture (PR 2). Open two viewer panes for the same orchestrator. Confirm both create independent placeholders, both apply status updates locally, neither pane's writes leak into the other's history, and only one SSE connection is open across both panes. Closing one pane keeps the SSE alive (refcount=1); closing both tears it down.
Test fixture migration. The existing pure-function tests (maps_*_to_*, unknown_state_maps_to_error) carry over. The apply_children_fetch integration tests are rewritten as streamer-event-driven tests with the same coverage.

Manual validation

Open a shared session viewer for a teammate's orchestrator with active children. Observe the new SSE open in the network panel; no periodic ?ancestor_run_id= REST requests after the cold-start seed.
Watch a new child spawn server-side — verify the pill bar updates within seconds without any REST polling (apart from the one-shot per-child metadata fetch).
Switch to a different terminal pane (hide the viewer). With the streamer's visibility gating, the SSE should tear down when the last viewer-mode consumer unregisters. Switch back — SSE re-opens with the persisted cursor; no events are lost.
Open the viewer for an orchestrator while the network is down. Observe the driver's backoff schedule. When the network returns, events resume from the persisted cursor.
Kill the app while the viewer is open, restart, rejoin the same orchestrator. The cursor is loaded from SQLite; the new SSE includes a since= parameter; no duplicate events appear.
Open the same orchestrator in two panes / windows. Confirm only one SSE is open (cross-pane dedupe). Close one pane — the SSE stays. Close both — the SSE tears down.
Open a viewer on an orchestrator whose children have already terminated. Confirm the cold-start REST snapshot populates the pill bar with their terminal status, and no spurious status flicker occurs as the SSE catches up.

Server-side spot check

After PR 2 + flag on:

For one viewer pane on a finished orchestration left open for 30 minutes: the prior baseline is ~60 hits to /agent/runs?ancestor_run_id= over that window. Expected post-change: one REST hit on open (the cold-start seed), plus one SSE that recycles every ~14m via the proactive reconnect.

Risks and mitigations

is_remote_run_view relaxation scope. Must apply to is_viewing_shared_session() only, NOT is_remote_child(). Owner-side remote children continue to receive events through the parent's existing SSE; a second SSE for the placeholder would double-deliver. Mitigation: explicit branch in is_eligible plus a test asserting owner-side remote children remain ineligible.
Cursor-server-update suppression. The existing persist_event_cursor pushes cursors to the server. Viewer-mode conversations must not touch the server cursor for the underlying task. Mitigation: gate the server-update branch on !is_viewing_shared_session() and add a test that exercises the streamer's viewer-mode write path without a corresponding update_event_sequence_on_server call.
Streamer dispatch regression. PR 1 changes handle_event_batch, which is hot code on the orchestrator-owner side. Mitigation: viewer-mode branch is gated on a per-conversation marker that defaults off for owner-side conversations; existing tests for OrchestrationEventService::enqueue_event_batch integration must continue to pass without modification.
Double-write during flag transition. With the flag on, the streamer drives status updates via emitted events; with the flag off, the old polling loop does. While both code paths exist, ensure they cannot both run for the same conversation. Mitigation: the polling loop in OrchestrationViewerModel::new is gated on the same feature flag — flag on means no polling loop is created.
Cross-pane state divergence. Two viewer panes of the same orchestrator subscribe to the shared ChildSpawned event. Race: one pane creates its placeholder before the other. The placeholders are independent (different AIConversationIds) but reference the same server-side run. Mitigation: viewer model uses its own local run_id → conversation_id map keyed on the run_id, so each pane resolves the right placeholder regardless of timing. Add a test fixture for the two-pane case.
agent_id_to_conversation_id collisions (pre-existing). BlocklistAIHistoryModel::agent_id_to_conversation_id (history_model.rs:238) is a 1:1 index keyed on orchestration_agent_id(). Two viewer panes that create independent placeholders for the same server-side run can collide on this index, particularly on app restart where restore_conversations populates the index unconditionally for any conversation with an agent_id_key(). This is not introduced by the new design — current code already creates viewer placeholders via set_task_id. Mitigation: walk the existing conversation_id_for_agent_id call sites (orchestration_events.rs:221,359,738, agent_view/orchestration_conversation_links.rs:39,127, agent_conversations_model/entry.rs:290, conversation_details_panel.rs:227, block/view_impl/orchestration.rs:24,95,805) in PR 1 and confirm none are exercised by viewer-mode placeholders (all the current call sites are owner-side paths). If a viewer-mode use is found, scope the index to non-viewer conversations.
Multi-pane SSE refcount race. First pane opens the SSE; second pane registers (refcount=2); first pane closes (refcount=1, SSE stays). Standard refcount territory but worth a test fixture, especially given the existing streams HashMap uses entity drop for cleanup.
Blocked payload. The lifecycle event currently emits Blocked without blocked_action. The viewer's ConversationStatus::Blocked accepts an empty string and already does so on the REST path. Mitigation: documented; populate blocked_action on the server side as a separate follow-up if the surface gains value.
External link-shared viewer. Users with neither team membership nor direct ViewAction on the parent are not covered by either the REST poll or the new SSE endpoint. Mitigation: documented in non-goals; tracked separately.
Cloud Run streaming timeout vs 14m proactive recycle. Same risk as the existing stream. Mitigation: reuse DEFAULT_AGENT_EVENT_PROACTIVE_RECONNECT and the existing driver.
Server-side live-stream fan-out for new children. The server must add newly-spawned children to active ancestor-stream subscriptions without requiring a re-subscription. Mitigation: pubsub fan-out on task insert can publish to a per-parent topic; subscribers register interest in the parent. Server team owns this; flagged in the paired server spec.
update_conversation_status echoing back through TaskStatusSyncModel. TaskStatusSyncModel::on_conversation_status_updated (app/src/ai/blocklist/task_status_sync_model.rs:146-148) explicitly early-returns for is_viewing_shared_session() conversations. Mitigation: existing guard verified; add a test asserting no update_agent_task call is made for viewer-side status writes.
Race between REST snapshot and SSE open. During the gap between the cold-start snapshot and the SSE handshake, new children may spawn or existing ones may receive events. Both flows converge correctly: a child spawned during the gap is reported as ChildSpawned when its first lifecycle event arrives on the SSE (its run_id isn't in the known-child set); a concurrent insert visible in the snapshot pre-populates the known-child set and the first SSE event becomes a ChildStatusChanged. Mitigation: documented; add a test that races a child-spawn with the seed.
Drop refcount race. OrchestrationViewerModel's Drop impl runs while the streamer may still be iterating its ancestor consumer. Standard reference-counting hazard. Mitigation: unregister_viewer_mode_consumer is idempotent and safe to call after the streamer has already torn down its entry (e.g., logout flow). Add a test fixture for the drop-during-iteration case.
Restore order for orchestrator vs child placeholders. On app startup, restore_conversations loads orchestrator and child placeholders in an unspecified order. If the streamer subscribes for the orchestrator before child placeholders are restored, replayed ChildSpawned events would refer to children not yet in the history model. The existing update_conversation_status no-ops on missing conversations, so the worst case is a missed status flicker that the next event corrects. Mitigation: add a restored-state test fixture; if the missed-flicker behaviour is unacceptable, defer SSE registration until restore completes.

Parallelization

The implementation spans two repositories (warp client and warp-server) with no overlapping code, so the bulk of the work can be delegated to parallel agents working on sibling worktrees. Three phases, three agents.

Phase A — server endpoint and client wiring in parallel

Two agents run concurrently against a frozen wire contract (this spec).

Server endpoint agent. Worktree /Users/matthew/src/orch-polling/warp-server, branch matthew/orch-polling. Owns: the GET /api/v1/agent/events/stream?ancestor_run_id= endpoint plus the paired warp-server/specs/... spec. Includes the per-parent pubsub fan-out (so new children appear without a re-subscription). Coordinates only at the wire contract; needs no client knowledge. Outputs a server PR ready for review.
Client wiring agent (PR 1). Worktree /Users/matthew/src/orch-polling/warp, branch matthew/orch-polling. Owns: streamer viewer-mode plumbing per PR 1 — feature flag, register_viewer_mode_consumer / unregister_viewer_mode_consumer API, is_remote_run_view relaxation, LifecycleEventType → ConversationStatus mapping, ChildSpawned / ChildStatusChanged event variants, is_known_child helper. Pure additive; no behaviour change with the flag in either state. Outputs a client PR ready for review. Both agents work in separate git repositories, so there are no merge conflicts between them. They coordinate only through this spec.

Phase B — consumer + migration (after Phase A merges)

One agent picks up where Phase A left off, once both Phase A PRs are merged (or at least mergeable) and the server endpoint is reachable from staging.

Consumer + migration agent (PR 2). Worktree /Users/matthew/src/orch-polling/warp, same branch (continuing from PR 1). Owns: the AncestorConsumer, the SSE driver wiring, the cold-start REST seed, the viewer-model rewrite, the test fixture migration, and the comment fix at orchestration_viewer_model.rs:243-245. Validates against staging via the manual checklist before opening the PR. Outputs a client PR ready for review. This agent benefits from holding both halves of the change in one head: the streamer-side event emission and the viewer-model-side consumption are interlocked enough that splitting them across two agents would create unnecessary coordination overhead.

Phase C — cleanup (post-rollout)

Manual, not delegated. After the feature flag has been at stable for long enough to confirm no regressions:

Delete the feature flag and the old polling code path.
Update specs/review-sse-connection-strategy/TECH.md (the "passive views never open their own SSE" invariant).
Add the debug snapshot for the streamer's viewer-mode entries.

Pre-launch checklist

Before launching the Phase A agents:

Final approval of this spec.
Decide the feature flag name (suggested: FeatureFlag::OrchestrationViewerStreamer).
Confirm the warp-server worktree is on matthew/orch-polling and clean.
Confirm the warp worktree is on matthew/orch-polling and clean.
Draft the agent prompts: each prompt should include this spec's section references and the explicit scope above.

Coordination model

All three agents share this spec as their source of truth. No agent-to-agent messaging required for normal operation.
If the wire contract needs to change mid-work, the spec is updated first, then the agents are re-prompted from the new spec. Avoid out-of-band coordination.
Each agent is expected to surface uncertainty by stopping and asking, not by deciding unilaterally. The orchestrator monitors for blocked status.

Environment

Client worktree: /Users/matthew/src/orch-polling/warp, branch matthew/orch-polling.
Server worktree (sibling): /Users/matthew/src/orch-polling/warp-server, branch to match.
Touched files on the client (Phase 1):
- app/src/ai/blocklist/orchestration_event_streamer.rs — viewer-mode plumbing, ancestor consumer, broadcast events.
- app/src/ai/blocklist/orchestration_event_streamer_tests.rs — new tests.
- app/src/terminal/shared_session/viewer/orchestration_viewer_model.rs — consumer migration, local run_id → conversation_id map.
- app/src/terminal/shared_session/viewer/orchestration_viewer_model_tests.rs — fixture rewrite.
- app/src/server/server_api.rs — new ancestor-scoped stream helper alongside stream_agent_events.
- app/src/ai/agent_events/driver.rs — extend the AgentEventSource trait with a filter enum (or add a sibling method for the ancestor endpoint).
- crates/warp_core/src/features.rs — new feature flag.
Touched files on the server: TBD pending the paired server spec.
No other client files should need to change.

40 KiB Raw Permalink Blame History Unescape Escape