diff --git a/src/daemon/providers/claude/index.ts b/src/daemon/providers/claude/index.ts index 3f6861f..a1a24da 100644 --- a/src/daemon/providers/claude/index.ts +++ b/src/daemon/providers/claude/index.ts @@ -183,6 +183,14 @@ export class ClaudeProvider implements SessionProvider { // Long-running event queue — closed only when the SDK loop ends. // turn_done events are emitted as regular items; Session decides when to stop. #currentTurnQueue: AsyncQueue | null = null; + /** + * Exact tool names the SDK auto-approves from `allowedTools`, which means it + * never calls canUseTool for them. Since canUseTool is this provider's only + * tool_start emitter, the PreToolUse hook emits on their behalf — consulted + * there to decide which calls need that, and to guarantee we never + * double-emit for a tool that does reach the gate. + */ + #autoApprovedTools = new Set(); /** Id-keyed lifecycle events that arrived with no live queue, replayed into * the next turn. See #handleUndeliverable. */ #carryover: ProviderEvent[] = []; @@ -417,6 +425,20 @@ export class ClaudeProvider implements SessionProvider { const desiredAppend = opts.systemPromptAppend ?? ""; const skillAllowRules = this.#resolveSkillGrants(opts); const desiredGrants = skillAllowRules.join("\n"); + + // Exact tool names we hand the SDK as pre-approved. Kept as its own list + // (rather than inlined into `allowedTools`) because the PreToolUse hook has + // to know precisely which tools will SKIP canUseTool, so it can emit the + // tool_start that canUseTool would otherwise have emitted. + const autoApprovedToolNames = [ + ...(this.#init.memory ? MEMORY_TOOL_NAMES.map((t) => `mcp__codeoid_memory__${t}`) : []), + // Widened for the conductor's fleet server — without these entries the + // mounted server's tools stay unreachable (design §3 gotcha). Note this + // is FLEET_TOOL_NAMES (the READ set) only: the send-class verbs are + // deliberately absent so they still ride the owner's approval flow. + ...(this.#init.fleet ? FLEET_TOOL_NAMES.map((t) => `mcp__codeoid_fleet__${t}`) : []), + ]; + this.#autoApprovedTools = new Set(autoApprovedToolNames); if (this.#consumerTask && this.#inputQueue && !this.#inputQueue.closed) { if ( this.#builtSystemPromptAppend === desiredAppend && @@ -499,14 +521,18 @@ export class ClaudeProvider implements SessionProvider { // buildAgentEnv (GHSA-38vh vector 3). env: buildAgentEnv(), allowedTools: [ - ...(init.memory - ? MEMORY_TOOL_NAMES.map((t) => `mcp__codeoid_memory__${t}`) - : []), - // Widened for the conductor's fleet server — without these entries - // the mounted server's tools stay unreachable (design §3 gotcha). - ...(init.fleet - ? FLEET_TOOL_NAMES.map((t) => `mcp__codeoid_fleet__${t}`) - : []), + // Every EXACT TOOL NAME here is auto-approved by the SDK BEFORE + // canUseTool is consulted, so it never reaches our gate — the SDK + // says so itself via CLAUDE_SDK_CAN_USE_TOOL_SHADOWED. Since + // canUseTool is this provider's only tool_start emitter, each of + // these would otherwise run completely invisibly. They are recorded + // in #autoApprovedTools so the PreToolUse hook can emit their + // tool_start instead. + // + // Bash allow-RULES (`Bash(cmd:*)`) are patterns, not tool names, and + // are deliberately NOT recorded: Bash itself still goes through + // canUseTool, which already emits for it. + ...autoApprovedToolNames, // Verbatim grants for the shell substitutions our installed skills // declare — without these a headless session silently expands the // whole slash command to nothing. See skillCommandAllowRules. @@ -561,6 +587,31 @@ export class ClaudeProvider implements SessionProvider { PreToolUse: [{ hooks: [async (rawInput) => { const input = rawInput as PreToolUseHookInput; + // Stand in for canUseTool on the tools the SDK pre-approved. + // Those never reach the gate, and the gate is the only place this + // provider emits tool_start — so without this every memory recall + // and every conductor fleet read executed with no tool_call + // message at all: absent from the transcript, absent from the UI, + // and absent from the verbatim episode record that is the point + // of capturing them. Confirmed on a live instance: 409 mcp.init + // listings across 18 transcripts, and zero memory tool calls. + // + // Emitted ONLY for names in #autoApprovedTools, which is exactly + // the set that skips canUseTool, so a tool can never be emitted + // twice. tool_use_id is the SDK's own id, matching what + // canUseTool would have used, so tool_complete correlates and + // Session's own auto-approve (these are isSafeTool reads) keeps + // them from prompting. + if (this.#autoApprovedTools.has(input.tool_name)) { + this.#emit({ + type: "tool_start", + toolId: randomUUID(), + sdkToolUseId: input.tool_use_id, + name: input.tool_name, + input: (input.tool_input ?? {}) as Record, + approvalId: randomUUID(), + }); + } init.store.audit( this.#currentSender?.sub ?? "unknown", "session.tool_call", diff --git a/src/daemon/session-manager.ts b/src/daemon/session-manager.ts index bec016f..6a7d037 100644 --- a/src/daemon/session-manager.ts +++ b/src/daemon/session-manager.ts @@ -2081,6 +2081,33 @@ mcpHub: this.#mcpHub, } } + // Kick the orchestrator off on its own goal. + // + // Everything above only BUILDS the collaboration: the goal is compiled into + // the orchestrator's constitution and the role-children are brought up + // deliberately silent (see #spawnCollaborationChildren — a fleet of N costs + // zero tokens). Nothing sent a turn, so before this the whole goal sat idle + // at "no messages, no progress" until the owner happened to type something + // into a session that already knew exactly what it was for. + // + // The goal text is sent as the opening user turn rather than a bare "begin": + // it makes the transcript self-describing (the goal is the first thing you + // read on attach, and on resume) instead of opening with a directive whose + // subject lives only in the constitution. + // + // Fire-and-forget on purpose — create must not block on the first model + // call, and a send failure has to leave a usable (if idle) collaboration + // rather than failing the create that already spawned children. + if (collaboration) { + void session.send(collaboration.goal, auth).catch((err: unknown) => { + console.error( + `[codeoid] collaboration ${session.id.slice(0, 8)} failed to start on its goal (send it a message to begin): ${ + err instanceof Error ? err.message : String(err) + }`, + ); + }); + } + return { type: "response.ok", requestId: msg.id, diff --git a/src/tests/collaboration.test.ts b/src/tests/collaboration.test.ts index 0e48c7f..6e85196 100644 --- a/src/tests/collaboration.test.ts +++ b/src/tests/collaboration.test.ts @@ -2106,3 +2106,60 @@ describe("collaboration.panels", () => { expect(panels.length).toBeGreaterThan(0); }); }); + +describe("collaboration auto-start", () => { + /** + * Creating a collaboration used to BUILD everything and start nothing: the + * goal was compiled into the orchestrator's constitution, the role-children + * came up silent, and then the whole goal sat at "idle" with no transcript + * until the owner typed into a session that already knew what it was for. + * Observed as a collaboration reporting its children in the sidebar while the + * centre pane stayed empty and no work ever happened. + */ + test("starts the orchestrator on its goal instead of leaving it idle", async () => { + const created: MockSessionProvider[] = []; + const registry = new ProviderRegistry("claude"); + for (const id of ["claude", "gemini"] as const) { + registry.register({ + id, + displayName: id, + create: () => { + const p = new MockSessionProvider(id, [textTurn(`${id} ok`)]); + created.push(p); + return p; + }, + }); + } + manager = new SessionManager(store, transcript, undefined, undefined, undefined, { + config: mkConfig(), + providers: registry, + }); + + const resp = await run({ + type: "session.create", + id: "auto-start", + name: "collab-auto", + workdir, + collaboration: VALID, + }); + expect(resp.type).toBe("response.ok"); + + // The orchestrator's provider is built first; the role-children follow. + const orchestrator = created[0]!; + const deadline = Date.now() + 2000; + while (orchestrator.capturedOpts.length === 0) { + if (Date.now() > deadline) throw new Error("orchestrator never took a turn"); + await Bun.sleep(10); + } + + // It opens on the goal itself, so the transcript is self-describing on + // attach and on resume rather than starting with a contentless directive. + expect(orchestrator.capturedOpts[0]!.userMessage).toContain(VALID.goal); + + // The children must STILL be silent — bringing up a fleet of N costs zero + // tokens, and none of them should burn a turn learning to wait. + for (const child of created.slice(1)) { + expect(child.capturedOpts).toHaveLength(0); + } + }); +}); diff --git a/src/tests/dispatch-host.test.ts b/src/tests/dispatch-host.test.ts index 27a8cd5..bcc2880 100644 --- a/src/tests/dispatch-host.test.ts +++ b/src/tests/dispatch-host.test.ts @@ -330,7 +330,20 @@ describe("dispatch host — event routing", () => { { id: "cl", auth: AUTH, send: () => {} }, ); if (resp.type !== "response.ok") throw new Error(`create failed: ${JSON.stringify(resp)}`); - return resp.data as SessionInfo; + const info = resp.data as SessionInfo; + // Creating a collaboration now starts the orchestrator on its goal, so it + // is BUSY the moment create returns. These tests are about dispatch + // routing, not about that opening turn: let it settle so each test starts + // from an idle orchestrator, the precondition they were written against. + // Wait for that turn to have RUN, not merely for the session to look idle: + // the kickoff send is fire-and-forget, so an immediate status read still + // sees "idle" before it has started, and the test would then tick the + // dispatcher into a mid-turn orchestrator and see its event held back. + await until(() => { + const s = manager._sessionForTest(info.id); + return (s?.toInfo().usage?.numTurns ?? 0) > 0 && s?.status === "idle"; + }); + return info; }; const childrenOf = async (parentId: string): Promise => { @@ -389,15 +402,16 @@ describe("dispatch host — event routing", () => { now: Date.now(), }); - expect(turnsOf(goal.id)).toBe(0); + // Baseline, not zero: the orchestrator already took its opening goal turn. + const turnsBefore = turnsOf(goal.id); await manager.dispatcher.tick(); // Delivered TO THE ORCHESTRATOR — proven by it having taken a turn, not // merely by the queue draining (a retired event drains it too). // Delivered TO THE ORCHESTRATOR — proven by a completed turn on that exact // session, not by the queue draining (a retired event drains it too). - await untilTurn(goal.id); - expect(turnsOf(goal.id)).toBeGreaterThan(0); + await until(() => turnsOf(goal.id) > turnsBefore); + expect(turnsOf(goal.id)).toBeGreaterThan(turnsBefore); expect(pending()).toHaveLength(0); }); diff --git a/src/tests/provider-claude.test.ts b/src/tests/provider-claude.test.ts index 119cb7b..9f1f245 100644 --- a/src/tests/provider-claude.test.ts +++ b/src/tests/provider-claude.test.ts @@ -1328,3 +1328,110 @@ describe("ClaudeProvider – VWS wiring (#178 Phase 1)", () => { await provider.teardown(); }); }); + +describe("auto-approved tools still emit tool_start", () => { + /** + * Tools listed by EXACT NAME in `allowedTools` are approved by the SDK before + * canUseTool runs — it reports this itself as CLAUDE_SDK_CAN_USE_TOOL_SHADOWED. + * canUseTool is this provider's only tool_start emitter, so every such call + * used to execute completely invisibly: no tool_call message in the + * transcript, nothing in the UI, nothing in the verbatim episode record. + * + * Observed on a live instance: 409 mcp.init tool listings across 18 + * transcripts and zero memory tool calls, while `Read` appeared 594 times. + */ + const fleetStub = { type: "sdk", name: "codeoid_fleet", instance: {} } as never; + + function providerWithFleet(): ClaudeProvider { + return new ClaudeProvider({ + sessionId: "auto", initialBackingId: "b", workspaceId: "ws", + fleet: fleetStub, + store: { + audit: () => {}, + getClaudeCodeSessionId: () => null, + setClaudeCodeSessionId: () => {}, + getSkillCommandGrants: () => new Map(), + setSkillCommandGrant: () => {}, + } as never, + }); + } + + async function firstPreToolUseHook(): Promise<(i: unknown) => Promise> { + const deadline = Date.now() + 1000; + while (!capturedQueryOpts) { + if (Date.now() > deadline) throw new Error("query() never built"); + await new Promise((r) => setTimeout(r, 5)); + } + const options = (capturedQueryOpts as { options?: Record }).options ?? capturedQueryOpts!; + const hooks = (options as { hooks?: Record Promise> }>> }).hooks; + return hooks!.PreToolUse![0]!.hooks[0]!; + } + + it("emits tool_start for a pre-approved tool, correlated on the SDK's tool_use_id", async () => { + const provider = providerWithFleet(); + capturedQueryOpts = null; + sdkMessages = [{ type: "result", subtype: "success", is_error: false, num_turns: 1, result: "ok", modelUsage: {} }]; + let release!: () => void; + sdkGate = new Promise((r) => { release = r; }); + + const events: ProviderEvent[] = []; + const run = provider.runTurn({ + history: [], userMessage: "hi", workdir: ".", + canUseTool: async () => ({ behavior: "allow" as const }), + }); + const drain = (async () => { for await (const e of run.events) events.push(e); })(); + + const preToolUse = await firstPreToolUseHook(); + await preToolUse({ + hook_event_name: "PreToolUse", + tool_name: "mcp__codeoid_fleet__fleet_list", + tool_input: { scope: "all" }, + tool_use_id: "toolu_abc123", + }); + + release(); + sdkGate = null; + await drain; + + const started = events.filter((e) => e.type === "tool_start"); + expect(started).toHaveLength(1); + expect(started[0]).toMatchObject({ + name: "mcp__codeoid_fleet__fleet_list", + // The SDK's own id, so tool_complete correlates exactly as it would have + // if the call had gone through canUseTool. + sdkToolUseId: "toolu_abc123", + input: { scope: "all" }, + }); + }); + + it("does NOT emit for a tool that still reaches canUseTool (no double tool_start)", async () => { + const provider = providerWithFleet(); + capturedQueryOpts = null; + sdkMessages = [{ type: "result", subtype: "success", is_error: false, num_turns: 1, result: "ok", modelUsage: {} }]; + let release!: () => void; + sdkGate = new Promise((r) => { release = r; }); + + const events: ProviderEvent[] = []; + const run = provider.runTurn({ + history: [], userMessage: "hi", workdir: ".", + canUseTool: async () => ({ behavior: "allow" as const }), + }); + const drain = (async () => { for await (const e of run.events) events.push(e); })(); + + const preToolUse = await firstPreToolUseHook(); + // Read is NOT in allowedTools — it goes through canUseTool, which emits. + // Emitting here too would duplicate every ordinary tool call. + await preToolUse({ + hook_event_name: "PreToolUse", + tool_name: "Read", + tool_input: { file_path: "/tmp/x" }, + tool_use_id: "toolu_read", + }); + + release(); + sdkGate = null; + await drain; + + expect(events.filter((e) => e.type === "tool_start")).toHaveLength(0); + }); +}); diff --git a/web/src/components/NewSessionModal.tsx b/web/src/components/NewSessionModal.tsx index 75e5cf2..b65c915 100644 --- a/web/src/components/NewSessionModal.tsx +++ b/web/src/components/NewSessionModal.tsx @@ -12,6 +12,7 @@ import { Component, For, + Index, Show, createEffect, createMemo, @@ -520,24 +521,24 @@ const NewSessionModal: Component = () => { Roles - + {(r, i) => { - const isOrchestrator = () => r.name.trim().toLowerCase() === "orchestrator"; + const isOrchestrator = () => r().name.trim().toLowerCase() === "orchestrator"; return (
updateRole(i(), { name: e.currentTarget.value })} + value={r().name} + onInput={(e) => updateRole(i, { name: e.currentTarget.value })} class="min-w-0 flex-1 rounded border border-border bg-bg-elev px-2 py-1 font-mono text-[12px] text-fg outline-none focus:border-accent" disabled={busy() || isOrchestrator()} aria-label="Role name" />
); }} - +