From b63ddee2e409b1b2909236cd116e7d04cebc4685 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=B0=8F=E6=A2=A6?= <118527088+DottytheHomeless@users.noreply.github.com> Date: Sat, 12 Sep 2026 12:21:43 +0800 Subject: [PATCH] feat(cli): add --supersedes to --store and surface write-time warnings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The correction rule ("store a corrected version with supersedes, don't rewrite the store") was only reachable through MCP store_memory. Agents wired to the CLI had no way to execute it, so corrections piled up as new rows next to the stale ones they were meant to replace. --supersedes takes comma-separated rowids and fails the whole write if any target is not live (deleted, already superseded, or nonexistent). A partial supersede is worse than none: the write lands, the stale version stays recallable, and the correction looks applied. Also surfaces three guards the CLI was dropping into the log stream only — metaDowngrade, supersedeShrink and quotaRejected. The shrink guard in particular exists to tell the caller what the new version stopped carrying; silently discarding that defeats its purpose. Co-Authored-By: Claude Opus 5 --- cli-supersedes.test.mjs | 97 +++++++++++++++++++++++++++++++++++++++++ index.mjs | 36 ++++++++++++++- 2 files changed, 132 insertions(+), 1 deletion(-) create mode 100644 cli-supersedes.test.mjs diff --git a/cli-supersedes.test.mjs b/cli-supersedes.test.mjs new file mode 100644 index 0000000..a912635 --- /dev/null +++ b/cli-supersedes.test.mjs @@ -0,0 +1,97 @@ +// Regression tests for `--store --supersedes` on the CLI. +// Uses a fresh temp DB via TOKENMEM_DB_PATH; never touches tokenmem.db. +// Run: node cli-supersedes.test.mjs +// +// Why this flag needed a test rather than just a pass-through: a *partial* +// supersede is worse than none. If one of the requested rowids is already +// deleted/superseded/nonexistent, the write still lands and the stale version +// stays live and recallable — the correction looks applied and isn't. So the +// CLI pre-checks the targets and refuses the whole write, rather than letting +// storeMemory silently point at whatever still exists. + +import { spawnSync } from 'node:child_process' +import { mkdtempSync, rmSync } from 'node:fs' +import { tmpdir } from 'node:os' +import { dirname, resolve } from 'node:path' +import { fileURLToPath } from 'node:url' +import Database from 'better-sqlite3' + +const __dirname = dirname(fileURLToPath(import.meta.url)) +const root = mkdtempSync(resolve(tmpdir(), 'mneme-cli-supersedes-')) +const DB_PATH = resolve(root, 'tokenmem.test.db') +const INDEX = resolve(__dirname, 'index.mjs') + +let pass = 0, fail = 0 +function check(label, cond, detail = '') { + if (cond) { pass++; console.log(`✓ ${label}`) } + else { fail++; console.log(`✗ ${label}${detail ? ' -- ' + detail : ''}`) } +} + +function cli(...args) { + return spawnSync(process.execPath, [INDEX, ...args], { + encoding: 'utf-8', + env: { ...process.env, TOKENMEM_DB_PATH: DB_PATH, EMBEDDING_API_KEY: '' }, + }) +} + +function storedId(r) { + const m = /^stored: (\d+)$/m.exec(r.stdout || '') + return m ? m[1] : null +} + +try { + // Seed a record to supersede. + const first = cli('--store', 'first version of the claim', '--category', 'general', '--type', 'working') + const id1 = storedId(first) + check('plain --store still works without --supersedes', id1 !== null, first.stderr) + + // Malformed rowid → refuse before writing. + const bad = cli('--store', 'x', '--supersedes', 'abc') + check('non-numeric target exits 1', bad.status === 1, `status=${bad.status}`) + check('non-numeric target names the bad value', /abc/.test(bad.stderr || ''), bad.stderr) + + // Nonexistent rowid → refuse before writing. + const missing = cli('--store', 'x', '--supersedes', '99999999') + check('nonexistent target exits 1', missing.status === 1, `status=${missing.status}`) + + // A refused call must not have written anything. + const db1 = new Database(DB_PATH, { readonly: true }) + const afterRefusals = db1.prepare('SELECT COUNT(*) c FROM memories').get().c + db1.close() + check('refused calls write nothing', afterRefusals === 1, `rows=${afterRefusals}`) + + // Happy path. + const second = cli('--store', 'second version of the claim', '--category', 'general', '--type', 'working', '--supersedes', id1) + const id2 = storedId(second) + check('supersede stores a new record', id2 !== null, second.stderr) + check('supersede reports which ids it replaced', new RegExp(`superseded: ${id1}`).test(second.stdout || ''), second.stdout) + + const db2 = new Database(DB_PATH, { readonly: true }) + const old = db2.prepare('SELECT superseded_by FROM memories WHERE rowid = ?').get(Number(id1)) + db2.close() + check('old record points at the new one', String(old?.superseded_by) === String(id2), JSON.stringify(old)) + + // Superseding an already-superseded row is a no-op trap: refuse it too. + const again = cli('--store', 'third version', '--supersedes', id1) + check('already-superseded target exits 1', again.status === 1, `status=${again.status}`) + + // Partial batch must fail whole: one live id + one dead id writes nothing. + const third = cli('--store', 'a live record', '--category', 'general', '--type', 'working') + const id3 = storedId(third) + const db3 = new Database(DB_PATH, { readonly: true }) + const before = db3.prepare('SELECT COUNT(*) c FROM memories').get().c + db3.close() + const partial = cli('--store', 'x', '--supersedes', `${id3},99999999`) + const db4 = new Database(DB_PATH, { readonly: true }) + const after = db4.prepare('SELECT COUNT(*) c FROM memories').get().c + const stillLive = db4.prepare('SELECT superseded_by FROM memories WHERE rowid = ?').get(Number(id3)) + db4.close() + check('partly-dead target list exits 1', partial.status === 1, `status=${partial.status}`) + check('partly-dead target list writes nothing', after === before, `${before} → ${after}`) + check('partly-dead target list leaves the live target alone', stillLive?.superseded_by == null, JSON.stringify(stillLive)) + + console.log(`\n${pass} passed, ${fail} failed`) + process.exit(fail === 0 ? 0 : 1) +} finally { + try { rmSync(root, { recursive: true, force: true }) } catch {} +} diff --git a/index.mjs b/index.mjs index 408a62d..e731267 100644 --- a/index.mjs +++ b/index.mjs @@ -4292,6 +4292,26 @@ if (_isMain) { const category = getFlag('--category') || 'general' const memoryType = getFlag('--type') || 'long_term' const memoryLevel = getFlag('--level') || 'semi_abstract' + // Correction path: --supersedes points the old records' superseded_by at + // this new one. Fail loud on ids that are missing/already superseded — + // a partial supersede leaves the stale version live and recallable. + const supersedesRaw = getFlag('--supersedes') + let supersedes = [] + if (supersedesRaw !== null) { + supersedes = supersedesRaw.split(',').map(s => s.trim()).filter(Boolean) + const bad = supersedes.filter(s => !/^\d+$/.test(s)) + if (bad.length) { + process.stderr.write(`Error: --supersedes takes comma-separated rowids, got: ${bad.join(', ')}\n`) + process.exit(1) + } + const live = getMemoriesByIds(supersedes).map(r => String(r.rowid)) + const missing = supersedes.filter(s => !live.includes(s)) + if (missing.length) { + process.stderr.write(`Error: --supersedes target(s) not live (deleted, already superseded, or nonexistent): ${missing.join(', ')}\n`) + process.exit(1) + } + } + const out = {} const id = storeMemory({ content: content.trim(), memoryType, @@ -4300,8 +4320,21 @@ if (_isMain) { importance, source: 'manual', tags: ['cli', 'manual'], - }) + ...(supersedes.length ? { supersedes } : {}), + }, { out }) process.stdout.write(`stored: ${id}\n`) + if (id && supersedes.length) process.stdout.write(`superseded: ${supersedes.join(', ')}\n`) + // Surface the write-time guards the CLI used to swallow into the log stream. + if (out.metaDowngrade) { + process.stdout.write(`⚠ level ${out.metaDowngrade.fromLevel} → ${out.metaDowngrade.toLevel}: ${out.metaDowngrade.reasons.join(' | ')}\n`) + } + if (out.supersedeShrink?.length) { + process.stdout.write(`⚠ supersede shrink — the new version stopped carrying:\n`) + for (const s of out.supersedeShrink) process.stdout.write(` - ${typeof s === 'string' ? s : JSON.stringify(s)}\n`) + } + if (out.quotaRejected?.length) { + process.stdout.write(`⚠ quota rejected: ${out.quotaRejected.join(', ')}\n`) + } } else if (getFlag('--store-compact-summary') !== null) { const summary = process.env.TOKENMEM_COMPACT_SUMMARY @@ -4406,6 +4439,7 @@ if (_isMain) { ' [--importance 1-10] [--category general|people|project|...]', ' [--type working|short_term|long_term|permanent]', ' [--level concrete_trace|semi_abstract|meta_knowledge] abstraction level (default semi_abstract)', + ' [--supersedes 123,456] replace those live rowids with this record (fails if any is not live)', ' node index.mjs --compress Compress old conversations (requires claude CLI)', ' [--days 30]', ' node index.mjs --compress-all Batch compress all old conversations',