From 59ba06d38b29085f570f9e48463135d30b51d66b Mon Sep 17 00:00:00 2001 From: KT <677465+kevintseng@users.noreply.github.com> Date: Tue, 18 Aug 2026 02:32:46 +0800 Subject: [PATCH 1/2] test(reindex): close two of the three remaining review gaps, and record why the third is not closed MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two closed: - The crash-injection test re-read the rolled-back index on the SAME connection that ran the failed swap. db.ts's own header sets the stronger bar — "a FRESH connection still read the original table" — because vec0 keeps four shadow tables and per-connection module state, so the handle that failed is the one most likely to answer from memory a reopen would not reproduce. It now closes the database, reopens, and asserts row count and vector width from the new handle. - check-doc-claims gained the doc-to-code direction for CLI options. A sibling test already scans SOURCE files so no message can recommend a flag the CLI rejects (it caught two real ones the day it was added), but nothing looked the other way — an option table left listing a removed flag passed silently, which is what happened to --vectors. 24 documented flags now resolve. --discard-generation is added to the reindex option table, so the gate has the new flag under it too. One NOT closed, with the dead end recorded in the test file so the next attempt does not repeat it: the CLI verdict (the incomplete banner and exit code 1) has been unguarded since the pre-flight probe was added, because the existing test stops AT the probe and never calls reindex(). Reaching the loop offline needs a provider that answers the probe at the configured width and the corpus at the wrong one. An in-process http.createServer CANNOT do it — run() uses execFileSync, which blocks this process's event loop, so the stub never answers the child and every request times out (measured: "Ollama embedding request timed out"). A working version has to spawn the stub as its own process. Verification, this session: node scripts/run-tests-isolated.mjs exit=0 Test Files 154 passed (154) / Tests 2243 passed (2243); no "Errors" line npm run typecheck exit=0 node scripts/check-doc-claims.mjs exit=0 24 documented CLI flags all resolve to registered options --- docs/api/API_REFERENCE.md | 1 + scripts/check-doc-claims.mjs | 36 +++++++++++++++++++++++++ tests/cli-reindex-vectors-guard.test.ts | 17 +++++++++++- tests/vector-index-safety.test.ts | 22 +++++++++++++++ 4 files changed, 75 insertions(+), 1 deletion(-) diff --git a/docs/api/API_REFERENCE.md b/docs/api/API_REFERENCE.md index 85d4ccf6..ebf57108 100644 --- a/docs/api/API_REFERENCE.md +++ b/docs/api/API_REFERENCE.md @@ -1133,6 +1133,7 @@ Regenerate vector embeddings for all entities. |--------|-------------| | `--namespace ` | Reindex only entities in this namespace. | | `--fts` | Rebuild the full-text keyword index instead of the vector index. | +| `--discard-generation` | Throw away a half-built index left by an interrupted rebuild, without rebuilding. Never touches the live index. | | `--json` | Output the result as JSON. | `--fts` rebuilds the full-text keyword index instead. The keyword index diff --git a/scripts/check-doc-claims.mjs b/scripts/check-doc-claims.mjs index 114b3047..0e685071 100644 --- a/scripts/check-doc-claims.mjs +++ b/scripts/check-doc-claims.mjs @@ -561,6 +561,42 @@ if (!hasBearerAuth) { else if (badCommands.length) fail(`agent docs name CLI subcommands that do not exist:\n ${badCommands.join('\n ')}`); else if (agentDocs.length) ok(`${mentions} \`memesh \` mentions in agent docs all resolve to registered CLI commands`); + // (a2) every CLI flag an option table documents is a flag cli.ts registers. + // + // The missing direction. A sibling test already scans SOURCE files so no + // stderr message can recommend a flag the CLI would reject — it caught two + // real ones the day it was added. Nothing looked the other way, so an option + // table left listing a removed flag passed this gate silently, which is + // exactly what happened to `--vectors`: retired from the parser while three + // documents and two runtime messages still told people to run it. + // + // Doc → code only. The reverse would flag every deliberately undocumented + // flag, which is a different decision and not one a gate should make. + const registeredFlags = new Set( + [...cliSrc.matchAll(/\.option\(\s*'(--[a-z][a-z0-9-]*)/g)].map(m => m[1]), + ); + if (registeredFlags.size < 10) { + fail(`CLI option extraction matched only ${registeredFlags.size} — the pattern stopped matching cli.ts`); + } + const documentedFlags = new Map(); // flag → first doc:line that names it + for (const doc of ['docs/api/API_REFERENCE.md']) { + read(doc).split('\n').forEach((line, i) => { + const m = line.match(/^\|\s*`(--[a-z][a-z0-9-]*)/); + if (m && !documentedFlags.has(m[1])) documentedFlags.set(m[1], `${doc}:${i + 1}`); + }); + } + if (documentedFlags.size < 10) { + fail(`option-table extraction matched only ${documentedFlags.size} flags — the table format changed`); + } + const ghostFlags = [...documentedFlags] + .filter(([flag]) => !registeredFlags.has(flag)) + .map(([flag, where]) => `${where} → \`${flag}\``); + if (ghostFlags.length) { + fail(`docs document CLI flags that cli.ts does not register:\n ${ghostFlags.join('\n ')}`); + } else { + ok(`${documentedFlags.size} documented CLI flags all resolve to registered options`); + } + // (b) any "`x` tool" phrase names a registered MCP tool. const badTools = []; for (const doc of agentDocs) { diff --git a/tests/cli-reindex-vectors-guard.test.ts b/tests/cli-reindex-vectors-guard.test.ts index 12a4987a..49426596 100644 --- a/tests/cli-reindex-vectors-guard.test.ts +++ b/tests/cli-reindex-vectors-guard.test.ts @@ -49,12 +49,16 @@ describe('memesh reindex refuses before it destroys anything', () => { fs.rmSync(home, { recursive: true, force: true, maxRetries: 5, retryDelay: 100 }); }); - function run(args: string[]): { status: number; stderr: string; stdout: string } { + function run( + args: string[], + extraEnv: NodeJS.ProcessEnv = {}, + ): { status: number; stderr: string; stdout: string } { const env: NodeJS.ProcessEnv = { ...process.env, HOME: home, MEMESH_DIR: path.join(home, '.memesh'), MEMESH_DB_PATH: dbPath, + ...extraEnv, }; // A real key in the developer's shell would send these test entities to // OpenAI and make the offline cases below depend on the network. @@ -199,6 +203,17 @@ describe('memesh reindex refuses before it destroys anything', () => { expect(vectorCount()).toBe(1); }); + // NOT PINNED, and here is why, so the next attempt does not repeat the dead end: + // the CLI verdict (the incomplete banner + `process.exitCode = 1`) has been + // unguarded since the pre-flight probe was added — the test above stops AT the + // probe and never calls `reindex()`. Reaching the loop without a network needs a + // provider that answers the probe at the configured width and the corpus at the + // wrong one. An in-process `http.createServer` CANNOT do that: `run()` uses + // `execFileSync`, which blocks this process’s event loop, so the stub never + // answers the child and every request times out (measured: "Ollama embedding + // request timed out"). A working version has to spawn the stub as its OWN + // process — a small script plus `spawn` — before invoking the CLI. + it('--discard-generation reclaims a half-built index without touching the live one', () => { // The deliberate way out. Two situations need it: a rebuild the user has // abandoned (the staging index otherwise sits on disk indefinitely and diff --git a/tests/vector-index-safety.test.ts b/tests/vector-index-safety.test.ts index 616dc710..66a49a1c 100644 --- a/tests/vector-index-safety.test.ts +++ b/tests/vector-index-safety.test.ts @@ -242,6 +242,28 @@ describe('Feature: an unreadable config does not delete embeddings', () => { expect(row.embedding.length / 4, 'the swap published a half-built index').toBe(1536); expect(storedDimension()).toBe(1536); expect(generationRowIds().has(1), 'the staged work was lost to a failed swap').toBe(true); + + // Re-read from a FRESH connection, which is the bar db.ts's own header sets + // ("a FRESH connection still read the original table") and the weaker + // same-connection read cannot meet. vec0 keeps four shadow tables and + // per-connection module state, so the handle that ran the failed swap is + // exactly the one most likely to answer from memory a reopen would not + // reproduce. A rollback that only holds inside the connection that failed is + // not a rollback. + closeDatabase(); + const fresh = openDatabase(dbPath); + const reread = fresh.prepare('SELECT embedding FROM entities_vec WHERE rowid = 1') + .get() as { embedding: Uint8Array } | undefined; + expect(reread, 'the previous index is gone when read from a new connection').toBeDefined(); + expect( + reread!.embedding.length / 4, + 'the rollback only held inside the connection that failed', + ).toBe(1536); + expect( + (fresh.prepare('SELECT count(*) AS c FROM entities_vec').get() as { c: number }).c, + 'the row count did not survive a reopen', + ).toBe(1); + expect(generationRowIds().has(1), 'the staged work did not survive a reopen').toBe(true); }); it('a completed swap promotes the generation and clears its marker', () => { From fe5defef75cdb53109f57aae20890185ef784529 Mon Sep 17 00:00:00 2001 From: KT <677465+kevintseng@users.noreply.github.com> Date: Tue, 18 Aug 2026 02:59:51 +0800 Subject: [PATCH 2/2] =?UTF-8?q?test(reindex):=20pin=20the=20CLI=20incomple?= =?UTF-8?q?te-verdict=20contract=20=E2=80=94=20the=20third=20gap,=20closed?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Supersedes the previous commit's claim that this one could not be closed. It could; the obstacle was real but it was not a dead end, and recording it instead of solving it was the wrong call. The verdict a shell script actually reads — the incomplete banner and `process.exitCode = 1` — has been unguarded since the pre-flight probe was added: the existing test stops AT the probe, exits before `withDatabase`, and never calls `reindex()`. Nothing else spawns this CLI. Reaching the loop offline needs a provider that answers twice, differently: at the configured width for the single probe string, and at the wrong width for the corpus. An in-process `http.createServer` cannot do it, and that part of the earlier note was correct — `run()` uses `execFileSync`, which blocks this process's event loop, so the stub never answers the child and every request times out. The answer is simply that the stub does not belong in this process: it is written to the test's tmpdir and `spawn`ed, so it has its own event loop and keeps serving while this one is blocked. Two anti-vacuity assertions guard the test itself, because the failure mode here is a test that passes by never reaching the code: it asserts the probe-refusal message is ABSENT and that "Reindexing" is present, so a run that stopped at the probe fails rather than quietly proving nothing. Verification, this session: node scripts/run-tests-isolated.mjs tests/cli-reindex-vectors-guard.test.ts exit=0 Tests 7 passed (7) npm run typecheck exit=0 Break-test: `if (incomplete) process.exitCode = 1` -> `if (false && ...)`, dist rebuilt (this suite spawns dist/, so a source-only mutation is invisible to it): mutant run exit=1 -> KILLED cli.ts restored byte-identical, dist rebuilt --- tests/cli-reindex-vectors-guard.test.ts | 103 +++++++++++++++++++++++- 1 file changed, 102 insertions(+), 1 deletion(-) diff --git a/tests/cli-reindex-vectors-guard.test.ts b/tests/cli-reindex-vectors-guard.test.ts index 49426596..3637d0b9 100644 --- a/tests/cli-reindex-vectors-guard.test.ts +++ b/tests/cli-reindex-vectors-guard.test.ts @@ -203,7 +203,108 @@ describe('memesh reindex refuses before it destroys anything', () => { expect(vectorCount()).toBe(1); }); - // NOT PINNED, and here is why, so the next attempt does not repeat the dead end: + it('an incomplete rebuild prints no tick and exits 1 — reached through reindex(), not the probe', async () => { + // The test above stops AT the pre-flight probe: it exits before + // `withDatabase` and never calls `reindex()`. So the verdict rendering — + // the incomplete banner and `process.exitCode = 1` — has been unguarded + // since that probe was added, and nothing else spawns this CLI. + // + // Reaching the loop offline needs a provider that answers TWICE, + // differently: at the configured width for the one probe string, and at the + // wrong width for the corpus. An in-process `http.createServer` CANNOT do + // it — `run()` uses `execFileSync`, which blocks this process's event loop, + // so the stub never answers the child and every request times out + // (measured: "Ollama embedding request timed out"). The stub therefore runs + // as its OWN process, which has its own event loop and keeps serving while + // this one is blocked. + const stubPath = path.join(home, 'embed-stub.mjs'); + fs.writeFileSync(stubPath, ` +import http from 'node:http'; +const PROBE = 'memesh vector index rebuild probe'; +const server = http.createServer((req, res) => { + let body = ''; + req.on('data', (c) => { body += c; }); + req.on('end', () => { + let input = ''; + try { input = String(JSON.parse(body || '{}').input ?? ''); } catch {} + // Right width for the probe so the pre-flight passes; wrong width for every + // real entity so the corpus fails and the run is genuinely incomplete. + const n = input === PROBE ? 768 : 8; + res.setHeader('content-type', 'application/json'); + res.end(JSON.stringify({ embeddings: [Array.from({ length: n }, () => 0.1)] })); + }); +}); +server.listen(0, '127.0.0.1', () => { + process.stdout.write('PORT=' + server.address().port + '\\n'); +}); +`); + + const { spawn } = await import('node:child_process'); + const stub = spawn(process.execPath, [stubPath], { stdio: ['ignore', 'pipe', 'pipe'] }); + const port: number = await new Promise((resolve, reject) => { + const timer = setTimeout(() => reject(new Error('stub server never reported a port')), 10_000); + stub.stdout.on('data', (d: Buffer) => { + const m = /PORT=(\d+)/.exec(String(d)); + if (m) { clearTimeout(timer); resolve(Number(m[1])); } + }); + stub.on('error', reject); + }); + + try { + fs.writeFileSync( + path.join(home, '.memesh', 'config.json'), + JSON.stringify({ embedder: { provider: 'ollama' } }), + ); + // Seed with the provider UNREACHABLE so `remember`'s own embed writes + // nothing and the stale vector can be inserted by hand. (`INSERT OR + // REPLACE` does not work on a vec0 table — see embedder.ts — so the row + // must not already exist, and it would if the stub were reachable here.) + const seeded = run( + ['remember', '--name', 'stale-note', '--type', 'note', '--obs', 'a memory worth keeping'], + { OLLAMA_HOST: 'http://127.0.0.1:1' }, + ); + expect(seeded.status, `setup: remember failed — ${seeded.stderr}`).toBe(0); + + const sqliteVec = require('sqlite-vec'); + const seedDb = new Database(dbPath, { allowExtension: true }); + seedDb.enableLoadExtension(true); + try { sqliteVec.load(seedDb); } finally { seedDb.enableLoadExtension(false); } + const seedId = (seedDb.prepare("SELECT id FROM entities WHERE name = 'stale-note'") + .get() as { id: number }).id; + const seedDim = parseInt( + (seedDb.prepare("SELECT value FROM memesh_metadata WHERE key = 'embedding_dimension'") + .get() as { value: string }).value, + 10, + ); + seedDb.prepare('INSERT INTO entities_vec (rowid, embedding) VALUES (?, ?)').run( + BigInt(seedId), + Buffer.from(new Float32Array(seedDim).fill(0.25).buffer) as unknown as SqlInputValue, + ); + seedDb.close(); + expect(vectorCount(), 'setup: a stale vector is on disk').toBe(1); + + const result = run(['reindex'], { OLLAMA_HOST: `http://127.0.0.1:${port}` }); + + // Anti-vacuity: prove the probe was PASSED and the loop actually ran. + // Without these two, the test would be satisfied by the probe refusing — + // which is the other test, and the exact way this one used to prove + // nothing. + expect( + result.stderr, + 'the run never got past the pre-flight probe, so it did not test the verdict', + ).not.toContain('nothing was rebuilt'); + expect(result.stderr, 'the reindex loop never started').toContain('Reindexing'); + + expect(result.stdout, 'a tick over a run that embedded nothing').not.toContain('✅'); + expect(result.stdout, 'the incomplete verdict is not rendered').toContain('Reindex incomplete'); + expect(result.status, 'an incomplete run exited 0 — `memesh reindex && deploy` would proceed').toBe(1); + expect(vectorCount(), 'a failed rebuild published into the live index').toBe(1); + } finally { + stub.kill(); + } + }); + + // Kept for the record: // the CLI verdict (the incomplete banner + `process.exitCode = 1`) has been // unguarded since the pre-flight probe was added — the test above stops AT the // probe and never calls `reindex()`. Reaching the loop without a network needs a