From c4174831a28c62b6b7affcdaaf0af6352d2dff6a Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 16 Oct 2026 23:01:02 +0000 Subject: [PATCH 1/5] fix(metadata-protocol): duplicatePackage reports a refused install of the copy's package record The duplicate wrote the copy's package record through installPackage inside a bare catch that kept nothing, so a refused install left a copy with no registry record and no sys_packages row, answered success: true, and logged nothing. The refusal now rides failed[] as the source's package item (quoted when it declares a client status) and is logged once at warn with the target id and the refusal's code and sentence. The copy still proceeds. Claude-Session: https://claude.ai/code/session_01TQLy3Yq6vF4SDpTXdr3Uwi Co-authored-by: Claude --- ....duplicate-package-install-refusal.test.ts | 196 ++++++++++++++++++ ...duplicate-package-target-namespace.test.ts | 6 +- .../src/protocol.install-manifest-id.test.ts | 6 +- packages/metadata-protocol/src/protocol.ts | 57 ++++- 4 files changed, 248 insertions(+), 17 deletions(-) create mode 100644 packages/metadata-protocol/src/protocol.duplicate-package-install-refusal.test.ts diff --git a/packages/metadata-protocol/src/protocol.duplicate-package-install-refusal.test.ts b/packages/metadata-protocol/src/protocol.duplicate-package-install-refusal.test.ts new file mode 100644 index 00000000000..1b846d2e5ce --- /dev/null +++ b/packages/metadata-protocol/src/protocol.duplicate-package-install-refusal.test.ts @@ -0,0 +1,196 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#24041] `duplicatePackage` reports a refused install of the copy's package + * record — on its answer and in the log — and still copies the items. + * + * --------------------------------------------------------------------------- + * The gap these pins close + * --------------------------------------------------------------------------- + * The duplicate writes the copy's package record through `installPackage` + * before its copy loop, inside a `catch {}` that kept nothing. A refused + * install leaves no registry record and no `sys_packages` row, so the target + * id is not an installed package while every copied item is bound to it — and + * the caller read `success: true` with an empty `failed[]`, and no line was + * logged at any level. + * + * --------------------------------------------------------------------------- + * The two refusals, and why each is built the way it is + * --------------------------------------------------------------------------- + * - The REGISTRY'S NAMESPACE GATE. `SchemaRegistry.installPackage` refuses a + * namespace another installed package owns with `NamespaceConflictError` + * (`NAMESPACE_CONFLICT`, 422) — reachable from the HTTP door through an + * explicit `targetNamespace` or a derived one. That judge lives in + * `@objectstack/objectql`, which depends on this package, so it cannot be + * imported here: the registry below answers with that error's envelope + * (`code`, `status`, its sentence) at the registry's own boundary, and the + * protocol's real `installPackage` is the code that meets it. + * - A FAILED `sys_packages` WRITE. Built with no stub on the install path at + * all: the `package` service reports a driver fault the way the real one + * does (`{ success: false, … }`), and the real primitive undoes its registry + * write and throws its own 500. That one is NOT a client refusal, so the + * answer must not quote the store; the log carries it. + * + * Every refusal is answered by a CONTROL on the same harness: an accepted + * install adds no `package` entry, logs no line, and leaves the record in + * place — a pin on the refusal alone cannot tell "the refusal is reported" + * from "this door now reports every install". + */ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { ObjectStackProtocolImplementation } from './protocol.js'; + +const SOURCE = 'com.example.leave'; +const TARGET = 'com.example.leave-copy'; +/** The installed package whose namespace the copy asks for. */ +const OWNER = 'com.example.taken'; +const DRIVER_TEXT = 'SQLITE_ERROR: no such table: sys_packages'; + +/** `NamespaceConflictError`'s envelope, as `SchemaRegistry.installPackage` throws it. */ +function namespaceConflict(namespace: string, existing: string, incoming: string): Error { + return Object.assign( + new Error( + `Namespace conflict: namespace "${namespace}" is already owned by package "${existing}", ` + + `so package "${incoming}" cannot be installed alongside it.`, + ), + { name: 'NamespaceConflictError', code: 'NAMESPACE_CONFLICT', status: 422, httpStatus: 422 }, + ); +} + +/** + * A duplicate-door harness: one source package holding one object row, a + * registry that keeps its package records and namespace owners, and + * optionally a `package` service. The namespace gate refuses exactly as the + * real one does: a namespace another installed package owns. + */ +function makeHarness(opts: { publish?: (data: unknown) => Promise } = {}) { + const rows = [{ + id: 'r_1', + type: 'object', + name: 'leave_ticket', + organization_id: null, + package_id: SOURCE, + state: 'active', + metadata: JSON.stringify({ name: 'leave_ticket', label: 'Ticket' }), + }]; + const packages = new Map([ + [SOURCE, { manifest: { id: SOURCE, name: 'Leave', namespace: 'leave', version: '1.0.0' } }], + [OWNER, { manifest: { id: OWNER, name: 'Taken', namespace: 'taken', version: '1.0.0' } }], + ]); + const namespaceOwners = new Map([['leave', [SOURCE]], ['taken', [OWNER]]]); + const registry: any = { + getPackage: vi.fn((id: string) => packages.get(id)), + getNamespaceOwners: vi.fn((ns: string) => [...(namespaceOwners.get(ns) ?? [])]), + installPackage: vi.fn((manifest: any) => { + const owner = (namespaceOwners.get(manifest.namespace) ?? []).find((id) => id !== manifest.id); + if (owner) throw namespaceConflict(manifest.namespace, owner, manifest.id); + const row = { manifest, status: 'installed', enabled: true }; + packages.set(manifest.id, row); + namespaceOwners.set(manifest.namespace, [...(namespaceOwners.get(manifest.namespace) ?? []), manifest.id]); + return row; + }), + unregisterItem: vi.fn((type: string, id: string) => { + if (type === 'package') packages.delete(id); + }), + unregisterNamespace: vi.fn((ns: string, id: string) => { + namespaceOwners.set(ns, (namespaceOwners.get(ns) ?? []).filter((owner) => owner !== id)); + }), + }; + const engine: any = { find: vi.fn(async () => rows), registry }; + const services = new Map(); + if (opts.publish) services.set('package', { publish: vi.fn(opts.publish) }); + const impl = new ObjectStackProtocolImplementation(engine as never, () => services as never); + const saveMetaItem = vi.spyOn(impl, 'saveMetaItem' as never); + (saveMetaItem as any).mockResolvedValue({ success: true } as never); + return { impl, registry, packages, saveMetaItem }; +} + +/** The `[protocol.duplicatePackage]` lines about the copy's package record. */ +function packageRecordLines(warn: ReturnType): string[] { + return warn.mock.calls + .map((args) => String(args[0])) + .filter((line) => line.startsWith('[protocol.duplicatePackage] package record')); +} + +describe('[#24041] duplicatePackage reports a refused install of the copy\'s package record', () => { + let warn: ReturnType; + beforeEach(() => { + warn = vi.spyOn(console, 'warn').mockImplementation(() => {}); + }); + afterEach(() => { + warn.mockRestore(); + }); + + it('carries the registry\'s namespace refusal on failed[], logs it once, and still copies', async () => { + const { impl, packages, saveMetaItem } = makeHarness(); + const refusal = namespaceConflict('taken', OWNER, TARGET); + + const res = await (impl as any).duplicatePackage({ + sourcePackageId: SOURCE, + targetPackageId: TARGET, + targetNamespace: 'taken', + }); + + // The answer names the refusal: one `package` entry, under the + // source's own package item, quoting the registry's sentence (a 422 + // is a client refusal) and naming the target id. + const entries = res.failed.filter((f: any) => f.type === 'package'); + expect(entries).toHaveLength(1); + expect(entries[0].name).toBe(SOURCE); + expect(entries[0].error).toContain(`'${TARGET}'`); + expect(entries[0].error).toContain(refusal.message); + expect(res.success).toBe(false); + expect(res.failedCount).toBe(res.failed.length); + + // The copy still goes on: the item was written, bound to the target. + expect(res.copied).toEqual([{ type: 'object', name: 'taken_ticket' }]); + expect(saveMetaItem).toHaveBeenCalledTimes(1); + expect((saveMetaItem.mock.calls[0]![0] as any).packageId).toBe(TARGET); + // …and the target is not an installed package: the refusal preceded + // every registry write. + expect(packages.has(TARGET)).toBe(false); + + // The log names the target id, the refusal's code and its sentence, once. + const lines = packageRecordLines(warn); + expect(lines).toHaveLength(1); + expect(lines[0]).toContain(`'${TARGET}'`); + expect(lines[0]).toContain('NAMESPACE_CONFLICT'); + expect(lines[0]).toContain(refusal.message); + }); + + it('carries a failed sys_packages write without quoting the store, and logs the primitive\'s sentence', async () => { + const { impl, packages } = makeHarness({ + publish: async () => ({ success: false, driverFault: { message: DRIVER_TEXT } }), + }); + + const res = await (impl as any).duplicatePackage({ sourcePackageId: SOURCE, targetPackageId: TARGET }); + + const entries = res.failed.filter((f: any) => f.type === 'package'); + expect(entries).toHaveLength(1); + expect(entries[0].name).toBe(SOURCE); + expect(entries[0].error).toContain(`'${TARGET}'`); + // A 500 is not a client refusal: nothing the store said reaches the answer. + expect(entries[0].error).not.toContain('SQLITE'); + expect(res.success).toBe(false); + // The real primitive undid its registry write before it threw. + expect(packages.has(TARGET)).toBe(false); + // The copy still goes on. + expect(res.copied).toEqual([{ type: 'object', name: 'leave_copy_ticket' }]); + + const lines = packageRecordLines(warn); + expect(lines).toHaveLength(1); + expect(lines[0]).toContain(`'${TARGET}'`); + expect(lines[0]).toContain(`Package '${TARGET}' was not installed`); + }); + + it('CONTROL: an accepted install adds no package entry and logs no line', async () => { + const { impl, packages } = makeHarness({ publish: async () => ({ success: true }) }); + + const res = await (impl as any).duplicatePackage({ sourcePackageId: SOURCE, targetPackageId: TARGET }); + + expect(res.failed).toEqual([]); + expect(res.success).toBe(true); + expect(res.copied).toEqual([{ type: 'object', name: 'leave_copy_ticket' }]); + expect(packages.get(TARGET)?.manifest?.namespace).toBe('leave_copy'); + expect(packageRecordLines(warn)).toEqual([]); + }); +}); diff --git a/packages/metadata-protocol/src/protocol.duplicate-package-target-namespace.test.ts b/packages/metadata-protocol/src/protocol.duplicate-package-target-namespace.test.ts index f26368850f8..d5d0d35ef01 100644 --- a/packages/metadata-protocol/src/protocol.duplicate-package-target-namespace.test.ts +++ b/packages/metadata-protocol/src/protocol.duplicate-package-target-namespace.test.ts @@ -128,9 +128,9 @@ describe('[#19577] duplicatePackage refuses an explicit targetNamespace the decl expect(err.message).toContain(declarationSentence(ns)); // ⭐ Nothing was minted and nothing was scanned: the refusal - // precedes the manifest write AND the copy loop. The manifest write - // sits inside a best-effort `catch {}`, so a refusal raised only - // there would be swallowed and reported as `success: true`. + // precedes the manifest write AND the copy loop. A refusal of the + // manifest write does not abort the copy, so a refusal raised only + // there would be a `failed[]` entry on a copy that went on anyway. expect(installed).toHaveLength(0); expect(engine.registry.installPackage).not.toHaveBeenCalled(); expect(engine.find).not.toHaveBeenCalled(); diff --git a/packages/metadata-protocol/src/protocol.install-manifest-id.test.ts b/packages/metadata-protocol/src/protocol.install-manifest-id.test.ts index 0f6822adc4c..97b27d7b51d 100644 --- a/packages/metadata-protocol/src/protocol.install-manifest-id.test.ts +++ b/packages/metadata-protocol/src/protocol.install-manifest-id.test.ts @@ -183,9 +183,9 @@ describe('[#19417] duplicatePackage refuses a target id the declaration refuses' expect(err.message).toBe(manifestIdRefusal('targetPackageId', id)); // ⭐ Nothing was minted and nothing was scanned: the refusal // precedes the manifest write AND the copy loop, so no empty shell - // is left behind. The manifest write below sits inside a - // best-effort `catch {}` — a refusal raised only there would be - // swallowed and reported as `success: true`. + // is left behind. A refusal of the manifest write does not abort + // the copy — a refusal raised only there would be a `failed[]` + // entry on a copy that went on anyway. expect(installed).toHaveLength(0); expect(engine.registry.installPackage).not.toHaveBeenCalled(); expect(engine.find).not.toHaveBeenCalled(); diff --git a/packages/metadata-protocol/src/protocol.ts b/packages/metadata-protocol/src/protocol.ts index e696e4fad42..0bb93644cef 100644 --- a/packages/metadata-protocol/src/protocol.ts +++ b/packages/metadata-protocol/src/protocol.ts @@ -27056,12 +27056,11 @@ export class ObjectStackProtocolImplementation implements // covers the WRITE. It does not cover this method, for two reasons that // are both about where the refusal lands: // - // ① the `installPackage` call below sits inside a `catch {}` that is - // deliberately best-effort (a manifest row is not worth aborting a - // copy for), so a refusal raised THERE would be swallowed here and - // the caller would read `success: true` on a package with no - // manifest row — a silent partial state, strictly worse than the - // 201 this card set out to close; + // ① a refusal of the `installPackage` call below does not abort the + // copy (a manifest row is not worth aborting a copy for): it is + // reported as a `failed[]` entry on a duplicate that goes on, so + // a refusal of the TARGET ID raised there would arrive as a + // partial copy under an id the declaration refuses outright; // ② the refusal has to precede the MUTATIONS, and the manifest write // is not the first of them. #14451 already established the position // for this door's other precondition: `duplicatePackage` mints the @@ -27434,6 +27433,9 @@ export class ObjectStackProtocolImplementation implements return { fields: Object.fromEntries(out) }; }; + const copied: Array<{ type: string; name: string }> = []; + const failed: Array<{ type: string; name: string; error: string }> = []; + if (srcPkg?.manifest && typeof registry?.installPackage === 'function') { try { // Route through installPackage (not a bare registry write) so the @@ -27451,13 +27453,46 @@ export class ObjectStackProtocolImplementation implements }; delete dupManifest.scope; await this.installPackage({ manifest: dupManifest } as InstallPackageRequest); - } catch { - /* best-effort — the per-item package binding still works without a manifest row */ + } catch (e: any) { + // [#24041] A REFUSED PACKAGE RECORD IS REPORTED, NEVER SWALLOWED. + // The copy still goes on: the per-item package binding works + // without a package record, and aborting here would leave the + // caller nothing it has today (refusing the duplicate outright + // is a separate decision). What it no longer does is go on in + // SILENCE. A refused install leaves no registry record and no + // `sys_packages` row, so `targetPackageId` is not an installed + // package — absent from `GET /packages` at once, not only after + // a restart — while every item copied below is bound to it. + // + // Reachable from the HTTP door: `SchemaRegistry.installPackage`'s + // namespace gate (`NAMESPACE_CONFLICT`, 422) refuses a target + // namespace another installed package owns — an explicit + // `targetNamespace`, or one derived from a target id whose last + // segment another package's namespace already uses — and a + // failed `sys_packages` write fails the install (#21243). + // + // ⭐ It rides `failed[]`, this verb's one per-item outcome + // carrier, under the source's own `package` item: the clone + // could not copy it. So `success` reads false and `failedCount` + // counts it, as for any item. The sentence is quoted only when + // the refusal declares a client status; a store fault is not + // quoted, and the warn below carries its full text, exactly as + // the per-item save failure does further down. + failed.push({ + type: 'package', + name: request.sourcePackageId, + error: `Package '${request.sourcePackageId}' is copied without its package record: installing ` + + `'${request.targetPackageId}' was refused, so '${request.targetPackageId}' is not an ` + + 'installed package while the items copied into it are bound to it. ' + + clientFacingFailureText(e, 'The install failed; the reason is in the server log.'), + }); + console.warn( + `[protocol.duplicatePackage] package record for '${request.targetPackageId}' ` + + `(copy of '${request.sourcePackageId}') was not installed: ` + + `${typeof e?.code === 'string' ? `${e.code}: ` : ''}${e?.message ?? e}`, + ); } } - - const copied: Array<{ type: string; name: string }> = []; - const failed: Array<{ type: string; name: string; error: string }> = []; // Resolved once for the whole copy: every flow row in this package needs // the same engine, and a package with fifty flows should not walk the // service table fifty times. From 0361b4ebd4286782262e41bbeda1f2939ad91c82 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 16 Oct 2026 23:09:56 +0000 Subject: [PATCH 2/5] test(metadata-protocol): type the install-refusal pin's console and save spies Claude-Session: https://claude.ai/code/session_01TQLy3Yq6vF4SDpTXdr3Uwi Co-authored-by: Claude --- .../protocol.duplicate-package-install-refusal.test.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/metadata-protocol/src/protocol.duplicate-package-install-refusal.test.ts b/packages/metadata-protocol/src/protocol.duplicate-package-install-refusal.test.ts index 1b846d2e5ce..80a79ae2c8d 100644 --- a/packages/metadata-protocol/src/protocol.duplicate-package-install-refusal.test.ts +++ b/packages/metadata-protocol/src/protocol.duplicate-package-install-refusal.test.ts @@ -105,10 +105,10 @@ function makeHarness(opts: { publish?: (data: unknown) => Promise } = { } /** The `[protocol.duplicatePackage]` lines about the copy's package record. */ -function packageRecordLines(warn: ReturnType): string[] { +function packageRecordLines(warn: { mock: { calls: unknown[][] } }): string[] { return warn.mock.calls - .map((args) => String(args[0])) - .filter((line) => line.startsWith('[protocol.duplicatePackage] package record')); + .map((args: unknown[]) => String(args[0])) + .filter((line: string) => line.startsWith('[protocol.duplicatePackage] package record')); } describe('[#24041] duplicatePackage reports a refused install of the copy\'s package record', () => { @@ -144,7 +144,7 @@ describe('[#24041] duplicatePackage reports a refused install of the copy\'s pac // The copy still goes on: the item was written, bound to the target. expect(res.copied).toEqual([{ type: 'object', name: 'taken_ticket' }]); expect(saveMetaItem).toHaveBeenCalledTimes(1); - expect((saveMetaItem.mock.calls[0]![0] as any).packageId).toBe(TARGET); + expect((saveMetaItem as any).mock.calls[0][0].packageId).toBe(TARGET); // …and the target is not an installed package: the refusal preceded // every registry write. expect(packages.has(TARGET)).toBe(false); From fdb7697163861061cf9cf92a5c4979e9c1e8c216 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 16 Oct 2026 23:12:28 +0000 Subject: [PATCH 3/5] chore(changeset): metadata-protocol patch for the duplicate's reported install refusal Claude-Session: https://claude.ai/code/session_01TQLy3Yq6vF4SDpTXdr3Uwi Co-authored-by: Claude --- .changeset/24041-duplicate-install-refusal.md | 12 ++++++++++++ 1 file changed, 12 insertions(+) create mode 100644 .changeset/24041-duplicate-install-refusal.md diff --git a/.changeset/24041-duplicate-install-refusal.md b/.changeset/24041-duplicate-install-refusal.md new file mode 100644 index 00000000000..1eca7946b58 --- /dev/null +++ b/.changeset/24041-duplicate-install-refusal.md @@ -0,0 +1,12 @@ +--- +"@objectstack/metadata-protocol": patch +--- + +fix(metadata-protocol): `duplicatePackage` reports a refused install of the copy's package record, on its answer and in the log + +Clause-②: no + +- **What was silent.** The duplicate installs the copy's package record before it copies the items, and a refusal of that install was caught and dropped. A refused install leaves no registry record and no `sys_packages` row, so the copy's id was not an installed package (absent from `GET /packages` at once, not only after a restart) while every copied item was bound to it, and the answer read `success: true` with an empty `failed[]`. Nothing was logged. It is reached whenever the copy's namespace is one another installed package owns (`NAMESPACE_CONFLICT`, 422): an explicit `targetNamespace`, or one derived from a target id whose last segment another package's namespace already uses. A failed `sys_packages` write fails the install the same way. +- **The answer now names it.** One `failed[]` entry `{ type: 'package', name, error }`, where `name` is the source package id (the source's package item, which the clone could not copy). Its `error` names the target id and quotes the refusal when the refusal declares a client status, so a namespace conflict arrives with its own remedy. A store fault is not quoted. The duplicate's `success` is therefore `false` and `failedCount` counts the entry; the HTTP status stays 200, so a consumer reading `data.success` now sees a partial duplicate. +- **The log now names it.** One `warn` line from `[protocol.duplicatePackage]` names the target id and the source id, then the refusal's code and full message, store faults included. +- **Unchanged:** the duplicate still copies the items after a refused install; the response's shape; every other `failed[]` entry. From 68429a829bb632ad02a7f049d77897313d0340b9 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 17 Oct 2026 00:15:45 +0000 Subject: [PATCH 4/5] fix(metadata-protocol): a refused install of the copy's package record refuses the duplicate duplicatePackage now rethrows installPackage's refusal unchanged, before any item is copied, so the door answers the refusal's own status (422 NAMESPACE_CONFLICT with the registry's remedy, or the primitive's 500 for a store fault) and nothing is minted or copied. The install is the duplicate's first mutation and a refused install leaves nothing behind, so no rollback is needed. The refusal is logged once at warn; a store fault's error is attached so its cause is not lost. The failed[] package entry is retired. Claude-Session: https://claude.ai/code/session_01TQLy3Yq6vF4SDpTXdr3Uwi Co-authored-by: Claude --- ....duplicate-package-install-refusal.test.ts | 187 ++++++++++-------- ...duplicate-package-target-namespace.test.ts | 6 +- .../src/protocol.install-manifest-id.test.ts | 6 +- packages/metadata-protocol/src/protocol.ts | 80 ++++---- 4 files changed, 147 insertions(+), 132 deletions(-) diff --git a/packages/metadata-protocol/src/protocol.duplicate-package-install-refusal.test.ts b/packages/metadata-protocol/src/protocol.duplicate-package-install-refusal.test.ts index 80a79ae2c8d..d470a686192 100644 --- a/packages/metadata-protocol/src/protocol.duplicate-package-install-refusal.test.ts +++ b/packages/metadata-protocol/src/protocol.duplicate-package-install-refusal.test.ts @@ -1,42 +1,46 @@ // Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. /** - * [#24041] `duplicatePackage` reports a refused install of the copy's package - * record — on its answer and in the log — and still copies the items. + * [#24041] `duplicatePackage` is refused, with nothing copied, when the install + * of the copy's package record is refused — and the refusal it answers is the + * install's own, unchanged. * * --------------------------------------------------------------------------- * The gap these pins close * --------------------------------------------------------------------------- * The duplicate writes the copy's package record through `installPackage` * before its copy loop, inside a `catch {}` that kept nothing. A refused - * install leaves no registry record and no `sys_packages` row, so the target - * id is not an installed package while every copied item is bound to it — and - * the caller read `success: true` with an empty `failed[]`, and no line was - * logged at any level. + * install leaves no registry record and no `sys_packages` row, so the copy's + * items were live, bound to an id no package record holds — after a namespace + * conflict, inside a namespace another package owns — and the caller read + * `success: true`, with no line logged at any level. The door now keeps the + * rule its sibling `importUnmanagedPackage` states: "There is no half-copied + * app." (ruling B, claim amendment 6227008947). * * --------------------------------------------------------------------------- * The two refusals, and why each is built the way it is * --------------------------------------------------------------------------- - * - The REGISTRY'S NAMESPACE GATE. `SchemaRegistry.installPackage` refuses a - * namespace another installed package owns with `NamespaceConflictError` - * (`NAMESPACE_CONFLICT`, 422) — reachable from the HTTP door through an - * explicit `targetNamespace` or a derived one. That judge lives in - * `@objectstack/objectql`, which depends on this package, so it cannot be - * imported here: the registry below answers with that error's envelope - * (`code`, `status`, its sentence) at the registry's own boundary, and the - * protocol's real `installPackage` is the code that meets it. - * - A FAILED `sys_packages` WRITE. Built with no stub on the install path at - * all: the `package` service reports a driver fault the way the real one - * does (`{ success: false, … }`), and the real primitive undoes its registry - * write and throws its own 500. That one is NOT a client refusal, so the - * answer must not quote the store; the log carries it. + * - The REGISTRY'S NAMESPACE GATE (ADR-0048 Phase 1). + * `SchemaRegistry.installPackage` refuses a namespace another installed + * package owns with `NamespaceConflictError` (`NAMESPACE_CONFLICT`, 422). That + * judge lives in `@objectstack/objectql`, which depends on this package, so it + * cannot be imported here: the registry below refuses exactly when another + * package owns the namespace and throws that error's envelope (`code`, + * `status`, its sentence) at the registry's own boundary, and the protocol's + * real `installPackage` is the code that meets it. + * - A FAILED `sys_packages` WRITE. No stub on the install path at all: the + * `package` service reports a driver fault the way the real one does + * (`{ success: false, … }`), and the real primitive undoes its registry write + * and throws its own 500. * - * Every refusal is answered by a CONTROL on the same harness: an accepted - * install adds no `package` entry, logs no line, and leaves the record in - * place — a pin on the refusal alone cannot tell "the refusal is reported" - * from "this door now reports every install". + * Each asserts the refusal as the ENVELOPE an HTTP boundary resolves it to + * (`resolveThrownHttpError`, the resolver the door's `errorFromThrown` uses), + * that it is the install's own error object, rethrown unchanged, and that + * nothing was copied. A CONTROL on the same harness holds the other side: an + * accepted install still copies, and logs nothing. */ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { resolveThrownHttpError } from '@objectstack/types'; import { ObjectStackProtocolImplementation } from './protocol.js'; const SOURCE = 'com.example.leave'; @@ -50,7 +54,8 @@ function namespaceConflict(namespace: string, existing: string, incoming: string return Object.assign( new Error( `Namespace conflict: namespace "${namespace}" is already owned by package "${existing}", ` - + `so package "${incoming}" cannot be installed alongside it.`, + + `so package "${incoming}" cannot be installed alongside it. Choose a different namespace for ` + + `"${incoming}", or uninstall "${existing}" first.`, ), { name: 'NamespaceConflictError', code: 'NAMESPACE_CONFLICT', status: 422, httpStatus: 422 }, ); @@ -60,7 +65,8 @@ function namespaceConflict(namespace: string, existing: string, incoming: string * A duplicate-door harness: one source package holding one object row, a * registry that keeps its package records and namespace owners, and * optionally a `package` service. The namespace gate refuses exactly as the - * real one does: a namespace another installed package owns. + * real one does: a namespace another installed package owns. Whatever it + * throws is kept in `thrown`, so a pin can ask for that very object back. */ function makeHarness(opts: { publish?: (data: unknown) => Promise } = {}) { const rows = [{ @@ -77,12 +83,17 @@ function makeHarness(opts: { publish?: (data: unknown) => Promise } = { [OWNER, { manifest: { id: OWNER, name: 'Taken', namespace: 'taken', version: '1.0.0' } }], ]); const namespaceOwners = new Map([['leave', [SOURCE]], ['taken', [OWNER]]]); + const thrown: unknown[] = []; const registry: any = { getPackage: vi.fn((id: string) => packages.get(id)), getNamespaceOwners: vi.fn((ns: string) => [...(namespaceOwners.get(ns) ?? [])]), installPackage: vi.fn((manifest: any) => { const owner = (namespaceOwners.get(manifest.namespace) ?? []).find((id) => id !== manifest.id); - if (owner) throw namespaceConflict(manifest.namespace, owner, manifest.id); + if (owner) { + const refusal = namespaceConflict(manifest.namespace, owner, manifest.id); + thrown.push(refusal); + throw refusal; + } const row = { manifest, status: 'installed', enabled: true }; packages.set(manifest.id, row); namespaceOwners.set(manifest.namespace, [...(namespaceOwners.get(manifest.namespace) ?? []), manifest.id]); @@ -101,96 +112,102 @@ function makeHarness(opts: { publish?: (data: unknown) => Promise } = { const impl = new ObjectStackProtocolImplementation(engine as never, () => services as never); const saveMetaItem = vi.spyOn(impl, 'saveMetaItem' as never); (saveMetaItem as any).mockResolvedValue({ success: true } as never); - return { impl, registry, packages, saveMetaItem }; + return { impl, packages, namespaceOwners, saveMetaItem, thrown }; } -/** The `[protocol.duplicatePackage]` lines about the copy's package record. */ -function packageRecordLines(warn: { mock: { calls: unknown[][] } }): string[] { - return warn.mock.calls - .map((args: unknown[]) => String(args[0])) - .filter((line: string) => line.startsWith('[protocol.duplicatePackage] package record')); +/** The thrown refusal, or a failure naming what happened instead. */ +async function refusalOf(run: () => Promise): Promise { + try { + await run(); + } catch (e) { + return e; + } + throw new Error('expected the duplicate to be refused, but it resolved'); } -describe('[#24041] duplicatePackage reports a refused install of the copy\'s package record', () => { - let warn: ReturnType; +/** The `[protocol.duplicatePackage] refused:` log calls, each with its arguments. */ +function refusalLogs(warn: { mock: { calls: unknown[][] } }): unknown[][] { + return warn.mock.calls.filter((args: unknown[]) => String(args[0]).startsWith('[protocol.duplicatePackage] refused:')); +} + +describe('[#24041] duplicatePackage is refused when the copy\'s package record is refused', () => { + let warn: { mock: { calls: unknown[][] }; mockRestore: () => void }; beforeEach(() => { - warn = vi.spyOn(console, 'warn').mockImplementation(() => {}); + warn = vi.spyOn(console, 'warn').mockImplementation(() => {}) as never; }); afterEach(() => { warn.mockRestore(); }); - it('carries the registry\'s namespace refusal on failed[], logs it once, and still copies', async () => { - const { impl, packages, saveMetaItem } = makeHarness(); - const refusal = namespaceConflict('taken', OWNER, TARGET); + it('answers the registry\'s namespace refusal (422 NAMESPACE_CONFLICT) unchanged, and copies nothing', async () => { + const { impl, packages, namespaceOwners, saveMetaItem, thrown } = makeHarness(); - const res = await (impl as any).duplicatePackage({ + const err = await refusalOf(() => (impl as any).duplicatePackage({ sourcePackageId: SOURCE, targetPackageId: TARGET, targetNamespace: 'taken', - }); - - // The answer names the refusal: one `package` entry, under the - // source's own package item, quoting the registry's sentence (a 422 - // is a client refusal) and naming the target id. - const entries = res.failed.filter((f: any) => f.type === 'package'); - expect(entries).toHaveLength(1); - expect(entries[0].name).toBe(SOURCE); - expect(entries[0].error).toContain(`'${TARGET}'`); - expect(entries[0].error).toContain(refusal.message); - expect(res.success).toBe(false); - expect(res.failedCount).toBe(res.failed.length); - - // The copy still goes on: the item was written, bound to the target. - expect(res.copied).toEqual([{ type: 'object', name: 'taken_ticket' }]); - expect(saveMetaItem).toHaveBeenCalledTimes(1); - expect((saveMetaItem as any).mock.calls[0][0].packageId).toBe(TARGET); - // …and the target is not an installed package: the refusal preceded - // every registry write. + })); + + // The registry's own refusal, rethrown unchanged — so the door answers + // its status, its code and its remedy. + expect(thrown).toHaveLength(1); + expect(err).toBe(thrown[0]); + const envelope = resolveThrownHttpError(err); + expect(envelope.status).toBe(422); + expect(envelope.code).toBe('NAMESPACE_CONFLICT'); + + // Nothing was copied, and nothing was minted. + expect(saveMetaItem).not.toHaveBeenCalled(); expect(packages.has(TARGET)).toBe(false); - - // The log names the target id, the refusal's code and its sentence, once. - const lines = packageRecordLines(warn); - expect(lines).toHaveLength(1); - expect(lines[0]).toContain(`'${TARGET}'`); - expect(lines[0]).toContain('NAMESPACE_CONFLICT'); - expect(lines[0]).toContain(refusal.message); + expect(namespaceOwners.get('taken')).toEqual([OWNER]); + + // Logged once, naming the target, the code and the sentence; a client + // refusal is not handed to the log as an error object. + const logs = refusalLogs(warn); + expect(logs).toHaveLength(1); + expect(logs[0]).toHaveLength(1); + expect(String(logs[0]![0])).toContain(`'${TARGET}'`); + expect(String(logs[0]![0])).toContain('NAMESPACE_CONFLICT'); + expect(String(logs[0]![0])).toContain(err.message); }); - it('carries a failed sys_packages write without quoting the store, and logs the primitive\'s sentence', async () => { - const { impl, packages } = makeHarness({ + it('answers a failed sys_packages write with the primitive\'s 500, undone, and copies nothing', async () => { + const { impl, packages, saveMetaItem } = makeHarness({ publish: async () => ({ success: false, driverFault: { message: DRIVER_TEXT } }), }); - const res = await (impl as any).duplicatePackage({ sourcePackageId: SOURCE, targetPackageId: TARGET }); + const err = await refusalOf(() => (impl as any).duplicatePackage({ sourcePackageId: SOURCE, targetPackageId: TARGET })); + + // The primitive's own failure: a 500 whose sentence names the target and + // quotes nothing of the store. + const envelope = resolveThrownHttpError(err); + expect(envelope.status).toBe(500); + expect(err.message).toContain(`'${TARGET}'`); + expect(err.message).not.toContain('SQLITE'); - const entries = res.failed.filter((f: any) => f.type === 'package'); - expect(entries).toHaveLength(1); - expect(entries[0].name).toBe(SOURCE); - expect(entries[0].error).toContain(`'${TARGET}'`); - // A 500 is not a client refusal: nothing the store said reaches the answer. - expect(entries[0].error).not.toContain('SQLITE'); - expect(res.success).toBe(false); - // The real primitive undid its registry write before it threw. + // Nothing was copied, and the primitive undid its registry write. + expect(saveMetaItem).not.toHaveBeenCalled(); expect(packages.has(TARGET)).toBe(false); - // The copy still goes on. - expect(res.copied).toEqual([{ type: 'object', name: 'leave_copy_ticket' }]); - const lines = packageRecordLines(warn); - expect(lines).toHaveLength(1); - expect(lines[0]).toContain(`'${TARGET}'`); - expect(lines[0]).toContain(`Package '${TARGET}' was not installed`); + // Logged once, with the error itself attached: its `cause` is the only + // place the driver's text survives. + const logs = refusalLogs(warn); + expect(logs).toHaveLength(1); + expect(String(logs[0]![0])).toContain(`'${TARGET}'`); + expect(logs[0]![1]).toBe(err); + expect((err as { cause?: any }).cause?.driverFault?.message).toBe(DRIVER_TEXT); }); - it('CONTROL: an accepted install adds no package entry and logs no line', async () => { - const { impl, packages } = makeHarness({ publish: async () => ({ success: true }) }); + it('CONTROL: an accepted install still copies, and logs no refusal', async () => { + const { impl, packages, saveMetaItem } = makeHarness({ publish: async () => ({ success: true }) }); const res = await (impl as any).duplicatePackage({ sourcePackageId: SOURCE, targetPackageId: TARGET }); expect(res.failed).toEqual([]); expect(res.success).toBe(true); expect(res.copied).toEqual([{ type: 'object', name: 'leave_copy_ticket' }]); + expect(saveMetaItem).toHaveBeenCalledTimes(1); expect(packages.get(TARGET)?.manifest?.namespace).toBe('leave_copy'); - expect(packageRecordLines(warn)).toEqual([]); + expect(refusalLogs(warn)).toEqual([]); }); }); diff --git a/packages/metadata-protocol/src/protocol.duplicate-package-target-namespace.test.ts b/packages/metadata-protocol/src/protocol.duplicate-package-target-namespace.test.ts index d5d0d35ef01..289322c09f5 100644 --- a/packages/metadata-protocol/src/protocol.duplicate-package-target-namespace.test.ts +++ b/packages/metadata-protocol/src/protocol.duplicate-package-target-namespace.test.ts @@ -128,9 +128,9 @@ describe('[#19577] duplicatePackage refuses an explicit targetNamespace the decl expect(err.message).toContain(declarationSentence(ns)); // ⭐ Nothing was minted and nothing was scanned: the refusal - // precedes the manifest write AND the copy loop. A refusal of the - // manifest write does not abort the copy, so a refusal raised only - // there would be a `failed[]` entry on a copy that went on anyway. + // precedes the manifest write AND the copy loop. The manifest write + // does not judge a namespace's charset at all, so this is the only + // place the refusal can be raised. expect(installed).toHaveLength(0); expect(engine.registry.installPackage).not.toHaveBeenCalled(); expect(engine.find).not.toHaveBeenCalled(); diff --git a/packages/metadata-protocol/src/protocol.install-manifest-id.test.ts b/packages/metadata-protocol/src/protocol.install-manifest-id.test.ts index 97b27d7b51d..5e6c084aaf8 100644 --- a/packages/metadata-protocol/src/protocol.install-manifest-id.test.ts +++ b/packages/metadata-protocol/src/protocol.install-manifest-id.test.ts @@ -183,9 +183,9 @@ describe('[#19417] duplicatePackage refuses a target id the declaration refuses' expect(err.message).toBe(manifestIdRefusal('targetPackageId', id)); // ⭐ Nothing was minted and nothing was scanned: the refusal // precedes the manifest write AND the copy loop, so no empty shell - // is left behind. A refusal of the manifest write does not abort - // the copy — a refusal raised only there would be a `failed[]` - // entry on a copy that went on anyway. + // is left behind. A refusal raised only at the manifest write would + // refuse the duplicate too, but under `manifest.id`, a key the + // caller never wrote. expect(installed).toHaveLength(0); expect(engine.registry.installPackage).not.toHaveBeenCalled(); expect(engine.find).not.toHaveBeenCalled(); diff --git a/packages/metadata-protocol/src/protocol.ts b/packages/metadata-protocol/src/protocol.ts index 16e109bda3b..bf8238167ab 100644 --- a/packages/metadata-protocol/src/protocol.ts +++ b/packages/metadata-protocol/src/protocol.ts @@ -27002,7 +27002,9 @@ export class ObjectStackProtocolImplementation implements * `sys_metadata` keys on (type,name,org), so a same-name copy would collide * with the source — and rewriting every intra-package reference (lookup * `reference`, view `object`, expressions, etc.) to the new names. Per-item - * best-effort; one failure never aborts the whole clone. + * best-effort; one failure never aborts the whole clone. The copy's package + * record is not an item: when its install is refused, that refusal refuses + * the whole duplicate, before any item is written. * * Every copied object is named in the target namespace: `iojn_customer` * becomes `iojn2_customer`, and an object whose name lacks the source @@ -27056,11 +27058,10 @@ export class ObjectStackProtocolImplementation implements // covers the WRITE. It does not cover this method, for two reasons that // are both about where the refusal lands: // - // ① a refusal of the `installPackage` call below does not abort the - // copy (a manifest row is not worth aborting a copy for): it is - // reported as a `failed[]` entry on a duplicate that goes on, so - // a refusal of the TARGET ID raised there would arrive as a - // partial copy under an id the declaration refuses outright; + // ① a refusal of the `installPackage` call below is rethrown before + // any item is copied, so it would not be lost — but it names + // `manifest.id`, the key of a manifest this door builds, never the + // `targetPackageId` the caller wrote (the closing paragraph below); // ② the refusal has to precede the MUTATIONS, and the manifest write // is not the first of them. #14451 already established the position // for this door's other precondition: `duplicatePackage` mints the @@ -27433,9 +27434,6 @@ export class ObjectStackProtocolImplementation implements return { fields: Object.fromEntries(out) }; }; - const copied: Array<{ type: string; name: string }> = []; - const failed: Array<{ type: string; name: string; error: string }> = []; - if (srcPkg?.manifest && typeof registry?.installPackage === 'function') { try { // Route through installPackage (not a bare registry write) so the @@ -27454,45 +27452,45 @@ export class ObjectStackProtocolImplementation implements delete dupManifest.scope; await this.installPackage({ manifest: dupManifest } as InstallPackageRequest); } catch (e: any) { - // [#24041] A REFUSED PACKAGE RECORD IS REPORTED, NEVER SWALLOWED. - // The copy still goes on: the per-item package binding works - // without a package record, and aborting here would leave the - // caller nothing it has today (refusing the duplicate outright - // is a separate decision). What it no longer does is go on in - // SILENCE. A refused install leaves no registry record and no - // `sys_packages` row, so `targetPackageId` is not an installed - // package — absent from `GET /packages` at once, not only after - // a restart — while every item copied below is bound to it. + // [#24041] ⭐ A REFUSED PACKAGE RECORD REFUSES THE DUPLICATE — + // the refusal itself, rethrown unchanged, before any item is + // copied (ruling B, claim amendment 6227008947). // - // Reachable from the HTTP door: `SchemaRegistry.installPackage`'s - // namespace gate (`NAMESPACE_CONFLICT`, 422) refuses a target - // namespace another installed package owns — an explicit - // `targetNamespace`, or one derived from a target id whose last - // segment another package's namespace already uses — and a - // failed `sys_packages` write fails the install (#21243). + // This catch used to drop it and copy anyway. A refused install + // leaves no registry record and no `sys_packages` row, so the + // copy's items were live, bound to an id no package record + // holds, absent from `GET /packages`. After a namespace conflict + // they also sat inside a namespace another package owns — the + // very state ADR-0048 Phase 1's install-time gate in + // `SchemaRegistry.installPackage` exists to refuse. The sibling + // verb {@link importUnmanagedPackage} states the rule this door + // now keeps: "There is no half-copied app." // - // ⭐ It rides `failed[]`, this verb's one per-item outcome - // carrier, under the source's own `package` item: the clone - // could not copy it. So `success` reads false and `failedCount` - // counts it, as for any item. The sentence is quoted only when - // the refusal declares a client status; a store fault is not - // quoted, and the warn below carries its full text, exactly as - // the per-item save failure does further down. - failed.push({ - type: 'package', - name: request.sourcePackageId, - error: `Package '${request.sourcePackageId}' is copied without its package record: installing ` - + `'${request.targetPackageId}' was refused, so '${request.targetPackageId}' is not an ` - + 'installed package while the items copied into it are bound to it. ' - + clientFacingFailureText(e, 'The install failed; the reason is in the server log.'), - }); + // Nothing needs undoing: this call is the duplicate's FIRST + // mutation (everything above it reads), and a refused install + // leaves nothing behind — the registry judges before it writes, + // and a failed persist undoes the registry write before it + // throws (#21243). So the door answers the refusal's own status: + // `422 NAMESPACE_CONFLICT` with the registry's remedy (choose + // another namespace), or the primitive's `500` for a store fault. + // + // Logged once, because neither the primitive nor the HTTP door + // logs it. A store fault's driver text rides only on the error's + // `cause`, which no answer quotes, so the error itself is handed + // to the log for that one; a client refusal's sentence is + // already the whole story. console.warn( - `[protocol.duplicatePackage] package record for '${request.targetPackageId}' ` - + `(copy of '${request.sourcePackageId}') was not installed: ` + `[protocol.duplicatePackage] refused: the package record for '${request.targetPackageId}' ` + + `(copy of '${request.sourcePackageId}') was not installed, so nothing was copied: ` + `${typeof e?.code === 'string' ? `${e.code}: ` : ''}${e?.message ?? e}`, + ...(declaresClientRefusal(e) ? [] : [e]), ); + throw e; } } + + const copied: Array<{ type: string; name: string }> = []; + const failed: Array<{ type: string; name: string; error: string }> = []; // Resolved once for the whole copy: every flow row in this package needs // the same engine, and a package with fifty flows should not walk the // service table fifty times. From de14c8521a5d09a39d85476fa8434f9632b5be5a Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 17 Oct 2026 00:17:50 +0000 Subject: [PATCH 5/5] chore(changeset): the duplicate's refused install is a narrowing, minor, with its migration prose Claude-Session: https://claude.ai/code/session_01TQLy3Yq6vF4SDpTXdr3Uwi Co-authored-by: Claude --- .changeset/24041-duplicate-install-refusal.md | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/.changeset/24041-duplicate-install-refusal.md b/.changeset/24041-duplicate-install-refusal.md index 1eca7946b58..879c6d87382 100644 --- a/.changeset/24041-duplicate-install-refusal.md +++ b/.changeset/24041-duplicate-install-refusal.md @@ -1,12 +1,14 @@ --- -"@objectstack/metadata-protocol": patch +"@objectstack/metadata-protocol": minor --- -fix(metadata-protocol): `duplicatePackage` reports a refused install of the copy's package record, on its answer and in the log +fix(metadata-protocol): `duplicatePackage` is refused, with nothing copied, when the install of the copy's package record is refused -Clause-②: no +Clause-②: no (narrowing) -- **What was silent.** The duplicate installs the copy's package record before it copies the items, and a refusal of that install was caught and dropped. A refused install leaves no registry record and no `sys_packages` row, so the copy's id was not an installed package (absent from `GET /packages` at once, not only after a restart) while every copied item was bound to it, and the answer read `success: true` with an empty `failed[]`. Nothing was logged. It is reached whenever the copy's namespace is one another installed package owns (`NAMESPACE_CONFLICT`, 422): an explicit `targetNamespace`, or one derived from a target id whose last segment another package's namespace already uses. A failed `sys_packages` write fails the install the same way. -- **The answer now names it.** One `failed[]` entry `{ type: 'package', name, error }`, where `name` is the source package id (the source's package item, which the clone could not copy). Its `error` names the target id and quotes the refusal when the refusal declares a client status, so a namespace conflict arrives with its own remedy. A store fault is not quoted. The duplicate's `success` is therefore `false` and `failedCount` counts the entry; the HTTP status stays 200, so a consumer reading `data.success` now sees a partial duplicate. -- **The log now names it.** One `warn` line from `[protocol.duplicatePackage]` names the target id and the source id, then the refusal's code and full message, store faults included. -- **Unchanged:** the duplicate still copies the items after a refused install; the response's shape; every other `failed[]` entry. + + +- **Refused now.** `duplicatePackage`, and `POST /api/v1/packages/:id/duplicate` which calls it, installs the copy's package record before it copies any item. When that install is refused, the duplicate is now refused with that same refusal, and nothing is minted or copied. A copy whose namespace another installed package owns answers `422 NAMESPACE_CONFLICT` with the registry's own sentence (ADR-0048); that covers an explicit `targetNamespace` and one derived from the last segment of `targetPackageId`. A failed `sys_packages` write answers the install's own `500`. Before, both refusals were dropped and the items were copied anyway: bound to an id with no package record (absent from `GET /packages`), after a namespace conflict inside the namespace the other package owns, and the answer read `success: true`. +- **Unchanged.** A duplicate whose package record installs copies exactly as before, with the same answer shape, and per-item failures still ride `failed[]`. A source with no package record still copies without installing one. +- **Remedy.** Choose a target namespace no installed package owns: pass `targetNamespace` explicitly, or pick a `targetPackageId` whose last segment derives a free one. For a deliberate migration, `OS_METADATA_COLLISION=warn` still downgrades the namespace gate to a warning, and the duplicate then proceeds. +- **Logged.** The refusal is logged once at `warn` by `[protocol.duplicatePackage]`, naming the target and source ids and the refusal's code and message. For a store fault the error itself is attached, so its cause reaches the log.