diff --git a/docs/cli-registry.md b/docs/cli-registry.md index e72eefd8..4f2aaafb 100644 --- a/docs/cli-registry.md +++ b/docs/cli-registry.md @@ -51,6 +51,9 @@ interface CliEntry { // that ended waiting for workers it will resume from // .modelDetect?: { screenLine, screenLines? } // (where this CLI's own chrome names the model it runs: SessionState.displayModel) + // .launchDefaults?: { [launchParam]: settingsKey } + // (synced App Settings that seed a LOCAL launch's params the caller left unset; + // codex's model and reasoning effort, via src/web/launch-defaults.ts) overlays: CliOverlays; // remote-SSH / Docker pane commands, credential store } ``` diff --git a/src/config/cli-registry/schema.ts b/src/config/cli-registry/schema.ts index 90760fb1..5159a203 100644 --- a/src/config/cli-registry/schema.ts +++ b/src/config/cli-registry/schema.ts @@ -15,7 +15,7 @@ import { z } from 'zod'; import { compileVersionRegex, countCaptureGroups, TOKEN_PATTERNS } from './patterns.js'; import { isKnownLauncherProfile, isKnownSetenvProfile } from './profiles.js'; -import type { McpConfigFormat, ModelConfigResolverName } from './types.js'; +import type { LaunchDefaultSettingKey, McpConfigFormat, ModelConfigResolverName } from './types.js'; /** A bare CLI id: lowercase, starts with a letter, at most 24 chars. Also used as a CSS/URL token. */ const cliId = z @@ -409,6 +409,17 @@ const capabilitiesSchema = z 'rejectWords has nothing to filter without a screenLine' ) .optional(), + // Launch param -> synced App Settings key. The values are a closed enum, like + // configResolver: a clis.json override names one of the settings this build knows + // how to validate, never an arbitrary key. Params are checked against the declared + // ones in the superRefine below. + launchDefaults: z + .record( + z.string(), + z.enum(['codexModel', 'codexReasoningEffort'] as const satisfies readonly LaunchDefaultSettingKey[]) + ) + .refine((v) => Object.keys(v).length >= 1 && Object.keys(v).length <= 8, 'launchDefaults takes 1 to 8 params') + .optional(), privilegedParams: z .array( z @@ -627,6 +638,30 @@ export const CliEntrySchema = z } }); + // Same silent-no-op class again: a launch default for a param the entry never declared + // would be filled into the config object and then read by nothing. And without a + // `legacyConfigField` the entry's params are read off the request body itself, where a + // filled `model` would be a different field (claude's per-session one), so refuse it. + const { launchDefaults } = entry.capabilities; + if (launchDefaults !== undefined) { + if (entry.launch.legacyConfigField === undefined) { + ctx.addIssue({ + code: 'custom', + message: 'launchDefaults needs launch.legacyConfigField to fill', + path: ['capabilities', 'launchDefaults'], + }); + } + for (const param of Object.keys(launchDefaults)) { + if (!declaredParams.has(param)) { + ctx.addIssue({ + code: 'custom', + message: `launchDefaults param "${param}" is not a declared launch param`, + path: ['capabilities', 'launchDefaults', param], + }); + } + } + } + const { setenvProfile } = entry.env; if (setenvProfile !== undefined && !isKnownSetenvProfile(setenvProfile)) { ctx.addIssue({ diff --git a/src/config/cli-registry/stock.ts b/src/config/cli-registry/stock.ts index 8126a7eb..35b042e9 100644 --- a/src/config/cli-registry/stock.ts +++ b/src/config/cli-registry/stock.ts @@ -671,6 +671,11 @@ const CODEX: CliEntry = { // popup or a bare line of prose does not have that shape. A footer without an effort // word (a model with no reasoning setting) is not read, and the session keeps its // last known or launch model. + // The effort words are built from CODEX_REASONING_EFFORTS, the same list the + // `reasoningEffort` launch param above admits, plus `default` (what codex prints when + // no effort is configured). A hand-kept copy once left out `ultra`, so a session at + // that level never named its model. Every word is plain letters, so the join adds no + // quantifier and only a few characters to the 200-character compileVersionRegex cap. // ⚠️ It is not always the LAST row. 0.162.0 (measured 2026-10-09) adds a hint row // under it at rest, ` ← for agents · ? for shortcuts` or ` ? for shortcuts`, and // drops it again while a prompt is being typed. With a one-row window the footer was @@ -681,9 +686,13 @@ const CODEX: CliEntry = { // and the `›` composer, and a forged footer-shaped transcript line is not followed by // an indented row, so it is not read. modelDetect: { - screenLine: String.raw`(?:^|\n) {2}([A-Za-z0-9][\w.:/@+-]{0,79}) (?:none|minimal|low|medium|high|xhigh|max|default) · [^\n]*(?:\n {2}[^\n]*)?$`, + screenLine: String.raw`(?:^|\n) {2}([A-Za-z0-9][\w.:/@+-]{0,79}) (?:${[...CODEX_REASONING_EFFORTS, 'default'].join('|')}) · [^\n]*(?:\n {2}[^\n]*)?$`, screenLines: 2, }, + // App Settings → Codex model / reasoning effort (synced), filled into a LOCAL launch's + // codexConfig wherever the caller left the field unset. Launch-only: nothing writes + // codex's own config.toml. Read by applyLaunchDefaults() in src/web/launch-defaults.ts. + launchDefaults: { model: 'codexModel', reasoningEffort: 'codexReasoningEffort' }, // Two columns, like claude's, measured on a live 0.154.0 answer: the `•`/`›`/`⚠` // markers sit in the gutter, prose continuations sit at 2, and a nested YAML block // the model wrote rendered at 2/4/6/8 for its own 0/2/4/6. Replayed at 100, 120, diff --git a/src/config/cli-registry/types.ts b/src/config/cli-registry/types.ts index 114b4c40..09cc3136 100644 --- a/src/config/cli-registry/types.ts +++ b/src/config/cli-registry/types.ts @@ -96,6 +96,14 @@ export type NewlineSequence = 'line-feed' | 'esc-enter'; /** The config readers `capabilities.modelDetect.configResolver` may name (src/model-config-resolvers.ts). */ export type ModelConfigResolverName = 'deepseek-route'; +/** + * The synced App Settings keys `capabilities.launchDefaults` may name (src/web/launch-defaults.ts). + * A closed list rather than any settings key, so a clis.json override cannot feed an + * arbitrary setting onto a command line; each name must also be a `SettingsUpdateSchema` + * key, which the resolver's typing enforces. + */ +export type LaunchDefaultSettingKey = 'codexModel' | 'codexReasoningEffort'; + /** The MCP config dialects `src/mcp-sync.ts` has an adapter for. */ export type McpConfigFormat = 'claude-json' | 'gemini-json' | 'codex-toml' | 'opencode-json' | 'antigravity-json'; @@ -529,6 +537,20 @@ export interface CliCapabilities { rejectWords?: string[]; configResolver?: ModelConfigResolverName; }; + /** + * Synced App Settings that seed this CLI's launch params when the caller left them unset, + * keyed by LAUNCH PARAM name (`{ model: 'codexModel' }`), never the legacy wire name; the + * resolver translates through `launch.legacyConfigAliases` like every other `param`. + * + * Filled into the entry's `launch.legacyConfigField` object at create time by + * `applyLaunchDefaults()` (src/web/launch-defaults.ts), which re-validates each value + * with `SettingsUpdateSchema` and never overwrites a value the caller sent. Which + * launches get it is the CALLER's decision (local ones only: never remote, Docker or a + * custom model endpoint). `schema.ts` refuses an undeclared param, and an entry without + * a `legacyConfigField`, whose params would otherwise be read off the request body itself. + * Absent = no launch defaults. + */ + launchDefaults?: Record; /** * Params a non-granted multi-user owner may not set freely, and what they are forced to. * Data-driven so a CUSTOM CLI's bypass flag is clampable exactly like codex's. diff --git a/src/web/codex-launch-defaults.ts b/src/web/codex-launch-defaults.ts deleted file mode 100644 index 43a059e9..00000000 --- a/src/web/codex-launch-defaults.ts +++ /dev/null @@ -1,35 +0,0 @@ -/** - * @fileoverview Launch-time defaults for Codex sessions. - * - * Resolves the synced App Settings `codexModel` / `codexReasoningEffort` into the - * `codexConfig` a launch uses, filling ONLY the fields the caller left unset. - * Persisted values are re-validated with `SettingsUpdateSchema`, so a hand-edited - * settings.json can never smuggle an unchecked value onto the codex command line. - * - * Scope is the caller's decision: the create and quick-start routes apply it to - * local launches only, never to remote, Docker or custom-endpoint launches. - * Nothing here writes Codex's own config files. - */ - -import type { CodexConfig } from '../types.js'; -import { SettingsUpdateSchema } from './schemas.js'; -import { readJsonConfig, SETTINGS_PATH } from './route-helpers.js'; - -/** Resolve launch-only defaults without changing Codex's own configuration files. */ -export async function resolveCodexLaunchDefaults( - config: CodexConfig | undefined, - customEndpoint = false -): Promise { - if (customEndpoint) return config; - const settings = await readJsonConfig>(SETTINGS_PATH, 'Codex launch defaults', {}); - const model = SettingsUpdateSchema.shape.codexModel.safeParse(settings.codexModel); - const effort = SettingsUpdateSchema.shape.codexReasoningEffort.safeParse(settings.codexReasoningEffort); - const defaultModel = model.success ? model.data || undefined : undefined; - const defaultEffort = effort.success ? effort.data || undefined : undefined; - if (!defaultModel && !defaultEffort) return config; - return { - ...config, - model: config?.model ?? defaultModel, - reasoningEffort: config?.reasoningEffort ?? defaultEffort, - }; -} diff --git a/src/web/launch-defaults.ts b/src/web/launch-defaults.ts new file mode 100644 index 00000000..3fc5a183 --- /dev/null +++ b/src/web/launch-defaults.ts @@ -0,0 +1,49 @@ +/** + * @fileoverview Launch-time defaults from synced App Settings, driven by registry data. + * + * A CLI entry declares `capabilities.launchDefaults` (launch param -> settings key; today + * only codex, `{ model: 'codexModel', reasoningEffort: 'codexReasoningEffort' }`), and + * `applyLaunchDefaults()` fills those settings into the entry's `launch.legacyConfigField` + * object, setting ONLY the fields the caller left unset. Persisted values are re-validated + * with `SettingsUpdateSchema`, so a hand-edited settings.json can never smuggle an + * unchecked value onto the command line. + * + * Scope is the caller's decision: the create and quick-start routes apply it to local + * launches only, never to remote, Docker or custom-endpoint launches. Nothing here writes + * a CLI's own config files. + */ + +import { getCli } from '../config/cli-registry/registry.js'; +import { SettingsUpdateSchema } from './schemas.js'; +import { readJsonConfig, SETTINGS_PATH } from './route-helpers.js'; + +/** + * Return `configs` with the launch defaults of `mode`'s registry entry filled into its + * legacy config object (e.g. `codexConfig`). Every other field of `configs` is passed + * through untouched, and `configs` itself comes back unchanged (same object) when the entry + * declares no defaults, `customEndpoint` is set, or no setting names a value. + */ +export async function applyLaunchDefaults( + mode: string, + configs: T, + customEndpoint = false +): Promise { + const entry = getCli(mode); + const declared = entry?.capabilities.launchDefaults; + const field = entry?.launch.legacyConfigField; + if (customEndpoint || !declared || field === undefined) return configs; + + const settings = await readJsonConfig>(SETTINGS_PATH, 'CLI launch defaults', {}); + const aliases = entry.launch.legacyConfigAliases ?? {}; + const current = (configs as Record)[field] as Record | undefined; + const defaults: Record = {}; + for (const [param, settingKey] of Object.entries(declared)) { + const parsed = SettingsUpdateSchema.shape[settingKey].safeParse(settings[settingKey]); + // '' is the settings' "leave it to the CLI" value, the same as unset. + const value = parsed.success ? parsed.data || undefined : undefined; + const wireKey = aliases[param] ?? param; + if (value !== undefined && (current?.[wireKey] ?? undefined) === undefined) defaults[wireKey] = value; + } + if (Object.keys(defaults).length === 0) return configs; + return { ...configs, [field]: { ...current, ...defaults } }; +} diff --git a/src/web/routes/session-routes.ts b/src/web/routes/session-routes.ts index 51a30dc9..8affbeb0 100644 --- a/src/web/routes/session-routes.ts +++ b/src/web/routes/session-routes.ts @@ -116,7 +116,7 @@ import { clampEnvOverridesForOwner } from '../../session-env-clamp.js'; import { enabledClis, getCli } from '../../config/cli-registry/registry.js'; import type { NewlineSequence } from '../../config/cli-registry/types.js'; import { resolveCliLaunchError } from '../../utils/cli-launcher.js'; -import { resolveCodexLaunchDefaults } from '../codex-launch-defaults.js'; +import { applyLaunchDefaults } from '../launch-defaults.js'; import { legacyConfigForMode } from '../../session-cli-registry-bridge.js'; import { isMultiUserMode } from '../../config/multiuser.js'; import { AUTH_COOKIE_NAME } from '../middleware/auth.js'; @@ -1150,8 +1150,10 @@ export function registerSessionRoutes( const globalNice = await ctx.getGlobalNiceConfig(); const modelConfig = await ctx.getModelConfig(); const mode = body.mode || 'claude'; - const launchCodexConfig = - mode === 'codex' && !remote ? await resolveCodexLaunchDefaults(body.codexConfig) : body.codexConfig; + // Synced App Settings launch defaults (capabilities.launchDefaults, codex's model and + // effort today) fill the CLI's own config object where the caller left it unset. + // Local launches only: a remote attach runs whatever the remote pane already runs. + const launchBody = remote ? body : await applyLaunchDefaults(mode, body); // Where a model override comes from is a capability, and the three answers are // genuinely different mechanisms: // 'flag' — the CLI takes --model, so read the value the caller sent @@ -1166,10 +1168,9 @@ export function registerSessionRoutes( const modelSource = getCli(mode)?.capabilities.model; const model = modelSource?.source === 'flag' - ? (legacyConfigForMode(mode, { ...body, codexConfig: launchCodexConfig } as unknown as Record< - string, - unknown - >)?.[modelSource.param ?? 'model'] as string | undefined) + ? (legacyConfigForMode(mode, launchBody as unknown as Record)?.[ + modelSource.param ?? 'model' + ] as string | undefined) : modelSource?.source === 'claude-settings-file' ? body.model || modelConfig?.defaultModel || undefined : undefined; @@ -1186,12 +1187,12 @@ export function registerSessionRoutes( deepSeekConfig: gatedDeepSeekConfig, } = await _clampExternalCliBypassForOwner( owner, - launchCodexConfig, - body.geminiConfig, - body.antigravityConfig, - body.piConfig, - body.grokConfig, - body.deepSeekConfig + launchBody.codexConfig, + launchBody.geminiConfig, + launchBody.antigravityConfig, + launchBody.piConfig, + launchBody.grokConfig, + launchBody.deepSeekConfig ); const terminalHistoryConfig = await ctx.getTerminalHistoryConfig(); const session = new Session({ @@ -1204,14 +1205,14 @@ export function registerSessionRoutes( model, claudeMode: effectiveClaudeMode, allowedTools: claudeModeConfig.allowedTools, - openCodeConfig: mode === 'opencode' ? body.openCodeConfig : undefined, + openCodeConfig: mode === 'opencode' ? launchBody.openCodeConfig : undefined, codexConfig: mode === 'codex' ? gatedCodexConfig : undefined, geminiConfig: mode === 'gemini' ? gatedGeminiConfig : undefined, antigravityConfig: mode === 'antigravity' ? gatedAntigravityConfig : undefined, piConfig: mode === 'pi' ? gatedPiConfig : undefined, grokConfig: mode === 'grok' ? gatedGrokConfig : undefined, deepSeekConfig: mode === 'deepseek' ? gatedDeepSeekConfig : undefined, - ompConfig: resolveOmpConfigForCreate(mode, workingDir, body.ompConfig), + ompConfig: resolveOmpConfigForCreate(mode, workingDir, launchBody.ompConfig), resumeSessionId: validatedResumeId, envOverrides: await clampEnvOverridesForOwner(owner, body.envOverrides), effort: body.effort, @@ -3875,23 +3876,31 @@ export function registerSessionRoutes( // Apply global Nice priority config and model config from settings const niceConfig = await ctx.getGlobalNiceConfig(); const qsModelConfig = await ctx.getModelConfig(); - const qsLaunchCodexConfig = - mode === 'codex' && !remote && !docker - ? await resolveCodexLaunchDefaults(codexConfig, !!customModel) - : codexConfig; + // Synced App Settings launch defaults, as on the create path: local launches only, so + // never a remote or Docker case, and never a custom model endpoint launch. + const qsRequestConfigs = { + openCodeConfig, + codexConfig, + geminiConfig, + antigravityConfig, + piConfig, + grokConfig, + deepSeekConfig, + ompConfig, + }; + const qsLaunchConfigs = + remote || docker ? qsRequestConfigs : await applyLaunchDefaults(mode, qsRequestConfigs, !!customModel); + // ⚠️ The model is read from a bag WITHOUT ompConfig, as it always was here: quick-start + // has never taken omp's session model from ompConfig (the create path does). Kept as + // found rather than changed in passing. + const { ompConfig: qsLaunchOmpConfig, ...qsModelConfigs } = qsLaunchConfigs; // See the create path for why this is a capability rather than a mode ladder. const qsModelSource = getCli(mode)?.capabilities.model; const qsModel = qsModelSource?.source === 'flag' - ? (legacyConfigForMode(mode, { - openCodeConfig, - codexConfig: qsLaunchCodexConfig, - geminiConfig, - antigravityConfig, - piConfig, - grokConfig, - deepSeekConfig, - } as unknown as Record)?.[qsModelSource.param ?? 'model'] as string | undefined) + ? (legacyConfigForMode(mode, qsModelConfigs as unknown as Record)?.[ + qsModelSource.param ?? 'model' + ] as string | undefined) : qsModelSource?.source === 'claude-settings-file' ? qsModelConfig?.defaultModel || undefined : undefined; @@ -3907,16 +3916,16 @@ export function registerSessionRoutes( deepSeekConfig: qsGatedDeepSeekConfig, } = await _clampExternalCliBypassForOwner( owner, - qsLaunchCodexConfig, - geminiConfig, - antigravityConfig, - piConfig, - grokConfig, - deepSeekConfig + qsLaunchConfigs.codexConfig, + qsLaunchConfigs.geminiConfig, + qsLaunchConfigs.antigravityConfig, + qsLaunchConfigs.piConfig, + qsLaunchConfigs.grokConfig, + qsLaunchConfigs.deepSeekConfig ); const qsTerminalHistoryConfig = await ctx.getTerminalHistoryConfig(); const qsGatedEnvOverrides = await clampEnvOverridesForOwner(owner, envOverrides); - const qsResolvedOmpConfig = resolveOmpConfigForCreate(mode, resolvedCasePath, ompConfig); + const qsResolvedOmpConfig = resolveOmpConfigForCreate(mode, resolvedCasePath, qsLaunchOmpConfig); // Custom Model Endpoint Profiles, applied AT CREATE TIME (docs/custom-model-endpoints-plan.md) // rather than via the dedicated restart-in-place route (POST /api/sessions/:id/custom- @@ -4073,7 +4082,7 @@ export function registerSessionRoutes( claudeMode: qsEffectiveClaudeMode, allowedTools: qsClaudeModeConfig.allowedTools, owner, - openCodeConfig: mode === 'opencode' ? openCodeConfig : undefined, + openCodeConfig: mode === 'opencode' ? qsLaunchConfigs.openCodeConfig : undefined, codexConfig: mode === 'codex' ? qsGatedCodexConfig : undefined, geminiConfig: mode === 'gemini' ? qsGatedGeminiConfig : undefined, antigravityConfig: mode === 'antigravity' ? qsGatedAntigravityConfig : undefined, diff --git a/src/web/schemas.ts b/src/web/schemas.ts index c1938cfb..68f0e174 100644 --- a/src/web/schemas.ts +++ b/src/web/schemas.ts @@ -1406,14 +1406,14 @@ export const SettingsUpdateSchema = z /** * Tab layout, the arrangement of the tab list (Discussion #426). Display key * (per-device). + * 'classic' = one flat list in tab order, as before. The default. * 'state' = a row per state in the header strip (needs you, waiting, * working, idle; option C), sections in the flat side rail and - * the sidebar. The default. + * the sidebar. Opt-in. * 'case' = one cluster per case (option A): a labelled box in the strip, - * a section in the side rail and the sidebar. + * a section in the side rail and the sidebar. Opt-in. * 'ledger' = the flat list on an aligned column grid with a status bar - * per cell (option B). Header strip on desktop only. - * 'classic' = one flat list in tab order, as before. + * per cell (option B). Header strip on desktop only. Opt-in. */ tabArrangement: z.enum(['state', 'case', 'ledger', 'classic']).optional(), /** @@ -1457,8 +1457,9 @@ export const SettingsUpdateSchema = z * How the header draws its WS / CPU / MEM / plan-usage cluster. Display key * (per-device), desktop only (the cluster is hidden below 768px). * 'classic' = the bars and the 5H · 7D chip, as before - * 'compact' = two pills (WS/CPU/MEM, the plan windows), a ring beside every value - * 'tiles' = label over value with a bar underneath, no icons. The default. + * 'compact' = two pills (WS/CPU/MEM, the plan windows), a ring beside every value. + * The default. + * 'tiles' = label over value with a bar underneath, no icons */ headerStatsStyle: z.enum(['classic', 'compact', 'tiles']).optional(), showTokenCount: z.boolean().optional(), diff --git a/test/cli-registry-no-id-branching.test.ts b/test/cli-registry-no-id-branching.test.ts index 0c8cd1f1..e35faeca 100644 --- a/test/cli-registry-no-id-branching.test.ts +++ b/test/cli-registry-no-id-branching.test.ts @@ -33,6 +33,13 @@ * 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). */ @@ -67,47 +74,67 @@ const EXEMPT_FILES = new Set( ].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. + * 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 = { +const ALLOWED_BRANCHES: Record = { // --- Legacy Config plumbing (public wire shape, see the header) --- - "web/routes/session-routes.ts::mode === 'opencode'": 'legacy Config plumbing', - "web/routes/session-routes.ts::mode === 'codex'": 'legacy Config plumbing', - "web/routes/session-routes.ts::mode === 'gemini'": 'legacy Config plumbing', - "web/routes/session-routes.ts::mode === 'antigravity'": 'legacy Config plumbing', - "web/routes/session-routes.ts::mode === 'pi'": 'legacy Config plumbing', - "web/routes/session-routes.ts::mode === 'grok'": 'legacy Config plumbing', - "web/routes/session-routes.ts::mode === 'deepseek'": 'legacy Config plumbing', - "web/server.ts::mode === 'opencode'": 'legacy Config plumbing (session recovery)', - "web/server.ts::mode === 'codex'": 'legacy Config plumbing (session recovery)', - "web/server.ts::mode === 'gemini'": 'legacy Config plumbing (session recovery)', - "web/server.ts::mode === 'antigravity'": 'legacy Config plumbing (session recovery)', - "web/server.ts::mode === 'pi'": 'legacy Config plumbing (session recovery)', - "web/server.ts::mode === 'grok'": 'legacy Config plumbing (session recovery)', - "web/server.ts::mode === 'deepseek'": 'legacy Config plumbing (session recovery)', - "web/server.ts::mode === 'omp'": 'legacy Config plumbing (session recovery)', + "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'": + "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'": + '`--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', + '(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'": - 'an error message explaining why THIS mode in particular will never deliver a stop signal', - "web/routes/approval-routes.ts::mode === 'deepseek'": - 'the DeepSeek status bridge is the only non-claude source of approval items', - "cron/cron-service.ts::mode === 'claude'": 'cron launch handling, not yet generalised', - "cron/cron-service.ts::mode === 'shell'": 'cron launch handling, not yet generalised', - "web/routes/session-routes.ts::mode === 'claude'": 'docker case bookkeeping keyed on the claude conversation id', - "cli.ts::mode === 'shell'": 'a CLI-table label, not behaviour', + "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) --- // @@ -121,43 +148,58 @@ const ALLOWED_BRANCHES: Record = { // 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'": - 'deliberately mode-not-capability; pinned by deepseek-mode.test.ts', - "web/server.ts::mode !== 'claude'": + "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)', + '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'": 'TUI resume builds a claude --resume; claude-transcript-only by construction', - "tui/tui-render.ts::mode !== 'claude'": 'cosmetic: suppress the mode badge for the default mode', + "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'": - 'push buttons withheld where the answer route refuses keystrokes', + "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'": 'legacy Config plumbing (resolveOmpConfigForCreate)', + "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'": + "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'": 'scaffolded-case hooks (see the opencode entry)', - "web/routes/session-routes.ts::mode !== 'gemini'": 'scaffolded-case hooks (see the opencode entry)', - "web/routes/session-routes.ts::mode !== 'antigravity'": 'scaffolded-case hooks (see the opencode entry)', - "web/routes/session-routes.ts::mode !== 'pi'": 'scaffolded-case hooks (see the opencode entry)', - "web/routes/session-routes.ts::mode !== 'grok'": 'scaffolded-case hooks (see the opencode entry)', + '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. */ @@ -246,6 +288,43 @@ function scan(): { findings: Finding[]; filesScanned: number } { 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 @@ -293,12 +372,56 @@ describe('no CLI-id branching outside the stock catalog', () => { ).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. - 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([]); + // 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); + } }); }); diff --git a/test/cli-registry-schema.test.ts b/test/cli-registry-schema.test.ts index 7feb42cb..6424c6c8 100644 --- a/test/cli-registry-schema.test.ts +++ b/test/cli-registry-schema.test.ts @@ -291,6 +291,57 @@ describe('cross-field integrity', () => { }); }); +describe('capabilities.launchDefaults', () => { + /** Codex with its shipped launch defaults replaced by `launchDefaults` (or with them unchanged). */ + function codexWith(mutate: (entry: Record) => void): boolean { + const entry = baseEntry('codex'); + mutate(entry); + return CliEntrySchema.safeParse(entry).success; + } + const setDefaults = (value: unknown) => (e: Record) => { + (e.capabilities as Record).launchDefaults = value; + }; + + it('ships on codex alone, keyed by launch param', () => { + const declaring = STOCK_CLIS.filter((e) => e.capabilities.launchDefaults !== undefined).map((e) => e.id); + expect(declaring).toEqual(['codex']); + expect(codexWith(() => {})).toBe(true); + }); + + it('rejects a param the entry never declared', () => { + // Filled into the config object and then read by nothing: a silent no-op, like a + // privilegedParams clamp naming the wrong param. + expect(codexWith(setDefaults({ effort: 'codexReasoningEffort' }))).toBe(false); + }); + + it('rejects a settings key outside the closed list', () => { + // An override must not be able to pour an arbitrary setting onto a command line. + expect(codexWith(setDefaults({ model: 'claudeModel' }))).toBe(false); + expect(codexWith(setDefaults({ model: 'codexmodel' }))).toBe(false); + }); + + it('rejects an empty map', () => { + expect(codexWith(setDefaults({}))).toBe(false); + }); + + it('refuses an entry with no legacyConfigField to fill', () => { + // Without one the params are read off the request body itself, where `model` is + // claude's per-session field, not this CLI's. + expect( + codexWith((e) => { + delete (e.launch as Record).legacyConfigField; + }) + ).toBe(false); + // The same entry without launch defaults is fine: the refusal is about the pair. + expect( + codexWith((e) => { + delete (e.launch as Record).legacyConfigField; + delete (e.capabilities as Record).launchDefaults; + }) + ).toBe(true); + }); +}); + describe('the env allowlist cannot be widened by config', () => { it('requires a prefix to end with an underscore', () => { expectRejected((e) => { diff --git a/test/routes/session-routes-codex-defaults.test.ts b/test/routes/session-routes-codex-defaults.test.ts index 6a2ceb9c..f37fe671 100644 --- a/test/routes/session-routes-codex-defaults.test.ts +++ b/test/routes/session-routes-codex-defaults.test.ts @@ -7,18 +7,26 @@ import { createRouteTestHarness, type RouteTestHarness } from './_route-test-uti import { registerSessionRoutes } from '../../src/web/routes/session-routes.js'; import { registerSystemRoutes } from '../../src/web/routes/system-routes.js'; import { CASES_DIR, SETTINGS_PATH } from '../../src/web/route-helpers.js'; -import { resolveCodexLaunchDefaults } from '../../src/web/codex-launch-defaults.js'; +import { applyLaunchDefaults } from '../../src/web/launch-defaults.js'; import { buildCodexCommand } from '../../src/tmux-manager.js'; import { Session } from '../../src/session.js'; import { safeRmHomeTree } from '../mocks/index.js'; import { getDataDir } from '../../src/config/instance.js'; import { SettingsUpdateSchema } from '../../src/web/schemas.js'; +import { getCli } from '../../src/config/cli-registry/registry.js'; +import { STOCK_CLIS } from '../../src/config/cli-registry/stock.js'; +import type { CodexConfig } from '../../src/types.js'; vi.mock('../../src/utils/cli-launcher.js', async (importOriginal) => { const actual = await importOriginal(); return { ...actual, resolveCliLaunchError: vi.fn().mockResolvedValue(null) }; }); +/** The codexConfig a launch gets from the synced defaults alone (the caller sent `config`). */ +async function resolveCodexDefaults(config: CodexConfig | undefined, customEndpoint = false) { + return (await applyLaunchDefaults('codex', { codexConfig: config }, customEndpoint)).codexConfig; +} + describe('Codex launch defaults', () => { let harness: RouteTestHarness; const workingDir = join(homedir(), 'codex-default-test'); @@ -76,14 +84,45 @@ describe('Codex launch defaults', () => { { codexModel: 'bad;command', codexReasoningEffort: 'invalid' }, ]) { await writeFile(SETTINGS_PATH, JSON.stringify(settings)); - expect(await resolveCodexLaunchDefaults(undefined)).toBeUndefined(); - expect(buildCodexCommand(await resolveCodexLaunchDefaults(undefined))).toBe('codex'); + expect(await resolveCodexDefaults(undefined)).toBeUndefined(); + expect(buildCodexCommand(await resolveCodexDefaults(undefined))).toBe('codex'); } }); it('keeps defaults out of custom endpoint launches', async () => { const config = { model: 'local-model', animations: false }; - expect(await resolveCodexLaunchDefaults(config, true)).toBe(config); + expect(await resolveCodexDefaults(config, true)).toBe(config); + }); + + it('is driven by the registry: codex declares the defaults, and a CLI without them is untouched', async () => { + // The routes call applyLaunchDefaults for every mode and never ask which CLI it is, so + // what codex gets is exactly what its entry declares. + expect(getCli('codex')!.capabilities.launchDefaults).toEqual({ + model: 'codexModel', + reasoningEffort: 'codexReasoningEffort', + }); + for (const entry of STOCK_CLIS) { + if ((entry.id as string) === 'codex') continue; + expect(entry.capabilities.launchDefaults, entry.id).toBeUndefined(); + const configs = { codexConfig: undefined, geminiConfig: { model: 'g' }, piConfig: undefined }; + expect(await applyLaunchDefaults(entry.id, configs), entry.id).toBe(configs); + } + // Only the entry's own config object is filled; the rest of the bag passes through. + const bag = { codexConfig: { animations: false }, geminiConfig: { model: 'g' }, name: 'n' }; + const filled = await applyLaunchDefaults('codex', bag); + expect(filled).toEqual({ + codexConfig: { animations: false, model: 'gpt-6.1', reasoningEffort: 'high' }, + geminiConfig: { model: 'g' }, + name: 'n', + }); + expect(filled.geminiConfig).toBe(bag.geminiConfig); + expect(bag.codexConfig).toEqual({ animations: false }); + // A field the caller sent is never overwritten, one at a time. + expect(await resolveCodexDefaults({ model: 'mine' })).toEqual({ model: 'mine', reasoningEffort: 'high' }); + expect(await resolveCodexDefaults({ reasoningEffort: 'low' })).toEqual({ + model: 'gpt-6.1', + reasoningEffort: 'low', + }); }); it('does not record unused defaults for a Docker quick-start', async () => { @@ -150,7 +189,7 @@ describe('Codex launch defaults', () => { expect((await put({ codexReasoningEffort: 'bogus' })).statusCode).toBe(400); expect((await put({ codexModel: 'bad;command' })).statusCode).toBe(400); expect((await put({ codexModel: '', codexReasoningEffort: '' })).statusCode).toBe(200); - expect(await resolveCodexLaunchDefaults(undefined)).toBeUndefined(); + expect(await resolveCodexDefaults(undefined)).toBeUndefined(); } finally { await system.app.close(); } diff --git a/test/session-display-model.test.ts b/test/session-display-model.test.ts index 10b293ae..1cef731c 100644 --- a/test/session-display-model.test.ts +++ b/test/session-display-model.test.ts @@ -25,6 +25,7 @@ import { sanitizeModelName, } from '../src/session-display-model.js'; import { IDLE_SILENCE_MS } from '../src/session-activity.js'; +import { CODEX_REASONING_EFFORTS } from '../src/types/session.js'; const detectOf = (mode: string) => getCli(mode)!.capabilities.modelDetect!; const DSH = compileVersionRegex(detectOf('deepseek').screenLine)!; @@ -223,6 +224,20 @@ describe('readScreenModel', () => { expect(readScreenModel(codexPane(null, [], ' ← for agents · ? for shortcuts'), CODEX, CODEX_ROWS)).toBeUndefined(); }); + it("reads codex's model at every reasoning effort Codeman can launch it with, ultra included", () => { + // `ultra` is offered by the codexReasoningEffort App Setting and codex's own /model + // picker; a hand-kept effort list in the pattern once left it out, so an ultra + // session's header never named its model. The footer prints the lowercase level. + const ultra = codexPane(' GPT-6-Astra ultra · ~/codeman-cases/testcase', [], ' ? for shortcuts'); + expect(readScreenModel(ultra, CODEX, CODEX_ROWS)).toBe('GPT-6-Astra'); + for (const effort of [...CODEX_REASONING_EFFORTS, 'default']) { + const atRest = codexPane(` GPT-6-Astra ${effort} · ~/codeman-cases/testcase`, [], ' ? for shortcuts'); + expect(readScreenModel(atRest, CODEX, CODEX_ROWS), effort).toBe('GPT-6-Astra'); + const typing = codexPane(` GPT-6-Astra ${effort} · ~/codeman-cases/testcase`); + expect(readScreenModel(typing, CODEX, CODEX_ROWS), effort).toBe('GPT-6-Astra'); + } + }); + it('never takes a transcript line shaped like the footer', () => { // The agent printed a line exactly like each CLI's footer, and the real footer is // hidden (a dsh status bar switched off; a codex popup over its last row). The