Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 45 additions & 0 deletions .changeset/cli-option-b-config-load-boundaries.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
---
"@objectstack/cli": patch
---

fix(cli): `os serve` / `os dev` / `os build` / `os migrate` resolve `packages[]` when a stack carries no flattened top level

The CLI holds four independent config-load boundaries, and every read of a
package-owned collection behind them was an inline expression against the
FLATTENED top level. A multi-package stack that carries each definition once
under `packages[]` — the shape ADR-0130 D4's option B produces — reached those
expressions with the key simply absent, and nothing threw:

- `os serve` / `os dev` auto-register the ObjectQL engine and the storage driver
when the stack declares objects. Both gates read `config.objects`, so the app
booted with **no query engine and no storage driver** and reported healthy.
Nothing between the artifact and the gate could notice: the standalone stack
omits the `objects` key entirely when the array is absent rather than setting
`[]`, and the boot-config merge is a plain spread.
- `os serve` auto-registers the i18n service plugin when the stack carries
translations. `translations` is package-owned while `i18n` is an envelope key a
translations-only stack never sets, so the REST i18n routes silently did not
exist.
- `os dev` diffs the artifact's object inventory across recompiles to name a
newly added `*.object.ts`. It went permanently empty, so every recompile read
as all-green.
- `os build` runs the author-time rule table twice — once over the union, once
per package. The per-package run already read `packages[]`; the union run,
which is the only one of the two that can see a finding spanning packages,
judged an empty stack and published green.

All of them now resolve through one seam, in the dependency-topological order
`resolveArtifactPackageOrder` gives. Each answer starts from the expression it
replaced, so every stack that boots or builds today takes the identical branch —
including a stack declaring an empty `objects: []`, which stays a stack that gets
an engine — and `packages[]` is consulted only where the old read returned
nothing. A malformed `packages` list is refused with its ADR-0112 envelope on
that leg instead of resolving to the silent empty.

The predicate `os serve` and `os migrate` each carried their own copy of — "does
this config carry app metadata that needs an `AppPlugin` wrap" — is now one
function. Measured, it does not lose under the new shape; it is folded in because
it is the master gate for everything `AppPlugin` then reads.

No command emits anything different: the compiled artifact still carries both
copies, and the folded stack is a rule INPUT that reaches no writer.
20 changes: 18 additions & 2 deletions packages/cli/src/commands/compile.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import {
} from '@objectstack/spec';
import { loadConfig } from '../utils/config.js';
import { lowerCallables } from '../utils/lower-callables.js';
import { authoringRuleUnionStack } from '../utils/stack-collections.js';
import { buildAccessMatrix, diffAccessMatrix } from '@objectstack/lint';
import { runAuthoringRules, splitBySeverity, authoringRulesFor } from '@objectstack/lint';
import { resolveSduiManifest } from '../utils/sdui-manifest.js';
Expand Down Expand Up @@ -340,9 +341,24 @@ export default class Compile extends Command {
// is declared in `lint/authoring-rules.ts`. Do not add a call site here.
const registered = authoringRulesFor('build');
if (!flags.json) printStep(`Running author-time rules (${registered.length})...`);
// [ADR-0130 D4 / option B, #15006] The UNION run judges the flattened
// top level. Under option B that top level is gone — `packages[]`
// carries every definition once — so this run's input would be an
// EMPTY stack and `os build` would publish green having judged
// nothing. `authoringRuleUnionStack` folds each absent collection
// back in from `packages[]`, in `resolveArtifactPackageOrder`'s
// dependency order. It changes what the rules JUDGE and nothing this
// command EMITS: the artifact is written from `lowering.lowered` /
// `result.data`, which this does not touch, and a stack that still
// carries its collections is returned by identity.
//
// The per-package run below needs no such fold — it already reads
// `packages[]`. The union run is the only one of the two that can see
// a finding spanning packages, which is exactly what an empty input
// silently stops reporting.
const findings = runAuthoringRules('build', {
normalized: normalized as Record<string, unknown>,
parsed: result.data as Record<string, unknown>,
normalized: authoringRuleUnionStack(normalized as Record<string, unknown>),
parsed: authoringRuleUnionStack(result.data as Record<string, unknown>),
sduiManifest: resolveSduiManifest(),
});
const { errors: ruleErrors, advisories } = splitBySeverity(findings);
Expand Down
17 changes: 9 additions & 8 deletions packages/cli/src/commands/dev.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import {
formatMtimeGap,
} from '../utils/dev-restart.js';
import { childEnvWithResolvedArtifact } from '../utils/internal-artifact-channel.js';
import { artifactObjectNames } from '../utils/stack-collections.js';
import { readEnvWithDeprecation, isMcpServerEnabled } from '@objectstack/types';
// The ONE port contract, shared with `start` and with the `serve` child this
// command spawns (#12673). ⛔ Nothing about ports is declared in this file —
Expand Down Expand Up @@ -650,16 +651,16 @@ export default class Dev extends Command {
// newly added *.object.ts is called out explicitly (15.1 third-party
// eval: "recompiled" alone read as all-green while the new object's
// table/seed sync was invisible to the user).
// [ADR-0130 D4 / option B, #15006] The envelope unwrap and the object
// read are `artifactObjectNames` — one of this package's four reads of a
// PACKAGE-OWNED collection, and the only one whose loss is non-fatal: with
// the flattened top level gone this inventory went permanently EMPTY, so
// `os dev` stopped naming a newly added *.object.ts and every recompile
// read as all-green. The file read and the `null`-on-failure contract stay
// here; the seam is what the acceptance probe can call.
const readArtifactObjects = (): Set<string> | null => {
try {
const raw = JSON.parse(fs.readFileSync(opts.artifactPath, 'utf8'));
const meta = raw?.metadata ?? raw?.data?.metadata ?? raw;
const objects = Array.isArray(meta?.objects) ? meta.objects : [];
return new Set(
objects
.map((o: any) => o?.name)
.filter((n: any): n is string => typeof n === 'string'),
);
return new Set(artifactObjectNames(JSON.parse(fs.readFileSync(opts.artifactPath, 'utf8'))));
} catch {
return null;
}
Expand Down
54 changes: 31 additions & 23 deletions packages/cli/src/commands/serve.ts
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,17 @@ import {
} from '../utils/port-contract.js';
import { BootLogCapture, isVerboseBootLevel } from '../utils/boot-log-capture.js';
import { graftAuthoredRuntimeMembers, isAppPluginLike } from '../utils/graft-runtime-hooks.js';
// [ADR-0130 D4 / option B, #15006] Every read below that keys off a
// PACKAGE-OWNED collection goes through this seam, so an option-B artifact
// (flattened top level gone, `packages[]` carrying everything once) reaches
// the same decision — and so the acceptance probe can CALL the decision
// instead of re-implementing it.
import {
shouldAutoRegisterObjectQL,
shouldAutoRegisterStorageDriver,
stackDeclaresMetadata,
bundleDeclaresTranslations,
} from '../utils/stack-collections.js';
import { redactConnectionUrl, describeDriverConnection } from '../utils/connection-display.js';
// The posture prose `os serve` and `os doctor` BOTH print, declared once
// (#12492) — and, since #12579, the multi-org runtime SPELLING those two
Expand Down Expand Up @@ -2628,8 +2639,12 @@ export default class Serve extends Command {
}

// 1. Auto-register ObjectQL Plugin if objects define but plugins missing
const hasObjectQL = plugins.some((p: any) => p.name?.includes('objectql') || p.constructor?.name?.includes('ObjectQL'));
if (config.objects && !hasObjectQL) {
// [#15006] The whole gate — the `objects` read AND the already-composed
// check — is `shouldAutoRegisterObjectQL`. It answers exactly as the two
// inline expressions did for every stack that boots today, and resolves
// `packages[]` when the flattened top level is absent, which is the shape
// that used to boot with NO QUERY ENGINE and throw nothing.
if (shouldAutoRegisterObjectQL(config, plugins)) {
try {
const { ObjectQLPlugin } = await import('@objectstack/objectql');
await kernel.use(new ObjectQLPlugin());
Expand Down Expand Up @@ -2661,12 +2676,9 @@ export default class Serve extends Command {
// at boot through the datasource connection service, so building a
// storage driver here would construct a duplicate pool the engine then
// discards as already-registered.
const hasDriver = plugins.some((p: any) =>
p.name?.includes('driver') ||
p.constructor?.name?.includes('Driver') ||
p.name === 'com.objectstack.runtime.default-datasource' ||
p.constructor?.name === 'DefaultDatasourcePlugin');
if (!hasDriver && config.objects) {
// [#15006] Same seam, same reason — see the ObjectQL gate above. The
// driver-provider duck-typing moved into it with the read it guards.
if (shouldAutoRegisterStorageDriver(config, plugins)) {
const databaseUrl = process.env.OS_DATABASE_URL;
const driverType = resolveDriverType(process.env.OS_DATABASE_DRIVER, databaseUrl);
// libSQL/Turso's credential is the only one that does NOT ride inside the
Expand Down Expand Up @@ -2799,9 +2811,10 @@ export default class Serve extends Command {
// already holds an AppPlugin instance — and never on a named app, so it
// is checked structurally below.
const hasAppPluginAlready = plugins.some(isAppPluginLike);
const configHasMetadata = !!(
config.objects || config.manifest || config.apps || config.flows || config.apis
);
// [#15006] The same predicate `schema-migration-plugins.ts` runs after its
// own second `loadConfig` (B4) — folded into one seam rather than left as
// two copies whose comment already said they were the same.
const configHasMetadata = stackDeclaresMetadata(config);

// ── Decide the dev-only artifact door BEFORE the wrap (#14397) ────
// On a HOST config `os dev` composes TWO writers over ONE stack: the
Expand Down Expand Up @@ -2973,24 +2986,19 @@ export default class Serve extends Command {
// `plugins` array — a host/aggregator config may define no translations
// of its own and instead compose several `new AppPlugin(...)` entries,
// each carrying its own. Keyed on that shape, not on a named app.
const pluginBundleHasTranslations = (bundle: any): boolean => {
if (!bundle || typeof bundle !== 'object') return false;
if (Array.isArray(bundle.translations) && bundle.translations.length > 0) return true;
if (bundle.i18n) return true;
if (bundle.manifest && (
(Array.isArray(bundle.manifest.translations) && bundle.manifest.translations.length > 0)
|| bundle.manifest.i18n
)) return true;
return false;
};
// [#15006] `bundleDeclaresTranslations` is that same shape-keyed check plus
// the `packages[]` leg: `translations` is package-owned and `i18n` is an
// envelope key a translations-only stack never sets, so an option-B artifact
// reached this gate with neither and the REST i18n routes silently did not
// exist. MEASURED on the acceptance probe, not inferred.
const anyAppPluginHasTranslations = plugins.some((p: any) => {
if (!p) return false;
// AppPlugin instances expose their bundle on `.bundle`
if (p.bundle && pluginBundleHasTranslations(p.bundle)) return true;
if (p.bundle && bundleDeclaresTranslations(p.bundle)) return true;
return false;
});
const configHasTranslations = (
pluginBundleHasTranslations(config)
bundleDeclaresTranslations(config)
|| anyAppPluginHasTranslations
);
if (!hasI18nPlugin && configHasTranslations && tierEnabled('i18n')) {
Expand Down
9 changes: 6 additions & 3 deletions packages/cli/src/utils/schema-migration-plugins.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import path from 'node:path';
import fs from 'node:fs';
import { isAppPluginLike } from './graft-runtime-hooks.js';
import { stackDeclaresMetadata } from './stack-collections.js';

/**
* The object set a SCHEMA migration is planned against (#12938).
Expand Down Expand Up @@ -1091,9 +1092,11 @@ export async function buildSchemaMigrationPlugins(opts: {
// top-level metadata needs the wrap, or its `objects` never reach the
// registry and this composition would report a set smaller than the one
// the deployment serves.
const configHasMetadata = !!(
config?.objects || config?.manifest || config?.apps || config?.flows || config?.apis
);
// [#15006] `stackDeclaresMetadata` — the SAME seam `serve.ts` step 3 now
// calls, which is what the comment above already asserted about these two
// copies. B4 is this command's OWN second `loadConfig`, not behind B2, so
// it had to be reached separately.
const configHasMetadata = stackDeclaresMetadata(config);
const appAlready = hasArtifactApp || hostPlugins.some(isAppPluginLike);
if (configHasMetadata && !appAlready) {
const { AppPlugin } = await import('@objectstack/runtime');
Expand Down
Loading
Loading