fix: recognize custom compaction conversation#78390
Conversation
|
Codex review: needs maintainer review before merge. Summary 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 Next step before merge Security Review detailsBest 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:
Remaining risk / open question:
Codex review notes: model gpt-5.5, reasoning high; reviewed against 1fe15f230617. |
M-Lietz
left a comment
There was a problem hiding this comment.
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.
…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.
* fix: recognize custom compaction conversation * fix: use branch fallback for compaction safeguard --------- Co-authored-by: Alex Knight <[email protected]>
* fix: recognize custom compaction conversation * fix: use branch fallback for compaction safeguard --------- Co-authored-by: Alex Knight <[email protected]>
* fix: recognize custom compaction conversation * fix: use branch fallback for compaction safeguard --------- Co-authored-by: Alex Knight <[email protected]>
* fix: recognize custom compaction conversation * fix: use branch fallback for compaction safeguard --------- Co-authored-by: Alex Knight <[email protected]>
* fix: recognize custom compaction conversation * fix: use branch fallback for compaction safeguard --------- Co-authored-by: Alex Knight <[email protected]>
* fix: recognize custom compaction conversation * fix: use branch fallback for compaction safeguard --------- Co-authored-by: Alex Knight <[email protected]>
Summary
No prior historyboundary for custom-message/split-turn sessions that had real custom prompt/tool work.custom,bashExecution, andbranchSummarymessages as real conversation anchors, including nearby tool results.Verification
No prior historyfallback.pnpm test src/agents/pi-hooks/compaction-safeguard.test.ts src/agents/pi-embedded-runner/compact.hooks.test.ts(128 tests passed).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.tbx_01kqy8jxhem91xbrtva6bgbsm8failed with empty fallback when classifier behavior was restored toHEAD.tbx_01kqybhp2c6517haqm3rgwkaqyrannode scripts/run-node.mjs agent --local --agent main --session-id issue78300-cli --model fake/fake-model --message ... --json --timeout 60against a local fake OpenAI-compatible provider. Assertions passed:providerCalls=1,compactionCount=2,hasEmptyFallback=false,summaryHasCliOk=true.tbx_01kqybn9ymet502k8wjw9smkqpranpnpm check:changedwith exit 0.Security Impact
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.