feat(desktop): let the Git changes panel pick its comparison base branch

CHANGELOG.md

@@ -32,6 +32,9 @@
   offer an action to open the most recently imported task or create another
   independent import. An uncertain result remains visible as a warning while
   still allowing a user-initiated import again in both Desktop and TUI.
+- The Git changes panel can pick which branch it compares against — searchable,
+  per Session — instead of only the branch the backend resolves. The choice
+  persists, and falls back to the resolved branch when the pinned one disappears.
 - Added `/transcript` to browse long TUI sessions without depending on terminal
   scrollback, with line, page, and first/last navigation.
 

apps/desktop/src/main/__tests__/git-review-main.test.ts

@@ -51,11 +51,11 @@ describe('Git Review snapshot authority', () => {
     const branch = await readGitReview(root, 'branch');
     assert.equal(branch.ok, true);
     if (!branch.ok) return;
-    assert.equal(branch.snapshot.baseBranch, 'main');
+    assert.equal(branch.snapshot.baseBranch, 'refs/heads/main');
     assert.equal(branch.snapshot.currentBranch, 'feature/review');
     assert.deepEqual(branch.snapshot.baseBranchOptions, [
-      'feature/review',
-      'main',
+      { label: 'main', value: 'refs/heads/main' },
+      { label: 'feature/review', value: 'refs/heads/feature/review' },
     ]);
     assert.deepEqual(
       branch.snapshot.files.map((file) => file.path).sort(),
@@ -67,19 +67,22 @@ describe('Git Review snapshot authority', () => {
       root,
       'branch',
       undefined,
-      'feature/review',
+      'refs/heads/feature/review',
     );
     assert.equal(currentBranchOnly.ok, true);
     if (currentBranchOnly.ok) {
-      assert.equal(currentBranchOnly.snapshot.baseBranch, 'feature/review');
+      assert.equal(currentBranchOnly.snapshot.baseBranch, 'refs/heads/feature/review');
       assert.equal(
         currentBranchOnly.snapshot.files.some((file) => file.path === 'feature.txt'),
         false,
       );
     }
     assert.deepEqual(
       await readGitReview(root, 'branch', undefined, 'missing-branch'),
-      { ok: false, reason: 'invalid_base_branch' },
+      { ok: false, reason: 'invalid_base_branch', branches: {
+        currentBranch: branch.snapshot.currentBranch,
+        baseBranchOptions: branch.snapshot.baseBranchOptions,
+      } },
     );
 
     const unstaged = await readGitReview(root, 'unstaged');
@@ -99,6 +102,155 @@ describe('Git Review snapshot authority', () => {
     );
   });
 
+  it('lists the remote default branch before the branches it resolves from', async () => {
+    const origin = await repository();
+    await git(origin, 'branch', 'release/0.1');
+    const root = await temporaryRoot();
+    await git(root, 'clone', origin, '.');
+
+    const result = await readGitReview(root, 'branch');
+    assert.equal(result.ok, true);
+    if (!result.ok) return;
+    assert.equal(result.snapshot.baseBranch, 'refs/remotes/origin/main');
+    assert.deepEqual(result.snapshot.baseBranchOptions, [
+      { label: 'origin/HEAD', value: 'refs/remotes/origin/HEAD' },
+      { label: 'origin/main', value: 'refs/remotes/origin/main' },
+      { label: 'main', value: 'refs/heads/main' },
+      { label: 'origin/release/0.1', value: 'refs/remotes/origin/release/0.1' },
+    ]);
+  });
+
+  it('compares the branch rather than a same-named tag, using a qualified ref', async () => {
+    const root = await repository();
+    await git(root, 'tag', 'release');
+    await git(root, 'checkout', '-b', 'release');
+    await writeFile(join(root, 'release.txt'), 'release\n', 'utf8');
+    await git(root, 'add', '.');
+    await git(root, 'commit', '-m', 'release');
+    await git(root, 'checkout', '-b', 'feature');
+    await writeFile(join(root, 'feature.txt'), 'feature\n', 'utf8');
+    await git(root, 'add', '.');
+    await git(root, 'commit', '-m', 'feature');
+
+    const result = await readGitReview(root, 'branch', undefined, 'refs/heads/release');
+    assert.equal(result.ok, true);
+    if (!result.ok) return;
+    assert.equal(result.snapshot.baseBranch, 'refs/heads/release');
+    assert.deepEqual(result.snapshot.files.map((file) => file.path), ['feature.txt']);
+    assert.ok(result.snapshot.baseBranchOptions.every((option) =>
+      option.value.startsWith('refs/heads/') || option.value.startsWith('refs/remotes/')));
+    assert.deepEqual(await readGitReview(root, 'branch', undefined, 'refs/tags/release'),
+      { ok: false, reason: 'invalid_base_branch', branches: { currentBranch: 'feature', baseBranchOptions: [
+        { label: 'main', value: 'refs/heads/main' },
+        { label: 'feature', value: 'refs/heads/feature' },
+        { label: 'release', value: 'refs/heads/release' },
+      ] } });
+    await git(root, 'tag', 'tag-only');
+    assert.deepEqual(await readGitReview(root, 'branch', undefined, 'tag-only'),
+      { ok: false, reason: 'invalid_base_branch', branches: { currentBranch: 'feature', baseBranchOptions: [
+        { label: 'main', value: 'refs/heads/main' },
+        { label: 'feature', value: 'refs/heads/feature' },
+        { label: 'release', value: 'refs/heads/release' },
+      ] } });
+  });
+
+  it('keeps local and remote refs with the same label distinct and rejects unqualified names', async () => {
+    const root = await repository();
+    await git(root, 'update-ref', 'refs/remotes/origin/release', 'HEAD');
+    await git(root, 'checkout', '-b', 'origin/release');
+    await writeFile(join(root, 'local.txt'), 'local\n', 'utf8');
+    await git(root, 'add', '.');
+    await git(root, 'commit', '-m', 'local');
+    const local = await readGitReview(root, 'branch', undefined, 'refs/heads/origin/release');
+    const remote = await readGitReview(root, 'branch', undefined, 'refs/remotes/origin/release');
+    assert.equal(local.ok, true);
+    assert.equal(remote.ok, true);
+    if (!local.ok || !remote.ok) return;
+    assert.equal(local.snapshot.files.length, 0);
+    assert.deepEqual(remote.snapshot.files.map((file) => file.path), ['local.txt']);
+    assert.equal(local.snapshot.baseBranchOptions.filter((option) => option.label === 'origin/release').length, 2);
+    assert.deepEqual(await readGitReview(root, 'branch', undefined, 'origin/release'),
+      { ok: false, reason: 'invalid_base_branch', branches: {
+        currentBranch: local.snapshot.currentBranch,
+        baseBranchOptions: local.snapshot.baseBranchOptions,
+      } });
+  });
+
+  it('resolves the default branch without following a same-named tag', async () => {
+    const root = await repository();
+    await git(root, 'tag', 'main');
+    await writeFile(join(root, 'main.txt'), 'main\n', 'utf8');
+    await git(root, 'add', '.');
+    await git(root, 'commit', '-m', 'advance main');
+    await git(root, 'checkout', '-b', 'feature');
+    const result = await readGitReview(root, 'branch');
+    assert.equal(result.ok, true);
+    if (!result.ok) return;
+    assert.equal(result.snapshot.baseBranch, 'refs/heads/main');
+    assert.equal(result.snapshot.files.length, 0);
+  });
+
+  it('returns branch choices when unrelated history prevents a diff, including on repeated reads', async () => {
+    const root = await repository();
+    await git(root, 'checkout', '--orphan', 'gh-pages');
+    await git(root, 'rm', '-rf', '.');
+    await writeFile(join(root, 'index.html'), 'site\n', 'utf8');
+    await git(root, 'add', '.');
+    await git(root, 'commit', '-m', 'independent site history');
+    await git(root, 'checkout', 'main');
+
+    for (let attempt = 0; attempt < 2; attempt += 1) {
+      const result = await readGitReview(root, 'branch', undefined, 'refs/heads/gh-pages');
+      assert.equal(result.ok, false);
+      if (result.ok) return;
+      assert.equal(result.reason, 'git_failed');
+      assert.deepEqual(result.branches, {
+        currentBranch: 'main',
+        baseBranchOptions: [
+          { label: 'main', value: 'refs/heads/main' },
+          { label: 'gh-pages', value: 'refs/heads/gh-pages' },
+        ],
+      });
+    }
+    const recovered = await readGitReview(root, 'branch', undefined, 'refs/heads/main');
+    assert.equal(recovered.ok, true);
+    if (recovered.ok) assert.equal(recovered.snapshot.files.length, 0);
+  });
+
+  it('degrades a diff that overflows the git buffer to a truncated review', async () => {
+    const root = await repository();
+    await git(root, 'checkout', '-b', 'feature');
+    await writeFile(join(root, 'feature.txt'), 'feature\n', 'utf8');
+    await git(root, 'add', '.');
+    await git(root, 'commit', '-m', 'feature');
+
+    const result = await readGitReview(root, 'branch', async (gitRoot, args) => {
+      if (args.includes('--binary')) {
+        // Node rejects an over-limit child buffer, handing back what it read.
+        throw Object.assign(new Error('stdout maxBuffer length exceeded'), {
+          code: 'ERR_CHILD_PROCESS_STDIO_MAXBUFFER',
+          stdout: [
+            'diff --git a/feature.txt b/feature.txt',
+            'new file mode 100644',
+            '--- /dev/null',
+            '+++ b/feature.txt',
+            '@@ -0,0 +1 @@',
+            '+feature',
+            '',
+          ].join('\n'),
+        });
+      }
+      const { stdout } = await execFileAsync('git', ['-C', gitRoot, ...args], {
+        encoding: 'utf8',
+      });
+      return stdout;
+    });
+    assert.equal(result.ok, true);
+    if (!result.ok) return;
+    assert.equal(result.snapshot.truncated, true);
+    assert.deepEqual(result.snapshot.files.map((file) => file.path), ['feature.txt']);
+  });
+
   it('returns an explicit non-repository outcome', async () => {
     const root = await temporaryRoot();
     assert.deepEqual(await readGitReview(root, 'branch'), {
@@ -116,7 +268,7 @@ describe('Git Review snapshot authority', () => {
     const result = await readGitReview(root, 'branch');
     assert.equal(result.ok, true);
     if (!result.ok) return;
-    assert.equal(result.snapshot.baseBranch, null);
+    assert.equal(result.snapshot.baseBranch, 'refs/heads/main');
     assert.deepEqual(
       result.snapshot.files.map((file) => file.path).sort(),
       ['base.txt', 'staged.txt'],

apps/desktop/src/main/__tests__/review-base-branch-preferences.test.ts

@@ -0,0 +1,101 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import assert from 'node:assert/strict';
+import { afterEach, beforeEach, describe, it } from 'node:test';
+import {
+  createReviewBaseBranchPreferences,
+  REVIEW_BASE_BRANCH_STORAGE_KEY,
+} from '../../renderer/platform/desktop/review-base-branch-preferences.js';
+
+function memoryStorage() {
+  const items = new Map<string, string>();
+  return {
+    getItem: (key: string) => items.get(key) ?? null,
+    setItem: (key: string, value: string) => { items.set(key, value); },
+  };
+}
+
+describe('Desktop review base branch preferences', () => {
+  let storage: ReturnType<typeof memoryStorage>;
+  let originalStorage: PropertyDescriptor | undefined;
+
+  beforeEach(() => {
+    originalStorage = Object.getOwnPropertyDescriptor(globalThis, 'localStorage');
+    storage = memoryStorage();
+    Object.defineProperty(globalThis, 'localStorage', { configurable: true, value: storage });
+  });
+
+  afterEach(() => {
+    if (originalStorage) Object.defineProperty(globalThis, 'localStorage', originalStorage);
+    else Reflect.deleteProperty(globalThis, 'localStorage');
+  });
+
+  it('persists explicit refs across adapter instances without pinning new Sessions', () => {
+    const preferences = createReviewBaseBranchPreferences();
+    preferences.write('session-a', 'refs/remotes/origin/develop');
+    preferences.write('session-b', 'refs/heads/main');
+    const reopened = createReviewBaseBranchPreferences();
+    assert.equal(reopened.read('session-a'), 'refs/remotes/origin/develop');
+    assert.equal(reopened.read('session-b'), 'refs/heads/main');
+    assert.equal(reopened.read('new-session'), null);
+    reopened.write('session-a', null);
+    assert.equal(preferences.read('session-a'), null);
+    assert.equal(preferences.read('session-b'), 'refs/heads/main');
+  });
+
+  it('ignores malformed records and recovers on the next explicit choice', () => {
+    const preferences = createReviewBaseBranchPreferences();
+    for (const invalid of ['{ not json', 'null', '[]', '3']) {
+      storage.setItem(REVIEW_BASE_BRANCH_STORAGE_KEY, invalid);
+      assert.equal(preferences.read('session'), null);
+      preferences.write('session', 'refs/heads/main');
+      assert.equal(preferences.read('session'), 'refs/heads/main');
+    }
+    storage.setItem(REVIEW_BASE_BRANCH_STORAGE_KEY, JSON.stringify({
+      empty: '', invalid: 7, valid: 'refs/heads/main',
+    }));
+    assert.equal(preferences.read('empty'), null);
+    assert.equal(preferences.read('invalid'), null);
+    assert.equal(preferences.read('valid'), 'refs/heads/main');
+    preferences.write('another', 'refs/heads/develop');
+    assert.deepEqual(JSON.parse(storage.getItem(REVIEW_BASE_BRANCH_STORAGE_KEY)!), {
+      valid: 'refs/heads/main', another: 'refs/heads/develop',
+    });
+  });
+
+  it('handles a restricted localStorage getter lazily', () => {
+    Object.defineProperty(globalThis, 'localStorage', {
+      configurable: true,
+      get() { throw new Error('Storage access denied'); },
+    });
+    const preferences = createReviewBaseBranchPreferences();
+    assert.equal(preferences.read('session'), null);
+    assert.doesNotThrow(() => preferences.write('session', 'refs/heads/main'));
+    assert.doesNotThrow(() => preferences.write('session', null));
+  });
+
+  it('handles read failures and quota errors without breaking comparisons', () => {
+    storage.getItem = () => { throw new Error('Read denied'); };
+    storage.setItem = () => { throw new Error('Quota exceeded'); };
+    const preferences = createReviewBaseBranchPreferences();
+    assert.equal(preferences.read('session'), null);
+    assert.doesNotThrow(() => preferences.write('session', 'refs/heads/main'));
+  });
+});

apps/desktop/src/main/__tests__/session-review-base-branch.test.ts

@@ -0,0 +1,63 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import assert from 'node:assert/strict';
+import { describe, it } from 'node:test';
+import { createElement } from 'react';
+import { renderToStaticMarkup } from 'react-dom/server';
+import {
+  SessionReviewBaseBranchPicker,
+} from '../../renderer/features/workbar/testing.js';
+
+const AUTO_SENTINEL = 'AUTO_SENTINEL';
+
+function renderPicker(baseBranch: string | null) {
+  return renderToStaticMarkup(
+    createElement(SessionReviewBaseBranchPicker, {
+      baseBranch,
+      baseBranchOptions: [
+        { label: 'main', value: 'refs/heads/main' },
+        { label: 'origin/develop', value: 'refs/remotes/origin/develop' },
+      ],
+      label: AUTO_SENTINEL,
+      onSelect: () => undefined,
+    }),
+  );
+}
+
+/** The visible trigger only: `label` is required by Selector and always lands
+ * in the markup as a visually hidden element, sentinel and all. */
+function renderTrigger(baseBranch: string | null) {
+  const markup = renderPicker(baseBranch);
+  const start = markup.indexOf('<button');
+  return markup.slice(start, markup.indexOf('</button>', start));
+}
+
+describe('session review base branch', () => {
+  it('shows the compared branch instead of an auto pseudo-entry', () => {
+    const trigger = renderTrigger('refs/remotes/origin/develop');
+    assert.match(trigger, />origin\/develop</);
+    assert.doesNotMatch(trigger, /refs\/remotes/);
+    assert.doesNotMatch(trigger, new RegExp(AUTO_SENTINEL));
+  });
+
+  it('falls back to the plain label while nothing is pinned', () => {
+    assert.match(renderTrigger(null), new RegExp(AUTO_SENTINEL));
+  });
+});