From 88e69013778231dad8951f34ca4459632c06bd74 Mon Sep 17 00:00:00 2001 From: "olek.rudyy" Date: Fri, 3 Jul 2026 15:02:56 +0200 Subject: [PATCH 1/2] feat(review): add --no-worktree flag for in-place PR checkout MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `plannotator review ` defaults to `--local`, which creates a second working tree via `git worktree add`. On huge repos that is slow and disk-heavy. `--no-local` avoids it but disables all local inspection (platform-API diff only — no full-stack diff, context expansion, code-nav, or agents). `--no-worktree` fills the gap: for a same-repo PR it reuses the current repository by fetching and checking the PR head out in place, then hands that repo to the review server as agentCwd (no worktree pool). The existing resolvePRLocalCwd/ensurePRLocalCwd fallbacks already use agentCwd, so full-stack diff, file expansion, and code navigation keep working — without the worktree. Because an in-place checkout mutates the working tree it is guarded: it refuses to run on a dirty tree, remembers the original branch/HEAD, and restores it when the session exits (graceful close and process exit). Cross-repo PRs (or running outside a git repo) warn and fall back to the platform diff; `--no-local` takes precedence over `--no-worktree`. - packages/shared/review-args.ts: parse --no-worktree into noWorktree - apps/hook/server/index.ts: in-place checkout branch + guard + restore-on-exit - apps/hook/server/cli.ts: document the flag - packages/shared/review-args.test.ts: cover parsing + precedence Co-Authored-By: Claude Opus 4.8 (1M context) --- apps/hook/server/cli.ts | 16 +++-- apps/hook/server/index.ts | 96 ++++++++++++++++++++++++++++- packages/shared/review-args.test.ts | 34 ++++++++++ packages/shared/review-args.ts | 12 ++++ 4 files changed, 151 insertions(+), 7 deletions(-) diff --git a/apps/hook/server/cli.ts b/apps/hook/server/cli.ts index 49591fda2..5cb6b638e 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,24 @@ 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.", + " 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..a687f246f 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,9 @@ if (args[0] === "sessions") { const urlArg = reviewArgs.prUrl; const isPRMode = urlArg !== undefined; const useLocal = isPRMode && reviewArgs.useLocal; + // --no-worktree: reuse the current repo in place instead of a worktree. + // Only meaningful alongside a local checkout, so --no-local takes precedence. + const inPlace = useLocal && reviewArgs.noWorktree === true; let rawPatch: string; let gitRef: string; @@ -586,13 +590,103 @@ 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(); + // Validate inputs from platform API to prevent git flag/path injection + if (prMetadata.baseBranch.includes('..') || prMetadata.baseBranch.startsWith('-')) throw new Error(`Invalid base branch: ${prMetadata.baseBranch}`); + if (!/^[0-9a-f]{40,64}$/i.test(prMetadata.baseSha)) throw new Error(`Invalid base SHA: ${prMetadata.baseSha}`); + + // Detect same-repo vs cross-repo (must match both owner/repo AND host). + // Checking a PR out in place only makes sense when this repo IS the PR's repo. + 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) { + // Can't check another repo's PR out in place — review from the + // platform (remote) diff only, exactly like --no-local. + 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"); + } + + // Remember the branch (or detached SHA) so it can be restored on exit. + 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"); + + // Fetch base refs first (best-effort) so origin/ resolves for + // full-stack diffs, THEN fetch + checkout the PR head. The head fetch + // must be last because checkoutPRHead checks out FETCH_HEAD. + 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"); + + // The current repo now holds the PR head — hand it to the server as + // the agent working dir. No worktree pool is created. + agentCwd = repoDir; + + // Restore the user's original ref on exit — both a graceful session + // close (onCleanup) and a hard process exit (Ctrl-C). + 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/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..7e8d56bd5 100644 --- a/packages/shared/review-args.ts +++ b/packages/shared/review-args.ts @@ -5,6 +5,13 @@ 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. + */ + noWorktree: boolean; } export function parseReviewArgs(input: string | string[]): ParsedReviewArgs { @@ -14,6 +21,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 +35,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 +49,7 @@ export function parseReviewArgs(input: string | string[]): ParsedReviewArgs { prUrl: target && isReviewUrl(target) ? target : undefined, vcsType, useLocal, + noWorktree, }; } From 51793b8e2d648e0cd2ce26ee3840b6ef45807a05 Mon Sep 17 00:00:00 2001 From: "olek.rudyy" Date: Fri, 3 Jul 2026 16:06:01 +0200 Subject: [PATCH 2/2] address review; reject --no-worktree in OpenCode/Pi; add git-mechanics test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review feedback: trim the line-by-line narration comments in the in-place block (kept the top doc comment and the two terse inline notes). Runtime scope: --no-worktree is implemented only in the Claude Code runtime. Instead of silently accepting-and-ignoring the flag, OpenCode and Pi now reject it with an error (per CLAUDE.md's "both runtimes must be updated"), and the limitation is documented in the CLI help and the shared parser JSDoc. Testing: add packages/shared/no-worktree-checkout.test.ts — a real-git offline fixture (a bare origin carrying refs/pull//head) proving the in-place mechanics: HEAD moves to the PR head with no worktree added, the original ref restores, a dirty tree is detectable, and an unreachable ref fails without mutating HEAD. Generous timeouts avoid cold-run flakiness. Co-Authored-By: Claude Opus 4.8 (1M context) --- apps/hook/server/cli.ts | 1 + apps/hook/server/index.ts | 15 -- apps/opencode-plugin/commands.ts | 6 + apps/pi-extension/index.ts | 5 + packages/shared/no-worktree-checkout.test.ts | 136 +++++++++++++++++++ packages/shared/review-args.ts | 2 + 6 files changed, 150 insertions(+), 15 deletions(-) create mode 100644 packages/shared/no-worktree-checkout.test.ts diff --git a/apps/hook/server/cli.ts b/apps/hook/server/cli.ts index 5cb6b638e..53fe6f067 100644 --- a/apps/hook/server/cli.ts +++ b/apps/hook/server/cli.ts @@ -68,6 +68,7 @@ const SUBCOMMAND_HELP: Record = { " --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:", diff --git a/apps/hook/server/index.ts b/apps/hook/server/index.ts index a687f246f..65ed942ef 100644 --- a/apps/hook/server/index.ts +++ b/apps/hook/server/index.ts @@ -535,8 +535,6 @@ if (args[0] === "sessions") { const urlArg = reviewArgs.prUrl; const isPRMode = urlArg !== undefined; const useLocal = isPRMode && reviewArgs.useLocal; - // --no-worktree: reuse the current repo in place instead of a worktree. - // Only meaningful alongside a local checkout, so --no-local takes precedence. const inPlace = useLocal && reviewArgs.noWorktree === true; let rawPatch: string; @@ -601,12 +599,9 @@ if (args[0] === "sessions") { if (inPlace && prMetadata) { try { const repoDir = process.cwd(); - // Validate inputs from platform API to prevent git flag/path injection if (prMetadata.baseBranch.includes('..') || prMetadata.baseBranch.startsWith('-')) throw new Error(`Invalid base branch: ${prMetadata.baseBranch}`); if (!/^[0-9a-f]{40,64}$/i.test(prMetadata.baseSha)) throw new Error(`Invalid base SHA: ${prMetadata.baseSha}`); - // Detect same-repo vs cross-repo (must match both owner/repo AND host). - // Checking a PR out in place only makes sense when this repo IS the PR's repo. let isSameRepo = false; try { const remoteResult = await gitRuntime.runGit(["remote", "get-url", "origin"], { cwd: repoDir }); @@ -626,8 +621,6 @@ if (args[0] === "sessions") { } catch { /* not in a git repo */ } if (!isSameRepo) { - // Can't check another repo's PR out in place — review from the - // platform (remote) diff only, exactly like --no-local. 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. @@ -637,16 +630,12 @@ if (args[0] === "sessions") { throw new Error("working tree has uncommitted changes — commit or stash them, or drop --no-worktree"); } - // Remember the branch (or detached SHA) so it can be restored on exit. 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"); - // Fetch base refs first (best-effort) so origin/ resolves for - // full-stack diffs, THEN fetch + checkout the PR head. The head fetch - // must be last because checkoutPRHead checks out FETCH_HEAD. 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 }); @@ -655,12 +644,8 @@ if (args[0] === "sessions") { const checkedOut = await checkoutPRHead(gitRuntime, prMetadata, repoDir); if (!checkedOut) throw new Error("failed to fetch/checkout the PR head into the current repository"); - // The current repo now holds the PR head — hand it to the server as - // the agent working dir. No worktree pool is created. agentCwd = repoDir; - // Restore the user's original ref on exit — both a graceful session - // close (onCleanup) and a hard process exit (Ctrl-C). let restored = false; const restore = () => { if (restored) return; 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.ts b/packages/shared/review-args.ts index 7e8d56bd5..af9a5458a 100644 --- a/packages/shared/review-args.ts +++ b/packages/shared/review-args.ts @@ -10,6 +10,8 @@ export interface ParsedReviewArgs { * 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; }