fix(memory): detect session drift in status mode to avoid false-clean dirty flag#98072
fix(memory): detect session drift in status mode to avoid false-clean dirty flag#98072ly-wang19 wants to merge 1 commit into
Conversation
… dirty flag Plain `openclaw memory status` skipped `ensureSessionStartupCatchup()` because status mode is transient, so unindexed on-disk session transcripts never set `sessionsDirty`. Operators saw `dirty=false` while memory search silently missed recent transcripts. Status mode now runs the detection half (no sync) in the constructor and exposes `refreshSessionStartupDirtyDetection()` so the CLI status path can await it before reading `status()`. Fixes openclaw#97814
openclaw-barnacle
Bot
added
extensions: memory-core
|
Thanks for the context here. I swept through the related work, and this is now duplicate or superseded. Close as superseded: this PR targets the same memory-status false-clean bug as #97857, and that open clean branch is the stronger landing path because it awaits the existing session drift helper during status-purpose manager creation before any status caller reads the dirty flag. Root-cause cluster Members:
Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything. Canonical path: Use #97857 as the canonical landing path for the memory-status false-clean fix, then close sibling candidate branches once that path is resolved. So I’m closing this here and keeping the remaining discussion on #97857. Review detailsBest possible solution: Use #97857 as the canonical landing path for the memory-status false-clean fix, then close sibling candidate branches once that path is resolved. Do we have a high-confidence way to reproduce the issue? Yes, at source level. Current main constructs purpose- Is this the best way to solve the issue? No for this branch as the landing path. The safer solution is the open canonical PR because it awaits the existing dirty-file scan during status-purpose manager creation, so all status callers observe the refreshed dirty flag. Security review: Security review cleared: The diff only touches memory-core runtime and test code, with no dependency, CI, package metadata, secret, permission, or supply-chain surface changes. AGENTS.md: found and applied where relevant. What I checked:
Likely related people:
Codex review notes: model internal, reasoning high; reviewed against 56c2d637d940. |
clawsweeper
Bot
added
rating: 🦪 silver shellfish
|
ClawSweeper applied the proposed close for this PR.
|
1 participant
What Problem This Solves
Fixes #97814.
openclaw memory status(plain, no--index) can reportdirty=falsewhile a usage-counted session transcript exists on disk with zero rows inmemory_index_sources/memory_index_chunks. The operator sees no backlog but memory search silently misses the unindexed transcript. During a recent incident this false-clean state compounded triage confusion because the fleet appeared healthier than it was.Root cause: status-purpose manager init skips
ensureSessionStartupCatchup(), so on-disk session drift is never surfaced through the dirty flag in the plain status path.Why This Change Was Made
Status mode intentionally avoids the full catch-up sync to stay light, but the previous implementation also skipped drift detection, which made the dirty flag unreliable. This PR threads detection (without sync) through the status path so
status()reflects reality, then waits for that detection to settle before the CLI reads status.Design choice: detection-only, no sync in the status path. The status-purpose manager now records a promise that resolves with the list of unindexed session files (using the existing
markSessionStartupCatchupDirtyFiles()helper).status()already honors the dirty flag set by detection. The CLI status path calls a newrefreshSessionStartupDirtyDetection()method to await detection before reading status, so the response is accurate even when the manager was constructed milliseconds ago.User Impact
openclaw memory statusno longer claimsdirty=falsewhen unindexed session transcripts exist; operators can trust the dirty flag as a health signal.--index/--force/--deeppaths — they already triggered catch-up.Evidence
extensions/memory-core/src/memory/index.test.ts— "reports dirty in status mode when an unindexed session transcript exists ([Bug]: memory status reports dirty=false for unindexed usage-counted session transcripts (false-clean status path) #97814)". Creates a session transcript on disk, constructs a status-purpose manager, awaitsrefreshSessionStartupDirtyDetection(), and assertsstatus().dirty === true.tsgoclean against touched files.node scripts/run-vitest.mjs extensions/memory-core/src/memory/index.test.tsexits 0.The new method is thread-local to the builtin
MemoryIndexManager; the CLI uses type narrowing with a runtimetypeof === "function"guard so other backend implementations are unaffected.