Skip to content

Commit 1226f2e

Browse files
hotlongclaude
andauthored
fix(cli): resolve packages[] at the four config-load boundaries and the gates that key off config.objects (#15228)
* fix(cli): resolve `packages[]` at the four config-load boundaries and the gates that key off `config.objects` 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 stack carrying each definition once under `packages[]` reached those expressions with the key absent, and nothing threw: `os serve` booted with no query engine and no storage driver, its i18n plugin never registered, `os dev`'s recompile inventory went permanently empty, and `os build`'s union author-time rule run judged an empty stack and published green. One seam (`utils/stack-collections.ts`) now answers all of them, in `resolveArtifactPackageOrder`'s dependency order. Each predicate starts from the expression it replaced, so every stack that boots or builds today takes the identical branch — including one declaring an empty `objects: []` — and `packages[]` is consulted only where the old read returned nothing. Being callable is the other half of the point: the #15004 acceptance probe could not reach an expression inside an oclif command body, which is why these four sites sat outside its ledger. The probe now carries a row per site that calls the decision each command makes. No command emits anything different. `OPTION_B_LOSSES` is unchanged at 24 rows: none of them names a cli reader. Part of #15006 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UHvF5hyiZjnCyExFnfQB8m * docs(cli): name the filed issue for the one option-B site this card leaves alone `plugins` / `devPlugins` are package-owned collections that `serve.ts` and `schema-migration-plugins.ts` read off the top level only, and the mechanical repair is wrong there: `packages[i].manifest.plugins` in a JSON artifact is inert data where `kernel.use()` needs a live instance. Whether a live-object collection belongs in the package-owned key set at all is a `packages/spec` question upstream of every reader in this program, so the pin's header points at #15219 instead of at this card's report. Part of #15006 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UHvF5hyiZjnCyExFnfQB8m * perf(cli): resolve the package list once per caller, not once per collection key `resolveArtifactPackageOrder` parses every `packages[]` entry whole, and the `os build` union fold asks about all 37 package-owned collections — so the first cut ran that parse 38 times per build, and the `AppPlugin` wrap gate ran it four times per boot. The bodies are resolved once and handed to `collectFrom`, which is the only shape that changed; every answer is identical. Also corrects the probe's note: six rows enter, not four. The two beyond the card's named sites are the reads sitting beside them on the same boundaries — the `AppPlugin` wrap gate, which measured does NOT lose, and the i18n auto-registration gate, which does. Part of #15006 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UHvF5hyiZjnCyExFnfQB8m --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 9690d11 commit 1226f2e

9 files changed

Lines changed: 875 additions & 56 deletions
Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
---
2+
"@objectstack/cli": patch
3+
---
4+
5+
fix(cli): `os serve` / `os dev` / `os build` / `os migrate` resolve `packages[]` when a stack carries no flattened top level
6+
7+
The CLI holds four independent config-load boundaries, and every read of a
8+
package-owned collection behind them was an inline expression against the
9+
FLATTENED top level. A multi-package stack that carries each definition once
10+
under `packages[]` — the shape ADR-0130 D4's option B produces — reached those
11+
expressions with the key simply absent, and nothing threw:
12+
13+
- `os serve` / `os dev` auto-register the ObjectQL engine and the storage driver
14+
when the stack declares objects. Both gates read `config.objects`, so the app
15+
booted with **no query engine and no storage driver** and reported healthy.
16+
Nothing between the artifact and the gate could notice: the standalone stack
17+
omits the `objects` key entirely when the array is absent rather than setting
18+
`[]`, and the boot-config merge is a plain spread.
19+
- `os serve` auto-registers the i18n service plugin when the stack carries
20+
translations. `translations` is package-owned while `i18n` is an envelope key a
21+
translations-only stack never sets, so the REST i18n routes silently did not
22+
exist.
23+
- `os dev` diffs the artifact's object inventory across recompiles to name a
24+
newly added `*.object.ts`. It went permanently empty, so every recompile read
25+
as all-green.
26+
- `os build` runs the author-time rule table twice — once over the union, once
27+
per package. The per-package run already read `packages[]`; the union run,
28+
which is the only one of the two that can see a finding spanning packages,
29+
judged an empty stack and published green.
30+
31+
All of them now resolve through one seam, in the dependency-topological order
32+
`resolveArtifactPackageOrder` gives. Each answer starts from the expression it
33+
replaced, so every stack that boots or builds today takes the identical branch —
34+
including a stack declaring an empty `objects: []`, which stays a stack that gets
35+
an engine — and `packages[]` is consulted only where the old read returned
36+
nothing. A malformed `packages` list is refused with its ADR-0112 envelope on
37+
that leg instead of resolving to the silent empty.
38+
39+
The predicate `os serve` and `os migrate` each carried their own copy of — "does
40+
this config carry app metadata that needs an `AppPlugin` wrap" — is now one
41+
function. Measured, it does not lose under the new shape; it is folded in because
42+
it is the master gate for everything `AppPlugin` then reads.
43+
44+
No command emits anything different: the compiled artifact still carries both
45+
copies, and the folded stack is a rule INPUT that reaches no writer.

packages/cli/src/commands/compile.ts

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import {
1515
} from '@objectstack/spec';
1616
import { loadConfig } from '../utils/config.js';
1717
import { lowerCallables } from '../utils/lower-callables.js';
18+
import { authoringRuleUnionStack } from '../utils/stack-collections.js';
1819
import { buildAccessMatrix, diffAccessMatrix } from '@objectstack/lint';
1920
import { runAuthoringRules, splitBySeverity, authoringRulesFor } from '@objectstack/lint';
2021
import { resolveSduiManifest } from '../utils/sdui-manifest.js';
@@ -356,9 +357,24 @@ export default class Compile extends Command {
356357
// is declared in `lint/authoring-rules.ts`. Do not add a call site here.
357358
const registered = authoringRulesFor('build');
358359
if (!flags.json) printStep(`Running author-time rules (${registered.length})...`);
360+
// [ADR-0130 D4 / option B, #15006] The UNION run judges the flattened
361+
// top level. Under option B that top level is gone — `packages[]`
362+
// carries every definition once — so this run's input would be an
363+
// EMPTY stack and `os build` would publish green having judged
364+
// nothing. `authoringRuleUnionStack` folds each absent collection
365+
// back in from `packages[]`, in `resolveArtifactPackageOrder`'s
366+
// dependency order. It changes what the rules JUDGE and nothing this
367+
// command EMITS: the artifact is written from `lowering.lowered` /
368+
// `result.data`, which this does not touch, and a stack that still
369+
// carries its collections is returned by identity.
370+
//
371+
// The per-package run below needs no such fold — it already reads
372+
// `packages[]`. The union run is the only one of the two that can see
373+
// a finding spanning packages, which is exactly what an empty input
374+
// silently stops reporting.
359375
const findings = runAuthoringRules('build', {
360-
normalized: normalized as Record<string, unknown>,
361-
parsed: result.data as Record<string, unknown>,
376+
normalized: authoringRuleUnionStack(normalized as Record<string, unknown>),
377+
parsed: authoringRuleUnionStack(result.data as Record<string, unknown>),
362378
sduiManifest: resolveSduiManifest(),
363379
});
364380
const { errors: ruleErrors, advisories } = splitBySeverity(findings);

packages/cli/src/commands/dev.ts

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import {
1717
formatMtimeGap,
1818
} from '../utils/dev-restart.js';
1919
import { childEnvWithResolvedArtifact } from '../utils/internal-artifact-channel.js';
20+
import { artifactObjectNames } from '../utils/stack-collections.js';
2021
import { readEnvWithDeprecation, isMcpServerEnabled } from '@objectstack/types';
2122
// The ONE port contract, shared with `start` and with the `serve` child this
2223
// command spawns (#12673). ⛔ Nothing about ports is declared in this file —
@@ -650,16 +651,16 @@ export default class Dev extends Command {
650651
// newly added *.object.ts is called out explicitly (15.1 third-party
651652
// eval: "recompiled" alone read as all-green while the new object's
652653
// table/seed sync was invisible to the user).
654+
// [ADR-0130 D4 / option B, #15006] The envelope unwrap and the object
655+
// read are `artifactObjectNames` — one of this package's four reads of a
656+
// PACKAGE-OWNED collection, and the only one whose loss is non-fatal: with
657+
// the flattened top level gone this inventory went permanently EMPTY, so
658+
// `os dev` stopped naming a newly added *.object.ts and every recompile
659+
// read as all-green. The file read and the `null`-on-failure contract stay
660+
// here; the seam is what the acceptance probe can call.
653661
const readArtifactObjects = (): Set<string> | null => {
654662
try {
655-
const raw = JSON.parse(fs.readFileSync(opts.artifactPath, 'utf8'));
656-
const meta = raw?.metadata ?? raw?.data?.metadata ?? raw;
657-
const objects = Array.isArray(meta?.objects) ? meta.objects : [];
658-
return new Set(
659-
objects
660-
.map((o: any) => o?.name)
661-
.filter((n: any): n is string => typeof n === 'string'),
662-
);
663+
return new Set(artifactObjectNames(JSON.parse(fs.readFileSync(opts.artifactPath, 'utf8'))));
663664
} catch {
664665
return null;
665666
}

packages/cli/src/commands/serve.ts

Lines changed: 31 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,17 @@ import {
6363
} from '../utils/port-contract.js';
6464
import { BootLogCapture, isVerboseBootLevel } from '../utils/boot-log-capture.js';
6565
import { graftAuthoredRuntimeMembers, isAppPluginLike } from '../utils/graft-runtime-hooks.js';
66+
// [ADR-0130 D4 / option B, #15006] Every read below that keys off a
67+
// PACKAGE-OWNED collection goes through this seam, so an option-B artifact
68+
// (flattened top level gone, `packages[]` carrying everything once) reaches
69+
// the same decision — and so the acceptance probe can CALL the decision
70+
// instead of re-implementing it.
71+
import {
72+
shouldAutoRegisterObjectQL,
73+
shouldAutoRegisterStorageDriver,
74+
stackDeclaresMetadata,
75+
bundleDeclaresTranslations,
76+
} from '../utils/stack-collections.js';
6677
import { redactConnectionUrl, describeDriverConnection } from '../utils/connection-display.js';
6778
// The posture prose `os serve` and `os doctor` BOTH print, declared once
6879
// (#12492) — and, since #12579, the multi-org runtime SPELLING those two
@@ -2628,8 +2639,12 @@ export default class Serve extends Command {
26282639
}
26292640

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

28062819
// ── Decide the dev-only artifact door BEFORE the wrap (#14397) ────
28072820
// On a HOST config `os dev` composes TWO writers over ONE stack: the
@@ -2973,24 +2986,19 @@ export default class Serve extends Command {
29732986
// `plugins` array — a host/aggregator config may define no translations
29742987
// of its own and instead compose several `new AppPlugin(...)` entries,
29752988
// each carrying its own. Keyed on that shape, not on a named app.
2976-
const pluginBundleHasTranslations = (bundle: any): boolean => {
2977-
if (!bundle || typeof bundle !== 'object') return false;
2978-
if (Array.isArray(bundle.translations) && bundle.translations.length > 0) return true;
2979-
if (bundle.i18n) return true;
2980-
if (bundle.manifest && (
2981-
(Array.isArray(bundle.manifest.translations) && bundle.manifest.translations.length > 0)
2982-
|| bundle.manifest.i18n
2983-
)) return true;
2984-
return false;
2985-
};
2989+
// [#15006] `bundleDeclaresTranslations` is that same shape-keyed check plus
2990+
// the `packages[]` leg: `translations` is package-owned and `i18n` is an
2991+
// envelope key a translations-only stack never sets, so an option-B artifact
2992+
// reached this gate with neither and the REST i18n routes silently did not
2993+
// exist. MEASURED on the acceptance probe, not inferred.
29862994
const anyAppPluginHasTranslations = plugins.some((p: any) => {
29872995
if (!p) return false;
29882996
// AppPlugin instances expose their bundle on `.bundle`
2989-
if (p.bundle && pluginBundleHasTranslations(p.bundle)) return true;
2997+
if (p.bundle && bundleDeclaresTranslations(p.bundle)) return true;
29902998
return false;
29912999
});
29923000
const configHasTranslations = (
2993-
pluginBundleHasTranslations(config)
3001+
bundleDeclaresTranslations(config)
29943002
|| anyAppPluginHasTranslations
29953003
);
29963004
if (!hasI18nPlugin && configHasTranslations && tierEnabled('i18n')) {

packages/cli/src/utils/schema-migration-plugins.ts

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
import path from 'node:path';
44
import fs from 'node:fs';
55
import { isAppPluginLike } from './graft-runtime-hooks.js';
6+
import { stackDeclaresMetadata } from './stack-collections.js';
67

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

0 commit comments

Comments
 (0)