Fix Codex CLI timeout handling and improve CI workflows (#797)

* Changes from fix/codex-cli-timeout

* test: Clarify timeout values and multipliers in codex-provider tests

* refactor: Rename useWorktreesEnabled to worktreesEnabled for clarity
This commit is contained in:
gsxdsm
2026-02-21 23:58:09 -08:00
committed by GitHub
parent 91bff21d58
commit 72cb942788
5 changed files with 39 additions and 25 deletions

View File

@@ -33,7 +33,6 @@ import {
supportsReasoningEffort, supportsReasoningEffort,
validateBareModelId, validateBareModelId,
calculateReasoningTimeout, calculateReasoningTimeout,
DEFAULT_TIMEOUT_MS,
type CodexApprovalPolicy, type CodexApprovalPolicy,
type CodexSandboxMode, type CodexSandboxMode,
type CodexAuthStatus, type CodexAuthStatus,
@@ -98,7 +97,7 @@ const TEXT_ENCODING = 'utf-8';
* *
* @see calculateReasoningTimeout from @automaker/types * @see calculateReasoningTimeout from @automaker/types
*/ */
const CODEX_CLI_TIMEOUT_MS = DEFAULT_TIMEOUT_MS; const CODEX_CLI_TIMEOUT_MS = 120000; // 2 minutes — matches CLI provider base timeout
const CODEX_FEATURE_GENERATION_BASE_TIMEOUT_MS = 300000; // 5 minutes for feature generation const CODEX_FEATURE_GENERATION_BASE_TIMEOUT_MS = 300000; // 5 minutes for feature generation
const SYSTEM_PROMPT_SEPARATOR = '\n\n'; const SYSTEM_PROMPT_SEPARATOR = '\n\n';
const CODEX_INSTRUCTIONS_DIR = '.codex'; const CODEX_INSTRUCTIONS_DIR = '.codex';

View File

@@ -320,8 +320,10 @@ describe('codex-provider.ts', () => {
); );
const call = vi.mocked(spawnJSONLProcess).mock.calls[0][0]; const call = vi.mocked(spawnJSONLProcess).mock.calls[0][0];
// High reasoning effort should have 3x the default timeout (90000ms) // High reasoning effort should have 3x the CLI base timeout (120000ms)
expect(call.timeout).toBe(DEFAULT_TIMEOUT_MS * REASONING_TIMEOUT_MULTIPLIERS.high); // CODEX_CLI_TIMEOUT_MS = 120000, multiplier for 'high' = 3.0 → 360000ms
const CODEX_CLI_TIMEOUT_MS = 120000;
expect(call.timeout).toBe(CODEX_CLI_TIMEOUT_MS * REASONING_TIMEOUT_MULTIPLIERS.high);
}); });
it('passes extended timeout for xhigh reasoning effort', async () => { it('passes extended timeout for xhigh reasoning effort', async () => {
@@ -357,8 +359,10 @@ describe('codex-provider.ts', () => {
); );
const call = vi.mocked(spawnJSONLProcess).mock.calls[0][0]; const call = vi.mocked(spawnJSONLProcess).mock.calls[0][0];
// No reasoning effort should use the default timeout // No reasoning effort should use the CLI base timeout (2 minutes)
expect(call.timeout).toBe(DEFAULT_TIMEOUT_MS); // CODEX_CLI_TIMEOUT_MS = 120000ms, no multiplier applied
const CODEX_CLI_TIMEOUT_MS = 120000;
expect(call.timeout).toBe(CODEX_CLI_TIMEOUT_MS);
}); });
}); });

View File

@@ -84,17 +84,19 @@ export function useBoardActions({
onWorktreeAutoSelect, onWorktreeAutoSelect,
currentWorktreeBranch, currentWorktreeBranch,
}: UseBoardActionsProps) { }: UseBoardActionsProps) {
const { // IMPORTANT: Use individual selectors instead of bare useAppStore() to prevent
addFeature, // subscribing to the entire store. Bare useAppStore() causes the host component
updateFeature, // (BoardView) to re-render on EVERY store change, which cascades through effects
removeFeature, // and triggers React error #185 (maximum update depth exceeded).
moveFeature, const addFeature = useAppStore((s) => s.addFeature);
useWorktrees, const updateFeature = useAppStore((s) => s.updateFeature);
enableDependencyBlocking, const removeFeature = useAppStore((s) => s.removeFeature);
skipVerificationInAutoMode, const moveFeature = useAppStore((s) => s.moveFeature);
isPrimaryWorktreeBranch, const worktreesEnabled = useAppStore((s) => s.useWorktrees);
getPrimaryWorktreeBranch, const enableDependencyBlocking = useAppStore((s) => s.enableDependencyBlocking);
} = useAppStore(); const skipVerificationInAutoMode = useAppStore((s) => s.skipVerificationInAutoMode);
const isPrimaryWorktreeBranch = useAppStore((s) => s.isPrimaryWorktreeBranch);
const getPrimaryWorktreeBranch = useAppStore((s) => s.getPrimaryWorktreeBranch);
const autoMode = useAutoMode(); const autoMode = useAutoMode();
// React Query mutations for feature operations // React Query mutations for feature operations
@@ -549,7 +551,7 @@ export function useBoardActions({
const result = await api.autoMode.runFeature( const result = await api.autoMode.runFeature(
currentProject.path, currentProject.path,
feature.id, feature.id,
useWorktrees worktreesEnabled
// No worktreePath - server derives from feature.branchName // No worktreePath - server derives from feature.branchName
); );
@@ -560,7 +562,7 @@ export function useBoardActions({
throw new Error(result.error || 'Failed to start feature'); throw new Error(result.error || 'Failed to start feature');
} }
}, },
[currentProject, useWorktrees] [currentProject, worktreesEnabled]
); );
const handleStartImplementation = useCallback( const handleStartImplementation = useCallback(
@@ -693,9 +695,9 @@ export function useBoardActions({
logger.error('No current project'); logger.error('No current project');
return; return;
} }
resumeFeatureMutation.mutate({ featureId: feature.id, useWorktrees }); resumeFeatureMutation.mutate({ featureId: feature.id, useWorktrees: worktreesEnabled });
}, },
[currentProject, resumeFeatureMutation, useWorktrees] [currentProject, resumeFeatureMutation, worktreesEnabled]
); );
const handleManualVerify = useCallback( const handleManualVerify = useCallback(
@@ -780,7 +782,7 @@ export function useBoardActions({
followUpFeature.id, followUpFeature.id,
followUpPrompt, followUpPrompt,
imagePaths, imagePaths,
useWorktrees worktreesEnabled
); );
if (!result.success) { if (!result.success) {
@@ -818,7 +820,7 @@ export function useBoardActions({
setFollowUpPrompt, setFollowUpPrompt,
setFollowUpImagePaths, setFollowUpImagePaths,
setFollowUpPreviewMap, setFollowUpPreviewMap,
useWorktrees, worktreesEnabled,
]); ]);
const handleCommitFeature = useCallback( const handleCommitFeature = useCallback(

View File

@@ -33,7 +33,12 @@ export function useBoardDragDrop({
const [pendingDependencyLink, setPendingDependencyLink] = useState<PendingDependencyLink | null>( const [pendingDependencyLink, setPendingDependencyLink] = useState<PendingDependencyLink | null>(
null null
); );
const { moveFeature, updateFeature } = useAppStore(); // IMPORTANT: Use individual selectors instead of bare useAppStore() to prevent
// subscribing to the entire store. Bare useAppStore() causes the host component
// (BoardView) to re-render on EVERY store change, which cascades through effects
// and triggers React error #185 (maximum update depth exceeded).
const moveFeature = useAppStore((s) => s.moveFeature);
const updateFeature = useAppStore((s) => s.updateFeature);
const autoMode = useAutoMode(); const autoMode = useAutoMode();
// Note: getOrCreateWorktreeForFeature removed - worktrees are now created server-side // Note: getOrCreateWorktreeForFeature removed - worktrees are now created server-side

View File

@@ -14,7 +14,11 @@ interface UseBoardPersistenceProps {
} }
export function useBoardPersistence({ currentProject }: UseBoardPersistenceProps) { export function useBoardPersistence({ currentProject }: UseBoardPersistenceProps) {
const { updateFeature } = useAppStore(); // IMPORTANT: Use individual selector instead of bare useAppStore() to prevent
// subscribing to the entire store. Bare useAppStore() causes the host component
// (BoardView) to re-render on EVERY store change, which cascades through effects
// and triggers React error #185 (maximum update depth exceeded).
const updateFeature = useAppStore((s) => s.updateFeature);
const queryClient = useQueryClient(); const queryClient = useQueryClient();
// Persist feature update to API (replaces saveFeatures) // Persist feature update to API (replaces saveFeatures)