›_ Codex Issues
Codex Issues / #33468

Stale MultiAgentV2 branch in legacy completion watcher obscures the active completion path

Open 💬 2 comments Opened Jul 16, 2026 by StatPan

Summary

The legacy subagent completion watcher still contains a MultiAgentV2 delivery branch, even though normal V2 spawn/resume paths intentionally do not start that watcher. The active V2 completion path now lives in the session terminal-turn lifecycle instead.

This leaves a semantically stale branch and a direct unit test that can make changes appear correct without exercising production V2 routing.

Current behavior on main

  • AgentControl::maybe_start_completion_watcher contains a child_uses_multi_agent_v2 branch that sends an InterAgentCommunication message.
  • Normal V2 spawn and resume paths skip maybe_start_completion_watcher.
  • V2 child completion is actually forwarded by the session terminal-turn path (maybe_notify_parent_of_terminal_turn / forward_child_completion_to_parent).
  • multi_agent_v2_completion_queues_message_for_direct_parent calls the private watcher directly, so it covers a synthetic path rather than the normal V2 lifecycle.

I confirmed the mismatch by changing only the watcher's V2 trigger_turn behavior: the direct watcher unit test passed, while an integration test that spawned a real V2 child and completed its turn did not observe the corresponding automatic parent turn. The integration path only changed behavior when the session terminal-turn forwarding path was changed.

The watcher itself is not dead code: V1 still uses it. The concern is specifically the V2 branch inside the legacy watcher and the misleading V2 unit coverage around it.

Why this matters

  • It is easy to patch the wrong completion path when investigating V2 notification or wake-up bugs.
  • The watcher and session implementations can drift in delivery semantics such as trigger_turn.
  • A passing direct unit test can give false confidence while the production V2 path remains unchanged.
  • This obscures work on #15723, where the desired wake-up behavior needs to be implemented on the active path.

Related history and issues

  • #15570 introduced the custom MultiAgent V2 watcher.
  • #16308 added one-shot end-of-turn forwarding in the session lifecycle, which is now the active V2 completion path.
  • #15723 requests durable completion notification and wake-up behavior.
  • #33061 reports the opposite failure mode: a late subagent completion causing an extra user-visible response even with trigger_turn: false.

Proposed direction

  1. Make maybe_start_completion_watcher explicitly V1-only and remove its V2-specific branch, arguments, imports, and direct V2 watcher test where they are no longer needed.
  2. Replace the synthetic V2 watcher test with integration coverage that exercises spawn -> child terminal turn -> parent delivery through the session path.
  3. If V1 and V2 need shared code, share construction of a bounded, idempotent completion receipt rather than duplicating lifecycle ownership.
  4. Treat wake-up policy as a separate decision from this cleanup. A possible policy is:
  • enqueue one durable terminal receipt;
  • wake only when the parent is idle and has expressed that it is waiting for child work;
  • defer or coalesce while the parent is busy;
  • avoid unconditional trigger_turn: true, given the duplicate-response scenario in #33061.

Design questions

  • Should completion wake-up depend on explicit parent wait/join state rather than a fixed trigger_turn value?
  • Should V1 watcher delivery and V2 session delivery share a CompletionReceipt abstraction while keeping their lifecycle triggers separate?

View original on GitHub ↗

2 Comments

StatPan · 1 month ago

I reproduced this as an A/B path test on c7f21a852cc35c66b7936963cab2c80d6041a09c.

Static call-path evidence

Dynamic test setup

I added a local codex-core integration test named multi_agent_v2_completion_starts_follow_up_turn_when_parent_idle using the existing mock response helpers:

  1. Parent response calls the V2 spawn_agent tool.
  2. Child response completes with child done after a 300 ms delay.
  3. Parent consumes the spawn result and finishes before the child completion arrives.
  4. A third mock /responses request waits up to 6 seconds for an agent_message containing the child's FINAL_ANSWER envelope.

A/B result

| Experiment | Watcher trigger_turn | Session terminal path trigger_turn | Result |
|---|---:|---:|---|
| A: change only the legacy watcher and update its direct unit expectation | true | false | direct watcher unit test PASS; real V2 integration test FAIL twice with automatic follow-up request was not sent |
| B: restore the watcher and change the session terminal path | false | true | the same real V2 integration test PASS |

Commands and observed summaries:

$ just test -p codex-core multi_agent_v2_completion_queues_message_for_direct_parent
PASS codex-core agent::control::tests::multi_agent_v2_completion_queues_message_for_direct_parent

$ just test -p codex-core multi_agent_v2_completion_starts_follow_up_turn_when_parent_idle
TRY 1 FAIL ... Error: automatic follow-up request was not sent
TRY 2 FAIL ... Error: automatic follow-up request was not sent

# after moving the one-line trigger change to Session::forward_child_completion_to_parent
$ just test -p codex-core multi_agent_v2_completion_starts_follow_up_turn_when_parent_idle
PASS codex-core::all suite::subagent_notifications::multi_agent_v2_completion_starts_follow_up_turn_when_parent_idle

This demonstrates the routing/test-coverage mismatch. It does not establish that unconditional trigger_turn: true is the correct product policy; #33061 is still an important counterexample for that design decision. I can publish the integration-test PoC on a fork branch as a follow-up so the exact test can be fetched and run unchanged.

StatPan · 1 month ago

Scope correction: the previously linked completion-wake branch belongs to the behavior discussion in #15723, not to this watcher-cleanup issue. The runnable PoCs are now split.

#33468 cleanup PoC

This branch contains no wake-policy or session-terminal-path change. It only:

  1. renames the watcher and documents it as legacy/V1-only;
  2. makes fresh and resumed watcher attachment explicitly MultiAgentVersion::V1;
  3. removes the stale V2 branch and its child_agent_path plumbing;
  4. removes the synthetic V2 test that directly invoked the private watcher; and
  5. strengthens the missing-child regression test for a legacy child that has an agent path.

The last point captures a nuance found during cleanup: the stale branch is skipped by normal V2 spawn/resume, but it was still reachable when the watcher could not look up a child because None defaulted to “uses V2.” Since watcher attachment is already V1-only, that fallback could misclassify a missing legacy child and send the wrong envelope type.

Validation:

just test -p codex-core legacy_completion_watcher_notifies_parent_when_child_is_missing
just test -p codex-core spawn_child_completion_notifies_parent_history
just test -p codex-core plaintext_multi_agent_v2_completion_sends_agent_message
just test -p codex-core multi_agent_v2_completion_ignores_dead_direct_parent
just fix -p codex-core
just fmt

All five executed test cases passed (the plaintext V2 test covers both completed and terminal-error scenarios), and git diff --check passed. No upstream PR was opened.

The separate wake experiment is tracked under #15723 and is intentionally not part of this branch.