[QUALITY-772] Use ancestor streams for large orchestrators#12209
Conversation
Switch owner-side orchestration event delivery to a parent-family ancestor stream when the dogfood flag is enabled, while preserving child-only and viewer-mode stream behavior. Co-Authored-By: Oz <oz-agent@warp.dev>
…ents 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 |
![oz-for-oss[bot]](https://avatars.githubusercontent.com/in/3450139?s=60&v=4){kind=small}
There was a problem hiding this comment.
Overview
This PR adds a dogfood-gated owner-side ancestor SSE stream (ancestor_run_id&include_self=true) to avoid the explicit run_ids[] limit for large orchestrations, keeps viewer streams child-only, and adds regression tests for parent/child filtering and limit behavior. I did not find security-specific issues in the changed client-side URL/filter handling.
Concerns
- Oversized parent streams with the flag disabled set an internal
sse_delivery_blockedbit, but the diff does not read that bit or emit any event/UI state from it, so the claimed visible delivery failure is not actually surfaced to users. - This changes user-perceivable orchestration event delivery, but the PR description has no screenshot or recording and manual local testing is unchecked. Please attach a short screen recording showing a parent orchestrator receiving child lifecycle/message events through the new path end to end.
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
| OwnerOrchestrationAncestorStreamer to deliver events for large orchestrators" | ||
| ); | ||
| if let Some(stream) = self.streams.get_mut(&conversation_id) { | ||
| stream.sse_delivery_blocked = true; |
There was a problem hiding this comment.
Remove misleading private delivery-blocked state and tighten comments around owner ancestor streaming. Co-Authored-By: Oz <oz-agent@warp.dev>
| return DesiredSseFilter::NoFilter; | ||
| } | ||
| if is_parent && run_ids.len() > MAX_RUN_ID_STREAM_FILTER { | ||
| return DesiredSseFilter::UnsupportedRunIdCount(run_ids.len()); |
There was a problem hiding this comment.
OOC, why not always use the ancestor stream approach (as opposed to only using it when we have > n streams such that the number is no longer supported)
There was a problem hiding this comment.
We actually always do use it unless the flag is off - we'll exit at line 1582. This is just a temporary stop gap for cases where the flag is still off and we exceed 100.
cephalonaut
changed the title
2 participants
Description
What
OwnerOrchestrationAncestorStreamerfeature flag.ancestor_run_id&include_self=truestream when the flag is enabled.RunIds(self)and viewer-mode streams oninclude_self=false.run_ids[]streams from retrying forever when the flag is disabled.Why
Large orchestrations can exceed the server's 100 explicit-run-id SSE limit, causing parents to miss child lifecycle/message events. The parent-family ancestor stream keeps delivery to one ordered stream while preserving the existing cursor model.
How
AgentEventFilter::AncestorRunIdwith aninclude_selffield.include_self=trueonly when requested.Linked Issue
ready-to-specorready-to-implement.No linked issue; see companion server TECH spec at
warp-server/specs/QUALITY-790/TECH.md.Testing
./script/formatPATH=/usr/local/bin:$PATH cargo clippy --workspace --all-targets --all-features --tests -- -D warningscargo test -p warp --lib run_id_limit_without_flagcargo test -p warp --lib parent_with_many_children_opens_one_ancestor_include_self_streamcargo test -p warp --lib restored_parent_with_children_opens_ancestor_include_self_streamI have manually tested my changes locally with
./script/runAgent Mode
CHANGELOG-NONE
Co-Authored-By: Oz oz-agent@warp.dev