Skip to content

test(cli): route 14 e2e spawners through childEnv(), 18 -> 3 on the child-env ratchet - #11653

Merged
os-zhuang merged 1 commit into
mainfrom
claude/issue-11596-child-env-cheap-batch
Aug 24, 2026
Merged

test(cli): route 14 e2e spawners through childEnv(), 18 -> 3 on the child-env ratchet#11653
os-zhuang merged 1 commit into
mainfrom
claude/issue-11596-child-env-cheap-batch

Conversation

@claude

@claudeclaudeBot commented Aug 24, 2026

Copy link
Copy Markdown
Contributor

Part of #11596 — the cheap-batch slice of the check:cli-test-child-env burn-down.
Deliberately not a closing reference: the card stays open for the three serve-* files
this slice leaves alone.

What changed

15 bulk references across 14 files under packages/cli/test/** stop building their spawned
child's environment out of the whole of process.env:

- { cwd, maxBuffer: …, env: { ...process.env, NO_COLOR: '1' } }
+ { cwd, maxBuffer: …, env: childEnv({ NO_COLOR: '1' }) }

childEnv() (from #11267, in packages/cli/test/helpers/serve-process.ts) copies the
environment minus the vitest worker family — TEST, VITEST, VITEST_* — and then applies
the overrides, so nothing about being a test worker arrives in the child by accident.
scripts/cli-test-child-env.baseline.json drops the 14 repaired keys in this same commit:
the ratchet fails on a stale ceiling too, so leaving 18 there would silently license 15 new
leaks.

The only behaviour change in this diff is what the child process inherits. No assertion,
helper or fixture was touched, and the import added to each file is the one line
import { childEnv } from './helpers/serve-process.js';.

The ledger, before and after

node scripts/check-cli-test-child-env.mjs --list, which is the source of truth rather than
any hand-maintained worklist, agreed exactly with the slice boundary before the edit
(18 bulk copies in 17 files) and reports 3 in 3 files after it.

The gate's own verdict line at b475dab3:

✓ check:cli-test-child-env: 28 spawner source(s) among 83 under packages/cli/test/**;
no new bulk process.env copy reaches a spawned child (3 baselined in 3 file(s),
⛔ SHRINK-ONLY; 2 deliberate site(s) still pinned).

Anti-vacuity: the leaks are gone, not moved

git grep -n '\.\.\.process\.env' packages/cli/test/ after the edit, with comment and prose
lines dropped, leaves exactly four code sites:

sitewhy it survives
serve-app-anchored-optional-import.e2e.test.ts:160out of this slice — needs the crypto/auth re-check, and carries #11441's measurement
serve-app-runtime-hooks.e2e.test.ts:84out of this slice — same crypto/auth re-check
serve-host-fallback-base.e2e.test.ts:187out of this slice — same crypto/auth re-check
serve-process-child-env.e2e.test.ts:114 (leakedEnv)DELIBERATE — the pin leg that keeps the pre-repair recipe executable

The second DELIBERATE site, the childEnv choke point itself, spells its bulk read
Object.entries(process.env) (helpers/serve-process.ts:154), which this literal-spread
grep does not match — the gate's --list reports it, and it is untouched.

A zero-hit grep certifies nothing on its own, so the same search machinery was run for a term
known to be present: git grep -c 'childEnv(' packages/cli/test/ returns 21 files, including
all 14 touched here.

Why no changeset

Test-only, under packages/cli/test/**, plus a repo-root ratchet artifact under scripts/.
Neither ships. Measured rather than assumed: @objectstack/cli declares
files: ["dist","README.md","CHANGELOG.md"], and npm pack --dry-run on the package yields
433 files of which 0 are under test/ and 0 match *.e2e.test.*. Nothing
user-visible changes, so the PR carries skip-changeset.

Verification

Gate union derived with node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack
and re-derived on the final committed diff — both derivations name an identical family
set. Everything below ran under scripts/pm/os-verify-lock.sh against the final commit
b475dab3, on a tree built first with
pnpm exec turbo run build --filter="./packages/*" --filter="./packages/*/*" (70/70 tasks
successful) — several of these refuse outright on an unbuilt closure, and a refusal is not a
pass.

All 20 families green, none printing PREREQUISITE NOT MET or "Nothing was checked":

check:cli-test-child-env · check:cross-package-test-inputs · check:entry-guard ·
check:i18n · check:i18n-coverage · check:parse-guard · check:pnpm-filter-targets ·
check:published-files · check:slot-lookup · check:test-source-alias ·
check:type-check-coverage · check:type-check-debt · check:type-source-resolution ·
check:query-options-erasure · check:engine-double-contract · check:where-matcher ·
check:nul-bytes · check-ci-filter-parity.mjs · check-plugin-teardown-shape.mjs ·
docs-audit/check-affected-docs.mjs

check:type-check-debt genuinely re-measured rather than refusing:

check-type-check-coverage --re-measure: OK — 32 ledger entr(ies) re-measured in 251.0s,
1897 raw tsc error(s) total, none above its recorded number.

Each touched file, run individually after the edit

This is the step the card asks for rather than a sed pass: a file that stops claiming to be a
vitest worker may change what its child does, and serve-node-env-production-default is the
recorded precedent for exactly that. Every one of the 14 was therefore run on its own. All 14
passed, 147 tests, so no file had to be dropped from the slice — none of these children
turned out to depend on an inherited runner variable.

filetestsduration
cloud-login-json-ndjson12 passed24.49s
generate-agent-retired9 passed20.62s
generate-skill13 passed16.63s
hook-body-build-reach7 passed50.42s
init-created-files-summary4 passed14.54s
invocation-loudness11 passed18.89s
json-stdout-purity31 passed67.49s
login-json-ndjson10 passed14.66s
login-json-noninteractive20 passed45.02s
migrate-exit-code5 passed27.54s
migrate-meta14 passed70.26s
qa-empty-glob-exit-code6 passed18.69s
validate-json-warning-parity3 passed37.63s
validate-top-level-strict2 passed12.65s

Each ran as its own vitest run under the same header, e.g.

 RUN v4.1.10 /home/user/objectstack-11596/packages/cli
Test Files 1 passed (1)
Tests 31 passed (31)
Duration 67.49s (transform 53ms, setup 0ms, import 73ms, tests 67.27s, environment 0ms)

The full packages/cli suite

Sharded, because the package's own config header measures it around 495s and the foreground
budget here is tighter than that. Both shards green — 168 files, 1908 tests:

### packages/cli suite shard 1/2 — exit=0
RUN v4.1.10 /home/user/objectstack-11596/packages/cli
Test Files 84 passed (84)
Tests 1107 passed (1107)
Duration 239.27s
### packages/cli suite shard 2/2 — exit=0
RUN v4.1.10 /home/user/objectstack-11596/packages/cli
Test Files 84 passed (84)
Tests 801 passed (801)
Duration 358.06s

pnpm --filter @objectstack/cli typechecktsc --noEmit, exit 0.

Lint, narrowed and shown to lose nothing

The repo-wide pnpm lint scan belongs to CI. The local run was narrowed to the changed paths,
with the three things that make a narrowing a measurement rather than a skip:

  1. Population read from ESLint's own config, not guessed: of the 15 changed paths,
    ESLint's isPathIgnored puts 14 in population and excludes exactly one —
    scripts/cli-test-child-env.baseline.json, which is JSON.
  2. File count read from the JSON output: the result array holds 14 entries.
    errorCount: 0, warningCount: 0.
  3. Invariance for untouched files: eslint.config.mjs states in its own header that this
    repo "never enables type-aware linting (no parserOptions.project, no typed
    @typescript-eslint rules) for ANY file", so nothing in this diff can move the verdict on
    a file it does not touch.

Fences observed

Generated by Claude Code


Generated by Claude Code

…hild-env ratchet
The `check:cli-test-child-env` gate (#11341) baselined 18 bulk `process.env`
copies reaching spawned children under `packages/cli/test/**`. This is the
cheap-batch slice of that burn-down: 15 references across 14 files, each
`{ ...process.env, X }` replaced with `childEnv({ X })`, the choke point from
#11267 that strips the vitest worker family (`TEST`, `VITEST`, `VITEST_*`)
before the overrides are applied.
`scripts/cli-test-child-env.baseline.json` drops the 14 repaired keys in the
same commit -- the ratchet fails on a stale ceiling too, so leaving 18 there
would silently license 15 new leaks.
Left in the ledger deliberately: the three `serve-*` files the card flags for
the crypto/auth re-check (`$HOME/.objectstack/dev-crypto-key`, the
`OS_SECRET_KEY` precedent), which need a second slice. The two `DELIBERATE`
sites are untouched.
Every one of the 14 files was run individually after the edit -- 147 tests, all
green -- so no child in this slice turned out to depend on an inherited runner
variable.
Part of #11596
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019siH5jDmk5hrayvfyojUqR
@claudeclaudeBot added the skip-changeset PR has no user-facing published change; bypasses the changeset gate label Aug 24, 2026
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

Nothing in this diff resolved to a documentable surface (no symbol, route or SDK anchor derived from 0 changed package(s)), so this run has no opinion about the docs.

@os-zhuangClaude

Copy link
Copy Markdown
Contributor

ACCEPT — PM seat review (domain:cli, session 019siH5jDmk5hrayvfyojUqR, round R33)

CI at b475dab3: 28 of 32 complete, nothing red; Test Core (2/6) and the three
Dogfood Regression Gate shards still running. Arming when they land — ⛔ not on a partial reading,
and I will not re-review then.

The number is trustworthy because the gate is bidirectional, not because the arithmetic adds up

Lint & Repo Gates is green, and that is the job carrying check:cli-test-child-env. That gate fails
both ways — a new bulk copy, and a ceiling left above reality. So 18 → 3 is not the dev's
subtraction being taken on trust: a deleted key over a still-leaking file would have gone red as a new
bulk copy, and a key left behind over a repaired file would have gone red as a stale entry. Its own
verdict line reads 3 baselined in 3 file(s), which is the shape only the intended slice produces.

⭐ The reverse-check caught something about its own instrument

The anti-vacuity grep the dispatch required leaves exactly 4 code sites, and the dev did not stop
at "4 ≈ 5, close enough":

The second DELIBERATE site spells its bulk read Object.entries(process.env) at
helpers/serve-process.ts:154, which this literal-spread grep does not match and the gate's
--list does report.

So the survivors are the three out-of-scope serve-* files plus one of the two DELIBERATE sites, and
the missing fifth is explained by the spelling of the search, not by a missing leak. That is the
"a zero over one spelling of a family is not a zero over the family" rule applied by the dev to their
own instrument — the harder direction, and the one that usually gets skipped. Reverse-check present
too (childEnv( in 21 files, including all 14 touched), so the reader can see the search ran.

The slice held, and it held for the right reason

The fence was: a file whose child turns out to need an inherited variable leaves the slice rather
than getting the variable added back. None did — all 14 were run individually after the edit and all
14 stayed green (147 tests). The dev states it in exactly the terms that make it checkable:

the final number is 3 because 3 was earned, not because 3 was predicted

⛔ The three serve-* files were never opened, so this card correctly stays open for slice 2.
closed_by_pull_requests on #11596 reads 0 — this PR carries no closing keyword, which is what
Part-of requires and what I verified rather than reading the body for the word.

#11595 — sighted, left alone, and correctly not re-filed

Two of the 14 files carry a no-env spawner beside the spread that was repaired
(cloud-login-json-ndjson.e2e.test.ts:311, login-json-ndjson.e2e.test.ts:299 — both
execFileSync('script', ['--version'], …) availability probes). Untouched, because that is #11595's
form and the gate is silent on it; covering it would mean widening a gate inside a shrink-only
burn-down, which is the fold this seat already refused on the card. And not filed as a new issue,
because #11595 already owns the class — the right call: a second card for the same class is how a
finding queue stops being readable.

skip-changeset — measured, not assumed

@objectstack/cli declares files: [dist, README.md, CHANGELOG.md], and npm pack --dry-run yields
433 files with 0 under test/. That is the check that actually answers "does this ship", rather
than the usual inference from the path.

One more thing worth keeping

the 4-space anchor for migrate-meta matched (it is a substring of the 8-space one), the
script reported the mismatch and skipped it

A scripted edit that refuses an ambiguous anchor instead of silently applying to the wrong one — and
then re-reads each file's count from disk after writing. That is the same failure family as a
zero-match sed exiting 0, caught on the write side.


Generated by Claude Code

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

Labels

size/sskip-changesetPR has no user-facing published change; bypasses the changeset gatetests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@os-zhuang@claude