Skip to content

fix: recover precheck context overflows with measured counts#98090

Closed
jalehman wants to merge 1 commit into
openclaw:mainfrom
jalehman:fix/preflight-overflow-token-counts
Closed

fix: recover precheck context overflows with measured counts#98090
jalehman wants to merge 1 commit into
openclaw:mainfrom
jalehman:fix/preflight-overflow-token-counts

Conversation

@jalehman

@jalehman jalehman commented Jun 30, 2026

Copy link
Copy Markdown
Contributor

Related: #96625

What Problem This Solves

Fixes an issue where users with near-full conversations could see auto-compaction report recovery attempts but still fail with a context-overflow warning when OpenClaw's own pre-prompt check detected the overflow before a provider request was sent.

Why This Change Was Made

The pre-prompt and mid-turn overflow checks already measure the rendered prompt pressure, but the synthetic recovery signal dropped those budget facts before the outer compaction loop ran. This change carries the measured token counts through the attempt result and uses them when provider error text has no parseable token count, while preserving provider-reported counts as higher-priority evidence.

User Impact

Auto-compaction can now target the prompt size OpenClaw actually measured for precheck-triggered overflows, reducing false recovery loops where compaction reports success but the retry remains over budget.

Evidence

  • node scripts/run-vitest.mjs src/agents/embedded-agent-runner/run.overflow-compaction.test.ts src/agents/embedded-agent-runner/run.overflow-compaction.loop.test.ts src/agents/embedded-agent-runner/run/preemptive-compaction.test.ts src/agents/embedded-agent-runner/run/attempt.spawn-workspace.context-engine.test.ts
    • 4 files passed, 161 tests passed
  • .agents/skills/autoreview/scripts/autoreview --mode branch --base upstream/main
    • autoreview clean: no accepted/actionable findings reported
  • git diff --check upstream/main..HEAD
    • passed

@openclaw-barnacle openclaw-barnacle Bot added agents Agent runtime and tooling size: S maintainer Maintainer-authored PR labels Jun 30, 2026
@clawsweeper

clawsweeper Bot commented Jun 30, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Codex review: needs real behavior proof before merge. Reviewed June 30, 2026, 4:40 AM ET / 08:40 UTC.

Summary
The branch adds optional preflight budget fields to embedded attempt results, fills them from pre-prompt and mid-turn decisions, and uses the measured prompt count for overflow compaction when provider text lacks a parseable token count.

PR surface: Source +49, Tests +41. Total +90 across 5 files.

Reproducibility: yes. source-reproducible but not live-reproduced here: current main computes precheck budget counts, then returns a preflightRecovery object that drops them before outer overflow compaction chooses currentTokenCount.

Review metrics: none identified.

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:

  • [P1] Add redacted terminal output, copied live output, or logs from a real near-full conversation showing precheck overflow recovery uses the measured prompt count; redact private endpoints, API keys, phone numbers, and other sensitive details.

Proof guidance:

  • [P1] Needs real behavior proof before merge: The PR body lists targeted Vitest files, autoreview, and diff check, but no after-fix real setup proof such as redacted terminal/live logs from an actual near-full conversation. 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 PR body provides targeted tests and autoreview output, but no redacted real near-full conversation, terminal log, or live-output proof showing the after-fix recovery path in a real setup.

Maintainer options:

  1. Decide the mitigation before merge
    Land this narrow count-propagation fix after redacted real runtime proof is added or a maintainer explicitly overrides the proof gate.
  2. Pause or close
    Do not merge this PR until maintainers decide whether the risk is worth taking.

Next step before merge

  • [P1] The remaining blocker is contributor proof or maintainer override, and the protected maintainer label also prevents cleanup closure or automated repair routing.

Security
Cleared: The diff touches internal agent runner code and tests only; I found no concrete security or supply-chain concern.

Review details

Best possible solution:

Land this narrow count-propagation fix after redacted real runtime proof is added or a maintainer explicitly overrides the proof gate.

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

Yes, source-reproducible but not live-reproduced here: current main computes precheck budget counts, then returns a preflightRecovery object that drops them before outer overflow compaction chooses currentTokenCount.

Is this the best way to solve the issue?

Yes for the code shape: carrying the existing measured fields through the existing recovery object is narrower than adding config or a new compaction policy, and provider-reported counts still take precedence.

AGENTS.md: found and applied where relevant.

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

Label changes

Label justifications:

  • P2: The PR targets a bounded agent-runtime overflow recovery bug that can affect near-full conversations but has limited blast radius.
  • 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 targeted Vitest files, autoreview, and diff check, but no after-fix real setup proof such as redacted terminal/live logs from an actual near-full conversation. 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 +49, Tests +41. Total +90 across 5 files.

View PR surface stats
Area Files Added Removed Net
Source 3 54 5 +49
Tests 2 42 1 +41
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 5 96 6 +90

What I checked:

  • Repository policy read: Root and scoped agent runner policy require whole-path review, best-fix assessment, and explicit proof handling for agent runtime changes. (AGENTS.md:1, 3d4b7cade9cd)
  • Live PR state: Live GitHub metadata shows this PR is open, mergeable, labeled agents/maintainer/size:S, and at head 6a2ed87 with no review comments. (6a2ed87f81f1)
  • Current-main gap: Current main constructs synthetic precheck recovery objects with only route/source/handled fields, so the measured estimatedPromptTokens already logged in the same decision is not returned to the outer run loop. (src/agents/embedded-agent-runner/run/attempt.ts:3891, 3d4b7cade9cd)
  • Current-main fallback behavior: Current main can pass provider-observed counts into compaction, but when an overflow is synthetic or unparseable it falls back to ctxInfo.tokens + 1 rather than the measured rendered prompt size. (src/agents/embedded-agent-runner/run.ts:2639, 3d4b7cade9cd)
  • Decision source has measured counts: The pre-prompt decision contract already includes estimatedPromptTokens, promptBudgetBeforeReserve, and overflowTokens, so the PR reuses existing measured facts rather than adding a new policy surface. (src/agents/embedded-agent-runner/run/preemptive-compaction.ts:35, 3d4b7cade9cd)
  • PR implementation: At PR head, the outer overflow path keeps provider-observed counts highest priority, then uses preflightEstimatedPromptTokens before the old minimally-over-budget fallback. (src/agents/embedded-agent-runner/run.ts:2639, 6a2ed87f81f1)

Likely related people:

  • jalehman: Merged and authored recent context-engine precheck changes in fix(agents): skip pre-prompt precheck when context engine owns compaction #95342 and contributed earlier context-engine loop/precheck follow-up commits in related merged history. (role: recent area contributor and merger; confidence: high; commits: 14d361b0050a, 63059958a06c, e2758fee9b1d; files: src/agents/embedded-agent-runner/run/attempt.ts, src/agents/embedded-agent-runner/run.overflow-compaction.loop.test.ts, src/agents/embedded-agent-runner/run/attempt.spawn-workspace.context-engine.test.ts)
  • mpz4life: Authored the merged ownsCompaction precheck-skip PR that recently touched the same attempt runner precheck path now under review. (role: recent precheck behavior contributor; confidence: high; commits: 76de8a85fc25, 35a0667e7156; files: src/agents/embedded-agent-runner/run/attempt.ts, src/agents/embedded-agent-runner/run.overflow-compaction.loop.test.ts)
  • yetval: Authored the merged pre-prompt overflow estimator fix for bashExecution and summary turns, which shares the measured-token invariant this PR carries into recovery. (role: recent token-estimation contributor; confidence: medium; commits: c68993e94656; files: src/agents/embedded-agent-runner/run/preemptive-compaction.ts, src/agents/embedded-agent-runner/run/preemptive-compaction.bashexec.test.ts)
  • vincentkoc: Merged the recent pre-prompt overflow estimator fix and is therefore a useful routing candidate for the budget-estimation side of this path. (role: recent merger for adjacent estimator change; confidence: medium; commits: c68993e94656; files: src/agents/embedded-agent-runner/run/preemptive-compaction.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. labels Jun 30, 2026
@clawsweeper clawsweeper Bot mentioned this pull request Jun 30, 2026
Closed
@ChrisBot2026 ChrisBot2026 mentioned this pull request Jul 1, 2026
Open
This was referenced Jul 5, 2026
Open
Merged
@steipete

steipete commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

Closing because the same-author broader implementation landed in #101181 as commit d5e9b4d1a32f5c708aa38b3693debad23a9b1403. The landed change preserves measured preflight overflow counts and adds the surrounding recovery budgeting, so no unique behavior remains here.

Thank you @jalehman for the fix and follow-through.

@steipete steipete closed this Jul 10, 2026
@jalehman

jalehman commented Jul 10, 2026

Copy link
Copy Markdown
Contributor Author

Oops, thanks for cleaning this up!

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

agents Agent runtime and tooling maintainer Maintainer-authored PR P2 Normal backlog priority with limited blast radius. 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.

2 participants

@jalehman @steipete