From 7987186033ea267ce319ad8a7bfa966e4ce9802a Mon Sep 17 00:00:00 2001 From: t Date: Sun, 13 Sep 2026 14:14:51 +0800 Subject: [PATCH 1/7] fix(desktop): bind control socket outside macOS userData MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `sockaddr_un.sun_path` is 104 bytes on macOS (108 on Linux), NUL terminator included, and libuv returns EINVAL rather than truncating a path that does not fit. macOS userData spends 70 of those bytes before the UUID and `.sock` are added — `/Users//Library/Application Support/@ari/desktop/agent-control/` — so `server.listen()` always rejected and every dispatch failed with `listen EINVAL: invalid argument`, leaving macOS users unable to send prompts. Bind POSIX sockets in a short private directory under /tmp instead, keeping the whole path near 60 bytes, and remove it on close. Windows keeps its named pipe. Covers the fix with tests that reconstruct the overflowing path and pin the shipped socket root, and adds macOS to the verify matrix so platform-specific transport failures surface on PRs rather than at release. Co-Authored-By: Claude --- .github/workflows/verify.yml | 10 +++++- apps/desktop/src/main/agent-runtime.test.ts | 32 +++++++++++++++-- apps/desktop/src/main/agent-runtime.ts | 39 ++++++++++++++++++--- 3 files changed, 73 insertions(+), 8 deletions(-) diff --git a/.github/workflows/verify.yml b/.github/workflows/verify.yml index 81a03bc1..06534c94 100644 --- a/.github/workflows/verify.yml +++ b/.github/workflows/verify.yml @@ -20,7 +20,15 @@ concurrency: jobs: verify: - runs-on: ubuntu-latest + # macOS is where it breaks first: `sockaddr_un.sun_path` is 104 bytes there + # against 108 on Linux, and Electron's userData prefix is far longer, so + # platform-specific failures in the control transport only surface here. + # `fail-fast: false` so a macOS failure still reports the Linux result. + strategy: + fail-fast: false + matrix: + os: [ubuntu-latest, macos-latest] + runs-on: ${{ matrix.os }} steps: - uses: actions/checkout@v4 diff --git a/apps/desktop/src/main/agent-runtime.test.ts b/apps/desktop/src/main/agent-runtime.test.ts index 85a27bd0..76fcc0dc 100644 --- a/apps/desktop/src/main/agent-runtime.test.ts +++ b/apps/desktop/src/main/agent-runtime.test.ts @@ -1,21 +1,49 @@ // @vitest-environment node import { execFile, execFileSync } from 'node:child_process' import { promisify } from 'node:util' +import { randomUUID } from 'node:crypto' import { mkdtemp, mkdir, readFile, rm, writeFile } from 'node:fs/promises' import { tmpdir } from 'node:os' import { join, resolve } from 'node:path' -import { expect, it } from 'vitest' +import { describe, expect, it } from 'vitest' import { delegationSettingsSchema } from '@ari/contracts/agent-control' import type { Session } from '@ari/contracts/session' import { SessionStore } from '@ari/engine/session-store' import { DriverRegistry } from '@ari/providers/registry' import type { AdapterSession } from '@ari/providers/driver' import { Engine } from './engine' -import { startAgentRuntime } from './agent-runtime' +import { controlEndpoint, SOCKET_ROOT, startAgentRuntime } from './agent-runtime' const execute = promisify(execFile) const cliPath = resolve('resources/cli/ari.cjs') +/** `sockaddr_un.sun_path` is char[104] on macOS, NUL terminator included. */ +const MACOS_SUN_PATH_BYTES = 104 + +describe('control endpoint', () => { + // The reporter's path: Electron userData plus the runtime directory. + const macUserData = '/Users/jdholst/Library/Application Support/@ari/desktop/agent-control' + + it('overflows sun_path when bound under macOS userData', () => { + expect(Buffer.byteLength(join(macUserData, `${randomUUID()}.sock`))).toBeGreaterThanOrEqual( + MACOS_SUN_PATH_BYTES, + ) + }) + + it('stays within it when bound in a short root', async () => { + const { endpoint } = await controlEndpoint('darwin', tmpdir()) + expect(Buffer.byteLength(endpoint)).toBeLessThan(MACOS_SUN_PATH_BYTES) + }) + + // `controlEndpoint` is exercised above with a caller-supplied root, so this + // pins the root production actually passes. Pointing it back at userData is + // the exact regression that produced the reporter's EINVAL. + it('binds under a shipped root short enough for the macOS budget', () => { + const endpoint = join(SOCKET_ROOT, 'ari-XXXXXX', `${randomUUID()}.sock`) + expect(Buffer.byteLength(endpoint)).toBeLessThan(MACOS_SUN_PATH_BYTES) + }) +}) + it('runs the shipped CLI through scoped transport, a real isolated worker and exact integration', async () => { const dir = await mkdtemp(join(tmpdir(), 'ari-e2e-')) const repo = join(dir, 'repo space') diff --git a/apps/desktop/src/main/agent-runtime.ts b/apps/desktop/src/main/agent-runtime.ts index 41601a56..585c7bf2 100644 --- a/apps/desktop/src/main/agent-runtime.ts +++ b/apps/desktop/src/main/agent-runtime.ts @@ -1,4 +1,4 @@ -import { mkdir, writeFile, chmod } from 'node:fs/promises' +import { mkdir, mkdtemp, rm, writeFile, chmod } from 'node:fs/promises' import { join, delimiter } from 'node:path' import { randomUUID } from 'node:crypto' import { ControlFailure, type DelegationSettings } from '@ari/contracts/agent-control' @@ -32,16 +32,44 @@ export interface AgentRuntimeOptions { baseEnv?: NodeJS.ProcessEnv } +/** + * Unix socket paths live in the kernel's fixed-size `sockaddr_un.sun_path`: + * 104 bytes on macOS, 108 on Linux, NUL terminator included. libuv fails with + * EINVAL rather than truncating a path that does not fit, and `listen()` + * surfaces that as `listen EINVAL: invalid argument` — which rejects the whole + * runtime, so every dispatch fails and the user cannot send anything. + * + * macOS userData cannot fit one. `/Users//Library/Application Support/` + * plus `@ari/desktop/agent-control/` is 70 bytes on a 7-character user name — + * and 104 on no user name at all — before the 36-byte UUID and its `.sock` + * suffix are added. POSIX sockets therefore bind in a short private directory + * under /tmp instead, leaving ~60 bytes total. Only the CLI consumes the + * endpoint, and it reads the path from ARI_CONTROL_ENDPOINT. Windows uses a + * named pipe and has no such limit. + */ +export const SOCKET_ROOT = '/tmp' + +/** + * Creates this runtime's private socket directory and returns the endpoint the + * control server binds, along with the directory to remove on close (`null` on + * Windows, where the endpoint is a named pipe and there is nothing to clean). + */ +export async function controlEndpoint( + platform: NodeJS.Platform, + socketRoot: string, +): Promise<{ endpoint: string; dir: string | null }> { + if (platform === 'win32') return { endpoint: `\\\\.\\pipe\\ari-${randomUUID()}`, dir: null } + const dir = await mkdtemp(join(socketRoot, 'ari-')) + return { endpoint: join(dir, `${randomUUID()}.sock`), dir } +} + /** Hosts the transport and injects private CLI launchers without changing global PATH. */ export async function startAgentRuntime(options: AgentRuntimeOptions) { const { engine, store } = options const runtimeDir = join(options.userData, 'agent-control') const bin = join(runtimeDir, 'bin') await mkdir(bin, { recursive: true, mode: 0o700 }) - const endpoint = - process.platform === 'win32' - ? `\\\\.\\pipe\\ari-${randomUUID()}` - : join(runtimeDir, `${randomUUID()}.sock`) + const { endpoint, dir: socketDir } = await controlEndpoint(process.platform, SOCKET_ROOT) const launcher = join(bin, process.platform === 'win32' ? 'ari.cmd' : 'ari') const quote = (value: string) => `'${value.replaceAll("'", "'\\''")}'` const script = @@ -180,6 +208,7 @@ export async function startAgentRuntime(options: AgentRuntimeOptions) { unsubscribe() approvals.close() await server.close() + if (socketDir) await rm(socketDir, { recursive: true, force: true }) }, environment: (session: Session): NodeJS.ProcessEnv => { const env = { ...(options.baseEnv ?? process.env) } From 7859d1a4a426b72e02f0b5df33c30eefb1a65a3b Mon Sep 17 00:00:00 2001 From: t Date: Sun, 13 Sep 2026 15:16:34 +0800 Subject: [PATCH 2/7] fix(providers): make the model picker reflect what the CLI actually accepts The picker could offer a model the harness would not accept, and the drop happened at `log.debug`: the turn ran on the agent default while the composer kept showing the user's pick. Four defects, all shipping in v0.3.1: - `applyModel` now reports its outcome. When the agent does not offer the requested model the turn runs anyway and says so in the transcript via a new `notice` agent event. `error` was wrong for this: it settles the turn as failed, and a turn that ran on the wrong model still ran. - Closed from the better side too: an ACP session installs the model list its own agent advertises as the kind's live catalog, so the picker offers what the runtime that will actually run accepts. This matters most when the throwaway probe fails (uncached npx package under `--no-install`) while a real turn still works; the snapshot fallback then names ids the agent refuses outright. - `#writeDiskCache` persisted the post-probe merged overlay under the models.dev provider key, so one session's probe output outlived it as vendor registry data. It now writes only what the registry round carried. - Version-less family pointers (`opus`, `sonnet`) ride as aliases on the newest row instead of appearing as rows of their own; duplicate display names collapse; superseded family members become `isLegacy` rather than disappearing. Collapsing happens at read time so every source gets the same treatment and stored catalogs stay as-reported. - `findBinary` used a bare `existsSync`, so a *directory* named `codex` was a provider row. It is now a regular-file check, and the rail gates on installation and auth rather than on a non-null path alone. The picker records each catalog provenance and marks a non-live list as such, with a refresh control: the guarantee is that Ari reflects what the configured CLI exposes, with a clearly marked cached fallback rather than a silent one. Also fixed: Ari's placeholder id `default` was conflated with an agent's genuinely advertised `default` option, making a switch back to the CLI default a silent no-op. And a kind already serving live data is no longer re-probed on every refresh (6h window); a kind still on a fallback keeps retrying on the existing 60s throttle. Co-Authored-By: Claude --- apps/desktop/src/main/engine.test.ts | 51 ++++ apps/desktop/src/main/engine.ts | 10 + .../features/composer/ModelSelector.test.tsx | 213 ++++++++++++++++ .../src/features/composer/ModelSelector.tsx | 168 ++++++++++-- .../composer/provider-readiness.test.ts | 78 ++++++ .../features/composer/provider-readiness.ts | 65 +++++ packages/contracts/src/agent-event.ts | 7 + packages/contracts/src/rpc.ts | 4 + packages/providers/src/acp/acp-driver.test.ts | 240 +++++++++++++++++- packages/providers/src/acp/acp-driver.ts | 157 +++++++++++- packages/providers/src/acp/protocol.ts | 24 +- .../providers/src/catalog-service.test.ts | 117 +++++++-- packages/providers/src/catalog-service.ts | 47 +++- packages/providers/src/catalogs.test.ts | 98 ++++++- packages/providers/src/catalogs.ts | 191 +++++++++++++- packages/providers/src/detector.test.ts | 32 ++- packages/providers/src/detector.ts | 13 +- scripts/update-model-snapshot.ts | 4 + 18 files changed, 1439 insertions(+), 80 deletions(-) create mode 100644 apps/desktop/src/renderer/src/features/composer/provider-readiness.test.ts create mode 100644 apps/desktop/src/renderer/src/features/composer/provider-readiness.ts diff --git a/apps/desktop/src/main/engine.test.ts b/apps/desktop/src/main/engine.test.ts index 9de7bbf5..e8d35cfd 100644 --- a/apps/desktop/src/main/engine.test.ts +++ b/apps/desktop/src/main/engine.test.ts @@ -541,6 +541,57 @@ describe('engine end-to-end with scripted driver', () => { expect(model.status).toBe('error') }, 10000) + it('shows a notice in the transcript without failing the turn', async () => { + // A notice is the "this worked, but not the way you asked" channel — a + // refused model pick. Settling the turn as `error` here would report a + // failure that did not happen and play the error sound over a good reply. + function noticingDriver(): Driver { + function makeAdapter(): ProviderAdapter { + async function* start(): AsyncGenerator { + yield { + type: 'notice', + message: '"gpt-9" is not offered by this agent, so this turn ran on the default.', + } + yield { type: 'text-delta', text: 'hello' } + yield { type: 'done' } + } + return { + start: () => ({ [Symbol.asyncIterator]: () => start()[Symbol.asyncIterator]() }), + interrupt: () => undefined, + dispose: () => Promise.resolve(), + } + } + return { kind: 'claude', create: () => Promise.resolve(makeAdapter()) } + } + + const registry = new DriverRegistry() + registry.register(noticingDriver()) + const engine = new Engine({ + store, + registry, + publish: (sessionId, event) => published.push({ sessionId, event }), + git: { captureCheckpoint: async () => ({ ok: true, value: null }) }, + }) + const sessionId = 'sess_notice' + await seedSession(store, sessionId) + await engine.dispatch({ type: 'turn.start', sessionId, text: 'hi' } as Command) + for (let i = 0; i < 150; i++) { + if (published.some((p) => p.sessionId === sessionId && p.event.type === 'turn.settled')) break + if (i === 149) throw new Error('turn never settled') + await new Promise((r) => setTimeout(r, 20)) + } + + const model = await store.load(sessionId) + expect(model.status).toBe('idle') + const text = (model.messages ?? []) + .flatMap((m) => m.parts) + .filter((p) => p.type === 'text') + .map((p) => (p as { text: string }).text) + .join('') + expect(text).toContain('gpt-9') + expect(text).toContain('hello') + }, 10000) + it('passes the observed provider ref as resumeOf on the next turn only', async () => { const created: AdapterSession[] = [] function resumingDriver(): Driver { diff --git a/apps/desktop/src/main/engine.ts b/apps/desktop/src/main/engine.ts index 17f669f3..973037a3 100644 --- a/apps/desktop/src/main/engine.ts +++ b/apps/desktop/src/main/engine.ts @@ -740,6 +740,16 @@ export class Engine { // it instead of re-prompting cold. await append({ type: 'session.ref.observed', ref: event.ref }) break + case 'notice': + // Shown, but deliberately not recorded as `firstErrorMessage`: + // the turn is answering, just not with what the user picked. + await flush() + await append({ + type: 'assistant.parts.appended', + messageId, + parts: [{ type: 'text', text: `\n\n⚠ ${event.message}` }], + }) + break case 'error': if (firstErrorMessage === null) firstErrorMessage = event.message await append({ diff --git a/apps/desktop/src/renderer/src/features/composer/ModelSelector.test.tsx b/apps/desktop/src/renderer/src/features/composer/ModelSelector.test.tsx index 7e04d29e..4b31ace8 100644 --- a/apps/desktop/src/renderer/src/features/composer/ModelSelector.test.tsx +++ b/apps/desktop/src/renderer/src/features/composer/ModelSelector.test.tsx @@ -318,4 +318,217 @@ describe('ModelSelector', () => { await user.click(within(listbox).getByText('Opus 4')) expect(onChange).toHaveBeenCalledWith({ driverKind: 'claude', modelId: 'opus-4' }) }) + + describe('provider readiness', () => { + /** Replace the detected providers for one test. */ + function detectAs(rows: unknown[]): void { + const base = rpcMocks.invoke.getMockImplementation()! + rpcMocks.invoke.mockImplementation(async (method: string) => + method === 'providers.detect' ? rows : (base(method) as Promise), + ) + } + + it('hides a provider whose CLI is not installed and says why', async () => { + // The reported case: Claude Code present, Codex never installed — yet + // `providers.models` still returns a full Codex catalog, so the rail has + // to gate on the detection rather than on the catalog being non-empty. + detectAs([ + { + kind: 'claude', + installed: true, + binaryPath: 'C:/bin/claude', + version: '1', + authStatus: 'authenticated', + }, + { kind: 'codex', installed: false, binaryPath: null, version: null, authStatus: 'unknown' }, + ]) + setup() + const user = userEvent.setup() + await user.click(await screen.findByRole('button', { name: /model:/i })) + + const rail = screen.getByRole('presentation', { name: 'Providers' }) + expect(within(rail).getByText('Claude')).toBeInTheDocument() + expect(within(rail).queryByText('Codex')).not.toBeInTheDocument() + // ...and the picker explains the absence instead of going quiet about it. + expect(screen.getByText(/not installed/i)).toBeInTheDocument() + }) + + it('hides a provider that is installed but logged out', async () => { + detectAs([ + { + kind: 'claude', + installed: true, + binaryPath: 'C:/bin/claude', + version: '1', + authStatus: 'authenticated', + }, + { + kind: 'codex', + installed: true, + binaryPath: 'C:/bin/codex', + version: '1', + authStatus: 'unauthenticated', + }, + ]) + setup() + const user = userEvent.setup() + await user.click(await screen.findByRole('button', { name: /model:/i })) + + const rail = screen.getByRole('presentation', { name: 'Providers' }) + expect(within(rail).getByText('Claude')).toBeInTheDocument() + expect(within(rail).queryByText('Codex')).not.toBeInTheDocument() + expect(screen.getByText(/not signed in/i)).toBeInTheDocument() + }) + + it('keeps offering a provider whose auth verdict is unknown', async () => { + // No ~/.claude credentials file, but ANTHROPIC_API_KEY may still work — + // "Ari cannot tell" must not hide a usable provider. + detectAs([ + { kind: 'claude', installed: true, binaryPath: 'C:/bin/claude', version: '1', authStatus: 'unknown' }, + ]) + setup() + const user = userEvent.setup() + await user.click(await screen.findByRole('button', { name: /model:/i })) + + const rail = screen.getByRole('presentation', { name: 'Providers' }) + expect(within(rail).getByText('Claude')).toBeInTheDocument() + }) + + it('keeps a locked session on its own models even when that provider is withheld', async () => { + detectAs([ + { + kind: 'claude', + installed: true, + binaryPath: 'C:/bin/claude', + version: '1', + authStatus: 'authenticated', + }, + { + kind: 'codex', + installed: true, + binaryPath: 'C:/bin/codex', + version: '1', + authStatus: 'unauthenticated', + }, + ]) + setup('codex', 'gpt-5.6', 'codex') + const user = userEvent.setup() + await user.click(await screen.findByRole('button', { name: /model:/i })) + + // A mid-session logout must not swap the pane onto another provider. + const listbox = screen.getByRole('listbox', { name: 'Models' }) + expect(within(listbox).getByText('GPT-5.6')).toBeInTheDocument() + }) + }) + + describe('legacy models', () => { + /** Replace the served catalogs for one test. */ + function modelsAs(rows: unknown[]): void { + const base = rpcMocks.invoke.getMockImplementation()! + rpcMocks.invoke.mockImplementation(async (method: string) => + method === 'providers.models' ? rows : (base(method) as Promise), + ) + } + + const opusCatalog = [ + { + kind: 'claude', + source: 'live', + models: [ + { id: 'claude-opus-5', label: 'Claude Opus 5', aliases: ['opus'] }, + { id: 'claude-opus-4-8', label: 'Claude Opus 4.8', isLegacy: true }, + ], + }, + ] + + it('collapses superseded models behind a disclosure', async () => { + modelsAs(opusCatalog) + setup() + const user = userEvent.setup() + await user.click(await screen.findByRole('button', { name: /model:/i })) + + const models = screen.getByRole('listbox', { name: 'Models' }) + expect(within(models).getByText('Claude Opus 5')).toBeInTheDocument() + expect(within(models).queryByText('Claude Opus 4.8')).not.toBeInTheDocument() + + await user.click(screen.getByRole('button', { name: /older/i })) + expect(within(models).getByText('Claude Opus 4.8')).toBeInTheDocument() + }) + + it('checks the row a version-less saved id resolves to', async () => { + modelsAs(opusCatalog) + // The session was saved against the version-less id the CLI accepts. + setup('claude', 'opus') + const user = userEvent.setup() + await user.click(await screen.findByRole('button', { name: /model:/i })) + + const models = screen.getByRole('listbox', { name: 'Models' }) + const selected = within(models).getByRole('option', { selected: true }) + expect(selected).toHaveTextContent('Claude Opus 5') + }) + + it('names the resolved row on the trigger, not the raw saved id', async () => { + modelsAs(opusCatalog) + setup('claude', 'opus') + expect(await screen.findByRole('button', { name: /model:/i })).toHaveTextContent( + 'Claude Opus 5', + ) + }) + }) + + describe('catalog provenance', () => { + /** Replace the served catalogs for one test. */ + function modelsAs(rows: unknown[]): void { + const base = rpcMocks.invoke.getMockImplementation()! + rpcMocks.invoke.mockImplementation(async (method: string) => + method === 'providers.models' ? rows : (base(method) as Promise), + ) + } + + const bundled = [ + { + kind: 'claude', + source: 'snapshot', + models: [{ id: 'claude-opus-5', label: 'Claude Opus 5' }], + }, + ] + + it('marks a list the agent has not confirmed as bundled, not live', async () => { + modelsAs(bundled) + setup() + const user = userEvent.setup() + await user.click(await screen.findByRole('button', { name: /model:/i })) + + // A snapshot names models the agent may refuse outright, so the picker + // says where the list came from instead of implying the agent offered it. + expect(screen.getByText(/bundled list/i)).toBeInTheDocument() + }) + + it('does not mark a list the agent reported itself', async () => { + modelsAs([ + { + kind: 'claude', + source: 'live', + models: [{ id: 'sonnet', label: 'Sonnet' }], + }, + ]) + setup() + const user = userEvent.setup() + await user.click(await screen.findByRole('button', { name: /model:/i })) + expect(screen.queryByText(/bundled list/i)).not.toBeInTheDocument() + }) + + it('re-reads the catalogs when the refresh control is used', async () => { + modelsAs(bundled) + setup() + const user = userEvent.setup() + await user.click(await screen.findByRole('button', { name: /model:/i })) + const before = rpcMocks.invoke.mock.calls.filter(([m]) => m === 'providers.models').length + + await user.click(screen.getByRole('button', { name: /refresh/i })) + expect( + rpcMocks.invoke.mock.calls.filter(([m]) => m === 'providers.models').length, + ).toBeGreaterThan(before) + }) + }) }) diff --git a/apps/desktop/src/renderer/src/features/composer/ModelSelector.tsx b/apps/desktop/src/renderer/src/features/composer/ModelSelector.tsx index e2fe4ac6..ebb4f773 100644 --- a/apps/desktop/src/renderer/src/features/composer/ModelSelector.tsx +++ b/apps/desktop/src/renderer/src/features/composer/ModelSelector.tsx @@ -3,8 +3,11 @@ import { Check, ChevronDown, Search } from 'lucide-react' import type { DriverKind } from '@ari/contracts/common' import type { CatalogModelInfo } from '@ari/contracts/rpc' import { modelsFor } from '@ari/providers/catalogs' +import type { CatalogSource } from '@ari/providers/catalogs' import { rpc } from '../../lib/rpc' import { driverLabel } from './agent-mark' +import { partitionProviders } from './provider-readiness' +import type { ProviderReadiness } from './provider-readiness' import { ProviderLogo } from './provider-logo' export interface SelectorOption { @@ -12,11 +15,41 @@ export interface SelectorOption { label: string group: string hint?: string + /** Other ids that resolve to this same model; matched when marking selection. */ + aliases?: string[] + /** Superseded within its family; hidden behind the picker's disclosure. */ + isLegacy?: boolean +} + +/** True when an option is the current one, following version-less aliases. */ +function matchesCurrent(option: SelectorOption, currentId: string): boolean { + return option.id === currentId || (option.aliases?.includes(currentId) ?? false) } /** Live catalogs by kind; absent kinds fall back to the bundled snapshot. */ type CatalogByKind = Partial> +/** Where each served catalog came from, so the picker can say when it is not the agent's own. */ +type SourcesByKind = Partial> + +/** + * Names a non-`live` catalog for what it is. Only `live` is the agent's own + * list of what it accepts; everything else is Ari's guess at it, which is + * exactly the list that can offer a model the agent will refuse. + */ +function fallbackNote(source: CatalogSource): string | null { + switch (source) { + case 'live': + return null + case 'cache': + return 'a cached list from the model registry' + case 'snapshot': + return 'a bundled list' + case 'static': + return 'a placeholder list' + } +} + /** One left-rail entry: an installed provider and how many models it serves. */ interface ProviderRow { kind: DriverKind @@ -60,9 +93,11 @@ export function ModelSelector({ const [activeKind, setActiveKind] = useState(null) const [query, setQuery] = useState('') const [activeIndex, setActiveIndex] = useState(0) - const [drivers, setDrivers] = useState<{ kind: DriverKind; label: string }[]>([]) + const [detections, setDetections] = useState([]) const [catalog, setCatalog] = useState({}) + const [sources, setSources] = useState({}) const [endpointModels, setEndpointModels] = useState([]) + const [showLegacy, setShowLegacy] = useState(false) const [loaded, setLoaded] = useState(false) const listRef = useRef(null) const searchRef = useRef(null) @@ -74,22 +109,24 @@ export function ModelSelector({ * restart. */ const loadCatalogs = useCallback(() => { void Promise.allSettled([ - rpc.invoke('providers.detect').then((detections) => { - setDrivers( - detections - .filter((d) => d.binaryPath !== null || d.kind === 'ari-core') - .map((d) => ({ - kind: d.kind as DriverKind, - label: driverLabel(d.kind), - })), - ) + rpc.invoke('providers.detect').then((rows) => { + // Stored raw: the readiness split happens in a memo, because the + // withheld half is what the picker needs to explain an absent rail row. + setDetections(rows) }), rpc.invoke('providers.models').then((rows) => { const byKind: CatalogByKind = {} + const sources: SourcesByKind = {} // Every source is usable: snapshot/cache are curated to what each CLI // currently serves and `live` rows are the harness's own model list. - for (const row of rows) byKind[row.kind as DriverKind] = row.models + // Which one it was is kept: only `live` is the agent's own word, and + // the picker says so when a list came from anywhere else. + for (const row of rows) { + byKind[row.kind as DriverKind] = row.models + sources[row.kind as DriverKind] = row.source + } setCatalog(byKind) + setSources(sources) }), rpc.invoke('endpoints.list').then((endpoints) => { // One row per model an endpoint serves, so a single endpoint holding @@ -138,6 +175,8 @@ export function ModelSelector({ label: model.label, group: driverLabel(kind), hint: model.contextHint, + aliases: model.aliases?.map((alias) => `${kind}:${alias}`), + isLegacy: model.isLegacy, })) } return compute @@ -146,19 +185,43 @@ export function ModelSelector({ const currentId = `${driverKind}:${modelId ?? ''}` const currentKindLabel = driverLabel(driverKind) + const { ready, withheld } = useMemo(() => partitionProviders(detections), [detections]) + const providers = useMemo( - () => drivers.map((driver) => ({ ...driver, count: optionsFor(driver.kind).length })), - [drivers, optionsFor], + () => + ready.map((detection) => { + const kind = detection.kind as DriverKind + return { kind, label: driverLabel(kind), count: optionsFor(kind).length } + }), + [ready, optionsFor], ) const searching = query.trim().length > 0 - /** Pane rows: every model of the active provider, in catalog order. */ - const paneModels = useMemo( + /** Every model the active provider serves, superseded ones included. */ + const allPaneModels = useMemo( () => (activeKind === null ? [] : optionsFor(activeKind)), [activeKind, optionsFor], ) + /** Pane rows: the current models, plus superseded ones once disclosed. */ + const paneModels = useMemo( + () => (showLegacy ? allPaneModels : allPaneModels.filter((o) => o.isLegacy !== true)), + [allPaneModels, showLegacy], + ) + + const legacyCount = useMemo( + () => allPaneModels.filter((o) => o.isLegacy === true).length, + [allPaneModels], + ) + + /** Why the active pane's list is not the agent's own, when it is not. */ + const fallbackLabel = useMemo(() => { + if (activeKind === null || activeKind === 'ari-core') return null + const source = sources[activeKind] + return source === undefined ? null : fallbackNote(source) + }, [activeKind, sources]) + /** Search rows: matching models from every provider, grouped, flat order. */ const results = useMemo(() => { const q = query.trim().toLowerCase() @@ -186,10 +249,19 @@ export function ModelSelector({ /** Provider the pane opens on: the session's current one (or the lock). */ const defaultKind = useMemo(() => { - if (drivers.length === 0) return null - const wanted = lockedTo ?? driverKind - return drivers.some((d) => d.kind === wanted) ? wanted : (drivers[0]?.kind ?? null) - }, [drivers, lockedTo, driverKind]) + // A locked session keeps its own harness even when that harness is + // currently withheld — a mid-session logout must not silently swap the + // pane onto a different provider's models. + if (lockedTo !== null) return lockedTo + if (providers.length === 0) return null + return providers.some((p) => p.kind === driverKind) ? driverKind : (providers[0]?.kind ?? null) + }, [providers, lockedTo, driverKind]) + + useEffect(() => { + // Each provider starts on its current models; a disclosure left open + // across a provider switch would hide which list you are looking at. + setShowLegacy(false) + }, [activeKind]) useEffect(() => { if (!open) return @@ -237,6 +309,9 @@ export function ModelSelector({ } const onMenuKeyDown = (e: React.KeyboardEvent): void => { + // A focused button owns its own keys — the disclosure toggle would + // otherwise both expand and pick the highlighted model on one Enter. + if ((e.target as HTMLElement).tagName === 'BUTTON') return if (e.key === 'Escape') { e.preventDefault() close() @@ -292,7 +367,7 @@ export function ModelSelector({ return legacy?.label ?? modelId ?? 'Ari Core' } const list = optionsFor(driverKind) - return list.find((o) => o.id === currentId)?.label ?? modelId ?? 'CLI default' + return list.find((o) => matchesCurrent(o, currentId))?.label ?? modelId ?? 'CLI default' }, [driverKind, modelId, currentId, optionsFor, endpointModels]) const rowClasses = (isActive: boolean): string => @@ -301,7 +376,7 @@ export function ModelSelector({ }` const optionRow = (opt: SelectorOption, index: number, markKind: string | null) => { - const isSelected = opt.id === currentId + const isSelected = matchesCurrent(opt, currentId) return ( + ) : null} + + {!searching && fallbackLabel !== null && activeKind !== null ? ( +
+ + {driverLabel(activeKind)} has not reported its own models — this is{' '} + {fallbackLabel}. It may not accept every entry. + + +
+ ) : null} + {lockedTo !== null ? (

This session runs on {driverLabel(lockedTo)}. Start a new session to use another agent.

) : null} + + {lockedTo === null && withheld.length > 0 ? ( +
+

Not shown

+ {withheldNote} +
+ ) : null} ) : null} diff --git a/apps/desktop/src/renderer/src/features/composer/provider-readiness.test.ts b/apps/desktop/src/renderer/src/features/composer/provider-readiness.test.ts new file mode 100644 index 00000000..1b91c277 --- /dev/null +++ b/apps/desktop/src/renderer/src/features/composer/provider-readiness.test.ts @@ -0,0 +1,78 @@ +import { describe, expect, it } from 'vitest' +import type { Detection } from '@ari/providers/types' +import { partitionProviders } from './provider-readiness' + +function detection(overrides: Partial & { kind: Detection['kind'] }): Detection { + return { + installed: false, + binaryPath: null, + version: null, + authStatus: 'unknown', + ...overrides, + } +} + +const installedClaude = detection({ + kind: 'claude', + installed: true, + binaryPath: 'C:/bin/claude', + authStatus: 'authenticated', +}) + +/** The reporter's case: Claude present and working, Codex genuinely absent. */ +const absentCodex = detection({ kind: 'codex' }) + +describe('partitionProviders', () => { + it('offers an installed, authenticated provider', () => { + const { ready } = partitionProviders([installedClaude]) + expect(ready.map((d) => d.kind)).toEqual(['claude']) + }) + + it('offers an installed provider whose auth verdict is unknown', () => { + // "Ari cannot tell" is not "logged out" — Claude Code with an + // ANTHROPIC_API_KEY has no credentials file and must still be offered. + const { ready } = partitionProviders([ + detection({ kind: 'claude', installed: true, binaryPath: 'C:/bin/claude' }), + ]) + expect(ready.map((d) => d.kind)).toEqual(['claude']) + }) + + it('withholds an installed provider that is known to be logged out', () => { + const { ready, withheld } = partitionProviders([ + detection({ + kind: 'codex', + installed: true, + binaryPath: 'C:/bin/codex', + authStatus: 'unauthenticated', + }), + ]) + expect(ready).toEqual([]) + expect(withheld.map((entry) => entry.detection.kind)).toEqual(['codex']) + }) + + it('withholds a provider that is not installed', () => { + const { ready, withheld } = partitionProviders([absentCodex]) + expect(ready).toEqual([]) + expect(withheld.map((entry) => entry.detection.kind)).toEqual(['codex']) + }) + + it('never offers ari-core, which has no binary of its own', () => { + const { ready } = partitionProviders([detection({ kind: 'ari-core' })]) + expect(ready.map((d) => d.kind)).toEqual(['ari-core']) + }) + + it('keeps detection order in both partitions', () => { + const { ready, withheld } = partitionProviders([ + absentCodex, + installedClaude, + detection({ kind: 'pi', installed: true, binaryPath: 'C:/bin/pi' }), + ]) + expect(ready.map((d) => d.kind)).toEqual(['claude', 'pi']) + expect(withheld.map((entry) => entry.detection.kind)).toEqual(['codex']) + }) + + it('explains why a provider was withheld', () => { + const { withheld } = partitionProviders([absentCodex]) + expect(withheld[0]?.reason).toBeTruthy() + }) +}) diff --git a/apps/desktop/src/renderer/src/features/composer/provider-readiness.ts b/apps/desktop/src/renderer/src/features/composer/provider-readiness.ts new file mode 100644 index 00000000..d9ed055a --- /dev/null +++ b/apps/desktop/src/renderer/src/features/composer/provider-readiness.ts @@ -0,0 +1,65 @@ +/** + * The detection fields the rail gate reads. Structural rather than the full + * `Detection`, because the renderer receives these over RPC with `kind` and + * `authStatus` widened to `string`; a generic keeps both call sites typed. + */ +export interface ProviderReadiness { + kind: string + installed: boolean + authStatus: string + authReason?: string | undefined +} + +/** A provider the rail will not offer, paired with why. */ +export interface WithheldProvider { + detection: T + /** Sentence for the picker's "not shown" note; names no provider itself. */ + reason: string +} + +export interface ProviderPartition { + /** Providers the rail offers: installed and not known to be logged out. */ + ready: T[] + /** Detected-but-unusable providers, so the picker can say where one went. */ + withheld: WithheldProvider[] +} + +/** + * Splits detections into the providers the picker may offer and those it must + * withhold, so the rail never advertises a harness the user cannot run a turn + * on. Two conditions, deliberately different in kind: + * + * - `installed` — a real binary resolved on disk. A provider that is absent + * still gets a fully-populated catalog from `providers.models` (catalogs are + * fetched per kind, not per machine), so the rail has to gate on this or an + * uninstalled harness renders as a working one. + * - `authStatus !== 'unauthenticated'` — only `unauthenticated` withholds. + * `unknown` means Ari found no credential store but the CLI may authenticate + * another way (`ANTHROPIC_API_KEY`, a subscription session); hiding those + * would drop providers that work fine. + * + * `ari-core` is Ari's own runtime and has no binary to find, so it is always + * offered regardless of what the probe reported. + */ +export function partitionProviders( + detections: readonly T[], +): ProviderPartition { + const ready: T[] = [] + const withheld: WithheldProvider[] = [] + for (const detection of detections) { + if (detection.kind !== 'ari-core' && !detection.installed) { + withheld.push({ detection, reason: 'Not installed - no CLI found on PATH.' }) + continue + } + if (detection.authStatus === 'unauthenticated') { + withheld.push({ detection, reason: unauthenticatedReason(detection) }) + continue + } + ready.push(detection) + } + return { ready, withheld } +} + +function unauthenticatedReason(detection: ProviderReadiness): string { + return detection.authReason ?? 'Not signed in - run the CLI once to authenticate.' +} diff --git a/packages/contracts/src/agent-event.ts b/packages/contracts/src/agent-event.ts index a8813418..5fa3952c 100644 --- a/packages/contracts/src/agent-event.ts +++ b/packages/contracts/src/agent-event.ts @@ -57,6 +57,13 @@ export const agentEventSchema = z.discriminatedUnion('type', [ costUsd: z.number().nullable(), }), z.object({ type: z.literal('status'), status: sessionStatusSchema }), + /** + * Something the user must know that did not fail the turn — a requested + * model the harness does not offer, so the reply came from a different one. + * Distinct from `error` on purpose: `error` settles the turn as failed, and + * a turn that ran on the wrong model still ran. + */ + z.object({ type: z.literal('notice'), message: z.string() }), z.object({ type: z.literal('error'), message: z.string(), rawJson: z.string().nullable() }), z.object({ type: z.literal('done') }), ]) diff --git a/packages/contracts/src/rpc.ts b/packages/contracts/src/rpc.ts index e9961e38..fa8e8397 100644 --- a/packages/contracts/src/rpc.ts +++ b/packages/contracts/src/rpc.ts @@ -280,6 +280,10 @@ export const catalogModelSchema = z.object({ label: z.string().min(1), /** Short context-window hint rendered beside the label, e.g. `200k`. */ contextHint: z.string().optional(), + /** Other ids that resolve to this same model (version-less family pointers). */ + aliases: z.array(z.string().min(1)).optional(), + /** Superseded within its family; the picker collapses these behind a disclosure. */ + isLegacy: z.boolean().optional(), }) export type CatalogModelInfo = z.infer diff --git a/packages/providers/src/acp/acp-driver.test.ts b/packages/providers/src/acp/acp-driver.test.ts index 82064f4e..25044d1a 100644 --- a/packages/providers/src/acp/acp-driver.test.ts +++ b/packages/providers/src/acp/acp-driver.test.ts @@ -1,11 +1,13 @@ import { PassThrough } from 'node:stream' -import { beforeEach, describe, expect, it, vi } from 'vitest' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import type { AgentEvent } from '@ari/contracts/agent-event' +import { catalogSource, clearDynamicModels, modelsFor } from '../catalogs' import { createAcpAdapter, AcpDriver, launchWithEffort, pickAgentMode, + publishAdvertisedModels, shouldFallBack, __resetLearnedAcpSelectors, } from './acp-driver' @@ -227,6 +229,44 @@ function modeCalls(child: FakeChild): (string | undefined)[] { .map((m) => (m['params'] as { modeId?: string }).modeId) } +/** + * Writes a `config_option_update` notification, the way an agent announces + * that the session's model select has moved. + */ +function pushConfigUpdate(child: FakeChild, currentValue: string): void { + child.stdout.write( + `${JSON.stringify({ + jsonrpc: '2.0', + method: 'session/update', + params: { + sessionId: 'sess_acp_1', + update: { + sessionUpdate: 'config_option_update', + configOptions: [ + { + id: 'model', + name: 'Model', + category: 'model', + type: 'select', + currentValue, + options: [ + { value: 'm1', name: 'Model One' }, + { value: 'm2', name: 'Model Two' }, + { value: 'm3', name: 'Model Three' }, + ], + }, + ], + }, + }, + })}\n`, + ) +} + +/** Notices only: the events that tell a user their pick did not take. */ +function noticesOf(events: AgentEvent[]): string[] { + return events.flatMap((event) => (event.type === 'notice' ? [event.message] : [])) +} + describe('createAcpAdapter', () => { beforeEach(() => { __resetLearnedAcpSelectors() @@ -314,6 +354,176 @@ describe('createAcpAdapter', () => { await adapter.dispose() }, 15000) + it('says so when the agent does not offer the requested model', async () => { + const child = fakeChild() + script(child, CONFIG_OPTIONS_AGENT) + const adapter = await createAcpAdapter( + LAUNCH, + { ...SESSION, modelId: 'not-offered' }, + () => child, + ) + // The turn still runs — on the agent's default, not on the user's pick. + // Dropping that at `log.debug` is what let the composer keep showing + // `not-offered` as the active model while every reply came from `m1`. + const notices = (await collectEvents(adapter)).filter((event) => event.type === 'notice') + const messages = notices.map((event) => (event.type === 'notice' ? event.message : '')) + expect(messages).toHaveLength(1) + expect(messages[0]).toContain('not-offered') + await adapter.dispose() + }, 15000) + + it('stays quiet when the requested model is advertised', async () => { + const child = fakeChild() + script(child, CONFIG_OPTIONS_AGENT) + const adapter = await createAcpAdapter(LAUNCH, { ...SESSION, modelId: 'm2' }, () => child) + const types = await collectTypes(adapter) + expect(types).not.toContain('notice') + await adapter.dispose() + }, 15000) + + it('says so when the agent moves off the picked model mid-turn', async () => { + const child = fakeChild() + script(child, (method, params, id) => { + if (method === 'session/prompt') { + // The session advertises `m1`, Ari asked for `m2`, and the agent has + // since moved to `m3` — replies are about to come from a model the + // composer still shows as `m2`. + pushConfigUpdate(child, 'm3') + return { stopReason: 'end_turn' } + } + return CONFIG_OPTIONS_AGENT(method, params, id) + }) + const adapter = await createAcpAdapter(LAUNCH, { ...SESSION, modelId: 'm2' }, () => child) + const notices = noticesOf(await collectEvents(adapter)) + expect(notices).toHaveLength(1) + expect(notices[0]).toContain('m3') + expect(notices[0]).toContain('m2') + await adapter.dispose() + }, 15000) + + it('stays quiet when the agent just re-advertises the model already in use', async () => { + const child = fakeChild() + script(child, (method, params, id) => { + if (method === 'session/prompt') { + // Same value the session already advertised: a re-announcement, not a + // move. Warning here would put a ⚠ on every ordinary turn. + pushConfigUpdate(child, 'm1') + return { stopReason: 'end_turn' } + } + return CONFIG_OPTIONS_AGENT(method, params, id) + }) + const adapter = await createAcpAdapter(LAUNCH, { ...SESSION, modelId: 'm2' }, () => child) + expect(noticesOf(await collectEvents(adapter))).toEqual([]) + await adapter.dispose() + }, 15000) + + it('stays quiet when the agent confirms the pick the user asked for', async () => { + const child = fakeChild() + script(child, (method, params, id) => { + if (method === 'session/prompt') { + pushConfigUpdate(child, 'm2') + return { stopReason: 'end_turn' } + } + return CONFIG_OPTIONS_AGENT(method, params, id) + }) + const adapter = await createAcpAdapter(LAUNCH, { ...SESSION, modelId: 'm2' }, () => child) + expect(noticesOf(await collectEvents(adapter))).toEqual([]) + await adapter.dispose() + }, 15000) + + it('reports a drift only once for the same move', async () => { + const child = fakeChild() + script(child, (method, params, id) => { + if (method === 'session/prompt') { + pushConfigUpdate(child, 'm3') + pushConfigUpdate(child, 'm3') + return { stopReason: 'end_turn' } + } + return CONFIG_OPTIONS_AGENT(method, params, id) + }) + const adapter = await createAcpAdapter(LAUNCH, { ...SESSION, modelId: 'm2' }, () => child) + expect(noticesOf(await collectEvents(adapter))).toHaveLength(1) + await adapter.dispose() + }, 15000) + + it('sends the agent default when the agent advertises it as a choice', async () => { + const child = fakeChild() + script(child, (method, params, id) => { + if (method === 'session/new') { + return { + sessionId: 'sess_acp_1', + configOptions: [ + { + id: 'model', + category: 'model', + type: 'select', + currentValue: 'sonnet', + options: [ + { value: 'default', name: 'Default' }, + { value: 'sonnet', name: 'Sonnet' }, + ], + }, + ], + } + } + if (method === 'session/set_config_option') return { configOptions: [] as unknown[] } + return standardAgent()(method, params, id) + }) + // `default` is Ari's own placeholder id, so it used to be skipped outright — + // which made switching *back* to the CLI default from a picked model a + // silent no-op, leaving the previous model answering. + const adapter = await createAcpAdapter(LAUNCH, { ...SESSION, modelId: 'default' }, () => child) + const modelRequests = child.sent.filter( + (m) => + m['method'] === 'session/set_config_option' && + (m['params'] as { configId?: string })?.configId === 'model', + ) as { params?: { value?: string } }[] + expect(modelRequests.map((r) => r.params?.value)).toEqual(['default']) + await adapter.dispose() + }, 15000) + + it('sends nothing for the placeholder default an agent never offered', async () => { + const child = fakeChild() + script(child, CONFIG_OPTIONS_AGENT) + // No live catalog has landed, so `default` is only `CLI_DEFAULT_MODELS`' + // placeholder — not something this agent would understand. It must not be + // sent, and it must not produce a ⚠ about a model nobody picked. + const adapter = await createAcpAdapter(LAUNCH, { ...SESSION, modelId: 'default' }, () => child) + const types = await collectTypes(adapter) + const modelRequests = child.sent.filter( + (m) => + m['method'] === 'session/set_config_option' && + (m['params'] as { configId?: string })?.configId === 'model', + ) + expect(modelRequests).toEqual([]) + expect(types).not.toContain('notice') + await adapter.dispose() + }, 15000) + + it('reports the models the agent advertised, in the agent’s own vocabulary', async () => { + const child = fakeChild() + script(child, CONFIG_OPTIONS_AGENT) + const adapter = await createAcpAdapter(LAUNCH, SESSION, () => child) + // The picker's catalog for a kind comes from what the runtime a session + // actually uses advertises. A throwaway probe can fail for reasons that + // have nothing to do with the session — an uncached npx package under + // `--no-install` — and the snapshot it falls back to names models this + // agent will not accept at all. + expect(adapter.advertisedModels).toEqual([ + { id: 'm1', label: 'Model One' }, + { id: 'm2', label: 'Model Two' }, + ]) + await adapter.dispose() + }, 15000) + + it('reports nothing for an agent that advertises no model selector', async () => { + const child = fakeChild() + script(child, standardAgent()) + const adapter = await createAcpAdapter(LAUNCH, SESSION, () => child) + expect(adapter.advertisedModels).toEqual([]) + await adapter.dispose() + }, 15000) + it('streams a full turn and closes on the stop reason', async () => { const child = fakeChild() script(child, (method, params, id) => { @@ -1239,6 +1449,34 @@ describe('createAcpAdapter', () => { }, 15000) }) +describe('publishAdvertisedModels', () => { + afterEach(() => { + clearDynamicModels('claude') + clearDynamicModels('codex') + }) + + it('installs the session agent’s own models as the kind’s live catalog', () => { + publishAdvertisedModels('claude', [ + { id: 'sonnet', label: 'Sonnet' }, + { id: 'opus[1m]', label: 'Opus (1M)' }, + ]) + expect(catalogSource('claude')).toBe('live') + expect(modelsFor('claude')).toEqual([ + { id: 'sonnet', label: 'Sonnet' }, + { id: 'opus[1m]', label: 'Opus (1M)' }, + ]) + }) + + it('leaves a usable catalog alone when the agent advertises nothing', () => { + const before = modelsFor('codex') + publishAdvertisedModels('codex', []) + // An agent with no model selector is not evidence that the kind has no + // models — replacing the catalog with nothing would empty the picker. + expect(modelsFor('codex')).toEqual(before) + expect(catalogSource('codex')).not.toBe('live') + }) +}) + describe('launchWithEffort', () => { it('prepends --effort on a grok native ACP launch', () => { expect( diff --git a/packages/providers/src/acp/acp-driver.ts b/packages/providers/src/acp/acp-driver.ts index 4f6b9827..4b098052 100644 --- a/packages/providers/src/acp/acp-driver.ts +++ b/packages/providers/src/acp/acp-driver.ts @@ -26,7 +26,9 @@ import { findThoughtOption, looksLikeThoughtAxis } from './thought' import { classifyAgentMode, findModeOption, pickAgentMode } from './modes' export { pickAgentMode } from './modes' import { loadImageData, missingImagesNote, stagedImagesOf } from '../attachments' -import { AcpUpdateFolder, stopReasonEvents } from './protocol' +import type { CatalogModel } from '../catalogs' +import { setDynamicModels } from '../catalogs' +import { AcpUpdateFolder, configOptionsFromUpdate, currentModelId, stopReasonEvents } from './protocol' import type { AcpConfigOption, AcpInitializeResult, @@ -52,6 +54,12 @@ export interface AcpAdapter extends ProviderAdapter { respondApproval(approvalId: string, decision: AdapterApprovalDecision): void /** Answers a pending `input-requested` question (elicitation, ask-user, plan). */ respondInput(inputId: string, value: string): void + /** + * The models this session's agent advertises, in the agent's own vocabulary. + * The host installs these as the kind's live catalog: discovery from the + * runtime that actually runs is the only list guaranteed to be accepted. + */ + advertisedModels: CatalogModel[] } /** @@ -278,14 +286,32 @@ export async function createAcpAdapter( // folder has the exact prelude before that notification can be folded. folder.setStartupInfo(created._meta?.piAcp?.startupInfo) const sessionId = created.sessionId as string - connection.onSessionUpdate = (notification) => push(folder.fold(notification)) + const drift = new ModelDriftWatch( + session.modelId, + currentModelId(created.configOptions ?? null), + ) + connection.onSessionUpdate = (notification) => { + push(folder.fold(notification)) + // The agent announcing a configuration move has no transcript surface, but + // a model it switched to on its own is something the user has to hear. + const moved = configOptionsFromUpdate(notification) + if (moved !== null) { + const notice = drift.observe(moved) + if (notice !== null) push([notice]) + } + } // Publish the agent's session id so Ari can resume it via session/load on // the next turn instead of losing all context. push([{ type: 'session-ref', ref: sessionId }]) const selectors = resolveSelectors(launch.label, created, resumed) try { - await applyModel(connection, sessionId, selectors.configOptions, session.modelId) + const applied = await applyModel(connection, sessionId, selectors.configOptions, session.modelId) + // The pick did not reach the agent, so a different model is about to + // answer. Say it in the transcript: the composer still shows the pick. + if (applied.kind === 'unavailable' && session.modelId !== null) { + push([modelUnavailableNotice(session.modelId, applied.offered)]) + } await applyPermissionMode( connection, sessionId, @@ -470,9 +496,32 @@ export async function createAcpAdapter( if (!connection.closed) connection.cancel(sessionId) await connection.shutdown() }, + advertisedModels: modelsFromConfigOptions(selectors.configOptions), } } +/** + * The models an agent's own `model` selector offers, in its own vocabulary — + * `sonnet`, `opus[1m]`, `gpt-5.6-sol` — rather than the vendor ids a registry + * uses. These are the exact strings {@link applyModel} has to send back, so + * they are also the only list the picker can offer without risking a refusal. + */ +function modelsFromConfigOptions(configOptions: AcpConfigOption[]): CatalogModel[] { + const option = findModelOption(configOptions) + const values = option?.options ?? [] + return values.flatMap((value) => + typeof value.value === 'string' && value.value.length > 0 + ? [ + { + id: value.value, + label: + typeof value.name === 'string' && value.name.length > 0 ? value.name : value.value, + }, + ] + : [], + ) +} + /** True when the agent advertised `session/resume` (no history replay). */ export function sessionResumeSupported(initialize: AcpInitializeResult): boolean { return initialize.agentCapabilities?.sessionCapabilities?.resume === true @@ -565,24 +614,100 @@ export function __resetLearnedAcpSelectors(): void { LEARNED_SELECTORS.clear() } +/** + * What {@link applyModel} managed to do with the session's model pick. + * `unavailable` is the case that used to vanish into `log.debug`: the agent + * advertises models, just not that one, so the turn runs on whatever the + * agent defaults to. + */ +type ModelApplyOutcome = + | { kind: 'applied' } + | { kind: 'skipped' } + | { kind: 'unavailable'; offered: string[] } + async function applyModel( connection: AcpConnection, sessionId: string, configOptions: AcpConfigOption[], modelId: string | null, -): Promise { - if (modelId === null || modelId === 'default') return +): Promise { + if (modelId === null) return { kind: 'skipped' } const option = findModelOption(configOptions) - if (option === null || option.options === undefined) return + if (option === null || option.options === undefined) return { kind: 'skipped' } const value = option.options.find((v) => v.value === modelId)?.value if (value === undefined) { + // `default` is also Ari's own placeholder id (`CLI_DEFAULT_MODELS`), used + // when no live catalog has landed. An agent that does not offer it has + // nothing to be told and nothing to warn about. + if (modelId === 'default') return { kind: 'skipped' } log.debug('acp: requested model not advertised by agent', { modelId }) - return + return { + kind: 'unavailable', + offered: option.options + .map((v) => v.value) + .filter((v): v is string => typeof v === 'string' && v.length > 0), + } } try { await connection.setConfigOption(sessionId, option.id as string, value) } catch (error) { log.debug('acp: set_config_option(model) failed', { error: String(error) }) + // The agent took the request and refused it — from the user's side that is + // the same outcome as never offering the model: a different one answered. + return { kind: 'unavailable', offered: [] } + } + return { kind: 'applied' } +} + +/** + * The line a user sees when their model pick did not reach the agent. Names + * the pick and what actually ran, because "the model is not available" alone + * leaves the composer still showing the pick as active. + */ +function modelUnavailableNotice(modelId: string, offered: string[]): AgentEvent { + const fallback = + offered.length > 0 + ? `the agent's default. It offers: ${offered.join(', ')}` + : "the agent's default" + return { + type: 'notice', + message: `"${modelId}" is not offered by this agent, so this turn ran on ${fallback}.`, + } +} + +/** + * Watches the agent's own account of which model is running, so a session that + * moves off the user's pick says so instead of answering from a model the + * composer still shows as something else. + * + * `config_option_update` used to fall through {@link AcpUpdateFolder}'s default + * branch, which made the move invisible. The watch is transition-based rather + * than "current value differs from the pick": agents that echo their pre-set + * `currentValue` after a successful `set_config_option` would otherwise put a + * ⚠ on every ordinary turn. + */ +class ModelDriftWatch { + readonly #picked: string | null + /** The last model the agent itself reported; the baseline before any move. */ + #agentModel: string | null + + constructor(picked: string | null, advertised: string | null) { + this.#picked = picked + this.#agentModel = advertised + } + + /** Folds one round of advertised options into a notice, when one is due. */ + observe(configOptions: AcpConfigOption[] | null | undefined): AgentEvent | null { + const current = currentModelId(configOptions) + if (current === null || current === this.#agentModel) return null + this.#agentModel = current + // Moving onto the model the user asked for is the pick taking effect, not + // news. A session with no pick has nothing to be right or wrong about. + if (this.#picked === null || current === this.#picked) return null + return { + type: 'notice', + message: `The agent moved to "${current}"; this session was set to "${this.#picked}".`, + } } } /** @@ -708,6 +833,23 @@ export function shouldFallBack(error: unknown): boolean { return !(error instanceof AcpAuthRequiredError) } +/** + * Installs the model list a session's agent advertised as the kind's live + * catalog, so the picker offers what the runtime that will actually run + * accepts rather than what a registry says exists. + * + * This matters most when the throwaway probe *failed*: an agent whose npx + * package is not cached yet under `--no-install` still runs fine on a real + * turn, and the snapshot the picker fell back to in the meantime names models + * that agent refuses outright. An agent advertising nothing changes nothing — + * an empty list must never replace a usable catalog. + */ +export function publishAdvertisedModels(kind: DriverKind, models: CatalogModel[]): void { + if (models.length === 0) return + setDynamicModels(kind, 'live', models) + log.info('catalog taken from the session agent', { kind, count: models.length }) +} + /** * Driver that prefers the ACP transport and transparently falls back to the * legacy one-shot CLI driver whenever ACP is disabled, unresolvable, or its @@ -739,6 +881,7 @@ export class AcpDriver implements Driver { this.onAuthRequired ?? undefined, ) log.info('turn started over ACP', { kind: this.kind, launch: this.launch.label }) + publishAdvertisedModels(this.kind, adapter.advertisedModels) return adapter } catch (error) { if (!shouldFallBack(error)) throw error diff --git a/packages/providers/src/acp/protocol.ts b/packages/providers/src/acp/protocol.ts index 9618ca0d..f93d22b7 100644 --- a/packages/providers/src/acp/protocol.ts +++ b/packages/providers/src/acp/protocol.ts @@ -231,8 +231,28 @@ export function terminalLoginsFrom( return logins } -/** The argv for one auth method, or null when Ari has no way to run it. */ -function runnableLaunch( +/** + * The `configOptions` a `config_option_update` notification carries, or null + * when this notification is something else. The update is how an agent says + * its session configuration moved on its own — the model select reporting a + * new `currentValue` — which the folder has no transcript surface for. + */ +export function configOptionsFromUpdate( + notification: AcpSessionNotification, +): AcpConfigOption[] | null { + const update = notification.update + if (update?.sessionUpdate !== 'config_option_update') return null + return update.configOptions ?? null +} + +/** The model a round of config options reports as in use, if it names one. */ +export function currentModelId(configOptions: AcpConfigOption[] | null | undefined): string | null { + const option = (configOptions ?? []).find((o) => o.category === 'model' && o.type === 'select') + const current = option?.currentValue + return typeof current === 'string' && current.length > 0 ? current : null +} + +/** The argv for one auth method, or null when Ari has no way to run it. */function runnableLaunch( method: AcpAuthMethod, agentLaunch?: { command: string; args: string[] }, ): { command: string; args: string[] } | null { diff --git a/packages/providers/src/catalog-service.test.ts b/packages/providers/src/catalog-service.test.ts index d0b81fd0..258fab3f 100644 --- a/packages/providers/src/catalog-service.test.ts +++ b/packages/providers/src/catalog-service.test.ts @@ -4,7 +4,7 @@ import { join } from 'node:path' import { afterEach, describe, expect, it, vi } from 'vitest' import type { DriverKind } from '@ari/contracts/common' import { catalogSource, clearDynamicModels, modelsFor } from './catalogs' -import { CatalogService, DEFAULT_REGISTRY_URL, REGISTRY_PROVIDER } from './catalog-service' +import { CatalogService, DEFAULT_REGISTRY_URL, REFRESH_TTL_MS, REGISTRY_PROVIDER } from './catalog-service' import type { CatalogModel } from './catalogs' const ALL_KINDS: DriverKind[] = ['claude', 'codex', 'opencode', 'grok', 'pi', 'hermes', 'ari-core'] @@ -90,9 +90,6 @@ describe('CatalogService', () => { const service = new CatalogService({ fetchImpl: fetchImpl as unknown as typeof fetch }) await service.refresh() expect(modelsFor('claude')).toEqual([ - { id: 'fable', label: 'Fable (latest)' }, - { id: 'opus', label: 'Opus (latest)' }, - { id: 'sonnet', label: 'Sonnet (latest)' }, { id: 'claude-x', label: 'Claude X', contextHint: '200k' }, ]) expect(catalogSource('claude')).toBe('cache') @@ -123,7 +120,9 @@ describe('CatalogService', () => { const cached = JSON.parse(await readFile(cachePath, 'utf8')) as { providers: Record } - expect(cached.providers['anthropic']?.map((model) => model.id)).toContain('claude-opus-5') + // The round carried no anthropic rows, so it leaves nothing behind for + // claude: the key stays absent rather than freezing the snapshot. + expect(cached.providers['anthropic']).toBeUndefined() // Rounds that did carry usable rows still apply. expect(modelsFor('codex')).toEqual([{ id: 'gpt-6-astra', label: 'GPT-6 Astra' }]) }) @@ -217,11 +216,11 @@ describe('CatalogService', () => { at: number providers: Record } + // `family` rides along: a cold start with no network has only this cache + // to collapse pointers with. A model the registry gives no family is its + // own family, which is why it defaults to the id. expect(cached.providers['anthropic']).toEqual([ - { id: 'fable', label: 'Fable (latest)' }, - { id: 'opus', label: 'Opus (latest)' }, - { id: 'sonnet', label: 'Sonnet (latest)' }, - { id: 'claude-disk', label: 'Disk Model' }, + { id: 'claude-disk', label: 'Disk Model', family: 'claude-disk' }, ]) // Cold start with a dead network must restore the cached catalogs. @@ -231,15 +230,103 @@ describe('CatalogService', () => { }) second.start() await second.ready - expect(modelsFor('claude')).toEqual([ - { id: 'fable', label: 'Fable (latest)' }, - { id: 'opus', label: 'Opus (latest)' }, - { id: 'sonnet', label: 'Sonnet (latest)' }, - { id: 'claude-disk', label: 'Disk Model' }, - ]) + expect(modelsFor('claude')).toEqual([{ id: 'claude-disk', label: 'Disk Model' }]) expect(second.lastRefreshAt).toBe(cached.at) }) + it('never persists live probe output as registry data', async () => { + const dir = await mkdtemp(join(tmpdir(), 'ari-catalog-')) + const cachePath = join(dir, 'cache', 'models.json') + const service = new CatalogService({ + fetchImpl: vi.fn().mockResolvedValue( + registryResponse({ + openai: { models: { 'gpt-vendor': { id: 'gpt-vendor', name: 'Vendor GPT' } } }, + }), + ) as unknown as typeof fetch, + cachePath, + probeModels: vi.fn().mockResolvedValue([{ id: 'probe-only', label: 'From the harness' }]), + probeKinds: ['codex'], + }) + + await service.refresh() + // Second round rewrites the cache while the live probe result is the + // active catalog — the write must not pick that up. + await service.refresh() + expect(catalogSource('codex')).toBe('live') + + const cached = JSON.parse(await readFile(cachePath, 'utf8')) as { + providers: Record + } + // The cache is models.dev's answer. Replaying a harness's own model list + // as vendor data let a probe from one session outlive it as `cache`. + expect(cached.providers['openai']?.map((model) => model.id)).toEqual(['gpt-vendor']) + }) + + it('writes only the providers a round actually carried', async () => { + const dir = await mkdtemp(join(tmpdir(), 'ari-catalog-')) + const cachePath = join(dir, 'cache', 'models.json') + const service = new CatalogService({ + fetchImpl: vi.fn().mockResolvedValue( + registryResponse({ + anthropic: { models: {} }, + openai: { models: { 'gpt-6-astra': { id: 'gpt-6-astra', name: 'GPT-6 Astra' } } }, + }), + ) as unknown as typeof fetch, + cachePath, + }) + + await service.refresh() + + const cached = JSON.parse(await readFile(cachePath, 'utf8')) as { + providers: Record + } + // An anthropic-less round must leave the key absent rather than freezing + // whatever the picker happened to be serving (snapshot or probe output). + expect(cached.providers['anthropic']).toBeUndefined() + expect(cached.providers['openai']?.map((model) => model.id)).toEqual(['gpt-6-astra']) + }) + + it('does not re-probe a kind that already told us its own models', async () => { + vi.useFakeTimers() + const probeModels = vi.fn().mockResolvedValue([{ id: 'live-model', label: 'Live' }]) + const service = new CatalogService({ + fetchImpl: vi.fn().mockRejectedValue(new Error('offline')) as unknown as typeof fetch, + probeModels, + probeKinds: ['codex'], + }) + await service.refresh() + expect(probeModels).toHaveBeenCalledTimes(1) + + // Well past the 60s retry throttle, but a live catalog is the agent's own + // word for a vocabulary that belongs to the installed CLI. Re-asking on + // every picker open spawned an agent process per kind per minute to learn + // the same list again. + await vi.advanceTimersByTimeAsync(10 * 60_000) + await service.refresh() + expect(probeModels).toHaveBeenCalledTimes(1) + + // Past the window, ask again: an upgraded CLI can advertise new models. + await vi.advanceTimersByTimeAsync(REFRESH_TTL_MS) + await service.refresh() + expect(probeModels).toHaveBeenCalledTimes(2) + }) + + it('keeps re-probing a kind that never managed to report', async () => { + vi.useFakeTimers() + const probeModels = vi.fn().mockRejectedValue(new Error('adapter not cached')) + const service = new CatalogService({ + fetchImpl: vi.fn().mockRejectedValue(new Error('offline')) as unknown as typeof fetch, + probeModels, + probeKinds: ['codex'], + }) + await service.refresh() + await vi.advanceTimersByTimeAsync(10 * 60_000) + await service.refresh() + // A kind still on the snapshot has everything to gain from a retry, so the + // live-catalog window must not hold it back. + expect(probeModels).toHaveBeenCalledTimes(2) + }) + it('shares one in-flight refresh across concurrent callers', async () => { const fetchImpl = vi .fn() diff --git a/packages/providers/src/catalog-service.ts b/packages/providers/src/catalog-service.ts index ea1b1202..705ec9d0 100644 --- a/packages/providers/src/catalog-service.ts +++ b/packages/providers/src/catalog-service.ts @@ -2,7 +2,7 @@ import { mkdir, readFile, writeFile } from 'node:fs/promises' import { dirname } from 'node:path' import type { DriverKind } from '@ari/contracts/common' import { createLogger } from '@ari/shared/logger' -import { catalogSource, CLAUDE_ALIASES, modelsFor, setDynamicModels } from './catalogs' +import { catalogSetAt, catalogSource, setDynamicModels } from './catalogs' import type { CatalogModel } from './catalogs' const log = createLogger('providers:catalog') @@ -24,6 +24,13 @@ export const DEFAULT_REGISTRY_URL = 'https://models.dev/api.json' /** How long a fetched registry payload stays fresh. */ export const REFRESH_TTL_MS = 6 * 60 * 60 * 1000 +/** + * How long an agent's own model list is trusted before it is worth asking + * again. Deliberately far longer than the retry throttle: re-probing spawns + * the CLI to hear a vocabulary that only changes when the CLI is upgraded. + */ +export const LIVE_RECHECK_MS = 6 * 60 * 60 * 1000 + interface RegistryModel { id?: string name?: string @@ -85,6 +92,9 @@ function toCatalogModels(kind: DriverKind, models: Record catalog: { id, label: typeof model.name === 'string' && model.name.length > 0 ? model.name : id, + // Carried so the picker can tell a version-less pointer from a real + // sibling (`gpt-5.6` vs `gpt-5.6-sol`), which names alone cannot. + family: model.family ?? id, ...(context !== undefined && context > 0 ? { contextHint: `${Math.round(context / 1000)}k` } : {}), @@ -106,11 +116,10 @@ function toCatalogModels(kind: DriverKind, models: Record return true }) .slice(0, 12) - // Aliases ride along with a real catalog, never replace one: returning - // them alone would make a payload with no usable anthropic rows look - // non-empty and clobber the richer snapshot the caller falls back to. - if (current.length === 0) return [] - return [...CLAUDE_ALIASES, ...current.map((candidate) => candidate.catalog)] + // Rows only: version-less family ids ride as aliases on the newest member + // (see collapseCatalog). An anthropic-less round still yields [] so the + // caller keeps the richer snapshot rather than an empty picker. + return current.map((candidate) => candidate.catalog) } if (kind === 'codex') { @@ -208,25 +217,25 @@ export class CatalogService { }) if (!response.ok) throw new Error(`HTTP ${response.status}`) const body = (await response.json()) as RegistryPayload - let applied = 0 + // What this round actually got from the registry, kept separately from + // the dynamic overlay: the disk cache must hold the registry's answer, + // never the merged catalog the picker is currently serving. + const derived = new Map() for (const [kind, providerId] of Object.entries(REGISTRY_PROVIDER)) { const models = toCatalogModels(kind as DriverKind, body[providerId]?.models ?? {}) if (models.length === 0) continue + derived.set(kind as DriverKind, models) if (catalogSource(kind as DriverKind) !== 'live') { setDynamicModels(kind as DriverKind, 'cache', models) } - applied++ } this.#lastRefreshAt = Date.now() - log.info('registry catalog refreshed', { providers: applied, url: this.#registryUrl }) + log.info('registry catalog refreshed', { providers: derived.size, url: this.#registryUrl }) await this.#writeDiskCache({ at: Date.now(), url: this.#registryUrl, providers: Object.fromEntries( - Object.entries(REGISTRY_PROVIDER).map(([kind, providerId]) => [ - providerId, - modelsFor(kind as DriverKind), - ]), + [...derived].map(([kind, models]) => [REGISTRY_PROVIDER[kind] as string, models]), ), }) } catch (error) { @@ -242,6 +251,12 @@ export class CatalogService { if (this.#probeModels === null || this.#probeKinds.length === 0) return await Promise.all( this.#probeKinds.map(async (kind) => { + // A live catalog is the agent's own word, and the vocabulary belongs to + // the installed CLI rather than to the moment. Re-probing spawns that + // agent again to hear the same list, so a kind that has already + // answered waits out the window; one still on a fallback has + // everything to gain from a retry and is never held back. + if (this.#probeIsFresh(kind)) return try { const models = await this.#probeModels!(kind) if (models !== null && models.length > 0) { @@ -255,6 +270,12 @@ export class CatalogService { ) } + #probeIsFresh(kind: DriverKind): boolean { + if (catalogSource(kind) !== 'live') return false + const at = catalogSetAt(kind) + return at !== null && Date.now() - at < LIVE_RECHECK_MS + } + async #loadDiskCache(): Promise { if (this.#cachePath === null || this.lastRefreshAt > 0) return try { diff --git a/packages/providers/src/catalogs.test.ts b/packages/providers/src/catalogs.test.ts index f5b5365a..c04f90ac 100644 --- a/packages/providers/src/catalogs.test.ts +++ b/packages/providers/src/catalogs.test.ts @@ -5,6 +5,7 @@ import { clearDynamicEfforts, clearDynamicModels, clearDynamicModes, + collapseCatalog, effortsFor, MODEL_CATALOGS, modelsFor, @@ -103,10 +104,14 @@ describe('modelsFor fallback chain', () => { expect(codex.some((id) => id.startsWith('o1') || id.startsWith('o3'))).toBe(false) const claude = modelsFor('claude').map((m) => m.id) - expect(claude).toContain('fable') expect(claude).toContain('claude-fable-5-1') expect(claude).toContain('claude-opus-5') expect(claude).not.toContain('claude-opus-4-5-20251101') + // Version-less family pointers are metadata now, not rows of their own: + // a standalone `fable` row is what duplicated "Fable (latest)" beside + // "Claude Fable 5.1" in the picker. + expect(claude).not.toContain('fable') + expect(claude).not.toContain('opus') const grok = modelsFor('grok').map((m) => m.id) expect(grok).toContain('grok-4.6') @@ -114,6 +119,97 @@ describe('modelsFor fallback chain', () => { }) }) +describe('collapseCatalog', () => { + it('folds a version-less family alias onto the family freshest row', () => { + const models = collapseCatalog('claude', [ + { id: 'claude-opus-5', label: 'Claude Opus 5' }, + { id: 'claude-opus-4-8', label: 'Claude Opus 4.8' }, + ]) + expect(models.map((m) => m.id)).toEqual(['claude-opus-5', 'claude-opus-4-8']) + expect(models[0]?.aliases).toEqual(['opus']) + }) + + it('marks every superseded member of a family as legacy', () => { + const models = collapseCatalog('claude', [ + { id: 'claude-opus-5', label: 'Claude Opus 5' }, + { id: 'claude-opus-4-8', label: 'Claude Opus 4.8' }, + ]) + expect(models[0]?.isLegacy).toBeUndefined() + expect(models[1]?.isLegacy).toBe(true) + }) + + it('collapses two rows sharing a display name into one', () => { + const models = collapseCatalog('claude', [ + { id: 'claude-haiku-4-5-latest', label: 'Claude Haiku 4.5' }, + { id: 'claude-haiku-4-5', label: 'Claude Haiku 4.5' }, + ]) + expect(models).toHaveLength(1) + expect(models[0]?.id).toBe('claude-haiku-4-5-latest') + // The dropped id still resolves to the surviving row. + expect(models[0]?.aliases).toEqual(['claude-haiku-4-5']) + }) + + it('folds a version-less family pointer onto its dated sibling', () => { + // models.dev lists both `claude-haiku-4-5` (named "Claude Haiku 4.5 + // (latest)") and its dated release under one family; the two names differ + // only by the suffix, so label dedupe cannot see that they are one model. + const models = collapseCatalog('claude', [ + { id: 'claude-haiku-4-5-20251001', label: 'Claude Haiku 4.5', family: 'claude-haiku' }, + { id: 'claude-haiku-4-5', label: 'Claude Haiku 4.5 (latest)', family: 'claude-haiku' }, + ]) + expect(models).toHaveLength(1) + expect(models[0]?.id).toBe('claude-haiku-4-5-20251001') + expect(models[0]?.label).toBe('Claude Haiku 4.5') + expect(models[0]?.aliases).toEqual(['claude-haiku-4-5']) + }) + + it('folds a bare family id onto its named sibling', () => { + const models = collapseCatalog('codex', [ + { id: 'gpt-5.6-sol', label: 'GPT-5.6 Sol', family: 'gpt-sol' }, + { id: 'gpt-5.6', label: 'GPT-5.6', family: 'gpt-sol' }, + ]) + expect(models).toHaveLength(1) + expect(models[0]?.id).toBe('gpt-5.6-sol') + expect(models[0]?.aliases).toEqual(['gpt-5.6']) + }) + + it('keeps same-prefixed ids from different families apart', () => { + // `gpt-5.5` is family `gpt`, `gpt-5.5-pro` is family `gpt-pro`: the + // prefix alone would wrongly fold a genuinely different model away. + const models = collapseCatalog('codex', [ + { id: 'gpt-5.5-pro', label: 'GPT-5.5 Pro', family: 'gpt-pro' }, + { id: 'gpt-5.5', label: 'GPT-5.5', family: 'gpt' }, + ]) + expect(models.map((m) => m.id)).toEqual(['gpt-5.5-pro', 'gpt-5.5']) + }) + + it('marks superseded members of a registry family as legacy', () => { + const models = collapseCatalog('claude', [ + { id: 'claude-opus-5', label: 'Claude Opus 5', family: 'claude-opus' }, + { id: 'claude-opus-4-8', label: 'Claude Opus 4.8', family: 'claude-opus' }, + ]) + expect(models[0]?.isLegacy).toBeUndefined() + expect(models[1]?.isLegacy).toBe(true) + }) + + it('leaves kinds with no declared families untouched', () => { + const models = collapseCatalog('codex', [ + { id: 'gpt-5.6-terra', label: 'GPT-5.6 Terra' }, + { id: 'gpt-5.6-luna', label: 'GPT-5.6 Luna' }, + ]) + expect(models).toEqual([ + { id: 'gpt-5.6-terra', label: 'GPT-5.6 Terra' }, + { id: 'gpt-5.6-luna', label: 'GPT-5.6 Luna' }, + ]) + }) + + it('does not mutate the caller list', () => { + const input = [{ id: 'claude-opus-5', label: 'Claude Opus 5' }] + collapseCatalog('claude', input) + expect(input[0]).toEqual({ id: 'claude-opus-5', label: 'Claude Opus 5' }) + }) +}) + describe('effortsFor', () => { it('ships Grok and Ari Core vocabularies before any probe', () => { expect(effortsFor('grok').options.map((o) => o.id)).toEqual([ diff --git a/packages/providers/src/catalogs.ts b/packages/providers/src/catalogs.ts index 752b1a78..b825fdb9 100644 --- a/packages/providers/src/catalogs.ts +++ b/packages/providers/src/catalogs.ts @@ -12,6 +12,22 @@ export interface CatalogModel { label: string /** Short context-window hint rendered beside the label, e.g. `200k`. */ contextHint?: string + /** + * Other ids that resolve to this same model. Carries the version-less + * family pointers a CLI accepts (`opus` for `claude-opus-5`) and the ids of + * rows folded away as duplicate display names, so a session saved against + * any of them still finds this row. + */ + aliases?: string[] + /** Superseded by a newer model in its family; the picker collapses these. */ + isLegacy?: boolean + /** + * Vendor family grouping (`claude-opus`, `gpt-sol`), as the registry reports + * it. Rows that share one are the same model line, which is what lets + * {@link collapseCatalog} tell a version-less pointer from a real sibling. + * Absent on catalogs that predate it; those fall back to id prefixes. + */ + family?: string } /** Where the current catalog for a kind came from. */ @@ -20,16 +36,23 @@ export type CatalogSource = 'live' | 'cache' | 'snapshot' | 'static' const CLI_DEFAULT_MODELS: CatalogModel[] = [{ id: 'default', label: 'CLI default' }] /** - * Version-less ids the Claude CLI resolves to the newest model of each family. - * Additive rows on top of a real catalog, never a catalog on their own: both - * consumers prepend them only once real models are in hand, so an alias-only - * answer can never masquerade as a discovered catalog. + * Id prefixes that name a model family, mapped to the version-less id the CLI + * resolves to that family's newest member. These are not separate choices — a + * user picking "Opus (latest)" and one picking "Claude Opus 5" mean the same + * model — so they ride as {@link CatalogModel.aliases} on the family's newest + * row instead of appearing as rows of their own, which is what put + * "Fable (latest)" directly above "Claude Fable 5.1". + * + * Only kinds whose CLI has version-less ids appear here; everything else is + * left exactly as the source reported it. */ -export const CLAUDE_ALIASES: CatalogModel[] = [ - { id: 'fable', label: 'Fable (latest)' }, - { id: 'opus', label: 'Opus (latest)' }, - { id: 'sonnet', label: 'Sonnet (latest)' }, -] +const FAMILY_ALIASES: Partial>> = { + claude: { + 'claude-fable': 'fable', + 'claude-opus': 'opus', + 'claude-sonnet': 'sonnet', + }, +} /** * Last-resort static catalogs (M4.14). Used only when neither a live refresh @@ -69,7 +92,7 @@ const SNAPSHOT = snapshot as { * refreshes + ACP model probes). Renderer-safe: the module is pure until a * host process calls {@link setDynamicModels}. */ -const dynamic = new Map() +const dynamic = new Map() const dynamicEfforts = new Map() /** @@ -83,7 +106,17 @@ export function setDynamicModels( models: CatalogModel[], ): void { if (models.length === 0) return - dynamic.set(kind, { source, models }) + dynamic.set(kind, { source, models, at: Date.now() }) +} + +/** + * When a kind's current catalog was installed, or null when it is not a + * dynamic one. Callers use this to budget re-discovery: a list an agent + * reported itself is worth trusting for a while, and re-asking means spawning + * that agent again to hear the same answer. + */ +export function catalogSetAt(kind: DriverKind): number | null { + return dynamic.get(kind)?.at ?? null } /** Removes any dynamic overlay for a kind (tests, invalidation). */ @@ -201,8 +234,7 @@ function snapshotFor(kind: DriverKind): CatalogModel[] | null { const providerId = SNAPSHOT_PROVIDER[kind] const models = providerId !== undefined ? (SNAPSHOT.providers[providerId] ?? null) : null if (models === null || models.length === 0) return null - const curated = curateToCurrentModels(kind, models) ?? models - return kind === 'claude' ? [...CLAUDE_ALIASES, ...curated] : curated + return curateToCurrentModels(kind, models) ?? models } /** @@ -217,12 +249,143 @@ function curateToCurrentModels(kind: DriverKind, models: CatalogModel[]): Catalo return current.length > 0 ? current : null } +/** + * Reduces a raw model list to one row per real choice, ready for a picker. + * + * Callers pass catalogs newest-first, which is what makes this cheap: + * + * 1. **Duplicate display names collapse.** models.dev lists dated variants + * under one name ("Claude Haiku 4.5" twice), and two rows a user cannot + * tell apart are one row. The newest survives and the other id becomes an + * alias, so a session saved against it still resolves. + * 2. **Version-less family pointers become metadata.** `opus` is not a + * fifteenth model, it is another name for the newest Opus — see + * {@link FAMILY_ALIASES}. + * 3. **Superseded family members are flagged legacy**, not dropped: the + * picker collapses them behind a disclosure, so pinning an older version + * stays possible without them competing with the current ones. + * + * Returns new objects; the caller's list is never mutated. + */ +export function collapseCatalog(kind: DriverKind, models: CatalogModel[]): CatalogModel[] { + const families = FAMILY_ALIASES[kind] + const pointers = familyPointers(models) + const byLabel = new Map() + const kept: CatalogModel[] = [] + for (const model of models) { + // A pointer is not a choice of its own; its id lands on the row below. + if (pointers.ids.has(model.id)) continue + const existing = byLabel.get(model.label) + if (existing !== undefined) { + existing.aliases = mergeAliases(existing.aliases, [model.id, ...(model.aliases ?? [])]) + continue + } + const next: CatalogModel = { ...model } + const family = familyKeyOf(next, families) + const alias = family !== undefined ? families?.[family] : undefined + const aliases = mergeAliases(next.aliases, [ + ...(pointers.targets.get(next.id) ?? []), + ...(alias !== undefined ? [alias] : []), + ]) + if (aliases.length > 0) next.aliases = aliases + else delete next.aliases + byLabel.set(next.label, next) + kept.push(next) + } + return markSuperseded(kept, families).map(withoutFamily) +} + +/** + * Drops the grouping hint from a finished picker row. `family` is how the + * collapse tells a version-less pointer from a real sibling; it is an input + * to that decision, not something a picker row carries. + */ +function withoutFamily(model: CatalogModel): CatalogModel { + const row = { ...model } + delete row.family + return row +} + +interface FamilyPointers { + /** Every id that stands in for another row. */ + ids: Set + /** The concrete row id each pointer folds onto. */ + targets: Map +} + +/** + * Finds the version-less ids that stand in for concrete ones. models.dev + * lists both `claude-haiku-4-5` and `claude-haiku-4-5-20251001` under one + * family, and `gpt-5.6` beside `gpt-5.6-sol`; the shorter id is the floating + * pointer, and a display name differing only by a "(latest)" suffix is not + * enough for the label check to pair them up. + * + * Same family is required, which is what stops `gpt-5.5` from swallowing + * `gpt-5.5-pro` — a genuinely different model line that merely shares a prefix. + */ +function familyPointers(models: CatalogModel[]): FamilyPointers { + const ids = new Set() + const targets = new Map() + for (const pointer of models) { + for (const concrete of models) { + if (concrete.id === pointer.id) continue + if (pointer.family === undefined || concrete.family !== pointer.family) continue + if (!concrete.id.startsWith(pointer.id)) continue + const folded = targets.get(concrete.id) ?? [] + if (!folded.includes(pointer.id)) folded.push(pointer.id) + targets.set(concrete.id, folded) + ids.add(pointer.id) + } + } + return { ids, targets } +} + +/** The registry's family for a model, or the declared alias prefix without one. */ +function familyKeyOf( + model: CatalogModel, + families: Record | undefined, +): string | undefined { + if (model.family !== undefined) return model.family + if (families === undefined) return undefined + return Object.keys(families).find((prefix) => model.id.startsWith(prefix)) +} + +/** Flags every family member after the first — the list is newest-first. */ +function markSuperseded( + models: CatalogModel[], + families: Record | undefined, +): CatalogModel[] { + if (families === undefined) return models + const seen = new Set() + for (const model of models) { + const family = familyKeyOf(model, families) + if (family === undefined) continue + // First occurrence is the family's newest member: the row to offer. + if (seen.has(family)) model.isLegacy = true + else seen.add(family) + } + return models +} + +function mergeAliases(current: string[] | undefined, added: readonly string[]): string[] { + const merged = current === undefined ? [] : [...current] + for (const id of added) { + if (id.length > 0 && !merged.includes(id)) merged.push(id) + } + return merged +} + /** * Model catalog entries for a driver's picker, merged in priority order: * live provider data → cached refresh → bundled snapshot → static defaults. * Synchronous and renderer-safe; dynamic overlays arrive via * {@link setDynamicModels} in the main process. + * + * Collapsing happens here, at read time, so every source — probe, cache, + * snapshot or static — gets the same one-row-per-choice treatment and the + * stored catalogs stay exactly as their source reported them. */ export function modelsFor(kind: DriverKind): CatalogModel[] { - return dynamic.get(kind)?.models ?? snapshotFor(kind) ?? MODEL_CATALOGS[kind] + const models = dynamic.get(kind)?.models ?? snapshotFor(kind) ?? MODEL_CATALOGS[kind] + return collapseCatalog(kind, models) } diff --git a/packages/providers/src/detector.test.ts b/packages/providers/src/detector.test.ts index a852ac5f..15686a90 100644 --- a/packages/providers/src/detector.test.ts +++ b/packages/providers/src/detector.test.ts @@ -1,6 +1,6 @@ import { mkdtemp, mkdir, rm } from 'node:fs/promises' import { tmpdir } from 'node:os' -import { join } from 'node:path' +import { delimiter, join } from 'node:path' import { afterEach, beforeEach, describe, expect, it } from 'vitest' import { findBinary, detectDriver, readAuthStatus, wellKnownDirs } from './detector' import type { DetectEnvironment } from './types' @@ -54,6 +54,27 @@ describe('findBinary', () => { expect(findBinary('ari-core', makeEnv())).toBeNull() }) + it('ignores a directory named like the binary', async () => { + // A project folder called `codex` on PATH used to be reported as an + // installed CLI, which put a fully-populated Codex rail in the picker. + const binDir = join(dir, 'phantom') + await mkdir(join(binDir, 'codex'), { recursive: true }) + expect(findBinary('codex', { ...makeEnv(), pathEnv: binDir })).toBeNull() + }) + + it('prefers a real file over a same-named directory earlier on PATH', async () => { + const early = join(dir, 'early') + const later = join(dir, 'later') + await mkdir(join(early, 'codex'), { recursive: true }) + await mkdir(later, { recursive: true }) + await writeFileSafe(join(later, 'codex'), '') + const env: DetectEnvironment = { + ...makeEnv(), + pathEnv: [early, later].join(delimiter), + } + expect(findBinary('codex', env)).toBe(join(later, 'codex')) + }) + it('skips nonexistent well-known dirs without throwing', () => { const env: DetectEnvironment = { ...makeEnv(), homeDir: join(dir, 'nope') } const dirs = wellKnownDirs({ ...env, platform: 'linux' }) @@ -169,6 +190,15 @@ describe('detectDriver', () => { expect(detection.authReason).toBeTruthy() }) + it('does not report a directory named like a binary as installed', async () => { + const binDir = join(dir, 'phantom-driver') + await mkdir(join(binDir, 'codex'), { recursive: true }) + const detection = await detectDriver('codex', { ...makeEnv(), pathEnv: binDir }) + expect(detection.installed).toBe(false) + expect(detection.binaryPath).toBeNull() + expect(detection.authStatus).toBe('unknown') + }) + it('treats ari-core as installed and authenticated', async () => { const detection = await detectDriver('ari-core', makeEnv()) expect(detection.installed).toBe(true) diff --git a/packages/providers/src/detector.ts b/packages/providers/src/detector.ts index b6a23849..447cf991 100644 --- a/packages/providers/src/detector.ts +++ b/packages/providers/src/detector.ts @@ -1,4 +1,4 @@ -import { existsSync } from 'node:fs' +import { existsSync, statSync } from 'node:fs' import { delimiter, join } from 'node:path' import { spawn } from 'node:child_process' import type { DriverKind } from '@ari/contracts/common' @@ -44,6 +44,11 @@ export function wellKnownDirs(env: DetectEnvironment): string[] { return dirs.filter((d) => d.length > 0 && existsSync(d)) } +/** True when `path` names a regular file — not a directory, socket, or link to one. */ +function isRegularFile(path: string): boolean { + return statSync(path, { throwIfNoEntry: false })?.isFile() ?? false +} + /** Resolves a binary across PATH plus platform-specific install dirs. */ export function findBinary(kind: DriverKind, env: DetectEnvironment): string | null { if (kind === 'ari-core') return null @@ -55,8 +60,10 @@ export function findBinary(kind: DriverKind, env: DetectEnvironment): string | n for (const dir of searchDirs) { for (const name of names) { const candidate = join(dir, name) - // existsSync on a file also rejects directories named like the binary. - if (existsSync(candidate)) return candidate + // isFile, not existsSync: a *directory* named `codex` (a checked-out + // repo, a scratch folder) is not a CLI, and treating it as one put a + // fully-populated provider row in front of users who never installed it. + if (isRegularFile(candidate)) return candidate } } return null diff --git a/scripts/update-model-snapshot.ts b/scripts/update-model-snapshot.ts index 4f38459b..d5f8faa7 100644 --- a/scripts/update-model-snapshot.ts +++ b/scripts/update-model-snapshot.ts @@ -28,6 +28,7 @@ const EXCLUDED_ID = /embedding|whisper|tts|dall-e|image|video|audio|transcribe|m interface ModelsDevModel { id?: string name?: string + family?: string release_date?: string modalities?: { input?: string[]; output?: string[] } limit?: { context?: number } @@ -41,6 +42,8 @@ interface SnapshotEntry { id: string label: string contextHint?: string + /** Vendor family; lets the picker fold version-less pointers onto siblings. */ + family?: string } interface Snapshot { @@ -64,6 +67,7 @@ function toEntry(id: string, model: ModelsDevModel): SnapshotEntry | null { id, label: typeof model.name === 'string' && model.name.length > 0 ? model.name : id, } + if (typeof model.family === 'string' && model.family.length > 0) entry.family = model.family const hint = contextHint(model.limit?.context) if (hint !== undefined) entry.contextHint = hint return entry From 203fbe9a7bbc8571224f2eb052c74df6ef0608ef Mon Sep 17 00:00:00 2001 From: t Date: Sun, 13 Sep 2026 15:16:43 +0800 Subject: [PATCH 3/7] test(desktop): derive the music runtime key from the runtime itself The self-heal suite seeded a `win32-x64` / `linux-x64` key from a win32-vs-not branch, so on macOS it wrote a manifest under a key `MusicRuntime` never looks up. The runtime then found no binary and every test in the block returned RUNTIME_MISSING instead of the failure it meant to exercise. Derive the key from `musicRuntimeTargetKey()` and fail loudly on a platform the runtime does not support, so the fixture cannot drift from the lookup again. Unrelated to the model-catalog work in the previous commit; it was sitting uncommitted on this branch. Co-Authored-By: Claude --- apps/desktop/src/main/music-engine.test.ts | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/apps/desktop/src/main/music-engine.test.ts b/apps/desktop/src/main/music-engine.test.ts index 816663d5..816a7ef0 100644 --- a/apps/desktop/src/main/music-engine.test.ts +++ b/apps/desktop/src/main/music-engine.test.ts @@ -4,7 +4,7 @@ import { tmpdir } from 'node:os' import { join } from 'node:path' import { describe, expect, it, vi } from 'vitest' import { MusicEngine, classifyHelperError, errorCodeOf, videoIdFromUrl } from './music-engine' -import { MusicRuntime } from './music-runtime' +import { MusicRuntime, musicRuntimeTargetKey } from './music-runtime' const TRACK_URL = 'https://www.youtube.com/watch?v=abc123' @@ -185,7 +185,14 @@ describe('MusicEngine streaming', () => { describe('MusicEngine runtime self-heal', () => { const REMOTE_URL = 'https://example.invalid/music-runtime.json' - const KEY = process.platform === 'win32' ? 'win32-x64' : 'linux-x64' + // Must be the key MusicRuntime actually looks up, which is `${platform}-${arch}` + // including darwin and arm64. A win32/not-win32 branch seeded a linux key on + // macOS runners, so the runtime found no binary and every test here returned + // RUNTIME_MISSING instead of the failure it meant to exercise. + const unsupported = (): never => { + throw new Error(`unsupported test platform: ${process.platform}-${process.arch}`) + } + const KEY = musicRuntimeTargetKey() ?? unsupported() const BINARY = process.platform === 'win32' ? 'yt-dlp.exe' : 'yt-dlp' const V1 = Buffer.from('v1-bytes') const V1_SHA = createHash('sha256').update(V1).digest('hex') From 2f92bb62b87b70e02612fb311a2435d322e7931f Mon Sep 17 00:00:00 2001 From: t Date: Sun, 13 Sep 2026 15:34:06 +0800 Subject: [PATCH 4/7] fix(desktop): remove the socket directory when startup fails MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `controlEndpoint` creates the private /tmp/ari-* directory before the launcher write, the workspace setup and `server.listen()`, but the only removal of it lives in the `close` method of the object a successful start returns. Any rejection in between therefore left the directory behind — and because the same startup failure repeats on every launch, they accumulate one per attempt. Split the builder out and remove the directory when it rejects. `close` still takes ownership once the builder resolves, so the happy path is unchanged. Co-Authored-By: Claude --- apps/desktop/src/main/agent-runtime.test.ts | 42 ++++++++++++++++++++- apps/desktop/src/main/agent-runtime.ts | 19 +++++++++- 2 files changed, 59 insertions(+), 2 deletions(-) diff --git a/apps/desktop/src/main/agent-runtime.test.ts b/apps/desktop/src/main/agent-runtime.test.ts index 76fcc0dc..fe797bc3 100644 --- a/apps/desktop/src/main/agent-runtime.test.ts +++ b/apps/desktop/src/main/agent-runtime.test.ts @@ -2,7 +2,7 @@ import { execFile, execFileSync } from 'node:child_process' import { promisify } from 'node:util' import { randomUUID } from 'node:crypto' -import { mkdtemp, mkdir, readFile, rm, writeFile } from 'node:fs/promises' +import { mkdtemp, mkdir, readdir, readFile, rm, writeFile } from 'node:fs/promises' import { tmpdir } from 'node:os' import { join, resolve } from 'node:path' import { describe, expect, it } from 'vitest' @@ -44,6 +44,46 @@ describe('control endpoint', () => { }) }) +/** Socket directories the runtime owns under the shipped root. */ +async function socketDirs(): Promise { + return (await readdir(SOCKET_ROOT)).filter((name) => name.startsWith('ari-')).sort() +} + +// `close` only exists on the object a successful start returns, so a startup +// that rejected after creating the directory had nothing left to remove it — +// one directory per attempt, for a failure that repeats on every launch. +it('removes the socket directory when startup fails after creating it', async () => { + const dir = await mkdtemp(join(tmpdir(), 'ari-startup-fail-')) + const bin = join(dir, 'agent-control', 'bin') + await mkdir(bin, { recursive: true }) + // A directory where the launcher file belongs, so the write — which runs + // after the socket directory exists — is the step that rejects. + await mkdir(join(bin, process.platform === 'win32' ? 'ari.cmd' : 'ari')) + const store = new SessionStore({ rootDir: join(dir, 'sessions') }) + const engine = new Engine({ + store, + registry: new DriverRegistry(), + publish: () => undefined, + resolveWorkspace: async () => dir, + git: { captureCheckpoint: async () => ({ ok: true, value: null }) }, + }) + const before = await socketDirs() + await expect( + startAgentRuntime({ + engine, + store, + userData: dir, + cliPath, + executable: process.execPath, + version: 'test', + policy: () => delegationSettingsSchema.parse({ approvalMode: 'never' }), + providers: async () => [], + }), + ).rejects.toThrow() + expect(await socketDirs()).toEqual(before) + await rm(dir, { recursive: true, force: true }) +}) + it('runs the shipped CLI through scoped transport, a real isolated worker and exact integration', async () => { const dir = await mkdtemp(join(tmpdir(), 'ari-e2e-')) const repo = join(dir, 'repo space') diff --git a/apps/desktop/src/main/agent-runtime.ts b/apps/desktop/src/main/agent-runtime.ts index 585c7bf2..d6034c93 100644 --- a/apps/desktop/src/main/agent-runtime.ts +++ b/apps/desktop/src/main/agent-runtime.ts @@ -65,11 +65,28 @@ export async function controlEndpoint( /** Hosts the transport and injects private CLI launchers without changing global PATH. */ export async function startAgentRuntime(options: AgentRuntimeOptions) { + const { endpoint, dir: socketDir } = await controlEndpoint(process.platform, SOCKET_ROOT) + try { + return await startControlTransport(options, endpoint, socketDir) + } catch (error) { + // `close` only exists on the object a successful start returns, so a + // rejection after this point leaves nothing able to remove the directory — + // and because the same failure repeats on every launch, they accumulate. + if (socketDir) await rm(socketDir, { recursive: true, force: true }) + throw error + } +} + +/** Builds what the endpoint serves; `close` takes ownership of `socketDir` once this resolves. */ +async function startControlTransport( + options: AgentRuntimeOptions, + endpoint: string, + socketDir: string | null, +) { const { engine, store } = options const runtimeDir = join(options.userData, 'agent-control') const bin = join(runtimeDir, 'bin') await mkdir(bin, { recursive: true, mode: 0o700 }) - const { endpoint, dir: socketDir } = await controlEndpoint(process.platform, SOCKET_ROOT) const launcher = join(bin, process.platform === 'win32' ? 'ari.cmd' : 'ari') const quote = (value: string) => `'${value.replaceAll("'", "'\\''")}'` const script = From 38b9b463675e95962a6818f2f8cebcea353049d4 Mon Sep 17 00:00:00 2001 From: t Date: Sun, 13 Sep 2026 15:46:18 +0800 Subject: [PATCH 5/7] fix(providers): keep scanning PATH after an unreadable entry MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `statSync` with `throwIfNoEntry: false` suppresses a missing entry, but an unusable one still throws — a path through a file raises ENOTDIR, a directory that denies traversal raises EACCES. That escaped `findBinary` and aborted the whole scan, so one bad PATH entry hid every provider installed in the directories after it. Treat an unreadable candidate as a miss, which is what the scan already does for a missing one. Co-Authored-By: Claude --- packages/providers/src/detector.test.ts | 17 +++++++++++++++++ packages/providers/src/detector.ts | 10 +++++++++- 2 files changed, 26 insertions(+), 1 deletion(-) diff --git a/packages/providers/src/detector.test.ts b/packages/providers/src/detector.test.ts index 15686a90..d43677dd 100644 --- a/packages/providers/src/detector.test.ts +++ b/packages/providers/src/detector.test.ts @@ -75,6 +75,23 @@ describe('findBinary', () => { expect(findBinary('codex', env)).toBe(join(later, 'codex')) }) + it('skips a PATH entry that cannot be read rather than aborting the scan', async () => { + // A PATH entry that is a file, not a directory: resolving a candidate + // under it raises ENOTDIR instead of reporting that candidate missing, + // so one unusable entry used to abort detection before the directories + // after it were searched — hiding a provider that is installed. + const notADir = join(dir, 'not-a-dir') + await writeFileSafe(notADir, '') + const later = join(dir, 'later-readable') + await mkdir(later, { recursive: true }) + await writeFileSafe(join(later, 'codex'), '') + const env: DetectEnvironment = { + ...makeEnv(), + pathEnv: [notADir, later].join(delimiter), + } + expect(findBinary('codex', env)).toBe(join(later, 'codex')) + }) + it('skips nonexistent well-known dirs without throwing', () => { const env: DetectEnvironment = { ...makeEnv(), homeDir: join(dir, 'nope') } const dirs = wellKnownDirs({ ...env, platform: 'linux' }) diff --git a/packages/providers/src/detector.ts b/packages/providers/src/detector.ts index 447cf991..e9d68c16 100644 --- a/packages/providers/src/detector.ts +++ b/packages/providers/src/detector.ts @@ -46,7 +46,15 @@ export function wellKnownDirs(env: DetectEnvironment): string[] { /** True when `path` names a regular file — not a directory, socket, or link to one. */ function isRegularFile(path: string): boolean { - return statSync(path, { throwIfNoEntry: false })?.isFile() ?? false + try { + return statSync(path, { throwIfNoEntry: false })?.isFile() ?? false + } catch { + // `throwIfNoEntry: false` only covers a missing entry. An unusable one — + // a path through a file, a directory that denies traversal — still + // throws, and letting that escape aborted the whole scan instead of + // moving on to the directories after it. + return false + } } /** Resolves a binary across PATH plus platform-specific install dirs. */ From 03ffba48ca452ba0e2cb14967125f6dc7d80ba7b Mon Sep 17 00:00:00 2001 From: t Date: Sun, 13 Sep 2026 15:46:18 +0800 Subject: [PATCH 6/7] fix(desktop): close the control server when listen fails MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `AgentControlServer.listen` binds before it applies the socket's 0600 mode, so a rejection at that second step leaves the server listening — and because `startControlTransport` never returns, nothing can call the `close` that would release the handle. The listening handle outlives the startup failure for the life of the process. Close it on the way out, without masking the error that caused it. The caller still removes the socket directory. Co-Authored-By: Claude --- apps/desktop/src/main/agent-runtime.ts | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/apps/desktop/src/main/agent-runtime.ts b/apps/desktop/src/main/agent-runtime.ts index d6034c93..c8af3b9e 100644 --- a/apps/desktop/src/main/agent-runtime.ts +++ b/apps/desktop/src/main/agent-runtime.ts @@ -191,7 +191,15 @@ async function startControlTransport( approvals.cancel(id) server.revoke(id) } - await server.listen() + try { + await server.listen() + } catch (error) { + // `listen` binds before it applies the socket mode, so a rejection there + // leaves a listening handle that this function never returns — nothing + // else can close it. The caller still removes the directory. + await server.close().catch(() => undefined) + throw error + } const unsubscribe = store.subscribe((event) => { if (event.type === 'turn.settled') approvals.cancel(event.sessionId) }) From 8ed3e4caad15fd6c07f416cdb15aeddb964a0075 Mon Sep 17 00:00:00 2001 From: t Date: Sun, 13 Sep 2026 16:01:24 +0800 Subject: [PATCH 7/7] ci: keep the required `verify` check on the matrix build MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adding the macOS leg moved this job's check from `verify` to `verify (ubuntu-latest)` / `verify (macos-latest)`. The `protect main` ruleset requires a check named exactly `verify`, and a matrix job cannot produce that name — so every PR would have been permanently blocked the moment this landed on main. Run the matrix under its own name and carry the required one on an aggregator that runs regardless and fails when any leg does. Co-Authored-By: Claude --- .github/workflows/verify.yml | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/.github/workflows/verify.yml b/.github/workflows/verify.yml index 06534c94..5ab266f1 100644 --- a/.github/workflows/verify.yml +++ b/.github/workflows/verify.yml @@ -19,7 +19,11 @@ concurrency: cancel-in-progress: true jobs: - verify: + # The required check on main is named exactly `verify`. A matrix job reports + # per-OS names instead — `verify (ubuntu-latest)` and so on — which never + # satisfies it, so the matrix runs under its own name and the aggregator at + # the bottom of this file carries the stable one. + platform: # macOS is where it breaks first: `sockaddr_un.sun_path` is 104 bytes there # against 108 on Linux, and Electron's userData prefix is far longer, so # platform-specific failures in the control transport only surface here. @@ -48,3 +52,15 @@ jobs: - name: Verify (typecheck + lint + test) run: pnpm verify + + # Required by the `protect main` ruleset. `always()` so it still reports when + # a platform leg fails or is cancelled, and fails in that case rather than + # masking it — the required check has to reflect the whole matrix. + verify: + if: always() + needs: platform + runs-on: ubuntu-latest + steps: + - name: Confirm every platform leg passed + if: needs.platform.result != 'success' + run: exit 1