Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 11 additions & 6 deletions apps/hook/server/cli.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ export function formatTopLevelHelp(): string {
" plannotator --help",
" plannotator --version, -v",
" plannotator [--browser <name>]",
" plannotator review [--git] [PR_URL]",
" plannotator review [--git] [--local | --no-local | --no-worktree] [PR_URL]",
" plannotator annotate <file.md | file.txt | file.html | https://... | folder/> [--markdown] [--no-jina] [--gate] [--json] [--hook]",
" plannotator annotate-last [--stdin] [--gate] [--json] [--hook]",
" plannotator setup-goal <interview|facts> <bundle.json | -> [--json]",
Expand All @@ -57,20 +57,25 @@ export function formatTopLevelHelp(): string {
const SUBCOMMAND_HELP: Record<string, string> = {
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:",
Expand Down
81 changes: 80 additions & 1 deletion apps/hook/server/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down
6 changes: 6 additions & 0 deletions apps/opencode-plugin/commands.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
5 changes: 5 additions & 0 deletions apps/pi-extension/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
136 changes: 136 additions & 0 deletions packages/shared/no-worktree-checkout.test.ts
Original file line number Diff line number Diff line change
@@ -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/<N>/head` ref, so
// checkoutPRHead's `git fetch origin refs/pull/<N>/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> = {}): 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<PRMetadata>), clone)).toBe(false);
expect(await out(["rev-parse", "HEAD"], clone)).toBe(BASE_SHA);
}, TEST_TIMEOUT_MS);
});
34 changes: 34 additions & 0 deletions packages/shared/review-args.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ describe("parseReviewArgs", () => {
prUrl: undefined,
vcsType: undefined,
useLocal: true,
noWorktree: false,
});
});

Expand All @@ -15,6 +16,7 @@ describe("parseReviewArgs", () => {
prUrl: undefined,
vcsType: "git",
useLocal: true,
noWorktree: false,
});
});

Expand All @@ -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,
});
});

Expand All @@ -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,
});
});

Expand All @@ -44,6 +76,7 @@ describe("parseReviewArgs", () => {
prUrl: "https://github.com/acme/repo/pull/12",
vcsType: "git",
useLocal: false,
noWorktree: false,
});
});

Expand All @@ -59,6 +92,7 @@ describe("parseReviewArgs", () => {
prUrl: undefined,
vcsType: "git",
useLocal: true,
noWorktree: false,
});
});
});
Loading