diff --git a/apps/desktop/src/main/__tests__/agent-queue.test.ts b/apps/desktop/src/main/__tests__/agent-queue.test.ts new file mode 100644 index 0000000000..4317e8f9e7 --- /dev/null +++ b/apps/desktop/src/main/__tests__/agent-queue.test.ts @@ -0,0 +1,85 @@ +import { EventEmitter } from 'node:events'; +import { mkdtempSync, rmSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; + +const { + runRoadmapGenerationMock, + writeFileWithRetryMock, +} = vi.hoisted(() => ({ + runRoadmapGenerationMock: vi.fn(), + writeFileWithRetryMock: vi.fn().mockResolvedValue(undefined), +})); + +vi.mock('../ai/runners/roadmap', () => ({ + runRoadmapGeneration: runRoadmapGenerationMock, +})); + +vi.mock('../utils/atomic-file', () => ({ + writeFileWithRetry: writeFileWithRetryMock, +})); + +vi.mock('../utils/debounce', () => ({ + debounce: (fn: (...args: unknown[]) => unknown) => ({ + fn, + cancel: vi.fn(), + }), +})); + +import { AgentQueueManager } from '../agent/agent-queue'; +import { AgentState } from '../agent/agent-state'; + +describe('AgentQueueManager roadmap progress mapping', () => { + let projectPath: string; + + beforeEach(() => { + vi.clearAllMocks(); + projectPath = mkdtempSync(join(tmpdir(), 'agent-queue-')); + }); + + afterEach(() => { + rmSync(projectPath, { recursive: true, force: true }); + }); + + it('maps runner phase names to frontend roadmap phases', async () => { + runRoadmapGenerationMock.mockImplementation( + async ( + _config: unknown, + onStream?: (event: { type: string; phase?: string }) => void, + ) => { + onStream?.({ type: 'phase-start', phase: 'discovery' }); + onStream?.({ type: 'phase-start', phase: 'features' }); + + return { + success: false, + phases: [], + error: 'boom', + }; + }, + ); + + const emitter = new EventEmitter(); + const progressUpdates: Array<{ phase: string; progress: number; message: string }> = []; + emitter.on('roadmap-progress', (_projectId, status) => { + progressUpdates.push(status); + }); + + const queue = new AgentQueueManager( + new AgentState(), + {} as never, + { killProcess: vi.fn() } as never, + emitter, + ); + + await queue.startRoadmapGeneration('project-1', projectPath); + + expect(progressUpdates.map((status) => status.phase)).toEqual([ + 'analyzing', + 'discovering', + 'generating', + ]); + expect(progressUpdates[1]?.message).toContain('discovery'); + expect(progressUpdates[2]?.message).toContain('features'); + }); +}); diff --git a/apps/desktop/src/main/agent/agent-queue.ts b/apps/desktop/src/main/agent/agent-queue.ts index aada34a53f..7623791cae 100644 --- a/apps/desktop/src/main/agent/agent-queue.ts +++ b/apps/desktop/src/main/agent/agent-queue.ts @@ -418,7 +418,14 @@ export class AgentQueueManager { (event: RoadmapStreamEvent) => { switch (event.type) { case 'phase-start': { - progressPhase = event.phase; + // Map runner phase names to XState machine state names + // Runner uses: 'discovery', 'features' + // Machine expects: 'discovering', 'generating' + const phaseMap: Record = { + 'discovery': 'discovering', + 'features': 'generating', + }; + progressPhase = phaseMap[event.phase] ?? event.phase; progressPercent = Math.min(progressPercent + 20, 90); const msg = `Running ${event.phase} phase...`; this.emitter.emit('roadmap-log', projectId, msg); diff --git a/apps/desktop/src/main/ai/auth/resolver.ts b/apps/desktop/src/main/ai/auth/resolver.ts index 17e1feb1ae..7db8bfb119 100644 --- a/apps/desktop/src/main/ai/auth/resolver.ts +++ b/apps/desktop/src/main/ai/auth/resolver.ts @@ -51,6 +51,17 @@ type SettingsAccessor = (key: string) => string | undefined; let _getSettingsValue: SettingsAccessor | null = null; +function throwIfAborted(signal?: AbortSignal): void { + if (!signal?.aborted) return; + + const reason = signal.reason; + if (reason instanceof Error) { + throw reason; + } + + throw new Error(typeof reason === 'string' ? reason : 'Aborted'); +} + /** * Register a settings accessor function. * Called once during app initialization to wire up settings access. @@ -70,6 +81,8 @@ export function registerSettingsAccessor(accessor: SettingsAccessor): void { * This is the highest priority stage — checks providerAccounts array. */ async function resolveFromProviderAccount(ctx: AuthResolverContext): Promise { + throwIfAborted(ctx.abortSignal); + if (!_getSettingsValue) return null; // Read providerAccounts from settings @@ -95,7 +108,7 @@ async function resolveFromProviderAccount(ctx: AuthResolverContext): Promise { + throwIfAborted(ctx.abortSignal); + return ( (await resolveFromProviderAccount(ctx)) ?? (await resolveFromProfileOAuth(ctx)) ?? @@ -352,8 +368,11 @@ export async function resolveAuthFromQueue( excludeAccountIds?: string[]; userModelOverrides?: Record>>; autoSwitchSettings?: ClaudeAutoSwitchSettings; + abortSignal?: AbortSignal; } ): Promise { + throwIfAborted(options?.abortSignal); + const excludeSet = new Set(options?.excludeAccountIds ?? []); const defaultSettings: ClaudeAutoSwitchSettings = { enabled: true, @@ -367,6 +386,8 @@ export async function resolveAuthFromQueue( const settings = options?.autoSwitchSettings ?? defaultSettings; for (const account of queue) { + throwIfAborted(options?.abortSignal); + // Skip excluded accounts if (excludeSet.has(account.id)) continue; @@ -411,7 +432,7 @@ export async function resolveAuthFromQueue( // is needed here. All OpenAI models are eligible through Codex OAuth. // Resolve credentials for this account - const auth = await resolveCredentialsForAccount(account, supportedProvider); + const auth = await resolveCredentialsForAccount(account, supportedProvider, options?.abortSignal); if (!auth) continue; // Success — return the fully resolved auth @@ -495,7 +516,10 @@ function resolveZaiBaseUrl(account: ProviderAccount): string { async function resolveCredentialsForAccount( account: ProviderAccount, provider: SupportedProvider, + abortSignal?: AbortSignal, ): Promise { + throwIfAborted(abortSignal); + // No-auth providers (e.g., Ollama) — no API key required if (NO_AUTH_PROVIDERS.has(provider)) { return { @@ -511,7 +535,7 @@ async function resolveCredentialsForAccount( const { app } = await import('electron'); const tokenFilePath = path.join(app.getPath('userData'), 'codex-auth.json'); const { ensureValidOAuthToken } = await import('../providers/oauth-fetch'); - const token = await ensureValidOAuthToken(tokenFilePath, 'openai'); + const token = await ensureValidOAuthToken(tokenFilePath, 'openai', abortSignal); if (token) { return { apiKey: 'codex-oauth-placeholder', @@ -519,7 +543,10 @@ async function resolveCredentialsForAccount( oauthTokenFilePath: tokenFilePath, }; } - } catch { /* fall through */ } + } catch { + throwIfAborted(abortSignal); + /* fall through */ + } return null; } @@ -527,7 +554,7 @@ async function resolveCredentialsForAccount( if (account.authType === 'oauth' && account.provider === 'anthropic') { if (account.claudeProfileId) { // Delegate to profile OAuth resolution - const ctx: AuthResolverContext = { provider, profileId: account.claudeProfileId }; + const ctx: AuthResolverContext = { provider, profileId: account.claudeProfileId, abortSignal }; return resolveAuth(ctx); } return null; diff --git a/apps/desktop/src/main/ai/auth/types.ts b/apps/desktop/src/main/ai/auth/types.ts index da5b7be1f7..51c8d1f34e 100644 --- a/apps/desktop/src/main/ai/auth/types.ts +++ b/apps/desktop/src/main/ai/auth/types.ts @@ -59,6 +59,8 @@ export interface AuthResolverContext { profileId?: string; /** Optional CLAUDE_CONFIG_DIR for profile-specific keychain lookup */ configDir?: string; + /** Abort signal for cancellation during credential resolution */ + abortSignal?: AbortSignal; } // ============================================ diff --git a/apps/desktop/src/main/ai/client/factory.ts b/apps/desktop/src/main/ai/client/factory.ts index e1acc75719..2349b7fb69 100644 --- a/apps/desktop/src/main/ai/client/factory.ts +++ b/apps/desktop/src/main/ai/client/factory.ts @@ -84,6 +84,7 @@ export async function createAgentClient( thinkingLevel, maxSteps = DEFAULT_MAX_STEPS, profileId, + abortSignal, additionalMcpServers, queueConfig, } = config; @@ -101,6 +102,7 @@ export async function createAgentClient( { excludeAccountIds: queueConfig.excludeAccountIds, userModelOverrides: queueConfig.userModelOverrides as any, + abortSignal, } ); @@ -133,6 +135,7 @@ export async function createAgentClient( const auth = await resolveAuth({ provider: detectedProvider, profileId, + abortSignal, }); model = createProvider({ @@ -216,6 +219,7 @@ export async function createSimpleClient( modelShorthand = 'haiku', thinkingLevel = 'low', profileId, + abortSignal, maxSteps = DEFAULT_SIMPLE_MAX_STEPS, tools = {}, queueConfig: explicitQueueConfig, @@ -240,6 +244,7 @@ export async function createSimpleClient( { excludeAccountIds, userModelOverrides: userModelOverrides as any, + abortSignal, } ); @@ -271,6 +276,7 @@ export async function createSimpleClient( const auth = await resolveAuth({ provider: detectedProvider, profileId, + abortSignal, }); model = createProvider({ diff --git a/apps/desktop/src/main/ai/client/types.ts b/apps/desktop/src/main/ai/client/types.ts index 7c2ed76d9a..c3a7e9f3df 100644 --- a/apps/desktop/src/main/ai/client/types.ts +++ b/apps/desktop/src/main/ai/client/types.ts @@ -71,6 +71,8 @@ export interface SimpleClientConfig { thinkingLevel?: ThinkingLevel; /** Profile ID for credential resolution */ profileId?: string; + /** Abort signal for cancellation during auth/model resolution */ + abortSignal?: AbortSignal; /** Maximum agentic steps (defaults to 1 for single-turn) */ maxSteps?: number; /** Specific tools to include (if any) */ diff --git a/apps/desktop/src/main/ai/providers/oauth-fetch.ts b/apps/desktop/src/main/ai/providers/oauth-fetch.ts index 82d1d43eb5..a139d4dfda 100644 --- a/apps/desktop/src/main/ai/providers/oauth-fetch.ts +++ b/apps/desktop/src/main/ai/providers/oauth-fetch.ts @@ -105,6 +105,7 @@ async function refreshOAuthToken( refreshToken: string, providerSpec: OAuthProviderSpec, tokenFilePath: string, + abortSignal?: AbortSignal, ): Promise { debugLog('Refreshing OAuth token'); @@ -118,6 +119,7 @@ async function refreshOAuthToken( method: 'POST', headers: { 'Content-Type': 'application/x-www-form-urlencoded' }, body: body.toString(), + signal: abortSignal, }); debugLog('Token refresh response', { status: response.status, ok: response.ok }); @@ -186,6 +188,7 @@ function detectProvider(provider?: string): OAuthProviderSpec | undefined { export async function ensureValidOAuthToken( tokenFilePath: string, provider?: string, + abortSignal?: AbortSignal, ): Promise { debugLog('Ensuring valid OAuth token', { path: tokenFilePath, provider }); @@ -212,8 +215,12 @@ export async function ensureValidOAuthToken( } try { - return await refreshOAuthToken(stored.refresh_token, providerSpec, tokenFilePath); + return await refreshOAuthToken(stored.refresh_token, providerSpec, tokenFilePath, abortSignal); } catch (err) { + if (abortSignal?.aborted) { + throw (abortSignal.reason instanceof Error ? abortSignal.reason : new Error('Aborted')); + } + debugLog('Token refresh failed', { error: err instanceof Error ? err.message : String(err) }); return null; } @@ -238,7 +245,11 @@ export function createOAuthProviderFetch( return async (input: RequestInfo | URL, init?: RequestInit): Promise => { // 1. Get valid OAuth token (auto-refresh if needed) - const token = await ensureValidOAuthToken(tokenFilePath, provider); + const token = await ensureValidOAuthToken( + tokenFilePath, + provider, + init?.signal instanceof AbortSignal ? init.signal : undefined, + ); if (!token) { throw new Error('OAuth: No valid token available. Please re-authenticate.'); } diff --git a/apps/desktop/src/main/ai/runners/roadmap.test.ts b/apps/desktop/src/main/ai/runners/roadmap.test.ts new file mode 100644 index 0000000000..bbb3f6bc40 --- /dev/null +++ b/apps/desktop/src/main/ai/runners/roadmap.test.ts @@ -0,0 +1,90 @@ +import { mkdtempSync, rmSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; + +const { + createSimpleClientMock, + getToolsForAgentMock, +} = vi.hoisted(() => ({ + createSimpleClientMock: vi.fn(), + getToolsForAgentMock: vi.fn(() => ({})), +})); + +vi.mock('../client/factory', () => ({ + createSimpleClient: createSimpleClientMock, +})); + +vi.mock('../tools/build-registry', () => ({ + buildToolRegistry: () => ({ + getToolsForAgent: getToolsForAgentMock, + }), +})); + +import { runRoadmapGeneration } from './roadmap'; + +describe('runRoadmapGeneration client creation', () => { + let projectDir: string; + + beforeEach(() => { + vi.clearAllMocks(); + vi.useFakeTimers(); + projectDir = mkdtempSync(join(tmpdir(), 'roadmap-runner-')); + }); + + afterEach(() => { + vi.useRealTimers(); + rmSync(projectDir, { recursive: true, force: true }); + }); + + it('aborts client creation when the auth timeout elapses', async () => { + let receivedAbortSignal: AbortSignal | undefined; + + createSimpleClientMock.mockImplementation(({ abortSignal }: { abortSignal?: AbortSignal }) => { + receivedAbortSignal = abortSignal; + + return new Promise((_, reject) => { + abortSignal?.addEventListener('abort', () => { + reject(abortSignal.reason instanceof Error ? abortSignal.reason : new Error('Aborted')); + }, { once: true }); + }); + }); + + const resultPromise = runRoadmapGeneration({ projectDir }); + const rejection = expect(resultPromise).rejects.toThrow('Client creation timed out'); + + await vi.advanceTimersByTimeAsync(120_000); + + await rejection; + expect(receivedAbortSignal?.aborted).toBe(true); + expect(createSimpleClientMock).toHaveBeenCalledTimes(1); + }); + + it('propagates user cancellation into client creation', async () => { + const controller = new AbortController(); + let receivedAbortSignal: AbortSignal | undefined; + + createSimpleClientMock.mockImplementation(({ abortSignal }: { abortSignal?: AbortSignal }) => { + receivedAbortSignal = abortSignal; + + return new Promise((_, reject) => { + abortSignal?.addEventListener('abort', () => { + reject(abortSignal.reason instanceof Error ? abortSignal.reason : new Error('Aborted')); + }, { once: true }); + }); + }); + + const resultPromise = runRoadmapGeneration({ + projectDir, + abortSignal: controller.signal, + }); + const rejection = expect(resultPromise).rejects.toThrow('Aborted'); + + controller.abort(new Error('Aborted')); + await vi.runAllTimersAsync(); + + await rejection; + expect(receivedAbortSignal?.aborted).toBe(true); + expect(createSimpleClientMock).toHaveBeenCalledTimes(1); + }); +}); diff --git a/apps/desktop/src/main/ai/runners/roadmap.ts b/apps/desktop/src/main/ai/runners/roadmap.ts index b589af4c70..44774946fa 100644 --- a/apps/desktop/src/main/ai/runners/roadmap.ts +++ b/apps/desktop/src/main/ai/runners/roadmap.ts @@ -19,8 +19,9 @@ import { buildToolRegistry } from '../tools/build-registry'; import type { ToolContext } from '../tools/types'; import type { ModelShorthand, ThinkingLevel } from '../config/types'; import type { SecurityProfile } from '../security/bash-validator'; -import { safeParseJson } from '../../utils/json-repair'; +import { repairJson } from '../../utils/json-repair'; import { tryLoadPrompt } from '../prompts/prompt-loader'; +import { MAX_OAUTH_REFRESH_TOTAL_DURATION_MS } from '../../claude-profile/token-refresh'; // ============================================================================= // Constants @@ -31,6 +32,23 @@ const MAX_RETRIES = 3; /** Maximum agentic steps per phase */ const MAX_STEPS_PER_PHASE = 30; +/** + * Timeout for client creation (auth resolution). + * Leaves enough headroom for the full OAuth refresh retry budget plus a small buffer. + */ +const CLIENT_CREATION_TIMEOUT_MS = MAX_OAUTH_REFRESH_TOTAL_DURATION_MS + 5_000; + +function throwIfAborted(signal?: AbortSignal): void { + if (!signal?.aborted) return; + + const reason = signal.reason; + if (reason instanceof Error) { + throw reason; + } + + throw new Error(typeof reason === 'string' ? reason : 'Aborted'); +} + // ============================================================================= // Types // ============================================================================= @@ -88,6 +106,23 @@ export type RoadmapStreamEvent = | { type: 'tool-use'; name: string } | { type: 'error'; error: string }; +function readAndRepairJsonFile(filePath: string): T | null { + try { + const raw = readFileSync(filePath, 'utf-8'); + const repaired = repairJson(raw); + + if (repaired !== raw) { + const tmpFile = `${filePath}.tmp.${process.pid}`; + writeFileSync(tmpFile, repaired, 'utf-8'); + renameSync(tmpFile, filePath); + } + + return JSON.parse(repaired) as T; + } catch { + return null; + } +} + // ============================================================================= // Discovery Phase // ============================================================================= @@ -180,7 +215,7 @@ Do NOT ask questions. Make educated inferences and create the file.`; // Validate output if (existsSync(discoveryFile)) { - const data = safeParseJson>(readFileSync(discoveryFile, 'utf-8')); + const data = readAndRepairJsonFile>(discoveryFile); if (data) { const required = ['project_name', 'target_audience', 'product_vision']; const missing = required.filter((k) => !(k in data)); @@ -318,7 +353,7 @@ The JSON must contain: vision, target_audience (object with "primary" key), phas if ((err as NodeJS.ErrnoException).code !== 'ENOENT') throw err; } if (roadmapRaw !== null) { - const data = safeParseJson>(roadmapRaw); + const data = readAndRepairJsonFile>(roadmapFile); if (data) { const required = ['phases', 'features', 'vision', 'target_audience']; const missing = required.filter((k) => !(k in data)); @@ -366,7 +401,7 @@ The JSON must contain: vision, target_audience (object with "primary" key), phas function loadPreservedFeatures(roadmapFile: string): Record[] { if (!existsSync(roadmapFile)) return []; - const data = safeParseJson>(readFileSync(roadmapFile, 'utf-8')); + const data = readAndRepairJsonFile>(roadmapFile); if (!data) return []; const features: Record[] = (data.features as Record[]) ?? []; @@ -462,13 +497,40 @@ export async function runRoadmapGeneration( const registry = buildToolRegistry(); const tools = registry.getToolsForAgent('roadmap_discovery', toolContext); - const client = await createSimpleClient({ - systemPrompt: '', - modelShorthand, - thinkingLevel, - maxSteps: MAX_STEPS_PER_PHASE, - tools, - }); + const clientCreationTimeoutController = new AbortController(); + const clientAbortSignal = abortSignal + ? AbortSignal.any([abortSignal, clientCreationTimeoutController.signal]) + : clientCreationTimeoutController.signal; + + const clientCreationTimer = setTimeout(() => { + clientCreationTimeoutController.abort( + new Error('Client creation timed out — check your authentication credentials'), + ); + }, CLIENT_CREATION_TIMEOUT_MS); + + let client: SimpleClientResult; + try { + throwIfAborted(abortSignal); + + client = await createSimpleClient({ + systemPrompt: '', + modelShorthand, + thinkingLevel, + maxSteps: MAX_STEPS_PER_PHASE, + tools, + abortSignal: clientAbortSignal, + }); + } catch (error) { + if (abortSignal?.aborted) { + throw new Error('Aborted'); + } + if (clientCreationTimeoutController.signal.aborted) { + throw new Error('Client creation timed out — check your authentication credentials'); + } + throw error; + } finally { + clearTimeout(clientCreationTimer); + } const phases: RoadmapPhaseResult[] = []; diff --git a/apps/desktop/src/main/claude-profile/token-refresh.test.ts b/apps/desktop/src/main/claude-profile/token-refresh.test.ts index 585ccf3523..904c2d1166 100644 --- a/apps/desktop/src/main/claude-profile/token-refresh.test.ts +++ b/apps/desktop/src/main/claude-profile/token-refresh.test.ts @@ -201,6 +201,26 @@ describe('token-refresh', () => { expect(result.errorCode).toBe('network_error'); expect(mockFetch).toHaveBeenCalledTimes(3); // Initial + 2 retries }); + + it('should propagate caller aborts without retrying', async () => { + const controller = new AbortController(); + + mockFetch.mockImplementationOnce(async (_url: string, init?: RequestInit) => { + const signal = init?.signal; + + return await new Promise((_, reject) => { + signal?.addEventListener('abort', () => { + reject(signal.reason instanceof Error ? signal.reason : new Error('Aborted')); + }, { once: true }); + }); + }); + + const resultPromise = refreshOAuthToken('valid-refresh-token', undefined, controller.signal); + controller.abort(new Error('Aborted')); + + await expect(resultPromise).rejects.toThrow('Aborted'); + expect(mockFetch).toHaveBeenCalledTimes(1); + }); }); describe('ensureValidToken', () => { diff --git a/apps/desktop/src/main/claude-profile/token-refresh.ts b/apps/desktop/src/main/claude-profile/token-refresh.ts index 643d996b62..0c9820e42c 100644 --- a/apps/desktop/src/main/claude-profile/token-refresh.ts +++ b/apps/desktop/src/main/claude-profile/token-refresh.ts @@ -53,6 +53,60 @@ const MAX_REFRESH_RETRIES = 2; */ const RETRY_DELAY_BASE_MS = 1000; +/** + * Timeout for each token refresh HTTP request (30 seconds). + * Prevents indefinite hang if the OAuth endpoint is unreachable. + */ +const TOKEN_REFRESH_TIMEOUT_MS = 30_000; + +/** + * Maximum time needed to exhaust the configured refresh retry budget. + * Callers can use this to set outer auth-resolution timeouts that do not + * fire before the refresh policy has had a chance to complete. + */ +export const MAX_OAUTH_REFRESH_TOTAL_DURATION_MS = + ((MAX_REFRESH_RETRIES + 1) * TOKEN_REFRESH_TIMEOUT_MS) + + (RETRY_DELAY_BASE_MS * ((2 ** MAX_REFRESH_RETRIES) - 1)); + +function throwIfAborted(signal?: AbortSignal): void { + if (!signal?.aborted) return; + + const reason = signal.reason; + if (reason instanceof Error) { + throw reason; + } + + throw new Error(typeof reason === 'string' ? reason : 'Aborted'); +} + +async function delay(ms: number, signal?: AbortSignal): Promise { + throwIfAborted(signal); + + await new Promise((resolve, reject) => { + const timer = setTimeout(() => { + cleanup(); + resolve(); + }, ms); + + const cleanup = (): void => { + clearTimeout(timer); + signal?.removeEventListener('abort', onAbort); + }; + + const onAbort = (): void => { + cleanup(); + reject(signal?.reason instanceof Error ? signal.reason : new Error('Aborted')); + }; + + signal?.addEventListener('abort', onAbort, { once: true }); + }); +} + +function createRefreshRequestSignal(abortSignal?: AbortSignal): AbortSignal { + const timeoutSignal = AbortSignal.timeout(TOKEN_REFRESH_TIMEOUT_MS); + return abortSignal ? AbortSignal.any([abortSignal, timeoutSignal]) : timeoutSignal; +} + // ============================================================================= // Types // ============================================================================= @@ -167,7 +221,8 @@ export function formatTimeRemaining(ms: number | null): string { */ export async function refreshOAuthToken( refreshToken: string, - configDir?: string + configDir?: string, + abortSignal?: AbortSignal, ): Promise { const isDebug = process.env.DEBUG === 'true'; @@ -190,13 +245,15 @@ export async function refreshOAuthToken( let lastError: Error | null = null; for (let attempt = 0; attempt <= MAX_REFRESH_RETRIES; attempt++) { + throwIfAborted(abortSignal); + if (attempt > 0) { // Exponential backoff between retries - const delay = RETRY_DELAY_BASE_MS * 2 ** (attempt - 1); + const retryDelay = RETRY_DELAY_BASE_MS * 2 ** (attempt - 1); if (isDebug) { - console.warn('[TokenRefresh] Retrying after delay:', delay, 'ms (attempt', attempt + 1, ')'); + console.warn('[TokenRefresh] Retrying after delay:', retryDelay, 'ms (attempt', attempt + 1, ')'); } - await new Promise(resolve => setTimeout(resolve, delay)); + await delay(retryDelay, abortSignal); } try { @@ -212,7 +269,8 @@ export async function refreshOAuthToken( headers: { 'Content-Type': 'application/x-www-form-urlencoded' }, - body: body.toString() + body: body.toString(), + signal: createRefreshRequestSignal(abortSignal), }); if (!response.ok) { @@ -284,6 +342,10 @@ export async function refreshOAuthToken( expiresIn }; } catch (error) { + if (abortSignal?.aborted) { + throw (abortSignal.reason instanceof Error ? abortSignal.reason : new Error('Aborted')); + } + lastError = error instanceof Error ? error : new Error(String(error)); if (isDebug) { console.warn('[TokenRefresh] Network error, will retry:', lastError.message); @@ -319,7 +381,8 @@ export async function refreshOAuthToken( */ export async function ensureValidToken( configDir: string | undefined, - onRefreshed?: OnTokenRefreshedCallback + onRefreshed?: OnTokenRefreshedCallback, + abortSignal?: AbortSignal, ): Promise { const isDebug = process.env.DEBUG === 'true'; const isVerbose = process.env.VERBOSE === 'true'; @@ -335,6 +398,8 @@ export async function ensureValidToken( }); } + throwIfAborted(abortSignal); + // Step 1: Read full credentials from keychain const creds = getFullCredentialsFromKeychain(expandedConfigDir); @@ -391,7 +456,7 @@ export async function ensureValidToken( } // Step 4: Refresh the token - const refreshResult = await refreshOAuthToken(creds.refreshToken, expandedConfigDir); + const refreshResult = await refreshOAuthToken(creds.refreshToken, expandedConfigDir, abortSignal); if (!refreshResult.success || !refreshResult.accessToken || !refreshResult.refreshToken || !refreshResult.expiresAt) { console.error('[TokenRefresh:ensureValidToken] Token refresh failed:', refreshResult.error); @@ -484,7 +549,8 @@ export async function ensureValidToken( */ export async function reactiveTokenRefresh( configDir: string | undefined, - onRefreshed?: OnTokenRefreshedCallback + onRefreshed?: OnTokenRefreshedCallback, + abortSignal?: AbortSignal, ): Promise { const isDebug = process.env.DEBUG === 'true'; @@ -498,6 +564,8 @@ export async function reactiveTokenRefresh( }); } + throwIfAborted(abortSignal); + // Read credentials to get refresh token const creds = getFullCredentialsFromKeychain(expandedConfigDir); @@ -518,7 +586,7 @@ export async function reactiveTokenRefresh( } // Perform refresh - const refreshResult = await refreshOAuthToken(creds.refreshToken, expandedConfigDir); + const refreshResult = await refreshOAuthToken(creds.refreshToken, expandedConfigDir, abortSignal); if (!refreshResult.success || !refreshResult.accessToken || !refreshResult.refreshToken || !refreshResult.expiresAt) { return { diff --git a/apps/desktop/src/main/utils/__tests__/json-repair.test.ts b/apps/desktop/src/main/utils/__tests__/json-repair.test.ts index c203f18430..cda500e7ba 100644 --- a/apps/desktop/src/main/utils/__tests__/json-repair.test.ts +++ b/apps/desktop/src/main/utils/__tests__/json-repair.test.ts @@ -52,6 +52,13 @@ describe('repairJson', () => { expect(parsed.key).toBe('value'); }); + it('strips illegal control characters while preserving valid unicode', () => { + const broken = '{"text": "entrepren\u000f\u000fr og skogeier \u00f8 \u00e6 \u00e5"}'; + const result = repairJson(broken); + const parsed = JSON.parse(result); + expect(parsed.text).toBe('entreprenr og skogeier ø æ å'); + }); + it('handles the real-world implementation_plan.json missing comma bug', () => { // This is the actual pattern that caused the production bug const broken = `{ @@ -94,6 +101,11 @@ describe('safeParseJson', () => { expect(result).toEqual({ a: 1, b: 2 }); }); + it('returns parsed object when invalid control characters are present', () => { + const result = safeParseJson<{ text: string }>('{"text": "entrepren\u000f\u000fr og ø"}'); + expect(result).toEqual({ text: 'entreprenr og ø' }); + }); + it('returns null for unrepairable JSON', () => { const result = safeParseJson('{{{invalid'); expect(result).toBeNull(); diff --git a/apps/desktop/src/main/utils/json-repair.ts b/apps/desktop/src/main/utils/json-repair.ts index d11745b20b..ad3169ef92 100644 --- a/apps/desktop/src/main/utils/json-repair.ts +++ b/apps/desktop/src/main/utils/json-repair.ts @@ -44,6 +44,14 @@ function applyRepairs(raw: string, originalError: SyntaxError): string { // 1. Strip markdown code fences (```json ... ```) text = text.replace(/^```(?:json)?\s*\n?/gm, '').replace(/\n?```\s*$/gm, ''); + // 1.5. Strip illegal JSON control characters while preserving valid + // Unicode, tabs, line feeds, and carriage returns. + const strippedControlChars = text.replace(/[\u0000-\u0008\u000B\u000C\u000E-\u001F]/g, ''); + if (strippedControlChars !== text) { + text = strippedControlChars; + console.warn('[json-repair] Stripped illegal control characters from JSON'); + } + // 2. Remove trailing commas before } or ] text = text.replace(/,(\s*[}\]])/g, '$1');