Skip to content

fix(memory): refresh status dirty sessions#97836

Closed
lin-hongkuan wants to merge 2 commits into
openclaw:mainfrom
lin-hongkuan:lhk/fix-memory-status-dirty-97814
Closed

fix(memory): refresh status dirty sessions#97836
lin-hongkuan wants to merge 2 commits into
openclaw:mainfrom
lin-hongkuan:lhk/fix-memory-status-dirty-97814

Conversation

@lin-hongkuan

@lin-hongkuan lin-hongkuan commented Jun 29, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Summary

Fixes memory status reporting stale dirty=false for session transcripts that have been usage-counted but not yet indexed.

Issue

Fixes #97814.

Changes

  • Adds a status-only session startup catch-up dirty check.
  • Reuses the existing session startup catch-up dirty marking path without scheduling a full sync for status reads.
  • Covers the status-only path with a regression test.

Testing

  • node scripts/run-vitest.mjs extensions/memory-core/src/memory/manager-sync-ops.startup-catchup.test.ts — 16 tests passed
  • /root/.hermes/oss-contrib/worktrees/openclaw__openclaw/node_modules/.bin/oxfmt --check --threads=1 extensions/memory-core/src/memory/manager.ts extensions/memory-core/src/memory/manager-sync-ops.ts extensions/memory-core/src/memory/manager-sync-ops.startup-catchup.test.ts — passed
  • git diff --check && git diff --cached --check — passed
  • added-line secret scan — clean

What Problem This Solves

Fixes #97814. Memory status could report dirty=false for session transcripts that had already been usage-counted but had not yet been indexed. That made the status view look clean while a startup catch-up dirty mark was still required.

Evidence

  • node scripts/run-vitest.mjs extensions/memory-core/src/memory/manager-sync-ops.startup-catchup.test.ts — 16 tests passed
  • oxfmt --check --threads=1 extensions/memory-core/src/memory/manager.ts extensions/memory-core/src/memory/manager-sync-ops.ts extensions/memory-core/src/memory/manager-sync-ops.startup-catchup.test.ts — passed
  • git diff --check && git diff --cached --check — passed
  • Added-line secret scan — clean
  • Regression test covers the status-only startup catch-up path without scheduling a full sync.

@openclaw-barnacle openclaw-barnacle Bot added extensions: memory-core Extension: memory-core size: XS triage: needs-pr-context Candidate: external PR body lacks required problem context or evidence. labels Jun 29, 2026
@clawsweeper

clawsweeper Bot commented Jun 29, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Thanks for the context here. I swept through the related work, and this is now duplicate or superseded.

Close as superseded: #97857 is the stronger open landing path for the same memory status false-clean bug, while this branch still starts the dirty-session scan fire-and-forget and can race the synchronous status read.

Root-cause cluster
Relationship: superseded
Canonical: #97857
Summary: The open sibling PR is the stronger canonical landing candidate for the same memory status false-clean issue.

Members:

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Canonical path: GitHub search found three open PRs targeting the same memory status dirty false-clean issue; this branch and #97842 are competing candidates, while #97857 is proof-positive.

So I’m closing this here and keeping the remaining discussion on #97857 and #97842.

Review details

Best possible solution:

Land #97857 as the canonical awaited status-mode dirty detection fix, then close or redirect the competing same-issue PRs.

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

Yes by source inspection: status-purpose managers are transient and plain status immediately reads synchronous manager.status(), so an async catch-up path must be awaited to affect the reported dirty bit.

Is this the best way to solve the issue?

No. This branch starts the catch-up asynchronously from the constructor, while the sibling PR uses an awaited factory approach and directly tests the actual status-purpose manager flow.

Security review:

Security review cleared: The diff only changes memory-core runtime and test files; it does not touch dependencies, CI, package metadata, secrets, permissions, downloads, or other supply-chain surfaces.

AGENTS.md: found and applied where relevant.

What I checked:

Likely related people:

  • giodl73-repo: Authored and merged the earlier startup/CLI session catch-up PR that introduced the helper this status fix reuses. (role: adjacent feature contributor; confidence: high; commits: 8102434a234a, 5e02e33cb438, 8af2af24a53c; files: extensions/memory-core/src/memory/manager.ts, extensions/memory-core/src/memory/manager-sync-ops.ts, extensions/memory-core/src/memory/manager-sync-ops.startup-catchup.test.ts)
  • Peter Steinberger: Earlier memory manager state and session sync refactors touched the same manager/sync surfaces that now carry the status dirty-state invariant. (role: adjacent refactor author; confidence: medium; commits: d2a03eca1a5e, c9f288ceafbf, f4d8393bf4c1; files: extensions/memory-core/src/memory/manager.ts, extensions/memory-core/src/memory/manager-sync-ops.ts)

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

@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. P2 Normal backlog priority with limited blast radius. labels Jun 29, 2026
@zw-xysk zw-xysk mentioned this pull request Jun 29, 2026
Merged
@lin-hongkuan

lin-hongkuan commented Jun 30, 2026

Copy link
Copy Markdown
Contributor Author

Updated PR body with explicit What Problem This Solves / Evidence sections for the real-behavior proof check.

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented Jun 30, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

@openclaw-barnacle openclaw-barnacle Bot removed the triage: needs-pr-context Candidate: external PR body lacks required problem context or evidence. label Jun 30, 2026
@clawsweeper clawsweeper Bot added the merge-risk: 🚨 session-state 🚨 May lose, corrupt, stale, or mis-associate session, agent, or context state. label Jun 30, 2026
@clawsweeper

clawsweeper Bot commented Jun 30, 2026

Copy link
Copy Markdown
Contributor

ClawSweeper applied the proposed close for this PR.

@clawsweeper clawsweeper Bot closed this Jun 30, 2026
@clawsweeper clawsweeper Bot mentioned this pull request Jul 2, 2026
Closed
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

extensions: memory-core Extension: memory-core merge-risk: 🚨 session-state 🚨 May lose, corrupt, stale, or mis-associate session, agent, or context state. P2 Normal backlog priority with limited blast radius. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. size: XS 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.

[Bug]: memory status reports dirty=false for unindexed usage-counted session transcripts (false-clean status path)

1 participant

@lin-hongkuan