refactor: make GitHub PR status more reliable and up to date (#613)

* fix: avoid incorrect PR status on default branch Hide sidebar PR status when current branch is the repo default branch. Tighten PR fallback matching to avoid picking unrelated PRs with the same branch name. * fix: make sidebar PR status more reliable Resolve PRs more reliably across remotes and repo networks. Persist sidebar PR status across tab reloads while keeping updates fresh. Reduce noisy GitHub errors and correct blocked styling in PR view. * fix: correct blocked PR styling in sidebar Stop treating missing merge permissions as a blocked PR state in the sidebar. Align sidebar PR status colors and labels with the main PR view. * refactor: make GitHub PR status updates feel more current Refresh PR state on focus, visibility, and repo changes Add follow-up PR refreshes after create, merge, ready, and update actions Document GitHub PR resolution, consumers, polling, and triggers
2026-03-11 23:52:31 +02:00
parent 180cd57661
commit d1650343b1
9 changed files with 1462 additions and 584 deletions
@@ -100,7 +100,8 @@ const getPrVisualState = (status: GitHubPullRequestStatus | null): PrVisualState
    return 'draft';
  }
  const checksFailed = status?.checks?.state === 'failure';
-  const notMergeable = status?.canMerge === false || pr.mergeable === false;
+  const mergeableState = typeof pr.mergeableState === 'string' ? pr.mergeableState : '';
+  const notMergeable = pr.mergeable === false || mergeableState === 'blocked' || mergeableState === 'dirty';
  if (checksFailed || notMergeable) {
    return 'blocked';
  }
@@ -847,8 +848,8 @@ export const SessionSidebar: React.FC<SessionSidebarProps> = ({
    const result = new Map<string, PrIndicator>();

    Object.values(prStatusEntries).forEach((entry) => {
-      const directory = normalizePath(entry.params?.directory ?? null);
-      const branch = entry.params?.branch?.trim();
+      const directory = normalizePath(entry.params?.directory ?? entry.identity?.directory ?? null);
+      const branch = entry.params?.branch?.trim() ?? entry.identity?.branch?.trim();
      if (!directory || !branch) {
        return;
      }
@@ -232,9 +232,9 @@ export function SessionGroupSection(props: Props): React.ReactNode {
    ].filter((item): item is string => Boolean(item)).join(', ')
    : null;
  const mergeabilityLabel = prIndicator && prIndicator.state === 'open'
-    ? (prIndicator.canMerge === true
-        ? 'Mergeable'
-        : (prIndicator.canMerge === false ? 'Conflicts or blocked' : null))
+    ? (prIndicator.mergeableState === 'blocked' || prIndicator.mergeableState === 'dirty'
+        ? 'Conflicts or blocked'
+        : (prIndicator.mergeableState === 'clean' || prIndicator.canMerge === true ? 'Mergeable' : null))
    : null;
  const mergeStateLabel = prIndicator && prIndicator.state === 'open' && prIndicator.mergeableState
    ? `Merge state: ${prIndicator.mergeableState}`
@@ -93,13 +93,16 @@ const getPrVisualState = (status: GitHubPullRequestStatus | null): 'draft' | 'op
    return 'draft';
  }
  const checksFailed = status?.checks?.state === 'failure';
-  const notMergeable = status?.canMerge === false || pr.mergeable === false;
+  const mergeableState = typeof pr.mergeableState === 'string' ? pr.mergeableState : '';
+  const notMergeable = pr.mergeable === false || mergeableState === 'blocked' || mergeableState === 'dirty';
  if (checksFailed || notMergeable) {
    return 'blocked';
  }
  return 'open';
 };

+const PR_ACTION_REFRESH_DELAYS_MS = [2_000, 5_000] as const;
+
 const branchToTitle = (branch: string): string => {
  return branch
    .replace(/^refs\/heads\//, '')
@@ -449,6 +452,7 @@ export const PullRequestSection: React.FC<{
  const lastSyncedPrNumberRef = React.useRef<number | null>(null);
  const didUserOverrideRemoteRef = React.useRef(false);
  const autoRemoteProbeDoneRef = React.useRef<Set<string>>(new Set());
+  const pendingActionRefreshTimersRef = React.useRef<number[]>([]);

  const canShow = Boolean(directory && branch && baseBranch && branch !== baseBranch);

@@ -957,6 +961,15 @@ export const PullRequestSection: React.FC<{
    await refreshPrStatus(prStatusKey, options);
  }, [prStatusKey, refreshPrStatus]);

+  const scheduleActionRefresh = React.useCallback(() => {
+    pendingActionRefreshTimersRef.current.forEach((timerId) => {
+      window.clearTimeout(timerId);
+    });
+    pendingActionRefreshTimersRef.current = PR_ACTION_REFRESH_DELAYS_MS.map((delayMs) => window.setTimeout(() => {
+      void refresh({ force: true, silent: true, markInitialResolved: true });
+    }, delayMs));
+  }, [refresh]);
+
  // Refetch PR status when selected remote changes
  const handleRemoteChange = React.useCallback((remote: GitRemote) => {
    didUserOverrideRemoteRef.current = true;
@@ -1069,6 +1082,18 @@ export const PullRequestSection: React.FC<{
    void refresh({ force: true, silent: true, markInitialResolved: true });
  }, [canShow, refresh, selectedRemote?.name]);

+  React.useEffect(() => {
+    const resolvedRemoteName = status?.resolvedRemoteName?.trim();
+    if (!resolvedRemoteName || didUserOverrideRemoteRef.current) {
+      return;
+    }
+    const resolvedRemote = remotes.find((candidate) => candidate.name === resolvedRemoteName);
+    if (!resolvedRemote) {
+      return;
+    }
+    setSelectedRemote((prev) => (prev?.name === resolvedRemote.name ? prev : resolvedRemote));
+  }, [remotes, status?.resolvedRemoteName]);
+
  React.useEffect(() => {
    const isTerminal = status?.pr?.state === 'closed' || status?.pr?.state === 'merged';
    const lastRefreshAt = statusEntry?.lastRefreshAt ?? 0;
@@ -1116,6 +1141,16 @@ export const PullRequestSection: React.FC<{
    });
  }, [snapshotKey, title, body, draft, additionalContext, targetBaseBranch, selectedRemote?.name, directory, branch]);

+  React.useEffect(() => {
+    const pendingActionRefreshTimers = pendingActionRefreshTimersRef.current;
+    return () => {
+      pendingActionRefreshTimers.forEach((timerId) => {
+        window.clearTimeout(timerId);
+      });
+      pendingActionRefreshTimersRef.current = [];
+    };
+  }, []);
+
  const generateDescription = React.useCallback(async () => {
    if (isGenerating) return;
    if (!directory) return;
@@ -1182,13 +1217,14 @@ export const PullRequestSection: React.FC<{
      toast.success('PR created');
      updatePrStatus(prStatusKey, (prev) => (prev ? { ...prev, pr } : prev));
      await refresh({ force: true });
+      scheduleActionRefresh();
    } catch (e) {
      const message = e instanceof Error ? e.message : String(e);
      toast.error('Failed to create PR', { description: message });
    } finally {
      setIsCreating(false);
    }
-  }, [body, branch, directory, draft, github, prStatusKey, refresh, selectedRemote, targetBaseBranch, title, updatePrStatus]);
+  }, [body, branch, directory, draft, github, prStatusKey, refresh, scheduleActionRefresh, selectedRemote, targetBaseBranch, title, updatePrStatus]);

  const mergePr = React.useCallback(async (pr: GitHubPullRequest) => {
    if (!github?.prMerge) {
@@ -1204,6 +1240,7 @@ export const PullRequestSection: React.FC<{
        toast.message('PR not merged', { description: result.message || 'Not mergeable' });
      }
      await refresh({ force: true });
+      scheduleActionRefresh();
    } catch (e) {
      const message = e instanceof Error ? e.message : String(e);
      toast.error('Merge failed', { description: message });
@@ -1213,7 +1250,7 @@ export const PullRequestSection: React.FC<{
    } finally {
      setIsMerging(false);
    }
-  }, [directory, github, mergeMethod, refresh]);
+  }, [directory, github, mergeMethod, refresh, scheduleActionRefresh]);

  const markReady = React.useCallback(async (pr: GitHubPullRequest) => {
    if (!github?.prReady) {
@@ -1225,6 +1262,7 @@ export const PullRequestSection: React.FC<{
      await github.prReady({ directory, number: pr.number });
      toast.success('Marked ready for review');
      await refresh({ force: true });
+      scheduleActionRefresh();
    } catch (e) {
      const message = e instanceof Error ? e.message : String(e);
      toast.error('Failed to mark ready', { description: message });
@@ -1234,7 +1272,7 @@ export const PullRequestSection: React.FC<{
    } finally {
      setIsMarkingReady(false);
    }
-  }, [directory, github, refresh]);
+  }, [directory, github, refresh, scheduleActionRefresh]);

  const updatePr = React.useCallback(async (pr: GitHubPullRequest) => {
    if (!github?.prUpdate) {
@@ -1268,13 +1306,14 @@ export const PullRequestSection: React.FC<{
      setIsEditingPr(false);
      toast.success('PR updated');
      await refresh({ force: true });
+      scheduleActionRefresh();
    } catch (e) {
      const message = e instanceof Error ? e.message : String(e);
      toast.error('Failed to update PR', { description: message });
    } finally {
      setIsUpdating(false);
    }
-  }, [directory, editBody, editTitle, github, prStatusKey, refresh, updatePrStatus]);
+  }, [directory, editBody, editTitle, github, prStatusKey, refresh, scheduleActionRefresh, updatePrStatus]);

  if (!canShow) {
    return null;
@@ -8,8 +8,10 @@ import { useProjectsStore } from '@/stores/useProjectsStore';
 import { useSessionStore } from '@/stores/useSessionStore';

 const MAX_BACKGROUND_PR_DIRECTORIES = 50;
-const BRANCH_REFRESH_TTL_MS = 2 * 60_000;
-const BRANCH_REFRESH_INTERVAL_MS = 60_000;
+const ACTIVE_DIRECTORY_REFRESH_TTL_MS = 15_000;
+const BACKGROUND_DIRECTORY_REFRESH_TTL_MS = 2 * 60_000;
+const BRANCH_REFRESH_INTERVAL_MS = 15_000;
+const PR_EVENTUAL_CONSISTENCY_REFRESH_DELAY_MS = 5_000;

 const normalizePath = (value?: string | null): string | null => {
  if (typeof value !== 'string') {
@@ -33,9 +35,49 @@ type SessionLike = Session & {

 type BranchCacheEntry = {
  branch: string | null;
+  tracking: string | null;
+  ahead: number;
+  behind: number;
  fetchedAt: number;
 };

+type PrTarget = {
+  directory: string;
+  branch: string;
+  remoteName?: string | null;
+};
+
+const getBranchRefreshTtl = (directory: string, currentDirectory: string | null): number => {
+  return directory === currentDirectory ? ACTIVE_DIRECTORY_REFRESH_TTL_MS : BACKGROUND_DIRECTORY_REFRESH_TTL_MS;
+};
+
+const hasRepoSignalChanged = (previous: BranchCacheEntry | undefined, next: BranchCacheEntry): boolean => {
+  if (!previous) {
+    return Boolean(next.branch);
+  }
+
+  return previous.branch !== next.branch
+    || previous.tracking !== next.tracking
+    || previous.ahead !== next.ahead
+    || previous.behind !== next.behind;
+};
+
+const toPrTargets = (cache: Map<string, BranchCacheEntry>, directories: string[]): PrTarget[] => {
+  const result: PrTarget[] = [];
+  directories.forEach((directory) => {
+    const cached = cache.get(directory);
+    if (!cached?.branch) {
+      return;
+    }
+    result.push({
+      directory,
+      branch: cached.branch,
+      remoteName: null,
+    });
+  });
+  return result;
+};
+
 export const useGitHubPrBackgroundTracking = (
  github: RuntimeAPIs['github'] | undefined,
  git: RuntimeAPIs['git'],
@@ -52,14 +94,47 @@ export const useGitHubPrBackgroundTracking = (
  const refreshGitHubAuthStatus = useGitHubAuthStore((state) => state.refreshStatus);

  const syncBackgroundTargets = useGitHubPrStatusStore((state) => state.syncBackgroundTargets);
+  const refreshPrTargets = useGitHubPrStatusStore((state) => state.refreshTargets);

  const [branchCache, setBranchCache] = React.useState<Map<string, BranchCacheEntry>>(new Map());
  const branchCacheRef = React.useRef<Map<string, BranchCacheEntry>>(new Map());
+  const targetsRef = React.useRef<PrTarget[]>([]);
+  const burstTimeoutsRef = React.useRef<Map<string, number>>(new Map());

  React.useEffect(() => {
    branchCacheRef.current = branchCache;
  }, [branchCache]);

+  const scheduleBurstRefresh = React.useCallback((targetsToRefresh: PrTarget[]) => {
+    if (targetsToRefresh.length === 0) {
+      return;
+    }
+
+    const dedupedTargets = new Map<string, PrTarget>();
+    targetsToRefresh.forEach((target) => {
+      const key = `${target.directory}::${target.branch}`;
+      dedupedTargets.set(key, target);
+    });
+
+    dedupedTargets.forEach((target, key) => {
+      const existing = burstTimeoutsRef.current.get(key);
+      if (typeof existing === 'number') {
+        window.clearTimeout(existing);
+      }
+
+      const timeoutId = window.setTimeout(() => {
+        burstTimeoutsRef.current.delete(key);
+        void refreshPrTargets([target], {
+          force: true,
+          silent: true,
+          markInitialResolved: true,
+        });
+      }, PR_EVENTUAL_CONSISTENCY_REFRESH_DELAY_MS);
+
+      burstTimeoutsRef.current.set(key, timeoutId);
+    });
+  }, [refreshPrTargets]);
+
  React.useEffect(() => {
    if (!github || githubAuthChecked) {
      return;
@@ -104,7 +179,7 @@ export const useGitHubPrBackgroundTracking = (
  React.useEffect(() => {
    let cancelled = false;

-    const refreshBranches = async (force = false) => {
+    const refreshBranches = async (force = false): Promise<PrTarget[]> => {
      const now = Date.now();
      const directoriesToFetch = candidateDirectories.filter((directory) => {
        const cached = branchCacheRef.current.get(directory);
@@ -114,11 +189,11 @@ export const useGitHubPrBackgroundTracking = (
        if (force) {
          return true;
        }
-        return now - cached.fetchedAt > BRANCH_REFRESH_TTL_MS;
+        return now - cached.fetchedAt > getBranchRefreshTtl(directory, currentDirectory);
      });

      if (directoriesToFetch.length === 0) {
-        return;
+        return toPrTargets(branchCacheRef.current, candidateDirectories);
      }

      const results = await Promise.all(
@@ -126,38 +201,84 @@ export const useGitHubPrBackgroundTracking = (
          try {
            const status = await git.getGitStatus(directory);
            const branch = typeof status.current === 'string' ? status.current.trim() : '';
-            return { directory, branch: branch && branch !== 'HEAD' ? branch : null };
+            return {
+              directory,
+              branch: branch && branch !== 'HEAD' ? branch : null,
+              tracking: typeof status.tracking === 'string' ? status.tracking : null,
+              ahead: typeof status.ahead === 'number' ? status.ahead : 0,
+              behind: typeof status.behind === 'number' ? status.behind : 0,
+            };
          } catch {
-            return { directory, branch: null };
+            return { directory, branch: null, tracking: null, ahead: 0, behind: 0 };
          }
        }),
      );

      if (cancelled) {
-        return;
+        return [];
      }

-      setBranchCache((prev) => {
-        const next = new Map(prev);
-        let changed = false;
-        results.forEach(({ directory, branch }) => {
-          const previous = next.get(directory);
-          const fetchedAt = Date.now();
-          if (!previous || previous.branch !== branch) {
-            changed = true;
-          }
-          if (!previous || previous.fetchedAt !== fetchedAt || previous.branch !== branch) {
-            next.set(directory, { branch, fetchedAt });
-          }
-        });
+      const nextCache = new Map(branchCacheRef.current);
+      const changedTargets: PrTarget[] = [];

-        if (!changed && results.length > 0) {
+      results.forEach(({ directory, branch, tracking, ahead, behind }) => {
+        const previous = nextCache.get(directory);
+        const nextEntry = {
+          branch,
+          tracking,
+          ahead,
+          behind,
+          fetchedAt: Date.now(),
+        };
+
+        nextCache.set(directory, nextEntry);
+
+        if (branch && hasRepoSignalChanged(previous, nextEntry)) {
+          changedTargets.push({
+            directory,
+            branch,
+            remoteName: null,
+          });
+        }
+      });
+
+      setBranchCache((prev) => {
+        let changed = false;
+        if (prev.size !== nextCache.size) {
+          changed = true;
+        } else {
+          for (const [key, value] of nextCache.entries()) {
+            const previous = prev.get(key);
+            if (!previous
+              || previous.branch !== value.branch
+              || previous.tracking !== value.tracking
+              || previous.ahead !== value.ahead
+              || previous.behind !== value.behind) {
+              changed = true;
+              break;
+            }
+          }
+        }
+
+        branchCacheRef.current = nextCache;
+
+        if (!changed) {
          return prev;
        }

-        branchCacheRef.current = next;
-        return next;
+        return nextCache;
      });
+
+      if (changedTargets.length > 0) {
+        void refreshPrTargets(changedTargets, {
+          force: true,
+          silent: true,
+          markInitialResolved: true,
+        });
+        scheduleBurstRefresh(changedTargets);
+      }
+
+      return toPrTargets(nextCache, candidateDirectories);
    };

    void refreshBranches();
@@ -169,11 +290,48 @@ export const useGitHubPrBackgroundTracking = (
      void refreshBranches();
    }, BRANCH_REFRESH_INTERVAL_MS);

+    const refreshOnResume = () => {
+      if (typeof document !== 'undefined' && document.visibilityState !== 'visible') {
+        return;
+      }
+
+      void refreshBranches(true).then((nextTargets) => {
+        const currentTargets = nextTargets.length > 0 ? nextTargets : targetsRef.current;
+        if (currentTargets.length === 0) {
+          return;
+        }
+
+        const activeTargets = currentDirectory
+          ? currentTargets.filter((target) => target.directory === currentDirectory)
+          : [];
+
+        if (activeTargets.length > 0) {
+          void refreshPrTargets(activeTargets, {
+            force: true,
+            silent: true,
+            markInitialResolved: true,
+          });
+        }
+
+        void refreshPrTargets(currentTargets, {
+          force: true,
+          onlyExistingPr: true,
+          silent: true,
+          markInitialResolved: true,
+        });
+      });
+    };
+
+    window.addEventListener('focus', refreshOnResume);
+    document.addEventListener('visibilitychange', refreshOnResume);
+
    return () => {
      cancelled = true;
      window.clearInterval(intervalId);
+      window.removeEventListener('focus', refreshOnResume);
+      document.removeEventListener('visibilitychange', refreshOnResume);
    };
-  }, [candidateDirectories, git]);
+  }, [candidateDirectories, currentDirectory, git, refreshPrTargets, scheduleBurstRefresh]);

  React.useEffect(() => {
    const validDirectories = new Set(candidateDirectories);
@@ -196,21 +354,23 @@ export const useGitHubPrBackgroundTracking = (
  }, [candidateDirectories]);

  const targets = React.useMemo(() => {
-    const result: Array<{ directory: string; branch: string; remoteName?: string | null }> = [];
-    candidateDirectories.forEach((directory) => {
-      const cached = branchCache.get(directory);
-      if (!cached?.branch) {
-        return;
-      }
-      result.push({
-        directory,
-        branch: cached.branch,
-        remoteName: null,
-      });
-    });
-    return result;
+    return toPrTargets(branchCache, candidateDirectories);
  }, [branchCache, candidateDirectories]);

+  React.useEffect(() => {
+    targetsRef.current = targets;
+  }, [targets]);
+
+  React.useEffect(() => {
+    const burstTimeouts = burstTimeoutsRef.current;
+    return () => {
+      burstTimeouts.forEach((timeoutId) => {
+        window.clearTimeout(timeoutId);
+      });
+      burstTimeouts.clear();
+    };
+  }, []);
+
  React.useEffect(() => {
    syncBackgroundTargets({
      targets,
@@ -774,6 +774,8 @@ export type GitHubPullRequestStatus = {
  pr?: GitHubPullRequest | null;
  checks?: GitHubChecksSummary | null;
  canMerge?: boolean;
+  defaultBranch?: string | null;
+  resolvedRemoteName?: string | null;
 };

 export type GitHubPullRequestCreateInput = {