Skip to content

Commit d0bb78e

Browse files
fix(metadata): revertPackage finds a code-shipped package's members by the _packageId stamp (#22132)
Fixes #22113 Clause-②: no ## What changes ADR-0070 D2 makes a code or installed package read-only. The two package-wide doors now refuse one with the door's existing `422 WRITABLE_PACKAGE_REQUIRED`, through the same `requireWritablePackage` predicate that `PATCH /packages/:id/disable` and `DELETE /packages/:id` already ask. This follows triage's answer on the card: Q1 is A, Q2 is B. - **`POST /api/v1/packages/:id/publish`** refuses a non-writable package before `MetadataManager.publishPackage` runs. - **`POST /api/v1/packages/:id/revert`** refuses a non-writable package after the protocol's stored-row answer (`revertStoredPackage`). A stored row bound to a code package, such as an organization overlay draft, keeps the protocol's answer. - **`MetadataManager.publishPackage` and `revertPackage`** find a package's members through one private helper, `collectPackageMembers`. It reads `packageId`, `package` and the private `_packageId` stamp. - The stamp has two producers: the artifact loader writes it through `applyProtection`, and the ObjectQL object bridge copies `getAllObjects()`'s owner tag onto every object it registers. - `publishPackage` takes the helper only because the publish door now refuses a read-only package in front of it. `MetadataManager` has no notion of package kind. Before the guard existed, the helper in `publishPackage` was measured publishing a platform package's objects (`0edb299`), which is why `cffca05` held it back. The only in-repo caller of `publishPackage` is this door. ## Measured at the door These readings come from an `objectstack dev --fresh` boot of `examples/app-showcase`. Base is `51290bc`; head is `434d074`. | door | base | head | |---|---|---| | revert, all 26 packages `GET /packages` lists (`com.objectstack.setup` and 6 other registry-only packages, 18 platform packages that ship objects, `com.example.showcase`) | 25 × 404 "No metadata items found"; the showcase 409 "has never been published" | 26 × 422 `WRITABLE_PACKAGE_REQUIRED` | | publish `com.objectstack.setup`, `…platform-objects`, `…service.job` | 200, `success: false`, "No metadata items found" | 422 `WRITABLE_PACKAGE_REQUIRED` | | publish `com.example.showcase` | 200, `success: true`, `itemsPublished: 2` (writes onto two read-only capabilities) | 422 `WRITABLE_PACKAGE_REQUIRED` | | unknown id `com.example.no_such_package` | revert 404; publish 200, `success: false` | unchanged | | writable package `com.example.repairs` (created through `POST /packages`, one view draft-saved and published through `publish-drafts`) | publish 200, `success: false`, "No metadata items found"; revert 200 | unchanged | **Overlay drafts (triage's premise for Q1), measured at both base and head.** An organization overlay draft of `showcase_task.grid`, bound to `com.example.showcase`, gave the same sequence on both builds except for the first step: 1. `/publish` of the showcase. - At base it answered 200 with `itemsPublished: 2`, and the overlay draft was still pending afterwards. So this door never published overlay drafts. - At head it answers 422, and the draft is still pending. 2. `/revert` while the draft is pending: 409 `RESOURCE_CONFLICT` "Package 'com.example.showcase' has never been published, so there is no published version", the protocol's stored-row answer (#22090). 3. `/publish-drafts`: 200, `publishedCount: 1`. The draft is gone and the overlay label serves. 4. `/revert` with the overlay published and no draft pending: 200. Overlay drafts publish through `publish-drafts` and the per-item publish door, and neither passes the guarded branch. ## Pins - `packages/runtime/src/package-revert-code-shipped-members.integration.test.ts`. This uses a real ObjectQL over better-sqlite3, the real protocol, the real `MetadataManager` and `HttpDispatcher`, and the real producers of the stamp. Each refusal asserts `422` and `WRITABLE_PACKAGE_REQUIRED` plus the sentence's head. - A `scope: 'system'` package that ships objects: revert and publish both answer 422 (flipped from the 409 this PR first pinned), and nothing is snapshotted. - A `scope: 'system'` package the metadata service never holds, the setup shape: revert and publish both answer 422. - A booted package, the showcase shape, including an authored-`packageId` capability: publish and revert both answer 422, and the capability is not snapshotted. - Controls: - an unknown id: revert 404, publish 200 with `success: false`; - a writable package with authored `packageId` members: publish, edit and revert answer 200, and the snapshot is restored; - a writable base whose members carry only the stamp: revert 409 before a publish, then publish 200 (`itemsPublished: 1`), then revert 200; - a stored draft row bound to a booted package: revert keeps the protocol's 409. - `packages/runtime/src/package-revert-stored-members.integration.test.ts`. Its two showcase (d) cases are flipped. Each now boots the showcase manifest and asserts 422 `WRITABLE_PACKAGE_REQUIRED`; the "published then edited" case also asserts that nothing is restored. - `packages/metadata/src/metadata-service.test.ts`. - The revert pins hold: a stamped-only package answers 409 with the exact sentence, and membership is any of the three keys. - The publish pin is flipped: a stamped-only item is now a member and is snapshotted. ## Verification (head `434d074`) - `pnpm --filter @objectstack/metadata typecheck`: exit 0. - `pnpm --filter @objectstack/metadata test`: 58 files, 870 tests passed. - `pnpm --filter @objectstack/runtime typecheck`, which includes `check:test-typecheck`: exit 0, debt ledger held at 27 files / 190 errors / 68 signatures. - `pnpm --filter @objectstack/runtime test` (the `local` project): 334 files, 4717 passed, 19 skipped. - Reverse verification on the committed head, each leg through `scripts/ablation-replace.mjs` with the restore proved by blob: - (A1) Without the publish guard, 3 refusal pins went red ("expected 200 to be 422"). - (A2) Without the revert guard, 5 went red: 3 here and the 2 flipped (d) pins ("expected 409 / 404 / 200 to be 422"). - (A3) With the revert guard moved before the protocol's stored-row answer, the stored-row control went red ("expected 422 to be 409"). The first A3 attempt was refused as a no-op, because its replacement still contained the anchor; it was re-run with a new anchor. - (A4) With `publishPackage` back on two keys (metadata rebuilt, dist preflight hit in 4 files), the flipped unit pin and the stamped writable-base door pin went red. - Restore leg: both files matched their HEAD blobs, `git diff HEAD` was empty and the tree was clean; after a rebuild the suites read 6/6 unit and 16/16 door. - `node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack` (no paths) derived 64 families, and all 64 exited 0. `--ran` reconciliation: 64 derived, 64 run, 0 NOT-MEASURED (a derived zero). The new family since the revert-only head is `check:route-envelope`. - Narrowed lint: `eslint --no-inline-config --format json` over the 5 touched `.ts` files found 5 files, 0 errors, 0 warnings. `eslint.config.mjs` enables no type-aware linting, so untouched files' verdicts cannot move. The full `pnpm lint` is CI's. ## Acceptance notes - A revert of a flat (non-envelope) published item writes `metadata: publishedDefinition`, a nested copy of the whole item, because publish snapshots `data.metadata ?? data`. It is reachable only for a writable package's flat items now. Carrier: the claimant; no card. - `MetadataManager.unregisterPackage` and `query({ packageId })` still match on the old keys. `unregisterPackage` has no production caller in this repository. Carrier: the claimant; no card. - objectui's `PackagesPage` calls both doors. On a code package it now receives a 422 with a worded message instead of a 200 or a 404/409. The response shapes are unchanged, so the Console pin is not affected. --- _Generated by [Claude Code](https://claude.ai/code/session_01EUBvqtauTDmHi2ZgY759p2)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 7ef50a4 commit d0bb78e

6 files changed

Lines changed: 470 additions & 37 deletions
Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
---
2+
'@objectstack/metadata': patch
3+
'@objectstack/runtime': patch
4+
---
5+
6+
fix(runtime,metadata): publishing or reverting a read-only package is refused with `422 WRITABLE_PACKAGE_REQUIRED`, and package membership reads the `_packageId` stamp
7+
8+
Clause-②: no
9+
10+
ADR-0070 D2 makes a code or installed package read-only. `POST /api/v1/packages/:id/publish` and `POST /api/v1/packages/:id/revert` did not check this. These two answers change:
11+
12+
- **Publish of a code or installed package: 200 → 422.** Before, the publish door answered 200. For a package whose items carry an authored `packageId`, for example `com.example.showcase`'s two capabilities, it answered `success: true` and wrote `publishedDefinition`, `state` and `version` onto those read-only items. For every other code package it answered `success: false`, "No metadata items found". It now answers `422 WRITABLE_PACKAGE_REQUIRED` before anything is written, the same refusal that `PATCH /packages/:id/disable` and `DELETE /packages/:id` already give.
13+
- **Revert of a code or installed package: 404 or 409 → 422.** Before, the revert door answered `404 RESOURCE_NOT_FOUND` "No metadata items found" (for example `com.objectstack.setup` and the platform packages that ship objects) or `409 RESOURCE_CONFLICT` "Package '…' has never been published" (for example `com.example.showcase`). It now answers `422 WRITABLE_PACKAGE_REQUIRED`. The check runs after the protocol's stored-row answer, so a revert of a code package that has a stored row bound to it, such as an organization overlay draft, keeps its answer.
14+
15+
To customise what a code package provides, use an ADR-0005 organization overlay. Overlay drafts publish through `POST /api/v1/packages/:id/publish-drafts` and the per-item publish door, and this change leaves both alone.
16+
17+
**Unchanged:** a writable package's publish and revert, and an id that nothing carries (revert 404; publish 200 with `success: false`).
18+
19+
`MetadataManager.publishPackage` and `revertPackage` now find a package's members by `packageId`, `package` or the private `_packageId` stamp. The artifact loader writes that stamp through `applyProtection`, and the ObjectQL object bridge copies it onto every object it registers. Before, an item that carried only the stamp was not a member. `MetadataManager` has no notion of package kind; the refusal of read-only packages lives at the two doors above.

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

Lines changed: 46 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1795,6 +1795,47 @@ export class MetadataManager implements IMetadataService {
17951795
}
17961796
}
17971797

1798+
/**
1799+
* [#22113] The registry items that belong to `packageId`, for the two
1800+
* package-wide writes ({@link publishPackage}, {@link revertPackage}).
1801+
*
1802+
* An item belongs to a package when any of three keys names it:
1803+
*
1804+
* - `packageId` — the publish envelope's own key, and what a caller that
1805+
* registers an item for a package writes;
1806+
* - `package` — the legacy package stamp;
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.
1813+
*
1814+
* Reading the first two alone made a package's publish and revert miss
1815+
* every item that carries only the stamp ("No metadata items found" while
1816+
* every read served them).
1817+
*
1818+
* ⛔ Not a writability check. This class has no package-kind concept, so it
1819+
* cannot tell a read-only code package (ADR-0070 D2) from a writable one,
1820+
* and with the stamp read it would publish a code package's items like any
1821+
* other. The refusal belongs to the caller that holds the predicate: the
1822+
* `POST /packages/:id/publish` and `/revert` doors (`@objectstack/runtime`)
1823+
* refuse a non-writable package with `422 WRITABLE_PACKAGE_REQUIRED` before
1824+
* calling either method.
1825+
*/
1826+
private collectPackageMembers(packageId: string): Array<{ type: string; name: string; data: any }> {
1827+
const members: Array<{ type: string; name: string; data: any }> = [];
1828+
for (const [type, typeStore] of this.registry) {
1829+
for (const [name, data] of typeStore) {
1830+
const meta = data as any;
1831+
if (meta?.packageId === packageId || meta?.package === packageId || meta?._packageId === packageId) {
1832+
members.push({ type, name, data: meta });
1833+
}
1834+
}
1835+
}
1836+
return members;
1837+
}
1838+
17981839
/**
17991840
* Publish an entire package:
18001841
* 1. Validate all draft items
@@ -1832,16 +1873,10 @@ export class MetadataManager implements IMetadataService {
18321873
const shouldValidate = options?.validate !== false;
18331874
const publishedBy = options?.publishedBy;
18341875

1835-
// Collect all items belonging to this package
1836-
const packageItems: Array<{ type: string; name: string; data: any }> = [];
1837-
for (const [type, typeStore] of this.registry) {
1838-
for (const [name, data] of typeStore) {
1839-
const meta = data as any;
1840-
if (meta?.packageId === packageId || meta?.package === packageId) {
1841-
packageItems.push({ type, name, data: meta });
1842-
}
1843-
}
1844-
}
1876+
// Collect all items belonging to this package — the same membership
1877+
// `revertPackage` reads. A read-only code package never gets here through
1878+
// the publish door, which refuses it first (see `collectPackageMembers`).
1879+
const packageItems = this.collectPackageMembers(packageId);
18451880

18461881
if (packageItems.length === 0) {
18471882
return {
@@ -2060,15 +2095,7 @@ export class MetadataManager implements IMetadataService {
20602095
* Restores all metadata definitions from their published snapshots.
20612096
*/
20622097
async revertPackage(packageId: string): Promise<void> {
2063-
const packageItems: Array<{ type: string; name: string; data: any }> = [];
2064-
for (const [type, typeStore] of this.registry) {
2065-
for (const [name, data] of typeStore) {
2066-
const meta = data as any;
2067-
if (meta?.packageId === packageId || meta?.package === packageId) {
2068-
packageItems.push({ type, name, data: meta });
2069-
}
2070-
}
2071-
}
2098+
const packageItems = this.collectPackageMembers(packageId);
20722099

20732100
// [#7559] ADR-0112 — both refusals below carry a DECLARED `code` + `status`.
20742101
// They are the ordinary answers to an ordinary request (revert a package id

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

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -822,6 +822,71 @@ describe('MetadataManager — IMetadataService Contract', () => {
822822
message: expect.stringContaining('has never been published'),
823823
});
824824
});
825+
826+
// [#22113] An item can carry ONLY the private `_packageId` stamp —
827+
// `applyProtection` writes it on every item an artifact registers, and the
828+
// ObjectQL object bridge copies it onto every object. Collecting members by
829+
// `packageId` / `package` alone answered 404 "No metadata items found" for a
830+
// package every read serves. (Whether the package may be reverted at all is
831+
// the door's question — it refuses a read-only one with 422 first.)
832+
it('finds a package by the _packageId stamp: never published ⇒ 409, not 404', async () => {
833+
await manager.register('object', 'code_item', {
834+
name: 'code_item', label: 'Code Item', _packageId: 'com.acme.code', _provenance: 'package',
835+
});
836+
837+
await expect(manager.revertPackage('com.acme.code')).rejects.toMatchObject({
838+
code: 'RESOURCE_CONFLICT',
839+
status: 409,
840+
message: "Package 'com.acme.code' has never been published",
841+
});
842+
});
843+
844+
it('membership is any of packageId, package or _packageId: each member with a snapshot is restored', async () => {
845+
const snapshot = (label: string) => ({ label });
846+
await manager.register('object', 'by_package_id', {
847+
name: 'by_package_id', packageId: 'com.acme.keys', publishedDefinition: snapshot('A'), state: 'draft',
848+
});
849+
await manager.register('object', 'by_package', {
850+
name: 'by_package', package: 'com.acme.keys', publishedDefinition: snapshot('B'), state: 'draft',
851+
});
852+
await manager.register('object', 'by_stamp', {
853+
name: 'by_stamp', _packageId: 'com.acme.keys', publishedDefinition: snapshot('C'), state: 'draft',
854+
});
855+
// Control: a different package's item is not a member.
856+
await manager.register('object', 'other_pkg', {
857+
name: 'other_pkg', _packageId: 'com.acme.other', publishedDefinition: snapshot('D'), state: 'draft',
858+
});
859+
860+
await manager.revertPackage('com.acme.keys');
861+
862+
for (const [name, label] of [['by_package_id', 'A'], ['by_package', 'B'], ['by_stamp', 'C']] as const) {
863+
const item = await manager.get('object', name) as any;
864+
expect({ name, state: item.state, metadata: item.metadata }).toEqual({ name, state: 'active', metadata: { label } });
865+
}
866+
const other = await manager.get('object', 'other_pkg') as any;
867+
expect(other.state).toBe('draft');
868+
expect(other.metadata).toBeUndefined();
869+
});
870+
});
871+
872+
// [#22113] `publishPackage` reads the same membership as `revertPackage`:
873+
// an item carrying only the stamp is a member. (Flipped from the pin that
874+
// held it OUT while nothing refused a read-only code package; the
875+
// `POST /packages/:id/publish` door now refuses one with
876+
// `422 WRITABLE_PACKAGE_REQUIRED` before this method runs.)
877+
describe('publishPackage — an item carrying only the _packageId stamp', () => {
878+
it('is a member: it is snapshotted and published', async () => {
879+
await manager.register('object', 'stamped_item', {
880+
name: 'stamped_item', label: 'Stamped Item', _packageId: 'com.acme.stamped', _provenance: 'package',
881+
});
882+
883+
const result = await manager.publishPackage('com.acme.stamped', { validate: false });
884+
885+
expect(result).toMatchObject({ success: true, itemsPublished: 1, version: 1 });
886+
const item = await manager.get('object', 'stamped_item') as any;
887+
expect(item.state).toBe('active');
888+
expect(item.publishedDefinition).toMatchObject({ name: 'stamped_item', label: 'Stamped Item' });
889+
});
825890
});
826891

827892
describe('getPublished', () => {

‎packages/runtime/src/domains/packages.ts‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -378,6 +378,12 @@ function requireReadCapability(deps: DomainHandlerDeps, context: HttpProtocolCon
378378
* authoring one ("switch to a writable package in the package selector") names
379379
* a remedy that makes no sense for a delete.
380380
*
381+
* [#22113] `POST /packages/:id/publish` and `POST /packages/:id/revert` ask it
382+
* too, with the same predicate and refusal: publishing or reverting a code
383+
* package's shipped items is an authoring write into a read-only package
384+
* (ADR-0070 D2). Revert asks it only after the protocol's stored-row answer, so
385+
* a `sys_metadata` row bound to the package keeps its own answer.
386+
*
381387
* [#14451] The remedy it names is ADR-0005 org overlay, and it used to name
382388
* `POST /packages/:id/duplicate` instead. That sentence sent a caller holding a
383389
* read-only package at a route which — for exactly the packages this refusal
@@ -1459,6 +1465,16 @@ export async function handlePackagesRequest(deps: DomainHandlerDeps, path: strin
14591465
if (parts.length === 2 && parts[1] === 'publish' && m === 'POST') {
14601466
const denied = requireManageMetadata(deps, _context); if (denied) return denied;
14611467
const id = decodeURIComponent(parts[0]);
1468+
// [#22113] ADR-0070 D2: a code or installed package is read-only, so
1469+
// this door refuses it BEFORE `publishPackage`, which would otherwise
1470+
// snapshot and re-register the package's items as published (measured:
1471+
// a platform package's objects, and the showcase's capabilities, at
1472+
// 200). The metadata service has no package-kind concept to refuse
1473+
// with; the door's one predicate does. An organization overlay draft
1474+
// on a code package is not published here: it is a `sys_metadata` row
1475+
// the protocol publishes through `publish-drafts` and the per-item
1476+
// publish door, neither of which passes this branch.
1477+
const readOnly = requireWritablePackage(deps, qlService, id, 'publish'); if (readOnly) return readOnly;
14621478
const metadataService = await deps.getService(_context, CoreServiceName.enum.metadata);
14631479
if (metadataService && typeof (metadataService as any).publishPackage === 'function') {
14641480
const result = await (metadataService as any).publishPackage(id, body || {});
@@ -1917,6 +1933,14 @@ export async function handlePackagesRequest(deps: DomainHandlerDeps, path: strin
19171933
return { handled: true, response: deps.errorFromThrown(e, 500) };
19181934
}
19191935
}
1936+
// [#22113] ADR-0070 D2: a code or installed package has nothing of its
1937+
// own to revert, so it is refused here, AFTER the protocol's stored-row
1938+
// answer above. A stored row bound to it (an organization overlay
1939+
// draft) is still answered by the protocol, exactly as before; only the
1940+
// metadata service's revert of the package's shipped items is refused.
1941+
// "Has never been published" would point the caller at publishing,
1942+
// which the publish door above refuses for the same package.
1943+
const readOnly = requireWritablePackage(deps, qlService, id, 'revert'); if (readOnly) return readOnly;
19201944
const metadataService = await deps.getService(_context, CoreServiceName.enum.metadata);
19211945
if (metadataService && typeof (metadataService as any).revertPackage === 'function') {
19221946
await (metadataService as any).revertPackage(id);

0 commit comments

Comments
 (0)