From cb3c94d9e245617834fff1e19a138d4a3f0281a9 Mon Sep 17 00:00:00 2001 From: Jason Viloria Date: Mon, 19 Oct 2026 16:55:16 +0300 Subject: [PATCH 1/3] =?UTF-8?q?fix:=20the=20capture-fallback=20hint=20adve?= =?UTF-8?q?rtised=20--tags,=20which=20`add`=20rejects=20=E2=80=94=20it=20n?= =?UTF-8?q?ow=20says=20--tag?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The injected capture-fallback line (ADR-0065 §1) is what an agent pastes when the kb_* tools are gone. It advertised `add "…" [--tags a,b]`, and the CLI rejects that with "unknown flag --tags", the very trap args.ts names (four entries once landed untagged). So the fallback failed at the moment it was needed. Found by the #354 review. - src/engine.ts: [--tags a,b] -> [--tag a,b]. - test/capture-fallback.test.ts: a behavioural guard, not a source grep. It pulls every [--flag value] out of the rendered line and runs the real built CLI `add` with them against a temp brain, then asserts the entry landed with the tags. Observed RED before the fix ("unknown flag --tags"), green after. Suite 816/816. --- src/engine.ts | 2 +- test/capture-fallback.test.ts | 25 +++++++++++++++++++++++-- 2 files changed, 24 insertions(+), 3 deletions(-) diff --git a/src/engine.ts b/src/engine.ts index 56a8e833..e53b6b5f 100644 --- a/src/engine.ts +++ b/src/engine.ts @@ -537,7 +537,7 @@ export function renderCaptureFallback(engineDir = dirname(fileURLToPath(import.m return ( `capture fallback: if the kb_* tools error or disappear, the CLI still writes ` + `(same engine, separate process; journaled per ADR-0064) —\n` + - ` VFKB_DATA_DIR="${resolvePath(brainDir())}" ${target} add "…" [--tags a,b] [--why "…"]` + ` VFKB_DATA_DIR="${resolvePath(brainDir())}" ${target} add "…" [--tag a,b] [--why "…"]` ); } diff --git a/test/capture-fallback.test.ts b/test/capture-fallback.test.ts index bbeec187..275e5ad6 100644 --- a/test/capture-fallback.test.ts +++ b/test/capture-fallback.test.ts @@ -7,9 +7,10 @@ // runnable, and survive budget pressure — otherwise it is documentation that // vanishes exactly when it is needed. import { describe, it, expect, beforeEach, afterEach } from 'vitest'; -import { mkdtempSync, rmSync, writeFileSync, mkdirSync } from 'node:fs'; +import { mkdtempSync, rmSync, writeFileSync, mkdirSync, readFileSync } from 'node:fs'; +import { execFileSync } from 'node:child_process'; import { tmpdir } from 'node:os'; -import { join, basename } from 'node:path'; +import { join, basename, resolve } from 'node:path'; import { renderCaptureFallback, renderContextBundle, resolveCliFace, SESSION_BUDGET_CHARS } from '../src/engine.js'; let brain: string; @@ -51,6 +52,26 @@ describe('ADR-0065 §1 — the capture-fallback line', () => { expect(line).toMatch(/node .+ add /); }); + it('every optional flag it advertises is one the real CLI `add` accepts', () => { + // The hint is pasted by an agent mid-loss, so a flag the CLI rejects defeats it. + // It once advertised `--tags`, which `add` rejects with "unknown flag --tags" + // (args.ts names it the observed trap). This drives the real built CLI with + // every [--flag value] the line advertises; it does not grep the source. + const d = join(brain, 'r'); + mkdirSync(d, { recursive: true }); + writeFileSync(join(d, 'cli.js'), ''); + const line = renderCaptureFallback(d); + const flags = [...line.matchAll(/\[(--[a-z-]+) ([^\]]+)\]/g)].map((m) => m[1]); + expect(flags.length).toBeGreaterThan(0); + const cli = resolve(__dirname, '../dist/cli.js'); + const args = ['add', 'fact', 'fallback hint probe']; + for (const f of flags) args.push(f, f === '--tag' || f === '--tags' ? 'probe,hint' : 'because'); + execFileSync('node', [cli, ...args], { env: { ...process.env, VFKB_DATA_DIR: brain }, encoding: 'utf8', stdio: 'pipe' }); + const entry = readFileSync(join(brain, 'entries.jsonl'), 'utf8').trim().split('\n').map((l) => JSON.parse(l)).pop(); + expect(entry.text).toContain('fallback hint probe'); + if (flags.includes('--tag')) expect(entry.tags).toEqual(['probe', 'hint']); + }); + it('QUOTES both paths — an unquoted path truncates at the first space', () => { // Plugin cache paths carry a version segment and may contain spaces. An // unquoted command would silently run against the wrong path: precisely the From aefb5700713bdcd9b9cd44a8b305905b78ace4ed Mon Sep 17 00:00:00 2001 From: Jason Viloria Date: Mon, 19 Oct 2026 17:44:41 +0300 Subject: [PATCH 2/3] test: the fallback-hint guard checks every --flag token, not only [--lower value] (review round 1 M1) The gate on #355 found that the guard extracted only bracketed lower-case flags. The same --tags bug written unbracketed, a capitalised --Tag, or a bare --bogus all stayed green. Every --flag token after `add` is now extracted. Each of those three mutants, plus the original [--tags a,b], was observed red; green after restore. --- test/capture-fallback.test.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/test/capture-fallback.test.ts b/test/capture-fallback.test.ts index 275e5ad6..c48830b8 100644 --- a/test/capture-fallback.test.ts +++ b/test/capture-fallback.test.ts @@ -61,7 +61,10 @@ describe('ADR-0065 §1 — the capture-fallback line', () => { mkdirSync(d, { recursive: true }); writeFileSync(join(d, 'cli.js'), ''); const line = renderCaptureFallback(d); - const flags = [...line.matchAll(/\[(--[a-z-]+) ([^\]]+)\]/g)].map((m) => m[1]); + // EVERY --flag token after `add`, bracketed or not and whatever its case, so the + // same bug written another way (`--tags a,b` without brackets, `--Tag`) is caught too. + const cmd = line.slice(line.indexOf(' add ')); + const flags = [...new Set([...cmd.matchAll(/(--[A-Za-z][A-Za-z-]*)/g)].map((m) => m[1]))]; expect(flags.length).toBeGreaterThan(0); const cli = resolve(__dirname, '../dist/cli.js'); const args = ['add', 'fact', 'fallback hint probe']; From f77ecc4bafa9d86fba66eb0728117eebeb5e2d60 Mon Sep 17 00:00:00 2001 From: Jason Viloria Date: Mon, 19 Oct 2026 17:47:38 +0300 Subject: [PATCH 3/3] =?UTF-8?q?review:=20ADR-0052=20gate=20record=20for=20?= =?UTF-8?q?the=20fallback-hint=20fix=20=E2=80=94=20MERGE,=202=20rounds?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- ...b5700713bdcd9b9cd44a8b305905b78ace4ed.json | 83 +++++++++++++++++++ 1 file changed, 83 insertions(+) create mode 100644 reviews/aefb5700713bdcd9b9cd44a8b305905b78ace4ed.json diff --git a/reviews/aefb5700713bdcd9b9cd44a8b305905b78ace4ed.json b/reviews/aefb5700713bdcd9b9cd44a8b305905b78ace4ed.json new file mode 100644 index 00000000..a9f38cc7 --- /dev/null +++ b/reviews/aefb5700713bdcd9b9cd44a8b305905b78ace4ed.json @@ -0,0 +1,83 @@ +{ + "recordVersion": 1, + "sha": "aefb5700713bdcd9b9cd44a8b305905b78ace4ed", + "governing": [ + "docs/adr/ADR-0065-write-health-loudness.md" + ], + "reviewer": { + "agent": "general-purpose", + "model": "opus", + "rounds": 2 + }, + "generated": "2026-10-19T14:47:38.212Z", + "findings": [ + { + "id": "F1", + "severity": "minor", + "status": "fixed", + "summary": "guard extracted only [--lower value] flags: unbracketed --tags, --Why, bare --bogus, unbracketed --zzz stayed green; now every --flag token after `add` is extracted", + "acceptedBy": null + }, + { + "id": "F2", + "severity": "minor", + "status": "open", + "summary": "guard feeds its own values, not the advertised ones, so a bad value form ([--tag a b]) passes; carried, non-blocking", + "acceptedBy": null + }, + { + "id": "F3", + "severity": "minor", + "status": "open", + "summary": "slice starts at the first \" add \": a TMPDIR containing \" add --x\" false-reds; default macOS/CI temp paths do not; non-blocking", + "acceptedBy": null + } + ], + "findingsCount": 3, + "ruledOut": [ + "source-text guard: drives dist/cli.js via the real `add`", + "--status/--valid-until/--constitutional with a dummy value: accepted, no false red", + "other agent-facing --tags in src/docs/skills: none (args.ts trap comment, RFC-033 history, rejection tests only)", + "AI attribution / secrets: none", + "stale dist: npm test runs tsc in pretest; CI builds" + ], + "mutations": [ + { + "guard": "test/capture-fallback.test.ts > every optional flag it advertises is one the real CLI `add` accepts", + "mutation": "hint reverted to [--tags a,b]", + "observedRed": true + }, + { + "guard": "test/capture-fallback.test.ts > every optional flag it advertises is one the real CLI `add` accepts", + "mutation": "unbracketed --tags a,b", + "observedRed": true + }, + { + "guard": "test/capture-fallback.test.ts > every optional flag it advertises is one the real CLI `add` accepts", + "mutation": "[--Why \"…\"]", + "observedRed": true + }, + { + "guard": "test/capture-fallback.test.ts > every optional flag it advertises is one the real CLI `add` accepts", + "mutation": "bare [--bogus]", + "observedRed": true + }, + { + "guard": "test/capture-fallback.test.ts > every optional flag it advertises is one the real CLI `add` accepts", + "mutation": "unbracketed --zzz x", + "observedRed": true + }, + { + "guard": "test/capture-fallback.test.ts > every optional flag it advertises is one the real CLI `add` accepts", + "mutation": "invented [--foo x]", + "observedRed": true + }, + { + "guard": "test/capture-fallback.test.ts > every optional flag it advertises is one the real CLI `add` accepts", + "mutation": "all bracketed flags removed", + "observedRed": true + } + ], + "mutationsNote": null, + "verdict": "MERGE" +}