diff --git a/docs/agent-mode-and-tools.md b/docs/agent-mode-and-tools.md index 9c3b4d82..afda1c12 100644 --- a/docs/agent-mode-and-tools.md +++ b/docs/agent-mode-and-tools.md @@ -160,9 +160,11 @@ While the agent is working, the chat shows status indicators for each tool call: This lets you see what the agent is doing as it works. -### Parallel Subagents +### Delegated Agents and Shell Commands -Claude can delegate independent work to background subagents. Agent Mode keeps the turn active while those subagents run, shows the current step, tool count, and elapsed time on each subagent card, then keeps the final report under the card that launched it. Internal result-collection calls stay out of the trail once their report is attached; if Copilot cannot safely match one to its launch, it leaves that result visible instead of risking lost output. Background shell commands and other non-agent tasks do not delay the turn. +For v4, Claude runs delegated agents and Bash commands synchronously. Copilot waits for each supported tool to finish within the current response so the result arrives predictably before the turn ends. This temporarily favors reliable completion over parallel background execution. + +The Workflow tool and remote-isolated agents are temporarily unavailable because they require background execution. Local agents, including worktree-isolated agents, remain available and run synchronously. Background execution will return after Copilot moves Claude sessions to a persistent streaming lifecycle. This requires Claude Code 2.1.206 or newer. Copilot checks the installed version when it starts and whenever you apply or auto-detect a Claude binary. An older binary is marked **Incompatible version** in Settings → Copilot → Agents → Claude and cannot start a session. If you select Claude in chat, Copilot shows the required version and a **Configure Claude** button that opens the same setup dialog; installation guidance stays in that dialog. diff --git a/src/agentMode/sdk/ClaudeSdkBackendProcess.test.ts b/src/agentMode/sdk/ClaudeSdkBackendProcess.test.ts index 663aa178..6181aa61 100644 --- a/src/agentMode/sdk/ClaudeSdkBackendProcess.test.ts +++ b/src/agentMode/sdk/ClaudeSdkBackendProcess.test.ts @@ -1,9 +1,4 @@ -import type { - BackgroundTaskSummary, - HookCallback, - ModelInfo, - SDKMessage, -} from "@anthropic-ai/claude-agent-sdk"; +import type { HookCallback, ModelInfo, SDKMessage } from "@anthropic-ai/claude-agent-sdk"; import type { BackendDescriptor, SessionEvent } from "@/agentMode/session/types"; import { mkdir, mkdtemp, rm, writeFile } from "node:fs/promises"; import os from "node:os"; @@ -61,9 +56,8 @@ jest.mock("./effortOption", () => ({ import { ClaudeSdkBackendProcess, - createStopHook, + enforceForegroundToolUse, promptInputToAnthropicContent, - STOP_BLOCK_REASON, } from "./ClaudeSdkBackendProcess"; import { getCachedSdkCatalog } from "./effortOption"; import { AuthRequiredError } from "@/agentMode/session/errors"; @@ -324,16 +318,12 @@ describe("ClaudeSdkBackendProcess", () => { const call = promptCalls[0][0] as { options: Record }; expect(call.options.pathToClaudeCodeExecutable).toBe("/usr/local/bin/claude"); expect(Object.keys(call.options.mcpServers as object)).not.toContain("obsidian-vault"); - expect(call.options.allowedTools).toEqual([ - "Read", - "Write", - "Edit", - "Glob", - "Grep", - "LS", - "TaskOutput", - ]); - expect(call.options.disallowedTools).toBeUndefined(); + expect(call.options.allowedTools).toEqual(["Read", "Write", "Edit", "Glob", "Grep", "LS"]); + expect(call.options.disallowedTools).toEqual(["TaskOutput", "Workflow", "Monitor"]); + expect(call.options.hooks).toEqual({ + PreToolUse: [{ hooks: [enforceForegroundToolUse] }], + }); + expect((call.options.hooks as Record).Stop).toBeUndefined(); // First turn → sessionId is seeded, no resume. expect(call.options.sessionId).toBe(sessionId); expect(call.options.resume).toBeUndefined(); @@ -1171,95 +1161,68 @@ describe("ClaudeSdkBackendProcess", () => { }); }); - describe("createStopHook()", () => { - const abort = { signal: new AbortController().signal }; - const stopInput = (stopHookActive: boolean, backgroundTasks?: BackgroundTaskSummary[]) => - ({ - hook_event_name: "Stop", - stop_hook_active: stopHookActive, - ...(backgroundTasks ? { background_tasks: backgroundTasks } : {}), - }) as unknown as Parameters[0]; - it("allows the turn to end when no background subagents are running", async () => { - const hook = createStopHook(); - await expect(hook(stopInput(false), undefined, abort)).resolves.toEqual({}); - }); - - it("blocks for a subagent in the SDK background-task snapshot", async () => { - const hook = createStopHook(); - - await expect( - hook( - stopInput(false, [ - { - id: "agent-1", - type: "subagent", - status: "running", - description: "Review the implementation", - }, - ]), - undefined, - abort - ) - ).resolves.toEqual({ decision: "block", reason: STOP_BLOCK_REASON }); - }); - - it("blocks for a workflow in the SDK background-task snapshot", async () => { - const hook = createStopHook(); - - await expect( - hook( - stopInput(false, [ - { - id: "workflow-1", - type: "workflow", - status: "running", - description: "Run the release workflow", - }, - ]), - undefined, - abort - ) - ).resolves.toEqual({ decision: "block", reason: STOP_BLOCK_REASON }); - }); - - it("does not block for background work that cannot be collected with TaskOutput", async () => { - const hook = createStopHook(); - - await expect( - hook( - stopInput(false, [ - { - id: "shell-1", - type: "shell", - status: "running", - description: "npm test", - }, - ]), - undefined, - abort - ) - ).resolves.toEqual({}); - }); - - it("keeps blocking a re-entered Stop hook while a collectible task remains", async () => { - const hook = createStopHook(); - const runningSubagent: BackgroundTaskSummary[] = [ + describe("enforceForegroundToolUse()", () => { + const invoke = (toolName: string, toolInput: unknown) => + enforceForegroundToolUse( { - id: "agent-1", - type: "subagent", - status: "running", - description: "Review the implementation", - }, - ]; + hook_event_name: "PreToolUse", + tool_name: toolName, + tool_input: toolInput, + tool_use_id: "tool-1", + } as Parameters[0], + "tool-1", + { signal: new AbortController().signal } + ); - await expect(hook(stopInput(false, runningSubagent), undefined, abort)).resolves.toEqual({ - decision: "block", - reason: STOP_BLOCK_REASON, + it.each(["Agent", "Task", "Bash"])( + "forces %s to the foreground while preserving its other input", + async (toolName) => { + await expect( + invoke(toolName, { description: "Inspect the vault", run_in_background: true }) + ).resolves.toEqual({ + hookSpecificOutput: { + hookEventName: "PreToolUse", + updatedInput: { + description: "Inspect the vault", + run_in_background: false, + }, + }, + }); + } + ); + + it.each(["Agent", "Task"])("denies a remote-isolated %s", async (toolName) => { + await expect( + invoke(toolName, { description: "Inspect remotely", isolation: "remote" }) + ).resolves.toEqual({ + hookSpecificOutput: { + hookEventName: "PreToolUse", + permissionDecision: "deny", + permissionDecisionReason: expect.stringContaining("temporarily unavailable"), + }, }); - await expect(hook(stopInput(true, runningSubagent), undefined, abort)).resolves.toEqual({ - decision: "block", - reason: STOP_BLOCK_REASON, + }); + + it("keeps worktree-isolated agents synchronous", async () => { + await expect(invoke("Agent", { isolation: "worktree" })).resolves.toEqual({ + hookSpecificOutput: { + hookEventName: "PreToolUse", + updatedInput: { isolation: "worktree", run_in_background: false }, + }, }); }); + + it("normalizes malformed foreground-tool input without throwing", async () => { + await expect(invoke("Bash", null)).resolves.toEqual({ + hookSpecificOutput: { + hookEventName: "PreToolUse", + updatedInput: { run_in_background: false }, + }, + }); + }); + + it("leaves unrelated tools unchanged", async () => { + await expect(invoke("Read", { file_path: "note.md" })).resolves.toEqual({}); + }); }); }); diff --git a/src/agentMode/sdk/ClaudeSdkBackendProcess.ts b/src/agentMode/sdk/ClaudeSdkBackendProcess.ts index 708d533b..289e0e53 100644 --- a/src/agentMode/sdk/ClaudeSdkBackendProcess.ts +++ b/src/agentMode/sdk/ClaudeSdkBackendProcess.ts @@ -20,7 +20,6 @@ import { type Options, type PermissionMode, type Query, - type StopHookInput, type SDKUserMessage, } from "@anthropic-ai/claude-agent-sdk"; import { App } from "obsidian"; @@ -130,31 +129,43 @@ interface SessionState { systemPromptAppend: string; } -/** - * Fed back to the model when it tries to end a turn with background subagents - * still running — steers it to collect their results in-turn instead of - * vanishing with an "I'll report back" message. - */ -export const STOP_BLOCK_REASON = - "Background subagents are still running. Before ending your turn, call the " + - "TaskOutput tool with block=true for each running task to wait for and " + - "collect its result, then fold those results into your reply."; +const FOREGROUND_ONLY_TOOLS = new Set(["Agent", "Task", "Bash"]); +const REMOTE_AGENT_DENIAL_REASON = + "Remote-isolated agents require background execution, which is temporarily unavailable in Copilot v4."; /** - * Prevents a turn from ending before collectible background reports are available, keeping the final answer complete. + * Keeps v4 Claude turns on the query lifecycle that can deliver their complete result. + * Background work becomes safe again once the adapter owns a persistent response consumer. */ -export function createStopHook(): HookCallback { - return async (input) => { - const stopInput = input as StopHookInput; - const hasCollectibleTask = stopInput.background_tasks?.some( - (task) => task.type === "subagent" || task.type === "workflow" - ); - if (!hasCollectibleTask) return {}; - // Re-entry means a previous block kept Claude running, not that the task - // finished. Claude Code bounds this continuation loop at eight blocks. - return { decision: "block", reason: STOP_BLOCK_REASON }; +export const enforceForegroundToolUse: HookCallback = async (input) => { + if (input.hook_event_name !== "PreToolUse" || !FOREGROUND_ONLY_TOOLS.has(input.tool_name)) { + return {}; + } + const toolInput = + typeof input.tool_input === "object" && + input.tool_input !== null && + !Array.isArray(input.tool_input) + ? (input.tool_input as Record) + : {}; + if ( + (input.tool_name === "Agent" || input.tool_name === "Task") && + toolInput.isolation === "remote" + ) { + return { + hookSpecificOutput: { + hookEventName: "PreToolUse", + permissionDecision: "deny", + permissionDecisionReason: REMOTE_AGENT_DENIAL_REASON, + }, + }; + } + return { + hookSpecificOutput: { + hookEventName: "PreToolUse", + updatedInput: { ...toolInput, run_in_background: false }, + }, }; -} +}; export interface ClaudeSdkBackendProcessOptions { pathToClaudeCodeExecutable: string; @@ -415,11 +426,8 @@ export class ClaudeSdkBackendProcess implements BackendProcess { cwd: session.cwd ?? undefined, includePartialMessages: true, mcpServers: session.mcpServers, - // `TaskOutput` is the read-only collection call the Stop hook steers the - // model to make to gather a background subagent's result. Auto-approve it - // so the recovery path never pauses on a permission card and wedges an - // unattended turn (unlisted tools otherwise fall through to `canUseTool`). - allowedTools: ["Read", "Write", "Edit", "Glob", "Grep", "LS", "TaskOutput"], + allowedTools: ["Read", "Write", "Edit", "Glob", "Grep", "LS"], + disallowedTools: ["TaskOutput", "Workflow", "Monitor"], canUseTool: this.bridge.canUseTool, }; // Append the composed Copilot system prompt (captured at newSession time) @@ -470,10 +478,9 @@ export class ClaudeSdkBackendProcess implements BackendProcess { options.env = { ...process.env, ...extraEnv }; } - // Claude Code v2.1.198+ backgrounds subagents by default. The Stop hook - // reads the CLI's background-task snapshot and steers the model to collect - // subagent results in-turn before this adapter stops at `result`. - options.hooks = { Stop: [{ hooks: [createStopHook()] }] }; + // Each prompt currently owns one finite SDK query. Keep work foregrounded + // until a persistent consumer can receive results after that query returns. + options.hooks = { PreToolUse: [{ hooks: [enforceForegroundToolUse] }] }; logSdkOutbound( "prompt", @@ -486,6 +493,7 @@ export class ClaudeSdkBackendProcess implements BackendProcess { effort: options.effort ?? null, mcpServers: Object.keys(options.mcpServers ?? {}), allowedTools: options.allowedTools, + disallowedTools: options.disallowedTools, }, params.sessionId ); diff --git a/src/agentMode/sdk/__fixtures__/claude-notification-only-subagents.ndjson b/src/agentMode/sdk/__fixtures__/claude-notification-only-subagents.ndjson new file mode 100644 index 00000000..537fc061 --- /dev/null +++ b/src/agentMode/sdk/__fixtures__/claude-notification-only-subagents.ndjson @@ -0,0 +1,13 @@ +{"tag":"launch-a","payload":{"type":"stream_event","event":{"type":"content_block_start","index":0,"content_block":{"type":"tool_use","id":"launch-a","name":"Agent","input":{"subagent_type":"Explore","description":"Analyze structure","prompt":"Inspect sanitized structure."}}},"parent_tool_use_id":null,"uuid":"10000000-0000-0000-0000-000000000001","session_id":"replay-session"}} +{"tag":"started-a","payload":{"type":"system","subtype":"task_started","task_id":"task-a","tool_use_id":"launch-a","description":"Analyze structure","uuid":"10000000-0000-0000-0000-000000000002","session_id":"replay-session"}} +{"tag":"ack-a","payload":{"type":"user","tool_use_result":{"isAsync":true,"status":"async_launched","agentId":"task-a","outputFile":"/sanitized/task-a.output"},"message":{"content":[{"type":"tool_result","tool_use_id":"launch-a","content":[{"type":"text","text":"Async agent launched successfully."}]}]},"parent_tool_use_id":null,"session_id":"replay-session"}} +{"tag":"launch-b","payload":{"type":"stream_event","event":{"type":"content_block_start","index":1,"content_block":{"type":"tool_use","id":"launch-b","name":"Agent","input":{"subagent_type":"Explore","description":"Analyze themes","prompt":"Inspect sanitized themes."}}},"parent_tool_use_id":null,"uuid":"10000000-0000-0000-0000-000000000003","session_id":"replay-session"}} +{"tag":"notification-b-before-start","payload":{"type":"system","subtype":"task_notification","task_id":"task-b","status":"completed","summary":"Themes report from the automatic notification.","usage":{"tool_uses":5,"duration_ms":42000,"total_tokens":8000},"uuid":"10000000-0000-0000-0000-000000000004","session_id":"replay-session"}} +{"tag":"started-b-after-notification","payload":{"type":"system","subtype":"task_started","task_id":"task-b","tool_use_id":"launch-b","description":"Analyze themes","uuid":"10000000-0000-0000-0000-000000000005","session_id":"replay-session"}} +{"tag":"ack-b","payload":{"type":"user","tool_use_result":{"isAsync":true,"status":"async_launched","agentId":"task-b","outputFile":"/sanitized/task-b.output"},"message":{"content":[{"type":"tool_result","tool_use_id":"launch-b","content":[{"type":"text","text":"Async agent launched successfully."}]}]},"parent_tool_use_id":null,"session_id":"replay-session"}} +{"tag":"progress-a","payload":{"type":"system","subtype":"task_progress","task_id":"task-a","tool_use_id":"launch-a","description":"Counting notes","last_tool_name":"Glob","usage":{"tool_uses":3,"duration_ms":12000,"total_tokens":4000},"uuid":"10000000-0000-0000-0000-000000000006","session_id":"replay-session"}} +{"tag":"notification-a","payload":{"type":"system","subtype":"task_notification","task_id":"task-a","tool_use_id":"launch-a","status":"completed","summary":"Structure report from the automatic notification.","usage":{"tool_uses":7,"duration_ms":53000,"total_tokens":9900},"uuid":"10000000-0000-0000-0000-000000000007","session_id":"replay-session"}} +{"tag":"late-progress-a","payload":{"type":"system","subtype":"task_progress","task_id":"task-a","tool_use_id":"launch-a","description":"Too late to reopen","last_tool_name":"Read","usage":{"tool_uses":8,"duration_ms":54000,"total_tokens":10000},"uuid":"10000000-0000-0000-0000-000000000008","session_id":"replay-session"}} +{"tag":"snapshot-empty","payload":{"type":"system","subtype":"background_tasks_changed","background_tasks":[],"uuid":"10000000-0000-0000-0000-000000000009","session_id":"replay-session"}} +{"tag":"final-response","payload":{"type":"stream_event","event":{"type":"content_block_delta","index":2,"delta":{"type":"text_delta","text":"Combined final response from both reports."}},"parent_tool_use_id":null,"uuid":"10000000-0000-0000-0000-000000000010","session_id":"replay-session"}} +{"tag":"result","payload":{"type":"result","subtype":"success","duration_ms":54000,"duration_api_ms":50000,"is_error":false,"num_turns":5,"result":"Combined final response from both reports.","stop_reason":"end_turn","total_cost_usd":0,"usage":{},"modelUsage":{},"permission_denials":[],"uuid":"10000000-0000-0000-0000-000000000011","session_id":"replay-session"}} diff --git a/src/agentMode/sdk/sdkMessageTranslator.test.ts b/src/agentMode/sdk/sdkMessageTranslator.test.ts index a6acb2a7..3f8fca6f 100644 --- a/src/agentMode/sdk/sdkMessageTranslator.test.ts +++ b/src/agentMode/sdk/sdkMessageTranslator.test.ts @@ -1,8 +1,27 @@ +import fs from "fs"; +import path from "path"; import type { SDKMessage } from "@anthropic-ai/claude-agent-sdk"; -import type { SessionUsage } from "@/agentMode/session/types"; +import type { SessionEvent, SessionUpdate, SessionUsage } from "@/agentMode/session/types"; import { createTranslatorState, mapStopReason, translateSdkMessage } from "./sdkMessageTranslator"; const SESSION_ID = "session-test-1"; + +interface FixtureFrame { + tag: string; + payload: SDKMessage; +} + +function readFixture(): FixtureFrame[] { + const fixturePath = path.join( + __dirname, + "__fixtures__/claude-notification-only-subagents.ndjson" + ); + return fs + .readFileSync(fixturePath, "utf8") + .trim() + .split("\n") + .map((line) => JSON.parse(line) as FixtureFrame); +} type Uuid = `${string}-${string}-${string}-${string}-${string}`; function streamEvent(event: object): SDKMessage { @@ -724,6 +743,63 @@ describe("translateSdkMessage", () => { ); expect(out).toEqual([]); }); + + it("preserves notification-only lifecycle ordering at the session-event boundary", () => { + const frames = readFixture(); + const state = createTranslatorState(); + const events: SessionEvent[] = []; + const completedLaunches = new Set(); + + expect(JSON.stringify(frames)).not.toContain("TaskOutput"); + for (const frame of frames) { + const translated = translateSdkMessage(frame.payload, "replay-session", state); + events.push(...translated); + for (const { update } of translated) { + if (update.sessionUpdate !== "tool_call_update") continue; + if (!update.toolCallId.startsWith("launch-")) continue; + if (update.status === "completed") completedLaunches.add(update.toolCallId); + if (completedLaunches.has(update.toolCallId)) expect(update.status).not.toBe("in_progress"); + } + } + + const updates = events.map(({ update }) => update); + const launches = updates.filter( + (update): update is Extract => + update.sessionUpdate === "tool_call" && update.vendorToolName === "Agent" + ); + expect(launches.map(({ toolCallId }) => toolCallId)).toEqual(["launch-a", "launch-b"]); + expect(updates).toContainEqual( + expect.objectContaining({ + sessionUpdate: "tool_call_update", + toolCallId: "launch-a", + status: "completed", + content: [ + { + type: "content", + content: { type: "text", text: "Structure report from the automatic notification." }, + }, + ], + }) + ); + expect(updates).toContainEqual( + expect.objectContaining({ + sessionUpdate: "tool_call_update", + toolCallId: "launch-b", + status: "completed", + content: [ + { + type: "content", + content: { type: "text", text: "Themes report from the automatic notification." }, + }, + ], + }) + ); + expect(updates).toContainEqual({ + sessionUpdate: "agent_message_chunk", + content: { type: "text", text: "Combined final response from both reports." }, + }); + }); + describe("background subagent system frames", () => { const UUID = "uuid-1" as `${string}-${string}-${string}-${string}-${string}`;