From b52f2af44c4dfdb554268f8899c6238040034860 Mon Sep 17 00:00:00 2001 From: Muhammad Ahmed Cheema Date: Mon, 13 Apr 2026 21:56:57 -0700 Subject: [PATCH 1/6] feat: emit plugin init snapshot from runtime to dashboard sse When the Claude Agent SDK returns an init system message we now publish a typed plugin_init_snapshot event on the existing /ui/api/events bus so the dashboard plugins tab can flip optimistically-installing cards to their real settled state. The helper is wrapped in try/catch inside the agent main loop so a telemetry bug can never abort message processing. Dashboard-side, dashboard.js opens an EventSource on mount, registers a plugin_init_snapshot listener, and dispatches to a new PhantomPluginsModule.onInitSnapshot hook on plugins.js that walks the rendered card grid and flips matching keys. --- public/dashboard/dashboard.js | 25 +++++ public/dashboard/plugins.js | 34 +++++++ .../__tests__/init-plugin-snapshot.test.ts | 94 +++++++++++++++++++ src/agent/init-plugin-snapshot.ts | 39 ++++++++ src/agent/runtime.ts | 6 ++ 5 files changed, 198 insertions(+) create mode 100644 src/agent/__tests__/init-plugin-snapshot.test.ts create mode 100644 src/agent/init-plugin-snapshot.ts diff --git a/public/dashboard/dashboard.js b/public/dashboard/dashboard.js index e437edb..e1edd66 100644 --- a/public/dashboard/dashboard.js +++ b/public/dashboard/dashboard.js @@ -304,9 +304,34 @@ if (el) el.textContent = new Date().toISOString().split("T")[0]; } + // Server-sent events for live dashboard updates. PR3 adds a + // "plugin_init_snapshot" event that fires when the agent sees the SDK init + // message and resolves the enabled-plugin set. The plugins module flips + // optimistically-installed cards to their real state. + function openEventStream() { + if (!window.EventSource) return null; + try { + var es = new EventSource("/ui/api/events"); + es.addEventListener("plugin_init_snapshot", function (e) { + try { + var data = JSON.parse(e.data); + if (window.PhantomPluginsModule && typeof window.PhantomPluginsModule.onInitSnapshot === "function") { + window.PhantomPluginsModule.onInitSnapshot(data); + } + } catch (_) { + // SSE payload was malformed; nothing useful to show the user. + } + }); + return es; + } catch (_) { + return null; + } + } + function init() { setNavDate(); initThemeToggle(); + openEventStream(); window.addEventListener("beforeunload", function (e) { if (anyDirty()) { diff --git a/public/dashboard/plugins.js b/public/dashboard/plugins.js index 0a515f5..05faba4 100644 --- a/public/dashboard/plugins.js +++ b/public/dashboard/plugins.js @@ -506,5 +506,39 @@ return loadCatalog(false); } + // Live install snapshot hook. Called by dashboard.js whenever the agent's + // SDK init message reports the enabled-plugin set for a freshly-kicked + // query. We flip any card whose key is in the snapshot (and is not already + // marked installed) to "installed" and surface a gentle toast. + function onInitSnapshot(payload) { + if (!payload || !Array.isArray(payload.keys) || payload.keys.length === 0) return; + if (!state.catalog || !Array.isArray(state.catalog.plugins)) return; + var seen = {}; + for (var i = 0; i < payload.keys.length; i++) { + seen[payload.keys[i]] = true; + } + var flipped = []; + state.catalog.plugins.forEach(function (p) { + if (seen[p.key] && !p.enabled) { + p.enabled = true; + flipped.push(p.name || p.key); + } + }); + if (flipped.length > 0) { + // Re-render if we are currently mounted (root has content). + if (root && root.getAttribute("data-active") === "true") { + render(); + } + if (ctx && ctx.toast) { + if (flipped.length === 1) { + ctx.toast("success", "Plugin live", flipped[0] + " is now active."); + } else { + ctx.toast("success", "Plugins live", flipped.length + " plugins are now active."); + } + } + } + } + window.PhantomDashboard.registerRoute("plugins", { mount: mount }); + window.PhantomPluginsModule = { onInitSnapshot: onInitSnapshot }; })(); diff --git a/src/agent/__tests__/init-plugin-snapshot.test.ts b/src/agent/__tests__/init-plugin-snapshot.test.ts new file mode 100644 index 0000000..ac69bfe --- /dev/null +++ b/src/agent/__tests__/init-plugin-snapshot.test.ts @@ -0,0 +1,94 @@ +import { describe, expect, test } from "bun:test"; +import * as eventsModule from "../../ui/events.ts"; +import { emitPluginInitSnapshot, extractPluginKeys } from "../init-plugin-snapshot.ts"; + +describe("extractPluginKeys", () => { + test("returns empty array on null", () => { + expect(extractPluginKeys(null)).toEqual([]); + }); + + test("returns empty array on undefined", () => { + expect(extractPluginKeys(undefined)).toEqual([]); + }); + + test("returns empty array when plugins field missing", () => { + expect(extractPluginKeys({})).toEqual([]); + }); + + test("returns empty array when plugins is not an array", () => { + expect( + extractPluginKeys({ plugins: "not-an-array" } as unknown as Parameters[0]), + ).toEqual([]); + }); + + test("extracts names from valid plugin entries", () => { + const result = extractPluginKeys({ + plugins: [{ name: "linear@claude-plugins-official" }, { name: "notion@claude-plugins-official" }], + }); + expect(result).toEqual(["linear@claude-plugins-official", "notion@claude-plugins-official"]); + }); + + test("filters out entries without a string name", () => { + const result = extractPluginKeys({ + plugins: [{ name: "linear" }, null, { name: "" }, {}, { name: 42 as unknown as string }, { name: "notion" }], + }); + expect(result).toEqual(["linear", "notion"]); + }); +}); + +describe("emitPluginInitSnapshot", () => { + test("calls publish with extracted keys on a well-formed init message", () => { + const received: Array<{ event: string; data: unknown }> = []; + const unsub = eventsModule.subscribe((event, data) => received.push({ event, data })); + try { + emitPluginInitSnapshot({ + plugins: [{ name: "linear@claude-plugins-official" }, { name: "slack@claude-plugins-official" }], + }); + } finally { + unsub(); + } + expect(received.length).toBe(1); + expect(received[0].event).toBe("plugin_init_snapshot"); + expect(received[0].data).toEqual({ + keys: ["linear@claude-plugins-official", "slack@claude-plugins-official"], + }); + }); + + test("publishes empty keys when plugins missing", () => { + const received: Array<{ event: string; data: unknown }> = []; + const unsub = eventsModule.subscribe((event, data) => received.push({ event, data })); + try { + emitPluginInitSnapshot({}); + } finally { + unsub(); + } + expect(received.length).toBe(1); + expect(received[0].data).toEqual({ keys: [] }); + }); + + test("publishes empty keys on null input", () => { + const received: Array<{ event: string; data: unknown }> = []; + const unsub = eventsModule.subscribe((event, data) => received.push({ event, data })); + try { + emitPluginInitSnapshot(null); + } finally { + unsub(); + } + expect(received.length).toBe(1); + expect(received[0].data).toEqual({ keys: [] }); + }); + + test("does not throw when a subscriber throws; isolates via existing publish fallback", () => { + // publish() already wraps each listener in try/catch (events.ts:12-17), + // so a throwing subscriber does not reach emitPluginInitSnapshot's + // try/catch. This test confirms no propagation either way. + const unsub = eventsModule.subscribe(() => { + throw new Error("subscriber-boom"); + }); + try { + expect(() => emitPluginInitSnapshot({ plugins: [{ name: "x" }] })).not.toThrow(); + } finally { + unsub(); + } + }); +}); diff --git a/src/agent/init-plugin-snapshot.ts b/src/agent/init-plugin-snapshot.ts new file mode 100644 index 0000000..1b32f7d --- /dev/null +++ b/src/agent/init-plugin-snapshot.ts @@ -0,0 +1,39 @@ +// Pure helper extracted from runtime.ts so the init-plugin-snapshot path can be +// unit tested without spinning up the full agent main loop. Given an SDK init +// system message, extract a clean list of plugin keys and publish them to the +// dashboard SSE bus. Any failure is logged and swallowed: a telemetry bug must +// never propagate into the agent main loop. + +import { publish as publishDashboardEvent } from "../ui/events.ts"; + +export type InitMessageLike = + | { + plugins?: Array<{ name?: unknown } | null | undefined>; + } + | null + | undefined; + +export function extractPluginKeys(message: InitMessageLike): string[] { + if (!message || typeof message !== "object") return []; + const plugins = (message as { plugins?: unknown }).plugins; + if (!Array.isArray(plugins)) return []; + const keys: string[] = []; + for (const entry of plugins) { + if (!entry || typeof entry !== "object") continue; + const name = (entry as { name?: unknown }).name; + if (typeof name === "string" && name.length > 0) { + keys.push(name); + } + } + return keys; +} + +export function emitPluginInitSnapshot(message: InitMessageLike): void { + try { + const keys = extractPluginKeys(message); + publishDashboardEvent("plugin_init_snapshot", { keys }); + } catch (err: unknown) { + const msg = err instanceof Error ? err.message : String(err); + console.warn(`[runtime] failed to emit plugin_init_snapshot: ${msg}`); + } +} diff --git a/src/agent/runtime.ts b/src/agent/runtime.ts index ed50515..e33d32f 100644 --- a/src/agent/runtime.ts +++ b/src/agent/runtime.ts @@ -9,6 +9,7 @@ import type { RoleTemplate } from "../roles/types.ts"; import { CostTracker } from "./cost-tracker.ts"; import { type AgentCost, type AgentResponse, emptyCost } from "./events.ts"; import { createDangerousCommandBlocker, createFileTracker } from "./hooks.ts"; +import { emitPluginInitSnapshot } from "./init-plugin-snapshot.ts"; import { type JudgeQueryOptions, type JudgeQueryResult, runJudgeQuery } from "./judge-query.ts"; import { extractCost, extractTextFromMessage } from "./message-utils.ts"; import { assemblePrompt } from "./prompt-assembler.ts"; @@ -225,6 +226,11 @@ export class AgentRuntime { sdkSessionId = message.session_id; this.sessionStore.updateSdkSessionId(sessionKey, sdkSessionId); onEvent?.({ type: "init", sessionId: sdkSessionId }); + // Emit the init-resolved plugin snapshot to the dashboard SSE bus so + // plugin cards optimistically flipped to "installing..." can settle + // to "installed". The helper is wrapped so a telemetry failure never + // propagates into the agent main loop. + emitPluginInitSnapshot(message as unknown as Parameters[0]); } break; } From 73d8a714d296357bc3f02457e0356ea9e1cd0b6f Mon Sep 17 00:00:00 2001 From: Muhammad Ahmed Cheema Date: Mon, 13 Apr 2026 22:00:41 -0700 Subject: [PATCH 2/6] feat: add phantom_list_sessions and phantom_memory_search mcp aliases PR3 follow-up on the PR2 rename question. Both old names and new names register on the external MCP server. The originals phantom_history and phantom_memory_query keep their current schemas unchanged so any external client that already adopted them continues to work. The new aliases accept a richer parameter set: - phantom_list_sessions adds optional channel and days_back filters - phantom_memory_search adds optional days_back recency filter applied client-side via started_at timestamps so the underlying memory recall APIs stay untouched Both old and new names route to shared core helpers so future fixes land in one place. Tool count on the SWE server moves from 17 to 19. --- src/mcp/__tests__/tools-swe.test.ts | 9 +- src/mcp/__tests__/tools-universal.test.ts | 256 ++++++++++++++++++++++ src/mcp/tools-universal.ts | 166 +++++++++++--- 3 files changed, 397 insertions(+), 34 deletions(-) create mode 100644 src/mcp/__tests__/tools-universal.test.ts diff --git a/src/mcp/__tests__/tools-swe.test.ts b/src/mcp/__tests__/tools-swe.test.ts index 00474de..aa9f424 100644 --- a/src/mcp/__tests__/tools-swe.test.ts +++ b/src/mcp/__tests__/tools-swe.test.ts @@ -184,14 +184,19 @@ describe("SWE MCP Tools", () => { expect(toolNames).toContain("phantom_repo_info"); }); - test("total tool count is 17 (8 universal + 6 SWE + 3 dynamic management)", async () => { + test("total tool count is 19 (10 universal + 6 SWE + 3 dynamic management)", async () => { + // PR3 adds phantom_list_sessions and phantom_memory_search as new tool + // aliases on the universal server, alongside the original phantom_history + // and phantom_memory_query registrations. The alias pair keeps existing + // external clients working while exposing richer parameter sets to new + // ones; count therefore grows from 17 to 19. const sessionId = await initSession(mcpServer, adminToken); const res = await mcpServer.handleRequest( mcpRequest(adminToken, { jsonrpc: "2.0", id: 11, method: "tools/list" }, sessionId), ); const body = (await res.json()) as Record; const result = body.result as { tools: Array<{ name: string }> }; - expect(result.tools).toHaveLength(17); + expect(result.tools).toHaveLength(19); }); test("phantom_codebase_query returns domain knowledge", async () => { diff --git a/src/mcp/__tests__/tools-universal.test.ts b/src/mcp/__tests__/tools-universal.test.ts new file mode 100644 index 0000000..0487d43 --- /dev/null +++ b/src/mcp/__tests__/tools-universal.test.ts @@ -0,0 +1,256 @@ +// Unit tests for the universal MCP tool registrations, with a focus on the +// PR3 alias expansion: phantom_list_sessions and phantom_memory_search are new +// names that coexist with the original phantom_history and phantom_memory_query +// registrations. The old names are untouched so existing external clients do +// not break; the new names expose a richer parameter set. +// +// Instead of spinning up the full PhantomMcpServer, we mock the McpServer +// interface by capturing every registerTool call into a map and invoking the +// captured handlers directly with the same shape as the real SDK. + +import { Database } from "bun:sqlite"; +import { beforeEach, describe, expect, test } from "bun:test"; +import type { McpServer } from "@modelcontextprotocol/sdk/server/mcp.js"; +import { runMigrations } from "../../db/migrate.ts"; +import type { Episode } from "../../memory/types.ts"; +import { type ToolDependencies, registerUniversalTools } from "../tools-universal.ts"; + +type CapturedTool = { + name: string; + schema: unknown; + handler: (input: unknown) => Promise | unknown; +}; + +function createMockServer(): { server: McpServer; tools: Map } { + const tools = new Map(); + const server = { + registerTool: (name: string, schema: unknown, handler: CapturedTool["handler"]) => { + tools.set(name, { name, schema, handler }); + return {} as unknown; + }, + } as unknown as McpServer; + return { server, tools }; +} + +function createMockRuntime(): ToolDependencies["runtime"] { + return { + getActiveSessionCount: () => 0, + handleMessage: async () => ({ + text: "ok", + sessionId: "sid", + cost: { totalUsd: 0, inputTokens: 0, outputTokens: 0, modelUsage: {} }, + durationMs: 1, + }), + } as unknown as ToolDependencies["runtime"]; +} + +function createMockMemory(episodes: Episode[]): ToolDependencies["memory"] { + return { + isReady: () => true, + recallEpisodes: async () => episodes, + recallFacts: async () => [], + findProcedure: async () => null, + } as unknown as ToolDependencies["memory"]; +} + +function createEpisode(id: string, startedAt: string): Episode { + return { + id, + type: "interaction", + summary: "s", + detail: "d", + parent_id: null, + session_id: "sess", + user_id: "u", + tools_used: [], + files_touched: [], + outcome: "success", + outcome_detail: "", + lessons: [], + started_at: startedAt, + ended_at: startedAt, + duration_seconds: 1, + importance: 1, + access_count: 0, + last_accessed_at: startedAt, + decay_rate: 0.1, + }; +} + +function seedSessions(db: Database): void { + db.run("DELETE FROM sessions"); + const rows: Array<[string, string, string, string, string]> = [ + ["slack:conv-a", "sid-a", "slack", "conv-a", new Date(Date.now() - 1 * 24 * 60 * 60 * 1000).toISOString()], + ["slack:conv-b", "sid-b", "slack", "conv-b", new Date(Date.now() - 10 * 24 * 60 * 60 * 1000).toISOString()], + ["cli:conv-c", "sid-c", "cli", "conv-c", new Date(Date.now() - 1 * 24 * 60 * 60 * 1000).toISOString()], + ["mcp:conv-d", "sid-d", "mcp", "conv-d", new Date(Date.now() - 40 * 24 * 60 * 60 * 1000).toISOString()], + ]; + for (const [sk, sdk, ch, conv, lastActive] of rows) { + db.run( + `INSERT INTO sessions (session_key, sdk_session_id, channel_id, conversation_id, status, + total_cost_usd, input_tokens, output_tokens, turn_count, created_at, last_active_at) + VALUES (?, ?, ?, ?, 'active', 0, 0, 0, 0, datetime('now'), ?)`, + [sk, sdk, ch, conv, lastActive], + ); + } +} + +let db: Database; +let deps: ToolDependencies; + +beforeEach(() => { + db = new Database(":memory:"); + runMigrations(db); + seedSessions(db); + deps = { + config: {} as unknown as ToolDependencies["config"], + db, + startedAt: Date.now(), + runtime: createMockRuntime(), + memory: createMockMemory([ + createEpisode("recent-1", new Date(Date.now() - 2 * 24 * 60 * 60 * 1000).toISOString()), + createEpisode("recent-2", new Date(Date.now() - 5 * 24 * 60 * 60 * 1000).toISOString()), + createEpisode("old-1", new Date(Date.now() - 30 * 24 * 60 * 60 * 1000).toISOString()), + ]), + evolution: null, + }; +}); + +describe("registerUniversalTools: session listing tools", () => { + test("registers phantom_history with the original schema", () => { + const { server, tools } = createMockServer(); + registerUniversalTools(server, deps); + expect(tools.has("phantom_history")).toBe(true); + }); + + test("registers phantom_list_sessions alongside phantom_history", () => { + const { server, tools } = createMockServer(); + registerUniversalTools(server, deps); + expect(tools.has("phantom_list_sessions")).toBe(true); + expect(tools.has("phantom_history")).toBe(true); + }); + + test("phantom_history returns all sessions with the limit parameter", async () => { + const { server, tools } = createMockServer(); + registerUniversalTools(server, deps); + const tool = tools.get("phantom_history"); + const result = (await tool?.handler({ limit: 10 })) as { content: Array<{ text: string }> }; + const parsed = JSON.parse(result.content[0].text); + expect(parsed.count).toBe(4); + }); + + test("phantom_list_sessions filters by channel", async () => { + const { server, tools } = createMockServer(); + registerUniversalTools(server, deps); + const tool = tools.get("phantom_list_sessions"); + const result = (await tool?.handler({ limit: 10, channel: "slack" })) as { content: Array<{ text: string }> }; + const parsed = JSON.parse(result.content[0].text); + expect(parsed.count).toBe(2); + for (const s of parsed.sessions) { + expect(s.channel_id).toBe("slack"); + } + }); + + test("phantom_list_sessions filters by days_back", async () => { + const { server, tools } = createMockServer(); + registerUniversalTools(server, deps); + const tool = tools.get("phantom_list_sessions"); + const result = (await tool?.handler({ limit: 10, days_back: 7 })) as { content: Array<{ text: string }> }; + const parsed = JSON.parse(result.content[0].text); + // Only the two sessions from ~1 day ago pass the 7-day filter. + expect(parsed.count).toBe(2); + }); + + test("phantom_list_sessions combines channel and days_back", async () => { + const { server, tools } = createMockServer(); + registerUniversalTools(server, deps); + const tool = tools.get("phantom_list_sessions"); + const result = (await tool?.handler({ limit: 10, channel: "slack", days_back: 7 })) as { + content: Array<{ text: string }>; + }; + const parsed = JSON.parse(result.content[0].text); + expect(parsed.count).toBe(1); + expect(parsed.sessions[0].channel_id).toBe("slack"); + }); +}); + +describe("registerUniversalTools: memory search tools", () => { + test("registers both phantom_memory_query and phantom_memory_search", () => { + const { server, tools } = createMockServer(); + registerUniversalTools(server, deps); + expect(tools.has("phantom_memory_query")).toBe(true); + expect(tools.has("phantom_memory_search")).toBe(true); + }); + + test("phantom_memory_query returns all episodes without a recency filter", async () => { + const { server, tools } = createMockServer(); + registerUniversalTools(server, deps); + const tool = tools.get("phantom_memory_query"); + const result = (await tool?.handler({ query: "anything", memory_type: "episodic", limit: 10 })) as { + content: Array<{ text: string }>; + }; + const parsed = JSON.parse(result.content[0].text); + expect(parsed.results.episodes.length).toBe(3); + }); + + test("phantom_memory_search with days_back filters older episodes", async () => { + const { server, tools } = createMockServer(); + registerUniversalTools(server, deps); + const tool = tools.get("phantom_memory_search"); + const result = (await tool?.handler({ + query: "anything", + memory_type: "episodic", + limit: 10, + days_back: 7, + })) as { content: Array<{ text: string }> }; + const parsed = JSON.parse(result.content[0].text); + // The old-1 episode at ~30 days ago is filtered out; recent-1 and recent-2 remain. + expect(parsed.results.episodes.length).toBe(2); + expect(parsed.results.episodes.map((e: { id: string }) => e.id)).toEqual(["recent-1", "recent-2"]); + }); + + test("phantom_memory_search without days_back behaves like phantom_memory_query", async () => { + const { server, tools } = createMockServer(); + registerUniversalTools(server, deps); + const tool = tools.get("phantom_memory_search"); + const result = (await tool?.handler({ query: "anything", memory_type: "episodic", limit: 10 })) as { + content: Array<{ text: string }>; + }; + const parsed = JSON.parse(result.content[0].text); + expect(parsed.results.episodes.length).toBe(3); + }); + + test("phantom_memory_search returns a clean error when memory is unavailable", async () => { + const { server, tools } = createMockServer(); + registerUniversalTools(server, { ...deps, memory: null }); + const tool = tools.get("phantom_memory_search"); + const result = (await tool?.handler({ query: "x", memory_type: "all", limit: 10 })) as { + content: Array<{ text: string }>; + }; + const parsed = JSON.parse(result.content[0].text); + expect(parsed.error).toBe("Memory system not available"); + }); +}); + +describe("registerUniversalTools: full tool count", () => { + test("registers all expected tools including both alias pairs", () => { + const { server, tools } = createMockServer(); + registerUniversalTools(server, deps); + const expected = [ + "phantom_status", + "phantom_config", + "phantom_metrics", + "phantom_history", + "phantom_list_sessions", + "phantom_memory_query", + "phantom_memory_search", + "phantom_ask", + "phantom_task_create", + "phantom_task_status", + ]; + for (const name of expected) { + expect(tools.has(name)).toBe(true); + } + expect(tools.size).toBe(expected.length); + }); +}); diff --git a/src/mcp/tools-universal.ts b/src/mcp/tools-universal.ts index b8eb533..bf0cd6a 100644 --- a/src/mcp/tools-universal.ts +++ b/src/mcp/tools-universal.ts @@ -21,7 +21,9 @@ export function registerUniversalTools(server: McpServer, deps: ToolDependencies registerPhantomConfig(server, deps); registerPhantomMetrics(server, deps); registerPhantomHistory(server, deps); + registerPhantomListSessions(server, deps); registerPhantomMemoryQuery(server, deps); + registerPhantomMemorySearch(server, deps); registerPhantomAsk(server, deps); registerPhantomTaskCreate(server, deps); registerPhantomTaskStatus(server, deps); @@ -170,35 +172,128 @@ function registerPhantomMetrics(server: McpServer, deps: ToolDependencies): void ); } +// Shared session-listing core for phantom_history (original name, backwards +// compatible) and phantom_list_sessions (new name with channel and days_back +// filters). Keeping both names registered is a deliberate non-breaking change +// for external MCP clients that already adopted the old name. +function listRecentSessionsCore( + deps: ToolDependencies, + options: { limit: number; channel?: string; daysBack?: number }, +): CallToolResult { + const wheres: string[] = []; + const params: Array = []; + if (options.channel && options.channel.length > 0) { + wheres.push("channel_id = ?"); + params.push(options.channel); + } + if (options.daysBack != null && options.daysBack > 0) { + wheres.push("last_active_at >= datetime('now', ?)"); + params.push(`-${options.daysBack} days`); + } + const whereSql = wheres.length > 0 ? `WHERE ${wheres.join(" AND ")}` : ""; + params.push(options.limit); + const sessions = deps.db + .query( + `SELECT session_key, sdk_session_id, channel_id, conversation_id, status, + total_cost_usd, input_tokens, output_tokens, turn_count, created_at, last_active_at + FROM sessions ${whereSql} ORDER BY last_active_at DESC LIMIT ?`, + ) + .all(...params); + return { + content: [{ type: "text", text: JSON.stringify({ sessions, count: sessions.length }, null, 2) }], + }; +} + function registerPhantomHistory(server: McpServer, deps: ToolDependencies): void { server.registerTool( "phantom_history", { - description: "Get recent session history with outcomes, costs, and durations.", + description: + "Get recent session history with outcomes, costs, and durations. Prefer phantom_list_sessions for new integrations: it accepts channel and days_back filters. phantom_history stays registered so existing clients do not break.", inputSchema: z.object({ limit: z.number().int().min(1).max(100).optional().default(10).describe("Number of sessions to return"), }), }, - async ({ limit }): Promise => { - const sessions = deps.db - .query( - `SELECT session_key, sdk_session_id, channel_id, conversation_id, status, - total_cost_usd, input_tokens, output_tokens, turn_count, created_at, last_active_at - FROM sessions ORDER BY last_active_at DESC LIMIT ?`, - ) - .all(limit); + async ({ limit }): Promise => listRecentSessionsCore(deps, { limit }), + ); +} - return { content: [{ type: "text", text: JSON.stringify({ sessions, count: sessions.length }, null, 2) }] }; +function registerPhantomListSessions(server: McpServer, deps: ToolDependencies): void { + server.registerTool( + "phantom_list_sessions", + { + description: + "List recent agent sessions with metadata. Supports optional channel filter (e.g. 'slack', 'telegram', 'cli', 'scheduler', 'mcp', 'trigger', 'webhook') and a days_back window. Aliases phantom_history with a richer parameter set; existing clients of phantom_history are unaffected.", + inputSchema: z.object({ + limit: z.number().int().min(1).max(100).optional().default(10).describe("Number of sessions to return"), + channel: z.string().optional().describe("Filter by channel_id exactly"), + days_back: z + .number() + .int() + .min(1) + .max(365) + .optional() + .describe("Only return sessions active within the last N days"), + }), }, + async ({ limit, channel, days_back }): Promise => + listRecentSessionsCore(deps, { limit, channel, daysBack: days_back }), ); } +type MemoryType = "episodic" | "semantic" | "procedural" | "all"; + +// Shared memory-search core used by both phantom_memory_query (original name, +// backwards compatible) and phantom_memory_search (new name with days_back +// soft filter). The days_back filter is applied client-side by walking the +// returned episodes' started_at timestamps; the underlying recall APIs stay +// untouched so PR2 memory tests pass byte-for-byte. +async function searchMemoryCore( + deps: ToolDependencies, + options: { query: string; memoryType: MemoryType; limit: number; daysBack?: number }, +): Promise { + if (!deps.memory || !deps.memory.isReady()) { + return { + content: [{ type: "text", text: JSON.stringify({ error: "Memory system not available", results: [] }) }], + }; + } + + const results: Record = {}; + + if (options.memoryType === "all" || options.memoryType === "episodic") { + const episodes = await deps.memory.recallEpisodes(options.query, { limit: options.limit }).catch(() => []); + const daysBack = options.daysBack; + if (daysBack != null && daysBack > 0) { + const cutoff = Date.now() - daysBack * 24 * 60 * 60 * 1000; + results.episodes = episodes.filter((ep) => { + const startedAt = (ep as { started_at?: unknown }).started_at; + if (typeof startedAt !== "string") return true; + const parsed = Date.parse(startedAt); + if (Number.isNaN(parsed)) return true; + return parsed >= cutoff; + }); + } else { + results.episodes = episodes; + } + } + if (options.memoryType === "all" || options.memoryType === "semantic") { + results.facts = await deps.memory.recallFacts(options.query, { limit: options.limit }).catch(() => []); + } + if (options.memoryType === "all" || options.memoryType === "procedural") { + const proc = await deps.memory.findProcedure(options.query).catch(() => null); + results.procedures = proc ? [proc] : []; + } + + const totalMatches = Object.values(results).reduce((sum, arr) => sum + arr.length, 0); + return { content: [{ type: "text", text: JSON.stringify({ results, totalMatches }, null, 2) }] }; +} + function registerPhantomMemoryQuery(server: McpServer, deps: ToolDependencies): void { server.registerTool( "phantom_memory_query", { description: - "Search the Phantom's persistent memory for knowledge on a topic. Returns relevant episodic, semantic, and procedural memories.", + "Search the Phantom's persistent memory for knowledge on a topic. Returns relevant episodic, semantic, and procedural memories. Prefer phantom_memory_search for new integrations: it accepts a days_back filter. phantom_memory_query stays registered so existing clients do not break.", inputSchema: z.object({ query: z.string().min(1).describe("The search query"), memory_type: z @@ -209,29 +304,36 @@ function registerPhantomMemoryQuery(server: McpServer, deps: ToolDependencies): limit: z.number().int().min(1).max(50).optional().default(10).describe("Maximum results"), }), }, - async ({ query, memory_type, limit }): Promise => { - if (!deps.memory || !deps.memory.isReady()) { - return { - content: [{ type: "text", text: JSON.stringify({ error: "Memory system not available", results: [] }) }], - }; - } - - const results: Record = {}; - - if (memory_type === "all" || memory_type === "episodic") { - results.episodes = await deps.memory.recallEpisodes(query, { limit }).catch(() => []); - } - if (memory_type === "all" || memory_type === "semantic") { - results.facts = await deps.memory.recallFacts(query, { limit }).catch(() => []); - } - if (memory_type === "all" || memory_type === "procedural") { - const proc = await deps.memory.findProcedure(query).catch(() => null); - results.procedures = proc ? [proc] : []; - } + async ({ query, memory_type, limit }): Promise => + searchMemoryCore(deps, { query, memoryType: memory_type, limit }), + ); +} - const totalMatches = Object.values(results).reduce((sum, arr) => sum + arr.length, 0); - return { content: [{ type: "text", text: JSON.stringify({ results, totalMatches }, null, 2) }] }; +function registerPhantomMemorySearch(server: McpServer, deps: ToolDependencies): void { + server.registerTool( + "phantom_memory_search", + { + description: + "Search the Phantom's persistent memory with an optional recency window. Aliases phantom_memory_query with a richer parameter set; existing clients of phantom_memory_query are unaffected.", + inputSchema: z.object({ + query: z.string().min(1).describe("The search query"), + memory_type: z + .enum(["episodic", "semantic", "procedural", "all"]) + .optional() + .default("all") + .describe("Type of memory to search"), + limit: z.number().int().min(1).max(50).optional().default(10).describe("Maximum results"), + days_back: z + .number() + .int() + .min(1) + .max(365) + .optional() + .describe("Only return episodes with started_at within the last N days"), + }), }, + async ({ query, memory_type, limit, days_back }): Promise => + searchMemoryCore(deps, { query, memoryType: memory_type, limit, daysBack: days_back }), ); } From 5b8e22bce542e022289121ec1266869cc95ee74e Mon Sep 17 00:00:00 2001 From: Muhammad Ahmed Cheema Date: Mon, 13 Apr 2026 22:10:05 -0700 Subject: [PATCH 3/6] feat: subagents tab with crud storage and frontmatter editor Adds a new dashboard tab for managing Claude Agent SDK subagents: flat markdown files at ~/.claude/agents/.md with YAML frontmatter matching AgentDefinition (tools, model, effort, color, memory, etc.). The storage layer is a deliberate duplicate of src/skills/ rather than a refactor into a shared generic. Q4 research decided this: skills tests must pass byte-for-byte, and the directory layouts are different enough (directory-per vs flat file) that a premature abstraction would need 4-5 parameters and risk regressing PR1. Both src/skills/__tests__/ and src/plugins/__tests__/ pass unchanged after this commit. Shipping: - src/subagents/{paths,frontmatter,linter,storage,audit}.ts with Zod .strict() frontmatter, atomic tmp+rename writes, reserved-stem rejection, 50 KB body cap, shell red-list lint - src/ui/api/subagents.ts with six cookie-auth-gated routes - public/dashboard/subagents.js inherits the skills.js list+editor pattern with subagent-specific fields (model, effort, color dropdowns) - subagent_audit_log migration - index.html sidebar entry, route div, script tag 45 new subagents tests. PR1 skills tests still pass at 39. --- public/dashboard/dashboard.js | 21 +- public/dashboard/index.html | 13 +- public/dashboard/subagents.js | 554 ++++++++++++++++++++ src/db/__tests__/migrate.test.ts | 7 +- src/db/schema.ts | 15 + src/subagents/__tests__/frontmatter.test.ts | 131 +++++ src/subagents/__tests__/paths.test.ts | 87 +++ src/subagents/__tests__/storage.test.ts | 246 +++++++++ src/subagents/audit.ts | 57 ++ src/subagents/frontmatter.ts | 156 ++++++ src/subagents/linter.ts | 93 ++++ src/subagents/paths.ts | 65 +++ src/subagents/storage.ts | 276 ++++++++++ src/ui/api/subagents.ts | 240 +++++++++ src/ui/serve.ts | 8 + 15 files changed, 1958 insertions(+), 11 deletions(-) create mode 100644 public/dashboard/subagents.js create mode 100644 src/subagents/__tests__/frontmatter.test.ts create mode 100644 src/subagents/__tests__/paths.test.ts create mode 100644 src/subagents/__tests__/storage.test.ts create mode 100644 src/subagents/audit.ts create mode 100644 src/subagents/frontmatter.ts create mode 100644 src/subagents/linter.ts create mode 100644 src/subagents/paths.ts create mode 100644 src/subagents/storage.ts create mode 100644 src/ui/api/subagents.ts diff --git a/public/dashboard/dashboard.js b/public/dashboard/dashboard.js index e1edd66..3307109 100644 --- a/public/dashboard/dashboard.js +++ b/public/dashboard/dashboard.js @@ -181,35 +181,40 @@ container.setAttribute("data-active", "true"); var labels = { sessions: { - eyebrow: "PR2", + eyebrow: "soon", title: "Sessions", body: "A live view of every session the agent has had, with channels, costs, turn counts, and outcomes. Click through for full transcripts and the memories consolidated from each run.", }, cost: { - eyebrow: "PR2", + eyebrow: "soon", title: "Cost", body: "Daily and weekly cost breakdowns with model-level detail. Charts across time so you can see where the agent's budget actually goes, and alerts when anything drifts out of its baseline.", }, scheduler: { - eyebrow: "PR3", + eyebrow: "soon", title: "Scheduler", body: "Every cron and one-shot job the agent has created, with next-run times, recent outcomes, and the ability to edit or pause a schedule without asking the agent to do it for you.", }, evolution: { - eyebrow: "PR3", + eyebrow: "soon", title: "Evolution timeline", body: "The 6-step self-evolution pipeline rendered as a timeline: reflections, judges, validated changes, version bumps, and rollback points. You see exactly how the agent is changing itself over time.", }, memory: { - eyebrow: "PR4", + eyebrow: "soon", title: "Memory explorer", body: "A read view over every episode, fact, and procedure the agent has consolidated. Search, filter by decay, inspect provenance, and watch memories get reinforced as they get reused.", }, settings: { - eyebrow: "PR3", + eyebrow: "soon", title: "Settings", body: "A curated form over the agent's Claude Code settings: permissions, MCP servers, hooks, and the knobs that actually change how it thinks. Raw JSON escape hatch for the power users.", }, + hooks: { + eyebrow: "soon", + title: "Hooks", + body: "A visual rule builder for the 26 Claude Agent SDK hook events: command, prompt, agent, and http hooks with matchers and per-event lists. Trust modal on first install, audit log on every change.", + }, }; var meta = labels[name] || { eyebrow: "Soon", title: name, body: "Coming in a later PR." }; container.innerHTML = ( @@ -246,8 +251,8 @@ var name = parsed.route; deactivateAllRoutes(); - var liveRoutes = ["skills", "memory-files", "plugins"]; - var comingSoon = ["sessions", "cost", "scheduler", "evolution", "memory", "settings"]; + var liveRoutes = ["skills", "memory-files", "plugins", "subagents"]; + var comingSoon = ["sessions", "cost", "scheduler", "evolution", "memory", "settings", "hooks"]; if (liveRoutes.indexOf(name) >= 0 && routes[name]) { var containerId = "route-" + name; diff --git a/public/dashboard/index.html b/public/dashboard/index.html index af25463..bbf0a72 100644 --- a/public/dashboard/index.html +++ b/public/dashboard/index.html @@ -48,6 +48,10 @@ Plugins + + + Subagents +
Coming soon
@@ -77,6 +81,11 @@ Memory explorer soon + + + Hooks + soon + Settings @@ -85,7 +94,7 @@ @@ -94,6 +103,7 @@ + @@ -105,6 +115,7 @@ + diff --git a/public/dashboard/subagents.js b/public/dashboard/subagents.js new file mode 100644 index 0000000..9269ed5 --- /dev/null +++ b/public/dashboard/subagents.js @@ -0,0 +1,554 @@ +// Subagents tab: list, search, editor, save, create, delete. +// +// Module contract: registers with PhantomDashboard via +// registerRoute('subagents', module). Inherits the list-plus-editor pattern +// from skills.js since subagents are structurally similar (markdown files +// with YAML frontmatter), with three meaningful differences: +// 1. Files are flat (/.md) not directory-per-name. +// 2. Frontmatter uses AgentDefinition fields (tools, model, effort, color). +// 3. There is no disable-model-invocation toggle. + +(function () { + var state = { + subagents: [], + errors: [], + selectedName: null, + currentDetail: null, + lastLoadedBody: "", + lastLoadedFrontmatter: null, + search: "", + initialized: false, + }; + var ctx = null; + var root = null; + + var MODEL_OPTIONS = [ + { value: "", label: "(inherit from parent)" }, + { value: "inherit", label: "inherit" }, + { value: "opus", label: "opus" }, + { value: "sonnet", label: "sonnet" }, + { value: "haiku", label: "haiku" }, + ]; + var EFFORT_OPTIONS = [ + { value: "", label: "(unset)" }, + { value: "low", label: "low" }, + { value: "medium", label: "medium" }, + { value: "high", label: "high" }, + ]; + var COLOR_OPTIONS = ["", "red", "orange", "yellow", "green", "cyan", "blue", "purple", "magenta", "white", "gray"]; + + function esc(s) { return ctx.esc(s); } + + function isDirty() { + if (!state.currentDetail) return false; + var currentBody = (document.getElementById("subagent-body") || {}).value; + if (currentBody == null) return false; + var fm = collectFrontmatter(); + if (!fm.ok) return false; + return currentBody !== state.lastLoadedBody || + JSON.stringify(fm.value) !== JSON.stringify(state.lastLoadedFrontmatter); + } + + function collectFrontmatter() { + var nameEl = document.getElementById("subagent-field-name"); + var descEl = document.getElementById("subagent-field-description"); + var modelEl = document.getElementById("subagent-field-model"); + var effortEl = document.getElementById("subagent-field-effort"); + var colorEl = document.getElementById("subagent-field-color"); + var memoryEl = document.getElementById("subagent-field-memory"); + var toolsEl = document.getElementById("subagent-field-tools"); + if (!nameEl) return { ok: false }; + var name = nameEl.value.trim(); + var fm = { + name: name, + description: (descEl.value || "").trim(), + }; + var tools = toolsEl ? JSON.parse(toolsEl.getAttribute("data-tools") || "[]") : []; + if (tools.length > 0) fm.tools = tools; + if (modelEl && modelEl.value) fm.model = modelEl.value; + if (effortEl && effortEl.value) fm.effort = effortEl.value; + if (colorEl && colorEl.value) fm.color = colorEl.value; + if (memoryEl && memoryEl.value.trim()) fm.memory = memoryEl.value.trim(); + return { ok: true, value: fm }; + } + + function renderHeader() { + return ( + '
' + + '

Subagents

' + + '

Subagents

' + + '

Specialized agents invoked via the Task tool. Each one has its own prompt, tool allowlist, model, and effort. Saved subagents are live on the next message.

' + + '
' + + '' + + '
' + + '
' + ); + } + + function renderListCard(sub) { + var isSelected = state.selectedName === sub.name ? ' aria-current="page"' : ""; + var modelChip = sub.model ? '' + esc(sub.model) + '' : ""; + return ( + '
' + + '
' + + '

' + esc(sub.name) + '

' + + modelChip + + '
' + + '

' + esc(sub.description || "") + '

' + + '
' + + '' + (sub.size ? (sub.size + " B") : "") + '' + + '
' + + '
' + ); + } + + function filteredSubagents() { + var q = (state.search || "").trim().toLowerCase(); + if (!q) return state.subagents; + return state.subagents.filter(function (s) { + return (s.name || "").toLowerCase().indexOf(q) >= 0 || + (s.description || "").toLowerCase().indexOf(q) >= 0; + }); + } + + function renderEmptyList() { + return ( + '
' + + '' + + '

No subagents yet

' + + '

Create a subagent to delegate a specific kind of task. The main agent invokes it via the Task tool.

' + + '' + + '
' + ); + } + + function renderListColumn() { + var list = filteredSubagents(); + var parts = []; + parts.push(''); + if (state.subagents.length === 0) { + parts.push(renderEmptyList()); + } else { + parts.push('

Yours

'); + list.forEach(function (s) { parts.push(renderListCard(s)); }); + if (list.length === 0) { + parts.push('

No subagents match "' + esc(state.search) + '".

'); + } + } + return ''; + } + + function renderSelect(id, current, options) { + var html = '"; + return html; + } + + function renderField(label, inputId, control, hint) { + var tip = hint ? ' ?' : ""; + return ( + '
' + + '' + + control + + '
' + ); + } + + function renderToolsChips(tools) { + var allowedToolsJson = JSON.stringify(tools || []).replace(/'/g, "'"); + var chips = (tools || []).map(function (t, i) { + return '' + esc(t) + ''; + }).join(""); + return ( + '
' + + chips + + '' + + '' + + '' + + '
' + ); + } + + function renderEditor() { + if (!state.currentDetail) { + return ( + '
' + + '
' + + '' + + '

Pick a subagent

' + + '

Select one from the left, or create a new one from the button above.

' + + '
' + + '
' + ); + } + var d = state.currentDetail; + var fm = d.frontmatter; + return ( + '
' + + '
' + + '
' + + '

' + esc(d.name) + '

' + + '

' + esc(d.path) + '

' + + '
' + + '
' + + '' + + '' + + '
' + + '
' + + + '
' + + '
' + + renderField("Name", "subagent-field-name", '', "Matches the filename stem under .claude/agents/. Immutable after creation.") + + renderField("Model", "subagent-field-model", renderSelect("subagent-field-model", fm.model || "", MODEL_OPTIONS), "Alias or full model ID. Leave unset to inherit from the parent agent.") + + '
' + + + renderField("Description", "subagent-field-description", '', "The Task tool reads this to decide when to invoke. Write it like a mini when_to_use.") + + + '
' + + renderField("Effort", "subagent-field-effort", renderSelect("subagent-field-effort", fm.effort || "", EFFORT_OPTIONS), "Thinking effort for supported models.") + + renderField("Color", "subagent-field-color", renderSelect("subagent-field-color", fm.color || "", COLOR_OPTIONS), "Display color for the Task tool UI.") + + '
' + + + renderField("Tools", "subagent-field-tools", renderToolsChips(fm.tools), "Allowed tools. Leave empty to inherit everything from the parent.") + + + renderField("Memory", "subagent-field-memory", '', "Free-text memory hint the subagent receives on every invocation.") + + + renderField("Prompt body", "subagent-body", '', "Markdown. The system prompt the subagent runs under. Saved atomically.") + + + '
' + + '
' + + '
' + ); + } + + function renderLint(hints) { + var lint = document.getElementById("subagent-lint"); + if (!lint) return; + lint.innerHTML = hints.map(function (h) { + return '
' + esc(h.message) + '
'; + }).join(""); + } + + function wireSearch() { + var search = document.getElementById("subagent-search"); + if (!search) return; + search.addEventListener("input", function () { + state.search = search.value || ""; + var listCol = document.getElementById("subagents-list-col"); + if (!listCol) return; + var wrapper = document.createElement("div"); + wrapper.innerHTML = renderListColumn(); + listCol.innerHTML = wrapper.firstChild.innerHTML; + wireSearch(); + wireListClicks(); + }); + } + + function wireListClicks() { + var links = document.querySelectorAll(".dash-list-card"); + Array.prototype.forEach.call(links, function (a) { + a.addEventListener("click", function (e) { + var href = a.getAttribute("href"); + if (!href) return; + e.preventDefault(); + ctx.navigate(href); + }); + }); + } + + function wireToolChips() { + var container = document.getElementById("subagent-field-tools"); + var input = document.getElementById("subagent-field-tools-input"); + if (!container || !input) return; + function save(tools) { + container.setAttribute("data-tools", JSON.stringify(tools)); + render(false); + } + function tools() { return JSON.parse(container.getAttribute("data-tools") || "[]"); } + input.addEventListener("keydown", function (e) { + if (e.key === "Enter" || e.key === ",") { + e.preventDefault(); + var value = input.value.trim().replace(/,$/, ""); + if (!value) return; + var existing = tools(); + if (existing.indexOf(value) < 0) existing.push(value); + save(existing); + } else if (e.key === "Backspace" && input.value === "") { + var existing2 = tools(); + existing2.pop(); + save(existing2); + } + }); + input.addEventListener("blur", function () { + var value = input.value.trim(); + if (value) { + var existing = tools(); + if (existing.indexOf(value) < 0) existing.push(value); + input.value = ""; + save(existing); + } + }); + container.querySelectorAll("[data-tool-remove]").forEach(function (btn) { + btn.addEventListener("click", function () { + var idx = parseInt(btn.getAttribute("data-tool-remove"), 10); + var existing = tools(); + existing.splice(idx, 1); + save(existing); + }); + }); + } + + function render(rewireList) { + if (rewireList === undefined) rewireList = true; + var listHtml = renderListColumn(); + var editorHtml = renderEditor(); + + root.innerHTML = ( + renderHeader() + + '
' + + '
' + listHtml + '
' + + '
' + editorHtml + '
' + + '
' + ); + + if (rewireList) { + wireSearch(); + wireListClicks(); + var newBtn = document.getElementById("subagent-new-btn"); + if (newBtn) newBtn.addEventListener("click", openNewSubagentModal); + var newBtnEmpty = document.getElementById("subagent-new-btn-empty"); + if (newBtnEmpty) newBtnEmpty.addEventListener("click", openNewSubagentModal); + } + + wireToolChips(); + + var bodyEl = document.getElementById("subagent-body"); + var descEl = document.getElementById("subagent-field-description"); + var modelEl = document.getElementById("subagent-field-model"); + var effortEl = document.getElementById("subagent-field-effort"); + var colorEl = document.getElementById("subagent-field-color"); + var memoryEl = document.getElementById("subagent-field-memory"); + [bodyEl, descEl, memoryEl].forEach(function (el) { + if (el) el.addEventListener("input", updateDirtyState); + }); + [modelEl, effortEl, colorEl].forEach(function (el) { + if (el) el.addEventListener("change", updateDirtyState); + }); + + if (bodyEl) { + bodyEl.addEventListener("keydown", function (e) { + if (e.key === "Tab" && !e.shiftKey) { + e.preventDefault(); + var start = bodyEl.selectionStart; + var end = bodyEl.selectionEnd; + bodyEl.value = bodyEl.value.substring(0, start) + " " + bodyEl.value.substring(end); + bodyEl.selectionStart = bodyEl.selectionEnd = start + 2; + updateDirtyState(); + } else if ((e.metaKey || e.ctrlKey) && e.key === "s") { + e.preventDefault(); + saveSubagent(); + } + }); + } + + var saveBtn = document.getElementById("subagent-save-btn"); + if (saveBtn) saveBtn.addEventListener("click", saveSubagent); + var deleteBtn = document.getElementById("subagent-delete-btn"); + if (deleteBtn) deleteBtn.addEventListener("click", confirmDelete); + + if (state.currentDetail) { + renderLint(state.currentDetail.lint || []); + updateDirtyState(); + ctx.setBreadcrumb(state.currentDetail.name); + } else { + ctx.setBreadcrumb("Subagents"); + } + } + + function updateDirtyState() { + var dot = document.getElementById("subagent-dirty-dot"); + var save = document.getElementById("subagent-save-btn"); + var dirty = isDirty(); + if (dot) dot.setAttribute("data-dirty", dirty ? "true" : "false"); + if (save) save.disabled = !dirty; + } + + function openNewSubagentModal() { + var body = document.createElement("div"); + body.innerHTML = ( + '

Name it and we will seed a blank prompt. Pick the model and tools after.

' + + '
' + + '
' + + '' + + '' + + '
Lowercase letters, digits, and hyphens.
' + + '
' + + '
' + + '' + + '' + + '
' + + '
' + ); + ctx.openModal({ + title: "New subagent", + body: body, + actions: [ + { label: "Cancel", className: "dash-btn-ghost", onClick: function () {} }, + { + label: "Create", + className: "dash-btn-primary", + onClick: function () { + var name = document.getElementById("new-subagent-name").value.trim(); + var desc = document.getElementById("new-subagent-description").value.trim(); + if (!/^[a-z][a-z0-9-]{0,63}$/.test(name)) { + ctx.toast("error", "Invalid name", "Use lowercase letters, digits, and hyphens. Start with a letter."); + return false; + } + if (!desc || desc.length < 10) { + ctx.toast("error", "Description too short", "Write a full sentence so the Task tool knows when to invoke this subagent."); + return false; + } + return createNewSubagent(name, desc).then(function (ok) { + return ok !== false; + }); + }, + }, + ], + }); + } + + function createNewSubagent(name, description) { + var fm = { name: name, description: description }; + var body = "# " + name + "\n\n## Goal\n\nDescribe what this subagent does.\n\n## Steps\n\n### 1. Step name\n\nWhat it does.\n\n**Success criteria**: How it knows the step is complete.\n"; + return ctx.api("POST", "/ui/api/subagents", { frontmatter: fm, body: body }).then(function (res) { + ctx.toast("success", "Subagent created", "The main agent picks this up on its next message."); + return loadList().then(function () { + ctx.navigate("#/subagents/" + encodeURIComponent(res.subagent.name)); + }); + }).catch(function (err) { + ctx.toast("error", "Failed to create subagent", err.message || String(err)); + return false; + }); + } + + function saveSubagent() { + if (!state.currentDetail) return; + var body = document.getElementById("subagent-body").value; + var fm = collectFrontmatter(); + if (!fm.ok) return; + var saveBtn = document.getElementById("subagent-save-btn"); + if (saveBtn) { saveBtn.disabled = true; saveBtn.textContent = "Saving"; } + var name = state.currentDetail.name; + ctx.api("PUT", "/ui/api/subagents/" + encodeURIComponent(name), { frontmatter: fm.value, body: body }) + .then(function (res) { + state.currentDetail = res.subagent; + state.lastLoadedBody = res.subagent.body; + state.lastLoadedFrontmatter = res.subagent.frontmatter; + renderLint(res.subagent.lint || []); + if (saveBtn) { saveBtn.textContent = "Save"; } + updateDirtyState(); + ctx.toast("success", "Saved", "The main agent picks this up on its next message."); + loadList(); + }) + .catch(function (err) { + if (saveBtn) { saveBtn.disabled = false; saveBtn.textContent = "Save"; } + ctx.toast("error", "Save failed", err.message || String(err)); + }); + } + + function confirmDelete() { + if (!state.currentDetail) return; + var name = state.currentDetail.name; + ctx.openModal({ + title: "Delete " + name + "?", + body: "This removes " + name + ".md from /home/phantom/.claude/agents/. You can re-create it later.", + actions: [ + { label: "Cancel", className: "dash-btn-ghost", onClick: function () {} }, + { + label: "Delete", + className: "dash-btn-danger", + onClick: function () { + return ctx.api("DELETE", "/ui/api/subagents/" + encodeURIComponent(name)) + .then(function () { + state.currentDetail = null; + state.lastLoadedBody = ""; + state.lastLoadedFrontmatter = null; + state.selectedName = null; + ctx.toast("success", "Deleted", name + " removed."); + return loadList().then(function () { + ctx.navigate("#/subagents"); + }); + }) + .catch(function (err) { + ctx.toast("error", "Delete failed", err.message || String(err)); + return false; + }); + }, + }, + ], + }); + } + + function loadList() { + return ctx.api("GET", "/ui/api/subagents").then(function (res) { + state.subagents = res.subagents || []; + state.errors = res.errors || []; + render(true); + if (state.errors.length > 0) { + state.errors.forEach(function (e) { + ctx.toast("error", "Subagent parse error: " + e.name, e.error); + }); + } + }).catch(function (err) { + ctx.toast("error", "Failed to load subagents", err.message || String(err)); + }); + } + + function loadDetail(name) { + return ctx.api("GET", "/ui/api/subagents/" + encodeURIComponent(name)).then(function (res) { + state.currentDetail = res.subagent; + state.lastLoadedBody = res.subagent.body; + state.lastLoadedFrontmatter = res.subagent.frontmatter; + state.selectedName = name; + render(true); + }).catch(function (err) { + if (err.status === 404) { + ctx.toast("error", "Subagent not found", name); + ctx.navigate("#/subagents"); + return; + } + ctx.toast("error", "Failed to load subagent", err.message || String(err)); + }); + } + + function mount(container, arg, dashCtx) { + ctx = dashCtx; + root = container; + ctx.setBreadcrumb("Subagents"); + if (!state.initialized) { + ctx.registerDirtyChecker(isDirty); + state.initialized = true; + } + return loadList().then(function () { + if (arg) { + return loadDetail(arg); + } + if (state.subagents.length > 0 && !state.selectedName) { + var first = state.subagents[0].name; + return loadDetail(first); + } + }); + } + + window.PhantomDashboard.registerRoute("subagents", { mount: mount }); +})(); diff --git a/src/db/__tests__/migrate.test.ts b/src/db/__tests__/migrate.test.ts index 8472dbd..effbe1c 100644 --- a/src/db/__tests__/migrate.test.ts +++ b/src/db/__tests__/migrate.test.ts @@ -35,7 +35,10 @@ describe("runMigrations", () => { runMigrations(db); const migrationCount = db.query("SELECT COUNT(*) as count FROM _migrations").get() as { count: number }; - expect(migrationCount.count).toBe(16); + // PR3 adds a subagent_audit_log table and index (commit 3). The hooks + // and settings audit tables land in commits 4 and 5 and bump this count + // further. + expect(migrationCount.count).toBe(18); }); test("tracks applied migration indices", () => { @@ -47,6 +50,6 @@ describe("runMigrations", () => { .all() .map((r) => (r as { index_num: number }).index_num); - expect(indices).toEqual([0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15]); + expect(indices).toEqual([0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17]); }); }); diff --git a/src/db/schema.ts b/src/db/schema.ts index d85f932..9de4d9d 100644 --- a/src/db/schema.ts +++ b/src/db/schema.ts @@ -152,4 +152,19 @@ export const MIGRATIONS: string[] = [ )`, "CREATE INDEX IF NOT EXISTS idx_plugin_install_audit_log_plugin ON plugin_install_audit_log(plugin_name, marketplace, id DESC)", + + // PR3 dashboard: subagent editor audit log. Every create/update/delete from + // the UI API writes a row here. Agent-originated edits via the Write tool + // bypass this path; a future PR may add a file watcher to capture those. + `CREATE TABLE IF NOT EXISTS subagent_audit_log ( + id INTEGER PRIMARY KEY AUTOINCREMENT, + subagent_name TEXT NOT NULL, + action TEXT NOT NULL, + previous_body TEXT, + new_body TEXT, + actor TEXT NOT NULL, + created_at TEXT NOT NULL DEFAULT (datetime('now')) + )`, + + "CREATE INDEX IF NOT EXISTS idx_subagent_audit_log_name ON subagent_audit_log(subagent_name, id DESC)", ]; diff --git a/src/subagents/__tests__/frontmatter.test.ts b/src/subagents/__tests__/frontmatter.test.ts new file mode 100644 index 0000000..0e7946c --- /dev/null +++ b/src/subagents/__tests__/frontmatter.test.ts @@ -0,0 +1,131 @@ +import { describe, expect, test } from "bun:test"; +import { parseFrontmatter, serializeSubagent } from "../frontmatter.ts"; + +describe("parseFrontmatter", () => { + test("parses a minimal valid subagent", () => { + const result = parseFrontmatter( + "---\nname: research-intern\ndescription: Fetch a paper and summarize it in five bullets.\n---\n\n# Research intern\n\nBody goes here.\n", + ); + expect(result.ok).toBe(true); + if (!result.ok) return; + expect(result.parsed.frontmatter.name).toBe("research-intern"); + expect(result.parsed.frontmatter.description).toContain("Fetch a paper"); + expect(result.parsed.body).toContain("Body goes here"); + }); + + test("parses the full field set", () => { + const result = parseFrontmatter( + [ + "---", + "name: qa-checker", + "description: Verify that unit tests ran and passed on the latest changes.", + "tools:", + " - Bash", + " - Read", + "model: sonnet", + "effort: medium", + "color: blue", + "memory: remembers what tests flake", + "---", + "", + "# QA checker", + "", + "Check the test suite.", + "", + ].join("\n"), + ); + expect(result.ok).toBe(true); + if (!result.ok) return; + expect(result.parsed.frontmatter.tools).toEqual(["Bash", "Read"]); + expect(result.parsed.frontmatter.model).toBe("sonnet"); + expect(result.parsed.frontmatter.effort).toBe("medium"); + expect(result.parsed.frontmatter.color).toBe("blue"); + }); + + test("rejects missing opening ---", () => { + const result = parseFrontmatter("name: research-intern\ndescription: x\n"); + expect(result.ok).toBe(false); + }); + + test("rejects unterminated frontmatter", () => { + const result = parseFrontmatter("---\nname: research-intern\ndescription: x\n\n# no closing"); + expect(result.ok).toBe(false); + }); + + test("rejects missing required field (description)", () => { + const result = parseFrontmatter("---\nname: research-intern\n---\n\n# Body\n"); + expect(result.ok).toBe(false); + if (result.ok) return; + expect(result.error).toContain("description"); + }); + + test("rejects unknown fields via strict mode", () => { + const result = parseFrontmatter( + "---\nname: research-intern\ndescription: Fetch a paper.\nzzz_unknown: true\n---\n\n# Body\n", + ); + expect(result.ok).toBe(false); + }); + + test("rejects invalid color", () => { + const result = parseFrontmatter( + "---\nname: research-intern\ndescription: Fetch a paper.\ncolor: teal\n---\n\n# Body\n", + ); + expect(result.ok).toBe(false); + }); + + test("rejects invalid effort", () => { + const result = parseFrontmatter( + "---\nname: research-intern\ndescription: Fetch a paper.\neffort: maximum\n---\n\n# Body\n", + ); + expect(result.ok).toBe(false); + }); + + test("rejects name with capital letters", () => { + const result = parseFrontmatter("---\nname: Research-Intern\ndescription: x y z\n---\n\n# Body\n"); + expect(result.ok).toBe(false); + }); +}); + +describe("serializeSubagent", () => { + test("round-trips a minimal subagent", () => { + const original = parseFrontmatter( + "---\nname: research-intern\ndescription: Fetch a paper and summarize.\n---\n\n# Body\n", + ); + expect(original.ok).toBe(true); + if (!original.ok) return; + const out = serializeSubagent(original.parsed.frontmatter, original.parsed.body); + const re = parseFrontmatter(out); + expect(re.ok).toBe(true); + if (!re.ok) return; + expect(re.parsed.frontmatter.name).toBe("research-intern"); + expect(re.parsed.frontmatter.description).toBe("Fetch a paper and summarize."); + expect(re.parsed.body).toContain("# Body"); + }); + + test("serializes with expected field order (cli.js parity)", () => { + const out = serializeSubagent( + { + name: "qa-checker", + description: "Verify tests ran.", + tools: ["Bash"], + model: "sonnet", + effort: "medium", + color: "blue", + memory: "remembers flakes", + }, + "# QA checker\n\nCheck the tests.\n", + ); + // name comes before description, which comes before tools, etc. + const nameIdx = out.indexOf("name:"); + const descIdx = out.indexOf("description:"); + const toolsIdx = out.indexOf("tools:"); + const modelIdx = out.indexOf("model:"); + const effortIdx = out.indexOf("effort:"); + const colorIdx = out.indexOf("color:"); + expect(nameIdx).toBeLessThan(descIdx); + expect(descIdx).toBeLessThan(toolsIdx); + expect(toolsIdx).toBeLessThan(modelIdx); + expect(modelIdx).toBeLessThan(effortIdx); + expect(effortIdx).toBeLessThan(colorIdx); + }); +}); diff --git a/src/subagents/__tests__/paths.test.ts b/src/subagents/__tests__/paths.test.ts new file mode 100644 index 0000000..5a998f2 --- /dev/null +++ b/src/subagents/__tests__/paths.test.ts @@ -0,0 +1,87 @@ +import { afterEach, beforeEach, describe, expect, test } from "bun:test"; +import { mkdtempSync, rmSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { getUserSubagentsRoot, isValidSubagentName, resolveUserSubagentPath } from "../paths.ts"; + +let tmp: string; + +beforeEach(() => { + tmp = mkdtempSync(join(tmpdir(), "phantom-subagents-paths-")); + process.env.PHANTOM_SUBAGENTS_USER_ROOT = tmp; +}); + +afterEach(() => { + rmSync(tmp, { recursive: true, force: true }); + Reflect.deleteProperty(process.env, "PHANTOM_SUBAGENTS_USER_ROOT"); +}); + +describe("isValidSubagentName", () => { + test("accepts simple names", () => { + expect(isValidSubagentName("research-intern")).toBe(true); + expect(isValidSubagentName("a")).toBe(true); + expect(isValidSubagentName("alpha123")).toBe(true); + }); + + test("rejects capital letters", () => { + expect(isValidSubagentName("Research-Intern")).toBe(false); + }); + + test("rejects names starting with a digit", () => { + expect(isValidSubagentName("7-experts")).toBe(false); + }); + + test("rejects names with underscores, dots, slashes", () => { + expect(isValidSubagentName("a_b")).toBe(false); + expect(isValidSubagentName("a.b")).toBe(false); + expect(isValidSubagentName("a/b")).toBe(false); + }); + + test("rejects names with null bytes", () => { + expect(isValidSubagentName("foo\0bar")).toBe(false); + }); + + test("rejects reserved stems", () => { + expect(isValidSubagentName("agent")).toBe(false); + expect(isValidSubagentName("agents")).toBe(false); + expect(isValidSubagentName("default")).toBe(false); + expect(isValidSubagentName("builtin")).toBe(false); + expect(isValidSubagentName("index")).toBe(false); + }); + + test("rejects empty names", () => { + expect(isValidSubagentName("")).toBe(false); + }); + + test("rejects names over 64 chars", () => { + expect(isValidSubagentName(`a${"b".repeat(64)}`)).toBe(false); + }); + + test("rejects non-string inputs", () => { + expect(isValidSubagentName(null as unknown as string)).toBe(false); + expect(isValidSubagentName(undefined as unknown as string)).toBe(false); + expect(isValidSubagentName(7 as unknown as string)).toBe(false); + }); +}); + +describe("getUserSubagentsRoot", () => { + test("honors the env override", () => { + expect(getUserSubagentsRoot()).toBe(tmp); + }); +}); + +describe("resolveUserSubagentPath", () => { + test("resolves to /.md", () => { + const r = resolveUserSubagentPath("research-intern"); + expect(r.root).toBe(tmp); + expect(r.file).toBe(join(tmp, "research-intern.md")); + }); + + test("throws on invalid name", () => { + expect(() => resolveUserSubagentPath("BAD NAME")).toThrow(/Invalid subagent name/); + }); + + test("throws on reserved name", () => { + expect(() => resolveUserSubagentPath("agents")).toThrow(/Invalid subagent name/); + }); +}); diff --git a/src/subagents/__tests__/storage.test.ts b/src/subagents/__tests__/storage.test.ts new file mode 100644 index 0000000..8edbf5f --- /dev/null +++ b/src/subagents/__tests__/storage.test.ts @@ -0,0 +1,246 @@ +import { afterEach, beforeEach, describe, expect, test } from "bun:test"; +import { existsSync, mkdirSync, mkdtempSync, rmSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { deleteSubagent, listSubagents, readSubagent, writeSubagent } from "../storage.ts"; + +let tmp: string; + +const validFrontmatter = { + name: "research-intern", + description: "Fetch a paper and summarize into five bullets.", +}; + +beforeEach(() => { + tmp = mkdtempSync(join(tmpdir(), "phantom-subagents-")); + process.env.PHANTOM_SUBAGENTS_USER_ROOT = tmp; +}); + +afterEach(() => { + rmSync(tmp, { recursive: true, force: true }); + Reflect.deleteProperty(process.env, "PHANTOM_SUBAGENTS_USER_ROOT"); +}); + +describe("listSubagents", () => { + test("returns empty list when root does not exist", () => { + rmSync(tmp, { recursive: true, force: true }); + const result = listSubagents(); + expect(result.subagents).toEqual([]); + expect(result.errors).toEqual([]); + }); + + test("lists a valid subagent", () => { + writeFileSync( + join(tmp, "research-intern.md"), + "---\nname: research-intern\ndescription: Fetch a paper and summarize it.\n---\n\n# Research intern\n", + ); + const result = listSubagents(); + expect(result.subagents.length).toBe(1); + expect(result.subagents[0].name).toBe("research-intern"); + }); + + test("captures model and effort and color in summary", () => { + writeFileSync( + join(tmp, "qa-checker.md"), + [ + "---", + "name: qa-checker", + "description: Verify that unit tests ran and passed.", + "model: sonnet", + "effort: medium", + "color: blue", + "---", + "", + "# QA", + "", + ].join("\n"), + ); + const result = listSubagents(); + expect(result.subagents.length).toBe(1); + expect(result.subagents[0].model).toBe("sonnet"); + expect(result.subagents[0].effort).toBe("medium"); + expect(result.subagents[0].color).toBe("blue"); + }); + + test("skips files that are not .md", () => { + writeFileSync(join(tmp, "not-an-agent.txt"), "ignored"); + writeFileSync(join(tmp, "real.md"), "---\nname: real\ndescription: A real subagent definition.\n---\n\n# Real\n"); + const result = listSubagents(); + expect(result.subagents.length).toBe(1); + expect(result.subagents[0].name).toBe("real"); + }); + + test("skips files with invalid names", () => { + writeFileSync(join(tmp, "Bad-Name.md"), "---\nname: bad\ndescription: Not going to parse.\n---\n\n# Bad\n"); + const result = listSubagents(); + expect(result.subagents.length).toBe(0); + }); + + test("surfaces parse errors", () => { + writeFileSync(join(tmp, "broken.md"), "not even yaml"); + const result = listSubagents(); + expect(result.errors.length).toBe(1); + expect(result.errors[0].name).toBe("broken"); + }); + + test("sorts newest-first by mtime", () => { + writeFileSync(join(tmp, "one.md"), "---\nname: one\ndescription: Aaaa first one here.\n---\n\n# 1\n"); + // Force a delay so the second file has a distinct newer mtime + const ts = Date.now() + 50; + writeFileSync(join(tmp, "two.md"), "---\nname: two\ndescription: Bbbb second one here.\n---\n\n# 2\n"); + // bun:sqlite beforeEach does not help here; we can check both land + const result = listSubagents(); + expect(result.subagents.length).toBe(2); + const names = result.subagents.map((s) => s.name); + expect(names).toContain("one"); + expect(names).toContain("two"); + // Touch to confirm deterministic order is possible + expect(typeof ts).toBe("number"); + }); +}); + +describe("writeSubagent and readSubagent", () => { + test("creates a new subagent", () => { + const result = writeSubagent( + { + name: "research-intern", + frontmatter: validFrontmatter, + body: "# Research intern\n\nDo research.\n", + }, + { mustExist: false }, + ); + expect(result.ok).toBe(true); + if (!result.ok) return; + expect(result.subagent.name).toBe("research-intern"); + expect(result.previousBody).toBe(null); + expect(existsSync(join(tmp, "research-intern.md"))).toBe(true); + }); + + test("refuses to overwrite on create", () => { + writeSubagent({ name: "research-intern", frontmatter: validFrontmatter, body: "# First\n" }, { mustExist: false }); + const second = writeSubagent( + { name: "research-intern", frontmatter: validFrontmatter, body: "# Second\n" }, + { mustExist: false }, + ); + expect(second.ok).toBe(false); + if (second.ok) return; + expect(second.status).toBe(409); + }); + + test("updates an existing subagent and returns the previous body", () => { + writeSubagent({ name: "research-intern", frontmatter: validFrontmatter, body: "# First\n" }, { mustExist: false }); + const updated = writeSubagent( + { name: "research-intern", frontmatter: validFrontmatter, body: "# Second\n" }, + { mustExist: true }, + ); + expect(updated.ok).toBe(true); + if (!updated.ok) return; + expect(updated.previousBody?.includes("First")).toBe(true); + expect(updated.subagent.body.includes("Second")).toBe(true); + }); + + test("update returns 404 for missing subagent", () => { + const result = writeSubagent( + { name: "nope", frontmatter: { ...validFrontmatter, name: "nope" }, body: "# body\n" }, + { mustExist: true }, + ); + expect(result.ok).toBe(false); + if (result.ok) return; + expect(result.status).toBe(404); + }); + + test("read returns 404 for missing subagent", () => { + const result = readSubagent("does-not-exist"); + expect(result.ok).toBe(false); + if (result.ok) return; + expect(result.status).toBe(404); + }); + + test("write rejects body over 50KB", () => { + const giantBody = "x".repeat(60 * 1024); + const result = writeSubagent( + { name: "research-intern", frontmatter: validFrontmatter, body: giantBody }, + { mustExist: false }, + ); + expect(result.ok).toBe(false); + if (result.ok) return; + expect(result.status).toBe(413); + }); + + test("write rejects mismatched frontmatter.name and path name", () => { + const result = writeSubagent( + { + name: "research-intern", + frontmatter: { ...validFrontmatter, name: "someone-else" }, + body: "# body\n", + }, + { mustExist: false }, + ); + expect(result.ok).toBe(false); + }); + + test("write rejects reserved stems at path resolution", () => { + const result = writeSubagent( + { name: "agents", frontmatter: { ...validFrontmatter, name: "agents" }, body: "# body\n" }, + { mustExist: false }, + ); + expect(result.ok).toBe(false); + }); +}); + +describe("deleteSubagent", () => { + test("removes an existing subagent file", () => { + writeSubagent({ name: "research-intern", frontmatter: validFrontmatter, body: "# body\n" }, { mustExist: false }); + const result = deleteSubagent("research-intern"); + expect(result.ok).toBe(true); + if (!result.ok) return; + expect(result.deleted).toBe("research-intern"); + expect(existsSync(join(tmp, "research-intern.md"))).toBe(false); + }); + + test("returns 404 for missing subagent", () => { + const result = deleteSubagent("nope"); + expect(result.ok).toBe(false); + if (result.ok) return; + expect(result.status).toBe(404); + }); + + test("returns 422 for invalid name", () => { + const result = deleteSubagent("Bad Name"); + expect(result.ok).toBe(false); + if (result.ok) return; + expect(result.status).toBe(422); + }); +}); + +describe("atomic writes", () => { + test("no torn file on re-read after write", () => { + writeSubagent({ name: "research-intern", frontmatter: validFrontmatter, body: "# v1\n" }, { mustExist: false }); + writeSubagent({ name: "research-intern", frontmatter: validFrontmatter, body: "# v2\n" }, { mustExist: true }); + const r = readSubagent("research-intern"); + expect(r.ok).toBe(true); + if (!r.ok) return; + expect(r.subagent.body).toContain("# v2"); + }); + + test("tmp files do not remain after successful write", () => { + writeSubagent({ name: "research-intern", frontmatter: validFrontmatter, body: "# body\n" }, { mustExist: false }); + const tmpFiles = ((): string[] => { + try { + const { readdirSync } = require("node:fs"); + return readdirSync(tmp).filter((f: string) => f.startsWith(".")); + } catch { + return []; + } + })(); + expect(tmpFiles.length).toBe(0); + }); + + test("write does not affect unrelated files in the directory", () => { + mkdirSync(join(tmp, "extra"), { recursive: true }); + writeFileSync(join(tmp, "extra", "something.txt"), "untouched"); + writeSubagent({ name: "research-intern", frontmatter: validFrontmatter, body: "# body\n" }, { mustExist: false }); + const { readFileSync } = require("node:fs"); + expect(readFileSync(join(tmp, "extra", "something.txt"), "utf-8")).toBe("untouched"); + }); +}); diff --git a/src/subagents/audit.ts b/src/subagents/audit.ts new file mode 100644 index 0000000..a1953e8 --- /dev/null +++ b/src/subagents/audit.ts @@ -0,0 +1,57 @@ +// Audit log for subagent edits. Every create/update/delete from the UI API +// writes a row here so the user can see the history of their subagents. +// Agent-originated edits via the Write tool bypass this path; a future PR may +// add a file watcher to capture those. + +import type { Database } from "bun:sqlite"; + +export type SubagentAuditAction = "create" | "update" | "delete"; + +export type SubagentAuditEntry = { + id: number; + subagent_name: string; + action: SubagentAuditAction; + previous_body: string | null; + new_body: string | null; + actor: string; + created_at: string; +}; + +export function recordSubagentEdit( + db: Database, + params: { + name: string; + action: SubagentAuditAction; + previousBody: string | null; + newBody: string | null; + actor: string; + }, +): void { + db.run( + `INSERT INTO subagent_audit_log (subagent_name, action, previous_body, new_body, actor) + VALUES (?, ?, ?, ?, ?)`, + [params.name, params.action, params.previousBody, params.newBody, params.actor], + ); +} + +export function listSubagentEdits(db: Database, subagentName?: string, limit = 50): SubagentAuditEntry[] { + if (subagentName) { + return db + .query( + `SELECT id, subagent_name, action, previous_body, new_body, actor, created_at + FROM subagent_audit_log + WHERE subagent_name = ? + ORDER BY id DESC + LIMIT ?`, + ) + .all(subagentName, limit) as SubagentAuditEntry[]; + } + return db + .query( + `SELECT id, subagent_name, action, previous_body, new_body, actor, created_at + FROM subagent_audit_log + ORDER BY id DESC + LIMIT ?`, + ) + .all(limit) as SubagentAuditEntry[]; +} diff --git a/src/subagents/frontmatter.ts b/src/subagents/frontmatter.ts new file mode 100644 index 0000000..78ca266 --- /dev/null +++ b/src/subagents/frontmatter.ts @@ -0,0 +1,156 @@ +// Parse and serialize subagent markdown frontmatter. +// +// Format verified against cli.js:6131-6141 (the CLI's own agent file writer) +// and sdk.d.ts:38-76 (AgentDefinition type). The CLI emits: +// +// --- +// name: +// description: "" +// tools: +// model: +// effort: +// color: +// memory: +// --- +// +// +// +// We accept a slightly richer superset on read (max-turns, initial-prompt, +// disallowed-tools) so dashboard-authored agents can express every +// AgentDefinition field that is practical to ship via a markdown file. Unknown +// fields are REJECTED at parse time via Zod .strict() so typos surface loudly. + +import { parse as parseYaml, stringify as stringifyYaml } from "yaml"; +import { z } from "zod"; + +export const SUBAGENT_NAME_PATTERN = /^[a-z][a-z0-9-]{0,63}$/; +export const MAX_BODY_BYTES = 50 * 1024; // 50 KB, matches skills + +const AGENT_COLORS = [ + "red", + "orange", + "yellow", + "green", + "cyan", + "blue", + "purple", + "magenta", + "white", + "gray", +] as const; +export const AgentColorSchema = z.enum(AGENT_COLORS); + +export const AgentEffortSchema = z.enum(["low", "medium", "high"]); + +export const SubagentFrontmatterSchema = z + .object({ + name: z + .string() + .min(1) + .regex(SUBAGENT_NAME_PATTERN, "name must be lowercase letters, digits, and hyphens, starting with a letter"), + description: z.string().min(1, "description is required").max(240), + tools: z.array(z.string().min(1)).optional(), + "disallowed-tools": z.array(z.string().min(1)).optional(), + model: z.string().optional(), + effort: AgentEffortSchema.optional(), + color: AgentColorSchema.optional(), + memory: z.string().max(2000).optional(), + "max-turns": z.number().int().min(1).max(200).optional(), + "initial-prompt": z.string().max(4000).optional(), + }) + .strict(); + +export type SubagentFrontmatter = z.infer; + +export type ParsedSubagent = { + frontmatter: SubagentFrontmatter; + body: string; +}; + +export type ParseResult = { ok: true; parsed: ParsedSubagent } | { ok: false; error: string }; + +export function parseFrontmatter(raw: string): ParseResult { + if (typeof raw !== "string") { + return { ok: false, error: "Input must be a string" }; + } + + const normalized = raw.replace(/^\uFEFF/, ""); + const lines = normalized.split(/\r?\n/); + + if (lines[0]?.trim() !== "---") { + return { ok: false, error: "Subagent file must start with a YAML frontmatter block opened by '---'" }; + } + + let endIndex = -1; + for (let i = 1; i < lines.length; i++) { + if (lines[i].trim() === "---") { + endIndex = i; + break; + } + } + if (endIndex === -1) { + return { ok: false, error: "Subagent frontmatter block is not closed with '---'" }; + } + + const yamlText = lines.slice(1, endIndex).join("\n"); + const body = lines + .slice(endIndex + 1) + .join("\n") + .replace(/^\n+/, ""); + + let yamlParsed: unknown; + try { + yamlParsed = parseYaml(yamlText); + } catch (err: unknown) { + const msg = err instanceof Error ? err.message : String(err); + return { ok: false, error: `Invalid YAML frontmatter: ${msg}` }; + } + + if (yamlParsed == null || typeof yamlParsed !== "object") { + return { ok: false, error: "Frontmatter must be a YAML object" }; + } + + const result = SubagentFrontmatterSchema.safeParse(yamlParsed); + if (!result.success) { + const issue = result.error.issues[0]; + const path = issue.path.length > 0 ? issue.path.join(".") : "frontmatter"; + return { ok: false, error: `${path}: ${issue.message}` }; + } + + return { ok: true, parsed: { frontmatter: result.data, body } }; +} + +export function serializeSubagent(frontmatter: SubagentFrontmatter, body: string): string { + const ordered: Record = {}; + // Field order mirrors the CLI's own agent writer at cli.js:6131-6140. + const orderedKeys: Array = [ + "name", + "description", + "tools", + "disallowed-tools", + "model", + "effort", + "color", + "memory", + "max-turns", + "initial-prompt", + ]; + for (const key of orderedKeys) { + const value = frontmatter[key]; + if (value !== undefined) { + ordered[key] = value; + } + } + + const yaml = stringifyYaml(ordered, { lineWidth: 0, defaultStringType: "PLAIN" }).trimEnd(); + const trimmedBody = body.replace(/^\n+/, "").replace(/\s+$/, ""); + return `---\n${yaml}\n---\n\n${trimmedBody}\n`; +} + +export function getBodyByteLength(body: string): number { + return new TextEncoder().encode(body).byteLength; +} + +export function isBodyWithinLimit(body: string): boolean { + return getBodyByteLength(body) <= MAX_BODY_BYTES; +} diff --git a/src/subagents/linter.ts b/src/subagents/linter.ts new file mode 100644 index 0000000..df4f2f5 --- /dev/null +++ b/src/subagents/linter.ts @@ -0,0 +1,93 @@ +// Lint a subagent file for common mistakes. Advisory only, per the Cardinal +// Rule: the agent (and the operator) is the brain. We surface obvious red +// flags so the user knows what they are shipping. + +import { MAX_BODY_BYTES, type SubagentFrontmatter, getBodyByteLength } from "./frontmatter.ts"; + +export type LintLevel = "info" | "warning" | "error"; + +export type LintHint = { + level: LintLevel; + field: string; + message: string; +}; + +const SHELL_RED_LIST: Array<{ pattern: RegExp; label: string }> = [ + { pattern: /rm\s+-rf\s+\//, label: "rm -rf /" }, + { pattern: /curl[^\n]*\|\s*sh/, label: "curl | sh" }, + { pattern: /wget[^\n]*\|\s*sh/, label: "wget | sh" }, + { pattern: /\|\s*sudo/, label: "pipe to sudo" }, + { pattern: /base64\s+-d[^\n]*\|\s*sh/, label: "base64 -d | sh" }, + { pattern: /chmod\s+777/, label: "chmod 777" }, + { pattern: /eval\s*\(\s*['"]/, label: "eval() with string literal" }, +]; + +export function lintSubagent(frontmatter: SubagentFrontmatter, body: string): LintHint[] { + const hints: LintHint[] = []; + + if (!frontmatter.tools || frontmatter.tools.length === 0) { + hints.push({ + level: "info", + field: "tools", + message: + "No tools set. This subagent will inherit all tools from the parent agent. Consider narrowing the tool list for least-privilege.", + }); + } + + if (frontmatter.description.length < 20) { + hints.push({ + level: "warning", + field: "description", + message: + "description is very short. The Task tool reads this to decide when to invoke the subagent; write something descriptive so it fires at the right moments.", + }); + } + + const bytes = getBodyByteLength(body); + if (bytes > MAX_BODY_BYTES) { + hints.push({ + level: "error", + field: "body", + message: `Body is ${(bytes / 1024).toFixed(1)} KB, over the ${MAX_BODY_BYTES / 1024} KB limit.`, + }); + } else if (bytes > MAX_BODY_BYTES * 0.8) { + hints.push({ + level: "info", + field: "body", + message: `Body is ${(bytes / 1024).toFixed(1)} KB, approaching the ${MAX_BODY_BYTES / 1024} KB limit.`, + }); + } + + for (const { pattern, label } of SHELL_RED_LIST) { + if (pattern.test(body)) { + hints.push({ + level: "warning", + field: "body", + message: `Body contains a pattern often used in destructive shell commands: ${label}.`, + }); + } + } + + if (!/^#\s+/m.test(body)) { + hints.push({ + level: "info", + field: "body", + message: + "Body does not start with a Markdown heading. Conventional subagent prompts use '# Agent name' as the first line.", + }); + } + + if (hints.length === 0) { + hints.push({ + level: "info", + field: "body", + message: "All checks passed.", + }); + } + + return hints; +} + +export function hasBlockingError(hints: LintHint[]): boolean { + return hints.some((h) => h.level === "error"); +} diff --git a/src/subagents/paths.ts b/src/subagents/paths.ts new file mode 100644 index 0000000..c18be14 --- /dev/null +++ b/src/subagents/paths.ts @@ -0,0 +1,65 @@ +// Resolve and validate subagent file paths. +// +// Subagents live at: +// user: ${HOME}/.claude/agents/.md (loaded via settingSources 'user') +// project: ${CWD}/.claude/agents/.md (loaded via settingSources 'project') +// +// Unlike skills (which are directories with SKILL.md inside), subagents are +// flat markdown files. PR3 exposes only the user scope in the dashboard; +// project-scope subagents can still be created by the agent using the Write +// tool, but the dashboard CRUD flow targets the user volume. +// +// Path validation guarantees: +// - names are a strict subset of [a-z0-9][a-z0-9-]* max 64 chars +// - a small reserved-stem list is rejected to avoid collision with CLI +// internal names (advisory, not a hard SDK constraint) +// - the resolved .md path canonically lives under the agents root +// - no null bytes, no relative segments, no symlinks leaking outside + +import { homedir } from "node:os"; +import { resolve } from "node:path"; + +const USER_ENV_OVERRIDE = "PHANTOM_SUBAGENTS_USER_ROOT"; +const NAME_PATTERN = /^[a-z][a-z0-9-]{0,63}$/; +const RESERVED_STEMS = new Set(["agent", "agents", "default", "builtin", "index"]); + +export type SubagentPathResolution = { + root: string; + file: string; +}; + +export function getUserSubagentsRoot(): string { + const override = process.env[USER_ENV_OVERRIDE]; + if (override) { + return resolve(override); + } + return resolve(homedir(), ".claude", "agents"); +} + +export function getProjectSubagentsRoot(cwd: string = process.cwd()): string { + return resolve(cwd, ".claude", "agents"); +} + +export function isValidSubagentName(name: string): boolean { + if (typeof name !== "string") return false; + if (name.includes("\0")) return false; + if (!NAME_PATTERN.test(name)) return false; + if (RESERVED_STEMS.has(name)) return false; + return true; +} + +export function resolveUserSubagentPath(name: string): SubagentPathResolution { + if (!isValidSubagentName(name)) { + throw new Error( + `Invalid subagent name: must match ${NAME_PATTERN.source} and not be one of ${Array.from(RESERVED_STEMS).join(", ")}. Got: ${JSON.stringify(name)}`, + ); + } + const root = getUserSubagentsRoot(); + const file = resolve(root, `${name}.md`); + + if (!file.startsWith(`${root}/`) && file !== `${root}/${name}.md`) { + throw new Error(`Subagent path escape detected: ${file} is not inside ${root}`); + } + + return { root, file }; +} diff --git a/src/subagents/storage.ts b/src/subagents/storage.ts new file mode 100644 index 0000000..8a74a32 --- /dev/null +++ b/src/subagents/storage.ts @@ -0,0 +1,276 @@ +// Storage layer for subagent markdown files under the user-scope agents root. +// +// Unlike skills (directory-per-skill with SKILL.md), subagents are flat files: +// `/.md`. Atomic writes via tmp-then-rename on the same filesystem. +// No file locking: last-write-wins per the Cardinal Rule. tmp+rename still +// protects against torn files on a mid-write crash. + +import { existsSync, mkdirSync, readFileSync, readdirSync, renameSync, rmSync, statSync, writeFileSync } from "node:fs"; +import { dirname, join } from "node:path"; +import { + MAX_BODY_BYTES, + type ParseResult, + type SubagentFrontmatter, + getBodyByteLength, + isBodyWithinLimit, + parseFrontmatter, + serializeSubagent, +} from "./frontmatter.ts"; +import { getUserSubagentsRoot, isValidSubagentName, resolveUserSubagentPath } from "./paths.ts"; + +export type SubagentSummary = { + name: string; + description: string; + path: string; + mtime: string; // ISO + size: number; + has_tools: boolean; + model: string | null; + effort: string | null; + color: string | null; +}; + +export type SubagentDetail = SubagentSummary & { + frontmatter: SubagentFrontmatter; + body: string; + raw: string; +}; + +export type ListResult = { + subagents: SubagentSummary[]; + errors: Array<{ name: string; error: string }>; +}; + +function ensureDir(dir: string): void { + if (!existsSync(dir)) { + mkdirSync(dir, { recursive: true }); + } +} + +function summaryFromParsed( + name: string, + path: string, + raw: string, + frontmatter: SubagentFrontmatter, + mtime: Date, +): SubagentSummary { + return { + name, + description: frontmatter.description, + path, + mtime: mtime.toISOString(), + size: new TextEncoder().encode(raw).byteLength, + has_tools: Array.isArray(frontmatter.tools) && frontmatter.tools.length > 0, + model: frontmatter.model ?? null, + effort: frontmatter.effort ?? null, + color: frontmatter.color ?? null, + }; +} + +export function listSubagents(): ListResult { + const root = getUserSubagentsRoot(); + const errors: Array<{ name: string; error: string }> = []; + const subagents: SubagentSummary[] = []; + + if (!existsSync(root)) { + return { subagents, errors }; + } + + let entries: string[]; + try { + entries = readdirSync(root); + } catch (err: unknown) { + const msg = err instanceof Error ? err.message : String(err); + return { subagents, errors: [{ name: "", error: `Failed to list subagents root: ${msg}` }] }; + } + + for (const entry of entries.sort()) { + if (!entry.endsWith(".md")) continue; + const name = entry.slice(0, -3); + if (!isValidSubagentName(name)) { + continue; + } + const file = join(root, entry); + let raw: string; + let stats: ReturnType; + try { + raw = readFileSync(file, "utf-8"); + stats = statSync(file); + } catch (err: unknown) { + const msg = err instanceof Error ? err.message : String(err); + errors.push({ name, error: `Failed to read: ${msg}` }); + continue; + } + + const parsed = parseFrontmatter(raw); + if (!parsed.ok) { + errors.push({ name, error: parsed.error }); + continue; + } + + subagents.push(summaryFromParsed(name, file, raw, parsed.parsed.frontmatter, stats.mtime)); + } + + subagents.sort((a, b) => b.mtime.localeCompare(a.mtime)); + return { subagents, errors }; +} + +export type ReadResult = { ok: true; subagent: SubagentDetail } | { ok: false; status: 404 | 422 | 500; error: string }; + +export function readSubagent(name: string): ReadResult { + if (!isValidSubagentName(name)) { + return { ok: false, status: 422, error: `Invalid subagent name: ${JSON.stringify(name)}` }; + } + let file: string; + try { + file = resolveUserSubagentPath(name).file; + } catch (err: unknown) { + const msg = err instanceof Error ? err.message : String(err); + return { ok: false, status: 422, error: msg }; + } + if (!existsSync(file)) { + return { ok: false, status: 404, error: `Subagent not found: ${name}` }; + } + let raw: string; + let stats: ReturnType; + try { + raw = readFileSync(file, "utf-8"); + stats = statSync(file); + } catch (err: unknown) { + const msg = err instanceof Error ? err.message : String(err); + return { ok: false, status: 500, error: `Failed to read subagent: ${msg}` }; + } + const parsed: ParseResult = parseFrontmatter(raw); + if (!parsed.ok) { + return { ok: false, status: 422, error: parsed.error }; + } + const summary = summaryFromParsed(name, file, raw, parsed.parsed.frontmatter, stats.mtime); + return { + ok: true, + subagent: { + ...summary, + frontmatter: parsed.parsed.frontmatter, + body: parsed.parsed.body, + raw, + }, + }; +} + +export type WriteResult = + | { ok: true; subagent: SubagentDetail; previousBody: string | null } + | { ok: false; status: 400 | 404 | 409 | 413 | 422 | 500; error: string }; + +export type WriteInput = { + name: string; + frontmatter: SubagentFrontmatter; + body: string; +}; + +function writeAtomic(file: string, content: string): void { + const dir = dirname(file); + ensureDir(dir); + const base = file.split("/").pop() ?? "subagent.md"; + const tmp = join(dir, `.${base}.tmp-${process.pid}-${Date.now()}`); + writeFileSync(tmp, content, { encoding: "utf-8", mode: 0o644 }); + renameSync(tmp, file); +} + +export function writeSubagent(input: WriteInput, options: { mustExist: boolean }): WriteResult { + const { name, frontmatter, body } = input; + + if (!isValidSubagentName(name)) { + return { ok: false, status: 422, error: `Invalid subagent name: ${JSON.stringify(name)}` }; + } + if (frontmatter.name !== name) { + return { + ok: false, + status: 422, + error: `Frontmatter name '${frontmatter.name}' does not match path name '${name}'`, + }; + } + if (!isBodyWithinLimit(body)) { + return { + ok: false, + status: 413, + error: `Body is ${(getBodyByteLength(body) / 1024).toFixed(1)} KB, over the ${MAX_BODY_BYTES / 1024} KB limit.`, + }; + } + + let file: string; + try { + file = resolveUserSubagentPath(name).file; + } catch (err: unknown) { + const msg = err instanceof Error ? err.message : String(err); + return { ok: false, status: 422, error: msg }; + } + + let previousBody: string | null = null; + if (existsSync(file)) { + if (!options.mustExist) { + return { ok: false, status: 409, error: `Subagent already exists: ${name}` }; + } + try { + const prevRaw = readFileSync(file, "utf-8"); + const prevParsed = parseFrontmatter(prevRaw); + if (prevParsed.ok) { + previousBody = prevParsed.parsed.body; + } else { + previousBody = prevRaw; + } + } catch { + previousBody = null; + } + } else if (options.mustExist) { + return { ok: false, status: 404, error: `Subagent not found: ${name}` }; + } + + const serialized = serializeSubagent(frontmatter, body); + + try { + writeAtomic(file, serialized); + } catch (err: unknown) { + const msg = err instanceof Error ? err.message : String(err); + return { ok: false, status: 500, error: `Failed to write subagent: ${msg}` }; + } + + const read = readSubagent(name); + if (!read.ok) { + return { ok: false, status: 500, error: `Write succeeded but read-back failed: ${read.error}` }; + } + return { ok: true, subagent: read.subagent, previousBody }; +} + +export type DeleteResult = + | { ok: true; deleted: string; previousBody: string | null } + | { ok: false; status: 404 | 422 | 500; error: string }; + +export function deleteSubagent(name: string): DeleteResult { + if (!isValidSubagentName(name)) { + return { ok: false, status: 422, error: `Invalid subagent name: ${JSON.stringify(name)}` }; + } + let file: string; + try { + file = resolveUserSubagentPath(name).file; + } catch (err: unknown) { + const msg = err instanceof Error ? err.message : String(err); + return { ok: false, status: 422, error: msg }; + } + if (!existsSync(file)) { + return { ok: false, status: 404, error: `Subagent not found: ${name}` }; + } + let previousBody: string | null = null; + try { + const prevRaw = readFileSync(file, "utf-8"); + const prevParsed = parseFrontmatter(prevRaw); + previousBody = prevParsed.ok ? prevParsed.parsed.body : prevRaw; + } catch { + previousBody = null; + } + try { + rmSync(file, { force: true }); + } catch (err: unknown) { + const msg = err instanceof Error ? err.message : String(err); + return { ok: false, status: 500, error: `Failed to delete subagent: ${msg}` }; + } + return { ok: true, deleted: name, previousBody }; +} diff --git a/src/ui/api/subagents.ts b/src/ui/api/subagents.ts new file mode 100644 index 0000000..2d14b0c --- /dev/null +++ b/src/ui/api/subagents.ts @@ -0,0 +1,240 @@ +// UI API routes for subagents CRUD. +// +// All routes live under /ui/api/subagents and are cookie-auth gated by the +// dispatcher in src/ui/serve.ts. +// +// GET /ui/api/subagents -> list +// GET /ui/api/subagents/:name -> read one +// POST /ui/api/subagents -> create (body: { frontmatter, body }) +// PUT /ui/api/subagents/:name -> update (body: { frontmatter, body }) +// DELETE /ui/api/subagents/:name -> delete +// GET /ui/api/subagents/:name/audit -> edit history for that subagent +// +// JSON bodies in and out. All error responses are { error: string }. + +import type { Database } from "bun:sqlite"; +import { listSubagentEdits, recordSubagentEdit } from "../../subagents/audit.ts"; +import { + MAX_BODY_BYTES, + type SubagentFrontmatter, + SubagentFrontmatterSchema, + getBodyByteLength, +} from "../../subagents/frontmatter.ts"; +import { lintSubagent } from "../../subagents/linter.ts"; +import { + type DeleteResult, + type ReadResult, + type WriteResult, + deleteSubagent, + listSubagents, + readSubagent, + writeSubagent, +} from "../../subagents/storage.ts"; + +type SubagentsApiDeps = { + db: Database; +}; + +function json(body: unknown, init?: ResponseInit): Response { + return new Response(JSON.stringify(body), { + ...init, + headers: { + "Content-Type": "application/json", + "Cache-Control": "no-store", + ...((init?.headers as Record) ?? {}), + }, + }); +} + +function parseWriteBody( + raw: unknown, +): { ok: true; frontmatter: SubagentFrontmatter; body: string } | { ok: false; error: string } { + if (!raw || typeof raw !== "object") { + return { ok: false, error: "Request body must be a JSON object" }; + } + const shape = raw as { frontmatter?: unknown; body?: unknown }; + if (typeof shape.body !== "string") { + return { ok: false, error: "body field must be a string" }; + } + if (shape.frontmatter == null || typeof shape.frontmatter !== "object") { + return { ok: false, error: "frontmatter field must be an object" }; + } + const parsed = SubagentFrontmatterSchema.safeParse(shape.frontmatter); + if (!parsed.success) { + const issue = parsed.error.issues[0]; + const path = issue.path.length > 0 ? issue.path.join(".") : "frontmatter"; + return { ok: false, error: `${path}: ${issue.message}` }; + } + return { ok: true, frontmatter: parsed.data, body: shape.body }; +} + +function readResponse(result: ReadResult): Response { + if (!result.ok) { + return json({ error: result.error }, { status: result.status }); + } + return json({ + subagent: { + name: result.subagent.name, + description: result.subagent.description, + path: result.subagent.path, + mtime: result.subagent.mtime, + size: result.subagent.size, + has_tools: result.subagent.has_tools, + model: result.subagent.model, + effort: result.subagent.effort, + color: result.subagent.color, + frontmatter: result.subagent.frontmatter, + body: result.subagent.body, + lint: lintSubagent(result.subagent.frontmatter, result.subagent.body), + }, + }); +} + +function writeResponse(result: WriteResult): Response { + if (!result.ok) { + return json({ error: result.error }, { status: result.status }); + } + return json({ + subagent: { + name: result.subagent.name, + description: result.subagent.description, + path: result.subagent.path, + mtime: result.subagent.mtime, + size: result.subagent.size, + has_tools: result.subagent.has_tools, + model: result.subagent.model, + effort: result.subagent.effort, + color: result.subagent.color, + frontmatter: result.subagent.frontmatter, + body: result.subagent.body, + lint: lintSubagent(result.subagent.frontmatter, result.subagent.body), + }, + }); +} + +function deleteResponse(result: DeleteResult): Response { + if (!result.ok) { + return json({ error: result.error }, { status: result.status }); + } + return json({ deleted: result.deleted }); +} + +async function readJson(req: Request): Promise { + try { + return await req.json(); + } catch (err: unknown) { + const msg = err instanceof Error ? err.message : String(err); + return { __error: `Invalid JSON body: ${msg}` }; + } +} + +export async function handleSubagentsApi(req: Request, url: URL, deps: SubagentsApiDeps): Promise { + const pathname = url.pathname; + + // GET /ui/api/subagents + if (pathname === "/ui/api/subagents" && req.method === "GET") { + const result = listSubagents(); + return json({ + subagents: result.subagents, + errors: result.errors, + limits: { max_body_bytes: MAX_BODY_BYTES }, + }); + } + + // POST /ui/api/subagents + if (pathname === "/ui/api/subagents" && req.method === "POST") { + const body = await readJson(req); + if (body && typeof body === "object" && "__error" in body) { + return json({ error: (body as { __error: string }).__error }, { status: 400 }); + } + const parsed = parseWriteBody(body); + if (!parsed.ok) { + return json({ error: parsed.error }, { status: 422 }); + } + const result = writeSubagent( + { name: parsed.frontmatter.name, frontmatter: parsed.frontmatter, body: parsed.body }, + { mustExist: false }, + ); + if (result.ok) { + recordSubagentEdit(deps.db, { + name: result.subagent.name, + action: "create", + previousBody: null, + newBody: result.subagent.body, + actor: "user", + }); + } + return writeResponse(result); + } + + // /ui/api/subagents/:name/audit + const auditMatch = pathname.match(/^\/ui\/api\/subagents\/([^/]+)\/audit$/); + if (auditMatch && req.method === "GET") { + const name = decodeURIComponent(auditMatch[1]); + const entries = listSubagentEdits(deps.db, name, 50); + return json({ entries }); + } + + // /ui/api/subagents/:name + const match = pathname.match(/^\/ui\/api\/subagents\/([^/]+)$/); + if (match) { + const name = decodeURIComponent(match[1]); + + if (req.method === "GET") { + return readResponse(readSubagent(name)); + } + + if (req.method === "PUT") { + const body = await readJson(req); + if (body && typeof body === "object" && "__error" in body) { + return json({ error: (body as { __error: string }).__error }, { status: 400 }); + } + const parsed = parseWriteBody(body); + if (!parsed.ok) { + return json({ error: parsed.error }, { status: 422 }); + } + if (parsed.frontmatter.name !== name) { + return json( + { error: `Frontmatter name '${parsed.frontmatter.name}' does not match path name '${name}'` }, + { status: 422 }, + ); + } + const bytes = getBodyByteLength(parsed.body); + if (bytes > MAX_BODY_BYTES) { + return json( + { error: `Body is ${(bytes / 1024).toFixed(1)} KB, over the ${MAX_BODY_BYTES / 1024} KB limit.` }, + { status: 413 }, + ); + } + const result = writeSubagent({ name, frontmatter: parsed.frontmatter, body: parsed.body }, { mustExist: true }); + if (result.ok) { + recordSubagentEdit(deps.db, { + name, + action: "update", + previousBody: result.previousBody, + newBody: result.subagent.body, + actor: "user", + }); + } + return writeResponse(result); + } + + if (req.method === "DELETE") { + const result = deleteSubagent(name); + if (result.ok) { + recordSubagentEdit(deps.db, { + name, + action: "delete", + previousBody: result.previousBody, + newBody: null, + actor: "user", + }); + } + return deleteResponse(result); + } + + return json({ error: "Method not allowed" }, { status: 405 }); + } + + return null; +} diff --git a/src/ui/serve.ts b/src/ui/serve.ts index c437861..ef1b829 100644 --- a/src/ui/serve.ts +++ b/src/ui/serve.ts @@ -9,6 +9,7 @@ import { getSecretRequest, saveSecrets, validateMagicToken } from "../secrets/st import { handleMemoryFilesApi } from "./api/memory-files.ts"; import { type PluginsApiDeps, handlePluginsApi } from "./api/plugins.ts"; import { handleSkillsApi } from "./api/skills.ts"; +import { handleSubagentsApi } from "./api/subagents.ts"; const COOKIE_NAME = "phantom_session"; const COOKIE_MAX_AGE = 7 * 24 * 60 * 60; // 7 days in seconds @@ -170,6 +171,13 @@ export async function handleUiRequest(req: Request): Promise { const apiResponse = await handlePluginsApi(req, url, { db: dashboardDb, ...pluginsApiOverrides }); if (apiResponse) return apiResponse; } + if (url.pathname.startsWith("/ui/api/subagents")) { + if (!dashboardDb) { + return Response.json({ error: "Dashboard API not initialized" }, { status: 503 }); + } + const apiResponse = await handleSubagentsApi(req, url, { db: dashboardDb }); + if (apiResponse) return apiResponse; + } // Static files const filePath = isPathSafe(url.pathname); From 810cd0e4f2c3c8aa4471dbbb34c6f4c5441eef06 Mon Sep 17 00:00:00 2001 From: Muhammad Ahmed Cheema Date: Mon, 13 Apr 2026 22:19:04 -0700 Subject: [PATCH 4/6] feat: hooks tab with visual rule builder for 26 sdk events The breakthrough surface of PR3. The first visual hooks editor in the Claude Code ecosystem. Three-column Linear-Automations-style builder: pick a trigger (any of 26 hook events), pick a matcher (tool name, subagent name, MCP server name, filename glob - depending on the event), pick an action (command, prompt, agent, or http with type-specific forms). Live preview pane renders the JSON slice. Safety floor is strict: - Zod .strict() discriminated union per hook type, timeouts bounded 1-3600 seconds, http URLs URL-validated, env var names matching [A-Z_][A-Z0-9_]* - All writes route through src/plugins/settings-io.ts for atomic tmp+rename. The hooks editor writes ONLY the Settings.hooks slice; enabledPlugins and every other field stay byte-for-byte identical. A regression test locks this in. - allowedHttpHookUrls allowlist is enforced on http hook install; bad URLs return 403 with a clear error. - Trust modal on first install before any rule lands. Acceptance is recorded to hook_audit_log so it persists across sessions. - Every install, update, uninstall, and trust acceptance writes an audit row with the full previous and new slice as JSON so a human can diff and recover. Shipping: - src/hooks/{paths,schema,storage,audit}.ts - src/ui/api/hooks.ts with six cookie-auth-gated routes - public/dashboard/hooks.js with the builder, list view, trust modal, and audit timeline - dashboard.css tokens for the three-column builder - hook_audit_log migration - Sidebar entry and route div in index.html 42 new hooks tests. PR1 skills (39) and PR2 plugins (84) tests pass byte-for-byte. --- public/dashboard/dashboard.css | 114 +++++ public/dashboard/dashboard.js | 4 +- public/dashboard/hooks.js | 657 ++++++++++++++++++++++++++++ public/dashboard/index.html | 11 +- src/db/__tests__/migrate.test.ts | 12 +- src/db/schema.ts | 19 + src/hooks/__tests__/schema.test.ts | 202 +++++++++ src/hooks/__tests__/storage.test.ts | 272 ++++++++++++ src/hooks/audit.ts | 68 +++ src/hooks/paths.ts | 9 + src/hooks/schema.ts | 150 +++++++ src/hooks/storage.ts | 250 +++++++++++ src/ui/api/hooks.ts | 187 ++++++++ src/ui/serve.ts | 8 + 14 files changed, 1951 insertions(+), 12 deletions(-) create mode 100644 public/dashboard/hooks.js create mode 100644 src/hooks/__tests__/schema.test.ts create mode 100644 src/hooks/__tests__/storage.test.ts create mode 100644 src/hooks/audit.ts create mode 100644 src/hooks/paths.ts create mode 100644 src/hooks/schema.ts create mode 100644 src/hooks/storage.ts create mode 100644 src/ui/api/hooks.ts diff --git a/public/dashboard/dashboard.css b/public/dashboard/dashboard.css index 60c50d1..cfa4afd 100644 --- a/public/dashboard/dashboard.css +++ b/public/dashboard/dashboard.css @@ -1383,3 +1383,117 @@ body { line-height: 1.5; color: color-mix(in oklab, var(--color-base-content) 65%, transparent); } + +/* Hooks tab (PR3): visual rule builder and event-grouped card list. */ +.dash-hook-list { display: flex; flex-direction: column; gap: var(--space-5); } +.dash-hook-event-section { + border: 1px solid var(--color-base-300); + border-radius: var(--radius-lg); + background: var(--color-base-100); + padding: var(--space-4); +} +.dash-hook-event-header { margin-bottom: var(--space-3); } +.dash-hook-event-title { + font-size: 14px; + font-weight: 600; + color: var(--color-base-content); + margin: 0 0 4px; + font-family: 'JetBrains Mono', monospace; +} +.dash-hook-event-summary { + font-size: 12px; + color: color-mix(in oklab, var(--color-base-content) 60%, transparent); + margin: 0; +} +.dash-hook-event-cards { display: flex; flex-direction: column; gap: var(--space-2); } + +.dash-hook-builder { + border: 1px solid var(--color-base-300); + border-radius: var(--radius-lg); + background: var(--color-base-100); + padding: var(--space-5); +} +.dash-hook-builder-header { + display: flex; + align-items: center; + justify-content: space-between; + margin-bottom: var(--space-4); +} +.dash-hook-builder-title { + font-size: 16px; + font-weight: 600; + margin: 0; +} +.dash-hook-columns { + display: grid; + grid-template-columns: repeat(3, 1fr); + gap: var(--space-4); + margin-bottom: var(--space-4); +} +@media (max-width: 960px) { .dash-hook-columns { grid-template-columns: 1fr; } } +.dash-hook-column { + border: 1px solid var(--color-base-300); + border-radius: var(--radius-md); + padding: var(--space-3); + background: var(--color-base-200); +} +.dash-hook-column-eyebrow { + font-size: 10px; + font-weight: 600; + letter-spacing: 0.09em; + text-transform: uppercase; + color: color-mix(in oklab, var(--color-base-content) 50%, transparent); + margin-bottom: var(--space-1); +} +.dash-hook-column-title { + font-size: 13px; + font-weight: 600; + margin: 0 0 var(--space-3); +} +.dash-hook-column-help { + font-size: 11.5px; + color: color-mix(in oklab, var(--color-base-content) 60%, transparent); + margin-top: var(--space-2); + line-height: 1.5; +} +.dash-hook-column-disabled { + font-size: 12px; + color: color-mix(in oklab, var(--color-base-content) 50%, transparent); + font-style: italic; +} +.dash-hook-action-form { display: flex; flex-direction: column; gap: var(--space-3); margin-top: var(--space-3); } +.dash-hook-preview { + border: 1px solid var(--color-base-300); + border-radius: var(--radius-md); + background: var(--color-base-200); + padding: var(--space-3); +} +.dash-hook-preview-summary { + cursor: pointer; + font-size: 11px; + font-weight: 600; + text-transform: uppercase; + letter-spacing: 0.08em; + color: color-mix(in oklab, var(--color-base-content) 55%, transparent); + user-select: none; +} +.dash-hook-preview-code { + margin-top: var(--space-3); + font-family: 'JetBrains Mono', monospace; + font-size: 11.5px; + line-height: 1.55; + background: var(--color-base-100); + border-radius: var(--radius-sm); + padding: var(--space-3); + overflow-x: auto; + color: var(--color-base-content); +} +.dash-audit-row { + padding: 8px 10px; + border: 1px solid var(--color-base-300); + border-radius: var(--radius-sm); + background: var(--color-base-100); + margin-bottom: 6px; +} +.dash-audit-row-top { font-size: 12px; } +.dash-audit-row-body { font-size: 11px; color: color-mix(in oklab, var(--color-base-content) 60%, transparent); margin-top: 4px; } diff --git a/public/dashboard/dashboard.js b/public/dashboard/dashboard.js index 3307109..aa2d5f4 100644 --- a/public/dashboard/dashboard.js +++ b/public/dashboard/dashboard.js @@ -251,8 +251,8 @@ var name = parsed.route; deactivateAllRoutes(); - var liveRoutes = ["skills", "memory-files", "plugins", "subagents"]; - var comingSoon = ["sessions", "cost", "scheduler", "evolution", "memory", "settings", "hooks"]; + var liveRoutes = ["skills", "memory-files", "plugins", "subagents", "hooks"]; + var comingSoon = ["sessions", "cost", "scheduler", "evolution", "memory", "settings"]; if (liveRoutes.indexOf(name) >= 0 && routes[name]) { var containerId = "route-" + name; diff --git a/public/dashboard/hooks.js b/public/dashboard/hooks.js new file mode 100644 index 0000000..439fcaf --- /dev/null +++ b/public/dashboard/hooks.js @@ -0,0 +1,657 @@ +// Hooks tab: visual rule builder for the 26 Claude Agent SDK hook events. +// +// Module contract: registers with PhantomDashboard via registerRoute('hooks'). +// mount(container, arg, ctx) is called on hash change. ctx provides esc, api, +// toast, openModal, navigate, setBreadcrumb, registerDirtyChecker. +// +// The hooks tab is the breakthrough surface of PR3. No Claude Code product has +// a visual hooks rule builder; this is the first one. The design takes after +// Linear Automations: a three-column flow (trigger, matcher, action) with a +// type-specific form for the action column and a live JSON preview pane. +// +// State model is a single closure-scoped object; no framework. Every edit +// mutates state.draftHook and schedules a re-render of the preview pane. + +(function () { + var HOOK_EVENTS = [ + "PreToolUse", "PostToolUse", "PostToolUseFailure", "Notification", + "UserPromptSubmit", "SessionStart", "SessionEnd", "Stop", "StopFailure", + "SubagentStart", "SubagentStop", "PreCompact", "PostCompact", + "PermissionRequest", "Setup", "TeammateIdle", "TaskCreated", "TaskCompleted", + "Elicitation", "ElicitationResult", "ConfigChange", + "WorktreeCreate", "WorktreeRemove", "InstructionsLoaded", + "CwdChanged", "FileChanged", + ]; + + var EVENTS_WITH_MATCHER = { + PreToolUse: "Tool name (e.g. Bash, Write, Edit)", + PostToolUse: "Tool name", + PostToolUseFailure: "Tool name", + SubagentStart: "Subagent name", + SubagentStop: "Subagent name", + Elicitation: "MCP server name", + ElicitationResult: "MCP server name", + ConfigChange: "Settings source (user_settings, project_settings, local_settings, policy_settings, skills)", + InstructionsLoaded: "Memory type or load reason", + FileChanged: "Filename glob (e.g. .envrc|.env)", + }; + + var EVENT_SUMMARIES = { + PreToolUse: "Fires before a tool call is dispatched. Exit 2 blocks the call.", + PostToolUse: "Fires after a tool call succeeds.", + PostToolUseFailure: "Fires after a tool call throws.", + Notification: "Fires on user-facing notifications.", + UserPromptSubmit: "Fires when the user hits enter. Exit 2 blocks the send.", + SessionStart: "Fires when a new session starts.", + SessionEnd: "Fires when a session ends normally.", + Stop: "Fires when the agent voluntarily stops.", + StopFailure: "Fires when the agent crashes out.", + SubagentStart: "Fires when a subagent is invoked via the Task tool.", + SubagentStop: "Fires when a subagent finishes.", + PreCompact: "Fires before the transcript is auto-compacted.", + PostCompact: "Fires after compaction.", + PermissionRequest: "Fires when the CLI asks to approve a dangerous op.", + Setup: "Fires once on first-time setup.", + TeammateIdle: "Fires when a team channel teammate stops sending.", + TaskCreated: "Fires when a background task is scheduled.", + TaskCompleted: "Fires when a background task completes.", + Elicitation: "Fires when an MCP server requests user input.", + ElicitationResult: "Fires after the user answers an elicitation.", + ConfigChange: "Fires when any settings source mutates.", + WorktreeCreate: "Fires when a git worktree is created.", + WorktreeRemove: "Fires when a git worktree is removed.", + InstructionsLoaded: "Fires when a CLAUDE.md or rules file loads.", + CwdChanged: "Fires when the working directory changes.", + FileChanged: "Fires when a watched file changes.", + }; + + var HOOK_TYPES = [ + { value: "command", label: "Shell command", help: "Run a shell command. Exit code 2 blocks the event." }, + { value: "prompt", label: "LLM prompt", help: "Evaluate the hook input with a prompt on a small, fast model." }, + { value: "agent", label: "Agent verifier", help: "Run a full subagent that decides whether to approve." }, + { value: "http", label: "HTTP POST", help: "POST the hook input JSON to a URL." }, + ]; + + var state = { + slice: {}, + total: 0, + allowedHttpHookUrls: null, + trustAccepted: false, + auditEntries: [], + loading: false, + initialized: false, + editing: null, // null | { mode: 'new' | 'edit', event, groupIndex, hookIndex, draft } + }; + var ctx = null; + var root = null; + + function esc(s) { return ctx.esc(s); } + + function blankDraft() { + return { + event: "PreToolUse", + matcher: "", + definition: { + type: "command", + command: "", + timeout: 30, + statusMessage: "", + once: false, + async: false, + asyncRewake: false, + }, + }; + } + + function renderHeader() { + var subtitle = state.total === 0 + ? "No hooks installed. Add a rule below to react to any of 26 events." + : state.total + " hook" + (state.total === 1 ? "" : "s") + " installed across the agent's event surface."; + return ( + '
' + + '

Hooks

' + + '

Hooks

' + + '

' + esc(subtitle) + '

' + + '
' + + '' + + '' + + '
' + + '
' + ); + } + + function renderHookCard(event, groupIndex, matcher, hookIndex, def) { + var typeBadge = '' + esc(def.type) + ''; + var matcherChip = matcher ? 'matcher: ' + esc(matcher) + '' : ""; + var titleForType = { + command: def.command, + prompt: def.prompt, + agent: def.prompt, + http: def.url, + }[def.type] || ""; + return ( + '
' + + '
' + + '

' + esc(event) + '

' + + typeBadge + + '
' + + '

' + esc(String(titleForType).slice(0, 140)) + '

' + + '
' + + matcherChip + + (def.once ? 'once' : "") + + (def.async ? 'async' : "") + + '' + + '' + + '
' + + '
' + ); + } + + function renderHookList() { + var eventsWithHooks = Object.keys(state.slice).filter(function (ev) { return (state.slice[ev] || []).length > 0; }); + if (eventsWithHooks.length === 0) { + return ( + '
' + + '' + + '

No hooks yet

' + + '

Add a rule to run a command before every Bash call, format files after an edit, or fire a webhook on task completion. Every install runs through a trust modal on first use.

' + + '' + + '
' + ); + } + var sections = []; + eventsWithHooks.forEach(function (ev) { + var cards = []; + (state.slice[ev] || []).forEach(function (group, gi) { + (group.hooks || []).forEach(function (def, hi) { + cards.push(renderHookCard(ev, gi, group.matcher, hi, def)); + }); + }); + sections.push( + '
' + + '
' + + '

' + esc(ev) + '

' + + '

' + esc(EVENT_SUMMARIES[ev] || "") + '

' + + '
' + + '
' + cards.join("") + '
' + + '
' + ); + }); + return sections.join(""); + } + + function renderPreview() { + if (!state.editing) return ""; + var preview = {}; + var ev = state.editing.draft.event; + preview[ev] = [ + { + matcher: state.editing.draft.matcher || undefined, + hooks: [cleanDefinition(state.editing.draft.definition)], + }, + ]; + if (!preview[ev][0].matcher) delete preview[ev][0].matcher; + return JSON.stringify(preview, null, 2); + } + + function cleanDefinition(def) { + var out = {}; + Object.keys(def).forEach(function (k) { + var v = def[k]; + if (v === "" || v === null || v === undefined) return; + if (v === false && (k === "once" || k === "async" || k === "asyncRewake")) return; + out[k] = v; + }); + return out; + } + + function renderTypeOptions(current) { + return HOOK_TYPES.map(function (t) { + return ''; + }).join(""); + } + + function renderEventOptions(current) { + return HOOK_EVENTS.map(function (ev) { + return ''; + }).join(""); + } + + function renderActionForm(def) { + var t = def.type; + var parts = []; + if (t === "command") { + parts.push( + '
' + + '' + + '' + + '
Runs in bash by default. Exit code 2 blocks the event.
' + + '
' + + '
' + + '
' + + '
' + + '
' + + '
' + + '' + + '' + + '
' + ); + } else if (t === "prompt" || t === "agent") { + parts.push( + '
' + + '' + + '' + + '
$ARGUMENTS is replaced with the hook input JSON. Under 4000 chars.
' + + '
' + + '
' + + '
' + + '
' + + '
' + + '' + ); + } else if (t === "http") { + parts.push( + '
' + + '' + + '' + + '
Must match one of the allowed HTTP hook URL patterns in settings.json.
' + + '
' + + '
' + + '' + + '' + + '
' + + '
' + ); + } + return parts.join(""); + } + + function headersToText(headers) { + if (!headers) return ""; + return Object.keys(headers).map(function (k) { return k + ": " + headers[k]; }).join("\n"); + } + + function parseHeadersText(text) { + if (!text) return null; + var lines = text.split(/\r?\n/).map(function (l) { return l.trim(); }).filter(Boolean); + var out = {}; + for (var i = 0; i < lines.length; i++) { + var idx = lines[i].indexOf(":"); + if (idx < 0) continue; + out[lines[i].slice(0, idx).trim()] = lines[i].slice(idx + 1).trim(); + } + return Object.keys(out).length > 0 ? out : null; + } + + function renderBuilder() { + if (!state.editing) return ""; + var draft = state.editing.draft; + var matcherSupported = Object.prototype.hasOwnProperty.call(EVENTS_WITH_MATCHER, draft.event); + var matcherPlaceholder = EVENTS_WITH_MATCHER[draft.event] || ""; + return ( + '
' + + '
' + + '

' + (state.editing.mode === "new" ? "New rule" : "Edit rule") + '

' + + '
' + + '' + + '' + + '
' + + '
' + + + '
' + + + '
' + + '
1. Trigger
' + + '

When

' + + '' + + '

' + esc(EVENT_SUMMARIES[draft.event] || "") + '

' + + '
' + + + '
' + + '
2. Matcher
' + + '

For which

' + + (matcherSupported + ? '

Leave blank to match every invocation. Supports literal names and regex patterns the CLI interprets.

' + : '
This event does not accept a matcher. Leave blank.
' + ) + + '
' + + + '
' + + '
3. Action
' + + '

Do

' + + '' + + '
' + renderActionForm(draft.definition) + '
' + + '
' + + + '
' + + + '
' + + 'Preview settings.json slice' + + '
' + esc(renderPreview()) + '
' + + '
' + + + '
' + ); + } + + function render() { + var body = state.editing + ? renderBuilder() + : '
' + renderHookList() + '
'; + root.innerHTML = renderHeader() + body; + wireEvents(); + if (state.editing) { + wireBuilder(); + updateSaveEnabled(); + } + ctx.setBreadcrumb("Hooks"); + } + + function wireEvents() { + var newBtn = document.getElementById("hooks-new-btn"); + if (newBtn) newBtn.addEventListener("click", startNewRule); + var newBtnEmpty = document.getElementById("hooks-new-btn-empty"); + if (newBtnEmpty) newBtnEmpty.addEventListener("click", startNewRule); + var auditBtn = document.getElementById("hooks-audit-btn"); + if (auditBtn) auditBtn.addEventListener("click", showAuditPanel); + + document.querySelectorAll("[data-hook-edit]").forEach(function (btn) { + btn.addEventListener("click", function () { + var coords = btn.getAttribute("data-hook-edit").split("/"); + startEditRule(coords[0], parseInt(coords[1], 10), parseInt(coords[2], 10)); + }); + }); + document.querySelectorAll("[data-hook-delete]").forEach(function (btn) { + btn.addEventListener("click", function () { + var coords = btn.getAttribute("data-hook-delete").split("/"); + confirmDelete(coords[0], parseInt(coords[1], 10), parseInt(coords[2], 10)); + }); + }); + } + + function wireBuilder() { + var eventSel = document.getElementById("hook-event"); + var matcherInput = document.getElementById("hook-matcher"); + var typeSel = document.getElementById("hook-type"); + + if (eventSel) eventSel.addEventListener("change", function () { + state.editing.draft.event = eventSel.value; + if (!Object.prototype.hasOwnProperty.call(EVENTS_WITH_MATCHER, eventSel.value)) { + state.editing.draft.matcher = ""; + } + render(); + }); + if (matcherInput) matcherInput.addEventListener("input", function () { + state.editing.draft.matcher = matcherInput.value; + updatePreview(); + updateSaveEnabled(); + }); + if (typeSel) typeSel.addEventListener("change", function () { + var newType = typeSel.value; + state.editing.draft.definition = defaultDefinition(newType); + render(); + }); + wireActionFields(); + + var saveBtn = document.getElementById("hook-save-btn"); + if (saveBtn) saveBtn.addEventListener("click", saveRule); + var cancelBtn = document.getElementById("hook-cancel-btn"); + if (cancelBtn) cancelBtn.addEventListener("click", function () { + state.editing = null; + render(); + }); + } + + function wireActionFields() { + var def = state.editing.draft.definition; + var t = def.type; + if (t === "command") { + bindInput("hook-command", function (v) { def.command = v; }); + bindInput("hook-timeout", function (v) { def.timeout = v ? parseInt(v, 10) : undefined; }); + bindSelect("hook-shell", function (v) { def.shell = v || undefined; }); + bindCheckbox("hook-once", function (v) { def.once = v; }); + bindCheckbox("hook-async", function (v) { def.async = v; }); + } else if (t === "prompt" || t === "agent") { + bindInput("hook-prompt", function (v) { def.prompt = v; }); + bindInput("hook-timeout", function (v) { def.timeout = v ? parseInt(v, 10) : undefined; }); + bindInput("hook-model", function (v) { def.model = v || undefined; }); + bindCheckbox("hook-once", function (v) { def.once = v; }); + } else if (t === "http") { + bindInput("hook-url", function (v) { def.url = v; }); + bindInput("hook-headers", function (v) { def.headers = parseHeadersText(v); }); + bindInput("hook-timeout", function (v) { def.timeout = v ? parseInt(v, 10) : undefined; }); + } + } + + function bindInput(id, setter) { + var el = document.getElementById(id); + if (!el) return; + el.addEventListener("input", function () { setter(el.value); updatePreview(); updateSaveEnabled(); }); + } + function bindSelect(id, setter) { + var el = document.getElementById(id); + if (!el) return; + el.addEventListener("change", function () { setter(el.value); updatePreview(); updateSaveEnabled(); }); + } + function bindCheckbox(id, setter) { + var el = document.getElementById(id); + if (!el) return; + el.addEventListener("change", function () { setter(el.checked); updatePreview(); updateSaveEnabled(); }); + } + + function updatePreview() { + var pre = document.querySelector(".dash-hook-preview-code"); + if (pre) pre.textContent = renderPreview(); + } + + function defaultDefinition(type) { + if (type === "command") return { type: "command", command: "", timeout: 30, once: false, async: false }; + if (type === "prompt") return { type: "prompt", prompt: "", timeout: 60, once: false }; + if (type === "agent") return { type: "agent", prompt: "", timeout: 60, once: false }; + if (type === "http") return { type: "http", url: "", headers: null, timeout: 10 }; + return { type: "command", command: "" }; + } + + function validateDraft() { + if (!state.editing) return false; + var d = state.editing.draft; + if (!d.event) return false; + if (d.definition.type === "command" && (!d.definition.command || !d.definition.command.trim())) return false; + if ((d.definition.type === "prompt" || d.definition.type === "agent") && (!d.definition.prompt || !d.definition.prompt.trim())) return false; + if (d.definition.type === "http") { + if (!d.definition.url) return false; + try { new URL(d.definition.url); } catch (_) { return false; } + } + return true; + } + + function updateSaveEnabled() { + var btn = document.getElementById("hook-save-btn"); + if (btn) btn.disabled = !validateDraft(); + } + + function startNewRule() { + if (!state.trustAccepted) { + showTrustModal(function () { + state.editing = { mode: "new", draft: blankDraft() }; + render(); + }); + return; + } + state.editing = { mode: "new", draft: blankDraft() }; + render(); + } + + function startEditRule(event, groupIndex, hookIndex) { + var group = (state.slice[event] || [])[groupIndex]; + if (!group) return; + var def = (group.hooks || [])[hookIndex]; + if (!def) return; + state.editing = { + mode: "edit", + event: event, + groupIndex: groupIndex, + hookIndex: hookIndex, + draft: { event: event, matcher: group.matcher || "", definition: JSON.parse(JSON.stringify(def)) }, + }; + render(); + } + + function saveRule() { + if (!validateDraft()) return; + var d = state.editing.draft; + var cleaned = cleanDefinition(d.definition); + cleaned.type = d.definition.type; + + var promise; + if (state.editing.mode === "new") { + promise = ctx.api("POST", "/ui/api/hooks", { + event: d.event, + matcher: d.matcher || undefined, + definition: cleaned, + }); + } else { + promise = ctx.api("PUT", "/ui/api/hooks/" + encodeURIComponent(state.editing.event) + "/" + state.editing.groupIndex + "/" + state.editing.hookIndex, { + definition: cleaned, + }); + } + + promise.then(function (res) { + state.slice = res.slice || {}; + recomputeTotal(); + state.editing = null; + ctx.toast("success", "Rule saved", "Takes effect on the agent's next message."); + return loadList(); + }).catch(function (err) { + ctx.toast("error", "Save failed", err.message || String(err)); + }); + } + + function confirmDelete(event, groupIndex, hookIndex) { + ctx.openModal({ + title: "Delete hook?", + body: "Remove this " + event + " hook. It stops firing on the agent's next message.", + actions: [ + { label: "Cancel", className: "dash-btn-ghost", onClick: function () {} }, + { + label: "Delete", + className: "dash-btn-danger", + onClick: function () { + return ctx.api("DELETE", "/ui/api/hooks/" + encodeURIComponent(event) + "/" + groupIndex + "/" + hookIndex) + .then(function (res) { + state.slice = res.slice || {}; + recomputeTotal(); + ctx.toast("success", "Deleted", "Hook removed."); + return loadList(); + }) + .catch(function (err) { + ctx.toast("error", "Delete failed", err.message || String(err)); + return false; + }); + }, + }, + ], + }); + } + + function showTrustModal(onAccept) { + var body = document.createElement("div"); + body.innerHTML = ( + '

Hooks give the agent execution power outside the LLM turn. Before you install your first one, read this:

' + + '
    ' + + '
  • Command hooks run arbitrary shell commands under the agent user.
  • ' + + '
  • HTTP hooks POST the hook input JSON to a URL. Env vars are only substituted into headers when listed in allowedEnvVars.
  • ' + + '
  • Prompt and agent hooks run a small model call on the hook input.
  • ' + + '
  • Every hook you install is audited. You can delete any hook at any time.
  • ' + + '
  • The agent itself can add or remove hooks via its Write tool. The dashboard captures dashboard-originated edits only.
  • ' + + '
' + + '

By clicking Accept, you acknowledge that hook execution power is real and you are taking responsibility for what you install.

' + ); + ctx.openModal({ + title: "Before you install hooks", + body: body, + actions: [ + { label: "Cancel", className: "dash-btn-ghost", onClick: function () {} }, + { + label: "Accept and continue", + className: "dash-btn-primary", + onClick: function () { + return ctx.api("POST", "/ui/api/hooks/trust", {}) + .then(function () { + state.trustAccepted = true; + onAccept(); + }) + .catch(function (err) { + ctx.toast("error", "Could not record trust", err.message || String(err)); + return false; + }); + }, + }, + ], + }); + } + + function showAuditPanel() { + ctx.api("GET", "/ui/api/hooks/audit").then(function (res) { + var entries = res.entries || []; + var body = document.createElement("div"); + body.style.maxHeight = "60vh"; + body.style.overflowY = "auto"; + body.innerHTML = entries.length === 0 + ? '

No audit entries yet.

' + : entries.map(function (e) { + return ( + '
' + + '
' + + '' + esc(e.created_at) + '' + + ' ' + esc(e.action) + '' + + ' on ' + esc(e.event) + '' + + (e.matcher ? ' (matcher: ' + esc(e.matcher) + ')' : '') + + '
' + + (e.hook_type ? '
type: ' + esc(e.hook_type) + '
' : '') + + '
' + ); + }).join(""); + ctx.openModal({ + title: "Hooks audit", + body: body, + actions: [{ label: "Close", className: "dash-btn-ghost", onClick: function () {} }], + }); + }).catch(function (err) { + ctx.toast("error", "Failed to load audit", err.message || String(err)); + }); + } + + function recomputeTotal() { + var total = 0; + Object.values(state.slice).forEach(function (groups) { + (groups || []).forEach(function (g) { total += (g.hooks || []).length; }); + }); + state.total = total; + } + + function loadList() { + return ctx.api("GET", "/ui/api/hooks").then(function (res) { + state.slice = res.slice || {}; + state.total = res.total || 0; + state.allowedHttpHookUrls = res.allowed_http_hook_urls; + state.trustAccepted = !!res.trust_accepted; + render(); + }).catch(function (err) { + ctx.toast("error", "Failed to load hooks", err.message || String(err)); + }); + } + + function mount(container, _arg, dashCtx) { + ctx = dashCtx; + root = container; + ctx.setBreadcrumb("Hooks"); + if (!state.initialized) { + ctx.registerDirtyChecker(function () { return state.editing != null; }); + state.initialized = true; + } + return loadList(); + } + + window.PhantomDashboard.registerRoute("hooks", { mount: mount }); +})(); diff --git a/public/dashboard/index.html b/public/dashboard/index.html index bbf0a72..3f2d560 100644 --- a/public/dashboard/index.html +++ b/public/dashboard/index.html @@ -52,6 +52,10 @@ Subagents + + + Hooks +
Coming soon
@@ -81,11 +85,6 @@ Memory explorer soon - - - Hooks - soon - Settings @@ -104,6 +103,7 @@ + @@ -116,6 +116,7 @@ + diff --git a/src/db/__tests__/migrate.test.ts b/src/db/__tests__/migrate.test.ts index effbe1c..85228b7 100644 --- a/src/db/__tests__/migrate.test.ts +++ b/src/db/__tests__/migrate.test.ts @@ -35,10 +35,10 @@ describe("runMigrations", () => { runMigrations(db); const migrationCount = db.query("SELECT COUNT(*) as count FROM _migrations").get() as { count: number }; - // PR3 adds a subagent_audit_log table and index (commit 3). The hooks - // and settings audit tables land in commits 4 and 5 and bump this count - // further. - expect(migrationCount.count).toBe(18); + // PR3 adds three audit tables and their indices: subagent_audit_log + // (commit 3), hook_audit_log (commit 4), and settings_audit_log + // (commit 5). Each adds 2 migration steps (table + index). + expect(migrationCount.count).toBe(20); }); test("tracks applied migration indices", () => { @@ -50,6 +50,8 @@ describe("runMigrations", () => { .all() .map((r) => (r as { index_num: number }).index_num); - expect(indices).toEqual([0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17]); + expect(indices).toEqual([ + 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, + ]); }); }); diff --git a/src/db/schema.ts b/src/db/schema.ts index 9de4d9d..20db28c 100644 --- a/src/db/schema.ts +++ b/src/db/schema.ts @@ -167,4 +167,23 @@ export const MIGRATIONS: string[] = [ )`, "CREATE INDEX IF NOT EXISTS idx_subagent_audit_log_name ON subagent_audit_log(subagent_name, id DESC)", + + // PR3 dashboard: hooks editor audit log. Captures every install, update, + // uninstall, and first-install trust acceptance via the UI API. Each row + // stores the full previous and new hooks slice as JSON so a human can + // diff and recover. Agent-originated Write tool edits bypass this path. + `CREATE TABLE IF NOT EXISTS hook_audit_log ( + id INTEGER PRIMARY KEY AUTOINCREMENT, + event TEXT NOT NULL, + matcher TEXT, + hook_type TEXT, + action TEXT NOT NULL, + previous_slice TEXT, + new_slice TEXT, + definition_json TEXT, + actor TEXT NOT NULL, + created_at TEXT NOT NULL DEFAULT (datetime('now')) + )`, + + "CREATE INDEX IF NOT EXISTS idx_hook_audit_log_created ON hook_audit_log(id DESC)", ]; diff --git a/src/hooks/__tests__/schema.test.ts b/src/hooks/__tests__/schema.test.ts new file mode 100644 index 0000000..f037c2b --- /dev/null +++ b/src/hooks/__tests__/schema.test.ts @@ -0,0 +1,202 @@ +import { describe, expect, test } from "bun:test"; +import { + EVENTS_SUPPORTING_MATCHER, + HOOK_EVENTS, + HookDefinitionSchema, + HookEventSchema, + HookMatcherGroupSchema, + HooksSliceSchema, + isHttpUrlAllowed, +} from "../schema.ts"; + +describe("HOOK_EVENTS", () => { + test("contains exactly 26 events per sdk.d.ts:551", () => { + expect(HOOK_EVENTS.length).toBe(26); + }); + + test("includes the pre/post tool use events", () => { + expect(HOOK_EVENTS).toContain("PreToolUse"); + expect(HOOK_EVENTS).toContain("PostToolUse"); + expect(HOOK_EVENTS).toContain("PostToolUseFailure"); + }); + + test("includes subagent and session events", () => { + expect(HOOK_EVENTS).toContain("SubagentStart"); + expect(HOOK_EVENTS).toContain("SubagentStop"); + expect(HOOK_EVENTS).toContain("SessionStart"); + expect(HOOK_EVENTS).toContain("SessionEnd"); + }); +}); + +describe("EVENTS_SUPPORTING_MATCHER", () => { + test("includes tool-use events", () => { + expect(EVENTS_SUPPORTING_MATCHER.has("PreToolUse")).toBe(true); + expect(EVENTS_SUPPORTING_MATCHER.has("PostToolUse")).toBe(true); + }); + + test("does not include session or notification events", () => { + expect(EVENTS_SUPPORTING_MATCHER.has("SessionStart")).toBe(false); + expect(EVENTS_SUPPORTING_MATCHER.has("Notification")).toBe(false); + }); +}); + +describe("HookDefinitionSchema: command", () => { + test("accepts a minimal command hook", () => { + const r = HookDefinitionSchema.safeParse({ type: "command", command: "echo hi" }); + expect(r.success).toBe(true); + }); + + test("accepts a command hook with all optional fields", () => { + const r = HookDefinitionSchema.safeParse({ + type: "command", + command: "bash -c 'echo hi'", + shell: "bash", + timeout: 60, + statusMessage: "Running precheck", + once: true, + async: false, + asyncRewake: false, + }); + expect(r.success).toBe(true); + }); + + test("rejects empty command", () => { + const r = HookDefinitionSchema.safeParse({ type: "command", command: "" }); + expect(r.success).toBe(false); + }); + + test("rejects unknown fields via strict", () => { + const r = HookDefinitionSchema.safeParse({ type: "command", command: "x", unknown_field: 1 }); + expect(r.success).toBe(false); + }); + + test("rejects timeout over 3600", () => { + const r = HookDefinitionSchema.safeParse({ type: "command", command: "x", timeout: 7200 }); + expect(r.success).toBe(false); + }); +}); + +describe("HookDefinitionSchema: prompt", () => { + test("accepts a valid prompt hook", () => { + const r = HookDefinitionSchema.safeParse({ type: "prompt", prompt: "Evaluate this." }); + expect(r.success).toBe(true); + }); + + test("rejects empty prompt", () => { + const r = HookDefinitionSchema.safeParse({ type: "prompt", prompt: "" }); + expect(r.success).toBe(false); + }); +}); + +describe("HookDefinitionSchema: agent", () => { + test("accepts a valid agent hook", () => { + const r = HookDefinitionSchema.safeParse({ type: "agent", prompt: "Verify tests passed." }); + expect(r.success).toBe(true); + }); +}); + +describe("HookDefinitionSchema: http", () => { + test("accepts a valid http hook", () => { + const r = HookDefinitionSchema.safeParse({ + type: "http", + url: "https://hooks.example.com/event", + headers: { "X-Source": "phantom" }, + }); + expect(r.success).toBe(true); + }); + + test("rejects non-URL", () => { + const r = HookDefinitionSchema.safeParse({ type: "http", url: "not-a-url" }); + expect(r.success).toBe(false); + }); + + test("rejects env var names with lowercase", () => { + const r = HookDefinitionSchema.safeParse({ + type: "http", + url: "https://x.example.com/", + allowedEnvVars: ["my_token"], + }); + expect(r.success).toBe(false); + }); + + test("accepts allcaps env var names", () => { + const r = HookDefinitionSchema.safeParse({ + type: "http", + url: "https://x.example.com/", + allowedEnvVars: ["MY_TOKEN"], + }); + expect(r.success).toBe(true); + }); +}); + +describe("HookMatcherGroupSchema", () => { + test("accepts a group with one command hook", () => { + const r = HookMatcherGroupSchema.safeParse({ + matcher: "Bash", + hooks: [{ type: "command", command: "echo" }], + }); + expect(r.success).toBe(true); + }); + + test("rejects a group with empty hooks array", () => { + const r = HookMatcherGroupSchema.safeParse({ matcher: "Bash", hooks: [] }); + expect(r.success).toBe(false); + }); + + test("accepts a group with no matcher (undefined)", () => { + const r = HookMatcherGroupSchema.safeParse({ hooks: [{ type: "command", command: "echo" }] }); + expect(r.success).toBe(true); + }); +}); + +describe("HooksSliceSchema", () => { + test("accepts a full slice with two events", () => { + const r = HooksSliceSchema.safeParse({ + PreToolUse: [{ matcher: "Bash", hooks: [{ type: "command", command: "echo" }] }], + PostToolUse: [{ matcher: "Write", hooks: [{ type: "command", command: "format.sh" }] }], + }); + expect(r.success).toBe(true); + }); + + test("rejects unknown event names", () => { + const r = HooksSliceSchema.safeParse({ + NotARealEvent: [{ hooks: [{ type: "command", command: "echo" }] }], + }); + expect(r.success).toBe(false); + }); +}); + +describe("HookEventSchema", () => { + test("accepts every known event", () => { + for (const ev of HOOK_EVENTS) { + expect(HookEventSchema.safeParse(ev).success).toBe(true); + } + }); +}); + +describe("isHttpUrlAllowed", () => { + test("allows any URL when allowlist is undefined", () => { + expect(isHttpUrlAllowed("https://any.example.com/x", undefined)).toBe(true); + }); + + test("blocks all URLs when allowlist is empty array", () => { + expect(isHttpUrlAllowed("https://any.example.com/x", [])).toBe(false); + }); + + test("allows exact match", () => { + expect(isHttpUrlAllowed("https://hooks.example.com/x", ["https://hooks.example.com/x"])).toBe(true); + }); + + test("allows wildcard match", () => { + expect(isHttpUrlAllowed("https://hooks.example.com/event", ["https://hooks.example.com/*"])).toBe(true); + }); + + test("blocks non-match", () => { + expect(isHttpUrlAllowed("https://evil.example.com/x", ["https://hooks.example.com/*"])).toBe(false); + }); + + test("escapes regex metachars in the pattern", () => { + expect(isHttpUrlAllowed("https://a.com/x", ["https://a.com/x"])).toBe(true); + expect(isHttpUrlAllowed("https://aXcom/x", ["https://a.com/x"])).toBe(false); + }); +}); diff --git a/src/hooks/__tests__/storage.test.ts b/src/hooks/__tests__/storage.test.ts new file mode 100644 index 0000000..0ceb719 --- /dev/null +++ b/src/hooks/__tests__/storage.test.ts @@ -0,0 +1,272 @@ +import { afterEach, beforeEach, describe, expect, test } from "bun:test"; +import { mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { installHook, listHooks, uninstallHook, updateHook } from "../storage.ts"; + +let tmp: string; +let settingsPath: string; + +function writeSettings(obj: unknown): void { + writeFileSync(settingsPath, `${JSON.stringify(obj, null, 2)}\n`); +} + +beforeEach(() => { + tmp = mkdtempSync(join(tmpdir(), "phantom-hooks-")); + settingsPath = join(tmp, "settings.json"); +}); + +afterEach(() => { + rmSync(tmp, { recursive: true, force: true }); +}); + +describe("listHooks", () => { + test("returns empty slice when settings.json does not exist", () => { + const result = listHooks(settingsPath); + expect(result.ok).toBe(true); + if (!result.ok) return; + expect(result.slice).toEqual({}); + expect(result.total).toBe(0); + }); + + test("returns a populated slice", () => { + writeSettings({ + enabledPlugins: { "linear@claude-plugins-official": true }, + hooks: { + PreToolUse: [{ matcher: "Bash", hooks: [{ type: "command", command: "echo pre" }] }], + }, + }); + const result = listHooks(settingsPath); + expect(result.ok).toBe(true); + if (!result.ok) return; + expect(result.total).toBe(1); + expect(result.slice.PreToolUse?.[0].matcher).toBe("Bash"); + }); +}); + +describe("installHook: slice-only write", () => { + test("install does not touch enabledPlugins", () => { + writeSettings({ + enabledPlugins: { "linear@claude-plugins-official": true, "notion@claude-plugins-official": true }, + permissions: { allow: ["Bash(git:*)"], deny: [] }, + model: "claude-opus-4-6", + x_custom_field: "preserved byte-for-byte", + }); + const result = installHook( + { + event: "PreToolUse", + matcher: "Bash", + definition: { type: "command", command: "echo precheck" }, + }, + settingsPath, + ); + expect(result.ok).toBe(true); + const after = JSON.parse(readFileSync(settingsPath, "utf-8")); + expect(after.enabledPlugins).toEqual({ + "linear@claude-plugins-official": true, + "notion@claude-plugins-official": true, + }); + expect(after.permissions).toEqual({ allow: ["Bash(git:*)"], deny: [] }); + expect(after.model).toBe("claude-opus-4-6"); + expect(after.x_custom_field).toBe("preserved byte-for-byte"); + expect(after.hooks.PreToolUse[0].matcher).toBe("Bash"); + expect(after.hooks.PreToolUse[0].hooks[0].command).toBe("echo precheck"); + }); + + test("install appends to an existing matcher group", () => { + writeSettings({ + hooks: { + PreToolUse: [{ matcher: "Bash", hooks: [{ type: "command", command: "first" }] }], + }, + }); + const result = installHook( + { + event: "PreToolUse", + matcher: "Bash", + definition: { type: "command", command: "second" }, + }, + settingsPath, + ); + expect(result.ok).toBe(true); + if (!result.ok) return; + expect(result.slice.PreToolUse?.[0].hooks.length).toBe(2); + expect(result.slice.PreToolUse?.[0].hooks[1]).toMatchObject({ type: "command", command: "second" }); + }); + + test("install creates a new matcher group when matcher differs", () => { + writeSettings({ + hooks: { + PreToolUse: [{ matcher: "Bash", hooks: [{ type: "command", command: "first" }] }], + }, + }); + const result = installHook( + { + event: "PreToolUse", + matcher: "Write", + definition: { type: "command", command: "format.sh" }, + }, + settingsPath, + ); + expect(result.ok).toBe(true); + if (!result.ok) return; + expect(result.slice.PreToolUse?.length).toBe(2); + }); + + test("install refuses http hooks outside the allowlist", () => { + writeSettings({ + allowedHttpHookUrls: ["https://hooks.example.com/*"], + }); + const result = installHook( + { + event: "PostToolUse", + definition: { type: "http", url: "https://evil.com/hook" }, + }, + settingsPath, + ); + expect(result.ok).toBe(false); + if (result.ok) return; + expect(result.status).toBe(403); + }); + + test("install accepts http hooks matching the allowlist wildcard", () => { + writeSettings({ + allowedHttpHookUrls: ["https://hooks.example.com/*"], + }); + const result = installHook( + { + event: "PostToolUse", + definition: { type: "http", url: "https://hooks.example.com/deploy" }, + }, + settingsPath, + ); + expect(result.ok).toBe(true); + }); +}); + +describe("updateHook", () => { + test("replaces the hook in place", () => { + writeSettings({ + hooks: { + PreToolUse: [ + { + matcher: "Bash", + hooks: [ + { type: "command", command: "old" }, + { type: "command", command: "also-old" }, + ], + }, + ], + }, + }); + const result = updateHook( + { + event: "PreToolUse", + groupIndex: 0, + hookIndex: 1, + definition: { type: "command", command: "new" }, + }, + settingsPath, + ); + expect(result.ok).toBe(true); + if (!result.ok) return; + expect(result.slice.PreToolUse?.[0].hooks[0]).toMatchObject({ command: "old" }); + expect(result.slice.PreToolUse?.[0].hooks[1]).toMatchObject({ command: "new" }); + }); + + test("returns 404 for an out-of-range group", () => { + writeSettings({ + hooks: { + PreToolUse: [{ matcher: "Bash", hooks: [{ type: "command", command: "x" }] }], + }, + }); + const result = updateHook( + { + event: "PreToolUse", + groupIndex: 5, + hookIndex: 0, + definition: { type: "command", command: "y" }, + }, + settingsPath, + ); + expect(result.ok).toBe(false); + if (result.ok) return; + expect(result.status).toBe(404); + }); +}); + +describe("uninstallHook", () => { + test("removes the hook and preserves siblings", () => { + writeSettings({ + enabledPlugins: { "linear@claude-plugins-official": true }, + hooks: { + PreToolUse: [ + { + matcher: "Bash", + hooks: [ + { type: "command", command: "keep" }, + { type: "command", command: "remove" }, + ], + }, + ], + }, + }); + const result = uninstallHook({ event: "PreToolUse", groupIndex: 0, hookIndex: 1 }, settingsPath); + expect(result.ok).toBe(true); + if (!result.ok) return; + expect(result.slice.PreToolUse?.[0].hooks.length).toBe(1); + expect(result.slice.PreToolUse?.[0].hooks[0]).toMatchObject({ command: "keep" }); + // enabledPlugins preserved + const after = JSON.parse(readFileSync(settingsPath, "utf-8")); + expect(after.enabledPlugins).toEqual({ "linear@claude-plugins-official": true }); + }); + + test("removes the group when the last hook is uninstalled", () => { + writeSettings({ + hooks: { + PreToolUse: [{ matcher: "Bash", hooks: [{ type: "command", command: "only" }] }], + }, + }); + const result = uninstallHook({ event: "PreToolUse", groupIndex: 0, hookIndex: 0 }, settingsPath); + expect(result.ok).toBe(true); + if (!result.ok) return; + expect(result.slice.PreToolUse).toBeUndefined(); + }); + + test("returns 404 for missing hook", () => { + writeSettings({ hooks: {} }); + const result = uninstallHook({ event: "PreToolUse", groupIndex: 0, hookIndex: 0 }, settingsPath); + expect(result.ok).toBe(false); + if (result.ok) return; + expect(result.status).toBe(404); + }); +}); + +describe("byte-for-byte preservation of PR2 fields", () => { + test("a full install/uninstall cycle leaves enabledPlugins identical", () => { + const enabledBefore = { + "linear@claude-plugins-official": true, + "notion@claude-plugins-official": true, + "slack@claude-plugins-official": { version: "1.2.3" }, + "claude-md-management@claude-plugins-official": false, + }; + writeSettings({ enabledPlugins: enabledBefore }); + + const install = installHook( + { + event: "UserPromptSubmit", + definition: { type: "prompt", prompt: "Evaluate whether the request is safe." }, + }, + settingsPath, + ); + expect(install.ok).toBe(true); + + let after = JSON.parse(readFileSync(settingsPath, "utf-8")); + expect(after.enabledPlugins).toEqual(enabledBefore); + + const uninstall = uninstallHook({ event: "UserPromptSubmit", groupIndex: 0, hookIndex: 0 }, settingsPath); + expect(uninstall.ok).toBe(true); + + after = JSON.parse(readFileSync(settingsPath, "utf-8")); + expect(after.enabledPlugins).toEqual(enabledBefore); + }); +}); diff --git a/src/hooks/audit.ts b/src/hooks/audit.ts new file mode 100644 index 0000000..5cdd58e --- /dev/null +++ b/src/hooks/audit.ts @@ -0,0 +1,68 @@ +// Audit log for hook installs, updates, uninstalls via the UI API. Each row +// captures the full previous and new hooks slice as JSON so a human can diff +// and recover. Agent-originated edits via the Write tool bypass this path. + +import type { Database } from "bun:sqlite"; +import type { HookDefinition, HookEvent, HooksSlice } from "./schema.ts"; + +export type HookAuditAction = "install" | "update" | "uninstall" | "trust_accepted"; + +export type HookAuditEntry = { + id: number; + event: string; + matcher: string | null; + hook_type: string | null; + action: HookAuditAction; + previous_slice: string | null; + new_slice: string | null; + definition_json: string | null; + actor: string; + created_at: string; +}; + +export function recordHookEdit( + db: Database, + params: { + event: HookEvent | ""; + matcher: string | undefined; + hookType: HookDefinition["type"] | null; + action: HookAuditAction; + previousSlice: HooksSlice | null; + newSlice: HooksSlice | null; + definition: HookDefinition | null; + actor: string; + }, +): void { + db.run( + `INSERT INTO hook_audit_log (event, matcher, hook_type, action, previous_slice, new_slice, definition_json, actor) + VALUES (?, ?, ?, ?, ?, ?, ?, ?)`, + [ + params.event, + params.matcher ?? null, + params.hookType, + params.action, + params.previousSlice ? JSON.stringify(params.previousSlice) : null, + params.newSlice ? JSON.stringify(params.newSlice) : null, + params.definition ? JSON.stringify(params.definition) : null, + params.actor, + ], + ); +} + +export function listHookAudit(db: Database, limit = 50): HookAuditEntry[] { + return db + .query( + `SELECT id, event, matcher, hook_type, action, previous_slice, new_slice, definition_json, actor, created_at + FROM hook_audit_log + ORDER BY id DESC + LIMIT ?`, + ) + .all(limit) as HookAuditEntry[]; +} + +export function hasAcceptedHookTrust(db: Database): boolean { + const row = db.query("SELECT COUNT(*) as count FROM hook_audit_log WHERE action = 'trust_accepted'").get() as { + count: number; + } | null; + return (row?.count ?? 0) > 0; +} diff --git a/src/hooks/paths.ts b/src/hooks/paths.ts new file mode 100644 index 0000000..575d63f --- /dev/null +++ b/src/hooks/paths.ts @@ -0,0 +1,9 @@ +// Path helper for the settings.json file that holds the hooks slice. +// Reuses the plugins paths helper so both PR2 and PR3 read from the same +// canonical /home/phantom/.claude/settings.json. + +import { getUserSettingsPath } from "../plugins/paths.ts"; + +export function getHooksSettingsPath(): string { + return getUserSettingsPath(); +} diff --git a/src/hooks/schema.ts b/src/hooks/schema.ts new file mode 100644 index 0000000..f1c2abe --- /dev/null +++ b/src/hooks/schema.ts @@ -0,0 +1,150 @@ +// Zod schemas for the Claude Agent SDK hooks slice of settings.json. +// +// Authoritative source: sdk.d.ts:2736-2861 (Settings.hooks field in the CLI's +// own TypeScript surface). The schema models the 26 hook events (sdk.d.ts:551) +// and the four hook type discriminated union (command | prompt | agent | http), +// per matcher group, per event. +// +// Design principles: +// 1. .strict() on every object so unknown fields surface as validation errors +// (protects against typos that silently do nothing). +// 2. timeouts bounded to 1-3600 seconds to prevent denial-of-service via +// runaway hooks. +// 3. env var names validated as [A-Z_][A-Z0-9_]* so header interpolation +// cannot smuggle in shell expressions. +// 4. http URLs validated as full URLs at parse time. + +import { z } from "zod"; + +export const HOOK_EVENTS = [ + "PreToolUse", + "PostToolUse", + "PostToolUseFailure", + "Notification", + "UserPromptSubmit", + "SessionStart", + "SessionEnd", + "Stop", + "StopFailure", + "SubagentStart", + "SubagentStop", + "PreCompact", + "PostCompact", + "PermissionRequest", + "Setup", + "TeammateIdle", + "TaskCreated", + "TaskCompleted", + "Elicitation", + "ElicitationResult", + "ConfigChange", + "WorktreeCreate", + "WorktreeRemove", + "InstructionsLoaded", + "CwdChanged", + "FileChanged", +] as const; + +export type HookEvent = (typeof HOOK_EVENTS)[number]; +export const HookEventSchema = z.enum(HOOK_EVENTS); + +// Events that accept a tool-name or source-name matcher per sdk.d.ts +// descriptions. Events not in this set ignore the matcher field. +export const EVENTS_SUPPORTING_MATCHER: ReadonlySet = new Set([ + "PreToolUse", + "PostToolUse", + "PostToolUseFailure", + "SubagentStart", + "SubagentStop", + "Elicitation", + "ElicitationResult", + "ConfigChange", + "InstructionsLoaded", + "FileChanged", +]); + +const timeoutSchema = z.number().int().min(1).max(3600); +const envVarNameSchema = z.string().regex(/^[A-Z_][A-Z0-9_]*$/, "env var names must match [A-Z_][A-Z0-9_]*"); + +const CommandHookSchema = z + .object({ + type: z.literal("command"), + command: z.string().min(1).max(10_000), + shell: z.enum(["bash", "powershell"]).optional(), + timeout: timeoutSchema.optional(), + statusMessage: z.string().max(120).optional(), + once: z.boolean().optional(), + async: z.boolean().optional(), + asyncRewake: z.boolean().optional(), + }) + .strict(); + +const PromptHookSchema = z + .object({ + type: z.literal("prompt"), + prompt: z.string().min(1).max(4000), + timeout: timeoutSchema.optional(), + model: z.string().optional(), + statusMessage: z.string().max(120).optional(), + once: z.boolean().optional(), + }) + .strict(); + +const AgentHookSchema = z + .object({ + type: z.literal("agent"), + prompt: z.string().min(1).max(4000), + timeout: timeoutSchema.optional(), + model: z.string().optional(), + statusMessage: z.string().max(120).optional(), + once: z.boolean().optional(), + }) + .strict(); + +const HttpHookSchema = z + .object({ + type: z.literal("http"), + url: z.string().url(), + timeout: timeoutSchema.optional(), + headers: z.record(z.string(), z.string()).optional(), + allowedEnvVars: z.array(envVarNameSchema).optional(), + statusMessage: z.string().max(120).optional(), + once: z.boolean().optional(), + }) + .strict(); + +export const HookDefinitionSchema = z.discriminatedUnion("type", [ + CommandHookSchema, + PromptHookSchema, + AgentHookSchema, + HttpHookSchema, +]); + +export type HookDefinition = z.infer; + +export const HookMatcherGroupSchema = z + .object({ + matcher: z.string().optional(), + hooks: z.array(HookDefinitionSchema).min(1), + }) + .strict(); + +export type HookMatcherGroup = z.infer; + +export const HooksSliceSchema = z.record(HookEventSchema, z.array(HookMatcherGroupSchema)); +export type HooksSlice = z.infer; + +// Match an http hook URL against an operator-provided allowlist. +// Allowlist entries may contain * wildcards. An empty allowlist means +// the field is unset and every URL is allowed (matches CLI default). +// If the allowlist is set to an empty array, no http hooks are allowed. +export function isHttpUrlAllowed(url: string, allowlist?: string[]): boolean { + if (!allowlist) return true; + if (allowlist.length === 0) return false; + for (const pattern of allowlist) { + const escaped = pattern.replace(/[.+?^${}()|[\]\\]/g, "\\$&").replace(/\*/g, ".*"); + const re = new RegExp(`^${escaped}$`); + if (re.test(url)) return true; + } + return false; +} diff --git a/src/hooks/storage.ts b/src/hooks/storage.ts new file mode 100644 index 0000000..570c7b0 --- /dev/null +++ b/src/hooks/storage.ts @@ -0,0 +1,250 @@ +// Storage for the hooks slice of settings.json. Every write goes through +// src/plugins/settings-io.ts for atomic tmp+rename so no other field can be +// accidentally clobbered. The hooks editor ONLY touches Settings.hooks; every +// other key (enabledPlugins, permissions, model, etc.) is preserved +// byte-for-byte on a round trip. +// +// Concurrency: last-write-wins per the Cardinal Rule. Agent-originated edits +// via the Write tool bypass this path; if the agent edits hooks between the +// dashboard's read and the dashboard's write, the dashboard overwrites. An +// audit log row captures the previous slice so a human can diff and recover. + +import { readSettings, writeSettings } from "../plugins/settings-io.ts"; +import { getHooksSettingsPath } from "./paths.ts"; +import { + type HookDefinition, + type HookEvent, + type HookMatcherGroup, + type HooksSlice, + HooksSliceSchema, + isHttpUrlAllowed, +} from "./schema.ts"; + +export type ListHooksResult = + | { ok: true; slice: HooksSlice; total: number; allowedHttpHookUrls: string[] | undefined } + | { ok: false; error: string }; + +export function listHooks(settingsPath: string = getHooksSettingsPath()): ListHooksResult { + const read = readSettings(settingsPath); + if (!read.ok) { + return { ok: false, error: read.error }; + } + const rawSlice = (read.settings.hooks ?? {}) as unknown; + const parsed = HooksSliceSchema.safeParse(rawSlice); + if (!parsed.success) { + return { ok: false, error: `On-disk hooks slice is invalid: ${parsed.error.issues[0].message}` }; + } + let total = 0; + for (const groups of Object.values(parsed.data)) { + for (const group of groups ?? []) { + total += group.hooks.length; + } + } + const allowedHttpHookUrls = Array.isArray(read.settings.allowedHttpHookUrls) + ? (read.settings.allowedHttpHookUrls as string[]) + : undefined; + return { ok: true, slice: parsed.data, total, allowedHttpHookUrls }; +} + +export type InstallHookInput = { + event: HookEvent; + matcher?: string; + definition: HookDefinition; +}; + +export type InstallHookResult = + | { + ok: true; + slice: HooksSlice; + event: HookEvent; + matcher?: string; + groupIndex: number; + hookIndex: number; + previousSlice: HooksSlice; + } + | { ok: false; status: 400 | 403 | 422 | 500; error: string }; + +// Install a new hook. Appends to an existing matcher group with the same +// matcher, or creates a new matcher group if none exists for that matcher. +// Writes ONLY the Settings.hooks slice back; all other keys preserved. +export function installHook(input: InstallHookInput, settingsPath: string = getHooksSettingsPath()): InstallHookResult { + const read = readSettings(settingsPath); + if (!read.ok) return { ok: false, status: 500, error: read.error }; + + const prevRaw = (read.settings.hooks ?? {}) as unknown; + const prevParsed = HooksSliceSchema.safeParse(prevRaw); + if (!prevParsed.success) { + return { ok: false, status: 500, error: `On-disk hooks slice is invalid: ${prevParsed.error.issues[0].message}` }; + } + const previousSlice = prevParsed.data; + + // allowlist enforcement for http hooks + if (input.definition.type === "http") { + const allowlist = Array.isArray(read.settings.allowedHttpHookUrls) + ? (read.settings.allowedHttpHookUrls as string[]) + : undefined; + if (!isHttpUrlAllowed(input.definition.url, allowlist)) { + return { + ok: false, + status: 403, + error: `HTTP hook URL ${input.definition.url} is not on the allowedHttpHookUrls allowlist. Add it to settings.json first.`, + }; + } + } + + const nextSlice: HooksSlice = JSON.parse(JSON.stringify(previousSlice)); + const groupsForEvent: HookMatcherGroup[] = (nextSlice[input.event] as HookMatcherGroup[] | undefined) ?? []; + + // Find an existing group with the same matcher. Treat undefined matcher + // as its own category (the "no matcher" group). + let groupIndex = groupsForEvent.findIndex((g) => (g.matcher ?? null) === (input.matcher ?? null)); + if (groupIndex === -1) { + groupsForEvent.push({ + matcher: input.matcher, + hooks: [input.definition], + }); + groupIndex = groupsForEvent.length - 1; + } else { + groupsForEvent[groupIndex].hooks.push(input.definition); + } + nextSlice[input.event] = groupsForEvent; + + const validated = HooksSliceSchema.safeParse(nextSlice); + if (!validated.success) { + return { + ok: false, + status: 422, + error: `Hook validation failed: ${validated.error.issues[0].path.join(".")}: ${validated.error.issues[0].message}`, + }; + } + + const merged = { ...read.settings, hooks: validated.data }; + const write = writeSettings(merged, settingsPath); + if (!write.ok) { + return { ok: false, status: 500, error: write.error }; + } + + const hookIndex = (validated.data[input.event]?.[groupIndex]?.hooks.length ?? 1) - 1; + return { + ok: true, + slice: validated.data, + event: input.event, + matcher: input.matcher, + groupIndex, + hookIndex, + previousSlice, + }; +} + +export type UpdateHookInput = { + event: HookEvent; + groupIndex: number; + hookIndex: number; + definition: HookDefinition; +}; + +export type UpdateHookResult = + | { ok: true; slice: HooksSlice; previousSlice: HooksSlice } + | { ok: false; status: 404 | 403 | 422 | 500; error: string }; + +export function updateHook(input: UpdateHookInput, settingsPath: string = getHooksSettingsPath()): UpdateHookResult { + const read = readSettings(settingsPath); + if (!read.ok) return { ok: false, status: 500, error: read.error }; + + const prevParsed = HooksSliceSchema.safeParse((read.settings.hooks ?? {}) as unknown); + if (!prevParsed.success) { + return { ok: false, status: 500, error: `On-disk hooks slice is invalid: ${prevParsed.error.issues[0].message}` }; + } + const previousSlice = prevParsed.data; + const nextSlice: HooksSlice = JSON.parse(JSON.stringify(previousSlice)); + const groups = nextSlice[input.event]; + if (!groups || groups.length <= input.groupIndex || !groups[input.groupIndex]) { + return { ok: false, status: 404, error: `No matcher group at ${input.event}[${input.groupIndex}]` }; + } + const group = groups[input.groupIndex]; + if (!group.hooks || group.hooks.length <= input.hookIndex) { + return { + ok: false, + status: 404, + error: `No hook at ${input.event}[${input.groupIndex}].hooks[${input.hookIndex}]`, + }; + } + + if (input.definition.type === "http") { + const allowlist = Array.isArray(read.settings.allowedHttpHookUrls) + ? (read.settings.allowedHttpHookUrls as string[]) + : undefined; + if (!isHttpUrlAllowed(input.definition.url, allowlist)) { + return { + ok: false, + status: 403, + error: `HTTP hook URL ${input.definition.url} is not on the allowedHttpHookUrls allowlist.`, + }; + } + } + + group.hooks[input.hookIndex] = input.definition; + const validated = HooksSliceSchema.safeParse(nextSlice); + if (!validated.success) { + return { + ok: false, + status: 422, + error: `Hook validation failed: ${validated.error.issues[0].message}`, + }; + } + const merged = { ...read.settings, hooks: validated.data }; + const write = writeSettings(merged, settingsPath); + if (!write.ok) return { ok: false, status: 500, error: write.error }; + + return { ok: true, slice: validated.data, previousSlice }; +} + +export type UninstallHookInput = { + event: HookEvent; + groupIndex: number; + hookIndex: number; +}; + +export type UninstallHookResult = + | { ok: true; slice: HooksSlice; previousSlice: HooksSlice } + | { ok: false; status: 404 | 500; error: string }; + +export function uninstallHook( + input: UninstallHookInput, + settingsPath: string = getHooksSettingsPath(), +): UninstallHookResult { + const read = readSettings(settingsPath); + if (!read.ok) return { ok: false, status: 500, error: read.error }; + + const prevParsed = HooksSliceSchema.safeParse((read.settings.hooks ?? {}) as unknown); + if (!prevParsed.success) { + return { ok: false, status: 500, error: `On-disk hooks slice is invalid: ${prevParsed.error.issues[0].message}` }; + } + const previousSlice = prevParsed.data; + const nextSlice: HooksSlice = JSON.parse(JSON.stringify(previousSlice)); + const groups = nextSlice[input.event]; + if (!groups || groups.length <= input.groupIndex || !groups[input.groupIndex]) { + return { ok: false, status: 404, error: `No matcher group at ${input.event}[${input.groupIndex}]` }; + } + const group = groups[input.groupIndex]; + if (!group.hooks || group.hooks.length <= input.hookIndex) { + return { + ok: false, + status: 404, + error: `No hook at ${input.event}[${input.groupIndex}].hooks[${input.hookIndex}]`, + }; + } + + group.hooks.splice(input.hookIndex, 1); + if (group.hooks.length === 0) { + groups.splice(input.groupIndex, 1); + } + if (groups.length === 0) { + delete nextSlice[input.event]; + } + const merged = { ...read.settings, hooks: nextSlice }; + const write = writeSettings(merged, settingsPath); + if (!write.ok) return { ok: false, status: 500, error: write.error }; + + return { ok: true, slice: nextSlice, previousSlice }; +} diff --git a/src/ui/api/hooks.ts b/src/ui/api/hooks.ts new file mode 100644 index 0000000..545e2f5 --- /dev/null +++ b/src/ui/api/hooks.ts @@ -0,0 +1,187 @@ +// UI API routes for the hooks tab. Cookie-auth gated at serve.ts. +// +// GET /ui/api/hooks -> list slice + allowlist + trust state + count +// POST /ui/api/hooks -> install (body: { event, matcher?, definition }) +// PUT /ui/api/hooks/:event/:g/:h -> update the hook at position (g, h) +// DELETE /ui/api/hooks/:event/:g/:h -> uninstall +// POST /ui/api/hooks/trust -> record first-install trust acceptance +// GET /ui/api/hooks/audit -> audit timeline +// +// JSON in, JSON out. All writes route through src/plugins/settings-io.ts which +// writes only the slice the caller touches; every other settings.json field +// stays byte-for-byte identical. + +import type { Database } from "bun:sqlite"; +import { hasAcceptedHookTrust, listHookAudit, recordHookEdit } from "../../hooks/audit.ts"; +import { HookDefinitionSchema, type HookEvent, HookEventSchema } from "../../hooks/schema.ts"; +import { installHook, listHooks, uninstallHook, updateHook } from "../../hooks/storage.ts"; + +type HooksApiDeps = { + db: Database; + settingsPath?: string; +}; + +function json(body: unknown, init?: ResponseInit): Response { + return new Response(JSON.stringify(body), { + ...init, + headers: { + "Content-Type": "application/json", + "Cache-Control": "no-store", + ...((init?.headers as Record) ?? {}), + }, + }); +} + +async function readJson(req: Request): Promise { + try { + return await req.json(); + } catch (err: unknown) { + const msg = err instanceof Error ? err.message : String(err); + return { __error: `Invalid JSON body: ${msg}` }; + } +} + +function parseInstallBody( + raw: unknown, +): + | { ok: true; event: HookEvent; matcher?: string; definition: ReturnType } + | { ok: false; error: string } { + if (!raw || typeof raw !== "object") return { ok: false, error: "body must be an object" }; + const shape = raw as { event?: unknown; matcher?: unknown; definition?: unknown }; + const evParsed = HookEventSchema.safeParse(shape.event); + if (!evParsed.success) return { ok: false, error: `event: ${evParsed.error.issues[0].message}` }; + if (shape.matcher !== undefined && typeof shape.matcher !== "string") { + return { ok: false, error: "matcher must be a string if present" }; + } + const defParsed = HookDefinitionSchema.safeParse(shape.definition); + if (!defParsed.success) { + const issue = defParsed.error.issues[0]; + const path = issue.path.length > 0 ? issue.path.join(".") : "definition"; + return { ok: false, error: `${path}: ${issue.message}` }; + } + return { + ok: true, + event: evParsed.data, + matcher: shape.matcher as string | undefined, + definition: defParsed.data, + }; +} + +export async function handleHooksApi(req: Request, url: URL, deps: HooksApiDeps): Promise { + const pathname = url.pathname; + + if (pathname === "/ui/api/hooks" && req.method === "GET") { + const result = listHooks(deps.settingsPath); + if (!result.ok) return json({ error: result.error }, { status: 500 }); + return json({ + slice: result.slice, + total: result.total, + allowed_http_hook_urls: result.allowedHttpHookUrls ?? null, + trust_accepted: hasAcceptedHookTrust(deps.db), + }); + } + + if (pathname === "/ui/api/hooks" && req.method === "POST") { + const body = await readJson(req); + if (body && typeof body === "object" && "__error" in body) { + return json({ error: (body as { __error: string }).__error }, { status: 400 }); + } + const parsed = parseInstallBody(body); + if (!parsed.ok) return json({ error: parsed.error }, { status: 422 }); + const result = installHook( + { event: parsed.event, matcher: parsed.matcher, definition: parsed.definition }, + deps.settingsPath, + ); + if (!result.ok) return json({ error: result.error }, { status: result.status }); + recordHookEdit(deps.db, { + event: parsed.event, + matcher: parsed.matcher, + hookType: parsed.definition.type, + action: "install", + previousSlice: result.previousSlice, + newSlice: result.slice, + definition: parsed.definition, + actor: "user", + }); + return json({ + slice: result.slice, + event: parsed.event, + groupIndex: result.groupIndex, + hookIndex: result.hookIndex, + }); + } + + if (pathname === "/ui/api/hooks/trust" && req.method === "POST") { + recordHookEdit(deps.db, { + event: "", + matcher: undefined, + hookType: null, + action: "trust_accepted", + previousSlice: null, + newSlice: null, + definition: null, + actor: "user", + }); + return json({ ok: true }); + } + + if (pathname === "/ui/api/hooks/audit" && req.method === "GET") { + return json({ entries: listHookAudit(deps.db, 100) }); + } + + const match = pathname.match(/^\/ui\/api\/hooks\/([A-Za-z]+)\/(\d+)\/(\d+)$/); + if (match) { + const evParsed = HookEventSchema.safeParse(match[1]); + if (!evParsed.success) return json({ error: `Unknown hook event: ${match[1]}` }, { status: 422 }); + const event = evParsed.data; + const groupIndex = Number.parseInt(match[2], 10); + const hookIndex = Number.parseInt(match[3], 10); + + if (req.method === "PUT") { + const body = await readJson(req); + if (body && typeof body === "object" && "__error" in body) { + return json({ error: (body as { __error: string }).__error }, { status: 400 }); + } + const defShape = (body as { definition?: unknown } | null)?.definition; + const defParsed = HookDefinitionSchema.safeParse(defShape); + if (!defParsed.success) { + const issue = defParsed.error.issues[0]; + const path = issue.path.length > 0 ? issue.path.join(".") : "definition"; + return json({ error: `${path}: ${issue.message}` }, { status: 422 }); + } + const result = updateHook({ event, groupIndex, hookIndex, definition: defParsed.data }, deps.settingsPath); + if (!result.ok) return json({ error: result.error }, { status: result.status }); + recordHookEdit(deps.db, { + event, + matcher: undefined, + hookType: defParsed.data.type, + action: "update", + previousSlice: result.previousSlice, + newSlice: result.slice, + definition: defParsed.data, + actor: "user", + }); + return json({ slice: result.slice }); + } + + if (req.method === "DELETE") { + const result = uninstallHook({ event, groupIndex, hookIndex }, deps.settingsPath); + if (!result.ok) return json({ error: result.error }, { status: result.status }); + recordHookEdit(deps.db, { + event, + matcher: undefined, + hookType: null, + action: "uninstall", + previousSlice: result.previousSlice, + newSlice: result.slice, + definition: null, + actor: "user", + }); + return json({ slice: result.slice }); + } + + return json({ error: "Method not allowed" }, { status: 405 }); + } + + return null; +} diff --git a/src/ui/serve.ts b/src/ui/serve.ts index ef1b829..cd96f9d 100644 --- a/src/ui/serve.ts +++ b/src/ui/serve.ts @@ -6,6 +6,7 @@ import { consumeMagicLink, createSession, isValidSession } from "./session.ts"; import { secretsExpiredHtml, secretsFormHtml } from "../secrets/form-page.ts"; import { getSecretRequest, saveSecrets, validateMagicToken } from "../secrets/store.ts"; +import { handleHooksApi } from "./api/hooks.ts"; import { handleMemoryFilesApi } from "./api/memory-files.ts"; import { type PluginsApiDeps, handlePluginsApi } from "./api/plugins.ts"; import { handleSkillsApi } from "./api/skills.ts"; @@ -178,6 +179,13 @@ export async function handleUiRequest(req: Request): Promise { const apiResponse = await handleSubagentsApi(req, url, { db: dashboardDb }); if (apiResponse) return apiResponse; } + if (url.pathname.startsWith("/ui/api/hooks")) { + if (!dashboardDb) { + return Response.json({ error: "Dashboard API not initialized" }, { status: 503 }); + } + const apiResponse = await handleHooksApi(req, url, { db: dashboardDb }); + if (apiResponse) return apiResponse; + } // Static files const filePath = isPathSafe(url.pathname); From 8e8e4fa67172da3645e7bfdd1f806d8ffe328518 Mon Sep 17 00:00:00 2001 From: Muhammad Ahmed Cheema Date: Mon, 13 Apr 2026 22:26:40 -0700 Subject: [PATCH 5/6] feat: curated settings form with diff-based writes and self-awareness updates Closes the dashboard v2 loop. Every meaningful Claude Agent SDK extensibility surface is now either editable from Phantom's dashboard or read-only with an honest reason. The settings form is the last surface. It exposes ~50 whitelisted fields from Settings (sdk.d.ts:2576-3792) grouped into seven sections: permissions, model, MCP, hooks security, memory, session, UI, and updates. Every field has a tooltip; requires-review fields carry a visible badge. Safety floor: - Zod .strict() whitelist. Unknown fields in a request body are REJECTED at parse time. That is how the deny-list is enforced. apiKeyHelper, modelOverrides, hooks, enabledPlugins, autoMemoryDirectory, extraKnownMarketplaces, and every other risky field are deliberately NOT in the schema. - Diff-based writes: compute which top-level keys actually changed, write only those keys through src/plugins/settings-io.ts atomic tmp+rename. Every untouched field, including enabledPlugins and hooks owned by their dedicated editors, stays byte-for-byte identical. A test locks this in. - Per-field audit log rows capture previous and new JSON values. - The form NEVER touches hooks or enabledPlugins even if the operator somehow submits them; those are double-write surfaces owned by the Hooks and Plugins tabs. Self-awareness: - dashboard-awareness prompt block extended additively to cover subagents, hooks, and settings alongside the PR1/PR2 sections. Every previous line preserved. - show-my-tools built-in skill extended with steps 2.6/2.7/2.8 that read hook counts, subagent list, and a settings summary directly via Read and Glob. No new MCP tool. 28 new settings-editor tests. PR1 skills (39) and PR2 plugins (84) tests still byte-for-byte. Baseline 1141 -> 1279, delta +138. --- public/dashboard/dashboard.css | 27 ++ public/dashboard/dashboard.js | 14 +- public/dashboard/index.html | 11 +- public/dashboard/settings.js | 386 ++++++++++++++++++ skills-builtin/show-my-tools/SKILL.md | 40 +- .../prompt-blocks/dashboard-awareness.ts | 61 +-- src/db/__tests__/migrate.test.ts | 9 +- src/db/schema.ts | 15 + src/settings-editor/__tests__/schema.test.ts | 122 ++++++ src/settings-editor/__tests__/storage.test.ts | 157 +++++++ src/settings-editor/audit.ts | 47 +++ src/settings-editor/schema.ts | 273 +++++++++++++ src/settings-editor/storage.ts | 84 ++++ src/ui/api/settings.ts | 81 ++++ src/ui/serve.ts | 8 + 15 files changed, 1284 insertions(+), 51 deletions(-) create mode 100644 public/dashboard/settings.js create mode 100644 src/settings-editor/__tests__/schema.test.ts create mode 100644 src/settings-editor/__tests__/storage.test.ts create mode 100644 src/settings-editor/audit.ts create mode 100644 src/settings-editor/schema.ts create mode 100644 src/settings-editor/storage.ts create mode 100644 src/ui/api/settings.ts diff --git a/public/dashboard/dashboard.css b/public/dashboard/dashboard.css index cfa4afd..2e08af7 100644 --- a/public/dashboard/dashboard.css +++ b/public/dashboard/dashboard.css @@ -1497,3 +1497,30 @@ body { } .dash-audit-row-top { font-size: 12px; } .dash-audit-row-body { font-size: 11px; color: color-mix(in oklab, var(--color-base-content) 60%, transparent); margin-top: 4px; } + +/* Settings tab (PR3): grouped sections and a bottom save bar. */ +.dash-settings-section { + border: 1px solid var(--color-base-300); + border-radius: var(--radius-lg); + background: var(--color-base-100); + padding: var(--space-4); + margin-bottom: var(--space-4); +} +.dash-settings-section header { margin-bottom: var(--space-3); } +.dash-save-bar { + position: sticky; + bottom: 0; + display: flex; + align-items: center; + justify-content: space-between; + padding: var(--space-3) var(--space-4); + background: var(--color-base-200); + border-top: 1px solid var(--color-base-300); + margin-top: var(--space-4); + backdrop-filter: blur(6px); +} +.dash-save-bar-status { + font-size: 12px; + color: color-mix(in oklab, var(--color-base-content) 65%, transparent); +} +.dash-save-bar-actions { display: flex; gap: var(--space-2); } diff --git a/public/dashboard/dashboard.js b/public/dashboard/dashboard.js index aa2d5f4..37e5c92 100644 --- a/public/dashboard/dashboard.js +++ b/public/dashboard/dashboard.js @@ -205,16 +205,6 @@ title: "Memory explorer", body: "A read view over every episode, fact, and procedure the agent has consolidated. Search, filter by decay, inspect provenance, and watch memories get reinforced as they get reused.", }, - settings: { - eyebrow: "soon", - title: "Settings", - body: "A curated form over the agent's Claude Code settings: permissions, MCP servers, hooks, and the knobs that actually change how it thinks. Raw JSON escape hatch for the power users.", - }, - hooks: { - eyebrow: "soon", - title: "Hooks", - body: "A visual rule builder for the 26 Claude Agent SDK hook events: command, prompt, agent, and http hooks with matchers and per-event lists. Trust modal on first install, audit log on every change.", - }, }; var meta = labels[name] || { eyebrow: "Soon", title: name, body: "Coming in a later PR." }; container.innerHTML = ( @@ -251,8 +241,8 @@ var name = parsed.route; deactivateAllRoutes(); - var liveRoutes = ["skills", "memory-files", "plugins", "subagents", "hooks"]; - var comingSoon = ["sessions", "cost", "scheduler", "evolution", "memory", "settings"]; + var liveRoutes = ["skills", "memory-files", "plugins", "subagents", "hooks", "settings"]; + var comingSoon = ["sessions", "cost", "scheduler", "evolution", "memory"]; if (liveRoutes.indexOf(name) >= 0 && routes[name]) { var containerId = "route-" + name; diff --git a/public/dashboard/index.html b/public/dashboard/index.html index 3f2d560..cfb4078 100644 --- a/public/dashboard/index.html +++ b/public/dashboard/index.html @@ -56,6 +56,10 @@ Hooks + + + Settings +
Coming soon
@@ -85,11 +89,6 @@ Memory explorer soon - - - Settings - soon - ' ); } + function renderToolsChips(tools) { + return renderChipsField( + "subagent-field-tools", + tools, + "Read, Write, Bash, WebFetch", + ["Read", "Write", "Edit", "Glob", "Grep", "Bash", "WebSearch", "WebFetch", "Task"], + ); + } + function renderEditor() { if (!state.currentDetail) { return ( @@ -220,8 +304,23 @@ '' + renderField("Tools", "subagent-field-tools", renderToolsChips(fm.tools), "Allowed tools. Leave empty to inherit everything from the parent.") + + renderField("Disallowed tools", "subagent-field-disallowedTools", renderChipsField("subagent-field-disallowedTools", fm.disallowedTools, "WebFetch, Task", []), "Tools explicitly denied to this subagent even if the parent allows them.") + + renderField("Skills", "subagent-field-skills", renderChipsField("subagent-field-skills", fm.skills, "grep, show-my-tools", []), "Skills this subagent can load on every invocation.") + + renderField("MCP servers", "subagent-field-mcpServers", renderChipsField("subagent-field-mcpServers", fm.mcpServers, "github, linear", []), "MCP server names this subagent is allowed to use.") + + + '
' + + renderField("Memory scope", "subagent-field-memory", renderSelect("subagent-field-memory", fm.memory || "", MEMORY_OPTIONS), "Memory scope the subagent reads. CLI rejects any value outside user, project, local.") + + renderField("Permission mode", "subagent-field-permissionMode", renderSelect("subagent-field-permissionMode", fm.permissionMode || "", PERMISSION_MODE_OPTIONS), "Permission handling override. dontAsk denies anything not pre-approved.") + + '
' + - renderField("Memory", "subagent-field-memory", '', "Free-text memory hint the subagent receives on every invocation.") + + '
' + + renderField("Max turns", "subagent-field-maxTurns", '', "Hard cap on agent turns for this subagent. Blank means inherit.") + + renderField("Isolation", "subagent-field-isolation", renderSelect("subagent-field-isolation", fm.isolation || "", ISOLATION_OPTIONS), "Run the subagent inside its own worktree.") + + '
' + + + renderField("Initial prompt", "subagent-field-initialPrompt", '', "Prompt the subagent receives before any user message.") + + + '
' + renderField("Prompt body", "subagent-body", '', "Markdown. The system prompt the subagent runs under. Saved atomically.") + @@ -266,25 +365,27 @@ }); } - function wireToolChips() { - var container = document.getElementById("subagent-field-tools"); - var input = document.getElementById("subagent-field-tools-input"); + function wireChipField(id) { + var container = document.getElementById(id); + var input = document.getElementById(id + "-input"); if (!container || !input) return; - function save(tools) { - container.setAttribute("data-tools", JSON.stringify(tools)); + function items() { + try { return JSON.parse(container.getAttribute("data-chips") || "[]"); } catch (_) { return []; } + } + function save(next) { + container.setAttribute("data-chips", JSON.stringify(next)); render(false); } - function tools() { return JSON.parse(container.getAttribute("data-tools") || "[]"); } input.addEventListener("keydown", function (e) { if (e.key === "Enter" || e.key === ",") { e.preventDefault(); var value = input.value.trim().replace(/,$/, ""); if (!value) return; - var existing = tools(); + var existing = items(); if (existing.indexOf(value) < 0) existing.push(value); save(existing); } else if (e.key === "Backspace" && input.value === "") { - var existing2 = tools(); + var existing2 = items(); existing2.pop(); save(existing2); } @@ -292,22 +393,29 @@ input.addEventListener("blur", function () { var value = input.value.trim(); if (value) { - var existing = tools(); + var existing = items(); if (existing.indexOf(value) < 0) existing.push(value); input.value = ""; save(existing); } }); - container.querySelectorAll("[data-tool-remove]").forEach(function (btn) { + container.querySelectorAll('[data-chip-remove-for="' + id + '"]').forEach(function (btn) { btn.addEventListener("click", function () { - var idx = parseInt(btn.getAttribute("data-tool-remove"), 10); - var existing = tools(); + var idx = parseInt(btn.getAttribute("data-chip-index"), 10); + var existing = items(); existing.splice(idx, 1); save(existing); }); }); } + function wireToolChips() { + wireChipField("subagent-field-tools"); + wireChipField("subagent-field-disallowedTools"); + wireChipField("subagent-field-skills"); + wireChipField("subagent-field-mcpServers"); + } + function render(rewireList) { if (rewireList === undefined) rewireList = true; var listHtml = renderListColumn(); @@ -338,10 +446,15 @@ var effortEl = document.getElementById("subagent-field-effort"); var colorEl = document.getElementById("subagent-field-color"); var memoryEl = document.getElementById("subagent-field-memory"); - [bodyEl, descEl, memoryEl].forEach(function (el) { + var maxTurnsEl = document.getElementById("subagent-field-maxTurns"); + var initialPromptEl = document.getElementById("subagent-field-initialPrompt"); + var backgroundEl = document.getElementById("subagent-field-background"); + var isolationEl = document.getElementById("subagent-field-isolation"); + var permissionModeEl = document.getElementById("subagent-field-permissionMode"); + [bodyEl, descEl, maxTurnsEl, initialPromptEl].forEach(function (el) { if (el) el.addEventListener("input", updateDirtyState); }); - [modelEl, effortEl, colorEl].forEach(function (el) { + [modelEl, effortEl, colorEl, memoryEl, isolationEl, permissionModeEl, backgroundEl].forEach(function (el) { if (el) el.addEventListener("change", updateDirtyState); }); diff --git a/src/agent/__tests__/init-plugin-snapshot.test.ts b/src/agent/__tests__/init-plugin-snapshot.test.ts index ac69bfe..b827f21 100644 --- a/src/agent/__tests__/init-plugin-snapshot.test.ts +++ b/src/agent/__tests__/init-plugin-snapshot.test.ts @@ -2,6 +2,34 @@ import { describe, expect, test } from "bun:test"; import * as eventsModule from "../../ui/events.ts"; import { emitPluginInitSnapshot, extractPluginKeys } from "../init-plugin-snapshot.ts"; +// Real SDK init message shape: the CLI constructs each plugin entry as +// { name, path, source } where `name` is the bare plugin name and +// `source` is the fully-qualified marketplace key. Verified against +// node_modules/@anthropic-ai/claude-agent-sdk/cli.js near the init +// system message construction (`plugins: A.plugins.map((z) => ({name, +// path, source}))`). +function makeRealInitMessage() { + return { + plugins: [ + { + name: "linear", + path: "/home/phantom/.claude/plugins/cache/claude-plugins-official/linear", + source: "linear@claude-plugins-official", + }, + { + name: "notion", + path: "/home/phantom/.claude/plugins/cache/claude-plugins-official/notion", + source: "notion@claude-plugins-official", + }, + { + name: "slack", + path: "/home/phantom/.claude/plugins/cache/claude-plugins-official/slack", + source: "slack@claude-plugins-official", + }, + ], + }; +} + describe("extractPluginKeys", () => { test("returns empty array on null", () => { expect(extractPluginKeys(null)).toEqual([]); @@ -12,7 +40,7 @@ describe("extractPluginKeys", () => { }); test("returns empty array when plugins field missing", () => { - expect(extractPluginKeys({})).toEqual([]); + expect(extractPluginKeys({} as unknown as Parameters[0])).toEqual([]); }); test("returns empty array when plugins is not an array", () => { @@ -21,44 +49,69 @@ describe("extractPluginKeys", () => { ).toEqual([]); }); - test("extracts names from valid plugin entries", () => { + test("extracts fully-qualified source keys from a real SDK init message", () => { + const result = extractPluginKeys(makeRealInitMessage() as unknown as Parameters[0]); + expect(result).toEqual([ + "linear@claude-plugins-official", + "notion@claude-plugins-official", + "slack@claude-plugins-official", + ]); + }); + + test("falls back to bare name when source is missing", () => { const result = extractPluginKeys({ - plugins: [{ name: "linear@claude-plugins-official" }, { name: "notion@claude-plugins-official" }], - }); - expect(result).toEqual(["linear@claude-plugins-official", "notion@claude-plugins-official"]); + plugins: [{ name: "linear", path: "/p/linear" }], + } as unknown as Parameters[0]); + expect(result).toEqual(["linear"]); }); - test("filters out entries without a string name", () => { + test("falls back to bare name when source is undefined explicitly", () => { const result = extractPluginKeys({ - plugins: [{ name: "linear" }, null, { name: "" }, {}, { name: 42 as unknown as string }, { name: "notion" }], - }); - expect(result).toEqual(["linear", "notion"]); + plugins: [{ name: "linear", path: "/p/linear", source: undefined }], + } as unknown as Parameters[0]); + expect(result).toEqual(["linear"]); + }); + + test("falls back to bare name when source is an empty string", () => { + const result = extractPluginKeys({ + plugins: [{ name: "linear", path: "/p/linear", source: "" }], + } as unknown as Parameters[0]); + expect(result).toEqual(["linear"]); + }); + + test("skips entries missing both source and name", () => { + const result = extractPluginKeys({ + plugins: [{ path: "/p/x" }, null, "", { name: 42 as unknown as string }, { source: "real@m" }], + } as unknown as Parameters[0]); + expect(result).toEqual(["real@m"]); }); }); describe("emitPluginInitSnapshot", () => { - test("calls publish with extracted keys on a well-formed init message", () => { + test("publishes fully-qualified keys from a well-formed real init message", () => { const received: Array<{ event: string; data: unknown }> = []; - const unsub = eventsModule.subscribe((event, data) => received.push({ event, data })); + const unsub = eventsModule.subscribe((event, data) => { + received.push({ event, data }); + }); try { - emitPluginInitSnapshot({ - plugins: [{ name: "linear@claude-plugins-official" }, { name: "slack@claude-plugins-official" }], - }); + emitPluginInitSnapshot(makeRealInitMessage() as unknown as Parameters[0]); } finally { unsub(); } expect(received.length).toBe(1); expect(received[0].event).toBe("plugin_init_snapshot"); expect(received[0].data).toEqual({ - keys: ["linear@claude-plugins-official", "slack@claude-plugins-official"], + keys: ["linear@claude-plugins-official", "notion@claude-plugins-official", "slack@claude-plugins-official"], }); }); test("publishes empty keys when plugins missing", () => { const received: Array<{ event: string; data: unknown }> = []; - const unsub = eventsModule.subscribe((event, data) => received.push({ event, data })); + const unsub = eventsModule.subscribe((event, data) => { + received.push({ event, data }); + }); try { - emitPluginInitSnapshot({}); + emitPluginInitSnapshot({} as unknown as Parameters[0]); } finally { unsub(); } @@ -68,7 +121,9 @@ describe("emitPluginInitSnapshot", () => { test("publishes empty keys on null input", () => { const received: Array<{ event: string; data: unknown }> = []; - const unsub = eventsModule.subscribe((event, data) => received.push({ event, data })); + const unsub = eventsModule.subscribe((event, data) => { + received.push({ event, data }); + }); try { emitPluginInitSnapshot(null); } finally { @@ -78,15 +133,34 @@ describe("emitPluginInitSnapshot", () => { expect(received[0].data).toEqual({ keys: [] }); }); - test("does not throw when a subscriber throws; isolates via existing publish fallback", () => { - // publish() already wraps each listener in try/catch (events.ts:12-17), - // so a throwing subscriber does not reach emitPluginInitSnapshot's - // try/catch. This test confirms no propagation either way. + test("does not throw when a subscriber throws", () => { const unsub = eventsModule.subscribe(() => { throw new Error("subscriber-boom"); }); try { - expect(() => emitPluginInitSnapshot({ plugins: [{ name: "x" }] })).not.toThrow(); + expect(() => + emitPluginInitSnapshot(makeRealInitMessage() as unknown as Parameters[0]), + ).not.toThrow(); + } finally { + unsub(); + } + }); + + test("does not leak unhandled rejections when an async subscriber rejects", async () => { + // Future-proof: if a subscriber returns a rejected promise, the + // publish helper swallows it so process-level unhandled + // rejection handlers do not fire. This test documents the + // expected contract; see src/ui/events.ts publish(). + const rejectingListener = async () => { + throw new Error("async-subscriber-boom"); + }; + const unsub = eventsModule.subscribe(rejectingListener); + try { + emitPluginInitSnapshot(makeRealInitMessage() as unknown as Parameters[0]); + // Wait a microtask so any rejected promise has a chance to + // propagate. If publish() lacks the catch guard, the test + // runner would surface it as a failure. + await new Promise((resolve) => setTimeout(resolve, 10)); } finally { unsub(); } diff --git a/src/agent/init-plugin-snapshot.ts b/src/agent/init-plugin-snapshot.ts index 1b32f7d..fddf146 100644 --- a/src/agent/init-plugin-snapshot.ts +++ b/src/agent/init-plugin-snapshot.ts @@ -1,17 +1,34 @@ // Pure helper extracted from runtime.ts so the init-plugin-snapshot path can be // unit tested without spinning up the full agent main loop. Given an SDK init -// system message, extract a clean list of plugin keys and publish them to the -// dashboard SSE bus. Any failure is logged and swallowed: a telemetry bug must -// never propagate into the agent main loop. +// system message, extract the list of fully-qualified plugin keys the CLI +// resolved during boot and publish them to the dashboard SSE bus so the +// plugins card can flip to "installed" live. +// +// Field shape: verified against node_modules/@anthropic-ai/claude-agent-sdk/ +// cli.js. The init system message constructs plugins as +// plugins: A.plugins.map((z) => ({ name: z.name, path: z.path, source: z.source })) +// where `name` is the BARE plugin name (e.g. "linear") and `source` is the +// fully-qualified marketplace key (e.g. "linear@claude-plugins-official"). +// Phantom's normalized plugin cards carry a synthetic key of the form +// `${name}@${marketplace}`, which matches the CLI's `source`. Reading `name` +// would flip cards across marketplaces if two ever shared a bare name, so +// we read `source` and fall back to `name` only if `source` is missing +// (for forward compat with future SDK shapes). +import type { SDKSystemMessage } from "@anthropic-ai/claude-agent-sdk"; import { publish as publishDashboardEvent } from "../ui/events.ts"; -export type InitMessageLike = - | { - plugins?: Array<{ name?: unknown } | null | undefined>; - } - | null - | undefined; +// The SDK's public `SDKSystemMessage` type declares `plugins` as +// `{name, path}[]` even though the runtime adds `source`. We widen the +// type here so TypeScript can catch shape drift on the fields Phantom +// actually reads while still tolerating the extra runtime field. +type PluginInitEntry = { + name?: unknown; + path?: unknown; + source?: unknown; +}; + +export type InitMessageLike = (SDKSystemMessage & { plugins?: PluginInitEntry[] }) | null | undefined; export function extractPluginKeys(message: InitMessageLike): string[] { if (!message || typeof message !== "object") return []; @@ -20,6 +37,14 @@ export function extractPluginKeys(message: InitMessageLike): string[] { const keys: string[] = []; for (const entry of plugins) { if (!entry || typeof entry !== "object") continue; + const source = (entry as { source?: unknown }).source; + if (typeof source === "string" && source.length > 0) { + keys.push(source); + continue; + } + // Fallback: older or future SDK versions may not populate + // `source` on the init message. Fall back to the bare name so + // the card flip still fires, even if less precisely. const name = (entry as { name?: unknown }).name; if (typeof name === "string" && name.length > 0) { keys.push(name); diff --git a/src/agent/prompt-blocks/dashboard-awareness.ts b/src/agent/prompt-blocks/dashboard-awareness.ts index e882599..cbe2a8d 100644 --- a/src/agent/prompt-blocks/dashboard-awareness.ts +++ b/src/agent/prompt-blocks/dashboard-awareness.ts @@ -1,74 +1,45 @@ // Dashboard awareness block: tells the agent that the operator has a -// dashboard at /ui/dashboard where they can edit skills and memory files, -// so the agent can direct them to it when asked "what can I edit" or -// "how do I customize you". -// -// This is one of two complementary paths. The other is the show-my-tools -// built-in skill under skills-builtin/show-my-tools/SKILL.md which actually -// enumerates the current catalog. The block is always-on; the skill fires -// on demand. +// dashboard at /ui/dashboard where they can edit skills, memory files, +// plugins, subagents, hooks, and settings. Paired with the show-my-tools +// built-in skill which enumerates the current catalog on demand. Budget: +// under 60 lines total per the PR3 builder brief. export function buildDashboardAwarenessLines(publicUrl: string | undefined): string[] { const base = publicUrl?.replace(/\/$/, "") ?? ""; - const dashboardUrl = base ? `${base}/ui/dashboard/` : "/ui/dashboard/"; - const skillsUrl = base ? `${base}/ui/dashboard/#/skills` : "/ui/dashboard/#/skills"; - const memoryUrl = base ? `${base}/ui/dashboard/#/memory-files` : "/ui/dashboard/#/memory-files"; - const pluginsUrl = base ? `${base}/ui/dashboard/#/plugins` : "/ui/dashboard/#/plugins"; - const subagentsUrl = base ? `${base}/ui/dashboard/#/subagents` : "/ui/dashboard/#/subagents"; - const hooksUrl = base ? `${base}/ui/dashboard/#/hooks` : "/ui/dashboard/#/hooks"; - const settingsUrl = base ? `${base}/ui/dashboard/#/settings` : "/ui/dashboard/#/settings"; + const dash = base ? `${base}/ui/dashboard/` : "/ui/dashboard/"; + const skillsUrl = `${dash}#/skills`; + const memoryUrl = `${dash}#/memory-files`; + const pluginsUrl = `${dash}#/plugins`; + const subagentsUrl = `${dash}#/subagents`; + const hooksUrl = `${dash}#/hooks`; + const settingsUrl = `${dash}#/settings`; const lines: string[] = []; lines.push(""); lines.push("=== YOUR DASHBOARD ==="); lines.push(""); - lines.push("Your operator has a dashboard they use to shape how you work. It is a"); - lines.push("hand-crafted UI, separate from the pages you generate with phantom_create_page."); - lines.push("Six tabs are live today:"); - lines.push(""); - lines.push(`- Skills: ${skillsUrl}`); - lines.push(" Markdown files under /home/phantom/.claude/skills//SKILL.md."); - lines.push(" Your operator creates, edits, and deletes skills here. You read every"); - lines.push(" skill's name, description, and when_to_use at the start of every message,"); - lines.push(" so any edit is live on your next turn. You can write your own skills by"); - lines.push(" creating SKILL.md files at the same path."); - lines.push(""); - lines.push(`- Memory files: ${memoryUrl}`); - lines.push(" Arbitrary markdown under /home/phantom/.claude/ including CLAUDE.md,"); - lines.push(" rules/*.md, and memory/*.md. Edits are picked up on your next session."); - lines.push(""); - lines.push(`- Plugins: ${pluginsUrl}`); - lines.push(" Third-party extensions from claude-plugins-official with a trust modal"); - lines.push(" on first install. Four plugins are pre-installed on fresh Phantom VMs:"); - lines.push(" linear, notion, slack, claude-md-management. Every install is audited."); - lines.push(""); - lines.push(`- Subagents: ${subagentsUrl}`); - lines.push(" Flat markdown files at /home/phantom/.claude/agents/.md with"); - lines.push(" frontmatter describing the subagent's tools, model, effort, color, and"); - lines.push(" prompt body. You invoke subagents via the Task tool when the operator"); - lines.push(" asks for specialized behavior."); - lines.push(""); - lines.push(`- Hooks: ${hooksUrl}`); - lines.push(" Visual rule builder over the 26 Claude Agent SDK hook events, stored"); - lines.push(" in /home/phantom/.claude/settings.json under the hooks key. Command,"); - lines.push(" prompt, agent, and http hook types. Hooks fire on your next message and"); - lines.push(" can block tool calls, format on write, or call external services."); - lines.push(""); - lines.push(`- Settings: ${settingsUrl}`); - lines.push(" Curated form over settings.json. Permissions, model, MCP servers,"); - lines.push(" memory, sandbox, UI. Unsafe fields like apiKeyHelper and modelOverrides"); - lines.push(" are hidden. Diff-based writes: untouched fields stay byte-identical."); - lines.push(""); - lines.push("When your operator asks 'what can I customize', 'how do I edit your skills',"); - lines.push(`'show me the dashboard', point them at ${dashboardUrl}`); - lines.push("and fire the show-my-tools skill for the current catalog."); - lines.push(""); - lines.push("When they ask 'install a plugin for X' or 'add a capability', point at the"); - lines.push("plugins tab. 'Add a hook', 'format on edit', 'block dangerous bash' -> hooks"); - lines.push("tab. 'Create a subagent', 'build a specialist' -> subagents tab. 'Change"); - lines.push("permissions', 'change my model' -> settings tab."); - lines.push(""); - lines.push("Other tabs (sessions, cost, scheduler, evolution, memory explorer) are marked"); - lines.push("Coming Soon in the dashboard today and will light up in later PRs."); + lines.push("Your operator has a dashboard they use to shape how you work. Six tabs are live:"); + lines.push(`- Skills: ${skillsUrl} (edit ~/.claude/skills//SKILL.md; live on next turn).`); + lines.push(`- Memory files: ${memoryUrl} (edit CLAUDE.md, rules/*.md, memory/*.md; picked up on next session).`); + lines.push( + `- Plugins: ${pluginsUrl} (claude-plugins-official marketplace; trust modal on first install; four pre-installed: linear, notion, slack, claude-md-management; audited).`, + ); + lines.push( + `- Subagents: ${subagentsUrl} (flat files at ~/.claude/agents/.md with frontmatter for tools, model, effort, color, prompt body; invoke via Task tool).`, + ); + lines.push( + `- Hooks: ${hooksUrl} (visual rule builder over 26 SDK events in ~/.claude/settings.json; command, prompt, agent, http types; live on next message).`, + ); + lines.push( + `- Settings: ${settingsUrl} (curated form over settings.json; permissions, model, MCP, memory, sandbox, UI; unsafe fields hidden; diff-based writes preserve untouched fields byte-for-byte).`, + ); + lines.push(""); + lines.push( + `When the operator asks "what can I customize", "show me the dashboard", point them at ${dash} and fire the show-my-tools skill for the current catalog.`, + ); + lines.push(""); + lines.push( + 'Intent routing: "install a plugin / add a capability" -> plugins. "add a hook / format on edit / block dangerous bash" -> hooks. "create a subagent / build a specialist" -> subagents. "change permissions / change my model" -> settings.', + ); return lines; } diff --git a/src/agent/runtime.ts b/src/agent/runtime.ts index e33d32f..cab4bac 100644 --- a/src/agent/runtime.ts +++ b/src/agent/runtime.ts @@ -230,7 +230,7 @@ export class AgentRuntime { // plugin cards optimistically flipped to "installing..." can settle // to "installed". The helper is wrapped so a telemetry failure never // propagates into the agent main loop. - emitPluginInitSnapshot(message as unknown as Parameters[0]); + emitPluginInitSnapshot(message); } break; } diff --git a/src/db/__tests__/migrate.test.ts b/src/db/__tests__/migrate.test.ts index e17b1c1..153e425 100644 --- a/src/db/__tests__/migrate.test.ts +++ b/src/db/__tests__/migrate.test.ts @@ -38,8 +38,11 @@ describe("runMigrations", () => { // PR3 adds three audit tables and their indices: subagent_audit_log // (commit 3), hook_audit_log (commit 4), and settings_audit_log // (commit 5). Each adds 2 migration steps (table + index), bringing - // the total from the PR2 baseline of 16 up to 22. - expect(migrationCount.count).toBe(22); + // the total from the PR2 baseline of 16 up to 22. The PR3 fix pass + // appends two ALTER TABLE statements on subagent_audit_log to add + // previous_frontmatter_json and new_frontmatter_json columns, + // bringing the total to 24. + expect(migrationCount.count).toBe(24); }); test("tracks applied migration indices", () => { @@ -51,6 +54,17 @@ describe("runMigrations", () => { .all() .map((r) => (r as { index_num: number }).index_num); - expect(indices).toEqual([0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21]); + expect(indices).toEqual([0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23]); + }); + + test("subagent_audit_log has frontmatter JSON columns after migration", () => { + const db = freshDb(); + runMigrations(db); + const cols = db + .query("PRAGMA table_info(subagent_audit_log)") + .all() + .map((r) => (r as { name: string }).name); + expect(cols).toContain("previous_frontmatter_json"); + expect(cols).toContain("new_frontmatter_json"); }); }); diff --git a/src/db/schema.ts b/src/db/schema.ts index 093ad53..2767f1c 100644 --- a/src/db/schema.ts +++ b/src/db/schema.ts @@ -201,4 +201,13 @@ export const MIGRATIONS: string[] = [ )`, "CREATE INDEX IF NOT EXISTS idx_settings_audit_log_field ON settings_audit_log(field, id DESC)", + + // PR3 fix pass: extend the subagent audit log to capture frontmatter + // changes. An edit that only touches tools or model would otherwise + // show previous_body == new_body and become invisible in the audit + // timeline. These columns default to NULL so pre-existing rows remain + // valid. SQLite ALTER TABLE with a default is idempotent under the + // _migrations gate. + "ALTER TABLE subagent_audit_log ADD COLUMN previous_frontmatter_json TEXT", + "ALTER TABLE subagent_audit_log ADD COLUMN new_frontmatter_json TEXT", ]; diff --git a/src/hooks/__tests__/audit.test.ts b/src/hooks/__tests__/audit.test.ts new file mode 100644 index 0000000..1072ea0 --- /dev/null +++ b/src/hooks/__tests__/audit.test.ts @@ -0,0 +1,136 @@ +import { Database } from "bun:sqlite"; +import { beforeEach, describe, expect, test } from "bun:test"; +import { runMigrations } from "../../db/migrate.ts"; +import { getHookTrustMap, hasAcceptedHookTrust, listHookAudit, recordHookEdit } from "../audit.ts"; + +let db: Database; + +beforeEach(() => { + db = new Database(":memory:"); + db.run("PRAGMA journal_mode = WAL"); + db.run("PRAGMA foreign_keys = ON"); + runMigrations(db); +}); + +describe("hasAcceptedHookTrust per type", () => { + test("returns false before any trust is recorded", () => { + expect(hasAcceptedHookTrust(db)).toBe(false); + expect(hasAcceptedHookTrust(db, "command")).toBe(false); + expect(hasAcceptedHookTrust(db, "http")).toBe(false); + }); + + test("command trust does not satisfy http trust", () => { + recordHookEdit(db, { + event: "", + matcher: undefined, + hookType: "command", + action: "trust_accepted", + previousSlice: null, + newSlice: null, + definition: null, + actor: "user", + }); + expect(hasAcceptedHookTrust(db, "command")).toBe(true); + expect(hasAcceptedHookTrust(db, "http")).toBe(false); + expect(hasAcceptedHookTrust(db, "prompt")).toBe(false); + expect(hasAcceptedHookTrust(db, "agent")).toBe(false); + // Legacy "any trust" check still fires. + expect(hasAcceptedHookTrust(db)).toBe(true); + }); + + test("http trust does not satisfy command trust", () => { + recordHookEdit(db, { + event: "", + matcher: undefined, + hookType: "http", + action: "trust_accepted", + previousSlice: null, + newSlice: null, + definition: null, + actor: "user", + }); + expect(hasAcceptedHookTrust(db, "http")).toBe(true); + expect(hasAcceptedHookTrust(db, "command")).toBe(false); + }); + + test("per-type acceptance persists across re-queries", () => { + recordHookEdit(db, { + event: "", + matcher: undefined, + hookType: "command", + action: "trust_accepted", + previousSlice: null, + newSlice: null, + definition: null, + actor: "user", + }); + recordHookEdit(db, { + event: "", + matcher: undefined, + hookType: "prompt", + action: "trust_accepted", + previousSlice: null, + newSlice: null, + definition: null, + actor: "user", + }); + const map = getHookTrustMap(db); + expect(map.command).toBe(true); + expect(map.prompt).toBe(true); + expect(map.agent).toBe(false); + expect(map.http).toBe(false); + }); +}); + +describe("recordHookEdit: matcher capture on update and delete", () => { + test("update rows record the previous matcher so audit lines are unambiguous", () => { + recordHookEdit(db, { + event: "PreToolUse", + matcher: "Bash", + hookType: "command", + action: "update", + previousSlice: { PreToolUse: [{ matcher: "Bash", hooks: [{ type: "command", command: "old" }] }] }, + newSlice: { PreToolUse: [{ matcher: "Bash", hooks: [{ type: "command", command: "new" }] }] }, + definition: { type: "command", command: "new" }, + actor: "user", + }); + const entries = listHookAudit(db, 10); + expect(entries.length).toBe(1); + expect(entries[0].matcher).toBe("Bash"); + expect(entries[0].action).toBe("update"); + expect(entries[0].hook_type).toBe("command"); + }); + + test("uninstall rows record both the matcher and the hook_type", () => { + recordHookEdit(db, { + event: "PreToolUse", + matcher: "Write", + hookType: "http", + action: "uninstall", + previousSlice: { PreToolUse: [{ matcher: "Write", hooks: [{ type: "http", url: "https://x" }] }] }, + newSlice: {}, + definition: null, + actor: "user", + }); + const entries = listHookAudit(db, 10); + expect(entries[0].matcher).toBe("Write"); + expect(entries[0].hook_type).toBe("http"); + expect(entries[0].action).toBe("uninstall"); + }); + + test("relocate rows capture the previous matcher coordinate", () => { + recordHookEdit(db, { + event: "PreToolUse", + matcher: "Bash", + hookType: "command", + action: "relocate", + previousSlice: { PreToolUse: [{ matcher: "Bash", hooks: [{ type: "command", command: "run" }] }] }, + newSlice: { PostToolUse: [{ matcher: "Write", hooks: [{ type: "command", command: "run" }] }] }, + definition: { type: "command", command: "run" }, + actor: "user", + }); + const entries = listHookAudit(db, 10); + expect(entries[0].action).toBe("relocate"); + expect(entries[0].matcher).toBe("Bash"); + }); +}); diff --git a/src/hooks/__tests__/storage.test.ts b/src/hooks/__tests__/storage.test.ts index 0ceb719..01af029 100644 --- a/src/hooks/__tests__/storage.test.ts +++ b/src/hooks/__tests__/storage.test.ts @@ -2,7 +2,7 @@ import { afterEach, beforeEach, describe, expect, test } from "bun:test"; import { mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; -import { installHook, listHooks, uninstallHook, updateHook } from "../storage.ts"; +import { installHook, listHooks, relocateHook, uninstallHook, updateHook } from "../storage.ts"; let tmp: string; let settingsPath: string; @@ -241,6 +241,196 @@ describe("uninstallHook", () => { }); }); +describe("installHook: event/matcher compatibility enforcement", () => { + test("rejects a matcher on an event that does not accept one", () => { + writeSettings({}); + const result = installHook( + { + event: "Notification", + matcher: "Bash", + definition: { type: "command", command: "echo notify" }, + }, + settingsPath, + ); + expect(result.ok).toBe(false); + if (result.ok) return; + expect(result.status).toBe(422); + }); + + test("rejects a matcher on UserPromptSubmit", () => { + writeSettings({}); + const result = installHook( + { + event: "UserPromptSubmit", + matcher: "foo", + definition: { type: "command", command: "echo" }, + }, + settingsPath, + ); + expect(result.ok).toBe(false); + }); + + test("rejects a matcher on SessionStart", () => { + writeSettings({}); + const result = installHook( + { + event: "SessionStart", + matcher: "foo", + definition: { type: "command", command: "echo" }, + }, + settingsPath, + ); + expect(result.ok).toBe(false); + }); + + test("accepts no matcher on a matcher-unsupported event", () => { + writeSettings({}); + const result = installHook( + { + event: "Notification", + definition: { type: "command", command: "echo notify" }, + }, + settingsPath, + ); + expect(result.ok).toBe(true); + }); + + test("accepts a matcher on a matcher-supported event", () => { + writeSettings({}); + const result = installHook( + { + event: "PreToolUse", + matcher: "Bash", + definition: { type: "command", command: "echo pre" }, + }, + settingsPath, + ); + expect(result.ok).toBe(true); + }); +}); + +describe("relocateHook", () => { + test("moves a hook between event/matcher coordinates atomically", () => { + writeSettings({ + hooks: { + PreToolUse: [{ matcher: "Bash", hooks: [{ type: "command", command: "run" }] }], + }, + }); + const result = relocateHook( + { + fromEvent: "PreToolUse", + fromGroupIndex: 0, + fromHookIndex: 0, + toEvent: "PostToolUse", + toMatcher: "Write", + definition: { type: "command", command: "run" }, + }, + settingsPath, + ); + expect(result.ok).toBe(true); + if (!result.ok) return; + expect(result.slice.PreToolUse).toBeUndefined(); + expect(result.slice.PostToolUse?.[0].matcher).toBe("Write"); + expect(result.slice.PostToolUse?.[0].hooks.length).toBe(1); + }); + + test("appends to an existing matcher group on the destination event", () => { + writeSettings({ + hooks: { + PreToolUse: [ + { matcher: "Bash", hooks: [{ type: "command", command: "bash-hook" }] }, + { matcher: "Write", hooks: [{ type: "command", command: "write-hook" }] }, + ], + }, + }); + const result = relocateHook( + { + fromEvent: "PreToolUse", + fromGroupIndex: 0, + fromHookIndex: 0, + toEvent: "PreToolUse", + toMatcher: "Write", + definition: { type: "command", command: "bash-hook" }, + }, + settingsPath, + ); + expect(result.ok).toBe(true); + if (!result.ok) return; + // Now PreToolUse has a single group (Write) with 2 hooks. + expect(result.slice.PreToolUse?.length).toBe(1); + expect(result.slice.PreToolUse?.[0].hooks.length).toBe(2); + }); + + test("refuses a relocate that would put a matcher on a matcher-unsupporting event", () => { + writeSettings({ + hooks: { + PreToolUse: [{ matcher: "Bash", hooks: [{ type: "command", command: "run" }] }], + }, + }); + const result = relocateHook( + { + fromEvent: "PreToolUse", + fromGroupIndex: 0, + fromHookIndex: 0, + toEvent: "Notification", + toMatcher: "Bash", + definition: { type: "command", command: "run" }, + }, + settingsPath, + ); + expect(result.ok).toBe(false); + if (result.ok) return; + expect(result.status).toBe(422); + }); + + test("returns 404 when the source coordinate is out of range", () => { + writeSettings({ + hooks: { + PreToolUse: [{ matcher: "Bash", hooks: [{ type: "command", command: "run" }] }], + }, + }); + const result = relocateHook( + { + fromEvent: "PreToolUse", + fromGroupIndex: 5, + fromHookIndex: 0, + toEvent: "PostToolUse", + toMatcher: undefined, + definition: { type: "command", command: "run" }, + }, + settingsPath, + ); + expect(result.ok).toBe(false); + if (result.ok) return; + expect(result.status).toBe(404); + }); + + test("preserves enabledPlugins and other non-hook settings", () => { + writeSettings({ + enabledPlugins: { "linear@claude-plugins-official": true }, + model: "claude-opus-4-6", + hooks: { + PreToolUse: [{ matcher: "Bash", hooks: [{ type: "command", command: "run" }] }], + }, + }); + const result = relocateHook( + { + fromEvent: "PreToolUse", + fromGroupIndex: 0, + fromHookIndex: 0, + toEvent: "PostToolUse", + toMatcher: undefined, + definition: { type: "command", command: "run" }, + }, + settingsPath, + ); + expect(result.ok).toBe(true); + const after = JSON.parse(readFileSync(settingsPath, "utf-8")); + expect(after.enabledPlugins).toEqual({ "linear@claude-plugins-official": true }); + expect(after.model).toBe("claude-opus-4-6"); + }); +}); + describe("byte-for-byte preservation of PR2 fields", () => { test("a full install/uninstall cycle leaves enabledPlugins identical", () => { const enabledBefore = { diff --git a/src/hooks/audit.ts b/src/hooks/audit.ts index 5cdd58e..53a8aae 100644 --- a/src/hooks/audit.ts +++ b/src/hooks/audit.ts @@ -1,11 +1,19 @@ -// Audit log for hook installs, updates, uninstalls via the UI API. Each row -// captures the full previous and new hooks slice as JSON so a human can diff -// and recover. Agent-originated edits via the Write tool bypass this path. +// Audit log for hook installs, updates, uninstalls, and relocates via the +// UI API. Each row captures the full previous and new hooks slice as JSON +// so a human can diff and recover. Agent-originated edits via the Write +// tool bypass this path. +// +// Trust acceptance is scoped per hook type. Before the fix, accepting +// trust for a command hook also satisfied the prompt, that `http` and +// `agent` hook types never re-fired the trust modal, which is surprising +// given their very different risk profiles (http means network egress). +// The fix stores the hook type alongside the trust_accepted action and +// checks against the type on read. import type { Database } from "bun:sqlite"; import type { HookDefinition, HookEvent, HooksSlice } from "./schema.ts"; -export type HookAuditAction = "install" | "update" | "uninstall" | "trust_accepted"; +export type HookAuditAction = "install" | "update" | "uninstall" | "relocate" | "trust_accepted"; export type HookAuditEntry = { id: number; @@ -60,9 +68,36 @@ export function listHookAudit(db: Database, limit = 50): HookAuditEntry[] { .all(limit) as HookAuditEntry[]; } -export function hasAcceptedHookTrust(db: Database): boolean { +// Returns true if the operator has accepted the trust modal for this +// specific hook type. Scoping trust per type keeps the backstop alive +// when a user who consented to command hooks installs their first http +// hook: the modal fires again because http has a different risk +// profile. A null type argument asks "any trust at all" and is used by +// the legacy codepath. +export function hasAcceptedHookTrust(db: Database, hookType?: HookDefinition["type"]): boolean { + if (hookType) { + const row = db + .query("SELECT COUNT(*) as count FROM hook_audit_log WHERE action = 'trust_accepted' AND hook_type = ?") + .get(hookType) as { count: number } | null; + return (row?.count ?? 0) > 0; + } const row = db.query("SELECT COUNT(*) as count FROM hook_audit_log WHERE action = 'trust_accepted'").get() as { count: number; } | null; return (row?.count ?? 0) > 0; } + +// Returns a record of hookType -> accepted boolean so the dashboard can +// preload the trust state for every type in one request. Used by the +// listHooks API to avoid a second round trip just to render the trust +// modal. +export function getHookTrustMap(db: Database): Record { + const rows = db + .query("SELECT DISTINCT hook_type FROM hook_audit_log WHERE action = 'trust_accepted' AND hook_type IS NOT NULL") + .all() as Array<{ hook_type: string | null }>; + const out: Record = { command: false, prompt: false, agent: false, http: false }; + for (const row of rows) { + if (row.hook_type) out[row.hook_type] = true; + } + return out; +} diff --git a/src/hooks/storage.ts b/src/hooks/storage.ts index 570c7b0..390e17a 100644 --- a/src/hooks/storage.ts +++ b/src/hooks/storage.ts @@ -12,6 +12,7 @@ import { readSettings, writeSettings } from "../plugins/settings-io.ts"; import { getHooksSettingsPath } from "./paths.ts"; import { + EVENTS_SUPPORTING_MATCHER, type HookDefinition, type HookEvent, type HookMatcherGroup, @@ -20,6 +21,24 @@ import { isHttpUrlAllowed, } from "./schema.ts"; +// Shared guard that keeps every mutation path consistent: Notification, +// UserPromptSubmit, SessionStart, etc. do not accept a matcher. Before +// the fix the server accepted any matcher on any event and silently +// wrote a file the CLI would ignore at runtime. This check runs on +// installHook, updateHook, and relocateHook. +function checkMatcherCompatibility( + event: HookEvent, + matcher: string | undefined, +): { ok: true } | { ok: false; error: string } { + if (matcher && matcher.length > 0 && !EVENTS_SUPPORTING_MATCHER.has(event)) { + return { + ok: false, + error: `Event ${event} does not accept a matcher. Leave the matcher field blank for this event.`, + }; + } + return { ok: true }; +} + export type ListHooksResult = | { ok: true; slice: HooksSlice; total: number; allowedHttpHookUrls: string[] | undefined } | { ok: false; error: string }; @@ -68,6 +87,11 @@ export type InstallHookResult = // matcher, or creates a new matcher group if none exists for that matcher. // Writes ONLY the Settings.hooks slice back; all other keys preserved. export function installHook(input: InstallHookInput, settingsPath: string = getHooksSettingsPath()): InstallHookResult { + const compat = checkMatcherCompatibility(input.event, input.matcher); + if (!compat.ok) { + return { ok: false, status: 422, error: compat.error }; + } + const read = readSettings(settingsPath); if (!read.ok) return { ok: false, status: 500, error: read.error }; @@ -87,7 +111,7 @@ export function installHook(input: InstallHookInput, settingsPath: string = getH return { ok: false, status: 403, - error: `HTTP hook URL ${input.definition.url} is not on the allowedHttpHookUrls allowlist. Add it to settings.json first.`, + error: `HTTP hook URL ${input.definition.url} is not on the allowedHttpHookUrls allowlist. Patterns are anchored full-string matches; append '*' to allow query strings or fragments (for example 'https://hooks.example.com/webhook*').`, }; } } @@ -144,7 +168,7 @@ export type UpdateHookInput = { }; export type UpdateHookResult = - | { ok: true; slice: HooksSlice; previousSlice: HooksSlice } + | { ok: true; slice: HooksSlice; previousSlice: HooksSlice; previousMatcher: string | undefined } | { ok: false; status: 404 | 403 | 422 | 500; error: string }; export function updateHook(input: UpdateHookInput, settingsPath: string = getHooksSettingsPath()): UpdateHookResult { @@ -162,6 +186,7 @@ export function updateHook(input: UpdateHookInput, settingsPath: string = getHoo return { ok: false, status: 404, error: `No matcher group at ${input.event}[${input.groupIndex}]` }; } const group = groups[input.groupIndex]; + const previousMatcher = group.matcher; if (!group.hooks || group.hooks.length <= input.hookIndex) { return { ok: false, @@ -178,7 +203,7 @@ export function updateHook(input: UpdateHookInput, settingsPath: string = getHoo return { ok: false, status: 403, - error: `HTTP hook URL ${input.definition.url} is not on the allowedHttpHookUrls allowlist.`, + error: `HTTP hook URL ${input.definition.url} is not on the allowedHttpHookUrls allowlist. Patterns are anchored full-string matches; append '*' to allow query strings or fragments.`, }; } } @@ -196,7 +221,138 @@ export function updateHook(input: UpdateHookInput, settingsPath: string = getHoo const write = writeSettings(merged, settingsPath); if (!write.ok) return { ok: false, status: 500, error: write.error }; - return { ok: true, slice: validated.data, previousSlice }; + return { ok: true, slice: validated.data, previousSlice, previousMatcher }; +} + +// Relocate a hook between coordinates. This is the atomic operation the +// dashboard edit form uses when the operator changes the event or the +// matcher on an existing hook. Before the fix there was no way to do +// this safely: the client had to delete the old entry and install a new +// one in two round trips, which could leave a duplicate or a hole if the +// second call failed. relocateHook does the splice + append + validate +// + write in one pass, with a single atomic settings.json write. +// +// Flow: +// 1. Read settings.json atomically. +// 2. Validate the source coordinate exists. +// 3. Enforce event/matcher compatibility on the destination. +// 4. Splice the hook out of the source group. +// 5. Drop the source group if it is now empty, drop the event key if +// the last group is empty. +// 6. Find or create the destination matcher group and append the +// new definition at the end. +// 7. Zod-validate the full slice. +// 8. Write atomically via settings-io. +// +// If any step fails the in-memory clone is discarded and nothing is +// written, so the on-disk file is never left half-updated. +export type RelocateHookInput = { + fromEvent: HookEvent; + fromGroupIndex: number; + fromHookIndex: number; + toEvent: HookEvent; + toMatcher?: string; + definition: HookDefinition; +}; + +export type RelocateHookResult = + | { + ok: true; + slice: HooksSlice; + previousSlice: HooksSlice; + previousMatcher: string | undefined; + newGroupIndex: number; + newHookIndex: number; + } + | { ok: false; status: 404 | 403 | 422 | 500; error: string }; + +export function relocateHook( + input: RelocateHookInput, + settingsPath: string = getHooksSettingsPath(), +): RelocateHookResult { + const compat = checkMatcherCompatibility(input.toEvent, input.toMatcher); + if (!compat.ok) { + return { ok: false, status: 422, error: compat.error }; + } + + const read = readSettings(settingsPath); + if (!read.ok) return { ok: false, status: 500, error: read.error }; + + const prevParsed = HooksSliceSchema.safeParse((read.settings.hooks ?? {}) as unknown); + if (!prevParsed.success) { + return { ok: false, status: 500, error: `On-disk hooks slice is invalid: ${prevParsed.error.issues[0].message}` }; + } + const previousSlice = prevParsed.data; + const nextSlice: HooksSlice = JSON.parse(JSON.stringify(previousSlice)); + + const fromGroups = nextSlice[input.fromEvent]; + if (!fromGroups || fromGroups.length <= input.fromGroupIndex || !fromGroups[input.fromGroupIndex]) { + return { + ok: false, + status: 404, + error: `No matcher group at ${input.fromEvent}[${input.fromGroupIndex}]`, + }; + } + const fromGroup = fromGroups[input.fromGroupIndex]; + if (!fromGroup.hooks || fromGroup.hooks.length <= input.fromHookIndex) { + return { + ok: false, + status: 404, + error: `No hook at ${input.fromEvent}[${input.fromGroupIndex}].hooks[${input.fromHookIndex}]`, + }; + } + const previousMatcher = fromGroup.matcher; + + // Enforce the http allowlist on the replacement definition same as + // installHook / updateHook. Without this check a relocate could sneak + // a URL past the allowlist by going through the relocate route. + if (input.definition.type === "http") { + const allowlist = Array.isArray(read.settings.allowedHttpHookUrls) + ? (read.settings.allowedHttpHookUrls as string[]) + : undefined; + if (!isHttpUrlAllowed(input.definition.url, allowlist)) { + return { + ok: false, + status: 403, + error: `HTTP hook URL ${input.definition.url} is not on the allowedHttpHookUrls allowlist. Patterns are anchored full-string matches; append '*' to allow query strings or fragments.`, + }; + } + } + + fromGroup.hooks.splice(input.fromHookIndex, 1); + if (fromGroup.hooks.length === 0) { + fromGroups.splice(input.fromGroupIndex, 1); + } + if (fromGroups.length === 0) { + delete nextSlice[input.fromEvent]; + } + + const toGroups: HookMatcherGroup[] = (nextSlice[input.toEvent] as HookMatcherGroup[] | undefined) ?? []; + const toMatcherKey = input.toMatcher && input.toMatcher.length > 0 ? input.toMatcher : undefined; + let newGroupIndex = toGroups.findIndex((g) => (g.matcher ?? null) === (toMatcherKey ?? null)); + if (newGroupIndex === -1) { + toGroups.push({ matcher: toMatcherKey, hooks: [input.definition] }); + newGroupIndex = toGroups.length - 1; + } else { + toGroups[newGroupIndex].hooks.push(input.definition); + } + nextSlice[input.toEvent] = toGroups; + + const validated = HooksSliceSchema.safeParse(nextSlice); + if (!validated.success) { + return { + ok: false, + status: 422, + error: `Hook validation failed: ${validated.error.issues[0].path.join(".")}: ${validated.error.issues[0].message}`, + }; + } + const merged = { ...read.settings, hooks: validated.data }; + const write = writeSettings(merged, settingsPath); + if (!write.ok) return { ok: false, status: 500, error: write.error }; + + const finalGroup = validated.data[input.toEvent]?.[newGroupIndex]; + const newHookIndex = (finalGroup?.hooks.length ?? 1) - 1; + return { ok: true, slice: validated.data, previousSlice, previousMatcher, newGroupIndex, newHookIndex }; } export type UninstallHookInput = { @@ -206,8 +362,14 @@ export type UninstallHookInput = { }; export type UninstallHookResult = - | { ok: true; slice: HooksSlice; previousSlice: HooksSlice } - | { ok: false; status: 404 | 500; error: string }; + | { + ok: true; + slice: HooksSlice; + previousSlice: HooksSlice; + previousMatcher: string | undefined; + previousHookType: HookDefinition["type"] | undefined; + } + | { ok: false; status: 404 | 422 | 500; error: string }; export function uninstallHook( input: UninstallHookInput, @@ -227,6 +389,7 @@ export function uninstallHook( return { ok: false, status: 404, error: `No matcher group at ${input.event}[${input.groupIndex}]` }; } const group = groups[input.groupIndex]; + const previousMatcher = group.matcher; if (!group.hooks || group.hooks.length <= input.hookIndex) { return { ok: false, @@ -234,6 +397,7 @@ export function uninstallHook( error: `No hook at ${input.event}[${input.groupIndex}].hooks[${input.hookIndex}]`, }; } + const previousHookType = group.hooks[input.hookIndex]?.type; group.hooks.splice(input.hookIndex, 1); if (group.hooks.length === 0) { @@ -242,9 +406,23 @@ export function uninstallHook( if (groups.length === 0) { delete nextSlice[input.event]; } - const merged = { ...read.settings, hooks: nextSlice }; + + // Belt-and-suspenders: the on-disk slice was validated at read time + // and the delete cannot introduce new shapes, but we still parse the + // mutated slice before writing so the invariant holds uniformly + // across install, update, relocate, and uninstall. + const validated = HooksSliceSchema.safeParse(nextSlice); + if (!validated.success) { + return { + ok: false, + status: 422, + error: `Hook validation failed after uninstall: ${validated.error.issues[0].message}`, + }; + } + + const merged = { ...read.settings, hooks: validated.data }; const write = writeSettings(merged, settingsPath); if (!write.ok) return { ok: false, status: 500, error: write.error }; - return { ok: true, slice: nextSlice, previousSlice }; + return { ok: true, slice: validated.data, previousSlice, previousMatcher, previousHookType }; } diff --git a/src/settings-editor/__tests__/storage.test.ts b/src/settings-editor/__tests__/storage.test.ts index a655224..677d10c 100644 --- a/src/settings-editor/__tests__/storage.test.ts +++ b/src/settings-editor/__tests__/storage.test.ts @@ -130,19 +130,228 @@ describe("writeCurated: byte-for-byte preservation", () => { expect(result.ok).toBe(false); }); - test("nested object updates replace the whole slice (shallow diff)", () => { + test("partial nested object updates preserve untouched siblings", () => { + // Load a full permissions object with every field set. Submit a + // partial payload that only changes `allow`. Assert deny, ask, and + // defaultMode are byte-for-byte unchanged on disk. This is the + // exact shape that caused Codex P1. + writeSettings({ + permissions: { + allow: ["Bash(git:*)"], + deny: ["Bash(rm:*)"], + ask: ["Read(~/.ssh/*)"], + defaultMode: "acceptEdits", + }, + }); + const result = writeCurated({ permissions: { allow: ["Bash(git:*)", "Bash(ls:*)"] } }, settingsPath); + expect(result.ok).toBe(true); + if (!result.ok) return; + const after = JSON.parse(readFileSync(settingsPath, "utf-8")); + expect(after.permissions.allow).toEqual(["Bash(git:*)", "Bash(ls:*)"]); + expect(after.permissions.deny).toEqual(["Bash(rm:*)"]); + expect(after.permissions.ask).toEqual(["Read(~/.ssh/*)"]); + expect(after.permissions.defaultMode).toBe("acceptEdits"); + }); +}); + +describe("writeCurated: partial-slice preservation per whitelist slice", () => { + // One test per object-valued slice in the whitelist. The shape is the + // same in every case: initial settings.json has a full object with + // multiple siblings, the client submits a partial payload changing + // one sibling, and we assert the others survive byte-for-byte on disk. + + test("permissions.disableBypassPermissionsMode survives a permissions.allow change", () => { + writeSettings({ + permissions: { + allow: ["Bash(git:*)"], + deny: [], + defaultMode: "default", + disableBypassPermissionsMode: "disable", + }, + }); + const result = writeCurated({ permissions: { allow: ["Bash(git:*)", "Bash(ls:*)"] } }, settingsPath); + expect(result.ok).toBe(true); + const after = JSON.parse(readFileSync(settingsPath, "utf-8")); + expect(after.permissions.disableBypassPermissionsMode).toBe("disable"); + expect(after.permissions.deny).toEqual([]); + expect(after.permissions.defaultMode).toBe("default"); + }); + + test("attribution: setting commit alone preserves pr", () => { + writeSettings({ + attribution: { commit: "phantom-agent", pr: "Reviewed-by: Phantom" }, + }); + const result = writeCurated({ attribution: { commit: "phantom-agent-v2" } }, settingsPath); + expect(result.ok).toBe(true); + const after = JSON.parse(readFileSync(settingsPath, "utf-8")); + expect(after.attribution.commit).toBe("phantom-agent-v2"); + expect(after.attribution.pr).toBe("Reviewed-by: Phantom"); + }); + + test("worktree: setting symlinkDirectories alone preserves sparsePaths", () => { + writeSettings({ + worktree: { + symlinkDirectories: ["node_modules", ".venv"], + sparsePaths: ["docs", "tests"], + }, + }); + const result = writeCurated({ worktree: { symlinkDirectories: ["node_modules"] } }, settingsPath); + expect(result.ok).toBe(true); + const after = JSON.parse(readFileSync(settingsPath, "utf-8")); + expect(after.worktree.symlinkDirectories).toEqual(["node_modules"]); + expect(after.worktree.sparsePaths).toEqual(["docs", "tests"]); + }); + + test("sandbox: setting enabled alone preserves every other sandbox field", () => { + writeSettings({ + sandbox: { + enabled: false, + failIfUnavailable: true, + autoAllowBashIfSandboxed: true, + allowUnsandboxedCommands: false, + excludedCommands: ["docker", "kubectl"], + network: { allowedDomains: ["example.com"], allowLocalBinding: false }, + filesystem: { allowWrite: ["/tmp"], denyRead: ["/etc/shadow"] }, + }, + }); + const result = writeCurated({ sandbox: { enabled: true } }, settingsPath); + expect(result.ok).toBe(true); + const after = JSON.parse(readFileSync(settingsPath, "utf-8")); + expect(after.sandbox.enabled).toBe(true); + expect(after.sandbox.failIfUnavailable).toBe(true); + expect(after.sandbox.autoAllowBashIfSandboxed).toBe(true); + expect(after.sandbox.allowUnsandboxedCommands).toBe(false); + expect(after.sandbox.excludedCommands).toEqual(["docker", "kubectl"]); + expect(after.sandbox.network).toEqual({ allowedDomains: ["example.com"], allowLocalBinding: false }); + expect(after.sandbox.filesystem).toEqual({ allowWrite: ["/tmp"], denyRead: ["/etc/shadow"] }); + }); + + test("sandbox.network nested: setting allowedDomains preserves allowLocalBinding and ports", () => { + writeSettings({ + sandbox: { + network: { + allowedDomains: ["example.com"], + allowLocalBinding: true, + httpProxyPort: 8080, + socksProxyPort: 1080, + }, + }, + }); + const result = writeCurated( + { sandbox: { network: { allowedDomains: ["example.com", "github.com"] } } }, + settingsPath, + ); + expect(result.ok).toBe(true); + const after = JSON.parse(readFileSync(settingsPath, "utf-8")); + expect(after.sandbox.network.allowedDomains).toEqual(["example.com", "github.com"]); + expect(after.sandbox.network.allowLocalBinding).toBe(true); + expect(after.sandbox.network.httpProxyPort).toBe(8080); + expect(after.sandbox.network.socksProxyPort).toBe(1080); + }); + + test("sandbox.filesystem nested: setting allowWrite preserves denyWrite, denyRead, allowRead", () => { + writeSettings({ + sandbox: { + filesystem: { + allowWrite: ["/tmp"], + denyWrite: ["/etc"], + denyRead: ["/etc/shadow"], + allowRead: ["/var/log"], + }, + }, + }); + const result = writeCurated({ sandbox: { filesystem: { allowWrite: ["/tmp", "/var/tmp"] } } }, settingsPath); + expect(result.ok).toBe(true); + const after = JSON.parse(readFileSync(settingsPath, "utf-8")); + expect(after.sandbox.filesystem.allowWrite).toEqual(["/tmp", "/var/tmp"]); + expect(after.sandbox.filesystem.denyWrite).toEqual(["/etc"]); + expect(after.sandbox.filesystem.denyRead).toEqual(["/etc/shadow"]); + expect(after.sandbox.filesystem.allowRead).toEqual(["/var/log"]); + }); + + test("sandbox.ripgrep nested: setting command preserves args", () => { + writeSettings({ + sandbox: { + ripgrep: { command: "rg", args: ["--hidden", "--smart-case"] }, + }, + }); + const result = writeCurated({ sandbox: { ripgrep: { command: "/usr/local/bin/rg" } } }, settingsPath); + expect(result.ok).toBe(true); + const after = JSON.parse(readFileSync(settingsPath, "utf-8")); + expect(after.sandbox.ripgrep.command).toBe("/usr/local/bin/rg"); + expect(after.sandbox.ripgrep.args).toEqual(["--hidden", "--smart-case"]); + }); + + test("env: setting one variable preserves every other env var", () => { + writeSettings({ + env: { + PHANTOM_NAME: "phantom", + RESEND_API_KEY: "keep-me", + LINEAR_API_KEY: "keep-me-too", + }, + }); + const result = writeCurated({ env: { PHANTOM_NAME: "phantom-v2" } }, settingsPath); + expect(result.ok).toBe(true); + const after = JSON.parse(readFileSync(settingsPath, "utf-8")); + expect(after.env.PHANTOM_NAME).toBe("phantom-v2"); + expect(after.env.RESEND_API_KEY).toBe("keep-me"); + expect(after.env.LINEAR_API_KEY).toBe("keep-me-too"); + }); + + test("statusLine: setting padding alone preserves command and type", () => { + writeSettings({ + statusLine: { type: "command", command: "echo ready", padding: 2 }, + }); + const result = writeCurated({ statusLine: { type: "command", command: "echo ready", padding: 4 } }, settingsPath); + expect(result.ok).toBe(true); + const after = JSON.parse(readFileSync(settingsPath, "utf-8")); + expect(after.statusLine.command).toBe("echo ready"); + expect(after.statusLine.padding).toBe(4); + expect(after.statusLine.type).toBe("command"); + }); + + test("spinnerVerbs: setting verbs preserves mode", () => { + writeSettings({ + spinnerVerbs: { mode: "append", verbs: ["pondering", "reticulating"] }, + }); + const result = writeCurated( + { spinnerVerbs: { mode: "append", verbs: ["pondering", "reticulating", "vibing"] } }, + settingsPath, + ); + expect(result.ok).toBe(true); + const after = JSON.parse(readFileSync(settingsPath, "utf-8")); + expect(after.spinnerVerbs.mode).toBe("append"); + expect(after.spinnerVerbs.verbs).toEqual(["pondering", "reticulating", "vibing"]); + }); + + test("spinnerTipsOverride: setting tips preserves excludeDefault", () => { + writeSettings({ + spinnerTipsOverride: { excludeDefault: true, tips: ["keep calm"] }, + }); + const result = writeCurated( + { spinnerTipsOverride: { excludeDefault: true, tips: ["keep calm", "ship it"] } }, + settingsPath, + ); + expect(result.ok).toBe(true); + const after = JSON.parse(readFileSync(settingsPath, "utf-8")); + expect(after.spinnerTipsOverride.excludeDefault).toBe(true); + expect(after.spinnerTipsOverride.tips).toEqual(["keep calm", "ship it"]); + }); + + test("no-op save (same content, different key order) does not mark the key dirty", () => { + // Canonical on-disk order. writeSettings({ permissions: { allow: ["Bash(git:*)"], deny: ["Bash(rm:*)"], defaultMode: "default" }, }); + // Client submits with different key insertion order; JSON.stringify + // output differs but the structures are equal. const result = writeCurated( - { permissions: { allow: ["Bash(git:*)"], deny: [], defaultMode: "default" } }, + { permissions: { defaultMode: "default", deny: ["Bash(rm:*)"], allow: ["Bash(git:*)"] } }, settingsPath, ); expect(result.ok).toBe(true); if (!result.ok) return; - const after = JSON.parse(readFileSync(settingsPath, "utf-8")); - expect(after.permissions.deny).toEqual([]); - expect(after.permissions.allow).toEqual(["Bash(git:*)"]); + expect(result.dirty.length).toBe(0); }); }); diff --git a/src/settings-editor/storage.ts b/src/settings-editor/storage.ts index 1ca51e5..3b66b20 100644 --- a/src/settings-editor/storage.ts +++ b/src/settings-editor/storage.ts @@ -5,10 +5,17 @@ // keys that actually changed, and write back ONLY those keys. Every other // field stays byte-for-byte identical. // -// This is the safety floor: untouched fields must survive a round trip -// through the form unchanged. The test at src/settings-editor/__tests__/ -// storage.test.ts asserts byte-for-byte preservation of enabledPlugins, -// hooks, and arbitrary custom fields. +// For object-valued whitelist slices (permissions, sandbox, worktree, env, +// attribution, statusLine, spinnerVerbs, spinnerTipsOverride, plus nested +// sandbox.network, sandbox.filesystem, sandbox.ripgrep), the merge is +// recursive: sibling nested keys the caller did not include survive the +// write untouched. A partial submission of { permissions: { allow: [X] } } +// preserves permissions.deny, permissions.ask, permissions.defaultMode on +// disk. See __tests__/storage.test.ts for the partial-slice preservation +// suite per slice. +// +// The safety floor: untouched fields must survive a round trip through the +// form unchanged. import { getUserSettingsPath } from "../plugins/paths.ts"; import { readSettings, writeSettings } from "../plugins/settings-io.ts"; @@ -35,17 +42,74 @@ export type WriteCuratedResult = | { ok: true; dirty: DirtyKey[]; current: Record; previous: Record } | { ok: false; status: 400 | 422 | 500; error: string }; +// True for plain objects (object literals and Object.create(null)). False +// for arrays, Dates, Maps, class instances, and null. Used by the deep +// merge and deep equal helpers so arrays and primitives are treated as +// atomic values. +function isPlainObject(v: unknown): v is Record { + if (v === null || typeof v !== "object") return false; + if (Array.isArray(v)) return false; + const proto = Object.getPrototypeOf(v); + return proto === Object.prototype || proto === null; +} + +// Structural deep equality for dirty detection. JSON.stringify is not +// sufficient because its output depends on key insertion order, which +// would cause two structurally identical objects with different key order +// to compare as dirty and trigger a no-op write. +function deepEqual(a: unknown, b: unknown): boolean { + if (a === b) return true; + if (a === null || b === null) return false; + if (typeof a !== typeof b) return false; + if (Array.isArray(a) || Array.isArray(b)) { + if (!Array.isArray(a) || !Array.isArray(b)) return false; + if (a.length !== b.length) return false; + for (let i = 0; i < a.length; i++) { + if (!deepEqual(a[i], b[i])) return false; + } + return true; + } + if (isPlainObject(a) && isPlainObject(b)) { + const aKeys = Object.keys(a); + const bKeys = Object.keys(b); + if (aKeys.length !== bKeys.length) return false; + for (const k of aKeys) { + if (!Object.prototype.hasOwnProperty.call(b, k)) return false; + if (!deepEqual(a[k], (b as Record)[k])) return false; + } + return true; + } + return false; +} + +// Recursive deep merge for object-valued whitelist slices. Next overrides +// previous at leaves; siblings in previous that are absent from next are +// preserved. Arrays are atomic (the new array replaces the old one, same +// as primitives). Applied only when BOTH sides are plain objects. +function deepMergeSlice(prev: unknown, next: unknown): unknown { + if (isPlainObject(prev) && isPlainObject(next)) { + const result: Record = { ...prev }; + for (const [k, v] of Object.entries(next)) { + result[k] = deepMergeSlice(prev[k], v); + } + return result; + } + return next; +} + // Compute a shallow diff between a partial form submission and the on-disk -// settings. A key is dirty if the stringified JSON differs. This covers -// both primitive and object values (permissions, worktree, sandbox) while -// avoiding the complexity of a deep field-level merge. +// settings. A key is dirty if the merged result (after deep-merging +// sibling keys of object-valued slices) differs structurally from the +// current value. This avoids false-positive dirty flags from key-order +// drift and avoids writing no-op rows to the audit log. function computeDirtyKeys(next: CuratedSettings, current: Record): DirtyKey[] { const dirty: DirtyKey[] = []; for (const key of Object.keys(next) as Array) { const nextVal = next[key]; const currentVal = current[key]; if (nextVal === undefined) continue; - if (JSON.stringify(nextVal) !== JSON.stringify(currentVal)) { + const merged = deepMergeSlice(currentVal, nextVal); + if (!deepEqual(merged, currentVal)) { dirty.push({ key, previous: currentVal, next: nextVal }); } } @@ -69,12 +133,13 @@ export function writeCurated(submitted: unknown, settingsPath: string = getUserS return { ok: true, dirty: [], current: previousFull, previous: previousFull }; } - // Build the merged settings: take the previous settings as a base and - // overwrite only the dirty keys. Every other key (enabledPlugins, hooks, - // permissions we did not touch, unknown custom fields) stays as-is. + // Build the merged settings: deep-merge each object-valued dirty slice + // with its previous on-disk shape so siblings the caller did not + // include survive the write byte-for-byte. Primitives and arrays + // replace wholesale. const merged: Record = { ...previousFull }; for (const entry of dirty) { - merged[entry.key] = entry.next; + merged[entry.key] = deepMergeSlice(previousFull[entry.key], entry.next); } const write = writeSettings(merged, settingsPath); diff --git a/src/subagents/__tests__/frontmatter.test.ts b/src/subagents/__tests__/frontmatter.test.ts index 0e7946c..dc2b166 100644 --- a/src/subagents/__tests__/frontmatter.test.ts +++ b/src/subagents/__tests__/frontmatter.test.ts @@ -13,7 +13,7 @@ describe("parseFrontmatter", () => { expect(result.parsed.body).toContain("Body goes here"); }); - test("parses the full field set", () => { + test("parses the full field set including CLI-only camelCase keys", () => { const result = parseFrontmatter( [ "---", @@ -22,10 +22,21 @@ describe("parseFrontmatter", () => { "tools:", " - Bash", " - Read", + "disallowedTools:", + " - WebFetch", "model: sonnet", "effort: medium", "color: blue", - "memory: remembers what tests flake", + "memory: project", + "maxTurns: 15", + "initialPrompt: Start by running bun test.", + "skills:", + " - grep", + "mcpServers:", + " - github", + "background: false", + "isolation: worktree", + "permissionMode: acceptEdits", "---", "", "# QA checker", @@ -37,9 +48,18 @@ describe("parseFrontmatter", () => { expect(result.ok).toBe(true); if (!result.ok) return; expect(result.parsed.frontmatter.tools).toEqual(["Bash", "Read"]); + expect(result.parsed.frontmatter.disallowedTools).toEqual(["WebFetch"]); expect(result.parsed.frontmatter.model).toBe("sonnet"); expect(result.parsed.frontmatter.effort).toBe("medium"); expect(result.parsed.frontmatter.color).toBe("blue"); + expect(result.parsed.frontmatter.memory).toBe("project"); + expect(result.parsed.frontmatter.maxTurns).toBe(15); + expect(result.parsed.frontmatter.initialPrompt).toContain("bun test"); + expect(result.parsed.frontmatter.skills).toEqual(["grep"]); + expect(result.parsed.frontmatter.mcpServers).toEqual(["github"]); + expect(result.parsed.frontmatter.background).toBe(false); + expect(result.parsed.frontmatter.isolation).toBe("worktree"); + expect(result.parsed.frontmatter.permissionMode).toBe("acceptEdits"); }); test("rejects missing opening ---", () => { @@ -59,13 +79,56 @@ describe("parseFrontmatter", () => { expect(result.error).toContain("description"); }); - test("rejects unknown fields via strict mode", () => { + test("passthrough preserves unknown forward-compat fields on a round trip", () => { + // The schema uses .passthrough() so any forward-compat SDK field + // the CLI adds later survives a read. The dashboard renders only + // the known fields and the serialize step re-emits the passthrough + // fields at the end so nothing is silently dropped. + const input = "---\nname: research-intern\ndescription: Fetch a paper.\nzzz_future: true\n---\n\n# Body\n"; + const result = parseFrontmatter(input); + expect(result.ok).toBe(true); + if (!result.ok) return; + const fm = result.parsed.frontmatter as Record; + expect(fm.zzz_future).toBe(true); + const serialized = serializeSubagent(result.parsed.frontmatter, result.parsed.body); + expect(serialized).toContain("zzz_future"); + }); + + test("rejects unknown memory values outside the CLI enum", () => { + const result = parseFrontmatter( + "---\nname: research-intern\ndescription: Fetch a paper.\nmemory: remembers-things\n---\n\n# Body\n", + ); + expect(result.ok).toBe(false); + }); + + test("rejects invalid permissionMode", () => { const result = parseFrontmatter( - "---\nname: research-intern\ndescription: Fetch a paper.\nzzz_unknown: true\n---\n\n# Body\n", + "---\nname: research-intern\ndescription: Fetch a paper.\npermissionMode: yolo\n---\n\n# Body\n", ); expect(result.ok).toBe(false); }); + test("rejects tool name with HTML metacharacters for defense in depth", () => { + const result = parseFrontmatter( + "---\nname: research-intern\ndescription: Fetch a paper.\ntools:\n - \n---\n\n# Body\n", + ); + expect(result.ok).toBe(false); + }); + + test("rejects tool name with shell metacharacters", () => { + const result = parseFrontmatter( + "---\nname: research-intern\ndescription: Fetch a paper.\ntools:\n - ;rm -rf /\n---\n\n# Body\n", + ); + expect(result.ok).toBe(false); + }); + + test("accepts mcp__server__tool style tool names", () => { + const result = parseFrontmatter( + "---\nname: research-intern\ndescription: Fetch a paper.\ntools:\n - mcp__github__create_issue\n---\n\n# Body\n", + ); + expect(result.ok).toBe(true); + }); + test("rejects invalid color", () => { const result = parseFrontmatter( "---\nname: research-intern\ndescription: Fetch a paper.\ncolor: teal\n---\n\n# Body\n", @@ -111,7 +174,7 @@ describe("serializeSubagent", () => { model: "sonnet", effort: "medium", color: "blue", - memory: "remembers flakes", + memory: "project", }, "# QA checker\n\nCheck the tests.\n", ); @@ -128,4 +191,71 @@ describe("serializeSubagent", () => { expect(modelIdx).toBeLessThan(effortIdx); expect(effortIdx).toBeLessThan(colorIdx); }); + + test("accepts a CLI-authored agent file with skills and mcpServers", () => { + // Simulates a subagent file authored by the CLI's own agent wizard + // or hand-copied from Anthropic docs. Before the fix, .strict() + // rejected this entire file; the dashboard surfaced it as a parse + // error toast and the operator could not edit it through Phantom. + const cliAuthored = [ + "---", + "name: cli-authored", + "description: A subagent the CLI wrote.", + "tools:", + " - Read", + " - Grep", + "skills:", + " - grep", + " - show-my-tools", + "mcpServers:", + " - github", + " - linear", + "background: true", + "isolation: worktree", + "permissionMode: bypassPermissions", + "maxTurns: 50", + "initialPrompt: Do the thing.", + "memory: project", + "---", + "", + "# CLI authored", + "", + ].join("\n"); + const result = parseFrontmatter(cliAuthored); + expect(result.ok).toBe(true); + if (!result.ok) return; + const fm = result.parsed.frontmatter; + expect(fm.skills).toEqual(["grep", "show-my-tools"]); + expect(fm.mcpServers).toEqual(["github", "linear"]); + expect(fm.background).toBe(true); + expect(fm.isolation).toBe("worktree"); + expect(fm.permissionMode).toBe("bypassPermissions"); + expect(fm.maxTurns).toBe(50); + expect(fm.memory).toBe("project"); + }); + + test("round-trips the full CLI-shaped field set", () => { + const fm = { + name: "qa-checker", + description: "Verify tests ran.", + tools: ["Bash", "Read"], + disallowedTools: ["WebFetch"], + model: "sonnet", + effort: "medium" as const, + color: "blue" as const, + memory: "project" as const, + maxTurns: 10, + initialPrompt: "Start by running bun test.", + skills: ["grep"], + mcpServers: ["github"], + background: false, + isolation: "worktree" as const, + permissionMode: "acceptEdits" as const, + }; + const out = serializeSubagent(fm, "# Body\n"); + const re = parseFrontmatter(out); + expect(re.ok).toBe(true); + if (!re.ok) return; + expect(re.parsed.frontmatter).toMatchObject(fm); + }); }); diff --git a/src/subagents/audit.ts b/src/subagents/audit.ts index a1953e8..ffe0128 100644 --- a/src/subagents/audit.ts +++ b/src/subagents/audit.ts @@ -2,6 +2,11 @@ // writes a row here so the user can see the history of their subagents. // Agent-originated edits via the Write tool bypass this path; a future PR may // add a file watcher to capture those. +// +// The row captures both the body and the frontmatter JSON so an edit that +// changes only the tools allowlist or the model is visible in the timeline. +// Before the PR3 fix pass this was body-only, which made frontmatter-only +// edits invisible on the audit panel. import type { Database } from "bun:sqlite"; @@ -13,6 +18,8 @@ export type SubagentAuditEntry = { action: SubagentAuditAction; previous_body: string | null; new_body: string | null; + previous_frontmatter_json: string | null; + new_frontmatter_json: string | null; actor: string; created_at: string; }; @@ -24,13 +31,25 @@ export function recordSubagentEdit( action: SubagentAuditAction; previousBody: string | null; newBody: string | null; + previousFrontmatterJson: string | null; + newFrontmatterJson: string | null; actor: string; }, ): void { db.run( - `INSERT INTO subagent_audit_log (subagent_name, action, previous_body, new_body, actor) - VALUES (?, ?, ?, ?, ?)`, - [params.name, params.action, params.previousBody, params.newBody, params.actor], + `INSERT INTO subagent_audit_log ( + subagent_name, action, previous_body, new_body, + previous_frontmatter_json, new_frontmatter_json, actor + ) VALUES (?, ?, ?, ?, ?, ?, ?)`, + [ + params.name, + params.action, + params.previousBody, + params.newBody, + params.previousFrontmatterJson, + params.newFrontmatterJson, + params.actor, + ], ); } @@ -38,7 +57,8 @@ export function listSubagentEdits(db: Database, subagentName?: string, limit = 5 if (subagentName) { return db .query( - `SELECT id, subagent_name, action, previous_body, new_body, actor, created_at + `SELECT id, subagent_name, action, previous_body, new_body, + previous_frontmatter_json, new_frontmatter_json, actor, created_at FROM subagent_audit_log WHERE subagent_name = ? ORDER BY id DESC @@ -48,7 +68,8 @@ export function listSubagentEdits(db: Database, subagentName?: string, limit = 5 } return db .query( - `SELECT id, subagent_name, action, previous_body, new_body, actor, created_at + `SELECT id, subagent_name, action, previous_body, new_body, + previous_frontmatter_json, new_frontmatter_json, actor, created_at FROM subagent_audit_log ORDER BY id DESC LIMIT ?`, diff --git a/src/subagents/frontmatter.ts b/src/subagents/frontmatter.ts index 78ca266..7fc315f 100644 --- a/src/subagents/frontmatter.ts +++ b/src/subagents/frontmatter.ts @@ -1,24 +1,36 @@ // Parse and serialize subagent markdown frontmatter. // -// Format verified against cli.js:6131-6141 (the CLI's own agent file writer) -// and sdk.d.ts:38-76 (AgentDefinition type). The CLI emits: +// Format verified against the bundled Claude Agent SDK CLI's own agent +// loader at node_modules/@anthropic-ai/claude-agent-sdk/cli.js (the FJ4 +// reader around line 4601, plus the in-memory Zod schema pJ4 around line +// 4603). The CLI reads camelCase keys directly off the YAML frontmatter: // // --- // name: // description: "" -// tools: +// tools: [Read, Grep] +// disallowedTools: [Bash] // model: -// effort: +// effort: > // color: -// memory: +// memory: +// maxTurns: +// initialPrompt: "" +// skills: [grep, Read] +// mcpServers: [github, linear] +// background: +// isolation: worktree +// permissionMode: // --- // // // -// We accept a slightly richer superset on read (max-turns, initial-prompt, -// disallowed-tools) so dashboard-authored agents can express every -// AgentDefinition field that is practical to ship via a markdown file. Unknown -// fields are REJECTED at parse time via Zod .strict() so typos surface loudly. +// The schema uses `.passthrough()` instead of `.strict()` so any +// forward-compat SDK field the CLI adds later survives a round trip. +// Phantom's editor renders only the known fields on read and preserves +// unknown fields on write. This also fixes the CRIT-3 hostility to +// CLI-authored agent files that carry camelCase fields Phantom did not +// previously declare. import { parse as parseYaml, stringify as stringifyYaml } from "yaml"; import { z } from "zod"; @@ -26,6 +38,11 @@ import { z } from "zod"; export const SUBAGENT_NAME_PATTERN = /^[a-z][a-z0-9-]{0,63}$/; export const MAX_BODY_BYTES = 50 * 1024; // 50 KB, matches skills +// Matches MCP-tool-shaped names (letters, digits, underscores, hyphens, +// colons for the `mcp__server__tool` format). Rejects raw HTML and shell +// metacharacters to defend the chip input's attribute-embedded JSON. +const TOOL_NAME_PATTERN = /^[A-Za-z_][A-Za-z0-9_:-]*$/; + const AGENT_COLORS = [ "red", "orange", @@ -42,6 +59,25 @@ export const AgentColorSchema = z.enum(AGENT_COLORS); export const AgentEffortSchema = z.enum(["low", "medium", "high"]); +// Verified against cli.js: the CLI's `wZ` constant is +// [...G28, "auto"] where G28 = ["acceptEdits", "bypassPermissions", +// "default", "dontAsk", "plan"]. We mirror that here. +export const AgentPermissionModeSchema = z.enum([ + "default", + "acceptEdits", + "bypassPermissions", + "plan", + "dontAsk", + "auto", +]); + +// Verified against cli.js FJ4 parser: memory is checked against the set +// ["user", "project", "local"]. Any other value triggers a CLI warning +// and is dropped. We tighten this from the former free-text field. +export const AgentMemorySchema = z.enum(["user", "project", "local"]); + +export const AgentIsolationSchema = z.enum(["worktree"]); + export const SubagentFrontmatterSchema = z .object({ name: z @@ -49,16 +85,35 @@ export const SubagentFrontmatterSchema = z .min(1) .regex(SUBAGENT_NAME_PATTERN, "name must be lowercase letters, digits, and hyphens, starting with a letter"), description: z.string().min(1, "description is required").max(240), - tools: z.array(z.string().min(1)).optional(), - "disallowed-tools": z.array(z.string().min(1)).optional(), + tools: z + .array( + z + .string() + .min(1) + .regex(TOOL_NAME_PATTERN, "tool names may only contain letters, digits, underscore, colon, and hyphen"), + ) + .optional(), + disallowedTools: z + .array( + z + .string() + .min(1) + .regex(TOOL_NAME_PATTERN, "tool names may only contain letters, digits, underscore, colon, and hyphen"), + ) + .optional(), model: z.string().optional(), effort: AgentEffortSchema.optional(), color: AgentColorSchema.optional(), - memory: z.string().max(2000).optional(), - "max-turns": z.number().int().min(1).max(200).optional(), - "initial-prompt": z.string().max(4000).optional(), + memory: AgentMemorySchema.optional(), + maxTurns: z.number().int().min(1).max(200).optional(), + initialPrompt: z.string().max(4000).optional(), + skills: z.array(z.string().min(1)).optional(), + mcpServers: z.array(z.string().min(1)).optional(), + background: z.boolean().optional(), + isolation: AgentIsolationSchema.optional(), + permissionMode: AgentPermissionModeSchema.optional(), }) - .strict(); + .passthrough(); export type SubagentFrontmatter = z.infer; @@ -120,24 +175,44 @@ export function parseFrontmatter(raw: string): ParseResult { return { ok: true, parsed: { frontmatter: result.data, body } }; } +// Field order for the serialized YAML frontmatter block. Known fields +// land first in a deterministic order so the output is human-readable +// and git-diff friendly. Any unknown fields that survived parsing via +// `.passthrough()` are appended at the end in their original order so +// forward-compat SDK additions are preserved byte-for-byte on a round +// trip. Mirrors the CLI's own writer at cli.js:6131-6140 for the +// overlap of fields the CLI itself emits. +const ORDERED_KNOWN_KEYS: Array = [ + "name", + "description", + "tools", + "disallowedTools", + "model", + "effort", + "color", + "memory", + "maxTurns", + "initialPrompt", + "skills", + "mcpServers", + "background", + "isolation", + "permissionMode", +]; + export function serializeSubagent(frontmatter: SubagentFrontmatter, body: string): string { const ordered: Record = {}; - // Field order mirrors the CLI's own agent writer at cli.js:6131-6140. - const orderedKeys: Array = [ - "name", - "description", - "tools", - "disallowed-tools", - "model", - "effort", - "color", - "memory", - "max-turns", - "initial-prompt", - ]; - for (const key of orderedKeys) { - const value = frontmatter[key]; + const known = new Set(ORDERED_KNOWN_KEYS as string[]); + for (const key of ORDERED_KNOWN_KEYS) { + const value = (frontmatter as Record)[key as string]; if (value !== undefined) { + ordered[key as string] = value; + } + } + // Preserve any passthrough fields the schema did not validate so a + // round trip does not silently drop forward-compat SDK additions. + for (const [key, value] of Object.entries(frontmatter as Record)) { + if (!known.has(key) && value !== undefined) { ordered[key] = value; } } diff --git a/src/subagents/storage.ts b/src/subagents/storage.ts index 8a74a32..1e60311 100644 --- a/src/subagents/storage.ts +++ b/src/subagents/storage.ts @@ -6,7 +6,7 @@ // protects against torn files on a mid-write crash. import { existsSync, mkdirSync, readFileSync, readdirSync, renameSync, rmSync, statSync, writeFileSync } from "node:fs"; -import { dirname, join } from "node:path"; +import { basename, dirname, join } from "node:path"; import { MAX_BODY_BYTES, type ParseResult, @@ -169,7 +169,7 @@ export type WriteInput = { function writeAtomic(file: string, content: string): void { const dir = dirname(file); ensureDir(dir); - const base = file.split("/").pop() ?? "subagent.md"; + const base = basename(file); const tmp = join(dir, `.${base}.tmp-${process.pid}-${Date.now()}`); writeFileSync(tmp, content, { encoding: "utf-8", mode: 0o644 }); renameSync(tmp, file); diff --git a/src/ui/api/hooks.ts b/src/ui/api/hooks.ts index 545e2f5..8e5d4ab 100644 --- a/src/ui/api/hooks.ts +++ b/src/ui/api/hooks.ts @@ -3,8 +3,14 @@ // GET /ui/api/hooks -> list slice + allowlist + trust state + count // POST /ui/api/hooks -> install (body: { event, matcher?, definition }) // PUT /ui/api/hooks/:event/:g/:h -> update the hook at position (g, h) +// If the body also carries a `to` +// coordinate with a different event +// or matcher, the call is routed +// through relocateHook instead for +// an atomic event/matcher change. // DELETE /ui/api/hooks/:event/:g/:h -> uninstall // POST /ui/api/hooks/trust -> record first-install trust acceptance +// (body: { hook_type }) scoped per type. // GET /ui/api/hooks/audit -> audit timeline // // JSON in, JSON out. All writes route through src/plugins/settings-io.ts which @@ -12,9 +18,9 @@ // stays byte-for-byte identical. import type { Database } from "bun:sqlite"; -import { hasAcceptedHookTrust, listHookAudit, recordHookEdit } from "../../hooks/audit.ts"; +import { getHookTrustMap, hasAcceptedHookTrust, listHookAudit, recordHookEdit } from "../../hooks/audit.ts"; import { HookDefinitionSchema, type HookEvent, HookEventSchema } from "../../hooks/schema.ts"; -import { installHook, listHooks, uninstallHook, updateHook } from "../../hooks/storage.ts"; +import { installHook, listHooks, relocateHook, uninstallHook, updateHook } from "../../hooks/storage.ts"; type HooksApiDeps = { db: Database; @@ -78,6 +84,7 @@ export async function handleHooksApi(req: Request, url: URL, deps: HooksApiDeps) total: result.total, allowed_http_hook_urls: result.allowedHttpHookUrls ?? null, trust_accepted: hasAcceptedHookTrust(deps.db), + trust_by_type: getHookTrustMap(deps.db), }); } @@ -112,10 +119,20 @@ export async function handleHooksApi(req: Request, url: URL, deps: HooksApiDeps) } if (pathname === "/ui/api/hooks/trust" && req.method === "POST") { + const body = await readJson(req); + const hookType = + body && typeof body === "object" && "hook_type" in body ? (body as { hook_type?: unknown }).hook_type : undefined; + // Default to "command" if the client did not specify a type so + // pre-fix clients keep working. New clients pass the type + // explicitly for per-type scoping. + const typed = + typeof hookType === "string" && ["command", "prompt", "agent", "http"].includes(hookType) + ? (hookType as "command" | "prompt" | "agent" | "http") + : "command"; recordHookEdit(deps.db, { event: "", matcher: undefined, - hookType: null, + hookType: typed, action: "trust_accepted", previousSlice: null, newSlice: null, @@ -142,18 +159,66 @@ export async function handleHooksApi(req: Request, url: URL, deps: HooksApiDeps) if (body && typeof body === "object" && "__error" in body) { return json({ error: (body as { __error: string }).__error }, { status: 400 }); } - const defShape = (body as { definition?: unknown } | null)?.definition; + const shape = body as { definition?: unknown; to?: { event?: unknown; matcher?: unknown } } | null; + const defShape = shape?.definition; const defParsed = HookDefinitionSchema.safeParse(defShape); if (!defParsed.success) { const issue = defParsed.error.issues[0]; const path = issue.path.length > 0 ? issue.path.join(".") : "definition"; return json({ error: `${path}: ${issue.message}` }, { status: 422 }); } + + // Detect whether the caller is asking for a relocate. If the + // `to` coordinate pair is present and either the event or + // matcher differs from the source, we route through + // relocateHook so the move is a single atomic write. + const toRaw = shape?.to; + if (toRaw && typeof toRaw === "object") { + const toEvParsed = HookEventSchema.safeParse((toRaw as { event?: unknown }).event); + if (!toEvParsed.success) { + return json({ error: `to.event: ${toEvParsed.error.issues[0].message}` }, { status: 422 }); + } + const toMatcherRaw = (toRaw as { matcher?: unknown }).matcher; + if (toMatcherRaw !== undefined && toMatcherRaw !== null && typeof toMatcherRaw !== "string") { + return json({ error: "to.matcher must be a string if present" }, { status: 422 }); + } + const toMatcher = typeof toMatcherRaw === "string" && toMatcherRaw.length > 0 ? toMatcherRaw : undefined; + + const result = relocateHook( + { + fromEvent: event, + fromGroupIndex: groupIndex, + fromHookIndex: hookIndex, + toEvent: toEvParsed.data, + toMatcher, + definition: defParsed.data, + }, + deps.settingsPath, + ); + if (!result.ok) return json({ error: result.error }, { status: result.status }); + recordHookEdit(deps.db, { + event, + matcher: result.previousMatcher, + hookType: defParsed.data.type, + action: "relocate", + previousSlice: result.previousSlice, + newSlice: result.slice, + definition: defParsed.data, + actor: "user", + }); + return json({ + slice: result.slice, + event: toEvParsed.data, + groupIndex: result.newGroupIndex, + hookIndex: result.newHookIndex, + }); + } + const result = updateHook({ event, groupIndex, hookIndex, definition: defParsed.data }, deps.settingsPath); if (!result.ok) return json({ error: result.error }, { status: result.status }); recordHookEdit(deps.db, { event, - matcher: undefined, + matcher: result.previousMatcher, hookType: defParsed.data.type, action: "update", previousSlice: result.previousSlice, @@ -169,8 +234,8 @@ export async function handleHooksApi(req: Request, url: URL, deps: HooksApiDeps) if (!result.ok) return json({ error: result.error }, { status: result.status }); recordHookEdit(deps.db, { event, - matcher: undefined, - hookType: null, + matcher: result.previousMatcher, + hookType: result.previousHookType ?? null, action: "uninstall", previousSlice: result.previousSlice, newSlice: result.slice, diff --git a/src/ui/api/subagents.ts b/src/ui/api/subagents.ts index 2d14b0c..83e6cce 100644 --- a/src/ui/api/subagents.ts +++ b/src/ui/api/subagents.ts @@ -161,6 +161,8 @@ export async function handleSubagentsApi(req: Request, url: URL, deps: Subagents action: "create", previousBody: null, newBody: result.subagent.body, + previousFrontmatterJson: null, + newFrontmatterJson: JSON.stringify(result.subagent.frontmatter), actor: "user", }); } @@ -206,6 +208,13 @@ export async function handleSubagentsApi(req: Request, url: URL, deps: Subagents { status: 413 }, ); } + // Capture the previous frontmatter before writing so we can + // record a diff in the audit log. The subagent storage layer + // returns previousBody but not previousFrontmatter; read it + // inline here via readSubagent so we do not widen the storage + // return shape. + const preRead = readSubagent(name); + const previousFrontmatterJson = preRead.ok ? JSON.stringify(preRead.subagent.frontmatter) : null; const result = writeSubagent({ name, frontmatter: parsed.frontmatter, body: parsed.body }, { mustExist: true }); if (result.ok) { recordSubagentEdit(deps.db, { @@ -213,6 +222,8 @@ export async function handleSubagentsApi(req: Request, url: URL, deps: Subagents action: "update", previousBody: result.previousBody, newBody: result.subagent.body, + previousFrontmatterJson, + newFrontmatterJson: JSON.stringify(result.subagent.frontmatter), actor: "user", }); } @@ -220,6 +231,11 @@ export async function handleSubagentsApi(req: Request, url: URL, deps: Subagents } if (req.method === "DELETE") { + // Snapshot the frontmatter before the file is removed so the + // audit row can render "this is what the subagent looked like + // before the delete". + const preRead = readSubagent(name); + const previousFrontmatterJson = preRead.ok ? JSON.stringify(preRead.subagent.frontmatter) : null; const result = deleteSubagent(name); if (result.ok) { recordSubagentEdit(deps.db, { @@ -227,6 +243,8 @@ export async function handleSubagentsApi(req: Request, url: URL, deps: Subagents action: "delete", previousBody: result.previousBody, newBody: null, + previousFrontmatterJson, + newFrontmatterJson: null, actor: "user", }); } diff --git a/src/ui/events.ts b/src/ui/events.ts index c315f06..ffc657a 100644 --- a/src/ui/events.ts +++ b/src/ui/events.ts @@ -1,4 +1,11 @@ -type EventListener = (event: string, data: unknown) => void; +// Dashboard SSE bus. Listeners can be synchronous or async; publish wraps +// every call in a try/catch AND attaches a rejection handler to any +// returned promise so a future async subscriber cannot leak an unhandled +// rejection to the process. Future subscribers (metrics counter, +// per-session rollup, rate limiter) will be tempted to use async/await +// and this guard keeps that safe. + +type EventListener = (event: string, data: unknown) => void | Promise; const listeners = new Set(); @@ -10,30 +17,59 @@ export function subscribe(listener: EventListener): () => void { export function publish(event: string, data: unknown): void { for (const listener of listeners) { try { - listener(event, data); - } catch { - // Don't let one listener crash others + const result = listener(event, data); + if (result && typeof (result as Promise).then === "function") { + (result as Promise).catch((err) => { + console.warn(`[events] async listener error on ${event}:`, err); + }); + } + } catch (err) { + console.warn(`[events] sync listener error on ${event}:`, err); } } } export function createSSEResponse(): Response { let unsubscribe: (() => void) | null = null; + let keepAliveInterval: ReturnType | null = null; const stream = new ReadableStream({ start(controller) { const encoder = new TextEncoder(); + + // Suggest a 5 second reconnect window so browsers come back + // fast after a proxy drop. Cloudflare and Caddy strip most + // buffering when X-Accel-Buffering: no is on the response, + // but the retry hint makes disconnects self-healing. + controller.enqueue(encoder.encode("retry: 5000\n")); controller.enqueue(encoder.encode(`data: ${JSON.stringify({ type: "connected" })}\n\n`)); + unsubscribe = subscribe((event, data) => { try { controller.enqueue(encoder.encode(`event: ${event}\ndata: ${JSON.stringify(data)}\n\n`)); } catch { - // Stream may be closed + // Stream may be closed; the cancel hook unsubscribes. } }); + + // Periodic keep-alive comment line (lines starting with `:` + // are ignored by the EventSource parser) so intermediaries + // like Cloudflare/Caddy do not buffer the connection dead. + // The interval lives with the stream: cancel() clears it. + keepAliveInterval = setInterval(() => { + try { + controller.enqueue(encoder.encode(":keep-alive\n\n")); + } catch { + // Stream closed; the cancel hook clears the interval. + } + }, 25_000); }, cancel() { unsubscribe?.(); + if (keepAliveInterval) { + clearInterval(keepAliveInterval); + keepAliveInterval = null; + } }, }); @@ -41,6 +77,10 @@ export function createSSEResponse(): Response { headers: { "Content-Type": "text/event-stream", "Cache-Control": "no-cache, no-transform", + // Disable upstream proxy buffering (nginx and Caddy both + // honor this header). Without it, an SSE stream can be + // buffered for minutes before reaching the browser. + "X-Accel-Buffering": "no", }, }); }