fix(create-objectstack): make the package-manager verdict hermetic and honest (#11616)

.changeset/create-objectstack-honest-pm-probe.md

@@ -0,0 +1,35 @@
+---
+"create-objectstack": patch
+---
+
+fix(create-objectstack): stop reporting a failed pnpm probe as a deliberate npm choice (#11616)
+
+`detectPackageManager()` was `try { execSync('pnpm --version') } catch { return
+'npm' }`, so every failure mode collapsed into one answer. `npm install` in the
+scaffolder's output meant either *this machine has no pnpm* or *the probe
+threw*, and nothing — no log line, no message — could tell the two apart.
+
+That second case is reachable on an ordinary developer machine, not just in
+theory: `pnpm --version` resolves through Corepack and therefore depends on the
+directory it runs in. Measured on one machine, one binary, two directories —
+`10.31.0` inside a repo that pins `packageManager`, `10.33.0` outside it, where
+Corepack has to resolve, and may have to fetch, a version nothing pinned. A
+user who has pnpm installed but is on a slow or offline network was silently
+told to run npm.
+
+The probe now reports why as well as what:
+
+- `probe: 'ok'` — pnpm answered, so pnpm is used (unchanged, silent).
+- `probe: 'absent'` — no pnpm on PATH at all, so npm is a real choice
+ (unchanged, silent).
+- `probe: 'failed'` — pnpm **is** on PATH and the probe still threw. npm is
+ used exactly as before, and the run now says so, naming the underlying
+ failure: `pnpm is installed but \`pnpm --version\` failed (<reason>); using
+ npm as a fallback.`
+
+**Which package manager a run uses is unchanged in all three cases** — it is
+still pnpm if and only if the probe succeeded. The PATH lookup that separates
+`absent` from `failed` runs only after the decision is already made and feeds
+the message alone, so a miss there can change a warning's wording and never the
+tool's behaviour. The only output that moves is one warning in a case that was
+previously silent and wrong.

packages/create-objectstack/src/detect-package-manager.test.ts

@@ -0,0 +1,187 @@
+// Copyright (c) 2026 ObjectStack contributors. Apache-2.0 license.
+//
+// Pins the package-manager probe's VERDICT — both what it decides and why.
+//
+// The card this file answers is a flake, but the flake was a symptom. The old
+// detector was `try { execSync('pnpm --version') } catch { return 'npm' }`, so
+// `npm` in a transcript meant either "this machine has no pnpm" or "the probe
+// threw" and nothing could tell which. `pnpm --version` resolves through
+// Corepack and therefore depends on the cwd it runs in, so the second case is
+// reachable on any machine with a slow or offline network — which is how a
+// merge-queue job on a diff that could not reach this package went red.
+//
+// Every test here is hermetic by construction: both ambient reads (the probe
+// and the PATH lookup) are injected, so nothing in this file can be decided by
+// the runner. That is deliberate and it is the point of the card — a pin that
+// asks the environment a question it cannot pin the answer to is measuring the
+// runner, not the code.
+
+import { describe, it, expect } from 'vitest';
+import path from 'node:path';
+import fs from 'node:fs';
+import os from 'node:os';
+import {
+ detectPackageManager,
+ probeFailureDetail,
+ resolveOnPath,
+} from './detect-package-manager.js';
+
+/** A probe that fails the way a real `execSync` failure does. */
+function throwingProbe(err: unknown): () => void {
+ return () => {
+ throw err;
+ };
+}
+
+/** The shape `execSync` throws: a status, and stderr only if it was piped. */
+function execError(over: Record<string, unknown> = {}): Error {
+ return Object.assign(new Error('Command failed: pnpm --version'), {
+ status: 1,
+ signal: null,
+ stderr: Buffer.from(''),
+ ...over,
+ });
+}
+
+describe('detectPackageManager — the decision', () => {
+ it('probe succeeds -> pnpm', () => {
+ const out = detectPackageManager({ probe: () => {}, pnpmOnPath: () => true });
+ expect(out).toEqual({ pm: 'pnpm', probe: 'ok' });
+ });
+
+ it('probe throws, pnpm absent from PATH -> npm', () => {
+ const out = detectPackageManager({
+ probe: throwingProbe(execError({ status: 127 })),
+ pnpmOnPath: () => false,
+ });
+ expect(out).toEqual({ pm: 'npm', probe: 'absent' });
+ });
+
+ it('probe throws, pnpm present on PATH -> npm', () => {
+ const out = detectPackageManager({
+ probe: throwingProbe(execError({ stderr: Buffer.from('Error: getaddrinfo ENOTFOUND registry.npmjs.org\n') })),
+ pnpmOnPath: () => true,
+ });
+ expect(out.pm).toBe('npm');
+ });
+
+ // Clause-② guard for this change: the change was allowed to move what the
+ // tool REPORTS, never what it DOES. `pm` must still be a pure function of
+ // "did the probe succeed", exactly as the collapsed version was — the PATH
+ // lookup must not be able to move it. Green before and after the fix by
+ // design; a regression guard, not evidence the fix was needed.
+ it('regression guard: pm is pnpm if and only if the probe succeeded', () => {
+ for (const pnpmOnPath of [true, false]) {
+ expect(detectPackageManager({ probe: () => {}, pnpmOnPath: () => pnpmOnPath }).pm).toBe('pnpm');
+ expect(
+ detectPackageManager({ probe: throwingProbe(execError()), pnpmOnPath: () => pnpmOnPath }).pm,
+ ).toBe('npm');
+ }
+ });
+});
+
+describe('detectPackageManager — the distinction that used to be collapsed', () => {
+ // THE collapse guard. Both of these answer `npm`; if a future edit folds the
+ // two failure modes back into one answer, these two objects become equal and
+ // this test goes red. Asserting only `pm` cannot catch that — that is the
+ // whole defect — so the assertion is on the reason.
+ it('"probe threw" and "chose npm" are different verdicts, not one', () => {
+ const absent = detectPackageManager({
+ probe: throwingProbe(execError({ status: 127 })),
+ pnpmOnPath: () => false,
+ });
+ const failed = detectPackageManager({
+ probe: throwingProbe(execError({ stderr: Buffer.from('corepack: fetch failed\n') })),
+ pnpmOnPath: () => true,
+ });
+
+ expect(absent.pm).toBe(failed.pm); // same decision...
+ expect(absent.probe).not.toBe(failed.probe); // ...different reason
+ expect(absent.probe).toBe('absent');
+ expect(failed.probe).toBe('failed');
+ expect(absent).not.toEqual(failed);
+ });
+
+ it('only the "probe threw" verdict carries a detail to report', () => {
+ const failed = detectPackageManager({
+ probe: throwingProbe(execError({ stderr: Buffer.from('corepack: fetch failed\n') })),
+ pnpmOnPath: () => true,
+ });
+ expect(failed).toHaveProperty('detail', 'corepack: fetch failed');
+
+ const absent = detectPackageManager({
+ probe: throwingProbe(execError({ status: 127 })),
+ pnpmOnPath: () => false,
+ });
+ expect(absent).not.toHaveProperty('detail');
+
+ const ok = detectPackageManager({ probe: () => {}, pnpmOnPath: () => true });
+ expect(ok).not.toHaveProperty('detail');
+ });
+
+ it('the PATH lookup is not consulted when the probe succeeds', () => {
+ let consulted = false;
+ detectPackageManager({
+ probe: () => {},
+ pnpmOnPath: () => {
+ consulted = true;
+ return true;
+ },
+ });
+ expect(consulted).toBe(false);
+ });
+});
+
+describe('probeFailureDetail — one bounded line, most specific evidence first', () => {
+ it('names the signal when the child was killed', () => {
+ expect(probeFailureDetail(execError({ signal: 'SIGTERM', stderr: Buffer.from('noise\n') })))
+ .toBe('killed by SIGTERM');
+ });
+
+ it("uses stderr's first non-empty line when there is one", () => {
+ expect(probeFailureDetail(execError({ stderr: Buffer.from('\n\n corepack: fetch failed \nmore\n') })))
+ .toBe('corepack: fetch failed');
+ });
+
+ it('falls back to a libuv code, then to the exit status', () => {
+ expect(probeFailureDetail(execError({ code: 'ENOENT', status: null }))).toBe('ENOENT');
+ expect(probeFailureDetail(execError({ status: 127 }))).toBe('exited 127');
+ });
+
+ it('never returns a multi-line or unbounded string — it lands in a console warning', () => {
+ const detail = probeFailureDetail(execError({ stderr: Buffer.from(`${'x'.repeat(5000)}\nsecond\n`) }));
+ expect(detail).not.toContain('\n');
+ expect(detail.length).toBeLessThanOrEqual(200);
+ });
+
+ it('degrades to a fixed string rather than throwing on a non-Error', () => {
+ expect(probeFailureDetail(undefined)).toBe('unknown error');
+ expect(probeFailureDetail(null)).toBe('unknown error');
+ });
+});
+
+describe('resolveOnPath — the PATH read, without spawning', () => {
+ it('finds an executable in an earlier PATH entry and returns its full path', () => {
+ const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'create-objectstack-pathprobe-'));
+ try {
+ const bin = path.join(dir, 'pnpm');
+ fs.writeFileSync(bin, '#!/bin/sh\nexit 0\n', { mode: 0o755 });
+ expect(resolveOnPath('pnpm', { PATH: `${dir}${path.delimiter}/nonexistent` })).toBe(bin);
+ expect(resolveOnPath('pnpm', { PATH: '/nonexistent' })).toBeNull();
+ expect(resolveOnPath('pnpm', { PATH: '' })).toBeNull();
+ expect(resolveOnPath('pnpm', {})).toBeNull();
+ } finally {
+ fs.rmSync(dir, { recursive: true, force: true });
+ }
+ });
+
+ it('does not mistake a directory of the same name for an executable', () => {
+ const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'create-objectstack-pathprobe-'));
+ try {
+ fs.mkdirSync(path.join(dir, 'pnpm'));
+ expect(resolveOnPath('pnpm', { PATH: dir })).toBeNull();
+ } finally {
+ fs.rmSync(dir, { recursive: true, force: true });
+ }
+ });
+});

packages/create-objectstack/src/detect-package-manager.ts

@@ -0,0 +1,140 @@
+// Copyright (c) 2026 ObjectStack contributors. Apache-2.0 license.
+//
+// The package-manager probe, lifted out of index.ts so it can be tested
+// without importing that module (which calls `program.parse()` at import
+// time) and without spawning anything — the same reason pkg-utils.ts,
+// rewrite-identity.ts and created-summary.ts live outside index.ts.
+//
+// WHY THE OUTCOME IS A RECORD AND NOT A BARE STRING
+//
+// The probe used to be `try { execSync('pnpm --version') } catch { return
+// 'npm' }`, which collapses every failure mode into one answer. `npm` in the
+// output then meant *the probe threw*, not *the code chose npm*, and nothing
+// downstream — no log line, no assertion message — could tell the two apart.
+//
+// That is not only a diagnostics problem. `pnpm --version` resolves through
+// Corepack, so it depends on the cwd it runs in: measured on one machine, one
+// binary, two directories, `pnpm --version` answered 10.31.0 inside this repo
+// (the pinned `packageManager`) and 10.33.0 outside it, where Corepack has to
+// resolve — and may have to FETCH — a version nothing pinned. A user who has
+// pnpm installed, on a slow or offline network, was silently told to run npm.
+//
+// So the decision and the reason are now separate values. The DECISION is
+// byte-for-byte the old one — `pnpm` if the probe succeeds, `npm` otherwise —
+// because which package manager actually runs is not a thing this change is
+// entitled to move. Only the REASON is new, and it distinguishes the two
+// cases that were collapsed:
+//
+// probe: 'ok' the probe succeeded -> pnpm
+// probe: 'absent' no pnpm on PATH at all -> npm, a real choice
+// probe: 'failed' pnpm IS on PATH, probe threw -> npm, a FALLBACK
+//
+// Only 'failed' is new information, and only 'failed' prints anything extra:
+// on every path that was already correct the output is unchanged.
+
+import fs from 'node:fs';
+import path from 'node:path';
+import { execSync } from 'node:child_process';
+
+export type PackageManagerDetection =
+ | { pm: 'pnpm'; probe: 'ok' }
+ | { pm: 'npm'; probe: 'absent' }
+ | { pm: 'npm'; probe: 'failed'; detail: string };
+
+/** The two ambient reads this module makes, injectable so tests can be hermetic. */
+export interface DetectDeps {
+ /** Runs the version probe. Returns normally on success, throws on any failure. */
+ probe: () => void;
+ /** Whether a `pnpm` executable is resolvable on PATH at all. */
+ pnpmOnPath: () => boolean;
+}
+
+/**
+ * Resolve an executable on PATH without spawning anything.
+ *
+ * Deliberately NOT a second subprocess: this runs only after the probe has
+ * already failed, and a machine whose probe just failed is the last place to
+ * spend another spawn. It also cannot change which package manager is chosen
+ * — it feeds the reason field only — so a miss here degrades a warning's
+ * wording and never the tool's behaviour.
+ */
+export function resolveOnPath(cmd: string, env: NodeJS.ProcessEnv = process.env): string | null {
+ const raw = env.PATH ?? '';
+ if (!raw) return null;
+ // PATHEXT is Windows' list of what counts as executable; pnpm ships there as
+ // `pnpm.cmd`, so a bare-name check would miss it. Elsewhere the name is the
+ // whole story.
+ const exts = process.platform === 'win32'
+ ? (env.PATHEXT ?? '.COM;.EXE;.BAT;.CMD').split(';').filter(Boolean)
+ : [''];
+ for (const dir of raw.split(path.delimiter)) {
+ if (!dir) continue;
+ for (const ext of exts) {
+ const candidate = path.join(dir, cmd + ext);
+ try {
+ if (fs.statSync(candidate).isFile()) return candidate;
+ } catch {
+ // Unreadable or missing entry — just not a hit.
+ }
+ }
+ }
+ return null;
+}
+
+/**
+ * A one-line, log-safe description of why the probe threw.
+ *
+ * Order matters: a killed child reports its signal and no useful stderr, and a
+ * child that never launched reports a libuv code and no status — so the most
+ * specific evidence available is taken first and everything is flattened to a
+ * single bounded line, because this ends up inside a console warning.
+ */
+export function probeFailureDetail(err: unknown): string {
+ const e = (err ?? {}) as {
+ signal?: string | null;
+ status?: number | null;
+ code?: string | number | null;
+ stderr?: Buffer | string | null;
+ message?: string;
+ };
+ if (e.signal) return `killed by ${e.signal}`;
+ const stderr = e.stderr == null ? '' : String(e.stderr);
+ const firstLine = stderr.split('\n').map((l) => l.trim()).find((l) => l.length > 0);
+ if (firstLine) return firstLine.length > 200 ? `${firstLine.slice(0, 197)}...` : firstLine;
+ if (typeof e.code === 'string') return e.code;
+ if (typeof e.status === 'number') return `exited ${e.status}`;
+ const msg = (e.message ?? '').split('\n')[0]?.trim();
+ return msg || 'unknown error';
+}
+
+/** The real probe: a read-only `pnpm --version`, silent on success and on failure. */
+function defaultProbe(): void {
+ // stdin/stdout ignored, stderr CAPTURED rather than ignored: execSync
+ // attaches it to the thrown error, which is the only way the warning can
+ // name what actually went wrong. An explicit triple keeps the child's
+ // stderr off this process's stderr, so a run that succeeds — or one that
+ // fails — still prints nothing except what this module chooses to print.
+ execSync('pnpm --version', { stdio: ['ignore', 'ignore', 'pipe'] });
+}
+
+/**
+ * Decide which package manager this run should name, and why.
+ *
+ * The `pm` field is exactly the old function's return value. `probe` is the
+ * new part, and is the only thing callers should branch on when deciding
+ * whether to explain themselves to the user.
+ */
+export function detectPackageManager(deps: Partial<DetectDeps> = {}): PackageManagerDetection {
+ const probe = deps.probe ?? defaultProbe;
+ const pnpmOnPath = deps.pnpmOnPath ?? (() => resolveOnPath('pnpm') !== null);
+ try {
+ probe();
+ return { pm: 'pnpm', probe: 'ok' };
+ } catch (err) {
+ // The decision is already made at this point and does not depend on
+ // anything below: the probe threw, so it is npm either way. What follows
+ // only chooses which of the two npm cases to report.
+ if (!pnpmOnPath()) return { pm: 'npm', probe: 'absent' };
+ return { pm: 'npm', probe: 'failed', detail: probeFailureDetail(err) };
+ }
+}