From 1a7e09f73148fb15fb704809e0922c5aab44112b Mon Sep 17 00:00:00 2001 From: Yash Datta Date: Sat, 12 Sep 2026 01:25:55 +0800 Subject: [PATCH] feat(web): show repo, branch and target on a dispatch approval MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit P5.3 (conductor-design R3, conductor-frontends-design §8). A send-class fleet dispatch now proposes itself with where it is going, not just what the tool is called. R3 names silent misrouting as the one failure that would kill trust in the conductor: "any send-class action to an existing user-owned session first proposes it with repo + branch + content shown". The approval bar showed the tool name and its description, which is what the CONDUCTOR called the target — not what the owner needs to verify. Resolution lives in `lib/dispatch-preview.ts` as pure functions, since turning a target name into a session is the decision worth testing. Two rules matter more than the layout: **An ambiguous id prefix resolves to NOTHING.** The conductor is told to name targets by name, but its tools also accept an id or prefix. Picking one of several prefix matches would be a coin flip presented as a fact, on the prompt whose entire job is preventing that. An exact name beats an id prefix for the same reason. **An unresolvable target is shown, never hidden.** That is exactly when an owner is at risk of approving the wrong thing, and a prompt that quietly shows no repo reads as "no repo involved" rather than "I could not tell you which". It warns without blocking: the daemon resolves names itself and this client's view can legitimately be stale, so refusing the approval would be wrong. A spawn is previewed by its workdir instead, and deliberately never flagged unresolved — there is no existing session to resolve, so flagging would cry wolf on every spawn. `classifyFleetInput` is split out of `classifyFleetTool` so the approval gate and the transcript share ONE classifier. The gate holds a raw (toolName, input) pair rather than a ToolInfo, and letting it grow its own parser is how the prompt and the transcript end up disagreeing about what a dispatch says. Non-fleet approvals are untouched: the classifier returns null and the existing bar renders exactly as before. 14 dispatch-preview tests; 243 web tests; typecheck and build clean. The 5 remaining ApprovalBar lint warnings are pre-existing — verified by linting the stashed baseline. Co-Authored-By: Claude Opus 5 (1M context) --- web/src/components/transcript/ApprovalBar.tsx | 80 ++++++++++ web/src/lib/dispatch-preview.test.ts | 125 ++++++++++++++++ web/src/lib/dispatch-preview.ts | 137 ++++++++++++++++++ web/src/lib/fleet-cards.ts | 18 ++- 4 files changed, 357 insertions(+), 3 deletions(-) create mode 100644 web/src/lib/dispatch-preview.test.ts create mode 100644 web/src/lib/dispatch-preview.ts diff --git a/web/src/components/transcript/ApprovalBar.tsx b/web/src/components/transcript/ApprovalBar.tsx index 75e9577..aa449f5 100644 --- a/web/src/components/transcript/ApprovalBar.tsx +++ b/web/src/components/transcript/ApprovalBar.tsx @@ -38,9 +38,25 @@ import { newRequestId, request } from "../../state/connection"; import { epochOf, focusedSessionMessages } from "../../state/messages"; import { focusedSession, focusedSessionId } from "../../state/sessions"; import { findPendingApproval } from "../../lib/approvals"; +import { classifyFleetInput } from "../../lib/fleet-cards"; +import { + dispatchPreview, + hasUnresolved, + type DispatchPreview as DispatchPreviewModel, +} from "../../lib/dispatch-preview"; +import { sessionList } from "../../state/sessions"; import type { CollaborationCost, SessionMessage } from "../../protocol/types"; import { formatCollaborationCost } from "../../lib/format"; +/** + * Repo/branch/content preview for a send-class fleet dispatch, or null for any + * other tool. Non-fleet approvals are untouched. + */ +function dispatchFor(toolName: string, input: unknown): DispatchPreviewModel | null { + const card = classifyFleetInput(toolName, input); + return card ? dispatchPreview(card, sessionList()) : null; +} + /** Custom event the prompt listens for so "Refine" can focus + hint. */ function focusPromptWithHint(hint: string): void { window.dispatchEvent( @@ -207,6 +223,7 @@ const ApprovalBar: Component = () => { toolName={snap().toolName} description={snap().description} collaborationCost={snap().collaborationCost} + dispatch={dispatchFor(snap().toolName, snap().input)} isPlanMode={isPlanMode()} busy={busy()} onApprove={() => safeApprove(true)} @@ -235,11 +252,67 @@ const ApprovalBar: Component = () => { ); }; +/** + * Where a dispatch is actually going — the R3 preview. + * + * Deliberately compact: this sits above the prompt on every dispatch, and a + * block that pushes the buttons off screen gets dismissed rather than read. The + * content is clamped for the same reason; the full brief is in the tool card in + * the transcript above. + */ +const DispatchDetails: Component<{ preview: DispatchPreviewModel }> = (props) => ( +
+ + {(t) => ( +
+ {t.name} + + {(w) => {w()}} + + + {(b) => ( + + {b()} + + )} + + {/* Never silent: a prompt showing no repo would read as "no repo + involved" rather than "I could not tell you which". The daemon + resolves names itself and may still succeed, so this warns + without blocking. */} + + + unresolved + + +
+ )} + + + {(c) => ( +

+ {c()} +

+ )} + + +

+ One or more targets did not resolve here — verify before approving. +

+ +
+); + const BinaryBar: Component<{ toolName: string; description: string; /** Present only for a send-class fleet dispatch from a collaborative session. */ collaborationCost?: CollaborationCost; + /** Present only for a send-class fleet dispatch — repo/branch/content (R3). */ + dispatch?: DispatchPreviewModel | null; isPlanMode: boolean; busy: boolean; onApprove: () => void; @@ -263,6 +336,13 @@ const BinaryBar: Component<{ {/* What the goal has already cost, on the button that authorizes more. Shared formatter so web, Telegram and the TUI show the owner the same number in the same words. */} + {/* R3: a send-class dispatch is proposed with repo + branch + content + shown, because silent misrouting is the failure that would kill + trust in the conductor. Rendered ABOVE the cost roll-up — where the + instruction is going outranks what it has spent. */} + + {(d) => } + {(c) => (
diff --git a/web/src/lib/dispatch-preview.test.ts b/web/src/lib/dispatch-preview.test.ts new file mode 100644 index 0000000..2d94d70 --- /dev/null +++ b/web/src/lib/dispatch-preview.test.ts @@ -0,0 +1,125 @@ +import { describe, it, expect } from "vitest"; + +import { classifyFleetInput } from "./fleet-cards"; +import { dispatchPreview, hasUnresolved, resolveTarget } from "./dispatch-preview"; +import type { SessionInfo } from "../protocol/types"; + +const session = (id: string, name: string, over: Partial = {}): SessionInfo => + ({ id, name, workdir: `/repo/${name}`, status: "idle", ...over }) as SessionInfo; + +const API = session("aaaa1111-2222", "api", { + worktree: { branch: "codeoid/fix-login", path: "/repo/api" }, +} as Partial); +const WEB = session("bbbb3333-4444", "web"); +const SESSIONS = [API, WEB]; + +const card = (verb: string, input: unknown) => classifyFleetInput(`mcp__codeoid_fleet__${verb}`, input)!; + +describe("resolveTarget", () => { + it("prefers an exact name — how the conductor is told to address targets", () => { + expect(resolveTarget("api", SESSIONS)?.id).toBe(API.id); + }); + + it("accepts an id and an unambiguous id prefix", () => { + expect(resolveTarget(API.id, SESSIONS)?.name).toBe("api"); + expect(resolveTarget("aaaa1111", SESSIONS)?.name).toBe("api"); + }); + + it("resolves an AMBIGUOUS prefix to nothing rather than guessing", () => { + // Picking one would be a coin flip presented as a fact, on the prompt + // whose whole job is to prevent misrouting. + const twins = [session("dup1", "a"), session("dup2", "b")]; + expect(resolveTarget("dup", twins)).toBeNull(); + }); + + it("prefers an exact name over an id prefix that also matches", () => { + const odd = [session("zzz", "shared"), session("shared-id", "other")]; + expect(resolveTarget("shared", odd)?.id).toBe("zzz"); + }); + + it("returns null for a name nothing matches", () => { + expect(resolveTarget("ghost", SESSIONS)).toBeNull(); + }); +}); + +describe("dispatchPreview — send-class", () => { + it("shows repo and branch for a send, per R3", () => { + const p = dispatchPreview( + card("fleet_send", { session: "api", message: "run the linter" }), + SESSIONS, + )!; + expect(p.kind).toBe("send"); + expect(p.content).toBe("run the linter"); + expect(p.targets).toHaveLength(1); + expect(p.targets[0]).toMatchObject({ + name: "api", + workdir: "/repo/api", + branch: "codeoid/fix-login", + unresolved: false, + }); + expect(hasUnresolved(p)).toBe(false); + }); + + it("reports a branchless session without inventing one", () => { + const p = dispatchPreview(card("fleet_send", { session: "web", message: "hi" }), SESSIONS)!; + expect(p.targets[0]!.workdir).toBe("/repo/web"); + expect(p.targets[0]!.branch).toBeNull(); + }); + + it("FLAGS a target that resolves to nothing", () => { + // The case where an owner is most at risk of approving the wrong thing. A + // prompt that quietly shows no repo reads as "no repo involved" rather + // than "I could not tell you". + const p = dispatchPreview(card("fleet_send", { session: "ghost", message: "x" }), SESSIONS)!; + expect(p.targets[0]!.unresolved).toBe(true); + expect(hasUnresolved(p)).toBe(true); + }); + + it("expands every member of a panel", () => { + const p = dispatchPreview( + card("fleet_panel", { sessions: ["api", "web", "ghost"], message: "review" }), + SESSIONS, + )!; + expect(p.targets.map((t) => t.name)).toEqual(["api", "web", "ghost"]); + expect(p.targets.map((t) => t.unresolved)).toEqual([false, false, true]); + expect(p.content).toBe("review"); + }); + + it("handles an interrupt, which carries a target but no content", () => { + const p = dispatchPreview(card("fleet_interrupt", { session: "api" }), SESSIONS)!; + expect(p.targets[0]!.workdir).toBe("/repo/api"); + expect(p.content).toBeNull(); + }); + + it("survives a dispatch with no target at all", () => { + const p = dispatchPreview(card("fleet_send", { message: "orphan" }), SESSIONS)!; + expect(p.targets).toEqual([]); + expect(hasUnresolved(p)).toBe(false); + }); +}); + +describe("dispatchPreview — spawn", () => { + it("shows the workdir being created in, and does not cry wolf about resolution", () => { + const p = dispatchPreview( + card("fleet_spawn", { workdir: "/repo/new", shape: "scout", task: "investigate" }), + SESSIONS, + )!; + expect(p.kind).toBe("spawn"); + expect(p.targets[0]).toMatchObject({ name: "scout", workdir: "/repo/new", unresolved: false }); + expect(p.content).toBe("investigate"); + // There is no existing session to resolve, so flagging it would fire on + // every spawn. + expect(hasUnresolved(p)).toBe(false); + }); +}); + +describe("dispatchPreview — non-dispatch", () => { + it("returns null for a read verb, which has nothing to propose", () => { + expect(dispatchPreview(card("fleet_find", { query: "x" }), SESSIONS)).toBeNull(); + expect(dispatchPreview(card("machine_map", {}), SESSIONS)).toBeNull(); + }); + + it("returns null for an unrecognised verb rather than treating it as a send", () => { + expect(dispatchPreview(card("fleet_detonate", {}), SESSIONS)).toBeNull(); + }); +}); diff --git a/web/src/lib/dispatch-preview.ts b/web/src/lib/dispatch-preview.ts new file mode 100644 index 0000000..24d8889 --- /dev/null +++ b/web/src/lib/dispatch-preview.ts @@ -0,0 +1,137 @@ +/** + * What a fleet dispatch is about to touch — repo, branch, target session. + * + * conductor-design R3 is the reason this exists: a send-class action to an + * existing session "first proposes it with **repo + branch + content shown**, + * and acts only on confirm". Silent misrouting is named there as the one + * failure that would kill trust in the conductor, so the approval prompt has to + * show the owner where the instruction is actually going — not just the tool's + * own words for it. + * + * Pure: resolving a target name to a session is the decision worth testing. + */ + +import type { FleetCard } from "./fleet-cards"; +import type { SessionInfo } from "../protocol/types"; + +export interface DispatchTarget { + /** The name the conductor used, exactly as it will be dispatched. */ + name: string; + /** The session it resolves to, or null when nothing matches. */ + session: SessionInfo | null; + /** Absolute workdir of the resolved session. */ + workdir: string | null; + /** Worktree branch, when the session has one. */ + branch: string | null; + /** + * True when the name matched nothing this client can see. + * + * Surfaced rather than hidden: an unresolvable target is EXACTLY the case + * where an owner is at risk of approving the wrong thing, and a prompt that + * quietly shows no repo reads as "no repo involved" rather than "I could not + * tell you". The daemon resolves names itself and may still succeed — the + * client's view can legitimately be stale — so this is a warning, never a + * reason to block the approval. + */ + unresolved: boolean; +} + +export interface DispatchPreview { + /** `spawn` creates a new worker; `send` routes into sessions that already exist. */ + kind: "spawn" | "send"; + targets: DispatchTarget[]; + /** The instruction or brief being delivered, when the card carries one. */ + content: string | null; +} + +/** + * Resolve a session reference the way a human reading the prompt would. + * + * The conductor is told to name targets by NAME so the owner can verify the + * repo at a glance, but its own tools also accept an id or id prefix — so all + * three are matched here. An exact name wins over a prefix: two sessions can + * share a prefix, and silently preferring the wrong one is the misrouting R3 + * exists to prevent. + */ +export function resolveTarget( + ref: string, + sessions: readonly SessionInfo[], +): SessionInfo | null { + const exactName = sessions.find((s) => s.name === ref); + if (exactName) return exactName; + const exactId = sessions.find((s) => s.id === ref); + if (exactId) return exactId; + + const byPrefix = sessions.filter((s) => s.id.startsWith(ref)); + // An ambiguous prefix resolves to NOTHING. Picking one would be a coin flip + // presented as a fact, on the prompt whose job is to prevent exactly that. + return byPrefix.length === 1 ? byPrefix[0]! : null; +} + +function describe(name: string, sessions: readonly SessionInfo[]): DispatchTarget { + const session = resolveTarget(name, sessions); + return { + name, + session, + workdir: session?.workdir ?? null, + branch: session?.worktree?.branch ?? null, + unresolved: session === null, + }; +} + +/** Read a field off a card by label, or null. */ +function field(card: FleetCard, label: string): string | null { + return card.fields.find((f) => f.label === label)?.value ?? null; +} + +/** + * Build the preview for a send-class dispatch, or null when the card is not + * one (a read verb has nothing to propose — it already ran). + */ +export function dispatchPreview( + card: FleetCard, + sessions: readonly SessionInfo[], +): DispatchPreview | null { + if (!card.sendClass) return null; + + if (card.verb === "fleet_spawn") { + // A spawn has no existing session to verify — the workdir IS the thing to + // check, and it comes straight off the card. + const workdir = field(card, "workdir"); + return { + kind: "spawn", + targets: [ + { + name: field(card, "shape") ?? "worker", + session: null, + workdir, + branch: null, + // Not "unresolved": there is nothing to resolve yet, and flagging it + // would cry wolf on every spawn. + unresolved: false, + }, + ], + content: field(card, "task"), + }; + } + + // send / interrupt / panel all route into sessions that already exist. + const raw = field(card, "sessions") ?? field(card, "target"); + const names = raw + ? raw + .split(",") + .map((n) => n.trim()) + .filter((n) => n.length > 0) + : []; + + return { + kind: "send", + targets: names.map((n) => describe(n, sessions)), + content: field(card, "message"), + }; +} + +/** True when any target could not be resolved — drives the prompt's warning. */ +export function hasUnresolved(preview: DispatchPreview): boolean { + return preview.targets.some((t) => t.unresolved); +} diff --git a/web/src/lib/fleet-cards.ts b/web/src/lib/fleet-cards.ts index 499d7b2..a43fee1 100644 --- a/web/src/lib/fleet-cards.ts +++ b/web/src/lib/fleet-cards.ts @@ -106,11 +106,23 @@ export function fleetVerb(toolName: string): string | null { * ordinary tool that should keep its existing rendering. */ export function classifyFleetTool(tool: ToolInfo): FleetCard | null { - const verb = fleetVerb(tool.name); + return classifyFleetInput(tool.name, resolveToolInput(tool)); +} + +/** + * Same classification from a raw `(toolName, input)` pair. + * + * The approval gate holds those two directly rather than a `ToolInfo`, and it + * is the most important consumer of this module — it renders what the owner is + * about to authorize. Splitting the entry point keeps both callers on ONE + * classifier rather than letting the approval surface grow its own parser that + * could disagree with the transcript about what a dispatch says. + */ +export function classifyFleetInput(toolName: string, rawInput: unknown): FleetCard | null { + const verb = fleetVerb(toolName); if (verb === null) return null; - const resolved = resolveToolInput(tool); - const input = isRecord(resolved) ? resolved : {}; + const input = isRecord(rawInput) ? rawInput : {}; const sendClass = SEND_SET.has(verb); const known = sendClass || READ_SET.has(verb);