mirror of
https://github.com/Ark0N/Codeman.git
synced 2026-09-30 12:39:42 +02:00
feat(cli-registry): drive the run-menu frontend from the CLI catalogue (PR B2)
PR #380 (PR B) held back the frontend half of the CLI registry refactor, explicitly deferring window.__codemanCliCatalog and making session-ui.js / mobile-overview.js catalogue-driven as "PR B2". - Inject window.__codemanCliCatalog in renderIndexHtml(), following the existing __codemanCustomModelClis pattern (escapeScriptJson-guarded, resolved per-request). Reading CliEntry.shortBadge here is what makes it genuinely read, so it drops out of types.ts's DECLARED_FOR_LATER list. - Consolidate session-ui.js's 8 near-duplicate run<Mode>() launch functions (opencode/codex/gemini/antigravity/pi/omp/grok/deepseek) into one shared _runCliMode() plus a local RUN_MODE_LAUNCH config table. The 8 method names stay as thin wrappers (index.html calls them by name; tests assert on the name). Also collapses a duplicated 8-way isAltMode/isExternalCli OR-chain (same expression, copy-pasted twice in openSessionOptions) into one EXTERNAL_CLI_MODES check. - Add test/frontend-cli-no-id-branching.test.ts, a guard scoped to session-ui.js/mobile-overview.js only (not the rest of src/web/public/, which stays explicitly out of scope per CLAUDE.md), mirroring the backend's own no-id-branching guard. mobile-overview.js and the wiring of accent/echo/wheelForward/ keyboardAccessory were investigated and deliberately left alone: the first is already a single, tested, gated table (not duplicated logic); the second set belongs to terminal-ui.js/keyboard-accessory.js/styles.css, files outside this PR's mandate. Verified on a tmux-capable devbox (this sandbox has no tmux): full CI gate at 405 files / 7717 tests / 0 failures, typecheck clean, 94 targeted tests covering exact per-CLI wire-body shapes unmodified and passing, and a live anti-vacuity check on the new guard (injected a real branch, confirmed it fails, reverted, confirmed green). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GuHtuPiHXdykq9T6rKQJ9n
This commit is contained in:
Devvyn
co-authored by
Claude Sonnet 5
Claude Sonnet 5
parent
2d573d8a34
commit
cd64b0a3f7
@@ -314,7 +314,6 @@ describe('declared-for-later fields', () => {
|
||||
* list — and wiring one up should make its line here fail, which is the good direction.
|
||||
*/
|
||||
const DECLARED_FOR_LATER = [
|
||||
'shortBadge',
|
||||
'accent',
|
||||
'capabilities.echo',
|
||||
'capabilities.wheelForward',
|
||||
|
||||
@@ -0,0 +1,185 @@
|
||||
/**
|
||||
* @fileoverview Static guard: no NEW CLI-id branch in the two files PR B2 touched
|
||||
* (`session-ui.js`, `mobile-overview.js`), mirroring
|
||||
* `test/cli-registry-no-id-branching.test.ts` for the backend registry.
|
||||
*
|
||||
* Deliberately scoped to ONLY these two files, not all of `src/web/public/`.
|
||||
* `docs/cli-registry.md` and CLAUDE.md are explicit that the rest of the
|
||||
* frontend (`app.js`, `terminal-ui.js`, `styles.css`, `settings-ui.js`, …)
|
||||
* keeps its own hand-authored per-CLI rules deliberately — "moving them is
|
||||
* its own piece of work verified by a browser/mobile suite the CI gate cannot
|
||||
* see." Widening this guard to the whole directory would force either fixing
|
||||
* or allowlisting dozens of branches in files nobody has touched or reviewed
|
||||
* for this change, which is scope B2 never took on.
|
||||
*
|
||||
* Port: none (pure static analysis).
|
||||
*/
|
||||
|
||||
import { describe, it, expect } from 'vitest';
|
||||
import { readFileSync } from 'node:fs';
|
||||
import { fileURLToPath } from 'node:url';
|
||||
import { STOCK_CLIS } from '../src/config/cli-registry/stock.js';
|
||||
|
||||
const PUBLIC = fileURLToPath(new URL('../src/web/public/', import.meta.url));
|
||||
const SCANNED_FILES = ['session-ui.js', 'mobile-overview.js'];
|
||||
|
||||
/**
|
||||
* Every currently-surviving branch, each with the reason it is not a
|
||||
* CLI-behaviour branch a `CliCapabilities` field should express, keyed
|
||||
* `<file>::<line>::<the matched expression>`. Found by running the scanner
|
||||
* below against the post-B2 state of both files (2026-09-19) and reviewing
|
||||
* each hit in context — none of these are leftover oversights, each is a
|
||||
* verified, deliberate exception.
|
||||
*/
|
||||
const ALLOWED_BRANCHES: Record<string, string> = {
|
||||
// --- run(): claude/shell get their own dispatch path, everything else goes ---
|
||||
// --- through the shared _runCliMode() — see RUN_MODE_LAUNCH's header comment ---
|
||||
"session-ui.js::507::mode === 'shell'": 'run() dispatch: shell needs no CLI probe at all',
|
||||
"session-ui.js::510::mode === 'claude'":
|
||||
'run() dispatch: claude has its own remote/docker branching and parallel-create path ' +
|
||||
'(runClaude), unlike every RUN_MODE_LAUNCH entry',
|
||||
|
||||
// --- runCustomModelEntry(): claude restarts its CLI process in place; every ---
|
||||
// --- other custom-model-eligible CLI applies the pick one-shot, before the ---
|
||||
// --- session exists at all. Documented in CLAUDE.md's Custom Model Endpoint ---
|
||||
// --- Profiles section: "Two launch paths, chosen by mechanism, not preference." ---
|
||||
"session-ui.js::862::mode === 'claude'": 'restart-vs-one-shot custom-model launch mechanism, not a preference',
|
||||
|
||||
// --- Button-label ternary (Open Question 7, DEPLOYMENT_PLAN.md): pinned by ---
|
||||
// --- test/run-mode-ui.test.ts's exact-text assertions (e.g. 'Run OMP'), which ---
|
||||
// --- diverge from CliEntry.shortBadge for at least one CLI (omp: 'OM' vs the ---
|
||||
// --- displayed 'OMP') — a catalogue-driven rewrite would silently change ---
|
||||
// --- user-visible text and break that pinned test. ---
|
||||
"session-ui.js::1527::mode === 'opencode'": 'button-label ternary, pinned exact text (see Open Question 7)',
|
||||
"session-ui.js::1527::mode === 'codex'": 'button-label ternary, pinned exact text (see Open Question 7)',
|
||||
"session-ui.js::1527::mode === 'gemini'": 'button-label ternary, pinned exact text (see Open Question 7)',
|
||||
"session-ui.js::1527::mode === 'antigravity'": 'button-label ternary, pinned exact text (see Open Question 7)',
|
||||
"session-ui.js::1527::mode === 'pi'": 'button-label ternary, pinned exact text (see Open Question 7)',
|
||||
"session-ui.js::1527::mode === 'grok'": 'button-label ternary, pinned exact text (see Open Question 7)',
|
||||
"session-ui.js::1527::mode === 'deepseek'": 'button-label ternary, pinned exact text (see Open Question 7)',
|
||||
"session-ui.js::1527::mode === 'omp'": 'button-label ternary, pinned exact text (see Open Question 7)',
|
||||
"session-ui.js::1527::mode === 'shell'": 'button-label ternary, pinned exact text (see Open Question 7)',
|
||||
|
||||
// --- Respawn/Ralph section: claude-only by product design, mirroring the ---
|
||||
// --- backend's own capabilities.ralph gate (isExternalCliMode() already ---
|
||||
// --- excludes Ralph tracking for every non-claude mode server-side). ---
|
||||
"session-ui.js::2200::mode === 'claude'": 'Respawn/Ralph section is claude-only by design',
|
||||
|
||||
// --- runMode property setter: a validity allowlist, not a behaviour branch. ---
|
||||
// --- Left untouched in Phase 2 (uncertain 'shell' asymmetry — this chain has ---
|
||||
// --- no shell arm at all — and no evidence of what callers rely on it). ---
|
||||
"session-ui.js::4431::mode === 'opencode'": 'runMode setter validity check, not behaviour',
|
||||
"session-ui.js::4431::mode === 'codex'": 'runMode setter validity check, not behaviour',
|
||||
"session-ui.js::4431::mode === 'gemini'": 'runMode setter validity check, not behaviour',
|
||||
"session-ui.js::4431::mode === 'antigravity'": 'runMode setter validity check, not behaviour',
|
||||
"session-ui.js::4431::mode === 'pi'": 'runMode setter validity check, not behaviour',
|
||||
"session-ui.js::4431::mode === 'grok'": 'runMode setter validity check, not behaviour',
|
||||
"session-ui.js::4431::mode === 'deepseek'": 'runMode setter validity check, not behaviour',
|
||||
"session-ui.js::4431::mode === 'omp'": 'runMode setter validity check, not behaviour',
|
||||
"session-ui.js::4431::mode === 'claude'": 'runMode setter validity check, not behaviour',
|
||||
|
||||
// --- mobile-overview.js: shell is exempt from the isCliAvailable() gate the ---
|
||||
// --- same way the toolbar's #runModeMenu exempts it (shell needs no CLI). ---
|
||||
"mobile-overview.js::574::mode !== 'shell'": 'shell needs no CLI, so it is exempt from the availability gate',
|
||||
};
|
||||
|
||||
/** 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('|');
|
||||
|
||||
/** Same four shapes as the backend guard — see its own comment for why all four matter. */
|
||||
const BRANCH_PATTERN = new RegExp(
|
||||
[
|
||||
`\\b(?:mode|id|agentType)\\s*[!=]==\\s*'(?:${ID_ALT})'`,
|
||||
`\\bcase\\s+'(?:${ID_ALT})'\\s*:`,
|
||||
`'(?:${ID_ALT})'\\s*(?:,\\s*'(?:${ID_ALT})'\\s*)*\\]\\s*\\.includes\\(`,
|
||||
].join('|'),
|
||||
'g'
|
||||
);
|
||||
|
||||
/** Blanks comment lines before scanning — see the backend guard's own comment on why. */
|
||||
function uncommented(source: string): string {
|
||||
return source
|
||||
.split('\n')
|
||||
.map((line) => (/^\s*(\/\/|\*|\/\*)/.test(line) ? '' : line))
|
||||
.join('\n');
|
||||
}
|
||||
|
||||
interface Finding {
|
||||
file: string;
|
||||
expression: string;
|
||||
line: number;
|
||||
key: string;
|
||||
}
|
||||
|
||||
function scan(): Finding[] {
|
||||
const findings: Finding[] = [];
|
||||
for (const file of SCANNED_FILES) {
|
||||
const lines = uncommented(readFileSync(PUBLIC + file, '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');
|
||||
findings.push({ file, expression, line: i + 1, key: `${file}::${i + 1}::${expression}` });
|
||||
}
|
||||
});
|
||||
}
|
||||
return findings;
|
||||
}
|
||||
|
||||
const findings = scan();
|
||||
|
||||
describe('no NEW CLI-id branching in session-ui.js / mobile-overview.js (PR B2)', () => {
|
||||
it('scans both files (sanity)', () => {
|
||||
// If this drops to zero the scanner or the file list drifted and every
|
||||
// assertion below would pass vacuously.
|
||||
const scannedBytes = SCANNED_FILES.reduce((n, f) => n + readFileSync(PUBLIC + f, 'utf-8').length, 0);
|
||||
expect(scannedBytes).toBeGreaterThan(10_000);
|
||||
});
|
||||
|
||||
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)', () => {
|
||||
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} new CLI-id branch(es) in session-ui.js/mobile-overview.js:\n${detail}\n\n` +
|
||||
'Two ways out, in order of preference:\n' +
|
||||
' 1. Derive the difference from window.__codemanCliCatalog (server.ts) or a shared\n' +
|
||||
' module-level constant, the way _runCliMode()/EXTERNAL_CLI_MODES do.\n' +
|
||||
' 2. If it is a genuine mechanism difference (not a CLI-behaviour branch), add it to\n' +
|
||||
' ALLOWED_BRANCHES in this file WITH the reason.'
|
||||
).toEqual([]);
|
||||
});
|
||||
|
||||
it('has no stale allowlist entries', () => {
|
||||
// An allowlisted branch that no longer exists at that line is a lie about the
|
||||
// codebase, and the next person to reintroduce that exact branch elsewhere
|
||||
// would sail straight through under the old line number.
|
||||
const present = new Set(findings.map((f) => f.key));
|
||||
const stale = Object.keys(ALLOWED_BRANCHES).filter((key) => !present.has(key));
|
||||
expect(stale, `ALLOWED_BRANCHES entries no longer present — delete them:\n ${stale.join('\n ')}`).toEqual([]);
|
||||
});
|
||||
});
|
||||
@@ -209,6 +209,38 @@ describe('WebServer.renderIndexHtml', () => {
|
||||
}
|
||||
});
|
||||
|
||||
it('exposes the general run-menu CLI catalogue with the menu-facing fields only', async () => {
|
||||
// Unlike __codemanCustomModelClis above (narrowed to one picker's needs), this
|
||||
// one carries every ENABLED CliEntry, agent and shell alike, with no capability
|
||||
// filter — but still excludes launch/env/capabilities/overlays, the same rule
|
||||
// scripts/generate-cli-catalog.mts follows for config/clis.stock.json.
|
||||
const { server } = makeServer({});
|
||||
const html = await render(server);
|
||||
expect(html).toContain('window.__codemanCliCatalog=');
|
||||
const catalog = JSON.parse(html.match(/window\.__codemanCliCatalog=(\[.*?\]);/)![1]) as Array<{
|
||||
id: string;
|
||||
label: string;
|
||||
shortBadge: string;
|
||||
order: number;
|
||||
kind: string;
|
||||
}>;
|
||||
const ids = catalog.map((c) => c.id);
|
||||
expect(ids).toContain('claude');
|
||||
expect(ids).toContain('shell');
|
||||
for (const cli of catalog) {
|
||||
expect(typeof cli.id).toBe('string');
|
||||
expect(typeof cli.label).toBe('string');
|
||||
expect(typeof cli.shortBadge).toBe('string');
|
||||
expect(typeof cli.order).toBe('number');
|
||||
expect(['agent', 'shell']).toContain(cli.kind);
|
||||
expect(cli).not.toHaveProperty('launch');
|
||||
expect(cli).not.toHaveProperty('env');
|
||||
expect(cli).not.toHaveProperty('capabilities');
|
||||
expect(cli).not.toHaveProperty('overlays');
|
||||
expect(cli).not.toHaveProperty('discovery');
|
||||
}
|
||||
});
|
||||
|
||||
it('escapeScriptJson neutralizes a literal </script>, and still round-trips as a JS literal', () => {
|
||||
// CliEntry.label is a plain string a user's own clis.json can set (up to 60
|
||||
// chars), unlike __codemanCliAvailable's booleans-only payload, so this is
|
||||
@@ -254,6 +286,7 @@ describe('WebServer.renderIndexHtml', () => {
|
||||
const html = await render(server, 'sess-123');
|
||||
expect(html).not.toContain('__codemanCliAvailable');
|
||||
expect(html).not.toContain('__codemanCustomModelClis');
|
||||
expect(html).not.toContain('__codemanCliCatalog');
|
||||
});
|
||||
|
||||
it('does not expose gesture at all when CODEMAN_GESTURE is unset', async () => {
|
||||
|
||||
@@ -96,9 +96,9 @@ describe('WebServer index.html <title> templating (#82)', () => {
|
||||
|
||||
it('only substitutes the <title> tag — the rest of the template is identical (modulo asset cache-busting)', async () => {
|
||||
// renderIndexHtml also appends ?v=<mtime> cache-bust params to same-origin
|
||||
// .js/.css refs, and injects the CLI-availability flags plus the custom-model
|
||||
// Run-menu picker's CLI list before </head>; strip all so the title remains
|
||||
// the only other change.
|
||||
// .js/.css refs, and injects the CLI-availability flags, the custom-model
|
||||
// Run-menu picker's CLI list, and the general run-menu CLI catalogue (PR B2)
|
||||
// before </head>; strip all so the title remains the only other change.
|
||||
//
|
||||
// The flag strips are what keep this test environment-independent. The
|
||||
// CLI-availability one used to pass here by luck: that script was injected
|
||||
@@ -109,7 +109,8 @@ describe('WebServer index.html <title> templating (#82)', () => {
|
||||
const html = (await render('laptop'))
|
||||
.replace(/(\.(?:js|css))\?v=[^"]*/g, '$1')
|
||||
.replace(/<script>window\.__codemanCliAvailable=\{.*?\};<\/script>\n/, '')
|
||||
.replace(/<script>window\.__codemanCustomModelClis=\[.*?\];<\/script>\n/, '');
|
||||
.replace(/<script>window\.__codemanCustomModelClis=\[.*?\];<\/script>\n/, '')
|
||||
.replace(/<script>window\.__codemanCliCatalog=\[.*?\];<\/script>\n/, '');
|
||||
const beforeTitle = rawTemplate.split('<title>Codeman</title>')[0];
|
||||
const afterTitle = rawTemplate.split('<title>Codeman</title>')[1];
|
||||
expect(html.startsWith(beforeTitle)).toBe(true);
|
||||
|
||||
Reference in New Issue
Block a user