QUALITY-928: Orchestration unified stack — pane path + transcript (M2) - #14480
Open
cephalonaut wants to merge 29 commits into
Open
QUALITY-928: Orchestration unified stack — pane path + transcript (M2)#14480cephalonaut wants to merge 29 commits into
cephalonaut wants to merge 29 commits into
Conversation
cephalonaut
force-pushed
the
matthew/orch-unified-m2
branch
4 times, most recently
from
July 31, 2026 15:30
db13489 to
7bb3aa3
Compare
cephalonaut
marked this pull request as ready for review
July 31, 2026 21:49
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 the orchestration unified stack for child pane materialization, viewer-side transcript loading, observer restoration, and completed-child continuation behavior.
Concerns
- The unified viewer child join-failure recovery path is gated on
is_viewing_shared_session(), but unified viewer children are now represented as remote-child conversations, so missing/inaccessible child live sessions can fail to recover. - The PR changes user-facing child-pane behavior and adds a visible unavailable-session state, but the PR description does not include screenshots or a screen recording demonstrating the end-to-end UI.
- Security pass: no additional security-specific findings beyond the recovery/access-control-adjacent behavior above.
- Spec check: no approved or repository spec context was provided for implementation drift validation.
Verdict
Found: 0 critical, 2 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
| } | ||
| let Some(task_id) = BlocklistAIHistoryModel::as_ref(ctx) | ||
| .conversation(&child_id) | ||
| .filter(|conversation| conversation.is_viewing_shared_session()) |
Contributor
There was a problem hiding this comment.
is_viewing_shared_session(), so this filter makes OrchestrationChildSharedSessionJoinFailed return before marking the pane unavailable or refetching for transcript recovery. Accept the unified remote-child observer shape here as well.
| Flex::column() | ||
| .with_child( | ||
| Text::new_inline( | ||
| "Live session unavailable", |
Contributor
There was a problem hiding this comment.
cephalonaut
force-pushed
the
matthew/orch-unified-m2
branch
5 times, most recently
from
August 1, 2026 17:26
8dd85d8 to
6c23d40
Compare
cephalonaut
force-pushed
the
matthew/orch-unified-m1
branch
from
August 2, 2026 20:03
d957cd3 to
cedb591
Compare
…fication Wires M1's tracker into the client pane group and conversation model: - OrchestrationViewerModel: uses AgentConversationsModel as fetch authority; pending_task_ids_for_discovery drain path; drain_pending_task_discoveries. - Unified child pane materialization: apply_child_pane_materialization, attach_ambient_orchestration_child_session, hydrate_child_transcript, and process_pending_child_hydrations replace the parallel owner/viewer paths. ChildPaneOrigin dropped from the pending map; stale-session guard universal. - new_for_ambient_orchestration_child: TerminalManager constructor combining is_ambient_agent=true with orchestration_child_conversation_id=Some for FailedToJoin recovery routing on all child panes. - TaskOwnership/TaskScope removed; simple creator.uid check used instead. - Dead code removed: create_orchestration_child_shared_session_viewer, new_for_orchestration_child, ChildPaneOrigin enum. - EnsureSharedSessionViewerChildPane handler simplified (flag-ON branch removed). - Orchestration tracker/streamer tests updated for M2 behavioral changes: Started inserts TrackedChild immediately (children non-empty after observe_child). Co-Authored-By: Oz <oz-agent@warp.dev>
cephalonaut
force-pushed
the
matthew/orch-unified-m2
branch
from
August 2, 2026 20:12
081761c to
53b8105
Compare
write_updated_conversation_state now skips the is_viewing_shared_session early-return when the conversation belongs to the current user (OrchestrationUnifiedStack only). This makes the parent conversation's AIConversationId stable across restarts: children's persisted parent_conversation_id continues to match, so the initialize_historical_conversations indexing pass rebuilds children_by_parent correctly on startup, restoring the pill bar without any changes to the restore path itself. Co-Authored-By: Oz <oz-agent@warp.dev>
…ned cloud runs When the parent cloud agent conversation is now persisted locally (b388b3a), resolve_entry_open_action returns RestoreOrNavigateToConversation instead of OpenConversationTranscriptViewer because has_local_persisted_data is now true. The previous handler for that arm created a zero-state compose pane. Fix: fall through to transcript loading using task.conversation_id() as the server token, matching the OpenConversationTranscriptViewer path. Also extract restore_pane_with_transcript to reduce duplication between the two arms. Co-Authored-By: Oz <oz-agent@warp.dev>
Document Fix 2 (seed child conversations from AmbientAgentTask.children at restore time) alongside the existing persistence fix, plus the loading-pane regression fix that shipped with it. Covers the two cases the SQLite-based persistence fix cannot handle: the WASM web client (no local_fs feature, so no cross-session agent_conversations table) and the first restore of a run whose parent was never persisted, which also needs an explicit parent-link reconciliation for its stale child rows. Co-Authored-By: Oz <oz-agent@warp.dev>
Rebuild the parent to child conversation index for a restored cloud agent parent from the server-reported `task.children` list instead of relying on the local conversation index. This is the only pill-bar source on clients without cross-session SQLite (web) and on the first restore of a run whose parent was never persisted locally. `load_data_into_restored_ambient_cloud_mode_view` now returns the parent's local `AIConversationId` so both `&mut self` call sites can drive the new seeding entry point without re-entering the view. Parents whose task (or a child's task) is still being fetched are held in `pending_parent_child_seeds` and re-driven from the existing `TasksUpdated` subscription. All new behavior is gated behind `OrchestrationUnifiedStack` and degrades to current behavior when `task.children` is empty. Co-Authored-By: Oz <oz-agent@warp.dev>
1. Hoist terminal-surface lookup before the child loop in seed_child_conversations_from_task — logs one warn instead of N, and correctly leaves the entry pending when the surface isn't available rather than retrying per-child on every TasksUpdated. 2. Clean up pending_parent_child_seeds in remove_child_agent_panes so closed parent panes no longer fire the re-drive subscription. 3. Guard RestoreOrNavigateToConversation against double-open: if the conversation already has a terminal surface, discard the extra loading pane instead of loading the transcript onto a second surface. Co-Authored-By: Oz <oz-agent@warp.dev>
Co-Authored-By: Oz <oz-agent@warp.dev>
Proposes replacing DB-persisted parent restore (TECH-parent-restore.md Fix 1/Fix 2) with a restore path driven entirely by server task data, addressing BUG-2 and removing the stale-parent reconciliation needed under persistence. Co-Authored-By: Oz <oz-agent@warp.dev>
…gent conversations Instead of persisting the parent cloud agent conversation to give it a stable local ID across restarts, drive restore entirely from server task data. The pane snapshot's task_id is the stable anchor; transcript and child conversations are seeded on restore via server APIs already wired on the client. Removes: - is_owned_cloud_agent_conversation + write_updated_conversation_state carve-out (Fix 1) - RestoreOrNavigateToConversation arm in ambient_pane_restoration (now unreachable) - ensure_remote_child_conversation stale-parent reconciliation (BUG-1 fix, no longer needed) Modifies: - seed_child_conversations_from_task (Fix 2): repoints from task.children (server never returns it) to list_ambient_agent_tasks(ancestor_run_id=task_id), the same query the OVM ancestor seed uses successfully Co-Authored-By: Oz <oz-agent@warp.dev>
The live-session rejoin path creates a fresh viewer conversation ID each time it joins. Children persisted from primary-mode operation retain their previous parent ID, so the pill bar finds nothing when the pill bar queries children_by_parent[new_id]. Re-index the child under the new parent when the recorded parent differs. This fix is needed even under the task-driven restore approach: task-driven eliminates stale pointers for completed-session restore (no persistence), but not for live-session restore where children are still persisted. Co-Authored-By: Oz <oz-agent@warp.dev>
Remote child conversations are re-created from server task data on every restore via seed_child_conversations_from_task (ancestor_run_id query). Persisting them produces stale parent_conversation_id pointers that require reconciliation on each live-session rejoin — simpler not to persist at all. Also reverts the stale-parent reconciliation in ensure_remote_child_conversation, which is no longer needed since no stale DB entries can accumulate. Co-Authored-By: Oz <oz-agent@warp.dev>
Reverts the non-persistence of remote children (previous commit): without DB-persisted children the pill bar renders empty on restore, because the async ancestor_run_id seed hasn't returned yet when the first render fires. Instead, fix the root cause properly: - index_child_conversation now removes the child from its old parent entry before inserting under the new parent, preventing double-listing after a live-session rejoin. - ensure_remote_child_conversation re-calls set_parent_for_conversation when the persisted parent ID differs from the requested one (same guard as before, but now the underlying index stays consistent). Co-Authored-By: Oz <oz-agent@warp.dev>
Traces the notification chain from finish_seed_child_conversations_from_task through ensure_remote_child_conversation to OrchestrationPillBar's model subscription, and documents why the current implicit reliance on StartedNewConversation is fragile. Recommends adding ConversationServerTokenAssigned to the pill bar's subscription as the minimal, robust fix, with parallelizing the ancestor-list fetch as an optional follow-on latency improvement. Co-Authored-By: Oz <oz-agent@warp.dev>
…hildren Two changes that together make the task-only restore approach work: 1. OrchestrationPillBar now re-renders on ConversationServerTokenAssigned. This is the event emitted when assign_run_id_for_conversation links a remote child to its run id — i.e. when seed_child_conversations_from_task creates the child conversation after the ancestor-list fetch returns. StartedNewConversation was incidentally triggering a re-render for new children, but fell silent for the idempotent case where the conversation already existed. ConversationServerTokenAssigned fires in both cases. 2. Remote child placeholder conversations are no longer persisted to DB. With the notification fix in place, seed_child_conversations_from_task always restores the children_by_parent index after restore, making DB persistence of child conversations unnecessary. Not persisting avoids stale parent_conversation_id pointers (which accumulate because the parent is also not persisted and gets a fresh ID on each live-session join). Co-Authored-By: Oz <oz-agent@warp.dev>
Reverts the non-persistence of remote children — this caused problems. Children are needed in DB for immediate pill bar rendering at startup (before the async ancestor-list seed returns) and for child pane restore. The pill bar ConversationServerTokenAssigned fix from the previous commit is kept: this ensures the pill bar re-renders after the seed completes for both the cold-start case (new conversations) and the idempotent case (existing conversations found via conversation_id_for_agent_id). Also restores the stale-parent reconciliation in ensure_remote_child_conversation: needed because children are persisted with the viewer's previous parent conversation ID, which changes on each live-session rejoin. Co-Authored-By: Oz <oz-agent@warp.dev>
Traces every place that currently depends on is_remote_child=true rows being persisted to DB: initialize_historical_conversations' synchronous bulk-load at boot (the sole source of children_by_parent + task_id for local parents), child_conversation_ids_of callers, the EnteredAgentView-triggered restore_missing_child_agent_panes_for_parent path (no pending/retry, unlike the ambient path), RestoredAgentConversations (a lazy DB-backed cache, not a discovery mechanism), and confirms the conversation list already excludes children independent of persistence. Root-causes the local-agent/remote-children regression: local parents never call seed_child_conversations_from_task today, so removing child persistence leaves no discovery mechanism for that scenario. Recommends wiring the same ancestor-list seed into the local-parent restore trigger, unifying local and ambient-parent restore onto one mechanism. Co-Authored-By: Oz <oz-agent@warp.dev>
Unifies local-parent and cloud-agent-parent restore onto the same ancestor-list-seed mechanism: - restore_missing_child_agent_panes_for_parent now calls seed_child_conversations_from_task when a parent has no locally-known children but does have its own task_id, matching the discovery path already used for ambient/viewer parents. - write_updated_conversation_state no longer persists is_remote_child conversations; they are always rediscovered on restore via the ancestor-list seed, so a persisted row only risked going stale. - ensure_remote_child_conversation's stale-parent reconciliation branch is reverted to a simple early return: with no persisted child rows to reconcile against a prior session's parent id, the mismatch it handled can no longer occur. Co-Authored-By: Oz <oz-agent@warp.dev>
…child is set start_new_child_conversation unconditionally persisted the freshly-created child conversation before ensure_remote_child_conversation's follow-up calls (mark_conversation_as_remote_child, assign_run_id_for_conversation) had a chance to set is_remote_child/run_id. This wrote a garbage DB row with is_remote_child=false and run_id=None, which then permanently blocked all subsequent, correct persists of that conversation via the is_remote_child guard in write_updated_conversation_state. On restart, initialize_historical_conversations loaded this garbage row, and restore_missing_child_agent_panes_for_parent tried to materialize a child pane from it with no task_id, producing an empty compose pane instead of the real transcript. Gate the initial persist behind OrchestrationUnifiedStack: under the unified stack, ensure_remote_child_conversation always follows up with assign_run_id_for_conversation (which persists correctly-tagged remote children, and is a no-op for the ones that must stay unpersisted), so the premature persist is unnecessary. Flag-off local child agents have no such follow-up call and still persist immediately as before. Co-Authored-By: Oz <oz-agent@warp.dev>
…_remote_child is set" This reverts commit 2c9aedd.
Adds an is_remote parameter to start_new_child_conversation, applied before its first persist_conversation_state call, so a remote child's very first potential DB write already has is_remote_child=true and is correctly skipped by the guard in write_updated_conversation_state. Previously is_remote_child was only set by a separate, later call (mark_conversation_as_remote_child), so the first persist could race ahead and write a garbage row (is_remote_child=false, run_id=None) that then permanently blocked all later, correct persists of that conversation via the same guard. Updates all four call sites: ensure_remote_child_conversation and launch_remote_child (both remote children) now pass is_remote=true; create_hidden_child_agent_conversation and the flag-off viewer child discovery path pass is_remote=false. launch_remote_child is the live run_agents dispatch path and had the identical bug pattern independent of ensure_remote_child_conversation, so it needed the same fix. Co-Authored-By: Oz <oz-agent@warp.dev>
restore_missing_child_agent_panes_for_parent's local-parent ancestor-list seed trigger fired unconditionally whenever child_ids was empty. Since finish_seed_child_conversations_from_task calls this same function after every seed completion (including ones that legitimately resolve to zero children), a parent with no children caused it to immediately re-seed, whose completion re-called this function, looping indefinitely. Adds a trigger_seed_if_empty parameter: true from entry points that are not downstream of a seed completion (EnteredAgentView, restore_missing_child_agent_panes_for_terminal_pane_if_needed, ensure_hidden_child_agent_pane_for_conversation), false from finish_seed_child_conversations_from_task itself, so a zero-children result there does not re-trigger its own seed. Co-Authored-By: Oz <oz-agent@warp.dev>
Co-Authored-By: Oz <oz-agent@warp.dev>
restore_missing_child_agent_panes_for_parent's local-parent ancestor-list seed only fired when child_ids was empty. A parent with both persisted local children (still loaded via initialize_historical_conversations) and not-yet-discovered remote children has a non-empty child_ids from the local children alone, so the seed never fired and the remote children were never discovered. Trigger the seed whenever none of the currently-known children are remote (child_ids empty, or all local), rather than only when child_ids is empty outright. Once the seed discovers at least one remote child, this naturally stops re-firing. Co-Authored-By: Oz <oz-agent@warp.dev>
Design specs are kept at ~/src/child-agent-started-events/ outside the repo to keep the M2 diff free of spec noise. Co-Authored-By: Oz <oz-agent@warp.dev>
Co-Authored-By: Oz <oz-agent@warp.dev>
…sisted Two related fixes to restore pre-flag behavior for the flag-off path: 1. write_updated_conversation_state: gate the is_remote_child early-return behind OrchestrationUnifiedStack so remote children ARE persisted when the flag is off. Previously this guard prevented assign_run_id_for_conversation from saving the run_id, causing task_id=None on restore -> empty transcript and Unknown attribution. 2. launch_remote_child: pass is_remote=false when the flag is off, so start_new_child_conversation does not call mark_as_remote_child before the first persist. Co-Authored-By: Oz <oz-agent@warp.dev>
Contributor
Author
|
Demo: https://www.loom.com/share/5feec7c256044f03a5d63d2c46fe4a75 Demonstrates: pill bar restore after cloud agent restart, local parent with remote child restore, and flag-off behavior verified against pre-M1 baseline. |
Master added execution_location to AmbientAgentTask after M2 branched. CI tests the merge commit (M2 + master), so the struct literals in our tracker and streamer tests need this field. Co-Authored-By: Oz <oz-agent@warp.dev>
2 tasks
Member
|
Heads-up on a merge-order interaction (posted by Daniel's Warp Agent on his behalf): #14762 (multi-level orchestration: depth-capable client core + drill-down pill bar) is landing ahead of this PR, and it rewrites the pill-bar render region this branch touches — the visible row now renders direct children only, with deeper levels reached by drilling into a pill (replacing the flattened What we did to keep your rebase trivial: we absorbed this branch's How to resolve when you rebase
One re-verification note: the manual-testing assumptions here ("pill bar appears after restart" etc.) were made against the flattened-descendants rendering. Post-rebase, restored children appear under drill-down semantics — worth a quick re-check that the restore-seeded pills show up at the level you expect. The deeper interaction points (unified-stack flag-ON pane path vs the |
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
3 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.
Description
Unifies the orchestration child pane path and moves parent/child restore onto task data (QUALITY-928).
What
TaskOwnership,TaskScope, andChildPaneOriginare removed along with the branching they supported.OrchestrationViewerModelnow usesAgentConversationsModelas its fetch authority, routing child discovery throughpending_task_ids_for_discovery.GET /agent/runs?ancestor_run_id=, the same query the OVM uses live.ConversationServerTokenAssigned, ensuring correct notification after the async restore seed completes.restore_missing_child_agent_panes_for_parenttriggers the server seed to discover remote children.finish_seed_child_conversations_from_taskdoes not re-trigger the seed on completion (trigger_seed_if_empty=false), preventing an infinite loop.Why
The parallel owner/viewer hydration paths had accumulated code duplication and subtle behavioral differences. Consolidating around task data as the authoritative source simplifies restore and makes behavior consistent across native and web clients.
Linked Issue
QUALITY-928
Testing
Manual: pill bar appears after restart for a completed cloud agent run.
Manual: local parent with remote children restores correctly after restart.
Unit coverage for child pane materialization and task-driven restore seeding.
./script/formatandcargo clippy --workspace --exclude warp_completer --all-targets --tests -- -D warningsclean.I have manually tested my changes locally with
./script/runAgent Mode