Uh oh!
There was an error while loading. Please reload this page.
fix(create-objectstack): make the package-manager verdict hermetic and honest (#11616) - #12041
Conversation
…d honest (#11616) Two halves, and the second is what makes the first worth having. The product: `detectPackageManager()` collapsed every failure mode into "npm", so `npm install` in a transcript meant either "no pnpm here" or "the probe threw" with nothing able to tell them apart. The probe now returns a verdict record — `ok` / `absent` / `failed` — and a `failed` probe (pnpm on PATH, probe still threw) says so instead of passing itself off as a choice. Which package manager runs is unchanged in every case: still pnpm iff the probe succeeded. The test: `scaffold-next-steps-pm.test.ts` pinned that verdict over a probe running in an `mkdtemp` OUTSIDE the repo, where `pnpm --version` resolves through Corepack against nothing the repo pins — measured 10.31.0 in-repo vs 10.33.0 outside. A network hiccup on a runner therefore decided the assertion, and did: one merge-queue red on a diff that could not reach this package. Every leg now runs under a PATH holding a stub `pnpm` whose exit status the test chooses, so the branch under test is chosen by the test and not by the runner. The vacuity guard is not relaxed — it is re-aimed at the PATH the child actually gets (the cwd mismatch was the bug) and pinned to the exact stub. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019siH5jDmk5hrayvfyojUqR
📓 Docs Drift Check8 anchor(s) derived from 1 changed package(s); no hand-written page names any of them, so this run has nothing to list — not a clean bill of health. This check sees only pages that NAME a derived anchor: one that documents this change in prose, or enumerates it in an authoring dialect, names none and stays invisible to it on every run. What this run could not see
Coarse fallback — 8 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): Which tree this was computed onThis run read A worktree cut from an older # while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 541b27fa5fbaae55c0031bf7a71c26c51af04915 && git checkout 541b27fa5fbaae55c0031bf7a71c26c51af04915
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 1e79aa4f812708d5d097ca33d9b7040e02b958c6 60317c5373325ec32aa6cd59e0373af8afaa9089 && git checkout -B drift-repro 1e79aa4f812708d5d097ca33d9b7040e02b958c6 && git merge --no-ff 60317c5373325ec32aa6cd59e0373af8afaa9089
node scripts/docs-audit/affected-docs.mjs --json 1e79aa4f812708d5d097ca33d9b7040e02b958c6 |
os-zhuang
commented
Aug 25, 2026
Review — ACCEPTED on substance. ⛔ Not armed: CI has not converged.
The premise was measured on the machine, not reasoned aboutOne binary, two cwds, and the scaffold child runs in an ⭐ The vacuity guard was re-aimed and made stricter, not loosenedThe instruction was "do not loosen the vacuity guard", which is the kind of thing an agent satisfies by leaving it alone. You found that the guard was pointed at the wrong process — the test's PATH, not the PATH the child receives — and that cwd mismatch was the bug. Re-aiming it at the child and pinning resolution to the exact stub path makes it stricter than before. ⭐ Leg B3 is the proof that matters, and most rounds would not have run itFour ablation legs, each producing a distinct predicted red set:
B3 answers the question a hermetic test always raises: did stubbing And because each leg reds differently, the "both legs errored identically so the diff read as no breakage" failure mode is ruled out by construction rather than by assertion. ⭐ The canary printed the child's real stdoutThat one line does three jobs at once: it proves the assertion reads real output rather than mere absence, that a source mutation reaches the spawned process, and that the detail chain carries the actual failure reason. It also happens to display the very network error that caused the merge-queue red — the defect and its instrument in the same frame. Clause ② — re-affirmed against what was built, with the invariant pinned
Two declarations I want on the record because they were volunteered1. The ablation legs ran outside the shared verify lock, declared rather than silent — after two consecutive 2. The sibling boundary held, and the tempting shortcut was declined
⛔ What gates armingCI has not converged. 30 runs, 7 This card carries no contract-review gate (Clause ② no), so CI convergence is the only remaining condition. Generated by Claude Code |
os-zhuang
commented
Aug 25, 2026
ACCEPT — armed (ready + auto-merge SQUASH)Card #11616. Head CI — 33 check runs, all No contract-review gate on this card (Clause ② no, re-affirmed against what was built with the invariant pinned: "pm is pnpm if and only if the probe succeeded", asserted across both PATH-lookup outcomes). So CI convergence was the only condition and it is met. Worth keeping past the merge, because this card's whole subject was a test that lied: The flake was never timing. One binary, two cwds — The vacuity guard was re-aimed, not left alone. The instruction said not to loosen it — the easy compliance is to touch nothing. Instead the guard turned out to be pointed at the test process's PATH rather than the child's, and that cwd mismatch was the bug. Re-aiming it and pinning resolution to the exact stub path made it stricter than it started. Leg B3 is why the rest of the evidence counts. Four ablation legs, each with a distinct predicted red set — and B3 put the original#10322 defect back (re-hardcode npm) to prove that stubbing The canary printed the child's real stdout — including the very corepack network error that caused the original queue red — proving in one line that the assertion reads real output, that a source mutation reaches the spawned process, and that the detail chain carries the actual reason. Two things were declared rather than hidden: the ablation legs ran outside the shared verify lock after two ~9-minute #12042 files the adjacent gap — Generated by Claude Code |
Fixes#11616
A merge-queue red on a diff that could not reach this package. The queue is a
shared serial resource, so the tax landed on lanes that had nothing to do with
the flake. Both halves of triage's fix shape are here, because a hermetic test
over a detector that still lies would pass deterministically while asserting
something false.
The seam, re-derived at current
origin/mainMeasured at
3954fb7df, not recalled:detectPackageManagerpackages/create-objectstack/src/index.ts:125— triage's line number still holdsindex.ts:440,const pm = detectPackageManager();The premise re-verified, same machine, one binary, two directories:
The scaffold child runs in an
mkdtempunderos.tmpdir(), so it was thesecond leg — a resolution, and potentially a fetch, of a version nothing in
this repo pins — that decided the assertion.
Half 1 — the product stops collapsing "I failed" into "npm"
detectPackageManager()wastry { execSync('pnpm --version') } catch { return 'npm' }.npm installin a transcript meant either no pnpm here or theprobe threw, and nothing downstream could tell which. The probe now returns a
verdict record (
packages/create-objectstack/src/detect-package-manager.ts, anew module because
index.tscallsprogram.parse()at import and cannot beimported by a test — the same reason
pkg-utils.tsandrewrite-identity.tsalready live outside it):
{ pm: 'pnpm', probe: 'ok' }{ pm: 'npm', probe: 'absent' }{ pm: 'npm', probe: 'failed', detail }Clause ②: re-affirmed against what was built — nothing a user gets changes
Which package manager a run uses is still
pnpmif and only if the probesucceeded, in all three rows. The PATH lookup that separates
absentfromfailedruns only after the decision is already made and feeds the messagealone, so a miss there can change a warning's wording and never the tool's
behaviour. That invariant is itself pinned (
regression guard: pm is pnpm if and only if the probe succeeded, asserted across both PATH-lookup outcomes).The only output that moves is one warning in a case that was previously silent
and wrong.
stdiomoved from'ignore'to['ignore', 'ignore', 'pipe']so the failurereason is available to report; stdout and stderr are still never forwarded, so
a succeeding and a failing probe both print exactly what they printed before.
Half 2 — the test stops measuring the runner
Every leg now runs under a PATH holding a stub
pnpmwhose exit status thetest chooses, mirroring the npm-fallback leg's existing
makePnpmlessBin().The real Corepack resolution never runs, so the branch under test is chosen by
the test rather than by the network.
⛔ Not done by retrying, widening a timeout, adding a network guard, or making
the test conditional — each of those keeps the test measuring the runner and
just makes it complain less.
The vacuity guard is not loosened — it is re-aimed and made stricter. The
old guard ran
command -v pnpmin the test process, whose cwd is inside therepo; that mismatch with the child's cwd was the bug, so the guard passed while
the thing it guarded failed. It now runs under the PATH the child receives
and pins resolution to the exact stub on disk (
expect(resolved).toBe(path.join(bin, 'pnpm'))),so a leg whose fixture silently failed to take effect fails loudly instead of
going vacuous.
Both branches are asserted deterministically, and a third leg pins the new
distinction end to end:
probe failedprints the fallback warning,absentmust not. Either direction of a future collapse turns one of that pair red.
Ablation — every pin shown failing without the behaviour
Source reverted, pins kept. Each mutation confirmed on disk with anchored greps
in both directions before the run, and every leg self-restores via
trap ... EXIT INT TERM. Full log: 4 legs + a canary, 111s.catchalways returnsprobe: 'absent'if (detected.probe === 'failed')->if (false)const pm = detected.pm->'npm'with pnpm on PATH: prints pnpm consistently+both branches ... validate stepdiffbyte-identical to pristineB3 is the anti-vacuity proof that mattered most: stubbing pnpm did not
neuter the pnpm leg. It still catches the original #10322 regression — and now
catches it deterministically instead of whenever Corepack cooperates.
Each leg produced a distinct, predicted red set, which is what rules out
the "both legs errored identically so the diff read as no breakage" failure.
Canary control — the instrument produces a positive before its negative is trusted
The warning marker was replaced with
CANARY9137, confirmed on disk in bothdirections (real marker 1 -> 0, canary 0 -> 1), and the run reported it exactly
where expected —
src/scaffold-next-steps-pm.test.ts:238:That line is the spawned child's real stdout, so it proves three things at
once: the assertion reads real output rather than mere absence; the source
mutation reaches the spawned process; and the whole
detailchain works (stubexits 1 ->
execSyncthrows with stderr captured -> PATH lookup finds the stub->
probe: 'failed'-> reason printed).No rebuild is involved on either ablation leg, and that is measured rather
than assumed: the test spawns
tsx src/index.tsandBUNDLED_TEMPLATES_DIRresolves to
src/templates, so nothing in this pin readsdist/. The canaryabove is the empirical proof that a source edit reaches the child.
Green in both states — regression guards, NOT red-before evidence
Named rather than counted:
regression guard: pm is pnpm iff the probe succeeded(the Clause ② guard),pnpm simply absent: NO fallback warning(green in B1 only because B1 collapsed toward
absent— it is the pin thatcatches a collapse in the other direction),
the PATH lookup is not consulted when the probe succeeds,a succeeding probe never claims a fallback, and thepre-existing install-failure-remedy source pin.
Checks — each gate's own verdict line, at
60317c537Gate family re-derived from the actual change set with
node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack, andagain after the changeset grew it to 5 paths (13 matched families, all run).
Every one of those ran against tree
60317c537, which is this PR's head — theablation and canary legs both restored byte-identically (
diffempty, treeclean) before the gate sweep was read.
check:type-check-debt— declared not run, with the reason. Its ratchethalf needs the whole workspace closure built, and
create-objectstackcarriesno ledger row for it to move: no
test-typecheck-debt.json, no DEBT entry,and
check:type-check-coveragereports it among the 65/78 fully covered. Bothnew files are inside the tsc program
typecheckruns (verified withtsc --listFiles; onlysrc/templatesis excluded), and that program isgreen. CI runs the ratchet regardless.
Declared narrowing — the ablation legs ran outside the shared verify lock.
Two consecutive acquisitions returned
VERDICT queue-timeout (exit 99) · never acquired · waited 540s, ~18 minutes with no progress, behind another lane'smulti-minute turbo build. The narrowing is measured, not asserted:
create-objectstackhas zero workspace dependencies (no build closure atall) and the earlier superset run — all 14 files, 177 tests — held the lock
for 17 seconds. The whole 5-leg matrix took 111s of single-package vitest.
The full-repo
pnpm lintabove was not narrowed; it ran whole.Changeset
.changeset/create-objectstack-honest-pm-probe.md,patch. Required ratherthan
skip-changeset: the product's reporting changes — a user whose pnpmprobe fails now gets a warning naming the failure instead of silent npm advice.
Files
Declared surface was the
detectPackageManagerseam plusscaffold-next-steps-pm.test.ts. Two files were added to it, both new and soheld by no other claim:
detect-package-manager.ts(the seam, extracted so itcan be injected) and
detect-package-manager.test.ts(its hermetic pins).packages/cli/src/commands/init.ts(#11598) andcompile.ts(#11772) wereread only, never edited — the former's
npm_config_user_agentdetectoranswers a different question ("which PM invoked me") and was deliberately not
adopted here, since switching strategies would change what the tool does. The
TEST_DEBT / type-check-debt ledger (#11788) is untouched.
⛔ Left as ordered: not marked ready, auto-merge not enabled.
Generated by Claude Code