/** * Code Review Extension (inspired by Codex's review feature) * * Provides a `/review` command that prompts the agent to review code changes. * Supports multiple review modes: * - Review a GitHub pull request (checks out the PR locally) * - Review against a base branch (PR style) * - Review uncommitted changes * - Review a specific commit * - Shared custom review instructions (applied to all review modes when configured) * * Usage: * - `/review` - show interactive selector * - `/review pr 123` - review PR #123 (checks out locally) * - `/review pr https://github.com/owner/repo/pull/123` - review PR from URL * - `/review uncommitted` - review uncommitted changes directly * - `/review branch main` - review against main branch * - `/review commit abc123` - review specific commit * - `/review folder src docs` - review specific folders/files (snapshot, not diff) * - `/review` selector includes Add/Remove custom review instructions (applies to all modes) * - `/review --extra "focus on performance regressions"` - add extra review instruction (works with any mode) * * Project-specific review guidelines: * - If a REVIEW_GUIDELINES.md file exists in the same directory as .pi, * its contents are appended to the review prompt. * * Note: PR review requires a clean working tree (no uncommitted changes to tracked files). * * Provenance: this file is a copy of the upstream Pi review extension, * https://github.com/earendil-works/pi-review (MIT), bundled here so goodies * ships /review without a second install. Upstream owns the design, prompts, * and review rubric — before changing review behavior, check upstream for * fixes and new features and port them instead of letting this copy drift. * * Local deltas on top of upstream f1de050 (2026-07-19; upstream main's tip as * of 2026-09-12) — re-apply these when re-syncing: * - prettier-formatted (2-space) and wired to this package's describeError() * - parsePrReference(): exported, strictly numeric bare refs, anchored PR-URL * regex, and the URL's owner/repo forwarded to gh via --repo * - tokenizeArgs(): hoisted to module scope and exported for tests * - quote-aware path parsing: parseArgs() + parseReviewPaths() hoisted to * module scope (exported for tests); the folder flow no longer joins tokens * back into a string (join+re-split destroyed quoted paths), and the folder * editor tokenizes its raw text with tokenizeArgs() instead of splitting on * whitespace * - navigateWithSummary(): aborting the summary loader detaches the * navigation instead of pretending it stopped — a result that completes * after the abort is discarded and the review state is cleared, so a stale * {active: true} entry can't resurrect the review widget (pi exposes no * cancellation for an in-flight navigateTree summarization) */ import type { ExtensionAPI, ExtensionContext, ExtensionCommandContext, } from "@earendil-works/pi-coding-agent"; import { DynamicBorder, BorderedLoader } from "@earendil-works/pi-coding-agent"; import { Container, fuzzyFilter, Input, type SelectItem, SelectList, Spacer, Text, type AutocompleteItem, type AutocompleteProvider, } from "@earendil-works/pi-tui"; import path from "node:path"; import { promises as fs } from "node:fs"; import { describeError } from "./json-file.ts"; // State to track fresh session review (where we branched from). // Module-level state means only one review can be active at a time. // This is intentional - the UI and /end-review command assume a single active review. let reviewOriginId: string | undefined = undefined; let endReviewInProgress = false; let reviewCustomInstructions: string | undefined = undefined; const REVIEW_STATE_TYPE = "review-session"; const REVIEW_ANCHOR_TYPE = "review-anchor"; const REVIEW_SETTINGS_TYPE = "review-settings"; const GH_SETUP_INSTRUCTIONS = "Install GitHub CLI (`gh`) from https://cli.github.com/ (macOS: `brew install gh`), then sign in with `gh auth login` and verify with `gh auth status`."; const PR_CHECKOUT_BLOCKED_BY_PENDING_CHANGES_MESSAGE = "Cannot checkout PR: you have uncommitted changes. Please commit or stash them first."; type ReviewSessionState = { active: boolean; originId?: string; }; type ReviewSettingsState = { customInstructions?: string; }; function setReviewWidget(ctx: ExtensionContext, active: boolean) { if (!ctx.hasUI) return; if (!active) { ctx.ui.setWidget("review", undefined); return; } ctx.ui.setWidget("review", (_tui, theme) => { const message = "Review session active, return with /end-review"; const text = new Text(theme.fg("warning", message), 0, 0); return { render(width: number) { return text.render(width); }, invalidate() { text.invalidate(); }, }; }); } function getReviewState(ctx: ExtensionContext): ReviewSessionState | undefined { let state: ReviewSessionState | undefined; for (const entry of ctx.sessionManager.getBranch()) { if (entry.type === "custom" && entry.customType === REVIEW_STATE_TYPE) { state = entry.data as ReviewSessionState | undefined; } } return state; } function applyReviewState(ctx: ExtensionContext) { const state = getReviewState(ctx); if (state?.active && state.originId) { reviewOriginId = state.originId; setReviewWidget(ctx, true); return; } reviewOriginId = undefined; setReviewWidget(ctx, false); } function getReviewSettings(ctx: ExtensionContext): ReviewSettingsState { let state: ReviewSettingsState | undefined; for (const entry of ctx.sessionManager.getEntries()) { if (entry.type === "custom" && entry.customType === REVIEW_SETTINGS_TYPE) { state = entry.data as ReviewSettingsState | undefined; } } return { customInstructions: state?.customInstructions?.trim() || undefined, }; } function applyReviewSettings(ctx: ExtensionContext) { const state = getReviewSettings(ctx); reviewCustomInstructions = state.customInstructions?.trim() || undefined; } // Review target types (matching Codex's approach) type ReviewTarget = | { type: "uncommitted" } | { type: "baseBranch"; branch: string } | { type: "commit"; sha: string; title?: string } | { type: "pullRequest"; prNumber: number; baseBranch: string; title: string } | { type: "folder"; paths: string[] }; // Prompts (adapted from Codex) const UNCOMMITTED_PROMPT = "Review the current code changes (staged, unstaged, and untracked files) and provide prioritized findings."; const BASE_BRANCH_PROMPT_WITH_MERGE_BASE = "Review the code changes against the base branch '{baseBranch}'. The merge base commit for this comparison is {mergeBaseSha}. Run `git diff {mergeBaseSha}` to inspect the changes relative to {baseBranch}. Provide prioritized, actionable findings."; const BASE_BRANCH_PROMPT_FALLBACK = 'Review the code changes against the base branch \'{branch}\'. Start by finding the merge diff between the current branch and {branch}\'s upstream e.g. (`git merge-base HEAD "$(git rev-parse --abbrev-ref "{branch}@{upstream}")"`), then run `git diff` against that SHA to see what changes we would merge into the {branch} branch. Provide prioritized, actionable findings.'; const COMMIT_PROMPT_WITH_TITLE = 'Review the code changes introduced by commit {sha} ("{title}"). Provide prioritized, actionable findings.'; const COMMIT_PROMPT = "Review the code changes introduced by commit {sha}. Provide prioritized, actionable findings."; const PULL_REQUEST_PROMPT = "Review pull request #{prNumber} (\"{title}\") against the base branch '{baseBranch}'. The merge base commit for this comparison is {mergeBaseSha}. Run `git diff {mergeBaseSha}` to inspect the changes that would be merged. Provide prioritized, actionable findings."; const PULL_REQUEST_PROMPT_FALLBACK = "Review pull request #{prNumber} (\"{title}\") against the base branch '{baseBranch}'. Start by finding the merge base between the current branch and {baseBranch} (e.g., `git merge-base HEAD {baseBranch}`), then run `git diff` against that SHA to see the changes that would be merged. Provide prioritized, actionable findings."; const FOLDER_REVIEW_PROMPT = "Review the code in the following paths: {paths}. This is a snapshot review (not a diff). Read the files directly in these paths and provide prioritized, actionable findings."; // The detailed review rubric (adapted from Codex's review_prompt.md) const REVIEW_RUBRIC = `# Review Guidelines You are acting as a code reviewer for a proposed code change made by another engineer. Below are default guidelines for determining what to flag. These are not the final word — if you encounter more specific guidelines elsewhere (in a developer message, user message, file, or project review guidelines appended below), those override these general instructions. ## Determining what to flag Flag issues that: 1. Meaningfully impact the accuracy, performance, security, or maintainability of the code. 2. Are discrete and actionable (not general issues or multiple combined issues). 3. Don't demand rigor inconsistent with the rest of the codebase. 4. Were introduced in the changes being reviewed (not pre-existing bugs). 5. The author would likely fix if aware of them. 6. Don't rely on unstated assumptions about the codebase or author's intent. 7. Have provable impact on other parts of the code — it is not enough to speculate that a change may disrupt another part, you must identify the parts that are provably affected. 8. Are clearly not intentional changes by the author. 9. Be particularly careful with untrusted user input and follow the specific guidelines to review. 10. Treat silent local error recovery (especially parsing/IO/network fallbacks) as high-signal review candidates unless there is explicit boundary-level justification. 11. Violate the clean-code guidelines below. 12. Introduce error handling that conflicts with the fail-fast guidelines below. ## Clean-code guidelines 1. Check whether each newly added function duplicates existing functionality elsewhere in the codebase. Flag actual duplication and identify the existing implementation. 2. Flag one-off helper functions that add indirection without improving clarity or reuse (for example, \`isRecord\` or \`asString\`). 3. Flag abstractions introduced without a concrete need in the reviewed change, including wrappers created only for hypothetical future use. 4. Flag defensive checks or fallback behavior that mask programming errors, especially when callers already guarantee the relevant invariants. ## Untrusted User Input 1. Be careful with open redirects, they must always be checked to only go to trusted domains (?next_page=...) 2. Always flag SQL that is not parametrized 3. In systems with user supplied URL input, http fetches always need to be protected against access to local resources (intercept DNS resolver!) 4. Escape, don't sanitize if you have the option (eg: HTML escaping) ## Comment guidelines 1. Be clear about why the issue is a problem. 2. Communicate severity appropriately - don't exaggerate. 3. Be brief - at most 1 paragraph. 4. Keep code snippets under 3 lines, wrapped in inline code or code blocks. 5. Use \`\`\`suggestion blocks ONLY for concrete replacement code (minimal lines; no commentary inside the block). Preserve the exact leading whitespace of the replaced lines. 6. Explicitly state scenarios/environments where the issue arises. 7. Use a matter-of-fact tone - helpful AI assistant, not accusatory. 8. Write for quick comprehension without close reading. 9. Avoid excessive flattery or unhelpful phrases like "Great job...". ## Review priorities 1. Surface critical non-blocking human callouts (migrations, dependency churn, auth/permissions, compatibility, destructive operations) at the end. 2. Prefer simple, direct solutions over wrappers or abstractions without clear value. 3. Treat back pressure handling as critical to system stability. 4. Apply system-level thinking; flag changes that increase operational risk or on-call wakeups. 5. Ensure that errors are always checked against codes or stable identifiers, never error messages. ## Fail-fast error handling (strict) When reviewing added or modified error handling, default to fail-fast behavior. 1. Evaluate every new or changed \`try/catch\`: identify what can fail and why local handling is correct at that exact layer. 2. Prefer propagation over local recovery. If the current scope cannot fully recover while preserving correctness, rethrow (optionally with context) instead of returning fallbacks. 3. Flag catch blocks that hide failure signals (e.g. returning \`null\`/\`[]\`/\`false\`, swallowing JSON parse failures, logging-and-continue, or “best effort” silent recovery). 4. JSON parsing/decoding should fail loudly by default. Quiet fallback parsing is only acceptable with an explicit compatibility requirement and clear tested behavior. 5. Boundary handlers (HTTP routes, CLI entrypoints, supervisors) may translate errors, but must not pretend success or silently degrade. 6. If a catch exists only to satisfy lint/style without real handling, treat it as a bug. 7. When uncertain, prefer crashing fast over silent degradation. ## Required human callouts (non-blocking, at the very end) After findings/verdict, you MUST append this final section: ## Human Reviewer Callouts (Non-Blocking) Include only applicable callouts (no yes/no lines): - **This change adds a database migration:** - **This change introduces a new dependency:** - **This change changes a dependency (or the lockfile):** - **This change modifies auth/permission behavior:** - **This change introduces backwards-incompatible public schema/API/contract changes:** - **This change includes irreversible or destructive operations:** - **This change adds or removes feature flags:** (call out re-use of dormant feature flags!) - **This change changes configuration defaults:** Rules for this section: 1. These are informational callouts for the human reviewer, not fix items. 2. Do not include them in Findings unless there is an independent defect. 3. These callouts alone must not change the verdict. 4. Only include callouts that apply to the reviewed change. 5. Keep each emitted callout bold exactly as written. 6. If none apply, write "- (none)". ## Priority levels Tag each finding with a priority level in the title: - [P0] - Drop everything to fix. Blocking release/operations. Only for universal issues that do not depend on assumptions about inputs. - [P1] - Urgent. Should be addressed in the next cycle. - [P2] - Normal. To be fixed eventually. - [P3] - Low. Nice to have. ## Output format Provide your findings in a clear, structured format: 1. List each finding with its priority tag, file location, and explanation. 2. Findings must reference locations that overlap with the actual diff — don't flag pre-existing code. 3. Keep line references as short as possible (avoid ranges over 5-10 lines; pick the most suitable subrange). 4. Provide an overall verdict: "correct" (no blocking issues) or "needs attention" (has blocking issues). 5. Ignore trivial style issues unless they obscure meaning or violate documented standards. 6. Do not generate a full PR fix — only flag issues and optionally provide short suggestion blocks. 7. End with the required "Human Reviewer Callouts (Non-Blocking)" section and all applicable bold callouts (no yes/no). Output all findings the author would fix if they knew about them. If there are no qualifying findings, explicitly state the code looks good. Don't stop at the first finding - list every qualifying issue. Then append the required non-blocking callouts section.`; async function loadProjectReviewGuidelines( cwd: string, ): Promise { let currentDir = path.resolve(cwd); while (true) { const piDir = path.join(currentDir, ".pi"); const guidelinesPath = path.join(currentDir, "REVIEW_GUIDELINES.md"); const piStats = await fs.stat(piDir).catch(() => null); if (piStats?.isDirectory()) { const guidelineStats = await fs.stat(guidelinesPath).catch(() => null); if (guidelineStats?.isFile()) { try { const content = await fs.readFile(guidelinesPath, "utf8"); const trimmed = content.trim(); return trimmed ? trimmed : null; } catch { return null; } } return null; } const parentDir = path.dirname(currentDir); if (parentDir === currentDir) { return null; } currentDir = parentDir; } } /** * Get the merge base between HEAD and a branch */ async function getMergeBase( pi: ExtensionAPI, branch: string, ): Promise { try { // First try to get the upstream tracking branch const { stdout: upstream, code: upstreamCode } = await pi.exec("git", [ "rev-parse", "--abbrev-ref", `${branch}@{upstream}`, ]); if (upstreamCode === 0 && upstream.trim()) { const { stdout: mergeBase, code } = await pi.exec("git", [ "merge-base", "HEAD", upstream.trim(), ]); if (code === 0 && mergeBase.trim()) { return mergeBase.trim(); } } // Fall back to using the branch directly const { stdout: mergeBase, code } = await pi.exec("git", [ "merge-base", "HEAD", branch, ]); if (code === 0 && mergeBase.trim()) { return mergeBase.trim(); } return null; } catch { return null; } } /** * Get list of local branches */ async function getLocalBranches(pi: ExtensionAPI): Promise { const { stdout, code } = await pi.exec("git", [ "branch", "--format=%(refname:short)", ]); if (code !== 0) return []; return stdout .trim() .split("\n") .filter((b) => b.trim()); } /** * Get list of remote-tracking branches (e.g. origin/main, origin/master). * * Filters out the bare remote name (`origin`), the symbolic HEAD refs * (origin/HEAD), and any non-origin remotes — origin is the conventional * upstream and the only one the merge-base fallback path is meaningful for. */ async function getRemoteBranches(pi: ExtensionAPI): Promise { const { stdout, code } = await pi.exec("git", [ "branch", "--remotes", "--format=%(refname:short)", ]); if (code !== 0) return []; return stdout .trim() .split("\n") .map((b) => b.trim()) .filter((b) => b.startsWith("origin/") && !b.endsWith("/HEAD")); } /** * Get list of recent commits */ async function getRecentCommits( pi: ExtensionAPI, limit: number = 10, ): Promise> { const { stdout, code } = await pi.exec("git", [ "log", `--oneline`, `-n`, `${limit}`, ]); if (code !== 0) return []; return stdout .trim() .split("\n") .filter((line) => line.trim()) .map((line) => { const [sha, ...rest] = line.trim().split(" "); return { sha, title: rest.join(" ") }; }); } /** * Check if there are uncommitted changes (staged, unstaged, or untracked) */ async function hasUncommittedChanges(pi: ExtensionAPI): Promise { const { stdout, code } = await pi.exec("git", ["status", "--porcelain"]); return code === 0 && stdout.trim().length > 0; } /** * Check if there are changes that would prevent switching branches * (staged or unstaged changes to tracked files - untracked files are fine) */ async function hasPendingChanges(pi: ExtensionAPI): Promise { // Check for staged or unstaged changes to tracked files const { stdout, code } = await pi.exec("git", ["status", "--porcelain"]); if (code !== 0) return false; // Filter out untracked files (lines starting with ??) const lines = stdout .trim() .split("\n") .filter((line) => line.trim()); const trackedChanges = lines.filter((line) => !line.startsWith("??")); return trackedChanges.length > 0; } /** * Parse a PR reference (URL or number) and return the PR number plus an * optional "owner/repo" slug extracted from URL pastes. A bare number * resolves against the current repo (gh default); a URL carries its own * owner/repo, which MUST be forwarded to gh via --repo — without it, * `gh pr view 123` silently resolves against whatever repo the cwd is in, * and a review tool reviewing the wrong repo's diff is its worst failure. */ export function parsePrReference(ref: string): { prNumber: number; repo?: string; } | null { const trimmed = ref.trim(); // Try as a number first — reject anything that isn't purely numeric // (parseInt("123abc", 10) → 123 would silently accept garbage). if (/^\d+$/.test(trimmed)) { const num = parseInt(trimmed, 10); if (num > 0) return { prNumber: num }; } // Try to extract from GitHub URL // Formats: https://github.com/owner/repo/pull/123 // github.com/owner/repo/pull/123/files#diff-abc // github.com/owner/repo/pull/123?diff=1 // The trailing anchor ([/?#]|$) rejects malformed references such as // /pull/123abc — without it the regex matched a numeric prefix and silently // resolved "123abc" as PR 123. const urlMatch = trimmed.match( /github\.com\/([^/]+)\/([^/]+)\/pull\/(\d+)(?:[/?#]|$)/, ); if (urlMatch) { const num = parseInt(urlMatch[3], 10); if (num > 0) { // Strip trailing .git if present in the repo segment. const repo = urlMatch[2].replace(/\.git$/, ""); return { prNumber: num, repo: `${urlMatch[1]}/${repo}` }; } } return null; } /** * Tokenize a command argument string, respecting shell quoting rules. * * - Single quotes: no escaping (backslash is literal, per POSIX). * - Double quotes: backslash escapes the next char. * - Empty quoted strings ("") produce an empty-string token, not nothing. * * Extracted to module level and exported so the pure logic is testable * without spinning up the review command's closure. */ export function tokenizeArgs(value: string): string[] { const tokens: string[] = []; let current = ""; let quote: '"' | "'" | null = null; let hadQuote = false; // tracks whether the current token opened a quote for (let i = 0; i < value.length; i++) { const char = value[i]; if (quote) { // Backslash escapes only inside double quotes; inside single quotes // every character (including backslash) is literal (POSIX shell). if (quote === '"' && char === "\\" && i + 1 < value.length) { current += value[i + 1]; i += 1; continue; } if (char === quote) { quote = null; continue; } current += char; continue; } if (char === '"' || char === "'") { quote = char; hadQuote = true; continue; } // Outside quotes, backslash escapes the next char (POSIX): `\rm` → `rm`, // `foo\ bar` → `foo bar` (one token). if (char === "\\" && i + 1 < value.length) { current += value[i + 1]; i += 1; continue; } if (/\s/.test(char)) { if (current.length > 0 || hadQuote) { tokens.push(current); current = ""; hadQuote = false; } continue; } current += char; } if (current.length > 0 || hadQuote) { tokens.push(current); } return tokens; } /** * Normalize already-tokenized review paths: trim and drop empties (e.g. "" * tokens). Callers must tokenize raw text with tokenizeArgs() first — never * re-split token output on whitespace, that destroys quoted paths. * * Extracted to module level and exported so the pure logic is testable * without spinning up the review command's closure. */ export function parseReviewPaths(tokens: string[]): string[] { return tokens.map((item) => item.trim()).filter((item) => item.length > 0); } /** * Parse command arguments for direct invocation. * Returns the target or a special marker for PR that needs async handling. * * Extracted to module level and exported so the pure logic is testable * without spinning up the review command's closure. */ export type ParsedReviewArgs = { target: ReviewTarget | { type: "pr"; ref: string } | null; extraInstruction?: string; error?: string; }; export function parseArgs(args: string | undefined): ParsedReviewArgs { if (!args?.trim()) return { target: null }; const rawParts = tokenizeArgs(args.trim()); const parts: string[] = []; let extraInstruction: string | undefined; for (let i = 0; i < rawParts.length; i++) { const part = rawParts[i]; if (part === "--extra") { const next = rawParts[i + 1]; if (!next) { return { target: null, error: "Missing value for --extra" }; } extraInstruction = next; i += 1; continue; } if (part.startsWith("--extra=")) { extraInstruction = part.slice("--extra=".length); continue; } parts.push(part); } if (parts.length === 0) { return { target: null, extraInstruction }; } const subcommand = parts[0]?.toLowerCase(); switch (subcommand) { case "uncommitted": return { target: { type: "uncommitted" }, extraInstruction }; case "branch": { const branch = parts[1]; if (!branch) return { target: null, extraInstruction }; return { target: { type: "baseBranch", branch }, extraInstruction }; } case "commit": { const sha = parts[1]; if (!sha) return { target: null, extraInstruction }; const title = parts.slice(2).join(" ") || undefined; return { target: { type: "commit", sha, title }, extraInstruction }; } case "folder": { // parts are already tokenizeArgs() output — pass the tokens through, // don't join(" ") + re-split (that would destroy quoted paths). const paths = parseReviewPaths(parts.slice(1)); if (paths.length === 0) return { target: null, extraInstruction }; return { target: { type: "folder", paths }, extraInstruction }; } case "pr": { const ref = parts[1]; if (!ref) return { target: null, extraInstruction }; return { target: { type: "pr", ref }, extraInstruction }; } default: return { target: null, extraInstruction }; } } /** * Get PR information from GitHub CLI */ async function getPrInfo( pi: ExtensionAPI, prNumber: number, repo?: string, ): Promise<{ baseBranch: string; title: string; headBranch: string } | null> { const args = ["pr", "view", String(prNumber)]; if (repo) args.push("--repo", repo); args.push("--json", "baseRefName,title,headRefName"); const { stdout, code } = await pi.exec("gh", args); if (code !== 0) return null; try { const data = JSON.parse(stdout); return { baseBranch: data.baseRefName, title: data.title, headBranch: data.headRefName, }; } catch { return null; } } /** * Checkout a PR using GitHub CLI */ async function checkoutPr( pi: ExtensionAPI, prNumber: number, repo?: string, ): Promise<{ success: boolean; error?: string }> { const args = ["pr", "checkout", String(prNumber)]; if (repo) args.push("--repo", repo); const { stdout, stderr, code } = await pi.exec("gh", args); if (code !== 0) { return { success: false, error: stderr || stdout || "Failed to checkout PR", }; } return { success: true }; } /** * Get the current branch name */ async function getCurrentBranch(pi: ExtensionAPI): Promise { const { stdout, code } = await pi.exec("git", ["branch", "--show-current"]); if (code === 0 && stdout.trim()) { return stdout.trim(); } return null; } /** * Get the default branch (main or master) */ async function getDefaultBranch(pi: ExtensionAPI): Promise { // Try to get from remote HEAD const { stdout, code } = await pi.exec("git", [ "symbolic-ref", "refs/remotes/origin/HEAD", "--short", ]); if (code === 0 && stdout.trim()) { return stdout.trim().replace("origin/", ""); } // Fall back to checking if main or master exists const branches = await getLocalBranches(pi); if (branches.includes("main")) return "main"; if (branches.includes("master")) return "master"; return "main"; // Default fallback } /** * Build the review prompt based on target */ export async function buildReviewPrompt( pi: ExtensionAPI, target: ReviewTarget, ): Promise { // Titles/branches/paths flow into replace() as replacements — use function // replacements so `$&`/`$'`/`` $` `` in them stay literal instead of being // expanded as patterns. switch (target.type) { case "uncommitted": return UNCOMMITTED_PROMPT; case "baseBranch": { const mergeBase = await getMergeBase(pi, target.branch); const basePrompt = mergeBase ? BASE_BRANCH_PROMPT_WITH_MERGE_BASE.replace( /{baseBranch}/g, () => target.branch, ).replace(/{mergeBaseSha}/g, () => mergeBase) : BASE_BRANCH_PROMPT_FALLBACK.replace(/{branch}/g, () => target.branch); return basePrompt; } case "commit": if (target.title) { // local copies: closures don't inherit property narrowing on target const { sha, title } = target; return COMMIT_PROMPT_WITH_TITLE.replace("{sha}", () => sha).replace( "{title}", () => title, ); } return COMMIT_PROMPT.replace("{sha}", () => target.sha); case "pullRequest": { const mergeBase = await getMergeBase(pi, target.baseBranch); const basePrompt = mergeBase ? PULL_REQUEST_PROMPT.replace(/{prNumber}/g, String(target.prNumber)) .replace(/{title}/g, () => target.title) .replace(/{baseBranch}/g, () => target.baseBranch) .replace(/{mergeBaseSha}/g, () => mergeBase) : PULL_REQUEST_PROMPT_FALLBACK.replace( /{prNumber}/g, String(target.prNumber), ) .replace(/{title}/g, () => target.title) .replace(/{baseBranch}/g, () => target.baseBranch); return basePrompt; } case "folder": return FOLDER_REVIEW_PROMPT.replace("{paths}", () => target.paths.join(", "), ); } } /** * Get user-facing hint for the review target */ function getUserFacingHint(target: ReviewTarget): string { switch (target.type) { case "uncommitted": return "current changes"; case "baseBranch": return `changes against '${target.branch}'`; case "commit": { const shortSha = target.sha.slice(0, 7); return target.title ? `commit ${shortSha}: ${target.title}` : `commit ${shortSha}`; } case "pullRequest": { const shortTitle = target.title.length > 30 ? target.title.slice(0, 27) + "..." : target.title; return `PR #${target.prNumber}: ${shortTitle}`; } case "folder": { const joined = target.paths.join(", "); return joined.length > 40 ? `folders: ${joined.slice(0, 37)}...` : `folders: ${joined}`; } } } // Review preset options for the selector (keep this order stable) const REVIEW_PRESETS = [ { value: "uncommitted", label: "Review uncommitted changes", description: "", }, { value: "baseBranch", label: "Review against a base branch", description: "(local or origin/*)", }, { value: "commit", label: "Review a commit", description: "" }, { value: "pullRequest", label: "Review a pull request", description: "(GitHub PR)", }, { value: "folder", label: "Review a folder (or more)", description: "(snapshot, not diff)", }, ] as const; const TOGGLE_CUSTOM_INSTRUCTIONS_VALUE = "toggleCustomInstructions" as const; type ReviewPresetValue = | (typeof REVIEW_PRESETS)[number]["value"] | typeof TOGGLE_CUSTOM_INSTRUCTIONS_VALUE; // ── /review argument completion ───────────────────────────────────────── const REVIEW_VERBS_WITH_ARGUMENT = new Set([ "branch", "commit", "folder", "pr", "--extra", ]); const REVIEW_VERBS = [ "uncommitted", "branch", "commit", "folder", "pr", "--extra", ]; /** * Complete /review arguments: the target verb (uncommitted/branch/commit/ * folder/pr) and the --extra flag. Values are refs, paths, or free text — * no synchronous completion can do those justice, so returning null lets * pi fall back to file completion (right for folder, harmless for refs). */ export function completeReviewArgument( prefix: string, ): AutocompleteItem[] | null { if (!/\s/.test(prefix)) { const query = prefix.trim().toLowerCase(); const matches = REVIEW_VERBS.filter((w) => w.startsWith(query)); return matches.length ? matches.map((w) => ({ value: REVIEW_VERBS_WITH_ARGUMENT.has(w) ? `${w} ` : w, label: w, })) : null; } const m = prefix.match(/^(\S+)\s+([\s\S]*)$/); if (!m) return null; const rest = m[2]; const tokens = rest.split(/\s+/).filter(Boolean); const cur = /\s$/.test(rest) ? "" : (tokens[tokens.length - 1] ?? ""); // Only a partial flag completes past the verb; values stay free-form. if (cur.startsWith("-")) { return "--extra".startsWith(cur) ? [{ value: "--extra ", label: "--extra" }] : null; } return null; } /** The argument text of a /review line, or null outside that context. */ function reviewArgumentText( lines: string[], cursorLine: number, cursorCol: number, ): string | null { if (cursorLine !== 0) return null; const match = /^\/review\s+(.*)$/.exec((lines[0] ?? "").slice(0, cursorCol)); return match ? match[1] : null; } /** * Pi's editor turns Tab in slash-command argument context into a forced file * completion that never consults the command's getArgumentCompletions, so Tab * after "/review " would list cwd paths. Claim that context and answer from * the command's own completions — mirrors goodies' wrapGoodiesAutocomplete. * Returning null (values, unknown flags) falls back to file completion. */ export function wrapReviewAutocomplete( current: AutocompleteProvider, ): AutocompleteProvider { return { async getSuggestions(lines, cursorLine, cursorCol, options) { if (options.force) { const argumentText = reviewArgumentText(lines, cursorLine, cursorCol); if (argumentText !== null) { const items = completeReviewArgument(argumentText); return items ? { items, prefix: argumentText } : null; } } return current.getSuggestions(lines, cursorLine, cursorCol, options); }, applyCompletion(lines, cursorLine, cursorCol, item, prefix) { return current.applyCompletion( lines, cursorLine, cursorCol, item, prefix, ); }, shouldTriggerFileCompletion(lines, cursorLine, cursorCol) { if (reviewArgumentText(lines, cursorLine, cursorCol) !== null) { return true; } return ( current.shouldTriggerFileCompletion?.(lines, cursorLine, cursorCol) ?? true ); }, }; } let reviewAutocompleteWrapped = false; export default function reviewExtension(pi: ExtensionAPI) { function persistReviewSettings() { pi.appendEntry(REVIEW_SETTINGS_TYPE, { customInstructions: reviewCustomInstructions, }); } function setReviewCustomInstructions(instructions: string | undefined) { reviewCustomInstructions = instructions?.trim() || undefined; persistReviewSettings(); } function applyAllReviewState(ctx: ExtensionContext) { applyReviewSettings(ctx); applyReviewState(ctx); } async function ensureGithubCliReady(ctx: ExtensionContext): Promise { const ghVersion = await pi.exec("gh", ["--version"]); if (ghVersion.code !== 0) { ctx.ui.notify( `PR review requires GitHub CLI (\`gh\`). ${GH_SETUP_INSTRUCTIONS}`, "error", ); return false; } const ghAuthStatus = await pi.exec("gh", ["auth", "status"]); if (ghAuthStatus.code !== 0) { ctx.ui.notify( "GitHub CLI is installed, but you're not signed in. Run `gh auth login`, then verify with `gh auth status`.", "error", ); return false; } return true; } async function resolvePullRequestTarget( ctx: ExtensionContext, ref: string, options: { skipInitialPendingChangesCheck?: boolean } = {}, ): Promise { if (!(await ensureGithubCliReady(ctx))) { return null; } if ( !options.skipInitialPendingChangesCheck && (await hasPendingChanges(pi)) ) { ctx.ui.notify(PR_CHECKOUT_BLOCKED_BY_PENDING_CHANGES_MESSAGE, "error"); return null; } const prRef = parsePrReference(ref); if (!prRef) { ctx.ui.notify( "Invalid PR reference. Enter a number or GitHub PR URL.", "error", ); return null; } const { prNumber, repo } = prRef; ctx.ui.notify(`Fetching PR #${prNumber} info...`, "info"); const prInfo = await getPrInfo(pi, prNumber, repo); if (!prInfo) { ctx.ui.notify( `Could not fetch PR #${prNumber}. Make sure it exists and your GitHub auth has access (check with \`gh auth status\`).`, "error", ); return null; } // Re-check right before checkout to avoid switching branches with newly introduced changes. if (await hasPendingChanges(pi)) { ctx.ui.notify(PR_CHECKOUT_BLOCKED_BY_PENDING_CHANGES_MESSAGE, "error"); return null; } ctx.ui.notify(`Checking out PR #${prNumber}...`, "info"); const checkoutResult = await checkoutPr(pi, prNumber, repo); if (!checkoutResult.success) { ctx.ui.notify(`Failed to checkout PR: ${checkoutResult.error}`, "error"); return null; } ctx.ui.notify(`Checked out PR #${prNumber} (${prInfo.headBranch})`, "info"); return { type: "pullRequest", prNumber, baseBranch: prInfo.baseBranch, title: prInfo.title, }; } pi.on("session_start", (_event, ctx) => { applyAllReviewState(ctx); // Claim the /review forced-Tab context once per extension load (guarded: // harness stubs and limited contexts lack addAutocompleteProvider). const ui = ( ctx as { ui?: { addAutocompleteProvider?: ( factory: (current: AutocompleteProvider) => AutocompleteProvider, ) => void; }; } ).ui; const add = ui?.addAutocompleteProvider; if (!reviewAutocompleteWrapped && typeof add === "function") { reviewAutocompleteWrapped = true; add(wrapReviewAutocomplete); } }); pi.on("session_tree", (_event, ctx) => { applyAllReviewState(ctx); }); /** * Determine the smart default review type based on git state */ async function getSmartDefault(): Promise< "uncommitted" | "baseBranch" | "commit" > { // Priority 1: If there are uncommitted changes, default to reviewing them if (await hasUncommittedChanges(pi)) { return "uncommitted"; } // Priority 2: If on a feature branch (not the default branch), default to PR-style review const currentBranch = await getCurrentBranch(pi); const defaultBranch = await getDefaultBranch(pi); if (currentBranch && currentBranch !== defaultBranch) { return "baseBranch"; } // Priority 3: Default to reviewing a specific commit return "commit"; } /** * Show the review preset selector */ async function showReviewSelector( ctx: ExtensionContext, ): Promise { // Determine smart default (but keep the list order stable) const smartDefault = await getSmartDefault(); const presetItems: SelectItem[] = REVIEW_PRESETS.map((preset) => ({ value: preset.value, label: preset.label, description: preset.description, })); const smartDefaultIndex = presetItems.findIndex( (item) => item.value === smartDefault, ); while (true) { const customInstructionsLabel = reviewCustomInstructions ? "Remove custom review instructions" : "Add custom review instructions"; const customInstructionsDescription = reviewCustomInstructions ? "(currently set)" : "(applies to all review modes)"; const items: SelectItem[] = [ ...presetItems, { value: TOGGLE_CUSTOM_INSTRUCTIONS_VALUE, label: customInstructionsLabel, description: customInstructionsDescription, }, ]; const result = await ctx.ui.custom( (tui, theme, _kb, done) => { const container = new Container(); container.addChild( new DynamicBorder((str) => theme.fg("accent", str)), ); container.addChild( new Text(theme.fg("accent", theme.bold("Select a review preset"))), ); const selectList = new SelectList(items, Math.min(items.length, 10), { selectedPrefix: (text) => theme.fg("accent", text), selectedText: (text) => theme.fg("accent", text), description: (text) => theme.fg("muted", text), scrollInfo: (text) => theme.fg("dim", text), noMatch: (text) => theme.fg("warning", text), }); // Preselect the smart default without reordering the list if (smartDefaultIndex >= 0) { selectList.setSelectedIndex(smartDefaultIndex); } selectList.onSelect = (item) => done(item.value as ReviewPresetValue); selectList.onCancel = () => done(null); container.addChild(selectList); container.addChild( new Text( theme.fg("dim", "Press enter to confirm or esc to go back"), ), ); container.addChild( new DynamicBorder((str) => theme.fg("accent", str)), ); return { render(width: number) { return container.render(width); }, invalidate() { container.invalidate(); }, handleInput(data: string) { selectList.handleInput(data); tui.requestRender(); }, }; }, ); if (!result) return null; if (result === TOGGLE_CUSTOM_INSTRUCTIONS_VALUE) { if (reviewCustomInstructions) { setReviewCustomInstructions(undefined); ctx.ui.notify("Custom review instructions removed", "info"); continue; } const customInstructions = await ctx.ui.editor( "Enter custom review instructions (applies to all review modes):", "", ); if (!customInstructions?.trim()) { ctx.ui.notify("Custom review instructions not changed", "info"); continue; } setReviewCustomInstructions(customInstructions); ctx.ui.notify("Custom review instructions saved", "info"); continue; } // Handle each preset type switch (result) { case "uncommitted": return { type: "uncommitted" }; case "baseBranch": { const target = await showBranchSelector(ctx); if (target) return target; break; } case "commit": { const target = await showCommitSelector(ctx); if (target) return target; break; } case "folder": { const target = await showFolderInput(ctx); if (target) return target; break; } case "pullRequest": { const target = await showPrInput(ctx); if (target) return target; break; } default: return null; } } } /** * Show branch selector for base branch review */ async function showBranchSelector( ctx: ExtensionContext, ): Promise { const [localBranches, remoteBranches] = await Promise.all([ getLocalBranches(pi), getRemoteBranches(pi), ]); const currentBranch = await getCurrentBranch(pi); const defaultBranch = await getDefaultBranch(pi); // Combine local and remote-tracking branches. Dedupe by name (a local // branch and its origin/ counterpart are distinct refs, so both // stay — the user explicitly asked for origin/main | origin/master). const seen = new Set(); const combined = [...localBranches, ...remoteBranches].filter((b) => { if (seen.has(b)) return false; seen.add(b); return true; }); // Never offer the current branch as a base branch (reviewing against // itself is meaningless). Remote-tracking branches are never equal to a // local current-branch name, so they pass through untouched. const candidateBranches = currentBranch ? combined.filter((b) => b !== currentBranch) : combined; if (candidateBranches.length === 0) { ctx.ui.notify( currentBranch ? `No other branches found (current branch: ${currentBranch})` : "No branches found", "error", ); return null; } const defaultRemote = `origin/${defaultBranch}`; // Sort: default local branch first, then its origin/ counterpart, // then remaining locals, then remaining remotes — each group alphabetical. const sortedBranches = candidateBranches.sort((a, b) => { if (a === defaultBranch) return -1; if (b === defaultBranch) return 1; if (a === defaultRemote) return -1; if (b === defaultRemote) return 1; const aRemote = a.startsWith("origin/"); const bRemote = b.startsWith("origin/"); if (aRemote !== bRemote) return aRemote ? 1 : -1; return a.localeCompare(b); }); const items: SelectItem[] = sortedBranches.map((branch) => { let description = ""; if (branch === defaultBranch) description = "(default)"; else if (branch === defaultRemote) description = "(remote default)"; else if (branch.startsWith("origin/")) description = "(remote)"; return { value: branch, label: branch, description }; }); const result = await ctx.ui.custom( (tui, theme, keybindings, done) => { const container = new Container(); container.addChild(new DynamicBorder((str) => theme.fg("accent", str))); container.addChild( new Text(theme.fg("accent", theme.bold("Select base branch"))), ); const searchInput = new Input(); container.addChild(searchInput); container.addChild(new Spacer(1)); const listContainer = new Container(); container.addChild(listContainer); container.addChild( new Text( theme.fg("dim", "Type to filter • enter to select • esc to cancel"), ), ); container.addChild(new DynamicBorder((str) => theme.fg("accent", str))); let filteredItems = items; let selectList: SelectList | null = null; const updateList = () => { listContainer.clear(); if (filteredItems.length === 0) { listContainer.addChild( new Text(theme.fg("warning", " No matching branches")), ); selectList = null; return; } selectList = new SelectList( filteredItems, Math.min(filteredItems.length, 10), { selectedPrefix: (text) => theme.fg("accent", text), selectedText: (text) => theme.fg("accent", text), description: (text) => theme.fg("muted", text), scrollInfo: (text) => theme.fg("dim", text), noMatch: (text) => theme.fg("warning", text), }, ); selectList.onSelect = (item) => done(item.value); selectList.onCancel = () => done(null); listContainer.addChild(selectList); }; const applyFilter = () => { const query = searchInput.getValue(); filteredItems = query ? fuzzyFilter( items, query, (item) => `${item.label} ${item.value} ${item.description ?? ""}`, ) : items; updateList(); }; applyFilter(); return { render(width: number) { return container.render(width); }, invalidate() { container.invalidate(); }, handleInput(data: string) { if ( keybindings.matches(data, "tui.select.up") || keybindings.matches(data, "tui.select.down") || keybindings.matches(data, "tui.select.confirm") || keybindings.matches(data, "tui.select.cancel") ) { if (selectList) { selectList.handleInput(data); } else if (keybindings.matches(data, "tui.select.cancel")) { done(null); } tui.requestRender(); return; } searchInput.handleInput(data); applyFilter(); tui.requestRender(); }, }; }, ); if (!result) return null; return { type: "baseBranch", branch: result }; } /** * Show commit selector */ async function showCommitSelector( ctx: ExtensionContext, ): Promise { const commits = await getRecentCommits(pi, 20); if (commits.length === 0) { ctx.ui.notify("No commits found", "error"); return null; } const items: SelectItem[] = commits.map((commit) => ({ value: commit.sha, label: `${commit.sha.slice(0, 7)} ${commit.title}`, description: "", })); const result = await ctx.ui.custom<{ sha: string; title: string } | null>( (tui, theme, keybindings, done) => { const container = new Container(); container.addChild(new DynamicBorder((str) => theme.fg("accent", str))); container.addChild( new Text(theme.fg("accent", theme.bold("Select commit to review"))), ); const searchInput = new Input(); container.addChild(searchInput); container.addChild(new Spacer(1)); const listContainer = new Container(); container.addChild(listContainer); container.addChild( new Text( theme.fg("dim", "Type to filter • enter to select • esc to cancel"), ), ); container.addChild(new DynamicBorder((str) => theme.fg("accent", str))); let filteredItems = items; let selectList: SelectList | null = null; const updateList = () => { listContainer.clear(); if (filteredItems.length === 0) { listContainer.addChild( new Text(theme.fg("warning", " No matching commits")), ); selectList = null; return; } selectList = new SelectList( filteredItems, Math.min(filteredItems.length, 10), { selectedPrefix: (text) => theme.fg("accent", text), selectedText: (text) => theme.fg("accent", text), description: (text) => theme.fg("muted", text), scrollInfo: (text) => theme.fg("dim", text), noMatch: (text) => theme.fg("warning", text), }, ); selectList.onSelect = (item) => { const commit = commits.find((c) => c.sha === item.value); if (commit) { done(commit); } else { done(null); } }; selectList.onCancel = () => done(null); listContainer.addChild(selectList); }; const applyFilter = () => { const query = searchInput.getValue(); filteredItems = query ? fuzzyFilter( items, query, (item) => `${item.label} ${item.value} ${item.description ?? ""}`, ) : items; updateList(); }; applyFilter(); return { render(width: number) { return container.render(width); }, invalidate() { container.invalidate(); }, handleInput(data: string) { if ( keybindings.matches(data, "tui.select.up") || keybindings.matches(data, "tui.select.down") || keybindings.matches(data, "tui.select.confirm") || keybindings.matches(data, "tui.select.cancel") ) { if (selectList) { selectList.handleInput(data); } else if (keybindings.matches(data, "tui.select.cancel")) { done(null); } tui.requestRender(); return; } searchInput.handleInput(data); applyFilter(); tui.requestRender(); }, }; }, ); if (!result) return null; return { type: "commit", sha: result.sha, title: result.title }; } /** * Show folder input */ async function showFolderInput( ctx: ExtensionContext, ): Promise { const result = await ctx.ui.editor( "Enter folders/files to review (space-separated or one per line):", ".", ); if (!result?.trim()) return null; const paths = parseReviewPaths(tokenizeArgs(result)); if (paths.length === 0) return null; return { type: "folder", paths }; } /** * Show PR input and handle checkout */ async function showPrInput( ctx: ExtensionContext, ): Promise { // First check for pending changes that would prevent branch switching if (await hasPendingChanges(pi)) { ctx.ui.notify(PR_CHECKOUT_BLOCKED_BY_PENDING_CHANGES_MESSAGE, "error"); return null; } // Get PR reference from user const prRef = await ctx.ui.editor( "Enter PR number or URL (e.g. 123 or https://github.com/owner/repo/pull/123):", "", ); if (!prRef?.trim()) return null; return await resolvePullRequestTarget(ctx, prRef, { skipInitialPendingChangesCheck: true, }); } /** * Execute the review */ async function executeReview( ctx: ExtensionCommandContext, target: ReviewTarget, useFreshSession: boolean, options?: { extraInstruction?: string }, ): Promise { // Check if we're already in a review if (reviewOriginId) { ctx.ui.notify( "Already in a review. Use /end-review to finish first.", "warning", ); return false; } // Handle fresh session mode if (useFreshSession) { // Store current position (where we'll return to). // In an empty session there is no leaf yet, so create a lightweight anchor first. let originId = ctx.sessionManager.getLeafId() ?? undefined; if (!originId) { pi.appendEntry(REVIEW_ANCHOR_TYPE, { createdAt: new Date().toISOString(), }); originId = ctx.sessionManager.getLeafId() ?? undefined; } if (!originId) { ctx.ui.notify("Failed to determine review origin.", "error"); return false; } reviewOriginId = originId; // Keep a local copy so session_tree events during navigation don't wipe it const lockedOriginId = originId; // Find the first user message in the session. // If none exists (e.g. brand-new session), we'll stay on the current leaf. const entries = ctx.sessionManager.getEntries(); const firstUserMessage = entries.find( (e) => e.type === "message" && e.message.role === "user", ); if (firstUserMessage) { // Navigate to first user message to create a new branch from that point // Label it as "code-review" so it's visible in the tree try { const result = await ctx.navigateTree(firstUserMessage.id, { summarize: false, label: "code-review", }); if (result.cancelled) { reviewOriginId = undefined; return false; } } catch (error) { // Clean up state if navigation fails reviewOriginId = undefined; ctx.ui.notify( `Failed to start review: ${describeError(error)}`, "error", ); return false; } // Clear the editor (navigating to user message fills it with the message text) ctx.ui.setEditorText(""); } // Restore origin after navigation events (session_tree can reset it) reviewOriginId = lockedOriginId; // Show widget indicating review is active setReviewWidget(ctx, true); // Persist review state so tree navigation can restore/reset it pi.appendEntry(REVIEW_STATE_TYPE, { active: true, originId: lockedOriginId, }); } const prompt = await buildReviewPrompt(pi, target); const hint = getUserFacingHint(target); const projectGuidelines = await loadProjectReviewGuidelines(ctx.cwd); // Combine the review rubric with the specific prompt let fullPrompt = `${REVIEW_RUBRIC}\n\n---\n\nPlease perform a code review with the following focus:\n\n${prompt}`; if (reviewCustomInstructions) { fullPrompt += `\n\nShared custom review instructions (applies to all reviews):\n\n${reviewCustomInstructions}`; } if (options?.extraInstruction?.trim()) { fullPrompt += `\n\nAdditional user-provided review instruction:\n\n${options.extraInstruction.trim()}`; } if (projectGuidelines) { fullPrompt += `\n\nThis project has additional instructions for code reviews:\n\n${projectGuidelines}`; } const modeHint = useFreshSession ? " (fresh session)" : ""; ctx.ui.notify(`Starting review: ${hint}${modeHint}`, "info"); // Send as a user message that triggers a turn. deliverAs matters when // pi is mid-stream: without it prompt() throws ("Agent is already // processing. Specify streamingBehavior…") and the runtime swallows the // rejection into a generic send_user_message error — the user sees // "Starting review…" and nothing happens. followUp queues the review for // after the current turn; when idle, deliverAs is ignored, so a normal // start is unchanged. Same contract as the fix-findings send below. pi.sendUserMessage(fullPrompt, { deliverAs: "followUp" }); return true; } /** * Handle PR checkout and return a ReviewTarget (or null on failure) */ async function handlePrCheckout( ctx: ExtensionContext, ref: string, ): Promise { return await resolvePullRequestTarget(ctx, ref); } // Register the /review command pi.registerCommand("review", { description: "Review code changes (PR, uncommitted, branch, commit, or folder)", getArgumentCompletions: completeReviewArgument, handler: async (args, ctx) => { if (!ctx.hasUI) { ctx.ui.notify("Review requires interactive mode", "error"); return; } // Check if we're already in a review if (reviewOriginId) { ctx.ui.notify( "Already in a review. Use /end-review to finish first.", "warning", ); return; } // Check if we're in a git repository const { code } = await pi.exec("git", ["rev-parse", "--git-dir"]); if (code !== 0) { ctx.ui.notify("Not a git repository", "error"); return; } // Try to parse direct arguments let target: ReviewTarget | null = null; let fromSelector = false; let extraInstruction: string | undefined; const parsed = parseArgs(args); if (parsed.error) { ctx.ui.notify(parsed.error, "error"); return; } extraInstruction = parsed.extraInstruction?.trim() || undefined; if (parsed.target) { if (parsed.target.type === "pr") { // Handle PR checkout (async operation) target = await handlePrCheckout(ctx, parsed.target.ref); if (!target) { ctx.ui.notify( "PR review failed. Returning to review menu.", "warning", ); } } else { target = parsed.target; } } // If no args or invalid args, show selector if (!target) { fromSelector = true; } while (true) { if (!target && fromSelector) { target = await showReviewSelector(ctx); } if (!target) { ctx.ui.notify("Review cancelled", "info"); return; } // Determine if we should use fresh session mode // Check if this is a new session (no messages yet) const entries = ctx.sessionManager.getEntries(); const messageCount = entries.filter((e) => e.type === "message").length; // In an empty session, default to fresh review mode so /end-review works consistently. let useFreshSession = messageCount === 0; if (messageCount > 0) { // Existing session - ask user which mode they want const choice = await ctx.ui.select("Start review in:", [ "Empty branch", "Current session", ]); if (choice === undefined) { if (fromSelector) { target = null; continue; } ctx.ui.notify("Review cancelled", "info"); return; } useFreshSession = choice === "Empty branch"; } await executeReview(ctx, target, useFreshSession, { extraInstruction }); return; } }, }); // Custom prompt for review summaries - focuses on preserving actionable findings const REVIEW_SUMMARY_PROMPT = `We are leaving a code-review branch and returning to the main coding branch. Create a structured handoff that can be used immediately to implement fixes. You MUST summarize the review that happened in this branch so findings can be acted on. Do not omit findings: include every actionable issue that was identified. Required sections (in order): ## Review Scope - What was reviewed (files/paths, changes, and scope) ## Verdict - "correct" or "needs attention" ## Findings For EACH finding, include: - Priority tag ([P0]..[P3]) and short title - File location (\`path/to/file.ext:line\`) - Why it matters (brief) - What should change (brief, actionable) ## Fix Queue 1. Ordered implementation checklist (highest priority first) ## Constraints & Preferences - Any constraints or preferences mentioned during review - Or "(none)" ## Human Reviewer Callouts (Non-Blocking) Include only applicable callouts (no yes/no lines): - **This change adds a database migration:** - **This change introduces a new dependency:** - **This change changes a dependency (or the lockfile):** - **This change modifies auth/permission behavior:** - **This change introduces backwards-incompatible public schema/API/contract changes:** - **This change includes irreversible or destructive operations:** If none apply, write "- (none)". These are informational callouts for humans and are not fix items by themselves. Preserve exact file paths, function names, and error messages where available.`; const REVIEW_FIX_FINDINGS_PROMPT = `Use the latest review summary in this session and implement the review findings now. Instructions: 1. Treat the summary's Findings/Fix Queue as a checklist. 2. Fix in priority order: P0, P1, then P2 (include P3 if quick and safe). 3. If a finding is invalid/already fixed/not possible right now, briefly explain why and continue. 4. Treat "Human Reviewer Callouts (Non-Blocking)" as informational only; do not convert them into fix tasks unless there is a separate explicit finding. 5. Follow fail-fast error handling: do not add local catch/fallback recovery unless this scope is an explicit boundary that can safely translate the failure. 6. If you add or keep a \`try/catch\`, explain the expected failure mode and either rethrow with context or return a boundary-safe error response. 7. JSON parsing/decoding should fail loudly by default; avoid silent fallback parsing. 8. Run relevant tests/checks for touched code where practical. 9. End with: fixed items, deferred/skipped items (with reasons), and verification results.`; type EndReviewAction = "returnOnly" | "returnAndFix" | "returnAndSummarize"; type EndReviewActionResult = "ok" | "cancelled" | "error"; type EndReviewActionOptions = { showSummaryLoader?: boolean; notifySuccess?: boolean; }; function getActiveReviewOrigin(ctx: ExtensionContext): string | undefined { if (reviewOriginId) { return reviewOriginId; } const state = getReviewState(ctx); if (state?.active && state.originId) { reviewOriginId = state.originId; return reviewOriginId; } if (state?.active) { setReviewWidget(ctx, false); pi.appendEntry(REVIEW_STATE_TYPE, { active: false }); ctx.ui.notify( "Review state was missing origin info; cleared review status.", "warning", ); } return undefined; } function clearReviewState(ctx: ExtensionContext) { setReviewWidget(ctx, false); reviewOriginId = undefined; pi.appendEntry(REVIEW_STATE_TYPE, { active: false }); } async function navigateWithSummary( ctx: ExtensionCommandContext, originId: string, showLoader: boolean, ): Promise<{ cancelled: boolean; error?: string } | null> { if (showLoader && ctx.hasUI) { return ctx.ui.custom<{ cancelled: boolean; error?: string } | null>( (tui, theme, _kb, done) => { const loader = new BorderedLoader( tui, theme, "Returning and summarizing review branch...", ); // Esc resolves this dialog immediately, but pi has no // extension-facing way to cancel the in-flight navigation // (session.abortBranchSummary() is internal, navigateTree takes no // signal), so the work can race past the abort. Track it and // reconcile when the promise settles. let aborted = false; loader.onAbort = () => { aborted = true; done(null); }; ctx .navigateTree(originId, { summarize: true, customInstructions: REVIEW_SUMMARY_PROMPT, replaceInstructions: true, }) .then((result) => { if (aborted) { if (!result.cancelled) { // Completed after the abort: the tree already moved back // to the origin, so the review is really over. Discard // the result (it must not be applied below) and clear the // persisted state so the stale {active: true} entry can't // resurrect the review widget on replay. try { clearReviewState(ctx); ctx.ui.notify( "Summarization finished in the background after cancel; review ended.", "info", ); } catch (err) { // ctx can be stale (reload/session switch) by the time // the navigation settles — surface it instead of // crashing on a dead context. console.error( `[review] failed to clear review state after aborted summarization: ${describeError(err)}`, ); } } return; } done(result); }) .catch((err) => { if (aborted) { // pi throws before mutating the tree, so the review is // intact and retrying /end-review stays valid — but don't // swallow the failure silently. console.error( `[review] abandoned summarization navigation failed: ${describeError(err)}`, ); return; } done({ cancelled: false, error: describeError(err), }); }); return loader; }, ); } try { return await ctx.navigateTree(originId, { summarize: true, customInstructions: REVIEW_SUMMARY_PROMPT, replaceInstructions: true, }); } catch (error) { return { cancelled: false, error: describeError(error), }; } } async function executeEndReviewAction( ctx: ExtensionCommandContext, action: EndReviewAction, options: EndReviewActionOptions = {}, ): Promise { const originId = getActiveReviewOrigin(ctx); if (!originId) { if (!getReviewState(ctx)?.active) { ctx.ui.notify( "Not in a review branch (use /review first, or review was started in current session mode)", "info", ); } return "error"; } const notifySuccess = options.notifySuccess ?? true; if (action === "returnOnly") { try { const result = await ctx.navigateTree(originId, { summarize: false }); if (result.cancelled) { ctx.ui.notify( "Navigation cancelled. Use /end-review to try again.", "info", ); return "cancelled"; } } catch (error) { ctx.ui.notify(`Failed to return: ${describeError(error)}`, "error"); return "error"; } clearReviewState(ctx); if (notifySuccess) { ctx.ui.notify( "Review complete! Returned to original position.", "info", ); } return "ok"; } const summaryResult = await navigateWithSummary( ctx, originId, options.showSummaryLoader ?? false, ); if (summaryResult === null) { ctx.ui.notify( "Summarization cancelled. Use /end-review to try again.", "info", ); return "cancelled"; } if (summaryResult.error) { ctx.ui.notify(`Summarization failed: ${summaryResult.error}`, "error"); return "error"; } if (summaryResult.cancelled) { ctx.ui.notify( "Navigation cancelled. Use /end-review to try again.", "info", ); return "cancelled"; } clearReviewState(ctx); if (action === "returnAndSummarize") { if (!ctx.ui.getEditorText().trim()) { ctx.ui.setEditorText("Act on the review findings"); } if (notifySuccess) { ctx.ui.notify("Review complete! Returned and summarized.", "info"); } return "ok"; } pi.sendUserMessage(REVIEW_FIX_FINDINGS_PROMPT, { deliverAs: "followUp" }); if (notifySuccess) { ctx.ui.notify( "Review complete! Returned and queued a follow-up to fix findings.", "info", ); } return "ok"; } async function runEndReview(ctx: ExtensionCommandContext): Promise { if (!ctx.hasUI) { ctx.ui.notify("End-review requires interactive mode", "error"); return; } if (endReviewInProgress) { ctx.ui.notify("/end-review is already running", "info"); return; } endReviewInProgress = true; try { const choice = await ctx.ui.select("Finish review:", [ "Return only", "Return and fix findings", "Return and summarize", ]); if (choice === undefined) { ctx.ui.notify("Cancelled. Use /end-review to try again.", "info"); return; } const action: EndReviewAction = choice === "Return and fix findings" ? "returnAndFix" : choice === "Return and summarize" ? "returnAndSummarize" : "returnOnly"; await executeEndReviewAction(ctx, action, { showSummaryLoader: true, notifySuccess: true, }); } finally { endReviewInProgress = false; } } // Register the /end-review command pi.registerCommand("end-review", { description: "Complete review and return to original position", handler: async (_args, ctx) => { await runEndReview(ctx); }, }); }