From 9dc0d41826008169ed2ecef3d83b31f60d9c553e Mon Sep 17 00:00:00 2001 From: HeavyGee <133152184+heavygee@users.noreply.github.com> Date: Wed, 2 Sep 2026 11:18:08 +0000 Subject: [PATCH 1/4] fix(runner): reject a spawn model missing from the machine's catalog MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Spawning with a model the agent does not have (`--agent cursor --model gpt-5`) booted a child that died at the agent handshake, leaving an archived session with zero turns and no explanation anywhere in the UI. Add a runner preflight next to the agent-availability check: when the machine already has a cached model catalog for the flavor, an id that is not in it fails the spawn with `model_unavailable` and a message naming the accepted ids (near-misses first). Cursor is the only flavor with a catalog readable without a probe today; every other flavor reports an empty catalog, which the validator treats as "unknown" and never rejects — a spawn must not block on `agent --list-models`, which can take 30s and contends with the ACP spawn lease. Fixes #1752 via [HAPI](https://hapi.run) Co-Authored-By: HAPI --- cli/src/modules/common/cursorModels.ts | 37 ++++++++ .../modules/common/cursorModelsSharedCache.ts | 12 ++- cli/src/modules/common/rpcTypes.ts | 2 +- cli/src/runner/run.ts | 15 +++ cli/src/runner/spawnModelPreflight.test.ts | 91 ++++++++++++++++++ cli/src/runner/spawnModelPreflight.ts | 23 +++++ hub/src/sync/rpcGateway.ts | 4 +- shared/src/apiTypes.ts | 2 +- shared/src/index.ts | 1 + shared/src/spawnModelCatalog.test.ts | 77 +++++++++++++++ shared/src/spawnModelCatalog.ts | 93 +++++++++++++++++++ 11 files changed, 352 insertions(+), 5 deletions(-) create mode 100644 cli/src/runner/spawnModelPreflight.test.ts create mode 100644 cli/src/runner/spawnModelPreflight.ts create mode 100644 shared/src/spawnModelCatalog.test.ts create mode 100644 shared/src/spawnModelCatalog.ts diff --git a/cli/src/modules/common/cursorModels.ts b/cli/src/modules/common/cursorModels.ts index 33a17c5e85..68c69b4746 100644 --- a/cli/src/modules/common/cursorModels.ts +++ b/cli/src/modules/common/cursorModels.ts @@ -11,6 +11,7 @@ import { killProcessByChildProcess } from '@/utils/process'; import { getCursorAcpModelsSnapshot } from '@/cursor/utils/cursorAcpModelsBridge'; import { getErrorMessage } from './rpcResponses'; import { + getSharedCursorModelsCacheAgeMs, readSharedCursorModelsCache, writeSharedCursorModelsCache, _resetSharedCursorModelsCacheForTests @@ -162,6 +163,8 @@ interface CacheEntry { } const CACHE_TTL_MS = 60_000; +/** Beyond this, a cached catalog is too old to reject a model id against. */ +const CACHED_CATALOG_MAX_AGE_MS = 24 * 60 * 60 * 1000; const PROBE_TIMEOUT_MS = 30_000; const cache: CacheEntry = { expiresAt: 0, @@ -376,6 +379,40 @@ export async function listCursorModels(): Promise { return inflight; } +/** + * Catalog ids from whatever is already cached (ACP snapshot, in-process cache, + * on-disk shared cache). Never probes: the spawn preflight must not block on + * `agent --list-models`, which can take 30s and contends with the ACP spawn + * lease. Returns [] when nothing usable is cached, which callers must read as + * "unknown catalog", not "no models". + */ +export function getCachedCursorModelIds(): string[] { + const sources: (readonly CursorModelSummary[] | undefined)[] = []; + // Live ACP session snapshot — current by construction. + sources.push(getCursorAcpModelsSnapshot()?.availableModels); + + // The in-memory entry and the file are written together by + // applyInMemoryCache, so the file's mtime dates both. Past the age bound + // both are dropped rather than trusted past their TTL: a runner can stay up + // for weeks, and a catalog that predates a Cursor upgrade would reject an id + // that is now valid. + const sharedAgeMs = getSharedCursorModelsCacheAgeMs(); + if (sharedAgeMs !== null && sharedAgeMs <= CACHED_CATALOG_MAX_AGE_MS) { + sources.push(cache.response.availableModels, cache.response.cliModelSkus); + const shared = readSharedCursorModelsCache(); + sources.push(shared?.availableModels, shared?.cliModelSkus); + } + + const ids = new Set(); + for (const source of sources) { + for (const entry of source ?? []) { + const modelId = entry.modelId.trim(); + if (modelId) ids.add(modelId); + } + } + return [...ids]; +} + export function seedCursorModelsCache(response: ListCursorModelsResponse): void { if ((response.availableModels?.length ?? 0) > 0) { writeSharedCursorModelsCache(response); diff --git a/cli/src/modules/common/cursorModelsSharedCache.ts b/cli/src/modules/common/cursorModelsSharedCache.ts index b7e508497d..c99605d241 100644 --- a/cli/src/modules/common/cursorModelsSharedCache.ts +++ b/cli/src/modules/common/cursorModelsSharedCache.ts @@ -1,4 +1,4 @@ -import { existsSync, mkdirSync, readFileSync, rmSync, writeFileSync } from 'node:fs'; +import { existsSync, mkdirSync, readFileSync, rmSync, statSync, writeFileSync } from 'node:fs'; import { dirname, join } from 'node:path'; import type { CursorModelsResponse } from '@hapi/protocol/apiTypes'; import { resolveHapiHomeDir } from '@/configuration'; @@ -33,6 +33,16 @@ export function readSharedCursorModelsCache(): CursorModelsResponse | null { } } +/** Age of the on-disk catalog in ms, or null when there is no cache file. */ +export function getSharedCursorModelsCacheAgeMs(): number | null { + const path = getSharedCachePath(); + try { + return Math.max(0, Date.now() - statSync(path).mtimeMs); + } catch { + return null; + } +} + export function writeSharedCursorModelsCache(response: CursorModelsResponse): void { if (!isUsableModelsResponse(response)) { return; diff --git a/cli/src/modules/common/rpcTypes.ts b/cli/src/modules/common/rpcTypes.ts index 53ae84f12f..766c62c266 100644 --- a/cli/src/modules/common/rpcTypes.ts +++ b/cli/src/modules/common/rpcTypes.ts @@ -37,6 +37,6 @@ export type SpawnSessionResult = | { type: 'error' errorMessage: string - code?: 'agent_unavailable' | 'outside_workspace_roots' + code?: 'agent_unavailable' | 'model_unavailable' | 'outside_workspace_roots' agent?: AgentFlavor } diff --git a/cli/src/runner/run.ts b/cli/src/runner/run.ts index 245dc398fe..ee0e4cfa57 100644 --- a/cli/src/runner/run.ts +++ b/cli/src/runner/run.ts @@ -29,6 +29,7 @@ import { buildMachineMetadata } from '@/agent/sessionFactory'; import { resolveWorkspaceRoots } from '@/utils/workspaceRoot'; import { hashRunnerCliApiToken, hashRunnerExtraHeaders } from './runnerIdentity'; import { scheduleCursorModelsPrewarm } from '@/modules/common/cursorModelsPrewarm'; +import { checkSpawnModel } from './spawnModelPreflight'; import { isLinkedGitWorktree } from '@/utils/isLinkedGitWorktree'; import { agentUnavailableMessage, getAgentAvailability } from '@/agent/agentAvailability'; @@ -515,6 +516,20 @@ export async function startRunner(options: { workspaceRoots?: string[] } = {}): agent }; } + const modelCheck = checkSpawnModel(agent, options.model); + if (!modelCheck.ok) { + logger.debug(`[RUNNER RUN] Model preflight failed: ${modelCheck.message}`); + reportSpawnOutcomeToHub?.({ + type: 'error', + details: { message: modelCheck.message } + }); + return { + type: 'error', + errorMessage: modelCheck.message, + code: 'model_unavailable', + agent + }; + } if (options.validateDirectory && !(await options.validateDirectory(directory))) { return { type: 'error', diff --git a/cli/src/runner/spawnModelPreflight.test.ts b/cli/src/runner/spawnModelPreflight.test.ts new file mode 100644 index 0000000000..2cba96d9f8 --- /dev/null +++ b/cli/src/runner/spawnModelPreflight.test.ts @@ -0,0 +1,91 @@ +import { afterAll, afterEach, describe, expect, it } from 'vitest'; +import { mkdtempSync, rmSync, utimesSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { setCursorAcpModelsSnapshot } from '@/cursor/utils/cursorAcpModelsBridge'; +import { _resetCursorModelsCacheForTests, seedCursorModelsCache } from '@/modules/common/cursorModels'; +import { checkSpawnModel } from './spawnModelPreflight'; + +// Isolate the on-disk cursor-models cache to this file's own HAPI_HOME so +// parallel vitest workers don't race on the shared $HAPI_HOME/cache path. +const previousHapiHome = process.env.HAPI_HOME; +const testHapiHome = mkdtempSync(join(tmpdir(), 'hapi-spawn-model-preflight-')); +process.env.HAPI_HOME = testHapiHome; + +afterEach(() => { + setCursorAcpModelsSnapshot(null); + _resetCursorModelsCacheForTests(); +}); + +afterAll(() => { + if (previousHapiHome === undefined) delete process.env.HAPI_HOME; + else process.env.HAPI_HOME = previousHapiHome; + rmSync(testHapiHome, { recursive: true, force: true }); +}); + +describe('checkSpawnModel', () => { + it('rejects a Cursor model missing from the cached catalog (#1752)', () => { + setCursorAcpModelsSnapshot({ + availableModels: [{ modelId: 'composer-2.5[thinking]' }, { modelId: 'gpt-5.1[reasoning=high]' }], + currentModelId: 'composer-2.5[thinking]' + }); + + const result = checkSpawnModel('cursor', 'gpt-5'); + + expect(result.ok).toBe(false); + expect(result.ok === false && result.message).toContain("Model 'gpt-5' is not in the cursor model catalog"); + expect(result.ok === false && result.message).toContain('Accepted: gpt-5.1, composer-2.5'); + }); + + it('accepts a model present in the cached catalog', () => { + setCursorAcpModelsSnapshot({ + availableModels: [{ modelId: 'composer-2.5[thinking]' }], + currentModelId: 'composer-2.5[thinking]' + }); + + expect(checkSpawnModel('cursor', 'composer-2.5[thinking]')).toEqual({ ok: true }); + }); + + it('reads the on-disk shared catalog when no ACP snapshot is live', () => { + seedCursorModelsCache({ + success: true, + availableModels: [{ modelId: 'composer-2.5[thinking]' }], + currentModelId: 'composer-2.5[thinking]' + }); + + expect(checkSpawnModel('cursor', 'composer-2.5')).toEqual({ ok: true }); + expect(checkSpawnModel('cursor', 'gpt-5').ok).toBe(false); + }); + + it('ignores a shared catalog older than the freshness bound', () => { + seedCursorModelsCache({ + success: true, + availableModels: [{ modelId: 'composer-2.5[thinking]' }], + currentModelId: 'composer-2.5[thinking]' + }); + // A catalog that predates a Cursor upgrade must not reject an id that is + // now valid; the preflight goes dormant instead. + const twoDaysAgo = new Date(Date.now() - 2 * 24 * 60 * 60 * 1000); + utimesSync(join(testHapiHome, 'cache', 'cursor-models.json'), twoDaysAgo, twoDaysAgo); + + expect(checkSpawnModel('cursor', 'gpt-5')).toEqual({ ok: true }); + }); + + it('allows any model when no catalog is cached', () => { + expect(checkSpawnModel('cursor', 'gpt-5')).toEqual({ ok: true }); + }); + + it('allows flavors without a cached catalog', () => { + expect(checkSpawnModel('claude', 'claude-opus-5')).toEqual({ ok: true }); + expect(checkSpawnModel('codex', 'gpt-5')).toEqual({ ok: true }); + }); + + it('allows a spawn with no model', () => { + setCursorAcpModelsSnapshot({ + availableModels: [{ modelId: 'composer-2.5[thinking]' }], + currentModelId: 'composer-2.5[thinking]' + }); + + expect(checkSpawnModel('cursor', undefined)).toEqual({ ok: true }); + }); +}); diff --git a/cli/src/runner/spawnModelPreflight.ts b/cli/src/runner/spawnModelPreflight.ts new file mode 100644 index 0000000000..77f27c1a70 --- /dev/null +++ b/cli/src/runner/spawnModelPreflight.ts @@ -0,0 +1,23 @@ +import type { AgentFlavor, SpawnModelValidation } from '@hapi/protocol'; +import { validateSpawnModelAgainstCatalog } from '@hapi/protocol'; +import { getCachedCursorModelIds } from '@/modules/common/cursorModels'; + +/** + * Cached-only model catalogs for the spawn preflight. Flavors without a cached + * catalog return [], which `validateSpawnModelAgainstCatalog` treats as + * "unknown" and never rejects. Add a flavor here only when its ids can be read + * without spawning a probe. + */ +function getCachedModelCatalog(agent: AgentFlavor): string[] { + if (agent === 'cursor') return getCachedCursorModelIds(); + return []; +} + +/** + * Fail-closed guard against spawning a child that would die at agent handshake + * because the requested model does not exist (e.g. `--agent cursor --model gpt-5`), + * leaving an archived session with no turns behind. + */ +export function checkSpawnModel(agent: AgentFlavor, model: string | null | undefined): SpawnModelValidation { + return validateSpawnModelAgainstCatalog(agent, model, getCachedModelCatalog(agent)); +} diff --git a/hub/src/sync/rpcGateway.ts b/hub/src/sync/rpcGateway.ts index b749647f24..88672b0289 100644 --- a/hub/src/sync/rpcGateway.ts +++ b/hub/src/sync/rpcGateway.ts @@ -193,7 +193,7 @@ export class RpcGateway { | { type: 'error' message: string - code?: 'agent_unavailable' | 'outside_workspace_roots' + code?: 'agent_unavailable' | 'model_unavailable' | 'outside_workspace_roots' agent?: AgentFlavor } > { @@ -228,7 +228,7 @@ export class RpcGateway { return { type: 'success', sessionId: obj.sessionId } } if (obj.type === 'error' && typeof obj.errorMessage === 'string') { - const code = obj.code === 'agent_unavailable' || obj.code === 'outside_workspace_roots' + const code = obj.code === 'agent_unavailable' || obj.code === 'model_unavailable' || obj.code === 'outside_workspace_roots' ? obj.code : undefined const unavailableAgent = typeof obj.agent === 'string' ? obj.agent as AgentFlavor : undefined diff --git a/shared/src/apiTypes.ts b/shared/src/apiTypes.ts index f0c0d132dc..df275baedc 100644 --- a/shared/src/apiTypes.ts +++ b/shared/src/apiTypes.ts @@ -133,7 +133,7 @@ export type SpawnResponse = | { type: 'error' message: string - code?: 'agent_unavailable' | 'runner_upgrade_required' | 'outside_workspace_roots' + code?: 'agent_unavailable' | 'model_unavailable' | 'runner_upgrade_required' | 'outside_workspace_roots' agent?: z.infer } diff --git a/shared/src/index.ts b/shared/src/index.ts index 4fa14330f9..8d4bf0861a 100644 --- a/shared/src/index.ts +++ b/shared/src/index.ts @@ -17,6 +17,7 @@ export * from './socket' export * from './sessionSummary' export * from './sessionCitation' export * from './sessionExport' +export * from './spawnModelCatalog' export * from './piThinkingLevel' export * from './runnerCapabilities' export * from './agentConfig' diff --git a/shared/src/spawnModelCatalog.test.ts b/shared/src/spawnModelCatalog.test.ts new file mode 100644 index 0000000000..3f9f77ba7d --- /dev/null +++ b/shared/src/spawnModelCatalog.test.ts @@ -0,0 +1,77 @@ +import { describe, expect, it } from 'vitest'; +import { validateSpawnModelAgainstCatalog } from './spawnModelCatalog'; + +const CURSOR_CATALOG = [ + 'auto', + 'composer-2.5[thinking]', + 'gpt-5.1[reasoning=high]', + 'cursor-grok-4.5-fast' +]; + +describe('validateSpawnModelAgainstCatalog', () => { + it('rejects a model the catalog does not contain and names the accepted ids', () => { + const result = validateSpawnModelAgainstCatalog('cursor', 'gpt-5', CURSOR_CATALOG); + + expect(result.ok).toBe(false); + expect(result.ok === false && result.message).toContain("Model 'gpt-5' is not in the cursor model catalog"); + // Cursor wire ids collapse to the base slug the user can actually pass. + expect(result.ok === false && result.message).toContain('Accepted: gpt-5.1, auto, composer-2.5, cursor-grok-4.5'); + }); + + it('accepts an exact catalog id', () => { + expect(validateSpawnModelAgainstCatalog('cursor', 'gpt-5.1[reasoning=high]', CURSOR_CATALOG)).toEqual({ ok: true }); + }); + + it('accepts a Cursor base slug for a parameterized wire id', () => { + expect(validateSpawnModelAgainstCatalog('cursor', 'composer-2.5', CURSOR_CATALOG)).toEqual({ ok: true }); + }); + + it('accepts a Cursor CLI sku whose base is in the catalog', () => { + expect(validateSpawnModelAgainstCatalog('cursor', 'cursor-grok-4.5-high', CURSOR_CATALOG)).toEqual({ ok: true }); + }); + + it('accepts a renamed Cursor base that the stale-model remap would resolve', () => { + expect(validateSpawnModelAgainstCatalog('cursor', 'grok-4.5', ['cursor-grok-4.5-fast'])).toEqual({ ok: true }); + }); + + it('matches catalog ids case-insensitively', () => { + expect(validateSpawnModelAgainstCatalog('cursor', 'Composer-2.5', CURSOR_CATALOG)).toEqual({ ok: true }); + }); + + it('accepts wildcard ids and no model at all', () => { + expect(validateSpawnModelAgainstCatalog('cursor', 'auto', CURSOR_CATALOG)).toEqual({ ok: true }); + expect(validateSpawnModelAgainstCatalog('cursor', 'Auto', CURSOR_CATALOG)).toEqual({ ok: true }); + expect(validateSpawnModelAgainstCatalog('cursor', 'default', CURSOR_CATALOG)).toEqual({ ok: true }); + // Bare ACP wire form the spawn path also reads as "let the agent pick". + expect(validateSpawnModelAgainstCatalog('cursor', 'default[]', CURSOR_CATALOG)).toEqual({ ok: true }); + expect(validateSpawnModelAgainstCatalog('cursor', undefined, CURSOR_CATALOG)).toEqual({ ok: true }); + expect(validateSpawnModelAgainstCatalog('cursor', ' ', CURSOR_CATALOG)).toEqual({ ok: true }); + }); + + it('never rejects when the machine could not enumerate a catalog', () => { + expect(validateSpawnModelAgainstCatalog('cursor', 'gpt-5', [])).toEqual({ ok: true }); + expect(validateSpawnModelAgainstCatalog('claude', 'claude-opus-5', [])).toEqual({ ok: true }); + }); + + it('does not apply Cursor sku-suffix stripping to other flavors', () => { + // agy ships `-` ids; `-high` is part of the id, not a sku suffix. + expect(validateSpawnModelAgainstCatalog('agy', 'gemini-3.7-flash-low', ['gemini-3.7-flash-high'])).toEqual({ + ok: false, + message: "Model 'gemini-3.7-flash-low' is not in the agy model catalog on this machine. Accepted: gemini-3.7-flash-high" + }); + }); + + it('lists near-miss ids before the rest of the catalog', () => { + const catalog = ['claude-opus-5', 'composer-2.5', 'gpt-5.2', 'gpt-5.5']; + const result = validateSpawnModelAgainstCatalog('cursor', 'gpt-5', catalog); + + expect(result.ok === false && result.message).toContain('Accepted: gpt-5.2, gpt-5.5, claude-opus-5, composer-2.5'); + }); + + it('truncates a long accepted list', () => { + const catalog = Array.from({ length: 20 }, (_, index) => `model-${index}`); + const result = validateSpawnModelAgainstCatalog('cursor', 'nope', catalog); + + expect(result.ok === false && result.message).toContain('… (20 total)'); + }); +}); diff --git a/shared/src/spawnModelCatalog.ts b/shared/src/spawnModelCatalog.ts new file mode 100644 index 0000000000..9968566979 --- /dev/null +++ b/shared/src/spawnModelCatalog.ts @@ -0,0 +1,93 @@ +import { cursorCliSkuBaseId, cursorModelBaseId, resolveCursorLegacyModelBase } from './cursorCliSku' +import type { AgentFlavor } from './modes' + +/** + * Ids that mean "let the agent pick"; never matched against a catalog. Matches + * the spawn path's own reading of them (see cursorAcpBackend / cursorModeConfig), + * which is case-insensitive and also accepts the bare wire form `default[]`. + */ +const WILDCARD_MODEL_IDS = new Set(['auto', 'default']) + +/** Keeps the rejection message readable when a catalog has dozens of skus. */ +const MAX_LISTED_MODELS = 12 + +export type SpawnModelValidation = + | { ok: true } + | { ok: false; message: string } + +/** + * Ids a catalog entry (or a requested model) should be compared under. Cursor + * ships the same model as an ACP wire id (`composer-2.5[thinking]`) and as CLI + * skus (`composer-2.5-high-fast`), so both collapse to their base slug. + */ +function catalogCandidates(agent: AgentFlavor, modelId: string): string[] { + const trimmed = modelId.trim().toLowerCase() + if (!trimmed) return [] + if (agent !== 'cursor') return [trimmed] + // Renamed bases (grok-4.5 → cursor-grok-4.5) still reach spawn from stale hub + // rows and mobile drafts, where cursorStaleModelRemap resolves them; the + // preflight must not reject what that remap would have fixed. + const bases = [trimmed, cursorModelBaseId(trimmed), cursorCliSkuBaseId(trimmed)] + return [...new Set([...bases, ...bases.map(resolveCursorLegacyModelBase)])].filter(Boolean) +} + +function isWildcardModelId(agent: AgentFlavor, modelId: string): boolean { + const trimmed = modelId.trim().toLowerCase() + const base = agent === 'cursor' ? cursorModelBaseId(trimmed) : trimmed + return WILDCARD_MODEL_IDS.has(trimmed) || WILDCARD_MODEL_IDS.has(base) +} + +export function buildSpawnModelCatalogIndex(agent: AgentFlavor, catalog: readonly string[]): Set { + const index = new Set() + for (const entry of catalog) { + for (const candidate of catalogCandidates(agent, entry)) index.add(candidate) + } + return index +} + +/** + * Near-misses first: a rejected `gpt-5` is most usefully answered with the + * `gpt-5.x` ids, which plain alphabetical order would truncate away. + * + * Cursor entries collapse to their base slug — a catalog of ACP wire ids would + * otherwise spend the whole list on `gpt-5-mini[fast=false]`-style variants of + * two or three models, and the base slug is itself accepted. + */ +function formatAcceptedModels(agent: AgentFlavor, requested: string, catalog: readonly string[]): string { + const prefix = requested.toLowerCase() + const display = (id: string): string => (agent === 'cursor' ? cursorCliSkuBaseId(id) : id) + const ids = [...new Set(catalog.map((id) => display(id.trim())).filter(Boolean))].sort() + const ranked = [ + ...ids.filter((id) => id.toLowerCase().startsWith(prefix)), + ...ids.filter((id) => !id.toLowerCase().startsWith(prefix)) + ] + const listed = ranked.slice(0, MAX_LISTED_MODELS).join(', ') + return ranked.length > MAX_LISTED_MODELS ? `${listed}, … (${ranked.length} total)` : listed +} + +/** + * Rejects a spawn model the machine's catalog for this flavor definitely does + * not contain, so the runner can fail before booting a child that would die at + * agent handshake and leave an archived session with no turns. + * + * An empty catalog means "this machine could not enumerate models", not "no + * models exist" — never reject there, or every flavor without a probe would + * stop accepting valid ids. + */ +export function validateSpawnModelAgainstCatalog( + agent: AgentFlavor, + model: string | null | undefined, + catalog: readonly string[] +): SpawnModelValidation { + const requested = model?.trim() ?? '' + if (!requested || isWildcardModelId(agent, requested)) return { ok: true } + if (catalog.length === 0) return { ok: true } + + const index = buildSpawnModelCatalogIndex(agent, catalog) + if (catalogCandidates(agent, requested).some((candidate) => index.has(candidate))) return { ok: true } + + return { + ok: false, + message: `Model '${requested}' is not in the ${agent} model catalog on this machine. Accepted: ${formatAcceptedModels(agent, requested, catalog)}` + } +} From d6cd1489edc2c351ee07739c88624c1f4f7cccc4 Mon Sep 17 00:00:00 2001 From: HeavyGee <133152184+heavygee@users.noreply.github.com> Date: Fri, 4 Sep 2026 14:14:43 +0000 Subject: [PATCH 2/4] fix(runner): stop a cache read from renewing a stale catalog's freshness The spawn preflight dates the Cursor catalog by the shared cache file's mtime, but `listCursorModels` fed the file's own contents back through `applyInMemoryCache`, which writes it out again. Runner startup prewarm therefore re-stamped an arbitrarily old snapshot as current on every boot, so the 24h fail-open never triggered and a catalog from before a Cursor upgrade kept rejecting models that had become valid since. Only a live source (ACP snapshot or probe) may stamp the file now; the two shared-cache read branches update the in-process cache without persisting. Reported by the upstream PR review bot on #1753. via [HAPI](https://hapi.run) Co-Authored-By: HAPI --- cli/src/modules/common/cursorModels.ts | 19 +++++++++++---- cli/src/runner/spawnModelPreflight.test.ts | 27 +++++++++++++++++++--- 2 files changed, 39 insertions(+), 7 deletions(-) diff --git a/cli/src/modules/common/cursorModels.ts b/cli/src/modules/common/cursorModels.ts index 68c69b4746..352cf469f0 100644 --- a/cli/src/modules/common/cursorModels.ts +++ b/cli/src/modules/common/cursorModels.ts @@ -286,12 +286,23 @@ async function runCursorModelProbe(): Promise { }); } -async function applyInMemoryCache(response: ListCursorModelsResponse): Promise { +/** + * `persist` must stay false whenever `response` was itself read back off the + * shared cache. The file's mtime is what dates the catalog for the spawn + * preflight (getCachedCursorModelIds), so writing an old snapshot back would + * renew its freshness without anyone having asked Cursor for a new one — a + * catalog from before a Cursor upgrade would then keep rejecting models that + * are now valid. Only a live source (ACP snapshot or probe) may stamp the file. + */ +async function applyInMemoryCache( + response: ListCursorModelsResponse, + persist = true +): Promise { const enriched = await enrichCursorModelsWithCliSkus(response); if ((enriched.availableModels?.length ?? 0) > 0) { cache.expiresAt = Date.now() + CACHE_TTL_MS; cache.response = enriched; - writeSharedCursorModelsCache(enriched); + if (persist) writeSharedCursorModelsCache(enriched); } return enriched; } @@ -305,7 +316,7 @@ async function listCursorModelsWhileAcpActive(): Promise Date.now() && (cache.response.availableModels?.length ?? 0) > 0) { const shared = readSharedCursorModelsCache(); @@ -334,7 +345,7 @@ export async function listCursorModels(): Promise { const shared = readSharedCursorModelsCache(); if (shared) { - return applyInMemoryCache(shared); + return applyInMemoryCache(shared, false); } if (inflight) { diff --git a/cli/src/runner/spawnModelPreflight.test.ts b/cli/src/runner/spawnModelPreflight.test.ts index 2cba96d9f8..2b4cf9d7d4 100644 --- a/cli/src/runner/spawnModelPreflight.test.ts +++ b/cli/src/runner/spawnModelPreflight.test.ts @@ -3,7 +3,8 @@ import { mkdtempSync, rmSync, utimesSync } from 'node:fs'; import { tmpdir } from 'node:os'; import { join } from 'node:path'; import { setCursorAcpModelsSnapshot } from '@/cursor/utils/cursorAcpModelsBridge'; -import { _resetCursorModelsCacheForTests, seedCursorModelsCache } from '@/modules/common/cursorModels'; +import { _resetCursorModelsCacheForTests, listCursorModels, seedCursorModelsCache } from '@/modules/common/cursorModels'; +import { writeSharedCursorModelsCache } from '@/modules/common/cursorModelsSharedCache'; import { checkSpawnModel } from './spawnModelPreflight'; // Isolate the on-disk cursor-models cache to this file's own HAPI_HOME so @@ -12,6 +13,11 @@ const previousHapiHome = process.env.HAPI_HOME; const testHapiHome = mkdtempSync(join(tmpdir(), 'hapi-spawn-model-preflight-')); process.env.HAPI_HOME = testHapiHome; +function ageSharedCache(ageMs: number): void { + const when = new Date(Date.now() - ageMs); + utimesSync(join(testHapiHome, 'cache', 'cursor-models.json'), when, when); +} + afterEach(() => { setCursorAcpModelsSnapshot(null); _resetCursorModelsCacheForTests(); @@ -65,8 +71,23 @@ describe('checkSpawnModel', () => { }); // A catalog that predates a Cursor upgrade must not reject an id that is // now valid; the preflight goes dormant instead. - const twoDaysAgo = new Date(Date.now() - 2 * 24 * 60 * 60 * 1000); - utimesSync(join(testHapiHome, 'cache', 'cursor-models.json'), twoDaysAgo, twoDaysAgo); + ageSharedCache(2 * 24 * 60 * 60 * 1000); + + expect(checkSpawnModel('cursor', 'gpt-5')).toEqual({ ok: true }); + }); + + it('does not let a cache read renew the freshness of a stale catalog', async () => { + // Fresh runner: cold in-process cache, shared file left over from before a + // Cursor upgrade. Startup prewarm reads that file; it must not stamp it as + // current, or the preflight would keep rejecting models added since. + writeSharedCursorModelsCache({ + success: true, + availableModels: [{ modelId: 'composer-2.5' }], + currentModelId: 'composer-2.5' + }); + ageSharedCache(2 * 24 * 60 * 60 * 1000); + + await listCursorModels(); expect(checkSpawnModel('cursor', 'gpt-5')).toEqual({ ok: true }); }); From 410be9818aed6ad57c3a88eb575421e0ee49eb2d Mon Sep 17 00:00:00 2001 From: HeavyGee <133152184+heavygee@users.noreply.github.com> Date: Fri, 4 Sep 2026 14:26:07 +0000 Subject: [PATCH 3/4] fix(runner): require an exact Cursor sku when the catalog enumerates variants Base collapsing let an explicit CLI sku through whenever any variant of its base existed, so `gpt-5.5-high-fast` passed a catalog offering only `gpt-5.5-medium`. remapStaleCursorModelId deliberately refuses to downgrade that pair and resolveCursorSpawnModel forwards the original, so the handshake failure this guard exists to prevent landed anyway. When the catalog enumerates CLI sku variants of the requested base, an explicit variant of that base must now match one exactly, and the error names the siblings that do exist. Scoped to CLI skus deliberately. Rejecting any variant the remap declines to rewrite - as first suggested - would reject every suffixed sku on this machine: its cached catalog is 74 ACP wire ids and zero CLI skus, and the remap returns null for `cursor-grok-4.5-fast` and `gpt-5.5-medium` alike there. A wire-only catalog is a different naming of the same models and says nothing about which skus exist, so those keep matching on base. Reported by the upstream PR review bot on #1753. via [HAPI](https://hapi.run) Co-Authored-By: HAPI --- shared/src/spawnModelCatalog.test.ts | 28 +++++++++++++- shared/src/spawnModelCatalog.ts | 56 +++++++++++++++++++++++++--- 2 files changed, 77 insertions(+), 7 deletions(-) diff --git a/shared/src/spawnModelCatalog.test.ts b/shared/src/spawnModelCatalog.test.ts index 3f9f77ba7d..3c9547a31e 100644 --- a/shared/src/spawnModelCatalog.test.ts +++ b/shared/src/spawnModelCatalog.test.ts @@ -1,11 +1,13 @@ import { describe, expect, it } from 'vitest'; import { validateSpawnModelAgainstCatalog } from './spawnModelCatalog'; +// Wire-id shaped, as a real cached catalog is: `agent --list-models` slugs and +// ACP wires are separate namings, and machines commonly cache only the wires. const CURSOR_CATALOG = [ 'auto', 'composer-2.5[thinking]', 'gpt-5.1[reasoning=high]', - 'cursor-grok-4.5-fast' + 'cursor-grok-4.5[fast=true]' ]; describe('validateSpawnModelAgainstCatalog', () => { @@ -26,8 +28,30 @@ describe('validateSpawnModelAgainstCatalog', () => { expect(validateSpawnModelAgainstCatalog('cursor', 'composer-2.5', CURSOR_CATALOG)).toEqual({ ok: true }); }); - it('accepts a Cursor CLI sku whose base is in the catalog', () => { + it('accepts a Cursor CLI sku whose base is in a wire-id catalog', () => { + // A wire-only catalog is a different naming of the same models — it says + // nothing about which effort/speed skus exist, so the sku must not be + // rejected for being absent from it. expect(validateSpawnModelAgainstCatalog('cursor', 'cursor-grok-4.5-high', CURSOR_CATALOG)).toEqual({ ok: true }); + expect(validateSpawnModelAgainstCatalog('cursor', 'gpt-5.1-high-fast', CURSOR_CATALOG)).toEqual({ ok: true }); + }); + + it('rejects an explicit Cursor sku when the catalog enumerates other variants of its base', () => { + const result = validateSpawnModelAgainstCatalog('cursor', 'gpt-5.5-high-fast', ['gpt-5.5-medium']); + + expect(result).toEqual({ + ok: false, + message: "Model 'gpt-5.5-high-fast' is not an available cursor variant of 'gpt-5.5' on this machine. Accepted: gpt-5.5-medium" + }); + }); + + it('accepts an enumerated Cursor sku variant and the bare base beside it', () => { + const catalog = ['gpt-5.5-medium', 'gpt-5.5-high']; + + expect(validateSpawnModelAgainstCatalog('cursor', 'gpt-5.5-high', catalog)).toEqual({ ok: true }); + expect(validateSpawnModelAgainstCatalog('cursor', 'gpt-5.5', catalog)).toEqual({ ok: true }); + // Variants of a base the catalog does not enumerate keep matching on base. + expect(validateSpawnModelAgainstCatalog('cursor', 'composer-2.5-high', [...catalog, 'composer-2.5'])).toEqual({ ok: true }); }); it('accepts a renamed Cursor base that the stale-model remap would resolve', () => { diff --git a/shared/src/spawnModelCatalog.ts b/shared/src/spawnModelCatalog.ts index 9968566979..1b362fdf5f 100644 --- a/shared/src/spawnModelCatalog.ts +++ b/shared/src/spawnModelCatalog.ts @@ -1,4 +1,9 @@ -import { cursorCliSkuBaseId, cursorModelBaseId, resolveCursorLegacyModelBase } from './cursorCliSku' +import { + cursorCliSkuBaseId, + cursorModelBaseId, + isCursorCliSkuVariantId, + resolveCursorLegacyModelBase +} from './cursorCliSku' import type { AgentFlavor } from './modes' /** @@ -53,16 +58,47 @@ export function buildSpawnModelCatalogIndex(agent: AgentFlavor, catalog: readonl * otherwise spend the whole list on `gpt-5-mini[fast=false]`-style variants of * two or three models, and the base slug is itself accepted. */ +function formatIdList(ids: readonly string[]): string { + const listed = ids.slice(0, MAX_LISTED_MODELS).join(', ') + return ids.length > MAX_LISTED_MODELS ? `${listed}, … (${ids.length} total)` : listed +} + function formatAcceptedModels(agent: AgentFlavor, requested: string, catalog: readonly string[]): string { const prefix = requested.toLowerCase() const display = (id: string): string => (agent === 'cursor' ? cursorCliSkuBaseId(id) : id) const ids = [...new Set(catalog.map((id) => display(id.trim())).filter(Boolean))].sort() - const ranked = [ + return formatIdList([ ...ids.filter((id) => id.toLowerCase().startsWith(prefix)), ...ids.filter((id) => !id.toLowerCase().startsWith(prefix)) - ] - const listed = ranked.slice(0, MAX_LISTED_MODELS).join(', ') - return ranked.length > MAX_LISTED_MODELS ? `${listed}, … (${ranked.length} total)` : listed + ]) +} + +/** + * A catalog that enumerates CLI sku variants of a base (`gpt-5.5-medium`) is + * authoritative about which variants exist, so an explicit variant of that base + * has to match one exactly. Collapsing to the base would wave `gpt-5.5-high-fast` + * through when only `gpt-5.5-medium` is offered — and resolveCursorSpawnModel + * deliberately forwards an unavailable sku rather than downgrade it + * (cursorStaleModelRemap), so the handshake failure lands anyway. + * + * Scoped to CLI skus on purpose. ACP wire ids are a different naming of the same + * models, with parameter sets the remap resolves for us, and a wire-only catalog + * says nothing at all about which skus exist — requiring an exact variant there + * would reject every valid suffixed sku on a machine whose cache holds wires. + */ +function unavailableCursorSkuVariants(requested: string, catalog: readonly string[]): string[] | null { + if (!isCursorCliSkuVariantId(requested)) return null + + const wanted = requested.trim().toLowerCase() + const base = cursorCliSkuBaseId(wanted) + const siblings = [...new Set( + catalog + .map((id) => id.trim().toLowerCase()) + .filter((id) => isCursorCliSkuVariantId(id) && cursorCliSkuBaseId(id) === base) + )].sort() + + if (siblings.length === 0 || siblings.includes(wanted)) return null + return siblings } /** @@ -83,6 +119,16 @@ export function validateSpawnModelAgainstCatalog( if (!requested || isWildcardModelId(agent, requested)) return { ok: true } if (catalog.length === 0) return { ok: true } + if (agent === 'cursor') { + const siblings = unavailableCursorSkuVariants(requested, catalog) + if (siblings) { + return { + ok: false, + message: `Model '${requested}' is not an available cursor variant of '${cursorCliSkuBaseId(requested.toLowerCase())}' on this machine. Accepted: ${formatIdList(siblings)}` + } + } + } + const index = buildSpawnModelCatalogIndex(agent, catalog) if (catalogCandidates(agent, requested).some((candidate) => index.has(candidate))) return { ok: true } From d5fa8963fa91b825a04af6532a9d870dbd466088 Mon Sep 17 00:00:00 2001 From: HeavyGee <133152184+heavygee@users.noreply.github.com> Date: Fri, 4 Sep 2026 14:35:42 +0000 Subject: [PATCH 4/4] revert: drop the exact-Cursor-sku rule; pin the base-level contract instead 410be98 assumed a catalog listing CLI sku variants of a base was authoritative about that base's variants. It is not. Shared cliModelSkus rows are explicitly partial and unioned with a later probe, and that probe is skipped entirely while an ACP session holds the CLI lock - so a cache holding only `gpt-5.5-medium` would have rejected a perfectly valid `gpt-5.5-high` for as long as it stayed partial. That is a worse failure than the handshake error it pre-empted. Reverts to base-level matching and states the contract where it belongs: this guard rejects a model only when its base appears nowhere in the catalog. Variant-level availability stays the handshake's job. Tests pin both an unlisted sku and a contradictory bracket wire as allowed, so the scope is not re-narrowed by accident. Reported by the upstream PR review bot on #1753. via [HAPI](https://hapi.run) Co-Authored-By: HAPI --- shared/src/spawnModelCatalog.test.ts | 41 ++++++--------- shared/src/spawnModelCatalog.ts | 77 +++++++++------------------- 2 files changed, 39 insertions(+), 79 deletions(-) diff --git a/shared/src/spawnModelCatalog.test.ts b/shared/src/spawnModelCatalog.test.ts index 3c9547a31e..8f81e9a3d5 100644 --- a/shared/src/spawnModelCatalog.test.ts +++ b/shared/src/spawnModelCatalog.test.ts @@ -1,13 +1,11 @@ import { describe, expect, it } from 'vitest'; import { validateSpawnModelAgainstCatalog } from './spawnModelCatalog'; -// Wire-id shaped, as a real cached catalog is: `agent --list-models` slugs and -// ACP wires are separate namings, and machines commonly cache only the wires. const CURSOR_CATALOG = [ 'auto', 'composer-2.5[thinking]', 'gpt-5.1[reasoning=high]', - 'cursor-grok-4.5[fast=true]' + 'cursor-grok-4.5-fast' ]; describe('validateSpawnModelAgainstCatalog', () => { @@ -28,30 +26,8 @@ describe('validateSpawnModelAgainstCatalog', () => { expect(validateSpawnModelAgainstCatalog('cursor', 'composer-2.5', CURSOR_CATALOG)).toEqual({ ok: true }); }); - it('accepts a Cursor CLI sku whose base is in a wire-id catalog', () => { - // A wire-only catalog is a different naming of the same models — it says - // nothing about which effort/speed skus exist, so the sku must not be - // rejected for being absent from it. + it('accepts a Cursor CLI sku whose base is in the catalog', () => { expect(validateSpawnModelAgainstCatalog('cursor', 'cursor-grok-4.5-high', CURSOR_CATALOG)).toEqual({ ok: true }); - expect(validateSpawnModelAgainstCatalog('cursor', 'gpt-5.1-high-fast', CURSOR_CATALOG)).toEqual({ ok: true }); - }); - - it('rejects an explicit Cursor sku when the catalog enumerates other variants of its base', () => { - const result = validateSpawnModelAgainstCatalog('cursor', 'gpt-5.5-high-fast', ['gpt-5.5-medium']); - - expect(result).toEqual({ - ok: false, - message: "Model 'gpt-5.5-high-fast' is not an available cursor variant of 'gpt-5.5' on this machine. Accepted: gpt-5.5-medium" - }); - }); - - it('accepts an enumerated Cursor sku variant and the bare base beside it', () => { - const catalog = ['gpt-5.5-medium', 'gpt-5.5-high']; - - expect(validateSpawnModelAgainstCatalog('cursor', 'gpt-5.5-high', catalog)).toEqual({ ok: true }); - expect(validateSpawnModelAgainstCatalog('cursor', 'gpt-5.5', catalog)).toEqual({ ok: true }); - // Variants of a base the catalog does not enumerate keep matching on base. - expect(validateSpawnModelAgainstCatalog('cursor', 'composer-2.5-high', [...catalog, 'composer-2.5'])).toEqual({ ok: true }); }); it('accepts a renamed Cursor base that the stale-model remap would resolve', () => { @@ -62,6 +38,19 @@ describe('validateSpawnModelAgainstCatalog', () => { expect(validateSpawnModelAgainstCatalog('cursor', 'Composer-2.5', CURSOR_CATALOG)).toEqual({ ok: true }); }); + it('leaves variant-level availability to the handshake', () => { + // Absence of a variant from a cached catalog is not proof it is gone: + // shared cliModelSkus rows are partial until a probe unions more in, and + // that probe is skipped while an ACP session holds the CLI lock. Rejecting + // here would block valid spawns on any mid-union or wire-only cache. + expect(validateSpawnModelAgainstCatalog('cursor', 'gpt-5.5-high-fast', ['gpt-5.5-medium'])).toEqual({ ok: true }); + expect(validateSpawnModelAgainstCatalog( + 'cursor', + 'claude-opus-4-8[thinking=false,effort=high]', + ['claude-opus-4-8[thinking=true,context=300k,effort=high,fast=false]'] + )).toEqual({ ok: true }); + }); + it('accepts wildcard ids and no model at all', () => { expect(validateSpawnModelAgainstCatalog('cursor', 'auto', CURSOR_CATALOG)).toEqual({ ok: true }); expect(validateSpawnModelAgainstCatalog('cursor', 'Auto', CURSOR_CATALOG)).toEqual({ ok: true }); diff --git a/shared/src/spawnModelCatalog.ts b/shared/src/spawnModelCatalog.ts index 1b362fdf5f..cdb8a39ebf 100644 --- a/shared/src/spawnModelCatalog.ts +++ b/shared/src/spawnModelCatalog.ts @@ -1,9 +1,4 @@ -import { - cursorCliSkuBaseId, - cursorModelBaseId, - isCursorCliSkuVariantId, - resolveCursorLegacyModelBase -} from './cursorCliSku' +import { cursorCliSkuBaseId, cursorModelBaseId, resolveCursorLegacyModelBase } from './cursorCliSku' import type { AgentFlavor } from './modes' /** @@ -58,47 +53,16 @@ export function buildSpawnModelCatalogIndex(agent: AgentFlavor, catalog: readonl * otherwise spend the whole list on `gpt-5-mini[fast=false]`-style variants of * two or three models, and the base slug is itself accepted. */ -function formatIdList(ids: readonly string[]): string { - const listed = ids.slice(0, MAX_LISTED_MODELS).join(', ') - return ids.length > MAX_LISTED_MODELS ? `${listed}, … (${ids.length} total)` : listed -} - function formatAcceptedModels(agent: AgentFlavor, requested: string, catalog: readonly string[]): string { const prefix = requested.toLowerCase() const display = (id: string): string => (agent === 'cursor' ? cursorCliSkuBaseId(id) : id) const ids = [...new Set(catalog.map((id) => display(id.trim())).filter(Boolean))].sort() - return formatIdList([ + const ranked = [ ...ids.filter((id) => id.toLowerCase().startsWith(prefix)), ...ids.filter((id) => !id.toLowerCase().startsWith(prefix)) - ]) -} - -/** - * A catalog that enumerates CLI sku variants of a base (`gpt-5.5-medium`) is - * authoritative about which variants exist, so an explicit variant of that base - * has to match one exactly. Collapsing to the base would wave `gpt-5.5-high-fast` - * through when only `gpt-5.5-medium` is offered — and resolveCursorSpawnModel - * deliberately forwards an unavailable sku rather than downgrade it - * (cursorStaleModelRemap), so the handshake failure lands anyway. - * - * Scoped to CLI skus on purpose. ACP wire ids are a different naming of the same - * models, with parameter sets the remap resolves for us, and a wire-only catalog - * says nothing at all about which skus exist — requiring an exact variant there - * would reject every valid suffixed sku on a machine whose cache holds wires. - */ -function unavailableCursorSkuVariants(requested: string, catalog: readonly string[]): string[] | null { - if (!isCursorCliSkuVariantId(requested)) return null - - const wanted = requested.trim().toLowerCase() - const base = cursorCliSkuBaseId(wanted) - const siblings = [...new Set( - catalog - .map((id) => id.trim().toLowerCase()) - .filter((id) => isCursorCliSkuVariantId(id) && cursorCliSkuBaseId(id) === base) - )].sort() - - if (siblings.length === 0 || siblings.includes(wanted)) return null - return siblings + ] + const listed = ranked.slice(0, MAX_LISTED_MODELS).join(', ') + return ranked.length > MAX_LISTED_MODELS ? `${listed}, … (${ranked.length} total)` : listed } /** @@ -106,8 +70,25 @@ function unavailableCursorSkuVariants(requested: string, catalog: readonly strin * not contain, so the runner can fail before booting a child that would die at * agent handshake and leave an archived session with no turns. * - * An empty catalog means "this machine could not enumerate models", not "no - * models exist" — never reject there, or every flavor without a probe would + * The contract is deliberately **base-level**: a model is rejected only when its + * base appears nowhere in the catalog. Variant-level availability — an effort + * sku like `gpt-5.5-high-fast`, or a wire with contradictory params like + * `claude-opus-4-8[thinking=false,effort=high]` — stays the handshake's job, + * because a cached catalog is not authoritative about variants: + * + * - shared cliModelSkus rows are explicitly partial, unioned with a later + * probe (cursorModels: "unions shared partial cliModelSkus with probe + * results"), and that probe is skipped entirely while an ACP session holds + * the CLI lock, so a missing sku proves nothing; + * - a wire-only cache carries no sku naming at all, and a sku-only cache + * carries no param sets. + * + * Treating absence at variant level as proof would reject valid spawns on every + * machine whose cache is mid-union or wire-only — a worse failure than the + * handshake error it would pre-empt. + * + * An empty catalog likewise means "this machine could not enumerate models", not + * "no models exist" — never reject there, or every flavor without a probe would * stop accepting valid ids. */ export function validateSpawnModelAgainstCatalog( @@ -119,16 +100,6 @@ export function validateSpawnModelAgainstCatalog( if (!requested || isWildcardModelId(agent, requested)) return { ok: true } if (catalog.length === 0) return { ok: true } - if (agent === 'cursor') { - const siblings = unavailableCursorSkuVariants(requested, catalog) - if (siblings) { - return { - ok: false, - message: `Model '${requested}' is not an available cursor variant of '${cursorCliSkuBaseId(requested.toLowerCase())}' on this machine. Accepted: ${formatIdList(siblings)}` - } - } - } - const index = buildSpawnModelCatalogIndex(agent, catalog) if (catalogCandidates(agent, requested).some((candidate) => index.has(candidate))) return { ok: true }