From 85a1e89df71c7053b5ee33dd69e53e269799fb80 Mon Sep 17 00:00:00 2001 From: Jack Zhuang <277994282+os-zhuang@users.noreply.github.com> Date: Fri, 21 Aug 2026 01:37:39 +0800 Subject: [PATCH 1/3] fix(metadata-protocol): judge a package publish against the batch's own pending declarations `publishPackageDrafts` built its author-time validation context from `engine.registry` alone -- the ALREADY-LIVE universe. A draft is not in that registry (the write-through runs on `mode: 'publish'`), and the batch's own promotions do not put it there either: `applyRegistryWriteThrough` lives in Phase 2, after the Phase-1 transaction in which every draft is gated and promoted. So while a batch was judged, no member of it was visible to any other member, in any order. Measured 2026-08-21 on a cloud rig: a package carrying `dataset/shyx_customer_ds` and a `dashboard` whose widget binds it rolled back at the dashboard with `[widget-dataset-unknown] ... does not resolve to a declared dataset`, on every attempt and under both dataset names the author tried. So a package shipping a dashboard together with its dataset could never publish. The batch door now reads its own pending drafts ONCE, before any promotion (a promote deletes the row it reads), and folds them into all four collections the closure carries -- `objects`, `permissions`, `books`, `datasets` -- replacing a live declaration of the same name rather than sitting beside it. A name in neither the batch nor the live universe is still refused exactly as before. Fixes #10377 Co-Authored-By: Claude Fable 5 --- ...sh-batch-closure-carries-pending-drafts.md | 24 + .../protocol-publish-drafts-closure.test.ts | 462 ++++++++++++++++++ packages/metadata-protocol/src/protocol.ts | 130 ++++- .../src/runtime-authoring-gate.ts | 124 ++++- scripts/engine-double-contract.pinned.json | 10 + 5 files changed, 745 insertions(+), 5 deletions(-) create mode 100644 .changeset/publish-batch-closure-carries-pending-drafts.md create mode 100644 packages/metadata-protocol/src/protocol-publish-drafts-closure.test.ts diff --git a/.changeset/publish-batch-closure-carries-pending-drafts.md b/.changeset/publish-batch-closure-carries-pending-drafts.md new file mode 100644 index 0000000000..fab2fbdd63 --- /dev/null +++ b/.changeset/publish-batch-closure-carries-pending-drafts.md @@ -0,0 +1,24 @@ +--- +'@objectstack/metadata-protocol': patch +--- + +fix: a package publishes as a self-consistent unit — `publishPackageDrafts` judges each draft against the batch's own pending declarations + +The batch publish door built the author-time validation context from +`engine.registry` alone, i.e. the ALREADY-LIVE universe. A draft is not in that +registry, and the batch's own promotions do not put it there either: the +registry write-through runs in Phase 2, after the Phase-1 transaction that gates +and promotes every draft. So while a batch was being judged, no member of it was +visible to any other member — in any order. + +Measured consequence: a package shipping `dataset/x` together with a `dashboard` +whose widget binds `x` could NEVER publish. `validateWidgetBindings` raises +`widget-dataset-unknown` at `severity: 'error'`, which refuses the promotion, +and the batch being all-or-nothing rolls the whole package back. Renaming the +dataset could not help, and neither could re-ordering the items. + +`publishPackageDrafts` now reads its own pending drafts once, before any +promotion, and folds them into all four context collections the closure carries +(`objects`, `permissions`, `books`, `datasets`) — pending declarations replace a +live one of the same name, never sit beside it. A binding that resolves to +neither the batch nor the live universe is still refused exactly as before. diff --git a/packages/metadata-protocol/src/protocol-publish-drafts-closure.test.ts b/packages/metadata-protocol/src/protocol-publish-drafts-closure.test.ts new file mode 100644 index 0000000000..6ec30d0810 --- /dev/null +++ b/packages/metadata-protocol/src/protocol-publish-drafts-closure.test.ts @@ -0,0 +1,462 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * #10377 — the BATCH publish door judges a package against its OWN closure: + * the same batch's pending drafts are part of the resolution universe. + * + * ## The defect, measured on a cloud rig 2026-08-21 + * + * An AI-built package `app.shyx` drafted `dataset/shyx_customer_ds` and + * `dashboard/customer_dashboard` (a widget bound to that dataset) together. + * Every `publishPackageDrafts` attempt rolled back: + * + * ``` + * batch publish of 'app.shyx' rolled back at dashboard/customer_dashboard: + * [invalid_metadata] … dashboards[0].widgets[0]: + * [widget-dataset-unknown] dataset "shyx_customer_ds" does not resolve … + * ``` + * + * Root cause, verified at source before this file was written: + * `assertRuntimeAuthoringRules` builds every context collection with + * `listCollection(…)`, which reads `engine.registry` — the LIVE universe. A + * draft is deliberately NOT in that registry (`saveMetaItem` write-through + * runs on `mode: 'publish'` only), and the batch door's own promotions do not + * put it there either: `applyRegistryWriteThrough` runs in Phase 2, AFTER the + * Phase-1 transaction that gates and promotes every draft. So NO same-batch + * draft is ever visible to any sibling's gate pass — which is why the symptom + * is order-independent, and why re-naming the dataset (which the build agent + * tried twice) could never help. + * + * ## Why datasets is where it BLOCKS, and the other collections do not + * + * `validateWidgetBindings` raises `widget-dataset-unknown` at + * `severity: 'error'`, and an error finding refuses the promotion — the batch + * being all-or-nothing (ADR-0067 D2), that refusal aborts the whole package. + * The `objects` and `permissions` gaps are the same defect at advisory + * severity: they do not refuse, they manufacture findings that describe + * nothing. Both are pinned below, because a closure that is uniform is the + * property #9612/#10058 declared and a per-collection patch is what produced + * this card. + * + * ## The discriminating tests are the ones that expect SILENCE + * + * A "still refuses a genuinely absent dataset" test alone would also pass with + * the whole gate disabled. So each pair here is (clean batch publishes) + + * (genuinely dangling still refuses), and the first half is the one that fails + * on `origin/main`. + * + * Harness: the faithful multi-table stub engine used by + * `protocol-publish-drafts-advisories.test.ts` (kept local — self-contained + * harnesses are the established shape here, so two tripwires fail + * independently). Nothing on the publish path is stubbed: the REAL + * `saveMetaItem` / `publishPackageDrafts` run. + */ + +import { describe, expect, it, vi, beforeEach, afterEach } from 'vitest'; +// The producer's OWN write-verb dispatch decisions, so the fake engine below +// cannot accept a call ObjectQL refuses. From `@objectstack/metadata-core`, +// never from `@objectstack/objectql` — that import would close a cycle. +import { assertEngineDeleteDispatch, assertEngineUpdateDispatch } from '@objectstack/metadata-core'; +import { ObjectStackProtocolImplementation } from './protocol.js'; + +interface Row { + id: string; + type: string; + name: string; + organization_id: string | null; + package_id: string | null; + state: string; + metadata: string; + checksum?: string; + version?: number; + updated_at?: string; + created_at?: string; +} + +interface HistoryRow { + id: string; + event_seq: number; + name: string; + type: string; + version: number; + operation_type: string; + metadata: string | null; + checksum: string | null; + previous_checksum: string | null; + change_note?: string | null; + source?: string | null; + organization_id: string | null; + recorded_by?: string | null; + recorded_at: string; +} + +// Overlay rows are keyed by (type, name, org, state, package_id) — the ADR-0048 key. +function keyOf(w: Record) { + return `${w.type}|${w.name}|${w.organization_id ?? '__env__'}|${w.state ?? 'active'}|${w.package_id ?? '__nopkg__'}`; +} + +function matchesMetadataWhere(r: Row, where: Record): boolean { + for (const [k, v] of Object.entries(where)) { + if (k === '$or') { + const clauses = v as Array>; + if (!clauses.some((c) => matchesMetadataWhere(r, c))) return false; + continue; + } + if (v === undefined) continue; + if ((r as any)[k] !== v) return false; + } + return true; +} + +/** The LIVE object universe — the base object every fixture dataset reads. */ +const liveCustomerObject = { + name: 'shyx_customer', + label: 'Customer', + fields: { + status: { type: 'text', label: 'Status' }, + amount: { type: 'number', label: 'Amount' }, + }, +}; + +/** The LIVE permission universe — one set, granting only the live object. */ +const liveReadonlySet = { + name: 'shyx_readonly', + label: 'Read Only', + objects: { shyx_customer: { allowRead: true } }, +}; + +function makeStubEngine(options?: { liveObjects?: unknown[]; livePermissions?: unknown[] }) { + const rows = new Map(); + const historyRows: HistoryRow[] = []; + let nextId = 0; + + const findRow = (w: Record): { key: string; row: Row } | null => { + if (w.id !== undefined) { + for (const [k, r] of rows) if (r.id === w.id) return { key: k, row: r }; + return null; + } + if (w.package_id !== undefined) { + const k = keyOf(w); + const r = rows.get(k); + return r ? { key: k, row: r } : null; + } + for (const [k, r] of rows) if (matchesMetadataWhere(r, w)) return { key: k, row: r }; + return null; + }; + + const matchesHistory = (h: HistoryRow, w: Record): boolean => { + if (w.organization_id !== undefined && h.organization_id !== w.organization_id) return false; + if (w.type !== undefined && h.type !== w.type) return false; + if (w.name !== undefined && h.name !== w.name) return false; + if (w.version !== undefined && h.version !== w.version) return false; + if (w.operation_type !== undefined && h.operation_type !== w.operation_type) return false; + return true; + }; + + const engine: any = { + async findOne(table: string, opts: { where: Record }) { + if (table === 'sys_metadata_history') { + return historyRows.find((h) => matchesHistory(h, opts.where)) ?? null; + } + return findRow(opts.where)?.row ?? null; + }, + async find(table: string, opts: { where: Record }) { + if (table === 'sys_metadata_history') { + return historyRows.filter((h) => matchesHistory(h, opts.where)); + } + return Array.from(rows.values()).filter((r) => matchesMetadataWhere(r, opts.where)); + }, + async insert(table: string, data: Record) { + if (table === 'sys_metadata_audit') return { id: 'audit_skip' }; + if (table === 'sys_metadata_history') { + nextId += 1; + const h: HistoryRow = { id: `h_${nextId}`, ...(data as any) }; + historyRows.push(h); + return { id: h.id }; + } + nextId += 1; + const row = { id: `r_${nextId}`, ...(data as any) } as Row; + rows.set(keyOf(data), row); + return { id: row.id }; + }, + async update(_t: string, data: Record, opts: { where: Record }) { + assertEngineUpdateDispatch(data, opts); + const found = findRow(opts.where); + if (!found) return { id: null }; + const merged = { ...found.row, ...(data as any) }; + rows.delete(found.key); + rows.set(keyOf(merged), merged); + return { id: found.row.id }; + }, + async delete(_t: string, opts: { where: Record }) { + assertEngineDeleteDispatch(opts); + const found = findRow(opts.where); + if (!found) return { deleted: 0 }; + rows.delete(found.key); + return { deleted: 1 }; + }, + async transaction(cb: (ctx: any, info: { owned: boolean }) => Promise): Promise { + return cb(undefined, { owned: true }); + }, + registry: { + registerItem: () => {}, + registerObject: () => {}, + // The LIVE universe. Deliberately holds NO dataset and NO + // permission set: everything the fixtures resolve against comes + // from the batch's own pending drafts, which is the whole subject. + listItems: (type: string) => { + if (type === 'object') return options?.liveObjects ?? [liveCustomerObject]; + // ONE live permission set, granting something unrelated to the + // fixtures below. It is what ACTIVATES + // `security-master-detail-ungranted` (the rule is silent when a + // stack declares no sets at all), so the batch's own grant is + // measured against a rule that is running in both worlds rather + // than against a rule that is off. + if (type === 'permission') return options?.livePermissions ?? [liveReadonlySet]; + return []; + }, + // No declared package namespace → the ADR-0028 prefix pre-flight is + // skipped (legacy-grandfathered path), and `resolveWritePackageScope` + // narrows nothing. + getPackage: () => undefined, + }, + }; + return { engine, rows, historyRows }; +} + +const PKG = 'app.shyx'; + +// ───────────────────────────────────────────────────────────────────────────── +// Fixtures — every one Zod-valid, so the verdict under test is the gate's and +// not a schema failure wearing its clothes. +// ───────────────────────────────────────────────────────────────────────────── + +const customerDataset = (name: string) => ({ + name, + label: 'Customers', + object: 'shyx_customer', + dimensions: [{ name: 'status', field: 'status' }], + measures: [{ name: 'customer_count', aggregate: 'count' }], +}); + +const boardBoundTo = (datasetName: string) => ({ + name: 'customer_dashboard', + label: 'Customer Dashboard', + widgets: [ + { + id: 'kpi_customers', + type: 'metric', + title: 'Customers', + dataset: datasetName, + values: ['customer_count'], + }, + ], +}); + +/** A Zod-valid autolaunched flow whose start node fires on `objectName`. */ +const flowOn = (name: string, objectName: string) => ({ + name, + label: name, + description: `fires on ${objectName}`, + version: 1, + status: 'active', + type: 'autolaunched', + runAs: 'system', + variables: [], + nodes: [ + { + id: 'start', + type: 'start', + label: 'On update', + config: { objectName, triggerType: 'record-after-update' }, + }, + { id: 'end', type: 'end', label: 'End' }, + ], + edges: [{ id: 'e1', source: 'start', target: 'end', type: 'default', isDefault: false }], +}); + +/** + * A detail object whose master_detail child needs an object-level CRUD grant + * — the `security-master-detail-ungranted` subject, which resolves against the + * `permissions` collection. + */ +const detailObject = (name: string) => ({ + name, + label: name, + // `controlled_by_parent` is the authored OWD a master-detail child wants + // (ADR-0090 D1 refuses an unset one at publish, `security-owd-unset`). + sharingModel: 'controlled_by_parent', + fields: { + // `reference`, the sole spelling `FieldSchema` declares — the aliases do + // not parse (strict schema, #5017) and `refOf` in the security rule + // reads only this one. + parent: { type: 'master_detail', label: 'Parent', reference: 'shyx_customer', required: true }, + note: { type: 'text', label: 'Note' }, + }, +}); + +/** A permission set granting object-level CRUD on `objectName`. */ +const permissionGranting = (name: string, objectName: string) => ({ + name, + label: name, + objects: { + [objectName]: { allowRead: true, allowCreate: true, allowEdit: true }, + }, +}); + +/** Stage one env-wide, package-bound draft (Studio's "Save Draft" shape). */ +async function stageDraft( + protocol: ObjectStackProtocolImplementation, + type: string, + item: { name: string }, +): Promise { + await (protocol as any).saveMetaItem({ + type, name: item.name, item, packageId: PKG, mode: 'draft', + }); +} + +const rulesOf = (res: any, name: string): string[] => + (res.published.find((p: any) => p.name === name)?.advisories ?? []).map((a: any) => a.rule); + +describe('publishPackageDrafts judges each draft against the BATCH closure (#10377)', () => { + let warn: ReturnType; + beforeEach(() => { + warn = vi.spyOn(console, 'warn').mockImplementation(() => {}); + delete process.env.OS_ALLOW_UNLINTED_METADATA_WRITES; + }); + afterEach(() => { + warn.mockRestore(); + delete process.env.OS_ALLOW_UNLINTED_METADATA_WRITES; + }); + + // ── datasets: the card's own defect, and the only one that REFUSES ── + + it('publishes a dashboard together with the dataset it binds — dataset drafted FIRST', async () => { + const { engine } = makeStubEngine(); + const protocol = new ObjectStackProtocolImplementation(engine); + await stageDraft(protocol, 'dataset', customerDataset('shyx_customer_ds')); + await stageDraft(protocol, 'dashboard', boardBoundTo('shyx_customer_ds')); + + const res = await protocol.publishPackageDrafts({ packageId: PKG }); + + expect( + res.failed, + `the dataset is IN THIS BATCH. A [widget-dataset-unknown] rollback here means the gate's ` + + `datasets context still carries only ALREADY-LIVE declarations, so a package shipping a ` + + `dashboard together with its dataset can never publish — the #10377 symptom verbatim.`, + ).toEqual([]); + expect(res).toMatchObject({ success: true, publishedCount: 2, failedCount: 0 }); + }); + + it('publishes the same pair with the dashboard drafted FIRST — order-independent', async () => { + // The registry write-through that would make a promoted sibling visible + // runs in Phase 2, AFTER the whole Phase-1 gate+promote transaction. So + // intra-batch order can never be the fix, and this pins that the closure + // — not luck of iteration order — is what resolves the binding. + const { engine } = makeStubEngine(); + const protocol = new ObjectStackProtocolImplementation(engine); + await stageDraft(protocol, 'dashboard', boardBoundTo('shyx_customer_ds')); + await stageDraft(protocol, 'dataset', customerDataset('shyx_customer_ds')); + + const res = await protocol.publishPackageDrafts({ packageId: PKG }); + + expect(res.failed).toEqual([]); + expect(res).toMatchObject({ success: true, publishedCount: 2, failedCount: 0 }); + }); + + it('STILL rolls back a dashboard bound to a genuinely absent dataset', async () => { + // ⭐ The boundary in the other direction: the closure is the package's + // own drafts plus the live universe, NOT "anything goes". A name that is + // in neither place is still the #7529 refusal, with its located path. + const { engine } = makeStubEngine(); + const protocol = new ObjectStackProtocolImplementation(engine); + await stageDraft(protocol, 'dataset', customerDataset('shyx_customer_ds')); + await stageDraft(protocol, 'dashboard', boardBoundTo('no_such_dataset_xyz')); + + const res = await protocol.publishPackageDrafts({ packageId: PKG }); + + expect(res).toMatchObject({ success: false, publishedCount: 0 }); + expect(res.published).toEqual([]); + const causal = res.failed.find((f) => f.name === 'customer_dashboard')!; + expect(causal.code).toBe('INVALID_METADATA'); + expect(causal.error).toMatch(/widget-dataset-unknown/); + expect(causal.error).toMatch(/no_such_dataset_xyz/); + // ADR-0067 D2 — all-or-nothing: the healthy sibling is aborted, not + // published around the refusal. + expect(res.failed.find((f) => f.name === 'shyx_customer_ds')?.code).toBe('BATCH_ABORTED'); + }); + + it('refuses a lone dashboard whose dataset is nowhere — no batch, same verdict', async () => { + const { engine } = makeStubEngine(); + const protocol = new ObjectStackProtocolImplementation(engine); + await stageDraft(protocol, 'dashboard', boardBoundTo('shyx_customer_ds')); + + const res = await protocol.publishPackageDrafts({ packageId: PKG }); + + expect(res).toMatchObject({ success: false, publishedCount: 0 }); + expect(res.failed[0]!.code).toBe('INVALID_METADATA'); + expect(res.failed[0]!.error).toMatch(/widget-dataset-unknown/); + }); + + // ── objects: the same gap at advisory severity ── + + it('a flow bound to a same-batch OBJECT draft raises no phantom trigger advisory', async () => { + const { engine } = makeStubEngine(); + const protocol = new ObjectStackProtocolImplementation(engine); + await stageDraft(protocol, 'object', detailObject('shyx_ticket')); + await stageDraft(protocol, 'flow', flowOn('shyx_on_ticket', 'shyx_ticket')); + + const res = await protocol.publishPackageDrafts({ packageId: PKG }); + + expect(res.failed).toEqual([]); + expect( + rulesOf(res, 'shyx_on_ticket'), + `'shyx_ticket' is drafted in THIS batch, so the flow's trigger object resolves. A ` + + `[flow-trigger-unknown-object] here is the datasets defect at advisory severity: it does ` + + `not refuse, it describes nothing.`, + ).not.toContain('flow-trigger-unknown-object'); + }); + + it('STILL reports a flow bound to an object that is in neither the batch nor the registry', async () => { + const { engine } = makeStubEngine(); + const protocol = new ObjectStackProtocolImplementation(engine); + await stageDraft(protocol, 'object', detailObject('shyx_ticket')); + await stageDraft(protocol, 'flow', flowOn('shyx_on_ghost', 'shyx_ghost')); + + const res = await protocol.publishPackageDrafts({ packageId: PKG }); + + expect(res.failed).toEqual([]); + expect(rulesOf(res, 'shyx_on_ghost')).toContain('flow-trigger-unknown-object'); + }); + + // ── permissions: same gap, reached through an object publish ── + + it('an object granted by a same-batch PERMISSION draft raises no phantom ungranted advisory', async () => { + const { engine } = makeStubEngine(); + const protocol = new ObjectStackProtocolImplementation(engine); + await stageDraft(protocol, 'object', detailObject('shyx_ticket')); + await stageDraft(protocol, 'permission', permissionGranting('shyx_agent', 'shyx_ticket')); + + const res = await protocol.publishPackageDrafts({ packageId: PKG }); + + expect(res.failed).toEqual([]); + expect( + rulesOf(res, 'shyx_ticket'), + `the grant is IN THIS BATCH. A [security-master-detail-ungranted] here is the ` + + `permissions half of the same closure gap — the per-write phantom class PR #7886 ` + + `already paid for on the live collection.`, + ).not.toContain('security-master-detail-ungranted'); + }); + + it('STILL reports a master-detail object no permission set in the batch grants', async () => { + const { engine } = makeStubEngine(); + const protocol = new ObjectStackProtocolImplementation(engine); + await stageDraft(protocol, 'object', detailObject('shyx_ticket')); + await stageDraft(protocol, 'permission', permissionGranting('shyx_agent', 'shyx_other')); + + const res = await protocol.publishPackageDrafts({ packageId: PKG }); + + expect(res.failed).toEqual([]); + expect(rulesOf(res, 'shyx_ticket')).toContain('security-master-detail-ungranted'); + }); +}); diff --git a/packages/metadata-protocol/src/protocol.ts b/packages/metadata-protocol/src/protocol.ts index 4ad5b87ac8..ff8c6f149d 100644 --- a/packages/metadata-protocol/src/protocol.ts +++ b/packages/metadata-protocol/src/protocol.ts @@ -12,7 +12,11 @@ import { readEnvWithDeprecation, resolveTenancyPosture, resolveThrownHttpError } import { postureEnforcesWall } from '@objectstack/spec/security'; import type { MetadataHostEngine } from './host-engine.js'; import { omitInternalFieldsFromWriteResponse } from './write-response-internal-fields.js'; -import { evaluateRuntimeAuthoringGate } from './runtime-authoring-gate.js'; +import { + evaluateRuntimeAuthoringGate, + CLOSURE_CONTEXT_KEY_BY_TYPE, + type RuntimePendingDeclarations, +} from './runtime-authoring-gate.js'; // [#7560] ADR-0070's read-only-package rule, shared with the `/packages` // lifecycle gate in `@objectstack/runtime` — see `./package-writability.js`. import { isWritablePackage as isWritablePackageShared } from './package-writability.js'; @@ -3920,6 +3924,19 @@ export class ObjectStackProtocolImplementation implements * sentinel ⇒ nothing is narrowed. */ packageId?: string | null; + /** + * [#10377] The declarations this write's own BATCH is publishing + * alongside it, when the caller HAS a batch. Only + * `publishPackageDrafts` does; every other door writes one item, so its + * closure is the live universe and it states nothing here. + * + * Threaded rather than gathered: the batch's pending drafts are + * `sys_metadata` rows the caller has already listed and is about to + * consume, and re-deriving them from this side would mean guessing + * WHICH batch a write belongs to — which is the question the caller is + * the only one holding the answer to. + */ + pending?: RuntimePendingDeclarations; }): RuntimeAuthoringIssue[] { // [#6710] The ADR-0005 carve-out, now DECLARED instead of inferred. // @@ -4020,6 +4037,9 @@ export class ObjectStackProtocolImplementation implements permissions, books, datasets, + // [#10377] The batch's own pending drafts join the four + // collections above. Absent on every non-batch door. + ...(evt.pending !== undefined ? { pending: evt.pending } : {}), ...(packageScope !== undefined ? { packageScope } : {}), ...(evt.organizationId !== undefined ? { organizationId: evt.organizationId } : {}), orgWallEnforced: this.orgWallEnforced(), @@ -14398,6 +14418,17 @@ export class ObjectStackProtocolImplementation implements * the caller actually has a binding to state. */ packageId?: string | null; + /** + * [#10377] The OTHER drafts this promotion is part of a batch with, + * projected into the gate's context collections. Forwarded verbatim to + * {@link assertRuntimeAuthoringRules} — this method computes nothing + * from it and holds no opinion about its contents. + * + * Stated by `publishPackageDrafts` (a package publishes as a unit); + * absent on the `publishMetaItem` path, whose batch is one item, so a + * "same-batch" closure would be the item itself and change nothing. + */ + pending?: RuntimePendingDeclarations; }): Promise<{ singularType: string; orgId: string | null; @@ -14502,6 +14533,13 @@ export class ObjectStackProtocolImplementation implements // fires while looking like it does. Filed rather than widened // here, because `MetadataItem` is a `packages/spec` contract. ...(request.packageId !== undefined ? { packageId: request.packageId } : {}), + // [#10377] The batch's own pending drafts, when this promotion + // is part of one. The package closure above says WHICH packages + // this write may resolve against; this says which of its own + // package's declarations are in flight beside it — two + // different narrowings, both needed for a package to be + // judged as a self-consistent unit. + ...(request.pending !== undefined ? { pending: request.pending } : {}), }) : []; @@ -14805,6 +14843,82 @@ export class ObjectStackProtocolImplementation implements return { drafts }; } + /** + * [#10377] The batch's own pending declarations, projected into the gate's + * context collections — the half of the closure that makes a package + * publishable as a SELF-CONSISTENT UNIT. + * + * ## What was broken + * + * `assertRuntimeAuthoringRules` resolves every context collection off + * `engine.registry`, i.e. the LIVE universe. A draft is not in it — the + * write-through runs on `mode: 'publish'` — and the batch's own promotions + * do not put it there either, because `applyRegistryWriteThrough` lives in + * Phase 2, after the Phase-1 transaction in which every draft is gated and + * promoted. So while a batch is being judged NO sibling of that batch is + * visible to any other member's gate pass, in ANY order. Measured + * 2026-08-21 on a cloud rig: a package carrying `dataset/shyx_customer_ds` + * and a `dashboard` whose widget binds it rolled back at the dashboard + * with `[widget-dataset-unknown] … does not resolve to a declared + * dataset`, on every attempt and under both dataset names the author + * tried. The dataset was in the same batch, three rows away. + * + * ## Why the bodies are read here and not inside the loop + * + * The promote DELETES the draft row (`repo.promoteDraft` = active-row put + + * draft delete), so a body read after the first promotion is a body that + * may already be gone — the same reason the seed capture inside Phase 1 + * reads BEFORE its own promote. Reading the whole set up front also makes + * the closure ORDER-INDEPENDENT by construction rather than by luck of + * iteration: every member is judged against the same complete set. + * + * ## Scope, and why no extra filtering is needed + * + * `drafts` is already exactly this package's pending set — `listDrafts` + * narrows by `package_id`, and surfaces env-wide plus own-org rows, which + * is precisely the population the caller is about to promote. So the + * closure is "this package's own declarations", restated from the list the + * batch is defined by, never a second query with a second scope. + * + * ⛔ Read failures PROPAGATE. This runs before Phase 1's transaction, so + * nothing has been written and the publish fails having changed nothing — + * and every row read here is a row `promoteDraftForPublish` is about to + * read again anyway. Swallowing would silently shrink the closure, which + * does not fail open, it manufactures a refusal that names a declaration + * the author can SEE in their own package (ADR-0110 D3: a miss and a fault + * are different facts). + */ + private async collectBatchPendingDeclarations( + drafts: ReadonlyArray<{ type: string; name: string; organizationId: string | null }>, + ): Promise { + const pending: { + objects: unknown[]; permissions: unknown[]; books: unknown[]; datasets: unknown[]; + } = { objects: [], permissions: [], books: [], datasets: [] }; + let any = false; + for (const d of drafts) { + // The canonical fold, same boundary the promote applies: a stored + // manifest-plural spelling must route to the same collection its + // singular does, or one row's shape would decide whether the + // package is judged against itself. + const singular = canonicalMetaType(d.type); + const key = (CLOSURE_CONTEXT_KEY_BY_TYPE as Record)[singular]; + if (!key) continue; + const draftOrgId = d.organizationId ?? null; + const repo = this.getOverlayRepo(draftOrgId); + const ref = { + type: singular, name: d.name, org: draftOrgId ?? 'env', + } as unknown as Parameters[0]; + const draft = await repo.get(ref, { state: 'draft' }); + if (draft?.body === undefined || draft.body === null) continue; + pending[key].push(draft.body); + any = true; + } + // Absent, not empty: `undefined` is what keeps a batch with no + // closure-relevant drafts byte-identical to the pre-#10377 gate call, + // rather than routing it through a merge that would be a no-op. + return any ? pending : undefined; + } + /** * Publish every pending DRAFT bound to a package in one shot (ADR-0033) — * the "publish whole app" action. Promotes each draft→active by reusing the @@ -15153,6 +15267,14 @@ export class ObjectStackProtocolImplementation implements ]; const seedBodies: unknown[] = []; + // [#10377] The closure this batch is judged against, read ONCE and + // BEFORE any promotion (a promote deletes the draft row it reads). + // Every member of the batch is then gated against the same complete + // set, which is what makes a dashboard-plus-its-dataset package + // publishable in either order — see + // {@link collectBatchPendingDeclarations}. + const pendingDeclarations = await this.collectBatchPendingDeclarations(drafts); + // ADR-0067 — capture each artifact's PRE-publish state so this turn can // be recorded as ONE revertible commit. existedBefore=false → the commit // creates it (revert = soft-remove); true → it edits an existing artifact @@ -15340,6 +15462,12 @@ export class ObjectStackProtocolImplementation implements // `d.packageId` is the row's own binding, so this // is the listed key restated, never a new one. packageId: d.packageId, + // [#10377] The whole batch's own declarations, so + // this member is judged against the package it is + // being published AS PART OF and not against the + // universe as it stood before the publish started. + ...(pendingDeclarations !== undefined + ? { pending: pendingDeclarations } : {}), ...(request.actor ? { actor: request.actor } : {}), message: `publish app package '${request.packageId}'`, }); diff --git a/packages/metadata-protocol/src/runtime-authoring-gate.ts b/packages/metadata-protocol/src/runtime-authoring-gate.ts index da1c3dbdda..bdecfe1fde 100644 --- a/packages/metadata-protocol/src/runtime-authoring-gate.ts +++ b/packages/metadata-protocol/src/runtime-authoring-gate.ts @@ -50,6 +50,7 @@ import { runRuntimeAuthoringRules, type AuthoringFinding, type RuntimePackageScope, + type RuntimeStackContext, } from '@objectstack/lint/runtime'; // The ONE declaration of "which `config` key on which container node type holds // a nested region" (#4401, `spec/src/automation/region-slots.ts`). Four passes @@ -338,6 +339,99 @@ export function findPlatformScheduleOrgGaps(args: { return issues; } +// ───────────────────────────────────────────────────────────────────────────── +// #10377 — the batch's OWN pending drafts are part of the closure it is judged +// against. +// ───────────────────────────────────────────────────────────────────────────── + +/** + * The declarations a package is publishing IN THIS BATCH, keyed exactly like + * the live resolution context they join. + * + * ## The defect this exists for + * + * The gate's context collections are read off `engine.registry` — the LIVE + * universe. A draft is deliberately not in that registry (the write-through + * runs on `mode: 'publish'`), and the batch door's own promotions do not put + * it there either: `applyRegistryWriteThrough` runs in Phase 2, AFTER the + * Phase-1 transaction that gates and promotes every draft. So while a batch is + * being judged, NO sibling of the batch exists in any collection — measured + * 2026-08-21 on a cloud rig as `[widget-dataset-unknown] dataset + * "shyx_customer_ds" does not resolve`, with the dataset sitting in the very + * same batch. A package shipping a dashboard together with its dataset could + * never publish, and no intra-batch ordering could help, because the registry + * is not written until the whole transaction has committed. + * + * ## Why the type is `RuntimeStackContext` rather than a new shape + * + * It is the SAME set of collections, resolved from a second source. Reusing + * the declaration is what stops the two from drifting when #8309's "widening + * the snapshot is a one-key edit" note is next acted on: a key added there + * arrives here typed, and {@link CLOSURE_CONTEXT_KEY_BY_TYPE} below is the + * only thing that then needs a decision. + */ +export type RuntimePendingDeclarations = RuntimeStackContext; + +/** + * Which context collection a pending draft of a given metadata type joins. + * + * The `satisfies` clause is the drift guard, not decoration: rename a key of + * `RuntimeStackContext` in `@objectstack/lint` and this table stops compiling, + * instead of silently routing a collection nowhere — the #4449 + * wired-onto-nothing shape that `TYPE_TO_STACK_KEY`'s own `seed: 'data'` note + * records paying for. + * + * Every metadata type NOT listed here contributes nothing to the closure, and + * that is the correct answer rather than a gap: a collection is carried + * because some rule RESOLVES REFERENCES INTO IT, and only these four are read + * that way (`RuntimeStackContext`'s own docblock records the measurement). + */ +export const CLOSURE_CONTEXT_KEY_BY_TYPE = { + object: 'objects', + permission: 'permissions', + book: 'books', + dataset: 'datasets', +} as const satisfies Readonly>; + +/** + * The live collection with this batch's pending drafts folded in — REPLACING + * by name, never appended beside. + * + * Replace-not-erase is the same rule `buildRuntimeWriteSnapshots` already + * applies when a written item lands in its own context collection, and for the + * same reason: a draft that EDITS a live declaration is one declaration in two + * states, so appending it would make an update read as a duplicate name — for + * `objects` that turns every lookup in the tenant's model into an ambiguity, + * and for `permissions` it double-counts grants. + * + * ⛔ The direction is deliberately additive: a pending draft can only ever make + * MORE names resolvable, never fewer. A name in neither the batch nor the live + * universe is still unresolved, which is what keeps the #7529 refusal — the + * one this change must not weaken — intact for a genuinely dangling binding. + * + * Pure and total: an entry that is not an object, or carries no usable `name`, + * is kept rather than inspected. + */ +export function mergePendingDeclarations( + live: readonly unknown[], + pending: readonly unknown[] | undefined, +): readonly unknown[] { + if (!pending || pending.length === 0) return live; + const supersededNames = new Set(); + for (const entry of pending) { + if (!isRec(entry)) continue; + const name = entry.name; + if (typeof name === 'string' && name !== '') supersededNames.add(name); + } + if (supersededNames.size === 0) return [...live, ...pending]; + const kept = live.filter((entry) => { + if (!isRec(entry)) return true; + const name = entry.name; + return !(typeof name === 'string' && supersededNames.has(name)); + }); + return [...kept, ...pending]; +} + const toIssue = (f: AuthoringFinding): RuntimeAuthoringIssue => ({ rule: f.rule, path: f.path, @@ -426,6 +520,22 @@ export function evaluateRuntimeAuthoringGate(args: { * so the thread-through is load-bearing, not optional. */ datasets?: readonly unknown[]; + /** + * [#10377] The declarations this write's own BATCH is publishing alongside + * it — folded into the four collections above by + * {@link mergePendingDeclarations} before any rule runs. + * + * Stated by the batch door (`publishPackageDrafts`), which is the only + * caller that HAS a batch; the single-item door publishes one item, so its + * batch is itself and it passes nothing. Absent ⇒ the closure is the live + * universe alone, exactly as before, which is the correct answer for every + * write that is not part of a package publish rather than a fallback. + * + * See {@link RuntimePendingDeclarations} for the measured defect: without + * it a package shipping a dashboard together with its dataset can never + * publish. + */ + pending?: RuntimePendingDeclarations; /** ADR-0080 SDUI manifest when the host has one. */ sduiManifest?: unknown; /** @@ -474,11 +584,17 @@ export function evaluateRuntimeAuthoringGate(args: { type: args.type, item: args.body, ...(args.packageScope !== undefined ? { packageScope: args.packageScope } : {}), + // [#10377] Live universe + this batch's own pending drafts, folded per + // collection. Uniform across all four on purpose: the closure ruling + // judges a package as a self-consistent UNIT, and a per-collection + // closure is precisely the state that produced this card — `objects` + // had been threaded, `datasets` had not, and the difference was + // invisible until an error-severity rule landed on the un-threaded one. context: { - objects: args.objects ?? [], - permissions: args.permissions ?? [], - books: args.books ?? [], - datasets: args.datasets ?? [], + objects: mergePendingDeclarations(args.objects ?? [], args.pending?.objects), + permissions: mergePendingDeclarations(args.permissions ?? [], args.pending?.permissions), + books: mergePendingDeclarations(args.books ?? [], args.pending?.books), + datasets: mergePendingDeclarations(args.datasets ?? [], args.pending?.datasets), }, ...(args.sduiManifest !== undefined ? { sduiManifest: args.sduiManifest } : {}), }); diff --git a/scripts/engine-double-contract.pinned.json b/scripts/engine-double-contract.pinned.json index 2803ec6e7d..f461c6d8ee 100644 --- a/scripts/engine-double-contract.pinned.json +++ b/scripts/engine-double-contract.pinned.json @@ -66,6 +66,16 @@ "verb": "update", "pinned": 1 }, + { + "file": "packages/metadata-protocol/src/protocol-publish-drafts-closure.test.ts", + "verb": "delete", + "pinned": 1 + }, + { + "file": "packages/metadata-protocol/src/protocol-publish-drafts-closure.test.ts", + "verb": "update", + "pinned": 1 + }, { "file": "packages/metadata-protocol/src/protocol-publish-drafts-endpoint-gate.test.ts", "verb": "delete", From 9caa98fd1859d8cb33a8c93e425e1e4da3628fa3 Mon Sep 17 00:00:00 2001 From: Jack Zhuang <277994282+os-zhuang@users.noreply.github.com> Date: Fri, 21 Aug 2026 02:05:14 +0800 Subject: [PATCH 2/3] fix(metadata-protocol): degrade the batch closure on a repository that declares no `get` Collecting the batch's pending declarations introduced the batch door's FIRST dependency on `repo.get`. `SysMetadataRepository` has always had it, but `getOverlayRepo` is the seam every publish double replaces, and the door's only previous body read -- the `seed` capture -- fires solely when a seed draft is in the batch, so a repository shape without `get` had never been asked for one. Measured: nine `publishPackageDrafts` cases in `@objectstack/objectql` drive a double declaring `listDrafts` alone, and the new read turned every one of them into `TypeError: repo.get is not a function`, thrown BEFORE any promotion -- the batch door dying on a shape it used to accept. Answered with a declared capability check rather than a wider `try`: a missing member is a fact about the repository, knowable up front, and it must not read like a failed read. A read failure on a repository that HAS the member still propagates. Degrading returns the closure to its pre-#10377 state (the live universe alone), which is the safe direction -- the gate keeps judging and can only be MORE strict, never fail open -- and it is all-or-nothing, so the verdict cannot depend on which org a draft happens to live in. The degrade SAYS WHY, once per process: which member was missing, where it was first reached, and that a same-batch sibling reference may therefore be refused. A silent fallback is the shape that reads as "clean" from every surface downstream. The batch's existing enumeration cannot supply the bodies: `listDrafts` is a declared header projection whose other caller is the console's pending-changes list, and the doubles that lack `repo.get` stub `listDrafts` too -- so widening it would degrade silently instead of avoiding the guard. Regression test pins that a minimal repo double publishes a batch and that the degrade is announced. Part of #10377 Co-Authored-By: Claude Fable 5 --- .../protocol-publish-drafts-closure.test.ts | 40 +++++++++ packages/metadata-protocol/src/protocol.ts | 81 ++++++++++++++++++- 2 files changed, 117 insertions(+), 4 deletions(-) diff --git a/packages/metadata-protocol/src/protocol-publish-drafts-closure.test.ts b/packages/metadata-protocol/src/protocol-publish-drafts-closure.test.ts index 6ec30d0810..681e6c31c2 100644 --- a/packages/metadata-protocol/src/protocol-publish-drafts-closure.test.ts +++ b/packages/metadata-protocol/src/protocol-publish-drafts-closure.test.ts @@ -398,6 +398,46 @@ describe('publishPackageDrafts judges each draft against the BATCH closure (#103 expect(res.failed[0]!.error).toMatch(/widget-dataset-unknown/); }); + // ── The repository-shape guard: a minimal double must still publish ── + + it('publishes through a repo double declaring only `listDrafts` — the closure degrades, it never throws', async () => { + // ⭐ The patch-round regression. Collecting the batch's pending + // declarations introduced the batch door's FIRST dependency on + // `repo.get`; `getOverlayRepo` is the seam every publish double + // replaces, and nine cases in `@objectstack/objectql` drive a double + // that declares `listDrafts` alone. Unguarded, this door answered + // `TypeError: repo.get is not a function` BEFORE any promotion — a + // shape it used to accept, now fatal. + const { engine } = makeStubEngine(); + const protocol: any = new ObjectStackProtocolImplementation(engine); + protocol.ensureOverlayIndex = async () => {}; + protocol.getOverlayRepo = () => ({ + listDrafts: async () => [ + { type: 'dataset', name: 'shyx_customer_ds', organizationId: null, packageId: PKG }, + ], + }); + protocol.runPublishSideEffects = async () => ({}); + vi.spyOn(protocol, 'promoteDraftForPublish').mockImplementation(async (req: any) => ({ + singularType: req.type, + orgId: null, + advisories: [], + result: { version: 'h', seq: 1, item: { body: { name: req.name } }, packageId: null }, + })); + + const res = await protocol.publishPackageDrafts({ packageId: PKG }); + + expect(res).toMatchObject({ success: true, publishedCount: 1, failedCount: 0 }); + expect(res.failed).toEqual([]); + + // ⛔ And the degrade SAYS WHY. A gate that quietly stops seeing part of + // its input reads as "clean" from every surface downstream, so the + // missing member and the consequence are both named — a bare silent + // fallback would be the defect this assertion exists to forbid. + const said = warn.mock.calls.map((c) => String(c[0])).join('\n'); + expect(said).toContain("declares no 'get'"); + expect(said).toMatch(/LIVE declarations only/); + }); + // ── objects: the same gap at advisory severity ── it('a flow bound to a same-batch OBJECT draft raises no phantom trigger advisory', async () => { diff --git a/packages/metadata-protocol/src/protocol.ts b/packages/metadata-protocol/src/protocol.ts index ff8c6f149d..97d62f02d7 100644 --- a/packages/metadata-protocol/src/protocol.ts +++ b/packages/metadata-protocol/src/protocol.ts @@ -14880,13 +14880,27 @@ export class ObjectStackProtocolImplementation implements * closure is "this package's own declarations", restated from the list the * batch is defined by, never a second query with a second scope. * - * ⛔ Read failures PROPAGATE. This runs before Phase 1's transaction, so - * nothing has been written and the publish fails having changed nothing — + * ⛔ Read failures PROPAGATE — a repository that HAS the read and fails it + * is a fault, not a miss. This runs before Phase 1's transaction, so + * nothing has been written and the publish fails having changed nothing, * and every row read here is a row `promoteDraftForPublish` is about to * read again anyway. Swallowing would silently shrink the closure, which - * does not fail open, it manufactures a refusal that names a declaration + * does not fail open — it manufactures a refusal that names a declaration * the author can SEE in their own package (ADR-0110 D3: a miss and a fault - * are different facts). + * are different facts). The MISSING-member case is the other fact and is + * handled below, loudly and without a throw. + * + * ## Why the batch's existing enumeration cannot supply the bodies + * + * `listDrafts` — the read that DEFINES this batch — is a declared header + * projection: it maps rows to `(type, name, organizationId, packageId, + * updatedAt, updatedBy)` and drops `metadata` on purpose, because its other + * caller is the console's "pending changes" list. Widening it would put + * every draft BODY on that listing, and it would not even remove the guard + * below: the doubles that lack `repo.get` stub `listDrafts` too, so a + * body-carrying projection would hand back headers there anyway — degrading + * SILENTLY instead of degrading with a reason. So the second read stays, + * and the absence of the member it needs is stated rather than assumed. */ private async collectBatchPendingDeclarations( drafts: ReadonlyArray<{ type: string; name: string; organizationId: string | null }>, @@ -14905,6 +14919,37 @@ export class ObjectStackProtocolImplementation implements if (!key) continue; const draftOrgId = d.organizationId ?? null; const repo = this.getOverlayRepo(draftOrgId); + // ── The repository-shape guard, and why it degrades ALL-OR-NOTHING + // + // This method introduced the batch door's first dependency on + // `repo.get`. `SysMetadataRepository` has always had it, but the + // seam is overridable (`getOverlayRepo` is the injection point every + // publish double replaces), and the door's only previous body read — + // the `seed` capture — fires solely when a seed draft is in the + // batch, so a repository shape without `get` had never been asked + // for one. Measured: nine `publishPackageDrafts` cases in + // `@objectstack/objectql` drive a double declaring `listDrafts` + // alone, and this call turned every one of them into + // `TypeError: repo.get is not a function` — thrown BEFORE any + // promotion, so the batch door died on a shape it used to accept. + // + // The answer is a declared capability check, not a wider `try`: a + // missing member is a fact about the repository, knowable up front, + // and it must not read like a failed read. Degrading returns the + // closure to its pre-#10377 state (the live universe alone), which + // is the safe direction — the gate keeps judging and can only be + // MORE strict, never fail open. + // + // ⛔ All-or-nothing on purpose. Bailing out of the whole collection + // rather than skipping this one draft is what keeps the verdict from + // depending on WHICH org a draft happens to live in: a partial + // closure would resolve some of a package's own names and not + // others, which is a third behaviour nobody declared and nobody + // could reproduce. + if (typeof repo.get !== 'function') { + this.warnClosureReadUnavailable(singular, d.name); + return undefined; + } const ref = { type: singular, name: d.name, org: draftOrgId ?? 'env', } as unknown as Parameters[0]; @@ -14919,6 +14964,34 @@ export class ObjectStackProtocolImplementation implements return any ? pending : undefined; } + /** + * [#10377] Say WHY the batch closure degraded — once per process. + * + * A bare degrade is the failure shape this repo has paid for before: a gate + * that quietly stops seeing part of its input reads as "clean", and the + * only symptom is a refusal somewhere else that names a declaration the + * author can see. So the reason is stated, with the member that was + * missing and what the consequence is. + * + * Deduped because Studio republishes the same package repeatedly and a + * repository shape does not change between two publishes — the first line + * carries the whole fact, and the hundredth adds nothing. + */ + private warnClosureReadUnavailable(type: string, name: string): void { + if (this.closureReadWarned) return; + this.closureReadWarned = true; + console.warn( + `[Protocol] publishPackageDrafts: this overlay repository declares no 'get', so the batch's own ` + + `pending drafts cannot be read (first reached at ${type}/${name}). Author-time validation falls ` + + `back to the LIVE declarations only — the pre-#10377 closure — so a draft that references a ` + + `sibling drafted in the SAME batch may be refused as unresolved. Nothing is published unchecked: ` + + `the gate still runs, with strictly less resolution context.`, + ); + } + + /** One warn per process for the {@link warnClosureReadUnavailable} degrade. */ + private closureReadWarned = false; + /** * Publish every pending DRAFT bound to a package in one shot (ADR-0033) — * the "publish whole app" action. Promotes each draft→active by reusing the From 3495477a9f25620c918278c1a7267f9a1d5b9c5f Mon Sep 17 00:00:00 2001 From: Jack Zhuang <277994282+os-zhuang@users.noreply.github.com> Date: Fri, 21 Aug 2026 02:13:01 +0800 Subject: [PATCH 3/3] fix(metadata-protocol): type the degrade-notice assertion's callback The new regression test read `warn.mock.calls.map((c) => ...)`, whose parameter tsc infers as implicit `any` -- one new raw error in the package, which `check:type-check-debt --re-measure` catches as an upward drift of a shrink-only ratchet (63 recorded, 64 measured). Fixed at the source, which is the author's remedy; the ledger entry is untouched. Part of #10377 Co-Authored-By: Claude Fable 5 --- .../src/protocol-publish-drafts-closure.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/metadata-protocol/src/protocol-publish-drafts-closure.test.ts b/packages/metadata-protocol/src/protocol-publish-drafts-closure.test.ts index 681e6c31c2..2136344c9d 100644 --- a/packages/metadata-protocol/src/protocol-publish-drafts-closure.test.ts +++ b/packages/metadata-protocol/src/protocol-publish-drafts-closure.test.ts @@ -433,7 +433,7 @@ describe('publishPackageDrafts judges each draft against the BATCH closure (#103 // its input reads as "clean" from every surface downstream, so the // missing member and the consequence are both named — a bare silent // fallback would be the defect this assertion exists to forbid. - const said = warn.mock.calls.map((c) => String(c[0])).join('\n'); + const said = warn.mock.calls.map((c: unknown[]) => String(c[0])).join('\n'); expect(said).toContain("declares no 'get'"); expect(said).toMatch(/LIVE declarations only/); });