Skip to content

fix(compaction): add circuit breaker to stop token burn when summarizer unavailable#86900

Merged
steipete merged 5 commits into
openclaw:mainfrom
tanshanshan:fix/compaction-circuit-breaker-58838
Jul 17, 2026
Merged

fix(compaction): add circuit breaker to stop token burn when summarizer unavailable#86900
steipete merged 5 commits into
openclaw:mainfrom
tanshanshan:fix/compaction-circuit-breaker-58838

Conversation

@tanshanshan

@tanshanshan tanshanshan commented May 26, 2026

Copy link
Copy Markdown
Contributor

Summary

Fixes #58838. Staged compaction no longer burns requests across every split when summarization repeatedly falls back generically.

The maintainer rewrite:

  • classifies real summaries and generic fallbacks with a discriminated internal result;
  • opens the circuit after two consecutive generic split fallbacks;
  • cancels the incomplete compaction so unprocessed transcript messages remain intact;
  • resets the consecutive counter after a successful split while retaining degraded state through merge;
  • preserves a known prior summary when an isolated generic fallback prevents redistillation.

What Problem This Solves

When the summarizer is persistently unavailable, staged compaction can exhaust the retry/fallback path for every split. That multiplies token and latency cost. Stopping the loop by returning the summaries collected so far is unsafe: later transcript chunks were never processed, yet compaction would commit and drop them. This fix bounds the storm and cancels the incomplete compaction so the transcript survives.

Evidence

node scripts/run-vitest.mjs src/agents/compaction.circuit-breaker.test.ts src/agents/compaction.identifier-preservation.test.ts src/agents/compaction.summarize-fallback.test.ts src/agents/compaction-partial-summary.test.ts src/agents/agent-hooks/compaction-safeguard.test.ts passed 5 files and 133 tests. The breaker test proves exactly two calls before circuit-open and rejection; the intermittent test proves counter reset plus degraded merge state; safeguard coverage proves prior-summary retention. Fresh full-diff autoreview is clean.

Change Type (select all)

  • Bug fix
  • Feature
  • Refactor required for the fix
  • Docs
  • Security hardening
  • Chore/infra

Scope (select all touched areas)

  • Gateway / orchestration
  • Skills / tool execution
  • Auth / tokens
  • Memory / storage
  • Integrations
  • API / contracts
  • UI / DX
  • CI/CD / infra

Linked Issue/PR

Motivation

A persistently unavailable summarizer could make every staged split exhaust its retry/fallback path, multiplying token and latency cost. Returning an incomplete partial summary would also risk committing away splits that were never processed. The breaker must stop the request storm and preserve the source transcript.

Real Behavior Proof

Focused regression command:

node scripts/run-vitest.mjs src/agents/compaction.circuit-breaker.test.ts src/agents/compaction.identifier-preservation.test.ts src/agents/compaction.summarize-fallback.test.ts src/agents/compaction-partial-summary.test.ts src/agents/agent-hooks/compaction-safeguard.test.ts

Result: 5 test files passed, 133 tests passed.

The circuit-breaker regression proves two consecutive generic fallbacks make only two summarization calls and reject the incomplete compaction. The intermittent sequence proves a successful split resets the consecutive counter, the merge still runs, and the typed result remains degraded. Safeguard coverage proves a prior summary survives degraded redistillation.

Scoped changed-gate command:

node scripts/check-changed.mjs -- src/agents/compaction.ts src/agents/compaction.circuit-breaker.test.ts src/agents/agent-hooks/compaction-safeguard.ts src/agents/agent-hooks/compaction-safeguard.test.ts

Conflict markers, max-lines ratchet, changelog attribution, wildcard export guards, duplicate coverage, dependency pins, and formatting passed. The delegated run then encountered pre-existing Plugin SDK API baseline drift on current main; this PR changes no Plugin SDK path or export. Exact-head hosted CI remains the landing gate.

Fresh full-diff autoreview result: clean, no accepted/actionable findings.

Root Cause

The staged loop treated a generic fallback string like a usable summary and had no typed signal for repeated failure. That made it impossible to bound repeated calls without brittle string matching, and an early partial return could silently discard unprocessed transcript chunks.

Regression Test Plan

  1. Two consecutive generic fallbacks open the circuit after two calls and reject the compaction.
  2. A successful split resets the consecutive counter and allows the merge.
  3. Any generic split keeps the merged result typed as degraded.
  4. A degraded result preserves the previous summary at the safeguard owner.
  5. Existing fallback, partial-summary, and identifier-preservation tests remain green.

User-visible / Behavior Changes

During a sustained summarizer outage, OpenClaw stops staged summarization after two consecutive generic fallbacks and cancels that compaction attempt. The session transcript remains available for a later retry instead of being replaced by an incomplete placeholder summary.

Release-note context: compaction now stops repeated staged fallback storms while preserving session history when the summarizer is unavailable. Thanks @tanshanshan.

Security Impact (required)

  • New permissions/capabilities? No
  • Secrets/tokens handling changed? No
  • New/changed network calls? No
  • Command/tool execution surface changed? No
  • Data access scope changed? No

Human Verification (required)

  • Verified scenarios: consecutive fallback circuit-open; intermittent fallback recovery; degraded merge classification; prior-summary retention.
  • Edge cases checked: partial model output remains a real summary; aborts still propagate; single-pass and custom-provider paths remain unchanged.
  • Not verified: a deliberate live production-provider outage. Deterministic regression tests exercise the same internal failure states without spending external tokens.

Review Conversations

  • I replied to or resolved every bot review conversation I addressed in this PR.
  • I left unresolved only the conversations that still need reviewer or maintainer judgment.

Compatibility / Migration

  • Backward compatible? Yes
  • Config/env changes? No
  • Migration needed? No

Risks and Mitigations

Risk: stopping early could lose current transcript context if an incomplete summary were committed. Mitigation: circuit-open throws into the safeguard's existing cancel path, which retains the original transcript. No config, storage schema, or public API changes.

@openclaw-barnacle openclaw-barnacle Bot added agents Agent runtime and tooling size: S triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup. labels May 26, 2026
@clawsweeper

clawsweeper Bot commented May 26, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs real behavior proof before merge. Reviewed July 17, 2026, 3:14 AM ET / 07:14 UTC.

Summary
The PR adds typed compaction summary outcomes, aborts staged compaction after two consecutive generic fallbacks, and preserves an existing summary when degraded redistillation fails.

PR surface: Source +52, Tests +142. Total +194 across 4 files.

Reproducibility: yes. at source level: persistent summarizer failures traverse each staged split's retry and generic-fallback path, and the focused tests deterministically exercise that condition. A failing live current-main run was not performed during this review.

Review metrics: 1 noteworthy metric.

  • Fixed Fallback Cutoff: 1 threshold added: 2 consecutive generic fallbacks. This fixed cutoff determines when staged compaction abandons later transcript splits and enters the session-preserving cancellation path.

Root-cause cluster
Relationship: fixed_by_candidate
Canonical: #58838
Summary: This PR is the active candidate fix for the canonical staged-compaction token-burn issue; earlier breaker PRs closed unmerged, while the merged partial-summary and lock fixes address adjacent failure modes.

Members:

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

Merge readiness
Overall: 🦪 silver shellfish
Proof: 🦪 silver shellfish
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:

  • [P1] Add current-head terminal or log proof showing the breaker stops later calls and cancellation preserves the source transcript; redact keys, endpoints, identifiers, and other private data.
  • [P1] Refresh against current main if needed and let exact-head CI finish.

Proof guidance:

  • [P1] Needs real behavior proof before merge: The rewritten head is supported by focused mocked regression tests but no current-head runtime or compiled fault-injection proof; add redacted terminal or log evidence to the PR body, which should trigger a fresh review, or ask a maintainer to 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 changed head has not demonstrated in a real runtime or compiled fault-injection setup that two generic fallbacks stop subsequent requests and reach the transcript-preserving cancellation owner.
  • [P1] Merging changes session-state failure behavior: sustained summarizer outages now abort compaction after a fixed cutoff instead of continuing later splits and merge summarization.
  • [P1] The PR is behind current main and exact-head CI remains incomplete, so final integration behavior is not yet established.

Maintainer options:

  1. Prove cancellation on the rewritten head (recommended)
    Run a real or compiled fault-injection compaction that triggers two generic fallbacks and shows both bounded calls and unchanged source transcript before merging.
  2. Apply an explicit proof override
    A maintainer may accept the deterministic regression suite as sufficient for this internal path while explicitly owning the unobserved runtime cancellation risk.

Next step before merge

  • [P1] The existing branch already contains the focused repair; the remaining work is current-head real-behavior proof, exact-head CI, and normal maintainer merge review rather than another fix PR.

Security
Cleared: The diff changes internal compaction control flow and tests without new dependencies, permissions, secrets, downloads, workflow privileges, or package execution surfaces.

Review details

Best possible solution:

Keep the typed internal result and incomplete-compaction cancellation design, add current-head runtime or compiled fault-injection output proving that later calls stop and the original transcript remains intact, then merge after exact-head checks pass.

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

Yes at source level: persistent summarizer failures traverse each staged split's retry and generic-fallback path, and the focused tests deterministically exercise that condition. A failing live current-main run was not performed during this review.

Is this the best way to solve the issue?

Yes. Typed internal outcomes plus cancellation of an incomplete compaction is narrower and safer than string matching, committing partial coverage, or adding a process-wide time-based breaker.

AGENTS.md: found and applied where relevant.

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

Label changes

Label changes:

  • add rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🦪 silver shellfish and patch quality is 🐚 platinum hermit.
  • add status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: The rewritten head is supported by focused mocked regression tests but no current-head runtime or compiled fault-injection proof; add redacted terminal or log evidence to the PR body, which should trigger a fresh review, or ask a maintainer to 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.
  • remove proof: sufficient: Current real behavior proof status is mock_only, not sufficient.
  • remove rating: 🐚 platinum hermit: Current PR rating is rating: 🦪 silver shellfish, so this older rating label is no longer current.
  • remove status: 👀 ready for maintainer look: Current PR status label is status: 📣 needs proof.

Label justifications:

  • P2: The PR addresses bounded but meaningful compaction token burn and session continuity during sustained summarizer failures.
  • merge-risk: 🚨 session-state: The patch changes when compaction aborts and which summary state survives, so an incorrect caller transition could lose, duplicate, or stale session context.
  • rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🦪 silver shellfish 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 rewritten head is supported by focused mocked regression tests but no current-head runtime or compiled fault-injection proof; add redacted terminal or log evidence to the PR body, which should trigger a fresh review, or ask a maintainer to 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 +52, Tests +142. Total +194 across 4 files.

View PR surface stats
Area Files Added Removed Net
Source 2 76 24 +52
Tests 2 205 63 +142
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 4 281 87 +194

What I checked:

  • Typed fallback state: The PR head distinguishes usable summaries from generic fallbacks and defines a fixed two-fallback cutoff without adding public API or configuration surface. (src/agents/compaction.ts:42, 77024951dc43)
  • Transcript-preserving cancellation: The rewritten branch rejects an incomplete circuit-open compaction rather than committing summaries that omit unprocessed transcript splits. (src/agents/compaction.ts:265, 77024951dc43)
  • Prior-summary retention: The safeguard unwraps successful results and retains the known previous summary when redistillation returns a generic fallback. (src/agents/agent-hooks/compaction-safeguard.ts:397, 77024951dc43)
  • Focused regression coverage: The added test injects summarizer failures and checks the staged circuit-breaker path; the PR reports five focused files and 133 passing tests. (src/agents/compaction.circuit-breaker.test.ts:1, 77024951dc43)
  • Current-head proof gap: The rewritten PR body explicitly says a deliberate runtime provider-outage scenario was not verified; the supplied evidence for this head consists of deterministic regression tests and mocks. (77024951dc43)
  • Maintainer adoption: steipete assigned the PR and rewrote the branch into four focused compaction commits on July 17, 2026, providing current maintainer sponsorship for the implementation direction. (src/agents/compaction.ts:42, 1581286021bd)

Likely related people:

  • steipete: Introduced the staged-compaction helper in earlier history and authored the current maintainer rewrite across the breaker, degraded-state, and cancellation paths. (role: feature owner and recent rewriter; confidence: high; commits: ae0741a3466c, 1581286021bd, c59249f9c246; files: src/agents/compaction.ts, src/agents/agent-hooks/compaction-safeguard.ts, src/agents/compaction.circuit-breaker.test.ts)
  • amabito: Added the nearby per-chunk summary retry behavior central to the reported token-burn path and its regression coverage. (role: retry behavior contributor; confidence: high; commits: 068b9c97498d; files: src/agents/compaction.ts, src/agents/compaction.retry.test.ts)
  • SebTardif: Authored the merged partial-summary recovery work that established nearby fallback and incomplete-summary behavior used by this fix. (role: recent adjacent fallback contributor; confidence: high; commits: f0061ddc54c9; files: src/agents/compaction.ts, src/agents/compaction-partial-summary.test.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 (1 earlier review cycle)
  • reviewed 2026-06-24T18:19:27.405Z sha 0188074 :: needs maintainer review before merge. :: none

@openclaw-barnacle openclaw-barnacle Bot added proof: supplied External PR includes structured after-fix real behavior proof. and removed triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup. labels May 26, 2026
@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. labels May 26, 2026
@clawsweeper

clawsweeper Bot commented May 26, 2026

Copy link
Copy Markdown
Contributor

ClawSweeper PR egg: 🎁 locked until real behavior proof passes.

Details
  • No creature or rarity is rolled until proof passes.
  • Eggs are collectible flavor only; they do not affect labels, ratings, merge decisions, or automation.

@martingarramon martingarramon 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.

Resetting consecutiveChunkFailures on each successful chunk is correct — the breaker targets a persistently unavailable summarizer, not a single hard-to-summarize chunk.

One accuracy issue in the circuit-breaker catch block. When the breaker trips at the second consecutive failure (chunk at zero-based index N), completedChunks = chunks.indexOf(chunk) equals N. But the preceding chunk (index N-1) also failed and was not summarized, so the annotation:

chunks 1-${completedChunks} of ${chunks.length} were summarized

overcounts by one — it should read chunks 1-${completedChunks - 1}. The accumulated summary content is correct (it only updates on success); the annotation text is what's off.

The previousSummary ?? DEFAULT_SUMMARY_FALLBACK path when no chunk has succeeded handles the cold-start unavailable case cleanly.

@clawsweeper clawsweeper Bot added status: 🛠️ actively grinding The PR author has acted after the latest ClawSweeper review and work remains. merge-risk: 🚨 session-state 🚨 May lose, corrupt, stale, or mis-associate session, agent, or context state. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. and removed status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. status: 🛠️ actively grinding The PR author has acted after the latest ClawSweeper review and work remains. labels May 27, 2026
@tanshanshan

This comment was marked as low quality.

@clawsweeper

clawsweeper Bot commented May 27, 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.

Re-review progress:

@clawsweeper clawsweeper Bot added rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. and removed rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. labels May 27, 2026
@clawsweeper clawsweeper Bot added status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. and removed 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 May 28, 2026
@openclaw-barnacle openclaw-barnacle Bot added proof: supplied External PR includes structured after-fix real behavior proof. and removed triage: mock-only-proof Candidate: PR proof only shows tests, mocks, snapshots, lint, typecheck, or CI. labels May 28, 2026
@tanshanshan

This comment was marked as low quality.

@RomneyDa

Copy link
Copy Markdown
Member

Heads up: this PR needs to be updated against current main before the new required Dependency Guard check can pass.

@tanshanshan

This comment was marked as low quality.

@tanshanshan

This comment was marked as low quality.

@clawsweeper

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

@tanshanshan

This comment was marked as low quality.

@clawsweeper

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

Re-review progress:

@tanshanshan

This comment was marked as low quality.

@clawsweeper

clawsweeper Bot commented Jun 3, 2026

Copy link
Copy Markdown
Contributor

🦞👀
ClawSweeper picked this up.

Command router queued. I will update this comment with the next step.

Re-review progress:

@clawsweeper

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

Re-review progress:

@tanshanshan

This comment was marked as low quality.

@openclaw-barnacle

Copy link
Copy Markdown

This pull request has been automatically marked as stale due to inactivity.
Please add updates or it will be closed.

@steipete

Copy link
Copy Markdown
Contributor

Merged via squash.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

agents Agent runtime and tooling merge-risk: 🚨 session-state 🚨 May lose, corrupt, stale, or mis-associate session, agent, or context state. P2 Normal backlog priority with limited blast radius. proof: supplied External PR includes structured after-fix real behavior proof. rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. size: M stale Marked as stale due to inactivity status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix(compaction): retry loop can burn tokens when summarizer model is unavailable

4 participants