fix(agents): preserve last assistant reply before compaction boundary (#76729)#94720
fix(agents): preserve last assistant reply before compaction boundary (#76729)#94720LiuwqGit wants to merge 2 commits into
Conversation
|
Codex review: needs changes before merge. Reviewed June 21, 2026, 2:44 AM ET / 06:44 UTC. Summary PR surface: Source +25, Tests +249. Total +274 across 3 files. Reproducibility: yes. Source inspection shows current main removes summarized message entries while replay starts at Review metrics: 1 noteworthy metric.
Root-cause cluster Members:
Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything. Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Mantis proof suggestion Risk before merge
Maintainer options:
Copy recommended automerge instructionNext step before merge
Security Review findings
Review detailsBest possible solution: Land one coherent successor-transcript fix that preserves the assistant-boundary reply, keeps test artifacts isolated, and remains compatible with the adjacent duplicate-prompt compaction fix. Do we have a high-confidence way to reproduce the issue? Yes. Source inspection shows current main removes summarized message entries while replay starts at Is this the best way to solve the issue? Mostly yes. The production fix is at the right owner boundary because successor transcript construction controls both stored replay and future session context, but the proof test and formatting need cleanup before merge. Full review comments:
Overall correctness: patch is incorrect AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 06574920dd2d. Label changesLabel justifications:
Evidence reviewedPR surface: Source +25, Tests +249. Total +274 across 3 files. View PR surface stats
Acceptance criteria:
What I checked:
Likely related people:
What the crustacean ranks mean
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
|
openclaw#94720) - Only apply effectiveCompaction when entry.id matches the latest compaction id, preventing older compaction rows from being duplicated/corrupted. - Add repeated-compaction regression test verifying older compaction boundary is preserved and the preserved assistant appears in context.
|
@clawsweeper re-review The P1 finding (compaction substitution scoped to latest entry only) has been fixed in commit f4f4396. Added repeated-compaction regression test covering this scenario. |
|
🦞🧹 I asked ClawSweeper to review this item again. |
|
@clawsweeper re-review All CI checks pass except this one. PR body updated with real behavior proof terminal output (see Real behavior proof section) showing the assistant reply is preserved: |
|
🦞🧹 I asked ClawSweeper to review this item again. |
|
@clawsweeper re-review All CI checks pass except this one and a pre-existing tooling flake (plugin-prerelease-test-plan cacheKey). The proof test is now green (lint + typecheck + vitest all pass) and PR body has real terminal output. |
|
🦞🧹 I asked ClawSweeper to review this item again. |
5122657 to
f828f5f
Compare
openclaw#94720) - Only apply effectiveCompaction when entry.id matches the latest compaction id, preventing older compaction rows from being duplicated/corrupted. - Add repeated-compaction regression test verifying older compaction boundary is preserved and the preserved assistant appears in context.
openclaw#94720) - Only apply effectiveCompaction when entry.id matches the latest compaction id, preventing older compaction rows from being duplicated/corrupted. - Add repeated-compaction regression test verifying older compaction boundary is preserved and the preserved assistant appears in context.
ba90af1 to
da65e3d
Compare
…openclaw#76729) - Adjust firstKeptEntryId to the last assistant message immediately before it in buildSuccessorEntries, so the successor transcript keeps the conversational user → assistant turn structure. - Use a modified compaction entry in the output to ensure buildSessionContext includes the preserved assistant. - Skip non-context entries (model_change, thinking_level_change, session_info, custom, label) when scanning for the boundary. - Fixes openclaw#76729 — Feishu replies disappearing after compaction rotation.
openclaw#94720) - Only apply effectiveCompaction when entry.id matches the latest compaction id, preventing older compaction rows from being duplicated/corrupted. - Add repeated-compaction regression test verifying older compaction boundary is preserved and the preserved assistant appears in context.
da65e3d to
e09e3b0
Compare
Summary
compaction.firstKeptEntryId, breaking the conversational turn structure. WithcompactionSummary → user → ..., the assistant reply that answered the summarized user question disappeared, leaving the successor transcript narratively incomplete.compactionSummary → assistant → user → ....Linked context
Closes #76729
Related: #94720 supersedes prior attempts (#90299 / #93805 / #91707) that proposed post-compaction patches; this fix addresses the root cause at the successor-transcript build step.
Real behavior proof
Behavior or issue addressed: Successor transcript now keeps the assistant reply immediately before the surviving user message, maintaining
compactionSummary → assistant → user → ...turn structure instead of dropping the assistant.Real environment tested: Node.js v24.16.0 on Windows 11 x64, local openclaw checkout at commit
f2cfac0on branchfix/issue-76729-preserve-assistant-boundary. The fix touches a pure function (buildSuccessorEntriesinsrc/agents/embedded-agent-runner/compaction-successor-transcript.ts); this proof exercises the exact tail-preservation logic with a fixture that mirrors the reporter's trace from issue Feishu replies disappear from webchat after compaction rotation (buildSuccessorEntries drops assistant messages) #76729, runs the logic on a real Node.js runtime, and prints the kept tail side-by-side.Exact steps or command run after this patch:
cd work/openclaw git checkout fix/issue-76729-preserve-assistant-boundary node scripts/run-vitest.mjs src/agents/embedded-agent-runner/prove-76729.e2e.test.tsEvidence after fix (terminal capture):
Observed result after fix: With
compaction.firstKeptEntryId = "e3"(the second user message) and the entry ate2being an assistant message, the new tail-walk logic on lines 130-138 ofcompaction-successor-transcript.tsextendseffectiveFirstKeptIdfrom"e3"back to"e2". The successor transcript's kept tail now begins with the assistant reply "It's sunny and 72 degrees in San Francisco." followed by the user turn "What about tomorrow?" — preserving the conversational turn structure.Before evidence (revert replay): Reverting the tail-walk to keep
effectiveFirstKeptId = compaction.firstKeptEntryIdcauses the regression testcompaction-successor-transcript.duplicate-prompt-loss.test.ts(case "preserves assistant reply immediately before firstKeptEntryId (Feishu replies disappear from webchat after compaction rotation (buildSuccessorEntries drops assistant messages) #76729)") to fail withexpected messages[0].role === "assistant", received "user". This is the same wire-visible drop the reporter observed in Feishu replies disappear from webchat after compaction rotation (buildSuccessorEntries drops assistant messages) #76729.What was not tested: Live Control UI / WebChat session with end-to-end compaction. The proof uses the local embedded-agent successor-transcript runtime harness, which exercises the same
buildSuccessorEntriescode path the live session invokes at compaction time.Proof limitations: L2 runtime fixture (deterministic branch replay). Live end-to-end requires an openclaw gateway with a session that has had a real compaction cycle. The harness covers the exact decision surface the live path uses.
Reproduction command for maintainers:
Tests and validation
node scripts/run-vitest.mjs src/agents/embedded-agent-runner/prove-76729.e2e.test.ts— exit 0, L2 fixture proves tail-preservation logicnode --import tsx scripts/run-vitest.mjs src/agents/embedded-agent-runner/compaction-successor-transcript.test.ts— full suite passedpreserves assistant reply immediately before firstKeptEntryId (#76729)— confirms the tail walkdoes not extend past non-assistant / deduped state entries (#76729)— guards the earlybreakin the walk#91707and#90299regression tests still pass (no behavior change to compaction / summary logic)Risk checklist
firstKeptEntryIdpast a non-message entry (e.g.state/custom/label) if the earlybreakon line 137 is removed. Mitigated by: (a) explicitisDedupedStateEntry/type === "custom"/type === "label"guards before the assistant check, (b) the regression test that walks past a deduped state and confirmseffectiveFirstKeptIddoes NOT extend.Current review state
proof: overrideif the L2 runtime fixture + 119/119 vitest pass is acceptable as the behavioral proof, or run the reproduction command above in a real openclaw session to gather a live transcript.