Skip to content

Commit c550baf

Browse files
hotlongclaude
andauthored
fix(verify): a multi-package app no longer gets an os verify that measures nothing (#15229) (#15281)
* test(cli): ledger the four `@objectstack/verify` option-B losses (#15229) Step 1 of the card's ordered pair: make the loss visible BEFORE fixing it, so the fix has something to be checked against. The by-shape sweep (#15210) found these four reads; neither `@objectstack/verify` site had a row in the #15004 acceptance pin, which is why the sweep and not the pin is what found them. Four rows, all at B2 (`os verify` has one door — `loadConfig` — and never opens a compiled artifact, so a B1 twin would measure a path no command drives): - `deriveCrudCases` · objects — zero CRUD round-trip cases derived - `deriveCrudCases` · datasources — the ADR-0015 double write gate - `declaredPositionNames` · positions — no persona for any declared position - `rlsProbePermissionSet` · objects — an empty probe permission set Every row calls a reader the package SHIPS; none re-implements one. The datasource row needed a fixture member the zoo lacked: the datasource-by- name map decides exactly one thing — whether a federated object's probe insert clears ADR-0015's double opt-in — so watching it requires a federated object. It is declared in the module package while the datasource that gates it is in the App package, which makes the row a cross-package resolution. The control's registry list grows by that object; the control itself is untouched. RED, with only the rows added and the ledger lines absent: A subsystem lost a collection that the ledger does not carry — this is the failure #15004 exists to make loud. 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 The BASELINE (additive shape) stayed green in that same run, so the red is a discrimination and not a broken fixture. The four lines are then added to OPTION_B_LOSSES, which is what this commit leaves in the tree; the next commit fixes the readers and deletes them. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UHvF5hyiZjnCyExFnfQB8m * 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 * tooling(verify,cli): resolve the new cross-package imports to SOURCE (#15229) Both halves are what `check:test-source-alias` and `check:type-source-resolution` prescribe for a dep a change newly reaches; neither registry is widened (both are shrink-only, and widening is never the fix): - `packages/verify/vitest.config.ts` gains its first `resolve.alias` — one ANCHORED entry for `@objectstack/core`, which `artifact-collections.ts` now imports and every test in the package reaches through `derive.ts` / `rls.ts`. Without it those tests resolve core through its `exports` map to `dist/`, i.e. they become verdicts about build state, and the dangerous half of that is a suite that passes GREEN over a stale artifact. - `packages/verify/tsconfig.json` gains the matching `paths` rule, and the `rootDir` widening that pulling core's source into the program requires (`packages/core/tsconfig.json`'s own precedent; emit is unaffected, this package builds with tsup). - `packages/cli/tsconfig.test.json` gains a FOURTH bare-name rule beside the three #15004 added, for the `@objectstack/verify` the probe rows now import. Bare names, no stars: `@objectstack/core` publishes a `./logger` subpath that keeps resolving through its own `exports` map, and a starred rule would swallow it and resolve it THROUGH a file (ENOTDIR). Measured after: `pnpm check:test-source-alias` OK (72 packages scanned), `pnpm check:type-source-resolution` OK (121 programs / 78 packages), `pnpm --filter @objectstack/verify typecheck` and `pnpm --filter @objectstack/cli typecheck` both clean, and the two suites the alias re-resolves stay green: verify 58/58, the option-B pin 6/6. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UHvF5hyiZjnCyExFnfQB8m * test(cli): raise the option-B probe anti-vacuity floor to the measured 35 The merge with main brings #15261's re-anchored floor (a real bound now that `OPTION_B_LOSSES` is empty) together with this card's four `@objectstack/verify` rows. Merged, the probe measures 35 rows, so the floor moves 30 -> 35. 35 is measured, not remembered: with the assertion temporarily written `toBe(-1)` the run reports `expected 35 to be -1`; a floor of 36 goes RED on the same fixture. The raise also closes an off-by-one it inherited — 30 stood against a probe that measured 31. 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 c0c07ef commit c550baf

11 files changed

Lines changed: 503 additions & 23 deletions
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/fixtures/option-b-collection-zoo.ts‎

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,28 @@ export const ORDERS_PACKAGE_ID = 'com.example.probe.orders';
7979
export const PROBE_DATASOURCE = 'probe_primary';
8080
/** Relative to the project root — `resolve-project-database` anchors it there. */
8181
export const PROBE_DATASOURCE_FILE = '.objectstack/data/probe-primary.db';
82+
/**
83+
* A FEDERATED datasource (ADR-0015), write gate OPEN.
84+
*
85+
* It is here for one reader: `deriveCrudCases` builds a datasource-by-name map
86+
* and consults it for exactly ONE decision — whether a federated object's probe
87+
* insert is allowed through the double write gate — so the only way to watch
88+
* that map is to carry an external object whose gate depends on it. Declared in
89+
* the App package while the object it gates lives in the module package, so the
90+
* row watching it measures a CROSS-PACKAGE resolution and not a within-body
91+
* read.
92+
*
93+
* ⚠️ The lean probe kernel builds no live driver for it, so every boot in this
94+
* fixture's probe logs one `federated (external) object(s) are NOT bound to
95+
* their remote table` ERROR. That is the truthful verdict for a federated
96+
* object with no remote behind it; it touches no row here (nothing in this
97+
* probe reads the object's data) and is expected output, not a fixture defect.
98+
*/
99+
export const PROBE_FEDERATED_DATASOURCE = 'probe_federated';
100+
/** Relative to the project root, same anchoring as the primary. */
101+
export const PROBE_FEDERATED_FILE = '.objectstack/data/probe-federated.db';
102+
/** The write-opted-in federated object the datasource gate above admits. */
103+
export const PROBE_FEDERATED_OBJECT = 'probe_federated_order';
82104
/** The `isDefault` permission set `appSecurityPluginOptions` must resolve. */
83105
export const PROBE_DEFAULT_PERMISSION_SET = 'probe_default_profile';
84106
export const PROBE_POSITION = 'probe_position';
@@ -137,6 +159,19 @@ const coreStack = (): ObjectStackDefinition =>
137159
config: { filename: PROBE_DATASOURCE_FILE },
138160
active: true,
139161
},
162+
// The federated half — see `PROBE_FEDERATED_DATASOURCE`. `datasourceMapping`
163+
// does NOT route to it (the project default stays `probe_primary`), so the
164+
// only thing that reaches it is the object in the OTHER package binding to
165+
// it by name.
166+
{
167+
name: PROBE_FEDERATED_DATASOURCE,
168+
label: 'Probe Federated',
169+
driver: 'sqlite',
170+
config: { filename: PROBE_FEDERATED_FILE },
171+
schemaMode: 'external',
172+
external: { allowWrites: true },
173+
active: true,
174+
},
140175
],
141176
datasourceMapping: [
142177
{ datasource: PROBE_DATASOURCE, default: true },
@@ -199,6 +234,22 @@ const ordersStack = (): ObjectStackDefinition =>
199234
name: { name: 'name', type: 'text', label: 'Number', required: true },
200235
},
201236
},
237+
// The FEDERATED object, bound to the datasource the OTHER package
238+
// declares and write-opted-in on its own half of ADR-0015's double gate.
239+
// A reader that resolves objects but not datasources still gets this one
240+
// wrong — it reports the object `blocked` as read-only — which is what
241+
// makes the two collections separately observable through one function.
242+
{
243+
name: PROBE_FEDERATED_OBJECT,
244+
label: 'Probe Federated Order',
245+
pluralLabel: 'Probe Federated Orders',
246+
sharingModel: 'private',
247+
datasource: PROBE_FEDERATED_DATASOURCE,
248+
external: { remoteName: 'remote_orders', writable: true },
249+
fields: {
250+
name: { name: 'name', type: 'text', label: 'Number', required: true },
251+
},
252+
},
202253
],
203254
actions: [
204255
{

‎packages/cli/test/fixtures/option-b-reader-probe.ts‎

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,11 @@ import {
6464
} from '@objectstack/runtime';
6565
import { appSecurityPluginOptions } from '@objectstack/plugin-security';
6666
import { devI18nPluginOptions } from '@objectstack/plugin-dev';
67+
import {
68+
declaredPositionNames,
69+
deriveCrudCases,
70+
rlsProbePermissionSet,
71+
} from '@objectstack/verify';
6772
import { ObjectStackDefinitionSchema, normalizeStackInput } from '@objectstack/spec';
6873

6974
// The lowering itself, not a copy of it — reached as SOURCE, by relative path,
@@ -88,6 +93,7 @@ import {
8893
import {
8994
PACKAGE_OWNED_COLLECTION_KEYS,
9095
PROBE_DEFAULT_PERMISSION_SET,
96+
PROBE_FEDERATED_OBJECT,
9197
PROBE_FUNCTION,
9298
PROBE_FUNCTION_EFFECT,
9399
} from './option-b-collection-zoo.js';
@@ -476,6 +482,59 @@ export async function measureShape(project: unknown, projectRoot: string): Promi
476482
unionItems,
477483
));
478484

485+
// ── B2 · `os verify`'s readers (#15229) ──────────────────────────────────
486+
//
487+
// `os verify` has exactly ONE door — `loadConfig` (`verify.ts:92`) — and it
488+
// never opens a compiled artifact, so these rows are B2 and there is
489+
// deliberately no B1 twin. A B1 row here would measure a path no command
490+
// drives, which reads as coverage this probe does not have.
491+
//
492+
// Every row calls a reader `@objectstack/verify` SHIPS, per this file's rule.
493+
// What makes these the most expensive rows in the table is what the readers
494+
// are FOR: `os verify` derives its whole proof set from the app's metadata,
495+
// so a collection it cannot see is not a missing feature — it is a run that
496+
// asserts nothing and still prints `✓ verify passed`.
497+
498+
const crudCases = deriveCrudCases(project) as Array<{ object: string; blocked?: string }>;
499+
rows.push(countRow(
500+
'B2 · verify deriveCrudCases (CRUD round-trip case derivation) · objects',
501+
crudCases.length,
502+
));
503+
504+
// The datasource-by-name map, watched through the ONE decision it makes: the
505+
// ADR-0015 double write gate. A reader that resolved `objects` but not
506+
// `datasources` still fails this row — the case comes back `blocked` as
507+
// "external read-only", which is a verifier silently skipping an object the
508+
// app explicitly opted into writes for.
509+
const federated = crudCases.find((c) => c.object === PROBE_FEDERATED_OBJECT);
510+
rows.push(row(
511+
'B2 · verify deriveCrudCases federated write gate (ADR-0015 double opt-in) · datasources',
512+
federated ? (federated.blocked ?? 'derived — probe insert allowed') : 'no case derived at all',
513+
!federated || Boolean(federated.blocked),
514+
));
515+
516+
rows.push(countRow(
517+
'B2 · verify declaredPositionNames (one RLS persona per declared position) · positions',
518+
declaredPositionNames(project).length,
519+
));
520+
521+
// The probe permission set is what makes an RLS run a PROBE at all: the
522+
// object grants are what stop `checkObjectPermission` answering 403 before
523+
// record scope is consulted, and the owner-scoped SELECT narrowing is what
524+
// puts the persona outside the record scope. Empty on either half and every
525+
// verdict the run prints is about neither.
526+
const probeSet = rlsProbePermissionSet(project) as unknown as {
527+
objects?: Record<string, unknown>;
528+
rowLevelSecurity?: unknown[];
529+
};
530+
const grants = Object.keys(probeSet?.objects ?? {}).length;
531+
const narrowings = (probeSet?.rowLevelSecurity ?? []).length;
532+
rows.push(row(
533+
'B2 · verify rlsProbePermissionSet (RLS probe grants + owner narrowing) · objects',
534+
`${grants} granted object(s), ${narrowings} owner-scope rule(s)`,
535+
grants === 0 || narrowings === 0,
536+
));
537+
479538
// ── The booted AppPlugin, on BOTH entry paths ────────────────────────────
480539

481540
const bootedFromArtifact = await bootAndRecord(bundle);

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

Lines changed: 31 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -165,6 +165,14 @@ import { measureShape, type ProbeRow, type ShapeMeasurement } from './fixtures/o
165165
* measures must now be `present` in BOTH shapes, so a reader that regresses —
166166
* or a new reader that arrives unresolved — is red on arrival with nothing left
167167
* to absorb it. Adding a line is never how that red is fixed.
168+
*
169+
* `@objectstack/verify`'s four rows (#15229) are absent from this list for
170+
* exactly that reason, and the reason is worth stating rather than inferring:
171+
* the by-shape sweep (#15210) found those sites, not this pin, so that card
172+
* LEDGERED them red first and then DELETED the entries again inside one PR, by
173+
* fixing the readers rather than the ledger. Both halves are in its history;
174+
* the probe still measures all four, which is what keeps a regression in them
175+
* red.
168176
*/
169177
const OPTION_B_LOSSES: readonly string[] = [];
170178

@@ -222,10 +230,19 @@ describe('#15004 — option-B acceptance pin: every subsystem must see its colle
222230
// control for every LOST row below: it proves the option-B fixture really
223231
// carries every definition under `packages[]`, so a zero is a reader losing
224232
// a collection and never a fixture that shipped an empty package.
225-
expect(additive.registryObjectsFromArtifact).toEqual(['probe_account', 'probe_order']);
233+
// `probe_federated_order` is the ADR-0015 federated object #15229 added to
234+
// the zoo: `deriveCrudCases`'s datasource-by-name map decides exactly one
235+
// thing — whether that object's probe insert clears the double write gate —
236+
// so watching the map required carrying one. It registers like any other
237+
// object, which is why it is in this control's list.
238+
expect(additive.registryObjectsFromArtifact).toEqual(
239+
['probe_account', 'probe_federated_order', 'probe_order'],
240+
);
226241
expect(optionB.registryObjectsFromArtifact).toEqual(additive.registryObjectsFromArtifact);
227242
expect(optionB.registryObjectsFromSource).toEqual(additive.registryObjectsFromSource);
228-
expect(optionB.registryObjectsFromSource).toEqual(['probe_account', 'probe_order']);
243+
expect(optionB.registryObjectsFromSource).toEqual(
244+
['probe_account', 'probe_federated_order', 'probe_order'],
245+
);
229246
});
230247

231248
// ── The baseline: green today, and it must stay green ────────────────────
@@ -248,23 +265,30 @@ describe('#15004 — option-B acceptance pin: every subsystem must see its colle
248265
// comment, and the fourth direction this file's header claims ("the probe
249266
// itself quietly measuring less ⇒ RED") had silently stopped existing.
250267
//
251-
// 30 is MEASURED, not remembered: with this line temporarily written
268+
// 35 is MEASURED, not remembered: with this line temporarily written
252269
// `expect(additive.rows.length).toBe(-1)`, the run reports
253-
// `expected 30 to be -1`. Verified live at the boundary in the same
254-
// session — a floor of 31 goes RED on the same fixture, so the assertion
270+
// `expected 35 to be -1`. Verified live at the boundary in the same
271+
// session — a floor of 36 goes RED on the same fixture, so the assertion
255272
// is not satisfied by construction.
256273
//
274+
// ⚠️ RAISED by #15229, which added four `@objectstack/verify` rows, and the
275+
// raise repaired an off-by-one while it was here: the floor read 30 against
276+
// a probe that measured 31, so one row could stop being measured with
277+
// nothing going red. It is now the measured count EXACTLY — no slack — and
278+
// that is what makes the next card's raise a step it cannot skip without
279+
// this line failing.
280+
//
257281
// `>=` rather than `toBe` on purpose, and it is the same shrink-only
258282
// direction the ledger uses: a row ADDED to the probe is welcome and stays
259283
// green, a row that stops being measured is red. Raise the floor when the
260284
// probe grows; ⛔ never lower it to make a red run green.
261285
expect(
262286
additive.rows.length,
263-
`The probe measured ${additive.rows.length} rows, fewer than the 30 it measured when ` +
287+
`The probe measured ${additive.rows.length} rows, fewer than the 35 it measured when ` +
264288
`this floor was set. A row that stops being measured stops being able to fail, which ` +
265289
`is the one direction this pin cannot detect anywhere else — fix the probe rather ` +
266290
`than the floor.`,
267-
).toBeGreaterThanOrEqual(30);
291+
).toBeGreaterThanOrEqual(35);
268292
});
269293

270294
// ── The pin ──────────────────────────────────────────────────────────────

‎packages/cli/tsconfig.test.json‎

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -126,9 +126,10 @@
126126
"moduleResolution": "bundler",
127127
"lib": ["ES2022", "DOM", "DOM.Iterable"],
128128
"types": ["node"],
129-
// [#15004] THREE bare-name rules, no star, for the three workspace deps the
130-
// option-B acceptance pin reaches (`test/option-b-reader-acceptance.pin.test.ts`
131-
// and its two fixtures). Without them tsc resolves those specifiers through
129+
// [#15004, #15229] FOUR bare-name rules, no star, for the four workspace
130+
// deps the option-B acceptance pin reaches
131+
// (`test/option-b-reader-acceptance.pin.test.ts` and its two fixtures;
132+
// `@objectstack/verify` joined them with card 5/4's probe rows). Without them tsc resolves those specifiers through
132133
// each package's `exports` map to `dist/index.d.ts` — A BUILD ARTIFACT — so
133134
// this suite's type verdict about the readers the reader program is about to
134135
// CHANGE would be a verdict about the last `pnpm build` instead, which is
@@ -142,8 +143,8 @@
142143
// the specifier EXACTLY, which is load-bearing here rather than incidental:
143144
// `@objectstack/objectql` publishes a second subpath (`./core`), and that
144145
// specifier deliberately keeps resolving through the package's own
145-
// `exports` map, untouched by this table. `plugin-security` and `runtime`
146-
// each publish only `"."`. And a target matching nothing on disk is worse
146+
// `exports` map, untouched by this table. `plugin-security`, `runtime` and
147+
// `verify` each publish only `"."`. And a target matching nothing on disk is worse
147148
// than absent, because tsc then falls back to node resolution — i.e. to
148149
// `dist` — silently.
149150
//
@@ -159,7 +160,8 @@
159160
// package publishes only `"."`.
160161
"@objectstack/plugin-dev": ["../plugins/plugin-dev/src/index.ts"],
161162
"@objectstack/plugin-security": ["../plugins/plugin-security/src/index.ts"],
162-
"@objectstack/runtime": ["../runtime/src/index.ts"]
163+
"@objectstack/runtime": ["../runtime/src/index.ts"],
164+
"@objectstack/verify": ["../verify/src/index.ts"]
163165
}
164166
},
165167
"include": ["test/**/*", "vitest.config.ts", "vitest-tiers.ts", "vitest-tiers.fixtures.ts"],

0 commit comments

Comments
 (0)