fix(compaction): keep accepted sessions_spawn results out of tool-failure summaries#96842
Conversation
…lure summaries Accepted sessions_spawn launches can be persisted as a toolResult with isError:true while the payload carries status=accepted. The compaction safeguard collected every isError toolResult, so normal accepted subagent launches surfaced as repeated '## Tool Failures' lines in user-visible summaries. collectToolFailures now skips results whose toolName is sessions_spawn and whose payload normalizes as an accepted child spawn, mirroring the observer's existing detection; this covers legacy transcripts too. Genuinely failed spawns (status=error/forbidden) and non-spawn tools are still reported.
|
Codex review: needs maintainer review before merge. Reviewed June 25, 2026, 3:46 PM ET / 19:46 UTC. Summary PR surface: Source +12, Tests +99. Total +111 across 2 files. Reproducibility: yes. Current main's Review metrics: none identified. Stored data model Root-cause cluster Members:
Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything. Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Risk before merge
Maintainer options:
Next step before merge
Security Review detailsBest possible solution: Land the compaction guard as a focused mitigation, then keep #96833 open if maintainers still want canonical source-side Do we have a high-confidence way to reproduce the issue? Yes. Current main's Is this the best way to solve the issue? Yes for the compaction symptom. Reusing the existing accepted-spawn helper at the compaction read boundary is the narrowest maintainable fix for legacy transcripts, while source-side normalization can remain separate. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 1aa7cafc35a1. Label changesLabel justifications:
Evidence reviewedPR surface: Source +12, Tests +99. Total +111 across 2 files. View PR surface stats
What I checked:
Likely related people:
What the crustacean ranks mean
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 re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. |
…lure summaries (openclaw#96842) Accepted sessions_spawn launches can be persisted as a toolResult with isError:true while the payload carries status=accepted. The compaction safeguard collected every isError toolResult, so normal accepted subagent launches surfaced as repeated '## Tool Failures' lines in user-visible summaries. collectToolFailures now skips results whose toolName is sessions_spawn and whose payload normalizes as an accepted child spawn, mirroring the observer's existing detection; this covers legacy transcripts too. Genuinely failed spawns (status=error/forbidden) and non-spawn tools are still reported.
…lure summaries (openclaw#96842) Accepted sessions_spawn launches can be persisted as a toolResult with isError:true while the payload carries status=accepted. The compaction safeguard collected every isError toolResult, so normal accepted subagent launches surfaced as repeated '## Tool Failures' lines in user-visible summaries. collectToolFailures now skips results whose toolName is sessions_spawn and whose payload normalizes as an accepted child spawn, mirroring the observer's existing detection; this covers legacy transcripts too. Genuinely failed spawns (status=error/forbidden) and non-spawn tools are still reported. (cherry picked from commit d4441f6)
…lure summaries (openclaw#96842) Accepted sessions_spawn launches can be persisted as a toolResult with isError:true while the payload carries status=accepted. The compaction safeguard collected every isError toolResult, so normal accepted subagent launches surfaced as repeated '## Tool Failures' lines in user-visible summaries. collectToolFailures now skips results whose toolName is sessions_spawn and whose payload normalizes as an accepted child spawn, mirroring the observer's existing detection; this covers legacy transcripts too. Genuinely failed spawns (status=error/forbidden) and non-spawn tools are still reported.
…lure summaries (openclaw#96842) Accepted sessions_spawn launches can be persisted as a toolResult with isError:true while the payload carries status=accepted. The compaction safeguard collected every isError toolResult, so normal accepted subagent launches surfaced as repeated '## Tool Failures' lines in user-visible summaries. collectToolFailures now skips results whose toolName is sessions_spawn and whose payload normalizes as an accepted child spawn, mirroring the observer's existing detection; this covers legacy transcripts too. Genuinely failed spawns (status=error/forbidden) and non-spawn tools are still reported.
…lure summaries (openclaw#96842) Accepted sessions_spawn launches can be persisted as a toolResult with isError:true while the payload carries status=accepted. The compaction safeguard collected every isError toolResult, so normal accepted subagent launches surfaced as repeated '## Tool Failures' lines in user-visible summaries. collectToolFailures now skips results whose toolName is sessions_spawn and whose payload normalizes as an accepted child spawn, mirroring the observer's existing detection; this covers legacy transcripts too. Genuinely failed spawns (status=error/forbidden) and non-spawn tools are still reported.
Related: #96833
What Problem This Solves
Fixes an issue where operators reviewing an agent's compaction summaries would see normal, successful
sessions_spawnsubagent launches reported as repeated## Tool Failures(for examplesessions_spawn: failed), even though each launch was accepted. An active agent that spawns subagents accumulates many of these false-positive failure lines, making the summary's failure list untrustworthy.Why This Change Was Made
An accepted
sessions_spawnlaunch can be persisted on the transcript as atoolResultcarryingisError: truewhile its payload isdetails.status: "accepted"(withrunIdandchildSessionKey). The compaction safeguard'scollectToolFailurescollected everytoolResultwithisError === true, so accepted launches were swept into the failure summary.The fix teaches the failure collector that an accepted child spawn is a successful launch, not a failure: after the existing
isErrorguard,collectToolFailuresnow skips results that the existingnormalizeAcceptedSessionSpawnResulthelper recognizes as accepted spawns. This is applied at the read boundary so it also covers legacy transcripts already persisted withisError: true. Spawns that genuinely failed (status: "error"/"forbidden") are unaffected and still reported.Scope boundary: this is a read-side guard in the compaction summary only. It does not change how tool results are persisted or
sessions_spawnexecution. The same persistedisError: trueon an accepted spawn also reaches other read paths (model replay mapstoolResult.isErrortois_error; the TUI renders the historical result with error styling); normalizing the accepted-spawn classification at its source is a separate follow-up and is intentionally not bundled here. The detection mirrors the existing observer check (toolName === "sessions_spawn"plus the accepted child-run identity), so non-spawn tools are never matched by payload shape alone. Because the linked issue's expected behavior also covers source-sideisErrornormalization, this PR links it withRelated:rather than a closing keyword, so merging does not automatically close the linked issue. Maintainers can decide whether the compaction-only fix should close it or whether the source-side normalization follow-up stays open.User Impact
Compaction summaries no longer list accepted subagent launches as tool failures. The
## Tool Failuressection now reflects only genuine tool failures, including spawns that were actually rejected or errored.Evidence
Tier: internal deterministic logic (
collectToolFailuresis a pure function overAgentMessage[]; no runtime/network boundary). Proof is a focused regression test that fails before and passes after the change, plus the existing suite, plus a rendered before/after of the real compaction section (below).Added regression tests in
src/agents/agent-hooks/compaction-safeguard.test.ts:sessions_spawnresult withisError: true+details.status: "accepted"is excluded fromcollectToolFailures(asserts length 0).sessions_spawnresults (status: "error"andstatus: "forbidden") are still reported (deny symmetry, guards against over-correction).jsonResulthelper: the accepted spawn is excluded, a failedexecis still reported, and a non-spawn tool carrying the same accepted-shaped details is still reported (proves thetoolNamegate and that the skip does not perturb ordering).Before the fix, the accepted-exclusion test failed (
expected length 0, got 1), reproducing the bug. After the fix:Touched-file gates (local):
oxfmt --checkclean;oxlintclean;tsgocore and core-test lanes exit 0.Real-behavior before/after (rendered compaction section)
Beyond the unit test, the actual exported
formatToolFailuresSection(collectToolFailures(messages))was run over a realistic transcript: two acceptedsessions_spawnlaunches (persistedisError: true,details.status: "accepted"withrunId+childSessionKey), one genuineexecfailure (exit 127), and one rejected (forbidden) spawn. The same harness was run against the pre-fix tree and this branch. The transcript is synthetic, so there is nothing private to redact.Before (pre-fix tree, no guard) — accepted launches pollute the section (
collectToolFailurescount = 4):After (this branch) — accepted launches are gone; the real
execfailure and theforbiddenspawn are still reported (count = 2):Only the two accepted launches are removed. To reproduce, import the
testingexports fromsrc/agents/agent-hooks/compaction-safeguard.tsand render the section on any transcript carrying an acceptedsessions_spawnresult.Security / Compatibility
No auth, secret, dependency, permission, migration, config, or data-access surface changes. No persisted-state shape change: the read-side guard leaves stored transcripts untouched and resolves the symptom for both new and existing transcripts.
Risks and Mitigations
The exclusion is keyed on the accepted-spawn contract (
status: "accepted"plusrunIdandchildSessionKey) via the sharednormalizeAcceptedSessionSpawnResulthelper, so only accepted child spawns are skipped; failed/forbidden spawns and all other tool failures continue to be reported, covered by the deny-symmetry test.AI-assisted; the contributor manually reviewed the change and is responsible for it.