diff --git a/README.md b/README.md index afe6ea54..28d742d7 100644 --- a/README.md +++ b/README.md @@ -164,6 +164,9 @@ Every agent launched by Dispatch gets access to MCP tools via an agent-scoped en | `dispatch_pin` | Surface key info in the sidebar (URLs, ports, PRs, files) | | `dispatch_share` | Upload screenshots and media to the agent's media pane | | `dispatch_list_media` | List media files shared with or by this agent | +| `dispatch_delete_media` | Permanently remove a shared media file | +| `dispatch_list_pins` | List current sidebar pins for this agent | +| `dispatch_delete_pin` | Permanently remove a pin by its listed stable ID | | `list_personas` | List available persona reviewers for this project | | `dispatch_launch_persona` | Launch a persona child agent for automated review | | `dispatch_review_list_feedback` | List human review feedback items with statuses and threads | diff --git a/apps/server/src/agents/manager.ts b/apps/server/src/agents/manager.ts index b8b9f6a5..baefc764 100644 --- a/apps/server/src/agents/manager.ts +++ b/apps/server/src/agents/manager.ts @@ -101,7 +101,10 @@ const MAX_PINS = 50; function normalizeInitialPins(pins: AgentPin[]): AgentPin[] { const byLabel = new Map(); for (const pin of pins) { - byLabel.set(pin.label.toLowerCase(), pin); + byLabel.set(pin.label.toLowerCase(), { + ...pin, + id: pin.id ?? randomUUID(), + }); } const deduped = Array.from(byLabel.values()); if (deduped.length > MAX_PINS) { @@ -1078,42 +1081,76 @@ export class AgentManager { } async upsertPin(id: string, pin: AgentPin): Promise { - const current = await this.getAgent(id); - if (!current) throw new AgentError("Agent not found.", 404); - - const pins = (current.pins ?? []).filter( - (p) => p.label.toLowerCase() !== pin.label.toLowerCase() - ); - if (pins.length >= MAX_PINS) { - throw new AgentError(`Maximum of ${MAX_PINS} pins reached.`, 400); - } - pins.push(pin); - - await this.pool.query( - `UPDATE agents SET pins = $2::jsonb, updated_at = NOW() WHERE id = $1`, - [id, JSON.stringify(pins)] - ); + await this.mutatePins(id, (currentPins) => { + const existing = currentPins.find( + (p) => p.label.toLowerCase() === pin.label.toLowerCase() + ); + const pins = currentPins.filter( + (p) => p.label.toLowerCase() !== pin.label.toLowerCase() + ); + if (pins.length >= MAX_PINS) { + throw new AgentError(`Maximum of ${MAX_PINS} pins reached.`, 400); + } + pins.push({ ...pin, id: existing?.id ?? pin.id ?? randomUUID() }); + return pins; + }); return (await this.getAgent(id)) as AgentRecord; } - async deletePin(id: string, label: string): Promise { - const current = await this.getAgent(id); - if (!current) throw new AgentError("Agent not found.", 404); + async deletePinById(id: string, pinId: string): Promise { + await this.mutatePins(id, (currentPins) => { + const pins = currentPins.filter((p) => p.id !== pinId); + if (pins.length === currentPins.length) { + throw new AgentError("Pin not found.", 404); + } + return pins; + }); - const lowerLabel = label.toLowerCase(); - const pins = (current.pins ?? []).filter( - (p) => p.label.toLowerCase() !== lowerLabel - ); + return (await this.getAgent(id)) as AgentRecord; + } - await this.pool.query( - `UPDATE agents SET pins = $2::jsonb, updated_at = NOW() WHERE id = $1`, - [id, JSON.stringify(pins)] - ); + async deletePinByLabel(id: string, label: string): Promise { + await this.mutatePins(id, (currentPins) => { + const pins = currentPins.filter( + (pin) => pin.label.toLowerCase() !== label.toLowerCase() + ); + if (pins.length === currentPins.length) { + throw new AgentError("Pin not found.", 404); + } + return pins; + }); return (await this.getAgent(id)) as AgentRecord; } + private async mutatePins( + id: string, + mutate: (pins: AgentPin[]) => AgentPin[] + ): Promise { + const client = await this.pool.connect(); + try { + await client.query("BEGIN"); + const result = await client.query<{ pins: AgentPin[] }>( + "SELECT pins FROM agents WHERE id = $1 FOR UPDATE", + [id] + ); + if (result.rows.length === 0) + throw new AgentError("Agent not found.", 404); + const pins = mutate(result.rows[0]!.pins ?? []); + await client.query( + "UPDATE agents SET pins = $2::jsonb, updated_at = NOW() WHERE id = $1", + [id, JSON.stringify(pins)] + ); + await client.query("COMMIT"); + } catch (error) { + await client.query("ROLLBACK").catch(() => {}); + throw error; + } finally { + client.release(); + } + } + async reconcileAgents(): Promise { // Two passes: status reconciliation + orphan-session cleanup. The // SSE broadcaster doesn't need the changed-record list at this diff --git a/apps/server/src/agents/tmux/command-builder.ts b/apps/server/src/agents/tmux/command-builder.ts index 92759e29..d5a916e9 100644 --- a/apps/server/src/agents/tmux/command-builder.ts +++ b/apps/server/src/agents/tmux/command-builder.ts @@ -173,7 +173,7 @@ export function buildLaunchGuidance( "Report status with dispatch_event. Types: working (making progress — includes debugging, fixing test failures, investigating errors), blocked (completely stuck with no further approach to try — NOT for errors or test failures you plan to fix next), waiting_user (need a decision or approval), done (task complete), idle (no-op, just answered a question). Emit working at turn start and when shifting phases. Emit a terminal event before your final response. Your reported status is verified against session activity and auto-corrected when it doesn't match." ); rules.push( - "Pin key info with dispatch_pin so it surfaces in the sidebar — especially values users may need to copy/paste: URLs, commands, branch names, IDs, tokens, simulator UDIDs. Types: url (dev servers, docs), port (server ports), pr (PR links), filename (key files), code (short snippets, env vars, IDs), string (status, decisions), markdown (short structured summaries). Update or delete stale pins. For longer artifacts, write a file via dispatch_share and pin a reference." + "Pin key info with dispatch_pin so it surfaces in the sidebar — especially values users may need to copy/paste: URLs, commands, branch names, IDs, tokens, simulator UDIDs. Types: url (dev servers, docs), port (server ports), pr (PR links), filename (key files), code (short snippets, env vars, IDs), string (status, decisions), markdown (short structured summaries). To delete a stale pin, call dispatch_list_pins then dispatch_delete_pin with its id. For longer artifacts, write a file via dispatch_share and pin a reference." ); rules.push( "Playwright: default headless. Capture at least one screenshot per UI flow via dispatch_share. Call browser_close when done." diff --git a/apps/server/src/agents/types.ts b/apps/server/src/agents/types.ts index fa1e5b1d..2d3f7327 100644 --- a/apps/server/src/agents/types.ts +++ b/apps/server/src/agents/types.ts @@ -37,6 +37,7 @@ export type PinType = | "markdown"; export type AgentPin = { + id?: string; label: string; value: string; type: PinType; diff --git a/apps/server/src/db/migrations/0036_pin-ids.sql b/apps/server/src/db/migrations/0036_pin-ids.sql new file mode 100644 index 00000000..3120df83 --- /dev/null +++ b/apps/server/src/db/migrations/0036_pin-ids.sql @@ -0,0 +1,23 @@ +-- Normalize legacy pins and give each usable entry a unique stable ID. +UPDATE agents +SET pins = CASE + WHEN jsonb_typeof(pins) <> 'array' THEN '[]'::jsonb + ELSE COALESCE(( + SELECT jsonb_agg( + pin || jsonb_build_object( + 'id', + CASE WHEN existing_id <> '' AND id_count = 1 THEN existing_id + ELSE 'pin_' || md5(agents.id || ordinal::text || jsonb_extract_path_text(pin, 'label') || jsonb_extract_path_text(pin, 'value')) END + ) ORDER BY ordinal + ) + FROM ( + SELECT pin, ordinal, COALESCE(jsonb_extract_path_text(pin, 'id'), '') AS existing_id, + COUNT(*) OVER (PARTITION BY COALESCE(jsonb_extract_path_text(pin, 'id'), '')) AS id_count + FROM jsonb_array_elements(pins) WITH ORDINALITY AS items(pin, ordinal) + WHERE jsonb_typeof(pin) = 'object' + AND jsonb_extract_path_text(pin, 'label') IS NOT NULL + AND COALESCE(jsonb_extract_path_text(pin, 'value'), '') <> '' + AND jsonb_extract_path_text(pin, 'type') IN ('string', 'url', 'port', 'code', 'pr', 'filename', 'markdown') + ) AS valid_pins + ), '[]'::jsonb) +END; diff --git a/apps/server/src/routes/mcp.ts b/apps/server/src/routes/mcp.ts index 559b5996..1af3a1c8 100644 --- a/apps/server/src/routes/mcp.ts +++ b/apps/server/src/routes/mcp.ts @@ -47,6 +47,8 @@ type McpRouteDeps = { mcpRenameSession: unknown; mcpShareMedia: unknown; mcpListMedia: unknown; + mcpDeleteMedia: unknown; + mcpListPins: unknown; mcpListPersonas: unknown; mcpLaunchPersona: unknown; mcpLaunchAgent: unknown; @@ -58,6 +60,7 @@ type McpRouteDeps = { mcpListReviewFeedback: unknown; mcpUpsertPin: unknown; mcpDeletePin: unknown; + mcpDeletePinByLabel: unknown; mcpGetParentContext: unknown; mcpJobComplete: unknown; mcpJobFailed: unknown; @@ -196,8 +199,11 @@ export async function registerMcpRoutes( renameSession: deps.mcpRenameSession, shareMedia: deps.mcpShareMedia, listMedia: deps.mcpListMedia, + deleteMedia: deps.mcpDeleteMedia, upsertPin: deps.mcpUpsertPin, deletePin: deps.mcpDeletePin, + deletePinByLabel: deps.mcpDeletePinByLabel, + listPins: deps.mcpListPins, listPersonas: deps.mcpListPersonas, launchPersona: deps.mcpLaunchPersona, launchAgent: deps.mcpLaunchAgent, @@ -279,6 +285,7 @@ export async function registerMcpRoutes( renameSession: deps.mcpRenameSession, shareMedia: deps.mcpShareMedia, listMedia: deps.mcpListMedia, + deleteMedia: deps.mcpDeleteMedia, listPersonas: deps.mcpListPersonas, launchPersona: deps.mcpLaunchPersona, launchAgent: deps.mcpLaunchAgent, @@ -292,6 +299,8 @@ export async function registerMcpRoutes( listAgentsForAgent: deps.mcpListAgentsForAgent, upsertPin: deps.mcpUpsertPin, deletePin: deps.mcpDeletePin, + deletePinByLabel: deps.mcpDeletePinByLabel, + listPins: deps.mcpListPins, getParentContext: deps.mcpGetParentContext, getActivitySummary: (params: Record) => telemetry.getActivitySummary(deps.pool, params as never) as Promise< diff --git a/apps/server/src/server.ts b/apps/server/src/server.ts index 1ab5b743..214e0faf 100644 --- a/apps/server/src/server.ts +++ b/apps/server/src/server.ts @@ -489,6 +489,7 @@ async function registerRoutes() { mcpRenameSession: mcpHandlers.renameSession, mcpShareMedia: mcpHandlers.shareMedia, mcpListMedia: mcpHandlers.listMedia, + mcpDeleteMedia: mcpHandlers.deleteMedia, mcpListPersonas: mcpHandlers.listPersonas, mcpLaunchPersona: mcpHandlers.launchPersona, mcpLaunchAgent: mcpHandlers.launchAgent, @@ -502,6 +503,8 @@ async function registerRoutes() { mcpListAgentsForAgent: mcpHandlers.listAgentsForAgent, mcpUpsertPin: mcpHandlers.upsertPin, mcpDeletePin: mcpHandlers.deletePin, + mcpDeletePinByLabel: mcpHandlers.deletePinByLabel, + mcpListPins: mcpHandlers.listPins, mcpGetParentContext: mcpHandlers.getParentContext, mcpJobComplete: mcpHandlers.jobComplete, mcpJobFailed: mcpHandlers.jobFailed, diff --git a/apps/server/src/server/mcp-handlers.ts b/apps/server/src/server/mcp-handlers.ts index 19f55832..1e1101fc 100644 --- a/apps/server/src/server/mcp-handlers.ts +++ b/apps/server/src/server/mcp-handlers.ts @@ -1,6 +1,6 @@ import path from "node:path"; import { randomUUID } from "node:crypto"; -import { mkdir, readFile, writeFile } from "node:fs/promises"; +import { mkdir, readFile, unlink, writeFile } from "node:fs/promises"; import type { FastifyBaseLogger } from "fastify"; import type { Pool } from "pg"; @@ -181,17 +181,41 @@ async function handleUpsertPin( } async function handleDeletePin( + deps: CreateMcpHandlersDeps, + agentId: string, + pinId: string +): Promise { + const agent = await deps.agentManager.deletePinById(agentId, pinId); + deps.publishUiEvent({ + type: "agent.upsert", + agent: deps.withStreamFlag(agent), + }); +} + +async function handleDeletePinByLabel( deps: CreateMcpHandlersDeps, agentId: string, label: string ): Promise { - const agent = await deps.agentManager.deletePin(agentId, label); + const agent = await deps.agentManager.deletePinByLabel(agentId, label); deps.publishUiEvent({ type: "agent.upsert", agent: deps.withStreamFlag(agent), }); } +async function handleListPins( + deps: CreateMcpHandlersDeps, + agentId: string +): Promise> { + const agent = await deps.agentManager.getAgent(agentId); + if (!agent) throw new Error("Agent not found."); + return (agent.pins ?? []).map((pin) => { + if (!pin.id) throw new Error("Pin is missing its stable ID."); + return { id: pin.id, label: pin.label, value: pin.value, type: pin.type }; + }); +} + async function handleRenameSession( deps: CreateMcpHandlersDeps, agentId: string, @@ -635,6 +659,54 @@ async function handleListMedia( })); } +async function handleDeleteMedia( + deps: CreateMcpHandlersDeps, + agentId: string, + fileName: string +): Promise { + const agent = await deps.agentManager.getAgent(agentId); + if (!agent) throw new Error("Agent not found."); + + const result = await deps.pool.query<{ file_name: string }>( + "SELECT file_name FROM media WHERE agent_id = $1 AND file_name = $2", + [agentId, fileName] + ); + if (result.rows.length === 0) { + throw new Error( + "No media file found with the given fileName for this agent." + ); + } + + const storedFileName = result.rows[0].file_name; + const mediaDir = resolveMediaDir(agentId, agent.mediaDir, deps.mediaRoot); + const filePath = path.join(mediaDir, storedFileName); + const resolvedMediaDir = path.resolve(mediaDir); + if (!path.resolve(filePath).startsWith(resolvedMediaDir + path.sep)) { + throw new Error("Invalid media file path."); + } + + try { + await unlink(filePath); + } catch (error: unknown) { + if ( + !( + error && + typeof error === "object" && + "code" in error && + error.code === "ENOENT" + ) + ) { + throw error; + } + } + + await deps.pool.query( + "DELETE FROM media WHERE agent_id = $1 AND file_name = $2", + [agentId, storedFileName] + ); + deps.publishUiEvent({ type: "media.changed", agentId }); +} + // --------------------------------------------------------------------------- // Thin delegation layer // --------------------------------------------------------------------------- @@ -669,8 +741,13 @@ export function createMcpHandlers(deps: CreateMcpHandlersDeps) { pin: { label: string; value: string; type: string } ) => handleUpsertPin(deps, agentId, pin), - deletePin: (agentId: string, label: string) => - handleDeletePin(deps, agentId, label), + deletePin: (agentId: string, pinId: string) => + handleDeletePin(deps, agentId, pinId), + + deletePinByLabel: (agentId: string, label: string) => + handleDeletePinByLabel(deps, agentId, label), + + listPins: (agentId: string) => handleListPins(deps, agentId), renameSession: (agentId: string, name: string) => handleRenameSession(deps, agentId, name), @@ -730,5 +807,8 @@ export function createMcpHandlers(deps: CreateMcpHandlersDeps) { listMedia: (agentId: string, opts: { source?: string }) => handleListMedia(deps, agentId, opts), + + deleteMedia: (agentId: string, fileName: string) => + handleDeleteMedia(deps, agentId, fileName), }; } diff --git a/apps/server/src/shared/mcp/agent-lifecycle-tools.ts b/apps/server/src/shared/mcp/agent-lifecycle-tools.ts index ddc37cf2..158b42a8 100644 --- a/apps/server/src/shared/mcp/agent-lifecycle-tools.ts +++ b/apps/server/src/shared/mcp/agent-lifecycle-tools.ts @@ -34,6 +34,12 @@ export type AgentLifecycleContext = { createdAt: string; }> >; + deleteMedia?: (agentId: string, fileName: string) => Promise; + listPins?: ( + agentId: string + ) => Promise< + Array<{ id: string; label: string; value: string; type: string }> + >; }; export function registerAgentLifecycleTools( @@ -217,4 +223,56 @@ export function registerAgentLifecycleTools( } ); } + + if (allowed.has("dispatch_delete_media") && context.deleteMedia) { + const deleteMedia = context.deleteMedia; + server.registerTool( + "dispatch_delete_media", + { + description: + "Permanently remove one of this agent's shared media files. Call dispatch_list_media first to identify the exact fileName. This removes both the stored file and its Dispatch media record.", + inputSchema: { + fileName: z + .string() + .describe("Exact fileName returned by dispatch_list_media."), + }, + }, + async (args) => { + try { + await deleteMedia(agentId, args.fileName); + return { + content: [ + { type: "text", text: `Deleted media \"${args.fileName}\".` }, + ], + }; + } catch (error) { + return toToolError(error); + } + } + ); + } + + if (allowed.has("dispatch_list_pins") && context.listPins) { + const listPins = context.listPins; + server.registerTool( + "dispatch_list_pins", + { + description: + "List this agent's current Dispatch sidebar pins. Use dispatch_delete_pin with a returned id to remove a stale pin.", + inputSchema: {}, + }, + async () => { + try { + const pins = await listPins(agentId); + return { + content: [ + { type: "text" as const, text: JSON.stringify(pins, null, 2) }, + ], + }; + } catch (error) { + return toToolError(error); + } + } + ); + } } diff --git a/apps/server/src/shared/mcp/server.ts b/apps/server/src/shared/mcp/server.ts index 4d94a559..85bf64e5 100644 --- a/apps/server/src/shared/mcp/server.ts +++ b/apps/server/src/shared/mcp/server.ts @@ -52,8 +52,11 @@ const AGENT_TOOLS = new Set([ "dispatch_rename_session", "dispatch_notify", "dispatch_pin", + "dispatch_delete_pin", "dispatch_share", "dispatch_list_media", + "dispatch_delete_media", + "dispatch_list_pins", "list_personas", "dispatch_launch_persona", "dispatch_review_list_feedback", @@ -97,8 +100,11 @@ const JOB_TOOLS = new Set([ "dispatch_rename_session", "dispatch_notify", "dispatch_pin", + "dispatch_delete_pin", "dispatch_share", "dispatch_list_media", + "dispatch_delete_media", + "dispatch_list_pins", "dispatch_launch_persona", "dispatch_review_list_feedback", "dispatch_review_resolve", @@ -142,7 +148,11 @@ const JOB_TOOLS = new Set([ const REVIEW_AGENT_TOOLS = new Set([ "dispatch_event", "dispatch_pin", + "dispatch_delete_pin", "dispatch_share", + "dispatch_list_media", + "dispatch_delete_media", + "dispatch_list_pins", "dispatch_review_submit", "dispatch_review_add_feedback", "dispatch_review_list_feedback", @@ -159,7 +169,7 @@ const TOOL_SETS: Record> = { }; export type ParentContextResult = { - pins: Array<{ label: string; value: string; type: string }>; + pins: Array<{ id?: string; label: string; value: string; type: string }>; media: Array<{ fileName: string; filePath: string; @@ -218,6 +228,12 @@ export type McpRequestContext = { createdAt: string; }> >; + deleteMedia?: (agentId: string, fileName: string) => Promise; + listPins?: ( + agentId: string + ) => Promise< + Array<{ id: string; label: string; value: string; type: string }> + >; listPersonas?: ( agentCwd: string ) => Promise>; @@ -334,7 +350,8 @@ export type McpRequestContext = { agentId: string, pin: { label: string; value: string; type: string } ) => Promise; - deletePin?: (agentId: string, label: string) => Promise; + deletePin?: (agentId: string, pinId: string) => Promise; + deletePinByLabel?: (agentId: string, label: string) => Promise; getParentContext?: (parentAgentId: string) => Promise; sendMessage?: ( agentId: string, @@ -437,10 +454,14 @@ async function createDispatchMcpServer( renameSession: context.renameSession, sendNotify: context.sendNotify, listMedia: context.listMedia, + deleteMedia: context.deleteMedia, + listPins: context.listPins, }); } if (allowed.has("dispatch_pin")) registerPinTool(server, context); + if (allowed.has("dispatch_delete_pin")) + registerDeletePinTool(server, context); if (allowed.has("dispatch_share")) registerShareTool(server, context); // ── Persona launch and unified review tools ─────────────────────── if (context.agent) { @@ -558,16 +579,16 @@ async function createDispatchMcpServer( // ── Shared tool registrations (used by both persona and standard agents) ── function registerPinTool(server: McpServer, context: McpRequestContext): void { - if (!context.agent || !context.upsertPin || !context.deletePin) return; + if (!context.agent || !context.upsertPin) return; const agentId = context.agent.id; const upsertPin = context.upsertPin; - const deletePin = context.deletePin; + const deletePinByLabel = context.deletePinByLabel; server.registerTool( "dispatch_pin", { description: - "Pin a key-value pair to the Dispatch UI for this agent. Pins are displayed in the sidebar so users can quickly find important info. To update a pin, set it again with the same label. To remove a pin, pass delete: true. " + + "Pin a key-value pair to the Dispatch UI for this agent. Pins are displayed in the sidebar so users can quickly find important info. To update a pin, set it again with the same label. To remove a pin, use dispatch_list_pins followed by dispatch_delete_pin. The delete parameter is retained temporarily only for agents that initialized before this tool upgrade. " + "Good things to pin: dev server URLs (url), PR links (pr), key files changed (filename), test/build result summaries (string), DB migration names (string), relevant doc or issue links (url), architecture decisions or assumptions (string), short structured summaries (markdown), the specific blocking question when in waiting_user state (string).", inputSchema: { label: z @@ -580,7 +601,7 @@ function registerPinTool(server: McpServer, context: McpRequestContext): void { .string() .max(2000) .optional() - .describe("The value to display. Required unless delete is true."), + .describe("The value to display."), type: z .enum(["string", "url", "port", "code", "pr", "filename", "markdown"]) .default("string") @@ -589,21 +610,26 @@ function registerPinTool(server: McpServer, context: McpRequestContext): void { ), delete: z .boolean() - .default(false) - .describe("Set to true to remove the pin with this label."), + .optional() + .describe("Deprecated compatibility option for deleting by label."), }, }, async (args) => { try { if (args.delete) { - await deletePin(agentId, args.label); + if (!deletePinByLabel) { + return toToolError( + new Error("Legacy pin deletion is unavailable.") + ); + } + await deletePinByLabel(agentId, args.label); return { - content: [{ type: "text", text: `Removed pin "${args.label}".` }], + content: [{ type: "text", text: `Removed pin \"${args.label}\".` }], }; } - if (!args.value) { + if (args.value === undefined) { return toToolError( - new Error("value is required when not deleting a pin.") + new Error("value is required when creating or updating a pin.") ); } await upsertPin(agentId, { @@ -623,6 +649,36 @@ function registerPinTool(server: McpServer, context: McpRequestContext): void { ); } +function registerDeletePinTool( + server: McpServer, + context: McpRequestContext +): void { + if (!context.agent || !context.deletePin) return; + const agentId = context.agent.id; + const deletePin = context.deletePin; + server.registerTool( + "dispatch_delete_pin", + { + description: + "Permanently remove one current sidebar pin by its stable ID. Call dispatch_list_pins first and pass the exact returned id.", + inputSchema: { + id: z + .string() + .min(1) + .describe("Exact pin id returned by dispatch_list_pins."), + }, + }, + async (args) => { + try { + await deletePin(agentId, args.id); + return { content: [{ type: "text", text: `Removed pin ${args.id}.` }] }; + } catch (error) { + return toToolError(error); + } + } + ); +} + function registerShareTool( server: McpServer, context: McpRequestContext diff --git a/apps/server/test/agent-lifecycle-tools.test.ts b/apps/server/test/agent-lifecycle-tools.test.ts index b9f1c5f7..76bdc1ba 100644 --- a/apps/server/test/agent-lifecycle-tools.test.ts +++ b/apps/server/test/agent-lifecycle-tools.test.ts @@ -36,6 +36,8 @@ function baseContext(): AgentLifecycleContext { renameSession: vi.fn(async () => ({ id: AGENT_ID, name: "New Name" })), sendNotify: vi.fn(async () => ({ sent: true })), listMedia: vi.fn(async () => []), + deleteMedia: vi.fn(async () => {}), + listPins: vi.fn(async () => []), }; } @@ -49,12 +51,14 @@ describe("registerAgentLifecycleTools", () => { // ── Conditional registration ──────────────────────────────────── describe("conditional registration", () => { - it("registers all four tools when all are allowed and context is complete", () => { + it("registers all lifecycle tools when all are allowed and context is complete", () => { const allowed = new Set([ "dispatch_event", "dispatch_rename_session", "dispatch_notify", "dispatch_list_media", + "dispatch_delete_media", + "dispatch_list_pins", ]); registerAgentLifecycleTools(server as never, allowed, baseContext()); @@ -64,6 +68,8 @@ describe("registerAgentLifecycleTools", () => { "dispatch_rename_session", "dispatch_notify", "dispatch_list_media", + "dispatch_delete_media", + "dispatch_list_pins", ]); }); @@ -116,6 +122,28 @@ describe("registerAgentLifecycleTools", () => { expect(server.tools).toHaveLength(0); }); + it("skips dispatch_delete_media when deleteMedia is missing", () => { + const ctx = baseContext(); + delete ctx.deleteMedia; + registerAgentLifecycleTools( + server as never, + new Set(["dispatch_delete_media"]), + ctx + ); + expect(server.tools).toHaveLength(0); + }); + + it("skips dispatch_list_pins when listPins is missing", () => { + const ctx = baseContext(); + delete ctx.listPins; + registerAgentLifecycleTools( + server as never, + new Set(["dispatch_list_pins"]), + ctx + ); + expect(server.tools).toHaveLength(0); + }); + it("only registers tools that are in the allowed set", () => { registerAgentLifecycleTools( server as never, @@ -389,4 +417,47 @@ describe("registerAgentLifecycleTools", () => { }); }); }); + + describe("dispatch_delete_media handler", () => { + it("deletes the named media file", async () => { + const ctx = baseContext(); + ctx.deleteMedia = vi.fn(async () => {}); + registerAgentLifecycleTools( + server as never, + new Set(["dispatch_delete_media"]), + ctx + ); + + const result = await server.tools[0]!.handler({ + fileName: "screenshot.png", + }); + + expect(ctx.deleteMedia).toHaveBeenCalledWith(AGENT_ID, "screenshot.png"); + expect(result).toEqual({ + content: [{ type: "text", text: 'Deleted media "screenshot.png".' }], + }); + }); + }); + + describe("dispatch_list_pins handler", () => { + it("returns the current pins", async () => { + const pins = [ + { label: "Dev", value: "http://localhost:5173", type: "url" }, + ]; + const ctx = baseContext(); + ctx.listPins = vi.fn(async () => pins); + registerAgentLifecycleTools( + server as never, + new Set(["dispatch_list_pins"]), + ctx + ); + + const result = await server.tools[0]!.handler({}); + + expect(ctx.listPins).toHaveBeenCalledWith(AGENT_ID); + expect(result).toEqual({ + content: [{ type: "text", text: JSON.stringify(pins, null, 2) }], + }); + }); + }); }); diff --git a/apps/server/test/db/agent-manager.test.ts b/apps/server/test/db/agent-manager.test.ts index 605831a9..9cb2cf44 100644 --- a/apps/server/test/db/agent-manager.test.ts +++ b/apps/server/test/db/agent-manager.test.ts @@ -2371,8 +2371,8 @@ describe("AgentManager", () => { }); }); - describe("deletePin", () => { - it("should remove a pin by label (case-insensitive)", async () => { + describe("deletePinById", () => { + it("should remove a pin by stable ID", async () => { const agent = await manager.createAgent({ cwd: "/tmp", useWorktree: false, @@ -2384,11 +2384,12 @@ describe("AgentManager", () => { value: "https://example.com", }); - const updated = await manager.deletePin(agent.id, "url"); + const pinId = (await manager.getAgent(agent.id))!.pins[0]!.id!; + const updated = await manager.deletePinById(agent.id, pinId); expect(updated.pins).toHaveLength(0); }); - it("should be a no-op when the label does not exist", async () => { + it("should throw when the pin ID does not exist", async () => { const agent = await manager.createAgent({ cwd: "/tmp", useWorktree: false, @@ -2400,13 +2401,13 @@ describe("AgentManager", () => { value: "v", }); - const updated = await manager.deletePin(agent.id, "nope"); - expect(updated.pins).toHaveLength(1); - expect(updated.pins![0]!.label).toBe("Keep"); + await expect(manager.deletePinById(agent.id, "nope")).rejects.toThrow( + /pin not found/i + ); }); it("should throw 404 for non-existent agent", async () => { - await expect(manager.deletePin("agt_nope", "X")).rejects.toThrow( + await expect(manager.deletePinById("agt_nope", "X")).rejects.toThrow( /not found/i ); }); diff --git a/apps/server/test/db/pin-ids-migration.test.ts b/apps/server/test/db/pin-ids-migration.test.ts new file mode 100644 index 00000000..0f171d58 --- /dev/null +++ b/apps/server/test/db/pin-ids-migration.test.ts @@ -0,0 +1,85 @@ +import { readdirSync } from "node:fs"; +import path from "node:path"; +import { fileURLToPath } from "node:url"; +import { afterAll, beforeAll, describe, expect, it } from "vitest"; +import type { Pool } from "pg"; + +import { runMigrations } from "../../src/db/migrate.js"; +import { getTestDatabaseUrl, setupTestDb, teardownTestDb } from "./setup.js"; + +const migrationsDir = path.resolve( + path.dirname(fileURLToPath(import.meta.url)), + "../../src/db/migrations" +); +const pinIdsMigration = "0036_pin-ids.sql"; +const pinIdsMigrationIndex = readdirSync(migrationsDir) + .filter((file) => file.endsWith(".sql")) + .sort() + .indexOf(pinIdsMigration); + +let pool: Pool; + +beforeAll(async () => { + pool = await setupTestDb(); + await runMigrations({ + databaseUrl: getTestDatabaseUrl(), + count: pinIdsMigrationIndex, + }); +}); + +afterAll(async () => { + await teardownTestDb(); +}); + +describe("0036_pin-ids upgrade", () => { + it("normalizes malformed legacy pins and assigns usable stable IDs", async () => { + await pool.query(` + INSERT INTO agents (id, name, status, cwd, pins) VALUES + ('pin-valid', 'valid', 'stopped', '/tmp', + '[{"label":"URL","value":"https://example.com","type":"url"}, {"label":"","value":"legacy-empty-label","type":"string"}]'), + ('pin-malformed', 'malformed', 'stopped', '/tmp', '{"label":"not-an-array"}'), + ('pin-mixed', 'mixed', 'stopped', '/tmp', + '[null, "bad", {"label":"Missing value","type":"url"}, {"label":"Good","value":"v","type":"string"}]'), + ('pin-duplicates', 'duplicates', 'stopped', '/tmp', + '[{"id":"same","label":"One","value":"1","type":"string"}, {"id":"same","label":"Two","value":"2","type":"string"}, {"id":"","label":"Three","value":"3","type":"string"}]') + `); + + await runMigrations(getTestDatabaseUrl()); + + const rows = await pool.query<{ + id: string; + pins: Array>; + }>("SELECT id, pins FROM agents WHERE id LIKE 'pin-%' ORDER BY id"); + const byId = new Map(rows.rows.map((row) => [row.id, row.pins])); + + expect(byId.get("pin-malformed")).toEqual([]); + expect(byId.get("pin-valid")).toEqual( + expect.arrayContaining([ + expect.objectContaining({ + label: "", + value: "legacy-empty-label", + id: expect.any(String), + }), + ]) + ); + expect(byId.get("pin-mixed")).toEqual([ + expect.objectContaining({ + label: "Good", + value: "v", + type: "string", + id: expect.any(String), + }), + ]); + + for (const pin of [ + ...(byId.get("pin-valid") ?? []), + ...(byId.get("pin-duplicates") ?? []), + ]) { + expect(pin.id).toMatch(/^\S+$/); + } + const duplicateIds = (byId.get("pin-duplicates") ?? []).map( + (pin) => pin.id + ); + expect(new Set(duplicateIds).size).toBe(duplicateIds.length); + }); +}); diff --git a/apps/server/test/db/upgrade.test.ts b/apps/server/test/db/upgrade.test.ts index 9a63b75e..cbe9ca0d 100644 --- a/apps/server/test/db/upgrade.test.ts +++ b/apps/server/test/db/upgrade.test.ts @@ -193,7 +193,12 @@ describe.skipIf(!hasMigrationsToTest)( expect(agent1.full_access).toBe(true); expect(agent1.codex_args).toEqual(["--model", "opus"]); expect(agent1.pins).toEqual([ - { label: "API", value: "http://localhost:3000", type: "url" }, + expect.objectContaining({ + id: expect.any(String), + label: "API", + value: "http://localhost:3000", + type: "url", + }), ]); const agent2 = agents.rows[1]; diff --git a/apps/server/test/mcp-handlers.test.ts b/apps/server/test/mcp-handlers.test.ts index 2f8fa6d1..0de8e2ee 100644 --- a/apps/server/test/mcp-handlers.test.ts +++ b/apps/server/test/mcp-handlers.test.ts @@ -111,6 +111,7 @@ vi.mock("node:fs/promises", () => ({ readFile: vi.fn(async () => Buffer.from("file-content")), writeFile: vi.fn(async () => {}), mkdir: vi.fn(async () => {}), + unlink: vi.fn(async () => {}), })); import { @@ -170,9 +171,16 @@ function createMockDeps() { upsertPin: vi.fn(async (id: string) => ({ id, name: "test-agent", - pins: [{ label: "URL", value: "http://localhost", type: "url" }], + pins: [ + { + id: "pin_url", + label: "URL", + value: "http://localhost", + type: "url", + }, + ], })), - deletePin: vi.fn(async (id: string) => ({ + deletePinById: vi.fn(async (id: string) => ({ id, name: "test-agent", pins: [], @@ -354,10 +362,10 @@ describe("createMcpHandlers", () => { describe("deletePin", () => { it("deletes pin and publishes event", async () => { - await handlers.deletePin("agt_test1", "URL"); - expect(deps.agentManager.deletePin).toHaveBeenCalledWith( + await handlers.deletePin("agt_test1", "pin_123"); + expect(deps.agentManager.deletePinById).toHaveBeenCalledWith( "agt_test1", - "URL" + "pin_123" ); expect(deps.publishUiEvent).toHaveBeenCalledWith( expect.objectContaining({ type: "agent.upsert" }) @@ -365,6 +373,52 @@ describe("createMcpHandlers", () => { }); }); + describe("listPins", () => { + it("returns the current agent pins", async () => { + deps.agentManager.getAgent.mockResolvedValue({ + id: "agt_test1", + pins: [ + { + id: "pin_url", + label: "URL", + value: "http://localhost", + type: "url", + }, + ], + }); + + await expect(handlers.listPins("agt_test1")).resolves.toEqual([ + { + id: "pin_url", + label: "URL", + value: "http://localhost", + type: "url", + }, + ]); + }); + }); + + describe("deleteMedia", () => { + it("removes the file, media record, seen records, and publishes an update", async () => { + deps.pool.query.mockResolvedValueOnce({ + rows: [{ file_name: "shot.png" }], + }); + await handlers.deleteMedia("agt_test1", "shot.png"); + + const { unlink } = await import("node:fs/promises"); + expect(unlink).toHaveBeenCalledWith("/tmp/media/agt_test1/shot.png"); + expect(deps.pool.query).toHaveBeenNthCalledWith( + 2, + "DELETE FROM media WHERE agent_id = $1 AND file_name = $2", + ["agt_test1", "shot.png"] + ); + expect(deps.publishUiEvent).toHaveBeenCalledWith({ + type: "media.changed", + agentId: "agt_test1", + }); + }); + }); + describe("getParentContext", () => { it("returns pins and media for parent agent", async () => { deps.agentManager.getAgent.mockResolvedValue({ diff --git a/apps/web/src/components/app/docs-sections/media.tsx b/apps/web/src/components/app/docs-sections/media.tsx index e37498f9..a460be5c 100644 --- a/apps/web/src/components/app/docs-sections/media.tsx +++ b/apps/web/src/components/app/docs-sections/media.tsx @@ -71,6 +71,12 @@ export function MediaContent() { "user", "screenshot", "text",{" "} "simulator", or "stream".

+

+ To remove an item that is no longer relevant, call{" "} + dispatch_delete_media with its exact{" "} + fileName from the listing. This permanently removes the + file and its media entry. +

diff --git a/apps/web/src/components/app/docs-sections/tools.tsx b/apps/web/src/components/app/docs-sections/tools.tsx index f2c1add3..0610dc07 100644 --- a/apps/web/src/components/app/docs-sections/tools.tsx +++ b/apps/web/src/components/app/docs-sections/tools.tsx @@ -141,6 +141,18 @@ export function ToolsContent() { dispatch_list_media — list media shared with or by the current agent +
  • + dispatch_delete_media — permanently remove a shared + media file by its listed file name +
    • +
  • + dispatch_list_pins — list the current sidebar pins so + stale pins can be removed by ID +
    • +
  • + dispatch_delete_pin — permanently remove a pin using + its stable ID from dispatch_list_pins +
  • list_personas — list persona reviewers defined for the current repo diff --git a/e2e/agent-crud.spec.ts b/e2e/agent-crud.spec.ts index cb0b70a2..cb4c2608 100644 --- a/e2e/agent-crud.spec.ts +++ b/e2e/agent-crud.spec.ts @@ -229,15 +229,16 @@ test.describe("Agent CRUD", () => { const { agent } = (await res.json()) as { agent: { id: string; - pins: Array<{ label: string; value: string; type: string }>; + pins: Array<{ id: string; label: string; value: string; type: string }>; }; }; expect(agent.pins).toEqual([ - { + expect.objectContaining({ + id: expect.any(String), label: "example.com", value: "https://example.com/task", type: "url", - }, + }), ]); });