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..2136344c9d --- /dev/null +++ b/packages/metadata-protocol/src/protocol-publish-drafts-closure.test.ts @@ -0,0 +1,502 @@ +// 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/); + }); + + // ── 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: unknown[]) => 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 () => { + 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..97d62f02d7 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,155 @@ 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 — 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 + * the author can SEE in their own package (ADR-0110 D3: a miss and a fault + * 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 }>, + ): 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); + // ── 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]; + 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; + } + + /** + * [#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 @@ -15153,6 +15340,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 +15535,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",