Skip to content

Commit f4ec939

Browse files
committed
fix(cli): move the CLI's resolved artifact off OS_ARTIFACT_PATH onto an internal channel (#8985)
`os start` and `os dev` wrote their own resolved artifact path into the child `serve` environment as `OS_ARTIFACT_PATH` — the same variable an operator sets. The write happens before the downstream `objectstack.config.ts` is evaluated, so a config saw the variable set on every boot and could not tell an operator's instruction from the CLI's plumbing. The resolved path now travels on `OS_INTERNAL_ARTIFACT_PATH`, owned by `packages/cli/src/utils/internal-artifact-channel.ts`. `serve` reads it strictly between `OS_ARTIFACT_URL` and `OS_ARTIFACT_PATH`, which preserves every answer the resolution ladder gives today: it must beat the operator knob so `--artifact X` still wins over an exported `OS_ARTIFACT_PATH=Y`, and it must lose to the reference because `dev` sends its answer unconditionally. Unchanged and now pinned: the ladder itself, `start`'s refusal to set `OS_BOOT_EMPTY` on a reference boot, and the named-vs-conventional distinction that keeps a missing resolved artifact a loud refusal. Co-Authored-By: Claude <noreply@anthropic.com>
1 parent 0a5adba commit f4ec939

6 files changed

Lines changed: 540 additions & 26 deletions

File tree

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
---
2+
"@objectstack/cli": minor
3+
---
4+
5+
fix(cli): `os start` / `os dev` stop writing `OS_ARTIFACT_PATH` into the child `serve` environment — the CLI's own plumbing moves to an internal channel (#8985)
6+
7+
`os start` and `os dev` are supervisors: each resolves an artifact, then spawns
8+
`os serve` to boot it. Both handed the resolved path down by writing
9+
**`OS_ARTIFACT_PATH`** into the child environment — the same variable an operator
10+
sets to name an artifact. `dev` wrote it unconditionally; `start` wrote it
11+
whenever it had resolved anything and no `OS_ARTIFACT_URL` was in play. Both
12+
writes happen **before** the downstream `objectstack.config.ts` is evaluated.
13+
14+
So inside any config, that variable was set on **every** boot, including boots
15+
where no operator had ever mentioned it — measured from the shipped EE image
16+
with its own `ENV` deliberately deleted: `os start` still printed
17+
`Artifact: dist/objectstack.json` and handed that path down. A config could not
18+
answer *"did a human ask for this, or did the CLI put it here?"*
19+
20+
The resolved path now travels on **`OS_INTERNAL_ARTIFACT_PATH`**, a channel the
21+
CLI owns both ends of (`packages/cli/src/utils/internal-artifact-channel.ts`),
22+
and the property downstream consumers need is restored:
23+
24+
> **the presence of `OS_ARTIFACT_PATH` in a config's environment means an
25+
> operator set it.**
26+
27+
**Nothing about resolution changed.** Each command's ladder resolves in the
28+
parent exactly as before, and `serve` reads the new channel strictly between the
29+
reference and the operator knob:
30+
31+
```
32+
--artifact > OS_ARTIFACT_URL > OS_INTERNAL_ARTIFACT_PATH > OS_ARTIFACT_PATH > <cwd>/dist/objectstack.json
33+
```
34+
35+
That position is what preserves today's answers in both directions. It beats
36+
`OS_ARTIFACT_PATH` because `os start --artifact X` run with an operator's
37+
`OS_ARTIFACT_PATH=Y` exported boots **X** today — the parent used to overwrite
38+
the variable on the way down, and now inherits it untouched. It loses to
39+
`OS_ARTIFACT_URL` because `os dev` writes the channel unconditionally, as it
40+
wrote the old variable unconditionally, and the reference has always outranked
41+
the path.
42+
43+
Two further behaviours are unchanged and now pinned rather than incidental:
44+
`start` still refuses to set `OS_BOOT_EMPTY` when a reference is driving the
45+
boot (an unreachable artifact host stays a loud refusal instead of a silently
46+
empty platform), and a resolved-but-missing artifact is still "named" to
47+
`resolveDefaultArtifactPath`, so it fails loudly rather than booting empty.
48+
49+
**If you depended on the old side effect** — a config reading
50+
`process.env.OS_ARTIFACT_PATH` and expecting the CLI to have populated it — set
51+
the variable yourself, or read the artifact from the config's own inputs.
52+
`OS_ARTIFACT_PATH` remains a fully supported operator knob on the exact rung it
53+
has always occupied; the CLI simply no longer manufactures it on your behalf.
54+
`OS_INTERNAL_ARTIFACT_PATH` is not a supported knob and is deliberately absent
55+
from the environment-variable reference.
Lines changed: 241 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,241 @@
1+
// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* Pin: **the presence of `OS_ARTIFACT_PATH` in a config's environment means an
5+
* operator set it.**
6+
*
7+
* `os start` and `os dev` spawn `os serve`, and the downstream
8+
* `objectstack.config.ts` is evaluated inside that child. While the supervisors
9+
* wrote their own resolved artifact path into the child's `OS_ARTIFACT_PATH`,
10+
* the variable was set on **every** boot — so a config could not tell an
11+
* operator's instruction from the CLI's own plumbing, and a consumer wanting to
12+
* refuse the retired knob could only do so by inspecting its *value*.
13+
*
14+
* The plumbing now travels on `OS_INTERNAL_ARTIFACT_PATH`
15+
* (`utils/internal-artifact-channel.ts`). This file pins both halves of the
16+
* property, plus the two behaviours that had to survive the move: the
17+
* resolution ladder, and `start`'s deliberate refusal to declare an empty boot
18+
* acceptable when a reference is driving the boot.
19+
*
20+
* Two kinds of assertion here, and both are needed:
21+
*
22+
* - **Behavioural** — over `childEnvWithResolvedArtifact`, which is the whole
23+
* of what each command contributes to its child's artifact environment.
24+
* - **Structural** — a source assertion that neither command writes
25+
* `OS_ARTIFACT_PATH` into an env object at all. The behavioural pins describe
26+
* the helper; only this one refuses a future edit that re-adds the write
27+
* beside it. Both files compose their child env as
28+
* `{ ...childEnvWithResolvedArtifact(process.env, …), …other keys }`, so
29+
* "the helper is correct" plus "nothing else writes the key" is what makes
30+
* the composed env correct.
31+
*/
32+
33+
import { describe, it, expect, beforeEach, afterEach } from 'vitest';
34+
import { mkdtempSync, mkdirSync, rmSync, writeFileSync, readFileSync } from 'node:fs';
35+
import { tmpdir } from 'node:os';
36+
import path from 'path';
37+
import {
38+
INTERNAL_ARTIFACT_PATH_ENV,
39+
childEnvWithResolvedArtifact,
40+
readInternalArtifactPath,
41+
} from '../utils/internal-artifact-channel.js';
42+
import { resolveArtifactSource } from './start.js';
43+
44+
const ARTIFACT = '/srv/app/objectstack.json';
45+
46+
describe('the child `serve` env — OS_ARTIFACT_PATH means an operator set it', () => {
47+
it('carries NO OS_ARTIFACT_PATH when the operator did not set one', () => {
48+
const parentEnv = { PATH: '/usr/bin', NODE_ENV: 'production' };
49+
50+
for (const decision of [
51+
{ kind: 'resolved', path: ARTIFACT },
52+
{ kind: 'reference' },
53+
{ kind: 'empty' },
54+
] as const) {
55+
const childEnv = childEnvWithResolvedArtifact(parentEnv, decision);
56+
expect(
57+
Object.prototype.hasOwnProperty.call(childEnv, 'OS_ARTIFACT_PATH'),
58+
`decision ${decision.kind} must not introduce OS_ARTIFACT_PATH`,
59+
).toBe(false);
60+
expect(childEnv.OS_ARTIFACT_PATH).toBeUndefined();
61+
}
62+
});
63+
64+
it('still carries OS_ARTIFACT_PATH — verbatim — when the operator DID set one', () => {
65+
const parentEnv = { OS_ARTIFACT_PATH: './dist/from-operator.json' };
66+
67+
for (const decision of [
68+
{ kind: 'resolved', path: '/abs/dist/from-operator.json' },
69+
{ kind: 'reference' },
70+
{ kind: 'empty' },
71+
] as const) {
72+
const childEnv = childEnvWithResolvedArtifact(parentEnv, decision);
73+
// Inherited untouched: the child sees exactly what the operator wrote,
74+
// not an absolutised rewrite of it.
75+
expect(childEnv.OS_ARTIFACT_PATH).toBe('./dist/from-operator.json');
76+
}
77+
});
78+
79+
it('hands the resolved artifact down on the internal channel instead', () => {
80+
const childEnv = childEnvWithResolvedArtifact({}, { kind: 'resolved', path: ARTIFACT });
81+
expect(childEnv[INTERNAL_ARTIFACT_PATH_ENV]).toBe(ARTIFACT);
82+
expect(readInternalArtifactPath(childEnv)).toBe(ARTIFACT);
83+
});
84+
85+
it('lets the parent OWN the internal channel — an inherited value never speaks for it', () => {
86+
const parentEnv = { [INTERNAL_ARTIFACT_PATH_ENV]: '/stale/inherited.json' };
87+
88+
expect(childEnvWithResolvedArtifact(parentEnv, { kind: 'resolved', path: ARTIFACT }))
89+
.toMatchObject({ [INTERNAL_ARTIFACT_PATH_ENV]: ARTIFACT });
90+
91+
for (const decision of [{ kind: 'reference' }, { kind: 'empty' }] as const) {
92+
const childEnv = childEnvWithResolvedArtifact(parentEnv, decision);
93+
expect(
94+
readInternalArtifactPath(childEnv),
95+
`decision ${decision.kind} resolved nothing, so the channel must be empty`,
96+
).toBeUndefined();
97+
expect(Object.prototype.hasOwnProperty.call(childEnv, INTERNAL_ARTIFACT_PATH_ENV)).toBe(false);
98+
}
99+
});
100+
101+
it('reads a blank channel value as no decision at all', () => {
102+
expect(readInternalArtifactPath({})).toBeUndefined();
103+
expect(readInternalArtifactPath({ [INTERNAL_ARTIFACT_PATH_ENV]: '' })).toBeUndefined();
104+
expect(readInternalArtifactPath({ [INTERNAL_ARTIFACT_PATH_ENV]: ' ' })).toBeUndefined();
105+
});
106+
});
107+
108+
describe('OS_BOOT_EMPTY — the artifact-reference refusal survives the move', () => {
109+
it('is NOT set when a reference (OS_ARTIFACT_URL) is driving the boot', () => {
110+
// Load-bearing: setting it here would tell `serve` that booting an app-less
111+
// kernel is an acceptable outcome, turning an unreachable artifact host
112+
// into a silently empty platform instead of a loud refusal.
113+
const childEnv = childEnvWithResolvedArtifact({}, { kind: 'reference' });
114+
expect(childEnv.OS_BOOT_EMPTY).toBeUndefined();
115+
expect(readInternalArtifactPath(childEnv)).toBeUndefined();
116+
});
117+
118+
it('is NOT set when an artifact was resolved', () => {
119+
expect(childEnvWithResolvedArtifact({}, { kind: 'resolved', path: ARTIFACT }).OS_BOOT_EMPTY)
120+
.toBeUndefined();
121+
});
122+
123+
it('is set only when nothing resolved and an empty boot IS the intent', () => {
124+
expect(childEnvWithResolvedArtifact({}, { kind: 'empty' }).OS_BOOT_EMPTY).toBe('1');
125+
});
126+
127+
it('never CLEARS an operator-exported OS_BOOT_EMPTY (add-only, as before)', () => {
128+
const parentEnv = { OS_BOOT_EMPTY: '1' };
129+
for (const decision of [
130+
{ kind: 'resolved', path: ARTIFACT },
131+
{ kind: 'reference' },
132+
{ kind: 'empty' },
133+
] as const) {
134+
expect(
135+
childEnvWithResolvedArtifact(parentEnv, decision).OS_BOOT_EMPTY,
136+
`decision ${decision.kind} must not start clearing an inherited OS_BOOT_EMPTY`,
137+
).toBe('1');
138+
}
139+
});
140+
});
141+
142+
describe('resolveArtifactSource — the resolution ladder is unchanged', () => {
143+
let cwd: string;
144+
let home: string;
145+
146+
const write = (dir: string, rel: string) => {
147+
const abs = path.join(dir, rel);
148+
mkdirSync(path.dirname(abs), { recursive: true });
149+
writeFileSync(abs, '{}');
150+
return abs;
151+
};
152+
153+
beforeEach(() => {
154+
cwd = mkdtempSync(path.join(tmpdir(), 'os-artifact-cwd-'));
155+
home = mkdtempSync(path.join(tmpdir(), 'os-artifact-home-'));
156+
});
157+
afterEach(() => {
158+
for (const d of [cwd, home]) {
159+
try { rmSync(d, { recursive: true, force: true }); } catch { /* noop */ }
160+
}
161+
});
162+
163+
it('rung 1: --artifact wins over everything, including an operator OS_ARTIFACT_PATH', () => {
164+
const flagFile = write(cwd, 'build/pinned.json');
165+
write(cwd, 'dist/objectstack.json');
166+
write(home, 'dist/objectstack.json');
167+
168+
const r = resolveArtifactSource('build/pinned.json', home, {
169+
cwd,
170+
env: { OS_ARTIFACT_PATH: '/from/env.json' },
171+
});
172+
expect(r?.path).toBe(flagFile);
173+
});
174+
175+
it('rung 1: --artifact passes an http(s) URL through untouched', () => {
176+
const url = 'https://cdn.example.com/app.json';
177+
expect(resolveArtifactSource(url, home, { cwd, env: {} })?.path).toBe(url);
178+
});
179+
180+
it('rung 2: $OS_ARTIFACT_PATH wins over both auto-detected locations', () => {
181+
write(cwd, 'dist/objectstack.json');
182+
write(home, 'dist/objectstack.json');
183+
184+
const r = resolveArtifactSource(undefined, home, {
185+
cwd,
186+
env: { OS_ARTIFACT_PATH: 'custom/app.json' },
187+
});
188+
// Anchored on the cwd, exactly as before — the ladder resolves it; the
189+
// variable itself is inherited by the child untouched.
190+
expect(r?.path).toBe(path.join(cwd, 'custom/app.json'));
191+
});
192+
193+
it('rung 2: $OS_ARTIFACT_PATH may itself be an http(s) URL', () => {
194+
const url = 'https://cdn.example.com/env.json';
195+
expect(resolveArtifactSource(undefined, home, { cwd, env: { OS_ARTIFACT_PATH: url } })?.path)
196+
.toBe(url);
197+
});
198+
199+
it('rung 3: <cwd>/dist/objectstack.json wins over <home>/dist', () => {
200+
const cwdArtifact = write(cwd, 'dist/objectstack.json');
201+
write(home, 'dist/objectstack.json');
202+
expect(resolveArtifactSource(undefined, home, { cwd, env: {} })?.path).toBe(cwdArtifact);
203+
});
204+
205+
it('rung 4: <home>/dist/objectstack.json is the last resort', () => {
206+
const homeArtifact = write(home, 'dist/objectstack.json');
207+
expect(resolveArtifactSource(undefined, home, { cwd, env: {} })?.path).toBe(homeArtifact);
208+
});
209+
210+
it('rung 5: nothing reachable resolves to undefined', () => {
211+
expect(resolveArtifactSource(undefined, home, { cwd, env: {} })).toBeUndefined();
212+
});
213+
});
214+
215+
describe('structural: the supervisors never write the operator knob', () => {
216+
// Strip comments first — both files discuss OS_ARTIFACT_PATH at length, and
217+
// the prose is exactly what this assertion must NOT read.
218+
const codeOf = (file: string): string => {
219+
const src = readFileSync(new URL(`./${file}`, import.meta.url), 'utf8');
220+
return src
221+
.replace(/\/\*[\s\S]*?\*\//g, '')
222+
.split('\n')
223+
.map((line) => (line.trim().startsWith('//') ? '' : line.replace(/\/\/.*$/, '')))
224+
.join('\n');
225+
};
226+
227+
for (const file of ['start.ts', 'dev.ts']) {
228+
it(`${file} contains no OS_ARTIFACT_PATH assignment`, () => {
229+
const offenders = codeOf(file)
230+
.split('\n')
231+
.filter((line) => /OS_ARTIFACT_PATH\s*[:=]/.test(line));
232+
expect(
233+
offenders,
234+
`${file} must not write OS_ARTIFACT_PATH into a child environment — the CLI's own `
235+
+ `resolved artifact travels on ${INTERNAL_ARTIFACT_PATH_ENV}, so that a downstream `
236+
+ `objectstack.config.ts seeing OS_ARTIFACT_PATH knows an operator set it. `
237+
+ `Reading process.env.OS_ARTIFACT_PATH (the operator's value) stays correct.`,
238+
).toEqual([]);
239+
});
240+
}
241+
});

‎packages/cli/src/commands/dev.ts‎

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import {
1616
assessArtifactStaleness,
1717
formatMtimeGap,
1818
} from '../utils/dev-restart.js';
19+
import { childEnvWithResolvedArtifact } from '../utils/internal-artifact-channel.js';
1920
import { readEnvWithDeprecation, isMcpServerEnabled } from '@objectstack/types';
2021
import type { ResolvedProjectDatabaseUrl } from '@objectstack/runtime';
2122

@@ -327,10 +328,18 @@ export default class Dev extends Command {
327328
fs.mkdirSync(path.dirname(resolvedDb.url.replace(/^file:/, '')), { recursive: true });
328329
}
329330
const effectiveDb = resolvedDb.url;
331+
// `dev` always has a resolved artifact by this point (it compiled one, or
332+
// was handed one with `--artifact`, or is pointing at the canonical
333+
// `<cwd>/dist/objectstack.json`), so the decision is unconditionally
334+
// `resolved` — exactly as unconditional as the `OS_ARTIFACT_PATH` write
335+
// it replaces. What changed is the channel: the resolved path travels on
336+
// the CLI's own `OS_INTERNAL_ARTIFACT_PATH`, so an `OS_ARTIFACT_PATH`
337+
// seen by a downstream `objectstack.config.ts` means an operator set it.
338+
// The operator's own value is inherited verbatim, and `dev`'s ladder
339+
// above still honours it on the rung it has always occupied.
330340
const localEnv: NodeJS.ProcessEnv = {
331-
...process.env,
341+
...childEnvWithResolvedArtifact(process.env, { kind: 'resolved', path: artifactPath }),
332342
OS_ENVIRONMENT_ID: environmentId,
333-
OS_ARTIFACT_PATH: artifactPath,
334343
OS_SEED_ADMIN: seedAdmin ? '1' : '0',
335344
...(seedAdmin && flags['admin-email'] ? { OS_SEED_ADMIN_EMAIL: flags['admin-email'] } : {}),
336345
...(seedAdmin && flags['admin-password'] ? { OS_SEED_ADMIN_PASSWORD: flags['admin-password'] } : {}),

‎packages/cli/src/commands/serve.ts‎

Lines changed: 33 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import { bundleRequire } from 'bundle-require';
99
import { loadConfig, BUNDLE_REQUIRE_EXTERNALS } from '../utils/config.js';
1010
import { mergeBootConfig } from '../utils/merge-boot-config.js';
1111
import { isHostConfig, shouldBootWithLibrary } from '../utils/plugin-detection.js';
12+
import { readInternalArtifactPath } from '../utils/internal-artifact-channel.js';
1213
import {
1314
resolveDriverType,
1415
resolveStorageDefinition,
@@ -956,7 +957,19 @@ export default class Serve extends Command {
956957
// fragment. It is resolved here, before anything else looks for an
957958
// artifact, and it wins over every local lookup:
958959
//
959-
// --artifact > OS_ARTIFACT_URL > OS_ARTIFACT_PATH > <cwd>/dist/…
960+
// --artifact > OS_ARTIFACT_URL > OS_INTERNAL_ARTIFACT_PATH
961+
// > OS_ARTIFACT_PATH > <cwd>/dist/…
962+
//
963+
// `OS_INTERNAL_ARTIFACT_PATH` is the CLI's private parent-to-child channel:
964+
// an `os start` / `os dev` supervisor resolved an artifact through its own
965+
// ladder and is handing the answer down. It sits BELOW the reference (a
966+
// supervisor that saw OS_ARTIFACT_URL resolves nothing and sends nothing,
967+
// and `os dev` sends its answer unconditionally, so the reference has to
968+
// keep outranking it) and ABOVE the operator's OS_ARTIFACT_PATH (which the
969+
// supervisor no longer overwrites on the way down, so only a higher rung
970+
// keeps `--artifact` beating an exported OS_ARTIFACT_PATH the way it does
971+
// today). See `utils/internal-artifact-channel.ts` for why the CLI stopped
972+
// writing the operator's knob at all.
960973
//
961974
// Beating OS_ARTIFACT_PATH is not a nicety, it is the acceptance
962975
// criterion: the official runtime image sets
@@ -1018,7 +1031,11 @@ export default class Serve extends Command {
10181031

10191032
if (configMissing && !pinnedArtifact) {
10201033
const { resolveDefaultArtifactPath } = await import('@objectstack/runtime');
1021-
const artifactSource = resolveDefaultArtifactPath();
1034+
// A supervising `os start` / `os dev` passes its already-resolved answer
1035+
// as the explicit override — the same position `OS_ARTIFACT_PATH` used to
1036+
// occupy when the supervisor wrote it, so a named-but-missing artifact is
1037+
// still a loud refusal rather than a silent empty boot.
1038+
const artifactSource = resolveDefaultArtifactPath(readInternalArtifactPath());
10221039
if (!artifactSource) {
10231040
// Quick-start mode: `objectstack start` lets the user boot an
10241041
// empty kernel with no config and no artifact, then install apps
@@ -1246,7 +1263,15 @@ export default class Serve extends Command {
12461263
// what stops the loader from fetching the URL a second time — a pin
12471264
// that verifies one response while a different response boots would
12481265
// verify nothing.
1249-
...(pinnedArtifact ? { artifactPath: pinnedArtifact.localPath } : {}),
1266+
// Same reasoning one rung down: when no reference is in play, a
1267+
// supervisor's resolved answer is handed over explicitly instead of
1268+
// being re-derived from the environment.
1269+
...(pinnedArtifact
1270+
? { artifactPath: pinnedArtifact.localPath }
1271+
: (() => {
1272+
const internal = readInternalArtifactPath();
1273+
return internal ? { artifactPath: internal } : {};
1274+
})()),
12501275
});
12511276
// [#4002] `api` merges per key — see mergeBootConfig. A shallow spread
12521277
// let the boot builder's two scoping keys wipe the author's whole `api`
@@ -1765,7 +1790,11 @@ export default class Serve extends Command {
17651790
if (!hasMetadataPlugin) {
17661791
try {
17671792
const { resolveDefaultArtifactPath } = await import('@objectstack/runtime');
1768-
const hmrArtifactPath = resolveDefaultArtifactPath();
1793+
// `os dev` is the only caller that reaches here, and it is a
1794+
// supervisor: read its channel, or the artifact this HMR watcher
1795+
// polls would silently drift to `<cwd>/dist/objectstack.json`
1796+
// whenever `os dev --artifact <elsewhere>` was used.
1797+
const hmrArtifactPath = resolveDefaultArtifactPath(readInternalArtifactPath());
17691798
if (hmrArtifactPath && !/^https?:\/\//i.test(hmrArtifactPath)) {
17701799
const { MetadataPlugin } = await import('@objectstack/metadata');
17711800
// Mirror the standalone stack's dev config exactly

0 commit comments

Comments
 (0)