fix(codex): successful sessions_spawn and goal tool results recorded as failures by nxmxbbd · Pull Request #96856 · openclaw/openclaw

nxmxbbd · 2026-06-25T21:05:49Z

Related: #96833

What Problem This Solves

Fixes an issue where a Codex-runtime agent's successful dynamic tool calls are recorded as tool failures. When an agent launches a subagent, the accepted launch (sessions_spawn) is persisted on the session transcript as a failed tool result; the same happens for successful create_goal / update_goal calls. The model then sees its own successful actions as failures in the live transcript, and the compaction "Tool Failures" summary lists them as failures, making an agent's failure list untrustworthy.

This is the source-side counterpart to #96842 (which guards the compaction summary on the read side). This PR fixes the classification at its origin so the wrong terminal state is never produced for these results.

Why This Change Was Made

In the Codex app-server runtime, isCodexToolResultError (extensions/codex/src/app-server/dynamic-tools.ts) classifies a dynamic tool result by a non-error status allowlist and fail-closes any status not in the list. A result classified as an error becomes success: false in the response sent to Codex, which Codex maps to a Failed dynamic-tool item status, and which OpenClaw persists on the transcript as toolResult.isError: true.

Several success statuses emitted by OpenClaw tools that are exposed to Codex agents were missing from the allowlist:

sessions_spawn accepted launches → details.status: "accepted"
create_goal / update_goal results → details.status: "created" / "updated"

The fix adds these statuses to the allowlist alongside their sibling success statuses (completed, recorded, started, running, …). Genuinely failed or rejected results (status: "error" / "forbidden") remain fail-closed and continue to be reported as failures.

Scope boundary: this changes only the Codex dynamic-tool result classifier. The Pi agent-loop runtime is unaffected — it uses an inverse failure-status allowlist (src/agents/tool-result-error.ts) under which accepted / created / updated are already non-errors. The message tool's suppressed status is intentionally left out, since a suppressed outbound send may be intended as a model-facing failure (a product decision, not a clear bug).

User Impact

Codex-runtime agents no longer have their successful subagent launches and successful goal create/update calls recorded as tool failures. The live transcript and the compaction summary reflect only genuinely failed tool calls. As a side benefit, post-tool-use hooks now fire correctly for accepted launches (the Codex runtime gates those hooks on the same success flag).

Evidence

Tier: internal deterministic classifier (isCodexToolResultError is pure logic over a tool result), but its effect is observable end-to-end through the real persistence path, so a real-runtime before/after is included below in addition to focused tests.

Focused regression tests

extensions/codex/src/app-server/dynamic-tools.test.ts, exercised through the real createCodexDynamicToolBridge executor:

an accepted sessions_spawn result is reported as a successful dynamic tool call (fails before the change, passes after);
create_goal ("created") and update_goal ("updated") results are reported as successful;
a forbidden sessions_spawn result is still reported as a failure (deny symmetry, guards against over-correction);
the existing "unrecognized non-success statuses stay fail-closed" contract test is unchanged and still passes.

node scripts/run-vitest.mjs extensions/codex/src/app-server/dynamic-tools.test.ts
 Test Files  1 passed (1)
      Tests  59 passed (59)

Touched-file gates (local): oxfmt --check clean; oxlint clean.

Real behavior proof

Behavior or issue addressed: an accepted sessions_spawn result is persisted on the session transcript with toolResult.isError: true (and reported to Codex as success: false) instead of as a success.
Real environment tested: the real runCodexAppServerAttempt orchestration driving the real dynamic-tool bridge → isCodexToolResultError → event projector → session-JSONL transcript writer. The tools that return the accepted/created/updated payloads are supplied via the test tool registry; the classification and persistence path are the production code paths. (The classifier, not the Codex binary, computes isError: the Rust app-server only relays success and maps it to the item status — verified against the upstream Codex source.)
Exact steps run after this patch: drive one attempt with a sessions_spawn (accepted), a create_goal (created), and an update_goal (updated) dynamic-tool result, then read the persisted session.jsonl and inspect each toolResult entry. Run the identical scenario on the base branch (no change) and on this branch.
Evidence after fix: persisted session.jsonl entry for the accepted launch (copied from the run; the created/updated entries are identical with their own tool/status):

{"role":"toolResult","toolCallId":"call-accepted","toolName":"sessions_spawn","isError":false,
 "content":[{"type":"toolResult","name":"sessions_spawn","content":"Accepted: launching child session to scan logs."}]}

Observed result after fix: for all three (accepted / created / updated), protocol success = true and persisted toolResult.isError = false. On the base branch (without the change), the identical run persists toolResult.isError = true and reports success = false for all three.
What was not tested: no live model or external network path is involved in this classifier, so none was exercised; the proof is the real in-process orchestration + persistence path described above.

Security / Compatibility

No auth, secret, dependency, permission, migration, config, or data-access surface changes. No persisted-state shape change. The change only widens which statuses are classified as non-errors; failure statuses are unaffected.

Risks and Mitigations

The allowlist is tool-name-agnostic, so any dynamic tool result carrying status: "accepted" / "created" / "updated" is now treated as a success. A survey of producers found these statuses are only emitted by successful operations (failures use error / forbidden), and the deny-symmetry test guards the failure path. The allowlist remains hand-maintained and could diverge from producers again as new tools are added; a follow-up to derive it from a shared typed status contract would make this class of divergence statically impossible.

AI-assisted; the contributor manually reviewed the change and is responsible for it.

…s success isCodexToolResultError fail-closes every tool-result status not in its non-error allowlist, but the allowlist omitted several success statuses emitted by OpenClaw tools that are exposed to codex agents: - sessions_spawn accepted launches -> details.status "accepted" - create_goal / update_goal results -> details.status "created" / "updated" So a successful accepted spawn (openclaw#96833), and successful goal create/update, were classified as errors: reported to codex as success: false (mapped to a Failed item status) and persisted on the transcript as isError: true. This adds those statuses to the allowlist alongside their sibling success statuses (completed/recorded/started/running). Genuinely failed or forbidden results (status "error"/"forbidden") stay fail-closed. Adds regression tests: accepted spawn and created/updated goal results are reported as successful dynamic tool calls; a forbidden spawn still fails.

clawsweeper · 2026-06-25T21:06:51Z

Codex review: needs maintainer review before merge. Reviewed June 25, 2026, 5:13 PM ET / 21:13 UTC.

Summary
The PR classifies Codex dynamic-tool results with accepted, created, and updated statuses as successful and adds regression tests for accepted spawns, goal tools, and forbidden spawns.

PR surface: Source +3, Tests +94. Total +97 across 2 files.

Reproducibility: yes. Source inspection shows current main fail-closes accepted, created, and updated status strings in the Codex dynamic-tool classifier, causing successful producer results to be reported and persisted as failures.

Review metrics: 1 noteworthy metric.

Dynamic success statuses: 3 added. The classifier is tool-name-agnostic, so maintainers should notice the exact statuses newly treated as successful before merge.

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.

Next step before merge

No automated repair is needed; the patch has no concrete correctness findings and awaits normal maintainer review.

Security
Cleared: The diff changes only Codex runtime classification logic and colocated tests, with no dependency, workflow, auth, permission, secret, package, or migration surface touched.

Review details

Best possible solution:

Land the narrow Codex classifier fix with its focused tests if CI remains green; keep the compaction/core PRs separate because they do not replace this bridge path.

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

Yes. Source inspection shows current main fail-closes accepted, created, and updated status strings in the Codex dynamic-tool classifier, causing successful producer results to be reported and persisted as failures.

Is this the best way to solve the issue?

Yes. Updating the Codex dynamic-tool classifier is the narrow origin fix for this bridge; compaction guards or core embedded-runner changes alone do not fix goal-tool statuses through Codex app-server dynamic tools.

AGENTS.md: found and applied where relevant.

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

Label changes

Label justifications:

P2: This fixes a concrete Codex runtime transcript/session-state correctness bug with limited blast radius and focused coverage.
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 body includes after-fix runtime-path output showing persisted toolResult.isError: false for accepted/created/updated statuses plus before behavior showing the same scenario persisted as errors.
proof: sufficient: Contributor real behavior proof is sufficient. The PR body includes after-fix runtime-path output showing persisted toolResult.isError: false for accepted/created/updated statuses plus before behavior showing the same scenario persisted as errors.

Evidence reviewed

PR surface:

Source +3, Tests +94. Total +97 across 2 files.

View PR surface stats

Area	Files	Added	Net
Source	1	3	+3
Tests	1	94	+94
Docs	0	0	0
Config	0	0	0
Generated	0	0	0
Other	0	0	0
Total	2	97	+97

What I checked:

PR diff: The patch adds created, updated, and accepted to the Codex dynamic-tool non-error status allowlist and adds tests for accepted sessions_spawn, create_goal/update_goal, and forbidden sessions_spawn. (extensions/codex/src/app-server/dynamic-tools.ts:902, d5b0adc40042)
Current main classifier: On current main, isCodexToolResultError fail-closes any nonempty details.status not in its success allowlist; the allowlist does not include accepted, created, or updated. (extensions/codex/src/app-server/dynamic-tools.ts:877, 13ecca5408cb)
Codex persistence path: The Codex app-server bridge returns success: !resultIsError, run-attempt records that protocol success, and the event projector persists transcript tool results as isError: !params.success. (extensions/codex/src/app-server/event-projector.ts:450, 13ecca5408cb)
Success status producers: sessions_spawn returns status: "accepted" with child-run identity on successful spawn, while goal tools return status: "created" and status: "updated" after successful create/update operations. (src/agents/tools/goal-tools.ts:92, 13ecca5408cb)
Sibling runtime classifier: The core/Pi-style isToolResultError classifier uses explicit failure statuses, so accepted, created, and updated are already non-errors outside the Codex app-server allowlist path. (src/agents/tool-result-error.ts:17, 13ecca5408cb)
Upstream Codex contract: Upstream Codex app-server forwards OpenClaw's dynamic-tool success boolean into core dynamic tool responses, and core uses that success value for tool output and post-tool-use hook behavior.

Likely related people:

vincentkoc: Recent GitHub history shows Codex dynamic-tool protocol synchronization and unreadable dynamic-tool quarantine work on the exact Codex app-server file. (role: recent area contributor; confidence: high; commits: ab1e5832d2fb, 3ffb3609a141; files: extensions/codex/src/app-server/dynamic-tools.ts)
steipete: Recent history shows work on Codex terminal outcome ordering and classifier refactors, and the goal tools trace to session goals work authored by steipete. (role: recent area contributor; confidence: high; commits: a09f6b1b2787, 0314819f918a, a509c48f0ea2; files: extensions/codex/src/app-server/dynamic-tools.ts, src/agents/tools/goal-tools.ts)
jalehman: Recent subagent-spawn history includes embedded run session target work on the producer file for accepted spawn results. (role: recent adjacent contributor; confidence: medium; commits: 0dfa22c6e0fc; files: src/agents/subagent-spawn.ts)
brokemac79: Recent path history includes dynamic-tool result reporting work in the Codex dynamic-tool bridge area, which shares the success/error classification invariant. (role: adjacent owner; confidence: medium; commits: d1299658ac4b; files: extensions/codex/src/app-server/dynamic-tools.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.

…s success (openclaw#96856) isCodexToolResultError fail-closes every tool-result status not in its non-error allowlist, but the allowlist omitted several success statuses emitted by OpenClaw tools that are exposed to codex agents: - sessions_spawn accepted launches -> details.status "accepted" - create_goal / update_goal results -> details.status "created" / "updated" So a successful accepted spawn (openclaw#96833), and successful goal create/update, were classified as errors: reported to codex as success: false (mapped to a Failed item status) and persisted on the transcript as isError: true. This adds those statuses to the allowlist alongside their sibling success statuses (completed/recorded/started/running). Genuinely failed or forbidden results (status "error"/"forbidden") stay fail-closed. Adds regression tests: accepted spawn and created/updated goal results are reported as successful dynamic tool calls; a forbidden spawn still fails.

…l calls (#98659) isCodexToolResultError fail-closes any dynamic-tool result whose details.status is absent from its non-error allowlist. get_goal returns details.status "found" or "missing" (a successful read of the thread goal, or its absence), neither of which was in the allowlist, so every get_goal call was classified as an error: reported to codex as success: false and persisted on the transcript with isError: true. Sibling #96856 added the write-side goal statuses (created/updated) and the accepted spawn status but missed the read-side get_goal statuses. This adds found/missing alongside them. Genuinely failed statuses stay fail-closed.

…s success (openclaw#96856) isCodexToolResultError fail-closes every tool-result status not in its non-error allowlist, but the allowlist omitted several success statuses emitted by OpenClaw tools that are exposed to codex agents: - sessions_spawn accepted launches -> details.status "accepted" - create_goal / update_goal results -> details.status "created" / "updated" So a successful accepted spawn (openclaw#96833), and successful goal create/update, were classified as errors: reported to codex as success: false (mapped to a Failed item status) and persisted on the transcript as isError: true. This adds those statuses to the allowlist alongside their sibling success statuses (completed/recorded/started/running). Genuinely failed or forbidden results (status "error"/"forbidden") stay fail-closed. Adds regression tests: accepted spawn and created/updated goal results are reported as successful dynamic tool calls; a forbidden spawn still fails.

…l calls (openclaw#98659) isCodexToolResultError fail-closes any dynamic-tool result whose details.status is absent from its non-error allowlist. get_goal returns details.status "found" or "missing" (a successful read of the thread goal, or its absence), neither of which was in the allowlist, so every get_goal call was classified as an error: reported to codex as success: false and persisted on the transcript with isError: true. Sibling openclaw#96856 added the write-side goal statuses (created/updated) and the accepted spawn status but missed the read-side get_goal statuses. This adds found/missing alongside them. Genuinely failed statuses stay fail-closed.

…s success (openclaw#96856) isCodexToolResultError fail-closes every tool-result status not in its non-error allowlist, but the allowlist omitted several success statuses emitted by OpenClaw tools that are exposed to codex agents: - sessions_spawn accepted launches -> details.status "accepted" - create_goal / update_goal results -> details.status "created" / "updated" So a successful accepted spawn (openclaw#96833), and successful goal create/update, were classified as errors: reported to codex as success: false (mapped to a Failed item status) and persisted on the transcript as isError: true. This adds those statuses to the allowlist alongside their sibling success statuses (completed/recorded/started/running). Genuinely failed or forbidden results (status "error"/"forbidden") stay fail-closed. Adds regression tests: accepted spawn and created/updated goal results are reported as successful dynamic tool calls; a forbidden spawn still fails.

…l calls (openclaw#98659) isCodexToolResultError fail-closes any dynamic-tool result whose details.status is absent from its non-error allowlist. get_goal returns details.status "found" or "missing" (a successful read of the thread goal, or its absence), neither of which was in the allowlist, so every get_goal call was classified as an error: reported to codex as success: false and persisted on the transcript with isError: true. Sibling openclaw#96856 added the write-side goal statuses (created/updated) and the accepted spawn status but missed the read-side get_goal statuses. This adds found/missing alongside them. Genuinely failed statuses stay fail-closed.

openclaw-barnacle Bot added extensions: codex size: S labels Jun 25, 2026

clawsweeper Bot mentioned this pull request Jun 27, 2026

fix: prevent sessions_spawn accepted results from being classified as tool errors (#96833) #97239

Closed

vincentkoc merged commit b3ff641 into openclaw:main Jun 29, 2026
133 of 141 checks passed

github-actions Bot mentioned this pull request Jun 29, 2026

📡 Upstream Digest — 2026-06-29 02:42 UTC curtismercier/openclaw-mods#1171

Open

clawsweeper Bot mentioned this pull request Jun 29, 2026

fix(agents): stop recording accepted sessions_spawn launches as tool failures #96851

Merged

yetval mentioned this pull request Jul 1, 2026

fix(codex): classify get_goal read statuses as successful dynamic tool calls #98659

Merged

vincentkoc mentioned this pull request Jul 6, 2026

chore(jobs): prune resolved OpenClaw inbox jobs openclaw/clownfish#221

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

fix(codex): successful sessions_spawn and goal tool results recorded as failures#96856

fix(codex): successful sessions_spawn and goal tool results recorded as failures#96856
vincentkoc merged 1 commit into
openclaw:mainfrom
nxmxbbd:nex/96833-codex-accepted-source

nxmxbbd commented Jun 25, 2026

Uh oh!

clawsweeper Bot commented Jun 25, 2026 •

edited

Loading

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Uh oh!

Conversation

nxmxbbd commented Jun 25, 2026

What Problem This Solves

Why This Change Was Made

User Impact

Evidence

Focused regression tests

Real behavior proof

Security / Compatibility

Risks and Mitigations

Uh oh!

clawsweeper Bot commented Jun 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

clawsweeper Bot commented Jun 25, 2026 •

edited

Loading