/** * @fileoverview Static guard: no code outside the stock catalog branches on a CLI's ID. * * The whole point of the registry is that behaviour which differs between CLIs is DATA (a * `CliEntry` field) or a NAMED PROFILE selected by a field — never `mode === 'codex'`. A * single reintroduced id-check is how the old shape grows back, one "just this once" at a * time, until adding a CLI means editing forty files again. * * This guard was cited by name in three separate file headers of an earlier attempt at this * refactor and never actually written — and in its absence four id-branches survived that * migration, one of them dead code sitting directly under the generic check that replaced it. * So the guard is not decoration: it is the thing that makes the rule true rather than * aspirational. * * ## What is allowlisted, and why an allowlist rather than zero * * Some branches are not CLI-behaviour branches at all, and forcing them through a capability * would make the code worse, not better. Each entry below carries its reason. The categories: * * - **Legacy `Config` plumbing.** `POST /api/sessions` has carried named per-CLI * config objects since before the registry, and `docs/versioning-policy.md` makes that * wire shape public. Selecting `codexConfig` for codex is a fact about the HTTP API, not * about codex, and the `Session` constructor mirrors it. The registry already owns the * translation (`launch.legacyConfigField`); collapsing the constructor too is a public-API * change and belongs in its own PR. * - **Claude's remote/docker command construction.** Claude's pane command varies with the * session's permission mode and its docker form is `--session-id … || resume`, semantics * no other CLI has and a static `overlays.command` string cannot express. * - **Genuinely per-CLI prose.** One error message that explains why a deepseek session in * particular will never deliver a `stop` signal. * * ⚠️ Adding an entry here is a decision, not a formality. If the branch is about what a CLI * CAN DO, it belongs in `CliCapabilities` instead — and if it needs to run code, in * `config/cli-registry/profiles.ts` as a named profile. * * ⚠️ Each entry also carries how many times its expression occurs in that file, compared * EXACTLY. The key is only `::`, so without a count an approved branch * approved every later copy of the same text in the same file: the codex launch defaults * added two more `mode === 'codex'` branches to session-routes.ts and passed silently, * because the legacy-plumbing entry already covered that string. A new copy now fails as * an unapproved branch would, and a removed one fails as stale until the count drops. * * Port: none (pure static analysis). */ import { describe, it, expect } from 'vitest'; import { readdirSync, readFileSync, statSync } from 'node:fs'; import { join, relative, sep } from 'node:path'; import { fileURLToPath } from 'node:url'; import { STOCK_CLIS } from '../src/config/cli-registry/stock.js'; const SRC = fileURLToPath(new URL('../src', import.meta.url)); /** * Files exempt from the scan entirely, because naming CLI ids IS their job. * * `stock.ts` is the catalog. The per-CLI resolver modules are each ABOUT one CLI and look up * their own entry by id — the same reason the catalog may, and the reason they are not a * loophole: they resolve a binary, they decide no behaviour. */ const EXEMPT_FILES = new Set( [ 'config/cli-registry/stock.ts', 'utils/claude-cli-resolver.ts', 'utils/opencode-cli-resolver.ts', 'utils/codex-cli-resolver.ts', 'utils/gemini-cli-resolver.ts', 'utils/antigravity-cli-resolver.ts', 'utils/pi-cli-resolver.ts', 'utils/grok-cli-resolver.ts', 'utils/deepseek-cli-resolver.ts', // Names the deepseek launcher profile's implementation; keyed by profile, not by id. 'utils/cli-launcher.ts', ].map((p) => p.split('/').join(sep)) ); /** One approved branch: how many copies of it the file holds, and why it is not a capability. */ interface Allowance { count: number; reason: string; } const allow = (count: number, reason: string): Allowance => ({ count, reason }); /** * Specific surviving branches, each with the reason it is not a capability and the exact * number of times the expression occurs in that file (see the header). * Keyed `::`. */ const ALLOWED_BRANCHES: Record = { // --- Legacy Config plumbing (public wire shape, see the header) --- "web/routes/session-routes.ts::mode === 'opencode'": allow(2, 'legacy Config plumbing'), "web/routes/session-routes.ts::mode === 'codex'": allow(2, 'legacy Config plumbing'), "web/routes/session-routes.ts::mode === 'gemini'": allow(2, 'legacy Config plumbing'), "web/routes/session-routes.ts::mode === 'antigravity'": allow(2, 'legacy Config plumbing'), "web/routes/session-routes.ts::mode === 'pi'": allow(2, 'legacy Config plumbing'), "web/routes/session-routes.ts::mode === 'grok'": allow(2, 'legacy Config plumbing'), "web/routes/session-routes.ts::mode === 'deepseek'": allow(2, 'legacy Config plumbing'), "web/server.ts::mode === 'opencode'": allow(1, 'legacy Config plumbing (session recovery)'), "web/server.ts::mode === 'codex'": allow(1, 'legacy Config plumbing (session recovery)'), "web/server.ts::mode === 'gemini'": allow(1, 'legacy Config plumbing (session recovery)'), "web/server.ts::mode === 'antigravity'": allow(1, 'legacy Config plumbing (session recovery)'), "web/server.ts::mode === 'pi'": allow(1, 'legacy Config plumbing (session recovery)'), "web/server.ts::mode === 'grok'": allow(1, 'legacy Config plumbing (session recovery)'), "web/server.ts::mode === 'deepseek'": allow(1, 'legacy Config plumbing (session recovery)'), "web/server.ts::mode === 'omp'": allow(1, 'legacy Config plumbing (session recovery)'), // --- Claude's remote/docker command construction --- "tmux-manager.ts::mode === 'claude'": allow( 2, "claude's remote pane command carries per-session permission flags, and its docker form is " + '`--session-id … || resume`; neither fits a static overlays.command string' ), "tmux-manager.ts::mode === 'omp'": allow( 1, 'remote omp respawn needs the pinned/continue --resume override threaded through ' + '(resumeSessionId/ompConfig), which the static overlays.remote.command string has no ' + 'room for; the command itself is still rendered through buildSpawnCommandFromRegistry, ' + 'the same mode-agnostic engine local/docker spawns use — only the BRANCH is per-mode' ), // --- Per-CLI prose and launch handling not yet generalised --- "web/session-wait-registry.ts::mode === 'deepseek'": allow( 1, 'an error message explaining why THIS mode in particular will never deliver a stop signal' ), "web/routes/approval-routes.ts::mode === 'deepseek'": allow( 1, 'the DeepSeek status bridge is the only non-claude source of approval items' ), "cron/cron-service.ts::mode === 'claude'": allow(1, 'cron launch handling, not yet generalised'), "cron/cron-service.ts::mode === 'shell'": allow(1, 'cron launch handling, not yet generalised'), "web/routes/session-routes.ts::mode === 'claude'": allow( 2, 'docker case bookkeeping keyed on the claude conversation id' ), "cli.ts::mode === 'shell'": allow(1, 'a CLI-table label, not behaviour'), // --- Negated forms surfaced when BRANCH_PATTERN widened past `===` (see its comment) --- // // None of these is a regression: every one predates the registry and survived the // conversion only because the guard could not see `!==`. They are listed here with reasons // rather than silently converted, because each would change behaviour or invent a // capability field, and this change is meant to change nothing a user can see. // Read My Mind + intent capture read CLAUDE's OWN transcript, so `mode === 'claude'` is // the right question and `hooksAvailableForMode()` is NOT — once `deepseek` earned a yes // there, the shared predicate silently widened both to a mode with no transcript to read. // CLAUDE.md documents this as deliberate and `test/deepseek-mode.test.ts` pins it, so a // capability here would be actively wrong. "web/routes/readmymind-routes.ts::mode !== 'claude'": allow( 1, 'deliberately mode-not-capability; pinned by deepseek-mode.test.ts' ), "web/server.ts::mode !== 'claude'": allow( 2, "intent capture reads Claude's own transcript, and the recovered-workspace hook sweep " + 'writes .claude hooks — both are claude questions, not capability ones (see CLAUDE.md)' ), // The TUI is a CLIENT of the server, and these two are about what it can offer for a row: // resume builds a `claude --resume`, and the mode badge is suppressed for the default mode // purely so the common case reads clean. The badge one is cosmetic and not a capability at // all; the resume one would need a "resumable from a claude transcript" field that nothing // else would read. "tui/tui-app.ts::mode !== 'claude'": allow( 1, 'TUI resume builds a claude --resume; claude-transcript-only by construction' ), "tui/tui-render.ts::mode !== 'claude'": allow(1, 'cosmetic: suppress the mode badge for the default mode'), // Push approve/deny BUTTONS are withheld for dsh because the answer route refuses // keystrokes for its dialogs (third-party TUI, unmeasured contract) — a button whose // answer would be refused is worse than none. Arguably wants an "answerable dialogs" // capability; deliberately not invented here. "web/routes/hook-event-routes.ts::mode !== 'deepseek'": allow( 1, 'push buttons withheld where the answer route refuses keystrokes' ), // Legacy Config plumbing, same category as the `===` entries above. "web/routes/session-routes.ts::mode !== 'omp'": allow( 2, 'legacy Config plumbing (resolveOmpConfigForCreate), and one link of the scaffolded-case hooks ' + 'chain (see the opencode entry below)' ), // ⚠️ Scaffolded-case hooks. This chain excludes seven CLIs but NOT `deepseek`, while its // own comment says DeepSeek uses its own system — so a scaffolded deepseek case gets a // Claude hooks block written into it. That inconsistency is UPSTREAM's and predates this // change; expressing the chain as a capability would have to pick a side and would // therefore be a behaviour change. Left exactly as found, and named here so it is visible. "web/routes/session-routes.ts::mode !== 'opencode'": allow( 2, 'scaffolded-case hooks + the COD-91 self-heal skip; the chain omits deepseek upstream, ' + 'so any capability form would change behaviour — see PR discussion' ), "web/routes/session-routes.ts::mode !== 'codex'": allow(1, 'scaffolded-case hooks (see the opencode entry)'), "web/routes/session-routes.ts::mode !== 'gemini'": allow(1, 'scaffolded-case hooks (see the opencode entry)'), "web/routes/session-routes.ts::mode !== 'antigravity'": allow(1, 'scaffolded-case hooks (see the opencode entry)'), "web/routes/session-routes.ts::mode !== 'pi'": allow(1, 'scaffolded-case hooks (see the opencode entry)'), "web/routes/session-routes.ts::mode !== 'grok'": allow(1, 'scaffolded-case hooks (see the opencode entry)'), }; /** Every stock CLI id, derived rather than restated so a new entry is covered automatically. */ const IDS = STOCK_CLIS.map((e) => e.id as string); const ID_ALT = IDS.join('|'); /** * The shapes an id-branch actually takes, all four of them. * * ⚠️ An earlier version of this guard matched `===` ONLY, and that was not a small gap: the * refactor it guards converted the `===` sites and left the negated ones, so 36 * `mode !== ''` branches survived it — 28 in session-routes.ts alone, including a * seven-mode chain auto-enabling Ralph under a comment asking the next person to keep it in * step with a predicate BY HAND, while the sibling quick-start path already read * `capabilities.ralph`. A guard that sees half the shapes reports a count measured over the * half it happens to catch. * * `switch`/`case` and `[...].includes(mode)` are here for the same reason: each is a way of * writing the banned rule that the narrower pattern could not see. */ const BRANCH_PATTERN = new RegExp( [ // mode === 'codex' / mode !== 'codex' `\\b(?:mode|id|agentType)\\s*[!=]==\\s*'(?:${ID_ALT})'`, // case 'codex': `\\bcase\\s+'(?:${ID_ALT})'\\s*:`, // ['codex', 'gemini'].includes(mode) — the id list IS the branch, wherever `mode` sits `'(?:${ID_ALT})'\\s*(?:,\\s*'(?:${ID_ALT})'\\s*)*\\]\\s*\\.includes\\(`, ].join('|'), 'g' ); /** * BLANK comment lines before scanning, rather than dropping them. Comments legitimately quote * the very pattern being banned — several of them explain WHY a branch was removed — and * flagging those would push the next author to delete the explanation rather than the code. * * ⚠️ Blanking rather than removing is what keeps reported line numbers pointing at the real * file. Dropping the lines shifted every finding upward by however many comments preceded it, * so the guard's own diagnostic sent you to the wrong place — which for a rule about not * writing a branch is exactly the moment you need the right one. */ function uncommented(source: string): string { return source .split('\n') .map((line) => (/^\s*(\/\/|\*|\/\*)/.test(line) ? '' : line)) .join('\n'); } function walk(dir: string, out: string[] = []): string[] { for (const name of readdirSync(dir)) { const full = join(dir, name); if (statSync(full).isDirectory()) walk(full, out); else if (name.endsWith('.ts')) out.push(full); } return out; } interface Finding { file: string; expression: string; line: number; key: string; } function scan(): { findings: Finding[]; filesScanned: number } { const findings: Finding[] = []; const files = walk(SRC); let scanned = 0; for (const full of files) { const rel = relative(SRC, full); if (EXEMPT_FILES.has(rel)) continue; scanned++; const lines = uncommented(readFileSync(full, 'utf-8')).split('\n'); lines.forEach((line, i) => { BRANCH_PATTERN.lastIndex = 0; // shared /g regex — see utils/regex-patterns.ts for (const match of line.matchAll(BRANCH_PATTERN)) { const expression = match[0].replace(/\s+/g, ' ').replace(/^(?:id|agentType)/, 'mode'); const posix = rel.split(sep).join('/'); findings.push({ file: posix, expression, line: i + 1, key: `${posix}::${expression}` }); } }); } return { findings, filesScanned: scanned }; } const { findings, filesScanned } = scan(); interface CountMismatch { key: string; allowed: number; found: number; lines: string[]; } /** * Allowlisted keys whose occurrence count differs from the approved one: `grown` holds the * keys with MORE copies than approved (a new branch riding an old approval), `shrunk` the * ones with fewer (a stale approval that would let the next copy back in unseen). * Unallowlisted keys are not this function's business; the offenders check covers them. */ function countMismatches( found: Finding[], allowed: Record ): { grown: CountMismatch[]; shrunk: CountMismatch[] } { const byKey = new Map(); for (const f of found) byKey.set(f.key, [...(byKey.get(f.key) ?? []), f]); const grown: CountMismatch[] = []; const shrunk: CountMismatch[] = []; for (const [key, allowance] of Object.entries(allowed)) { const hits = byKey.get(key) ?? []; const mismatch = { key, allowed: allowance.count, found: hits.length, lines: hits.map((f) => `${f.file}:${f.line}`), }; if (hits.length > allowance.count) grown.push(mismatch); else if (hits.length < allowance.count) shrunk.push(mismatch); } return { grown, shrunk }; } const { grown, shrunk } = countMismatches(findings, ALLOWED_BRANCHES); describe('no CLI-id branching outside the stock catalog', () => { it('scans a meaningful number of source files (sanity)', () => { // If this collapses toward zero the walker or the exemption list drifted and every // assertion below would pass vacuously. Fix the scanner, do not delete the test. expect(filesScanned).toBeGreaterThan(100); }); it('builds its id list from the live catalog (sanity)', () => { expect(IDS).toContain('claude'); expect(IDS).toContain('deepseek'); expect(IDS.length).toBeGreaterThanOrEqual(9); }); it('still detects a branch when one exists (anti-vacuity)', () => { // Proves the pattern actually matches every shape it is meant to ban, so a regex typo // cannot silently turn this whole file into a no-op. One case per alternative, because // the `===`-only version of this test passed happily while `!==` went unseen. const samples = [ "if (session.mode === 'codex') { doSomething(); }", "if (mode !== 'shell' && mode !== 'deepseek') { doSomething(); }", "switch (mode) { case 'gemini': return 1; }", "if (['codex', 'gemini'].includes(mode)) { doSomething(); }", ]; for (const sample of samples) { BRANCH_PATTERN.lastIndex = 0; expect(sample.match(BRANCH_PATTERN), `pattern missed: ${sample}`).not.toBeNull(); } BRANCH_PATTERN.lastIndex = 0; expect(uncommented(" // mode === 'codex'\ncode();").match(BRANCH_PATTERN)).toBeNull(); }); it('has no unapproved id branches', () => { const offenders = findings.filter((f) => !(f.key in ALLOWED_BRANCHES)); const detail = offenders.map((f) => ` ${f.file}:${f.line} ${f.expression}`).join('\n'); expect( offenders, offenders.length === 0 ? '' : `Found ${offenders.length} CLI-id branch(es) outside the stock catalog:\n${detail}\n\n` + 'Two ways out, in order of preference:\n' + ' 1. Express the difference as data on the CliEntry (a CliCapabilities field), or as a\n' + ' NAMED PROFILE in config/cli-registry/profiles.ts if it genuinely needs to run code.\n' + ' 2. If it is not a CLI-behaviour branch at all, add it to ALLOWED_BRANCHES in this file\n' + " WITH the reason. Read this file's header before choosing option 2." ).toEqual([]); }); it('has no new copy of an allowlisted branch', () => { // An approval covers the copies that were reviewed, not every later line that happens // to spell the same expression in the same file. const detail = grown .map((m) => ` ${m.key}: ${m.found} found, ${m.allowed} approved\n ${m.lines.join('\n ')}`) .join('\n'); expect( grown, grown.length === 0 ? '' : `Found more copies of an allowlisted CLI-id branch than were approved:\n${detail}\n\n` + 'Express the new copy as a CliCapabilities field (or a named profile) rather than raising ' + "the count. Raise it only for another branch of the SAME kind, and read this file's header first." ).toEqual([]); }); it('has no stale allowlist entries', () => { // An allowlisted branch that no longer exists is a lie about the codebase, and the next // person to reintroduce that exact branch would sail straight through. The same holds // for an approved count above what the file still has. const stale = shrunk.map((m) => `${m.key}: ${m.found} found, ${m.allowed} approved`); expect( stale, `ALLOWED_BRANCHES entries no longer (fully) present; delete them or lower the count:\n ${stale.join('\n ')}` ).toEqual([]); }); it('counts copies per key, so one extra copy of an approved branch fails (anti-vacuity)', () => { const at = (line: number): Finding => ({ file: 'web/example.ts', expression: "mode === 'codex'", line, key: "web/example.ts::mode === 'codex'", }); const approved = { "web/example.ts::mode === 'codex'": allow(1, 'synthetic') }; expect(countMismatches([at(10)], approved)).toEqual({ grown: [], shrunk: [] }); const extra = countMismatches([at(10), at(42)], approved); expect(extra.grown).toEqual([ { key: "web/example.ts::mode === 'codex'", allowed: 1, found: 2, lines: ['web/example.ts:10', 'web/example.ts:42'], }, ]); expect(countMismatches([], approved).shrunk.map((m) => m.key)).toEqual(["web/example.ts::mode === 'codex'"]); // Every live entry carries a positive whole count, or the comparison means nothing. for (const [key, { count }] of Object.entries(ALLOWED_BRANCHES)) { expect(Number.isInteger(count) && count > 0, key).toBe(true); } }); }); describe('declared-for-later fields', () => { /** * The fields `CliEntry`'s header declares as not-yet-read. Each is frontend behaviour, and * the frontend is untouched by this change. * * This is here so the list cannot quietly GROW. An unread field is a promise the code does * not keep, and the failure mode is a reader trusting one: the next person sees * `echo.policy: 'buffer'` on an entry and assumes the terminal honours it. Adding a field * nobody reads should be a decision someone makes on purpose, which means updating this * list — and wiring one up should make its line here fail, which is the good direction. */ const DECLARED_FOR_LATER = [ 'accent', 'capabilities.echo', 'capabilities.wheelForward', 'capabilities.keyboardAccessory', 'capabilities.maxFrameBytes', // The Docker credential-seeding path still reads its own CRED_STORES table: this shape // allows ONE store per CLI and the live table needs two for gemini. See CliOverlays. 'overlays.credStore', ]; /** Read every `.ts` under src/, minus the registry itself (which of course names them). */ function sourceOutsideRegistry(): string { const parts: string[] = []; const stack = [SRC]; while (stack.length > 0) { const dir = stack.pop()!; for (const name of readdirSync(dir)) { const full = join(dir, name); if (statSync(full).isDirectory()) { if (name !== 'cli-registry') stack.push(full); continue; } if (name.endsWith('.ts')) parts.push(uncommented(readFileSync(full, 'utf-8'))); } } return parts.join('\n'); } /** * Receivers whose same-named property is NOT this field. A leaf-name match is all a static * check can do, and `cli.ts` calls `palette.accent('admin')` — the terminal colour helper, * unrelated to `CliEntry.accent`. Listing the receiver is better than dropping the field * from the check: a real read through any OTHER receiver still fails. */ const UNRELATED_RECEIVERS: Record = { accent: ['palette'] }; const outside = sourceOutsideRegistry(); it.each(DECLARED_FOR_LATER)('%s is still unread outside the registry', (field) => { const leaf = field.split('.').pop()!; const ignore = UNRELATED_RECEIVERS[leaf] ?? []; // `.` as a property access. Comment lines are already blanked, so a mention in // prose does not count as a read; a receiver listed above does not either. const pattern = new RegExp(`(\\w*)\\.${leaf}\\b`, 'g'); const uses = [...outside.matchAll(pattern)].filter((m) => !ignore.includes(m[1])).map((m) => m[0]); expect( uses, `${field} now looks READ outside config/cli-registry. If that is deliberate, drop it ` + "from DECLARED_FOR_LATER here and from CliEntry's header comment — the point of both " + 'is that a reader can tell which fields are load-bearing.' ).toEqual([]); }); it('still catches a read when there is one (anti-vacuity)', () => { // The check is only worth having if it fires, so prove it against a field that IS read. // `capabilities.ralph` is live in session-routes; if this ever stops matching, the // scanner has drifted and every assertion above is passing vacuously. expect(outside).toMatch(/\.ralph\b/); expect(DECLARED_FOR_LATER.length).toBeGreaterThan(0); }); });