From 18d3545bae0d9eefc1651bb7aa28d41bc5e6cd1b Mon Sep 17 00:00:00 2001 From: Jinwoo Hong <73622457+Jinwoo-H@users.noreply.github.com> Date: Wed, 17 Jun 2026 13:58:24 -0700 Subject: [PATCH] fix: prevent remote port scanner rescan loops (#5632) Co-authored-by: Orca --- .../ports/WorkspacePortScanner.test.tsx | 167 ++++++++++++++++++ .../components/ports/WorkspacePortScanner.tsx | 155 +++++++++------- 2 files changed, 258 insertions(+), 64 deletions(-) create mode 100644 src/renderer/src/components/ports/WorkspacePortScanner.test.tsx diff --git a/src/renderer/src/components/ports/WorkspacePortScanner.test.tsx b/src/renderer/src/components/ports/WorkspacePortScanner.test.tsx new file mode 100644 index 00000000000..fa72745c3c3 --- /dev/null +++ b/src/renderer/src/components/ports/WorkspacePortScanner.test.tsx @@ -0,0 +1,167 @@ +// @vitest-environment happy-dom + +import { act } from 'react' +import { createRoot, type Root } from 'react-dom/client' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { getDefaultSettings } from '../../../../shared/constants' +import { toRuntimeExecutionHostId } from '../../../../shared/execution-host' +import { + MIN_COMPATIBLE_RUNTIME_CLIENT_VERSION, + RUNTIME_PROTOCOL_VERSION +} from '../../../../shared/protocol-version' +import type { WorkspacePortScanResult } from '../../../../shared/workspace-ports' +import { + clearRuntimeCompatibilityCache, + markRuntimeEnvironmentCompatible +} from '@/runtime/runtime-rpc-client' +import { useAppStore } from '@/store' +import { WorkspacePortScanner } from './WorkspacePortScanner' + +const localScan = vi.fn() +const runtimeEnvironmentCall = vi.fn() +let container: HTMLDivElement | null = null +let root: Root | null = null + +const emptyScan: WorkspacePortScanResult = { + platform: 'darwin', + scannedAt: 1, + ports: [] +} + +const compatibleStatus = { + runtimeId: 'env-1', + graphStatus: 'ready', + runtimeProtocolVersion: RUNTIME_PROTOCOL_VERSION, + minCompatibleRuntimeClientVersion: MIN_COMPATIBLE_RUNTIME_CLIENT_VERSION +} + +async function flushPromises(): Promise { + await Promise.resolve() + await Promise.resolve() +} + +function seedRemoteWorkspace(): void { + useAppStore.setState({ + settings: { + ...getDefaultSettings('/tmp/orca-workspaces'), + activeRuntimeEnvironmentId: 'env-1' + }, + repos: [ + { + id: 'repo-1', + path: '/remote/repo', + displayName: 'Remote Repo', + connectionId: null, + executionHostId: toRuntimeExecutionHostId('env-1') + } + ] as never, + worktreesByRepo: { + 'repo-1': [ + { + id: 'repo-1::/remote/repo', + repoId: 'repo-1', + path: '/remote/repo', + displayName: 'main' + } + ] + } as never, + workspacePortScan: null, + workspacePortScansByKey: {}, + workspacePortScanRefreshing: false + }) +} + +beforeEach(() => { + vi.useFakeTimers() + vi.setSystemTime(0) + localScan.mockReset() + runtimeEnvironmentCall.mockReset() + localScan.mockResolvedValue(emptyScan) + runtimeEnvironmentCall.mockImplementation(({ method }) => { + if (method === 'status.get') { + return Promise.resolve({ ok: true, result: compatibleStatus }) + } + if (method === 'workspacePorts.scan') { + return Promise.resolve({ ok: true, result: emptyScan }) + } + return Promise.resolve({ ok: false, error: { code: 'method_not_found', message: method } }) + }) + vi.stubGlobal('window', { + ...window, + setTimeout: globalThis.setTimeout, + clearTimeout: globalThis.clearTimeout, + setInterval: globalThis.setInterval, + clearInterval: globalThis.clearInterval, + api: { + workspacePorts: { + scan: localScan, + onAdvertisedUrlChanged: vi.fn(() => vi.fn()) + }, + runtimeEnvironments: { + call: runtimeEnvironmentCall + } + } + }) + clearRuntimeCompatibilityCache() + markRuntimeEnvironmentCompatible('env-1') + seedRemoteWorkspace() + container = document.createElement('div') + document.body.appendChild(container) + root = createRoot(container) +}) + +afterEach(() => { + if (root) { + act(() => root?.unmount()) + } + root = null + container?.remove() + container = null + vi.useRealTimers() + vi.unstubAllGlobals() + clearRuntimeCompatibilityCache() +}) + +describe('WorkspacePortScanner', () => { + it('does not restart remote scans before the background interval when host inputs rerender', async () => { + await act(async () => { + root?.render() + await flushPromises() + }) + + expect(runtimeEnvironmentCall).toHaveBeenCalledTimes(1) + expect(runtimeEnvironmentCall).toHaveBeenLastCalledWith({ + selector: 'env-1', + method: 'workspacePorts.scan', + params: {}, + timeoutMs: 15_000 + }) + const firstPublishedScan = useAppStore.getState().workspacePortScan + expect(firstPublishedScan).not.toBeNull() + + await act(async () => { + useAppStore.setState({ + settings: { + ...getDefaultSettings('/tmp/orca-workspaces'), + activeRuntimeEnvironmentId: 'env-1' + } + }) + await flushPromises() + }) + + expect(runtimeEnvironmentCall).toHaveBeenCalledTimes(1) + expect(useAppStore.getState().workspacePortScan).toBe(firstPublishedScan) + + await act(async () => { + vi.advanceTimersByTime(29_999) + await flushPromises() + }) + expect(runtimeEnvironmentCall).toHaveBeenCalledTimes(1) + + await act(async () => { + vi.advanceTimersByTime(1) + await flushPromises() + }) + expect(runtimeEnvironmentCall).toHaveBeenCalledTimes(2) + }) +}) diff --git a/src/renderer/src/components/ports/WorkspacePortScanner.tsx b/src/renderer/src/components/ports/WorkspacePortScanner.tsx index 490e3a66ff1..be7fd923044 100644 --- a/src/renderer/src/components/ports/WorkspacePortScanner.tsx +++ b/src/renderer/src/components/ports/WorkspacePortScanner.tsx @@ -1,7 +1,7 @@ import { useCallback, useEffect, useMemo, useRef } from 'react' import { useAppStore } from '@/store' import { getHasAnyWorktreesFromState } from '@/store/selectors' -import { getActiveRuntimeTarget } from '@/runtime/runtime-rpc-client' +import { getActiveRuntimeTarget, type RuntimeClientTarget } from '@/runtime/runtime-rpc-client' import { mergeWorkspacePortScans, runtimeTargetForExecutionHostId, @@ -14,6 +14,9 @@ import { buildExecutionHostRegistry } from '../../../../shared/execution-host-re const WORKSPACE_PORT_SCAN_INTERVAL_MS = 30_000 const WORKSPACE_PORT_ADVERTISED_URL_SETTLE_MS = 1_000 +type WorkspacePortScannerRefreshOptions = { + force?: boolean +} function makeUnavailableScan(reason: string): WorkspacePortScanResult { return { @@ -33,6 +36,8 @@ export function WorkspacePortScanner({ enabled = true }: { enabled?: boolean }): const setWorkspacePortScanRefreshing = useAppStore((s) => s.setWorkspacePortScanRefreshing) const inFlightRef = useRef | null>(null) const generationRef = useRef(0) + const lastRefreshStartedAtRef = useRef(Number.NEGATIVE_INFINITY) + const scanTargetsRef = useRef([]) const runtimeTarget = useMemo(() => getActiveRuntimeTarget(settings), [settings]) const scanKey = workspacePortScanKeyForTarget(runtimeTarget) @@ -43,69 +48,91 @@ export function WorkspacePortScanner({ enabled = true }: { enabled?: boolean }): .filter((target): target is NonNullable => target !== null), [repos, settings] ) + const scanTargetsSignature = useMemo( + () => + scanTargets + .map((target) => workspacePortScanKeyForTarget(target)) + .sort() + .join('\n'), + [scanTargets] + ) + scanTargetsRef.current = scanTargets - const refresh = useCallback(() => { - if (!hasWorktrees || scanTargets.length === 0) { - setWorkspacePortScan(null) - setWorkspacePortScanRefreshing(false) - return Promise.resolve() - } - if (inFlightRef.current) { - return inFlightRef.current - } + const refresh = useCallback( + (options: WorkspacePortScannerRefreshOptions = {}) => { + const targets = scanTargetsRef.current + if (!hasWorktrees || targets.length === 0) { + setWorkspacePortScan(null) + setWorkspacePortScanRefreshing(false) + return Promise.resolve() + } + if (inFlightRef.current) { + return inFlightRef.current + } + const now = Date.now() + // Why: host/runtime state can rerender this singleton without changing the + // desired poll cadence; remote scans must not restart on every such pass. + if ( + !options.force && + now - lastRefreshStartedAtRef.current < WORKSPACE_PORT_SCAN_INTERVAL_MS + ) { + return Promise.resolve() + } + lastRefreshStartedAtRef.current = now - const generation = generationRef.current - setWorkspacePortScanRefreshing(true) - const promise = Promise.all( - scanTargets.map(async (target) => { - const key = workspacePortScanKeyForTarget(target) - try { - const result = await scanWorkspacePortsForTarget(target) - return { key, result } - } catch (error) { - const message = error instanceof Error ? error.message : String(error) - return { key, result: makeUnavailableScan(message || 'Workspace port scan failed.') } - } - }) - ) - .then((results) => { - if (generation === generationRef.current) { - const scansByKey = Object.fromEntries(results.map(({ key, result }) => [key, result])) - for (const { key, result } of results) { - setWorkspacePortScanForKey(key, result) + const generation = generationRef.current + setWorkspacePortScanRefreshing(true) + const promise = Promise.all( + targets.map(async (target) => { + const key = workspacePortScanKeyForTarget(target) + try { + const result = await scanWorkspacePortsForTarget(target) + return { key, result } + } catch (error) { + const message = error instanceof Error ? error.message : String(error) + return { key, result: makeUnavailableScan(message || 'Workspace port scan failed.') } } - const activeScan = scansByKey[scanKey] - const merged = mergeWorkspacePortScans(scansByKey) - const projectionKey = - results.length > 1 ? 'all-hosts:all' : activeScan ? scanKey : results[0].key - setWorkspacePortScan( - merged - ? { - key: projectionKey, - result: merged - } - : null - ) - } - }) - .finally(() => { - if (inFlightRef.current === promise) { - inFlightRef.current = null - } - if (generation === generationRef.current) { - setWorkspacePortScanRefreshing(false) - } - }) - inFlightRef.current = promise - return promise - }, [ - hasWorktrees, - scanKey, - scanTargets, - setWorkspacePortScan, - setWorkspacePortScanForKey, - setWorkspacePortScanRefreshing - ]) + }) + ) + .then((results) => { + if (generation === generationRef.current) { + const scansByKey = Object.fromEntries(results.map(({ key, result }) => [key, result])) + for (const { key, result } of results) { + setWorkspacePortScanForKey(key, result) + } + const activeScan = scansByKey[scanKey] + const merged = mergeWorkspacePortScans(scansByKey) + const projectionKey = + results.length > 1 ? 'all-hosts:all' : activeScan ? scanKey : results[0].key + setWorkspacePortScan( + merged + ? { + key: projectionKey, + result: merged + } + : null + ) + } + }) + .finally(() => { + if (inFlightRef.current === promise) { + inFlightRef.current = null + } + if (generation === generationRef.current) { + setWorkspacePortScanRefreshing(false) + } + }) + inFlightRef.current = promise + return promise + }, + [ + hasWorktrees, + scanKey, + setWorkspacePortScan, + setWorkspacePortScanForKey, + setWorkspacePortScanRefreshing + ] + ) useEffect(() => { if (!enabled) { @@ -127,7 +154,7 @@ export function WorkspacePortScanner({ enabled = true }: { enabled?: boolean }): inFlightRef.current = null stopVisibleInterval() } - }, [enabled, refresh, setWorkspacePortScan]) + }, [enabled, refresh, scanTargetsSignature, setWorkspacePortScan]) useEffect(() => { if (!enabled) { @@ -155,7 +182,7 @@ export function WorkspacePortScanner({ enabled = true }: { enabled?: boolean }): if (!isWindowVisible()) { return } - void refresh().finally(() => { + void refresh({ force: true }).finally(() => { if (disposed || sequence !== eventSequence || !isWindowVisible()) { return } @@ -165,7 +192,7 @@ export function WorkspacePortScanner({ enabled = true }: { enabled?: boolean }): if (disposed || sequence !== eventSequence || !isWindowVisible()) { return } - void refresh() + void refresh({ force: true }) }, WORKSPACE_PORT_ADVERTISED_URL_SETTLE_MS) }) })