From 92afb826d93fe93f0d0d877dd0b3c6ed0c6debe0 Mon Sep 17 00:00:00 2001 From: Mohamed Boudra Date: Thu, 30 Apr 2026 21:35:58 +0800 Subject: [PATCH] Add inline review comments to git diff pane (#530) * Add inline git diff review comments Adds a review draft store keyed per diff target, inline gutter + thread rendering in the diff pane, and a generated review attachment that flows through the composer and agent providers. * Strip generated review attachments before saving to draft store Drafts persist only UserComposerAttachment (no review kind), so filter the auto-generated review attachment out when seeding the draft or restoring after a failed auto-submit. * Fix inline review typecheck and test harness drift * Unslop inline review feature - Fix stale-closure bug in composer queueMessage: clear drafts for the actually-queued attachments, not the latest props - Drop dead useEffect mirroring initialBody in inline review editor (parent already remounts via key) - Restore typed AgentAttachment signature on renderPromptAttachmentAsText; delete schema redispatch helper and drop null-checks across 5 providers - Inline single-use review prompt formatters and replace marker switch with const map - Drop normalizeComment field-copy redundancy after type guard - Remove __reviewDraftStoreTestUtils export; test migration via rehydrate() public path - Refactor findTarget to return the target line directly (no non-null assertion at call site) - Drop redundant isGeneratedReviewAttachment helper; inline kind check * Update review pill to withUnistyles after rebase Main's unistyles refactor stripped useUnistyles in favor of withUnistyles + StyleSheet. Convert ReviewAttachmentPill to use ThemedCircleDot + ICON_SIZE so the icon picks up the foreground-muted mapping like the other pills. * Fix lint and typecheck after rebase onto main Resolve drift after main's withUnistyles refactor: - Composer complexity: dedup voice?.isVoiceSwitching to drop to 20 - Move per-comment Pressable handlers in git-diff-inline-review into CommentRow subcomponent so callbacks are stable - Hoist style/onPress factories to module level (iconButton, iconButtonDestructive, ghostButton) - Memoize style arrays in git-diff-pane review wrappers - Replace inline {} placeholders with useMemo styles - review-draft-store: extract applyCommentUpdates to avoid map-spread, include input.key in useMemo deps - Test fixtures: hoist EMPTY_COMMENTS, add buildReviewActions helper * Drop GitDiffPane default param to keep complexity at 20 After the rebase, main's `enabled = true` default param combined with the inline review additions pushed cyclomatic complexity to 21. Drop the default and resolve `enabled !== false` at the call site (comparison doesn't count toward complexity). * Reshape inline review behind @/review module Drop the `generated: true` attachment flag and consolidate inline-review logic behind a deep `@/review` module so composer and git-diff-pane stop hand-composing review primitives. Workspace-scoped review snapshots are now derived at submit/queue time rather than persisted in the draft store. Composer review references drop from 51 to 4; git-diff-pane drops ~108 lines and now consumes a controller hook + slot components. * Fix optimistic review attachments --- .../composer-workspace-attachments.tsx | 280 ++++++++ packages/app/src/attachments/types.ts | 14 +- .../workspace-attachments-store.test.ts | 89 +++ .../workspace-attachments-store.ts | 115 +++ .../app/src/components/agent-stream-view.tsx | 1 + .../src/components/composer-attachments.ts | 9 + packages/app/src/components/composer.test.tsx | 328 ++++++++- packages/app/src/components/composer.tsx | 145 +++- packages/app/src/components/git-diff-pane.tsx | 468 ++++++++++--- packages/app/src/components/message.tsx | 56 ++ .../app/src/hooks/use-agent-input-draft.ts | 16 +- .../src/hooks/use-draft-agent-create-flow.ts | 14 +- packages/app/src/panels/agent-panel.test.tsx | 1 + packages/app/src/panels/agent-panel.tsx | 46 +- packages/app/src/review/index.ts | 33 + packages/app/src/review/store.test.ts | 279 ++++++++ packages/app/src/review/store.ts | 456 ++++++++++++ packages/app/src/review/surface.test.tsx | 389 ++++++++++ packages/app/src/review/surface.tsx | 663 ++++++++++++++++++ .../app/src/screens/new-workspace-screen.tsx | 10 +- .../workspace-draft-agent-tab.test.tsx | 34 + .../workspace/workspace-draft-agent-tab.tsx | 40 +- packages/app/src/stores/create-flow-store.ts | 2 + .../src/stores/draft-store.migration.test.ts | 69 +- packages/app/src/stores/draft-store.ts | 20 +- packages/app/src/types/stream.ts | 3 +- packages/app/src/utils/diff-layout.test.ts | 179 ++++- packages/app/src/utils/diff-layout.ts | 289 +++++--- .../src/utils/pending-create-images.test.ts | 59 ++ .../app/src/utils/pending-create-images.ts | 18 +- .../server/agent/prompt-attachments.test.ts | 67 ++ .../src/server/agent/prompt-attachments.ts | 33 + .../src/server/agent/providers/acp-agent.ts | 26 +- .../server/agent/providers/claude-agent.ts | 7 +- .../providers/codex-app-server-agent.test.ts | 46 ++ .../agent/providers/codex-app-server-agent.ts | 61 +- .../server/agent/providers/opencode-agent.ts | 24 +- .../src/shared/messages.attachments.test.ts | 142 ++++ packages/server/src/shared/messages.ts | 30 + 39 files changed, 4244 insertions(+), 317 deletions(-) create mode 100644 packages/app/src/attachments/composer-workspace-attachments.tsx create mode 100644 packages/app/src/attachments/workspace-attachments-store.test.ts create mode 100644 packages/app/src/attachments/workspace-attachments-store.ts create mode 100644 packages/app/src/review/index.ts create mode 100644 packages/app/src/review/store.test.ts create mode 100644 packages/app/src/review/store.ts create mode 100644 packages/app/src/review/surface.test.tsx create mode 100644 packages/app/src/review/surface.tsx diff --git a/packages/app/src/attachments/composer-workspace-attachments.tsx b/packages/app/src/attachments/composer-workspace-attachments.tsx new file mode 100644 index 000000000..0dcad7d3a --- /dev/null +++ b/packages/app/src/attachments/composer-workspace-attachments.tsx @@ -0,0 +1,280 @@ +import { useCallback, useEffect, useMemo, useState, type ReactElement } from "react"; +import { Text, View } from "react-native"; +import { StyleSheet, withUnistyles } from "react-native-unistyles"; +import { CircleDot } from "lucide-react-native"; +import type { + ComposerAttachment, + UserComposerAttachment, + WorkspaceComposerAttachment, +} from "@/attachments/types"; +import { AttachmentPill } from "@/components/attachment-pill"; +import { ICON_SIZE, type Theme } from "@/styles/theme"; +import type { AgentAttachment } from "@server/shared/messages"; +import { useClearReviewDraft } from "@/review/store"; + +interface WorkspaceAttachmentBindingInput { + normalAttachments: UserComposerAttachment[]; + workspaceAttachments: readonly WorkspaceComposerAttachment[]; + onOpenWorkspaceAttachment?: (attachment: WorkspaceComposerAttachment) => void; +} + +interface RemoveWorkspaceAttachmentInput { + selectedAttachments: readonly ComposerAttachment[]; + index: number; +} + +interface OpenWorkspaceAttachmentInput { + attachment: ComposerAttachment; +} + +interface CompleteSubmitInput { + result: "noop" | "queued" | "submitted" | "failed"; + outgoingAttachments: readonly ComposerAttachment[]; +} + +interface ComposerWorkspaceAttachmentBinding { + selectedAttachments: ComposerAttachment[]; + buildOutgoingAttachments: (normalAttachments: UserComposerAttachment[]) => ComposerAttachment[]; + removeAttachment: (input: RemoveWorkspaceAttachmentInput) => boolean; + openAttachment: (input: OpenWorkspaceAttachmentInput) => boolean; + clearSentAttachments: (attachments: readonly ComposerAttachment[]) => void; + completeSubmit: (input: CompleteSubmitInput) => void; + resetSuppression: () => void; +} + +function getAttachmentKey(attachment: WorkspaceComposerAttachment): string { + return JSON.stringify({ + type: "review", + cwd: attachment.attachment.cwd, + mode: attachment.attachment.mode, + baseRef: attachment.attachment.baseRef ?? null, + reviewDraftKey: attachment.reviewDraftKey, + comments: attachment.attachment.comments.map((comment) => ({ + filePath: comment.filePath, + side: comment.side, + lineNumber: comment.lineNumber, + body: comment.body, + })), + }); +} + +function isWorkspaceAttachment( + attachment: ComposerAttachment | undefined, +): attachment is WorkspaceComposerAttachment { + return attachment?.kind === "review"; +} + +function userAttachmentsOnly(attachments: readonly ComposerAttachment[]): UserComposerAttachment[] { + return attachments.filter( + (attachment): attachment is UserComposerAttachment => attachment.kind !== "review", + ); +} + +function toSubmitAttachment(attachment: ComposerAttachment): AgentAttachment | null { + return isWorkspaceAttachment(attachment) ? attachment.attachment : null; +} + +function renderPill(args: RenderWorkspaceAttachmentPillArgs): ReactElement { + return ( + + ); +} + +function useWorkspaceAttachmentBinding({ + normalAttachments, + workspaceAttachments, + onOpenWorkspaceAttachment, +}: WorkspaceAttachmentBindingInput): ComposerWorkspaceAttachmentBinding { + const clearReviewDraft = useClearReviewDraft(); + const [suppressedKeys, setSuppressedKeys] = useState([]); + const workspaceAttachmentKeys = useMemo( + () => workspaceAttachments.map(getAttachmentKey), + [workspaceAttachments], + ); + const activeWorkspaceAttachments = useMemo( + () => + workspaceAttachments.filter( + (attachment, index) => !suppressedKeys.includes(workspaceAttachmentKeys[index] ?? ""), + ), + [suppressedKeys, workspaceAttachmentKeys, workspaceAttachments], + ); + + const selectedAttachments = useMemo( + () => + activeWorkspaceAttachments.length > 0 + ? [...normalAttachments, ...activeWorkspaceAttachments] + : normalAttachments, + [activeWorkspaceAttachments, normalAttachments], + ); + + useEffect(() => { + setSuppressedKeys((current) => + current.filter((suppressedKey) => workspaceAttachmentKeys.includes(suppressedKey)), + ); + }, [workspaceAttachmentKeys]); + + const buildOutgoingAttachments = useCallback( + (attachments: UserComposerAttachment[]): ComposerAttachment[] => + activeWorkspaceAttachments.length > 0 + ? [...attachments, ...activeWorkspaceAttachments] + : attachments, + [activeWorkspaceAttachments], + ); + + const suppressWorkspaceAttachment = useCallback((attachment: WorkspaceComposerAttachment) => { + const key = getAttachmentKey(attachment); + setSuppressedKeys((current) => (current.includes(key) ? current : [...current, key])); + }, []); + + const clearSentAttachments = useCallback( + (attachments: readonly ComposerAttachment[]) => { + for (const attachment of attachments) { + if (isWorkspaceAttachment(attachment)) { + clearReviewDraft({ key: attachment.reviewDraftKey }); + } + } + }, + [clearReviewDraft], + ); + + const removeAttachment = useCallback( + ({ selectedAttachments: current, index }: RemoveWorkspaceAttachmentInput) => { + const selected = current[index]; + if (isWorkspaceAttachment(selected)) { + suppressWorkspaceAttachment(selected); + return true; + } + return false; + }, + [suppressWorkspaceAttachment], + ); + + const openAttachment = useCallback( + ({ attachment }: OpenWorkspaceAttachmentInput) => { + if (!isWorkspaceAttachment(attachment)) { + return false; + } + onOpenWorkspaceAttachment?.(attachment); + return true; + }, + [onOpenWorkspaceAttachment], + ); + + const resetSuppression = useCallback(() => { + setSuppressedKeys([]); + }, []); + + const completeSubmit = useCallback( + ({ result, outgoingAttachments }: CompleteSubmitInput) => { + if (result === "submitted") { + clearSentAttachments(outgoingAttachments); + } + if (result === "queued" || result === "submitted") { + resetSuppression(); + } + }, + [clearSentAttachments, resetSuppression], + ); + + return { + selectedAttachments, + buildOutgoingAttachments, + removeAttachment, + openAttachment, + clearSentAttachments, + completeSubmit, + resetSuppression, + }; +} + +interface RenderWorkspaceAttachmentPillArgs { + attachment: WorkspaceComposerAttachment; + index: number; + disabled: boolean; + onOpen: (attachment: ComposerAttachment) => void; + onRemove: (index: number) => void; +} + +interface WorkspaceAttachmentPillProps extends Omit< + RenderWorkspaceAttachmentPillArgs, + "attachment" +> { + attachment: WorkspaceComposerAttachment; +} + +function WorkspaceAttachmentPill({ + attachment, + index, + disabled, + onOpen, + onRemove, +}: WorkspaceAttachmentPillProps) { + const label = + attachment.commentCount === 1 + ? "Review · 1 comment" + : `Review · ${attachment.commentCount} comments`; + const handleOpen = useCallback(() => { + onOpen(attachment); + }, [onOpen, attachment]); + const handleRemove = useCallback(() => { + onRemove(index); + }, [onRemove, index]); + return ( + + + + + + + {label} + + + + ); +} + +export const composerWorkspaceAttachment = { + is: isWorkspaceAttachment, + renderPill, + toSubmitAttachment, + userAttachmentsOnly, + useBinding: useWorkspaceAttachmentBinding, +}; + +const styles = StyleSheet.create((theme: Theme) => ({ + pillBody: { + minHeight: 48, + maxWidth: 260, + flexDirection: "row", + alignItems: "center", + gap: theme.spacing[2], + paddingHorizontal: theme.spacing[3], + paddingVertical: theme.spacing[2], + backgroundColor: theme.colors.surface1, + }, + pillIcon: { + width: 18, + alignItems: "center", + justifyContent: "center", + }, + pillText: { + minWidth: 0, + flexShrink: 1, + color: theme.colors.foreground, + fontSize: theme.fontSize.sm, + }, +})) as unknown as Record; + +const ThemedCircleDot = withUnistyles(CircleDot); +const iconForegroundMutedMapping = (theme: Theme) => ({ color: theme.colors.foregroundMuted }); diff --git a/packages/app/src/attachments/types.ts b/packages/app/src/attachments/types.ts index 72de571d9..f82423225 100644 --- a/packages/app/src/attachments/types.ts +++ b/packages/app/src/attachments/types.ts @@ -1,4 +1,4 @@ -import type { GitHubSearchItem } from "@server/shared/messages"; +import type { AgentAttachment, GitHubSearchItem } from "@server/shared/messages"; export type AttachmentStorageType = "web-indexeddb" | "desktop-file" | "native-file"; @@ -20,7 +20,17 @@ export interface AttachmentMetadata { export type ComposerAttachment = | { kind: "image"; metadata: AttachmentMetadata } | { kind: "github_issue"; item: GitHubSearchItem } - | { kind: "github_pr"; item: GitHubSearchItem }; + | { kind: "github_pr"; item: GitHubSearchItem } + | { + kind: "review"; + attachment: Extract; + reviewDraftKey: string; + commentCount: number; + }; + +export type UserComposerAttachment = Exclude; + +export type WorkspaceComposerAttachment = Extract; export type AttachmentDataSource = | { kind: "base64"; base64: string } diff --git a/packages/app/src/attachments/workspace-attachments-store.test.ts b/packages/app/src/attachments/workspace-attachments-store.test.ts new file mode 100644 index 000000000..07b3a4d15 --- /dev/null +++ b/packages/app/src/attachments/workspace-attachments-store.test.ts @@ -0,0 +1,89 @@ +import { describe, expect, it } from "vitest"; +import type { WorkspaceComposerAttachment } from "./types"; +import { + buildWorkspaceAttachmentScopeKey, + resetWorkspaceAttachmentsStore, + useWorkspaceAttachmentsStore, +} from "./workspace-attachments-store"; + +function reviewAttachment(body: string): WorkspaceComposerAttachment { + return { + kind: "review", + reviewDraftKey: `review:${body}`, + commentCount: 1, + attachment: { + type: "review", + mimeType: "application/paseo-review", + cwd: "/repo", + mode: "uncommitted", + baseRef: null, + comments: [ + { + filePath: "src/example.ts", + side: "new", + lineNumber: 41, + body, + context: { + hunkHeader: "@@ -40,1 +40,1 @@", + targetLine: { + oldLineNumber: null, + newLineNumber: 41, + type: "add", + content: "const value = newValue;", + }, + lines: [ + { + oldLineNumber: null, + newLineNumber: 41, + type: "add", + content: "const value = newValue;", + }, + ], + }, + }, + ], + }, + }; +} + +describe("workspace attachments store", () => { + it("scopes workspace attachments by server and workspace before cwd fallback", () => { + expect( + buildWorkspaceAttachmentScopeKey({ + serverId: " local ", + workspaceId: " workspace-1 ", + cwd: "/repo", + }), + ).toBe("workspace-attachments:server=local:workspace=workspace-1"); + + expect( + buildWorkspaceAttachmentScopeKey({ + serverId: "local", + workspaceId: null, + cwd: "/repo/", + }), + ).toBe("workspace-attachments:server=local:cwd=%2Frepo"); + }); + + it("publishes and clears attachments for a workspace scope", () => { + resetWorkspaceAttachmentsStore(); + const scopeKey = buildWorkspaceAttachmentScopeKey({ + serverId: "local", + workspaceId: "workspace-1", + cwd: "/repo", + }); + const attachment = reviewAttachment("Please simplify this."); + + useWorkspaceAttachmentsStore + .getState() + .setWorkspaceAttachments({ scopeKey, attachments: [attachment] }); + + expect(useWorkspaceAttachmentsStore.getState().attachmentsByScope[scopeKey]).toEqual([ + attachment, + ]); + + useWorkspaceAttachmentsStore.getState().clearWorkspaceAttachments({ scopeKey }); + + expect(useWorkspaceAttachmentsStore.getState().attachmentsByScope[scopeKey]).toBeUndefined(); + }); +}); diff --git a/packages/app/src/attachments/workspace-attachments-store.ts b/packages/app/src/attachments/workspace-attachments-store.ts new file mode 100644 index 000000000..faa8dbca7 --- /dev/null +++ b/packages/app/src/attachments/workspace-attachments-store.ts @@ -0,0 +1,115 @@ +import { useMemo } from "react"; +import { create } from "zustand"; +import type { WorkspaceComposerAttachment } from "./types"; + +const EMPTY_WORKSPACE_ATTACHMENTS: readonly WorkspaceComposerAttachment[] = []; + +export interface WorkspaceAttachmentScopeInput { + serverId: string; + workspaceId?: string | null; + cwd: string; +} + +interface WorkspaceAttachmentsStoreState { + attachmentsByScope: Record; +} + +interface WorkspaceAttachmentsStoreActions { + setWorkspaceAttachments: (input: { + scopeKey: string; + attachments: readonly WorkspaceComposerAttachment[]; + }) => void; + clearWorkspaceAttachments: (input: { scopeKey: string }) => void; +} + +type WorkspaceAttachmentsStore = WorkspaceAttachmentsStoreState & WorkspaceAttachmentsStoreActions; + +function encodeScopePart(value: string): string { + return encodeURIComponent(value.trim()); +} + +function normalizeCwd(cwd: string): string { + const trimmed = cwd.trim(); + if (trimmed === "/") { + return trimmed; + } + return trimmed.replace(/\/+$/, ""); +} + +export function buildWorkspaceAttachmentScopeKey(input: WorkspaceAttachmentScopeInput): string { + const workspaceId = input.workspaceId?.trim(); + const workspacePart = workspaceId + ? `workspace=${encodeScopePart(workspaceId)}` + : `cwd=${encodeScopePart(normalizeCwd(input.cwd))}`; + + return ["workspace-attachments", `server=${encodeScopePart(input.serverId)}`, workspacePart].join( + ":", + ); +} + +function areWorkspaceAttachmentsEqual( + left: readonly WorkspaceComposerAttachment[], + right: readonly WorkspaceComposerAttachment[], +): boolean { + if (left === right) { + return true; + } + if (left.length !== right.length) { + return false; + } + return left.every((attachment, index) => attachment === right[index]); +} + +export const useWorkspaceAttachmentsStore = create()((set) => ({ + attachmentsByScope: {}, + setWorkspaceAttachments: ({ scopeKey, attachments }) => { + set((state) => { + const current = state.attachmentsByScope[scopeKey] ?? EMPTY_WORKSPACE_ATTACHMENTS; + if (areWorkspaceAttachmentsEqual(current, attachments)) { + return state; + } + if (attachments.length === 0) { + if (!state.attachmentsByScope[scopeKey]) { + return state; + } + const next = { ...state.attachmentsByScope }; + delete next[scopeKey]; + return { attachmentsByScope: next }; + } + return { + attachmentsByScope: { + ...state.attachmentsByScope, + [scopeKey]: attachments, + }, + }; + }); + }, + clearWorkspaceAttachments: ({ scopeKey }) => { + set((state) => { + if (!state.attachmentsByScope[scopeKey]) { + return state; + } + const next = { ...state.attachmentsByScope }; + delete next[scopeKey]; + return { attachmentsByScope: next }; + }); + }, +})); + +export function useWorkspaceAttachmentScopeKey(input: WorkspaceAttachmentScopeInput): string { + const { serverId, workspaceId, cwd } = input; + return useMemo( + () => buildWorkspaceAttachmentScopeKey({ serverId, workspaceId, cwd }), + [serverId, workspaceId, cwd], + ); +} + +export function useWorkspaceAttachments(scopeKey: string): readonly WorkspaceComposerAttachment[] { + return useWorkspaceAttachmentsStore( + (state) => state.attachmentsByScope[scopeKey] ?? EMPTY_WORKSPACE_ATTACHMENTS, + ); +} + +export function resetWorkspaceAttachmentsStore(): void { + useWorkspaceAttachmentsStore.setState({ attachmentsByScope: {} }); +} diff --git a/packages/app/src/components/agent-stream-view.tsx b/packages/app/src/components/agent-stream-view.tsx index 5c0fcafbb..754c3fa61 100644 --- a/packages/app/src/components/agent-stream-view.tsx +++ b/packages/app/src/components/agent-stream-view.tsx @@ -382,6 +382,7 @@ const AgentStreamViewComponent = forwardRef vi.fn()); @@ -27,6 +32,7 @@ const { setAgentStreamHeadMock, setQueuedMessagesMock, agentDirectoryStatusMock, + appSendBehavior, } = vi.hoisted(() => { const hoistedTheme = { spacing: { 1: 4, 2: 8, 3: 12, 4: 16, 6: 24, 8: 32 }, @@ -35,6 +41,7 @@ const { borderRadius: { full: 999, md: 6, lg: 8, "2xl": 16 }, fontSize: { xs: 11, sm: 13, base: 15, lg: 18 }, fontWeight: { normal: "400", medium: "500" }, + lineHeight: { diff: 18 }, shadow: { md: {} }, colors: { surface0: "#000", @@ -99,7 +106,12 @@ const { cancelAgent: vi.fn(async () => {}), }; - const hoistedSetQueuedMessagesMock = vi.fn(); + const hoistedSetQueuedMessagesMock = vi.fn( + (serverId: string, updater: (prev: Map) => Map) => { + const session = hoistedMockSessionState.sessions[serverId]; + session.queuedMessages = updater(session.queuedMessages); + }, + ); const hoistedMockSessionState: { sessions: Record< string, @@ -161,6 +173,7 @@ const { hoistedMockSessionState.setAgentStreamTail = hoistedSetAgentStreamTailMock; hoistedMockSessionState.setAgentStreamHead = hoistedSetAgentStreamHeadMock; const hoistedAgentDirectoryStatusMock = vi.fn(() => "ready"); + const hoistedAppSendBehavior = { current: "interrupt" as "interrupt" | "queue" }; return { theme: hoistedTheme, @@ -178,6 +191,7 @@ const { setAgentStreamHeadMock: hoistedSetAgentStreamHeadMock, setQueuedMessagesMock: hoistedSetQueuedMessagesMock, agentDirectoryStatusMock: hoistedAgentDirectoryStatusMock, + appSendBehavior: hoistedAppSendBehavior, }; }); @@ -297,7 +311,7 @@ vi.mock("@/utils/open-external-url", () => ({ })); vi.mock("@/hooks/use-settings", () => ({ - useAppSettings: () => ({ settings: { sendBehavior: "interrupt" } }), + useAppSettings: () => ({ settings: { sendBehavior: appSendBehavior.current } }), })); vi.mock("@/hooks/use-agent-autocomplete", () => ({ @@ -556,8 +570,72 @@ let container: HTMLElement | null = null; let queryClient: QueryClient | null = null; let latestAttachments: ComposerAttachment[] = []; +type ReviewComposerAttachment = Extract; +type ReviewAttachment = Extract; + +function reviewAttachment(body: string): ReviewAttachment { + return { + type: "review", + mimeType: "application/paseo-review", + cwd: "/repo", + mode: "uncommitted", + baseRef: null, + comments: [ + { + filePath: "src/example.ts", + side: "new", + lineNumber: 41, + body, + context: { + hunkHeader: "@@ -40,2 +40,2 @@", + targetLine: { + oldLineNumber: null, + newLineNumber: 41, + type: "add", + content: "const value = newValue;", + }, + lines: [ + { + oldLineNumber: null, + newLineNumber: 41, + type: "add", + content: "const value = newValue;", + }, + ], + }, + }, + ], + }; +} + +function reviewComposerAttachment(body: string): ReviewComposerAttachment { + return { + kind: "review", + reviewDraftKey: `review:${body}`, + commentCount: 1, + attachment: reviewAttachment(body), + }; +} + +function seedReviewDraft(key: string) { + addReviewDraftComment({ + key, + comment: { + id: `${key}:comment`, + filePath: "src/example.ts", + side: "new", + lineNumber: 41, + body: "Please simplify this.", + createdAt: "2026-04-21T00:00:00.000Z", + updatedAt: "2026-04-21T00:00:00.000Z", + }, + }); +} + beforeEach(() => { - const dom = new JSDOM(""); + const dom = new JSDOM("", { + url: "http://localhost", + }); vi.stubGlobal("React", React); vi.stubGlobal("IS_REACT_ACT_ENVIRONMENT", true); vi.stubGlobal("window", dom.window); @@ -570,6 +648,7 @@ beforeEach(() => { attachEvent: vi.fn(), detachEvent: vi.fn(), }); + vi.stubGlobal("localStorage", dom.window.localStorage); container = document.createElement("div"); document.body.appendChild(container); @@ -590,6 +669,7 @@ beforeEach(() => { keyboardActionHandlerMock.mockClear(); agentDirectoryStatusMock.mockReset(); agentDirectoryStatusMock.mockReturnValue("ready"); + appSendBehavior.current = "interrupt"; mockSessionState.sessions.server.serverInfo = { serverId: "server", hostname: "test", @@ -607,6 +687,10 @@ beforeEach(() => { mockSessionState.sessions.server.agentStreamHead = new Map(); mockSessionState.sessions.server.agentStreamTail = new Map(); mockSessionState.sessions.server.queuedMessages = new Map(); + mockSessionState.sessions.server.agents = new Map([ + ["agent", { status: "idle", lastUsage: null }], + ]); + resetReviewDraftStore(); }); afterEach(() => { @@ -634,20 +718,30 @@ function imageAttachment(id: string): AttachmentMetadata { function ComposerHarness({ initialText = "", initialAttachments = [], + workspaceAttachment = null, isSubmitLoading = false, submitBehavior, }: { initialText?: string; - initialAttachments?: ComposerAttachment[]; + initialAttachments?: UserComposerAttachment[]; + workspaceAttachment?: ReviewComposerAttachment | null; isSubmitLoading?: boolean; submitBehavior?: "clear" | "preserve-and-lock"; }) { const [text, setText] = useState(initialText); const [attachments, setAttachments] = useState(initialAttachments); + const workspaceAttachments = React.useMemo( + () => (workspaceAttachment ? [workspaceAttachment] : []), + [workspaceAttachment], + ); latestAttachments = attachments; const handleChangeAttachments = React.useCallback( - (updater: ComposerAttachment[] | ((current: ComposerAttachment[]) => ComposerAttachment[])) => { + ( + updater: + | UserComposerAttachment[] + | ((current: UserComposerAttachment[]) => UserComposerAttachment[]), + ) => { setAttachments((current) => { const next = typeof updater === "function" ? updater(current) : updater; latestAttachments = next; @@ -666,6 +760,7 @@ function ComposerHarness({ value={text} onChangeText={setText} attachments={attachments} + workspaceAttachments={workspaceAttachments} onChangeAttachments={handleChangeAttachments} isSubmitLoading={isSubmitLoading} submitBehavior={submitBehavior} @@ -679,7 +774,8 @@ function ComposerHarness({ function renderComposer( input: { initialText?: string; - initialAttachments?: ComposerAttachment[]; + initialAttachments?: UserComposerAttachment[]; + workspaceAttachment?: ReviewComposerAttachment | null; isSubmitLoading?: boolean; submitBehavior?: "clear" | "preserve-and-lock"; } = {}, @@ -903,6 +999,222 @@ describe("Composer attachments", () => { ); }); + it("serializes workspace review attachments through the structured attachment path", async () => { + const review = reviewComposerAttachment("Please simplify this."); + expect(splitComposerAttachmentsForSubmit([review])).toEqual({ + images: [], + attachments: [review.attachment], + }); + }); + + it("renders and submits a workspace review attachment pill", async () => { + const review = reviewComposerAttachment("Please simplify this."); + renderComposer({ + initialText: "review this", + workspaceAttachment: review, + }); + + expect(queryByTestId("composer-review-attachment-pill")?.textContent).toContain("Review"); + expect(queryByTestId("composer-review-attachment-pill")?.textContent).toContain("1 comment"); + + click(document.querySelector('[aria-label="Send message"]')!); + await flushAsyncWork(); + + expect(mockClient.sendAgentMessage).toHaveBeenCalledWith( + "agent", + "review this", + expect.objectContaining({ + attachments: [review.attachment], + }), + ); + }); + + it("clears the included workspace review draft after a successful submit", async () => { + const review = reviewComposerAttachment("Clear submitted review draft."); + seedReviewDraft(review.reviewDraftKey); + renderComposer({ + initialText: "review this", + workspaceAttachment: review, + }); + + click(document.querySelector('[aria-label="Send message"]')!); + await flushAsyncWork(); + + expect(getReviewDraftComments(review.reviewDraftKey)).toBeUndefined(); + }); + + it("restores only normal attachments when a submit with a workspace review fails", async () => { + const image = imageAttachment("img-failure"); + const review = reviewComposerAttachment("This should be sent but not persisted."); + seedReviewDraft(review.reviewDraftKey); + mockClient.sendAgentMessage.mockRejectedValueOnce(new Error("network down")); + renderComposer({ + initialText: "review this", + initialAttachments: [{ kind: "image", metadata: image }], + workspaceAttachment: review, + }); + + click(document.querySelector('[aria-label="Send message"]')!); + await flushAsyncWork(); + + expect(mockClient.sendAgentMessage).toHaveBeenCalledWith( + "agent", + "review this", + expect.objectContaining({ + attachments: [review.attachment], + }), + ); + expect(latestAttachments).toEqual([{ kind: "image", metadata: image }]); + expect(getReviewDraftComments(review.reviewDraftKey)).toHaveLength(1); + }); + + it("clears workspace review suppression after a send lifecycle", async () => { + const review = reviewComposerAttachment("Keep this available for the next message."); + renderComposer({ + initialText: "send without review", + workspaceAttachment: review, + }); + + click(document.querySelector('[aria-label="Remove review attachment"]')!); + expect(queryByTestId("composer-review-attachment-pill")).toBeNull(); + + click(document.querySelector('[aria-label="Send message"]')!); + await flushAsyncWork(); + + expect(mockClient.sendAgentMessage).toHaveBeenCalledWith( + "agent", + "send without review", + expect.objectContaining({ + attachments: [], + }), + ); + expect(queryByTestId("composer-review-attachment-pill")).not.toBeNull(); + }); + + it("keeps workspace review suppressed after a failed send", async () => { + const review = reviewComposerAttachment("Do not send this on retry."); + mockClient.sendAgentMessage.mockRejectedValueOnce(new Error("network down")); + renderComposer({ + initialText: "retry without review", + workspaceAttachment: review, + }); + + click(document.querySelector('[aria-label="Remove review attachment"]')!); + expect(queryByTestId("composer-review-attachment-pill")).toBeNull(); + + click(document.querySelector('[aria-label="Send message"]')!); + await flushAsyncWork(); + + expect(mockClient.sendAgentMessage).toHaveBeenNthCalledWith( + 1, + "agent", + "retry without review", + expect.objectContaining({ + attachments: [], + }), + ); + expect(queryByTestId("composer-review-attachment-pill")).toBeNull(); + + click(document.querySelector('[aria-label="Send message"]')!); + await flushAsyncWork(); + + expect(mockClient.sendAgentMessage).toHaveBeenNthCalledWith( + 2, + "agent", + "retry without review", + expect.objectContaining({ + attachments: [], + }), + ); + }); + + it("captures workspace review attachments at queue time", async () => { + appSendBehavior.current = "queue"; + mockSessionState.sessions.server.agents.set("agent", { status: "running", lastUsage: null }); + const initialReview = reviewComposerAttachment("Initial queued review."); + const editedReview = reviewComposerAttachment("Edited after queue."); + renderComposer({ + initialText: "queue this", + workspaceAttachment: initialReview, + }); + + click(document.querySelector('[aria-label="Queue message"]')!); + await flushAsyncWork(); + renderComposer({ + initialText: "", + workspaceAttachment: editedReview, + }); + + const queued = mockSessionState.sessions.server.queuedMessages.get("agent") as Array<{ + attachments: ComposerAttachment[]; + }>; + expect(queued[0]?.attachments).toEqual([initialReview]); + }); + + it("clears the included workspace review draft after queueing", async () => { + appSendBehavior.current = "queue"; + mockSessionState.sessions.server.agents.set("agent", { status: "running", lastUsage: null }); + const review = reviewComposerAttachment("Clear queued review draft."); + seedReviewDraft(review.reviewDraftKey); + renderComposer({ + initialText: "queue this", + workspaceAttachment: review, + }); + + click(document.querySelector('[aria-label="Queue message"]')!); + await flushAsyncWork(); + + expect(getReviewDraftComments(review.reviewDraftKey)).toBeUndefined(); + }); + + it("clears workspace review suppression after queueing a message", async () => { + appSendBehavior.current = "queue"; + mockSessionState.sessions.server.agents.set("agent", { status: "running", lastUsage: null }); + const review = reviewComposerAttachment("Queue without this review first."); + renderComposer({ + initialText: "queue without review", + workspaceAttachment: review, + }); + + click(document.querySelector('[aria-label="Remove review attachment"]')!); + expect(queryByTestId("composer-review-attachment-pill")).toBeNull(); + + click(document.querySelector('[aria-label="Queue message"]')!); + await flushAsyncWork(); + + const queued = mockSessionState.sessions.server.queuedMessages.get("agent") as Array<{ + attachments: ComposerAttachment[]; + }>; + expect(queued[0]?.attachments).toEqual([]); + expect(queryByTestId("composer-review-attachment-pill")).not.toBeNull(); + }); + + it("does not restore queued workspace review attachments into live draft attachments when editing", async () => { + appSendBehavior.current = "queue"; + mockSessionState.sessions.server.agents.set("agent", { status: "running", lastUsage: null }); + const image = imageAttachment("img-queued-edit"); + const review = reviewComposerAttachment("Queued snapshot."); + renderComposer({ + initialText: "queue this", + initialAttachments: [{ kind: "image", metadata: image }], + workspaceAttachment: review, + }); + + click(document.querySelector('[aria-label="Queue message"]')!); + await flushAsyncWork(); + expect( + ( + mockSessionState.sessions.server.queuedMessages.get("agent") as Array<{ + attachments: ComposerAttachment[]; + }> + )[0]?.attachments, + ).toEqual([{ kind: "image", metadata: image }, review]); + + click(document.querySelector('[aria-label="Edit queued message"]')!); + + expect(latestAttachments).toEqual([{ kind: "image", metadata: image }]); + }); + it("submits empty wire arrays when there are no composer attachments", async () => { renderComposer({ initialText: "plain message" }); diff --git a/packages/app/src/components/composer.tsx b/packages/app/src/components/composer.tsx index cd26fde4d..a97c56e78 100644 --- a/packages/app/src/components/composer.tsx +++ b/packages/app/src/components/composer.tsx @@ -70,7 +70,13 @@ import { submitAgentInput } from "@/components/agent-input-submit"; import { useAppSettings } from "@/hooks/use-settings"; import { isWeb, isNative } from "@/constants/platform"; import type { GitHubSearchItem } from "@server/shared/messages"; -import type { AttachmentMetadata, ComposerAttachment } from "@/attachments/types"; +import type { + AttachmentMetadata, + ComposerAttachment, + UserComposerAttachment, + WorkspaceComposerAttachment, +} from "@/attachments/types"; +import { composerWorkspaceAttachment } from "@/attachments/composer-workspace-attachments"; import { useAttachmentPreviewUrl } from "@/attachments/use-attachment-preview-url"; import { Combobox, ComboboxItem, type ComboboxOption } from "@/components/ui/combobox"; import { splitComposerAttachmentsForSubmit } from "@/components/composer-attachments"; @@ -86,8 +92,8 @@ interface QueuedMessage { } type AttachmentListUpdater = - | ComposerAttachment[] - | ((prev: ComposerAttachment[]) => ComposerAttachment[]); + | UserComposerAttachment[] + | ((prev: UserComposerAttachment[]) => UserComposerAttachment[]); function noop() {} @@ -145,7 +151,7 @@ async function pickAndPersistImages( ); } -function removeAttachmentAtIndex(prev: ComposerAttachment[], index: number): ComposerAttachment[] { +function removeAttachmentAtIndex(prev: T[], index: number): T[] { const removed = prev[index]; if (removed?.kind === "image") { void deleteAttachments([removed.metadata]); @@ -242,26 +248,28 @@ function findGithubItemByOption( } function isAttachmentSelectedForGithubItem( - attachments: readonly ComposerAttachment[], + current: readonly ComposerAttachment[], item: GitHubSearchItem, ): boolean { - return attachments.some( - (attachment) => - attachment.kind !== "image" && - attachment.item.kind === item.kind && - attachment.item.number === item.number, - ); + return composerWorkspaceAttachment + .userAttachmentsOnly(current) + .some( + (attachment) => + attachment.kind !== "image" && + attachment.item.kind === item.kind && + attachment.item.number === item.number, + ); } -function buildGithubAttachment(item: GitHubSearchItem): ComposerAttachment { +function buildGithubAttachment(item: GitHubSearchItem): UserComposerAttachment { return item.kind === "pr" ? { kind: "github_pr", item } : { kind: "github_issue", item }; } function toggleGithubAttachment( - current: ComposerAttachment[], + current: UserComposerAttachment[], item: GitHubSearchItem, -): ComposerAttachment[] { - const matches = (attachment: ComposerAttachment) => +): UserComposerAttachment[] { + const matches = (attachment: UserComposerAttachment) => attachment.kind !== "image" && attachment.item.kind === item.kind && attachment.item.number === item.number; @@ -361,6 +369,15 @@ function renderComposerAttachmentPill(args: RenderComposerAttachmentPillArgs): R /> ); } + if (composerWorkspaceAttachment.is(attachment)) { + return composerWorkspaceAttachment.renderPill({ + attachment, + index, + disabled, + onOpen, + onRemove, + }); + } return ( 0 ? { images: wirePayload.images } : {}), + ...(wirePayload.attachments.length > 0 ? { attachments: wirePayload.attachments } : {}), }; appendUserMessageToStream({ ...args, userMessage }); const imagesData = await encodeImages(wirePayload.images); @@ -459,11 +477,16 @@ async function dispatchAgentMessageSend(args: DispatchAgentMessageSendArgs): Pro function openComposerAttachment( attachment: ComposerAttachment, setLightboxMetadata: (metadata: AttachmentMetadata) => void, + openWorkspaceAttachment: (input: { attachment: ComposerAttachment }) => boolean, ): void { if (attachment.kind === "image") { setLightboxMetadata(attachment.metadata); return; } + if (composerWorkspaceAttachment.is(attachment)) { + openWorkspaceAttachment({ attachment }); + return; + } void openExternalUrl(attachment.item.url); } @@ -596,10 +619,18 @@ function QueuedMessageRow({ item, onEdit, onSendNow }: QueuedMessageRowProps) { {item.text} - + - + @@ -652,7 +683,7 @@ function ImageAttachmentPill({ } interface GithubAttachmentPillProps { - attachment: Exclude; + attachment: Extract; index: number; disabled: boolean; onOpen: (attachment: ComposerAttachment) => void; @@ -759,7 +790,9 @@ interface ComposerProps { blurOnSubmit?: boolean; value: string; onChangeText: (text: string) => void; - attachments: ComposerAttachment[]; + attachments: UserComposerAttachment[]; + workspaceAttachments?: readonly WorkspaceComposerAttachment[]; + onOpenWorkspaceAttachment?: (attachment: WorkspaceComposerAttachment) => void; onChangeAttachments: (updater: AttachmentListUpdater) => void; cwd: string; clearDraft: (lifecycle: "sent" | "abandoned") => void; @@ -957,6 +990,8 @@ export function Composer({ value, onChangeText, attachments, + workspaceAttachments = [], + onOpenWorkspaceAttachment, onChangeAttachments, cwd, clearDraft, @@ -1005,7 +1040,19 @@ export function Composer({ const messagePlaceholder = resolveMessagePlaceholder(isDesktopWebBreakpoint); const userInput = value; const setUserInput = onChangeText; - const selectedAttachments = attachments; + const { + selectedAttachments, + buildOutgoingAttachments, + removeAttachment, + openAttachment, + clearSentAttachments, + completeSubmit, + resetSuppression, + } = composerWorkspaceAttachment.useBinding({ + normalAttachments: attachments, + workspaceAttachments, + onOpenWorkspaceAttachment, + }); const setSelectedAttachments = onChangeAttachments; const [cursorIndex, setCursorIndex] = useState(0); const [isProcessing, setIsProcessing] = useState(false); @@ -1162,8 +1209,18 @@ export function Composer({ setUserInput(""); setSelectedAttachments([]); + resetSuppression(); + clearSentAttachments(queuedAttachments); }, - [agentId, serverId, setQueuedMessages, setSelectedAttachments, setUserInput], + [ + agentId, + clearSentAttachments, + resetSuppression, + serverId, + setQueuedMessages, + setSelectedAttachments, + setUserInput, + ], ); const sendMessageWithContent = useCallback( @@ -1172,7 +1229,7 @@ export function Composer({ outgoingAttachments: ComposerAttachment[], forceSend?: boolean, ) => { - await submitAgentInput({ + const result = await submitAgentInput({ message: outgoingMessage, attachments: outgoingAttachments, hasExternalContent, @@ -1192,7 +1249,7 @@ export function Composer({ clearDraft, setUserInput, setAttachments: (nextAttachments) => { - setSelectedAttachments(nextAttachments); + setSelectedAttachments(composerWorkspaceAttachment.userAttachmentsOnly(nextAttachments)); }, setSendError, setIsProcessing, @@ -1200,10 +1257,15 @@ export function Composer({ console.error("[AgentInput] Failed to send message:", error); }, }); + completeSubmit({ + result, + outgoingAttachments, + }); }, [ allowEmptySubmit, clearDraft, + completeSubmit, hasExternalContent, isAgentRunning, queueMessage, @@ -1219,9 +1281,13 @@ export function Composer({ if (blurOnSubmit) { messageInputRef.current?.blur(); } - void sendMessageWithContent(payload.text, payload.attachments, payload.forceSend); + void sendMessageWithContent( + payload.text, + buildOutgoingAttachments(attachments), + payload.forceSend, + ); }, - [blurOnSubmit, sendMessageWithContent], + [attachments, blurOnSubmit, buildOutgoingAttachments, sendMessageWithContent], ); const handlePickImage = useCallback(async () => { @@ -1232,14 +1298,24 @@ export function Composer({ const handleRemoveAttachment = useCallback( (index: number) => { + const didRemoveWorkspaceAttachment = removeAttachment({ + selectedAttachments, + index, + }); + if (didRemoveWorkspaceAttachment) { + return; + } setSelectedAttachments((prev) => removeAttachmentAtIndex(prev, index)); }, - [setSelectedAttachments], + [removeAttachment, selectedAttachments, setSelectedAttachments], ); - const handleOpenAttachment = useCallback((attachment: ComposerAttachment) => { - openComposerAttachment(attachment, setLightboxMetadata); - }, []); + const handleOpenAttachment = useCallback( + (attachment: ComposerAttachment) => { + openComposerAttachment(attachment, setLightboxMetadata, openAttachment); + }, + [openAttachment], + ); useEffect(() => { if (!isAgentRunning || !isConnected) { @@ -1327,7 +1403,7 @@ export function Composer({ updateQueue((current) => current.filter((q) => q.id !== id)); setUserInput(item.text); - setSelectedAttachments(item.attachments); + setSelectedAttachments(composerWorkspaceAttachment.userAttachmentsOnly(item.attachments)); }, [queuedMessages, setSelectedAttachments, setUserInput, updateQueue], ); @@ -1353,9 +1429,9 @@ export function Composer({ const handleQueue = useCallback( (payload: MessagePayload) => { - queueMessage(payload.text, payload.attachments); + queueMessage(payload.text, buildOutgoingAttachments(attachments)); }, - [queueMessage], + [attachments, buildOutgoingAttachments, queueMessage], ); const hasSendableContent = userInput.trim().length > 0 || selectedAttachments.length > 0; @@ -1408,7 +1484,6 @@ export function Composer({ ], ); - const isVoiceSwitchingValue = voice?.isVoiceSwitching ?? false; const rightContent = useMemo( () => ( ; }) { const containerStyle = useMemo( - () => [styles.gutterCell, lineTypeBackground(type), { width: gutterWidth }], - [type, gutterWidth], + () => [styles.gutterCell, lineTypeBackground(type), { width: gutterWidth }, style], + [type, gutterWidth, style], ); const textStyle = useMemo( () => [ @@ -234,12 +265,29 @@ function DiffGutterCell({ ], [type], ); + const comments = useMemo( + () => + reviewTarget + ? (reviewActions?.commentsByTarget.get(reviewTarget.key) ?? EMPTY_COMMENTS) + : EMPTY_COMMENTS, + [reviewTarget, reviewActions?.commentsByTarget], + ); + const isEditorOpen = isInlineReviewEditorForTarget(reviewActions?.editor ?? null, reviewTarget); + const onStartComment = reviewActions?.onStartComment ?? noopStartComment; + return ( - + {formatDiffGutterText(lineNumber)} - + ); } @@ -314,11 +362,15 @@ function DiffLineView({ lineNumber, gutterWidth, wrapLines, + reviewTarget, + reviewActions, }: { line: DiffLine; lineNumber: number | null; gutterWidth: number; wrapLines: boolean; + reviewTarget?: ReviewableDiffTarget | null; + reviewActions?: InlineReviewActions; }) { const visibleTokens = hasVisibleDiffTokens(line.tokens) ? line.tokens : null; @@ -326,18 +378,6 @@ function DiffLineView({ () => [styles.diffLineContainer, lineTypeBackground(line.type)], [line.type], ); - const gutterStyle = useMemo( - () => [styles.lineNumberGutter, { width: gutterWidth }], - [gutterWidth], - ); - const gutterTextStyle = useMemo( - () => [ - styles.lineNumberText, - line.type === "add" && styles.addLineNumberText, - line.type === "remove" && styles.removeLineNumberText, - ], - [line.type], - ); const textStyle = useMemo( () => [ styles.diffLineText, @@ -352,11 +392,14 @@ function DiffLineView({ return ( - - - {formatDiffGutterText(lineNumber)} - - + {line.type !== "header" && visibleTokens ? ( ) : ( @@ -370,10 +413,12 @@ function SplitDiffLine({ line, gutterWidth, wrapLines, + reviewActions, }: { line: SplitDiffDisplayLine | null; gutterWidth: number; wrapLines: boolean; + reviewActions?: InlineReviewActions; }) { const visibleTokens = line && hasVisibleDiffTokens(line.tokens) ? line.tokens : null; @@ -381,18 +426,6 @@ function SplitDiffLine({ () => [styles.diffLineContainer, lineTypeBackground(line?.type)], [line?.type], ); - const gutterStyle = useMemo( - () => [styles.lineNumberGutter, { width: gutterWidth }], - [gutterWidth], - ); - const gutterTextStyle = useMemo( - () => [ - styles.lineNumberText, - line?.type === "add" && styles.addLineNumberText, - line?.type === "remove" && styles.removeLineNumberText, - ], - [line?.type], - ); const textStyle = useMemo( () => [ styles.diffLineText, @@ -407,11 +440,14 @@ function SplitDiffLine({ return ( - - - {formatDiffGutterText(line?.lineNumber ?? null)} - - + {visibleTokens ? ( ) : ( @@ -421,17 +457,119 @@ function SplitDiffLine({ ); } +function InlineReviewThreadContent({ + reviewTarget, + reviewActions, + reservedHeight, + viewportWidth, + pinToViewport, +}: { + reviewTarget: ReviewableDiffTarget | null | undefined; + reviewActions?: InlineReviewActions; + reservedHeight?: number; + viewportWidth?: number; + pinToViewport?: boolean; +}) { + const threadState = getInlineReviewThreadState({ reviewTarget, reviewActions }); + const height = reservedHeight ?? threadState?.height ?? 0; + const placeholderStyle = useMemo(() => ({ minHeight: height }), [height]); + if (height === 0) { + return null; + } + if (!reviewTarget || !reviewActions || !threadState) { + return ; + } + + return ( + + ); +} + +function InlineReviewGutterSpacer({ + reviewTarget, + reviewActions, + gutterWidth, + reservedHeight, + style, +}: { + reviewTarget: ReviewableDiffTarget | null | undefined; + reviewActions?: InlineReviewActions; + gutterWidth: number; + reservedHeight?: number; + style?: StyleProp; +}) { + const threadState = getInlineReviewThreadState({ reviewTarget, reviewActions }); + const height = reservedHeight ?? threadState?.height ?? 0; + const spacerStyle = useMemo>( + () => [styles.inlineReviewGutterSpacer, { width: gutterWidth, minHeight: height }, style], + [gutterWidth, height, style], + ); + if (height === 0) { + return null; + } + + return ; +} + +function InlineReviewRow({ + reviewTarget, + reviewActions, + gutterWidth, + reservedHeight, +}: { + reviewTarget: ReviewableDiffTarget | null | undefined; + reviewActions?: InlineReviewActions; + gutterWidth: number; + reservedHeight?: number; +}) { + const threadState = getInlineReviewThreadState({ reviewTarget, reviewActions }); + const height = reservedHeight ?? threadState?.height ?? 0; + const gutterSpacerStyle = useMemo>( + () => [styles.inlineReviewGutterSpacer, { width: gutterWidth }], + [gutterWidth], + ); + const placeholderStyle = useMemo(() => ({ minHeight: height }), [height]); + if (height === 0) { + return null; + } + + return ( + + + {reviewTarget && reviewActions && threadState ? ( + + ) : ( + + )} + + ); +} + function SplitDiffColumn({ rows, side, gutterWidth, wrapLines, + reviewActions, showDivider = false, }: { rows: SplitDiffRow[]; side: "left" | "right"; gutterWidth: number; wrapLines: boolean; + reviewActions?: InlineReviewActions; showDivider?: boolean; }) { const [scrollWidth, setScrollWidth] = useState(0); @@ -463,13 +601,27 @@ function SplitDiffColumn({ ); } + const line = side === "left" ? row.left : row.right; + const reviewRowState = getSplitInlineReviewThreadState({ + left: row.left?.reviewTarget, + right: row.right?.reviewTarget, + reviewActions, + }); return ( - + + + + ); })} @@ -487,13 +639,27 @@ function SplitDiffColumn({ ); } const line = side === "left" ? row.left : row.right; + const reviewRowState = getSplitInlineReviewThreadState({ + left: row.left?.reviewTarget, + right: row.right?.reviewTarget, + reviewActions, + }); return ( - + + + + ); })} @@ -512,12 +678,23 @@ function SplitDiffColumn({ ); } + const line = side === "left" ? row.left : row.right; + const reviewRowState = getSplitInlineReviewThreadState({ + left: row.left?.reviewTarget, + right: row.right?.reviewTarget, + reviewActions, + }); return ( - + + + + ); })} @@ -620,12 +797,14 @@ function DiffFileBody({ file, layout, wrapLines, + reviewActions, onBodyHeightChange, testID, }: { file: ParsedDiffFile; layout: "unified" | "split"; wrapLines: boolean; + reviewActions?: InlineReviewActions; onBodyHeightChange?: (file: ParsedDiffFile, height: number) => void; testID?: string; }) { @@ -679,12 +858,14 @@ function DiffFileBody({ side="left" gutterWidth={gutterWidth} wrapLines={wrapLines} + reviewActions={reviewActions} /> @@ -697,30 +878,48 @@ function DiffFileBody({ return ( - {computedLines.map(({ line, lineNumber, key }) => ( - + {computedLines.map(({ line, lineNumber, key, reviewTarget }) => ( + + + + ))} ); } + const textViewportWidth = + scrollViewWidth > 0 ? scrollViewWidth : Math.max(0, bodyWidth - gutterWidth); return ( - {computedLines.map(({ line, lineNumber, key }) => ( - + {computedLines.map(({ line, lineNumber, key, reviewTarget }) => ( + + + + ))} - {computedLines.map(({ line, key }) => ( - + {computedLines.map(({ line, key, reviewTarget }) => ( + + + + ))} @@ -1524,7 +1731,7 @@ export function GitDiffPane({ workspaceId, cwd, hideHeaderRow, - enabled = true, + enabled, }: GitDiffPaneProps) { const { theme } = useUnistyles(); const toast = useToast(); @@ -1532,7 +1739,7 @@ export function GitDiffPane({ const showDesktopWebScrollbar = isWeb && !isMobile; const canUseSplitLayout = isWeb && !isMobile; const router = useRouter(); - const [diffModeOverride, setDiffModeOverride] = useState<"uncommitted" | "base" | null>(null); + const [diffModeOverride, setDiffModeOverride] = useState(null); const [postShipArchiveSuggested, setPostShipArchiveSuggested] = useState(false); const [shipDefault, setShipDefault] = useState<"merge" | "pr">("merge"); const { preferences: changesPreferences, updatePreferences: updateChangesPreferences } = @@ -1555,13 +1762,8 @@ export function GitDiffPane({ void updateChangesPreferences({ hideWhitespace: !changesPreferences.hideWhitespace }); }, [changesPreferences.hideWhitespace, updateChangesPreferences]); - const handleSelectUncommitted = useCallback(() => { - setDiffModeOverride("uncommitted"); - }, []); - - const handleSelectBase = useCallback(() => { - setDiffModeOverride("base"); - }, []); + // handleSelectUncommitted/handleSelectBase are defined later, after reviewDraftScopeKey + // and setActiveReviewMode are available, so they can record the active review mode. const handleLayoutUnified = useCallback(() => { handleLayoutChange("unified"); @@ -1617,8 +1819,20 @@ export function GitDiffPane({ } = statusState; // Auto-select diff mode based on state: uncommitted when dirty, base when clean - const autoDiffMode = hasUncommittedChanges ? "uncommitted" : "base"; - const diffMode = diffModeOverride ?? autoDiffMode; + const autoDiffMode: ReviewDraftMode = hasUncommittedChanges ? "uncommitted" : "base"; + const reviewDraftScopeKey = useMemo( + () => + buildReviewDraftScopeKey({ + serverId, + workspaceId, + cwd, + baseRef, + ignoreWhitespace: changesPreferences.hideWhitespace, + }), + [baseRef, changesPreferences.hideWhitespace, cwd, serverId, workspaceId], + ); + const activeReviewMode = useActiveReviewDraftMode({ scopeKey: reviewDraftScopeKey }); + const diffMode = diffModeOverride ?? activeReviewMode ?? autoDiffMode; const { files, @@ -1630,8 +1844,69 @@ export function GitDiffPane({ mode: diffMode, baseRef, ignoreWhitespace: changesPreferences.hideWhitespace, - enabled: shouldEnableCheckoutDiff({ paneEnabled: enabled, isGit }), + enabled: shouldEnableCheckoutDiff({ paneEnabled: enabled !== false, isGit }), }); + const reviewDraftKey = useMemo( + () => + buildReviewDraftKey({ + serverId, + workspaceId, + cwd, + mode: diffMode, + baseRef, + ignoreWhitespace: changesPreferences.hideWhitespace, + }), + [baseRef, changesPreferences.hideWhitespace, cwd, diffMode, serverId, workspaceId], + ); + const setActiveReviewMode = useSetActiveReviewDraftMode(); + + const handleSelectUncommitted = useCallback(() => { + setDiffModeOverride("uncommitted"); + setActiveReviewMode({ scopeKey: reviewDraftScopeKey, mode: "uncommitted" }); + }, [reviewDraftScopeKey, setActiveReviewMode]); + + const handleSelectBase = useCallback(() => { + setDiffModeOverride("base"); + setActiveReviewMode({ scopeKey: reviewDraftScopeKey, mode: "base" }); + }, [reviewDraftScopeKey, setActiveReviewMode]); + + const reviewActions = useInlineReviewController({ + reviewDraftKey, + showPersistentAction: isMobile, + }); + const reviewAttachment = useReviewAttachmentSnapshot({ + key: reviewDraftKey, + diffFiles: files, + cwd, + mode: diffMode, + baseRef, + }); + const workspaceAttachmentScopeKey = useMemo( + () => buildWorkspaceAttachmentScopeKey({ serverId, workspaceId, cwd }), + [cwd, serverId, workspaceId], + ); + const setWorkspaceAttachments = useWorkspaceAttachmentsStore( + (state) => state.setWorkspaceAttachments, + ); + const clearWorkspaceAttachments = useWorkspaceAttachmentsStore( + (state) => state.clearWorkspaceAttachments, + ); + + useEffect(() => { + setWorkspaceAttachments({ + scopeKey: workspaceAttachmentScopeKey, + attachments: reviewAttachment ? [reviewAttachment] : [], + }); + + return () => { + clearWorkspaceAttachments({ scopeKey: workspaceAttachmentScopeKey }); + }; + }, [ + clearWorkspaceAttachments, + reviewAttachment, + setWorkspaceAttachments, + workspaceAttachmentScopeKey, + ]); const { status: prStatus, githubFeaturesEnabled, @@ -2026,6 +2301,7 @@ export function GitDiffPane({ file={item.file} layout={effectiveLayout} wrapLines={wrapLines} + reviewActions={reviewActions} onBodyHeightChange={handleBodyHeightChange} testID={`diff-file-${item.fileIndex}-body`} /> @@ -2036,6 +2312,7 @@ export function GitDiffPane({ handleBodyHeightChange, handleHeaderHeightChange, handleToggleExpanded, + reviewActions, wrapLines, ], ); @@ -2075,8 +2352,14 @@ export function GitDiffPane({ ); const flatExtraData = useMemo( - () => ({ expandedPathsArray, effectiveLayout, heightVersion, wrapLines }), - [expandedPathsArray, effectiveLayout, heightVersion, wrapLines], + () => ({ + expandedPathsArray, + effectiveLayout, + heightVersion, + wrapLines, + reviewActions, + }), + [expandedPathsArray, effectiveLayout, heightVersion, wrapLines, reviewActions], ); const hasChanges = files.length > 0; @@ -2655,6 +2938,17 @@ const styles = StyleSheet.create((theme) => ({ borderRightColor: theme.colors.border, justifyContent: "flex-start", }, + inlineReviewRow: { + flexDirection: "row", + alignItems: "stretch", + backgroundColor: theme.colors.surface1, + }, + inlineReviewGutterSpacer: { + borderRightWidth: theme.borderWidth[1], + borderRightColor: theme.colors.border, + backgroundColor: theme.colors.surface1, + flexShrink: 0, + }, textLineContainer: { flexDirection: "row", alignItems: "stretch", diff --git a/packages/app/src/components/message.tsx b/packages/app/src/components/message.tsx index 110560194..b6146286f 100644 --- a/packages/app/src/components/message.tsx +++ b/packages/app/src/components/message.tsx @@ -64,6 +64,7 @@ import { createMarkdownStyles } from "@/styles/markdown-styles"; import { Fonts } from "@/constants/theme"; import * as Clipboard from "expo-clipboard"; import type { TodoEntry, UserMessageImageAttachment } from "@/types/stream"; +import type { AgentAttachment } from "@server/shared/messages"; import type { ToolCallDetail } from "@server/server/agent/agent-sdk-types"; import { buildToolCallDisplayModel } from "@/utils/tool-call-display"; import { resolveToolCallIcon } from "@/utils/tool-call-icon"; @@ -107,6 +108,7 @@ type MarkdownStyles = Record ({ gap: theme.spacing[2], flexWrap: "wrap", }, + attachmentPreviewContainer: { + flexDirection: "row", + gap: theme.spacing[2], + flexWrap: "wrap", + }, imagePreviewSpacing: { marginBottom: theme.spacing[2], }, @@ -374,6 +381,19 @@ const userMessageStylesheet = StyleSheet.create((theme) => ({ height: 48, backgroundColor: theme.colors.surface1, }, + structuredAttachmentPill: { + maxWidth: 220, + borderRadius: theme.borderRadius.md, + borderWidth: 1, + borderColor: theme.colors.borderAccent, + backgroundColor: theme.colors.surface1, + paddingHorizontal: theme.spacing[3], + paddingVertical: theme.spacing[2], + }, + structuredAttachmentText: { + color: theme.colors.foreground, + fontSize: theme.fontSize.sm, + }, copyButton: { alignSelf: "flex-end", padding: theme.spacing[1], @@ -396,9 +416,23 @@ function UserMessageAttachmentThumbnail({ image }: { image: UserMessageImageAtta return ; } +function getUserMessageAttachmentLabel(attachment: AgentAttachment): string { + switch (attachment.type) { + case "review": { + const count = attachment.comments.length; + return count === 1 ? "Review · 1 comment" : `Review · ${count} comments`; + } + case "github_pr": + return `PR #${attachment.number}`; + case "github_issue": + return `Issue #${attachment.number}`; + } +} + export const UserMessage = memo(function UserMessage({ message, images = [], + attachments = [], timestamp: _timestamp, isFirstInGroup = true, isLastInGroup = true, @@ -410,6 +444,7 @@ export const UserMessage = memo(function UserMessage({ const resolvedDisableOuterSpacing = useDisableOuterSpacing(disableOuterSpacing); const hasText = message.trim().length > 0; const hasImages = images.length > 0; + const hasAttachments = attachments.length > 0; const showCopyButton = hasText && (isCompact || messageHovered || copyButtonHovered); const handleHoverIn = useCallback(() => setMessageHovered(true), []); @@ -430,6 +465,13 @@ export const UserMessage = memo(function UserMessage({ const imagePreviewContainerStyle = useMemo( () => [ userMessageStylesheet.imagePreviewContainer, + hasText || hasAttachments ? userMessageStylesheet.imagePreviewSpacing : undefined, + ], + [hasAttachments, hasText], + ); + const attachmentPreviewContainerStyle = useMemo( + () => [ + userMessageStylesheet.attachmentPreviewContainer, hasText ? userMessageStylesheet.imagePreviewSpacing : undefined, ], [hasText], @@ -461,6 +503,20 @@ export const UserMessage = memo(function UserMessage({ ))} ) : null} + {hasAttachments ? ( + + {attachments.map((attachment, index) => ( + + + {getUserMessageAttachmentLabel(attachment)} + + + ))} + + ) : null} {hasText ? ( {message} diff --git a/packages/app/src/hooks/use-agent-input-draft.ts b/packages/app/src/hooks/use-agent-input-draft.ts index 73e6e08be..0fab4135b 100644 --- a/packages/app/src/hooks/use-agent-input-draft.ts +++ b/packages/app/src/hooks/use-agent-input-draft.ts @@ -1,5 +1,5 @@ import { useCallback, useEffect, useMemo, useRef, useState } from "react"; -import type { ComposerAttachment } from "@/attachments/types"; +import type { UserComposerAttachment } from "@/attachments/types"; import type { DraftAgentStatusBarProps } from "@/components/agent-status-bar"; import type { DraftCommandConfig } from "@/hooks/use-agent-commands-query"; import { @@ -13,8 +13,8 @@ import type { AgentModelDefinition } from "@server/server/agent/agent-sdk-types" import type { AgentProvider } from "@server/server/agent/agent-sdk-types"; type AttachmentUpdater = - | ComposerAttachment[] - | ((prev: ComposerAttachment[]) => ComposerAttachment[]); + | UserComposerAttachment[] + | ((prev: UserComposerAttachment[]) => UserComposerAttachment[]); interface AgentInputDraftComposerOptions { initialServerId: string | null; @@ -48,7 +48,7 @@ type DraftComposerState = UseAgentFormStateResult & { interface AgentInputDraft { text: string; setText: (text: string) => void; - attachments: ComposerAttachment[]; + attachments: UserComposerAttachment[]; setAttachments: (updater: AttachmentUpdater) => void; cwd: string; setCwd: (cwd: string) => void; @@ -59,7 +59,7 @@ interface AgentInputDraft { function hasDraftContent(input: { text: string; - attachments: ComposerAttachment[]; + attachments: UserComposerAttachment[]; cwd: string; }): boolean { return ( @@ -68,8 +68,8 @@ function hasDraftContent(input: { } function areAttachmentsEqual(input: { - left: ComposerAttachment[]; - right: ComposerAttachment[]; + left: UserComposerAttachment[]; + right: UserComposerAttachment[]; }): boolean { if (input.left.length !== input.right.length) { return false; @@ -190,7 +190,7 @@ export function useAgentInputDraft(input: UseAgentInputDraftInput): AgentInputDr [formState.selectedServerId, input.draftKey], ); const [text, setText] = useState(""); - const [attachments, setAttachmentsState] = useState([]); + const [attachments, setAttachmentsState] = useState([]); const [cwd, setCwd] = useState(input.initialCwd ?? ""); const [isHydrated, setIsHydrated] = useState(false); const draftGenerationRef = useRef(0); diff --git a/packages/app/src/hooks/use-draft-agent-create-flow.ts b/packages/app/src/hooks/use-draft-agent-create-flow.ts index ea5eba739..afc14d4d5 100644 --- a/packages/app/src/hooks/use-draft-agent-create-flow.ts +++ b/packages/app/src/hooks/use-draft-agent-create-flow.ts @@ -16,6 +16,7 @@ interface CreateAttempt { text: string; timestamp: Date; images?: UserMessageImageAttachment[]; + attachments?: AgentAttachment[]; } type DraftAgentMachineState = @@ -118,7 +119,11 @@ export function useDraftAgentCreateFlow({ return EMPTY_STREAM_ITEMS; } - if (!machine.attempt.text && (!machine.attempt.images || machine.attempt.images.length === 0)) { + if ( + !machine.attempt.text && + (!machine.attempt.images || machine.attempt.images.length === 0) && + (!machine.attempt.attachments || machine.attempt.attachments.length === 0) + ) { return EMPTY_STREAM_ITEMS; } @@ -131,6 +136,9 @@ export function useDraftAgentCreateFlow({ ...(machine.attempt.images && machine.attempt.images.length > 0 ? { images: machine.attempt.images } : {}), + ...(machine.attempt.attachments && machine.attempt.attachments.length > 0 + ? { attachments: machine.attempt.attachments } + : {}), }, ]; }, [machine]); @@ -182,6 +190,7 @@ export function useDraftAgentCreateFlow({ text: trimmedPrompt, timestamp: new Date(), ...(images && images.length > 0 ? { images } : {}), + ...(wirePayload.attachments.length > 0 ? { attachments: wirePayload.attachments } : {}), }; setPendingCreateAttempt({ @@ -192,6 +201,9 @@ export function useDraftAgentCreateFlow({ text: attempt.text, timestamp: attempt.timestamp.getTime(), ...(attempt.images && attempt.images.length > 0 ? { images: attempt.images } : {}), + ...(attempt.attachments && attempt.attachments.length > 0 + ? { attachments: attempt.attachments } + : {}), }); onBeforeSubmit?.({ diff --git a/packages/app/src/panels/agent-panel.test.tsx b/packages/app/src/panels/agent-panel.test.tsx index e2f8b99cb..16928a8ab 100644 --- a/packages/app/src/panels/agent-panel.test.tsx +++ b/packages/app/src/panels/agent-panel.test.tsx @@ -109,6 +109,7 @@ vi.mock("react-native-unistyles", () => { create: createPanelTestStyles, }, withUnistyles: (component: T) => component, + useUnistyles: () => ({ theme, rt: { breakpoint: "lg" } }), }; }); diff --git a/packages/app/src/panels/agent-panel.tsx b/packages/app/src/panels/agent-panel.tsx index 8c48415c5..4aefa9260 100644 --- a/packages/app/src/panels/agent-panel.tsx +++ b/packages/app/src/panels/agent-panel.tsx @@ -14,6 +14,12 @@ import { FileDropZone } from "@/components/file-drop-zone"; import type { ImageAttachment } from "@/components/message-input"; import { getProviderIcon } from "@/components/provider-icons"; import { ToastViewport, useToastHost } from "@/components/toast-host"; +import type { WorkspaceComposerAttachment } from "@/attachments/types"; +import { + useWorkspaceAttachments, + useWorkspaceAttachmentScopeKey, +} from "@/attachments/workspace-attachments-store"; +import { useIsCompactFormFactor } from "@/constants/layout"; import { isNative, isWeb } from "@/constants/platform"; import { useAgentAttentionClear } from "@/hooks/use-agent-attention-clear"; import { useAgentInitialization } from "@/hooks/use-agent-initialization"; @@ -43,6 +49,7 @@ import { } from "@/screens/agent/agent-ready-screen-bottom-anchor"; import { useCreateFlowStore } from "@/stores/create-flow-store"; import { buildDraftStoreKey } from "@/stores/draft-keys"; +import { usePanelStore } from "@/stores/panel-store"; import { type Agent, useSessionStore } from "@/stores/session-store"; import type { Theme } from "@/styles/theme"; import type { PendingPermission } from "@/types/shared"; @@ -1071,6 +1078,9 @@ function AgentStreamSection({ ...(pendingCreate.images && pendingCreate.images.length > 0 ? { images: pendingCreate.images } : {}), + ...(pendingCreate.attachments && pendingCreate.attachments.length > 0 + ? { attachments: pendingCreate.attachments } + : {}), }, ]; }, [pendingCreate, shouldUseOptimisticStream]); @@ -1101,7 +1111,10 @@ function AgentStreamSection({ } const pendingImages = pendingCreate.images; - if (agentId && pendingImages && pendingImages.length > 0) { + const pendingAttachments = pendingCreate.attachments; + const hasPendingImages = Boolean(pendingImages && pendingImages.length > 0); + const hasPendingAttachments = Boolean(pendingAttachments && pendingAttachments.length > 0); + if (agentId && (hasPendingImages || hasPendingAttachments)) { setAgentStreamTail(serverId, (previous) => { const current = previous.get(agentId); if (!current) { @@ -1112,6 +1125,7 @@ function AgentStreamSection({ streamItems: current, clientMessageId: pendingCreate.clientMessageId, images: pendingImages, + attachments: pendingAttachments, }); if (merged === current) { return previous; @@ -1231,6 +1245,8 @@ function ActiveAgentComposer({ onMessageSent: () => void; }) { const insets = useSafeAreaInsets(); + const isCompact = useIsCompactFormFactor(); + const { workspaceId } = usePaneContext(); const agentInputDraft = useAgentInputDraft({ draftKey: buildDraftStoreKey({ serverId, @@ -1238,6 +1254,32 @@ function ActiveAgentComposer({ }), initialCwd, }); + const workspaceAttachmentScopeKey = useWorkspaceAttachmentScopeKey({ + serverId, + cwd: agentInputDraft.cwd, + workspaceId, + }); + const workspaceAttachments = useWorkspaceAttachments(workspaceAttachmentScopeKey); + const openFileExplorerForCheckout = usePanelStore((state) => state.openFileExplorerForCheckout); + const setExplorerTabForCheckout = usePanelStore((state) => state.setExplorerTabForCheckout); + const handleOpenWorkspaceAttachment = useCallback( + (attachment: WorkspaceComposerAttachment) => { + const checkout = { + serverId, + cwd: attachment.attachment.cwd, + isGit: true, + }; + openFileExplorerForCheckout({ + checkout, + isCompact, + }); + setExplorerTabForCheckout({ + ...checkout, + tab: "changes", + }); + }, + [isCompact, openFileExplorerForCheckout, serverId, setExplorerTabForCheckout], + ); const inputAreaStyle = useMemo( () => [styles.inputAreaWrapper, { paddingBottom: insets.bottom }], @@ -1253,6 +1295,8 @@ function ActiveAgentComposer({ value={agentInputDraft.text} onChangeText={agentInputDraft.setText} attachments={agentInputDraft.attachments} + workspaceAttachments={workspaceAttachments} + onOpenWorkspaceAttachment={handleOpenWorkspaceAttachment} onChangeAttachments={agentInputDraft.setAttachments} cwd={agentInputDraft.cwd} clearDraft={agentInputDraft.clear} diff --git a/packages/app/src/review/index.ts b/packages/app/src/review/index.ts new file mode 100644 index 000000000..1504082f6 --- /dev/null +++ b/packages/app/src/review/index.ts @@ -0,0 +1,33 @@ +export { + buildReviewAttachmentSnapshot, + buildReviewDraftKey, + buildReviewDraftScopeKey, + getReviewDraftComments, + resetReviewDraftStore, + useActiveReviewDraftMode, + useClearReviewDraft, + useReviewAttachmentSnapshot, + useSetActiveReviewDraftMode, + addReviewDraftComment, + type BuildReviewDraftKeyInput, + type BuildReviewDraftScopeKeyInput, + type ReviewDraftCommentInput, + type ReviewDraftComment, + type ReviewDraftMode, + type ReviewDraftSide, +} from "./store"; + +export { + getInlineReviewThreadState, + getInlineReviewThreadViewportStyle, + getSplitInlineReviewThreadState, + groupInlineReviewCommentsByTarget, + InlineReviewEditor, + InlineReviewGutterCell, + InlineReviewThread, + isInlineReviewEditorForTarget, + SMALL_ACTION_HIT_SLOP, + useInlineReviewController, + type InlineReviewActions, + type InlineReviewEditorState, +} from "./surface"; diff --git a/packages/app/src/review/store.test.ts b/packages/app/src/review/store.test.ts new file mode 100644 index 000000000..923655b3e --- /dev/null +++ b/packages/app/src/review/store.test.ts @@ -0,0 +1,279 @@ +import "@/test/window-local-storage"; +import AsyncStorage from "@react-native-async-storage/async-storage"; +import { describe, expect, it } from "vitest"; +import type { ParsedDiffFile } from "@/hooks/use-checkout-diff-query"; +import { + buildReviewAttachmentSnapshot, + buildReviewDraftKey, + buildReviewDraftScopeKey, + useReviewDraftStore, +} from "./store"; + +const STORE_STORAGE_KEY = "@paseo:review-draft-store"; + +function makeFile(): ParsedDiffFile { + return { + path: "src/example.ts", + isNew: false, + isDeleted: false, + additions: 1, + deletions: 1, + status: "ok", + hunks: [ + { + oldStart: 40, + oldCount: 4, + newStart: 40, + newCount: 4, + lines: [ + { type: "header", content: "@@ -40,4 +40,4 @@" }, + { type: "context", content: "const before = true;" }, + { type: "remove", content: "const value = oldValue;" }, + { type: "add", content: "const value = newValue;" }, + { type: "context", content: "return value;" }, + ], + }, + ], + }; +} + +describe("buildReviewDraftKey", () => { + it("scopes by server, workspace-or-cwd, diff mode, base ref, and whitespace mode", () => { + const base = buildReviewDraftKey({ + serverId: " local ", + workspaceId: " workspace-1 ", + cwd: "/repo", + mode: "base", + baseRef: " main ", + ignoreWhitespace: false, + }); + + expect(base).toBe( + "review:server=local:workspace=workspace-1:mode=base:base=main:ignoreWhitespace=false", + ); + expect( + buildReviewDraftKey({ + serverId: "local", + workspaceId: "workspace-1", + cwd: "/repo", + mode: "base", + baseRef: "main", + ignoreWhitespace: true, + }), + ).not.toBe(base); + expect( + buildReviewDraftKey({ + serverId: "local", + workspaceId: null, + cwd: "/repo/", + mode: "base", + baseRef: "main", + ignoreWhitespace: false, + }), + ).toBe("review:server=local:cwd=%2Frepo:mode=base:base=main:ignoreWhitespace=false"); + }); + + it("builds a mode-free scope key for active review mode sharing", () => { + const scope = buildReviewDraftScopeKey({ + serverId: "local", + workspaceId: "workspace-1", + cwd: "/repo", + baseRef: "main", + ignoreWhitespace: false, + }); + + expect(scope).toBe( + "review:server=local:workspace=workspace-1:base=main:ignoreWhitespace=false", + ); + expect(scope).not.toContain("mode="); + }); +}); + +describe("review draft store", () => { + it("normalizes persisted active review modes with draft comments", async () => { + await AsyncStorage.setItem( + STORE_STORAGE_KEY, + JSON.stringify({ + version: 0, + state: { + drafts: { + "review:key": [ + { + id: "comment-1", + filePath: "src/example.ts", + side: "new", + lineNumber: 41, + body: "Keep me.", + createdAt: "2026-04-21T00:00:00.000Z", + updatedAt: "2026-04-21T00:00:00.000Z", + }, + { id: "bad", filePath: "src/example.ts" }, + ], + }, + activeModesByScope: { + "review:scope:base": "base", + "review:scope:dirty": "uncommitted", + "review:scope:bad": "other", + }, + }, + }), + ); + + await useReviewDraftStore.persist.rehydrate(); + + const state = useReviewDraftStore.getState(); + expect(state.activeModesByScope).toEqual({ + "review:scope:base": "base", + "review:scope:dirty": "uncommitted", + }); + expect(state.drafts["review:key"]).toEqual([ + { + id: "comment-1", + filePath: "src/example.ts", + side: "new", + lineNumber: 41, + body: "Keep me.", + createdAt: "2026-04-21T00:00:00.000Z", + updatedAt: "2026-04-21T00:00:00.000Z", + }, + ]); + }); + + it("persists compact draft comments separately from workspace attachment wire context", () => { + const key = buildReviewDraftKey({ + serverId: "local", + workspaceId: "workspace-1", + cwd: "/repo", + mode: "uncommitted", + baseRef: null, + ignoreWhitespace: false, + }); + + useReviewDraftStore.getState().clearReview({ key }); + useReviewDraftStore.getState().addComment({ + key, + comment: { + id: "comment-1", + filePath: "src/example.ts", + side: "new", + lineNumber: 41, + body: "Please simplify this.", + createdAt: "2026-04-21T00:00:00.000Z", + updatedAt: "2026-04-21T00:00:00.000Z", + }, + }); + + expect(useReviewDraftStore.getState().drafts[key]).toEqual([ + { + id: "comment-1", + filePath: "src/example.ts", + side: "new", + lineNumber: 41, + body: "Please simplify this.", + createdAt: "2026-04-21T00:00:00.000Z", + updatedAt: "2026-04-21T00:00:00.000Z", + }, + ]); + + useReviewDraftStore.getState().updateComment({ + key, + id: "comment-1", + updates: { body: "Please simplify this condition." }, + updatedAt: "2026-04-21T00:01:00.000Z", + }); + expect(useReviewDraftStore.getState().drafts[key]?.[0]?.body).toBe( + "Please simplify this condition.", + ); + + useReviewDraftStore.getState().deleteComment({ key, id: "comment-1" }); + expect(useReviewDraftStore.getState().drafts[key]).toEqual([]); + }); +}); + +describe("buildReviewAttachmentSnapshot", () => { + it("builds a bounded workspace review attachment and skips missing targets", () => { + const snapshot = buildReviewAttachmentSnapshot({ + reviewDraftKey: "review:key", + cwd: "/repo", + mode: "base", + baseRef: "main", + comments: [ + { + id: "comment-1", + filePath: "src/example.ts", + side: "new", + lineNumber: 41, + body: "Please simplify this.", + createdAt: "2026-04-21T00:00:00.000Z", + updatedAt: "2026-04-21T00:00:00.000Z", + }, + { + id: "comment-2", + filePath: "src/missing.ts", + side: "new", + lineNumber: 99, + body: "This target is stale.", + createdAt: "2026-04-21T00:00:00.000Z", + updatedAt: "2026-04-21T00:00:00.000Z", + }, + ], + diffFiles: [makeFile()], + }); + + expect(snapshot).toEqual({ + kind: "review", + reviewDraftKey: "review:key", + commentCount: 1, + attachment: { + type: "review", + mimeType: "application/paseo-review", + cwd: "/repo", + mode: "base", + baseRef: "main", + comments: [ + { + filePath: "src/example.ts", + side: "new", + lineNumber: 41, + body: "Please simplify this.", + context: { + hunkHeader: "@@ -40,4 +40,4 @@", + targetLine: { + oldLineNumber: null, + newLineNumber: 41, + type: "add", + content: "const value = newValue;", + }, + lines: [ + { + oldLineNumber: 40, + newLineNumber: 40, + type: "context", + content: "const before = true;", + }, + { + oldLineNumber: 41, + newLineNumber: null, + type: "remove", + content: "const value = oldValue;", + }, + { + oldLineNumber: null, + newLineNumber: 41, + type: "add", + content: "const value = newValue;", + }, + { + oldLineNumber: 42, + newLineNumber: 42, + type: "context", + content: "return value;", + }, + ], + }, + }, + ], + }, + }); + }); +}); diff --git a/packages/app/src/review/store.ts b/packages/app/src/review/store.ts new file mode 100644 index 000000000..cdc7b5faf --- /dev/null +++ b/packages/app/src/review/store.ts @@ -0,0 +1,456 @@ +import { useMemo } from "react"; +import AsyncStorage from "@react-native-async-storage/async-storage"; +import { create } from "zustand"; +import { createJSONStorage, persist } from "zustand/middleware"; +import type { ComposerAttachment } from "@/attachments/types"; +import type { ParsedDiffFile } from "@/hooks/use-checkout-diff-query"; +import { generateMessageId } from "@/types/stream"; +import { buildNumberedDiffHunks, type NumberedDiffLine } from "@/utils/diff-layout"; +import type { AgentAttachment } from "@server/shared/messages"; + +const STORE_VERSION = 1; +const CONTEXT_RADIUS = 3; +const EMPTY_REVIEW_DRAFT_COMMENTS: ReviewDraftComment[] = []; + +type ReviewAttachment = Extract; +type ReviewAttachmentContextLine = ReviewAttachment["comments"][number]["context"]["targetLine"]; +type ReviewComposerAttachment = Extract; + +export type ReviewDraftMode = "uncommitted" | "base"; +export type ReviewDraftSide = "old" | "new"; + +export interface ReviewDraftComment { + id: string; + filePath: string; + side: ReviewDraftSide; + lineNumber: number; + body: string; + createdAt: string; + updatedAt: string; +} + +export interface BuildReviewDraftKeyInput { + serverId: string; + workspaceId?: string | null; + cwd: string; + mode: ReviewDraftMode; + baseRef?: string | null; + ignoreWhitespace: boolean; +} + +export type BuildReviewDraftScopeKeyInput = Omit; + +export interface BuildReviewAttachmentSnapshotInput { + reviewDraftKey: string; + cwd: string; + mode: ReviewDraftMode; + baseRef?: string | null; + comments: readonly ReviewDraftComment[]; + diffFiles: readonly ParsedDiffFile[]; +} + +interface ReviewDraftStoreState { + drafts: Record; + activeModesByScope: Record; +} + +interface ReviewDraftStoreActions { + setActiveMode: (input: { scopeKey: string; mode: ReviewDraftMode }) => void; + addComment: (input: { key: string; comment: ReviewDraftCommentInput }) => ReviewDraftComment; + updateComment: (input: { + key: string; + id: string; + updates: Partial>; + updatedAt?: string; + }) => void; + deleteComment: (input: { key: string; id: string }) => void; + clearReview: (input: { key: string }) => void; +} + +export type ReviewDraftCommentInput = Omit & + Partial>; + +type ReviewDraftStore = ReviewDraftStoreState & ReviewDraftStoreActions; + +function encodeKeyPart(value: string): string { + return encodeURIComponent(value.trim()); +} + +function normalizeCwd(cwd: string): string { + const trimmed = cwd.trim(); + if (trimmed === "/") { + return trimmed; + } + return trimmed.replace(/\/+$/, ""); +} + +function normalizeBaseRef(baseRef: string | null | undefined): string { + return baseRef?.trim() ?? ""; +} + +function buildReviewDraftScopeParts(input: BuildReviewDraftScopeKeyInput): string[] { + const workspaceId = input.workspaceId?.trim(); + const workspacePart = workspaceId + ? `workspace=${encodeKeyPart(workspaceId)}` + : `cwd=${encodeKeyPart(normalizeCwd(input.cwd))}`; + + return [ + "review", + `server=${encodeKeyPart(input.serverId)}`, + workspacePart, + `base=${encodeKeyPart(normalizeBaseRef(input.baseRef))}`, + `ignoreWhitespace=${input.ignoreWhitespace ? "true" : "false"}`, + ]; +} + +export function buildReviewDraftScopeKey(input: BuildReviewDraftScopeKeyInput): string { + return buildReviewDraftScopeParts(input).join(":"); +} + +export function buildReviewDraftKey(input: BuildReviewDraftKeyInput): string { + const [prefix, serverPart, workspacePart, basePart, whitespacePart] = + buildReviewDraftScopeParts(input); + return [prefix, serverPart, workspacePart, `mode=${input.mode}`, basePart, whitespacePart].join( + ":", + ); +} + +function createDraftComment(input: ReviewDraftCommentInput): ReviewDraftComment { + const now = new Date().toISOString(); + return { + id: input.id ?? generateMessageId(), + filePath: input.filePath, + side: input.side, + lineNumber: input.lineNumber, + body: input.body, + createdAt: input.createdAt ?? now, + updatedAt: input.updatedAt ?? input.createdAt ?? now, + }; +} + +function applyCommentUpdates( + comment: ReviewDraftComment, + targetId: string, + updates: Partial>, + updatedAt: string, +): ReviewDraftComment { + if (comment.id !== targetId) { + return comment; + } + return { + id: comment.id, + filePath: comment.filePath, + side: comment.side, + lineNumber: comment.lineNumber, + body: updates.body ?? comment.body, + createdAt: comment.createdAt, + updatedAt, + }; +} + +function normalizePersistedState(state: unknown): ReviewDraftStoreState { + if (!state || typeof state !== "object") { + return { drafts: {}, activeModesByScope: {} }; + } + const persisted = state as { drafts?: unknown; activeModesByScope?: unknown }; + const drafts = persisted.drafts; + if (!drafts || typeof drafts !== "object" || Array.isArray(drafts)) { + return { drafts: {}, activeModesByScope: {} }; + } + + const normalized: Record = {}; + for (const [key, value] of Object.entries(drafts)) { + if (!Array.isArray(value)) { + continue; + } + normalized[key] = value.filter((comment): comment is ReviewDraftComment => + isReviewDraftComment(comment), + ); + } + + const activeModesByScope: Record = {}; + const persistedActiveModes = persisted.activeModesByScope; + if ( + persistedActiveModes && + typeof persistedActiveModes === "object" && + !Array.isArray(persistedActiveModes) + ) { + for (const [key, mode] of Object.entries(persistedActiveModes)) { + if (mode === "base" || mode === "uncommitted") { + activeModesByScope[key] = mode; + } + } + } + return { drafts: normalized, activeModesByScope }; +} + +function isReviewDraftComment(value: unknown): value is ReviewDraftComment { + if (!value || typeof value !== "object") { + return false; + } + const record = value as Record; + return ( + typeof record.id === "string" && + typeof record.filePath === "string" && + (record.side === "old" || record.side === "new") && + typeof record.lineNumber === "number" && + Number.isInteger(record.lineNumber) && + record.lineNumber > 0 && + typeof record.body === "string" && + typeof record.createdAt === "string" && + typeof record.updatedAt === "string" + ); +} + +export const useReviewDraftStore = create()( + persist( + (set) => ({ + drafts: {}, + activeModesByScope: {}, + setActiveMode: ({ scopeKey, mode }) => { + set((state) => { + if (state.activeModesByScope[scopeKey] === mode) { + return state; + } + return { + activeModesByScope: { + ...state.activeModesByScope, + [scopeKey]: mode, + }, + }; + }); + }, + addComment: ({ key, comment }) => { + const nextComment = createDraftComment(comment); + set((state) => ({ + drafts: { + ...state.drafts, + [key]: [...(state.drafts[key] ?? []), nextComment], + }, + })); + return nextComment; + }, + updateComment: ({ key, id, updates, updatedAt }) => { + set((state) => { + const comments = state.drafts[key] ?? []; + if (!comments.some((comment) => comment.id === id)) { + return state; + } + const nextUpdatedAt = updatedAt ?? new Date().toISOString(); + return { + drafts: { + ...state.drafts, + [key]: comments.map((comment) => + applyCommentUpdates(comment, id, updates, nextUpdatedAt), + ), + }, + }; + }); + }, + deleteComment: ({ key, id }) => { + set((state) => { + const comments = state.drafts[key] ?? []; + if (!comments.some((comment) => comment.id === id)) { + return state; + } + return { + drafts: { + ...state.drafts, + [key]: comments.filter((comment) => comment.id !== id), + }, + }; + }); + }, + clearReview: ({ key }) => { + set((state) => { + if (!state.drafts[key]) { + return state; + } + const next = { ...state.drafts }; + delete next[key]; + return { drafts: next }; + }); + }, + }), + { + name: "@paseo:review-draft-store", + version: STORE_VERSION, + storage: createJSONStorage(() => AsyncStorage), + partialize: (state) => ({ + drafts: state.drafts, + activeModesByScope: state.activeModesByScope, + }), + migrate: async (state) => normalizePersistedState(state), + }, + ), +); + +function toContextLine(line: NumberedDiffLine): ReviewAttachmentContextLine | null { + if (line.line.type === "header") { + return null; + } + return { + oldLineNumber: line.oldLineNumber, + newLineNumber: line.newLineNumber, + type: line.line.type, + content: line.line.content, + }; +} + +function findTarget(input: { comment: ReviewDraftComment; diffFiles: readonly ParsedDiffFile[] }): { + hunkHeader: string; + hunkLines: NumberedDiffLine[]; + targetIndex: number; + targetLine: NumberedDiffLine; +} | null { + const file = input.diffFiles.find((candidate) => candidate.path === input.comment.filePath); + if (!file) { + return null; + } + + for (const hunk of buildNumberedDiffHunks(file)) { + const targetIndex = hunk.lines.findIndex((line) => { + const cell = input.comment.side === "old" ? line.oldCell : line.newCell; + return cell?.lineNumber === input.comment.lineNumber; + }); + const targetLine = hunk.lines[targetIndex]; + if (targetLine) { + return { + hunkHeader: hunk.hunkHeader, + hunkLines: hunk.lines, + targetIndex, + targetLine, + }; + } + } + + return null; +} + +export function buildReviewAttachmentSnapshot( + input: BuildReviewAttachmentSnapshotInput, +): ReviewComposerAttachment | null { + const comments: ReviewAttachment["comments"] = []; + + for (const draftComment of input.comments) { + const target = findTarget({ + comment: draftComment, + diffFiles: input.diffFiles, + }); + if (!target) { + continue; + } + + const targetLine = toContextLine(target.targetLine); + if (!targetLine) { + continue; + } + + const contextStart = Math.max(0, target.targetIndex - CONTEXT_RADIUS); + const contextEnd = Math.min(target.hunkLines.length, target.targetIndex + CONTEXT_RADIUS + 1); + const lines = target.hunkLines + .slice(contextStart, contextEnd) + .map(toContextLine) + .filter((line): line is ReviewAttachmentContextLine => line !== null); + + comments.push({ + filePath: draftComment.filePath, + side: draftComment.side, + lineNumber: draftComment.lineNumber, + body: draftComment.body, + context: { + hunkHeader: target.hunkHeader, + targetLine, + lines, + }, + }); + } + + if (comments.length === 0) { + return null; + } + + const attachment: ReviewAttachment = { + type: "review", + mimeType: "application/paseo-review", + cwd: input.cwd, + mode: input.mode, + baseRef: normalizeBaseRef(input.baseRef) || null, + comments, + }; + + return { + kind: "review", + reviewDraftKey: input.reviewDraftKey, + commentCount: comments.length, + attachment, + }; +} + +export function useReviewDraftComments(key: string): ReviewDraftComment[] { + return useReviewDraftStore((state) => state.drafts[key] ?? EMPTY_REVIEW_DRAFT_COMMENTS); +} + +export function useSetActiveReviewDraftMode(): ReviewDraftStoreActions["setActiveMode"] { + return useReviewDraftStore((state) => state.setActiveMode); +} + +export function useClearReviewDraft(): ReviewDraftStoreActions["clearReview"] { + return useReviewDraftStore((state) => state.clearReview); +} + +export function addReviewDraftComment(input: { + key: string; + comment: ReviewDraftCommentInput; +}): ReviewDraftComment { + return useReviewDraftStore.getState().addComment(input); +} + +export function getReviewDraftComments(key: string): ReviewDraftComment[] | undefined { + return useReviewDraftStore.getState().drafts[key]; +} + +export function resetReviewDraftStore(): void { + useReviewDraftStore.setState({ drafts: {}, activeModesByScope: {} }); +} + +export function useReviewDraftCommentsForAttachment(input: { + key: string; + enabled: boolean; +}): ReviewDraftComment[] { + return useReviewDraftStore((state) => + input.enabled + ? (state.drafts[input.key] ?? EMPTY_REVIEW_DRAFT_COMMENTS) + : EMPTY_REVIEW_DRAFT_COMMENTS, + ); +} + +export function useReviewCommentCount(key: string): number { + return useReviewDraftStore((state) => state.drafts[key]?.length ?? 0); +} + +export function useActiveReviewDraftMode(input: { scopeKey: string }): ReviewDraftMode | null { + return useReviewDraftStore((state) => state.activeModesByScope[input.scopeKey] ?? null); +} + +export function useReviewAttachmentSnapshot(input: { + key: string; + diffFiles: readonly ParsedDiffFile[]; + cwd: string; + mode: ReviewDraftMode; + baseRef?: string | null; +}): ReviewComposerAttachment | null { + const comments = useReviewDraftComments(input.key); + return useMemo( + () => + buildReviewAttachmentSnapshot({ + reviewDraftKey: input.key, + cwd: input.cwd, + mode: input.mode, + baseRef: input.baseRef, + comments, + diffFiles: input.diffFiles, + }), + [comments, input.key, input.cwd, input.mode, input.baseRef, input.diffFiles], + ); +} diff --git a/packages/app/src/review/surface.test.tsx b/packages/app/src/review/surface.test.tsx new file mode 100644 index 000000000..050209339 --- /dev/null +++ b/packages/app/src/review/surface.test.tsx @@ -0,0 +1,389 @@ +// @vitest-environment jsdom +import "@/test/window-local-storage"; +import { act, fireEvent, render, renderHook, cleanup } from "@testing-library/react"; +import React from "react"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { useReviewDraftStore, type ReviewDraftComment } from "./store"; +import { buildReviewableDiffTargetKey, type ReviewableDiffTarget } from "@/utils/diff-layout"; +import { + getInlineReviewThreadState, + getInlineReviewThreadViewportStyle, + getSplitInlineReviewThreadState, + groupInlineReviewCommentsByTarget, + InlineReviewEditor, + InlineReviewGutterCell, + InlineReviewThread, + SMALL_ACTION_HIT_SLOP, + useInlineReviewController, + type InlineReviewActions, +} from "./index"; + +const { theme, pressablePropsByLabel } = vi.hoisted(() => { + Object.assign(globalThis, { __DEV__: false }); + return { + theme: { + spacing: { 1: 4, 2: 8, 3: 12 }, + borderWidth: { 1: 1 }, + borderRadius: { base: 4 }, + fontSize: { xs: 11, sm: 13 }, + fontWeight: { medium: "500" }, + lineHeight: { diff: 18 }, + colors: { + accent: "#0a84ff", + accentForeground: "#fff", + border: "#555", + destructive: "#ff453a", + foreground: "#fff", + foregroundMuted: "#aaa", + surface1: "#111", + surface2: "#222", + surface3: "#333", + }, + }, + pressablePropsByLabel: new Map>(), + }; +}); + +vi.mock("react-native", async (importOriginal) => { + const ReactModule = await import("react"); + const actual = await importOriginal(); + return { + ...actual, + Pressable: ({ + accessibilityLabel, + children, + onPress, + ...props + }: { + accessibilityLabel?: string; + children?: + | React.ReactNode + | ((state: { hovered: boolean; pressed: boolean }) => React.ReactNode); + onPress?: () => void; + [key: string]: unknown; + }) => { + if (accessibilityLabel) { + pressablePropsByLabel.set(accessibilityLabel, props); + } + const resolvedChildren = + typeof children === "function" ? children({ hovered: false, pressed: false }) : children; + return ReactModule.createElement( + "button", + { + "aria-label": accessibilityLabel, + "data-testid": typeof props.testID === "string" ? props.testID : undefined, + disabled: props.disabled === true, + onClick: onPress, + type: "button", + }, + resolvedChildren, + ); + }, + }; +}); + +vi.mock("react-native-unistyles", () => ({ + StyleSheet: { + create: (factory: unknown) => (typeof factory === "function" ? factory(theme) : factory), + }, + withUnistyles: (component: T) => component, + useUnistyles: () => ({ theme }), +})); + +vi.mock("@/constants/platform", () => ({ + isNative: false, + isWeb: true, +})); + +vi.mock("lucide-react-native", () => { + const createIcon = (name: string) => (props: Record) => + React.createElement("span", { ...props, "data-icon": name }); + return { + Check: createIcon("Check"), + CircleDot: createIcon("CircleDot"), + MessageCircle: createIcon("MessageCircle"), + Pencil: createIcon("Pencil"), + Trash2: createIcon("Trash2"), + X: createIcon("X"), + }; +}); + +function target(overrides: Partial = {}): ReviewableDiffTarget { + return { + filePath: "src/example.ts", + hunkHeader: "@@ -1,2 +1,2 @@", + hunkIndex: 0, + lineIndex: 2, + oldLineNumber: null, + newLineNumber: 2, + side: "new", + lineNumber: 2, + lineType: "add", + content: "const value = next;", + ...overrides, + key: buildReviewableDiffTargetKey({ + filePath: overrides.filePath ?? "src/example.ts", + side: overrides.side ?? "new", + lineNumber: overrides.lineNumber ?? 2, + }), + }; +} + +const EMPTY_COMMENTS: ReviewDraftComment[] = []; + +function buildReviewActions(overrides: Partial = {}): InlineReviewActions { + return { + commentsByTarget: new Map(), + editor: null, + showPersistentAction: false, + onStartComment: vi.fn(), + onEditComment: vi.fn(), + onCancelEditor: vi.fn(), + onSaveEditor: vi.fn(), + onDeleteComment: vi.fn(), + ...overrides, + }; +} + +function comment(overrides: Partial = {}): ReviewDraftComment { + return { + id: "comment-1", + filePath: "src/example.ts", + side: "new", + lineNumber: 2, + body: "Please simplify this.", + createdAt: "2026-04-21T00:00:00.000Z", + updatedAt: "2026-04-21T00:00:00.000Z", + ...overrides, + }; +} + +describe("useInlineReviewController", () => { + beforeEach(() => { + useReviewDraftStore.setState({ drafts: {}, activeModesByScope: {} }); + }); + + afterEach(() => { + cleanup(); + vi.clearAllMocks(); + }); + + it("owns draft comment add, edit, delete, cancel, and key-change editor reset", () => { + const reviewTarget = target(); + const firstKey = "review:key-1"; + const secondKey = "review:key-2"; + const { result, rerender } = renderHook( + ({ reviewDraftKey }) => + useInlineReviewController({ reviewDraftKey, showPersistentAction: true }), + { initialProps: { reviewDraftKey: firstKey } }, + ); + + expect(result.current.showPersistentAction).toBe(true); + + act(() => result.current.onStartComment(reviewTarget)); + expect(result.current.editor).toEqual({ target: reviewTarget, commentId: null, body: "" }); + + act(() => result.current.onSaveEditor(" first comment ")); + const savedComment = useReviewDraftStore.getState().drafts[firstKey]?.[0]; + expect(savedComment).toMatchObject({ + filePath: "src/example.ts", + side: "new", + lineNumber: 2, + body: "first comment", + }); + expect(result.current.editor).toBeNull(); + expect(result.current.commentsByTarget.get(reviewTarget.key)?.[0]).toMatchObject({ + body: "first comment", + }); + + act(() => result.current.onEditComment(reviewTarget, savedComment!)); + expect(result.current.editor).toEqual({ + target: reviewTarget, + commentId: savedComment?.id, + body: "first comment", + }); + + act(() => result.current.onSaveEditor(" updated comment ")); + const updatedComment = useReviewDraftStore.getState().drafts[firstKey]?.[0]; + expect(updatedComment).toMatchObject({ id: savedComment?.id, body: "updated comment" }); + + act(() => result.current.onEditComment(reviewTarget, updatedComment!)); + act(() => result.current.onDeleteComment(updatedComment!.id)); + expect(useReviewDraftStore.getState().drafts[firstKey]).toEqual([]); + expect(result.current.editor).toBeNull(); + + act(() => result.current.onStartComment(reviewTarget)); + act(() => result.current.onCancelEditor()); + expect(result.current.editor).toBeNull(); + + act(() => result.current.onStartComment(reviewTarget)); + rerender({ reviewDraftKey: secondKey }); + expect(result.current.editor).toBeNull(); + }); +}); + +describe("git diff inline review helpers", () => { + afterEach(() => { + cleanup(); + vi.clearAllMocks(); + pressablePropsByLabel.clear(); + }); + + it("maps persisted draft comments to their reviewable diff target", () => { + const comments = [ + comment(), + comment({ id: "comment-2", side: "old", lineNumber: 8 }), + comment({ id: "comment-3", filePath: "src/other.ts" }), + ]; + const commentsByTarget = groupInlineReviewCommentsByTarget(comments); + + expect(commentsByTarget.get("src/example.ts:new:2")).toEqual([comments[0]]); + expect(commentsByTarget.get("src/example.ts:old:8")).toEqual([comments[1]]); + expect(commentsByTarget.get("src/other.ts:new:2")).toEqual([comments[2]]); + expect( + getInlineReviewThreadState({ + reviewTarget: target(), + reviewActions: { + commentsByTarget, + editor: null, + showPersistentAction: false, + onStartComment: vi.fn(), + onEditComment: vi.fn(), + onCancelEditor: vi.fn(), + onSaveEditor: vi.fn(), + onDeleteComment: vi.fn(), + }, + })?.comments, + ).toEqual([comments[0]]); + }); + + it("reserves split inline review height from the taller side", () => { + const leftTarget = target({ side: "old", lineNumber: 8, oldLineNumber: 8 }); + const rightTarget = target(); + const rightComment = comment(); + const actions: InlineReviewActions = { + commentsByTarget: groupInlineReviewCommentsByTarget([rightComment]), + editor: { target: rightTarget, commentId: null, body: "" }, + showPersistentAction: false, + onStartComment: vi.fn(), + onEditComment: vi.fn(), + onCancelEditor: vi.fn(), + onSaveEditor: vi.fn(), + onDeleteComment: vi.fn(), + }; + + const rowState = getSplitInlineReviewThreadState({ + left: leftTarget, + right: rightTarget, + reviewActions: actions, + }); + + expect(rowState?.left).toBeNull(); + expect(rowState?.right?.comments).toEqual([rightComment]); + expect(rowState?.height).toBe(210); + }); + + it("pins no-wrap review threads to the visible diff viewport", () => { + expect( + getInlineReviewThreadViewportStyle({ + viewportWidth: 320, + pinToViewport: true, + }), + ).toEqual([{ position: "sticky", left: 0 }, { width: 320 }]); + }); + + it("keeps the gutter add-comment target accessible", () => { + const onStartComment = vi.fn(); + const reviewTarget = target(); + const { getByLabelText } = render( + + 2 + , + ); + + fireEvent.click(getByLabelText("Add review comment")); + expect(onStartComment).toHaveBeenCalledWith(reviewTarget); + expect(pressablePropsByLabel.get("Add review comment")?.hitSlop).toBe(SMALL_ACTION_HIT_SLOP); + }); + + it("replaces the line number with the message icon in the same gutter cell", () => { + const reviewTarget = target(); + const { container, queryByText } = render( + + 2 + , + ); + + expect(queryByText("2")).toBeNull(); + expect(container.querySelector("[data-icon='MessageCircle']")).toBeTruthy(); + }); +}); + +describe("InlineReviewEditor", () => { + afterEach(() => { + cleanup(); + vi.clearAllMocks(); + }); + + it("saves trimmed bodies and cancels without saving", () => { + const onCancel = vi.fn(); + const onSave = vi.fn(); + const { getByTestId } = render( + , + ); + + fireEvent.change(getByTestId("editor-input"), { target: { value: " updated comment " } }); + fireEvent.click(getByTestId("editor-save")); + expect(onSave).toHaveBeenCalledWith("updated comment"); + + fireEvent.click(getByTestId("editor-cancel")); + expect(onCancel).toHaveBeenCalledTimes(1); + }); +}); + +describe("InlineReviewThread", () => { + afterEach(() => { + cleanup(); + vi.clearAllMocks(); + }); + + it("exposes edit and delete actions for existing comments", () => { + const reviewTarget = target(); + const draftComment = comment(); + const actions = buildReviewActions({ + commentsByTarget: groupInlineReviewCommentsByTarget([draftComment]), + }); + + const { getByTestId, getByText } = render( + , + ); + + expect(getByText("Please simplify this.")).toBeTruthy(); + fireEvent.click(getByTestId("review-comment-edit-comment-1")); + expect(actions.onEditComment).toHaveBeenCalledWith(reviewTarget, draftComment); + fireEvent.click(getByTestId("review-comment-delete-comment-1")); + expect(actions.onDeleteComment).toHaveBeenCalledWith("comment-1"); + }); +}); diff --git a/packages/app/src/review/surface.tsx b/packages/app/src/review/surface.tsx new file mode 100644 index 000000000..9522da890 --- /dev/null +++ b/packages/app/src/review/surface.tsx @@ -0,0 +1,663 @@ +import React, { useCallback, useEffect, useMemo, useRef, useState } from "react"; +import type { ReactNode } from "react"; +import { MessageCircle, Pencil, Trash2 } from "lucide-react-native"; +import { + Pressable, + type PressableStateCallbackType, + Text, + TextInput, + type TextStyle, + View, + type StyleProp, + type ViewStyle, +} from "react-native"; +import { StyleSheet, useUnistyles } from "react-native-unistyles"; +import { isNative, isWeb } from "@/constants/platform"; +import { useReviewDraftComments, useReviewDraftStore, type ReviewDraftComment } from "./store"; +import { buildReviewableDiffTargetKey, type ReviewableDiffTarget } from "@/utils/diff-layout"; + +type PressableState = PressableStateCallbackType & { hovered?: boolean }; + +function iconButtonStyle({ hovered, pressed }: PressableState): StyleProp { + return [styles.iconButton, (hovered || pressed) && styles.iconButtonHovered]; +} + +function iconButtonDestructiveStyle({ hovered, pressed }: PressableState): StyleProp { + return [styles.iconButton, (hovered || pressed) && styles.iconButtonDestructiveHovered]; +} + +function ghostButtonStyle({ hovered, pressed }: PressableState): StyleProp { + return [styles.ghostButton, (hovered || pressed) && styles.ghostButtonHovered]; +} + +export const INLINE_REVIEW_COMMENT_HEIGHT = 72; +export const INLINE_REVIEW_EDITOR_HEIGHT = 132; +const INLINE_REVIEW_GAP = 6; +export const SMALL_ACTION_HIT_SLOP = 8; + +export interface InlineReviewEditorState { + target: ReviewableDiffTarget; + commentId: string | null; + body: string; +} + +export interface InlineReviewActions { + commentsByTarget: ReadonlyMap; + editor: InlineReviewEditorState | null; + showPersistentAction: boolean; + onStartComment: (target: ReviewableDiffTarget) => void; + onEditComment: (target: ReviewableDiffTarget, comment: ReviewDraftComment) => void; + onCancelEditor: () => void; + onSaveEditor: (body: string) => void; + onDeleteComment: (id: string) => void; +} + +export function groupInlineReviewCommentsByTarget( + comments: readonly ReviewDraftComment[], +): Map { + const grouped = new Map(); + for (const comment of comments) { + const key = buildReviewableDiffTargetKey(comment); + grouped.set(key, [...(grouped.get(key) ?? []), comment]); + } + return grouped; +} + +export function useInlineReviewController(input: { + reviewDraftKey: string; + showPersistentAction: boolean; +}): InlineReviewActions { + const reviewComments = useReviewDraftComments(input.reviewDraftKey); + const commentsByTarget = useMemo( + () => groupInlineReviewCommentsByTarget(reviewComments), + [reviewComments], + ); + const [editor, setEditor] = useState(null); + const addComment = useReviewDraftStore((state) => state.addComment); + const updateComment = useReviewDraftStore((state) => state.updateComment); + const deleteComment = useReviewDraftStore((state) => state.deleteComment); + + useEffect(() => { + setEditor(null); + }, [input.reviewDraftKey]); + + const handleStartComment = useCallback((target: ReviewableDiffTarget) => { + setEditor({ target, commentId: null, body: "" }); + }, []); + + const handleEditComment = useCallback( + (target: ReviewableDiffTarget, comment: ReviewDraftComment) => { + setEditor({ target, commentId: comment.id, body: comment.body }); + }, + [], + ); + + const handleCancelEditor = useCallback(() => { + setEditor(null); + }, []); + + const handleSaveEditor = useCallback( + (body: string) => { + const trimmedBody = body.trim(); + if (!editor || trimmedBody.length === 0) { + return; + } + + if (editor.commentId) { + updateComment({ + key: input.reviewDraftKey, + id: editor.commentId, + updates: { body: trimmedBody }, + }); + } else { + addComment({ + key: input.reviewDraftKey, + comment: { + filePath: editor.target.filePath, + side: editor.target.side, + lineNumber: editor.target.lineNumber, + body: trimmedBody, + }, + }); + } + setEditor(null); + }, + [addComment, editor, input.reviewDraftKey, updateComment], + ); + + const handleDeleteComment = useCallback( + (id: string) => { + deleteComment({ key: input.reviewDraftKey, id }); + setEditor((current) => (current?.commentId === id ? null : current)); + }, + [deleteComment, input.reviewDraftKey], + ); + + return useMemo( + () => ({ + commentsByTarget, + editor, + showPersistentAction: input.showPersistentAction, + onStartComment: handleStartComment, + onEditComment: handleEditComment, + onCancelEditor: handleCancelEditor, + onSaveEditor: handleSaveEditor, + onDeleteComment: handleDeleteComment, + }), + [ + commentsByTarget, + editor, + handleCancelEditor, + handleDeleteComment, + handleEditComment, + handleSaveEditor, + handleStartComment, + input.showPersistentAction, + ], + ); +} + +export function isInlineReviewEditorForTarget( + editor: InlineReviewEditorState | null, + target: ReviewableDiffTarget | null | undefined, +): boolean { + return Boolean( + editor && + target && + buildReviewableDiffTargetKey(editor.target) === buildReviewableDiffTargetKey(target), + ); +} + +export function getInlineReviewThreadState(input: { + reviewTarget: ReviewableDiffTarget | null | undefined; + reviewActions?: InlineReviewActions; +}): { + comments: ReviewDraftComment[]; + hasEditor: boolean; + editingCommentId: string | null; + height: number; +} | null { + const { reviewTarget, reviewActions } = input; + if (!reviewTarget || !reviewActions) { + return null; + } + + const comments = reviewActions.commentsByTarget.get(reviewTarget.key) ?? []; + const editorForTarget = isInlineReviewEditorForTarget(reviewActions.editor, reviewTarget) + ? reviewActions.editor + : null; + const hasEditor = editorForTarget !== null; + const editingCommentId = editorForTarget?.commentId ?? null; + const editingExisting = + editingCommentId !== null && comments.some((comment) => comment.id === editingCommentId); + + const visibleCommentCount = editingExisting ? comments.length - 1 : comments.length; + const editorCount = hasEditor ? 1 : 0; + const visibleBlockCount = visibleCommentCount + editorCount; + if (visibleBlockCount === 0) { + return null; + } + + const height = + visibleCommentCount * INLINE_REVIEW_COMMENT_HEIGHT + + editorCount * INLINE_REVIEW_EDITOR_HEIGHT + + Math.max(0, visibleBlockCount - 1) * INLINE_REVIEW_GAP; + + return { comments, hasEditor, editingCommentId, height }; +} + +export function getSplitInlineReviewThreadState(input: { + left: ReviewableDiffTarget | null | undefined; + right: ReviewableDiffTarget | null | undefined; + reviewActions?: InlineReviewActions; +}): { + left: ReturnType; + right: ReturnType; + height: number; +} | null { + const left = getInlineReviewThreadState({ + reviewTarget: input.left, + reviewActions: input.reviewActions, + }); + const right = getInlineReviewThreadState({ + reviewTarget: input.right, + reviewActions: input.reviewActions, + }); + const height = Math.max(left?.height ?? 0, right?.height ?? 0); + if (height === 0) { + return null; + } + return { left, right, height }; +} + +export function InlineReviewGutterCell({ + children, + reviewTarget, + comments, + isEditorOpen, + showPersistentAction, + onStartComment, + style, +}: { + children: ReactNode; + reviewTarget: ReviewableDiffTarget | null | undefined; + comments: readonly ReviewDraftComment[]; + isEditorOpen: boolean; + showPersistentAction: boolean; + onStartComment: (target: ReviewableDiffTarget) => void; + style?: StyleProp; +}) { + const { theme } = useUnistyles(); + const canComment = Boolean(reviewTarget); + const hasComments = comments.length > 0; + + const handlePress = useCallback(() => { + if (reviewTarget) { + onStartComment(reviewTarget); + } + }, [reviewTarget, onStartComment]); + + const pressableStyle = useCallback( + ({ hovered, pressed }: PressableState): StyleProp => [ + style, + canComment && (hovered || pressed) && styles.gutterHovered, + ], + [style, canComment], + ); + + const labelStyle = useMemo>( + () => [styles.gutterLabel, hasComments && styles.gutterLabelActive], + [hasComments], + ); + + return ( + + {({ hovered, pressed }) => { + const showAction = + canComment && + (hovered || pressed || isNative || showPersistentAction || hasComments || isEditorOpen); + return ( + + + {showAction ? ( + + ) : ( + children + )} + + + ); + }} + + ); +} + +export function InlineReviewThread({ + reviewTarget, + reviewActions, + height, + viewportWidth, + pinToViewport = false, + testID, +}: { + reviewTarget: ReviewableDiffTarget; + reviewActions: InlineReviewActions; + height: number; + viewportWidth?: number; + pinToViewport?: boolean; + testID?: string; +}) { + const comments = reviewActions.commentsByTarget.get(reviewTarget.key) ?? []; + const editor = isInlineReviewEditorForTarget(reviewActions.editor, reviewTarget) + ? reviewActions.editor + : null; + const editingCommentId = editor?.commentId ?? null; + const editingExisting = + editingCommentId !== null && comments.some((comment) => comment.id === editingCommentId); + + const editorElement = editor ? ( + + ) : null; + + const containerStyle = useMemo>( + () => [ + styles.threadContainer, + getInlineReviewThreadViewportStyle({ viewportWidth, pinToViewport }), + { minHeight: height }, + ], + [viewportWidth, pinToViewport, height], + ); + + return ( + + {comments.map((comment) => { + if (comment.id === editingCommentId) { + return {editorElement}; + } + return ( + + ); + })} + {editor && !editingExisting ? editorElement : null} + + ); +} + +function CommentRow({ + comment, + reviewTarget, + onEditComment, + onDeleteComment, +}: { + comment: ReviewDraftComment; + reviewTarget: ReviewableDiffTarget; + onEditComment: (target: ReviewableDiffTarget, comment: ReviewDraftComment) => void; + onDeleteComment: (id: string) => void; +}) { + const { theme } = useUnistyles(); + + const handleEdit = useCallback( + () => onEditComment(reviewTarget, comment), + [onEditComment, reviewTarget, comment], + ); + + const handleDelete = useCallback( + () => onDeleteComment(comment.id), + [onDeleteComment, comment.id], + ); + + return ( + + + {comment.body} + + + + + + + + + + + ); +} + +export function getInlineReviewThreadViewportStyle({ + viewportWidth, + pinToViewport, +}: { + viewportWidth?: number; + pinToViewport: boolean; +}): StyleProp { + const widthStyle = viewportWidth && viewportWidth > 0 ? { width: viewportWidth } : null; + if (!pinToViewport || !isWeb) { + return widthStyle; + } + const stickyStyle = { position: "sticky", left: 0 } as unknown as ViewStyle; + return [stickyStyle, widthStyle]; +} + +export function InlineReviewEditor({ + initialBody, + onCancel, + onSave, + testID, +}: { + initialBody: string; + onCancel: () => void; + onSave: (body: string) => void; + testID?: string; +}) { + const { theme } = useUnistyles(); + const inputRef = useRef(null); + const [body, setBody] = useState(initialBody); + const [isFocused, setIsFocused] = useState(false); + const trimmedBody = body.trim(); + const canSave = trimmedBody.length > 0; + + useEffect(() => { + inputRef.current?.focus(); + }, []); + + const handleFocus = useCallback(() => setIsFocused(true), []); + const handleBlur = useCallback(() => setIsFocused(false), []); + const handleSave = useCallback(() => onSave(trimmedBody), [onSave, trimmedBody]); + + const inputStyle = useMemo>( + () => [styles.editorInput, isFocused && styles.editorInputFocused], + [isFocused], + ); + + const saveButtonStyle = useCallback( + ({ hovered, pressed }: PressableState): StyleProp => [ + styles.saveButton, + !canSave && styles.saveButtonDisabled, + canSave && (hovered || pressed) && styles.saveButtonHovered, + ], + [canSave], + ); + + return ( + + + + + Cancel + + + Save + + + + ); +} + +const styles = StyleSheet.create((theme) => ({ + gutterHovered: { + backgroundColor: theme.colors.surface2, + }, + gutterInner: { + minHeight: theme.lineHeight.diff, + alignItems: "center", + justifyContent: "center", + }, + gutterLabel: { + width: "100%", + minWidth: 0, + height: theme.lineHeight.diff, + alignItems: "center", + justifyContent: "center", + }, + gutterLabelActive: { + backgroundColor: theme.colors.surface2, + }, + threadContainer: { + flex: 1, + minWidth: 0, + gap: INLINE_REVIEW_GAP, + paddingVertical: theme.spacing[2], + paddingHorizontal: theme.spacing[3], + }, + commentBlock: { + backgroundColor: theme.colors.surface2, + borderWidth: theme.borderWidth[1], + borderColor: theme.colors.borderAccent, + borderRadius: theme.borderRadius.lg, + paddingHorizontal: theme.spacing[3], + paddingVertical: theme.spacing[2], + flexDirection: "row", + alignItems: "center", + gap: theme.spacing[2], + }, + commentBody: { + flex: 1, + minWidth: 0, + color: theme.colors.foreground, + fontSize: theme.fontSize.sm, + lineHeight: theme.fontSize.sm * 1.4, + }, + commentActions: { + flexDirection: "row", + alignItems: "center", + gap: theme.spacing[1], + flexShrink: 0, + }, + iconButton: { + width: 26, + height: 26, + alignItems: "center", + justifyContent: "center", + borderRadius: theme.borderRadius.full, + ...(isWeb + ? { + transitionProperty: "background-color", + transitionDuration: "120ms", + transitionTimingFunction: "ease-in-out", + } + : {}), + }, + iconButtonHovered: { + backgroundColor: theme.colors.surface3, + }, + iconButtonDestructiveHovered: { + backgroundColor: theme.colors.surface3, + }, + editorBlock: { + minHeight: INLINE_REVIEW_EDITOR_HEIGHT, + backgroundColor: theme.colors.surface2, + borderWidth: theme.borderWidth[1], + borderColor: theme.colors.borderAccent, + borderRadius: theme.borderRadius.lg, + paddingHorizontal: theme.spacing[3], + paddingVertical: theme.spacing[3], + gap: theme.spacing[3], + }, + editorInput: { + flex: 1, + minHeight: 0, + color: theme.colors.foreground, + backgroundColor: theme.colors.surface1, + borderWidth: theme.borderWidth[1], + borderColor: theme.colors.border, + borderRadius: theme.borderRadius.md, + paddingHorizontal: theme.spacing[3], + paddingVertical: theme.spacing[2], + fontSize: theme.fontSize.sm, + lineHeight: theme.fontSize.sm * 1.4, + textAlignVertical: "top", + ...(isWeb + ? { + outlineWidth: 0, + outlineColor: "transparent", + } + : {}), + }, + editorInputFocused: { + borderColor: theme.colors.accent, + }, + editorActions: { + flexDirection: "row", + justifyContent: "flex-end", + alignItems: "center", + gap: theme.spacing[2], + }, + ghostButton: { + height: 28, + alignItems: "center", + justifyContent: "center", + paddingHorizontal: theme.spacing[3], + borderRadius: theme.borderRadius.md, + }, + ghostButtonHovered: { + backgroundColor: theme.colors.surface3, + }, + ghostButtonText: { + color: theme.colors.foregroundMuted, + fontSize: theme.fontSize.xs, + fontWeight: theme.fontWeight.medium, + }, + saveButton: { + height: 28, + alignItems: "center", + justifyContent: "center", + paddingHorizontal: theme.spacing[3], + borderRadius: theme.borderRadius.md, + backgroundColor: theme.colors.accent, + }, + saveButtonHovered: { + opacity: 0.9, + }, + saveButtonDisabled: { + opacity: 0.4, + }, + saveButtonText: { + color: theme.colors.accentForeground, + fontSize: theme.fontSize.xs, + fontWeight: theme.fontWeight.semibold, + }, +})); diff --git a/packages/app/src/screens/new-workspace-screen.tsx b/packages/app/src/screens/new-workspace-screen.tsx index 3421c27ad..894a72c8a 100644 --- a/packages/app/src/screens/new-workspace-screen.tsx +++ b/packages/app/src/screens/new-workspace-screen.tsx @@ -26,7 +26,7 @@ import { useDraftStore } from "@/stores/draft-store"; import { useWorkspaceDraftSubmissionStore } from "@/stores/workspace-draft-submission-store"; import { toErrorMessage } from "@/utils/error-messages"; import { navigateToPreparedWorkspaceTab } from "@/utils/workspace-navigation"; -import type { ComposerAttachment } from "@/attachments/types"; +import type { ComposerAttachment, UserComposerAttachment } from "@/attachments/types"; import type { ImageAttachment, MessagePayload } from "@/components/message-input"; import type { AgentAttachment, GitHubSearchItem } from "@server/shared/messages"; import type { AgentProvider } from "@server/server/agent/agent-sdk-types"; @@ -195,10 +195,10 @@ function pickerItemTriggerLabel(item: PickerItem): string { } function syncPickerPrAttachment(input: { - attachments: ComposerAttachment[]; + attachments: UserComposerAttachment[]; previousPickerPrNumber: number | null; item: PickerItem; -}): { attachments: ComposerAttachment[]; attachedPrNumber: number | null } { +}): { attachments: UserComposerAttachment[]; attachedPrNumber: number | null } { let nextAttachments = input.attachments; let attachedPrNumber: number | null = null; @@ -373,7 +373,9 @@ function submitWorkspaceDraft(input: SubmitDraftInput): void { }), draft: { text, - attachments, + attachments: attachments.filter( + (attachment): attachment is UserComposerAttachment => attachment.kind !== "review", + ), cwd: workspaceDirectory, }, }); diff --git a/packages/app/src/screens/workspace/workspace-draft-agent-tab.test.tsx b/packages/app/src/screens/workspace/workspace-draft-agent-tab.test.tsx index 3f35a9d03..d090e4f20 100644 --- a/packages/app/src/screens/workspace/workspace-draft-agent-tab.test.tsx +++ b/packages/app/src/screens/workspace/workspace-draft-agent-tab.test.tsx @@ -15,12 +15,16 @@ import { WorkspaceDraftAgentTab } from "./workspace-draft-agent-tab"; const { createAgentMock, + getCheckoutStatusMock, + onRuntimeEventMock, latestComposerText, latestComposerDisabled, latestStreamText, onCreatedMock, } = vi.hoisted(() => ({ createAgentMock: vi.fn(), + getCheckoutStatusMock: vi.fn(), + onRuntimeEventMock: vi.fn(() => () => {}), latestComposerText: { current: null as string | null }, latestComposerDisabled: { current: null as boolean | null }, latestStreamText: { current: null as string | null }, @@ -43,6 +47,7 @@ vi.mock("react-native-unistyles", () => ({ }) : factory, }, + useUnistyles: () => ({ rt: { breakpoint: "lg" } }), })); vi.mock("react-native-safe-area-context", () => ({ @@ -57,6 +62,8 @@ vi.mock("@/constants/platform", () => ({ vi.mock("@/runtime/host-runtime", () => ({ useHostRuntimeClient: () => ({ createAgent: createAgentMock, + getCheckoutStatus: getCheckoutStatusMock, + on: onRuntimeEventMock, }), useHostRuntimeIsConnected: () => true, })); @@ -131,6 +138,16 @@ vi.mock("@/components/composer-attachments", () => ({ }), })); +vi.mock("@/attachments/composer-workspace-attachments", () => ({ + composerWorkspaceAttachment: { + userAttachmentsOnly: (attachments: ComposerAttachment[]) => + attachments.filter( + (attachment): attachment is Exclude => + attachment.kind !== "review", + ), + }, +})); + vi.mock("@/components/composer", () => ({ Composer: ({ value, @@ -196,6 +213,23 @@ beforeEach(() => { root = createRoot(container); queryClient = new QueryClient({ defaultOptions: { queries: { retry: false } } }); createAgentMock.mockReset(); + getCheckoutStatusMock.mockResolvedValue({ + cwd: "/repo/.paseo/worktrees/workspace-1", + error: null, + requestId: "checkout-status-1", + isGit: false, + isPaseoOwnedWorktree: true, + repoRoot: null, + currentBranch: null, + isDirty: false, + baseRef: null, + aheadBehind: null, + aheadOfOrigin: null, + behindOfOrigin: null, + hasRemote: false, + remoteUrl: null, + }); + onRuntimeEventMock.mockClear(); onCreatedMock.mockReset(); latestComposerText.current = null; latestComposerDisabled.current = null; diff --git a/packages/app/src/screens/workspace/workspace-draft-agent-tab.tsx b/packages/app/src/screens/workspace/workspace-draft-agent-tab.tsx index ade61b7ca..be647367f 100644 --- a/packages/app/src/screens/workspace/workspace-draft-agent-tab.tsx +++ b/packages/app/src/screens/workspace/workspace-draft-agent-tab.tsx @@ -6,12 +6,14 @@ import invariant from "tiny-invariant"; import { Composer } from "@/components/composer"; import { FileDropZone } from "@/components/file-drop-zone"; import { AgentStreamView } from "@/components/agent-stream-view"; +import { composerWorkspaceAttachment } from "@/attachments/composer-workspace-attachments"; import type { ImageAttachment } from "@/components/message-input"; import { useAgentInputDraft } from "@/hooks/use-agent-input-draft"; import { useDraftAgentCreateFlow } from "@/hooks/use-draft-agent-create-flow"; import { useHostRuntimeClient, useHostRuntimeIsConnected } from "@/runtime/host-runtime"; import { buildWorkspaceDraftAgentConfig } from "@/screens/workspace/workspace-draft-agent-config"; import { buildDraftStoreKey } from "@/stores/draft-keys"; +import { usePanelStore } from "@/stores/panel-store"; import type { Agent } from "@/stores/session-store"; import { useWorkspaceExecutionAuthority } from "@/stores/session-store-hooks"; import { useWorkspaceDraftSubmissionStore } from "@/stores/workspace-draft-submission-store"; @@ -20,8 +22,13 @@ import { shouldAutoFocusWorkspaceDraftComposer } from "@/screens/workspace/works import type { AgentCapabilityFlags } from "@server/server/agent/agent-sdk-types"; import type { AgentSnapshotPayload } from "@server/shared/messages"; import type { DaemonClient } from "@server/client/daemon-client"; -import type { ComposerAttachment } from "@/attachments/types"; +import type { ComposerAttachment, WorkspaceComposerAttachment } from "@/attachments/types"; +import { + useWorkspaceAttachments, + useWorkspaceAttachmentScopeKey, +} from "@/attachments/workspace-attachments-store"; import type { UserMessageImageAttachment } from "@/types/stream"; +import { useIsCompactFormFactor } from "@/constants/layout"; import { isWeb } from "@/constants/platform"; const EMPTY_PENDING_PERMISSIONS = new Map(); @@ -324,6 +331,33 @@ export function WorkspaceDraftAgentTab({ ); const autoSubmitConfig = resolveAutoSubmitConfig(pendingAutoSubmit); const allowsEmptyAutoSubmit = pendingAutoSubmit?.allowEmptyText === true; + const isCompact = useIsCompactFormFactor(); + const workspaceAttachmentScopeKey = useWorkspaceAttachmentScopeKey({ + serverId, + cwd: draftInput.cwd, + workspaceId, + }); + const workspaceAttachments = useWorkspaceAttachments(workspaceAttachmentScopeKey); + const openFileExplorerForCheckout = usePanelStore((state) => state.openFileExplorerForCheckout); + const setExplorerTabForCheckout = usePanelStore((state) => state.setExplorerTabForCheckout); + const handleOpenWorkspaceAttachment = useCallback( + (attachment: WorkspaceComposerAttachment) => { + const checkout = { + serverId, + cwd: attachment.attachment.cwd, + isGit: true, + }; + openFileExplorerForCheckout({ + checkout, + isCompact, + }); + setExplorerTabForCheckout({ + ...checkout, + tab: "changes", + }); + }, + [isCompact, openFileExplorerForCheckout, serverId, setExplorerTabForCheckout], + ); const { formErrorMessage, @@ -408,7 +442,7 @@ export function WorkspaceDraftAgentTab({ cwd: submission.cwd, }).catch(() => { setDraftText(submission.text); - setDraftAttachments(submission.attachments); + setDraftAttachments(composerWorkspaceAttachment.userAttachmentsOnly(submission.attachments)); autoSubmitKeyRef.current = null; }); }, [ @@ -562,6 +596,8 @@ export function WorkspaceDraftAgentTab({ value={draftInput.text} onChangeText={draftInput.setText} attachments={draftInput.attachments} + workspaceAttachments={workspaceAttachments} + onOpenWorkspaceAttachment={handleOpenWorkspaceAttachment} onChangeAttachments={draftInput.setAttachments} cwd={draftInput.cwd} clearDraft={draftInput.clear} diff --git a/packages/app/src/stores/create-flow-store.ts b/packages/app/src/stores/create-flow-store.ts index a1f7a9ed0..84cad6e49 100644 --- a/packages/app/src/stores/create-flow-store.ts +++ b/packages/app/src/stores/create-flow-store.ts @@ -1,5 +1,6 @@ import { create } from "zustand"; import type { UserMessageImageAttachment } from "@/types/stream"; +import type { AgentAttachment } from "@server/shared/messages"; export type CreateFlowLifecycleState = "active" | "abandoned" | "sent"; @@ -12,6 +13,7 @@ interface PendingCreateAttempt { timestamp: number; lifecycle: CreateFlowLifecycleState; images?: UserMessageImageAttachment[]; + attachments?: AgentAttachment[]; } interface CreateFlowState { diff --git a/packages/app/src/stores/draft-store.migration.test.ts b/packages/app/src/stores/draft-store.migration.test.ts index 4770ae77c..dc7abf507 100644 --- a/packages/app/src/stores/draft-store.migration.test.ts +++ b/packages/app/src/stores/draft-store.migration.test.ts @@ -1,8 +1,53 @@ import "@/test/window-local-storage"; -import { describe, expect, it } from "vitest"; -import { __draftStoreTestUtils } from "./draft-store"; +import { beforeEach, describe, expect, it } from "vitest"; +import type { ComposerAttachment } from "@/attachments/types"; +import { __draftStoreTestUtils, useDraftStore } from "./draft-store"; + +function workspaceReviewAttachment(): Extract { + return { + kind: "review", + reviewDraftKey: "review:key", + commentCount: 1, + attachment: { + type: "review", + mimeType: "application/paseo-review", + cwd: "/repo", + mode: "uncommitted", + baseRef: null, + comments: [ + { + filePath: "src/example.ts", + side: "new", + lineNumber: 41, + body: "Please simplify this.", + context: { + hunkHeader: "@@ -40,1 +40,1 @@", + targetLine: { + oldLineNumber: null, + newLineNumber: 41, + type: "add", + content: "const value = newValue;", + }, + lines: [ + { + oldLineNumber: null, + newLineNumber: 41, + type: "add", + content: "const value = newValue;", + }, + ], + }, + }, + ], + }, + }; +} describe("draft-store migration", () => { + beforeEach(() => { + useDraftStore.setState({ drafts: {}, createModalDraft: null }); + }); + it("normalizes legacy image metadata into image attachments and strips persisted preview URLs", async () => { const migrated = await __draftStoreTestUtils.migratePersistedState({ drafts: { @@ -79,4 +124,24 @@ describe("draft-store migration", () => { expect(twice).toEqual(once); }); + + it("rejects workspace review attachments from migrated draft attachments", async () => { + const migrated = await __draftStoreTestUtils.migratePersistedState({ + drafts: { + "agent:server:agent": { + input: { + text: "hello", + attachments: [workspaceReviewAttachment()], + cwd: "/repo", + }, + lifecycle: "active", + updatedAt: 1700000000001, + version: 2, + }, + }, + createModalDraft: null, + }); + + expect(migrated.drafts["agent:server:agent"]?.input.attachments).toEqual([]); + }); }); diff --git a/packages/app/src/stores/draft-store.ts b/packages/app/src/stores/draft-store.ts index f6b96518a..5520ed4c6 100644 --- a/packages/app/src/stores/draft-store.ts +++ b/packages/app/src/stores/draft-store.ts @@ -1,7 +1,7 @@ import { create } from "zustand"; import { createJSONStorage, persist } from "zustand/middleware"; import AsyncStorage from "@react-native-async-storage/async-storage"; -import type { AttachmentMetadata, ComposerAttachment } from "@/attachments/types"; +import type { AttachmentMetadata, UserComposerAttachment } from "@/attachments/types"; import { GitHubSearchItemSchema } from "@server/shared/messages"; import { garbageCollectAttachments, @@ -23,7 +23,7 @@ type PersistedDraftImage = AttachmentMetadata | LegacyDraftImage; export interface DraftInput { text: string; - attachments: ComposerAttachment[]; + attachments: UserComposerAttachment[]; cwd: string; } @@ -135,7 +135,7 @@ function normalizePersistedImage(value: unknown): PersistedDraftImage | null { return null; } -function isComposerAttachment(value: unknown): value is ComposerAttachment { +function isUserComposerAttachment(value: unknown): value is UserComposerAttachment { if (!value || typeof value !== "object") { return false; } @@ -150,7 +150,7 @@ function isComposerAttachment(value: unknown): value is ComposerAttachment { return GitHubSearchItemSchema.safeParse(record.item).success; } -function normalizeComposerAttachment(attachment: ComposerAttachment): ComposerAttachment { +function normalizeComposerAttachment(attachment: UserComposerAttachment): UserComposerAttachment { if (attachment.kind === "image") { return { kind: "image", @@ -160,14 +160,16 @@ function normalizeComposerAttachment(attachment: ComposerAttachment): ComposerAt return attachment; } -function normalizePersistedComposerAttachment(value: unknown): ComposerAttachment | null { - if (!isComposerAttachment(value)) { +function normalizePersistedComposerAttachment(value: unknown): UserComposerAttachment | null { + if (!isUserComposerAttachment(value)) { return null; } return normalizeComposerAttachment(value); } -function legacyImagesToAttachments(images: readonly AttachmentMetadata[]): ComposerAttachment[] { +function legacyImagesToAttachments( + images: readonly AttachmentMetadata[], +): UserComposerAttachment[] { return images.map((metadata) => ({ kind: "image", metadata, @@ -182,7 +184,7 @@ function isCanonicalDraftInput(value: unknown): value is CanonicalDraftInput { return ( typeof input.text === "string" && Array.isArray(input.attachments) && - input.attachments.every(isComposerAttachment) && + input.attachments.every(isUserComposerAttachment) && typeof input.cwd === "string" ); } @@ -426,7 +428,7 @@ async function migrateDraftInput(input: { const attachments = Array.isArray(rawInput.attachments) ? rawInput.attachments .map((entry) => normalizePersistedComposerAttachment(entry)) - .filter((entry): entry is ComposerAttachment => entry !== null) + .filter((entry): entry is UserComposerAttachment => entry !== null) : []; const legacyImages = Array.isArray(rawInput.images) ? rawInput.images diff --git a/packages/app/src/types/stream.ts b/packages/app/src/types/stream.ts index 0dfc9d215..abc861aca 100644 --- a/packages/app/src/types/stream.ts +++ b/packages/app/src/types/stream.ts @@ -1,5 +1,5 @@ import type { AgentProvider, ToolCallDetail } from "@server/server/agent/agent-sdk-types"; -import type { AgentStreamEventPayload } from "@server/shared/messages"; +import type { AgentAttachment, AgentStreamEventPayload } from "@server/shared/messages"; import type { AttachmentMetadata } from "@/attachments/types"; import { extractTaskEntriesFromToolCall } from "../utils/tool-call-parsers"; import { splitMarkdownBlocks } from "@/utils/split-markdown-blocks"; @@ -60,6 +60,7 @@ export interface UserMessageItem { text: string; timestamp: Date; images?: UserMessageImageAttachment[]; + attachments?: AgentAttachment[]; } export interface AssistantMessageItem { diff --git a/packages/app/src/utils/diff-layout.test.ts b/packages/app/src/utils/diff-layout.test.ts index a6f00f0cc..6a2adf01e 100644 --- a/packages/app/src/utils/diff-layout.test.ts +++ b/packages/app/src/utils/diff-layout.test.ts @@ -1,8 +1,18 @@ import { describe, expect, it } from "vitest"; -import { buildSplitDiffRows, buildUnifiedDiffLines } from "./diff-layout"; +import { + buildReviewableDiffTargetKey, + buildSplitDiffRows, + buildUnifiedDiffLines, +} from "./diff-layout"; import type { ParsedDiffFile } from "@/hooks/use-checkout-diff-query"; +import type { ReviewableDiffTarget } from "./diff-layout"; -function makeFile(lines: ParsedDiffFile["hunks"][number]["lines"]): ParsedDiffFile { +function makeFile( + lines: ParsedDiffFile["hunks"][number]["lines"], + options: { oldStart?: number; newStart?: number } = {}, +): ParsedDiffFile { + const oldStart = options.oldStart ?? 10; + const newStart = options.newStart ?? 10; return { path: "example.ts", isNew: false, @@ -12,9 +22,9 @@ function makeFile(lines: ParsedDiffFile["hunks"][number]["lines"]): ParsedDiffFi status: "ok", hunks: [ { - oldStart: 10, + oldStart, oldCount: 4, - newStart: 10, + newStart, newCount: 5, lines, }, @@ -22,7 +32,39 @@ function makeFile(lines: ParsedDiffFile["hunks"][number]["lines"]): ParsedDiffFi }; } +function expectReviewTarget( + target: ReviewableDiffTarget | null | undefined, + expected: Omit, +) { + expect(target).toMatchObject({ + filePath: "example.ts", + ...expected, + }); + expect(target?.key).toBe( + buildReviewableDiffTargetKey({ + filePath: "example.ts", + side: expected.side, + lineNumber: expected.lineNumber, + }), + ); +} + describe("buildSplitDiffRows", () => { + it("uses one canonical persisted key for rendered review targets", () => { + const rows = buildSplitDiffRows( + makeFile([ + { type: "header", content: "@@ -10,1 +20,1 @@" }, + { type: "context", content: "same line" }, + ]), + ); + + if (rows[1]?.kind !== "pair") { + throw new Error("Expected split pair row"); + } + expect(rows[1].left?.reviewTarget?.key).toBe("example.ts:old:10"); + expect(rows[1].right?.reviewTarget?.key).toBe("example.ts:new:10"); + }); + it("pairs replacement runs by index", () => { const rows = buildSplitDiffRows( makeFile([ @@ -78,6 +120,86 @@ describe("buildSplitDiffRows", () => { right: { type: "context", content: "same line", lineNumber: 10 }, }); }); + + it("emits old and new review targets for split context cells", () => { + const rows = buildSplitDiffRows( + makeFile( + [ + { type: "header", content: "@@ -10,1 +20,1 @@" }, + { type: "context", content: "same line" }, + ], + { newStart: 20 }, + ), + ); + + expect(rows[0]).toEqual({ + kind: "header", + content: "@@ -10,1 +20,1 @@", + }); + expect(rows[1]).toMatchObject({ + kind: "pair", + left: { type: "context", content: "same line", lineNumber: 10 }, + right: { type: "context", content: "same line", lineNumber: 20 }, + }); + if (rows[1]?.kind !== "pair") { + throw new Error("Expected split pair row"); + } + expectReviewTarget(rows[1].left?.reviewTarget, { + hunkHeader: "@@ -10,1 +20,1 @@", + hunkIndex: 0, + lineIndex: 1, + oldLineNumber: 10, + newLineNumber: 20, + side: "old", + lineNumber: 10, + lineType: "context", + content: "same line", + }); + expectReviewTarget(rows[1].right?.reviewTarget, { + hunkHeader: "@@ -10,1 +20,1 @@", + hunkIndex: 0, + lineIndex: 1, + oldLineNumber: 10, + newLineNumber: 20, + side: "new", + lineNumber: 20, + lineType: "context", + content: "same line", + }); + }); + + it("does not emit review targets for split headers or empty cells", () => { + const rows = buildSplitDiffRows( + makeFile([ + { type: "header", content: "@@ -10,1 +10,2 @@" }, + { type: "add", content: "after" }, + ]), + ); + + expect(rows[0]).toEqual({ + kind: "header", + content: "@@ -10,1 +10,2 @@", + }); + expect(rows[1]).toMatchObject({ + kind: "pair", + left: null, + right: { type: "add", content: "after", lineNumber: 10 }, + }); + if (rows[1]?.kind !== "pair") { + throw new Error("Expected split pair row"); + } + expectReviewTarget(rows[1].right?.reviewTarget, { + hunkHeader: "@@ -10,1 +10,2 @@", + hunkIndex: 0, + lineIndex: 1, + oldLineNumber: null, + newLineNumber: 10, + side: "new", + lineNumber: 10, + lineType: "add", + content: "after", + }); + }); }); describe("buildUnifiedDiffLines", () => { @@ -151,4 +273,53 @@ describe("buildUnifiedDiffLines", () => { expect(lines[5]?.lineNumber).toBe(166); expect(lines[6]?.lineNumber).toBe(167); }); + + it("emits canonical review targets for unified add, remove, and context lines", () => { + const lines = buildUnifiedDiffLines( + makeFile( + [ + { type: "header", content: "@@ -10,2 +20,2 @@" }, + { type: "context", content: "before" }, + { type: "remove", content: "removed" }, + { type: "add", content: "inserted" }, + ], + { newStart: 20 }, + ), + ); + + expect(lines[0]?.reviewTarget).toBeNull(); + expectReviewTarget(lines[1]?.reviewTarget, { + hunkHeader: "@@ -10,2 +20,2 @@", + hunkIndex: 0, + lineIndex: 1, + oldLineNumber: 10, + newLineNumber: 20, + side: "new", + lineNumber: 20, + lineType: "context", + content: "before", + }); + expectReviewTarget(lines[2]?.reviewTarget, { + hunkHeader: "@@ -10,2 +20,2 @@", + hunkIndex: 0, + lineIndex: 2, + oldLineNumber: 11, + newLineNumber: null, + side: "old", + lineNumber: 11, + lineType: "remove", + content: "removed", + }); + expectReviewTarget(lines[3]?.reviewTarget, { + hunkHeader: "@@ -10,2 +20,2 @@", + hunkIndex: 0, + lineIndex: 3, + oldLineNumber: null, + newLineNumber: 21, + side: "new", + lineNumber: 21, + lineType: "add", + content: "inserted", + }); + }); }); diff --git a/packages/app/src/utils/diff-layout.ts b/packages/app/src/utils/diff-layout.ts index e6d9a2473..7f5680cb2 100644 --- a/packages/app/src/utils/diff-layout.ts +++ b/packages/app/src/utils/diff-layout.ts @@ -1,16 +1,69 @@ import type { DiffLine, ParsedDiffFile } from "@/hooks/use-checkout-diff-query"; +type ReviewSide = "old" | "new"; +type ReviewableLineType = "add" | "remove" | "context"; + +export interface ReviewableDiffTargetKeyInput { + filePath: string; + side: ReviewSide; + lineNumber: number; +} + +export interface ReviewableDiffTarget { + key: string; + filePath: string; + hunkHeader: string; + hunkIndex: number; + lineIndex: number; + oldLineNumber: number | null; + newLineNumber: number | null; + side: ReviewSide; + lineNumber: number; + lineType: ReviewableLineType; + content: string; +} + +export function buildReviewableDiffTargetKey(input: ReviewableDiffTargetKeyInput): string { + return `${input.filePath}:${input.side}:${input.lineNumber}`; +} + +export interface NumberedDiffCell extends ReviewableDiffTarget { + line: DiffLine; +} + +export interface NumberedDiffLine { + key: string; + filePath: string; + hunkHeader: string; + hunkIndex: number; + lineIndex: number; + line: DiffLine; + oldLineNumber: number | null; + newLineNumber: number | null; + unifiedCell: NumberedDiffCell | null; + oldCell: NumberedDiffCell | null; + newCell: NumberedDiffCell | null; +} + +export interface NumberedDiffHunk { + hunkIndex: number; + hunkHeader: string; + lines: NumberedDiffLine[]; +} + export interface SplitDiffDisplayLine { type: DiffLine["type"]; content: string; tokens?: DiffLine["tokens"]; lineNumber: number | null; + reviewTarget: ReviewableDiffTarget | null; } export interface UnifiedDiffDisplayLine { key: string; line: DiffLine; lineNumber: number | null; + reviewTarget: ReviewableDiffTarget | null; } export type SplitDiffRow = @@ -24,95 +77,175 @@ export type SplitDiffRow = right: SplitDiffDisplayLine | null; }; -function toDisplayLine(input: { - line: DiffLine; - oldLineNumber: number | null; - newLineNumber: number | null; - side: "left" | "right"; -}): SplitDiffDisplayLine | null { - const { line, oldLineNumber, newLineNumber, side } = input; - if (line.type === "header") { +function toSplitDisplayLine(cell: NumberedDiffCell | null): SplitDiffDisplayLine | null { + if (!cell) { return null; } - if (line.type === "remove") { - if (side !== "left") { - return null; - } - return { - type: "remove", - content: line.content, - tokens: line.tokens, - lineNumber: oldLineNumber, - }; - } - - if (line.type === "add") { - if (side !== "right") { - return null; - } - return { - type: "add", - content: line.content, - tokens: line.tokens, - lineNumber: newLineNumber, - }; - } - return { - type: "context", - content: line.content, - tokens: line.tokens, - lineNumber: side === "left" ? oldLineNumber : newLineNumber, + type: cell.lineType, + content: cell.content, + ...(cell.line.tokens ? { tokens: cell.line.tokens } : {}), + lineNumber: cell.lineNumber, + reviewTarget: toReviewTarget(cell), }; } -export function buildUnifiedDiffLines(file: ParsedDiffFile): UnifiedDiffDisplayLine[] { - const lines: UnifiedDiffDisplayLine[] = []; +function toReviewTarget(cell: NumberedDiffCell): ReviewableDiffTarget { + return { + key: cell.key, + filePath: cell.filePath, + hunkHeader: cell.hunkHeader, + hunkIndex: cell.hunkIndex, + lineIndex: cell.lineIndex, + oldLineNumber: cell.oldLineNumber, + newLineNumber: cell.newLineNumber, + side: cell.side, + lineNumber: cell.lineNumber, + lineType: cell.lineType, + content: cell.content, + }; +} +function getHunkHeader(hunk: ParsedDiffFile["hunks"][number]): string { + const headerLine = hunk.lines.find((line) => line.type === "header"); + return headerLine?.content ?? "@@"; +} + +export function buildNumberedDiffHunks(file: ParsedDiffFile): NumberedDiffHunk[] { + const numberedHunks: NumberedDiffHunk[] = []; for (const [hunkIndex, hunk] of file.hunks.entries()) { let oldLineNo = hunk.oldStart; let newLineNo = hunk.newStart; + const hunkHeader = getHunkHeader(hunk); + const lines: NumberedDiffLine[] = []; for (const [lineIndex, line] of hunk.lines.entries()) { - let lineNumber: number | null = null; + let oldLineNumber: number | null = null; + let newLineNumber: number | null = null; if (line.type === "remove") { - lineNumber = oldLineNo; + oldLineNumber = oldLineNo; oldLineNo += 1; } else if (line.type === "add") { - lineNumber = newLineNo; + newLineNumber = newLineNo; newLineNo += 1; } else if (line.type === "context") { - lineNumber = newLineNo; + oldLineNumber = oldLineNo; + newLineNumber = newLineNo; oldLineNo += 1; newLineNo += 1; } + const oldCell = buildNumberedCell({ + filePath: file.path, + hunkHeader, + hunkIndex, + lineIndex, + line, + oldLineNumber, + newLineNumber, + side: "old", + }); + const newCell = buildNumberedCell({ + filePath: file.path, + hunkHeader, + hunkIndex, + lineIndex, + line, + oldLineNumber, + newLineNumber, + side: "new", + }); + lines.push({ key: `${hunkIndex}-${lineIndex}`, + filePath: file.path, + hunkHeader, + hunkIndex, + lineIndex, line, - lineNumber, + oldLineNumber, + newLineNumber, + unifiedCell: line.type === "remove" ? oldCell : newCell, + oldCell, + newCell, }); } + + numberedHunks.push({ hunkIndex, hunkHeader, lines }); } - return lines; + return numberedHunks; +} + +function buildNumberedCell(input: { + filePath: string; + hunkHeader: string; + hunkIndex: number; + lineIndex: number; + line: DiffLine; + oldLineNumber: number | null; + newLineNumber: number | null; + side: ReviewSide; +}): NumberedDiffCell | null { + if (input.line.type === "header") { + return null; + } + if (input.line.type === "remove" && input.side !== "old") { + return null; + } + if (input.line.type === "add" && input.side !== "new") { + return null; + } + + const lineNumber = input.side === "old" ? input.oldLineNumber : input.newLineNumber; + if (lineNumber === null) { + return null; + } + + return { + key: buildReviewableDiffTargetKey({ + filePath: input.filePath, + side: input.side, + lineNumber, + }), + filePath: input.filePath, + hunkHeader: input.hunkHeader, + hunkIndex: input.hunkIndex, + lineIndex: input.lineIndex, + oldLineNumber: input.oldLineNumber, + newLineNumber: input.newLineNumber, + side: input.side, + lineNumber, + lineType: input.line.type, + content: input.line.content, + line: input.line, + }; +} + +export function buildUnifiedDiffLines(file: ParsedDiffFile): UnifiedDiffDisplayLine[] { + return buildNumberedDiffHunks(file).flatMap((hunk) => + hunk.lines.map((numberedLine) => ({ + key: numberedLine.key, + line: numberedLine.line, + lineNumber: numberedLine.unifiedCell?.lineNumber ?? null, + reviewTarget: numberedLine.unifiedCell ? toReviewTarget(numberedLine.unifiedCell) : null, + })), + ); } export function buildSplitDiffRows(file: ParsedDiffFile): SplitDiffRow[] { const rows: SplitDiffRow[] = []; - for (const hunk of file.hunks) { - let oldLineNo = hunk.oldStart; - let newLineNo = hunk.newStart; + for (const hunk of buildNumberedDiffHunks(file)) { rows.push({ kind: "header", - content: hunk.lines[0]?.type === "header" ? hunk.lines[0].content : "@@", + content: hunk.hunkHeader, }); - let pendingRemovals: Array<{ line: DiffLine; oldLineNumber: number }> = []; - let pendingAdditions: Array<{ line: DiffLine; newLineNumber: number }> = []; + let pendingRemovals: NumberedDiffCell[] = []; + let pendingAdditions: NumberedDiffCell[] = []; const flushPendingRows = () => { const pairCount = Math.max(pendingRemovals.length, pendingAdditions.length); @@ -121,61 +254,41 @@ export function buildSplitDiffRows(file: ParsedDiffFile): SplitDiffRow[] { const addition = pendingAdditions[index] ?? null; rows.push({ kind: "pair", - left: removal - ? toDisplayLine({ - line: removal.line, - oldLineNumber: removal.oldLineNumber, - newLineNumber: null, - side: "left", - }) - : null, - right: addition - ? toDisplayLine({ - line: addition.line, - oldLineNumber: null, - newLineNumber: addition.newLineNumber, - side: "right", - }) - : null, + left: toSplitDisplayLine(removal), + right: toSplitDisplayLine(addition), }); } pendingRemovals = []; pendingAdditions = []; }; - for (const line of hunk.lines.slice(1)) { - if (line.type === "remove") { - pendingRemovals.push({ line, oldLineNumber: oldLineNo }); - oldLineNo += 1; + for (const numberedLine of hunk.lines) { + if (numberedLine.line.type === "header") { continue; } - if (line.type === "add") { - pendingAdditions.push({ line, newLineNumber: newLineNo }); - newLineNo += 1; + if (numberedLine.line.type === "remove") { + if (numberedLine.oldCell) { + pendingRemovals.push(numberedLine.oldCell); + } + continue; + } + + if (numberedLine.line.type === "add") { + if (numberedLine.newCell) { + pendingAdditions.push(numberedLine.newCell); + } continue; } flushPendingRows(); - if (line.type === "context") { + if (numberedLine.line.type === "context") { rows.push({ kind: "pair", - left: toDisplayLine({ - line, - oldLineNumber: oldLineNo, - newLineNumber: newLineNo, - side: "left", - }), - right: toDisplayLine({ - line, - oldLineNumber: oldLineNo, - newLineNumber: newLineNo, - side: "right", - }), + left: toSplitDisplayLine(numberedLine.oldCell), + right: toSplitDisplayLine(numberedLine.newCell), }); - oldLineNo += 1; - newLineNo += 1; } } diff --git a/packages/app/src/utils/pending-create-images.test.ts b/packages/app/src/utils/pending-create-images.test.ts index 77e87b5ba..ba84c1da2 100644 --- a/packages/app/src/utils/pending-create-images.test.ts +++ b/packages/app/src/utils/pending-create-images.test.ts @@ -1,6 +1,7 @@ import { describe, expect, it } from "vitest"; import type { StreamItem } from "@/types/stream"; import { mergePendingCreateImages } from "./pending-create-images"; +import type { AgentAttachment } from "@server/shared/messages"; function userMessage(params: { id: string; @@ -12,6 +13,7 @@ function userMessage(params: { mimeType: string; createdAt: number; }>; + attachments?: AgentAttachment[]; }): StreamItem { return { kind: "user_message", @@ -19,6 +21,7 @@ function userMessage(params: { text: params.text, timestamp: new Date("2026-01-01T00:00:00Z"), ...(params.images ? { images: params.images } : {}), + ...(params.attachments ? { attachments: params.attachments } : {}), }; } @@ -34,6 +37,16 @@ function buildImage(id: string) { ]; } +function buildReviewAttachment(): AgentAttachment { + return { + type: "review", + mimeType: "application/paseo-review", + cwd: "/repo", + mode: "base", + comments: [], + }; +} + describe("mergePendingCreateImages", () => { it("returns same reference when pending images are absent", () => { const streamItems = [userMessage({ id: "msg-1", text: "hello" })]; @@ -63,6 +76,24 @@ describe("mergePendingCreateImages", () => { expect(updated.images).toEqual(images); }); + it("merges attachments by clientMessageId when matched message has none", () => { + const streamItems = [userMessage({ id: "msg-1", text: "hello" })]; + const attachments = [buildReviewAttachment()]; + const result = mergePendingCreateImages({ + streamItems, + clientMessageId: "msg-1", + attachments, + }); + + expect(result).not.toBe(streamItems); + const updated = result[0]; + expect(updated?.kind).toBe("user_message"); + if (updated?.kind !== "user_message") { + throw new Error("Expected user_message item"); + } + expect(updated.attachments).toEqual(attachments); + }); + it("does not merge when clientMessageId does not match", () => { const streamItems = [userMessage({ id: "msg-1", text: "same text" })]; const images = buildImage("image-2"); @@ -92,4 +123,32 @@ describe("mergePendingCreateImages", () => { } expect(unchanged.images).toEqual(existingImages); }); + + it("does not overwrite existing user message attachments", () => { + const existingAttachments = [buildReviewAttachment()]; + const streamItems = [ + userMessage({ id: "msg-1", text: "hello", attachments: existingAttachments }), + ]; + const result = mergePendingCreateImages({ + streamItems, + clientMessageId: "msg-1", + attachments: [ + { + type: "github_issue", + mimeType: "application/github-issue", + number: 7, + title: "Issue", + url: "https://github.com/getpaseo/paseo/issues/7", + }, + ], + }); + + expect(result).toBe(streamItems); + const unchanged = result[0]; + expect(unchanged?.kind).toBe("user_message"); + if (unchanged?.kind !== "user_message") { + throw new Error("Expected user_message item"); + } + expect(unchanged.attachments).toEqual(existingAttachments); + }); }); diff --git a/packages/app/src/utils/pending-create-images.ts b/packages/app/src/utils/pending-create-images.ts index c84f1ff42..ea700f754 100644 --- a/packages/app/src/utils/pending-create-images.ts +++ b/packages/app/src/utils/pending-create-images.ts @@ -1,17 +1,22 @@ import type { StreamItem, UserMessageImageAttachment } from "@/types/stream"; +import type { AgentAttachment } from "@server/shared/messages"; interface MergePendingCreateImagesParams { streamItems: StreamItem[]; clientMessageId: string; images?: UserMessageImageAttachment[]; + attachments?: AgentAttachment[]; } export function mergePendingCreateImages({ streamItems, clientMessageId, images, + attachments, }: MergePendingCreateImagesParams): StreamItem[] { - if (!images || images.length === 0) { + const hasPendingImages = Boolean(images && images.length > 0); + const hasPendingAttachments = Boolean(attachments && attachments.length > 0); + if (!hasPendingImages && !hasPendingAttachments) { return streamItems; } @@ -26,11 +31,18 @@ export function mergePendingCreateImages({ if (target.kind !== "user_message") { return streamItems; } - if (target.images && target.images.length > 0) { + const shouldMergeImages = hasPendingImages && (!target.images || target.images.length === 0); + const shouldMergeAttachments = + hasPendingAttachments && (!target.attachments || target.attachments.length === 0); + if (!shouldMergeImages && !shouldMergeAttachments) { return streamItems; } const next = [...streamItems]; - next[targetIndex] = { ...target, images }; + next[targetIndex] = { + ...target, + ...(shouldMergeImages ? { images } : {}), + ...(shouldMergeAttachments ? { attachments } : {}), + }; return next; } diff --git a/packages/server/src/server/agent/prompt-attachments.test.ts b/packages/server/src/server/agent/prompt-attachments.test.ts index a76c60392..95e1f16e0 100644 --- a/packages/server/src/server/agent/prompt-attachments.test.ts +++ b/packages/server/src/server/agent/prompt-attachments.test.ts @@ -18,6 +18,73 @@ describe("prompt attachments", () => { ).toContain("GitHub PR #123: Fix race in worktree setup"); }); + it("renders review attachments with compact file, line, comment, and context details", () => { + expect( + renderPromptAttachmentAsText({ + type: "review", + mimeType: "application/paseo-review", + cwd: "/tmp/repo", + mode: "base", + baseRef: "main", + comments: [ + { + filePath: "src/index.ts", + side: "new", + lineNumber: 42, + body: "Please guard this nullable value.", + context: { + hunkHeader: "@@ -40,3 +40,4 @@", + targetLine: { + oldLineNumber: null, + newLineNumber: 42, + type: "add", + content: "const value = maybeNull.name;", + }, + lines: [ + { + oldLineNumber: 41, + newLineNumber: 41, + type: "context", + content: "const before = true;", + }, + { + oldLineNumber: null, + newLineNumber: 42, + type: "add", + content: "const value = maybeNull.name;", + }, + ], + }, + }, + ], + }), + ).toBe( + [ + "Paseo review attachment (base)", + "CWD: /tmp/repo", + "Base: main", + "", + "Comment 1: src/index.ts:new:42", + "Please guard this nullable value.", + "@@ -40,3 +40,4 @@", + " 41 41 const before = true;", + "> - 42 +const value = maybeNull.name;", + ].join("\n"), + ); + }); + + it("renders github_issue attachments as readable text", () => { + expect( + renderPromptAttachmentAsText({ + type: "github_issue", + mimeType: "application/github-issue", + number: 55, + title: "Issue", + url: "https://github.com/getpaseo/paseo/issues/55", + }), + ).toContain("GitHub Issue #55: Issue"); + }); + it("finds the first github_pr attachment", () => { expect( findGitHubPrAttachment([ diff --git a/packages/server/src/server/agent/prompt-attachments.ts b/packages/server/src/server/agent/prompt-attachments.ts index 07417ae08..8fe696d95 100644 --- a/packages/server/src/server/agent/prompt-attachments.ts +++ b/packages/server/src/server/agent/prompt-attachments.ts @@ -1,5 +1,7 @@ import type { AgentAttachment } from "../../shared/messages.js"; +const REVIEW_LINE_MARKERS = { add: "+", remove: "-", context: " " } as const; + export function renderPromptAttachmentAsText(attachment: AgentAttachment): string { switch (attachment.type) { case "github_pr": { @@ -22,9 +24,40 @@ export function renderPromptAttachmentAsText(attachment: AgentAttachment): strin } return lines.join("\n"); } + case "review": { + const lines = [`Paseo review attachment (${attachment.mode})`, `CWD: ${attachment.cwd}`]; + if (attachment.baseRef) { + lines.push(`Base: ${attachment.baseRef}`); + } + attachment.comments.forEach((comment, index) => { + lines.push( + "", + `Comment ${index + 1}: ${comment.filePath}:${comment.side}:${comment.lineNumber}`, + comment.body, + comment.context.hunkHeader, + ); + const target = comment.context.targetLine; + for (const line of comment.context.lines) { + const isTarget = + line.oldLineNumber === target.oldLineNumber && + line.newLineNumber === target.newLineNumber && + line.type === target.type && + line.content === target.content; + const prefix = isTarget ? "> " : " "; + const oldLn = padLineNumber(line.oldLineNumber); + const newLn = padLineNumber(line.newLineNumber); + lines.push(`${prefix}${oldLn} ${newLn} ${REVIEW_LINE_MARKERS[line.type]}${line.content}`); + } + }); + return lines.join("\n"); + } } } +function padLineNumber(lineNumber: number | null): string { + return (lineNumber?.toString() ?? "-").padStart(2); +} + export function findGitHubPrAttachment( attachments: readonly AgentAttachment[] | undefined, ): Extract | null { diff --git a/packages/server/src/server/agent/providers/acp-agent.ts b/packages/server/src/server/agent/providers/acp-agent.ts index 39144419b..4944cb9e6 100644 --- a/packages/server/src/server/agent/providers/acp-agent.ts +++ b/packages/server/src/server/agent/providers/acp-agent.ts @@ -2091,19 +2091,21 @@ function toACPContentBlocks(prompt: AgentPromptInput): ContentBlock[] { return [{ type: "text", text: prompt }]; } - return prompt.map((block: AgentPromptContentBlock) => { - if (block.type === "text") { - return { type: "text", text: block.text }; + const contentBlocks: ContentBlock[] = []; + for (const block of prompt) { + switch (block.type) { + case "text": + contentBlocks.push({ type: "text", text: block.text }); + break; + case "image": + contentBlocks.push({ type: "image", data: block.data, mimeType: block.mimeType }); + break; + default: + contentBlocks.push({ type: "text", text: renderPromptAttachmentAsText(block) }); + break; } - if (block.type === "github_pr" || block.type === "github_issue") { - return { type: "text", text: renderPromptAttachmentAsText(block) }; - } - return { - type: "image", - data: block.data, - mimeType: block.mimeType, - }; - }); + } + return contentBlocks; } function extractPromptText(prompt: AgentPromptInput): string { diff --git a/packages/server/src/server/agent/providers/claude-agent.ts b/packages/server/src/server/agent/providers/claude-agent.ts index 833838d82..8776881bd 100644 --- a/packages/server/src/server/agent/providers/claude-agent.ts +++ b/packages/server/src/server/agent/providers/claude-agent.ts @@ -2345,11 +2345,8 @@ class ClaudeAgentSession implements AgentSession { data: chunk.data, }, }); - } else if (chunk.type === "github_pr" || chunk.type === "github_issue") { - content.push({ - type: "text", - text: renderPromptAttachmentAsText(chunk), - }); + } else { + content.push({ type: "text", text: renderPromptAttachmentAsText(chunk) }); } } } else { diff --git a/packages/server/src/server/agent/providers/codex-app-server-agent.test.ts b/packages/server/src/server/agent/providers/codex-app-server-agent.test.ts index b35cb3d47..0580b2b0d 100644 --- a/packages/server/src/server/agent/providers/codex-app-server-agent.test.ts +++ b/packages/server/src/server/agent/providers/codex-app-server-agent.test.ts @@ -357,6 +357,52 @@ describe("Codex app-server provider", () => { ]); }); + test("separates Codex text prompts from rendered attachment text", async () => { + const input = await codexAppServerTurnInputFromPrompt( + [ + { type: "text", text: "Please review this" }, + { + type: "github_issue", + mimeType: "application/github-issue", + number: 456, + title: "Attachment spacing", + url: "https://github.com/getpaseo/paseo/issues/456", + }, + ], + logger, + ); + + expect(input).toEqual([ + { type: "text", text: "Please review this" }, + { + type: "text", + text: expect.stringMatching(/^\n\nGitHub Issue #456: Attachment spacing/), + }, + ]); + }); + + test("does not prefix Codex attachment-only prompts with a blank line", async () => { + const input = await codexAppServerTurnInputFromPrompt( + [ + { + type: "github_issue", + mimeType: "application/github-issue", + number: 456, + title: "Attachment spacing", + url: "https://github.com/getpaseo/paseo/issues/456", + }, + ], + logger, + ); + + expect(input).toEqual([ + { + type: "text", + text: expect.stringMatching(/^GitHub Issue #456: Attachment spacing/), + }, + ]); + }); + test("maps patch notifications with array-style changes and alias diff keys", () => { const item = __codexAppServerInternals.mapCodexPatchNotificationToToolCall({ callId: "patch-array-alias", diff --git a/packages/server/src/server/agent/providers/codex-app-server-agent.ts b/packages/server/src/server/agent/providers/codex-app-server-agent.ts index 3814785d0..1c64d705d 100644 --- a/packages/server/src/server/agent/providers/codex-app-server-agent.ts +++ b/packages/server/src/server/agent/providers/codex-app-server-agent.ts @@ -2410,41 +2410,36 @@ export async function codexAppServerTurnInputFromPrompt( return [{ type: "text", text: prompt }]; } - const blocks = prompt as Array; - const output = await Promise.all( - blocks.map(async (block) => { - if (!block || typeof block !== "object") { - return block; - } - const record = block as { type?: unknown; mimeType?: unknown; data?: unknown }; - if ( - record.type === "image" && - typeof record.mimeType === "string" && - typeof record.data === "string" - ) { - try { - const filePath = await writeImageAttachment(record.mimeType, record.data); - return { type: "localImage", path: filePath }; - } catch (error) { - const message = error instanceof Error ? error.message : String(error); - logger.warn({ message }, "Failed to write Codex image attachment"); - return { - type: "text", - text: `User attached image (failed to write temp file): ${message}`, - }; - } - } - if (record.type === "github_pr" || record.type === "github_issue") { - return { + const output: unknown[] = []; + let previousTextBlock = false; + for (const block of prompt) { + if (block.type === "text") { + output.push(block); + previousTextBlock = block.text.length > 0; + continue; + } + if (block.type === "image") { + try { + const filePath = await writeImageAttachment(block.mimeType, block.data); + output.push({ type: "localImage", path: filePath }); + } catch (error) { + const message = error instanceof Error ? error.message : String(error); + logger.warn({ message }, "Failed to write Codex image attachment"); + output.push({ type: "text", - text: renderPromptAttachmentAsText( - record as Extract, - ), - }; + text: `User attached image (failed to write temp file): ${message}`, + }); } - return block; - }), - ); + previousTextBlock = false; + continue; + } + const attachmentText = renderPromptAttachmentAsText(block); + output.push({ + type: "text", + text: previousTextBlock ? `\n\n${attachmentText}` : attachmentText, + }); + previousTextBlock = true; + } return output; } diff --git a/packages/server/src/server/agent/providers/opencode-agent.ts b/packages/server/src/server/agent/providers/opencode-agent.ts index 44ce051ba..96bb847c4 100644 --- a/packages/server/src/server/agent/providers/opencode-agent.ts +++ b/packages/server/src/server/agent/providers/opencode-agent.ts @@ -698,20 +698,20 @@ function buildOpenCodePromptParts( output.push({ type: "text", text: part.text }); continue; } - if (part.type === "github_pr" || part.type === "github_issue") { - output.push({ type: "text", text: renderPromptAttachmentAsText(part) }); + if (part.type === "image") { + attachmentOrdinal += 1; + const normalized = toOpenCodeDataUrl(part.mimeType, part.data); + output.push({ + type: "file", + mime: normalized.mimeType, + filename: `attachment-${attachmentOrdinal}.${getOpenCodeAttachmentExtension( + normalized.mimeType, + )}`, + url: normalized.url, + }); continue; } - attachmentOrdinal += 1; - const normalized = toOpenCodeDataUrl(part.mimeType, part.data); - output.push({ - type: "file", - mime: normalized.mimeType, - filename: `attachment-${attachmentOrdinal}.${getOpenCodeAttachmentExtension( - normalized.mimeType, - )}`, - url: normalized.url, - }); + output.push({ type: "text", text: renderPromptAttachmentAsText(part) }); } return output; } diff --git a/packages/server/src/shared/messages.attachments.test.ts b/packages/server/src/shared/messages.attachments.test.ts index 4f9d8cabd..32fa8c4eb 100644 --- a/packages/server/src/shared/messages.attachments.test.ts +++ b/packages/server/src/shared/messages.attachments.test.ts @@ -7,6 +7,148 @@ import { } from "./messages.js"; describe("shared messages attachments", () => { + it("keeps valid review attachments", () => { + const parsed = SendAgentMessageRequestSchema.parse({ + type: "send_agent_message_request", + requestId: "req-review", + agentId: "agent-1", + text: "Please address these comments", + attachments: [ + { + type: "review", + mimeType: "application/paseo-review", + cwd: "/tmp/repo", + mode: "base", + baseRef: "main", + comments: [ + { + filePath: "src/index.ts", + side: "new", + lineNumber: 42, + body: "Please guard this nullable value.", + context: { + hunkHeader: "@@ -40,3 +40,4 @@", + targetLine: { + oldLineNumber: null, + newLineNumber: 42, + type: "add", + content: "const value = maybeNull.name;", + }, + lines: [ + { + oldLineNumber: 41, + newLineNumber: 41, + type: "context", + content: "const before = true;", + }, + { + oldLineNumber: null, + newLineNumber: 42, + type: "add", + content: "const value = maybeNull.name;", + }, + ], + }, + }, + ], + }, + ], + }); + + expect(parsed.attachments).toEqual([ + { + type: "review", + mimeType: "application/paseo-review", + cwd: "/tmp/repo", + mode: "base", + baseRef: "main", + comments: [ + { + filePath: "src/index.ts", + side: "new", + lineNumber: 42, + body: "Please guard this nullable value.", + context: { + hunkHeader: "@@ -40,3 +40,4 @@", + targetLine: { + oldLineNumber: null, + newLineNumber: 42, + type: "add", + content: "const value = maybeNull.name;", + }, + lines: [ + { + oldLineNumber: 41, + newLineNumber: 41, + type: "context", + content: "const before = true;", + }, + { + oldLineNumber: null, + newLineNumber: 42, + type: "add", + content: "const value = maybeNull.name;", + }, + ], + }, + }, + ], + }, + ]); + }); + + it("drops malformed review attachments while keeping valid attachments", () => { + const parsed = SendAgentMessageRequestSchema.parse({ + type: "send_agent_message_request", + requestId: "req-bad-review", + agentId: "agent-1", + text: "Review", + attachments: [ + { + type: "review", + mimeType: "application/paseo-review", + cwd: "/tmp/repo", + mode: "uncommitted", + comments: [ + { + filePath: "src/index.ts", + side: "new", + lineNumber: "42", + body: "This line number is malformed.", + context: { + hunkHeader: "@@ -40,3 +40,4 @@", + targetLine: { + oldLineNumber: null, + newLineNumber: 42, + type: "add", + content: "const value = maybeNull.name;", + }, + lines: [], + }, + }, + ], + }, + { + type: "github_issue", + mimeType: "application/github-issue", + number: 55, + title: "Improve startup error details", + url: "https://github.com/getpaseo/paseo/issues/55", + }, + ], + }); + + expect(parsed.attachments).toEqual([ + { + type: "github_issue", + mimeType: "application/github-issue", + number: 55, + title: "Improve startup error details", + url: "https://github.com/getpaseo/paseo/issues/55", + }, + ]); + }); + it("keeps known attachments and drops unknown create-agent attachments", () => { const parsed = CreateAgentRequestMessageSchema.parse({ type: "create_agent_request", diff --git a/packages/server/src/shared/messages.ts b/packages/server/src/shared/messages.ts index 2995630a5..91a2f21d4 100644 --- a/packages/server/src/shared/messages.ts +++ b/packages/server/src/shared/messages.ts @@ -733,9 +733,38 @@ export const GitHubIssueAttachmentSchema = z.object({ body: z.string().nullable().optional(), }); +export const ReviewAttachmentContextLineSchema = z.object({ + oldLineNumber: z.number().int().positive().nullable(), + newLineNumber: z.number().int().positive().nullable(), + type: z.enum(["add", "remove", "context"]), + content: z.string(), +}); + +export const ReviewAttachmentCommentSchema = z.object({ + filePath: z.string(), + side: z.enum(["old", "new"]), + lineNumber: z.number().int().positive(), + body: z.string(), + context: z.object({ + hunkHeader: z.string(), + targetLine: ReviewAttachmentContextLineSchema, + lines: z.array(ReviewAttachmentContextLineSchema), + }), +}); + +export const ReviewAttachmentSchema = z.object({ + type: z.literal("review"), + mimeType: z.literal("application/paseo-review"), + cwd: z.string(), + mode: z.enum(["uncommitted", "base"]), + baseRef: z.string().nullable().optional(), + comments: z.array(ReviewAttachmentCommentSchema), +}); + export const AgentAttachmentSchema = z.discriminatedUnion("type", [ GitHubPrAttachmentSchema, GitHubIssueAttachmentSchema, + ReviewAttachmentSchema, ]); function normalizeAgentAttachments(input: unknown): AgentAttachment[] { @@ -3471,6 +3500,7 @@ export type DictationStreamFinishMessage = z.infer; export type CreateAgentRequestMessage = z.infer; export type AgentAttachment = z.infer; +export type ReviewAttachment = z.infer; export type ListProviderModelsRequestMessage = z.infer< typeof ListProviderModelsRequestMessageSchema >;