Skip to content

fix(cli): os serve defaults NODE_ENV to production when unset - #11268

Merged
os-zhuang merged 5 commits into
mainfrom
claude/issue-11113-serve-node-env-production-default
Aug 23, 2026
Merged

fix(cli): os serve defaults NODE_ENV to production when unset#11268
os-zhuang merged 5 commits into
mainfrom
claude/issue-11113-serve-node-env-production-default

Conversation

@os-elon

@os-elonos-elon commented Aug 23, 2026

Copy link
Copy Markdown
Collaborator

Fixes#11113

The fix

packages/cli/src/commands/serve.ts (~1093-1116): the --dev branch that already
defaults process.env.NODE_ENV to 'development' when unset now has a sibling else
branch that defaults it to 'production' when unset and --dev was not passed. Both
branches sit at the same point in run() — before any await import(...) in the file
and before every environment gate the boot reaches:

if(flags.dev){if(!process.env.NODE_ENV)process.env.NODE_ENV='development';}elseif(!process.env.NODE_ENV){process.env.NODE_ENV='production';}

os start already forces NODE_ENV='production' on the unset case
(start.ts:347), but on localEnv — a child environment assembled for a spawn.
os serve runs in-process, so there is no child env to default; the equivalent has
to mutate process.env.NODE_ENV itself, at the same point that already has to be early
enough for the --dev case. An explicitly-set NODE_ENV (development, test,
anything else) is never touched — both branches are if (!process.env.NODE_ENV).

Ordering proof

The trap this card calls out by name: setting the default anywhere below the first
await import(...) (line ~1250) or below the first gate read (portAutoShiftAllowed at
~1114, mode at ~1140) would look identical in a source diff and still ship with the
door open, because those reads would already have run against undefined.

Proof, not assumption: the regression pin (below) spawns the built, shipped
entrypoint (bin/run.js), asserts the observable behaviour of a gate several layers
below serve.ts (plugin-auth's CSRF/origin middleware, reached through better-auth's
own request pipeline) on a real HTTP response, and is reverse-verified against the
pre-fix tree. If the default were set too late, this pin would still fail post-fix —
it does not.

Behavior-flip table

Every NODE_ENV-reading predicate in the tree (excluding *.test.ts), surveyed at
origin/main. Two populations, kept distinct — conflating them over-reports:

Population A — reads the raw variable with !== 'production' (or an equivalent
"default when absent" shape). These flip
, because unset used to read as "not
production" and now reads as 'production':

#File : lineWhat it gatesBefore (unset)After (unset → 'production')
1plugin-auth/auth-manager.ts:1927localhost trustedOrigins wildcard substitutionsubstituted (open)omitted (closed)
2plugin-auth/auth-manager.ts:3672CSRF Origin/Referer synthesis for headerless requestssynthesized (open)not synthesized (closed)
3plugin-auth/auth-manager.ts:3161 (generateSecret)fallback to forgeable dev-secret-<timestamp> when OS_AUTH_SECRET unsetfalls back, warnsthrows, refuses to boot
4plugin-auth/auth-manager.ts:2625invitation-email accept URL printed to console when no email service wiredprintedsuppressed
5plugin-auth/auth-manager.ts:2788magic-link URL printed to console when no email service wiredprintedsuppressed
6plugin-auth/auth-manager.ts:5149 (isPhoneOtpDeliverable)OTP advertised deliverable when SMS transport is log-onlyadvertised deliverableadvertised NOT deliverable
7plugin-dev/dev-plugin.ts:126 (assertNotProduction, ADR-0115 D6)refuse to initialize the dev assembly (well-known auth secret + seeded admin)does NOT refuserefuses (escape hatch: OS_ALLOW_DEV_PLUGIN)
8driver-sql/sql-driver.ts:9188 (isProductionEnv)auto-apply safe schema drift when autoMigrate: 'safe'auto-appliesrefuses, warns to run os migrate
9service-sms/sms-service.ts:53 (LogSmsTransport)OTP/message body printed to logsprintedsuppressed
10service-settings/local-crypto-provider.ts:134 (detectMode)tolerate an auto-generated / ephemeral crypto keytoleratedrequires a stable key (file or env); throws otherwise
11runtime/standalone-stack.ts:710artifact-file HMR polling watcheronoff
12metadata-protocol/seed-loader.ts (resolveEnvConfig)env-scoped Seed.env datasetsunresolvable → seeded EVERYWHERE (fail-open), with a warningresolves to 'prod' → scoped correctly

Rows 3 and 10 are the two most consequential: a deployment that boots today purely
because it never set OS_AUTH_SECRET / never configured a stable crypto key will now
refuse to boot instead of running with a forgeable session secret / an ephemeral
data-encryption key. That is os serve gaining the exact guarantee os start has had
since #5673 — not a new hazard, and not something this PR carves an exception for.

Not flipped, deliberately checked and excluded — already correct before this
change:

File : lineShapeWhy it doesn't flip
serve.ts:1140-1141 (mode)NODE_ENV === 'test' ? 'test' : (NODE_ENV || 'production')already defaults unset to 'production' — this computes the .env* cascade selector, never the raw variable
serve.ts:1114 (portAutoShiftAllowed), serve.ts:1188 (isDev)flags.dev || NODE_ENV === 'development'equality-tests 'development'; unset and 'production' both fail it identically
start.ts:156-157 (mode)same shape as serve's modesame reason
doctor.ts:159 (doctorNodeEnv), doctor.ts:205 (nodeEnvCheck)NODE_ENV || 'production', if (NODE_ENV) …os doctor is a separate process — unaffected by this fix; its own fallback already reads unset as production
spec/discovery.zod.tsresolveDiscoveryEnvironment (http-dispatcher.ts, metadata-protocol/protocol.ts)explicit if (spelling === '') return 'production'the 2026-08-06/07 rulings (#5673, #5936) already closed this one; the /discoveryenvironment field was already 'production' on an unset boot
plugin-auth/auth-plugin.ts:1485 (maybeSeedDevAdmin), runtime/app-plugin.ts:1379 (hot-reload seeder), plugin-auth/auth-manager.ts:4300 (devSeedAdmin)NODE_ENV !== 'development' / === 'development'equality-tests 'development'; unset was already excluded, 'production' still is
service-analytics/analytics-service.ts:701 (debugSql)NODE_ENV === 'development'same shape
service-datasource/sqlite-driver-fallback.ts:113, default-datasource-driver-factory.ts (doc), standalone-stack.ts:566 (factoryDev)NODE_ENV === 'development'same shape

Out of scope for the survey: verify/harness.ts (test harness that force-sets
NODE_ENV='development' for its own in-memory posture, unrelated to any real boot) and
bin/run-dev.js (the tsx dev-shim entrypoint, which unconditionally sets
NODE_ENV='development' and is never used to serve a real deployment).

⚠️ A measured correction to the ruling's own regression-pin wording

The ruling's condition names the pin as "os serve with unset NODE_ENV ⇒ the localhost
trusted-origin convenience gate is CLOSED." Measured against os serve's own default
wiring (letting the CLI auto-inject AuthPlugin, the shape the wording most naturally
reads as), that statement is already true today, unconditionally, unrelated to this
fix
— and stays true with the fix reverted. Two things in serve.ts mask row 1 of the
table above from that path entirely:

  1. serve.tsalways pushes the resolved baseUrl's origin into trustedOrigins
    before handing it to AuthPlugin, so auth-manager.ts's own !origins.length gate
    never sees an empty array through that path.
  2. serve.ts has its own, separate localhost-wildcard convenience
    (if (isDev && …) trustedOrigins.push('http://localhost:*')), gated on
    isDev = flags.dev || NODE_ENV === 'development' — an equality test against
    'development' that was never open on unset NODE_ENV to begin with.

So a pin built against the auto-injected path would be green with the fix reverted
vacuous, one layer further down than the ordering trap the card names explicitly. The
gate row 1 is actually about — auth-manager.ts's own substitution — is reached by a
host app that constructs AuthPlugin itself without pre-populating trustedOrigins (a
supported, real shape: serve.ts's hasAuthPlugin check exists precisely to detect and
defer to it). The committed pin uses that shape. Full reasoning and the measured evidence
are in the pin file's own header comment
(packages/cli/test/serve-node-env-production-default.e2e.test.ts).

I did not widen the fix to change serve.ts's own auto-injection wiring (rows outside
this card's scope) — flagging this as the pin's target, not as a defect to fix here.

Regression pin (anti-vacuity)

New file: packages/cli/test/serve-node-env-production-default.e2e.test.ts. Boots the
real, builtbin/run.js (not the tsx dev shim — that unconditionally sets
NODE_ENV=development before argv is even parsed, which would make the unset-NODE_ENV
input this card is about unreachable) three times, each with an explicit OS_AUTH_SECRET
threaded into the fixture's own new AuthPlugin({ secret }) (so the pin measures only the
trusted-origin gate, never the orthogonal "AuthPlugin.init() throws: secret is required"
path), and POSTs a bogus sign-in with Origin: http://localhost:<a port different from the one the server bound to> and no cookie — a shape better-auth's CSRF middleware forces an
origin check on before it looks at the credentials, so the outcome is observable
independent of whether the sign-in itself would have succeeded:

  • NODE_ENV unset → asserts 403 with body.code === 'INVALID_ORIGIN' (gate CLOSED).
    This is the card's regression pin, on the fixture shape that actually reaches the
    gate (see the correction above).
  • NODE_ENV=development (explicit) → asserts the gate stays OPEN (not
    403/INVALID_ORIGIN) — proves the default never overrides an explicitly-set value.
  • NODE_ENV=test (explicit) → same assertion, same reason.

Reverse-verified, real measurements (packages/cli built both ways, pnpm --filter @objectstack/cli exec vitest run test/serve-node-env-production-default.e2e.test.ts):

  • Pre-fix tree: × NODE_ENV unset: … (regression pin)AssertionError: expected 401 to be 403, naming the exact gate (the trusted-origin substitution left the probe's
    mismatched-port origin accepted; better-auth answered 401 INVALID_EMAIL_OR_PASSWORD instead of 403 INVALID_ORIGIN). The other two legs passed.
    Tests 1 failed | 2 passed (3).
  • Post-fix tree (this PR): Tests 3 passed (3).

A second manual measurement (curl, outside any test harness) on the same pre/post
builds confirms the identical flip at the HTTP layer:
{"message":"Invalid origin","code":"INVALID_ORIGIN"} (403) post-fix vs.
{"message":"Invalid email or password","code":"INVALID_EMAIL_OR_PASSWORD"} (401)
pre-fix, for the same probe.

A second, unrelated trap the pin's own header documents

Early attempts at this pin passed regardless of the fix, both directions — a silent
vacuity one layer below the one the ordering trap warns about. Cause: env: { ...process.env, … } when constructing the spawned child's environment carries the
vitest worker's own env
, and vitest's worker sets TEST=true unconditionally.
better-auth 1.7.1 reads TEST directly (@better-auth/core/env's
isTest = () => nodeENV === 'test' || toBoolean(env.TEST)), and create-context.mjs
defaults skipOriginCheck: … isTest() ? true : falseindependent of NODE_ENV. A
child that inherits TEST=true has origin validation disabled by better-auth itself, no
matter what this fix does. The pin now explicitly unsets TEST for the spawned child,
the same way it unsets NODE_ENV, with the mechanism documented inline. This is a latent
footgun for any other e2e test in this repo that spawns a real os serve under vitest
and cares about auth/CSRF behaviour while spreading ...process.env (the shared
runServe() helper included) — filed as #11267 rather than fixed repo-wide here.

Explicit-NODE_ENV legs

Covered by the pin's second and third it() blocks above (development, test) —
both pass on the fixed tree, proving the production default never overrides an
explicitly-set value.

The CI red, and why closing it took a build-graph fix (handover, R31)

Test Core (2/6) was red at adb1b9ea8c with only this PR's own new pin failing
(1 failed / 1824 passed; check-test-completeness accounted for all 1825, so nothing
was silently skipped):

##[error]Error: serve did not reach "Server is ready"
--- stderr ---
› Error: command serve not found

⛔ Not the fork-back condition, re-derived rather than inherited: the CLI never started,
so no NODE_ENV-reading predicate in the table above was ever evaluated. No gate outside
the table flipped, and no legitimate workflow is implicated. ⛔ Not the Corepack/undici
infrastructure signature of #11300 either — that job set STORE_PATH and ran 1825 tests.

The mechanism — this card's own subject matter, turned back on the harness

turbo.json declared "@objectstack/cli#test": { "dependsOn": ["^build"] }
dependencies only, never the package's own build. Measured with
pnpm exec turbo run test --filter=@objectstack/cli --dry=json: 58 tasks, 57 of them
#build, and @objectstack/cli#buildabsent from the test task's resolved
dependency list. So packages/cli/dist does not exist when the shard runs, and never has.
invocation-loudness.e2e.test.ts:34-35 already states that fact in prose.

The question that fact alone cannot answer is why the four other files in the same
161-file suite that spawn bin/run.js all passed in the same job. The answer is
@oclif/core's tsPath(). It skips the TypeScript path lookup only when isProd(), and
lib/util/util.js:66 defines that as:

functionisProd(){return['development','test'].includes(process.env.NODE_ENV??'');}

Under 'development' or 'test', oclif silently rewrites the command target from the
declared ./dist/commands to ./src/commands and auto-transpiles. Two of those files
only assert the path as a string; the three that actually spawn it
(serve-mcp-stdio-answers:115, serve-mcp-capability-collision:119,
serve-stdio-stdout-purity:109) each pass NODE_ENV: 'development' to the child for its
--dev admin seed — so they have never touched dist/ at all, and the undeclared
build dependency stayed invisible for as long as it did.

This pin cannot dodge it. Unset NODE_ENV is the input under test, and unset is exactly
the value that leaves isProd() true and the reroute off. Measured directly, dist absent:

child NODE_ENVoutcome
unset › Error: command serve not found (byte-identical to CI)
productionsame
testresolution proceeds into src/ (task: findCommand (compile))
developmentsame

this PR's pin is the only file in packages/cli that genuinely consumes
packages/cli/dist, and the first to depend on a prerequisite the graph did not declare.

Corroborated by running the pre-existing serve-mcp-stdio-answers.e2e.test.ts on the same
dist-absent tree: it fails identically (Test Files 1 failed (1), Tests 1 skipped (1)).

The fix: the seam, not the symptom

turbo.json, one key — "@objectstack/cli#test""dependsOn": ["build"] (turbo's own
build task already dependsOn: ["^build"], so this is a superset, not a replacement).
Same shape as "@objectstack/metadata#test" 38 lines above in the same file.

Rejected alternative: having the pin build or verify dist itself, which would have
stayed inside this PR's original four-file surface. It is consumer-side compensation for a
producer-side omission: it makes one test file responsible for running tsc, nests a
turbo invocation inside the turbo task already running test, and — hoisted to a
globalSetup — would tax all 161 files and force a test block into a config whose header
documents why it deliberately has none. The deciding reason is the fourth: it leaves the
task graph still asserting a false dependency, so the next author who writes a built-CLI
test here hits the identical wall with no signal. Widening the surface by this one key was
ruled on explicitly before it was edited.

Measured cost: 9s. pnpm --filter @objectstack/cli run build on a tree whose
dependency closure is already built — the shard's exact state after ^build — ran in 9s,
and ~0 on a turbo cache hit.

Verified end-to-end through the real mechanism, not by inspection: rm -rf packages/cli/dist, then
pnpm exec turbo run test --filter=@objectstack/cli -- test/serve-node-env-production-default.e2e.test.ts
Tasks: 57 successful, 57 total / Cached: 0 cached / Test Files 1 passed (1) /
Tests 3 passed (3), with packages/cli/dist/commands/serve.js present afterwards.

The bounded retry is removed — because the theory was falsified, not because a rule forbids it

adb1b9ea8c added bootServeWithRetry / looksLikeMissingCliCommand, retrying once on
oclif's exact "command not found" sentence, on the theory that the failure was a transient
dist/commands glob read under this suite's concurrent-spawn load. That retry was
already live in the failing job 97166275854 and did not change the outcome.
The failure
is deterministic — the NODE_ENV table above reproduces it on demand — so there was never
a transient to absorb. The measurement is what makes removing it safe rather than dogmatic;
the file returns to its reviewed pre-retry shape, and the prerequisite is now declared in
the build graph where it is true for whoever writes the next built-CLI test here.

The packages/cli/vitest.config.ts alias from that same commit stays: it is a separate,
correct fix for check:test-source-alias, and that gate is green.

Reverse-verified again on the final tree

Both legs rebuilt, the mutation confirmed on disk by anchored grep counts before each
rebuild and inside dist/ by scripts/ablation-dist-preflight.mjsafter it (marker
else if (!process.env.NODE_ENV), verified unique to dist/commands/serve.js); exit codes
captured by redirect-then-capture, never through a pipe; a trap … EXIT INT TERM restored
and rebuilt on every leg.

  • Fix ablated✓ marker absent from all 420 built files
    × NODE_ENV unset: the localhost trusted-origin convenience gate is CLOSED (regression pin),
    AssertionError: expected 401 to be 403 // Object.is equality,
    Test Files 1 failed (1) / Tests 1 failed | 2 passed (3).
  • Fix restored✓ marker present in 1 built fileTest Files 1 passed (1) /
    Tests 3 passed (3).

So the pin still catches the original defect on exactly the tree this change produces, and
the only difference between the CI red and 3/3 green is whether packages/cli/dist exists.

Out-of-scope finding, filed not fixed

#11317 (unassigned): the three e2e files above each carry the comment
"bin/run.js — the SHIPPED entrypoint" while never reaching dist/, for the tsPath
reason measured here. Distinct from #11267 (inherited TEST=true / better-auth origin
skip) — same family, different mechanism. Not fixed here; this PR's surface is the CI red.

Gate results

Re-run in full at 98e8399f (the final head, after the handover commit), workspace
closure built first — every line below re-measured there, all green, all exit codes
captured before any pipe. The gate list was re-derived at that head with
node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack (self-derived
from the merge base); adding turbo.json named no new family. Two additions to what
the list below already recorded: pnpm check:type-source-resolution
(OK — 77 packages with a tsconfig.json scanned), which the derivation names and the
earlier run omitted, and pnpm check:type-check-debt --re-measure, which is now a real
verdict at this head rather than a refusal: OK — 33 ledger entr(ies) re-measured in 232.7s, 1897 raw tsc error(s) total, none above its recorded number.
pnpm check:single-claim-paths is green and turbo.json is not one of its declared
paths (the list holds only .objectui-sha), so contention was checked directly instead:
none of the 12 other open PRs touches turbo.json.

Originally built and measured at 61810a5a0e: @objectstack/cli and its full workspace
dependency closure.

  • pnpm --filter @objectstack/cli typechecktsc --noEmit, clean, no output.
  • pnpm --filter @objectstack/cli exec vitest run test/serve-node-env-production-default.e2e.test.tsTest Files 1 passed (1) /
    Tests 3 passed (3).
  • node scripts/pm/dispatch-gates.mjs (no paths — self-derived from the merge base),
    every named local gate run:
    • pnpm check:changeset-gate-self-tests — ✓ (all three self-tests pass)
    • node scripts/check-empty-changeset.mjs✓ No empty-frontmatter changeset introduced
    • node scripts/check-adr-0087-registration.mjs✓ 1 declared-breaking changeset(s), each carrying an ADR-0087 disposition ([BREAKING] not-required (no-migration-prescription))
    • node scripts/check-changeset-no-major.mjs✓ This diff introduces no major bump
    • pnpm check:objectui-changeset✓ objectui-range --self-test: all checks passed
    • pnpm check:published-files — ✓ (self-test + repo check both pass)
    • pnpm check:route-envelope — ✓ (existing ratchets unmoved by this diff)
    • pnpm check:slot-lookup✓ ratchet holds … none new
    • pnpm check:test-source-aliascheck-test-source-alias OK
    • node scripts/check-ci-filter-parity.mjsOK: all 88 declared cross-package glob(s) …
    • node scripts/check-cross-package-test-inputs.mjsOK: 14 package(s) read outside themselves, all declared
    • node scripts/check-plugin-teardown-shape.mjs — ✓ (baseline fully burned down, unmoved)
    • node scripts/docs-audit/check-affected-docs.mjs — self-test passes, exit 0
    • pnpm check:type-check-coverageOK — 65/78 workspace packages type-checked … (unmoved by this diff)
    • Convention-triggered by the new test file, run explicitly:
      • pnpm check:query-options-erasure✓ ratchet holds: … none new
      • pnpm check:engine-double-contractOK — 384 pinned, 133 in the DEBT ledger, 2 exempt (unmoved)
      • pnpm check:where-matcher✓ conformance holds: 282 matcher(s) … 0 silently-wrong … none new
      • node scripts/check-nul-bytes.mjsOK (scanned 6529 text file(s) … no raw ASCII control bytes)
  • pnpm check:type-check-debt --re-measure (full workspace closure built first) —
    check-type-check-coverage --re-measure: OK — 33 ledger entr(ies) re-measured in 266.0s, 1897 raw tsc error(s) total, none above its recorded number. First
    measurement caught a real regression: the new pin's spawn(..., { stdio: ['ignore', 'pipe', 'pipe'] }) was cast to ChildProcessWithoutNullStreams, which has a
    stdin, when the actual return type (ChildProcessByStdio<null, Readable, Readable>) doesn't — @objectstack/cli's TEST_DEBT ledger moved 146 → 147
    (TS2352). Fixed by typing it correctly; re-measured green.

Changeset

.changeset/serve-node-env-production-default.md, @objectstack/cli: minor. This is
a deliberate default flip with the breakage profile spelled out above (rows 3 and 10 in
particular can turn a previously-booting unset-env deployment into a boot refusal), not a
patch-level bugfix — argued explicitly in the changeset body per this card's own
instruction. Carries an ADR-0087 not-required (no-migration-prescription) disposition:
this changes a CLI runtime default, not an authorable metadata contract — confirmed by
check-adr-0087-registration.mjs above.

Out of scope

Generated by Claude Code


Generated by Claude Code


Generated by Claude Code

os start already forces NODE_ENV='production' on the unset case, but on the
child environment it assembles for its spawn. os serve runs in-process, so
there was no equivalent write, and the whole family of NODE_ENV !==
'production' gates across the tree read the raw undefined and took the
non-production branch on a boot that never declared itself anything else.
Adds the same default os serve already applies for --dev, at the same early
point in run() - before any dynamically-imported runtime module and before
every gate downstream reads the variable. An explicitly-set NODE_ENV
(development, test, anything else) is never overridden.
…xture
Measured: os serve's own auto-injected AuthPlugin wiring always seeds
trustedOrigins with the resolved baseUrl origin and carries its own
isDev-gated localhost wildcard, so auth-manager.ts's NODE_ENV-gated
substitution is unreachable through that path regardless of NODE_ENV. The
pin now uses a host-authored AuthPlugin construction, which is the shape
that actually reaches the gate, and reverse-verifies against the pre-fix
tree.
Also strips the vitest worker's own TEST env var from the spawned child:
better-auth 1.7.1 reads TEST directly (independent of NODE_ENV) to decide
whether to skip origin validation entirely, and leaving it inherited made
the pin pass regardless of the fix.
…config
spawn(..., { stdio: ['ignore', 'pipe', 'pipe'] }) returns
ChildProcessByStdio<null, Readable, Readable> (no stdin), not
ChildProcessWithoutNullStreams. TS2352 on the `as` cast was the one raw tsc
error the TEST_DEBT re-measure ledger caught (146 -> 147) that the package's
own `typecheck` script cannot see, since packages/cli hides its test/ tree
from tsc.
@github-actions

github-actionsBot commented Aug 23, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/cli, touching 1 documentable anchor(s).

16 hand-written doc(s) name something this change touched — list omitted above 15 rows. Re-derive on the tree named below: node scripts/docs-audit/affected-docs.mjs --json e278a2970d2dbdb662db66dd61bc07264157fa51.

4 release-owned page(s) also affected — read-only, see AGENTS.md Documentation Guardrails.

What this run could not see
  • 1 changed file(s) yielded no anchor (packages/cli/vitest.config.ts) — pages documenting those are invisible to this run
  • 1 name(s) were too generic to anchor anything (single lowercase words)

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 e278a2970d2dbdb662db66dd61bc07264157fa51packageMentionDocs.

Which tree this was computed on

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

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

Advisory only, and a precision-first one (#9192): a page is listed because it names a
symbol, wire route or SDK method this diff touched — not because it mentions a changed
package. Each row says which anchor put it there, so a wrong row is reportable rather than
merely annoying. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs e278a2970d2dbdb662db66dd61bc07264157fa51 → pass the list as
args.docs, on the commit named under Which tree this was computed on.

…te + spawn-load flake)
Lint & Repo Gates: check:test-source-alias flagged @objectstack/plugin-auth as
a new unaliased artifact import — the gate's dependency-free text scanner
cannot tell a fixture-config string (written to a file for a spawned child
process) apart from a real import in this file. Added the gate's own
prescribed remedy: an anchored resolve.alias entry in packages/cli/vitest.config.ts.
Verified locally: `pnpm check:test-source-alias` green.
Test Core (2/6): the pin's first sub-test failed once with oclif's own
"Error: command serve not found" before reaching "Server is ready" — 1824 of
1825 other tests in the same shard passed, and the ordering fix in serve.ts
runs strictly after command resolution, so it cannot be the cause. This
package's own vitest.config.ts header documents ~20 files (this one now among
them) that spawn the built CLI as a real child process under heavy concurrent
load as the suite's dominant cost (56.1% of wall time) — a shape this repo's
own scripts/cli-build-prerequisite.mjs names as the canonical signature of a
`dist/commands` glob read that transiently looks incomplete under load (no
oclif.manifest.json cache in this repo — every invocation re-globs).
Added a bounded, signature-scoped retry (bootServeWithRetry) that retries
EXACTLY ONCE, and only when the failure text matches that exact oclif
sentence (looksLikeMissingCliCommand, replicating cli-build-prerequisite.mjs's
line-flattening) — any other failure shape (real assertion, real crash, a
timeout with a different tail) still fails unretried, so this cannot mask a
genuine regression.
Verified locally (this commit, workspace closure rebuilt first):
- `pnpm --filter @objectstack/cli build` — clean
- `pnpm --filter @objectstack/cli typecheck` — clean
- `pnpm --filter @objectstack/cli exec vitest run test/serve-node-env-production-default.e2e.test.ts` — 3/3 passed
- `pnpm --filter @objectstack/cli exec eslint test/serve-node-env-production-default.e2e.test.ts vitest.config.ts` — clean
Not yet done at push time: a dedicated local reproduction of the CI spawn-load
flake (an in-progress 24-file concurrent-spawn run was interrupted, inconclusive
either way — this remains a documented-but-unreproduced-locally diagnosis, not
a confirmed one) and check:type-check-debt --re-measure on this exact diff.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019bmVFqoQPq63zhKrxdYG1r
@os-elonClaude

Copy link
Copy Markdown
CollaboratorAuthor

PM seat — shift ending. Not enqueued. Recording a split verdict on the two fixes so the next seat does not inherit only the dev report's recommendation.

Fix 1 — check:test-source-alias — accepted

The gate's text scanner matched a literal import … from '@objectstack/plugin-auth'inside a fixture-config template that the pin writes to a file for a spawned child process — never resolved through this file's own module graph. The remedy applied is the gate's own prescribed one (an anchored resolve.alias entry). Correct, and the same family this lane hit twice today with check:cross-package-test-inputs: a text scanner cannot tell a string literal from a real specifier, and the fix is to satisfy the convention, never to widen the gate.

Fix 2 — the scoped retry — ⛔ declined as the resolution

The dev's reasoning is careful and the scoping is genuinely narrow (one signature, exactly one retry, any other failure shape still fails). I am still declining it, for three reasons:

  1. This seat's standing constraint is explicit: a test is never skipped, quarantined, or retry-wrapped to get green. Narrow scoping makes it a smaller instance of the forbidden shape, not a different one. That rule is not mine to relax at the end of a shift.
  2. The cause is inferred, not measured. The report says so plainly and honestly: the local reproduction was attempted and did not finish — "inconclusive, not a confirmed local repro". A retry built on an unreproduced cause is a guard whose trigger nobody has observed. If the inference is wrong, the retry hides whatever the real cause is.
  3. If the inference is right, it is a finding about the test infrastructure, not this PR's to absorb. The argument is that this repo ships no oclif.manifest.json, so every invocation re-globs dist/commands live, and ~21 e2e files spawn the real built CLI under concurrent load. That affects every one of those files, not just this pin. Absorbing it here fixes one symptom and leaves the other twenty invisible — and quietly establishes retry-on-this-signature as a precedent nobody decided on.

Suggested disposition for the next seat: drop the retry from this PR and file the spawn-contention behaviour as its own unassigned card (with the oclif.manifest.json / live-reglob mechanism and the measured 56.1%-of-wall-time spawn cost the dev gathered — that is good evidence and should not be lost). Re-run CI on the alias fix alone. It then passes or fails honestly, and if it fails again the flake has a second observation instead of a suppression.

⛔ I am not ruling on whether the retry is technically sound — it may well be. I am ruling that this seat does not land a retry-wrapped test, and that the choice belongs to the maintainer.

State

  • Pushed head adb1b9ea8c; PR stays draft, auto-merge not armed.
  • Flip table needed no correction — neither failure named a NODE_ENV predicate, which is consistent with both being tooling rather than a gate flip. So the ruling's fork-back condition is not triggered by either.
  • Not re-run on this exact head: check:type-check-debt --re-measure and the full Lint & Repo Gates set (both green at 61810a5a0e, before these two fixes). Whoever picks this up should re-run rather than inherit those.

The underlying fix — os serve defaulting NODE_ENV to production, per the maintainer's ruling — is unaffected by any of the above and remains verified: reverse-verified 401 → 403 naming the trusted-origin gate, corroborated by curl against both builds.


Generated by Claude Code

The CI red on #11268 was one test file failing with oclif's own
"Error: command serve not found" before serve.ts ran a line. Re-derived
rather than inherited, and the mechanism is this card's own subject matter
turned back on the harness.
turbo.json declared "@objectstack/cli#test": { dependsOn: ["^build"] } —
dependencies only, never the package's own build. Measured with
`turbo run test --filter=@objectstack/cli --dry=json`: 58 tasks, 57 of them
#build, and @objectstack/cli#build absent from the test task's resolved
dependencies. So packages/cli/dist does not exist when the Test Core shard
runs, and it never has.
Five other files in this package name bin/run.js. Two only assert the path
as a string. The three that spawn it pass NODE_ENV: 'development' to the
child for its --dev admin seed — and that is also the value that makes
@oclif/core's tsPath() rewrite the command target from the declared
./dist/commands to ./src/commands and auto-transpile
(lib/util/util.js: isProd = () =>
!['development','test'].includes(process.env.NODE_ENV ?? '')). Those three
have never touched dist/, so the undeclared build dependency stayed
invisible.
This pin cannot dodge it: unset NODE_ENV is the input under test, and unset
is exactly the value that leaves isProd() true and the reroute off. So it is
the only file in packages/cli that genuinely consumes dist/, and the first
to depend on a prerequisite the graph did not declare.
Fixed at the seam, not the symptom: dependsOn ["build"] (turbo's own build
task already dependsOn ^build, so this is a superset). Same shape as
"@objectstack/metadata#test" 38 lines above. Marginal cost measured at 9s on
a tree whose dependency closure is already built — the shard's exact state —
and ~0 on a turbo cache hit.
Also removes bootServeWithRetry / looksLikeMissingCliCommand, added one
commit earlier on the theory that the failure was a transient dist/commands
read under concurrent spawn load. Removed because the theory is falsified by
measurement, not because a rule forbids retry-wrapping: that retry was
already live in the failing job 97166275854 and did not change the outcome.
The failure is deterministic — with dist absent, unset and production both
answer "command serve not found", test and development both resolve from
src/. The file returns to its reviewed pre-retry shape.
Verified in a dedicated worktree:
- turbo graph after the change: 59 tasks, @objectstack/cli#test depends on
@objectstack/cli#build.
- End-to-end through the real mechanism: `rm -rf packages/cli/dist` then
`turbo run test --filter=@objectstack/cli -- test/serve-node-env-production-default.e2e.test.ts`
-> 57 tasks, 0 cached, `Test Files 1 passed (1)` / `Tests 3 passed (3)`.
- Reverse-verified both legs on this exact tree, each rebuilt, the mutation
confirmed on disk by anchored grep counts and in dist/ by
ablation-dist-preflight.mjs: fix ablated -> `AssertionError: expected 401
to be 403`, `Tests 1 failed | 2 passed (3)`; restored -> `Tests 3 passed (3)`.
- pnpm --filter @objectstack/cli typecheck / eslint — clean.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019siH5jDmk5hrayvfyojUqR
os-zhuang pushed a commit that referenced this pull request Aug 23, 2026
…ker's TEST
Any `packages/cli` e2e test that spawns a real `os serve` and builds the
child env with `{ ...process.env, … }` carries the vitest WORKER's
environment into the child. Vitest sets `TEST=true` on that worker
unconditionally, independent of `NODE_ENV`, and better-auth 1.7.1 reads
`TEST` directly:
@better-auth/core/dist/env/env-impl.mjs:36
const isTest = () => nodeENV === "test" || toBoolean(env.TEST);
better-auth/dist/context/create-context.mjs:210
skipOriginCheck: … isTest() ? true : false,
so the child had better-auth's own origin/CSRF validation switched OFF,
one layer below anything `serve.ts` or `plugin-auth` decide.
`helpers/serve-process.ts` now exports `childEnv()`, which drops `TEST`
and the whole `VITEST*` namespace before the caller's overrides go on,
and `runServe()` builds through it. The four `os serve` spawners in this
directory that roll their own child env are swept onto it too, including
#11268's pin, which had to unset `TEST` by hand.
Measured, same fixture and same probe, the env family the only
difference — POST /api/v1/auth/sign-in/email with
`Origin: https://evil.example.com`:
{ ...process.env, … } 401 INVALID_EMAIL_OR_PASSWORD (origin ACCEPTED)
childEnv({ … }) 403 INVALID_ORIGIN
only TEST stripped 403 INVALID_ORIGIN
`TEST` alone is load-bearing; the `VITEST*` entries are hygiene. Both
rows are pinned as real boots in `serve-process-child-env.e2e.test.ts`,
so the repair stays distinguishable from a no-op.
Part of #11267
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019siH5jDmk5hrayvfyojUqR
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentationImprovements or additions to documentationsize/mteststooling

Projects

None yet

3 participants

@os-elon@os-zhuang@claude