From 82ab74d1e85c62a322b0c0ac23eb94c6981c0c19 Mon Sep 17 00:00:00 2001 From: Yash Datta Date: Sun, 6 Sep 2026 15:46:16 +0800 Subject: [PATCH] fix(conductor): let the fleet read surface run without prompting MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit conductor-design §3 specifies that the fleet READ verbs — fleet_list, fleet_find, fleet_summary, fleet_recall, fleet_tasks, machine_map — "run silently". They did not. `isSafeTool` knew the memory and blackboard mounts but not the fleet, so in guarded mode every one of them raised an approval prompt. Found by driving a real conductor rather than reading the code: asking it to resolve a session reference produced an approval request for `fleet_find`, which is a read. An assistant that asks permission to look something up is not an assistant, and this is the first friction anyone meets in the new conductor pane (#325). The daemon already put these verbs in the provider's `allowedTools`, so the SDK warned it would auto-approve them — but codeoid's own gate is consulted independently and did not recognise them, which is why the two disagreed. Follows the established shape exactly: match the namespace prefix, then require the suffix to be a known read verb. Never a bare prefix match — an over-broad match here is a prompt bypass, which is why a look-alike segment (`mcp__evil_codeoid_fleet__…`) is asserted to gain nothing. The read list is `FLEET_READ_TOOLS` from the shared protocol package, so the send half cannot leak in by someone editing one of two copies. Send verbs remain hard-gated BEFORE this function is consulted (`isFleetSendTool` in Session#shouldAutoApprove, checked ahead of any mode logic); this is defence in depth, not the fence. Verified live, both directions. With the fix, fleet_list / fleet_find / machine_map go straight to `executing` with zero prompts; fleet_spawn still lands in `waiting_confirmation` and waits for the owner. The R3 invariant is intact. Also lists the bare (non-`mcp__`) namespacing so a mounted fleet (#245) does not silently regress to prompting on every read. 10 tool-safety tests; 2452 daemon tests; typecheck and lint clean. Co-Authored-By: Claude Opus 5 (1M context) --- src/daemon/providers/tool-safety.ts | 24 ++++++++++++++++++ src/tests/tool-safety.test.ts | 39 +++++++++++++++++++++++++++++ 2 files changed, 63 insertions(+) diff --git a/src/daemon/providers/tool-safety.ts b/src/daemon/providers/tool-safety.ts index cbe57b5..54f15e1 100644 --- a/src/daemon/providers/tool-safety.ts +++ b/src/daemon/providers/tool-safety.ts @@ -8,6 +8,7 @@ import { BLACKBOARD_MCP_SERVER_NAME } from "../blackboard/mcp-http.js"; import { MEMORY_MCP_SERVER_NAME } from "../memory/mcp-http.js"; import { MEMORY_TOOL_NAMES } from "../memory/tools.js"; +import { FLEET_READ_TOOLS, FLEET_TOOL_PREFIX } from "../../protocol/types.js"; /** Built-in read-only tools that never require confirmation. */ const SAFE_TOOLS = new Set(["Read", "Grep", "Glob"]); @@ -24,6 +25,18 @@ const BLACKBOARD_TOOL_PREFIXES = [ `${BLACKBOARD_MCP_SERVER_NAME}__`, ] as const; +/** + * Same two namespacing conventions, for the conductor's fleet mount. + * + * The bare form is unused today — the fleet server is a Claude in-process MCP + * object — but is listed so a mounted fleet (#245) does not silently regress to + * prompting on every read. + */ +const FLEET_TOOL_PREFIXES = [ + FLEET_TOOL_PREFIX, // `mcp__codeoid_fleet__` — Claude in-process MCP + FLEET_TOOL_PREFIX.replace(/^mcp__/, ""), // bare mount +] as const; + /** * Blackboard tools that may run unprompted. * @@ -60,6 +73,17 @@ export function isSafeTool(name: string): boolean { return (BLACKBOARD_SAFE_TOOLS as readonly string[]).includes(name.slice(prefix.length)); } } + // Fleet READS only. The send-class verbs are absent by construction — + // `FLEET_READ_TOOLS` is the read half of the shared vocabulary, so a verb + // added to the send half can never leak in here by editing one list. They are + // additionally hard-gated before this function is ever consulted + // (`isFleetSendTool` in Session#shouldAutoApprove), which is the invariant; + // this is defence in depth, not the fence. + for (const prefix of FLEET_TOOL_PREFIXES) { + if (name.startsWith(prefix)) { + return (FLEET_READ_TOOLS as readonly string[]).includes(name.slice(prefix.length)); + } + } return false; } diff --git a/src/tests/tool-safety.test.ts b/src/tests/tool-safety.test.ts index 4bb8746..0a8f46b 100644 --- a/src/tests/tool-safety.test.ts +++ b/src/tests/tool-safety.test.ts @@ -1,4 +1,5 @@ import { describe, test, expect } from "bun:test"; +import { FLEET_READ_TOOLS, FLEET_SEND_TOOLS } from "../protocol/types.js"; import { isElicitationTool, isSafeTool } from "../daemon/providers/tool-safety.js"; import { MEMORY_TOOL_NAMES } from "../daemon/memory/tools.js"; @@ -50,3 +51,41 @@ describe("isElicitationTool", () => { } }); }); + +describe("isSafeTool — the conductor's fleet mount", () => { + // The fleet READ surface is specified to run silently (conductor-design §3): + // "fleet_list / fleet_find / fleet_summary / fleet_recall / fleet_tasks / + // machine_map run silently". They did not — `isSafeTool` knew the memory and + // blackboard mounts but not the fleet, so in guarded mode every `fleet_find` + // raised an approval prompt. An assistant that asks permission to look + // something up is not an assistant. + + test("every read verb runs unprompted, under both namespacings", () => { + for (const verb of FLEET_READ_TOOLS) { + expect(isSafeTool(`mcp__codeoid_fleet__${verb}`)).toBe(true); + expect(isSafeTool(`codeoid_fleet__${verb}`)).toBe(true); + } + }); + + test("NO send-class verb is ever safe", () => { + // The one that must never regress. These are hard-gated earlier too + // (isFleetSendTool, before any mode logic), so this is the second fence. + for (const verb of FLEET_SEND_TOOLS) { + expect(isSafeTool(`mcp__codeoid_fleet__${verb}`)).toBe(false); + expect(isSafeTool(`codeoid_fleet__${verb}`)).toBe(false); + } + }); + + test("an unknown verb on the fleet prefix prompts rather than auto-approving", () => { + expect(isSafeTool("mcp__codeoid_fleet__fleet_detonate")).toBe(false); + expect(isSafeTool("mcp__codeoid_fleet__")).toBe(false); + }); + + test("a look-alike server segment does not inherit fleet safety", () => { + // Matching on the server segment alone would let these through; the prefix + // must match exactly, then the suffix must be a known read verb. + expect(isSafeTool("mcp__evil_codeoid_fleet__fleet_find")).toBe(false); + expect(isSafeTool("x_codeoid_fleet__fleet_find")).toBe(false); + expect(isSafeTool("mcp__codeoid_fleet_x__fleet_find")).toBe(false); + }); +});