Skip to content

test(cli): strip NODE_PATH in childEnv() so a spawned CJS resolution measures its real base - #12294

Merged
os-trump merged 1 commit into
mainfrom
claude/issue-11773-childenv-strip-node-path
Aug 26, 2026
Merged

test(cli): strip NODE_PATH in childEnv() so a spawned CJS resolution measures its real base#12294
os-trump merged 1 commit into
mainfrom
claude/issue-11773-childenv-strip-node-path

Conversation

@os-trump

@os-trumpos-trump commented Aug 25, 2026

Copy link
Copy Markdown
Collaborator

Fixes#11773

The defect

A vitest worker runs with NODE_PATH pointing at pnpm's hoisted store
(node_modules/.pnpm/node_modules), which holds everything transitively
reachable anywhere in the workspace. childEnv() stripped exactly the vitest
worker family (TEST, VITEST, VITEST_*) and forwarded everything else, so
NODE_PATH rode into every spawned child.

CJS createRequire().resolve() honours NODE_PATH; ESM import() /
import.meta.resolve ignore it. And it is a fallback, not an override — the
node_modules walk wins whenever it hits — so the store can only turn a MISS
into a HIT. The dangerous direction is therefore an ACCEPTANCE claim ("this base
CAN reach X"): green because the store supplied X, not because the base did.

Spawning a real Node child is this directory's remedy for the resolution-base
collapse an in-process test cannot measure at all (#11412). It escapes Vite's
rewrite — but not this. So a spawned resolution pin routed through CJS was as
vacuous as the in-process one it replaced.

The fix

childEnv() now strips NODE_PATH as well.

⚠️Not folded into VITEST_WORKER_ENV_KEYS, and that is deliberate. The
dispatch framed this as "one more entry in the vitest-family strip"; measuring
it says otherwise, and the report records the divergence. That list is what
vitest sets on its own worker
and serve-process-child-env.e2e.test.ts asserts
over it by name; NODE_PATH is not vitest's — every pnpm bin shim exports one,
so a real serve/dev child in production carries it (that is #4719's whole
history). Folding it in would have made both the constant's name and that pin's
meaning silently wrong. It gets its own named family instead,
RESOLUTION_BASE_ENV_KEYS, with the opposite shape: exact names only, no
prefix class
, because NODE_PATH is a variable real children legitimately
carry and widening it by namespace would strip things nobody measured.

The strip is a default, not a prohibition. overrides are still applied
after it, so a test that reproduces the pnpm bin shim on purpose asks for it
explicitly — which the #4719 pin in
serve-organizations-host-resolution.e2e.test.ts already does
(env: { …, NODE_PATH: hoistedStore }). What changes is that the fidelity is
now declared by the test that wants it rather than inherited by every child.

The pin — and why it is not another vacuous one

The defect being fixed was itself a vacuous pin, so a new pin asserting
NODE_PATH is merely absent from the child environment would repeat that
failure one level up: it would pass against a child that resolves nothing at
all. Every assertion in the new block is the outcome of a real resolution in
a real spawned Node child anchored at packages/types:

legassertionwhat it rules out
ANCHORthe child's CJS resolve.paths()[0] is packages/types/node_modulesthat the CJS leg is measuring some other base (the existing esmReferrer control proves this for ESM only — two different resolvers, neither one's anchor is evidence for the other's)
POSITIVECJS resolves @objectstack/spec, the base's own declared depa probe that can only ever say MISS
NEGATIVECJS cannot resolve chalk (declared by packages/cli, not by packages/types)the base having been erased by the store

withStore — the same child with NODE_PATH put back explicitly — is retained
precisely so the NEGATIVE stays a measurement: it is the same probe answering
the other way, and its HIT names the .pnpm store path it came from, so the
acceptance is provably the fallback supplying chalk rather than the base
reaching it.

Ablation — the instrument produces a positive before its negative is trusted

Mutation: drop RESOLUTION_BASE_ENV_SET.has(key) from childEnv()'s loop, i.e.
restore the defect. Confirmed on disk with anchored greps in both directions
(removed-text-count 0 / injected-text-count 1), restored under
trap … EXIT INT TERM, and the restore leg re-confirmed both ways plus
git status --porcelain empty.

⚠️No rebuild leg, and here is why that is sound rather than skipped: the pin
imports the mutated helper by relative path inside the same package
(import { childEnv } from './helpers/serve-process.js', line 102). Nothing
resolves through a dependency's exports to a dist/, so there is no stale
artifact that could keep a mutated tree green.

legverdict
defect restoredRED — 3 failed / 10 passed
fix restoredGREEN — 13 passed (13)

The three that went red, with what they actually saw:

  • #11773 … NEGATIVE: cannot reach a package its base does not declare
    RESOLVED:…/node_modules/.pnpm/chalk@6.0.0/node_modules/chalk/source/index.js.
    The store supplied chalk. This is the card's defect, reproduced.
  • M2 … CJS honours NODE_PATH → same .pnpm path.
  • M2 … the worker HAS a NODE_PATH, and childEnv() no longer hands it over
    received the worker's real value, three entries ending in
    node_modules/.pnpm/node_modules — the hoisted store, exactly as filed.

⚠️#11773 ANCHOR and #11773 POSITIVE were green in both states. They are
named here as regression guards / instrument-validity controls, never as
red-before evidence — being green in both states is their design.

Fallout measurement

This was the load-bearing half of the dispatch: a test legitimately relying on
inherited NODE_PATH would surface immediately.

Result: nothing relies on it. packages/cli — 180 files, 2048 tests, 2048
passed, exit 0.

⚠️ One honest detour, recorded because the first reading was wrong. On the first
sweep serve-node-env-production-default.e2e.test.ts failed with
Error: command serve not found. That is not fallout: packages/cli/dist
did not exist in the worktree (building --filter '@objectstack/cli^...' builds
the dependency closure, not packages/cli itself), and that file spawns the
shippedbin/run.js, which its own header says needs "a genuinely built
dist/". After pnpm --filter @objectstack/cli build, the whole
bin/run.js class — all 8 files, 45 tests — passes, and so does the full suite.

Notably green throughout: serve-organizations-host-resolution.e2e.test.ts, the
#4719 pin that does want the pnpm shim shape. It passes NODE_PATH explicitly,
so the strip cannot reach it — which is the design, not luck.

Verification

Gate family re-derived with node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack against the actual change set, at
52a8b9fd — the final commit, which is also the tree every run below was
measured on.

  • check:cli-test-child-envexit 0, its own line: "✓ check:cli-test-child-env: 33 spawner source(s) among 92 under packages/cli/test/**; no new bulk process.env copy reaches a spawned child (0 baselined in 0 file(s), ⛔ SHRINK-ONLY; 2 deliberate site(s) still pinned)."
  • check:test-source-aliasexit 0: "check-test-source-alias OK — 72 packages with tests scanned; 61 registered as still resolving a workspace dep through dist/; 45 published subpath(s) resolved through every alias table."
  • exit 0 each: check:published-files, check:slot-lookup,
    check:type-source-resolution, check:type-check-coverage,
    check:engine-double-contract, check:cross-package-test-inputs,
    check:where-matcher, check:query-options-erasure, check:i18n,
    check-plugin-teardown-shape.mjs, check-nul-bytes.mjs
  • pnpm lintfull repo, not narrowed — exit 0
  • pnpm --filter @objectstack/cli exec tsc --noEmit — exit 0

Not measured, declared rather than counted:

  • check:i18n-coverageREFUSED — its own words: "COULD NOT MEASURE — 1 of
    12 config(s) failed to lint"
    , because @objectstack/connector-mcp has no
    build output in this worktree; it adds "this result says NOTHING about whether
    any declared label went untranslated"
    . A refusal, not a pass and not a
    failure. It needs a full-workspace build, and this diff touches no i18n
    surface.
  • check:type-check-debt --re-measure — not run locally; it requires the full
    workspace build for the same reason. The half that this diff could move —
    whether the edited test files typecheck — is the green tsc --noEmit above.

Why no changeset

skip-changeset. packages/cli publishes ["dist", "README.md", "CHANGELOG.md"];
this diff is packages/cli/test/** plus one comment in a scripts/ gate header.
Nothing published changes.

Footprint note

Beyond childEnv() and the pin, this touches two things, both consequences of
the strip rather than drive-by edits:

  1. vitest-resolution-base-collapse.e2e.test.ts — its M2 block asserted the
    old behaviour (childEnv() hands NODE_PATH to the child), so the strip
    necessarily moves it. Its inherited leg became withStore, built with an
    explicit override, which keeps the platform measurement alive.
  2. scripts/check-test-source-alias.mjs header — it instructed authors that
    "childEnv() forwards it … a spawned pin whose claim routes through CJS must
    strip it (childEnv({ NODE_PATH: undefined }))"
    . This PR is what makes that
    sentence false, so correcting it is part of the change, not a rider. No gate
    logic touched. ⛔ scripts/check-cli-test-child-env.mjs deliberately untouched
    — that is finding: 8 packages/cli/test spawners pass no env at all, so the child inherits the vitest worker environment verbatim — the purer form of #11341's leak, and the new gate is silent on it #11595's, and a domain:devx surface.

#11595 remains queued behind this and is not addressed here.

…measures its real base
A vitest worker runs with NODE_PATH pointing at pnpm's hoisted store, which
holds everything transitively reachable anywhere in the workspace. childEnv()
stripped only the vitest worker family, so NODE_PATH rode into every spawned
child. CJS createRequire() honours it (ESM import() does not), and it is a
FALLBACK rather than an override -- so the store can only turn a MISS into a
HIT. That makes an ACCEPTANCE claim ("this base CAN reach X") green because the
store supplied X, not because the base did: spawning a real Node child escapes
Vite's rewrite but not this, so a spawned resolution pin routed through CJS was
as vacuous as the in-process one it replaced.
childEnv() now strips NODE_PATH too, under its own named family
(RESOLUTION_BASE_ENV_KEYS) rather than folded into VITEST_WORKER_ENV_KEYS:
NODE_PATH is not something vitest sets, every pnpm bin shim exports one, and a
real serve/dev child in production carries it. Overrides still apply after the
strip, so a test reproducing the shim shape asks for it explicitly -- which the
#4719 pin already does.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HbG3rGVLjZStHQxHDtzJdJ
@os-trumpos-trump added skip-changeset PR has no user-facing published change; bypasses the changeset gate and removed size/m labels Aug 25, 2026 — with Claude
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

3 anchor(s) derived from 1 changed package(s); no hand-written page names any of them, so this run has nothing to listnot 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
  • the SDK route bridge reached 45 of 222 client-bound route-ledger rows — the other 177 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 177: 14 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 56 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 107 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.

Coarse fallback — 23 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 0acadda3dd414288514a20cba128ab5d9d381bd2packageMentionDocs.

Which tree this was computed on

This run read content/docs from f2d95b6654c7c9170973898d5ced5c2220e13232 — the merge of head 52a8b9fd1438f43d8f79be2bae7ff19117021001 into base 0acadda3dd414288514a20cba128ab5d9d381bd2, 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 f2d95b6654c7c9170973898d5ced5c2220e13232 && git checkout f2d95b6654c7c9170973898d5ced5c2220e13232
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 0acadda3dd414288514a20cba128ab5d9d381bd2 52a8b9fd1438f43d8f79be2bae7ff19117021001 && git checkout -B drift-repro 0acadda3dd414288514a20cba128ab5d9d381bd2 && git merge --no-ff 52a8b9fd1438f43d8f79be2bae7ff19117021001
node scripts/docs-audit/affected-docs.mjs --json 0acadda3dd414288514a20cba128ab5d9d381bd2

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

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

skip-changesetPR has no user-facing published change; bypasses the changeset gatetests

Projects

None yet

2 participants

@os-trump@claude