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
This commit is contained in:
Chris Staley
2026-03-31 11:41:19 -06:00
parent d852a51672
commit 2df8b234fe
3 changed files with 27 additions and 3 deletions

View File

@@ -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,

View File

@@ -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)

View File

@@ -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);