fix(metadata): publish/discard package drafts in the draft's own org scope (#3115)

.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 = <org>` 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.

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 = <org>` 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<string, unknown>) {
+    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<string, unknown>): boolean {
+    for (const [k, v] of Object.entries(where)) {
+        if (k === '$or') {
+            const clauses = v as Array<Record<string, unknown>>;
+            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<string, Row>();
+    const historyRows: HistoryRow[] = [];
+    let nextId = 0;
+
+    const findRow = (w: Record<string, unknown>): { 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<string, unknown>): 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<string, unknown> }) {
+            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<string, unknown> }) {
+            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<string, unknown>) {
+            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<string, unknown>, opts: { where: Record<string, unknown> }) {
+            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<string, unknown> }) {
+            const found = findRow(opts.where);
+            if (!found) return { deleted: 0 };
+            rows.delete(found.key);
+            return { deleted: 1 };
+        },
+        async transaction<T>(cb: (ctx: any) => Promise<T>): Promise<T> {
+            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');
+    });
+});

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<typeof repo.get>[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<typeof seedRepo.get>[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 });

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,

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 () => {