child_process: document the error when cwd does not exist - #34505
child_process: document the error when cwd does not exist#34505FeelyChau wants to merge 2 commits into
Conversation
If the option cwd does not exist, the error ENOENT is the same as the error emitted when the command does not exist, it's confusing.
|
Welcome @FeelyChau and thanks for the pull request. |
|
@Trott Thanks for review. const ChildProcess = require('child_process');
const cp = ChildProcess.spawn('/usr/local/bin/node', [], { cwd: 'path_not_exists' });
cp.on('error', (err) => {
console.log(err);
})the output: Error: spawn /usr/local/bin/node ENOENT
at Process.ChildProcess._handle.onexit (internal/child_process.js:267:19)
at onErrorNT (internal/child_process.js:469:16)
at processTicksAndRejections (internal/process/task_queues.js:84:21) {
errno: 'ENOENT',
code: 'ENOENT',
syscall: 'spawn /usr/local/bin/node',
path: '/usr/local/bin/node',
spawnargs: []
}The error is not specific. node/deps/uv/src/unix/process.c Line 342 in 3751662 |
|
I did address a bug related in mine until I found the call to |
aduh95
left a comment
There was a problem hiding this comment.
I'm good with this personally.
|
This issue/PR was marked as stalled, it will be automatically closed in 30 days. If it should remain open, please leave a comment explaining why it should remain open. |
|
Landed in bea9857 |
If the option cwd does not exist, the error ENOENT is the same as the error emitted when the command does not exist, it's confusing. PR-URL: #34505 Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
If the option cwd does not exist, the error ENOENT is the same as the error emitted when the command does not exist, it's confusing. PR-URL: #34505 Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
If the option cwd does not exist, the error ENOENT is the same as the error emitted when the command does not exist, it's confusing. PR-URL: #34505 Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
If the option cwd does not exist, the error ENOENT is the same as the error emitted when the command does not exist, it's confusing. PR-URL: #34505 Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
If the option cwd does not exist, the error ENOENT is the same as the error emitted when the command does not exist, it's confusing. PR-URL: #34505 Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
… 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>
If the option cwd does not exist, the error
ENOENTis the same as the error emitted when the command does not exist, it's confusing.Checklist