diff --git a/src/browser/components/AppLoader/AppLoader.tsx b/src/browser/components/AppLoader/AppLoader.tsx index 13a52af414..78f350ceac 100644 --- a/src/browser/components/AppLoader/AppLoader.tsx +++ b/src/browser/components/AppLoader/AppLoader.tsx @@ -179,6 +179,15 @@ function AppLoaderInner() { // Sync stores when metadata finishes loading useEffect(() => { + // #4662: git/PR probes for a workspace wait for its chat replay to settle. Wire the gate + // before setClient/syncWorkspaces below, which can refresh synchronously. + const chatReplayGate = { + isReplayPending: workspaceStore.isWorkspaceChatReplayPending, + subscribeKey: workspaceStore.subscribeKey, + }; + gitStatusStore.setChatReplayGate(chatReplayGate); + getPRStatusStoreInstance().setChatReplayGate(chatReplayGate); + // Keep store clients in sync even during backend restarts (api can be null while reconnecting). workspaceStoreInstance.setClient(api ?? null); gitStatusStore.setClient(api ?? null); diff --git a/src/browser/stores/GitStatusStore.test.ts b/src/browser/stores/GitStatusStore.test.ts index 0243620944..3fddefa225 100644 --- a/src/browser/stores/GitStatusStore.test.ts +++ b/src/browser/stores/GitStatusStore.test.ts @@ -484,6 +484,47 @@ describe("GitStatusStore", () => { unsub(); }); + // #4662: opening a workspace must not spawn git status/fetch while its chat replay runs. + it("defers a workspace's status and fetch until its chat replay settles", async () => { + const pendingId = "replay-pending"; + const readyId = "replay-settled"; + let pending = true; + const gateListeners = new Set<() => void>(); + store.setChatReplayGate({ + isReplayPending: (workspaceId) => pending && workspaceId === pendingId, + subscribeKey: (workspaceId, listener) => { + if (workspaceId !== pendingId) return () => undefined; + gateListeners.add(listener); + return () => gateListeners.delete(listener); + }, + }); + const scriptsFor = (workspaceId: string) => + mockExecuteBash.mock.calls + .map((call) => (call as unknown[])[0] as { workspaceId: string; script: string }) + .filter((args) => args.workspaceId === workspaceId) + .map((args) => (args.script === GIT_FETCH_SCRIPT ? "fetch" : "status")); + store.syncWorkspaces( + new Map([ + // Separate projects: local fetches are deduplicated per project. + [pendingId, { ...createWorkspaceMetadata(pendingId), projectName: "pending-project" }], + [readyId, createWorkspaceMetadata(readyId)], + ]) + ); + const unsubscribers = [pendingId, readyId].map((id) => store.subscribeKey(id, jest.fn())); + + await waitUntil(() => scriptsFor(readyId).length === 2); + expect(scriptsFor(pendingId)).toEqual([]); + expect(gateListeners.size).toBe(1); + + pending = false; + for (const listener of Array.from(gateListeners)) listener(); + + await waitUntil(() => scriptsFor(pendingId).length === 2); + expect(scriptsFor(pendingId).sort()).toEqual(["fetch", "status"]); + expect(gateListeners.size).toBe(0); + for (const unsubscribe of unsubscribers) unsubscribe(); + }); + describe("passive fetch runtime gating", () => { it("skips passive fetch and status checks for devcontainer with unresolved runtime status", async () => { store.dispose(); diff --git a/src/browser/stores/GitStatusStore.ts b/src/browser/stores/GitStatusStore.ts index b339167f07..75dcec2634 100644 --- a/src/browser/stores/GitStatusStore.ts +++ b/src/browser/stores/GitStatusStore.ts @@ -5,6 +5,7 @@ import type { FrontendWorkspaceMetadata, GitStatus } from "@/common/types/worksp import { readPersistedState } from "@/browser/hooks/usePersistedState"; import { RefreshController } from "@/browser/utils/RefreshController"; import { repoRootBashOptions } from "@/browser/utils/executeBash"; +import { deferWhileChatReplayPending, type ChatReplayGate } from "@/browser/utils/chatReplayGate"; import { canRunPassiveRuntimeCommand, onPassiveRuntimeEligible, @@ -97,6 +98,8 @@ export class GitStatusStore { private fetchCache = new Map(); private runtimeStatusRetryUnsubscribers = new Map void>(); private runtimeFetchRetryUnsubscribers = new Map void>(); + private chatReplayGate: ChatReplayGate | null = null; + private chatReplayRetryUnsubscribers = new Map void>(); private client: RouterClient | null = null; private immediateUpdateQueued = false; private workspaceMetadata = new Map(); @@ -141,6 +144,11 @@ export class GitStatusStore { } } + /** Defer refreshes of a workspace while its chat replay is pending; null disables gating. */ + setChatReplayGate(gate: ChatReplayGate | null): void { + this.chatReplayGate = gate; + } + /** * Subscribe to git status changes (any workspace). * Delegates to MapStore's subscribeAny. @@ -338,6 +346,7 @@ export class GitStatusStore { this.cleanupRuntimeRetryMap(this.runtimeStatusRetryUnsubscribers, metadata); this.cleanupRuntimeRetryMap(this.runtimeFetchRetryUnsubscribers, metadata); + this.cleanupRuntimeRetryMap(this.chatReplayRetryUnsubscribers, metadata); // Remove statuses for deleted workspaces // Iterate plain map (statusCache) for membership, not reactive store @@ -371,9 +380,17 @@ export class GitStatusStore { return; } - // Only poll workspaces that have active subscribers. - const workspaces = Array.from(this.workspaceMetadata.values()).filter((ws) => - this.hasWorkspaceSubscribers(ws.id) + // Only poll workspaces that have active subscribers and whose chat replay has settled + // (#4662: a deferred workspace skips both the status script and git fetch). + const workspaces = Array.from(this.workspaceMetadata.values()).filter( + (ws) => + this.hasWorkspaceSubscribers(ws.id) && + !deferWhileChatReplayPending( + this.chatReplayGate, + this.chatReplayRetryUnsubscribers, + ws.id, + () => this.refreshController.requestImmediate() + ) ); if (workspaces.length === 0) { @@ -1085,6 +1102,10 @@ export class GitStatusStore { unsub(); } this.runtimeFetchRetryUnsubscribers.clear(); + for (const unsub of this.chatReplayRetryUnsubscribers.values()) { + unsub(); + } + this.chatReplayRetryUnsubscribers.clear(); this.refreshController.dispose(); } diff --git a/src/browser/stores/PRStatusStore.test.ts b/src/browser/stores/PRStatusStore.test.ts index 675461ea8e..92c09ea35c 100644 --- a/src/browser/stores/PRStatusStore.test.ts +++ b/src/browser/stores/PRStatusStore.test.ts @@ -211,6 +211,60 @@ describe("passive refresh runtime gating", () => { }); }); +// #4662: opening a workspace must not spawn gh pr/stack probes while its chat replay runs. +describe("chat replay gating", () => { + it.each(["the chat replay settles", "the last subscriber leaves"] as const)( + "defers PR and stack probes until %s", + async (release) => { + const metadata = createWorkspaceMetadata("replay-pending", DEFAULT_RUNTIME_CONFIG); + const executeBash = mock(() => + Promise.resolve({ success: false as const, error: "gh unavailable" }) + ); + let pending = true; + const gateListeners = new Set<() => void>(); + const store = new PRStatusStore({ getStatus: () => null }); + + try { + store.setChatReplayGate({ + isReplayPending: () => pending, + subscribeKey: (_workspaceId, listener) => { + gateListeners.add(listener); + return () => gateListeners.delete(listener); + }, + }); + store.setClient({ + workspace: { executeBash }, + } as unknown as Parameters[0]); + store.syncWorkspaces(new Map([[metadata.id, metadata]])); + const unsubscribe = store.subscribeWorkspace(metadata.id, () => undefined); + + await waitUntil(() => gateListeners.size === 1); + expect(executeBash.mock.calls.length).toBe(0); + + if (release === "the last subscriber leaves") { + // The last subscriber leaving must release the watcher on WorkspaceStore. + unsubscribe(); + expect(gateListeners.size).toBe(0); + return; + } + pending = false; + for (const listener of Array.from(gateListeners)) listener(); + + await waitUntil(() => executeBash.mock.calls.length === 2); + const scripts = executeBash.mock.calls.map( + (call) => ((call as unknown[])[0] as { script: string }).script + ); + expect(scripts.some((script) => script.includes("gh pr view"))).toBe(true); + expect(scripts.some((script) => script.includes("gh stack view"))).toBe(true); + expect(gateListeners.size).toBe(0); + unsubscribe(); + } finally { + store.dispose(); + } + } + ); +}); + describe("parseMergeQueueEntry", () => { it("returns null for null and undefined", () => { expect(parseMergeQueueEntry(null)).toBeNull(); diff --git a/src/browser/stores/PRStatusStore.ts b/src/browser/stores/PRStatusStore.ts index ce7fc5938a..5dc737cb0e 100644 --- a/src/browser/stores/PRStatusStore.ts +++ b/src/browser/stores/PRStatusStore.ts @@ -29,6 +29,7 @@ import { onPassiveRuntimeEligible, type PassiveRuntimeDeps, } from "@/browser/utils/runtimeExecutionPolicy"; +import { deferWhileChatReplayPending, type ChatReplayGate } from "@/browser/utils/chatReplayGate"; /** * Parse a GitHub PR URL to extract owner, repo, and number. * Returns null if the URL is not a valid GitHub PR URL. @@ -331,6 +332,8 @@ export class PRStatusStore { private workspacePRCache = new Map(); private workspaceStackCache = new Map(); private runtimeRetryUnsubscribers = new Map void>(); + private chatReplayGate: ChatReplayGate | null = null; + private chatReplayRetryUnsubscribers = new Map void>(); // Track active subscriptions per workspace so we only refresh workspaces that are actually visible. private workspaceSubscriptionCounts = new Map(); @@ -384,6 +387,11 @@ export class PRStatusStore { } } + /** Defer refreshes of a workspace while its chat replay is pending; null disables gating. */ + setChatReplayGate(gate: ChatReplayGate | null): void { + this.chatReplayGate = gate; + } + syncWorkspaces(metadata: Map): void { if (!this.isActive && metadata.size > 0) { this.isActive = true; @@ -396,6 +404,12 @@ export class PRStatusStore { this.runtimeRetryUnsubscribers.delete(id); } } + for (const [id, unsubscribe] of this.chatReplayRetryUnsubscribers) { + if (!metadata.has(id)) { + unsubscribe(); + this.chatReplayRetryUnsubscribers.delete(id); + } + } this.refreshController.bindListeners(); this.refreshController.requestImmediate(); } @@ -428,6 +442,8 @@ export class PRStatusStore { this.workspaceSubscriptionCounts.delete(workspaceId); this.runtimeRetryUnsubscribers.get(workspaceId)?.(); this.runtimeRetryUnsubscribers.delete(workspaceId); + this.chatReplayRetryUnsubscribers.get(workspaceId)?.(); + this.chatReplayRetryUnsubscribers.delete(workspaceId); } else { this.workspaceSubscriptionCounts.set(workspaceId, next); } @@ -909,6 +925,18 @@ export class PRStatusStore { const refreshes: Array> = []; for (const workspaceId of workspaceIds) { + // #4662: defer gh pr/stack probes while the workspace's chat replay is pending. + if ( + deferWhileChatReplayPending( + this.chatReplayGate, + this.chatReplayRetryUnsubscribers, + workspaceId, + () => this.refreshController.requestImmediate() + ) + ) { + continue; + } + const shouldFetchPR = this.shouldFetchWorkspace(this.workspacePRCache.get(workspaceId), now); const shouldFetchStack = this.shouldFetchStack( this.workspaceStackCache.get(workspaceId), @@ -976,6 +1004,10 @@ export class PRStatusStore { unsubscribe(); } this.runtimeRetryUnsubscribers.clear(); + for (const unsubscribe of this.chatReplayRetryUnsubscribers.values()) { + unsubscribe(); + } + this.chatReplayRetryUnsubscribers.clear(); this.refreshController.dispose(); } } diff --git a/src/browser/stores/WorkspaceStore.test.ts b/src/browser/stores/WorkspaceStore.test.ts index 71ebf9d662..289a1abb65 100644 --- a/src/browser/stores/WorkspaceStore.test.ts +++ b/src/browser/stores/WorkspaceStore.test.ts @@ -1476,6 +1476,41 @@ describe("WorkspaceStore", () => { expect.anything() ); }); + // #4662: workspace-open git/PR probes wait on this gate, so it must open on every way + // the first replay can settle, or the probes would never run for the workspace. + it.each(["caught-up", "attempt end", "switch away"] as const)( + "reports the chat replay pending until %s", + async (settle) => { + const workspaceId = `workspace-replay-gate-${settle.replace(" ", "-")}`; + const firstAttempt = createControllableAsyncIterable(); + let subscriptions = 0; + mockOnChat.mockImplementation(async function* (input, options) { + if (input?.workspaceId !== workspaceId || subscriptions++ > 0) { + await waitForAbortSignal(options?.signal); + return; + } + options?.signal?.addEventListener("abort", () => firstAttempt.close(), { once: true }); + yield* firstAttempt.iterable; + }); + createAndAddWorkspace(store, workspaceId); + expect(store.isWorkspaceChatReplayPending(workspaceId)).toBe(true); + expect(await waitUntil(() => subscriptions === 1)).toBe(true); + + let notified = false; + const unsubscribe = store.subscribeKey(workspaceId, () => { + notified ||= !store.isWorkspaceChatReplayPending(workspaceId); + }); + if (settle === "caught-up") firstAttempt.push(caughtUpEvent()); + else if (settle === "attempt end") firstAttempt.close(); + else createAndAddWorkspace(store, `${workspaceId}-other`); + + expect(await waitUntil(() => notified)).toBe(true); + expect(store.isWorkspaceChatReplayPending(workspaceId)).toBe(false); + unsubscribe(); + mockChatScript([], { keepOpen: true }); + } + ); + it("keeps transcript hydration active across full replay resets", async () => { const workspaceId = "workspace-full-replay-hydration"; diff --git a/src/browser/stores/WorkspaceStore.ts b/src/browser/stores/WorkspaceStore.ts index 806f448e8f..42c1f11eb3 100644 --- a/src/browser/stores/WorkspaceStore.ts +++ b/src/browser/stores/WorkspaceStore.ts @@ -900,6 +900,9 @@ export class WorkspaceStore { // Workspace currently owning the live onChat subscription. private activeOnChatWorkspaceId: string | null = null; + // Workspaces whose first onChat replay since activation has not settled yet (#4662). + // Kept outside chatTransientState because full-replay resets replace transient objects. + private chatReplayPendingWorkspaces = new Set(); // Loop signal of that subscription, so a refresh request can bind to the loop it was made under. private activeOnChatSignal: AbortSignal | null = null; // The in-flight onChat attempt per workspace (set in subscribe, cleared when the attempt finishes). @@ -1803,6 +1806,7 @@ export class WorkspaceStore { unsubscribe(); } this.ipcUnsubscribers.delete(previousActiveWorkspaceId); + this.chatReplayPendingWorkspaces.delete(previousActiveWorkspaceId); this.activeOnChatWorkspaceId = null; this.activeOnChatSignal = null; } @@ -1819,6 +1823,9 @@ export class WorkspaceStore { const controller = new AbortController(); this.ipcUnsubscribers.set(targetWorkspaceId, () => controller.abort()); + // Set even without a client: probes cannot run without one either, and the replay + // starts once the client arrives. + this.chatReplayPendingWorkspaces.add(targetWorkspaceId); this.activeOnChatWorkspaceId = targetWorkspaceId; this.activeOnChatSignal = controller.signal; void this.runOnChatSubscription(targetWorkspaceId, controller.signal); @@ -3186,6 +3193,21 @@ export class WorkspaceStore { return transient !== undefined && transient.caughtUp && transient.historyVerified; } + /** + * Whether the active workspace is still waiting for its first onChat replay since it was + * activated. Workspace-open git/PR probes wait for this (#4662) so their process spawns + * and result renders do not compete with the replay's history read and transcript paint. + * The gate opens on caught-up (complete or failed replay), when an attempt ends without + * caught-up (transport error, stall watchdog), or when the workspace stops being active. + * Subscribers of {@link subscribeKey} are notified when it opens. + */ + isWorkspaceChatReplayPending(workspaceId: string): boolean { + return ( + this.activeOnChatWorkspaceId === workspaceId && + this.chatReplayPendingWorkspaces.has(workspaceId) + ); + } + getWorkspaceHistoryEpoch(workspaceId: string): number { return this.aggregators.get(workspaceId)?.getHistoryEpoch() ?? 0; } @@ -4378,10 +4400,15 @@ export class WorkspaceStore { } if (!this.isWorkspaceRegistered(workspaceId)) return; this.clearReplayBuffers(workspaceId); + // An attempt that ended without caught-up (transport error, stall watchdog) opens + // the replay gate anyway, so deferred git/PR probes cannot wait forever on retries. + const openedReplayGate = this.chatReplayPendingWorkspaces.delete(workspaceId); const transient = this.chatTransientState.get(workspaceId); if (transient) { // Backoff is still catch-up; cleared stream buffers must also invalidate cached barriers. transient.isHydratingTranscript = true; + } + if (transient || openedReplayGate) { this.states.bump(workspaceId); } if (transient && !transient.caughtUp && this.preReplayUsageSnapshot.delete(workspaceId)) @@ -4576,6 +4603,7 @@ export class WorkspaceStore { this.activeOnChatWorkspaceId = null; this.activeOnChatSignal = null; } + this.chatReplayPendingWorkspaces.delete(workspaceId); this.currentOnChatAttempts.delete(workspaceId); // A pending refresh can never get its baseline from a removed workspace. this.settleTranscriptRefresh(workspaceId, { @@ -4696,6 +4724,7 @@ export class WorkspaceStore { this.activeWorkspaceId = null; this.activeOnChatWorkspaceId = null; this.activeOnChatSignal = null; + this.chatReplayPendingWorkspaces.clear(); this.pendingReplayReset.clear(); this.states.clear(); this.derived.clear(); @@ -5072,6 +5101,7 @@ export class WorkspaceStore { // store never requests live mode (it replays no history), so a live caught-up lets // events flow but leaves the mutation barrier closed. transient.caughtUp = true; + this.chatReplayPendingWorkspaces.delete(workspaceId); transient.historyVerified = replay !== "live"; transient.replayFailed = false; transient.isHydratingTranscript = false; @@ -5436,6 +5466,8 @@ export const workspaceStore = { */ isWorkspaceTranscriptCaughtUp: (workspaceId: string) => getStoreInstance().isWorkspaceTranscriptCaughtUp(workspaceId), + isWorkspaceChatReplayPending: (workspaceId: string) => + getStoreInstance().isWorkspaceChatReplayPending(workspaceId), /** Per-workspace change notifications, so barrier-derived disabled states can subscribe. */ subscribeKey: (workspaceId: string, listener: () => void) => getStoreInstance().subscribeKey(workspaceId, listener), diff --git a/src/browser/utils/chatReplayGate.ts b/src/browser/utils/chatReplayGate.ts new file mode 100644 index 0000000000..bc24cc1f93 --- /dev/null +++ b/src/browser/utils/chatReplayGate.ts @@ -0,0 +1,47 @@ +/** + * Read-only view of WorkspaceStore's chat-replay state used to defer passive + * git/PR probes while a workspace's onChat history replay is still pending (#4662). + */ +export interface ChatReplayGate { + isReplayPending(workspaceId: string): boolean; + subscribeKey(workspaceId: string, listener: () => void): () => void; +} + +/** + * #4662: returns true while the workspace's chat replay is pending, so the caller skips its + * git/PR probes for now. Each executeBash spawn blocks the Electron main process ~8-16 ms and + * the results render during the transcript paint; the cached status stays visible meanwhile, + * and deferring costs roughly the replay duration (~0.1-0.2 s on cold open). + * + * Arms at most one retry per workspace in `retries` (workspace ID -> cancel function). The + * retry removes its entry and calls `onSettled()` once the replay is no longer pending. + */ +export function deferWhileChatReplayPending( + gate: ChatReplayGate | null, + retries: Map void>, + workspaceId: string, + onSettled: () => void +): boolean { + if (!gate?.isReplayPending(workspaceId)) { + return false; + } + if (retries.has(workspaceId)) { + return true; + } + + let done = false; + const unsubscribe = gate.subscribeKey(workspaceId, () => { + if (done || gate.isReplayPending(workspaceId)) { + return; + } + done = true; + unsubscribe(); + retries.delete(workspaceId); + onSettled(); + }); + retries.set(workspaceId, () => { + done = true; + unsubscribe(); + }); + return true; +}