mirror of
https://github.com/wu736139669/hapi.git
synced 2026-08-05 06:24:37 +00:00
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 <cursoragent@cursor.com> * 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 <cursoragent@cursor.com> * 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 <cursoragent@cursor.com> * 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 <cursoragent@cursor.com> * 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 <cursoragent@cursor.com> --------- Co-authored-by: Cursor <cursoragent@cursor.com>
This commit is contained in:
@@ -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<string, Array<(...args: unknown[]) => 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<typeof vi.fn> };
|
||||
} }).process;
|
||||
const stderrHandlers = (proc.stderr.on as ReturnType<typeof vi.fn>).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<typeof vi.fn> };
|
||||
} }).process;
|
||||
const stderrHandlers = (proc.stderr.on as ReturnType<typeof vi.fn>).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<typeof vi.fn> };
|
||||
} }).process;
|
||||
const stderrHandlers = (proc.stderr.on as ReturnType<typeof vi.fn>).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<typeof vi.fn> };
|
||||
} }).process;
|
||||
const stderrHandlers = (proc.stderr.on as ReturnType<typeof vi.fn>).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<typeof vi.fn> };
|
||||
} }).process;
|
||||
const stderrHandlers = (proc.stderr.on as ReturnType<typeof vi.fn>).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<typeof vi.fn> };
|
||||
} }).process;
|
||||
const stderrHandlers = (proc.stderr.on as ReturnType<typeof vi.fn>).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<typeof vi.fn> };
|
||||
} }).process;
|
||||
const stderrHandlers = (proc.stderr.on as ReturnType<typeof vi.fn>).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');
|
||||
|
||||
@@ -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: <id>` 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<unknown> {
|
||||
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: <id>` 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({
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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<string, unknown> {
|
||||
if (error instanceof Error) {
|
||||
const record: Record<string, unknown> = {
|
||||
@@ -592,6 +662,10 @@ function formatAcpLoadError(error: unknown): Record<string, unknown> {
|
||||
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
|
||||
|
||||
Reference in New Issue
Block a user