From a122d34ebfdb7e03a259b2b7f616eb17beffe896 Mon Sep 17 00:00:00 2001 From: Alex Newman Date: Mon, 4 May 2026 21:07:42 -0700 Subject: [PATCH] fix: address Greptile P1 + CodeRabbit follow-ups (cycle 9) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - waitForSlot now accepts an optional AbortSignal. When the signal fires (e.g. session.abortController.abort() during shutdown or cancel), the queued waiter is removed from slotWaiters and the promise rejects immediately, instead of hanging until a slot naturally opens. Restores the cancellation guarantee that the removed 60s timeout used to provide. ClaudeProvider.startSession now passes session.abortController.signal at the call site. - EnvManager: a bare ANTHROPIC_BASE_URL now also short-circuits the OAuth lookup. Tokenless gateways (allowed by the new install flow) were otherwise being authenticated against api.anthropic.com via the injected OS-keychain OAuth token. - install.ts: resolveClaudeAuthMethod now reads the raw stored CLAUDE_MEM_CLAUDE_AUTH_METHOD value via a direct settings.json read (readRawStoredAuthMethod), bypassing SettingsDefaultsManager's default backfill. Without this, getSetting() always returned 'subscription' for unmigrated installs and the env-based fallback never ran — so the previous fix only addressed the optics, not the actual misclassification. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/npx-cli/commands/install.ts | 23 +++++++++++++++-------- src/services/worker/ClaudeProvider.ts | 2 +- src/shared/EnvManager.ts | 10 ++++++++-- src/supervisor/process-registry.ts | 25 ++++++++++++++++++++++--- 4 files changed, 46 insertions(+), 14 deletions(-) diff --git a/src/npx-cli/commands/install.ts b/src/npx-cli/commands/install.ts index 09ff0f19..c32bbf62 100644 --- a/src/npx-cli/commands/install.ts +++ b/src/npx-cli/commands/install.ts @@ -588,15 +588,22 @@ type ProviderId = 'claude' | 'gemini' | 'openrouter'; type ClaudeAccessMode = 'subscription' | 'api-key'; type ClaudeApiMode = 'direct' | 'gateway'; -function resolveClaudeAuthMethod(): 'subscription' | 'api-key' | 'gateway' { - const stored = getSetting('CLAUDE_MEM_CLAUDE_AUTH_METHOD') as - | 'subscription' - | 'api-key' - | 'gateway' - | undefined; - if (stored === 'subscription' || stored === 'api-key' || stored === 'gateway') { - return stored; +function readRawStoredAuthMethod(): 'subscription' | 'api-key' | 'gateway' | undefined { + try { + if (!existsSync(USER_SETTINGS_PATH)) return undefined; + const raw = JSON.parse(readFileSync(USER_SETTINGS_PATH, 'utf-8')) as Record; + const flat = (raw.env && typeof raw.env === 'object' ? raw.env : raw) as Record; + const value = flat.CLAUDE_MEM_CLAUDE_AUTH_METHOD; + if (value === 'subscription' || value === 'api-key' || value === 'gateway') return value; + return undefined; + } catch { + return undefined; } +} + +function resolveClaudeAuthMethod(): 'subscription' | 'api-key' | 'gateway' { + const stored = readRawStoredAuthMethod(); + if (stored) return stored; const env = loadClaudeMemEnv(); if (env.ANTHROPIC_BASE_URL?.trim()) return 'gateway'; if (env.ANTHROPIC_API_KEY?.trim()) return 'api-key'; diff --git a/src/services/worker/ClaudeProvider.ts b/src/services/worker/ClaudeProvider.ts index 64c54588..35df3131 100644 --- a/src/services/worker/ClaudeProvider.ts +++ b/src/services/worker/ClaudeProvider.ts @@ -150,7 +150,7 @@ export class ClaudeProvider { const settings = SettingsDefaultsManager.loadFromFile(USER_SETTINGS_PATH); const maxConcurrent = parseInt(settings.CLAUDE_MEM_MAX_CONCURRENT_AGENTS, 10) || 2; - await waitForSlot(maxConcurrent); + await waitForSlot(maxConcurrent, session.abortController.signal); const isolatedEnv = sanitizeEnv(await buildIsolatedEnvWithFreshOAuth()); const authMethod = getAuthMethodDescription(); diff --git a/src/shared/EnvManager.ts b/src/shared/EnvManager.ts index af442670..44fc62a6 100644 --- a/src/shared/EnvManager.ts +++ b/src/shared/EnvManager.ts @@ -231,8 +231,14 @@ export async function buildIsolatedEnvWithFreshOAuth( if (!includeCredentials) return isolatedEnv; // If the user already configured explicit Anthropic/gateway credentials in - // ~/.claude-mem/.env, honor those and skip OAuth lookup entirely. - if (isolatedEnv.ANTHROPIC_API_KEY || isolatedEnv.ANTHROPIC_AUTH_TOKEN) { + // ~/.claude-mem/.env, honor those and skip OAuth lookup entirely. A bare + // ANTHROPIC_BASE_URL counts because gateways may be tokenless, and falling + // back to OAuth would silently route requests to api.anthropic.com. + if ( + isolatedEnv.ANTHROPIC_API_KEY || + isolatedEnv.ANTHROPIC_BASE_URL || + isolatedEnv.ANTHROPIC_AUTH_TOKEN + ) { clearStaleMarker(); return isolatedEnv; } diff --git a/src/supervisor/process-registry.ts b/src/supervisor/process-registry.ts index e6b3e471..4fd68799 100644 --- a/src/supervisor/process-registry.ts +++ b/src/supervisor/process-registry.ts @@ -448,7 +448,7 @@ function notifySlotAvailable(): void { if (waiter) waiter(); } -export async function waitForSlot(maxConcurrent: number): Promise { +export async function waitForSlot(maxConcurrent: number, signal?: AbortSignal): Promise { getProcessRegistry().pruneDeadEntries(); const activeCount = getActiveSdkCount(); if (activeCount >= TOTAL_PROCESS_HARD_CAP) { @@ -457,26 +457,45 @@ export async function waitForSlot(maxConcurrent: number): Promise { if (activeCount < maxConcurrent) return; + if (signal?.aborted) { + throw new Error('waitForSlot aborted before queuing'); + } + logger.info('PROCESS', `Pool limit reached (${activeCount}/${maxConcurrent}), waiting for slot...`); return new Promise((resolve, reject) => { let recheckTimer: ReturnType | null = null; + let abortHandler: (() => void) | null = null; + const cleanup = () => { + if (recheckTimer) clearInterval(recheckTimer); + if (abortHandler && signal) signal.removeEventListener('abort', abortHandler); + const idx = slotWaiters.indexOf(onSlot); + if (idx >= 0) slotWaiters.splice(idx, 1); + }; const onSlot = () => { const count = getActiveSdkCount(); if (count >= TOTAL_PROCESS_HARD_CAP) { - if (recheckTimer) clearInterval(recheckTimer); + cleanup(); reject(new Error(`Hard cap exceeded: ${count} processes in registry (cap=${TOTAL_PROCESS_HARD_CAP}). Refusing to spawn more.`)); return; } if (count < maxConcurrent) { - if (recheckTimer) clearInterval(recheckTimer); + cleanup(); resolve(); } else { slotWaiters.push(onSlot); } }; + if (signal) { + abortHandler = () => { + cleanup(); + reject(new Error('waitForSlot aborted')); + }; + signal.addEventListener('abort', abortHandler, { once: true }); + } + slotWaiters.push(onSlot); recheckTimer = setInterval(() => { const removed = getProcessRegistry().pruneDeadEntries();