Skip to content

Commit cffca05

Browse files
committed
fix(metadata): revertPackage only takes the _packageId stamp; publishPackage keeps its two keys
Measured on a showcase boot: with the stamp read by publishPackage, the publish door publishes a read-only platform package's objects (ADR-0070 D2), so publish keeps its two-key lookup pending a decision. Unit pins for the helper's key set and the publish non-change; door pins for the bridge-stamped and applyProtection-stamped revert and the controls. Claude-Session: https://claude.ai/code/session_01EUBvqtauTDmHi2ZgY759p2 Co-authored-by: Claude <noreply@anthropic.com>
1 parent 0edb299 commit cffca05

3 files changed

Lines changed: 299 additions & 9 deletions

File tree

‎packages/metadata/src/metadata-manager.ts‎

Lines changed: 27 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1796,19 +1796,20 @@ export class MetadataManager implements IMetadataService {
17961796
}
17971797

17981798
/**
1799-
* The registry items that belong to `packageId`, for the two package-wide
1800-
* writes ({@link publishPackage}, {@link revertPackage}).
1799+
* [#22113] The registry items that belong to `packageId`, for
1800+
* {@link revertPackage}.
18011801
*
18021802
* An item belongs to a package when any of three keys names it:
18031803
*
18041804
* - `packageId` — the publish envelope's own key, and what a caller that
18051805
* registers an item for a package writes;
18061806
* - `package` — the legacy package stamp;
1807-
* - `_packageId` — the private provenance stamp `applyProtection`
1808-
* (`@objectstack/spec/shared`) writes on every item a code-shipped
1809-
* artifact registers here (ADR-0010 §3.7). A code-shipped item carries
1810-
* ONLY this key, and it is the key the metadata protocol scopes registry
1811-
* items by for every read and for `isArtifactBacked`.
1807+
* - `_packageId` — the private provenance stamp. `applyProtection`
1808+
* (`@objectstack/spec/shared`, ADR-0010 §3.7) writes it on every item a
1809+
* code-shipped artifact registers here, and the ObjectQL object bridge
1810+
* copies it onto every object it registers here. A code-shipped item
1811+
* carries ONLY this key, and it is the key the metadata protocol scopes
1812+
* this registry's items by when it serves a package's reads.
18121813
*
18131814
* Reading the first two alone made a code-shipped package's revert answer
18141815
* 404 "No metadata items found" while every read served its items. With the
@@ -1867,8 +1868,25 @@ export class MetadataManager implements IMetadataService {
18671868
const shouldValidate = options?.validate !== false;
18681869
const publishedBy = options?.publishedBy;
18691870

1870-
// Collect all items belonging to this package
1871-
const packageItems = this.collectPackageMembers(packageId);
1871+
// Collect all items belonging to this package.
1872+
//
1873+
// [#22113] Deliberately NOT `collectPackageMembers`: these two keys miss a
1874+
// code-shipped item, which carries only the `_packageId` stamp, and that
1875+
// miss is what keeps this method from publishing a read-only code package
1876+
// (ADR-0070 D2). With the stamp read here, nothing below refuses one: a
1877+
// platform package's objects validate, are snapshotted and are
1878+
// re-registered as published (measured on a showcase boot). How publish
1879+
// answers a read-only package is a decision this method does not make by
1880+
// changing its member set.
1881+
const packageItems: Array<{ type: string; name: string; data: any }> = [];
1882+
for (const [type, typeStore] of this.registry) {
1883+
for (const [name, data] of typeStore) {
1884+
const meta = data as any;
1885+
if (meta?.packageId === packageId || meta?.package === packageId) {
1886+
packageItems.push({ type, name, data: meta });
1887+
}
1888+
}
1889+
}
18721890

18731891
if (packageItems.length === 0) {
18741892
return {

‎packages/metadata/src/metadata-service.test.ts‎

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -822,6 +822,70 @@ describe('MetadataManager — IMetadataService Contract', () => {
822822
message: expect.stringContaining('has never been published'),
823823
});
824824
});
825+
826+
// [#22113] A code-shipped item carries ONLY the private `_packageId`
827+
// stamp — `applyProtection` writes it on every item an artifact registers,
828+
// and the ObjectQL object bridge copies it onto every object. Collecting
829+
// members by `packageId` / `package` alone answered 404 "No metadata items
830+
// found" for a package every read serves.
831+
it('finds a code-shipped package by the _packageId stamp: never published ⇒ 409, not 404', async () => {
832+
await manager.register('object', 'code_item', {
833+
name: 'code_item', label: 'Code Item', _packageId: 'com.acme.code', _provenance: 'package',
834+
});
835+
836+
await expect(manager.revertPackage('com.acme.code')).rejects.toMatchObject({
837+
code: 'RESOURCE_CONFLICT',
838+
status: 409,
839+
message: "Package 'com.acme.code' has never been published",
840+
});
841+
});
842+
843+
it('membership is any of packageId, package or _packageId: each member with a snapshot is restored', async () => {
844+
const snapshot = (label: string) => ({ label });
845+
await manager.register('object', 'by_package_id', {
846+
name: 'by_package_id', packageId: 'com.acme.keys', publishedDefinition: snapshot('A'), state: 'draft',
847+
});
848+
await manager.register('object', 'by_package', {
849+
name: 'by_package', package: 'com.acme.keys', publishedDefinition: snapshot('B'), state: 'draft',
850+
});
851+
await manager.register('object', 'by_stamp', {
852+
name: 'by_stamp', _packageId: 'com.acme.keys', publishedDefinition: snapshot('C'), state: 'draft',
853+
});
854+
// Control: a different package's item is not a member.
855+
await manager.register('object', 'other_pkg', {
856+
name: 'other_pkg', _packageId: 'com.acme.other', publishedDefinition: snapshot('D'), state: 'draft',
857+
});
858+
859+
await manager.revertPackage('com.acme.keys');
860+
861+
for (const [name, label] of [['by_package_id', 'A'], ['by_package', 'B'], ['by_stamp', 'C']] as const) {
862+
const item = await manager.get('object', name) as any;
863+
expect({ name, state: item.state, metadata: item.metadata }).toEqual({ name, state: 'active', metadata: { label } });
864+
}
865+
const other = await manager.get('object', 'other_pkg') as any;
866+
expect(other.state).toBe('draft');
867+
expect(other.metadata).toBeUndefined();
868+
});
869+
});
870+
871+
// [#22113] `publishPackage` deliberately keeps the two-key lookup: with the
872+
// stamp read, nothing in it refuses a read-only code package (ADR-0070 D2),
873+
// so a code package's items would be snapshotted and re-registered as
874+
// published. This pins that an item carrying only the stamp is NOT
875+
// published by this method until that decision is made.
876+
describe('publishPackage — an item carrying only the _packageId stamp', () => {
877+
it('is not a member: nothing is snapshotted', async () => {
878+
await manager.register('object', 'code_item', {
879+
name: 'code_item', label: 'Code Item', _packageId: 'com.acme.code', _provenance: 'package',
880+
});
881+
882+
const result = await manager.publishPackage('com.acme.code', { validate: false });
883+
884+
expect(result).toMatchObject({ success: false, itemsPublished: 0 });
885+
const item = await manager.get('object', 'code_item') as any;
886+
expect(item.publishedDefinition).toBeUndefined();
887+
expect(item.state).toBeUndefined();
888+
});
825889
});
826890

827891
describe('getPublished', () => {
Lines changed: 208 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,208 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
//
3+
// Real-engine door pins for #22113 — `POST /packages/:id/revert` on a
4+
// code-shipped package answered 404 "No metadata items found" while every read
5+
// served the package's items.
6+
7+
import { describe, it, expect, afterEach } from 'vitest';
8+
import { mkdtempSync, rmSync } from 'node:fs';
9+
import { tmpdir } from 'node:os';
10+
import { join } from 'node:path';
11+
import { ObjectQL } from '@objectstack/objectql';
12+
import { SqlDriver } from '@objectstack/driver-sql';
13+
import { MetadataManager } from '@objectstack/metadata';
14+
import { ObjectStackProtocolImplementation } from '@objectstack/metadata-protocol';
15+
import {
16+
SysMetadataObject,
17+
SysMetadataHistoryObject,
18+
SysMetadataAuditObject,
19+
SysMetadataCommitObject,
20+
} from '@objectstack/metadata-core';
21+
import { DEFAULT_METADATA_TYPE_REGISTRY } from '@objectstack/spec/kernel';
22+
import { applyProtection } from '@objectstack/spec/shared';
23+
import { HttpDispatcher } from './http-dispatcher.js';
24+
25+
/**
26+
* ## The defect, measured at the door
27+
*
28+
* On a `objectstack dev` boot of `examples/app-showcase` at the base of this
29+
* change, `POST /api/v1/packages/:id/revert` answered
30+
* `404 RESOURCE_NOT_FOUND "No metadata items found for package '…'"` for
31+
* eighteen code-shipped platform packages whose objects every read serves
32+
* (`com.objectstack.platform-objects`, `com.objectstack.service.job`, …).
33+
*
34+
* `MetadataManager.revertPackage` collected members by an item's own
35+
* `packageId` / `package` key. A code-shipped item carries only the private
36+
* `_packageId` stamp, written by two producers: the ObjectQL object bridge
37+
* copies `SchemaRegistry.getAllObjects()`'s owner tag onto every object it
38+
* registers in the metadata service, and the metadata plugin's artifact loader
39+
* stamps every artifact item through `applyProtection`. Both producers are
40+
* used here as they are in production, so the stamp under test is the real
41+
* one, not a hand-written key.
42+
*
43+
* ## Why a REAL engine, a REAL protocol and a REAL metadata service
44+
*
45+
* The door asks the protocol first (`revertStoredPackage`) and the metadata
46+
* service only when the package has no stored row, so the answer for a
47+
* code-shipped package is a composition of both; a double of either would pin
48+
* the double. The sibling suite `package-revert-stored-members.integration.test.ts`
49+
* holds the stored-row half and the authored-`packageId` control.
50+
*/
51+
52+
/** The package the engine's objects are registered under, as a platform package's are. */
53+
const CODE_PKG = 'com.example.code_objects';
54+
/** An artifact-loaded package: its items are stamped by `applyProtection`. */
55+
const ARTIFACT_PKG = 'com.example.artifact';
56+
/** A package whose items carry an authored `packageId` (the control). */
57+
const AUTHORED_PKG = 'com.example.authored';
58+
59+
let cleanup: Array<() => void> = [];
60+
afterEach(() => {
61+
for (const c of cleanup) c();
62+
cleanup = [];
63+
});
64+
65+
/** REAL ObjectQL over on-disk better-sqlite3, the REAL protocol, the REAL metadata service, one door. */
66+
async function boot() {
67+
const dir = mkdtempSync(join(tmpdir(), 'os-22113-'));
68+
cleanup.push(() => rmSync(dir, { recursive: true, force: true }));
69+
70+
const driver = new SqlDriver({
71+
client: 'better-sqlite3',
72+
connection: { filename: join(dir, 'data.sqlite') },
73+
useNullAsDefault: true,
74+
});
75+
const sysObjects = [
76+
SysMetadataObject,
77+
SysMetadataHistoryObject,
78+
SysMetadataAuditObject,
79+
SysMetadataCommitObject,
80+
] as any[];
81+
await driver.initObjects(sysObjects);
82+
83+
const engine = new ObjectQL();
84+
engine.registerDriver(driver as any, true);
85+
await engine.init();
86+
// The four `sys_metadata*` objects stand in for a code package's objects:
87+
// registered under CODE_PKG, they are what the SchemaRegistry hands the
88+
// object bridge with that package as their owner.
89+
for (const o of sysObjects) engine.registry.registerObject(o, CODE_PKG);
90+
cleanup.push(() => { void engine.destroy(); });
91+
92+
const protocol = new ObjectStackProtocolImplementation(
93+
engine as any, undefined, undefined, 'package-author',
94+
);
95+
96+
const metadata = new MetadataManager({ formats: ['json'] });
97+
metadata.setTypeRegistry(DEFAULT_METADATA_TYPE_REGISTRY);
98+
99+
const services: Record<string, unknown> = { protocol, objectql: engine, metadata };
100+
const get = (name: string) => services[name] ?? null;
101+
const kernel: any = { context: { getService: get }, getService: get, getServiceAsync: async (n: string) => get(n) };
102+
const dispatcher = new HttpDispatcher(kernel);
103+
104+
/** `POST /packages/:id/<verb>`, as a caller holding `manage_metadata`. */
105+
const call = async (packageId: string, verb: 'revert' | 'publish') => {
106+
const context = {
107+
request: {},
108+
executionContext: { userId: 'u_pkg_admin', systemPermissions: ['manage_metadata', 'studio.access'] },
109+
} as any;
110+
const result = await dispatcher.handlePackages(`/${encodeURIComponent(packageId)}/${verb}`, 'POST', {}, {}, context);
111+
expect(result.handled).toBe(true);
112+
return { status: result.response?.status, body: result.response?.body };
113+
};
114+
115+
return { engine, metadata, revert: (id: string) => call(id, 'revert'), publish: (id: string) => call(id, 'publish') };
116+
}
117+
118+
/** The ObjectQL object bridge's loop body (`bridgeObjectsToMetadataService`), on the real registry. */
119+
async function bridgeObjects(engine: ObjectQL, metadata: MetadataManager): Promise<string[]> {
120+
const bridged: string[] = [];
121+
for (const obj of engine.registry.getAllObjects()) {
122+
await metadata.register('object', obj.name, obj, { notify: false });
123+
bridged.push(obj.name);
124+
}
125+
return bridged;
126+
}
127+
128+
/** The metadata plugin's artifact registration of one item: stamp, then register. */
129+
async function registerArtifactItem(metadata: MetadataManager, type: string, item: Record<string, unknown>) {
130+
applyProtection(item, { packageId: ARTIFACT_PKG, packageVersion: '1.0.0' });
131+
await metadata.register(type, String(item.name), item, { notify: false });
132+
}
133+
134+
describe('#22113 POST /packages/:id/revert on a code-shipped package', () => {
135+
it('objects the bridge registered carry only the _packageId stamp: 409 "has never been published" (was: 404)', async () => {
136+
const { engine, metadata, revert } = await boot();
137+
expect(await bridgeObjects(engine, metadata)).toContain('sys_metadata');
138+
// The precondition the defect needs, read off the real producer: the
139+
// stamp is there, the two keys the old lookup read are not.
140+
const sample = await metadata.get('object', 'sys_metadata') as any;
141+
expect({ _packageId: sample._packageId, packageId: sample.packageId, package: sample.package })
142+
.toEqual({ _packageId: CODE_PKG, packageId: undefined, package: undefined });
143+
144+
const answer = await revert(CODE_PKG);
145+
146+
expect(answer.status).toBe(409);
147+
expect(answer.body?.error?.code).toBe('RESOURCE_CONFLICT');
148+
expect(answer.body?.error?.message).toBe(`Package '${CODE_PKG}' has never been published`);
149+
});
150+
151+
it('an artifact-loaded package stamped by applyProtection: 409 "has never been published" (was: 404)', async () => {
152+
const { metadata, revert } = await boot();
153+
await registerArtifactItem(metadata, 'capability', { name: 'artifact.export_data', label: 'Export', scope: 'org' });
154+
155+
const answer = await revert(ARTIFACT_PKG);
156+
157+
expect(answer.status).toBe(409);
158+
expect(answer.body?.error?.code).toBe('RESOURCE_CONFLICT');
159+
expect(answer.body?.error?.message).toBe(`Package '${ARTIFACT_PKG}' has never been published`);
160+
});
161+
162+
it('control: a package nothing in this process carries is still 404 RESOURCE_NOT_FOUND', async () => {
163+
const { engine, metadata, revert } = await boot();
164+
await bridgeObjects(engine, metadata);
165+
166+
const answer = await revert('com.example.no_such_package');
167+
168+
expect(answer.status).toBe(404);
169+
expect(answer.body?.error?.code).toBe('RESOURCE_NOT_FOUND');
170+
});
171+
});
172+
173+
describe('#22113 controls — what this change leaves as it was', () => {
174+
it('a package with authored packageId members: publish then edit then revert restores the snapshot (200)', async () => {
175+
const { metadata, revert, publish } = await boot();
176+
await metadata.register('capability', 'authored.export_data', {
177+
name: 'authored.export_data', label: 'Export Authored Data', scope: 'org', packageId: AUTHORED_PKG,
178+
});
179+
180+
const published = await publish(AUTHORED_PKG);
181+
expect(published.status).toBe(200);
182+
expect(published.body?.data).toMatchObject({ success: true, itemsPublished: 1 });
183+
const item = await metadata.get('capability', 'authored.export_data') as any;
184+
await metadata.register('capability', 'authored.export_data', {
185+
...item, metadata: { ...item.publishedDefinition, label: 'Edited after publish' }, state: 'draft',
186+
});
187+
188+
const answer = await revert(AUTHORED_PKG);
189+
190+
expect(answer.status).toBe(200);
191+
const reverted = await metadata.get('capability', 'authored.export_data') as any;
192+
expect(reverted.state).toBe('active');
193+
expect(reverted.metadata.label).toBe('Export Authored Data');
194+
});
195+
196+
it('publish does not take the stamp: a code-shipped package is not published, nothing is snapshotted', async () => {
197+
const { engine, metadata, publish } = await boot();
198+
expect(await bridgeObjects(engine, metadata)).toContain('sys_metadata');
199+
200+
const answer = await publish(CODE_PKG);
201+
202+
expect(answer.status).toBe(200);
203+
expect(answer.body?.data).toMatchObject({ success: false, itemsPublished: 0 });
204+
const obj = await metadata.get('object', 'sys_metadata') as any;
205+
expect(obj.publishedDefinition).toBeUndefined();
206+
expect(obj.state).toBeUndefined();
207+
});
208+
});

0 commit comments

Comments
 (0)