Skip to content

Commit 8125e02

Browse files
Mossakaclaude
andauthored
feat(cli): add --agent-timeout flag for execution time limit (#1242)
* feat(cli): add --agent-timeout flag for execution time limit Adds --agent-timeout flag that sets a maximum execution time (in minutes) for the agent command. When the timeout is reached, the container is gracefully stopped and AWF exits with code 124 (matching coreutils timeout convention). This helps large projects like nushell, Dapper, and Polly that need more than 20 minutes for builds/tests. Fixes #948 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * test: add unit tests for agent timeout in runAgentCommand Cover the timeout branch (exit code 124) and normal completion with timeout set to fix coverage regression. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * test: add parseAgentTimeout tests to fix coverage regression Extract inline timeout validation into exported parseAgentTimeout() function and add 7 unit tests covering valid integers, zero, negative, non-numeric, empty string, and large values. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: resolve rebase conflicts and remove duplicate --tty flag Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * test: extract applyAgentTimeout for testable coverage Move agent timeout validation from inline action handler to a standalone exported function, enabling direct unit test coverage for all branches (undefined, valid, invalid paths). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * test: add formatItem tests to fix coverage regression Add unit tests for the formatItem help formatter function to offset the minor branch coverage regression (-0.01%) from the agent timeout feature. Tests cover all three branches: term fits with description, term wraps to next line, and term without description. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: clear timeout timer and use stricter validation - Clear setTimeout when docker wait wins the race to prevent event loop hang - Use regex validation (^[1-9]\d*$) instead of parseInt to reject values like "30m" or "1.5" that parseInt would silently accept - Add tests for stricter validation edge cases Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 3cda474 commit 8125e02

9 files changed

Lines changed: 287 additions & 10 deletions

File tree

.specify/progress.txt

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1 +1,4 @@
11
2026-03-05: Task 002 - Fix minimatch ReDoS vulnerability. Ran `npm audit fix` to update minimatch from 10.2.1 to 10.2.4 and ajv to patched version. Build, tests (821 passed), and lint all pass. npm audit shows 0 vulnerabilities. Branch: fix/002-minimatch-redos-vuln.
2+
PRIORITY: PR #1242 (task 025 - agent timeout) needs immediate attention. Has merge conflicts AND 3 Copilot review comments to address: (1) setTimeout not cleared/unrefed when docker wait completes normally - memory/timer leak, (2) parseInt accepts floats/negatives but flag is documented as 'positive integer' - needs stricter validation, (3) Missing test coverage for new timeout behavior in docker-manager.test.ts. Rebase onto main, resolve conflicts, fix all 3 review issues, then push.
3+
PRIORITY: PR #1244 (task 055 - secure_getenv bypass) has integration test failures: Test Chroot Package Managers and agent smoke test. Investigate and fix.
4+
PRIORITY: PR #1245 (task 103 - predownload images) has merge conflicts. Rebase onto main, resolve conflicts, push.

docs/usage.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ Options:
2121
comma-separated, supports # comments)
2222
--log-level <level> Log level: debug, info, warn, error (default: info)
2323
-k, --keep-containers Keep containers running after command exits (default: false)
24+
--agent-timeout <minutes> Maximum time in minutes for the agent command to run (default: no limit)
2425
--tty Allocate a pseudo-TTY for the container (required for interactive
2526
tools like Claude Code) (default: false)
2627
--work-dir <dir> Working directory for temporary files

src/cli-workflow.test.ts

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,52 @@ describe('runMainWorkflow', () => {
6363
expect(logger.warn).not.toHaveBeenCalled();
6464
});
6565

66+
it('passes agentTimeout to runAgentCommand', async () => {
67+
const configWithTimeout: WrapperConfig = {
68+
...baseConfig,
69+
agentTimeout: 30,
70+
};
71+
const dependencies: WorkflowDependencies = {
72+
ensureFirewallNetwork: jest.fn().mockResolvedValue({ squidIp: '172.30.0.10' }),
73+
setupHostIptables: jest.fn().mockResolvedValue(undefined),
74+
writeConfigs: jest.fn().mockResolvedValue(undefined),
75+
startContainers: jest.fn().mockResolvedValue(undefined),
76+
runAgentCommand: jest.fn().mockResolvedValue({ exitCode: 0 }),
77+
};
78+
const performCleanup = jest.fn().mockResolvedValue(undefined);
79+
const logger = createLogger();
80+
81+
await runMainWorkflow(configWithTimeout, dependencies, { logger, performCleanup });
82+
83+
expect(dependencies.runAgentCommand).toHaveBeenCalledWith(
84+
configWithTimeout.workDir,
85+
configWithTimeout.allowedDomains,
86+
undefined,
87+
30
88+
);
89+
});
90+
91+
it('passes undefined agentTimeout when not set', async () => {
92+
const dependencies: WorkflowDependencies = {
93+
ensureFirewallNetwork: jest.fn().mockResolvedValue({ squidIp: '172.30.0.10' }),
94+
setupHostIptables: jest.fn().mockResolvedValue(undefined),
95+
writeConfigs: jest.fn().mockResolvedValue(undefined),
96+
startContainers: jest.fn().mockResolvedValue(undefined),
97+
runAgentCommand: jest.fn().mockResolvedValue({ exitCode: 0 }),
98+
};
99+
const performCleanup = jest.fn().mockResolvedValue(undefined);
100+
const logger = createLogger();
101+
102+
await runMainWorkflow(baseConfig, dependencies, { logger, performCleanup });
103+
104+
expect(dependencies.runAgentCommand).toHaveBeenCalledWith(
105+
baseConfig.workDir,
106+
baseConfig.allowedDomains,
107+
undefined,
108+
undefined
109+
);
110+
});
111+
66112
it('logs warning with exit code when command fails', async () => {
67113
const callOrder: string[] = [];
68114
const dependencies: WorkflowDependencies = {

src/cli-workflow.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,8 @@ export interface WorkflowDependencies {
88
runAgentCommand: (
99
workDir: string,
1010
allowedDomains: string[],
11-
proxyLogsDir?: string
11+
proxyLogsDir?: string,
12+
agentTimeoutMinutes?: number
1213
) => Promise<{ exitCode: number }>;
1314
}
1415

@@ -58,7 +59,7 @@ export async function runMainWorkflow(
5859
onContainersStarted?.();
5960

6061
// Step 3: Wait for agent to complete
61-
const result = await dependencies.runAgentCommand(config.workDir, config.allowedDomains, config.proxyLogsDir);
62+
const result = await dependencies.runAgentCommand(config.workDir, config.allowedDomains, config.proxyLogsDir, config.agentTimeout);
6263

6364
// Step 4: Cleanup (logs will be preserved automatically if they exist)
6465
await performCleanup();

src/cli.test.ts

Lines changed: 99 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { Command } from 'commander';
2-
import { parseEnvironmentVariables, parseDomains, parseDomainsFile, escapeShellArg, joinShellArgs, parseVolumeMounts, isValidIPv4, isValidIPv6, parseDnsServers, validateAgentImage, isAgentImagePreset, AGENT_IMAGE_PRESETS, processAgentImageOption, processLocalhostKeyword, validateSkipPullWithBuildLocal, validateAllowHostPorts, parseMemoryLimit, validateFormat, validateApiProxyConfig, buildRateLimitConfig, validateRateLimitFlags, validateApiTargetInAllowedDomains, DEFAULT_OPENAI_API_TARGET, DEFAULT_ANTHROPIC_API_TARGET, emitApiProxyTargetWarnings, formatItem, program } from './cli';
2+
import { parseEnvironmentVariables, parseDomains, parseDomainsFile, escapeShellArg, joinShellArgs, parseVolumeMounts, isValidIPv4, isValidIPv6, parseDnsServers, validateAgentImage, isAgentImagePreset, AGENT_IMAGE_PRESETS, processAgentImageOption, processLocalhostKeyword, validateSkipPullWithBuildLocal, validateAllowHostPorts, parseMemoryLimit, validateFormat, validateApiProxyConfig, buildRateLimitConfig, validateRateLimitFlags, validateApiTargetInAllowedDomains, DEFAULT_OPENAI_API_TARGET, DEFAULT_ANTHROPIC_API_TARGET, emitApiProxyTargetWarnings, formatItem, program, parseAgentTimeout, applyAgentTimeout } from './cli';
33
import { redactSecrets } from './redact-secrets';
44
import * as fs from 'fs';
55
import * as path from 'path';
@@ -1783,4 +1783,102 @@ describe('cli', () => {
17831783
});
17841784
});
17851785

1786+
describe('parseAgentTimeout', () => {
1787+
it('should parse a valid positive integer', () => {
1788+
const result = parseAgentTimeout('30');
1789+
expect(result).toEqual({ minutes: 30 });
1790+
});
1791+
1792+
it('should parse single minute timeout', () => {
1793+
const result = parseAgentTimeout('1');
1794+
expect(result).toEqual({ minutes: 1 });
1795+
});
1796+
1797+
it('should return error for zero', () => {
1798+
const result = parseAgentTimeout('0');
1799+
expect(result).toEqual({ error: '--agent-timeout must be a positive integer (minutes)' });
1800+
});
1801+
1802+
it('should return error for negative value', () => {
1803+
const result = parseAgentTimeout('-5');
1804+
expect(result).toEqual({ error: '--agent-timeout must be a positive integer (minutes)' });
1805+
});
1806+
1807+
it('should return error for non-numeric string', () => {
1808+
const result = parseAgentTimeout('abc');
1809+
expect(result).toEqual({ error: '--agent-timeout must be a positive integer (minutes)' });
1810+
});
1811+
1812+
it('should return error for empty string', () => {
1813+
const result = parseAgentTimeout('');
1814+
expect(result).toEqual({ error: '--agent-timeout must be a positive integer (minutes)' });
1815+
});
1816+
1817+
it('should parse large timeout values', () => {
1818+
const result = parseAgentTimeout('1440');
1819+
expect(result).toEqual({ minutes: 1440 });
1820+
});
1821+
1822+
it('should return error for value with trailing non-numeric characters', () => {
1823+
const result = parseAgentTimeout('30m');
1824+
expect(result).toEqual({ error: '--agent-timeout must be a positive integer (minutes)' });
1825+
});
1826+
1827+
it('should return error for decimal value', () => {
1828+
const result = parseAgentTimeout('1.5');
1829+
expect(result).toEqual({ error: '--agent-timeout must be a positive integer (minutes)' });
1830+
});
1831+
1832+
it('should return error for value with leading zero', () => {
1833+
const result = parseAgentTimeout('030');
1834+
expect(result).toEqual({ error: '--agent-timeout must be a positive integer (minutes)' });
1835+
});
1836+
});
1837+
1838+
describe('applyAgentTimeout', () => {
1839+
it('should do nothing when agentTimeout is undefined', () => {
1840+
const config: any = {};
1841+
const logger = { error: jest.fn(), info: jest.fn() };
1842+
applyAgentTimeout(undefined, config, logger);
1843+
expect(config.agentTimeout).toBeUndefined();
1844+
expect(logger.info).not.toHaveBeenCalled();
1845+
expect(logger.error).not.toHaveBeenCalled();
1846+
});
1847+
1848+
it('should set agentTimeout on config for valid value', () => {
1849+
const config: any = {};
1850+
const logger = { error: jest.fn(), info: jest.fn() };
1851+
applyAgentTimeout('30', config, logger);
1852+
expect(config.agentTimeout).toBe(30);
1853+
expect(logger.info).toHaveBeenCalledWith('Agent timeout set to 30 minutes');
1854+
});
1855+
1856+
it('should call process.exit for invalid value', () => {
1857+
const config: any = {};
1858+
const logger = { error: jest.fn(), info: jest.fn() };
1859+
const mockExit = jest.spyOn(process, 'exit').mockImplementation((() => {}) as any);
1860+
applyAgentTimeout('abc', config, logger);
1861+
expect(logger.error).toHaveBeenCalledWith('--agent-timeout must be a positive integer (minutes)');
1862+
expect(mockExit).toHaveBeenCalledWith(1);
1863+
mockExit.mockRestore();
1864+
});
1865+
});
1866+
1867+
describe('formatItem', () => {
1868+
it('should format term with description when term fits within width', () => {
1869+
const result = formatItem('--flag', 'Description text', 20, 2, 2, 80);
1870+
expect(result).toBe(' --flag Description text');
1871+
});
1872+
1873+
it('should wrap description to next line when term exceeds width', () => {
1874+
const result = formatItem('--very-long-flag-name-that-exceeds-width', 'Description', 10, 2, 2, 80);
1875+
expect(result).toContain('--very-long-flag-name-that-exceeds-width\n');
1876+
expect(result).toContain('Description');
1877+
});
1878+
1879+
it('should format term without description', () => {
1880+
const result = formatItem('--flag', '', 20, 2, 2, 80);
1881+
expect(result).toBe(' --flag');
1882+
});
1883+
});
17861884
});

src/cli.ts

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -509,6 +509,38 @@ export function parseMemoryLimit(input: string): { value: string; error?: undefi
509509
return { value: input.toLowerCase() };
510510
}
511511

512+
/**
513+
* Parses and validates the --agent-timeout option
514+
* @param value - The raw string value from the CLI option
515+
* @returns The parsed timeout in minutes, or an error
516+
*/
517+
export function parseAgentTimeout(value: string): { minutes: number } | { error: string } {
518+
if (!/^[1-9]\d*$/.test(value)) {
519+
return { error: '--agent-timeout must be a positive integer (minutes)' };
520+
}
521+
const timeoutMinutes = parseInt(value, 10);
522+
return { minutes: timeoutMinutes };
523+
}
524+
525+
/**
526+
* Applies the --agent-timeout option to the config if present.
527+
* Exits with code 1 if the value is invalid.
528+
*/
529+
export function applyAgentTimeout(
530+
agentTimeout: string | undefined,
531+
config: WrapperConfig,
532+
logger: { error: (msg: string) => void; info: (msg: string) => void }
533+
): void {
534+
if (agentTimeout === undefined) return;
535+
const result = parseAgentTimeout(agentTimeout);
536+
if ('error' in result) {
537+
logger.error(result.error);
538+
process.exit(1);
539+
}
540+
config.agentTimeout = result.minutes;
541+
logger.info(`Agent timeout set to ${result.minutes} minutes`);
542+
}
543+
512544
/**
513545
* Parses and validates DNS servers from a comma-separated string
514546
* @param input - Comma-separated DNS server string (e.g., "8.8.8.8,1.1.1.1")
@@ -1019,6 +1051,10 @@ program
10191051
'Keep containers running after command exits',
10201052
false
10211053
)
1054+
.option(
1055+
'--agent-timeout <minutes>',
1056+
'Maximum time in minutes for the agent command to run (default: no limit)',
1057+
)
10221058
.option(
10231059
'--work-dir <dir>',
10241060
'Working directory for temporary files',
@@ -1305,6 +1341,9 @@ program
13051341
anthropicApiTarget: options.anthropicApiTarget || process.env.ANTHROPIC_API_TARGET,
13061342
};
13071343

1344+
// Parse and validate --agent-timeout
1345+
applyAgentTimeout(options.agentTimeout, config, logger);
1346+
13081347
// Build rate limit config when API proxy is enabled
13091348
if (config.enableApiProxy) {
13101349
const rateLimitResult = buildRateLimitConfig(options);

src/docker-manager.test.ts

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2564,6 +2564,52 @@ describe('docker-manager', () => {
25642564
expect(result.exitCode).toBe(0);
25652565
expect(result.blockedDomains).toEqual([]);
25662566
});
2567+
2568+
it('should return exit code 124 when agent times out', async () => {
2569+
jest.useFakeTimers();
2570+
2571+
// Mock docker logs -f
2572+
mockExecaFn.mockResolvedValueOnce({ stdout: '', stderr: '', exitCode: 0 } as any);
2573+
// Mock docker wait - never resolves (simulates long-running command)
2574+
mockExecaFn.mockReturnValueOnce(new Promise(() => {}));
2575+
// Mock docker stop
2576+
mockExecaFn.mockResolvedValueOnce({ stdout: '', stderr: '', exitCode: 0 } as any);
2577+
2578+
const resultPromise = runAgentCommand(testDir, ['github.com'], undefined, 1);
2579+
2580+
// Use advanceTimersByTimeAsync to flush microtasks between timer advances
2581+
// This handles the 60s timeout AND the subsequent 500ms log flush delay
2582+
await jest.advanceTimersByTimeAsync(60 * 1000 + 1000);
2583+
2584+
const result = await resultPromise;
2585+
2586+
expect(result.exitCode).toBe(124);
2587+
// Verify docker stop was called
2588+
expect(mockExecaFn).toHaveBeenCalledWith('docker', ['stop', '-t', '10', 'awf-agent'], { reject: false });
2589+
2590+
jest.useRealTimers();
2591+
});
2592+
2593+
it('should return normal exit code when agent completes before timeout', async () => {
2594+
jest.useFakeTimers();
2595+
2596+
// Mock docker logs -f
2597+
mockExecaFn.mockResolvedValueOnce({ stdout: '', stderr: '', exitCode: 0 } as any);
2598+
// Mock docker wait - resolves immediately with exit code 0
2599+
mockExecaFn.mockResolvedValueOnce({ stdout: '0', stderr: '', exitCode: 0 } as any);
2600+
2601+
const resultPromise = runAgentCommand(testDir, ['github.com'], undefined, 30);
2602+
2603+
// Advance past the 500ms log flush delay
2604+
await jest.advanceTimersByTimeAsync(1000);
2605+
2606+
const result = await resultPromise;
2607+
2608+
expect(result.exitCode).toBe(0);
2609+
expect(result.blockedDomains).toEqual([]);
2610+
2611+
jest.useRealTimers();
2612+
});
25672613
});
25682614

25692615
describe('cleanup', () => {

src/docker-manager.ts

Lines changed: 34 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1513,7 +1513,7 @@ export async function startContainers(workDir: string, allowedDomains: string[],
15131513
/**
15141514
* Runs the agent command in the container and reports any blocked domains
15151515
*/
1516-
export async function runAgentCommand(workDir: string, allowedDomains: string[], proxyLogsDir?: string): Promise<{ exitCode: number; blockedDomains: string[] }> {
1516+
export async function runAgentCommand(workDir: string, allowedDomains: string[], proxyLogsDir?: string, agentTimeoutMinutes?: number): Promise<{ exitCode: number; blockedDomains: string[] }> {
15171517
logger.info('Executing agent command...');
15181518

15191519
try {
@@ -1524,13 +1524,40 @@ export async function runAgentCommand(workDir: string, allowedDomains: string[],
15241524
reject: false,
15251525
});
15261526

1527-
// Wait for the container to exit (this will run concurrently with log streaming)
1528-
const { stdout: exitCodeStr } = await execa('docker', [
1529-
'wait',
1530-
'awf-agent',
1531-
]);
1527+
let exitCode: number;
1528+
1529+
if (agentTimeoutMinutes) {
1530+
const timeoutMs = agentTimeoutMinutes * 60 * 1000;
1531+
logger.info(`Agent timeout: ${agentTimeoutMinutes} minutes`);
1532+
1533+
// Race docker wait against a timeout
1534+
const waitPromise = execa('docker', ['wait', 'awf-agent']).then(result => ({
1535+
type: 'completed' as const,
1536+
exitCodeStr: result.stdout,
1537+
}));
1538+
1539+
let timeoutTimer: ReturnType<typeof setTimeout>;
1540+
const timeoutPromise = new Promise<{ type: 'timeout' }>(resolve => {
1541+
timeoutTimer = setTimeout(() => resolve({ type: 'timeout' }), timeoutMs);
1542+
});
15321543

1533-
const exitCode = parseInt(exitCodeStr.trim(), 10);
1544+
const raceResult = await Promise.race([waitPromise, timeoutPromise]);
1545+
1546+
if (raceResult.type === 'timeout') {
1547+
logger.warn(`Agent command timed out after ${agentTimeoutMinutes} minutes, stopping container...`);
1548+
// Stop the container gracefully (10 second grace period before SIGKILL)
1549+
await execa('docker', ['stop', '-t', '10', 'awf-agent'], { reject: false });
1550+
exitCode = 124; // Standard timeout exit code (same as coreutils timeout)
1551+
} else {
1552+
// Clear the timeout timer so it doesn't keep the event loop alive
1553+
clearTimeout(timeoutTimer!);
1554+
exitCode = parseInt(raceResult.exitCodeStr.trim(), 10);
1555+
}
1556+
} else {
1557+
// No timeout - wait indefinitely
1558+
const { stdout: exitCodeStr } = await execa('docker', ['wait', 'awf-agent']);
1559+
exitCode = parseInt(exitCodeStr.trim(), 10);
1560+
}
15341561

15351562
// Wait for the logs process to finish (it should exit automatically when container stops)
15361563
await logsProcess;

src/types.ts

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -550,6 +550,22 @@ export interface WrapperConfig {
550550
* ```
551551
*/
552552
anthropicApiTarget?: string;
553+
554+
/**
555+
* Maximum time in minutes to allow the agent command to run
556+
*
557+
* When specified, the agent container is forcibly stopped after this many
558+
* minutes. Useful for large projects where builds or tests may exceed
559+
* default CI timeouts.
560+
*
561+
* When not specified, the agent runs indefinitely until the command completes
562+
* or the process is externally terminated.
563+
*
564+
* @default undefined (no timeout)
565+
* @example 30
566+
* @example 45
567+
*/
568+
agentTimeout?: number;
553569
}
554570

555571
/**

0 commit comments

Comments
 (0)