fix(cli-registry): guard workDetect.workingLine like every other config regex

#385 made the composer glyph and the working status line per-CLI registry
data, which is right, but `workingLine` arrived as a config-supplied regex
validated with a bare `new RegExp()`. That skips `compileVersionRegex()`,
the helper the registry uses for exactly this: a `~/.codeman/clis.json`
override can set the field, the compiled pattern is run against every
accumulated PTY chunk and every pane capture, and a nested quantifier there
backtracks on the event loop for the whole server rather than one session.

Route it through the helper in both places, which are not redundant: the
schema refine rejects the entry at LOAD time so a bad pattern never reaches
a session, and `_workingLinePattern()` compiles through the same helper so
the runtime cannot hold a pattern the schema would have refused. The helper
returns null instead of throwing, so the Claude-pattern fallback stops being
a try/catch and becomes structural. Both shipped patterns compile unchanged,
and Claude's is behaviourally identical to CLAUDE_WORKING_LINE_PATTERN.

Also match the Codex footer case-insensitively on the E. It was
characterised against codex-cli 0.152.1, which prints a lowercase `esc`;
a version capitalising it would make the whole fix silently inert, since
the pane would simply never look like it was working.

Docs: CLAUDE.md, architecture-invariants and cli-registry.md all still
stated the Claude-mode-only rule this PR retires, and none of them named
the new capability or the regex guard.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Codeman maintainer
2026-09-07 22:42:20 +02:00
parent a49be03f96
commit f1b7283393
8 changed files with 65 additions and 24 deletions
+3 -3
View File
File diff suppressed because one or more lines are too long
+1 -1
View File
@@ -20,7 +20,7 @@ Implementation detail extracted from `CLAUDE.md` so that file stays small enough
### External CLI modes (OpenCode, Codex, Gemini, Antigravity, Pi, Grok, DeepSeek, OMP)
**External CLI modes (OpenCode, Codex, Gemini, Antigravity, Pi, Grok, DeepSeek, OMP)**: `isExternalCliMode()` in `session.ts` (`mode === 'opencode' || 'codex' || 'gemini' || 'antigravity' || 'pi' || 'grok' || 'deepseek'`) gates Claude-specific behavior — Ralph tracker, BashToolParser, token/CLI-info parsing, and ❯-prompt readiness detection are all skipped (these CLIs render their own TUIs; readiness = output stabilization instead). All seven modes **require tmux — no direct PTY fallback** — because secrets are injected via `tmux setenv` (socket-scoped `${this.tmux()} setenv`, never on the spawn command line): OpenCode gets `OPENCODE_CONFIG_CONTENT` etc., Codex gets `OPENAI_API_KEY`/`CODEX_API_KEY`/`CODEX_HOME` (`setCodexEnvVars`), Gemini gets `GEMINI_API_KEY`/`GOOGLE_API_KEY`/`GOOGLE_CLOUD_PROJECT`/`GOOGLE_APPLICATION_CREDENTIALS`/`GOOGLE_GENAI_USE_VERTEXAI` etc. (`setGeminiEnvVars`, all in `tmux-manager.ts`). Codex specifics: command built by `buildCodexCommand()` (`--model`, `resume <id>`, `--dangerously-bypass-approvals-and-sandbox` from the `codexConfig` payload / `codexDangerouslyBypassApprovals` app setting; `renderMode` is schema-coerced to `'hybrid'`, the only supported mode). Gemini specifics: command built by `buildGeminiCommand()` (`--skip-trust` always, `--approval-mode <default|auto_edit|yolo|plan>` defaulting to `yolo` for parity with Claude's `--dangerously-skip-permissions`, `--model`, `--resume` from the `geminiConfig` payload); availability via `GET /api/gemini/status` — session/quick-start routes fail with `OPERATION_FAILED` + install hint (`npm install -g @google/gemini-cli`) when missing. Codex AND Gemini export `COLORTERM=truecolor` + unset `NO_COLOR` (other modes unset `COLORTERM`); Gemini joins `isAltScreenStripMode()` (Codex/Claude/Gemini are Ink TUIs that repaint inline → strip alt-screen/`3J` so scrollback survives). Codex availability via `GET /api/codex/status`. Antigravity specifics: command built by `buildAntigravityCommand()` (`--model`, `--conversation <id>` resume, `--dangerously-skip-permissions` from the `antigravityConfig` payload); availability via `GET /api/antigravity/status` — routes fail with `OPERATION_FAILED` + install hint (`curl -fsSL https://antigravity.google/cli/install.sh | bash`) when missing. Unlike the other three it is NOT an npm package (standalone binary, `~/.local/bin/agy`), which is why `docker/agent.Dockerfile` installs it with its own `--dir /usr/local/bin` step rather than in the `npm install -g` line, and why it does NOT join `isAltScreenStripMode()`. Frontend: run-mode dropdown → `runCodex()`/`runGemini()` in `session-ui.js` ("Run CX"/"Run GM" labels), App Settings → Agents & CLIs → Codex; Respawn/Ralph options are Claude-only, so session options open on the Session tab for external CLI sessions. ⚠️ `run*()` MUST unwrap the `{success,data}` envelope (`(await res.json()).data.available` / `data.data.sessionId`) — reading the raw shape silently breaks the run. Tests: `test/run-mode-ui.test.ts` + `test/gemini-mode.test.ts` (vm-sandbox harness, no real DOM). Grok specifics: command built by `buildGrokCommand()` (`--always-approve` from `grokConfig.alwaysApprove` — grok's `bypassPermissions` permission mode, deny rules still apply; `--model`; `--resume <id>` / `--continue`, id-regexed so grok's resume-by-TITLE feature can never put an arbitrary string on the spawn line); availability via `GET /api/grok/status`, which carries `version` because the resolver version-probes candidates (`grok` has npm squatters, e.g. @vibe-kit/grok-cli — `GROK_VERSION_REGEX` is shared with the dependency registry so doctor and run mode agree). Like antigravity it is a standalone binary (xAI installer → `~/.grok/bin`, symlinked into `~/.local/bin`), so `docker/agent.Dockerfile` installs it in its own step (copy to `/usr/local/bin`, drop root's `~/.grok` in the same layer) and it stays OUT of `isAltScreenStripMode()` (fullscreen alt-screen TUI with mouse support — the opencode case, not the Ink case). Env allowlist: `GROK_*` plus the vendor namespace `XAI_*` (`XAI_API_KEY` is grok's documented headless auth var — the same narrow-vendor-namespace reasoning as `GOOGLE_*` for gemini). Docker cred seeding is per-file (`auth.json`, `config.toml`, `pager.toml` from `~/.grok` — the dir also holds `sessions/`, `memory/`, and the ~160MB binary under `downloads/`). Grok tests: `test/grok-mode.test.ts`, `test/grok-cli-resolver.test.ts`.
**External CLI modes (OpenCode, Codex, Gemini, Antigravity, Pi, Grok, DeepSeek, OMP)**: `isExternalCliMode()` in `session.ts` (`mode === 'opencode' || 'codex' || 'gemini' || 'antigravity' || 'pi' || 'grok' || 'deepseek'`) gates Claude-specific behavior — Ralph tracker, BashToolParser, token/CLI-info parsing, and ❯-prompt readiness detection are all skipped (these CLIs render their own TUIs; readiness = output stabilization instead). ⚠️ **Work detection left this gate in #385** and is now per-CLI `capabilities.workDetect` data (`promptGlyph` + `workingLine`), because gating it on the mode left every Codex session reporting `idle` for its entire life; a CLI declaring neither falls back to Claude's pair, which is logic-identical to the pre-registry behaviour. All seven modes **require tmux — no direct PTY fallback** — because secrets are injected via `tmux setenv` (socket-scoped `${this.tmux()} setenv`, never on the spawn command line): OpenCode gets `OPENCODE_CONFIG_CONTENT` etc., Codex gets `OPENAI_API_KEY`/`CODEX_API_KEY`/`CODEX_HOME` (`setCodexEnvVars`), Gemini gets `GEMINI_API_KEY`/`GOOGLE_API_KEY`/`GOOGLE_CLOUD_PROJECT`/`GOOGLE_APPLICATION_CREDENTIALS`/`GOOGLE_GENAI_USE_VERTEXAI` etc. (`setGeminiEnvVars`, all in `tmux-manager.ts`). Codex specifics: command built by `buildCodexCommand()` (`--model`, `resume <id>`, `--dangerously-bypass-approvals-and-sandbox` from the `codexConfig` payload / `codexDangerouslyBypassApprovals` app setting; `renderMode` is schema-coerced to `'hybrid'`, the only supported mode). Gemini specifics: command built by `buildGeminiCommand()` (`--skip-trust` always, `--approval-mode <default|auto_edit|yolo|plan>` defaulting to `yolo` for parity with Claude's `--dangerously-skip-permissions`, `--model`, `--resume` from the `geminiConfig` payload); availability via `GET /api/gemini/status` — session/quick-start routes fail with `OPERATION_FAILED` + install hint (`npm install -g @google/gemini-cli`) when missing. Codex AND Gemini export `COLORTERM=truecolor` + unset `NO_COLOR` (other modes unset `COLORTERM`); Gemini joins `isAltScreenStripMode()` (Codex/Claude/Gemini are Ink TUIs that repaint inline → strip alt-screen/`3J` so scrollback survives). Codex availability via `GET /api/codex/status`. Antigravity specifics: command built by `buildAntigravityCommand()` (`--model`, `--conversation <id>` resume, `--dangerously-skip-permissions` from the `antigravityConfig` payload); availability via `GET /api/antigravity/status` — routes fail with `OPERATION_FAILED` + install hint (`curl -fsSL https://antigravity.google/cli/install.sh | bash`) when missing. Unlike the other three it is NOT an npm package (standalone binary, `~/.local/bin/agy`), which is why `docker/agent.Dockerfile` installs it with its own `--dir /usr/local/bin` step rather than in the `npm install -g` line, and why it does NOT join `isAltScreenStripMode()`. Frontend: run-mode dropdown → `runCodex()`/`runGemini()` in `session-ui.js` ("Run CX"/"Run GM" labels), App Settings → Agents & CLIs → Codex; Respawn/Ralph options are Claude-only, so session options open on the Session tab for external CLI sessions. ⚠️ `run*()` MUST unwrap the `{success,data}` envelope (`(await res.json()).data.available` / `data.data.sessionId`) — reading the raw shape silently breaks the run. Tests: `test/run-mode-ui.test.ts` + `test/gemini-mode.test.ts` (vm-sandbox harness, no real DOM). Grok specifics: command built by `buildGrokCommand()` (`--always-approve` from `grokConfig.alwaysApprove` — grok's `bypassPermissions` permission mode, deny rules still apply; `--model`; `--resume <id>` / `--continue`, id-regexed so grok's resume-by-TITLE feature can never put an arbitrary string on the spawn line); availability via `GET /api/grok/status`, which carries `version` because the resolver version-probes candidates (`grok` has npm squatters, e.g. @vibe-kit/grok-cli — `GROK_VERSION_REGEX` is shared with the dependency registry so doctor and run mode agree). Like antigravity it is a standalone binary (xAI installer → `~/.grok/bin`, symlinked into `~/.local/bin`), so `docker/agent.Dockerfile` installs it in its own step (copy to `/usr/local/bin`, drop root's `~/.grok` in the same layer) and it stays OUT of `isAltScreenStripMode()` (fullscreen alt-screen TUI with mouse support — the opencode case, not the Ink case). Env allowlist: `GROK_*` plus the vendor namespace `XAI_*` (`XAI_API_KEY` is grok's documented headless auth var — the same narrow-vendor-namespace reasoning as `GOOGLE_*` for gemini). Docker cred seeding is per-file (`auth.json`, `config.toml`, `pager.toml` from `~/.grok` — the dir also holds `sessions/`, `memory/`, and the ~160MB binary under `downloads/`). Grok tests: `test/grok-mode.test.ts`, `test/grok-cli-resolver.test.ts`.
**DeepSeek Harness (`dsh`) specifics** — the mode that breaks three of the assumptions the six above share, so read this before changing anything about it.
+7
View File
@@ -36,12 +36,19 @@ interface CliEntry {
launch: CliLaunch; // the structured argv template
env: CliEnv; // exports, tmux setenv keys, the env-override allowlist
capabilities: CliCapabilities; // what every call site reads instead of the id
// .workDetect?: { promptGlyph, workingLine } — how this CLI's pane shows work
overlays: CliOverlays; // remote-SSH / Docker pane commands, credential store
}
```
`capabilities` is the important part. It is what `isExternalCliMode()`, `isAltScreenStripMode()`, `hooksAvailableForMode()` and every other former per-mode branch actually read.
### Regexes that come from config
Two capability fields carry a regular expression an override file can set: `discovery.version.regex` and `capabilities.workDetect.workingLine`. Both go through `compileVersionRegex()`, which caps the source at 200 characters, refuses the nested-quantifier shapes that cause catastrophic backtracking, and returns `null` rather than throwing so every caller degrades instead of crashing.
`workingLine` is the one that matters most, because it is compiled once per session and then run against every accumulated PTY chunk and every pane capture. A nested quantifier there is a ReDoS against the event loop for the whole server, not just that session. The guard therefore runs in two places, and neither is redundant: `schema.ts` rejects the entry at LOAD time so a bad pattern never reaches a session, and `_workingLinePattern()` in `session.ts` compiles through the same helper so the runtime cannot end up with a pattern the schema would have refused.
### Three capabilities that must stay independent
`external`, `hooks` and `altScreen` describe three different, deliberately unequal sets, and deriving any one from another 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. `deepseek` is the mirror image: it IS external and it DOES have hooks.
+9 -12
View File
@@ -13,7 +13,7 @@
*/
import { z } from 'zod';
import { TOKEN_PATTERNS } from './patterns.js';
import { compileVersionRegex, TOKEN_PATTERNS } from './patterns.js';
import { isKnownLauncherProfile, isKnownSetenvProfile } from './profiles.js';
/** A bare CLI id: lowercase, starts with a letter, at most 24 chars. Also used as a CSS/URL token. */
@@ -277,20 +277,17 @@ const capabilitiesSchema = z
workDetect: z
.object({
promptGlyph: z.string().min(1).max(8),
// Compiled per session, so a broken pattern must fail at LOAD time rather than
// throw inside the PTY data handler.
// Config-supplied regex, so it goes through the same guard as `version.regex`:
// ~/.codeman/clis.json can set this, and the compiled pattern runs on the PTY
// hot path, where a nested quantifier would be a ReDoS against the event loop.
// A broken pattern must also fail at LOAD time rather than inside a data handler.
workingLine: z
.string()
.min(1)
.max(400)
.refine((src) => {
try {
new RegExp(src);
return true;
} catch {
return false;
}
}, 'workingLine must be a valid regular expression'),
.refine(
(src) => compileVersionRegex(src) !== null,
'workingLine must be a regex compileVersionRegex() accepts: at most 200 characters, no nested quantifiers'
),
})
.strict()
.optional(),
+1 -1
View File
@@ -429,7 +429,7 @@ const CODEX: CliEntry = {
// `Working (2m 49s • esc to interrupt)` above it while a turn runs. It animates no
// braille spinner, and it never prints `esc to interrupt` at rest, so that phrase
// alone separates a running turn from an idle one.
workDetect: { promptGlyph: '›', workingLine: 'esc to interrupt' },
workDetect: { promptGlyph: '›', workingLine: '[Ee]sc to interrupt' },
transcript: 'codex-rollout',
altScreen: 'strip-full',
echo: { policy: 'predict', anchor: { kind: 'cursor' }, predictProfile: 'codex' },
+6 -7
View File
@@ -106,6 +106,7 @@ import {
import { DEFAULT_TMUX_HISTORY_LIMIT } from './config/terminal-history.js';
import { EXEC_TIMEOUT_MS } from './config/exec-timeout.js';
import { getCli } from './config/cli-registry/registry.js';
import { compileVersionRegex } from './config/cli-registry/patterns.js';
import { resolveSessionCliVersion } from './utils/cli-resolver.js';
import {
buildInteractiveArgs,
@@ -2469,13 +2470,11 @@ export class Session extends EventEmitter {
private _workingLinePattern(): RegExp {
if (this._workingLineRe === undefined) {
const src = getCli(this.mode)?.capabilities.workDetect?.workingLine;
// The schema validates `workingLine` at load time, so a throw here would mean a
// registry that never loaded. Falling back beats taking the session down.
try {
this._workingLineRe = src ? new RegExp(src) : CLAUDE_WORKING_LINE_PATTERN;
} catch {
this._workingLineRe = CLAUDE_WORKING_LINE_PATTERN;
}
// Same guard the schema applies, not a second opinion: `compileVersionRegex()` is
// what keeps a nested quantifier out of this pattern, and this one runs on the PTY
// hot path. It returns null rather than throwing, and Claude's pattern is the
// fallback every session used before the registry carried one.
this._workingLineRe = (src ? compileVersionRegex(src) : null) ?? CLAUDE_WORKING_LINE_PATTERN;
}
return this._workingLineRe;
}
+29
View File
@@ -17,6 +17,7 @@
import { describe, it, expect } from 'vitest';
import { CliEntrySchema } from '../src/config/cli-registry/schema.js';
import { compileVersionRegex } from '../src/config/cli-registry/patterns.js';
import { STOCK_CLIS } from '../src/config/cli-registry/stock.js';
import type { CliEntry } from '../src/config/cli-registry/types.js';
@@ -75,6 +76,34 @@ describe('strictness', () => {
});
});
describe('workDetect.workingLine is guarded like every other config regex', () => {
it('rejects a nested quantifier', () => {
expectRejected((e) => {
(e.capabilities as Record<string, unknown>).workDetect = { promptGlyph: '>', workingLine: '(a+)+b' };
}, 'this pattern is compiled once and then run against every accumulated PTY chunk, so catastrophic backtracking here freezes the event loop for the whole server');
});
it('rejects a source longer than compileVersionRegex() will compile', () => {
expectRejected((e) => {
(e.capabilities as Record<string, unknown>).workDetect = { promptGlyph: '>', workingLine: 'a'.repeat(201) };
}, 'the schema must not accept a pattern the runtime will then refuse to compile, or the CLI silently falls back to the Claude pattern');
});
it('rejects a pattern that is not a regex at all', () => {
expectRejected((e) => {
(e.capabilities as Record<string, unknown>).workDetect = { promptGlyph: '>', workingLine: '([unclosed' };
}, 'a broken pattern must fail at LOAD time, not inside the PTY data handler');
});
it('accepts both shipped patterns unchanged', () => {
for (const entry of STOCK_CLIS) {
const src = entry.capabilities.workDetect?.workingLine;
if (!src) continue;
expect(compileVersionRegex(src), `${entry.id} declares a workingLine the guard refuses`).not.toBeNull();
}
});
});
describe('no shell text can reach the command line', () => {
it('rejects a literal carrying shell metacharacters', () => {
for (const evil of ['pi; rm -rf /', 'pi && curl evil.sh', 'pi`whoami`', 'pi $(id)', 'pi | tee', 'pi > /etc/x']) {
+9
View File
@@ -307,6 +307,15 @@ describe("codex's work-detection descriptor", () => {
expect(new RegExp(codex!.workingLine).test(CODEX_FINISHED)).toBe(false);
});
it('matches the footer case-insensitively on the E', () => {
// Characterised on codex-cli 0.152.1, which prints a lowercase `esc`. A future
// version capitalising it would otherwise make the whole fix silently inert:
// the pane would simply never look like it was working.
expect(new RegExp(codex!.workingLine).test(CODEX_WORKING.replace('esc to interrupt', 'Esc to interrupt'))).toBe(
true
);
});
it('names the glyph Codex actually draws on its composer row', () => {
expect(CODEX_COMPOSER_REPAINT).toContain(codex!.promptGlyph);
expect(CODEX_WORKING).toContain(codex!.promptGlyph);