feat(orchestration-v2): attribute reused subagents to the run driving them (3/5) - #4662
feat(orchestration-v2): attribute reused subagents to the run driving them (3/5)#4662shivamhwp wants to merge 1 commit into
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
github-actions
Bot
added
vouch:trusted
… them A subagent row keeps the run id it was spawned under. Routing admits a row by run or by parent thread, and the parent thread is excluded by construction, so a later run re-activating that subagent could only see its updates while the spawning run's ingestion fiber happened to still be alive. The Claude adapter already re-attributed on reopen for exactly this reason; this brings the same treatment to routing and to Codex. Reuse arrives on item/completed, not item/started: resumeAgent and sendInput only ever complete, and registerSubagentThreads ignores them because they are not spawnAgent. Hooking the rebind there is what makes it fire — subagent_continue now asserts the subagent is attributed to the run that drove it, where before it stayed pinned to the spawning run. Routing gains a third test: subagent identity, seeded from the projection at turn start. The child thread is adopted only while the agent is live, because a settled row is also re-emitted by trailing traffic — a token usage frame or a collab state sweep — and adopting on those would hand a later run an interrupted agent's thread, readmitting the stale events that post-interrupt recovery exists to exclude. A subagent's row and its timeline item are emitted together, so both route by identity; matching only the row advanced the agent while its item stayed frozen on the spawning run. The reactivation predicate is separate from canRouteRelatedSubagent rather than a widening of it, for the same reason: an interrupted agent must not have its child thread pre-owned, but its identity is still resumable, because the user stopped it rather than losing it. Rehydration seeds turn ordinals alongside the registry — restarting them at 1 would re-derive childRootNodeId for a different native turn — and parks non-terminal leftovers at idle, since a new session can never terminalize an activation it did not drive and would otherwise keep the session pinned open. Not proven: rehydration itself. The fixture that would exercise it is recorded but does not replay to completion, for reasons unrelated to these changes, and is left unregistered with the open question written up in fixtures/index.ts.
9296337 to
760a4c2
Compare
5187d5b to
cde2598
Compare
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
There was a problem hiding this comment.
🟡 Medium
A reused subagent's turn_item.updated event passes the new ownsSubagent route check but trackChildLifecycle only considers belongsToRootRun = event.turnItem.runId === input.run.id for the subagent-item branch, so the item is never inserted into openRunOwnedSubagents.turnItems. When the reactivating run is interrupted or fails, the cascade terminalizes the subagent row and activation but leaves this timeline item non-terminal, so it stays visibly stuck in the projection. Extend the root-ownership test in trackChildLifecycle to include routing.ownedSubagentIds.has(event.turnItem.subagentId) (matching the routing logic).
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @apps/server/src/orchestration-v2/RunExecutionService.ts around line 775:
A reused subagent's `turn_item.updated` event passes the new `ownsSubagent` route check but `trackChildLifecycle` only considers `belongsToRootRun = event.turnItem.runId === input.run.id` for the subagent-item branch, so the item is never inserted into `openRunOwnedSubagents.turnItems`. When the reactivating run is interrupted or fails, the cascade terminalizes the subagent row and activation but leaves this timeline item non-terminal, so it stays visibly stuck in the projection. Extend the root-ownership test in `trackChildLifecycle` to include `routing.ownedSubagentIds.has(event.turnItem.subagentId)` (matching the routing logic).
| sandboxPolicy: workspaceWriteSandbox(), | ||
| }, | ||
| steps: [ | ||
| { |
There was a problem hiding this comment.
🟡 Medium scripts/record-codex-app-server-replay-fixture.ts:467
The subagent_reuse_after_interrupt scenario hangs indefinitely during recording. When interruptAfterCommandExecutionStarted is set, runTurnStep waits on a commandExecution notification keyed by the root turn's id. But the command that the fixture prompt asks the subagent to run (sleep 30) is reported with the child subagent's turn id, not the root turn id. So the deferred for the root turn is never completed, and the recorder blocks before sending turn/interrupt and never reaches the reuse step. Consider keying startCommandExecution (or the wait condition) on the spawned subagent's turn id, or using item/started/item/completed notifications from the child turn rather than the root turnId.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @apps/server/scripts/record-codex-app-server-replay-fixture.ts around line 467:
The `subagent_reuse_after_interrupt` scenario hangs indefinitely during recording. When `interruptAfterCommandExecutionStarted` is set, `runTurnStep` waits on a `commandExecution` notification keyed by the root turn's id. But the command that the fixture prompt asks the subagent to run (`sleep 30`) is reported with the child subagent's turn id, not the root turn id. So the deferred for the root turn is never completed, and the recorder blocks before sending `turn/interrupt` and never reaches the reuse step. Consider keying `startCommandExecution` (or the wait condition) on the spawned subagent's turn id, or using `item/started`/`item/completed` notifications from the child turn rather than the root `turnId`.
1 participant
Third of five stacked PRs replacing #4551. Stacked on #4629 — review that first.
A subagent row keeps the run id it was spawned under. Routing admits a row by run or by parent thread, and the parent thread is excluded by construction — so a later run re-activating that subagent could only see its updates while the spawning run's ingestion fiber happened to still be alive. The Claude adapter already re-attributed on reopen for exactly this reason; this brings the same treatment to routing and to Codex.
The bug that made this invisible
The rebind was hooked to
item/started. Codex's reuse frames —resumeAgent,sendInput— only ever arrive onitem/completed, andregisterSubagentThreadsignores them because they aren'tspawnAgent. So the rebind never fired, and I could not reproduce the fix working.Moving the hook is what makes it observable.
subagent_continuenow asserts the subagent is attributed to the run that drove it:subagent.runIdordinal:1(spawning run)ordinal:2(run that drove the reuse)Routing
Identity becomes a third ownership test, seeded from the projection at turn start. Two constraints shape it:
That is also why the reactivation predicate is separate from
canRouteRelatedSubagentrather than a widening of it: an interrupted agent must not have its child thread pre-owned, but its identity is still resumable — the user stopped it, they didn't lose it.Rehydration
Seeds turn ordinals alongside the registry (restarting at 1 would re-derive
childRootNodeIdfor a different native turn) and parks non-terminal leftovers atidle, since a new session can never terminalize an activation it didn't drive and would otherwise keep the session pinned open.Not proven. The fixture that would exercise rehydration is recorded and contains a genuine reuse, but doesn't replay to completion — for reasons unrelated to these changes; the hang reproduces with them reverted, and persists now that the rebind demonstrably fires. Left unregistered with the open question written up in
fixtures/index.ts.Testing
Typecheck clean across server, contracts, client-runtime, web. Server 1,684 passing.
Reviewed by an independent agent pass, which found the
item/started/item/completedbug above plus the child-thread adoption hole, the row/item routing asymmetry, and both rehydration gaps. All fixed here.Note
Attribute reused subagents to the run reactivating them in orchestration-v2
CodexAdapterV2by rebuilding the process-local registry fromexistingSubagentsand rebinding them to the active run on collab agent interactions.RunExecutionServiceto match subagent rows, activations, and turn items by identity (not just run/thread), so reused subagents are admitted and tracked correctly.canReactivateSubagentto allow reactivation of idle/interrupted subagents; failed/cancelled are excluded. Classifiesidleas a terminal status for cascade/routing purposes.currentActivationIdviacascadeTerminalizeRunOwnedSubagents.subagent_reuse_after_idlescenario; the fixture is scaffolded but not yet registered in the test suite due to a replay hang.currentActivationIdis null, replacing the removedisSettledCodexSubagentcheck.📊 Macroscope summarized cde2598. 10 files reviewed, 0 issues evaluated, 0 issues filtered, 0 comments posted
🗂️ Filtered Issues
No issues evaluated.