From 2df8b234fec96dbac6d2f0311af0849e830cefdc Mon Sep 17 00:00:00 2001
From: Chris Staley <christopher-s@users.noreply.github.com>
Date: Tue, 31 Mar 2026 11:41:19 -0600
Subject: [PATCH] fix: address PR review feedback

- Deduplicate in-flight loadCodeAssist requests to prevent thundering herd
- Add typeof guard on cloudaicompanionProject.id before calling .trim()
- Evict oldest cache entry when all entries are still valid
- Fix unwrapGeminiChunk to use explicit null-safe check
- Update test description for null response case
---
 open-sse/executors/gemini-cli.ts  | 23 ++++++++++++++++++++++-
 open-sse/utils/streamHelpers.ts   |  5 ++++-
 tests/unit/streamHelpers.test.mjs |  2 +-
 3 files changed, 27 insertions(+), 3 deletions(-)

diff --git a/open-sse/executors/gemini-cli.ts b/open-sse/executors/gemini-cli.ts
index 0605da5994..f8b71e6623 100644
--- a/open-sse/executors/gemini-cli.ts
+++ b/open-sse/executors/gemini-cli.ts
@@ -8,6 +8,8 @@ const LOAD_CODE_ASSIST_TIMEOUT_MS = 10_000; // 10 seconds timeout
 
 // Per-account cache: accessToken -> { projectId, expiresAt }
 const projectCache = new Map<string, { projectId: string; expiresAt: number }>();
+// In-flight deduplication: prevents thundering herd on cache miss
+const inflightRefresh = new Map<string, Promise<string | null>>();
 
 export class GeminiCLIExecutor extends BaseExecutor {
   constructor() {
@@ -45,6 +47,20 @@ export class GeminiCLIExecutor extends BaseExecutor {
       return cached.projectId;
     }
 
+    // Deduplicate in-flight requests (thundering herd prevention)
+    const inflight = inflightRefresh.get(accessToken);
+    if (inflight) return inflight;
+
+    const promise = this._doRefresh(accessToken);
+    inflightRefresh.set(accessToken, promise);
+    try {
+      return await promise;
+    } finally {
+      inflightRefresh.delete(accessToken);
+    }
+  }
+
+  async _doRefresh(accessToken: string): Promise<string | null> {
     try {
       const controller = new AbortController();
       const timeoutId = setTimeout(() => controller.abort(), LOAD_CODE_ASSIST_TIMEOUT_MS);
@@ -81,7 +97,7 @@ export class GeminiCLIExecutor extends BaseExecutor {
       let projectId = "";
       if (typeof data.cloudaicompanionProject === "string") {
         projectId = data.cloudaicompanionProject.trim();
-      } else if (data.cloudaicompanionProject?.id) {
+      } else if (typeof data.cloudaicompanionProject?.id === "string") {
         projectId = data.cloudaicompanionProject.id.trim();
       }
 
@@ -96,6 +112,11 @@ export class GeminiCLIExecutor extends BaseExecutor {
         for (const [key, val] of projectCache) {
           if (val.expiresAt <= now) projectCache.delete(key);
         }
+        // If still full, evict the oldest entry (Map maintains insertion order)
+        if (projectCache.size >= MAX_CACHE_SIZE) {
+          const firstKey = projectCache.keys().next().value;
+          if (firstKey !== undefined) projectCache.delete(firstKey);
+        }
       }
       projectCache.set(accessToken, {
         projectId,
diff --git a/open-sse/utils/streamHelpers.ts b/open-sse/utils/streamHelpers.ts
index 3af232bae4..7221cd1035 100644
--- a/open-sse/utils/streamHelpers.ts
+++ b/open-sse/utils/streamHelpers.ts
@@ -89,7 +89,10 @@ export function hasValuableContent(chunk, format) {
  * while standard Gemini returns { candidates: [...] } directly.
  */
 export function unwrapGeminiChunk(parsed) {
-  return parsed.candidates ? parsed : parsed.response || parsed;
+  if (!parsed.candidates && parsed.response) {
+    return parsed.response;
+  }
+  return parsed;
 }
 
 // Fix invalid id (generic or too short)
diff --git a/tests/unit/streamHelpers.test.mjs b/tests/unit/streamHelpers.test.mjs
index d5f9190ad1..0dd1758354 100644
--- a/tests/unit/streamHelpers.test.mjs
+++ b/tests/unit/streamHelpers.test.mjs
@@ -92,7 +92,7 @@ describe("unwrapGeminiChunk", () => {
     assert.strictEqual(result, chunk);
   });
 
-  it("returns parsed when response exists but is null/undefined", () => {
+  it("returns parsed when response is null (falsy) — no valid envelope to unwrap", () => {
     const chunk = { response: null, other: "data" };
     const result = unwrapGeminiChunk(chunk);
     assert.strictEqual(result, chunk);