mirror of
https://github.com/Ark0N/Codeman.git
synced 2026-10-05 23:19:43 +02:00
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
This commit is contained in:
+41
-17
@@ -10,6 +10,8 @@
|
||||
import { v4 as uuidv4 } from 'uuid';
|
||||
import { readFile } from 'node:fs/promises';
|
||||
import { statSync, realpathSync } from 'node:fs';
|
||||
import { getCli } from '../config/cli-registry/registry.js';
|
||||
import { resolveCliLaunchError } from '../utils/cli-launcher.js';
|
||||
import { Session } from '../session.js';
|
||||
import { applyWorkspaceHooks } from '../hooks-config.js';
|
||||
import { SseEvent } from '../web/sse-events.js';
|
||||
@@ -58,9 +60,26 @@ export function clampCronExternalCliConfigs(
|
||||
ownerGranted: boolean
|
||||
): { geminiConfig: GeminiConfig | undefined; piConfig: PiConfig | undefined } {
|
||||
if (ownerGranted) return { geminiConfig: undefined, piConfig: undefined };
|
||||
|
||||
// A cron job carries no per-CLI config at all, so ONLY the materialize-when-absent params
|
||||
// can apply here — an only-if-sent clamp has nothing to clamp. Reading them off the
|
||||
// registry rather than naming gemini and pi means a future CLI whose bare spawn is unsafe
|
||||
// is covered the moment its entry says so, instead of silently missing this path.
|
||||
const entry = getCli(mode);
|
||||
const aliases = entry?.launch.legacyConfigAliases ?? {};
|
||||
const materialized: Record<string, unknown> = {};
|
||||
for (const { param, clampTo, materializeWhenAbsent } of entry?.capabilities.privilegedParams ?? []) {
|
||||
// Same registry-param → legacy-wire-field hop the HTTP clamp makes. Neither gemini's
|
||||
// `approvalMode` nor pi's `approveProjectTrust` is aliased today, so this changes nothing
|
||||
// now — but the two are DIFFERENT namespaces, and writing the raw param here would make
|
||||
// this path stop clamping the moment one of them gained an alias, silently.
|
||||
if (materializeWhenAbsent) materialized[aliases[param] ?? param] = clampTo;
|
||||
}
|
||||
const has = Object.keys(materialized).length > 0;
|
||||
const field = entry?.launch.legacyConfigField;
|
||||
return {
|
||||
geminiConfig: mode === 'gemini' ? { approvalMode: 'auto_edit' } : undefined,
|
||||
piConfig: mode === 'pi' ? { approveProjectTrust: false } : undefined,
|
||||
geminiConfig: has && field === 'geminiConfig' ? (materialized as GeminiConfig) : undefined,
|
||||
piConfig: has && field === 'piConfig' ? (materialized as PiConfig) : undefined,
|
||||
};
|
||||
}
|
||||
|
||||
@@ -387,7 +406,7 @@ export class CronService {
|
||||
// Section 6.3: re-resolve the owner's grant at FIRE time (it may have been revoked
|
||||
// since create). Gates shell/launchCommand AND clamps the external-CLI bypass below.
|
||||
const ownerGranted = await canUsernameRunPrivilegedCommands(job.owner);
|
||||
if ((job.agentType === 'shell' || job.launchCommand) && !ownerGranted) {
|
||||
if ((getCli(job.agentType)?.capabilities.privilegedCommandGate || job.launchCommand) && !ownerGranted) {
|
||||
return this.failRun(job, run, 'Owner lacks the can-bypass-permissions grant for shell/launchCommand jobs');
|
||||
}
|
||||
|
||||
@@ -395,23 +414,26 @@ export class CronService {
|
||||
let session: Session;
|
||||
try {
|
||||
const mode = job.agentType;
|
||||
// Same two-part availability gate the HTTP create paths run: `dsh` is a
|
||||
// profile LAUNCHER, so without this a job on a box with only the stock
|
||||
// web/headless profiles spawns a bare `dsh` that boots a profile unable
|
||||
// to drive a pane, and the prompt is typed into a logging server or a
|
||||
// dead pane instead of failing the run with the actionable message.
|
||||
if (mode === 'deepseek') {
|
||||
const { resolveDeepSeekLaunchError } = await import('../utils/deepseek-cli-resolver.js');
|
||||
const launchError = resolveDeepSeekLaunchError();
|
||||
if (launchError) return this.failRun(job, run, launchError);
|
||||
}
|
||||
// Refuse a launch the CLI cannot survive, rather than opening a dead pane. The
|
||||
// launcher CLIs answer with their own specific reason (for dsh: binary missing, no
|
||||
// pane-capable profile, or the named profile cannot drive a pane); ordinary CLIs
|
||||
// answer with the resolver's not-found message. Cron sends no per-CLI config, so
|
||||
// there is no caller-named target to report on.
|
||||
const cronLaunchError = await resolveCliLaunchError(mode);
|
||||
if (cronLaunchError) return this.failRun(job, run, cronLaunchError);
|
||||
const globalNice = await this.deps.getGlobalNiceConfig();
|
||||
const modelConfig = await this.deps.getModelConfig();
|
||||
const claudeModeConfig = await this.deps.getClaudeModeConfig();
|
||||
const effectiveClaudeMode = await resolveClaudeModeForUsername(claudeModeConfig.claudeMode, job.owner);
|
||||
// DeepSeek's model is a composition entry in the profile's config tree,
|
||||
// not a session flag — mirror the HTTP routes' exclusion.
|
||||
const model = mode !== 'shell' && mode !== 'deepseek' ? modelConfig?.defaultModel || undefined : undefined;
|
||||
// Cron carries no per-CLI config object, so the only model it can supply is the global
|
||||
// default — and only to a CLI that takes one that way. `capabilities.model` is the same
|
||||
// question the HTTP routes ask; the ladder it replaces named `shell` and `deepseek` by
|
||||
// hand and had to be edited in step with them (deepseek's model is a profile
|
||||
// composition entry, not a session flag).
|
||||
const model =
|
||||
getCli(mode)?.capabilities.model.source === 'claude-settings-file'
|
||||
? modelConfig?.defaultModel || undefined
|
||||
: undefined;
|
||||
// Section 6.3: materialize the safe default for a non-granted owner (see
|
||||
// clampCronExternalCliConfigs — cron sends no per-CLI config, so the CLI's own
|
||||
// spawn default is what would otherwise apply).
|
||||
@@ -560,7 +582,9 @@ export class CronService {
|
||||
private sendPromptWhenReady(sessionId: string, prompt: string, job: CronJob, run: CronJobRun): void {
|
||||
setImmediate(() => {
|
||||
const poll = async (): Promise<void> => {
|
||||
if (job.agentType !== 'shell') {
|
||||
// A shell pane is ready the moment it exists; an agent CLI has a TUI to paint
|
||||
// first. That is the `kind` the registry already records, not a fact about shell.
|
||||
if (getCli(job.agentType)?.kind !== 'shell') {
|
||||
for (let attempt = 0; attempt < CRON_READY_MAX_ATTEMPTS; attempt++) {
|
||||
await delay(500);
|
||||
const s = this.deps.sessions.get(sessionId);
|
||||
|
||||
Reference in New Issue
Block a user