Skip to content

Commit 13be4e8

Browse files
committed
codiff -w defaults to HEAD if there are no local changes.
1 parent 621d8b4 commit 13be4e8

12 files changed

Lines changed: 165 additions & 23 deletions

README.md

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -58,15 +58,18 @@ Full GitHub and GitLab review URLs are also supported. GitLab hosts and nested p
5858
derived from the URL or local Git remote and authenticated through `glab`; Codiff does not require
5959
instance-specific configuration.
6060

61-
Start with an LLM-generated narrative walkthrough:
61+
Start with an LLM-generated narrative walkthrough. When generating a walkthrough without an
62+
explicit target, Codiff uses local changes when present and falls back to `HEAD` when the working
63+
tree is clean:
6264

6365
```bash
6466
codiff -w
6567
codiff -w a1b2c3d
6668
```
6769

6870
When walkthrough sharing is available for your Git identity, generate and upload the same
69-
walkthrough without opening Codiff. The command prints the final URL:
71+
walkthrough without opening Codiff. The same default applies to generated walkthroughs, and the
72+
command prints the final URL:
7073

7174
```bash
7275
codiff --share

bin/arguments.js

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ export const flagDefinitions = [
4646
type: 'string',
4747
},
4848
{
49-
description: 'Generate and share a narrative walkthrough, then print its URL.',
49+
description: 'Generate and share a walkthrough, then print its URL; use HEAD when clean.',
5050
name: 'share',
5151
type: 'boolean',
5252
},
@@ -57,7 +57,7 @@ export const flagDefinitions = [
5757
type: 'boolean',
5858
},
5959
{
60-
description: 'Start with an LLM-generated narrative walkthrough.',
60+
description: 'Start an LLM walkthrough; use HEAD when there are no local changes.',
6161
name: 'walkthrough',
6262
short: 'w',
6363
type: 'boolean',
@@ -89,9 +89,9 @@ export const usageExamples = [
8989
{ command: "codiff '#75'", description: 'Review pull request #75.' },
9090
{ command: 'codiff pr 75', description: 'Review pull request #75 (alternate syntax).' },
9191
{ command: 'codiff mr 75', description: 'Review GitLab merge request !75.' },
92-
{ command: 'codiff -w', description: 'Start with an LLM narrative walkthrough.' },
92+
{ command: 'codiff -w', description: 'Walk through local changes, or HEAD when clean.' },
9393
{ command: 'codiff -w a1b2c3d', description: 'Generate a narrative walkthrough for a commit.' },
94-
{ command: 'codiff --share', description: 'Share an LLM narrative walkthrough.' },
94+
{ command: 'codiff --share', description: 'Share local changes, or HEAD when clean.' },
9595
{ command: 'codiff --share HEAD', description: 'Share a walkthrough for a commit.' },
9696
];
9797

bin/codiff-app

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -39,9 +39,9 @@ show_help() {
3939
printf ' %-38s%s%s%s\n' "--help, -h" "$gray" "Show this help message and exit." "$reset"
4040
printf ' %-38s%s%s%s\n' "--opencode-session <id>" "$gray" "Attach OpenCode session metadata to a walkthrough." "$reset"
4141
printf ' %-38s%s%s%s\n' "--pi-session <id>" "$gray" "Attach Pi session metadata to a walkthrough." "$reset"
42-
printf ' %-38s%s%s%s\n' "--share" "$gray" "Generate and share a narrative walkthrough, then print its URL." "$reset"
42+
printf ' %-38s%s%s%s\n' "--share" "$gray" "Generate and share a walkthrough, then print its URL; use HEAD when clean." "$reset"
4343
printf ' %-38s%s%s%s\n' "--version, -v" "$gray" "Show version number and exit." "$reset"
44-
printf ' %-38s%s%s%s\n' "--walkthrough, -w" "$gray" "Start with an LLM-generated narrative walkthrough." "$reset"
44+
printf ' %-38s%s%s%s\n' "--walkthrough, -w" "$gray" "Start an LLM walkthrough; use HEAD when there are no local changes." "$reset"
4545
printf ' %-38s%s%s%s\n' "--walkthrough-context <file>" "$gray" "Seed a walkthrough with Codex conversation context JSON." "$reset"
4646
printf ' %-38s%s%s%s\n' "--walkthrough-file <file>" "$gray" "Open a pre-authored narrative walkthrough JSON file." "$reset"
4747
printf ' %-38s%s%s%s\n' "--walkthrough-guide" "$gray" "Print the narrative walkthrough authoring guide and schema, then exit." "$reset"
@@ -54,9 +54,9 @@ show_help() {
5454
printf ' %-22s%s%s%s\n' "codiff '#75'" "$gray" "Review pull request #75." "$reset"
5555
printf ' %-22s%s%s%s\n' "codiff pr 75" "$gray" "Review pull request #75 (alternate syntax)." "$reset"
5656
printf ' %-22s%s%s%s\n' "codiff mr 75" "$gray" "Review GitLab merge request !75." "$reset"
57-
printf ' %-22s%s%s%s\n' "codiff -w" "$gray" "Start with an LLM narrative walkthrough." "$reset"
57+
printf ' %-22s%s%s%s\n' "codiff -w" "$gray" "Walk through local changes, or HEAD when clean." "$reset"
5858
printf ' %-22s%s%s%s\n' "codiff -w a1b2c3d" "$gray" "Walkthrough a specific commit." "$reset"
59-
printf ' %-22s%s%s%s\n' "codiff --share" "$gray" "Share an LLM narrative walkthrough." "$reset"
59+
printf ' %-22s%s%s%s\n' "codiff --share" "$gray" "Share local changes, or HEAD when clean." "$reset"
6060
printf ' %-22s%s%s%s\n' "codiff --share HEAD" "$gray" "Share a walkthrough for a commit." "$reset"
6161
}
6262

bin/codiff.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -164,7 +164,7 @@ const run = async () => {
164164
? { ref: commitRef, type: 'commit' }
165165
: branchRef
166166
? { ref: branchRef, type: 'branch' }
167-
: { type: 'working-tree' };
167+
: undefined;
168168

169169
try {
170170
const commonOptions = {

core/__tests__/codiff-cli.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1022,7 +1022,7 @@ test('formatHelpText styles titles and descriptions', () => {
10221022
expect(text).toContain('\u001b[90mShow this help message and exit.\u001b[0m');
10231023
expect(text).toContain(' codiff -w');
10241024
expect(text).not.toContain('\u001b[1;34mcodiff -w\u001b[0m');
1025-
expect(text).toContain('\u001b[90mStart with an LLM narrative walkthrough.\u001b[0m');
1025+
expect(text).toContain('\u001b[90mWalk through local changes, or HEAD when clean.\u001b[0m');
10261026
});
10271027

10281028
test('codiff-app prints help text and exits 0', async () => {

core/__tests__/codiff-share-cli.test.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -180,7 +180,7 @@ test('headless share uploads the canonical snapshot and prints its URL', async (
180180
}
181181
});
182182

183-
test('codiff --share generates a walkthrough for HEAD and prints only its URL', async () => {
183+
test('codiff --share falls back to HEAD for a clean working tree and prints only its URL', async () => {
184184
const directory = await mkdtemp(join(tmpdir(), 'codiff-generate-share-'));
185185
const repositoryPath = join(directory, 'repo');
186186
const fakeCodexPath = join(directory, 'codex');
@@ -285,7 +285,7 @@ exit 1
285285
const origin = `http://127.0.0.1:${(server.address() as { port: number }).port}`;
286286
const { stderr, stdout } = await execFileAsync(
287287
process.execPath,
288-
[resolve('bin/codiff.js'), '--share', 'HEAD', repositoryPath],
288+
[resolve('bin/codiff.js'), '--share', repositoryPath],
289289
{
290290
encoding: 'utf8',
291291
env: {

core/__tests__/git-state.test.ts

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,11 @@ type GitStateModule = {
8585
source?: ReviewSource,
8686
options?: { showWhitespace?: boolean },
8787
) => Promise<RepositoryState>;
88+
readWalkthroughRepositoryState: (
89+
launchPath: string,
90+
source?: ReviewSource,
91+
options?: { showWhitespace?: boolean },
92+
) => Promise<RepositoryState>;
8893
readWorkingTreeState: (
8994
launchPath: string,
9095
options?: { eagerContents?: boolean; showWhitespace?: boolean },
@@ -116,6 +121,7 @@ const {
116121
readDiffSectionContent,
117122
readRepositoryChangeSignature,
118123
readRepositoryState,
124+
readWalkthroughRepositoryState,
119125
readWorkingTreeState,
120126
resolvePullRequestContentRefs,
121127
selectUnresolvedReviewComments,
@@ -694,6 +700,35 @@ test('readRepositoryState and history handle fresh repositories', async () => {
694700
});
695701
});
696702

703+
test('readWalkthroughRepositoryState falls back to HEAD only for a clean implicit source', () =>
704+
withRepo(async (repo) => {
705+
await writeRepoFile(repo, 'example.txt', 'before\n');
706+
await commitAll(repo, 'initial commit');
707+
await writeRepoFile(repo, 'example.txt', 'after\n');
708+
await commitAll(repo, 'update example');
709+
710+
const cleanState = await readWalkthroughRepositoryState(repo);
711+
expect(cleanState.source).toMatchObject({ type: 'commit' });
712+
expect(cleanState.commitMetadata?.subject).toBe('update example');
713+
714+
const explicitWorkingTreeState = await readWalkthroughRepositoryState(repo, {
715+
type: 'working-tree',
716+
});
717+
expect(explicitWorkingTreeState.source).toEqual({ type: 'working-tree' });
718+
expect(explicitWorkingTreeState.files).toEqual([]);
719+
720+
await writeRepoFile(repo, 'example.txt', 'local\n');
721+
const dirtyState = await readWalkthroughRepositoryState(repo);
722+
expect(dirtyState.source).toEqual({ type: 'working-tree' });
723+
}));
724+
725+
test('readWalkthroughRepositoryState keeps a fresh repository on the working tree', () =>
726+
withRepo(async (repo) => {
727+
const state = await readWalkthroughRepositoryState(repo);
728+
expect(state.source).toEqual({ type: 'working-tree' });
729+
expect(state.files).toEqual([]);
730+
}));
731+
697732
test('readRepositoryState reports commit metadata for root commits', async () => {
698733
await withRepo(async (repo) => {
699734
await writeRepoFile(repo, 'notes/todo.txt', 'write tests\nship polish\n');

electron/__tests__/window-identity.test.ts

Lines changed: 29 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { execFile } from 'node:child_process';
2-
import { mkdir, mkdtemp, rm } from 'node:fs/promises';
2+
import { mkdir, mkdtemp, rm, writeFile } from 'node:fs/promises';
33
import { createRequire } from 'node:module';
44
import { tmpdir } from 'node:os';
55
import { join } from 'node:path';
@@ -28,6 +28,8 @@ const { findMatchingWindowIdentity, getWindowIdentity, parseGitHubPullRequestUrl
2828
type: 'pull-request';
2929
url: string;
3030
};
31+
walkthrough?: boolean;
32+
walkthroughFile?: string;
3133
},
3234
) => { key: string; repositoryRoot: string; sourceKey: string } | null;
3335
parseGitHubPullRequestUrl: (value: string) => {
@@ -91,6 +93,32 @@ test('window identities resolve commit refs to the same commit sha', async () =>
9193
}
9294
});
9395

96+
test('implicit walkthrough identities use HEAD only when the working tree is clean', async () => {
97+
const repositoryPath = await createRepository();
98+
99+
try {
100+
const headIdentity = getWindowIdentity(repositoryPath, {
101+
source: { ref: 'HEAD', type: 'commit' },
102+
});
103+
expect(getWindowIdentity(repositoryPath, { walkthrough: true })?.key).toBe(headIdentity?.key);
104+
105+
await writeFile(join(repositoryPath, 'local.txt'), 'change\n');
106+
expect(getWindowIdentity(repositoryPath, { walkthrough: true })?.sourceKey).toBe(
107+
'working-tree',
108+
);
109+
110+
await rm(join(repositoryPath, 'local.txt'));
111+
expect(
112+
getWindowIdentity(repositoryPath, {
113+
walkthrough: true,
114+
walkthroughFile: '/tmp/walkthrough.json',
115+
})?.sourceKey,
116+
).toBe('working-tree');
117+
} finally {
118+
await rm(repositoryPath, { force: true, recursive: true });
119+
}
120+
});
121+
94122
test('window identities distinguish branch history launches', async () => {
95123
const repositoryPath = await createRepository();
96124

electron/git-state.cjs

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,25 @@ const readRepositoryState = async (launchPath, source = { type: 'working-tree' }
8181
return { ...state, branch };
8282
};
8383

84+
/**
85+
* An implicit walkthrough reviews local changes when present and otherwise
86+
* reviews the current commit. Explicit sources always retain their semantics.
87+
*
88+
* @param {string} launchPath
89+
* @param {ReviewSource} [source]
90+
* @param {{showWhitespace?: boolean}} [options]
91+
* @returns {Promise<RepositoryState>}
92+
*/
93+
const readWalkthroughRepositoryState = async (launchPath, source, options = {}) => {
94+
const state = await readRepositoryState(launchPath, source, options);
95+
if (source || state.source.type !== 'working-tree' || state.files.length > 0) {
96+
return state;
97+
}
98+
99+
const head = (await gitOrEmpty(state.root, ['rev-parse', '--verify', 'HEAD'])).trim();
100+
return head ? readRepositoryState(launchPath, { ref: 'HEAD', type: 'commit' }, options) : state;
101+
};
102+
84103
/** @param {Extract<ReviewSource, {type: 'pull-request'}>} source */
85104
const isGitLabReviewSource = (source) =>
86105
source.provider === 'gitlab' || parseReviewUrl(source.url)?.provider === 'gitlab';
@@ -170,6 +189,7 @@ module.exports = {
170189
readCommitState,
171190
readPullRequestState,
172191
readRepositoryState,
192+
readWalkthroughRepositoryState,
173193
readWorkingTreeState,
174194
resolvePullRequestContentRefs,
175195
submitPullRequestComment: (launchPath, request) =>

electron/headless-walkthrough-share.cjs

Lines changed: 20 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,11 @@ const { userInfo } = require('node:os');
55
const { getAgent } = require('./agent.cjs');
66
const { readConfig, writeConfig } = require('./config.cjs');
77
const { createCloudflareAccessClient } = require('./cloudflare-access.cjs');
8-
const { readGitIdentity, readRepositoryState } = require('./git-state.cjs');
8+
const {
9+
readGitIdentity,
10+
readRepositoryState,
11+
readWalkthroughRepositoryState,
12+
} = require('./git-state.cjs');
913
const {
1014
normalizeNarrativeWalkthrough,
1115
readNarrativeWalkthrough,
@@ -106,6 +110,19 @@ const readShareState = async (repositoryPath, source, config) =>
106110
readGitIdentity(repositoryPath),
107111
]);
108112

113+
/**
114+
* @param {string} repositoryPath
115+
* @param {ReviewSource | undefined} source
116+
* @param {ReturnType<typeof readConfig>} config
117+
*/
118+
const readGeneratedShareState = async (repositoryPath, source, config) =>
119+
Promise.all([
120+
readWalkthroughRepositoryState(repositoryPath, source, {
121+
showWhitespace: config.settings.showWhitespace,
122+
}),
123+
readGitIdentity(repositoryPath),
124+
]);
125+
109126
/**
110127
* @param {{
111128
* agent?: 'claude' | 'codex' | 'opencode' | 'pi';
@@ -181,12 +198,12 @@ const generateAndShareWalkthrough = async ({
181198
piSessionId,
182199
repositoryPath,
183200
serviceUrlOverride,
184-
source = { type: 'working-tree' },
201+
source,
185202
walkthroughContextPath,
186203
}) => {
187204
const config = readConfig();
188205
const agent = getAgent(agentOverride || config.settings.agentBackend);
189-
const [state, uploader] = await readShareState(repositoryPath, source, config);
206+
const [state, uploader] = await readGeneratedShareState(repositoryPath, source, config);
190207
const sessionIds = {
191208
claudeSessionId,
192209
codexSessionId,

0 commit comments

Comments
 (0)