Skip to content

fix: recognize custom compaction conversation#78390

Merged
amknight merged 2 commits into
mainfrom
codex/fix-compaction-safeguard-custom-turns
May 6, 2026
Merged

fix: recognize custom compaction conversation#78390
amknight merged 2 commits into
mainfrom
codex/fix-compaction-safeguard-custom-turns

Conversation

@amknight

@amknight amknight commented May 6, 2026

Copy link
Copy Markdown
Member

Summary

  • Fixes [Bug]: Compaction safeguard mode produces empty fallback summaries that loop until 60s aggregate timeout #78300: safeguard-mode compaction could write the empty No prior history boundary for custom-message/split-turn sessions that had real custom prompt/tool work.
  • Treat visible custom, bashExecution, and branchSummary messages as real conversation anchors, including nearby tool results.
  • Adds a production-shape fallback for cases where Pi's compaction preparation omits the real custom-message content: the safeguard inspects the live session branch before deciding to write an empty boundary.
  • Adds a changelog entry credited to @amknight.

Verification

  • Local pre-fix targeted repro: failed with empty No prior history fallback.
  • Local targeted tests: pnpm test src/agents/pi-hooks/compaction-safeguard.test.ts src/agents/pi-embedded-runner/compact.hooks.test.ts (128 tests passed).
  • Local formatting/checks: git diff --check; pnpm exec oxfmt --check --threads=1 src/agents/compaction-real-conversation.ts src/agents/pi-hooks/compaction-safeguard.ts src/agents/pi-hooks/compaction-safeguard.test.ts src/agents/pi-embedded-runner/compact.hooks.test.ts.
  • Crabbox pre-fix repro: tbx_01kqy8jxhem91xbrtva6bgbsm8 failed with empty fallback when classifier behavior was restored to HEAD.
  • Crabbox CLI proof: tbx_01kqybhp2c6517haqm3rgwkaqy ran node scripts/run-node.mjs agent --local --agent main --session-id issue78300-cli --model fake/fake-model --message ... --json --timeout 60 against a local fake OpenAI-compatible provider. Assertions passed: providerCalls=1, compactionCount=2, hasEmptyFallback=false, summaryHasCliOk=true.
  • Crabbox final changed gate on latest pushed branch: tbx_01kqybn9ymet502k8wjw9smkqp ran pnpm check:changed with exit 0.

Security Impact

  • New permissions/capabilities: No
  • Secrets/tokens handling changed: No
  • New/changed network calls: No
  • Command/tool execution surface changed: No

Notes

The CLI proof deliberately uses a tiny fake model context to force OpenClaw's real CLI compaction path. The final agent payload still reports context overflow after the successful compaction because the synthetic transcript remains too large for the fake model; the regression assertion is that compaction no longer writes the empty fallback summary.

@openclaw-barnacle openclaw-barnacle Bot added agents Agent runtime and tooling size: S maintainer Maintainer-authored PR labels May 6, 2026
@amknight
amknight marked this pull request as ready for review May 6, 2026 09:22
@clawsweeper

clawsweeper Bot commented May 6, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs maintainer review before merge.

Summary
The PR updates compaction’s real-conversation classifier and safeguard fallback to treat visible custom, bashExecution, and branchSummary entries plus anchored tool results as substantive, with regression tests and a changelog entry.

Reproducibility: yes. source-reproducible: current main ignores visible custom turn-prefix/session-branch messages as real conversation anchors, so the safeguard can reach the empty fallback branch despite substantive custom prompt and tool work. The linked issue and PR body add log symptoms plus pre-fix and after-fix Crabbox proof for the same path.

Real behavior proof
Not applicable: This is a maintainer-authored PR, so the external contributor proof gate does not apply; the body still includes local and Crabbox before/after CLI proof.

Next step before merge
No ClawSweeper repair lane is appropriate because this protected maintainer PR has no discrete automated repair target; the remaining action is normal maintainer review and merge gating.

Security
Cleared: No concrete security or supply-chain concern found; the diff changes classifier/fallback logic, tests, and changelog text only.

Review details

Best possible solution:

Land the focused classifier and branch-fallback fix with its regression coverage after normal maintainer review and required checks finish.

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

Yes, source-reproducible: current main ignores visible custom turn-prefix/session-branch messages as real conversation anchors, so the safeguard can reach the empty fallback branch despite substantive custom prompt and tool work. The linked issue and PR body add log symptoms plus pre-fix and after-fix Crabbox proof for the same path.

Is this the best way to solve the issue?

Yes. Updating the shared classifier is the narrowest maintainable fix, and the branch fallback is scoped to the case where Pi’s compaction preparation contains no real conversation even though the live session branch does.

What I checked:

Likely related people:

  • steipete: GitHub path history shows recent compaction safeguard and embedded-runner maintenance, including runtime-context and manual compaction fixes adjacent to this PR’s code path. (role: recent maintainer; confidence: high; commits: 538605ff44d2, 11e6928b3edc, 7920f8d4fdeb; files: src/agents/pi-hooks/compaction-safeguard.ts, src/agents/pi-embedded-runner/compact.ts)
  • samzong: Authored the content-aware compaction guard commit that created the real-conversation classifier behavior this PR extends. (role: introduced adjacent behavior; confidence: medium; commits: 5c05347d119e; files: src/agents/compaction-real-conversation.ts, src/agents/pi-hooks/compaction-safeguard.ts)
  • jalehman: The content-aware compaction guard commit records jalehman as co-author/reviewer, making them a useful routing candidate for this classifier boundary. (role: reviewer and adjacent owner; confidence: medium; commits: 5c05347d119e; files: src/agents/compaction-real-conversation.ts, src/agents/pi-hooks/compaction-safeguard.ts)

Remaining risk / open question:

  • This read-only review did not execute the targeted tests locally; it relies on source inspection, PR body Crabbox/local proof, and GitHub check metadata.

Codex review notes: model gpt-5.5, reasoning high; reviewed against 1fe15f230617.

@M-Lietz M-Lietz left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good fix. The compaction safeguard currently treats custom compaction conversations (like memory flush or tool-result compaction) the same as regular conversation compaction, which can trigger false positives in the quality guard.

The change to recognize custom compaction conversation markers prevents the safeguard from canceling intentional compaction operations. This aligns with the documented compaction behavior where memory flush compaction is a distinct operation from overflow recovery compaction.

@amknight
amknight merged commit 1c29156 into main May 6, 2026
121 of 123 checks passed
@amknight
amknight deleted the codex/fix-compaction-safeguard-custom-turns branch May 6, 2026 10:13
martingarramon added a commit to martingarramon/openclaw that referenced this pull request May 6, 2026
…ion-real-conversation

Adds a colocated test file for src/agents/compaction-real-conversation.ts,
covering the bashExecution and branchSummary role branches added in
openclaw#78390. Currently neither branch has direct unit coverage —
bashExecution/excludeFromContext is referenced nowhere in src/agents/
test files, and branchSummary is only exercised transitively through
compaction-safeguard.test.ts.

Coverage:
- hasMeaningfulConversationContent: bashExecution happy path,
  excludeFromContext === true short-circuit, empty/missing
  command+output, branchSummary string/empty/whitespace/non-string/missing.
- isRealConversationMessage: toolResult lookback resolves through
  bashExecution + branchSummary anchors, returns false when no anchor
  exists, returns false when the only anchor sits past the 20-message
  lookback window.

Refs openclaw#78390.
github-actions Bot pushed a commit to Desicool/openclaw that referenced this pull request May 9, 2026
* fix: recognize custom compaction conversation

* fix: use branch fallback for compaction safeguard

---------

Co-authored-by: Alex Knight <[email protected]>
rogerdigital pushed a commit to rogerdigital/openclaw that referenced this pull request May 9, 2026
* fix: recognize custom compaction conversation

* fix: use branch fallback for compaction safeguard

---------

Co-authored-by: Alex Knight <[email protected]>
lykeion-dev pushed a commit to lykeion-dev/openclaw--rev that referenced this pull request May 14, 2026
* fix: recognize custom compaction conversation

* fix: use branch fallback for compaction safeguard

---------

Co-authored-by: Alex Knight <[email protected]>
github-actions Bot pushed a commit to Desicool/openclaw that referenced this pull request May 24, 2026
* fix: recognize custom compaction conversation

* fix: use branch fallback for compaction safeguard

---------

Co-authored-by: Alex Knight <[email protected]>
jameslcowan pushed a commit to jameslcowan/openclaw that referenced this pull request Jun 2, 2026
* fix: recognize custom compaction conversation

* fix: use branch fallback for compaction safeguard

---------

Co-authored-by: Alex Knight <[email protected]>
sablehead pushed a commit to sablehead/openclaw that referenced this pull request Jun 10, 2026
* fix: recognize custom compaction conversation

* fix: use branch fallback for compaction safeguard

---------

Co-authored-by: Alex Knight <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

agents Agent runtime and tooling maintainer Maintainer-authored PR size: M

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Compaction safeguard mode produces empty fallback summaries that loop until 60s aggregate timeout

2 participants