From fc59ad20a44e7eff0397601dd8a00737b06fb853 Mon Sep 17 00:00:00 2001 From: Brad Harris Date: Fri, 4 Sep 2026 11:26:35 -0600 Subject: [PATCH] feat(chat): reviews in the stream; quick prompts as chat messages MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two things a user can see in an agent's Chat feed that were invisible there before. Reviews now render as a block in the stream, linking to the review itself. The entry is derived at read time from `reviews` — a fifth feed source alongside chat messages, status events, agent-to-agent messages and shared media — so a review's status and counts in the feed are always its current ones, with no second write path to keep in sync and no backfill for reviews that already exist. Ranked above the other sources so existing cursors keep their meaning; `cursorClause` gained an optional table alias, since the review source is the only one whose query joins. The block is the Reviews sidebar's own collapsed row, extracted as `ReviewSummaryBlock`: the sidebar renders it with a chevron and a date, the feed without either (the post it sits in is already stamped). Clicking it opens that review in the sidebar, through the same handler the Changes tab already used after submitting one. Quick phrases and shortcut pins now deliver as Chat messages when the surface is on: the same `sendUserMessage` the composer calls, so they get a user row, the envelope carrying its id, a post in the feed, and reply threading. A phrase pasted to be edited (`submit: false`) and a terminal session keep the pane path, as does everything with the flag off. These two clicks now honour the quiet gate, as a composed message does; the pin remains the trust boundary, and an undeliverable click still fails before any row is written. Co-Authored-By: Claude Opus 5 --- apps/server/src/chat/feed.ts | 102 ++++++++-- apps/server/src/chat/user-prompt.ts | 82 +++++++++ apps/server/src/routes/agents/shared.ts | 8 + .../src/routes/agents/terminal-routes.ts | 41 +++++ apps/server/src/server.ts | 3 + apps/server/test/chat-feed.test.ts | 81 +++++++- apps/server/test/chat-user-prompt.test.ts | 132 +++++++++++++ apps/server/test/pin-run-route.test.ts | 65 +++++++ apps/web/src/components/app/agent-pane.tsx | 4 + apps/web/src/components/app/agents-view.tsx | 10 +- .../src/components/app/chat/chat-entries.tsx | 86 +++++++++ .../components/app/chat/chat-feed.test.tsx | 72 ++++++++ .../web/src/components/app/chat/chat-feed.tsx | 13 ++ .../web/src/components/app/chat/chat-pane.tsx | 5 + .../components/app/review-summary-block.tsx | 174 ++++++++++++++++++ .../components/app/reviews-sidebar-row.tsx | 124 +++---------- apps/web/src/hooks/use-sse.test.ts | 4 + apps/web/src/hooks/use-sse.ts | 4 + packages/shared/src/chat-types.ts | 25 ++- packages/shared/src/index.ts | 1 + 20 files changed, 914 insertions(+), 122 deletions(-) create mode 100644 apps/server/src/chat/user-prompt.ts create mode 100644 apps/server/test/chat-user-prompt.test.ts create mode 100644 apps/web/src/components/app/review-summary-block.tsx 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,