Commit Graph
4 Commits
Author SHA1 Message Date
Codeman maintainer 0f955327b2 fix(cli-registry): merge-time fixes for the run-menu consolidation (#458)
- test/opencode-resize.test.ts: retarget the launcher guard at the real code (this.selectSession(firstSessionId), any this.activeSessionId assignment) with an anti-vacuity check; the old strings existed nowhere, so it could never fail
- session-ui.js: restore as comments the two invariants the merged bodies lost (deepseek leaves statusReporting unset, i.e. ON; no effort field for external CLIs, it is Claude-specific)
- docs/cli-registry.md: move the frontend-guard paragraph below the two backend-guard paragraphs so they keep their antecedent, and note the widened comparison shape
- test/frontend-cli-no-id-branching.test.ts: the comparison shape accepts any left-hand identifier (const m = this._runMode; m === 'codex' was invisible), normalized to `mode`; the two `m !== 'shell'` display filters are allowlisted and the remaining blind spots documented
- test/run-mode-dispatch.test.ts: table-driven pin of run() dispatch (claude to runClaude, each RUN_MODE_LAUNCH id to _runCliMode(id), shell to runShell, unknown to runClaude, lock held and released)
- CLAUDE.md: name the second CI-gated guard next to the backend one
- server.ts: every </head> injection passes a replacer function; a clis.json label containing $' re-injected the rest of the document past escapeScriptJson (two render tests pin it, proven failing on the string form)
- _isAltCliMode(): no reference anywhere in the tree, nothing to fix

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
(cherry picked from commit 1ea363ff808a62861559bc141e724b163cc1c56e)
2026-09-21 04:37:45 +02:00
DevvynandClaude Sonnet 5 d9e6ebb20a fix(cli-registry): address round-2 review on #458 — count-based allowlist, RUN_MODE_LAUNCH drift guard
Three of Ark0N's four "will take at merge" items, applied instead since
they were straightforward to do properly:

1. test/frontend-cli-no-id-branching.test.ts's ALLOWED_BRANCHES keyed on
   <file>::<expression> (fixed last round) closed the line-shift problem
   but opened a new one: every stock id was already allowlisted for
   session-ui.js in the `mode === '<id>'` form, so a BRAND NEW branch
   reusing that exact expression anywhere in the file passed unnoticed.
   Reproduced live (`if (this.mode === 'codex')` injected into
   runOpenCode()) — stayed green under the old version. Each allowlist
   entry now carries the exact count of approved call sites, and a new
   test asserts actual-vs-declared count for every key; a mismatch in
   either direction is real (higher = new unreviewed branch riding in on
   an existing approval, lower = a reviewed site was removed and the
   entry is now stale). Reproduced again against the fix: same injection
   now fails with an exact diagnostic (expected 2, found 3).

2. Added test/run-mode-launch-table-drift.test.ts. RUN_MODE_LAUNCH
   restates four things stock.ts already owns (label, install command,
   supportsCustomModel, the external-mode key set), and they agree today
   with nothing enforcing it. supportsCustomModel is the dangerous one:
   the Run-menu picker's rows come from the server-injected
   window.__codemanCustomModelClis (built from
   capabilities.customModelInjection.kind), so a CLI gaining a real
   injection recipe later would be OFFERED in the picker while
   _runCliMode silently drops the customModel field for it — the session
   launches on the vendor's cloud while the UI claims the local endpoint.
   Drives the real session-ui.js via JSDOM and compares RUN_MODE_LAUNCH
   against STOCK_CLIS on all four axes.

3. Inlined the "Open Question 7 in PR-B2.md" references in the allowlist
   reasons — PR-B2.md is a local planning doc, never part of the
   committed tree, so the reference was dead on arrival for anyone
   reading the repo. Points at the PR #458 review thread instead.

4. Added a sentence to docs/cli-registry.md naming the new frontend guard
   alongside the backend one it mirrors.

Full gate: 406 files / 7721 tests / 0 failures, typecheck/lint/format/
check:frontend-syntax all clean.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GuHtuPiHXdykq9T6rKQJ9n
2026-09-20 21:37:05 +08:00
DevvynandClaude Sonnet 5 2df9355367 fix(cli-registry): address PR B2 review — fix two test guards, drop unused catalogue
Two required fixes from Ark0N's review of #458:

1. test/frontend-cli-no-id-branching.test.ts's ALLOWED_BRANCHES keyed on
   <file>::<line>::<expression>. A single inserted line anywhere above an
   entry shifted every subsequent line number, so all 21 entries went stale
   simultaneously and the same 21 branches were reported as "new" — on a
   file six other open PRs also touch. Dropped the line number from the key
   (<file>::<expression>, matching the backend guard's own design), which
   collapses 21 line-keyed entries to 11 or-collapse where the same
   expression recurs at multiple call sites in the same file.

2. test/run-mode-ui.test.ts's terminal-ownership guard scanned method
   bodies via `^ {2}async (run[A-Za-z]*)\(\) \{$`, which matched the 8
   one-line run<Mode>() wrappers PR B2 introduced but not _runCliMode(mode),
   where the real logic (and the actual risk the guard exists to catch) now
   lives. Fixed the regex to `^ {2}async (_?run[A-Za-z]*)\(\w*\) \{$` and
   added _runCliMode to the sanity list. Same-class fix in
   test/opencode-resize.test.ts, which had the identical blind spot via
   runOpenCode.toString().

Both reproduced live before fixing (inserted the same comment line; added
this.terminal.clear() to _runCliMode) to confirm the bug, then confirmed
the fix catches it and the suite stays green otherwise.

Also resolves Open Question 2 by dropping window.__codemanCliCatalog
entirely: nothing consumed it, and a registry DECLARED_FOR_LATER field
costs nothing until read while an unconsumed script tag on every page
render is a different trade. Reverts Phase 1 cleanly — server.ts's
injection, shortBadge back in types.ts's DECLARED_FOR_LATER list and the
pinned guard test, and the three associated render-index-html.test.ts /
server-index-title.test.ts assertions.

Full gate: 405 files / 7717 tests / 0 failures (net unchanged), typecheck/
lint/format clean.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GuHtuPiHXdykq9T6rKQJ9n
2026-09-20 02:04:37 +08:00
DevvynandClaude Sonnet 5 cd64b0a3f7 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
2026-09-19 21:16:31 +08:00