Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 33 additions & 0 deletions .changeset/6593-duplicate-package-envelope-unwrap.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
---
'@object-ui/app-shell': patch
---

Studio's "duplicate base" now reports a partial or empty duplicate as a failure instead of
a complete success (objectui#6593).

`duplicatePackage()` read `success` at the **top level** of the response body. That is the
runtime dispatcher's envelope — `deps.success(result)` answers
`{ success: true, data }` — so on every HTTP 200 the flag it read was `true` by
construction. The operation's own verdict lives one level down in `data` and is a real
three-state: the server computes it as `failed.length === 0 && copied.length > 0`.

Two outcomes therefore answered 200 with `envelope.success: true` while the duplicate had
not succeeded, and both were shown to the author as "created", followed by a navigation
into the new base:

- **partial** — some items failed to copy. `failed[]` carries a per-item `error` string
that is the only place the reason is ever stated, and none of it was read.
- **empty** — nothing was copied at all, e.g. an all-env-wide source package under a
session that resolves no active organization (`copiedCount: 0`, `failedCount: 0`).

`duplicatePackage()` now unwraps `data` before reading the operation flag, and rejects with
a message built from what actually happened: the copied/failed counts, plus each
`failed[].error` (the first five by name, then a `+N more` tail). A generic `HTTP nnn`
message is deliberately not sufficient for the partial arm — it is the half an author needs
to act on. The non-2xx arm is unchanged and still surfaces the error envelope's message.

The unwrap-then-read order is the one `revertCommit` already uses for the sibling
commit-revert route in `preview/commitHistory.ts`; this is one consumer converging on that,
not a new convention. The route's underlying contract absence (it publishes no response
schema, so reading the wrong `success` typechecks perfectly) stays upstream in
objectstack#12038.
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,203 @@
// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license.

/**
* `duplicatePackage` must read the OPERATION's verdict, not the ENVELOPE's.
*
* `POST /packages/:id/duplicate` is served only by the runtime dispatcher, and
* that door always wraps: `deps.success(result)` answers
* `{ status: 200, body: { success: true, data } }`. So the top-level `success`
* is `true` by construction on every 200, and reading it classifies every 200
* as a complete success. The operation's own verdict lives one level down in
* `data`, computed server-side as `failed.length === 0 && copied.length > 0`.
*
* That leaves TWO reachable outcomes which answer HTTP 200 while the duplicate
* did not succeed, and this file pins BOTH — a suite that only covered
* `res.ok === false` would re-create the defect, because the transport arm was
* never the broken one:
*
* 1. PARTIAL — `failed.length > 0`. The per-item `error` strings in `failed[]`
* are the only place the reason is ever stated, so a generic `HTTP nnn`
* message is explicitly NOT sufficient here; that is asserted, not implied.
* 2. EMPTY — `copied.length === 0` with nothing named as failed, e.g. an
* all-env-wide source package under a session that resolves no active
* organization. Measured upstream as `{ success: false, copiedCount: 0,
* failedCount: 0 }`.
*
* The pattern being converged on is the sibling commit-revert helper in
* `preview/commitHistory.ts` (`revertCommit`), which already unwraps `data`
* before reading the flag. This is one consumer catching up to it, not a new
* convention.
*/

import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
import { duplicatePackage } from './packages-io';

/** The dispatcher's success envelope: the operation result under `data`. */
function envelope(data: unknown, status = 200) {
return { ok: status >= 200 && status < 300, status, json: async () => ({ success: true, data }) } as unknown as Response;
}

/** The dispatcher's error envelope — top-level, with no `data` to unwrap. */
function errorEnvelope(status: number, message: string) {
return {
ok: false,
status,
json: async () => ({ success: false, error: { message, code: 'PERMISSION_DENIED', status } }),
} as unknown as Response;
}

function stubFetch(response: Response) {
const fetchMock = vi.fn().mockResolvedValue(response);
vi.stubGlobal('fetch', fetchMock);
return fetchMock;
}

beforeEach(() => {
vi.restoreAllMocks();
});

afterEach(() => {
vi.unstubAllGlobals();
});

describe('duplicatePackage — the operation verdict lives under `data`', () => {
it('rejects a PARTIAL duplicate and names the counts and every per-item error', async () => {
stubFetch(
envelope({
success: false,
copiedCount: 7,
failedCount: 2,
targetPackageId: 'com.example.leave-copy',
copied: [],
failed: [
{ type: 'object', name: 'leave_request', error: 'copy failed: duplicate key' },
{ type: 'view', name: 'leave_list', error: 'the source item does not convert' },
],
}),
);

const err = await duplicatePackage('com.example.leave', 'com.example.leave-copy').then(
() => null,
(e: unknown) => e as Error,
);

expect(err).toBeInstanceOf(Error);
// The counts…
expect(err?.message).toContain('7');
expect(err?.message).toContain('2');
// …and the per-item reasons, which exist nowhere else in the response.
expect(err?.message).toContain('object/leave_request');
expect(err?.message).toContain('copy failed: duplicate key');
expect(err?.message).toContain('view/leave_list');
expect(err?.message).toContain('the source item does not convert');
// A generic transport message is NOT sufficient for this arm — it is the
// half the fix exists to deliver.
expect(err?.message).not.toMatch(/HTTP \d/);
});

it('summarizes the tail rather than pasting an unbounded failure list', async () => {
stubFetch(
envelope({
success: false,
copiedCount: 0,
failedCount: 7,
failed: Array.from({ length: 7 }, (_, i) => ({
type: 'object',
name: `obj_${i}`,
error: `copy failed ${i}`,
})),
}),
);

const err = await duplicatePackage('a.b.c', 'a.b.d').catch((e: unknown) => e as Error);

expect(err?.message).toContain('object/obj_0');
expect(err?.message).toContain('object/obj_4');
expect(err?.message).not.toContain('object/obj_5');
expect(err?.message).toContain('+2 more');
});

it('rejects an EMPTY duplicate — 200, nothing copied, nothing named as failed', async () => {
stubFetch(
envelope({
success: false,
copiedCount: 0,
failedCount: 0,
targetPackageId: 'com.example.leave-copy',
copied: [],
failed: [],
}),
);

const err = await duplicatePackage('com.example.leave', 'com.example.leave-copy').then(
() => null,
(e: unknown) => e as Error,
);

expect(err).toBeInstanceOf(Error);
expect(err?.message).toMatch(/nothing was copied/i);
// The empty arm has no `failed[]` to quote, so the message must still say
// something the author can act on instead of falling through to a status.
expect(err?.message).not.toMatch(/HTTP \d/);
});

it('does NOT let the envelope alone stand in for the operation verdict', async () => {
// The exact shape the defect read as success: `success: true` at the top
// level, `success: false` one level down.
const fetchMock = stubFetch(
envelope({ success: false, copiedCount: 0, failedCount: 1, failed: [{ type: 'object', name: 'x', error: 'nope' }] }),
);

await expect(duplicatePackage('a.b.c', 'a.b.d')).rejects.toThrow();
expect(fetchMock).toHaveBeenCalledTimes(1);
});

it('resolves when the operation itself succeeded', async () => {
stubFetch(
envelope({
success: true,
copiedCount: 9,
failedCount: 0,
targetPackageId: 'com.example.leave-copy',
copied: [{ type: 'object', name: 'leave_request' }],
failed: [],
}),
);

await expect(duplicatePackage('com.example.leave', 'com.example.leave-copy')).resolves.toBeUndefined();
});

it('still surfaces the error envelope message on a non-2xx', async () => {
stubFetch(errorEnvelope(403, 'Permission denied: manage_metadata is required'));

await expect(duplicatePackage('a.b.c', 'a.b.d')).rejects.toThrow(
'Permission denied: manage_metadata is required',
);
});

it('falls back to the status when a non-2xx carries no readable body', async () => {
stubFetch({
ok: false,
status: 502,
json: async () => {
throw new SyntaxError('Unexpected token < in JSON at position 0');
},
} as unknown as Response);

await expect(duplicatePackage('a.b.c', 'a.b.d')).rejects.toThrow('HTTP 502');
});

it('posts the target id and name to the duplicate route', async () => {
const fetchMock = stubFetch(envelope({ success: true, copiedCount: 1, failedCount: 0, copied: [{}], failed: [] }));

await duplicatePackage('com.example.leave', 'com.example.leave-copy', 'Leave (copy)');

expect(fetchMock).toHaveBeenCalledWith(
'/api/v1/packages/com.example.leave/duplicate',
expect.objectContaining({
method: 'POST',
body: JSON.stringify({ targetPackageId: 'com.example.leave-copy', targetName: 'Leave (copy)' }),
}),
);
});
});
81 changes: 76 additions & 5 deletions packages/app-shell/src/views/studio-design/packages-io.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -54,11 +54,71 @@ export async function fetchPackages(): Promise<PkgEntry[]> {
return parsePackages(await res.json());
}

/**
* The duplicate operation's OWN result, which travels inside the dispatcher
* envelope's `data` — not at the top level.
*
* `POST /packages/:id/duplicate` is served only by the runtime dispatcher, and
* that door always wraps (`deps.success(result)` → `{ success: true, data }`).
* So a top-level `success` read is reading the ENVELOPE, whose value is `true`
* by construction on a 200. The operation's verdict is one level down, and it
* is a real three-state: the server computes it as
* `failed.length === 0 && copied.length > 0`, which leaves TWO reachable
* outcomes that answer HTTP 200 with `data.success === false`:
*
* 1. PARTIAL — some items failed to copy. `failed[]` carries a per-item
* `error` string and is the ONLY place the reason is ever stated; a
* generic `HTTP nnn` here tells the author nothing.
* 2. EMPTY — nothing was copied at all (e.g. a source package whose rows are
* outside this caller's scope). `failed[]` is empty in this arm — nothing
* is named as having failed — so the counts are the only signal.
*
* Those two are exhaustive for `success === false`: with no failures AND at
* least one copy the server would have said `true`.
*/
interface DuplicateOutcome {
success?: boolean;
copiedCount?: number;
failedCount?: number;
failed?: Array<{ type?: string; name?: string; error?: string }>;
}

/** Per-item failures named in full before the message summarizes the rest. */
const DUPLICATE_FAILURE_DETAIL_LIMIT = 5;

/**
* Turn a false operation verdict into something the Studio author can act on.
* The counts say how far the copy got; `failed[].error` says why each item did
* not make it. Both arms are reachable on a 200 — see {@link DuplicateOutcome}.
*/
function describeDuplicateFailure(outcome: DuplicateOutcome): string {
const failed = Array.isArray(outcome.failed) ? outcome.failed : [];
const failedCount = typeof outcome.failedCount === 'number' ? outcome.failedCount : failed.length;
const copiedCount = typeof outcome.copiedCount === 'number' ? outcome.copiedCount : 0;
if (failedCount > 0) {
const shown = failed
.slice(0, DUPLICATE_FAILURE_DETAIL_LIMIT)
.map((f) => `${f.type || 'item'}/${f.name || '?'}: ${f.error || 'copy failed'}`);
const rest = failed.length - shown.length;
const detail = shown.length ? ` ${shown.join('; ')}${rest > 0 ? `; +${rest} more` : ''}` : '';
return `Partial duplicate: ${copiedCount} item(s) copied, ${failedCount} failed.${detail}`;
}
return (
'Nothing was copied: the duplicate is empty (0 items copied, 0 reported as failed). ' +
"The source package may have no active items, or its rows may be outside this session's scope."
);
}

/**
* Duplicate a package into a NEW writable base (ADR-0070 D4 — the Airtable
* "duplicate base" gesture; POST /packages/:id/duplicate). This is how a
* read-only code package becomes a customizable starting point: objects are
* re-namespaced and intra-package references rewritten server-side.
*
* Rejects on a transport/error-envelope failure AND on a false operation
* verdict, which a 200 can carry. Unwrapping before reading that verdict is
* the same order `revertCommit` uses for the sibling commit-revert route in
* `preview/commitHistory.ts`.
*/
export async function duplicatePackage(sourceId: string, targetId: string, targetName?: string): Promise<void> {
const res = await fetch(`/api/v1/packages/${encodeURIComponent(sourceId)}/duplicate`, {
Expand All@@ -67,11 +127,22 @@ export async function duplicatePackage(sourceId: string, targetId: string, targe
headers: { 'Content-Type': 'application/json', Accept: 'application/json' },
body: JSON.stringify({ targetPackageId: targetId, ...(targetName ? { targetName } : {}) }),
});
const payload = (await res.json().catch(() => null)) as
| { success?: boolean; error?: { message?: string } }
| null;
if (!res.ok || payload?.success === false) {
throw new Error(payload?.error?.message || `HTTP ${res.status}`);
const payload = (await res.json().catch(() => null)) as Record<string, unknown> | null;
if (!res.ok) {
// The error envelope IS top-level (`{ success: false, error }`) — no `data`
// to unwrap on this arm.
const message = (payload?.error as { message?: string } | undefined)?.message;
throw new Error(message || `HTTP ${res.status}`);
}
// Unwrap FIRST, then read the operation's flag. The `?? payload` arm mirrors
// the commit-revert helper: it would classify a hypothetical bare (unwrapped)
// answer instead of reading it as success. Against today's single wrapping
// surface only the `data` arm is live.
const inner = ((payload?.data as Record<string, unknown> | undefined) ?? payload ?? undefined) as
| DuplicateOutcome
| undefined;
if (inner?.success === false) {
throw new Error(describeDuplicateFailure(inner));
}
}

Expand Down
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 33 additions & 0 deletions .changeset/6593-duplicate-package-envelope-unwrap.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
---
'@object-ui/app-shell': patch
---

Studio's "duplicate base" now reports a partial or empty duplicate as a failure instead of
a complete success (objectui#6593).

`duplicatePackage()` read `success` at the **top level** of the response body. That is the
runtime dispatcher's envelope — `deps.success(result)` answers
`{ success: true, data }` — so on every HTTP 200 the flag it read was `true` by
construction. The operation's own verdict lives one level down in `data` and is a real
three-state: the server computes it as `failed.length === 0 && copied.length > 0`.

Two outcomes therefore answered 200 with `envelope.success: true` while the duplicate had
not succeeded, and both were shown to the author as "created", followed by a navigation
into the new base:

- **partial** — some items failed to copy. `failed[]` carries a per-item `error` string
that is the only place the reason is ever stated, and none of it was read.
- **empty** — nothing was copied at all, e.g. an all-env-wide source package under a
session that resolves no active organization (`copiedCount: 0`, `failedCount: 0`).

`duplicatePackage()` now unwraps `data` before reading the operation flag, and rejects with
a message built from what actually happened: the copied/failed counts, plus each
`failed[].error` (the first five by name, then a `+N more` tail). A generic `HTTP nnn`
message is deliberately not sufficient for the partial arm — it is the half an author needs
to act on. The non-2xx arm is unchanged and still surfaces the error envelope's message.

The unwrap-then-read order is the one `revertCommit` already uses for the sibling
commit-revert route in `preview/commitHistory.ts`; this is one consumer converging on that,
not a new convention. The route's underlying contract absence (it publishes no response
schema, so reading the wrong `success` typechecks perfectly) stays upstream in
objectstack#12038.
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,203 @@
// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license.

/**
* `duplicatePackage` must read the OPERATION's verdict, not the ENVELOPE's.
*
* `POST /packages/:id/duplicate` is served only by the runtime dispatcher, and
* that door always wraps: `deps.success(result)` answers
* `{ status: 200, body: { success: true, data } }`. So the top-level `success`
* is `true` by construction on every 200, and reading it classifies every 200
* as a complete success. The operation's own verdict lives one level down in
* `data`, computed server-side as `failed.length === 0 && copied.length > 0`.
*
* That leaves TWO reachable outcomes which answer HTTP 200 while the duplicate
* did not succeed, and this file pins BOTH — a suite that only covered
* `res.ok === false` would re-create the defect, because the transport arm was
* never the broken one:
*
* 1. PARTIAL — `failed.length > 0`. The per-item `error` strings in `failed[]`
* are the only place the reason is ever stated, so a generic `HTTP nnn`
* message is explicitly NOT sufficient here; that is asserted, not implied.
* 2. EMPTY — `copied.length === 0` with nothing named as failed, e.g. an
* all-env-wide source package under a session that resolves no active
* organization. Measured upstream as `{ success: false, copiedCount: 0,
* failedCount: 0 }`.
*
* The pattern being converged on is the sibling commit-revert helper in
* `preview/commitHistory.ts` (`revertCommit`), which already unwraps `data`
* before reading the flag. This is one consumer catching up to it, not a new
* convention.
*/

import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
import { duplicatePackage } from './packages-io';

/** The dispatcher's success envelope: the operation result under `data`. */
function envelope(data: unknown, status = 200) {
return { ok: status >= 200 && status < 300, status, json: async () => ({ success: true, data }) } as unknown as Response;
}

/** The dispatcher's error envelope — top-level, with no `data` to unwrap. */
function errorEnvelope(status: number, message: string) {
return {
ok: false,
status,
json: async () => ({ success: false, error: { message, code: 'PERMISSION_DENIED', status } }),
} as unknown as Response;
}

function stubFetch(response: Response) {
const fetchMock = vi.fn().mockResolvedValue(response);
vi.stubGlobal('fetch', fetchMock);
return fetchMock;
}

beforeEach(() => {
vi.restoreAllMocks();
});

afterEach(() => {
vi.unstubAllGlobals();
});

describe('duplicatePackage — the operation verdict lives under `data`', () => {
it('rejects a PARTIAL duplicate and names the counts and every per-item error', async () => {
stubFetch(
envelope({
success: false,
copiedCount: 7,
failedCount: 2,
targetPackageId: 'com.example.leave-copy',
copied: [],
failed: [
{ type: 'object', name: 'leave_request', error: 'copy failed: duplicate key' },
{ type: 'view', name: 'leave_list', error: 'the source item does not convert' },
],
}),
);

const err = await duplicatePackage('com.example.leave', 'com.example.leave-copy').then(
() => null,
(e: unknown) => e as Error,
);

expect(err).toBeInstanceOf(Error);
// The counts…
expect(err?.message).toContain('7');
expect(err?.message).toContain('2');
// …and the per-item reasons, which exist nowhere else in the response.
expect(err?.message).toContain('object/leave_request');
expect(err?.message).toContain('copy failed: duplicate key');
expect(err?.message).toContain('view/leave_list');
expect(err?.message).toContain('the source item does not convert');
// A generic transport message is NOT sufficient for this arm — it is the
// half the fix exists to deliver.
expect(err?.message).not.toMatch(/HTTP \d/);
});

it('summarizes the tail rather than pasting an unbounded failure list', async () => {
stubFetch(
envelope({
success: false,
copiedCount: 0,
failedCount: 7,
failed: Array.from({ length: 7 }, (_, i) => ({
type: 'object',
name: `obj_${i}`,
error: `copy failed ${i}`,
})),
}),
);

const err = await duplicatePackage('a.b.c', 'a.b.d').catch((e: unknown) => e as Error);

expect(err?.message).toContain('object/obj_0');
expect(err?.message).toContain('object/obj_4');
expect(err?.message).not.toContain('object/obj_5');
expect(err?.message).toContain('+2 more');
});

it('rejects an EMPTY duplicate — 200, nothing copied, nothing named as failed', async () => {
stubFetch(
envelope({
success: false,
copiedCount: 0,
failedCount: 0,
targetPackageId: 'com.example.leave-copy',
copied: [],
failed: [],
}),
);

const err = await duplicatePackage('com.example.leave', 'com.example.leave-copy').then(
() => null,
(e: unknown) => e as Error,
);

expect(err).toBeInstanceOf(Error);
expect(err?.message).toMatch(/nothing was copied/i);
// The empty arm has no `failed[]` to quote, so the message must still say
// something the author can act on instead of falling through to a status.
expect(err?.message).not.toMatch(/HTTP \d/);
});

it('does NOT let the envelope alone stand in for the operation verdict', async () => {
// The exact shape the defect read as success: `success: true` at the top
// level, `success: false` one level down.
const fetchMock = stubFetch(
envelope({ success: false, copiedCount: 0, failedCount: 1, failed: [{ type: 'object', name: 'x', error: 'nope' }] }),
);

await expect(duplicatePackage('a.b.c', 'a.b.d')).rejects.toThrow();
expect(fetchMock).toHaveBeenCalledTimes(1);
});

it('resolves when the operation itself succeeded', async () => {
stubFetch(
envelope({
success: true,
copiedCount: 9,
failedCount: 0,
targetPackageId: 'com.example.leave-copy',
copied: [{ type: 'object', name: 'leave_request' }],
failed: [],
}),
);

await expect(duplicatePackage('com.example.leave', 'com.example.leave-copy')).resolves.toBeUndefined();
});

it('still surfaces the error envelope message on a non-2xx', async () => {
stubFetch(errorEnvelope(403, 'Permission denied: manage_metadata is required'));

await expect(duplicatePackage('a.b.c', 'a.b.d')).rejects.toThrow(
'Permission denied: manage_metadata is required',
);
});

it('falls back to the status when a non-2xx carries no readable body', async () => {
stubFetch({
ok: false,
status: 502,
json: async () => {
throw new SyntaxError('Unexpected token < in JSON at position 0');
},
} as unknown as Response);

await expect(duplicatePackage('a.b.c', 'a.b.d')).rejects.toThrow('HTTP 502');
});

it('posts the target id and name to the duplicate route', async () => {
const fetchMock = stubFetch(envelope({ success: true, copiedCount: 1, failedCount: 0, copied: [{}], failed: [] }));

await duplicatePackage('com.example.leave', 'com.example.leave-copy', 'Leave (copy)');

expect(fetchMock).toHaveBeenCalledWith(
'/api/v1/packages/com.example.leave/duplicate',
expect.objectContaining({
method: 'POST',
body: JSON.stringify({ targetPackageId: 'com.example.leave-copy', targetName: 'Leave (copy)' }),
}),
);
});
});
81 changes: 76 additions & 5 deletions packages/app-shell/src/views/studio-design/packages-io.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -54,11 +54,71 @@ export async function fetchPackages(): Promise<PkgEntry[]> {
return parsePackages(await res.json());
}

/**
* The duplicate operation's OWN result, which travels inside the dispatcher
* envelope's `data` — not at the top level.
*
* `POST /packages/:id/duplicate` is served only by the runtime dispatcher, and
* that door always wraps (`deps.success(result)` → `{ success: true, data }`).
* So a top-level `success` read is reading the ENVELOPE, whose value is `true`
* by construction on a 200. The operation's verdict is one level down, and it
* is a real three-state: the server computes it as
* `failed.length === 0 && copied.length > 0`, which leaves TWO reachable
* outcomes that answer HTTP 200 with `data.success === false`:
*
* 1. PARTIAL — some items failed to copy. `failed[]` carries a per-item
* `error` string and is the ONLY place the reason is ever stated; a
* generic `HTTP nnn` here tells the author nothing.
* 2. EMPTY — nothing was copied at all (e.g. a source package whose rows are
* outside this caller's scope). `failed[]` is empty in this arm — nothing
* is named as having failed — so the counts are the only signal.
*
* Those two are exhaustive for `success === false`: with no failures AND at
* least one copy the server would have said `true`.
*/
interface DuplicateOutcome {
success?: boolean;
copiedCount?: number;
failedCount?: number;
failed?: Array<{ type?: string; name?: string; error?: string }>;
}

/** Per-item failures named in full before the message summarizes the rest. */
const DUPLICATE_FAILURE_DETAIL_LIMIT = 5;

/**
* Turn a false operation verdict into something the Studio author can act on.
* The counts say how far the copy got; `failed[].error` says why each item did
* not make it. Both arms are reachable on a 200 — see {@link DuplicateOutcome}.
*/
function describeDuplicateFailure(outcome: DuplicateOutcome): string {
const failed = Array.isArray(outcome.failed) ? outcome.failed : [];
const failedCount = typeof outcome.failedCount === 'number' ? outcome.failedCount : failed.length;
const copiedCount = typeof outcome.copiedCount === 'number' ? outcome.copiedCount : 0;
if (failedCount > 0) {
const shown = failed
.slice(0, DUPLICATE_FAILURE_DETAIL_LIMIT)
.map((f) => `${f.type || 'item'}/${f.name || '?'}: ${f.error || 'copy failed'}`);
const rest = failed.length - shown.length;
const detail = shown.length ? ` ${shown.join('; ')}${rest > 0 ? `; +${rest} more` : ''}` : '';
return `Partial duplicate: ${copiedCount} item(s) copied, ${failedCount} failed.${detail}`;
}
return (
'Nothing was copied: the duplicate is empty (0 items copied, 0 reported as failed). ' +
"The source package may have no active items, or its rows may be outside this session's scope."
);
}

/**
* Duplicate a package into a NEW writable base (ADR-0070 D4 — the Airtable
* "duplicate base" gesture; POST /packages/:id/duplicate). This is how a
* read-only code package becomes a customizable starting point: objects are
* re-namespaced and intra-package references rewritten server-side.
*
* Rejects on a transport/error-envelope failure AND on a false operation
* verdict, which a 200 can carry. Unwrapping before reading that verdict is
* the same order `revertCommit` uses for the sibling commit-revert route in
* `preview/commitHistory.ts`.
*/
export async function duplicatePackage(sourceId: string, targetId: string, targetName?: string): Promise<void> {
const res = await fetch(`/api/v1/packages/${encodeURIComponent(sourceId)}/duplicate`, {
Expand All@@ -67,11 +127,22 @@ export async function duplicatePackage(sourceId: string, targetId: string, targe
headers: { 'Content-Type': 'application/json', Accept: 'application/json' },
body: JSON.stringify({ targetPackageId: targetId, ...(targetName ? { targetName } : {}) }),
});
const payload = (await res.json().catch(() => null)) as
| { success?: boolean; error?: { message?: string } }
| null;
if (!res.ok || payload?.success === false) {
throw new Error(payload?.error?.message || `HTTP ${res.status}`);
const payload = (await res.json().catch(() => null)) as Record<string, unknown> | null;
if (!res.ok) {
// The error envelope IS top-level (`{ success: false, error }`) — no `data`
// to unwrap on this arm.
const message = (payload?.error as { message?: string } | undefined)?.message;
throw new Error(message || `HTTP ${res.status}`);
}
// Unwrap FIRST, then read the operation's flag. The `?? payload` arm mirrors
// the commit-revert helper: it would classify a hypothetical bare (unwrapped)
// answer instead of reading it as success. Against today's single wrapping
// surface only the `data` arm is live.
const inner = ((payload?.data as Record<string, unknown> | undefined) ?? payload ?? undefined) as
| DuplicateOutcome
| undefined;
if (inner?.success === false) {
throw new Error(describeDuplicateFailure(inner));
}
}

Expand Down
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix"); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 33 additions & 0 deletions .changeset/6593-duplicate-package-envelope-unwrap.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
---
'@object-ui/app-shell': patch
---

Studio's "duplicate base" now reports a partial or empty duplicate as a failure instead of
a complete success (objectui#6593).

`duplicatePackage()` read `success` at the **top level** of the response body. That is the
runtime dispatcher's envelope — `deps.success(result)` answers
`{ success: true, data }` — so on every HTTP 200 the flag it read was `true` by
construction. The operation's own verdict lives one level down in `data` and is a real
three-state: the server computes it as `failed.length === 0 && copied.length > 0`.

Two outcomes therefore answered 200 with `envelope.success: true` while the duplicate had
not succeeded, and both were shown to the author as "created", followed by a navigation
into the new base:

- **partial** — some items failed to copy. `failed[]` carries a per-item `error` string
that is the only place the reason is ever stated, and none of it was read.
- **empty** — nothing was copied at all, e.g. an all-env-wide source package under a
session that resolves no active organization (`copiedCount: 0`, `failedCount: 0`).

`duplicatePackage()` now unwraps `data` before reading the operation flag, and rejects with
a message built from what actually happened: the copied/failed counts, plus each
`failed[].error` (the first five by name, then a `+N more` tail). A generic `HTTP nnn`
message is deliberately not sufficient for the partial arm — it is the half an author needs
to act on. The non-2xx arm is unchanged and still surfaces the error envelope's message.

The unwrap-then-read order is the one `revertCommit` already uses for the sibling
commit-revert route in `preview/commitHistory.ts`; this is one consumer converging on that,
not a new convention. The route's underlying contract absence (it publishes no response
schema, so reading the wrong `success` typechecks perfectly) stays upstream in
objectstack#12038.
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,203 @@
// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license.

/**
* `duplicatePackage` must read the OPERATION's verdict, not the ENVELOPE's.
*
* `POST /packages/:id/duplicate` is served only by the runtime dispatcher, and
* that door always wraps: `deps.success(result)` answers
* `{ status: 200, body: { success: true, data } }`. So the top-level `success`
* is `true` by construction on every 200, and reading it classifies every 200
* as a complete success. The operation's own verdict lives one level down in
* `data`, computed server-side as `failed.length === 0 && copied.length > 0`.
*
* That leaves TWO reachable outcomes which answer HTTP 200 while the duplicate
* did not succeed, and this file pins BOTH — a suite that only covered
* `res.ok === false` would re-create the defect, because the transport arm was
* never the broken one:
*
* 1. PARTIAL — `failed.length > 0`. The per-item `error` strings in `failed[]`
* are the only place the reason is ever stated, so a generic `HTTP nnn`
* message is explicitly NOT sufficient here; that is asserted, not implied.
* 2. EMPTY — `copied.length === 0` with nothing named as failed, e.g. an
* all-env-wide source package under a session that resolves no active
* organization. Measured upstream as `{ success: false, copiedCount: 0,
* failedCount: 0 }`.
*
* The pattern being converged on is the sibling commit-revert helper in
* `preview/commitHistory.ts` (`revertCommit`), which already unwraps `data`
* before reading the flag. This is one consumer catching up to it, not a new
* convention.
*/

import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
import { duplicatePackage } from './packages-io';

/** The dispatcher's success envelope: the operation result under `data`. */
function envelope(data: unknown, status = 200) {
return { ok: status >= 200 && status < 300, status, json: async () => ({ success: true, data }) } as unknown as Response;
}

/** The dispatcher's error envelope — top-level, with no `data` to unwrap. */
function errorEnvelope(status: number, message: string) {
return {
ok: false,
status,
json: async () => ({ success: false, error: { message, code: 'PERMISSION_DENIED', status } }),
} as unknown as Response;
}

function stubFetch(response: Response) {
const fetchMock = vi.fn().mockResolvedValue(response);
vi.stubGlobal('fetch', fetchMock);
return fetchMock;
}

beforeEach(() => {
vi.restoreAllMocks();
});

afterEach(() => {
vi.unstubAllGlobals();
});

describe('duplicatePackage — the operation verdict lives under `data`', () => {
it('rejects a PARTIAL duplicate and names the counts and every per-item error', async () => {
stubFetch(
envelope({
success: false,
copiedCount: 7,
failedCount: 2,
targetPackageId: 'com.example.leave-copy',
copied: [],
failed: [
{ type: 'object', name: 'leave_request', error: 'copy failed: duplicate key' },
{ type: 'view', name: 'leave_list', error: 'the source item does not convert' },
],
}),
);

const err = await duplicatePackage('com.example.leave', 'com.example.leave-copy').then(
() => null,
(e: unknown) => e as Error,
);

expect(err).toBeInstanceOf(Error);
// The counts…
expect(err?.message).toContain('7');
expect(err?.message).toContain('2');
// …and the per-item reasons, which exist nowhere else in the response.
expect(err?.message).toContain('object/leave_request');
expect(err?.message).toContain('copy failed: duplicate key');
expect(err?.message).toContain('view/leave_list');
expect(err?.message).toContain('the source item does not convert');
// A generic transport message is NOT sufficient for this arm — it is the
// half the fix exists to deliver.
expect(err?.message).not.toMatch(/HTTP \d/);
});

it('summarizes the tail rather than pasting an unbounded failure list', async () => {
stubFetch(
envelope({
success: false,
copiedCount: 0,
failedCount: 7,
failed: Array.from({ length: 7 }, (_, i) => ({
type: 'object',
name: `obj_${i}`,
error: `copy failed ${i}`,
})),
}),
);

const err = await duplicatePackage('a.b.c', 'a.b.d').catch((e: unknown) => e as Error);

expect(err?.message).toContain('object/obj_0');
expect(err?.message).toContain('object/obj_4');
expect(err?.message).not.toContain('object/obj_5');
expect(err?.message).toContain('+2 more');
});

it('rejects an EMPTY duplicate — 200, nothing copied, nothing named as failed', async () => {
stubFetch(
envelope({
success: false,
copiedCount: 0,
failedCount: 0,
targetPackageId: 'com.example.leave-copy',
copied: [],
failed: [],
}),
);

const err = await duplicatePackage('com.example.leave', 'com.example.leave-copy').then(
() => null,
(e: unknown) => e as Error,
);

expect(err).toBeInstanceOf(Error);
expect(err?.message).toMatch(/nothing was copied/i);
// The empty arm has no `failed[]` to quote, so the message must still say
// something the author can act on instead of falling through to a status.
expect(err?.message).not.toMatch(/HTTP \d/);
});

it('does NOT let the envelope alone stand in for the operation verdict', async () => {
// The exact shape the defect read as success: `success: true` at the top
// level, `success: false` one level down.
const fetchMock = stubFetch(
envelope({ success: false, copiedCount: 0, failedCount: 1, failed: [{ type: 'object', name: 'x', error: 'nope' }] }),
);

await expect(duplicatePackage('a.b.c', 'a.b.d')).rejects.toThrow();
expect(fetchMock).toHaveBeenCalledTimes(1);
});

it('resolves when the operation itself succeeded', async () => {
stubFetch(
envelope({
success: true,
copiedCount: 9,
failedCount: 0,
targetPackageId: 'com.example.leave-copy',
copied: [{ type: 'object', name: 'leave_request' }],
failed: [],
}),
);

await expect(duplicatePackage('com.example.leave', 'com.example.leave-copy')).resolves.toBeUndefined();
});

it('still surfaces the error envelope message on a non-2xx', async () => {
stubFetch(errorEnvelope(403, 'Permission denied: manage_metadata is required'));

await expect(duplicatePackage('a.b.c', 'a.b.d')).rejects.toThrow(
'Permission denied: manage_metadata is required',
);
});

it('falls back to the status when a non-2xx carries no readable body', async () => {
stubFetch({
ok: false,
status: 502,
json: async () => {
throw new SyntaxError('Unexpected token < in JSON at position 0');
},
} as unknown as Response);

await expect(duplicatePackage('a.b.c', 'a.b.d')).rejects.toThrow('HTTP 502');
});

it('posts the target id and name to the duplicate route', async () => {
const fetchMock = stubFetch(envelope({ success: true, copiedCount: 1, failedCount: 0, copied: [{}], failed: [] }));

await duplicatePackage('com.example.leave', 'com.example.leave-copy', 'Leave (copy)');

expect(fetchMock).toHaveBeenCalledWith(
'/api/v1/packages/com.example.leave/duplicate',
expect.objectContaining({
method: 'POST',
body: JSON.stringify({ targetPackageId: 'com.example.leave-copy', targetName: 'Leave (copy)' }),
}),
);
});
});
81 changes: 76 additions & 5 deletions packages/app-shell/src/views/studio-design/packages-io.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -54,11 +54,71 @@ export async function fetchPackages(): Promise<PkgEntry[]> {
return parsePackages(await res.json());
}

/**
* The duplicate operation's OWN result, which travels inside the dispatcher
* envelope's `data` — not at the top level.
*
* `POST /packages/:id/duplicate` is served only by the runtime dispatcher, and
* that door always wraps (`deps.success(result)` → `{ success: true, data }`).
* So a top-level `success` read is reading the ENVELOPE, whose value is `true`
* by construction on a 200. The operation's verdict is one level down, and it
* is a real three-state: the server computes it as
* `failed.length === 0 && copied.length > 0`, which leaves TWO reachable
* outcomes that answer HTTP 200 with `data.success === false`:
*
* 1. PARTIAL — some items failed to copy. `failed[]` carries a per-item
* `error` string and is the ONLY place the reason is ever stated; a
* generic `HTTP nnn` here tells the author nothing.
* 2. EMPTY — nothing was copied at all (e.g. a source package whose rows are
* outside this caller's scope). `failed[]` is empty in this arm — nothing
* is named as having failed — so the counts are the only signal.
*
* Those two are exhaustive for `success === false`: with no failures AND at
* least one copy the server would have said `true`.
*/
interface DuplicateOutcome {
success?: boolean;
copiedCount?: number;
failedCount?: number;
failed?: Array<{ type?: string; name?: string; error?: string }>;
}

/** Per-item failures named in full before the message summarizes the rest. */
const DUPLICATE_FAILURE_DETAIL_LIMIT = 5;

/**
* Turn a false operation verdict into something the Studio author can act on.
* The counts say how far the copy got; `failed[].error` says why each item did
* not make it. Both arms are reachable on a 200 — see {@link DuplicateOutcome}.
*/
function describeDuplicateFailure(outcome: DuplicateOutcome): string {
const failed = Array.isArray(outcome.failed) ? outcome.failed : [];
const failedCount = typeof outcome.failedCount === 'number' ? outcome.failedCount : failed.length;
const copiedCount = typeof outcome.copiedCount === 'number' ? outcome.copiedCount : 0;
if (failedCount > 0) {
const shown = failed
.slice(0, DUPLICATE_FAILURE_DETAIL_LIMIT)
.map((f) => `${f.type || 'item'}/${f.name || '?'}: ${f.error || 'copy failed'}`);
const rest = failed.length - shown.length;
const detail = shown.length ? ` ${shown.join('; ')}${rest > 0 ? `; +${rest} more` : ''}` : '';
return `Partial duplicate: ${copiedCount} item(s) copied, ${failedCount} failed.${detail}`;
}
return (
'Nothing was copied: the duplicate is empty (0 items copied, 0 reported as failed). ' +
"The source package may have no active items, or its rows may be outside this session's scope."
);
}

/**
* Duplicate a package into a NEW writable base (ADR-0070 D4 — the Airtable
* "duplicate base" gesture; POST /packages/:id/duplicate). This is how a
* read-only code package becomes a customizable starting point: objects are
* re-namespaced and intra-package references rewritten server-side.
*
* Rejects on a transport/error-envelope failure AND on a false operation
* verdict, which a 200 can carry. Unwrapping before reading that verdict is
* the same order `revertCommit` uses for the sibling commit-revert route in
* `preview/commitHistory.ts`.
*/
export async function duplicatePackage(sourceId: string, targetId: string, targetName?: string): Promise<void> {
const res = await fetch(`/api/v1/packages/${encodeURIComponent(sourceId)}/duplicate`, {
Expand All@@ -67,11 +127,22 @@ export async function duplicatePackage(sourceId: string, targetId: string, targe
headers: { 'Content-Type': 'application/json', Accept: 'application/json' },
body: JSON.stringify({ targetPackageId: targetId, ...(targetName ? { targetName } : {}) }),
});
const payload = (await res.json().catch(() => null)) as
| { success?: boolean; error?: { message?: string } }
| null;
if (!res.ok || payload?.success === false) {
throw new Error(payload?.error?.message || `HTTP ${res.status}`);
const payload = (await res.json().catch(() => null)) as Record<string, unknown> | null;
if (!res.ok) {
// The error envelope IS top-level (`{ success: false, error }`) — no `data`
// to unwrap on this arm.
const message = (payload?.error as { message?: string } | undefined)?.message;
throw new Error(message || `HTTP ${res.status}`);
}
// Unwrap FIRST, then read the operation's flag. The `?? payload` arm mirrors
// the commit-revert helper: it would classify a hypothetical bare (unwrapped)
// answer instead of reading it as success. Against today's single wrapping
// surface only the `data` arm is live.
const inner = ((payload?.data as Record<string, unknown> | undefined) ?? payload ?? undefined) as
| DuplicateOutcome
| undefined;
if (inner?.success === false) {
throw new Error(describeDuplicateFailure(inner));
}
}

Expand Down
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms"); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 33 additions & 0 deletions .changeset/6593-duplicate-package-envelope-unwrap.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
---
'@object-ui/app-shell': patch
---

Studio's "duplicate base" now reports a partial or empty duplicate as a failure instead of
a complete success (objectui#6593).

`duplicatePackage()` read `success` at the **top level** of the response body. That is the
runtime dispatcher's envelope — `deps.success(result)` answers
`{ success: true, data }` — so on every HTTP 200 the flag it read was `true` by
construction. The operation's own verdict lives one level down in `data` and is a real
three-state: the server computes it as `failed.length === 0 && copied.length > 0`.

Two outcomes therefore answered 200 with `envelope.success: true` while the duplicate had
not succeeded, and both were shown to the author as "created", followed by a navigation
into the new base:

- **partial** — some items failed to copy. `failed[]` carries a per-item `error` string
that is the only place the reason is ever stated, and none of it was read.
- **empty** — nothing was copied at all, e.g. an all-env-wide source package under a
session that resolves no active organization (`copiedCount: 0`, `failedCount: 0`).

`duplicatePackage()` now unwraps `data` before reading the operation flag, and rejects with
a message built from what actually happened: the copied/failed counts, plus each
`failed[].error` (the first five by name, then a `+N more` tail). A generic `HTTP nnn`
message is deliberately not sufficient for the partial arm — it is the half an author needs
to act on. The non-2xx arm is unchanged and still surfaces the error envelope's message.

The unwrap-then-read order is the one `revertCommit` already uses for the sibling
commit-revert route in `preview/commitHistory.ts`; this is one consumer converging on that,
not a new convention. The route's underlying contract absence (it publishes no response
schema, so reading the wrong `success` typechecks perfectly) stays upstream in
objectstack#12038.
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,203 @@
// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license.

/**
* `duplicatePackage` must read the OPERATION's verdict, not the ENVELOPE's.
*
* `POST /packages/:id/duplicate` is served only by the runtime dispatcher, and
* that door always wraps: `deps.success(result)` answers
* `{ status: 200, body: { success: true, data } }`. So the top-level `success`
* is `true` by construction on every 200, and reading it classifies every 200
* as a complete success. The operation's own verdict lives one level down in
* `data`, computed server-side as `failed.length === 0 && copied.length > 0`.
*
* That leaves TWO reachable outcomes which answer HTTP 200 while the duplicate
* did not succeed, and this file pins BOTH — a suite that only covered
* `res.ok === false` would re-create the defect, because the transport arm was
* never the broken one:
*
* 1. PARTIAL — `failed.length > 0`. The per-item `error` strings in `failed[]`
* are the only place the reason is ever stated, so a generic `HTTP nnn`
* message is explicitly NOT sufficient here; that is asserted, not implied.
* 2. EMPTY — `copied.length === 0` with nothing named as failed, e.g. an
* all-env-wide source package under a session that resolves no active
* organization. Measured upstream as `{ success: false, copiedCount: 0,
* failedCount: 0 }`.
*
* The pattern being converged on is the sibling commit-revert helper in
* `preview/commitHistory.ts` (`revertCommit`), which already unwraps `data`
* before reading the flag. This is one consumer catching up to it, not a new
* convention.
*/

import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
import { duplicatePackage } from './packages-io';

/** The dispatcher's success envelope: the operation result under `data`. */
function envelope(data: unknown, status = 200) {
return { ok: status >= 200 && status < 300, status, json: async () => ({ success: true, data }) } as unknown as Response;
}

/** The dispatcher's error envelope — top-level, with no `data` to unwrap. */
function errorEnvelope(status: number, message: string) {
return {
ok: false,
status,
json: async () => ({ success: false, error: { message, code: 'PERMISSION_DENIED', status } }),
} as unknown as Response;
}

function stubFetch(response: Response) {
const fetchMock = vi.fn().mockResolvedValue(response);
vi.stubGlobal('fetch', fetchMock);
return fetchMock;
}

beforeEach(() => {
vi.restoreAllMocks();
});

afterEach(() => {
vi.unstubAllGlobals();
});

describe('duplicatePackage — the operation verdict lives under `data`', () => {
it('rejects a PARTIAL duplicate and names the counts and every per-item error', async () => {
stubFetch(
envelope({
success: false,
copiedCount: 7,
failedCount: 2,
targetPackageId: 'com.example.leave-copy',
copied: [],
failed: [
{ type: 'object', name: 'leave_request', error: 'copy failed: duplicate key' },
{ type: 'view', name: 'leave_list', error: 'the source item does not convert' },
],
}),
);

const err = await duplicatePackage('com.example.leave', 'com.example.leave-copy').then(
() => null,
(e: unknown) => e as Error,
);

expect(err).toBeInstanceOf(Error);
// The counts…
expect(err?.message).toContain('7');
expect(err?.message).toContain('2');
// …and the per-item reasons, which exist nowhere else in the response.
expect(err?.message).toContain('object/leave_request');
expect(err?.message).toContain('copy failed: duplicate key');
expect(err?.message).toContain('view/leave_list');
expect(err?.message).toContain('the source item does not convert');
// A generic transport message is NOT sufficient for this arm — it is the
// half the fix exists to deliver.
expect(err?.message).not.toMatch(/HTTP \d/);
});

it('summarizes the tail rather than pasting an unbounded failure list', async () => {
stubFetch(
envelope({
success: false,
copiedCount: 0,
failedCount: 7,
failed: Array.from({ length: 7 }, (_, i) => ({
type: 'object',
name: `obj_${i}`,
error: `copy failed ${i}`,
})),
}),
);

const err = await duplicatePackage('a.b.c', 'a.b.d').catch((e: unknown) => e as Error);

expect(err?.message).toContain('object/obj_0');
expect(err?.message).toContain('object/obj_4');
expect(err?.message).not.toContain('object/obj_5');
expect(err?.message).toContain('+2 more');
});

it('rejects an EMPTY duplicate — 200, nothing copied, nothing named as failed', async () => {
stubFetch(
envelope({
success: false,
copiedCount: 0,
failedCount: 0,
targetPackageId: 'com.example.leave-copy',
copied: [],
failed: [],
}),
);

const err = await duplicatePackage('com.example.leave', 'com.example.leave-copy').then(
() => null,
(e: unknown) => e as Error,
);

expect(err).toBeInstanceOf(Error);
expect(err?.message).toMatch(/nothing was copied/i);
// The empty arm has no `failed[]` to quote, so the message must still say
// something the author can act on instead of falling through to a status.
expect(err?.message).not.toMatch(/HTTP \d/);
});

it('does NOT let the envelope alone stand in for the operation verdict', async () => {
// The exact shape the defect read as success: `success: true` at the top
// level, `success: false` one level down.
const fetchMock = stubFetch(
envelope({ success: false, copiedCount: 0, failedCount: 1, failed: [{ type: 'object', name: 'x', error: 'nope' }] }),
);

await expect(duplicatePackage('a.b.c', 'a.b.d')).rejects.toThrow();
expect(fetchMock).toHaveBeenCalledTimes(1);
});

it('resolves when the operation itself succeeded', async () => {
stubFetch(
envelope({
success: true,
copiedCount: 9,
failedCount: 0,
targetPackageId: 'com.example.leave-copy',
copied: [{ type: 'object', name: 'leave_request' }],
failed: [],
}),
);

await expect(duplicatePackage('com.example.leave', 'com.example.leave-copy')).resolves.toBeUndefined();
});

it('still surfaces the error envelope message on a non-2xx', async () => {
stubFetch(errorEnvelope(403, 'Permission denied: manage_metadata is required'));

await expect(duplicatePackage('a.b.c', 'a.b.d')).rejects.toThrow(
'Permission denied: manage_metadata is required',
);
});

it('falls back to the status when a non-2xx carries no readable body', async () => {
stubFetch({
ok: false,
status: 502,
json: async () => {
throw new SyntaxError('Unexpected token < in JSON at position 0');
},
} as unknown as Response);

await expect(duplicatePackage('a.b.c', 'a.b.d')).rejects.toThrow('HTTP 502');
});

it('posts the target id and name to the duplicate route', async () => {
const fetchMock = stubFetch(envelope({ success: true, copiedCount: 1, failedCount: 0, copied: [{}], failed: [] }));

await duplicatePackage('com.example.leave', 'com.example.leave-copy', 'Leave (copy)');

expect(fetchMock).toHaveBeenCalledWith(
'/api/v1/packages/com.example.leave/duplicate',
expect.objectContaining({
method: 'POST',
body: JSON.stringify({ targetPackageId: 'com.example.leave-copy', targetName: 'Leave (copy)' }),
}),
);
});
});
81 changes: 76 additions & 5 deletions packages/app-shell/src/views/studio-design/packages-io.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -54,11 +54,71 @@ export async function fetchPackages(): Promise<PkgEntry[]> {
return parsePackages(await res.json());
}

/**
* The duplicate operation's OWN result, which travels inside the dispatcher
* envelope's `data` — not at the top level.
*
* `POST /packages/:id/duplicate` is served only by the runtime dispatcher, and
* that door always wraps (`deps.success(result)` → `{ success: true, data }`).
* So a top-level `success` read is reading the ENVELOPE, whose value is `true`
* by construction on a 200. The operation's verdict is one level down, and it
* is a real three-state: the server computes it as
* `failed.length === 0 && copied.length > 0`, which leaves TWO reachable
* outcomes that answer HTTP 200 with `data.success === false`:
*
* 1. PARTIAL — some items failed to copy. `failed[]` carries a per-item
* `error` string and is the ONLY place the reason is ever stated; a
* generic `HTTP nnn` here tells the author nothing.
* 2. EMPTY — nothing was copied at all (e.g. a source package whose rows are
* outside this caller's scope). `failed[]` is empty in this arm — nothing
* is named as having failed — so the counts are the only signal.
*
* Those two are exhaustive for `success === false`: with no failures AND at
* least one copy the server would have said `true`.
*/
interface DuplicateOutcome {
success?: boolean;
copiedCount?: number;
failedCount?: number;
failed?: Array<{ type?: string; name?: string; error?: string }>;
}

/** Per-item failures named in full before the message summarizes the rest. */
const DUPLICATE_FAILURE_DETAIL_LIMIT = 5;

/**
* Turn a false operation verdict into something the Studio author can act on.
* The counts say how far the copy got; `failed[].error` says why each item did
* not make it. Both arms are reachable on a 200 — see {@link DuplicateOutcome}.
*/
function describeDuplicateFailure(outcome: DuplicateOutcome): string {
const failed = Array.isArray(outcome.failed) ? outcome.failed : [];
const failedCount = typeof outcome.failedCount === 'number' ? outcome.failedCount : failed.length;
const copiedCount = typeof outcome.copiedCount === 'number' ? outcome.copiedCount : 0;
if (failedCount > 0) {
const shown = failed
.slice(0, DUPLICATE_FAILURE_DETAIL_LIMIT)
.map((f) => `${f.type || 'item'}/${f.name || '?'}: ${f.error || 'copy failed'}`);
const rest = failed.length - shown.length;
const detail = shown.length ? ` ${shown.join('; ')}${rest > 0 ? `; +${rest} more` : ''}` : '';
return `Partial duplicate: ${copiedCount} item(s) copied, ${failedCount} failed.${detail}`;
}
return (
'Nothing was copied: the duplicate is empty (0 items copied, 0 reported as failed). ' +
"The source package may have no active items, or its rows may be outside this session's scope."
);
}

/**
* Duplicate a package into a NEW writable base (ADR-0070 D4 — the Airtable
* "duplicate base" gesture; POST /packages/:id/duplicate). This is how a
* read-only code package becomes a customizable starting point: objects are
* re-namespaced and intra-package references rewritten server-side.
*
* Rejects on a transport/error-envelope failure AND on a false operation
* verdict, which a 200 can carry. Unwrapping before reading that verdict is
* the same order `revertCommit` uses for the sibling commit-revert route in
* `preview/commitHistory.ts`.
*/
export async function duplicatePackage(sourceId: string, targetId: string, targetName?: string): Promise<void> {
const res = await fetch(`/api/v1/packages/${encodeURIComponent(sourceId)}/duplicate`, {
Expand All@@ -67,11 +127,22 @@ export async function duplicatePackage(sourceId: string, targetId: string, targe
headers: { 'Content-Type': 'application/json', Accept: 'application/json' },
body: JSON.stringify({ targetPackageId: targetId, ...(targetName ? { targetName } : {}) }),
});
const payload = (await res.json().catch(() => null)) as
| { success?: boolean; error?: { message?: string } }
| null;
if (!res.ok || payload?.success === false) {
throw new Error(payload?.error?.message || `HTTP ${res.status}`);
const payload = (await res.json().catch(() => null)) as Record<string, unknown> | null;
if (!res.ok) {
// The error envelope IS top-level (`{ success: false, error }`) — no `data`
// to unwrap on this arm.
const message = (payload?.error as { message?: string } | undefined)?.message;
throw new Error(message || `HTTP ${res.status}`);
}
// Unwrap FIRST, then read the operation's flag. The `?? payload` arm mirrors
// the commit-revert helper: it would classify a hypothetical bare (unwrapped)
// answer instead of reading it as success. Against today's single wrapping
// surface only the `data` arm is live.
const inner = ((payload?.data as Record<string, unknown> | undefined) ?? payload ?? undefined) as
| DuplicateOutcome
| undefined;
if (inner?.success === false) {
throw new Error(describeDuplicateFailure(inner));
}
}

Expand Down
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links"); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + '
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 33 additions & 0 deletions .changeset/6593-duplicate-package-envelope-unwrap.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
---
'@object-ui/app-shell': patch
---

Studio's "duplicate base" now reports a partial or empty duplicate as a failure instead of
a complete success (objectui#6593).

`duplicatePackage()` read `success` at the **top level** of the response body. That is the
runtime dispatcher's envelope — `deps.success(result)` answers
`{ success: true, data }` — so on every HTTP 200 the flag it read was `true` by
construction. The operation's own verdict lives one level down in `data` and is a real
three-state: the server computes it as `failed.length === 0 && copied.length > 0`.

Two outcomes therefore answered 200 with `envelope.success: true` while the duplicate had
not succeeded, and both were shown to the author as "created", followed by a navigation
into the new base:

- **partial** — some items failed to copy. `failed[]` carries a per-item `error` string
that is the only place the reason is ever stated, and none of it was read.
- **empty** — nothing was copied at all, e.g. an all-env-wide source package under a
session that resolves no active organization (`copiedCount: 0`, `failedCount: 0`).

`duplicatePackage()` now unwraps `data` before reading the operation flag, and rejects with
a message built from what actually happened: the copied/failed counts, plus each
`failed[].error` (the first five by name, then a `+N more` tail). A generic `HTTP nnn`
message is deliberately not sufficient for the partial arm — it is the half an author needs
to act on. The non-2xx arm is unchanged and still surfaces the error envelope's message.

The unwrap-then-read order is the one `revertCommit` already uses for the sibling
commit-revert route in `preview/commitHistory.ts`; this is one consumer converging on that,
not a new convention. The route's underlying contract absence (it publishes no response
schema, so reading the wrong `success` typechecks perfectly) stays upstream in
objectstack#12038.
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,203 @@
// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license.

/**
* `duplicatePackage` must read the OPERATION's verdict, not the ENVELOPE's.
*
* `POST /packages/:id/duplicate` is served only by the runtime dispatcher, and
* that door always wraps: `deps.success(result)` answers
* `{ status: 200, body: { success: true, data } }`. So the top-level `success`
* is `true` by construction on every 200, and reading it classifies every 200
* as a complete success. The operation's own verdict lives one level down in
* `data`, computed server-side as `failed.length === 0 && copied.length > 0`.
*
* That leaves TWO reachable outcomes which answer HTTP 200 while the duplicate
* did not succeed, and this file pins BOTH — a suite that only covered
* `res.ok === false` would re-create the defect, because the transport arm was
* never the broken one:
*
* 1. PARTIAL — `failed.length > 0`. The per-item `error` strings in `failed[]`
* are the only place the reason is ever stated, so a generic `HTTP nnn`
* message is explicitly NOT sufficient here; that is asserted, not implied.
* 2. EMPTY — `copied.length === 0` with nothing named as failed, e.g. an
* all-env-wide source package under a session that resolves no active
* organization. Measured upstream as `{ success: false, copiedCount: 0,
* failedCount: 0 }`.
*
* The pattern being converged on is the sibling commit-revert helper in
* `preview/commitHistory.ts` (`revertCommit`), which already unwraps `data`
* before reading the flag. This is one consumer catching up to it, not a new
* convention.
*/

import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
import { duplicatePackage } from './packages-io';

/** The dispatcher's success envelope: the operation result under `data`. */
function envelope(data: unknown, status = 200) {
return { ok: status >= 200 && status < 300, status, json: async () => ({ success: true, data }) } as unknown as Response;
}

/** The dispatcher's error envelope — top-level, with no `data` to unwrap. */
function errorEnvelope(status: number, message: string) {
return {
ok: false,
status,
json: async () => ({ success: false, error: { message, code: 'PERMISSION_DENIED', status } }),
} as unknown as Response;
}

function stubFetch(response: Response) {
const fetchMock = vi.fn().mockResolvedValue(response);
vi.stubGlobal('fetch', fetchMock);
return fetchMock;
}

beforeEach(() => {
vi.restoreAllMocks();
});

afterEach(() => {
vi.unstubAllGlobals();
});

describe('duplicatePackage — the operation verdict lives under `data`', () => {
it('rejects a PARTIAL duplicate and names the counts and every per-item error', async () => {
stubFetch(
envelope({
success: false,
copiedCount: 7,
failedCount: 2,
targetPackageId: 'com.example.leave-copy',
copied: [],
failed: [
{ type: 'object', name: 'leave_request', error: 'copy failed: duplicate key' },
{ type: 'view', name: 'leave_list', error: 'the source item does not convert' },
],
}),
);

const err = await duplicatePackage('com.example.leave', 'com.example.leave-copy').then(
() => null,
(e: unknown) => e as Error,
);

expect(err).toBeInstanceOf(Error);
// The counts…
expect(err?.message).toContain('7');
expect(err?.message).toContain('2');
// …and the per-item reasons, which exist nowhere else in the response.
expect(err?.message).toContain('object/leave_request');
expect(err?.message).toContain('copy failed: duplicate key');
expect(err?.message).toContain('view/leave_list');
expect(err?.message).toContain('the source item does not convert');
// A generic transport message is NOT sufficient for this arm — it is the
// half the fix exists to deliver.
expect(err?.message).not.toMatch(/HTTP \d/);
});

it('summarizes the tail rather than pasting an unbounded failure list', async () => {
stubFetch(
envelope({
success: false,
copiedCount: 0,
failedCount: 7,
failed: Array.from({ length: 7 }, (_, i) => ({
type: 'object',
name: `obj_${i}`,
error: `copy failed ${i}`,
})),
}),
);

const err = await duplicatePackage('a.b.c', 'a.b.d').catch((e: unknown) => e as Error);

expect(err?.message).toContain('object/obj_0');
expect(err?.message).toContain('object/obj_4');
expect(err?.message).not.toContain('object/obj_5');
expect(err?.message).toContain('+2 more');
});

it('rejects an EMPTY duplicate — 200, nothing copied, nothing named as failed', async () => {
stubFetch(
envelope({
success: false,
copiedCount: 0,
failedCount: 0,
targetPackageId: 'com.example.leave-copy',
copied: [],
failed: [],
}),
);

const err = await duplicatePackage('com.example.leave', 'com.example.leave-copy').then(
() => null,
(e: unknown) => e as Error,
);

expect(err).toBeInstanceOf(Error);
expect(err?.message).toMatch(/nothing was copied/i);
// The empty arm has no `failed[]` to quote, so the message must still say
// something the author can act on instead of falling through to a status.
expect(err?.message).not.toMatch(/HTTP \d/);
});

it('does NOT let the envelope alone stand in for the operation verdict', async () => {
// The exact shape the defect read as success: `success: true` at the top
// level, `success: false` one level down.
const fetchMock = stubFetch(
envelope({ success: false, copiedCount: 0, failedCount: 1, failed: [{ type: 'object', name: 'x', error: 'nope' }] }),
);

await expect(duplicatePackage('a.b.c', 'a.b.d')).rejects.toThrow();
expect(fetchMock).toHaveBeenCalledTimes(1);
});

it('resolves when the operation itself succeeded', async () => {
stubFetch(
envelope({
success: true,
copiedCount: 9,
failedCount: 0,
targetPackageId: 'com.example.leave-copy',
copied: [{ type: 'object', name: 'leave_request' }],
failed: [],
}),
);

await expect(duplicatePackage('com.example.leave', 'com.example.leave-copy')).resolves.toBeUndefined();
});

it('still surfaces the error envelope message on a non-2xx', async () => {
stubFetch(errorEnvelope(403, 'Permission denied: manage_metadata is required'));

await expect(duplicatePackage('a.b.c', 'a.b.d')).rejects.toThrow(
'Permission denied: manage_metadata is required',
);
});

it('falls back to the status when a non-2xx carries no readable body', async () => {
stubFetch({
ok: false,
status: 502,
json: async () => {
throw new SyntaxError('Unexpected token < in JSON at position 0');
},
} as unknown as Response);

await expect(duplicatePackage('a.b.c', 'a.b.d')).rejects.toThrow('HTTP 502');
});

it('posts the target id and name to the duplicate route', async () => {
const fetchMock = stubFetch(envelope({ success: true, copiedCount: 1, failedCount: 0, copied: [{}], failed: [] }));

await duplicatePackage('com.example.leave', 'com.example.leave-copy', 'Leave (copy)');

expect(fetchMock).toHaveBeenCalledWith(
'/api/v1/packages/com.example.leave/duplicate',
expect.objectContaining({
method: 'POST',
body: JSON.stringify({ targetPackageId: 'com.example.leave-copy', targetName: 'Leave (copy)' }),
}),
);
});
});
81 changes: 76 additions & 5 deletions packages/app-shell/src/views/studio-design/packages-io.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -54,11 +54,71 @@ export async function fetchPackages(): Promise<PkgEntry[]> {
return parsePackages(await res.json());
}

/**
* The duplicate operation's OWN result, which travels inside the dispatcher
* envelope's `data` — not at the top level.
*
* `POST /packages/:id/duplicate` is served only by the runtime dispatcher, and
* that door always wraps (`deps.success(result)` → `{ success: true, data }`).
* So a top-level `success` read is reading the ENVELOPE, whose value is `true`
* by construction on a 200. The operation's verdict is one level down, and it
* is a real three-state: the server computes it as
* `failed.length === 0 && copied.length > 0`, which leaves TWO reachable
* outcomes that answer HTTP 200 with `data.success === false`:
*
* 1. PARTIAL — some items failed to copy. `failed[]` carries a per-item
* `error` string and is the ONLY place the reason is ever stated; a
* generic `HTTP nnn` here tells the author nothing.
* 2. EMPTY — nothing was copied at all (e.g. a source package whose rows are
* outside this caller's scope). `failed[]` is empty in this arm — nothing
* is named as having failed — so the counts are the only signal.
*
* Those two are exhaustive for `success === false`: with no failures AND at
* least one copy the server would have said `true`.
*/
interface DuplicateOutcome {
success?: boolean;
copiedCount?: number;
failedCount?: number;
failed?: Array<{ type?: string; name?: string; error?: string }>;
}

/** Per-item failures named in full before the message summarizes the rest. */
const DUPLICATE_FAILURE_DETAIL_LIMIT = 5;

/**
* Turn a false operation verdict into something the Studio author can act on.
* The counts say how far the copy got; `failed[].error` says why each item did
* not make it. Both arms are reachable on a 200 — see {@link DuplicateOutcome}.
*/
function describeDuplicateFailure(outcome: DuplicateOutcome): string {
const failed = Array.isArray(outcome.failed) ? outcome.failed : [];
const failedCount = typeof outcome.failedCount === 'number' ? outcome.failedCount : failed.length;
const copiedCount = typeof outcome.copiedCount === 'number' ? outcome.copiedCount : 0;
if (failedCount > 0) {
const shown = failed
.slice(0, DUPLICATE_FAILURE_DETAIL_LIMIT)
.map((f) => `${f.type || 'item'}/${f.name || '?'}: ${f.error || 'copy failed'}`);
const rest = failed.length - shown.length;
const detail = shown.length ? ` ${shown.join('; ')}${rest > 0 ? `; +${rest} more` : ''}` : '';
return `Partial duplicate: ${copiedCount} item(s) copied, ${failedCount} failed.${detail}`;
}
return (
'Nothing was copied: the duplicate is empty (0 items copied, 0 reported as failed). ' +
"The source package may have no active items, or its rows may be outside this session's scope."
);
}

/**
* Duplicate a package into a NEW writable base (ADR-0070 D4 — the Airtable
* "duplicate base" gesture; POST /packages/:id/duplicate). This is how a
* read-only code package becomes a customizable starting point: objects are
* re-namespaced and intra-package references rewritten server-side.
*
* Rejects on a transport/error-envelope failure AND on a false operation
* verdict, which a 200 can carry. Unwrapping before reading that verdict is
* the same order `revertCommit` uses for the sibling commit-revert route in
* `preview/commitHistory.ts`.
*/
export async function duplicatePackage(sourceId: string, targetId: string, targetName?: string): Promise<void> {
const res = await fetch(`/api/v1/packages/${encodeURIComponent(sourceId)}/duplicate`, {
Expand All@@ -67,11 +127,22 @@ export async function duplicatePackage(sourceId: string, targetId: string, targe
headers: { 'Content-Type': 'application/json', Accept: 'application/json' },
body: JSON.stringify({ targetPackageId: targetId, ...(targetName ? { targetName } : {}) }),
});
const payload = (await res.json().catch(() => null)) as
| { success?: boolean; error?: { message?: string } }
| null;
if (!res.ok || payload?.success === false) {
throw new Error(payload?.error?.message || `HTTP ${res.status}`);
const payload = (await res.json().catch(() => null)) as Record<string, unknown> | null;
if (!res.ok) {
// The error envelope IS top-level (`{ success: false, error }`) — no `data`
// to unwrap on this arm.
const message = (payload?.error as { message?: string } | undefined)?.message;
throw new Error(message || `HTTP ${res.status}`);
}
// Unwrap FIRST, then read the operation's flag. The `?? payload` arm mirrors
// the commit-revert helper: it would classify a hypothetical bare (unwrapped)
// answer instead of reading it as success. Against today's single wrapping
// surface only the `data` arm is live.
const inner = ((payload?.data as Record<string, unknown> | undefined) ?? payload ?? undefined) as
| DuplicateOutcome
| undefined;
if (inner?.success === false) {
throw new Error(describeDuplicateFailure(inner));
}
}

Expand Down
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default"); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 33 additions & 0 deletions .changeset/6593-duplicate-package-envelope-unwrap.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
---
'@object-ui/app-shell': patch
---

Studio's "duplicate base" now reports a partial or empty duplicate as a failure instead of
a complete success (objectui#6593).

`duplicatePackage()` read `success` at the **top level** of the response body. That is the
runtime dispatcher's envelope — `deps.success(result)` answers
`{ success: true, data }` — so on every HTTP 200 the flag it read was `true` by
construction. The operation's own verdict lives one level down in `data` and is a real
three-state: the server computes it as `failed.length === 0 && copied.length > 0`.

Two outcomes therefore answered 200 with `envelope.success: true` while the duplicate had
not succeeded, and both were shown to the author as "created", followed by a navigation
into the new base:

- **partial** — some items failed to copy. `failed[]` carries a per-item `error` string
that is the only place the reason is ever stated, and none of it was read.
- **empty** — nothing was copied at all, e.g. an all-env-wide source package under a
session that resolves no active organization (`copiedCount: 0`, `failedCount: 0`).

`duplicatePackage()` now unwraps `data` before reading the operation flag, and rejects with
a message built from what actually happened: the copied/failed counts, plus each
`failed[].error` (the first five by name, then a `+N more` tail). A generic `HTTP nnn`
message is deliberately not sufficient for the partial arm — it is the half an author needs
to act on. The non-2xx arm is unchanged and still surfaces the error envelope's message.

The unwrap-then-read order is the one `revertCommit` already uses for the sibling
commit-revert route in `preview/commitHistory.ts`; this is one consumer converging on that,
not a new convention. The route's underlying contract absence (it publishes no response
schema, so reading the wrong `success` typechecks perfectly) stays upstream in
objectstack#12038.
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,203 @@
// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license.

/**
* `duplicatePackage` must read the OPERATION's verdict, not the ENVELOPE's.
*
* `POST /packages/:id/duplicate` is served only by the runtime dispatcher, and
* that door always wraps: `deps.success(result)` answers
* `{ status: 200, body: { success: true, data } }`. So the top-level `success`
* is `true` by construction on every 200, and reading it classifies every 200
* as a complete success. The operation's own verdict lives one level down in
* `data`, computed server-side as `failed.length === 0 && copied.length > 0`.
*
* That leaves TWO reachable outcomes which answer HTTP 200 while the duplicate
* did not succeed, and this file pins BOTH — a suite that only covered
* `res.ok === false` would re-create the defect, because the transport arm was
* never the broken one:
*
* 1. PARTIAL — `failed.length > 0`. The per-item `error` strings in `failed[]`
* are the only place the reason is ever stated, so a generic `HTTP nnn`
* message is explicitly NOT sufficient here; that is asserted, not implied.
* 2. EMPTY — `copied.length === 0` with nothing named as failed, e.g. an
* all-env-wide source package under a session that resolves no active
* organization. Measured upstream as `{ success: false, copiedCount: 0,
* failedCount: 0 }`.
*
* The pattern being converged on is the sibling commit-revert helper in
* `preview/commitHistory.ts` (`revertCommit`), which already unwraps `data`
* before reading the flag. This is one consumer catching up to it, not a new
* convention.
*/

import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
import { duplicatePackage } from './packages-io';

/** The dispatcher's success envelope: the operation result under `data`. */
function envelope(data: unknown, status = 200) {
return { ok: status >= 200 && status < 300, status, json: async () => ({ success: true, data }) } as unknown as Response;
}

/** The dispatcher's error envelope — top-level, with no `data` to unwrap. */
function errorEnvelope(status: number, message: string) {
return {
ok: false,
status,
json: async () => ({ success: false, error: { message, code: 'PERMISSION_DENIED', status } }),
} as unknown as Response;
}

function stubFetch(response: Response) {
const fetchMock = vi.fn().mockResolvedValue(response);
vi.stubGlobal('fetch', fetchMock);
return fetchMock;
}

beforeEach(() => {
vi.restoreAllMocks();
});

afterEach(() => {
vi.unstubAllGlobals();
});

describe('duplicatePackage — the operation verdict lives under `data`', () => {
it('rejects a PARTIAL duplicate and names the counts and every per-item error', async () => {
stubFetch(
envelope({
success: false,
copiedCount: 7,
failedCount: 2,
targetPackageId: 'com.example.leave-copy',
copied: [],
failed: [
{ type: 'object', name: 'leave_request', error: 'copy failed: duplicate key' },
{ type: 'view', name: 'leave_list', error: 'the source item does not convert' },
],
}),
);

const err = await duplicatePackage('com.example.leave', 'com.example.leave-copy').then(
() => null,
(e: unknown) => e as Error,
);

expect(err).toBeInstanceOf(Error);
// The counts…
expect(err?.message).toContain('7');
expect(err?.message).toContain('2');
// …and the per-item reasons, which exist nowhere else in the response.
expect(err?.message).toContain('object/leave_request');
expect(err?.message).toContain('copy failed: duplicate key');
expect(err?.message).toContain('view/leave_list');
expect(err?.message).toContain('the source item does not convert');
// A generic transport message is NOT sufficient for this arm — it is the
// half the fix exists to deliver.
expect(err?.message).not.toMatch(/HTTP \d/);
});

it('summarizes the tail rather than pasting an unbounded failure list', async () => {
stubFetch(
envelope({
success: false,
copiedCount: 0,
failedCount: 7,
failed: Array.from({ length: 7 }, (_, i) => ({
type: 'object',
name: `obj_${i}`,
error: `copy failed ${i}`,
})),
}),
);

const err = await duplicatePackage('a.b.c', 'a.b.d').catch((e: unknown) => e as Error);

expect(err?.message).toContain('object/obj_0');
expect(err?.message).toContain('object/obj_4');
expect(err?.message).not.toContain('object/obj_5');
expect(err?.message).toContain('+2 more');
});

it('rejects an EMPTY duplicate — 200, nothing copied, nothing named as failed', async () => {
stubFetch(
envelope({
success: false,
copiedCount: 0,
failedCount: 0,
targetPackageId: 'com.example.leave-copy',
copied: [],
failed: [],
}),
);

const err = await duplicatePackage('com.example.leave', 'com.example.leave-copy').then(
() => null,
(e: unknown) => e as Error,
);

expect(err).toBeInstanceOf(Error);
expect(err?.message).toMatch(/nothing was copied/i);
// The empty arm has no `failed[]` to quote, so the message must still say
// something the author can act on instead of falling through to a status.
expect(err?.message).not.toMatch(/HTTP \d/);
});

it('does NOT let the envelope alone stand in for the operation verdict', async () => {
// The exact shape the defect read as success: `success: true` at the top
// level, `success: false` one level down.
const fetchMock = stubFetch(
envelope({ success: false, copiedCount: 0, failedCount: 1, failed: [{ type: 'object', name: 'x', error: 'nope' }] }),
);

await expect(duplicatePackage('a.b.c', 'a.b.d')).rejects.toThrow();
expect(fetchMock).toHaveBeenCalledTimes(1);
});

it('resolves when the operation itself succeeded', async () => {
stubFetch(
envelope({
success: true,
copiedCount: 9,
failedCount: 0,
targetPackageId: 'com.example.leave-copy',
copied: [{ type: 'object', name: 'leave_request' }],
failed: [],
}),
);

await expect(duplicatePackage('com.example.leave', 'com.example.leave-copy')).resolves.toBeUndefined();
});

it('still surfaces the error envelope message on a non-2xx', async () => {
stubFetch(errorEnvelope(403, 'Permission denied: manage_metadata is required'));

await expect(duplicatePackage('a.b.c', 'a.b.d')).rejects.toThrow(
'Permission denied: manage_metadata is required',
);
});

it('falls back to the status when a non-2xx carries no readable body', async () => {
stubFetch({
ok: false,
status: 502,
json: async () => {
throw new SyntaxError('Unexpected token < in JSON at position 0');
},
} as unknown as Response);

await expect(duplicatePackage('a.b.c', 'a.b.d')).rejects.toThrow('HTTP 502');
});

it('posts the target id and name to the duplicate route', async () => {
const fetchMock = stubFetch(envelope({ success: true, copiedCount: 1, failedCount: 0, copied: [{}], failed: [] }));

await duplicatePackage('com.example.leave', 'com.example.leave-copy', 'Leave (copy)');

expect(fetchMock).toHaveBeenCalledWith(
'/api/v1/packages/com.example.leave/duplicate',
expect.objectContaining({
method: 'POST',
body: JSON.stringify({ targetPackageId: 'com.example.leave-copy', targetName: 'Leave (copy)' }),
}),
);
});
});
81 changes: 76 additions & 5 deletions packages/app-shell/src/views/studio-design/packages-io.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -54,11 +54,71 @@ export async function fetchPackages(): Promise<PkgEntry[]> {
return parsePackages(await res.json());
}

/**
* The duplicate operation's OWN result, which travels inside the dispatcher
* envelope's `data` — not at the top level.
*
* `POST /packages/:id/duplicate` is served only by the runtime dispatcher, and
* that door always wraps (`deps.success(result)` → `{ success: true, data }`).
* So a top-level `success` read is reading the ENVELOPE, whose value is `true`
* by construction on a 200. The operation's verdict is one level down, and it
* is a real three-state: the server computes it as
* `failed.length === 0 && copied.length > 0`, which leaves TWO reachable
* outcomes that answer HTTP 200 with `data.success === false`:
*
* 1. PARTIAL — some items failed to copy. `failed[]` carries a per-item
* `error` string and is the ONLY place the reason is ever stated; a
* generic `HTTP nnn` here tells the author nothing.
* 2. EMPTY — nothing was copied at all (e.g. a source package whose rows are
* outside this caller's scope). `failed[]` is empty in this arm — nothing
* is named as having failed — so the counts are the only signal.
*
* Those two are exhaustive for `success === false`: with no failures AND at
* least one copy the server would have said `true`.
*/
interface DuplicateOutcome {
success?: boolean;
copiedCount?: number;
failedCount?: number;
failed?: Array<{ type?: string; name?: string; error?: string }>;
}

/** Per-item failures named in full before the message summarizes the rest. */
const DUPLICATE_FAILURE_DETAIL_LIMIT = 5;

/**
* Turn a false operation verdict into something the Studio author can act on.
* The counts say how far the copy got; `failed[].error` says why each item did
* not make it. Both arms are reachable on a 200 — see {@link DuplicateOutcome}.
*/
function describeDuplicateFailure(outcome: DuplicateOutcome): string {
const failed = Array.isArray(outcome.failed) ? outcome.failed : [];
const failedCount = typeof outcome.failedCount === 'number' ? outcome.failedCount : failed.length;
const copiedCount = typeof outcome.copiedCount === 'number' ? outcome.copiedCount : 0;
if (failedCount > 0) {
const shown = failed
.slice(0, DUPLICATE_FAILURE_DETAIL_LIMIT)
.map((f) => `${f.type || 'item'}/${f.name || '?'}: ${f.error || 'copy failed'}`);
const rest = failed.length - shown.length;
const detail = shown.length ? ` ${shown.join('; ')}${rest > 0 ? `; +${rest} more` : ''}` : '';
return `Partial duplicate: ${copiedCount} item(s) copied, ${failedCount} failed.${detail}`;
}
return (
'Nothing was copied: the duplicate is empty (0 items copied, 0 reported as failed). ' +
"The source package may have no active items, or its rows may be outside this session's scope."
);
}

/**
* Duplicate a package into a NEW writable base (ADR-0070 D4 — the Airtable
* "duplicate base" gesture; POST /packages/:id/duplicate). This is how a
* read-only code package becomes a customizable starting point: objects are
* re-namespaced and intra-package references rewritten server-side.
*
* Rejects on a transport/error-envelope failure AND on a false operation
* verdict, which a 200 can carry. Unwrapping before reading that verdict is
* the same order `revertCommit` uses for the sibling commit-revert route in
* `preview/commitHistory.ts`.
*/
export async function duplicatePackage(sourceId: string, targetId: string, targetName?: string): Promise<void> {
const res = await fetch(`/api/v1/packages/${encodeURIComponent(sourceId)}/duplicate`, {
Expand All@@ -67,11 +127,22 @@ export async function duplicatePackage(sourceId: string, targetId: string, targe
headers: { 'Content-Type': 'application/json', Accept: 'application/json' },
body: JSON.stringify({ targetPackageId: targetId, ...(targetName ? { targetName } : {}) }),
});
const payload = (await res.json().catch(() => null)) as
| { success?: boolean; error?: { message?: string } }
| null;
if (!res.ok || payload?.success === false) {
throw new Error(payload?.error?.message || `HTTP ${res.status}`);
const payload = (await res.json().catch(() => null)) as Record<string, unknown> | null;
if (!res.ok) {
// The error envelope IS top-level (`{ success: false, error }`) — no `data`
// to unwrap on this arm.
const message = (payload?.error as { message?: string } | undefined)?.message;
throw new Error(message || `HTTP ${res.status}`);
}
// Unwrap FIRST, then read the operation's flag. The `?? payload` arm mirrors
// the commit-revert helper: it would classify a hypothetical bare (unwrapped)
// answer instead of reading it as success. Against today's single wrapping
// surface only the `data` arm is live.
const inner = ((payload?.data as Record<string, unknown> | undefined) ?? payload ?? undefined) as
| DuplicateOutcome
| undefined;
if (inner?.success === false) {
throw new Error(describeDuplicateFailure(inner));
}
}

Expand Down
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers"); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 33 additions & 0 deletions .changeset/6593-duplicate-package-envelope-unwrap.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
---
'@object-ui/app-shell': patch
---

Studio's "duplicate base" now reports a partial or empty duplicate as a failure instead of
a complete success (objectui#6593).

`duplicatePackage()` read `success` at the **top level** of the response body. That is the
runtime dispatcher's envelope — `deps.success(result)` answers
`{ success: true, data }` — so on every HTTP 200 the flag it read was `true` by
construction. The operation's own verdict lives one level down in `data` and is a real
three-state: the server computes it as `failed.length === 0 && copied.length > 0`.

Two outcomes therefore answered 200 with `envelope.success: true` while the duplicate had
not succeeded, and both were shown to the author as "created", followed by a navigation
into the new base:

- **partial** — some items failed to copy. `failed[]` carries a per-item `error` string
that is the only place the reason is ever stated, and none of it was read.
- **empty** — nothing was copied at all, e.g. an all-env-wide source package under a
session that resolves no active organization (`copiedCount: 0`, `failedCount: 0`).

`duplicatePackage()` now unwraps `data` before reading the operation flag, and rejects with
a message built from what actually happened: the copied/failed counts, plus each
`failed[].error` (the first five by name, then a `+N more` tail). A generic `HTTP nnn`
message is deliberately not sufficient for the partial arm — it is the half an author needs
to act on. The non-2xx arm is unchanged and still surfaces the error envelope's message.

The unwrap-then-read order is the one `revertCommit` already uses for the sibling
commit-revert route in `preview/commitHistory.ts`; this is one consumer converging on that,
not a new convention. The route's underlying contract absence (it publishes no response
schema, so reading the wrong `success` typechecks perfectly) stays upstream in
objectstack#12038.
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,203 @@
// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license.

/**
* `duplicatePackage` must read the OPERATION's verdict, not the ENVELOPE's.
*
* `POST /packages/:id/duplicate` is served only by the runtime dispatcher, and
* that door always wraps: `deps.success(result)` answers
* `{ status: 200, body: { success: true, data } }`. So the top-level `success`
* is `true` by construction on every 200, and reading it classifies every 200
* as a complete success. The operation's own verdict lives one level down in
* `data`, computed server-side as `failed.length === 0 && copied.length > 0`.
*
* That leaves TWO reachable outcomes which answer HTTP 200 while the duplicate
* did not succeed, and this file pins BOTH — a suite that only covered
* `res.ok === false` would re-create the defect, because the transport arm was
* never the broken one:
*
* 1. PARTIAL — `failed.length > 0`. The per-item `error` strings in `failed[]`
* are the only place the reason is ever stated, so a generic `HTTP nnn`
* message is explicitly NOT sufficient here; that is asserted, not implied.
* 2. EMPTY — `copied.length === 0` with nothing named as failed, e.g. an
* all-env-wide source package under a session that resolves no active
* organization. Measured upstream as `{ success: false, copiedCount: 0,
* failedCount: 0 }`.
*
* The pattern being converged on is the sibling commit-revert helper in
* `preview/commitHistory.ts` (`revertCommit`), which already unwraps `data`
* before reading the flag. This is one consumer catching up to it, not a new
* convention.
*/

import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
import { duplicatePackage } from './packages-io';

/** The dispatcher's success envelope: the operation result under `data`. */
function envelope(data: unknown, status = 200) {
return { ok: status >= 200 && status < 300, status, json: async () => ({ success: true, data }) } as unknown as Response;
}

/** The dispatcher's error envelope — top-level, with no `data` to unwrap. */
function errorEnvelope(status: number, message: string) {
return {
ok: false,
status,
json: async () => ({ success: false, error: { message, code: 'PERMISSION_DENIED', status } }),
} as unknown as Response;
}

function stubFetch(response: Response) {
const fetchMock = vi.fn().mockResolvedValue(response);
vi.stubGlobal('fetch', fetchMock);
return fetchMock;
}

beforeEach(() => {
vi.restoreAllMocks();
});

afterEach(() => {
vi.unstubAllGlobals();
});

describe('duplicatePackage — the operation verdict lives under `data`', () => {
it('rejects a PARTIAL duplicate and names the counts and every per-item error', async () => {
stubFetch(
envelope({
success: false,
copiedCount: 7,
failedCount: 2,
targetPackageId: 'com.example.leave-copy',
copied: [],
failed: [
{ type: 'object', name: 'leave_request', error: 'copy failed: duplicate key' },
{ type: 'view', name: 'leave_list', error: 'the source item does not convert' },
],
}),
);

const err = await duplicatePackage('com.example.leave', 'com.example.leave-copy').then(
() => null,
(e: unknown) => e as Error,
);

expect(err).toBeInstanceOf(Error);
// The counts…
expect(err?.message).toContain('7');
expect(err?.message).toContain('2');
// …and the per-item reasons, which exist nowhere else in the response.
expect(err?.message).toContain('object/leave_request');
expect(err?.message).toContain('copy failed: duplicate key');
expect(err?.message).toContain('view/leave_list');
expect(err?.message).toContain('the source item does not convert');
// A generic transport message is NOT sufficient for this arm — it is the
// half the fix exists to deliver.
expect(err?.message).not.toMatch(/HTTP \d/);
});

it('summarizes the tail rather than pasting an unbounded failure list', async () => {
stubFetch(
envelope({
success: false,
copiedCount: 0,
failedCount: 7,
failed: Array.from({ length: 7 }, (_, i) => ({
type: 'object',
name: `obj_${i}`,
error: `copy failed ${i}`,
})),
}),
);

const err = await duplicatePackage('a.b.c', 'a.b.d').catch((e: unknown) => e as Error);

expect(err?.message).toContain('object/obj_0');
expect(err?.message).toContain('object/obj_4');
expect(err?.message).not.toContain('object/obj_5');
expect(err?.message).toContain('+2 more');
});

it('rejects an EMPTY duplicate — 200, nothing copied, nothing named as failed', async () => {
stubFetch(
envelope({
success: false,
copiedCount: 0,
failedCount: 0,
targetPackageId: 'com.example.leave-copy',
copied: [],
failed: [],
}),
);

const err = await duplicatePackage('com.example.leave', 'com.example.leave-copy').then(
() => null,
(e: unknown) => e as Error,
);

expect(err).toBeInstanceOf(Error);
expect(err?.message).toMatch(/nothing was copied/i);
// The empty arm has no `failed[]` to quote, so the message must still say
// something the author can act on instead of falling through to a status.
expect(err?.message).not.toMatch(/HTTP \d/);
});

it('does NOT let the envelope alone stand in for the operation verdict', async () => {
// The exact shape the defect read as success: `success: true` at the top
// level, `success: false` one level down.
const fetchMock = stubFetch(
envelope({ success: false, copiedCount: 0, failedCount: 1, failed: [{ type: 'object', name: 'x', error: 'nope' }] }),
);

await expect(duplicatePackage('a.b.c', 'a.b.d')).rejects.toThrow();
expect(fetchMock).toHaveBeenCalledTimes(1);
});

it('resolves when the operation itself succeeded', async () => {
stubFetch(
envelope({
success: true,
copiedCount: 9,
failedCount: 0,
targetPackageId: 'com.example.leave-copy',
copied: [{ type: 'object', name: 'leave_request' }],
failed: [],
}),
);

await expect(duplicatePackage('com.example.leave', 'com.example.leave-copy')).resolves.toBeUndefined();
});

it('still surfaces the error envelope message on a non-2xx', async () => {
stubFetch(errorEnvelope(403, 'Permission denied: manage_metadata is required'));

await expect(duplicatePackage('a.b.c', 'a.b.d')).rejects.toThrow(
'Permission denied: manage_metadata is required',
);
});

it('falls back to the status when a non-2xx carries no readable body', async () => {
stubFetch({
ok: false,
status: 502,
json: async () => {
throw new SyntaxError('Unexpected token < in JSON at position 0');
},
} as unknown as Response);

await expect(duplicatePackage('a.b.c', 'a.b.d')).rejects.toThrow('HTTP 502');
});

it('posts the target id and name to the duplicate route', async () => {
const fetchMock = stubFetch(envelope({ success: true, copiedCount: 1, failedCount: 0, copied: [{}], failed: [] }));

await duplicatePackage('com.example.leave', 'com.example.leave-copy', 'Leave (copy)');

expect(fetchMock).toHaveBeenCalledWith(
'/api/v1/packages/com.example.leave/duplicate',
expect.objectContaining({
method: 'POST',
body: JSON.stringify({ targetPackageId: 'com.example.leave-copy', targetName: 'Leave (copy)' }),
}),
);
});
});
81 changes: 76 additions & 5 deletions packages/app-shell/src/views/studio-design/packages-io.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -54,11 +54,71 @@ export async function fetchPackages(): Promise<PkgEntry[]> {
return parsePackages(await res.json());
}

/**
* The duplicate operation's OWN result, which travels inside the dispatcher
* envelope's `data` — not at the top level.
*
* `POST /packages/:id/duplicate` is served only by the runtime dispatcher, and
* that door always wraps (`deps.success(result)` → `{ success: true, data }`).
* So a top-level `success` read is reading the ENVELOPE, whose value is `true`
* by construction on a 200. The operation's verdict is one level down, and it
* is a real three-state: the server computes it as
* `failed.length === 0 && copied.length > 0`, which leaves TWO reachable
* outcomes that answer HTTP 200 with `data.success === false`:
*
* 1. PARTIAL — some items failed to copy. `failed[]` carries a per-item
* `error` string and is the ONLY place the reason is ever stated; a
* generic `HTTP nnn` here tells the author nothing.
* 2. EMPTY — nothing was copied at all (e.g. a source package whose rows are
* outside this caller's scope). `failed[]` is empty in this arm — nothing
* is named as having failed — so the counts are the only signal.
*
* Those two are exhaustive for `success === false`: with no failures AND at
* least one copy the server would have said `true`.
*/
interface DuplicateOutcome {
success?: boolean;
copiedCount?: number;
failedCount?: number;
failed?: Array<{ type?: string; name?: string; error?: string }>;
}

/** Per-item failures named in full before the message summarizes the rest. */
const DUPLICATE_FAILURE_DETAIL_LIMIT = 5;

/**
* Turn a false operation verdict into something the Studio author can act on.
* The counts say how far the copy got; `failed[].error` says why each item did
* not make it. Both arms are reachable on a 200 — see {@link DuplicateOutcome}.
*/
function describeDuplicateFailure(outcome: DuplicateOutcome): string {
const failed = Array.isArray(outcome.failed) ? outcome.failed : [];
const failedCount = typeof outcome.failedCount === 'number' ? outcome.failedCount : failed.length;
const copiedCount = typeof outcome.copiedCount === 'number' ? outcome.copiedCount : 0;
if (failedCount > 0) {
const shown = failed
.slice(0, DUPLICATE_FAILURE_DETAIL_LIMIT)
.map((f) => `${f.type || 'item'}/${f.name || '?'}: ${f.error || 'copy failed'}`);
const rest = failed.length - shown.length;
const detail = shown.length ? ` ${shown.join('; ')}${rest > 0 ? `; +${rest} more` : ''}` : '';
return `Partial duplicate: ${copiedCount} item(s) copied, ${failedCount} failed.${detail}`;
}
return (
'Nothing was copied: the duplicate is empty (0 items copied, 0 reported as failed). ' +
"The source package may have no active items, or its rows may be outside this session's scope."
);
}

/**
* Duplicate a package into a NEW writable base (ADR-0070 D4 — the Airtable
* "duplicate base" gesture; POST /packages/:id/duplicate). This is how a
* read-only code package becomes a customizable starting point: objects are
* re-namespaced and intra-package references rewritten server-side.
*
* Rejects on a transport/error-envelope failure AND on a false operation
* verdict, which a 200 can carry. Unwrapping before reading that verdict is
* the same order `revertCommit` uses for the sibling commit-revert route in
* `preview/commitHistory.ts`.
*/
export async function duplicatePackage(sourceId: string, targetId: string, targetName?: string): Promise<void> {
const res = await fetch(`/api/v1/packages/${encodeURIComponent(sourceId)}/duplicate`, {
Expand All@@ -67,11 +127,22 @@ export async function duplicatePackage(sourceId: string, targetId: string, targe
headers: { 'Content-Type': 'application/json', Accept: 'application/json' },
body: JSON.stringify({ targetPackageId: targetId, ...(targetName ? { targetName } : {}) }),
});
const payload = (await res.json().catch(() => null)) as
| { success?: boolean; error?: { message?: string } }
| null;
if (!res.ok || payload?.success === false) {
throw new Error(payload?.error?.message || `HTTP ${res.status}`);
const payload = (await res.json().catch(() => null)) as Record<string, unknown> | null;
if (!res.ok) {
// The error envelope IS top-level (`{ success: false, error }`) — no `data`
// to unwrap on this arm.
const message = (payload?.error as { message?: string } | undefined)?.message;
throw new Error(message || `HTTP ${res.status}`);
}
// Unwrap FIRST, then read the operation's flag. The `?? payload` arm mirrors
// the commit-revert helper: it would classify a hypothetical bare (unwrapped)
// answer instead of reading it as success. Against today's single wrapping
// surface only the `data` arm is live.
const inner = ((payload?.data as Record<string, unknown> | undefined) ?? payload ?? undefined) as
| DuplicateOutcome
| undefined;
if (inner?.success === false) {
throw new Error(describeDuplicateFailure(inner));
}
}

Expand Down
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode"); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })();
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 33 additions & 0 deletions .changeset/6593-duplicate-package-envelope-unwrap.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
---
'@object-ui/app-shell': patch
---

Studio's "duplicate base" now reports a partial or empty duplicate as a failure instead of
a complete success (objectui#6593).

`duplicatePackage()` read `success` at the **top level** of the response body. That is the
runtime dispatcher's envelope — `deps.success(result)` answers
`{ success: true, data }` — so on every HTTP 200 the flag it read was `true` by
construction. The operation's own verdict lives one level down in `data` and is a real
three-state: the server computes it as `failed.length === 0 && copied.length > 0`.

Two outcomes therefore answered 200 with `envelope.success: true` while the duplicate had
not succeeded, and both were shown to the author as "created", followed by a navigation
into the new base:

- **partial** — some items failed to copy. `failed[]` carries a per-item `error` string
that is the only place the reason is ever stated, and none of it was read.
- **empty** — nothing was copied at all, e.g. an all-env-wide source package under a
session that resolves no active organization (`copiedCount: 0`, `failedCount: 0`).

`duplicatePackage()` now unwraps `data` before reading the operation flag, and rejects with
a message built from what actually happened: the copied/failed counts, plus each
`failed[].error` (the first five by name, then a `+N more` tail). A generic `HTTP nnn`
message is deliberately not sufficient for the partial arm — it is the half an author needs
to act on. The non-2xx arm is unchanged and still surfaces the error envelope's message.

The unwrap-then-read order is the one `revertCommit` already uses for the sibling
commit-revert route in `preview/commitHistory.ts`; this is one consumer converging on that,
not a new convention. The route's underlying contract absence (it publishes no response
schema, so reading the wrong `success` typechecks perfectly) stays upstream in
objectstack#12038.
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,203 @@
// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license.

/**
* `duplicatePackage` must read the OPERATION's verdict, not the ENVELOPE's.
*
* `POST /packages/:id/duplicate` is served only by the runtime dispatcher, and
* that door always wraps: `deps.success(result)` answers
* `{ status: 200, body: { success: true, data } }`. So the top-level `success`
* is `true` by construction on every 200, and reading it classifies every 200
* as a complete success. The operation's own verdict lives one level down in
* `data`, computed server-side as `failed.length === 0 && copied.length > 0`.
*
* That leaves TWO reachable outcomes which answer HTTP 200 while the duplicate
* did not succeed, and this file pins BOTH — a suite that only covered
* `res.ok === false` would re-create the defect, because the transport arm was
* never the broken one:
*
* 1. PARTIAL — `failed.length > 0`. The per-item `error` strings in `failed[]`
* are the only place the reason is ever stated, so a generic `HTTP nnn`
* message is explicitly NOT sufficient here; that is asserted, not implied.
* 2. EMPTY — `copied.length === 0` with nothing named as failed, e.g. an
* all-env-wide source package under a session that resolves no active
* organization. Measured upstream as `{ success: false, copiedCount: 0,
* failedCount: 0 }`.
*
* The pattern being converged on is the sibling commit-revert helper in
* `preview/commitHistory.ts` (`revertCommit`), which already unwraps `data`
* before reading the flag. This is one consumer catching up to it, not a new
* convention.
*/

import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
import { duplicatePackage } from './packages-io';

/** The dispatcher's success envelope: the operation result under `data`. */
function envelope(data: unknown, status = 200) {
return { ok: status >= 200 && status < 300, status, json: async () => ({ success: true, data }) } as unknown as Response;
}

/** The dispatcher's error envelope — top-level, with no `data` to unwrap. */
function errorEnvelope(status: number, message: string) {
return {
ok: false,
status,
json: async () => ({ success: false, error: { message, code: 'PERMISSION_DENIED', status } }),
} as unknown as Response;
}

function stubFetch(response: Response) {
const fetchMock = vi.fn().mockResolvedValue(response);
vi.stubGlobal('fetch', fetchMock);
return fetchMock;
}

beforeEach(() => {
vi.restoreAllMocks();
});

afterEach(() => {
vi.unstubAllGlobals();
});

describe('duplicatePackage — the operation verdict lives under `data`', () => {
it('rejects a PARTIAL duplicate and names the counts and every per-item error', async () => {
stubFetch(
envelope({
success: false,
copiedCount: 7,
failedCount: 2,
targetPackageId: 'com.example.leave-copy',
copied: [],
failed: [
{ type: 'object', name: 'leave_request', error: 'copy failed: duplicate key' },
{ type: 'view', name: 'leave_list', error: 'the source item does not convert' },
],
}),
);

const err = await duplicatePackage('com.example.leave', 'com.example.leave-copy').then(
() => null,
(e: unknown) => e as Error,
);

expect(err).toBeInstanceOf(Error);
// The counts…
expect(err?.message).toContain('7');
expect(err?.message).toContain('2');
// …and the per-item reasons, which exist nowhere else in the response.
expect(err?.message).toContain('object/leave_request');
expect(err?.message).toContain('copy failed: duplicate key');
expect(err?.message).toContain('view/leave_list');
expect(err?.message).toContain('the source item does not convert');
// A generic transport message is NOT sufficient for this arm — it is the
// half the fix exists to deliver.
expect(err?.message).not.toMatch(/HTTP \d/);
});

it('summarizes the tail rather than pasting an unbounded failure list', async () => {
stubFetch(
envelope({
success: false,
copiedCount: 0,
failedCount: 7,
failed: Array.from({ length: 7 }, (_, i) => ({
type: 'object',
name: `obj_${i}`,
error: `copy failed ${i}`,
})),
}),
);

const err = await duplicatePackage('a.b.c', 'a.b.d').catch((e: unknown) => e as Error);

expect(err?.message).toContain('object/obj_0');
expect(err?.message).toContain('object/obj_4');
expect(err?.message).not.toContain('object/obj_5');
expect(err?.message).toContain('+2 more');
});

it('rejects an EMPTY duplicate — 200, nothing copied, nothing named as failed', async () => {
stubFetch(
envelope({
success: false,
copiedCount: 0,
failedCount: 0,
targetPackageId: 'com.example.leave-copy',
copied: [],
failed: [],
}),
);

const err = await duplicatePackage('com.example.leave', 'com.example.leave-copy').then(
() => null,
(e: unknown) => e as Error,
);

expect(err).toBeInstanceOf(Error);
expect(err?.message).toMatch(/nothing was copied/i);
// The empty arm has no `failed[]` to quote, so the message must still say
// something the author can act on instead of falling through to a status.
expect(err?.message).not.toMatch(/HTTP \d/);
});

it('does NOT let the envelope alone stand in for the operation verdict', async () => {
// The exact shape the defect read as success: `success: true` at the top
// level, `success: false` one level down.
const fetchMock = stubFetch(
envelope({ success: false, copiedCount: 0, failedCount: 1, failed: [{ type: 'object', name: 'x', error: 'nope' }] }),
);

await expect(duplicatePackage('a.b.c', 'a.b.d')).rejects.toThrow();
expect(fetchMock).toHaveBeenCalledTimes(1);
});

it('resolves when the operation itself succeeded', async () => {
stubFetch(
envelope({
success: true,
copiedCount: 9,
failedCount: 0,
targetPackageId: 'com.example.leave-copy',
copied: [{ type: 'object', name: 'leave_request' }],
failed: [],
}),
);

await expect(duplicatePackage('com.example.leave', 'com.example.leave-copy')).resolves.toBeUndefined();
});

it('still surfaces the error envelope message on a non-2xx', async () => {
stubFetch(errorEnvelope(403, 'Permission denied: manage_metadata is required'));

await expect(duplicatePackage('a.b.c', 'a.b.d')).rejects.toThrow(
'Permission denied: manage_metadata is required',
);
});

it('falls back to the status when a non-2xx carries no readable body', async () => {
stubFetch({
ok: false,
status: 502,
json: async () => {
throw new SyntaxError('Unexpected token < in JSON at position 0');
},
} as unknown as Response);

await expect(duplicatePackage('a.b.c', 'a.b.d')).rejects.toThrow('HTTP 502');
});

it('posts the target id and name to the duplicate route', async () => {
const fetchMock = stubFetch(envelope({ success: true, copiedCount: 1, failedCount: 0, copied: [{}], failed: [] }));

await duplicatePackage('com.example.leave', 'com.example.leave-copy', 'Leave (copy)');

expect(fetchMock).toHaveBeenCalledWith(
'/api/v1/packages/com.example.leave/duplicate',
expect.objectContaining({
method: 'POST',
body: JSON.stringify({ targetPackageId: 'com.example.leave-copy', targetName: 'Leave (copy)' }),
}),
);
});
});
81 changes: 76 additions & 5 deletions packages/app-shell/src/views/studio-design/packages-io.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -54,11 +54,71 @@ export async function fetchPackages(): Promise<PkgEntry[]> {
return parsePackages(await res.json());
}

/**
* The duplicate operation's OWN result, which travels inside the dispatcher
* envelope's `data` — not at the top level.
*
* `POST /packages/:id/duplicate` is served only by the runtime dispatcher, and
* that door always wraps (`deps.success(result)` → `{ success: true, data }`).
* So a top-level `success` read is reading the ENVELOPE, whose value is `true`
* by construction on a 200. The operation's verdict is one level down, and it
* is a real three-state: the server computes it as
* `failed.length === 0 && copied.length > 0`, which leaves TWO reachable
* outcomes that answer HTTP 200 with `data.success === false`:
*
* 1. PARTIAL — some items failed to copy. `failed[]` carries a per-item
* `error` string and is the ONLY place the reason is ever stated; a
* generic `HTTP nnn` here tells the author nothing.
* 2. EMPTY — nothing was copied at all (e.g. a source package whose rows are
* outside this caller's scope). `failed[]` is empty in this arm — nothing
* is named as having failed — so the counts are the only signal.
*
* Those two are exhaustive for `success === false`: with no failures AND at
* least one copy the server would have said `true`.
*/
interface DuplicateOutcome {
success?: boolean;
copiedCount?: number;
failedCount?: number;
failed?: Array<{ type?: string; name?: string; error?: string }>;
}

/** Per-item failures named in full before the message summarizes the rest. */
const DUPLICATE_FAILURE_DETAIL_LIMIT = 5;

/**
* Turn a false operation verdict into something the Studio author can act on.
* The counts say how far the copy got; `failed[].error` says why each item did
* not make it. Both arms are reachable on a 200 — see {@link DuplicateOutcome}.
*/
function describeDuplicateFailure(outcome: DuplicateOutcome): string {
const failed = Array.isArray(outcome.failed) ? outcome.failed : [];
const failedCount = typeof outcome.failedCount === 'number' ? outcome.failedCount : failed.length;
const copiedCount = typeof outcome.copiedCount === 'number' ? outcome.copiedCount : 0;
if (failedCount > 0) {
const shown = failed
.slice(0, DUPLICATE_FAILURE_DETAIL_LIMIT)
.map((f) => `${f.type || 'item'}/${f.name || '?'}: ${f.error || 'copy failed'}`);
const rest = failed.length - shown.length;
const detail = shown.length ? ` ${shown.join('; ')}${rest > 0 ? `; +${rest} more` : ''}` : '';
return `Partial duplicate: ${copiedCount} item(s) copied, ${failedCount} failed.${detail}`;
}
return (
'Nothing was copied: the duplicate is empty (0 items copied, 0 reported as failed). ' +
"The source package may have no active items, or its rows may be outside this session's scope."
);
}

/**
* Duplicate a package into a NEW writable base (ADR-0070 D4 — the Airtable
* "duplicate base" gesture; POST /packages/:id/duplicate). This is how a
* read-only code package becomes a customizable starting point: objects are
* re-namespaced and intra-package references rewritten server-side.
*
* Rejects on a transport/error-envelope failure AND on a false operation
* verdict, which a 200 can carry. Unwrapping before reading that verdict is
* the same order `revertCommit` uses for the sibling commit-revert route in
* `preview/commitHistory.ts`.
*/
export async function duplicatePackage(sourceId: string, targetId: string, targetName?: string): Promise<void> {
const res = await fetch(`/api/v1/packages/${encodeURIComponent(sourceId)}/duplicate`, {
Expand All@@ -67,11 +127,22 @@ export async function duplicatePackage(sourceId: string, targetId: string, targe
headers: { 'Content-Type': 'application/json', Accept: 'application/json' },
body: JSON.stringify({ targetPackageId: targetId, ...(targetName ? { targetName } : {}) }),
});
const payload = (await res.json().catch(() => null)) as
| { success?: boolean; error?: { message?: string } }
| null;
if (!res.ok || payload?.success === false) {
throw new Error(payload?.error?.message || `HTTP ${res.status}`);
const payload = (await res.json().catch(() => null)) as Record<string, unknown> | null;
if (!res.ok) {
// The error envelope IS top-level (`{ success: false, error }`) — no `data`
// to unwrap on this arm.
const message = (payload?.error as { message?: string } | undefined)?.message;
throw new Error(message || `HTTP ${res.status}`);
}
// Unwrap FIRST, then read the operation's flag. The `?? payload` arm mirrors
// the commit-revert helper: it would classify a hypothetical bare (unwrapped)
// answer instead of reading it as success. Against today's single wrapping
// surface only the `data` arm is live.
const inner = ((payload?.data as Record<string, unknown> | undefined) ?? payload ?? undefined) as
| DuplicateOutcome
| undefined;
if (inner?.success === false) {
throw new Error(describeDuplicateFailure(inner));
}
}

Expand Down
Loading