Skip to content

fix(cli): os doctor tells a tool missing from PATH apart from one that ran and failed - #24493

Merged
objectstack-fleet[bot] merged 2 commits into
mainfrom
claude/issue-24464-doctor-pnpm-probe
Oct 19, 2026
Merged

objectstack-fleet[bot] merged 2 commits into
mainfrom
claude/issue-24464-doctor-pnpm-probe

Conversation

@objectstack-fleet

Copy link
Copy Markdown
Contributor

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:

  ✗ 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:

  ✗ pnpm                 Not found
      → Install pnpm: npm install -g pnpm@10.28.1

After:

  ✗ 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:

  ✗ 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

…t 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>
…H holds nothing else

Claude-Session: https://claude.ai/code/session_01SYEJqDL3NXxrJRBVqaMu9T
Co-authored-by: Claude <noreply@anthropic.com>
@github-actions github-actions Bot added size/l documentation Improvements or additions to documentation tests tooling labels Oct 19, 2026
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/cli, touching 15 documentable anchor(s).

10 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:

  • content/docs/automation/flows.mdx (via os doctor (command, read off packages/cli/src/commands/doctor.ts))
  • content/docs/automation/jobs.mdx (via os doctor (command, read off packages/cli/src/commands/doctor.ts))
  • content/docs/data-modeling/indexing.mdx (via os doctor (command, read off packages/cli/src/commands/doctor.ts))
  • content/docs/deployment/cli.mdx (via os doctor (command, read off packages/cli/src/commands/doctor.ts))
  • content/docs/deployment/environment-variables.mdx (via os doctor (command, read off packages/cli/src/commands/doctor.ts))
  • content/docs/deployment/production-readiness.mdx (via os doctor (command, read off packages/cli/src/commands/doctor.ts))
  • content/docs/getting-started/index.mdx (via packageManager (literal, a string literal in COREPACK_FETCH_FIX))
  • content/docs/protocol/backward-compatibility.mdx (via os doctor (command, read off packages/cli/src/commands/doctor.ts))
  • content/docs/protocol/kernel/config-resolution.mdx (via os doctor (command, read off packages/cli/src/commands/doctor.ts))
  • content/docs/protocol/kernel/http-protocol.mdx (via os doctor (command, read off packages/cli/src/commands/doctor.ts))

⛔ 2 release-owned page(s) also name something this change touched. These are read-only:

  • content/docs/releases/v16.mdx (via os doctor (command, read off packages/cli/src/commands/doctor.ts))
  • content/docs/releases/v17/17-5.mdx (via os doctor (command, read off packages/cli/src/commands/doctor.ts))

content/docs/releases/ is RELEASE-OWNED (AGENTS.md "Documentation Guardrails"): release
notes are written centrally at release time, and a code PR that edits them is the exact PR
that guardrail exists to stop. They are still audited — read-only. If one of them is actually
wrong, file an issue or open a dedicated docs-only PR; do not edit it here.

What this run could not see
  • 8 name(s) were too generic to anchor anything (single lowercase words)
  • the SDK route bridge reached 54 of 206 client-bound route-ledger rows — the other 152 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 152: 0 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 55 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 97 are undecided (no in-repo declaration, on a ledger that has other in-repo registrars — absence and an unreadable spelling are not distinguishable here). The rows themselves: node scripts/docs-audit/affected-docs.mjs --bridge-coverage
  • a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.
  • a key NAME is not a key, so the hand re-read the line above prescribes can land on the wrong schema. The same spelling is authorable on one governed type and a [REMOVED] tombstone on another for each of active, aria, joins, objects, template, tools and version (censused on [finding] tools is a key on BOTH AgentSchema (tombstoned, dead) and SkillSchema (live, cloud-attested), so a name-based search attributes skill examples to the agent key — it produced a false stop-the-line alarm on PR #19059 #19093 over the liveness ledger's governed types, top-level keys); nothing in a search result distinguishes the two, so a grep hit on a LIVE example reads as evidence about the DEAD key. Measured on fix(spec): the agent.tools liveness row says dead — it claimed live on a key the schema tombstoned #19059: content/docs/ai/agents.mdx was reported as contradicting the agent.tools tombstone over its tools: example at :161, which is inside the defineSkill({ block opened at :155 — the page was already correct. Settle ownership by PARSING the value against both schemas, never by the name: that literal PASSES SkillSchema, and as an AgentSchema it FAILS at tools with the tombstone prescription. ⛔ These names are not the whole class — a key retired through a .strict() guidance map leaves no tombstone in the walked shape and none of them here (tool.category, live as AIToolDefinition.category).

Coarse fallback — 29 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json 33a45ccaddaf479256f78341e58b38727add9de4 → packageMentionDocs.

Which tree this was computed on

This run read content/docs from 3cbc166bfb839fd471b8dcbb9c05e52becb422ea — the merge of head 023dd79d042e93b8fae0cfa89c72aa1b085b543d into base 33a45ccaddaf479256f78341e58b38727add9de4, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 3cbc166bfb839fd471b8dcbb9c05e52becb422ea && git checkout 3cbc166bfb839fd471b8dcbb9c05e52becb422ea
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 33a45ccaddaf479256f78341e58b38727add9de4 023dd79d042e93b8fae0cfa89c72aa1b085b543d && git checkout -B drift-repro 33a45ccaddaf479256f78341e58b38727add9de4 && git merge --no-ff 023dd79d042e93b8fae0cfa89c72aa1b085b543d

node scripts/docs-audit/affected-docs.mjs --json 33a45ccaddaf479256f78341e58b38727add9de4

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

Advisory only, and a precision-first one (#9192): a page is listed because it names a
symbol, wire route or SDK method this diff touched — not because it mentions a changed
package. Each row says which anchor put it there, so a wrong row is reportable rather than
merely annoying. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs 33a45ccaddaf479256f78341e58b38727add9de4 → pass the list as
args.docs, on the commit named under Which tree this was computed on.

@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

席位核对 — ACCEPT

domain:cli seat 1 (seat post #6024) · zhuangjianguo · session session_01SYEJqDL3NXxrJRBVqaMu9T · 2026-10-19T07:43Z · on head 023dd79d0 · #24464 (p3, claim 6261405871, report 6262235637). This is the seat's own review at the default tier, read against the diff on GitHub, not the report.

  1. What moved, read on the compare e11ae4b6b...023dd79d0:
    • doctor.ts: the three inline probes become pnpmCheck / typescriptCheck / gitCheck over one probeTool. It runs execSync(command) through the shell as before, and only after a failure asks isOnShellPath: is a FILE of that name on the PATH the probe ran with? On win32 that means the cwd first, then PATH × PATHEXT, and never an extensionless file.
    • A miss returns today's row byte for byte. A hit reports the child's first meaningful line, preferring an …Error: line over Node's <file>:<line> header. The fix is Corepack's (network, proxy or a packageManager pin) only when stderr carries Corepack's own fetch-failure words, and the neutral "run it and read its error" otherwise.
    • Row statuses are unchanged (pnpm error, TypeScript / Git warning), so doctor's exit code does not move.
  2. The premise, measured rather than assumed. The classifier rests on the dev's table: shell exit 127 covers "not on PATH", "its node missing" and "not executable" alike. So 127 is not the discriminator, and a lookup is. A lookup that misses falls back to today's row, so in that direction the change can be no more wrong than the code it replaces.
  3. Shape:
    • Fixes #24464, a line-initial Clause-②: no with its basis, and a patch changeset on @objectstack/cli.
    • 3 files, +458 / −50. Not governed.
    • The 10 new named exports in doctor.ts are not published. @objectstack/cli's exports map is ., ./console and ./hook-body, and src/index.ts re-exports only doctor.ts's default (DoctorCommand). So Clause-②: no holds.
    • The surface is the claim's: doctor.ts, one new test and the changeset.
  4. Tests and reverse verification:
    • test/doctor-tool-probe-failure.test.ts: 17/17, every row through the real probe with a test-owned PATH, so no real pnpm, Corepack or registry is reachable.
    • Ablation (the lookup swapped for today's catch-all): exactly the 8 ran-and-failed cases go red. The restore was proven by blob.
    • cli unit tier: 312 files, 4627 tests. Typecheck and changed-file eslint exited 0.
    • Before and after under env -i: no-pnpm and working-toolchain output is byte-identical; a real Corepack shim against a local 429 now names the 429 and the Corepack fix.
    • Windows was not run on a host. The win32 branch is pinned by four unit cases, and the body says why.
  5. Gates:
    • dispatch-gates: 68 derived, 68 run, 0 NOT-MEASURED. Two families were first NOT MEASURED for unbuilt dependencies and passed once those were built.
    • The 11 declared-wide families and the full-union pnpm lint all exited 0.
    • Landing waits on every check of this head.
  6. Findings, both Acceptance notes:
    • The probes have no execSync timeout: not observed, since Corepack failed fast in every measured run.
    • The not-found hint names pnpm@10.28.1: drift. The row was required to stay byte-identical, so it is not moved here.
  7. Landing: ready plus auto-merge through the queue once every check of 023dd79d0 is green or a rostered skip. The card closes on the merge.

Generated by Claude Code

@objectstack-fleet
objectstack-fleet Bot marked this pull request as ready for review October 19, 2026 08:01
@objectstack-fleet
objectstack-fleet Bot enabled auto-merge October 19, 2026 08:01
@objectstack-fleet
objectstack-fleet Bot added this pull request to the merge queue Oct 19, 2026
Merged via the queue into main with commit 959ee31 Oct 19, 2026
41 checks passed
@objectstack-fleet
objectstack-fleet Bot deleted the claude/issue-24464-doctor-pnpm-probe branch October 19, 2026 08:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation size/l tests tooling

Projects

None yet

2 participants