diff --git a/src/renderer/src/hooks/useDetectedAgents.test.tsx b/src/renderer/src/hooks/useDetectedAgents.test.tsx index 7e9febda32a..48b7cf4ce6f 100644 --- a/src/renderer/src/hooks/useDetectedAgents.test.tsx +++ b/src/renderer/src/hooks/useDetectedAgents.test.tsx @@ -4,7 +4,11 @@ import { act, createElement } from 'react' import { createRoot, type Root } from 'react-dom/client' import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import { useAppStore } from '@/store' -import { useDetectedAgents, type AgentDetectionTarget } from './useDetectedAgents' +import { + useDetectedAgents, + type AgentDetectionTarget, + type UseDetectedAgentsResult +} from './useDetectedAgents' import { MIN_COMPATIBLE_RUNTIME_CLIENT_VERSION, RUNTIME_PROTOCOL_VERSION @@ -14,9 +18,10 @@ const detectRemoteAgents = vi.fn() const runtimeEnvironmentCall = vi.fn() const initialAppState = useAppStore.getInitialState() const roots: Root[] = [] +let latestHookResult: UseDetectedAgentsResult | null = null function HookProbe({ target }: { target: AgentDetectionTarget }): null { - useDetectedAgents(target) + latestHookResult = useDetectedAgents(target) return null } @@ -41,6 +46,7 @@ async function renderProbe(target: AgentDetectionTarget): Promise { beforeEach(() => { useAppStore.setState(initialAppState, true) + latestHookResult = null detectRemoteAgents.mockReset().mockResolvedValue([]) runtimeEnvironmentCall.mockReset().mockImplementation(({ method }: { method: string }) => { const result = @@ -116,6 +122,59 @@ describe('useDetectedAgents (ssh call site)', () => { }) describe('useDetectedAgents (runtime call site)', () => { + it('does not re-probe after an explicit refresh finds no agents', async () => { + useAppStore.setState({ + runtimeDetectedAgentIds: { 'env-1': ['claude'] }, + isDetectingRuntimeAgents: { 'env-1': false } + }) + let detectCalls = 0 + let refreshCalls = 0 + runtimeEnvironmentCall.mockImplementation(({ method }: { method: string }) => { + let result: unknown + if (method === 'status.get') { + result = { + runtimeId: 'remote-runtime', + rendererGraphEpoch: 1, + graphStatus: 'ready', + authoritativeWindowId: null, + liveTabCount: 0, + liveLeafCount: 0, + runtimeProtocolVersion: RUNTIME_PROTOCOL_VERSION, + minCompatibleRuntimeClientVersion: MIN_COMPATIBLE_RUNTIME_CLIENT_VERSION + } + } else if (method === 'preflight.refreshAgents') { + refreshCalls += 1 + result = { + agents: [], + addedPathSegments: [], + shellHydrationOk: true, + pathSource: 'shell_hydrate', + pathFailureReason: 'none' + } + } else { + detectCalls += 1 + result = ['claude'] + } + return Promise.resolve({ + id: method, + ok: true, + result, + _meta: { runtimeId: 'remote-runtime' } + }) + }) + + await renderProbe({ kind: 'runtime', environmentId: 'env-1' }) + await renderProbe({ kind: 'runtime', environmentId: 'env-1' }) + await act(async () => { + await latestHookResult?.refresh() + }) + await flushEffects() + + expect(refreshCalls).toBe(1) + expect(detectCalls).toBe(0) + expect(useAppStore.getState().runtimeDetectedAgentIds['env-1']).toEqual([]) + }) + it('retries a cached empty runtime result when the launch surface is reopened', async () => { let detectCalls = 0 runtimeEnvironmentCall.mockImplementation(({ method }: { method: string }) => { diff --git a/src/renderer/src/hooks/useDetectedAgents.ts b/src/renderer/src/hooks/useDetectedAgents.ts index dbd96ab89cf..e4fe7bd0b3a 100644 --- a/src/renderer/src/hooks/useDetectedAgents.ts +++ b/src/renderer/src/hooks/useDetectedAgents.ts @@ -51,7 +51,7 @@ export function useDetectedAgents( connectionId: AgentDetectionTarget | string | null | undefined = null ): UseDetectedAgentsResult { const target = normalizeAgentDetectionTarget(connectionId) - const retriedEmptyTargetRef = useRef(null) + const observedRemoteTargetRef = useRef(null) // Why: undefined means "store not yet hydrated" — we don't know if the // worktree is local or remote yet. This prevents flashing local agents for // remote worktrees during hydration. @@ -63,6 +63,12 @@ export function useDetectedAgents( : target?.kind === 'runtime' ? target.environmentId : null + const remoteTargetKey = + targetKind === 'ssh' && targetId + ? `ssh:${targetId}` + : targetKind === 'runtime' && targetId + ? `runtime:${targetId}` + : null const detectedIds = useAppStore((s) => { if (isUnknown) { @@ -119,30 +125,24 @@ export function useDetectedAgents( if (isUnknown) { return } - const emptyRetryKey = - targetKind === 'ssh' && targetId - ? `ssh:${targetId}` - : targetKind === 'runtime' && targetId - ? `runtime:${targetId}` - : null + const isNewRemoteTarget = + remoteTargetKey !== null && observedRemoteTargetRef.current !== remoteTargetKey + // Why: a later refresh to [] is not a newly opened surface and must not trigger another probe. + observedRemoteTargetRef.current = remoteTargetKey if (targetKind === 'ssh' && targetId) { if (detectedIds === null) { - retriedEmptyTargetRef.current = emptyRetryKey void ensureRemote(targetId) - } else if (detectedIds.length === 0 && retriedEmptyTargetRef.current !== emptyRetryKey) { + } else if (detectedIds.length === 0 && isNewRemoteTarget) { // Why: a newly opened remote launch surface should get one fresh probe // after a prior empty result, but must not spin while the host has no agents. - retriedEmptyTargetRef.current = emptyRetryKey void ensureRemote(targetId) } } else if (targetKind === 'runtime' && targetId) { if (detectedIds === null) { - retriedEmptyTargetRef.current = emptyRetryKey void ensureRuntime(targetId) - } else if (detectedIds.length === 0 && retriedEmptyTargetRef.current !== emptyRetryKey) { + } else if (detectedIds.length === 0 && isNewRemoteTarget) { // Why: remote `orca serve` users can install/fix PATH without reconnecting; // retry once per mounted surface so the menu can pick that up. - retriedEmptyTargetRef.current = emptyRetryKey void ensureRuntime(targetId) } } else { @@ -150,7 +150,16 @@ export function useDetectedAgents( void ensureLocal() } } - }, [isUnknown, targetKind, targetId, detectedIds, ensureLocal, ensureRemote, ensureRuntime]) + }, [ + isUnknown, + targetKind, + targetId, + remoteTargetKey, + detectedIds, + ensureLocal, + ensureRemote, + ensureRuntime + ]) return { detectedIds, isLoading, isRefreshing, refresh } } diff --git a/src/renderer/src/store/slices/detected-agents.test.ts b/src/renderer/src/store/slices/detected-agents.test.ts index 5b6367363b1..1755f355072 100644 --- a/src/renderer/src/store/slices/detected-agents.test.ts +++ b/src/renderer/src/store/slices/detected-agents.test.ts @@ -652,6 +652,89 @@ describe('createDetectedAgentsSlice remote detection', () => { ).toHaveLength(1) }) + it('keeps a late initial detect from overwriting a runtime refresh', async () => { + const store = createTestStore() + let resolveDetect: (value: unknown) => void = () => {} + let resolveRefresh: (value: unknown) => void = () => {} + runtimeEnvironmentCall.mockImplementation(({ method }: { method: string }) => { + if (method === 'status.get') { + return Promise.resolve({ + id: method, + ok: true, + result: { + runtimeId: 'remote-runtime', + rendererGraphEpoch: 1, + graphStatus: 'ready', + authoritativeWindowId: null, + liveTabCount: 0, + liveLeafCount: 0, + runtimeProtocolVersion: RUNTIME_PROTOCOL_VERSION, + minCompatibleRuntimeClientVersion: MIN_COMPATIBLE_RUNTIME_CLIENT_VERSION + }, + _meta: { runtimeId: 'remote-runtime' } + }) + } + return new Promise((resolve) => { + if (method === 'preflight.detectAgents') { + resolveDetect = resolve + } else { + resolveRefresh = resolve + } + }) + }) + + const detect = store.getState().ensureRuntimeDetectedAgents('env-1') + await vi.waitFor(() => { + expect( + runtimeEnvironmentCall.mock.calls.filter( + ([{ method }]) => method === 'preflight.detectAgents' + ) + ).toHaveLength(1) + }) + + const refresh = store.getState().refreshRuntimeDetectedAgents('env-1') + expect(store.getState().ensureRuntimeDetectedAgents('env-1')).toBe(refresh) + expect(store.getState().isDetectingRuntimeAgents['env-1']).toBe(true) + expect(store.getState().isRefreshingRuntimeAgents['env-1']).toBe(true) + await vi.waitFor(() => { + expect( + runtimeEnvironmentCall.mock.calls.filter( + ([{ method }]) => method === 'preflight.refreshAgents' + ) + ).toHaveLength(1) + }) + resolveRefresh({ + id: 'preflight.refreshAgents', + ok: true, + result: { + agents: ['kilo'], + addedPathSegments: [], + shellHydrationOk: true, + pathSource: 'shell_hydrate', + pathFailureReason: 'none' + }, + _meta: { runtimeId: 'remote-runtime' } + }) + await expect(refresh).resolves.toEqual(['kilo']) + + resolveDetect({ + id: 'preflight.detectAgents', + ok: true, + result: ['claude'], + _meta: { runtimeId: 'remote-runtime' } + }) + await expect(detect).resolves.toEqual(['claude']) + + expect(store.getState().runtimeDetectedAgentIds['env-1']).toEqual(['kilo']) + expect(store.getState().isDetectingRuntimeAgents['env-1']).toBe(false) + expect(store.getState().isRefreshingRuntimeAgents['env-1']).toBe(false) + expect( + runtimeEnvironmentCall.mock.calls.filter( + ([{ method }]) => method === 'preflight.refreshAgents' + ) + ).toHaveLength(1) + }) + it('falls back to plain runtime re-detection when the server lacks preflight.refreshAgents', async () => { const store = createTestStore() runtimeEnvironmentCall.mockImplementation(({ method }: { method: string }) => { diff --git a/src/renderer/src/store/slices/runtime-detected-agents.ts b/src/renderer/src/store/slices/runtime-detected-agents.ts index 4a3da863e5b..7da5bef6da9 100644 --- a/src/renderer/src/store/slices/runtime-detected-agents.ts +++ b/src/renderer/src/store/slices/runtime-detected-agents.ts @@ -50,6 +50,10 @@ export const createRuntimeDetectedAgentsSlice: StateCreator< isRefreshingRuntimeAgents: {}, ensureRuntimeDetectedAgents: (environmentId: string) => { + const inflightRefresh = runtimeRefreshPromises.get(environmentId) + if (inflightRefresh) { + return inflightRefresh + } const existing = get().runtimeDetectedAgentIds[environmentId] // Why: an empty result ([]) is truthy, so a prior "no agents found" detection // must not be treated as cached — re-detect so a later install / PATH fix is @@ -113,6 +117,9 @@ export const createRuntimeDetectedAgentsSlice: StateCreator< return inflight } + // Why: a refresh is newer and authoritative; detach an older detect so its + // late result cannot overwrite the freshly hydrated PATH result. + runtimeDetectPromises.delete(environmentId) set((s) => ({ isRefreshingRuntimeAgents: { ...s.isRefreshingRuntimeAgents, [environmentId]: true } })) @@ -140,6 +147,10 @@ export const createRuntimeDetectedAgentsSlice: StateCreator< if (runtimeRefreshPromises.get(environmentId) === pending) { set((s) => ({ runtimeDetectedAgentIds: { ...s.runtimeDetectedAgentIds, [environmentId]: typed }, + isDetectingRuntimeAgents: { + ...s.isDetectingRuntimeAgents, + [environmentId]: false + }, isRefreshingRuntimeAgents: { ...s.isRefreshingRuntimeAgents, [environmentId]: false } })) } @@ -150,6 +161,10 @@ export const createRuntimeDetectedAgentsSlice: StateCreator< // wipe the last known agent list. if (runtimeRefreshPromises.get(environmentId) === pending) { set((s) => ({ + isDetectingRuntimeAgents: { + ...s.isDetectingRuntimeAgents, + [environmentId]: false + }, isRefreshingRuntimeAgents: { ...s.isRefreshingRuntimeAgents, [environmentId]: false } })) }