feat(cli): add --memory-limit flag for configurable container memory#1243
feat(cli): add --memory-limit flag for configurable container memory#1243
Conversation
Reduce default agent container memory from 4GB to 2GB for better DoS protection in shared environments. Add --memory-limit flag to override (e.g., --memory-limit 8g for AI agent workloads). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
✅ Coverage Check PassedOverall Coverage
📁 Per-file Coverage Changes (2 files)
Coverage comparison generated by |
There was a problem hiding this comment.
Pull request overview
Adds a configurable memory limit for the agent execution container to improve resource-hardening in shared CI/CD environments, lowering the default memory cap while allowing explicit overrides via CLI.
Changes:
- Introduces
WrapperConfig.memoryLimit(documented default2g) and wires it into Docker Compose generation for the agent container. - Adds
--memory-limitCLI flag with validation (parseMemoryLimit) and passes the validated value into runtime config. - Updates and extends unit tests to cover the new default and custom overrides.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/types.ts | Adds memoryLimit?: string to WrapperConfig with docstring/default guidance. |
| src/docker-manager.ts | Uses config.memoryLimit (fallback 2g) to set agent mem_limit / memswap_limit. |
| src/docker-manager.test.ts | Updates default resource limit assertions and adds a custom memory limit test. |
| src/cli.ts | Adds --memory-limit option, validation via parseMemoryLimit, and passes it into WrapperConfig. |
| src/cli.test.ts | Adds unit tests for parseMemoryLimit valid/invalid inputs. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| export function parseMemoryLimit(input: string): { value: string; error?: undefined } | { value?: undefined; error: string } { | ||
| const pattern = /^(\d+)([bkmg])$/i; | ||
| const match = input.match(pattern); | ||
| if (!match) { | ||
| return { error: `Invalid --memory-limit value "${input}". Expected format: <number><unit> (e.g., 2g, 512m, 4g)` }; | ||
| } | ||
| const num = parseInt(match[1], 10); | ||
| if (num <= 0) { | ||
| return { error: `Invalid --memory-limit value "${input}". Memory limit must be a positive number.` }; | ||
| } | ||
| return { value: input.toLowerCase() }; | ||
| } |
There was a problem hiding this comment.
parseMemoryLimit matches the input string as-is; unlike other parsers in this file (e.g., DNS parsing), it doesn’t trim whitespace first. This causes values like "2g " (or values copied with trailing whitespace) to be rejected even though they’re otherwise valid. Consider normalizing with input.trim() (and returning the normalized value) before applying the regex.
| mem_limit: config.memoryLimit || '2g', | ||
| memswap_limit: config.memoryLimit || '2g', // No swap (same as mem_limit) |
There was a problem hiding this comment.
mem_limit and memswap_limit both repeat the same fallback expression. To avoid accidental divergence and make the default clearer, consider computing a single local value (e.g., memoryLimit = config.memoryLimit ?? '2g') and reusing it for both fields.
| .option( | ||
| '--memory-limit <limit>', | ||
| 'Memory limit for the agent container (e.g., 1g, 2g, 4g, 512m). Default: 2g', | ||
| '2g' | ||
| ) |
There was a problem hiding this comment.
PR description / issue acceptance criteria mention updating user-facing docs (README.md / AGENTS.md) for the new --memory-limit flag and the new 2g default, but this PR only changes code/tests. Either add the documentation updates or adjust the PR description so it matches what’s actually included.
Smoke Test Results
Overall: PASS
|
Smoke Test Results
Overall: PASS | Author:
|
|
PR titles: test: expand credential hiding tests to all 14 protected paths; test(docker): verify capsh execution chain after PR #715; feat(cli): add --memory-limit flag for configurable container memory; feat(cli): add --agent-timeout flag for execution time limit
|
Chroot Version Comparison Results
Result: ❌ Not all tests passed — Python and Node.js versions differ between host and chroot environment.
|
🏗️ Build Test Suite Results
Overall: 8/8 ecosystems passed — ✅ PASS
|
Summary
--memory-limitCLI flag accepting Docker memory format (e.g.,2g,512m,8g) to override the defaultFixes #310
Test plan
parseMemoryLimitvalidation (valid formats, invalid formats, zero)🤖 Generated with Claude Code