test(memoryOps): stop the memory-op tests writing into the real machine-wide index - #8
Merged
TheArchitectit merged 1 commit intoJul 29, 2026
Conversation
…ne-wide index applyMemoryOps() mirrors every write into the machine-wide PGlite memory index via the fire-and-forget indexMemoryWrite() → upsertMemoryEmbedding(), keyed by repoKey(stateDir). For this file's tmp state dirs that key is the tmp path itself, so running the file wrote rows like /tmp/mc-memops-w3XL9J/add "we use node:sqlite as the store" /tmp/mc-memops-w3XL9J/dup "threshold is 50k" /tmp/mc-memops-w3XL9J/replace "the threshold is 100k" into the developer's real ~/.pi/mega-compact-vector. They persist, and cross-repo recall is enabled by default and searches the global index with no repo filter (src/recall.ts falls back to a 0.3 cosine floor), so a live session in an unrelated project can be handed a stale test assertion — the contradictory 50k/100k pair included — as if it were another project's memory. Found on a real machine holding 33 such rows and nothing else. scripts/run-tests.mjs already sets MEGACOMPACT_INDEX_DIR per child, so the suite was never the leak; invoking the file directly was. Disables PGlite for the file (the same guard memoryRoundtrip.test.ts uses, for the same reason — these suites exercise the sync node:sqlite path only) and points MEGACOMPACT_INDEX_DIR at the per-run tmp dir so the isolation still holds if a later test re-enables the index. Verified: on the current code a run adds 3 rows to the live index; with this change three consecutive runs add none. The file also gets faster and quieter — 9/9 in <1s instead of 11s, with no "asynchronous activity after the test ended" from an initdb still running past the first test.
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
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.
The leak
applyMemoryOps()mirrors every write into the machine-wide PGlite memory indexvia the fire-and-forget
indexMemoryWrite()→upsertMemoryEmbedding(), keyed byrepoKey(stateDir). For this file's tmp state dirs that key is the tmp pathitself, so running
src/memoryOps.test.tswrites rows into the developer's real~/.pi/mega-compact-vector:They persist, and they are reachable. Cross-repo recall is on by default
(
crossRepoEnabled: true),recallMemoriesCrossRepo()searches the global indexwith no repo filter, and the floor it uses is
opts.crossRepoCosine ?? 0.3—no caller sets it, so the strict
crossRepoCosine: 0.90from config never reachesthis path (it is passed as
dedupSiminextensions/mega-pipeline/recall.ts).So a live session in an unrelated project can be handed a stale test assertion as
if it were another project's memory — including the contradictory 50k/100k pair.
This was found on a real machine whose index held 33 such rows and nothing
else: every entry available to cross-repo recall was test junk.
scripts/run-tests.mjsalready setsMEGACOMPACT_INDEX_DIRper child, so thesuite was never the leak — invoking the file directly was, which is exactly what
one does while debugging.
The change
Disables PGlite for the file — the same guard
memoryRoundtrip.test.tsalreadyuses for the same reason, since these suites exercise the sync
node:sqlitepathonly — and points
MEGACOMPACT_INDEX_DIRat the per-run tmp dir so the isolationstill holds if a later test re-enables the index.
Verification
The file also stops emitting
"asynchronous activity after the test ended"— witha per-run index dir, an
initdbwas still running when the first test finished.Purely test-side; no
src/behavior change.