fix(pr-checks): tighten post-push refresh and branch detection

- Gate the post-push refresh suppression on a new hasFetched flag so the
  first fetch after a brand-new watcher is never swallowed; the old
  lastRefreshedAt==0 heuristic broke as soon as refreshPrChecksWatcher
  bumped the timestamp.
- Stop requiring the local branch head to match the PR head when
  detecting a branch PR (failed whenever the local branch was ahead).
  Pre-check that the worktree directory still exists before spawning gh,
  and only flip the gh-missing kill switch when the ENOENT actually
  points at gh — a vanished cwd no longer disables detection.
- Clear an auto-detected prUrl when the task branch is renamed so the
  next scan repopulates from the new branch.
- Renderer: extract a startWatcher helper used by both the createEffect
  pass and post-detection path, and add a disposed flag so async results
  don't mutate state after cleanup.
- TaskBranchInfoBar: label the Source and PR buttons, and keep the PR
  status indicator in a fixed-size slot so the row doesn't jitter as
  state moves between none/pending/settled.

Author: johannesjo

electron/ipc/pr-checks.test.ts

@@ -1,6 +1,10 @@
 import { describe, it, expect, vi, beforeEach } from 'vitest';
 import { promisify } from 'util';
 
+const { mockStat } = vi.hoisted(() => ({
+  mockStat: vi.fn(),
+}));
+
 vi.mock('child_process', () => {
   const mockExecFile = vi.fn();
   (mockExecFile as unknown as Record<symbol, unknown>)[promisify.custom] = (
@@ -17,6 +21,10 @@ vi.mock('child_process', () => {
   return { execFile: mockExecFile };
 });
 
+vi.mock('fs/promises', () => ({
+  stat: mockStat,
+}));
+
 vi.mock('electron', () => ({
   Notification: class {
     static isSupported(): boolean {
@@ -37,6 +45,9 @@ import {
   detectPrUrlForBranch,
   __resetForTests,
   __getStateForTests,
+  __runTickForTests,
+  initPrChecks,
+  refreshPrChecksWatcher,
   startPrChecksWatcher,
   type PrCheckRun,
 } from './pr-checks.js';
@@ -59,6 +70,23 @@ const run = (name: string, bucket: PrCheckRun['bucket']): PrCheckRun => ({
   bucket,
 });
 
+const flushPromises = (): Promise<void> => new Promise((resolve) => setImmediate(resolve));
+
+function fakeWindow(send: ReturnType<typeof vi.fn>): Parameters<typeof initPrChecks>[0] {
+  return {
+    on: vi.fn(),
+    isDestroyed: () => false,
+    isVisible: () => false,
+    webContents: { send },
+    show: vi.fn(),
+    focus: vi.fn(),
+  } as unknown as Parameters<typeof initPrChecks>[0];
+}
+
+beforeEach(() => {
+  mockStat.mockResolvedValue({ isDirectory: () => true });
+});
+
 describe('summarize', () => {
   it('empty list is none with zero counts', () => {
     expect(summarize([])).toEqual({ overall: 'none', passing: 0, pending: 0, failing: 0 });
@@ -206,71 +234,178 @@ describe('detectPrUrlForBranch', () => {
   });
 
   it('finds an open PR for a branch', async () => {
-    const calls = stubGh((_args, cb, cmd) => {
-      if (cmd === 'git') {
-        cb(null, 'abc123\n', '');
-        return;
-      }
+    const calls = stubGh((_args, cb) => {
       cb(
         null,
         JSON.stringify([
           {
             url: 'https://github.com/a/b/pull/11',
             headRefName: 'task/my-branch',
-            headRefOid: 'old',
-          },
-          {
-            url: 'https://github.com/a/b/pull/12',
-            headRefName: 'task/my-branch',
-            headRefOid: 'abc123',
           },
         ]),
         '',
       );
     });
     await expect(detectPrUrlForBranch('/repo/worktree', 'task/my-branch')).resolves.toBe(
-      'https://github.com/a/b/pull/12',
+      'https://github.com/a/b/pull/11',
     );
-    expect(calls[0]).toEqual(['rev-parse', 'task/my-branch']);
-    expect(calls[1]).toEqual([
+    expect(calls).toHaveLength(1);
+    expect(calls[0]).toEqual([
       'pr',
       'list',
       '--state',
       'open',
       '--head',
       'task/my-branch',
       '--json',
-      'url,headRefName,headRefOid',
+      'url,headRefName',
       '--limit',
       '20',
     ]);
   });
 
+  it('does not require the local branch head to match the PR head', async () => {
+    stubGh((_args, cb) =>
+      cb(
+        null,
+        JSON.stringify([
+          {
+            url: 'https://github.com/a/b/pull/12',
+            headRefName: 'task/ahead-locally',
+            headRefOid: 'remote-sha',
+          },
+        ]),
+        '',
+      ),
+    );
+    await expect(detectPrUrlForBranch('/repo/worktree', 'task/ahead-locally')).resolves.toBe(
+      'https://github.com/a/b/pull/12',
+    );
+  });
+
   it('returns null when the branch has no open PR', async () => {
-    stubGh((_args, cb, cmd) => cb(null, cmd === 'git' ? 'abc123\n' : JSON.stringify([]), ''));
+    stubGh((_args, cb) => cb(null, JSON.stringify([]), ''));
     await expect(detectPrUrlForBranch('/repo/worktree', 'task/no-pr')).resolves.toBe(null);
   });
 
+  it('returns null without invoking gh when the worktree path is gone', async () => {
+    const err = new Error('missing worktree') as NodeJS.ErrnoException;
+    err.code = 'ENOENT';
+    mockStat.mockRejectedValueOnce(err);
+    const calls = stubGh((_args, cb) => cb(null, JSON.stringify([]), ''));
+
+    await expect(detectPrUrlForBranch('/repo/missing-worktree', 'task/no-pr')).resolves.toBe(null);
+
+    expect(calls).toHaveLength(0);
+  });
+
   it('rejects malformed PR URLs from gh output', async () => {
-    stubGh((_args, cb, cmd) =>
+    stubGh((_args, cb) =>
       cb(
         null,
-        cmd === 'git'
-          ? 'abc123\n'
-          : JSON.stringify([
-              {
-                url: 'https://github.com/a/b/issues/12',
-                headRefName: 'task/issue',
-                headRefOid: 'abc123',
-              },
-            ]),
+        JSON.stringify([
+          {
+            url: 'https://github.com/a/b/issues/12',
+            headRefName: 'task/issue',
+          },
+        ]),
         '',
       ),
     );
     await expect(detectPrUrlForBranch('/repo/worktree', 'task/issue')).resolves.toBe(null);
   });
 });
 
+describe('refreshPrChecksWatcher', () => {
+  beforeEach(() => {
+    vi.clearAllMocks();
+    __resetForTests();
+  });
+
+  it('does not suppress the first fetched status after a post-push refresh', async () => {
+    const send = vi.fn();
+    initPrChecks(fakeWindow(send));
+    stubGh((_args, cb) =>
+      cb(
+        null,
+        JSON.stringify({
+          state: 'OPEN',
+          headRefOid: 'new-sha',
+          statusCheckRollup: [{ name: 'build', status: 'COMPLETED', conclusion: 'SUCCESS' }],
+        }),
+        '',
+      ),
+    );
+
+    startPrChecksWatcher({
+      taskId: 't1',
+      prUrl: 'https://github.com/a/b/pull/1',
+      taskName: 'test',
+    });
+    refreshPrChecksWatcher('t1');
+    await flushPromises();
+
+    expect(send).toHaveBeenCalledTimes(2);
+    expect(send.mock.calls[0][1]).toMatchObject({ taskId: 't1', overall: 'pending' });
+    expect(send.mock.calls[1][1]).toMatchObject({
+      taskId: 't1',
+      overall: 'success',
+      passing: 1,
+    });
+  });
+
+  it('keeps stale post-push status hidden until GitHub reports a new head', async () => {
+    const send = vi.fn();
+    initPrChecks(fakeWindow(send));
+    const statuses = [
+      {
+        state: 'OPEN',
+        headRefOid: 'old-sha',
+        statusCheckRollup: [{ name: 'build', status: 'COMPLETED', conclusion: 'SUCCESS' }],
+      },
+      {
+        state: 'OPEN',
+        headRefOid: 'old-sha',
+        statusCheckRollup: [{ name: 'build', status: 'COMPLETED', conclusion: 'SUCCESS' }],
+      },
+      {
+        state: 'OPEN',
+        headRefOid: 'new-sha',
+        statusCheckRollup: [{ name: 'build', status: 'COMPLETED', conclusion: 'SUCCESS' }],
+      },
+    ];
+    let nextStatus = 0;
+    stubGh((_args, cb) => {
+      const payload = statuses[Math.min(nextStatus, statuses.length - 1)];
+      nextStatus += 1;
+      cb(null, JSON.stringify(payload), '');
+    });
+
+    startPrChecksWatcher({
+      taskId: 't1',
+      prUrl: 'https://github.com/a/b/pull/1',
+      taskName: 'test',
+    });
+    await flushPromises();
+    expect(send).toHaveBeenCalledTimes(1);
+
+    refreshPrChecksWatcher('t1');
+    expect(send).toHaveBeenCalledTimes(2);
+    expect(send.mock.calls[1][1]).toMatchObject({ taskId: 't1', overall: 'pending' });
+
+    await __runTickForTests();
+    expect(send).toHaveBeenCalledTimes(2);
+
+    await __runTickForTests();
+    expect(send).toHaveBeenCalledTimes(3);
+    expect(send.mock.calls[2][1]).toMatchObject({
+      taskId: 't1',
+      overall: 'success',
+      passing: 1,
+    });
+  });
+});
+
 describe('startPrChecksWatcher — graceful degradation', () => {
   beforeEach(() => {
     vi.clearAllMocks();

electron/ipc/pr-checks.ts

@@ -1,6 +1,7 @@
 import { execFile } from 'child_process';
 import { promisify } from 'util';
 import { Notification, type BrowserWindow } from 'electron';
+import { stat } from 'fs/promises';
 import { IPC } from './channels.js';
 
 const exec = promisify(execFile);
@@ -40,6 +41,7 @@ interface TaskEntry {
   failing: number;
   checks: PrCheckRun[];
   headRefOid: string | null;
+  hasFetched: boolean;
   lastRefreshedAt: number;
   /** SHA at which we last fired a settled notification. null if never notified. */
   lastNotifiedSha: string | null;
@@ -114,6 +116,7 @@ export function startPrChecksWatcher(args: {
         failing: 0,
         checks: [],
         headRefOid: null,
+        hasFetched: false,
         lastRefreshedAt: 0,
         lastNotifiedSha: null,
         lastNotifiedOutcome: null,
@@ -136,6 +139,10 @@ export function refreshPrChecksWatcher(taskId: string): void {
   if (disabled) return;
   const entry = tasks.get(taskId);
   if (!entry) return;
+  // Deliberately leave entry.headRefOid in place: refreshOne uses it as the
+  // pre-push SHA to detect when GitHub has caught up. Clearing it here would
+  // make every fetch within the grace window look like a SHA change and let
+  // stale old-head check data through.
   entry.postPushRefreshUntil = Date.now() + POST_PUSH_REFRESH_GRACE_MS;
   entry.overall = 'pending';
   entry.passing = 0;
@@ -226,10 +233,13 @@ async function refreshOne(taskId: string): Promise<void> {
   const { overall, passing, pending, failing } = summarize(checks);
   const counts = { passing, pending, failing };
   const prevSha = entry.headRefOid;
-  const shaChanged = prevSha !== null && prevSha !== view.headRefOid;
-  const firstRefresh = entry.lastRefreshedAt === 0;
+  const shaChanged = entry.hasFetched && prevSha !== view.headRefOid;
+  const firstRefresh = !entry.hasFetched;
   const waitingForPostPushHead =
-    entry.postPushRefreshUntil !== null && !shaChanged && Date.now() < entry.postPushRefreshUntil;
+    entry.postPushRefreshUntil !== null &&
+    entry.hasFetched &&
+    !shaChanged &&
+    Date.now() < entry.postPushRefreshUntil;
   if (waitingForPostPushHead) {
     entry.lastRefreshedAt = Date.now();
     return;
@@ -257,6 +267,7 @@ async function refreshOne(taskId: string): Promise<void> {
   entry.failing = counts.failing;
   entry.checks = checks;
   entry.headRefOid = view.headRefOid;
+  entry.hasFetched = true;
   entry.lastRefreshedAt = Date.now();
 
   if (nothingChanged) return;
@@ -384,12 +395,7 @@ export async function detectPrUrlForBranch(
   worktreePath: string,
   branchName: string,
 ): Promise<string | null> {
-  const { stdout: headStdout } = await exec('git', ['rev-parse', branchName], {
-    cwd: worktreePath,
-    timeout: GH_TIMEOUT_MS,
-    maxBuffer: GH_MAX_BUFFER,
-  });
-  const localHead = headStdout.trim();
+  if (!(await isDirectory(worktreePath))) return null;
   const { stdout } = await exec(
     'gh',
     [
@@ -400,7 +406,7 @@ export async function detectPrUrlForBranch(
       '--head',
       branchName,
       '--json',
-      'url,headRefName,headRefOid',
+      'url,headRefName',
       '--limit',
       '20',
     ],
@@ -411,13 +417,21 @@ export async function detectPrUrlForBranch(
   const match = parsed.find((item) => {
     if (!item || typeof item !== 'object') return false;
     const pr = item as Record<string, unknown>;
-    return pr['headRefName'] === branchName && pr['headRefOid'] === localHead;
+    return pr['headRefName'] === branchName;
   });
   if (!match || typeof match !== 'object') return null;
   const url = (match as Record<string, unknown>)['url'];
   return typeof url === 'string' && isPrUrl(url) ? url : null;
 }
 
+async function isDirectory(path: string): Promise<boolean> {
+  try {
+    return (await stat(path)).isDirectory();
+  } catch {
+    return false;
+  }
+}
+
 /** Single gh call: combines PR state, head SHA, and check runs in one fork.
  *  Uses `statusCheckRollup` which bundles check-run data with a normalised
  *  conclusion, so we map that to the same `bucket` taxonomy `gh pr checks`
@@ -534,3 +548,7 @@ export function __getStateForTests(): {
 } {
   return { disabled, disabledReason, taskIds: Array.from(tasks.keys()) };
 }
+
+export function __runTickForTests(): Promise<void> {
+  return runTick();
+}