feat(review): add --no-worktree flag for in-place PR checkout#992
Open
TovRudyy wants to merge 2 commits into
Open
feat(review): add --no-worktree flag for in-place PR checkout#992TovRudyy wants to merge 2 commits into
TovRudyy wants to merge 2 commits into
Conversation
`plannotator review <PR_URL>` 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) <noreply@anthropic.com>
…s test 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/<N>/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) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
plannotator review <PR_URL>defaults to--local, which builds a second working tree withgit worktree add --detach. On large repositories that is slow and disk-heavy (it materializes a full second copy of the tree).The existing escape hatch,
--no-local, avoids the worktree but disables all local inspection — it reviews from the platform-API diff only, so full-stack diffs, context expansion, code navigation, and AI review agents are all unavailable.--no-worktreefills the gap.What
--no-worktreedoesFor a same-repo PR it reuses the current repository instead of a worktree:
origin/<base>resolves for full-stack diffs), then fetches +git checkouts the PR head in place (via the existingcheckoutPRHeadhelper).agentCwdwith no worktree pool.SIGINT/SIGTERMhandlers route throughprocess.exit(), which fires the restore).Because
agentCwdis set and there's no pool, the server's existingresolvePRLocalCwd/ensurePRLocalCwdfallbacks resolve to the current repo — so full-stack diff, file-content expansion, and code-nav keep working, just without the cost ofgit worktree add.Behavior / precedence
--no-local+--no-worktree:--no-localwins (no local checkout at all).fetchonly touches refs, and a failedcheckoutaborts atomically, so nothing leaks.Runtime scope
The in-place behavior is implemented in the Claude Code runtime (
apps/hook/server/index.ts). Per CLAUDE.md's "both runtimes must be updated" rule, the flag is not silently swallowed elsewhere: OpenCode and Pi reject--no-worktreewith an error rather than accepting-and-ignoring it. This is documented in the CLI help and the shared parser JSDoc. (Neither of those runtimes has an entry-level worktree path to mirror; full parity can be a follow-up.)Changes
packages/shared/review-args.ts— parse--no-worktreeintonoWorktree.apps/hook/server/index.ts— in-place checkout branch (dirty-tree guard + save/restore ref + exit hook). The existing--localworktree block is untouched (now anelse if).apps/opencode-plugin/commands.ts,apps/pi-extension/index.ts— reject--no-worktreewith an error.apps/hook/server/cli.ts— document the flag and its runtime scope.packages/shared/review-args.test.ts— parsing,--gitcombo,--no-local/--no-worktreeprecedence.packages/shared/no-worktree-checkout.test.ts— new git-mechanics integration test.No changes were needed in
packages/server/review.ts— theagentCwdfallbacks already cover the pool-less path.Testing
bun test packages/shared/review-args.test.ts— parser/precedence (10 tests).bun test packages/shared/no-worktree-checkout.test.ts— new: a real-git offline fixture (a bare origin carryingrefs/pull/<N>/head) proving the in-place mechanics: HEAD moves to the PR head with no worktree added, the original ref restores, a dirty tree is detected, and an unreachable ref fails without mutating HEAD (4 tests, stable across repeated runs).bun test packages/shared/worktree-pool.test.ts— no regressions (18 tests).tsc --noEmitonpackages/shared,packages/server,packages/ai, andapps/pi-extension— 0 errors.Live verification
Ran
plannotator review --no-worktree <this PR>in a clone whoseoriginisbacknotprop/plannotator: confirmed the logChecked out PR head in … (no worktree),git worktree listshowed no new worktree, HEAD moved to the PR head, and the branch was restored on session exit.