Files
Codeman/test/cli-capability-predicates.test.ts
DevvynandClaude Opus 5 4830e662f9 refactor(cli-registry): make CLI backends data instead of per-mode branching
Every run mode is now a `CliEntry` in `src/config/cli-registry/` — discovery
(search dirs, version + identity probes), the launch argv template, env
handling, the `capabilities` flags that replace per-CLI branching, and the
`overlays` that back the remote/docker pane commands. Code that used to ask
"which CLI is this?" reads the entry instead.

Behaviour is unchanged. `test/cli-registry-spawn-golden.test.ts` pins every
spawn command as a literal string, captured from the hand-written builders
before they were deleted, and `test/location-overlay-commands.test.ts` does the
same for all 20 remote and in-container pane commands.

Config can never contain shell text: an entry declares typed argv tokens,
literals are validated against a safe-word pattern at LOAD time (a bad literal
rejects the whole entry — a silently dropped `--no-approve` is not cosmetic),
and values resolve through patterns NAMED in code, so a user `clis.json` cannot
widen its own validation. `~/.codeman/clis.json` overrides any entry, read-only
in this release.

OMP is included as a registry entry rather than a tenth hand-written builder,
so `buildOmpCommand()`, the omp availability pre-flight, the omp arm of
`buildPathExport()` and the omp entries in the truecolor/NO_COLOR, alt-screen
and doctor ladders all drop out.

Guard rails:

- `test/cli-registry-no-id-branching.test.ts` fails the build if per-CLI-id
  branching reappears outside `stock.ts`, in any of its four shapes (`===`,
  `!==`, `switch`/`case`, `includes`) — an `===`-only version would miss the
  negated forms, which is how 36 of them survived an earlier pass. Every
  allowlisted branch carries its reason.
- `external`, `hooks` and `altScreen` stay three INDEPENDENT capabilities;
  deriving one from another shipped the `until=stop`-hangs-on-shell bug.
- `param` is two namespaces. `launch.params` keys, `configSetenv.fromParam` and
  `privilegedParams[].param` all name a LAUNCH param; the legacy `<Mode>Config`
  wire field is separate, bridged only by `legacyConfigAliases`. Getting
  `privilegedParams[].param` wrong is SILENT — it is the multi-user bypass
  clamp's only handle on a CLI's privilege switch, and a wrong name clamps
  nothing with no error and no failing test — so `schema.ts` rejects an entry
  naming a param it never declared.
- Registry data resolves AT CALL TIME (`sessionModeSchema()`,
  `allowedEnvPrefixes()`, `dependencyRegistry()`, the resolvers' `searchDirs`
  thunks). A module-level const freezes at first import, so a CLI enabled while
  the server ran moved the run menu but not that surface.
- Six fields are annotated DECLARED-FOR-LATER and read by nothing
  (`shortBadge`, `accent`, `capabilities.echo`/`wheelForward`/
  `keyboardAccessory`/`maxFrameBytes`): all frontend behaviour, transcribed
  rather than measured. A test pins the list so it cannot quietly grow.

Three user-visible changes, all deliberate and named:

- `probeDockerCliVersion()` derives the in-container binary from the registry
  rather than assuming it equals the mode name (`antigravity` runs `agy`).
- The remote CLI version probe now covers grok and deepseek, which the
  hardcoded map it replaces omitted while its own comment said the rule was
  "every mode except shell".
- `codeman doctor`'s CLI rows are generated from the entries, so Claude's
  install hint is the install command rather than a docs URL, five CLIs gain
  hints they never had, and the row order follows the catalog.

Also hardened along the way: `sessionModeSchema()` is bounded at 24 chars
(matching the `cliId` pattern) before its failure message quotes the value
back, and `deepMerge` skips `__proto__`/`constructor`/`prototype` when reading
the hand-editable `clis.json`.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WQkoi1cNegqVwZHgzx5SbJ
2026-09-02 08:26:45 +08:00

102 lines
4.8 KiB
TypeScript

/**
* @fileoverview The three per-mode predicates that used to be hand-written id lists, and the
* invariant that they are INDEPENDENT.
*
* `isExternalCliMode()`, `isAltScreenStripMode()` and `hooksAvailableForMode()` describe three
* different, deliberately unequal sets. Deriving any one of them from another looks like a
* tidy-up and has already shipped a bug: `shell` has no hooks but is NOT an external CLI, so
* a hooks predicate written as `!isExternalCliMode()` accepted `until=stop` on a shell session
* and then blocked the caller for their entire timeout — an infinite wait wearing a timeout's
* clothes, which is precisely what that guard exists to prevent.
*
* Keeping them as three separate `CliCapabilities` fields makes that structural. This file is
* what stops someone collapsing them again.
*
* Port: none (pure predicates over registry data).
*/
import { describe, it, expect } from 'vitest';
import { isExternalCliMode, isAltScreenStripMode } from '../src/session.js';
import { hooksAvailableForMode } from '../src/web/session-wait-registry.js';
import { enabledCliIds } from '../src/config/cli-registry/registry.js';
import type { SessionMode } from '../src/types/session.js';
const MODES = enabledCliIds() as SessionMode[];
describe('per-mode capability predicates', () => {
it.each([
// mode external altScreenStrip hooks
['claude', false, true, true],
['shell', false, false, false],
['opencode', true, false, false],
['codex', true, true, false],
['gemini', true, true, false],
['antigravity', true, false, false],
['pi', true, false, false],
['grok', true, false, false],
['deepseek', true, false, true],
['omp', true, false, false],
] as Array<[SessionMode, boolean, boolean, boolean]>)(
'%s: external=%s altScreenStrip=%s hooks=%s',
(mode, external, altScreen, hooks) => {
expect(isExternalCliMode(mode)).toBe(external);
expect(isAltScreenStripMode(mode)).toBe(altScreen);
expect(hooksAvailableForMode(mode)).toBe(hooks);
}
);
it('covers every enabled mode (sanity)', () => {
// If a CLI is added without a row above, this fails rather than the table silently
// describing a subset of reality.
expect(MODES.length).toBe(10);
});
it('keeps the three predicates genuinely distinct', () => {
// Not "they happen to differ today" — each pair differs on a NAMED mode, and each of
// those disagreements is load-bearing.
const external = MODES.filter(isExternalCliMode);
const altScreen = MODES.filter(isAltScreenStripMode);
const hooks = MODES.filter((m) => hooksAvailableForMode(m));
expect(external).not.toEqual(altScreen);
expect(external).not.toEqual(hooks);
expect(altScreen).not.toEqual(hooks);
// claude is the mode that separates all three: not external, IS stripped, HAS hooks.
expect(isExternalCliMode('claude')).toBe(false);
expect(isAltScreenStripMode('claude')).toBe(true);
expect(hooksAvailableForMode('claude')).toBe(true);
// deepseek is external AND has hooks — the pairing that makes "external ⇒ no hooks" false.
expect(isExternalCliMode('deepseek')).toBe(true);
expect(hooksAvailableForMode('deepseek')).toBe(true);
});
it('does not accept a hook-only wait on a shell session', () => {
// The exact historical bug, reproduced. `shell` is not external, so any hooks predicate
// derived from `isExternalCliMode` would answer true here and hang the caller.
expect(isExternalCliMode('shell')).toBe(false);
expect(hooksAvailableForMode('shell')).toBe(false);
});
it("treats deepseek's hooks as a per-SESSION question, not a per-mode one", () => {
// 'supervised': real signals, but only while this session's bridge is actually armed and
// reachable. Answering from the mode alone promises a `stop` that never arrives.
expect(hooksAvailableForMode('deepseek')).toBe(true);
expect(hooksAvailableForMode('deepseek', { deepSeekStatusReporting: false })).toBe(false);
expect(hooksAvailableForMode('deepseek', { deepSeekBridgeUnreachable: true })).toBe(false);
// claude's are unconditional, so the same options change nothing.
expect(hooksAvailableForMode('claude', { deepSeekStatusReporting: false })).toBe(true);
expect(hooksAvailableForMode('claude', { deepSeekBridgeUnreachable: true })).toBe(true);
});
it('falls back conservatively for an unregistered mode', () => {
const unknown = 'not-a-cli' as SessionMode;
// External: disables Claude-specific parsing rather than pointing it at foreign output.
expect(isExternalCliMode(unknown)).toBe(true);
// No hooks: never promise a signal nothing will send.
expect(hooksAvailableForMode(unknown)).toBe(false);
// No full strip: leaving the alt screen alone is the safe default for an unknown TUI.
expect(isAltScreenStripMode(unknown)).toBe(false);
});
});