mirror of
https://github.com/multica-ai/multica.git
synced 2026-06-17 03:38:32 +02:00
Merge branch 'feat/skill-import-conflict-backend' into agent/niko/mul-2802-skill-import-conflict-cli
This commit is contained in:
J
@@ -67,6 +67,16 @@ export async function resolveRuntimeLocalSkillImport(
|
||||
current = await api.getImportLocalSkillResult(runtimeId, initial.id);
|
||||
}
|
||||
|
||||
if (current.status === "conflict") {
|
||||
if (!current.conflict) {
|
||||
throw new Error("runtime local skill import conflict missing details");
|
||||
}
|
||||
return {
|
||||
status: "conflict",
|
||||
conflict: current.conflict,
|
||||
};
|
||||
}
|
||||
|
||||
if (current.status === "failed" || current.status === "timeout") {
|
||||
throw new Error(current.error || "runtime local skill import failed");
|
||||
}
|
||||
@@ -74,7 +84,10 @@ export async function resolveRuntimeLocalSkillImport(
|
||||
throw new Error("runtime local skill import did not return a skill");
|
||||
}
|
||||
|
||||
return { skill: current.skill };
|
||||
return {
|
||||
status: current.action === "overwrite" ? "updated" : "created",
|
||||
skill: current.skill,
|
||||
};
|
||||
}
|
||||
|
||||
export function runtimeLocalSkillsOptions(runtimeId: string | null | undefined) {
|
||||
|
||||
@@ -617,9 +617,18 @@ export type RuntimeLocalSkillStatus =
|
||||
| "pending"
|
||||
| "running"
|
||||
| "completed"
|
||||
| "conflict"
|
||||
| "failed"
|
||||
| "timeout";
|
||||
|
||||
export type RuntimeLocalSkillImportAction = "overwrite";
|
||||
|
||||
export interface RuntimeLocalSkillImportConflict {
|
||||
existing_skill_id: string;
|
||||
existing_created_by?: string;
|
||||
can_overwrite: boolean;
|
||||
}
|
||||
|
||||
export interface RuntimeLocalSkillSummary {
|
||||
key: string;
|
||||
name: string;
|
||||
@@ -644,6 +653,9 @@ export interface CreateRuntimeLocalSkillImportRequest {
|
||||
skill_key: string;
|
||||
name?: string;
|
||||
description?: string;
|
||||
action?: RuntimeLocalSkillImportAction;
|
||||
target_skill_id?: string;
|
||||
supports_conflict?: boolean;
|
||||
}
|
||||
|
||||
export interface RuntimeLocalSkillImportRequest {
|
||||
@@ -652,8 +664,12 @@ export interface RuntimeLocalSkillImportRequest {
|
||||
skill_key: string;
|
||||
name?: string;
|
||||
description?: string;
|
||||
action?: RuntimeLocalSkillImportAction;
|
||||
target_skill_id?: string;
|
||||
supports_conflict?: boolean;
|
||||
status: RuntimeLocalSkillStatus;
|
||||
skill?: Skill;
|
||||
conflict?: RuntimeLocalSkillImportConflict;
|
||||
error?: string;
|
||||
created_at: string;
|
||||
updated_at: string;
|
||||
@@ -665,5 +681,7 @@ export interface RuntimeLocalSkillsResult {
|
||||
}
|
||||
|
||||
export interface RuntimeLocalSkillImportResult {
|
||||
skill: Skill;
|
||||
status: "created" | "updated" | "conflict";
|
||||
skill?: Skill;
|
||||
conflict?: RuntimeLocalSkillImportConflict;
|
||||
}
|
||||
|
||||
@@ -44,6 +44,8 @@ export type {
|
||||
RuntimeModelListStatus,
|
||||
RuntimeModelsResult,
|
||||
RuntimeLocalSkillStatus,
|
||||
RuntimeLocalSkillImportAction,
|
||||
RuntimeLocalSkillImportConflict,
|
||||
RuntimeLocalSkillSummary,
|
||||
RuntimeLocalSkillListRequest,
|
||||
CreateRuntimeLocalSkillImportRequest,
|
||||
|
||||
@@ -231,9 +231,27 @@
|
||||
"bulk_complete_hint": "Import complete.",
|
||||
"bulk_cancelled_hint": "Import cancelled.",
|
||||
"bulk_done_button": "Done",
|
||||
"bulk_summary_imported": "Imported",
|
||||
"bulk_summary_created": "Created",
|
||||
"bulk_summary_updated": "Updated",
|
||||
"bulk_summary_conflicts": "Conflicts",
|
||||
"bulk_summary_skipped": "Skipped",
|
||||
"bulk_summary_failed": "Failed"
|
||||
"bulk_summary_failed": "Failed",
|
||||
"conflict_single_title": "A skill with this name already exists",
|
||||
"conflict_bulk_title": "{{count}} skills need a decision",
|
||||
"conflict_hint": "Choose whether to overwrite the existing workspace skill, import with a new name, or skip it.",
|
||||
"conflict_locked_creator": "This skill was created by {{creator}}. Only the creator can overwrite it from local import; edit it from the Skill detail page if you need to change it.",
|
||||
"conflict_locked": "Only the creator can overwrite this skill from local import; edit it from the Skill detail page if you need to change it.",
|
||||
"conflict_overwrite": "Overwrite",
|
||||
"conflict_rename": "Rename",
|
||||
"conflict_skip": "Skip",
|
||||
"conflict_cancel": "Cancel",
|
||||
"conflict_overwrite_all": "Overwrite all",
|
||||
"conflict_skip_all": "Skip all",
|
||||
"conflict_rename_label": "New skill name",
|
||||
"conflict_footer": "{{count}} conflict decisions pending.",
|
||||
"conflict_apply_button": "Apply decisions",
|
||||
"conflict_missing_resolution": "Choose how to resolve this conflict.",
|
||||
"conflict_name_still_exists": "That name already exists. Choose another name or skip."
|
||||
},
|
||||
"file_tree": {
|
||||
"no_files": "No files"
|
||||
|
||||
@@ -243,9 +243,27 @@
|
||||
"bulk_complete_hint": "导入完成。",
|
||||
"bulk_cancelled_hint": "导入已取消。",
|
||||
"bulk_done_button": "完成",
|
||||
"bulk_summary_imported": "已导入",
|
||||
"bulk_summary_created": "已创建",
|
||||
"bulk_summary_updated": "已更新",
|
||||
"bulk_summary_conflicts": "冲突",
|
||||
"bulk_summary_skipped": "已跳过",
|
||||
"bulk_summary_failed": "失败"
|
||||
"bulk_summary_failed": "失败",
|
||||
"conflict_single_title": "工作区里已存在同名 skill",
|
||||
"conflict_bulk_title": "{{count}} 个 skill 需要处理冲突",
|
||||
"conflict_hint": "选择覆盖现有工作区 skill、改名导入，或跳过。",
|
||||
"conflict_locked_creator": "该 skill 由 {{creator}} 创建，只能由创建者通过本地导入覆盖；如需修改请到 Skill 详情页编辑。",
|
||||
"conflict_locked": "该 skill 只能由创建者通过本地导入覆盖；如需修改请到 Skill 详情页编辑。",
|
||||
"conflict_overwrite": "覆盖",
|
||||
"conflict_rename": "改名",
|
||||
"conflict_skip": "跳过",
|
||||
"conflict_cancel": "取消",
|
||||
"conflict_overwrite_all": "全部覆盖",
|
||||
"conflict_skip_all": "全部跳过",
|
||||
"conflict_rename_label": "新的 skill 名称",
|
||||
"conflict_footer": "还有 {{count}} 个冲突决策待处理。",
|
||||
"conflict_apply_button": "应用决策",
|
||||
"conflict_missing_resolution": "请选择如何处理这个冲突。",
|
||||
"conflict_name_still_exists": "这个名称仍然已存在。请换一个名称或跳过。"
|
||||
},
|
||||
"file_tree": {
|
||||
"no_files": "无文件"
|
||||
|
||||
@@ -148,6 +148,7 @@ describe("RuntimeLocalSkillImportPanel", () => {
|
||||
}),
|
||||
});
|
||||
mockResolveRuntimeLocalSkillImport.mockResolvedValue({
|
||||
status: "created",
|
||||
skill: MOCK_IMPORTED_SKILL_A,
|
||||
});
|
||||
});
|
||||
@@ -183,6 +184,7 @@ describe("RuntimeLocalSkillImportPanel", () => {
|
||||
skill_key: "review-helper",
|
||||
name: "Review Helper",
|
||||
description: "Review pull requests",
|
||||
supports_conflict: true,
|
||||
},
|
||||
);
|
||||
},
|
||||
@@ -200,8 +202,8 @@ describe("RuntimeLocalSkillImportPanel", () => {
|
||||
}),
|
||||
});
|
||||
mockResolveRuntimeLocalSkillImport
|
||||
.mockResolvedValueOnce({ skill: MOCK_IMPORTED_SKILL_A })
|
||||
.mockResolvedValueOnce({ skill: MOCK_IMPORTED_SKILL_B });
|
||||
.mockResolvedValueOnce({ status: "created", skill: MOCK_IMPORTED_SKILL_A })
|
||||
.mockResolvedValueOnce({ status: "created", skill: MOCK_IMPORTED_SKILL_B });
|
||||
|
||||
renderPanel();
|
||||
|
||||
@@ -240,8 +242,8 @@ describe("RuntimeLocalSkillImportPanel", () => {
|
||||
|
||||
expect(mockResolveRuntimeLocalSkillImport).toHaveBeenCalledTimes(2);
|
||||
|
||||
// Verify summary shows both as imported
|
||||
expect(screen.getByText("Imported")).toBeInTheDocument();
|
||||
// Verify summary shows both as created
|
||||
expect(screen.getByText("Created")).toBeInTheDocument();
|
||||
});
|
||||
|
||||
it("handles partial failures gracefully", async () => {
|
||||
@@ -254,7 +256,7 @@ describe("RuntimeLocalSkillImportPanel", () => {
|
||||
}),
|
||||
});
|
||||
mockResolveRuntimeLocalSkillImport
|
||||
.mockResolvedValueOnce({ skill: MOCK_IMPORTED_SKILL_A })
|
||||
.mockResolvedValueOnce({ status: "created", skill: MOCK_IMPORTED_SKILL_A })
|
||||
.mockRejectedValueOnce(new Error("409 conflict: already exists"));
|
||||
|
||||
renderPanel();
|
||||
@@ -291,8 +293,8 @@ describe("RuntimeLocalSkillImportPanel", () => {
|
||||
{ timeout: 10000 },
|
||||
);
|
||||
|
||||
// Summary should show imported and skipped
|
||||
expect(screen.getByText("Imported")).toBeInTheDocument();
|
||||
// Summary should show created and skipped
|
||||
expect(screen.getByText("Created")).toBeInTheDocument();
|
||||
expect(screen.getByText("Skipped")).toBeInTheDocument();
|
||||
});
|
||||
|
||||
@@ -344,8 +346,8 @@ describe("RuntimeLocalSkillImportPanel", () => {
|
||||
}),
|
||||
});
|
||||
mockResolveRuntimeLocalSkillImport
|
||||
.mockResolvedValueOnce({ skill: MOCK_IMPORTED_SKILL_A })
|
||||
.mockResolvedValueOnce({ skill: MOCK_IMPORTED_SKILL_B });
|
||||
.mockResolvedValueOnce({ status: "created", skill: MOCK_IMPORTED_SKILL_A })
|
||||
.mockResolvedValueOnce({ status: "created", skill: MOCK_IMPORTED_SKILL_B });
|
||||
|
||||
const onImported = vi.fn();
|
||||
const onBulkDone = vi.fn();
|
||||
@@ -385,4 +387,145 @@ describe("RuntimeLocalSkillImportPanel", () => {
|
||||
expect(onBulkDone).toHaveBeenCalledTimes(1);
|
||||
expect(onImported).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("resolves a creator conflict by overwriting the existing skill", async () => {
|
||||
mockResolveRuntimeLocalSkillImport
|
||||
.mockResolvedValueOnce({
|
||||
status: "conflict",
|
||||
conflict: {
|
||||
existing_skill_id: "existing-skill-1",
|
||||
existing_created_by: "user-1",
|
||||
can_overwrite: true,
|
||||
},
|
||||
})
|
||||
.mockResolvedValueOnce({
|
||||
status: "updated",
|
||||
skill: {
|
||||
...MOCK_IMPORTED_SKILL_A,
|
||||
id: "existing-skill-1",
|
||||
},
|
||||
});
|
||||
|
||||
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);
|
||||
|
||||
expect(
|
||||
await screen.findByText(/A skill with this name already exists/i),
|
||||
).toBeInTheDocument();
|
||||
|
||||
const applyButton = screen.getByRole("button", {
|
||||
name: /Apply decisions/i,
|
||||
});
|
||||
await waitFor(() => expect(applyButton).not.toBeDisabled(), {
|
||||
timeout: 5000,
|
||||
});
|
||||
fireEvent.click(applyButton);
|
||||
|
||||
await waitFor(
|
||||
() => {
|
||||
expect(mockResolveRuntimeLocalSkillImport).toHaveBeenLastCalledWith(
|
||||
"runtime-1",
|
||||
{
|
||||
skill_key: "review-helper",
|
||||
name: "Review Helper",
|
||||
description: "Review pull requests",
|
||||
supports_conflict: true,
|
||||
action: "overwrite",
|
||||
target_skill_id: "existing-skill-1",
|
||||
},
|
||||
);
|
||||
},
|
||||
{ timeout: 5000 },
|
||||
);
|
||||
|
||||
expect(await screen.findByText("Updated")).toBeInTheDocument();
|
||||
});
|
||||
|
||||
it("keeps bulk completion behavior when conflict resolution leaves one success", async () => {
|
||||
mockRuntimeLocalSkillsOptions.mockReturnValue({
|
||||
queryKey: ["runtimes", "local-skills", "runtime-1"],
|
||||
queryFn: () =>
|
||||
Promise.resolve({
|
||||
supported: true,
|
||||
skills: [MOCK_SKILL_A, MOCK_SKILL_B],
|
||||
}),
|
||||
});
|
||||
mockResolveRuntimeLocalSkillImport
|
||||
.mockResolvedValueOnce({
|
||||
status: "conflict",
|
||||
conflict: {
|
||||
existing_skill_id: "existing-skill-1",
|
||||
existing_created_by: "user-1",
|
||||
can_overwrite: true,
|
||||
},
|
||||
})
|
||||
.mockRejectedValueOnce(new Error("daemon failed"))
|
||||
.mockResolvedValueOnce({
|
||||
status: "updated",
|
||||
skill: {
|
||||
...MOCK_IMPORTED_SKILL_A,
|
||||
id: "existing-skill-1",
|
||||
},
|
||||
});
|
||||
|
||||
const onImported = vi.fn();
|
||||
const onBulkDone = vi.fn();
|
||||
renderPanel({ onImported, onBulkDone });
|
||||
|
||||
expect(
|
||||
await screen.findByText("Review Helper", {}, { timeout: 5000 }),
|
||||
).toBeInTheDocument();
|
||||
|
||||
const selectAllLabel = screen.getByText(/Select all/i);
|
||||
const selectAllCheckbox = selectAllLabel
|
||||
.closest("label")!
|
||||
.querySelector("input[type='checkbox']")!;
|
||||
fireEvent.click(selectAllCheckbox);
|
||||
|
||||
const importButton = screen.getByRole("button", {
|
||||
name: /Import 2 Skills/i,
|
||||
});
|
||||
await waitFor(() => expect(importButton).not.toBeDisabled(), {
|
||||
timeout: 5000,
|
||||
});
|
||||
fireEvent.click(importButton);
|
||||
|
||||
expect(
|
||||
await screen.findByText(/A skill with this name already exists/i),
|
||||
).toBeInTheDocument();
|
||||
|
||||
const applyButton = screen.getByRole("button", {
|
||||
name: /Apply decisions/i,
|
||||
});
|
||||
await waitFor(() => expect(applyButton).not.toBeDisabled(), {
|
||||
timeout: 5000,
|
||||
});
|
||||
fireEvent.click(applyButton);
|
||||
|
||||
await waitFor(
|
||||
() => {
|
||||
expect(
|
||||
screen.getByRole("button", { name: /Done/i }),
|
||||
).toBeInTheDocument();
|
||||
},
|
||||
{ timeout: 10000 },
|
||||
);
|
||||
|
||||
fireEvent.click(screen.getByRole("button", { name: /Done/i }));
|
||||
expect(onBulkDone).toHaveBeenCalledTimes(1);
|
||||
expect(onImported).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
@@ -4,14 +4,18 @@ import { useEffect, useMemo, useRef, useState } from "react";
|
||||
import { useQuery, useQueryClient } from "@tanstack/react-query";
|
||||
import {
|
||||
AlertCircle,
|
||||
AlertTriangle,
|
||||
CheckCircle2,
|
||||
Download,
|
||||
HardDrive,
|
||||
Loader2,
|
||||
RefreshCw,
|
||||
SkipForward,
|
||||
XCircle,
|
||||
} from "lucide-react";
|
||||
import type {
|
||||
AgentRuntime,
|
||||
RuntimeLocalSkillImportConflict,
|
||||
RuntimeLocalSkillSummary,
|
||||
Skill,
|
||||
} from "@multica/core/types";
|
||||
@@ -53,22 +57,33 @@ import { isNameConflictError } from "../lib/utils";
|
||||
type BulkImportResult = {
|
||||
key: string;
|
||||
name: string;
|
||||
status: "success" | "skipped" | "failed";
|
||||
description?: string;
|
||||
status: "created" | "updated" | "conflict" | "skipped" | "failed";
|
||||
conflict?: RuntimeLocalSkillImportConflict;
|
||||
error?: string;
|
||||
skill?: Skill;
|
||||
};
|
||||
|
||||
type BulkImportState = {
|
||||
phase: "idle" | "importing" | "done" | "cancelled";
|
||||
phase: "idle" | "importing" | "resolving" | "done" | "cancelled";
|
||||
total: number;
|
||||
completed: number;
|
||||
selectedCount: number;
|
||||
results: BulkImportResult[];
|
||||
};
|
||||
|
||||
type ConflictResolutionAction = "overwrite" | "rename" | "skip";
|
||||
|
||||
type ConflictResolutionState = {
|
||||
action: ConflictResolutionAction;
|
||||
renameName: string;
|
||||
};
|
||||
|
||||
const INITIAL_BULK_STATE: BulkImportState = {
|
||||
phase: "idle",
|
||||
total: 0,
|
||||
completed: 0,
|
||||
selectedCount: 0,
|
||||
results: [],
|
||||
};
|
||||
|
||||
@@ -90,6 +105,25 @@ function runtimeLabel(runtime: AgentRuntime): string {
|
||||
return `${runtime.name} (${runtime.provider})`;
|
||||
}
|
||||
|
||||
function defaultRenameName(name: string): string {
|
||||
return `${name} copy`;
|
||||
}
|
||||
|
||||
function ResultIcon({ status }: { status: BulkImportResult["status"] }) {
|
||||
switch (status) {
|
||||
case "created":
|
||||
return <CheckCircle2 className="h-3.5 w-3.5 shrink-0 text-green-600" />;
|
||||
case "updated":
|
||||
return <RefreshCw className="h-3.5 w-3.5 shrink-0 text-blue-600" />;
|
||||
case "conflict":
|
||||
return <AlertTriangle className="h-3.5 w-3.5 shrink-0 text-amber-600" />;
|
||||
case "skipped":
|
||||
return <SkipForward className="h-3.5 w-3.5 shrink-0 text-yellow-600" />;
|
||||
case "failed":
|
||||
return <AlertCircle className="h-3.5 w-3.5 shrink-0 text-destructive" />;
|
||||
}
|
||||
}
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// Skill row with checkbox
|
||||
// ---------------------------------------------------------------------------
|
||||
@@ -195,20 +229,38 @@ function SkillItem({
|
||||
|
||||
function BulkImportSummary({ results }: { results: BulkImportResult[] }) {
|
||||
const { t } = useT("skills");
|
||||
const succeeded = results.filter((r) => r.status === "success");
|
||||
const created = results.filter((r) => r.status === "created");
|
||||
const updated = results.filter((r) => r.status === "updated");
|
||||
const conflicts = results.filter((r) => r.status === "conflict");
|
||||
const skipped = results.filter((r) => r.status === "skipped");
|
||||
const failed = results.filter((r) => r.status === "failed");
|
||||
|
||||
return (
|
||||
<div className="space-y-4 py-2">
|
||||
{/* Summary counts */}
|
||||
<div className="grid grid-cols-3 gap-2 text-center">
|
||||
<div className="grid grid-cols-2 gap-2 text-center sm:grid-cols-5">
|
||||
<div className="rounded-md bg-green-50 px-3 py-2 dark:bg-green-950/30">
|
||||
<div className="text-lg font-semibold text-green-700 dark:text-green-400">
|
||||
{succeeded.length}
|
||||
{created.length}
|
||||
</div>
|
||||
<div className="text-xs text-muted-foreground">
|
||||
{t(($) => $.runtime_import.bulk_summary_imported)}
|
||||
{t(($) => $.runtime_import.bulk_summary_created)}
|
||||
</div>
|
||||
</div>
|
||||
<div className="rounded-md bg-blue-50 px-3 py-2 dark:bg-blue-950/30">
|
||||
<div className="text-lg font-semibold text-blue-700 dark:text-blue-400">
|
||||
{updated.length}
|
||||
</div>
|
||||
<div className="text-xs text-muted-foreground">
|
||||
{t(($) => $.runtime_import.bulk_summary_updated)}
|
||||
</div>
|
||||
</div>
|
||||
<div className="rounded-md bg-amber-50 px-3 py-2 dark:bg-amber-950/30">
|
||||
<div className="text-lg font-semibold text-amber-700 dark:text-amber-400">
|
||||
{conflicts.length}
|
||||
</div>
|
||||
<div className="text-xs text-muted-foreground">
|
||||
{t(($) => $.runtime_import.bulk_summary_conflicts)}
|
||||
</div>
|
||||
</div>
|
||||
<div className="rounded-md bg-yellow-50 px-3 py-2 dark:bg-yellow-950/30">
|
||||
@@ -236,15 +288,7 @@ function BulkImportSummary({ results }: { results: BulkImportResult[] }) {
|
||||
key={r.key}
|
||||
className="flex items-center gap-2 rounded px-2 py-1.5 text-xs"
|
||||
>
|
||||
{r.status === "success" && (
|
||||
<CheckCircle2 className="h-3.5 w-3.5 shrink-0 text-green-600" />
|
||||
)}
|
||||
{r.status === "skipped" && (
|
||||
<SkipForward className="h-3.5 w-3.5 shrink-0 text-yellow-600" />
|
||||
)}
|
||||
{r.status === "failed" && (
|
||||
<AlertCircle className="h-3.5 w-3.5 shrink-0 text-destructive" />
|
||||
)}
|
||||
<ResultIcon status={r.status} />
|
||||
<span className="min-w-0 flex-1 truncate">{r.name}</span>
|
||||
{r.error && (
|
||||
<span className="max-w-[200px] shrink-0 truncate text-muted-foreground">
|
||||
@@ -258,6 +302,166 @@ function BulkImportSummary({ results }: { results: BulkImportResult[] }) {
|
||||
);
|
||||
}
|
||||
|
||||
function ConflictResolutionPanel({
|
||||
conflicts,
|
||||
resolutions,
|
||||
onChange,
|
||||
onOverwriteAll,
|
||||
onSkipAll,
|
||||
}: {
|
||||
conflicts: BulkImportResult[];
|
||||
resolutions: Record<string, ConflictResolutionState>;
|
||||
onChange: (key: string, next: ConflictResolutionState) => void;
|
||||
onOverwriteAll: () => void;
|
||||
onSkipAll: () => void;
|
||||
}) {
|
||||
const { t } = useT("skills");
|
||||
const single = conflicts.length === 1;
|
||||
const canOverwriteAny = conflicts.some((r) => r.conflict?.can_overwrite);
|
||||
|
||||
return (
|
||||
<div className="space-y-4 py-2">
|
||||
<div className="rounded-md border border-amber-200 bg-amber-50 px-3 py-2 text-sm text-amber-950 dark:border-amber-900/60 dark:bg-amber-950/30 dark:text-amber-100">
|
||||
<div className="flex items-start gap-2">
|
||||
<AlertTriangle className="mt-0.5 h-4 w-4 shrink-0" />
|
||||
<div className="min-w-0">
|
||||
<p className="font-medium">
|
||||
{single
|
||||
? t(($) => $.runtime_import.conflict_single_title)
|
||||
: t(($) => $.runtime_import.conflict_bulk_title, {
|
||||
count: conflicts.length,
|
||||
})}
|
||||
</p>
|
||||
<p className="mt-1 text-xs opacity-85">
|
||||
{t(($) => $.runtime_import.conflict_hint)}
|
||||
</p>
|
||||
</div>
|
||||
</div>
|
||||
</div>
|
||||
|
||||
{!single && (
|
||||
<div className="flex flex-wrap gap-2">
|
||||
<Button
|
||||
type="button"
|
||||
size="sm"
|
||||
variant="outline"
|
||||
onClick={onOverwriteAll}
|
||||
disabled={!canOverwriteAny}
|
||||
>
|
||||
<RefreshCw className="h-3 w-3" />
|
||||
{t(($) => $.runtime_import.conflict_overwrite_all)}
|
||||
</Button>
|
||||
<Button type="button" size="sm" variant="outline" onClick={onSkipAll}>
|
||||
<SkipForward className="h-3 w-3" />
|
||||
{t(($) => $.runtime_import.conflict_skip_all)}
|
||||
</Button>
|
||||
</div>
|
||||
)}
|
||||
|
||||
<div className="space-y-2">
|
||||
{conflicts.map((r) => {
|
||||
const resolution =
|
||||
resolutions[r.key] ??
|
||||
({
|
||||
action: r.conflict?.can_overwrite ? "overwrite" : "rename",
|
||||
renameName: defaultRenameName(r.name),
|
||||
} satisfies ConflictResolutionState);
|
||||
const creator = r.conflict?.existing_created_by;
|
||||
return (
|
||||
<div key={r.key} className="rounded-lg border bg-card p-3">
|
||||
<div className="flex items-start gap-2">
|
||||
<AlertTriangle className="mt-0.5 h-4 w-4 shrink-0 text-amber-600" />
|
||||
<div className="min-w-0 flex-1">
|
||||
<div className="truncate text-sm font-medium">{r.name}</div>
|
||||
{r.error && (
|
||||
<p className="mt-1 text-xs text-destructive">{r.error}</p>
|
||||
)}
|
||||
{!r.conflict?.can_overwrite && (
|
||||
<p className="mt-1 text-xs text-muted-foreground">
|
||||
{creator
|
||||
? t(($) => $.runtime_import.conflict_locked_creator, {
|
||||
creator,
|
||||
})
|
||||
: t(($) => $.runtime_import.conflict_locked)}
|
||||
</p>
|
||||
)}
|
||||
</div>
|
||||
</div>
|
||||
|
||||
<div className="mt-3 flex flex-wrap gap-2">
|
||||
<Button
|
||||
type="button"
|
||||
size="sm"
|
||||
variant={
|
||||
resolution.action === "overwrite" ? "default" : "outline"
|
||||
}
|
||||
onClick={() =>
|
||||
onChange(r.key, {
|
||||
action: "overwrite",
|
||||
renameName: resolution.renameName,
|
||||
})
|
||||
}
|
||||
disabled={!r.conflict?.can_overwrite}
|
||||
>
|
||||
<RefreshCw className="h-3 w-3" />
|
||||
{t(($) => $.runtime_import.conflict_overwrite)}
|
||||
</Button>
|
||||
<Button
|
||||
type="button"
|
||||
size="sm"
|
||||
variant={resolution.action === "rename" ? "default" : "outline"}
|
||||
onClick={() =>
|
||||
onChange(r.key, {
|
||||
action: "rename",
|
||||
renameName: resolution.renameName,
|
||||
})
|
||||
}
|
||||
>
|
||||
{t(($) => $.runtime_import.conflict_rename)}
|
||||
</Button>
|
||||
<Button
|
||||
type="button"
|
||||
size="sm"
|
||||
variant={resolution.action === "skip" ? "default" : "outline"}
|
||||
onClick={() =>
|
||||
onChange(r.key, {
|
||||
action: "skip",
|
||||
renameName: resolution.renameName,
|
||||
})
|
||||
}
|
||||
>
|
||||
<XCircle className="h-3 w-3" />
|
||||
{single
|
||||
? t(($) => $.runtime_import.conflict_cancel)
|
||||
: t(($) => $.runtime_import.conflict_skip)}
|
||||
</Button>
|
||||
</div>
|
||||
|
||||
{resolution.action === "rename" && (
|
||||
<div className="mt-3 space-y-1">
|
||||
<Label className="text-xs text-muted-foreground">
|
||||
{t(($) => $.runtime_import.conflict_rename_label)}
|
||||
</Label>
|
||||
<Input
|
||||
value={resolution.renameName}
|
||||
onChange={(e) =>
|
||||
onChange(r.key, {
|
||||
action: "rename",
|
||||
renameName: e.target.value,
|
||||
})
|
||||
}
|
||||
className="h-8 text-sm"
|
||||
/>
|
||||
</div>
|
||||
)}
|
||||
</div>
|
||||
);
|
||||
})}
|
||||
</div>
|
||||
</div>
|
||||
);
|
||||
}
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// Panel
|
||||
// ---------------------------------------------------------------------------
|
||||
@@ -288,12 +492,17 @@ export function RuntimeLocalSkillImportPanel({
|
||||
const [selectedRuntimeId, setSelectedRuntimeId] = useState<string>("");
|
||||
const [selectedKeys, setSelectedKeys] = useState<Set<string>>(new Set());
|
||||
const [bulkState, setBulkState] = useState<BulkImportState>(INITIAL_BULK_STATE);
|
||||
const [conflictResolutions, setConflictResolutions] = useState<
|
||||
Record<string, ConflictResolutionState>
|
||||
>({});
|
||||
const cancelRef = useRef(false);
|
||||
// Single-select inline edit fields (shown when exactly 1 skill is checked)
|
||||
const [editName, setEditName] = useState("");
|
||||
const [editDescription, setEditDescription] = useState("");
|
||||
|
||||
const importing = bulkState.phase === "importing";
|
||||
const resolvingConflicts = bulkState.phase === "resolving";
|
||||
const busy = importing || resolvingConflicts;
|
||||
|
||||
useEffect(() => {
|
||||
setSelectedRuntimeId((prev) => prev || localRuntimes[0]?.id || "");
|
||||
@@ -302,6 +511,7 @@ export function RuntimeLocalSkillImportPanel({
|
||||
useEffect(() => {
|
||||
setSelectedKeys(new Set());
|
||||
setBulkState(INITIAL_BULK_STATE);
|
||||
setConflictResolutions({});
|
||||
setEditName("");
|
||||
setEditDescription("");
|
||||
}, [selectedRuntimeId]);
|
||||
@@ -354,6 +564,56 @@ export function RuntimeLocalSkillImportPanel({
|
||||
const allSelected =
|
||||
runtimeSkills.length > 0 && selectedKeys.size === runtimeSkills.length;
|
||||
const someSelected = selectedKeys.size > 0 && !allSelected;
|
||||
const pendingConflicts = bulkState.results.filter(
|
||||
(r) => r.status === "conflict" && r.conflict,
|
||||
);
|
||||
const canApplyConflictResolutions =
|
||||
pendingConflicts.length > 0 &&
|
||||
pendingConflicts.every((r) => {
|
||||
const resolution = conflictResolutions[r.key];
|
||||
if (!resolution) return false;
|
||||
if (resolution.action === "overwrite") return !!r.conflict?.can_overwrite;
|
||||
if (resolution.action === "rename") return !!resolution.renameName.trim();
|
||||
return true;
|
||||
});
|
||||
|
||||
const setConflictResolution = (
|
||||
key: string,
|
||||
next: ConflictResolutionState,
|
||||
) => {
|
||||
setConflictResolutions((prev) => ({ ...prev, [key]: next }));
|
||||
};
|
||||
|
||||
const seedConflictResolutions = (results: BulkImportResult[]) => {
|
||||
const next: Record<string, ConflictResolutionState> = {};
|
||||
for (const r of results) {
|
||||
if (r.status !== "conflict" || !r.conflict) continue;
|
||||
next[r.key] = {
|
||||
action: r.conflict.can_overwrite ? "overwrite" : "rename",
|
||||
renameName: defaultRenameName(r.name),
|
||||
};
|
||||
}
|
||||
setConflictResolutions(next);
|
||||
};
|
||||
|
||||
const refreshImportedSkills = async (results: BulkImportResult[]) => {
|
||||
await Promise.all([
|
||||
qc.invalidateQueries({
|
||||
queryKey: runtimeLocalSkillsKeys.forRuntime(selectedRuntimeId),
|
||||
}),
|
||||
qc.invalidateQueries({ queryKey: workspaceKeys.skills(wsId) }),
|
||||
qc.invalidateQueries({ queryKey: workspaceKeys.agents(wsId) }),
|
||||
]);
|
||||
|
||||
for (const r of results) {
|
||||
if ((r.status === "created" || r.status === "updated") && r.skill) {
|
||||
qc.setQueryData(
|
||||
skillDetailOptions(wsId, r.skill.id).queryKey,
|
||||
r.skill,
|
||||
);
|
||||
}
|
||||
}
|
||||
};
|
||||
|
||||
// -- Bulk import handler --
|
||||
|
||||
@@ -364,7 +624,13 @@ export function RuntimeLocalSkillImportPanel({
|
||||
const total = skillsToImport.length;
|
||||
|
||||
cancelRef.current = false;
|
||||
setBulkState({ phase: "importing", total, completed: 0, results: [] });
|
||||
setBulkState({
|
||||
phase: "importing",
|
||||
total,
|
||||
completed: 0,
|
||||
selectedCount: total,
|
||||
results: [],
|
||||
});
|
||||
|
||||
const results: BulkImportResult[] = [];
|
||||
|
||||
@@ -381,19 +647,32 @@ export function RuntimeLocalSkillImportPanel({
|
||||
skill_key: skill.key,
|
||||
name: importName,
|
||||
description: importDescription,
|
||||
supports_conflict: true,
|
||||
});
|
||||
results.push({
|
||||
key: skill.key,
|
||||
name: importName,
|
||||
status: "success",
|
||||
skill: result.skill,
|
||||
});
|
||||
if (result.status === "conflict") {
|
||||
results.push({
|
||||
key: skill.key,
|
||||
name: importName,
|
||||
description: importDescription,
|
||||
status: "conflict",
|
||||
conflict: result.conflict,
|
||||
});
|
||||
} else {
|
||||
results.push({
|
||||
key: skill.key,
|
||||
name: result.skill?.name ?? importName,
|
||||
description: importDescription,
|
||||
status: result.status,
|
||||
skill: result.skill,
|
||||
});
|
||||
}
|
||||
} catch (error) {
|
||||
const msg = error instanceof Error ? error.message : "";
|
||||
const isSkipped = isNameConflictError(msg);
|
||||
results.push({
|
||||
key: skill.key,
|
||||
name: skill.name,
|
||||
description: importDescription,
|
||||
status: isSkipped ? "skipped" : "failed",
|
||||
error: msg || t(($) => $.runtime_import.toast_import_failed),
|
||||
});
|
||||
@@ -420,23 +699,16 @@ export function RuntimeLocalSkillImportPanel({
|
||||
}
|
||||
await Promise.all(executing);
|
||||
|
||||
// Invalidate queries ONCE at the end
|
||||
await Promise.all([
|
||||
qc.invalidateQueries({
|
||||
queryKey: runtimeLocalSkillsKeys.forRuntime(selectedRuntimeId),
|
||||
}),
|
||||
qc.invalidateQueries({ queryKey: workspaceKeys.skills(wsId) }),
|
||||
qc.invalidateQueries({ queryKey: workspaceKeys.agents(wsId) }),
|
||||
]);
|
||||
await refreshImportedSkills(results);
|
||||
|
||||
// Seed query cache for each successfully imported skill
|
||||
for (const r of results) {
|
||||
if (r.status === "success" && r.skill) {
|
||||
qc.setQueryData(
|
||||
skillDetailOptions(wsId, r.skill.id).queryKey,
|
||||
r.skill,
|
||||
);
|
||||
}
|
||||
const conflicts = results.filter((r) => r.status === "conflict");
|
||||
if (!cancelRef.current && conflicts.length > 0) {
|
||||
seedConflictResolutions(results);
|
||||
setBulkState((prev) => ({
|
||||
...prev,
|
||||
phase: "resolving",
|
||||
}));
|
||||
return;
|
||||
}
|
||||
|
||||
setBulkState((prev) => ({
|
||||
@@ -445,13 +717,120 @@ export function RuntimeLocalSkillImportPanel({
|
||||
}));
|
||||
};
|
||||
|
||||
const handleApplyConflictResolutions = async () => {
|
||||
if (!selectedRuntimeId || pendingConflicts.length === 0) return;
|
||||
|
||||
const conflicts = [...pendingConflicts];
|
||||
let nextResults = bulkState.results;
|
||||
const applyResult = (key: string, next: Partial<BulkImportResult>) => {
|
||||
nextResults = nextResults.map((r) =>
|
||||
r.key === key ? { ...r, ...next } : r,
|
||||
);
|
||||
setBulkState((prev) => ({
|
||||
...prev,
|
||||
results: prev.results.map((r) =>
|
||||
r.key === key ? { ...r, ...next } : r,
|
||||
),
|
||||
}));
|
||||
};
|
||||
|
||||
setBulkState((prev) => ({
|
||||
...prev,
|
||||
phase: "importing",
|
||||
total: conflicts.length,
|
||||
completed: 0,
|
||||
}));
|
||||
|
||||
for (const r of conflicts) {
|
||||
const resolution = conflictResolutions[r.key];
|
||||
if (!resolution) {
|
||||
applyResult(r.key, {
|
||||
status: "failed",
|
||||
error: t(($) => $.runtime_import.conflict_missing_resolution),
|
||||
});
|
||||
setBulkState((prev) => ({ ...prev, completed: prev.completed + 1 }));
|
||||
continue;
|
||||
}
|
||||
|
||||
if (resolution.action === "skip") {
|
||||
applyResult(r.key, { status: "skipped", error: undefined });
|
||||
setBulkState((prev) => ({ ...prev, completed: prev.completed + 1 }));
|
||||
continue;
|
||||
}
|
||||
|
||||
try {
|
||||
const result = await resolveRuntimeLocalSkillImport(selectedRuntimeId, {
|
||||
skill_key: r.key,
|
||||
name:
|
||||
resolution.action === "rename"
|
||||
? resolution.renameName.trim()
|
||||
: r.name,
|
||||
description: r.description,
|
||||
supports_conflict: true,
|
||||
...(resolution.action === "overwrite" && r.conflict
|
||||
? {
|
||||
action: "overwrite" as const,
|
||||
target_skill_id: r.conflict.existing_skill_id,
|
||||
}
|
||||
: {}),
|
||||
});
|
||||
|
||||
if (result.status === "conflict") {
|
||||
applyResult(r.key, {
|
||||
name:
|
||||
resolution.action === "rename"
|
||||
? resolution.renameName.trim()
|
||||
: r.name,
|
||||
status: "conflict",
|
||||
conflict: result.conflict,
|
||||
error: t(($) => $.runtime_import.conflict_name_still_exists),
|
||||
});
|
||||
if (result.conflict) {
|
||||
setConflictResolution(r.key, {
|
||||
action: result.conflict.can_overwrite ? "overwrite" : "rename",
|
||||
renameName: defaultRenameName(
|
||||
resolution.action === "rename"
|
||||
? resolution.renameName.trim()
|
||||
: r.name,
|
||||
),
|
||||
});
|
||||
}
|
||||
} else {
|
||||
applyResult(r.key, {
|
||||
name: result.skill?.name ?? r.name,
|
||||
status: result.status,
|
||||
skill: result.skill,
|
||||
conflict: undefined,
|
||||
error: undefined,
|
||||
});
|
||||
}
|
||||
} catch (error) {
|
||||
const msg = error instanceof Error ? error.message : "";
|
||||
applyResult(r.key, {
|
||||
status: "failed",
|
||||
error: msg || t(($) => $.runtime_import.toast_import_failed),
|
||||
});
|
||||
}
|
||||
|
||||
setBulkState((prev) => ({ ...prev, completed: prev.completed + 1 }));
|
||||
}
|
||||
|
||||
await refreshImportedSkills(nextResults);
|
||||
const unresolved = nextResults.some((r) => r.status === "conflict");
|
||||
setBulkState((prev) => ({
|
||||
...prev,
|
||||
results: nextResults,
|
||||
phase: unresolved ? "resolving" : "done",
|
||||
}));
|
||||
};
|
||||
|
||||
const canImport =
|
||||
!!selectedRuntime &&
|
||||
selectedRuntime.status === "online" &&
|
||||
selectedKeys.size > 0 &&
|
||||
// Single-select requires a non-empty name (user may be renaming)
|
||||
(selectedKeys.size > 1 || !!editName.trim()) &&
|
||||
!importing;
|
||||
!busy;
|
||||
|
||||
const handleCancel = () => {
|
||||
cancelRef.current = true;
|
||||
@@ -488,15 +867,7 @@ export function RuntimeLocalSkillImportPanel({
|
||||
key={r.key}
|
||||
className="flex items-center gap-2 rounded px-2 py-1 text-xs"
|
||||
>
|
||||
{r.status === "success" && (
|
||||
<CheckCircle2 className="h-3.5 w-3.5 shrink-0 text-green-600" />
|
||||
)}
|
||||
{r.status === "skipped" && (
|
||||
<SkipForward className="h-3.5 w-3.5 shrink-0 text-yellow-600" />
|
||||
)}
|
||||
{r.status === "failed" && (
|
||||
<AlertCircle className="h-3.5 w-3.5 shrink-0 text-destructive" />
|
||||
)}
|
||||
<ResultIcon status={r.status} />
|
||||
<span className="truncate">{r.name}</span>
|
||||
</div>
|
||||
))}
|
||||
@@ -510,6 +881,41 @@ export function RuntimeLocalSkillImportPanel({
|
||||
return <BulkImportSummary results={bulkState.results} />;
|
||||
}
|
||||
|
||||
if (bulkState.phase === "resolving") {
|
||||
return (
|
||||
<ConflictResolutionPanel
|
||||
conflicts={pendingConflicts}
|
||||
resolutions={conflictResolutions}
|
||||
onChange={setConflictResolution}
|
||||
onOverwriteAll={() => {
|
||||
setConflictResolutions((prev) => {
|
||||
const next = { ...prev };
|
||||
for (const r of pendingConflicts) {
|
||||
if (!r.conflict?.can_overwrite) continue;
|
||||
next[r.key] = {
|
||||
action: "overwrite",
|
||||
renameName: prev[r.key]?.renameName ?? defaultRenameName(r.name),
|
||||
};
|
||||
}
|
||||
return next;
|
||||
});
|
||||
}}
|
||||
onSkipAll={() => {
|
||||
setConflictResolutions((prev) => {
|
||||
const next = { ...prev };
|
||||
for (const r of pendingConflicts) {
|
||||
next[r.key] = {
|
||||
action: "skip",
|
||||
renameName: prev[r.key]?.renameName ?? defaultRenameName(r.name),
|
||||
};
|
||||
}
|
||||
return next;
|
||||
});
|
||||
}}
|
||||
/>
|
||||
);
|
||||
}
|
||||
|
||||
// --- Idle phase: skill selection list ---
|
||||
|
||||
if (localRuntimes.length === 0) {
|
||||
@@ -626,10 +1032,16 @@ export function RuntimeLocalSkillImportPanel({
|
||||
// -- Handle "Done" button after import --
|
||||
|
||||
const handleDone = () => {
|
||||
const succeeded = bulkState.results.filter((r) => r.status === "success");
|
||||
const succeeded = bulkState.results.filter(
|
||||
(r) => r.status === "created" || r.status === "updated",
|
||||
);
|
||||
// Single-import flow: navigate to the imported skill detail page.
|
||||
// Multi-import flow: close the dialog even if only one succeeded.
|
||||
if (bulkState.total === 1 && succeeded.length === 1 && succeeded[0]!.skill) {
|
||||
if (
|
||||
bulkState.selectedCount === 1 &&
|
||||
succeeded.length === 1 &&
|
||||
succeeded[0]!.skill
|
||||
) {
|
||||
onImported?.(succeeded[0]!.skill);
|
||||
} else {
|
||||
onBulkDone?.();
|
||||
@@ -640,9 +1052,9 @@ export function RuntimeLocalSkillImportPanel({
|
||||
<div className="flex min-h-0 flex-1 flex-col">
|
||||
{/* Sticky top: runtime picker + status */}
|
||||
<div
|
||||
aria-disabled={importing || undefined}
|
||||
aria-disabled={busy || undefined}
|
||||
className={`shrink-0 space-y-2 border-b px-5 py-3 ${
|
||||
importing ? "pointer-events-none opacity-60" : ""
|
||||
busy ? "pointer-events-none opacity-60" : ""
|
||||
}`}
|
||||
>
|
||||
<div className="space-y-1.5">
|
||||
@@ -691,9 +1103,7 @@ export function RuntimeLocalSkillImportPanel({
|
||||
style={fadeStyle}
|
||||
aria-disabled={importing || undefined}
|
||||
className={`min-h-0 flex-1 overflow-y-auto px-5 py-3 ${
|
||||
importing && bulkState.phase !== "importing"
|
||||
? "pointer-events-none opacity-60"
|
||||
: ""
|
||||
importing && bulkState.phase !== "importing" ? "pointer-events-none opacity-60" : ""
|
||||
}`}
|
||||
>
|
||||
{middle}
|
||||
@@ -717,6 +1127,22 @@ export function RuntimeLocalSkillImportPanel({
|
||||
{t(($) => $.runtime_import.bulk_done_button)}
|
||||
</Button>
|
||||
</>
|
||||
) : resolvingConflicts ? (
|
||||
<>
|
||||
<div className="min-w-0 flex-1 text-xs text-muted-foreground">
|
||||
{t(($) => $.runtime_import.conflict_footer, {
|
||||
count: pendingConflicts.length,
|
||||
})}
|
||||
</div>
|
||||
<Button
|
||||
type="button"
|
||||
size="sm"
|
||||
onClick={handleApplyConflictResolutions}
|
||||
disabled={!canApplyConflictResolutions}
|
||||
>
|
||||
{t(($) => $.runtime_import.conflict_apply_button)}
|
||||
</Button>
|
||||
</>
|
||||
) : importing ? (
|
||||
<>
|
||||
<div className="min-w-0 flex-1 text-xs text-muted-foreground">
|
||||
|
||||
@@ -3,6 +3,7 @@ package handler
|
||||
import (
|
||||
"context"
|
||||
"encoding/json"
|
||||
"errors"
|
||||
"log/slog"
|
||||
"net/http"
|
||||
"sort"
|
||||
@@ -11,6 +12,8 @@ import (
|
||||
"time"
|
||||
|
||||
"github.com/go-chi/chi/v5"
|
||||
"github.com/jackc/pgx/v5"
|
||||
"github.com/jackc/pgx/v5/pgtype"
|
||||
"github.com/multica-ai/multica/server/internal/util"
|
||||
db "github.com/multica-ai/multica/server/pkg/db/generated"
|
||||
"github.com/multica-ai/multica/server/pkg/protocol"
|
||||
@@ -24,8 +27,35 @@ const (
|
||||
RuntimeLocalSkillCompleted RuntimeLocalSkillRequestStatus = "completed"
|
||||
RuntimeLocalSkillFailed RuntimeLocalSkillRequestStatus = "failed"
|
||||
RuntimeLocalSkillTimeout RuntimeLocalSkillRequestStatus = "timeout"
|
||||
// RuntimeLocalSkillConflict is a terminal state set when a fresh import
|
||||
// hits an existing same-name skill. It is not an error: the request carries
|
||||
// structured Conflict metadata so the caller (Desktop UI / CLI) can offer
|
||||
// overwrite / rename / skip instead of silently failing. See MUL-2800.
|
||||
RuntimeLocalSkillConflict RuntimeLocalSkillRequestStatus = "conflict"
|
||||
)
|
||||
|
||||
// LocalSkillImportAction selects how a runtime-local-skill import resolves when
|
||||
// a skill with the same name already exists in the workspace.
|
||||
type LocalSkillImportAction string
|
||||
|
||||
const (
|
||||
// LocalSkillImportActionCreate is the default: create a new skill, and
|
||||
// surface a structured `conflict` if the name is already taken.
|
||||
LocalSkillImportActionCreate LocalSkillImportAction = ""
|
||||
// LocalSkillImportActionOverwrite re-imports onto an existing skill,
|
||||
// identified by TargetSkillID. Only the skill's creator may overwrite.
|
||||
LocalSkillImportActionOverwrite LocalSkillImportAction = "overwrite"
|
||||
)
|
||||
|
||||
// LocalSkillImportConflict is the structured result attached to a request that
|
||||
// terminated in RuntimeLocalSkillConflict. CanOverwrite reflects the
|
||||
// creator-only re-import policy (canOverwriteSkillByLocalImport).
|
||||
type LocalSkillImportConflict struct {
|
||||
ExistingSkillID string `json:"existing_skill_id"`
|
||||
ExistingCreatedBy string `json:"existing_created_by,omitempty"`
|
||||
CanOverwrite bool `json:"can_overwrite"`
|
||||
}
|
||||
|
||||
const (
|
||||
// runtimeLocalSkillPendingTimeout bounds how long a request can sit in
|
||||
// pending before the server marks it timed out. The value must accommodate
|
||||
@@ -62,11 +92,28 @@ type LocalSkillListStore interface {
|
||||
Fail(ctx context.Context, id string, errMsg string) error
|
||||
}
|
||||
|
||||
// LocalSkillImportRequestInput carries the fields needed to enqueue a
|
||||
// runtime-local-skill import. SupportsConflict gates the structured-conflict
|
||||
// contract: only clients that opt in receive the `conflict` terminal status;
|
||||
// older clients keep the legacy `failed` ("a skill with this name already
|
||||
// exists") behavior so an already-installed Desktop build doesn't regress when
|
||||
// it talks to an upgraded backend. See MUL-2800.
|
||||
type LocalSkillImportRequestInput struct {
|
||||
RuntimeID string
|
||||
CreatorID string
|
||||
SkillKey string
|
||||
Name *string
|
||||
Description *string
|
||||
Action LocalSkillImportAction
|
||||
TargetSkillID string
|
||||
SupportsConflict bool
|
||||
}
|
||||
|
||||
// LocalSkillImportStore is the same contract as LocalSkillListStore but for
|
||||
// runtime-local-skill import requests. Kept as a separate interface because the
|
||||
// Create signature carries import-specific fields (skill_key, optional rename).
|
||||
type LocalSkillImportStore interface {
|
||||
Create(ctx context.Context, runtimeID, creatorID, skillKey string, name, description *string) (*RuntimeLocalSkillImportRequest, error)
|
||||
Create(ctx context.Context, input LocalSkillImportRequestInput) (*RuntimeLocalSkillImportRequest, error)
|
||||
Get(ctx context.Context, id string) (*RuntimeLocalSkillImportRequest, error)
|
||||
HasPending(ctx context.Context, runtimeID string) (bool, error)
|
||||
PopPending(ctx context.Context, runtimeID string) (*RuntimeLocalSkillImportRequest, error)
|
||||
@@ -75,6 +122,9 @@ type LocalSkillImportStore interface {
|
||||
// multiple imports per heartbeat cycle.
|
||||
PopPendingBatch(ctx context.Context, runtimeID string, limit int) ([]*RuntimeLocalSkillImportRequest, error)
|
||||
Complete(ctx context.Context, id string, skill SkillResponse) error
|
||||
// Conflict transitions a request to the terminal RuntimeLocalSkillConflict
|
||||
// state, attaching structured conflict metadata for the caller to act on.
|
||||
Conflict(ctx context.Context, id string, info LocalSkillImportConflict) error
|
||||
Fail(ctx context.Context, id string, errMsg string) error
|
||||
}
|
||||
|
||||
@@ -143,18 +193,25 @@ type RuntimeLocalSkillListRequest struct {
|
||||
}
|
||||
|
||||
type RuntimeLocalSkillImportRequest struct {
|
||||
ID string `json:"id"`
|
||||
RuntimeID string `json:"runtime_id"`
|
||||
SkillKey string `json:"skill_key"`
|
||||
Name *string `json:"name,omitempty"`
|
||||
Description *string `json:"description,omitempty"`
|
||||
Status RuntimeLocalSkillRequestStatus `json:"status"`
|
||||
Skill *SkillResponse `json:"skill,omitempty"`
|
||||
Error string `json:"error,omitempty"`
|
||||
CreatedAt time.Time `json:"created_at"`
|
||||
UpdatedAt time.Time `json:"updated_at"`
|
||||
CreatorID string `json:"-"`
|
||||
RunStartedAt *time.Time `json:"-"`
|
||||
ID string `json:"id"`
|
||||
RuntimeID string `json:"runtime_id"`
|
||||
SkillKey string `json:"skill_key"`
|
||||
Name *string `json:"name,omitempty"`
|
||||
Description *string `json:"description,omitempty"`
|
||||
Action LocalSkillImportAction `json:"action,omitempty"`
|
||||
TargetSkillID string `json:"target_skill_id,omitempty"`
|
||||
// SupportsConflict records whether the initiating client opted into the
|
||||
// structured-conflict contract; consulted at report time to decide between
|
||||
// the new `conflict` status and the legacy `failed` behavior.
|
||||
SupportsConflict bool `json:"supports_conflict,omitempty"`
|
||||
Status RuntimeLocalSkillRequestStatus `json:"status"`
|
||||
Skill *SkillResponse `json:"skill,omitempty"`
|
||||
Conflict *LocalSkillImportConflict `json:"conflict,omitempty"`
|
||||
Error string `json:"error,omitempty"`
|
||||
CreatedAt time.Time `json:"created_at"`
|
||||
UpdatedAt time.Time `json:"updated_at"`
|
||||
CreatorID string `json:"-"`
|
||||
RunStartedAt *time.Time `json:"-"`
|
||||
}
|
||||
|
||||
// InMemoryLocalSkillListStore is the single-node implementation — good enough
|
||||
@@ -277,7 +334,7 @@ func NewInMemoryLocalSkillImportStore() *InMemoryLocalSkillImportStore {
|
||||
return &InMemoryLocalSkillImportStore{requests: make(map[string]*RuntimeLocalSkillImportRequest)}
|
||||
}
|
||||
|
||||
func (s *InMemoryLocalSkillImportStore) Create(_ context.Context, runtimeID, creatorID, skillKey string, name, description *string) (*RuntimeLocalSkillImportRequest, error) {
|
||||
func (s *InMemoryLocalSkillImportStore) Create(_ context.Context, input LocalSkillImportRequestInput) (*RuntimeLocalSkillImportRequest, error) {
|
||||
s.mu.Lock()
|
||||
defer s.mu.Unlock()
|
||||
|
||||
@@ -288,15 +345,18 @@ func (s *InMemoryLocalSkillImportStore) Create(_ context.Context, runtimeID, cre
|
||||
}
|
||||
|
||||
req := &RuntimeLocalSkillImportRequest{
|
||||
ID: randomID(),
|
||||
RuntimeID: runtimeID,
|
||||
SkillKey: skillKey,
|
||||
Name: name,
|
||||
Description: description,
|
||||
Status: RuntimeLocalSkillPending,
|
||||
CreatedAt: time.Now(),
|
||||
UpdatedAt: time.Now(),
|
||||
CreatorID: creatorID,
|
||||
ID: randomID(),
|
||||
RuntimeID: input.RuntimeID,
|
||||
SkillKey: input.SkillKey,
|
||||
Name: input.Name,
|
||||
Description: input.Description,
|
||||
Action: input.Action,
|
||||
TargetSkillID: input.TargetSkillID,
|
||||
SupportsConflict: input.SupportsConflict,
|
||||
Status: RuntimeLocalSkillPending,
|
||||
CreatedAt: time.Now(),
|
||||
UpdatedAt: time.Now(),
|
||||
CreatorID: input.CreatorID,
|
||||
}
|
||||
s.requests[req.ID] = req
|
||||
return req, nil
|
||||
@@ -396,6 +456,19 @@ func (s *InMemoryLocalSkillImportStore) Complete(_ context.Context, id string, s
|
||||
return nil
|
||||
}
|
||||
|
||||
func (s *InMemoryLocalSkillImportStore) Conflict(_ context.Context, id string, info LocalSkillImportConflict) error {
|
||||
s.mu.Lock()
|
||||
defer s.mu.Unlock()
|
||||
|
||||
if req, ok := s.requests[id]; ok {
|
||||
req.Status = RuntimeLocalSkillConflict
|
||||
conflict := info
|
||||
req.Conflict = &conflict
|
||||
req.UpdatedAt = time.Now()
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
func (s *InMemoryLocalSkillImportStore) Fail(_ context.Context, id string, errMsg string) error {
|
||||
s.mu.Lock()
|
||||
defer s.mu.Unlock()
|
||||
@@ -412,6 +485,14 @@ type CreateRuntimeLocalSkillImportRequest struct {
|
||||
SkillKey string `json:"skill_key"`
|
||||
Name *string `json:"name,omitempty"`
|
||||
Description *string `json:"description,omitempty"`
|
||||
// Action selects create (default) vs overwrite. When overwrite,
|
||||
// TargetSkillID must reference the existing same-name skill.
|
||||
Action LocalSkillImportAction `json:"action,omitempty"`
|
||||
TargetSkillID string `json:"target_skill_id,omitempty"`
|
||||
// SupportsConflict opts the client into the structured-conflict contract.
|
||||
// Omit it (older clients) to keep the legacy `failed` behavior on a
|
||||
// same-name collision. An overwrite request implies the new contract.
|
||||
SupportsConflict bool `json:"supports_conflict,omitempty"`
|
||||
}
|
||||
|
||||
type reportedRuntimeLocalSkill struct {
|
||||
@@ -435,7 +516,8 @@ func cleanOptionalString(value *string) *string {
|
||||
}
|
||||
|
||||
func runtimeLocalSkillRequestTerminal(status RuntimeLocalSkillRequestStatus) bool {
|
||||
return status == RuntimeLocalSkillCompleted || status == RuntimeLocalSkillFailed || status == RuntimeLocalSkillTimeout
|
||||
return status == RuntimeLocalSkillCompleted || status == RuntimeLocalSkillFailed ||
|
||||
status == RuntimeLocalSkillTimeout || status == RuntimeLocalSkillConflict
|
||||
}
|
||||
|
||||
func (h *Handler) requireRuntimeLocalSkillAccess(w http.ResponseWriter, r *http.Request, runtimeID string) (runtimeIDAndWorkspace, bool) {
|
||||
@@ -542,14 +624,36 @@ func (h *Handler) InitiateImportLocalSkill(w http.ResponseWriter, r *http.Reques
|
||||
return
|
||||
}
|
||||
|
||||
importReq, err := h.LocalSkillImportStore.Create(
|
||||
r.Context(),
|
||||
rt.runtimeID,
|
||||
creatorID,
|
||||
strings.TrimSpace(req.SkillKey),
|
||||
cleanOptionalString(req.Name),
|
||||
cleanOptionalString(req.Description),
|
||||
)
|
||||
targetSkillID := ""
|
||||
switch req.Action {
|
||||
case LocalSkillImportActionCreate:
|
||||
// nothing extra
|
||||
case LocalSkillImportActionOverwrite:
|
||||
// Existence + creator permission are re-verified authoritatively at
|
||||
// report time (the skill may change between confirm and write); here we
|
||||
// only require a well-formed target so we never enqueue a doomed write.
|
||||
uuid, ok := parseUUIDOrBadRequest(w, strings.TrimSpace(req.TargetSkillID), "target_skill_id")
|
||||
if !ok {
|
||||
return
|
||||
}
|
||||
targetSkillID = uuidToString(uuid)
|
||||
default:
|
||||
writeError(w, http.StatusBadRequest, "invalid action")
|
||||
return
|
||||
}
|
||||
|
||||
importReq, err := h.LocalSkillImportStore.Create(r.Context(), LocalSkillImportRequestInput{
|
||||
RuntimeID: rt.runtimeID,
|
||||
CreatorID: creatorID,
|
||||
SkillKey: strings.TrimSpace(req.SkillKey),
|
||||
Name: cleanOptionalString(req.Name),
|
||||
Description: cleanOptionalString(req.Description),
|
||||
Action: req.Action,
|
||||
TargetSkillID: targetSkillID,
|
||||
// An overwrite request is inherently a new-client action, so it implies
|
||||
// the structured-conflict contract even if the flag is omitted.
|
||||
SupportsConflict: req.SupportsConflict || req.Action == LocalSkillImportActionOverwrite,
|
||||
})
|
||||
if err != nil {
|
||||
writeError(w, http.StatusInternalServerError, "failed to enqueue local skill import: "+err.Error())
|
||||
return
|
||||
@@ -671,21 +775,11 @@ func (h *Handler) ReportLocalSkillImportResult(w http.ResponseWriter, r *http.Re
|
||||
}
|
||||
|
||||
if body.Status != "completed" {
|
||||
if err := h.LocalSkillImportStore.Fail(r.Context(), requestID, body.Error); err != nil {
|
||||
slog.Error("local skill import Fail failed", "error", err, "request_id", requestID)
|
||||
writeError(w, http.StatusInternalServerError, "failed to persist failure")
|
||||
return
|
||||
}
|
||||
writeJSON(w, http.StatusOK, map[string]string{"status": "ok"})
|
||||
h.failLocalSkillImport(w, r, requestID, body.Error)
|
||||
return
|
||||
}
|
||||
if body.Skill == nil {
|
||||
if err := h.LocalSkillImportStore.Fail(r.Context(), requestID, "daemon returned an empty skill bundle"); err != nil {
|
||||
slog.Error("local skill import Fail failed", "error", err, "request_id", requestID)
|
||||
writeError(w, http.StatusInternalServerError, "failed to persist failure")
|
||||
return
|
||||
}
|
||||
writeJSON(w, http.StatusOK, map[string]string{"status": "ok"})
|
||||
h.failLocalSkillImport(w, r, requestID, "daemon returned an empty skill bundle")
|
||||
return
|
||||
}
|
||||
creatorUUID, err := util.ParseUUID(req.CreatorID)
|
||||
@@ -715,33 +809,103 @@ func (h *Handler) ReportLocalSkillImportResult(w http.ResponseWriter, r *http.Re
|
||||
files = append(files, f)
|
||||
}
|
||||
|
||||
config := map[string]any{
|
||||
"origin": map[string]any{
|
||||
"type": "runtime_local",
|
||||
"runtime_id": runtimeID,
|
||||
"provider": body.Skill.Provider,
|
||||
"source_path": body.Skill.SourcePath,
|
||||
},
|
||||
}
|
||||
|
||||
// Overwrite path: re-import onto an existing skill. Existence and creator
|
||||
// permission are re-verified inside overwriteSkillWithFiles, in the same tx
|
||||
// as the write, so a target deleted (or a creator change) between the user's
|
||||
// confirm and this report fails cleanly without falling back to create.
|
||||
if req.Action == LocalSkillImportActionOverwrite {
|
||||
targetUUID, perr := util.ParseUUID(req.TargetSkillID)
|
||||
if perr != nil {
|
||||
failMsg := "stored target_skill_id is invalid"
|
||||
if ferr := h.LocalSkillImportStore.Fail(r.Context(), requestID, failMsg); ferr != nil {
|
||||
slog.Error("local skill import Fail failed", "error", ferr, "request_id", requestID)
|
||||
}
|
||||
writeError(w, http.StatusInternalServerError, failMsg)
|
||||
return
|
||||
}
|
||||
resp, oerr := h.overwriteSkillWithFiles(r.Context(), skillOverwriteInput{
|
||||
WorkspaceID: rt.WorkspaceID,
|
||||
TargetSkillID: targetUUID,
|
||||
UserID: req.CreatorID,
|
||||
ExpectedName: sanitizeNullBytes(name),
|
||||
Description: description,
|
||||
Content: body.Skill.Content,
|
||||
Config: config,
|
||||
Files: files,
|
||||
})
|
||||
if oerr != nil {
|
||||
failMsg := oerr.Error()
|
||||
switch {
|
||||
case errors.Is(oerr, errSkillOverwriteNotFound):
|
||||
failMsg = "target skill no longer exists"
|
||||
case errors.Is(oerr, errSkillOverwriteForbidden):
|
||||
failMsg = "you no longer have permission to overwrite this skill"
|
||||
case errors.Is(oerr, errSkillOverwriteNameMismatch):
|
||||
failMsg = "target skill name no longer matches the imported skill"
|
||||
}
|
||||
h.failLocalSkillImport(w, r, requestID, failMsg)
|
||||
return
|
||||
}
|
||||
if err := h.LocalSkillImportStore.Complete(r.Context(), requestID, resp.SkillResponse); err != nil {
|
||||
// The overwrite already committed; unlike the create path we must
|
||||
// NOT delete the skill to "roll back" (that would destroy a
|
||||
// pre-existing skill and its agent bindings). Surface 5xx so the
|
||||
// daemon retries — the retry re-applies the same UPDATE idempotently.
|
||||
slog.Error("local skill import overwrite Complete failed",
|
||||
"error", err, "request_id", requestID, "skill_id", resp.ID)
|
||||
writeError(w, http.StatusInternalServerError, "failed to persist import completion")
|
||||
return
|
||||
}
|
||||
h.publish(protocol.EventSkillUpdated, uuidToString(rt.WorkspaceID), "member", req.CreatorID, map[string]any{"skill": resp})
|
||||
slog.Debug("runtime local skill overwritten", "runtime_id", runtimeID, "request_id", requestID, "skill_id", resp.ID)
|
||||
writeJSON(w, http.StatusOK, map[string]string{"status": "ok"})
|
||||
return
|
||||
}
|
||||
|
||||
// Create path: detect a same-name conflict before writing. For opted-in
|
||||
// clients this is a structured terminal state (not a failure) so the caller
|
||||
// can offer overwrite / rename / skip; older clients keep the legacy
|
||||
// `failed` behavior (see resolveLocalSkillConflict).
|
||||
if existing, found, lerr := h.lookupSkillByName(r.Context(), rt.WorkspaceID, sanitizeNullBytes(name)); lerr != nil {
|
||||
h.failLocalSkillImport(w, r, requestID, "failed to check for existing skill: "+lerr.Error())
|
||||
return
|
||||
} else if found {
|
||||
h.resolveLocalSkillConflict(w, r, req, existing)
|
||||
return
|
||||
}
|
||||
|
||||
resp, err := h.createSkillWithFiles(r.Context(), skillCreateInput{
|
||||
WorkspaceID: rt.WorkspaceID,
|
||||
CreatorID: creatorUUID,
|
||||
Name: name,
|
||||
Description: description,
|
||||
Content: body.Skill.Content,
|
||||
Config: map[string]any{
|
||||
"origin": map[string]any{
|
||||
"type": "runtime_local",
|
||||
"runtime_id": runtimeID,
|
||||
"provider": body.Skill.Provider,
|
||||
"source_path": body.Skill.SourcePath,
|
||||
},
|
||||
},
|
||||
Files: files,
|
||||
Config: config,
|
||||
Files: files,
|
||||
})
|
||||
if err != nil {
|
||||
failMsg := err.Error()
|
||||
// A unique-violation here means another import won the race between our
|
||||
// lookup and the insert — surface it as a conflict, not a hard failure.
|
||||
if isUniqueViolation(err) {
|
||||
failMsg = "a skill with this name already exists"
|
||||
}
|
||||
if ferr := h.LocalSkillImportStore.Fail(r.Context(), requestID, failMsg); ferr != nil {
|
||||
slog.Error("local skill import Fail failed", "error", ferr, "request_id", requestID)
|
||||
writeError(w, http.StatusInternalServerError, "failed to persist failure")
|
||||
if existing, found, lerr := h.lookupSkillByName(r.Context(), rt.WorkspaceID, sanitizeNullBytes(name)); lerr == nil && found {
|
||||
h.resolveLocalSkillConflict(w, r, req, existing)
|
||||
return
|
||||
}
|
||||
// Lost the row again (deleted between insert-fail and re-lookup):
|
||||
// fall through to the legacy unique-violation message.
|
||||
h.failLocalSkillImport(w, r, requestID, "a skill with this name already exists")
|
||||
return
|
||||
}
|
||||
writeJSON(w, http.StatusOK, map[string]string{"status": "ok"})
|
||||
h.failLocalSkillImport(w, r, requestID, err.Error())
|
||||
return
|
||||
}
|
||||
|
||||
@@ -766,3 +930,64 @@ func (h *Handler) ReportLocalSkillImportResult(w http.ResponseWriter, r *http.Re
|
||||
slog.Debug("runtime local skill imported", "runtime_id", runtimeID, "request_id", requestID, "skill_id", resp.ID)
|
||||
writeJSON(w, http.StatusOK, map[string]string{"status": "ok"})
|
||||
}
|
||||
|
||||
// failLocalSkillImport marks the request failed and writes the standard daemon
|
||||
// response (200 ok). If the store write itself fails it returns 500 so the
|
||||
// daemon retries.
|
||||
func (h *Handler) failLocalSkillImport(w http.ResponseWriter, r *http.Request, requestID, failMsg string) {
|
||||
if err := h.LocalSkillImportStore.Fail(r.Context(), requestID, failMsg); err != nil {
|
||||
slog.Error("local skill import Fail failed", "error", err, "request_id", requestID)
|
||||
writeError(w, http.StatusInternalServerError, "failed to persist failure")
|
||||
return
|
||||
}
|
||||
writeJSON(w, http.StatusOK, map[string]string{"status": "ok"})
|
||||
}
|
||||
|
||||
// resolveLocalSkillConflict terminates a same-name create import. Clients that
|
||||
// opted into the structured-conflict contract (SupportsConflict) receive the
|
||||
// `conflict` status plus metadata so they can offer overwrite / rename / skip;
|
||||
// older clients keep the legacy `failed` ("a skill with this name already
|
||||
// exists") behavior so an installed Desktop build that predates the contract
|
||||
// doesn't regress when it hits an upgraded backend.
|
||||
func (h *Handler) resolveLocalSkillConflict(w http.ResponseWriter, r *http.Request, req *RuntimeLocalSkillImportRequest, existing db.Skill) {
|
||||
if req.SupportsConflict {
|
||||
h.reportLocalSkillConflict(w, r, req.ID, req.CreatorID, existing)
|
||||
return
|
||||
}
|
||||
h.failLocalSkillImport(w, r, req.ID, "a skill with this name already exists")
|
||||
}
|
||||
|
||||
// reportLocalSkillConflict records a same-name conflict as the terminal
|
||||
// RuntimeLocalSkillConflict state with structured metadata the caller uses to
|
||||
// offer overwrite / rename / skip.
|
||||
func (h *Handler) reportLocalSkillConflict(w http.ResponseWriter, r *http.Request, requestID, creatorID string, existing db.Skill) {
|
||||
info := LocalSkillImportConflict{
|
||||
ExistingSkillID: uuidToString(existing.ID),
|
||||
CanOverwrite: canOverwriteSkillByLocalImport(creatorID, existing),
|
||||
}
|
||||
if existing.CreatedBy.Valid {
|
||||
info.ExistingCreatedBy = uuidToString(existing.CreatedBy)
|
||||
}
|
||||
if err := h.LocalSkillImportStore.Conflict(r.Context(), requestID, info); err != nil {
|
||||
slog.Error("local skill import Conflict failed", "error", err, "request_id", requestID)
|
||||
writeError(w, http.StatusInternalServerError, "failed to persist conflict")
|
||||
return
|
||||
}
|
||||
writeJSON(w, http.StatusOK, map[string]string{"status": "ok"})
|
||||
}
|
||||
|
||||
// lookupSkillByName resolves a skill by (workspace, name). found=false with a
|
||||
// nil error means there is no such skill — i.e. no conflict.
|
||||
func (h *Handler) lookupSkillByName(ctx context.Context, workspaceID pgtype.UUID, name string) (db.Skill, bool, error) {
|
||||
skill, err := h.Queries.GetSkillByWorkspaceAndName(ctx, db.GetSkillByWorkspaceAndNameParams{
|
||||
WorkspaceID: workspaceID,
|
||||
Name: name,
|
||||
})
|
||||
if err != nil {
|
||||
if errors.Is(err, pgx.ErrNoRows) {
|
||||
return db.Skill{}, false, nil
|
||||
}
|
||||
return db.Skill{}, false, err
|
||||
}
|
||||
return skill, true, nil
|
||||
}
|
||||
|
||||
434
server/internal/handler/runtime_local_skills_overwrite_test.go
Normal file
434
server/internal/handler/runtime_local_skills_overwrite_test.go
Normal file
@@ -0,0 +1,434 @@
|
||||
package handler
|
||||
|
||||
import (
|
||||
"context"
|
||||
"encoding/json"
|
||||
"fmt"
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"testing"
|
||||
"time"
|
||||
)
|
||||
|
||||
// createImportTargetSkill inserts a skill (owned by ownerID) plus the given
|
||||
// path->content files directly into the DB, returning its id. Used as the
|
||||
// pre-existing skill that conflict / overwrite imports collide with.
|
||||
func createImportTargetSkill(t *testing.T, name, ownerID string, files map[string]string) string {
|
||||
t.Helper()
|
||||
|
||||
var skillID string
|
||||
if err := testPool.QueryRow(context.Background(), `
|
||||
INSERT INTO skill (workspace_id, name, description, content, config, created_by)
|
||||
VALUES ($1, $2, 'original description', '# original', '{}'::jsonb, $3)
|
||||
RETURNING id
|
||||
`, testWorkspaceID, name, ownerID).Scan(&skillID); err != nil {
|
||||
t.Fatalf("create target skill: %v", err)
|
||||
}
|
||||
for path, content := range files {
|
||||
if _, err := testPool.Exec(context.Background(), `
|
||||
INSERT INTO skill_file (skill_id, path, content) VALUES ($1, $2, $3)
|
||||
`, skillID, path, content); err != nil {
|
||||
t.Fatalf("create skill file: %v", err)
|
||||
}
|
||||
}
|
||||
t.Cleanup(func() {
|
||||
testPool.Exec(context.Background(), `DELETE FROM skill WHERE id = $1`, skillID)
|
||||
})
|
||||
return skillID
|
||||
}
|
||||
|
||||
// bindAgentToSkill creates a workspace agent and binds it to skillID via
|
||||
// agent_skill, returning the agent id. Lets overwrite tests assert the binding
|
||||
// survives the re-import.
|
||||
func bindAgentToSkill(t *testing.T, skillID string) string {
|
||||
t.Helper()
|
||||
|
||||
agentName := fmt.Sprintf("overwrite-test-agent-%d", time.Now().UnixNano())
|
||||
var agentID string
|
||||
if err := testPool.QueryRow(context.Background(), `
|
||||
INSERT INTO agent (
|
||||
workspace_id, name, description, runtime_mode, runtime_config,
|
||||
runtime_id, visibility, max_concurrent_tasks, owner_id
|
||||
)
|
||||
VALUES ($1, $2, '', 'cloud', '{}'::jsonb, $3, 'workspace', 1, $4)
|
||||
RETURNING id
|
||||
`, testWorkspaceID, agentName, testRuntimeID, testUserID).Scan(&agentID); err != nil {
|
||||
t.Fatalf("create agent: %v", err)
|
||||
}
|
||||
if _, err := testPool.Exec(context.Background(), `
|
||||
INSERT INTO agent_skill (agent_id, skill_id) VALUES ($1, $2)
|
||||
`, agentID, skillID); err != nil {
|
||||
t.Fatalf("bind agent skill: %v", err)
|
||||
}
|
||||
t.Cleanup(func() {
|
||||
testPool.Exec(context.Background(), `DELETE FROM agent WHERE id = $1`, agentID)
|
||||
})
|
||||
return agentID
|
||||
}
|
||||
|
||||
func countAgentSkillBindings(t *testing.T, skillID string) int {
|
||||
t.Helper()
|
||||
|
||||
var count int
|
||||
if err := testPool.QueryRow(context.Background(), `
|
||||
SELECT count(*) FROM agent_skill WHERE skill_id = $1
|
||||
`, skillID).Scan(&count); err != nil {
|
||||
t.Fatalf("count agent_skill: %v", err)
|
||||
}
|
||||
return count
|
||||
}
|
||||
|
||||
func getSkillRow(t *testing.T, skillID string) (name, description, content, createdBy string) {
|
||||
t.Helper()
|
||||
|
||||
if err := testPool.QueryRow(context.Background(), `
|
||||
SELECT name, description, content, COALESCE(created_by::text, '')
|
||||
FROM skill WHERE id = $1
|
||||
`, skillID).Scan(&name, &description, &content, &createdBy); err != nil {
|
||||
t.Fatalf("get skill row: %v", err)
|
||||
}
|
||||
return
|
||||
}
|
||||
|
||||
// reportBundleBody builds the daemon "completed" report body for an import.
|
||||
func reportBundleBody(name, description, content string, files map[string]string) map[string]any {
|
||||
fileList := make([]map[string]any, 0, len(files))
|
||||
for p, c := range files {
|
||||
fileList = append(fileList, map[string]any{"path": p, "content": c})
|
||||
}
|
||||
return map[string]any{
|
||||
"status": "completed",
|
||||
"skill": map[string]any{
|
||||
"name": name,
|
||||
"description": description,
|
||||
"content": content,
|
||||
"source_path": "~/.claude/skills/review-helper",
|
||||
"provider": "claude",
|
||||
"files": fileList,
|
||||
},
|
||||
}
|
||||
}
|
||||
|
||||
func initiateLocalSkillImport(t *testing.T, runtimeID string, body map[string]any) string {
|
||||
t.Helper()
|
||||
|
||||
w := httptest.NewRecorder()
|
||||
req := withURLParams(
|
||||
newRequestAsUser(testUserID, http.MethodPost, "/api/runtimes/"+runtimeID+"/local-skills/import", body),
|
||||
"runtimeId", runtimeID,
|
||||
)
|
||||
testHandler.InitiateImportLocalSkill(w, req)
|
||||
if w.Code != http.StatusOK {
|
||||
t.Fatalf("InitiateImportLocalSkill: expected 200, got %d: %s", w.Code, w.Body.String())
|
||||
}
|
||||
var importReq RuntimeLocalSkillImportRequest
|
||||
if err := json.NewDecoder(w.Body).Decode(&importReq); err != nil {
|
||||
t.Fatalf("decode import request: %v", err)
|
||||
}
|
||||
return importReq.ID
|
||||
}
|
||||
|
||||
func reportLocalSkillImport(t *testing.T, runtimeID, requestID string, body map[string]any) {
|
||||
t.Helper()
|
||||
|
||||
w := httptest.NewRecorder()
|
||||
req := withURLParams(
|
||||
newDaemonTokenRequest(http.MethodPost, "/api/daemon/runtimes/"+runtimeID+"/local-skills/import/"+requestID+"/result", body, testWorkspaceID, "overwrite-test-daemon"),
|
||||
"runtimeId", runtimeID,
|
||||
"requestId", requestID,
|
||||
)
|
||||
testHandler.ReportLocalSkillImportResult(w, req)
|
||||
if w.Code != http.StatusOK {
|
||||
t.Fatalf("ReportLocalSkillImportResult: expected 200, got %d: %s", w.Code, w.Body.String())
|
||||
}
|
||||
}
|
||||
|
||||
func pollLocalSkillImport(t *testing.T, runtimeID, requestID string) RuntimeLocalSkillImportRequest {
|
||||
t.Helper()
|
||||
|
||||
w := httptest.NewRecorder()
|
||||
req := withURLParams(
|
||||
newRequestAsUser(testUserID, http.MethodGet, "/api/runtimes/"+runtimeID+"/local-skills/import/"+requestID, nil),
|
||||
"runtimeId", runtimeID,
|
||||
"requestId", requestID,
|
||||
)
|
||||
testHandler.GetLocalSkillImportRequest(w, req)
|
||||
if w.Code != http.StatusOK {
|
||||
t.Fatalf("GetLocalSkillImportRequest: expected 200, got %d: %s", w.Code, w.Body.String())
|
||||
}
|
||||
var got RuntimeLocalSkillImportRequest
|
||||
if err := json.NewDecoder(w.Body).Decode(&got); err != nil {
|
||||
t.Fatalf("decode poll response: %v", err)
|
||||
}
|
||||
return got
|
||||
}
|
||||
|
||||
// runLocalSkillImport drives initiate -> report -> poll and returns the
|
||||
// terminal request.
|
||||
func runLocalSkillImport(t *testing.T, runtimeID string, initBody, reportBody map[string]any) RuntimeLocalSkillImportRequest {
|
||||
t.Helper()
|
||||
requestID := initiateLocalSkillImport(t, runtimeID, initBody)
|
||||
reportLocalSkillImport(t, runtimeID, requestID, reportBody)
|
||||
return pollLocalSkillImport(t, runtimeID, requestID)
|
||||
}
|
||||
|
||||
func TestRuntimeLocalSkillImport_ConflictCreatorCanOverwrite(t *testing.T) {
|
||||
if testHandler == nil {
|
||||
t.Skip("database not available")
|
||||
}
|
||||
|
||||
runtimeID := createRuntimeLocalSkillTestRuntime(t, testUserID)
|
||||
name := fmt.Sprintf("conflict-creator-%d", time.Now().UnixNano())
|
||||
existingID := createImportTargetSkill(t, name, testUserID, nil)
|
||||
|
||||
got := runLocalSkillImport(t, runtimeID,
|
||||
map[string]any{"skill_key": "review-helper", "supports_conflict": true},
|
||||
reportBundleBody(name, "incoming description", "# incoming", map[string]string{"a.md": "A"}),
|
||||
)
|
||||
|
||||
if got.Status != RuntimeLocalSkillConflict {
|
||||
t.Fatalf("status = %s, want conflict", got.Status)
|
||||
}
|
||||
if got.Conflict == nil {
|
||||
t.Fatal("expected conflict metadata")
|
||||
}
|
||||
if got.Conflict.ExistingSkillID != existingID {
|
||||
t.Fatalf("existing_skill_id = %q, want %q", got.Conflict.ExistingSkillID, existingID)
|
||||
}
|
||||
if got.Conflict.ExistingCreatedBy != testUserID {
|
||||
t.Fatalf("existing_created_by = %q, want %q", got.Conflict.ExistingCreatedBy, testUserID)
|
||||
}
|
||||
if !got.Conflict.CanOverwrite {
|
||||
t.Fatal("creator should be allowed to overwrite")
|
||||
}
|
||||
// A conflict must neither create a second skill nor mutate the original.
|
||||
if n := countSkillsByName(t, name); n != 1 {
|
||||
t.Fatalf("expected exactly 1 skill named %q, got %d", name, n)
|
||||
}
|
||||
if _, desc, _, _ := getSkillRow(t, existingID); desc != "original description" {
|
||||
t.Fatalf("conflict must not modify the existing skill, description = %q", desc)
|
||||
}
|
||||
}
|
||||
|
||||
func TestRuntimeLocalSkillImport_ConflictNonCreatorCannotOverwrite(t *testing.T) {
|
||||
if testHandler == nil {
|
||||
t.Skip("database not available")
|
||||
}
|
||||
|
||||
runtimeID := createRuntimeLocalSkillTestRuntime(t, testUserID)
|
||||
otherUserID := createRuntimeLocalSkillTestMember(t, "member")
|
||||
name := fmt.Sprintf("conflict-noncreator-%d", time.Now().UnixNano())
|
||||
existingID := createImportTargetSkill(t, name, otherUserID, nil)
|
||||
|
||||
got := runLocalSkillImport(t, runtimeID,
|
||||
map[string]any{"skill_key": "review-helper", "supports_conflict": true},
|
||||
reportBundleBody(name, "incoming description", "# incoming", nil),
|
||||
)
|
||||
|
||||
if got.Status != RuntimeLocalSkillConflict {
|
||||
t.Fatalf("status = %s, want conflict", got.Status)
|
||||
}
|
||||
if got.Conflict == nil {
|
||||
t.Fatal("expected conflict metadata")
|
||||
}
|
||||
if got.Conflict.ExistingSkillID != existingID {
|
||||
t.Fatalf("existing_skill_id = %q, want %q", got.Conflict.ExistingSkillID, existingID)
|
||||
}
|
||||
if got.Conflict.ExistingCreatedBy != otherUserID {
|
||||
t.Fatalf("existing_created_by = %q, want %q", got.Conflict.ExistingCreatedBy, otherUserID)
|
||||
}
|
||||
if got.Conflict.CanOverwrite {
|
||||
t.Fatal("a non-creator must not be allowed to overwrite")
|
||||
}
|
||||
}
|
||||
|
||||
func TestRuntimeLocalSkillImport_OverwritePreservesIdentityAndBindings(t *testing.T) {
|
||||
if testHandler == nil {
|
||||
t.Skip("database not available")
|
||||
}
|
||||
|
||||
runtimeID := createRuntimeLocalSkillTestRuntime(t, testUserID)
|
||||
name := fmt.Sprintf("overwrite-keep-%d", time.Now().UnixNano())
|
||||
existingID := createImportTargetSkill(t, name, testUserID, map[string]string{
|
||||
"keep.md": "old keep",
|
||||
"prune.md": "should be removed",
|
||||
})
|
||||
bindAgentToSkill(t, existingID)
|
||||
|
||||
got := runLocalSkillImport(t, runtimeID,
|
||||
map[string]any{"skill_key": "review-helper", "action": "overwrite", "target_skill_id": existingID},
|
||||
reportBundleBody(name, "overwritten description", "# overwritten", map[string]string{"keep.md": "new keep"}),
|
||||
)
|
||||
|
||||
if got.Status != RuntimeLocalSkillCompleted {
|
||||
t.Fatalf("status = %s, want completed (error=%q)", got.Status, got.Error)
|
||||
}
|
||||
if got.Skill == nil {
|
||||
t.Fatal("expected overwritten skill in response")
|
||||
}
|
||||
// Same row: UUID and creator preserved.
|
||||
if got.Skill.ID != existingID {
|
||||
t.Fatalf("overwrite must preserve UUID: got %q, want %q", got.Skill.ID, existingID)
|
||||
}
|
||||
if got.Skill.CreatedBy == nil || *got.Skill.CreatedBy != testUserID {
|
||||
t.Fatalf("created_by not preserved: %v", got.Skill.CreatedBy)
|
||||
}
|
||||
if got.Skill.Description != "overwritten description" {
|
||||
t.Fatalf("description not replaced: %q", got.Skill.Description)
|
||||
}
|
||||
// Files fully replaced: prune.md (absent from the new bundle) is gone.
|
||||
if n := countSkillFiles(t, existingID); n != 1 {
|
||||
t.Fatalf("expected 1 file after overwrite, got %d", n)
|
||||
}
|
||||
// Agent binding preserved — the agent must NOT need to re-add the skill.
|
||||
if n := countAgentSkillBindings(t, existingID); n != 1 {
|
||||
t.Fatalf("expected agent binding to survive overwrite, got %d", n)
|
||||
}
|
||||
}
|
||||
|
||||
func TestRuntimeLocalSkillImport_OverwriteNonCreatorFails(t *testing.T) {
|
||||
if testHandler == nil {
|
||||
t.Skip("database not available")
|
||||
}
|
||||
|
||||
runtimeID := createRuntimeLocalSkillTestRuntime(t, testUserID)
|
||||
otherUserID := createRuntimeLocalSkillTestMember(t, "member")
|
||||
name := fmt.Sprintf("overwrite-forbidden-%d", time.Now().UnixNano())
|
||||
existingID := createImportTargetSkill(t, name, otherUserID, nil)
|
||||
|
||||
got := runLocalSkillImport(t, runtimeID,
|
||||
map[string]any{"skill_key": "review-helper", "action": "overwrite", "target_skill_id": existingID},
|
||||
reportBundleBody(name, "incoming description", "# incoming", nil),
|
||||
)
|
||||
|
||||
if got.Status != RuntimeLocalSkillFailed {
|
||||
t.Fatalf("status = %s, want failed", got.Status)
|
||||
}
|
||||
// Original skill (owned by someone else) must be untouched.
|
||||
if _, desc, _, _ := getSkillRow(t, existingID); desc != "original description" {
|
||||
t.Fatalf("forbidden overwrite must not mutate the skill, description = %q", desc)
|
||||
}
|
||||
}
|
||||
|
||||
func TestRuntimeLocalSkillImport_OverwriteTargetDeletedFails(t *testing.T) {
|
||||
if testHandler == nil {
|
||||
t.Skip("database not available")
|
||||
}
|
||||
|
||||
runtimeID := createRuntimeLocalSkillTestRuntime(t, testUserID)
|
||||
name := fmt.Sprintf("overwrite-deleted-%d", time.Now().UnixNano())
|
||||
deletedID := createImportTargetSkill(t, name, testUserID, nil)
|
||||
// Simulate the target being deleted between the user's confirm and the
|
||||
// daemon report.
|
||||
if _, err := testPool.Exec(context.Background(), `DELETE FROM skill WHERE id = $1`, deletedID); err != nil {
|
||||
t.Fatalf("delete target skill: %v", err)
|
||||
}
|
||||
|
||||
got := runLocalSkillImport(t, runtimeID,
|
||||
map[string]any{"skill_key": "review-helper", "action": "overwrite", "target_skill_id": deletedID},
|
||||
reportBundleBody(name, "incoming description", "# incoming", map[string]string{"a.md": "A"}),
|
||||
)
|
||||
|
||||
if got.Status != RuntimeLocalSkillFailed {
|
||||
t.Fatalf("status = %s, want failed", got.Status)
|
||||
}
|
||||
// Must NOT fall back to creating a new skill by name.
|
||||
if n := countSkillsByName(t, name); n != 0 {
|
||||
t.Fatalf("deleted-target overwrite must not create a skill, got %d", n)
|
||||
}
|
||||
}
|
||||
|
||||
func TestRuntimeLocalSkillImport_OverwriteRetryIsIdempotent(t *testing.T) {
|
||||
if testHandler == nil {
|
||||
t.Skip("database not available")
|
||||
}
|
||||
|
||||
runtimeID := createRuntimeLocalSkillTestRuntime(t, testUserID)
|
||||
name := fmt.Sprintf("overwrite-idempotent-%d", time.Now().UnixNano())
|
||||
existingID := createImportTargetSkill(t, name, testUserID, map[string]string{"old.md": "old"})
|
||||
|
||||
requestID := initiateLocalSkillImport(t, runtimeID, map[string]any{
|
||||
"skill_key": "review-helper",
|
||||
"action": "overwrite",
|
||||
"target_skill_id": existingID,
|
||||
})
|
||||
|
||||
// First report wins and overwrites the skill.
|
||||
reportLocalSkillImport(t, runtimeID, requestID,
|
||||
reportBundleBody(name, "first overwrite", "# first", map[string]string{"first.md": "1"}))
|
||||
|
||||
// A retry of the SAME request id with a different bundle must be ignored
|
||||
// (the request is already terminal) — no second write.
|
||||
reportLocalSkillImport(t, runtimeID, requestID,
|
||||
reportBundleBody(name, "second overwrite", "# second", map[string]string{"second.md": "2", "extra.md": "3"}))
|
||||
|
||||
got := pollLocalSkillImport(t, runtimeID, requestID)
|
||||
if got.Status != RuntimeLocalSkillCompleted {
|
||||
t.Fatalf("status = %s, want completed", got.Status)
|
||||
}
|
||||
if _, desc, _, _ := getSkillRow(t, existingID); desc != "first overwrite" {
|
||||
t.Fatalf("retry must not re-apply, description = %q", desc)
|
||||
}
|
||||
if n := countSkillFiles(t, existingID); n != 1 {
|
||||
t.Fatalf("retry must not re-write files, got %d files", n)
|
||||
}
|
||||
}
|
||||
|
||||
// TestRuntimeLocalSkillImport_LegacyClientGetsFailedOnConflict verifies the
|
||||
// installed-app compatibility gate: a client that does NOT opt into the
|
||||
// structured-conflict contract keeps the legacy `failed` + "already exists"
|
||||
// behavior on a same-name collision, instead of the new `conflict` status its
|
||||
// older poll loop wouldn't understand.
|
||||
func TestRuntimeLocalSkillImport_LegacyClientGetsFailedOnConflict(t *testing.T) {
|
||||
if testHandler == nil {
|
||||
t.Skip("database not available")
|
||||
}
|
||||
|
||||
runtimeID := createRuntimeLocalSkillTestRuntime(t, testUserID)
|
||||
name := fmt.Sprintf("legacy-conflict-%d", time.Now().UnixNano())
|
||||
createImportTargetSkill(t, name, testUserID, nil)
|
||||
|
||||
got := runLocalSkillImport(t, runtimeID,
|
||||
// No supports_conflict (and no action) — an old client.
|
||||
map[string]any{"skill_key": "review-helper"},
|
||||
reportBundleBody(name, "incoming description", "# incoming", nil),
|
||||
)
|
||||
|
||||
if got.Status != RuntimeLocalSkillFailed {
|
||||
t.Fatalf("status = %s, want failed (legacy contract)", got.Status)
|
||||
}
|
||||
if got.Conflict != nil {
|
||||
t.Fatalf("legacy client must not receive structured conflict metadata: %+v", got.Conflict)
|
||||
}
|
||||
if got.Error != "a skill with this name already exists" {
|
||||
t.Fatalf("error = %q, want legacy already-exists message", got.Error)
|
||||
}
|
||||
}
|
||||
|
||||
// TestRuntimeLocalSkillImport_OverwriteNameMismatchFails verifies the guard
|
||||
// against a stale / wrong target_skill_id: if the target's name no longer
|
||||
// matches the imported skill, the overwrite fails instead of writing one
|
||||
// skill's content onto another.
|
||||
func TestRuntimeLocalSkillImport_OverwriteNameMismatchFails(t *testing.T) {
|
||||
if testHandler == nil {
|
||||
t.Skip("database not available")
|
||||
}
|
||||
|
||||
runtimeID := createRuntimeLocalSkillTestRuntime(t, testUserID)
|
||||
targetName := fmt.Sprintf("overwrite-target-%d", time.Now().UnixNano())
|
||||
otherName := fmt.Sprintf("overwrite-other-%d", time.Now().UnixNano())
|
||||
targetID := createImportTargetSkill(t, targetName, testUserID, nil)
|
||||
|
||||
// Overwrite targets targetID but the imported bundle is named otherName.
|
||||
got := runLocalSkillImport(t, runtimeID,
|
||||
map[string]any{"skill_key": "review-helper", "action": "overwrite", "target_skill_id": targetID},
|
||||
reportBundleBody(otherName, "incoming description", "# incoming", map[string]string{"a.md": "A"}),
|
||||
)
|
||||
|
||||
if got.Status != RuntimeLocalSkillFailed {
|
||||
t.Fatalf("status = %s, want failed (name mismatch)", got.Status)
|
||||
}
|
||||
if _, desc, _, _ := getSkillRow(t, targetID); desc != "original description" {
|
||||
t.Fatalf("name-mismatch overwrite must not mutate the target, description = %q", desc)
|
||||
}
|
||||
}
|
||||
@@ -262,18 +262,21 @@ func NewRedisLocalSkillImportStore(rdb *redis.Client) *RedisLocalSkillImportStor
|
||||
return &RedisLocalSkillImportStore{rdb: rdb}
|
||||
}
|
||||
|
||||
func (s *RedisLocalSkillImportStore) Create(ctx context.Context, runtimeID, creatorID, skillKey string, name, description *string) (*RuntimeLocalSkillImportRequest, error) {
|
||||
func (s *RedisLocalSkillImportStore) Create(ctx context.Context, input LocalSkillImportRequestInput) (*RuntimeLocalSkillImportRequest, error) {
|
||||
now := time.Now()
|
||||
req := &RuntimeLocalSkillImportRequest{
|
||||
ID: randomID(),
|
||||
RuntimeID: runtimeID,
|
||||
SkillKey: skillKey,
|
||||
Name: name,
|
||||
Description: description,
|
||||
Status: RuntimeLocalSkillPending,
|
||||
CreatedAt: now,
|
||||
UpdatedAt: now,
|
||||
CreatorID: creatorID,
|
||||
ID: randomID(),
|
||||
RuntimeID: input.RuntimeID,
|
||||
SkillKey: input.SkillKey,
|
||||
Name: input.Name,
|
||||
Description: input.Description,
|
||||
Action: input.Action,
|
||||
TargetSkillID: input.TargetSkillID,
|
||||
SupportsConflict: input.SupportsConflict,
|
||||
Status: RuntimeLocalSkillPending,
|
||||
CreatedAt: now,
|
||||
UpdatedAt: now,
|
||||
CreatorID: input.CreatorID,
|
||||
}
|
||||
data, err := s.marshalImport(req)
|
||||
if err != nil {
|
||||
@@ -282,11 +285,11 @@ func (s *RedisLocalSkillImportStore) Create(ctx context.Context, runtimeID, crea
|
||||
|
||||
pipe := s.rdb.TxPipeline()
|
||||
pipe.Set(ctx, localSkillImportKey(req.ID), data, runtimeLocalSkillStoreRetention)
|
||||
pipe.ZAdd(ctx, localSkillImportPendingKey(runtimeID), redis.Z{
|
||||
pipe.ZAdd(ctx, localSkillImportPendingKey(input.RuntimeID), redis.Z{
|
||||
Score: float64(now.UnixNano()),
|
||||
Member: req.ID,
|
||||
})
|
||||
pipe.Expire(ctx, localSkillImportPendingKey(runtimeID), runtimeLocalSkillStoreRetention*2)
|
||||
pipe.Expire(ctx, localSkillImportPendingKey(input.RuntimeID), runtimeLocalSkillStoreRetention*2)
|
||||
if _, err := pipe.Exec(ctx); err != nil {
|
||||
return nil, fmt.Errorf("persist import request: %w", err)
|
||||
}
|
||||
@@ -494,6 +497,21 @@ func (s *RedisLocalSkillImportStore) Complete(ctx context.Context, id string, sk
|
||||
return s.persistImportRequest(ctx, req)
|
||||
}
|
||||
|
||||
func (s *RedisLocalSkillImportStore) Conflict(ctx context.Context, id string, info LocalSkillImportConflict) error {
|
||||
req, err := s.loadImportRequest(ctx, id)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
if req == nil {
|
||||
return nil
|
||||
}
|
||||
req.Status = RuntimeLocalSkillConflict
|
||||
conflict := info
|
||||
req.Conflict = &conflict
|
||||
req.UpdatedAt = time.Now()
|
||||
return s.persistImportRequest(ctx, req)
|
||||
}
|
||||
|
||||
func (s *RedisLocalSkillImportStore) Fail(ctx context.Context, id string, errMsg string) error {
|
||||
req, err := s.loadImportRequest(ctx, id)
|
||||
if err != nil {
|
||||
|
||||
@@ -225,7 +225,15 @@ func TestRedisLocalSkillImportStore_PreservesCreatorID(t *testing.T) {
|
||||
|
||||
name := "Review Helper"
|
||||
desc := "Desc"
|
||||
req, err := store.Create(ctx, "runtime-1", "user-42", "review-helper", &name, &desc)
|
||||
req, err := store.Create(ctx, LocalSkillImportRequestInput{
|
||||
RuntimeID: "runtime-1",
|
||||
CreatorID: "user-42",
|
||||
SkillKey: "review-helper",
|
||||
Name: &name,
|
||||
Description: &desc,
|
||||
Action: LocalSkillImportActionOverwrite,
|
||||
TargetSkillID: "target-skill-99",
|
||||
})
|
||||
if err != nil {
|
||||
t.Fatalf("create: %v", err)
|
||||
}
|
||||
@@ -249,6 +257,47 @@ func TestRedisLocalSkillImportStore_PreservesCreatorID(t *testing.T) {
|
||||
if got.Description == nil || *got.Description != desc {
|
||||
t.Fatalf("description lost: %v", got.Description)
|
||||
}
|
||||
// The overwrite intent must survive the round trip — it is consumed at
|
||||
// report time, not delivered to the daemon.
|
||||
if got.Action != LocalSkillImportActionOverwrite {
|
||||
t.Fatalf("action lost round trip: %q", got.Action)
|
||||
}
|
||||
if got.TargetSkillID != "target-skill-99" {
|
||||
t.Fatalf("target_skill_id lost round trip: %q", got.TargetSkillID)
|
||||
}
|
||||
}
|
||||
|
||||
func TestRedisLocalSkillImportStore_PreservesConflict(t *testing.T) {
|
||||
rdb := newRedisTestClient(t)
|
||||
ctx := context.Background()
|
||||
store := NewRedisLocalSkillImportStore(rdb)
|
||||
|
||||
req, err := store.Create(ctx, LocalSkillImportRequestInput{
|
||||
RuntimeID: "runtime-1",
|
||||
CreatorID: "user-1",
|
||||
SkillKey: "review-helper",
|
||||
})
|
||||
if err != nil {
|
||||
t.Fatalf("create: %v", err)
|
||||
}
|
||||
info := LocalSkillImportConflict{ExistingSkillID: "skill-7", ExistingCreatedBy: "user-2", CanOverwrite: false}
|
||||
if err := store.Conflict(ctx, req.ID, info); err != nil {
|
||||
t.Fatalf("conflict: %v", err)
|
||||
}
|
||||
|
||||
got, err := store.Get(ctx, req.ID)
|
||||
if err != nil {
|
||||
t.Fatalf("get: %v", err)
|
||||
}
|
||||
if got.Status != RuntimeLocalSkillConflict {
|
||||
t.Fatalf("status = %s, want conflict", got.Status)
|
||||
}
|
||||
if got.Conflict == nil {
|
||||
t.Fatalf("conflict metadata lost round trip")
|
||||
}
|
||||
if got.Conflict.ExistingSkillID != "skill-7" || got.Conflict.ExistingCreatedBy != "user-2" || got.Conflict.CanOverwrite {
|
||||
t.Fatalf("conflict metadata corrupted: %+v", got.Conflict)
|
||||
}
|
||||
}
|
||||
|
||||
func TestRedisLocalSkillImportStore_PopPendingAcrossInstances(t *testing.T) {
|
||||
@@ -258,7 +307,11 @@ func TestRedisLocalSkillImportStore_PopPendingAcrossInstances(t *testing.T) {
|
||||
nodeA := NewRedisLocalSkillImportStore(rdb)
|
||||
nodeB := NewRedisLocalSkillImportStore(rdb)
|
||||
|
||||
req, err := nodeA.Create(ctx, "runtime-import", "user-1", "review-helper", nil, nil)
|
||||
req, err := nodeA.Create(ctx, LocalSkillImportRequestInput{
|
||||
RuntimeID: "runtime-import",
|
||||
CreatorID: "user-1",
|
||||
SkillKey: "review-helper",
|
||||
})
|
||||
if err != nil {
|
||||
t.Fatalf("create: %v", err)
|
||||
}
|
||||
@@ -365,7 +418,11 @@ func TestRedisLocalSkillImportStore_PopPendingBatch(t *testing.T) {
|
||||
// Create 5 pending imports.
|
||||
ids := make([]string, 5)
|
||||
for i := range ids {
|
||||
req, err := store.Create(ctx, "runtime-batch", "user-1", fmt.Sprintf("skill-%d", i), nil, nil)
|
||||
req, err := store.Create(ctx, LocalSkillImportRequestInput{
|
||||
RuntimeID: "runtime-batch",
|
||||
CreatorID: "user-1",
|
||||
SkillKey: fmt.Sprintf("skill-%d", i),
|
||||
})
|
||||
if err != nil {
|
||||
t.Fatalf("create %d: %v", i, err)
|
||||
}
|
||||
|
||||
@@ -187,7 +187,11 @@ func TestInMemoryLocalSkillListStore_TimesOutRunningRequests(t *testing.T) {
|
||||
func TestInMemoryLocalSkillImportStore_TimesOutRunningRequests(t *testing.T) {
|
||||
ctx := context.Background()
|
||||
store := NewInMemoryLocalSkillImportStore()
|
||||
req, err := store.Create(ctx, "runtime-xyz", "user-1", "review-helper", nil, nil)
|
||||
req, err := store.Create(ctx, LocalSkillImportRequestInput{
|
||||
RuntimeID: "runtime-xyz",
|
||||
CreatorID: "user-1",
|
||||
SkillKey: "review-helper",
|
||||
})
|
||||
if err != nil {
|
||||
t.Fatalf("create: %v", err)
|
||||
}
|
||||
@@ -237,7 +241,11 @@ func TestGetLocalSkillImportRequest_RequiresRuntimeOwner(t *testing.T) {
|
||||
|
||||
runtimeID := createRuntimeLocalSkillTestRuntime(t, testUserID)
|
||||
adminUserID := createRuntimeLocalSkillTestMember(t, "admin")
|
||||
importReq, err := testHandler.LocalSkillImportStore.Create(context.Background(), runtimeID, testUserID, "review-helper", nil, nil)
|
||||
importReq, err := testHandler.LocalSkillImportStore.Create(context.Background(), LocalSkillImportRequestInput{
|
||||
RuntimeID: runtimeID,
|
||||
CreatorID: testUserID,
|
||||
SkillKey: "review-helper",
|
||||
})
|
||||
if err != nil {
|
||||
t.Fatalf("create import request: %v", err)
|
||||
}
|
||||
@@ -477,14 +485,13 @@ func TestReportLocalSkillImportResult_IgnoresTimedOutRequests(t *testing.T) {
|
||||
|
||||
runtimeID := createRuntimeLocalSkillTestRuntime(t, testUserID)
|
||||
ctx := context.Background()
|
||||
importReq, err := testHandler.LocalSkillImportStore.Create(
|
||||
ctx,
|
||||
runtimeID,
|
||||
testUserID,
|
||||
"review-helper",
|
||||
cleanOptionalString(ptr("Timed Out Import")),
|
||||
cleanOptionalString(ptr("Should not be created")),
|
||||
)
|
||||
importReq, err := testHandler.LocalSkillImportStore.Create(ctx, LocalSkillImportRequestInput{
|
||||
RuntimeID: runtimeID,
|
||||
CreatorID: testUserID,
|
||||
SkillKey: "review-helper",
|
||||
Name: cleanOptionalString(ptr("Timed Out Import")),
|
||||
Description: cleanOptionalString(ptr("Should not be created")),
|
||||
})
|
||||
if err != nil {
|
||||
t.Fatalf("create import request: %v", err)
|
||||
}
|
||||
@@ -534,7 +541,11 @@ func TestReportLocalSkillImportResult_RejectsCrossWorkspaceDaemonToken(t *testin
|
||||
}
|
||||
|
||||
runtimeID := createRuntimeLocalSkillTestRuntime(t, testUserID)
|
||||
importReq, err := testHandler.LocalSkillImportStore.Create(context.Background(), runtimeID, testUserID, "review-helper", nil, nil)
|
||||
importReq, err := testHandler.LocalSkillImportStore.Create(context.Background(), LocalSkillImportRequestInput{
|
||||
RuntimeID: runtimeID,
|
||||
CreatorID: testUserID,
|
||||
SkillKey: "review-helper",
|
||||
})
|
||||
if err != nil {
|
||||
t.Fatalf("create import request: %v", err)
|
||||
}
|
||||
|
||||
@@ -390,6 +390,15 @@ func (h *Handler) canManageSkill(w http.ResponseWriter, r *http.Request, skill d
|
||||
return true
|
||||
}
|
||||
|
||||
// canOverwriteSkillByLocalImport reports whether userID may overwrite skill via
|
||||
// a runtime-local-skill re-import. This is intentionally NARROWER than
|
||||
// canManageSkill: only the original creator may overwrite by re-importing.
|
||||
// Workspace owners/admins who want to change a skill they did not create must
|
||||
// edit it in-app instead. See MUL-2701 / MUL-2800.
|
||||
func canOverwriteSkillByLocalImport(userID string, skill db.Skill) bool {
|
||||
return skill.CreatedBy.Valid && uuidToString(skill.CreatedBy) == userID
|
||||
}
|
||||
|
||||
func (h *Handler) UpdateSkill(w http.ResponseWriter, r *http.Request) {
|
||||
id := chi.URLParam(r, "id")
|
||||
skill, ok := h.loadSkillForUser(w, r, id)
|
||||
|
||||
@@ -3,7 +3,9 @@ package handler
|
||||
import (
|
||||
"context"
|
||||
"encoding/json"
|
||||
"errors"
|
||||
|
||||
"github.com/jackc/pgx/v5"
|
||||
"github.com/jackc/pgx/v5/pgtype"
|
||||
skillpkg "github.com/multica-ai/multica/server/internal/skill"
|
||||
db "github.com/multica-ai/multica/server/pkg/db/generated"
|
||||
@@ -89,3 +91,126 @@ func (h *Handler) createSkillWithFiles(ctx context.Context, input skillCreateInp
|
||||
|
||||
return result, nil
|
||||
}
|
||||
|
||||
// errSkillOverwriteNotFound / errSkillOverwriteForbidden are the terminal
|
||||
// boundary cases of overwriteSkillWithFiles: the target was deleted (or moved
|
||||
// out of the workspace) or the caller lost overwrite permission between the
|
||||
// user's confirm and this write. Callers map them to a failed import and must
|
||||
// NOT fall back to creating a new skill.
|
||||
var (
|
||||
errSkillOverwriteNotFound = errors.New("target skill not found")
|
||||
errSkillOverwriteForbidden = errors.New("not permitted to overwrite target skill")
|
||||
errSkillOverwriteNameMismatch = errors.New("target skill name does not match the imported skill")
|
||||
)
|
||||
|
||||
type skillOverwriteInput struct {
|
||||
WorkspaceID pgtype.UUID
|
||||
TargetSkillID pgtype.UUID
|
||||
UserID string // re-checked against the skill creator inside the tx
|
||||
// ExpectedName, when non-empty, must equal the target's current name. Guards
|
||||
// against a client sending the wrong target_skill_id and overwriting a
|
||||
// different skill than the one the conflict dialog showed the user. The
|
||||
// caller passes the sanitized effective import name.
|
||||
ExpectedName string
|
||||
Description string
|
||||
Content string
|
||||
Config any
|
||||
Files []CreateSkillFileRequest
|
||||
}
|
||||
|
||||
// overwriteSkillWithFiles re-imports a bundle onto an existing skill in a single
|
||||
// transaction. It re-verifies, inside that tx, that the target still exists in
|
||||
// the workspace and that UserID may overwrite it (creator-only — see
|
||||
// canOverwriteSkillByLocalImport). A target deleted or a creator change between
|
||||
// the user's confirm and this write fails cleanly via errSkillOverwriteNotFound
|
||||
// / errSkillOverwriteForbidden rather than falling back to create.
|
||||
//
|
||||
// Preserved: id, created_by, created_at, name, and agent_skill bindings (the
|
||||
// row identity and the binding table are never touched). Replaced: description,
|
||||
// content, config (origin), and the full file set — files absent from the new
|
||||
// bundle are pruned via DeleteSkillFilesBySkill. On any error the tx rolls back,
|
||||
// leaving the original skill unchanged.
|
||||
func (h *Handler) overwriteSkillWithFiles(ctx context.Context, input skillOverwriteInput) (SkillWithFilesResponse, error) {
|
||||
config, err := json.Marshal(input.Config)
|
||||
if err != nil {
|
||||
return SkillWithFilesResponse{}, err
|
||||
}
|
||||
if input.Config == nil {
|
||||
config = []byte("{}")
|
||||
}
|
||||
|
||||
tx, err := h.TxStarter.Begin(ctx)
|
||||
if err != nil {
|
||||
return SkillWithFilesResponse{}, err
|
||||
}
|
||||
defer tx.Rollback(ctx)
|
||||
|
||||
qtx := h.Queries.WithTx(tx)
|
||||
|
||||
existing, err := qtx.GetSkillInWorkspace(ctx, db.GetSkillInWorkspaceParams{
|
||||
ID: input.TargetSkillID,
|
||||
WorkspaceID: input.WorkspaceID,
|
||||
})
|
||||
if err != nil {
|
||||
if errors.Is(err, pgx.ErrNoRows) {
|
||||
return SkillWithFilesResponse{}, errSkillOverwriteNotFound
|
||||
}
|
||||
return SkillWithFilesResponse{}, err
|
||||
}
|
||||
if !canOverwriteSkillByLocalImport(input.UserID, existing) {
|
||||
return SkillWithFilesResponse{}, errSkillOverwriteForbidden
|
||||
}
|
||||
// The overwrite is keyed on target_skill_id, but the conflict the user
|
||||
// confirmed was a same-name collision; reject if the target's name no longer
|
||||
// matches the imported skill so a stale/wrong target_skill_id can't write
|
||||
// one skill's content onto another.
|
||||
if input.ExpectedName != "" && existing.Name != input.ExpectedName {
|
||||
return SkillWithFilesResponse{}, errSkillOverwriteNameMismatch
|
||||
}
|
||||
|
||||
// Name is intentionally left unset (COALESCE keeps the existing name): the
|
||||
// overwrite targets the same-name skill, so preserving it avoids any
|
||||
// unique-name churn.
|
||||
skill, err := qtx.UpdateSkill(ctx, db.UpdateSkillParams{
|
||||
ID: existing.ID,
|
||||
Description: pgtype.Text{String: sanitizeNullBytes(input.Description), Valid: true},
|
||||
Content: pgtype.Text{String: sanitizeNullBytes(input.Content), Valid: true},
|
||||
Config: config,
|
||||
})
|
||||
if err != nil {
|
||||
// A committed concurrent DELETE can land between the read above and this
|
||||
// UPDATE (READ COMMITTED), so UpdateSkill matches 0 rows. Classify it as
|
||||
// the same "target gone" terminal case rather than a generic failure.
|
||||
if errors.Is(err, pgx.ErrNoRows) {
|
||||
return SkillWithFilesResponse{}, errSkillOverwriteNotFound
|
||||
}
|
||||
return SkillWithFilesResponse{}, err
|
||||
}
|
||||
|
||||
// Full replace: drop every existing file, then re-insert the new set so
|
||||
// files no longer present in the local source are removed.
|
||||
if err := qtx.DeleteSkillFilesBySkill(ctx, skill.ID); err != nil {
|
||||
return SkillWithFilesResponse{}, err
|
||||
}
|
||||
fileResps := make([]SkillFileResponse, 0, len(input.Files))
|
||||
for _, f := range input.Files {
|
||||
sf, err := qtx.UpsertSkillFile(ctx, db.UpsertSkillFileParams{
|
||||
SkillID: skill.ID,
|
||||
Path: sanitizeNullBytes(f.Path),
|
||||
Content: sanitizeNullBytes(f.Content),
|
||||
})
|
||||
if err != nil {
|
||||
return SkillWithFilesResponse{}, err
|
||||
}
|
||||
fileResps = append(fileResps, skillFileToResponse(sf))
|
||||
}
|
||||
|
||||
if err := tx.Commit(ctx); err != nil {
|
||||
return SkillWithFilesResponse{}, err
|
||||
}
|
||||
|
||||
return SkillWithFilesResponse{
|
||||
SkillResponse: skillToResponse(skill),
|
||||
Files: fileResps,
|
||||
}, nil
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user