diff --git a/AGENTS.md b/AGENTS.md index 971609f5..465b4c8a 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -223,8 +223,9 @@ During normal plan review, an Archive sidebar tab provides the same browsing via | Endpoint | Method | Purpose | | --------------------- | ------ | ------------------------------------------ | -| `/api/diff` | GET | Returns `{ rawPatch, gitRef, origin, diffType, gitContext }` | -| `/api/file-content` | GET | Returns `{ oldContent, newContent }` for expandable diff context | +| `/api/diff` | GET | Returns `{ rawPatch, gitRef, origin, diffType, base, gitContext }` | +| `/api/diff/switch` | POST | Switch diff type and/or base branch (body: `{ diffType, base? }`) | +| `/api/file-content` | GET | Returns `{ oldContent, newContent }` for expandable diff context (`?path=&oldPath=&base=`) | | `/api/git-add` | POST | Stage/unstage a file (body: `{ filePath, undo? }`) | | `/api/feedback` | POST | Submit review (body: feedback, annotations, agentSwitch) | | `/api/image` | GET | Serve image by path query param | diff --git a/apps/pi-extension/plannotator-browser.ts b/apps/pi-extension/plannotator-browser.ts index 4032959f..039609d6 100644 --- a/apps/pi-extension/plannotator-browser.ts +++ b/apps/pi-extension/plannotator-browser.ts @@ -181,6 +181,7 @@ export async function openCodeReview( let prMetadata: Awaited>["metadata"] | undefined; let diffType: DiffType | undefined; let agentCwd: string | undefined; + let initialBase: string | undefined; let worktreeCleanup: (() => void | Promise) | undefined; let exitHandler: (() => void) | undefined; @@ -342,6 +343,10 @@ export async function openCodeReview( rawPatch = result.patch; gitRef = result.label; diffError = result.error; + // Remember which base the initial diff was computed against so it can + // be forwarded to the server below. Only matters when the caller + // overrode the detected default; otherwise it matches gitCtx already. + initialBase = defaultBranch; } const server = await startReviewServer({ @@ -351,6 +356,7 @@ export async function openCodeReview( origin: "pi", diffType, gitContext: gitCtx, + initialBase, prMetadata, agentCwd, htmlContent: reviewHtmlContent, diff --git a/apps/pi-extension/server.test.ts b/apps/pi-extension/server.test.ts index 007ac539..34eb7949 100644 --- a/apps/pi-extension/server.test.ts +++ b/apps/pi-extension/server.test.ts @@ -305,4 +305,139 @@ describe("pi review server", () => { server.stop(); } }, 15_000); + + test("round-trips the active base branch through /api/diff and /api/diff/switch", async () => { + const homeDir = makeTempDir("plannotator-pi-home-"); + const repoDir = initRepo(); + process.env.HOME = homeDir; + process.chdir(repoDir); + process.env.PLANNOTATOR_PORT = String(await reservePort()); + + // Create a second branch the picker can switch to, then branch off it so + // currentBranch !== defaultBranch and the branch/merge-base options appear. + git(repoDir, ["checkout", "-b", "develop"]); + writeFileSync(join(repoDir, "develop-file.txt"), "develop\n", "utf-8"); + git(repoDir, ["add", "develop-file.txt"]); + git(repoDir, ["commit", "-m", "develop commit"]); + git(repoDir, ["checkout", "-b", "feature/x"]); + writeFileSync(join(repoDir, "feature-file.txt"), "feature\n", "utf-8"); + git(repoDir, ["add", "feature-file.txt"]); + git(repoDir, ["commit", "-m", "feature commit"]); + + const gitContext = await getGitContext(); + const diff = await runGitDiff("uncommitted", gitContext.defaultBranch); + + const server = await startReviewServer({ + rawPatch: diff.patch, + gitRef: diff.label, + error: diff.error, + diffType: "uncommitted", + gitContext, + origin: "pi", + htmlContent: "review", + }); + + try { + // Initial load: server echoes the detected default as the active base. + const initial = await fetch(`${server.url}/api/diff`).then((r) => r.json()) as { + base?: string; + gitContext?: { defaultBranch: string }; + }; + expect(initial.base).toBe(gitContext.defaultBranch); + expect(initial.base).toBe(initial.gitContext?.defaultBranch); + + // Switch to a custom base — response must echo the resolved base. + const switchResponse = await fetch(`${server.url}/api/diff/switch`, { + method: "POST", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify({ diffType: "branch", base: "develop" }), + }); + expect(switchResponse.status).toBe(200); + const switched = await switchResponse.json() as { base?: string; diffType: string }; + expect(switched.base).toBe("develop"); + expect(switched.diffType).toBe("branch"); + + // Subsequent /api/diff load reflects the switched base — this is what + // survives a page refresh / reconnect. + const rehydrate = await fetch(`${server.url}/api/diff`).then((r) => r.json()) as { + base?: string; + }; + expect(rehydrate.base).toBe("develop"); + + // Unknown refs pass through verbatim — the resolver trusts callers so + // unusual-but-valid refs (tags, SHAs, non-origin remotes) work. Truly + // invalid refs surface via the diff error, not via a silent swap. + const unknownResponse = await fetch(`${server.url}/api/diff/switch`, { + method: "POST", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify({ diffType: "branch", base: "nope-does-not-exist" }), + }); + const unknown = await unknownResponse.json() as { base?: string; error?: string }; + expect(unknown.base).toBe("nope-does-not-exist"); + expect(unknown.error).toBeTruthy(); + + // Feedback to clean up the waitForDecision promise. + await fetch(`${server.url}/api/feedback`, { + method: "POST", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify({ approved: false, feedback: "done", annotations: [] }), + }); + await server.waitForDecision(); + } finally { + server.stop(); + } + }, 15_000); + + test("initialBase overrides gitContext.defaultBranch in server state", async () => { + // Simulates a programmatic caller (Pi event bus, other extensions) that + // opens a review against a non-default base. The server's currentBase — + // which drives /api/diff, agent prompts, and file-content fetches — must + // honor that override instead of falling back to the detected default. + const homeDir = makeTempDir("plannotator-pi-home-"); + const repoDir = initRepo(); + process.env.HOME = homeDir; + process.chdir(repoDir); + process.env.PLANNOTATOR_PORT = String(await reservePort()); + + git(repoDir, ["checkout", "-b", "develop"]); + writeFileSync(join(repoDir, "develop-file.txt"), "develop\n", "utf-8"); + git(repoDir, ["add", "develop-file.txt"]); + git(repoDir, ["commit", "-m", "develop commit"]); + git(repoDir, ["checkout", "-b", "feature/x"]); + + const gitContext = await getGitContext(); + // Detected default is "main"; caller explicitly wants "develop". + expect(gitContext.defaultBranch).toBe("main"); + const diff = await runGitDiff("branch", "develop"); + + const server = await startReviewServer({ + rawPatch: diff.patch, + gitRef: diff.label, + error: diff.error, + diffType: "branch", + gitContext, + initialBase: "develop", + origin: "pi", + htmlContent: "review", + }); + + try { + const payload = await fetch(`${server.url}/api/diff`).then((r) => r.json()) as { + base?: string; + gitContext?: { defaultBranch: string }; + }; + // The server must echo the caller's override, not the detected default. + expect(payload.base).toBe("develop"); + expect(payload.gitContext?.defaultBranch).toBe("main"); + + await fetch(`${server.url}/api/feedback`, { + method: "POST", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify({ approved: false, feedback: "done", annotations: [] }), + }); + await server.waitForDecision(); + } finally { + server.stop(); + } + }, 15_000); }); diff --git a/apps/pi-extension/server/agent-jobs.ts b/apps/pi-extension/server/agent-jobs.ts index 4b327cb3..c4ec278b 100644 --- a/apps/pi-extension/server/agent-jobs.ts +++ b/apps/pi-extension/server/agent-jobs.ts @@ -73,6 +73,8 @@ export interface AgentJobHandlerOptions { reasoningEffort?: string; /** Whether Codex fast mode was enabled. */ fastMode?: boolean; + /** Diff context snapshot at launch (stored on AgentJobInfo for per-job "Copy All"). */ + diffContext?: AgentJobInfo["diffContext"]; } | null>; /** Called when a job completes successfully — parse results and push annotations. */ onJobComplete?: (job: AgentJobInfo, meta: { outputPath?: string; stdout?: string; cwd?: string }) => void | Promise; @@ -118,7 +120,7 @@ export function createAgentJobHandler(options: AgentJobHandlerOptions) { command: string[], label: string, outputPath?: string, - spawnOptions?: { captureStdout?: boolean; stdinPrompt?: string; cwd?: string; prompt?: string; engine?: string; model?: string; effort?: string; reasoningEffort?: string; fastMode?: boolean }, + spawnOptions?: { captureStdout?: boolean; stdinPrompt?: string; cwd?: string; prompt?: string; engine?: string; model?: string; effort?: string; reasoningEffort?: string; fastMode?: boolean; diffContext?: AgentJobInfo["diffContext"] }, ): AgentJobInfo { const id = crypto.randomUUID(); const source = jobSource(id); @@ -137,6 +139,7 @@ export function createAgentJobHandler(options: AgentJobHandlerOptions) { ...(spawnOptions?.effort && { effort: spawnOptions.effort }), ...(spawnOptions?.reasoningEffort && { reasoningEffort: spawnOptions.reasoningEffort }), ...(spawnOptions?.fastMode && { fastMode: spawnOptions.fastMode }), + ...(spawnOptions?.diffContext && { diffContext: spawnOptions.diffContext }), }; let proc: ChildProcess | null = null; @@ -419,6 +422,7 @@ export function createAgentJobHandler(options: AgentJobHandlerOptions) { let jobEffort: string | undefined; let jobReasoningEffort: string | undefined; let jobFastMode: boolean | undefined; + let jobDiffContext: AgentJobInfo["diffContext"] | undefined; if (options.buildCommand) { // Thread config from POST body to buildCommand const config: Record = {}; @@ -441,6 +445,7 @@ export function createAgentJobHandler(options: AgentJobHandlerOptions) { jobEffort = built.effort; jobReasoningEffort = built.reasoningEffort; jobFastMode = built.fastMode; + jobDiffContext = built.diffContext; } } @@ -459,6 +464,7 @@ export function createAgentJobHandler(options: AgentJobHandlerOptions) { effort: jobEffort, reasoningEffort: jobReasoningEffort, fastMode: jobFastMode, + diffContext: jobDiffContext, }); json(res, { job }, 201); } catch { diff --git a/apps/pi-extension/server/serverReview.ts b/apps/pi-extension/server/serverReview.ts index f5e5fc33..6ca1e35b 100644 --- a/apps/pi-extension/server/serverReview.ts +++ b/apps/pi-extension/server/serverReview.ts @@ -31,6 +31,7 @@ import { gitAddFile as gitAddFileCore, gitResetFile as gitResetFileCore, parseWorktreeDiffType, + resolveBaseBranch, type ReviewGitRuntime, runGitDiff as runGitDiffCore, validateFilePath, @@ -38,6 +39,7 @@ import { import { createEditorAnnotationHandler } from "./annotations.js"; import { createAgentJobHandler } from "./agent-jobs.js"; +import type { AgentJobInfo } from "../generated/agent-jobs.js"; import { createExternalAnnotationHandler } from "./external-annotations.js"; import { handleDraftRequest, @@ -145,6 +147,14 @@ export async function startReviewServer(options: { origin?: string; diffType?: DiffType; gitContext?: GitContext; + /** + * Initial base branch the caller used to compute `rawPatch`. When a caller + * overrides the detected default (e.g. `openCodeReview({ defaultBranch })`), + * this must be forwarded so the server's internal `currentBase` state, the + * `/api/diff` response, and downstream agent prompts stay consistent with + * the patch that's already on screen. + */ + initialBase?: string; error?: string; sharingEnabled?: boolean; shareBaseUrl?: string; @@ -192,6 +202,11 @@ export async function startReviewServer(options: { let currentGitRef = options.gitRef; let currentDiffType: DiffType = options.diffType || "uncommitted"; let currentError = options.error; + // Tracks the base branch the user picked from the UI. Agent review prompts + // read this (not gitContext.defaultBranch) so they analyze the same diff + // the reviewer is currently looking at. Honors an explicit initialBase from + // the caller — e.g. programmatic Pi callers can request a non-detected base. + let currentBase = options.initialBase || options.gitContext?.defaultBranch || "main"; // Agent jobs — background process manager (late-binds serverUrl via getter) let serverUrl = ""; @@ -213,10 +228,24 @@ export async function startReviewServer(options: { async buildCommand(provider, config) { const cwd = resolveAgentCwd(); const hasAgentLocalAccess = !!options.agentCwd || !!options.gitContext; - const userMessageOptions = { defaultBranch: options.gitContext?.defaultBranch, hasLocalAccess: hasAgentLocalAccess }; + const userMessageOptions = { defaultBranch: currentBase, hasLocalAccess: hasAgentLocalAccess }; + + // Snapshot the diff context at launch (see review.ts buildCommand + // for the rationale — keeps downstream "Copy All" honest across + // subsequent context switches). + const worktreeParts = currentDiffType.startsWith("worktree:") + ? parseWorktreeDiffType(currentDiffType) + : null; + const diffContext: AgentJobInfo["diffContext"] | undefined = options.prMetadata + ? undefined + : { + mode: (worktreeParts?.subType ?? currentDiffType) as string, + base: currentBase, + worktreePath: worktreeParts?.path ?? null, + }; if (provider === "tour") { - return tour.buildCommand({ + const built = await tour.buildCommand({ cwd, patch: currentPatch, diffType: currentDiffType, @@ -224,6 +253,7 @@ export async function startReviewServer(options: { prMetadata: options.prMetadata, config, }); + return built ? { ...built, diffContext } : built; } const userMessage = buildCodexReviewUserMessage(currentPatch, currentDiffType, userMessageOptions, options.prMetadata); @@ -235,7 +265,7 @@ export async function startReviewServer(options: { const outputPath = generateOutputPath(); const prompt = CODEX_REVIEW_SYSTEM_PROMPT + "\n\n---\n\n" + userMessage; const command = await buildCodexCommand({ cwd, outputPath, prompt, model, reasoningEffort, fastMode }); - return { command, outputPath, prompt, label: "Code Review", model, reasoningEffort, fastMode: fastMode || undefined }; + return { command, outputPath, prompt, label: "Code Review", model, reasoningEffort, fastMode: fastMode || undefined, diffContext }; } if (provider === "claude") { @@ -243,7 +273,7 @@ export async function startReviewServer(options: { const effort = typeof config?.effort === "string" && config.effort ? config.effort : undefined; const prompt = CLAUDE_REVIEW_PROMPT + "\n\n---\n\n" + userMessage; const { command, stdinPrompt } = buildClaudeCommand(prompt, model, effort); - return { command, stdinPrompt, prompt, cwd, label: "Code Review", captureStdout: true, model, effort }; + return { command, stdinPrompt, prompt, cwd, label: "Code Review", captureStdout: true, model, effort, diffContext }; } return null; @@ -479,6 +509,9 @@ export async function startReviewServer(options: { gitRef: currentGitRef, origin: options.origin ?? "pi", diffType: hasLocalAccess ? currentDiffType : undefined, + // Echo the active base so page refresh/reconnect rehydrates the + // picker to what the server is actually using, not the detected default. + base: hasLocalAccess ? currentBase : undefined, gitContext: hasLocalAccess ? options.gitContext : undefined, sharingEnabled, shareBaseUrl, @@ -503,17 +536,43 @@ export async function startReviewServer(options: { json(res, { error: "Missing diffType" }, 400); return; } - const defaultBranch = options.gitContext?.defaultBranch || "main"; + const detectedBase = options.gitContext?.defaultBranch || "main"; + const base = resolveBaseBranch( + typeof body.base === "string" ? body.base : undefined, + detectedBase, + ); const defaultCwd = options.gitContext?.cwd; - const result = await runGitDiff(newType, defaultBranch, defaultCwd); + const result = await runGitDiff(newType, base, defaultCwd); currentPatch = result.patch; currentGitRef = result.label; currentDiffType = newType; + currentBase = base; currentError = result.error; + + // Recompute gitContext for the effective cwd so the client's + // sidebar reflects the worktree we're now reviewing. + // Best-effort: on failure the client keeps its existing context. + let updatedContext: GitContext | undefined; + if (options.gitContext) { + try { + const worktreeParsed = parseWorktreeDiffType(newType); + const effectiveCwd = worktreeParsed?.path ?? options.gitContext.cwd; + updatedContext = await getGitContextCore(reviewRuntime, effectiveCwd); + } catch { + /* best-effort */ + } + } + json(res, { rawPatch: currentPatch, gitRef: currentGitRef, diffType: currentDiffType, + // Echo the base the server actually used. resolveBaseBranch + // trusts the caller verbatim; this echo lets the client + // confirm the request landed (and pick it up when the client + // didn't supply one and we fell back to detected default). + base: currentBase, + ...(updatedContext ? { gitContext: updatedContext } : {}), ...(currentError ? { error: currentError } : {}), }); } catch (err) { @@ -613,12 +672,16 @@ export async function startReviewServer(options: { // Local mode first (matches Bun server priority) if (hasLocalAccess && !isPRMode) { - const defaultBranch = options.gitContext?.defaultBranch || "main"; + const detectedBase = options.gitContext?.defaultBranch || "main"; + const base = resolveBaseBranch( + url.searchParams.get("base") ?? undefined, + detectedBase, + ); const defaultCwd = options.gitContext?.cwd; const result = await getFileContentsForDiffCore( reviewRuntime, currentDiffType, - defaultBranch, + base, filePath, oldPath, defaultCwd, diff --git a/bun.lock b/bun.lock index 2ff0a627..0a5e4727 100644 --- a/bun.lock +++ b/bun.lock @@ -64,7 +64,7 @@ }, "apps/opencode-plugin": { "name": "@plannotator/opencode", - "version": "0.18.0", + "version": "0.19.0", "dependencies": { "@opencode-ai/plugin": "^1.1.10", }, @@ -86,7 +86,7 @@ }, "apps/pi-extension": { "name": "@plannotator/pi-extension", - "version": "0.18.0", + "version": "0.19.0", "dependencies": { "@joplin/turndown-plugin-gfm": "^1.0.64", "turndown": "^7.2.4", @@ -176,6 +176,9 @@ "@plannotator/ui": "workspace:*", "@radix-ui/react-checkbox": "^1.3.3", "@radix-ui/react-collapsible": "^1.1.12", + "@radix-ui/react-dialog": "^1.1.15", + "@radix-ui/react-dropdown-menu": "^2.1.15", + "@radix-ui/react-popover": "^1.1.15", "@radix-ui/react-tooltip": "^1.2.8", "highlight.js": "^11.11.1", "motion": "^12.38.0", @@ -186,7 +189,7 @@ }, "packages/server": { "name": "@plannotator/server", - "version": "0.18.0", + "version": "0.19.0", "dependencies": { "@plannotator/ai": "workspace:*", "@plannotator/shared": "workspace:*", @@ -210,6 +213,7 @@ "@plannotator/shared": "workspace:*", "@plannotator/web-highlighter": "^0.8.1", "@radix-ui/react-dialog": "^1.1.15", + "@radix-ui/react-popover": "^1.1.15", "@radix-ui/react-tooltip": "^1.2.8", "@tanstack/react-table": "^8.21.3", "@viz-js/viz": "^3.25.0", @@ -720,20 +724,30 @@ "@radix-ui/react-collapsible": ["@radix-ui/react-collapsible@1.1.12", "", { "dependencies": { "@radix-ui/primitive": "1.1.3", "@radix-ui/react-compose-refs": "1.1.2", "@radix-ui/react-context": "1.1.2", "@radix-ui/react-id": "1.1.1", "@radix-ui/react-presence": "1.1.5", "@radix-ui/react-primitive": "2.1.3", "@radix-ui/react-use-controllable-state": "1.2.2", "@radix-ui/react-use-layout-effect": "1.1.1" }, "peerDependencies": { "@types/react": "*", "@types/react-dom": "*", "react": "^16.8 || ^17.0 || ^18.0 || ^19.0 || ^19.0.0-rc", "react-dom": "^16.8 || ^17.0 || ^18.0 || ^19.0 || ^19.0.0-rc" }, "optionalPeers": ["@types/react", "@types/react-dom"] }, "sha512-Uu+mSh4agx2ib1uIGPP4/CKNULyajb3p92LsVXmH2EHVMTfZWpll88XJ0j4W0z3f8NK1eYl1+Mf/szHPmcHzyA=="], + "@radix-ui/react-collection": ["@radix-ui/react-collection@1.1.7", "", { "dependencies": { "@radix-ui/react-compose-refs": "1.1.2", "@radix-ui/react-context": "1.1.2", "@radix-ui/react-primitive": "2.1.3", "@radix-ui/react-slot": "1.2.3" }, "peerDependencies": { "@types/react": "*", "@types/react-dom": "*", "react": "^16.8 || ^17.0 || ^18.0 || ^19.0 || ^19.0.0-rc", "react-dom": "^16.8 || ^17.0 || ^18.0 || ^19.0 || ^19.0.0-rc" }, "optionalPeers": ["@types/react", "@types/react-dom"] }, "sha512-Fh9rGN0MoI4ZFUNyfFVNU4y9LUz93u9/0K+yLgA2bwRojxM8JU1DyvvMBabnZPBgMWREAJvU2jjVzq+LrFUglw=="], + "@radix-ui/react-compose-refs": ["@radix-ui/react-compose-refs@1.1.2", "", { "peerDependencies": { "@types/react": "*", "react": "^16.8 || ^17.0 || ^18.0 || ^19.0 || ^19.0.0-rc" }, "optionalPeers": ["@types/react"] }, "sha512-z4eqJvfiNnFMHIIvXP3CY57y2WJs5g2v3X0zm9mEJkrkNv4rDxu+sg9Jh8EkXyeqBkB7SOcboo9dMVqhyrACIg=="], "@radix-ui/react-context": ["@radix-ui/react-context@1.1.2", "", { "peerDependencies": { "@types/react": "*", "react": "^16.8 || ^17.0 || ^18.0 || ^19.0 || ^19.0.0-rc" }, "optionalPeers": ["@types/react"] }, "sha512-jCi/QKUM2r1Ju5a3J64TH2A5SpKAgh0LpknyqdQ4m6DCV0xJ2HG1xARRwNGPQfi1SLdLWZ1OJz6F4OMBBNiGJA=="], "@radix-ui/react-dialog": ["@radix-ui/react-dialog@1.1.15", "", { "dependencies": { "@radix-ui/primitive": "1.1.3", "@radix-ui/react-compose-refs": "1.1.2", "@radix-ui/react-context": "1.1.2", "@radix-ui/react-dismissable-layer": "1.1.11", "@radix-ui/react-focus-guards": "1.1.3", "@radix-ui/react-focus-scope": "1.1.7", "@radix-ui/react-id": "1.1.1", "@radix-ui/react-portal": "1.1.9", "@radix-ui/react-presence": "1.1.5", "@radix-ui/react-primitive": "2.1.3", "@radix-ui/react-slot": "1.2.3", "@radix-ui/react-use-controllable-state": "1.2.2", "aria-hidden": "^1.2.4", "react-remove-scroll": "^2.6.3" }, "peerDependencies": { "@types/react": "*", "@types/react-dom": "*", "react": "^16.8 || ^17.0 || ^18.0 || ^19.0 || ^19.0.0-rc", "react-dom": "^16.8 || ^17.0 || ^18.0 || ^19.0 || ^19.0.0-rc" }, "optionalPeers": ["@types/react", "@types/react-dom"] }, "sha512-TCglVRtzlffRNxRMEyR36DGBLJpeusFcgMVD9PZEzAKnUs1lKCgX5u9BmC2Yg+LL9MgZDugFFs1Vl+Jp4t/PGw=="], + "@radix-ui/react-direction": ["@radix-ui/react-direction@1.1.1", "", { "peerDependencies": { "@types/react": "*", "react": "^16.8 || ^17.0 || ^18.0 || ^19.0 || ^19.0.0-rc" }, "optionalPeers": ["@types/react"] }, "sha512-1UEWRX6jnOA2y4H5WczZ44gOOjTEmlqv1uNW4GAJEO5+bauCBhv8snY65Iw5/VOS/ghKN9gr2KjnLKxrsvoMVw=="], + "@radix-ui/react-dismissable-layer": ["@radix-ui/react-dismissable-layer@1.1.11", "", { "dependencies": { "@radix-ui/primitive": "1.1.3", "@radix-ui/react-compose-refs": "1.1.2", "@radix-ui/react-primitive": "2.1.3", "@radix-ui/react-use-callback-ref": "1.1.1", "@radix-ui/react-use-escape-keydown": "1.1.1" }, "peerDependencies": { "@types/react": "*", "@types/react-dom": "*", "react": "^16.8 || ^17.0 || ^18.0 || ^19.0 || ^19.0.0-rc", "react-dom": "^16.8 || ^17.0 || ^18.0 || ^19.0 || ^19.0.0-rc" }, "optionalPeers": ["@types/react", "@types/react-dom"] }, "sha512-Nqcp+t5cTB8BinFkZgXiMJniQH0PsUt2k51FUhbdfeKvc4ACcG2uQniY/8+h1Yv6Kza4Q7lD7PQV0z0oicE0Mg=="], + "@radix-ui/react-dropdown-menu": ["@radix-ui/react-dropdown-menu@2.1.16", "", { "dependencies": { "@radix-ui/primitive": "1.1.3", "@radix-ui/react-compose-refs": "1.1.2", "@radix-ui/react-context": "1.1.2", "@radix-ui/react-id": "1.1.1", "@radix-ui/react-menu": "2.1.16", "@radix-ui/react-primitive": "2.1.3", "@radix-ui/react-use-controllable-state": "1.2.2" }, "peerDependencies": { "@types/react": "*", "@types/react-dom": "*", "react": "^16.8 || ^17.0 || ^18.0 || ^19.0 || ^19.0.0-rc", "react-dom": "^16.8 || ^17.0 || ^18.0 || ^19.0 || ^19.0.0-rc" }, "optionalPeers": ["@types/react", "@types/react-dom"] }, "sha512-1PLGQEynI/3OX/ftV54COn+3Sud/Mn8vALg2rWnBLnRaGtJDduNW/22XjlGgPdpcIbiQxjKtb7BkcjP00nqfJw=="], + "@radix-ui/react-focus-guards": ["@radix-ui/react-focus-guards@1.1.3", "", { "peerDependencies": { "@types/react": "*", "react": "^16.8 || ^17.0 || ^18.0 || ^19.0 || ^19.0.0-rc" }, "optionalPeers": ["@types/react"] }, "sha512-0rFg/Rj2Q62NCm62jZw0QX7a3sz6QCQU0LpZdNrJX8byRGaGVTqbrW9jAoIAHyMQqsNpeZ81YgSizOt5WXq0Pw=="], "@radix-ui/react-focus-scope": ["@radix-ui/react-focus-scope@1.1.7", "", { "dependencies": { "@radix-ui/react-compose-refs": "1.1.2", "@radix-ui/react-primitive": "2.1.3", "@radix-ui/react-use-callback-ref": "1.1.1" }, "peerDependencies": { "@types/react": "*", "@types/react-dom": "*", "react": "^16.8 || ^17.0 || ^18.0 || ^19.0 || ^19.0.0-rc", "react-dom": "^16.8 || ^17.0 || ^18.0 || ^19.0 || ^19.0.0-rc" }, "optionalPeers": ["@types/react", "@types/react-dom"] }, "sha512-t2ODlkXBQyn7jkl6TNaw/MtVEVvIGelJDCG41Okq/KwUsJBwQ4XVZsHAVUkK4mBv3ewiAS3PGuUWuY2BoK4ZUw=="], "@radix-ui/react-id": ["@radix-ui/react-id@1.1.1", "", { "dependencies": { "@radix-ui/react-use-layout-effect": "1.1.1" }, "peerDependencies": { "@types/react": "*", "react": "^16.8 || ^17.0 || ^18.0 || ^19.0 || ^19.0.0-rc" }, "optionalPeers": ["@types/react"] }, "sha512-kGkGegYIdQsOb4XjsfM97rXsiHaBwco+hFI66oO4s9LU+PLAC5oJ7khdOVFxkhsmlbpUqDAvXw11CluXP+jkHg=="], + "@radix-ui/react-menu": ["@radix-ui/react-menu@2.1.16", "", { "dependencies": { "@radix-ui/primitive": "1.1.3", "@radix-ui/react-collection": "1.1.7", "@radix-ui/react-compose-refs": "1.1.2", "@radix-ui/react-context": "1.1.2", "@radix-ui/react-direction": "1.1.1", "@radix-ui/react-dismissable-layer": "1.1.11", "@radix-ui/react-focus-guards": "1.1.3", "@radix-ui/react-focus-scope": "1.1.7", "@radix-ui/react-id": "1.1.1", "@radix-ui/react-popper": "1.2.8", "@radix-ui/react-portal": "1.1.9", "@radix-ui/react-presence": "1.1.5", "@radix-ui/react-primitive": "2.1.3", "@radix-ui/react-roving-focus": "1.1.11", "@radix-ui/react-slot": "1.2.3", "@radix-ui/react-use-callback-ref": "1.1.1", "aria-hidden": "^1.2.4", "react-remove-scroll": "^2.6.3" }, "peerDependencies": { "@types/react": "*", "@types/react-dom": "*", "react": "^16.8 || ^17.0 || ^18.0 || ^19.0 || ^19.0.0-rc", "react-dom": "^16.8 || ^17.0 || ^18.0 || ^19.0 || ^19.0.0-rc" }, "optionalPeers": ["@types/react", "@types/react-dom"] }, "sha512-72F2T+PLlphrqLcAotYPp0uJMr5SjP5SL01wfEspJbru5Zs5vQaSHb4VB3ZMJPimgHHCHG7gMOeOB9H3Hdmtxg=="], + + "@radix-ui/react-popover": ["@radix-ui/react-popover@1.1.15", "", { "dependencies": { "@radix-ui/primitive": "1.1.3", "@radix-ui/react-compose-refs": "1.1.2", "@radix-ui/react-context": "1.1.2", "@radix-ui/react-dismissable-layer": "1.1.11", "@radix-ui/react-focus-guards": "1.1.3", "@radix-ui/react-focus-scope": "1.1.7", "@radix-ui/react-id": "1.1.1", "@radix-ui/react-popper": "1.2.8", "@radix-ui/react-portal": "1.1.9", "@radix-ui/react-presence": "1.1.5", "@radix-ui/react-primitive": "2.1.3", "@radix-ui/react-slot": "1.2.3", "@radix-ui/react-use-controllable-state": "1.2.2", "aria-hidden": "^1.2.4", "react-remove-scroll": "^2.6.3" }, "peerDependencies": { "@types/react": "*", "@types/react-dom": "*", "react": "^16.8 || ^17.0 || ^18.0 || ^19.0 || ^19.0.0-rc", "react-dom": "^16.8 || ^17.0 || ^18.0 || ^19.0 || ^19.0.0-rc" }, "optionalPeers": ["@types/react", "@types/react-dom"] }, "sha512-kr0X2+6Yy/vJzLYJUPCZEc8SfQcf+1COFoAqauJm74umQhta9M7lNJHP7QQS3vkvcGLQUbWpMzwrXYwrYztHKA=="], + "@radix-ui/react-popper": ["@radix-ui/react-popper@1.2.8", "", { "dependencies": { "@floating-ui/react-dom": "^2.0.0", "@radix-ui/react-arrow": "1.1.7", "@radix-ui/react-compose-refs": "1.1.2", "@radix-ui/react-context": "1.1.2", "@radix-ui/react-primitive": "2.1.3", "@radix-ui/react-use-callback-ref": "1.1.1", "@radix-ui/react-use-layout-effect": "1.1.1", "@radix-ui/react-use-rect": "1.1.1", "@radix-ui/react-use-size": "1.1.1", "@radix-ui/rect": "1.1.1" }, "peerDependencies": { "@types/react": "*", "@types/react-dom": "*", "react": "^16.8 || ^17.0 || ^18.0 || ^19.0 || ^19.0.0-rc", "react-dom": "^16.8 || ^17.0 || ^18.0 || ^19.0 || ^19.0.0-rc" }, "optionalPeers": ["@types/react", "@types/react-dom"] }, "sha512-0NJQ4LFFUuWkE7Oxf0htBKS6zLkkjBH+hM1uk7Ng705ReR8m/uelduy1DBo0PyBXPKVnBA6YBlU94MBGXrSBCw=="], "@radix-ui/react-portal": ["@radix-ui/react-portal@1.1.9", "", { "dependencies": { "@radix-ui/react-primitive": "2.1.3", "@radix-ui/react-use-layout-effect": "1.1.1" }, "peerDependencies": { "@types/react": "*", "@types/react-dom": "*", "react": "^16.8 || ^17.0 || ^18.0 || ^19.0 || ^19.0.0-rc", "react-dom": "^16.8 || ^17.0 || ^18.0 || ^19.0 || ^19.0.0-rc" }, "optionalPeers": ["@types/react", "@types/react-dom"] }, "sha512-bpIxvq03if6UNwXZ+HTK71JLh4APvnXntDc6XOX8UVq4XQOVl7lwok0AvIl+b8zgCw3fSaVTZMpAPPagXbKmHQ=="], @@ -742,6 +756,8 @@ "@radix-ui/react-primitive": ["@radix-ui/react-primitive@2.1.3", "", { "dependencies": { "@radix-ui/react-slot": "1.2.3" }, "peerDependencies": { "@types/react": "*", "@types/react-dom": "*", "react": "^16.8 || ^17.0 || ^18.0 || ^19.0 || ^19.0.0-rc", "react-dom": "^16.8 || ^17.0 || ^18.0 || ^19.0 || ^19.0.0-rc" }, "optionalPeers": ["@types/react", "@types/react-dom"] }, "sha512-m9gTwRkhy2lvCPe6QJp4d3G1TYEUHn/FzJUtq9MjH46an1wJU+GdoGC5VLof8RX8Ft/DlpshApkhswDLZzHIcQ=="], + "@radix-ui/react-roving-focus": ["@radix-ui/react-roving-focus@1.1.11", "", { "dependencies": { "@radix-ui/primitive": "1.1.3", "@radix-ui/react-collection": "1.1.7", "@radix-ui/react-compose-refs": "1.1.2", "@radix-ui/react-context": "1.1.2", "@radix-ui/react-direction": "1.1.1", "@radix-ui/react-id": "1.1.1", "@radix-ui/react-primitive": "2.1.3", "@radix-ui/react-use-callback-ref": "1.1.1", "@radix-ui/react-use-controllable-state": "1.2.2" }, "peerDependencies": { "@types/react": "*", "@types/react-dom": "*", "react": "^16.8 || ^17.0 || ^18.0 || ^19.0 || ^19.0.0-rc", "react-dom": "^16.8 || ^17.0 || ^18.0 || ^19.0 || ^19.0.0-rc" }, "optionalPeers": ["@types/react", "@types/react-dom"] }, "sha512-7A6S9jSgm/S+7MdtNDSb+IU859vQqJ/QAtcYQcfFC6W8RS4IxIZDldLR0xqCFZ6DCyrQLjLPsxtTNch5jVA4lA=="], + "@radix-ui/react-slot": ["@radix-ui/react-slot@1.2.3", "", { "dependencies": { "@radix-ui/react-compose-refs": "1.1.2" }, "peerDependencies": { "@types/react": "*", "react": "^16.8 || ^17.0 || ^18.0 || ^19.0 || ^19.0.0-rc" }, "optionalPeers": ["@types/react"] }, "sha512-aeNmHnBxbi2St0au6VBVC7JXFlhLlOnvIIlePNniyUNAClzmtAUEY8/pBiK3iHjufOlwA+c20/8jngo7xcrg8A=="], "@radix-ui/react-tooltip": ["@radix-ui/react-tooltip@1.2.8", "", { "dependencies": { "@radix-ui/primitive": "1.1.3", "@radix-ui/react-compose-refs": "1.1.2", "@radix-ui/react-context": "1.1.2", "@radix-ui/react-dismissable-layer": "1.1.11", "@radix-ui/react-id": "1.1.1", "@radix-ui/react-popper": "1.2.8", "@radix-ui/react-portal": "1.1.9", "@radix-ui/react-presence": "1.1.5", "@radix-ui/react-primitive": "2.1.3", "@radix-ui/react-slot": "1.2.3", "@radix-ui/react-use-controllable-state": "1.2.2", "@radix-ui/react-visually-hidden": "1.2.3" }, "peerDependencies": { "@types/react": "*", "@types/react-dom": "*", "react": "^16.8 || ^17.0 || ^18.0 || ^19.0 || ^19.0.0-rc", "react-dom": "^16.8 || ^17.0 || ^18.0 || ^19.0 || ^19.0.0-rc" }, "optionalPeers": ["@types/react", "@types/react-dom"] }, "sha512-tY7sVt1yL9ozIxvmbtN5qtmH2krXcBCfjEiCgKGLqunJHvgvZG2Pcl2oQ3kbcZARb1BGEHdkLzcYGO8ynVlieg=="], diff --git a/packages/ai/providers/codex-sdk.ts b/packages/ai/providers/codex-sdk.ts index 0c6da35c..1088be57 100644 --- a/packages/ai/providers/codex-sdk.ts +++ b/packages/ai/providers/codex-sdk.ts @@ -41,6 +41,7 @@ export class CodexSDKProvider implements AIProvider { tools: true, }; readonly models = [ + { id: 'gpt-5.5', label: 'GPT-5.5' }, { id: 'gpt-5.4', label: 'GPT-5.4', default: true }, { id: 'gpt-5.4-mini', label: 'GPT-5.4 Mini' }, { id: 'gpt-5.3-codex', label: 'GPT-5.3 Codex' }, diff --git a/packages/review-editor/App.tsx b/packages/review-editor/App.tsx index bfa17f80..cc20e42b 100644 --- a/packages/review-editor/App.tsx +++ b/packages/review-editor/App.tsx @@ -1,6 +1,7 @@ import React, { useState, useEffect, useCallback, useMemo, useRef } from 'react'; import { type Origin, getAgentName } from '@plannotator/shared/agents'; import { ThemeProvider, useTheme } from '@plannotator/ui/components/ThemeProvider'; +import { TooltipProvider } from '@plannotator/ui/components/Tooltip'; import { ConfirmDialog } from '@plannotator/ui/components/ConfirmDialog'; import { Settings } from '@plannotator/ui/components/Settings'; import { FeedbackButton, ApproveButton, ExitButton } from '@plannotator/ui/components/ToolbarButtons'; @@ -158,6 +159,17 @@ const ReviewApp: React.FC = () => { const [isWSL, setIsWSL] = useState(false); const [diffType, setDiffType] = useState('uncommitted'); const [gitContext, setGitContext] = useState(null); + // Two bases: + // selectedBase — what the picker is currently showing (UI intent). + // Updates immediately when the user picks, so the chip + // feels responsive. + // committedBase — the base the server last computed the patch against. + // Drives file-content fetches. Only updates after + // /api/diff/switch returns, so we never pair an old + // patch with a new base's file contents (race that + // produced "trailing context mismatch" warnings). + const [selectedBase, setSelectedBase] = useState(null); + const [committedBase, setCommittedBase] = useState(null); const [agentCwd, setAgentCwd] = useState(null); const [isLoadingDiff, setIsLoadingDiff] = useState(false); const [diffError, setDiffError] = useState(null); @@ -666,6 +678,7 @@ const ReviewApp: React.FC = () => { gitRef: string; origin?: Origin; diffType?: string; + base?: string; gitContext?: GitContext; agentCwd?: string; sharingEnabled?: boolean; @@ -694,7 +707,15 @@ const ReviewApp: React.FC = () => { setFiles(apiFiles); if (data.origin) setOrigin(data.origin); if (data.diffType) setDiffType(data.diffType); - if (data.gitContext) setGitContext(data.gitContext); + if (data.gitContext) { + setGitContext(data.gitContext); + // Prefer the server's active base (survives page refresh / reconnect) + // over the detected default, so the picker rehydrates to what the + // server is actually using. + const initial = data.base || data.gitContext.defaultBranch || null; + setSelectedBase(initial); + setCommittedBase(initial); + } if (data.agentCwd) setAgentCwd(data.agentCwd); if (data.sharingEnabled !== undefined) setSharingEnabled(data.sharingEnabled); if (data.repoInfo) setRepoInfo(data.repoInfo); @@ -904,7 +925,7 @@ const ReviewApp: React.FC = () => { const lastColon = rest.lastIndexOf(':'); if (lastColon !== -1) { const sub = rest.slice(lastColon + 1); - if (['uncommitted', 'staged', 'unstaged', 'last-commit', 'branch'].includes(sub)) { + if (['uncommitted', 'staged', 'unstaged', 'last-commit', 'branch', 'merge-base'].includes(sub)) { return { activeWorktreePath: rest.slice(0, lastColon), activeDiffBase: sub }; } } @@ -925,14 +946,21 @@ const ReviewApp: React.FC = () => { // Staging is never available in PR review mode — the server rejects it and the UI shouldn't offer it. const canStageFiles = canStageRaw && !prMetadata; - // Shared helper: fetch a diff switch and update state - const fetchDiffSwitch = useCallback(async (fullDiffType: string) => { + // Shared helper: fetch a diff switch and update state. + // Returns true on success, false on failure — callers that optimistically + // updated UI state (e.g. the base picker) can use this to revert. + const fetchDiffSwitch = useCallback(async (fullDiffType: string, baseOverride?: string): Promise => { setIsLoadingDiff(true); try { const res = await fetch('/api/diff/switch', { method: 'POST', headers: { 'Content-Type': 'application/json' }, - body: JSON.stringify({ diffType: fullDiffType }), + body: JSON.stringify({ + diffType: fullDiffType, + // Server ignores base for modes that don't use it (uncommitted/staged/etc), + // so forwarding unconditionally is safe and keeps the request shape uniform. + ...((baseOverride ?? selectedBase) && { base: baseOverride ?? selectedBase }), + }), }); if (!res.ok) throw new Error('Failed to switch diff'); @@ -941,6 +969,8 @@ const ReviewApp: React.FC = () => { rawPatch: string; gitRef: string; diffType: string; + base?: string; + gitContext?: GitContext; error?: string; }; @@ -950,17 +980,67 @@ const ReviewApp: React.FC = () => { setDiffData(prev => prev ? { ...prev, rawPatch: data.rawPatch, gitRef: data.gitRef, diffType: data.diffType } : prev); setFiles(nextFiles); setDiffType(data.diffType); + // Adopt the server's echoed base. The server trusts whatever we sent + // verbatim — this sync just makes sure selectedBase and committedBase + // match the server's view (important when the caller didn't send a + // base and the server used the detected default instead). + if (data.base) { + setSelectedBase(data.base); + setCommittedBase(data.base); + } + // Merge only the per-cwd fields so the sidebar reflects the worktree + // we're now in. Keep the original `worktrees` list (already filtered to + // exclude the server's startup cwd — replacing it with the new context's + // list would duplicate the "Main repo" entry) and `availableBranches` + // (shared across worktrees of the same repo). + // + // IMPORTANT: we deliberately do NOT overwrite `currentBranch`. The + // WorktreePicker's top "launch" row uses it as a label, and that row + // represents the cwd plannotator was launched in — not whichever + // worktree is currently active. Freezing `currentBranch` at its + // initial-load value keeps that label truthful. `defaultBranch` and + // `diffOptions` update because they describe the active diff, which + // other UI (empty-state text, diff-type picker) should see fresh. + if (data.gitContext) { + setGitContext((prev) => { + if (!prev) return data.gitContext!; + return { + ...prev, + defaultBranch: data.gitContext!.defaultBranch, + diffOptions: data.gitContext!.diffOptions, + }; + }); + } setActiveFileIndex(0); setPendingSelection(null); setDiffError(data.error || null); resetStagedFiles(); + return true; } catch (err) { console.error('Failed to switch diff:', err); setDiffError(err instanceof Error ? err.message : 'Failed to switch diff'); + return false; } finally { setIsLoadingDiff(false); } - }, [dockApi, resetStagedFiles]); + }, [dockApi, resetStagedFiles, selectedBase]); + + // Switch the base branch the current diff compares against. + // Only triggers a refetch when the active mode actually uses a base. + // Optimistically updates the picker; reverts if the server-side switch + // fails so the chip doesn't lie about what the viewer is actually showing. + const handleBaseSelect = useCallback( + async (branch: string) => { + if (branch === selectedBase) return; + const previous = selectedBase; + setSelectedBase(branch); + if (activeDiffBase === 'branch' || activeDiffBase === 'merge-base') { + const ok = await fetchDiffSwitch(diffType, branch); + if (!ok) setSelectedBase(previous); + } + }, + [selectedBase, activeDiffBase, diffType, fetchDiffSwitch], + ); // Switch diff type (uncommitted, last-commit, branch) — composes worktree prefix if active const handleDiffSwitch = useCallback(async (baseDiffType: string) => { @@ -971,14 +1051,17 @@ const ReviewApp: React.FC = () => { await fetchDiffSwitch(fullDiffType); }, [diffType, activeWorktreePath, fetchDiffSwitch]); - // Switch worktree context (or back to main repo) + // Switch worktree context (or back to main repo). Preserves the current + // diff mode across the switch — if the reviewer was looking at "PR Diff" + // in the main repo, they should keep looking at "PR Diff" in the target + // worktree rather than being silently snapped back to "Uncommitted". const handleWorktreeSwitch = useCallback(async (worktreePath: string | null) => { if (worktreePath === activeWorktreePath) return; const fullDiffType = worktreePath - ? `worktree:${worktreePath}:uncommitted` - : 'uncommitted'; + ? `worktree:${worktreePath}:${activeDiffBase}` + : activeDiffBase; await fetchDiffSwitch(fullDiffType); - }, [activeWorktreePath, fetchDiffSwitch]); + }, [activeWorktreePath, activeDiffBase, fetchDiffSwitch]); // Select annotation - switches file if needed and scrolls to it const handleSelectAnnotation = useCallback((id: string | null) => { @@ -1003,6 +1086,26 @@ const ReviewApp: React.FC = () => { setSelectedAnnotationId(id); }, [allAnnotations, files, handleFileSwitch]); + // Diff context bundled into local-mode feedback headers so the receiving + // agent knows which diff the annotations are anchored to. Uses committedBase + // (what the server actually computed) and activeDiffBase/activeWorktreePath + // (derived from the committed diffType). Skipped in PR mode — the PR header + // already carries the relevant context. + // Declared before reviewStateValue because both reviewStateValue and the + // feedbackMarkdown memo below read it; moving it below either would put it + // in the TDZ when those memos run on first render. + const feedbackDiffContext = useMemo( + () => + prMetadata || !activeDiffBase + ? undefined + : { + mode: activeDiffBase, + base: committedBase ?? undefined, + worktreePath: activeWorktreePath, + }, + [prMetadata, activeDiffBase, committedBase, activeWorktreePath], + ); + // Build ReviewState value for dock panel context const reviewStateValue = useMemo(() => ({ files, @@ -1016,6 +1119,17 @@ const ReviewApp: React.FC = () => { disableBackground: !diffShowBackground, fontFamily: diffFontFamily || undefined, fontSize: diffFontSize || undefined, + // Only propagate base for modes where it affects old/new content. Avoids + // needless file-content re-fetches when switching to uncommitted/staged/etc. + // Uses committedBase (not selectedBase) so file-content queries wait for + // the new patch to arrive before refetching — otherwise the viewer can + // briefly pair an old patch with the new base's content. + reviewBase: + (activeDiffBase === 'branch' || activeDiffBase === 'merge-base') + ? committedBase ?? undefined + : undefined, + activeDiffBase, + feedbackDiffContext, allAnnotations, externalAnnotations, selectedAnnotationId, @@ -1058,7 +1172,8 @@ const ReviewApp: React.FC = () => { }), [ files, activeFileIndex, diffStyle, diffOverflow, diffIndicators, diffLineDiffType, diffShowLineNumbers, diffShowBackground, - diffFontFamily, diffFontSize, allAnnotations, externalAnnotations, + diffFontFamily, diffFontSize, activeDiffBase, committedBase, feedbackDiffContext, + allAnnotations, externalAnnotations, selectedAnnotationId, pendingSelection, handleLineSelection, handleAddAnnotation, handleAddFileComment, handleEditAnnotation, handleSelectAnnotation, handleDeleteAnnotation, viewedFiles, @@ -1096,7 +1211,7 @@ const ReviewApp: React.FC = () => { return; } try { - const feedback = exportReviewFeedback(allAnnotations, prMetadata); + const feedback = exportReviewFeedback(allAnnotations, prMetadata, feedbackDiffContext); await navigator.clipboard.writeText(feedback); setCopyFeedback('Feedback copied!'); setTimeout(() => setCopyFeedback(null), 2000); @@ -1105,15 +1220,15 @@ const ReviewApp: React.FC = () => { setCopyFeedback('Failed to copy'); setTimeout(() => setCopyFeedback(null), 2000); } - }, [allAnnotations, prMetadata]); + }, [allAnnotations, prMetadata, feedbackDiffContext]); const feedbackMarkdown = useMemo(() => { - let output = exportReviewFeedback(allAnnotations, prMetadata); + let output = exportReviewFeedback(allAnnotations, prMetadata, feedbackDiffContext); if (editorAnnotations.length > 0) { output += exportEditorAnnotations(editorAnnotations); } return output; - }, [allAnnotations, prMetadata, editorAnnotations]); + }, [allAnnotations, prMetadata, feedbackDiffContext, editorAnnotations]); const totalAnnotationCount = allAnnotations.length + editorAnnotations.length; @@ -1404,6 +1519,7 @@ const ReviewApp: React.FC = () => { return ( +
@@ -1702,6 +1818,10 @@ const ReviewApp: React.FC = () => { activeWorktreePath={activeWorktreePath} onSelectWorktree={handleWorktreeSwitch} currentBranch={gitContext?.currentBranch} + availableBranches={prMetadata ? undefined : gitContext?.availableBranches} + selectedBase={prMetadata ? undefined : selectedBase ?? undefined} + detectedBase={prMetadata ? undefined : gitContext?.defaultBranch} + onSelectBase={prMetadata ? undefined : handleBaseSelect} stagedFiles={stagedFiles} onCopyRawDiff={handleCopyDiff} canCopyRawDiff={!!diffData?.rawPatch} @@ -1777,7 +1897,8 @@ const ReviewApp: React.FC = () => { {activeDiffBase === 'staged' && "No staged changes. Stage some files with git add."} {activeDiffBase === 'unstaged' && "No unstaged changes. All changes are staged."} {activeDiffBase === 'last-commit' && `No changes in the last commit${activeWorktreePath ? ' in this worktree' : ''}.`} - {activeDiffBase === 'branch' && `No changes vs ${gitContext?.defaultBranch || 'main'}${activeWorktreePath ? ' in this worktree' : ''}.`} + {activeDiffBase === 'branch' && `No changes vs ${selectedBase || gitContext?.defaultBranch || 'main'}${activeWorktreePath ? ' in this worktree' : ''}.`} + {activeDiffBase === 'merge-base' && `No changes vs ${selectedBase || gitContext?.defaultBranch || 'main'}${activeWorktreePath ? ' in this worktree' : ''}.`}

)} @@ -2078,6 +2199,7 @@ const ReviewApp: React.FC = () => { + ); }; diff --git a/packages/review-editor/components/BaseBranchPicker.tsx b/packages/review-editor/components/BaseBranchPicker.tsx new file mode 100644 index 00000000..5810d3f8 --- /dev/null +++ b/packages/review-editor/components/BaseBranchPicker.tsx @@ -0,0 +1,193 @@ +import React, { useMemo, useRef, useState } from 'react'; +import * as Popover from '@radix-ui/react-popover'; +import type { AvailableBranches } from '@plannotator/shared/types'; + +interface BaseBranchPickerProps { + availableBranches: AvailableBranches; + selectedBase: string; + detectedBase: string; + onSelectBase: (branch: string) => void; + disabled?: boolean; +} + +export const BaseBranchPicker: React.FC = ({ + availableBranches, + selectedBase, + detectedBase, + onSelectBase, + disabled, +}) => { + const [open, setOpen] = useState(false); + const [query, setQuery] = useState(''); + const searchRef = useRef(null); + + const { local, remote } = availableBranches; + const filtered = useMemo(() => { + const q = query.trim().toLowerCase(); + if (!q) return { local, remote }; + return { + local: local.filter((b) => b.toLowerCase().includes(q)), + remote: remote.filter((b) => b.toLowerCase().includes(q)), + }; + }, [local, remote, query]); + + const handleSelect = (branch: string) => { + onSelectBase(branch); + setOpen(false); + setQuery(''); + }; + + const handleReset = () => { + onSelectBase(detectedBase); + setOpen(false); + setQuery(''); + }; + + const isCustom = selectedBase !== detectedBase; + + return ( + { + setOpen(v); + if (!v) setQuery(''); + }} + > + + + + + { + e.preventDefault(); + searchRef.current?.focus(); + }} + > +
+ setQuery(e.target.value)} + placeholder="Search branches…" + className="w-full px-2 py-1.5 bg-muted rounded text-xs text-foreground placeholder:text-muted-foreground focus:outline-none focus:ring-1 focus:ring-primary/50" + /> +
+
+ {filtered.local.length === 0 && filtered.remote.length === 0 && ( +
+ No branches match. +
+ )} + {filtered.local.length > 0 && ( + + )} + {filtered.remote.length > 0 && ( + + )} +
+ {isCustom && ( +
+ +
+ )} +
+
+
+ ); +}; + +interface BranchGroupProps { + title: string; + branches: string[]; + selectedBase: string; + detectedBase: string; + onSelect: (branch: string) => void; +} + +const BranchGroup: React.FC = ({ + title, + branches, + selectedBase, + detectedBase, + onSelect, +}) => ( +
+
+ {title} +
+ {branches.map((branch) => { + const isSelected = branch === selectedBase; + const isDetected = branch === detectedBase; + return ( + + ); + })} +
+); diff --git a/packages/review-editor/components/DiffTypePicker.tsx b/packages/review-editor/components/DiffTypePicker.tsx new file mode 100644 index 00000000..3547c138 --- /dev/null +++ b/packages/review-editor/components/DiffTypePicker.tsx @@ -0,0 +1,117 @@ +import React, { useState } from 'react'; +import * as DropdownMenu from '@radix-ui/react-dropdown-menu'; +import { Tooltip } from '@plannotator/ui/components/Tooltip'; +import type { DiffOption } from '@plannotator/shared/types'; + +interface DiffTypePickerProps { + options: DiffOption[]; + activeDiffType: string; + onSelect: (diffType: string) => void; + isLoading?: boolean; + hasBasePicker: boolean; +} + +/** + * Plain-English explanations shown in a tooltip next to each option. + * Keep these short — they're hover hints, not docs. + */ +const OPTION_HINTS: Record = { + uncommitted: "All your local changes — anything you haven't committed yet.", + staged: "Only what you've run `git add` on.", + unstaged: "What `git diff` shows with no arguments.", + 'last-commit': "Just your most recent commit.", + branch: "Straight compare against the base branch (picked below). Can show commits that aren't yours if the base has new commits.", + 'merge-base': "Only what you've added on top of the base branch (picked below). Same as GitHub's PR view.", +}; + +export const DiffTypePicker: React.FC = ({ + options, + activeDiffType, + onSelect, + isLoading, + hasBasePicker, +}) => { + const [open, setOpen] = useState(false); + + // When a base picker is wired up, strip branch names from the + // base-dependent labels — the branch belongs in the picker. + const displayLabel = (opt: DiffOption) => { + if (!hasBasePicker) return opt.label; + if (opt.id === 'branch') return 'Branch diff'; + if (opt.id === 'merge-base') return 'PR Diff'; + return opt.label; + }; + + const active = options.find((o) => o.id === activeDiffType); + const activeLabel = active ? displayLabel(active) : 'Select diff'; + + return ( + + + + + + + {options.map((opt) => { + const hint = OPTION_HINTS[opt.id]; + const isActive = opt.id === activeDiffType; + return ( + onSelect(opt.id)} + className={`flex items-center gap-2 mx-1 px-2 py-1.5 text-xs rounded cursor-pointer outline-none data-[highlighted]:bg-muted ${ + isActive ? 'text-foreground font-medium' : 'text-foreground/80' + }`} + > + + {isActive && ( + + + + )} + + {displayLabel(opt)} + {hint && ( + + e.stopPropagation()} + onPointerUp={(e) => e.stopPropagation()} + onClick={(e) => e.stopPropagation()} + > + {/* Heroicons information-circle (outline) */} + + + + + + )} + + ); + })} + + + + ); +}; diff --git a/packages/review-editor/components/DiffViewer.tsx b/packages/review-editor/components/DiffViewer.tsx index 5a4aaf65..bd5cfba1 100644 --- a/packages/review-editor/components/DiffViewer.tsx +++ b/packages/review-editor/components/DiffViewer.tsx @@ -118,6 +118,8 @@ interface DiffViewerProps { patch: string; filePath: string; oldPath?: string; + /** Base branch override used for file-content lookups (branch / merge-base modes only). */ + reviewBase?: string; isFocused?: boolean; diffStyle: 'split' | 'unified'; diffOverflow?: 'scroll' | 'wrap'; @@ -162,6 +164,7 @@ export const DiffViewer: React.FC = ({ patch, filePath, oldPath, + reviewBase, isFocused = false, diffStyle, diffOverflow, @@ -275,6 +278,7 @@ export const DiffViewer: React.FC = ({ setFileContents(null); const params = new URLSearchParams({ path: filePath }); if (oldPath) params.set('oldPath', oldPath); + if (reviewBase) params.set('base', reviewBase); fetch(`/api/file-content?${params}`, { signal: controller.signal }) .then(res => res.ok ? res.json() : null) .then((data: { oldContent: string | null; newContent: string | null } | null) => { @@ -284,7 +288,7 @@ export const DiffViewer: React.FC = ({ }) .catch(() => {}); // Silent fallback — no expansion in demo mode return () => controller.abort(); - }, [filePath, oldPath]); + }, [filePath, oldPath, reviewBase]); // Re-parse the patch with full file contents so hunk indices are computed // against the complete file (isPartial: false), enabling expansion. diff --git a/packages/review-editor/components/FileTree.tsx b/packages/review-editor/components/FileTree.tsx index 4abbc1f7..284ff044 100644 --- a/packages/review-editor/components/FileTree.tsx +++ b/packages/review-editor/components/FileTree.tsx @@ -1,8 +1,11 @@ import React, { useEffect, useCallback, useState, useMemo } from 'react'; import { CodeAnnotation } from '@plannotator/ui/types'; -import type { DiffOption, WorktreeInfo } from '@plannotator/shared/types'; +import type { AvailableBranches, DiffOption, WorktreeInfo } from '@plannotator/shared/types'; import { buildFileTree, getAncestorPaths, getAllFolderPaths } from '../utils/buildFileTree'; import { FileTreeNodeItem } from './FileTreeNode'; +import { BaseBranchPicker } from './BaseBranchPicker'; +import { DiffTypePicker } from './DiffTypePicker'; +import { WorktreePicker } from './WorktreePicker'; import { getReviewSearchSideLabel, type ReviewSearchFileGroup, type ReviewSearchMatch } from '../utils/reviewSearch'; import type { DiffFile } from '../types'; import { OverlayScrollArea } from '@plannotator/ui/components/OverlayScrollArea'; @@ -27,6 +30,11 @@ interface FileTreeProps { activeWorktreePath?: string | null; onSelectWorktree?: (path: string | null) => void; currentBranch?: string; + /** Base-branch picker — only meaningful when activeDiffType is "branch" or "merge-base". */ + availableBranches?: AvailableBranches; + selectedBase?: string; + detectedBase?: string; + onSelectBase?: (branch: string) => void; stagedFiles?: Set; onCopyRawDiff?: () => void; canCopyRawDiff?: boolean; @@ -66,6 +74,10 @@ export const FileTree: React.FC = ({ activeWorktreePath, onSelectWorktree, currentBranch, + availableBranches, + selectedBase, + detectedBase, + onSelectBase, stagedFiles, onCopyRawDiff, canCopyRawDiff = false, @@ -95,6 +107,20 @@ export const FileTree: React.FC = ({ return; } + // Yield keyboard nav when a floating overlay owns the focus — Radix + // DropdownMenu / Popover / Dialog handle arrow keys themselves, and the + // old native onSelectWorktree(e.target.value || null)} +
+ - - {worktrees.map(wt => ( - - ))} - -
- - - -
+ />
)} {diffOptions && diffOptions.length > 0 && onSelectDiff && ( -
- -
- {isLoadingDiff ? ( - - - - - ) : ( - - - - )} -
+
+
)}
)} + {/* Base-branch picker — only relevant for branch / merge-base diff types */} + {onSelectBase && + selectedBase && + detectedBase && + availableBranches && + (activeDiffType === 'branch' || activeDiffType === 'merge-base') && ( +
+ + compare against + +
+ +
+
+ )} + {/* File tree or search results */}
diff --git a/packages/review-editor/components/WorktreePicker.tsx b/packages/review-editor/components/WorktreePicker.tsx new file mode 100644 index 00000000..5c589d5b --- /dev/null +++ b/packages/review-editor/components/WorktreePicker.tsx @@ -0,0 +1,191 @@ +import React, { useMemo, useRef, useState } from 'react'; +import * as Popover from '@radix-ui/react-popover'; +import type { WorktreeInfo } from '@plannotator/shared/types'; + +interface WorktreePickerProps { + worktrees: WorktreeInfo[]; + activeWorktreePath: string | null; + currentBranch?: string; + onSelect: (path: string | null) => void; + disabled?: boolean; +} + +const MAIN_REPO = null; + +export const WorktreePicker: React.FC = ({ + worktrees, + activeWorktreePath, + currentBranch, + onSelect, + disabled, +}) => { + const [open, setOpen] = useState(false); + const [query, setQuery] = useState(''); + const searchRef = useRef(null); + + const mainLabel = currentBranch || 'Main repo'; + + const filtered = useMemo(() => { + const q = query.trim().toLowerCase(); + if (!q) return worktrees; + return worktrees.filter((wt) => { + const branch = (wt.branch || '').toLowerCase(); + const path = wt.path.toLowerCase(); + return branch.includes(q) || path.includes(q); + }); + }, [worktrees, query]); + + const mainMatchesQuery = !query.trim() || mainLabel.toLowerCase().includes(query.trim().toLowerCase()); + + const handleSelect = (path: string | null) => { + onSelect(path); + setOpen(false); + setQuery(''); + }; + + const active = activeWorktreePath + ? worktrees.find((wt) => wt.path === activeWorktreePath) + : null; + const activeLabel = active + ? (active.branch || active.path.split('/').pop() || 'worktree') + : mainLabel; + const isCustom = activeWorktreePath !== null; + + return ( + { + setOpen(v); + if (!v) setQuery(''); + }} + > + + + + + { + // Only override Radix's default focus when the search input is + // actually rendered — otherwise the preventDefault() would leave + // focus on the trigger, and arrow keys would bubble out to the + // file-tree nav. For short worktree lists we let Radix focus the + // first picker button itself. + if (searchRef.current) { + e.preventDefault(); + searchRef.current.focus(); + } + }} + > + {worktrees.length > 3 && ( +
+ setQuery(e.target.value)} + placeholder="Search worktrees…" + className="w-full px-2 py-1.5 bg-muted rounded text-xs text-foreground placeholder:text-muted-foreground focus:outline-none focus:ring-1 focus:ring-primary/50" + /> +
+ )} +
+ {mainMatchesQuery && ( + handleSelect(MAIN_REPO)} + /> + )} + + {filtered.length > 0 && ( + <> + {mainMatchesQuery && ( +
+ )} +
+ Worktrees +
+ {filtered.map((wt) => ( + handleSelect(wt.path)} + /> + ))} + + )} + + {!mainMatchesQuery && filtered.length === 0 && ( +
+ No worktrees match. +
+ )} +
+ + + + ); +}; + +interface WorktreeRowProps { + label: string; + sublabel: string | null; + isSelected: boolean; + onClick: () => void; +} + +const WorktreeRow: React.FC = ({ label, sublabel, isSelected, onClick }) => ( + +); diff --git a/packages/review-editor/dock/ReviewStateContext.tsx b/packages/review-editor/dock/ReviewStateContext.tsx index 4aa75ba9..14110ae7 100644 --- a/packages/review-editor/dock/ReviewStateContext.tsx +++ b/packages/review-editor/dock/ReviewStateContext.tsx @@ -5,6 +5,7 @@ import type { DiffFile } from '../types'; import type { AIChatEntry } from '../hooks/useAIChat'; import type { ReviewSearchMatch } from '../utils/reviewSearch'; import type { PRMetadata, PRContext } from '@plannotator/shared/pr-provider'; +import type { FeedbackDiffContext } from '../utils/exportFeedback'; /** * Shared review state consumed by dockview panel wrappers. @@ -26,6 +27,15 @@ export interface ReviewState { disableBackground?: boolean; fontFamily?: string; fontSize?: string; + /** User-selected base branch; feeds the `base` query param on file-content fetches. */ + reviewBase?: string; + /** Active diff mode (e.g. "branch", "merge-base", "uncommitted"). Used as + * part of the DiffViewer remount key so mode switches invalidate cached + * file content — branch and merge-base compute different "old" sides. */ + activeDiffBase?: string; + /** Diff context baked into exported feedback so downstream panels (agent job + * detail, etc.) produce the same markdown the main feedback path sends. */ + feedbackDiffContext?: FeedbackDiffContext; // Annotations allAnnotations: CodeAnnotation[]; diff --git a/packages/review-editor/dock/panels/ReviewAgentJobDetailPanel.tsx b/packages/review-editor/dock/panels/ReviewAgentJobDetailPanel.tsx index b0045f69..c775b689 100644 --- a/packages/review-editor/dock/panels/ReviewAgentJobDetailPanel.tsx +++ b/packages/review-editor/dock/panels/ReviewAgentJobDetailPanel.tsx @@ -87,9 +87,21 @@ export const ReviewAgentJobDetailPanel: React.FC = (props) state.onSelectAnnotation(ann.id); }, [state.openDiffFile, state.onSelectAnnotation]); + // Copy All uses the diff context snapshotted on the JOB at launch, not the + // current UI state — so if the reviewer switches modes/bases after the job + // ran, the exported markdown still describes the diff the agent actually + // analyzed. Falls back to current UI state only if the job predates the + // snapshotting (older jobs without diffContext). const copyAllText = useMemo( - () => activeAnnotations.length > 0 ? exportReviewFeedback(activeAnnotations, state.prMetadata) : '', - [activeAnnotations, state.prMetadata] + () => + activeAnnotations.length > 0 + ? exportReviewFeedback( + activeAnnotations, + state.prMetadata, + job?.diffContext ?? state.feedbackDiffContext, + ) + : '', + [activeAnnotations, state.prMetadata, job?.diffContext, state.feedbackDiffContext], ); const { jobLogs } = useJobLogs(); diff --git a/packages/review-editor/dock/panels/ReviewDiffPanel.tsx b/packages/review-editor/dock/panels/ReviewDiffPanel.tsx index 75817db5..a5458b66 100644 --- a/packages/review-editor/dock/panels/ReviewDiffPanel.tsx +++ b/packages/review-editor/dock/panels/ReviewDiffPanel.tsx @@ -51,12 +51,17 @@ export const ReviewDiffPanel: React.FC = (props) => { ); } + // Keying on reviewBase forces a remount when the user picks a new base. + // Otherwise the file-content fetch for the new base can land before the new + // patch, and Pierre briefly reconciles old-patch + new-content → "trailing + // context mismatch" warnings in the console. return ( -
+
{ expect(result).not.toContain("PR Review"); }); + it("local mode with diff context: describes mode + base in the header", () => { + const result = exportReviewFeedback([ann()], undefined, { + mode: "branch", + base: "develop", + }); + expect(result).toContain("**Diff:** Branch diff vs `develop`"); + }); + + it("local mode with merge-base: labels PR Diff with the base", () => { + const result = exportReviewFeedback([ann()], undefined, { + mode: "merge-base", + base: "release/v2", + }); + expect(result).toContain("**Diff:** PR Diff vs `release/v2`"); + }); + + it("local mode with worktree path: appends worktree info", () => { + const result = exportReviewFeedback([ann()], undefined, { + mode: "uncommitted", + worktreePath: "/tmp/feature-wt", + }); + expect(result).toContain("**Diff:** Uncommitted changes _(worktree: /tmp/feature-wt)_"); + }); + + it("PR mode ignores diff context (PR header already carries branches)", () => { + const result = exportReviewFeedback([ann()], prMeta, { + mode: "branch", + base: "develop", + }); + // The PR-style branches line must appear. + expect(result).toContain("Branch: `fix/widget` → `main`"); + // The local-mode Diff line must not. + expect(result).not.toContain("**Diff:**"); + }); + it("PR mode: includes all PR context fields", () => { const result = exportReviewFeedback([ann()], prMeta); expect(result).toStartWith("# PR Review: acme/widgets#42\n\n"); diff --git a/packages/review-editor/utils/exportFeedback.ts b/packages/review-editor/utils/exportFeedback.ts index 7e298626..ed94df41 100644 --- a/packages/review-editor/utils/exportFeedback.ts +++ b/packages/review-editor/utils/exportFeedback.ts @@ -16,15 +16,47 @@ export function formatConventionalPrefix( return `**${label}${decs}:** `; } +/** + * Describes what the reviewer was looking at in local-review mode — diff mode, + * optional base branch, optional worktree. Threaded into the feedback header so + * the receiving agent knows which diff the annotations are anchored to. Ignored + * in PR mode, where `prMeta` already carries equivalent context. + */ +export interface FeedbackDiffContext { + mode: string; + base?: string; + worktreePath?: string | null; +} + +function describeDiff(ctx: FeedbackDiffContext): string { + const { mode, base, worktreePath } = ctx; + let label: string; + switch (mode) { + case "uncommitted": label = "Uncommitted changes"; break; + case "staged": label = "Staged changes"; break; + case "unstaged": label = "Unstaged changes"; break; + case "last-commit": label = "Last commit"; break; + case "branch": label = base ? `Branch diff vs \`${base}\`` : "Branch diff"; break; + case "merge-base": label = base ? `PR Diff vs \`${base}\`` : "PR Diff"; break; + default: label = mode; // p4-* or anything else — show raw + } + return worktreePath ? `${label} _(worktree: ${worktreePath})_` : label; +} + /** * Build markdown feedback from code review annotations. * * In PR mode (prMeta provided), the header includes repo, PR number, * title, branches, and URL so the receiving agent has full context. + * + * In local mode, an optional diffContext adds one line describing which + * diff the reviewer was looking at — otherwise the agent only sees file + * paths and line numbers and has to guess which diff those anchor to. */ export function exportReviewFeedback( annotations: CodeAnnotation[], prMeta?: PRMetadata | null, + diffContext?: FeedbackDiffContext, ): string { if (annotations.length === 0) { return '# Code Review\n\nNo feedback provided.'; @@ -42,7 +74,7 @@ export function exportReviewFeedback( `**${prMeta.title}**\n` + `Branch: \`${prMeta.headBranch}\` → \`${prMeta.baseBranch}\`\n` + `${prMeta.url}\n\n` - : '# Code Review Feedback\n\n'; + : `# Code Review Feedback\n\n${diffContext ? `**Diff:** ${describeDiff(diffContext)}\n\n` : ''}`; for (const [filePath, fileAnnotations] of grouped) { output += `## ${filePath}\n\n`; diff --git a/packages/server/agent-jobs.ts b/packages/server/agent-jobs.ts index 584491e0..c4917c7d 100644 --- a/packages/server/agent-jobs.ts +++ b/packages/server/agent-jobs.ts @@ -81,6 +81,8 @@ export interface AgentJobHandlerOptions { reasoningEffort?: string; /** Whether Codex fast mode was enabled. */ fastMode?: boolean; + /** Diff context snapshot at launch (stored on AgentJobInfo for per-job "Copy All"). */ + diffContext?: AgentJobInfo["diffContext"]; } | null>; /** * Called after a job process exits with exit code 0. @@ -130,7 +132,7 @@ export function createAgentJobHandler(options: AgentJobHandlerOptions): AgentJob command: string[], label: string, outputPath?: string, - spawnOptions?: { captureStdout?: boolean; stdinPrompt?: string; cwd?: string; prompt?: string; engine?: string; model?: string; effort?: string; reasoningEffort?: string; fastMode?: boolean }, + spawnOptions?: { captureStdout?: boolean; stdinPrompt?: string; cwd?: string; prompt?: string; engine?: string; model?: string; effort?: string; reasoningEffort?: string; fastMode?: boolean; diffContext?: AgentJobInfo["diffContext"] }, ): AgentJobInfo { const id = crypto.randomUUID(); const source = jobSource(id); @@ -149,6 +151,7 @@ export function createAgentJobHandler(options: AgentJobHandlerOptions): AgentJob ...(spawnOptions?.effort && { effort: spawnOptions.effort }), ...(spawnOptions?.reasoningEffort && { reasoningEffort: spawnOptions.reasoningEffort }), ...(spawnOptions?.fastMode && { fastMode: spawnOptions.fastMode }), + ...(spawnOptions?.diffContext && { diffContext: spawnOptions.diffContext }), }; let proc: ReturnType | null = null; @@ -445,6 +448,7 @@ export function createAgentJobHandler(options: AgentJobHandlerOptions): AgentJob let jobEffort: string | undefined; let jobReasoningEffort: string | undefined; let jobFastMode: boolean | undefined; + let jobDiffContext: AgentJobInfo["diffContext"] | undefined; if (options.buildCommand) { // Thread config from POST body to buildCommand const config: Record = {}; @@ -467,6 +471,7 @@ export function createAgentJobHandler(options: AgentJobHandlerOptions): AgentJob jobEffort = built.effort; jobReasoningEffort = built.reasoningEffort; jobFastMode = built.fastMode; + jobDiffContext = built.diffContext; } } @@ -487,6 +492,7 @@ export function createAgentJobHandler(options: AgentJobHandlerOptions): AgentJob effort: jobEffort, reasoningEffort: jobReasoningEffort, fastMode: jobFastMode, + diffContext: jobDiffContext, }); return Response.json({ job }, { status: 201 }); } catch { diff --git a/packages/server/codex-review.ts b/packages/server/codex-review.ts index 559f044c..abef52f6 100644 --- a/packages/server/codex-review.ts +++ b/packages/server/codex-review.ts @@ -206,6 +206,11 @@ export function buildCodexReviewUserMessage( return `Review the code changes against the base branch '${base}'. Run \`git diff ${base}..HEAD\` to inspect the changes. Provide prioritized, actionable findings.`; } + case "merge-base": { + const base = options?.defaultBranch || "main"; + return `Review the PR-style diff against base '${base}'. First find the common ancestor with \`git merge-base ${base} HEAD\`, then run \`git diff ..HEAD\` using that commit to inspect only the changes introduced on this branch (matches GitHub's PR view). Provide prioritized, actionable findings.`; + } + default: // p4 or unknown — fall back to generic with inlined diff return [ diff --git a/packages/server/p4.ts b/packages/server/p4.ts index 6b84a371..c9a0b199 100644 --- a/packages/server/p4.ts +++ b/packages/server/p4.ts @@ -125,6 +125,7 @@ export async function getP4Context(cwd?: string): Promise { defaultBranch: "", diffOptions: [], worktrees: [], + availableBranches: { local: [], remote: [] }, cwd, vcsType: "p4", }; @@ -174,6 +175,7 @@ export async function getP4Context(cwd?: string): Promise { defaultBranch: "", diffOptions, worktrees: [], + availableBranches: { local: [], remote: [] }, cwd: cwd ?? workspace.clientRoot, vcsType: "p4", }; diff --git a/packages/server/review.ts b/packages/server/review.ts index 2b9cbb04..f14306c0 100644 --- a/packages/server/review.ts +++ b/packages/server/review.ts @@ -11,7 +11,10 @@ import { isRemoteSession, getServerHostname, getServerPort } from "./remote"; import type { Origin } from "@plannotator/shared/agents"; -import { type DiffType, type GitContext, runVcsDiff, getVcsFileContentsForDiff, canStageFiles, stageFile, unstageFile, resolveVcsCwd, validateFilePath } from "./vcs"; +import { type DiffType, type GitContext, runVcsDiff, getVcsFileContentsForDiff, canStageFiles, stageFile, unstageFile, resolveVcsCwd, validateFilePath, getVcsContext } from "./vcs"; +import { parseWorktreeDiffType } from "@plannotator/shared/review-core"; +import type { AgentJobInfo } from "@plannotator/shared/agent-jobs"; +import { resolveBaseBranch } from "@plannotator/shared/review-core"; import { getRepoInfo } from "./repo"; import { handleImage, handleUpload, handleAgents, handleServerReady, handleDraftSave, handleDraftLoad, handleDraftDelete, handleFavicon, type OpencodeClient } from "./shared-handlers"; import { contentHash, deleteDraft } from "./draft"; @@ -62,6 +65,14 @@ export interface ReviewServerOptions { diffType?: DiffType; /** Git context with branch info and available diff options */ gitContext?: GitContext; + /** + * Initial base branch the caller used to compute `rawPatch`. When a caller + * overrides the detected default (e.g. Pi's `openCodeReview` accepting a + * custom `defaultBranch`), this must be forwarded so the server's internal + * `currentBase` state, the `/api/diff` response, and downstream agent + * prompts stay consistent with the patch that's already on screen. + */ + initialBase?: string; /** Whether URL sharing is enabled (default: true) */ sharingEnabled?: boolean; /** Custom base URL for share links (default: https://share.plannotator.ai) */ @@ -128,6 +139,11 @@ export async function startReviewServer( let currentGitRef = options.gitRef; let currentDiffType: DiffType = options.diffType || "uncommitted"; let currentError = options.error; + // Tracks the base branch the user picked from the UI. Agent review prompts + // read this (not gitContext.defaultBranch) so they analyze the same diff + // the reviewer is currently looking at. Honors an explicit initialBase from + // the caller — e.g. programmatic Pi callers can request a non-detected base. + let currentBase = options.initialBase || gitContext?.defaultBranch || "main"; // Agent jobs — background process manager (late-binds serverUrl via getter) let serverUrl = ""; @@ -141,10 +157,25 @@ export async function startReviewServer( async buildCommand(provider, config) { const cwd = resolveAgentCwd(); const hasAgentLocalAccess = !!options.agentCwd || !!gitContext; - const userMessageOptions = { defaultBranch: gitContext?.defaultBranch, hasLocalAccess: hasAgentLocalAccess }; + const userMessageOptions = { defaultBranch: currentBase, hasLocalAccess: hasAgentLocalAccess }; + + // Snapshot the diff context at launch — stored on the job so + // downstream "Copy All" produces the same markdown as /api/feedback + // would right now, even if the reviewer switches modes/bases later. + // Skipped in PR mode (prMetadata carries equivalent context). + const worktreeParts = currentDiffType.startsWith("worktree:") + ? parseWorktreeDiffType(currentDiffType) + : null; + const diffContext: AgentJobInfo["diffContext"] | undefined = prMetadata + ? undefined + : { + mode: (worktreeParts?.subType ?? currentDiffType) as string, + base: currentBase, + worktreePath: worktreeParts?.path ?? null, + }; if (provider === "tour") { - return tour.buildCommand({ + const built = await tour.buildCommand({ cwd, patch: currentPatch, diffType: currentDiffType, @@ -152,6 +183,7 @@ export async function startReviewServer( prMetadata, config, }); + return built ? { ...built, diffContext } : built; } const userMessage = buildCodexReviewUserMessage(currentPatch, currentDiffType, userMessageOptions, prMetadata); @@ -163,7 +195,7 @@ export async function startReviewServer( const outputPath = generateOutputPath(); const prompt = CODEX_REVIEW_SYSTEM_PROMPT + "\n\n---\n\n" + userMessage; const command = await buildCodexCommand({ cwd, outputPath, prompt, model, reasoningEffort, fastMode }); - return { command, outputPath, prompt, label: "Code Review", model, reasoningEffort, fastMode: fastMode || undefined }; + return { command, outputPath, prompt, label: "Code Review", model, reasoningEffort, fastMode: fastMode || undefined, diffContext }; } if (provider === "claude") { @@ -171,7 +203,7 @@ export async function startReviewServer( const effort = typeof config?.effort === "string" && config.effort ? config.effort : undefined; const prompt = CLAUDE_REVIEW_PROMPT + "\n\n---\n\n" + userMessage; const { command, stdinPrompt } = buildClaudeCommand(prompt, model, effort); - return { command, stdinPrompt, prompt, cwd, label: "Code Review", captureStdout: true, model, effort }; + return { command, stdinPrompt, prompt, cwd, label: "Code Review", captureStdout: true, model, effort, diffContext }; } return null; @@ -412,6 +444,10 @@ export async function startReviewServer( gitRef: currentGitRef, origin, diffType: hasLocalAccess ? currentDiffType : undefined, + // Echo the active base so a page refresh or reconnect rehydrates + // the picker to what the server is actually using — not the + // detected default. + base: hasLocalAccess ? currentBase : undefined, gitContext: hasLocalAccess ? gitContext : undefined, sharingEnabled, shareBaseUrl, @@ -434,7 +470,7 @@ export async function startReviewServer( ); } try { - const body = (await req.json()) as { diffType: DiffType }; + const body = (await req.json()) as { diffType: DiffType; base?: string }; let newDiffType = body.diffType; if (!newDiffType) { @@ -444,22 +480,49 @@ export async function startReviewServer( ); } - const defaultBranch = gitContext?.defaultBranch || "main"; + const detectedBase = gitContext?.defaultBranch || "main"; + // Guard against non-string payloads — resolveBaseBranch calls + // string methods and would throw a TypeError otherwise. Mirrors + // Pi's guard so both runtimes validate identically. + const requestedBase = typeof body.base === "string" ? body.base : undefined; + const base = resolveBaseBranch(requestedBase, detectedBase); const defaultCwd = gitContext?.cwd; // Run the new diff - const result = await runVcsDiff(newDiffType, defaultBranch, defaultCwd); + const result = await runVcsDiff(newDiffType, base, defaultCwd); // Update state currentPatch = result.patch; currentGitRef = result.label; currentDiffType = newDiffType; + currentBase = base; currentError = result.error; + // Recompute gitContext for the effective cwd so the client's + // sidebar (current branch, default branch, diff-mode options) + // reflects the worktree we're now reviewing — not the main + // repo's startup state. Best-effort: on failure the client + // keeps its existing context. + let updatedContext: GitContext | undefined; + if (gitContext) { + try { + const effectiveCwd = resolveVcsCwd(newDiffType, gitContext.cwd); + updatedContext = await getVcsContext(effectiveCwd); + } catch { + /* best-effort */ + } + } + return Response.json({ rawPatch: currentPatch, gitRef: currentGitRef, diffType: currentDiffType, + // Echo the base the server actually used. resolveBaseBranch + // trusts the caller verbatim; this echo lets the client + // confirm the request landed (and pick it up when the client + // didn't supply one and we fell back to detected default). + base: currentBase, + ...(updatedContext && { gitContext: updatedContext }), ...(currentError && { error: currentError }), }); } catch (err) { @@ -505,11 +568,15 @@ export async function startReviewServer( // Local review: read file contents from local git if (hasLocalAccess) { - const defaultBranch = gitContext?.defaultBranch || "main"; + const detectedBase = gitContext?.defaultBranch || "main"; + const base = resolveBaseBranch( + url.searchParams.get("base") ?? undefined, + detectedBase, + ); const defaultCwd = gitContext?.cwd; const result = await getVcsFileContentsForDiff( currentDiffType, - defaultBranch, + base, filePath, oldPath, defaultCwd, diff --git a/packages/server/tour/tour-review.ts b/packages/server/tour/tour-review.ts index bb71e12b..36d45128 100644 --- a/packages/server/tour/tour-review.ts +++ b/packages/server/tour/tour-review.ts @@ -322,6 +322,10 @@ function buildTourUserMessage( const base = options?.defaultBranch || "main"; return `Walk the reviewer through the code changes against the base branch '${base}' as a guided tour. Run \`git diff ${base}..HEAD\` to inspect the changes.`; } + case "merge-base": { + const base = options?.defaultBranch || "main"; + return `Walk the reviewer through the PR-style diff against base '${base}' as a guided tour. First find the common ancestor with \`git merge-base ${base} HEAD\`, then run \`git diff ..HEAD\` using that commit to inspect only the changes introduced on this branch (matches GitHub's PR view).`; + } default: return [ "Walk the reviewer through the following code changes as a guided tour.", diff --git a/packages/shared/agent-jobs.ts b/packages/shared/agent-jobs.ts index 20817e70..84283e83 100644 --- a/packages/shared/agent-jobs.ts +++ b/packages/shared/agent-jobs.ts @@ -14,6 +14,20 @@ export type AgentJobStatus = "starting" | "running" | "done" | "failed" | "killed"; +/** + * Snapshot of the diff the reviewer was looking at when this job was launched. + * Carried on the job so downstream UIs (agent-result panel "Copy All") export + * the same `**Diff:** ...` header the job was actually run against — if the + * reviewer switches the UI to a different diff afterwards, the job's snapshot + * still reflects truth. Structurally compatible with the UI-side + * `FeedbackDiffContext` in `packages/review-editor/utils/exportFeedback.ts`. + */ +export interface AgentJobDiffContext { + mode: string; + base?: string; + worktreePath?: string | null; +} + export interface AgentJobInfo { /** Unique job identifier (UUID). */ id: string; @@ -55,6 +69,8 @@ export interface AgentJobInfo { explanation: string; confidence: number; }; + /** Diff context at launch time (see AgentJobDiffContext). */ + diffContext?: AgentJobDiffContext; } export interface AgentCapability { diff --git a/packages/shared/review-core.test.ts b/packages/shared/review-core.test.ts index f08927e7..082552e1 100644 --- a/packages/shared/review-core.test.ts +++ b/packages/shared/review-core.test.ts @@ -4,9 +4,12 @@ import { mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from "nod import { tmpdir } from "node:os"; import { join, resolve as resolvePath } from "node:path"; import { + getDefaultBranch, getFileContentsForDiff, getGitContext, + parseWorktreeDiffType, runGitDiff, + type DiffType, type ReviewGitRuntime, } from "./review-core"; @@ -159,7 +162,11 @@ describe("review-core", () => { expect(result.patch).toBe(""); expect(result.label).toBe("Error: branch"); - expect(result.error).toContain("git diff --no-ext-diff master..HEAD"); + // Error is derived from the argv — assert the meaningful parts rather + // than the exact string so harmless argv reorders (e.g. --end-of-options) + // don't break it. + expect(result.error).toContain("git diff"); + expect(result.error).toContain("master..HEAD"); }); test("git context lists worktrees and file content lookup returns old/new content", async () => { @@ -199,4 +206,41 @@ describe("review-core", () => { expect(newFileContents.oldContent).toBeNull(); expect(newFileContents.newContent).toBe("brand new\n"); }); + + test("getDefaultBranch falls back to local when origin/HEAD points at an unfetched ref", () => { + // Simulates a narrow / partial clone where origin/HEAD is configured but + // the target ref was never fetched. Before the verify step, the server + // would return "origin/phantom" and every branch/merge-base diff would + // fail with "unknown revision". With the verify step we fall back to + // local main. + const repoDir = initRepo(); + + // Manually set origin/HEAD → origin/phantom without ever fetching it. + git(repoDir, ["symbolic-ref", "refs/remotes/origin/HEAD", "refs/remotes/origin/phantom"]); + + const runtime = makeRuntime(repoDir); + return getDefaultBranch(runtime).then((result) => { + expect(result).toBe("main"); + }); + }); + + test("parseWorktreeDiffType recognises every DiffType suffix, including merge-base", () => { + // Regression guard: every local diff type must round-trip through the + // worktree-prefixed form. Missing `merge-base` here previously routed + // "worktree:/path:merge-base" to { path: "/path:merge-base", subType: "uncommitted" } + // which pointed git at a non-existent cwd and silently collapsed the diff mode. + const subTypes = [ + "uncommitted", + "staged", + "unstaged", + "last-commit", + "branch", + "merge-base", + ] as const; + for (const sub of subTypes) { + const composite = `worktree:/tmp/my-worktree:${sub}` as DiffType; + const parsed = parseWorktreeDiffType(composite); + expect(parsed).toEqual({ path: "/tmp/my-worktree", subType: sub }); + } + }); }); diff --git a/packages/shared/review-core.ts b/packages/shared/review-core.ts index 1a719513..c9b4c3a5 100644 --- a/packages/shared/review-core.ts +++ b/packages/shared/review-core.ts @@ -29,11 +29,17 @@ export interface WorktreeInfo { head: string; } +export interface AvailableBranches { + local: string[]; + remote: string[]; +} + export interface GitContext { currentBranch: string; defaultBranch: string; diffOptions: DiffOption[]; worktrees: WorktreeInfo[]; + availableBranches: AvailableBranches; cwd?: string; vcsType?: "git" | "p4"; } @@ -73,13 +79,27 @@ export async function getDefaultBranch( runtime: ReviewGitRuntime, cwd?: string, ): Promise { + // Prefer the remote tracking ref (e.g. `origin/main`) so diffs run against + // the upstream tip, not a potentially stale local copy. Only fall back to + // a local ref when there's no remote configured at all. const remoteHead = await runtime.runGit( ["symbolic-ref", "refs/remotes/origin/HEAD"], { cwd }, ); if (remoteHead.exitCode === 0) { const ref = remoteHead.stdout.trim(); - if (ref) return ref.replace("refs/remotes/origin/", ""); + if (ref) { + // `symbolic-ref` only tells us what origin/HEAD *points at* — it does + // not guarantee that the target ref was actually fetched. In narrow + // or partial clones the pointer can be set while the target is + // missing, in which case a later `git diff origin/main..HEAD` would + // error. Verify the target exists before trusting it. + const verify = await runtime.runGit( + ["show-ref", "--verify", "--quiet", ref], + { cwd }, + ); + if (verify.exitCode === 0) return ref.replace("refs/remotes/", ""); + } } const mainBranch = await runtime.runGit( @@ -91,6 +111,66 @@ export async function getDefaultBranch( return "master"; } +export async function listBranches( + runtime: ReviewGitRuntime, + cwd?: string, +): Promise { + // Emit `\t` so we can classify by ref prefix + // without guessing from the short form — local branches can contain `/` + // (e.g. `feature/foo`), so `name.includes("/")` would misclassify them. + const result = await runtime.runGit( + [ + "for-each-ref", + "--format=%(refname)\t%(refname:short)", + "refs/heads", + "refs/remotes", + ], + { cwd }, + ); + if (result.exitCode !== 0) return { local: [], remote: [] }; + + const local: string[] = []; + const remote: string[] = []; + + for (const line of result.stdout.split("\n")) { + const [fullRef, shortName] = line.split("\t"); + if (!fullRef || !shortName) continue; + if (shortName.endsWith("/HEAD")) continue; + if (fullRef.startsWith("refs/heads/")) { + local.push(shortName); + } else if (fullRef.startsWith("refs/remotes/")) { + remote.push(shortName); + } + } + + // Keep both local and remote refs — they can point to different commits + // (stale local tracking branches are common) and users need to be able to + // pick either explicitly. The picker groups them separately for clarity. + local.sort(); + remote.sort(); + + return { local, remote }; +} + +/** + * Pick a safe base branch. Trusts the caller verbatim if they supplied one, + * otherwise falls back to the detected default. Shared by Bun (`review.ts`) + * and Pi (`serverReview.ts`) so both runtimes behave identically. + * + * Why trust the caller: the UI picker only ever sends refs from the known + * list, and external/programmatic callers may pass tags, SHAs, or refs under + * non-`origin` remotes that we must not silently rewrite (a tag `release` is + * not the same commit as a branch `origin/release`). Invalid refs surface as + * git errors on the next diff call, which is better than silently producing + * a patch against the wrong commit. + */ +export function resolveBaseBranch( + requested: string | undefined, + detected: string, +): string { + return requested || detected; +} + export async function getWorktrees( runtime: ReviewGitRuntime, cwd?: string, @@ -137,9 +217,10 @@ export async function getGitContext( runtime: ReviewGitRuntime, cwd?: string, ): Promise { - const [currentBranch, defaultBranch] = await Promise.all([ + const [currentBranch, defaultBranch, availableBranches] = await Promise.all([ getCurrentBranch(runtime, cwd), getDefaultBranch(runtime, cwd), + listBranches(runtime, cwd), ]); const diffOptions: DiffOption[] = [ @@ -149,7 +230,16 @@ export async function getGitContext( { id: "last-commit", label: "Last commit" }, ]; - if (currentBranch !== defaultBranch) { + // Always offer Branch diff / PR Diff when a default branch exists. The + // older guard hid them when the reviewer was on the default branch (the + // `vs ` diff from the default branch itself is always empty), but + // the base picker now lets reviewers compare against any branch from any + // branch, so there's no meaningless-by-construction option. Also: preserving + // diff mode across worktree switches and Pi's `initialBase` can land the + // reviewer on the default branch with branch/merge-base already active — the + // old guard hid the active mode's option, trapping them. Unconditional + // emission keeps the active option reachable in every flow. + if (defaultBranch) { diffOptions.push({ id: "branch", label: `vs ${defaultBranch}` }); diffOptions.push({ id: "merge-base", label: `Current PR Diff` }); } @@ -169,6 +259,7 @@ export async function getGitContext( defaultBranch, diffOptions, worktrees: worktrees.filter((wt) => wt.path !== currentTreePath), + availableBranches, cwd, }; } @@ -245,6 +336,7 @@ const WORKTREE_SUB_TYPES = new Set([ "unstaged", "last-commit", "branch", + "merge-base", ]); export function parseWorktreeDiffType( @@ -371,12 +463,17 @@ export async function runGitDiff( } case "branch": { + // `--end-of-options` hardens against a caller-supplied `defaultBranch` + // that starts with `-` being parsed as a git flag (e.g. `--output=...` + // would redirect diff output to an attacker-chosen path). Same pattern + // applied wherever user-controlled refs flow into a git argv. const branchDiffArgs = [ "diff", "--no-ext-diff", - `${defaultBranch}..HEAD`, "--src-prefix=a/", "--dst-prefix=b/", + "--end-of-options", + `${defaultBranch}..HEAD`, ]; const branchDiff = assertGitSuccess( await runtime.runGit(branchDiffArgs, { cwd }), @@ -388,17 +485,19 @@ export async function runGitDiff( } case "merge-base": { + const mergeBaseLookupArgs = ["merge-base", "--end-of-options", defaultBranch, "HEAD"]; const mergeBaseResult = assertGitSuccess( - await runtime.runGit(["merge-base", defaultBranch, "HEAD"], { cwd }), - ["merge-base", defaultBranch, "HEAD"], + await runtime.runGit(mergeBaseLookupArgs, { cwd }), + mergeBaseLookupArgs, ); const mergeBase = mergeBaseResult.stdout.trim(); const mergeBaseDiffArgs = [ "diff", "--no-ext-diff", - `${mergeBase}..HEAD`, "--src-prefix=a/", "--dst-prefix=b/", + "--end-of-options", + `${mergeBase}..HEAD`, ]; const mergeBaseDiff = assertGitSuccess( await runtime.runGit(mergeBaseDiffArgs, { cwd }), @@ -463,7 +562,8 @@ export async function getFileContentsForDiff( } async function gitShow(ref: string, path: string): Promise { - const result = await runtime.runGit(["show", `${ref}:${path}`], { cwd }); + // `--end-of-options` hardens against user-supplied refs starting with `-`. + const result = await runtime.runGit(["show", "--end-of-options", `${ref}:${path}`], { cwd }); return result.exitCode === 0 ? result.stdout : null; } @@ -499,7 +599,7 @@ export async function getFileContentsForDiff( newContent: await gitShow("HEAD", filePath), }; case "merge-base": { - const mbResult = await runtime.runGit(["merge-base", defaultBranch, "HEAD"], { cwd }); + const mbResult = await runtime.runGit(["merge-base", "--end-of-options", defaultBranch, "HEAD"], { cwd }); const mb = mbResult.exitCode === 0 ? mbResult.stdout.trim() : defaultBranch; return { oldContent: await gitShow(mb, oldFilePath), diff --git a/packages/shared/types.ts b/packages/shared/types.ts index d12a50d0..e6f24164 100644 --- a/packages/shared/types.ts +++ b/packages/shared/types.ts @@ -14,4 +14,5 @@ export type { DiffOption, WorktreeInfo, GitContext, + AvailableBranches, } from "./review-core"; diff --git a/packages/ui/components/AgentsTab.tsx b/packages/ui/components/AgentsTab.tsx index d82c2c7e..1f9c1cd5 100644 --- a/packages/ui/components/AgentsTab.tsx +++ b/packages/ui/components/AgentsTab.tsx @@ -21,6 +21,7 @@ const CLAUDE_EFFORT: Array<{ value: string; label: string }> = [ ]; const CODEX_MODELS: Array<{ value: string; label: string }> = [ + { value: 'gpt-5.5', label: 'GPT-5.5' }, { value: 'gpt-5.4', label: 'GPT-5.4' }, { value: 'gpt-5.3-codex', label: 'GPT-5.3 Codex' }, { value: 'gpt-5.3-codex-spark', label: 'GPT-5.3 Codex Spark' }, diff --git a/packages/ui/components/Tooltip.tsx b/packages/ui/components/Tooltip.tsx index 782fbf2b..89d28bb6 100644 --- a/packages/ui/components/Tooltip.tsx +++ b/packages/ui/components/Tooltip.tsx @@ -10,6 +10,12 @@ interface TooltipProps { align?: 'start' | 'center' | 'end'; delayDuration?: number; sideOffset?: number; + /** + * When true, allow the tooltip to wrap onto multiple lines with a reasonable + * max width. Default is single-line (nowrap) — matches the original callsites + * that use it for short button labels. + */ + wide?: boolean; } export const Tooltip: React.FC = ({ @@ -19,6 +25,7 @@ export const Tooltip: React.FC = ({ align = 'center', delayDuration, sideOffset = 8, + wide = false, }) => ( {children} @@ -27,7 +34,9 @@ export const Tooltip: React.FC = ({ side={side} align={align} sideOffset={sideOffset} - className="z-50 px-2 py-1 text-xs bg-popover text-popover-foreground border border-border rounded shadow-md whitespace-nowrap origin-[var(--radix-tooltip-content-transform-origin)] transition-[opacity,transform] duration-150 ease-out data-[state=closed]:opacity-0 data-[state=closed]:scale-95 data-[state=delayed-open]:opacity-100 data-[state=delayed-open]:scale-100 data-[state=instant-open]:opacity-100 data-[state=instant-open]:scale-100" + className={`z-50 px-2 py-1 text-xs bg-popover text-popover-foreground border border-border rounded shadow-md origin-[var(--radix-tooltip-content-transform-origin)] transition-[opacity,transform] duration-150 ease-out data-[state=closed]:opacity-0 data-[state=closed]:scale-95 data-[state=delayed-open]:opacity-100 data-[state=delayed-open]:scale-100 data-[state=instant-open]:opacity-100 data-[state=instant-open]:scale-100 ${ + wide ? 'max-w-[260px] leading-snug whitespace-normal' : 'whitespace-nowrap' + }`} > {content} diff --git a/packages/ui/package.json b/packages/ui/package.json index c994e8ba..da9a201e 100644 --- a/packages/ui/package.json +++ b/packages/ui/package.json @@ -17,6 +17,7 @@ "@plannotator/shared": "workspace:*", "@plannotator/web-highlighter": "^0.8.1", "@radix-ui/react-dialog": "^1.1.15", + "@radix-ui/react-popover": "^1.1.15", "@radix-ui/react-tooltip": "^1.2.8", "@tanstack/react-table": "^8.21.3", "@viz-js/viz": "^3.25.0",