Skip to content

fix(compaction): preserve assistant boundary replies#90641

Closed
wyf027 wants to merge 4 commits into
openclaw:mainfrom
wyf027:fix/compaction-preserve-assistant-boundary-76729
Closed

fix(compaction): preserve assistant boundary replies#90641
wyf027 wants to merge 4 commits into
openclaw:mainfrom
wyf027:fix/compaction-preserve-assistant-boundary-76729

Conversation

@wyf027

@wyf027 wyf027 commented Jun 5, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Summary

  • preserve the summarized assistant reply immediately before a surviving pre-compaction user message when writing successor transcripts
  • move the successor compaction replay boundary to that preserved assistant so agent context stays summary -> assistant -> user instead of hiding the reply
  • strip stale thinking signatures from preserved boundary assistants along with other pre-compaction kept messages

Fixes #76729

Tests

  • node scripts/run-vitest.mjs src/agents/embedded-agent-runner/compaction-successor-transcript.test.ts
  • git diff --check

Real behavior proof

  • Behavior addressed: successor transcripts now keep the assistant reply immediately before the first surviving pre-compaction user message so replay preserves the visible summary -> assistant -> user boundary.
  • Real environment tested: local OpenClaw checkout at PR head ba959f19332c40305f241e15ef483c4a8923cc7 on macOS 15.5 with Node v22.22.1 and pnpm 11.2.2.
  • Exact steps or command run after this patch: node scripts/run-vitest.mjs src/agents/embedded-agent-runner/compaction-successor-transcript.test.ts; git diff --check upstream/main...HEAD.
  • Evidence after fix: terminal output from the PR-head checkout:
$ node scripts/run-vitest.mjs src/agents/embedded-agent-runner/compaction-successor-transcript.test.ts
✓  agents  src/agents/embedded-agent-runner/compaction-successor-transcript.test.ts (12 tests) 132ms
Test Files  1 passed (1)
Tests  12 passed (12)
[test] passed 1 Vitest shard in 6.82s

$ git diff --check upstream/main...HEAD
# no output; exit code 0
  • Observed result after fix: the compaction successor transcript shard passed at PR head, including the preserved assistant-boundary replay coverage; the PR diff has no whitespace errors.
  • What was not tested: a live model session performing an end-to-end compaction run; this proof uses the local embedded-agent successor-transcript runtime harness.

@openclaw-barnacle openclaw-barnacle Bot added agents Agent runtime and tooling size: S triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup. labels Jun 5, 2026
@clawsweeper

clawsweeper Bot commented Jun 5, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Codex review: needs real behavior proof before merge. Reviewed June 15, 2026, 8:29 AM ET / 12:29 UTC.

Summary
The branch preserves a summarized assistant message before a retained pre-compaction user in successor transcripts, reanchors compaction replay to that assistant, strips stale thinking signatures, and adds focused tests.

PR surface: Source +69, Tests +84. Total +153 across 2 files.

Reproducibility: yes. at source level: current main removes summarized message entries before the retained boundary, and PR head still stops boundary discovery on ignored non-message entries. I did not run a live compaction session in this read-only pass.

Review metrics: none identified.

Merge readiness
Overall: 🧂 unranked krab
Proof: 🦪 silver shellfish
Patch quality: 🧂 unranked krab
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] Fix boundary discovery so ignored state/metadata entries do not block assistant preservation while context-producing entries still do.
  • [P1] Add redacted real-session compaction proof showing the reply remains visible and the next user turn is not batched with the prior one.

Proof guidance:

  • [P1] Needs real behavior proof before merge: The PR provides focused Vitest and diff-check output only, and explicitly omits a live compaction session; the contributor should add redacted terminal/log, recording, or linked-artifact proof and update the PR body for re-review.

Mantis proof suggestion
A real Control UI or webchat compaction run would materially prove the visible disappearing-reply and next-turn context behavior. A maintainer can ask Mantis to capture proof by posting a new PR comment that starts with the OpenClaw Mantis account mention, followed by:

visual task: verify a Control UI or webchat session with transcript compaction keeps the assistant reply visible after rotation and the next user turn is not batched with the prior one.

Risk before merge

  • [P1] Merging this head can still drop the assistant boundary when ignored state or metadata entries sit between the summarized assistant and retained user, preserving the disappearing-reply and combined-turn failure mode.
  • [P1] The PR changes persisted transcript membership and compaction replay anchors, so focused serialization tests are not enough to prove the visible webchat/next-turn context behavior.

Maintainer options:

  1. Fix boundary classification before merge (recommended)
    Update the scan to skip non-context entries and stop only at context-producing entries, then add regression coverage for state and metadata entries before the retained user.
  2. Require real compaction proof
    Keep the PR blocked until a redacted real Control UI, webchat, or log-based compaction run shows the assistant reply remains visible and the next turn receives the right context.
  3. Pause the draft
    If the contributor cannot provide the boundary repair and real proof, leave the draft paused rather than merging a partial session-state fix.

Next step before merge

  • [P1] Human/contributor follow-up is needed because the PR has a concrete P1 boundary defect and the external contributor proof gate cannot be satisfied by automation.

Security
Cleared: The source/test-only diff does not change dependencies, workflows, permissions, secrets, downloads, publishing, or other security-sensitive execution surfaces.

Review findings

  • [P1] Skip non-context entries before the assistant boundary — src/agents/embedded-agent-runner/compaction-successor-transcript.ts:261-262
Review details

Best possible solution:

Preserve boundary assistants across ignored state and metadata entries, stop only on context-producing entries, keep the stale-signature stripping, and provide redacted real compaction proof before merge.

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

Yes at source level: current main removes summarized message entries before the retained boundary, and PR head still stops boundary discovery on ignored non-message entries. I did not run a live compaction session in this read-only pass.

Is this the best way to solve the issue?

No, not yet. Preserving the assistant boundary is the right direction, but the implementation needs to classify intervening entries by buildSessionContext effect and still needs real after-fix compaction proof.

Full review comments:

  • [P1] Skip non-context entries before the assistant boundary — src/agents/embedded-agent-runner/compaction-successor-transcript.ts:261-262
    This breaks on every non-message entry, but custom, model_change, thinking_level_change, and session_info do not append conversational messages. If one sits between the summarized assistant and retained user, the assistant is still removed and the linked disappearing-reply bug remains; skip non-context entries and stop only at context-producing boundaries such as custom_message or non-empty branch_summary.
    Confidence: 0.96

Overall correctness: patch is incorrect
Overall confidence: 0.96

AGENTS.md: found and applied where relevant.

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

Label changes

Label changes:

  • add rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🦪 silver shellfish and patch quality is 🧂 unranked krab.
  • remove rating: 🦪 silver shellfish: Current PR rating is rating: 🧂 unranked krab, so this older rating label is no longer current.

Label justifications:

  • P1: The remaining defect affects visible assistant replies and future model context after compaction, which can break real agent/channel workflows.
  • merge-risk: 🚨 session-state: The patch changes which transcript entries survive rotation and which entry anchors replay, so an incorrect boundary can mis-shape future agent context.
  • rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🦪 silver shellfish and patch quality is 🧂 unranked krab.
  • status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: The PR provides focused Vitest and diff-check output only, and explicitly omits a live compaction session; the contributor should add redacted terminal/log, recording, or linked-artifact proof and update the PR body for re-review.
Evidence reviewed

PR surface:

Source +69, Tests +84. Total +153 across 2 files.

View PR surface stats
Area Files Added Removed Net
Source 1 73 4 +69
Tests 1 86 2 +84
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 2 159 6 +153

What I checked:

Likely related people:

  • steipete: Authored the merged OpenClaw agent runtime internalization that consolidated sessions, compaction, and transcript handling, and appears repeatedly in recent compaction successor test and docs history. (role: feature owner / recent area contributor; confidence: high; commits: bb46b79d3c14, 912f66317334, d07cce7bd166; files: src/agents/embedded-agent-runner/compaction-successor-transcript.ts, src/agents/embedded-agent-runner/compaction-successor-transcript.test.ts, packages/agent-core/src/harness/session/session.ts)
  • openperf: Authored the merged compaction successor thinking-signature fix that directly changed the same rotation module and tests shortly before this PR. (role: recent adjacent contributor; confidence: medium; commits: 6c259af759a5; files: src/agents/embedded-agent-runner/compaction-successor-transcript.ts, src/agents/embedded-agent-runner/compaction-successor-transcript.test.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.

chatgpt-codex-connector[bot]
chatgpt-codex-connector Bot reviewed Jun 5, 2026
View reviewed changes

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: ba959f1933

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/agents/embedded-agent-runner/compaction-successor-transcript.ts Outdated
Comment on lines +261 to +262
if (candidate?.type !== "message") {
continue;

@chatgpt-codex-connector chatgpt-codex-connector Bot Jun 5, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2 Badge Stop re-exposing summarized custom messages

When a surviving user message is preceded by a summarized assistant but has a custom_message or branch_summary between them, this loop skips the intervening non-message entry and still marks the assistant as the replay boundary. Since resolveSuccessorCompactionFirstKeptEntryId then moves firstKeptEntryId back to that assistant, buildSessionContext replays every appendable entry from there to the compaction (packages/agent-core/src/harness/session/session.ts:81-95), so summarized custom/branch content that previously stayed hidden behind the old first-kept user is sent to the model again after transcript rotation.

Useful? React with 👍 / 👎.

@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. labels Jun 5, 2026
@openclaw-barnacle openclaw-barnacle Bot added proof: supplied External PR includes structured after-fix real behavior proof. and removed triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup. labels Jun 5, 2026
@clawsweeper clawsweeper Bot added P1 High-priority user-facing bug, regression, or broken workflow. merge-risk: 🚨 session-state 🚨 May lose, corrupt, stale, or mis-associate session, agent, or context state. labels Jun 5, 2026
@github-actions github-actions Bot mentioned this pull request Jun 8, 2026
Open
@wyf027
wyf027 marked this pull request as draft June 12, 2026 13:18
@clawsweeper clawsweeper Bot added rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. and removed rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. labels Jun 12, 2026
@clawsweeper clawsweeper Bot mentioned this pull request Jun 14, 2026
Closed
@clawsweeper clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. and removed rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. labels Jun 15, 2026
@yetval yetval mentioned this pull request Jun 18, 2026
Merged
@wyf027

wyf027 commented Jun 18, 2026

Copy link
Copy Markdown
Contributor Author

Closing this stale draft PR because it has been open for a long time without merge. I can reopen or resubmit if needed.

@wyf027 wyf027 closed this Jun 18, 2026
@LiuwqGit LiuwqGit mentioned this pull request Jun 19, 2026
Closed
@clawsweeper clawsweeper Bot mentioned this pull request Jun 21, 2026
Merged
20 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@chatgpt-codex-connector chatgpt-codex-connector[bot] chatgpt-codex-connector[bot] left review comments

Assignees

No one assigned

Labels

agents Agent runtime and tooling merge-risk: 🚨 session-state 🚨 May lose, corrupt, stale, or mis-associate session, agent, or context state. P1 High-priority user-facing bug, regression, or broken workflow. proof: supplied External PR includes structured after-fix real behavior proof. 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.

Feishu replies disappear from webchat after compaction rotation (buildSuccessorEntries drops assistant messages)

1 participant

@wyf027