diff --git a/apps/hook/server/cli.ts b/apps/hook/server/cli.ts index 49591fda2..53fe6f067 100644 --- a/apps/hook/server/cli.ts +++ b/apps/hook/server/cli.ts @@ -32,7 +32,7 @@ export function formatTopLevelHelp(): string { " plannotator --help", " plannotator --version, -v", " plannotator [--browser ]", - " plannotator review [--git] [PR_URL]", + " plannotator review [--git] [--local | --no-local | --no-worktree] [PR_URL]", " plannotator annotate [--markdown] [--no-jina] [--gate] [--json] [--hook]", " plannotator annotate-last [--stdin] [--gate] [--json] [--hook]", " plannotator setup-goal [--json]", @@ -57,20 +57,25 @@ export function formatTopLevelHelp(): string { const SUBCOMMAND_HELP: Record = { review: [ "Usage:", - " plannotator review [--git] [--local | --no-local] [PR_URL]", + " plannotator review [--git] [--local | --no-local | --no-worktree] [PR_URL]", "", "Review local VCS changes (git/jj) or a GitHub/GitLab pull request in the browser.", "", "Options:", - " --git Force git as the VCS (skip auto-detection)", - " --local For PR review, prepare a local checkout for full file access (default)", - " --no-local For PR review, skip the local checkout (diff only)", - " PR_URL GitHub PR or GitLab MR URL to review", + " --git Force git as the VCS (skip auto-detection)", + " --local For PR review, prepare a local checkout for full file access (default)", + " --no-local For PR review, skip the local checkout (diff only)", + " --no-worktree For PR review, check the PR out in the current repo instead of a", + " separate worktree (same-repo only; needs a clean working tree; your", + " branch is restored on exit). Cheaper than a worktree on large repos.", + " Claude Code runtime only (OpenCode and Pi reject this flag).", + " PR_URL GitHub PR or GitLab MR URL to review", "", "Examples:", " plannotator review", " plannotator review --git", " plannotator review https://github.com/owner/repo/pull/123", + " plannotator review --no-worktree https://github.com/owner/repo/pull/123", ].join("\n"), annotate: [ "Usage:", diff --git a/apps/hook/server/index.ts b/apps/hook/server/index.ts index 98f0c47a0..65ed942ef 100644 --- a/apps/hook/server/index.ts +++ b/apps/hook/server/index.ts @@ -100,6 +100,7 @@ import { resolveMarkdownFile, resolveUserPath, hasMarkdownFiles } from "@plannot import { FILE_BROWSER_EXCLUDED } from "@plannotator/shared/reference-common"; import { statSync, rmSync, realpathSync, existsSync } from "fs"; import { parseRemoteUrl } from "@plannotator/shared/repo"; +import { checkoutPRHead } from "@plannotator/shared/pr-stack"; import { getReviewApprovedPrompt, getReviewDeniedSuffix, @@ -534,6 +535,7 @@ if (args[0] === "sessions") { const urlArg = reviewArgs.prUrl; const isPRMode = urlArg !== undefined; const useLocal = isPRMode && reviewArgs.useLocal; + const inPlace = useLocal && reviewArgs.noWorktree === true; let rawPatch: string; let gitRef: string; @@ -586,13 +588,90 @@ if (args[0] === "sessions") { process.exit(1); } + // --no-worktree: reuse the CURRENT repo for the PR checkout instead of + // adding a git worktree. `git worktree add` materializes a second full + // working tree, which is slow and disk-heavy on large repos; checking the + // PR head out in place avoids that. Same-repo only, requires a clean working + // tree, and the original branch/HEAD is restored when the session exits. + // With agentCwd set and no pool, the review server's resolvePRLocalCwd / + // ensurePRLocalCwd fallbacks use this repo, so full-stack diff, file + // expansion, and code-nav all keep working. + if (inPlace && prMetadata) { + try { + const repoDir = process.cwd(); + 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}`); + + let isSameRepo = false; + try { + const remoteResult = await gitRuntime.runGit(["remote", "get-url", "origin"], { cwd: repoDir }); + if (remoteResult.exitCode === 0) { + const remoteUrl = remoteResult.stdout.trim(); + const currentRepo = parseRemoteUrl(remoteUrl); + const prRepo = prMetadata.platform === "github" + ? `${prMetadata.owner}/${prMetadata.repo}` + : prMetadata.projectPath; + const repoMatches = !!currentRepo && currentRepo.toLowerCase() === prRepo.toLowerCase(); + const sshHost = remoteUrl.match(/^[^@]+@([^:]+):/)?.[1]; + const httpsHost = (() => { try { return new URL(remoteUrl).hostname; } catch { return null; } })(); + const remoteHost = (sshHost || httpsHost || "").toLowerCase(); + const prHost = prMetadata.host.toLowerCase(); + isSameRepo = repoMatches && remoteHost === prHost; + } + } catch { /* not in a git repo */ } + + if (!isSameRepo) { + console.error("Warning: --no-worktree only applies to a PR from the current repository; reviewing from the platform diff only."); + } else { + // Guard: never clobber uncommitted work. + const statusRes = await gitRuntime.runGit(["status", "--porcelain"], { cwd: repoDir }); + if (statusRes.exitCode !== 0) throw new Error(`git status failed: ${statusRes.stderr.trim()}`); + if (statusRes.stdout.trim() !== "") { + throw new Error("working tree has uncommitted changes — commit or stash them, or drop --no-worktree"); + } + + const branchRes = await gitRuntime.runGit(["symbolic-ref", "--quiet", "--short", "HEAD"], { cwd: repoDir }); + const origRef = branchRes.exitCode === 0 + ? branchRes.stdout.trim() + : (await gitRuntime.runGit(["rev-parse", "HEAD"], { cwd: repoDir })).stdout.trim(); + if (!origRef) throw new Error("could not determine the current git ref to restore later"); + + const baseFetchRes = await gitRuntime.runGit(["fetch", "origin", "--", prMetadata.baseBranch], { cwd: repoDir }); + if (baseFetchRes.exitCode !== 0) throw new Error(`git fetch origin ${prMetadata.baseBranch} failed: ${baseFetchRes.stderr.trim()}`); + const catRes = await gitRuntime.runGit(["cat-file", "-t", prMetadata.baseSha], { cwd: repoDir }); + if (catRes.exitCode !== 0) await gitRuntime.runGit(["fetch", "origin", "--", prMetadata.baseSha], { cwd: repoDir }); + + const checkedOut = await checkoutPRHead(gitRuntime, prMetadata, repoDir); + if (!checkedOut) throw new Error("failed to fetch/checkout the PR head into the current repository"); + + agentCwd = repoDir; + + let restored = false; + const restore = () => { + if (restored) return; + restored = true; + try { Bun.spawnSync(["git", "checkout", origRef], { cwd: repoDir }); } catch {} + }; + worktreeCleanup = restore; + process.once("exit", restore); + + console.error(`Checked out PR head in ${repoDir} (no worktree); '${origRef}' will be restored on exit.`); + } + } catch (err) { + console.error("Warning: --no-worktree failed, falling back to remote diff"); + console.error(err instanceof Error ? err.message : String(err)); + agentCwd = undefined; + worktreePool = undefined; + worktreeCleanup = undefined; + } + } // --local: create a local checkout with the PR head for full file access. // The checkout is built in the BACKGROUND — the platform diff is already // in hand, so the review server starts immediately. The pool entry starts // ready:false and flips to ready when the warmup completes; consumers that // need real files (agent jobs, full-stack diff, code-nav) await it via // pool.ensure(). - if (useLocal && prMetadata) { + else if (useLocal && prMetadata) { // Hoisted so catch block can clean up partially-created directories let localPath: string | undefined; let sessionDir: string | undefined; diff --git a/apps/opencode-plugin/commands.ts b/apps/opencode-plugin/commands.ts index 3ca36cd63..ea836d2fe 100644 --- a/apps/opencode-plugin/commands.ts +++ b/apps/opencode-plugin/commands.ts @@ -59,6 +59,12 @@ export async function handleReviewCommand( const urlArg = reviewArgs.prUrl; const isPRMode = urlArg !== undefined; + // --no-worktree (in-place PR checkout) is implemented only in the Claude Code runtime. + if (reviewArgs.noWorktree) { + client.app.log({ level: "error", message: "--no-worktree is only supported in the Claude Code runtime, not OpenCode." }); + return; + } + let rawPatch: string; let gitRef: string; let diffError: string | undefined; diff --git a/apps/pi-extension/index.ts b/apps/pi-extension/index.ts index 36dbcdef0..aa986ac8d 100644 --- a/apps/pi-extension/index.ts +++ b/apps/pi-extension/index.ts @@ -434,6 +434,11 @@ export default function plannotator(pi: ExtensionAPI): void { try { const reviewArgs = parseReviewArgs(args ?? ""); + // --no-worktree (in-place PR checkout) is implemented only in the Claude Code runtime. + if (reviewArgs.noWorktree) { + ctx.ui.notify("Plannotator: --no-worktree is only supported in the Claude Code runtime, not Pi.", "error"); + return; + } const session = await startCodeReviewBrowserSession(ctx, { prUrl: reviewArgs.prUrl, vcsType: reviewArgs.vcsType, diff --git a/packages/shared/no-worktree-checkout.test.ts b/packages/shared/no-worktree-checkout.test.ts new file mode 100644 index 000000000..0f3d65a12 --- /dev/null +++ b/packages/shared/no-worktree-checkout.test.ts @@ -0,0 +1,136 @@ +import { afterEach, beforeEach, describe, expect, test } from "bun:test"; +import { mkdtempSync, rmSync } from "fs"; +import { tmpdir } from "os"; +import path from "path"; +import type { ReviewGitRuntime } from "./review-core"; +import type { PRMetadata } from "./pr-types"; +import { checkoutPRHead } from "./pr-stack"; + +// Exercises the git mechanics behind `plannotator review --no-worktree` — the +// in-place PR checkout that skips `git worktree add`. Uses REAL git against a +// local bare "origin" that carries a GitHub-style `refs/pull//head` ref, so +// checkoutPRHead's `git fetch origin refs/pull//head` works fully offline. +// This proves the checkout/restore/guard mechanics; the CLI wiring (inPlace +// gate, isSameRepo detection, agentCwd handoff) is covered by the live/manual +// layer documented in the PR. +const gitRuntime: ReviewGitRuntime = { + async runGit(args, options) { + const proc = Bun.spawn(["git", "-c", "core.quotePath=false", ...args], { + cwd: options?.cwd, + stdout: "pipe", + stderr: "pipe", + env: { ...process.env, GIT_TERMINAL_PROMPT: "0" }, + }); + const [stdout, stderr, exitCode] = await Promise.all([ + new Response(proc.stdout).text(), + new Response(proc.stderr).text(), + proc.exited, + ]); + return { stdout, stderr, exitCode }; + }, + async readTextFile() { + return null; + }, +}; + +const git = (args: string[], cwd: string) => gitRuntime.runGit(args, { cwd }); +const out = async (args: string[], cwd: string) => (await git(args, cwd)).stdout.trim(); + +// The fixture drives real git (init/commit/clone) — generous timeouts so a cold +// run (slow first-time disk / AV scan on macOS/CI) doesn't flake on the 5s default. +const SETUP_TIMEOUT_MS = 60_000; +const TEST_TIMEOUT_MS = 30_000; + +let dir: string; +let clone: string; +let BASE_SHA: string; +let HEAD_SHA: string; + +beforeEach(async () => { + dir = mkdtempSync(path.join(tmpdir(), "pln-nowt-")); + const originBare = path.join(dir, "origin.git"); + const seed = path.join(dir, "seed"); + clone = path.join(dir, "clone"); + + const cfg = async (repo: string) => { + await git(["config", "user.email", "t@t"], repo); + await git(["config", "user.name", "t"], repo); + }; + + await git(["init", "--bare", "-b", "main", originBare], dir); + await git(["init", "-b", "main", seed], dir); + await cfg(seed); + + await Bun.write(path.join(seed, "f.txt"), "base\n"); + await git(["add", "."], seed); + await git(["commit", "-m", "base"], seed); + BASE_SHA = await out(["rev-parse", "HEAD"], seed); + + await git(["checkout", "-b", "pr-head"], seed); + await Bun.write(path.join(seed, "f.txt"), "base\npr change\n"); + await git(["add", "."], seed); + await git(["commit", "-m", "pr work"], seed); + HEAD_SHA = await out(["rev-parse", "HEAD"], seed); + + await git(["remote", "add", "origin", originBare], seed); + await git(["push", "origin", "main"], seed); + // The GitHub-style pull ref that checkoutPRHead fetches. + await git(["push", "origin", "pr-head:refs/pull/992/head"], seed); + + await git(["clone", originBare, clone], dir); + await cfg(clone); +}, SETUP_TIMEOUT_MS); + +afterEach(() => { + rmSync(dir, { recursive: true, force: true }); +}); + +const meta = (over: Partial = {}): PRMetadata => ({ + platform: "github", + host: "github.com", + owner: "acme", + repo: "widgets", + number: 992, + title: "t", + author: "a", + baseBranch: "main", + headBranch: "pr-head", + baseSha: BASE_SHA, + headSha: HEAD_SHA, + url: "https://github.com/acme/widgets/pull/992", + ...over, +}) as PRMetadata; + +describe("no-worktree in-place checkout — git mechanics", () => { + test("moves HEAD to the PR head without adding a worktree", async () => { + const before = (await out(["worktree", "list"], clone)).split("\n").length; + expect(await checkoutPRHead(gitRuntime, meta(), clone)).toBe(true); + expect(await out(["rev-parse", "HEAD"], clone)).toBe(HEAD_SHA); + expect((await out(["worktree", "list"], clone)).split("\n").length).toBe(before); + }, TEST_TIMEOUT_MS); + + test("captured original ref can be restored after checkout", async () => { + const origRef = await out(["symbolic-ref", "--quiet", "--short", "HEAD"], clone); + expect(origRef).toBe("main"); + + await checkoutPRHead(gitRuntime, meta(), clone); + expect(await out(["rev-parse", "HEAD"], clone)).toBe(HEAD_SHA); + + // What the restore closure does on session exit. + await git(["checkout", origRef], clone); + expect(await out(["symbolic-ref", "--quiet", "--short", "HEAD"], clone)).toBe("main"); + expect(await out(["rev-parse", "HEAD"], clone)).toBe(BASE_SHA); + }, TEST_TIMEOUT_MS); + + test("dirty working tree is detectable via status --porcelain", async () => { + await Bun.write(path.join(clone, "f.txt"), "dirty\n"); + expect(await out(["status", "--porcelain"], clone)).not.toBe(""); + // Guard leaves HEAD untouched. + expect(await out(["symbolic-ref", "--quiet", "--short", "HEAD"], clone)).toBe("main"); + }, TEST_TIMEOUT_MS); + + test("unreachable PR ref → checkoutPRHead returns false, HEAD unchanged", async () => { + expect(await checkoutPRHead(gitRuntime, meta({ number: 999999 } as Partial), clone)).toBe(false); + expect(await out(["rev-parse", "HEAD"], clone)).toBe(BASE_SHA); + }, TEST_TIMEOUT_MS); +}); diff --git a/packages/shared/review-args.test.ts b/packages/shared/review-args.test.ts index 6bf57ee42..3814bd8f5 100644 --- a/packages/shared/review-args.test.ts +++ b/packages/shared/review-args.test.ts @@ -7,6 +7,7 @@ describe("parseReviewArgs", () => { prUrl: undefined, vcsType: undefined, useLocal: true, + noWorktree: false, }); }); @@ -15,6 +16,7 @@ describe("parseReviewArgs", () => { prUrl: undefined, vcsType: "git", useLocal: true, + noWorktree: false, }); }); @@ -23,11 +25,13 @@ describe("parseReviewArgs", () => { prUrl: "https://github.com/acme/repo/pull/12", vcsType: "git", useLocal: true, + noWorktree: false, }); expect(parseReviewArgs("https://github.com/acme/repo/pull/12 --git")).toEqual({ prUrl: "https://github.com/acme/repo/pull/12", vcsType: "git", useLocal: true, + noWorktree: false, }); }); @@ -36,6 +40,34 @@ describe("parseReviewArgs", () => { prUrl: "https://github.com/acme/repo/pull/12", vcsType: undefined, useLocal: false, + noWorktree: false, + }); + }); + + test("parses --no-worktree for PR review mode", () => { + expect(parseReviewArgs("--no-worktree https://github.com/acme/repo/pull/12")).toEqual({ + prUrl: "https://github.com/acme/repo/pull/12", + vcsType: undefined, + useLocal: true, + noWorktree: true, + }); + }); + + test("combines --no-worktree with --git and a PR URL", () => { + expect(parseReviewArgs(["--git", "--no-worktree", "https://github.com/acme/repo/pull/12"])).toEqual({ + prUrl: "https://github.com/acme/repo/pull/12", + vcsType: "git", + useLocal: true, + noWorktree: true, + }); + }); + + test("parses --no-local and --no-worktree together (precedence resolved by the caller)", () => { + expect(parseReviewArgs("--no-local --no-worktree https://github.com/acme/repo/pull/12")).toEqual({ + prUrl: "https://github.com/acme/repo/pull/12", + vcsType: undefined, + useLocal: false, + noWorktree: true, }); }); @@ -44,6 +76,7 @@ describe("parseReviewArgs", () => { prUrl: "https://github.com/acme/repo/pull/12", vcsType: "git", useLocal: false, + noWorktree: false, }); }); @@ -59,6 +92,7 @@ describe("parseReviewArgs", () => { prUrl: undefined, vcsType: "git", useLocal: true, + noWorktree: false, }); }); }); diff --git a/packages/shared/review-args.ts b/packages/shared/review-args.ts index c6bc759e7..af9a5458a 100644 --- a/packages/shared/review-args.ts +++ b/packages/shared/review-args.ts @@ -5,6 +5,15 @@ export interface ParsedReviewArgs { prUrl?: string; vcsType?: VcsSelection; useLocal: boolean; + /** + * PR review only. When true, reuse the current repository for the local + * checkout (fetch + `git checkout` the PR head in place) instead of adding a + * separate git worktree. Avoids the cost of `git worktree add` on huge repos. + * Ignored when useLocal is false (--no-local wins) or for cross-repo PRs. + * Only the Claude Code runtime acts on this; OpenCode and Pi reject the flag + * with an error. + */ + noWorktree: boolean; } export function parseReviewArgs(input: string | string[]): ParsedReviewArgs { @@ -14,6 +23,7 @@ export function parseReviewArgs(input: string | string[]): ParsedReviewArgs { let vcsType: VcsSelection | undefined; let useLocal = true; + let noWorktree = false; const positional: string[] = []; for (const token of tokens) { @@ -27,6 +37,9 @@ export function parseReviewArgs(input: string | string[]): ParsedReviewArgs { case "--no-local": useLocal = false; break; + case "--no-worktree": + noWorktree = true; + break; default: positional.push(token); break; @@ -38,6 +51,7 @@ export function parseReviewArgs(input: string | string[]): ParsedReviewArgs { prUrl: target && isReviewUrl(target) ? target : undefined, vcsType, useLocal, + noWorktree, }; }