Skip to content

Commit 5ced825

Browse files
committed
fix(spec,metadata-protocol,runtime): one place decides the unset-NODE_ENV discovery environment (#5936)
Fixes #5936. Direction 1 of the maintainer's 2026-08-07 ruling (decision-inbox round 2), which returned the issue to the queue as a placement question after #5673 had already settled the semantics on 2026-08-06. ## Premise, re-verified against origin/main (3a1d9c7) Every line the issue cites is where it says, and the defect reproduces: - `resolveDiscoveryEnvironment` at `packages/spec/src/api/discovery.zod.ts:341` returned `development` for a non-string input — so `undefined` in. - `packages/metadata-protocol/src/protocol.ts:2900` (`getDiscovery`) passed `process.env.NODE_ENV` in raw, with no default of its own. Served by `packages/rest`. - `packages/runtime/src/http-dispatcher.ts:1331` carried the #5673 default at its own call site as `getEnv('NODE_ENV', 'production')`, with a comment stating this exact residue and naming #5936 as the follow-up. - The mapping table's `unset / anything else -> development` row at `discovery.zod.ts:314` was still there, and had been false for the runtime caller since #5673. Net effect on main: a production deployment whose operator forgot `NODE_ENV` was advertised `production` on the dispatcher's `/discovery` and `development` on the one served through `@objectstack/rest`. Same question, two answers. ## What lands The default moves INTO the shared mapper: an absent value resolves to `production`, and both producers pass the operator's value through as read. The dispatcher's local default is deleted; metadata-protocol never had one to delete and simply inherits the corrected mapper. #4828's rule is untouched and still points the other way — a set-but-unrecognised spelling (`qa`, `preview`) degrades to `development`, because absence is the host declining to answer while an unrecognised spelling is a guess. One judgement call the ruling did not spell out, decided and recorded rather than left implicit: **"unset" includes a blank value**. `NODE_ENV=` exports an empty string; the runtime's `getEnv` has always folded that into its default (it tests with `||`), and #5673's own pin already asserts the dispatcher answers `production` for it. Had the mapper treated blank as "anything else", the two producers would have gone on disagreeing about exactly that input — the drift this consolidation exists to end — so blank is unset. Stated in the TSDoc, in the changeset, and pinned on both producers. Direction 2 (patch metadata-protocol locally) was the alternative and is what the ruling rejected: it would have left a second copy of the default, which is what the shared table was created to prevent (#4828). ## Documentation, both faces of it - `discovery.zod.ts`: the `unset / anything else` row splits into two rows, one per rule, each with its reason, plus the note that the unset default moved here in #5936 and why blank counts as unset. - `http-protocol.mdx` already carried the corrected table from #5673 — but it describes the dispatcher, and until now the table was true of that producer only. It now states that the mapping and the default are shared by every `/discovery` producer, and names the behaviour change for anyone reading `environment` from a REST-served response. - `http-dispatcher.ts`: the comment block that explained *why the default sits at this call site* is replaced by one explaining why it no longer does, and telling the next author not to re-add one. - `protocol.ts`: a short comment at the call site making the same point from the other side — this producer adds no default; the mapper decides. ## Reverse verification — two revert shapes, opposite outcomes, both predicted first Predicted, then measured: - **Full revert to main's shape** (mapper back to `development` for a non-string AND the dispatcher back to `getEnv('NODE_ENV', 'production')`): metadata-protocol's two unset rows go RED, runtime stays **23/23 GREEN**. That asymmetry IS the bug this issue reports, reproduced on demand. Measured: `Tests 2 failed | 17 passed (19)` in metadata-protocol, `Tests 23 passed (23)` in runtime, `Tests 1 failed | 77 passed (78)` in spec's mapper suite. - **Mapper-only revert** (dispatcher keeps its local default deleted): BOTH producers go red — measured `2 failed | 17 passed (19)` and `2 failed | 21 passed (23)`. That is the consolidation stated as a test outcome: the dispatcher no longer has a second place to be right. The unrecognised-spelling rows stayed green under both reverts, as predicted — they are #4828's rule and this change does not touch it. Both source files were restored and verified byte-identical with `diff` afterwards. ## Tests - `packages/spec/src/api/discovery.test.ts`: the single case "never CLAIMS production for an unset or unrecognized value" split into two, because the two rules now point opposite ways. The unset case covers `undefined`, `null`, `''` and `' '`. - `packages/metadata-protocol/src/discovery-schema-conformance.test.ts`: a new block driving the REAL producer — unset and blank advertise `production`, unrecognised spellings still `development`, and the shared table (`test`/`staging`/`production`) reproduced end-to-end so the two producers' suites assert the same facts. - `packages/runtime/src/discovery-schema-conformance.test.ts`: pins unchanged and still green (this producer's behaviour does not move); its comment is rewritten, since the sentence explaining that a spec-side test "cannot see" the default stopped being true the moment the default moved there. Two end-to-end pins rather than one mapper test on purpose: a local default re-added at either call site would satisfy the mapper's own test and still be the drift #5936 removes. Only a producer-level assertion separates them. ## Verification (all foreground, real readings) - `pnpm --filter @objectstack/spec test` -> 340 files, **8727 passed** - `pnpm --filter @objectstack/metadata-protocol test` -> 55 files, **571 passed** - `pnpm --filter @objectstack/runtime test` -> 110 files, **1606 passed** - `pnpm --filter @objectstack/rest test` -> 64 files, **884 passed** - `pnpm --filter @objectstack/objectql test` -> 146 files, **2448 passed** - `pnpm --filter @objectstack/client test` -> 21 files, **263 passed** - `pnpm --filter @objectstack/hono test` -> 2 files, **73 passed** - `turbo run typecheck --filter=./packages/* --filter=./packages/*/*` -> **119 successful, 119 total** - `pnpm --filter @objectstack/spec check:generated` -> **10/10 up to date** (nothing regenerated: no `.describe()`, no export and no authorable key moved) - `pnpm --filter @objectstack/spec gen:openapi` -> ran (ungated generator); writes only the gitignored `json-schema/openapi.json`, no tracked diff - `pnpm lint` -> clean - `check:nul-bytes` / `check:empty-changeset` / `check:adr-0087-registration` / `check:route-envelope` / `check:error-code-casing` / `check:wildcard-fallthrough` / `check:adr-anchors` / `check:doc-authoring` / `check:docs-audit-scope` / `check:quick-reference-counts` / `check:startup-registry-verdict` / `check:service-providers` / `check:spec-parsed-alias` -> all green `check:adr-0087-registration` confirms no declared-breaking changeset, so no disposition marker is owed. The changeset covers all three packages at `patch` and states the behaviour change an operator can observe. One first-run red that was NOT this change: `@objectstack/client` reported 3 failed suites with "Failed to resolve entry for package `@objectstack/runtime`" — a fresh worktree with runtime's `dist` unbuilt (AGENTS.md §9's stale-artefact trap in mirror image). `pnpm --filter '@objectstack/client...' build` fixed it and the suite went 21/21. ## Ruling mapping | Ruling clause | Where it lands | |:---|:---| | unset -> `production`, folded into `resolveDiscoveryEnvironment` | `discovery.zod.ts` — the mapper body | | producers drop their local defaults | `http-dispatcher.ts` (deleted); `protocol.ts` had none and now says so | | correct the stale mapping-table comment | `discovery.zod.ts` — one row becomes two, each with its rule | | pin both producers under unset `NODE_ENV` | new block in metadata-protocol's conformance suite; runtime's #5673 pins kept and re-commented | | one PR across spec + metadata-protocol + runtime | this commit | | changeset covering the three packages, patch | `.changeset/discovery-environment-single-default.md` | ## Out-of-scope findings (recorded, not fixed) 1. `packages/rest` carries its own `discovery-schema-conformance.test.ts` and is the SERVER for the metadata-protocol producer, but it has no NODE_ENV block of its own — it composes over `getDiscovery()`, so the new pins do cover the behaviour it serves. Worth knowing if a third shape ever grows there. 2. `getEnv` (`packages/core/src/utils/env.ts:15`) collapses an empty string into its default via `||`, which is right for `NODE_ENV` but is a repo-wide policy applied to every variable — an `OS_*` value that is legitimately the empty string cannot be expressed. Not touched here; noted because this change depends on that behaviour at one call site. ## 交付摘要(PM 开 PR 素材) - **前提**:对 origin/main 3a1d9c7 逐条复核成立 —— `discovery.zod.ts:341` 非字符串返回 `development`、`protocol.ts:2900` 原样递传、`http-dispatcher.ts:1331` 本地默认连同「本条残留待 #5936」的注释俱在、:314 那行过时表格行仍在。缺陷可复现:同一台忘设 NODE_ENV 的部署,两个生产者给出两个答案。 - **落地**(裁决方向 1):缺省判定收进共享映射(unset → `production`),runtime 删本地默认,metadata-protocol 本无默认、随映射自动纠正;#4828 的「不在猜测上宣称 production」不动,两条规则拆成两行各述其理。 - **裁决未明说而必须定的一点**:空串算 unset。`NODE_ENV=` 导出空串,runtime 的 `getEnv` 一向用 `||` 把它折进默认,#5673 的 pin 也已断言该情形为 production;若映射把空串归入「anything else」,两个生产者就会在这一个输入上继续分歧 —— 正是本次收敛要消灭的漂移。已写进 TSDoc、changeset 并在两侧各 pin 一条。 - **反向验证**:两种回退形态,方向先判后跑,结果相反且都命中 —— 整体回退到 main 形态:metadata-protocol 红 2、runtime 23/23 全绿(**这就是 #5936 报告的不对称本身**);仅回退映射:两侧各红 2(说明 runtime 已无第二处可依赖)。未识别拼写行在两种回退下均保持绿。两个源文件事后 diff 验证逐字节还原。 - **文档面**:`discovery.zod.ts` 表格行订正;`http-protocol.mdx` 原表自 #5673 起就是对的,但只对 dispatcher 成立 —— 现补明该表对**所有** `/discovery` 生产者成立,并写清经 `@objectstack/rest` 读 `environment` 的调用方会看到的行为变化。 - **验证读数**:spec 8727、metadata-protocol 571、runtime 1606、rest 884、objectql 2448、client 263、hono 73 全绿;`turbo typecheck` 119/119;生成物 10/10 且本次无一需要重生成;lint 与门禁族全绿。client 首跑 3 个 suite 红是新 worktree 未建 runtime dist(AGENTS §9 陈旧产物陷阱的镜像),补建后 21/21,与本改动无关。 - **界外发现**两条见上,均未夹带。 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011btrhv6sHn6JkN93YRtGQp
1 parent 3a1d9c7 commit 5ced825

8 files changed

Lines changed: 250 additions & 53 deletions

File tree

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
---
2+
"@objectstack/spec": patch
3+
"@objectstack/metadata-protocol": patch
4+
"@objectstack/runtime": patch
5+
---
6+
7+
fix(spec,metadata-protocol,runtime): one place decides what an unset `NODE_ENV` advertises (#5936)
8+
9+
A deployment whose operator never exported `NODE_ENV` must not describe itself as
10+
`development` on `/discovery`: `environment` is a machine-readable field, a client
11+
reads it to answer "am I talking to production?", and it may skip production warnings
12+
or loosen a destructive action's confirmation on the answer. #5673 ruled that in and
13+
fixed it — but only for one of the two producers, because that dispatch put
14+
`packages/spec` out of scope. The other one, `MetadataProtocol.getDiscovery()` (served
15+
by `@objectstack/rest`), went on answering `development` for exactly that input.
16+
17+
The default now lives in the shared mapper, `resolveDiscoveryEnvironment`: an absent —
18+
or blank — value resolves to `production`, and both producers pass the operator's value
19+
through as they read it, neither carrying a default of its own. That is what makes it
20+
one decision instead of two copies, and it means the next discovery producer inherits
21+
the right answer without anyone remembering to copy a line. Patching only
22+
metadata-protocol would have left a second copy of the default — precisely the drift the
23+
shared table was created to prevent (#4828).
24+
25+
"Unset" includes a blank value: `NODE_ENV=` exports an empty string, the runtime's
26+
`getEnv` has always folded that into its default, and had the mapper treated blank as
27+
"anything else" the two producers would have drifted again on that one input.
28+
29+
**#4828's rule is untouched, and it points the other way on purpose.** A value that IS
30+
set but is not a spelling this repo recognises (`qa`, `preview`) still degrades to
31+
`development`, so nothing ever claims `production` on a guess. Absence is not a guess —
32+
it is the host declining to say.
33+
34+
Behaviour change to expect: a host that exports no `NODE_ENV` and serves `/discovery`
35+
through `@objectstack/rest` now advertises `environment: "production"` where it
36+
previously advertised `"development"`. A deployment that genuinely is development should
37+
say so — `NODE_ENV=development` — which is what the runtime dispatcher has already
38+
required since #5673.
39+
40+
The mapping table above `NODE_ENV_TO_DISCOVERY_ENVIRONMENT` is corrected in the same
41+
pass: its `unset / anything else -> development` row had been false for the runtime
42+
caller since #5673 and is now two rows, one per rule.

content/docs/protocol/kernel/http-protocol.mdx

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -187,6 +187,15 @@ field is machine-readable — a client uses it to decide whether it is talking t
187187
`development`. **An unrecognised spelling** (`qa`, `preview`, `uat`) is a different case: it
188188
is a guess, and this field never claims production on a guess.
189189

190+
This table is the whole answer for **every** producer of `/discovery` (#5936). The mapping
191+
and the unset default both live in one shared function, so the dispatcher and the
192+
`@objectstack/metadata-protocol` builder served by `@objectstack/rest` cannot disagree, and
193+
a future producer inherits the same answers without copying anything. Until #5936 the unset
194+
default lived at the dispatcher's own call site, so a deployment with no `NODE_ENV` was
195+
advertised as `production` there and `development` through `@objectstack/rest`. If you read
196+
`environment` from a REST-served `/discovery` and relied on the old answer, set
197+
`NODE_ENV=development` explicitly.
198+
190199
Local development is unaffected: `os dev` runs `serve --dev`, which sets
191200
`NODE_ENV=development` in-process before the runtime loads. Anything that boots the runtime
192201
*without* `os dev` — a bare `os serve`, an embedded host, a hand-written container entry

packages/metadata-protocol/src/discovery-schema-conformance.test.ts

Lines changed: 82 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@
2929
// from the schema instead of a hand-listed array is what stops this gate
3030
// from becoming a third dialect of the contract.
3131

32-
import { describe, it, expect } from 'vitest';
32+
import { describe, it, expect, afterEach } from 'vitest';
3333
import {
3434
ApiRoutesSchema,
3535
DiscoverySchema,
@@ -197,6 +197,87 @@ describe('[#4828] getDiscovery() conforms to DiscoverySchema', () => {
197197
expect(['production', 'sandbox', 'development']).toContain(discovery.environment);
198198
});
199199

200+
// ═══════════════════════════════════════════════════════════════════════════
201+
// [#5936] The unset-NODE_ENV default, asserted on THIS producer
202+
// ═══════════════════════════════════════════════════════════════════════════
203+
//
204+
// `/discovery` has two producers. #5673 ruled that a deployment whose operator
205+
// never set `NODE_ENV` must not call itself `development` — `environment` is
206+
// machine-readable and a client may loosen a destructive action's
207+
// confirmation on it — but that dispatch was scoped to the runtime dispatcher
208+
// and forbade touching `packages/spec`, so the default landed at that
209+
// producer's own call site and THIS producer went on answering `development`
210+
// for the same input. The 2026-08-07 ruling (direction 1) folded the default
211+
// into `resolveDiscoveryEnvironment`, which is what makes one decision reach
212+
// both.
213+
//
214+
// This case is the half a mapper test cannot cover: that this producer passes
215+
// the operator's value through AS READ and adds no default of its own. Its
216+
// sibling lives in `packages/runtime/src/discovery-schema-conformance.test.ts`
217+
// and asserts the identical fact about the dispatcher — the pair is what makes
218+
// "the two producers agree" a checked fact rather than a comment.
219+
//
220+
// Reverse verification, direction predicted BEFORE running — and the
221+
// interesting part is that the two revert shapes go DIFFERENT ways:
222+
//
223+
// * Revert the whole change (the mapper back to `development` for a
224+
// non-string AND the dispatcher back to `getEnv('NODE_ENV', 'production')`)
225+
// and the two unset rows HERE go RED reading `development` while the
226+
// runtime's sibling rows stay GREEN. That asymmetry IS the bug #5936
227+
// reports, reproduced on demand.
228+
// * Revert only the mapper and BOTH producers go red, because the
229+
// dispatcher no longer carries a default of its own to fall back on —
230+
// which is the point of the consolidation, stated as a test outcome.
231+
//
232+
// The unrecognised-spelling rows stay green under every revert; they are
233+
// #4828's rule, which this change deliberately leaves alone.
234+
describe('[#5936] NODE_ENV defaults are the shared mapper\'s decision, not this producer\'s', () => {
235+
const OLD_NODE_ENV = process.env.NODE_ENV;
236+
afterEach(() => {
237+
if (OLD_NODE_ENV === undefined) delete process.env.NODE_ENV;
238+
else process.env.NODE_ENV = OLD_NODE_ENV;
239+
});
240+
241+
it.each([
242+
['unset', undefined],
243+
// `NODE_ENV=` exports an empty string; the mapper reads a blank value as
244+
// "the host did not say", the same absence `os serve` / `os doctor` and
245+
// the runtime producer already read as production.
246+
['empty', ''],
247+
])('NODE_ENV %s advertises production — never development', async (_label, raw) => {
248+
if (raw === undefined) delete process.env.NODE_ENV;
249+
else process.env.NODE_ENV = raw;
250+
251+
const discovery: any = await makeImpl().getDiscovery();
252+
253+
expect(discovery.environment).toBe('production');
254+
expect(DiscoverySchema.safeParse(discovery).success).toBe(true);
255+
});
256+
257+
it.each(['qa', 'preview', 'nonsense'])(
258+
'NODE_ENV=%s is an unrecognised spelling — still development, never production (#4828)',
259+
async (raw) => {
260+
process.env.NODE_ENV = raw;
261+
262+
const discovery: any = await makeImpl().getDiscovery();
263+
264+
expect(discovery.environment).toBe('development');
265+
},
266+
);
267+
268+
it.each([
269+
['test', 'development'],
270+
['staging', 'sandbox'],
271+
['production', 'production'],
272+
])('NODE_ENV=%s advertises %s — the same table the dispatcher reads', async (raw, expected) => {
273+
process.env.NODE_ENV = raw;
274+
275+
const discovery: any = await makeImpl().getDiscovery();
276+
277+
expect(discovery.environment).toBe(expected);
278+
});
279+
});
280+
200281
it('reports a `locale` block derived from the i18n service when one is registered', async () => {
201282
const services = new Map<string, any>([
202283
['i18n', {

packages/metadata-protocol/src/protocol.ts

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2897,6 +2897,15 @@ export class ObjectStackProtocolImplementation implements
28972897
name,
28982898
/** @deprecated Use `name`. Removed in protocol 18 (#4828). */
28992899
apiName: name,
2900+
// [#5936] The operator's value, passed as read — no local default.
2901+
// What an ABSENT `NODE_ENV` advertises is decided once, inside
2902+
// `resolveDiscoveryEnvironment` (`production`, per the 2026-08-07
2903+
// ruling, direction 1), so this producer and the runtime dispatcher
2904+
// cannot drift on it. Before that ruling the default lived at the
2905+
// dispatcher's own call site and this producer had no equivalent, so
2906+
// a deployment that forgot the variable was told `development` here
2907+
// and `production` there — the exact drift the shared mapper exists
2908+
// to prevent (#4828). Do not re-introduce a default here.
29002909
environment: resolveDiscoveryEnvironment(
29012910
(globalThis as { process?: { env?: Record<string, string | undefined> } })
29022911
.process?.env?.NODE_ENV,

packages/runtime/src/discovery-schema-conformance.test.ts

Lines changed: 28 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -275,23 +275,37 @@ describe('[#4828] getDiscoveryInfo() conforms to DiscoverySchema', () => {
275275
expect(DiscoverySchema.safeParse(info).success).toBe(true);
276276
});
277277

278-
// [#5673] The pin for this issue, and the reason it is driven through the
279-
// REAL producer rather than through `resolveDiscoveryEnvironment` alone:
280-
// the UNSET default is decided at THIS call site (`getEnv`'s second
281-
// argument), so a green mapper test in `packages/spec` cannot see it. The
282-
// whole setup is deleting the variable — that is precisely the state of a
283-
// production deployment whose operator never set it.
278+
// [#5673] The pin for that issue: the whole setup is deleting the variable
279+
// — precisely the state of a production deployment whose operator never set
280+
// it — driven through the REAL producer rather than through
281+
// `resolveDiscoveryEnvironment` alone.
284282
//
285-
// Reverse verification, direction predicted BEFORE running: restore the old
286-
// `getEnv('NODE_ENV', 'development')` and these two cases go RED (they read
287-
// `development`), while every `it.each` row above stays green — the old
288-
// default was only ever consulted when NODE_ENV was absent, so nothing that
289-
// sets it can detect the change. Measured both ways.
283+
// [#5936] It stays here, and driven end-to-end, for a reason that survived
284+
// the default moving. When #5673 landed, the default WAS this call site
285+
// (`getEnv`'s second argument) and a green mapper test in `packages/spec`
286+
// could not have seen it. The 2026-08-07 ruling folded the default into the
287+
// mapper, so the spec-side case now covers the decision itself — and this
288+
// one covers the wiring: that this producer passes the operator's value
289+
// through as read and adds no default of its own. A local default here
290+
// would satisfy the mapper's test and still be the drift #5936 removed;
291+
// only an end-to-end assertion can tell the two apart. Its sibling in
292+
// `@objectstack/metadata-protocol` asserts the same fact about the other
293+
// producer, which is the pair that makes "one decision" checkable.
294+
//
295+
// Reverse verification, direction predicted BEFORE running: restore the
296+
// mapper's pre-#5936 `development` default and these two cases go RED
297+
// reading `development`, while every `it.each` row above stays green,
298+
// because the default is only ever consulted when NODE_ENV is absent. Note
299+
// this producer no longer has a second place to be fixed: the call site
300+
// carries no default, so the mapper's answer IS this producer's answer.
301+
// Measured both ways.
290302
it.each([
291303
['unset', undefined],
292-
// `getEnv` collapses `''` to its default (`process.env[key] || default`),
293-
// so `NODE_ENV=` is the same absence as never exporting it — and the same
294-
// absence `doctorNodeEnv()` and `os serve` already read as production.
304+
// `NODE_ENV=` exports an empty string. `getEnv` collapses it to its
305+
// default (`process.env[key] || default`) and, since #5936, the mapper
306+
// reads a blank string as unset too — so this is the same absence
307+
// `doctorNodeEnv()` and `os serve` already read as production, and it
308+
// answers the same on both producers.
295309
['empty', ''],
296310
])('NODE_ENV %s advertises production — never development (#5673)', async (_label, raw) => {
297311
if (raw === undefined) delete process.env.NODE_ENV;

packages/runtime/src/http-dispatcher.ts

Lines changed: 25 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -1293,42 +1293,39 @@ export class HttpDispatcher {
12931293
// enum on a machine-readable surface. The mapping table and the
12941294
// reasoning per row live with the enum, in `@objectstack/spec/api`.
12951295
//
1296-
// [#5673] The DEFAULT — what this producer says when the host set no
1297-
// `NODE_ENV` at all — flipped from `development` to `production` per
1298-
// the maintainer's 2026-08-06 ruling. Two facts made the old default
1299-
// the wrong one:
1296+
// [#5673] The DEFAULT — what a producer says when the host set no
1297+
// `NODE_ENV` at all — is `production`, per the maintainer's
1298+
// 2026-08-06 ruling, because `environment` is a MACHINE-READABLE
1299+
// field: a client reads it to answer "am I talking to production?"
1300+
// and may skip production warnings or loosen a destructive action's
1301+
// confirmation on the answer. Of the two ways to be wrong, claiming
1302+
// `development` on a real production deployment whose operator
1303+
// forgot the variable is the dangerous one. (Every other reader of
1304+
// that absence already said `production`: `os start` forces
1305+
// `NODE_ENV='production'` when unset, `os serve` resolves its
1306+
// `.env*` cascade for `NODE_ENV || 'production'`, `os doctor`
1307+
// derives the same expression. Discovery was the one surface
1308+
// reading it the other way.)
13001309
//
1301-
// • Every other reader of the same absence already said
1302-
// `production`. `os start` forces `NODE_ENV='production'` when
1303-
// unset (`packages/cli/src/commands/start.ts:248`), `os serve`
1304-
// resolves its `.env*` cascade for `NODE_ENV || 'production'`
1305-
// (`serve.ts:532-533`), and `os doctor` derives the identical
1306-
// expression (`doctor.ts` `doctorNodeEnv()`). Discovery was the
1307-
// one surface reading that absence the other way.
1308-
// • `environment` is a MACHINE-READABLE field: a client reads it to
1309-
// answer "am I talking to production?" and may skip production
1310-
// warnings or loosen a destructive action's confirmation on the
1311-
// answer. Of the two ways to be wrong here, claiming
1312-
// `development` on a real production deployment whose operator
1313-
// forgot the variable is the dangerous one.
1310+
// [#5936] That default no longer lives HERE. #5673's ruling put
1311+
// `packages/spec` out of scope, so this producer carried the default
1312+
// at its own call site — and the second producer (`getDiscovery()`
1313+
// in `@objectstack/metadata-protocol`, served by
1314+
// `@objectstack/rest`) went on answering `development` for the unset
1315+
// case, which is the drift the shared mapper was built to prevent
1316+
// (#4828). The 2026-08-07 ruling (direction 1) folded the default
1317+
// into `resolveDiscoveryEnvironment`, so both producers now inherit
1318+
// one decision and the next producer gets it without remembering to
1319+
// copy a line. Pass the operator's value as read; do NOT re-add a
1320+
// local default here or anywhere else.
13141321
//
13151322
// #4828's rule is untouched and is a DIFFERENT rule: a value that IS
13161323
// set but is not a spelling this repo recognises (`qa`, `preview`)
13171324
// still degrades to `development` inside the mapper, so nothing here
13181325
// ever CLAIMS production on a guess. Absence is not a guess — it is
13191326
// the host declining to say, and the conservative answer to that is
13201327
// `production`.
1321-
//
1322-
// The default is passed as `getEnv`'s second argument rather than
1323-
// moved into `resolveDiscoveryEnvironment` because the mapper lives
1324-
// in `@objectstack/spec`, which this issue's ruling put out of scope.
1325-
// Consequence, stated rather than hidden: the second discovery
1326-
// producer (`getDiscovery()` in `@objectstack/metadata-protocol`,
1327-
// served by `@objectstack/rest`) passes a genuinely-absent
1328-
// `NODE_ENV` straight into the mapper and therefore still answers
1329-
// `development` for the unset case. Filed as a follow-up (#5936);
1330-
// do not "fix" it by re-defaulting a consumer somewhere else.
1331-
environment: resolveDiscoveryEnvironment(getEnv('NODE_ENV', 'production')),
1328+
environment: resolveDiscoveryEnvironment(getEnv('NODE_ENV')),
13321329
routes,
13331330
// [#4828] `endpoints` (a verbatim duplicate of `routes`, commented
13341331
// "Alias for backward compatibility with some clients") and the

packages/spec/src/api/discovery.test.ts

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1181,9 +1181,25 @@ describe('[#4828] resolveDiscoveryEnvironment (decision 4 — enum, not passthro
11811181
expect(resolveDiscoveryEnvironment('STAGING')).toBe('sandbox');
11821182
});
11831183

1184-
it('never CLAIMS production for an unset or unrecognized value', () => {
1185-
for (const raw of [undefined, null, '', 'qa', 'preview', 'nonsense']) {
1186-
expect(resolveDiscoveryEnvironment(raw as any), String(raw)).toBe('development');
1184+
// [#5936] These were ONE case until the 2026-08-07 ruling folded the unset
1185+
// default into this mapper (direction 1). They are two different rules and
1186+
// they now point opposite ways, so they are two cases: absence is the host
1187+
// declining to answer and resolves conservatively to `production`; a spelling
1188+
// this repo does not recognise is a GUESS and never claims production.
1189+
// Collapsing them back into one is the regression these two guard.
1190+
it('an UNSET value advertises production — the host declined to say (#5673, #5936)', () => {
1191+
// Blank counts as unset: `NODE_ENV=` exports an empty string, and the
1192+
// runtime's `getEnv` has always folded that into its default. Were it
1193+
// treated as "anything else" the two producers would drift again on exactly
1194+
// that input — the drift this consolidation ends.
1195+
for (const raw of [undefined, null, '', ' ']) {
1196+
expect(resolveDiscoveryEnvironment(raw as any), JSON.stringify(raw)).toBe('production');
1197+
}
1198+
});
1199+
1200+
it('never CLAIMS production for an unrecognized spelling (#4828)', () => {
1201+
for (const raw of ['qa', 'preview', 'nonsense']) {
1202+
expect(resolveDiscoveryEnvironment(raw), raw).toBe('development');
11871203
}
11881204
});
11891205

0 commit comments

Comments
 (0)