mirror of
https://github.com/stablyai/orca.git
synced 2026-10-18 16:03:17 +00:00
fix(agents): prevent remote detection refresh races
This commit is contained in:
@@ -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<Root> {
|
||||
|
||||
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 }) => {
|
||||
|
||||
@@ -51,7 +51,7 @@ export function useDetectedAgents(
|
||||
connectionId: AgentDetectionTarget | string | null | undefined = null
|
||||
): UseDetectedAgentsResult {
|
||||
const target = normalizeAgentDetectionTarget(connectionId)
|
||||
const retriedEmptyTargetRef = useRef<string | null>(null)
|
||||
const observedRemoteTargetRef = useRef<string | null>(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 }
|
||||
}
|
||||
|
||||
@@ -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 }) => {
|
||||
|
||||
@@ -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 }
|
||||
}))
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user