From aee882325f9782eade524e73b37cf4b4b5edd718 Mon Sep 17 00:00:00 2001 From: Kelvin Amoaba <97001695+AmoabaKelvin@users.noreply.github.com> Date: Thu, 22 Oct 2026 21:39:35 +0000 Subject: [PATCH] fix(native-chat): a missing claude or codex cli no longer stalls the model picker on every open (#32014) --- src/main/claude/claude-model-catalog-probe.ts | 11 +-- src/main/codex/codex-model-catalog-probe.ts | 12 +-- .../agent-model-catalog-discovery.ts | 6 +- ...odel-catalog-independent-discovery.test.ts | 2 - .../agent-model-catalog-scope.test.ts | 76 ++++++++++++++++++- .../agent-model-catalog-scope.ts | 25 ++++-- 6 files changed, 108 insertions(+), 24 deletions(-) diff --git a/src/main/claude/claude-model-catalog-probe.ts b/src/main/claude/claude-model-catalog-probe.ts index 8270ed6559e..2cd3334fb52 100644 --- a/src/main/claude/claude-model-catalog-probe.ts +++ b/src/main/claude/claude-model-catalog-probe.ts @@ -25,6 +25,7 @@ import type { AgentModelCatalogProbe, AgentModelCatalogSuccess } from '../native-chat/agent-model-catalog/agent-model-catalog-store' +import type { AgentModelCatalogLookup } from '../native-chat/agent-model-catalog/agent-model-catalog-discovery' import { AgentModelCatalogProbeError, runAgentModelCatalogListing @@ -103,7 +104,7 @@ export function createClaudeModelCatalogProbe( const resolve = async ( accountHome: Parameters[0], cwd?: string, - savedOnly = false + lookup: AgentModelCatalogLookup = 'recover' ) => { const configuredArgs = claudeStructuredLaunchArgs( cwd ? ((await deps.resolveLaunchArgs?.()) ?? []) : [] @@ -122,12 +123,8 @@ export function createClaudeModelCatalogProbe( const args = argsFor(true) const launchDeps = { ...deps, - ...(savedOnly - ? { - resolveInheritedEnv: deps.resolveSavedEnvironment, - refreshEnvironmentAfterMiss: undefined - } - : {}), + ...(lookup === 'saved' ? { resolveInheritedEnv: deps.resolveSavedEnvironment } : {}), + ...(lookup === 'recover' ? {} : { refreshEnvironmentAfterMiss: undefined }), ...(cwd ? { resolveWorkingDirectory: () => cwd } : {}) } const sources = await resolveClaudeChildEnvSources(launchDeps) diff --git a/src/main/codex/codex-model-catalog-probe.ts b/src/main/codex/codex-model-catalog-probe.ts index 6ecafa33e46..fb8a8f769c5 100644 --- a/src/main/codex/codex-model-catalog-probe.ts +++ b/src/main/codex/codex-model-catalog-probe.ts @@ -16,6 +16,7 @@ import type { AgentModelCatalogProbe, AgentModelCatalogSuccess } from '../native-chat/agent-model-catalog/agent-model-catalog-store' +import type { AgentModelCatalogLookup } from '../native-chat/agent-model-catalog/agent-model-catalog-discovery' export type CodexModelCatalogProbeDeps = CodexStructuredInvocationDeps & { /** Existing captured environment only: saved preference reads never recover a missing CLI. */ @@ -52,17 +53,16 @@ export function createCodexModelCatalogProbe( const resolve = async ( accountHome: Parameters[0], cwd?: string, - savedOnly = false + lookup: AgentModelCatalogLookup = 'recover' ) => { const accountHomePath = requireLegacyAgentSessionAccountHome(accountHome).path const overlay = { ...deps.resolveEnv?.() } + const readEnvironment = + lookup === 'saved' ? deps.resolveSavedEnvironment : deps.resolveEnvironment const { command, environment } = await resolveCodexStructuredInvocation({ ...deps, - resolveEnvironment: async () => ({ - ...(await (savedOnly ? deps.resolveSavedEnvironment?.() : deps.resolveEnvironment?.())), - ...overlay - }), - ...(savedOnly ? { refreshEnvironmentAfterMiss: undefined } : {}), + resolveEnvironment: async () => ({ ...(await readEnvironment?.()), ...overlay }), + ...(lookup === 'recover' ? {} : { refreshEnvironmentAfterMiss: undefined }), ...(cwd ? { resolveWorkingDirectory: () => cwd } : {}) }) const args = [ diff --git a/src/main/native-chat/agent-model-catalog/agent-model-catalog-discovery.ts b/src/main/native-chat/agent-model-catalog/agent-model-catalog-discovery.ts index e75e9db4dcf..8f449059058 100644 --- a/src/main/native-chat/agent-model-catalog/agent-model-catalog-discovery.ts +++ b/src/main/native-chat/agent-model-catalog/agent-model-catalog-discovery.ts @@ -6,6 +6,10 @@ export type AgentModelCatalogResolvedProbe = { run: (signal?: AbortSignal) => Promise } +/** The shell env a resolution reads: the saved capture only, the current snapshot, or (the + * default) that snapshot reread by a fresh shell run when it finds no program. */ +export type AgentModelCatalogLookup = 'saved' | 'snapshot' | 'recover' + export type AgentModelCatalogProbe = (( accountHome: AgentSessionAccountHome, options?: { signal?: AbortSignal; workspacePath?: string; cwd?: string } @@ -14,7 +18,7 @@ export type AgentModelCatalogProbe = (( resolve?: ( home: AgentSessionAccountHome, cwd: string, - savedOnly?: boolean + lookup?: AgentModelCatalogLookup ) => Promise } diff --git a/src/main/native-chat/agent-model-catalog/agent-model-catalog-independent-discovery.test.ts b/src/main/native-chat/agent-model-catalog/agent-model-catalog-independent-discovery.test.ts index e19e757ecda..4deb4ef8343 100644 --- a/src/main/native-chat/agent-model-catalog/agent-model-catalog-independent-discovery.test.ts +++ b/src/main/native-chat/agent-model-catalog/agent-model-catalog-independent-discovery.test.ts @@ -60,7 +60,6 @@ function harness() { store, service, run, - resolve, update: (next: AgentModelCatalogSuccess) => { now += 120001 installed = next @@ -103,7 +102,6 @@ describe('independent installed CLI discovery', () => { h.read('b', { waitForListing: true }) ]) expect(h.run).toHaveBeenCalledTimes(1) - expect(h.resolve.mock.calls.every((call) => call.length === 2)).toBe(true) expect(h.service.peek('a', true)).toBe(h.service.peek('b', true)) expect(await h.read('b')).toMatchObject({ models: [{ id: 'old', isDefault: true, defaultEffort: 'high' }] diff --git a/src/main/native-chat/agent-model-catalog/agent-model-catalog-scope.test.ts b/src/main/native-chat/agent-model-catalog/agent-model-catalog-scope.test.ts index e23312cd68f..c1fa2b990a3 100644 --- a/src/main/native-chat/agent-model-catalog/agent-model-catalog-scope.test.ts +++ b/src/main/native-chat/agent-model-catalog/agent-model-catalog-scope.test.ts @@ -1,10 +1,20 @@ -import { describe, expect, it } from 'vitest' +import { chmodSync, mkdirSync, mkdtempSync, rmSync, writeFileSync } from 'node:fs' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { describe, expect, it, vi } from 'vitest' +import { resolveCliCommand } from '../../../shared/node-cli-command-resolution' import { createClaudeModelCatalogProbe } from '../../claude/claude-model-catalog-probe' import { createCodexModelCatalogProbe } from '../../codex/codex-model-catalog-probe' import { agentModelCatalogFingerprint, agentModelCatalogScopeFingerprint } from './agent-model-catalog-fingerprint' +import { AgentModelCatalogProbeError } from './agent-model-catalog-probe-runner' +import { createAgentModelCatalogService } from './agent-model-catalog-service' +import { + AGENT_MODEL_CATALOG_FAILURE_TTL_MS, + AgentModelCatalogStore +} from './agent-model-catalog-store' describe('installed catalog invocation identity', () => { it.each(['claude', 'codex'] as const)( @@ -62,3 +72,67 @@ describe('installed catalog invocation identity', () => { } ) }) + +describe('a picker read with the CLI missing', () => { + it('waits on at most one shell rerun per hold window, and finds a CLI installed after it', async () => { + const root = mkdtempSync(join(tmpdir(), 'orca-catalog-missing-cli-')) + try { + const name = `claude-missing-${process.pid}` + const profileBin = join(root, 'profile-bin') + mkdirSync(profileBin) + const program = join(profileBin, process.platform === 'win32' ? `${name}.cmd` : name) + writeFileSync(program, '#!/bin/sh\nexit 0\n') + chmodSync(program, 0o755) + let served = '/usr/bin' + const reruns = vi.fn(async (_askedAt: number) => {}) + const runListing = vi.fn(async (launch: { command: string }) => { + if (launch.command !== program) { + throw new AgentModelCatalogProbeError('spawn ENOENT', 'spawn', true) + } + return `${JSON.stringify({ + type: 'control_response', + response: { + subtype: 'success', + request_id: 'orca-model-discovery', + response: { models: [{ value: 'sonnet', displayName: 'Sonnet' }] } + } + })}\n` + }) + let now = 1_000 + const service = createAgentModelCatalogService({ + store: new AgentModelCatalogStore({ now: () => now }), + getRecord: () => undefined, + drivesRecord: () => true, + resolveAccountHome: async () => ({ variable: 'CLAUDE_CONFIG_DIR', path: root }), + probes: { + claude: createClaudeModelCatalogProbe({ + resolveCommand: (options) => resolveCliCommand(name, options), + resolveInheritedEnv: async () => ({ PATH: served, HOME: root }), + refreshEnvironmentAfterMiss: reruns, + resolveAuthPolicy: () => ({ account: 'system' }), + runListing + }) + } + }) + const read = () => + service.read({ agent: 'claude', workspacePath: root, waitForListing: true }) + + expect(await read()).toMatchObject({ unavailable: { reason: 'cliMissing' } }) + for (let i = 0; i < 3; i++) { + expect(await read()).toMatchObject({ unavailable: { reason: 'cliMissing' } }) + } + expect(reruns).toHaveBeenCalledOnce() + + reruns.mockImplementation(async () => { + served = profileBin + }) + now += AGENT_MODEL_CATALOG_FAILURE_TTL_MS + const found = await read() + expect(found).toMatchObject({ models: [{ id: 'sonnet' }] }) + expect(found).not.toHaveProperty('unavailable') + expect(reruns).toHaveBeenCalledTimes(2) + } finally { + rmSync(root, { recursive: true, force: true }) + } + }) +}) diff --git a/src/main/native-chat/agent-model-catalog/agent-model-catalog-scope.ts b/src/main/native-chat/agent-model-catalog/agent-model-catalog-scope.ts index c0f7bfc3003..953e33a0006 100644 --- a/src/main/native-chat/agent-model-catalog/agent-model-catalog-scope.ts +++ b/src/main/native-chat/agent-model-catalog/agent-model-catalog-scope.ts @@ -11,6 +11,7 @@ import type { import { agentModelCatalogScopeFingerprint } from './agent-model-catalog-fingerprint' import { independentAgentModelCatalog, + type AgentModelCatalogLookup, type AgentModelCatalogResolvedProbe } from './agent-model-catalog-discovery' import { sessionWorkspacePath } from './agent-model-catalog-workspace' @@ -37,10 +38,15 @@ export async function resolveAgentModelCatalogScope( } const probe = deps.probes?.[params.agent] if (params.legacy) { - const resolved = params.savedOnly ? null : await probe?.resolveLegacy?.(home).catch(() => null) + const fingerprint = agentModelCatalogScopeFingerprint(base, 'legacy-account') + // Only a listing runs the resolution, and it may wait on a shell rerun. + const resolved = + params.savedOnly || !deps.store.probeDue(fingerprint) + ? null + : await probe?.resolveLegacy?.(home).catch(() => null) return { base, - fingerprint: agentModelCatalogScopeFingerprint(base, 'legacy-account'), + fingerprint, exact: false, ...(resolved ? { resolved } : {}) } @@ -52,16 +58,21 @@ export async function resolveAgentModelCatalogScope( if (!cwd) { return { base, fingerprint: base, exact: false } } - const resolution = params.savedOnly - ? probe?.resolve?.(home, cwd, true) - : probe?.resolve?.(home, cwd) - const resolved = await resolution?.catch(() => null) + const resolve = (lookup: AgentModelCatalogLookup) => + probe?.resolve?.(home, cwd, lookup).catch(() => null) + const scopeKey = (resolved: AgentModelCatalogResolvedProbe | null | undefined) => + agentModelCatalogScopeFingerprint(base, cwd, resolved?.identity) + let resolved = await resolve(params.savedOnly ? 'saved' : 'snapshot') + // Only a read that will list waits on a shell rerun to find a newly installed CLI, a new scope. + if (!params.savedOnly && (!resolved || deps.store.probeDue(scopeKey(resolved)))) { + resolved = await resolve('recover') + } if (probe?.resolve && !resolved) { return null } return { base, - fingerprint: agentModelCatalogScopeFingerprint(base, cwd, resolved?.identity), + fingerprint: scopeKey(resolved), exact: true, cwd, ...(resolved ? { resolved } : {})