Skip to content

fix(codex): preserve subagent completions after sessions_yield#105724

Merged
steipete merged 2 commits into
openclaw:mainfrom
algal:fix/codex-subagent-announce
Jul 16, 2026
Merged

fix(codex): preserve subagent completions after sessions_yield#105724
steipete merged 2 commits into
openclaw:mainfrom
algal:fix/codex-subagent-announce

Conversation

@algal

@algal algal commented Jul 12, 2026 •
edited by steipete
Loading

Copy link
Copy Markdown
Contributor

Closes #104806

What Problem This Solves

Fixes a P0 message-loss race in the Codex runtime. A parent could accept a subagent completion steer, call sessions_yield, interrupt the turn, and lose that accepted input before Codex consumed it.

Why This Change Was Made

Codex distinguishes RPC acceptance from transcript consumption. turn/steer accepts input into the active turn queue; turn/interrupt clears pending input. The durable consumption signal is the matching completed userMessage item.

This revision keeps each steer pending under an exact clientUserMessageId until Codex emits item/completed for the matching userMessage.clientId. A successful terminal yield cancels unconsumed steering before interrupt, allowing the existing completion fallback to retry on a fresh parent turn. A failed or non-terminal yield leaves steering usable. Run abort closes the queue immediately, so no accepted-but-unconsumed wake can hang through cleanup.

The implementation replaces the earlier FIFO inference with one canonical identity-based path. No config, migration, or shared announce-routing change.

Upstream Codex contract checked at 1d941253e9354fe583a033660a6288df66e27488:

User Impact

Codex-backed orchestrators no longer falsely report a yielded subagent completion as delivered into an interrupted turn. The result reaches the parent exactly once through the normal retry path, and the parent session remains usable.

Evidence

Exact candidate: 5093e8aef1530af1971aa524c85e091f0c3ea8e3

  • focused steering and lifecycle suites: 22/22 passed locally and on Linux Testbox; terminal-yield assertions live at the deterministic lifecycle-controller boundary instead of the cold full-attempt harness
  • Codex extension runtime and test type checks: passed
  • targeted formatting and lint: passed
  • production build: passed at patch-identical pre-rebase head
  • fresh structured autoreview: no actionable findings
  • fresh Testbox production build and live proof at pre-rebase head 278a0aa20496b0af6294153864703f32fef91fba: real Gateway, forced Codex runtime, real sessions_spawn, successful sessions_yield, child completion in the parent transcript exactly once, and a successful second turn in the same session; 224 seconds including the production build. The final rebase changed only the main parent; git patch-id --stable remained 786317cb39b7e49361ce5a5d222b076037b1b116.

Release-note context: Fix Codex subagent completion delivery after sessions_yield. Thanks @algal.

AI-assisted: investigated, rewritten, tested, and drafted with agentic tools. Maintainer reviewed the dependency contract, behavior, proof, and final diff.

@clawsweeper clawsweeper Bot added rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. P1 High-priority user-facing bug, regression, or broken workflow. merge-risk: 🚨 message-delivery 🚨 May drop, duplicate, misroute, suppress, or wrongly target messages. merge-risk: 🚨 session-state 🚨 May lose, corrupt, stale, or mis-associate session, agent, or context state. labels Jul 12, 2026
@clawsweeper

clawsweeper Bot commented Jul 12, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Codex review: needs maintainer review before merge. Reviewed July 13, 2026, 4:40 AM ET / 08:40 UTC.

Summary
The PR makes Codex steering settle only after observed input consumption, coordinates queue cancellation and resumption with terminal sessions_yield, and preserves paused yielded-session lifecycle state.

PR surface: Source +212, Tests +422. Total +634 across 4 files.

Reproducibility: yes. at source and protocol-test level: the reviewed base acknowledges steering before consumption, and the focused regression models acknowledgment followed by terminal cancellation. The linked issue also provides repeated released-build reproduction with an OpenClaw-runtime control.

Review metrics: none identified.

Root-cause cluster
Relationship: fixed_by_candidate
Canonical: #104806
Summary: The linked issue is the canonical released-build report, and this PR is its explicit candidate fix for the Codex steering and terminal-yield race.

Members:

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Merge readiness
Overall: 🐚 platinum hermit
Proof: 🦞 diamond lobster
Patch quality: 🐚 platinum hermit
Result: ready for maintainer review.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

  • Refresh the merge result and focused Codex steering checks against current main.

Risk before merge

  • [P1] The patch changes when queued Codex input is considered delivered; an unhandled notification ordering or content-correlation case could leave a wake pending or settle the wrong logical batch.
  • [P2] The pause, cancel, and conditional-resume logic runs at a session-state and message-delivery boundary, so current-base integration must preserve fallback delivery and yielded-session wakeability.

Maintainer options:

  1. Refresh current-base integration (recommended)
    Rebase or inspect the merge result against current main, rerun the focused Codex steering tests, and land if delivery and yielded-liveness behavior remain intact.
  2. Pause for additional protocol review
    Hold the PR if maintainers want deeper review of user-message correlation or terminal dynamic-tool ordering before accepting the lifecycle change.

Next step before merge

  • [P2] No narrow automated repair remains; a maintainer should review the current-base merge result and land the linked beta-blocker fix if focused checks remain green.

Security
Cleared: The diff is confined to bundled Codex runtime logic and tests and introduces no dependency, workflow, secret, permission, download, publishing, or supply-chain change.

Review details

Best possible solution:

Integrate the consumption-confirmed steering design against current main, retain the focused ordering and terminal-yield regression coverage, and land only after the actual merge result preserves fallback delivery and paused-session wakeability.

Do we have a high-confidence way to reproduce the issue?

Yes at source and protocol-test level: the reviewed base acknowledges steering before consumption, and the focused regression models acknowledgment followed by terminal cancellation. The linked issue also provides repeated released-build reproduction with an OpenClaw-runtime control.

Is this the best way to solve the issue?

Yes, provisionally: confirming delivery from Codex's completed user-message lifecycle fixes the ownership boundary more directly than changing shared announce retries or adding configuration. Current-base integration still needs final maintainer review because the PR touches delivery and session-liveness semantics.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against d33254b7ec60.

Label changes

Label changes:

  • add proof: sufficient: Contributor real behavior proof is sufficient. Redacted exact-head live output shows the changed real-Codex path recovering the child result exactly once, reporting paused yielded lifecycle state, and accepting a later turn on the same parent session.
  • add rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🦞 diamond lobster and patch quality is 🐚 platinum hermit.
  • add status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (live_output): Redacted exact-head live output shows the changed real-Codex path recovering the child result exactly once, reporting paused yielded lifecycle state, and accepting a later turn on the same parent session.
  • remove rating: 🦪 silver shellfish: Current PR rating is rating: 🐚 platinum hermit, so this older rating label is no longer current.
  • remove status: 📣 needs proof: Current PR status label is status: 👀 ready for maintainer look.

Label justifications:

  • P1: The PR addresses silent loss of completed subagent results in an active Codex orchestration workflow affecting real users.
  • merge-risk: 🚨 message-delivery: The patch changes the point at which Codex steering is treated as delivered and coordinates rejection with the requester-agent fallback path.
  • merge-risk: 🚨 session-state: The patch changes yielded-run liveness metadata and steering pause, cancellation, and resumption behavior for parent sessions.
  • rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🦞 diamond lobster and patch quality is 🐚 platinum hermit.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (live_output): Redacted exact-head live output shows the changed real-Codex path recovering the child result exactly once, reporting paused yielded lifecycle state, and accepting a later turn on the same parent session.
  • proof: sufficient: Contributor real behavior proof is sufficient. Redacted exact-head live output shows the changed real-Codex path recovering the child result exactly once, reporting paused yielded lifecycle state, and accepting a later turn on the same parent session.
Evidence reviewed

PR surface:

Source +212, Tests +422. Total +634 across 4 files.

View PR surface stats
Area Files Added Removed Net
Source 2 227 15 +212
Tests 2 432 10 +422
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 4 659 25 +634

What I checked:

  • Current behavior and root cause: At the reviewed base, the Codex steering queue resolves when turn/steer is acknowledged rather than when Codex consumes the input, allowing an immediately interrupted turn to discard an already-reported delivery. (extensions/codex/src/app-server/attempt-steering.ts:54, 5f9a6ce7eb53)
  • Proposed delivery boundary: The branch tracks dispatched batches, confirms them from completed user-message lifecycle events, and exposes pause, resume, and cancellation operations for terminal release. (extensions/codex/src/app-server/attempt-steering.ts:22, da3d4555188c)
  • Yield integration: The runner pauses and cancels pending steering when yield begins and conditionally resumes it when the yield fails or the dynamic-tool batch is classified as non-terminal. (extensions/codex/src/app-server/run-attempt.ts:1024, da3d4555188c)
  • Regression coverage: Focused tests cover acknowledged-but-unconsumed steering, both response/consumption wire orders, batching and FIFO behavior, initial-prompt disambiguation, failed yields, non-terminal batches, and yielded lifecycle metadata. (extensions/codex/src/app-server/run-attempt.steering.test.ts:60, da3d4555188c)
  • Codex protocol contract: The provided upstream references at Codex commit 9121132c8f54 distinguish queued steering acceptance from persisted consumption and show interruption clearing pending input, supporting the accepted-versus-consumed boundary used by the patch. (codex-rs/core/src/session/mod.rs:2923, 9121132c8f54)
  • Exact-head real behavior proof: The contributor posted redacted output from two real-Codex runs at exact head da3d4555188c43773aa01857562757cd177cb2f4, showing child completion, rejection of the unsafe old-turn wake, requester-agent fallback, exactly one parent transcript result, paused yielded lifecycle metadata, and a successful later parent turn. (da3d4555188c)

Likely related people:

  • steipete: Recent merged history attributes several Codex app-server lifecycle, liveness, client-factory, configuration, and documentation changes in the central runner to this handle. (role: recent Codex app-server runtime contributor; confidence: high; commits: 54e6e3d7daf5, f12e9c41fa15, c22f414c6976; files: extensions/codex/src/app-server/run-attempt.ts, extensions/codex/src/app-server/config.ts, docs/plugins/codex-harness.md)
  • Radek Sienkiewicz: Feature history identifies commit 52e2d4e16a29 as introducing the app-server attempt files that contain the affected lifecycle path. (role: introduced current app-server attempt structure; confidence: medium; commits: 52e2d4e16a29; files: extensions/codex/src/app-server/run-attempt.ts, extensions/codex/src/app-server/run-attempt.test.ts)
  • joshavant: Commit 5fdef4c39e75 changed closely related Codex completion-liveness behavior and cross-runtime coverage. (role: adjacent liveness fix contributor; confidence: medium; commits: 5fdef4c39e75; files: extensions/codex/src/app-server/run-attempt.ts, extensions/codex/src/app-server/run-attempt.test.ts, src/agents/pi-embedded-runner/run.ts)
What the crustacean ranks mean
  • 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
  • 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
  • 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
  • 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
  • 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
  • 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
  • 🌊 off-meta tidepool: rating does not apply to this item.

Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

How this review workflow works
  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.
Review history (2 earlier review cycles)
  • reviewed 2026-07-12T23:39:11.187Z sha b74eeb9 :: needs real behavior proof before merge. :: none
  • reviewed 2026-07-13T00:23:04.906Z sha da3d455 :: needs real behavior proof before merge. :: none

@clawsweeper clawsweeper Bot added rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. and removed rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. labels Jul 13, 2026
@algal

algal commented Jul 13, 2026

Copy link
Copy Markdown
Contributor Author

Exact-head live proof (redacted)

This addresses the requested live evidence for the combined yield, interrupt, fallback, transcript, and session-liveness path.

Head tested: da3d4555188c43773aa01857562757cd177cb2f4
Runtime: codex (rt-codex, real provider/model; no mocked app-server protocol)
Downstream recovery disabled: SPARKY_OPENCLAW_COMPLETION_WATCHDOG_ENABLED=0 and SPARKY_OPENCLAW_TASK_COMPLETION_RECOVERY_ENABLED=0

The transient gateway was built from this exact PR head with Node 24.14.0. The live check was a temporary reporter-side probe using only Gateway RPCs (sessions.create, chat.send, tasks.list, and chat.history); it did not use Sparky's dialogue/speech materialization path. The probe was removed after the run and is not part of this PR.

The parent prompt used synthetic tokens and can also be pasted into any Codex-runtime session:

Call sessions_spawn exactly once. The child task is: reply exactly
PR105724_CHILD_<nonce> and nothing else. Then call sessions_yield with
message exactly PR105724_ACK_<nonce>. After the child completion arrives,
write a brief parent-session summary that includes PR105724_PARENT_<nonce>
and PR105724_CHILD_<nonce>.

Run A: delivery plus subsequent wakeability

Redacted gateway timeline (local time):

00:37:01.191  PR105724_CHILD_A98361
00:37:04.694  [warn] Active requester session could not be woken for subagent
              completion; falling back to requester-agent handoff:
              queue_message_failed reason=runtime_rejected
              error=codex app-server steering queue cancelled
00:37:04.707  [agent/embedded] codex app-server turn released after terminal
              dynamic tool result
00:37:07.962  Parent-session summary: PR105724_PARENT_A98361 completed, and
              the child returned PR105724_CHILD_A98361.
00:37:07.964  [agent] run announce:v1:agent:rt-codex:subagent:<redacted>
              ended with stopReason=stop

Independent Gateway observations from the same run:

tasks.list:
  status=completed
  child result=PR105724_CHILD_A98361

chat.history on the parent session:
  assistant rows containing both synthetic tokens=1
  text="Parent-session summary: PR105724_PARENT_A98361 completed, and the
        child returned PR105724_CHILD_A98361."

follow-up chat.send on the same parent session:
  prompt="Reply exactly PR105724_WAKEABLE_A98361 and nothing else."
  CHAT_FINAL="PR105724_WAKEABLE_A98361"

Probe output:

PROOF head=da3d4555188c43773aa01857562757cd177cb2f4 runtime=codex sparky_completion_watchdog=disabled sparky_task_completion_recovery=disabled
PROOF child status=completed token=PR105724_CHILD_A98361
PROOF yield terminal_release=true active_wake_rejected=true requester_handoff=true native_announce_run=true
PROOF parent_transcript assistant_matches=1 parent_token=PR105724_PARENT_A98361 child_token=PR105724_CHILD_A98361
PROOF post_yield_wakeable=true token=PR105724_WAKEABLE_A98361
1 passed in 30.59s

Run B: live yielded lifecycle metadata

A second exact-head run captured the Gateway lifecycle-end frame itself (IDs and timestamps redacted here):

{
  "phase": "end",
  "aborted": false,
  "yielded": true,
  "livenessState": "paused",
  "stopReason": "end_turn"
}

The same run then observed child_completed=true and parent_materialized=true; it passed in 27.82s.

Together, these observations show the requested chain on this exact head: the child completes; the unsafe old-turn wake is rejected; terminal yield releases the turn; OpenClaw starts the native requester-agent fallback; the child result appears once in the parent transcript; the yielded lifecycle is paused; and that parent accepts a later turn.

For the maintainer-friendly, credential-free bug reproducer, the PR's four-second in-repo Vitest remains the primary proof. It deterministically demonstrates the underlying accepted-but-unconsumed steer being cancelled by terminal release. The live output above validates that the same ownership boundary produces the intended end-to-end behavior with a real Codex runtime.

I also rechecked the dependency contract directly at OpenAI Codex commit 9121132c8f54: turn/steer queues input and returns acceptance, consumption persists the user message before its completed lifecycle, and interrupt clears pending input.

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented Jul 13, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

Re-review progress:

@clawsweeper clawsweeper Bot added proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. and removed rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. labels Jul 13, 2026
@algal

algal commented Jul 13, 2026

Copy link
Copy Markdown
Contributor Author

@Solvely-Colin please let me know if you need more from me. I am new to this particular review workflow.

@fuller-stack-dev fuller-stack-dev mentioned this pull request Jul 15, 2026
Merged
@steipete
steipete force-pushed the fix/codex-subagent-announce branch from da3d455 to 6a7b10a Compare July 16, 2026 08:17
@steipete
steipete force-pushed the fix/codex-subagent-announce branch 2 times, most recently from 714b340 to 278a0aa Compare July 16, 2026 09:52
@steipete
steipete force-pushed the fix/codex-subagent-announce branch from 278a0aa to 5093e8a Compare July 16, 2026 10:11
@steipete steipete self-assigned this Jul 16, 2026
@steipete
steipete merged commit 7a3c26b into openclaw:main Jul 16, 2026
87 of 90 checks passed
@steipete

steipete commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

Merged via squash.

@steipete steipete mentioned this pull request Jul 16, 2026
Closed
github-actions Bot pushed a commit to Desicool/openclaw that referenced this pull request Jul 17, 2026
@algal @steipete
fix(codex): preserve subagent completions after sessions_yield (openc…
dd6f7db 
…law#105724)

* fix(codex): preserve subagent completions across sessions_yield

* fix(codex): confirm steering delivery before yield

Co-authored-by: Alexis Gallagher <[email protected]>

---------

Co-authored-by: Peter Steinberger <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

@steipete steipete

Labels

extensions: codex merge-risk: 🚨 message-delivery 🚨 May drop, duplicate, misroute, suppress, or wrongly target messages. merge-risk: 🚨 session-state 🚨 May lose, corrupt, stale, or mis-associate session, agent, or context state. P1 High-priority user-facing bug, regression, or broken workflow. proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. size: L status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Beta blocker: codex - subagent completion announce never runs for codex-runtime parent sessions

2 participants

@algal @steipete