fix(metadata-protocol): duplicatePackage reports a refused install of the copy's package record

.changeset/24041-duplicate-install-refusal.md

@@ -0,0 +1,14 @@
+---
+"@objectstack/metadata-protocol": minor
+---
+
+fix(metadata-protocol): `duplicatePackage` is refused, with nothing copied, when the install of the copy's package record is refused
+
+Clause-②: no (narrowing)
+
+<!-- adr-0087: not-required (no-migration-prescription) The refusal is one verb's runtime answer at call time; no stored metadata, spec key or published type changes, so there is nothing for `objectstack migrate meta` to rewrite. The remedy is a different argument on the next call. -->
+
+- **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.

packages/metadata-protocol/src/protocol.duplicate-package-install-refusal.test.ts

@@ -0,0 +1,213 @@
+// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
+
+/**
+ * [#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 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 (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.
+ *
+ * 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';
+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. Choose a different namespace for `
+            + `"${incoming}", or uninstall "${existing}" first.`,
+        ),
+        { 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. Whatever it
+ * throws is kept in `thrown`, so a pin can ask for that very object back.
+ */
+function makeHarness(opts: { publish?: (data: unknown) => Promise<unknown> } = {}) {
+    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<string, any>([
+        [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<string, string[]>([['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) {
+                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]);
+            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<string, unknown>();
+    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, packages, namespaceOwners, saveMetaItem, thrown };
+}
+
+/** The thrown refusal, or a failure naming what happened instead. */
+async function refusalOf(run: () => Promise<unknown>): Promise<any> {
+    try {
+        await run();
+    } catch (e) {
+        return e;
+    }
+    throw new Error('expected the duplicate to be refused, but it resolved');
+}
+
+/** 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(() => {}) as never;
+    });
+    afterEach(() => {
+        warn.mockRestore();
+    });
+
+    it('answers the registry\'s namespace refusal (422 NAMESPACE_CONFLICT) unchanged, and copies nothing', async () => {
+        const { impl, packages, namespaceOwners, saveMetaItem, thrown } = makeHarness();
+
+        const err = await refusalOf(() => (impl as any).duplicatePackage({
+            sourcePackageId: SOURCE,
+            targetPackageId: TARGET,
+            targetNamespace: 'taken',
+        }));
+
+        // 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);
+        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('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 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');
+
+        // Nothing was copied, and the primitive undid its registry write.
+        expect(saveMetaItem).not.toHaveBeenCalled();
+        expect(packages.has(TARGET)).toBe(false);
+
+        // 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 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(refusalLogs(warn)).toEqual([]);
+    });
+});

packages/metadata-protocol/src/protocol.duplicate-package-target-namespace.test.ts

@@ -129,8 +129,8 @@ describe('[#19577] duplicatePackage refuses an explicit targetNamespace the decl
 
             // ⭐ 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`.
+            // 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();

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 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();

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,12 +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:
         //
-        //   ① 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 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
@@ -27451,8 +27451,41 @@ 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 REFUSES THE DUPLICATE —
+                // the refusal itself, rethrown unchanged, before any item is
+                // copied (ruling B, claim amendment 6227008947).
+                //
+                // 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."
+                //
+                // 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] 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;
             }
         }