Skip to content

[codex] fix native subagent completion retry cap#97715

Closed
NianJiuZst wants to merge 1 commit into
openclaw:mainfrom
NianJiuZst:codex/issue-97593-native-subagent-retry-cap
Closed

[codex] fix native subagent completion retry cap#97715
NianJiuZst wants to merge 1 commit into
openclaw:mainfrom
NianJiuZst:codex/issue-97593-native-subagent-retry-cap

Conversation

@NianJiuZst

@NianJiuZst NianJiuZst commented Jun 29, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

What Problem This Solves

Fixes #97593.

Codex native subagent completion delivery could retry forever when the parent handoff was permanently non-durable. The monitor marked the completion pending, called deliverAgentHarnessTaskCompletion, and on every non-durable result scheduled another retry. The retry counter only selected a delay with a clamped array index, so once it reached the final configured delay it re-fired every 300 seconds indefinitely, waking and re-running the parent agent forever.

Why This Change Was Made

The fix keeps ownership inside the Codex plugin monitor and does not add public config or SDK surface. completionDeliveryRetryDelaysMs remains the single retry policy: each configured delay is consumed exactly once, and once the schedule is exhausted the pending completion is cleared and its task delivery status is marked failed with a diagnostic error.

This intentionally avoids the two sharp edges from the open candidate PRs:

  • It does not add a separate max-attempt option for a plugin-local scheduler.
  • It does not skip the final configured retry delay; the default six-delay schedule still uses the 300 second final delay before giving up.

The failed terminal state mirrors the existing task delivery vocabulary and gives maintainers/logs a visible end state instead of leaving the child completion pending forever.

User Impact

Users running Codex native multi-agent sessions will no longer get an unbounded parent wake/re-run loop when a subagent completion cannot durably reach the parent. Transient failures still retry through the configured schedule and can still become delivered. Permanently non-durable failures settle as failed after the schedule is exhausted, preventing repeated parent reruns every five minutes.

Evidence

Commands run on this branch:

pnpm -s exec oxfmt --check extensions/codex/src/app-server/native-subagent-monitor.ts extensions/codex/src/app-server/native-subagent-monitor.test.ts
git diff --check
pnpm changed:lanes --json
node scripts/run-vitest.mjs extensions/codex/src/app-server/native-subagent-monitor.test.ts
node scripts/run-vitest.mjs src/plugin-sdk/agent-harness-task-runtime.test.ts
pnpm test:changed
.agents/skills/autoreview/scripts/autoreview --mode local

Observed results:

native-subagent-monitor.test.ts: 35 tests passed
agent-harness-task-runtime.test.ts: 6 tests passed
pnpm test:changed: 35 tests passed
changed lanes: extensions=true, extensionTests=true, all other lanes false
autoreview clean: no accepted/actionable findings reported

Regression coverage added:

  • Durable-on-final-retry path: a handoff that is non-durable twice and durable on the second configured retry is delivered, and is not marked failed.
  • Permanent non-durable path: a handoff with a two-delay schedule attempts initial + two retries, marks delivery failed with a retry-schedule-exhausted error, and does not invoke the parent delivery again after more timer advancement.

Dependency/Codex contract checked:

  • src/plugin-sdk/agent-harness-task-runtime.ts durable-delivery predicate returns false for delivered: false and true only for durable direct/steered delivery.
  • Sibling Codex source confirms native subagent activity/completion signals are upstream protocol/runtime events, while OpenClaw owns the monitor retry and parent-handoff policy: ../codex/codex-rs/app-server-protocol/schema/typescript/v2/ThreadItem.ts, ../codex/codex-rs/protocol/src/protocol.rs, ../codex/codex-rs/core/src/tools/handlers/multi_agents_v2/spawn.rs.

Screenshot evidence: The attached terminal screenshot shows node scripts/run-vitest.mjs extensions/codex/src/app-server/native-subagent-monitor.test.ts --reporter=verbose passing with the new retry-exhaustion and final-retry delivery regressions included in the 35-test Codex native subagent monitor suite.
image

Screenshot evidence shows a real OpenClaw Gateway + Codex app-server run on d31cb1b using the local Codex OAuth bootstrap: an aborted-parent Codex native subagent completion stayed pending through the configured retry schedule and then reached deliveryStatus: failed with retry schedule exhausted, rather than retrying indefinitely.
image

@clawsweeper

clawsweeper Bot commented Jun 29, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Codex review: needs real behavior proof before merge. Reviewed June 29, 2026, 3:58 AM ET / 07:58 UTC.

Summary
Caps Codex native subagent completion delivery retries by consuming each configured retry delay once, marking exhausted handoffs failed, and adding monitor regression tests.

PR surface: Source +32, Tests +122. Total +154 across 2 files.

Reproducibility: yes. Current main is source-reproducible: non-durable delivery calls scheduleCompletionDeliveryRetry, and that scheduler only clamps the delay index without a maximum attempt or deadline.

Review metrics: 1 noteworthy metric.

  • Retry Policy Surface: 1 cap added, 0 config options. The diff changes Codex native completion terminal behavior while keeping the existing delay array as the only retry policy.

Root-cause cluster
Relationship: fixed_by_candidate
Canonical: #97593
Summary: This PR is a candidate fix for the canonical unbounded Codex native monitor completion-delivery retry issue; two sibling open PRs target the same scheduler root cause, while adjacent delivery-visibility and parent-wake items use different paths.

Members:

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

Merge readiness
Overall: 🧂 unranked krab
Proof: 🧂 unranked krab
Patch quality: 🐚 platinum hermit
Result: blocked until real behavior proof from a real setup is added.

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

Rank-up moves:

Proof guidance:

  • [P1] Needs real behavior proof before merge: The PR body lists focused tests and a mocked runtime seam, but it does not include after-fix proof from a real setup such as redacted gateway/channel/provider logs, copied live output, or a recording; after adding proof, updating the PR body should trigger re-review, or a maintainer can comment @clawsweeper re-review. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Risk before merge

  • [P1] The diff intentionally changes exhausted non-durable Codex native completion delivery from indefinitely pending retries to a failed terminal state, so maintainers should accept the existing retry schedule as the give-up window before merge.
  • [P1] The PR body evidence is focused tests with a mocked runtime seam, not redacted real gateway, channel, provider, or runtime proof of the after-fix capped give-up path.
  • [P1] Two sibling open PRs target the same canonical issue, so maintainers should choose one landing path before merging any of the retry-cap branches.

Maintainer options:

  1. Prove And Select This Cap (recommended)
    Before merge, add redacted real runtime proof for the capped give-up path and have maintainers choose this branch as the canonical fix over the sibling retry-cap PRs.
  2. Accept Schedule-Based Give-Up
    Maintainers may intentionally accept terminal failed delivery after the existing schedule if the shorter bounded retry window is the desired product behavior.
  3. Pause For A Sibling
    If another retry-cap PR becomes the selected implementation, pause or close this PR after preserving the useful final-delay and failure-state test ideas.

Next step before merge

  • [P1] Human review is needed because the PR is draft, contributor proof is mock-only, and maintainers need to choose the canonical retry-cap PR.

Security
Cleared: The diff is limited to Codex plugin retry logic and colocated tests, with no dependency, workflow, package-resolution, permission, script, network, or secret-handling changes.

Review details

Best possible solution:

Land one canonical Codex native monitor retry cap that uses the existing retry schedule as the single policy, preserves success-after-retry behavior, records failed terminal delivery on give-up, and includes redacted real runtime proof.

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

Yes. Current main is source-reproducible: non-durable delivery calls scheduleCompletionDeliveryRetry, and that scheduler only clamps the delay index without a maximum attempt or deadline.

Is this the best way to solve the issue?

Mostly yes. The Codex native monitor is the right owner boundary and using the existing delay schedule avoids new config surface, but the branch still needs real behavior proof and canonical selection among sibling retry-cap PRs.

AGENTS.md: found and applied where relevant.

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

Label changes

Label justifications:

  • P1: The PR targets a Codex native subagent workflow that can repeatedly wake and rerun a parent session after permanently non-durable completion delivery.
  • merge-risk: 🚨 message-delivery: The diff changes when a Codex native child completion stops retrying and is marked failed instead of potentially delivering later.
  • merge-risk: 🚨 session-state: The diff clears pending completion state and writes failed delivery status on give-up for affected child task rows.
  • rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🧂 unranked krab and patch quality is 🐚 platinum hermit.
  • status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: The PR body lists focused tests and a mocked runtime seam, but it does not include after-fix proof from a real setup such as redacted gateway/channel/provider logs, copied live output, or a recording; after adding proof, updating the PR body should trigger re-review, or a maintainer can comment @clawsweeper re-review. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
Evidence reviewed

PR surface:

Source +32, Tests +122. Total +154 across 2 files.

View PR surface stats
Area Files Added Removed Net
Source 1 36 4 +32
Tests 1 122 0 +122
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 2 158 4 +154

What I checked:

Likely related people:

  • bryanpearson: Merged work introduced the Codex native subagent monitor, its tests, and the harness completion runtime used by this retry path. (role: introduced behavior; confidence: high; commits: f9d35dc68180, fcdd18427d47; files: extensions/codex/src/app-server/native-subagent-monitor.ts, extensions/codex/src/app-server/native-subagent-monitor.test.ts, src/plugin-sdk/agent-harness-task-runtime.ts)
  • vincentkoc: Recent GitHub path history shows multiple Codex native monitor fixes and transcript indexing work in the affected files. (role: recent area contributor; confidence: high; commits: 2ad2e4f2dc6c, 3bacf96cccae, e172f64f3fcc; files: extensions/codex/src/app-server/native-subagent-monitor.ts, extensions/codex/src/app-server/native-subagent-monitor.test.ts)
  • 849261680: Recent related work preserved Codex native subagent completion results and the same account owns the adjacent parent-wake repair path for subagent delivery give-up behavior. (role: adjacent repair contributor; confidence: medium; commits: 766c5b3d32ba, a849bfd57d18; files: extensions/codex/src/app-server/native-subagent-monitor.ts, extensions/codex/src/app-server/native-subagent-monitor.test.ts, src/tasks/task-registry.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.

@NianJiuZst
NianJiuZst marked this pull request as ready for review June 29, 2026 07:58
@clawsweeper clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. 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 Jun 29, 2026
This was referenced Jun 29, 2026
Closed
Closed
Closed
@NianJiuZst

NianJiuZst commented Jun 29, 2026

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented Jun 29, 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.

@clawsweeper clawsweeper Bot added proof: sufficient ClawSweeper judged the real behavior proof convincing. proof: 📸 screenshot Contributor real behavior proof includes screenshot evidence. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. and removed rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. proof: sufficient ClawSweeper judged the real behavior proof convincing. proof: 📸 screenshot Contributor real behavior proof includes screenshot evidence. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. labels Jun 29, 2026
@clawsweeper clawsweeper Bot mentioned this pull request Jul 8, 2026
Merged
@mjnkao mjnkao mentioned this pull request Jul 9, 2026
Closed
@NianJiuZst NianJiuZst closed this Jul 10, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

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. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. size: S status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask.

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

@NianJiuZst