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);