Uh oh!
There was an error while loading. Please reload this page.
docs(cli-test): repair the dead VITEST counterfactual in three serve spawner comments - #12504
Merged
Merged
Conversation
…spawners The three `packages/cli/test` serve spawners that supply `OS_SECRET_KEY` explicitly credited the `VITEST` strip (#11267) for putting the child's crypto layer in `development` posture "instead of `test`". That attribution is dead: `detectMode` has read `NODE_ENV` and nothing else since #11448 (`a58eac3e2`, merged 2026-08-23), so stripping `VITEST` cannot move crypto posture in either direction any more. The conclusion the comment supports is untouched and still correct — supply the key, because development posture PERSISTS a minted key to `$HOME/.objectstack/dev-crypto-key`. Only the reason is repaired: these three spawn `bin/run.js` (which pins no `NODE_ENV`) with the variable UNSET, so `serve.ts` assigns `process.env.NODE_ENV = 'development'` in-process for `--dev` before `runtime.start()`. Comment prose only; no executable line changes. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UjujZN219uFzBhSYfMykCd
Contributor
📓 Docs Drift CheckNothing 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. What this run could not see
Coarse fallback — 0 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): |
os-litant
marked this pull request as ready for review
August 26, 2026 07:24
Uh oh!
There was an error while loading. Please reload this page.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes#12498
Comment prose only. Verified mechanically: transpiling each file before and after with
removeCommentsemits byte-identical JS (7970/7970, 9080/9080, 8786/8786 bytes), and no added line carries a@ts-*,eslint-*,///or*-ignoredirective — so nothing executable moved.What was false
The three serve spawners that supply
OS_SECRET_KEYeach carried the same six-line comment — byte-identical across all three, proven rather than eyeballed (md5sumof the block isd8cd78d887531891f0f2f9a1e4b84d98in every file):detectModehas readNODE_ENVand nothing else since #11448 (a58eac3e2, "select crypto posture from the deployment signal, never the test runner", committed 2026-08-23 — sha and date re-derived here withgit log -1 a58eac3e2, not copied from the card). The live function sits atpackages/services/service-settings/src/local-crypto-provider.ts:185onorigin/maine966c5926— a line number read off this tree.So the attribution is dead in both directions: stripping
VITESTno longer selectsdevelopment, and thetest→developmentflip the wording predicted is unreachable, because a leakedVITESTwould no longer producetestposture either.What survives, untouched
The conclusion.
OS_SECRET_KEY: E2E_SECRET_KEYstays in all three, and the sentence explaining it is kept and strengthened rather than weakened: development posture persists a minted key to$HOME/.objectstack/dev-crypto-key, so an explicit key is what keeps these boots out of the runner's home directory and out of a cross-test ordering coupling.Measured on this tree, not argued
A runtime probe drove
LocalCryptoProvideron this worktree's source (each case with its own freshOS_HOME):keySourceNODE_ENV=developmentgenerated-fileVITEST=true,NODE_ENVabsentgenerated-fileVITEST=true,NODE_ENV=developmentgenerated-fileNODE_ENV=development+OS_SECRET_KEYenv:OS_SECRET_KEYNODE_ENV=testephemeralB is the dead counterfactual measured directly: a leaked
VITESTwithNODE_ENVabsent still answersdevelopment, nottest. E is the anti-vacuity control — the probe can produce thetest/ephemeraloutcome the deleted arm used to force, so B's answer is a measurement rather than a blind instrument. D is the conclusion this comment exists for.Each file's spawn shape measured separately
Per the ruling on PR #12495 — which repaired this class in two sibling files and needed different repairs for each — the three were measured, not assumed identical:
NODE_ENVon the childserve-mcp-stdio-answers.e2e.test.tsbin/run.js(line 84)serve -p PORT --devundefined, no call site overridesserve-mcp-capability-collision.e2e.test.tsbin/run.js(line 85)serve -p PORT --devundefined, no call site overridesserve-stdio-stdout-purity.e2e.test.tsbin/run.js(line 81)serve -p PORT --devundefined, no call site overridesThey are identical — and that is the measurement, not an inspection. Every
boot(...)call site in the three files was enumerated; none passesNODE_ENVthrough the...envspread.This is also where they differ from the sibling repaired in PR #12495: that file's children run
bin/run-dev.js, which pinsprocess.env.NODE_ENV = 'development'before argv is parsed. These three runbin/run.js, which pins noNODE_ENVat all (the whole 46-line file was read; the onlyNODE_ENVassignment anywhere underpackages/cli/binisrun-dev.js:28). With the variable unset,serve.tsassignsprocess.env.NODE_ENV = 'development'in-process on theflags.devbranch, well beforeruntime.start(), andresolveDataKeyreads a liveprocess.envwhen it resolves the key — so the posture isdevelopmentby the route theNODE_ENVentry twelve lines below already documents.Census, re-run unnarrowed
Swept all 6,912 tracked files, not just
packages/cli/test. Exactly three sites carry the dead counterfactual and they are the three in this PR — no fourth site, fenced or otherwise. Every other place that quotes the deleted arm already does so correctly in the past tense and was left alone:local-crypto-provider.ts(read-only here),crypto-posture-deployment-signal.test.ts,helpers/serve-process.ts,scripts/check-runner-env-posture.mjs,.github/workflows/lint.yml, and the.changesetfor the deletion.artifact-pinned-boot.e2e.test.tscites #11267 next to its ownOS_SECRET_KEY, but for theTEST/better-auth reason only and underNODE_ENV: 'production'— not this class.Instrument controls, because a zero is only worth what the instrument's "no" is worth: a control term present in all three target files and not a substring of anything searched (
OS_DISABLE_CONSOLE) returns 14 hits across 13 files; an absent token returns zero and exit 1. The instrument says both yes and no over the same population.Gates
Run on
541d9b77b(git rev-parse --short HEADat the final commit), derived withnode scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstackrather than from a hand-written list. Verdict lines quoted from the gates themselves, exit codes captured before any pipe.Green:
check:cli-test-child-env("35 spawner source(s) among 95 … no new bulk process.env copy reaches a spawned child"),check:cross-package-test-inputs,check:objectql-double-limit,check:page-declaration-shape,check:published-files,check:slot-lookup,check:test-source-alias,check:type-check-coverage,check:type-source-resolution,check:query-options-erasure,check:engine-double-contract,check:where-matcher,check:comment-mask-adoption,check:nul-bytes("scanned 6905 text file(s) … no raw ASCII control bytes"),check-ci-filter-parity,check-plugin-teardown-shape,docs-audit/check-affected-docs,docs-audit/check-drift-comment. Repo-widepnpm lint(eslint . --no-inline-config) also runs clean in 58s — no narrowing claimed, the whole tree was linted.NOT MEASURED, declared rather than implied:
check:i18n,check:i18n-coverageandcheck:type-check-debtall stop at PREREQUISITE NOT MET on an unbuilt worktree (the first two need the workspace CLI built; the third refuses--re-measurewithout 56 dependencies'dist/*.d.ts). Those refusals are not failures and say nothing about this tree. CI builds the closure and runs all three. Note also thatpackages/cli/tsconfig.jsonincludes onlysrc, so the package's owntypecheckreads none of these three files — a green there would have been a true sentence about nothing, and is not claimed.Deliberately not touched
packages/services/service-settings/src/local-crypto-provider.ts(read-only; its header quotes the deleted arm correctly, in the past tense),packages/cli/test/serve-node-env-production-default.e2e.test.tsandserve-process-child-env.e2e.test.ts(held by open PR #12495), andpackages/cli/vitest.config.ts(held by open PR #12494).skip-changeset: tests only, nothing published changes.Generated by Claude Code