From 084e2edbb86fc82339a5e230025a42cc54fedcc8 Mon Sep 17 00:00:00 2001 From: Garry Tan Date: Sun, 16 Aug 2026 13:59:33 -0700 Subject: [PATCH] test+docs: codex probe cache invalidation coverage, make-pdf --no-* structural pin, file the review-batch deferrals MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - test/codex-model-probe.test.ts: the 1h TTL and the auth.json half of the mtime signature had no coverage — a regression in either would silently serve a stale MODEL_OK after re-login or forever. Added TTL-expiry (backdated cache line re-probes) and auth.json-mtime invalidation cases, mirroring the existing config.toml case. - make-pdf/test/cli-args.test.ts: structural assertion derived from the commands.ts registry — every --no-* flag must be in BOOLEAN_FLAGS, so a new negation flag can't silently re-open #2514 (swallowing the next positional). - TODOS.md: filed five review-batch deferrals under the v1.67 queue with rationale and effort: setup host-function dedup, cmd.exe %VAR% quoting in gbrainInvocation (cross-spawn direction), make-pdf flag registry metadata (derive BOOLEAN_FLAGS), legacy codex/factory/kiro uninstall provenance gating (parity with the cursor gate), and cursor auto-detect breadth (product call). Co-Authored-By: Claude Fable 5 --- TODOS.md | 28 ++++++++++++++++++++++++++ make-pdf/test/cli-args.test.ts | 16 +++++++++++++++ test/codex-model-probe.test.ts | 36 ++++++++++++++++++++++++++++++++++ 3 files changed, 80 insertions(+) diff --git a/TODOS.md b/TODOS.md index 517b15a70..38f69c9de 100644 --- a/TODOS.md +++ b/TODOS.md @@ -134,6 +134,34 @@ silent regression: extraFields or record as intentional); terse-build's stale "all 4" set (main-side 5th terse-gated resolver). +### P2: v1.67 review-fix-batch deferrals (post-wave review army findings) + +Filed at review-fix-batch time, deferred with rationale: + +- **setup host-function dedup** — four near-verbatim `create_*_runtime_root` + + `link_*_skill_dirs` copies (codex/factory/opencode/cursor) drift + independently (the #2142 ownership gate had to be patched at every site). + Parameterize on host name + skills dir. Effort S with CC. +- **cmd.exe `%VAR%` expansion in gbrainInvocation quoting** — Windows-only, + contrived escalation (requires attacker-controlled env var names), but the + quoting is not cmd.exe-safe. Fix direction: route win32 spawns through + cross-spawn (dependency decision — bun-polyfill.cjs already carries it for + the browse daemon). Effort S. +- **make-pdf flag registry metadata** — commands.ts flags are bare strings; + add a takes-value field and DERIVE cli.ts's BOOLEAN_FLAGS from the + registry (the structural `--no-*` test added in this batch covers only the + negation shape). Effort S. +- **legacy host-glob uninstall provenance gating** — gstack-uninstall's + codex/factory/kiro `gstack*` globs still rm -rf without a provenance + check; bring them to parity with the cursor banner gate added in this + batch (v1.67 added cursor; the legacy three are inherited behavior). + Effort S. +- **cursor auto-detect breadth** — `-d ~/.cursor` triggers a full extra + render + install for every Cursor-having dev on every ./setup (the dir + exists for anyone who ever launched the IDE). Product call on narrowing to + CLI detection (`command -v cursor`) or an opt-in flag. Effort S, needs a + maintainer decision on the detection contract. + ### P2: Persona-fleet hostile-user harness (fork port wave 2 deferral) **What:** Port the methodology behind time-attack/gstack's 87-hostile-user diff --git a/make-pdf/test/cli-args.test.ts b/make-pdf/test/cli-args.test.ts index 3c428f492..2479e7720 100644 --- a/make-pdf/test/cli-args.test.ts +++ b/make-pdf/test/cli-args.test.ts @@ -9,6 +9,7 @@ import { describe, test, expect } from "bun:test"; import * as fs from "fs"; import * as path from "path"; import { parseArgs, BOOLEAN_FLAGS } from "../src/cli"; +import { COMMANDS } from "../src/commands"; // parseArgs slices argv from index 2 (node/bun + script path). const parse = (...args: string[]) => parseArgs(["bun", "cli.ts", ...args]); @@ -75,4 +76,19 @@ describe("#2514 boolean flags do not swallow positionals", () => { expect(b.flags.confidential).toBe(true); expect(b.positional).toEqual(["memo.md"]); }); + + test("structural (T4): every --no-* flag in the commands.ts registry is boolean", () => { + // A --no-* flag is a negation — it never takes a value. A new one added + // to the registry but missed in BOOLEAN_FLAGS silently re-opens #2514 + // (it would swallow the next positional). Derived from the registry, so + // this cannot rot as commands grow. + const missing: string[] = []; + for (const [cmd, spec] of COMMANDS) { + for (const flag of spec.flags ?? []) { + if (!/^--no-/.test(flag)) continue; + if (!BOOLEAN_FLAGS.has(flag.replace(/^--/, ""))) missing.push(`${cmd}: ${flag}`); + } + } + expect(missing).toEqual([]); + }); }); diff --git a/test/codex-model-probe.test.ts b/test/codex-model-probe.test.ts index 381379634..5d4b89981 100644 --- a/test/codex-model-probe.test.ts +++ b/test/codex-model-probe.test.ts @@ -164,6 +164,42 @@ describe('codex model probe (#2477)', () => { } }); + test('TTL expiry: a cached MODEL_OK older than 3600s re-probes (T5)', () => { + const f = makeFixture(); + try { + runProbe(f, 'ok'); + expect(invocations(f)).toBe(1); + // Backdate the cache line's timestamp past the 1h TTL, keeping the + // signature valid — TTL alone must force the re-probe. + const cachePath = path.join(f.gstackHome, '.codex-model-probe'); + const [status, ts, sig] = fs.readFileSync(cachePath, 'utf-8').trim().split(' '); + expect(status).toBe('MODEL_OK'); + fs.writeFileSync(cachePath, `MODEL_OK ${Number(ts) - 3700} ${sig}\n`); + const r = runProbe(f, 'ok'); + expect(r.stdout.trim()).toBe('MODEL_OK'); // not "(cached)" + expect(invocations(f)).toBe(2); // re-probed + } finally { + fs.rmSync(f.home, { recursive: true, force: true }); + } + }); + + test('auth.json mtime change invalidates the cached MODEL_OK (T5: re-login re-probes)', () => { + const f = makeFixture(); + try { + runProbe(f, 'ok'); + expect(invocations(f)).toBe(1); + // A re-login rewrites auth.json; the mtime signature must invalidate + // the cache even though config.toml is untouched. + const future = Date.now() / 1000 + 10; + fs.utimesSync(path.join(f.codexHome, 'auth.json'), future, future); + const r = runProbe(f, 'ok'); + expect(r.stdout.trim()).toBe('MODEL_OK'); + expect(invocations(f)).toBe(2); // re-probed + } finally { + fs.rmSync(f.home, { recursive: true, force: true }); + } + }); + test('config.toml change invalidates the cached MODEL_OK', () => { const f = makeFixture(); try {