Skip to content

fix(compaction): keep accepted sessions_spawn results out of tool-failure summaries#96842

Merged
vincentkoc merged 1 commit into
openclaw:mainfrom
nxmxbbd:fix/96833-spawn-accepted-not-toolfail
Jun 29, 2026
Merged

fix(compaction): keep accepted sessions_spawn results out of tool-failure summaries#96842
vincentkoc merged 1 commit into
openclaw:mainfrom
nxmxbbd:fix/96833-spawn-accepted-not-toolfail

Conversation

@nxmxbbd

@nxmxbbd nxmxbbd commented Jun 25, 2026

Copy link
Copy Markdown
Contributor

Related: #96833

What Problem This Solves

Fixes an issue where operators reviewing an agent's compaction summaries would see normal, successful sessions_spawn subagent launches reported as repeated ## Tool Failures (for example sessions_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_spawn launch can be persisted on the transcript as a toolResult carrying isError: true while its payload is details.status: "accepted" (with runId and childSessionKey). The compaction safeguard's collectToolFailures collected every toolResult with isError === 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 isError guard, collectToolFailures now skips results that the existing normalizeAcceptedSessionSpawnResult helper recognizes as accepted spawns. This is applied at the read boundary so it also covers legacy transcripts already persisted with isError: 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_spawn execution. The same persisted isError: true on an accepted spawn also reaches other read paths (model replay maps toolResult.isError to is_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-side isError normalization, this PR links it with Related: 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 Failures section now reflects only genuine tool failures, including spawns that were actually rejected or errored.

Evidence

Tier: internal deterministic logic (collectToolFailures is a pure function over AgentMessage[]; 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:

  • accepted sessions_spawn result with isError: true + details.status: "accepted" is excluded from collectToolFailures (asserts length 0).
  • genuinely failed sessions_spawn results (status: "error" and status: "forbidden") are still reported (deny symmetry, guards against over-correction).
  • a mixed batch where the accepted details are built via the production jsonResult helper: the accepted spawn is excluded, a failed exec is still reported, and a non-spawn tool carrying the same accepted-shaped details is still reported (proves the toolName gate 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:

node scripts/run-vitest.mjs src/agents/agent-hooks/compaction-safeguard.test.ts
 Test Files  1 passed (1)
      Tests  98 passed (98)

Touched-file gates (local): oxfmt --check clean; oxlint clean; tsgo core 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 accepted sessions_spawn launches (persisted isError: true, details.status: "accepted" with runId + childSessionKey), one genuine exec failure (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 (collectToolFailures count = 4):

## Tool Failures
- sessions_spawn (status=accepted): Accepted: launching child session to scan logs.
- sessions_spawn (status=accepted): Accepted: launching child session to repro locally.
- exec (status=error exitCode=127): command not found: pnpm (exit 127)
- sessions_spawn (status=forbidden): Forbidden: spawn limit reached.

After (this branch) — accepted launches are gone; the real exec failure and the forbidden spawn are still reported (count = 2):

## Tool Failures
- exec (status=error exitCode=127): command not found: pnpm (exit 127)
- sessions_spawn (status=forbidden): Forbidden: spawn limit reached.

Only the two accepted launches are removed. To reproduce, import the testing exports from src/agents/agent-hooks/compaction-safeguard.ts and render the section on any transcript carrying an accepted sessions_spawn result.

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" plus runId and childSessionKey) via the shared normalizeAcceptedSessionSpawnResult helper, 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.

…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.
@openclaw-barnacle openclaw-barnacle Bot added agents Agent runtime and tooling size: S labels Jun 25, 2026
@clawsweeper

clawsweeper Bot commented Jun 25, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs maintainer review before merge. Reviewed June 25, 2026, 3:46 PM ET / 19:46 UTC.

Summary
The branch skips accepted sessions_spawn tool results when collecting compaction tool-failure summaries and adds regression tests for accepted, failed, forbidden, and look-alike tool results.

PR surface: Source +12, Tests +99. Total +111 across 2 files.

Reproducibility: yes. Current main's collectToolFailures reports any toolResult with isError === true, and the PR tests exercise the accepted sessions_spawn payload that should be skipped.

Review metrics: none identified.

Stored data model
Persistent data-model change detected: serialized state: src/agents/agent-hooks/compaction-safeguard.test.ts. Confirm migration or upgrade compatibility proof before merge.

Root-cause cluster
Relationship: fixed_by_candidate
Canonical: #96833
Summary: This PR is a candidate partial fix for the canonical issue's compaction-summary symptom; the issue also tracks source-side classification that may remain after merge.

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] The source-side isError normalization requested by the linked issue remains outside this compaction-only PR, so maintainers should decide whether that issue stays open after merge.

Maintainer options:

  1. Decide the mitigation before merge
    Land the compaction guard as a focused mitigation, then keep sessions_spawn accepted results are recorded as tool failures #96833 open if maintainers still want canonical source-side isError normalization.
  2. Pause or close
    Do not merge this PR until maintainers decide whether the risk is worth taking.

Next step before merge

  • No ClawSweeper repair lane is needed; the branch has no concrete code defect from this review and is on the normal maintainer review path.

Security
Cleared: The diff only changes an internal compaction classifier and tests; it does not alter dependencies, permissions, auth, secrets, CI, or persisted state shape.

Review details

Best possible solution:

Land the compaction guard as a focused mitigation, then keep #96833 open if maintainers still want canonical source-side isError normalization.

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

Yes. Current main's collectToolFailures reports any toolResult with isError === true, and the PR tests exercise the accepted sessions_spawn payload that should be skipped.

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 changes

Label justifications:

  • P2: This fixes a concrete user-visible compaction summary correctness bug with limited blast radius in the agent transcript path.
  • 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 copied after-fix output from rendering the real compaction failure section on a synthetic transcript, plus the before output and focused test command output.
  • proof: sufficient: Contributor real behavior proof is sufficient. The PR body includes copied after-fix output from rendering the real compaction failure section on a synthetic transcript, plus the before output and focused test command output.
Evidence reviewed

PR surface:

Source +12, Tests +99. Total +111 across 2 files.

View PR surface stats
Area Files Added Removed Net
Source 1 12 0 +12
Tests 1 99 0 +99
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 2 111 0 +111

What I checked:

  • Current main reproduces the compaction false positive: collectToolFailures on current main includes every toolResult with isError === true before formatting the ## Tool Failures section, so an accepted spawn persisted with that flag is still summarized as a failure. (src/agents/agent-hooks/compaction-safeguard.ts:447, 1aa7cafc35a1)
  • Patch uses the existing accepted-spawn contract: The proposed guard checks toolName for sessions_spawn and then calls normalizeAcceptedSessionSpawnResult, so it suppresses only accepted child-session spawns with recognized runId and childSessionKey details. (src/agents/agent-hooks/compaction-safeguard.ts:451, 079397b2c385)
  • Accepted-spawn helper is narrow: normalizeAcceptedSessionSpawnResult only returns a value for details.status === "accepted" with non-empty normalized runId and childSessionKey, which matches the PR's intended skip boundary. (src/agents/accepted-session-spawn.ts:13, 1aa7cafc35a1)
  • Sibling observer path uses the same invariant: The embedded tool-result handler records accepted session spawns only when the normalized tool name is sessions_spawn, the result is not a tool error, and the same accepted-spawn helper recognizes the payload. (src/agents/embedded-agent-subscribe.handlers.tools.ts:1181, 1aa7cafc35a1)
  • Regression tests cover the risky boundaries: The PR adds tests for excluding accepted sessions_spawn, preserving genuine error and forbidden spawn failures, and still reporting non-spawn look-alikes that share the accepted-shaped details. (src/agents/agent-hooks/compaction-safeguard.test.ts:269, 079397b2c385)
  • Live PR state and CI: GitHub reports the PR mergeable/clean, and gh pr checks shows the relevant CI, lint/type, security, and real behavior proof checks passing for the current head. (079397b2c385)

Likely related people:

  • steipete: Recent merged terminal-classifier work added tool-result-error.ts and changed embedded runner/subscriber error-classification paths that define the sibling invariant for this compaction fix. (role: recent area contributor; confidence: medium; commits: 0314819f918a, 6443d067645f; files: src/agents/tool-result-error.ts, src/agents/embedded-agent-runner/extensions.ts, src/agents/embedded-agent-subscribe.handlers.tools.ts)
  • samzong: The accepted-spawn helper used by this PR traces to samzong's source PR for preserving accepted spawn terminal success, later merged through the replacement PR. (role: introduced related accepted-spawn behavior; confidence: medium; commits: 7f4bd454febf, 060e64a58c45; files: src/agents/accepted-session-spawn.ts, src/agents/embedded-agent-subscribe.handlers.tools.ts)
  • vincentkoc: Recent adjacent work touched embedded tool-result detail handling, and Vincent merged nearby agent/runtime changes in this area. (role: recent adjacent contributor; confidence: medium; commits: d75613e794da; files: src/agents/embedded-agent-subscribe.handlers.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.

@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. P2 Normal backlog priority with limited blast radius. merge-risk: 🚨 other 🚨 Merging this PR has meaningful risk outside the owned taxonomy. 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: 🧂 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. merge-risk: 🚨 other 🚨 Merging this PR has meaningful risk outside the owned taxonomy. 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. labels Jun 25, 2026
@nxmxbbd

nxmxbbd commented Jun 25, 2026

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented Jun 25, 2026

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. rating: 🦞 diamond lobster Very strong PR readiness with only minor maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. labels Jun 25, 2026
@vincentkoc
vincentkoc merged commit d4441f6 into openclaw:main Jun 29, 2026
200 of 220 checks passed
github-actions Bot pushed a commit to Desicool/openclaw that referenced this pull request Jun 29, 2026
…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.
Rorqualx pushed a commit to Rorqualx/cortex that referenced this pull request Jun 29, 2026
…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)
QiuYuang pushed a commit to QiuYuang/openclaw that referenced this pull request Jul 1, 2026
…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.
chenyangjun-xy pushed a commit to chenyangjun-xy/openclaw that referenced this pull request Jul 1, 2026
…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.
chenyangjun-xy pushed a commit to chenyangjun-xy/openclaw that referenced this pull request Jul 3, 2026
…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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

agents Agent runtime and tooling P2 Normal backlog priority with limited blast radius. proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🦞 diamond lobster Very strong PR readiness with only minor maintainer review expected. size: S status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants