fix(runtime,cli): NODE_ENV 未设置时 /discovery 报 production,doctor 新增缺省提示行 (#5673)

.changeset/discovery-node-env-unset-production.md

@@ -0,0 +1,44 @@
+---
+"@objectstack/runtime": minor
+"@objectstack/cli": minor
+---
+
+fix(runtime,cli): 未设置 `NODE_ENV` 时 `/discovery` 不再自称 `development`,统一按 `production` 解读 (#5673)
+
+同一个「宿主没有设置 `NODE_ENV`」的事实,仓里原本有两套相反的默认:`os start` 未设时强制
+`NODE_ENV='production'`(`start.ts:248`),`os serve` 与 `os doctor` 按
+`NODE_ENV || 'production'` 解析 `.env*` 级联,而 `/discovery` 的 `environment` 字段
+直接把缺省读成 `development`。
+
+**为什么这个方向的错报是危险的那个。** `environment` 是**机器可读面**上的字段,客户端
+拿它回答「我在不在生产环境」,并可能据此不显示生产警示、放宽破坏性操作的二次确认。一个
+忘记设 `NODE_ENV` 的真实生产部署,过去会拿到 `development` —— 两种错法里代价更高的那种。
+按 maintainer 2026-08-06 裁定,缺省统一收敛到保守值 `production`。
+
+**迁移说明(行为变更,请对照自己的部署方式读)**
+
+- **生产部署忘设 `NODE_ENV`**:`/discovery` 的 `environment` 由 `development` 变为
+ `production`。这正是本次修复的目标 —— 报的是实情,不需要任何动作。
+- **本地开发**:不受影响。`os dev` 会 spawn `serve --dev`,而 `serve` 在 `--dev` 且
+ `NODE_ENV` 未设时就地设 `NODE_ENV='development'`(`serve.ts:490-491`),所以
+ `pnpm dev` / `pnpm dev:showcase` / `dev:crm` / `dev:todo` 链路上 `NODE_ENV` 早已是
+ 显式的 `development`,`/discovery` 仍报 `development`。没有任何脚本因此改动。
+- **需要 `development` 却不走 `os dev` 的场景**(裸 `os serve`、以库形式内嵌运行时、
+ 自建容器入口):现在必须显式 `NODE_ENV=development`。这是本次唯一需要动手的一类。
+- **已设置的合法拼法一律不变**:`production`/`prod` → `production`,
+ `staging`/`sandbox` → `sandbox`,`development`/`dev`/`test` → `development`。
+- **无法识别的拼法处置不回退**:`qa`、`preview`、`uat` 这类**设了但认不出**的值仍然
+ 降级为 `development`,#4828 的「绝不凭猜测宣称 production」保持原样。缺省不是猜测,
+ 是宿主选择不说 —— 两条是不同的规则,本次只动前者。
+
+**`os doctor` 新增一行提示。** `NODE_ENV` 未设时报
+`NODE_ENV Not set — this environment is being treated as production`(warning,不影响
+退出码),`--verbose` 展开显式设置的两条命令。已设置的环境完全没有这一行,报告与从前
+逐字一致。统一默认让缺省变得**安全**,但也让「疏忽」和「有意的生产部署」变得无法区分;
+这一行是唯一能把两者分开的地方。
+
+**已知残留(已另开 #5936 跟进,本次不动)。** `/discovery` 有两个生产者。本次改的是
+`@objectstack/runtime` 的 `HttpDispatcher.getDiscoveryInfo()`;经 `@objectstack/rest`
+暴露的 `MetadataProtocol.getDiscovery()`(`packages/metadata-protocol`)把真实缺省原样
+递给共享映射函数,该函数对缺省仍返回 `development`。裁定把落点限定在 runtime 侧、把
+`packages/metadata-protocol` 标为跨域文件面,所以此处如实记录而非顺手绕过。

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

@@ -175,7 +175,30 @@ than advertised verbatim (#4828):
 | `staging` | `sandbox` — pre-production and production-like |
 | `development`, `dev` | `development` |
 | `test` | `development` — an ephemeral developer-class run |
-| unset / anything else | `development` — never claims production on a guess |
+| **unset** (absent, or `NODE_ENV=`) | `production` — the conservative reading of "the host did not say" (#5673) |
+| anything else | `development` — never claims production on a guess |
+
+The last two rows answer two different questions and were one row until #5673. **Unset is
+not a spelling**, it is the absence of one, and the rest of the platform already read that
+absence as production: `os start` forces `NODE_ENV=production` when it is unset, and both
+`os serve` and `os doctor` resolve the `.env*` cascade for `NODE_ENV || production`. This
+field is machine-readable — a client uses it to decide whether it is talking to production
+— so a real production deployment whose operator forgot the variable must not be told
+`development`. **An unrecognised spelling** (`qa`, `preview`, `uat`) is a different case: it
+is a guess, and this field never claims production on a guess.
+
+Local development is unaffected: `os dev` runs `serve --dev`, which sets
+`NODE_ENV=development` in-process before the runtime loads. Anything that boots the runtime
+*without* `os dev` — a bare `os serve`, an embedded host, a hand-written container entry
+point — must now set `NODE_ENV=development` explicitly to keep being advertised as such.
+
+<Callout type="warn">
+**The `@objectstack/rest`-served `/api/v1/discovery` document still reads an unset
+`NODE_ENV` as `development`.** #5673 landed on the dispatcher producer only; the second
+producer is tracked in [#5936](https://github.com/objectstack-ai/objectstack/issues/5936).
+Until it lands, the two documents agree on every *set* value and differ only when nothing
+is set.
+</Callout>
 
 <Callout type="warn">
 **Retired in protocol 17 (#4828):** this document used to carry a top-level `features`

packages/cli/src/commands/doctor-node-env-default.test.ts

@@ -0,0 +1,227 @@
+// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
+
+/**
+ * `os doctor`'s unset-`NODE_ENV` row (#5673).
+ *
+ * ── What #5673 was ───────────────────────────────────────────────────────
+ *
+ * One fact — "the operator never set `NODE_ENV`" — had two opposite readings in
+ * this repo. `os start` forced `production` (`start.ts:248`), `os serve` and
+ * `doctorNodeEnv()` derived `NODE_ENV || 'production'`, and the `/discovery`
+ * `environment` field advertised `development`. The maintainer's 2026-08-06
+ * ruling unified them on `production` — the conservative answer, because a
+ * client reads `environment` to decide whether it is talking to production and
+ * the dangerous direction of error is claiming `development` on a real one.
+ *
+ * Unifying the readings makes the default SAFE. It does not make it VISIBLE:
+ * afterwards an oversight and a deliberate production deployment produce
+ * byte-identical reports. The second half of the ruling — this file's subject —
+ * asked doctor to say the default out loud instead of leaving it documented.
+ *
+ * ── What this file pins, and what it deliberately does not ───────────────
+ *
+ * It pins the ROW: that it exists exactly when the variable is unset, that it
+ * says both halves of the sentence ("treated as production" + "set it
+ * explicitly"), and that it is a `warning` rather than an `error`. The severity
+ * is load-bearing and not cosmetic: doctor's display loop derives `hasErrors`
+ * from this field and `hasErrors` is what calls `process.exit(1)`. An unset
+ * `NODE_ENV` must not fail anyone's health check.
+ *
+ * It does NOT re-pin `doctorNodeEnv()`'s own derivation — that is
+ * `doctor-env-provenance.test.ts`'s subject and was already correct before
+ * #5673 (it is the alignment TARGET, not a thing this issue changed). What is
+ * pinned here instead is the CORRESPONDENCE: the row appears exactly when
+ * `doctorNodeEnv()` fell back to its default, so the row cannot start claiming
+ * a default that was not taken.
+ *
+ * The `/discovery` half of the same ruling lives in
+ * `packages/runtime/src/discovery-schema-conformance.test.ts`.
+ */
+
+import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest';
+import fs from 'node:fs';
+import os from 'node:os';
+import path from 'node:path';
+import { fileURLToPath } from 'node:url';
+
+import Doctor, { nodeEnvCheck, doctorNodeEnv } from './doctor.js';
+
+/** `packages/cli` — the oclif root the real command is loaded against below. */
+const CLI_ROOT = path.resolve(path.dirname(fileURLToPath(import.meta.url)), '..', '..');
+
+/**
+ * `chalk` may or may not emit SGR codes depending on TTY detection.
+ *
+ * The escape is written as `\x1b`, never as the byte itself: one raw control
+ * character makes grep treat the whole file as binary, and a test file no
+ * `git grep` can find stops being maintained (#4890 / #5157).
+ */
+const SGR = /\x1b\[[0-9;]*m/g;
+const plain = (s: string) => s.replace(SGR, '');
+
+describe('[#5673] nodeEnvCheck — the row, and when it exists', () => {
+ it('produces a finding when NODE_ENV is unset', () => {
+ const finding = nodeEnvCheck({} as NodeJS.ProcessEnv);
+ expect(finding).toBeDefined();
+ expect(finding!.name).toBe('NODE_ENV');
+ });
+
+ it('produces NOTHING for every environment that set the variable', () => {
+ for (const value of ['production', 'development', 'test', 'staging', 'sandbox', 'qa']) {
+ expect(nodeEnvCheck({ NODE_ENV: value } as NodeJS.ProcessEnv), value).toBeUndefined();
+ }
+ });
+
+ it("treats `NODE_ENV=` as unset — the same collapse doctorNodeEnv() makes", () => {
+ // `doctorNodeEnv({ NODE_ENV: '' })` is already `production` (pinned in
+ // doctor-env-provenance.test.ts). If this check disagreed, doctor would
+ // resolve the cascade for a default it then refused to mention.
+ expect(nodeEnvCheck({ NODE_ENV: '' } as NodeJS.ProcessEnv)).toBeDefined();
+ });
+
+ it('appears exactly when doctorNodeEnv() fell back to its default', () => {
+ // The correspondence, not a restatement of either function. The row claims
+ // "a default was taken"; this is the assertion that the claim is true for
+ // every input, including the ones where NODE_ENV is set to the default's own
+ // value (`production` — set explicitly, so no default was taken, so no row).
+ const cases: Array<NodeJS.ProcessEnv> = [
+ {},
+ { NODE_ENV: '' },
+ { NODE_ENV: 'production' },
+ { NODE_ENV: 'development' },
+ { NODE_ENV: 'test' },
+ ] as NodeJS.ProcessEnv[];
+
+ for (const env of cases) {
+ const defaulted = !env.NODE_ENV;
+ expect(nodeEnvCheck(env) !== undefined, JSON.stringify(env)).toBe(defaulted);
+ // …and when it was taken, the value taken really is `production`.
+ if (defaulted) expect(doctorNodeEnv(env)).toBe('production');
+ }
+ });
+
+ it('is a WARNING — the severity that leaves doctor exiting 0', () => {
+ // `status` is what doctor's display loop turns into `hasErrors` /
+ // `hasWarnings`, and `hasErrors` is the only path to `process.exit(1)`.
+ // Nothing here is broken: the environment starts, in the mode the row names.
+ expect(nodeEnvCheck({} as NodeJS.ProcessEnv)!.status).toBe('warning');
+ });
+
+ it('says BOTH halves: what is happening now, and what to do about it', () => {
+ const finding = nodeEnvCheck({} as NodeJS.ProcessEnv)!;
+ const text = plain(`${finding.message}\n${finding.fix ?? ''}`);
+
+ // Half one — the fact. Without this the row is a nag with no content.
+ expect(finding.message).toContain('production');
+ expect(text).toMatch(/not set/i);
+
+ // Half two — the way out, spelled as the two commands an operator can type.
+ expect(text).toContain('NODE_ENV=production');
+ expect(text).toContain('NODE_ENV=development');
+
+ // …and the one thing a reader would otherwise get wrong: this variable
+ // cannot be supplied by a `.env*` file, because it selects which of them
+ // load. Doctor's whole environment block is about `.env*` provenance, so
+ // omitting this invites exactly the wrong fix.
+ expect(text).toContain('.env*');
+ });
+});
+
+describe('[#5673] os doctor, end to end — the row reaches the report', () => {
+ /**
+ * `node_modules/` exists in the temp cwd on purpose — without it doctor's
+ * `Dependencies` check is itself an `error` and exits 1 on its own, which
+ * would make these assertions pass (or fail) for a reason having nothing to
+ * do with this change. Same trap PR #5390 wrote down.
+ */
+ let tmp: string;
+ let cwdSpy: ReturnType<typeof vi.spyOn>;
+ let savedNodeEnv: string | undefined;
+
+ beforeEach(() => {
+ savedNodeEnv = process.env.NODE_ENV;
+ tmp = fs.mkdtempSync(path.join(os.tmpdir(), 'os-doctor-5673-'));
+ fs.mkdirSync(path.join(tmp, 'node_modules'));
+ cwdSpy = vi.spyOn(process, 'cwd').mockReturnValue(tmp);
+ });
+
+ afterEach(() => {
+ if (savedNodeEnv === undefined) delete process.env.NODE_ENV;
+ else process.env.NODE_ENV = savedNodeEnv;
+ cwdSpy.mockRestore();
+ fs.rmSync(tmp, { recursive: true, force: true });
+ });
+
+ async function runDoctor(argv: string[] = []): Promise<{ out: string; exitCode: number | undefined }> {
+ const logs: string[] = [];
+ const logSpy = vi.spyOn(console, 'log').mockImplementation((...a: unknown[]) => {
+ logs.push(a.join(' '));
+ });
+ let exitCode: number | undefined;
+ const exitSpy = vi.spyOn(process, 'exit').mockImplementation(((code?: number) => {
+ exitCode = code;
+ throw new Error(`__PROCESS_EXIT__:${code}`);
+ }) as never);
+
+ try {
+ await Doctor.run(argv, { root: CLI_ROOT });
+ } catch (err) {
+ if (!(err instanceof Error) || !err.message.startsWith('__PROCESS_EXIT__')) throw err;
+ } finally {
+ logSpy.mockRestore();
+ exitSpy.mockRestore();
+ }
+ return { out: plain(logs.join('\n')), exitCode };
+ }
+
+ // A real `Doctor.run()` takes seconds (it shells out to `git --version`,
+ // walks the workspace and loads config), so every case here carries an
+ // explicit timeout instead of racing vitest's 5s default.
+ const E2E_TIMEOUT = 60_000;
+
+ it('prints the row when NODE_ENV is unset, and stays exit 0', async () => {
+ delete process.env.NODE_ENV;
+
+ const run = await runDoctor();
+
+ expect(run.out).toContain('NODE_ENV');
+ expect(run.out).toContain('treated as production');
+ // A warning, so the run still calls the environment functional and never
+ // reaches `process.exit(1)`.
+ expect(run.exitCode).toBeUndefined();
+ expect(run.out).toContain('Environment is functional');
+ }, E2E_TIMEOUT);
+
+ // Both directions, because "no row" has to hold for the environment that
+ // agrees with the default as well as for the one that contradicts it: the row
+ // reports that a DEFAULT was taken, not that the mode is production.
+ it.each(['development', 'production'])(
+ 'says nothing about NODE_ENV once the operator set it (NODE_ENV=%s)',
+ async (value) => {
+ process.env.NODE_ENV = value;
+
+ const run = await runDoctor();
+
+ // The whole sentence is absent, not merely softened: a configured
+ // environment's report is what it was before #5673.
+ expect(run.out).not.toContain('treated as production');
+ expect(run.exitCode).toBeUndefined();
+ },
+ E2E_TIMEOUT,
+ );
+
+ it('routes through the shared renderer — `fix` detail appears only under --verbose', async () => {
+ delete process.env.NODE_ENV;
+
+ // #5403's rule: a warning's detail is optional reading, an error's is not.
+ // This row is a warning, so its `fix` must be hidden until asked for — the
+ // proof that it went through `renderHealthCheckResult` rather than growing
+ // a second, flagless format of its own.
+ const quiet = await runDoctor();
+ expect(quiet.out).not.toContain('NODE_ENV=development');
+
+ const verbose = await runDoctor(['--verbose']);
+ expect(verbose.out).toContain('NODE_ENV=development');
+ expect(verbose.out).toContain('NODE_ENV=production');
+ }, E2E_TIMEOUT);
+});