fix(memories): record the resolved path, not the raw pointer (FT-MR11) - #1
Merged
Conversation
`generateIndex` resolved each MEMORY.md reference correctly — trying the store directory, then its parent — and then discarded the result, storing `ref.path` verbatim. The canonical store writes pointers as `memory/foo.md`, where `memory/` is a namespace label for the store rather than a subdirectory of it, so the CLI's `rewritePathsAbsolute` re-applied the prefix against the store root alone and emitted a doubled `…/memory/memory/foo.md`. On the canonical store that left 420 of 492 published entries (85%) pointing at files that do not exist. The UserPromptSubmit hook reads that published index on every prompt, so every session was silently handed dead paths and fell back to paraphrasing one-line summaries — precisely what the store's own rule forbids. Entries now record the location that actually resolved, relative to the store root with POSIX separators, so both of the store's drifted layouts resolve and no memory file has to move. Store-relative rather than absolute keeps the on-disk index portable, which is the contract `rewritePathsAbsolute` depends on. Live index: 72/492 -> 492/492. The original fixture put MEMORY.md *above* its `memory/` directory, so every ref matched on the first base and the parent-base fallback was never under test — which is how this reached production. `fixtures/flat-store/` pins the real shape, with one flat topic and one genuinely-nested topic, and asserts every entry resolves once joined onto the store root. Also drop the hardcoded home directory from `DEFAULT_STORE`: it was an absolute literal containing a username, so the shipped default resolved on exactly one computer and leaked that username into a public package. Now derived from `homedir()`, matching `defaultDest()` directly below it. The remaining username literals in repo instructions and test data are gone too; `identity-scan` reads RESULT CLEAN where it previously reported HIT 12. Verify: 383 tests across all four packages, 0 fail, exit 0. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
CI's `npm audit --audit-level=moderate` step went red on three high-severity brace-expansion DoS advisories (GHSA-3jxr-9vmj-r5cp, GHSA-mh99-v99m-4gvg, GHSA-rgw5-rvv9-x895). Not introduced here — the advisories postdate main's last green run on 2026-06-16, so main is red on this today too; it surfaced on this PR because this is the first run since. Dev-only transitive path: c8 -> test-exclude -> minimatch -> brace-expansion. `npm audit fix` resolves it inside the existing minimatch range, so the delta is five lockfile lines and one package. Audit now reports 0 vulnerabilities. Verify: 383 tests across all four packages, 0 fail, exit 0. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What was wrong
generateIndexresolved each MEMORY.md reference correctly — trying the store directory, then its parent — and then discarded the result, storingref.pathverbatim (index-gen.tsentry builders).The canonical store writes pointers as
memory/foo.md, wherememory/is a namespace label for the store, not a subdirectory of it. So the CLI'srewritePathsAbsolutere-applied the prefix against the store root alone and emitted a doubled…/memory/memory/foo.md.Why it mattered
On the canonical store this left 420 of 492 published entries (85%) pointing at files that do not exist.
The
UserPromptSubmithook reads that published index on every prompt in every session. When it hands a session a path that isn't there, the session either gets nothing or paraphrases from the one-line summary — which is exactly what the store's own rule forbids ("open the file pointer before acting, do not paraphrase from the summary"). This was silently degrading memory recall everywhere.The fix
Entries now record the location that actually resolved, relative to the store root with POSIX separators. Both of the store's drifted layouts (274 flat files + 70 nested) resolve, and no memory file has to move — the store is not version-controlled, so not touching it was the low-risk path.
Store-relative rather than absolute keeps the on-disk index portable, which is the contract
rewritePathsAbsolutedepends on.Acceptance test
Was
72/492. The 59 genuinely-nested entries still resolve correctly.Why the tests missed it
The original fixture puts
MEMORY.mdabove itsmemory/directory, so every ref matched on the first base and the parent-base fallback was never exercised. That is how this reached production.fixtures/flat-store/pins the real shape — MEMORY.md inside the store, one flat topic, one genuinely-nested topic — and asserts every entry resolves once joined onto the store root, mirroring the live acceptance check.Also in this PR
DEFAULT_STOREwas an absolute literal containing a username, so the shipped default resolved on exactly one computer and leaked that username into a public package. Now derived fromhomedir(), matchingdefaultDest()directly below it. Remaining username literals in repo instructions and test data are gone too.identity-scanreads RESULT CLEAN; it previously reported HIT 12 onmain.Verification
npm run verify— 383 tests across all four packages, 0 fail, exit 0identity-scan.py— RESULT CLEANloadout-os refresh— exit 0, no andon halt, live index repairedNot in scope
Publishing to npm is the Director's call, so the version is unbumped and the entry sits under
[Unreleased]. Until a release ships, the globalloadout-osbinary (v1.0.2) still carries the bug — the live index was repaired by running this branch's build directly.🤖 Generated with Claude Code