diff --git a/.changeset/20331-validate-view-container-name.md b/.changeset/20331-validate-view-container-name.md index 4c6b9e6bb6a..fc86180226c 100644 --- a/.changeset/20331-validate-view-container-name.md +++ b/.changeset/20331-validate-view-container-name.md @@ -29,5 +29,5 @@ derived object key is empty, because the boot registrar skips that entry with a warning and never refuses it. The boot registrar now calls this function. What it refuses, its message and its `VALIDATION_ERROR` / `400` envelope are unchanged. -Not changed: `os build` does not run this check, so it still writes an artifact -carrying such a container, and the server refuses that artifact when it loads it. +`os build` runs the same check as well (#20393, its own entry), so it no longer +writes an artifact carrying such a container. diff --git a/.changeset/20393-build-view-container-name.md b/.changeset/20393-build-view-container-name.md new file mode 100644 index 00000000000..b1c6466232e --- /dev/null +++ b/.changeset/20393-build-view-container-name.md @@ -0,0 +1,24 @@ +--- +'@objectstack/cli': patch +--- + +fix(cli): `os build` / `os compile` refuses a `views:` container whose own `name` disagrees with the object it binds to, and writes no artifact the server would refuse at boot (#20393) + +Clause-②: no + +A view container is registered under the object it binds to. When its own `name` +is set to something else, for example `{ name: 'order_line', object: 'my_app_order_line', list: { … } }`, +the server refuses the whole stack at boot. `os validate` has refused that stack +since #20331, but `os build` still exited `0` and wrote `dist/objectstack.json` +carrying the container, so `os serve` then refused the artifact it was handed. + +`os build` now runs the same check `os validate` runs, right after the schema +check and before anything is written, and prints the message the server prints +at boot. The text form and `--json` both exit `1`, and no artifact is written. The +`--json` failure payload is `{ success: false, errors, warnings, conversions }`, +with one `errors` entry per refused container: `path` (for example `views[0]`, or +`packages[1].manifest.views[0]` in a multi-package stack), `code: 'VALIDATION_ERROR'`, +`httpStatus: 400` and `message`, the same rows `os validate --json` reports. A stack +the server accepts builds exactly as before, with the same output. + +**Fix:** remove the container's `name`, or set it to the object name the message names. diff --git a/packages/cli/src/commands/compile.ts b/packages/cli/src/commands/compile.ts index ed964783ea4..6b4b887197c 100644 --- a/packages/cli/src/commands/compile.ts +++ b/packages/cli/src/commands/compile.ts @@ -59,6 +59,9 @@ import { formatPermissionSetNameCollisions, } from '../utils/permission-set-name-collisions.js'; import type { PermissionSetNameCollisionDiagnostic } from '@objectstack/plugin-security'; +// [#20393] The boot registrar's divergent view-container `name` refusal — the +// walk `os validate` step 2c runs, over `@objectstack/objectql`'s one judge. +import { findViewContainerNameRefusals } from '../utils/view-container-names.js'; export default class Compile extends Command { static override description = 'Compile ObjectStack configuration to JSON artifact'; @@ -378,6 +381,48 @@ export default class Compile extends Command { this.exit(1); } + // 3a. [#20393] The boot registrar's divergent view-container `name` + // refusal — the SAME walk `os validate` runs at its step 2c, over the + // same judge (`viewContainerNameRefusal`, `@objectstack/objectql`) + // `ObjectQL.registerMetadataCollections` throws the answer of. This + // door used to exit 0 on `{ name: 'order_line', object: + // 'my_app_order_line', list: {…} }` and WRITE an artifact carrying + // it, which `os serve` then refused at boot — the command that ships + // shipping the failure. + // + // ⛔ One judge, not a second rule, and not a second walk: the call is + // the one `validate.ts` makes, so the two doors cannot disagree about + // which `views:` entries boot registers or under which package id, + // and the message is the runtime's own, verbatim. + // + // The input is `result.data`, the parsed stack this command + // serializes: every `views:` entry the artifact carries — top level, + // or each `packages[i].manifest` body — is the one judged here + // (step 4 adds docs and `runtimeModule`, never a view). So the + // verdict is the one boot reaches on the artifact. + // + // Right after the parse, ahead of the rule table and of every + // artifact write, mirroring `os validate`: this is the runtime's own + // accept set, the same class as the schema. The `--json` face is the + // schema exit's envelope just above (`errors`, as `os validate --json` + // carries these rows); the text face is `os validate`'s. No step + // line, as on `os validate`: a passing build prints what it printed. + const containerNameRefusals = findViewContainerNameRefusals(result.data as Record); + if (containerNameRefusals.length > 0) { + if (flags.json) { + await emitJson({ success: false, errors: containerNameRefusals, warnings: warningsSoFar(), conversions: conversionNotices }, 0, { compact: true }); + this.exit(1); + } + const n = containerNameRefusals.length; + console.log(''); + printError(`The server would refuse this stack at boot (${n} view container${n > 1 ? 's' : ''})`); + printBulletList( + containerNameRefusals.map((r) => r.message), + { noun: 'view-container refusal(s)', remedy: JSON_FULL_LIST_REMEDY }, + ); + this.exit(1); + } + // 3b. The author-time rule registry (#4409) — one table, three commands. // `os build` was the WEAKEST of the three authoring gates before it: // it published stacks `os validate` or `os lint` refuse, because the diff --git a/packages/cli/src/commands/validate.ts b/packages/cli/src/commands/validate.ts index e30ff085b91..22336f90f41 100644 --- a/packages/cli/src/commands/validate.ts +++ b/packages/cli/src/commands/validate.ts @@ -350,8 +350,8 @@ export default class Validate extends Command { // Right after the parse, ahead of the rule table: this is the // runtime's own accept set, the same class as the schema, and // nothing below it is worth reading about a stack the server will - // not load. `os build` does not run it (see the ledger row in - // `test/validate-build-gate-parity.test.ts`). + // not load. `os build` runs the same call at its step 3a (#20393); + // `test/validate-build-gate-parity.test.ts` holds both doors to it. const containerNameRefusals = findViewContainerNameRefusals(result.data as Record); if (containerNameRefusals.length > 0) { if (flags.json) { diff --git a/packages/cli/test/build-view-container-name.test.ts b/packages/cli/test/build-view-container-name.test.ts new file mode 100644 index 00000000000..97980a3f112 --- /dev/null +++ b/packages/cli/test/build-view-container-name.test.ts @@ -0,0 +1,251 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#20393] `os build` refuses a `views:` container whose own `name` disagrees + * with the object key it binds to — the stack `os serve` refuses at boot — + * says so in the boot registrar's own words, and writes NO artifact. + * + * ## The defect, measured before the fix + * + * On an `os init -t app` project with a view `{ name: 'order_line', object: + * 'my_app_order_line', list: {…} }`, `os build` exited 0 and wrote + * `dist/objectstack.json` carrying that container; `os serve`, booting that + * artifact with no config, exited 1 with "Invalid `views:` container from + * manifest 'com.example.my-app': the container's own `name` is 'order_line', + * which disagrees with the object key it binds to, 'my_app_order_line' …". + * `os validate` had refused the same stack since #20331; the build — the door + * that SHIPS — did not, so it shipped the failure. + * + * ## One judge, one walk + * + * `compile.ts` makes the call `validate.ts` makes: `findViewContainerNameRefusals` + * over the parsed stack, which hands each entry to `@objectstack/objectql`'s + * `viewContainerNameRefusal` — the function the boot loop throws the answer of. + * So the pins below assert EQUALITY with the message the boot registrar + * actually throws for the same payload, driven through `ObjectQL.registerApp`, + * rather than a literal copy of the words. + * + * The CLI runs through `bin/run-dev.js` (source, via tsx); its dependencies — + * `@objectstack/objectql` among them — resolve through `exports` to `dist/`. + */ + +import { describe, it, expect, beforeAll, afterAll } from 'vitest'; +import { execFile } from 'node:child_process'; +import { existsSync, mkdtempSync, readFileSync, rmSync, writeFileSync, mkdirSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join, resolve } from 'node:path'; +import { fileURLToPath } from 'node:url'; +import { ObjectQL } from '@objectstack/objectql'; +import { childEnv } from './helpers/serve-process.js'; + +const HERE = resolve(fileURLToPath(import.meta.url), '..'); +const CLI = resolve(HERE, '../bin/run-dev.js'); +const TSX = resolve(HERE, '../../../node_modules/.bin/tsx'); +/** A cold `tsx` spawn of the CLI source entry runs well past vitest's 5 s default. */ +const SPAWN_TIMEOUT_MS = 120_000; +/** `os build`'s default `--output`, relative to the working directory. */ +const ARTIFACT = join('dist', 'objectstack.json'); + +interface Run { + code: number; + stdout: string; + stderr: string; +} + +function runCli(args: string[], cwd: string): Promise { + return new Promise((resolvePromise) => { + execFile( + TSX, + [CLI, ...args], + { cwd, maxBuffer: 16 * 1024 * 1024, env: childEnv({ NO_COLOR: '1' }) }, + (err, stdout, stderr) => { + resolvePromise({ + code: err ? (typeof (err as { code?: unknown }).code === 'number' ? (err as unknown as { code: number }).code : 1) : 0, + stdout: String(stdout), + stderr: String(stderr), + }); + }, + ); + }); +} + +function payloadOf(run: Run, label: string): Record { + try { + return JSON.parse(run.stdout) as Record; + } catch { + throw new Error(`${label}: stdout was not one JSON document (exit ${run.code})\n${run.stdout}\n${run.stderr}`); + } +} + +const NS = 'bvcn'; +const ID = `com.example.${NS}`; +const OBJECT = `${NS}_order_line`; + +const orderLineObject = (name: string) => ({ + name, + label: 'Order Line', + sharingModel: 'private', + fields: { name: { type: 'text', label: 'Name' } }, +}); + +const container = (object: string, viewName: string | undefined) => ({ + ...(viewName === undefined ? {} : { name: viewName }), + label: 'Order Line', + object, + list: { type: 'grid', columns: [{ field: 'name' }] }, +}); + +/** A one-package stack, as data — written to disk as the config AND handed to boot. */ +function stack(viewName: string | undefined): Record { + return { + manifest: { id: ID, name: NS, version: '1.0.0', type: 'app', namespace: NS }, + objects: [orderLineObject(OBJECT)], + views: [container(OBJECT, viewName)], + }; +} + +/** + * A `packages[]` stack (ADR-0130 D4): the load path registers each body under + * its own id and never the top level. The divergent container sits in the + * SECOND body; the first carries a matching one, which must not be reported. + */ +const CORE_ID = 'com.example.bvcn-core'; +const ORDERS_ID = 'com.example.bvcn-orders'; +const ordersBody = { + id: ORDERS_ID, + name: 'bvcn_orders', + version: '1.0.0', + type: 'app', + objects: [orderLineObject('bvcn_orders_line')], + views: [container('bvcn_orders_line', 'order_line')], +}; +function packagesStack(): Record { + return { + packages: [ + { + manifest: { + id: CORE_ID, + name: 'bvcn_core', + version: '1.0.0', + type: 'app', + objects: [orderLineObject('bvcn_core_line')], + views: [container('bvcn_core_line', 'bvcn_core_line')], + }, + }, + { manifest: ordersBody }, + ], + }; +} + +/** + * What the boot registrar throws for a payload, driven the way the load path + * drives it: one body into `registerApp`. + */ +function bootRefusal(payload: Record): any { + try { + new ObjectQL().registerApp(payload); + } catch (e) { + return e; + } + return undefined; +} + +/** A one-package stack (or its artifact) as `AppPlugin` hands it: `{ ...manifest, ...stack }`. */ +const asBootPayload = (s: Record) => ({ ...(s.manifest as Record), ...s }); + +const dirs: Record = {}; +let root = ''; + +beforeAll(() => { + root = mkdtempSync(join(tmpdir(), 'os-build-view-container-name-')); + const make = (label: string, s: Record) => { + const dir = join(root, label); + mkdirSync(dir, { recursive: true }); + writeFileSync(join(dir, 'objectstack.config.ts'), `export default ${JSON.stringify(s, null, 2)};\n`); + dirs[label] = dir; + }; + make('divergent-json', stack('order_line')); + make('divergent-text', stack('order_line')); + make('divergent-packages', packagesStack()); + make('matching', stack(OBJECT)); + make('anonymous', stack(undefined)); +}); + +afterAll(() => { + if (root) rmSync(root, { recursive: true, force: true }); +}); + +describe('#20393 — os build refuses what the boot registrar refuses, in its words, and ships nothing', () => { + it('premise: the boot registrar refuses both divergent payloads, and accepts both controls', () => { + // Without this, the pins below could agree with a boot loop that had + // stopped refusing anything. + const refusal = bootRefusal(asBootPayload(stack('order_line'))); + expect(refusal).toBeInstanceOf(Error); + expect(refusal.code).toBe('VALIDATION_ERROR'); + expect(bootRefusal(ordersBody)).toBeInstanceOf(Error); + expect(bootRefusal(asBootPayload(stack(OBJECT)))).toBeUndefined(); + expect(bootRefusal(asBootPayload(stack(undefined)))).toBeUndefined(); + }); + + it('THE PIN: --json exits 1, reports the boot registrar\'s refusal verbatim, and writes no artifact', async () => { + const dir = dirs['divergent-json']; + const run = await runCli(['build', '--json'], dir); + const payload = payloadOf(run, 'divergent --json'); + expect(run.code).toBe(1); + expect(payload.success).toBe(false); + expect(Array.isArray(payload.errors)).toBe(true); + expect(payload.errors).toHaveLength(1); + + const boot = bootRefusal(asBootPayload(stack('order_line'))); + const [row] = payload.errors; + expect(row.message).toBe(boot.message); + // The envelope boot throws with, carried onto the row. + expect(row.code).toBe(boot.code); + expect(row.httpStatus).toBe(boot.httpStatus); + expect(row.path).toBe('views[0]'); + // The point of the card: the door that ships ships nothing. + expect(existsSync(join(dir, ARTIFACT)), 'os build wrote an artifact the server refuses at boot').toBe(false); + }, SPAWN_TIMEOUT_MS); + + it('the text face exits 1, prints the same words, and writes no artifact', async () => { + const dir = dirs['divergent-text']; + const run = await runCli(['build'], dir); + expect(run.code).toBe(1); + expect(run.stdout).not.toContain('Build complete'); + expect(run.stdout).toContain(bootRefusal(asBootPayload(stack('order_line'))).message); + expect(existsSync(join(dir, ARTIFACT))).toBe(false); + }, SPAWN_TIMEOUT_MS); + + it('a `packages[]` stack: the body boot refuses is refused under its own id, and the matching one is not', async () => { + const dir = dirs['divergent-packages']; + const run = await runCli(['build', '--json'], dir); + const payload = payloadOf(run, 'packages --json'); + expect(run.code).toBe(1); + expect(payload.success).toBe(false); + expect(payload.errors).toHaveLength(1); + const [row] = payload.errors; + expect(row.path).toBe('packages[1].manifest.views[0]'); + expect(row.message).toBe(bootRefusal(ordersBody).message); + expect(row.message).toContain(`from manifest '${ORDERS_ID}'`); + expect(existsSync(join(dir, ARTIFACT))).toBe(false); + }, SPAWN_TIMEOUT_MS); + + it('CONTROL: a container whose `name` matches its object builds, and the artifact it writes boots', async () => { + const dir = dirs.matching; + const run = await runCli(['build', '--json'], dir); + const payload = payloadOf(run, 'matching --json'); + expect(payload.success, JSON.stringify(payload.errors ?? payload.error)).toBe(true); + expect(run.code).toBe(0); + const artifact = JSON.parse(readFileSync(join(dir, ARTIFACT), 'utf8')) as Record; + expect(bootRefusal(asBootPayload(artifact))).toBeUndefined(); + }, SPAWN_TIMEOUT_MS); + + it('CONTROL: a container with no `name` builds — the shape boot also accepts', async () => { + const dir = dirs.anonymous; + const run = await runCli(['build', '--json'], dir); + const payload = payloadOf(run, 'anonymous --json'); + expect(payload.success, JSON.stringify(payload.errors ?? payload.error)).toBe(true); + expect(run.code).toBe(0); + expect(existsSync(join(dir, ARTIFACT))).toBe(true); + }, SPAWN_TIMEOUT_MS); +}); diff --git a/packages/cli/test/validate-build-gate-parity.test.ts b/packages/cli/test/validate-build-gate-parity.test.ts index 7e29dbd6505..7c0fb907c37 100644 --- a/packages/cli/test/validate-build-gate-parity.test.ts +++ b/packages/cli/test/validate-build-gate-parity.test.ts @@ -123,6 +123,21 @@ const SHARED_NON_REGISTRY_GATES: readonly string[] = [ // `kind:'html'` page to check. Not a registry rule: the manifest is a file // in the working directory, not part of the stack a rule is handed. 'resolveJsxGateManifest', + // [#20331, #20393] The boot registrar's divergent view-container `name` + // refusal (`viewContainerNameRefusal`, @objectstack/objectql), judged at + // author time by the same function boot throws the answer of. Not a registry + // rule: the WALK decides which `views:` entries boot registers and under + // which package id — the top level under the manifest's id, or each + // `packages[i].manifest` body under its own — which is the artifact's + // package reading, not the one stack a rule is handed. + // + // ⭐ This row is the #20331 VALIDATE_ONLY_GATES entry CLOSED. That entry read + // "`os build` still emits an artifact carrying such a container, which the + // runtime refuses when it loads it", and it was right. `compile.ts` now makes + // the same call (#20393), so the row moved here, where both doors are held to + // it — deleted there rather than reworded, for the reason the + // `runPerPackageAuthoringRules` row above gives. + 'findViewContainerNameRefusals', ]; /** @@ -159,14 +174,14 @@ const BUILD_ONLY_GATES: Readonly> = { * validate-only` below: a row whose gate `validate.ts` no longer calls is * stale, and a row whose gate `compile.ts` now calls too belongs in * SHARED_NON_REGISTRY_GATES instead. + * + * EMPTY is this ledger's steady state: every row is a gap the build carries. + * Its one row, `findViewContainerNameRefusals` (#20331), moved to + * SHARED_NON_REGISTRY_GATES when `compile.ts` gained the call (#20393). The + * ledger stays, empty, because it is the only honest place the closed roster + * has for the next validate-only gate. */ -const VALIDATE_ONLY_GATES: Readonly> = { - findViewContainerNameRefusals: - '[#20331] The boot registrar\'s divergent view-container `name` refusal ' + - '(`viewContainerNameRefusal`, @objectstack/objectql), judged at author time by the same function ' + - 'boot throws the answer of. Wired into validate.ts only, by that card\'s scope: `os build` still ' + - 'emits an artifact carrying such a container, which the runtime refuses when it loads it.', -}; +const VALIDATE_ONLY_GATES: Readonly> = {}; /** * Everything else the two commands call, and the reason each one is NOT an