fix: the full-codebase review findings — data loss, a CSRF boundary, secrets in the graph, and 19 root causes

[kevintseng]

Fixes the findings from a full-codebase review, grouped by root cause rather
than by symptom. 18 commits, each with its own reasoning and its own
break-test.

The defects below are not a list of edge cases. Most of them are the same
shape: something was written correctly and then read by nothing, or read by
something asking a different question. Several had been live for months while
the suite stayed green.

Data loss and destruction

• forget with an empty or unknown selector archived the whole memory.
  if (args.observation) treated "" as "no selector", which selects the
  destructive branch. Three surfaces reached it: the CLI (--observation "",
  which an unset shell variable produces), the core, and MCP — where
  stripNullProps ran before .strict(), so forget({name, observations: null}) (plural, the word remember uses) arrived as {name} alone and
  archived the memory, reporting {archived: true} without mentioning the
  observation the caller asked about. Fixed at all three layers.

• A backup was not a backup. export → import restored with none of
  its relations — they were created inside the per-entity loop and skipped
  when the target "may not have been imported yet", which is the ordinary case
  because export writes newest-first and relations point newer → older. It also
  dropped created_at (so a restore stamped every memory with the day of the
  restore, flattening the timeline that drives ranking, clustering and memesh why), skipped archived entities entirely (so forget → export → restore
  brought the memory back to life), and lost metadata. Bundle version 3.1.0;
  older bundles import unchanged.

• A dream digest could overwrite a memory you wrote. applyProposal called
  createEntity with the model's chosen name and no collision check.
  INSERT OR IGNORE means a taken name skips the insert — so the LLM's
  observations appended to your memory, none of the digest metadata was
  written, and the same transaction archived up to five more of your memories
  under it and reported success. The sibling transcript path has carried this
  guard the whole time.

• A half-applied schema migration was permanent. Five ALTERs behind one
  guard; a SQLITE_BUSY on the second left the database permanently
  half-migrated with getEntity broken and no way to heal.

• Telemetry pruning deleted rows it was meant to keep. ts is SQLite's
  format, the cutoff was ISO — they differ at the separator, where ' ' sorts
  before 'T', so every row from the cutoff day was deleted regardless of
  its time. Four more comparisons had the same defect.

Security

• Any website you visited could drive the local API. The loopback listener
  has no authentication by design, so "only this machine" was the whole
  boundary — and a browser is on this machine. A page could auto-submit a form
  to http://127.0.0.1:3737/v1/demo/reset (a CORS simple request: no
  preflight) and the handler ran. Same reach for POST /v1/dream/run and the
  proposal accept/reject routes. Now checked on every /v1/* request:
  Sec-Fetch-Site, Origin against the arriving Host, and — first, because
  it defeats the other two — a loopback Host, which is what DNS rebinding
  forges. Non-browser clients send none of these and are unaffected.

• API keys were stored as memories and sent to your LLM provider. The Stop
  hook copied bash command lines and failed-tool error text into the graph
  verbatim — the two likeliest places a credential appears in a session.
  redactSecrets already existed, already exported to the hooks. Nothing
  called it.

• autoUpdate: off was overridable from the npm registry. The deprecation
  override fired only for a patch bump, and every policy above off already
  permits one — so off was the only setting it could change, triggered by a
  string the publisher writes. A remote switch on a user's explicit refusal,
  in a hook that spawns a detached npm install -g.

Things that were silently not working

• 344 of 499 memories had no vector, and doctor reported the database
  healthy. The Vector Index row read pending_reindex, a marker whose only
  writer is reindex() — the seven capture hooks, import, none of them set
  it. Semantic recall could not see 69% of the graph and nothing said so.
  Doctor now counts.

• Editing a memory left the old vector in place. clearEntityData cleared
  the observations, the tags and the keyword index and left the vector — so a
  memory edited to say the opposite of what it used to say still came back for
  the old query, with the new text attached.

• The impact factor was a constant. 10% of ranking. The hydrator behind
  every recall did not SELECT recall_hits/recall_misses, so
  impactScore(0, 0) = 0.5 for every row, forever. On a real graph those
  counters span 0.037 to 0.750.

• Commit signal scoring was inverted. A commit with a real body scored 0.3;
  one with no body scored 0.6. Measured on the live graph: 459 commits at 0.6,
  exactly one below 0.4. The rule written to demote mechanical commits was
  promoting them.

• export re-sorted your graph. Taking a backup bumped access_count and
  stamped last_accessed_at on up to a thousand memories — 20% of the ranking.

• Two user_patterns fields were parsed from a format nothing writes.
  toolPreferences and avgSessionMinutes were permanently [] and 0
  across the MCP tool, the dashboard and the docs. Removed rather than
  implemented; their test asserted the deadness. See CHANGELOG → Removed.

Diagnostics and messages

• Corrupt database → a 15-line stack trace with your install path, from every
  command. doctor handled the same state perfectly and was the only one that
  did.
• recall --limit abc → raw ERR_SQLITE_ERROR; export --limit abc silently
  ignored the flag. One coercion now covers all fourteen numeric flags.
• Version comparison was a string compare: at 4.6.10 doctor would say
  "running pre-release 4.6.9" while the banner urged an upgrade.
• Reinstall advice hardcoded npm install -g for all four install channels —
  for a plugin user that does not repair the install, it creates a second one
  beside it sharing one database.
• install-channel answered unknown whenever npm was not on PATH (Claude
  Code launched from a GUI app), and memesh update then refused a genuine
  npm-global install.
• No timeout on any LLM request. embedder.ts had already fixed this on its
  own path and written down what it measured.

Two that were invisible to tooling

• src/core/install-hooks.ts was a binary file to grep and rg. Its
  prune key used a literal NUL byte as a separator, and both tools suppress
  every match in a file containing one — exiting exactly as they do for a
  clean file. So the 485-line file that edits your ~/.claude/settings.json
  answered "no matches" to every pattern any grep-based reviewer ran against
  it. Now written as the \u0000 escape: identical runtime value, and the file
  is text again. Swept the repo — one file affected, and no gate was passing
  because of it.

• uninstall-hooks deleted the citation rule and then checked
  settings.json. On unparseable JSON it printed "refusing to modify", exited
  1, and left .claude/rules/ empty.

Tests

26 tests were green and could not fail. The two that mattered most:

• Semantic provenance had zero effective coverage on every machine. Both
  tests that claimed it are structural no-ops — one asserts inside a loop over
  an array that is always empty without a configured embedder, the other
  early-returns on the output that same machine produces.
• The LLM fallback chain was forwarded at eight sites and asserted at zero.
  Delete the forwarding everywhere and the suite stays green, while a user who
  configured llmFallbacks gets no failover and the outage reads as designed
  degradation.

Also: a hook that crashed before printing anything passed the hook-output
contract (empty stdout is a valid payload — the exit code was discarded), and
eleven identical locale blocks would have passed i18n parity, which compared
keys only.

Verification

node scripts/run-tests-isolated.mjs
  Test Files  183 passed (183)
  Tests       2516 passed (2516)          exit=0

npm run lint                exit=0
npm run typecheck           exit=0
npm run build               exit=0
node scripts/check-doc-claims.mjs        exit=0
node scripts/audit/verification-audit.mjs exit=0

Every fix was break-tested: the fix reverted, the test confirmed red, the file
restored and checksum-verified. Each commit message records its own mutations
and their results.

One break-test result is worth repeating because it nearly produced a false
report: two mutants survived and read as "these tests are weak" when they
actually meant "this mutation is wrong" — I had removed setEvidenceNode when
the banner is driven by setEgoNodeId, and replaced the wrong one of two
role="alert" occurrences. A surviving mutant is evidence about the mutation
until the mutation itself has been checked.

Not in this PR

22 minor CLI and doctor friction items from the same review remain open — a
memesh status that never probes the database, config set autoCapture yes
silently becoming false, --obs " " stored as whitespace, a maintainer-only
row shown in a user support report. All real, all small, none blocking. They
are recorded and can be picked off by priority rather than in bulk.