Skip to content

Commit b8a21ad

Browse files
os-zhuangclaude
andauthored
fix(metadata): publish/discard package drafts in the draft's own org scope (#3115) (#3156)
* fix(metadata): publish/discard package drafts in the draft's own org scope (#3115) Studio "Save Draft" goes through REST `PUT /meta/:type/:name?mode=draft`, which never threads 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 the active org (non-null once a default org is stamped on the session). PR #1852 taught `listDrafts` to surface those env-wide drafts to a non-null active org via `$or [{org}, {org IS NULL}]` — so the Publish CTA appears — but the publish path (`promoteDraft`) still looked the draft up with a STRICT `organization_id = <org>` equality and 404'd (`[no_draft] No pending draft exists …`) on the env-wide row it could never match. `discardPackageDrafts` had the identical latent gap (delete under the request org silently no-ops on env-wide rows). Fix: `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) — so the pending-changes list and the publish/discard paths agree. Seed-body capture and the ADR-0067 revert-plan pre-state read are scoped the same way. Regression tests reproduce the exact `no_draft` error (env-wide draft + non-null publish org) and assert publish now succeeds env-wide, plus a no-regression case for a genuinely org-scoped draft. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * chore(changeset): metadata-protocol patch for publish-drafts org-scope fix (#3115) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Jack Zhuang <277994282+os-zhuang@users.noreply.github.com> Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
1 parent 3a18b60 commit b8a21ad

5 files changed

Lines changed: 315 additions & 8 deletions

File tree

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
---
2+
"@objectstack/metadata-protocol": patch
3+
---
4+
5+
Publish/discard package drafts in the draft's own org scope, fixing `no_draft` after saving a draft via Studio.
6+
7+
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.
8+
9+
`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.
10+
11+
Fixes #3115.
Lines changed: 261 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,261 @@
1+
// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
import { describe, expect, it } from 'vitest';
4+
import { ObjectStackProtocolImplementation } from './protocol.js';
5+
6+
/**
7+
* Regression for #3115 — `publish-drafts` fails with `no_draft` after saving a
8+
* draft via Studio UI.
9+
*
10+
* Root cause (a cross-layer org-scope asymmetry):
11+
*
12+
* • Studio's "Save Draft" goes through REST `PUT /meta/:type/:name?mode=draft`,
13+
* which does NOT thread the session's `activeOrganizationId` into
14+
* `saveMetaItem` — so the draft row is written env-wide
15+
* (`organization_id = NULL`).
16+
* • "Publish" goes through `POST /packages/:id/publish-drafts`, which DOES
17+
* resolve `activeOrganizationId` (non-null when the session carries a
18+
* default org) and passes it to `publishPackageDrafts`.
19+
* • `listDrafts` was taught (PR #1852) to surface env-wide drafts to a
20+
* non-null-org caller via `$or [{org}, {org IS NULL}]`, so the Publish CTA
21+
* appears — but `promoteDraft` still looked the draft up with a STRICT
22+
* `organization_id = <org>` equality and 404'd (`no_draft`) on the
23+
* env-wide row it could never match.
24+
*
25+
* The fix promotes each listed draft in the org scope it actually lives in
26+
* (the scope `listDrafts` surfaced it from), so the pending-changes list and
27+
* the publish path agree.
28+
*
29+
* These tests exercise the REAL `listDrafts` + `promoteDraft` interaction
30+
* against a faithful multi-table stub engine (honours `$or` and
31+
* `organization_id IS NULL`), reproducing the exact save-env-wide /
32+
* publish-under-org mismatch.
33+
*/
34+
35+
interface Row {
36+
id: string;
37+
type: string;
38+
name: string;
39+
organization_id: string | null;
40+
package_id: string | null;
41+
state: string;
42+
metadata: string;
43+
checksum?: string;
44+
version?: number;
45+
updated_at?: string;
46+
created_at?: string;
47+
}
48+
49+
interface HistoryRow {
50+
id: string;
51+
event_seq: number;
52+
name: string;
53+
type: string;
54+
version: number;
55+
operation_type: string;
56+
metadata: string | null;
57+
checksum: string | null;
58+
previous_checksum: string | null;
59+
change_note?: string | null;
60+
source?: string | null;
61+
organization_id: string | null;
62+
recorded_by?: string | null;
63+
recorded_at: string;
64+
}
65+
66+
// Overlay rows are keyed by (type, name, org, state, package_id) — the ADR-0048
67+
// key — so an env-wide draft and an org-scoped active row for the same identity
68+
// can coexist without colliding.
69+
function keyOf(w: Record<string, unknown>) {
70+
return `${w.type}|${w.name}|${w.organization_id ?? '__env__'}|${w.state ?? 'active'}|${w.package_id ?? '__nopkg__'}`;
71+
}
72+
73+
/** Does row `r` satisfy `where` (top-level eq + `$or` + `organization_id IS NULL`)? */
74+
function matchesMetadataWhere(r: Row, where: Record<string, unknown>): boolean {
75+
for (const [k, v] of Object.entries(where)) {
76+
if (k === '$or') {
77+
const clauses = v as Array<Record<string, unknown>>;
78+
if (!clauses.some((c) => matchesMetadataWhere(r, c))) return false;
79+
continue;
80+
}
81+
// `undefined` = "dimension not constrained"; `null` = "must be NULL".
82+
if (v === undefined) continue;
83+
if ((r as any)[k] !== v) return false;
84+
}
85+
return true;
86+
}
87+
88+
function makeStubEngine() {
89+
const rows = new Map<string, Row>();
90+
const historyRows: HistoryRow[] = [];
91+
let nextId = 0;
92+
93+
const findRow = (w: Record<string, unknown>): { key: string; row: Row } | null => {
94+
if (w.id !== undefined) {
95+
for (const [k, r] of rows) if (r.id === w.id) return { key: k, row: r };
96+
return null;
97+
}
98+
// Exact-key lookups (findOne on a fully-qualified ref). When package_id
99+
// is not part of the where (promote's whereFor omits it → "match any
100+
// package"), fall back to a scan.
101+
if (w.package_id !== undefined) {
102+
const k = keyOf(w);
103+
const r = rows.get(k);
104+
return r ? { key: k, row: r } : null;
105+
}
106+
for (const [k, r] of rows) if (matchesMetadataWhere(r, w)) return { key: k, row: r };
107+
return null;
108+
};
109+
110+
const matchesHistory = (h: HistoryRow, w: Record<string, unknown>): boolean => {
111+
if (w.organization_id !== undefined && h.organization_id !== w.organization_id) return false;
112+
if (w.type !== undefined && h.type !== w.type) return false;
113+
if (w.name !== undefined && h.name !== w.name) return false;
114+
if (w.version !== undefined && h.version !== w.version) return false;
115+
if (w.operation_type !== undefined && h.operation_type !== w.operation_type) return false;
116+
return true;
117+
};
118+
119+
const engine: any = {
120+
async findOne(table: string, opts: { where: Record<string, unknown> }) {
121+
if (table === 'sys_metadata_history') {
122+
return historyRows.find((h) => matchesHistory(h, opts.where)) ?? null;
123+
}
124+
return findRow(opts.where)?.row ?? null;
125+
},
126+
async find(table: string, opts: { where: Record<string, unknown> }) {
127+
if (table === 'sys_metadata_history') {
128+
return historyRows.filter((h) => matchesHistory(h, opts.where));
129+
}
130+
return Array.from(rows.values()).filter((r) => matchesMetadataWhere(r, opts.where));
131+
},
132+
async insert(table: string, data: Record<string, unknown>) {
133+
if (table === 'sys_metadata_audit') return { id: 'audit_skip' };
134+
if (table === 'sys_metadata_history') {
135+
nextId += 1;
136+
const h: HistoryRow = { id: `h_${nextId}`, ...(data as any) };
137+
historyRows.push(h);
138+
return { id: h.id };
139+
}
140+
nextId += 1;
141+
const row = { id: `r_${nextId}`, ...(data as any) } as Row;
142+
rows.set(keyOf(data), row);
143+
return { id: row.id };
144+
},
145+
async update(_t: string, data: Record<string, unknown>, opts: { where: Record<string, unknown> }) {
146+
const found = findRow(opts.where);
147+
if (!found) return { id: null };
148+
const merged = { ...found.row, ...(data as any) };
149+
rows.delete(found.key);
150+
rows.set(keyOf(merged), merged);
151+
return { id: found.row.id };
152+
},
153+
async delete(_t: string, opts: { where: Record<string, unknown> }) {
154+
const found = findRow(opts.where);
155+
if (!found) return { deleted: 0 };
156+
rows.delete(found.key);
157+
return { deleted: 1 };
158+
},
159+
async transaction<T>(cb: (ctx: any) => Promise<T>): Promise<T> {
160+
return cb(undefined);
161+
},
162+
registry: {
163+
registerItem: () => {},
164+
registerObject: () => {},
165+
// No declared package namespace → publishPackageDrafts skips the
166+
// ADR-0028 prefix check (legacy-grandfathered path).
167+
getPackage: () => undefined,
168+
},
169+
};
170+
return { engine, rows, historyRows };
171+
}
172+
173+
const objectBody = (name: string) => ({
174+
name,
175+
label: 'Project Task',
176+
fields: {
177+
title: { type: 'text', label: 'Title' },
178+
done: { type: 'boolean', label: 'Done' },
179+
},
180+
});
181+
182+
describe('publishPackageDrafts — env-wide draft under a non-null active org (#3115)', () => {
183+
it('saves the object draft env-wide (organization_id = NULL) when no org is threaded', async () => {
184+
const { engine, rows } = makeStubEngine();
185+
const protocol = new ObjectStackProtocolImplementation(engine);
186+
187+
// Mirrors REST `PUT /meta/object/proj_task?mode=draft&package=app.projects`
188+
// — package bound, but no organizationId threaded.
189+
await protocol.saveMetaItem({
190+
type: 'object',
191+
name: 'proj_task',
192+
item: objectBody('proj_task'),
193+
packageId: 'app.projects',
194+
mode: 'draft',
195+
});
196+
197+
const draftRows = Array.from(rows.values()).filter((r) => r.state === 'draft');
198+
expect(draftRows).toHaveLength(1);
199+
expect(draftRows[0].organization_id).toBeNull();
200+
expect(draftRows[0].package_id).toBe('app.projects');
201+
});
202+
203+
it('publishes the env-wide draft even though the session carries a non-null active org', async () => {
204+
const { engine, rows } = makeStubEngine();
205+
const protocol = new ObjectStackProtocolImplementation(engine);
206+
207+
// 1. Studio "Save Draft" — env-wide (no active org threaded).
208+
await protocol.saveMetaItem({
209+
type: 'object',
210+
name: 'proj_task',
211+
item: objectBody('proj_task'),
212+
packageId: 'app.projects',
213+
mode: 'draft',
214+
});
215+
216+
// 2. Studio "Publish" — the dispatcher resolved a non-null active org.
217+
const res = await protocol.publishPackageDrafts({
218+
packageId: 'app.projects',
219+
organizationId: 'org_alpha',
220+
});
221+
222+
// Before the fix this returned { success:false, failedCount:1,
223+
// failed:[{ code:'no_draft' }] }.
224+
expect(res.failed).toEqual([]);
225+
expect(res).toMatchObject({ success: true, publishedCount: 1, failedCount: 0 });
226+
expect(res.published.map((p) => p.name)).toEqual(['proj_task']);
227+
228+
// The draft was consumed and the active row landed env-wide
229+
// (package-owned metadata is env-level, not per-org).
230+
const remaining = Array.from(rows.values());
231+
expect(remaining.filter((r) => r.state === 'draft')).toHaveLength(0);
232+
const active = remaining.filter((r) => r.state === 'active');
233+
expect(active).toHaveLength(1);
234+
expect(active[0].organization_id).toBeNull();
235+
});
236+
237+
it('still publishes an org-scoped draft under that same org (no regression)', async () => {
238+
const { engine, rows } = makeStubEngine();
239+
const protocol = new ObjectStackProtocolImplementation(engine);
240+
241+
// A per-org overlay draft (organization_id = org_alpha).
242+
await protocol.saveMetaItem({
243+
type: 'object',
244+
name: 'proj_task',
245+
item: objectBody('proj_task'),
246+
organizationId: 'org_alpha',
247+
packageId: 'app.projects',
248+
mode: 'draft',
249+
});
250+
251+
const res = await protocol.publishPackageDrafts({
252+
packageId: 'app.projects',
253+
organizationId: 'org_alpha',
254+
});
255+
256+
expect(res).toMatchObject({ success: true, publishedCount: 1, failedCount: 0 });
257+
const active = Array.from(rows.values()).filter((r) => r.state === 'active');
258+
expect(active).toHaveLength(1);
259+
expect(active[0].organization_id).toBe('org_alpha');
260+
});
261+
});

packages/metadata-protocol/src/protocol.ts

Lines changed: 26 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -4737,6 +4737,7 @@ export class ObjectStackProtocolImplementation implements ObjectStackProtocol {
47374737
drafts: Array<{
47384738
type: string;
47394739
name: string;
4740+
organizationId: string | null;
47404741
packageId: string | null;
47414742
updatedAt: string | null;
47424743
updatedBy: string | null;
@@ -4860,8 +4861,12 @@ export class ObjectStackProtocolImplementation implements ObjectStackProtocol {
48604861
const commitItems: Array<{ type: string; name: string; existedBefore: boolean; prevVersion: number | null }> = [];
48614862
for (const d of ordered) {
48624863
try {
4864+
// Read the pre-publish active row in the draft's OWN scope
4865+
// (env-wide drafts have env-wide active rows). Using the
4866+
// request's active org here would miss an env-wide edit and
4867+
// mis-record it as a create in the revert plan (#3115).
48634868
const activeRow = (await this.engine.findOne('sys_metadata', {
4864-
where: { organization_id: orgId, type: d.type, name: d.name, state: 'active' },
4869+
where: { organization_id: d.organizationId ?? null, type: d.type, name: d.name, state: 'active' },
48654870
})) as { version?: number } | null;
48664871
commitItems.push({
48674872
type: d.type,
@@ -4911,19 +4916,29 @@ export class ObjectStackProtocolImplementation implements ObjectStackProtocol {
49114916
await inTxn(async () => {
49124917
for (const d of ordered) {
49134918
try {
4919+
// Promote each draft in the scope `listDrafts` surfaced
4920+
// it from (#3115). Studio/package authoring writes the
4921+
// draft env-wide (`organization_id = NULL`) while the
4922+
// publishing session may carry a non-null active org;
4923+
// `listDrafts` includes those env-wide rows via its `$or`,
4924+
// so the promote MUST target the draft's own org or it
4925+
// 404s (`no_draft`) on a row it can never match.
4926+
const draftOrgId = d.organizationId ?? null;
49144927
if (d.type === 'seed') {
49154928
// Capture the body BEFORE promote (the draft row is
49164929
// deleted by the promote, and a post-publish read-back
49174930
// has org-scope resolution pitfalls — reading the
4918-
// draft is unambiguous).
4919-
const ref = { type: d.type, name: d.name, org: orgId ?? 'env' } as unknown as Parameters<typeof repo.get>[0];
4920-
const draft = await repo.get(ref, { state: 'draft' });
4931+
// draft is unambiguous). Read from the draft's own
4932+
// scope, not the request's active org.
4933+
const seedRepo = this.getOverlayRepo(draftOrgId);
4934+
const ref = { type: d.type, name: d.name, org: draftOrgId ?? 'env' } as unknown as Parameters<typeof seedRepo.get>[0];
4935+
const draft = await seedRepo.get(ref, { state: 'draft' });
49214936
if (draft?.body) seedBodies.push(draft.body);
49224937
}
49234938
const { singularType, result } = await this.promoteDraftForPublish({
49244939
type: d.type,
49254940
name: d.name,
4926-
...(request.organizationId ? { organizationId: request.organizationId } : {}),
4941+
...(draftOrgId ? { organizationId: draftOrgId } : {}),
49274942
...(request.actor ? { actor: request.actor } : {}),
49284943
message: `publish app package '${request.packageId}'`,
49294944
});
@@ -5123,11 +5138,16 @@ export class ObjectStackProtocolImplementation implements ObjectStackProtocol {
51235138

51245139
for (const d of drafts) {
51255140
try {
5141+
// Discard the draft in the scope it lives in (#3115). Like
5142+
// publish, `listDrafts` surfaces env-wide drafts to a non-null
5143+
// active org via `$or`; deleting under the request's active org
5144+
// would silently no-op on those env-wide rows.
5145+
const draftOrgId = d.organizationId ?? null;
51265146
await this.deleteMetaItem({
51275147
type: d.type,
51285148
name: d.name,
51295149
state: 'draft',
5130-
...(request.organizationId ? { organizationId: request.organizationId } : {}),
5150+
...(draftOrgId ? { organizationId: draftOrgId } : {}),
51315151
...(request.actor ? { actor: request.actor } : {}),
51325152
});
51335153
discarded.push({ type: d.type, name: d.name });

packages/metadata-protocol/src/sys-metadata-repository.ts

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -727,6 +727,14 @@ export class SysMetadataRepository implements MetadataRepository {
727727
Array<{
728728
type: string;
729729
name: string;
730+
/**
731+
* The scope the draft actually lives in — `null` for an env-wide draft,
732+
* a string for a per-org overlay draft. The `$or` below surfaces BOTH to
733+
* a non-null-org caller, so consumers that then act on a draft (promote /
734+
* discard) MUST route the write to THIS scope, not the caller's active
735+
* org, or they 404 on the env-wide row they can never match (#3115).
736+
*/
737+
organizationId: string | null;
730738
packageId: string | null;
731739
updatedAt: string | null;
732740
updatedBy: string | null;
@@ -756,6 +764,7 @@ export class SysMetadataRepository implements MetadataRepository {
756764
return (rows as any[]).map((row) => ({
757765
type: row.type,
758766
name: row.name,
767+
organizationId: row.organization_id ?? null,
759768
packageId: row.package_id ?? null,
760769
updatedAt: row.updated_at ?? row.created_at ?? null,
761770
updatedBy: row.updated_by ?? row.created_by ?? null,

packages/objectql/src/sys-metadata-repository-list-drafts.test.ts

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -75,8 +75,14 @@ describe('SysMetadataRepository.listDrafts (ADR-0033)', () => {
7575
{ type: 'object', name: 'other_org', state: 'draft', package_id: 'p', organization_id: 'org_other', updated_at: 't3' },
7676
];
7777
const { repo } = makeRepo(rows, 'org_acme');
78-
const names = (await repo.listDrafts()).map((d) => d.name).sort();
79-
expect(names).toEqual(['env_wide', 'org_scoped']);
78+
const out = await repo.listDrafts();
79+
expect(out.map((d) => d.name).sort()).toEqual(['env_wide', 'org_scoped']);
80+
// Each draft is projected with the scope it actually lives in, so the
81+
// publish/discard paths can promote/delete it in THAT scope rather than the
82+
// caller's active org — otherwise the env-wide row 404s as `no_draft`
83+
// (#3115). This projection is the contract those callers depend on.
84+
expect(out.find((d) => d.name === 'env_wide')?.organizationId).toBeNull();
85+
expect(out.find((d) => d.name === 'org_scoped')?.organizationId).toBe('org_acme');
8086
});
8187

8288
it('filters by type', async () => {

0 commit comments

Comments
 (0)