diff --git a/apps/server/src/chat/feed.ts b/apps/server/src/chat/feed.ts index 8e44f8ce..27916566 100644 --- a/apps/server/src/chat/feed.ts +++ b/apps/server/src/chat/feed.ts @@ -3,6 +3,7 @@ import type { ChatFeedEntry, ChatFeedResponse, ChatMediaEntry, + ChatReviewEntry, ChatStatusEntry, } from "@dispatch/shared"; @@ -24,7 +25,7 @@ export type ComposeChatFeedOptions = { /** * Feed ordering is (created_at desc, source rank desc, id desc) — a total - * order across the four tables, so a page boundary that falls on rows with + * order across the five tables, so a page boundary that falls on rows with * identical timestamps never drops or repeats a row. The cursor names the * last entry of the previous page in that order. `at` is Postgres microsecond * text (`to_char(..., 'YYYY-MM-DD HH24:MI:SS.US')`), not the millisecond ISO @@ -37,6 +38,7 @@ export type FeedCursor = { }; const SOURCE_RANK: Record = { + review: 4, chat: 3, status: 2, agent_message: 1, @@ -60,6 +62,7 @@ function isValidCursorId(type: ChatFeedEntry["type"], id: string): boolean { return isChatMessageId(id); case "status": case "media": + case "review": return SERIAL_ID_RE.test(id) && Number(id) <= 2_147_483_647; } } @@ -121,23 +124,26 @@ type Keyed = { * "Older than the cursor" for one source. `$1` is the agent id; the clause * appends its own parameters. Sources ranked below the cursor's include the * cursor timestamp itself; those above it exclude it; the cursor's own - * source breaks the tie on id. + * source breaks the tie on id. `alias` qualifies the columns for a source + * whose query joins other tables that have `id`/`created_at` of their own. */ function cursorClause( type: ChatFeedEntry["type"], idCast: "int" | "uuid", cursor: FeedCursor | null, - params: unknown[] + params: unknown[], + alias = "" ): string { if (!cursor) return ""; + const col = (name: string) => (alias ? `${alias}.${name}` : name); params.push(cursor.at); const ts = `($${params.length}::timestamp AT TIME ZONE 'UTC')`; const rank = SOURCE_RANK[type]; const cursorRank = SOURCE_RANK[cursor.type]; - if (rank > cursorRank) return `AND created_at < ${ts}`; - if (rank < cursorRank) return `AND created_at <= ${ts}`; + if (rank > cursorRank) return `AND ${col("created_at")} < ${ts}`; + if (rank < cursorRank) return `AND ${col("created_at")} <= ${ts}`; params.push(idCast === "int" ? Number(cursor.id) : cursor.id); - return `AND (created_at < ${ts} OR (created_at = ${ts} AND id < $${params.length}::${idCast}))`; + return `AND (${col("created_at")} < ${ts} OR (${col("created_at")} = ${ts} AND ${col("id")} < $${params.length}::${idCast}))`; } const intKey = (id: number) => String(id).padStart(20, "0"); @@ -313,6 +319,69 @@ async function listMediaEntries( })); } +/** + * Reviews left on this agent's work. Counts and status are read live rather + * than frozen at submission time, so the card in the feed says the same + * thing as the row in the Reviews sidebar it links to. + */ +async function listReviewEntries( + db: Queryable, + agentId: string, + cursor: FeedCursor | null, + limit: number +): Promise[]> { + const params: unknown[] = [agentId]; + const clause = cursorClause("review", "int", cursor, params, "r"); + params.push(limit); + const result = await db.query<{ + id: number; + reviewer_type: string; + reviewer_agent_id: string | null; + reviewer_name: string | null; + summary: string | null; + status: string; + item_count: number; + resolved_count: number; + created_at: Date; + at_key: string; + }>( + `SELECT r.id, r.reviewer_type, r.reviewer_agent_id, r.summary, r.status, + r.created_at, + COALESCE(reviewer.persona, reviewer.name) AS reviewer_name, + COUNT(fi.id)::int AS item_count, + COUNT(fi.id) FILTER (WHERE fi.status = 'resolved')::int + AS resolved_count, + to_char(r.created_at AT TIME ZONE 'UTC', + 'YYYY-MM-DD HH24:MI:SS.US') AS at_key + FROM reviews r + LEFT JOIN agents reviewer ON reviewer.id = r.reviewer_agent_id + LEFT JOIN review_feedback_items fi ON fi.review_id = r.id + WHERE r.agent_id = $1 ${clause} + GROUP BY r.id, reviewer.persona, reviewer.name + ORDER BY r.created_at DESC, r.id DESC + LIMIT $${params.length}`, + params + ); + return result.rows.map((row) => ({ + entry: { + type: "review", + id: `review:${row.id}`, + reviewId: row.id, + reviewerType: row.reviewer_type === "agent" ? "agent" : "human", + reviewerAgentId: row.reviewer_agent_id, + reviewerName: row.reviewer_name, + summary: row.summary, + status: row.status, + itemCount: row.item_count, + resolvedCount: row.resolved_count, + at: row.created_at.toISOString(), + }, + atKey: row.at_key, + rawId: String(row.id), + idKey: intKey(row.id), + })); +} + /** Newest first: (atKey, source rank, id) descending. */ function compareNewestFirst(a: Keyed, b: Keyed) { if (a.atKey !== b.atKey) return a.atKey < b.atKey ? 1 : -1; @@ -324,8 +393,8 @@ function compareNewestFirst(a: Keyed, b: Keyed) { /** * Compose one agent's Chat feed at read time from chat messages, status - * events, cross-agent messages, and shared media. Each source contributes - * its newest `limit + 1` rows past the cursor; the merge keeps the newest + * events, cross-agent messages, shared media, and reviews. Each source + * contributes its newest `limit + 1` rows past the cursor; the merge keeps the newest * `limit` overall, so any row that belongs on the page is present (a row in * the top `limit` overall is in the top `limit` of its source), and anything * left over proves an older page exists. @@ -338,19 +407,22 @@ export async function composeChatFeed( const limit = clampFeedLimit(opts.limit); const cursor = opts.cursor ?? null; const { db } = store; - const [chat, status, agentMessages, media, unreadCount] = await Promise.all([ - listChatEntries(db, agentId, cursor, limit + 1), - listStatusEntries(db, agentId, cursor, limit + 1), - listAgentMessageEntries(db, agentId, cursor, limit + 1), - listMediaEntries(db, agentId, cursor, limit + 1), - store.countUnread(agentId), - ]); + const [chat, status, agentMessages, media, reviews, unreadCount] = + await Promise.all([ + listChatEntries(db, agentId, cursor, limit + 1), + listStatusEntries(db, agentId, cursor, limit + 1), + listAgentMessageEntries(db, agentId, cursor, limit + 1), + listMediaEntries(db, agentId, cursor, limit + 1), + listReviewEntries(db, agentId, cursor, limit + 1), + store.countUnread(agentId), + ]); const merged: Keyed[] = [ ...chat, ...status, ...agentMessages, ...media, + ...reviews, ].sort(compareNewestFirst); const hasMore = merged.length > limit; const page = merged.slice(0, limit); diff --git a/apps/server/src/chat/user-prompt.ts b/apps/server/src/chat/user-prompt.ts new file mode 100644 index 00000000..23e8dff5 --- /dev/null +++ b/apps/server/src/chat/user-prompt.ts @@ -0,0 +1,82 @@ +/** + * Prompts the user fires from a UI control rather than typing into the Chat + * composer: a quick phrase, a shortcut pin. They inject the same kind of + * thing a composed message does — the user's words, addressed to the agent — + * so with the Chat surface on they take the same path: a user row, the + * envelope carrying its id, and a post in the feed the agent's reply can + * thread onto. See docs/chat-surface-plan.md, "Injection envelope". + */ + +export type UserPromptRouting = { + /** Whether the Chat surface is switched on for this installation. */ + chatSurfaceEnabled: boolean; + /** + * The target agent's type. Only `"terminal"` is excluded; an older row + * with no type is a CLI agent like any other. A caller that could not + * find the agent at all does not reach here — it keeps the pane path so + * its own 404 stands. + */ + agentType: string | null | undefined; + /** + * Whether the prompt is being submitted. A quick phrase pasted for the + * user to edit first (`submit: false`) is not a message yet — there is + * nothing to post, and the text still has to land in the pane's composer. + */ + submit: boolean; +}; + +/** + * Whether a user-fired prompt goes out as a Chat message instead of straight + * into the pane. Everything this decision needs is in `UserPromptRouting`, so + * the rule lives in one place rather than being restated at each route. + * + * A terminal session is excluded for the same reason it gets no launch post: + * there is no CLI behind it to read the envelope or to answer with + * `dispatch_chat_post`, so the envelope would be noise in the pane. + */ +export function routesUserPromptThroughChat( + routing: UserPromptRouting +): boolean { + if (!routing.chatSurfaceEnabled) return false; + if (!routing.submit) return false; + return routing.agentType !== "terminal"; +} + +/** What `deliverUserPrompt` needs; a subset of the agent routes' deps. */ +export type UserPromptDeps = { + isChatSurfaceEnabled: () => Promise; + getAgent: (agentId: string) => Promise<{ type?: string | null } | null>; + sendUserMessage: (agentId: string, text: string) => Promise; +}; + +/** + * Deliver a prompt the user fired from a UI control, returning whether Chat + * took it. `false` means the caller writes to the pane itself, which is all + * that ever happened before the surface existed. + * + * The failure modes line up with the pane path's on purpose: an agent with + * no live session makes `sendUserMessage` throw before any row is written, + * so a click that could not be delivered leaves nothing behind in the feed. + */ +export async function deliverUserPrompt( + deps: UserPromptDeps, + agentId: string, + text: string, + submit: boolean +): Promise { + if (!(await deps.isChatSurfaceEnabled())) return false; + // A missing agent keeps the pane path so the route's own 404 stands. + const agent = await deps.getAgent(agentId).catch(() => null); + if (!agent) return false; + if ( + !routesUserPromptThroughChat({ + chatSurfaceEnabled: true, + agentType: agent.type, + submit, + }) + ) { + return false; + } + await deps.sendUserMessage(agentId, text); + return true; +} diff --git a/apps/server/src/routes/agents/shared.ts b/apps/server/src/routes/agents/shared.ts index 3b73bf9f..18eade72 100644 --- a/apps/server/src/routes/agents/shared.ts +++ b/apps/server/src/routes/agents/shared.ts @@ -10,6 +10,7 @@ import type { } from "../../terminal/copy-mode-observer.js"; import type { CopyModeAssistManager } from "../../terminal/copy-mode-assist-manager.js"; import type { InjectionCoordinator } from "../../terminal/injection-coordinator.js"; +import type { ChatService } from "../../chat/service.js"; export const AGENT_INITIAL_PROMPT_MAX_CHARS = 16_000; export const CODEX_FULL_ACCESS_ARG = @@ -51,6 +52,13 @@ export type AgentRouteDeps = { trackArchivePromise: (agentId: string, archivePromise: Promise) => void; sendAgentPrompt: (agentId: string, prompt: string) => Promise; onAgentStarted: (agentId: string) => Promise; + /** + * Delivers a user-fired prompt (quick phrase, shortcut pin) as a Chat + * message when the Chat surface is on — see `chat/user-prompt.ts`. + */ + chat: ChatService; + /** Read per click: the flag is a cold path and must not be cached stale. */ + isChatSurfaceEnabled: () => Promise; }; export function escapeHtml(s: string): string { diff --git a/apps/server/src/routes/agents/terminal-routes.ts b/apps/server/src/routes/agents/terminal-routes.ts index ba83bcf3..adb93ecc 100644 --- a/apps/server/src/routes/agents/terminal-routes.ts +++ b/apps/server/src/routes/agents/terminal-routes.ts @@ -8,8 +8,28 @@ import { substituteArgs } from "../../templates/arg-parser.js"; import { TmuxTerminal } from "../../terminal/tmux-terminal.js"; import { errorMessage } from "../../shared/lib/error-message.js"; import { resolveShortcutRun } from "../../agents/pin-run.js"; +import { ChatServiceError } from "../../chat/service.js"; +import { + deliverUserPrompt, + type UserPromptDeps, +} from "../../chat/user-prompt.js"; import { decodeClientMessage, type AgentRouteDeps } from "./shared.js"; +/** + * The routes' deps, narrowed to what a user-fired prompt's delivery needs. + * With the Chat surface on it becomes a Chat message — a user post in the + * feed, wrapped in the envelope — so the agent's reply threads back to it + * exactly as it would for a message typed in the composer. + */ +function promptDeps(deps: AgentRouteDeps): UserPromptDeps { + return { + isChatSurfaceEnabled: deps.isChatSurfaceEnabled, + getAgent: (agentId) => deps.agentManager.getAgent(agentId), + sendUserMessage: (agentId, text) => + deps.chat.sendUserMessage(agentId, text), + }; +} + export async function registerAgentTerminalRoutes( app: FastifyInstance, deps: AgentRouteDeps @@ -184,6 +204,10 @@ export async function registerAgentTerminalRoutes( } try { + if (await deliverUserPrompt(promptDeps(deps), agentId, text, submit)) { + return reply.code(204).send(); + } + const access = await deps.agentManager.getTerminalAccess(agentId); if (access.mode !== "tmux") { return reply.code(409).send({ error: access.message }); @@ -200,6 +224,9 @@ export async function registerAgentTerminalRoutes( ); return reply.code(204).send(); } catch (error) { + if (error instanceof ChatServiceError) { + return reply.code(error.statusCode).send({ error: error.message }); + } return deps.handleAgentError(reply, error); } } @@ -275,6 +302,17 @@ export async function registerAgentTerminalRoutes( return reply.code(target.status).send({ error: target.error }); } + if ( + await deliverUserPrompt( + promptDeps(deps), + agentId, + target.prompt, + true + ) + ) { + return reply.code(204).send(); + } + const access = await deps.agentManager.getTerminalAccess(agentId); if (access.mode !== "tmux") { return reply.code(409).send({ error: access.message }); @@ -288,6 +326,9 @@ export async function registerAgentTerminalRoutes( ); return reply.code(204).send(); } catch (error) { + if (error instanceof ChatServiceError) { + return reply.code(error.statusCode).send({ error: error.message }); + } return deps.handleAgentError(reply, error); } } diff --git a/apps/server/src/server.ts b/apps/server/src/server.ts index 23110d0d..2e5ea79f 100644 --- a/apps/server/src/server.ts +++ b/apps/server/src/server.ts @@ -110,6 +110,7 @@ import { registerMediaRoutes } from "./routes/media.js"; import { registerMessagesRoutes } from "./routes/messages.js"; import { registerChatRoutes } from "./routes/chat.js"; import { ChatService } from "./chat/service.js"; +import { isChatSurfaceEnabled } from "./chat-surface-settings.js"; import { registerSurfaceRoutes } from "./routes/surfaces.js"; import { registerWhiteboardRoutes } from "./routes/whiteboard.js"; import { registerMcpRoutes } from "./routes/mcp.js"; @@ -837,6 +838,8 @@ async function registerRoutes() { injectAgentPrompt(agentId, prompt, { swallowFailure: false }), onAgentStarted: (agentId) => surfaceService.notifyQueuedAfterResume(agentId), + chat: chatService, + isChatSurfaceEnabled: () => isChatSurfaceEnabled(pool), }); // --- Personas --- diff --git a/apps/server/test/chat-feed.test.ts b/apps/server/test/chat-feed.test.ts index 1ced4095..f5821397 100644 --- a/apps/server/test/chat-feed.test.ts +++ b/apps/server/test/chat-feed.test.ts @@ -36,6 +36,7 @@ beforeEach(async () => { await pool.query("DELETE FROM agent_events"); await pool.query("DELETE FROM agent_messages"); await pool.query("DELETE FROM media"); + await pool.query("DELETE FROM reviews"); }); const at = (s: number) => new Date(Date.UTC(2026, 0, 1, 0, 0, s)); @@ -155,10 +156,15 @@ describe("composeChatFeed", () => { }); it("never drops or repeats rows that share a timestamp across sources", async () => { - // Nine rows at the same instant (three per source kind plus chat) with + // Twelve rows at the same instant (three per source kind) with // microsecond-identical created_at, paged two at a time. const t = at(10); for (let i = 0; i < 3; i++) { + await pool.query( + `INSERT INTO reviews (agent_id, reviewer_type, summary, created_at) + VALUES ($1, 'human', $2, $3)`, + [A, `review ${i}`, t] + ); await pool.query( `INSERT INTO agent_events (agent_id, event_type, message, created_at) VALUES ($1, 'working', $2, $3)`, @@ -198,9 +204,9 @@ describe("composeChatFeed", () => { expect(cursor).toBeTruthy(); expect(pages).toBeLessThan(20); } - expect(new Set(seen).size).toBe(9); - expect(seen).toHaveLength(9); - expect(pages).toBe(5); + expect(new Set(seen).size).toBe(12); + expect(seen).toHaveLength(12); + expect(pages).toBe(6); }); it("round-trips cursors and rejects foreign ones", () => { @@ -234,6 +240,10 @@ describe("composeChatFeed", () => { expect(forged({ ...cursor, type: "media", id: "7" })).toMatchObject({ id: "7", }); + expect(forged({ ...cursor, type: "review", id: "7" })).toMatchObject({ + id: "7", + }); + expect(forged({ ...cursor, type: "review", id: uuid })).toBeNull(); // Shape-valid but impossible instants. expect(forged({ ...cursor, at: "2026-02-30 00:00:00.000000" })).toBeNull(); expect(forged({ ...cursor, at: "2026-01-01 25:00:00.000000" })).toBeNull(); @@ -295,6 +305,69 @@ describe("composeChatFeed", () => { expect(names).not.toContain("from-composer.png"); }); + it("surfaces a review with its reviewer and live counts", async () => { + const reviewer = "agt_feed_reviewer"; + await pool.query( + `INSERT INTO agents (id, name, cwd, status, persona) + VALUES ($1, 'Reviewer', '/tmp', 'stopped', 'backend-security') + ON CONFLICT (id) DO NOTHING`, + [reviewer] + ); + const review = await pool.query<{ id: number }>( + `INSERT INTO reviews + (agent_id, reviewer_type, reviewer_agent_id, summary, status, created_at) + VALUES ($1, 'agent', $2, 'Two things to fix', 'partially_resolved', $3) + RETURNING id`, + [A, reviewer, at(3)] + ); + const reviewId = review.rows[0]!.id; + await pool.query( + `INSERT INTO review_feedback_items (review_id, status) + VALUES ($1, 'resolved'), ($1, 'open')`, + [reviewId] + ); + // Another agent's review must not reach this feed. + await pool.query( + `INSERT INTO reviews (agent_id, reviewer_type, summary, created_at) + VALUES ($1, 'human', 'not mine', $2)`, + [OTHER, at(4)] + ); + + const feed = await composeChatFeed(store, A); + expect(feed.entries).toHaveLength(1); + expect(feed.entries[0]).toEqual({ + type: "review", + id: `review:${reviewId}`, + reviewId, + reviewerType: "agent", + reviewerAgentId: reviewer, + reviewerName: "backend-security", + summary: "Two things to fix", + status: "partially_resolved", + itemCount: 2, + resolvedCount: 1, + at: at(3).toISOString(), + }); + }); + + it("surfaces a human review with no feedback items", async () => { + await pool.query( + `INSERT INTO reviews (agent_id, reviewer_type, summary, status, created_at) + VALUES ($1, 'human', 'Looks good', 'resolved', $2)`, + [A, at(2)] + ); + const feed = await composeChatFeed(store, A); + expect(feed.entries[0]).toMatchObject({ + type: "review", + reviewerType: "human", + reviewerAgentId: null, + reviewerName: null, + itemCount: 0, + resolvedCount: 0, + status: "resolved", + }); + }); + it("returns an empty feed for an agent with nothing", async () => { const feed = await composeChatFeed(store, "agt_feed_nobody"); expect(feed).toEqual({ diff --git a/apps/server/test/chat-user-prompt.test.ts b/apps/server/test/chat-user-prompt.test.ts new file mode 100644 index 00000000..387c4d3c --- /dev/null +++ b/apps/server/test/chat-user-prompt.test.ts @@ -0,0 +1,132 @@ +import { describe, expect, it, vi } from "vitest"; + +import { + deliverUserPrompt, + routesUserPromptThroughChat, +} from "../src/chat/user-prompt.js"; + +describe("routesUserPromptThroughChat", () => { + const base = { + chatSurfaceEnabled: true, + agentType: "claude" as string | null | undefined, + submit: true, + }; + + it("sends a submitted prompt through Chat when the surface is on", () => { + expect(routesUserPromptThroughChat(base)).toBe(true); + }); + + it("keeps the pane path with the surface off", () => { + // Nothing about these controls changes until the flag is on. + expect( + routesUserPromptThroughChat({ ...base, chatSurfaceEnabled: false }) + ).toBe(false); + }); + + it("keeps the pane path for a phrase pasted to be edited", () => { + // `submit: false` puts text in the CLI's composer for the user to finish; + // there is no message yet, so there is nothing to post. + expect(routesUserPromptThroughChat({ ...base, submit: false })).toBe(false); + }); + + it("keeps the pane path for a terminal session", () => { + // No CLI behind it to read the envelope or answer with dispatch_chat_post. + expect( + routesUserPromptThroughChat({ ...base, agentType: "terminal" }) + ).toBe(false); + }); + + it("treats a typeless agent as chat-capable", () => { + // Only a terminal session is excluded; an older row with no type is a + // CLI agent like any other. + expect(routesUserPromptThroughChat({ ...base, agentType: null })).toBe( + true + ); + expect(routesUserPromptThroughChat({ ...base, agentType: undefined })).toBe( + true + ); + }); +}); + +describe("deliverUserPrompt", () => { + function fakeDeps( + overrides: { + chatSurfaceEnabled?: boolean; + agent?: { type?: string | null } | null; + getAgentRejects?: boolean; + } = {} + ) { + const sendUserMessage = vi.fn(async () => ({})); + return { + sendUserMessage, + deps: { + isChatSurfaceEnabled: async () => overrides.chatSurfaceEnabled ?? true, + getAgent: async () => { + if (overrides.getAgentRejects) throw new Error("boom"); + return overrides.agent === undefined + ? { type: "claude" } + : overrides.agent; + }, + sendUserMessage, + }, + }; + } + + it("sends the prompt as a Chat message and reports it delivered", async () => { + const { deps, sendUserMessage } = fakeDeps(); + await expect( + deliverUserPrompt(deps, "agt_1", "run the tests", true) + ).resolves.toBe(true); + // The user's words, verbatim: the service wraps them in the envelope. + expect(sendUserMessage).toHaveBeenCalledWith("agt_1", "run the tests"); + }); + + it("leaves it to the pane when the surface is off", async () => { + const { deps, sendUserMessage } = fakeDeps({ chatSurfaceEnabled: false }); + await expect( + deliverUserPrompt(deps, "agt_1", "run the tests", true) + ).resolves.toBe(false); + expect(sendUserMessage).not.toHaveBeenCalled(); + }); + + it("leaves it to the pane for a terminal session", async () => { + const { deps, sendUserMessage } = fakeDeps({ agent: { type: "terminal" } }); + await expect( + deliverUserPrompt(deps, "agt_1", "run the tests", true) + ).resolves.toBe(false); + expect(sendUserMessage).not.toHaveBeenCalled(); + }); + + it("leaves it to the pane when the phrase is not being submitted", async () => { + const { deps, sendUserMessage } = fakeDeps(); + await expect( + deliverUserPrompt(deps, "agt_1", "half a thought", false) + ).resolves.toBe(false); + expect(sendUserMessage).not.toHaveBeenCalled(); + }); + + it("leaves it to the pane for an agent it cannot read", async () => { + // The route's own lookup answers with 404/409; Chat must not pre-empt it. + for (const overrides of [{ agent: null }, { getAgentRejects: true }]) { + const { deps, sendUserMessage } = fakeDeps(overrides); + await expect( + deliverUserPrompt(deps, "agt_missing", "run the tests", true) + ).resolves.toBe(false); + expect(sendUserMessage).not.toHaveBeenCalled(); + } + }); + + it("propagates a refused send so the route can map its status", async () => { + const sendUserMessage = vi.fn(async () => { + throw new Error("Agent is not running."); + }); + const deps = { + isChatSurfaceEnabled: async () => true, + getAgent: async () => ({ type: "claude" }), + sendUserMessage, + }; + await expect( + deliverUserPrompt(deps, "agt_1", "run the tests", true) + ).rejects.toThrow(/not running/); + }); +}); diff --git a/apps/server/test/pin-run-route.test.ts b/apps/server/test/pin-run-route.test.ts index 5943c00d..89ac89c5 100644 --- a/apps/server/test/pin-run-route.test.ts +++ b/apps/server/test/pin-run-route.test.ts @@ -41,8 +41,24 @@ async function setPins( beforeEach(async () => { await ctx.pool.query("DELETE FROM agents"); + await ctx.pool.query( + "DELETE FROM settings WHERE key = 'chat_surface_enabled'" + ); }); +async function enableChatSurface(): Promise { + const res = await ctx.app.inject({ + method: "POST", + url: "/api/v1/app/settings/chat-surface", + headers: { + cookie: await ctx.sessionCookie(), + "content-type": "application/json", + }, + payload: { enabled: true }, + }); + expect(res.statusCode).toBe(200); +} + // The prompt is looked up server-side by pin ID, so this route is the boundary // that decides what a click is allowed to inject. Every rejection path matters: // a client must not be able to fire text the agent never pinned. @@ -120,6 +136,55 @@ describe("POST /api/v1/agents/:id/terminal/inject-pin/:pinId", () => { expect(res.statusCode).toBe(409); }); + it("leaves no Chat post behind when the prompt could not be delivered", async () => { + // With the Chat surface on the click becomes a Chat message, but the + // agent is inert: the send refuses before writing, so a click that never + // reached the agent must not leave a post claiming it did. + await enableChatSurface(); + const agent = await createAgent(); + await setPins(agent.id, [ + { id: "p1", label: "Go", value: "do the thing", type: "shortcut" }, + ]); + + const res = await authedInject( + "POST", + `/api/v1/agents/${agent.id}/terminal/inject-pin/p1` + ); + expect(res.statusCode).toBe(409); + const rows = await ctx.pool.query( + "SELECT id FROM agent_chat_messages WHERE agent_id = $1", + [agent.id] + ); + expect(rows.rowCount).toBe(0); + }); + + it("still refuses a disabled pin with the Chat surface on", async () => { + // The pin is the trust boundary either way — routing through Chat must + // not become a way around it. + await enableChatSurface(); + const agent = await createAgent(); + await setPins(agent.id, [ + { + id: "p1", + label: "Launch", + value: "do the thing", + type: "shortcut", + disabled: true, + }, + ]); + + const res = await authedInject( + "POST", + `/api/v1/agents/${agent.id}/terminal/inject-pin/p1` + ); + expect(res.statusCode).toBe(400); + const rows = await ctx.pool.query( + "SELECT id FROM agent_chat_messages WHERE agent_id = $1", + [agent.id] + ); + expect(rows.rowCount).toBe(0); + }); + it("rejects an unauthenticated request", async () => { const agent = await createAgent(); await setPins(agent.id, [ diff --git a/apps/web/src/components/app/agent-pane.tsx b/apps/web/src/components/app/agent-pane.tsx index a178a5cc..853164ff 100644 --- a/apps/web/src/components/app/agent-pane.tsx +++ b/apps/web/src/components/app/agent-pane.tsx @@ -164,6 +164,8 @@ export type AgentPaneProps = { */ header: boolean; openLightbox: (file: MediaFile) => void; + /** Opens a review in the Reviews sidebar; from a review card in the feed. */ + onOpenReview?: (reviewId: number) => void; isMobile: boolean; }; @@ -193,6 +195,7 @@ export function AgentPane({ terminalSlotRef, header, openLightbox, + onOpenReview, isMobile, }: AgentPaneProps): JSX.Element { const chatShown = chatEnabled && view === "chat"; @@ -237,6 +240,7 @@ export function AgentPane({ childAgentIds={childAgentIds} onShowChildAgentsChange={onShowChildAgentsChange} openLightbox={openLightbox} + onOpenReview={onOpenReview} isMobile={isMobile} /> diff --git a/apps/web/src/components/app/agents-view.tsx b/apps/web/src/components/app/agents-view.tsx index c4c13673..61579f9a 100644 --- a/apps/web/src/components/app/agents-view.tsx +++ b/apps/web/src/components/app/agents-view.tsx @@ -403,7 +403,12 @@ export function AgentsView({ [focusedAgentId, isMobile, navTo, setMobileMediaOpen] ); - const handleReviewSubmitted = useCallback( + /** + * Show one review: the Reviews sidebar, opened on that review. Reached + * from the Changes tab after submitting one, and from a review card in + * the Chat feed. + */ + const handleOpenReview = useCallback( (reviewId: number) => { if (!focusedAgentId) return; navTo(`/agents/${focusedAgentId}?expandReview=${reviewId}`, { @@ -569,7 +574,7 @@ export function AgentsView({ agentId={focusedAgentId} active={true} isMobile={isMobile} - onReviewSubmitted={handleReviewSubmitted} + onReviewSubmitted={handleOpenReview} /> ) : null; @@ -622,6 +627,7 @@ export function AgentsView({ onShowChildAgentsChange: setShowChildAgents, childAgentIds: focusedSubAgentIds, openLightbox, + onOpenReview: handleOpenReview, isMobile, }; // Only in a split: the single-pane Agent pane is always rendered (hidden diff --git a/apps/web/src/components/app/chat/chat-entries.tsx b/apps/web/src/components/app/chat/chat-entries.tsx index d4d1cab6..bad33201 100644 --- a/apps/web/src/components/app/chat/chat-entries.tsx +++ b/apps/web/src/components/app/chat/chat-entries.tsx @@ -5,6 +5,7 @@ import type { ChatMediaEntry, ChatMessage, ChatQuestionOption, + ChatReviewEntry, ChatStatusEntry, } from "@dispatch/shared"; import { @@ -27,6 +28,10 @@ import { import { AgentRelationBadge } from "@/components/app/agent-relation-badge"; import { AgentTypeIcon } from "@/components/app/agent-type-icon"; import { PinItem } from "@/components/app/pin-item"; +import { + reviewerLabel, + ReviewSummaryBlock, +} from "@/components/app/review-summary-block"; import { type Agent, type AgentPin, @@ -129,6 +134,8 @@ export type FeedContext = { pins: AgentPin[]; workspaceRoot: string | null; onOpenMedia: (file: MediaFile) => void; + /** Opens a review in the Reviews sidebar, expanded. */ + onOpenReview?: (reviewId: number) => void; }; export type PostAuthor = { @@ -1090,3 +1097,82 @@ export function MediaEntryView({ ); } + +/** + * Who a review card reads as: the reviewer agent that submitted it, or the + * user for a review left by hand in the Changes tab. Its own group key, so + * a review card never collapses into an adjacent post's header — the card + * carries its own heading. + * + * The name is the server's `reviewerName` (the persona the agent reviewed + * as, falling back to its own name), which is what the block below the + * header says too — one actor must not read as two names in one post. The + * peer directory is only a fallback for a review whose reviewer the list no + * longer knows. + */ +export function reviewAuthor( + entry: ChatReviewEntry, + ctx: FeedContext +): PostAuthor { + if (entry.reviewerType === "agent" && entry.reviewerAgentId) { + const peer = ctx.peers?.[entry.reviewerAgentId]; + const author = peerAuthor( + entry.reviewerAgentId, + entry.reviewerName ?? + peer?.name ?? + reviewerLabel(entry.reviewerType, entry.reviewerName), + ctx + ); + return { ...author, key: `review:${entry.reviewerAgentId}` }; + } + return { ...userAuthor(), key: "review:human" }; +} + +/** + * A review in the channel, as the same block the Reviews sidebar shows for + * a collapsed review: who left it, how much is still open, its status. + * Clicking opens that review in the sidebar, where the summary and the + * feedback items live — the card is the notice, not a second copy of it. + */ +export function ReviewEntryView({ + entry, + grouped, + rule = false, + ctx, +}: { + entry: ChatReviewEntry; + grouped: boolean; + rule?: boolean; + ctx: FeedContext; +}): JSX.Element { + const { onOpenReview } = ctx; + return ( + + onOpenReview(entry.reviewId) : undefined} + ariaLabel={`Open review from ${reviewerLabel( + entry.reviewerType, + entry.reviewerName + )}`} + className="mt-1 max-w-sm" + data-testid="chat-review-block" + /> + + ); +} diff --git a/apps/web/src/components/app/chat/chat-feed.test.tsx b/apps/web/src/components/app/chat/chat-feed.test.tsx index a4d64bed..8858a267 100644 --- a/apps/web/src/components/app/chat/chat-feed.test.tsx +++ b/apps/web/src/components/app/chat/chat-feed.test.tsx @@ -1211,6 +1211,78 @@ describe("ChatFeed", () => { expect(first.textContent).toContain("Sending"); }); + it("renders a review card and opens the review it links to", () => { + const onOpenReview = vi.fn(); + renderFeed( + [ + { + type: "review", + id: "review:12", + reviewId: 12, + reviewerType: "agent", + reviewerAgentId: "agt_reviewer", + reviewerName: "backend-security", + summary: "Two things to fix", + status: "partially_resolved", + itemCount: 3, + resolvedCount: 1, + at: "2026-09-02T10:00:00.000Z", + }, + ], + {}, + { + onOpenReview, + peers: peerDirectory(AGENT_ID, [ + { + id: "agt_reviewer", + name: "Reviewer", + type: "codex", + parentAgentId: AGENT_ID, + }, + ]), + } + ); + const card = screen.getByTestId("chat-review"); + // Header and block name the same actor: the persona it reviewed as. + expect( + card.querySelector("[data-testid='chat-post-author']")?.textContent + ).toBe("backend-security"); + expect(card.textContent).toContain("Review · backend-security"); + expect(card.textContent).toContain("1/3 resolved"); + expect(card.textContent).toContain("Open"); + // The collapsed block is a status line, not a copy of the review body. + expect(card.textContent).not.toContain("Two things to fix"); + fireEvent.click( + screen.getByRole("button", { name: /open review from backend-security/i }) + ); + expect(onOpenReview).toHaveBeenCalledWith(12); + }); + + it("attributes a human review to the user and says when it approved", () => { + renderFeed([ + { + type: "review", + id: "review:13", + reviewId: 13, + reviewerType: "human", + reviewerAgentId: null, + reviewerName: null, + summary: null, + status: "resolved", + itemCount: 0, + resolvedCount: 0, + at: "2026-09-02T10:00:00.000Z", + }, + ]); + const card = screen.getByTestId("chat-review"); + expect( + card.querySelector("[data-testid='chat-post-author']")?.textContent + ).toBe("You"); + expect(card.textContent).toContain("Review · Human reviewer"); + expect(card.textContent).toContain("Approved · no feedback"); + expect(card.textContent).toContain("Resolved"); + }); + it("renders media entries and opens them in the lightbox", () => { const { onOpenMedia } = renderFeed([ { diff --git a/apps/web/src/components/app/chat/chat-feed.tsx b/apps/web/src/components/app/chat/chat-feed.tsx index 8801c041..28d93938 100644 --- a/apps/web/src/components/app/chat/chat-feed.tsx +++ b/apps/web/src/components/app/chat/chat-feed.tsx @@ -15,6 +15,8 @@ import { DayDivider, dayLabel, MediaEntryView, + reviewAuthor, + ReviewEntryView, StatusLine, } from "@/components/app/chat/chat-entries"; @@ -177,6 +179,8 @@ function authorKey( return agentMessageAuthor(entry, ctx).key; case "media": return "agent"; + case "review": + return reviewAuthor(entry, ctx).key; } } @@ -378,6 +382,15 @@ export function ChatFeed({ ctx={ctx} /> ); + case "review": + return ( + + ); } })(); return ( diff --git a/apps/web/src/components/app/chat/chat-pane.tsx b/apps/web/src/components/app/chat/chat-pane.tsx index 22055b0d..99467b5b 100644 --- a/apps/web/src/components/app/chat/chat-pane.tsx +++ b/apps/web/src/components/app/chat/chat-pane.tsx @@ -56,6 +56,8 @@ export type ChatPaneProps = { childAgentIds: readonly string[]; onShowChildAgentsChange: (show: boolean) => void; openLightbox: (file: MediaFile) => void; + /** Opens a review in the Reviews sidebar, expanded; from a review card. */ + onOpenReview?: (reviewId: number) => void; isMobile: boolean; }; @@ -118,6 +120,7 @@ export function ChatPane({ childAgentIds, onShowChildAgentsChange, openLightbox, + onOpenReview, isMobile, }: ChatPaneProps): JSX.Element { const feed = useChatFeed(agentId); @@ -346,6 +349,7 @@ export function ChatPane({ pins, workspaceRoot: agent?.worktreePath ?? agent?.cwd ?? null, onOpenMedia: openLightbox, + onOpenReview, }), [ agent?.cwd, @@ -353,6 +357,7 @@ export function ChatPane({ agent?.type, agent?.worktreePath, agentId, + onOpenReview, openLightbox, peers, pins, diff --git a/apps/web/src/components/app/review-summary-block.tsx b/apps/web/src/components/app/review-summary-block.tsx new file mode 100644 index 00000000..a72aebbe --- /dev/null +++ b/apps/web/src/components/app/review-summary-block.tsx @@ -0,0 +1,174 @@ +import { type ReactNode, type RefObject } from "react"; +import { + Bot, + CheckCircle2, + ChevronRight, + Clock, + MessageCircle, + User, +} from "lucide-react"; + +import { cn } from "@/lib/utils"; + +/** + * How a review's status reads: the rail down its left edge, the badge, and + * the word for it. `partially_resolved` says "Open" too — from the reader's + * side there is still something to do, and the counts say how much. + */ +export const REVIEW_STATUS_STYLES: Record< + string, + { rail: string; badge: string; label: string } +> = { + open: { + rail: "border-l-status-waiting/60", + badge: "bg-status-waiting/15 text-status-waiting", + label: "Open", + }, + partially_resolved: { + rail: "border-l-status-waiting/60", + badge: "bg-status-waiting/15 text-status-waiting", + label: "Open", + }, + resolved: { + rail: "border-l-status-working/60", + badge: "bg-status-working/15 text-status-working", + label: "Resolved", + }, +}; + +const DEFAULT_REVIEW_STYLE = REVIEW_STATUS_STYLES.open!; + +export function reviewStatusStyle( + status: string +): (typeof REVIEW_STATUS_STYLES)[string] { + return REVIEW_STATUS_STYLES[status] ?? DEFAULT_REVIEW_STYLE; +} + +/** Who left a review, as one line: a reviewer's persona, or a person. */ +export function reviewerLabel( + reviewerType: string, + reviewerName: string | null +): string { + if (reviewerType !== "agent") return "Human reviewer"; + return reviewerName || "Review agent"; +} + +/** The fields the block shows — the shape both the sidebar and Chat have. */ +export type ReviewSummary = { + reviewerType: string; + reviewerName: string | null; + status: string; + itemCount: number; + resolvedCount: number; + createdAt: string; +}; + +/** + * A review at a glance: who left it, how much of it is still open, and its + * status — the block the Reviews sidebar shows when a review is collapsed, + * and the same block the Chat feed shows when one lands. + * + * The sidebar uses it as the header of an expandable row (`expanded` renders + * the chevron and squares off the bottom corners); the Chat feed uses it as + * a plain button that opens the review in the sidebar, with no chevron and + * no date — the post it sits in is already stamped with the time. + */ +export function ReviewSummaryBlock({ + review, + expanded, + showTime = true, + onClick, + headerRef, + className, + buttonClassName, + ariaLabel, + ...rest +}: { + review: ReviewSummary; + /** Omit for a block that does not expand: no chevron is rendered. */ + expanded?: boolean; + /** The sidebar stamps each review; in Chat the post's own time serves. */ + showTime?: boolean; + onClick?: () => void; + headerRef?: RefObject; + className?: string; + buttonClassName?: string; + ariaLabel?: string; + [dataAttr: `data-${string}`]: string | undefined; +}): JSX.Element { + const style = reviewStatusStyle(review.status); + const label = reviewerLabel(review.reviewerType, review.reviewerName); + const time = new Date(review.createdAt).toLocaleDateString(undefined, { + month: "short", + day: "numeric", + hour: "2-digit", + minute: "2-digit", + }); + const meta: ReactNode = ( +
+ {review.itemCount === 0 ? ( + + + Approved · no feedback + + ) : ( + + + {review.resolvedCount}/{review.itemCount} resolved + + )} + {showTime ? ( + + + {time} + + ) : null} + + {style.label} + +
+ ); + return ( +
+ +
+ ); +} diff --git a/apps/web/src/components/app/reviews-sidebar-row.tsx b/apps/web/src/components/app/reviews-sidebar-row.tsx index 396bbd4a..50961dd1 100644 --- a/apps/web/src/components/app/reviews-sidebar-row.tsx +++ b/apps/web/src/components/app/reviews-sidebar-row.tsx @@ -1,12 +1,5 @@ import { useEffect, useRef, useState } from "react"; -import { - Bot, - CheckCircle2, - ChevronRight, - Clock, - MessageCircle, - User, -} from "lucide-react"; +import { CheckCircle2 } from "lucide-react"; import { AnimatePresence, motion } from "framer-motion"; import { @@ -16,29 +9,11 @@ import { import { cn } from "@/lib/utils"; import { Markdown } from "@/components/ui/markdown"; import { FeedbackItemRow } from "@/components/app/reviews-feedback-item"; - -const REVIEW_STATUS_STYLES: Record< - string, - { rail: string; badge: string; label: string } -> = { - open: { - rail: "border-l-status-waiting/60", - badge: "bg-status-waiting/15 text-status-waiting", - label: "Open", - }, - partially_resolved: { - rail: "border-l-status-waiting/60", - badge: "bg-status-waiting/15 text-status-waiting", - label: "Open", - }, - resolved: { - rail: "border-l-status-working/60", - badge: "bg-status-working/15 text-status-working", - label: "Resolved", - }, -}; - -const DEFAULT_REVIEW_STYLE = REVIEW_STATUS_STYLES.open!; +import { + reviewerLabel, + reviewStatusStyle, + ReviewSummaryBlock, +} from "@/components/app/review-summary-block"; export function ReviewRow({ agentId, @@ -112,20 +87,13 @@ export function ReviewRow({ }; }, [expanded]); - const statusStyle = - REVIEW_STATUS_STYLES[review.status] ?? DEFAULT_REVIEW_STYLE; - - const date = new Date(review.createdAt); - const timeStr = date.toLocaleDateString(undefined, { + const statusStyle = reviewStatusStyle(review.status); + const timeStr = new Date(review.createdAt).toLocaleDateString(undefined, { month: "short", day: "numeric", hour: "2-digit", minute: "2-digit", }); - const reviewerLabel = - review.reviewerType === "agent" - ? review.reviewerName || "Review agent" - : "Human reviewer"; return (
-
- -
+ buttonClassName={cn( + expanded && "rounded-b-none", + pinned && "rounded-none" + )} + /> {expanded && ( { expectInvalidatedSet(invalidateQueries, [ ["agent-reviews", "author"], ["agent-feedback-items", "author"], + // The Chat feed carries a card per review. + ["chat", "author"], ]); }); @@ -764,6 +766,8 @@ describe("useSSE message handling", () => { expectInvalidatedSet(invalidateQueries, [ ["agent-reviews", "author"], ["agent-feedback-items", "author"], + // The Chat feed carries a card per review. + ["chat", "author"], ]); }); diff --git a/apps/web/src/hooks/use-sse.ts b/apps/web/src/hooks/use-sse.ts index 60bfb6f7..b3a52fa3 100644 --- a/apps/web/src/hooks/use-sse.ts +++ b/apps/web/src/hooks/use-sse.ts @@ -350,6 +350,10 @@ export function useSSE(authState: AuthState): void { void queryClient.invalidateQueries({ queryKey: ["agent-feedback-items", payload.agentId], }); + // The Chat feed renders reviews as cards, with their live status + // and counts — so a new review, and every later change to one, + // has to reach the feed too. + invalidateChatFeed(queryClient, payload.agentId); return; } diff --git a/packages/shared/src/chat-types.ts b/packages/shared/src/chat-types.ts index f200628b..437e8948 100644 --- a/packages/shared/src/chat-types.ts +++ b/packages/shared/src/chat-types.ts @@ -148,6 +148,28 @@ export type ChatMediaEntry = { at: string; }; +/** + * A review submitted against this agent's work (`reviews`), surfaced as a + * card in the feed. Derived at read time, so the counts and the status are + * always the review's current ones — the card is a live link to the review + * in the Reviews sidebar, not a snapshot of when it landed. + */ +export type ChatReviewEntry = { + type: "review"; + id: string; + reviewId: number; + /** Who left it: an agent reviewer, or a person using the Changes tab. */ + reviewerType: "human" | "agent"; + reviewerAgentId: string | null; + /** The reviewer agent's persona or name; null for a human review. */ + reviewerName: string | null; + summary: string | null; + status: string; + itemCount: number; + resolvedCount: number; + at: string; +}; + export type ChatMessageEntry = { type: "chat"; id: string; @@ -159,7 +181,8 @@ export type ChatFeedEntry = | ChatMessageEntry | ChatStatusEntry | ChatAgentMessageEntry - | ChatMediaEntry; + | ChatMediaEntry + | ChatReviewEntry; export type ChatFeedResponse = { entries: ChatFeedEntry[]; diff --git a/packages/shared/src/index.ts b/packages/shared/src/index.ts index bfebb977..bf30d4f1 100644 --- a/packages/shared/src/index.ts +++ b/packages/shared/src/index.ts @@ -45,6 +45,7 @@ export type { ChatMessageOrigin, ChatQuestion, ChatQuestionOption, + ChatReviewEntry, ChatSendRequest, ChatSendResponse, ChatStatusEntry,