Skip to content

Commit 28b8d87

Browse files
committed
fix(cli): os doctor tells a tool missing from PATH apart from one that ran and failed
The pnpm, tsc and git probes answered "Not found" for every failure, so a Corepack-managed pnpm that could not download its version was told to reinstall pnpm. A failed probe now asks whether the tool is on the PATH it ran with (the shell's own lookup, PATHEXT on win32): absent keeps today's row, present reports the child's first error line with a fix that matches the cause. Statuses, and so the exit code, are unchanged. Claude-Session: https://claude.ai/code/session_01SYEJqDL3NXxrJRBVqaMu9T Co-authored-by: Claude <noreply@anthropic.com>
1 parent e11ae4b commit 28b8d87

3 files changed

Lines changed: 454 additions & 50 deletions

File tree

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
---
2+
'@objectstack/cli': patch
3+
---
4+
5+
fix(cli): `os doctor` tells a tool missing from PATH apart from a tool on PATH that failed when it ran
6+
7+
Clause-②: no
8+
9+
- **What was wrong.** `os doctor` runs `pnpm -v`, `tsc -v` and `git --version`, and reported every failure as "Not found". A `pnpm` that is installed but cannot start was reported `✗ pnpm Not found` with `Install pnpm: npm install -g pnpm@10.28.1`. One example is a Corepack-managed `pnpm` that cannot download the version it resolves: offline, behind a proxy, or with the registry answering 429. Reinstalling pnpm does not fix that.
10+
- **What changed.** When a probe fails, doctor checks whether the tool is on the PATH the probe ran with. This is the same lookup the shell does: `/bin/sh` walks `PATH`; on Windows, `cmd.exe` tries each `PATHEXT` extension in the current directory and then in each `PATH` entry. If the tool is not there, the row is unchanged. If it is there, the row's message is the tool's own first error line, such as Corepack's `Error: Server answered with HTTP 429 when performing the request to …`. The fix then matches the cause. For a Corepack download failure, it says to check the network or proxy, or to pin pnpm with a `"packageManager"` field in `package.json`. For any other failure, it says to run the command and read its error.
11+
- **Unchanged.** Each row keeps its status: `pnpm` is `error`, `TypeScript` and `Git` are `warning`. So doctor's exit code does not move. A tool that answers is reported as before. No package export or flag changes, and the report keeps its layout.

‎packages/cli/src/commands/doctor.ts‎

Lines changed: 205 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -2022,6 +2022,204 @@ export function installedPackageLedgerChecks(
20222022
return out;
20232023
}
20242024

2025+
// ─── Environment tool probes: "not on PATH" vs "ran and failed" (#24464) ──
2026+
//
2027+
// `pnpm -v`, `tsc -v` and `git --version` each ran inside a `try` whose
2028+
// `catch` answered "Not found" for EVERY failure. A tool that is on PATH and
2029+
// fails when it runs is a different fact, and the install fix does not fix
2030+
// it: a Corepack-managed `pnpm` that cannot fetch the version it resolves
2031+
// (offline, behind a proxy, a registry answering 429) was reported
2032+
// `✗ pnpm Not found` with `npm install -g pnpm@10.28.1`.
2033+
//
2034+
// How the failures show up — measured on Node 22.22 with `/bin/sh` = dash,
2035+
// through the probe's own call, `execSync(cmd, { encoding: 'utf-8' })`:
2036+
//
2037+
// world error.code status first stderr line
2038+
// no `pnpm` on PATH (none) 127 /bin/sh: 1: pnpm: not found
2039+
// `pnpm` on PATH, its `node` missing (none) 127 /usr/bin/env: 'node': No such file or directory
2040+
// `pnpm` on PATH, not executable (none) 127 /bin/sh: 1: pnpm: Permission denied
2041+
// Corepack `pnpm`, registry answers 429 (none) 1 ...corepack.cjs:22058 (then the source line,
2042+
// a caret, and `Error: Server answered with
2043+
// HTTP 429 when performing the request to ...`)
2044+
//
2045+
// The command string runs through a shell, so a missing binary is never
2046+
// Node's `ENOENT`, and the shell's 127 does not discriminate either: rows two
2047+
// and three exit 127 with the tool present. The shell's own words are
2048+
// localised, and on Windows `cmd.exe` answers "is not recognized as an
2049+
// internal or external command" with exit 1 — the code a failing `pnpm.cmd`
2050+
// exits with too. So once a probe has failed, the classifier asks the question
2051+
// itself: is there a file on the PATH the probe ran with that the shell would
2052+
// have found under that name? No ⇒ "not found", today's row unchanged. Yes ⇒
2053+
// it ran and failed, and the row reports the child's own first meaningful
2054+
// line. A lookup that misses answers today's row, so this can only be as
2055+
// wrong as the code it replaces, in that direction.
2056+
//
2057+
// Windows: `cmd.exe` resolves a bare name by trying each `PATHEXT` extension in
2058+
// the current directory and then in each `PATH` entry, and never runs an
2059+
// extensionless file, so on `win32` the lookup tries exactly those candidates.
2060+
// `execFileSync('pnpm', …)` without a shell is NOT a way round the shell
2061+
// here: it does not find `pnpm.cmd` on Windows. Not measured on a Windows
2062+
// host; the `win32` lookup is pinned in `test/doctor-tool-probe-failure.test.ts`.
2063+
2064+
/** How one environment tool probe ran. */
2065+
export type ToolProbe =
2066+
| { readonly outcome: 'ok'; readonly output: string }
2067+
| { readonly outcome: 'not-found' }
2068+
| {
2069+
readonly outcome: 'failed';
2070+
/** The child's first meaningful line ({@link firstMeaningfulLine}). */
2071+
readonly detail: string;
2072+
/** How it ended: `exited with code 1`, `was killed by SIGTERM`. */
2073+
readonly exit: string;
2074+
/** The child's whole stderr, which a cause-specific fix reads. */
2075+
readonly stderr: string;
2076+
};
2077+
2078+
/** What a probe runs with; each field defaults to this process's own. */
2079+
export interface ToolProbeOptions {
2080+
readonly env?: NodeJS.ProcessEnv;
2081+
readonly cwd?: string;
2082+
}
2083+
2084+
/** The PATH lookup's inputs; `platform` picks the shell whose resolution it mirrors. */
2085+
export interface ShellPathLookupOptions extends ToolProbeOptions {
2086+
readonly platform?: NodeJS.Platform;
2087+
}
2088+
2089+
/**
2090+
* Runs `command` through the shell, as the probes always have, and classifies
2091+
* a failure by whether `binary` is on the PATH it ran with.
2092+
*/
2093+
export function probeTool(command: string, binary: string, options: ToolProbeOptions = {}): ToolProbe {
2094+
try {
2095+
const output = execSync(command, { encoding: 'utf-8', env: options.env, cwd: options.cwd }).trim();
2096+
return { outcome: 'ok', output };
2097+
} catch (error) {
2098+
return classifyToolProbeFailure(command, binary, error, options);
2099+
}
2100+
}
2101+
2102+
/** The classifier behind {@link probeTool}: `error` is what `execSync(command)` threw. */
2103+
export function classifyToolProbeFailure(
2104+
command: string,
2105+
binary: string,
2106+
error: unknown,
2107+
options: ShellPathLookupOptions = {},
2108+
): ToolProbe {
2109+
if (!isOnShellPath(binary, options)) return { outcome: 'not-found' };
2110+
const failure = (error ?? {}) as { status?: unknown; signal?: unknown; code?: unknown; stdout?: unknown; stderr?: unknown };
2111+
const stderr = typeof failure.stderr === 'string' ? failure.stderr : '';
2112+
const stdout = typeof failure.stdout === 'string' ? failure.stdout : '';
2113+
const exit =
2114+
typeof failure.status === 'number'
2115+
? `exited with code ${failure.status}`
2116+
: typeof failure.signal === 'string'
2117+
? `was killed by ${failure.signal}`
2118+
: `could not run (${typeof failure.code === 'string' ? failure.code : 'unknown error'})`;
2119+
const detail = firstMeaningfulLine(stderr) ?? firstMeaningfulLine(stdout) ?? `\`${command}\` ${exit} with no output`;
2120+
return { outcome: 'failed', detail, exit, stderr };
2121+
}
2122+
2123+
/**
2124+
* Whether the shell `execSync` uses would find an executable named `binary`:
2125+
* `/bin/sh` walks `PATH` (an empty entry is the current directory) and takes
2126+
* the first FILE of that name — present but not executable is still found,
2127+
* and answered "Permission denied", not "not found"; `cmd.exe` tries the
2128+
* current directory and then each `PATH` entry with each `PATHEXT` extension.
2129+
*/
2130+
export function isOnShellPath(binary: string, options: ShellPathLookupOptions = {}): boolean {
2131+
const env = options.env ?? process.env;
2132+
const platform = options.platform ?? process.platform;
2133+
const cwd = options.cwd ?? process.cwd();
2134+
const isFile = (candidate: string): boolean => {
2135+
try {
2136+
return fs.statSync(candidate, { throwIfNoEntry: false })?.isFile() === true;
2137+
} catch {
2138+
return false;
2139+
}
2140+
};
2141+
if (platform === 'win32') {
2142+
// A plain object has no case-insensitive `Path` the way `process.env` does on Windows.
2143+
const read = (name: string): string | undefined =>
2144+
env[Object.keys(env).find((key) => key.toUpperCase() === name) ?? name];
2145+
const dirs = [cwd, ...(read('PATH') ?? '').split(';').map((dir) => dir.replace(/^"(.*)"$/, '$1')).filter(Boolean)];
2146+
// Windows file names are case-insensitive; npm writes the lower-case `.cmd`.
2147+
const extensions = (read('PATHEXT') || '.COM;.EXE;.BAT;.CMD').split(';').filter(Boolean).map((ext) => ext.toLowerCase());
2148+
return dirs.some((dir) => extensions.some((ext) => isFile(path.resolve(cwd, dir, `${binary}${ext}`))));
2149+
}
2150+
return (env.PATH ?? '').split(':').some((dir) => isFile(path.resolve(cwd, dir, binary)));
2151+
}
2152+
2153+
/**
2154+
* The line of a failed child's output a reader needs first. An uncaught Node
2155+
* error — Corepack's fetch failure is one — opens with `<file>:<line>`, the
2156+
* source line and a caret, and only then the `Error: …` line, so an error
2157+
* line is preferred wherever it sits; otherwise the first non-blank line.
2158+
*/
2159+
export function firstMeaningfulLine(text: string): string | undefined {
2160+
const lines = text.split(/\r?\n/).map((line) => line.trim()).filter((line) => line.length > 0);
2161+
return lines.find((line) => /^(?:[A-Z][A-Za-z]* )?[A-Za-z]*Error: /.test(line)) ?? lines[0];
2162+
}
2163+
2164+
/**
2165+
* Corepack's own words for a registry it could not use: `Error when
2166+
* performing the request to …` (no answer) and `Server answered with HTTP
2167+
* <status> when performing the request to …` (a refusal, 429 included), both
2168+
* from a stack under `corepack`; and `Network access disabled by the
2169+
* environment; can't reach …` (`COREPACK_ENABLE_NETWORK=0`).
2170+
*/
2171+
export function isCorepackFetchFailure(stderr: string): boolean {
2172+
return (
2173+
(stderr.includes(' when performing the request to ') && /corepack/i.test(stderr)) ||
2174+
/^Network access disabled by the environment; can't reach /m.test(stderr)
2175+
);
2176+
}
2177+
2178+
/** The fix for a pnpm whose Corepack could not fetch the pnpm it resolves. */
2179+
export const COREPACK_FETCH_FIX =
2180+
'pnpm is installed; Corepack could not download the pnpm version it resolves, and reinstalling pnpm will not fix that. ' +
2181+
'Check the network or proxy, or pin pnpm with a "packageManager" field in package.json';
2182+
2183+
/** The neutral fix for a tool that is on PATH and failed for any other reason. */
2184+
export function toolFailedFix(binary: string, command: string, exit: string): string {
2185+
return `${binary} is on PATH, but \`${command}\` ${exit}: run \`${command}\` and read its error`;
2186+
}
2187+
2188+
/** The `pnpm` row. */
2189+
export function pnpmCheck(options: ToolProbeOptions = {}): HealthCheckResult {
2190+
const probe = probeTool('pnpm -v', 'pnpm', options);
2191+
if (probe.outcome === 'ok') return { name: 'pnpm', status: 'ok', message: `Version ${probe.output}` };
2192+
if (probe.outcome === 'not-found') {
2193+
return { name: 'pnpm', status: 'error', message: 'Not found', fix: 'Install pnpm: npm install -g pnpm@10.28.1' };
2194+
}
2195+
return {
2196+
name: 'pnpm',
2197+
status: 'error',
2198+
message: probe.detail,
2199+
fix: isCorepackFetchFailure(probe.stderr) ? COREPACK_FETCH_FIX : toolFailedFix('pnpm', 'pnpm -v', probe.exit),
2200+
};
2201+
}
2202+
2203+
/** The `TypeScript` row: a warning either way, since a project-local `tsc` is the norm. */
2204+
export function typescriptCheck(options: ToolProbeOptions = {}): HealthCheckResult {
2205+
const probe = probeTool('tsc -v', 'tsc', options);
2206+
if (probe.outcome === 'ok') return { name: 'TypeScript', status: 'ok', message: probe.output };
2207+
if (probe.outcome === 'not-found') {
2208+
return { name: 'TypeScript', status: 'warning', message: 'Not found in PATH', fix: 'Installed locally via pnpm' };
2209+
}
2210+
return { name: 'TypeScript', status: 'warning', message: probe.detail, fix: toolFailedFix('tsc', 'tsc -v', probe.exit) };
2211+
}
2212+
2213+
/** The `Git` row. */
2214+
export function gitCheck(options: ToolProbeOptions = {}): HealthCheckResult {
2215+
const probe = probeTool('git --version', 'git', options);
2216+
if (probe.outcome === 'ok') return { name: 'Git', status: 'ok', message: probe.output };
2217+
if (probe.outcome === 'not-found') {
2218+
return { name: 'Git', status: 'warning', message: 'Not found', fix: 'Install Git for version control' };
2219+
}
2220+
return { name: 'Git', status: 'warning', message: probe.detail, fix: toolFailedFix('git', 'git --version', probe.exit) };
2221+
}
2222+
20252223
// ─── Command ────────────────────────────────────────────────────────
20262224

20272225
export default class Doctor extends Command {
@@ -2095,40 +2293,11 @@ export default class Doctor extends Command {
20952293
});
20962294
}
20972295

2098-
// Check pnpm
2099-
try {
2100-
const pnpmVersion = execSync('pnpm -v', { encoding: 'utf-8' }).trim();
2101-
results.push({
2102-
name: 'pnpm',
2103-
status: 'ok',
2104-
message: `Version ${pnpmVersion}`,
2105-
});
2106-
} catch (error) {
2107-
results.push({
2108-
name: 'pnpm',
2109-
status: 'error',
2110-
message: 'Not found',
2111-
fix: 'Install pnpm: npm install -g pnpm@10.28.1',
2112-
});
2113-
}
2114-
2115-
// Check TypeScript
2116-
try {
2117-
const tscVersion = execSync('tsc -v', { encoding: 'utf-8' }).trim();
2118-
results.push({
2119-
name: 'TypeScript',
2120-
status: 'ok',
2121-
message: tscVersion,
2122-
});
2123-
} catch (error) {
2124-
results.push({
2125-
name: 'TypeScript',
2126-
status: 'warning',
2127-
message: 'Not found in PATH',
2128-
fix: 'Installed locally via pnpm',
2129-
});
2130-
}
2131-
2296+
// Check pnpm, then TypeScript (#24464: "not on PATH" and "ran and failed"
2297+
// are two rows — see `probeTool`).
2298+
results.push(pnpmCheck());
2299+
results.push(typescriptCheck());
2300+
21322301
// Check if dependencies are installed
21332302
const cwd = process.cwd();
21342303
const nodeModulesPath = path.join(cwd, 'node_modules');
@@ -2186,22 +2355,8 @@ export default class Doctor extends Command {
21862355
}
21872356
}
21882357

2189-
// Check Git
2190-
try {
2191-
const gitVersion = execSync('git --version', { encoding: 'utf-8' }).trim();
2192-
results.push({
2193-
name: 'Git',
2194-
status: 'ok',
2195-
message: gitVersion,
2196-
});
2197-
} catch (error) {
2198-
results.push({
2199-
name: 'Git',
2200-
status: 'warning',
2201-
message: 'Not found',
2202-
fix: 'Install Git for version control',
2203-
});
2204-
}
2358+
// Check Git (#24464, the same classifier as pnpm's)
2359+
results.push(gitCheck());
22052360

22062361
// #5387 — what doctor read the environment FROM, reported before anything
22072362
// derived from it. Unconditional: an env-derived verdict whose inputs are

0 commit comments

Comments
 (0)