From a2313442f2e0f6fc2089ed1a1cbf281ef0d21882 Mon Sep 17 00:00:00 2001 From: Naiyuan Qing <145280634+NevilleQingNY@users.noreply.github.com> Date: Mon, 1 Jun 2026 12:49:45 +0800 Subject: [PATCH 1/6] fix(chat): improve history row interactions (#3591) --- .../views/chat/components/chat-window.tsx | 508 ++++++++++++------ packages/views/locales/en/chat.json | 16 + packages/views/locales/ko/chat.json | 16 + packages/views/locales/zh-Hans/chat.json | 16 + 4 files changed, 391 insertions(+), 165 deletions(-) diff --git a/packages/views/chat/components/chat-window.tsx b/packages/views/chat/components/chat-window.tsx index 9c7c31d30..a0d752dc6 100644 --- a/packages/views/chat/components/chat-window.tsx +++ b/packages/views/chat/components/chat-window.tsx @@ -3,8 +3,9 @@ import React, { useCallback, useEffect, useMemo, useRef, useState } from "react"; import { useQuery, useQueryClient } from "@tanstack/react-query"; import { motion } from "motion/react"; -import { Minus, Maximize2, Minimize2, ChevronDown, ChevronRight, Plus, Check, Trash2, Pencil } from "lucide-react"; +import { Minus, Maximize2, Minimize2, ChevronDown, ChevronRight, Plus, Check, Trash2, Pencil, Loader2, Square } from "lucide-react"; import { Button } from "@multica/ui/components/ui/button"; +import { cn } from "@multica/ui/lib/utils"; import { Tooltip, TooltipTrigger, TooltipContent } from "@multica/ui/components/ui/tooltip"; import { DropdownMenu, @@ -16,15 +17,10 @@ import { DropdownMenuTrigger, } from "@multica/ui/components/ui/dropdown-menu"; import { - AlertDialog, - AlertDialogAction, - AlertDialogCancel, - AlertDialogContent, - AlertDialogDescription, - AlertDialogFooter, - AlertDialogHeader, - AlertDialogTitle, -} from "@multica/ui/components/ui/alert-dialog"; + Popover, + PopoverContent, + PopoverTrigger, +} from "@multica/ui/components/ui/popover"; import { useWorkspaceId } from "@multica/core/hooks"; import { useAuthStore } from "@multica/core/auth"; import { agentListOptions, memberListOptions } from "@multica/core/workspace/queries"; @@ -60,7 +56,7 @@ import { import { ChatResizeHandles } from "./chat-resize-handles"; import { useChatResize } from "./use-chat-resize"; import { createLogger } from "@multica/core/logger"; -import type { Agent, ChatMessage, ChatPendingTask, ChatSession } from "@multica/core/types"; +import type { Agent, ChatMessage, ChatPendingTask, ChatSession, PendingChatTasksResponse } from "@multica/core/types"; import { useT } from "../../i18n"; const uiLogger = createLogger("chat.ui"); @@ -730,8 +726,14 @@ function SessionDropdown({ return { active, archived }; }, [sessions]); + const [isHistoryOpen, setIsHistoryOpen] = useState(false); const [showArchived, setShowArchived] = useState(false); - const [pendingDelete, setPendingDelete] = useState(null); + const [confirmingDeleteId, setConfirmingDeleteId] = useState(null); + const [confirmingStopId, setConfirmingStopId] = useState(null); + const [stoppingTaskId, setStoppingTaskId] = useState(null); + const [completedFlashIds, setCompletedFlashIds] = useState>(() => new Set()); + const previousInFlightRef = useRef>(new Set()); + const completedFlashTimersRef = useRef>>(new Map()); // Inline rename: only one row can be in edit mode at a time. We track the // session id (not the full session) so a stale closure can't overwrite a // newer rename pulled in via WS. @@ -739,16 +741,64 @@ function SessionDropdown({ const deleteSession = useDeleteChatSession(); const updateSession = useUpdateChatSession(); const setActiveSession = useChatStore((s) => s.setActiveSession); + const queryClient = useQueryClient(); const formatTimeAgo = useFormatTimeAgo(); // Aggregate "which sessions have an in-flight task right now". Reuses // the same workspace-scoped query the FAB consumes, so toggling the chat // window doesn't fire a second request — TanStack dedupes by key. const { data: pending } = useQuery(pendingChatTasksOptions(wsId)); - const inFlightSessionIds = useMemo( - () => new Set((pending?.tasks ?? []).map((t) => t.chat_session_id)), + const pendingTaskBySessionId = useMemo( + () => new Map((pending?.tasks ?? []).map((task) => [task.chat_session_id, task])), [pending], ); + const inFlightSessionIds = useMemo( + () => new Set(pendingTaskBySessionId.keys()), + [pendingTaskBySessionId], + ); + + useEffect(() => { + const previous = previousInFlightRef.current; + const unreadSessionIds = new Set(sessions.filter((s) => s.has_unread).map((s) => s.id)); + + for (const sessionId of previous) { + if (inFlightSessionIds.has(sessionId) || !unreadSessionIds.has(sessionId)) continue; + + setCompletedFlashIds((current) => { + if (current.has(sessionId)) return current; + return new Set(current).add(sessionId); + }); + + const existingTimer = completedFlashTimersRef.current.get(sessionId); + if (existingTimer) clearTimeout(existingTimer); + + const timer = setTimeout(() => { + setCompletedFlashIds((current) => { + if (!current.has(sessionId)) return current; + const next = new Set(current); + next.delete(sessionId); + return next; + }); + completedFlashTimersRef.current.delete(sessionId); + }, 1600); + completedFlashTimersRef.current.set(sessionId, timer); + } + + previousInFlightRef.current = inFlightSessionIds; + }, [inFlightSessionIds, sessions]); + + useEffect(() => { + const timers = completedFlashTimersRef.current; + return () => { + for (const timer of timers.values()) clearTimeout(timer); + timers.clear(); + }; + }, []); + + useEffect(() => { + if (!confirmingStopId || pendingTaskBySessionId.has(confirmingStopId)) return; + setConfirmingStopId(null); + }, [confirmingStopId, pendingTaskBySessionId]); // Cross-session aggregate signal for the closed-dropdown trigger. // "Active" here means there's something interesting happening in a @@ -763,16 +813,18 @@ function SessionDropdown({ (s) => s.id !== activeSessionId && s.has_unread, ); - const handleConfirmDelete = () => { - if (!pendingDelete) return; - const sessionId = pendingDelete.id; + const handleConfirmDelete = (session: ChatSession) => { + const sessionId = session.id; + const isDeletingCurrent = activeSessionId === sessionId; // Eager local clear when the user is deleting the session they're // currently looking at — otherwise messages / pendingTask queries // keep rendering the now-deleted session until chat:session_deleted // arrives over WS (~50–200ms gap). - if (activeSessionId === sessionId) setActiveSession(null); + if (isDeletingCurrent) { + setActiveSession(null); + } deleteSession.mutate(sessionId, { - onSettled: () => setPendingDelete(null), + onSettled: () => setConfirmingDeleteId(null), }); }; @@ -787,26 +839,91 @@ function SessionDropdown({ updateSession.mutate({ sessionId, title: trimmed }); }; + const handleSelectSession = (session: ChatSession) => { + onSelectSession(session); + setIsHistoryOpen(false); + }; + + const handleConfirmStop = (session: ChatSession, task: PendingChatTasksResponse["tasks"][number]) => { + setStoppingTaskId(task.task_id); + previousInFlightRef.current = new Set( + [...previousInFlightRef.current].filter((sessionId) => sessionId !== session.id), + ); + + // Same optimistic behavior as the active chat Stop button: remove the + // running affordance immediately, then let task:cancelled / refetches + // converge every open surface on the server truth. + queryClient.setQueryData(chatKeys.pendingTasks(wsId), (current) => { + if (!current) return current; + return { + ...current, + tasks: current.tasks.filter((item) => item.task_id !== task.task_id), + }; + }); + queryClient.setQueryData(chatKeys.pendingTask(session.id), {}); + queryClient.invalidateQueries({ queryKey: chatKeys.messages(session.id) }); + + api.cancelTaskById(task.task_id).then( + () => apiLogger.info("cancelTask.success (history row)", { taskId: task.task_id, sessionId: session.id }), + (err) => + apiLogger.warn("cancelTask.error (history row; task may have already finished)", { + taskId: task.task_id, + sessionId: session.id, + err, + }), + ).finally(() => { + queryClient.invalidateQueries({ queryKey: chatKeys.pendingTasks(wsId) }); + queryClient.invalidateQueries({ queryKey: chatKeys.pendingTask(session.id) }); + setStoppingTaskId(null); + setConfirmingStopId(null); + }); + }; + const renderRow = (session: ChatSession) => { const isCurrent = session.id === activeSessionId; const agent = agentById.get(session.agent_id) ?? null; - const isRunning = inFlightSessionIds.has(session.id); + const pendingTask = pendingTaskBySessionId.get(session.id); + const isRunning = !!pendingTask; + const showCompleted = completedFlashIds.has(session.id) && !isCurrent; + const showUnread = session.has_unread && !isCurrent; const isRenaming = renamingId === session.id; + const isConfirmingDelete = confirmingDeleteId === session.id; + const isConfirmingStop = confirmingStopId === session.id && !!pendingTask; + const isConfirmingAction = isConfirmingDelete || isConfirmingStop; + const titleText = session.title?.trim() || t(($) => $.window.untitled); + const trailingStatus = isRunning + ? t(($) => $.session_history.row_subtitle.working) + : showCompleted + ? t(($) => $.session_history.row_subtitle.completed) + : showUnread + ? t(($) => $.session_history.row_subtitle.new_reply) + : session.status === "archived" + ? t(($) => $.session_history.row_subtitle.archived_label) + : formatTimeAgo(session.updated_at); + return ( - { - if (isRenaming) return; - onSelectSession(session); + if (isRenaming || isConfirmingAction) return; + handleSelectSession(session); }} - className="group flex min-w-0 items-center gap-2" + onKeyDown={(e) => { + if (isRenaming || isConfirmingAction) return; + if (e.key !== "Enter" && e.key !== " ") return; + e.preventDefault(); + handleSelectSession(session); + }} + className={cn( + "group/history-row relative flex min-h-11 min-w-0 cursor-default items-center gap-2 overflow-hidden rounded-md py-1.5 pl-2 pr-2 outline-none transition-colors hover:bg-accent/60 focus-visible:bg-accent/60 focus-visible:ring-1 focus-visible:ring-ring", + isCurrent && "bg-accent/70", + isConfirmingAction && "bg-destructive/5 hover:bg-destructive/5", + session.status === "archived" && "opacity-75", + )} > + {isCurrent && } {agent ? ( )} -
+
{isRenaming ? ( handleSubmitRename(session.id, value)} onCancel={() => setRenamingId(null)} /> + ) : isConfirmingDelete ? ( +
+ {t(($) => $.session_history.delete_dialog.title)} +
+ ) : isConfirmingStop ? ( +
+ {t(($) => $.session_history.stop_dialog.title)} +
) : ( - <> -
- {session.title?.trim() || t(($) => $.window.untitled)} -
-
- {formatTimeAgo(session.updated_at)} -
- +
+ {titleText} +
)}
- {/* Right-edge status pip: in-flight wins over unread because - * "still working" is more actionable than "has reply" — and - * the two rarely coexist in practice (the unread flag fires - * on chat_message write, by which point the task has just - * finished). Same pip shape as unread for visual rhythm, - * amber + pulse to read as activity. - * - * Hidden while renaming so the inline input has room to - * breathe and trailing pips don't visually trail off-screen - * next to the editor caret. */} - {!isRenaming && isRunning ? ( - $.window.running)} - title={t(($) => $.window.running)} - className="size-1.5 shrink-0 rounded-full bg-amber-500 animate-pulse" - /> - ) : !isRenaming && session.has_unread ? ( - $.window.unread)} - title={t(($) => $.window.unread)} - className="size-1.5 shrink-0 rounded-full bg-brand" - /> - ) : null} - {!isRenaming && isCurrent && ( - - )} {!isRenaming && ( - <> - - - + isConfirmingDelete ? ( +
+ + +
+ ) : isConfirmingStop && pendingTask ? ( +
+ + +
+ ) : ( +
+
+ {isRunning && } + {showCompleted && !isRunning && } + {showUnread && !isRunning && !showCompleted && ( + $.window.unread)} + title={t(($) => $.window.unread)} + className="size-1.5 rounded-full bg-brand" + /> + )} + {trailingStatus} +
+
+ {isRunning && pendingTask && ( + + )} + + +
+
+ ) )} - +
); }; return ( <> - - + + {triggerAgent && ( {title} {otherSessionRunning ? ( - $.window.another_running)} - title={t(($) => $.window.another_running)} - className="size-1.5 shrink-0 rounded-full bg-amber-500 animate-pulse" + className="size-3.5 shrink-0 animate-spin text-muted-foreground" /> ) : otherSessionUnread ? ( ) : null} - - + e.stopPropagation()} > {sessions.length === 0 ? (
@@ -941,20 +1154,24 @@ function SessionDropdown({ ) : ( <> {active.length > 0 && ( - - {t(($) => $.window.active_group)} +
$.window.active_group)}> +
+ {t(($) => $.window.active_group)} +
{active.map(renderRow)} - +
)} {archived.length > 0 && ( <> - {active.length > 0 && } - 0 &&
} + {showArchived && ( - +
$.window.archived_group, { count: archived.length })}> {archived.map(renderRow)} - +
)} )} )} - - - - { - if (!open && !deleteSession.isPending) setPendingDelete(null); - }} - > - - - - {t(($) => $.session_history.delete_dialog.title)} - - - {pendingDelete?.title - ? t(($) => $.session_history.delete_dialog.description_with_title, { - title: pendingDelete.title, - }) - : t(($) => $.session_history.delete_dialog.description_default)} - - - - - {t(($) => $.session_history.delete_dialog.cancel)} - - - {deleteSession.isPending - ? t(($) => $.session_history.delete_dialog.confirming) - : t(($) => $.session_history.delete_dialog.confirm)} - - - - + + ); } @@ -1022,13 +1203,10 @@ function SessionDropdown({ * into the existing text. Enter commits, Escape cancels, a real click * outside the input also commits. * - * We do NOT commit on the input's `blur` event: Base UI's Menu uses - * focus-follows-cursor (hovering a sibling row drags DOM focus there), - * so a blur handler would fire on every mouse-move and "save" the user's - * half-typed title without them clicking anywhere. Instead a document- - * level `pointerdown` listener — registered in capture phase so it runs - * before Base UI's outside-click close handler — commits when the user - * actually clicks outside the input. + * We do NOT commit on the input's `blur` event: the history popover can + * move focus to sibling rows and nested actions while the user is still + * interacting with the panel. Instead a document-level `pointerdown` + * listener commits only when the user actually clicks outside the input. */ function SessionRenameInput({ initialValue, @@ -1061,9 +1239,8 @@ function SessionRenameInput({ if (input.contains(e.target as Node)) return; onSubmitRef.current(valueRef.current); }; - // Capture phase — Base UI registers its own outside-click handler in - // bubble; running first lets us commit before the menu starts to - // close (and unmount this component). + // Capture phase — commit before outside-click handling can close the + // popover and unmount this component. document.addEventListener("pointerdown", handlePointerDown, true); return () => { document.removeEventListener("pointerdown", handlePointerDown, true); @@ -1081,7 +1258,8 @@ function SessionRenameInput({ onClick={(e) => e.stopPropagation()} onPointerDown={(e) => e.stopPropagation()} onKeyDown={(e) => { - // Stop the menu from stealing arrow / typeahead / space input. + // Keep editing keys inside the input instead of letting the row + // selection keyboard handler consume them. e.stopPropagation(); if (e.key === "Enter") { e.preventDefault(); diff --git a/packages/views/locales/en/chat.json b/packages/views/locales/en/chat.json index 4d1d81c9d..6c75aae95 100644 --- a/packages/views/locales/en/chat.json +++ b/packages/views/locales/en/chat.json @@ -38,6 +38,22 @@ }, "row_delete_aria": "Delete chat session", "row_rename_aria": "Rename chat session", + "row_stop_aria": "Stop current run", + "stop_action": "Stop", + "row_subtitle": { + "working": "Working", + "completed": "Completed", + "new_reply": "New reply", + "archived_label": "Archived" + }, + "stop_dialog": { + "title": "Stop this run?", + "description_with_title": "This will cancel the current agent task in \"{{title}}\". Partial output will stay in the chat.", + "description_default": "This will cancel the current agent task. Partial output will stay in the chat.", + "cancel": "Keep running", + "confirm": "Stop run", + "confirming": "Stopping..." + }, "delete_dialog": { "title": "Delete chat session", "description_with_title": "\"{{title}}\" and its messages will be permanently removed. This action cannot be undone.", diff --git a/packages/views/locales/ko/chat.json b/packages/views/locales/ko/chat.json index 7161b0000..8364a85c3 100644 --- a/packages/views/locales/ko/chat.json +++ b/packages/views/locales/ko/chat.json @@ -38,6 +38,22 @@ }, "row_delete_aria": "채팅 세션 삭제", "row_rename_aria": "채팅 세션 이름 변경", + "row_stop_aria": "현재 실행 중지", + "stop_action": "중지", + "row_subtitle": { + "working": "작업 중", + "completed": "완료됨", + "new_reply": "새 답변", + "archived_label": "보관됨" + }, + "stop_dialog": { + "title": "이 실행을 중지할까요?", + "description_with_title": "\"{{title}}\"의 현재 에이전트 작업을 취소합니다. 이미 생성된 내용은 채팅에 남습니다.", + "description_default": "현재 에이전트 작업을 취소합니다. 이미 생성된 내용은 채팅에 남습니다.", + "cancel": "계속 실행", + "confirm": "실행 중지", + "confirming": "중지하는 중..." + }, "delete_dialog": { "title": "채팅 세션 삭제", "description_with_title": "\"{{title}}\"과(와) 메시지가 영구 삭제됩니다. 이 작업은 되돌릴 수 없습니다.", diff --git a/packages/views/locales/zh-Hans/chat.json b/packages/views/locales/zh-Hans/chat.json index c4e440b03..2b8be3e1a 100644 --- a/packages/views/locales/zh-Hans/chat.json +++ b/packages/views/locales/zh-Hans/chat.json @@ -35,6 +35,22 @@ }, "row_delete_aria": "删除对话", "row_rename_aria": "重命名对话", + "row_stop_aria": "停止当前运行", + "stop_action": "停止", + "row_subtitle": { + "working": "运行中", + "completed": "已完成", + "new_reply": "新回复", + "archived_label": "已归档" + }, + "stop_dialog": { + "title": "停止这次运行？", + "description_with_title": "这会取消 \"{{title}}\" 里的当前智能体任务。已产生的内容会保留在对话里。", + "description_default": "这会取消当前智能体任务。已产生的内容会保留在对话里。", + "cancel": "继续运行", + "confirm": "停止运行", + "confirming": "停止中..." + }, "delete_dialog": { "title": "删除对话", "description_with_title": "\"{{title}}\" 及其消息会被永久删除，无法撤销。", From 3c8645e546406773f0fa17980504926f6e6ce69e Mon Sep 17 00:00:00 2001 From: Naiyuan Qing <145280634+NevilleQingNY@users.noreply.github.com> Date: Mon, 1 Jun 2026 12:51:15 +0800 Subject: [PATCH 2/6] feat(cli): add squad member set-role (#3583) Co-authored-by: multica-agent --- apps/docs/content/docs/cli.ko.mdx | 2 +- apps/docs/content/docs/cli.mdx | 2 +- apps/docs/content/docs/cli.zh.mdx | 2 +- apps/docs/content/docs/squads.ko.mdx | 1 + apps/docs/content/docs/squads.mdx | 1 + apps/docs/content/docs/squads.zh.mdx | 1 + server/cmd/multica/cmd_squad.go | 57 ++++++++++ server/cmd/multica/cmd_squad_test.go | 107 ++++++++++++++++++ .../internal/daemon/execenv/execenv_test.go | 1 + .../internal/daemon/execenv/runtime_config.go | 2 + 10 files changed, 173 insertions(+), 3 deletions(-) create mode 100644 server/cmd/multica/cmd_squad_test.go diff --git a/apps/docs/content/docs/cli.ko.mdx b/apps/docs/content/docs/cli.ko.mdx index f4f47b395..6f32b5988 100644 --- a/apps/docs/content/docs/cli.ko.mdx +++ b/apps/docs/content/docs/cli.ko.mdx @@ -88,7 +88,7 @@ CI나 headless 환경에서는 브라우저 플로우를 건너뛰세요. 웹 | `multica squad create --name "..." --leader ` | 스쿼드 생성(owner / admin) | | `multica squad update ...` | 이름, 설명, 지침, 리더, 또는 아바타 업데이트 | | `multica squad delete ` | 보관(소프트 삭제) — 할당된 이슈를 리더에게 이관 | -| `multica squad member list/add/remove ` | 스쿼드 멤버 관리 | +| `multica squad member list/add/remove/set-role ` | 스쿼드 멤버 관리 및 역할 직접 업데이트 | | `multica squad activity --reason "..."` | 스쿼드 리더 에이전트가 매 턴마다 평가를 기록할 때 사용 | 전체 모델은 [스쿼드](/squads)를 참고하세요. diff --git a/apps/docs/content/docs/cli.mdx b/apps/docs/content/docs/cli.mdx index 94f3ec63a..2536fb777 100644 --- a/apps/docs/content/docs/cli.mdx +++ b/apps/docs/content/docs/cli.mdx @@ -88,7 +88,7 @@ For the difference between token types, see [Authentication and tokens](/auth-to | `multica squad create --name "..." --leader ` | Create a squad (owner / admin) | | `multica squad update ...` | Update name, description, instructions, leader, or avatar | | `multica squad delete ` | Archive (soft-delete) — transfers assigned issues to the leader | -| `multica squad member list/add/remove ` | Manage squad members | +| `multica squad member list/add/remove/set-role ` | Manage squad members and update roles in place | | `multica squad activity --reason "..."` | Used by squad leader agents to record an evaluation per turn | See [Squads](/squads) for the full model. diff --git a/apps/docs/content/docs/cli.zh.mdx b/apps/docs/content/docs/cli.zh.mdx index 961fb2fe0..1f8b0e660 100644 --- a/apps/docs/content/docs/cli.zh.mdx +++ b/apps/docs/content/docs/cli.zh.mdx @@ -88,7 +88,7 @@ Token 类型的详细区分见 [认证与令牌](/auth-tokens)。 | `multica squad create --name "..." --leader ` | 创建小队（owner / admin）| | `multica squad update ...` | 修改名字、描述、instructions、队长、头像 | | `multica squad delete ` | 归档（软删除）—— 同时把分配给小队的 issue 转给队长 | -| `multica squad member list/add/remove ` | 管理小队成员 | +| `multica squad member list/add/remove/set-role ` | 管理小队成员并原地更新 role | | `multica squad activity --reason "..."` | 队长智能体每轮结束时调用，记录 evaluation | 完整模型见 [小队](/squads)。 diff --git a/apps/docs/content/docs/squads.ko.mdx b/apps/docs/content/docs/squads.ko.mdx index 69a3c63de..1a18111f5 100644 --- a/apps/docs/content/docs/squads.ko.mdx +++ b/apps/docs/content/docs/squads.ko.mdx @@ -123,6 +123,7 @@ multica squad member add --member-id --type agen | `multica squad delete ` | 보관(소프트 삭제) — 할당된 이슈를 리더에게 이전 | | `multica squad member list ` | 스쿼드의 멤버 목록 표시 | | `multica squad member add --member-id --type agent\|member [--role "..."]` | 멤버 추가(owner / admin) | +| `multica squad member set-role --member-id --member-type agent\|member --role "..."` | 멤버를 제거하지 않고 역할 변경 | | `multica squad member remove --member-id --type agent\|member` | 멤버 제거(리더는 제거할 수 없습니다 — 먼저 리더를 변경하세요) | | `multica squad activity --reason "..."` | 리더 에이전트가 매 턴 종료 시 기록 | diff --git a/apps/docs/content/docs/squads.mdx b/apps/docs/content/docs/squads.mdx index 749b3a61b..f8eb8323b 100644 --- a/apps/docs/content/docs/squads.mdx +++ b/apps/docs/content/docs/squads.mdx @@ -123,6 +123,7 @@ There is currently no unarchive command; create a new squad if you need the rout | `multica squad delete ` | Archive (soft-delete) — transfers assigned issues to the leader | | `multica squad member list ` | List a squad's members | | `multica squad member add --member-id --type agent\|member [--role "..."]` | Add a member (owner / admin) | +| `multica squad member set-role --member-id --member-type agent\|member --role "..."` | Change a member's role without removing it | | `multica squad member remove --member-id --type agent\|member` | Remove a member (the leader cannot be removed — change leader first) | | `multica squad activity --reason "..."` | Recorded by the leader agent at the end of every turn | diff --git a/apps/docs/content/docs/squads.zh.mdx b/apps/docs/content/docs/squads.zh.mdx index 8b2d97f8a..b13a54d33 100644 --- a/apps/docs/content/docs/squads.zh.mdx +++ b/apps/docs/content/docs/squads.zh.mdx @@ -123,6 +123,7 @@ multica squad member add --member-id --type agen | `multica squad delete ` | 归档（软删除）——同时把当前分配给小队的 issue 转给队长 | | `multica squad member list ` | 列出小队成员 | | `multica squad member add --member-id --type agent\|member [--role "..."]` | 加成员（owner / admin）| +| `multica squad member set-role --member-id --member-type agent\|member --role "..."` | 不移除成员，直接修改 role | | `multica squad member remove --member-id --type agent\|member` | 移除成员（**不能移除队长**——先换队长）| | `multica squad activity --reason "..."` | 队长每次结束前由它自己调用 | diff --git a/server/cmd/multica/cmd_squad.go b/server/cmd/multica/cmd_squad.go index 5e9617aaa..e081ccf50 100644 --- a/server/cmd/multica/cmd_squad.go +++ b/server/cmd/multica/cmd_squad.go @@ -344,6 +344,56 @@ func runSquadMemberAdd(cmd *cobra.Command, args []string) error { return nil } +// ── Member Set Role ───────────────────────────────────────────────────────── + +var squadMemberSetRoleCmd = &cobra.Command{ + Use: "set-role ", + Short: "Change a squad member's role", + Args: exactArgs(1), + RunE: runSquadMemberSetRole, +} + +func runSquadMemberSetRole(cmd *cobra.Command, args []string) error { + memberID, _ := cmd.Flags().GetString("member-id") + memberType, _ := cmd.Flags().GetString("member-type") + role, _ := cmd.Flags().GetString("role") + + if memberID == "" { + return fmt.Errorf("--member-id is required") + } + if memberType != "agent" && memberType != "member" { + return fmt.Errorf("--member-type must be 'agent' or 'member'") + } + if role == "" { + return fmt.Errorf("--role is required") + } + + client, err := newAPIClient(cmd) + if err != nil { + return err + } + ctx, cancel := context.WithTimeout(context.Background(), 15*time.Second) + defer cancel() + + body := map[string]any{ + "member_type": memberType, + "member_id": memberID, + "role": role, + } + + var result map[string]any + if err := client.PatchJSON(ctx, "/api/squads/"+args[0]+"/members/role", body, &result); err != nil { + return fmt.Errorf("set member role: %w", err) + } + + output, _ := cmd.Flags().GetString("output") + if output == "json" { + return cli.PrintJSON(os.Stdout, result) + } + fmt.Fprintf(os.Stderr, "Member %s role updated to %s.\n", memberID, role) + return nil +} + // ── Member Remove ─────────────────────────────────────────────────────────── var squadMemberRemoveCmd = &cobra.Command{ @@ -487,6 +537,12 @@ func init() { squadMemberRemoveCmd.Flags().String("type", "agent", "Member type: agent or member") squadMemberRemoveCmd.Flags().String("output", "table", "Output format: table or json") + // member set-role + squadMemberSetRoleCmd.Flags().String("member-id", "", "Member or agent ID (required)") + squadMemberSetRoleCmd.Flags().String("member-type", "agent", "Member type: agent or member") + squadMemberSetRoleCmd.Flags().String("role", "", "New role in the squad (required)") + squadMemberSetRoleCmd.Flags().String("output", "json", "Output format: table or json") + // activity squadActivityCmd.Flags().String("reason", "", "Short explanation of the decision") squadActivityCmd.Flags().String("output", "table", "Output format: table or json") @@ -494,6 +550,7 @@ func init() { squadMemberCmd.AddCommand(squadMemberListCmd) squadMemberCmd.AddCommand(squadMemberAddCmd) squadMemberCmd.AddCommand(squadMemberRemoveCmd) + squadMemberCmd.AddCommand(squadMemberSetRoleCmd) squadCmd.AddCommand(squadListCmd) squadCmd.AddCommand(squadGetCmd) diff --git a/server/cmd/multica/cmd_squad_test.go b/server/cmd/multica/cmd_squad_test.go new file mode 100644 index 000000000..49d82324a --- /dev/null +++ b/server/cmd/multica/cmd_squad_test.go @@ -0,0 +1,107 @@ +package main + +import ( + "encoding/json" + "net/http" + "net/http/httptest" + "testing" + + "github.com/spf13/cobra" +) + +func newSquadMemberSetRoleTestCmd() *cobra.Command { + cmd := &cobra.Command{Use: "set-role"} + cmd.Flags().String("server-url", "", "") + cmd.Flags().String("workspace-id", "", "") + cmd.Flags().String("profile", "", "") + cmd.Flags().String("member-id", "", "") + cmd.Flags().String("member-type", "agent", "") + cmd.Flags().String("role", "", "") + cmd.Flags().String("output", "json", "") + return cmd +} + +func TestSquadMemberSetRoleCommandIsRegistered(t *testing.T) { + cmd, _, err := squadMemberCmd.Find([]string{"set-role", "squad-123"}) + if err != nil { + t.Fatalf("find set-role command: %v", err) + } + if cmd == nil || cmd.Name() != "set-role" { + t.Fatalf("set-role command not registered; got %#v", cmd) + } + for _, flag := range []string{"member-id", "member-type", "role", "output"} { + if cmd.Flags().Lookup(flag) == nil { + t.Fatalf("set-role command missing --%s flag", flag) + } + } +} + +func TestRunSquadMemberSetRolePatchesRole(t *testing.T) { + t.Setenv("HOME", t.TempDir()) + t.Setenv("MULTICA_TOKEN", "test-token") + t.Setenv("MULTICA_WORKSPACE_ID", "workspace-123") + + var gotMethod, gotPath string + var gotBody map[string]any + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + gotMethod = r.Method + gotPath = r.URL.Path + if err := json.NewDecoder(r.Body).Decode(&gotBody); err != nil { + t.Fatalf("decode request body: %v", err) + } + if r.Header.Get("X-Workspace-ID") != "workspace-123" { + t.Fatalf("X-Workspace-ID = %q, want workspace-123", r.Header.Get("X-Workspace-ID")) + } + w.Header().Set("Content-Type", "application/json") + _ = json.NewEncoder(w).Encode(map[string]any{ + "squad_id": "squad-123", + "member_id": "member-456", + "member_type": "agent", + "role": "reviewer", + }) + })) + defer srv.Close() + t.Setenv("MULTICA_SERVER_URL", srv.URL) + + cmd := newSquadMemberSetRoleTestCmd() + _ = cmd.Flags().Set("member-id", "member-456") + _ = cmd.Flags().Set("member-type", "agent") + _ = cmd.Flags().Set("role", "reviewer") + _ = cmd.Flags().Set("output", "json") + + if err := runSquadMemberSetRole(cmd, []string{"squad-123"}); err != nil { + t.Fatalf("runSquadMemberSetRole: %v", err) + } + if gotMethod != http.MethodPatch { + t.Fatalf("method = %s, want PATCH", gotMethod) + } + if gotPath != "/api/squads/squad-123/members/role" { + t.Fatalf("path = %q, want /api/squads/squad-123/members/role", gotPath) + } + wantBody := map[string]any{"member_id": "member-456", "member_type": "agent", "role": "reviewer"} + for k, want := range wantBody { + if gotBody[k] != want { + t.Fatalf("body[%s] = %v, want %v (full body: %#v)", k, gotBody[k], want, gotBody) + } + } +} + +func TestRunSquadMemberSetRoleValidatesRequiredFlags(t *testing.T) { + cmd := newSquadMemberSetRoleTestCmd() + if err := runSquadMemberSetRole(cmd, []string{"squad-123"}); err == nil { + t.Fatal("expected missing --member-id error") + } + + cmd = newSquadMemberSetRoleTestCmd() + _ = cmd.Flags().Set("member-id", "member-456") + _ = cmd.Flags().Set("member-type", "invalid") + if err := runSquadMemberSetRole(cmd, []string{"squad-123"}); err == nil { + t.Fatal("expected invalid --member-type error") + } + + cmd = newSquadMemberSetRoleTestCmd() + _ = cmd.Flags().Set("member-id", "member-456") + if err := runSquadMemberSetRole(cmd, []string{"squad-123"}); err == nil { + t.Fatal("expected missing --role error") + } +} diff --git a/server/internal/daemon/execenv/execenv_test.go b/server/internal/daemon/execenv/execenv_test.go index bcaf2c6a2..b95b15987 100644 --- a/server/internal/daemon/execenv/execenv_test.go +++ b/server/internal/daemon/execenv/execenv_test.go @@ -613,6 +613,7 @@ func TestInjectRuntimeConfigAvailableCommandsCoreOnly(t *testing.T) { "multica issue status ", "multica issue comment add ", "multica issue comment add --help", + "multica squad member set-role ", } { if !strings.Contains(s, want) { t.Errorf("AGENTS.md missing core command/help text %q\n---\n%s", want, s) diff --git a/server/internal/daemon/execenv/runtime_config.go b/server/internal/daemon/execenv/runtime_config.go index 78d30d2df..1430defcb 100644 --- a/server/internal/daemon/execenv/runtime_config.go +++ b/server/internal/daemon/execenv/runtime_config.go @@ -452,6 +452,8 @@ func buildMetaSkillContent(provider string, ctx TaskContextForEnv) string { b.WriteString("- `multica issue metadata list [--output json]` — List every metadata key pinned to an issue. Empty `{}` is normal.\n") b.WriteString("- `multica issue metadata set --key --value [--type string|number|bool]` — Pin (or overwrite) a single metadata key. The CLI auto-infers JSON primitives, so URLs and plain text are stored as strings — pass `--type number` or `--type bool` only when the semantic type matters.\n") b.WriteString("- `multica issue metadata delete --key ` — Remove a metadata key.\n\n") + b.WriteString("### Squad maintenance\n") + b.WriteString("- `multica squad member set-role --member-id --member-type --role [--output json]` — Change a squad member role in place; use this instead of remove+add when only the role changes.\n\n") if provider == "codex" { b.WriteString("## Codex-Specific Comment Formatting\n\n") From 41a1ca58add47f53bb64ddc6aa02be2d9a73faa9 Mon Sep 17 00:00:00 2001 From: Naiyuan Qing <145280634+NevilleQingNY@users.noreply.github.com> Date: Mon, 1 Jun 2026 13:20:21 +0800 Subject: [PATCH 3/6] fix(chat): clean up history indicators (#3592) --- .../views/chat/components/chat-window.tsx | 77 +++++++++++-------- 1 file changed, 45 insertions(+), 32 deletions(-) diff --git a/packages/views/chat/components/chat-window.tsx b/packages/views/chat/components/chat-window.tsx index a0d752dc6..10125540b 100644 --- a/packages/views/chat/components/chat-window.tsx +++ b/packages/views/chat/components/chat-window.tsx @@ -800,18 +800,16 @@ function SessionDropdown({ setConfirmingStopId(null); }, [confirmingStopId, pendingTaskBySessionId]); - // Cross-session aggregate signal for the closed-dropdown trigger. - // "Active" here means there's something interesting happening in a - // session OTHER than the one the user is currently looking at — the - // user already sees their own session's state via the StatusPill / - // unread auto-mark, so highlighting it on the trigger would be noise. - // Same priority rule as the row pips: running > unread. - const otherSessionRunning = sessions.some( + // Header state split: + // - inside the trigger: the current chat's own live state + // - beside the trigger: aggregate activity from other chats + const currentSessionRunning = activeSessionId ? inFlightSessionIds.has(activeSessionId) : false; + const otherRunningCount = sessions.filter( (s) => s.id !== activeSessionId && inFlightSessionIds.has(s.id), - ); - const otherSessionUnread = sessions.some( + ).length; + const otherUnreadCount = sessions.filter( (s) => s.id !== activeSessionId && s.has_unread, - ); + ).length; const handleConfirmDelete = (session: ChatSession) => { const sessionId = session.id; @@ -1039,7 +1037,7 @@ function SessionDropdown({
) : (
-
+
{isRunning && } {showCompleted && !isRunning && } {showUnread && !isRunning && !showCompleted && ( @@ -1051,7 +1049,7 @@ function SessionDropdown({ )} {trailingStatus}
-
+
{isRunning && pendingTask && (
Date: Mon, 1 Jun 2026 08:52:41 +0300 Subject: [PATCH 4/6] fix(agent): drain claude stdout while writing prompt to stdin (#3490) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The claude backend wrote the full prompt to the child's stdin and closed it before starting the stdout reader goroutine. With --verbose --output-format stream-json the CLI emits a startup banner before reading its first stdin frame; with no reader draining stdout, the child blocks on its stdout write, never reads stdin, and our stdin Write blocks until the per-task context fires. The field symptom is tasks failing exactly at the 2 h per-task timeout with "write |1: The pipe has been ended." Move writeClaudeInput into its own goroutine so the prompt write and the stdout drain proceed concurrently. Guard stdin close with sync.Once (it can now be called from both the writer goroutine and, previously, the result handler). Join the write result at cmd.Wait() and surface a write failure as a "failed" status only when no result event arrived and no session was established, so a genuine startup death still reports the stderr tail. Add a regression test that re-execs the test binary as a fake claude which bursts 256 KiB to stdout before reading stdin, with a 128 KiB prompt pushed at stdin — both past any plausible OS pipe buffer — so a regression hangs until the test deadline instead of passing. Co-authored-by: Claude Opus 4.8 --- server/pkg/agent/claude.go | 56 ++++++----- server/pkg/agent/claude_deadlock_test.go | 113 +++++++++++++++++++++++ 2 files changed, 145 insertions(+), 24 deletions(-) create mode 100644 server/pkg/agent/claude_deadlock_test.go diff --git a/server/pkg/agent/claude.go b/server/pkg/agent/claude.go index 2a73ef254..f59f526e5 100644 --- a/server/pkg/agent/claude.go +++ b/server/pkg/agent/claude.go @@ -4,13 +4,13 @@ import ( "bufio" "context" "encoding/json" - "errors" "fmt" "io" "log/slog" "os" "os/exec" "strings" + "sync" "time" ) @@ -78,12 +78,8 @@ func (b *claudeBackend) Execute(ctx context.Context, prompt string, opts ExecOpt cancel() return nil, fmt.Errorf("claude stdin pipe: %w", err) } - closeStdin := func() { - if stdin != nil { - _ = stdin.Close() - stdin = nil - } - } + var closeStdinOnce sync.Once + closeStdin := func() { closeStdinOnce.Do(func() { _ = stdin.Close() }) } // Capture stderr into both the daemon log (as before) and a bounded tail // buffer so we can include the last few KB in Result.Error when claude // exits unexpectedly. Without the tail, an exit-code-only failure looks @@ -97,19 +93,6 @@ func (b *claudeBackend) Execute(ctx context.Context, prompt string, opts ExecOpt cancel() return nil, fmt.Errorf("start claude: %w", err) } - if err := writeClaudeInput(stdin, prompt); err != nil { - // claude almost certainly died during startup (broken pipe). The - // real reason is sitting in stderrBuf — surface it the same way the - // post-handshake error path does, otherwise the daemon log is the - // only place that knows whether it was a V8 abort, a missing native - // module, or anything else. cmd.Wait() flushes os/exec's stderr - // copy goroutine, so stderrBuf.Tail() is safe to read. - closeStdin() - cancel() - _ = cmd.Wait() - return nil, errors.New(withAgentStderr(fmt.Sprintf("write claude input: %v", err), "claude", stderrBuf.Tail())) - } - closeStdin() b.cfg.Logger.Info("claude started", "pid", cmd.Process.Pid, "cwd", opts.Cwd, "model", opts.Model) @@ -119,6 +102,21 @@ func (b *claudeBackend) Execute(ctx context.Context, prompt string, opts ExecOpt msgCh := make(chan Message, 256) resCh := make(chan Result, 1) + // writeClaudeInput runs in its own goroutine so it cannot deadlock + // against the stdout reader. With --verbose --output-format stream-json + // the CLI emits a startup banner before reading its first stdin frame; + // if nothing is draining stdout while we write the prompt, claude blocks + // writing stdout, never reads stdin, and our Write blocks until runCtx + // fires. The field symptom is "write |1: The pipe has been ended." + // surfacing exactly at the per-task timeout when the kill invalidates + // the still-blocked pipe. + writeDone := make(chan error, 1) + go func() { + err := writeClaudeInput(stdin, prompt) + closeStdin() + writeDone <- err + }() + go func() { defer cancel() defer close(msgCh) @@ -165,7 +163,6 @@ func (b *claudeBackend) Execute(ctx context.Context, prompt string, opts ExecOpt } trySend(msgCh, Message{Type: MessageStatus, Status: "running", SessionID: sessionID}) case "result": - closeStdin() sessionID = msg.SessionID if msg.ResultText != "" { output.Reset() @@ -192,14 +189,25 @@ func (b *claudeBackend) Execute(ctx context.Context, prompt string, opts ExecOpt // Wait for process exit exitErr := cmd.Wait() duration := time.Since(startTime) + // writeDone is buffered (cap 1) and the writer always sends — by the + // time cmd has exited, the prompt write has either succeeded, hit a + // broken pipe, or been unblocked by the kill that ended cmd. + writeErr := <-writeDone - if runCtx.Err() == context.DeadlineExceeded { + switch { + case runCtx.Err() == context.DeadlineExceeded: finalStatus = "timeout" finalError = fmt.Sprintf("claude timed out after %s", timeout) - } else if runCtx.Err() == context.Canceled { + case runCtx.Err() == context.Canceled: finalStatus = "aborted" finalError = "execution cancelled" - } else if exitErr != nil && finalStatus == "completed" { + case writeErr != nil && finalStatus == "completed" && sessionID == "": + // No result event landed and the prompt write failed — claude + // died before reading the prompt. Surface the write error; the + // stderr tail attached below carries the real reason. + finalStatus = "failed" + finalError = fmt.Sprintf("write claude input: %v", writeErr) + case exitErr != nil && finalStatus == "completed": finalStatus = "failed" finalError = fmt.Sprintf("claude exited with error: %v", exitErr) } diff --git a/server/pkg/agent/claude_deadlock_test.go b/server/pkg/agent/claude_deadlock_test.go new file mode 100644 index 000000000..7b37d7f99 --- /dev/null +++ b/server/pkg/agent/claude_deadlock_test.go @@ -0,0 +1,113 @@ +package agent + +import ( + "bufio" + "context" + "fmt" + "io" + "log/slog" + "os" + "strings" + "testing" + "time" +) + +// TestMain intercepts when the test binary is re-executed as a fake +// child process by the agent backend. The fake's behavior is selected via +// CLAUDE_FAKE_MODE; absent that env var, this is a normal `go test` run. +func TestMain(m *testing.M) { + switch mode := os.Getenv("CLAUDE_FAKE_MODE"); mode { + case "": + os.Exit(m.Run()) + case "startup_stdout_burst": + runFakeClaudeStartupStdoutBurst() + os.Exit(0) + default: + fmt.Fprintf(os.Stderr, "unknown CLAUDE_FAKE_MODE: %q\n", mode) + os.Exit(2) + } +} + +// runFakeClaudeStartupStdoutBurst writes ~256 KiB to stdout BEFORE +// reading any byte from stdin, then drains stdin and emits a stream-json +// result. Reproduces the stdio deadlock: if the daemon writes the prompt +// to stdin before a stdout reader is running, the child blocks writing +// stdout and the daemon blocks writing stdin — neither side can progress +// until the per-task context times out and the child is killed. +func runFakeClaudeStartupStdoutBurst() { + line := strings.Repeat("x", 1020) + bw := bufio.NewWriter(os.Stdout) + for i := 0; i < 256; i++ { + if _, err := fmt.Fprintf(bw, `{"type":"log","log":{"level":"info","message":"%s"}}`+"\n", line); err != nil { + os.Exit(11) + } + } + if err := bw.Flush(); err != nil { + os.Exit(12) + } + if _, err := io.Copy(io.Discard, os.Stdin); err != nil { + os.Exit(13) + } + fmt.Println(`{"type":"result","subtype":"success","is_error":false,"session_id":"sess-deadlock","result":"done"}`) +} + +// TestClaudeExecuteDoesNotDeadlockOnStartupStdoutBurst verifies that the +// claude backend drains stdout concurrently with writing the prompt to +// stdin. The buggy path serialises the two: writeClaudeInput runs before +// the reader goroutine starts, so a child that emits startup output +// before its first stdin read deadlocks both directions. Field evidence +// in the daemon log shows tasks failing exactly at the 2 h per-task +// timeout with "write |1: The pipe has been ended.", produced when +// runCtx fires, the child is killed, and the blocked stdin Write +// finally unwinds. +// +// The fake child writes 256 KiB to stdout then 128 KiB of prompt is +// pushed at stdin — both well past any plausible OS pipe buffer +// (Linux ~64 KiB, Windows 4-64 KiB) — so a regression here hangs until +// the test deadline rather than passing slowly. +func TestClaudeExecuteDoesNotDeadlockOnStartupStdoutBurst(t *testing.T) { + t.Parallel() + + self, err := os.Executable() + if err != nil { + t.Fatalf("os.Executable: %v", err) + } + + backend, err := New("claude", Config{ + ExecutablePath: self, + Env: map[string]string{"CLAUDE_FAKE_MODE": "startup_stdout_burst"}, + Logger: slog.Default(), + }) + if err != nil { + t.Fatalf("new claude backend: %v", err) + } + + // 128 KiB prompt forces writeClaudeInput to block until the child + // drains stdin, which the buggy code cannot reach because the reader + // goroutine hasn't started yet. + prompt := strings.Repeat("p", 128*1024) + + ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) + defer cancel() + + session, err := backend.Execute(ctx, prompt, ExecOptions{Timeout: 20 * time.Second}) + if err != nil { + t.Fatalf("execute returned error: %v", err) + } + go func() { + for range session.Messages { + } + }() + + select { + case result, ok := <-session.Result: + if !ok { + t.Fatal("result channel closed without a value") + } + if result.Status != "completed" { + t.Fatalf("expected status=completed, got %q (error=%q)", result.Status, result.Error) + } + case <-time.After(15 * time.Second): + t.Fatal("timeout waiting for result — claude backend is deadlocked on writeClaudeInput because stdout is not being drained concurrently") + } +} From 2bb2d13e226058bc69d0e928b69981a95269c6e7 Mon Sep 17 00:00:00 2001 From: Naiyuan Qing <145280634+NevilleQingNY@users.noreply.github.com> Date: Mon, 1 Jun 2026 14:24:46 +0800 Subject: [PATCH 5/6] fix(chat): limit running history actions (#3598) --- .../views/chat/components/chat-window.tsx | 72 ++++++++++--------- 1 file changed, 38 insertions(+), 34 deletions(-) diff --git a/packages/views/chat/components/chat-window.tsx b/packages/views/chat/components/chat-window.tsx index 10125540b..f7b7cd1d9 100644 --- a/packages/views/chat/components/chat-window.tsx +++ b/packages/views/chat/components/chat-window.tsx @@ -1070,40 +1070,44 @@ function SessionDropdown({ {t(($) => $.session_history.stop_action)} )} - - + {!isRunning && ( + <> + + + + )}
) From 2b2888c23abdd8492feba1c47c5ab020d60dbcfd Mon Sep 17 00:00:00 2001 From: Naiyuan Qing <145280634+NevilleQingNY@users.noreply.github.com> Date: Mon, 1 Jun 2026 14:45:16 +0800 Subject: [PATCH 6/6] Handle duplicate skill imports as structured results (#3599) Co-authored-by: multica-agent --- server/cmd/multica/cmd_skill.go | 30 ++++++ server/cmd/multica/cmd_skill_test.go | 98 +++++++++++++++++++ server/internal/handler/skill.go | 34 ++++++- .../handler/skill_import_duplicate_test.go | 48 +++++++++ 4 files changed, 209 insertions(+), 1 deletion(-) create mode 100644 server/cmd/multica/cmd_skill_test.go create mode 100644 server/internal/handler/skill_import_duplicate_test.go diff --git a/server/cmd/multica/cmd_skill.go b/server/cmd/multica/cmd_skill.go index 1e30e1138..90f7d6678 100644 --- a/server/cmd/multica/cmd_skill.go +++ b/server/cmd/multica/cmd_skill.go @@ -4,7 +4,9 @@ import ( "bufio" "context" "encoding/json" + "errors" "fmt" + "net/http" "os" "strings" "time" @@ -346,6 +348,9 @@ func runSkillImport(cmd *cobra.Command, _ []string) error { var result map[string]any if err := client.PostJSON(ctx, "/api/skills/import", body, &result); err != nil { + if handleSkillImportConflict(cmd, err) { + return nil + } return fmt.Errorf("import skill: %w", err) } @@ -358,6 +363,31 @@ func runSkillImport(cmd *cobra.Command, _ []string) error { return nil } +func handleSkillImportConflict(cmd *cobra.Command, err error) bool { + var httpErr *cli.HTTPError + if !errors.As(err, &httpErr) || httpErr.StatusCode != http.StatusConflict || strings.TrimSpace(httpErr.Body) == "" { + return false + } + + var body map[string]any + if json.Unmarshal([]byte(httpErr.Body), &body) != nil { + return false + } + if _, ok := body["existing_skill"]; !ok { + return false + } + + output, _ := cmd.Flags().GetString("output") + if output == "json" { + _ = cli.PrintJSON(os.Stdout, body) + return true + } + + existing, _ := body["existing_skill"].(map[string]any) + fmt.Printf("Skill already exists: %s (%s)\n", strVal(existing, "name"), strVal(existing, "id")) + return true +} + // --------------------------------------------------------------------------- // Skill file subcommands // --------------------------------------------------------------------------- diff --git a/server/cmd/multica/cmd_skill_test.go b/server/cmd/multica/cmd_skill_test.go new file mode 100644 index 000000000..7e1e1c8c1 --- /dev/null +++ b/server/cmd/multica/cmd_skill_test.go @@ -0,0 +1,98 @@ +package main + +import ( + "encoding/json" + "io" + "net/http" + "net/http/httptest" + "os" + "testing" + + "github.com/spf13/cobra" +) + +func newSkillImportTestCmd() *cobra.Command { + cmd := &cobra.Command{Use: "import"} + cmd.Flags().String("server-url", "", "") + cmd.Flags().String("workspace-id", "", "") + cmd.Flags().String("profile", "", "") + cmd.Flags().String("url", "", "") + cmd.Flags().String("output", "json", "") + return cmd +} + +func captureStdout(t *testing.T, fn func() error) (string, error) { + t.Helper() + old := os.Stdout + r, w, err := os.Pipe() + if err != nil { + t.Fatalf("pipe stdout: %v", err) + } + os.Stdout = w + defer func() { os.Stdout = old }() + + runErr := fn() + if err := w.Close(); err != nil { + t.Fatalf("close stdout writer: %v", err) + } + out, err := io.ReadAll(r) + if err != nil { + t.Fatalf("read stdout: %v", err) + } + return string(out), runErr +} + +func TestRunSkillImportJsonTreatsDuplicateAsStructuredResult(t *testing.T) { + t.Setenv("HOME", t.TempDir()) + t.Setenv("MULTICA_TOKEN", "test-token") + t.Setenv("MULTICA_WORKSPACE_ID", "workspace-123") + + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Method != http.MethodPost { + t.Fatalf("method = %s, want POST", r.Method) + } + if r.URL.Path != "/api/skills/import" { + t.Fatalf("path = %q, want /api/skills/import", r.URL.Path) + } + if r.Header.Get("X-Workspace-ID") != "workspace-123" { + t.Fatalf("X-Workspace-ID = %q, want workspace-123", r.Header.Get("X-Workspace-ID")) + } + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusConflict) + _ = json.NewEncoder(w).Encode(map[string]any{ + "error": "a skill with this name already exists", + "existing_skill": map[string]any{ + "id": "skill-123", + "name": "review-helper", + }, + }) + })) + defer srv.Close() + t.Setenv("MULTICA_SERVER_URL", srv.URL) + + cmd := newSkillImportTestCmd() + _ = cmd.Flags().Set("url", "https://skills.sh/acme/review-helper") + _ = cmd.Flags().Set("output", "json") + + out, err := captureStdout(t, func() error { + return runSkillImport(cmd, nil) + }) + if err != nil { + t.Fatalf("runSkillImport returned error for duplicate import: %v", err) + } + + var got map[string]any + if err := json.Unmarshal([]byte(out), &got); err != nil { + t.Fatalf("decode stdout JSON %q: %v", out, err) + } + if got["error"] != "a skill with this name already exists" { + t.Fatalf("error = %v", got["error"]) + } + existing, ok := got["existing_skill"].(map[string]any) + if !ok { + t.Fatalf("existing_skill missing or wrong type: %#v", got["existing_skill"]) + } + if existing["id"] != "skill-123" || existing["name"] != "review-helper" { + t.Fatalf("existing_skill = %#v", existing) + } +} diff --git a/server/internal/handler/skill.go b/server/internal/handler/skill.go index 29372fb89..b7835e9d5 100644 --- a/server/internal/handler/skill.go +++ b/server/internal/handler/skill.go @@ -1,6 +1,7 @@ package handler import ( + "context" "encoding/json" "errors" "fmt" @@ -14,6 +15,7 @@ import ( "time" "github.com/go-chi/chi/v5" + "github.com/jackc/pgx/v5" "github.com/jackc/pgx/v5/pgtype" db "github.com/multica-ai/multica/server/pkg/db/generated" "github.com/multica-ai/multica/server/pkg/protocol" @@ -88,6 +90,18 @@ type SkillWithFilesResponse struct { Files []SkillFileResponse `json:"files"` } +type ExistingSkillIdentity struct { + ID string `json:"id"` + Name string `json:"name"` +} + +func writeSkillImportDuplicateConflict(w http.ResponseWriter, existing ExistingSkillIdentity) { + writeJSON(w, http.StatusConflict, map[string]any{ + "error": "a skill with this name already exists", + "existing_skill": existing, + }) +} + func skillToResponse(s db.Skill) SkillResponse { return SkillResponse{ ID: uuidToString(s.ID), @@ -102,6 +116,20 @@ func skillToResponse(s db.Skill) SkillResponse { } } +func (h *Handler) existingSkillIdentityByName(ctx context.Context, workspaceID pgtype.UUID, name string) (ExistingSkillIdentity, bool, error) { + skill, err := h.Queries.GetSkillByWorkspaceAndName(ctx, db.GetSkillByWorkspaceAndNameParams{ + WorkspaceID: workspaceID, + Name: name, + }) + if err != nil { + if errors.Is(err, pgx.ErrNoRows) { + return ExistingSkillIdentity{}, false, nil + } + return ExistingSkillIdentity{}, false, err + } + return ExistingSkillIdentity{ID: uuidToString(skill.ID), Name: skill.Name}, true, nil +} + // decodeSkillConfig decodes a JSONB skill.config blob, defaulting to {} when // missing or unparseable so the API surface always returns a JSON object. func decodeSkillConfig(raw []byte) any { @@ -1657,7 +1685,11 @@ func (h *Handler) ImportSkill(w http.ResponseWriter, r *http.Request) { }) if err != nil { if isUniqueViolation(err) { - writeError(w, http.StatusConflict, "a skill with this name already exists") + if existing, found, findErr := h.existingSkillIdentityByName(r.Context(), workspaceUUID, imported.name); findErr == nil && found { + writeSkillImportDuplicateConflict(w, existing) + } else { + writeError(w, http.StatusConflict, "a skill with this name already exists") + } return } writeError(w, http.StatusInternalServerError, "failed to create skill: "+err.Error()) diff --git a/server/internal/handler/skill_import_duplicate_test.go b/server/internal/handler/skill_import_duplicate_test.go new file mode 100644 index 000000000..1c4312ebb --- /dev/null +++ b/server/internal/handler/skill_import_duplicate_test.go @@ -0,0 +1,48 @@ +package handler + +import ( + "context" + "encoding/json" + "net/http/httptest" + "testing" +) + +func TestExistingSkillIdentityByNameReturnsIDAndName(t *testing.T) { + namePrefix := "duplicate-import-identity" + name := namePrefix + "-" + t.Name() + skillID := insertHandlerTestSkill(t, namePrefix, "# Duplicate import identity") + + existing, ok, err := testHandler.existingSkillIdentityByName(context.Background(), parseUUID(testWorkspaceID), name) + if err != nil { + t.Fatalf("existingSkillIdentityByName: %v", err) + } + if !ok { + t.Fatal("expected existing skill identity to be found") + } + if existing.ID != skillID || existing.Name != name { + t.Fatalf("existing skill = %#v, want id %s name %s", existing, skillID, name) + } +} + +func TestWriteSkillImportDuplicateConflictIncludesExistingSkill(t *testing.T) { + w := httptest.NewRecorder() + writeSkillImportDuplicateConflict(w, ExistingSkillIdentity{ID: "skill-123", Name: "review-helper"}) + + if w.Code != 409 { + t.Fatalf("status = %d, want 409: %s", w.Code, w.Body.String()) + } + var body map[string]any + if err := json.Unmarshal(w.Body.Bytes(), &body); err != nil { + t.Fatalf("decode body: %v", err) + } + if body["error"] != "a skill with this name already exists" { + t.Fatalf("error = %v", body["error"]) + } + existing, ok := body["existing_skill"].(map[string]any) + if !ok { + t.Fatalf("existing_skill missing or wrong type: %#v", body["existing_skill"]) + } + if existing["id"] != "skill-123" || existing["name"] != "review-helper" { + t.Fatalf("existing_skill = %#v", existing) + } +}