diff --git a/.changeset/publish-drafts-org-scope.md b/.changeset/publish-drafts-org-scope.md new file mode 100644 index 0000000000..be88095c76 --- /dev/null +++ b/.changeset/publish-drafts-org-scope.md @@ -0,0 +1,11 @@ +--- +"@objectstack/metadata-protocol": patch +--- + +Publish/discard package drafts in the draft's own org scope, fixing `no_draft` after saving a draft via Studio. + +Studio "Save Draft" (`PUT /meta/:type/:name?mode=draft`) never threads the session's `activeOrganizationId`, so the draft row is written env-wide (`organization_id = NULL`). "Publish" (`POST /packages/:id/publish-drafts`) resolves the active org and passed it to `promoteDraft`, which looked the draft up with a strict `organization_id = ` equality — so it 404'd (`[no_draft] No pending draft exists …`) on the env-wide row it could never match, even though `listDrafts` had already surfaced that draft to the publish CTA (PR #1852's `$or`). `discardPackageDrafts` had the same latent gap. + +`listDrafts` now projects each draft's own `organizationId`, and `publishPackageDrafts` / `discardPackageDrafts` promote / delete each draft in that scope (env-wide stays env-wide, per-org stays per-org). Seed-body capture and the ADR-0067 revert-plan pre-state read are scoped the same way. + +Fixes #3115. diff --git a/packages/metadata-protocol/src/protocol-publish-drafts-org-scope.test.ts b/packages/metadata-protocol/src/protocol-publish-drafts-org-scope.test.ts new file mode 100644 index 0000000000..0686a4902d --- /dev/null +++ b/packages/metadata-protocol/src/protocol-publish-drafts-org-scope.test.ts @@ -0,0 +1,261 @@ +// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. + +import { describe, expect, it } from 'vitest'; +import { ObjectStackProtocolImplementation } from './protocol.js'; + +/** + * Regression for #3115 — `publish-drafts` fails with `no_draft` after saving a + * draft via Studio UI. + * + * Root cause (a cross-layer org-scope asymmetry): + * + * • Studio's "Save Draft" goes through REST `PUT /meta/:type/:name?mode=draft`, + * which does NOT thread the session's `activeOrganizationId` into + * `saveMetaItem` — so the draft row is written env-wide + * (`organization_id = NULL`). + * • "Publish" goes through `POST /packages/:id/publish-drafts`, which DOES + * resolve `activeOrganizationId` (non-null when the session carries a + * default org) and passes it to `publishPackageDrafts`. + * • `listDrafts` was taught (PR #1852) to surface env-wide drafts to a + * non-null-org caller via `$or [{org}, {org IS NULL}]`, so the Publish CTA + * appears — but `promoteDraft` still looked the draft up with a STRICT + * `organization_id = ` equality and 404'd (`no_draft`) on the + * env-wide row it could never match. + * + * The fix promotes each listed draft in the org scope it actually lives in + * (the scope `listDrafts` surfaced it from), so the pending-changes list and + * the publish path agree. + * + * These tests exercise the REAL `listDrafts` + `promoteDraft` interaction + * against a faithful multi-table stub engine (honours `$or` and + * `organization_id IS NULL`), reproducing the exact save-env-wide / + * publish-under-org mismatch. + */ + +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 — so an env-wide draft and an org-scoped active row for the same identity +// can coexist without colliding. +function keyOf(w: Record) { + return `${w.type}|${w.name}|${w.organization_id ?? '__env__'}|${w.state ?? 'active'}|${w.package_id ?? '__nopkg__'}`; +} + +/** Does row `r` satisfy `where` (top-level eq + `$or` + `organization_id IS NULL`)? */ +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; + } + // `undefined` = "dimension not constrained"; `null` = "must be NULL". + if (v === undefined) continue; + if ((r as any)[k] !== v) return false; + } + return true; +} + +function makeStubEngine() { + 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; + } + // Exact-key lookups (findOne on a fully-qualified ref). When package_id + // is not part of the where (promote's whereFor omits it → "match any + // package"), fall back to a scan. + 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 }) { + 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 }) { + const found = findRow(opts.where); + if (!found) return { deleted: 0 }; + rows.delete(found.key); + return { deleted: 1 }; + }, + async transaction(cb: (ctx: any) => Promise): Promise { + return cb(undefined); + }, + registry: { + registerItem: () => {}, + registerObject: () => {}, + // No declared package namespace → publishPackageDrafts skips the + // ADR-0028 prefix check (legacy-grandfathered path). + getPackage: () => undefined, + }, + }; + return { engine, rows, historyRows }; +} + +const objectBody = (name: string) => ({ + name, + label: 'Project Task', + fields: { + title: { type: 'text', label: 'Title' }, + done: { type: 'boolean', label: 'Done' }, + }, +}); + +describe('publishPackageDrafts — env-wide draft under a non-null active org (#3115)', () => { + it('saves the object draft env-wide (organization_id = NULL) when no org is threaded', async () => { + const { engine, rows } = makeStubEngine(); + const protocol = new ObjectStackProtocolImplementation(engine); + + // Mirrors REST `PUT /meta/object/proj_task?mode=draft&package=app.projects` + // — package bound, but no organizationId threaded. + await protocol.saveMetaItem({ + type: 'object', + name: 'proj_task', + item: objectBody('proj_task'), + packageId: 'app.projects', + mode: 'draft', + }); + + const draftRows = Array.from(rows.values()).filter((r) => r.state === 'draft'); + expect(draftRows).toHaveLength(1); + expect(draftRows[0].organization_id).toBeNull(); + expect(draftRows[0].package_id).toBe('app.projects'); + }); + + it('publishes the env-wide draft even though the session carries a non-null active org', async () => { + const { engine, rows } = makeStubEngine(); + const protocol = new ObjectStackProtocolImplementation(engine); + + // 1. Studio "Save Draft" — env-wide (no active org threaded). + await protocol.saveMetaItem({ + type: 'object', + name: 'proj_task', + item: objectBody('proj_task'), + packageId: 'app.projects', + mode: 'draft', + }); + + // 2. Studio "Publish" — the dispatcher resolved a non-null active org. + const res = await protocol.publishPackageDrafts({ + packageId: 'app.projects', + organizationId: 'org_alpha', + }); + + // Before the fix this returned { success:false, failedCount:1, + // failed:[{ code:'no_draft' }] }. + expect(res.failed).toEqual([]); + expect(res).toMatchObject({ success: true, publishedCount: 1, failedCount: 0 }); + expect(res.published.map((p) => p.name)).toEqual(['proj_task']); + + // The draft was consumed and the active row landed env-wide + // (package-owned metadata is env-level, not per-org). + const remaining = Array.from(rows.values()); + expect(remaining.filter((r) => r.state === 'draft')).toHaveLength(0); + const active = remaining.filter((r) => r.state === 'active'); + expect(active).toHaveLength(1); + expect(active[0].organization_id).toBeNull(); + }); + + it('still publishes an org-scoped draft under that same org (no regression)', async () => { + const { engine, rows } = makeStubEngine(); + const protocol = new ObjectStackProtocolImplementation(engine); + + // A per-org overlay draft (organization_id = org_alpha). + await protocol.saveMetaItem({ + type: 'object', + name: 'proj_task', + item: objectBody('proj_task'), + organizationId: 'org_alpha', + packageId: 'app.projects', + mode: 'draft', + }); + + const res = await protocol.publishPackageDrafts({ + packageId: 'app.projects', + organizationId: 'org_alpha', + }); + + expect(res).toMatchObject({ success: true, publishedCount: 1, failedCount: 0 }); + const active = Array.from(rows.values()).filter((r) => r.state === 'active'); + expect(active).toHaveLength(1); + expect(active[0].organization_id).toBe('org_alpha'); + }); +}); diff --git a/packages/metadata-protocol/src/protocol.ts b/packages/metadata-protocol/src/protocol.ts index 5a83e8f5ba..aa92cbc714 100644 --- a/packages/metadata-protocol/src/protocol.ts +++ b/packages/metadata-protocol/src/protocol.ts @@ -4737,6 +4737,7 @@ export class ObjectStackProtocolImplementation implements ObjectStackProtocol { drafts: Array<{ type: string; name: string; + organizationId: string | null; packageId: string | null; updatedAt: string | null; updatedBy: string | null; @@ -4860,8 +4861,12 @@ export class ObjectStackProtocolImplementation implements ObjectStackProtocol { const commitItems: Array<{ type: string; name: string; existedBefore: boolean; prevVersion: number | null }> = []; for (const d of ordered) { try { + // Read the pre-publish active row in the draft's OWN scope + // (env-wide drafts have env-wide active rows). Using the + // request's active org here would miss an env-wide edit and + // mis-record it as a create in the revert plan (#3115). const activeRow = (await this.engine.findOne('sys_metadata', { - where: { organization_id: orgId, type: d.type, name: d.name, state: 'active' }, + where: { organization_id: d.organizationId ?? null, type: d.type, name: d.name, state: 'active' }, })) as { version?: number } | null; commitItems.push({ type: d.type, @@ -4911,19 +4916,29 @@ export class ObjectStackProtocolImplementation implements ObjectStackProtocol { await inTxn(async () => { for (const d of ordered) { try { + // Promote each draft in the scope `listDrafts` surfaced + // it from (#3115). Studio/package authoring writes the + // draft env-wide (`organization_id = NULL`) while the + // publishing session may carry a non-null active org; + // `listDrafts` includes those env-wide rows via its `$or`, + // so the promote MUST target the draft's own org or it + // 404s (`no_draft`) on a row it can never match. + const draftOrgId = d.organizationId ?? null; if (d.type === 'seed') { // Capture the body BEFORE promote (the draft row is // deleted by the promote, and a post-publish read-back // has org-scope resolution pitfalls — reading the - // draft is unambiguous). - const ref = { type: d.type, name: d.name, org: orgId ?? 'env' } as unknown as Parameters[0]; - const draft = await repo.get(ref, { state: 'draft' }); + // draft is unambiguous). Read from the draft's own + // scope, not the request's active org. + const seedRepo = this.getOverlayRepo(draftOrgId); + const ref = { type: d.type, name: d.name, org: draftOrgId ?? 'env' } as unknown as Parameters[0]; + const draft = await seedRepo.get(ref, { state: 'draft' }); if (draft?.body) seedBodies.push(draft.body); } const { singularType, result } = await this.promoteDraftForPublish({ type: d.type, name: d.name, - ...(request.organizationId ? { organizationId: request.organizationId } : {}), + ...(draftOrgId ? { organizationId: draftOrgId } : {}), ...(request.actor ? { actor: request.actor } : {}), message: `publish app package '${request.packageId}'`, }); @@ -5123,11 +5138,16 @@ export class ObjectStackProtocolImplementation implements ObjectStackProtocol { for (const d of drafts) { try { + // Discard the draft in the scope it lives in (#3115). Like + // publish, `listDrafts` surfaces env-wide drafts to a non-null + // active org via `$or`; deleting under the request's active org + // would silently no-op on those env-wide rows. + const draftOrgId = d.organizationId ?? null; await this.deleteMetaItem({ type: d.type, name: d.name, state: 'draft', - ...(request.organizationId ? { organizationId: request.organizationId } : {}), + ...(draftOrgId ? { organizationId: draftOrgId } : {}), ...(request.actor ? { actor: request.actor } : {}), }); discarded.push({ type: d.type, name: d.name }); diff --git a/packages/metadata-protocol/src/sys-metadata-repository.ts b/packages/metadata-protocol/src/sys-metadata-repository.ts index b8fa5339ba..8301e9f429 100644 --- a/packages/metadata-protocol/src/sys-metadata-repository.ts +++ b/packages/metadata-protocol/src/sys-metadata-repository.ts @@ -727,6 +727,14 @@ export class SysMetadataRepository implements MetadataRepository { Array<{ type: string; name: string; + /** + * The scope the draft actually lives in — `null` for an env-wide draft, + * a string for a per-org overlay draft. The `$or` below surfaces BOTH to + * a non-null-org caller, so consumers that then act on a draft (promote / + * discard) MUST route the write to THIS scope, not the caller's active + * org, or they 404 on the env-wide row they can never match (#3115). + */ + organizationId: string | null; packageId: string | null; updatedAt: string | null; updatedBy: string | null; @@ -756,6 +764,7 @@ export class SysMetadataRepository implements MetadataRepository { return (rows as any[]).map((row) => ({ type: row.type, name: row.name, + organizationId: row.organization_id ?? null, packageId: row.package_id ?? null, updatedAt: row.updated_at ?? row.created_at ?? null, updatedBy: row.updated_by ?? row.created_by ?? null, diff --git a/packages/objectql/src/sys-metadata-repository-list-drafts.test.ts b/packages/objectql/src/sys-metadata-repository-list-drafts.test.ts index b175626a42..f5f7891d4a 100644 --- a/packages/objectql/src/sys-metadata-repository-list-drafts.test.ts +++ b/packages/objectql/src/sys-metadata-repository-list-drafts.test.ts @@ -75,8 +75,14 @@ describe('SysMetadataRepository.listDrafts (ADR-0033)', () => { { type: 'object', name: 'other_org', state: 'draft', package_id: 'p', organization_id: 'org_other', updated_at: 't3' }, ]; const { repo } = makeRepo(rows, 'org_acme'); - const names = (await repo.listDrafts()).map((d) => d.name).sort(); - expect(names).toEqual(['env_wide', 'org_scoped']); + const out = await repo.listDrafts(); + expect(out.map((d) => d.name).sort()).toEqual(['env_wide', 'org_scoped']); + // Each draft is projected with the scope it actually lives in, so the + // publish/discard paths can promote/delete it in THAT scope rather than the + // caller's active org — otherwise the env-wide row 404s as `no_draft` + // (#3115). This projection is the contract those callers depend on. + expect(out.find((d) => d.name === 'env_wide')?.organizationId).toBeNull(); + expect(out.find((d) => d.name === 'org_scoped')?.organizationId).toBe('org_acme'); }); it('filters by type', async () => {