Skip to content

fix(memory): refresh rebuilt index handles#90727

Closed
dr00-eth wants to merge 1 commit into
openclaw:mainfrom
dr00-eth:fix/memory-search-refresh-index-identity
Closed

fix(memory): refresh rebuilt index handles#90727
dr00-eth wants to merge 1 commit into
openclaw:mainfrom
dr00-eth:fix/memory-search-refresh-index-identity

Conversation

@dr00-eth

@dr00-eth dr00-eth commented Jun 5, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Fixes a persistent live-manager failure where the injected memory search path can keep reporting index metadata is missing after a separate CLI/status manager has rebuilt the durable SQLite index successfully.
  • Ensures safe atomic reindex writes memory_index_meta_v1 into the fresh temp DB even when the previous DB had the same serialized metadata cached.
  • Tracks the memory DB file identity and reopens the live SQLite handle before status/search/sync when another manager has atomically swapped the index file underneath it.
  • Defers search warmup and on-search background sync until provider and index identity are valid, reducing self-induced dirty-state work before a search has passed readiness checks.
  • Out of scope: fully serializing searches while an active safe reindex is mid-build. That related transient guard still belongs with fix(memory-core): guard searches during safe reindex #90453 if maintainers want a separate in-progress reindex state.

Linked context

Closes #90414

Related #90361
Related #90338
Related #90395
Related #90453

Requested by an affected owner/user during live troubleshooting of a local OpenClaw install.

Real behavior proof (required for external PRs)

  • Behavior or issue addressed: live injected memory_search / openclaw.memory_search could report index metadata is missing even though the durable disk index was healthy and CLI/fresh-manager search worked.
  • Real environment tested: macOS local OpenClaw gateway, OpenClaw 2026.6.2-beta.1 (f03f57a), Node v25.6.1, builtin memory index, installed global npm runtime.
  • Exact steps or command run after this patch:
    1. Applied the same manager changes locally to /usr/local/lib/node_modules/openclaw/dist/manager-B5STWg-k.js.
    2. Verified the patched installed bundle with node --check /usr/local/lib/node_modules/openclaw/dist/manager-B5STWg-k.js.
    3. Restarted the gateway with openclaw gateway stop and openclaw gateway start.
    4. Warmed the cached live gateway manager through the injected-tool dispatcher:
      openclaw gateway call tools.invoke --json --params '{"name":"memory_search","args":{"query":"openclaw memory_search metadata","maxResults":1,"corpus":"memory"},"agentId":"main","sessionKey":"agent:main:main","idempotencyKey":"memory-pr-proof-warm-2"}' --timeout 30000
    5. Forced an external CLI rebuild while the gateway live manager was already warmed:
      openclaw memory index --agent main --force
    6. Confirmed the rebuilt durable index with:
      openclaw memory status --agent main --index --json
    7. Invoked the gateway-injected tool again after the external rebuild:
      openclaw gateway call tools.invoke --json --params '{"name":"memory_search","args":{"query":"openclaw memory_search metadata","maxResults":1,"corpus":"memory"},"agentId":"main","sessionKey":"agent:main:main","idempotencyKey":"memory-pr-proof-after-rebuild"}' --timeout 30000
  • Evidence after fix (redacted from real terminal output):
OpenClaw 2026.6.2-beta.1 (f03f57a)
node --version: v25.6.1
Gateway version: 2026.6.2-beta.1
Runtime: running
Connectivity probe: ok
Capability: admin-capable
Listening: 127.0.0.1:18789, [::1]:18789

Warm injected gateway tool call before external rebuild:
{
  "ok": true,
  "toolName": "memory_search",
  "source": "plugin",
  "output.details.provider": "openai",
  "output.details.model": "text-embedding-3-small",
  "output.details.debug.backend": "builtin",
  "output.details.debug.hits": 1,
  "output.details.debug.searchMs": 695,
  "output.details.results[0].path": "memory/2026-06-05.md",
  "output.details.results[0].citation": "memory/2026-06-05.md#L14"
}

External rebuild:
Memory index updated (main).

Fresh CLI status after rebuild:
{
  "agentId": "main",
  "status.backend": "builtin",
  "status.files": 115,
  "status.chunks": 1806,
  "status.dirty": false,
  "status.provider": "openai",
  "status.model": "text-embedding-3-small",
  "status.vector.semanticAvailable": true,
  "status.vector.dims": 1536,
  "status.custom.indexIdentity.status": "valid",
  "embeddingProbe.ok": true
}

Injected gateway tool call after external rebuild:
{
  "ok": true,
  "toolName": "memory_search",
  "source": "plugin",
  "output.details.provider": "openai",
  "output.details.model": "text-embedding-3-small",
  "output.details.debug.backend": "builtin",
  "output.details.debug.hits": 1,
  "output.details.debug.searchMs": 1327,
  "output.details.results[0].path": "memory/2026-06-05.md",
  "output.details.results[0].citation": "memory/2026-06-05.md#L14"
}
  • Observed result after fix: after the warmed live gateway manager and an external CLI full rebuild, the injected gateway memory_search tool returned ok: true with provider/model/debug hit details instead of disabled=true, unavailable=true, or error="index metadata is missing".
  • What was not tested: a live Discord message that lets an LLM choose the memory tool organically. The direct tools.invoke gateway path exercises the same gateway-visible injected tool dispatcher without depending on model tool-choice behavior.
  • Proof limitations or environment constraints: broad repository type-aware checks are blocked in this checkout by unrelated missing optional extension deps and plugin-sdk boundary generation issues; focused memory checks pass.
  • Before evidence: live troubleshooting showed the disk memory index was healthy and CLI memory search returned results, while the live injected tool still failed with index metadata is missing after gateway restart. One matching memory record from 2026-06-05 11:54 CDT documents a prior live openclaw.memory_search call returning disabled=true, unavailable=true, error="index metadata is missing" while CLI status showed indexIdentity.status=valid, vector/FTS ready, and embedding probe OK.

Tests and validation

Focused commands run after rebasing onto current origin/main:

git diff --check origin/main...HEAD
node_modules/.bin/oxlint extensions/memory-core/src/memory/manager.ts extensions/memory-core/src/memory/manager-sync-ops.ts extensions/memory-core/src/memory/index.test.ts --report-unused-disable-directives-severity error --threads=1
OPENCLAW_VITEST_INCLUDE_FILE=/private/tmp/openclaw-memory-include.json node scripts/run-vitest.mjs run --config test/vitest/vitest.extension-memory.config.ts
  Test Files  1 passed (1)
  Tests       36 passed (36)

Regression coverage added:

  • index.test.ts now covers a live manager reopening after another manager atomically rebuilds an index that had missing metadata.
  • index.test.ts also covers a live manager reopening after another manager atomically rebuilds a valid index with changed content, proving the stale handle is not kept after file replacement.

What failed before this fix:

  • runSafeReindex() could switch to a fresh temp DB while lastMetaSerialized still matched the previous DB, causing writeMeta() to skip writing metadata into the new DB.
  • A cached live manager could hold a SQLite handle to the old/unlinked DB after another manager atomically replaced main.sqlite, so status/search could keep seeing missing or stale metadata while disk and CLI were healthy.

Risk checklist

Did user-visible behavior change? (Yes/No): Yes, memory search should recover from externally rebuilt indexes instead of staying paused on stale live manager state.

Did config, environment, or migration behavior change? (Yes/No): No.

Did security, auth, secrets, network, or tool execution behavior change? (Yes/No): No.

What is the highest-risk area?

  • Reopening the SQLite handle when the index file identity changes while the live manager is otherwise idle.

How is that risk mitigated?

  • The refresh is skipped while the manager is closed or already syncing.
  • The old handle is kept unless the new handle opens, schema is ensured, metadata can be read, and vector/FTS state is reset consistently.
  • On failure, the code restores the previous DB handle and identity and logs a warning.
  • Focused tests cover both missing-meta and valid-meta external rebuild cases.
  • Real runtime proof now covers a warmed injected gateway memory_search call succeeding after an external CLI full rebuild.

Current review state

What is the next action?

  • Maintainer review, CI, and ClawSweeper re-review with direct injected-tool proof now added.

What is still waiting on author, maintainer, CI, or external proof?

Which bot or reviewer comments were addressed?

  • Addressed ClawSweeper's P1 proof request by adding redacted real output from a gateway-injected memory_search call after an external CLI rebuild showing the missing-metadata error is gone.

@clawsweeper

clawsweeper Bot commented Jun 5, 2026

Copy link
Copy Markdown
Contributor

Thanks for the context here. I did a careful shell check against current main, and this is already implemented.

Close as implemented on current main: the whole-file memory index replacement that made live managers keep stale rebuilt-index handles has been replaced by per-agent SQLite storage plus transactional memory-table publication, so the branch's central handle-refresh fix is no longer needed.

So I’m closing this as already implemented rather than keeping a duplicate issue open.

Review details

Best possible solution:

Keep the current per-agent SQLite/table-publication architecture and continue the broader safe-reindex concurrency work in its canonical issue/PR instead of merging this obsolete handle-refresh branch.

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

Yes for the original reported branch scenario: the PR discussion supplied live before/after output, and source inspection shows the old whole-file rebuild path could orphan a running manager's open DB handle; current main no longer uses that path.

Is this the best way to solve the issue?

No, this PR is no longer the best fix: current main removes the underlying file-swap handle problem by publishing rebuilt memory tables into the existing agent DB, while the remaining in-flight reindex/read race belongs to the separate canonical work.

Security review:

Security review cleared: The branch touches memory-core runtime and tests only; it does not change secrets, auth, dependencies, workflows, package resolution, or external code execution paths.

AGENTS.md: found and applied where relevant.

What I checked:

  • Current main reindexes without replacing the live agent DB file: runInPlaceReindex builds outside the shared agent DB and publishes memory-owned tables back into originalDb, removing the stale open-file-handle failure mode the PR targets. (extensions/memory-core/src/memory/manager-sync-ops.ts:2441, a42a1af942b2)
  • Transactional table publication replaces whole-file swaps: publishMemoryDatabaseTables attaches the shadow DB and updates main.memory_index_*, FTS, vector, and embedding-cache tables inside the existing database instead of moving the database file underneath a live process. (extensions/memory-core/src/memory/manager-db.ts:118, a42a1af942b2)
  • Docs confirm current storage is the per-agent SQLite DB: The memory configuration docs now describe built-in memory indexes as living in agents/<agentId>/agent/openclaw-agent.sqlite, which matches the current shared-agent-DB implementation rather than the old separate store.path index file contract. Public docs: docs/reference/memory-config.md. (docs/reference/memory-config.md:461, a42a1af942b2)
  • Old atomic sidecar reindex helpers are gone from active memory runtime: Repository search found no active runMemoryAtomicReindex, runSafeReindex, runUnsafeReindex, moveMemoryIndexFiles, or memory store.path runtime path; only the current shadow reindex database remains. (extensions/memory-core/src/memory/manager-sync-ops.ts:2486, a42a1af942b2)
  • Fix provenance on main: Commit 8b7269d19782fb4aad98b9200b5f84dda6df9a8e introduced the SQLite preservation/reindex-safety change that removed the whole-file replacement architecture for this path. (extensions/memory-core/src/memory/manager-db.ts:118, 8b7269d19782)
  • Current-main-only release status: The fix commit is ahead of the latest checked release tag v2026.6.8 and no local release tag contains the current main proof commit, so this is implemented on main but not yet identified in a shipped release. (8b7269d19782)

Likely related people:

  • vincentkoc: Authored the current-main SQLite reindex-safety change that introduced table publication and removed the PR's stale file-swap failure class. (role: recent area contributor; confidence: high; commits: 8b7269d19782; files: extensions/memory-core/src/memory/manager-sync-ops.ts, extensions/memory-core/src/memory/manager-db.ts)
  • Peter Steinberger: Local blame on the current memory reindex block points to the merge/graft carrier commit that brought the current implementation into this checkout. (role: current-main carrier; confidence: medium; commits: 5a00720de0; files: extensions/memory-core/src/memory/manager-sync-ops.ts)
  • dr00-eth: Authored this PR's focused stale-handle refresh branch and supplied live proof for the original failure mode, which is useful context even though current main now solves the central path differently. (role: candidate fix author; confidence: medium; commits: 203017314e5b; 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 a42a1af942b2; fix evidence: commit 8b7269d19782, main fix timestamp 2026-06-18T17:20:06Z.

@clawsweeper clawsweeper Bot added rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. P2 Normal backlog priority with limited blast radius. merge-risk: 🚨 session-state 🚨 May lose, corrupt, stale, or mis-associate session, agent, or context state. labels Jun 5, 2026
@dr00-eth

dr00-eth commented Jun 5, 2026

Copy link
Copy Markdown
Contributor Author

Updated the PR body with direct injected-tool proof for the P1 ClawSweeper ask.

Proof sequence now included:

  1. Warmed the cached live gateway manager through openclaw gateway call tools.invoke for memory_search.
  2. Ran external CLI rebuild: openclaw memory index --agent main --force -> Memory index updated (main).
  3. Confirmed CLI durable status after rebuild: indexIdentity.status=valid, dirty=false, semanticAvailable=true, dims=1536, embeddingProbe.ok=true.
  4. Invoked gateway memory_search again after the external rebuild and got ok=true, source=plugin, provider openai, model text-embedding-3-small, debug.backend=builtin, debug.hits=1, not disabled=true / unavailable=true / index metadata is missing.

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented Jun 5, 2026

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: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. labels Jun 5, 2026

@kagura-agent kagura-agent left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review from an affected user (OpenClaw 2026.6.1, 12 agents, all with missing meta)

✅ What this PR gets right

1. lastMetaSerialized = null before temp DB switch (manager-sync-ops.ts)
This is the primary fix. The root cause is that writeMeta() skips the INSERT when the serialized value matches the cached lastMetaSerialized from the original DB, but after this.db is swapped to the temp DB, the cache still holds the old value. Clearing it ensures the meta row is always written to the fresh temp DB. The save/restore in originalState is also correct for the error recovery path.

2. refreshDatabaseHandleIfPathChanged() with inode tracking (manager.ts)
This solves the second failure mode: CLI rebuilds the index (atomic file swap), but the gateway process keeps an open handle to the old (now-deleted) inode. Using dev + ino + size + mtimeMs to detect file replacement and reopen is a solid approach. Calling it at three points (search, sync, refreshIndexIdentityDirty) provides good coverage.

3. Moving async search sync after index identity validation
Smart change. The old position could trigger a background sync that itself hits the missing-meta state, creating noise. Deferring until after validation avoids self-induced dirty-state work.

🤔 Minor observations (not blocking)

1. statSync on the hot path
refreshDatabaseHandleIfPathChanged() calls statSync() on every search and sync. For most installations this is negligible (stat is fast on local fs, and the DB path is cached by the OS). But if anyone runs memory on a network mount, this could add latency. A debounce or TTL cache on the stat result might be worth considering — though I'd defer to maintainer judgment on whether the complexity is warranted.

2. The identity check uses size + mtimeMs alongside dev + ino
Since the atomic swap uses rename (which changes inode), dev + ino alone would be sufficient to detect a swap. Including size + mtimeMs adds robustness for edge cases where the file is modified in-place (e.g., WAL checkpoint changing size), but it could also cause unnecessary reopens during normal WAL activity. In practice this probably doesn't matter because the guard if (this.closed || this.syncing !== null) prevents reopens during active operations.

3. Test coverage looks good
Both test cases (missing meta + valid meta with content change) cover the key scenarios. The test that deletes the meta row and then rebuilds via a separate CLI manager mirrors the real-world failure path exactly.

Verdict

This PR correctly addresses the two root causes I independently identified through DB inspection and source analysis on my own installation. The lastMetaSerialized cache bug is the primary issue; the stale file handle is the secondary one. Both fixes are minimal and well-scoped. 👍

wc4482 commented Jun 12, 2026

Copy link
Copy Markdown

Supplemental affected-install field sequence from an unpatched OpenClaw 2026.6.1 install.

This is not branch validation of this PR. It is diagnostic evidence that the same stale live runtime / rebuilt-index split still appears on my install, and that a gateway/runtime restart clears the stale metadata state.

Environment:

  • OpenClaw CLI/Gateway: 2026.6.1
  • Node: v24.16.0
  • Platform: Linux x86_64 container
  • Memory backend: builtin
  • Agent: main
  • Provider/model: openai / text-embedding-3-small

Pre-repair baseline:

helper classification: metadata_missing
files/chunks: 1036 / 9588
dirty: true
provider/model: openai / text-embedding-3-small
index_identity_status: missing
index_identity_reason: index metadata is missing
db.files_table_present: true
db.chunks_table_present: true
db.fts_present: true
db.vector_present: true
db.metadata_table_exists: false

live memory_search:
disabled: true
unavailable: true
error: index metadata is missing

Hydrate command:

python3 scripts/openclaw-memory-semantic-guard.py hydrate

Wrapped command:

openclaw memory status --deep --index --agent main

Hydrate result:

exit_code: 0
Memory index complete.
Provider: openai (requested: openai)
Model: text-embedding-3-small
Indexed: 1039/1039 files, 9687 chunks
Dirty: no
Embeddings: ready
Vector dims: 1536
FTS: ready

Immediately after hydrate, CLI/helper state became valid:

helper classification: cli_valid_runtime_restart_may_be_needed
files/chunks: 1039 / 9687
dirty: false
provider/model: openai / text-embedding-3-small
index_identity_status: valid
vector.dims: 1536

But the already-running live runtime still returned the old failure:

live memory_search:
disabled: true
unavailable: true
error: index metadata is missing

helper classification with runtime canary supplied:
cli_valid_runtime_stale_suspected
runtime_canary.stale_error_observed: true

After a gateway-only restart, helper status was still healthy/valid:

gateway healthz: ok
gateway readyz: ok
gateway listener on port 18789: exactly 1
runtimeVersion: 2026.6.1

helper classification: healthy
files/chunks: 1041 / 9709
dirty: false
provider/model: openai / text-embedding-3-small
index_identity_status: valid
vector.dims: 1536

Post-restart live memory search no longer returned index metadata is missing. First canary timed out after 15s, while readyz showed event-loop degradation, so there may still be a separate latency/load issue:

first post-restart canary:
disabled: true
unavailable: true
error: memory_search timed out after 15s

A second confirmation canary succeeded through the builtin OpenAI-backed memory path:

results: 1
provider/model: openai / text-embedding-3-small
debug.backend: builtin
debug.hits: 1
debug.searchMs: 9003

Interpretation:

  1. Before hydrate, CLI identity and live runtime both reported index metadata is missing.
  2. Hydrate repaired durable disk/CLI state: clean index, valid identity, expected provider/model, vector dims 1536.
  3. The already-running runtime still reported the pre-hydrate missing-metadata failure.
  4. Restarting the gateway/runtime cleared the stale metadata failure; live search returned a result on retry.
  5. Remaining caveat: search latency/event-loop degradation remains visible, but that is distinct from the stale metadata state.

This matches the stale live SQLite handle/cache class this PR is addressing, distinct from the OpenRouter/openai-compatible provider-key reports being discussed elsewhere.

@clawsweeper clawsweeper Bot added rating: 🌊 off-meta tidepool PR readiness rating does not apply to this item. and removed 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. labels Jun 12, 2026
@clawsweeper clawsweeper Bot added proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. and removed rating: 🌊 off-meta tidepool PR readiness rating does not apply to this item. labels Jun 15, 2026
@clawsweeper clawsweeper Bot added status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. and removed status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. merge-risk: 🚨 session-state 🚨 May lose, corrupt, stale, or mis-associate session, agent, or context state. labels Jun 19, 2026
@clawsweeper

clawsweeper Bot commented Jun 19, 2026

Copy link
Copy Markdown
Contributor

ClawSweeper applied the proposed close for this PR.

@clawsweeper clawsweeper Bot closed this Jun 19, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

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: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. size: S status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

agentmemory__memory_search returns "index metadata is missing" persistently (memory-core manager state cache)

3 participants