From a7d769e13de6a9eb74e6ada4e9218eadfce74307 Mon Sep 17 00:00:00 2001 From: Jinjing <6427696+AmethystLiang@users.noreply.github.com> Date: Sat, 1 Aug 2026 15:45:45 -0700 Subject: [PATCH] Watcher explorer relay regressions (#12012) * fix(watch/relay): bound remote watcher fan-out and read the live relay grace Three P1 fixes from the SSH/remote freeze audit: - Remote watchers now debounce on the same 150/500 window as local ones (finding D), and every teardown path drops the trailing flush timer instead of letting it fire into a dead watch. The deferred send is wrapped so a frame disposed mid-window can't escape as a fatal main-process exception. - File Explorer refreshes are scheduled and concurrency-capped rather than fanned out unbounded over expanded dirs (finding C). Local transports use a zero window, since main already coalesced the burst. - relay.startGrace reads ptyHandler.configuredGraceTimeMs instead of the launch-time argv closure, so a grace raised after launch is honored. The branch selection moves to relay-grace-branch.ts because relay.ts has no exports and calls main() at import, making it untestable. Consequence: a host-sleep relay holding zero PTYs now exits after the idle cap. Pinned by test and documented in docs/reference/relay-grace-time-reconfiguration.md. Also drops the duplicated 150/500/5000 constants in the runtime-RPC batcher in favor of the shared window module. * docs(relay): correct grace-reconfiguration line numbers after the relay.ts edit Co-authored-by: Orca * refactor(file-explorer): use useMemo for paths; remove relay reference Replace manual ref-based caching with proper React hooks for content-stable path memoization. Remove outdated relay grace-time reference documentation from code review cycle. * rm design doc * fix(watcher/explorer/relay): coalesce POSIX paths by byte identity; pres - Remote watcher event coalescing now keeps NFC/NFD-distinct POSIX paths separate while still folding Windows path spellings, fixing cache invalidation when recreating directories with different Unicode compositions. - Relay grace reconfiguration now preserves shutdown-deferred state when grace is set to zero, preventing premature shutdown when the grace timer is reconfigured mid-flight. - File explorer refresh refactored from fixed-wave batching to concurrent task execution with `forEachWithConcurrency`, batching results every N settled reads instead of every wave, and reporting whether cancellation discarded pending work. - Watch handler now resyncs when events arrive after disposal, ensuring refresh requests aren't lost when events race cleanup during worktree switches. * fix(watcher/explorer): resilient commit batches on callback error Cache writes precede callbacks, so a throwing callback cannot strand the batch with stale marks. All callbacks complete despite errors, with the first error thrown after. - Fix commitBatchSize calculation for empty dirs - Add scheduler discard-report test - Move disposed variable before handleFsChanged - Expand batching-strategy comments * test(activity): stabilize portal readiness latch release under CI load The fixed 9-flip budget could miss enough MutationObserver deliveries under shard load for sibling DOM to still report loading. Drain readiness rAF after each flip and wait until latched unavailable is actually observed. * fix(watcher/explorer): stop batches on commit error; verify drain settle Enhance resilience under CI load and error conditions: - Portal readiness drain now returns a boolean confirming settlement; test expects verify the drain completed before proceeding. - File explorer commit batches stop executing after a callback throws, preventing stale writes after caller-observed rejection. - Watch hook requires worktree ID upfront in effect guard, eliminating redundant checks inside loops. * fix(relay): re-arm grace on configured change Refactor grace reconfiguration into a dedicated decision function. startGrace samples the configured grace at arm time, so a raised value landing mid-window would still fire at the old deadline without re-arming. Extract the logic with proper tests to ensure shutdown-deferred state is preserved across re-arms. * refactor(relay): extract grace reconfiguration to testable function Extract `applyRelayGraceTimeConfiguration` so the grace-time re-arming logic can be tested independently. relay.ts runs `main()` on import and exports nothing, making the call site (including retryDeferredShutdown hand-off into startGrace) otherwise untestable. --------- Co-authored-by: Orca --- src/relay/relay-grace-branch.test.ts | 169 ++++++++++++++++-- src/relay/relay-grace-branch.ts | 78 +++++++- src/relay/relay.ts | 31 +--- ...ty-portal-readiness-loop.react185.test.tsx | 59 +++++- ...ile-explorer-expanded-dirs-refresh.test.ts | 40 +++++ .../file-explorer-expanded-dirs-refresh.ts | 10 ++ .../right-sidebar/useFileExplorerWatch.ts | 7 +- 7 files changed, 344 insertions(+), 50 deletions(-) diff --git a/src/relay/relay-grace-branch.test.ts b/src/relay/relay-grace-branch.test.ts index 580ab7e17..e7fee5832 100644 --- a/src/relay/relay-grace-branch.test.ts +++ b/src/relay/relay-grace-branch.test.ts @@ -1,8 +1,11 @@ -import { describe, expect, it } from 'vitest' +import { describe, expect, it, vi } from 'vitest' import { + applyRelayGraceTimeConfiguration, decideRelayGrace, - retryDeferredShutdownAfterGraceReconfigure, - type RelayGraceDecisionInput + decideRelayGraceReconfigure, + type RelayGraceBranch, + type RelayGraceDecisionInput, + type RelayGraceReconfigureInput } from './relay-grace-branch' const EMPTY_DETACHED_STARTUP_GRACE_MS = 30_000 @@ -68,19 +71,6 @@ describe('decideRelayGrace', () => { ).toEqual({ branch: 'shutdown-deferred', timeoutMs: IDLE_RELAY_GRACE_MS }) }) - it('preserves shutdown-deferred when grace is reconfigured to zero', () => { - // Why a non-zero starting grace: configureRelayGraceTime only re-arms on an actual change, - // so the reconfiguration this models must cross a real boundary, not 0 → 0. - const current = decide({ configuredGraceMs: 10_000, retryDeferredShutdown: true }) - - expect( - decide({ - configuredGraceMs: 0, - retryDeferredShutdown: retryDeferredShutdownAfterGraceReconfigure(current.branch) - }) - ).toEqual({ branch: 'shutdown-deferred', timeoutMs: IDLE_RELAY_GRACE_MS }) - }) - it('prefers startup-empty-detached over the idle cap', () => { expect( decide({ @@ -115,3 +105,150 @@ describe('decideRelayGrace', () => { }) }) }) + +function reconfigure(overrides: Partial = {}) { + return decideRelayGraceReconfigure({ + previousConfiguredGraceMs: 10_000, + nextConfiguredGraceMs: 86_400_000, + graceTimerArmed: true, + shutdownInFlight: false, + currentBranch: 'configured', + ...overrides + }) +} + +describe('decideRelayGraceReconfigure', () => { + it('re-arms a running window so a raised grace takes effect at the new deadline', () => { + // Why: the reported bug's call site. startGrace samples the grace at arm time, so without this + // re-arm a raise landing mid-window still fires at the old deadline. + expect(reconfigure()).toEqual({ rearm: true, retryDeferredShutdown: false }) + }) + + it('preserves shutdown-deferred across the re-arm', () => { + const rearmed = reconfigure({ nextConfiguredGraceMs: 0, currentBranch: 'shutdown-deferred' }) + + expect(rearmed).toEqual({ rearm: true, retryDeferredShutdown: true }) + expect( + decide({ + configuredGraceMs: 0, + retryDeferredShutdown: rearmed.rearm && rearmed.retryDeferredShutdown + }) + ).toEqual({ branch: 'shutdown-deferred', timeoutMs: IDLE_RELAY_GRACE_MS }) + }) + + it('ignores a re-assertion of the same grace', () => { + // Why: the host re-asserts its grace on every establish; re-arming on those would keep the + // window alive indefinitely. + expect(reconfigure({ previousConfiguredGraceMs: 86_400_000 })).toEqual({ rearm: false }) + }) + + it('does not arm a window that was never running', () => { + expect(reconfigure({ graceTimerArmed: false })).toEqual({ rearm: false }) + }) + + it('does not re-arm once shutdown is in flight', () => { + expect(reconfigure({ shutdownInFlight: true })).toEqual({ rearm: false }) + }) +}) + +/** + * Models relay.ts's grace state around the configureGraceTime call site: `startGrace` re-decides the + * branch exactly as the relay does, so a dropped `retryDeferredShutdown` shows up as a branch downgrade. + */ +function relayGraceHost( + overrides: { + configuredGraceMs?: number + graceTimerArmed?: boolean + shutdownInFlight?: boolean + graceBranch?: RelayGraceBranch | null + relayIdle?: boolean + activePtyCount?: number + } = {} +) { + let configuredGraceMs = overrides.configuredGraceMs ?? 10_000 + let graceBranch: RelayGraceBranch | null = overrides.graceBranch ?? 'configured' + const startGrace = vi.fn( + (_reason: string, options?: { retryDeferredShutdown?: boolean }): void => { + graceBranch = decide({ + configuredGraceMs, + relayIdle: overrides.relayIdle ?? true, + activePtyCount: overrides.activePtyCount ?? 0, + retryDeferredShutdown: options?.retryDeferredShutdown === true + }).branch + } + ) + + return { + startGrace, + branch: () => graceBranch, + configuredGraceMs: () => configuredGraceMs, + apply: (graceTimeSeconds: unknown) => + applyRelayGraceTimeConfiguration(graceTimeSeconds, { + readConfiguredGraceMs: () => configuredGraceMs, + // Mirrors PtyHandler.setGraceTimeMs's clamp so the decision sees the stored value. + writeConfiguredGraceMs: (graceMs) => { + configuredGraceMs = Math.max(0, Math.floor(graceMs)) + }, + isGraceTimerArmed: () => overrides.graceTimerArmed ?? true, + isShutdownInFlight: () => overrides.shutdownInFlight ?? false, + readGraceBranch: () => graceBranch, + startGrace + }) + } +} + +describe('applyRelayGraceTimeConfiguration', () => { + it('re-arms a deferred shutdown without downgrading it to an ordinary configured window', () => { + // Why: the call site the branch selector alone cannot cover — dropping the startGrace option here + // would leave a refused kill with nothing left to retry it. + const host = relayGraceHost({ graceBranch: 'shutdown-deferred', configuredGraceMs: 0 }) + + expect(host.apply(86_400)).toEqual({ graceTimeMs: 86_400_000 }) + expect(host.startGrace).toHaveBeenCalledWith('grace reconfigured', { + retryDeferredShutdown: true + }) + expect(host.branch()).toBe('shutdown-deferred') + }) + + it('re-arms an ordinary window at the raised grace', () => { + const host = relayGraceHost() + + expect(host.apply(86_400)).toEqual({ graceTimeMs: 86_400_000 }) + expect(host.startGrace).toHaveBeenCalledWith('grace reconfigured', { + retryDeferredShutdown: false + }) + expect(host.branch()).toBe('configured') + }) + + it('stores the host-sleep zero and lets the idle cap own the window', () => { + const host = relayGraceHost() + + expect(host.apply(0)).toEqual({ graceTimeMs: 0 }) + expect(host.startGrace).toHaveBeenCalledTimes(1) + expect(host.branch()).toBe('idle-no-ptys') + }) + + it('does not re-arm on a re-asserted grace', () => { + const host = relayGraceHost({ configuredGraceMs: 86_400_000 }) + + expect(host.apply(86_400)).toEqual({ graceTimeMs: 86_400_000 }) + expect(host.startGrace).not.toHaveBeenCalled() + }) + + it('does not arm a window that was never running', () => { + const host = relayGraceHost({ graceTimerArmed: false }) + + expect(host.apply(86_400)).toEqual({ graceTimeMs: 86_400_000 }) + expect(host.startGrace).not.toHaveBeenCalled() + }) + + it('ignores a malformed or negative payload without touching the stored grace', () => { + for (const payload of [undefined, 'later', Number.NaN, -1]) { + const host = relayGraceHost() + + expect(host.apply(payload)).toEqual({ graceTimeMs: 10_000 }) + expect(host.configuredGraceMs()).toBe(10_000) + expect(host.startGrace).not.toHaveBeenCalled() + } + }) +}) diff --git a/src/relay/relay-grace-branch.ts b/src/relay/relay-grace-branch.ts index 52c6935df..68ba3e2ec 100644 --- a/src/relay/relay-grace-branch.ts +++ b/src/relay/relay-grace-branch.ts @@ -23,10 +23,82 @@ export type RelayGraceDecision = { timeoutMs: number } -export function retryDeferredShutdownAfterGraceReconfigure( +export type RelayGraceReconfigureInput = { + previousConfiguredGraceMs: number + nextConfiguredGraceMs: number + /** A grace window with a real deadline is running; an unlimited (deadline-less) one is not re-armed. */ + graceTimerArmed: boolean + shutdownInFlight: boolean currentBranch: RelayGraceBranch | null -): boolean { - return currentBranch === 'shutdown-deferred' +} + +export type RelayGraceReconfigureDecision = + | { rearm: false } + | { rearm: true; retryDeferredShutdown: boolean } + +/** Live relay grace state, read through accessors so the caller's `let` bindings stay authoritative. */ +export type RelayGraceTimeConfigurationInput = { + readConfiguredGraceMs: () => number + /** Writes the new grace; the value is read back so a normalizing setter wins. */ + writeConfiguredGraceMs: (graceMs: number) => void + isGraceTimerArmed: () => boolean + isShutdownInFlight: () => boolean + readGraceBranch: () => RelayGraceBranch | null + startGrace: (reason: string, options?: { retryDeferredShutdown?: boolean }) => void +} + +/** + * Applies a `relay.configureGraceTime` payload, re-arming the running window when the value changed. + * + * Why extracted from relay.ts: that file runs `main()` on import and exports nothing, so the call site + * — including the `retryDeferredShutdown` hand-off into `startGrace` — is otherwise untestable. + */ +export function applyRelayGraceTimeConfiguration( + graceTimeSeconds: unknown, + state: RelayGraceTimeConfigurationInput +): { graceTimeMs: number } { + const seconds = Number(graceTimeSeconds) + if (Number.isFinite(seconds) && seconds >= 0) { + const previousConfiguredGraceMs = state.readConfiguredGraceMs() + // Why: the host sends 0 before system sleep so live remote PTYs survive longer than the ordinary grace window. + state.writeConfiguredGraceMs(Math.floor(seconds) * 1000) + const reconfigure = decideRelayGraceReconfigure({ + previousConfiguredGraceMs, + nextConfiguredGraceMs: state.readConfiguredGraceMs(), + graceTimerArmed: state.isGraceTimerArmed(), + shutdownInFlight: state.isShutdownInFlight(), + currentBranch: state.readGraceBranch() + }) + if (reconfigure.rearm) { + state.startGrace('grace reconfigured', { + retryDeferredShutdown: reconfigure.retryDeferredShutdown + }) + } + } + return { graceTimeMs: state.readConfiguredGraceMs() } +} + +/** + * Decides whether a live grace change must re-arm the running grace timer. + * + * Why a decision rather than inline gating: `startGrace` samples the configured grace at arm time, so + * this gate is the only thing that makes a post-launch raise take effect on an already-running window. + */ +export function decideRelayGraceReconfigure( + input: RelayGraceReconfigureInput +): RelayGraceReconfigureDecision { + // Why only on an actual change: the host re-asserts the same value on every establish, and + // restarting the window on those would keep a grace alive indefinitely. + if ( + input.nextConfiguredGraceMs === input.previousConfiguredGraceMs || + !input.graceTimerArmed || + input.shutdownInFlight + ) { + return { rearm: false } + } + // Why carry the branch forward: re-arming without it downgrades a deferred shutdown to an ordinary + // configured window, and the refused kill is never retried. + return { rearm: true, retryDeferredShutdown: input.currentBranch === 'shutdown-deferred' } } /** diff --git a/src/relay/relay.ts b/src/relay/relay.ts index a0228e0cc..4bfc8bf3e 100644 --- a/src/relay/relay.ts +++ b/src/relay/relay.ts @@ -52,8 +52,8 @@ import { import { resolveSetupAgentSequenceLaunchCommand } from '../shared/setup-agent-sequencing' import { pickRemoteCliEnv } from './remote-cli-env' import { + applyRelayGraceTimeConfiguration, decideRelayGrace, - retryDeferredShutdownAfterGraceReconfigure, type RelayGraceBranch } from './relay-grace-branch' import { relayLogLine } from './relay-diagnostic-log' @@ -685,27 +685,14 @@ async function main(): Promise { }) function configureRelayGraceTime(params: Record): { graceTimeMs: number } { - const seconds = Number(params.graceTimeSeconds) - if (Number.isFinite(seconds) && seconds >= 0) { - const previousGraceMs = ptyHandler.configuredGraceTimeMs - // Why: the host sends 0 before system sleep so live remote PTYs survive longer than the ordinary grace window. - ptyHandler.setGraceTimeMs(Math.floor(seconds) * 1000) - // Why: startGrace samples the configured value at arm time, so a raise that lands while the idle - // timer is already running would still fire at the old deadline. Re-arm only on an actual change - // — the host re-asserts the same value on every establish, and restarting the window on those - // would keep a grace alive indefinitely. - if ( - ptyHandler.configuredGraceTimeMs !== previousGraceMs && - graceDeadlineAt !== null && - graceReason !== null && - !shutdownInFlight - ) { - startGrace('grace reconfigured', { - retryDeferredShutdown: retryDeferredShutdownAfterGraceReconfigure(graceBranch) - }) - } - } - return { graceTimeMs: ptyHandler.configuredGraceTimeMs } + return applyRelayGraceTimeConfiguration(params.graceTimeSeconds, { + readConfiguredGraceMs: () => ptyHandler.configuredGraceTimeMs, + writeConfiguredGraceMs: (graceMs) => ptyHandler.setGraceTimeMs(graceMs), + isGraceTimerArmed: () => graceDeadlineAt !== null && graceReason !== null, + isShutdownInFlight: () => shutdownInFlight, + readGraceBranch: () => graceBranch, + startGrace + }) } dispatcher.onNotification(SSH_RELAY_CONFIGURE_GRACE_TIME_METHOD, (params) => { diff --git a/src/renderer/src/components/activity/activity-portal-readiness-loop.react185.test.tsx b/src/renderer/src/components/activity/activity-portal-readiness-loop.react185.test.tsx index 50190295d..10d9fc2af 100644 --- a/src/renderer/src/components/activity/activity-portal-readiness-loop.react185.test.tsx +++ b/src/renderer/src/components/activity/activity-portal-readiness-loop.react185.test.tsx @@ -15,7 +15,10 @@ import { resolveActivityPortalSwap, type ActivityPortalThreadRef } from './activity-portal-thread-reconciliation' -import type { ActivityPortalReadinessStatus } from './activity-portal-readiness-oscillation' +import { + ACTIVITY_PORTAL_READINESS_MAX_FLIPS, + type ActivityPortalReadinessStatus +} from './activity-portal-readiness-oscillation' const WORKTREE_ID = 'wt-1' const TAB_ID = 'tab-react185' @@ -85,6 +88,28 @@ async function flushPortalFramesUntil( } } +// Drain MutationObserver microtasks and the readiness rAF they schedule. Reports whether the +// drain settled so a caller never reads a transition whose readiness callbacks are still queued. +async function flushPortalReadiness( + frames: ReturnType +): Promise { + for (let attempt = 0; attempt < 8; attempt += 1) { + await act(async () => { + await Promise.resolve() + }) + if (frames.pending() === 0) { + await act(async () => { + await Promise.resolve() + }) + if (frames.pending() === 0) { + return true + } + } + await frames.flush() + } + return frames.pending() === 0 +} + // Models the tab-root DOM and sibling hiding emitted by a portaled TerminalPane. function renderPortaledTerminalPane(target: HTMLElement, tabId: string, leafIds: string[]): void { const isolatedLeafId = leafIds[0] @@ -354,23 +379,47 @@ describe('Activity portal pane switching', () => { await act(async () => { root.render() }) + expect(await flushPortalReadiness(frames)).toBe(true) await flushPortalFramesUntil(frames, () => statuses.at(-1) === 'unavailable') expect(statuses.at(-1)).toBe('unavailable') - // Feed each observed DOM state separately so MutationObserver cannot coalesce the flips. - for (let flip = 0; flip < 9; flip += 1) { + // Feed each DOM state separately and keep going until sibling DOM reports latched + // unavailable. A fixed 9-flip budget flakes when CI load drops MutationObserver + // deliveries below ACTIVITY_PORTAL_READINESS_MAX_FLIPS transitions. + let sawSiblingLoading = false + let sawLatchedSibling = false + for ( + let flip = 0; + flip < ACTIVITY_PORTAL_READINESS_MAX_FLIPS * 4 && !sawLatchedSibling; + flip += 1 + ) { + const mode = flip % 2 === 0 ? 'sibling' : 'hidden' + const statusesBefore = statuses.length await act(async () => { - buildRoot(flip % 2 === 0 ? 'sibling' : 'hidden') + buildRoot(mode) await Promise.resolve() }) - await frames.flush() + expect(await flushPortalReadiness(frames)).toBe(true) + if (mode !== 'sibling') { + continue + } + // Transition-local evidence: an unlatched subscription answers sibling DOM with 'loading', + // so the latch is only proven once a sibling transition that previously emitted 'loading' + // stops doing so and leaves 'unavailable' standing. + if (statuses.slice(statusesBefore).includes('loading')) { + sawSiblingLoading = true + } else if (sawSiblingLoading && statuses.at(-1) === 'unavailable') { + sawLatchedSibling = true + } } + expect(sawLatchedSibling).toBe(true) expect(statuses.at(-1)).toBe('unavailable') await act(async () => { buildRoot('ready') await Promise.resolve() }) + expect(await flushPortalReadiness(frames)).toBe(true) await flushPortalFramesUntil(frames, () => statuses.at(-1) === 'ready') expect(statuses.at(-1)).toBe('ready') }) diff --git a/src/renderer/src/components/right-sidebar/file-explorer-expanded-dirs-refresh.test.ts b/src/renderer/src/components/right-sidebar/file-explorer-expanded-dirs-refresh.test.ts index f1faa368e..b3fdc5835 100644 --- a/src/renderer/src/components/right-sidebar/file-explorer-expanded-dirs-refresh.test.ts +++ b/src/renderer/src/components/right-sidebar/file-explorer-expanded-dirs-refresh.test.ts @@ -413,6 +413,46 @@ describe('refreshFileExplorerExpandedDirs', () => { expect(cache['/repo/b']).toMatchObject({ loading: false, children: [{ name: 'index.ts' }] }) }) + it('stops later batches after a commit callback throws', async () => { + let cache: Record = {} + const setDirCache = vi.fn((update: CacheUpdate) => { + cache = typeof update === 'function' ? update(cache) : update + }) + const commitError = new Error('commit failed') + const onDirCommitted = vi.fn((dirPath: string) => { + if (dirPath === '/repo/a') { + throw commitError + } + }) + + await expect( + refreshFileExplorerExpandedDirs({ + dirs: ['a', 'b', 'c', 'd'].map((name) => ({ dirPath: `/repo/${name}`, depth: 0 })), + worktreePath: '/repo', + dirLoadTracker: createFileExplorerDirLoadTracker(), + setDirCache, + readDirectory: async () => ({ + entries: [entry('index.ts')], + operationOwner: { kind: 'local' as const } + }), + // Two dirs per commit batch, so /repo/c and /repo/d belong to a later batch. + maxConcurrentReads: 2, + onDirCommitted + }) + ).rejects.toBe(commitError) + + // A surviving worker must not commit past the failure: callers observing the rejection + // would otherwise still get setDirCache writes and staleness clears afterwards. + expect(onDirCommitted.mock.calls.map(([dirPath]) => dirPath).sort()).toEqual([ + '/repo/a', + '/repo/b' + ]) + // One up-front loading write plus the single failed batch's result write. + expect(setDirCache).toHaveBeenCalledTimes(2) + expect(cache['/repo/c']).toMatchObject({ loading: true }) + expect(cache['/repo/d']).toMatchObject({ loading: true }) + }) + it('drops a queued directory superseded while an earlier read is blocked', async () => { const tracker = createFileExplorerDirLoadTracker() let cache: Record = {} diff --git a/src/renderer/src/components/right-sidebar/file-explorer-expanded-dirs-refresh.ts b/src/renderer/src/components/right-sidebar/file-explorer-expanded-dirs-refresh.ts index 06c5347b4..64dc0d587 100644 --- a/src/renderer/src/components/right-sidebar/file-explorer-expanded-dirs-refresh.ts +++ b/src/renderer/src/components/right-sidebar/file-explorer-expanded-dirs-refresh.ts @@ -51,6 +51,9 @@ export async function refreshFileExplorerExpandedDirs({ const pendingResults: { dirPath: string; cache: DirCache }[] = [] let settledSinceCommit = 0 let committedDirs = 0 + // Why: forEachWithConcurrency has no cancel hook, so a failed commit must stop the surviving + // workers itself — otherwise a later batch commits after the caller already saw this reject. + let stopped = false // Why: mark every dir loading up front — FileExplorer's auto-load // effect re-runs on any `expanded` change and fans out an unbounded loadDir per @@ -67,6 +70,9 @@ export async function refreshFileExplorerExpandedDirs({ }) const commitPendingResults = (): void => { + if (stopped) { + return + } settledSinceCommit = 0 const currentResults = pendingResults .splice(0) @@ -98,6 +104,7 @@ export async function refreshFileExplorerExpandedDirs({ } } if (commitFailed) { + stopped = true throw firstCommitError } } @@ -113,6 +120,9 @@ export async function refreshFileExplorerExpandedDirs({ } await forEachWithConcurrency(uniqueDirs, maxConcurrentReads, async ({ dirPath, depth }) => { + if (stopped) { + return + } const loadToken = loadTokens.get(dirPath)! // A superseding load owns this dir now; do not spend a round trip on a result we must drop. if (!dirLoadTracker.isCurrent(loadToken)) { diff --git a/src/renderer/src/components/right-sidebar/useFileExplorerWatch.ts b/src/renderer/src/components/right-sidebar/useFileExplorerWatch.ts index 0d9c42af6..d8b218b64 100644 --- a/src/renderer/src/components/right-sidebar/useFileExplorerWatch.ts +++ b/src/renderer/src/components/right-sidebar/useFileExplorerWatch.ts @@ -128,7 +128,9 @@ export function useFileExplorerWatch({ // Why: one atomic effect avoids a cleanup-ordering race that drops events on rapid worktree switches (review issue §3). useEffect(() => { - if (!worktreePath || activeRuntimeEnvironmentId === undefined) { + // Why require a worktree id: it is half of every watch key, and reconciliation already needs it — + // a null-keyed subscription would only park resync state no correctly-keyed one can consume. + if (!worktreePath || !activeWorktreeId || activeRuntimeEnvironmentId === undefined) { return } @@ -172,9 +174,6 @@ export function useFileExplorerWatch({ } function processPayload(payload: FsChangedPayload): void { - if (!currentWorktreeId) { - return - } processFileExplorerFsPayload({ payload, currentWorktreePath,