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(),
@@ -71,15 +71,18 @@ describe('Git Review snapshot authority', () => {
     );
     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,157 @@ 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, including legacy selections', 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');
+
+    for (const selection of ['refs/heads/release', 'release']) {
+      const result = await readGitReview(root, 'branch', undefined, selection);
+      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 ambiguous legacy 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 +270,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__/session-review-base-branch.test.ts

@@ -0,0 +1,165 @@
+/*
+ * 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, describe, it } from 'node:test';
+import { createElement } from 'react';
+import { renderToStaticMarkup } from 'react-dom/server';
+import {
+  persistSessionReviewBaseBranch,
+  readSessionReviewBaseBranch,
+  resolveAdoptedBaseBranch,
+  REVIEW_BASE_BRANCH_STORAGE_KEY,
+  reviewBaseBranchRequestValue,
+  SessionReviewBaseBranchPicker,
+} from '../../renderer/features/workbar/testing.js';
+
+function installMemoryLocalStorage(initial: Record<string, string> = {}) {
+  const store = new Map<string, string>(Object.entries(initial));
+  const previous = Object.getOwnPropertyDescriptor(globalThis, 'localStorage');
+  const memory: Storage = {
+    get length() {
+      return store.size;
+    },
+    clear: () => store.clear(),
+    getItem: (key) => store.get(key) ?? null,
+    key: (index) => [...store.keys()][index] ?? null,
+    removeItem: (key) => store.delete(key),
+    setItem: (key, value) => store.set(key, String(value)),
+  };
+  Object.defineProperty(globalThis, 'localStorage', {
+    configurable: true,
+    writable: true,
+    value: memory,
+  });
+  return () => {
+    if (previous) Object.defineProperty(globalThis, 'localStorage', previous);
+    else Reflect.deleteProperty(globalThis, 'localStorage');
+  };
+}
+
+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', () => {
+  const cleanups: Array<() => void> = [];
+  afterEach(() => {
+    while (cleanups.length > 0) cleanups.pop()?.();
+  });
+
+  it('omits the request value until a branch is selected', () => {
+    assert.equal(reviewBaseBranchRequestValue(null), undefined);
+    assert.equal(reviewBaseBranchRequestValue('origin/develop'), 'origin/develop');
+  });
+
+  it('adopts the resolved base branch only when the backend offers it', () => {
+    const options = [
+      { label: 'main', value: 'refs/heads/main' },
+      { label: 'origin/develop', value: 'refs/remotes/origin/develop' },
+    ];
+    assert.equal(
+      resolveAdoptedBaseBranch('refs/remotes/origin/develop', {
+        baseBranch: 'refs/heads/main',
+        baseBranchOptions: options,
+      }),
+      'refs/remotes/origin/develop',
+    );
+    assert.equal(
+      resolveAdoptedBaseBranch(null, { baseBranch: 'refs/heads/main', baseBranchOptions: options }),
+      null,
+    );
+    // A resolved branch the backend would reject on the next read stays unpinned.
+    assert.equal(
+      resolveAdoptedBaseBranch('origin/gone', {
+        baseBranch: 'origin/gone',
+        baseBranchOptions: options,
+      }),
+      null,
+    );
+    assert.equal(
+      resolveAdoptedBaseBranch(null, { baseBranch: null, baseBranchOptions: options }),
+      null,
+    );
+  });
+
+  it('persists the canonical selection when the backend migrates a legacy name', () => {
+    cleanups.push(installMemoryLocalStorage());
+    persistSessionReviewBaseBranch('legacy', 'main');
+    const adopted = resolveAdoptedBaseBranch(readSessionReviewBaseBranch('legacy'), {
+      baseBranch: 'refs/heads/main',
+      baseBranchOptions: [{ label: 'main', value: 'refs/heads/main' }],
+    });
+    assert.equal(adopted, 'refs/heads/main');
+    persistSessionReviewBaseBranch('legacy', adopted);
+    assert.equal(readSessionReviewBaseBranch('legacy'), 'refs/heads/main');
+  });
+
+  it('pins the branch per Session and survives corrupt storage', () => {
+    cleanups.push(installMemoryLocalStorage());
+    persistSessionReviewBaseBranch('session-a', 'origin/develop');
+    persistSessionReviewBaseBranch('session-b', 'main');
+    assert.equal(readSessionReviewBaseBranch('session-a'), 'origin/develop');
+    assert.equal(readSessionReviewBaseBranch('session-b'), 'main');
+
+    persistSessionReviewBaseBranch('session-a', null);
+    assert.equal(readSessionReviewBaseBranch('session-a'), null);
+    assert.equal(readSessionReviewBaseBranch('session-b'), 'main');
+
+    localStorage.setItem(REVIEW_BASE_BRANCH_STORAGE_KEY, '{ not json');
+    assert.equal(readSessionReviewBaseBranch('session-b'), null);
+    localStorage.setItem(
+      REVIEW_BASE_BRANCH_STORAGE_KEY,
+      JSON.stringify({ 'session-c': 7, 'session-d': 'main' }),
+    );
+    assert.equal(readSessionReviewBaseBranch('session-c'), null);
+    assert.equal(readSessionReviewBaseBranch('session-d'), 'main');
+  });
+
+  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));
+  });
+});