diff --git a/src/main/runtime/orca-runtime-test-lifecycle.spec.ts b/src/main/runtime/orca-runtime-test-lifecycle.spec.ts index 0aef43a1693..e815599728b 100644 --- a/src/main/runtime/orca-runtime-test-lifecycle.spec.ts +++ b/src/main/runtime/orca-runtime-test-lifecycle.spec.ts @@ -7,7 +7,8 @@ const { addGitHubPRReviewCommentReplyMock, addGitLabIssueCommentMock, addGitLabM const { addGitLabMRInlineCommentMock, addSparseWorktree, addWorktree, advertisedUrlWatcher } = mocks const { afterEach, applyAgentStatusHooksEnabledMock, assertWorktreeCleanForRemoval, beforeEach } = mocks -const { clearConfiguredWorktreeSharedDirectoriesCacheForTests, closeGitLabMRMock } = mocks +const { cancelLegacyWorkerTerminalRecoveryRetriesForTests, closeGitLabMRMock } = mocks +const { clearConfiguredWorktreeSharedDirectoriesCacheForTests } = mocks const { closeLocalWatcherForWorktreePathMock, closeRemoteWatcherForWorktreePathMock } = mocks const { computeWorktreePathMock, countGitHubWorkItemsMock, createGitHubIssueMock } = mocks const { createGitLabIssueMock, createHostedReviewMock, createSetupRunnerScript } = mocks @@ -82,6 +83,9 @@ function resetRuntimeTestMocks(): void { getPath: () => electronMocks.app.getPath(), isPackaged: () => electronMocks.app.isPackaged }) + // Why: a worker-recovery retry re-arms itself for as long as a deferred worker exists, so one + // left armed keeps rescanning worktrees through the shared git stubs for the rest of the run. + cancelLegacyWorkerTerminalRecoveryRetriesForTests() clearConfiguredWorktreeSharedDirectoriesCacheForTests() _resetTerminalViewAttributesForTest() advertisedUrlWatcher.clear() diff --git a/src/main/runtime/orca-runtime-test-mocks.spec.ts b/src/main/runtime/orca-runtime-test-mocks.spec.ts index 1203f069a68..b7acdff0d65 100644 --- a/src/main/runtime/orca-runtime-test-mocks.spec.ts +++ b/src/main/runtime/orca-runtime-test-mocks.spec.ts @@ -61,6 +61,8 @@ export const beforeEach = importedValues.exportedBeforeEach export const beginWatcherInstall = importedValues.exportedBeginWatcherInstall export const buildAgentPromptPasteBytes = importedValues.exportedBuildAgentPromptPasteBytes export const buildPreview = importedValues.exportedBuildPreview +export const cancelLegacyWorkerTerminalRecoveryRetriesForTests = + importedValues.exportedCancelLegacyWorkerTerminalRecoveryRetriesForTests export const clearConfiguredWorktreeSharedDirectoriesCacheForTests = importedValues.exportedClearConfiguredWorktreeSharedDirectoriesCacheForTests export const clearSubmodulePathsCacheForTests = diff --git a/src/main/runtime/orca-runtime-test-mocks/imported-values.spec.ts b/src/main/runtime/orca-runtime-test-mocks/imported-values.spec.ts index d0a0bae397f..218798e88bf 100644 --- a/src/main/runtime/orca-runtime-test-mocks/imported-values.spec.ts +++ b/src/main/runtime/orca-runtime-test-mocks/imported-values.spec.ts @@ -7,6 +7,7 @@ import { setRuntimeBrowserCommandsFactory, setRuntimeBrowserUnavailableCause } from '../runtime-browser-commands-factory' +import { __cancelLegacyWorkerTerminalRecoveryRetriesForTests } from '../runtime-legacy-worker-terminal-recovery-controller' import { setRuntimeTerminalUnavailableCause } from '../native-terminal-availability' import { setRuntimeDesktopSurface } from '../runtime-desktop-surface' import { installFakeAppEnvironment } from '../../../../config/scripts/vitest-host-ports-setup' @@ -178,6 +179,8 @@ export const exportedBeforeEach = beforeEach export const exportedBeginWatcherInstall = beginWatcherInstall export const exportedBuildAgentPromptPasteBytes = buildAgentPromptPasteBytes export const exportedBuildPreview = buildPreview +export const exportedCancelLegacyWorkerTerminalRecoveryRetriesForTests = + __cancelLegacyWorkerTerminalRecoveryRetriesForTests export const exportedClearConfiguredWorktreeSharedDirectoriesCacheForTests = clearConfiguredWorktreeSharedDirectoriesCacheForTests export const exportedClearSubmodulePathsCacheForTests = clearSubmodulePathsCacheForTests diff --git a/src/main/runtime/runtime-legacy-worker-terminal-recovery-controller.ts b/src/main/runtime/runtime-legacy-worker-terminal-recovery-controller.ts index ce034d8e349..59e29a1858d 100644 --- a/src/main/runtime/runtime-legacy-worker-terminal-recovery-controller.ts +++ b/src/main/runtime/runtime-legacy-worker-terminal-recovery-controller.ts @@ -14,6 +14,19 @@ type RecoveryRetry = { timer: ReturnType | null } +// Why a module-level set: a retry re-arms itself until its deferred worker materializes, so a host +// that never resolves one keeps a recovery loop running with no handle on it. A controller joins +// only while it has a timer armed and leaves as soon as it has none, so nothing is retained past +// the loop it belongs to. +const controllersWithArmedRetries = new Set() + +/** Stop every armed recovery retry. Test-only: a retry loop must not outlive the test that armed it. */ +export function __cancelLegacyWorkerTerminalRecoveryRetriesForTests(): void { + for (const controller of Array.from(controllersWithArmedRetries)) { + controller.cancelAllRetries() + } +} + export class RuntimeLegacyWorkerTerminalRecoveryController { private queue: Promise = Promise.resolve() private readonly retries = new Map() @@ -48,6 +61,15 @@ export class RuntimeLegacyWorkerTerminalRecoveryController { clearTimeout(retry.timer) } this.retries.delete(scopeKey) + if (this.retries.size === 0) { + controllersWithArmedRetries.delete(this) + } + } + + cancelAllRetries(): void { + for (const scopeKey of Array.from(this.retries.keys())) { + this.cancelScope(scopeKey) + } } updateRetry( @@ -126,5 +148,6 @@ export class RuntimeLegacyWorkerTerminalRecoveryController { }) }, delayMs) retry.timer.unref?.() + controllersWithArmedRetries.add(this) } } diff --git a/src/main/runtime/runtime-legacy-worker-terminal-recovery-retry.test.ts b/src/main/runtime/runtime-legacy-worker-terminal-recovery-retry.test.ts new file mode 100644 index 00000000000..e96d7fe6769 --- /dev/null +++ b/src/main/runtime/runtime-legacy-worker-terminal-recovery-retry.test.ts @@ -0,0 +1,92 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { + RuntimeLegacyWorkerTerminalRecoveryController, + __cancelLegacyWorkerTerminalRecoveryRetriesForTests +} from './runtime-legacy-worker-terminal-recovery-controller' +import type { + LegacyWorkerRecoveryPorts, + LegacyWorkerTerminalRecoveryResult +} from './runtime-legacy-worker-terminal-recovery-types' +import type { LegacyWorkerTerminalRecoveryPlan } from './orchestration/orchestration-legacy-worker-terminal-recovery' + +const DEFERRED_DISPATCH_ID = 'dispatch-1' + +const DEFERRED_PLAN: LegacyWorkerTerminalRecoveryPlan = { + ambiguousDispatchIds: [], + candidates: [ + { + dispatchId: DEFERRED_DISPATCH_ID, + dispatchStatus: 'dispatched', + contractVersion: 1, + taskId: 'task-1', + worktreeId: 'repo-1::/tmp/worktree-a', + terminalHandle: 'handle-1', + paneKey: 'tab-1:pane-1', + tabId: 'tab-1', + leafId: 'pane-1', + processIncarnation: 'pty-1:inc-1', + ptyId: 'pty-1', + incarnationId: 'inc-1' + } + ] +} + +const EMPTY_RESULT: LegacyWorkerTerminalRecoveryResult = { + adoptedDispatchIds: [], + exitedDispatchIds: [], + deferredDispatchIds: [DEFERRED_DISPATCH_ID] +} + +function armedController(): { + controller: RuntimeLegacyWorkerTerminalRecoveryController + reconcile: ReturnType +} { + const reconcile = vi.fn(async () => EMPTY_RESULT) + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: the retry timer only ever reaches `ports.reconcile`; the rest of the port surface is unreachable from `updateRetry`. + const ports = { reconcile } as unknown as LegacyWorkerRecoveryPorts + const controller = new RuntimeLegacyWorkerTerminalRecoveryController(ports) + controller.updateRetry(DEFERRED_PLAN, new Set([DEFERRED_DISPATCH_ID]), {}) + return { controller, reconcile } +} + +describe('RuntimeLegacyWorkerTerminalRecoveryController retry loop', () => { + beforeEach(() => { + vi.useFakeTimers() + }) + + afterEach(() => { + __cancelLegacyWorkerTerminalRecoveryRetriesForTests() + vi.useRealTimers() + }) + + it('keeps retrying recovery while a worker stays deferred', async () => { + const { reconcile } = armedController() + + await vi.advanceTimersByTimeAsync(1_000) + + expect(reconcile).toHaveBeenCalledTimes(1) + }) + + it('stops a controller retry loop once its scopes are cancelled', async () => { + const { controller, reconcile } = armedController() + + controller.cancelAllRetries() + await vi.advanceTimersByTimeAsync(60_000) + + expect(reconcile).not.toHaveBeenCalled() + }) + + it('reaches every armed controller from the test cancel hook', async () => { + // Why the hook exists: the retry re-arms itself for as long as a worker stays deferred, so a + // suite that never resolves one keeps a recovery loop — and the worktree scans it issues — + // running inside whichever later test happens to be executing when the timer fires. + const first = armedController() + const second = armedController() + + __cancelLegacyWorkerTerminalRecoveryRetriesForTests() + await vi.advanceTimersByTimeAsync(60_000) + + expect(first.reconcile).not.toHaveBeenCalled() + expect(second.reconcile).not.toHaveBeenCalled() + }) +})