diff --git a/apps/server/src/providers/cli-provider.ts b/apps/server/src/providers/cli-provider.ts index 667142ba..ea636cb8 100644 --- a/apps/server/src/providers/cli-provider.ts +++ b/apps/server/src/providers/cli-provider.ts @@ -35,7 +35,7 @@ import { type SubprocessOptions, type WslCliResult, } from '@automaker/platform'; -import { calculateReasoningTimeout, DEFAULT_TIMEOUT_MS } from '@automaker/types'; +import { calculateReasoningTimeout } from '@automaker/types'; import { createLogger, isAbortError } from '@automaker/utils'; import { execSync } from 'child_process'; import * as fs from 'fs'; @@ -108,6 +108,15 @@ export interface CliDetectionResult { // Create logger for CLI operations const cliLogger = createLogger('CliProvider'); +/** + * Base timeout for CLI operations in milliseconds. + * CLI tools have longer startup and processing times compared to direct API calls, + * so we use a higher base timeout (120s) than the default provider timeout (30s). + * This is multiplied by reasoning effort multipliers when applicable. + * @see calculateReasoningTimeout from @automaker/types + */ +const CLI_BASE_TIMEOUT_MS = 120000; + /** * Abstract base class for CLI-based providers * @@ -452,10 +461,7 @@ export abstract class CliProvider extends BaseProvider { } // Calculate dynamic timeout based on reasoning effort. - // CLI operations use a higher base timeout (120s) than the Codex provider default (30s) - // because CLI tools like cursor-agent may have longer startup and processing times. // This addresses GitHub issue #530 where reasoning models with 'xhigh' effort would timeout. - const CLI_BASE_TIMEOUT_MS = 120000; const timeout = calculateReasoningTimeout(options.reasoningEffort, CLI_BASE_TIMEOUT_MS); // WSL strategy diff --git a/apps/server/src/providers/codex-provider.ts b/apps/server/src/providers/codex-provider.ts index 0d340b0b..18838cb8 100644 --- a/apps/server/src/providers/codex-provider.ts +++ b/apps/server/src/providers/codex-provider.ts @@ -34,7 +34,7 @@ import { supportsReasoningEffort, validateBareModelId, calculateReasoningTimeout, - DEFAULT_TIMEOUT_MS as TYPES_DEFAULT_TIMEOUT_MS, + DEFAULT_TIMEOUT_MS, type CodexApprovalPolicy, type CodexSandboxMode, type CodexAuthStatus, @@ -100,7 +100,7 @@ const TEXT_ENCODING = 'utf-8'; * reasoning effort, this timeout is dynamically extended via calculateReasoningTimeout(). * @see calculateReasoningTimeout from @automaker/types */ -const CODEX_CLI_TIMEOUT_MS = TYPES_DEFAULT_TIMEOUT_MS; +const CODEX_CLI_TIMEOUT_MS = DEFAULT_TIMEOUT_MS; const CONTEXT_WINDOW_256K = 256000; const MAX_OUTPUT_32K = 32000; const MAX_OUTPUT_16K = 16000; diff --git a/apps/server/tests/unit/providers/codex-provider.test.ts b/apps/server/tests/unit/providers/codex-provider.test.ts index 5f67dbeb..ee9c7bad 100644 --- a/apps/server/tests/unit/providers/codex-provider.test.ts +++ b/apps/server/tests/unit/providers/codex-provider.test.ts @@ -388,6 +388,13 @@ describe('codex-provider.ts', () => { ); }); + it('falls back to 1.0 multiplier for invalid reasoning effort', () => { + // Test that invalid values fallback gracefully to default multiplier + // This tests the defensive ?? 1.0 in calculateReasoningTimeout + const invalidEffort = 'invalid_effort' as never; + expect(calculateReasoningTimeout(invalidEffort)).toBe(DEFAULT_TIMEOUT_MS); + }); + it('produces expected absolute timeout values', () => { // Verify the actual timeout values that will be used: // none: 30000ms (30s)