fix(cursor): requeue user message on transient agent exit (auth, rate limit) (#823)

* fix(cursor): requeue user message on transient agent exit (auth, rate limit) cursorLegacyRemoteLauncher.runMainLoop popped a user message off the queue before spawning `agent` and silently discarded it whenever `agent` exited non-zero (auth expiry, rate limit, transient network). The wrapper logged the failure at debug level only, never surfaced it to the web UI, and emitted `ready` as if a normal turn had ended. Capture stderr from the spawned process; classify exit-1 with a transient signature (Authentication required, rate limit, ETIMEDOUT, ECONNRESET, EAI_AGAIN) as recoverable; re-head the message via `queue.unshift`, surface a friendly banner via `sendSessionEvent({type:'message',...})`, and backoff ~2s before the loop picks it up again. Cap at 5 consecutive transient failures, after which the message is dropped with a clear "resolve and resend" event so we never spin forever on a genuinely broken auth. Non-transient non-zero exits also surface the stderr to the UI now (instead of only the local ring buffer), so a real crash is visible to the operator. Backoff is overridable via CURSOR_LEGACY_TRANSIENT_BACKOFF_MS for tests. Tests cover: success path, transient auth requeue + banner, rate-limit banner, non-transient crash surfaced without requeue, and the 5-failure drop cap. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(cursor): preserve slash-command isolation on requeue; wait for stderr flush Two findings from the cold-review bot on the requeue path: 1. `enqueueCursorUserMessage` uses `pushIsolated` for pass-through slash commands (e.g. `/compress`) so they never batch with sibling prompts. The transient-requeue path used plain `unshift`, which dropped the isolate bit and allowed the next collected batch to merge the slash command with a sibling - changing command semantics. Add `MessageQueue2.unshiftIsolated` and use it when the popped batch was isolated or when `parseCursorSpecialCommand` recognises the message. 2. `runAgentProcess` resolved on `child.on('exit', ...)`. Node may emit `exit` while the stderr pipe is still draining, so a fast "auth required" error printed-and-exited could be classified as non-transient with empty stderr and silently drop the user message - the exact bug this PR was supposed to fix. Resolve on `close` instead, which waits for stdio streams to flush. Adds a unit test that requeues `/compress` after a transient auth failure and asserts the second spawn still receives the slash command alone (not batched with a sibling). Co-authored-by: Cursor <cursoragent@cursor.com> * fix(cursor): restrict transient retry to exit code 1 only Upstream codex review #823 (Minor): the helper treated any non-zero exit with matching stderr as transient, which could requeue a signal-killed (SIGTERM 143, SIGKILL 137) or crashed (SIGABRT 134) process whose stderr happens to contain a keyword like "rate limit". Documented contract is exit-1-for-transient; tighten the classifier accordingly. Adds regression test covering exit 143 + rate-limit stderr → no retry. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(cursor): clean up transientBackoff abort listener on timer completion Upstream codex review #823 (Minor): transientBackoff added an abort listener with { once: true } but only removed it when the abort fired. Because the launcher reuses one AbortController, repeated transient retries accumulated stale listeners until the next abort. Switch to a single completion path that clears the timer AND removes the abort listener whichever side wins. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(cursor): cap in-memory stderr capture at 8 KB Upstream codex review #823 (Minor): runAgentProcess accumulated every stderr chunk for the full child lifetime. A noisy `agent` failure could grow CLI process memory without bound even though only the first 400 chars are ever displayed. Cap the retained copy at 8 KB; debug log of the full stream is unchanged. Co-authored-by: Cursor <cursoragent@cursor.com> --------- Co-authored-by: Cursor <cursoragent@cursor.com>
2026-11-11 02:11:37 +00:00 · 2026-06-08 13:27:35 +08:00
parent 9349acba62
commit edc3acc3e2
3 changed files with 440 additions and 49 deletions
@@ -1,4 +1,5 @@
-import { describe, expect, it, vi, beforeEach } from 'vitest';
+import { describe, expect, it, vi, beforeEach, afterEach } from 'vitest';
+import { PassThrough } from 'node:stream';

 const spawnMock = vi.hoisted(() => vi.fn());

@@ -26,24 +27,62 @@ import { MessageQueue2 } from '@/utils/MessageQueue2';
 import { CursorSession } from './session';
 import type { EnhancedMode } from './loop';

-function makeChild() {
-    const stdoutHandlers: Array<(chunk: string) => void> = [];
-    return {
-        stdout: { on: vi.fn((event: string, handler: (chunk: string) => void) => {
-            if (event === 'data') stdoutHandlers.push(handler);
-        }) },
-        stderr: { on: vi.fn() },
+type ChildOptions = {
+    exitCode?: number | null;
+    stderr?: string;
+};
+
+function makeChild(opts: ChildOptions = {}) {
+    const stdout = new PassThrough();
+    const stderr = new PassThrough();
+    const child = {
+        stdout,
+        stderr,
        on: vi.fn((event: string, handler: (...args: unknown[]) => void) => {
-            if (event === 'exit') {
-                setImmediate(() => handler(0, null));
+            if (event === 'close') {
+                setImmediate(() => {
+                    if (opts.stderr) {
+                        // emit synchronously so runAgentProcess captures it before
+                        // the close handler resolves.
+                        stderr.emit('data', Buffer.from(opts.stderr));
+                    }
+                    handler(opts.exitCode ?? 0, null);
+                });
            }
        }),
        emitStdout(line: string) {
-            for (const handler of stdoutHandlers) {
-                handler(`${line}\n`);
-            }
+            stdout.write(`${line}\n`);
        }
    };
+    return child;
+}
+
+function makeClient() {
+    return {
+        rpcHandlerManager: { registerHandler: vi.fn() },
+        updateMetadata: vi.fn((handler: (m: Record<string, unknown>) => Record<string, unknown>) => {
+            handler({ path: '/tmp', host: 'h', flavor: 'cursor' });
+        }),
+        sendSessionEvent: vi.fn(),
+        sendAgentMessage: vi.fn(),
+        keepAlive: vi.fn(),
+        emitMessagesConsumed: vi.fn()
+    };
+}
+
+function makeSession(queue: MessageQueue2<EnhancedMode>, client: ReturnType<typeof makeClient>): CursorSession {
+    return new CursorSession({
+        api: {} as never,
+        client: client as never,
+        path: '/tmp/project',
+        logPath: '/tmp/log',
+        sessionId: 'legacy-id',
+        messageQueue: queue,
+        onModeChange: vi.fn(),
+        mode: 'remote',
+        startedBy: 'runner',
+        startingMode: 'remote'
+    });
 }

 describe('cursorLegacyRemoteLauncher', () => {
@@ -51,6 +90,11 @@ describe('cursorLegacyRemoteLauncher', () => {
        spawnMock.mockReset();
        process.stdin.isTTY = false;
        process.stdout.isTTY = false;
+        process.env.CURSOR_LEGACY_TRANSIENT_BACKOFF_MS = '0';
+    });
+
+    afterEach(() => {
+        delete process.env.CURSOR_LEGACY_TRANSIENT_BACKOFF_MS;
    });

    it('spawns agent with stream-json and trust, not acp', async () => {
@@ -61,30 +105,8 @@ describe('cursorLegacyRemoteLauncher', () => {
        queue.push('hello', { permissionMode: 'default' });
        queue.close();

-        const metadataUpdates: unknown[] = [];
-        const client = {
-            rpcHandlerManager: { registerHandler: vi.fn() },
-            updateMetadata: vi.fn((handler: (m: Record<string, unknown>) => Record<string, unknown>) => {
-                metadataUpdates.push(handler({ path: '/tmp', host: 'h', flavor: 'cursor' }));
-            }),
-            sendSessionEvent: vi.fn(),
-            sendAgentMessage: vi.fn(),
-            keepAlive: vi.fn(),
-            emitMessagesConsumed: vi.fn()
-        };
-
-        const session = new CursorSession({
-            api: {} as never,
-            client: client as never,
-            path: '/tmp/project',
-            logPath: '/tmp/log',
-            sessionId: 'legacy-id',
-            messageQueue: queue,
-            onModeChange: vi.fn(),
-            mode: 'remote',
-            startedBy: 'runner',
-            startingMode: 'remote'
-        });
+        const client = makeClient();
+        const session = makeSession(queue, client);

        const { cursorLegacyRemoteLauncher } = await import('./cursorLegacyRemoteLauncher');
        await cursorLegacyRemoteLauncher(session);
@@ -97,10 +119,203 @@ describe('cursorLegacyRemoteLauncher', () => {
        expect(args).toContain('--resume');
        expect(args).toContain('legacy-id');
        expect(args).not.toContain('acp');
+    });

-        expect(metadataUpdates[0]).toEqual(expect.objectContaining({
-            cursorSessionId: 'legacy-id',
-            cursorSessionProtocol: 'stream-json'
+    it('requeues the user message and surfaces an auth banner when agent exits with auth-required stderr', async () => {
+        const queue = new MessageQueue2<EnhancedMode>(() => 'm');
+        queue.push('do thing', { permissionMode: 'default' });
+
+        let call = 0;
+        spawnMock.mockImplementation(() => {
+            call += 1;
+            if (call === 1) {
+                return makeChild({
+                    exitCode: 1,
+                    stderr: "Error: Authentication required. Please run 'agent login' first\n"
+                });
+            }
+            queue.close();
+            return makeChild({ exitCode: 0 });
+        });
+
+        const client = makeClient();
+        const session = makeSession(queue, client);
+
+        const { cursorLegacyRemoteLauncher } = await import('./cursorLegacyRemoteLauncher');
+        await cursorLegacyRemoteLauncher(session);
+
+        expect(spawnMock).toHaveBeenCalledTimes(2);
+        const messages = client.sendSessionEvent.mock.calls
+            .map((c) => c[0])
+            .filter((e: any) => e.type === 'message');
+        expect(messages).toHaveLength(1);
+        expect(messages[0].message).toContain('Cursor authentication expired');
+        expect(messages[0].message).toContain("'agent login'");
+        expect(messages[0].message).toContain('queued and will retry');
+
+        const firstPrompt = spawnMock.mock.calls[0]?.[1] as string[];
+        const secondPrompt = spawnMock.mock.calls[1]?.[1] as string[];
+        const pIndex1 = firstPrompt.indexOf('-p');
+        const pIndex2 = secondPrompt.indexOf('-p');
+        expect(firstPrompt[pIndex1 + 1]).toBe('do thing');
+        expect(secondPrompt[pIndex2 + 1]).toBe('do thing');
+    });
+
+    it('uses a rate-limit-specific banner for rate limit stderr', async () => {
+        const queue = new MessageQueue2<EnhancedMode>(() => 'm');
+        queue.push('do thing', { permissionMode: 'default' });
+
+        let call = 0;
+        spawnMock.mockImplementation(() => {
+            call += 1;
+            if (call === 1) {
+                return makeChild({
+                    exitCode: 1,
+                    stderr: 'Error: rate limit exceeded, please retry later\n'
+                });
+            }
+            queue.close();
+            return makeChild({ exitCode: 0 });
+        });
+
+        const client = makeClient();
+        const session = makeSession(queue, client);
+
+        const { cursorLegacyRemoteLauncher } = await import('./cursorLegacyRemoteLauncher');
+        await cursorLegacyRemoteLauncher(session);
+
+        const messages = client.sendSessionEvent.mock.calls
+            .map((c) => c[0])
+            .filter((e: any) => e.type === 'message');
+        expect(messages).toHaveLength(1);
+        expect(messages[0].message).toContain('rate limit');
+        expect(messages[0].message).toContain('queued and will retry');
+    });
+
+    it('does not requeue when stderr is non-transient (real crash); surfaces error and emits ready', async () => {
+        const queue = new MessageQueue2<EnhancedMode>(() => 'm');
+        queue.push('do thing', { permissionMode: 'default' });
+        queue.close();
+
+        spawnMock.mockReturnValue(makeChild({
+            exitCode: 134,
+            stderr: 'fatal: Segmentation fault\n'
        }));
+
+        const client = makeClient();
+        const session = makeSession(queue, client);
+
+        const { cursorLegacyRemoteLauncher } = await import('./cursorLegacyRemoteLauncher');
+        await cursorLegacyRemoteLauncher(session);
+
+        expect(spawnMock).toHaveBeenCalledTimes(1);
+        const messageEvents = client.sendSessionEvent.mock.calls
+            .map((c) => c[0])
+            .filter((e: any) => e.type === 'message');
+        expect(messageEvents).toHaveLength(1);
+        expect(messageEvents[0].message).toContain('Agent exited (134)');
+        expect(messageEvents[0].message).toContain('Segmentation fault');
+        expect(messageEvents[0].message).not.toContain('queued and will retry');
+
+        const readyEvents = client.sendSessionEvent.mock.calls
+            .map((c) => c[0])
+            .filter((e: any) => e.type === 'ready');
+        expect(readyEvents).toHaveLength(1);
+    });
+
+    it('does not retry signal-killed processes even if stderr contains a transient keyword', async () => {
+        // SIGTERM → exit 143; stderr happens to mention rate limit. Should NOT be
+        // classified transient because the documented contract is exit-1-only.
+        const queue = new MessageQueue2<EnhancedMode>(() => 'm');
+        queue.push('do thing', { permissionMode: 'default' });
+        queue.close();
+
+        spawnMock.mockReturnValue(makeChild({
+            exitCode: 143,
+            stderr: 'rate limit hit; aborting due to SIGTERM\n'
+        }));
+
+        const client = makeClient();
+        const session = makeSession(queue, client);
+
+        const { cursorLegacyRemoteLauncher } = await import('./cursorLegacyRemoteLauncher');
+        await cursorLegacyRemoteLauncher(session);
+
+        expect(spawnMock).toHaveBeenCalledTimes(1);
+        const messageEvents = client.sendSessionEvent.mock.calls
+            .map((c) => c[0])
+            .filter((e: any) => e.type === 'message');
+        expect(messageEvents).toHaveLength(1);
+        expect(messageEvents[0].message).toContain('Agent exited (143)');
+        expect(messageEvents[0].message).not.toContain('queued and will retry');
+    });
+
+    it('preserves isolation when requeueing a slash command after a transient failure', async () => {
+        const queue = new MessageQueue2<EnhancedMode>(() => 'm');
+        // /compress is a pass-through slash command; enqueueCursorUserMessage uses
+        // pushIsolated for these so they never batch with sibling prompts.
+        queue.pushIsolated('/compress', { permissionMode: 'default' });
+
+        let call = 0;
+        spawnMock.mockImplementation(() => {
+            call += 1;
+            if (call === 1) {
+                return makeChild({
+                    exitCode: 1,
+                    stderr: "Error: Authentication required. Please run 'agent login' first\n"
+                });
+            }
+            queue.close();
+            return makeChild({ exitCode: 0 });
+        });
+
+        const client = makeClient();
+        const session = makeSession(queue, client);
+
+        const { cursorLegacyRemoteLauncher } = await import('./cursorLegacyRemoteLauncher');
+        await cursorLegacyRemoteLauncher(session);
+
+        expect(spawnMock).toHaveBeenCalledTimes(2);
+        // Confirm the queue still flagged the requeued item as isolated
+        // (the second collectBatch saw it alone with isolate=true).
+        const secondPrompt = spawnMock.mock.calls[1]?.[1] as string[];
+        const pIdx = secondPrompt.indexOf('-p');
+        expect(secondPrompt[pIdx + 1]).toBe('/compress');
+    });
+
+    it('drops the message after MAX_CONSECUTIVE_TRANSIENT_FAILURES consecutive transient failures', async () => {
+        const queue = new MessageQueue2<EnhancedMode>(() => 'm');
+        queue.push('do thing', { permissionMode: 'default' });
+
+        let call = 0;
+        spawnMock.mockImplementation(() => {
+            call += 1;
+            if (call >= 5) {
+                queue.close();
+            }
+            return makeChild({
+                exitCode: 1,
+                stderr: "Error: Authentication required. Please run 'agent login' first\n"
+            });
+        });
+
+        const client = makeClient();
+        const session = makeSession(queue, client);
+
+        const { cursorLegacyRemoteLauncher } = await import('./cursorLegacyRemoteLauncher');
+        await cursorLegacyRemoteLauncher(session);
+
+        expect(spawnMock).toHaveBeenCalledTimes(5);
+
+        const messageEvents = client.sendSessionEvent.mock.calls
+            .map((c) => c[0])
+            .filter((e: any) => e.type === 'message');
+        // 4 transient retry banners + 1 drop banner = 5
+        expect(messageEvents).toHaveLength(5);
+        const banners = messageEvents.map((e: any) => e.message);
+        expect(banners.filter((m: string) => m.includes('queued and will retry'))).toHaveLength(4);
+        const drop = banners.find((m: string) => m.includes('5 times in a row'));
+        expect(drop).toBeDefined();
+        expect(drop).toContain('Dropping the queued message');
    });
 });