Filter internal compaction artifacts from chat history#70348
Filter internal compaction artifacts from chat history#70348cholaolu-boop wants to merge 1 commit into
Conversation
openclaw-barnacle
Bot
added
app: web-ui
Greptile SummaryThis PR filters three categories of internal compaction artifacts from Confidence Score: 5/5Safe to merge; filtering logic is correct, well-tested, and isolated to the sanitization layer. All findings are P2. The only observation is a redundant second call to No files require special attention. Prompt To Fix All With AIThis is a comment left during a code review.
Path: src/gateway/server-methods/chat.ts
Line: 1037-1040
Comment:
**Redundant post-sanitization drop check**
`sanitizeChatHistoryMessage` only truncates/strips envelope content — it never adds an `__openclaw.kind` field or rewrites text to begin with a compaction prefix, so any message already passing the pre-sanitization guard at line 1027 will never flip to matching `shouldDropInternalHistoryMessage` after processing. This second invocation is dead code in practice and could mislead future readers into thinking sanitization can manufacture internal markers.
How can I resolve this? If you propose a fix, please make it concise.Reviews (1): Last reviewed commit: "Filter internal compaction artifacts fro..." | Re-trigger Greptile |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c520a62240
ℹ️ 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".
| text.startsWith("Pre-compaction memory flush.") || | ||
| text.startsWith("[Post-compaction context refresh]") |
There was a problem hiding this comment.
The new prefix-based filter drops any history message whose text starts with Pre-compaction memory flush. or [Post-compaction context refresh] regardless of role or provenance, so legitimate user/assistant content that begins with those phrases is now silently removed from chat.history (and from session-history snapshots that reuse this sanitizer). This introduces false-positive data loss; the filter should be constrained to synthetic/internal entries (for example via metadata and/or stricter control-token checks) instead of raw text prefix alone.
Useful? React with 👍 / 👎.
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| if (shouldDropInternalHistoryMessage(res.message)) { | ||
| changed = true; | ||
| continue; | ||
| } |
There was a problem hiding this comment.
sanitizeChatHistoryMessage only truncates/strips envelope content — it never adds an __openclaw.kind field or rewrites text to begin with a compaction prefix, so any message already passing the pre-sanitization guard at line 1027 will never flip to matching shouldDropInternalHistoryMessage after processing. This second invocation is dead code in practice and could mislead future readers into thinking sanitization can manufacture internal markers.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/gateway/server-methods/chat.ts
Line: 1037-1040
Comment:
**Redundant post-sanitization drop check**
`sanitizeChatHistoryMessage` only truncates/strips envelope content — it never adds an `__openclaw.kind` field or rewrites text to begin with a compaction prefix, so any message already passing the pre-sanitization guard at line 1027 will never flip to matching `shouldDropInternalHistoryMessage` after processing. This second invocation is dead code in practice and could mislead future readers into thinking sanitization can manufacture internal markers.
How can I resolve this? If you propose a fix, please make it concise.
|
Thanks for the context here. I swept through the related work, and this is now duplicate or superseded. Close: the useful parts of this PR are now split across current-main behavior and the canonical remaining issue. Current main keeps the memory-flush prompt out of the transcript, intentionally renders compaction records as checkpoint dividers, and routes the broader internal-prompt leak to #64613; this branch is conflicting, not maintainer-modifiable, proof-missing, and would regress current divider behavior. Canonical path: Close this branch and keep the current-main compaction divider and memory-flush fixes; any remaining internal prompt filtering should proceed through #64613 with provenance-backed Gateway projection behavior. So I’m closing this here and keeping the remaining discussion on #64613. Review detailsBest possible solution: Close this branch and keep the current-main compaction divider and memory-flush fixes; any remaining internal prompt filtering should proceed through #64613 with provenance-backed Gateway projection behavior. Do we have a high-confidence way to reproduce the issue? Yes, at source level for the merge blockers: current main intentionally preserves compaction markers for WebChat checkpoint dividers, while this PR drops them, and the prefix filter is visibly not provenance-scoped. No after-fix real WebChat proof was supplied for the contributor branch. Is this the best way to solve the issue? No. The best fix is not this stale server-methods prefix filter; the maintainable path is current-main shared display projection behavior that preserves compaction dividers and uses provenance or another structural marker for remaining internal prompt rows. Security review: Security review cleared: The diff changes Gateway TypeScript and tests only; no dependency, workflow, permission, secret-handling, or supply-chain concern was found. AGENTS.md: found and applied where relevant. What I checked:
Likely related people:
Codex review notes: model internal, reasoning high; reviewed against c1219d161d3e. |
clawsweeper
Bot
added
rating: 🧂 unranked krab
|
ClawSweeper PR egg 🎁 Pass real behavior proof to wake the egg and unlock a hatchable treat. Where did the egg go?
|
openclaw-barnacle
Bot
added
the
triage: needs-real-behavior-proof
|
This pull request has been automatically marked as stale due to inactivity. |
|
ClawSweeper applied the proposed close for this PR.
|
1 participant
Summary
This patch addresses the dominant
chat.historyvisibility path where internal compaction and memory-flush artifacts leak back into normal chat history.What This Fix Does
chat.historyPre-compaction memory flush.prompts fromchat.history[Post-compaction context refresh]artifacts fromchat.historyExplicitly Not Part Of This Fix
audio payload removed during overflow recoveryis not part of this patchWhy This Scope
The patch is intentionally limited to the dominant
chat.history/ compaction / flush visibility path. It does not touch Route A/B, step 1b, step 2, approval, heartbeat, or think-level behavior.Optional Follow-up