Skip to content

child_process: document the error when cwd does not exist - #34505

Closed
FeelyChau wants to merge 2 commits into
nodejs:masterfrom
FeelyChau:child-process-spawn-doc
Closed

child_process: document the error when cwd does not exist#34505
FeelyChau wants to merge 2 commits into
nodejs:masterfrom
FeelyChau:child-process-spawn-doc

Conversation

@FeelyChau

@FeelyChau FeelyChau commented Jul 24, 2020

Copy link
Copy Markdown
Contributor

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.

Checklist

@nodejs-github-bot nodejs-github-bot added child_process Issues and PRs related to the child_process subsystem. doc Issues and PRs related to the documentations. labels Jul 24, 2020
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

Trott commented Jul 28, 2020

Copy link
Copy Markdown
Member

Welcome @FeelyChau and thanks for the pull request. ENOENT means a path (whether file, directory, or other) does not exist. The additional information included with the error should make it clear whether the issue is the directory path or the command path. I'm not really sure this is a good thing to add to the docs. The errors should be self-explanatory. But let's see if others think differently. @nodejs/documentation @nodejs/child_process

@FeelyChau

Copy link
Copy Markdown
Contributor Author

@Trott Thanks for review.
Here is the code:

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.
I think the reason is libuv throw the ENOENT only, so we could not know the specific reasons.
Look at:

uv__write_int(error_fd, UV__ERR(errno));

@yorkie

yorkie commented Jul 28, 2020

Copy link
Copy Markdown
Contributor

I did address a bug related in mine until I found the call to chdir() at libuv. So IMO the other developers may not have an idea about considering it's an error caused by the cwd path. The cwd in options usually means it's an optional argument, and it wouldn't throw the same error from the required arguments which may be a little bit confusing :)

@aduh95 aduh95 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm good with this personally.

@aduh95 aduh95 added author ready PRs that have at least one approval, no pending requests for changes, and a CI started. review wanted PRs that need reviews. labels Nov 8, 2020
Comment thread doc/api/child_process.md Outdated
@aduh95 aduh95 added stalled Issues and PRs that are stalled. and removed author ready PRs that have at least one approval, no pending requests for changes, and a CI started. labels Nov 11, 2020
@github-actions

Copy link
Copy Markdown
Contributor

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 aduh95 added author ready PRs that have at least one approval, no pending requests for changes, and a CI started. commit-queue Add this label to land a pull request using GitHub Actions. and removed stalled Issues and PRs that are stalled. commit-queue Add this label to land a pull request using GitHub Actions. labels Nov 12, 2020
@aduh95

aduh95 commented Nov 14, 2020

Copy link
Copy Markdown
Contributor

Landed in bea9857

aduh95 pushed a commit that referenced this pull request Nov 14, 2020
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>
@aduh95 aduh95 closed this Nov 14, 2020
codebytere pushed a commit that referenced this pull request Nov 22, 2020
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>
@codebytere codebytere mentioned this pull request Nov 22, 2020
BethGriggs pushed a commit that referenced this pull request Dec 9, 2020
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>
BethGriggs pushed a commit that referenced this pull request Dec 10, 2020
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>
@BethGriggs BethGriggs mentioned this pull request Dec 10, 2020
BethGriggs pushed a commit that referenced this pull request Dec 15, 2020
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>
edobry added a commit to edobry/minsky that referenced this pull request Jul 30, 2026
… 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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

author ready PRs that have at least one approval, no pending requests for changes, and a CI started. child_process Issues and PRs related to the child_process subsystem. doc Issues and PRs related to the documentations. review wanted PRs that need reviews.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants