diff --git a/.github/workflows/verify.yml b/.github/workflows/verify.yml index 81a03bc1..5ab266f1 100644 --- a/.github/workflows/verify.yml +++ b/.github/workflows/verify.yml @@ -19,8 +19,20 @@ concurrency: cancel-in-progress: true jobs: - verify: - runs-on: ubuntu-latest + # 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. + # `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 @@ -40,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 diff --git a/apps/desktop/src/main/agent-runtime.test.ts b/apps/desktop/src/main/agent-runtime.test.ts index 85a27bd0..fe797bc3 100644 --- a/apps/desktop/src/main/agent-runtime.test.ts +++ b/apps/desktop/src/main/agent-runtime.test.ts @@ -1,21 +1,89 @@ // @vitest-environment node import { execFile, execFileSync } from 'node:child_process' import { promisify } from 'node:util' -import { mkdtemp, mkdir, readFile, rm, writeFile } from 'node:fs/promises' +import { randomUUID } from 'node:crypto' +import { mkdtemp, mkdir, readdir, 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) + }) +}) + +/** 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 41601a56..c8af3b9e 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,61 @@ 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 { 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 = - process.platform === 'win32' - ? `\\\\.\\pipe\\ari-${randomUUID()}` - : join(runtimeDir, `${randomUUID()}.sock`) const launcher = join(bin, process.platform === 'win32' ? 'ari.cmd' : 'ari') const quote = (value: string) => `'${value.replaceAll("'", "'\\''")}'` const script = @@ -146,7 +191,15 @@ export async function startAgentRuntime(options: AgentRuntimeOptions) { 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) }) @@ -180,6 +233,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) } 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/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') 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..d43677dd 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,44 @@ 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 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' }) @@ -169,6 +207,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..e9d68c16 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,19 @@ 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 { + 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. */ export function findBinary(kind: DriverKind, env: DetectEnvironment): string | null { if (kind === 'ari-core') return null @@ -55,8 +68,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