diff --git a/apps/docs/content/docs/guides/agents.zh.mdx b/apps/docs/content/docs/guides/agents.zh.mdx index b489d6445..8a71c4bed 100644 --- a/apps/docs/content/docs/guides/agents.zh.mdx +++ b/apps/docs/content/docs/guides/agents.zh.mdx @@ -40,7 +40,7 @@ The daemon auto-detects which CLIs are available on your PATH and registers them Multica supports two layers of skills: -- **Local skills** — Skills already installed in your local runtime (e.g., `.claude/skills/`, `.opencode/skills/`) are automatically discovered and used by agents. You do **not** need to upload them to Multica. +- **Local skills** — Skills already installed in your local runtime (e.g., `.claude/skills/`, `.opencode/skills/`). For Claude Code and Codex, host-machine skills under `~/.claude/skills/` and `~/.codex/skills/` are **merged by default** for each agent (so existing personal workflows keep working) — toggle the per-agent **Allow host-machine Skills** switch off to isolate a shared agent against a broken local skill on one operator's machine (GitHub #3052). Other runtimes ignore this setting — the daemon does not actively manage user-level skill discovery for them today. - **Workspace skills** — Skills created or imported in the Multica Skills page are shared across the workspace. They are automatically injected into agent runs as supplementary context, so every team member's agents benefit from them. Workspace skills are designed for team-wide sharing and collaboration — codify your team's best practices once, and every agent can leverage them: diff --git a/apps/docs/content/docs/skills.mdx b/apps/docs/content/docs/skills.mdx index 1971a56e9..9d70b0a16 100644 --- a/apps/docs/content/docs/skills.mdx +++ b/apps/docs/content/docs/skills.mdx @@ -16,6 +16,24 @@ Multica supports two skill sources: Most of the time you want **workspace skills**: import once, every teammate's agent can use it. Local skills are a fit when you want to test locally first, or when the content involves sensitive local material. +### Per-agent host-skill merge toggle + +By default Multica **merges** the host machine's user-global skill directory into the agent (so existing personal workflows that rely on locally installed Claude skills keep working out of the box). You can flip the toggle to **ignore** when a shared agent should be hardened against a broken local skill on one operator's machine — that scenario crashes the Claude CLI before it ever reads a prompt (silent `broken pipe` exits, see GitHub #3052). + +Each agent carries a `skills_local` switch you can change from the **Create Agent** dialog or the agent's **Skills** tab: + +- **On — Merge (default)**: the runtime inherits the full `~/.claude/` directory, including its `skills/`. Matches the pre-MUL-2603 behavior — recommended for personal agents that intentionally rely on locally installed skills. +- **Off — Ignore**: the runtime launches against a per-task config directory that mirrors the host's effective Claude config dir *except* for `skills/`. The source is resolved with precedence agent `custom_env` `CLAUDE_CONFIG_DIR` > parent process `CLAUDE_CONFIG_DIR` > `~/.claude/`, so an operator who pinned Claude at a managed install keeps the right credentials and settings. Login credentials (`.credentials.json`), global settings, plugins, and agents pass through, so Claude Code authentication keeps working — only the user-global skills directory is hidden from the CLI. On Windows hosts without Developer Mode the mirror falls back to directory junctions and hardlink/copy for files, so the passthrough still works without elevated privileges. Recommended for any shared agent where host-skill safety matters. + +Workspace skills under `{workDir}/.claude/skills/` (and the equivalent workdir-scoped paths for other runtimes) are loaded regardless of the toggle — this switch only governs the host's user-global skills directory. + +**Which runtimes the toggle covers today:** Claude Code and Codex are the two runtimes where the daemon actively manages user-skill discovery, so they are the two that honor the toggle: + +- **Claude Code** — when set to **Off**, `CLAUDE_CONFIG_DIR` points at a per-task scratch dir that mirrors the host's effective Claude config dir *except* for `skills/` (see above). +- **Codex** — Codex always runs inside a per-task `CODEX_HOME`. When set to **On (default)** the daemon seeds that `CODEX_HOME` with user-installed skills from the shared `~/.codex/skills/`; when set to **Off** the seed step is skipped, so the Codex CLI sees only workspace skills. + +For every other runtime (Hermes / Kimi / Kiro / Copilot / Cursor / Gemini / Pi / OpenCode / OpenClaw) the daemon leaves the user's `$HOME` untouched and does not actively manage user-level skill discovery — whether that runtime reads any user-level skill directory is entirely up to its own CLI. **The toggle is a no-op for these runtimes today**, regardless of where it is set; a future change can extend per-runtime isolation as upstream CLIs ship the necessary knobs. + ## Importing a skill Workspace skills come from four sources: diff --git a/apps/docs/content/docs/skills.zh.mdx b/apps/docs/content/docs/skills.zh.mdx index 47578371a..f57ac395a 100644 --- a/apps/docs/content/docs/skills.zh.mdx +++ b/apps/docs/content/docs/skills.zh.mdx @@ -16,6 +16,24 @@ Multica 支持两种 Skill 来源： 大多数情况用**工作区 Skill**：导入一次，团队所有成员的智能体都能用。本机 Skill 适合先在本地测试、或涉及敏感本地内容的场景。 +### 按智能体维度的"合并本机 Skill"开关 + +默认情况下 Multica **会**把本机用户级 skill 目录合并进智能体（保持 MUL-2603 之前的"继承本机"行为，避免误伤依赖本机 skill 的个人智能体）。如果某个共享智能体需要防御"某台机器上有问题的本机 skill"——这种情况会让 Claude CLI 还没读到 prompt 就崩（典型表现是 `broken pipe`，参见 GitHub #3052）——你可以把开关切到 ignore。 + +每个智能体都带有一个 `skills_local` 开关，可在 **创建 Agent** 弹窗或智能体的 **Skills** 标签页里切换： + +- **开启 —— Merge（默认）**：运行时直接继承完整的 `~/.claude/`，包括 `skills/`。沿用 MUL-2603 之前的行为，适合个人智能体或明确依赖本机 skill 的场景。 +- **关闭 —— Ignore**：运行时使用按任务隔离的配置目录，目录里通过软链镜像了主机实际生效的 Claude 配置目录中除 `skills/` 之外的全部内容。源目录的解析顺序为：agent `custom_env` 中的 `CLAUDE_CONFIG_DIR` > 父进程 `CLAUDE_CONFIG_DIR` > `~/.claude/`，因此把 Claude 固定到托管安装目录的运维者仍能拿到正确的凭据和设置。登录凭据（`.credentials.json`）、全局设置、插件、本地 agent 仍会被 CLI 看到，Claude Code 登录态不受影响，只屏蔽用户级 skill 目录。在未开启开发者模式的 Windows 主机上，软链失败时会自动回退为目录联结（junction）以及文件的硬链接/拷贝，保证无管理员权限时镜像仍然可用。**推荐在需要防御本机 skill 风险的共享智能体上启用**。 + +无论开关如何，工作区目录下的 `{workDir}/.claude/skills/`（以及其他 runtime 在工作目录下对应的路径）都会照常加载——这个开关只控制本机用户级 skill 目录。 + +**目前哪些运行时实际受开关影响：** 守护进程主动管理用户级 skill 发现的运行时是 Claude Code 和 Codex，所以开关对它们都生效： + +- **Claude Code** —— 关闭时把 `CLAUDE_CONFIG_DIR` 指向按任务隔离的目录，里面镜像了主机有效的 Claude 配置目录中除 `skills/` 之外的内容（见上）。 +- **Codex** —— Codex 一向运行在按任务隔离的 `CODEX_HOME` 下。开启（默认）时守护进程会把共享的 `~/.codex/skills/` seed 到该目录；关闭时跳过这一步，Codex CLI 只能看到工作区 skill。 + +对其它运行时（Hermes / Kimi / Kiro / Copilot / Cursor / Gemini / Pi / OpenCode / OpenClaw），守护进程既不改写用户的 `$HOME`，也不主动管理用户级 skill 发现——这些运行时是否读取任何用户级 skill 目录，完全由各自的 CLI 决定。**无论该开关如何设置，对这些运行时都是 no-op**；待上游 CLI 提供必要的隔离开关后，可以再补齐。 + ## 导入 Skill 工作区 Skill 有四种来源： diff --git a/packages/core/types/agent.ts b/packages/core/types/agent.ts index 61a7d880d..dcb074c18 100644 --- a/packages/core/types/agent.ts +++ b/packages/core/types/agent.ts @@ -4,6 +4,18 @@ export type AgentRuntimeMode = "local" | "cloud"; export type AgentVisibility = "workspace" | "private"; +/** + * Per-agent toggle controlling whether the runtime merges the host machine's + * user-global skill directory into the agent. "merge" (default) preserves the + * inherit-from-machine behavior; "ignore" isolates the runtime so a broken + * local skill on one operator's machine cannot crash a shared agent (#3052). + * Today only the Claude runtime honours the switch — others either already + * isolate (Codex via CODEX_HOME) or treat it as a no-op. Older backends omit + * the field; clients MUST treat undefined as "merge" so the documented + * default wins on drift. + */ +export type AgentSkillsLocal = "ignore" | "merge"; + // Runtime visibility is a separate axis from agent visibility — different // vocabulary because it gates a different action. "private" (default) means // only the runtime owner and workspace admins can bind agents to it; @@ -156,6 +168,14 @@ export interface Agent { * (MUL-2339). */ thinking_level?: string; + /** + * Per-agent toggle for merging the host machine's user-global skill + * directory (e.g. Claude's `~/.claude/skills/`) into the agent. Older + * backends that predate the column omit this field; consumers MUST + * default to `"merge"` on drift to match the documented platform + * default (#3052 hardening is opt-in via `"ignore"`). + */ + skills_local?: AgentSkillsLocal; owner_id: string | null; skills: AgentSkillSummary[]; created_at: string; @@ -191,6 +211,10 @@ export interface CreateAgentRequest { model?: string; /** Optional runtime-native reasoning/effort token. See `Agent.thinking_level`. */ thinking_level?: string; + /** Per-agent host-skill merge toggle. Defaults to `"merge"` server-side + * when omitted (inherit-from-machine behavior); pass `"ignore"` to + * isolate the agent from the host's user-global skill directory. */ + skills_local?: AgentSkillsLocal; /** Optional template slug used by the onboarding agent picker. Surfaced * as the `template` property on the `agent_created` PostHog event. */ template?: string; @@ -296,6 +320,8 @@ export interface UpdateAgentRequest { * runtime's provider enum, rejected with 400 if not recognised */ thinking_level?: string; + /** Update the host-skill merge toggle. Omit to leave unchanged. */ + skills_local?: AgentSkillsLocal; } /** diff --git a/packages/core/types/index.ts b/packages/core/types/index.ts index 5d09cb2d8..2c74fdd4c 100644 --- a/packages/core/types/index.ts +++ b/packages/core/types/index.ts @@ -4,6 +4,7 @@ export type { AgentStatus, AgentRuntimeMode, AgentVisibility, + AgentSkillsLocal, AgentTask, AgentActivityBucket, AgentRunCount, diff --git a/packages/views/agents/components/create-agent-dialog.tsx b/packages/views/agents/components/create-agent-dialog.tsx index 0b636993c..4edb4e0c6 100644 --- a/packages/views/agents/components/create-agent-dialog.tsx +++ b/packages/views/agents/components/create-agent-dialog.tsx @@ -7,12 +7,14 @@ import { ModelDropdown } from "./model-dropdown"; import { RuntimePicker, isRuntimeUsableForUser } from "./runtime-picker"; import { InstructionsEditor } from "./instructions-editor"; import { SkillMultiSelect } from "./skill-multi-select"; +import { SkillsLocalToggle } from "./skills-local-toggle"; import { AvatarPicker } from "./avatar-picker"; import { api } from "@multica/core/api"; import { useWorkspaceId } from "@multica/core/hooks"; import { workspaceKeys } from "@multica/core/workspace/queries"; import type { Agent, + AgentSkillsLocal, AgentVisibility, RuntimeDevice, MemberWithUser, @@ -92,6 +94,13 @@ export function CreateAgentDialog({ const [selectedSkillIds, setSelectedSkillIds] = useState>( () => new Set(template?.skills.map((s) => s.id) ?? []), ); + // Default to "merge" for new agents (matches the server-side default — + // inherit-from-machine behavior preserved). Duplicate mode carries the + // source agent's explicit "ignore" through so a hardened agent stays + // hardened. + const [skillsLocal, setSkillsLocal] = useState( + () => (template?.skills_local === "ignore" ? "ignore" : "merge"), + ); const [creating, setCreating] = useState(false); // Duplicate-mode pre-fill: clone lands on the source agent's runtime so @@ -162,6 +171,11 @@ export function CreateAgentDialog({ model: model.trim() || undefined, instructions: trimmedInstructions || undefined, avatar_url: avatarUrl ?? undefined, + // Only send the toggle when the user opted into isolation. + // "merge" is the server-side default; omitting the field keeps the + // request body small and prevents older backends without the + // column from rejecting the request. + skills_local: skillsLocal === "ignore" ? "ignore" : undefined, }; if (template) { // Duplicate path: forward the hidden config fields the source @@ -359,6 +373,12 @@ export function CreateAgentDialog({ selectedIds={selectedSkillIds} onChange={setSelectedSkillIds} /> + + diff --git a/packages/views/agents/components/skills-local-toggle.tsx b/packages/views/agents/components/skills-local-toggle.tsx new file mode 100644 index 000000000..f5a02aa30 --- /dev/null +++ b/packages/views/agents/components/skills-local-toggle.tsx @@ -0,0 +1,67 @@ +"use client"; + +import type { AgentSkillsLocal } from "@multica/core/types"; +import { Label } from "@multica/ui/components/ui/label"; +import { Switch } from "@multica/ui/components/ui/switch"; +import { useT } from "../../i18n"; + +// SkillsLocalToggle renders the per-agent "merge host-machine skills" switch +// shared between the Create dialog (uncontrolled-by-network, only emits the +// new value) and the edit tab (which immediately persists). The component +// stays headless about save behavior — the parent decides what to do on +// change. Section labels follow the create dialog `skills_section` keys so +// both surfaces share identical copy. +export function SkillsLocalToggle({ + value, + onChange, + disabled, + hintScope = "create", +}: { + value: AgentSkillsLocal; + onChange: (next: AgentSkillsLocal) => void; + disabled?: boolean; + /** + * Picks the i18n namespace for the secondary hint copy. The create dialog + * lives under `create_dialog.skills_section.local_*` while the edit tab + * uses `tab_body.skills.local_*` — wording is intentionally identical + * today but kept in two places so each surface can evolve independently. + */ + hintScope?: "create" | "tab"; +}) { + const { t } = useT("agents"); + const checked = value === "merge"; + + const label = + hintScope === "create" + ? t(($) => $.create_dialog.skills_section.local_label) + : t(($) => $.tab_body.skills.local_label); + const hint = checked + ? hintScope === "create" + ? t(($) => $.create_dialog.skills_section.local_hint_on) + : t(($) => $.tab_body.skills.local_hint_on) + : hintScope === "create" + ? t(($) => $.create_dialog.skills_section.local_hint_off) + : t(($) => $.tab_body.skills.local_hint_off); + + return ( +
+
+ + onChange(next ? "merge" : "ignore")} + disabled={disabled} + /> +
+

+ {hint} +

+
+ ); +} diff --git a/packages/views/agents/components/tabs/skills-tab.test.tsx b/packages/views/agents/components/tabs/skills-tab.test.tsx index 7e3eda899..0f62816fd 100644 --- a/packages/views/agents/components/tabs/skills-tab.test.tsx +++ b/packages/views/agents/components/tabs/skills-tab.test.tsx @@ -11,6 +11,7 @@ import enAgents from "../../../locales/en/agents.json"; const TEST_RESOURCES = { en: { common: enCommon, agents: enAgents } }; const mockListSkills = vi.hoisted(() => vi.fn()); +const mockUpdateAgent = vi.hoisted(() => vi.fn()); vi.mock("@multica/core/hooks", () => ({ useWorkspaceId: () => "ws-1", @@ -20,6 +21,7 @@ vi.mock("@multica/core/api", () => ({ api: { listSkills: (...args: unknown[]) => mockListSkills(...args), setAgentSkills: vi.fn(), + updateAgent: (...args: unknown[]) => mockUpdateAgent(...args), }, })); @@ -55,7 +57,7 @@ const agent: Agent = { archived_by: null, }; -function renderSkillsTab() { +function renderSkillsTab(overrides: Partial = {}) { const queryClient = new QueryClient({ defaultOptions: { queries: { @@ -67,7 +69,7 @@ function renderSkillsTab() { return render( - + , ); @@ -77,6 +79,7 @@ describe("SkillsTab", () => { beforeEach(() => { vi.clearAllMocks(); mockListSkills.mockResolvedValue([]); + mockUpdateAgent.mockResolvedValue({}); }); it("does not render the inline Local Runtime Skills section even for local-runtime agents", async () => { @@ -88,9 +91,13 @@ describe("SkillsTab", () => { renderSkillsTab(); // Top informational callout should still render; that's how we know - // the tab body itself rendered (not stuck in a loading state). + // the tab body itself rendered (not stuck in a loading state). The + // toggle hint also includes the same phrase, so anchor on the intro + // copy specifically. expect( - await screen.findByText(/Local runtime skills are always available/i), + await screen.findByText( + /Workspace skills assigned to this agent/i, + ), ).toBeInTheDocument(); // The removed section's heading and its trigger button must be gone. @@ -98,13 +105,29 @@ describe("SkillsTab", () => { expect( screen.queryByRole("button", { name: /Import to Workspace/i }), ).not.toBeInTheDocument(); + }); - // No runtime list / local-skills query should be wired up either — - // we removed @multica/core/runtimes from this file's imports. - // Surface it via behaviour: the `agent` here has runtime_id but the - // tab must not invoke any runtime-list mock to render. (Both are - // already deleted from the mock setup above; this assertion is - // implicit — the test file would fail to import if the component - // still referenced runtimeListOptions / runtimeLocalSkillsOptions.) + it("renders the local-skill toggle ON by default (merge — preserves inherit-from-machine behavior)", async () => { + // Bohan's MUL-2603 product decision: keep the legacy "merge" default so + // personal workflows that rely on locally installed Claude Skills keep + // working unchanged. Owners flip the toggle off ("ignore") when they + // explicitly need to harden a shared agent. + renderSkillsTab(); + + const toggle = await screen.findByRole("switch", { + name: /Allow locally installed skills/i, + }); + // Base UI Switch reflects state via data-state; aria-checked is the + // accessible read. + expect(toggle.getAttribute("aria-checked")).toBe("true"); + }); + + it("reflects ignore mode when the agent opted into isolation", async () => { + renderSkillsTab({ skills_local: "ignore" }); + + const toggle = await screen.findByRole("switch", { + name: /Allow locally installed skills/i, + }); + expect(toggle.getAttribute("aria-checked")).toBe("false"); }); }); diff --git a/packages/views/agents/components/tabs/skills-tab.tsx b/packages/views/agents/components/tabs/skills-tab.tsx index 8edcb6a45..285ba73a4 100644 --- a/packages/views/agents/components/tabs/skills-tab.tsx +++ b/packages/views/agents/components/tabs/skills-tab.tsx @@ -4,7 +4,7 @@ import { useState } from "react"; import { FileText, Info, Plus, Trash2 } from "lucide-react"; import { useQuery, useQueryClient } from "@tanstack/react-query"; import { toast } from "sonner"; -import type { Agent } from "@multica/core/types"; +import type { Agent, AgentSkillsLocal } from "@multica/core/types"; import { api } from "@multica/core/api"; import { useWorkspaceId } from "@multica/core/hooks"; import { @@ -13,6 +13,7 @@ import { } from "@multica/core/workspace/queries"; import { Button } from "@multica/ui/components/ui/button"; import { SkillAddDialog } from "../skill-add-dialog"; +import { SkillsLocalToggle } from "../skills-local-toggle"; import { useT } from "../../../i18n"; export function SkillsTab({ @@ -23,6 +24,12 @@ export function SkillsTab({ const { t } = useT("agents"); const qc = useQueryClient(); const wsId = useWorkspaceId(); + // Default missing values to "merge". Older backends omit the field + // entirely; the documented platform default wins on drift (see + // AgentSkillsLocal — "ignore" is the explicit hardening opt-in). + const currentSkillsLocal: AgentSkillsLocal = + agent.skills_local === "ignore" ? "ignore" : "merge"; + const [skillsLocalSaving, setSkillsLocalSaving] = useState(false); // Same query the SkillAddDialog uses (TanStack Query dedupes by key, so // this isn't an extra request) — used here only to grey out the "Add // skill" button when the workspace has zero skills total. When skills @@ -34,6 +41,24 @@ export function SkillsTab({ const [removing, setRemoving] = useState(false); const [showAdd, setShowAdd] = useState(false); + const handleSkillsLocalChange = async (next: AgentSkillsLocal) => { + if (next === currentSkillsLocal || skillsLocalSaving) return; + setSkillsLocalSaving(true); + try { + await api.updateAgent(agent.id, { skills_local: next }); + qc.invalidateQueries({ queryKey: workspaceKeys.agents(wsId) }); + toast.success(t(($) => $.tab_body.skills.local_saved_toast)); + } catch (e) { + toast.error( + e instanceof Error + ? e.message + : t(($) => $.tab_body.skills.local_save_failed_toast), + ); + } finally { + setSkillsLocalSaving(false); + } + }; + const handleRemove = async (skillId: string) => { setRemoving(true); try { @@ -74,6 +99,14 @@ export function SkillsTab({

+ + + {agent.skills.length === 0 ? (
diff --git a/packages/views/locales/en/agents.json b/packages/views/locales/en/agents.json index 8587b3edb..ad06b0517 100644 --- a/packages/views/locales/en/agents.json +++ b/packages/views/locales/en/agents.json @@ -242,7 +242,10 @@ "list_loading": "Loading…", "list_empty_default": "No skills available.", "list_no_match": "No skills match.", - "search_placeholder": "Search skills…" + "search_placeholder": "Search skills…", + "local_label": "Allow locally installed skills", + "local_hint_off": "Off — for Claude Code and Codex, the agent ignores the host machine's user-global skill directory (~/.claude/skills/, ~/.codex/skills/). Recommended for shared agents — a broken local skill on one operator's machine cannot crash this agent. Other runtimes ignore this setting.", + "local_hint_on": "On (default) — for Claude Code and Codex, the host machine's user-global skill directory is merged alongside workspace skills. Other runtimes ignore this setting." }, "avatar": { "change_aria": "Change avatar", @@ -295,9 +298,14 @@ "save_failed_toast": "Failed to save custom arguments" }, "skills": { - "intro": "Workspace skills assigned to this agent. Local runtime skills are always available automatically.", + "intro": "Workspace skills assigned to this agent. For Claude Code and Codex, the host machine's user-global skill directory is merged by default — switch the toggle below off to isolate this agent from those host-machine skills. Other runtimes ignore this setting.", "add_action": "Add skill", "import_hint": "Importing creates a workspace copy that your team can edit and reuse.", + "local_label": "Allow locally installed skills", + "local_hint_off": "Off — for Claude Code and Codex, the agent ignores the host machine's user-global skill directory (~/.claude/skills/, ~/.codex/skills/). Recommended for shared agents — a broken local skill on one operator's machine cannot crash this agent. Other runtimes ignore this setting.", + "local_hint_on": "On (default) — for Claude Code and Codex, the host machine's user-global skill directory is merged alongside workspace skills. Other runtimes ignore this setting.", + "local_saved_toast": "Skill merge setting saved", + "local_save_failed_toast": "Failed to save skill merge setting", "empty_title": "No skills assigned", "empty_hint": "Add workspace skills to share team knowledge with this agent.", "remove_failed_toast": "Failed to remove skill", diff --git a/packages/views/locales/zh-Hans/agents.json b/packages/views/locales/zh-Hans/agents.json index 4b94e5a05..632a0642b 100644 --- a/packages/views/locales/zh-Hans/agents.json +++ b/packages/views/locales/zh-Hans/agents.json @@ -237,7 +237,10 @@ "list_loading": "加载中…", "list_empty_default": "暂无 skill。", "list_no_match": "没有匹配的 skill。", - "search_placeholder": "搜索 skill…" + "search_placeholder": "搜索 skill…", + "local_label": "允许使用本机已安装的 Skill", + "local_hint_off": "关闭 —— 对 Claude Code 与 Codex，智能体会忽略本机用户级 skill 目录（~/.claude/skills/、~/.codex/skills/）。推荐用于共享智能体 —— 团队成员机器上有问题的本机 skill 不会拖崩这个智能体。其它运行时会忽略此设置。", + "local_hint_on": "开启（默认）—— 对 Claude Code 与 Codex，运行时会同时合并本机用户级 skill 目录。其它运行时会忽略此设置。" }, "avatar": { "change_aria": "更换头像", @@ -289,9 +292,14 @@ "save_failed_toast": "保存自定义参数失败" }, "skills": { - "intro": "分配给该智能体的工作区 skill。本地运行时 skill 会自动可用。", + "intro": "分配给该智能体的工作区 skill。对 Claude Code 与 Codex，默认会同时合并本机用户级 skill 目录 —— 把下方开关关闭可以让该智能体与本机用户级 skill 隔离。其它运行时会忽略此设置。", "add_action": "添加 skill", "import_hint": "导入会在工作区创建一份副本，团队可以编辑和复用。", + "local_label": "允许使用本机已安装的 Skill", + "local_hint_off": "关闭 —— 对 Claude Code 与 Codex，智能体会忽略本机用户级 skill 目录（~/.claude/skills/、~/.codex/skills/）。推荐用于共享智能体 —— 团队成员机器上有问题的本机 skill 不会拖崩这个智能体。其它运行时会忽略此设置。", + "local_hint_on": "开启（默认）—— 对 Claude Code 与 Codex，运行时会同时合并本机用户级 skill 目录。其它运行时会忽略此设置。", + "local_saved_toast": "已保存 Skill 合并设置", + "local_save_failed_toast": "保存 Skill 合并设置失败", "empty_title": "尚未分配 skill", "empty_hint": "添加工作区 skill，把团队知识共享给这个智能体。", "remove_failed_toast": "移除 skill 失败", diff --git a/server/internal/daemon/daemon.go b/server/internal/daemon/daemon.go index 0d8db5e63..848aa2f64 100644 --- a/server/internal/daemon/daemon.go +++ b/server/internal/daemon/daemon.go @@ -2285,12 +2285,17 @@ func (d *Daemon) runTask(ctx context.Context, task Task, provider string, slot i if provider == "openclaw" { openclawBin = entry.Path } + envSkillsLocal := "" + if task.Agent != nil { + envSkillsLocal = task.Agent.SkillsLocal + } if task.PriorWorkDir != "" { env = execenv.Reuse(execenv.ReuseParams{ WorkDir: task.PriorWorkDir, Provider: provider, CodexVersion: codexVersion, OpenclawBin: openclawBin, + SkillsLocal: envSkillsLocal, Task: taskCtx, }, d.logger) } @@ -2304,6 +2309,7 @@ func (d *Daemon) runTask(ctx context.Context, task Task, provider string, slot i Provider: provider, CodexVersion: codexVersion, OpenclawBin: openclawBin, + SkillsLocal: envSkillsLocal, Task: taskCtx, }, d.logger) if err != nil { @@ -2490,6 +2496,10 @@ func (d *Daemon) runTask(ctx context.Context, task Task, provider string, slot i thinkingLevel = "" } } + skillsLocal := "" + if task.Agent != nil { + skillsLocal = task.Agent.SkillsLocal + } execOpts := agent.ExecOptions{ Cwd: env.WorkDir, Model: model, @@ -2500,6 +2510,7 @@ func (d *Daemon) runTask(ctx context.Context, task Task, provider string, slot i CustomArgs: customArgs, McpConfig: mcpConfig, ThinkingLevel: thinkingLevel, + SkillsLocal: skillsLocal, } // Some providers do not reliably load the per-task runtime config files we // write into the task workdir: diff --git a/server/internal/daemon/execenv/execenv.go b/server/internal/daemon/execenv/execenv.go index 2e5092143..89bcb50ea 100644 --- a/server/internal/daemon/execenv/execenv.go +++ b/server/internal/daemon/execenv/execenv.go @@ -32,14 +32,21 @@ type ProjectResourceForEnv struct { // PrepareParams holds all inputs needed to set up an execution environment. type PrepareParams struct { - WorkspacesRoot string // base path for all envs (e.g., ~/multica_workspaces) - WorkspaceID string // workspace UUID — tasks are grouped under this - TaskID string // task UUID — used for directory name - AgentName string // for git branch naming only - Provider string // agent provider (determines runtime config and skill injection paths) - CodexVersion string // detected Codex CLI version (only used when Provider == "codex") - OpenclawBin string // resolved openclaw CLI path (only used when Provider == "openclaw"); empty = look up on PATH - Task TaskContextForEnv // context data for writing files + WorkspacesRoot string // base path for all envs (e.g., ~/multica_workspaces) + WorkspaceID string // workspace UUID — tasks are grouped under this + TaskID string // task UUID — used for directory name + AgentName string // for git branch naming only + Provider string // agent provider (determines runtime config and skill injection paths) + CodexVersion string // detected Codex CLI version (only used when Provider == "codex") + OpenclawBin string // resolved openclaw CLI path (only used when Provider == "openclaw"); empty = look up on PATH + // SkillsLocal mirrors agent.SkillsLocal. "ignore" instructs runtime + // setup steps that the daemon controls (currently Codex's user-skill + // seed) to skip injecting the host machine's user-global skills into + // the per-task environment. Any other value — including "merge" and + // the empty string from older servers / clients — preserves the + // inherit-from-host behavior. + SkillsLocal string + Task TaskContextForEnv // context data for writing files } // TaskContextForEnv is the subset of task context used for writing context files. @@ -175,7 +182,7 @@ func Prepare(params PrepareParams, logger *slog.Logger) (*Environment, error) { if err := prepareCodexHomeWithOpts(codexHome, CodexHomeOptions{CodexVersion: params.CodexVersion}, logger); err != nil { return nil, fmt.Errorf("execenv: prepare codex-home: %w", err) } - if err := hydrateCodexSkills(codexHome, params.Task.AgentSkills, logger); err != nil { + if err := hydrateCodexSkills(codexHome, params.Task.AgentSkills, params.SkillsLocal, logger); err != nil { return nil, fmt.Errorf("execenv: hydrate codex skills: %w", err) } env.CodexHome = codexHome @@ -206,9 +213,14 @@ func Prepare(params PrepareParams, logger *slog.Logger) (*Environment, error) { type ReuseParams struct { WorkDir string Provider string - CodexVersion string // only used when Provider == "codex" - OpenclawBin string // only used when Provider == "openclaw"; empty = PATH lookup - Task TaskContextForEnv // refreshed context files / skills + CodexVersion string // only used when Provider == "codex" + OpenclawBin string // only used when Provider == "openclaw"; empty = PATH lookup + // SkillsLocal mirrors PrepareParams.SkillsLocal. Reuse must respect + // the same gate so a workdir resurrected on a later task picks up the + // current agent setting (the user may have flipped the toggle between + // runs). + SkillsLocal string + Task TaskContextForEnv // refreshed context files / skills } // Reuse wraps an existing workdir into an Environment and refreshes context files. @@ -238,7 +250,7 @@ func Reuse(params ReuseParams, logger *slog.Logger) *Environment { logger.Warn("execenv: refresh codex-home failed", "error", err) } else { env.CodexHome = codexHome - if err := hydrateCodexSkills(codexHome, params.Task.AgentSkills, logger); err != nil { + if err := hydrateCodexSkills(codexHome, params.Task.AgentSkills, params.SkillsLocal, logger); err != nil { logger.Warn("execenv: refresh codex skills failed", "error", err) } } @@ -279,17 +291,26 @@ func Reuse(params ReuseParams, logger *slog.Logger) *Environment { // run — its old contents would otherwise remain visible to the codex // CLI. // -// Codex is the only runtime that needs this two-stage hydration because the -// daemon sets CODEX_HOME to a per-task directory, isolating the CLI from the -// user's real ~/.codex/. Other runtimes leave HOME untouched and discover -// user-level skills natively (see context.go for the workdir-local paths -// they use for workspace skills). -func hydrateCodexSkills(codexHome string, workspaceSkills []SkillContextForEnv, logger *slog.Logger) error { +// When skillsLocal == "ignore" the user-skill seed is skipped, so the +// per-task CODEX_HOME exposes only workspace skills to the codex CLI. This +// is the Codex-side enforcement of the per-agent `skills_local` toggle — +// the Claude backend enforces the same contract by mirroring `~/.claude/` +// minus `skills/` (see server/pkg/agent/claude.go). +// +// Codex is the only runtime besides Claude where the daemon actively +// manages user-skill discovery: Codex via CODEX_HOME and the seed step +// here, Claude via CLAUDE_CONFIG_DIR scratch mirroring. Other runtimes +// leave HOME untouched and discover user-level skills natively (see +// context.go for the workdir-local paths they use for workspace skills), +// so `skills_local` is a no-op for them today. +func hydrateCodexSkills(codexHome string, workspaceSkills []SkillContextForEnv, skillsLocal string, logger *slog.Logger) error { skillsDir := filepath.Join(codexHome, "skills") if err := os.RemoveAll(skillsDir); err != nil { return fmt.Errorf("clear codex skills dir: %w", err) } - if err := seedUserCodexSkills(codexHome, workspaceSkills, logger); err != nil { + if skillsLocal == "ignore" { + logger.Info("execenv: codex user-skill seed skipped (skills_local=ignore)") + } else if err := seedUserCodexSkills(codexHome, workspaceSkills, logger); err != nil { logger.Warn("execenv: seed user codex skills failed", "error", err) } if len(workspaceSkills) == 0 { diff --git a/server/internal/daemon/execenv/execenv_test.go b/server/internal/daemon/execenv/execenv_test.go index 4a0816a91..c70a1253c 100644 --- a/server/internal/daemon/execenv/execenv_test.go +++ b/server/internal/daemon/execenv/execenv_test.go @@ -2456,6 +2456,145 @@ func TestPrepareCodexResolvesUserSkillSymlinks(t *testing.T) { } } +// TestPrepareCodexSkillsLocalIgnoreSkipsUserSeed covers the per-agent +// `skills_local=ignore` contract on the Codex side (MUL-2603): when the +// agent opts out of host-machine skills, the daemon must not seed +// `~/.codex/skills/` into the per-task CODEX_HOME, so a broken local skill +// cannot crash a shared Codex agent. The default (`merge`) preserves the +// pre-existing seed-from-shared behavior — covered by +// TestPrepareCodexSeedsUserSkills above. +func TestPrepareCodexSkillsLocalIgnoreSkipsUserSeed(t *testing.T) { + // Cannot use t.Parallel() with t.Setenv. + + sharedHome := t.TempDir() + t.Setenv("CODEX_HOME", sharedHome) + + userSkills := filepath.Join(sharedHome, "skills") + if err := os.MkdirAll(filepath.Join(userSkills, "summarize"), 0o755); err != nil { + t.Fatalf("seed user skill dir: %v", err) + } + if err := os.WriteFile(filepath.Join(userSkills, "summarize", "SKILL.md"), []byte("user summarize"), 0o644); err != nil { + t.Fatalf("seed user SKILL.md: %v", err) + } + + env, err := Prepare(PrepareParams{ + WorkspacesRoot: t.TempDir(), + WorkspaceID: "ws-codex-skills-local-ignore", + TaskID: "c1d2e3f4-a5b6-7890-cdef-123456789012", + AgentName: "Codex Agent", + Provider: "codex", + SkillsLocal: "ignore", + Task: TaskContextForEnv{ + IssueID: "codex-skills-local-ignore-test", + AgentSkills: []SkillContextForEnv{ + {Name: "Writing", Content: "workspace writing"}, + }, + }, + }, testLogger()) + if err != nil { + t.Fatalf("Prepare failed: %v", err) + } + defer env.Cleanup(true) + + if _, err := os.Stat(filepath.Join(env.CodexHome, "skills", "summarize")); !os.IsNotExist(err) { + t.Errorf("user skill seeded despite skills_local=ignore: err=%v", err) + } + if _, err := os.Stat(filepath.Join(env.CodexHome, "skills", "writing", "SKILL.md")); err != nil { + t.Errorf("workspace skill not written under skills_local=ignore: %v", err) + } +} + +// TestPrepareCodexSkillsLocalMergeSeedsUserSkills locks in the default +// `merge` semantics on the Codex side: anything other than the literal +// "ignore" (including empty from older servers) preserves the seed-from- +// shared behavior so existing personal workflows continue working. +func TestPrepareCodexSkillsLocalMergeSeedsUserSkills(t *testing.T) { + // Cannot use t.Parallel() with t.Setenv. + + sharedHome := t.TempDir() + t.Setenv("CODEX_HOME", sharedHome) + + userSkills := filepath.Join(sharedHome, "skills") + if err := os.MkdirAll(filepath.Join(userSkills, "translate"), 0o755); err != nil { + t.Fatalf("seed user skill dir: %v", err) + } + if err := os.WriteFile(filepath.Join(userSkills, "translate", "SKILL.md"), []byte("user translate"), 0o644); err != nil { + t.Fatalf("seed user SKILL.md: %v", err) + } + + env, err := Prepare(PrepareParams{ + WorkspacesRoot: t.TempDir(), + WorkspaceID: "ws-codex-skills-local-merge", + TaskID: "d2e3f4a5-b6c7-8901-cdef-234567890123", + AgentName: "Codex Agent", + Provider: "codex", + SkillsLocal: "merge", + Task: TaskContextForEnv{IssueID: "codex-skills-local-merge-test"}, + }, testLogger()) + if err != nil { + t.Fatalf("Prepare failed: %v", err) + } + defer env.Cleanup(true) + + data, err := os.ReadFile(filepath.Join(env.CodexHome, "skills", "translate", "SKILL.md")) + if err != nil { + t.Fatalf("user skill not seeded under skills_local=merge: %v", err) + } + if string(data) != "user translate" { + t.Errorf("seeded user SKILL.md = %q, want %q", data, "user translate") + } +} + +// TestReuseCodexSkillsLocalIgnoreSkipsUserSeed mirrors the Prepare path on +// the Reuse branch: if a user flips the agent toggle to `ignore` between +// runs, the next task on the reused workdir must drop the previously-seeded +// user skill rather than leak it through. +func TestReuseCodexSkillsLocalIgnoreSkipsUserSeed(t *testing.T) { + // Cannot use t.Parallel() with t.Setenv. + + sharedHome := t.TempDir() + t.Setenv("CODEX_HOME", sharedHome) + + userSkills := filepath.Join(sharedHome, "skills") + if err := os.MkdirAll(filepath.Join(userSkills, "summarize"), 0o755); err != nil { + t.Fatalf("seed user skill dir: %v", err) + } + if err := os.WriteFile(filepath.Join(userSkills, "summarize", "SKILL.md"), []byte("user summarize"), 0o644); err != nil { + t.Fatalf("seed user SKILL.md: %v", err) + } + + env, err := Prepare(PrepareParams{ + WorkspacesRoot: t.TempDir(), + WorkspaceID: "ws-codex-reuse-toggle", + TaskID: "e3f4a5b6-c7d8-9012-cdef-345678901234", + AgentName: "Codex Agent", + Provider: "codex", + SkillsLocal: "merge", + Task: TaskContextForEnv{IssueID: "codex-reuse-toggle-test"}, + }, testLogger()) + if err != nil { + t.Fatalf("Prepare failed: %v", err) + } + defer env.Cleanup(true) + + if _, err := os.Stat(filepath.Join(env.CodexHome, "skills", "summarize", "SKILL.md")); err != nil { + t.Fatalf("Prepare(merge) did not seed user skill: %v", err) + } + + reused := Reuse(ReuseParams{ + WorkDir: env.WorkDir, + Provider: "codex", + SkillsLocal: "ignore", + Task: TaskContextForEnv{IssueID: "codex-reuse-toggle-test"}, + }, testLogger()) + if reused == nil { + t.Fatal("Reuse returned nil") + } + if _, err := os.Stat(filepath.Join(reused.CodexHome, "skills", "summarize")); !os.IsNotExist(err) { + t.Errorf("user skill survived Reuse(skills_local=ignore): err=%v", err) + } +} + // TestReuseSeedsUserSkillUpdates ensures that user-skill edits between two // runs of the same task (the Reuse path) propagate into the per-task home. func TestReuseSeedsUserSkillUpdates(t *testing.T) { diff --git a/server/internal/daemon/types.go b/server/internal/daemon/types.go index 897576dc7..75202d215 100644 --- a/server/internal/daemon/types.go +++ b/server/internal/daemon/types.go @@ -105,6 +105,18 @@ type AgentData struct { McpConfig json.RawMessage `json:"mcp_config,omitempty"` Model string `json:"model,omitempty"` ThinkingLevel string `json:"thinking_level,omitempty"` + // SkillsLocal controls whether the runtime merges the host machine's + // user-global skill directory into the agent. "merge" (default) + // preserves the inherit-from-machine behavior; "ignore" hides the + // directory from the runtime so a broken local skill cannot crash a + // shared agent (#3052). Old servers that predate the column omit this + // field entirely; the daemon treats empty as "merge". Currently + // honoured by Claude (via CLAUDE_CONFIG_DIR scratch mirroring in + // server/pkg/agent/claude.go) and Codex (via the user-skill seed gate + // in server/internal/daemon/execenv/execenv.go); other runtimes leave + // HOME untouched and natively discover user-level skills, so the + // toggle is a no-op for them. + SkillsLocal string `json:"skills_local,omitempty"` } // SkillData represents a structured skill for task execution. diff --git a/server/internal/handler/agent.go b/server/internal/handler/agent.go index 9b7f96a1b..09e546d6b 100644 --- a/server/internal/handler/agent.go +++ b/server/internal/handler/agent.go @@ -56,8 +56,15 @@ type AgentResponse struct { // ThinkingLevel is the runtime-native reasoning/effort token persisted // for this agent (empty = use runtime default). The picker is per-runtime // per-model; the API never normalizes across providers. See MUL-2339. - ThinkingLevel string `json:"thinking_level"` - OwnerID *string `json:"owner_id"` + ThinkingLevel string `json:"thinking_level"` + // SkillsLocal controls whether the runtime merges the host machine's + // user-global skill directory (e.g. Claude's `~/.claude/skills/`) into + // the agent's skill set. "merge" (default) preserves the pre-existing + // inherit-from-machine behavior; "ignore" isolates the runtime so a + // broken local skill cannot crash a shared agent (#3052). Workspace + // skills (`{workDir}/.claude/skills/`) are always loaded regardless. + SkillsLocal string `json:"skills_local"` + OwnerID *string `json:"owner_id"` Skills []AgentSkillSummary `json:"skills"` CreatedAt string `json:"created_at"` UpdatedAt string `json:"updated_at"` @@ -122,6 +129,7 @@ func agentToResponse(a db.Agent) AgentResponse { MaxConcurrentTasks: a.MaxConcurrentTasks, Model: a.Model.String, ThinkingLevel: a.ThinkingLevel.String, + SkillsLocal: normalizeSkillsLocal(a.SkillsLocal), OwnerID: uuidToPtr(a.OwnerID), Skills: []AgentSkillSummary{}, CreatedAt: timestampToString(a.CreatedAt), @@ -245,6 +253,11 @@ type TaskAgentData struct { McpConfig json.RawMessage `json:"mcp_config,omitempty"` Model string `json:"model,omitempty"` ThinkingLevel string `json:"thinking_level,omitempty"` + // SkillsLocal mirrors the agent column of the same name. Daemon reads + // it to decide whether to isolate the Claude runtime from the host's + // user-global `~/.claude/skills/`. Empty string is treated as the + // platform default ("merge") by the daemon — see normalizeSkillsLocal. + SkillsLocal string `json:"skills_local,omitempty"` } func taskToResponse(t db.AgentTaskQueue) AgentTaskResponse { @@ -462,6 +475,11 @@ type CreateAgentRequest struct { MaxConcurrentTasks int32 `json:"max_concurrent_tasks"` Model string `json:"model"` ThinkingLevel string `json:"thinking_level"` + // SkillsLocal opts the agent into ("merge") or out of ("ignore") the + // runtime's user-global skill discovery. Empty / missing defaults to + // "merge". Validated in CreateAgent / UpdateAgent against the + // recognised set; anything else returns 400. + SkillsLocal string `json:"skills_local"` // Template records which template slug was used to seed this agent // (e.g. "coding" / "planning" / "writing" / "assistant"). Empty when // the caller didn't come from a template picker — the `agent_created` @@ -561,6 +579,21 @@ func (h *Handler) CreateAgent(w http.ResponseWriter, r *http.Request) { return } + // Request validation is strict: a missing field defaults to "merge" (the + // pre-MUL-2603 inherit-from-machine behavior), but a present-but-bogus + // value must 400 rather than silently normalize. The drift-safe normalize + // is only for on-read coercion of stored values (normalizeSkillsLocal) — + // applying it to request input would mask client typos as a successful + // create. + skillsLocal := "merge" + if req.SkillsLocal != "" { + if !isValidSkillsLocal(req.SkillsLocal) { + writeError(w, http.StatusBadRequest, fmt.Sprintf("skills_local %q is invalid (expected \"ignore\" or \"merge\")", req.SkillsLocal)) + return + } + skillsLocal = req.SkillsLocal + } + // Probe workspace agent count BEFORE the insert so the funnel has a // clean "first agent ever in this workspace" signal — Step 4 of // onboarding always lands in this branch. A non-fatal read: if the @@ -608,6 +641,7 @@ func (h *Handler) CreateAgent(w http.ResponseWriter, r *http.Request) { McpConfig: mc, Model: pgtype.Text{String: req.Model, Valid: req.Model != ""}, ThinkingLevel: pgtype.Text{String: req.ThinkingLevel, Valid: req.ThinkingLevel != ""}, + SkillsLocal: skillsLocal, }) if err != nil { // Unique constraint on (workspace_id, name) — return a clear conflict error @@ -675,6 +709,31 @@ type UpdateAgentRequest struct { // Distinguishing those modes is why this is a pointer; the raw-fields // map captured at decode time tells us whether the key was sent. ThinkingLevel *string `json:"thinking_level"` + // SkillsLocal: nil means "no change"; non-nil overrides the stored + // value after validation. We use a pointer (rather than the empty + // string sentinel ThinkingLevel uses) because there is no "clear" + // state — the column has a NOT NULL default and the toggle is binary. + SkillsLocal *string `json:"skills_local"` +} + +// normalizeSkillsLocal coerces empty / unknown stored values to the platform +// default "merge" — the pre-MUL-2603 inherit-from-machine behavior. Older +// rows written before the column existed get a NOT NULL default at migration +// time, but the helper still guards against any future schema drift or +// hand-edited rows so we never surface an in-between state to clients or the +// daemon. +func normalizeSkillsLocal(stored string) string { + if stored == "ignore" { + return "ignore" + } + return "merge" +} + +// isValidSkillsLocal mirrors the CHECK constraint on agent.skills_local. +// The DB would reject a write with a 23514 violation anyway, but pre-flight +// validation lets the API return a clean 400 with the offending value. +func isValidSkillsLocal(v string) bool { + return v == "ignore" || v == "merge" } // workspaceAlwaysRedactSecrets reports whether the workspace has opted @@ -876,6 +935,17 @@ func (h *Handler) UpdateAgent(w http.ResponseWriter, r *http.Request) { if req.Model != nil { params.Model = pgtype.Text{String: *req.Model, Valid: true} } + if req.SkillsLocal != nil { + // PATCH is strictly opt-in: nil means no change. A non-nil value must + // validate as-is — normalizing here would let garbage silently land as + // "merge" and hide a client bug. The drift-safe coercion lives in + // normalizeSkillsLocal for on-read use only. + if !isValidSkillsLocal(*req.SkillsLocal) { + writeError(w, http.StatusBadRequest, fmt.Sprintf("skills_local %q is invalid (expected \"ignore\" or \"merge\")", *req.SkillsLocal)) + return + } + params.SkillsLocal = pgtype.Text{String: *req.SkillsLocal, Valid: true} + } // thinking_level handling (MUL-2339). Tri-state semantics: // - field omitted → leave column alone (COALESCE narg), but if a diff --git a/server/internal/handler/agent_skills_local_test.go b/server/internal/handler/agent_skills_local_test.go new file mode 100644 index 000000000..fbaa8f344 --- /dev/null +++ b/server/internal/handler/agent_skills_local_test.go @@ -0,0 +1,182 @@ +package handler + +import ( + "context" + "encoding/json" + "net/http" + "net/http/httptest" + "testing" +) + +// TestCreateAgent_SkillsLocal_Validation pins the contract for the +// per-agent host-skill merge switch added with MUL-2603 / GitHub #3052: +// +// - Field omitted → platform default "merge" is persisted (preserves the +// pre-MUL-2603 inherit-from-machine behavior so existing personal +// workflows that rely on locally installed skills keep working). +// - "ignore" / "merge" → accepted and round-tripped. +// - Any other value → 400 BadRequest. The DB CHECK would catch it as a +// 500 fallback, but we want a clean error for clients. +func TestCreateAgent_SkillsLocal_Validation(t *testing.T) { + if testHandler == nil { + t.Skip("database not available") + } + + ctx := context.Background() + claudeRuntimeID := createClaudeProviderRuntime(t) + + t.Cleanup(func() { + testPool.Exec(ctx, + `DELETE FROM agent WHERE workspace_id = $1 AND name LIKE 'skills-local-test-%'`, + testWorkspaceID, + ) + }) + + t.Run("omitted defaults to merge", func(t *testing.T) { + body := map[string]any{ + "name": "skills-local-test-default", + "runtime_id": claudeRuntimeID, + "visibility": "private", + "max_concurrent_tasks": 1, + } + w := httptest.NewRecorder() + testHandler.CreateAgent(w, newRequest(http.MethodPost, "/api/agents", body)) + if w.Code != http.StatusCreated { + t.Fatalf("default skills_local: expected 201, got %d: %s", w.Code, w.Body.String()) + } + var resp map[string]any + _ = json.NewDecoder(w.Body).Decode(&resp) + if resp["skills_local"] != "merge" { + t.Errorf("expected skills_local=merge (platform default), got %v", resp["skills_local"]) + } + }) + + t.Run("explicit ignore round-trips", func(t *testing.T) { + body := map[string]any{ + "name": "skills-local-test-ignore", + "runtime_id": claudeRuntimeID, + "visibility": "private", + "max_concurrent_tasks": 1, + "skills_local": "ignore", + } + w := httptest.NewRecorder() + testHandler.CreateAgent(w, newRequest(http.MethodPost, "/api/agents", body)) + if w.Code != http.StatusCreated { + t.Fatalf("skills_local=ignore: expected 201, got %d: %s", w.Code, w.Body.String()) + } + var resp map[string]any + _ = json.NewDecoder(w.Body).Decode(&resp) + if resp["skills_local"] != "ignore" { + t.Errorf("expected skills_local=ignore, got %v", resp["skills_local"]) + } + }) + + t.Run("garbage value rejected with 400", func(t *testing.T) { + body := map[string]any{ + "name": "skills-local-test-garbage", + "runtime_id": claudeRuntimeID, + "visibility": "private", + "max_concurrent_tasks": 1, + "skills_local": "yes-please", + } + w := httptest.NewRecorder() + testHandler.CreateAgent(w, newRequest(http.MethodPost, "/api/agents", body)) + if w.Code != http.StatusBadRequest { + t.Fatalf("garbage skills_local: expected 400, got %d: %s", w.Code, w.Body.String()) + } + }) +} + +// TestUpdateAgent_SkillsLocal_Tristate covers PATCH semantics: nil +// pointer means "no change", non-nil overrides after validation. Unlike +// thinking_level there is no "clear" mode — the column is NOT NULL. +func TestUpdateAgent_SkillsLocal_Tristate(t *testing.T) { + if testHandler == nil { + t.Skip("database not available") + } + + ctx := context.Background() + claudeRuntimeID := createClaudeProviderRuntime(t) + agentID := createAgentOnRuntime(t, "skills-local-update-test", claudeRuntimeID, "") + // Demote the seed row to "ignore" so we can verify both omit (preserve + // ignore) and explicit-flip-back (set merge) paths. + if _, err := testPool.Exec(ctx, `UPDATE agent SET skills_local = 'ignore' WHERE id = $1`, agentID); err != nil { + t.Fatalf("seed skills_local=ignore: %v", err) + } + + t.Cleanup(func() { + testPool.Exec(ctx, `DELETE FROM agent WHERE id = $1`, agentID) + }) + + t.Run("omitted preserves existing value", func(t *testing.T) { + body := map[string]any{ + "name": "skills-local-update-test-renamed", + } + w := httptest.NewRecorder() + req := withURLParam(newRequest(http.MethodPatch, "/api/agents/"+agentID, body), "id", agentID) + testHandler.UpdateAgent(w, req) + if w.Code != http.StatusOK { + t.Fatalf("name-only update: expected 200, got %d: %s", w.Code, w.Body.String()) + } + var resp map[string]any + _ = json.NewDecoder(w.Body).Decode(&resp) + if resp["skills_local"] != "ignore" { + t.Errorf("name-only update silently changed skills_local: got %v, want ignore", resp["skills_local"]) + } + }) + + t.Run("explicit merge flips back", func(t *testing.T) { + body := map[string]any{ + "skills_local": "merge", + } + w := httptest.NewRecorder() + req := withURLParam(newRequest(http.MethodPatch, "/api/agents/"+agentID, body), "id", agentID) + testHandler.UpdateAgent(w, req) + if w.Code != http.StatusOK { + t.Fatalf("update skills_local=merge: expected 200, got %d: %s", w.Code, w.Body.String()) + } + var resp map[string]any + _ = json.NewDecoder(w.Body).Decode(&resp) + if resp["skills_local"] != "merge" { + t.Errorf("expected skills_local=merge after update, got %v", resp["skills_local"]) + } + }) + + t.Run("garbage value is 400 not silent default", func(t *testing.T) { + body := map[string]any{ + "skills_local": "all-of-them", + } + w := httptest.NewRecorder() + req := withURLParam(newRequest(http.MethodPatch, "/api/agents/"+agentID, body), "id", agentID) + testHandler.UpdateAgent(w, req) + if w.Code != http.StatusBadRequest { + t.Fatalf("garbage skills_local: expected 400, got %d: %s", w.Code, w.Body.String()) + } + }) +} + +// TestNormalizeSkillsLocal_DriftStaysSafe is a unit-level safety net for +// the on-read coercion: if a row gets written with anything other than +// the two recognised values (hand edits, future schema drift, older +// daemons sending up garbage), the API must surface "merge" — the +// platform default — never leak an indeterminate value to clients or the +// daemon. Only the exact literal "ignore" maps to ignore; anything else +// coerces to the documented default. +func TestNormalizeSkillsLocal_DriftStaysSafe(t *testing.T) { + cases := []struct { + stored string + want string + }{ + {"", "merge"}, + {"ignore", "ignore"}, + {"merge", "merge"}, + {"IGNORE", "merge"}, // case-sensitive — drift falls back to platform default + {"yes", "merge"}, + {"true", "merge"}, + } + for _, c := range cases { + if got := normalizeSkillsLocal(c.stored); got != c.want { + t.Errorf("normalizeSkillsLocal(%q) = %q, want %q", c.stored, got, c.want) + } + } +} diff --git a/server/internal/handler/agent_template.go b/server/internal/handler/agent_template.go index 68a82a20d..d82ee1ebb 100644 --- a/server/internal/handler/agent_template.go +++ b/server/internal/handler/agent_template.go @@ -446,6 +446,10 @@ func (h *Handler) CreateAgentFromTemplate(w http.ResponseWriter, r *http.Request CustomArgs: ca, McpConfig: nil, Model: pgtype.Text{String: req.Model, Valid: req.Model != ""}, + // skills_local has a NOT NULL CHECK ('ignore' | 'merge'); the Go zero + // value "" would fail the constraint instead of falling back to the + // column default. Match the public CreateAgent default explicitly. + SkillsLocal: "merge", }) if err != nil { // Mirror handler/agent.go:CreateAgent: when the duplicate is the diff --git a/server/internal/handler/daemon.go b/server/internal/handler/daemon.go index 6883ff1bc..d97ae8e67 100644 --- a/server/internal/handler/daemon.go +++ b/server/internal/handler/daemon.go @@ -1124,6 +1124,7 @@ func (h *Handler) ClaimTaskByRuntime(w http.ResponseWriter, r *http.Request) { McpConfig: mcpConfig, Model: agent.Model.String, ThinkingLevel: agent.ThinkingLevel.String, + SkillsLocal: normalizeSkillsLocal(agent.SkillsLocal), } } diff --git a/server/internal/handler/onboarding_shim.go b/server/internal/handler/onboarding_shim.go index fc075bf97..31e8b3546 100644 --- a/server/internal/handler/onboarding_shim.go +++ b/server/internal/handler/onboarding_shim.go @@ -227,6 +227,10 @@ func (h *Handler) BootstrapOnboardingRuntime(w http.ResponseWriter, r *http.Requ CustomArgs: []byte("[]"), McpConfig: nil, Model: pgtype.Text{}, + // skills_local has a NOT NULL CHECK ('ignore' | 'merge'); the Go + // zero value "" would trip the constraint rather than fall back to + // the column default. Match the public CreateAgent default. + SkillsLocal: "merge", }) if err != nil { slog.Warn("bootstrap onboarding (shim): create assistant failed", append(logger.RequestAttrs(r), "error", err, "workspace_id", req.WorkspaceID)...) diff --git a/server/migrations/108_agent_skills_local.down.sql b/server/migrations/108_agent_skills_local.down.sql new file mode 100644 index 000000000..2e023a3a7 --- /dev/null +++ b/server/migrations/108_agent_skills_local.down.sql @@ -0,0 +1 @@ +ALTER TABLE agent DROP COLUMN skills_local; diff --git a/server/migrations/108_agent_skills_local.up.sql b/server/migrations/108_agent_skills_local.up.sql new file mode 100644 index 000000000..d1ca4d7b4 --- /dev/null +++ b/server/migrations/108_agent_skills_local.up.sql @@ -0,0 +1,12 @@ +-- Per-agent switch controlling whether the Claude runtime merges the host +-- machine's `~/.claude/skills/` into the agent's skill set. 'merge' (default) +-- preserves the pre-existing inherit-from-machine behavior so existing +-- workflows that rely on locally installed skills keep working. 'ignore' +-- isolates the runtime so a broken local skill on one operator's machine +-- cannot silently crash a shared agent (GitHub #3052 — Claude exits before +-- reading stdin, leaving the daemon with "broken pipe"). Workspace skills +-- (`{workDir}/.claude/skills/`) are always loaded — the toggle only governs +-- the user-global directory. +ALTER TABLE agent + ADD COLUMN skills_local TEXT NOT NULL DEFAULT 'merge' + CHECK (skills_local IN ('ignore', 'merge')); diff --git a/server/pkg/agent/agent.go b/server/pkg/agent/agent.go index 9a08ea1bd..f07bc95e1 100644 --- a/server/pkg/agent/agent.go +++ b/server/pkg/agent/agent.go @@ -44,6 +44,19 @@ type ExecOptions struct { // field rather than fail (so MUL-2339 can grow runtime support // incrementally without breaking unrelated agents). ThinkingLevel string + // SkillsLocal opts the runtime into ("merge") or out of ("ignore") the + // host machine's user-global skill directory. Honoured by the Claude + // backend in this package — "merge" (platform default; anything other + // than "ignore") preserves the inherit-from-machine behavior so the + // CLI walks `~/.claude/` directly, and "ignore" redirects + // CLAUDE_CONFIG_DIR to an isolated per-task scratch dir so the CLI + // never reads `~/.claude/skills/` (used to harden shared agents + // against broken host skills — GitHub #3052). Codex enforces the same + // contract one layer up in the daemon: `execenv.hydrateCodexSkills` + // skips the `~/.codex/skills/` seed when this value is "ignore", so + // the per-task CODEX_HOME exposes only workspace skills. Other + // backends ignore this option today (no-op). + SkillsLocal string } // Session represents a running agent execution. diff --git a/server/pkg/agent/claude.go b/server/pkg/agent/claude.go index 2a73ef254..101fd8161 100644 --- a/server/pkg/agent/claude.go +++ b/server/pkg/agent/claude.go @@ -10,6 +10,7 @@ import ( "log/slog" "os" "os/exec" + "path/filepath" "strings" "time" ) @@ -66,7 +67,51 @@ func (b *claudeBackend) Execute(ctx context.Context, prompt string, opts ExecOpt if opts.Cwd != "" { cmd.Dir = opts.Cwd } - cmd.Env = buildEnv(b.cfg.Env) + + // Skill isolation. The platform default is "merge" — the CLI walks + // `~/.claude/` directly, including its `skills/`, so existing personal + // workflows that rely on locally installed Claude skills keep working + // out of the box. The agent owner can opt into "ignore" when a shared + // agent must be hardened against a broken local skill on one operator's + // machine, which otherwise crashes the CLI before it reads stdin + // (GitHub #3052: silent "broken pipe" exits). + // + // In "ignore" mode we point CLAUDE_CONFIG_DIR at a per-run scratch dir + // that mirrors `~/.claude/` as symlinks — except for `skills/`, which + // is omitted so the CLI cannot discover the host user's + // `~/.claude/skills/`. Everything else under `~/.claude/` — including + // the Linux/Windows `.credentials.json` login token, `settings.json`, + // `agents/`, `commands/`, `plugins/`, etc. — is symlinked through so + // Claude authentication and global config keep working without an + // `ANTHROPIC_API_KEY`. Workspace skills (`{cwd}/.claude/skills/`) load + // from cwd regardless and are not affected by either mode. + isolateClaudeConfig := opts.SkillsLocal == "ignore" + var claudeConfigDir string + var claudeConfigCleanup func() + if isolateClaudeConfig { + // Resolve the *effective* Claude config source before we strip + // CLAUDE_CONFIG_DIR from the child env in buildClaudeEnv. + // Precedence: agent custom_env > parent process env > `~/.claude`. + // Mirroring `~/.claude` blindly when the operator has explicitly + // pointed Claude at a different config dir (e.g. a managed install) + // would copy the wrong credentials into the scratch dir and break + // auth — see the second Must Fix in MUL-2603 review. + hostConfigDir := resolveHostClaudeConfigDir(b.cfg.Env) + dir, cleanup, err := newIsolatedClaudeConfigDir(opts.Cwd, hostConfigDir, b.cfg.Logger) + if err != nil { + cancel() + return nil, fmt.Errorf("isolate claude config dir: %w", err) + } + claudeConfigDir = dir + claudeConfigCleanup = cleanup + } + defer func() { + if claudeConfigCleanup != nil { + claudeConfigCleanup() + } + }() + + cmd.Env = buildClaudeEnv(b.cfg.Env, claudeConfigDir) stdout, err := cmd.StdoutPipe() if err != nil { @@ -111,10 +156,14 @@ func (b *claudeBackend) Execute(ctx context.Context, prompt string, opts ExecOpt } closeStdin() - b.cfg.Logger.Info("claude started", "pid", cmd.Process.Pid, "cwd", opts.Cwd, "model", opts.Model) + b.cfg.Logger.Info("claude started", "pid", cmd.Process.Pid, "cwd", opts.Cwd, "model", opts.Model, "skills_local", opts.SkillsLocal, "claude_config_dir", claudeConfigDir) // cmd.Start() succeeded — transfer temp file ownership to the goroutine. mcpFileCleanup = nil + // Transfer isolated-config cleanup the same way: the goroutine outlives + // this function, so it owns removal of the scratch dir. + isolatedConfigCleanup := claudeConfigCleanup + claudeConfigCleanup = nil msgCh := make(chan Message, 256) resCh := make(chan Result, 1) @@ -126,6 +175,9 @@ func (b *claudeBackend) Execute(ctx context.Context, prompt string, opts ExecOpt if mcpConfigPath != "" { defer os.Remove(mcpConfigPath) } + if isolatedConfigCleanup != nil { + defer isolatedConfigCleanup() + } startTime := time.Now() var output strings.Builder @@ -567,6 +619,31 @@ func buildEnv(extra map[string]string) []string { return mergeEnv(os.Environ(), extra) } +// buildClaudeEnv builds the env slice for the claude subprocess. When +// claudeConfigDir is non-empty (isolated-skills mode), it overrides +// CLAUDE_CONFIG_DIR for the child so the CLI looks at the per-task +// scratch dir instead of `~/.claude/`. We also strip the parent's +// CLAUDE_CONFIG_DIR so a daemon-host env var cannot accidentally win +// the override race. Callers in "merge" mode pass "" and behaviour +// matches the pre-MUL-2603 buildEnv path. +func buildClaudeEnv(extra map[string]string, claudeConfigDir string) []string { + env := mergeEnv(os.Environ(), extra) + if claudeConfigDir == "" { + return env + } + // Drop any CLAUDE_CONFIG_DIR already in the slice (from os.Environ or + // from custom_env) before appending the isolated override so the last + // entry wins deterministically. + filtered := env[:0] + for _, entry := range env { + if strings.HasPrefix(entry, "CLAUDE_CONFIG_DIR=") { + continue + } + filtered = append(filtered, entry) + } + return append(filtered, "CLAUDE_CONFIG_DIR="+claudeConfigDir) +} + func mergeEnv(base []string, extra map[string]string) []string { env := make([]string, 0, len(base)+len(extra)) for _, entry := range base { @@ -588,6 +665,175 @@ func isFilteredChildEnvKey(key string) bool { strings.HasPrefix(key, "CLAUDE_CODE_") } +// resolveHostClaudeConfigDir picks the directory we mirror into the per-task +// scratch CLAUDE_CONFIG_DIR. Precedence: +// +// 1. Agent custom_env CLAUDE_CONFIG_DIR (operator pinned this agent at a +// specific install — e.g. a managed shared profile). +// 2. Parent process CLAUDE_CONFIG_DIR (daemon-host env var; some self-host +// deployments set this globally so `claude login` writes there instead +// of `~/.claude`). +// 3. `~/.claude/` (the documented default for Linux/Windows; macOS keeps +// credentials in Keychain so the dir still holds settings/agents/etc.). +// +// Returns "" when none of the above are usable (no env, no home dir). The +// caller treats that as "host has no Claude config to mirror" — the scratch +// dir stays empty and the CLI relies on `ANTHROPIC_API_KEY` auth. +func resolveHostClaudeConfigDir(extraEnv map[string]string) string { + if v, ok := extraEnv["CLAUDE_CONFIG_DIR"]; ok && v != "" { + return v + } + if v := os.Getenv("CLAUDE_CONFIG_DIR"); v != "" { + return v + } + home, err := os.UserHomeDir() + if err != nil { + return "" + } + return filepath.Join(home, ".claude") +} + +// newIsolatedClaudeConfigDir creates a per-run scratch directory used as +// CLAUDE_CONFIG_DIR when the agent opted out of host-machine skill merging. +// The dir is placed under the task workdir when available so it lives and +// dies with the task's storage allocation; otherwise it falls back to the +// OS temp dir. +// +// The dir is NOT empty: every entry from the effective host Claude config +// (resolveHostClaudeConfigDir) is mirrored through *except* `skills/`. That +// preserves Claude Code's login state (`.credentials.json` on Linux/Windows), +// global settings, plugins, agents, commands, output styles, todos, etc. — +// which all live in this directory and would otherwise be invisible to the +// isolated child — while still keeping the user-global skills directory off +// the CLI's discovery path. Without this passthrough, opting into "ignore" +// mode on a non-API-key host would force the Claude agent to re-authenticate, +// breaking the documented "run `claude` once to log in" install flow. +// +// Each entry uses createDirLink / createFileLink, which try symlink first +// and fall back to a directory junction (Windows `mklink /J`) for dirs or a +// hardlink/copy for files when the platform rejects the symlink (Windows +// without Developer Mode / admin). Failing silently to symlink-only would +// leave Windows hosts with an empty scratch dir, defeating the auth +// passthrough — see MUL-2603 review. +// +// The returned cleanup removes the scratch directory; symlinks, junctions, +// and hardlinks are unlinked without touching the real host file. Safe to +// call from any goroutine exactly once. +func newIsolatedClaudeConfigDir(taskCwd, hostConfigDir string, logger *slog.Logger) (string, func(), error) { + parent := taskCwd + if parent == "" { + parent = os.TempDir() + } + dir, err := os.MkdirTemp(parent, "multica-claude-config-*") + if err != nil { + // Fall back to the OS temp dir if the cwd refused us (read-only + // volume, missing parent, etc.). A scratch dir somewhere on the + // host is still strictly better than letting the CLI auto-discover + // `~/.claude/skills/`. + if parent != os.TempDir() { + dir, err = os.MkdirTemp(os.TempDir(), "multica-claude-config-*") + } + if err != nil { + return "", nil, err + } + } + + if hostConfigDir != "" { + // Best effort. If the host config dir is missing entirely (env-var- + // auth-only setup) we just leave the scratch dir empty. If individual + // entries fail to mirror — even after fallback — we log so operators + // can correlate Claude Code auth failures with the missing mirror, + // but we do not abort: a partial mirror is still better than letting + // the CLI discover `~/.claude/skills/` directly. + if err := mirrorHostClaudeExceptSkills(hostConfigDir, dir); err != nil && logger != nil { + logger.Warn("claude: mirror host config dir failed", + "source", hostConfigDir, + "dest", dir, + "error", err, + ) + } + } + + cleanup := func() { + // Best effort — caller logs through normal channels if cleanup fails + // (a leftover scratch dir is non-fatal; the GC sweeper that reclaims + // orphaned task workdirs will catch it too). RemoveAll unlinks + // symlinks / junctions without following them, so the real host + // `~/.claude/` is not touched. Hardlinks share inodes so unlinking + // the mirror entry leaves the source file with refcount 1. + _ = os.RemoveAll(dir) + } + return dir, cleanup, nil +} + +// mirrorHostClaudeExceptSkills mirrors every direct entry under +// hostClaudeDir into destDir, skipping `skills/`. Each entry tries a +// symlink first and falls back to a directory junction (Windows +// `mklink /J`) or a hardlink/copy (Windows without Developer Mode) so the +// mirror still works on hosts that lack SeCreateSymbolicLinkPrivilege. The +// first per-entry error is returned so callers can log it; we do not abort +// the loop, because the goal — keeping `skills/` out of the CLI's discovery +// path — is met as soon as we own the scratch dir, and a partial mirror is +// still better than no isolation. +func mirrorHostClaudeExceptSkills(hostClaudeDir, destDir string) error { + return mirrorHostClaudeExceptSkillsWith(hostClaudeDir, destDir, createDirLink, createFileLink) +} + +// mirrorHostClaudeExceptSkillsWith is the testable seam behind +// mirrorHostClaudeExceptSkills. Tests inject custom dirLink / fileLink +// closures to exercise the symlink-failure fallback path on platforms +// (Linux/macOS) where os.Symlink would otherwise always succeed. +func mirrorHostClaudeExceptSkillsWith( + hostClaudeDir, destDir string, + dirLink, fileLink func(src, dst string) error, +) error { + entries, err := os.ReadDir(hostClaudeDir) + if err != nil { + return err + } + var firstErr error + for _, entry := range entries { + if entry.Name() == "skills" { + continue + } + src := filepath.Join(hostClaudeDir, entry.Name()) + dst := filepath.Join(destDir, entry.Name()) + var linkErr error + if entry.IsDir() { + linkErr = dirLink(src, dst) + } else { + linkErr = fileLink(src, dst) + } + if linkErr != nil && firstErr == nil { + firstErr = fmt.Errorf("link %s: %w", entry.Name(), linkErr) + } + } + return firstErr +} + +// copyFile copies the bytes of src into dst with a fresh file. Used as the +// last-resort fallback inside createFileLink on Windows when both symlink +// and hardlink are unavailable. Kept in the platform-agnostic file so the +// Unix build still compiles (it never calls copyFile, but the symbol must +// exist for the test fallback seam to be exercisable on Linux/macOS). +func copyFile(src, dst string) error { + in, err := os.Open(src) + if err != nil { + return fmt.Errorf("open %s: %w", src, err) + } + defer in.Close() + out, err := os.OpenFile(dst, os.O_WRONLY|os.O_CREATE|os.O_EXCL, 0o600) + if err != nil { + return fmt.Errorf("create %s: %w", dst, err) + } + defer out.Close() + if _, err := io.Copy(out, in); err != nil { + return fmt.Errorf("copy %s → %s: %w", src, dst, err) + } + return nil +} + + // blockedArgMode specifies whether a blocked arg takes a value or is standalone. type blockedArgMode int diff --git a/server/pkg/agent/claude_link_unix.go b/server/pkg/agent/claude_link_unix.go new file mode 100644 index 000000000..bb97e7e2d --- /dev/null +++ b/server/pkg/agent/claude_link_unix.go @@ -0,0 +1,20 @@ +//go:build !windows + +package agent + +import "os" + +// createDirLink symlinks src → dst for a directory entry mirrored into the +// per-task scratch CLAUDE_CONFIG_DIR. On Unix the kernel always honours +// symlinks, so a single os.Symlink call is sufficient and matches the +// existing behaviour. +func createDirLink(src, dst string) error { + return os.Symlink(src, dst) +} + +// createFileLink symlinks src → dst for a file entry mirrored into the +// per-task scratch CLAUDE_CONFIG_DIR. Unix has no equivalent of Windows's +// SeCreateSymbolicLinkPrivilege gate, so a single os.Symlink is enough. +func createFileLink(src, dst string) error { + return os.Symlink(src, dst) +} diff --git a/server/pkg/agent/claude_link_windows.go b/server/pkg/agent/claude_link_windows.go new file mode 100644 index 000000000..56f5892ff --- /dev/null +++ b/server/pkg/agent/claude_link_windows.go @@ -0,0 +1,45 @@ +//go:build windows + +package agent + +import ( + "fmt" + "os" + "os/exec" +) + +// createDirLink mirrors a directory from src to dst inside the per-task +// scratch CLAUDE_CONFIG_DIR. Windows os.Symlink requires +// SeCreateSymbolicLinkPrivilege (Developer Mode or admin), which most +// Multica CLI / Desktop installs do not have — without a fallback the +// mirror leaves an empty `~/.claude/`-equivalent, and the default +// "ignore" mode silently breaks Claude Code authentication on those +// hosts. A directory junction (`mklink /J`) does not require the +// privilege and behaves equivalently for our read-only mirror use, so +// it is the first fallback. See server/internal/daemon/execenv for the +// same pattern applied to CODEX_HOME. +func createDirLink(src, dst string) error { + if err := os.Symlink(src, dst); err == nil { + return nil + } + out, err := exec.Command("cmd", "/c", "mklink", "/J", dst, src).CombinedOutput() + if err != nil { + return fmt.Errorf("mklink /J %s %s: %s: %w", dst, src, out, err) + } + return nil +} + +// createFileLink mirrors a file from src to dst inside the per-task scratch +// CLAUDE_CONFIG_DIR. Tries symlink first, then hardlink (works on the same +// NTFS volume without elevated privileges and still shares content with the +// shared `~/.claude/.credentials.json` so token refreshes propagate), then +// a final content copy as a last resort. +func createFileLink(src, dst string) error { + if err := os.Symlink(src, dst); err == nil { + return nil + } + if err := os.Link(src, dst); err == nil { + return nil + } + return copyFile(src, dst) +} diff --git a/server/pkg/agent/claude_test.go b/server/pkg/agent/claude_test.go index f96dee036..fa061180b 100644 --- a/server/pkg/agent/claude_test.go +++ b/server/pkg/agent/claude_test.go @@ -4,6 +4,7 @@ import ( "bytes" "context" "encoding/json" + "errors" "log/slog" "os" "path/filepath" @@ -650,6 +651,367 @@ func mustMarshal(t *testing.T, v any) json.RawMessage { return data } +func TestClaudeExecuteIsolatesHostSkillsWhenIgnoreOptedIn(t *testing.T) { + t.Parallel() + if runtime.GOOS == "windows" { + t.Skip("shell-script fixture is POSIX-only") + } + + // Fake claude binary that prints its CLAUDE_CONFIG_DIR to stdout so we + // can confirm the runtime redirected the CLI off `~/.claude/` when the + // agent explicitly opted into "ignore" mode (the platform default is + // "merge", which preserves the host's CLAUDE_CONFIG_DIR). + fakePath := filepath.Join(t.TempDir(), "claude") + script := "#!/bin/sh\n" + + "cat >/dev/null\n" + + "printf '%s\\n' \"{\\\"type\\\":\\\"system\\\",\\\"session_id\\\":\\\"sess\\\"}\"\n" + + "printf '%s\\n' \"{\\\"type\\\":\\\"result\\\",\\\"subtype\\\":\\\"success\\\",\\\"is_error\\\":false,\\\"session_id\\\":\\\"sess\\\",\\\"result\\\":\\\"$CLAUDE_CONFIG_DIR\\\"}\"\n" + writeTestExecutable(t, fakePath, []byte(script)) + + backend, err := New("claude", Config{ExecutablePath: fakePath, Logger: slog.Default()}) + if err != nil { + t.Fatalf("new claude backend: %v", err) + } + + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + defer cancel() + cwd := t.TempDir() + // Explicit SkillsLocal == "ignore" → backend points CLAUDE_CONFIG_DIR at + // a per-task scratch dir under cwd. + session, err := backend.Execute(ctx, "ignored", ExecOptions{ + Cwd: cwd, + Timeout: 5 * time.Second, + SkillsLocal: "ignore", + }) + if err != nil { + t.Fatalf("execute: %v", err) + } + go func() { + for range session.Messages { + } + }() + + select { + case result := <-session.Result: + if result.Status != "completed" { + t.Fatalf("expected completed, got %q (err=%q)", result.Status, result.Error) + } + // The CLI saw a CLAUDE_CONFIG_DIR pointed at a multica-managed scratch + // dir under our task cwd, not the host user's ~/.claude. + got := strings.TrimSpace(result.Output) + if got == "" { + t.Fatalf("expected CLAUDE_CONFIG_DIR to be non-empty in ignore mode") + } + if !strings.Contains(got, "multica-claude-config-") { + t.Fatalf("expected isolated scratch dir, got %q", got) + } + if !strings.HasPrefix(got, cwd) { + t.Fatalf("expected isolated dir under %q, got %q", cwd, got) + } + case <-time.After(10 * time.Second): + t.Fatal("timeout waiting for result") + } +} + +func TestClaudeExecuteDefaultModeKeepsHostConfigDir(t *testing.T) { + // NOT parallel — t.Setenv mutates global env. + if runtime.GOOS == "windows" { + t.Skip("shell-script fixture is POSIX-only") + } + + // Default ExecOptions (no SkillsLocal) must preserve the host's + // CLAUDE_CONFIG_DIR — the platform default is "merge", which inherits + // the host's user-global skill directory (Bohan's product decision on + // MUL-2603: keep MUL-2603 hardening as an explicit opt-in to avoid + // regressing personal workflows that rely on locally installed skills). + fakePath := filepath.Join(t.TempDir(), "claude") + script := "#!/bin/sh\n" + + "cat >/dev/null\n" + + "printf '%s\\n' \"{\\\"type\\\":\\\"system\\\",\\\"session_id\\\":\\\"sess\\\"}\"\n" + + "printf '%s\\n' \"{\\\"type\\\":\\\"result\\\",\\\"subtype\\\":\\\"success\\\",\\\"is_error\\\":false,\\\"session_id\\\":\\\"sess\\\",\\\"result\\\":\\\"${CLAUDE_CONFIG_DIR:-unset}\\\"}\"\n" + writeTestExecutable(t, fakePath, []byte(script)) + + t.Setenv("CLAUDE_CONFIG_DIR", "/tmp/test-host-claude") + + backend, err := New("claude", Config{ExecutablePath: fakePath, Logger: slog.Default()}) + if err != nil { + t.Fatalf("new claude backend: %v", err) + } + + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + defer cancel() + // Default ExecOptions → SkillsLocal == "" → backend treats as merge + // (inherit-from-machine) and the host CLAUDE_CONFIG_DIR is preserved. + session, err := backend.Execute(ctx, "ignored", ExecOptions{ + Cwd: t.TempDir(), + Timeout: 5 * time.Second, + }) + if err != nil { + t.Fatalf("execute: %v", err) + } + go func() { + for range session.Messages { + } + }() + + select { + case result := <-session.Result: + if result.Status != "completed" { + t.Fatalf("expected completed, got %q (err=%q)", result.Status, result.Error) + } + got := strings.TrimSpace(result.Output) + if got != "/tmp/test-host-claude" { + t.Fatalf("expected host CLAUDE_CONFIG_DIR preserved in default mode, got %q", got) + } + case <-time.After(10 * time.Second): + t.Fatal("timeout waiting for result") + } +} + +func TestClaudeExecuteMergeModeKeepsHostConfigDir(t *testing.T) { + // NOT parallel — t.Setenv mutates global env which conflicts with + // concurrent tests reading CLAUDE_CONFIG_DIR or running under t.Parallel. + if runtime.GOOS == "windows" { + t.Skip("shell-script fixture is POSIX-only") + } + + fakePath := filepath.Join(t.TempDir(), "claude") + script := "#!/bin/sh\n" + + "cat >/dev/null\n" + + "printf '%s\\n' \"{\\\"type\\\":\\\"system\\\",\\\"session_id\\\":\\\"sess\\\"}\"\n" + + "printf '%s\\n' \"{\\\"type\\\":\\\"result\\\",\\\"subtype\\\":\\\"success\\\",\\\"is_error\\\":false,\\\"session_id\\\":\\\"sess\\\",\\\"result\\\":\\\"${CLAUDE_CONFIG_DIR:-unset}\\\"}\"\n" + writeTestExecutable(t, fakePath, []byte(script)) + + // Set a host-style CLAUDE_CONFIG_DIR so we can assert merge mode + // preserves it. The backend strips this in ignore mode but must leave + // it alone when the operator explicitly opted into merging. + t.Setenv("CLAUDE_CONFIG_DIR", "/tmp/test-host-claude") + + backend, err := New("claude", Config{ExecutablePath: fakePath, Logger: slog.Default()}) + if err != nil { + t.Fatalf("new claude backend: %v", err) + } + + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + defer cancel() + session, err := backend.Execute(ctx, "ignored", ExecOptions{ + Cwd: t.TempDir(), + Timeout: 5 * time.Second, + SkillsLocal: "merge", + }) + if err != nil { + t.Fatalf("execute: %v", err) + } + go func() { + for range session.Messages { + } + }() + + select { + case result := <-session.Result: + if result.Status != "completed" { + t.Fatalf("expected completed, got %q (err=%q)", result.Status, result.Error) + } + got := strings.TrimSpace(result.Output) + if got != "/tmp/test-host-claude" { + t.Fatalf("expected host CLAUDE_CONFIG_DIR preserved in merge mode, got %q", got) + } + case <-time.After(10 * time.Second): + t.Fatal("timeout waiting for result") + } +} + +func TestBuildClaudeEnvAppendsIsolatedConfigDir(t *testing.T) { + t.Parallel() + + env := buildClaudeEnv(nil, "/tmp/isolated-claude-config") + + var last string + hits := 0 + for _, entry := range env { + if strings.HasPrefix(entry, "CLAUDE_CONFIG_DIR=") { + hits++ + last = entry + } + } + if hits != 1 { + t.Fatalf("expected exactly one CLAUDE_CONFIG_DIR entry, got %d (%v)", hits, env) + } + if last != "CLAUDE_CONFIG_DIR=/tmp/isolated-claude-config" { + t.Fatalf("expected isolated CLAUDE_CONFIG_DIR override, got %q", last) + } +} + +func TestBuildClaudeEnvSkipsOverrideWhenEmpty(t *testing.T) { + t.Parallel() + + // Asking for "merge" mode passes "" through. We should not add a + // CLAUDE_CONFIG_DIR=… entry; the parent's value (if any) wins. + env := buildClaudeEnv(map[string]string{"FOO": "bar"}, "") + for _, entry := range env { + if strings.HasPrefix(entry, "CLAUDE_CONFIG_DIR=") { + // The parent env may legitimately have one set on a developer + // machine — only assert we did not *add* one. Find the unfiltered + // merge case to compare. + return + } + } +} + +func TestBuildClaudeEnvOverridesPreviousValue(t *testing.T) { + t.Parallel() + + // Even if custom_env supplies a CLAUDE_CONFIG_DIR, the isolated dir + // must take precedence: a stale custom_env entry must never be able + // to point the child back at `~/.claude/`. + env := buildClaudeEnv(map[string]string{"CLAUDE_CONFIG_DIR": "/etc/hostile"}, "/tmp/safe") + + hits := 0 + for _, entry := range env { + if entry == "CLAUDE_CONFIG_DIR=/etc/hostile" { + t.Fatalf("hostile custom_env CLAUDE_CONFIG_DIR was not stripped: %v", env) + } + if entry == "CLAUDE_CONFIG_DIR=/tmp/safe" { + hits++ + } + } + if hits != 1 { + t.Fatalf("expected isolated dir to be present exactly once, got %d (%v)", hits, env) + } +} + +func TestNewIsolatedClaudeConfigDirCreatesAndCleansUp(t *testing.T) { + t.Parallel() + + parent := t.TempDir() + dir, cleanup, err := newIsolatedClaudeConfigDir(parent, "", slog.Default()) + if err != nil { + t.Fatalf("newIsolatedClaudeConfigDir: %v", err) + } + defer cleanup() + + if dir == "" { + t.Fatal("expected non-empty dir") + } + if filepath.Dir(dir) != parent { + t.Fatalf("expected dir under %q, got %q", parent, dir) + } + if info, err := os.Stat(dir); err != nil || !info.IsDir() { + t.Fatalf("expected created dir to exist, stat err=%v", err) + } + // The dir may contain symlinks mirrored from the host's ~/.claude/ (auth + // token, settings, etc. — see mirrorHostClaudeExceptSkills). What it must + // NOT contain is a `skills/` entry; that is the entire point of the + // isolation. Mirroring behaviour is exercised in dedicated tests below. + if _, err := os.Lstat(filepath.Join(dir, "skills")); !os.IsNotExist(err) { + t.Fatalf("expected no skills/ entry in isolated dir, stat err=%v", err) + } + + // Cleanup is idempotent (Execute may double-defer in error paths). + cleanup() + cleanup() + if _, err := os.Stat(dir); !os.IsNotExist(err) { + t.Fatalf("expected dir removed, stat err=%v", err) + } +} + +// TestMirrorHostClaudeExceptSkills_PassesAuthAndSkipsSkills locks in the +// invariant Elon flagged in review: the isolation must not also isolate the +// Claude login token. Mirror reaches every non-skills entry — including +// `.credentials.json`, the Linux/Windows store for the OAuth token — so a +// host that has only "run `claude` to log in" (no ANTHROPIC_API_KEY) still +// authenticates inside the isolated config dir. +func TestMirrorHostClaudeExceptSkills_PassesAuthAndSkipsSkills(t *testing.T) { + t.Parallel() + + host := t.TempDir() + // Populate a realistic-ish ~/.claude/ layout. + for _, sub := range []string{"skills", "agents", "commands", "plugins"} { + if err := os.MkdirAll(filepath.Join(host, sub), 0o755); err != nil { + t.Fatalf("mkdir %s: %v", sub, err) + } + } + // A broken skill in `skills/`: if the mirror passes it through, the + // regression Elon's review highlighted is back. + if err := os.WriteFile(filepath.Join(host, "skills", "broken.md"), []byte("frontmatter-corrupt"), 0o644); err != nil { + t.Fatalf("write broken skill: %v", err) + } + // The OAuth credential file is the asset the reviewer specifically flagged. + if err := os.WriteFile(filepath.Join(host, ".credentials.json"), []byte(`{"token":"abc"}`), 0o600); err != nil { + t.Fatalf("write credentials: %v", err) + } + if err := os.WriteFile(filepath.Join(host, "settings.json"), []byte(`{}`), 0o644); err != nil { + t.Fatalf("write settings: %v", err) + } + + dest := t.TempDir() + if err := mirrorHostClaudeExceptSkills(host, dest); err != nil { + t.Fatalf("mirror: %v", err) + } + + // skills/ must be absent — claude would otherwise discover the broken + // host skill through the symlink and crash before reading stdin (#3052). + if _, err := os.Lstat(filepath.Join(dest, "skills")); !os.IsNotExist(err) { + t.Fatalf("expected skills/ to be skipped, got stat err=%v", err) + } + + // Everything else must reach the isolated dir with the same contents as + // the host source. We resolve through the mirrored entry to confirm the + // CLI sees real bytes whether the implementation used a symlink (Unix), + // a junction (Windows), a hardlink (Windows no Developer Mode, same + // volume), or a content copy (last-resort fallback). os.Stat follows + // every variant, so the assertion is platform-agnostic. + for _, expected := range []string{".credentials.json", "settings.json", "agents", "commands", "plugins"} { + dst := filepath.Join(dest, expected) + if _, err := os.Stat(dst); err != nil { + t.Fatalf("expected %s mirrored and reachable, stat err=%v", expected, err) + } + } + + // The credential file should round-trip — claude reading + // $CLAUDE_CONFIG_DIR/.credentials.json must see the live host token. + got, err := os.ReadFile(filepath.Join(dest, ".credentials.json")) + if err != nil { + t.Fatalf("read mirrored credentials: %v", err) + } + if string(got) != `{"token":"abc"}` { + t.Fatalf("mirrored credentials content drifted, got %q", got) + } +} + +// TestMirrorHostClaudeExceptSkills_MissingHostDirIsNoop documents that a host +// with no `~/.claude/` (env-var-auth-only setups) is a supported state, not +// an error. The isolated dir simply stays empty. +func TestMirrorHostClaudeExceptSkills_MissingHostDirIsNoop(t *testing.T) { + t.Parallel() + + dest := t.TempDir() + err := mirrorHostClaudeExceptSkills(filepath.Join(t.TempDir(), "nope"), dest) + if err == nil { + t.Fatal("expected a read error for missing host dir") + } + // dest must remain untouched (no skills/, no other entries). + entries, _ := os.ReadDir(dest) + if len(entries) != 0 { + t.Fatalf("expected empty dest on missing host, got %d entries", len(entries)) + } +} + +func TestNewIsolatedClaudeConfigDirFallsBackToTemp(t *testing.T) { + t.Parallel() + + // Empty cwd → falls back to OS temp dir without erroring. + dir, cleanup, err := newIsolatedClaudeConfigDir("", "", slog.Default()) + if err != nil { + t.Fatalf("expected fallback to OS temp, got err=%v", err) + } + defer cleanup() + + if !strings.HasPrefix(dir, os.TempDir()) { + t.Fatalf("expected fallback under %q, got %q", os.TempDir(), dir) + } +} + func TestBuildClaudeArgsExtraArgsBeforeCustomArgsAndFiltersBoth(t *testing.T) { args := buildClaudeArgs(ExecOptions{ ExtraArgs: []string{"--output-format", "text", "--max-budget-usd", "1.00"}, @@ -672,3 +1034,344 @@ func TestBuildClaudeArgsExtraArgsBeforeCustomArgsAndFiltersBoth(t *testing.T) { t.Fatalf("expected extra args before custom args, got %v", args) } } + +// TestResolveHostClaudeConfigDir locks in the precedence Elon's review asked +// for: agent custom_env wins over a daemon-host CLAUDE_CONFIG_DIR, which +// wins over the documented default at `~/.claude`. Without this, switching +// default mode to "ignore" would mirror the wrong source dir and overwrite +// a valid CLAUDE_CONFIG_DIR pointing at e.g. a managed shared profile. +func TestResolveHostClaudeConfigDir(t *testing.T) { + // NOT parallel — t.Setenv mutates global env which conflicts with + // concurrent tests reading CLAUDE_CONFIG_DIR. + + // 1. Agent custom_env wins over parent env. + t.Setenv("CLAUDE_CONFIG_DIR", "/from/parent/env") + got := resolveHostClaudeConfigDir(map[string]string{"CLAUDE_CONFIG_DIR": "/from/custom/env"}) + if got != "/from/custom/env" { + t.Fatalf("agent custom_env should win, got %q", got) + } + + // 2. Empty custom_env falls back to parent env. + got = resolveHostClaudeConfigDir(map[string]string{"CLAUDE_CONFIG_DIR": ""}) + if got != "/from/parent/env" { + t.Fatalf("parent env should win when custom_env is empty, got %q", got) + } + got = resolveHostClaudeConfigDir(nil) + if got != "/from/parent/env" { + t.Fatalf("parent env should win when custom_env is nil, got %q", got) + } + + // 3. With neither set, falls back to `~/.claude`. + t.Setenv("CLAUDE_CONFIG_DIR", "") + got = resolveHostClaudeConfigDir(nil) + home, err := os.UserHomeDir() + if err != nil { + t.Skipf("no UserHomeDir on this host: %v", err) + } + want := filepath.Join(home, ".claude") + if got != want { + t.Fatalf("default should be %q, got %q", want, got) + } +} + +// TestNewIsolatedClaudeConfigDirMirrorsCustomHostDir confirms the scratch +// dir reflects the effective CLAUDE_CONFIG_DIR source, not unconditionally +// `~/.claude/`. Previously the mirror was hardcoded to UserHomeDir, so an +// operator who pinned CLAUDE_CONFIG_DIR at a managed install would get the +// wrong credentials in the scratch dir. +func TestNewIsolatedClaudeConfigDirMirrorsCustomHostDir(t *testing.T) { + t.Parallel() + + host := t.TempDir() + if err := os.WriteFile(filepath.Join(host, ".credentials.json"), []byte(`{"token":"from-custom-host"}`), 0o600); err != nil { + t.Fatalf("seed credentials: %v", err) + } + + dir, cleanup, err := newIsolatedClaudeConfigDir(t.TempDir(), host, slog.Default()) + if err != nil { + t.Fatalf("newIsolatedClaudeConfigDir: %v", err) + } + defer cleanup() + + got, err := os.ReadFile(filepath.Join(dir, ".credentials.json")) + if err != nil { + t.Fatalf("read mirrored credentials from custom host: %v", err) + } + if string(got) != `{"token":"from-custom-host"}` { + t.Fatalf("mirror sourced from wrong dir: got %q", got) + } +} + +// TestNewIsolatedClaudeConfigDirEmptyHostIsNoop documents the env-var-auth +// case: with no host config dir (host has no `~/.claude/` and no +// CLAUDE_CONFIG_DIR set anywhere), the scratch dir is created but empty and +// nothing is mirrored. The CLI runs with `ANTHROPIC_API_KEY` only. +func TestNewIsolatedClaudeConfigDirEmptyHostIsNoop(t *testing.T) { + t.Parallel() + + dir, cleanup, err := newIsolatedClaudeConfigDir(t.TempDir(), "", slog.Default()) + if err != nil { + t.Fatalf("newIsolatedClaudeConfigDir: %v", err) + } + defer cleanup() + + entries, err := os.ReadDir(dir) + if err != nil { + t.Fatalf("read scratch dir: %v", err) + } + if len(entries) != 0 { + t.Fatalf("expected empty scratch dir with no host source, got %d entries", len(entries)) + } +} + +// TestMirrorHostClaudeExceptSkillsWith_FallbackWhenSymlinkFails locks in the +// Windows-no-Developer-Mode behaviour Elon's review asked for: when symlink +// raises a permission error, the mirror still places the entry in the +// scratch dir via a fallback (junction for dirs, hardlink/copy for files). +// Tested via the lower-level seam so the assertion runs on Linux/macOS CI; +// the production createDirLink / createFileLink wrappers encode the same +// "try symlink first, then fall back" chain in their platform builds. +func TestMirrorHostClaudeExceptSkillsWith_FallbackWhenSymlinkFails(t *testing.T) { + t.Parallel() + + host := t.TempDir() + if err := os.WriteFile(filepath.Join(host, ".credentials.json"), []byte(`{"token":"fallback"}`), 0o600); err != nil { + t.Fatalf("seed credentials: %v", err) + } + if err := os.MkdirAll(filepath.Join(host, "agents"), 0o755); err != nil { + t.Fatalf("seed agents dir: %v", err) + } + if err := os.WriteFile(filepath.Join(host, "agents", "global.md"), []byte("global agent"), 0o600); err != nil { + t.Fatalf("seed agent file: %v", err) + } + // `skills/` must still be skipped even when the linker reports an + // error — the broken-skill GitHub #3052 regression must not slip back. + if err := os.MkdirAll(filepath.Join(host, "skills"), 0o755); err != nil { + t.Fatalf("seed skills dir: %v", err) + } + if err := os.WriteFile(filepath.Join(host, "skills", "broken.md"), []byte("frontmatter-corrupt"), 0o600); err != nil { + t.Fatalf("seed broken skill: %v", err) + } + + dest := t.TempDir() + + // Simulated "Windows without Developer Mode": symlink always returns + // EPERM. The fallback path must still land the entry in dest. + failedSymlinkAttempts := 0 + fakeSymlinkErr := errors.New("simulated EPERM: symlink not permitted") + dirLink := func(src, dst string) error { + if err := os.Symlink(src, dst); err == nil { + // Forcing failure: if a symlink would have worked, pretend it + // didn't and engage the junction equivalent. We mimic the + // junction by using os.MkdirAll (a junction behaves like a + // directory entry from userspace) and copying the immediate + // child files into it. For the test we only need the entry to + // exist and be reachable; we do not need real recursive + // equivalence. + _ = os.Remove(dst) + } + failedSymlinkAttempts++ + _ = fakeSymlinkErr // referenced so the simulated-EPERM error is documented in the test body + if err := os.MkdirAll(dst, 0o755); err != nil { + return err + } + // Copy any direct children so the destination is non-empty and + // the test can read through it. + entries, err := os.ReadDir(src) + if err != nil { + return err + } + for _, e := range entries { + data, err := os.ReadFile(filepath.Join(src, e.Name())) + if err != nil { + continue + } + _ = os.WriteFile(filepath.Join(dst, e.Name()), data, 0o600) + } + return nil + } + fileLink := func(src, dst string) error { + if err := os.Symlink(src, dst); err == nil { + _ = os.Remove(dst) + } + failedSymlinkAttempts++ + // Hardlink fallback first (this is what createFileLink does on + // Windows when symlink is denied but the source/dest share a + // volume). If hardlink also fails (e.g. cross-volume), fall back + // to a content copy. + if err := os.Link(src, dst); err == nil { + return nil + } + return copyFile(src, dst) + } + + if err := mirrorHostClaudeExceptSkillsWith(host, dest, dirLink, fileLink); err != nil { + t.Fatalf("mirror with failing symlink: %v", err) + } + + if failedSymlinkAttempts == 0 { + t.Fatalf("expected fallback path to engage at least once") + } + + // `.credentials.json` must round-trip through whatever fallback the + // file linker used (hardlink or copy). This is the assertion Elon's + // review pinned to "no `.credentials.json` ⇒ default ignore breaks + // Claude Code auth on Windows". + got, err := os.ReadFile(filepath.Join(dest, ".credentials.json")) + if err != nil { + t.Fatalf("read mirrored credentials after fallback: %v", err) + } + if string(got) != `{"token":"fallback"}` { + t.Fatalf("mirrored credentials drifted after fallback, got %q", got) + } + + // Sub-directories must also reach the destination via the dir + // fallback (junction equivalent). We test reachability + child + // content rather than the underlying file kind, because junctions, + // symlinks, and the test's copy-based stand-in all present the same + // userspace view. + if _, err := os.Stat(filepath.Join(dest, "agents")); err != nil { + t.Fatalf("agents/ not mirrored: %v", err) + } + gotChild, err := os.ReadFile(filepath.Join(dest, "agents", "global.md")) + if err != nil { + t.Fatalf("read agent child file: %v", err) + } + if string(gotChild) != "global agent" { + t.Fatalf("agent child content drifted, got %q", gotChild) + } + + // `skills/` must be absent regardless of fallback engagement. + if _, err := os.Lstat(filepath.Join(dest, "skills")); !os.IsNotExist(err) { + t.Fatalf("skills/ leaked into scratch dir on fallback path, stat err=%v", err) + } +} + +// TestMirrorHostClaudeExceptSkillsWith_PropagatesFirstLinkError makes sure +// callers see a per-entry link failure when even the fallback fails — the +// scratch-dir caller logs the error so operators chasing auth issues on +// Windows can correlate the missing mirror with their permission setup. +func TestMirrorHostClaudeExceptSkillsWith_PropagatesFirstLinkError(t *testing.T) { + t.Parallel() + + host := t.TempDir() + if err := os.WriteFile(filepath.Join(host, ".credentials.json"), []byte("x"), 0o600); err != nil { + t.Fatalf("seed credentials: %v", err) + } + + hardFail := errors.New("link refused after fallback") + fail := func(src, dst string) error { return hardFail } + + err := mirrorHostClaudeExceptSkillsWith(host, t.TempDir(), fail, fail) + if err == nil { + t.Fatal("expected an error when every linker fails") + } + if !errors.Is(err, hardFail) { + t.Fatalf("expected wrapped hardFail, got %v", err) + } +} + +// TestCopyFileRoundTrip exercises the last-resort content-copy fallback used +// by createFileLink on Windows when both symlink and hardlink are +// unavailable (e.g. cross-volume scratch dir). The copy must produce a +// byte-for-byte equivalent destination so Claude Code reads the real +// credential bytes. +func TestCopyFileRoundTrip(t *testing.T) { + t.Parallel() + + src := filepath.Join(t.TempDir(), "creds") + if err := os.WriteFile(src, []byte(`{"token":"abc"}`), 0o600); err != nil { + t.Fatalf("seed source: %v", err) + } + dst := filepath.Join(t.TempDir(), "creds-copy") + if err := copyFile(src, dst); err != nil { + t.Fatalf("copyFile: %v", err) + } + got, err := os.ReadFile(dst) + if err != nil { + t.Fatalf("read copy: %v", err) + } + if string(got) != `{"token":"abc"}` { + t.Fatalf("copy drifted, got %q", got) + } + // EXCL semantics — copyFile refuses to overwrite an existing file so + // a stale mirror entry never silently shadows a fresh source. + if err := copyFile(src, dst); err == nil { + t.Fatal("expected copyFile to refuse overwriting existing dst") + } +} + +// TestClaudeExecuteIsolatesUsesCustomEnvSource confirms the runtime mirrors +// from the agent's custom_env CLAUDE_CONFIG_DIR — the exact bug Elon's +// review flagged: when an operator pins CLAUDE_CONFIG_DIR via custom_env, +// the scratch dir must mirror *that* source, not `~/.claude`. Otherwise +// default `ignore` mode would silently load the wrong credentials. +func TestClaudeExecuteIsolatesUsesCustomEnvSource(t *testing.T) { + t.Parallel() + if runtime.GOOS == "windows" { + t.Skip("shell-script fixture is POSIX-only") + } + + // Build a synthetic "host Claude config dir" the agent will pin via + // custom_env. The mirror should land its `.credentials.json` in the + // scratch dir. The token value is a plain quote-free string so the + // fake-claude shell script can echo it through stream-json's `result` + // field without escape gymnastics. + customHost := t.TempDir() + const expectedToken = "from-custom-host-token-ok" + if err := os.WriteFile(filepath.Join(customHost, ".credentials.json"), []byte(expectedToken), 0o600); err != nil { + t.Fatalf("seed credentials: %v", err) + } + + // Fake claude binary that prints the mirrored credentials content from + // the scratch CLAUDE_CONFIG_DIR — we then assert that we see the + // custom host's token, not whatever lives in real `~/.claude/`. + fakePath := filepath.Join(t.TempDir(), "claude") + script := "#!/bin/sh\n" + + "cat >/dev/null\n" + + "creds=$(cat \"$CLAUDE_CONFIG_DIR/.credentials.json\" 2>/dev/null || echo MISSING)\n" + + "printf '%s\\n' \"{\\\"type\\\":\\\"system\\\",\\\"session_id\\\":\\\"sess\\\"}\"\n" + + "printf '%s\\n' \"{\\\"type\\\":\\\"result\\\",\\\"subtype\\\":\\\"success\\\",\\\"is_error\\\":false,\\\"session_id\\\":\\\"sess\\\",\\\"result\\\":\\\"$creds\\\"}\"\n" + writeTestExecutable(t, fakePath, []byte(script)) + + backend, err := New("claude", Config{ + ExecutablePath: fakePath, + Logger: slog.Default(), + Env: map[string]string{"CLAUDE_CONFIG_DIR": customHost}, + }) + if err != nil { + t.Fatalf("new claude backend: %v", err) + } + + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + defer cancel() + // Explicit SkillsLocal == "ignore" → backend builds scratch dir mirrored + // from custom_env CLAUDE_CONFIG_DIR. (The platform default is "merge", + // which would just preserve the host CLAUDE_CONFIG_DIR untouched and + // never exercise the mirror path — see MUL-2603 product decision.) + session, err := backend.Execute(ctx, "ignored", ExecOptions{ + Cwd: t.TempDir(), + Timeout: 5 * time.Second, + SkillsLocal: "ignore", + }) + if err != nil { + t.Fatalf("execute: %v", err) + } + go func() { + for range session.Messages { + } + }() + + select { + case result := <-session.Result: + if result.Status != "completed" { + t.Fatalf("expected completed, got %q (err=%q)", result.Status, result.Error) + } + got := strings.TrimSpace(result.Output) + if got != expectedToken { + t.Fatalf("expected credentials mirrored from custom CLAUDE_CONFIG_DIR, got %q", got) + } + case <-time.After(10 * time.Second): + t.Fatal("timeout waiting for result") + } +} diff --git a/server/pkg/db/generated/agent.sql.go b/server/pkg/db/generated/agent.sql.go index 44848558f..1c78a47e0 100644 --- a/server/pkg/db/generated/agent.sql.go +++ b/server/pkg/db/generated/agent.sql.go @@ -14,7 +14,7 @@ import ( const archiveAgent = `-- name: ArchiveAgent :one UPDATE agent SET archived_at = now(), archived_by = $2, updated_at = now() WHERE id = $1 -RETURNING id, workspace_id, name, avatar_url, runtime_mode, runtime_config, visibility, status, max_concurrent_tasks, owner_id, created_at, updated_at, description, runtime_id, instructions, archived_at, archived_by, custom_env, custom_args, mcp_config, model, thinking_level +RETURNING id, workspace_id, name, avatar_url, runtime_mode, runtime_config, visibility, status, max_concurrent_tasks, owner_id, created_at, updated_at, description, runtime_id, instructions, archived_at, archived_by, custom_env, custom_args, mcp_config, model, thinking_level, skills_local ` type ArchiveAgentParams struct { @@ -48,6 +48,7 @@ func (q *Queries) ArchiveAgent(ctx context.Context, arg ArchiveAgentParams) (Age &i.McpConfig, &i.Model, &i.ThinkingLevel, + &i.SkillsLocal, ) return i, err } @@ -56,7 +57,7 @@ const archiveAgentsByRuntime = `-- name: ArchiveAgentsByRuntime :many UPDATE agent SET archived_at = now(), archived_by = $1, updated_at = now() WHERE runtime_id = ANY($2::uuid[]) AND archived_at IS NULL -RETURNING id, workspace_id, name, avatar_url, runtime_mode, runtime_config, visibility, status, max_concurrent_tasks, owner_id, created_at, updated_at, description, runtime_id, instructions, archived_at, archived_by, custom_env, custom_args, mcp_config, model, thinking_level +RETURNING id, workspace_id, name, avatar_url, runtime_mode, runtime_config, visibility, status, max_concurrent_tasks, owner_id, created_at, updated_at, description, runtime_id, instructions, archived_at, archived_by, custom_env, custom_args, mcp_config, model, thinking_level, skills_local ` type ArchiveAgentsByRuntimeParams struct { @@ -100,6 +101,7 @@ func (q *Queries) ArchiveAgentsByRuntime(ctx context.Context, arg ArchiveAgentsB &i.McpConfig, &i.Model, &i.ThinkingLevel, + &i.SkillsLocal, ); err != nil { return nil, err } @@ -520,7 +522,7 @@ func (q *Queries) ClaimAgentTask(ctx context.Context, agentID pgtype.UUID) (Agen const clearAgentMcpConfig = `-- name: ClearAgentMcpConfig :one UPDATE agent SET mcp_config = NULL, updated_at = now() WHERE id = $1 -RETURNING id, workspace_id, name, avatar_url, runtime_mode, runtime_config, visibility, status, max_concurrent_tasks, owner_id, created_at, updated_at, description, runtime_id, instructions, archived_at, archived_by, custom_env, custom_args, mcp_config, model, thinking_level +RETURNING id, workspace_id, name, avatar_url, runtime_mode, runtime_config, visibility, status, max_concurrent_tasks, owner_id, created_at, updated_at, description, runtime_id, instructions, archived_at, archived_by, custom_env, custom_args, mcp_config, model, thinking_level, skills_local ` func (q *Queries) ClearAgentMcpConfig(ctx context.Context, id pgtype.UUID) (Agent, error) { @@ -549,6 +551,7 @@ func (q *Queries) ClearAgentMcpConfig(ctx context.Context, id pgtype.UUID) (Agen &i.McpConfig, &i.Model, &i.ThinkingLevel, + &i.SkillsLocal, ) return i, err } @@ -556,7 +559,7 @@ func (q *Queries) ClearAgentMcpConfig(ctx context.Context, id pgtype.UUID) (Agen const clearAgentThinkingLevel = `-- name: ClearAgentThinkingLevel :one UPDATE agent SET thinking_level = NULL, updated_at = now() WHERE id = $1 -RETURNING id, workspace_id, name, avatar_url, runtime_mode, runtime_config, visibility, status, max_concurrent_tasks, owner_id, created_at, updated_at, description, runtime_id, instructions, archived_at, archived_by, custom_env, custom_args, mcp_config, model, thinking_level +RETURNING id, workspace_id, name, avatar_url, runtime_mode, runtime_config, visibility, status, max_concurrent_tasks, owner_id, created_at, updated_at, description, runtime_id, instructions, archived_at, archived_by, custom_env, custom_args, mcp_config, model, thinking_level, skills_local ` // Explicit NULL-clear for thinking_level. COALESCE-based UpdateAgent cannot @@ -588,6 +591,7 @@ func (q *Queries) ClearAgentThinkingLevel(ctx context.Context, id pgtype.UUID) ( &i.McpConfig, &i.Model, &i.ThinkingLevel, + &i.SkillsLocal, ) return i, err } @@ -660,9 +664,10 @@ const createAgent = `-- name: CreateAgent :one INSERT INTO agent ( workspace_id, name, description, avatar_url, runtime_mode, runtime_config, runtime_id, visibility, max_concurrent_tasks, owner_id, - instructions, custom_env, custom_args, mcp_config, model, thinking_level -) VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11, $12, $13, $14, $15, $16) -RETURNING id, workspace_id, name, avatar_url, runtime_mode, runtime_config, visibility, status, max_concurrent_tasks, owner_id, created_at, updated_at, description, runtime_id, instructions, archived_at, archived_by, custom_env, custom_args, mcp_config, model, thinking_level + instructions, custom_env, custom_args, mcp_config, model, thinking_level, + skills_local +) VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11, $12, $13, $14, $15, $16, $17) +RETURNING id, workspace_id, name, avatar_url, runtime_mode, runtime_config, visibility, status, max_concurrent_tasks, owner_id, created_at, updated_at, description, runtime_id, instructions, archived_at, archived_by, custom_env, custom_args, mcp_config, model, thinking_level, skills_local ` type CreateAgentParams struct { @@ -682,6 +687,7 @@ type CreateAgentParams struct { McpConfig []byte `json:"mcp_config"` Model pgtype.Text `json:"model"` ThinkingLevel pgtype.Text `json:"thinking_level"` + SkillsLocal string `json:"skills_local"` } func (q *Queries) CreateAgent(ctx context.Context, arg CreateAgentParams) (Agent, error) { @@ -702,6 +708,7 @@ func (q *Queries) CreateAgent(ctx context.Context, arg CreateAgentParams) (Agent arg.McpConfig, arg.Model, arg.ThinkingLevel, + arg.SkillsLocal, ) var i Agent err := row.Scan( @@ -727,6 +734,7 @@ func (q *Queries) CreateAgent(ctx context.Context, arg CreateAgentParams) (Agent &i.McpConfig, &i.Model, &i.ThinkingLevel, + &i.SkillsLocal, ) return i, err } @@ -1142,7 +1150,7 @@ func (q *Queries) FailStaleTasks(ctx context.Context, arg FailStaleTasksParams) } const getAgent = `-- name: GetAgent :one -SELECT id, workspace_id, name, avatar_url, runtime_mode, runtime_config, visibility, status, max_concurrent_tasks, owner_id, created_at, updated_at, description, runtime_id, instructions, archived_at, archived_by, custom_env, custom_args, mcp_config, model, thinking_level FROM agent +SELECT id, workspace_id, name, avatar_url, runtime_mode, runtime_config, visibility, status, max_concurrent_tasks, owner_id, created_at, updated_at, description, runtime_id, instructions, archived_at, archived_by, custom_env, custom_args, mcp_config, model, thinking_level, skills_local FROM agent WHERE id = $1 ` @@ -1172,12 +1180,13 @@ func (q *Queries) GetAgent(ctx context.Context, id pgtype.UUID) (Agent, error) { &i.McpConfig, &i.Model, &i.ThinkingLevel, + &i.SkillsLocal, ) return i, err } const getAgentInWorkspace = `-- name: GetAgentInWorkspace :one -SELECT id, workspace_id, name, avatar_url, runtime_mode, runtime_config, visibility, status, max_concurrent_tasks, owner_id, created_at, updated_at, description, runtime_id, instructions, archived_at, archived_by, custom_env, custom_args, mcp_config, model, thinking_level FROM agent +SELECT id, workspace_id, name, avatar_url, runtime_mode, runtime_config, visibility, status, max_concurrent_tasks, owner_id, created_at, updated_at, description, runtime_id, instructions, archived_at, archived_by, custom_env, custom_args, mcp_config, model, thinking_level, skills_local FROM agent WHERE id = $1 AND workspace_id = $2 ` @@ -1212,6 +1221,7 @@ func (q *Queries) GetAgentInWorkspace(ctx context.Context, arg GetAgentInWorkspa &i.McpConfig, &i.Model, &i.ThinkingLevel, + &i.SkillsLocal, ) return i, err } @@ -1625,7 +1635,7 @@ func (q *Queries) ListAgentTasks(ctx context.Context, agentID pgtype.UUID) ([]Ag } const listAgents = `-- name: ListAgents :many -SELECT id, workspace_id, name, avatar_url, runtime_mode, runtime_config, visibility, status, max_concurrent_tasks, owner_id, created_at, updated_at, description, runtime_id, instructions, archived_at, archived_by, custom_env, custom_args, mcp_config, model, thinking_level FROM agent +SELECT id, workspace_id, name, avatar_url, runtime_mode, runtime_config, visibility, status, max_concurrent_tasks, owner_id, created_at, updated_at, description, runtime_id, instructions, archived_at, archived_by, custom_env, custom_args, mcp_config, model, thinking_level, skills_local FROM agent WHERE workspace_id = $1 AND archived_at IS NULL ORDER BY created_at ASC ` @@ -1662,6 +1672,7 @@ func (q *Queries) ListAgents(ctx context.Context, workspaceID pgtype.UUID) ([]Ag &i.McpConfig, &i.Model, &i.ThinkingLevel, + &i.SkillsLocal, ); err != nil { return nil, err } @@ -1674,7 +1685,7 @@ func (q *Queries) ListAgents(ctx context.Context, workspaceID pgtype.UUID) ([]Ag } const listAllAgents = `-- name: ListAllAgents :many -SELECT id, workspace_id, name, avatar_url, runtime_mode, runtime_config, visibility, status, max_concurrent_tasks, owner_id, created_at, updated_at, description, runtime_id, instructions, archived_at, archived_by, custom_env, custom_args, mcp_config, model, thinking_level FROM agent +SELECT id, workspace_id, name, avatar_url, runtime_mode, runtime_config, visibility, status, max_concurrent_tasks, owner_id, created_at, updated_at, description, runtime_id, instructions, archived_at, archived_by, custom_env, custom_args, mcp_config, model, thinking_level, skills_local FROM agent WHERE workspace_id = $1 ORDER BY created_at ASC ` @@ -1711,6 +1722,7 @@ func (q *Queries) ListAllAgents(ctx context.Context, workspaceID pgtype.UUID) ([ &i.McpConfig, &i.Model, &i.ThinkingLevel, + &i.SkillsLocal, ); err != nil { return nil, err } @@ -2095,7 +2107,7 @@ SET status = CASE WHEN EXISTS ( ) THEN 'working' ELSE 'idle' END, updated_at = now() WHERE a.id = $1 -RETURNING id, workspace_id, name, avatar_url, runtime_mode, runtime_config, visibility, status, max_concurrent_tasks, owner_id, created_at, updated_at, description, runtime_id, instructions, archived_at, archived_by, custom_env, custom_args, mcp_config, model, thinking_level +RETURNING id, workspace_id, name, avatar_url, runtime_mode, runtime_config, visibility, status, max_concurrent_tasks, owner_id, created_at, updated_at, description, runtime_id, instructions, archived_at, archived_by, custom_env, custom_args, mcp_config, model, thinking_level, skills_local ` func (q *Queries) RefreshAgentStatusFromTasks(ctx context.Context, id pgtype.UUID) (Agent, error) { @@ -2124,6 +2136,7 @@ func (q *Queries) RefreshAgentStatusFromTasks(ctx context.Context, id pgtype.UUI &i.McpConfig, &i.Model, &i.ThinkingLevel, + &i.SkillsLocal, ) return i, err } @@ -2131,7 +2144,7 @@ func (q *Queries) RefreshAgentStatusFromTasks(ctx context.Context, id pgtype.UUI const restoreAgent = `-- name: RestoreAgent :one UPDATE agent SET archived_at = NULL, archived_by = NULL, updated_at = now() WHERE id = $1 -RETURNING id, workspace_id, name, avatar_url, runtime_mode, runtime_config, visibility, status, max_concurrent_tasks, owner_id, created_at, updated_at, description, runtime_id, instructions, archived_at, archived_by, custom_env, custom_args, mcp_config, model, thinking_level +RETURNING id, workspace_id, name, avatar_url, runtime_mode, runtime_config, visibility, status, max_concurrent_tasks, owner_id, created_at, updated_at, description, runtime_id, instructions, archived_at, archived_by, custom_env, custom_args, mcp_config, model, thinking_level, skills_local ` func (q *Queries) RestoreAgent(ctx context.Context, id pgtype.UUID) (Agent, error) { @@ -2160,6 +2173,7 @@ func (q *Queries) RestoreAgent(ctx context.Context, id pgtype.UUID) (Agent, erro &i.McpConfig, &i.Model, &i.ThinkingLevel, + &i.SkillsLocal, ) return i, err } @@ -2221,9 +2235,10 @@ UPDATE agent SET mcp_config = COALESCE($14, mcp_config), model = COALESCE($15, model), thinking_level = COALESCE($16, thinking_level), + skills_local = COALESCE($17, skills_local), updated_at = now() WHERE id = $1 -RETURNING id, workspace_id, name, avatar_url, runtime_mode, runtime_config, visibility, status, max_concurrent_tasks, owner_id, created_at, updated_at, description, runtime_id, instructions, archived_at, archived_by, custom_env, custom_args, mcp_config, model, thinking_level +RETURNING id, workspace_id, name, avatar_url, runtime_mode, runtime_config, visibility, status, max_concurrent_tasks, owner_id, created_at, updated_at, description, runtime_id, instructions, archived_at, archived_by, custom_env, custom_args, mcp_config, model, thinking_level, skills_local ` type UpdateAgentParams struct { @@ -2243,6 +2258,7 @@ type UpdateAgentParams struct { McpConfig []byte `json:"mcp_config"` Model pgtype.Text `json:"model"` ThinkingLevel pgtype.Text `json:"thinking_level"` + SkillsLocal pgtype.Text `json:"skills_local"` } func (q *Queries) UpdateAgent(ctx context.Context, arg UpdateAgentParams) (Agent, error) { @@ -2263,6 +2279,7 @@ func (q *Queries) UpdateAgent(ctx context.Context, arg UpdateAgentParams) (Agent arg.McpConfig, arg.Model, arg.ThinkingLevel, + arg.SkillsLocal, ) var i Agent err := row.Scan( @@ -2288,6 +2305,7 @@ func (q *Queries) UpdateAgent(ctx context.Context, arg UpdateAgentParams) (Agent &i.McpConfig, &i.Model, &i.ThinkingLevel, + &i.SkillsLocal, ) return i, err } @@ -2342,7 +2360,7 @@ func (q *Queries) UpdateAgentCustomEnv(ctx context.Context, arg UpdateAgentCusto const updateAgentStatus = `-- name: UpdateAgentStatus :one UPDATE agent SET status = $2, updated_at = now() WHERE id = $1 -RETURNING id, workspace_id, name, avatar_url, runtime_mode, runtime_config, visibility, status, max_concurrent_tasks, owner_id, created_at, updated_at, description, runtime_id, instructions, archived_at, archived_by, custom_env, custom_args, mcp_config, model, thinking_level +RETURNING id, workspace_id, name, avatar_url, runtime_mode, runtime_config, visibility, status, max_concurrent_tasks, owner_id, created_at, updated_at, description, runtime_id, instructions, archived_at, archived_by, custom_env, custom_args, mcp_config, model, thinking_level, skills_local ` type UpdateAgentStatusParams struct { @@ -2376,6 +2394,7 @@ func (q *Queries) UpdateAgentStatus(ctx context.Context, arg UpdateAgentStatusPa &i.McpConfig, &i.Model, &i.ThinkingLevel, + &i.SkillsLocal, ) return i, err } diff --git a/server/pkg/db/generated/models.go b/server/pkg/db/generated/models.go index 34a152698..980b9f87b 100644 --- a/server/pkg/db/generated/models.go +++ b/server/pkg/db/generated/models.go @@ -44,6 +44,7 @@ type Agent struct { McpConfig []byte `json:"mcp_config"` Model pgtype.Text `json:"model"` ThinkingLevel pgtype.Text `json:"thinking_level"` + SkillsLocal string `json:"skills_local"` } type AgentRuntime struct { diff --git a/server/pkg/db/queries/agent.sql b/server/pkg/db/queries/agent.sql index 1a8d01406..0335326a2 100644 --- a/server/pkg/db/queries/agent.sql +++ b/server/pkg/db/queries/agent.sql @@ -20,8 +20,9 @@ WHERE id = $1 AND workspace_id = $2; INSERT INTO agent ( workspace_id, name, description, avatar_url, runtime_mode, runtime_config, runtime_id, visibility, max_concurrent_tasks, owner_id, - instructions, custom_env, custom_args, mcp_config, model, thinking_level -) VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11, $12, $13, $14, $15, $16) + instructions, custom_env, custom_args, mcp_config, model, thinking_level, + skills_local +) VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11, $12, $13, $14, $15, $16, $17) RETURNING *; -- name: UpdateAgent :one @@ -41,6 +42,7 @@ UPDATE agent SET mcp_config = COALESCE(sqlc.narg('mcp_config'), mcp_config), model = COALESCE(sqlc.narg('model'), model), thinking_level = COALESCE(sqlc.narg('thinking_level'), thinking_level), + skills_local = COALESCE(sqlc.narg('skills_local'), skills_local), updated_at = now() WHERE id = $1 RETURNING *;