From 222cd0ef81bfdf1dc60fe45cb77aad1e528e19bb Mon Sep 17 00:00:00 2001 From: J Date: Thu, 11 Jun 2026 13:27:34 +0800 Subject: [PATCH] fix(skills): show creator name instead of UUID in import conflict UI When a local skill import hits a name conflict with a skill owned by another user, the locked-creator message rendered the raw existing_created_by UUID via the {{creator}} placeholder, which is unreadable. Resolve the UUID against the workspace member list and render the display name instead. When the creator has left the workspace (or the member list hasn't loaded), fall back to the unbranded conflict_locked message rather than leak the UUID. Adds two test cases covering both branches. MUL-2701 Co-authored-by: multica-agent --- .../runtime-local-skill-import-panel.test.tsx | 81 +++++++++++++++++++ .../runtime-local-skill-import-panel.tsx | 12 ++- 2 files changed, 90 insertions(+), 3 deletions(-) diff --git a/packages/views/skills/components/runtime-local-skill-import-panel.test.tsx b/packages/views/skills/components/runtime-local-skill-import-panel.test.tsx index dab605bdf..b6c7c2dbb 100644 --- a/packages/views/skills/components/runtime-local-skill-import-panel.test.tsx +++ b/packages/views/skills/components/runtime-local-skill-import-panel.test.tsx @@ -15,6 +15,13 @@ const TEST_RESOURCES = { const mockResolveRuntimeLocalSkillImport = vi.hoisted(() => vi.fn()); const mockRuntimeListOptions = vi.hoisted(() => vi.fn()); const mockRuntimeLocalSkillsOptions = vi.hoisted(() => vi.fn()); +const mockListMembers = vi.hoisted(() => vi.fn()); + +vi.mock("@multica/core/api", () => ({ + api: { + listMembers: (...args: unknown[]) => mockListMembers(...args), + }, +})); vi.mock("@multica/core/hooks", () => ({ useWorkspaceId: () => "ws-1", @@ -151,6 +158,10 @@ describe("RuntimeLocalSkillImportPanel", () => { status: "created", skill: MOCK_IMPORTED_SKILL_A, }); + mockListMembers.mockResolvedValue([ + { user_id: "user-1", name: "Alice", email: "alice@example.com" }, + { user_id: "user-2", name: "Bob", email: "bob@example.com" }, + ]); }); it("imports a single skill when selected via checkbox", async () => { @@ -528,4 +539,74 @@ describe("RuntimeLocalSkillImportPanel", () => { expect(onBulkDone).toHaveBeenCalledTimes(1); expect(onImported).not.toHaveBeenCalled(); }); + + it("renders the creator's display name for non-overwritable conflicts", async () => { + mockResolveRuntimeLocalSkillImport.mockResolvedValueOnce({ + status: "conflict", + conflict: { + existing_skill_id: "existing-skill-1", + existing_created_by: "user-2", + can_overwrite: false, + }, + }); + + renderPanel(); + + expect( + await screen.findByText("Review Helper", {}, { timeout: 5000 }), + ).toBeInTheDocument(); + + fireEvent.click(screen.getByRole("button", { name: /Review Helper/i })); + const importButton = screen.getByRole("button", { + name: /Import to Workspace/i, + }); + await waitFor(() => expect(importButton).not.toBeDisabled(), { + timeout: 5000, + }); + fireEvent.click(importButton); + + // Bob is user-2 in the mocked member list. The locked message must show + // the resolved name, never the raw UUID. + expect( + await screen.findByText(/created by Bob/i, {}, { timeout: 5000 }), + ).toBeInTheDocument(); + expect(screen.queryByText(/user-2/)).not.toBeInTheDocument(); + }); + + it("falls back to the unbranded locked message when the creator left the workspace", async () => { + mockResolveRuntimeLocalSkillImport.mockResolvedValueOnce({ + status: "conflict", + conflict: { + existing_skill_id: "existing-skill-1", + existing_created_by: "user-gone", + can_overwrite: false, + }, + }); + + renderPanel(); + + expect( + await screen.findByText("Review Helper", {}, { timeout: 5000 }), + ).toBeInTheDocument(); + + fireEvent.click(screen.getByRole("button", { name: /Review Helper/i })); + const importButton = screen.getByRole("button", { + name: /Import to Workspace/i, + }); + await waitFor(() => expect(importButton).not.toBeDisabled(), { + timeout: 5000, + }); + fireEvent.click(importButton); + + // user-gone is not in the workspace; the UI must not leak the UUID and + // should render the no-creator variant of the message. + expect( + await screen.findByText( + /Only the creator can overwrite this skill/i, + {}, + { timeout: 5000 }, + ), + ).toBeInTheDocument(); + expect(screen.queryByText(/user-gone/)).not.toBeInTheDocument(); + }); }); diff --git a/packages/views/skills/components/runtime-local-skill-import-panel.tsx b/packages/views/skills/components/runtime-local-skill-import-panel.tsx index 7a902a349..799b29079 100644 --- a/packages/views/skills/components/runtime-local-skill-import-panel.tsx +++ b/packages/views/skills/components/runtime-local-skill-import-panel.tsx @@ -28,6 +28,7 @@ import { resolveRuntimeLocalSkillImport, } from "@multica/core/runtimes"; import { + memberListOptions, skillDetailOptions, workspaceKeys, } from "@multica/core/workspace/queries"; @@ -316,6 +317,8 @@ function ConflictResolutionPanel({ onSkipAll: () => void; }) { const { t } = useT("skills"); + const wsId = useWorkspaceId(); + const { data: members = [] } = useQuery(memberListOptions(wsId)); const single = conflicts.length === 1; const canOverwriteAny = conflicts.some((r) => r.conflict?.can_overwrite); @@ -366,7 +369,10 @@ function ConflictResolutionPanel({ action: r.conflict?.can_overwrite ? "overwrite" : "rename", renameName: defaultRenameName(r.name), } satisfies ConflictResolutionState); - const creator = r.conflict?.existing_created_by; + const creatorId = r.conflict?.existing_created_by; + const creatorName = creatorId + ? members.find((m) => m.user_id === creatorId)?.name + : undefined; return (
@@ -378,9 +384,9 @@ function ConflictResolutionPanel({ )} {!r.conflict?.can_overwrite && (

- {creator + {creatorName ? t(($) => $.runtime_import.conflict_locked_creator, { - creator, + creator: creatorName, }) : t(($) => $.runtime_import.conflict_locked)}