fix(cron): scope the launch pre-flight to launcher CLIs, not every mode

CI caught three cron-service failures. Both are mine, from converting cron's
per-mode ladders to capability reads without checking what each ladder's scope
actually was.

**The pre-flight.** cron only ever pre-flighted `deepseek` — dsh is a profile
LAUNCHER, so "installed" is not "runnable" and a bare `dsh` can boot a profile
that cannot drive a pane. I replaced that with an unscoped
`resolveCliLaunchError(mode)`, which pre-flights EVERY mode, so a claude cron
job on a box with no claude binary now failed with "Claude CLI not found"
instead of reaching tmux-manager's own throw. Three tests assert the latter.
It is now gated on `discovery.launcherProfile !== undefined`, which is
byte-identical to the `mode === 'deepseek'` check it replaces and generalises to
the next launcher. The equivalent HTTP-route conversion was already scoped (to
`capabilities.external`, matching what that route has always pre-flighted); I
simply failed to carry the same reasoning across.

**The model.** cron's ladder was `mode !== 'shell' && mode !== 'deepseek'`, and
I read it as `capabilities.model.source === 'claude-settings-file'` — which is
the HTTP route's question, not cron's. There, every external CLI reads its model
from its own config object earlier in the chain, so only claude reaches the
global default; cron has no such config, so the same expression silently
narrowed the default model from eight modes to one. Now `!== 'none'`, which is
exactly the two entries the ladder excluded. Not caught by a test — found by
re-deriving each ladder's scope after the first failure.

Also names a fourth deliberate behaviour change in the changeset, found while
tracing these: `session.ts` carried a hand-written list of modes with no
direct-PTY fallback and OMP was missing from it, though CLAUDE.md's own text
says "all eight require tmux". `requiresMux` comes off the entry now, so an omp
session whose mux creation fails refuses instead of silently starting outside
tmux.

Verified by diffing failing tests BY NAME against an upstream/master baseline,
rather than by file as before — which is how the regression slipped through: the
three new failures landed inside a file already failing for unrelated
Windows-path reasons, and the aggregate count happened to collide.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WQkoi1cNegqVwZHgzx5SbJ
This commit is contained in:
Devvyn
2026-09-02 08:49:04 +08:00
co-authored by Claude Opus 5
parent 4830e662f9
commit 6acf0dea0f
2 changed files with 31 additions and 15 deletions
+6 -1
View File
@@ -28,7 +28,7 @@ env-prefix validation, the doctor's tool list, and each resolver's search direct
re-read the catalog, so a CLI enabled while the server is running moves every surface at once
rather than only the run menu.
Three user-visible changes, all small and all deliberate:
Four user-visible changes, all small and all deliberate:
- `probeDockerCliVersion()` derives the in-container binary name from the registry rather
than assuming it equals the mode name. Only Claude reaches that path today, so nothing was
@@ -38,6 +38,11 @@ Three user-visible changes, all small and all deliberate:
replaces simply omitted — its own comment said the rule was "every mode except shell", so
the two were an oversight from when those CLIs were added, and a remote session in either
mode reported no version at all.
- OMP now requires tmux like its seven siblings. `session.ts` carried a hand-written list of
modes with no direct-PTY fallback and omp was missing from it, even though CLAUDE.md's own
text says "all eight require tmux" — so an omp session whose mux creation failed silently
fell back to a direct PTY. `requiresMux` comes off the entry now, so the list cannot drift
from the rule again.
- `codeman doctor`'s CLI rows come from the registry, so Claude's install hint is now the
documented install command rather than a docs URL, five CLIs gain install hints they never
had, and the row order follows the catalog (Claude now sorts below tmux).
+25 -14
View File
@@ -414,26 +414,37 @@ export class CronService {
let session: Session;
try {
const mode = job.agentType;
// 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);
// A LAUNCHER CLI's binary is not its agent, so "installed" is not "runnable": without
// this, a job on a box carrying only dsh's 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 an actionable message.
//
// ⚠️ Scoped to `discovery.launcherProfile`, which is byte-identical to the
// `mode === 'deepseek'` check this replaces (dsh is the only launcher today) and
// generalises to the next one. Deliberately NOT every CLI: cron has never pre-flighted
// a merely-missing binary, and doing so replaces tmux-manager's own not-found throw
// ("Session launch failed") with a different message for claude and shell. An earlier
// draft of this line was unscoped and did exactly that — three cron tests caught it.
if (getCli(mode)?.discovery.launcherProfile !== undefined) {
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);
// 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).
// default — and only to a CLI that takes a model at all.
//
// ⚠️ `!== 'none'` is the faithful reading of the `mode !== 'shell' && mode !== 'deepseek'`
// ladder this replaces: those two are exactly the entries declaring `model.source: 'none'`
// (shell has no model; deepseek's is a profile composition entry, not a session flag).
// NOT `=== 'claude-settings-file'`, which is the HTTP route's question — there, every
// external CLI reads its model from its own config object earlier in the chain, so only
// claude reaches the global default. Cron has no such config, so the same expression
// means something different here.
const model =
getCli(mode)?.capabilities.model.source === 'claude-settings-file'
? modelConfig?.defaultModel || undefined
: undefined;
getCli(mode)?.capabilities.model.source !== 'none' ? 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).