mirror of
https://github.com/Ark0N/Codeman.git
synced 2026-10-05 23:19:43 +02:00
Maintainer merge-time fixes for the three PRs that landed together on the session create / launch / persistence path. #514 findings (bot verdict merge-with-fixes): - minor, fixed: SessionState.model was published and persisted for every mode, so a codex/opencode cron session reported the app-wide Claude default it never ran on. toState() now emits it only where the new cliTakesSessionModel() holds (registry capability model.source === 'claude-settings-file', no CLI id branch). POST /api/sessions uses the same helper for its non-claude refusal, so refusal and publication cannot drift. Recovery then hands back undefined for other modes on its own. - nit, fixed: the `model` schema admitted a leading dash (and '.', '['). The first character must now be a letter or digit; still a subset of the registry's model-claude pattern, so nothing accepted is refused at launch. - nit, fixed (reject, the consistent choice): `model` with attachRemoteSession was silently dropped. Now a 400 INVALID_INPUT, as #514 does for non-claude CLIs and quick-start does for remote cases. advisorModel (#530) gets the same refusal there. effort and envOverrides keep their older silent ignore on that branch so no existing caller breaks. #515 finding (bot verdict merge, one nit): - nit, fixed: the types/session.ts @fileoverview described CodexConfig as (model, resumeSessionId); it now lists reasoningEffort, bypass, animations and renderMode too. Audit of the merged combination (not reviewed before): - The conflict resolutions in session.ts (toState), types/session.ts, reboot-restore-routes.ts, server.ts (restoreMuxSessions), CLAUDE.md and skills/codeman/reference/endpoints.md (+ plugin mirror) keep both sides correctly; nothing was lost or doubled. - A claude session with both `model` and `advisorModel` launches with `--model <id>` and ONE merged `--settings` JSON (ultracode + advisorModel, or advisorModel beside `--effort <level>`), on the tmux template (including the resume || new variant and with the statusLine exporter) and on the direct-PTY fallback. Both values (and effort) survive restoreMuxSessions onto a dead pane, a reboot restore into a fresh pane, and restartCli/dead-pane respawn via _buildRespawnPaneOptions. - quick-start and ralph-loop take no per-session `model` (matching #514's scope, POST /api/sessions only) and launch on the app-wide default, which toState now persists for claude, so recovery stays consistent. - No defect found in the combination beyond the findings above. Noted, not changed: advisorModel is still published for any mode a caller sends it with (launch-inert there; the UI and skill send it for claude only). Tests: test/session-model-recovery.test.ts pins the pair through both recovery shapes for effort ultracode/high/none, the recovery constructors' fields, the tmux-manager builder hop, and the codex/opencode/shell non-publication; test/advisor-model.test.ts pins the launch lines and a real direct-PTY Session's pty.spawn argv; the route test covers flag-shaped models, attach refusals and the published fields. Docs: SessionState.model docstring, the reboot-restore-registry header, the golden test comment and the CLAUDE.md model/advisor bullets. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
323 lines
14 KiB
TypeScript
323 lines
14 KiB
TypeScript
/**
|
|
* @fileoverview GOLDEN spawn-command pins for the CLI registry's argv engine.
|
|
*
|
|
* Every expectation here is a LITERAL STRING, deliberately. An earlier version of this work
|
|
* compared the engine against `buildSpawnCommand()` instead — which read as a strong parity
|
|
* proof right up until `buildSpawnCommand` was itself switched over to call the engine, at
|
|
* which point it was comparing the engine with itself and would have happily accepted any
|
|
* regression the two shared. Literals cannot rot that way: they were captured from the
|
|
* hand-written builders BEFORE those builders were removed, and they are now the only
|
|
* surviving record of what those builders emitted.
|
|
*
|
|
* ⚠️ If a change here makes one of these fail, the question is never "what is the new string?"
|
|
* It is "which real CLI invocation just changed, and is that intended?" A byte that moves in
|
|
* this file is a byte that moves in a command line Codeman executes.
|
|
*
|
|
* Coverage note: every mode with a launch spec is pinned, `grok` and `deepseek` included.
|
|
* Grok had no parity coverage at all in the first draft of the registry, and deepseek did not
|
|
* exist in it — the two modes most likely to be transcribed wrong were the two nothing
|
|
* checked.
|
|
*
|
|
* Port: none (pure function over registry data).
|
|
*/
|
|
|
|
import { describe, it, expect } from 'vitest';
|
|
import { getCli } from '../src/config/cli-registry/registry.js';
|
|
import { buildSpawnCommandFromRegistry, type SpawnBridgeOptions } from '../src/session-cli-registry-bridge.js';
|
|
import { CODEX_REASONING_EFFORTS } from '../src/types/session.js';
|
|
|
|
/** A fixed session id, so `--session-id` is stable across runs. */
|
|
const SID = '0f9c2b14-1111-2222-3333-444455556666';
|
|
|
|
function render(options: SpawnBridgeOptions): string | undefined {
|
|
const entry = getCli(options.mode);
|
|
if (!entry) throw new Error(`no registry entry for mode ${options.mode}`);
|
|
return buildSpawnCommandFromRegistry(entry, options);
|
|
}
|
|
|
|
/** Every claude case pins an explicit `claudeCliVersion` so the --name gate is deterministic. */
|
|
function claude(extra: Partial<SpawnBridgeOptions> = {}): string | undefined {
|
|
return render({ mode: 'claude', sessionId: SID, claudeCliVersion: null, ...extra });
|
|
}
|
|
|
|
describe('claude', () => {
|
|
it('defaults to skip-permissions plus a new session id', () => {
|
|
expect(claude()).toBe('claude --dangerously-skip-permissions --session-id "0f9c2b14-1111-2222-3333-444455556666"');
|
|
});
|
|
|
|
it('maps each permission mode', () => {
|
|
expect(claude({ claudeMode: 'auto' })).toBe(
|
|
'claude --permission-mode auto --session-id "0f9c2b14-1111-2222-3333-444455556666"'
|
|
);
|
|
expect(claude({ claudeMode: 'normal' })).toBe('claude --session-id "0f9c2b14-1111-2222-3333-444455556666"');
|
|
expect(claude({ claudeMode: 'allowedTools', allowedTools: 'Bash(git:*), Read' })).toBe(
|
|
'claude --allowedTools "Bash(git:*), Read" --session-id "0f9c2b14-1111-2222-3333-444455556666"'
|
|
);
|
|
});
|
|
|
|
it('renders a model as the quoted value of --model, even one that opens with a dash', () => {
|
|
// POST /api/sessions refuses a leading '-' in `model`, but the registry's `model-claude`
|
|
// pattern still admits one, so the builder must stay safe on its own: the value lands
|
|
// quoted, and Claude's option parser takes the word after `--model` as its value whatever
|
|
// it starts with, so it can never become a flag of its own.
|
|
expect(claude({ model: 'claude-fable-5-1' })).toBe(
|
|
'claude --dangerously-skip-permissions --session-id "0f9c2b14-1111-2222-3333-444455556666" --model "claude-fable-5-1"'
|
|
);
|
|
expect(claude({ model: '--dangerously-skip-permissions' })).toBe(
|
|
'claude --dangerously-skip-permissions --session-id "0f9c2b14-1111-2222-3333-444455556666" ' +
|
|
'--model "--dangerously-skip-permissions"'
|
|
);
|
|
});
|
|
|
|
it('resumes through a shell fallback to a fresh session', () => {
|
|
// The ` || ` is emitted by the ENGINE, not by config — no registry field can hold shell
|
|
// text. This pin is what proves the fallback chain still renders as one command line.
|
|
expect(claude({ resumeSessionId: 'abc-123-def' })).toBe(
|
|
'claude --dangerously-skip-permissions --resume "abc-123-def" || ' +
|
|
'claude --dangerously-skip-permissions --session-id "0f9c2b14-1111-2222-3333-444455556666"'
|
|
);
|
|
});
|
|
|
|
it('carries effort as a flag, and ultracode as a settings blob', () => {
|
|
expect(claude({ effort: 'max' })).toBe(
|
|
'claude --dangerously-skip-permissions --session-id "0f9c2b14-1111-2222-3333-444455556666" --effort \'max\''
|
|
);
|
|
expect(claude({ effort: 'ultracode' })).toBe(
|
|
'claude --dangerously-skip-permissions --session-id "0f9c2b14-1111-2222-3333-444455556666" ' +
|
|
'--settings \'{"ultracode":true}\''
|
|
);
|
|
});
|
|
|
|
it('gates --name on the CLI version, failing closed when it is unknown', () => {
|
|
const named = { sessionName: 'w1 alpha' };
|
|
expect(claude({ ...named, claudeCliVersion: '2.1.226' })).toBe(
|
|
'claude --dangerously-skip-permissions --session-id "0f9c2b14-1111-2222-3333-444455556666" --name "w1 alpha"'
|
|
);
|
|
expect(claude({ ...named, claudeCliVersion: '2.1.223' })).toBe(
|
|
'claude --dangerously-skip-permissions --session-id "0f9c2b14-1111-2222-3333-444455556666"'
|
|
);
|
|
// Unknown version satisfies NO gate. A version probe that fails must not silently
|
|
// upgrade behaviour.
|
|
expect(claude({ ...named, claudeCliVersion: null })).toBe(
|
|
'claude --dangerously-skip-permissions --session-id "0f9c2b14-1111-2222-3333-444455556666"'
|
|
);
|
|
});
|
|
});
|
|
|
|
describe('opencode', () => {
|
|
const oc = (openCodeConfig?: SpawnBridgeOptions['openCodeConfig']) =>
|
|
render({ mode: 'opencode', sessionId: SID, openCodeConfig });
|
|
|
|
it('spawns bare by default', () => {
|
|
expect(oc()).toBe('opencode');
|
|
});
|
|
|
|
it('reads its resume id through the legacy `continueSession` alias', () => {
|
|
expect(oc({ model: 'anthropic/claude', continueSession: 'ses_9' })).toBe(
|
|
'opencode --model anthropic/claude --session ses_9'
|
|
);
|
|
});
|
|
|
|
it('only forks an existing session', () => {
|
|
expect(oc({ continueSession: 'ses_9', forkSession: true })).toBe('opencode --session ses_9 --fork');
|
|
// --fork with nothing to fork from would be meaningless, so it drops out entirely.
|
|
expect(oc({ forkSession: true })).toBe('opencode');
|
|
});
|
|
});
|
|
|
|
describe('codex', () => {
|
|
const cx = (codexConfig?: SpawnBridgeOptions['codexConfig']) =>
|
|
render({ mode: 'codex', sessionId: SID, codexConfig });
|
|
|
|
it('spawns bare by default', () => {
|
|
expect(cx()).toBe('codex');
|
|
});
|
|
|
|
it('emits the bypass flag only when asked', () => {
|
|
expect(cx({ dangerouslyBypassApprovals: true })).toBe('codex --dangerously-bypass-approvals-and-sandbox');
|
|
expect(cx({ dangerouslyBypassApprovals: false })).toBe('codex');
|
|
});
|
|
|
|
it('sends animations as an explicit true/false config pair', () => {
|
|
expect(cx({ animations: true })).toBe('codex --config tui.animations=true');
|
|
expect(cx({ animations: false })).toBe('codex --config tui.animations=false');
|
|
});
|
|
|
|
it('resumes with a POSITIONAL subcommand, not a flag', () => {
|
|
expect(cx({ model: 'gpt-5', resumeSessionId: 'roll_42' })).toBe('codex --model gpt-5 resume roll_42');
|
|
});
|
|
|
|
it('sends reasoning effort as one model_reasoning_effort config value, for every level', () => {
|
|
for (const level of CODEX_REASONING_EFFORTS) {
|
|
expect(cx({ reasoningEffort: level })).toBe(`codex --config model_reasoning_effort=${level}`);
|
|
}
|
|
});
|
|
|
|
it('keeps reasoning effort ahead of the resume subcommand', () => {
|
|
expect(cx({ model: 'gpt-5', reasoningEffort: 'high', resumeSessionId: 'roll_42' })).toBe(
|
|
'codex --model gpt-5 --config model_reasoning_effort=high resume roll_42'
|
|
);
|
|
});
|
|
});
|
|
|
|
describe('gemini', () => {
|
|
const gm = (geminiConfig?: SpawnBridgeOptions['geminiConfig']) =>
|
|
render({ mode: 'gemini', sessionId: SID, geminiConfig });
|
|
|
|
it('defaults an absent approval mode to yolo', () => {
|
|
// ⚠️ This is the DEFAULT-IS-UNSAFE case the multi-user clamp has to MATERIALIZE a config
|
|
// for: sending no geminiConfig at all still yields yolo, so an only-if-sent clamp would
|
|
// miss it entirely. See test/routes/external-cli-bypass-clamp.test.ts.
|
|
expect(gm()).toBe('gemini --skip-trust --approval-mode yolo');
|
|
});
|
|
|
|
it('honours an explicit approval mode', () => {
|
|
expect(gm({ approvalMode: 'auto_edit' })).toBe('gemini --skip-trust --approval-mode auto_edit');
|
|
});
|
|
|
|
it('reads its resume id through the legacy `resumeSession` alias', () => {
|
|
expect(gm({ model: 'gemini-3-pro', resumeSession: 'conv.7' })).toBe(
|
|
'gemini --skip-trust --approval-mode yolo --model gemini-3-pro --resume conv.7'
|
|
);
|
|
});
|
|
});
|
|
|
|
describe('antigravity', () => {
|
|
const ag = (antigravityConfig?: SpawnBridgeOptions['antigravityConfig']) =>
|
|
render({ mode: 'antigravity', sessionId: SID, antigravityConfig });
|
|
|
|
it('runs `agy`, not `antigravity`', () => {
|
|
// The mode name is not the binary name. Assuming it was is a bug this registry fixes.
|
|
expect(ag()).toBe('agy');
|
|
});
|
|
|
|
it('emits its flags', () => {
|
|
expect(ag({ dangerouslySkipPermissions: true, model: 'gemini-3-pro' })).toBe(
|
|
'agy --dangerously-skip-permissions --model gemini-3-pro'
|
|
);
|
|
expect(ag({ resumeConversationId: 'conv-99' })).toBe('agy --conversation conv-99');
|
|
});
|
|
});
|
|
|
|
describe('pi', () => {
|
|
const pi = (piConfig?: SpawnBridgeOptions['piConfig']) => render({ mode: 'pi', sessionId: SID, piConfig });
|
|
|
|
it('spawns bare by default', () => {
|
|
expect(pi()).toBe('pi');
|
|
});
|
|
|
|
it('renders the full option set', () => {
|
|
expect(pi({ model: 'sonnet:high', provider: 'anthropic', thinking: 'xhigh' })).toBe(
|
|
'pi --model sonnet:high --provider anthropic --thinking xhigh'
|
|
);
|
|
});
|
|
|
|
it('treats project trust as a TRI-state', () => {
|
|
// Absent is a third state, not a synonym for false: it leaves pi to ask interactively.
|
|
expect(pi({ approveProjectTrust: true })).toBe('pi --approve');
|
|
expect(pi({ approveProjectTrust: false })).toBe('pi --no-approve');
|
|
expect(pi()).toBe('pi');
|
|
});
|
|
|
|
it('prefers an explicit session id over -c', () => {
|
|
expect(pi({ resumeSessionId: '0f9c2b14' })).toBe('pi --session 0f9c2b14');
|
|
expect(pi({ continueSession: true })).toBe('pi -c');
|
|
expect(pi({ continueSession: true, resumeSessionId: '0f9c2b14' })).toBe('pi --session 0f9c2b14');
|
|
});
|
|
});
|
|
|
|
describe('grok', () => {
|
|
const gk = (grokConfig?: SpawnBridgeOptions['grokConfig']) => render({ mode: 'grok', sessionId: SID, grokConfig });
|
|
|
|
it('spawns bare by default', () => {
|
|
expect(gk()).toBe('grok');
|
|
});
|
|
|
|
it('emits its bypass flag only when asked', () => {
|
|
expect(gk({ alwaysApprove: true, model: 'grok-4.5' })).toBe('grok --always-approve --model grok-4.5');
|
|
expect(gk({ alwaysApprove: false })).toBe('grok');
|
|
});
|
|
|
|
it('prefers an explicit resume id over --continue', () => {
|
|
expect(gk({ resumeSessionId: '0198f2b4' })).toBe('grok --resume 0198f2b4');
|
|
expect(gk({ continueSession: true })).toBe('grok --continue');
|
|
expect(gk({ continueSession: true, resumeSessionId: '0198f2b4' })).toBe('grok --resume 0198f2b4');
|
|
});
|
|
|
|
it('never puts a credential on the command line', () => {
|
|
// grok authenticates from XAI_API_KEY, pushed via `tmux setenv`. There is no --api-key
|
|
// arg in its launch spec and there must never be one: the command line is visible to
|
|
// every process on the box.
|
|
const cmd = gk({ alwaysApprove: true, model: 'grok-4.5' }) ?? '';
|
|
expect(cmd).not.toContain('key');
|
|
expect(cmd).not.toContain('token');
|
|
});
|
|
});
|
|
|
|
describe('deepseek', () => {
|
|
const ds = (deepSeekConfig?: SpawnBridgeOptions['deepSeekConfig']) =>
|
|
render({ mode: 'deepseek', sessionId: SID, deepSeekConfig });
|
|
|
|
it('launches a named profile', () => {
|
|
expect(ds({ profile: 'dsh-tui' })).toBe('dsh --profile dsh-tui');
|
|
});
|
|
|
|
it('prefers an explicit resume id over the bare --resume', () => {
|
|
expect(ds({ profile: 'p', resumeSessionId: 'sess_42' })).toBe('dsh --profile p --resume sess_42');
|
|
expect(ds({ profile: 'p', resumeSession: true })).toBe('dsh --profile p --resume');
|
|
});
|
|
|
|
it('never puts the permission mode on the command line', () => {
|
|
// dsh has no permission FLAG — the switch is the DSH_PERMISSION_MODE env var, exported
|
|
// via `tmux setenv`. If this ever renders as an argument, the multi-user clamp and the
|
|
// env-key drop are both looking at the wrong surface.
|
|
const cmd = ds({ profile: 'p', permissionMode: 'danger-full-access' }) ?? '';
|
|
expect(cmd).toBe('dsh --profile p');
|
|
expect(cmd).not.toContain('danger-full-access');
|
|
expect(cmd).not.toContain('permission');
|
|
});
|
|
});
|
|
|
|
describe('shell', () => {
|
|
it('renders no command at all', () => {
|
|
// `undefined` is the signal to fall back to local login-shell resolution, which varies
|
|
// per user's /etc/passwd entry and so cannot be templated. An empty string would be a
|
|
// command, and a wrong one.
|
|
expect(render({ mode: 'shell', sessionId: SID })).toBeUndefined();
|
|
});
|
|
});
|
|
|
|
describe('unsafe values are DROPPED, never escaped into the command', () => {
|
|
// The hand-written builders silently omitted an argument whose value failed its allowlist,
|
|
// rather than quoting it through. That is the behaviour being preserved: a rejected value
|
|
// must not reach the CLI in ANY form, because "quoted but present" still lets a caller
|
|
// steer the agent (a bogus --model, a traversal path as a session id).
|
|
it.each([
|
|
['claude model', { mode: 'claude' as const, model: 'opus`whoami`' }, 'opus'],
|
|
['claude resume id', { mode: 'claude' as const, resumeSessionId: '../../etc/passwd' }, 'passwd'],
|
|
[
|
|
'claude allowedTools',
|
|
{ mode: 'claude' as const, claudeMode: 'allowedTools' as const, allowedTools: 'Bash(x); rm -rf /' },
|
|
'rm',
|
|
],
|
|
])('%s', (_label, extra, forbidden) => {
|
|
const cmd = claude(extra) ?? '';
|
|
expect(cmd).not.toContain(forbidden);
|
|
expect(cmd).not.toContain('`');
|
|
expect(cmd).not.toContain(';');
|
|
});
|
|
|
|
it('drops an unsafe pi model without falling back to a different one', () => {
|
|
expect(render({ mode: 'pi', sessionId: SID, piConfig: { model: 'a`b' } })).toBe('pi');
|
|
});
|
|
|
|
it('refuses a deepseek profile that is not a single path segment', () => {
|
|
// A profile name is joined into a filesystem path as well as a shell line, so `../evil`
|
|
// has to fail the token pattern rather than be quoted. With no valid name and no default
|
|
// profile installed, the flag drops out entirely and dsh picks its own.
|
|
const cmd = render({ mode: 'deepseek', sessionId: SID, deepSeekConfig: { profile: '../evil' } }) ?? '';
|
|
expect(cmd).not.toContain('evil');
|
|
expect(cmd).not.toContain('..');
|
|
});
|
|
});
|