diff --git a/README.md b/README.md index 8bc2d20..5908cb4 100644 --- a/README.md +++ b/README.md @@ -47,6 +47,18 @@ Review the current branch against a target branch: codiff main ``` +Review the current branch against the branch its pull request or merge request +targets, which is handy for stacked changes where the base is not `main`: + +```bash +codiff base +``` + +Codiff asks GitHub through `gh` or GitLab through `glab` (chosen from the +repository's remote) for the current branch's PR/MR base branch, refreshes that +branch from the remote, and diffs against it. When there is no open request, or +the relevant CLI is unavailable, it falls back to the remote's default branch. + Review a GitHub pull request or GitLab merge request using the current repository remote: ```bash diff --git a/bin/arguments.js b/bin/arguments.js index 475b738..2edd405 100644 --- a/bin/arguments.js +++ b/bin/arguments.js @@ -91,6 +91,7 @@ export const usageExamples = [ { command: 'codiff', description: 'Review staged and unstaged changes.' }, { command: 'codiff /path/to/repo', description: 'Review changes in a specific repository.' }, { command: 'codiff main', description: 'Review the current branch against main.' }, + { command: 'codiff base', description: "Review against the current branch's PR/MR base branch." }, { command: 'codiff a1b2c3d', description: 'Review a specific commit.' }, { command: "codiff '#75'", description: 'Review pull request #75.' }, { command: 'codiff pr 75', description: 'Review pull request #75 (alternate syntax).' }, @@ -216,6 +217,8 @@ const getReviewProviderMarker = (arg) => ? 'gitlab' : null; +const isBaseReviewMarker = (arg) => /^base$/i.test(arg); + const isPullRequestUrlArgument = (arg) => parseReviewUrl(arg) != null; export const resolvePullRequestUrl = (repositoryPath, number, provider) => @@ -250,6 +253,7 @@ export const parseArguments = (args) => { let pullRequestProvider = null; let pullRequestUrl = null; let requestedPath = null; + let reviewBase = false; let sourceCandidate = null; let rangeCandidate = null; const walkthroughContextPath = @@ -314,13 +318,17 @@ export const parseArguments = (args) => { : null; } if (!range && !commitRef && !branchRef && sourceCandidate) { - const source = resolveSourceCandidate(repositoryPath, sourceCandidate); - if (source?.branchRef) { - branchRef = source.branchRef; - } else if (source?.commitRef) { - commitRef = source.commitRef; - } else if (requestedPath == null) { - requestedPath = sourceCandidate; + if (isBaseReviewMarker(sourceCandidate)) { + reviewBase = true; + } else { + const source = resolveSourceCandidate(repositoryPath, sourceCandidate); + if (source?.branchRef) { + branchRef = source.branchRef; + } else if (source?.commitRef) { + commitRef = source.commitRef; + } else if (requestedPath == null) { + requestedPath = sourceCandidate; + } } } @@ -338,6 +346,7 @@ export const parseArguments = (args) => { pullRequestNumber, ...(pullRequestProvider ? { pullRequestProvider } : {}), pullRequestUrl, + ...(reviewBase ? { reviewBase: true } : {}), requestedPath: resolve(requestedPath ?? process.cwd()), ...(values.share === true ? { share: true } : {}), version: values.version === true, diff --git a/bin/codiff-app b/bin/codiff-app index 1dc0b50..d0c2a0c 100755 --- a/bin/codiff-app +++ b/bin/codiff-app @@ -51,6 +51,7 @@ show_help() { printf ' %-32s%s%s%s\n' "codiff" "$gray" "Review staged and unstaged changes." "$reset" printf ' %-32s%s%s%s\n' "codiff /path/to/repo" "$gray" "Review changes in a specific repository." "$reset" printf ' %-32s%s%s%s\n' "codiff main" "$gray" "Review the current branch against main." "$reset" + printf ' %-32s%s%s%s\n' "codiff base" "$gray" "Review against the current branch's PR/MR base branch." "$reset" printf ' %-32s%s%s%s\n' "codiff a1b2c3d" "$gray" "Review a specific commit." "$reset" printf ' %-32s%s%s%s\n' "codiff '#75'" "$gray" "Review pull request #75." "$reset" printf ' %-32s%s%s%s\n' "codiff pr 75" "$gray" "Review pull request #75 (alternate syntax)." "$reset" @@ -64,20 +65,22 @@ show_help() { } for arg in "$@"; do - if [ "$arg" = "--share" ]; then - runtime="${CODIFF_NODE_COMMAND:-}" - if [ -z "$runtime" ]; then - electron_exec="$(ls "$app_path/Contents/MacOS/" 2>/dev/null | head -n 1)" - if [ -n "$electron_exec" ]; then - runtime="$app_path/Contents/MacOS/$electron_exec" + case "$arg" in + --share|[Bb][Aa][Ss][Ee]) + runtime="${CODIFF_NODE_COMMAND:-}" + if [ -z "$runtime" ]; then + electron_exec="$(ls "$app_path/Contents/MacOS/" 2>/dev/null | head -n 1)" + if [ -n "$electron_exec" ]; then + runtime="$app_path/Contents/MacOS/$electron_exec" + fi fi - fi - if [ -z "$runtime" ] || [ ! -x "$runtime" ]; then - printf '%s\n' "codiff: could not find the bundled runtime for --share." >&2 - exit 1 - fi - ELECTRON_RUN_AS_NODE=1 exec "$runtime" "$script_dir/codiff.js" "$@" - fi + if [ -z "$runtime" ] || [ ! -x "$runtime" ]; then + printf '%s\n' "codiff: could not find the bundled runtime." >&2 + exit 1 + fi + ELECTRON_RUN_AS_NODE=1 exec "$runtime" "$script_dir/codiff.js" "$@" + ;; + esac done repository_path="" diff --git a/bin/codiff.js b/bin/codiff.js index 570356f..9a14846 100755 --- a/bin/codiff.js +++ b/bin/codiff.js @@ -1,6 +1,6 @@ #!/usr/bin/env node -import { spawn } from 'node:child_process'; +import { execFileSync, spawn } from 'node:child_process'; import { existsSync, mkdtempSync, readFileSync, rmSync } from 'node:fs'; import http from 'node:http'; import https from 'node:https'; @@ -19,6 +19,7 @@ const { shareWalkthroughFile, } = require('../electron/headless-walkthrough-share.cjs'); const { sharePlanFile } = require('../electron/headless-plan-share.cjs'); +const { resolveBaseBranchRef } = require('../electron/review-source.cjs'); // The renderer is the built dist/ by default. When Codiff's own Vite dev server // is running, use it instead so source edits hot-reload without a rebuild. The @@ -119,7 +120,6 @@ const run = async () => { const { agentBackend, - branchRef, claudeSessionId, codexSessionId, commitRef, @@ -130,12 +130,26 @@ const run = async () => { pullRequestProvider, range, requestedPath, + reviewBase, share, walkthrough, walkthroughContextPath, walkthroughFilePath, } = parsedArguments; - let { pullRequestUrl } = parsedArguments; + let { branchRef, pullRequestUrl } = parsedArguments; + + if (reviewBase && !branchRef && !commitRef && !range) { + const { base, ref, remote } = resolveBaseBranchRef(requestedPath); + branchRef = ref; + try { + execFileSync('git', ['-C', requestedPath, 'fetch', remote, base, '--quiet'], { + stdio: 'ignore', + timeout: 30_000, + }); + } catch { + // Keep going with the local remote ref when offline. + } + } if (planFilePath && (!existsSync(planFilePath) || !/\.md$/i.test(planFilePath))) { process.stderr.write(`codiff: plan file not found or not Markdown: ${planFilePath}\n`); diff --git a/core/__tests__/codiff-cli.test.ts b/core/__tests__/codiff-cli.test.ts index d16c692..90668ab 100644 --- a/core/__tests__/codiff-cli.test.ts +++ b/core/__tests__/codiff-cli.test.ts @@ -10,6 +10,7 @@ import { truncate, writeFile, } from 'node:fs/promises'; +import { createRequire } from 'node:module'; import { tmpdir } from 'node:os'; import { join, resolve } from 'node:path'; import { promisify } from 'node:util'; @@ -20,6 +21,48 @@ import { createFakeCommandLogger, createFakeOpenLogger } from './helpers/cli.ts' const execFileAsync = promisify(execFile); +const require = createRequire(import.meta.url); +const { resolveBaseBranchRef } = require('../../electron/review-source.cjs') as { + resolveBaseBranchRef: (repositoryPath: string) => { + base: string; + ref: string; + remote: string; + }; +}; + +const writeFakeExecutable = async (directory: string, name: string, body: string) => { + const path = join(directory, name); + await writeFile(path, body); + await chmod(path, 0o755); + return path; +}; + +const createTemporaryDirectory = async (prefix: string) => { + const directory = await realpath(await mkdtemp(join(tmpdir(), prefix))); + return { + directory, + [Symbol.asyncDispose]: () => rm(directory, { force: true, recursive: true }), + }; +}; + +const overridePath = (value: string) => { + const previousPath = process.env.PATH; + process.env.PATH = value; + return { + [Symbol.dispose]: () => { + process.env.PATH = previousPath; + }, + }; +}; + +const createDisposableFakeCommandLogger = async ( + prefix: string, + commandName: string, +): Promise> & AsyncDisposable> => { + const logger = await createFakeCommandLogger(prefix, commandName); + return { ...logger, [Symbol.asyncDispose]: logger.cleanup }; +}; + const git = async (repo: string, args: ReadonlyArray) => { await execFileAsync( 'git', @@ -329,6 +372,103 @@ test('parseArguments treats GitLab MR marker values as review sources', () => { }); }); +test('parseArguments treats the base keyword as a PR/MR base review', () => { + expect(parseArguments(['base'])).toEqual({ + commitRef: null, + help: false, + pullRequestNumber: null, + pullRequestUrl: null, + requestedPath: resolve(process.cwd()), + reviewBase: true, + version: false, + walkthrough: false, + }); + + expect(parseArguments(['BASE'])).toMatchObject({ reviewBase: true }); +}); + +test('parseArguments still lets --branch base review a literal base branch', async () => { + await withCwd(refRepositoryPath, () => { + expect(parseArguments(['--branch', 'base'])).toMatchObject({ + branchRef: 'base', + commitRef: null, + }); + expect(parseArguments(['--branch', 'base'])).not.toHaveProperty('reviewBase'); + }); +}); + +test('resolveBaseBranchRef reads the GitHub PR base branch through gh', async () => { + await using temporaryDirectory = await createTemporaryDirectory('codiff-base-github-'); + const repositoryPath = join(temporaryDirectory.directory, 'repo'); + const fakeBin = join(temporaryDirectory.directory, 'bin'); + + await mkdir(repositoryPath); + await mkdir(fakeBin); + await git(repositoryPath, ['init']); + await git(repositoryPath, ['remote', 'add', 'origin', 'git@github.com:owner/repo.git']); + await writeFakeExecutable(fakeBin, 'gh', '#!/bin/sh\nprintf "feature-base\\n"\n'); + using pathOverride = overridePath(`${fakeBin}:${process.env.PATH ?? ''}`); + void pathOverride; + + expect(resolveBaseBranchRef(repositoryPath)).toEqual({ + base: 'feature-base', + ref: 'origin/feature-base', + remote: 'origin', + }); +}); + +test('resolveBaseBranchRef reads the GitLab MR target branch through glab', async () => { + await using temporaryDirectory = await createTemporaryDirectory('codiff-base-gitlab-'); + const repositoryPath = join(temporaryDirectory.directory, 'repo'); + const fakeBin = join(temporaryDirectory.directory, 'bin'); + + await mkdir(repositoryPath); + await mkdir(fakeBin); + await git(repositoryPath, ['init']); + await git(repositoryPath, [ + 'remote', + 'add', + 'origin', + 'git@gitlab.example.com:group/project.git', + ]); + await writeFakeExecutable(fakeBin, 'glab', '#!/bin/sh\nprintf "release\\n"\n'); + using pathOverride = overridePath(`${fakeBin}:${process.env.PATH ?? ''}`); + void pathOverride; + + expect(resolveBaseBranchRef(repositoryPath)).toEqual({ + base: 'release', + ref: 'origin/release', + remote: 'origin', + }); +}); + +test('resolveBaseBranchRef falls back to the remote default branch without a request', async () => { + await using temporaryDirectory = await createTemporaryDirectory('codiff-base-fallback-'); + const repositoryPath = join(temporaryDirectory.directory, 'repo'); + const fakeBin = join(temporaryDirectory.directory, 'bin'); + + await mkdir(repositoryPath); + await mkdir(fakeBin); + await git(repositoryPath, ['init']); + await git(repositoryPath, ['config', 'user.email', 'codiff@example.com']); + await git(repositoryPath, ['config', 'user.name', 'Codiff Test']); + await git(repositoryPath, ['remote', 'add', 'origin', 'git@github.com:owner/repo.git']); + await git(repositoryPath, ['commit', '--allow-empty', '-m', 'init']); + const { stdout } = await execFileAsync('git', ['-C', repositoryPath, 'rev-parse', 'HEAD'], { + encoding: 'utf8', + }); + await git(repositoryPath, ['update-ref', 'refs/remotes/origin/main', stdout.trim()]); + await writeFakeExecutable(fakeBin, 'gh', '#!/bin/sh\nexit 1\n'); + using pathOverride = overridePath(`${fakeBin}:${process.env.PATH ?? ''}`); + void pathOverride; + + expect(resolveBaseBranchRef(repositoryPath)).toEqual({ + base: 'main', + ref: 'origin/main', + remote: 'origin', + }); +}); + test('parseArguments accepts nested GitLab merge request URLs', () => { expect( parseArguments(['https://gitlab.example.com/group/subgroup/project/-/merge_requests/23']), @@ -418,6 +558,19 @@ test('packaged terminal helper forwards GitLab MR markers to Electron', async () } }); +test('packaged terminal helper runs `base` through the bundled Node entry point', async () => { + await using logger = await createDisposableFakeCommandLogger('codiff-packaged-base-', 'runtime'); + + await execFileAsync(resolve('bin/codiff-app'), ['base'], { + env: { + ...logger.env, + CODIFF_NODE_COMMAND: logger.commandPath, + }, + }); + + expect(await logger.readArgs()).toEqual([resolve('bin/codiff.js'), 'base']); +}); + test('packaged terminal helper forwards HEAD^1 to Electron as a commit', async () => { const logger = await createFakeOpenLogger(); diff --git a/electron/review-source.cjs b/electron/review-source.cjs index f74902d..5771da4 100644 --- a/electron/review-source.cjs +++ b/electron/review-source.cjs @@ -104,6 +104,64 @@ const remotePriority = (remote) => ? 2 : 3; +const tryCommandOutput = (command, args, cwd) => { + try { + return execFileSync(command, args, { + cwd, + encoding: 'utf8', + stdio: ['ignore', 'pipe', 'ignore'], + timeout: 15_000, + }).trim(); + } catch { + return ''; + } +}; + +/** @param {string} repositoryPath */ +const getPrimaryReviewRemote = (repositoryPath) => + readReviewRemotes(repositoryPath).sort( + (left, right) => remotePriority(left) - remotePriority(right), + )[0]; + +/** @param {string} repositoryPath @param {string} remoteName */ +const resolveDefaultRemoteBranch = (repositoryPath, remoteName) => { + const head = tryCommandOutput( + 'git', + ['symbolic-ref', '--quiet', '--short', `refs/remotes/${remoteName}/HEAD`], + repositoryPath, + ); + const prefix = `${remoteName}/`; + if (head.startsWith(prefix)) { + return head.slice(prefix.length); + } + + return 'main'; +}; + +/** + * @param {string} repositoryPath + * @returns {{ base: string; ref: string; remote: string }} + */ +const resolveBaseBranchRef = (repositoryPath) => { + const remote = getPrimaryReviewRemote(repositoryPath); + const remoteName = remote.name; + const base = + remote.provider === 'gitlab' + ? tryCommandOutput( + 'glab', + ['mr', 'view', '--output', 'json', '--jq', '.target_branch'], + repositoryPath, + ) + : tryCommandOutput( + 'gh', + ['pr', 'view', '--json', 'baseRefName', '--jq', '.baseRefName'], + repositoryPath, + ); + + const branch = base || resolveDefaultRemoteBranch(repositoryPath, remoteName); + return { base: branch, ref: `${remoteName}/${branch}`, remote: remoteName }; +}; + /** * @param {string} repositoryPath * @param {number} number @@ -137,5 +195,6 @@ module.exports = { parseRemoteUrl, parseReviewUrl, readReviewRemotes, + resolveBaseBranchRef, resolveReviewUrl, };