Repository navigation
Commit 959ee31
fix(cli): os doctor tells a tool missing from PATH apart from one that ran and failed (#24493)
Fixes #24464
Clause-②: no. Basis: one failure class of three `os doctor` rows changes
its message and fix text. No accept set, package export, flag, row
status or exit code moves.
`os doctor` probes `pnpm -v`, `tsc -v` and `git --version`, and each
probe's `catch` reported every failure as "Not found". A `pnpm` that is
installed but fails when it runs was told to reinstall pnpm. This PR
tells "not on PATH" apart from "ran and failed". The first keeps today's
row. The second reports the child's own first error line, with a fix
that matches the cause.
## Before and after, measured
`os doctor` from the source entry (`tsx packages/cli/bin/run-dev.js
doctor`), in an empty `tmpdir()` project, run with `env -i` (only
`HOME`, `NO_COLOR=1` and the `PATH` named per case). Before =
`packages/cli/src/commands/doctor.ts` at the base `e11ae4b6b`. After =
this branch at `023dd79d0`. Only the probe rows are quoted. Every other
row was identical within each pair.
**1. No `pnpm` on PATH** (`PATH` = a directory holding only a `node`
symlink). stdout and stderr are byte-identical before and after. Exit 1
both times:
```text
✗ pnpm Not found
→ Install pnpm: npm install -g pnpm@10.28.1
⚠ TypeScript Not found in PATH
⚠ Git Not found
```
**2. A Corepack `pnpm` whose registry answers 429.** This is real
Corepack 0.34.0: shims from `corepack enable --install-directory DIR
pnpm`, a fresh `COREPACK_HOME`, and `COREPACK_NPM_REGISTRY` pointed at a
local server that answers every request with `429`. No external network
was reached. Exit 1 both times.
Before:
```text
✗ pnpm Not found
→ Install pnpm: npm install -g pnpm@10.28.1
```
After:
```text
✗ pnpm Error: Server answered with HTTP 429 when performing the request to http://127.0.0.1:44765/pnpm/latest; for troubleshooting help, see https://github.com/nodejs/corepack#troubleshooting
→ pnpm is installed; Corepack could not download the pnpm version it resolves, and reinstalling pnpm will not fix that. Check the network or proxy, or pin pnpm with a "packageManager" field in package.json
```
**3. A `pnpm` that fails for another reason.** A stub prints `
ERR_PNPM_UNSUPPORTED_ENGINE Unsupported environment (bad pnpm and/or
Node.js version)` to stderr and exits 1. Exit 1 both times. Before: the
same `Not found` and install fix as case 2. After:
```text
✗ pnpm ERR_PNPM_UNSUPPORTED_ENGINE Unsupported environment (bad pnpm and/or Node.js version)
→ pnpm is on PATH, but `pnpm -v` exited with code 1: run `pnpm -v` and read its error
```
**4. Control, a working toolchain**
(`PATH=/opt/node22/bin:/usr/bin:/bin`). stdout and stderr are
byte-identical before and after: `✓ pnpm Version 10.28.0`, `✓ TypeScript
Version 6.0.2`, `✓ Git git version 2.43.0`.
The exit code was 1 in all eight runs (the empty project also has no
`node_modules`, an `error` row). The `pnpm` row stays `error`, and the
`TypeScript` and `Git` rows stay `warning`, so no exit code moves. In
cases 2 and 3 the summary line under `❌ Some critical issues found`
changes with the fix, because it prints each error's fix. The child's
stderr is still echoed above the report: the probe's `execSync` call is
unchanged.
## The classifier, and why it is shaped this way
Measured on Node 22.22 with `/bin/sh` = dash, through the probe's own
call, `execSync(cmd, { encoding: 'utf-8' })`:
| world | `error.code` | `status` | first stderr line |
|---|---|---|---|
| no `pnpm` on PATH | none | 127 | `/bin/sh: 1: pnpm: not found` |
| `pnpm` on PATH, its `node` missing | none | 127 | `/usr/bin/env:
'node': No such file or directory` |
| `pnpm` on PATH, not executable | none | 127 | `/bin/sh: 1: pnpm:
Permission denied` |
| Corepack `pnpm`, registry unreachable | none | 1 |
`.../corepack.cjs:22051`, then `Error: Error when performing the request
to ...` |
| Corepack `pnpm`, registry answers 429 | none | 1 |
`.../corepack.cjs:22058`, then `Error: Server answered with HTTP 429
when performing the request to ...` |
| Corepack, `COREPACK_ENABLE_NETWORK=0` | none | 1 | `Network access
disabled by the environment; can't reach npm repository ...` |
- The command string runs through a shell, so a missing binary is never
Node's `ENOENT`.
- The shell's exit 127 does not tell the cases apart either: rows two
and three exit 127 with the tool present. The shell's words are
localised.
- So once a probe has failed, `classifyToolProbeFailure` asks the
question directly: is there a file named `pnpm` on the `PATH` the probe
ran with, where the shell would have looked? If not, the outcome is
`not-found`, and the row is today's, byte for byte. If so, the outcome
is `failed`.
- A `failed` row's message is the child's first meaningful line. That is
the first `...Error: ` line if there is one, because an uncaught Node
error opens with `FILE:LINE`, the source line and a caret. Otherwise it
is the first non-blank stderr line, then stdout's. With no output at
all, it reads `` `pnpm -v` exited with code N with no output ``.
- A file that is present but not executable counts as present. The shell
found it and refused it, and its "Permission denied" line is the
message.
- The lookup runs only after a probe has failed. A probe that answers is
untouched. If the lookup misses a tool, the row is today's "Not found",
so the lookup cannot make a row more wrong than it is now.
- The Corepack fix is chosen from Corepack's own error text, read from
Corepack 0.34.0's `fetch`. It fires on `when performing the request to`
together with `corepack` in the stderr (both the no-answer and the
HTTP-status shapes), or on `Network access disabled by the environment;
can't reach`. Any other failure gets the neutral fix.
**Windows.** `execSync` runs `cmd.exe /d /s /c "pnpm -v"`. A missing
`pnpm` there answers "'pnpm' is not recognized as an internal or
external command" (localised) with exit 1, the same code a failing
`pnpm.cmd` exits with. Neither the code nor the text can tell them
apart. `execFileSync('pnpm', ...)` without a shell does not find
`pnpm.cmd`, so dropping the shell is not an option either. The lookup's
`win32` branch therefore follows `cmd.exe`'s resolution:
- the current directory first, then each `PATH` entry, with `PATH` and
`PATHEXT` read case-insensitively;
- each `PATHEXT` extension in turn;
- never an extensionless file.
I could not run Windows here. Four unit cases drive that branch with
`platform: 'win32'` on Linux:
- a `pnpm.cmd` on `Path` is found;
- an extensionless `pnpm` is not counted (and the POSIX branch does
count it);
- the current directory is searched first;
- cmd's "is not recognized" with exit 1 is classified `not-found` when
no candidate exists, and `failed` when a `pnpm.cmd` does.
## Family census: `tsc -v` and `git --version`
Both moved to the same classifier. Nothing changes except the message
and the fix:
- `tsc -v` (`TypeScript`, `warning`). Not on PATH keeps `Not found in
PATH` / `Installed locally via pnpm`. On PATH and failing, it reports
the child's line with ``tsc is on PATH, but `tsc -v` exited with code 1:
run `tsc -v` and read its error``. One example is the npm `tsc` package,
which is not TypeScript: it prints "This is not the tsc command you are
looking for" and exits 1. The row is still a `warning`.
- `git --version` (`Git`, `warning`). Not on PATH keeps `Not found` /
`Install Git for version control`. On PATH and failing, it reports the
child's line with the neutral fix. One example is macOS's `/usr/bin/git`
without the developer tools, which prints `xcrun: error: invalid active
developer path ...`. The row is still a `warning`.
- No other `doctor.ts` check changed.
## Tests
`packages/cli/test/doctor-tool-probe-failure.test.ts` is new. It is in
the `unit` tier and imports no `child_process`. It has 17 cases. Each
row case runs the real probe: `pnpmCheck`, `typescriptCheck` and
`gitCheck` run `execSync` through the shell, as `os doctor` does. `PATH`
is a directory the test controls, either empty or holding one
`#!/bin/sh` stub that uses only shell builtins. Nothing else is on that
`PATH`, so no real `pnpm`, Corepack or registry is reachable.
- `pnpm`: not on PATH (today's row, asserted whole); a Corepack-shaped
429 failure (Corepack's error line, the Corepack fix); another failure
(its line, the neutral fix); present but exiting 127; present but not
executable; no output; and a working control.
- `tsc` and `git`: not on PATH (today's rows), and present but failing.
- The `win32` lookup cases above, `firstMeaningfulLine`, and
`isCorepackFetchFailure` (which stays false for a Corepack `Usage Error`
and for the request phrase with no `corepack` in the stderr).
Commands and results, all on `023dd79d0`:
- `pnpm --filter @objectstack/cli exec vitest run --maxWorkers=2
test/doctor-tool-probe-failure.test.ts`: exit 0, `Tests 17 passed (17)`.
- `pnpm --filter @objectstack/cli exec vitest run --project unit
--maxWorkers=2`: exit 0, `Test Files 312 passed (312)`, `Tests 4627
passed (4627)`. The `integration` tier is left to CI: this diff touches
no spawn entry and no integration-tier file.
- `pnpm --filter @objectstack/cli typecheck`: exit 0. The new test is in
the `tsconfig.test.json` program (`tsc --listFilesOnly` counts it).
- `pnpm --filter @objectstack/cli build`: exit 0.
- eslint over the two changed `.ts` files: exit 0, 0 errors, 0 warnings,
neither file ignored.
**Ablation (one-off, no permanent file).** The fix was committed first.
`scripts/ablation-replace.mjs` replaced `if (!isOnShellPath(binary,
options)) return { outcome: 'not-found' };` with `if (binary) return {
outcome: 'not-found' };`, which is always true here: today's catch-all.
The anchor went 1 to 0, the replacement 0 to 1, and the blob changed
(`7d2cb9411fc8` to `a59ad6a54cbc`). With that in place the new file gave
`Tests 8 failed | 9 passed (17)`. The 8 failures are exactly the "ran
and failed" cases (Corepack 429, other failure, exit 127, not
executable, no output, the `tsc` impostor, the `git` xcrun stub, and the
`win32` classification). The 9 passes are the not-found rows, the
control and the pure helpers. The restore was verified: blob equal to
HEAD and `git diff HEAD` empty. The test imports
`../src/commands/doctor` relatively, so it reads source, not `dist/`,
and no build leg was needed.
## Gates
- `node scripts/pm/dispatch-gates.mjs --commands`: 68 derived families.
Each was run with its exit code captured before any pipe. 66 exited 0 on
the first run. `check:dual-build-cjs-loads` and `check:i18n-coverage`
first exited 3 (`PREREQUISITE NOT MET`, unbuilt packages outside the
CLI's dependency closure). Re-run after those packages were built, both
exited 0.
- `dispatch-gates.mjs --ran`: `68 derived, 68 run, 0 NOT-MEASURED, 0
UNRUN`, exit 0.
- The 11 declared-wide families (`check:init-service-contract`,
`check:live-db-isolation`, `check:meta-type-normalized`,
`check:optional-error-sink`, `check:resume-authority-declared`,
`check:route-envelope`, `check:runner-env-posture`,
`check:settings-bind-window`, `check:startup-registry-verdict`,
`check:verify-stand-in`, `check:wildcard-fallthrough`): all exited 0.
- `pnpm lint` (`eslint . --no-inline-config`, the repository-wide
union), run after the final commit: `VERDICT command-exit 0`, no
findings printed, on `023dd79d0`.
## Acceptance notes
Noted, not filed. No carrier exists for any of these.
- The probes have no timeout. A `pnpm -v` that hangs would hang `os
doctor`. This was not observed: Corepack fails fast on a refused or
rate-limited registry in every run above.
- The pnpm install hint names `pnpm@10.28.1`, while this repository's
own `packageManager` is `pnpm@10.31.0`. The "not found" row was required
to stay unchanged, so the hint was not touched.
- `origin/main` moved four commits past this branch's base
(`4bd7215a1`). Those commits touch `packages/lint`, `packages/spec`,
`packages/plugins/plugin-security`, docs and an ADR, and nothing under
`packages/cli`. The branch was not merged with them before opening; CI's
merge ref covers the joint state.
---
_Generated by [Claude
Code](https://claude.ai/code/session_01SYEJqDL3NXxrJRBVqaMu9T)_
---------
Co-authored-by: Claude <noreply@anthropic.com>1 parent 8016a9d commit 959ee31
3 files changed
Lines changed: 458 additions & 50 deletions
File tree
- .changeset
- packages/cli
- src/commands
- test
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
2022 | 2022 | | |
2023 | 2023 | | |
2024 | 2024 | | |
| 2025 | + | |
| 2026 | + | |
| 2027 | + | |
| 2028 | + | |
| 2029 | + | |
| 2030 | + | |
| 2031 | + | |
| 2032 | + | |
| 2033 | + | |
| 2034 | + | |
| 2035 | + | |
| 2036 | + | |
| 2037 | + | |
| 2038 | + | |
| 2039 | + | |
| 2040 | + | |
| 2041 | + | |
| 2042 | + | |
| 2043 | + | |
| 2044 | + | |
| 2045 | + | |
| 2046 | + | |
| 2047 | + | |
| 2048 | + | |
| 2049 | + | |
| 2050 | + | |
| 2051 | + | |
| 2052 | + | |
| 2053 | + | |
| 2054 | + | |
| 2055 | + | |
| 2056 | + | |
| 2057 | + | |
| 2058 | + | |
| 2059 | + | |
| 2060 | + | |
| 2061 | + | |
| 2062 | + | |
| 2063 | + | |
| 2064 | + | |
| 2065 | + | |
| 2066 | + | |
| 2067 | + | |
| 2068 | + | |
| 2069 | + | |
| 2070 | + | |
| 2071 | + | |
| 2072 | + | |
| 2073 | + | |
| 2074 | + | |
| 2075 | + | |
| 2076 | + | |
| 2077 | + | |
| 2078 | + | |
| 2079 | + | |
| 2080 | + | |
| 2081 | + | |
| 2082 | + | |
| 2083 | + | |
| 2084 | + | |
| 2085 | + | |
| 2086 | + | |
| 2087 | + | |
| 2088 | + | |
| 2089 | + | |
| 2090 | + | |
| 2091 | + | |
| 2092 | + | |
| 2093 | + | |
| 2094 | + | |
| 2095 | + | |
| 2096 | + | |
| 2097 | + | |
| 2098 | + | |
| 2099 | + | |
| 2100 | + | |
| 2101 | + | |
| 2102 | + | |
| 2103 | + | |
| 2104 | + | |
| 2105 | + | |
| 2106 | + | |
| 2107 | + | |
| 2108 | + | |
| 2109 | + | |
| 2110 | + | |
| 2111 | + | |
| 2112 | + | |
| 2113 | + | |
| 2114 | + | |
| 2115 | + | |
| 2116 | + | |
| 2117 | + | |
| 2118 | + | |
| 2119 | + | |
| 2120 | + | |
| 2121 | + | |
| 2122 | + | |
| 2123 | + | |
| 2124 | + | |
| 2125 | + | |
| 2126 | + | |
| 2127 | + | |
| 2128 | + | |
| 2129 | + | |
| 2130 | + | |
| 2131 | + | |
| 2132 | + | |
| 2133 | + | |
| 2134 | + | |
| 2135 | + | |
| 2136 | + | |
| 2137 | + | |
| 2138 | + | |
| 2139 | + | |
| 2140 | + | |
| 2141 | + | |
| 2142 | + | |
| 2143 | + | |
| 2144 | + | |
| 2145 | + | |
| 2146 | + | |
| 2147 | + | |
| 2148 | + | |
| 2149 | + | |
| 2150 | + | |
| 2151 | + | |
| 2152 | + | |
| 2153 | + | |
| 2154 | + | |
| 2155 | + | |
| 2156 | + | |
| 2157 | + | |
| 2158 | + | |
| 2159 | + | |
| 2160 | + | |
| 2161 | + | |
| 2162 | + | |
| 2163 | + | |
| 2164 | + | |
| 2165 | + | |
| 2166 | + | |
| 2167 | + | |
| 2168 | + | |
| 2169 | + | |
| 2170 | + | |
| 2171 | + | |
| 2172 | + | |
| 2173 | + | |
| 2174 | + | |
| 2175 | + | |
| 2176 | + | |
| 2177 | + | |
| 2178 | + | |
| 2179 | + | |
| 2180 | + | |
| 2181 | + | |
| 2182 | + | |
| 2183 | + | |
| 2184 | + | |
| 2185 | + | |
| 2186 | + | |
| 2187 | + | |
| 2188 | + | |
| 2189 | + | |
| 2190 | + | |
| 2191 | + | |
| 2192 | + | |
| 2193 | + | |
| 2194 | + | |
| 2195 | + | |
| 2196 | + | |
| 2197 | + | |
| 2198 | + | |
| 2199 | + | |
| 2200 | + | |
| 2201 | + | |
| 2202 | + | |
| 2203 | + | |
| 2204 | + | |
| 2205 | + | |
| 2206 | + | |
| 2207 | + | |
| 2208 | + | |
| 2209 | + | |
| 2210 | + | |
| 2211 | + | |
| 2212 | + | |
| 2213 | + | |
| 2214 | + | |
| 2215 | + | |
| 2216 | + | |
| 2217 | + | |
| 2218 | + | |
| 2219 | + | |
| 2220 | + | |
| 2221 | + | |
| 2222 | + | |
2025 | 2223 | | |
2026 | 2224 | | |
2027 | 2225 | | |
| |||
2095 | 2293 | | |
2096 | 2294 | | |
2097 | 2295 | | |
2098 | | - | |
2099 | | - | |
2100 | | - | |
2101 | | - | |
2102 | | - | |
2103 | | - | |
2104 | | - | |
2105 | | - | |
2106 | | - | |
2107 | | - | |
2108 | | - | |
2109 | | - | |
2110 | | - | |
2111 | | - | |
2112 | | - | |
2113 | | - | |
2114 | | - | |
2115 | | - | |
2116 | | - | |
2117 | | - | |
2118 | | - | |
2119 | | - | |
2120 | | - | |
2121 | | - | |
2122 | | - | |
2123 | | - | |
2124 | | - | |
2125 | | - | |
2126 | | - | |
2127 | | - | |
2128 | | - | |
2129 | | - | |
2130 | | - | |
2131 | | - | |
| 2296 | + | |
| 2297 | + | |
| 2298 | + | |
| 2299 | + | |
| 2300 | + | |
2132 | 2301 | | |
2133 | 2302 | | |
2134 | 2303 | | |
| |||
2186 | 2355 | | |
2187 | 2356 | | |
2188 | 2357 | | |
2189 | | - | |
2190 | | - | |
2191 | | - | |
2192 | | - | |
2193 | | - | |
2194 | | - | |
2195 | | - | |
2196 | | - | |
2197 | | - | |
2198 | | - | |
2199 | | - | |
2200 | | - | |
2201 | | - | |
2202 | | - | |
2203 | | - | |
2204 | | - | |
| 2358 | + | |
| 2359 | + | |
2205 | 2360 | | |
2206 | 2361 | | |
2207 | 2362 | | |
| |||
0 commit comments