From b2ef51bbdfb319a3ad7486ee35c80db69b067bd6 Mon Sep 17 00:00:00 2001 From: Matt Toohey Date: Tue, 14 Apr 2026 17:29:38 +1000 Subject: [PATCH 1/9] fix(staged): replace per-component PR polling with centralized service Each BranchCardPrButton ran its own setInterval for PR status polling, leading to N concurrent gh CLI processes, stuck refreshing guards, and leaked event listeners. This replaces that with a single PrPollingService that coordinates all PR status refreshes via the existing backend refreshAllPrStatuses command. Key changes: - New PrPollingService with single timer, adaptive intervals, window focus awareness, concurrency protection, and stale-data tracking - Fix event listener race condition by awaiting listen() promises in cleanup instead of fire-and-forget .then() - Remove prStatusRefreshing guard, per-component setInterval, and window focus/blur handlers from BranchCardPrButton - Add stale-data indicator when polling fails repeatedly Co-Authored-By: Claude Opus 4.6 (1M context) --- .../branches/BranchCardPrButton.svelte | 171 ++++---------- .../src/lib/services/prPollingService.ts | 215 ++++++++++++++++++ 2 files changed, 264 insertions(+), 122 deletions(-) create mode 100644 apps/staged/src/lib/services/prPollingService.ts diff --git a/apps/staged/src/lib/features/branches/BranchCardPrButton.svelte b/apps/staged/src/lib/features/branches/BranchCardPrButton.svelte index 4a1972b90..c8848c03c 100644 --- a/apps/staged/src/lib/features/branches/BranchCardPrButton.svelte +++ b/apps/staged/src/lib/features/branches/BranchCardPrButton.svelte @@ -15,7 +15,7 @@ } from 'lucide-svelte'; import Spinner from '../../shared/Spinner.svelte'; import ConfirmDialog from '../../shared/ConfirmDialog.svelte'; - import { listen, type UnlistenFn } from '@tauri-apps/api/event'; + import { listen } from '@tauri-apps/api/event'; import type { Branch, BranchTimeline as BranchTimelineData } from '../../types'; import * as commands from '../../api/commands'; import { extractPrNumber, extractPrUrl, isPushRejectedNonFastForward } from './branchCardHelpers'; @@ -25,6 +25,7 @@ import { pushStateStore, type PushState } from '../../stores/pushState.svelte'; import { projectStateStore } from '../../stores/projectState.svelte'; import { sessionRegistry } from '../../stores/sessionRegistry.svelte'; + import * as prPollingService from '../../services/prPollingService'; interface Props { branch: Branch; @@ -79,10 +80,8 @@ // PR head SHA — updated from events and branch prop let prHeadSha = $state(null); - // PR status polling state - let prStatusPollTimer: ReturnType | null = null; - let prStatusRefreshing = $state(false); - let lastImmediateRefreshPrNumber = $state(null); + // Stale-data indicator (set by the centralized polling service) + let prStatusStale = $state(false); // PR status fields (local state, updated via events) let prStatusState = $state(null); @@ -124,17 +123,13 @@ let showPushErrorDialog = $state(false); let showForcePushDialog = $state(false); - // Window focus tracking for smart polling - let isWindowFocused = $state(true); - let handleFocus: (() => void) | null = null; - let handleBlur: (() => void) | null = null; - - let unlistenPrStatus: UnlistenFn | null = null; - let unlistenPrStatusCleared: UnlistenFn | null = null; - + // ========================================================================= + // Event listeners for PR status (fix race condition by awaiting promises) + // ========================================================================= $effect(() => { const branchId = branch.id; - listen<{ + + const unlistenStatusPromise = listen<{ branchId: string; prState: string; prChecksStatus: string; @@ -151,12 +146,12 @@ prStatusMergeable = payload.prMergeable; prStatusDraft = payload.prDraft; prHeadSha = payload.prHeadSha; + // Update the polling service with the new checks status + prPollingService.updateChecksStatus(branchId, payload.prChecksStatus === 'PENDING'); } - }).then((unlisten) => { - unlistenPrStatus = unlisten; }); - listen('pr-status-cleared', (event) => { + const unlistenClearedPromise = listen('pr-status-cleared', (event) => { if (event.payload === branchId) { prStatusState = null; prStatusChecks = null; @@ -165,13 +160,11 @@ prStatusDraft = null; prHeadSha = null; } - }).then((unlisten) => { - unlistenPrStatusCleared = unlisten; }); return () => { - unlistenPrStatus?.(); - unlistenPrStatusCleared?.(); + unlistenStatusPromise.then((fn) => fn()); + unlistenClearedPromise.then((fn) => fn()); }; }); @@ -219,119 +212,45 @@ return () => clearInterval(interval); }); - // PR status polling: adaptive intervals based on status - $effect(() => { - const shouldPoll = branch.prNumber && isWindowFocused; - - if (prStatusState === 'MERGED' || prStatusState === 'CLOSED') { - if (prStatusPollTimer) { - clearInterval(prStatusPollTimer); - prStatusPollTimer = null; - } - return; - } - - let pollInterval: number; - if (prStatusChecks === 'PENDING') { - pollInterval = 15_000; - } else { - pollInterval = 60_000; - } - - if (shouldPoll) { - if (prStatusPollTimer) { - clearInterval(prStatusPollTimer); - } - - prStatusPollTimer = setInterval(async () => { - if (prStatusRefreshing) { - return; - } - try { - prStatusRefreshing = true; - let timeoutId: ReturnType; - const timeout = new Promise((_, reject) => { - timeoutId = setTimeout( - () => reject(new Error('refreshPrStatus timed out after 60s')), - 60_000 - ); - }); - try { - await Promise.race([commands.refreshPrStatus(branch.id), timeout]); - } finally { - clearTimeout(timeoutId!); - } - } catch (e) { - console.error(`[BranchCardPrButton] Poll refresh failed for branch=${branch.id}:`, e); - } finally { - prStatusRefreshing = false; - } - }, pollInterval); - } else { - if (prStatusPollTimer) { - clearInterval(prStatusPollTimer); - prStatusPollTimer = null; - } - } - - return () => { - if (prStatusPollTimer) { - clearInterval(prStatusPollTimer); - prStatusPollTimer = null; - } - }; - }); + // ========================================================================= + // Polling service registration + // ========================================================================= - // Kick off an immediate refresh whenever this branch gains a PR number. - // This covers branches hydrated after mount, such as project/repo creation from an existing PR. + // Track/untrack this branch with the centralized polling service when it + // gains or loses a PR number, and when its terminal state changes. $effect(() => { const prNumber = branch.prNumber; + const branchId = branch.id; + const projectId = branch.projectId; - if (!prNumber) { - lastImmediateRefreshPrNumber = null; - return; - } - - if (!isWindowFocused || prStatusRefreshing || lastImmediateRefreshPrNumber === prNumber) { - return; + if (prNumber && prStatusState !== 'MERGED' && prStatusState !== 'CLOSED') { + prPollingService.track(branchId, projectId, prStatusChecks === 'PENDING'); + return () => { + prPollingService.untrack(branchId); + }; + } else { + prPollingService.untrack(branchId); } - - lastImmediateRefreshPrNumber = prNumber; - commands - .refreshPrStatus(branch.id) - .catch((e) => console.error('Failed to fetch initial PR status:', e)); }); + // Subscribe to stale-data notifications from the polling service + let unsubStale: (() => void) | null = null; + onMount(() => { window.addEventListener('keydown', handleOptionDown); window.addEventListener('keyup', handleOptionUp); - handleFocus = () => { - isWindowFocused = true; - if (branch.prNumber && !prStatusRefreshing) { - commands - .refreshPrStatus(branch.id) - .catch((e) => console.error('Failed to refresh PR status on focus:', e)); + unsubStale = prPollingService.onStale((branchId, isStale) => { + if (branchId === branch.id) { + prStatusStale = isStale; } - }; - handleBlur = () => { - isWindowFocused = false; - }; - window.addEventListener('focus', handleFocus); - window.addEventListener('blur', handleBlur); + }); }); onDestroy(() => { - unlistenPrStatus?.(); - unlistenPrStatusCleared?.(); - if (prStatusPollTimer) { - clearInterval(prStatusPollTimer); - prStatusPollTimer = null; - } - if (handleFocus) window.removeEventListener('focus', handleFocus); - if (handleBlur) window.removeEventListener('blur', handleBlur); window.removeEventListener('keydown', handleOptionDown); window.removeEventListener('keyup', handleOptionUp); + unsubStale?.(); }); // ========================================================================= @@ -429,9 +348,7 @@ if (prNumber) { await commands.updateBranchPr(branch.id, prNumber); branch.prNumber = prNumber; - commands - .refreshPrStatus(branch.id) - .catch((e) => console.error('Failed to fetch initial PR status:', e)); + prPollingService.refreshNow(branch.projectId); } prStateStore.setPrCreated(branch.id, foundUrl); } else { @@ -507,7 +424,7 @@ prHeadSha = latestCommit.sha; } // Immediately refresh PR status so checks update right away - commands.refreshPrStatus(branch.id).catch(() => {}); + prPollingService.refreshNow(branch.projectId); setTimeout(() => { pushStateStore.clearPushState(branch.id); }, 1_500); @@ -675,7 +592,9 @@ {optionHeld ? 'Create draft PR' : 'Create PR'} {/if} - {#if prStatusIndicator} + {#if prStatusStale} + ! + {:else if prStatusIndicator} {/if} @@ -815,6 +734,14 @@ background-color: var(--text-faint, #64748b); } + .pr-status-stale { + font-size: 9px; + font-weight: 700; + color: var(--text-faint, #94a3b8); + margin-left: 2px; + opacity: 0.7; + } + .pr-status-indicator.pending { background-color: var(--text-muted, #94a3b8); animation: pulse 2s cubic-bezier(0.4, 0, 0.6, 1) infinite; diff --git a/apps/staged/src/lib/services/prPollingService.ts b/apps/staged/src/lib/services/prPollingService.ts new file mode 100644 index 000000000..2162f58d5 --- /dev/null +++ b/apps/staged/src/lib/services/prPollingService.ts @@ -0,0 +1,215 @@ +/** + * Centralized PR status polling service. + * + * Replaces the per-component setInterval polling in BranchCardPrButton with a + * single coordinated timer that calls `refreshAllPrStatuses` on the backend. + * The backend already emits per-branch `pr-status-changed` events, so + * components only need to listen for those events — no per-branch IPC calls. + */ + +import { refreshAllPrStatuses } from '../commands'; + +// --------------------------------------------------------------------------- +// Types +// --------------------------------------------------------------------------- + +interface TrackedBranch { + projectId: string; + hasPendingChecks: boolean; +} + +type StaleCallback = (branchId: string, isStale: boolean) => void; + +// --------------------------------------------------------------------------- +// State +// --------------------------------------------------------------------------- + +/** Branches currently being tracked, keyed by branchId. */ +const tracked = new Map(); + +/** Consecutive failure count per projectId. */ +const failures = new Map(); + +/** Registered stale-data callbacks. */ +const staleCallbacks = new Set(); + +let timerId: ReturnType | null = null; +let refreshInFlight = false; +let windowFocused = true; + +// Intervals +const PENDING_INTERVAL = 15_000; +const NORMAL_INTERVAL = 60_000; +const MAX_CONSECUTIVE_FAILURES = 3; + +// --------------------------------------------------------------------------- +// Internal helpers +// --------------------------------------------------------------------------- + +function getInterval(): number { + for (const entry of tracked.values()) { + if (entry.hasPendingChecks) return PENDING_INTERVAL; + } + return NORMAL_INTERVAL; +} + +/** Collect the distinct projectIds that are currently tracked. */ +function trackedProjectIds(): Set { + const ids = new Set(); + for (const entry of tracked.values()) { + ids.add(entry.projectId); + } + return ids; +} + +async function poll() { + if (refreshInFlight || !windowFocused || tracked.size === 0) { + scheduleNext(); + return; + } + + refreshInFlight = true; + const projectIds = trackedProjectIds(); + + for (const projectId of projectIds) { + try { + await refreshAllPrStatuses(projectId); + // Reset failure counter on success + const prev = failures.get(projectId) ?? 0; + if (prev > 0) { + failures.set(projectId, 0); + notifyStale(projectId, false); + } + } catch (e) { + const count = (failures.get(projectId) ?? 0) + 1; + failures.set(projectId, count); + console.error( + `[PrPollingService] refreshAllPrStatuses failed for project=${projectId} (attempt ${count}):`, + e + ); + if (count >= MAX_CONSECUTIVE_FAILURES) { + notifyStale(projectId, true); + } + } + } + + refreshInFlight = false; + scheduleNext(); +} + +function scheduleNext() { + stopTimer(); + if (tracked.size === 0 || !windowFocused) return; + timerId = setTimeout(poll, getInterval()); +} + +function stopTimer() { + if (timerId !== null) { + clearTimeout(timerId); + timerId = null; + } +} + +function notifyStale(projectId: string, isStale: boolean) { + for (const [branchId, entry] of tracked) { + if (entry.projectId === projectId) { + for (const cb of staleCallbacks) { + try { + cb(branchId, isStale); + } catch { + // ignore callback errors + } + } + } + } +} + +// --------------------------------------------------------------------------- +// Focus / blur handlers +// --------------------------------------------------------------------------- + +function handleFocus() { + windowFocused = true; + // Immediate refresh on focus, then resume schedule + poll(); +} + +function handleBlur() { + windowFocused = false; + stopTimer(); +} + +let listenersAttached = false; + +function ensureWindowListeners() { + if (listenersAttached) return; + window.addEventListener('focus', handleFocus); + window.addEventListener('blur', handleBlur); + listenersAttached = true; +} + +function removeWindowListeners() { + if (!listenersAttached) return; + window.removeEventListener('focus', handleFocus); + window.removeEventListener('blur', handleBlur); + listenersAttached = false; +} + +// --------------------------------------------------------------------------- +// Public API +// --------------------------------------------------------------------------- + +/** Start tracking a branch for PR status polling. */ +export function track(branchId: string, projectId: string, hasPendingChecks = false): void { + const isFirst = tracked.size === 0; + tracked.set(branchId, { projectId, hasPendingChecks }); + + if (isFirst) { + ensureWindowListeners(); + // Start polling immediately + poll(); + } else { + // Interval may have changed — reschedule + scheduleNext(); + } +} + +/** Stop tracking a branch. */ +export function untrack(branchId: string): void { + tracked.delete(branchId); + if (tracked.size === 0) { + stopTimer(); + removeWindowListeners(); + failures.clear(); + } +} + +/** Update whether a tracked branch has pending checks (affects poll interval). */ +export function updateChecksStatus(branchId: string, hasPendingChecks: boolean): void { + const entry = tracked.get(branchId); + if (entry && entry.hasPendingChecks !== hasPendingChecks) { + entry.hasPendingChecks = hasPendingChecks; + // Interval may have changed — reschedule + scheduleNext(); + } +} + +/** Register a callback for stale-data notifications. Returns an unsubscribe function. */ +export function onStale(callback: StaleCallback): () => void { + staleCallbacks.add(callback); + return () => staleCallbacks.delete(callback); +} + +/** Trigger an immediate refresh for a specific project (e.g. after PR creation or push). */ +export function refreshNow(projectId: string): void { + if (refreshInFlight) return; + refreshInFlight = true; + refreshAllPrStatuses(projectId) + .catch((e) => + console.error(`[PrPollingService] immediate refresh failed for project=${projectId}:`, e) + ) + .finally(() => { + refreshInFlight = false; + scheduleNext(); + }); +} From c5502910987fb82f8d02dacde4a5aa88d8ef2437 Mon Sep 17 00:00:00 2001 From: Matt Toohey Date: Wed, 15 Apr 2026 11:31:41 +1000 Subject: [PATCH 2/9] feat(staged): add info-level debug logging to PR polling service and component Add [PrPolling] prefixed console.info logs to help diagnose PR status update failures. Logs cover track/untrack, poll start/skip/end, scheduleNext intervals, refreshNow accept/drop, window focus/blur, pr-status-changed events, and $effect lifecycle in the component. Co-Authored-By: Claude Opus 4.6 (1M context) --- .../branches/BranchCardPrButton.svelte | 14 +++++++++ .../src/lib/services/prPollingService.ts | 31 +++++++++++++++++-- 2 files changed, 42 insertions(+), 3 deletions(-) diff --git a/apps/staged/src/lib/features/branches/BranchCardPrButton.svelte b/apps/staged/src/lib/features/branches/BranchCardPrButton.svelte index c8848c03c..986915025 100644 --- a/apps/staged/src/lib/features/branches/BranchCardPrButton.svelte +++ b/apps/staged/src/lib/features/branches/BranchCardPrButton.svelte @@ -140,6 +140,9 @@ }>('pr-status-changed', (event) => { const payload = event.payload; if (payload.branchId === branchId) { + console.info( + `[PrPolling] pr-status-changed: branch=${branchId}, state=${payload.prState}, checks=${payload.prChecksStatus}, review=${payload.prReviewDecision}` + ); prStatusState = payload.prState; prStatusChecks = payload.prChecksStatus; prStatusReviewDecision = payload.prReviewDecision; @@ -224,11 +227,18 @@ const projectId = branch.projectId; if (prNumber && prStatusState !== 'MERGED' && prStatusState !== 'CLOSED') { + console.info( + `[PrPolling] $effect track: branch=${branchId}, prNumber=${prNumber}, prStatusState=${prStatusState}, checks=${prStatusChecks}` + ); prPollingService.track(branchId, projectId, prStatusChecks === 'PENDING'); return () => { + console.info(`[PrPolling] $effect cleanup untrack: branch=${branchId}`); prPollingService.untrack(branchId); }; } else { + console.info( + `[PrPolling] $effect untrack (terminal/no PR): branch=${branchId}, prNumber=${prNumber}, prStatusState=${prStatusState}` + ); prPollingService.untrack(branchId); } }); @@ -348,6 +358,9 @@ if (prNumber) { await commands.updateBranchPr(branch.id, prNumber); branch.prNumber = prNumber; + console.info( + `[PrPolling] refreshNow after PR creation: branch=${branch.id}, prNumber=${prNumber}` + ); prPollingService.refreshNow(branch.projectId); } prStateStore.setPrCreated(branch.id, foundUrl); @@ -424,6 +437,7 @@ prHeadSha = latestCommit.sha; } // Immediately refresh PR status so checks update right away + console.info(`[PrPolling] refreshNow after push: branch=${branch.id}`); prPollingService.refreshNow(branch.projectId); setTimeout(() => { pushStateStore.clearPushState(branch.id); diff --git a/apps/staged/src/lib/services/prPollingService.ts b/apps/staged/src/lib/services/prPollingService.ts index 2162f58d5..e16c3c305 100644 --- a/apps/staged/src/lib/services/prPollingService.ts +++ b/apps/staged/src/lib/services/prPollingService.ts @@ -64,12 +64,16 @@ function trackedProjectIds(): Set { async function poll() { if (refreshInFlight || !windowFocused || tracked.size === 0) { + console.info( + `[PrPolling] poll skipped: refreshInFlight=${refreshInFlight}, windowFocused=${windowFocused}, tracked=${tracked.size}` + ); scheduleNext(); return; } refreshInFlight = true; const projectIds = trackedProjectIds(); + console.info(`[PrPolling] poll start: projectIds=[${[...projectIds]}], tracked=${tracked.size}`); for (const projectId of projectIds) { try { @@ -94,13 +98,21 @@ async function poll() { } refreshInFlight = false; + console.info(`[PrPolling] poll end`); scheduleNext(); } function scheduleNext() { stopTimer(); - if (tracked.size === 0 || !windowFocused) return; - timerId = setTimeout(poll, getInterval()); + if (tracked.size === 0 || !windowFocused) { + console.info( + `[PrPolling] scheduleNext: not scheduling (tracked=${tracked.size}, windowFocused=${windowFocused})` + ); + return; + } + const interval = getInterval(); + console.info(`[PrPolling] scheduleNext: interval=${interval}ms, tracked=${tracked.size}`); + timerId = setTimeout(poll, interval); } function stopTimer() { @@ -129,12 +141,14 @@ function notifyStale(projectId: string, isStale: boolean) { // --------------------------------------------------------------------------- function handleFocus() { + console.info(`[PrPolling] window focus`); windowFocused = true; // Immediate refresh on focus, then resume schedule poll(); } function handleBlur() { + console.info(`[PrPolling] window blur`); windowFocused = false; stopTimer(); } @@ -163,6 +177,9 @@ function removeWindowListeners() { export function track(branchId: string, projectId: string, hasPendingChecks = false): void { const isFirst = tracked.size === 0; tracked.set(branchId, { projectId, hasPendingChecks }); + console.info( + `[PrPolling] track: branch=${branchId}, project=${projectId}, pendingChecks=${hasPendingChecks}, isFirst=${isFirst}, totalTracked=${tracked.size}` + ); if (isFirst) { ensureWindowListeners(); @@ -176,7 +193,11 @@ export function track(branchId: string, projectId: string, hasPendingChecks = fa /** Stop tracking a branch. */ export function untrack(branchId: string): void { + const existed = tracked.has(branchId); tracked.delete(branchId); + console.info( + `[PrPolling] untrack: branch=${branchId}, existed=${existed}, remaining=${tracked.size}` + ); if (tracked.size === 0) { stopTimer(); removeWindowListeners(); @@ -202,7 +223,11 @@ export function onStale(callback: StaleCallback): () => void { /** Trigger an immediate refresh for a specific project (e.g. after PR creation or push). */ export function refreshNow(projectId: string): void { - if (refreshInFlight) return; + if (refreshInFlight) { + console.info(`[PrPolling] refreshNow: DROPPED for project=${projectId} (refreshInFlight=true)`); + return; + } + console.info(`[PrPolling] refreshNow: accepted for project=${projectId}`); refreshInFlight = true; refreshAllPrStatuses(projectId) .catch((e) => From b473208b22b7047a4f49c1fa5c0a3e1e80d2fd60 Mon Sep 17 00:00:00 2001 From: Matt Toohey Date: Wed, 15 Apr 2026 16:19:22 +1000 Subject: [PATCH 3/9] fix(staged): emit session running event atomically from backend commands The create_pr and push_branch commands now emit a "running" session event (with branch_id, project_id, and session_type) before returning the session ID. This ensures the global sessionStatusListener registers the session before any completion event can arrive, fixing the race where fast-completing sessions hit "unknown session ID" warnings and PR numbers are never persisted. Remove the now-redundant sessionRegistry.register() and projectStateStore.addRunningSession() calls from the component's .then() callbacks. Co-Authored-By: Claude Opus 4.6 (1M context) --- apps/staged/src-tauri/src/prs.rs | 22 +++++++++++++++++++ .../branches/BranchCardPrButton.svelte | 12 +++++----- 2 files changed, 28 insertions(+), 6 deletions(-) diff --git a/apps/staged/src-tauri/src/prs.rs b/apps/staged/src-tauri/src/prs.rs index 44201eb49..743934a5e 100644 --- a/apps/staged/src-tauri/src/prs.rs +++ b/apps/staged/src-tauri/src/prs.rs @@ -153,6 +153,17 @@ This is critical - the application parses this to link the PR. None }; + // Emit "running" event *before* returning so the global session listener + // registers this session atomically — avoiding the race where the session + // completes before the frontend `.then()` callback fires. + session_runner::emit_session_running( + &app_handle, + &session.id, + &branch_id, + &branch.project_id, + "pr", + ); + session_runner::start_session( session_runner::SessionConfig { session_id: session.id.clone(), @@ -586,6 +597,17 @@ The push must succeed before you finish (unless you output the non-fast-forward None }; + // Emit "running" event *before* returning so the global session listener + // registers this session atomically — avoiding the race where the session + // completes before the frontend `.then()` callback fires. + session_runner::emit_session_running( + &app_handle, + &session.id, + &branch_id, + &branch.project_id, + "push", + ); + session_runner::start_session( session_runner::SessionConfig { session_id: session.id.clone(), diff --git a/apps/staged/src/lib/features/branches/BranchCardPrButton.svelte b/apps/staged/src/lib/features/branches/BranchCardPrButton.svelte index 986915025..ff0dfe323 100644 --- a/apps/staged/src/lib/features/branches/BranchCardPrButton.svelte +++ b/apps/staged/src/lib/features/branches/BranchCardPrButton.svelte @@ -23,8 +23,6 @@ import { agentState, REMOTE_AGENTS } from '../agents/agent.svelte'; import { prStateStore, type PrState } from '../../stores/prState.svelte'; import { pushStateStore, type PushState } from '../../stores/pushState.svelte'; - import { projectStateStore } from '../../stores/projectState.svelte'; - import { sessionRegistry } from '../../stores/sessionRegistry.svelte'; import * as prPollingService from '../../services/prPollingService'; interface Props { @@ -331,9 +329,10 @@ commands .createPr(branch.id, provider, draft) .then((sessionId) => { - sessionRegistry.register(sessionId, branch.projectId, 'pr', branch.id); + // Session is already registered by the global listener via the + // backend's "running" event — just update the local store with the + // real session ID so the fallback poller can track it. prStateStore.setPrCreating(branch.id, sessionId); - projectStateStore.addRunningSession(branch.projectId, sessionId); }) .catch((e) => { prStateStore.setPrError(branch.id, e instanceof Error ? e.message : String(e)); @@ -398,9 +397,10 @@ commands .pushBranch(branch.id, provider, force) .then((sessionId) => { - sessionRegistry.register(sessionId, branch.projectId, 'push', branch.id); + // Session is already registered by the global listener via the + // backend's "running" event — just update the local store with the + // real session ID so the fallback poller can track it. pushStateStore.setPushing(branch.id, sessionId); - projectStateStore.addRunningSession(branch.projectId, sessionId); }) .catch((e) => { pushStateStore.setPushError(branch.id, e instanceof Error ? e.message : String(e)); From 2fce3bc6348ec62039c986d6cbbce8fe9527e1d5 Mon Sep 17 00:00:00 2001 From: Matt Toohey Date: Wed, 15 Apr 2026 16:39:19 +1000 Subject: [PATCH 4/9] feat(staged): recover missing PR numbers on component mount When a branch has been pushed but has no prNumber (e.g. due to a race condition during PR creation), the BranchCardPrButton now checks GitHub for an existing open PR on mount via `gh pr view `. This runs async so the frontend isn't blocked. If a PR is found, the number is persisted and polling starts immediately. New backend command `recover_branch_pr` wraps the existing `get_pr_for_branch` GitHub CLI helper and updates the branch record. Co-Authored-By: Claude Opus 4.6 (1M context) --- apps/staged/src-tauri/src/lib.rs | 1 + apps/staged/src-tauri/src/prs.rs | 67 +++++++++++++++++++ apps/staged/src/lib/commands.ts | 6 ++ .../branches/BranchCardPrButton.svelte | 17 +++++ 4 files changed, 91 insertions(+) diff --git a/apps/staged/src-tauri/src/lib.rs b/apps/staged/src-tauri/src/lib.rs index 2dfa396cc..b73d5dbd9 100644 --- a/apps/staged/src-tauri/src/lib.rs +++ b/apps/staged/src-tauri/src/lib.rs @@ -1824,6 +1824,7 @@ pub fn run() { prs::has_unpushed_commits, prs::push_branch, prs::clear_branch_pr_status, + prs::recover_branch_pr, // Utilities util_commands::open_url, util_commands::is_sq_available, diff --git a/apps/staged/src-tauri/src/prs.rs b/apps/staged/src-tauri/src/prs.rs index 743934a5e..909c4b04c 100644 --- a/apps/staged/src-tauri/src/prs.rs +++ b/apps/staged/src-tauri/src/prs.rs @@ -433,6 +433,73 @@ pub fn clear_branch_pr_status( Ok(()) } +/// Look up an existing open PR for a branch on GitHub and persist it. +/// +/// Called on component mount when `branch.prNumber` is null but the branch has +/// been pushed. Runs `gh pr view ` in the background so the frontend +/// is not blocked. Returns the recovered PR number, or None if no PR exists. +#[tauri::command(rename_all = "camelCase")] +pub async fn recover_branch_pr( + store: tauri::State<'_, Mutex>>>, + branch_id: String, +) -> Result, String> { + let store = get_store(&store)?; + + let branch = store + .get_branch(&branch_id) + .map_err(|e| e.to_string())? + .ok_or_else(|| format!("Branch not found: {branch_id}"))?; + + // If the branch already has a PR number, nothing to recover + if branch.pr_number.is_some() { + return Ok(branch.pr_number); + } + + let project = store + .get_project(&branch.project_id) + .map_err(|e| e.to_string())? + .ok_or_else(|| format!("Project not found: {}", branch.project_id))?; + + let is_remote = branch.branch_type == store::BranchType::Remote; + let branch_name = branch.branch_name.clone(); + + let (repo_slug, _) = resolve_branch_repo_and_subpath(&store, &project, &branch)?; + + let working_dir = if is_remote { + crate::paths::repos_dir() + .map(|d| d.join(&repo_slug)) + .ok_or_else(|| "Cannot determine clone path for remote branch".to_string())? + } else { + let workdir = store + .get_workdir_for_branch(&branch_id) + .map_err(|e| e.to_string())? + .ok_or_else(|| format!("No worktree for branch: {branch_id}"))?; + PathBuf::from(&workdir.path) + }; + + let pr_info = tauri::async_runtime::spawn_blocking(move || { + git::get_pr_for_branch(&working_dir, &branch_name) + }) + .await + .map_err(|e| format!("recover_branch_pr task failed: {e}"))? + .map_err(|e| e.to_string())?; + + if let Some(ref info) = pr_info { + let pr_number = info.number; + store + .update_branch_pr_number(&branch_id, Some(pr_number)) + .map_err(|e| e.to_string())?; + log::info!( + "recover_branch_pr: recovered PR #{} for branch_id={}", + pr_number, + branch_id + ); + Ok(Some(pr_number)) + } else { + Ok(None) + } +} + /// Check if a branch has commits that haven't been pushed to the remote. #[tauri::command(rename_all = "camelCase")] pub async fn has_unpushed_commits( diff --git a/apps/staged/src/lib/commands.ts b/apps/staged/src/lib/commands.ts index 5d1abce6e..e4cd43b30 100644 --- a/apps/staged/src/lib/commands.ts +++ b/apps/staged/src/lib/commands.ts @@ -852,6 +852,12 @@ export function updateBranchPr(branchId: string, prNumber: number | null): Promi return invoke('update_branch_pr', { branchId, prNumber }); } +/** Look up an existing open PR for a branch on GitHub and persist it. + * Returns the recovered PR number, or null if no PR exists. */ +export function recoverBranchPr(branchId: string): Promise { + return invoke('recover_branch_pr', { branchId }); +} + /** Check whether a branch has local commits not yet pushed to the remote. */ export function hasUnpushedCommits(branchId: string): Promise { return invoke('has_unpushed_commits', { branchId }); diff --git a/apps/staged/src/lib/features/branches/BranchCardPrButton.svelte b/apps/staged/src/lib/features/branches/BranchCardPrButton.svelte index ff0dfe323..c528a06c5 100644 --- a/apps/staged/src/lib/features/branches/BranchCardPrButton.svelte +++ b/apps/staged/src/lib/features/branches/BranchCardPrButton.svelte @@ -253,6 +253,23 @@ prStatusStale = isStale; } }); + + // PR recovery: if the branch has been pushed but has no PR number, + // check GitHub for an existing open PR on this branch name. + if (!branch.prNumber && isRemote) { + commands + .recoverBranchPr(branch.id) + .then((prNumber) => { + if (prNumber) { + console.info(`[PrPolling] recovered PR #${prNumber} for branch=${branch.id}`); + branch.prNumber = prNumber; + prPollingService.refreshNow(branch.projectId); + } + }) + .catch((err) => { + console.warn(`[PrPolling] PR recovery failed for branch=${branch.id}:`, err); + }); + } }); onDestroy(() => { From 788f1159a64f50dafd2b8849496478521689b8d4 Mon Sep 17 00:00:00 2001 From: Matt Toohey Date: Wed, 15 Apr 2026 16:43:06 +1000 Subject: [PATCH 5/9] chore(staged): remove [PrPolling] debug logs from polling service and component The info-level console.info logs were added to diagnose PR status update failures and are no longer needed. Co-Authored-By: Claude Opus 4.6 (1M context) --- .../branches/BranchCardPrButton.svelte | 19 ++---------------- .../src/lib/services/prPollingService.ts | 20 ------------------- 2 files changed, 2 insertions(+), 37 deletions(-) diff --git a/apps/staged/src/lib/features/branches/BranchCardPrButton.svelte b/apps/staged/src/lib/features/branches/BranchCardPrButton.svelte index c528a06c5..b86a79e38 100644 --- a/apps/staged/src/lib/features/branches/BranchCardPrButton.svelte +++ b/apps/staged/src/lib/features/branches/BranchCardPrButton.svelte @@ -138,9 +138,6 @@ }>('pr-status-changed', (event) => { const payload = event.payload; if (payload.branchId === branchId) { - console.info( - `[PrPolling] pr-status-changed: branch=${branchId}, state=${payload.prState}, checks=${payload.prChecksStatus}, review=${payload.prReviewDecision}` - ); prStatusState = payload.prState; prStatusChecks = payload.prChecksStatus; prStatusReviewDecision = payload.prReviewDecision; @@ -225,18 +222,11 @@ const projectId = branch.projectId; if (prNumber && prStatusState !== 'MERGED' && prStatusState !== 'CLOSED') { - console.info( - `[PrPolling] $effect track: branch=${branchId}, prNumber=${prNumber}, prStatusState=${prStatusState}, checks=${prStatusChecks}` - ); prPollingService.track(branchId, projectId, prStatusChecks === 'PENDING'); return () => { - console.info(`[PrPolling] $effect cleanup untrack: branch=${branchId}`); prPollingService.untrack(branchId); }; } else { - console.info( - `[PrPolling] $effect untrack (terminal/no PR): branch=${branchId}, prNumber=${prNumber}, prStatusState=${prStatusState}` - ); prPollingService.untrack(branchId); } }); @@ -261,13 +251,12 @@ .recoverBranchPr(branch.id) .then((prNumber) => { if (prNumber) { - console.info(`[PrPolling] recovered PR #${prNumber} for branch=${branch.id}`); branch.prNumber = prNumber; prPollingService.refreshNow(branch.projectId); } }) - .catch((err) => { - console.warn(`[PrPolling] PR recovery failed for branch=${branch.id}:`, err); + .catch(() => { + // PR recovery is best-effort; ignore failures }); } }); @@ -374,9 +363,6 @@ if (prNumber) { await commands.updateBranchPr(branch.id, prNumber); branch.prNumber = prNumber; - console.info( - `[PrPolling] refreshNow after PR creation: branch=${branch.id}, prNumber=${prNumber}` - ); prPollingService.refreshNow(branch.projectId); } prStateStore.setPrCreated(branch.id, foundUrl); @@ -454,7 +440,6 @@ prHeadSha = latestCommit.sha; } // Immediately refresh PR status so checks update right away - console.info(`[PrPolling] refreshNow after push: branch=${branch.id}`); prPollingService.refreshNow(branch.projectId); setTimeout(() => { pushStateStore.clearPushState(branch.id); diff --git a/apps/staged/src/lib/services/prPollingService.ts b/apps/staged/src/lib/services/prPollingService.ts index e16c3c305..3d0d31047 100644 --- a/apps/staged/src/lib/services/prPollingService.ts +++ b/apps/staged/src/lib/services/prPollingService.ts @@ -64,16 +64,12 @@ function trackedProjectIds(): Set { async function poll() { if (refreshInFlight || !windowFocused || tracked.size === 0) { - console.info( - `[PrPolling] poll skipped: refreshInFlight=${refreshInFlight}, windowFocused=${windowFocused}, tracked=${tracked.size}` - ); scheduleNext(); return; } refreshInFlight = true; const projectIds = trackedProjectIds(); - console.info(`[PrPolling] poll start: projectIds=[${[...projectIds]}], tracked=${tracked.size}`); for (const projectId of projectIds) { try { @@ -98,20 +94,15 @@ async function poll() { } refreshInFlight = false; - console.info(`[PrPolling] poll end`); scheduleNext(); } function scheduleNext() { stopTimer(); if (tracked.size === 0 || !windowFocused) { - console.info( - `[PrPolling] scheduleNext: not scheduling (tracked=${tracked.size}, windowFocused=${windowFocused})` - ); return; } const interval = getInterval(); - console.info(`[PrPolling] scheduleNext: interval=${interval}ms, tracked=${tracked.size}`); timerId = setTimeout(poll, interval); } @@ -141,14 +132,12 @@ function notifyStale(projectId: string, isStale: boolean) { // --------------------------------------------------------------------------- function handleFocus() { - console.info(`[PrPolling] window focus`); windowFocused = true; // Immediate refresh on focus, then resume schedule poll(); } function handleBlur() { - console.info(`[PrPolling] window blur`); windowFocused = false; stopTimer(); } @@ -177,9 +166,6 @@ function removeWindowListeners() { export function track(branchId: string, projectId: string, hasPendingChecks = false): void { const isFirst = tracked.size === 0; tracked.set(branchId, { projectId, hasPendingChecks }); - console.info( - `[PrPolling] track: branch=${branchId}, project=${projectId}, pendingChecks=${hasPendingChecks}, isFirst=${isFirst}, totalTracked=${tracked.size}` - ); if (isFirst) { ensureWindowListeners(); @@ -193,11 +179,7 @@ export function track(branchId: string, projectId: string, hasPendingChecks = fa /** Stop tracking a branch. */ export function untrack(branchId: string): void { - const existed = tracked.has(branchId); tracked.delete(branchId); - console.info( - `[PrPolling] untrack: branch=${branchId}, existed=${existed}, remaining=${tracked.size}` - ); if (tracked.size === 0) { stopTimer(); removeWindowListeners(); @@ -224,10 +206,8 @@ export function onStale(callback: StaleCallback): () => void { /** Trigger an immediate refresh for a specific project (e.g. after PR creation or push). */ export function refreshNow(projectId: string): void { if (refreshInFlight) { - console.info(`[PrPolling] refreshNow: DROPPED for project=${projectId} (refreshInFlight=true)`); return; } - console.info(`[PrPolling] refreshNow: accepted for project=${projectId}`); refreshInFlight = true; refreshAllPrStatuses(projectId) .catch((e) => From 896d83e98061615caf5b8853dd8960b0f0e9c472 Mon Sep 17 00:00:00 2001 From: Matt Toohey Date: Wed, 15 Apr 2026 17:03:58 +1000 Subject: [PATCH 6/9] feat(staged): make PR polling app-wide with tiered intervals per project The polling service now refreshes all projects, not just the one being viewed. The selected project polls at the existing 60s interval, while background projects poll every 5 minutes. Projects with pending CI checks still use the faster 15s interval regardless of selection. Key changes: - Replace branch-level track/untrack API with project-level setProjects and setSelectedProject, driven by reactive $effects in App.svelte - Per-project lastPolledAt tracking so each project polls independently at its own interval via a single adaptive timer - Remove component-driven track/untrack from BranchCardPrButton; stale notifications now keyed by projectId instead of branchId Co-Authored-By: Claude Opus 4.6 (1M context) --- apps/staged/src/App.svelte | 13 ++ .../branches/BranchCardPrButton.svelte | 31 +-- .../src/lib/services/prPollingService.ts | 195 ++++++++++++------ 3 files changed, 149 insertions(+), 90 deletions(-) diff --git a/apps/staged/src/App.svelte b/apps/staged/src/App.svelte index 5a60062b7..510d88ec4 100644 --- a/apps/staged/src/App.svelte +++ b/apps/staged/src/App.svelte @@ -42,6 +42,8 @@ import { initBloxEnv } from './lib/stores/bloxEnv.svelte'; import { listenForSessionStatus } from './lib/listeners/sessionStatusListener'; import { darkMode } from './lib/stores/isDark.svelte'; + import * as prPollingService from './lib/services/prPollingService'; + import { projectsList } from './lib/features/projects/projectsSidebarState.svelte'; import type { StoreIncompatibility } from './lib/types'; const updaterEnabled = import.meta.env.VITE_UPDATER_ENABLED === 'true'; @@ -62,6 +64,17 @@ let resetting = $state(false); let storeError = $state(null); + // ========================================================================= + // App-wide PR polling — sync project list and selected project reactively + // ========================================================================= + $effect(() => { + prPollingService.setProjects(projectsList.current.map((p) => p.id)); + }); + + $effect(() => { + prPollingService.setSelectedProject(navigation.selectedProjectId); + }); + // Konami code: ↑↑↓↓←→←→BA const konamiSequence = [ 'ArrowUp', diff --git a/apps/staged/src/lib/features/branches/BranchCardPrButton.svelte b/apps/staged/src/lib/features/branches/BranchCardPrButton.svelte index b86a79e38..85371e33a 100644 --- a/apps/staged/src/lib/features/branches/BranchCardPrButton.svelte +++ b/apps/staged/src/lib/features/branches/BranchCardPrButton.svelte @@ -145,7 +145,11 @@ prStatusDraft = payload.prDraft; prHeadSha = payload.prHeadSha; // Update the polling service with the new checks status - prPollingService.updateChecksStatus(branchId, payload.prChecksStatus === 'PENDING'); + prPollingService.updateChecksStatus( + branchId, + branch.projectId, + payload.prChecksStatus === 'PENDING' + ); } }); @@ -210,27 +214,6 @@ return () => clearInterval(interval); }); - // ========================================================================= - // Polling service registration - // ========================================================================= - - // Track/untrack this branch with the centralized polling service when it - // gains or loses a PR number, and when its terminal state changes. - $effect(() => { - const prNumber = branch.prNumber; - const branchId = branch.id; - const projectId = branch.projectId; - - if (prNumber && prStatusState !== 'MERGED' && prStatusState !== 'CLOSED') { - prPollingService.track(branchId, projectId, prStatusChecks === 'PENDING'); - return () => { - prPollingService.untrack(branchId); - }; - } else { - prPollingService.untrack(branchId); - } - }); - // Subscribe to stale-data notifications from the polling service let unsubStale: (() => void) | null = null; @@ -238,8 +221,8 @@ window.addEventListener('keydown', handleOptionDown); window.addEventListener('keyup', handleOptionUp); - unsubStale = prPollingService.onStale((branchId, isStale) => { - if (branchId === branch.id) { + unsubStale = prPollingService.onStale((projectId, isStale) => { + if (projectId === branch.projectId) { prStatusStale = isStale; } }); diff --git a/apps/staged/src/lib/services/prPollingService.ts b/apps/staged/src/lib/services/prPollingService.ts index 3d0d31047..a09e3758d 100644 --- a/apps/staged/src/lib/services/prPollingService.ts +++ b/apps/staged/src/lib/services/prPollingService.ts @@ -1,10 +1,11 @@ /** * Centralized PR status polling service. * - * Replaces the per-component setInterval polling in BranchCardPrButton with a - * single coordinated timer that calls `refreshAllPrStatuses` on the backend. - * The backend already emits per-branch `pr-status-changed` events, so - * components only need to listen for those events — no per-branch IPC calls. + * Polls all projects app-wide. The selected project polls more frequently + * than background projects, and projects with pending CI checks poll fastest. + * + * The backend's `refreshAllPrStatuses` already emits per-branch + * `pr-status-changed` events, so components only need to listen for those. */ import { refreshAllPrStatuses } from '../commands'; @@ -13,19 +14,32 @@ import { refreshAllPrStatuses } from '../commands'; // Types // --------------------------------------------------------------------------- -interface TrackedBranch { - projectId: string; - hasPendingChecks: boolean; -} +type StaleCallback = (projectId: string, isStale: boolean) => void; -type StaleCallback = (branchId: string, isStale: boolean) => void; +// --------------------------------------------------------------------------- +// Intervals +// --------------------------------------------------------------------------- + +const PENDING_INTERVAL = 15_000; // any project with pending CI checks +const SELECTED_INTERVAL = 60_000; // selected project, no pending checks +const BACKGROUND_INTERVAL = 5 * 60_000; // non-selected, no pending checks +const MAX_CONSECUTIVE_FAILURES = 3; // --------------------------------------------------------------------------- // State // --------------------------------------------------------------------------- -/** Branches currently being tracked, keyed by branchId. */ -const tracked = new Map(); +/** All project IDs to poll. */ +const allProjectIds = new Set(); + +/** Currently selected (viewed) project. */ +let selectedProjectId: string | null = null; + +/** Branches with pending checks, keyed by branchId → projectId. */ +const pendingBranches = new Map(); + +/** When each project was last successfully polled. */ +const lastPolledAt = new Map(); /** Consecutive failure count per projectId. */ const failures = new Map(); @@ -36,44 +50,52 @@ const staleCallbacks = new Set(); let timerId: ReturnType | null = null; let refreshInFlight = false; let windowFocused = true; - -// Intervals -const PENDING_INTERVAL = 15_000; -const NORMAL_INTERVAL = 60_000; -const MAX_CONSECUTIVE_FAILURES = 3; +let listenersAttached = false; // --------------------------------------------------------------------------- // Internal helpers // --------------------------------------------------------------------------- -function getInterval(): number { - for (const entry of tracked.values()) { - if (entry.hasPendingChecks) return PENDING_INTERVAL; +function projectHasPendingChecks(projectId: string): boolean { + for (const pId of pendingBranches.values()) { + if (pId === projectId) return true; } - return NORMAL_INTERVAL; + return false; +} + +function getProjectInterval(projectId: string): number { + if (projectHasPendingChecks(projectId)) return PENDING_INTERVAL; + if (projectId === selectedProjectId) return SELECTED_INTERVAL; + return BACKGROUND_INTERVAL; } -/** Collect the distinct projectIds that are currently tracked. */ -function trackedProjectIds(): Set { - const ids = new Set(); - for (const entry of tracked.values()) { - ids.add(entry.projectId); +/** Return project IDs whose polling interval has elapsed. */ +function getProjectsDue(): string[] { + const now = Date.now(); + const due: string[] = []; + for (const projectId of allProjectIds) { + const interval = getProjectInterval(projectId); + const last = lastPolledAt.get(projectId) ?? 0; + if (now - last >= interval) { + due.push(projectId); + } } - return ids; + return due; } async function poll() { - if (refreshInFlight || !windowFocused || tracked.size === 0) { + if (refreshInFlight || !windowFocused || allProjectIds.size === 0) { scheduleNext(); return; } refreshInFlight = true; - const projectIds = trackedProjectIds(); + const due = getProjectsDue(); - for (const projectId of projectIds) { + for (const projectId of due) { try { await refreshAllPrStatuses(projectId); + lastPolledAt.set(projectId, Date.now()); // Reset failure counter on success const prev = failures.get(projectId) ?? 0; if (prev > 0) { @@ -99,11 +121,20 @@ async function poll() { function scheduleNext() { stopTimer(); - if (tracked.size === 0 || !windowFocused) { - return; + if (allProjectIds.size === 0 || !windowFocused) return; + + const now = Date.now(); + let minDelay = Infinity; + for (const projectId of allProjectIds) { + const interval = getProjectInterval(projectId); + const last = lastPolledAt.get(projectId) ?? 0; + const remaining = Math.max(0, interval - (now - last)); + minDelay = Math.min(minDelay, remaining); } - const interval = getInterval(); - timerId = setTimeout(poll, interval); + + if (!Number.isFinite(minDelay)) return; + // Floor at 1s to avoid tight loops + timerId = setTimeout(poll, Math.max(1_000, minDelay)); } function stopTimer() { @@ -114,15 +145,11 @@ function stopTimer() { } function notifyStale(projectId: string, isStale: boolean) { - for (const [branchId, entry] of tracked) { - if (entry.projectId === projectId) { - for (const cb of staleCallbacks) { - try { - cb(branchId, isStale); - } catch { - // ignore callback errors - } - } + for (const cb of staleCallbacks) { + try { + cb(projectId, isStale); + } catch { + // ignore callback errors } } } @@ -133,7 +160,6 @@ function notifyStale(projectId: string, isStale: boolean) { function handleFocus() { windowFocused = true; - // Immediate refresh on focus, then resume schedule poll(); } @@ -142,8 +168,6 @@ function handleBlur() { stopTimer(); } -let listenersAttached = false; - function ensureWindowListeners() { if (listenersAttached) return; window.addEventListener('focus', handleFocus); @@ -162,37 +186,67 @@ function removeWindowListeners() { // Public API // --------------------------------------------------------------------------- -/** Start tracking a branch for PR status polling. */ -export function track(branchId: string, projectId: string, hasPendingChecks = false): void { - const isFirst = tracked.size === 0; - tracked.set(branchId, { projectId, hasPendingChecks }); +/** Set the full list of project IDs to poll. Starts/stops polling as needed. */ +export function setProjects(projectIds: string[]): void { + const newIds = new Set(projectIds); - if (isFirst) { + // Remove projects no longer in the list + for (const id of allProjectIds) { + if (!newIds.has(id)) { + allProjectIds.delete(id); + lastPolledAt.delete(id); + failures.delete(id); + } + } + + // Clean up pending branches for removed projects + for (const [branchId, projectId] of pendingBranches) { + if (!newIds.has(projectId)) { + pendingBranches.delete(branchId); + } + } + + // Add new projects + for (const id of newIds) { + allProjectIds.add(id); + } + + if (allProjectIds.size > 0) { ensureWindowListeners(); - // Start polling immediately + // Trigger poll — new projects have no lastPolledAt so they'll be due poll(); } else { - // Interval may have changed — reschedule - scheduleNext(); - } -} - -/** Stop tracking a branch. */ -export function untrack(branchId: string): void { - tracked.delete(branchId); - if (tracked.size === 0) { stopTimer(); removeWindowListeners(); failures.clear(); } } -/** Update whether a tracked branch has pending checks (affects poll interval). */ -export function updateChecksStatus(branchId: string, hasPendingChecks: boolean): void { - const entry = tracked.get(branchId); - if (entry && entry.hasPendingChecks !== hasPendingChecks) { - entry.hasPendingChecks = hasPendingChecks; - // Interval may have changed — reschedule +/** Set the currently selected project (polls more frequently). */ +export function setSelectedProject(projectId: string | null): void { + if (selectedProjectId === projectId) return; + selectedProjectId = projectId; + if (projectId && allProjectIds.has(projectId)) { + // Selected project's interval just changed — trigger a poll if it's due + poll(); + } else { + scheduleNext(); + } +} + +/** Update whether a branch has pending CI checks (affects its project's poll interval). */ +export function updateChecksStatus( + branchId: string, + projectId: string, + hasPendingChecks: boolean +): void { + const hadPending = pendingBranches.has(branchId); + if (hasPendingChecks) { + pendingBranches.set(branchId, projectId); + } else { + pendingBranches.delete(branchId); + } + if (hadPending !== hasPendingChecks) { scheduleNext(); } } @@ -210,6 +264,15 @@ export function refreshNow(projectId: string): void { } refreshInFlight = true; refreshAllPrStatuses(projectId) + .then(() => { + lastPolledAt.set(projectId, Date.now()); + // Reset failure counter on success + const prev = failures.get(projectId) ?? 0; + if (prev > 0) { + failures.set(projectId, 0); + notifyStale(projectId, false); + } + }) .catch((e) => console.error(`[PrPollingService] immediate refresh failed for project=${projectId}:`, e) ) From c7abde0b8d26f69991204c602afa8e6cfc358632 Mon Sep 17 00:00:00 2001 From: Matt Toohey Date: Wed, 22 Apr 2026 16:13:25 +1000 Subject: [PATCH 7/9] =?UTF-8?q?fix(staged):=20address=20PR=20polling=20rev?= =?UTF-8?q?iew=20comments=20=E2=80=94=20concurrency=20and=20cleanup?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Resolve code review feedback on the centralized PR polling service: - poll(): remove scheduleNext() from the early-return path when refreshInFlight is true — the in-flight operation's finally block already reschedules, avoiding a potential busy-wait cycle - setProjects(): short-circuit when the set of project IDs hasn't changed, preventing unnecessary poll triggers from Svelte $effect re-fires - refreshNow(): queue the projectId when a refresh is already in-flight instead of silently dropping it, so post-PR-creation and post-push refreshes are never lost - Stale notification: use exact equality (=== MAX_CONSECUTIVE_FAILURES) so the callback fires only on the transition to stale, not every subsequent failure cycle - Move unsubStale from onMount/onDestroy to $effect with cleanup, making it immune to double-mount (HMR, keyed re-render) and automatically reactive to branch.projectId changes - Add shouldAttemptRecovery() guard in the polling service to prevent N concurrent `gh pr view` CLI calls when many BranchCardPrButton components mount simultaneously for branches without PR numbers --- .../branches/BranchCardPrButton.svelte | 25 ++++++----- .../src/lib/services/prPollingService.ts | 44 +++++++++++++++++-- 2 files changed, 56 insertions(+), 13 deletions(-) diff --git a/apps/staged/src/lib/features/branches/BranchCardPrButton.svelte b/apps/staged/src/lib/features/branches/BranchCardPrButton.svelte index 85371e33a..d711b8620 100644 --- a/apps/staged/src/lib/features/branches/BranchCardPrButton.svelte +++ b/apps/staged/src/lib/features/branches/BranchCardPrButton.svelte @@ -214,22 +214,28 @@ return () => clearInterval(interval); }); - // Subscribe to stale-data notifications from the polling service - let unsubStale: (() => void) | null = null; + // Subscribe to stale-data notifications from the polling service. + // Using $effect with cleanup so the subscription is immune to double-mount + // (e.g. HMR, keyed re-render) and automatically tracks branch.projectId. + $effect(() => { + const projectId = branch.projectId; + const unsub = prPollingService.onStale((staleProjectId, isStale) => { + if (staleProjectId === projectId) { + prStatusStale = isStale; + } + }); + return () => unsub(); + }); onMount(() => { window.addEventListener('keydown', handleOptionDown); window.addEventListener('keyup', handleOptionUp); - unsubStale = prPollingService.onStale((projectId, isStale) => { - if (projectId === branch.projectId) { - prStatusStale = isStale; - } - }); - // PR recovery: if the branch has been pushed but has no PR number, // check GitHub for an existing open PR on this branch name. - if (!branch.prNumber && isRemote) { + // The shouldAttemptRecovery guard prevents N concurrent `gh pr view` + // CLI calls when many components mount simultaneously. + if (!branch.prNumber && isRemote && prPollingService.shouldAttemptRecovery(branch.id)) { commands .recoverBranchPr(branch.id) .then((prNumber) => { @@ -247,7 +253,6 @@ onDestroy(() => { window.removeEventListener('keydown', handleOptionDown); window.removeEventListener('keyup', handleOptionUp); - unsubStale?.(); }); // ========================================================================= diff --git a/apps/staged/src/lib/services/prPollingService.ts b/apps/staged/src/lib/services/prPollingService.ts index a09e3758d..7c8ea9d0d 100644 --- a/apps/staged/src/lib/services/prPollingService.ts +++ b/apps/staged/src/lib/services/prPollingService.ts @@ -52,6 +52,9 @@ let refreshInFlight = false; let windowFocused = true; let listenersAttached = false; +/** Project ID queued for immediate refresh while another refresh is in-flight. */ +let pendingRefreshProjectId: string | null = null; + // --------------------------------------------------------------------------- // Internal helpers // --------------------------------------------------------------------------- @@ -85,7 +88,9 @@ function getProjectsDue(): string[] { async function poll() { if (refreshInFlight || !windowFocused || allProjectIds.size === 0) { - scheduleNext(); + // Don't reschedule here — the in-flight operation's `finally` block + // already calls scheduleNext(), and the other two cases (unfocused / + // empty) intentionally have no timer running. return; } @@ -109,7 +114,7 @@ async function poll() { `[PrPollingService] refreshAllPrStatuses failed for project=${projectId} (attempt ${count}):`, e ); - if (count >= MAX_CONSECUTIVE_FAILURES) { + if (count === MAX_CONSECUTIVE_FAILURES) { notifyStale(projectId, true); } } @@ -190,6 +195,11 @@ function removeWindowListeners() { export function setProjects(projectIds: string[]): void { const newIds = new Set(projectIds); + // Short-circuit if the set of project IDs hasn't changed + if (newIds.size === allProjectIds.size && projectIds.every((id) => allProjectIds.has(id))) { + return; + } + // Remove projects no longer in the list for (const id of allProjectIds) { if (!newIds.has(id)) { @@ -257,9 +267,30 @@ export function onStale(callback: StaleCallback): () => void { return () => staleCallbacks.delete(callback); } +// --------------------------------------------------------------------------- +// PR recovery coordination +// --------------------------------------------------------------------------- + +/** Branch IDs for which recovery has already been attempted (or is in progress). */ +const recoveryAttempted = new Set(); + +/** + * Guard for PR recovery: returns true if recovery should proceed for this + * branch, false if it has already been attempted or is in progress. + * Prevents N concurrent `gh pr view` CLI calls when many BranchCardPrButton + * components mount simultaneously for branches without PR numbers. + */ +export function shouldAttemptRecovery(branchId: string): boolean { + if (recoveryAttempted.has(branchId)) return false; + recoveryAttempted.add(branchId); + return true; +} + /** Trigger an immediate refresh for a specific project (e.g. after PR creation or push). */ export function refreshNow(projectId: string): void { if (refreshInFlight) { + // Queue so the project is refreshed as soon as the current operation finishes. + pendingRefreshProjectId = projectId; return; } refreshInFlight = true; @@ -278,6 +309,13 @@ export function refreshNow(projectId: string): void { ) .finally(() => { refreshInFlight = false; - scheduleNext(); + // Drain any queued immediate-refresh request before rescheduling. + const queued = pendingRefreshProjectId; + pendingRefreshProjectId = null; + if (queued) { + refreshNow(queued); + } else { + scheduleNext(); + } }); } From 2cb1f0624c4077226d4becbbf4ceb9de38e48bb4 Mon Sep 17 00:00:00 2001 From: Matt Toohey Date: Thu, 23 Apr 2026 09:12:42 +1000 Subject: [PATCH 8/9] =?UTF-8?q?fix(staged):=20address=20review=20=E2=80=94?= =?UTF-8?q?=20use=20Set=20for=20pending=20refreshes=20and=20retry=20failed?= =?UTF-8?q?=20recovery?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Resolve two review comments on the PR polling service: - pendingRefreshProjectId: replace single-slot string with a Set so rapid refreshNow() calls for different projects (e.g. user creates PRs across multiple projects) all get queued and drained, instead of silently dropping all but the last one. - recoveryAttempted: add clearRecoveryAttempt() and call it from the component's catch handler so transient failures (network blips at startup) don't permanently prevent PR number recovery for a branch. The guard still prevents concurrent calls during the same mount cycle. Co-Authored-By: Claude Opus 4.6 (1M context) --- .../branches/BranchCardPrButton.svelte | 4 ++- .../src/lib/services/prPollingService.ts | 32 ++++++++++++++----- 2 files changed, 27 insertions(+), 9 deletions(-) diff --git a/apps/staged/src/lib/features/branches/BranchCardPrButton.svelte b/apps/staged/src/lib/features/branches/BranchCardPrButton.svelte index d711b8620..484094287 100644 --- a/apps/staged/src/lib/features/branches/BranchCardPrButton.svelte +++ b/apps/staged/src/lib/features/branches/BranchCardPrButton.svelte @@ -245,7 +245,9 @@ } }) .catch(() => { - // PR recovery is best-effort; ignore failures + // PR recovery is best-effort; clear the guard so it can be + // retried on next mount (e.g. after a transient network error). + prPollingService.clearRecoveryAttempt(branch.id); }); } }); diff --git a/apps/staged/src/lib/services/prPollingService.ts b/apps/staged/src/lib/services/prPollingService.ts index 7c8ea9d0d..72330726a 100644 --- a/apps/staged/src/lib/services/prPollingService.ts +++ b/apps/staged/src/lib/services/prPollingService.ts @@ -52,8 +52,8 @@ let refreshInFlight = false; let windowFocused = true; let listenersAttached = false; -/** Project ID queued for immediate refresh while another refresh is in-flight. */ -let pendingRefreshProjectId: string | null = null; +/** Project IDs queued for immediate refresh while another refresh is in-flight. */ +const pendingRefreshProjectIds = new Set(); // --------------------------------------------------------------------------- // Internal helpers @@ -286,11 +286,20 @@ export function shouldAttemptRecovery(branchId: string): boolean { return true; } +/** + * Clear the recovery guard for a branch so it can be retried. + * Call this when recovery fails (e.g. network error) so a transient + * failure doesn't permanently prevent recovery for that branch. + */ +export function clearRecoveryAttempt(branchId: string): void { + recoveryAttempted.delete(branchId); +} + /** Trigger an immediate refresh for a specific project (e.g. after PR creation or push). */ export function refreshNow(projectId: string): void { if (refreshInFlight) { // Queue so the project is refreshed as soon as the current operation finishes. - pendingRefreshProjectId = projectId; + pendingRefreshProjectIds.add(projectId); return; } refreshInFlight = true; @@ -309,11 +318,18 @@ export function refreshNow(projectId: string): void { ) .finally(() => { refreshInFlight = false; - // Drain any queued immediate-refresh request before rescheduling. - const queued = pendingRefreshProjectId; - pendingRefreshProjectId = null; - if (queued) { - refreshNow(queued); + // Drain all queued immediate-refresh requests before rescheduling. + if (pendingRefreshProjectIds.size > 0) { + const queued = [...pendingRefreshProjectIds]; + pendingRefreshProjectIds.clear(); + // Refresh each queued project sequentially via the same path + // (the first call sets refreshInFlight, subsequent ones queue again). + for (const queuedId of queued) { + refreshNow(queuedId); + // Only the first call will actually run — the rest re-queue + // because refreshInFlight is set by the first call. This is + // correct: they'll drain on the next finally cycle. + } } else { scheduleNext(); } From 92ead487586d373f5fa74e965843a984dbce6c34 Mon Sep 17 00:00:00 2001 From: Matt Toohey Date: Thu, 23 Apr 2026 09:23:05 +1000 Subject: [PATCH 9/9] fix(staged): simplify pending refresh drain loop in PR polling service Replace the indirect loop-that-re-queues pattern in refreshNow's .finally() block with a clearer approach: take the first queued project, put the rest back in the set, and call refreshNow once. The previous pattern called refreshNow() for every queued ID in a loop, but only the first call actually ran (the rest hit the refreshInFlight guard and re-queued themselves). The new code makes this one-runs-rest-re-queue contract explicit without relying on the guard as a side effect. Co-Authored-By: Claude Opus 4.6 (1M context) --- apps/staged/src/lib/services/prPollingService.ts | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/apps/staged/src/lib/services/prPollingService.ts b/apps/staged/src/lib/services/prPollingService.ts index 72330726a..447b91c01 100644 --- a/apps/staged/src/lib/services/prPollingService.ts +++ b/apps/staged/src/lib/services/prPollingService.ts @@ -318,18 +318,15 @@ export function refreshNow(projectId: string): void { ) .finally(() => { refreshInFlight = false; - // Drain all queued immediate-refresh requests before rescheduling. + // Drain queued immediate-refresh requests one at a time. if (pendingRefreshProjectIds.size > 0) { const queued = [...pendingRefreshProjectIds]; pendingRefreshProjectIds.clear(); - // Refresh each queued project sequentially via the same path - // (the first call sets refreshInFlight, subsequent ones queue again). - for (const queuedId of queued) { - refreshNow(queuedId); - // Only the first call will actually run — the rest re-queue - // because refreshInFlight is set by the first call. This is - // correct: they'll drain on the next finally cycle. + // Re-queue all but the first; they'll drain on the next finally cycle. + for (let i = 1; i < queued.length; i++) { + pendingRefreshProjectIds.add(queued[i]); } + refreshNow(queued[0]); } else { scheduleNext(); }