Skip to content

fix(memory): sweep orphaned reindex temp files#92972

Closed
pony-maggie wants to merge 2 commits into
openclaw:mainfrom
pony-maggie:fix-memory-reindex-temp-sweep
Closed

fix(memory): sweep orphaned reindex temp files#92972
pony-maggie wants to merge 2 commits into
openclaw:mainfrom
pony-maggie:fix-memory-reindex-temp-sweep

Conversation

@pony-maggie

@pony-maggie pony-maggie commented Jun 14, 2026 •
edited
Loading

Copy link
Copy Markdown

What changed

  • Added a memory-core startup sweep for aged sibling reindex temp files matching <index>.sqlite.tmp-*, including -wal and -shm sidecars.
  • Runs the sweep before opening the live memory index manager so hard-restart leftovers are reclaimed on the next startup.
  • Keeps young temp files untouched with a grace window to avoid racing active reindexes.
  • Added regression coverage for the sweep helper and manager startup path.

Why

A hard restart or SIGKILL during atomic memory reindex can bypass the in-process cleanup path and leave full-size SQLite temp triplets behind. Those orphaned files are not authoritative once the live index exists, but previously nothing reclaimed them on startup.

Fixes #92874

Real behavior proof

Behavior addressed: aged orphaned memory reindex temp SQLite triplets are removed on the startup cleanup path while the live memory index remains in place.

Real environment tested: local OpenClaw source checkout on macOS, using the PR branch code and a real temporary filesystem directory under /var/folders/.../T/.

Exact steps or command run after this patch: ran a one-off npm exec -- pnpm exec tsx --eval script that imported sweepMemoryIndexTempFiles from extensions/memory-core/src/memory/manager-atomic-reindex.ts, created memory.sqlite plus an aged memory.sqlite.tmp-proof/-wal/-shm triplet, then invoked the production sweep helper with a 30-second grace window.

Evidence after fix: terminal output from the real temporary directory run:

real setup: /var/folders/sh/pd5lfdqs3h7csf9l6f790sb40000gn/T/openclaw-memory-sweep-proof-lxStI8
before: memory.sqlite, memory.sqlite.tmp-proof, memory.sqlite.tmp-proof-shm, memory.sqlite.tmp-proof-wal
removed: <proof-dir>/memory.sqlite.tmp-proof
after: memory.sqlite

Observed result after fix: the aged temp base and both SQLite sidecars were removed, and the live memory.sqlite file remained as the only file in the directory.

What was not tested: a destructive live SIGKILL against a running user agent was not performed; the proof uses the same real filesystem artifact shape and the production startup sweep helper directly.

How tested

  • RED: CI=true npm exec -- pnpm test extensions/memory-core/src/memory/manager.atomic-reindex.test.ts failed with TypeError: sweepMemoryIndexTempFiles is not a function.
  • CI=true npm exec -- pnpm docs:list
  • CI=true npm exec -- pnpm exec oxfmt --check --threads=1 extensions/memory-core/src/memory/manager-atomic-reindex.ts extensions/memory-core/src/memory/manager.ts extensions/memory-core/src/memory/manager.atomic-reindex.test.ts extensions/memory-core/src/memory/index.test.ts
  • CI=true npm exec -- pnpm test extensions/memory-core/src/memory/manager.atomic-reindex.test.ts extensions/memory-core/src/memory/index.test.ts
  • CI=true npm exec --package corepack -- corepack pnpm check:changed
  • npm exec -- pnpm exec oxfmt --check --threads=1 CHANGELOG.md
  • git diff --check

Existing PRs

Searched open PRs for memory reindex/temp SQLite cleanup terms and did not find an existing fix PR.

@openclaw-barnacle openclaw-barnacle Bot added extensions: memory-core Extension: memory-core size: S triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup. labels Jun 14, 2026
@clawsweeper

clawsweeper Bot commented Jun 14, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Codex review: needs maintainer review before merge. Reviewed June 14, 2026, 9:54 AM ET / 13:54 UTC.

Summary
The PR adds a memory-core startup sweep for aged atomic reindex SQLite temp files and covers the helper plus manager startup path with regression tests.

PR surface: Source +100, Tests +62. Total +162 across 4 files.

Reproducibility: yes. Source inspection shows current main creates memory reindex temp SQLite files and only removes them through in-process success/error cleanup paths that a hard kill cannot run.

Review metrics: none identified.

Stored data model
Persistent data-model change detected: migration/backfill/repair: extensions/memory-core/src/memory/manager.ts, serialized state: extensions/memory-core/src/memory/index.test.ts, serialized state: extensions/memory-core/src/memory/manager.atomic-reindex.test.ts, unknown-data-model-change: extensions/memory-core/src/memory/manager-atomic-reindex.ts, unknown-data-model-change: extensions/memory-core/src/memory/manager.atomic-reindex.test.ts, vector/embedding metadata: extensions/memory-core/src/memory/index.test.ts, and 2 more. Confirm migration or upgrade compatibility proof before merge.

Merge readiness
Overall: 🐚 platinum hermit
Proof: 🐚 platinum hermit
Patch quality: 🐚 platinum hermit
Result: ready for maintainer review.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

  • none.

Risk before merge

  • [P1] The proof does not run a destructive live SIGKILL or long concurrent reindex; the remaining confidence comes from source-level root cause, focused tests, terminal proof of the production sweep helper, and green checks.

Maintainer options:

  1. Decide the mitigation before merge
    Land the focused memory-core startup sweep through normal maintainer review once merge gates are satisfied, keeping release-note text in the PR body and release process rather than CHANGELOG.md.
  2. Pause or close
    Do not merge this PR until maintainers decide whether the risk is worth taking.

Next step before merge

  • [P2] No repair job is needed; the prior proof and changelog blockers were addressed and no narrow actionable defect remains.

Security
Cleared: No concrete security or supply-chain concern found; the diff changes memory-core SQLite cleanup code and tests without new dependencies, workflows, permissions, package scripts, or secret handling.

Review details

Best possible solution:

Land the focused memory-core startup sweep through normal maintainer review once merge gates are satisfied, keeping release-note text in the PR body and release process rather than CHANGELOG.md.

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

Yes. Source inspection shows current main creates memory reindex temp SQLite files and only removes them through in-process success/error cleanup paths that a hard kill cannot run.

Is this the best way to solve the issue?

Yes. The startup sweep in memory-core before opening the manager is the narrow owner-boundary fix, while a config flag, doctor-only cleanup, or broad storage migration would be heavier than this source-backed bug requires.

AGENTS.md: found and applied where relevant.

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

Label changes

Label changes:

  • add proof: sufficient: Contributor real behavior proof is sufficient. The updated PR body supplies terminal output from a real temporary filesystem run invoking the production sweep helper after the patch, showing aged SQLite temp triplets removed and the live DB retained.
  • add rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🐚 platinum hermit and patch quality is 🐚 platinum hermit.
  • add status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (terminal): The updated PR body supplies terminal output from a real temporary filesystem run invoking the production sweep helper after the patch, showing aged SQLite temp triplets removed and the live DB retained.
  • remove rating: 🧂 unranked krab: Current PR rating is rating: 🐚 platinum hermit, so this older rating label is no longer current.
  • remove status: 📣 needs proof: Current PR status label is status: 👀 ready for maintainer look.

Label justifications:

  • P2: This PR addresses a normal-priority memory-core disk-bloat bug with a narrow affected surface and no emergency availability or security signal.
  • rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🐚 platinum hermit and patch quality is 🐚 platinum hermit.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (terminal): The updated PR body supplies terminal output from a real temporary filesystem run invoking the production sweep helper after the patch, showing aged SQLite temp triplets removed and the live DB retained.
  • proof: sufficient: Contributor real behavior proof is sufficient. The updated PR body supplies terminal output from a real temporary filesystem run invoking the production sweep helper after the patch, showing aged SQLite temp triplets removed and the live DB retained.
Evidence reviewed

PR surface:

Source +100, Tests +62. Total +162 across 4 files.

View PR surface stats
Area Files Added Removed Net
Source 2 105 5 +100
Tests 2 62 0 +62
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 4 167 5 +162

What I checked:

Likely related people:

  • vincentkoc: Merged the recent memory reindex rollback recovery PR and authored follow-up commits touching the same atomic reindex and manager-sync surfaces. (role: recent area contributor and merger; confidence: high; commits: e99ab385cfe1, 2431e06ae2b6, 4b24143f39c3; files: extensions/memory-core/src/memory/manager-atomic-reindex.ts, extensions/memory-core/src/memory/manager-sync-ops.ts, extensions/memory-core/src/memory/manager.atomic-reindex.test.ts)
  • TSHOGX: Co-authored the source repair carried through the recent memory reindex rollback recovery PR that touched the same manager and atomic reindex area. (role: recent memory reindex recovery contributor; confidence: medium; commits: 9c6a3ed1243c, e99ab385cfe1; files: extensions/memory-core/src/memory/manager-sync-ops.ts, extensions/memory-core/src/memory/manager-atomic-reindex.ts, extensions/memory-core/src/memory/manager.reindex-recovery.test.ts)
  • steipete: Authored the extraction of the memory atomic reindex helper that owns the cleanup and swap behavior this PR extends. (role: area refactor contributor; confidence: medium; commits: c9f288ceafbf; files: extensions/memory-core/src/memory/manager-atomic-reindex.ts, extensions/memory-core/src/memory/manager-sync-ops.ts)
  • Jefsky: Current-main blame for the temp path and atomic helper lines points to a broad grafted commit; the title is unrelated, so this is useful only as a low-confidence routing signal. (role: current-main blame signal; confidence: low; commits: 462c076a24e2; files: extensions/memory-core/src/memory/manager-sync-ops.ts, extensions/memory-core/src/memory/manager-atomic-reindex.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.

@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 14, 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 14, 2026
@pony-maggie

pony-maggie commented Jun 14, 2026

Copy link
Copy Markdown
Author

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented Jun 14, 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.

Re-review progress:

@clawsweeper clawsweeper Bot added proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. and removed 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 14, 2026
@clawsweeper clawsweeper Bot mentioned this pull request Jun 14, 2026
Merged
@vincentkoc

vincentkoc commented Jun 14, 2026

Copy link
Copy Markdown
Member

Superseded by #92891, landed as a42bda5.

The landed canonical path uses a SQLite-exclusive owner guard shared by safe reindex, destructive aged-temp cleanup, and missing canonical-path creation. That preserves active reindexes and closes the Windows swap/create race. This PR's age-only sweep does not exclude an active long-running reindex, and landing both would duplicate startup cleanup behavior.

Thanks for the focused cleanup proposal.

@vincentkoc vincentkoc closed this Jun 14, 2026
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 P2 Normal backlog priority with limited blast radius. proof: sufficient ClawSweeper judged the real behavior proof convincing. proof: supplied External PR includes structured after-fix real behavior proof. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. size: S status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Builtin memory backend leaks orphaned *.sqlite.tmp-<uuid> reindex files on hard restart (no startup sweep)

2 participants

@pony-maggie @vincentkoc