From d8aa8736cdb2d541ff9f74a89623de54c610092b Mon Sep 17 00:00:00 2001 From: HeavyGee <133152184+heavygee@users.noreply.github.com> Date: Wed, 29 Jul 2026 03:05:41 +0100 Subject: [PATCH] fix(cli): surface real Cursor ACP session/load errors (#1198) * fix(cli): surface real Cursor ACP session/load errors Stop mislabeling every ACP session/load failure as a legacy stream-json protocol problem. Prefer Cursor's Cannot use this model stderr (including Available models when present), attach drained stderr on process close, and keep structured formatAcpLoadError logs. Co-authored-by: Cursor * fix(cli): accumulate ACP stderr across split chunks Address Codex Major on #1198: child_process stderr data events are not message boundaries. Concatenate raw chunks in a rolling window, extract Cannot use this model from the window on close, and prefer that over a partial onStderrError hint when classifying resume failures. Co-authored-by: Cursor * fix(cli): pin ACP model-rejection stderr when catalog overflows Once Cannot use this model appears, keep the buffer from that match head so a long Available models list cannot roll the rejection out of the rolling window (Codex follow-up on #1198). Co-authored-by: Cursor * fix(cli): wait for model id before ACP model-rejection emit Only emit Cannot use this model via onStderrError once a non-space token follows the colon, so a split before the id cannot suppress the completed rolling-window message (Codex Minor on #1198). Co-authored-by: Cursor * fix(cli): block ACP writes between process exit and close Keep the post-exit stdin write guard while deferring markClosed until stdio close so stderr can still enrich the failure (Codex Minor on #1198). Co-authored-by: Cursor --------- Co-authored-by: Cursor --- .../backends/acp/AcpStdioTransport.test.ts | 214 +++++++++++++++++- .../agent/backends/acp/AcpStdioTransport.ts | 111 ++++++++- .../cursor/cursorAcpRemoteLauncher.test.ts | 109 ++++++++- cli/src/cursor/cursorAcpRemoteLauncher.ts | 80 ++++++- 4 files changed, 494 insertions(+), 20 deletions(-) diff --git a/cli/src/agent/backends/acp/AcpStdioTransport.test.ts b/cli/src/agent/backends/acp/AcpStdioTransport.test.ts index c800ee85..57506b48 100644 --- a/cli/src/agent/backends/acp/AcpStdioTransport.test.ts +++ b/cli/src/agent/backends/acp/AcpStdioTransport.test.ts @@ -7,6 +7,7 @@ const guard = vi.hoisted(() => ({ const spawnState = vi.hoisted(() => ({ exitHandlers: [] as Array<(code: number | null, signal: NodeJS.Signals | null) => void>, + closeHandlers: [] as Array<(code: number | null, signal: NodeJS.Signals | null) => void>, stdinWrite: vi.fn<(chunk: string) => boolean>(() => true), exitCode: null as number | null })); @@ -19,6 +20,7 @@ vi.mock('./agentCliGuard', () => ({ vi.mock('node:child_process', () => ({ spawn: vi.fn(() => { spawnState.exitHandlers = []; + spawnState.closeHandlers = []; const handlers = new Map void>>(); const proc = { get exitCode() { @@ -44,6 +46,9 @@ vi.mock('node:child_process', () => ({ if (event === 'exit') { spawnState.exitHandlers.push(handler as (code: number | null, signal: NodeJS.Signals | null) => void); } + if (event === 'close') { + spawnState.closeHandlers.push(handler as (code: number | null, signal: NodeJS.Signals | null) => void); + } handlers.set(`proc:${event}`, [...(handlers.get(`proc:${event}`) ?? []), handler]); }), kill: vi.fn() @@ -62,6 +67,7 @@ describe('AcpStdioTransport agent CLI guard', () => { spawnState.stdinWrite.mockReturnValue(true); spawnState.exitCode = null; spawnState.exitHandlers = []; + spawnState.closeHandlers = []; }); test('registers cross-process guard only for Cursor agent command', async () => { @@ -88,6 +94,28 @@ describe('AcpStdioTransport closed stdin writes', () => { spawnState.stdinWrite.mockReturnValue(true); spawnState.exitCode = null; spawnState.exitHandlers = []; + spawnState.closeHandlers = []; + }); + + test('rejects new requests after process exit before close without writing stdin', async () => { + const transport = new AcpStdioTransport({ command: 'gemini' }); + spawnState.exitCode = 1; + spawnState.stdinWrite.mockClear(); + + for (const handler of spawnState.exitHandlers) { + handler(1, null); + } + + await expect(transport.sendRequest('session/new')).rejects.toThrow( + 'ACP process exited (code=1, signal=null)' + ); + expect(spawnState.stdinWrite).not.toHaveBeenCalled(); + expect(() => transport.sendNotification('session/cancel', {})).not.toThrow(); + expect(spawnState.stdinWrite).not.toHaveBeenCalled(); + + for (const handler of spawnState.closeHandlers) { + handler(1, null); + } }); test('rejects new requests after the ACP process exits instead of throwing from stdin.write', async () => { @@ -97,7 +125,7 @@ describe('AcpStdioTransport closed stdin writes', () => { throw new Error('WritableIterable is closed'); }); - for (const handler of spawnState.exitHandlers) { + for (const handler of spawnState.closeHandlers) { handler(1, null); } @@ -107,6 +135,190 @@ describe('AcpStdioTransport closed stdin writes', () => { expect(() => transport.sendNotification('session/cancel', {})).not.toThrow(); }); + test('includes recent stderr on process close so callers can classify model rejection', async () => { + const transport = new AcpStdioTransport({ command: 'agent', args: ['acp'] }); + const proc = (transport as unknown as { process: { + stderr: { on: ReturnType }; + } }).process; + const stderrHandlers = (proc.stderr.on as ReturnType).mock.calls + .filter((call) => call[0] === 'data') + .map((call) => call[1] as (chunk: string) => void); + expect(stderrHandlers.length).toBeGreaterThan(0); + + for (const handler of stderrHandlers) { + handler('Cannot use this model: grok-4.5[fast=true]. Available models: auto, composer-2.5\n'); + } + + spawnState.exitCode = 1; + for (const handler of spawnState.closeHandlers) { + handler(1, null); + } + + await expect(transport.sendRequest('session/load')).rejects.toThrow( + /ACP process exited \(code=1, signal=null\)\. stderr: Cannot use this model: grok-4\.5\[fast=true\]/ + ); + }); + + test('accumulates split stderr chunks so Cannot use this model survives a catalog follow-up chunk', async () => { + const transport = new AcpStdioTransport({ command: 'agent', args: ['acp'] }); + const proc = (transport as unknown as { process: { + stderr: { on: ReturnType }; + } }).process; + const stderrHandlers = (proc.stderr.on as ReturnType).mock.calls + .filter((call) => call[0] === 'data') + .map((call) => call[1] as (chunk: string) => void); + + for (const handler of stderrHandlers) { + handler('Cannot use this model: grok-4.5[fast=true]. Available models: auto, '); + handler('composer-2.5, cursor-grok-4.5-high-fast\n'); + } + + spawnState.exitCode = 1; + for (const handler of spawnState.closeHandlers) { + handler(1, null); + } + + await expect(transport.sendRequest('session/load')).rejects.toThrow( + /Cannot use this model: grok-4\.5\[fast=true\][\s\S]*Available models:[\s\S]*composer-2\.5/ + ); + }); + + test('preserves Cannot use this model when the keyword itself is split across chunks', async () => { + const transport = new AcpStdioTransport({ command: 'agent', args: ['acp'] }); + const proc = (transport as unknown as { process: { + stderr: { on: ReturnType }; + } }).process; + const stderrHandlers = (proc.stderr.on as ReturnType).mock.calls + .filter((call) => call[0] === 'data') + .map((call) => call[1] as (chunk: string) => void); + + const seen: Array<{ type: string; message: string }> = []; + transport.onStderrError((error) => { + seen.push({ type: error.type, message: error.message }); + }); + + for (const handler of stderrHandlers) { + handler('Cannot use this mo'); + handler('del: grok-4.5[fast=true]. Available models: auto\n'); + } + + spawnState.exitCode = 1; + for (const handler of spawnState.closeHandlers) { + handler(1, null); + } + + await expect(transport.sendRequest('session/load')).rejects.toThrow( + /Cannot use this model: grok-4\.5\[fast=true\]/ + ); + expect(seen.some((entry) => /Cannot use this model: grok-4\.5\[fast=true\]/.test(entry.message))).toBe(true); + }); + + test('waits for the model id before emitting Cannot use this model via onStderrError', async () => { + const transport = new AcpStdioTransport({ command: 'agent', args: ['acp'] }); + const proc = (transport as unknown as { process: { + stderr: { on: ReturnType }; + } }).process; + const stderrHandlers = (proc.stderr.on as ReturnType).mock.calls + .filter((call) => call[0] === 'data') + .map((call) => call[1] as (chunk: string) => void); + + const seen: string[] = []; + transport.onStderrError((error) => { + seen.push(error.message); + }); + + for (const handler of stderrHandlers) { + handler('Cannot use this model: '); + expect(seen).toEqual([]); + handler('stale-id. Available models: auto\n'); + } + + expect(seen).toEqual([ + 'Cannot use this model: stale-id. Available models: auto' + ]); + + spawnState.exitCode = 1; + for (const handler of spawnState.closeHandlers) { + handler(1, null); + } + await expect(transport.sendRequest('session/load')).rejects.toThrow( + /Cannot use this model: stale-id/ + ); + }); + + test('pins Cannot use this model head when Available models catalog exceeds the rolling window', async () => { + const transport = new AcpStdioTransport({ command: 'agent', args: ['acp'] }); + const proc = (transport as unknown as { process: { + stderr: { on: ReturnType }; + } }).process; + const stderrHandlers = (proc.stderr.on as ReturnType).mock.calls + .filter((call) => call[0] === 'data') + .map((call) => call[1] as (chunk: string) => void); + + const hugeCatalog = Array.from({ length: 2_000 }, (_, i) => `model-${i}`).join(', '); + for (const handler of stderrHandlers) { + handler(`Cannot use this model: stale-id. Available models: ${hugeCatalog}\n`); + } + + spawnState.exitCode = 1; + for (const handler of spawnState.closeHandlers) { + handler(1, null); + } + + await expect(transport.sendRequest('session/load')).rejects.toThrow( + /Cannot use this model: stale-id/ + ); + }); + + test('keeps the head of long stderr so Cannot use this model survives Available models lists', async () => { + const transport = new AcpStdioTransport({ command: 'agent', args: ['acp'] }); + const proc = (transport as unknown as { process: { + stderr: { on: ReturnType }; + } }).process; + const stderrHandlers = (proc.stderr.on as ReturnType).mock.calls + .filter((call) => call[0] === 'data') + .map((call) => call[1] as (chunk: string) => void); + + const longCatalog = Array.from({ length: 400 }, (_, i) => `model-${i}`).join(', '); + for (const handler of stderrHandlers) { + handler(`Cannot use this model: stale-id. Available models: ${longCatalog}\n`); + } + + spawnState.exitCode = 1; + for (const handler of spawnState.closeHandlers) { + handler(1, null); + } + + await expect(transport.sendRequest('session/load')).rejects.toThrow( + /Cannot use this model: stale-id/ + ); + }); + + test('reports Cannot use this model stderr via onStderrError with Cursor text intact', () => { + const transport = new AcpStdioTransport({ command: 'agent', args: ['acp'] }); + const seen: Array<{ type: string; message: string; raw: string }> = []; + transport.onStderrError((error) => { + seen.push(error); + }); + + const proc = (transport as unknown as { process: { + stderr: { on: ReturnType }; + } }).process; + const stderrHandlers = (proc.stderr.on as ReturnType).mock.calls + .filter((call) => call[0] === 'data') + .map((call) => call[1] as (chunk: string) => void); + + for (const handler of stderrHandlers) { + handler('Cannot use this model: grok-4.5[fast=true]. Available models: auto\n'); + } + + expect(seen).toEqual([{ + type: 'model_not_found', + message: 'Cannot use this model: grok-4.5[fast=true]. Available models: auto', + raw: 'Cannot use this model: grok-4.5[fast=true]. Available models: auto' + }]); + }); + test('rejects pending requests when stdin.write throws', async () => { spawnState.stdinWrite.mockImplementation(() => { throw new Error('WritableIterable is closed'); diff --git a/cli/src/agent/backends/acp/AcpStdioTransport.ts b/cli/src/agent/backends/acp/AcpStdioTransport.ts index e157bc82..42020632 100644 --- a/cli/src/agent/backends/acp/AcpStdioTransport.ts +++ b/cli/src/agent/backends/acp/AcpStdioTransport.ts @@ -60,11 +60,21 @@ export class AcpStdioTransport { private notificationHandler: ((method: string, params: unknown) => void) | null = null; private stderrErrorHandler: ((error: AcpStderrError) => void) | null = null; private buffer = ''; + private recentStderr = ''; + private emittedModelRejection = false; private nextId = 1; private protocolError: Error | null = null; private guardReleased = false; private closed = false; private closeError: Error | null = null; + /** True after process 'exit'; blocks new writes until 'close' drains stderr. */ + private exited = false; + private exitError: Error | null = null; + + /** Rolling join window for stderr before close-time classification. */ + private static readonly RECENT_STDERR_WINDOW = 8_000; + /** Max stderr attached to the close Error (prefer model-rejection head). */ + private static readonly CLOSE_STDERR_CAP = 4_000; constructor(options: { command: string; @@ -87,16 +97,63 @@ export class AcpStdioTransport { this.process.stderr.setEncoding('utf8'); this.process.stderr.on('data', (chunk) => { - const text = chunk.toString().trim(); + // Chunks are arbitrary byte slices — concatenate raw, do not inject + // separators (a mid-word split would otherwise break keyword match). + const raw = chunk.toString(); + if (raw) { + const next = this.recentStderr + raw; + const matchIdx = next.search(/Cannot use this model:/i); + if (matchIdx >= 0) { + // Pin from the rejection head so a long Available models catalog + // cannot roll `Cannot use this model: ` out of the window. + const modelStderr = next.slice(matchIdx); + this.recentStderr = modelStderr.length > AcpStdioTransport.RECENT_STDERR_WINDOW + ? modelStderr.slice(0, AcpStdioTransport.RECENT_STDERR_WINDOW) + : modelStderr; + } else { + this.recentStderr = next.length > AcpStdioTransport.RECENT_STDERR_WINDOW + ? next.slice(-AcpStdioTransport.RECENT_STDERR_WINDOW) + : next; + } + } + const text = raw.trim(); logger.debug(`[ACP][stderr] ${text}`); this.parseStderrError(text); + // If this chunk alone missed a split keyword/id, retry against the window. + if ( + text + && !/Cannot use this model:\s*\S/i.test(text) + && /Cannot use this model:\s*\S/i.test(this.recentStderr) + ) { + this.parseStderrError(this.stderrForCloseError() ?? this.recentStderr); + } }); + // Block new stdin writes as soon as the process exits, but defer markClosed + // until 'close' so final stderr chunks can still enrich the failure. this.process.on('exit', (code, signal) => { this.releaseAgentCliGuard(); - const message = `ACP process exited (code=${code ?? 'null'}, signal=${signal ?? 'null'})`; + this.exited = true; + this.exitError = new Error( + `ACP process exited (code=${code ?? 'null'}, signal=${signal ?? 'null'})` + ); + }); + + // Use 'close' (not only 'exit') so final stderr chunks are drained before we + // classify the failure — Node may fire 'exit' before the last stderr 'data'. + this.process.on('close', (code, signal) => { + this.releaseAgentCliGuard(); + const stderr = this.stderrForCloseError(); + let message = `ACP process exited (code=${code ?? 'null'}, signal=${signal ?? 'null'})`; + if (stderr) { + message = `${message}. stderr: ${stderr}`; + } logger.debug(message); - this.markClosed(new Error(message)); + const error = new Error(message); + if (stderr) { + (error as Error & { stderr?: string }).stderr = stderr; + } + this.markClosed(error); }); this.process.on('error', (error) => { @@ -126,8 +183,10 @@ export class AcpStdioTransport { static readonly DEFAULT_TIMEOUT_MS = 120_000; async sendRequest(method: string, params?: unknown, options?: { timeoutMs?: number }): Promise { - if (this.closed) { - return Promise.reject(this.closeError ?? new Error('ACP transport is closed')); + if (this.closed || this.exited) { + return Promise.reject( + this.closeError ?? this.exitError ?? new Error('ACP transport is closed') + ); } const id = this.nextId++; @@ -173,7 +232,7 @@ export class AcpStdioTransport { } sendNotification(method: string, params?: unknown): void { - if (this.closed) { + if (this.closed || this.exited) { return; } @@ -342,6 +401,26 @@ export class AcpStdioTransport { this.pending.clear(); } + /** + * Prefer Cursor model-rejection text when present in the rolling stderr window. + * Cap from the match start so `Cannot use this model: ` survives long catalogs. + */ + private stderrForCloseError(): string | null { + if (!this.recentStderr) { + return null; + } + const matchIdx = this.recentStderr.search(/Cannot use this model:/i); + const source = matchIdx >= 0 + ? this.recentStderr.slice(matchIdx).trim() + : this.recentStderr.trim(); + if (!source) { + return null; + } + return source.length > AcpStdioTransport.CLOSE_STDERR_CAP + ? source.slice(0, AcpStdioTransport.CLOSE_STDERR_CAP) + : source; + } + private parseStderrError(text: string): void { if (!this.stderrErrorHandler) { return; @@ -349,6 +428,26 @@ export class AcpStdioTransport { const lowerText = text.toLowerCase(); + // Cursor rejects `--model` / config ids with this exact stderr shape. + // Require at least one non-space after the colon so a split before the + // model id does not emit a partial line and suppress the completed one. + // Pass the agent text through (including any Available models hint); do not + // invent a Gemini-style catalog here. + const modelRejection = text.match(/Cannot use this model:\s*\S[\s\S]*/i); + if (modelRejection) { + if (this.emittedModelRejection) { + return; + } + const message = modelRejection[0].trim(); + this.emittedModelRejection = true; + this.stderrErrorHandler({ + type: 'model_not_found', + message, + raw: message + }); + return; + } + // Rate limit errors (429) if (lowerText.includes('status 429') || lowerText.includes('ratelimitexceeded') || lowerText.includes('rate limit')) { this.stderrErrorHandler({ diff --git a/cli/src/cursor/cursorAcpRemoteLauncher.test.ts b/cli/src/cursor/cursorAcpRemoteLauncher.test.ts index 55a3d3d7..1f0656ec 100644 --- a/cli/src/cursor/cursorAcpRemoteLauncher.test.ts +++ b/cli/src/cursor/cursorAcpRemoteLauncher.test.ts @@ -135,7 +135,7 @@ vi.mock('@/ui/logger', () => ({ logger: { debug: vi.fn(), warn: vi.fn(), info: vi.fn() } })); -import { cursorAcpRemoteLauncher } from './cursorAcpRemoteLauncher'; +import { classifyCursorAcpLoadError, cursorAcpRemoteLauncher } from './cursorAcpRemoteLauncher'; import { createCursorAcpBackend } from './utils/cursorAcpBackend'; import { CursorSession } from './session'; import { ApiSessionClient } from '@/api/apiSession'; @@ -218,13 +218,28 @@ describe('cursorAcpRemoteLauncher', () => { await expect(cursorAcpRemoteLauncher(session)).rejects.toThrow( /Cursor ACP mode is required for new Cursor remote sessions/ ); - - expect(client.sendAgentMessage).toHaveBeenCalledWith({ - type: 'error', - message: expect.stringContaining('agent acp not found') - }); expect(legacyLauncher).not.toHaveBeenCalled(); - expect(harness.newSessionCalled).toBe(false); + expect(client.sendAgentMessage).toHaveBeenCalled(); + }); + + it('surfaces Cursor model rejection during initialize instead of the generic ACP-required message', async () => { + harness.initializeError = new Error( + 'ACP process exited (code=1, signal=null). stderr: Cannot use this model: grok-4.5[fast=true]. Available models: auto' + ); + const session = makeSession(null); + + const error = await cursorAcpRemoteLauncher(session).then( + () => null, + (err: unknown) => err + ); + expect(error).toBeInstanceOf(Error); + expect((error as Error).message).toMatch( + /^Failed to start Cursor ACP session: Cannot use this model: grok-4\.5\[fast=true\]/ + ); + expect((error as Error).message).toMatch(/Available models: auto/); + expect((error as Error).message).not.toMatch(/Cursor ACP mode is required/); + expect((error as Error).message).not.toMatch(/Legacy stream-json/); + expect(legacyLauncher).not.toHaveBeenCalled(); }); it('registers cursorSessionId before session/load completes', async () => { @@ -252,15 +267,38 @@ describe('cursorAcpRemoteLauncher', () => { harness.loadSessionError = new Error('session not found'); const session = makeSession('old-stream-json-id'); - await expect(cursorAcpRemoteLauncher(session)).rejects.toThrow( - /Legacy stream-json sessions cannot be loaded via ACP/ + const error = await cursorAcpRemoteLauncher(session).then( + () => null, + (err: unknown) => err ); + expect(error).toBeInstanceOf(Error); + expect((error as Error).message).toMatch(/Failed to resume Cursor ACP session: session not found/); + expect((error as Error).message).not.toMatch(/Legacy stream-json/); expect(harness.loadSessionCalled).toBe(true); expect(harness.newSessionCalled).toBe(false); expect(legacyLauncher).not.toHaveBeenCalled(); }); + it('surfaces Cursor model rejection from session/load instead of claiming legacy protocol', async () => { + harness.loadSessionError = new Error( + 'ACP process exited (code=1, signal=null). stderr: Cannot use this model: grok-4.5[fast=true]. Available models: auto, cursor-grok-4.5-high-fast' + ); + const session = makeSession('acp-thread-1'); + + const error = await cursorAcpRemoteLauncher(session).then( + () => null, + (err: unknown) => err + ); + expect(error).toBeInstanceOf(Error); + expect((error as Error).message).toMatch(/Cannot use this model: grok-4\.5\[fast=true\]/); + expect((error as Error).message).toMatch(/Available models: auto, cursor-grok-4\.5-high-fast/); + expect((error as Error).message).not.toMatch(/Legacy stream-json/); + + expect(harness.newSessionCalled).toBe(false); + expect(legacyLauncher).not.toHaveBeenCalled(); + }); + it('throws when resume id is set but session/load is unsupported', async () => { harness.supportsLoadSession = false; const session = makeSession('some-session-id'); @@ -297,12 +335,63 @@ describe('cursorAcpRemoteLauncher', () => { const session = makeSession('old-stream-json-id'); await expect(cursorAcpRemoteLauncher(session)).rejects.toThrow( - /Legacy stream-json sessions cannot be loaded via ACP/ + /Failed to resume Cursor ACP session: session not found/ ); expect(session.client.emitSessionReady).not.toHaveBeenCalled(); }); + describe('classifyCursorAcpLoadError', () => { + it('prefers Cannot use this model text from the underlying error', () => { + const message = classifyCursorAcpLoadError( + new Error('ACP process exited (code=1, signal=null). stderr: Cannot use this model: grok-4.5[fast=true]. Available models: auto, composer-2.5') + ); + expect(message).toContain('Cannot use this model: grok-4.5[fast=true]'); + expect(message).toContain('Available models: auto, composer-2.5'); + expect(message).not.toMatch(/Legacy stream-json/); + }); + + it('uses recentStderr hint when exit error omits the model line', () => { + const message = classifyCursorAcpLoadError( + new Error('ACP process exited (code=1, signal=null)'), + { recentStderr: 'Cannot use this model: stale-id. Available models: auto' } + ); + expect(message).toContain('Cannot use this model: stale-id'); + expect(message).toContain('Available models: auto'); + expect(message).not.toMatch(/Legacy stream-json/); + }); + + it('prefers accumulated close stderr over a partial recentStderr hint', () => { + const message = classifyCursorAcpLoadError( + new Error( + 'ACP process exited (code=1, signal=null). stderr: Cannot use this model: full-id. Available models: auto, composer-2.5' + ), + { recentStderr: 'Cannot use this mo' } + ); + expect(message).toContain('Cannot use this model: full-id'); + expect(message).toContain('Available models: auto, composer-2.5'); + expect(message).not.toContain('Cannot use this mo:'); + }); + + it('propagates generic load failures without inventing a legacy diagnosis', () => { + const message = classifyCursorAcpLoadError(new Error('Session "abc" not found')); + expect(message).toBe('Failed to resume Cursor ACP session: Session "abc" not found'); + expect(message).not.toMatch(/Legacy stream-json/); + }); + + it('uses start action prefix for spawn-time model rejection', () => { + const message = classifyCursorAcpLoadError( + new Error('ACP process exited (code=1, signal=null)'), + { + recentStderr: 'Cannot use this model: stale-id. Available models: auto', + action: 'start' + } + ); + expect(message).toMatch(/^Failed to start Cursor ACP session: Cannot use this model: stale-id/); + expect(message).not.toMatch(/Failed to resume/); + }); + }); + // tiann/hapi#913: fresh ACP sessions previously persisted `cursorSessionId` // via fire-and-forget `updateMetadata`. A SIGTERM within ~1s of the first // turn (hub-restart cascade) could strand the session because the ACK diff --git a/cli/src/cursor/cursorAcpRemoteLauncher.ts b/cli/src/cursor/cursorAcpRemoteLauncher.ts index 8cbe7af2..7ec75ec1 100644 --- a/cli/src/cursor/cursorAcpRemoteLauncher.ts +++ b/cli/src/cursor/cursorAcpRemoteLauncher.ts @@ -91,8 +91,10 @@ class CursorAcpRemoteLauncher extends RemoteLauncherBase { backend.setUsageUpdateListener((message) => this.handleAgentMessage(message)); + let recentStderrHint: string | null = null; backend.onStderrError((error) => { logger.debug('[cursor-acp] stderr error', error); + recentStderrHint = error.raw || error.message; const converted = convertAgentMessage({ type: 'error', message: error.message }); if (converted) { session.sendAgentMessage(converted); @@ -104,6 +106,20 @@ class CursorAcpRemoteLauncher extends RemoteLauncherBase { await backend.initialize(); } catch (error) { const errMsg = error instanceof Error ? error.message : String(error); + const modelRejection = extractCannotUseThisModelMessage(errMsg) + ?? extractCannotUseThisModelMessage(recentStderrHint); + if (modelRejection) { + const fullMsg = classifyCursorAcpLoadError(error, { + recentStderr: recentStderrHint, + action: 'start' + }); + const converted = convertAgentMessage({ type: 'error', message: fullMsg }); + if (converted) { + session.sendAgentMessage(converted); + } + messageBuffer.addMessage(fullMsg, 'status'); + throw new Error(fullMsg); + } const fullMsg = `${CURSOR_ACP_REQUIRED_MESSAGE} (${errMsg})`; const converted = convertAgentMessage({ type: 'error', message: fullMsg }); if (converted) { @@ -145,9 +161,7 @@ class CursorAcpRemoteLauncher extends RemoteLauncherBase { }); } catch (error) { logger.warn('[cursor-acp] session/load failed', formatAcpLoadError(error)); - throw new Error( - 'Failed to resume Cursor ACP session. Legacy stream-json sessions cannot be loaded via ACP.' - ); + throw new Error(classifyCursorAcpLoadError(error, { recentStderr: recentStderrHint })); } } else if (resumeSessionId) { throw new Error( @@ -578,6 +592,62 @@ class CursorAcpRemoteLauncher extends RemoteLauncherBase { } } +const CANNOT_USE_THIS_MODEL_RE = /Cannot use this model:\s*.+/i; + +/** + * Operator-facing ACP failure text. Prefer Cursor's model-rejection stderr; + * never invent a legacy stream-json diagnosis for unrelated failures. + */ +export function classifyCursorAcpLoadError( + error: unknown, + options?: { recentStderr?: string | null; action?: 'resume' | 'start' } +): string { + const action = options?.action ?? 'resume'; + const prefix = action === 'start' + ? 'Failed to start Cursor ACP session' + : 'Failed to resume Cursor ACP session'; + + const detailSources = [ + // Prefer the close Error (accumulated stderr) over live onStderrError hints, + // which may have seen only the first fragment of a split rejection line. + error instanceof Error ? error.message : null, + error instanceof Error ? String((error as Error & { stderr?: unknown }).stderr ?? '') : null, + error instanceof Error && error.cause instanceof Error ? error.cause.message : null, + options?.recentStderr, + typeof error === 'string' ? error : null + ].filter((value): value is string => Boolean(value && value.trim())); + + for (const source of detailSources) { + const modelRejection = extractCannotUseThisModelMessage(source); + if (modelRejection) { + return `${prefix}: ${modelRejection}`; + } + } + + const detail = error instanceof Error + ? error.message + : typeof error === 'string' + ? error + : String(error); + const trimmed = detail.trim() || 'unknown error'; + if (new RegExp(`^${prefix}:`, 'i').test(trimmed)) { + return trimmed; + } + return `${prefix}: ${trimmed}`; +} + +function extractCannotUseThisModelMessage(text: string | null | undefined): string | null { + if (!text) { + return null; + } + const match = text.match(CANNOT_USE_THIS_MODEL_RE); + if (!match) { + return null; + } + // Keep Cursor's Available models hint when present; do not invent a catalog. + return match[0].trim().replace(/\s+/g, ' '); +} + function formatAcpLoadError(error: unknown): Record { if (error instanceof Error) { const record: Record = { @@ -592,6 +662,10 @@ function formatAcpLoadError(error: unknown): Record { if (data !== undefined) { record.data = data; } + const stderr = (error as Error & { stderr?: unknown }).stderr; + if (stderr !== undefined) { + record.stderr = stderr; + } const cause = error.cause; if (cause !== undefined) { record.cause = cause instanceof Error