mirror of
https://github.com/AutoMaker-Org/automaker.git
synced 2026-02-02 08:33:36 +00:00
refactor: update timeout constants in CLI and Codex providers
- Removed redundant definition of CLI base timeout in `cli-provider.ts` and added a detailed comment explaining its purpose. - Updated `codex-provider.ts` to use the imported `DEFAULT_TIMEOUT_MS` directly instead of an alias. - Enhanced unit tests to ensure fallback behavior for invalid reasoning effort values in timeout calculations.
This commit is contained in:
@@ -35,7 +35,7 @@ import {
|
|||||||
type SubprocessOptions,
|
type SubprocessOptions,
|
||||||
type WslCliResult,
|
type WslCliResult,
|
||||||
} from '@automaker/platform';
|
} from '@automaker/platform';
|
||||||
import { calculateReasoningTimeout, DEFAULT_TIMEOUT_MS } from '@automaker/types';
|
import { calculateReasoningTimeout } from '@automaker/types';
|
||||||
import { createLogger, isAbortError } from '@automaker/utils';
|
import { createLogger, isAbortError } from '@automaker/utils';
|
||||||
import { execSync } from 'child_process';
|
import { execSync } from 'child_process';
|
||||||
import * as fs from 'fs';
|
import * as fs from 'fs';
|
||||||
@@ -108,6 +108,15 @@ export interface CliDetectionResult {
|
|||||||
// Create logger for CLI operations
|
// Create logger for CLI operations
|
||||||
const cliLogger = createLogger('CliProvider');
|
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
|
* Abstract base class for CLI-based providers
|
||||||
*
|
*
|
||||||
@@ -452,10 +461,7 @@ export abstract class CliProvider extends BaseProvider {
|
|||||||
}
|
}
|
||||||
|
|
||||||
// Calculate dynamic timeout based on reasoning effort.
|
// 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.
|
// 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);
|
const timeout = calculateReasoningTimeout(options.reasoningEffort, CLI_BASE_TIMEOUT_MS);
|
||||||
|
|
||||||
// WSL strategy
|
// WSL strategy
|
||||||
|
|||||||
@@ -34,7 +34,7 @@ import {
|
|||||||
supportsReasoningEffort,
|
supportsReasoningEffort,
|
||||||
validateBareModelId,
|
validateBareModelId,
|
||||||
calculateReasoningTimeout,
|
calculateReasoningTimeout,
|
||||||
DEFAULT_TIMEOUT_MS as TYPES_DEFAULT_TIMEOUT_MS,
|
DEFAULT_TIMEOUT_MS,
|
||||||
type CodexApprovalPolicy,
|
type CodexApprovalPolicy,
|
||||||
type CodexSandboxMode,
|
type CodexSandboxMode,
|
||||||
type CodexAuthStatus,
|
type CodexAuthStatus,
|
||||||
@@ -100,7 +100,7 @@ const TEXT_ENCODING = 'utf-8';
|
|||||||
* reasoning effort, this timeout is dynamically extended via calculateReasoningTimeout().
|
* reasoning effort, this timeout is dynamically extended via calculateReasoningTimeout().
|
||||||
* @see calculateReasoningTimeout from @automaker/types
|
* @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 CONTEXT_WINDOW_256K = 256000;
|
||||||
const MAX_OUTPUT_32K = 32000;
|
const MAX_OUTPUT_32K = 32000;
|
||||||
const MAX_OUTPUT_16K = 16000;
|
const MAX_OUTPUT_16K = 16000;
|
||||||
|
|||||||
@@ -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', () => {
|
it('produces expected absolute timeout values', () => {
|
||||||
// Verify the actual timeout values that will be used:
|
// Verify the actual timeout values that will be used:
|
||||||
// none: 30000ms (30s)
|
// none: 30000ms (30s)
|
||||||
|
|||||||
Reference in New Issue
Block a user