From dd2315536ee07e91b281c49270feb43dfb3b89c2 Mon Sep 17 00:00:00 2001 From: Michael Ramos Date: Sat, 4 Apr 2026 17:46:29 -0700 Subject: [PATCH 01/37] feat(review): add Codex AI review agent with live logs, --local worktree, and panel UI MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Hook up Codex as the first AI review agent in the code review system: - Spawn `codex exec` with Codex's native review prompt and output schema - Parse structured findings (ReviewOutputEvent) and push as external annotations - Annotations appear inline in the diff viewer pinned to specific lines - Review verdict (correct/incorrect + confidence + explanation) displayed in panel Agent job infrastructure enhancements: - Server-side command building via `buildCommand` callback (providers don't need frontend commands) - Result ingestion via `onJobComplete` callback (reads output file, transforms findings) - `addAnnotations` method on external annotation handler (bypasses HTTP for server-internal producers) - Live stderr streaming via `job:log` SSE events with 200ms buffer-and-flush - `cwd` and `summary` fields on AgentJobInfo PR review with --local worktree: - `plannotator review --local` creates a temp git worktree with the PR branch - Agent gets full local file access without touching the user's working tree - Hybrid server mode: both prMetadata (platform features) and gitContext (local access) - Automatic worktree cleanup on session end - Runtime-agnostic worktree primitives in packages/shared/worktree.ts Panel UI redesign: - Findings | Logs tab system with underline-style tabs - LiveLogViewer component with auto-scroll, truncation, and copy - Review verdict card (correct/incorrect with confidence and explanation) - Pending state with labeled "Review Verdict — Pending..." (not skeleton bars) - Job card click opens detail panel directly (removed separate icon button) - Dismissed annotation tracking (deleted annotations persist as "dismissed" in panel) - Copy all annotations as formatted markdown - Worktree badge in header with info dialog showing path - CopyButton extended with inline variant for reuse - ConfirmDialog extended with wide option For provenance purposes, this commit was AI assisted. --- apps/hook/server/index.ts | 78 ++- apps/pi-extension/vendor.sh | 2 +- packages/review-editor/App.tsx | 39 +- .../review-editor/components/CopyButton.tsx | 49 +- .../components/LiveLogViewer.tsx | 84 ++++ .../components/ReviewSidebar.tsx | 4 +- .../review-editor/dock/ReviewStateContext.tsx | 2 + .../dock/panels/ReviewAgentJobDetailPanel.tsx | 468 ++++++++++++------ packages/server/AGENT_LIVE_LOGS.md | 103 ++++ packages/server/PR_LOCAL_WORKTREE.md | 85 ++++ packages/server/agent-jobs.ts | 71 ++- packages/server/codex-review-schema.json | 41 ++ packages/server/codex-review.ts | 385 ++++++++++++++ packages/server/external-annotations.ts | 9 + packages/server/git.ts | 3 +- packages/server/review.ts | 98 +++- packages/server/vcs.ts | 2 +- packages/shared/agent-jobs.ts | 9 + packages/shared/package.json | 3 +- packages/shared/worktree.ts | 119 +++++ packages/ui/components/AgentsTab.tsx | 28 +- packages/ui/components/ConfirmDialog.tsx | 4 +- packages/ui/hooks/useAgentJobs.ts | 11 +- 23 files changed, 1470 insertions(+), 227 deletions(-) create mode 100644 packages/review-editor/components/LiveLogViewer.tsx create mode 100644 packages/server/AGENT_LIVE_LOGS.md create mode 100644 packages/server/PR_LOCAL_WORKTREE.md create mode 100644 packages/server/codex-review-schema.json create mode 100644 packages/server/codex-review.ts create mode 100644 packages/shared/worktree.ts diff --git a/apps/hook/server/index.ts b/apps/hook/server/index.ts index bee9b9ba..7b01f1ab 100644 --- a/apps/hook/server/index.ts +++ b/apps/hook/server/index.ts @@ -63,7 +63,8 @@ import { startAnnotateServer, handleAnnotateServerReady, } from "@plannotator/server/annotate"; -import { type DiffType, getVcsContext, runVcsDiff } from "@plannotator/server/vcs"; +import { type DiffType, getVcsContext, runVcsDiff, gitRuntime } from "@plannotator/server/vcs"; +import { fetchRef, createWorktree, removeWorktree, ensureObjectAvailable } from "@plannotator/shared/worktree"; import { parsePRUrl, checkPRAuth, fetchPR, getCliName, getCliInstallUrl, getMRLabel, getMRNumberLabel, getDisplayRepo } from "@plannotator/server/pr"; import { writeRemoteShareLink } from "@plannotator/server/share-url"; import { resolveMarkdownFile, hasMarkdownFiles } from "@plannotator/shared/resolve-file"; @@ -85,6 +86,7 @@ import { isTopLevelHelpInvocation, } from "./cli"; import path from "path"; +import { tmpdir } from "os"; // Embed the built HTML at compile time // @ts-ignore - Bun import attribute for text @@ -184,15 +186,26 @@ if (args[0] === "sessions") { // CODE REVIEW MODE // ============================================ + // Parse --local flag (strip before URL detection) + const localIdx = args.indexOf("--local"); + const useLocal = localIdx !== -1; + if (localIdx !== -1) args.splice(localIdx, 1); + const urlArg = args[1]; const isPRMode = urlArg?.startsWith("http://") || urlArg?.startsWith("https://"); + if (useLocal && !isPRMode) { + console.error("--local requires a PR/MR URL"); + process.exit(1); + } + let rawPatch: string; let gitRef: string; let diffError: string | undefined; let gitContext: Awaited> | undefined; let prMetadata: Awaited>["metadata"] | undefined; let initialDiffType: DiffType | undefined; + let worktreeCleanup: (() => void | Promise) | undefined; if (isPRMode) { // --- PR Review Mode --- @@ -231,6 +244,66 @@ if (args[0] === "sessions") { console.error(err instanceof Error ? err.message : "Failed to fetch PR"); process.exit(1); } + + // --local: create a temporary worktree with the PR head for local file access + if (useLocal && prMetadata) { + try { + const repoDir = process.cwd(); + let fetchRefStr: string; + let identifier: string; + + if (prMetadata.platform === "github") { + fetchRefStr = `refs/pull/${prMetadata.number}/head`; + identifier = `${prMetadata.owner}-${prMetadata.repo}-${prMetadata.number}`; + } else { + fetchRefStr = `refs/merge-requests/${prMetadata.iid}/head`; + identifier = `${prMetadata.projectPath.replace(/\//g, "-")}-${prMetadata.iid}`; + } + + console.error("Fetching PR branch and creating local worktree..."); + await fetchRef(gitRuntime, fetchRefStr, { cwd: repoDir }); + + // Ensure base SHA is available (needed for branch diff) + const baseAvailable = await ensureObjectAvailable(gitRuntime, prMetadata.baseSha, { cwd: repoDir }); + if (!baseAvailable) { + console.error(`Warning: base SHA ${prMetadata.baseSha.slice(0, 8)} not available locally — branch diff may be incomplete`); + } + + const suffix = Math.random().toString(36).slice(2, 8); + const worktreePath = path.join(tmpdir(), `plannotator-pr-${identifier}-${suffix}`); + + const { worktreePath: createdPath } = await createWorktree(gitRuntime, { + ref: "FETCH_HEAD", + path: worktreePath, + detach: true, + cwd: repoDir, + }); + + // Build git context from the worktree + gitContext = await getVcsContext(createdPath); + // Override default branch with the PR's actual base branch + gitContext.defaultBranch = prMetadata.baseBranch; + + // Compute diff locally (more accurate than platform diff) + initialDiffType = "branch"; + const localDiff = await runVcsDiff("branch", prMetadata.baseBranch, createdPath); + rawPatch = localDiff.patch; + gitRef = localDiff.label || gitRef; + + // Register cleanup + worktreeCleanup = () => removeWorktree(gitRuntime, createdPath, { force: true, cwd: repoDir }); + process.on("exit", () => { + try { Bun.spawnSync(["git", "worktree", "remove", "--force", createdPath]); } catch {} + }); + + console.error(`Local worktree created at ${createdPath}`); + } catch (err) { + console.error(`Warning: --local worktree creation failed, falling back to remote diff`); + console.error(err instanceof Error ? err.message : String(err)); + // gitContext remains undefined = pure PR mode fallback + worktreeCleanup = undefined; + } + } } else { // --- Local Review Mode --- gitContext = await getVcsContext(); @@ -249,12 +322,13 @@ if (args[0] === "sessions") { gitRef, error: diffError, origin: detectedOrigin, - diffType: isPRMode ? undefined : (initialDiffType ?? "uncommitted"), + diffType: gitContext ? (initialDiffType ?? "uncommitted") : undefined, gitContext, prMetadata, sharingEnabled, shareBaseUrl, htmlContent: reviewHtmlContent, + onCleanup: worktreeCleanup, onReady: async (url, isRemote, port) => { handleReviewServerReady(url, isRemote, port); diff --git a/apps/pi-extension/vendor.sh b/apps/pi-extension/vendor.sh index c383d592..3fb954ca 100755 --- a/apps/pi-extension/vendor.sh +++ b/apps/pi-extension/vendor.sh @@ -6,7 +6,7 @@ cd "$(dirname "$0")" mkdir -p generated generated/ai/providers -for f in feedback-templates review-core storage draft project pr-provider pr-github pr-gitlab checklist integrations-common repo reference-common favicon resolve-file config external-annotation agent-jobs; do +for f in feedback-templates review-core storage draft project pr-provider pr-github pr-gitlab checklist integrations-common repo reference-common favicon resolve-file config external-annotation agent-jobs worktree; do src="../../packages/shared/$f.ts" printf '// @generated — DO NOT EDIT. Source: packages/shared/%s.ts\n' "$f" | cat - "$src" > "generated/$f.ts" done diff --git a/packages/review-editor/App.tsx b/packages/review-editor/App.tsx index 7ac60cc4..5e015669 100644 --- a/packages/review-editor/App.tsx +++ b/packages/review-editor/App.tsx @@ -114,6 +114,7 @@ const ReviewApp: React.FC = () => { const [selectedAnnotationId, setSelectedAnnotationId] = useState(null); const [pendingSelection, setPendingSelection] = useState(null); const [showExportModal, setShowExportModal] = useState(false); + const [showWorktreeDialog, setShowWorktreeDialog] = useState(false); const [openSettingsMenu, setOpenSettingsMenu] = useState(false); const [showNoAnnotationsDialog, setShowNoAnnotationsDialog] = useState(false); const [isLoading, setIsLoading] = useState(true); @@ -965,6 +966,7 @@ const ReviewApp: React.FC = () => { onClickAIMarker: handleClickAIMarker, aiHistoryForSelection, agentJobs: agentJobs.jobs, + jobLogs: agentJobs.jobLogs, prMetadata, prContext, isPRContextLoading, @@ -983,7 +985,7 @@ const ReviewApp: React.FC = () => { activeFileSearchMatches, activeSearchMatchId, activeSearchMatch, aiAvailable, aiChat.messages, aiChat.isCreatingSession, aiChat.isStreaming, handleAskAI, handleViewAIResponse, handleClickAIMarker, - aiHistoryForSelection, agentJobs.jobs, prMetadata, prContext, + aiHistoryForSelection, agentJobs.jobs, agentJobs.jobLogs, prMetadata, prContext, isPRContextLoading, prContextError, fetchPRContext, openDiffFile, ]); @@ -1306,6 +1308,14 @@ const ReviewApp: React.FC = () => {
{prMetadata ? (
+ {prMetadata && gitContext && ( + + )} { />
+ {/* Worktree info dialog */} + {gitContext?.cwd && prMetadata && ( + setShowWorktreeDialog(false)} + title="Local Worktree" + wide + message={ +
+

This PR is checked out in a temporary local worktree for full file access.

+
+ Path + +
+

Automatically removed when this review session ends.

+
+ } + variant="info" + /> + )} + {/* No annotations dialog */} = ({ text, className = '' }) => { +const CopyIcon = () => ( + +); + +const CheckIcon = () => ( + +); + +/** Copy-to-clipboard button with "Copied" flash. */ +export const CopyButton: React.FC = ({ text, className = '', variant = 'overlay', label }) => { const [copied, setCopied] = useState(false); const handleCopy = async (e: React.MouseEvent) => { @@ -21,6 +38,24 @@ export const CopyButton: React.FC = ({ text, className = '' }) } }; + if (variant === 'inline') { + return ( + + ); + } + return ( ); }; diff --git a/packages/review-editor/components/LiveLogViewer.tsx b/packages/review-editor/components/LiveLogViewer.tsx new file mode 100644 index 00000000..13297ebe --- /dev/null +++ b/packages/review-editor/components/LiveLogViewer.tsx @@ -0,0 +1,84 @@ +import React, { useRef, useEffect, useMemo, useCallback } from 'react'; +import { CopyButton } from './CopyButton'; + +interface LiveLogViewerProps { + /** The full accumulated log text. */ + content: string; + /** Whether the source is still producing output. */ + isLive?: boolean; + /** Max bytes to render (trim from top). Default 50KB. */ + maxRenderSize?: number; + /** Optional className for the outer container. */ + className?: string; +} + +/** + * Reusable streaming log viewer with auto-scroll, truncation, and copy. + * + * Auto-scrolls to bottom as new content arrives — unless the user has + * scrolled up to read earlier output. Follows the AITab streaming pattern. + */ +export const LiveLogViewer: React.FC = ({ + content, + isLive = false, + maxRenderSize = 50_000, + className, +}) => { + const containerRef = useRef(null); + const isAtBottomRef = useRef(true); + + const handleScroll = useCallback(() => { + const el = containerRef.current; + if (!el) return; + isAtBottomRef.current = el.scrollHeight - el.scrollTop - el.clientHeight < 40; + }, []); + + // Auto-scroll on new content if user is at bottom + useEffect(() => { + if (isAtBottomRef.current && containerRef.current) { + containerRef.current.scrollTop = containerRef.current.scrollHeight; + } + }, [content]); + + const displayText = useMemo(() => { + if (content.length <= maxRenderSize) return content; + const sliceFrom = content.indexOf('\n', content.length - maxRenderSize); + return '[earlier output truncated]\n' + content.slice(sliceFrom === -1 ? content.length - maxRenderSize : sliceFrom + 1); + }, [content, maxRenderSize]); + + if (!content && !isLive) { + return ( +
+

No output captured.

+
+ ); + } + + return ( +
+
+ {!content && isLive ? ( + + Waiting for output... + + ) : ( +
+            {displayText}
+            {isLive && content && (
+              
+            )}
+          
+ )} +
+ {content && ( +
+ +
+ )} +
+ ); +}; diff --git a/packages/review-editor/components/ReviewSidebar.tsx b/packages/review-editor/components/ReviewSidebar.tsx index 55bd5047..a08e5353 100644 --- a/packages/review-editor/components/ReviewSidebar.tsx +++ b/packages/review-editor/components/ReviewSidebar.tsx @@ -17,9 +17,7 @@ import type { DiffFile } from '../types'; type ReviewSidebarTab = 'annotations' | 'ai' | 'agents'; -// Temporary hard kill-switch for review agents in the sidebar. -// Keep local and explicit until the feature is ready to expose again. -const REVIEW_AGENTS_ENABLED = false; +const REVIEW_AGENTS_ENABLED = true; interface ReviewSidebarProps { isOpen: boolean; diff --git a/packages/review-editor/dock/ReviewStateContext.tsx b/packages/review-editor/dock/ReviewStateContext.tsx index 3fe6e797..b173455a 100644 --- a/packages/review-editor/dock/ReviewStateContext.tsx +++ b/packages/review-editor/dock/ReviewStateContext.tsx @@ -67,6 +67,8 @@ export interface ReviewState { // Agent jobs agentJobs: AgentJobInfo[]; + /** Live stderr logs per job ID (accumulated deltas). */ + jobLogs: Map; // PR prMetadata: PRMetadata | null; diff --git a/packages/review-editor/dock/panels/ReviewAgentJobDetailPanel.tsx b/packages/review-editor/dock/panels/ReviewAgentJobDetailPanel.tsx index 4324af56..da734ba8 100644 --- a/packages/review-editor/dock/panels/ReviewAgentJobDetailPanel.tsx +++ b/packages/review-editor/dock/panels/ReviewAgentJobDetailPanel.tsx @@ -1,13 +1,18 @@ -import React, { useMemo, useState, useEffect } from 'react'; +import React, { useMemo, useState, useEffect, useCallback } from 'react'; import type { IDockviewPanelProps } from 'dockview-react'; import type { AgentJobInfo, CodeAnnotation } from '@plannotator/ui/types'; import { isTerminalStatus } from '@plannotator/shared/agent-jobs'; import { useReviewState } from '../ReviewStateContext'; +import { CopyButton } from '../../components/CopyButton'; +import { LiveLogViewer } from '../../components/LiveLogViewer'; +import { exportReviewFeedback } from '../../utils/exportFeedback'; + +// --------------------------------------------------------------------------- +// Panel +// --------------------------------------------------------------------------- + +type DetailTab = 'findings' | 'logs'; -/** - * Center dock panel showing full detail for a single agent job. - * Displays all AgentJobInfo fields + linked annotations. - */ export const ReviewAgentJobDetailPanel: React.FC = (props) => { const jobId: string = props.params?.jobId ?? ''; const state = useReviewState(); @@ -17,192 +22,335 @@ export const ReviewAgentJobDetailPanel: React.FC = (props) [state.agentJobs, jobId] ); - const linkedAnnotations = useMemo( - () => - job - ? (state.externalAnnotations.filter( - (a) => a.source === job.source - ) as CodeAnnotation[]) - : [], - [state.externalAnnotations, job] + const terminal = job ? isTerminalStatus(job.status) : false; + const [activeTab, setActiveTab] = useState('findings'); + + // --- Prompt extraction --- + const { fullCommand, userMessage, systemPrompt } = useMemo(() => { + const cmd = job?.command ?? []; + const full = cmd.join(' '); + const prompt = cmd.length > 0 ? cmd[cmd.length - 1] : ''; + const sep = '\n\n---\n\n'; + const i = prompt.indexOf(sep); + return { + fullCommand: full, + userMessage: i !== -1 ? prompt.substring(i + sep.length) : prompt, + systemPrompt: i !== -1 ? prompt.substring(0, i) : '', + }; + }, [job]); + + // --- Annotation snapshot --- + const [annotationSnapshot, setAnnotationSnapshot] = useState< + Map + >(new Map()); + + useEffect(() => { + if (!job) return; + const currentIds = new Set( + state.externalAnnotations.filter((a) => a.source === job.source).map((a) => a.id) + ); + setAnnotationSnapshot((prev) => { + const next = new Map(prev); + for (const ann of state.externalAnnotations) { + if (ann.source !== job.source) continue; + next.set(ann.id, { annotation: ann as CodeAnnotation, dismissed: false }); + } + for (const [id, entry] of next) { + if (!entry.dismissed && !currentIds.has(id)) next.set(id, { ...entry, dismissed: true }); + } + return next; + }); + }, [state.externalAnnotations, job]); + + const displayAnnotations = useMemo(() => + Array.from(annotationSnapshot.values()).sort((a, b) => { + if (a.dismissed !== b.dismissed) return a.dismissed ? 1 : -1; + return a.annotation.filePath.localeCompare(b.annotation.filePath) || a.annotation.lineStart - b.annotation.lineStart; + }), + [annotationSnapshot]); + + const activeAnnotations = useMemo(() => displayAnnotations.filter((d) => !d.dismissed).map((d) => d.annotation), [displayAnnotations]); + const dismissedCount = useMemo(() => displayAnnotations.filter((d) => d.dismissed).length, [displayAnnotations]); + + const handleAnnotationClick = useCallback((ann: CodeAnnotation) => { + state.onSelectAnnotation(ann.id); + }, [state.onSelectAnnotation]); + + const copyAllText = useMemo( + () => activeAnnotations.length > 0 ? exportReviewFeedback(activeAnnotations, state.prMetadata) : '', + [activeAnnotations, state.prMetadata] ); + const logContent = state.jobLogs.get(jobId) ?? ''; + if (!job) { - return ( -
- Job not found -
- ); + return
Job not found
; } - const handleAnnotationClick = (ann: CodeAnnotation) => { - state.openDiffFile(ann.filePath); - state.onSelectAnnotation(ann.id); - }; + const isCorrect = job.summary + ? job.summary.correctness.toLowerCase().includes('correct') && !job.summary.correctness.toLowerCase().includes('incorrect') + : null; return ( -
- {/* Header */} -
-
- - - {!isTerminalStatus(job.status) && ( - - - - )} +
+ {/* ── Header ── */} +
+
+ + + {job.label} + + {terminal && job.endedAt ? formatDuration(job.endedAt - job.startedAt) : } +
-

{job.label}

+ {job.cwd && ( +

{job.cwd}

+ )}
-
- {/* Metadata */} -
-

Details

-
-
Source
-
{job.source}
- -
Command
-
{job.command.join(' ')}
- -
Started
-
{new Date(job.startedAt).toLocaleTimeString()}
- - {job.endedAt && ( - <> -
Ended
-
{new Date(job.endedAt).toLocaleTimeString()}
- - )} - - {job.endedAt && ( - <> -
Duration
-
{formatDuration(job.endedAt - job.startedAt)}
- - )} - - {job.exitCode !== undefined && ( - <> -
Exit code
-
- {job.exitCode} -
- - )} -
-
- - {/* Stderr */} - {job.error && ( -
-

- Stderr -

-
-              {job.error}
-            
-
- )} + {/* ── Verdict ── */} +
+ +
- {/* Linked Annotations */} -
-

- Annotations ({linkedAnnotations.length}) -

- {linkedAnnotations.length === 0 ? ( -

- {isTerminalStatus(job.status) - ? 'No annotations were produced by this job.' - : 'Annotations will appear here as the agent produces them.'} -

+ {/* ── Tabs ── */} +
+ setActiveTab('findings')}> + Findings{activeAnnotations.length > 0 && ` (${activeAnnotations.length})`} + + setActiveTab('logs')}> + Logs + {!terminal && } + +
+ + {/* ── Content ── */} + {activeTab === 'findings' ? ( +
+ {/* Annotations */} + {displayAnnotations.length > 0 && ( +
+ + {activeAnnotations.length} finding{activeAnnotations.length !== 1 ? 's' : ''} + {dismissedCount > 0 && ` · ${dismissedCount} dismissed`} + + {copyAllText && } +
+ )} + + {displayAnnotations.length === 0 ? ( + ) : ( -
- {linkedAnnotations.map((ann) => ( - +
+ {displayAnnotations.map(({ annotation: ann, dismissed }) => ( + ))}
)} -
- {/* Output placeholder */} - {!isTerminalStatus(job.status) && linkedAnnotations.length === 0 && ( -
- Agent output will appear as annotations in the diff. -
+ {/* Debug context — collapsed by default */} + +
+
+ Started {new Date(job.startedAt).toLocaleTimeString()} + {job.exitCode !== undefined && ( + Exit {job.exitCode} + )} +
+ {userMessage && ( + +
{userMessage}
+
+ )} + {systemPrompt && ( + +
{systemPrompt}
+
+ )} + {fullCommand && ( +
+ {fullCommand.slice(0, 60)}... + +
+ )} +
+
+
+ ) : ( +
+ + {!logContent && job.error && terminal && ( +
+
+                {job.error}
+              
+
+ )} +
+ )} +
+ ); +}; + +// --------------------------------------------------------------------------- +// Subcomponents +// --------------------------------------------------------------------------- + +function VerdictCard({ summary, isCorrect, terminal }: { + summary: AgentJobInfo['summary']; + isCorrect: boolean | null; + terminal: boolean; +}) { + if (summary) { + return ( +
+
+ + {isCorrect ? 'Correct' : 'Incorrect'} + + + Confidence {Math.round(summary.confidence * 100)}% + +
+

{summary.explanation}

+
+ ); + } + + return ( +
+
+ + Review Verdict + + {!terminal && ( + Pending... )}
+ {terminal ? ( +

No verdict available.

+ ) : ( +

Will appear when the review completes.

+ )}
); -}; +} -// --- Small helper components (mirrors from AgentsTab, kept local) --- +function StatusDot({ status }: { status: AgentJobInfo['status'] }) { + if (status === 'starting' || status === 'running') { + return ( + + + + + ); + } + const c: Record = { done: 'bg-success', failed: 'bg-destructive', killed: 'bg-muted-foreground' }; + return ; +} -function formatDuration(ms: number): string { - const seconds = Math.floor(ms / 1000); - if (seconds < 60) return `${seconds}s`; - const minutes = Math.floor(seconds / 60); - const remainingSeconds = seconds % 60; - return `${minutes}m ${remainingSeconds}s`; +function ProviderPill({ provider }: { provider: string }) { + const label = provider === 'claude' ? 'Claude' : provider === 'codex' ? 'Codex' : 'Shell'; + return {label}; } -function JobElapsedTime({ startedAt }: { startedAt: number }) { - const [, setTick] = useState(0); - useEffect(() => { - const timer = setInterval(() => setTick((t) => t + 1), 1000); - return () => clearInterval(timer); - }, []); - return <>{formatDuration(Date.now() - startedAt)}; +function TabButton({ active, onClick, children }: { active: boolean; onClick: () => void; children: React.ReactNode }) { + return ( + + ); } -function JobStatusBadge({ status }: { status: AgentJobInfo['status'] }) { - const config: Record = { - starting: { className: 'text-primary', label: 'Starting' }, - running: { className: 'text-primary', label: 'Running' }, - done: { className: 'text-success', label: 'Done' }, - failed: { className: 'text-destructive', label: 'Failed' }, - killed: { className: 'text-muted-foreground', label: 'Killed' }, - }; - const c = config[status] ?? config.killed; +function Disclosure({ title, copyText, nested, children }: { + title: string; + copyText?: string; + nested?: boolean; + children: React.ReactNode; +}) { + const [open, setOpen] = useState(false); return ( - - {(status === 'starting' || status === 'running') && ( - - - - +
+
+ + {open && copyText && } +
+ {open &&
{children}
} +
+ ); +} + +function AnnotationRow({ annotation: ann, dismissed, onClick }: { + annotation: CodeAnnotation; + dismissed: boolean; + onClick: (ann: CodeAnnotation) => void; +}) { + return ( + ); } -function JobProviderBadge({ provider }: { provider: string }) { - const label = provider === 'claude' ? 'Claude' : provider === 'codex' ? 'Codex' : 'Shell'; +function EmptyState({ terminal }: { terminal: boolean }) { return ( - - {label} - +
+

+ {terminal ? 'No findings were produced.' : 'Findings will appear as the agent works.'} +

+
); } + +function ElapsedTime({ startedAt }: { startedAt: number }) { + const [, setTick] = useState(0); + useEffect(() => { + const timer = setInterval(() => setTick((t) => t + 1), 1000); + return () => clearInterval(timer); + }, []); + return <>{formatDuration(Date.now() - startedAt)}; +} + +function formatDuration(ms: number): string { + const s = Math.floor(ms / 1000); + if (s < 60) return `${s}s`; + return `${Math.floor(s / 60)}m ${s % 60}s`; +} diff --git a/packages/server/AGENT_LIVE_LOGS.md b/packages/server/AGENT_LIVE_LOGS.md new file mode 100644 index 00000000..00051f4d --- /dev/null +++ b/packages/server/AGENT_LIVE_LOGS.md @@ -0,0 +1,103 @@ +# Agent Live Logs — Design Notes + +Follow-up to the Codex review agent integration. Surfaces stderr in real-time in the agent job detail panel. + +## Problem + +Stderr is drained continuously during job execution but only saved to `job.error` (last 500 chars) on non-zero exit. Users have no visibility into what the agent is doing while it runs. + +## Approach: `job:log` SSE event + +New SSE event type — does not modify `AgentJobInfo`, no Pi breakage, backward compatible (old clients ignore unknown events). + +```typescript +| { type: "job:log"; jobId: string; delta: string } +``` + +### Server (`packages/server/agent-jobs.ts`) + +In the stderr drain loop, buffer-and-flush at 200ms intervals: + +```typescript +let logFlushTimer: ReturnType | null = null; +let logPending = ""; + +for await (const chunk of reader) { + const text = decode(chunk); + stderrBuf = (stderrBuf + text).slice(-500); // existing error capture + logPending += text; + + if (!logFlushTimer) { + logFlushTimer = setTimeout(() => { + if (logPending) { + broadcast({ type: "job:log", jobId: id, delta: logPending }); + logPending = ""; + } + logFlushTimer = null; + }, 200); + } +} + +// Flush remaining on stream close +if (logPending) { + broadcast({ type: "job:log", jobId: id, delta: logPending }); +} +``` + +Buffer-and-flush (not time-based throttle) ensures a single large stderr dump sends one event with all the content, and rapid small writes batch into fewer events. + +### Shared types (`packages/shared/agent-jobs.ts`) + +Add the new event variant to the union: + +```typescript +export type AgentJobEvent = + | { type: "snapshot"; jobs: AgentJobInfo[] } + | { type: "job:started"; job: AgentJobInfo } + | { type: "job:updated"; job: AgentJobInfo } + | { type: "job:completed"; job: AgentJobInfo } + | { type: "job:log"; jobId: string; delta: string } // NEW + | { type: "jobs:cleared" }; +``` + +`AgentJobInfo` unchanged — no Pi mirror breakage. + +### Client hook (`packages/ui/hooks/useAgentJobs.ts`) + +Accumulate deltas in a parallel Map (same pattern as AI chat `text + msg.delta`): + +```typescript +const [jobLogs, setJobLogs] = useState>(new Map()); + +// In SSE handler: +case 'job:log': + setJobLogs(prev => { + const next = new Map(prev); + next.set(parsed.jobId, (prev.get(parsed.jobId) ?? '') + parsed.delta); + return next; + }); + break; +``` + +Expose `jobLogs` in the hook return value. No cap on accumulation — cap at the render layer. + +### UI (`ReviewAgentJobDetailPanel.tsx`) + +Replace the current stderr section with a live log view: + +- While running: show `jobLogs.get(job.id)` in a scrollable monospace container +- On terminal state: show `jobLogs.get(job.id)` (full history) or fall back to `job.error` (for jobs that started before the SSE connection) +- Auto-scroll to bottom on new content (unless user has scrolled up — detect with scroll position check) +- No max-height cap on the log container (let it fill available space in the dockview panel) +- Max render window: if logs exceed ~50KB, trim from the top and show "[earlier output truncated]" + +### Polling fallback + +Clients using polling (SSE failure) won't receive `job:log` events. They still get `job.error` on completion. Live logs are a best-effort enhancement, not a guarantee. + +### Performance notes + +- React batches rapid `setJobLogs` calls automatically +- 200ms buffer-and-flush server-side prevents SSE flooding +- Map-based state avoids re-rendering unrelated job cards (only the detail panel for the active job re-renders) +- String concatenation is O(n) over the job lifetime but stderr output is typically small (< 100KB) diff --git a/packages/server/PR_LOCAL_WORKTREE.md b/packages/server/PR_LOCAL_WORKTREE.md new file mode 100644 index 00000000..0dfe166e --- /dev/null +++ b/packages/server/PR_LOCAL_WORKTREE.md @@ -0,0 +1,85 @@ +# PR Local Worktree — Design Notes + +When reviewing a PR/MR via URL, create a temporary git worktree so the review agent has full local file access — without touching the user's current working tree. + +## Problem + +In PR mode today, the review server has no local file access. The diff comes from `gh pr diff`, file content from GitHub/GitLab API. This means: +- Codex can't read files, run git commands, or explore the codebase +- The agent is limited to analyzing the inlined diff text +- Review quality is degraded compared to local reviews + +## Approach + +New `--local` flag on the review command. When reviewing a PR with `--local`: + +1. **Fetch the PR head** into the user's local repo (doesn't change their checkout): + ```bash + # GitHub + git fetch origin refs/pull/123/head + + # GitLab + git fetch origin refs/merge-requests/42/head + ``` + +2. **Create a temporary worktree** pointing at the fetched commit: + ```bash + git worktree add --detach /tmp/plannotator-pr-123- FETCH_HEAD + ``` + + Detached HEAD — no branch created, no local branch pollution. + +3. **Start the review server** with: + - `gitContext.cwd` pointing to the worktree (not the user's working directory) + - Full `gitContext` populated (branch info, diff options) — NOT pure PR mode + - The diff computed locally: `git diff ...` in the worktree + - Agent's `getCwd()` resolves to the worktree path + + The review server operates as if it were a local review, but in the worktree sandbox. + +4. **Cleanup on shutdown** (default behavior): + ```bash + git worktree remove /tmp/plannotator-pr-123- --force + ``` + + Wired into the existing `process.once("exit")` cleanup pattern. + +## User Experience + +```bash +# Pure remote PR review (current behavior, no local files) +plannotator review https://github.com/owner/repo/pull/123 + +# PR review with local worktree (agent gets full file access) +plannotator review https://github.com/owner/repo/pull/123 --local +``` + +The user's current checkout is completely untouched. They can be mid-work on their own branch. The worktree is created in `/tmp`, the review server runs there, and it's cleaned up when the review session ends. + +## Prerequisites + +- User must be in a clone of the same repository (or have the remote configured) +- `gh` / `glab` CLI authenticated (already required for PR mode) +- Git must support worktrees (any modern git) + +## Edge Cases + +- **GitHub fork PRs**: `refs/pull/N/head` is always fetchable from the upstream repo — works +- **GitLab cross-project MRs**: Depends on access permissions to the source project +- **Fetch failure**: Fall back to pure PR mode (no local files) with a warning +- **Worktree cleanup failure**: Log warning but don't block — orphaned `/tmp` dirs get cleaned by OS +- **User wants to keep worktree**: Future `--keep-worktree` flag, not default + +## Implementation Location + +- `apps/hook/server/index.ts` — parse `--local` flag, create worktree before `startReviewServer` +- New utility in `packages/shared/review-core.ts` or `packages/server/vcs.ts`: + - `createPRWorktree(prMetadata, repoPath)` → `{ worktreePath, cleanup }` +- `packages/server/review.ts` — no changes needed, already supports custom `cwd` via `gitContext` + +## Relationship to Agent Review + +This dramatically improves Codex review quality for PRs: +- Agent runs in the worktree with full git + file access +- User message changes from "analyze this inlined diff" to "review the changes in this repo" +- Same review experience as local uncommitted changes diff --git a/packages/server/agent-jobs.ts b/packages/server/agent-jobs.ts index 494d75b3..0e486e77 100644 --- a/packages/server/agent-jobs.ts +++ b/packages/server/agent-jobs.ts @@ -56,6 +56,17 @@ export interface AgentJobHandlerOptions { getServerUrl: () => string; /** Returns the working directory for spawned processes. */ getCwd: () => string; + /** + * Build the command server-side for a given provider. + * Return an object with the command to spawn (and optional output path for result ingestion). + * Return null to reject or fall through to frontend-supplied command. + */ + buildCommand?: (provider: string) => Promise<{ command: string[]; outputPath?: string; label?: string } | null>; + /** + * Called after a job process exits with exit code 0. + * Use for result ingestion (e.g., reading an output file and pushing annotations). + */ + onJobComplete?: (job: AgentJobInfo, meta: { outputPath?: string }) => void | Promise; } export function createAgentJobHandler(options: AgentJobHandlerOptions): AgentJobHandler { @@ -63,6 +74,7 @@ export function createAgentJobHandler(options: AgentJobHandlerOptions): AgentJob // --- State --- const jobs = new Map | null }>(); + const jobOutputPaths = new Map(); const subscribers = new Set(); const encoder = new TextEncoder(); let version = 0; @@ -97,6 +109,7 @@ export function createAgentJobHandler(options: AgentJobHandlerOptions): AgentJob provider: string, command: string[], label: string, + outputPath?: string, ): AgentJobInfo { const id = crypto.randomUUID(); const source = jobSource(id); @@ -109,6 +122,7 @@ export function createAgentJobHandler(options: AgentJobHandlerOptions): AgentJob status: "starting", startedAt: Date.now(), command, + cwd: getCwd(), }; let proc: ReturnType | null = null; @@ -127,10 +141,14 @@ export function createAgentJobHandler(options: AgentJobHandlerOptions): AgentJob info.status = "running"; jobs.set(id, { info, proc }); + if (outputPath) jobOutputPaths.set(id, outputPath); broadcast({ type: "job:started", job: { ...info } }); - // Drain stderr continuously to prevent pipe-full deadlock + // Drain stderr: capture tail for error reporting + broadcast live log deltas let stderrBuf = ""; + let logPending = ""; + let logFlushTimer: ReturnType | null = null; + if (proc.stderr && typeof proc.stderr !== "number") { (async () => { try { @@ -138,6 +156,23 @@ export function createAgentJobHandler(options: AgentJobHandlerOptions): AgentJob for await (const chunk of reader) { const text = typeof chunk === "string" ? chunk : new TextDecoder().decode(chunk); stderrBuf = (stderrBuf + text).slice(-500); + logPending += text; + + if (!logFlushTimer) { + logFlushTimer = setTimeout(() => { + if (logPending) { + broadcast({ type: "job:log", jobId: id, delta: logPending }); + logPending = ""; + } + logFlushTimer = null; + }, 200); + } + } + // Flush remaining on stream close + if (logFlushTimer) { clearTimeout(logFlushTimer); logFlushTimer = null; } + if (logPending) { + broadcast({ type: "job:log", jobId: id, delta: logPending }); + logPending = ""; } } catch { // Stream closed or already consumed @@ -146,7 +181,7 @@ export function createAgentJobHandler(options: AgentJobHandlerOptions): AgentJob } // Monitor process exit - proc.exited.then((exitCode) => { + proc.exited.then(async (exitCode) => { const entry = jobs.get(id); if (!entry || isTerminalStatus(entry.info.status)) return; @@ -158,7 +193,20 @@ export function createAgentJobHandler(options: AgentJobHandlerOptions): AgentJob entry.info.error = stderrBuf; } + // Ingest results before broadcasting completion so annotations arrive first + const outputPath = jobOutputPaths.get(id); + if (exitCode === 0 && options.onJobComplete) { + try { + await options.onJobComplete(entry.info, { outputPath }); + } catch { + // Result ingestion failure shouldn't prevent job completion broadcast + } + } + jobOutputPaths.delete(id); + broadcast({ type: "job:completed", job: { ...entry.info } }); + }).catch(() => { + // Guard against unhandled rejection from unexpected runtime errors }); } catch (err) { // Spawn itself failed (e.g., command not found). @@ -284,9 +332,10 @@ export function createAgentJobHandler(options: AgentJobHandlerOptions): AgentJob try { const body = await req.json(); const provider = typeof body.provider === "string" ? body.provider : "shell"; - const rawCommand = Array.isArray(body.command) ? body.command : []; - const command = rawCommand.filter((c: unknown): c is string => typeof c === "string"); - const label = typeof body.label === "string" ? body.label : `${provider} agent`; + let rawCommand = Array.isArray(body.command) ? body.command : []; + let command = rawCommand.filter((c: unknown): c is string => typeof c === "string"); + let label = typeof body.label === "string" ? body.label : `${provider} agent`; + let outputPath: string | undefined; // Validate provider is a known, available capability const cap = capabilities.find((c) => c.id === provider); @@ -297,6 +346,16 @@ export function createAgentJobHandler(options: AgentJobHandlerOptions): AgentJob ); } + // For non-shell providers, try server-side command building + if (options.buildCommand && provider !== "shell") { + const built = await options.buildCommand(provider); + if (built) { + command = built.command; + outputPath = built.outputPath; + if (built.label) label = built.label; + } + } + if (command.length === 0) { return Response.json( { error: 'Missing "command" array' }, @@ -304,7 +363,7 @@ export function createAgentJobHandler(options: AgentJobHandlerOptions): AgentJob ); } - const job = spawnJob(provider, command, label); + const job = spawnJob(provider, command, label, outputPath); return Response.json({ job }, { status: 201 }); } catch { return Response.json({ error: "Invalid JSON" }, { status: 400 }); diff --git a/packages/server/codex-review-schema.json b/packages/server/codex-review-schema.json new file mode 100644 index 00000000..4fce4054 --- /dev/null +++ b/packages/server/codex-review-schema.json @@ -0,0 +1,41 @@ +{ + "type": "object", + "properties": { + "findings": { + "type": "array", + "items": { + "type": "object", + "properties": { + "title": { "type": "string" }, + "body": { "type": "string" }, + "confidence_score": { "type": "number" }, + "priority": { "type": ["integer", "null"] }, + "code_location": { + "type": "object", + "properties": { + "absolute_file_path": { "type": "string" }, + "line_range": { + "type": "object", + "properties": { + "start": { "type": "integer" }, + "end": { "type": "integer" } + }, + "required": ["start", "end"], + "additionalProperties": false + } + }, + "required": ["absolute_file_path", "line_range"], + "additionalProperties": false + } + }, + "required": ["title", "body", "confidence_score", "priority", "code_location"], + "additionalProperties": false + } + }, + "overall_correctness": { "type": "string" }, + "overall_explanation": { "type": "string" }, + "overall_confidence_score": { "type": "number" } + }, + "required": ["findings", "overall_correctness", "overall_explanation", "overall_confidence_score"], + "additionalProperties": false +} diff --git a/packages/server/codex-review.ts b/packages/server/codex-review.ts new file mode 100644 index 00000000..392581f7 --- /dev/null +++ b/packages/server/codex-review.ts @@ -0,0 +1,385 @@ +/** + * Codex Review Agent — prompt, command builder, output parser, and finding transformer. + * + * Encapsulates all Codex-specific logic for the AI review agent integration. + * The review server (review.ts) calls into this module via the agent-jobs callbacks. + */ + +import { join } from "node:path"; +import { homedir, tmpdir } from "node:os"; +import { appendFile, mkdir, unlink } from "node:fs/promises"; +import type { DiffType, GitContext } from "./vcs"; +import type { PRMetadata } from "./pr"; + +// --------------------------------------------------------------------------- +// Debug log — writes to ~/.plannotator/codex-review-debug.log +// --------------------------------------------------------------------------- + +const DEBUG_LOG_PATH = join(homedir(), ".plannotator", "codex-review-debug.log"); + +async function debugLog(label: string, data?: unknown): Promise { + try { + await mkdir(join(homedir(), ".plannotator"), { recursive: true }); + const timestamp = new Date().toISOString(); + const line = data !== undefined + ? `[${timestamp}] ${label}: ${typeof data === "string" ? data : JSON.stringify(data, null, 2)}\n` + : `[${timestamp}] ${label}\n`; + await appendFile(DEBUG_LOG_PATH, line); + } catch { /* never fail the main flow */ } +} + +export { DEBUG_LOG_PATH }; + +// --------------------------------------------------------------------------- +// Schema — embedded as a string, written to disk on first use. +// Bun's compiled binary uses a virtual FS that external processes (codex) +// can't read, so we materialize the schema to a real file. +// --------------------------------------------------------------------------- + +const CODEX_REVIEW_SCHEMA = JSON.stringify({ + type: "object", + properties: { + findings: { + type: "array", + items: { + type: "object", + properties: { + title: { type: "string" }, + body: { type: "string" }, + confidence_score: { type: "number" }, + priority: { type: ["integer", "null"] }, + code_location: { + type: "object", + properties: { + absolute_file_path: { type: "string" }, + line_range: { + type: "object", + properties: { + start: { type: "integer" }, + end: { type: "integer" }, + }, + required: ["start", "end"], + additionalProperties: false, + }, + }, + required: ["absolute_file_path", "line_range"], + additionalProperties: false, + }, + }, + required: ["title", "body", "confidence_score", "priority", "code_location"], + additionalProperties: false, + }, + }, + overall_correctness: { type: "string" }, + overall_explanation: { type: "string" }, + overall_confidence_score: { type: "number" }, + }, + required: ["findings", "overall_correctness", "overall_explanation", "overall_confidence_score"], + additionalProperties: false, +}); + +const SCHEMA_DIR = join(homedir(), ".plannotator"); +const SCHEMA_FILE = join(SCHEMA_DIR, "codex-review-schema.json"); +let schemaMaterialized = false; + +/** Ensure the schema file exists on disk and return its path. */ +async function ensureSchemaFile(): Promise { + if (!schemaMaterialized) { + await mkdir(SCHEMA_DIR, { recursive: true }); + await Bun.write(SCHEMA_FILE, CODEX_REVIEW_SCHEMA); + schemaMaterialized = true; + } + return SCHEMA_FILE; +} + +export { SCHEMA_FILE as CODEX_REVIEW_SCHEMA_PATH }; + +// --------------------------------------------------------------------------- +// System prompt — copied verbatim from codex-rs/core/review_prompt.md +// --------------------------------------------------------------------------- + +export const CODEX_REVIEW_SYSTEM_PROMPT = `# Review guidelines: + +You are acting as a reviewer for a proposed code change made by another engineer. + +Below are some default guidelines for determining whether the original author would appreciate the issue being flagged. + +These are not the final word in determining whether an issue is a bug. In many cases, you will encounter other, more specific guidelines. These may be present elsewhere in a developer message, a user message, a file, or even elsewhere in this system message. +Those guidelines should be considered to override these general instructions. + +Here are the general guidelines for determining whether something is a bug and should be flagged. + +1. It meaningfully impacts the accuracy, performance, security, or maintainability of the code. +2. The bug is discrete and actionable (i.e. not a general issue with the codebase or a combination of multiple issues). +3. Fixing the bug does not demand a level of rigor that is not present in the rest of the codebase (e.g. one doesn't need very detailed comments and input validation in a repository of one-off scripts in personal projects) +4. The bug was introduced in the commit (pre-existing bugs should not be flagged). +5. The author of the original PR would likely fix the issue if they were made aware of it. +6. The bug does not rely on unstated assumptions about the codebase or author's intent. +7. It is not enough to speculate that a change may disrupt another part of the codebase, to be considered a bug, one must identify the other parts of the code that are provably affected. +8. The bug is clearly not just an intentional change by the original author. + +When flagging a bug, you will also provide an accompanying comment. Once again, these guidelines are not the final word on how to construct a comment -- defer to any subsequent guidelines that you encounter. + +1. The comment should be clear about why the issue is a bug. +2. The comment should appropriately communicate the severity of the issue. It should not claim that an issue is more severe than it actually is. +3. The comment should be brief. The body should be at most 1 paragraph. It should not introduce line breaks within the natural language flow unless it is necessary for the code fragment. +4. The comment should not include any chunks of code longer than 3 lines. Any code chunks should be wrapped in markdown inline code tags or a code block. +5. The comment should clearly and explicitly communicate the scenarios, environments, or inputs that are necessary for the bug to arise. The comment should immediately indicate that the issue's severity depends on these factors. +6. The comment's tone should be matter-of-fact and not accusatory or overly positive. It should read as a helpful AI assistant suggestion without sounding too much like a human reviewer. +7. The comment should be written such that the original author can immediately grasp the idea without close reading. +8. The comment should avoid excessive flattery and comments that are not helpful to the original author. The comment should avoid phrasing like "Great job ...", "Thanks for ...". + +Below are some more detailed guidelines that you should apply to this specific review. + +HOW MANY FINDINGS TO RETURN: + +Output all findings that the original author would fix if they knew about it. If there is no finding that a person would definitely love to see and fix, prefer outputting no findings. Do not stop at the first qualifying finding. Continue until you've listed every qualifying finding. + +GUIDELINES: + +- Ignore trivial style unless it obscures meaning or violates documented standards. +- Use one comment per distinct issue (or a multi-line range if necessary). +- Use \`\`\`suggestion blocks ONLY for concrete replacement code (minimal lines; no commentary inside the block). +- In every \`\`\`suggestion block, preserve the exact leading whitespace of the replaced lines (spaces vs tabs, number of spaces). +- Do NOT introduce or remove outer indentation levels unless that is the actual fix. + +The comments will be presented in the code review as inline comments. You should avoid providing unnecessary location details in the comment body. Always keep the line range as short as possible for interpreting the issue. Avoid ranges longer than 5–10 lines; instead, choose the most suitable subrange that pinpoints the problem. + +At the beginning of the finding title, tag the bug with priority level. For example "[P1] Un-padding slices along wrong tensor dimensions". [P0] – Drop everything to fix. Blocking release, operations, or major usage. Only use for universal issues that do not depend on any assumptions about the inputs. · [P1] – Urgent. Should be addressed in the next cycle · [P2] – Normal. To be fixed eventually · [P3] – Low. Nice to have. + +Additionally, include a numeric priority field in the JSON output for each finding: set "priority" to 0 for P0, 1 for P1, 2 for P2, or 3 for P3. If a priority cannot be determined, omit the field or use null. + +At the end of your findings, output an "overall correctness" verdict of whether or not the patch should be considered "correct". +Correct implies that existing code and tests will not break, and the patch is free of bugs and other blocking issues. +Ignore non-blocking issues such as style, formatting, typos, documentation, and other nits. + +FORMATTING GUIDELINES: +The finding description should be one paragraph.`; + +// --------------------------------------------------------------------------- +// User message builder +// --------------------------------------------------------------------------- + +/** Build the dynamic user message based on review context. */ +export function buildCodexReviewUserMessage( + patch: string, + diffType: DiffType, + gitContext?: GitContext, + prMetadata?: PRMetadata, +): string { + // PR/MR mode — just pass the link, Codex knows what to do + if (prMetadata) { + if (gitContext) { + return `${prMetadata.url}\n\nYou are in a local worktree checked out from the PR branch. The code is available locally.`; + } + return prMetadata.url; + } + + // Local mode — Codex has full file/git access + const effectiveDiffType = diffType.startsWith("worktree:") + ? diffType.split(":").pop() || "uncommitted" + : diffType; + + switch (effectiveDiffType) { + case "uncommitted": + return "Review the current code changes (staged, unstaged, and untracked files) and provide prioritized findings."; + + case "staged": + return "Review the currently staged code changes (`git diff --staged`) and provide prioritized findings."; + + case "unstaged": + return "Review the unstaged code changes (tracked modifications and untracked files) and provide prioritized findings."; + + case "last-commit": + return "Review the code changes introduced in the last commit (`git diff HEAD~1..HEAD`) and provide prioritized findings."; + + case "branch": { + const base = gitContext?.defaultBranch || "main"; + return `Review the code changes against the base branch '${base}'. Run \`git diff ${base}...HEAD\` to inspect the changes. Provide prioritized, actionable findings.`; + } + + default: + // p4 or unknown — fall back to generic with inlined diff + return [ + "Review the following code changes and provide prioritized findings.", + "", + "```diff", + patch, + "```", + ].join("\n"); + } +} + +// --------------------------------------------------------------------------- +// Command builder +// --------------------------------------------------------------------------- + +export interface CodexCommandOptions { + cwd: string; + outputPath: string; + prompt: string; +} + +/** Build the `codex exec` argv array. Materializes the schema file on first call. */ +export async function buildCodexCommand(options: CodexCommandOptions): Promise { + const { cwd, outputPath, prompt } = options; + const schemaPath = await ensureSchemaFile(); + + const command = [ + "codex", + "exec", + "--output-schema", schemaPath, + "-o", outputPath, + "--full-auto", + "--ephemeral", + "-C", cwd, + prompt, + ]; + + debugLog("BUILD_COMMAND", { + cwd, + outputPath, + schemaPath, + promptLength: prompt.length, + command: command.map((c, i) => i === command.length - 1 ? `` : c), + }); + + return command; +} + +/** Generate a unique temp file path for Codex output. */ +export function generateOutputPath(): string { + return join(tmpdir(), `plannotator-codex-${crypto.randomUUID()}.json`); +} + +// --------------------------------------------------------------------------- +// Output parsing — matches Codex's native ReviewOutputEvent schema +// --------------------------------------------------------------------------- + +export interface CodexCodeLocation { + absolute_file_path: string; + line_range: { start: number; end: number }; +} + +export interface CodexFinding { + title: string; + body: string; + confidence_score: number; + priority: number | null; + code_location: CodexCodeLocation; +} + +export interface CodexReviewOutput { + findings: CodexFinding[]; + overall_correctness: string; + overall_explanation: string; + overall_confidence_score: number; +} + +/** Read and parse the Codex -o output file. Returns null on any failure. */ +export async function parseCodexOutput(outputPath: string): Promise { + await debugLog("PARSE_OUTPUT_START", { outputPath }); + + try { + const file = Bun.file(outputPath); + if (!await file.exists()) { + await debugLog("PARSE_OUTPUT_FILE_MISSING", outputPath); + return null; + } + + const text = await file.text(); + await debugLog("PARSE_OUTPUT_RAW", text); + + // Clean up temp file + try { await unlink(outputPath); } catch { /* ignore */ } + + if (!text.trim()) { + await debugLog("PARSE_OUTPUT_EMPTY"); + return null; + } + + const parsed = JSON.parse(text); + if (!parsed || !Array.isArray(parsed.findings)) { + await debugLog("PARSE_OUTPUT_INVALID_SHAPE", { hasFindings: !!parsed?.findings }); + return null; + } + + await debugLog("PARSE_OUTPUT_SUCCESS", { + findingsCount: parsed.findings.length, + overall_correctness: parsed.overall_correctness, + overall_confidence_score: parsed.overall_confidence_score, + }); + + return parsed as CodexReviewOutput; + } catch (err) { + await debugLog("PARSE_OUTPUT_ERROR", err instanceof Error ? err.message : String(err)); + // Clean up on error too + try { await unlink(outputPath); } catch { /* ignore */ } + return null; + } +} + +// --------------------------------------------------------------------------- +// Finding → external annotation transform +// --------------------------------------------------------------------------- + +export interface ReviewAnnotationInput { + source: string; + filePath: string; + lineStart: number; + lineEnd: number; + type: string; + side: string; + scope: string; + text: string; + author: string; +} + +/** + * Strip the cwd prefix from an absolute path to get a repo-relative path. + * Diff headers use relative paths (e.g., "packages/server/codex-review.ts") + * but Codex reports absolute paths (e.g., "/Users/.../packages/server/codex-review.ts"). + */ +function toRelativePath(absolutePath: string, cwd?: string): string { + if (!cwd) return absolutePath; + const prefix = cwd.endsWith("/") ? cwd : cwd + "/"; + return absolutePath.startsWith(prefix) ? absolutePath.slice(prefix.length) : absolutePath; +} + +/** Transform Codex findings (native schema) into the external annotation format. */ +export function transformCodexFindings( + findings: CodexFinding[], + source: string, + cwd?: string, +): ReviewAnnotationInput[] { + const annotations = findings + .filter((f) => + f.code_location?.absolute_file_path && + typeof f.code_location?.line_range?.start === "number" && + typeof f.code_location?.line_range?.end === "number" + ) + .map((f) => ({ + source, + filePath: toRelativePath(f.code_location.absolute_file_path, cwd), + lineStart: f.code_location.line_range.start, + lineEnd: f.code_location.line_range.end, + type: "comment", + side: "new", + scope: "line", + text: `${f.title}\n\n${f.body}`.trim(), + author: "Codex", + })); + + debugLog("TRANSFORM_FINDINGS", { + inputCount: findings.length, + outputCount: annotations.length, + annotations: annotations.map((a) => ({ + filePath: a.filePath, + lineStart: a.lineStart, + lineEnd: a.lineEnd, + textPreview: a.text.slice(0, 80), + })), + }); + + return annotations; +} diff --git a/packages/server/external-annotations.ts b/packages/server/external-annotations.ts index d1562165..0f49be66 100644 --- a/packages/server/external-annotations.ts +++ b/packages/server/external-annotations.ts @@ -33,6 +33,8 @@ export interface ExternalAnnotationHandler { url: URL, options?: { disableIdleTimeout?: () => void }, ) => Promise; + /** Push annotations directly into the store (bypasses HTTP, reuses same validation). */ + addAnnotations: (body: unknown) => { ids: string[] } | { error: string }; } // --------------------------------------------------------------------------- @@ -68,6 +70,13 @@ export function createExternalAnnotationHandler( }); return { + addAnnotations(body: unknown): { ids: string[] } | { error: string } { + const parsed = transform(body); + if ("error" in parsed) return { error: parsed.error }; + const created = store.add(parsed.annotations); + return { ids: created.map((a) => a.id) }; + }, + async handle( req: Request, url: URL, diff --git a/packages/server/git.ts b/packages/server/git.ts index 9f837b31..864c46ba 100644 --- a/packages/server/git.ts +++ b/packages/server/git.ts @@ -53,7 +53,8 @@ async function runGit( return { stdout, stderr, exitCode }; } -const runtime: ReviewGitRuntime = { +/** Bun-based git runtime. Exported for use with shared utilities (worktree, etc.) */ +export const runtime: ReviewGitRuntime = { runGit, async readTextFile(path: string): Promise { try { diff --git a/packages/server/review.ts b/packages/server/review.ts index 37904810..a7d51e47 100644 --- a/packages/server/review.ts +++ b/packages/server/review.ts @@ -18,6 +18,14 @@ import { contentHash, deleteDraft } from "./draft"; import { createEditorAnnotationHandler } from "./editor-annotations"; import { createExternalAnnotationHandler } from "./external-annotations"; import { createAgentJobHandler } from "./agent-jobs"; +import { + CODEX_REVIEW_SYSTEM_PROMPT, + buildCodexReviewUserMessage, + buildCodexCommand, + generateOutputPath, + parseCodexOutput, + transformCodexFindings, +} from "./codex-review"; import { saveConfig, detectGitUser, getServerConfig } from "./config"; import { type PRMetadata, type PRReviewFileComment, fetchPRFileContent, fetchPRContext, submitPRReview, fetchPRViewedFiles, markPRFilesViewed, getPRUser, prRefFromMetadata, getDisplayRepo, getMRLabel, getMRNumberLabel } from "./pr"; import { createAIEndpoints, ProviderRegistry, SessionManager, createProvider, type AIEndpoints, type PiSDKConfig } from "@plannotator/ai"; @@ -57,6 +65,8 @@ export interface ReviewServerOptions { opencodeClient?: OpencodeClient; /** PR metadata when reviewing a pull request (PR mode) */ prMetadata?: PRMetadata; + /** Cleanup callback invoked when server stops (e.g., remove temp worktree) */ + onCleanup?: () => void | Promise; } export interface ReviewServerResult { @@ -96,6 +106,7 @@ export async function startReviewServer( const { htmlContent, origin, gitContext, sharingEnabled = true, shareBaseUrl, onReady, prMetadata } = options; const isPRMode = !!prMetadata; + const hasLocalAccess = !!gitContext; const draftKey = contentHash(options.rawPatch); const editorAnnotations = createEditorAnnotationHandler(); const externalAnnotations = createExternalAnnotationHandler("review"); @@ -114,6 +125,49 @@ export async function startReviewServer( getCwd: () => { return resolveVcsCwd(currentDiffType, gitContext?.cwd) ?? process.cwd(); }, + + async buildCommand(provider) { + if (provider !== "codex") return null; + + const cwd = resolveVcsCwd(currentDiffType, gitContext?.cwd) ?? process.cwd(); + const outputPath = generateOutputPath(); + + // Build the two-layer prompt: system prompt + dynamic user message + const userMessage = buildCodexReviewUserMessage(currentPatch, currentDiffType, gitContext, prMetadata); + const prompt = CODEX_REVIEW_SYSTEM_PROMPT + "\n\n---\n\n" + userMessage; + + const command = await buildCodexCommand({ + cwd, + outputPath, + prompt, + }); + + return { command, outputPath, label: "Codex Review" }; + }, + + async onJobComplete(job, meta) { + if (!meta.outputPath || job.provider !== "codex") return; + + const output = await parseCodexOutput(meta.outputPath); + if (!output) return; + + // Set summary on the job — flows through job:completed broadcast + job.summary = { + correctness: output.overall_correctness, + explanation: output.overall_explanation, + confidence: output.overall_confidence_score, + }; + + if (output.findings.length > 0) { + const cwd = resolveVcsCwd(currentDiffType, gitContext?.cwd) ?? process.cwd(); + const annotations = transformCodexFindings(output.findings, job.source, cwd); + const result = externalAnnotations.addAnnotations({ annotations }); + + if ("error" in result) { + console.error("[codex-review] addAnnotations error:", result.error); + } + } + }, }); // AI provider setup (graceful — AI features degrade if SDK unavailable) @@ -260,8 +314,8 @@ export async function startReviewServer( rawPatch: currentPatch, gitRef: currentGitRef, origin, - diffType: isPRMode ? undefined : currentDiffType, - gitContext: isPRMode ? undefined : gitContext, + diffType: hasLocalAccess ? currentDiffType : undefined, + gitContext: hasLocalAccess ? gitContext : undefined, sharingEnabled, shareBaseUrl, repoInfo, @@ -273,11 +327,11 @@ export async function startReviewServer( }); } - // API: Switch diff type (disabled in PR mode) + // API: Switch diff type (requires local file access) if (url.pathname === "/api/diff/switch" && req.method === "POST") { - if (isPRMode) { + if (!hasLocalAccess) { return Response.json( - { error: "Not available for PR reviews" }, + { error: "Not available without local file access" }, { status: 400 }, ); } @@ -351,8 +405,22 @@ export async function startReviewServer( } } + // Prefer local file access (works for both local mode and hybrid --local mode) + if (hasLocalAccess) { + const defaultBranch = gitContext?.defaultBranch || "main"; + const defaultCwd = gitContext?.cwd; + const result = await getVcsFileContentsForDiff( + currentDiffType, + defaultBranch, + filePath, + oldPath, + defaultCwd, + ); + return Response.json(result); + } + if (isPRMode) { - // Fetch file content from platform API using base/head SHAs + // Pure PR mode: fetch from platform API using base/head SHAs const [oldContent, newContent] = await Promise.all([ fetchPRFileContent(prRef!, prMetadata.baseSha, oldPath || filePath), fetchPRFileContent(prRef!, prMetadata.headSha, filePath), @@ -360,16 +428,7 @@ export async function startReviewServer( return Response.json({ oldContent, newContent }); } - const defaultBranch = gitContext?.defaultBranch || "main"; - const defaultCwd = gitContext?.cwd; - const result = await getVcsFileContentsForDiff( - currentDiffType, - defaultBranch, - filePath, - oldPath, - defaultCwd, - ); - return Response.json(result); + return Response.json({ error: "No file access available" }, { status: 400 }); } // API: Stage / unstage a file (disabled when VCS doesn't support it) @@ -595,6 +654,13 @@ export async function startReviewServer( aiSessionManager.disposeAll(); aiRegistry.disposeAll(); server.stop(); + // Invoke cleanup callback (e.g., remove temp worktree) + if (options.onCleanup) { + try { + const result = options.onCleanup(); + if (result instanceof Promise) result.catch(() => {}); + } catch { /* best effort */ } + } }, }; } diff --git a/packages/server/vcs.ts b/packages/server/vcs.ts index 0d20ec97..846e28b1 100644 --- a/packages/server/vcs.ts +++ b/packages/server/vcs.ts @@ -151,7 +151,7 @@ export type { WorktreeInfo, } from "./git"; -export { parseWorktreeDiffType, validateFilePath } from "./git"; +export { parseWorktreeDiffType, validateFilePath, runtime as gitRuntime } from "./git"; // --- Detection cache --- diff --git a/packages/shared/agent-jobs.ts b/packages/shared/agent-jobs.ts index efa5de45..6520358f 100644 --- a/packages/shared/agent-jobs.ts +++ b/packages/shared/agent-jobs.ts @@ -35,6 +35,14 @@ export interface AgentJobInfo { error?: string; /** The actual command that was spawned (for display/debug). */ command: string[]; + /** Working directory where the process was spawned. */ + cwd?: string; + /** Review summary set by the agent on completion. */ + summary?: { + correctness: string; + explanation: string; + confidence: number; + }; } export interface AgentCapability { @@ -59,6 +67,7 @@ export type AgentJobEvent = | { type: "job:started"; job: AgentJobInfo } | { type: "job:updated"; job: AgentJobInfo } | { type: "job:completed"; job: AgentJobInfo } + | { type: "job:log"; jobId: string; delta: string } | { type: "jobs:cleared" }; // --------------------------------------------------------------------------- diff --git a/packages/shared/package.json b/packages/shared/package.json index 0ba04fe3..cdbb4b0c 100644 --- a/packages/shared/package.json +++ b/packages/shared/package.json @@ -22,6 +22,7 @@ "./external-annotation": "./external-annotation.ts", "./agent-jobs": "./agent-jobs.ts", "./config": "./config.ts", - "./improvement-hooks": "./improvement-hooks.ts" + "./improvement-hooks": "./improvement-hooks.ts", + "./worktree": "./worktree.ts" } } diff --git a/packages/shared/worktree.ts b/packages/shared/worktree.ts new file mode 100644 index 00000000..d40028fa --- /dev/null +++ b/packages/shared/worktree.ts @@ -0,0 +1,119 @@ +/** + * Worktree — runtime-agnostic git worktree primitives. + * + * Uses ReviewGitRuntime so both Bun and Node runtimes can use the same logic. + * Lives in packages/shared/ and gets vendored to Pi via vendor.sh. + * + * Designed as composable primitives, not tied to any specific use case. + * PR local checkout, agent sandboxes, parallel sessions — all compose from these. + */ + +import type { ReviewGitRuntime } from "./review-core"; + +// --------------------------------------------------------------------------- +// Types +// --------------------------------------------------------------------------- + +export interface CreateWorktreeOptions { + /** Git ref to check out (branch name, SHA, FETCH_HEAD, etc.) */ + ref: string; + /** Absolute path where the worktree will be created. */ + path: string; + /** Create in detached HEAD mode (no branch). Default: false. */ + detach?: boolean; + /** CWD of the source repository. Defaults to process.cwd(). */ + cwd?: string; +} + +export interface RemoveWorktreeOptions { + /** Force removal even if the worktree has modifications. Default: false. */ + force?: boolean; + /** CWD of the source repository. Required if the worktree was created from a different cwd. */ + cwd?: string; +} + +// --------------------------------------------------------------------------- +// Primitives +// --------------------------------------------------------------------------- + +/** + * Fetch a ref from origin. + * Runs: `git fetch origin ` + * Throws on failure. + */ +export async function fetchRef( + runtime: ReviewGitRuntime, + ref: string, + options?: { cwd?: string }, +): Promise { + const result = await runtime.runGit(["fetch", "origin", ref], { cwd: options?.cwd }); + if (result.exitCode !== 0) { + throw new Error(`git fetch origin ${ref} failed: ${result.stderr.trim() || `exit code ${result.exitCode}`}`); + } +} + +/** + * Ensure a git object (commit SHA) is available locally. + * Checks with `git cat-file -t`, fetches from origin if missing. + * Returns true if the object is available after the attempt. + */ +export async function ensureObjectAvailable( + runtime: ReviewGitRuntime, + sha: string, + options?: { cwd?: string }, +): Promise { + const check = await runtime.runGit(["cat-file", "-t", sha], { cwd: options?.cwd }); + if (check.exitCode === 0) return true; + + // Object missing locally — try fetching it + const fetch = await runtime.runGit(["fetch", "origin", sha], { cwd: options?.cwd }); + if (fetch.exitCode !== 0) return false; + + // Verify it's now available + const recheck = await runtime.runGit(["cat-file", "-t", sha], { cwd: options?.cwd }); + return recheck.exitCode === 0; +} + +/** + * Create a git worktree. + * Runs: `git worktree add [--detach] ` + * Throws on failure with a descriptive error. + */ +export async function createWorktree( + runtime: ReviewGitRuntime, + options: CreateWorktreeOptions, +): Promise<{ worktreePath: string }> { + const args = ["worktree", "add"]; + if (options.detach) args.push("--detach"); + args.push(options.path, options.ref); + + const result = await runtime.runGit(args, { cwd: options.cwd }); + if (result.exitCode !== 0) { + throw new Error(`git worktree add failed: ${result.stderr.trim() || `exit code ${result.exitCode}`}`); + } + + return { worktreePath: options.path }; +} + +/** + * Remove a git worktree. Best-effort — logs errors but does not throw. + * Runs: `git worktree remove [--force] ` + */ +export async function removeWorktree( + runtime: ReviewGitRuntime, + worktreePath: string, + options?: RemoveWorktreeOptions, +): Promise { + const args = ["worktree", "remove"]; + if (options?.force) args.push("--force"); + args.push(worktreePath); + + try { + const result = await runtime.runGit(args, { cwd: options?.cwd }); + if (result.exitCode !== 0) { + console.error(`Warning: git worktree remove failed for ${worktreePath}: ${result.stderr.trim()}`); + } + } catch (err) { + console.error(`Warning: worktree cleanup error: ${err instanceof Error ? err.message : String(err)}`); + } +} diff --git a/packages/ui/components/AgentsTab.tsx b/packages/ui/components/AgentsTab.tsx index 65e9eed1..88e98461 100644 --- a/packages/ui/components/AgentsTab.tsx +++ b/packages/ui/components/AgentsTab.tsx @@ -112,12 +112,12 @@ function JobCard({ return (
onViewDetails() : (isTerminal ? onToggle : undefined)} >
@@ -142,20 +142,6 @@ function JobCard({
- {onViewDetails && ( - - )} {!isTerminal && (
- {/* Error details */} - {job.status === 'failed' && job.error && expanded && ( + {/* Error details — fallback for when dockview detail panel is not available */} + {!onViewDetails && job.status === 'failed' && job.error && expanded && (
             {job.error}
@@ -247,9 +233,7 @@ export const AgentsTab: React.FC = ({
     const provider = availableProviders.find((p) => p.id === selectedProvider);
     onLaunch({
       provider: selectedProvider,
-      // Stub command — will be replaced with real agent invocation
-      command: ['sleep', String(10 + Math.floor(Math.random() * 21))],
-      label: provider ? `${provider.name}` : selectedProvider,
+      label: provider ? `${provider.name} Review` : selectedProvider,
     });
   };
 
diff --git a/packages/ui/components/ConfirmDialog.tsx b/packages/ui/components/ConfirmDialog.tsx
index 4b100535..dafb88c5 100644
--- a/packages/ui/components/ConfirmDialog.tsx
+++ b/packages/ui/components/ConfirmDialog.tsx
@@ -15,6 +15,7 @@ export interface ConfirmDialogProps {
   cancelText?: string;
   variant?: 'info' | 'warning';
   showCancel?: boolean;
+  wide?: boolean;
 }
 
 export const ConfirmDialog: React.FC = ({
@@ -28,6 +29,7 @@ export const ConfirmDialog: React.FC = ({
   cancelText = 'Cancel',
   variant = 'info',
   showCancel = false,
+  wide = false,
 }) => {
   if (!isOpen) return null;
 
@@ -56,7 +58,7 @@ export const ConfirmDialog: React.FC = ({
 
   return (
     
-
+
{icons[variant]} diff --git a/packages/ui/hooks/useAgentJobs.ts b/packages/ui/hooks/useAgentJobs.ts index dce7b087..016ef3be 100644 --- a/packages/ui/hooks/useAgentJobs.ts +++ b/packages/ui/hooks/useAgentJobs.ts @@ -20,6 +20,7 @@ const CAPABILITIES_URL = '/api/agents/capabilities'; interface UseAgentJobsReturn { jobs: AgentJobInfo[]; + jobLogs: Map; capabilities: AgentCapabilities | null; launchJob: (params: { provider?: string; command?: string[]; label?: string }) => Promise; killJob: (id: string) => Promise; @@ -31,6 +32,7 @@ export function useAgentJobs( ): UseAgentJobsReturn { const enabled = options?.enabled ?? true; const [jobs, setJobs] = useState([]); + const [jobLogs, setJobLogs] = useState>(new Map()); const [capabilities, setCapabilities] = useState(null); const versionRef = useRef(0); const fallbackRef = useRef(false); @@ -83,6 +85,13 @@ export function useAgentJobs( prev.map((j) => (j.id === parsed.job.id ? parsed.job : j)), ); break; + case 'job:log': + setJobLogs((prev) => { + const next = new Map(prev); + next.set(parsed.jobId, (prev.get(parsed.jobId) ?? '') + parsed.delta); + return next; + }); + break; case 'jobs:cleared': // No-op: killAll() already broadcasts individual job:completed events // for each killed job, so the UI updates incrementally. @@ -189,5 +198,5 @@ export function useAgentJobs( } }, []); - return { jobs, capabilities, launchJob, killJob, killAll }; + return { jobs, jobLogs, capabilities, launchJob, killJob, killAll }; } From 9efc16a2502f7735eabd5b9d8d85e68a63cd09b0 Mon Sep 17 00:00:00 2001 From: Michael Ramos Date: Sun, 5 Apr 2026 10:31:50 -0700 Subject: [PATCH 02/37] =?UTF-8?q?style(review):=20UX=20polish=20pass=20?= =?UTF-8?q?=E2=80=94=20visual=20quality=20improvements=20across=20code=20r?= =?UTF-8?q?eview=20UI?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - VerdictCard: remove AI-template left-border, use background-only tint - Inline annotations: 6px radius, subtle shadow, hover elevation, action button scale - File tree: tighter indentation (4 + depth*10), reduced container padding - Select dropdowns: normalized to 4px border-radius matching pierre diffs - Dockview tabs: close button pushed to far right with margin-left auto, visible at 0.25 opacity - PR icons moved from sidebar to header (next to PR link) - AnnotationRow: translate-x hover feedback - Border-radius normalized to `rounded` (4px) across all components - Type scale consolidated: text-[8px]/[9px] → text-[10px], text-[11px] → text-xs - Sidebar tab hit targets increased (px-2.5 py-1.5, w-4 icons) - Sticky file group headers in annotation sidebar - FileTree controls collapsed (worktree + diff selectors share one row) - Tab micro-animations (transition-all duration-150) - ScrollFade component for gradient indicators on scrollable containers - Prose containment: max-w-2xl + px-6 padding on PR Summary/Comments/Checks panels - MarkdownBody: leading-relaxed for comfortable reading - PR Comments: surface lift (bg-muted/10), hover feedback, author font-semibold - PR Checks: link affordance (text-primary, underline on hover, external link icon) For provenance purposes, this commit was AI assisted. --- bun.lock | 6 +- packages/review-editor/App.tsx | 11 ++ .../review-editor/components/FileTree.tsx | 124 +++++++++--------- .../review-editor/components/FileTreeNode.tsx | 2 +- .../components/LiveLogViewer.tsx | 4 +- .../review-editor/components/PRChecksTab.tsx | 19 +-- .../components/PRCommentsTab.tsx | 26 ++-- .../review-editor/components/PRSummaryTab.tsx | 4 +- .../components/ReviewSidebar.tsx | 57 ++------ .../review-editor/components/ScrollFade.tsx | 54 ++++++++ .../dock/panels/ReviewAgentJobDetailPanel.tsx | 63 +++++---- packages/review-editor/index.css | 24 ++-- packages/ui/components/AgentsTab.tsx | 14 +- 13 files changed, 222 insertions(+), 186 deletions(-) create mode 100644 packages/review-editor/components/ScrollFade.tsx diff --git a/bun.lock b/bun.lock index 39dc4a2c..4ea77e06 100644 --- a/bun.lock +++ b/bun.lock @@ -62,7 +62,7 @@ }, "apps/opencode-plugin": { "name": "@plannotator/opencode", - "version": "0.16.4", + "version": "0.16.6", "dependencies": { "@opencode-ai/plugin": "^1.1.10", }, @@ -84,7 +84,7 @@ }, "apps/pi-extension": { "name": "@plannotator/pi-extension", - "version": "0.16.4", + "version": "0.16.6", "peerDependencies": { "@mariozechner/pi-coding-agent": ">=0.53.0", }, @@ -170,7 +170,7 @@ }, "packages/server": { "name": "@plannotator/server", - "version": "0.16.4", + "version": "0.16.6", "dependencies": { "@plannotator/ai": "workspace:*", "@plannotator/shared": "workspace:*", diff --git a/packages/review-editor/App.tsx b/packages/review-editor/App.tsx index 5e015669..eb925ffb 100644 --- a/packages/review-editor/App.tsx +++ b/packages/review-editor/App.tsx @@ -1334,6 +1334,17 @@ const ReviewApp: React.FC = () => { {mrNumberLabel} {prMetadata.title} +
+ + + +
) : repoInfo ? (
diff --git a/packages/review-editor/components/FileTree.tsx b/packages/review-editor/components/FileTree.tsx index 54969dc7..23c93798 100644 --- a/packages/review-editor/components/FileTree.tsx +++ b/packages/review-editor/components/FileTree.tsx @@ -270,7 +270,7 @@ export const FileTree: React.FC = ({ } }} placeholder="Search diff..." - className="w-full pl-7 py-1.5 pr-7 bg-muted rounded-md text-xs text-foreground placeholder:text-muted-foreground/50 focus:outline-none focus:ring-1 focus:ring-primary/50" + className="w-full pl-7 py-1.5 pr-7 bg-muted rounded text-xs text-foreground placeholder:text-muted-foreground/50 focus:outline-none focus:ring-1 focus:ring-primary/50" />
{searchQuery.trim() && !isSearchPending && ( @@ -292,74 +292,68 @@ export const FileTree: React.FC = ({
)} - {/* Worktree context switcher — only shown when worktrees exist */} - {worktrees && worktrees.length > 0 && onSelectWorktree && ( -
-
Context
-
- -
- - - -
-
-
- )} - - {/* Diff type selector — always the same base options */} - {diffOptions && diffOptions.length > 0 && onSelectDiff && ( -
- {worktrees && worktrees.length > 0 && ( -
View
- )} -
- -
- {isLoadingDiff ? ( - - - - - ) : ( + {/* Worktree + diff selectors — combined row when both present */} + {((worktrees && worktrees.length > 0 && onSelectWorktree) || (diffOptions && diffOptions.length > 0 && onSelectDiff)) && ( +
+ {worktrees && worktrees.length > 0 && onSelectWorktree && ( +
+ +
- )} +
-
+ )} + {diffOptions && diffOptions.length > 0 && onSelectDiff && ( +
+ +
+ {isLoadingDiff ? ( + + + + + ) : ( + + + + )} +
+
+ )}
)} {/* File tree or search results */} -
+
{searchQuery.trim() ? ( isSearchPending ? (
@@ -417,7 +411,7 @@ export const FileTree: React.FC = ({ ); diff --git a/packages/review-editor/components/FileTreeNode.tsx b/packages/review-editor/components/FileTreeNode.tsx index f5163888..d197168f 100644 --- a/packages/review-editor/components/FileTreeNode.tsx +++ b/packages/review-editor/components/FileTreeNode.tsx @@ -45,7 +45,7 @@ export const FileTreeNodeItem: React.FC = ({ getAnnotationCount, stagedFiles, }) => { - const paddingLeft = 8 + node.depth * 12; + const paddingLeft = 4 + node.depth * 10; if (node.type === 'folder') { if (!hasVisibleChildren(node, viewedFiles, activeFileIndex, hideViewedFiles)) { diff --git a/packages/review-editor/components/LiveLogViewer.tsx b/packages/review-editor/components/LiveLogViewer.tsx index 13297ebe..52b037ba 100644 --- a/packages/review-editor/components/LiveLogViewer.tsx +++ b/packages/review-editor/components/LiveLogViewer.tsx @@ -59,14 +59,14 @@ export const LiveLogViewer: React.FC = ({
{!content && isLive ? ( Waiting for output... ) : ( -
+          
             {displayText}
             {isLive && content && (
               
diff --git a/packages/review-editor/components/PRChecksTab.tsx b/packages/review-editor/components/PRChecksTab.tsx
index f1f0f21f..f5229d2c 100644
--- a/packages/review-editor/components/PRChecksTab.tsx
+++ b/packages/review-editor/components/PRChecksTab.tsx
@@ -94,7 +94,7 @@ export const PRChecksTab: React.FC = ({ context }) => {
   const mergeableConflict = !isMerged && !isClosed && context.mergeable === 'CONFLICTING';
 
   return (
-    
+
{/* Merge readiness */}

@@ -104,7 +104,7 @@ export const PRChecksTab: React.FC = ({ context }) => {
{/* Review decision */} {decisionStyle && ( -
+
{decisionStyle.label} @@ -112,7 +112,7 @@ export const PRChecksTab: React.FC = ({ context }) => { )} {/* Merge state */} -
+
{mergeableConflict ? 'Has merge conflicts' : mergeStyle.label} @@ -134,23 +134,26 @@ export const PRChecksTab: React.FC = ({ context }) => {
-
+
{Array.from(groupedChecks.entries()).map(([workflow, checks]) => (
-
{workflow}
-
+
{workflow}
+
{checks.map((check, i) => ( - + {check.name} + + + ))}
diff --git a/packages/review-editor/components/PRCommentsTab.tsx b/packages/review-editor/components/PRCommentsTab.tsx index a6d13122..df09e5dd 100644 --- a/packages/review-editor/components/PRCommentsTab.tsx +++ b/packages/review-editor/components/PRCommentsTab.tsx @@ -68,29 +68,29 @@ export const PRCommentsTab: React.FC = ({ context }) => { } return ( -
+
{timeline.map((entry) => { if (entry.kind === 'review') { const review = entry.data; const style = REVIEW_STATE_STYLES[review.state] ?? REVIEW_STATE_STYLES.COMMENTED; return ( -
-
-
- +
+
+
+ {review.author || 'unknown'} - + {style.label}
- + {formatRelativeTime(review.submittedAt)}
{review.body && ( -
+
)} @@ -100,16 +100,16 @@ export const PRCommentsTab: React.FC = ({ context }) => { const comment = entry.data; return ( -
-
- +
+
+ {comment.author || 'unknown'} - + {formatRelativeTime(comment.createdAt)}
-
+
diff --git a/packages/review-editor/components/PRSummaryTab.tsx b/packages/review-editor/components/PRSummaryTab.tsx index fc130aac..3066a83d 100644 --- a/packages/review-editor/components/PRSummaryTab.tsx +++ b/packages/review-editor/components/PRSummaryTab.tsx @@ -45,7 +45,7 @@ export function MarkdownBody({ markdown }: { markdown: string }) { const blocks = useMemo(() => parseMarkdownToBlocks(markdown), [markdown]); return ( -
+
{blocks.map((block) => { switch (block.type) { case 'heading': { @@ -97,7 +97,7 @@ export function MarkdownBody({ markdown }: { markdown: string }) { export const PRSummaryTab: React.FC = ({ context, metadata }) => { return ( -
+
{/* PR title + state */}
diff --git a/packages/review-editor/components/ReviewSidebar.tsx b/packages/review-editor/components/ReviewSidebar.tsx index a08e5353..884a1d7f 100644 --- a/packages/review-editor/components/ReviewSidebar.tsx +++ b/packages/review-editor/components/ReviewSidebar.tsx @@ -226,14 +226,14 @@ export const ReviewSidebar: React.FC = ({ {/* Annotations tab (always visible) */} @@ -242,14 +242,14 @@ export const ReviewSidebar: React.FC = ({ {aiAvailable && ( )} - {/* PR tabs — open as center dock panels */} - {!!prMetadata && ( - <> - - - - - )} + {/* PR tabs moved to header — see App.tsx */}
@@ -330,7 +299,7 @@ export const ReviewSidebar: React.FC = ({
{Array.from(groupedAnnotations.entries()).map(([filePath, fileAnnotations]) => (
-
+
{filePath.split('/').pop()}
@@ -341,10 +310,10 @@ export const ReviewSidebar: React.FC = ({
onSelectAnnotation(annotation.id)} - className={`group relative p-2.5 rounded-lg border cursor-pointer transition-all ${ + className={`group relative p-2.5 rounded border cursor-pointer transition-colors duration-150 ${ isSelected - ? 'bg-primary/5 border-primary/30 shadow-sm' - : 'border-transparent hover:bg-muted/50 hover:border-border/50' + ? 'bg-primary/5 border-primary/30' + : 'border-transparent hover:bg-muted/30' }`} >
@@ -385,7 +354,7 @@ export const ReviewSidebar: React.FC = ({ e.stopPropagation(); onDeleteAnnotation(annotation.id); }} - className="absolute top-2 right-2 p-1 rounded-md opacity-0 group-hover:opacity-100 hover:bg-destructive/10 text-muted-foreground hover:text-destructive transition-all" + className="absolute top-2 right-2 p-1 rounded opacity-0 group-hover:opacity-100 hover:bg-destructive/10 text-muted-foreground hover:text-destructive transition-all" title="Delete annotation" > @@ -463,7 +432,7 @@ export const ReviewSidebar: React.FC = ({
{ann.text && ( -

+

{ann.text}

)} @@ -333,7 +332,7 @@ function AnnotationRow({ annotation: ann, dismissed, onClick }: { function EmptyState({ terminal }: { terminal: boolean }) { return (
-

+

{terminal ? 'No findings were produced.' : 'Findings will appear as the agent works.'}

diff --git a/packages/review-editor/index.css b/packages/review-editor/index.css index c8511761..e73fa734 100644 --- a/packages/review-editor/index.css +++ b/packages/review-editor/index.css @@ -56,8 +56,9 @@ .dv-review-tab { display: flex; align-items: center; - gap: 0.375rem; - padding: 0 0.5rem; + gap: 0.5rem; + padding: 0 0.625rem; + width: 100%; height: 100%; font-size: 0.6875rem; user-select: none; @@ -73,13 +74,14 @@ .dv-review-tab-close { flex-shrink: 0; + margin-left: auto; display: flex; align-items: center; justify-content: center; width: 1rem; height: 1rem; border-radius: 3px; - opacity: 0; + opacity: 0.25; color: var(--muted-foreground); transition: opacity 150ms, background 150ms, color 150ms; } @@ -128,16 +130,18 @@ diffs-container { min-width: 0; background: var(--popover); border: 1px solid var(--border); - border-radius: 2px; - padding: 0.5rem 0.625rem; + border-radius: 6px; + padding: 0.625rem 0.75rem; font-size: 0.8125rem; margin: 0.5rem 0; cursor: pointer; - transition: border-color 150ms ease; + box-shadow: 0 1px 2px rgba(0,0,0,0.04); + transition: border-color 150ms, box-shadow 150ms, background 150ms; } .review-comment:hover { border-color: var(--muted-foreground); + box-shadow: 0 2px 6px rgba(0,0,0,0.06); } .review-comment-header { @@ -155,18 +159,20 @@ diffs-container { display: flex; gap: 0.125rem; opacity: 0; - transition: opacity 150ms; + transform: scale(0.95); + transition: opacity 100ms, transform 100ms; } .review-comment:hover .review-comment-actions { opacity: 1; + transform: scale(1); } .review-comment-action { padding: 0.25rem; - border-radius: 2px; + border-radius: 4px; color: var(--muted-foreground); - transition: color 150ms, background 150ms; + transition: color 100ms, background 100ms; } .review-comment-action:hover { diff --git a/packages/ui/components/AgentsTab.tsx b/packages/ui/components/AgentsTab.tsx index 88e98461..cea96b8b 100644 --- a/packages/ui/components/AgentsTab.tsx +++ b/packages/ui/components/AgentsTab.tsx @@ -85,7 +85,7 @@ function ProviderBadge({ provider }: { provider: string }) { provider === 'codex' ? 'Codex' : 'Shell'; return ( - + {label} ); @@ -112,7 +112,7 @@ function JobCard({ return (
@@ -247,7 +247,7 @@ export const AgentsTab: React.FC = ({ ) : ( - + {availableProviders[0]?.name} )} @@ -307,7 +307,7 @@ export const AgentsTab: React.FC = ({
+ )} +
+
+ + {/* Controls row */} +
+ {/* Author pills — click to exclude, click again to re-include */} +
+ {excludedAuthors.size > 0 && ( + + )} + {uniqueAuthors.map((author) => ( + { + setExcludedAuthors((prev) => { + const next = new Set(prev); + if (next.has(author)) next.delete(author); else next.add(author); + return next; + }); + }} + /> + ))} +
+ + {/* Sort + collapse controls */} +
+ + +
- ); - })} +
+
+ + {/* ── Timeline ── */} +
+
+ {displayTimeline.length === 0 ? ( +
+

No comments match your filters.

+ +
+ ) : ( + displayTimeline.map((entry) => { + const id = entry.data.id; + const isSelected = selectedId === id; + const isCollapsed = collapsedIds.has(id); + const isReview = entry.kind === 'review'; + const review = isReview ? (entry.data as PRReview) : null; + // Only show badge for meaningful review states — "COMMENTED" is noise + const style = review && review.state !== 'COMMENTED' + ? (REVIEW_STATE_STYLES[review.state] ?? null) + : null; + + return ( +
setSelectedId(isSelected ? null : id)} + className={`group/card rounded bg-muted/10 border p-3 cursor-pointer transition-all duration-150 ${ + isSelected + ? 'border-primary/20 bg-primary/5 ring-1 ring-primary/10' + : 'border-border/30 hover:bg-muted/20' + }`} + > + {/* Header — always visible, click to collapse */} +
{ e.stopPropagation(); toggleCollapsed(id); }} + > +
+ + {entry.data.author || 'unknown'} + + {style && ( + + {style.label} + + )} +
+
+ + {formatRelativeTime(getEntryTime(entry))} + + + + +
+
+ + {/* Body — hidden when collapsed */} + {!isCollapsed && entry.data.body && ( +
+ +
+ )} + + {/* Actions — hover reveal */} + {!isCollapsed && ( + + )} +
+ ); + }) + )} +
+
); }; + +// --------------------------------------------------------------------------- +// Subcomponents +// --------------------------------------------------------------------------- + +function CommentActions({ url, body }: { url?: string; body: string }) { + const [copied, setCopied] = useState(false); + + const handleCopy = (e: React.MouseEvent) => { + e.stopPropagation(); + navigator.clipboard.writeText(body).then(() => { + setCopied(true); + setTimeout(() => setCopied(false), 1500); + }).catch(() => {}); + }; + + if (!url && !body) return null; + + return ( + + ); +} + +function AuthorPill({ label, excluded, onClick }: { label: string; excluded: boolean; onClick: () => void }) { + return ( + + ); +} diff --git a/packages/review-editor/components/PRSummaryTab.tsx b/packages/review-editor/components/PRSummaryTab.tsx index 3066a83d..7f39c3ce 100644 --- a/packages/review-editor/components/PRSummaryTab.tsx +++ b/packages/review-editor/components/PRSummaryTab.tsx @@ -133,11 +133,10 @@ export const PRSummaryTab: React.FC = ({ context, metadata }) {context.labels.map((label) => ( {label.name} @@ -160,9 +159,8 @@ export const PRSummaryTab: React.FC = ({ context, metadata }) rel="noopener noreferrer" className="flex items-center gap-1.5 text-xs text-primary hover:underline" > - - - + + #{issue.number} {issue.repo && ({issue.repo})} diff --git a/packages/review-editor/dock/ReviewStateContext.tsx b/packages/review-editor/dock/ReviewStateContext.tsx index b173455a..300d6e72 100644 --- a/packages/review-editor/dock/ReviewStateContext.tsx +++ b/packages/review-editor/dock/ReviewStateContext.tsx @@ -76,6 +76,7 @@ export interface ReviewState { isPRContextLoading: boolean; prContextError: string | null; fetchPRContext: () => void; + platformUser: string | null; // Diff navigation openDiffFile: (filePath: string) => void; diff --git a/packages/review-editor/dock/panels/ReviewPRCommentsPanel.tsx b/packages/review-editor/dock/panels/ReviewPRCommentsPanel.tsx index 110710e0..ace339f0 100644 --- a/packages/review-editor/dock/panels/ReviewPRCommentsPanel.tsx +++ b/packages/review-editor/dock/panels/ReviewPRCommentsPanel.tsx @@ -7,7 +7,7 @@ import { PRCommentsTab } from '../../components/PRCommentsTab'; * Dock panel wrapper for PR Comments. */ export const ReviewPRCommentsPanel: React.FC = () => { - const { prContext, isPRContextLoading, prContextError, fetchPRContext } = useReviewState(); + const { prContext, isPRContextLoading, prContextError, fetchPRContext, platformUser } = useReviewState(); useEffect(() => { if (!prContext && !prContextError && !isPRContextLoading) fetchPRContext(); @@ -39,8 +39,8 @@ export const ReviewPRCommentsPanel: React.FC = () => { if (!prContext) return null; return ( -
- +
+
); }; diff --git a/packages/shared/pr-github.ts b/packages/shared/pr-github.ts index 958e2606..30a10c04 100644 --- a/packages/shared/pr-github.ts +++ b/packages/shared/pr-github.ts @@ -161,6 +161,7 @@ function parseGhPRContext(raw: Record): PRContext { state: str(r?.state), body: str(r?.body), submittedAt: str(r?.submittedAt), + ...(r?.url ? { url: str(r.url) } : {}), })), checks: arr(raw.statusCheckRollup).map((c: any) => ({ name: str(c?.name), diff --git a/packages/shared/pr-provider.ts b/packages/shared/pr-provider.ts index a8407cd5..675b8db4 100644 --- a/packages/shared/pr-provider.ts +++ b/packages/shared/pr-provider.ts @@ -107,6 +107,7 @@ export interface PRReview { state: string; body: string; submittedAt: string; + url?: string; } export interface PRCheck { From 0c41712b1f387e20b1ed77f444df61eef3f8ceb7 Mon Sep 17 00:00:00 2001 From: Michael Ramos Date: Sun, 5 Apr 2026 12:01:39 -0700 Subject: [PATCH 04/37] feat(review): inline review threads with outdated/resolved state and diff hunk previews PR Review Threads: - Fetch inline code review comments via GitHub GraphQL (reviewThreads query) - PRReviewThread and PRThreadComment types with isResolved, isOutdated, path, line, diffSide - ThreadCard component: file/line context, Outdated/Resolved badges, nested replies - Resolved/outdated threads: gradient fade on body with "Show full comment" expand - GitLab: reviewThreads placeholder (TODO: parse DiffNote positions from notes) DiffHunkPreview: - Renders diff hunks using @pierre/diffs FileDiff component (read-only, compact) - Full theme integration: reads computed CSS vars, injects via unsafeCSS (same as main DiffViewer) - Respects user font settings from ReviewState context - Handles bare GitHub diffHunk format (prepends synthetic file headers for pierre parsing) Comments Panel Polish: - Comment cards: bg-card + subtle shadow for depth and isolation from panel background - Hover: shadow elevation (0_2px_6px) for interactive feedback - Thread cards: dimmed shadow for resolved/outdated, full shadow for active - Prose padding: px-8 (32px) across all dockview panels (Summary, Comments, Checks, Findings) For provenance purposes, this commit was AI assisted. --- .../components/DiffHunkPreview.tsx | 115 +++++++++++ .../review-editor/components/PRChecksTab.tsx | 2 +- .../components/PRCommentsTab.tsx | 189 ++++++++++++++++-- .../review-editor/components/PRSummaryTab.tsx | 2 +- .../dock/panels/ReviewAgentJobDetailPanel.tsx | 2 +- packages/shared/pr-github.ts | 85 +++++++- packages/shared/pr-gitlab.ts | 1 + packages/shared/pr-provider.ts | 21 ++ 8 files changed, 396 insertions(+), 21 deletions(-) create mode 100644 packages/review-editor/components/DiffHunkPreview.tsx diff --git a/packages/review-editor/components/DiffHunkPreview.tsx b/packages/review-editor/components/DiffHunkPreview.tsx new file mode 100644 index 00000000..20d1eb06 --- /dev/null +++ b/packages/review-editor/components/DiffHunkPreview.tsx @@ -0,0 +1,115 @@ +import React, { useMemo, useState, useEffect } from 'react'; +import { FileDiff } from '@pierre/diffs/react'; +import { getSingularPatch } from '@pierre/diffs'; +import { useTheme } from '@plannotator/ui/components/ThemeProvider'; +import { useReviewState } from '../dock/ReviewStateContext'; + +interface DiffHunkPreviewProps { + /** Raw diff hunk string (unified diff format). */ + hunk: string; + /** Max height in pixels before "Show more" toggle. Default 128. */ + maxHeight?: number; + className?: string; +} + +/** + * Renders a small inline diff hunk using @pierre/diffs. + * Compact, read-only, no file header. Shares theme + font settings + * with the main DiffViewer via the same unsafeCSS injection pattern. + */ +export const DiffHunkPreview: React.FC = ({ + hunk, + maxHeight = 128, + className, +}) => { + const { resolvedMode } = useTheme(); + const state = useReviewState(); + const [expanded, setExpanded] = useState(false); + + const fileDiff = useMemo(() => { + if (!hunk) return undefined; + try { + const needsHeaders = !hunk.startsWith('diff --git') && !hunk.startsWith('--- '); + const patch = needsHeaders + ? `diff --git a/file b/file\n--- a/file\n+++ b/file\n${hunk}` + : hunk; + return getSingularPatch(patch); + } catch { + return undefined; + } + }, [hunk]); + + // Theme injection — same pattern as DiffViewer (reads computed CSS vars, injects via unsafeCSS) + const [pierreTheme, setPierreTheme] = useState<{ type: 'dark' | 'light'; css: string }>({ + type: resolvedMode, + css: '', + }); + + useEffect(() => { + requestAnimationFrame(() => { + const styles = getComputedStyle(document.documentElement); + const bg = styles.getPropertyValue('--background').trim(); + const fg = styles.getPropertyValue('--foreground').trim(); + if (!bg || !fg) return; + + const fontFamily = state.fontFamily; + const fontSize = state.fontSize; + const fontCSS = fontFamily || fontSize ? ` + pre, code, [data-line-content], [data-column-number] { + ${fontFamily ? `font-family: '${fontFamily}', monospace !important;` : ''} + ${fontSize ? `font-size: ${fontSize} !important; line-height: 1.5 !important;` : ''} + }` : ''; + + setPierreTheme({ + type: resolvedMode, + css: ` + :host, [data-diff], [data-file], [data-diffs-header], [data-error-wrapper], [data-virtualizer-buffer] { + --diffs-bg: ${bg} !important; + --diffs-fg: ${fg} !important; + --diffs-dark-bg: ${bg}; + --diffs-light-bg: ${bg}; + --diffs-dark: ${fg}; + --diffs-light: ${fg}; + } + pre, code { background-color: ${bg} !important; } + [data-column-number] { background-color: ${bg} !important; } + [data-file-info] { display: none !important; } + [data-diffs-header] { display: none !important; } + ${fontCSS} + `, + }); + }); + }, [resolvedMode, state.fontFamily, state.fontSize]); + + if (!fileDiff) return null; + + return ( +
+
+ +
+ {!expanded && ( + + )} +
+ ); +}; diff --git a/packages/review-editor/components/PRChecksTab.tsx b/packages/review-editor/components/PRChecksTab.tsx index f5229d2c..ff785d70 100644 --- a/packages/review-editor/components/PRChecksTab.tsx +++ b/packages/review-editor/components/PRChecksTab.tsx @@ -94,7 +94,7 @@ export const PRChecksTab: React.FC = ({ context }) => { const mergeableConflict = !isMerged && !isClosed && context.mergeable === 'CONFLICTING'; return ( -
+
{/* Merge readiness */}

diff --git a/packages/review-editor/components/PRCommentsTab.tsx b/packages/review-editor/components/PRCommentsTab.tsx index 48ca505c..e99dec1d 100644 --- a/packages/review-editor/components/PRCommentsTab.tsx +++ b/packages/review-editor/components/PRCommentsTab.tsx @@ -1,6 +1,7 @@ import React, { useMemo, useState, useEffect, useRef, useCallback } from 'react'; -import type { PRContext, PRComment, PRReview } from '@plannotator/shared/pr-provider'; +import type { PRContext, PRComment, PRReview, PRReviewThread } from '@plannotator/shared/pr-provider'; import { MarkdownBody } from './PRSummaryTab'; +import { DiffHunkPreview } from './DiffHunkPreview'; // --------------------------------------------------------------------------- // Types @@ -13,7 +14,8 @@ interface PRCommentsTabProps { type TimelineEntry = | { kind: 'comment'; data: PRComment } - | { kind: 'review'; data: PRReview }; + | { kind: 'review'; data: PRReview } + | { kind: 'thread'; data: PRReviewThread }; const REVIEW_STATE_STYLES: Record = { APPROVED: { bg: 'bg-success/15', text: 'text-success', label: 'Approved' }, @@ -42,12 +44,29 @@ function formatRelativeTime(iso: string): string { } function getEntryTime(entry: TimelineEntry): string { - return entry.kind === 'comment' ? entry.data.createdAt : (entry.data as PRReview).submittedAt; + if (entry.kind === 'comment') return entry.data.createdAt; + if (entry.kind === 'review') return entry.data.submittedAt; + return entry.data.comments[0]?.createdAt ?? ''; +} + +function getEntryAuthor(entry: TimelineEntry): string { + if (entry.kind === 'thread') return entry.data.comments[0]?.author ?? ''; + return entry.data.author; +} + +function getEntryBody(entry: TimelineEntry): string { + if (entry.kind === 'thread') return entry.data.comments.map((c) => c.body).join(' '); + return entry.data.body; } function matchesSearch(entry: TimelineEntry, query: string): boolean { const q = query.toLowerCase(); - return entry.data.author.toLowerCase().includes(q) || entry.data.body.toLowerCase().includes(q); + const author = getEntryAuthor(entry).toLowerCase(); + const body = getEntryBody(entry).toLowerCase(); + if (entry.kind === 'thread') { + return author.includes(q) || body.includes(q) || entry.data.path.toLowerCase().includes(q); + } + return author.includes(q) || body.includes(q); } function isTypingTarget(target: EventTarget | null): boolean { @@ -78,20 +97,23 @@ export const PRCommentsTab: React.FC = ({ context, platformU ...context.reviews .filter((r) => r.state !== 'COMMENTED' || r.body) .map((r): TimelineEntry => ({ kind: 'review', data: r })), + ...(context.reviewThreads ?? []) + .filter((t) => t.comments.length > 0) + .map((t): TimelineEntry => ({ kind: 'thread', data: t })), ]; entries.sort((a, b) => new Date(getEntryTime(a)).getTime() - new Date(getEntryTime(b)).getTime()); return entries; - }, [context.comments, context.reviews]); + }, [context.comments, context.reviews, context.reviewThreads]); const uniqueAuthors = useMemo( - () => [...new Set(baseTimeline.map((e) => e.data.author))].sort(), + () => [...new Set(baseTimeline.map((e) => getEntryAuthor(e)).filter(Boolean))].sort(), [baseTimeline], ); const filteredTimeline = useMemo(() => { let result = baseTimeline; if (excludedAuthors.size > 0) { - result = result.filter((e) => !excludedAuthors.has(e.data.author)); + result = result.filter((e) => !excludedAuthors.has(getEntryAuthor(e))); } if (searchQuery.trim()) { result = result.filter((e) => matchesSearch(e, searchQuery.trim())); @@ -190,7 +212,7 @@ export const PRCommentsTab: React.FC = ({ context, platformU // --- Empty state (no comments at all) --- if (baseTimeline.length === 0) { return ( -
+
@@ -206,7 +228,7 @@ export const PRCommentsTab: React.FC = ({ context, platformU return (
{/* ── Toolbar ── */} -
+
{/* Search */}
@@ -290,7 +312,7 @@ export const PRCommentsTab: React.FC = ({ context, platformU
{/* ── Timeline ── */} -
+
{displayTimeline.length === 0 ? (
@@ -304,9 +326,22 @@ export const PRCommentsTab: React.FC = ({ context, platformU const id = entry.data.id; const isSelected = selectedId === id; const isCollapsed = collapsedIds.has(id); + + if (entry.kind === 'thread') { + return ( + setSelectedId(isSelected ? null : id)} + onToggleCollapse={() => toggleCollapsed(id)} + /> + ); + } + const isReview = entry.kind === 'review'; const review = isReview ? (entry.data as PRReview) : null; - // Only show badge for meaningful review states — "COMMENTED" is noise const style = review && review.state !== 'COMMENTED' ? (REVIEW_STATE_STYLES[review.state] ?? null) : null; @@ -316,13 +351,12 @@ export const PRCommentsTab: React.FC = ({ context, platformU key={id} data-comment-id={id} onClick={() => setSelectedId(isSelected ? null : id)} - className={`group/card rounded bg-muted/10 border p-3 cursor-pointer transition-all duration-150 ${ + className={`group/card rounded bg-card border p-3 cursor-pointer transition-all duration-150 shadow-[0_1px_3px_rgba(0,0,0,0.06)] ${ isSelected ? 'border-primary/20 bg-primary/5 ring-1 ring-primary/10' - : 'border-border/30 hover:bg-muted/20' + : 'border-border/40 hover:shadow-[0_2px_6px_rgba(0,0,0,0.08)]' }`} > - {/* Header — always visible, click to collapse */}
{ e.stopPropagation(); toggleCollapsed(id); }} @@ -347,14 +381,12 @@ export const PRCommentsTab: React.FC = ({ context, platformU
- {/* Body — hidden when collapsed */} {!isCollapsed && entry.data.body && (
)} - {/* Actions — hover reveal */} {!isCollapsed && ( = ({ context, platformU // Subcomponents // --------------------------------------------------------------------------- +function ThreadCard({ thread, isSelected, isCollapsed, onSelect, onToggleCollapse }: { + thread: PRReviewThread; + isSelected: boolean; + isCollapsed: boolean; + onSelect: () => void; + onToggleCollapse: () => void; +}) { + const [isExpanded, setIsExpanded] = useState(false); + const first = thread.comments[0]; + if (!first) return null; + const replies = thread.comments.slice(1); + const isDimmed = thread.isResolved || thread.isOutdated; + const lineLabel = thread.startLine && thread.startLine !== thread.line + ? `L${thread.startLine}–${thread.line}` + : thread.line ? `L${thread.line}` : ''; + + return ( +
+ {/* Header */} +
{ e.stopPropagation(); onToggleCollapse(); }} + > +
+ + {first.author || 'unknown'} + + {thread.isOutdated && ( + + Outdated + + )} + {thread.isResolved && ( + + Resolved + + )} + {thread.path && ( + + {thread.path.split('/').pop()}{lineLabel ? `:${lineLabel}` : ''} + + )} +
+
+ {replies.length > 0 && ( + + {replies.length} repl{replies.length === 1 ? 'y' : 'ies'} + + )} + + {formatRelativeTime(first.createdAt)} + + + + +
+
+ + {/* Body */} + {!isCollapsed && ( + <> + {/* Diff hunk */} + {first.diffHunk && ( +
+ +
+ )} + + {/* First comment body — truncated with fade for resolved/outdated */} + {first.body && ( +
+
+ +
+ {isDimmed && !isExpanded && ( +
+ )} +
+ )} + + {/* Expand button for dimmed threads */} + {isDimmed && !isExpanded && ( + + )} + + {/* Replies — only shown when expanded or not dimmed */} + {(!isDimmed || isExpanded) && replies.length > 0 && ( +
+ {replies.map((reply) => ( +
+
+ {reply.author} + {formatRelativeTime(reply.createdAt)} +
+
+ +
+
+ ))} +
+ )} + + {/* Actions */} + + + )} +
+ ); +} + function CommentActions({ url, body }: { url?: string; body: string }) { const [copied, setCopied] = useState(false); diff --git a/packages/review-editor/components/PRSummaryTab.tsx b/packages/review-editor/components/PRSummaryTab.tsx index 7f39c3ce..22f40061 100644 --- a/packages/review-editor/components/PRSummaryTab.tsx +++ b/packages/review-editor/components/PRSummaryTab.tsx @@ -97,7 +97,7 @@ export function MarkdownBody({ markdown }: { markdown: string }) { export const PRSummaryTab: React.FC = ({ context, metadata }) => { return ( -
+
{/* PR title + state */}
diff --git a/packages/review-editor/dock/panels/ReviewAgentJobDetailPanel.tsx b/packages/review-editor/dock/panels/ReviewAgentJobDetailPanel.tsx index 175f1a21..c78b7ca0 100644 --- a/packages/review-editor/dock/panels/ReviewAgentJobDetailPanel.tsx +++ b/packages/review-editor/dock/panels/ReviewAgentJobDetailPanel.tsx @@ -121,7 +121,7 @@ export const ReviewAgentJobDetailPanel: React.FC = (props) {/* ── Content ── */} {activeTab === 'findings' ? ( -
+
{/* Verdict — scrolls with content */} diff --git a/packages/shared/pr-github.ts b/packages/shared/pr-github.ts index 30a10c04..86489525 100644 --- a/packages/shared/pr-github.ts +++ b/packages/shared/pr-github.ts @@ -4,7 +4,7 @@ * All functions use the `gh` CLI via the PRRuntime abstraction. */ -import type { PRRuntime, PRMetadata, PRContext, PRReviewFileComment, CommandResult } from "./pr-provider"; +import type { PRRuntime, PRMetadata, PRContext, PRReviewThread, PRThreadComment, PRReviewFileComment, CommandResult } from "./pr-provider"; import { encodeApiFilePath } from "./pr-provider"; // GitHub-specific PRRef shape (used internally) @@ -163,6 +163,7 @@ function parseGhPRContext(raw: Record): PRContext { submittedAt: str(r?.submittedAt), ...(r?.url ? { url: str(r.url) } : {}), })), + reviewThreads: [], // populated via GraphQL after initial fetch checks: arr(raw.statusCheckRollup).map((c: any) => ({ name: str(c?.name), status: str(c?.status), @@ -199,7 +200,87 @@ export async function fetchGhPRContext( } const raw = JSON.parse(result.stdout) as Record; - return parseGhPRContext(raw); + const context = parseGhPRContext(raw); + + // Fetch inline review threads via GraphQL (parallel-safe, non-blocking failure) + try { + context.reviewThreads = await fetchGhReviewThreads(runtime, ref); + } catch { + // GraphQL may not be available or may fail — degrade gracefully + context.reviewThreads = []; + } + + return context; +} + +// --- Review Threads (GraphQL) --- + +const REVIEW_THREADS_QUERY = ` +query($owner: String!, $repo: String!, $number: Int!) { + repository(owner: $owner, name: $repo) { + pullRequest(number: $number) { + reviewThreads(first: 100) { + nodes { + id + isResolved + isOutdated + line + startLine + path + diffSide + comments(first: 50) { + nodes { + id + body + author { login } + createdAt + url + diffHunk + } + } + } + } + } + } +}`; + +async function fetchGhReviewThreads( + runtime: PRRuntime, + ref: GhPRRef, +): Promise { + const result = await runtime.runCommand("gh", hostnameArgs(ref.host, [ + "api", "graphql", + "-f", `query=${REVIEW_THREADS_QUERY}`, + "-f", `owner=${ref.owner}`, + "-f", `repo=${ref.repo}`, + "-F", `number=${ref.number}`, + ])); + + if (result.exitCode !== 0) return []; + + const data = JSON.parse(result.stdout); + const threads = data?.data?.repository?.pullRequest?.reviewThreads?.nodes; + if (!Array.isArray(threads)) return []; + + return threads.map((t: any): PRReviewThread => ({ + id: String(t.id ?? ''), + isResolved: t.isResolved === true, + isOutdated: t.isOutdated === true, + path: String(t.path ?? ''), + line: typeof t.line === 'number' ? t.line : null, + startLine: typeof t.startLine === 'number' ? t.startLine : null, + diffSide: t.diffSide === 'LEFT' || t.diffSide === 'RIGHT' ? t.diffSide : null, + comments: Array.isArray(t.comments?.nodes) + ? t.comments.nodes.map((c: any): PRThreadComment => ({ + id: String(c.id ?? ''), + author: c.author?.login ? String(c.author.login) : '', + body: String(c.body ?? ''), + createdAt: String(c.createdAt ?? ''), + url: String(c.url ?? ''), + ...(c.diffHunk ? { diffHunk: String(c.diffHunk) } : {}), + })) + : [], + })); } // --- File Content --- diff --git a/packages/shared/pr-gitlab.ts b/packages/shared/pr-gitlab.ts index 905044de..66e0d4ca 100644 --- a/packages/shared/pr-gitlab.ts +++ b/packages/shared/pr-gitlab.ts @@ -342,6 +342,7 @@ export async function fetchGlMRContext( mergeStateStatus, comments: notes, reviews, + reviewThreads: [], // TODO: parse DiffNote positions from notes for thread support checks, linkedIssues, }; diff --git a/packages/shared/pr-provider.ts b/packages/shared/pr-provider.ts index 675b8db4..aac519c4 100644 --- a/packages/shared/pr-provider.ts +++ b/packages/shared/pr-provider.ts @@ -124,6 +124,26 @@ export interface PRLinkedIssue { repo: string; } +export interface PRThreadComment { + id: string; + author: string; + body: string; + createdAt: string; + url: string; + diffHunk?: string; +} + +export interface PRReviewThread { + id: string; + isResolved: boolean; + isOutdated: boolean; + path: string; + line: number | null; + startLine: number | null; + diffSide: 'LEFT' | 'RIGHT' | null; + comments: PRThreadComment[]; +} + export interface PRContext { body: string; state: string; @@ -134,6 +154,7 @@ export interface PRContext { mergeStateStatus: string; comments: PRComment[]; reviews: PRReview[]; + reviewThreads: PRReviewThread[]; checks: PRCheck[]; linkedIssues: PRLinkedIssue[]; } From 272573947c82a5760344d4cdae16e68b99374867 Mon Sep 17 00:00:00 2001 From: Michael Ramos Date: Sun, 5 Apr 2026 17:49:04 -0700 Subject: [PATCH 05/37] feat(review): Claude Code agent, cross-repo --local, render fixes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Claude Code review agent: - claude-review.ts: prompt (adapted from code-review plugin), command builder (dontAsk + granular allowedTools/disallowedTools), JSONL stream output parser - Prompt sent via stdin (not argv) to avoid quoting/variadic flag conflicts - stream-json --verbose for live JSONL streaming + final structured_output - Same schema as Codex — transformReviewFindings is now provider-agnostic Agent jobs infrastructure: - stdout capture (captureStdout option) for providers that return results on stdout - stdin prompt writing (stdinPrompt option) for providers that read prompt from stdin - cwd override in buildCommand return for providers without -C flag - await stdoutDone before onJobComplete to prevent drain race condition - job.prompt field for transparent prompt display in detail panel Cross-repo --local: - Detect same-repo vs cross-repo via parseRemoteUrl comparison - Cross-repo: shallow clone via gh/glab repo clone (--depth 1 --no-checkout + targeted fetch) - Cross-repo uses platform diff (gh pr diff) for display, clone for agent file access - Same-repo: existing worktree path unchanged - Cleanup: rmSync for clones, worktree remove for same-repo Performance: jobLogs context split - Separate JobLogsContext to prevent high-frequency log SSE from re-rendering all panels - Only ReviewAgentJobDetailPanel subscribes to JobLogsProvider - Standard React pattern: split contexts by update frequency Image error handling: - SafeHtmlBlock component wraps dangerouslySetInnerHTML with img onerror handlers - Broken images (expired GitHub JWTs) hide on first 404 instead of flickering - Prevents console 404 flood from re-render retry loops For provenance purposes, this commit was AI assisted. --- apps/hook/server/index.ts | 136 ++++++---- packages/review-editor/App.tsx | 9 +- .../review-editor/components/PRSummaryTab.tsx | 18 +- .../review-editor/dock/JobLogsContext.tsx | 23 ++ .../review-editor/dock/ReviewStateContext.tsx | 2 - .../dock/panels/ReviewAgentJobDetailPanel.tsx | 15 +- packages/server/agent-jobs.ts | 85 ++++++- packages/server/claude-review.ts | 232 ++++++++++++++++++ packages/server/codex-review.ts | 7 +- packages/server/review.ts | 46 ++-- packages/shared/agent-jobs.ts | 2 + 11 files changed, 492 insertions(+), 83 deletions(-) create mode 100644 packages/review-editor/dock/JobLogsContext.tsx create mode 100644 packages/server/claude-review.ts diff --git a/apps/hook/server/index.ts b/apps/hook/server/index.ts index 7b01f1ab..330d4a90 100644 --- a/apps/hook/server/index.ts +++ b/apps/hook/server/index.ts @@ -69,7 +69,8 @@ import { parsePRUrl, checkPRAuth, fetchPR, getCliName, getCliInstallUrl, getMRLa import { writeRemoteShareLink } from "@plannotator/server/share-url"; import { resolveMarkdownFile, hasMarkdownFiles } from "@plannotator/shared/resolve-file"; import { FILE_BROWSER_EXCLUDED } from "@plannotator/shared/reference-common"; -import { statSync } from "fs"; +import { statSync, rmSync } from "fs"; +import { parseRemoteUrl } from "@plannotator/shared/repo"; import { registerSession, unregisterSession, listSessions } from "@plannotator/server/sessions"; import { openBrowser } from "@plannotator/server/browser"; import { detectProjectName } from "@plannotator/server/project"; @@ -245,62 +246,107 @@ if (args[0] === "sessions") { process.exit(1); } - // --local: create a temporary worktree with the PR head for local file access + // --local: create a local checkout with the PR head for full file access if (useLocal && prMetadata) { try { const repoDir = process.cwd(); - let fetchRefStr: string; - let identifier: string; - - if (prMetadata.platform === "github") { - fetchRefStr = `refs/pull/${prMetadata.number}/head`; - identifier = `${prMetadata.owner}-${prMetadata.repo}-${prMetadata.number}`; + const identifier = prMetadata.platform === "github" + ? `${prMetadata.owner}-${prMetadata.repo}-${prMetadata.number}` + : `${prMetadata.projectPath.replace(/\//g, "-")}-${prMetadata.iid}`; + const suffix = Math.random().toString(36).slice(2, 8); + const localPath = path.join(tmpdir(), `plannotator-pr-${identifier}-${suffix}`); + const fetchRefStr = prMetadata.platform === "github" + ? `refs/pull/${prMetadata.number}/head` + : `refs/merge-requests/${prMetadata.iid}/head`; + + // Detect same-repo vs cross-repo + let isSameRepo = false; + try { + const remoteResult = await gitRuntime.runGit(["remote", "get-url", "origin"]); + if (remoteResult.exitCode === 0) { + const currentRepo = parseRemoteUrl(remoteResult.stdout.trim()); + const prRepo = prMetadata.platform === "github" + ? `${prMetadata.owner}/${prMetadata.repo}` + : prMetadata.projectPath; + isSameRepo = !!currentRepo && currentRepo.toLowerCase() === prRepo.toLowerCase(); + } + } catch { /* not in a git repo — cross-repo path */ } + + if (isSameRepo) { + // ── Same-repo: fast worktree path ── + console.error("Fetching PR branch and creating local worktree..."); + await fetchRef(gitRuntime, fetchRefStr, { cwd: repoDir }); + await ensureObjectAvailable(gitRuntime, prMetadata.baseSha, { cwd: repoDir }); + + await createWorktree(gitRuntime, { + ref: "FETCH_HEAD", + path: localPath, + detach: true, + cwd: repoDir, + }); + + worktreeCleanup = () => removeWorktree(gitRuntime, localPath, { force: true, cwd: repoDir }); + process.on("exit", () => { + try { Bun.spawnSync(["git", "worktree", "remove", "--force", localPath]); } catch {} + }); } else { - fetchRefStr = `refs/merge-requests/${prMetadata.iid}/head`; - identifier = `${prMetadata.projectPath.replace(/\//g, "-")}-${prMetadata.iid}`; - } - - console.error("Fetching PR branch and creating local worktree..."); - await fetchRef(gitRuntime, fetchRefStr, { cwd: repoDir }); - - // Ensure base SHA is available (needed for branch diff) - const baseAvailable = await ensureObjectAvailable(gitRuntime, prMetadata.baseSha, { cwd: repoDir }); - if (!baseAvailable) { - console.error(`Warning: base SHA ${prMetadata.baseSha.slice(0, 8)} not available locally — branch diff may be incomplete`); + // ── Cross-repo: shallow clone + fetch PR head ── + const prRepo = prMetadata.platform === "github" + ? `${prMetadata.owner}/${prMetadata.repo}` + : prMetadata.projectPath; + const cli = prMetadata.platform === "github" ? "gh" : "glab"; + const host = prMetadata.host; + const hostnameArgs = (host === "github.com" || host === "gitlab.com") ? [] : ["--hostname", host]; + + // Step 1: Fast skeleton clone (no checkout, depth 1 — minimal data transfer) + console.error(`Cloning ${prRepo} (shallow)...`); + const cloneResult = Bun.spawnSync( + [cli, "repo", "clone", prRepo, localPath, ...hostnameArgs, "--", "--depth=1", "--no-checkout"], + { stderr: "pipe" }, + ); + if (cloneResult.exitCode !== 0) { + throw new Error(`${cli} repo clone failed: ${new TextDecoder().decode(cloneResult.stderr).trim()}`); + } + + // Step 2: Fetch only the PR head ref (targeted, much faster than full fetch) + console.error("Fetching PR branch..."); + const fetchResult = Bun.spawnSync( + ["git", "fetch", "--depth=50", "origin", fetchRefStr], + { cwd: localPath, stderr: "pipe" }, + ); + if (fetchResult.exitCode !== 0) throw new Error(`Failed to fetch PR head ref: ${new TextDecoder().decode(fetchResult.stderr).trim()}`); + + // Step 3: Checkout and ensure base is available + Bun.spawnSync(["git", "checkout", "FETCH_HEAD"], { cwd: localPath, stderr: "pipe" }); + + // Fetch the exact base SHA (the merge-base GitHub computed for this PR) + // Using baseSha instead of baseBranch avoids diff explosion from branch divergence + Bun.spawnSync(["git", "fetch", "--depth=50", "origin", prMetadata.baseSha], { cwd: localPath, stderr: "pipe" }); + + worktreeCleanup = () => { try { rmSync(localPath, { recursive: true, force: true }); } catch {} }; + process.on("exit", () => { + try { Bun.spawnSync(["rm", "-rf", localPath]); } catch {} + }); } - const suffix = Math.random().toString(36).slice(2, 8); - const worktreePath = path.join(tmpdir(), `plannotator-pr-${identifier}-${suffix}`); - - const { worktreePath: createdPath } = await createWorktree(gitRuntime, { - ref: "FETCH_HEAD", - path: worktreePath, - detach: true, - cwd: repoDir, - }); - - // Build git context from the worktree - gitContext = await getVcsContext(createdPath); - // Override default branch with the PR's actual base branch + // ── Common path: build context + diff ── + gitContext = await getVcsContext(localPath); gitContext.defaultBranch = prMetadata.baseBranch; - - // Compute diff locally (more accurate than platform diff) initialDiffType = "branch"; - const localDiff = await runVcsDiff("branch", prMetadata.baseBranch, createdPath); - rawPatch = localDiff.patch; - gitRef = localDiff.label || gitRef; - // Register cleanup - worktreeCleanup = () => removeWorktree(gitRuntime, createdPath, { force: true, cwd: repoDir }); - process.on("exit", () => { - try { Bun.spawnSync(["git", "worktree", "remove", "--force", createdPath]); } catch {} - }); + if (isSameRepo) { + // Same-repo: compute diff locally (full history available, accurate) + const localDiff = await runVcsDiff("branch", prMetadata.baseBranch, localPath); + rawPatch = localDiff.patch; + gitRef = localDiff.label || gitRef; + } + // Cross-repo: keep the rawPatch from gh pr diff (already set above). + // Local git diff in shallow clones produces malformed hunks that break the diff viewer. - console.error(`Local worktree created at ${createdPath}`); + console.error(`Local checkout ready at ${localPath}`); } catch (err) { - console.error(`Warning: --local worktree creation failed, falling back to remote diff`); + console.error(`Warning: --local failed, falling back to remote diff`); console.error(err instanceof Error ? err.message : String(err)); - // gitContext remains undefined = pure PR mode fallback worktreeCleanup = undefined; } } diff --git a/packages/review-editor/App.tsx b/packages/review-editor/App.tsx index aa92881e..6e17bdfe 100644 --- a/packages/review-editor/App.tsx +++ b/packages/review-editor/App.tsx @@ -38,6 +38,7 @@ import { FileTree } from './components/FileTree'; import { DEMO_DIFF } from './demoData'; import { exportReviewFeedback } from './utils/exportFeedback'; import { ReviewStateProvider, type ReviewState } from './dock/ReviewStateContext'; +import { JobLogsProvider } from './dock/JobLogsContext'; import { reviewPanelComponents } from './dock/reviewPanelComponents'; import { ReviewDockTabRenderer } from './dock/ReviewDockTabRenderer'; import { usePRContext } from './hooks/usePRContext'; @@ -966,7 +967,6 @@ const ReviewApp: React.FC = () => { onClickAIMarker: handleClickAIMarker, aiHistoryForSelection, agentJobs: agentJobs.jobs, - jobLogs: agentJobs.jobLogs, prMetadata, prContext, isPRContextLoading, @@ -986,10 +986,13 @@ const ReviewApp: React.FC = () => { activeFileSearchMatches, activeSearchMatchId, activeSearchMatch, aiAvailable, aiChat.messages, aiChat.isCreatingSession, aiChat.isStreaming, handleAskAI, handleViewAIResponse, handleClickAIMarker, - aiHistoryForSelection, agentJobs.jobs, agentJobs.jobLogs, prMetadata, prContext, + aiHistoryForSelection, agentJobs.jobs, prMetadata, prContext, isPRContextLoading, prContextError, fetchPRContext, platformUser, openDiffFile, ]); + // Separate context for high-frequency job logs — prevents re-rendering all panels on every SSE event + const jobLogsValue = useMemo(() => ({ jobLogs: agentJobs.jobLogs }), [agentJobs.jobLogs]); + // Copy raw diff to clipboard const handleCopyDiff = useCallback(async () => { if (!diffData) return; @@ -1303,6 +1306,7 @@ const ReviewApp: React.FC = () => { return ( +
{/* Header */}
@@ -1929,6 +1933,7 @@ const ReviewApp: React.FC = () => {
)}
+ ); diff --git a/packages/review-editor/components/PRSummaryTab.tsx b/packages/review-editor/components/PRSummaryTab.tsx index 22f40061..0654c256 100644 --- a/packages/review-editor/components/PRSummaryTab.tsx +++ b/packages/review-editor/components/PRSummaryTab.tsx @@ -1,8 +1,20 @@ -import React, { useMemo } from 'react'; +import React, { useMemo, useRef, useEffect } from 'react'; import DOMPurify from 'dompurify'; import { parseMarkdownToBlocks } from '@plannotator/ui/utils/parser'; import { renderInlineMarkdown } from '../utils/renderInlineMarkdown'; import type { PRContext } from '@plannotator/shared/pr-provider'; + +/** Renders sanitized HTML with onerror handlers on images to prevent 404 flicker loops. */ +function SafeHtmlBlock({ html, tag: Tag = 'div', ...props }: { html: string; tag?: 'div' | 'span'; [key: string]: any }) { + const ref = useRef(null); + useEffect(() => { + if (!ref.current) return; + ref.current.querySelectorAll('img').forEach((img) => { + img.onerror = () => { img.style.display = 'none'; img.onerror = null; }; + }); + }, [html]); + return ; +} import type { PRMetadata } from '@plannotator/shared/pr-provider'; interface PRSummaryTabProps { @@ -35,7 +47,7 @@ function sanitizeHtml(html: string): string { */ function renderContent(content: string): React.ReactNode { if (containsHtml(content)) { - return ; + return ; } return renderInlineMarkdown(content); } @@ -86,7 +98,7 @@ export function MarkdownBody({ markdown }: { markdown: string }) { if (!block.content) return null; // If the entire paragraph is HTML, sanitize and render if (containsHtml(block.content)) { - return
; + return ; } return

{renderInlineMarkdown(block.content)}

; } diff --git a/packages/review-editor/dock/JobLogsContext.tsx b/packages/review-editor/dock/JobLogsContext.tsx new file mode 100644 index 00000000..a3e53319 --- /dev/null +++ b/packages/review-editor/dock/JobLogsContext.tsx @@ -0,0 +1,23 @@ +/** + * Separate context for live agent job logs. + * + * Split from ReviewStateContext to prevent high-frequency SSE log updates + * (every ~200ms) from re-rendering all dockview panels. Only the agent + * detail panel subscribes to this context. + * + * Pattern: Dan Abramov's "split contexts by update frequency" optimization. + */ + +import { createContext, useContext } from 'react'; + +interface JobLogsState { + jobLogs: Map; +} + +const JobLogsContext = createContext({ jobLogs: new Map() }); + +export const JobLogsProvider = JobLogsContext.Provider; + +export function useJobLogs(): JobLogsState { + return useContext(JobLogsContext); +} diff --git a/packages/review-editor/dock/ReviewStateContext.tsx b/packages/review-editor/dock/ReviewStateContext.tsx index 300d6e72..22947042 100644 --- a/packages/review-editor/dock/ReviewStateContext.tsx +++ b/packages/review-editor/dock/ReviewStateContext.tsx @@ -67,8 +67,6 @@ export interface ReviewState { // Agent jobs agentJobs: AgentJobInfo[]; - /** Live stderr logs per job ID (accumulated deltas). */ - jobLogs: Map; // PR prMetadata: PRMetadata | null; diff --git a/packages/review-editor/dock/panels/ReviewAgentJobDetailPanel.tsx b/packages/review-editor/dock/panels/ReviewAgentJobDetailPanel.tsx index c78b7ca0..4b93526a 100644 --- a/packages/review-editor/dock/panels/ReviewAgentJobDetailPanel.tsx +++ b/packages/review-editor/dock/panels/ReviewAgentJobDetailPanel.tsx @@ -3,6 +3,7 @@ import type { IDockviewPanelProps } from 'dockview-react'; import type { AgentJobInfo, CodeAnnotation } from '@plannotator/ui/types'; import { isTerminalStatus } from '@plannotator/shared/agent-jobs'; import { useReviewState } from '../ReviewStateContext'; +import { useJobLogs } from '../JobLogsContext'; import { CopyButton } from '../../components/CopyButton'; import { LiveLogViewer } from '../../components/LiveLogViewer'; import { ScrollFade } from '../../components/ScrollFade'; @@ -29,13 +30,16 @@ export const ReviewAgentJobDetailPanel: React.FC = (props) const { fullCommand, userMessage, systemPrompt } = useMemo(() => { const cmd = job?.command ?? []; const full = cmd.join(' '); - const prompt = cmd.length > 0 ? cmd[cmd.length - 1] : ''; + + // Use job.prompt if available (stored explicitly for all providers), + // fallback to parsing last command arg (legacy Codex behavior) + const promptText = job?.prompt || (cmd.length > 0 ? cmd[cmd.length - 1] : ''); const sep = '\n\n---\n\n'; - const i = prompt.indexOf(sep); + const i = promptText.indexOf(sep); return { fullCommand: full, - userMessage: i !== -1 ? prompt.substring(i + sep.length) : prompt, - systemPrompt: i !== -1 ? prompt.substring(0, i) : '', + userMessage: i !== -1 ? promptText.substring(i + sep.length) : promptText, + systemPrompt: i !== -1 ? promptText.substring(0, i) : '', }; }, [job]); @@ -80,7 +84,8 @@ export const ReviewAgentJobDetailPanel: React.FC = (props) [activeAnnotations, state.prMetadata] ); - const logContent = state.jobLogs.get(jobId) ?? ''; + const { jobLogs } = useJobLogs(); + const logContent = jobLogs.get(jobId) ?? ''; if (!job) { return
Job not found
; diff --git a/packages/server/agent-jobs.ts b/packages/server/agent-jobs.ts index 0e486e77..2d3f7828 100644 --- a/packages/server/agent-jobs.ts +++ b/packages/server/agent-jobs.ts @@ -61,12 +61,21 @@ export interface AgentJobHandlerOptions { * Return an object with the command to spawn (and optional output path for result ingestion). * Return null to reject or fall through to frontend-supplied command. */ - buildCommand?: (provider: string) => Promise<{ command: string[]; outputPath?: string; label?: string } | null>; + buildCommand?: (provider: string) => Promise<{ + command: string[]; + outputPath?: string; + captureStdout?: boolean; + stdinPrompt?: string; + cwd?: string; + label?: string; + /** The full prompt text for display in the detail panel. */ + prompt?: string; + } | null>; /** * Called after a job process exits with exit code 0. * Use for result ingestion (e.g., reading an output file and pushing annotations). */ - onJobComplete?: (job: AgentJobInfo, meta: { outputPath?: string }) => void | Promise; + onJobComplete?: (job: AgentJobInfo, meta: { outputPath?: string; stdout?: string; cwd?: string }) => void | Promise; } export function createAgentJobHandler(options: AgentJobHandlerOptions): AgentJobHandler { @@ -110,6 +119,7 @@ export function createAgentJobHandler(options: AgentJobHandlerOptions): AgentJob command: string[], label: string, outputPath?: string, + spawnOptions?: { captureStdout?: boolean; stdinPrompt?: string; cwd?: string; prompt?: string }, ): AgentJobInfo { const id = crypto.randomUUID(); const source = jobSource(id); @@ -128,9 +138,15 @@ export function createAgentJobHandler(options: AgentJobHandlerOptions): AgentJob let proc: ReturnType | null = null; try { + const spawnCwd = spawnOptions?.cwd ?? getCwd(); + const captureStdout = spawnOptions?.captureStdout ?? false; + + const hasStdinPrompt = !!spawnOptions?.stdinPrompt; + proc = Bun.spawn(command, { - cwd: getCwd(), - stdout: "ignore", + cwd: spawnCwd, + stdin: hasStdinPrompt ? "pipe" : undefined, + stdout: captureStdout ? "pipe" : "ignore", stderr: "pipe", env: { ...process.env, @@ -139,9 +155,18 @@ export function createAgentJobHandler(options: AgentJobHandlerOptions): AgentJob }, }); + // Write prompt to stdin and close (for providers that read prompt from stdin) + if (hasStdinPrompt && proc.stdin) { + proc.stdin.write(spawnOptions!.stdinPrompt!); + proc.stdin.end(); + } + info.status = "running"; + info.cwd = spawnCwd; + if (spawnOptions?.prompt) info.prompt = spawnOptions.prompt; jobs.set(id, { info, proc }); if (outputPath) jobOutputPaths.set(id, outputPath); + if (spawnOptions?.cwd) jobOutputPaths.set(`${id}:cwd`, spawnOptions.cwd); broadcast({ type: "job:started", job: { ...info } }); // Drain stderr: capture tail for error reporting + broadcast live log deltas @@ -180,8 +205,37 @@ export function createAgentJobHandler(options: AgentJobHandlerOptions): AgentJob })(); } + // Drain stdout when capturing (for providers that return results on stdout) + let stdoutBuf = ""; + const stdoutDone = (captureStdout && proc.stdout && typeof proc.stdout !== "number") + ? (async () => { + try { + const reader = proc!.stdout as ReadableStream; + for await (const chunk of reader) { + const text = typeof chunk === "string" ? chunk : new TextDecoder().decode(chunk); + stdoutBuf += text; + + // Forward JSONL lines as log events (skip result events) + const lines = text.split('\n'); + for (const line of lines) { + if (!line.trim()) continue; + try { + const event = JSON.parse(line); + if (event.type === 'result') continue; // handled in onJobComplete + } catch { /* not JSON — forward as raw log */ } + broadcast({ type: "job:log", jobId: id, delta: line + '\n' }); + } + } + } catch { + // Stream closed + } + })() + : Promise.resolve(); + // Monitor process exit proc.exited.then(async (exitCode) => { + // Ensure stdout is fully drained before reading results + await stdoutDone; const entry = jobs.get(id); if (!entry || isTerminalStatus(entry.info.status)) return; @@ -195,14 +249,20 @@ export function createAgentJobHandler(options: AgentJobHandlerOptions): AgentJob // Ingest results before broadcasting completion so annotations arrive first const outputPath = jobOutputPaths.get(id); + const jobCwd = jobOutputPaths.get(`${id}:cwd`); if (exitCode === 0 && options.onJobComplete) { try { - await options.onJobComplete(entry.info, { outputPath }); + await options.onJobComplete(entry.info, { + outputPath, + stdout: captureStdout ? stdoutBuf : undefined, + cwd: jobCwd, + }); } catch { // Result ingestion failure shouldn't prevent job completion broadcast } } jobOutputPaths.delete(id); + jobOutputPaths.delete(`${id}:cwd`); broadcast({ type: "job:completed", job: { ...entry.info } }); }).catch(() => { @@ -347,11 +407,19 @@ export function createAgentJobHandler(options: AgentJobHandlerOptions): AgentJob } // For non-shell providers, try server-side command building + let captureStdout = false; + let stdinPrompt: string | undefined; + let spawnCwd: string | undefined; + let promptText: string | undefined; if (options.buildCommand && provider !== "shell") { const built = await options.buildCommand(provider); if (built) { command = built.command; outputPath = built.outputPath; + captureStdout = built.captureStdout ?? false; + stdinPrompt = built.stdinPrompt; + spawnCwd = built.cwd; + promptText = built.prompt; if (built.label) label = built.label; } } @@ -363,7 +431,12 @@ export function createAgentJobHandler(options: AgentJobHandlerOptions): AgentJob ); } - const job = spawnJob(provider, command, label, outputPath); + const job = spawnJob(provider, command, label, outputPath, { + captureStdout, + stdinPrompt, + cwd: spawnCwd, + prompt: promptText, + }); return Response.json({ job }, { status: 201 }); } catch { return Response.json({ error: "Invalid JSON" }, { status: 400 }); diff --git a/packages/server/claude-review.ts b/packages/server/claude-review.ts new file mode 100644 index 00000000..2b78cd74 --- /dev/null +++ b/packages/server/claude-review.ts @@ -0,0 +1,232 @@ +/** + * Claude Code Review Agent — prompt, command builder, and JSONL output parser. + * + * Complements codex-review.ts. Both produce the same schema (ReviewOutputEvent). + * Claude uses --json-schema (inline JSON + Ajv validation with retries) instead + * of Codex's --output-schema (file path + API-level strict mode). + * + * Claude uses --output-format stream-json for live JSONL streaming. The final + * event is type:"result" with structured_output containing validated findings. + */ + +import type { CodexReviewOutput } from "./codex-review"; + +// --------------------------------------------------------------------------- +// Schema — same as Codex, serialized as inline JSON for --json-schema flag +// --------------------------------------------------------------------------- + +export const CLAUDE_REVIEW_SCHEMA_JSON = JSON.stringify({ + type: "object", + properties: { + findings: { + type: "array", + items: { + type: "object", + properties: { + title: { type: "string" }, + body: { type: "string" }, + confidence_score: { type: "number" }, + priority: { type: ["integer", "null"] }, + code_location: { + type: "object", + properties: { + absolute_file_path: { type: "string" }, + line_range: { + type: "object", + properties: { + start: { type: "integer" }, + end: { type: "integer" }, + }, + required: ["start", "end"], + additionalProperties: false, + }, + }, + required: ["absolute_file_path", "line_range"], + additionalProperties: false, + }, + }, + required: ["title", "body", "confidence_score", "priority", "code_location"], + additionalProperties: false, + }, + }, + overall_correctness: { type: "string" }, + overall_explanation: { type: "string" }, + overall_confidence_score: { type: "number" }, + }, + required: ["findings", "overall_correctness", "overall_explanation", "overall_confidence_score"], + additionalProperties: false, +}); + +// --------------------------------------------------------------------------- +// Review prompt — adapted from anthropics/claude-code code-review plugin +// --------------------------------------------------------------------------- + +export const CLAUDE_REVIEW_PROMPT = `You are a code reviewer. Provide a thorough code review of the changes. + +**Agent assumptions (applies to all agents and subagents):** +- All tools are functional and will work without error. Do not test tools or make exploratory calls. +- Only call a tool if it is required to complete the task. Every tool call should have a clear purpose. + +Follow these steps: + +1. Examine the diff to understand what changed. Use git diff, git log, git show, or gh pr diff as appropriate for the context. + +2. Launch parallel agents to independently review the changes. Each agent should return the list of issues, where each issue includes a description and the reason it was flagged. The agents should: + + Agent 1: Bug agent + Scan for obvious bugs. Focus only on the diff itself without reading extra context. Flag only significant bugs; ignore nitpicks and likely false positives. Do not flag issues that you cannot validate without looking at context outside of the git diff. + + Agent 2: Deep analysis agent + Look for problems that exist in the introduced code. This could be security issues, incorrect logic, etc. Only look for issues that fall within the changed code. Read surrounding code for context. + + **CRITICAL: We only want HIGH SIGNAL issues.** Flag issues where: + - The code will fail to compile or parse (syntax errors, type errors, missing imports, unresolved references) + - The code will definitely produce wrong results regardless of inputs (clear logic errors) + - Security vulnerabilities with concrete exploit paths + + Do NOT flag: + - Code style or quality concerns + - Potential issues that depend on specific inputs or state + - Subjective suggestions or improvements + - Missing tests or documentation + + If you are not certain an issue is real, do not flag it. False positives erode trust and waste reviewer time. + +3. For each issue found, launch parallel agents to validate. Each validator should confirm the issue is real with high confidence by examining the actual code. If validation fails, drop the issue. + +4. Return all validated findings as structured output matching the JSON schema. Include an overall correctness verdict. + +At the beginning of each finding title, tag with priority level: +- [P0] – Drop everything to fix. Blocking. +- [P1] – Urgent. Should be addressed in the next cycle. +- [P2] – Normal. To be fixed eventually. +- [P3] – Low. Nice to have. + +Set the numeric priority field accordingly (0-3). If priority cannot be determined, use null. + +Do NOT post any comments to GitHub or GitLab. Do NOT use gh pr comment or any commenting tool. Your only output is the structured JSON findings.`; + +// --------------------------------------------------------------------------- +// Command builder +// --------------------------------------------------------------------------- + +export interface ClaudeCommandResult { + command: string[]; + /** Prompt text to write to stdin (Claude reads prompt from stdin, not argv). */ + stdinPrompt: string; +} + +/** + * Build the `claude -p` command. Prompt is passed via stdin, not as a + * positional arg — avoids quoting issues, argv limits, and variadic flag conflicts. + */ +export function buildClaudeCommand(prompt: string): ClaudeCommandResult { + const allowedTools = [ + "Agent", "Read", "Glob", "Grep", + "Bash(gh pr view:*)", "Bash(gh pr diff:*)", "Bash(gh pr list:*)", + "Bash(gh issue view:*)", "Bash(gh issue list:*)", + "Bash(gh api repos/*/*/pulls/*)", "Bash(gh api repos/*/*/pulls/*/files*)", + "Bash(gh api repos/*/*/pulls/*/comments*)", "Bash(gh api repos/*/*/issues/*/comments*)", + "Bash(git status:*)", "Bash(git diff:*)", "Bash(git log:*)", + "Bash(git show:*)", "Bash(git blame:*)", "Bash(git branch:*)", + "Bash(git grep:*)", "Bash(git ls-remote:*)", "Bash(git ls-tree:*)", + "Bash(git merge-base:*)", "Bash(git remote:*)", "Bash(git rev-parse:*)", + "Bash(git show-ref:*)", + "Bash(find:*)", "Bash(ls:*)", "Bash(cat:*)", "Bash(wc:*)", + ].join(","); + + const disallowedTools = [ + "Edit", "Write", "NotebookEdit", "WebFetch", "WebSearch", + "Bash(python:*)", "Bash(python3:*)", "Bash(node:*)", "Bash(npx:*)", + "Bash(bun:*)", "Bash(bunx:*)", "Bash(sh:*)", "Bash(bash:*)", "Bash(zsh:*)", + "Bash(curl:*)", "Bash(wget:*)", + ].join(","); + + return { + command: [ + "claude", "-p", + "--permission-mode", "dontAsk", + "--output-format", "stream-json", + "--verbose", + "--json-schema", CLAUDE_REVIEW_SCHEMA_JSON, + "--no-session-persistence", + "--model", "sonnet", + "--tools", "Agent,Bash,Read,Glob,Grep", + "--allowedTools", allowedTools, + "--disallowedTools", disallowedTools, + ], + stdinPrompt: prompt, + }; +} + +// --------------------------------------------------------------------------- +// JSONL stream output parser +// --------------------------------------------------------------------------- + +/** + * Parse Claude Code's stream-json output (JSONL). + * Extracts structured_output from the final type:"result" event. + * Returns the same CodexReviewOutput shape for provider-agnostic transform. + */ +export function parseClaudeStreamOutput(stdout: string): CodexReviewOutput | null { + if (!stdout.trim()) return null; + + // Find the last result event in the JSONL stream + const lines = stdout.trim().split('\n'); + for (let i = lines.length - 1; i >= 0; i--) { + const line = lines[i].trim(); + if (!line) continue; + + try { + const event = JSON.parse(line); + + if (event.type === 'result') { + if (event.is_error) return null; + + const output = event.structured_output; + if (!output || !Array.isArray(output.findings)) return null; + + return output as CodexReviewOutput; + } + } catch { + // Not valid JSON — skip (could be a partial line or log noise) + } + } + + return null; +} + +/** + * Extract log-worthy content from a JSONL line for the LiveLogViewer. + * Returns a human-readable string, or null if the line should be skipped. + */ +export function formatClaudeLogEvent(line: string): string | null { + try { + const event = JSON.parse(line); + + // Skip the final result event — handled separately + if (event.type === 'result') return null; + + // Assistant messages (the agent's thinking/responses) + if (event.type === 'assistant' && event.message?.content) { + const parts = Array.isArray(event.message.content) ? event.message.content : [event.message.content]; + const texts = parts + .filter((p: any) => p.type === 'text' && p.text) + .map((p: any) => p.text); + if (texts.length > 0) return texts.join('\n'); + } + + // Tool use events + if (event.type === 'assistant' && event.message?.content) { + const tools = (Array.isArray(event.message.content) ? event.message.content : []) + .filter((p: any) => p.type === 'tool_use'); + if (tools.length > 0) { + return tools.map((t: any) => `[${t.name}] ${typeof t.input === 'string' ? t.input.slice(0, 100) : JSON.stringify(t.input).slice(0, 100)}`).join('\n'); + } + } + + return null; + } catch { + return null; + } +} diff --git a/packages/server/codex-review.ts b/packages/server/codex-review.ts index 392581f7..30417642 100644 --- a/packages/server/codex-review.ts +++ b/packages/server/codex-review.ts @@ -346,11 +346,12 @@ function toRelativePath(absolutePath: string, cwd?: string): string { return absolutePath.startsWith(prefix) ? absolutePath.slice(prefix.length) : absolutePath; } -/** Transform Codex findings (native schema) into the external annotation format. */ -export function transformCodexFindings( +/** Transform review findings (provider-agnostic) into the external annotation format. */ +export function transformReviewFindings( findings: CodexFinding[], source: string, cwd?: string, + author?: string, ): ReviewAnnotationInput[] { const annotations = findings .filter((f) => @@ -367,7 +368,7 @@ export function transformCodexFindings( side: "new", scope: "line", text: `${f.title}\n\n${f.body}`.trim(), - author: "Codex", + author: author ?? "Review Agent", })); debugLog("TRANSFORM_FINDINGS", { diff --git a/packages/server/review.ts b/packages/server/review.ts index a7d51e47..b6a88a98 100644 --- a/packages/server/review.ts +++ b/packages/server/review.ts @@ -24,8 +24,13 @@ import { buildCodexCommand, generateOutputPath, parseCodexOutput, - transformCodexFindings, + transformReviewFindings, } from "./codex-review"; +import { + CLAUDE_REVIEW_PROMPT, + buildClaudeCommand, + parseClaudeStreamOutput, +} from "./claude-review"; import { saveConfig, detectGitUser, getServerConfig } from "./config"; import { type PRMetadata, type PRReviewFileComment, fetchPRFileContent, fetchPRContext, submitPRReview, fetchPRViewedFiles, markPRFilesViewed, getPRUser, prRefFromMetadata, getDisplayRepo, getMRLabel, getMRNumberLabel } from "./pr"; import { createAIEndpoints, ProviderRegistry, SessionManager, createProvider, type AIEndpoints, type PiSDKConfig } from "@plannotator/ai"; @@ -127,28 +132,34 @@ export async function startReviewServer( }, async buildCommand(provider) { - if (provider !== "codex") return null; - const cwd = resolveVcsCwd(currentDiffType, gitContext?.cwd) ?? process.cwd(); - const outputPath = generateOutputPath(); - - // Build the two-layer prompt: system prompt + dynamic user message const userMessage = buildCodexReviewUserMessage(currentPatch, currentDiffType, gitContext, prMetadata); - const prompt = CODEX_REVIEW_SYSTEM_PROMPT + "\n\n---\n\n" + userMessage; - const command = await buildCodexCommand({ - cwd, - outputPath, - prompt, - }); + if (provider === "codex") { + const outputPath = generateOutputPath(); + const prompt = CODEX_REVIEW_SYSTEM_PROMPT + "\n\n---\n\n" + userMessage; + const command = await buildCodexCommand({ cwd, outputPath, prompt }); + return { command, outputPath, prompt, label: "Codex Review" }; + } - return { command, outputPath, label: "Codex Review" }; + if (provider === "claude") { + const prompt = CLAUDE_REVIEW_PROMPT + "\n\n---\n\n" + userMessage; + const { command, stdinPrompt } = buildClaudeCommand(prompt); + return { command, stdinPrompt, prompt, cwd, label: "Claude Code Review", captureStdout: true }; + } + + return null; }, async onJobComplete(job, meta) { - if (!meta.outputPath || job.provider !== "codex") return; + let output: Awaited> = null; + + if (job.provider === "codex" && meta.outputPath) { + output = await parseCodexOutput(meta.outputPath); + } else if (job.provider === "claude" && meta.stdout) { + output = parseClaudeStreamOutput(meta.stdout); + } - const output = await parseCodexOutput(meta.outputPath); if (!output) return; // Set summary on the job — flows through job:completed broadcast @@ -160,11 +171,12 @@ export async function startReviewServer( if (output.findings.length > 0) { const cwd = resolveVcsCwd(currentDiffType, gitContext?.cwd) ?? process.cwd(); - const annotations = transformCodexFindings(output.findings, job.source, cwd); + const authorName = job.provider === "codex" ? "Codex" : job.provider === "claude" ? "Claude Code" : "Review Agent"; + const annotations = transformReviewFindings(output.findings, job.source, cwd, authorName); const result = externalAnnotations.addAnnotations({ annotations }); if ("error" in result) { - console.error("[codex-review] addAnnotations error:", result.error); + console.error(`[${job.provider}-review] addAnnotations error:`, result.error); } } }, diff --git a/packages/shared/agent-jobs.ts b/packages/shared/agent-jobs.ts index 6520358f..41434a51 100644 --- a/packages/shared/agent-jobs.ts +++ b/packages/shared/agent-jobs.ts @@ -37,6 +37,8 @@ export interface AgentJobInfo { command: string[]; /** Working directory where the process was spawned. */ cwd?: string; + /** The review prompt text (system + user message). Stored separately from command for providers that use stdin. */ + prompt?: string; /** Review summary set by the agent on completion. */ summary?: { correctness: string; From 81d9332fb30167d2b4650016e7f2d57018a9b7c5 Mon Sep 17 00:00:00 2001 From: Michael Ramos Date: Sun, 5 Apr 2026 21:11:26 -0700 Subject: [PATCH 06/37] fix(review): decontaminate --local from diff pipeline, fix worktree setup, default local for PRs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The --local flag was setting gitContext from the worktree, which contaminated the diff rendering pipeline — causing pierre "trailing context mismatch" errors because /api/file-content read worktree files instead of using the GitHub API. Root cause: gitContext serves two purposes — diff pipeline (file contents, diff switching, staging) and agent sandbox (cwd for agent processes). These are now properly separated via a new agentCwd option on ReviewServerOptions. Changes: - Add agentCwd to ReviewServerOptions, independent of gitContext - Agent handler (getCwd, buildCommand, onJobComplete) prefers agentCwd - Stop setting gitContext in --local PR path — diff pipeline untouched - Revert band-aid !isPRMode guards (no longer needed) - Fix same-repo worktree: fetch origin/ so agents see correct diff - Fix cross-repo clone: create local branch at baseSha for git diff accuracy - Fix FETCH_HEAD ordering: fetch base branch before PR head (createWorktree needs PR tip) - Fix macOS path mismatch: realpathSync(tmpdir()) so agent paths strip correctly - Change buildCodexReviewUserMessage signature from GitContext to focused options - Make --local the default for PR/MR reviews (--no-local to opt out) - Pass agentCwd to client for worktree badge display For provenance purposes, this commit was AI assisted. --- apps/hook/server/index.ts | 49 +++++++++++++++++---------------- packages/server/codex-review.ts | 18 ++++++++---- packages/server/review.ts | 21 ++++++++++---- 3 files changed, 53 insertions(+), 35 deletions(-) diff --git a/apps/hook/server/index.ts b/apps/hook/server/index.ts index 330d4a90..a369b00c 100644 --- a/apps/hook/server/index.ts +++ b/apps/hook/server/index.ts @@ -69,7 +69,7 @@ import { parsePRUrl, checkPRAuth, fetchPR, getCliName, getCliInstallUrl, getMRLa import { writeRemoteShareLink } from "@plannotator/server/share-url"; import { resolveMarkdownFile, hasMarkdownFiles } from "@plannotator/shared/resolve-file"; import { FILE_BROWSER_EXCLUDED } from "@plannotator/shared/reference-common"; -import { statSync, rmSync } from "fs"; +import { statSync, rmSync, realpathSync } from "fs"; import { parseRemoteUrl } from "@plannotator/shared/repo"; import { registerSession, unregisterSession, listSessions } from "@plannotator/server/sessions"; import { openBrowser } from "@plannotator/server/browser"; @@ -187,18 +187,17 @@ if (args[0] === "sessions") { // CODE REVIEW MODE // ============================================ - // Parse --local flag (strip before URL detection) + // Parse local flags (strip before URL detection) + // --local is now the default for PR/MR reviews; --no-local opts out. + // --local kept for backwards compat (no-op). const localIdx = args.indexOf("--local"); - const useLocal = localIdx !== -1; if (localIdx !== -1) args.splice(localIdx, 1); + const noLocalIdx = args.indexOf("--no-local"); + if (noLocalIdx !== -1) args.splice(noLocalIdx, 1); const urlArg = args[1]; const isPRMode = urlArg?.startsWith("http://") || urlArg?.startsWith("https://"); - - if (useLocal && !isPRMode) { - console.error("--local requires a PR/MR URL"); - process.exit(1); - } + const useLocal = isPRMode && noLocalIdx === -1; let rawPatch: string; let gitRef: string; @@ -206,6 +205,7 @@ if (args[0] === "sessions") { let gitContext: Awaited> | undefined; let prMetadata: Awaited>["metadata"] | undefined; let initialDiffType: DiffType | undefined; + let agentCwd: string | undefined; let worktreeCleanup: (() => void | Promise) | undefined; if (isPRMode) { @@ -254,7 +254,9 @@ if (args[0] === "sessions") { ? `${prMetadata.owner}-${prMetadata.repo}-${prMetadata.number}` : `${prMetadata.projectPath.replace(/\//g, "-")}-${prMetadata.iid}`; const suffix = Math.random().toString(36).slice(2, 8); - const localPath = path.join(tmpdir(), `plannotator-pr-${identifier}-${suffix}`); + // Resolve tmpdir to its real path — on macOS, tmpdir() returns /var/folders/... + // but processes report /private/var/folders/... which breaks path stripping. + const localPath = path.join(realpathSync(tmpdir()), `plannotator-pr-${identifier}-${suffix}`); const fetchRefStr = prMetadata.platform === "github" ? `refs/pull/${prMetadata.number}/head` : `refs/merge-requests/${prMetadata.iid}/head`; @@ -275,6 +277,12 @@ if (args[0] === "sessions") { if (isSameRepo) { // ── Same-repo: fast worktree path ── console.error("Fetching PR branch and creating local worktree..."); + // Fetch base branch FIRST so origin/ is current — without this, + // `git diff origin/main...HEAD` in the worktree uses stale refs and shows + // changes from other PRs that were merged since the last fetch. + await fetchRef(gitRuntime, prMetadata.baseBranch, { cwd: repoDir }); + // Fetch PR head SECOND — this sets FETCH_HEAD to the PR tip, which + // createWorktree needs. Order matters: fetchRef overwrites FETCH_HEAD. await fetchRef(gitRuntime, fetchRefStr, { cwd: repoDir }); await ensureObjectAvailable(gitRuntime, prMetadata.baseSha, { cwd: repoDir }); @@ -319,9 +327,10 @@ if (args[0] === "sessions") { // Step 3: Checkout and ensure base is available Bun.spawnSync(["git", "checkout", "FETCH_HEAD"], { cwd: localPath, stderr: "pipe" }); - // Fetch the exact base SHA (the merge-base GitHub computed for this PR) - // Using baseSha instead of baseBranch avoids diff explosion from branch divergence + // Fetch the exact base SHA and create a local branch for it so + // `git diff main...HEAD` shows only the PR changes. Bun.spawnSync(["git", "fetch", "--depth=50", "origin", prMetadata.baseSha], { cwd: localPath, stderr: "pipe" }); + Bun.spawnSync(["git", "branch", prMetadata.baseBranch, prMetadata.baseSha], { cwd: localPath, stderr: "pipe" }); worktreeCleanup = () => { try { rmSync(localPath, { recursive: true, force: true }); } catch {} }; process.on("exit", () => { @@ -329,24 +338,15 @@ if (args[0] === "sessions") { }); } - // ── Common path: build context + diff ── - gitContext = await getVcsContext(localPath); - gitContext.defaultBranch = prMetadata.baseBranch; - initialDiffType = "branch"; - - if (isSameRepo) { - // Same-repo: compute diff locally (full history available, accurate) - const localDiff = await runVcsDiff("branch", prMetadata.baseBranch, localPath); - rawPatch = localDiff.patch; - gitRef = localDiff.label || gitRef; - } - // Cross-repo: keep the rawPatch from gh pr diff (already set above). - // Local git diff in shallow clones produces malformed hunks that break the diff viewer. + // --local only provides a sandbox path for agent processes. + // Do NOT set gitContext — that would contaminate the diff pipeline. + agentCwd = localPath; console.error(`Local checkout ready at ${localPath}`); } catch (err) { console.error(`Warning: --local failed, falling back to remote diff`); console.error(err instanceof Error ? err.message : String(err)); + agentCwd = undefined; worktreeCleanup = undefined; } } @@ -371,6 +371,7 @@ if (args[0] === "sessions") { diffType: gitContext ? (initialDiffType ?? "uncommitted") : undefined, gitContext, prMetadata, + agentCwd, sharingEnabled, shareBaseUrl, htmlContent: reviewHtmlContent, diff --git a/packages/server/codex-review.ts b/packages/server/codex-review.ts index 30417642..cf9f2861 100644 --- a/packages/server/codex-review.ts +++ b/packages/server/codex-review.ts @@ -8,7 +8,7 @@ import { join } from "node:path"; import { homedir, tmpdir } from "node:os"; import { appendFile, mkdir, unlink } from "node:fs/promises"; -import type { DiffType, GitContext } from "./vcs"; +import type { DiffType } from "./vcs"; import type { PRMetadata } from "./pr"; // --------------------------------------------------------------------------- @@ -164,13 +164,19 @@ The finding description should be one paragraph.`; export function buildCodexReviewUserMessage( patch: string, diffType: DiffType, - gitContext?: GitContext, + options?: { defaultBranch?: string; hasLocalAccess?: boolean }, prMetadata?: PRMetadata, ): string { - // PR/MR mode — just pass the link, Codex knows what to do + // PR/MR mode — pass the link, with local context if --local if (prMetadata) { - if (gitContext) { - return `${prMetadata.url}\n\nYou are in a local worktree checked out from the PR branch. The code is available locally.`; + if (options?.hasLocalAccess) { + return [ + prMetadata.url, + "", + "You are in a local worktree checked out at the PR head. The code is available locally.", + `To see the PR changes, diff against the remote base branch: git diff origin/${prMetadata.baseBranch}...HEAD`, + "Do NOT diff against the local `main` branch — it may be stale. Always use origin/.", + ].join("\n"); } return prMetadata.url; } @@ -194,7 +200,7 @@ export function buildCodexReviewUserMessage( return "Review the code changes introduced in the last commit (`git diff HEAD~1..HEAD`) and provide prioritized findings."; case "branch": { - const base = gitContext?.defaultBranch || "main"; + const base = options?.defaultBranch || "main"; return `Review the code changes against the base branch '${base}'. Run \`git diff ${base}...HEAD\` to inspect the changes. Provide prioritized, actionable findings.`; } diff --git a/packages/server/review.ts b/packages/server/review.ts index b6a88a98..7a2092aa 100644 --- a/packages/server/review.ts +++ b/packages/server/review.ts @@ -70,6 +70,8 @@ export interface ReviewServerOptions { opencodeClient?: OpencodeClient; /** PR metadata when reviewing a pull request (PR mode) */ prMetadata?: PRMetadata; + /** Working directory for agent processes (e.g., --local worktree). Independent of diff pipeline. */ + agentCwd?: string; /** Cleanup callback invoked when server stops (e.g., remove temp worktree) */ onCleanup?: () => void | Promise; } @@ -128,12 +130,19 @@ export async function startReviewServer( mode: "review", getServerUrl: () => serverUrl, getCwd: () => { + if (options.agentCwd) return options.agentCwd; return resolveVcsCwd(currentDiffType, gitContext?.cwd) ?? process.cwd(); }, async buildCommand(provider) { - const cwd = resolveVcsCwd(currentDiffType, gitContext?.cwd) ?? process.cwd(); - const userMessage = buildCodexReviewUserMessage(currentPatch, currentDiffType, gitContext, prMetadata); + const cwd = options.agentCwd ?? resolveVcsCwd(currentDiffType, gitContext?.cwd) ?? process.cwd(); + const hasAgentLocalAccess = !!options.agentCwd || !!gitContext; + const userMessage = buildCodexReviewUserMessage( + currentPatch, + currentDiffType, + { defaultBranch: gitContext?.defaultBranch, hasLocalAccess: hasAgentLocalAccess }, + prMetadata, + ); if (provider === "codex") { const outputPath = generateOutputPath(); @@ -170,7 +179,7 @@ export async function startReviewServer( }; if (output.findings.length > 0) { - const cwd = resolveVcsCwd(currentDiffType, gitContext?.cwd) ?? process.cwd(); + const cwd = options.agentCwd ?? resolveVcsCwd(currentDiffType, gitContext?.cwd) ?? process.cwd(); const authorName = job.provider === "codex" ? "Codex" : job.provider === "claude" ? "Claude Code" : "Review Agent"; const annotations = transformReviewFindings(output.findings, job.source, cwd, authorName); const result = externalAnnotations.addAnnotations({ annotations }); @@ -260,6 +269,7 @@ export async function startReviewServer( registry: aiRegistry, sessionManager: aiSessionManager, getCwd: () => { + if (options.agentCwd) return options.agentCwd; return resolveVcsCwd(currentDiffType, gitContext?.cwd) ?? process.cwd(); }, }); @@ -332,6 +342,7 @@ export async function startReviewServer( shareBaseUrl, repoInfo, isWSL: wslFlag, + ...(options.agentCwd && { agentCwd: options.agentCwd }), ...(isPRMode && { prMetadata, platformUser }), ...(isPRMode && initialViewedFiles.length > 0 && { viewedFiles: initialViewedFiles }), ...(currentError && { error: currentError }), @@ -417,7 +428,7 @@ export async function startReviewServer( } } - // Prefer local file access (works for both local mode and hybrid --local mode) + // Local review: read file contents from local git if (hasLocalAccess) { const defaultBranch = gitContext?.defaultBranch || "main"; const defaultCwd = gitContext?.cwd; @@ -431,8 +442,8 @@ export async function startReviewServer( return Response.json(result); } + // PR mode: fetch from platform API using base/head SHAs if (isPRMode) { - // Pure PR mode: fetch from platform API using base/head SHAs const [oldContent, newContent] = await Promise.all([ fetchPRFileContent(prRef!, prMetadata.baseSha, oldPath || filePath), fetchPRFileContent(prRef!, prMetadata.headSha, filePath), From b5e27d528d415f906da4295e4dc4771c277cab1b Mon Sep 17 00:00:00 2001 From: Michael Ramos Date: Sun, 5 Apr 2026 21:11:51 -0700 Subject: [PATCH 07/37] style(review): unify panel headers, responsive buttons, dockview polish - Unify all panel headers at 33px via --panel-header-h CSS variable - FileHeader now uses shared variable instead of hardcoded 30px - Dockview tab bar height, font size, and padding aligned with sidebars - FileTree header uses fixed height instead of padding-based sizing - Consistent border opacity (border-border/50) across all panels - Consistent font weight (font-semibold) on all header labels - Remove dockview tab focus outline (::after pseudo-element) - Dockview tab close button pushed to right edge - Tab bar void area uses muted background - Top app header compacted from h-12 to py-1 - Sidebar footer: copy button and diff stats side by side - FeedbackButton responsive labels (Send/Post at md, full labels at lg) - Move ReviewAgentsIcon to packages/ui for shared use - Agent empty state uses shared ReviewAgentsIcon instead of hardcoded SVG - Sidebar header label truncates when narrow (tabs never clip) - Detail panel prompt disclosures get proper spacing - React.memo on PR tab components (PRSummaryTab, PRCommentsTab) - Inline onerror on img tags for broken GitHub image handling For provenance purposes, this commit was AI assisted. --- packages/review-editor/App.tsx | 17 +++-- .../review-editor/components/FileHeader.tsx | 4 +- .../review-editor/components/FileTree.tsx | 69 +++++++++---------- .../components/PRCommentsTab.tsx | 4 +- .../review-editor/components/PRSummaryTab.tsx | 32 +++------ .../components/ReviewSidebar.tsx | 8 +-- .../dock/ReviewDockTabRenderer.tsx | 2 +- .../dock/panels/ReviewAgentJobDetailPanel.tsx | 54 +++++++-------- packages/review-editor/index.css | 22 ++++-- packages/ui/components/AgentsTab.tsx | 5 +- .../components/ReviewAgentsIcon.tsx | 0 packages/ui/components/ToolbarButtons.tsx | 13 +++- 12 files changed, 115 insertions(+), 115 deletions(-) rename packages/{review-editor => ui}/components/ReviewAgentsIcon.tsx (100%) diff --git a/packages/review-editor/App.tsx b/packages/review-editor/App.tsx index 6e17bdfe..ce1d5abf 100644 --- a/packages/review-editor/App.tsx +++ b/packages/review-editor/App.tsx @@ -153,6 +153,7 @@ const ReviewApp: React.FC = () => { const [isWSL, setIsWSL] = useState(false); const [diffType, setDiffType] = useState('uncommitted'); const [gitContext, setGitContext] = useState(null); + const [agentCwd, setAgentCwd] = useState(null); const [isLoadingDiff, setIsLoadingDiff] = useState(false); const [diffError, setDiffError] = useState(null); const [isSendingFeedback, setIsSendingFeedback] = useState(false); @@ -619,6 +620,7 @@ const ReviewApp: React.FC = () => { origin?: Origin; diffType?: string; gitContext?: GitContext; + agentCwd?: string; sharingEnabled?: boolean; repoInfo?: { display: string; branch?: string }; prMetadata?: PRMetadata; @@ -646,6 +648,7 @@ const ReviewApp: React.FC = () => { if (data.origin) setOrigin(data.origin); if (data.diffType) setDiffType(data.diffType); if (data.gitContext) setGitContext(data.gitContext); + if (data.agentCwd) setAgentCwd(data.agentCwd); if (data.sharingEnabled !== undefined) setSharingEnabled(data.sharingEnabled); if (data.repoInfo) setRepoInfo(data.repoInfo); if (data.prMetadata) setPrMetadata(data.prMetadata); @@ -1309,11 +1312,11 @@ const ReviewApp: React.FC = () => {
{/* Header */} -
+
{prMetadata ? (
- {prMetadata && gitContext && ( + {prMetadata && (gitContext || agentCwd) && (
{/* Worktree info dialog */} - {gitContext?.cwd && prMetadata && ( + {(gitContext?.cwd || agentCwd) && prMetadata && ( setShowWorktreeDialog(false)} @@ -1801,15 +1806,15 @@ const ReviewApp: React.FC = () => { wide message={
-

This PR is checked out in a temporary local worktree for full file access.

+

This PR is checked out locally so review agents have full file access.

Path

Automatically removed when this review session ends.

diff --git a/packages/review-editor/components/FileHeader.tsx b/packages/review-editor/components/FileHeader.tsx index a40d359f..30759cf0 100644 --- a/packages/review-editor/components/FileHeader.tsx +++ b/packages/review-editor/components/FileHeader.tsx @@ -79,8 +79,8 @@ export const FileHeader: React.FC = ({ return (
diff --git a/packages/review-editor/components/FileTree.tsx b/packages/review-editor/components/FileTree.tsx index 23c93798..d657476f 100644 --- a/packages/review-editor/components/FileTree.tsx +++ b/packages/review-editor/components/FileTree.tsx @@ -166,11 +166,11 @@ export const FileTree: React.FC = ({ }, []); return ( -
); -}; +}); // --------------------------------------------------------------------------- // Subcomponents diff --git a/packages/review-editor/components/PRSummaryTab.tsx b/packages/review-editor/components/PRSummaryTab.tsx index 0654c256..16426e32 100644 --- a/packages/review-editor/components/PRSummaryTab.tsx +++ b/packages/review-editor/components/PRSummaryTab.tsx @@ -1,21 +1,8 @@ -import React, { useMemo, useRef, useEffect } from 'react'; +import React, { useMemo } from 'react'; import DOMPurify from 'dompurify'; import { parseMarkdownToBlocks } from '@plannotator/ui/utils/parser'; import { renderInlineMarkdown } from '../utils/renderInlineMarkdown'; -import type { PRContext } from '@plannotator/shared/pr-provider'; - -/** Renders sanitized HTML with onerror handlers on images to prevent 404 flicker loops. */ -function SafeHtmlBlock({ html, tag: Tag = 'div', ...props }: { html: string; tag?: 'div' | 'span'; [key: string]: any }) { - const ref = useRef(null); - useEffect(() => { - if (!ref.current) return; - ref.current.querySelectorAll('img').forEach((img) => { - img.onerror = () => { img.style.display = 'none'; img.onerror = null; }; - }); - }, [html]); - return ; -} -import type { PRMetadata } from '@plannotator/shared/pr-provider'; +import type { PRContext, PRMetadata } from '@plannotator/shared/pr-provider'; interface PRSummaryTabProps { context: PRContext; @@ -28,7 +15,7 @@ const containsHtml = (text: string) => HTML_TAG_RE.test(text); /** Sanitize HTML using DOMPurify — defense-in-depth for GitHub API content. */ function sanitizeHtml(html: string): string { - return DOMPurify.sanitize(html, { + const clean = DOMPurify.sanitize(html, { ALLOWED_TAGS: [ 'sub', 'sup', 'b', 'i', 'em', 'strong', 'br', 'hr', 'p', 'span', 'del', 'ins', 'mark', 'small', 'abbr', 'kbd', 'var', 'samp', @@ -37,8 +24,11 @@ function sanitizeHtml(html: string): string { 'table', 'thead', 'tbody', 'tr', 'th', 'td', 'a', 'img', 'div', ], - ALLOWED_ATTR: ['href', 'src', 'alt', 'title', 'rel', 'target', 'width', 'height', 'align'], + ALLOWED_ATTR: ['href', 'src', 'alt', 'title', 'rel', 'target', 'width', 'height', 'align', 'onerror'], }); + // Inject inline onerror on img tags — hides broken images on first 404 synchronously + // (useEffect-based approaches are too late — the browser loads the image before effects fire) + return clean.replace(/; + return ; } return renderInlineMarkdown(content); } @@ -98,7 +88,7 @@ export function MarkdownBody({ markdown }: { markdown: string }) { if (!block.content) return null; // If the entire paragraph is HTML, sanitize and render if (containsHtml(block.content)) { - return ; + return
; } return

{renderInlineMarkdown(block.content)}

; } @@ -107,7 +97,7 @@ export function MarkdownBody({ markdown }: { markdown: string }) { ); } -export const PRSummaryTab: React.FC = ({ context, metadata }) => { +export const PRSummaryTab: React.FC = React.memo(({ context, metadata }) => { return (
{/* PR title + state */} @@ -194,4 +184,4 @@ export const PRSummaryTab: React.FC = ({ context, metadata }) )}
); -}; +}); diff --git a/packages/review-editor/components/ReviewSidebar.tsx b/packages/review-editor/components/ReviewSidebar.tsx index 884a1d7f..1a2abd8a 100644 --- a/packages/review-editor/components/ReviewSidebar.tsx +++ b/packages/review-editor/components/ReviewSidebar.tsx @@ -8,7 +8,7 @@ import { renderInlineMarkdown } from '../utils/renderInlineMarkdown'; import { formatRelativeTime } from '../utils/formatRelativeTime'; import { AITab } from './AITab'; import { SparklesIcon } from './SparklesIcon'; -import { ReviewAgentsIcon } from './ReviewAgentsIcon'; +import { ReviewAgentsIcon } from '@plannotator/ui/components/ReviewAgentsIcon'; import { AgentsTab } from '@plannotator/ui/components/AgentsTab'; import type { PRMetadata } from '@plannotator/shared/pr-provider'; import type { AIChatEntry } from '../hooks/useAIChat'; @@ -202,8 +202,8 @@ export const ReviewSidebar: React.FC = ({
); } diff --git a/packages/review-editor/dock/panels/ReviewAgentJobDetailPanel.tsx b/packages/review-editor/dock/panels/ReviewAgentJobDetailPanel.tsx index 373ec5ed..c6811710 100644 --- a/packages/review-editor/dock/panels/ReviewAgentJobDetailPanel.tsx +++ b/packages/review-editor/dock/panels/ReviewAgentJobDetailPanel.tsx @@ -1,6 +1,6 @@ import React, { useMemo, useState, useEffect, useCallback } from 'react'; import type { IDockviewPanelProps } from 'dockview-react'; -import type { AgentJobInfo, CodeAnnotation } from '@plannotator/ui/types'; +import { SEVERITY_STYLES, type AgentJobInfo, type CodeAnnotation } from '@plannotator/ui/types'; import { isTerminalStatus } from '@plannotator/shared/agent-jobs'; import { useReviewState } from '../ReviewStateContext'; import { useJobLogs } from '../JobLogsContext'; @@ -81,8 +81,9 @@ export const ReviewAgentJobDetailPanel: React.FC = (props) const dismissedCount = useMemo(() => displayAnnotations.filter((d) => d.dismissed).length, [displayAnnotations]); const handleAnnotationClick = useCallback((ann: CodeAnnotation) => { + state.openDiffFile(ann.filePath); state.onSelectAnnotation(ann.id); - }, [state.onSelectAnnotation]); + }, [state.openDiffFile, state.onSelectAnnotation]); const copyAllText = useMemo( () => activeAnnotations.length > 0 ? exportReviewFeedback(activeAnnotations, state.prMetadata) : '', @@ -311,11 +312,6 @@ function Disclosure({ title, copyText, nested, children }: { ); } -const SEVERITY_STYLES: Record = { - important: { dot: 'bg-destructive', label: 'Important' }, - nit: { dot: 'bg-amber-500', label: 'Nit' }, - pre_existing: { dot: 'bg-muted-foreground', label: 'Pre-existing' }, -}; function AnnotationRow({ annotation: ann, dismissed, onClick }: { annotation: CodeAnnotation; diff --git a/packages/server/agent-jobs.ts b/packages/server/agent-jobs.ts index 1362830c..8a53e742 100644 --- a/packages/server/agent-jobs.ts +++ b/packages/server/agent-jobs.ts @@ -306,6 +306,8 @@ export function createAgentJobHandler(options: AgentJobHandlerOptions): AgentJob entry.info.status = "killed"; entry.info.endedAt = Date.now(); + jobOutputPaths.delete(id); + jobOutputPaths.delete(`${id}:cwd`); broadcast({ type: "job:completed", job: { ...entry.info } }); return true; } diff --git a/packages/server/codex-review.ts b/packages/server/codex-review.ts index 6730c4ec..62abef7e 100644 --- a/packages/server/codex-review.ts +++ b/packages/server/codex-review.ts @@ -203,7 +203,7 @@ export function buildCodexReviewUserMessage( case "branch": { const base = options?.defaultBranch || "main"; - return `Review the code changes against the base branch '${base}'. Run \`git diff ${base}...HEAD\` to inspect the changes. Provide prioritized, actionable findings.`; + return `Review the code changes against the base branch '${base}'. Run \`git diff ${base}..HEAD\` to inspect the changes. Provide prioritized, actionable findings.`; } default: diff --git a/packages/server/path-utils.ts b/packages/server/path-utils.ts index 7e495f41..f0ffc080 100644 --- a/packages/server/path-utils.ts +++ b/packages/server/path-utils.ts @@ -2,9 +2,17 @@ * Strip a cwd prefix from an absolute path to get a repo-relative path. * Used by review agent transforms to convert absolute file paths from * agent output into diff-compatible relative paths. + * + * Uses path.relative for cross-platform support (Windows backslashes) + * and normalizes to forward slashes for git diff path matching. */ +import { relative } from "node:path"; + export function toRelativePath(absolutePath: string, cwd?: string): string { if (!cwd) return absolutePath; - const prefix = cwd.endsWith("/") ? cwd : cwd + "/"; - return absolutePath.startsWith(prefix) ? absolutePath.slice(prefix.length) : absolutePath; + const rel = relative(cwd, absolutePath); + // Don't relativize if the result goes outside cwd (different drive, symlink escape) + if (rel.startsWith("..")) return absolutePath; + // Normalize to forward slashes for diff path matching + return rel.replace(/\\/g, "/"); } diff --git a/packages/ui/types.ts b/packages/ui/types.ts index 89aa32ea..732d7e6e 100644 --- a/packages/ui/types.ts +++ b/packages/ui/types.ts @@ -80,6 +80,13 @@ export interface CodeAnnotation { reasoning?: string; // Validation chain — how the issue was confirmed (Claude) } +/** Severity display styles — shared between agent detail panel and inline diff annotations. */ +export const SEVERITY_STYLES: Record = { + important: { dot: 'bg-destructive', label: 'Important' }, + nit: { dot: 'bg-amber-500', label: 'Nit' }, + pre_existing: { dot: 'bg-muted-foreground', label: 'Pre-existing' }, +}; + // For @pierre/diffs integration export interface DiffAnnotationMetadata { annotationId: string; From 4124841b9cad3a5dd653430469a4761f93c36fbb Mon Sep 17 00:00:00 2001 From: Michael Ramos Date: Mon, 6 Apr 2026 09:09:36 -0700 Subject: [PATCH 24/37] perf: wrap ReviewSidebar in React.memo to prevent re-renders during log streaming MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Every job:log SSE event triggers setJobLogs in useAgentJobs, which re-renders App.tsx. Without memo, the sidebar re-renders on every event (~5/sec) even though its props (agentJobs.jobs, capabilities, callbacks) haven't changed. This caused visible flickering when a review tab was open during agent runs. React.memo shallow-compares props — all sidebar props are stable references (jobs array only changes on status events, callbacks are useCallback-wrapped), so the sidebar correctly skips re-renders during log streaming. For provenance purposes, this commit was AI assisted. --- packages/review-editor/components/ReviewSidebar.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/review-editor/components/ReviewSidebar.tsx b/packages/review-editor/components/ReviewSidebar.tsx index fdb768e7..3afa916d 100644 --- a/packages/review-editor/components/ReviewSidebar.tsx +++ b/packages/review-editor/components/ReviewSidebar.tsx @@ -107,7 +107,7 @@ function compareCodeAnnotations(a: CodeAnnotation, b: CodeAnnotation): number { } -export const ReviewSidebar: React.FC = ({ +export const ReviewSidebar: React.FC = React.memo(({ isOpen, onToggle, annotations, @@ -461,4 +461,4 @@ export const ReviewSidebar: React.FC = ({ )} ); -}; +}); From cda32454f37c1f50400609a514a47d793a83b128 Mon Sep 17 00:00:00 2001 From: Michael Ramos Date: Mon, 6 Apr 2026 09:19:06 -0700 Subject: [PATCH 25/37] fix(security): remove find/ls/cat from Claude allowed tools, add glab CLI MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Security: Bash(find:*) allowed find -exec to spawn arbitrary subprocesses that bypassed --disallowedTools. Removed find, ls, and cat — Claude has Glob, Read, and Grep built-in which cover file access without shell exec. Feature: Added glab mr view/diff/list and glab api to allowed tools so Claude can inspect GitLab MR context in remote-mode reviews. For provenance purposes, this commit was AI assisted. --- packages/server/claude-review.ts | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/packages/server/claude-review.ts b/packages/server/claude-review.ts index f124f22b..55dd167f 100644 --- a/packages/server/claude-review.ts +++ b/packages/server/claude-review.ts @@ -192,16 +192,21 @@ export interface ClaudeCommandResult { export function buildClaudeCommand(prompt: string): ClaudeCommandResult { const allowedTools = [ "Agent", "Read", "Glob", "Grep", + // GitHub CLI "Bash(gh pr view:*)", "Bash(gh pr diff:*)", "Bash(gh pr list:*)", "Bash(gh issue view:*)", "Bash(gh issue list:*)", "Bash(gh api repos/*/*/pulls/*)", "Bash(gh api repos/*/*/pulls/*/files*)", "Bash(gh api repos/*/*/pulls/*/comments*)", "Bash(gh api repos/*/*/issues/*/comments*)", + // GitLab CLI + "Bash(glab mr view:*)", "Bash(glab mr diff:*)", "Bash(glab mr list:*)", + "Bash(glab api:*)", + // Git (read-only) "Bash(git status:*)", "Bash(git diff:*)", "Bash(git log:*)", "Bash(git show:*)", "Bash(git blame:*)", "Bash(git branch:*)", "Bash(git grep:*)", "Bash(git ls-remote:*)", "Bash(git ls-tree:*)", "Bash(git merge-base:*)", "Bash(git remote:*)", "Bash(git rev-parse:*)", "Bash(git show-ref:*)", - "Bash(find:*)", "Bash(ls:*)", "Bash(cat:*)", "Bash(wc:*)", + "Bash(wc:*)", ].join(","); const disallowedTools = [ From 5bad2e958920e5b34fba8451fa80e08c1469849c Mon Sep 17 00:00:00 2001 From: Michael Ramos Date: Mon, 6 Apr 2026 09:21:47 -0700 Subject: [PATCH 26/37] fix: Pi addAnnotations, Pi stdout drain, cross-repo exit codes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Pi extension: - Add addAnnotations() to external-annotations.ts return object — serverReview.ts calls it when agent jobs complete but the method was missing (build/runtime error) - Change proc.on('exit') to proc.on('close') in agent-jobs.ts — Node's 'exit' fires before stdio streams drain, so stdoutBuf could be incomplete when onJobComplete parses Claude's JSONL result Cross-repo --local: - Check git checkout FETCH_HEAD exit code — throw if it fails so the outer catch falls back to remote-only with a clear warning - Log warning if baseSha fetch fails (non-fatal, agents just can't diff locally) For provenance purposes, this commit was AI assisted. --- apps/hook/server/index.ts | 13 ++++++++----- apps/pi-extension/server/agent-jobs.ts | 5 +++-- apps/pi-extension/server/external-annotations.ts | 8 ++++++++ 3 files changed, 19 insertions(+), 7 deletions(-) diff --git a/apps/hook/server/index.ts b/apps/hook/server/index.ts index 27e80fe2..f8bb77ca 100644 --- a/apps/hook/server/index.ts +++ b/apps/hook/server/index.ts @@ -336,12 +336,15 @@ if (args[0] === "sessions") { ); if (fetchResult.exitCode !== 0) throw new Error(`Failed to fetch PR head ref: ${new TextDecoder().decode(fetchResult.stderr).trim()}`); - // Step 3: Checkout and ensure base is available - Bun.spawnSync(["git", "checkout", "FETCH_HEAD"], { cwd: localPath, stderr: "pipe" }); + // Step 3: Checkout PR head (critical — if this fails, worktree is empty) + const checkoutResult = Bun.spawnSync(["git", "checkout", "FETCH_HEAD"], { cwd: localPath, stderr: "pipe" }); + if (checkoutResult.exitCode !== 0) { + throw new Error(`git checkout FETCH_HEAD failed: ${new TextDecoder().decode(checkoutResult.stderr).trim()}`); + } - // Fetch the exact base SHA and create both local and remote-tracking refs - // so `git diff main...HEAD` and `git diff origin/main...HEAD` both work. - Bun.spawnSync(["git", "fetch", "--depth=50", "origin", prMetadata.baseSha], { cwd: localPath, stderr: "pipe" }); + // Best-effort: create base refs so `git diff main...HEAD` and `git diff origin/main...HEAD` work + const baseFetch = Bun.spawnSync(["git", "fetch", "--depth=50", "origin", prMetadata.baseSha], { cwd: localPath, stderr: "pipe" }); + if (baseFetch.exitCode !== 0) console.error("Warning: failed to fetch baseSha, agent diffs may be inaccurate"); Bun.spawnSync(["git", "branch", "--", prMetadata.baseBranch, prMetadata.baseSha], { cwd: localPath, stderr: "pipe" }); Bun.spawnSync(["git", "update-ref", `refs/remotes/origin/${prMetadata.baseBranch}`, prMetadata.baseSha], { cwd: localPath, stderr: "pipe" }); diff --git a/apps/pi-extension/server/agent-jobs.ts b/apps/pi-extension/server/agent-jobs.ts index d019cc93..2ca1b1b8 100644 --- a/apps/pi-extension/server/agent-jobs.ts +++ b/apps/pi-extension/server/agent-jobs.ts @@ -208,8 +208,9 @@ export function createAgentJobHandler(options: AgentJobHandlerOptions) { }); } - // Monitor process exit - proc.on("exit", async (exitCode) => { + // Monitor process close (fires after stdio streams are fully drained, + // unlike 'exit' which fires before — critical for stdout capture) + proc.on("close", async (exitCode) => { // Flush remaining stderr if (logFlushTimer) { clearTimeout(logFlushTimer); logFlushTimer = null; } if (logPending) { diff --git a/apps/pi-extension/server/external-annotations.ts b/apps/pi-extension/server/external-annotations.ts index 6ba151b5..4bb48aa3 100644 --- a/apps/pi-extension/server/external-annotations.ts +++ b/apps/pi-extension/server/external-annotations.ts @@ -49,6 +49,14 @@ export function createExternalAnnotationHandler(mode: "plan" | "review") { }); return { + /** Push annotations directly into the store (bypasses HTTP, reuses same validation). */ + addAnnotations(body: unknown): { ids: string[] } | { error: string } { + const parsed = transform(body); + if ("error" in parsed) return { error: parsed.error }; + const created = store.add(parsed.annotations); + return { ids: created.map((a: { id: string }) => a.id) }; + }, + async handle( req: IncomingMessage, res: ServerResponse, From ee6ede39e844fc988bb37368fdc892c3bf4925bf Mon Sep 17 00:00:00 2001 From: Michael Ramos Date: Mon, 6 Apr 2026 09:46:31 -0700 Subject: [PATCH 27/37] fix: FETCH_HEAD ordering, Pi worktree-aware cwd, SEVERITY_ORDER hoisted MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Critical: - Move ensureObjectAvailable before PR head fetch — it can overwrite FETCH_HEAD if baseSha needs fetching, causing createWorktree to check out the base commit instead of the PR head - Pi serverReview.ts: extract resolveAgentCwd() helper used by getCwd, buildCommand, and onJobComplete — was bypassing worktree-aware path resolution, causing agents to run in wrong directory Cleanup: - Hoist SEVERITY_ORDER to module scope in ReviewAgentJobDetailPanel (was recreated inside component body on every render) For provenance purposes, this commit was AI assisted. --- apps/hook/server/index.ts | 12 +++++------ apps/pi-extension/server/serverReview.ts | 20 ++++++++++--------- .../dock/panels/ReviewAgentJobDetailPanel.tsx | 3 ++- 3 files changed, 19 insertions(+), 16 deletions(-) diff --git a/apps/hook/server/index.ts b/apps/hook/server/index.ts index f8bb77ca..84d03088 100644 --- a/apps/hook/server/index.ts +++ b/apps/hook/server/index.ts @@ -287,14 +287,14 @@ if (args[0] === "sessions") { if (isSameRepo) { // ── Same-repo: fast worktree path ── console.error("Fetching PR branch and creating local worktree..."); - // Fetch base branch FIRST so origin/ is current — without this, - // `git diff origin/main...HEAD` in the worktree uses stale refs and shows - // changes from other PRs that were merged since the last fetch. + // Fetch base branch so origin/ is current for agent diffs. + // Ensure baseSha is available (may fetch, which overwrites FETCH_HEAD). + // Both MUST happen before the PR head fetch since FETCH_HEAD is what + // createWorktree uses — the PR head fetch must be last. await fetchRef(gitRuntime, prMetadata.baseBranch, { cwd: repoDir }); - // Fetch PR head SECOND — this sets FETCH_HEAD to the PR tip, which - // createWorktree needs. Order matters: fetchRef overwrites FETCH_HEAD. - await fetchRef(gitRuntime, fetchRefStr, { cwd: repoDir }); await ensureObjectAvailable(gitRuntime, prMetadata.baseSha, { cwd: repoDir }); + // Fetch PR head LAST — sets FETCH_HEAD to the PR tip for createWorktree. + await fetchRef(gitRuntime, fetchRefStr, { cwd: repoDir }); await createWorktree(gitRuntime, { ref: "FETCH_HEAD", diff --git a/apps/pi-extension/server/serverReview.ts b/apps/pi-extension/server/serverReview.ts index ef6a7aaf..6a66af9b 100644 --- a/apps/pi-extension/server/serverReview.ts +++ b/apps/pi-extension/server/serverReview.ts @@ -168,19 +168,21 @@ export async function startReviewServer(options: { // Agent jobs — background process manager (late-binds serverUrl via getter) let serverUrl = ""; + // Worktree-aware cwd resolver — shared by getCwd, buildCommand, and onJobComplete + function resolveAgentCwd(): string { + if (currentDiffType.startsWith("worktree:")) { + const parsed = parseWorktreeDiffType(currentDiffType); + if (parsed) return parsed.path; + } + return options.gitContext?.cwd ?? process.cwd(); + } const agentJobs = createAgentJobHandler({ mode: "review", getServerUrl: () => serverUrl, - getCwd: () => { - if (currentDiffType.startsWith("worktree:")) { - const parsed = parseWorktreeDiffType(currentDiffType); - if (parsed) return parsed.path; - } - return options.gitContext?.cwd ?? process.cwd(); - }, + getCwd: resolveAgentCwd, async buildCommand(provider) { - const cwd = options.gitContext?.cwd ?? process.cwd(); + const cwd = resolveAgentCwd(); const hasLocalAccess = !!options.gitContext; const userMessage = buildCodexReviewUserMessage( currentPatch, @@ -206,7 +208,7 @@ export async function startReviewServer(options: { }, async onJobComplete(job, meta) { - const cwd = options.gitContext?.cwd ?? process.cwd(); + const cwd = resolveAgentCwd(); if (job.provider === "codex" && meta.outputPath) { const output = await parseCodexOutput(meta.outputPath); diff --git a/packages/review-editor/dock/panels/ReviewAgentJobDetailPanel.tsx b/packages/review-editor/dock/panels/ReviewAgentJobDetailPanel.tsx index c6811710..11190e37 100644 --- a/packages/review-editor/dock/panels/ReviewAgentJobDetailPanel.tsx +++ b/packages/review-editor/dock/panels/ReviewAgentJobDetailPanel.tsx @@ -15,6 +15,8 @@ import { exportReviewFeedback } from '../../utils/exportFeedback'; type DetailTab = 'findings' | 'logs'; +const SEVERITY_ORDER: Record = { important: 0, nit: 1, pre_existing: 2 }; + export const ReviewAgentJobDetailPanel: React.FC = (props) => { const jobId: string = props.params?.jobId ?? ''; const state = useReviewState(); @@ -65,7 +67,6 @@ export const ReviewAgentJobDetailPanel: React.FC = (props) }); }, [state.externalAnnotations, job]); - const SEVERITY_ORDER: Record = { important: 0, nit: 1, pre_existing: 2 }; const displayAnnotations = useMemo(() => Array.from(annotationSnapshot.values()).sort((a, b) => { if (a.dismissed !== b.dismissed) return a.dismissed ? 1 : -1; From 2da62a4064904667a60288c75a0e2aeca42ff1ee Mon Sep 17 00:00:00 2001 From: Michael Ramos Date: Mon, 6 Apr 2026 09:52:17 -0700 Subject: [PATCH 28/37] =?UTF-8?q?fix:=20Bun/Pi=20parity=20=E2=80=94=20prov?= =?UTF-8?q?ider=20default,=20annotation=20error=20logging?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Bun agent-jobs: change provider default from "shell" to "" (shell was removed from capabilities, default should match Pi) - Pi serverReview: log errors from addAnnotations in onJobComplete (Bun logs them, Pi was silently ignoring) For provenance purposes, this commit was AI assisted. --- apps/pi-extension/server/serverReview.ts | 6 ++++-- packages/server/agent-jobs.ts | 2 +- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/apps/pi-extension/server/serverReview.ts b/apps/pi-extension/server/serverReview.ts index 6a66af9b..1b11c704 100644 --- a/apps/pi-extension/server/serverReview.ts +++ b/apps/pi-extension/server/serverReview.ts @@ -222,7 +222,8 @@ export async function startReviewServer(options: { if (output.findings.length > 0) { const annotations = transformReviewFindings(output.findings, job.source, cwd, "Codex"); - externalAnnotations.addAnnotations({ annotations }); + const result = externalAnnotations.addAnnotations({ annotations }); + if ("error" in result) console.error(`[codex-review] addAnnotations error:`, result.error); } return; } @@ -240,7 +241,8 @@ export async function startReviewServer(options: { if (output.findings.length > 0) { const annotations = transformClaudeFindings(output.findings, job.source, cwd); - externalAnnotations.addAnnotations({ annotations }); + const result = externalAnnotations.addAnnotations({ annotations }); + if ("error" in result) console.error(`[claude-review] addAnnotations error:`, result.error); } return; } diff --git a/packages/server/agent-jobs.ts b/packages/server/agent-jobs.ts index 8a53e742..14964921 100644 --- a/packages/server/agent-jobs.ts +++ b/packages/server/agent-jobs.ts @@ -402,7 +402,7 @@ export function createAgentJobHandler(options: AgentJobHandlerOptions): AgentJob if (url.pathname === JOBS && req.method === "POST") { try { const body = await req.json(); - const provider = typeof body.provider === "string" ? body.provider : "shell"; + const provider = typeof body.provider === "string" ? body.provider : ""; let rawCommand = Array.isArray(body.command) ? body.command : []; let command = rawCommand.filter((c: unknown): c is string => typeof c === "string"); let label = typeof body.label === "string" ? body.label : `${provider} agent`; From 82dc068c1f3ff80a0fc32a783a99d3b2c6b2a4f8 Mon Sep 17 00:00:00 2001 From: Michael Ramos Date: Mon, 6 Apr 2026 09:56:57 -0700 Subject: [PATCH 29/37] docs: add AI Code Review Agents guide with full prompt transparency New docs page covering: - Overview of Codex and Claude review agents - How findings work (severity/priority, reasoning, navigation) - Local worktree behavior (same-repo vs cross-repo) - Full transparency section with: - Claude multi-agent pipeline prompt (all 6 steps) - Claude command and allowed/blocked tools - Codex review prompt and command - Both output schemas (Claude severity + Codex priority) - Security notes (read-only, no network, local execution, no commenting) - Customization via CLAUDE.md and REVIEW.md For provenance purposes, this commit was AI assisted. --- .../src/content/docs/guides/ai-code-review.md | 233 ++++++++++++++++++ 1 file changed, 233 insertions(+) create mode 100644 apps/marketing/src/content/docs/guides/ai-code-review.md diff --git a/apps/marketing/src/content/docs/guides/ai-code-review.md b/apps/marketing/src/content/docs/guides/ai-code-review.md new file mode 100644 index 00000000..6329a2cc --- /dev/null +++ b/apps/marketing/src/content/docs/guides/ai-code-review.md @@ -0,0 +1,233 @@ +--- +title: "AI Code Review Agents" +description: "Automated code review using Codex and Claude Code agents with live findings, severity classification, and full prompt transparency." +sidebar: + order: 26 +section: "Guides" +--- + +Plannotator can launch AI review agents that analyze your code changes and produce structured findings directly in the diff viewer. Agents run as background processes with live log streaming — you continue reviewing while they work. + +## Overview + +When reviewing code (local changes or a PR), click **Run Agent** in the Agents tab to launch a review. Two providers are supported: + +- **Codex CLI** — uses OpenAI's Codex with structured output schema and priority-based findings (P0–P3) +- **Claude Code** — uses Anthropic's Claude with a multi-agent pipeline and severity-based findings (Important / Nit / Pre-existing) + +Each provider has its own review model. Findings appear as annotations in the diff viewer, sorted by severity, with inline reasoning explaining how each issue was verified. + +## How it works + +``` +User clicks Run Agent (Codex or Claude) + ↓ +Server builds the review command with appropriate prompt + schema + ↓ +Agent process spawns in the background + ↓ +Live logs stream to the Logs tab in real time + ↓ +On completion, findings are parsed and injected as annotations + ↓ +Annotations appear inline in the diff viewer + in the Findings tab +``` + +For PR reviews, a local worktree is created by default so the agent has full file access. Use `--no-local` to opt out. + +## Findings + +Each finding includes: + +- **File and line range** — pinpointed to the specific code +- **Description** — what's wrong and why it matters +- **Severity** (Claude) or **Priority** (Codex) — how urgent the fix is +- **Reasoning** (Claude) — the validation chain explaining how the issue was confirmed + +Findings are clickable — clicking navigates to the file and highlights the annotation. Copy buttons on each finding and a "Copy All" action export findings as structured markdown. + +### Claude severity levels + +| Severity | Meaning | +|----------|---------| +| **Important** | A bug to fix before merging — build failures, logic errors, security vulnerabilities | +| **Nit** | Minor issue worth fixing but non-blocking — style deviations, edge cases, code quality | +| **Pre-existing** | A bug in surrounding code not introduced by this PR | + +### Codex priority levels + +| Priority | Meaning | +|----------|---------| +| **P0** | Drop everything to fix — blocking | +| **P1** | Urgent — address in the next cycle | +| **P2** | Normal — to be fixed eventually | +| **P3** | Low — nice to have | + +## Local worktree + +For PR/MR reviews, Plannotator automatically creates a temporary local checkout so agents can read files, follow imports, and understand the codebase — not just the diff. + +- **Same-repo PRs**: fast git worktree (shared objects, no network transfer for files) +- **Cross-repo PRs**: shallow clone with targeted fetch of the PR head + +The worktree is cleaned up automatically when the review session ends. Use `--no-local` to skip worktree creation and review in remote-only mode. + +## Transparency + +Plannotator is fully transparent about what it sends to AI providers. Every prompt, schema, and command is visible in the review UI and documented here. + +### Claude Code review prompt + +Claude uses a multi-agent pipeline with 4 parallel review agents and a validation step: + +``` +Step 1: Gather context + - Retrieve the PR diff + - Read CLAUDE.md and REVIEW.md at the repo root and in modified directories + - Build a map of which rules apply to which file paths + +Step 2: Launch 4 parallel review agents + Agent 1 — Bug + Regression (Opus-level reasoning) + Agent 2 — Security + Deep Analysis (Opus-level reasoning) + Agent 3 — Code Quality + Reusability (Sonnet-level reasoning) + Agent 4 — Guideline Compliance (Haiku-level reasoning) + +Step 3: Validate each candidate finding + For each candidate, a validation agent traces the actual code path, + checks if the issue is handled elsewhere, and confirms with high + confidence. Failed validations are silently dropped. + +Step 4: Classify as important / nit / pre_existing + +Step 5: Deduplicate and rank by severity + +Step 6: Return structured JSON findings +``` + +**Hard constraints enforced in the prompt:** +- Never approve or block the PR +- Never comment on formatting or code style unless guidance files say to +- Never flag missing test coverage unless guidance files say to +- Never invent rules — only enforce what CLAUDE.md or REVIEW.md state +- Prefer silence over false positives + +### Claude Code command + +```bash +claude -p \ + --permission-mode dontAsk \ + --output-format stream-json \ + --verbose \ + --json-schema \ + --no-session-persistence \ + --model sonnet \ + --tools Agent,Bash,Read,Glob,Grep \ + --allowedTools \ + --disallowedTools +``` + +The prompt is written to stdin. The `--json-schema` flag enforces structured output with findings, severity, and reasoning fields. + +### Claude allowed tools + +The agent can use these tools during review: + +| Category | Tools | +|----------|-------| +| **Built-in** | Agent (subagents), Read, Glob, Grep | +| **GitHub CLI** | `gh pr view/diff/list`, `gh issue view/list`, `gh api repos/*/pulls/*` | +| **GitLab CLI** | `glab mr view/diff/list`, `glab api` | +| **Git (read-only)** | `git status/diff/log/show/blame/branch/grep/ls-remote/ls-tree/merge-base/remote/rev-parse/show-ref` | +| **Utility** | `wc` (word/line count) | + +**Explicitly blocked:** Edit, Write, WebFetch, WebSearch, Python, Node, Bash shells (sh/bash/zsh), curl, wget. The agent is read-only — it cannot modify your code. + +### Codex review prompt + +Codex uses OpenAI's structured output with a review-specific system prompt adapted from the [Codex CLI review guidelines](https://github.com/openai/codex): + +- Focus on bugs the original author would fix if they knew about them +- One finding per distinct issue with minimal line ranges +- Priority tagging (P0–P3) based on severity +- Overall correctness verdict at the end + +### Codex command + +```bash +codex exec \ + --output-schema \ + -o \ + --full-auto \ + --ephemeral \ + -C \ + "" +``` + +Codex writes structured JSON to the output file. The schema enforces findings with title, body, confidence score, priority, and code location. + +### Output schemas + +**Claude schema:** +```json +{ + "findings": [{ + "severity": "important | nit | pre_existing", + "file": "path/to/file.ts", + "line": 42, + "end_line": 45, + "description": "What's wrong", + "reasoning": "How it was verified" + }], + "summary": { + "important": 2, + "nit": 1, + "pre_existing": 0 + } +} +``` + +**Codex schema:** +```json +{ + "findings": [{ + "title": "[P1] Issue summary", + "body": "Detailed explanation", + "confidence_score": 0.95, + "priority": 1, + "code_location": { + "absolute_file_path": "/path/to/file.ts", + "line_range": { "start": 42, "end": 45 } + } + }], + "overall_correctness": "Incorrect", + "overall_explanation": "Summary of verdict", + "overall_confidence_score": 0.85 +} +``` + +## Security notes + +- **Read-only agents**: Review agents cannot modify your code. Edit, Write, and shell execution tools are explicitly blocked. +- **No external network access**: WebFetch, WebSearch, curl, and wget are blocked. Agents can only read local files and use `gh`/`glab` CLI for platform API access. +- **Local execution**: Agents run on your machine as background processes. No code is sent to Plannotator servers — all AI communication goes directly to the provider (Anthropic or OpenAI). +- **Temporary worktrees**: Local checkouts are created in system temp directories and cleaned up on session end. They use shallow clones or detached worktrees to minimize disk usage. +- **CLAUDE.md / REVIEW.md**: The Claude agent reads these files to understand project conventions. These files are part of your repository and under your control. +- **No GitHub commenting**: Agents never post comments, approve, or block PRs. All findings stay in the Plannotator UI unless you explicitly use "Post Comments" to submit them. + +## Customization + +### CLAUDE.md + +Add a `CLAUDE.md` file to your repository root or any subdirectory. The Claude review agent reads it to understand project-specific rules: + +```markdown +# Code Review Rules + +- Always check for SQL injection in database queries +- Skip test fixtures in test-fixtures/ +- Enforce snake_case for Python files +``` + +### REVIEW.md + +Similar to CLAUDE.md but specifically for review rules. Both files are additive — REVIEW.md adds to CLAUDE.md, it doesn't replace it. From 49347b802908feda547624e61011bb9873e19741 Mon Sep 17 00:00:00 2001 From: Michael Ramos Date: Mon, 6 Apr 2026 10:01:58 -0700 Subject: [PATCH 30/37] docs: add provenance links for Claude and Codex review integrations Credit Anthropic's Claude Code Review service, the open-source code-review plugin, and OpenAI's Codex CLI as the foundations for our review agent integrations. For provenance purposes, this commit was AI assisted. --- apps/marketing/src/content/docs/guides/ai-code-review.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/apps/marketing/src/content/docs/guides/ai-code-review.md b/apps/marketing/src/content/docs/guides/ai-code-review.md index 6329a2cc..ace6931d 100644 --- a/apps/marketing/src/content/docs/guides/ai-code-review.md +++ b/apps/marketing/src/content/docs/guides/ai-code-review.md @@ -17,6 +17,8 @@ When reviewing code (local changes or a PR), click **Run Agent** in the Agents t Each provider has its own review model. Findings appear as annotations in the diff viewer, sorted by severity, with inline reasoning explaining how each issue was verified. +Our Claude integration is derived from Anthropic's official [Claude Code Review](https://code.claude.com/docs/en/code-review) service and the open-source [code-review plugin](https://github.com/anthropics/claude-code/blob/main/plugins/code-review/README.md). Our Codex integration is based on [OpenAI Codex CLI](https://github.com/openai/codex) and its structured output capabilities. We adapted both for Plannotator's interactive review UI — the underlying review methodology comes from each provider's official tooling. + ## How it works ``` From 0e8e0a80a97f8eaa05a9ad5992fdd573476c92fc Mon Sep 17 00:00:00 2001 From: Michael Ramos Date: Mon, 6 Apr 2026 10:20:22 -0700 Subject: [PATCH 31/37] fix: temp clone leak, j/k key conflict, thread header null line MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Cross-repo: clean up localPath in catch block when fetch/checkout fails after clone succeeds (directory was leaking in /tmp) - Remove j/k/arrow keyboard navigation from PR comments panel — these shortcuts belong to the file tree only, both registering global handlers caused double-navigation - Thread header null guard: check thread.line before building range label to prevent "L12–null" for outdated GitHub threads For provenance purposes, this commit was AI assisted. --- apps/hook/server/index.ts | 2 ++ .../components/PRCommentsTab.tsx | 33 ++----------------- 2 files changed, 4 insertions(+), 31 deletions(-) diff --git a/apps/hook/server/index.ts b/apps/hook/server/index.ts index 84d03088..e522736b 100644 --- a/apps/hook/server/index.ts +++ b/apps/hook/server/index.ts @@ -362,6 +362,8 @@ if (args[0] === "sessions") { } catch (err) { console.error(`Warning: --local failed, falling back to remote diff`); console.error(err instanceof Error ? err.message : String(err)); + // Clean up partially-created clone directory (clone may have succeeded before fetch/checkout failed) + try { rmSync(localPath, { recursive: true, force: true }); } catch {} agentCwd = undefined; worktreeCleanup = undefined; } diff --git a/packages/review-editor/components/PRCommentsTab.tsx b/packages/review-editor/components/PRCommentsTab.tsx index 89d23969..5eaf9e4b 100644 --- a/packages/review-editor/components/PRCommentsTab.tsx +++ b/packages/review-editor/components/PRCommentsTab.tsx @@ -141,41 +141,12 @@ export const PRCommentsTab: React.FC = React.memo(({ context if ((e.metaKey || e.ctrlKey) && e.shiftKey && e.key.toLowerCase() === 'f') { e.preventDefault(); searchInputRef.current?.focus(); - return; - } - - if (isTypingTarget(e.target)) return; - - // j / ArrowDown → next - if (e.key === 'j' || e.key === 'ArrowDown') { - e.preventDefault(); - if (displayTimeline.length === 0) return; - const idx = selectedId ? displayTimeline.findIndex((e) => e.data.id === selectedId) : -1; - const next = Math.min(idx + 1, displayTimeline.length - 1); - setSelectedId(displayTimeline[next].data.id); - return; - } - - // k / ArrowUp → previous - if (e.key === 'k' || e.key === 'ArrowUp') { - e.preventDefault(); - if (displayTimeline.length === 0) return; - const idx = selectedId ? displayTimeline.findIndex((e) => e.data.id === selectedId) : displayTimeline.length; - const prev = Math.max(idx - 1, 0); - setSelectedId(displayTimeline[prev].data.id); - return; - } - - // Escape → deselect - if (e.key === 'Escape' && selectedId) { - setSelectedId(null); - return; } }; window.addEventListener('keydown', handler); return () => window.removeEventListener('keydown', handler); - }, [displayTimeline, selectedId]); + }, []); // --- Collapse helpers --- const toggleCollapsed = useCallback((id: string) => { @@ -420,7 +391,7 @@ function ThreadCard({ thread, isSelected, isCollapsed, onSelect, onToggleCollaps if (!first) return null; const replies = thread.comments.slice(1); const isDimmed = thread.isResolved || thread.isOutdated; - const lineLabel = thread.startLine && thread.startLine !== thread.line + const lineLabel = thread.startLine && thread.line && thread.startLine !== thread.line ? `L${thread.startLine}–${thread.line}` : thread.line ? `L${thread.line}` : ''; From f8b54997f66fe0bac83a486bd01cd048e189836a Mon Sep 17 00:00:00 2001 From: Michael Ramos Date: Mon, 6 Apr 2026 10:38:37 -0700 Subject: [PATCH 32/37] fix: hoist localPath for catch-block scope, validate baseSha format MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The previous rmSync(localPath) in the catch block was dead code — const declarations inside try are not in scope in catch (separate lexical environments per ECMAScript spec). The ReferenceError was silently swallowed by the inner try/catch, so temp directories still leaked on failed fetch/checkout. Fix: hoist `let localPath` before the try block so it's accessible in catch. Guard with `if (localPath)` since the error could occur before assignment. Also: validate baseSha is a hex SHA (40-64 chars) to prevent git flag injection via crafted API responses. Validate baseBranch rejects both '..' and '-' prefixes. For provenance purposes, this commit was AI assisted. --- apps/hook/server/index.ts | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/apps/hook/server/index.ts b/apps/hook/server/index.ts index e522736b..c3a4fe67 100644 --- a/apps/hook/server/index.ts +++ b/apps/hook/server/index.ts @@ -248,6 +248,8 @@ if (args[0] === "sessions") { // --local: create a local checkout with the PR head for full file access if (useLocal && prMetadata) { + // Hoisted so catch block can clean up partially-created directories + let localPath: string | undefined; try { const repoDir = process.cwd(); const identifier = prMetadata.platform === "github" @@ -256,13 +258,14 @@ if (args[0] === "sessions") { const suffix = Math.random().toString(36).slice(2, 8); // Resolve tmpdir to its real path — on macOS, tmpdir() returns /var/folders/... // but processes report /private/var/folders/... which breaks path stripping. - const localPath = path.join(realpathSync(tmpdir()), `plannotator-pr-${identifier}-${suffix}`); + localPath = path.join(realpathSync(tmpdir()), `plannotator-pr-${identifier}-${suffix}`); const fetchRefStr = prMetadata.platform === "github" ? `refs/pull/${prMetadata.number}/head` : `refs/merge-requests/${prMetadata.iid}/head`; - // Validate baseBranch to prevent path traversal in git ref operations - if (prMetadata.baseBranch.includes('..')) throw new Error(`Invalid base branch: ${prMetadata.baseBranch}`); + // Validate inputs from platform API to prevent git flag/path injection + if (prMetadata.baseBranch.includes('..') || prMetadata.baseBranch.startsWith('-')) throw new Error(`Invalid base branch: ${prMetadata.baseBranch}`); + if (!/^[0-9a-f]{40,64}$/i.test(prMetadata.baseSha)) throw new Error(`Invalid base SHA: ${prMetadata.baseSha}`); // Detect same-repo vs cross-repo (must match both owner/repo AND host) let isSameRepo = false; @@ -362,8 +365,8 @@ if (args[0] === "sessions") { } catch (err) { console.error(`Warning: --local failed, falling back to remote diff`); console.error(err instanceof Error ? err.message : String(err)); - // Clean up partially-created clone directory (clone may have succeeded before fetch/checkout failed) - try { rmSync(localPath, { recursive: true, force: true }); } catch {} + // Clean up partially-created directory (clone may have succeeded before fetch/checkout failed) + if (localPath) try { rmSync(localPath, { recursive: true, force: true }); } catch {} agentCwd = undefined; worktreeCleanup = undefined; } From 70034657e8d2a2aea49fd52c43c467334c5c57dd Mon Sep 17 00:00:00 2001 From: Michael Ramos Date: Mon, 6 Apr 2026 10:47:11 -0700 Subject: [PATCH 33/37] docs: rewrite AI Code Review guide for clarity and readability Restructured for a technical audience: shorter paragraphs, cleaner tables, removed em-dashes and filler prose, tightened the pipeline diagram, streamlined security section into scannable single-line items. For provenance purposes, this commit was AI assisted. --- .../src/content/docs/guides/ai-code-review.md | 223 ++++++++---------- 1 file changed, 96 insertions(+), 127 deletions(-) diff --git a/apps/marketing/src/content/docs/guides/ai-code-review.md b/apps/marketing/src/content/docs/guides/ai-code-review.md index ace6931d..72184886 100644 --- a/apps/marketing/src/content/docs/guides/ai-code-review.md +++ b/apps/marketing/src/content/docs/guides/ai-code-review.md @@ -6,230 +6,199 @@ sidebar: section: "Guides" --- -Plannotator can launch AI review agents that analyze your code changes and produce structured findings directly in the diff viewer. Agents run as background processes with live log streaming — you continue reviewing while they work. +Launch AI review agents from the Plannotator diff viewer. Agents analyze your changes in the background and produce structured findings inline. -## Overview +Two providers are supported: -When reviewing code (local changes or a PR), click **Run Agent** in the Agents tab to launch a review. Two providers are supported: +- **Codex CLI** uses priority-based findings (P0 through P3) +- **Claude Code** uses a multi-agent pipeline with severity-based findings (Important, Nit, Pre-existing) -- **Codex CLI** — uses OpenAI's Codex with structured output schema and priority-based findings (P0–P3) -- **Claude Code** — uses Anthropic's Claude with a multi-agent pipeline and severity-based findings (Important / Nit / Pre-existing) +Both integrations are derived from official tooling. Claude's review model is based on Anthropic's [Claude Code Review](https://code.claude.com/docs/en/code-review) service and the open-source [code-review plugin](https://github.com/anthropics/claude-code/blob/main/plugins/code-review/README.md). Codex uses [OpenAI Codex CLI](https://github.com/openai/codex) structured output. -Each provider has its own review model. Findings appear as annotations in the diff viewer, sorted by severity, with inline reasoning explaining how each issue was verified. +## Flow -Our Claude integration is derived from Anthropic's official [Claude Code Review](https://code.claude.com/docs/en/code-review) service and the open-source [code-review plugin](https://github.com/anthropics/claude-code/blob/main/plugins/code-review/README.md). Our Codex integration is based on [OpenAI Codex CLI](https://github.com/openai/codex) and its structured output capabilities. We adapted both for Plannotator's interactive review UI — the underlying review methodology comes from each provider's official tooling. +1. Click **Run Agent** in the Agents tab (choose Codex or Claude) +2. The server builds the command with the appropriate prompt and schema +3. Agent runs in the background; live logs stream to the Logs tab +4. On completion, findings are parsed and appear as inline annotations -## How it works - -``` -User clicks Run Agent (Codex or Claude) - ↓ -Server builds the review command with appropriate prompt + schema - ↓ -Agent process spawns in the background - ↓ -Live logs stream to the Logs tab in real time - ↓ -On completion, findings are parsed and injected as annotations - ↓ -Annotations appear inline in the diff viewer + in the Findings tab -``` - -For PR reviews, a local worktree is created by default so the agent has full file access. Use `--no-local` to opt out. +For PR reviews, a temporary local checkout is created by default so the agent has file access beyond the diff. Pass `--no-local` to skip this. ## Findings -Each finding includes: - -- **File and line range** — pinpointed to the specific code -- **Description** — what's wrong and why it matters -- **Severity** (Claude) or **Priority** (Codex) — how urgent the fix is -- **Reasoning** (Claude) — the validation chain explaining how the issue was confirmed +Each finding includes a file path, line range, description, and severity or priority. Claude findings also include a reasoning trace that explains how the issue was verified. -Findings are clickable — clicking navigates to the file and highlights the annotation. Copy buttons on each finding and a "Copy All" action export findings as structured markdown. +Click any finding to navigate to the relevant file and line. Use the copy button on individual findings or "Copy All" to export as markdown. -### Claude severity levels +### Severity (Claude) -| Severity | Meaning | -|----------|---------| -| **Important** | A bug to fix before merging — build failures, logic errors, security vulnerabilities | -| **Nit** | Minor issue worth fixing but non-blocking — style deviations, edge cases, code quality | -| **Pre-existing** | A bug in surrounding code not introduced by this PR | +| Level | Meaning | +|-------|---------| +| **Important** | Fix before merging. Build failures, logic errors, security issues. | +| **Nit** | Worth fixing, not blocking. Style, edge cases, code quality. | +| **Pre-existing** | Bug in surrounding code, not introduced by this PR. | -### Codex priority levels +### Priority (Codex) -| Priority | Meaning | -|----------|---------| -| **P0** | Drop everything to fix — blocking | -| **P1** | Urgent — address in the next cycle | -| **P2** | Normal — to be fixed eventually | -| **P3** | Low — nice to have | +| Level | Meaning | +|-------|---------| +| **P0** | Blocking. Drop everything. | +| **P1** | Urgent. Next cycle. | +| **P2** | Normal. Fix eventually. | +| **P3** | Low. Nice to have. | ## Local worktree -For PR/MR reviews, Plannotator automatically creates a temporary local checkout so agents can read files, follow imports, and understand the codebase — not just the diff. +PR and MR reviews automatically create a temporary checkout so agents can read files, follow imports, and understand the codebase. -- **Same-repo PRs**: fast git worktree (shared objects, no network transfer for files) -- **Cross-repo PRs**: shallow clone with targeted fetch of the PR head +- **Same-repo**: git worktree (shared objects, fast) +- **Cross-repo**: shallow clone with targeted PR head fetch -The worktree is cleaned up automatically when the review session ends. Use `--no-local` to skip worktree creation and review in remote-only mode. +Cleaned up when the session ends. Use `--no-local` to review in remote-only mode. ## Transparency -Plannotator is fully transparent about what it sends to AI providers. Every prompt, schema, and command is visible in the review UI and documented here. +Every prompt, schema, and command is visible in the review UI under the "Prompt" and "Review Prompt" disclosures. Full details below. -### Claude Code review prompt +### Claude review pipeline -Claude uses a multi-agent pipeline with 4 parallel review agents and a validation step: +Claude spawns 4 parallel review agents, validates each candidate finding, then returns structured JSON. ``` -Step 1: Gather context - - Retrieve the PR diff - - Read CLAUDE.md and REVIEW.md at the repo root and in modified directories - - Build a map of which rules apply to which file paths - -Step 2: Launch 4 parallel review agents - Agent 1 — Bug + Regression (Opus-level reasoning) - Agent 2 — Security + Deep Analysis (Opus-level reasoning) - Agent 3 — Code Quality + Reusability (Sonnet-level reasoning) - Agent 4 — Guideline Compliance (Haiku-level reasoning) - -Step 3: Validate each candidate finding - For each candidate, a validation agent traces the actual code path, - checks if the issue is handled elsewhere, and confirms with high - confidence. Failed validations are silently dropped. - -Step 4: Classify as important / nit / pre_existing - -Step 5: Deduplicate and rank by severity - -Step 6: Return structured JSON findings +1. Gather context + Read the diff, CLAUDE.md, and REVIEW.md files + +2. Parallel review agents + Agent 1 Bug + Regression (Opus-level) + Agent 2 Security + Deep Analysis (Opus-level) + Agent 3 Code Quality (Sonnet-level) + Agent 4 Guideline Compliance (Haiku-level) + +3. Validate findings + Each candidate is traced through the code to confirm it is real. + Failed validations are dropped silently. + +4. Classify → important / nit / pre_existing +5. Deduplicate and rank by severity +6. Return structured JSON ``` -**Hard constraints enforced in the prompt:** +Hard constraints in the prompt: + - Never approve or block the PR -- Never comment on formatting or code style unless guidance files say to -- Never flag missing test coverage unless guidance files say to -- Never invent rules — only enforce what CLAUDE.md or REVIEW.md state +- Never flag formatting, style, or missing tests unless guidance files say to +- Never invent rules. Only enforce what CLAUDE.md or REVIEW.md state. - Prefer silence over false positives -### Claude Code command +### Claude command ```bash claude -p \ --permission-mode dontAsk \ --output-format stream-json \ - --verbose \ --json-schema \ --no-session-persistence \ --model sonnet \ - --tools Agent,Bash,Read,Glob,Grep \ --allowedTools \ --disallowedTools ``` -The prompt is written to stdin. The `--json-schema` flag enforces structured output with findings, severity, and reasoning fields. +Prompt is written to stdin. The schema enforces findings with severity, description, and reasoning fields. ### Claude allowed tools -The agent can use these tools during review: - | Category | Tools | |----------|-------| -| **Built-in** | Agent (subagents), Read, Glob, Grep | -| **GitHub CLI** | `gh pr view/diff/list`, `gh issue view/list`, `gh api repos/*/pulls/*` | -| **GitLab CLI** | `glab mr view/diff/list`, `glab api` | -| **Git (read-only)** | `git status/diff/log/show/blame/branch/grep/ls-remote/ls-tree/merge-base/remote/rev-parse/show-ref` | -| **Utility** | `wc` (word/line count) | +| Built-in | Agent, Read, Glob, Grep | +| GitHub | `gh pr view/diff/list`, `gh issue view/list`, `gh api` | +| GitLab | `glab mr view/diff/list`, `glab api` | +| Git | `status`, `diff`, `log`, `show`, `blame`, `branch`, `grep`, `merge-base`, and other read-only commands | +| Utility | `wc` | -**Explicitly blocked:** Edit, Write, WebFetch, WebSearch, Python, Node, Bash shells (sh/bash/zsh), curl, wget. The agent is read-only — it cannot modify your code. +Blocked: Edit, Write, WebFetch, WebSearch, Python, Node, shell interpreters, curl, wget. The agent is read-only. ### Codex review prompt -Codex uses OpenAI's structured output with a review-specific system prompt adapted from the [Codex CLI review guidelines](https://github.com/openai/codex): +Adapted from the [Codex CLI review guidelines](https://github.com/openai/codex): -- Focus on bugs the original author would fix if they knew about them -- One finding per distinct issue with minimal line ranges -- Priority tagging (P0–P3) based on severity -- Overall correctness verdict at the end +- Flag bugs the author would fix if they knew +- One finding per issue, minimal line ranges +- Priority tagging P0 through P3 +- Overall correctness verdict ### Codex command ```bash codex exec \ - --output-schema \ + --output-schema \ -o \ - --full-auto \ - --ephemeral \ + --full-auto --ephemeral \ -C \ "" ``` -Codex writes structured JSON to the output file. The schema enforces findings with title, body, confidence score, priority, and code location. +Results are written as structured JSON to the output file. ### Output schemas -**Claude schema:** +Claude: + ```json { "findings": [{ - "severity": "important | nit | pre_existing", - "file": "path/to/file.ts", + "severity": "important", + "file": "src/auth.ts", "line": 42, "end_line": 45, - "description": "What's wrong", + "description": "What is wrong", "reasoning": "How it was verified" }], - "summary": { - "important": 2, - "nit": 1, - "pre_existing": 0 - } + "summary": { "important": 2, "nit": 1, "pre_existing": 0 } } ``` -**Codex schema:** +Codex: + ```json { "findings": [{ "title": "[P1] Issue summary", - "body": "Detailed explanation", + "body": "Explanation", "confidence_score": 0.95, "priority": 1, "code_location": { - "absolute_file_path": "/path/to/file.ts", + "absolute_file_path": "/repo/src/auth.ts", "line_range": { "start": 42, "end": 45 } } }], "overall_correctness": "Incorrect", - "overall_explanation": "Summary of verdict", + "overall_explanation": "Summary", "overall_confidence_score": 0.85 } ``` -## Security notes +## Security -- **Read-only agents**: Review agents cannot modify your code. Edit, Write, and shell execution tools are explicitly blocked. -- **No external network access**: WebFetch, WebSearch, curl, and wget are blocked. Agents can only read local files and use `gh`/`glab` CLI for platform API access. -- **Local execution**: Agents run on your machine as background processes. No code is sent to Plannotator servers — all AI communication goes directly to the provider (Anthropic or OpenAI). -- **Temporary worktrees**: Local checkouts are created in system temp directories and cleaned up on session end. They use shallow clones or detached worktrees to minimize disk usage. -- **CLAUDE.md / REVIEW.md**: The Claude agent reads these files to understand project conventions. These files are part of your repository and under your control. -- **No GitHub commenting**: Agents never post comments, approve, or block PRs. All findings stay in the Plannotator UI unless you explicitly use "Post Comments" to submit them. +**Read-only.** Agents cannot modify code. Edit, Write, and shell execution are blocked. -## Customization +**No network.** WebFetch, WebSearch, curl, and wget are blocked. Agents access the platform API only through `gh` or `glab`. -### CLAUDE.md +**Local execution.** Agents run on your machine. No code goes to Plannotator servers. AI communication goes directly to the provider. -Add a `CLAUDE.md` file to your repository root or any subdirectory. The Claude review agent reads it to understand project-specific rules: +**Temp worktrees.** Checkouts use system temp directories and are cleaned up on session end. + +**No commenting.** Agents never post to GitHub or GitLab. Findings stay in the UI unless you explicitly submit them. + +## Customization + +Add `CLAUDE.md` or `REVIEW.md` to your repo root or any subdirectory. The Claude agent reads them to understand project rules. ```markdown -# Code Review Rules +# Review Rules -- Always check for SQL injection in database queries -- Skip test fixtures in test-fixtures/ -- Enforce snake_case for Python files +- Check for SQL injection in database queries +- Skip files in test-fixtures/ +- Enforce snake_case in Python ``` -### REVIEW.md - -Similar to CLAUDE.md but specifically for review rules. Both files are additive — REVIEW.md adds to CLAUDE.md, it doesn't replace it. +Both files are additive. REVIEW.md extends CLAUDE.md for review-specific guidance. From 8ce82530c0fec863edb8e587746c8c156b115d77 Mon Sep 17 00:00:00 2001 From: Michael Ramos Date: Mon, 6 Apr 2026 10:51:36 -0700 Subject: [PATCH 34/37] docs: rewrite transparency section with exact prompts and commands Replaced summarized/paraphrased transparency section with the actual prompts, commands, schemas, and tool allowlists as they exist in the code. One short security note at the top, then raw content. For provenance purposes, this commit was AI assisted. --- .../src/content/docs/guides/ai-code-review.md | 305 ++++++++++++------ 1 file changed, 205 insertions(+), 100 deletions(-) diff --git a/apps/marketing/src/content/docs/guides/ai-code-review.md b/apps/marketing/src/content/docs/guides/ai-code-review.md index 72184886..99144b11 100644 --- a/apps/marketing/src/content/docs/guides/ai-code-review.md +++ b/apps/marketing/src/content/docs/guides/ai-code-review.md @@ -58,137 +58,242 @@ Cleaned up when the session ends. Use `--no-local` to review in remote-only mode ## Transparency -Every prompt, schema, and command is visible in the review UI under the "Prompt" and "Review Prompt" disclosures. Full details below. +Agents are read-only. They cannot modify code, access the network, or post comments. All AI communication goes directly to your provider (Anthropic or OpenAI). No code passes through Plannotator servers. Prompts and commands are visible in the review UI. -### Claude review pipeline +Below are the exact prompts, commands, and schemas used. -Claude spawns 4 parallel review agents, validates each candidate finding, then returns structured JSON. - -``` -1. Gather context - Read the diff, CLAUDE.md, and REVIEW.md files - -2. Parallel review agents - Agent 1 Bug + Regression (Opus-level) - Agent 2 Security + Deep Analysis (Opus-level) - Agent 3 Code Quality (Sonnet-level) - Agent 4 Guideline Compliance (Haiku-level) - -3. Validate findings - Each candidate is traced through the code to confirm it is real. - Failed validations are dropped silently. - -4. Classify → important / nit / pre_existing -5. Deduplicate and rank by severity -6. Return structured JSON -``` +--- -Hard constraints in the prompt: +### Claude Code: full prompt +``` +# Claude Code Review System Prompt + +## Identity +You are a code review system. Your job is to find bugs that would break +production. You are not a linter, formatter, or style checker unless +project guidance files explicitly expand your scope. + +## Pipeline + +Step 1: Gather context + - Retrieve the PR diff (gh pr diff or git diff) + - Read CLAUDE.md and REVIEW.md at the repo root and in every directory + containing modified files + - Build a map of which rules apply to which file paths + - Identify any skip rules (paths, patterns, or file types to ignore) + +Step 2: Launch 4 parallel review agents + + Agent 1 — Bug + Regression (Opus-level reasoning) + Scan for logic errors, regressions, broken edge cases, build failures, + and code that will produce wrong results. Focus on the diff but read + surrounding code to understand call sites and data flow. Flag only + issues where the code is demonstrably wrong — not stylistic concerns, + not missing tests, not "could be cleaner." + + Agent 2 — Security + Deep Analysis (Opus-level reasoning) + Look for security vulnerabilities with concrete exploit paths, race + conditions, incorrect assumptions about trust boundaries, and subtle + issues in introduced code. Read surrounding code for context. Do not + flag theoretical risks without a plausible path to harm. + + Agent 3 — Code Quality + Reusability (Sonnet-level reasoning) + Look for code smells, unnecessary duplication, missed opportunities to + reuse existing utilities or patterns in the codebase, overly complex + implementations that could be simpler, and elegance issues. Read the + surrounding codebase to understand existing patterns before flagging. + Only flag issues a senior engineer would care about. + + Agent 4 — Guideline Compliance (Haiku-level reasoning) + Audit changes against rules from CLAUDE.md and REVIEW.md gathered in + Step 1. Only flag clear, unambiguous violations where you can cite the + exact rule broken. If a PR makes a CLAUDE.md statement outdated, flag + that the docs need updating. Respect all skip rules — never flag files + or patterns that guidance says to ignore. + + All agents: + - Do not duplicate each other's findings + - Do not flag issues in paths excluded by guidance files + - Provide file, line number, and a concise description for each candidate + +Step 3: Validate each candidate finding + For each candidate, launch a validation agent. The validator: + - Traces the actual code path to confirm the issue is real + - Checks whether the issue is handled elsewhere (try/catch, upstream + guard, fallback logic, type system guarantees) + - Confirms the finding is not a false positive with high confidence + - If validation fails, drop the finding silently + - If validation passes, write a clear reasoning chain explaining how + the issue was confirmed — this becomes the reasoning field + +Step 4: Classify each validated finding + Assign exactly one severity: + + important — A bug that should be fixed before merging. Build failures, + clear logic errors, security vulnerabilities with exploit paths, data + loss risks, race conditions with observable consequences. + + nit — A minor issue worth fixing but non-blocking. Style deviations + from project guidelines, code quality concerns, edge cases that are + unlikely but worth noting, convention violations that don't affect + correctness. + + pre_existing — A bug that exists in the surrounding codebase but was + NOT introduced by this PR. Only flag when directly relevant to the + changed code path. + +Step 5: Deduplicate and rank + - Merge findings that describe the same underlying issue from different + agents — keep the most specific description and the highest severity + - Sort by severity: important → nit → pre_existing + - Within each severity, sort by file path and line number + +Step 6: Return structured JSON output matching the schema. + If no issues are found, return an empty findings array with zeroed summary. + +## Hard constraints - Never approve or block the PR -- Never flag formatting, style, or missing tests unless guidance files say to -- Never invent rules. Only enforce what CLAUDE.md or REVIEW.md state. -- Prefer silence over false positives +- Never comment on formatting or code style unless guidance files say to +- Never flag missing test coverage unless guidance files say to +- Never invent rules — only enforce what CLAUDE.md or REVIEW.md state +- Never flag issues in skipped paths or generated files unless guidance + explicitly includes them +- Prefer silence over false positives — when in doubt, drop the finding +- Do NOT post any comments to GitHub or GitLab +- Do NOT use gh pr comment or any commenting tool +- Your only output is the structured JSON findings +``` -### Claude command +### Claude Code: command ```bash claude -p \ --permission-mode dontAsk \ --output-format stream-json \ - --json-schema \ + --verbose \ + --json-schema '{"type":"object","properties":{"findings":{"type":"array","items":{"type":"object","properties":{"severity":{"type":"string","enum":["important","nit","pre_existing"]},"file":{"type":"string"},"line":{"type":"integer"},"end_line":{"type":"integer"},"description":{"type":"string"},"reasoning":{"type":"string"}},"required":["severity","file","line","end_line","description","reasoning"],"additionalProperties":false}},"summary":{"type":"object","properties":{"important":{"type":"integer"},"nit":{"type":"integer"},"pre_existing":{"type":"integer"}},"required":["important","nit","pre_existing"],"additionalProperties":false}},"required":["findings","summary"],"additionalProperties":false}' \ --no-session-persistence \ --model sonnet \ - --allowedTools \ - --disallowedTools + --tools Agent,Bash,Read,Glob,Grep \ + --allowedTools Agent,Read,Glob,Grep,Bash(gh pr view:*),Bash(gh pr diff:*),Bash(gh pr list:*),Bash(gh issue view:*),Bash(gh issue list:*),Bash(gh api repos/*/*/pulls/*),Bash(gh api repos/*/*/pulls/*/files*),Bash(gh api repos/*/*/pulls/*/comments*),Bash(gh api repos/*/*/issues/*/comments*),Bash(glab mr view:*),Bash(glab mr diff:*),Bash(glab mr list:*),Bash(glab api:*),Bash(git status:*),Bash(git diff:*),Bash(git log:*),Bash(git show:*),Bash(git blame:*),Bash(git branch:*),Bash(git grep:*),Bash(git ls-remote:*),Bash(git ls-tree:*),Bash(git merge-base:*),Bash(git remote:*),Bash(git rev-parse:*),Bash(git show-ref:*),Bash(wc:*) \ + --disallowedTools Edit,Write,NotebookEdit,WebFetch,WebSearch,Bash(python:*),Bash(python3:*),Bash(node:*),Bash(npx:*),Bash(bun:*),Bash(bunx:*),Bash(sh:*),Bash(bash:*),Bash(zsh:*),Bash(curl:*),Bash(wget:*) ``` -Prompt is written to stdin. The schema enforces findings with severity, description, and reasoning fields. - -### Claude allowed tools +Prompt is written to stdin. -| Category | Tools | -|----------|-------| -| Built-in | Agent, Read, Glob, Grep | -| GitHub | `gh pr view/diff/list`, `gh issue view/list`, `gh api` | -| GitLab | `glab mr view/diff/list`, `glab api` | -| Git | `status`, `diff`, `log`, `show`, `blame`, `branch`, `grep`, `merge-base`, and other read-only commands | -| Utility | `wc` | - -Blocked: Edit, Write, WebFetch, WebSearch, Python, Node, shell interpreters, curl, wget. The agent is read-only. - -### Codex review prompt +--- -Adapted from the [Codex CLI review guidelines](https://github.com/openai/codex): +### Codex: full prompt -- Flag bugs the author would fix if they knew -- One finding per issue, minimal line ranges -- Priority tagging P0 through P3 -- Overall correctness verdict +``` +# Review guidelines: + +You are acting as a reviewer for a proposed code change made by another engineer. + +Below are some default guidelines for determining whether the original author +would appreciate the issue being flagged. + +These are not the final word in determining whether an issue is a bug. In many +cases, you will encounter other, more specific guidelines. These may be present +elsewhere in a developer message, a user message, a file, or even elsewhere in +this system message. Those guidelines should be considered to override these +general instructions. + +Here are the general guidelines for determining whether something is a bug and +should be flagged. + +1. It meaningfully impacts the accuracy, performance, security, or + maintainability of the code. +2. The bug is discrete and actionable (i.e. not a general issue with the + codebase or a combination of multiple issues). +3. Fixing the bug does not demand a level of rigor that is not present in the + rest of the codebase. +4. The bug was introduced in the commit (pre-existing bugs should not be + flagged). +5. The author of the original PR would likely fix the issue if they were made + aware of it. +6. The bug does not rely on unstated assumptions about the codebase or + author's intent. +7. It is not enough to speculate that a change may disrupt another part of the + codebase; to be considered a bug, one must identify the other parts of the + code that are provably affected. +8. The bug is clearly not just an intentional change by the original author. + +Comment guidelines: + +1. Clear about why the issue is a bug. +2. Appropriately communicates severity. Does not overclaim. +3. Brief. Body is at most 1 paragraph. +4. No code chunks longer than 3 lines. +5. Clearly communicates the scenarios or inputs necessary for the bug to arise. +6. Tone is matter-of-fact, not accusatory or overly positive. +7. Written so the original author can immediately grasp the idea. +8. Avoids flattery ("Great job ...", "Thanks for ..."). + +Output all findings that the original author would fix if they knew about it. If +there is no finding that a person would definitely love to see and fix, prefer +outputting no findings. + +Priority tags: [P0] Blocking. [P1] Urgent. [P2] Normal. [P3] Low. + +At the end, output an overall correctness verdict. +``` -### Codex command +### Codex: command ```bash codex exec \ - --output-schema \ - -o \ - --full-auto --ephemeral \ + --output-schema ~/.plannotator/codex-review-schema.json \ + -o /tmp/plannotator-codex-.json \ + --full-auto \ + --ephemeral \ -C \ - "" -``` - -Results are written as structured JSON to the output file. - -### Output schemas - -Claude: - -```json -{ - "findings": [{ - "severity": "important", - "file": "src/auth.ts", - "line": 42, - "end_line": 45, - "description": "What is wrong", - "reasoning": "How it was verified" - }], - "summary": { "important": 2, "nit": 1, "pre_existing": 0 } -} + "\n\n---\n\n" ``` -Codex: +### Codex: output schema ```json { - "findings": [{ - "title": "[P1] Issue summary", - "body": "Explanation", - "confidence_score": 0.95, - "priority": 1, - "code_location": { - "absolute_file_path": "/repo/src/auth.ts", - "line_range": { "start": 42, "end": 45 } - } - }], - "overall_correctness": "Incorrect", - "overall_explanation": "Summary", - "overall_confidence_score": 0.85 + "type": "object", + "properties": { + "findings": { + "type": "array", + "items": { + "type": "object", + "properties": { + "title": { "type": "string" }, + "body": { "type": "string" }, + "confidence_score": { "type": "number" }, + "priority": { "type": ["integer", "null"] }, + "code_location": { + "type": "object", + "properties": { + "absolute_file_path": { "type": "string" }, + "line_range": { + "type": "object", + "properties": { + "start": { "type": "integer" }, + "end": { "type": "integer" } + }, + "required": ["start", "end"] + } + }, + "required": ["absolute_file_path", "line_range"] + } + }, + "required": ["title", "body", "confidence_score", "priority", "code_location"] + } + }, + "overall_correctness": { "type": "string" }, + "overall_explanation": { "type": "string" }, + "overall_confidence_score": { "type": "number" } + }, + "required": ["findings", "overall_correctness", "overall_explanation", "overall_confidence_score"] } ``` -## Security - -**Read-only.** Agents cannot modify code. Edit, Write, and shell execution are blocked. - -**No network.** WebFetch, WebSearch, curl, and wget are blocked. Agents access the platform API only through `gh` or `glab`. - -**Local execution.** Agents run on your machine. No code goes to Plannotator servers. AI communication goes directly to the provider. - -**Temp worktrees.** Checkouts use system temp directories and are cleaned up on session end. - -**No commenting.** Agents never post to GitHub or GitLab. Findings stay in the UI unless you explicitly submit them. - ## Customization Add `CLAUDE.md` or `REVIEW.md` to your repo root or any subdirectory. The Claude agent reads them to understand project rules. From fa53662e9db78a544f52662ae501cd27b37be773 Mon Sep 17 00:00:00 2001 From: Michael Ramos Date: Mon, 6 Apr 2026 10:53:02 -0700 Subject: [PATCH 35/37] docs: add mini TOC to transparency section For provenance purposes, this commit was AI assisted. --- apps/marketing/src/content/docs/guides/ai-code-review.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/apps/marketing/src/content/docs/guides/ai-code-review.md b/apps/marketing/src/content/docs/guides/ai-code-review.md index 99144b11..5d79ad2f 100644 --- a/apps/marketing/src/content/docs/guides/ai-code-review.md +++ b/apps/marketing/src/content/docs/guides/ai-code-review.md @@ -62,6 +62,12 @@ Agents are read-only. They cannot modify code, access the network, or post comme Below are the exact prompts, commands, and schemas used. +- [Claude Code: full prompt](#claude-code-full-prompt) +- [Claude Code: command](#claude-code-command) +- [Codex: full prompt](#codex-full-prompt) +- [Codex: command](#codex-command) +- [Codex: output schema](#codex-output-schema) + --- ### Claude Code: full prompt From 6f444397dde3e222e74574dfe99a6b638437878b Mon Sep 17 00:00:00 2001 From: Michael Ramos Date: Mon, 6 Apr 2026 12:07:44 -0700 Subject: [PATCH 36/37] fix: stdout drain hang, Codex verdict override, Claude parse logging, memo removal MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Critical: - Race stdoutDone against 2s timeout after proc.exited — prevents permanent job hang when Bun's ReadableStream doesn't close after process exit. The process is dead; 2s is a cleanup deadline. Bug fix: - Codex verdict: override to "Issues Found" when P0/P1 findings exist, regardless of the freeform overall_correctness string. Prevents green "Correct" badge when Codex says "mostly correct but has issues." P2/P3-only findings still trust Codex's verdict. Observability: - Log Claude parse failures with buffer size and last 200 bytes so we can diagnose empty-findings cases. Performance: - Remove React.memo from ReviewSidebar — was blocking legitimate re-renders (job status, findings) to prevent cosmetic log flickering. The tradeoff was wrong. Docs: - Remove "shell" from CLAUDE.md capabilities table (provider was removed). For provenance purposes, this commit was AI assisted. --- AGENTS.md | 2 +- packages/review-editor/components/ReviewSidebar.tsx | 4 ++-- packages/server/agent-jobs.ts | 5 +++-- packages/server/review.ts | 10 ++++++++-- 4 files changed, 14 insertions(+), 7 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index b028fdc4..3914db3c 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -234,7 +234,7 @@ During normal plan review, an Archive sidebar tab provides the same browsing via | `/api/external-annotations` | POST | Add external annotations (single or batch `{ annotations: [...] }`) | | `/api/external-annotations` | PATCH | Update fields on a single annotation (`?id=`) | | `/api/external-annotations` | DELETE | Remove by `?id=`, `?source=`, or clear all | -| `/api/agents/capabilities` | GET | Check available agent providers (claude, codex, shell) | +| `/api/agents/capabilities` | GET | Check available agent providers (claude, codex) | | `/api/agents/jobs/stream` | GET | SSE stream for real-time agent job status updates | | `/api/agents/jobs` | GET | Snapshot of agent jobs (polling fallback, `?since=N` for version gating) | | `/api/agents/jobs` | POST | Launch an agent job (body: `{ provider, command, label }`) | diff --git a/packages/review-editor/components/ReviewSidebar.tsx b/packages/review-editor/components/ReviewSidebar.tsx index 3afa916d..d751bed3 100644 --- a/packages/review-editor/components/ReviewSidebar.tsx +++ b/packages/review-editor/components/ReviewSidebar.tsx @@ -107,7 +107,7 @@ function compareCodeAnnotations(a: CodeAnnotation, b: CodeAnnotation): number { } -export const ReviewSidebar: React.FC = React.memo(({ +export const ReviewSidebar: React.FC = /* React.memo */({ isOpen, onToggle, annotations, @@ -461,4 +461,4 @@ export const ReviewSidebar: React.FC = React.memo(({ )} ); -}); +}; diff --git a/packages/server/agent-jobs.ts b/packages/server/agent-jobs.ts index 14964921..177d9997 100644 --- a/packages/server/agent-jobs.ts +++ b/packages/server/agent-jobs.ts @@ -243,8 +243,9 @@ export function createAgentJobHandler(options: AgentJobHandlerOptions): AgentJob // Monitor process exit proc.exited.then(async (exitCode) => { - // Ensure stdout is fully drained before reading results - await stdoutDone; + // Wait for stdout to drain — grace period in case the pipe doesn't close cleanly. + // The process is dead; if the stream hasn't flushed in 2s, the runtime has a bug. + await Promise.race([stdoutDone, new Promise(r => setTimeout(r, 2000))]); const entry = jobs.get(id); if (!entry || isTerminalStatus(entry.info.status)) return; diff --git a/packages/server/review.ts b/packages/server/review.ts index fe072709..2c44aeb9 100644 --- a/packages/server/review.ts +++ b/packages/server/review.ts @@ -169,8 +169,11 @@ export async function startReviewServer( const output = await parseCodexOutput(meta.outputPath); if (!output) return; + // Override verdict if there are blocking findings (P0/P1) — Codex's + // freeform correctness string can say "mostly correct" with real bugs. + const hasBlockingFindings = output.findings.some(f => f.priority !== null && f.priority <= 1); job.summary = { - correctness: output.overall_correctness, + correctness: hasBlockingFindings ? "Issues Found" : output.overall_correctness, explanation: output.overall_explanation, confidence: output.overall_confidence_score, }; @@ -186,7 +189,10 @@ export async function startReviewServer( // --- Claude path --- if (job.provider === "claude" && meta.stdout) { const output = parseClaudeStreamOutput(meta.stdout); - if (!output) return; + if (!output) { + console.error(`[claude-review] Failed to parse output (${meta.stdout.length} bytes, last 200: ${meta.stdout.slice(-200)})`); + return; + } const total = output.summary.important + output.summary.nit + output.summary.pre_existing; job.summary = { From 4a2a919590a85e96d09b3c5b1e4e2b4110ae50a0 Mon Sep 17 00:00:00 2001 From: Michael Ramos Date: Mon, 6 Apr 2026 12:11:58 -0700 Subject: [PATCH 37/37] fix: type assertions for Bun ReadableStream async iteration MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Bun's proc.stdout/stderr support for-await at runtime but TypeScript's ReadableStream type doesn't declare [Symbol.asyncIterator]. Cast through unknown to AsyncIterable — standard Bun workaround, same pattern as the FileSink cast for stdin. For provenance purposes, this commit was AI assisted. --- packages/server/agent-jobs.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/server/agent-jobs.ts b/packages/server/agent-jobs.ts index 177d9997..513fdb53 100644 --- a/packages/server/agent-jobs.ts +++ b/packages/server/agent-jobs.ts @@ -178,7 +178,7 @@ export function createAgentJobHandler(options: AgentJobHandlerOptions): AgentJob if (proc.stderr && typeof proc.stderr !== "number") { (async () => { try { - const reader = proc!.stderr as ReadableStream; + const reader = proc!.stderr as unknown as AsyncIterable; for await (const chunk of reader) { const text = typeof chunk === "string" ? chunk : new TextDecoder().decode(chunk); stderrBuf = (stderrBuf + text).slice(-500); @@ -211,7 +211,7 @@ export function createAgentJobHandler(options: AgentJobHandlerOptions): AgentJob const stdoutDone = (captureStdout && proc.stdout && typeof proc.stdout !== "number") ? (async () => { try { - const reader = proc!.stdout as ReadableStream; + const reader = proc!.stdout as unknown as AsyncIterable; for await (const chunk of reader) { const text = typeof chunk === "string" ? chunk : new TextDecoder().decode(chunk); stdoutBuf += text;