From 5c17885b1ca2149c7680a36766482eb97676a085 Mon Sep 17 00:00:00 2001 From: Yash Datta Date: Sat, 5 Sep 2026 23:50:10 +0800 Subject: [PATCH 1/2] feat(web): render conductor fleet calls as action cards MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Second slice of P5.1 (docs/conductor-frontends-design.md §5, §12). The conductor drives the whole fleet through `mcp__codeoid_fleet__*` tools, and they rendered like any other tool: a long prefixed name and a collapsed blob of raw JSON. That is the right default for an arbitrary tool and the wrong one here, because these few verbs ARE the conductor's vocabulary — "which session did it pick, what did it send, where did it spawn" is the thing you open the transcript to find out. Splits the work so the part worth testing needs no reactive root, the same shape `lib/fleet.ts` uses for grouping: `lib/fleet-cards.ts` holds classification and field extraction as pure functions, and MessageRow renders the result. Ordinary tools are untouched — the classifier returns null and the existing path runs unchanged. Three decisions carry most of the value. **Unknown verbs fail safe.** The read/send split is security-relevant and enforced daemon-side (`FLEET_SEND_TOOL_NAMES`); the web cannot import daemon code, so the vocabulary is duplicated and can drift. An unrecognised verb is therefore classified `unknown`, never `observe`, so a send-class verb added daemon-side can never render here as a harmless read. Pinned by a test. **Input is model-generated and typed `unknown`.** Every field is narrowed rather than cast, and a wrong-typed or hallucinated field is omitted rather than rendered — an invalid `shape` produces no shape claim at all. Asserted against missing/null/string/number/array inputs. **The input is read off the STATE while awaiting approval.** A call at `waiting_confirmation` carries its complete input on `state.input`, not `tool.input` — and that is exactly the card that has to be readable, since it is the approval prompt where the owner decides whether a dispatch runs. Reading only `tool.input` would have blanked it. A `streaming` phase is deliberately NOT consulted: `partialInput` is a half-generated fragment, and a card built from it would show a workdir the model has not finished writing. Field names were taken from the daemon's own zod schemas rather than guessed — `fleet_panel` uses `sessions`/`message`, not `targets`/`prompt`, which the first draft had wrong. Near-miss aliases are still tolerated for the target of single-target verbs, because a blank target on an approval prompt is worse than a tolerated alias. The card's left border carries the read/act distinction, and send-class calls get an `act` badge. `unknown` deliberately does not borrow a colour it has not earned. 17 new tests (180 web tests total), typecheck, lint and build clean; the two remaining MessageRow lint warnings are pre-existing on main. Co-Authored-By: Claude Opus 5 (1M context) --- web/src/components/transcript/MessageRow.tsx | 77 +++++ web/src/lib/fleet-cards.test.ts | 193 ++++++++++++ web/src/lib/fleet-cards.ts | 297 +++++++++++++++++++ 3 files changed, 567 insertions(+) create mode 100644 web/src/lib/fleet-cards.test.ts create mode 100644 web/src/lib/fleet-cards.ts diff --git a/web/src/components/transcript/MessageRow.tsx b/web/src/components/transcript/MessageRow.tsx index a96ce73..c89dd61 100644 --- a/web/src/components/transcript/MessageRow.tsx +++ b/web/src/components/transcript/MessageRow.tsx @@ -14,6 +14,7 @@ import { identityLabel, shortSub, } from "../../lib/identity"; +import { classifyFleetTool, type FleetCard } from "../../lib/fleet-cards"; import { safeImageUri, safeLinkUri } from "../../lib/sanitize-url"; import { createFrameThrottled, @@ -269,7 +270,12 @@ const ToolBlock: Component<{ msg: SessionMessage }> = (props) => { const candidate = t0.input ?? fromState; return isWriteInput(candidate) ? candidate : null; }; + // A fleet call is the conductor's whole vocabulary, so it gets a card that + // says what it DOES instead of a raw-JSON `
` (conductor-frontends + // §5). Everything else keeps the generic rendering unchanged. + const fleet = () => classifyFleetTool(t()); return ( +
{t().name} @@ -302,6 +308,77 @@ const ToolBlock: Component<{ msg: SessionMessage }> = (props) => {
+ }> + {(card) => } + + ); +}; + +/** + * A conductor fleet call, rendered as what it does. + * + * The left border carries the read/act distinction, because that is the thing + * you scan a conductor transcript for: `dispatch` is accented (it changed + * something, and the owner approved it), `resolve` is warn-toned because a + * wrong resolution is what silently misroutes a later dispatch, and plain + * observes recede. `unknown` deliberately looks like nothing familiar rather + * than borrowing a colour it has not earned — see `classifyFleetTool`. + */ +const FleetActionCard: Component<{ + card: FleetCard; + state: ToolState; + toolId: string; +}> = (props) => { + const accent = () => { + switch (props.card.kind) { + case "dispatch": + return "border-l-accent"; + case "resolve": + return "border-l-warn/60"; + case "unknown": + return "border-l-danger/50"; + default: + return "border-l-role-tool/40"; + } + }; + return ( +
+
+ {props.card.summary} + + + act + + + + + {props.card.verb} · {shortSub(props.toolId)} + +
+ 0}> +
+ + {(f) => ( +
+
{f().label}
+
+ {f().value} +
+
+ )} + +
+ +
); }; diff --git a/web/src/lib/fleet-cards.test.ts b/web/src/lib/fleet-cards.test.ts new file mode 100644 index 0000000..1f61316 --- /dev/null +++ b/web/src/lib/fleet-cards.test.ts @@ -0,0 +1,193 @@ +import { describe, it, expect } from "vitest"; + +import { classifyFleetTool, fleetVerb } from "./fleet-cards"; +import type { ToolInfo } from "../protocol/types"; + +function tool(name: string, input?: unknown): ToolInfo { + return { + toolId: "t1", + name, + state: { phase: "completed", output: "", success: true }, + ...(input === undefined ? {} : { input }), + } as ToolInfo; +} + +const fleet = (verb: string, input?: unknown) => + tool(`mcp__codeoid_fleet__${verb}`, input); + +const valueOf = (card: ReturnType, label: string) => + card?.fields.find((f) => f.label === label)?.value; + +describe("fleetVerb", () => { + it("strips the MCP server prefix and ignores ordinary tools", () => { + expect(fleetVerb("mcp__codeoid_fleet__fleet_spawn")).toBe("fleet_spawn"); + expect(fleetVerb("Bash")).toBeNull(); + // A different MCP server must not be mistaken for the fleet. + expect(fleetVerb("mcp__codeoid_memory__recall")).toBeNull(); + }); +}); + +describe("classifyFleetTool", () => { + it("returns null for a non-fleet tool so it keeps its normal rendering", () => { + expect(classifyFleetTool(tool("Bash", { command: "ls" }))).toBeNull(); + }); + + it("reads fleet_find as a resolve, quoting the query", () => { + const card = classifyFleetTool(fleet("fleet_find", { query: "the authz fix" }))!; + expect(card.kind).toBe("resolve"); + expect(card.sendClass).toBe(false); + expect(card.summary).toContain("the authz fix"); + expect(valueOf(card, "query")).toBe("the authz fix"); + }); + + it("reads fleet_spawn as a dispatch with shape, workdir basename and backend", () => { + const card = classifyFleetTool( + fleet("fleet_spawn", { + shape: "scout", + workdir: "/home/me/Workspace/codeoid", + task: "Read README.md and report", + provider: "claude", + model: "opus", + }), + )!; + expect(card.kind).toBe("dispatch"); + expect(card.sendClass).toBe(true); + // The header stays short; the full path is still a field. + expect(card.summary).toBe("Spawn scout in codeoid"); + expect(valueOf(card, "workdir")).toBe("/home/me/Workspace/codeoid"); + expect(valueOf(card, "backend")).toBe("claude · opus"); + expect(card.fields.find((f) => f.label === "task")?.block).toBe(true); + }); + + it("keeps a partial backend rather than dropping it", () => { + const only = classifyFleetTool(fleet("fleet_spawn", { provider: "qwen" }))!; + expect(valueOf(only, "backend")).toBe("qwen"); + const none = classifyFleetTool(fleet("fleet_spawn", {}))!; + expect(valueOf(none, "backend")).toBeUndefined(); + }); + + it("reads fleet_send with the schema's own field names", () => { + // Matches the zod schema in src/daemon/fleet.ts: session / message / shape. + const send = classifyFleetTool( + fleet("fleet_send", { session: "studio-870", message: "run the linter", shape: "scout" }), + )!; + expect(send.kind).toBe("dispatch"); + expect(send.summary).toBe("Send to studio-870"); + expect(valueOf(send, "shape")).toBe("scout"); + expect(send.fields.find((f) => f.label === "message")?.block).toBe(true); + + expect(classifyFleetTool(fleet("fleet_interrupt", { session: "y" }))!.summary).toBe( + "Interrupt y", + ); + }); + + it("still shows a target when the model proposes a near-miss field name", () => { + // A card can render input the model PROPOSED, which reaches the approval + // gate before the tool schema validates it. A blank target on an approval + // prompt is worse than a tolerated alias. + expect(classifyFleetTool(fleet("fleet_send", { name: "x" }))!.summary).toBe("Send to x"); + expect(classifyFleetTool(fleet("fleet_interrupt", { target: "y" }))!.summary).toBe( + "Interrupt y", + ); + }); + + it("counts panel sessions and pluralises honestly", () => { + const one = classifyFleetTool(fleet("fleet_panel", { sessions: ["a"] }))!; + expect(one.summary).toBe("Panel — 1 session"); + const many = classifyFleetTool( + fleet("fleet_panel", { sessions: ["a", "b"], shape: "ship", message: "review this" }), + )!; + expect(many.summary).toBe("Panel — 2 sessions"); + expect(valueOf(many, "sessions")).toBe("a, b"); + expect(valueOf(many, "shape")).toBe("ship"); + expect(valueOf(many, "message")).toBe("review this"); + }); + + it("labels the remaining read verbs without inventing structure", () => { + expect(classifyFleetTool(fleet("machine_map"))!.kind).toBe("observe"); + expect(classifyFleetTool(fleet("fleet_tasks", { limit: 5 }))!.summary).toBe( + "Checking the task board", + ); + // `limit` is not a field we claim to render; it is simply omitted. + expect(classifyFleetTool(fleet("fleet_tasks", { limit: 5 }))!.fields).toEqual([]); + }); + + it("reads the input off the STATE while awaiting approval", () => { + // The approval prompt is the card that most has to be readable, and at + // waiting_confirmation the complete input lives on state, not tool.input. + const awaiting = { + toolId: "t1", + name: "mcp__codeoid_fleet__fleet_spawn", + state: { + phase: "waiting_confirmation", + input: { shape: "ship", workdir: "/repo/api", task: "bump the dep" }, + }, + } as unknown as ToolInfo; + const card = classifyFleetTool(awaiting)!; + expect(card.summary).toBe("Spawn ship in api"); + expect(valueOf(card, "task")).toBe("bump the dep"); + }); + + it("ignores a half-generated streaming input", () => { + // partialInput is a fragment; a card built from it would show a workdir the + // model has not finished writing. + const streaming = { + toolId: "t1", + name: "mcp__codeoid_fleet__fleet_spawn", + state: { phase: "streaming", partialInput: { workdir: "/repo/ap" } }, + } as unknown as ToolInfo; + const card = classifyFleetTool(streaming)!; + expect(card.summary).toBe("Spawn worker"); + expect(card.fields).toEqual([]); + }); + + describe("hostile and malformed input", () => { + it("never reports an unrecognised verb as a safe read", () => { + // The read/send split is enforced daemon-side; this module duplicates the + // vocabulary and can drift. A future send-class verb must not render as + // an innocuous observe card just because this list is stale. + const card = classifyFleetTool(fleet("fleet_detonate", { yes: true }))!; + expect(card.kind).toBe("unknown"); + expect(card.sendClass).toBe(false); + expect(card.fields).toEqual([]); + expect(card.summary).toContain("unrecognised"); + }); + + it("survives input that is missing, null, or the wrong type", () => { + for (const bad of [undefined, null, "a string", 42, ["an", "array"]]) { + const card = classifyFleetTool(fleet("fleet_spawn", bad))!; + expect(card.kind).toBe("dispatch"); + expect(card.summary).toBe("Spawn worker"); + expect(card.fields).toEqual([]); + } + }); + + it("ignores wrong-typed fields instead of rendering them", () => { + const card = classifyFleetTool( + fleet("fleet_spawn", { shape: "explode", workdir: 42, task: { nested: true } }), + )!; + expect(card.summary).toBe("Spawn worker"); // invalid shape → no claim + expect(card.fields).toEqual([]); + }); + + it("treats a whitespace-only string as absent", () => { + const card = classifyFleetTool(fleet("fleet_find", { query: " " }))!; + expect(card.summary).toBe("Resolving a session reference"); + expect(card.fields).toEqual([]); + }); + + it("drops non-string entries from a sessions array rather than the whole array", () => { + const card = classifyFleetTool(fleet("fleet_panel", { sessions: ["a", 7, null, "b"] }))!; + expect(valueOf(card, "sessions")).toBe("a, b"); + }); + + it("keeps a workdir that is only separators legible in the header", () => { + expect(classifyFleetTool(fleet("fleet_spawn", { workdir: "/" }))!.summary).toBe( + "Spawn worker in /", + ); + expect(classifyFleetTool(fleet("fleet_spawn", { workdir: "/a/b/" }))!.summary).toBe( + "Spawn worker in b", + ); + }); + }); +}); diff --git a/web/src/lib/fleet-cards.ts b/web/src/lib/fleet-cards.ts new file mode 100644 index 0000000..d1345d0 --- /dev/null +++ b/web/src/lib/fleet-cards.ts @@ -0,0 +1,297 @@ +/** + * Fleet tool calls → a typed card model. + * + * The conductor drives the fleet through `mcp__codeoid_fleet__*` tools, and + * today they render like any other tool: a name and a `
` blob of raw + * JSON. That is the correct default for an arbitrary tool and the wrong one + * here, because these few verbs ARE the conductor's whole vocabulary — "which + * session did it pick, what did it send, where did it spawn" is the thing you + * are reading the transcript to find out (conductor-frontends-design §5). + * + * This module is the pure half: classification and field extraction, with no + * Solid and no JSX, so the part worth testing needs no reactive root — the same + * split `lib/fleet.ts` uses for grouping. + * + * Two rules shape everything below. + * + * **`input` is model-generated and typed `unknown`.** Every field is narrowed + * rather than cast; a malformed or hallucinated input degrades to a card with + * missing fields, never a crash and never a confident lie. + * + * **Unknown verbs fail safe.** The read/send split is a SECURITY-relevant + * classification that the daemon enforces (`FLEET_SEND_TOOL_NAMES` in + * `src/daemon/fleet.ts` — send-class verbs can never be auto-approved). This + * module cannot import daemon code, so the vocabulary is duplicated below and + * can drift. A verb this module does not recognise is therefore classified + * `"unknown"` — never `"observe"` — so a future send-class verb added daemon- + * side can never be rendered as a harmless read here. + */ + +import type { ToolInfo } from "../protocol/types"; + +/** The daemon's in-process fleet MCP server key. */ +const FLEET_TOOL_PREFIX = "mcp__codeoid_fleet__"; + +/** + * Read-class verbs — observe only, auto-approved daemon-side. + * Mirrors `FLEET_TOOL_NAMES`; see the fail-safe note in the module header. + */ +const READ_VERBS = [ + "fleet_list", + "fleet_find", + "fleet_summary", + "fleet_recall", + "fleet_tasks", + "machine_map", +] as const; + +/** + * Send-class verbs — act on the fleet, and never auto-approved daemon-side. + * Mirrors `FLEET_SEND_TOOL_NAMES`. + */ +const SEND_VERBS = [ + "fleet_send", + "fleet_spawn", + "fleet_interrupt", + "fleet_panel", +] as const; + +export type FleetVerb = (typeof READ_VERBS)[number] | (typeof SEND_VERBS)[number]; + +const READ_SET: ReadonlySet = new Set(READ_VERBS); +const SEND_SET: ReadonlySet = new Set(SEND_VERBS); + +/** + * What a fleet call is *for*, which is what decides how loud its card should be. + * + * `resolve` is split out of `observe` because it is the one read the owner must + * actually check: a wrong resolution silently routes a later dispatch at the + * wrong repo, which §6 of the design calls the failure that would kill trust in + * the feature. + */ +export type FleetCardKind = "resolve" | "observe" | "dispatch" | "unknown"; + +export interface FleetCard { + kind: FleetCardKind; + /** Bare verb (`fleet_spawn`), with the MCP server prefix stripped. */ + verb: string; + /** + * True only for verbs known to be send-class. An unknown verb is NOT + * reported as safe — see the module header. + */ + sendClass: boolean; + /** One-line summary for the card header. Never raw JSON. */ + summary: string; + /** Ordered detail rows the card renders. Absent fields are omitted, not blanked. */ + fields: FleetCardField[]; +} + +export interface FleetCardField { + label: string; + value: string; + /** + * Long free text (a task brief, a message body) that a card should render in + * a block rather than inline on one row. + */ + block?: boolean; +} + +/** Strip the MCP prefix, or return null when this is not a fleet tool at all. */ +export function fleetVerb(toolName: string): string | null { + return toolName.startsWith(FLEET_TOOL_PREFIX) + ? toolName.slice(FLEET_TOOL_PREFIX.length) + : null; +} + +/** + * Build the card model for a fleet tool call, or null when `tool` is an + * ordinary tool that should keep its existing rendering. + */ +export function classifyFleetTool(tool: ToolInfo): FleetCard | null { + const verb = fleetVerb(tool.name); + if (verb === null) return null; + + const resolved = resolveToolInput(tool); + const input = isRecord(resolved) ? resolved : {}; + const sendClass = SEND_SET.has(verb); + const known = sendClass || READ_SET.has(verb); + + if (!known) { + // Fail safe: name it, show nothing we cannot vouch for, and do not imply + // a read/write posture we have no basis for. + return { + kind: "unknown", + verb, + sendClass: false, + summary: `${verb} — unrecognised fleet verb`, + fields: [], + }; + } + + switch (verb) { + case "fleet_find": { + const query = str(input.query); + return { + kind: "resolve", + verb, + sendClass, + summary: query ? `Resolving “${query}”` : "Resolving a session reference", + fields: field("query", query), + }; + } + case "fleet_spawn": { + const shape = shapeOf(input.shape); + const workdir = str(input.workdir); + return { + kind: "dispatch", + verb, + sendClass, + summary: `Spawn ${shape ?? "worker"}${workdir ? ` in ${basename(workdir)}` : ""}`, + fields: [ + ...field("shape", shape), + ...field("workdir", workdir), + ...field("backend", joinBackend(str(input.provider), str(input.model))), + ...field("task", str(input.task), true), + ], + }; + } + case "fleet_send": { + const target = sessionRef(input); + return { + kind: "dispatch", + verb, + sendClass, + summary: target ? `Send to ${target}` : "Send to a session", + fields: [ + ...field("target", target), + ...field("shape", shapeOf(input.shape)), + ...field("message", str(input.message), true), + ], + }; + } + case "fleet_interrupt": { + const target = sessionRef(input); + return { + kind: "dispatch", + verb, + sendClass, + summary: target ? `Interrupt ${target}` : "Interrupt a session", + fields: field("target", target), + }; + } + case "fleet_panel": { + const sessions = strArray(input.sessions); + return { + kind: "dispatch", + verb, + sendClass, + summary: + sessions.length > 0 + ? `Panel — ${sessions.length} session${sessions.length === 1 ? "" : "s"}` + : "Panel dispatch", + fields: [ + ...field("shape", shapeOf(input.shape)), + ...field("sessions", sessions.length > 0 ? sessions.join(", ") : null), + ...field("message", str(input.message), true), + ], + }; + } + default: { + // The remaining read verbs carry little or no input; a bare, honest + // header beats inventing structure for them. + return { + kind: "observe", + verb, + sendClass, + summary: OBSERVE_SUMMARY[verb] ?? verb, + fields: [...field("query", str(input.query)), ...field("session", sessionRef(input))], + }; + } + } +} + +const OBSERVE_SUMMARY: Record = { + fleet_list: "Listing the fleet", + fleet_summary: "Reading a session digest", + fleet_recall: "Recalling past context", + fleet_tasks: "Checking the task board", + machine_map: "Mapping the machine", +}; + +// ── narrowing helpers ──────────────────────────────────────────────────────── +// Everything below exists because `ToolInfo.input` is `unknown` and produced by +// a model: nothing here may assume a shape it has not checked. + +/** + * The tool's input, wherever this phase keeps it. + * + * `ToolInfo.input` is populated for most phases, but a call sitting at + * `waiting_confirmation` carries its complete input on the STATE instead. That + * is precisely the phase these cards matter most in — it is the approval + * prompt, where the owner decides whether to let a dispatch run — so reading + * only `tool.input` would blank exactly the card that has to be readable. + * `streaming` is deliberately not consulted: its `partialInput` is a + * half-generated fragment, and a card built from it would show a target or + * workdir that the model has not finished writing. + */ +function resolveToolInput(tool: ToolInfo): unknown { + if (tool.input !== undefined) return tool.input; + return tool.state.phase === "waiting_confirmation" ? tool.state.input : undefined; +} + +function isRecord(v: unknown): v is Record { + return typeof v === "object" && v !== null && !Array.isArray(v); +} + +/** A non-empty string, or null. Whitespace-only is treated as absent. */ +function str(v: unknown): string | null { + if (typeof v !== "string") return null; + const t = v.trim(); + return t.length > 0 ? t : null; +} + +function strArray(v: unknown): string[] { + if (!Array.isArray(v)) return []; + return v.map(str).filter((s): s is string => s !== null); +} + +function shapeOf(v: unknown): "ship" | "scout" | null { + return v === "ship" || v === "scout" ? v : null; +} + +/** + * The target session of a single-target verb. + * + * `session` is the field every such tool actually declares (`fleet_send`, + * `fleet_summary`, `fleet_interrupt` — see their zod schemas in + * `src/daemon/fleet.ts`). The aliases are tolerance, not guesswork: a card can + * render an input the model PROPOSED, which reaches the approval gate before + * the tool's schema has validated it — so a near-miss field name should still + * show the owner what is about to be dispatched rather than a blank target. + */ +function sessionRef(input: Record): string | null { + return str(input.session) ?? str(input.name) ?? str(input.target); +} + +/** `claude · opus` — either half may be absent. */ +function joinBackend(provider: string | null, model: string | null): string | null { + if (provider && model) return `${provider} · ${model}`; + return provider ?? model; +} + +/** + * Last path segment, for a compact header. Trailing separators are ignored so + * `/a/b/` reads as `b`, and a path that is only separators falls back to the + * original string rather than an empty label. + */ +function basename(path: string): string { + const trimmed = path.replace(/[/\\]+$/, ""); + const idx = Math.max(trimmed.lastIndexOf("/"), trimmed.lastIndexOf("\\")); + const tail = idx >= 0 ? trimmed.slice(idx + 1) : trimmed; + return tail.length > 0 ? tail : path; +} + +/** Zero or one field — absent values are omitted rather than rendered blank. */ +function field(label: string, value: string | null, block = false): FleetCardField[] { + return value === null ? [] : [{ label, value, ...(block ? { block: true } : {}) }]; +} From cdbf59ca265d02bf8b5694a63977b74c922408c9 Mon Sep 17 00:00:00 2001 From: Yash Datta Date: Sun, 6 Sep 2026 00:25:14 +0800 Subject: [PATCH 2/2] refactor: one source of truth for the fleet verb vocabulary MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review follow-up on #323 (Oracle suggestions 1-3). **The verb lists move to @highflame/codeoid-protocol.** Oracle asked for a CI assertion that the web's duplicated copy still mirrors the daemon's `FLEET_SEND_TOOL_NAMES`, since the fail-safe only CONTAINS drift — a send-class verb added daemon-side would render as `unknown` here indefinitely and nobody would notice. A mirror test turned out to be the weaker of the two options Oracle offered. Tests can import daemon code, but doing so drags `bun:sqlite` and the `Bun` global into web's `tsc -b`, which fails the web-check job. More importantly, a tripwire only DETECTS drift. Both sides now import `FLEET_READ_TOOLS` / `FLEET_SEND_TOOLS` / `FLEET_TOOL_PREFIX` from the shared protocol package, so there is no second copy to fall out of date. It sits beside `CAPABILITIES`, which is already a shared runtime constant there for the same reason. The daemon keeps its existing export names as aliases, so its providers and tests are untouched. The fail-safe stays: a verb NEITHER list names — an older client meeting a newer daemon — still classifies as `unknown` rather than `observe`. **`resolveToolInput` now tests `!= null`, not `!== undefined`.** A real bug. `input` is typed `unknown`, so `null` is representable, and a JSON round-trip preserves an explicit `null` while turning a missing key into `undefined`. The old check accepted that `null`, `isRecord` then rejected it, and the card rendered fieldless — precisely the approval-prompt card the state fallback exists to protect. Covered by a test. **The card model now states its rendering contract.** `summary` and `value` are plain text, to be rendered by text interpolation only, never `innerHTML` or a raw-HTML markdown pass. The content is doubly untrusted: model-generated, and often lifted from repo content the model just read, which is the path a prompt injection takes into a `task` or `message` field. Solid escapes by default so today's renderer is safe; the note is for whoever later adds a rich-text affordance. Web 184 tests, daemon 2434, typecheck and lint clean on both. Co-Authored-By: Claude Opus 5 (1M context) --- packages/protocol/src/types.ts | 38 +++++++++++++++ src/daemon/fleet.ts | 28 ++++------- web/src/lib/fleet-cards.test.ts | 60 +++++++++++++++++++++++- web/src/lib/fleet-cards.ts | 83 +++++++++++++++++---------------- 4 files changed, 149 insertions(+), 60 deletions(-) diff --git a/packages/protocol/src/types.ts b/packages/protocol/src/types.ts index eb3641c..852129e 100644 --- a/packages/protocol/src/types.ts +++ b/packages/protocol/src/types.ts @@ -43,6 +43,44 @@ export const PROTOCOL_VERSION = 1; * can produce, and either side simply doesn't use what the other didn't * declare. Unknown capability strings MUST be ignored, never rejected. */ +/** + * The conductor's fleet MCP server key, and its verb vocabulary split by + * class. Shared because BOTH sides need it and they must not disagree: the + * daemon uses the read set to build the provider's `allowedTools` and the send + * set as the hard "never auto-approve" gate, while a client uses the same split + * to render a fleet call as an observation or as an act. + * + * It lives here rather than in the daemon because a browser bundle cannot + * import daemon code — and a duplicated copy in the web client would let the + * two drift, with a newly-added send-class verb quietly rendering as a + * harmless read until somebody noticed. + */ +export const FLEET_TOOL_PREFIX = "mcp__codeoid_fleet__"; + +/** Read-class: observe only. Safe to auto-approve. */ +export const FLEET_READ_TOOLS = [ + "fleet_list", + "fleet_find", + "fleet_summary", + "fleet_recall", + "fleet_tasks", + "machine_map", +] as const; + +/** + * Send-class: acts on the fleet. NEVER auto-approved — the owner confirms each + * one with the full tool input visible (conductor-design R3). + */ +export const FLEET_SEND_TOOLS = [ + "fleet_send", + "fleet_interrupt", + "fleet_spawn", + "fleet_panel", +] as const; + +export type FleetReadTool = (typeof FLEET_READ_TOOLS)[number]; +export type FleetSendTool = (typeof FLEET_SEND_TOOLS)[number]; + export const CAPABILITIES = { /** Client renders rich `parts[]` content (vs the plain `content` fallback). */ PARTS: "parts", diff --git a/src/daemon/fleet.ts b/src/daemon/fleet.ts index e0d303e..11444d6 100644 --- a/src/daemon/fleet.ts +++ b/src/daemon/fleet.ts @@ -26,6 +26,11 @@ import { type McpSdkServerConfigWithInstance, } from "@anthropic-ai/claude-agent-sdk"; import type { MemoryEngine } from "./memory/index.js"; +import { + FLEET_READ_TOOLS, + FLEET_SEND_TOOLS, + FLEET_TOOL_PREFIX, +} from "../protocol/types.js"; const execFileAsync = promisify(execFile); @@ -122,14 +127,7 @@ export interface FleetDeps { * READ-class tool names — these (and only these) go into the provider's * `allowedTools`, so they run silently. (Server key `codeoid_fleet`.) */ -export const FLEET_TOOL_NAMES = [ - "fleet_list", - "fleet_find", - "fleet_summary", - "fleet_recall", - "fleet_tasks", - "machine_map", -] as const; +export const FLEET_TOOL_NAMES = FLEET_READ_TOOLS; /** * SEND-class tool names (P4). Deliberately a SEPARATE list that must NEVER @@ -138,15 +136,7 @@ export const FLEET_TOOL_NAMES = [ * makes every dispatch ride the existing approvalId flow, with the full tool * input shown to the owner (design R3). */ -export const FLEET_SEND_TOOL_NAMES = [ - "fleet_send", - "fleet_interrupt", - "fleet_spawn", - // A panel is N sends at once, so it is send-class by definition. Being on - // this list is what makes it ride the R3 approval flow (and, for a - // collaborative session, carry the goal's cost roll-up into that prompt). - "fleet_panel", -] as const; +export const FLEET_SEND_TOOL_NAMES = FLEET_SEND_TOOLS; /** * True for the fully-qualified MCP name of a send-class fleet tool. Session @@ -155,9 +145,7 @@ export const FLEET_SEND_TOOL_NAMES = [ * invariant, not a mode default. */ export function isFleetSendTool(toolName: string): boolean { - return FLEET_SEND_TOOL_NAMES.some( - (t) => toolName === `mcp__codeoid_fleet__${t}`, - ); + return FLEET_SEND_TOOL_NAMES.some((t) => toolName === `${FLEET_TOOL_PREFIX}${t}`); } /** diff --git a/web/src/lib/fleet-cards.test.ts b/web/src/lib/fleet-cards.test.ts index 1f61316..a5281ca 100644 --- a/web/src/lib/fleet-cards.test.ts +++ b/web/src/lib/fleet-cards.test.ts @@ -1,7 +1,11 @@ import { describe, it, expect } from "vitest"; import { classifyFleetTool, fleetVerb } from "./fleet-cards"; -import type { ToolInfo } from "../protocol/types"; +import { + FLEET_READ_TOOLS, + FLEET_SEND_TOOLS, + type ToolInfo, +} from "../protocol/types"; function tool(name: string, input?: unknown): ToolInfo { return { @@ -128,6 +132,25 @@ describe("classifyFleetTool", () => { expect(valueOf(card, "task")).toBe("bump the dep"); }); + it("falls back to state.input when tool.input is an explicit null", () => { + // `input` is typed `unknown`, so null is representable — and a JSON round + // trip preserves an explicit null while turning a missing key into + // undefined. Testing `!== undefined` would accept the null and render the + // approval card fieldless, which is the one card that must stay readable. + const awaiting = { + toolId: "t1", + name: "mcp__codeoid_fleet__fleet_send", + input: null, + state: { + phase: "waiting_confirmation", + input: { session: "studio-870", message: "run the linter" }, + }, + } as unknown as ToolInfo; + const card = classifyFleetTool(awaiting)!; + expect(card.summary).toBe("Send to studio-870"); + expect(valueOf(card, "message")).toBe("run the linter"); + }); + it("ignores a half-generated streaming input", () => { // partialInput is a fragment; a card built from it would show a workdir the // model has not finished writing. @@ -191,3 +214,38 @@ describe("classifyFleetTool", () => { }); }); }); + +describe("the daemon's own vocabulary", () => { + // Both sides import these lists from @highflame/codeoid-protocol, so the two + // cannot drift apart by construction — there is no second copy to fall out of + // date. What is still worth asserting is that every verb the shared lists + // name actually gets a classification consistent with its class. + + it("never classifies a send-class verb as a read", () => { + // The security-relevant direction: `unknown` would be acceptable + // (fail-safe), a read classification is the bug. + for (const verb of FLEET_SEND_TOOLS) { + const card = classifyFleetTool(fleet(verb, {}))!; + expect(card.kind === "observe" || card.kind === "resolve").toBe(false); + expect(card.sendClass).toBe(true); + } + }); + + it("classifies every read verb without falling back to unknown", () => { + // The other direction is not dangerous, but an unhandled read verb means + // the transcript quietly stops explaining itself. + for (const verb of FLEET_READ_TOOLS) { + const card = classifyFleetTool(fleet(verb, {}))!; + expect(card.kind).not.toBe("unknown"); + expect(card.sendClass).toBe(false); + } + }); + + it("still fails safe for a verb neither list names", () => { + // An older client meeting a newer daemon: unrecognised, so it must not be + // dressed up as a harmless read. + const card = classifyFleetTool(fleet("fleet_detonate", {}))!; + expect(card.kind).toBe("unknown"); + expect(card.sendClass).toBe(false); + }); +}); diff --git a/web/src/lib/fleet-cards.ts b/web/src/lib/fleet-cards.ts index d1345d0..499d7b2 100644 --- a/web/src/lib/fleet-cards.ts +++ b/web/src/lib/fleet-cards.ts @@ -19,47 +19,28 @@ * missing fields, never a crash and never a confident lie. * * **Unknown verbs fail safe.** The read/send split is a SECURITY-relevant - * classification that the daemon enforces (`FLEET_SEND_TOOL_NAMES` in - * `src/daemon/fleet.ts` — send-class verbs can never be auto-approved). This - * module cannot import daemon code, so the vocabulary is duplicated below and - * can drift. A verb this module does not recognise is therefore classified - * `"unknown"` — never `"observe"` — so a future send-class verb added daemon- - * side can never be rendered as a harmless read here. + * classification the daemon enforces — send-class verbs can never be + * auto-approved (conductor-design R3). The vocabulary is NOT duplicated here: + * both sides import `FLEET_READ_TOOLS` / `FLEET_SEND_TOOLS` from + * `@highflame/codeoid-protocol`, so the two cannot drift apart. A verb neither + * list names — an older client meeting a newer daemon — is still classified + * `"unknown"` rather than `"observe"`, so an unrecognised verb can never be + * rendered as a harmless read. */ -import type { ToolInfo } from "../protocol/types"; +import { + FLEET_READ_TOOLS, + FLEET_SEND_TOOLS, + FLEET_TOOL_PREFIX, + type ToolInfo, +} from "../protocol/types"; -/** The daemon's in-process fleet MCP server key. */ -const FLEET_TOOL_PREFIX = "mcp__codeoid_fleet__"; +export type FleetVerb = + | (typeof FLEET_READ_TOOLS)[number] + | (typeof FLEET_SEND_TOOLS)[number]; -/** - * Read-class verbs — observe only, auto-approved daemon-side. - * Mirrors `FLEET_TOOL_NAMES`; see the fail-safe note in the module header. - */ -const READ_VERBS = [ - "fleet_list", - "fleet_find", - "fleet_summary", - "fleet_recall", - "fleet_tasks", - "machine_map", -] as const; - -/** - * Send-class verbs — act on the fleet, and never auto-approved daemon-side. - * Mirrors `FLEET_SEND_TOOL_NAMES`. - */ -const SEND_VERBS = [ - "fleet_send", - "fleet_spawn", - "fleet_interrupt", - "fleet_panel", -] as const; - -export type FleetVerb = (typeof READ_VERBS)[number] | (typeof SEND_VERBS)[number]; - -const READ_SET: ReadonlySet = new Set(READ_VERBS); -const SEND_SET: ReadonlySet = new Set(SEND_VERBS); +const READ_SET: ReadonlySet = new Set(FLEET_READ_TOOLS); +const SEND_SET: ReadonlySet = new Set(FLEET_SEND_TOOLS); /** * What a fleet call is *for*, which is what decides how loud its card should be. @@ -80,7 +61,12 @@ export interface FleetCard { * reported as safe — see the module header. */ sendClass: boolean; - /** One-line summary for the card header. Never raw JSON. */ + /** + * One-line summary for the card header. Never raw JSON. + * + * PLAIN TEXT, and partly model-generated — render via text interpolation + * only, never `innerHTML` or a raw-HTML markdown pass. See `FleetCardField`. + */ summary: string; /** Ordered detail rows the card renders. Absent fields are omitted, not blanked. */ fields: FleetCardField[]; @@ -88,6 +74,18 @@ export interface FleetCard { export interface FleetCardField { label: string; + /** + * PLAIN TEXT. Render via text interpolation only — never `innerHTML`, and + * never through a markdown renderer that emits raw HTML. + * + * This module is safe by construction because it only ever produces strings, + * so the guarantee lives entirely at the render site. It matters because the + * content is doubly untrusted: model-generated, and frequently lifted from + * repo content the model just read — which is exactly the path a prompt + * injection takes to put an attacker-chosen string in a `task` or `message` + * field. Solid interpolates as text by default, so today's renderer is fine; + * this note exists for whoever later adds a rich-text affordance here. + */ value: string; /** * Long free text (a task brief, a message body) that a card should render in @@ -233,9 +231,16 @@ const OBSERVE_SUMMARY: Record = { * `streaming` is deliberately not consulted: its `partialInput` is a * half-generated fragment, and a card built from it would show a target or * workdir that the model has not finished writing. + * + * The `!= null` is load-bearing, not sloppiness. `input` is typed `unknown`, + * so `null` is representable — and a JSON round-trip preserves an explicit + * `null` while turning a missing key into `undefined`. Testing `!== undefined` + * would accept that `null`, `isRecord` would then reject it, and the card would + * render fieldless: precisely the approval-prompt card this fallback exists to + * protect. */ function resolveToolInput(tool: ToolInfo): unknown { - if (tool.input !== undefined) return tool.input; + if (tool.input != null) return tool.input; return tool.state.phase === "waiting_confirmation" ? tool.state.input : undefined; }