fix(spec,metadata-protocol,runtime): one place decides the unset-NODE_ENV discovery environment - #6554
Merged
Conversation
…_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
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
qq9340100
enabled auto-merge
August 8, 2026 04:30
Contributor
📓 Docs Drift CheckThis PR changes 3 package(s): 118 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:
|
github-actions
Bot
added
documentation
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 free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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 #5936
裁决方向 1(2026-08-07 决策箱第二轮)落地。前提对 origin/main
3a1d9c7逐条复核成立:discovery.zod.ts:341对非字符串输入返回development、protocol.ts:2900(getDiscovery)原样递传process.env.NODE_ENV、http-dispatcher.ts:1331带着本地默认getEnv('NODE_ENV', 'production')和「本条残留待 #5936」的注释、:314的过时映射行仍在。缺陷可复现:同一台忘设NODE_ENV的部署,dispatcher 的/discovery宣称production,经@objectstack/rest服务的同一端点宣称development—— 同一个问题,两个答案。改动
缺省判定收进共享映射:
resolveDiscoveryEnvironment对 absent 值解析为production,两个生产者都原样递传操作员的值。dispatcher 删本地默认;metadata-protocol 本无默认、随映射自动纠正。#4828 的规则不动、方向依旧相反 —— set 而不识别的拼写(qa、preview)降级development(那是猜测),absent 是宿主拒答(默认production)。原「unset / anything else」一行拆成两行各述其理。裁决未明说、按其理据自判的一点(否决窗口):空串算 unset。
NODE_ENV=导出空串;runtime 的getEnv一向用||折进默认,#5673 的 pin 已断言该情形为production;若映射把空串归「anything else」,两个生产者会在这一个输入上继续分歧 —— 正是本次收敛要消灭的漂移。已写进 TSDoc、changeset,两侧生产者各 pin 一条。验证与逆向验证(方向先判后跑)
turbo typecheck119/119;check:generated10/10(本次无一生成物移动);lint 与门禁族(check:route-envelope/check:error-code-casing/check:spec-parsed-alias/check:adr-0087-registration等)全绿。patch,无 declared-breaking,无 ADR-0087 标记欠账。界外发现(已记录、未夹带)
packages/rest自有 conformance 套件但无 NODE_ENV 块 —— 它组合在getDiscovery()之上,新 pin 已覆盖其行为;若将来长出第三个形态值得留意。getEnv(core/src/utils/env.ts:15)用||把空串折进默认是仓级策略,合法为空串的OS_*变量无法表达;本改动在一个调用点依赖该行为,未触碰。交付通道注记
云端工头 B 会话(
session_011btrhv6sHn6JkN93YRtGQp)无 GitHub 工具,走交付降级通道:dev 实现并 push 分支,PM(session_011M7UwH25Unfi73UHim7ajY)代开本 PR 并跟进 CI 至合并。完整交付摘要见分支 commit message。Generated by Claude Code