(fix): Compaction: preserve recent context and sync session memory post-compact#20038
Closed
rodrigouroz wants to merge 2 commits intoopenclaw:mainfrom
Closed
(fix): Compaction: preserve recent context and sync session memory post-compact#20038rodrigouroz wants to merge 2 commits intoopenclaw:mainfrom
rodrigouroz wants to merge 2 commits intoopenclaw:mainfrom
Conversation
18 tasks
Comment on lines
+298
to
+302
| function extractOpaqueIdentifiers(text: string): string[] { | ||
| const matches = | ||
| text.match( | ||
| /([a-f0-9]{8,}|https?:\/\/\S+|\/[\w./-]+|[A-Za-z]:\\[\w\\.-]+|[A-Za-z0-9._-]+\.[A-Za-z0-9._/-]+:\d{1,5}|\b\d{6,}\b)/g, | ||
| ) ?? []; |
Contributor
There was a problem hiding this comment.
Hex pattern only matches lowercase
The regex [a-f0-9]{8,} only matches lowercase hex characters. Identifiers containing uppercase hex digits (e.g., A1B2C3D4E5F6 or mixed-case UUIDs like A1b2C3d4E5f6G7h8) will not be extracted by this pattern. If the codebase deals with identifiers in any casing, consider using [a-fA-F0-9]{8,} or adding the case-insensitive flag to this alternative.
This is mitigated by the fact that the LLM is separately instructed to preserve identifiers verbatim, so this regex serves as a quality audit rather than the sole mechanism. Still worth noting since it could cause false-negative audit failures (missing identifiers → unnecessary retry).
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/agents/pi-extensions/compaction-safeguard.ts
Line: 298-302
Comment:
**Hex pattern only matches lowercase**
The regex `[a-f0-9]{8,}` only matches lowercase hex characters. Identifiers containing uppercase hex digits (e.g., `A1B2C3D4E5F6` or mixed-case UUIDs like `A1b2C3d4E5f6G7h8`) will not be extracted by this pattern. If the codebase deals with identifiers in any casing, consider using `[a-fA-F0-9]{8,}` or adding the case-insensitive flag to this alternative.
This is mitigated by the fact that the LLM is separately instructed to preserve identifiers verbatim, so this regex serves as a quality audit rather than the sole mechanism. Still worth noting since it could cause false-negative audit failures (missing identifiers → unnecessary retry).
How can I resolve this? If you propose a fix, please make it concise.8f870ee to
9a0520a
Compare
77e9ad6 to
f972d80
Compare
Contributor
Author
|
@greptileai can you update your review on this? |
This was referenced Feb 24, 2026
Contributor
Author
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Change Type (select all)
Scope (select all touched areas)
Linked Issue/PR
User-visible / Behavior Changes
agents.defaults.compaction.recentTurnsPreserveagents.defaults.compaction.postIndexSync(off|async|await)agents.defaults.compaction.qualityGuard.enabledagents.defaults.compaction.qualityGuard.maxRetriesagents.defaults.memorySearch.sync.sessions.postCompactionForceSecurity Impact (required)
No)No)No)No)No)Yes, explain risk + mitigation:Repro + Verification
Environment
Steps
pnpm tsgo.pnpm vitest run src/agents/compaction.retry.test.ts src/agents/compaction.identifier-preservation.test.ts src/agents/pi-embedded-runner/run.overflow-compaction.test.ts src/auto-reply/reply/post-compaction-context.test.ts src/auto-reply/reply/post-compaction-audit.test.ts src/memory/manager.async-search.test.ts src/memory/manager.watcher-config.test.ts src/memory/index.test.ts src/memory/manager.atomic-reindex.test.tspnpm vitest run src/auto-reply/reply/followup-runner.test.ts src/auto-reply/reply/agent-runner.runreplyagent.test.ts src/auto-reply/reply/commands.test.ts src/auto-reply/reply/agent-runner.misc.runreplyagent.test.tsExpected
Actual
pnpm tsgopassed.Evidence
Attach at least one:
Human Verification (required)
What you personally verified (not just CI), and how:
off) and supports async/await modes.pnpm testmatrix.Compatibility / Migration
Yes)No)No)Failure Recovery (if this breaks)
agents.defaults.compaction.postIndexSync: "off"to disable post-compaction index sync.agents.defaults.compaction.qualityGuard.enabled: falseto disable quality-guard retries.agents.defaults.compaction.recentTurnsPreserve: 0to disable preserved-tail behavior.openclaw.json.awaitmode due to blocking index sync.Risks and Mitigations
awaitmode may increase latency for replies right after compaction.async;offandasyncremain available.Greptile Summary
This PR enhances compaction reliability by preserving recent context and syncing session memory post-compaction. The implementation adds structured summary quality checks with retry logic, verbatim preservation of recent turns, and configurable memory index syncing.
Key changes:
off,async(default), andawaitThe code follows project conventions, includes proper error handling with best-effort fallbacks, and provides escape hatches via configuration for all new behaviors.
Confidence Score: 4/5
Last reviewed commit: f972d80