[AI-1861] Post live subagent-stop from Codex collab child watchers - #526
Merged
Conversation
A Codex collab child's SubagentCompleted was written only by the parent's session-end teardown (CodexSubagentTeardown), so on a long-lived parent — a hosted reviewer running for hours across rounds — every finished child's chat card spun for the parent's whole lifetime (verified live: 8 SubagentStarted, 0 SubagentCompleted on the diagnosed session). Codex fires no per-child stop hook and sub_agent_activity has no completed kind; the deterministic per-turn signal is the child rollout's own event_msg.task_complete, which the child watcher already tails. The new CodexSubagentTurnTracker folds that turn state (task_complete sets completed; any response_item or task_started re-opens it — re-engagement; trailing event_msg noise leaves it alone), and the child watcher's polling loop posts /hooks/subagent-stop once the turn is complete, no tool call is in flight, and an idle grace elapsed (KCAP_CODEX_SUBAGENT_IDLE_MINUTES, default 5m, 0 = immediate — sized to absorb the observed 2-6 min same-round re-engagement gaps). One-shot by design: the server dedupes lifecycle events per (session, agent), so the tracker latches after the first successful POST and the parent-end teardown remains the backstop, its duplicate stop deduped the same way. A child re-engaged after the grace still streams into its subsession (appends are not lifecycle-gated); its card stays completed with duration frozen — accepted trade-off. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
PR Summary by QodoPost live subagent-stop from Codex collab child watchers
AI Description
Diagram
High-Level Assessment
Files changed (5)
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 66430829af
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Comment on lines
+1700
to
+1702
| if (vendor == "codex" && agentId is not null) { | ||
| foreach (var line in newLines) { | ||
| state.CodexSubagentTurn.Observe(line); |
There was a problem hiding this comment.
When a child watcher restarts after the batch containing task_complete was acknowledged but before the grace-period stop was posted, WatcherConnect resumes from the server watermark and DrainNewLines returns only later lines. Because the fresh tracker observes only newLines here, it never sees the already-acknowledged completion and therefore never posts the live stop; the child card remains in progress until the parent eventually ends. Reconstruct the tracker from the acknowledged prefix, or otherwise persist its completion state when resuming.
Useful? React with 👍 / 👎.
Code Review by Qodo
1.
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
… restart Review fix: a restarted child watcher resumes from the server watermark (WatcherConnect), so DrainNewLines never re-delivers already-acknowledged lines — including the task_complete that should drive the live stop. A child watcher dying between that ack and the grace-delayed stop POST left a fresh tracker permanently disarmed, spinning the card until the parent-end teardown. SeedCodexSubagentTurnState now folds the rollout's full on-disk prefix through both the turn tracker and the pending-call set at child watcher startup; the first drain re-observing the unacknowledged suffix is harmless (both folds converge on the same last-state), and an unreadable rollout degrades to the pre-seed behavior with the teardown as backstop. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Member
Author
|
Addressed the reviewer finding about restart recovery in 7ddcb38: a restarted child watcher resumes from the server watermark ( |
…README - Strip the Linear issue token from all new C# comments (repo rule + the lint-linear-ids CI check; references live in the PR/commit metadata). - Bound one live child-stop POST attempt to 5s overall (linked CTS across auth client creation + the POST, and an explicit PostWithRetryAsync timeout): the main loop touches the watcher heartbeat once per iteration and awaits this call inline, so the previous default 30s retry budget could blow past the 20s staleness threshold and churn the watcher exactly at turn completion. Retries ride later loop iterations instead. - Space failed stop attempts 60s apart (was: every 1s tick) and log each failure with the HTTP status / exception message. Deliberately no terminal give-up: an auth refresh can heal, and the parent-end teardown dedupes whatever never posts. - Document KCAP_CODEX_SUBAGENT_IDLE_MINUTES in the README's Codex session-end tuning table. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This was referenced Aug 11, 2026
alexeyzimarev
added a commit
that referenced
this pull request
Aug 11, 2026
) * Feed the Codex watcher state trackers raw lines, not redacted ones RedactLine replaces any line over 64 KiB with a placeholder carrying no call_id and a type neither tracker recognises, and an oversized function_call_output — a build log, a large file read, a verbose test run — is routine rather than exceptional. UpdateCodexPendingToolCalls therefore never removed the call_id, leaving PendingCodexToolCalls non-empty for the life of the watcher. toolInFlight was then permanently true and ShouldEndOnIdle could never fire. For Codex that is not a degraded backstop: the desktop app's shared app-server never exits per conversation, so the idle timeout is the ONLY per-conversation session-end path. The session stays Active in the read model indefinitely and the watcher never exits. The same stranded call_id also permanently blocks the live subagent-stop added in #526, which requires no tool call in flight — reinstating the symptom it fixed, a finished child's chat card spinning for the parent's whole lifetime. CodexSubagentTurn.Observe read the same list and fails in the other direction: it treats any response_item as the turn re-opening, so an oversized one is not recognised and a child that re-engaged after task_complete could be reported stopped while still working. Both now read drainRead.Lines, matching what SeedCodexSubagentTurnState already does when it folds this state from disk — the seed was correct and only the live drain path was inconsistent. Neither tracker emits anything; they read call_id and type only, so no unredacted content leaves the process and redaction still governs everything sent to the server. Same defect and same fix as the Claude tracker in #517. Closes #528 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Gate the Codex tool tracker on vendor now that it reads raw lines Both reviewers caught the same regression. The tracker's loop was never gated on vendor — it ran for every watcher on the grounds that a non-Codex line has no response_item and so is a cheap no-op. That held while it read the redacted list, where any oversized line arrived as a ~90-byte placeholder. Reading raw lines removes that bound, so a Claude or Gemini watcher would JsonDocument.Parse a multi-megabyte tool output on every drain to discover, again, that it is not a Codex record. TracksCodexToolCalls mirrors TracksClaudeToolCalls and makes the decision testable rather than an inline condition. Not gated on watcher role: a collab child needs the tracking for ShouldPostSubagentStop. Also trims the comments added in the previous commit, per the repo's keep-comments-minimal rule. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
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
1 participant
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.
Fixes AI-1861 (follow-up to AI-1839 / #515).
Problem
A Codex collab child's
SubagentCompletedwas written only by the parent's session-end teardown (CodexSubagentTeardown), so on a long-lived parent — a hosted reviewer running for hours across review rounds — every finished child's chat card spun for the parent's whole lifetime. Verified live on the diagnosed session: 8SubagentStarted, 0SubagentCompletedon the parent stream.Codex fires no per-child stop hook, and the live signal audit (real 0.148 rollouts) showed:
sub_agent_activitykinds are onlystarted/interacted— no completed kindwait_agentoutputs timed out in every observed call;list_agentsstatus is authoritative but model-drivenevent_msg.task_complete— in the file the child watcher already tailsFix
New
CodexSubagentTurnTracker(Cli.Core, pure) folds the child rollout's turn state:task_completemarks the turn completed; anyresponse_itemortask_startedre-opens it (re-engagement); trailingevent_msgnoise (token_countetc.) leaves it alone. The codex child watcher's polling loop posts/hooks/subagent-stoponce the turn is complete, no tool call is in flight, and an idle grace elapsed —KCAP_CODEX_SUBAGENT_IDLE_MINUTES, default 5m (sized to the observed 2–6 min same-round re-engagement gaps),0= immediate.One-shot by design: the server dedupes lifecycle events per
(session, agent)(AgentLifecycleDeterministicId), so the tracker latches after the first successful POST; a failed POST retries next tick. The parent-end teardown stays as the backstop and its duplicate stop dedupes server-side (doc comment updated — it claimed to be the ONLY finalizer).Accepted trade-off: a child re-engaged after the grace window still streams into its subsession (appends aren't lifecycle-gated) but its card stays completed with duration frozen — mildly wrong in the far less annoying direction than spinning forever. The fuller reactivation-aware (generation-keyed) lifecycle is deliberately out of scope.
Tests
CodexSubagentTurnTrackerTests(15 tests): grace boundary, zero-grace immediate, pending-tool-call suppression, re-engagement re-opening (response_item/task_started), noise immunity, one-shot latching across a latertask_complete, malformed-line safety, env parsing.origin/mainbaseline worktree — 5 vs 12 of the same class) are the known-flakyAgentOrchestratorVendorTeststimer races (Fix telemetry lane-overlap, leftover-token, and consent timer-race CI flakes #525 territory), untouched by this change.Note for the kcap-server submodule bump:
docs/CODEX_NORMALIZER.md's "Session end" section says child watchers are finalized by the parent's session-end synthesis — after this, that's the backstop, not the only path.🤖 Generated with Claude Code