Skip to content
Draft
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
12 changes: 12 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
23 changes: 16 additions & 7 deletions bin/arguments.js
Original file line number Diff line number Diff line change
Expand Up @@ -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).' },
Expand Down Expand Up @@ -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) =>
Expand Down Expand Up @@ -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 =
Expand Down Expand Up @@ -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;
}
}
}

Expand All @@ -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,
Expand Down
29 changes: 16 additions & 13 deletions bin/codiff-app
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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=""
Expand Down
20 changes: 17 additions & 3 deletions bin/codiff.js
Original file line number Diff line number Diff line change
@@ -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';
Expand All @@ -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
Expand Down Expand Up @@ -119,7 +120,6 @@ const run = async () => {

const {
agentBackend,
branchRef,
claudeSessionId,
codexSessionId,
commitRef,
Expand All @@ -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`);
Expand Down
153 changes: 153 additions & 0 deletions core/__tests__/codiff-cli.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -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<Awaited<ReturnType<typeof createFakeCommandLogger>> & AsyncDisposable> => {
const logger = await createFakeCommandLogger(prefix, commandName);
return { ...logger, [Symbol.asyncDispose]: logger.cleanup };
};

const git = async (repo: string, args: ReadonlyArray<string>) => {
await execFileAsync(
'git',
Expand Down Expand Up @@ -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']),
Expand Down Expand Up @@ -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();

Expand Down
Loading