QUALITY-919: register orchestrators for child events on wait_for_events#13208
QUALITY-919: register orchestrators for child events on wait_for_events#13208cephalonaut wants to merge 3 commits into
Conversation
Children created out-of-band (Oz CLI / web API) by passing parent_run_id directly never call register_watched_run_id, so the parent's client never learns it is a parent and misses its children's owner-side events. Use the wait_for_events tool — the moment an orchestrator blocks on its descendants — to confirm parent status against the server and register for the ancestor stream. - Add dogfood-gated FeatureFlag WaitForEventsParentRegistration (enum + DOGFOOD_FLAGS in warp_features, Cargo feature, enabled_features bridge). - Add OrchestrationEventStreamer::register_parent_on_wait with the spec's decision flow (flag check, child short-circuit, already-parent short-circuit, self_run_id resolve, get_ambient_agent_task), plus a shared apply_task_children helper reused by the restore path. - Invoke it from wait_for_events.rs::execute() via the streamer handle. - Unit + executor tests covering every case in the spec's Testing section. - Check in specs/QUALITY-919/TECH.md. Co-Authored-By: Oz <oz-agent@warp.dev>
|
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 |
…nt_sequence register_parent_on_wait now short-circuits on is_remote_run_view so passive shared-session / remote-child views do not register or issue a wasted fetch (mirrors is_eligible). The spec documents that last_event_sequence is the client's confirmed-processing (delivery) cursor — NULL means replay from 0, so a first-time-registering parent still receives pending child events — and reflects the remote-view guard in the decision flow and sketch. Co-Authored-By: Oz <oz-agent@warp.dev>
There was a problem hiding this comment.
Overview
This PR adds a dogfood-gated wait_for_events registration path that fetches the current run's server-recorded children, marks the conversation as a parent, and reuses the existing orchestration SSE machinery to deliver child lifecycle and inbox events. It also factors child/cursor application into a shared helper and adds focused unit coverage for the new streamer and executor behavior.
Concerns
- The wait-time registration path inserts server-reported child run IDs without ensuring the parent run ID is watched. That works for the ancestor stream, but if
OwnerOrchestrationAncestorStreameris disabled while this new flag is enabled, the fallback explicitRunIdsstream watches only children and drops the parent's own inbox events. The existingregister_watched_run_idpath avoids this by callingensure_self_run_id_watched.
Verdict
Found: 0 critical, 1 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
| conversation_id: AIConversationId, | ||
| task: &crate::ai::ambient_agents::task::AmbientAgentTask, | ||
| base_cursor: i64, | ||
| ) { |
There was a problem hiding this comment.
WaitForEventsParentRegistration is enabled while OwnerOrchestrationAncestorStreamer is disabled, desired_sse_filter falls back to RunIds(watched_run_ids) and this stream watches only children, so parent-directed inbox events are missed; mirror register_watched_run_id by ensuring self_run_id is present before reevaluating eligibility.
There was a problem hiding this comment.
Fixed though likely moot when OwnerOrchestrationAncestorStreamer flag is removed.
Mirror register_watched_run_id so the parent's own inbox is delivered even when desired_sse_filter falls back to RunIds (OwnerOrchestrationAncestorStreamer disabled, where the filter would otherwise watch only children). Adds a RunIds-fallback unit test. Addresses the Oz review finding on PR #13208. Co-Authored-By: Oz <oz-agent@warp.dev>
Description
Orchestrators that spawn children out-of-band (Oz CLI / web API, passing
parent_run_id) never registered as a parent on the client, so they missed their children's lifecycle and inbox events. This change useswait_for_events— the moment an orchestrator blocks on its descendants — to confirm parent status with the server and subscribe to the owner-side ancestor stream.What:
FeatureFlag::WaitForEventsParentRegistrationgating the behavior.wait_for_events, a non-child root asks the server whether it has children and, if so, registers for theinclude_selfancestor stream (which then tracks all children, including out-of-band ones). Already-registered parents and passive/remote views are skipped.specs/QUALITY-919/TECH.md.Why:
Closes the event-delivery gap so an orchestrator receives its children's events regardless of how they were created. No user-visible behavior change; gated behind a dogfood flag for safe rollout.
Testing
cargo nextest run -p warp(8/8);cargo fmtandcargo clippy --all-targets -- -D warningsclean.Agent Mode
CHANGELOG-NONE
Conversation: https://staging.warp.dev/conversation/ea1d8c5d-964d-4752-9570-0124af5416bc
Run: https://oz.staging.warp.dev/runs/019f15a4-3b17-78c3-8a8f-5ab8b23df398
This PR was generated with Oz.