fix(core): prevent duplicate MCP processes from concurrent discovery

.serena/.gitignore

@@ -0,0 +1,3 @@
+/cache
+/project.local.yml
+/project.yml

packages/core/src/core/geminiChat.test.ts

@@ -1700,6 +1700,180 @@ describe('GeminiChat', async () => {
         ).toHaveBeenCalledTimes(2);
       });
 
+      it('should retry on 408 Request Timeout errors', async () => {
+        const error408 = new ApiError({
+          message: 'Request Timeout',
+          status: 408,
+        });
+
+        vi.mocked(mockContentGenerator.generateContentStream)
+          .mockRejectedValueOnce(error408)
+          .mockResolvedValueOnce(
+            (async function* () {
+              yield {
+                candidates: [
+                  {
+                    content: { parts: [{ text: 'Recovered from 408' }] },
+                    finishReason: 'STOP',
+                  },
+                ],
+              } as unknown as GenerateContentResponse;
+            })(),
+          );
+
+        const stream = await chat.sendMessageStream(
+          'test-model',
+          { message: 'test' },
+          'prompt-id-408-retry',
+        );
+
+        const events: StreamEvent[] = [];
+        for await (const event of stream) {
+          events.push(event);
+        }
+
+        // Should be called twice (initial + retry)
+        expect(
+          mockContentGenerator.generateContentStream,
+        ).toHaveBeenCalledTimes(2);
+
+        // Should have successful content
+        expect(
+          events.some(
+            (e) =>
+              e.type === StreamEventType.CHUNK &&
+              e.value.candidates?.[0]?.content?.parts?.[0]?.text ===
+                'Recovered from 408',
+          ),
+        ).toBe(true);
+      });
+
+      it('should retry on 409 transient conflict errors', async () => {
+        const error409 = new ApiError({
+          message: 'Lock contention detected',
+          status: 409,
+        });
+
+        vi.mocked(mockContentGenerator.generateContentStream)
+          .mockRejectedValueOnce(error409)
+          .mockResolvedValueOnce(
+            (async function* () {
+              yield {
+                candidates: [
+                  {
+                    content: { parts: [{ text: 'Recovered from 409' }] },
+                    finishReason: 'STOP',
+                  },
+                ],
+              } as unknown as GenerateContentResponse;
+            })(),
+          );
+
+        const stream = await chat.sendMessageStream(
+          'test-model',
+          { message: 'test' },
+          'prompt-id-409-retry',
+        );
+
+        const events: StreamEvent[] = [];
+        for await (const event of stream) {
+          events.push(event);
+        }
+
+        // Should be called twice (initial + retry)
+        expect(
+          mockContentGenerator.generateContentStream,
+        ).toHaveBeenCalledTimes(2);
+
+        expect(
+          events.some(
+            (e) =>
+              e.type === StreamEventType.CHUNK &&
+              e.value.candidates?.[0]?.content?.parts?.[0]?.text ===
+                'Recovered from 409',
+          ),
+        ).toBe(true);
+      });
+
+      it('should NOT retry on 409 deterministic conflict errors', async () => {
+        const error409 = new ApiError({
+          message: 'Resource already exists',
+          status: 409,
+        });
+
+        vi.mocked(mockContentGenerator.generateContentStream).mockRejectedValue(
+          error409,
+        );
+
+        const stream = await chat.sendMessageStream(
+          'test-model',
+          { message: 'test' },
+          'prompt-id-409-noretry',
+        );
+
+        await expect(
+          (async () => {
+            for await (const _ of stream) {
+              /* consume stream */
+            }
+          })(),
+        ).rejects.toThrow(error409);
+
+        // Should only be called once (no retry)
+        expect(
+          mockContentGenerator.generateContentStream,
+        ).toHaveBeenCalledTimes(1);
+      });
+
+      it('should retry on network errors (ECONNRESET)', async () => {
+        const networkError = new Error(
+          'Connection reset',
+        ) as NodeJS.ErrnoException;
+        networkError.code = 'ECONNRESET';
+
+        vi.mocked(mockContentGenerator.generateContentStream)
+          .mockRejectedValueOnce(networkError)
+          .mockResolvedValueOnce(
+            (async function* () {
+              yield {
+                candidates: [
+                  {
+                    content: {
+                      parts: [{ text: 'Recovered from network error' }],
+                    },
+                    finishReason: 'STOP',
+                  },
+                ],
+              } as unknown as GenerateContentResponse;
+            })(),
+          );
+
+        const stream = await chat.sendMessageStream(
+          'test-model',
+          { message: 'test' },
+          'prompt-id-network-retry',
+        );
+
+        const events: StreamEvent[] = [];
+        for await (const event of stream) {
+          events.push(event);
+        }
+
+        // Should be called twice (initial + retry)
+        expect(
+          mockContentGenerator.generateContentStream,
+        ).toHaveBeenCalledTimes(2);
+
+        expect(
+          events.some(
+            (e) =>
+              e.type === StreamEventType.CHUNK &&
+              e.value.candidates?.[0]?.content?.parts?.[0]?.text ===
+                'Recovered from network error',
+          ),
+        ).toBe(true);
+      });
+
       afterEach(() => {
         // Reset to default behavior
         mockRetryWithBackoff.mockImplementation(async (apiCall) => apiCall());

packages/core/src/core/geminiChat.ts

@@ -17,8 +17,11 @@ import type {
   GenerateContentResponseUsageMetadata,
 } from '@google/genai';
 import { createUserContent, FinishReason } from '@google/genai';
-import { retryWithBackoff, isUnattendedMode } from '../utils/retry.js';
-import { getErrorStatus } from '../utils/errors.js';
+import {
+  retryWithBackoff,
+  isUnattendedMode,
+  classifyError,
+} from '../utils/retry.js';
 import { createDebugLogger } from '../utils/debugLogger.js';
 import { parseAndFormatApiError } from '../utils/errorParsing.js';
 import { isRateLimitError, type RetryInfo } from '../utils/rateLimit.js';
@@ -713,17 +716,28 @@ export class GeminiChat {
       );
     const streamResponse = await retryWithBackoff(apiCall, {
       shouldRetryOnError: (error: unknown) => {
+        // Independent safety-net guards not covered by classifyError:
+        // never retry schema-depth-limit or invalid-argument errors regardless
+        // of what classifyError returns.
         if (error instanceof Error) {
           if (isSchemaDepthError(error.message)) return false;
           if (isInvalidArgumentError(error.message)) return false;
         }
 
-        const status = getErrorStatus(error);
-        if (status === 400) return false;
-        if (status === 429) return true;
-        if (status && status >= 500 && status < 600) return true;
-
-        return false;
+        // isSchemaDepthError / isInvalidArgumentError above are independent
+        // safety nets not covered by classifyError — do not remove them.
+        // Delegate to classifyError for all remaining cases. Explicitly accepted
+        // retryable categories for Gemini streaming: 408 (timeout), 409 (transient
+        // lock/contention only), 429 (rate limit), 5xx (server errors), network
+        // transport errors. Deterministic errors (400, 401, 403, 404, 422) are
+        // handled by classifyError and return retryable=false.
+        const classification = classifyError(error);
+        if (classification.retryable) {
+          debugLogger.debug(
+            `geminiChat retrying: status=${classification.status ?? 'none'}, reason=${classification.reason}`,
+          );
+        }
+        return classification.retryable;
       },
       authType: this.config.getContentGeneratorConfig()?.authType,
       persistentMode: isUnattendedMode(),