Skip to content

Commit f3f7808

Browse files
edobryminsky-ai[bot]
edobry
and
minsky-ai[bot]
authored
fix(mt#3397): Detect a deleted workspace cwd instead of crashing with a misleading ENOENT
## Summary A driven session whose workspace directory was deleted out from under it showed the operator a crash card reading **`Failed to start claude: ENOENT: no such file or directory, posix_spawn 'claude'`** — which points at the binary and the PATH, when the missing thing was the directory. Every subsequent resume re-crashed the same way, indefinitely. Node causes the ambiguity and documents it rather than fixing it: for `options.cwd`, *"if given, but the path does not exist, the child process emits an `ENOENT` error and exits immediately. `ENOENT` is also emitted when the command does not exist."* Pre-validating the cwd was proposed in [nodejs/node#11520](nodejs/node#11520) and declined in favour of the doc change ([PR #34505](nodejs/node#34505)) — the check belongs to the caller. This PR is that caller-side check. ## What was actually wrong `"unrecoverable"` has named **deleted cwd** as one of its cases since mt#3038 (`driven-session-host.ts`, the status docblock). Nothing ever checked for it. The only classifier was: ```ts const resumable = row.harnessSessionId !== null; ``` so a row whose workspace had been deleted stayed `"reconnecting"`, got resumed on every operator action and client reconnect, and crashed identically each time — a designed terminal state that no code path could reach. Observed 2026-07-30 on driven session `9f172ae8`: generations 1 and 2 (18:59, 19:33) resumed fine; the workspace was deleted; generations 3 and 4 (20:03, 20:36) failed with the ENOENT above. An in-window control from the same daemon at 19:48 spawned a driven session with an existing cwd and no error, which rules out PATH. ## Key changes - **`probeSpawnCwd(cwd)`** — three-valued (`present` / `missing` / `unknown`). `"unknown"` (a permission error, an I/O error, an unresponsive mount) fails **open**: the caller spawns anyway and lets the real error surface. Only a definitive `ENOENT`/`ENOTDIR`, or a path that exists but is not a directory, returns `"missing"` — because `"missing"` retires a conversation permanently, and a transiently unreadable workspace must not do that. - **`startDrivenSession` / `resumeDrivenSession`** preflight the cwd and produce a terminal `unrecoverable` record instead of spawning. The resume path uses `registry.replace`, so an open socket is swapped and redials onto the terminal state; the actuator generation is not incremented, because no actuator was created. - **Boot reconciliation and `orchestrateDrivenSessionResume`** classify a deleted cwd as non-resumable and write the verdict back (the existing mt#3269 path), so it is not re-derived every boot. The resume path checks before taking the cross-process resume lock and before the orphan-cleanup kill — both are wasted work for a session that cannot return. - **ENOENT disambiguation** for anything that still escapes the preflight (the directory vanishing between the check and the spawn): cwd-gone is `unrecoverable` and emits `minsky_unrecoverable`; otherwise the message says the binary was not found and names PATH. - **The WS layer** skips its synthetic `minsky_unrecoverable` frame when the replayed event log already carries one. The `unrecoverableReason` deliberately says the **workspace** is gone, not the work: the harness transcript survives both the actuator's death and the workspace's deletion. What is lost is resuming *in place* — `claude --resume` needs the original cwd. ### Persisting this verdict (a judgment call worth noting) mt#3269 deliberately persists only verdicts that provably cannot change, and explicitly declines to persist a staleness judgment. The deleted-cwd verdict is persisted too, on the grounds that it is an observed fact rather than a policy call, and that a Minsky workspace path is a session id — once deleted, nothing recreates that directory. The precision that makes it safe is `probeSpawnCwd`'s `"unknown"` branch, which leaves such rows `reconnecting`. ## Testing `Execution evidence:` ``` $ bun test --preload ./tests/setup.ts --timeout=15000 \ src/cockpit/driven-session-host.test.ts \ src/cockpit/driven-session-launch-persistence.test.ts \ src/cockpit/driven-session-ws.test.ts \ src/cockpit/routes/driven-sessions.test.ts \ src/cockpit/entity-thread-launch.test.ts \ src/cockpit/principal-channel-actuator.test.ts \ src/cockpit/web/lib/driven-session-accumulator.test.ts 210 pass 0 fail 495 expect() calls Ran 210 tests across 7 files. [1171.00ms] ``` Per-acceptance-test coverage, using the spec's numbering: - **AT1** — `resumeDrivenSession — missing cwd (mt#3397, acceptance test 1)`: no `spawnFn` call, record is `unrecoverable`, reason contains the missing path, same `localId` / `harnessSessionId` / generation. Plus a sibling asserting subscribers are swapped. - **AT2** — `boot reconciliation PERSISTS the deleted-cwd verdict so it is not re-derived forever`, and `orchestrateDrivenSessionResume writes the verdict back, and never takes the resume lock`. - **AT3** — `boot reconciliation classifies a linked row with a deleted cwd as unrecoverable` (non-null `harnessSessionId`, so it is the case the old classifier passed through). - **AT4** — `spawn ENOENT disambiguation (mt#3397, acceptance test 4)`: cwd-intact reports a PATH problem; cwd-vanished is `unrecoverable` and emits `minsky_unrecoverable`; a non-ENOENT error is untouched. - **AT5** — `a deleted-workspace session renders unrecoverable naming the path — never the crash card` (accumulator), plus a WS test that the unrecoverable frame is not duplicated. Also: `probeSpawnCwd` unit tests (present / missing / exists-but-is-a-file), and a `startDrivenSession` pair covering the no-spawn path and the state-change observer that persists the verdict. Full gated suite (`bun scripts/run-tests-gated.ts`) passes in this session. Typecheck clean across all 5 projects; ESLint 0 errors / 0 warnings. ### Test-fixture change worth a reviewer's eye Seven cockpit suites previously spawned into fabricated paths (`/tmp/x`, `/tmp/scratch`, `/repo/checkout`, …). With the preflight in place those would not have failed loudly — they would have quietly taken the missing-cwd branch and gone on asserting against records that never spawned. Each suite now creates a real `mkdtemp` directory. The files that gained real-fs usage carry a scoped `eslint-disable` for `custom/no-real-fs-in-tests`: the preflight's contract *is* the real filesystem, so there is nothing to inject through the code path under test. ## Live verification Ran against the real filesystem in the session workspace — the created-then-deleted directory is the same shape as the incident: ``` probe(workspace, still present) = present probe(workspace, now deleted) = missing probe(this live session dir) = present probe(a file, not a directory) = missing reason -> deleted cwd — the workspace directory /var/folders/.../mt3397-live-JnmTPj no longer exists, so this conversation cannot be resumed in place (its transcript is unaffected) unguarded spawn into deleted cwd -> ENOENT: no such file or directory, posix_spawn 'claude' ``` The last line is the unguarded behavior this PR replaces, reproduced on demand: Node naming the binary for a directory problem, with `claude` present on PATH throughout. Deploy verification: no deploy-surface files are touched (no `infra/**`, Dockerfile, `railway.json`, `deploy.config.ts`, or deploy workflow). The change is cockpit daemon source; it takes effect on the operator's next cockpit rebuild, which is a build/install step rather than a deploy. ## Related Filed while landing this: **mt#3406** — two pre-commit steps (`lint-staged` at 28.1s/30s, repo-wide ESLint at 119.5s/120s) are running at the edge of their timeouts and report a timeout as if the author's diff were broken. Independent of this change, but it blocked this PR five times. Co-Authored-By: minsky-ai[bot] <minsky-ai[bot]@users.noreply.github.com>
2 parents 2c63f54 + 99fb548 commit f3f7808

10 files changed

Lines changed: 788 additions & 76 deletions

‎src/cockpit/driven-session-host.test.ts‎

Lines changed: 250 additions & 23 deletions
Large diffs are not rendered by default.

‎src/cockpit/driven-session-host.ts‎

Lines changed: 179 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,8 @@
4646

4747
import { spawn as nodeSpawn } from "child_process";
4848
import { randomUUID } from "crypto";
49+
import { statSync } from "fs";
50+
import { stat } from "fs/promises";
4951
import { PassThrough } from "stream";
5052
import { log } from "@minsky/shared/logger";
5153
import {
@@ -137,6 +139,105 @@ function permissionModeArgs(mode: PermissionMode): string[] {
137139
/** The genuine binary this host spawns. Never anything from `@anthropic-ai/*`. */
138140
export const CLAUDE_BINARY = "claude";
139141

142+
// ---------------------------------------------------------------------------
143+
// Spawn-cwd preflight (mt#3397)
144+
// ---------------------------------------------------------------------------
145+
146+
/**
147+
* Verdict of {@link probeSpawnCwd}. Deliberately THREE-valued: `"unknown"` is
148+
* not merged into `"missing"`, because the two carry opposite consequences —
149+
* `"missing"` retires a conversation permanently, `"unknown"` must not.
150+
*/
151+
export type CwdProbeResult = "present" | "missing" | "unknown";
152+
153+
/**
154+
* Check whether a spawn cwd is a directory that exists.
155+
*
156+
* Why this exists: Node reports a missing `options.cwd` as an `ENOENT` naming
157+
* the COMMAND — `ENOENT: no such file or directory, posix_spawn 'claude'` —
158+
* indistinguishable from the binary being absent from PATH. Node documents this
159+
* ("If given, but the path does not exist, the child process emits an ENOENT
160+
* error and exits immediately. ENOENT is also emitted when the command does not
161+
* exist") and deliberately declined to pre-validate the cwd itself
162+
* (nodejs/node#11520 → doc-only PR nodejs/node#34505), leaving the check to the
163+
* caller. This is that caller-side check.
164+
*
165+
* `"unknown"` (a permission error, an I/O error, an unresponsive network mount)
166+
* fails OPEN — the caller spawns anyway and lets the real error surface. Only a
167+
* definitive ENOENT/ENOTDIR, or a path that exists but is not a directory,
168+
* returns `"missing"`, because `"missing"` is what marks a conversation
169+
* unrecoverable FOREVER. A transiently unreadable workspace must never retire a
170+
* conversation the principal may still want.
171+
*/
172+
export function probeSpawnCwd(cwd: string): CwdProbeResult {
173+
try {
174+
return classifyCwdStat(statSync(cwd).isDirectory());
175+
} catch (err) {
176+
return classifyCwdProbeError(err, cwd);
177+
}
178+
}
179+
180+
/**
181+
* Async twin of {@link probeSpawnCwd}, for callers that are already async.
182+
*
183+
* PR #2452 R1 (BLOCKING): boot reconciliation probes every non-terminal row, so
184+
* a synchronous `statSync` there can stall the daemon's event loop for as long
185+
* as the slowest path takes to answer — unbounded on an unresponsive network
186+
* mount. The two spawn paths keep the sync probe deliberately: they are
187+
* synchronous by contract (`startDrivenSession` hands the caller a session id
188+
* without awaiting the child) and they are about to block on `spawn` anyway, so
189+
* making them async would ripple through four call sites to remove a stat that
190+
* immediately precedes a process launch. The loops get this one instead.
191+
*/
192+
export async function probeSpawnCwdAsync(cwd: string): Promise<CwdProbeResult> {
193+
try {
194+
const stats = await stat(cwd);
195+
return classifyCwdStat(stats.isDirectory());
196+
} catch (err) {
197+
return classifyCwdProbeError(err, cwd);
198+
}
199+
}
200+
201+
function classifyCwdStat(isDirectory: boolean): CwdProbeResult {
202+
return isDirectory ? "present" : "missing";
203+
}
204+
205+
/**
206+
* Shared error classification for both probes.
207+
*
208+
* The `"unknown"` branch logs at WARN (PR #2452 R1): failing open is the right
209+
* behavior, but doing it silently means an operator seeing a session stuck in
210+
* `reconnecting` has no way to find out that the probe could not read the
211+
* workspace. The errno is the diagnostic — EACCES reads very differently from
212+
* EIO or a hung mount's ETIMEDOUT.
213+
*/
214+
function classifyCwdProbeError(err: unknown, cwd: string): CwdProbeResult {
215+
const code = (err as NodeJS.ErrnoException).code;
216+
if (code === "ENOENT" || code === "ENOTDIR") return "missing";
217+
log.warn(
218+
`[driven-session] could not determine whether ${cwd} exists (${code ?? "no errno"}) — ` +
219+
`treating it as possibly-present so the conversation is not retired on a transient fault`
220+
);
221+
return "unknown";
222+
}
223+
224+
/**
225+
* The `unrecoverableReason` for a session whose workspace is gone.
226+
*
227+
* Deliberately says the WORKSPACE is gone, not that the work is: a driven
228+
* session's conversation lives in the harness's own on-disk transcript, which
229+
* survives both the actuator's death and the workspace's deletion (memory
230+
* mem#669 — "the process died" is NOT "the work is gone"). What is lost is the
231+
* ability to RESUME in place: `claude --resume` needs the original cwd, both to
232+
* run in and because the harness keys its transcript directory off that path.
233+
*/
234+
export function missingCwdReason(cwd: string): string {
235+
return (
236+
`deleted cwd — the workspace directory ${cwd} no longer exists, so this conversation ` +
237+
`cannot be resumed in place (its transcript is unaffected)`
238+
);
239+
}
240+
140241
/**
141242
* The documented headless invocation (mt#2750 spec Context — Claude Code
142243
* headless docs, code.claude.com/docs/en/headless): `-p` is required for
@@ -789,6 +890,32 @@ export function startDrivenSession(opts: StartDrivenSessionOptions): StartDriven
789890
opts.mcpConfig === undefined ? buildDrivenSessionMcpConfig(opts.cwd) : opts.mcpConfig;
790891
const argv = buildDrivenSessionArgs(permissionMode, opts.model, mcpConfig);
791892

893+
// mt#3397 — cwd preflight. Spawning into a directory that does not exist
894+
// fails with an ENOENT that NAMES THE BINARY (see probeSpawnCwd), so without
895+
// this check the operator reads "Failed to start claude" and goes looking at
896+
// their PATH. Terminal-and-registered rather than thrown: the caller
897+
// (POST /api/driven-session, the WS resume path) still gets a record back,
898+
// and the state-change observer persists the verdict like any other.
899+
if (probeSpawnCwd(opts.cwd) === "missing") {
900+
const reason = missingCwdReason(opts.cwd);
901+
log.error(`[driven-session] not spawning ${command} — ${reason}`);
902+
const record = buildReconnectingDrivenSessionRecord({
903+
localId: opts.localId ?? randomUUID(),
904+
harnessSessionId: null,
905+
cwd: opts.cwd,
906+
permissionMode,
907+
taskId: opts.taskId ?? null,
908+
minskySessionId: opts.minskySessionId ?? null,
909+
status: "unrecoverable",
910+
unrecoverableReason: reason,
911+
actuatorGeneration: 0,
912+
startedAt: new Date().toISOString(),
913+
});
914+
registry.register(record);
915+
notifyStateChange(record, opts.onStateChange);
916+
return { record };
917+
}
918+
792919
log.info(
793920
`[driven-session] spawning ${command} ${redactMcpConfigForLog(argv)} ` +
794921
`(cwd=${opts.cwd}, permissionMode=${permissionMode})`
@@ -895,8 +1022,27 @@ function wireChildProcess(
8951022
});
8961023

8971024
proc.on("error", (err: Error) => {
1025+
// mt#3397 — an ENOENT here is ambiguous by Node's own design: it means
1026+
// EITHER the binary is not on PATH OR the cwd does not exist, and its
1027+
// message names the binary in both cases. The preflight in
1028+
// startDrivenSession/resumeDrivenSession catches the ordinary
1029+
// missing-cwd case before we ever get here, so reaching this branch with a
1030+
// missing cwd means the directory vanished BETWEEN the preflight and the
1031+
// spawn — rare, but real, and still unrecoverable rather than crashed.
1032+
const isEnoent = (err as NodeJS.ErrnoException).code === "ENOENT";
1033+
if (isEnoent && probeSpawnCwd(record.cwd) === "missing") {
1034+
const reason = missingCwdReason(record.cwd);
1035+
record.status = "unrecoverable";
1036+
record.unrecoverableReason = reason;
1037+
log.error(`[driven-session] spawn failed for ${record.localId} — ${reason}`);
1038+
appendEvent(record, { type: "minsky_unrecoverable", reason });
1039+
notifyStateChange(record, opts.onStateChange);
1040+
return;
1041+
}
8981042
record.status = "crashed";
899-
record.crashError = `Failed to start ${command}: ${err.message}`;
1043+
record.crashError = isEnoent
1044+
? `Failed to start ${command}: not found — is '${command}' on this process's PATH? (${err.message})`
1045+
: `Failed to start ${command}: ${err.message}`;
9001046
log.error(`[driven-session] spawn error for ${record.localId}: ${err.message}`);
9011047
appendEvent(record, {
9021048
type: "minsky_error",
@@ -1021,6 +1167,32 @@ export function resumeDrivenSession(opts: ResumeDrivenSessionOptions): StartDriv
10211167
mcpConfig
10221168
);
10231169

1170+
// mt#3397 — same cwd preflight as startDrivenSession, and the path the
1171+
// originating incident actually took: a workspace deleted out from under a
1172+
// live conversation left every resume attempt crashing with an ENOENT that
1173+
// named `claude`. `registry.replace` (not `register`) so the old record's
1174+
// subscribers get the swap signal and redial onto the terminal state; the
1175+
// generation is NOT incremented, because no new actuator was created.
1176+
if (probeSpawnCwd(previous.cwd) === "missing") {
1177+
const reason = missingCwdReason(previous.cwd);
1178+
log.error(`[driven-session] not resuming ${previous.localId} — ${reason}`);
1179+
const record = buildReconnectingDrivenSessionRecord({
1180+
localId: previous.localId,
1181+
harnessSessionId: previous.harnessSessionId,
1182+
cwd: previous.cwd,
1183+
permissionMode: previous.permissionMode,
1184+
taskId: previous.taskId,
1185+
minskySessionId: previous.minskySessionId,
1186+
status: "unrecoverable",
1187+
unrecoverableReason: reason,
1188+
actuatorGeneration: previous.actuatorGeneration,
1189+
startedAt: previous.startedAt,
1190+
});
1191+
registry.replace(previous.localId, record);
1192+
notifyStateChange(record, opts.onStateChange);
1193+
return { record };
1194+
}
1195+
10241196
log.info(
10251197
`[driven-session] resuming ${command} ${redactMcpConfigForLog(argv)} (localId=${previous.localId}, ` +
10261198
`harnessSessionId=${previous.harnessSessionId}, generation=${previous.actuatorGeneration + 1}, cwd=${previous.cwd})`
@@ -1104,8 +1276,12 @@ export interface ReconnectingRecordInput {
11041276
}
11051277

11061278
/**
1107-
* Build a placeholder `DrivenSessionRecord` for a persisted row loaded at
1108-
* daemon boot (RFC minimal-first-slice step 2) — registered into the
1279+
* Build a placeholder `DrivenSessionRecord` — one with no live actuator behind
1280+
* it. Two callers: a persisted row loaded at daemon boot (RFC
1281+
* minimal-first-slice step 2, the original use), and the mt#3397 cwd preflight,
1282+
* which produces a terminal `unrecoverable` record INSTEAD of spawning. Both
1283+
* want the same thing: a well-formed record whose `proc` is inert. Registered
1284+
* into the
11091285
* in-memory registry as `"reconnecting"` (or `"unrecoverable"`, for a
11101286
* persisted row already known to be unresumable) WITHOUT spawning anything.
11111287
* The domain-layer caller (../driven-session-launch.ts) is responsible for

0 commit comments

Comments
 (0)