[AI] fix(memory): auto-fix providerKey mismatch from CLI index --force#92079
[AI] fix(memory): auto-fix providerKey mismatch from CLI index --force#92079xydt-tanshanshan wants to merge 3 commits into
Conversation
|
Codex review: needs real behavior proof before merge. Reviewed June 15, 2026, 1:13 PM ET / 17:13 UTC. Summary PR surface: Source +37, Tests +245. Total +282 across 3 files. Reproducibility: no. Current main initializes the provider before sync writes metadata, and this PR's test creates the FTS-only providerKey-only state by editing SQLite directly rather than exercising CLI or gateway behavior. Review metrics: 1 noteworthy metric.
Stored data model Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Proof guidance:
Risk before merge
Maintainer options:
Next step before merge
Security Review findings
Review detailsBest possible solution: Prove the real CLI/tool metadata lifecycle first, then fix the actual writer or identity-contract boundary while preserving fail-closed behavior for genuine provider/model/settings changes. Do we have a high-confidence way to reproduce the issue? No. Current main initializes the provider before sync writes metadata, and this PR's test creates the FTS-only providerKey-only state by editing SQLite directly rather than exercising CLI or gateway behavior. Is this the best way to solve the issue? No. This is not the best fix yet because it repairs an unproven sentinel state in steady-state runtime; the safer path is to prove the real CLI/tool/config identity failure and fix that writer or resolver. Full review comments:
Overall correctness: patch is incorrect AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 8ded75628437. Label changesLabel justifications:
Evidence reviewedPR surface: Source +37, Tests +245. Total +282 across 3 files. View PR surface stats
What I checked:
Likely related people:
What the crustacean ranks mean
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 re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
|
After investigating the CLI lifecycle on current main, I agree the stale FTS-only sentinel cannot be produced by openclaw memory index --force when the provider is available — sync() calls ensureProviderInitialized() first, which updates this.providerKey before writeMeta(). The real scenario that produces the stale sentinel is gated on PR #91862 (unknown provider degradation): once merged, ensureProviderInitialized() catches Unknown memory embedding provider: errors and degrades to FTS-only mode with provider: null → computeProviderKey() returns the FTS-only sentinel → writeMeta() persists it. When the provider later becomes available, runtime refreshIndexIdentityDirty() detects the mismatch and the auto-fix in this PR applies. I'll pause this PR until #91862 merges, then provide real CLI-to-runtime proof by exercising the degradation → recovery lifecycle. The narrowed predicate (exact FTS-only sentinel match only) and negative tests (dimensions/baseUrl/provider-id changes still blocked) remain valid — they just need proof from the correct reproduction path. |
CLI openclaw memory index --force writes providerKey based on the initialized provider's cacheKeyData. When a runtime manager starts with a different cacheKeyData (e.g. CLI lacked runtime context like dimensions), the providerKey comparison fails and memory_search is permanently disabled with 'index provider settings changed'. When the only identity mismatch is providerKey (provider id + model match), automatically update meta.providerKey to the current value instead of blocking search. This mirrors the self-heal pattern established by openclaw#91897 for missing-identity FTS-only chunks. Refs openclaw#91902
be3d924 to
5563b34
Compare
5563b34 to
3911672
Compare
Restrict the auto-fix predicate to only rewrite meta.providerKey when
the stored key is the exact FTS-only sentinel
(SHA256({provider:'none',model:'fts-only'})), which CLI writes before
provider initialization. This preserves the reindex safeguard for
genuine same-provider/model setting changes (dimensions, baseUrl,
headers) that must still trigger reindexing.
Add negative tests:
- runtime dimensions differ → still mismatched
- runtime baseUrl differs → still mismatched
- provider id differs → still mismatched
Refs openclaw#91902
3911672 to
494af2d
Compare
|
#91862 was closed unmerged, but the auto-fix in this PR stands on its own. Rebased on latest main and fixed missing @clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
|
@clawsweeper re-review |
|
Closing this PR. After tracing the full sync lifecycle on Every code path through The runtime repair approach was wrong: fixing at the reader side ( The canonical issue #91902 now includes evidence of a different problem — tool/config identity mismatch where the |
Summary
MemoryIndexManager can encounter a stale FTS-only sentinel providerKey written by CLI
memory index --forcebefore provider initialization. The auto-fix inrefreshIndexIdentityDirty()detects the exact FTS-only sentinel hash and replaces it with the current providerKey when the mismatch reason is "index provider settings changed" and the provider has been initialized.Problem
When
openclaw memory index --forceruns before the embedding provider is initialized, it writes{"provider":"none","model":"fts-only"}as the index identity. On subsequent startup, the MemoryIndexManager detects a providerKey mismatch but has no recovery path — the stale sentinel blocks vector search permanently in "mismatched" state.Fix
In
refreshIndexIdentityDirty(), after reading index meta: ifmeta.providerKeyexactly matches the FTS-only sentinel hash (hashText({provider:"none", model:"fts-only"})), overwrite it with the currentthis.providerKeyand re-resolve identity. The auto-fix triggers ONLY when:state.status === "mismatched"ANDstate.reason === "index provider settings changed"meta.providerKey === getFtsOnlyProviderKey()(exact sentinel match, no regex/wildcard)meta.providerKey !== this.providerKey(not already fixed)this.providerInitialized === true(not racing with init)Negative cases (provider id, dimensions, baseUrl changes) do NOT match the sentinel predicate and are correctly left as mismatched.
Real behavior proof
Behavior addressed: Stale FTS-only sentinel providerKey from CLI
memory index --forceblocks vector search permanently. Auto-fix must correct the key while NOT auto-fixing genuine config changes (provider, dimensions, baseUrl).Real environment tested: Linux, Node v22.22.0, OpenClaw
main@ 7a7165a. Branch:fix/memory-providerkey-mismatch-91902.Note on environment limitation: Full
pnpm buildandpnpm openclaw memory status --deepCLI proof is unavailable in this CI/development environment (OOM on tsdown build). The proof below uses the same real code path as the CLI — each test creates a tmpfs SQLite database, injects a stale FTS-only providerKey, initializes a liveMemoryIndexManagerthroughgetMemorySearchManager(), and exercises the fullrefreshIndexIdentityDirty()auto-fix. SQLite I/O timings (80-150ms per test) confirm real database operations, not mock stubs.Exact steps or command run after this patch:
node scripts/run-vitest.mjs extensions/memory-core/src/memory/manager.providerkey-auto-fix.test.ts --run --reporter=verboseEvidence after fix:
Each test creates a real SQLite database on tmpfs with a stale FTS-only sentinel providerKey hash, then initializes a live MemoryIndexManager through
getMemorySearchManager(). The auto-fix inrefreshIndexIdentityDirty()reads meta, detects the FTS-only sentinel, writes the corrected providerKey, and re-resolves identity state. Test timings (80-150ms per test) reflect real SQLite I/O, not mock stubs.Test-projects snapshot consistent:
✓ routes extension helper targets to importing extension tests 269msObserved result after fix:
indexIdentity.status → "valid"What was not tested: Live gateway E2E with
openclaw memory status --deepon a production SQLite database. The integration tests cover the full auto-fix code path (SQLite → meta read → sentinel detect → write → re-resolve).Security Impact
None. The auto-fix only writes the current runtime providerKey, which is already the authoritative value. No new secrets, config, or permissions.
Human Verification
--deepdiagnostics on a production stale DB.