fix(preflight): do not pin empty WSL/local agent detection (#8366)#8391
fix(preflight): do not pin empty WSL/local agent detection (#8366)#8391bbingz wants to merge 2 commits into
Conversation
…8366) Empty [] is truthy, so a cold-start soft-fail permanently hid agents from the launcher until a context switch. Match remote/runtime non-sticky empty caching, clear empty detectPromise snapshots, and retry one WSL ETIMEDOUT.
📝 WalkthroughWalkthroughAdds one retry for WSL detection command failures caused by timeouts or killed child processes, while leaving other failures non-retriable. Expands tests for retry counts and returned results. Updates detected-agent state handling so empty detection or refresh results do not remain cached, allowing subsequent detection calls to run again. Adds local hook retry behavior and coverage for empty results, remounts, and later detected agents. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/main/ipc/preflight-wsl-agent-detection.test.ts (1)
99-135: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider adding a retry exhaustion test.
The three new tests cover single-retry success and non-retriable failure, but don't verify behavior when both attempts timeout (retry exhaustion). A test confirming
detectWslCommandsOnPathreturns an emptySetandexecFileAsyncMockis called exactly twice after two consecutiveETIMEDOUTrejections would close the gap.🧪 Suggested test for retry exhaustion
it('returns an empty set when both attempts timeout', async () => { const timeoutError = Object.assign(new Error('Timed out running wsl.exe'), { code: 'ETIMEDOUT' }) execFileAsyncMock.mockRejectedValue(timeoutError) const found = await detectWslCommandsOnPath({ distro: 'Ubuntu' }, ['grok']) expect(found).toEqual(new Set()) expect(execFileAsyncMock).toHaveBeenCalledTimes(2) })
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: d1b358ea-54b0-4880-a68a-aef5c16402c7
📒 Files selected for processing (4)
src/main/ipc/preflight-wsl-agent-detection.test.tssrc/main/ipc/preflight-wsl-agent-detection.tssrc/renderer/src/store/slices/detected-agents.test.tssrc/renderer/src/store/slices/detected-agents.ts
Store non-sticky empty was not enough: useDetectedAgents only called ensureLocal when detectedIds was null, so TabBar/QuickLaunch never re-probed after a WSL cold-start []. Mirror SSH/runtime one-shot empty remount retry for local surfaces (stablyai#8366 / stablyai#8391).
Summary
WSL cold-start / probe failures return an empty agent list, and the renderer treated that empty
[]as a successful cache hit (if (existing)is true for arrays). The launcher then permanently hid installed agents (including Grok) until a context switch. This also adds one cold-start retry for WSLETIMEDOUT/ killed probes, and wires the local launch surfaces to re-probe once on remount after an empty result (same contract as SSH/runtime).Closes #8366
Root cause
ensureDetectedAgentsshort-circuited on any non-nulldetectedAgentIds, including[]. Remote/runtime already usedexisting?.length(Detect agents in the serve user's package-manager bins; don't pin empty remote-detect results #6029); local did not.detectPromisefor the same context key could still short-circuit.detectWslCommandsOnPathswallowed timeouts intonew Set()with no retry, feeding the sticky empty cache.useDetectedAgentsonly calledensureLocalwhendetectedIds === null, so TabBar/QuickLaunch never re-entered after[]. SSH/runtime already had one empty remount retry; local did not.Fix
existing?.length && detectedContextKey === contextKey(mirror remote/runtime).detectPromisewhen the detection/refresh result is empty so the next ensure re-probes.ETIMEDOUT/killedonly; hard shell failures still fail immediately to empty.useDetectedAgentslocal path: one empty remount retry per mounted surface (emptyRetryKey = 'local'), same shape as SSH/runtime.Merge order
Land Grok-compat fixes in this order (independent of this PR's file set unless noted):
/bin/sh '<path>', closes [Bug] Codex hooks failing when launched via Orca (from Grok) #8110)auth.x.aientry) — can land in any order relative to this PRThis PR does not depend on #8386 / #8336 / #8390 and may land as soon as it is green. Prefer landing after #8390 only for review sequencing of the Grok-compat batch, not for correctness.
Test plan
pnpm exec vitest run --config config/vitest.config.ts src/renderer/src/store/slices/detected-agents.test.ts src/main/ipc/preflight-wsl-agent-detection.test.ts src/renderer/src/hooks/useDetectedAgents.test.tsx