Skip to content

Force manual compaction past empty kept-tail cuts#94046

Open
amknight wants to merge 2 commits into
mainfrom
fix/skip-empty-compaction-summary
Open

Force manual compaction past empty kept-tail cuts#94046
amknight wants to merge 2 commits into
mainfrom
fix/skip-empty-compaction-summary

Conversation

@amknight

@amknight amknight commented Jun 17, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

Problem

Manual /compact failed to create a checkpoint when a session reported high context usage but held only a tiny transcript.

Context size is measured from provider usage (usage.totalTokens, which includes the cached system prompt and tool definitions), so it can read ~170k even when the actual conversation is just a couple of short messages. In that state the cut-point planner keeps the whole branch within the keepRecentTokens (20k) budget and selects zero older messages, so prepareCompaction returns undefined:

  • Automatic compaction correctly skips — there is no old prefix worth summarizing yet.
  • Manual /compact instead surfaced a confusing Nothing to compact (session too small), even though the user explicitly asked for a checkpoint.

"0 messages to summarize" here means "the retained-tail heuristic found no old prefix to replace" — not "the session is empty".

Fix

  1. Add a force option to prepareCompaction. Manual session/harness compaction pass force: true; automatic does not. When the heuristic would summarize zero messages, the forced path summarizes the whole branch so /compact produces a real checkpoint.
  2. The forced boundary anchors firstKeptEntryId on the last valid cut point (findValidCutPoints, which excludes tool results), not the raw final entry.

Further context

  • Automatic behavior is unchanged. Without force, the zero-message case still returns undefined → skip, so automatic compaction never writes empty no-op summaries.
  • The force path is narrowly scoped. It only changes the zero-old-prefix case; every normal cut runs untouched. Truly empty / already-compacted sessions are still refused — the path proceeds only when there is at least one real message.
  • No content is lost. The whole branch is summarized, so when manual compaction later collapses the boundary to summary-only (hardenManualCompactionBoundary), nothing is dropped silently.
  • The retained tail is always provider-valid. Anchoring on a valid cut point — the same rule the normal path already trusts — keeps any trailing tool result paired with the assistant tool call that owns it, instead of replaying an orphaned tool result that providers reject. If no valid anchor exists, the path returns undefined rather than emitting an orphan.

Tests

  • pnpm test packages/agent-core/src/harness/compaction/compaction.test.ts — automatic skip, manual force, and the valid-anchor / no-orphan-tool-result regression
  • pnpm test src/agents/embedded-agent-runner/manual-compaction-boundary.test.ts
  • pnpm test src/agents/embedded-agent-runner/compact.hooks.test.ts

@openclaw-barnacle openclaw-barnacle Bot added size: S maintainer Maintainer-authored PR labels Jun 17, 2026
@amknight amknight changed the title Skip no-op compaction when nothing would be summarized Force manual compaction past empty kept-tail cuts Jun 17, 2026
@amknight
amknight force-pushed the fix/skip-empty-compaction-summary branch from 97fafad to 7706e3e Compare June 17, 2026 09:20
@openclaw-barnacle openclaw-barnacle Bot added the agents Agent runtime and tooling label Jun 17, 2026
@blacksmith-sh

This comment has been minimized.

Sign in to view
@amknight
amknight force-pushed the fix/skip-empty-compaction-summary branch from 70e9f51 to fd09d2e Compare June 17, 2026 11:23
@amknight

amknight commented Jun 17, 2026

Copy link
Copy Markdown
Member Author

Local E2E verification (real LLM call)

Exercised the forced manual-compaction path end-to-end from source — in-process embedded runner, isolated OPENCLAW_STATE_DIR, real provider — by sending a message, then triggering manual /compact.

Forcing works as intended. A 2-message session (transcript text well under the 20k keepRecentTokens budget) made the normal cut-planner select 0 messages; force: true summarized the whole branch and appended a real compaction entry (tokensBefore ≈ 20.5k, 292-char summary). Pre-PR, this path threw Nothing to compact (session too small). ✔

Session lifecycle. Stays in the same session — same session id, same transcript file, compaction entry appended. firstKeptEntryId resolves to the compaction's own id (summary-only, because keepRecentTokens is unset → preserveRecentTail=false → hardenManualCompactionBoundary collapses the tail). A new session / rotated transcript only happens with truncateAfterCompaction: true.

Proof caveats (what wasn't covered):

  • Ran with xai/grok-4.3 (standard harness), not gpt-5.5 — the only local OpenAI auth is an openai-codex OAuth profile, which refuses embedded execution under tools.exec.mode=deny. The compaction code path is model-agnostic, so this exercises the same logic.
  • In this default config the boundary collapses to summary-only, so commit 2's anchor change (last-valid-cut vs. raw final entry) isn't differentially exercised here — it only changes the outcome when keepRecentTokens is set or via harness-direct callers. The added unit test (anchors a forced boundary on the assistant tool call, not a trailing tool result) covers that case.

@clawsweeper clawsweeper Bot mentioned this pull request Jun 17, 2026
Merged
@clawsweeper

clawsweeper Bot commented Jun 18, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Codex review: found issues before merge. Reviewed June 26, 2026, 2:11 PM ET / 18:11 UTC.

Summary
The PR changes manual /compact and harness compaction to force prepareCompaction past zero-message kept-tail cuts, adds a valid-cut anchor for trailing tool-result cases, and expands regression coverage.

PR surface: Source +48, Tests +196. Total +244 across 6 files.

Reproducibility: yes. with medium confidence: the PR tests construct a high-usage tiny transcript where the planner selects no messages, and the discussion includes after-fix live output from manual /compact; I did not run tests in this read-only review.

Review metrics: 1 noteworthy metric.

  • Public Compaction API: 1 option/signature added. The third prepareCompaction argument is reachable from the published plugin SDK agent-core subpath, so maintainers need to notice the API expansion before merge.

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

Merge readiness
Overall: 🦐 gold shrimp
Proof: 🦀 challenger crab
Patch quality: 🦐 gold shrimp
Result: needs maintainer review before merge.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

  • Keep the force option out of the public plugin SDK agent-core contract, or get explicit maintainer approval for that API change.
  • After the surface decision, rerun the focused compaction tests and the plugin SDK surface/baseline guard.

Risk before merge

  • [P1] Merging as-is adds a manual-only force option to the public openclaw/plugin-sdk/agent-core compaction surface; plugin callers could depend on that behavior before maintainers decide whether it is an SDK contract.

Maintainer options:

  1. Keep Manual Force Internal (recommended)
    Move the forced path behind an internal manual compaction wrapper or helper so the public agent-core SDK signature does not grow a manual-only option.
  2. Approve The SDK Expansion
    If maintainers want plugins to call forced compaction directly, document the new option as an intentional SDK contract and update the plugin SDK surface/baseline checks.
Copy recommended automerge instruction 
@clawsweeper automerge

Special instructions:
Keep the forced manual compaction behavior but do not expose `force` or `CompactionPreparationOptions` through the public `openclaw/plugin-sdk/agent-core` / package agent-core barrel; add focused tests for manual force and automatic skip.

Next step before merge

  • [P2] The remaining blocker is a maintainer SDK contract decision rather than a safe automated code repair.

Security
Cleared: The diff changes TypeScript compaction logic, tests, and type exports; I found no concrete secret, dependency, workflow, package-resolution, or supply-chain regression.

Review findings

  • [P1] Keep force out of the public compaction contract — packages/agent-core/src/harness/compaction/compaction.ts:638
Review details

Best possible solution:

Keep the forced manual checkpoint behavior, but route the manual-only force through an internal helper or explicitly approve and document it as public SDK API before merge.

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

Yes, with medium confidence: the PR tests construct a high-usage tiny transcript where the planner selects no messages, and the discussion includes after-fix live output from manual /compact; I did not run tests in this read-only review.

Is this the best way to solve the issue?

No as submitted: the manual compaction behavior is targeted, but exposing the control as a public SDK option still needs maintainer approval or internalization.

Full review comments:

  • [P1] Keep force out of the public compaction contract — packages/agent-core/src/harness/compaction/compaction.ts:638
    Adding the optional options parameter makes the manual-only force knob reachable through openclaw/plugin-sdk/agent-core, because that public SDK facade re-exports the agent-core package barrel. Keep the forced manual path behind an internal helper, or get explicit SDK approval plus docs/surface checks before merging.
    Confidence: 0.93

Overall correctness: patch is incorrect
Overall confidence: 0.9

AGENTS.md: found and applied where relevant.

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

Label changes

Label justifications:

  • P2: This is a focused manual compaction bug fix with limited blast radius, pending a public SDK compatibility decision.
  • merge-risk: 🚨 compatibility: The PR exposes a new compaction option through the public plugin SDK agent-core path unless the surface change is removed or intentionally accepted.
  • rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🦀 challenger crab and patch quality is 🦐 gold shrimp.
  • status: ⏳ waiting on author: ClawSweeper has contributor-facing work open and is waiting for author action. Sufficient (live_output): The PR discussion includes after-fix live output from an isolated-state real-provider manual /compact run that produced a compaction entry for the reported short-transcript high-usage case.
  • proof: sufficient: Contributor real behavior proof is sufficient. The PR discussion includes after-fix live output from an isolated-state real-provider manual /compact run that produced a compaction entry for the reported short-transcript high-usage case.
Evidence reviewed

PR surface:

Source +48, Tests +196. Total +244 across 6 files.

View PR surface stats
Area Files Added Removed Net
Source 5 51 3 +48
Tests 1 197 1 +196
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 6 248 4 +244

What I checked:

  • Root policy read and applied: Root review guidance treats plugin SDK/API surfaces as compatibility-sensitive and requires whole-path PR review rather than diff-only review. (AGENTS.md:22, 1cd6f81a46ae)
  • Scoped SDK policy read and applied: The scoped SDK guide says src/plugin-sdk is the public contract between plugins and core, and cautions against exposing internal convenience from agent internals unless intentionally promoting a public contract. (src/plugin-sdk/AGENTS.md:1, 1cd6f81a46ae)
  • Current main does not already contain the PR behavior: Current main has prepareCompaction(pathEntries, settings) without the PR's third options argument, so the central behavior is not already implemented on main. (packages/agent-core/src/harness/compaction/compaction.ts:634, 1cd6f81a46ae)
  • Manual compaction currently reaches the shared planner: The manual/auto session compaction path calls the shared prepareCompaction wrapper before deciding whether to throw Nothing to compact (session too small) or skip. (src/agents/sessions/agent-session.ts:1923, 1cd6f81a46ae)
  • PR adds a public planner option: The PR diff adds CompactionPreparationOptions with force?: boolean and a third options parameter to exported prepareCompaction. (packages/agent-core/src/harness/compaction/compaction.ts:638, fd09d2e7d0cb)
  • Public SDK facade re-exports agent-core: openclaw/plugin-sdk/agent-core wildcard re-exports the package agent-core barrel, so new agent-core barrel exports become plugin-facing. (src/plugin-sdk/agent-core.ts:24, 1cd6f81a46ae)

Likely related people:

  • amknight: Live GitHub metadata shows several adjacent compaction/session runtime PRs, including merged compaction summarization tracing and prior compaction token-pressure fixes. (role: adjacent compaction contributor; confidence: high; commits: 74f5c6ced6c2, a501917dc07d, c167e5ad7d5e; files: src/agents/embedded-agent-runner/compact.ts, src/agents/compaction-real-conversation.ts, src/auto-reply/reply/agent-runner-memory.ts)
  • jalehman: Merged related PR 93695 changed the manual sessions.compact transcript lifecycle and is cross-referenced from this PR timeline. (role: adjacent manual compaction lifecycle contributor; confidence: medium; commits: 1877d0f48e48, 425212a1ec68, e442b575b986; files: src/config/sessions/session-accessor.ts, src/gateway/server-methods/sessions.ts)
  • Ayaan Zaidi: Current-main blame for the central compaction planner, session compaction caller, and SDK facade resolves to the same snapshot commit in this checkout; useful as routing context, not original feature blame. (role: recent current-main snapshot contributor; confidence: low; commits: 8bc069f76f62; files: packages/agent-core/src/harness/compaction/compaction.ts, src/agents/sessions/agent-session.ts, src/plugin-sdk/agent-core.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 proof: sufficient ClawSweeper judged the real behavior proof convincing. 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. P2 Normal backlog priority with limited blast radius. merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. labels Jun 18, 2026
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 merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. P2 Normal backlog priority with limited blast radius. proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. size: M status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@amknight