diff --git a/.changeset/serve-named-artifact-and-ordering-truth.md b/.changeset/serve-named-artifact-and-ordering-truth.md new file mode 100644 index 0000000000..379a276ae3 --- /dev/null +++ b/.changeset/serve-named-artifact-and-ordering-truth.md @@ -0,0 +1,47 @@ +--- +'@objectstack/cli': patch +'@objectstack/runtime': patch +--- + +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) + +Three corrections, all from the same principle: a platform may boot with no +application (#4085), and that says nothing about how a MISSING NAMED INPUT +should be read. + +- **A named-but-missing artifact boots empty and silently.** #4110 made an + absent artifact non-fatal all the way down — right for the conventional + `/dist/objectstack.json`, which is just "not compiled yet". But + `OS_ARTIFACT_PATH` / `{ artifactPath }` skip the existence check by design, so + the tolerance reached them too: `OS_ARTIFACT_PATH=/nope os serve` printed + "booting from artifact", reached `Server is ready`, and named the missing path + NOWHERE in its output (serve's boot-quiet window drops the loader's calm + line). `createDefaultHostConfig` — the boot with no config, where the artifact + IS the deployment — now rejects a named local artifact that does not exist, + naming both the path and which source named it. The loader keeps its + tolerance, so the config-boot path #4085 fixed is untouched. + +- **"Configuration file not found" 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 of the new message survived a pipe, i.e. exactly the part that says + where to look went missing. Both of `serve`'s pre-flight refusals now emit one + write. Caught by the e2e added here, not by review. + +Also corrects the plugin-ordering claims in `createStandaloneStack` and in the +test that pinned them: the comment said the datasource plugin's array position +"MUST precede ObjectQLPlugin: its start() connects the default driver", and the +test asserted that index with the same rationale. The connect happens in +`init()`, and the kernel resolves order from the dependency graph — which hoists +ObjectQLPlugin ahead of the datasource plugin (measured: 6 slots earlier), the +reverse of what the slot reads as. The test now pins the declared dependency +that actually orders the two inits, which deleting the array position cannot +break and deleting the declaration does. #4131 tracks making the AppPlugin end +of that contract enforced rather than conventional. diff --git a/content/docs/deployment/cli.mdx b/content/docs/deployment/cli.mdx index 7b77db491a..a8ed63fd2f 100644 --- a/content/docs/deployment/cli.mdx +++ b/content/docs/deployment/cli.mdx @@ -323,6 +323,16 @@ os start **Resolution priority (artifact):** `--artifact` > `OS_ARTIFACT_PATH` > `/dist/objectstack.json` > `/dist/objectstack.json` > auto-compile from `objectstack.config.ts` (when present) > empty kernel. **Resolution priority (database):** `--database` > `OS_DATABASE_URL` > `DATABASE_URL` (legacy) > `file:/data/objectstack.db`. + +A **named** artifact (`--artifact` or `OS_ARTIFACT_PATH`) does *not* participate +in that fall-through: it is used as given, and a local path that does not exist +**fails the boot** — naming the path and which of the two named it — instead of +quietly continuing down the list. You asked for a specific artifact, so booting +something else (or an empty kernel) would hide the typo behind a running server. +The fall-through applies to the **conventional** locations only. Remote +(`http(s)://`) sources cannot be checked up front and are validated when fetched. + + **What it boots:** - Reads the artifact's `manifest`, `objects`, `views`, `flows`, … - Auto-registers the platform services declared in `requires: [...]` (e.g. `ai`, `automation`, `analytics`, `auth`, `ui`). Declaring a **service** capability (`automation`, `analytics`, `ai`, `audit`, …) is a *requirement*: if its provider package isn't installed, boot **fails fast** with a clear error instead of silently starting without a capability you asked for. (`auth` and `ui` are tier-gated with their own opt-in rules — `auth`'s secret-gated skip is described below.) diff --git a/packages/cli/src/commands/serve.ts b/packages/cli/src/commands/serve.ts index 3175f23749..cda150ecc3 100644 --- a/packages/cli/src/commands/serve.ts +++ b/packages/cli/src/commands/serve.ts @@ -474,11 +474,19 @@ export default class Serve extends Command { // Ignore — fall through and try the requested port. } } else if (!(await isPortAvailable(requestedPort))) { - console.log(''); - printError(`Port ${requestedPort} is already in use.`); - console.log(chalk.dim(' ObjectStack does not auto-select a different port in production mode:')); - console.log(chalk.dim(' a drifted port silently breaks reverse-proxy, OAuth callback, and CORS config.')); - console.log(chalk.dim(' Free the port, or pick another via PORT= (or --port ).')); + // One write, for the reason spelled out at the "Nothing to serve" exit + // below: `this.exit(1)` reaches `process.exit` without draining a piped + // stdout, so a multi-call diagnostic loses its tail. Same defect, same + // shape — this one is fixed by construction (the e2e that measured the + // truncation drives the other exit; reaching this one needs a busy port in + // production mode). + console.log( + '\n' + + chalk.red(` ✗ Port ${requestedPort} is already in use.\n`) + + chalk.dim(' ObjectStack does not auto-select a different port in production mode:\n') + + chalk.dim(' a drifted port silently breaks reverse-proxy, OAuth callback, and CORS config.\n') + + chalk.dim(' Free the port, or pick another via PORT= (or --port ).'), + ); this.exit(1); } @@ -516,10 +524,35 @@ export default class Serve extends Command { if (process.env.OS_BOOT_EMPTY === '1') { useEmptyBoot = true; } else { - printError(`Configuration file not found: ${absolutePath}`); - console.log(chalk.dim(' Hint: Run `objectstack init` to create a new project,')); - console.log(chalk.dim(' `objectstack start` to boot an empty kernel against your marketplace,')); - console.log(chalk.dim(' or run `objectstack build` first / set OS_ARTIFACT_PATH.')); + // Say WHERE it looked. "Not found" alone cannot distinguish the two + // things that actually happen — a typo'd filename and the wrong cwd + // (running from a monorepo root instead of the app folder) — and the + // second is the common one, which listing the searched paths makes + // self-evident. This stays an ERROR rather than degrading into an + // empty boot: `os serve` was told to load something, and inventing a + // zero-object platform instead would hide the mistake behind a + // running server. Booting with no app at all is a real, supported + // thing (`os serve` on a config with no metadata, or `os start`) — + // but it is a stated intent, not a guess made on the user's behalf. + // ONE write, deliberately. `this.exit(1)` unwinds to oclif's + // `process.exit`, which does NOT wait for a piped stdout to drain — so + // a diagnostic split across several `console.log` calls gets + // truncated mid-message, and the reader loses exactly the part that + // says where to look. (Measured: as separate calls, only the first two + // lines survived a pipe.) An error whose tail can vanish is the #4012 + // shape all over again; assembling it into a single write keeps it + // inside one pipe-buffer flush. + console.log( + chalk.red(' ✗ Nothing to serve — no config and no compiled artifact.') + '\n' + + chalk.dim(` Looked for a config at: ${absolutePath}\n`) + + chalk.dim(` Looked for an artifact at: ${path.resolve(process.cwd(), 'dist/objectstack.json')}\n`) + + chalk.dim(' OS_ARTIFACT_PATH is not set.\n') + + '\n' + + chalk.dim(' Hint: `objectstack init` scaffolds a new project;\n') + + chalk.dim(' `objectstack start` boots an app-less kernel against your marketplace;\n') + + chalk.dim(' `objectstack build` (or OS_ARTIFACT_PATH) supplies a compiled artifact.\n') + + chalk.dim(' Already have a project? Check your working directory.'), + ); this.exit(1); } } diff --git a/packages/cli/test/helpers/serve-process.ts b/packages/cli/test/helpers/serve-process.ts index 2db9b1f29e..1909dc93ae 100644 --- a/packages/cli/test/helpers/serve-process.ts +++ b/packages/cli/test/helpers/serve-process.ts @@ -40,7 +40,11 @@ export interface ServeRun { export function runServe( cwd: string, args: string[], - opts: { waitFor: RegExp; timeoutMs?: number; config?: string; env?: Record }, + // `env` values may be `undefined` to UNSET a variable for the child (Node + // omits undefined entries), which is how a test asserts behaviour that depends + // on a variable being absent — `''` would not do it, since the resolvers this + // exercises use `??` and an empty string is not nullish. + opts: { waitFor: RegExp; timeoutMs?: number; config?: string; env?: Record }, ): Promise { return new Promise((resolveRun, rejectRun) => { const child = spawn(TSX, [CLI, 'serve', opts.config ?? 'objectstack.config.ts', ...args], { diff --git a/packages/cli/test/serve-no-artifact.e2e.test.ts b/packages/cli/test/serve-no-artifact.e2e.test.ts index 75c934caa8..bc467b2ed2 100644 --- a/packages/cli/test/serve-no-artifact.e2e.test.ts +++ b/packages/cli/test/serve-no-artifact.e2e.test.ts @@ -155,6 +155,48 @@ describe('os serve — boots without a compiled artifact (#4085)', () => { 240_000, ); + // The other side of the same principle. "The platform boots with no app" is a + // statement about CAPABILITY, and it does not license guessing that a missing + // input meant "boot empty". `os serve` was told to load something; the two + // things that actually happen here are a typo'd filename and the wrong working + // directory, and inventing a zero-object platform would hide both behind a + // running server — the exact failure class #4085 was. So it errors, and it says + // where it looked, which is what makes "wrong cwd" self-evident. + it( + 'refuses, and names where it looked, when there is no config and no artifact', + async () => { + const emptyDir = mkdtempSync(join(tmpdir(), 'os-nothing-to-serve-')); + try { + const { stdout, stderr } = await runServe(emptyDir, ['--port', randomPort()], { + // It never boots, so wait on something that can only appear on the way + // down; the harness resolves on exit regardless. + waitFor: /Nothing to serve/, + timeoutMs: 120_000, + // An inherited OS_ARTIFACT_PATH would send this down the + // artifact-fallback branch instead. Unset it for the child. + env: { OS_ARTIFACT_PATH: undefined, OS_BOOT_EMPTY: undefined }, + }); + const out = stdout + stderr; + const seen = `\n--- stdout ---\n${stdout}\n--- stderr ---\n${stderr}`; + + expect(out, `did not refuse${seen}`).toContain('Nothing to serve'); + // Both searched locations, by absolute path. + expect(out, `config location not named${seen}`).toContain( + join(emptyDir, 'objectstack.config.ts'), + ); + expect(out, `artifact location not named${seen}`).toContain('dist/objectstack.json'); + // And it points at the command that DOES boot an app-less platform, + // instead of silently becoming it. + expect(out).toContain('objectstack start'); + // It must not have booted. + expect(out).not.toContain('Server is ready'); + } finally { + rmSync(emptyDir, { recursive: true, force: true }); + } + }, + 120_000, + ); + it( 'serves on, and says why, when the config carries an app it cannot register', async () => { diff --git a/packages/runtime/src/default-host.ts b/packages/runtime/src/default-host.ts index bb535c92b8..c0f7e87995 100644 --- a/packages/runtime/src/default-host.ts +++ b/packages/runtime/src/default-host.ts @@ -92,6 +92,40 @@ export async function createDefaultHostConfig( ); } + // A NAMED artifact that is not there is a broken instruction, and this is + // the boot with no `objectstack.config.ts` — the artifact IS the whole + // deployment, so there is nothing else to serve. Fail loudly. + // + // The distinction that matters is **named vs conventional**, not the errno. + // `/dist/objectstack.json` missing means "not compiled yet", which is a + // healthy state a development platform must boot from (#4085) — and + // `resolveDefaultArtifactPath` already returns `undefined` for it, so it + // never reaches here. But `OS_ARTIFACT_PATH` / `{ artifactPath }` are + // returned WITHOUT an existence check (validated lazily by the loader), and + // since #4110 made an absent artifact non-fatal all the way down — + // `loadArtifactBundle` logs and returns null, `MetadataPlugin` starts + // empty — that laziness turned a typo'd path into a silent empty boot: + // `OS_ARTIFACT_PATH=/nope os serve` printed "booting from artifact", then + // "Server is ready", and named the missing path NOWHERE in its output + // (serve's boot-quiet window drops the loader's calm line). Checked here so + // the fix lands where the intent is known, leaving the loader's tolerance + // intact for the config-boot path that needs it. + const namedBy = standaloneOpts.artifactPath + ? '`artifactPath`' + : (process.env.OS_ARTIFACT_PATH ? 'OS_ARTIFACT_PATH' : null); + if ( + namedBy + && resolvedArtifact + && !isHttpUrl(resolvedArtifact) + && !existsSync(resolvedArtifact) + ) { + throw new Error( + `[createDefaultHostConfig] The artifact named by ${namedBy} does not exist: ` + + `"${resolvedArtifact}". Run 'os compile' to build it, correct the path, ` + + `or unset ${namedBy} to boot from /dist/objectstack.json.`, + ); + } + // Empty-boot path: synthesize a minimal artifact stub inside the // ObjectStack home directory so MetadataPlugin has a real file to // read (and to watch for marketplace installs that land later). diff --git a/packages/runtime/src/standalone-stack.test.ts b/packages/runtime/src/standalone-stack.test.ts index 7f0f71ff29..e2b2012d9f 100644 --- a/packages/runtime/src/standalone-stack.test.ts +++ b/packages/runtime/src/standalone-stack.test.ts @@ -18,7 +18,7 @@ import { mkdtempSync, rmSync, writeFileSync } from 'node:fs'; import { tmpdir } from 'node:os'; import { join } from 'node:path'; import { createStandaloneStack } from './standalone-stack.js'; -import { createDefaultHostConfig } from './default-host.js'; +import { createDefaultHostConfig, resolveDefaultArtifactPath } from './default-host.js'; // A minimal `objectstack build` artifact carrying an app-declared default // 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 expect(r.titles).toContain('hello-driver'); }, BOOT_TIMEOUT); - it('the DefaultDatasourcePlugin precedes ObjectQLPlugin (schema sync needs the driver)', async () => { + // The composition ships both plugins, with the datasource ahead of the engine + // in the array. That LIST SHAPE is all this asserts — it is NOT what orders + // them, and this test's previous title ("…precedes ObjectQLPlugin (schema sync + // needs the driver)") claimed otherwise. The kernel resolves init and start + // order from the dependency graph, which HOISTS ObjectQLPlugin ahead of the + // datasource plugin on a real boot (measured: objectql inits 6 slots earlier). + // Pinning an array index as if it were the guarantee is how #4085 happened — + // a reader trusts the index, moves a plugin, and nothing fails. + it('composes the default datasource alongside the engine', async () => { const stack = await createStandaloneStack({ databaseUrl: 'memory://default-order' }); const names = stack.plugins.map((p: any) => String(p?.name ?? p?.constructor?.name ?? '')); const dsIdx = names.indexOf('com.objectstack.runtime.default-datasource'); const qlIdx = names.findIndex((n: string) => /objectql/i.test(n)); expect(dsIdx).toBeGreaterThanOrEqual(0); - expect(qlIdx).toBeGreaterThan(dsIdx); + expect(qlIdx).toBeGreaterThanOrEqual(0); }, BOOT_TIMEOUT); + + // …and THIS is the guarantee. The driver exists before boot schema-sync + // because the datasource plugin connects in `init()` (Phase 1 completes before + // ANY `start()` runs) and declares a hard dependency on ObjectQL, so the engine + // is registered by the time that init runs. Delete the declaration and the + // kernel stops ordering the two inits — which the array cannot notice, and + // this does. + it('declares the ObjectQL dependency that actually orders the two inits', async () => { + const stack = await createStandaloneStack({ databaseUrl: 'memory://default-order-deps' }); + const ds = stack.plugins.find( + (p: any) => p?.name === 'com.objectstack.runtime.default-datasource', + ) as any; + expect(ds).toBeDefined(); + expect(ds.dependencies).toContain('com.objectstack.engine.objectql'); + }, BOOT_TIMEOUT); +}); + +// #4110 follow-up — a NAMED artifact that does not exist is a broken +// instruction, and `createDefaultHostConfig` is the boot with no +// `objectstack.config.ts`: the artifact IS the deployment, so there is nothing +// else to serve. +// +// #4110 made an absent artifact non-fatal all the way down (`loadArtifactBundle` +// logs and returns null; `MetadataPlugin` starts empty) — correct for the +// CONVENTIONAL `/dist/objectstack.json`, which is simply "not compiled +// yet". But `OS_ARTIFACT_PATH` / `{ artifactPath }` skip the existence check by +// design, so that tolerance reached them too and turned a typo into a silent +// empty boot: `OS_ARTIFACT_PATH=/nope os serve` reached "Server is ready" with +// the missing path named NOWHERE in its output. The distinction that matters is +// named vs conventional, not the errno. +describe('createDefaultHostConfig — a named-but-missing artifact fails loudly (#4110 follow-up)', () => { + const originalArtifactPath = process.env.OS_ARTIFACT_PATH; + + afterAll(() => { + if (originalArtifactPath === undefined) delete process.env.OS_ARTIFACT_PATH; + else process.env.OS_ARTIFACT_PATH = originalArtifactPath; + }); + + it('rejects an OS_ARTIFACT_PATH that does not exist, naming the path and the source', async () => { + const missing = join(tmpdir(), `os-named-missing-${process.pid}`, 'objectstack.json'); + process.env.OS_ARTIFACT_PATH = missing; + try { + await expect(createDefaultHostConfig({ requireArtifact: true })).rejects.toThrow( + /OS_ARTIFACT_PATH does not exist/, + ); + await expect(createDefaultHostConfig({ requireArtifact: true })).rejects.toThrow(missing); + } finally { + delete process.env.OS_ARTIFACT_PATH; + } + }); + + it('rejects an explicit `artifactPath` that does not exist', async () => { + const missing = join(tmpdir(), `os-named-missing-opt-${process.pid}`, 'objectstack.json'); + delete process.env.OS_ARTIFACT_PATH; + await expect( + createDefaultHostConfig({ requireArtifact: true, artifactPath: missing }), + ).rejects.toThrow(/`artifactPath` does not exist/); + }); + + // …and it stays loud in empty-boot mode too: `requireArtifact: false` means + // "an artifact is optional", not "ignore the one I named". + it('rejects a named-but-missing artifact even when requireArtifact is false', async () => { + const missing = join(tmpdir(), `os-named-missing-empty-${process.pid}`, 'objectstack.json'); + delete process.env.OS_ARTIFACT_PATH; + await expect( + createDefaultHostConfig({ requireArtifact: false, artifactPath: missing }), + ).rejects.toThrow(/does not exist/); + }); + + // The control: nothing NAMED an artifact, so the conventional path being + // absent keeps its own pre-existing message — this guard must not swallow it. + it('keeps the "no artifact source" error when nothing named one', async () => { + delete process.env.OS_ARTIFACT_PATH; + const emptyDir = mkdtempSync(join(tmpdir(), 'os-no-artifact-source-')); + const cwd = process.cwd(); + process.chdir(emptyDir); + try { + await expect(createDefaultHostConfig({ requireArtifact: true })).rejects.toThrow( + /No artifact source available/, + ); + } finally { + process.chdir(cwd); + rmSync(emptyDir, { recursive: true, force: true }); + } + }); + + // A remote artifact is never stat'ed — a URL cannot be cheaply checked, and + // the loader owns that failure. + it('passes an http(s) artifact source through without a filesystem check', () => { + delete process.env.OS_ARTIFACT_PATH; + expect(resolveDefaultArtifactPath('https://example.com/objectstack.json')).toBe( + 'https://example.com/objectstack.json', + ); + }); }); diff --git a/packages/runtime/src/standalone-stack.ts b/packages/runtime/src/standalone-stack.ts index ee2ac06d27..2282ecac8b 100644 --- a/packages/runtime/src/standalone-stack.ts +++ b/packages/runtime/src/standalone-stack.ts @@ -306,9 +306,24 @@ export async function createStandaloneStack(config?: StandaloneStackConfig): Pro stampSearchPinyinEnabled(artifactBundle?.i18n); const plugins: any[] = [ - // MUST precede ObjectQLPlugin: its start() connects the default driver - // through the datasource connection service, and ObjectQLPlugin.start() - // runs boot schema-sync right after — the driver has to exist by then. + // This position buys NOTHING, and reading it as an ordering guarantee is + // how #4085 happened. The kernel resolves both init and start order from + // the plugin dependency graph, and `DefaultDatasourcePlugin` declares a + // hard dependency on ObjectQL — which HOISTS ObjectQLPlugin ahead of it + // (measured on a serve boot: `com.objectstack.engine.objectql` inits + // 6 slots BEFORE `com.objectstack.runtime.default-datasource`), the exact + // opposite of what this slot looks like it is asking for. + // + // What actually makes the driver exist before boot schema-sync is the + // PHASE split, not this list: the datasource plugin connects in `init()` + // (Phase 1 completes before ANY `start()` runs) and depends on ObjectQL + // so ITS init registers the engine first. That contract, and the earlier + // no-tables boot that taught it to us, are documented on the plugin + // itself — see the "Ordering — phase, not list position" note in + // `default-datasource-plugin.ts`. Order requirements belong THERE, next + // to the code the kernel enforces them from; a comment on an array index + // cannot enforce anything. (#4131 tracks making the AppPlugin end of this + // contract enforced rather than conventional.) defaultDatasourcePlugin, new MetadataPlugin({ // Source-file scanner OFF — declarative metadata is loaded