Skip to content

fix(codex): preserve native subagent results after parent cleanup#102060

Merged
steipete merged 9 commits into
mainfrom
codex/native-subagent-monitor-history
Jul 12, 2026
Merged

fix(codex): preserve native subagent results after parent cleanup#102060
steipete merged 9 commits into
mainfrom
codex/native-subagent-monitor-history

Conversation

@steipete

@steipete steipete commented Jul 8, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Closes #97593
Related: #93313

What Problem This Solves

Fixes issues where Codex native subagent results could be lost after the parent turn cleaned up its app-server client, stale local rollout data could win after a child resumed, and permanently non-durable delivery could rerun the parent forever. The local-rollout recovery path also could not cover non-stdio app-server transports.

Why This Change Was Made

The monitor now treats the app-server's thread/read and thread/turns/list responses as canonical recovery state while preserving streamed terminal messages as the fast path. It retains the shared physical client only while detached children need monitoring, fences stale reads across lifecycle changes, keeps interrupted children resumable, and bounds terminal delivery retries with a durable failed state on exhaustion.

This removes rollout-file discovery, transcript parsing, periodic whole-client task rescans, and the bespoke deferred one-shot cleanup path.

User Impact

Codex native subagent completions remain reliable across parent cleanup, websocket/non-local transports, app-server persistence races, resumptions, and transient system errors. Permanently non-durable parent delivery now stops after bounded retries instead of waking the parent indefinitely.

Evidence

  • Exact Codex 0.142.5 app-server protocol/source checked for thread lineage, status transitions, live turn history, and intentionally unloaded turn/completed items.
  • pnpm test extensions/codex/src/app-server/native-subagent-monitor.test.ts extensions/codex/src/app-server/native-subagent-task-mirror.test.ts extensions/codex/src/app-server/run-attempt-thread-cleanup.test.ts extensions/codex/src/app-server/shared-client.test.ts — 88 passed.
  • pnpm check:changed — passed.
  • pnpm test:extension codex — 2,008 passed across 96 test files.

AI-assisted implementation and review. No agent transcript attached.

@clawsweeper

clawsweeper Bot commented Jul 8, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Codex review: needs maintainer review before merge. Reviewed July 12, 2026, 2:06 AM ET / 06:06 UTC.

Summary
The PR rewrites Codex native-subagent monitoring to recover from app-server thread history, retain shared clients through detached completion, fence stale lifecycle reads, preserve resumable interruptions, and bound failed parent-delivery retries.

PR surface: Source +157, Tests +183, Other +8. Total +348 across 8 files.

Reproducibility: yes. Current-main source provides a high-confidence path through rollout-dependent recovery and unbounded non-durable retries, while the branch's live run demonstrates exact detached delivery and fresh-monitor history recovery after the fix; this read-only review did not execute current main.

Review metrics: none identified.

Root-cause cluster
Relationship: fixed_by_candidate
Canonical: #97593
Summary: This PR is the active broad fix candidate for the canonical infinite-retry issue; it supersedes closed narrower candidates and partially overlaps a closed lifecycle refactor.

Members:

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

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

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

Risk before merge

  • [P1] This remains a large delivery and lifecycle state-machine replacement; an unproven ownership transition could drop, duplicate, or mis-associate a detached completion despite the strong focused and live proof.
  • [P1] The exact head currently has two unrelated failing CI shards, so normal landing gates still need a green rerun or maintainer-confirmed attribution before merge.

Maintainer options:

  1. Land after lifecycle-owner and CI confirmation (recommended)
    Accept the exact head after owner sign-off and after the unrelated failing shards are green or formally attributed.
  2. Extract bounded delivery retries
    Reduce scope to the canonical infinite-retry fix and review the history-recovery rewrite separately.
  3. Pause the lifecycle rewrite
    Do not merge if maintainers prefer the current recovery path over the broader state-machine change.

Next step before merge

  • [P2] The patch is review-clean and sufficiently proven; the next action is explicit Codex lifecycle-owner landing judgment plus routine CI rerun or attribution, not an automated repair.

Maintainer decision needed

  • Question: Should the Codex lifecycle owner accept and land this exact-head history-recovery rewrite once the two unrelated failing CI shards are green or formally attributed?
  • Rationale: The patch is correct and strongly proven, but the protected PR replaces a large message-delivery and session-state machine whose residual merge risk requires explicit human ownership.
  • Likely owner: steipete — The active rewrite, original adjacent ownership work, upstream contract audit, and live proof show the strongest current connection to this landing decision.
  • Options:
    • Land after owner and CI confirmation (recommended): Accept the canonical history-based rewrite after lifecycle-owner sign-off and a green rerun or formal attribution of the two unrelated failing shards.
    • Split bounded retries: Land only the infinite-retry repair and defer the broader recovery rewrite, retaining rollout-file and non-stdio limitations.
    • Pause the rewrite: Defer the branch if maintainers do not want to own the larger lifecycle change now.

Security
Cleared: The diff changes internal Codex runtime, tests, and live-shard registration without dependency, package-resolution, permission, secret, download, or third-party execution changes.

Review details

Best possible solution:

Land the exact history-based rewrite after Codex lifecycle-owner sign-off and normal CI resolution, preserving one canonical app-server recovery and delivery path without restoring rollout parsing or parallel cleanup fallbacks.

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

Yes. Current-main source provides a high-confidence path through rollout-dependent recovery and unbounded non-durable retries, while the branch's live run demonstrates exact detached delivery and fresh-monitor history recovery after the fix; this read-only review did not execute current main.

Is this the best way to solve the issue?

Yes. Direct inspection of pinned Codex source confirms app-server history is the narrow transport-independent source of truth, and the patch removes duplicated rollout discovery and bespoke cleanup without adding public API or configuration.

AGENTS.md: found and applied where relevant.

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

Label changes

Label justifications:

  • P1: The PR fixes lost native-subagent completions and indefinite parent reruns in an active agent workflow.
  • merge-risk: 🚨 message-delivery: The patch rewrites terminal result discovery, deduplication, retry ownership, exhaustion, and delivery to the parent.
  • merge-risk: 🚨 session-state: The patch rewrites child lifecycle recovery, stale-read fencing, resumability, persisted task reconciliation, and shared-client lifetime.
  • rating: 🦞 diamond lobster: Overall readiness is 🦞 diamond lobster; proof is 🦞 diamond lobster and patch quality is 🦞 diamond lobster.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (live_output): The PR provides a real OpenAI/Codex app-server run showing parent completion before detached exact-result delivery, authoritative history visibility, and identical recovery by a fresh monitor.
  • proof: sufficient: Contributor real behavior proof is sufficient. The PR provides a real OpenAI/Codex app-server run showing parent completion before detached exact-result delivery, authoritative history visibility, and identical recovery by a fresh monitor.
Evidence reviewed

PR surface:

Source +157, Tests +183, Other +8. Total +348 across 8 files.

View PR surface stats
Area Files Added Removed Net
Source 3 1194 1037 +157
Tests 4 1581 1398 +183
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 1 8 0 +8
Total 8 2783 2435 +348

What I checked:

Likely related people:

  • steipete: Authored this rewrite, contributed adjacent shared task-scope behavior in the original merged feature, and supplied the direct Codex contract audit and live behavior proof. (role: recent feature owner; confidence: high; commits: bbc4a50eb444, 9d8c179eab33; files: extensions/codex/src/app-server/native-subagent-monitor.ts, extensions/codex/src/app-server/run-attempt.ts, extensions/codex/src/app-server/shared-client.ts)
  • bryanpearson: Authored the original native spawn_agent completion-delivery implementation merged through fix(codex): deliver native spawn_agent completions #83445. (role: introduced behavior; confidence: high; commits: fcdd18427d47; files: extensions/codex/src/app-server/native-subagent-monitor.ts)
  • Vincent Koc: The synchronized current-main history attributes a recent broad touch across the monitor and adjacent app-server files to commit 4040267, making this a weaker but relevant routing signal. (role: recent area contributor; confidence: low; commits: 4040267af9d1; files: extensions/codex/src/app-server/native-subagent-monitor.ts, extensions/codex/src/app-server/run-attempt.ts, extensions/codex/src/app-server/shared-client.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 (6 earlier review cycles)
  • reviewed 2026-07-08T07:38:49.557Z sha 540939f :: needs real behavior proof before merge. :: none
  • reviewed 2026-07-08T07:58:53.823Z sha 540939f :: needs real behavior proof before merge. :: none
  • reviewed 2026-07-11T22:35:28.756Z sha 236ad88 :: needs maintainer review before merge. :: none
  • reviewed 2026-07-11T23:44:31.639Z sha b022768 :: needs maintainer review before merge. :: none
  • reviewed 2026-07-12T05:32:19.554Z sha 6f3576e :: needs changes before merge. :: [P3] Format the new live test before merge
  • reviewed 2026-07-12T05:46:31.863Z sha e48dd54 :: needs maintainer review before merge. :: none

@steipete
steipete force-pushed the codex/native-subagent-monitor-history branch from 9040d9c to e6d5c68 Compare July 8, 2026 07:06
chatgpt-codex-connector[bot]
chatgpt-codex-connector Bot reviewed Jul 8, 2026
View reviewed changes

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 2125be861f

ℹ️ 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 thread extensions/codex/src/app-server/native-subagent-monitor.ts
@blacksmith-sh

This comment has been minimized.

Sign in to view
@clawsweeper clawsweeper Bot added 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. 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 8, 2026
@steipete

steipete commented Jul 8, 2026

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented Jul 8, 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.

@openclaw-barnacle openclaw-barnacle Bot added the scripts Repository scripts label Jul 11, 2026
@steipete

steipete commented Jul 11, 2026

Copy link
Copy Markdown
Contributor Author

Maintainer review + land-ready proof (head 236ad8836959).

Verdict: best-fix. History-backed recovery via thread/read / thread/turns/list replaces rollout-file discovery, transcript parsing, and the 10s whole-client rescan; lease retention on the shared client subsumes the deferred one-shot cleanup; delivery retries are bounded with a durable failed state. Works on non-stdio transports.

Codex contract verification (sibling openai/codex checkout at rust-v0.144.1, the pinned harness):

  • v2/thread.rs:1253,1275,1306 — ThreadStatus tags (notLoaded|idle|systemError|active), thread/read includeTurns, thread/turns/list params; SortDirection snake_case.
  • v2/thread_data.rs — parentThreadId (subagents only), turns populated only on read-with-turns, completedAt in seconds (monitor ×1000 correct), TurnError.message/codexErrorInfo, TurnItemsView.
  • v2/item.rs / protocol/src/models.rs:897 — AgentMessage phase (commentary|final_answer), CollabAgentToolCall senderThreadId/receiverThreadIds/agentsStates, spawnAgent.
  • app-server/src/request_processors/thread_processor.rs:2230-2320,2417-2470,4100-4133 — thread/read live-first view; turns/list merges the live active-turn snapshot after marking stale in-progress turns interrupted (justifies systemError latest-turn handling and interrupted-resumable settle).
  • app-server/src/bespoke_event_handling.rs:1216-1245 — turn/completed ships items: [], itemsView: notLoaded (streamed capture + history recovery are required, as implemented).
  • app-server/src/message_processor.rs:798-801 + client.ts:212 — turns/list experimental gating satisfied (experimentalApi: true).
  • features/src/lib.rs:1037 — collab/spawn tools stable + default-enabled; core/src/agent/control.rs:533 — subagent_notification envelope contract.

Validation:

  • Hosted CI green on prior head 540939fda7ea (53 pass / 12 expected skips); CI rerunning on 236ad8836959.
  • Testbox-through-Crabbox tbx_01kx9ep79h8c9am31ky7fzmf3v: focused monitor/mirror/cleanup/shared-client suites + full pnpm check:changed green; monitor suite rerun after the spoof-test addition (47/47) + extensions test typecheck green.
  • Live E2E (AWS Crabbox cbx_2bd66337e5cb, run run_4d09702762eb, real OpenAI API key): new opt-in native-subagent-monitor.live.test.ts spawns a real native subagent via the app-server, parent turn completes with zero deliveries observed, detached completion then arrives with the child's exact answer, thread/read proves lineage + terminal turn, thread/turns/list returns the completed turn, and a fresh monitor with only a persisted task row recovers the identical result from history. 1/1 passed.
  • Autoreview (Codex gpt-5.6-sol, xhigh) over the split bundles: prod half clean (one split-artifact finding rejected); tests half clean after addressing findings.

Improvements added on top (236ad8836959):

  • Restored the registered-child user-spoof regression test the rewrite dropped (trust boundary: user-authored subagent_notification text must never finalize a known child).
  • Added the opt-in live lane above (OPENCLAW_LIVE_TEST=1 OPENCLAW_LIVE_CODEX_NATIVE_SUBAGENT=1).
  • Registered both codex opt-in live files in OPTIONAL_LIVE_SHARD_FILE_ENVS so the native-live-extensions-a-k release shard doesn't treat the env-gated skips as missing proof (the coexistence lane had the same latent gap).

Known gaps: none blocking. Recovery polling is unbounded by design (accepted trade-off: losing results is worse; client close still disposes cleanly).

@clawsweeper clawsweeper Bot added proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🦞 diamond lobster Very strong PR readiness with only minor maintainer review expected. and removed rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. labels Jul 11, 2026
@steipete
steipete force-pushed the codex/native-subagent-monitor-history branch from b022768 to 3faf684 Compare July 12, 2026 05:16
@clawsweeper clawsweeper Bot added rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. rating: 🦞 diamond lobster Very strong PR readiness with only minor maintainer review expected. and removed rating: 🦞 diamond lobster Very strong PR readiness with only minor maintainer review expected. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. labels Jul 12, 2026
steipete added 6 commits July 11, 2026 22:51
@steipete
fix(codex): preserve native subagent results after parent cleanup
d68a33c
@steipete
fix(codex): preserve nested native subagent lineage
2fafafa
@steipete
fix(codex): keep live native subagent turns active
0df1f28
@steipete
docs(changelog): note Codex subagent recovery
9b894ee
@steipete
fix(codex): honor native completion retry backoff
82d9488
@steipete
test(codex): prove native subagent recovery live and guard spoofed co…
198a1a1 
…mpletions

Adds an opt-in live lane (OPENCLAW_LIVE_CODEX_NATIVE_SUBAGENT=1) that spawns a real
native subagent through the app-server with an OpenAI key, proves the parent turn
completes before detached delivery, and recovers the same result from thread/read
history with a fresh monitor. Registers both codex opt-in live files in the
release shard optional-env map and restores the registered-child user-spoof
regression test dropped by the monitor rewrite.
@steipete
steipete force-pushed the codex/native-subagent-monitor-history branch from e48dd54 to 9d8c179 Compare July 12, 2026 05:51
@steipete
steipete merged commit 401ae11 into main Jul 12, 2026
215 of 220 checks passed
@steipete
steipete deleted the codex/native-subagent-monitor-history branch July 12, 2026 07:27
@steipete

steipete commented Jul 12, 2026

Copy link
Copy Markdown
Contributor Author

Landed as 401ae11 (squash, admin merge on maintainer instruction).

Merge context: full PR-side proof was green — Testbox focused suites + pnpm check:changed on the rebased head, live OpenAI-key E2E (detached delivery + fresh-monitor history recovery), clean split autoreview, and a green exact-SHA release-gate CI run (29172549533) on the pre-rebase patch. The final full-CI gate could not complete because main was mid-flight on Control UI localization (locale files structurally behind en.ts; translate.test.ts, raw-copy baseline, and hovercard locale failures in files this PR does not touch) while rapid pushes kept cancelling main's own CI. The rebase absorbed main's multi-agent V2 subAgentActivity child registration into the rewritten monitor and ported its regression test.

@github-actions github-actions Bot mentioned this pull request Jul 12, 2026
Open
github-actions Bot pushed a commit to Desicool/openclaw that referenced this pull request Jul 13, 2026
@steipete
fix(codex): preserve native subagent results after parent cleanup (op…
c98957a 
…enclaw#102060)

Codex native subagent results now survive parent cleanup, app-server persistence races, resumptions, and transient delivery failures. The monitor treats app-server thread/read and thread/turns/list history as canonical recovery state (works on non-stdio transports), retains the shared physical client only while detached children need monitoring, fences stale reads across lifecycle changes, keeps interrupted children resumable, preserves multi-agent V2 subAgentActivity lineage, and bounds terminal delivery retries with a durable failed state. Removes rollout-file discovery, transcript parsing, periodic whole-client rescans, and the deferred one-shot cleanup path.

Adds an opt-in live E2E lane (OPENCLAW_LIVE_CODEX_NATIVE_SUBAGENT=1) proving detached delivery and history-only recovery against a real app-server with an OpenAI key, restores the registered-child user-spoof regression test, and registers both codex opt-in live files in the release shard optional-env map.

Closes openclaw#97593
Related: openclaw#93313
This was referenced Jul 13, 2026
Closed
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@chatgpt-codex-connector chatgpt-codex-connector[bot] chatgpt-codex-connector[bot] left review comments

Assignees

No one assigned

Labels

extensions: codex maintainer Maintainer-authored PR 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: 🦞 diamond lobster Very strong PR readiness with only minor maintainer review expected. scripts Repository scripts size: XL 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.

Codex native subagent completion delivery retries forever (no attempt cap, no deadline) on a permanently non-durable delivery

1 participant

@steipete