Skip to content

Commit 7d292fa

Browse files
committed
fix(cli,runtime): a NAMED artifact that is missing is a broken instruction, not an empty boot (#4110 follow-up)
#4110 made an absent artifact non-fatal all the way down — right for the conventional `<cwd>/dist/objectstack.json`, which is simply "not compiled yet". But `OS_ARTIFACT_PATH` / `{ artifactPath }` skip the existence check by design (the loader was to validate lazily), so that tolerance reached them too: OS_ARTIFACT_PATH=/nope os serve → No objectstack.config.ts found — booting from artifact (default host)... → ✓ Server is ready with the missing path named NOWHERE in the output — serve's boot-quiet window drops the loader's calm ENOENT line, which #4110 had moved from `warn` (buffered and replayed by #4084) to `log` (discarded). Before #4110 this died loudly. The distinction that matters is NAMED vs CONVENTIONAL, not the errno. `createDefaultHostConfig` — the boot with no config, where the artifact IS the deployment — now rejects a named local artifact that does not exist, naming the path and which source named it. The loader keeps its tolerance, so the config-boot path #4085 fixed is untouched, and `createStandaloneStack` is not touched at all. Two more corrections in the same spirit: - `serve`'s "config not found" refusal never said WHERE it looked. The two things that actually happen are a typo'd filename and the wrong working directory, and the second is the common one. It now names the config path, the artifact path, and that OS_ARTIFACT_PATH is unset — and still refuses rather than inventing a zero-object platform, pointing at `objectstack start` for a boot that is app-less on purpose. - That refusal was being TRUNCATED. `this.exit(1)` unwinds to oclif's `process.exit`, which does not drain a piped stdout, so a diagnostic split across several `console.log` calls loses its tail: measured, only the first two lines survived — i.e. exactly the "where to look" part went missing. Both of serve's pre-flight refusals emit one write now. The e2e added here caught it; review had not. Also fixes a test that pinned the wrong thing: `the DefaultDatasourcePlugin precedes ObjectQLPlugin (schema sync needs the driver)` asserted an ARRAY INDEX with a rationale the kernel does not implement — the connect happens in `init()`, and the dependency graph hoists ObjectQLPlugin ahead of the datasource plugin (measured: 6 slots earlier). It now pins the declared dependency that actually orders the two inits, which deleting the array position cannot break and deleting the declaration does. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Ve9HidCGRGtS2UjNPUGHSV
1 parent 73256b7 commit 7d292fa

7 files changed

Lines changed: 275 additions & 18 deletions

File tree

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
---
2+
'@objectstack/cli': patch
3+
'@objectstack/runtime': patch
4+
---
5+
6+
fix(cli,runtime): an artifact you NAMED and a boot input you don't have are different failures — say which (#4110 follow-up, #4131 step 1)
7+
8+
Three corrections, all from the same principle: a platform may boot with no
9+
application (#4085), and that says nothing about how a MISSING NAMED INPUT
10+
should be read.
11+
12+
- **A named-but-missing artifact boots empty and silently.** #4110 made an
13+
absent artifact non-fatal all the way down — right for the conventional
14+
`<cwd>/dist/objectstack.json`, which is just "not compiled yet". But
15+
`OS_ARTIFACT_PATH` / `{ artifactPath }` skip the existence check by design, so
16+
the tolerance reached them too: `OS_ARTIFACT_PATH=/nope os serve` printed
17+
"booting from artifact", reached `Server is ready`, and named the missing path
18+
NOWHERE in its output (serve's boot-quiet window drops the loader's calm
19+
line). `createDefaultHostConfig` — the boot with no config, where the artifact
20+
IS the deployment — now rejects a named local artifact that does not exist,
21+
naming both the path and which source named it. The loader keeps its
22+
tolerance, so the config-boot path #4085 fixed is untouched.
23+
24+
- **"Configuration file not found" never said where it looked.** The two things
25+
that actually happen are a typo'd filename and the wrong working directory,
26+
and the second is the common one. It now names the config path, the artifact
27+
path, and that `OS_ARTIFACT_PATH` is unset — and still refuses rather than
28+
inventing a zero-object platform, pointing at `objectstack start` for a boot
29+
that is app-less on purpose.
30+
31+
- **That refusal was being truncated.** `this.exit(1)` unwinds to oclif's
32+
`process.exit`, which does not drain a piped stdout, so a diagnostic split
33+
across several `console.log` calls loses its tail — measured: only the first
34+
two lines of the new message survived a pipe, i.e. exactly the part that says
35+
where to look went missing. Both of `serve`'s pre-flight refusals now emit one
36+
write. Caught by the e2e added here, not by review.
37+
38+
Also corrects the plugin-ordering claims in `createStandaloneStack` and in the
39+
test that pinned them: the comment said the datasource plugin's array position
40+
"MUST precede ObjectQLPlugin: its start() connects the default driver", and the
41+
test asserted that index with the same rationale. The connect happens in
42+
`init()`, and the kernel resolves order from the dependency graph — which hoists
43+
ObjectQLPlugin ahead of the datasource plugin (measured: 6 slots earlier), the
44+
reverse of what the slot reads as. The test now pins the declared dependency
45+
that actually orders the two inits, which deleting the array position cannot
46+
break and deleting the declaration does. #4131 tracks making the AppPlugin end
47+
of that contract enforced rather than conventional.

.changeset/standalone-stack-ordering-comment.md

Lines changed: 0 additions & 5 deletions
This file was deleted.

packages/cli/src/commands/serve.ts

Lines changed: 42 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -474,11 +474,19 @@ export default class Serve extends Command {
474474
// Ignore — fall through and try the requested port.
475475
}
476476
} else if (!(await isPortAvailable(requestedPort))) {
477-
console.log('');
478-
printError(`Port ${requestedPort} is already in use.`);
479-
console.log(chalk.dim(' ObjectStack does not auto-select a different port in production mode:'));
480-
console.log(chalk.dim(' a drifted port silently breaks reverse-proxy, OAuth callback, and CORS config.'));
481-
console.log(chalk.dim(' Free the port, or pick another via PORT=<port> (or --port <port>).'));
477+
// One write, for the reason spelled out at the "Nothing to serve" exit
478+
// below: `this.exit(1)` reaches `process.exit` without draining a piped
479+
// stdout, so a multi-call diagnostic loses its tail. Same defect, same
480+
// shape — this one is fixed by construction (the e2e that measured the
481+
// truncation drives the other exit; reaching this one needs a busy port in
482+
// production mode).
483+
console.log(
484+
'\n'
485+
+ chalk.red(` ✗ Port ${requestedPort} is already in use.\n`)
486+
+ chalk.dim(' ObjectStack does not auto-select a different port in production mode:\n')
487+
+ chalk.dim(' a drifted port silently breaks reverse-proxy, OAuth callback, and CORS config.\n')
488+
+ chalk.dim(' Free the port, or pick another via PORT=<port> (or --port <port>).'),
489+
);
482490
this.exit(1);
483491
}
484492

@@ -516,10 +524,35 @@ export default class Serve extends Command {
516524
if (process.env.OS_BOOT_EMPTY === '1') {
517525
useEmptyBoot = true;
518526
} else {
519-
printError(`Configuration file not found: ${absolutePath}`);
520-
console.log(chalk.dim(' Hint: Run `objectstack init` to create a new project,'));
521-
console.log(chalk.dim(' `objectstack start` to boot an empty kernel against your marketplace,'));
522-
console.log(chalk.dim(' or run `objectstack build` first / set OS_ARTIFACT_PATH.'));
527+
// Say WHERE it looked. "Not found" alone cannot distinguish the two
528+
// things that actually happen — a typo'd filename and the wrong cwd
529+
// (running from a monorepo root instead of the app folder) — and the
530+
// second is the common one, which listing the searched paths makes
531+
// self-evident. This stays an ERROR rather than degrading into an
532+
// empty boot: `os serve` was told to load something, and inventing a
533+
// zero-object platform instead would hide the mistake behind a
534+
// running server. Booting with no app at all is a real, supported
535+
// thing (`os serve` on a config with no metadata, or `os start`) —
536+
// but it is a stated intent, not a guess made on the user's behalf.
537+
// ONE write, deliberately. `this.exit(1)` unwinds to oclif's
538+
// `process.exit`, which does NOT wait for a piped stdout to drain — so
539+
// a diagnostic split across several `console.log` calls gets
540+
// truncated mid-message, and the reader loses exactly the part that
541+
// says where to look. (Measured: as separate calls, only the first two
542+
// lines survived a pipe.) An error whose tail can vanish is the #4012
543+
// shape all over again; assembling it into a single write keeps it
544+
// inside one pipe-buffer flush.
545+
console.log(
546+
chalk.red(' ✗ Nothing to serve — no config and no compiled artifact.') + '\n'
547+
+ chalk.dim(` Looked for a config at: ${absolutePath}\n`)
548+
+ chalk.dim(` Looked for an artifact at: ${path.resolve(process.cwd(), 'dist/objectstack.json')}\n`)
549+
+ chalk.dim(' OS_ARTIFACT_PATH is not set.\n')
550+
+ '\n'
551+
+ chalk.dim(' Hint: `objectstack init` scaffolds a new project;\n')
552+
+ chalk.dim(' `objectstack start` boots an app-less kernel against your marketplace;\n')
553+
+ chalk.dim(' `objectstack build` (or OS_ARTIFACT_PATH) supplies a compiled artifact.\n')
554+
+ chalk.dim(' Already have a project? Check your working directory.'),
555+
);
523556
this.exit(1);
524557
}
525558
}

packages/cli/test/helpers/serve-process.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,11 @@ export interface ServeRun {
4040
export function runServe(
4141
cwd: string,
4242
args: string[],
43-
opts: { waitFor: RegExp; timeoutMs?: number; config?: string; env?: Record<string, string> },
43+
// `env` values may be `undefined` to UNSET a variable for the child (Node
44+
// omits undefined entries), which is how a test asserts behaviour that depends
45+
// on a variable being absent — `''` would not do it, since the resolvers this
46+
// exercises use `??` and an empty string is not nullish.
47+
opts: { waitFor: RegExp; timeoutMs?: number; config?: string; env?: Record<string, string | undefined> },
4448
): Promise<ServeRun> {
4549
return new Promise((resolveRun, rejectRun) => {
4650
const child = spawn(TSX, [CLI, 'serve', opts.config ?? 'objectstack.config.ts', ...args], {

packages/cli/test/serve-no-artifact.e2e.test.ts

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -155,6 +155,48 @@ describe('os serve — boots without a compiled artifact (#4085)', () => {
155155
240_000,
156156
);
157157

158+
// The other side of the same principle. "The platform boots with no app" is a
159+
// statement about CAPABILITY, and it does not license guessing that a missing
160+
// input meant "boot empty". `os serve` was told to load something; the two
161+
// things that actually happen here are a typo'd filename and the wrong working
162+
// directory, and inventing a zero-object platform would hide both behind a
163+
// running server — the exact failure class #4085 was. So it errors, and it says
164+
// where it looked, which is what makes "wrong cwd" self-evident.
165+
it(
166+
'refuses, and names where it looked, when there is no config and no artifact',
167+
async () => {
168+
const emptyDir = mkdtempSync(join(tmpdir(), 'os-nothing-to-serve-'));
169+
try {
170+
const { stdout, stderr } = await runServe(emptyDir, ['--port', randomPort()], {
171+
// It never boots, so wait on something that can only appear on the way
172+
// down; the harness resolves on exit regardless.
173+
waitFor: /Nothing to serve/,
174+
timeoutMs: 120_000,
175+
// An inherited OS_ARTIFACT_PATH would send this down the
176+
// artifact-fallback branch instead. Unset it for the child.
177+
env: { OS_ARTIFACT_PATH: undefined, OS_BOOT_EMPTY: undefined },
178+
});
179+
const out = stdout + stderr;
180+
const seen = `\n--- stdout ---\n${stdout}\n--- stderr ---\n${stderr}`;
181+
182+
expect(out, `did not refuse${seen}`).toContain('Nothing to serve');
183+
// Both searched locations, by absolute path.
184+
expect(out, `config location not named${seen}`).toContain(
185+
join(emptyDir, 'objectstack.config.ts'),
186+
);
187+
expect(out, `artifact location not named${seen}`).toContain('dist/objectstack.json');
188+
// And it points at the command that DOES boot an app-less platform,
189+
// instead of silently becoming it.
190+
expect(out).toContain('objectstack start');
191+
// It must not have booted.
192+
expect(out).not.toContain('Server is ready');
193+
} finally {
194+
rmSync(emptyDir, { recursive: true, force: true });
195+
}
196+
},
197+
120_000,
198+
);
199+
158200
it(
159201
'serves on, and says why, when the config carries an app it cannot register',
160202
async () => {

packages/runtime/src/default-host.ts

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,40 @@ export async function createDefaultHostConfig(
9292
);
9393
}
9494

95+
// A NAMED artifact that is not there is a broken instruction, and this is
96+
// the boot with no `objectstack.config.ts` — the artifact IS the whole
97+
// deployment, so there is nothing else to serve. Fail loudly.
98+
//
99+
// The distinction that matters is **named vs conventional**, not the errno.
100+
// `<cwd>/dist/objectstack.json` missing means "not compiled yet", which is a
101+
// healthy state a development platform must boot from (#4085) — and
102+
// `resolveDefaultArtifactPath` already returns `undefined` for it, so it
103+
// never reaches here. But `OS_ARTIFACT_PATH` / `{ artifactPath }` are
104+
// returned WITHOUT an existence check (validated lazily by the loader), and
105+
// since #4110 made an absent artifact non-fatal all the way down —
106+
// `loadArtifactBundle` logs and returns null, `MetadataPlugin` starts
107+
// empty — that laziness turned a typo'd path into a silent empty boot:
108+
// `OS_ARTIFACT_PATH=/nope os serve` printed "booting from artifact", then
109+
// "Server is ready", and named the missing path NOWHERE in its output
110+
// (serve's boot-quiet window drops the loader's calm line). Checked here so
111+
// the fix lands where the intent is known, leaving the loader's tolerance
112+
// intact for the config-boot path that needs it.
113+
const namedBy = standaloneOpts.artifactPath
114+
? '`artifactPath`'
115+
: (process.env.OS_ARTIFACT_PATH ? 'OS_ARTIFACT_PATH' : null);
116+
if (
117+
namedBy
118+
&& resolvedArtifact
119+
&& !isHttpUrl(resolvedArtifact)
120+
&& !existsSync(resolvedArtifact)
121+
) {
122+
throw new Error(
123+
`[createDefaultHostConfig] The artifact named by ${namedBy} does not exist: `
124+
+ `"${resolvedArtifact}". Run 'os compile' to build it, correct the path, `
125+
+ `or unset ${namedBy} to boot from <cwd>/dist/objectstack.json.`,
126+
);
127+
}
128+
95129
// Empty-boot path: synthesize a minimal artifact stub inside the
96130
// ObjectStack home directory so MetadataPlugin has a real file to
97131
// read (and to watch for marketplace installs that land later).

packages/runtime/src/standalone-stack.test.ts

Lines changed: 105 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ import { mkdtempSync, rmSync, writeFileSync } from 'node:fs';
1818
import { tmpdir } from 'node:os';
1919
import { join } from 'node:path';
2020
import { createStandaloneStack } from './standalone-stack.js';
21-
import { createDefaultHostConfig } from './default-host.js';
21+
import { createDefaultHostConfig, resolveDefaultArtifactPath } from './default-host.js';
2222

2323
// A minimal `objectstack build` artifact carrying an app-declared default
2424
// profile with a hierarchy read scope, an add-on permission set, app roles,
@@ -253,12 +253,114 @@ describe('createStandaloneStack — default datasource declared, built via the s
253253
expect(r.titles).toContain('hello-driver');
254254
}, BOOT_TIMEOUT);
255255

256-
it('the DefaultDatasourcePlugin precedes ObjectQLPlugin (schema sync needs the driver)', async () => {
256+
// The composition ships both plugins, with the datasource ahead of the engine
257+
// in the array. That LIST SHAPE is all this asserts — it is NOT what orders
258+
// them, and this test's previous title ("…precedes ObjectQLPlugin (schema sync
259+
// needs the driver)") claimed otherwise. The kernel resolves init and start
260+
// order from the dependency graph, which HOISTS ObjectQLPlugin ahead of the
261+
// datasource plugin on a real boot (measured: objectql inits 6 slots earlier).
262+
// Pinning an array index as if it were the guarantee is how #4085 happened —
263+
// a reader trusts the index, moves a plugin, and nothing fails.
264+
it('composes the default datasource alongside the engine', async () => {
257265
const stack = await createStandaloneStack({ databaseUrl: 'memory://default-order' });
258266
const names = stack.plugins.map((p: any) => String(p?.name ?? p?.constructor?.name ?? ''));
259267
const dsIdx = names.indexOf('com.objectstack.runtime.default-datasource');
260268
const qlIdx = names.findIndex((n: string) => /objectql/i.test(n));
261269
expect(dsIdx).toBeGreaterThanOrEqual(0);
262-
expect(qlIdx).toBeGreaterThan(dsIdx);
270+
expect(qlIdx).toBeGreaterThanOrEqual(0);
263271
}, BOOT_TIMEOUT);
272+
273+
// …and THIS is the guarantee. The driver exists before boot schema-sync
274+
// because the datasource plugin connects in `init()` (Phase 1 completes before
275+
// ANY `start()` runs) and declares a hard dependency on ObjectQL, so the engine
276+
// is registered by the time that init runs. Delete the declaration and the
277+
// kernel stops ordering the two inits — which the array cannot notice, and
278+
// this does.
279+
it('declares the ObjectQL dependency that actually orders the two inits', async () => {
280+
const stack = await createStandaloneStack({ databaseUrl: 'memory://default-order-deps' });
281+
const ds = stack.plugins.find(
282+
(p: any) => p?.name === 'com.objectstack.runtime.default-datasource',
283+
) as any;
284+
expect(ds).toBeDefined();
285+
expect(ds.dependencies).toContain('com.objectstack.engine.objectql');
286+
}, BOOT_TIMEOUT);
287+
});
288+
289+
// #4110 follow-up — a NAMED artifact that does not exist is a broken
290+
// instruction, and `createDefaultHostConfig` is the boot with no
291+
// `objectstack.config.ts`: the artifact IS the deployment, so there is nothing
292+
// else to serve.
293+
//
294+
// #4110 made an absent artifact non-fatal all the way down (`loadArtifactBundle`
295+
// logs and returns null; `MetadataPlugin` starts empty) — correct for the
296+
// CONVENTIONAL `<cwd>/dist/objectstack.json`, which is simply "not compiled
297+
// yet". But `OS_ARTIFACT_PATH` / `{ artifactPath }` skip the existence check by
298+
// design, so that tolerance reached them too and turned a typo into a silent
299+
// empty boot: `OS_ARTIFACT_PATH=/nope os serve` reached "Server is ready" with
300+
// the missing path named NOWHERE in its output. The distinction that matters is
301+
// named vs conventional, not the errno.
302+
describe('createDefaultHostConfig — a named-but-missing artifact fails loudly (#4110 follow-up)', () => {
303+
const originalArtifactPath = process.env.OS_ARTIFACT_PATH;
304+
305+
afterAll(() => {
306+
if (originalArtifactPath === undefined) delete process.env.OS_ARTIFACT_PATH;
307+
else process.env.OS_ARTIFACT_PATH = originalArtifactPath;
308+
});
309+
310+
it('rejects an OS_ARTIFACT_PATH that does not exist, naming the path and the source', async () => {
311+
const missing = join(tmpdir(), `os-named-missing-${process.pid}`, 'objectstack.json');
312+
process.env.OS_ARTIFACT_PATH = missing;
313+
try {
314+
await expect(createDefaultHostConfig({ requireArtifact: true })).rejects.toThrow(
315+
/OS_ARTIFACT_PATH does not exist/,
316+
);
317+
await expect(createDefaultHostConfig({ requireArtifact: true })).rejects.toThrow(missing);
318+
} finally {
319+
delete process.env.OS_ARTIFACT_PATH;
320+
}
321+
});
322+
323+
it('rejects an explicit `artifactPath` that does not exist', async () => {
324+
const missing = join(tmpdir(), `os-named-missing-opt-${process.pid}`, 'objectstack.json');
325+
delete process.env.OS_ARTIFACT_PATH;
326+
await expect(
327+
createDefaultHostConfig({ requireArtifact: true, artifactPath: missing }),
328+
).rejects.toThrow(/`artifactPath` does not exist/);
329+
});
330+
331+
// …and it stays loud in empty-boot mode too: `requireArtifact: false` means
332+
// "an artifact is optional", not "ignore the one I named".
333+
it('rejects a named-but-missing artifact even when requireArtifact is false', async () => {
334+
const missing = join(tmpdir(), `os-named-missing-empty-${process.pid}`, 'objectstack.json');
335+
delete process.env.OS_ARTIFACT_PATH;
336+
await expect(
337+
createDefaultHostConfig({ requireArtifact: false, artifactPath: missing }),
338+
).rejects.toThrow(/does not exist/);
339+
});
340+
341+
// The control: nothing NAMED an artifact, so the conventional path being
342+
// absent keeps its own pre-existing message — this guard must not swallow it.
343+
it('keeps the "no artifact source" error when nothing named one', async () => {
344+
delete process.env.OS_ARTIFACT_PATH;
345+
const emptyDir = mkdtempSync(join(tmpdir(), 'os-no-artifact-source-'));
346+
const cwd = process.cwd();
347+
process.chdir(emptyDir);
348+
try {
349+
await expect(createDefaultHostConfig({ requireArtifact: true })).rejects.toThrow(
350+
/No artifact source available/,
351+
);
352+
} finally {
353+
process.chdir(cwd);
354+
rmSync(emptyDir, { recursive: true, force: true });
355+
}
356+
});
357+
358+
// A remote artifact is never stat'ed — a URL cannot be cheaply checked, and
359+
// the loader owns that failure.
360+
it('passes an http(s) artifact source through without a filesystem check', () => {
361+
delete process.env.OS_ARTIFACT_PATH;
362+
expect(resolveDefaultArtifactPath('https://example.com/objectstack.json')).toBe(
363+
'https://example.com/objectstack.json',
364+
);
365+
});
264366
});

0 commit comments

Comments
 (0)