From 5175c64459a46ee9b9264a8ce4346c597f9c9f9d Mon Sep 17 00:00:00 2001 From: "Builder.io" Date: Thu, 10 Sep 2026 14:28:51 +0000 Subject: [PATCH 1/5] Allow admins to delete shared design systems --- .../.agents/skills/design-systems/SKILL.md | 13 ++ .../actions/delete-design-system.spec.ts | 131 ++++++++++++++++++ .../design/actions/delete-design-system.ts | 6 +- .../design/actions/list-design-systems.ts | 10 +- .../design/server/lib/design-system-access.ts | 14 ++ 5 files changed, 167 insertions(+), 7 deletions(-) create mode 100644 templates/design/actions/delete-design-system.spec.ts create mode 100644 templates/design/server/lib/design-system-access.ts diff --git a/templates/design/.agents/skills/design-systems/SKILL.md b/templates/design/.agents/skills/design-systems/SKILL.md index 63f8f7ee03f..1497bb016d5 100644 --- a/templates/design/.agents/skills/design-systems/SKILL.md +++ b/templates/design/.agents/skills/design-systems/SKILL.md @@ -181,6 +181,19 @@ pnpm action set-default-design-system --id Pass `--isDefault false` to clear the current default. Setting a system as the default unsets the previous default in the same user and organization scope. +### Deleting a Design System + +```bash +pnpm action delete-design-system --id +``` + +Requires admin access or higher — the owner, or anyone holding an `admin` +share role. That is the same `canManage` flag `list-design-systems` returns +and the Design Systems page renders its Delete control from. Removes the +system and its shares, and clears `designSystemId` on every linked design. +Those designs keep the tokens already baked into their HTML, so a design can +still look on-brand while no longer linked to a system. + ## Multi-Source Import Flow The design system setup page collects brand assets from multiple sources. When the user clicks "Continue to generation", a structured message is sent to the agent with all sources. Process each source type with the appropriate action: diff --git a/templates/design/actions/delete-design-system.spec.ts b/templates/design/actions/delete-design-system.spec.ts new file mode 100644 index 00000000000..e793a2488d1 --- /dev/null +++ b/templates/design/actions/delete-design-system.spec.ts @@ -0,0 +1,131 @@ +import { ForbiddenError, ROLE_RANK } from "@agent-native/core/sharing"; +import { beforeEach, describe, expect, it, vi } from "vitest"; + +const mocks = vi.hoisted(() => { + const txDeleteChain = { where: vi.fn() }; + const txUpdateChain = { set: vi.fn(), where: vi.fn() }; + txUpdateChain.set.mockReturnValue(txUpdateChain); + + const tx = { + delete: vi.fn(() => txDeleteChain), + update: vi.fn(() => txUpdateChain), + }; + + return { + tx, + txDeleteChain, + txUpdateChain, + db: { transaction: vi.fn(async (callback: any) => callback(tx)) }, + assertAccess: vi.fn(), + }; +}); + +vi.mock("@agent-native/core/sharing", async (importOriginal) => { + const original = + await importOriginal(); + return { ...original, assertAccess: mocks.assertAccess }; +}); + +vi.mock("drizzle-orm", async (importOriginal) => { + const original = await importOriginal(); + return { + ...original, + eq: (left: unknown, right: unknown) => ({ left, right }), + }; +}); + +vi.mock("../server/db/index.js", () => ({ + getDb: () => mocks.db, + schema: { + designs: { + designSystemId: "designs.designSystemId", + updatedAt: "designs.updatedAt", + }, + designSystemShares: { resourceId: "designSystemShares.resourceId" }, + designSystems: { id: "designSystems.id" }, + }, +})); + +import { + DESIGN_SYSTEM_MANAGE_ROLE, + canManageDesignSystemRole, +} from "../server/lib/design-system-access.js"; +import action from "./delete-design-system.js"; + +describe("delete-design-system", () => { + beforeEach(() => { + vi.clearAllMocks(); + mocks.assertAccess.mockReset(); + mocks.assertAccess.mockResolvedValue({ + role: "owner", + resource: { id: "ds_shared" }, + }); + }); + + // The Design Systems page renders its Delete menu item and bulk-delete + // checkbox from the `canManage` flag list-design-systems reports. Enforcing + // a stricter role here hands every shared admin a button that 403s. + it("enforces exactly the role the UI reports as manageable", async () => { + await action.run({ id: "ds_shared" }); + + expect(mocks.assertAccess).toHaveBeenCalledWith( + "design-system", + "ds_shared", + DESIGN_SYSTEM_MANAGE_ROLE, + ); + + const enforcedRole = mocks.assertAccess.mock.calls[0][2] as + | "owner" + | "admin" + | "editor" + | "commenter" + | "viewer"; + expect(canManageDesignSystemRole(enforcedRole)).toBe(true); + }); + + it("lets a non-owner admin delete a shared design system", async () => { + // Mirrors assertAccess's own rank comparison against the caller's role. + mocks.assertAccess.mockImplementation( + async (type: string, id: string, minRole: "owner" | "admin") => { + const callerRole = "admin"; + if (ROLE_RANK[callerRole] < ROLE_RANK[minRole]) { + throw new ForbiddenError( + `Requires ${minRole} role on ${type} ${id} (have ${callerRole})`, + ); + } + return { role: callerRole, resource: { id } }; + }, + ); + + await expect(action.run({ id: "ds_shared" })).resolves.toEqual({ + id: "ds_shared", + deleted: true, + }); + expect(mocks.db.transaction).toHaveBeenCalledTimes(1); + }); + + it("still refuses a viewer", async () => { + mocks.assertAccess.mockRejectedValue(new ForbiddenError("No access")); + + await expect(action.run({ id: "ds_shared" })).rejects.toThrow( + ForbiddenError, + ); + expect(mocks.db.transaction).not.toHaveBeenCalled(); + }); + + it("unlinks designs and drops share rows in one transaction", async () => { + await action.run({ id: "ds_shared" }); + + expect(mocks.txUpdateChain.set).toHaveBeenCalledWith( + expect.objectContaining({ designSystemId: null }), + ); + expect(mocks.tx.delete).toHaveBeenCalledWith( + expect.objectContaining({ + resourceId: "designSystemShares.resourceId", + }), + ); + expect(mocks.tx.delete).toHaveBeenCalledWith( + expect.objectContaining({ id: "designSystems.id" }), + ); + }); +}); diff --git a/templates/design/actions/delete-design-system.ts b/templates/design/actions/delete-design-system.ts index 5ea2d774677..08f3bf792e2 100644 --- a/templates/design/actions/delete-design-system.ts +++ b/templates/design/actions/delete-design-system.ts @@ -4,15 +4,17 @@ import { eq } from "drizzle-orm"; import { z } from "zod"; import { getDb, schema } from "../server/db/index.js"; +import { DESIGN_SYSTEM_MANAGE_ROLE } from "../server/lib/design-system-access.js"; export default defineAction({ description: - "Delete a design system. Requires owner access. Designs linked to it are unlinked.", + "Delete a design system. Requires admin access or higher. Designs linked " + + "to it are unlinked.", schema: z.object({ id: z.string().min(1).describe("Design system ID to delete"), }), run: async ({ id }) => { - await assertAccess("design-system", id, "owner"); + await assertAccess("design-system", id, DESIGN_SYSTEM_MANAGE_ROLE); const db = getDb(); diff --git a/templates/design/actions/list-design-systems.ts b/templates/design/actions/list-design-systems.ts index 04f65a3edb1..6fa1307d91b 100644 --- a/templates/design/actions/list-design-systems.ts +++ b/templates/design/actions/list-design-systems.ts @@ -9,12 +9,9 @@ import { desc } from "drizzle-orm"; import { z } from "zod"; import { getDb, schema } from "../server/db/index.js"; +import { canManageDesignSystemRole } from "../server/lib/design-system-access.js"; import { resolveDefaultDesignSystemId } from "../server/lib/design-system-defaults.js"; -function canManageRole(role: "owner" | ShareRole) { - return role === "owner" || role === "admin"; -} - export default defineAction({ description: "List all design systems accessible to the current user. Returns title, " + @@ -69,7 +66,10 @@ export default defineAction({ ); } const role = access?.role ?? "viewer"; - accessById.set(row.id, { role, canManage: canManageRole(role) }); + accessById.set(row.id, { + role, + canManage: canManageDesignSystemRole(role), + }); }), ); diff --git a/templates/design/server/lib/design-system-access.ts b/templates/design/server/lib/design-system-access.ts new file mode 100644 index 00000000000..b35aec70b1d --- /dev/null +++ b/templates/design/server/lib/design-system-access.ts @@ -0,0 +1,14 @@ +import { ROLE_RANK, type ShareRole } from "@agent-native/core/sharing"; + +/** + * One role decides both what `list-design-systems` reports as `canManage` and + * what `delete-design-system` enforces. When those two drifted apart, the + * Design Systems page rendered Delete for every shared admin and the action + * answered "Requires owner role" — the affordance and the boundary must read + * the same constant, not two copies of the same intent. + */ +export const DESIGN_SYSTEM_MANAGE_ROLE: ShareRole = "admin"; + +export function canManageDesignSystemRole(role: "owner" | ShareRole): boolean { + return ROLE_RANK[role] >= ROLE_RANK[DESIGN_SYSTEM_MANAGE_ROLE]; +} From 01f0357650aa38d4e22ddcc6bf07ab6d4533dc51 Mon Sep 17 00:00:00 2001 From: "Builder.io" Date: Thu, 10 Sep 2026 14:32:43 +0000 Subject: [PATCH 2/5] Preserve sharing exports in design system test mocks --- .../design/actions/list-design-systems.test.ts | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/templates/design/actions/list-design-systems.test.ts b/templates/design/actions/list-design-systems.test.ts index 7cc229a5387..f533328b9b7 100644 --- a/templates/design/actions/list-design-systems.test.ts +++ b/templates/design/actions/list-design-systems.test.ts @@ -56,10 +56,15 @@ vi.mock("@agent-native/core/server/request-context", () => ({ getRequestOrgId: () => null, })); -vi.mock("@agent-native/core/sharing", () => ({ - accessFilter: () => ({ __accessFilter: true }), - resolveAccess: (...args: [string, string]) => mocks.resolveAccess(...args), -})); +vi.mock("@agent-native/core/sharing", async (importOriginal) => { + const original = + await importOriginal(); + return { + ...original, + accessFilter: () => ({ __accessFilter: true }), + resolveAccess: (...args: [string, string]) => mocks.resolveAccess(...args), + }; +}); vi.mock("drizzle-orm", () => ({ and: (...values: unknown[]) => ({ and: values }), From f7a7c5feb1234d6dc0c48db9f302a39d5fdf1bfd Mon Sep 17 00:00:00 2001 From: "Builder.io" Date: Thu, 10 Sep 2026 14:58:04 +0000 Subject: [PATCH 3/5] fix(slides): keep malformed design systems deletable --- .../.agents/skills/design-systems/SKILL.md | 8 +++ .../app/pages/DesignSystems.data.test.ts | 54 +++++++++++++++++++ templates/slides/app/pages/DesignSystems.tsx | 35 ++++++------ 3 files changed, 79 insertions(+), 18 deletions(-) create mode 100644 templates/slides/app/pages/DesignSystems.data.test.ts diff --git a/templates/slides/.agents/skills/design-systems/SKILL.md b/templates/slides/.agents/skills/design-systems/SKILL.md index 01d83111f77..f7c8fbff65c 100644 --- a/templates/slides/.agents/skills/design-systems/SKILL.md +++ b/templates/slides/.agents/skills/design-systems/SKILL.md @@ -113,6 +113,14 @@ promoted to default so future deck creation doesn't silently drop to "no design system". Deletion does not remove an upstream Builder-indexed design system. +The Design Systems page renders every row `list-design-systems` returns — +including rows written before `data` validation existed, whose `colors` or +`typography` sections may be empty or missing. `parseDesignSystemListData` in +`app/pages/DesignSystems.tsx` fills gaps with the same defaults +`useDeckDesignSystem` applies rather than hiding the row, so a legacy or +malformed design system always keeps a visible card and a working Delete +control. + ## Applying to Slides Before creating or extending a system, read the `creative-context` skill and diff --git a/templates/slides/app/pages/DesignSystems.data.test.ts b/templates/slides/app/pages/DesignSystems.data.test.ts new file mode 100644 index 00000000000..49d83af19f0 --- /dev/null +++ b/templates/slides/app/pages/DesignSystems.data.test.ts @@ -0,0 +1,54 @@ +import { describe, expect, it } from "vitest"; + +import { parseDesignSystemListData } from "./DesignSystems.js"; + +describe("parseDesignSystemListData", () => { + // Design systems written before create/update validation existed can have + // `colors: {}` or be missing whole sections. This page used to hide any + // such row from the grid entirely — no card, no Delete menu item, no way + // to remove it from the UI at all. It must always return a renderable + // DesignSystemData instead of a falsy value. + it("fills in missing color and typography fields instead of signaling absence", () => { + const result = parseDesignSystemListData(JSON.stringify({ colors: {} })); + + expect(result).toBeTruthy(); + expect(result.colors.primary).toBeTruthy(); + expect(result.colors.background).toBeTruthy(); + expect(result.typography.headingFont).toBeTruthy(); + expect(result.typography.bodyFont).toBeTruthy(); + }); + + it("falls back to full defaults for corrupt, non-JSON stored data", () => { + const result = parseDesignSystemListData("not json"); + + expect(result).toBeTruthy(); + expect(result.colors.primary).toBeTruthy(); + expect(result.typography.headingFont).toBeTruthy(); + }); + + it("preserves fields a fully-formed design system already has", () => { + const stored = { + colors: { + primary: "#123456", + secondary: "#654321", + accent: "#abcdef", + background: "#ffffff", + surface: "#f0f0f0", + text: "#000000", + textMuted: "#888888", + }, + typography: { + headingFont: "Custom Heading", + bodyFont: "Custom Body", + headingWeight: "700", + bodyWeight: "400", + headingSizes: { h1: "48px", h2: "32px", h3: "24px" }, + }, + }; + + const result = parseDesignSystemListData(JSON.stringify(stored)); + + expect(result.colors.primary).toBe("#123456"); + expect(result.typography.headingFont).toBe("Custom Heading"); + }); +}); diff --git a/templates/slides/app/pages/DesignSystems.tsx b/templates/slides/app/pages/DesignSystems.tsx index 7cac4fc294c..c0212c8a81d 100644 --- a/templates/slides/app/pages/DesignSystems.tsx +++ b/templates/slides/app/pages/DesignSystems.tsx @@ -26,11 +26,26 @@ import { AlertDialogTitle, } from "@/components/ui/alert-dialog"; import { Button } from "@/components/ui/button"; +import { mergeDesignSystemData } from "@/hooks/use-deck-design-system"; import { useDesignSystems } from "@/hooks/use-design-systems"; import { useWorkspaceDefaults } from "@/hooks/use-workspace-defaults"; import type { DesignSystemData } from "../../shared/api"; -import { missingDesignSystemDataFields } from "../../shared/design-system-validation"; + +// DesignSystemCard reads colors.* / typography.* unconditionally, down to +// nested fields like typography.headingFont. Rows written before +// create/update validation existed can have `colors: {}`, which used to make +// this page hide the row entirely — the card, its Delete menu item, and the +// only UI path to remove it all disappeared with no error. Filling gaps with +// the same defaults `useDeckDesignSystem` applies keeps the card (and its +// delete affordance) visible instead. +export function parseDesignSystemListData(dataStr: string): DesignSystemData { + try { + return mergeDesignSystemData(JSON.parse(dataStr)); + } catch { + return mergeDesignSystemData(undefined); + } +} export default function DesignSystems() { const t = useT(); @@ -143,21 +158,6 @@ export default function DesignSystems() { }); }; - const parseDesignData = (dataStr: string): DesignSystemData | null => { - try { - const parsed = JSON.parse(dataStr) as DesignSystemData; - // DesignSystemCard reads colors.* / typography.* unconditionally, down - // to nested fields like typography.headingFont. Rows written before - // create/update validation existed can have `colors: {}` and still - // pass a truthy check, so reuse the same nested-field validator the - // actions use rather than only checking the top-level objects exist. - if (missingDesignSystemDataFields(parsed).length > 0) return null; - return parsed; - } catch { - return null; - } - }; - useSetPageTitle(t("header.designSystems")); useSetHeaderActions( @@ -254,8 +254,7 @@ export default function DesignSystems() { {/* Design system cards */} {designSystems.map((ds) => { - const parsed = parseDesignData(ds.data); - if (!parsed) return null; + const parsed = parseDesignSystemListData(ds.data); return ( Date: Thu, 10 Sep 2026 17:32:48 +0000 Subject: [PATCH 4/5] Safely unlink accessible designs when deleting systems --- .../.agents/skills/design-systems/SKILL.md | 14 +- .../actions/delete-design-system.spec.ts | 191 ++++++++++++++++-- .../design/actions/delete-design-system.ts | 143 ++++++++++++- .../app/hooks/use-deck-design-system.test.ts | 20 ++ .../app/hooks/use-deck-design-system.ts | 9 +- 5 files changed, 348 insertions(+), 29 deletions(-) diff --git a/templates/design/.agents/skills/design-systems/SKILL.md b/templates/design/.agents/skills/design-systems/SKILL.md index 1497bb016d5..84da7fcdf03 100644 --- a/templates/design/.agents/skills/design-systems/SKILL.md +++ b/templates/design/.agents/skills/design-systems/SKILL.md @@ -190,9 +190,17 @@ pnpm action delete-design-system --id Requires admin access or higher — the owner, or anyone holding an `admin` share role. That is the same `canManage` flag `list-design-systems` returns and the Design Systems page renders its Delete control from. Removes the -system and its shares, and clears `designSystemId` on every linked design. -Those designs keep the tokens already baked into their HTML, so a design can -still look on-brand while no longer linked to a system. +system and its shares, and clears `designSystemId` on every linked design and +saved template the caller can edit — a design system's admin share does not +grant write access to every design or template that happens to reference it, +so ones the caller can't edit keep a dangling reference instead, reported +back as `designsSkippedForAccess` / `templatesSkippedForAccess` (both +`get-design-template` and `list-design-templates` already resolve a dangling +`designSystemId` back to `null` rather than erroring). Those designs keep the +tokens already baked into their HTML, so a design can still look on-brand +while no longer linked to a system. If the deleted system was the owner's +default, another of their design systems is promoted to default so future +design creation doesn't silently drop to "no design system". ## Multi-Source Import Flow diff --git a/templates/design/actions/delete-design-system.spec.ts b/templates/design/actions/delete-design-system.spec.ts index e793a2488d1..db16125d21d 100644 --- a/templates/design/actions/delete-design-system.spec.ts +++ b/templates/design/actions/delete-design-system.spec.ts @@ -2,6 +2,52 @@ import { ForbiddenError, ROLE_RANK } from "@agent-native/core/sharing"; import { beforeEach, describe, expect, it, vi } from "vitest"; const mocks = vi.hoisted(() => { + const designsTable = { + id: "designs.id", + designSystemId: "designs.designSystemId", + updatedAt: "designs.updatedAt", + }; + const designTemplatesTable = { + id: "designTemplates.id", + designSystemId: "designTemplates.designSystemId", + updatedAt: "designTemplates.updatedAt", + }; + const designSystemsTable = { + id: "designSystems.id", + ownerEmail: "designSystems.ownerEmail", + orgId: "designSystems.orgId", + updatedAt: "designSystems.updatedAt", + }; + const designSystemSharesTable = { resourceId: "designSystemShares.resourceId" }; + + const state = { + linkedDesignRows: [] as Array<{ id: string }>, + linkedTemplateRows: [] as Array<{ id: string }>, + promotionCandidateRows: [] as Array<{ id: string }>, + resolvedAccess: new Map(), + }; + + const designsSelectChain = { + where: vi.fn(() => Promise.resolve(state.linkedDesignRows)), + }; + const designTemplatesSelectChain = { + where: vi.fn(() => Promise.resolve(state.linkedTemplateRows)), + }; + const dbFromRouter = { + from: vi.fn((table: unknown) => { + if (table === designsTable) return designsSelectChain; + if (table === designTemplatesTable) return designTemplatesSelectChain; + throw new Error("unexpected table passed to db.select().from()"); + }), + }; + + const promotionSelectChain = { + from: vi.fn(() => promotionSelectChain), + where: vi.fn(() => promotionSelectChain), + orderBy: vi.fn(() => promotionSelectChain), + limit: vi.fn(() => Promise.resolve(state.promotionCandidateRows)), + }; + const txDeleteChain = { where: vi.fn() }; const txUpdateChain = { set: vi.fn(), where: vi.fn() }; txUpdateChain.set.mockReturnValue(txUpdateChain); @@ -9,27 +55,64 @@ const mocks = vi.hoisted(() => { const tx = { delete: vi.fn(() => txDeleteChain), update: vi.fn(() => txUpdateChain), + select: vi.fn(() => promotionSelectChain), + }; + + const dbUpdateChain = { set: vi.fn(), where: vi.fn() }; + dbUpdateChain.set.mockReturnValue(dbUpdateChain); + + const db = { + select: vi.fn(() => dbFromRouter), + update: vi.fn(() => dbUpdateChain), + transaction: vi.fn(async (callback: (tx: typeof tx) => unknown) => + callback(tx), + ), }; + const resolveAccess = vi.fn( + async (type: string, id: string) => state.resolvedAccess.get(`${type}:${id}`) ?? null, + ); + return { + state, + designsTable, + designTemplatesTable, + designSystemsTable, + designSystemSharesTable, + designsSelectChain, + designTemplatesSelectChain, + promotionSelectChain, tx, txDeleteChain, txUpdateChain, - db: { transaction: vi.fn(async (callback: any) => callback(tx)) }, + db, + dbUpdateChain, assertAccess: vi.fn(), + resolveAccess, }; }); vi.mock("@agent-native/core/sharing", async (importOriginal) => { const original = await importOriginal(); - return { ...original, assertAccess: mocks.assertAccess }; + return { + ...original, + assertAccess: mocks.assertAccess, + resolveAccess: mocks.resolveAccess, + }; }); +vi.mock("@agent-native/core/server/request-context", () => ({ + getRequestOrgId: () => null, +})); + vi.mock("drizzle-orm", async (importOriginal) => { const original = await importOriginal(); return { ...original, + and: (...values: unknown[]) => ({ and: values }), + desc: (value: unknown) => ({ desc: value }), + isNull: (value: unknown) => ({ isNull: value }), eq: (left: unknown, right: unknown) => ({ left, right }), }; }); @@ -37,12 +120,10 @@ vi.mock("drizzle-orm", async (importOriginal) => { vi.mock("../server/db/index.js", () => ({ getDb: () => mocks.db, schema: { - designs: { - designSystemId: "designs.designSystemId", - updatedAt: "designs.updatedAt", - }, - designSystemShares: { resourceId: "designSystemShares.resourceId" }, - designSystems: { id: "designSystems.id" }, + designs: mocks.designsTable, + designTemplates: mocks.designTemplatesTable, + designSystemShares: mocks.designSystemSharesTable, + designSystems: mocks.designSystemsTable, }, })); @@ -55,10 +136,19 @@ import action from "./delete-design-system.js"; describe("delete-design-system", () => { beforeEach(() => { vi.clearAllMocks(); + mocks.state.linkedDesignRows = []; + mocks.state.linkedTemplateRows = []; + mocks.state.promotionCandidateRows = []; + mocks.state.resolvedAccess = new Map(); mocks.assertAccess.mockReset(); mocks.assertAccess.mockResolvedValue({ role: "owner", - resource: { id: "ds_shared" }, + resource: { + id: "ds_shared", + ownerEmail: "owner@example.com", + orgId: null, + isDefault: false, + }, }); }); @@ -93,7 +183,15 @@ describe("delete-design-system", () => { `Requires ${minRole} role on ${type} ${id} (have ${callerRole})`, ); } - return { role: callerRole, resource: { id } }; + return { + role: callerRole, + resource: { + id, + ownerEmail: "owner@example.com", + orgId: null, + isDefault: false, + }, + }; }, ); @@ -113,12 +211,9 @@ describe("delete-design-system", () => { expect(mocks.db.transaction).not.toHaveBeenCalled(); }); - it("unlinks designs and drops share rows in one transaction", async () => { + it("drops share rows and the design-system row in one transaction", async () => { await action.run({ id: "ds_shared" }); - expect(mocks.txUpdateChain.set).toHaveBeenCalledWith( - expect.objectContaining({ designSystemId: null }), - ); expect(mocks.tx.delete).toHaveBeenCalledWith( expect.objectContaining({ resourceId: "designSystemShares.resourceId", @@ -128,4 +223,72 @@ describe("delete-design-system", () => { expect.objectContaining({ id: "designSystems.id" }), ); }); + + // A shared admin's write access to the design system does not extend to + // every design that happens to reference it — those belong to whoever + // owns them. Only designs the caller can actually edit get unlinked. + it("unlinks designs the caller can edit and skips the rest", async () => { + mocks.state.linkedDesignRows = [{ id: "design-1" }, { id: "design-2" }]; + mocks.state.resolvedAccess.set("design:design-1", { role: "editor" }); + mocks.state.resolvedAccess.set("design:design-2", { role: "viewer" }); + + const result = await action.run({ id: "ds_shared" }); + + expect(mocks.resolveAccess).toHaveBeenCalledWith("design", "design-1"); + expect(mocks.resolveAccess).toHaveBeenCalledWith("design", "design-2"); + expect(mocks.dbUpdateChain.set).toHaveBeenCalledTimes(1); + expect(mocks.dbUpdateChain.set).toHaveBeenCalledWith( + expect.objectContaining({ designSystemId: null }), + ); + expect(result).toMatchObject({ + id: "ds_shared", + deleted: true, + designsSkippedForAccess: ["design-2"], + }); + }); + + it("unlinks saved templates the caller manages and skips the rest", async () => { + mocks.state.linkedTemplateRows = [{ id: "tmpl-1" }, { id: "tmpl-2" }]; + mocks.state.resolvedAccess.set("design-template:tmpl-1", { role: "admin" }); + mocks.state.resolvedAccess.set("design-template:tmpl-2", { + role: "editor", + }); + + const result = await action.run({ id: "ds_shared" }); + + expect(result).toMatchObject({ + id: "ds_shared", + deleted: true, + templatesSkippedForAccess: ["tmpl-2"], + }); + }); + + it("promotes another of the owner's design systems when the deleted one was their default", async () => { + mocks.assertAccess.mockResolvedValue({ + role: "owner", + resource: { + id: "ds_shared", + ownerEmail: "owner@example.com", + orgId: null, + isDefault: true, + }, + }); + mocks.state.promotionCandidateRows = [{ id: "ds_next" }]; + + await action.run({ id: "ds_shared" }); + + expect(mocks.tx.select).toHaveBeenCalled(); + expect(mocks.txUpdateChain.set).toHaveBeenCalledWith( + expect.objectContaining({ isDefault: true }), + ); + expect(mocks.txUpdateChain.where).toHaveBeenCalledWith( + expect.objectContaining({ left: "designSystems.id", right: "ds_next" }), + ); + }); + + it("does not touch other design systems when the deleted one was not the default", async () => { + await action.run({ id: "ds_shared" }); + + expect(mocks.tx.select).not.toHaveBeenCalled(); + }); }); diff --git a/templates/design/actions/delete-design-system.ts b/templates/design/actions/delete-design-system.ts index 08f3bf792e2..98ee3d70a88 100644 --- a/templates/design/actions/delete-design-system.ts +++ b/templates/design/actions/delete-design-system.ts @@ -1,29 +1,97 @@ import { defineAction } from "@agent-native/core/action"; -import { assertAccess } from "@agent-native/core/sharing"; -import { eq } from "drizzle-orm"; +import { getRequestOrgId } from "@agent-native/core/server/request-context"; +import { + assertAccess, + resolveAccess, + ROLE_RANK, + type ShareRole, +} from "@agent-native/core/sharing"; +import { and, desc, eq, isNull } from "drizzle-orm"; import { z } from "zod"; import { getDb, schema } from "../server/db/index.js"; import { DESIGN_SYSTEM_MANAGE_ROLE } from "../server/lib/design-system-access.js"; +type EffectiveRole = "owner" | ShareRole; + +function canEditDesignRole(role: EffectiveRole) { + return ROLE_RANK[role] >= ROLE_RANK.editor; +} + +function canManageTemplateRole(role: EffectiveRole) { + return ROLE_RANK[role] >= ROLE_RANK.admin; +} + +type UnlinkResult = { id: string; status: "unlinked" | "skipped-no-access" }; + +// Admin access on a shared design system does not grant write access to +// every design or saved template that happens to reference it — those are +// separate resources with their own owners and shares. Skip anything the +// caller can't edit instead of silently rewriting it, and report the skips +// back so the caller can decide whether to follow up. +async function unlinkDesign(designId: string): Promise { + const access = await resolveAccess("design", designId); + if (!access || !canEditDesignRole(access.role)) { + return { id: designId, status: "skipped-no-access" }; + } + await getDb() + .update(schema.designs) + .set({ designSystemId: null, updatedAt: new Date().toISOString() }) + .where(eq(schema.designs.id, designId)); + return { id: designId, status: "unlinked" }; +} + +async function unlinkDesignTemplate(templateId: string): Promise { + const access = await resolveAccess("design-template", templateId); + if (!access || !canManageTemplateRole(access.role)) { + return { id: templateId, status: "skipped-no-access" }; + } + await getDb() + .update(schema.designTemplates) + .set({ designSystemId: null, updatedAt: new Date().toISOString() }) + .where(eq(schema.designTemplates.id, templateId)); + return { id: templateId, status: "unlinked" }; +} + export default defineAction({ description: - "Delete a design system. Requires admin access or higher. Designs linked " + - "to it are unlinked.", + "Delete a design system. Requires admin access or higher. Designs and " + + "saved templates linked to it that the caller can edit are unlinked; " + + "others keep a dangling reference. If the deleted system was the " + + "owner's default, another of their design systems is promoted to " + + "default.", schema: z.object({ id: z.string().min(1).describe("Design system ID to delete"), }), run: async ({ id }) => { - await assertAccess("design-system", id, DESIGN_SYSTEM_MANAGE_ROLE); + const access = await assertAccess( + "design-system", + id, + DESIGN_SYSTEM_MANAGE_ROLE, + ); const db = getDb(); + const orgId = getRequestOrgId(); - await db.transaction(async (tx) => { - await tx - .update(schema.designs) - .set({ designSystemId: null, updatedAt: new Date().toISOString() }) - .where(eq(schema.designs.designSystemId, id)); + const [linkedDesignIds, linkedTemplateIds] = await Promise.all([ + db + .select({ id: schema.designs.id }) + .from(schema.designs) + .where(eq(schema.designs.designSystemId, id)) + .then((rows) => rows.map((row) => row.id)), + db + .select({ id: schema.designTemplates.id }) + .from(schema.designTemplates) + .where(eq(schema.designTemplates.designSystemId, id)) + .then((rows) => rows.map((row) => row.id)), + ]); + // Delete the design system (and its shares) before touching linked + // designs/templates. Once the row is gone, other actions' + // assertAccess("design-system", ...) checks fail for anyone trying to + // attach a fresh link, shrinking the window for a new link to attach to + // a design system being deleted. + await db.transaction(async (tx) => { await tx .delete(schema.designSystemShares) .where(eq(schema.designSystemShares.resourceId, id)); @@ -31,8 +99,61 @@ export default defineAction({ await tx .delete(schema.designSystems) .where(eq(schema.designSystems.id, id)); + + if (access.resource.isDefault) { + const ownerScope = orgId + ? and( + eq(schema.designSystems.ownerEmail, access.resource.ownerEmail), + eq(schema.designSystems.orgId, orgId), + ) + : and( + eq(schema.designSystems.ownerEmail, access.resource.ownerEmail), + isNull(schema.designSystems.orgId), + ); + const [next] = await tx + .select({ id: schema.designSystems.id }) + .from(schema.designSystems) + .where(ownerScope) + .orderBy(desc(schema.designSystems.updatedAt)) + .limit(1); + if (next) { + await tx + .update(schema.designSystems) + .set({ isDefault: true, updatedAt: new Date().toISOString() }) + .where(eq(schema.designSystems.id, next.id)); + } + } }); - return { id, deleted: true }; + // Best-effort cleanup: the design system is already gone, so a design or + // template we can't touch (missing access) is left dangling rather than + // retried here — both get-design-template and list-design-templates + // already mask a dangling designSystemId by resolving it back to null. + const [designResults, templateResults] = await Promise.all([ + Promise.allSettled(linkedDesignIds.map(unlinkDesign)), + Promise.allSettled(linkedTemplateIds.map(unlinkDesignTemplate)), + ]); + + const designsSkippedForAccess = linkedDesignIds.filter( + (_, index) => + designResults[index].status === "fulfilled" && + (designResults[index] as PromiseFulfilledResult).value + .status === "skipped-no-access", + ); + const templatesSkippedForAccess = linkedTemplateIds.filter( + (_, index) => + templateResults[index].status === "fulfilled" && + (templateResults[index] as PromiseFulfilledResult).value + .status === "skipped-no-access", + ); + + return { + id, + deleted: true, + ...(designsSkippedForAccess.length > 0 ? { designsSkippedForAccess } : {}), + ...(templatesSkippedForAccess.length > 0 + ? { templatesSkippedForAccess } + : {}), + }; }, }); diff --git a/templates/slides/app/hooks/use-deck-design-system.test.ts b/templates/slides/app/hooks/use-deck-design-system.test.ts index 8f681f40cb8..cae196b6d22 100644 --- a/templates/slides/app/hooks/use-deck-design-system.test.ts +++ b/templates/slides/app/hooks/use-deck-design-system.test.ts @@ -39,6 +39,26 @@ describe("mergeDesignSystemData", () => { expect(merged.logos).toEqual([]); }); + it("falls back to the default when a leaf value has the wrong runtime type", () => { + // DesignSystemCard's firstFontName() calls `.split()` on + // typography.headingFont with no type guard. An interrupted generation + // that persisted an object here must not survive the merge -- it would + // crash the whole Design Systems list, not just this row. + const merged = mergeDesignSystemData({ + typography: { headingFont: {}, bodyWeight: 450 }, + borders: { radius: ["14px"] }, + }); + + expect(merged.typography.headingFont).toBe( + DEFAULT_DESIGN_SYSTEM.typography.headingFont, + ); + expect(typeof merged.typography.headingFont).toBe("string"); + expect(merged.typography.bodyWeight).toBe( + DEFAULT_DESIGN_SYSTEM.typography.bodyWeight, + ); + expect(merged.borders.radius).toBe(DEFAULT_DESIGN_SYSTEM.borders.radius); + }); + it("normalizes design-system image style reference urls", () => { expect( getDesignSystemImageStyleReferenceUrls({ diff --git a/templates/slides/app/hooks/use-deck-design-system.ts b/templates/slides/app/hooks/use-deck-design-system.ts index f13046ab0e6..adc77720180 100644 --- a/templates/slides/app/hooks/use-deck-design-system.ts +++ b/templates/slides/app/hooks/use-deck-design-system.ts @@ -58,7 +58,14 @@ function mergeWithDefaults(defaults: T, value: unknown): T { return merged as T; } - return (value === undefined || value === null ? defaults : value) as T; + // Every leaf in DEFAULT_DESIGN_SYSTEM is a string (including union-typed + // ones like slideDefaults.labelStyle), and DesignSystemCard/slide renderers + // call string methods (e.g. `.split()` on typography.headingFont) with no + // type guard. A persisted value of the wrong runtime type — an empty + // object from an interrupted generation, a stray number — must fall back + // to the default rather than reach those call sites and crash the caller. + if (value === undefined || value === null) return defaults; + return (typeof value === typeof defaults ? value : defaults) as T; } export function getDesignSystemImageStyleReferenceUrls( From 100e00428ae6442f479ddc15e4dd7c9b206f9b6f Mon Sep 17 00:00:00 2001 From: Liam DeBeasi Date: Thu, 10 Sep 2026 13:58:58 -0400 Subject: [PATCH 5/5] lint --- .../2026-09-10-open-mcp-connections-in-new-tabs.md | 1 + templates/design/actions/delete-design-system.spec.ts | 7 +++++-- templates/design/actions/delete-design-system.ts | 4 +++- 3 files changed, 9 insertions(+), 3 deletions(-) diff --git a/templates/clips/changelog/2026-09-10-open-mcp-connections-in-new-tabs.md b/templates/clips/changelog/2026-09-10-open-mcp-connections-in-new-tabs.md index 307766d512c..62dedc07160 100644 --- a/templates/clips/changelog/2026-09-10-open-mcp-connections-in-new-tabs.md +++ b/templates/clips/changelog/2026-09-10-open-mcp-connections-in-new-tabs.md @@ -2,4 +2,5 @@ type: fixed date: 2026-09-10 --- + Builder and other MCP connections now open setup in a new tab. diff --git a/templates/design/actions/delete-design-system.spec.ts b/templates/design/actions/delete-design-system.spec.ts index db16125d21d..5b7f464c19e 100644 --- a/templates/design/actions/delete-design-system.spec.ts +++ b/templates/design/actions/delete-design-system.spec.ts @@ -18,7 +18,9 @@ const mocks = vi.hoisted(() => { orgId: "designSystems.orgId", updatedAt: "designSystems.updatedAt", }; - const designSystemSharesTable = { resourceId: "designSystemShares.resourceId" }; + const designSystemSharesTable = { + resourceId: "designSystemShares.resourceId", + }; const state = { linkedDesignRows: [] as Array<{ id: string }>, @@ -70,7 +72,8 @@ const mocks = vi.hoisted(() => { }; const resolveAccess = vi.fn( - async (type: string, id: string) => state.resolvedAccess.get(`${type}:${id}`) ?? null, + async (type: string, id: string) => + state.resolvedAccess.get(`${type}:${id}`) ?? null, ); return { diff --git a/templates/design/actions/delete-design-system.ts b/templates/design/actions/delete-design-system.ts index 98ee3d70a88..cb0a5ea250c 100644 --- a/templates/design/actions/delete-design-system.ts +++ b/templates/design/actions/delete-design-system.ts @@ -150,7 +150,9 @@ export default defineAction({ return { id, deleted: true, - ...(designsSkippedForAccess.length > 0 ? { designsSkippedForAccess } : {}), + ...(designsSkippedForAccess.length > 0 + ? { designsSkippedForAccess } + : {}), ...(templatesSkippedForAccess.length > 0 ? { templatesSkippedForAccess } : {}),