Skip to content

Commit 5c0de74

Browse files
committed
fix(verify): resolve objects, datasources and positions from packages[] (#15229)
Step 2 of the card's ordered pair: the readers the previous commit ledgered now see a multi-package app, and the four ledger lines are deleted in this commit. `os verify` derives its entire proof set from the app's metadata, so a collection it cannot see is not a missing feature — it is a run that asserts nothing and still prints `✓ verify passed`. Under an option-B artifact `deriveCrudCases` derived ZERO cases and `rlsProbePermissionSet` built an EMPTY probe permission set: the persona that makes an RLS run a probe granted nothing and narrowed nothing, and no persona was minted for any declared position. The four reads go through one resolver (`declaredCollection`), which answers the caller's ORIGINAL expression first and consults `packages[]` only where that came back falsy: - the top level is returned untouched when truthy, so today's additive artifact answers bit-identically and this card is revertible on its own; - `objects: []` is TRUTHY, so a declared-empty collection stays empty — the behaviour a re-expression as `resolve(...).length > 0` would have changed (measured on the sibling card #15006); - package order comes from `resolveArtifactPackageOrder` (`@objectstack/core`, ADR-0130 D4+D5), never from a second traversal of `config.packages`; - a malformed `packages` now raises that function's ADR-0112 refusal instead of reading as "this app declares nothing". Both `deriveCrudCases` reads move together on purpose. Objects alone would leave ADR-0015's double write gate judging against an empty datasource map, which reports a write-opted-in federated object as read-only — a verifier silently skipping an object the app explicitly opted into writes for. The pin's RED before this commit named exactly the four rows to delete: A ledgered subsystem now SEES its collections under option B — the reader program moved forward. Delete these lines from OPTION_B_LOSSES: B2 · verify declaredPositionNames (one RLS persona per declared position) · positions B2 · verify deriveCrudCases (CRUD round-trip case derivation) · objects B2 · verify deriveCrudCases federated write gate (ADR-0015 double opt-in) · datasources B2 · verify rlsProbePermissionSet (RLS probe grants + owner narrowing) · objects `composeStacks`, `packages/spec/src/stack.zod.ts` and what every command emits are untouched: the artifact stays additive through this card. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UHvF5hyiZjnCyExFnfQB8m
1 parent 8583bfb commit 5c0de74

6 files changed

Lines changed: 310 additions & 12 deletions

File tree

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
---
2+
"@objectstack/verify": patch
3+
---
4+
5+
fix(verify): `os verify` no longer reports a green run over a multi-package app it measured nothing about
6+
7+
Every reader in this package took the artifact's **flattened** top level and
8+
nothing else. A multi-package app whose definitions live under `packages[]`
9+
the shape ADR-0130 D4's option B emits — therefore reached `deriveCrudCases`
10+
with no objects and no datasources, and reached `rlsProbePermissionSet` and
11+
`declaredPositionNames` with no objects and no positions. Nothing threw. The run
12+
derived zero CRUD round-trip cases, built an empty RLS probe permission set,
13+
minted no persona for any declared position, and printed `✓ verify passed`.
14+
15+
That is the most expensive place in the platform for a false green: `verify`'s
16+
entire job is to be the thing that notices. A missing collection is at least
17+
missing — zero coverage dressed as a passing run is not.
18+
19+
The four reads now resolve through `resolveArtifactPackageOrder`
20+
(`@objectstack/core`, ADR-0130 D4+D5), **flattened top level first**:
21+
22+
- `deriveCrudCases` — the objects it derives cases for, and the datasource-by-
23+
name map behind ADR-0015's double write gate. Both, because objects alone
24+
would leave a write-opted-in federated object judged against an empty
25+
datasource map and reported read-only, i.e. skipped by a verifier that says it
26+
covered it.
27+
- `declaredPositionNames` — one RLS persona per declared position.
28+
- `rlsProbePermissionSet` — the object grants and the owner-scoped narrowing
29+
that are what make an RLS run a probe rather than a report about the object
30+
gate.
31+
32+
The top-level read still answers first and is returned untouched, so an app on
33+
today's additive artifact gets a bit-identical answer, and a stack that declares
34+
an empty collection (`objects: []` is truthy) still gets an empty one. Only a
35+
top level that does not carry the key at all consults `packages[]`. A malformed
36+
`packages` array now surfaces `resolveArtifactPackageOrder`'s ADR-0112 refusal
37+
instead of reading as "this app declares nothing".

packages/cli/test/option-b-reader-acceptance.pin.test.ts

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -122,6 +122,13 @@ import { measureShape, type ProbeRow, type ShapeMeasurement } from './fixtures/o
122122
*
123123
* ⛔ SHRINK-ONLY, audited in BOTH directions (see the header). Each line names
124124
* a boundary, a subsystem and the collection it reads.
125+
*
126+
* `@objectstack/verify`'s four rows are absent for a reason worth stating
127+
* rather than inferring: the by-shape sweep (#15210) found those sites, not
128+
* this pin, so card 5/4 (#15229) ADDED them here and DELETED them again inside
129+
* one PR — ledgered red first, then fixed. Both halves are in that card's
130+
* history; the probe still measures all four, which is what keeps a regression
131+
* in them red.
125132
*/
126133
const OPTION_B_LOSSES: readonly string[] = [
127134
'B1 · AppPlugin declared-datasource auto-connect (compiled artifact) · datasources',
@@ -147,10 +154,6 @@ const OPTION_B_LOSSES: readonly string[] = [
147154
'B2 · runtime collectBundleActions over the from-source config · actions + objects[].actions',
148155
'B2 · runtime collectBundleFunctionEntries over the from-source config · functions',
149156
'B2 · runtime collectBundleHooks over the from-source config · hooks',
150-
'B2 · verify declaredPositionNames (one RLS persona per declared position) · positions',
151-
'B2 · verify deriveCrudCases (CRUD round-trip case derivation) · objects',
152-
'B2 · verify deriveCrudCases federated write gate (ADR-0015 double opt-in) · datasources',
153-
'B2 · verify rlsProbePermissionSet (RLS probe grants + owner narrowing) · objects',
154157
'B5 · resolve-project-database readConfigDeclaredDefault (project database tier) · datasourceMapping + datasources',
155158
];
156159

Lines changed: 167 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,167 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* ADR-0130 D4 / option B — this package's readers over a multi-package app
5+
* (#15229, reader program 5/4 of the ruling on #14512).
6+
*
7+
* The acceptance pin for the program lives in `@objectstack/cli`
8+
* (`option-b-reader-acceptance.pin.test.ts`, #15004) and measures these readers
9+
* through a booted two-package fixture. What is here instead is the CONTRACT of
10+
* the resolution itself, which that pin cannot see: that the flattened top level
11+
* still answers FIRST — including when it is an empty array — and that a
12+
* malformed `packages` refuses rather than reading as "no collections".
13+
*
14+
* Every test drives a SHIPPED reader (`deriveCrudCases`, `declaredPositionNames`,
15+
* `rlsProbePermissionSet`), never `declaredCollection` directly: a test shaped
16+
* like the helper would pass over a reader that never calls it.
17+
*/
18+
19+
import { describe, expect, it } from 'vitest';
20+
21+
import { deriveCrudCases } from './derive.js';
22+
import { declaredPositionNames, rlsProbePermissionSet } from './rls.js';
23+
24+
/**
25+
* One package body, as `packages[i].manifest` carries it: an
26+
* `AssembledPackageBodySchema` — `ManifestSchema` fields at the TOP of the body
27+
* with the collections beside them, never a nested `manifest` key.
28+
* `resolveArtifactPackageOrder` parses each entry whole, so these bodies are
29+
* real definitions and not sketches.
30+
*/
31+
const corePackage = {
32+
id: 'com.example.readers.core',
33+
name: 'Readers Core',
34+
version: '1.0.0',
35+
type: 'app',
36+
objects: [
37+
{
38+
name: 'reader_account',
39+
label: 'Reader Account',
40+
fields: { name: { name: 'name', type: 'text', label: 'Name', required: true } },
41+
},
42+
],
43+
datasources: [
44+
{
45+
name: 'reader_warehouse',
46+
label: 'Reader Warehouse',
47+
driver: 'sqlite',
48+
config: { filename: '.objectstack/data/reader-warehouse.db' },
49+
schemaMode: 'external',
50+
external: { allowWrites: true },
51+
},
52+
],
53+
positions: [{ name: 'reader_position', label: 'Reader Position' }],
54+
};
55+
56+
const ordersPackage = {
57+
id: 'com.example.readers.orders',
58+
name: 'Readers Orders',
59+
version: '1.0.0',
60+
type: 'module',
61+
dependencies: { 'com.example.readers.core': '^1.0.0' },
62+
objects: [
63+
{
64+
name: 'reader_wh_order',
65+
label: 'Warehouse Order',
66+
datasource: 'reader_warehouse',
67+
external: { remoteName: 'orders', writable: true },
68+
fields: { name: { name: 'name', type: 'text', label: 'Number', required: true } },
69+
},
70+
],
71+
positions: [{ name: 'reader_second_position', label: 'Second Position' }],
72+
};
73+
74+
/** The option-B shape: `packages[]` carries everything, nothing is flattened. */
75+
const optionB = () => ({
76+
manifest: { id: 'com.example.readers.app', name: 'Readers App', version: '1.0.0', type: 'app' },
77+
packages: [{ manifest: corePackage }, { manifest: ordersPackage }],
78+
});
79+
80+
describe('#15229 — `@objectstack/verify` reads its collections from `packages[]` too', () => {
81+
it('deriveCrudCases derives a case per package-owned object', () => {
82+
const cases = deriveCrudCases(optionB());
83+
expect(cases.map((c) => c.object).sort()).toEqual(['reader_account', 'reader_wh_order']);
84+
});
85+
86+
it('the ADR-0015 write gate resolves the datasource from the OTHER package', () => {
87+
// The object is in `orders`, the datasource that opens the write gate is in
88+
// `core`. A reader that resolved `objects` but not `datasources` reports the
89+
// app's write-opted-in external object as read-only and skips it — a
90+
// verifier quietly proving less, which is the failure mode of this card.
91+
const federated = deriveCrudCases(optionB()).find((c) => c.object === 'reader_wh_order');
92+
expect(federated?.blocked).toBeUndefined();
93+
});
94+
95+
it('declaredPositionNames covers every package, in package order', () => {
96+
expect(declaredPositionNames(optionB())).toEqual(['reader_position', 'reader_second_position']);
97+
});
98+
99+
it('rlsProbePermissionSet grants AND narrows every package-owned object', () => {
100+
const set = rlsProbePermissionSet(optionB()) as unknown as {
101+
objects: Record<string, unknown>;
102+
rowLevelSecurity: Array<{ object: string; operation: string }>;
103+
};
104+
expect(Object.keys(set.objects).sort()).toEqual(['reader_account', 'reader_wh_order']);
105+
// Both halves are load-bearing: the grants stop the OBJECT gate answering
106+
// 403 first, the owner-scoped select is what puts the persona outside the
107+
// record scope. A set with grants and no narrowing is not a probe.
108+
expect(set.rowLevelSecurity.map((r) => r.object).sort())
109+
.toEqual(['reader_account', 'reader_wh_order']);
110+
expect(new Set(set.rowLevelSecurity.map((r) => r.operation))).toEqual(new Set(['select']));
111+
});
112+
113+
describe('the flattened top level answers FIRST — `packages[]` supplies only what it lacks', () => {
114+
it("today's additive artifact answers bit-identically, and does not merge the second copy", () => {
115+
// The additive shape carries every definition TWICE. `packages[]` here
116+
// deliberately carries an object the top level does NOT — if the reader
117+
// merged instead of preferring, this would come back with three cases and
118+
// every app on the additive artifact would be verified against a stack
119+
// that is not the one it composed.
120+
const additive = {
121+
objects: [
122+
{ name: 'reader_account', label: 'Account', fields: { name: { name: 'name', type: 'text' } } },
123+
{ name: 'reader_order', label: 'Order', fields: { name: { name: 'name', type: 'text' } } },
124+
],
125+
packages: [{ manifest: corePackage }, { manifest: ordersPackage }],
126+
};
127+
expect(deriveCrudCases(additive).map((c) => c.object))
128+
.toEqual(['reader_account', 'reader_order']);
129+
});
130+
131+
it('a DECLARED-EMPTY collection stays empty (`objects: []` is truthy)', () => {
132+
// Measured on the sibling card #15006: re-expressing one of these reads as
133+
// "resolve, then take what came back" silently changes the answer for a
134+
// stack that declares an empty collection. Falsy — absent or null — is the
135+
// only thing that reaches `packages[]`.
136+
const declaredEmpty = { objects: [], positions: [], packages: [{ manifest: corePackage }] };
137+
expect(deriveCrudCases(declaredEmpty)).toEqual([]);
138+
expect(declaredPositionNames(declaredEmpty)).toEqual([]);
139+
expect(Object.keys(
140+
(rlsProbePermissionSet(declaredEmpty) as unknown as { objects: Record<string, unknown> }).objects,
141+
)).toEqual([]);
142+
});
143+
144+
it('a single-package app with no `packages` key is unchanged', () => {
145+
const flat = { objects: [{ name: 'reader_solo', fields: { name: { name: 'name', type: 'text' } } }] };
146+
expect(deriveCrudCases(flat).map((c) => c.object)).toEqual(['reader_solo']);
147+
expect(declaredPositionNames({ positions: [{ name: 'solo_position' }] }))
148+
.toEqual(['solo_position']);
149+
expect(deriveCrudCases(undefined)).toEqual([]);
150+
expect(declaredPositionNames(null)).toEqual([]);
151+
});
152+
});
153+
154+
it('a malformed `packages` REFUSES with the ADR-0112 envelope, never as "no collections"', () => {
155+
// `resolveArtifactPackageOrder` owns this verdict (`@objectstack/core`,
156+
// ADR-0130 D4) — asserted here as the envelope (`code` + `status`) rather
157+
// than as a bare throw, so a driver that throws a plain Error cannot pass.
158+
let raised: (Error & { code?: string; status?: number }) | undefined;
159+
try {
160+
deriveCrudCases({ packages: [{ notAManifest: true }] });
161+
} catch (e) {
162+
raised = e as Error & { code?: string; status?: number };
163+
}
164+
expect(raised?.code).toBe('INVALID_ARTIFACT_PACKAGE_ENTRY');
165+
expect(raised?.status).toBe(422);
166+
});
167+
});
Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,80 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* Where this package reads the app's declared collections from
5+
* (ADR-0130 D4/D5 — option B; #15229, reader program 5/4 of the ruling on
6+
* #14512).
7+
*
8+
* ## The loss this closes
9+
*
10+
* A multi-package artifact today serializes every definition TWICE: once
11+
* flattened onto the top level, once inside `packages[i].manifest`. Option B
12+
* removes the flattened copy. Every reader in this package took the flattened
13+
* copy and nothing else, so under option B `deriveCrudCases` derived ZERO cases
14+
* and `rlsProbePermissionSet` built an EMPTY probe permission set — and
15+
* `os verify` printed `✓ verify passed` over an app it had asserted nothing
16+
* about. That is the reason this package's card carries `priority:p2` while the
17+
* rest of the reader program carries p3: every other reader loses a capability,
18+
* this one loses the verification itself and reports success while doing it.
19+
*
20+
* ## The rule, and why it is in this order
21+
*
22+
* **The caller's original expression answers first; `packages[]` supplies only
23+
* what the top level LACKS.** Not a re-expression of the same question:
24+
*
25+
* - `config.objects` is truthy for an EMPTY ARRAY, so re-expressing a read as
26+
* "resolve everything, then take what came back" silently changes the
27+
* answer for a stack that declares `objects: []` — measured on the sibling
28+
* card #15006. A falsy top level (absent / null) is the only thing that
29+
* reaches `packages[]` here, so a declared-empty collection stays empty.
30+
* - Today's artifact is still additive, so the top level is present and the
31+
* answer is bit-identical to the one before this change. That is what makes
32+
* this card revertible on its own and safe to land BEFORE the emitter half
33+
* (#14512) — it is a widening, never a switch.
34+
*
35+
* ## Why `resolveArtifactPackageOrder` and never `config.packages`
36+
*
37+
* The package order is `@objectstack/core`'s decision (ADR-0130 D4+D5, since
38+
* #14643): dependency-topological, entry-gated, duplicate-refusing. Iterating
39+
* `config.packages` here would be a SECOND traversal and therefore a second
40+
* ordering — two answers to a question the artifact contract answers once. The
41+
* one behavioural consequence worth stating: a malformed `packages` array now
42+
* raises that function's ADR-0112 refusal (`code` + `status: 422`) instead of
43+
* being read as "no collections", which is the loud-over-silent direction this
44+
* whole card is about.
45+
*/
46+
47+
import { resolveArtifactPackageOrder } from '@objectstack/core';
48+
49+
/**
50+
* The app's declared members of one collection — the flattened top level when
51+
* it carries the key, otherwise every package's contribution in dependency
52+
* order.
53+
*
54+
* @param config - The loaded app config (`os verify` gets it from `loadConfig`),
55+
* or a compiled artifact. Nullish is tolerated exactly as the `?.` reads it
56+
* replaced were.
57+
* @param key - The collection key, e.g. `objects` / `datasources` / `positions`.
58+
* @returns The top-level value UNTOUCHED when it is truthy — including a
59+
* non-array one, so a malformed config still fails where it used to rather
60+
* than being quietly repaired here — otherwise the concatenation of the
61+
* package bodies' arrays.
62+
* @throws The ADR-0112 refusal from `resolveArtifactPackageOrder` when the
63+
* artifact carries a malformed `packages` array or a duplicate package id.
64+
*/
65+
export function declaredCollection(config: any, key: string): any[] {
66+
const declared = config?.[key];
67+
if (declared) return declared as any[];
68+
69+
// `resolveArtifactPackageOrder` reads a bare config with no `packages` key as
70+
// a single-package artifact and hands it straight back, so this branch on
71+
// today's single-package apps re-reads the same absent key and answers `[]` —
72+
// the same empty array the `?? []` it replaced produced.
73+
const bodies = resolveArtifactPackageOrder(config) as Array<Record<string, unknown>> | undefined;
74+
const merged: any[] = [];
75+
for (const body of bodies ?? []) {
76+
const items = body?.[key];
77+
if (Array.isArray(items)) merged.push(...items);
78+
}
79+
return merged;
80+
}

packages/verify/src/derive.ts

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,8 @@
1919
// is reported `blocked` with a precise reason — the gate stays honest.
2020

2121

22+
import { declaredCollection } from './artifact-collections.js';
23+
2224
const COMPUTED = new Set(['formula', 'summary', 'autonumber', 'rollup', 'vector']);
2325
const RELATIONAL = new Set(['lookup', 'master_detail', 'master-detail', 'masterdetail', 'tree']);
2426
const STRUCTURED = new Set(['composite', 'repeater', 'record', 'location', 'address']);
@@ -173,11 +175,17 @@ interface Draft {
173175
* - a required non-relational field that can't be synthesized (unchanged from v0).
174176
*/
175177
export function deriveCrudCases(config: any): CrudCase[] {
176-
const objects: any[] = config?.objects ?? [];
178+
// ADR-0130 D4 (#15229): the flattened top level answers first and
179+
// `packages[]` supplies only what it lacks, so a multi-package app under
180+
// option B derives its cases instead of deriving NONE and passing. Both reads
181+
// go through the same resolver — `objects` alone would leave the federated
182+
// write gate below judging against an empty datasource map, which reports an
183+
// app's write-opted-in external objects as read-only and silently skips them.
184+
const objects: any[] = declaredCollection(config, 'objects');
177185
const byName = new Map<string, any>();
178186
for (const o of objects) if (o?.name) byName.set(o.name, o);
179187
const dsByName = new Map<string, any>();
180-
for (const ds of (config?.datasources ?? [])) if (ds?.name) dsByName.set(ds.name, ds);
188+
for (const ds of declaredCollection(config, 'datasources')) if (ds?.name) dsByName.set(ds.name, ds);
181189

182190
const drafts = new Map<string, Draft>();
183191

packages/verify/src/rls.ts

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -51,8 +51,9 @@
5151
// against `positions: ['contributor']` rules.
5252
//
5353
// So a run now FANS OUT: one persona per position the app DECLARES
54-
// (`declaredPositionNames`, read from `config.positions` — never a transcribed
55-
// list, so a position added next month is covered without touching this file),
54+
// (`declaredPositionNames`, read from the app's DECLARED positions — never a
55+
// transcribed list, so a position added next month is covered without touching
56+
// this file),
5657
// each holding that position and nothing else, i.e. exactly the capability the app
5758
// itself binds to it. The same invariant then runs unchanged for each.
5859
//
@@ -84,6 +85,7 @@ import type { PermissionSet } from '@objectstack/spec/security';
8485

8586
import type { VerifyStack } from './harness.js';
8687
import { deriveCrudCases, fillRelationalRefs } from './derive.js';
88+
import { declaredCollection } from './artifact-collections.js';
8789

8890
const PROBE_TYPES = new Set(['text', 'textarea', 'string']);
8991
const MUTATION = 'rls-mutated-by-B';
@@ -111,8 +113,9 @@ export function rlsPositionProbeEmail(position: string): string {
111113
}
112114

113115
/**
114-
* Machine names of the positions the app DECLARES (`config.positions`), in
115-
* declaration order, deduplicated.
116+
* Machine names of the positions the app DECLARES, in declaration order,
117+
* deduplicated — read from the flattened `config.positions` when it carries
118+
* them and from `packages[]` when it does not (ADR-0130 D4, #15229).
116119
*
117120
* ⛔ DERIVED, never transcribed. A hand-written roster is how a verifier quietly
118121
* stops covering the position someone adds next month — it keeps passing, over a
@@ -131,7 +134,7 @@ export function declaredPositionNames(config: any): string[] {
131134
const anchors = AUDIENCE_ANCHOR_POSITIONS as readonly string[];
132135
const seen = new Set<string>();
133136
const names: string[] = [];
134-
for (const declared of (config?.positions ?? []) as any[]) {
137+
for (const declared of declaredCollection(config, 'positions')) {
135138
const name = typeof declared === 'string' ? declared : declared?.name;
136139
if (typeof name !== 'string' || name.length === 0) continue;
137140
if (anchors.includes(name) || seen.has(name)) continue;
@@ -343,7 +346,7 @@ function rowsOf(payload: any): any[] {
343346
export function rlsProbePermissionSet(config: any): PermissionSet {
344347
const objects: Record<string, { allowRead: boolean; allowEdit: boolean }> = {};
345348
const rowLevelSecurity: Array<Record<string, unknown>> = [];
346-
for (const o of (config?.objects ?? []) as any[]) {
349+
for (const o of declaredCollection(config, 'objects')) {
347350
if (!o?.name) continue;
348351
objects[o.name] = { allowRead: true, allowEdit: true };
349352
rowLevelSecurity.push({

0 commit comments

Comments
 (0)