Uh oh!
There was an error while loading. Please reload this page.
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.
Trott
commented
Jul 28, 2020
Welcome @FeelyChau and thanks for the pull request. |
FeelyChau
commented
Jul 28, 2020
@Trott Thanks for review. constChildProcess=require('child_process');constcp=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 |
yorkie
commented
Jul 28, 2020
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.
Uh oh!
There was an error while loading. Please reload this page.
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. |
aduh95
commented
Nov 14, 2020
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