feat: add parallel sandbox execution via --multi flag (v2.1)#54
Merged
feat: add parallel sandbox execution via --multi flag (v2.1)#54
Conversation
Add support for executing multiple tasks in parallel E2B sandboxes: - New ParallelExecutor class orchestrates concurrent task execution - ConcurrencyLimiter utility provides semaphore-based parallelism control - Extend sandbox run command with --multi, --task, --task-file options - Support fail-fast mode, configurable concurrency, and result aggregation - Generate markdown summary reports with execution statistics New CLI options: --multi Execute multiple tasks in parallel --task <text...> Task description (repeatable) --task-file <path> File with one task per line --max-concurrent <n> Max parallel sandboxes (default: 3) --fail-fast Stop all tasks on first failure --output-dir <path> Results directory (default: ./parallel-results) Includes 28 unit tests and 24 CLI integration tests.
WalkthroughThis pull request introduces parallel multi-task execution capabilities (v2.1) for sandbox runs, enabling users to execute multiple tasks concurrently with configurable concurrency limits, fail-fast semantics, and aggregated results reporting. The implementation adds new CLI flags, a ParallelExecutor orchestrator, concurrency utilities, and comprehensive test coverage. Changes
Sequence Diagram(s)sequenceDiagram
participant User as User/CLI
participant CLI as CLI Handler
participant PE as ParallelExecutor
participant CL as ConcurrencyLimiter
participant Coord as Coordinator
participant SM as SandboxManager
participant SB as Sandbox
participant FS as File System
User->>CLI: sandbox run --multi --task="..." --max-concurrent=3
CLI->>CLI: Parse & validate tasks, credentials
CLI->>PE: new ParallelExecutor(config)
CLI->>PE: execute(onProgress callback)
PE->>PE: Initialize task queue
PE->>CL: Enforce max concurrency
par Parallel Task Execution
PE->>Coord: registerSession(task1)
PE->>SM: createWorktree(task1)
PE->>SM: uploadTarballs(task1)
Coord->>SB: Launch sandbox
PE->>SB: Execute Claude in sandbox
Note over SB: Process task logic
PE->>SM: downloadChangedFiles(task1)
PE->>FS: Write output/task1/
PE->>PE: Emit progress update
PE->>Coord: registerSession(task2)
PE->>SM: createWorktree(task2)
PE->>SM: uploadTarballs(task2)
Coord->>SB: Launch sandbox
PE->>SB: Execute Claude in sandbox
PE->>SM: downloadChangedFiles(task2)
PE->>FS: Write output/task2/
PE->>PE: Emit progress update
PE->>Coord: registerSession(task3)
PE->>SM: createWorktree(task3)
PE->>SM: uploadTarballs(task3)
Coord->>SB: Launch sandbox
PE->>SB: Execute Claude in sandbox
PE->>SM: downloadChangedFiles(task3)
PE->>FS: Write output/task3/
PE->>PE: Emit progress update
end
PE->>PE: Aggregate results & summary
PE->>FS: Write markdown report
PE->>SM: Cleanup (terminate sandboxes)
PE->>Coord: Release sessions
PE-->>CLI: ParallelExecutionResult
CLI->>User: Output summary (JSON or human-readable)
CLI->>User: Exit with status code
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes The changes span multiple systems with substantial new logic: a complex ParallelExecutor with concurrent task orchestration, extensive CLI modifications with validation and routing, new type definitions, concurrency utilities, and 2,500+ lines across heterogeneous domains. Review requires understanding parallel execution semantics, state management, error handling, and integration across CLI, coordination, and sandbox layers. Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
Contributor
Add parallel sandbox execution to CLI
|
5 tasks
Code Review: Parallel Sandbox Execution (v2.1)Overall AssessmentThis PR adds a well-architected parallel execution feature for E2B sandboxes. The code follows project conventions, includes comprehensive tests (28 unit + 24 CLI integration tests), and passes the TypeScript build. The implementation is clean with proper error handling and resource cleanup. Code Quality ✅Strengths:
Minor Suggestions:
Potential Bugs / Issues
|
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
| private async cancelRemainingTasks(): Promise<void> { | ||
| this.cancelled = true; | ||
| this.logger.info('Cancelling remaining tasks'); | ||
|
|
Contributor
There was a problem hiding this comment.
🟡 Medium
src/e2b/parallel-executor.ts:431
Race condition: tasks initializing when cancelRemainingTasks runs can escape cancellation. The sandbox ID isn't in taskSandboxIds until after createSandbox completes, and executeTask doesn't re-check this.cancelled after its await points. Consider adding cancellation checks after createSandbox and register calls in executeTask.
🚀 Want me to fix this? Reply ex: "fix it for me".
| worktreePath = registerResult.worktreePath; | ||
|
|
||
| this.logger.info(`[${taskId}] Registered session ${sessionId}, worktree: ${worktreePath}`); | ||
|
|
Contributor
There was a problem hiding this comment.
🟡 Medium
src/e2b/parallel-executor.ts:255
Fake PIDs generated here will fail the isProcessAlive check in Coordinator, causing all tasks to use the main repo instead of isolated worktrees. Consider using actual child process PIDs or modifying the Coordinator API to accept session-based registration without PID validation.
🚀 Want me to fix this? Reply ex: "fix it for me".
| const downloadPath = path.join(outputPath, 'changed-files'); | ||
| await fs.mkdir(downloadPath, { recursive: true }); | ||
|
|
||
| const downloadResult = await downloadChangedFiles(sandbox, '/workspace', downloadPath); |
Contributor
There was a problem hiding this comment.
🟡 Medium
src/e2b/parallel-executor.ts:388
Consider checking downloadResult.success before returning. Currently, a download failure returns success: true with 0 files changed, masking potential data loss.
- const downloadResult = await downloadChangedFiles(sandbox, '/workspace', downloadPath);
+ const downloadResult = await downloadChangedFiles(sandbox, '/workspace', downloadPath);
+ if (!downloadResult.success) {
+ return {
+ success: false,
+ exitCode: executionResult.exitCode,
+ filesChanged: 0,
+ error: `Download failed: ${downloadResult.error}`
+ };
+ }🚀 Want me to fix this? Reply ex: "fix it for me".
|
fatal: could not read Username for 'https://github.com': No such device or address |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@src/cli.ts`:
- Around line 2396-2485: The code does not validate options.authMethod and the
parsed maxConcurrent, so an invalid auth method can bypass checks and NaN
maxConcurrent stalls the limiter; update the validation around authMethod (the
authMethod variable derived from options.authMethod) to explicitly reject values
other than 'api-key' or 'oauth' with the same JSON/console error +
process.exit(1) flow used elsewhere, and validate maxConcurrent by parsing
parseInt(options.maxConcurrent || '3', 10), checking Number.isFinite(parsed) and
falling back to a safe default (e.g., 3) or erroring if invalid before building
the config (the config.maxConcurrent property), ensuring the limiter always
receives a valid numeric concurrency.
In `@src/e2b/parallel-executor.ts`:
- Around line 383-413: The code currently ignores failures from
downloadChangedFiles; update the block after calling
downloadChangedFiles(sandbox, '/workspace', downloadPath) to check
downloadResult.success and, if false, set executionResult.success = false (or a
local success flag), merge downloadResult.error into the metadata error field,
include a clear error message in the metadata.json and return payload (the
returned object from this function) with success: false, error:
downloadResult.error (and filesChanged: downloadResult.filesDownloaded if
present); ensure the log (execution.log) is still written but that metadata and
the returned object reflect the download failure so callers do not treat the
task as successful.
In `@tests/cli-sandbox-run-multi.test.ts`:
- Around line 12-37: The runCli helper currently shells the joined args string
through execSync which breaks quoted arguments like "Task 1"; change runCli to
call a child-process API that accepts an args array (e.g., spawnSync or
execFileSync) instead of execSync with a joined string so spaces are preserved;
invoke the binary as ['dist/cli.js', ...args] (or spawnSync('node',
['dist/cli.js', ...args], ...)) and update the result/error handling to read
stdout, stderr, and status from the sync call’s return value
(result.stdout/result.stderr/result.status) rather than relying on thrown
error.stdout/error.stderr so tests keep arguments with spaces intact.
🧹 Nitpick comments (1)
src/cli.ts (1)
2400-2441: Include actionablehintfields in JSON errors for missing credentials.
This keeps machine output consistent with the guidance printed in non-JSON mode. As per coding guidelines: Use consistent error messages with actionable troubleshooting guidance for common failures.🛠️ Suggested fix
if (!process.env.ANTHROPIC_API_KEY) { if (options.json) { console.log(JSON.stringify({ success: false, - error: 'ANTHROPIC_API_KEY environment variable not set' + error: 'ANTHROPIC_API_KEY environment variable not set', + hint: 'Set ANTHROPIC_API_KEY or use --auth-method oauth' })); } else { console.error(chalk.red('✗ ANTHROPIC_API_KEY environment variable not set')); console.log(chalk.dim('Set ANTHROPIC_API_KEY or use --auth-method oauth')); } @@ if (!existsSync(credentialsPath)) { if (options.json) { console.log(JSON.stringify({ success: false, - error: 'OAuth credentials not found. Run "claude /login" first' + error: 'OAuth credentials not found. Run "claude /login" first', + hint: 'Run `claude /login` to create ~/.claude/.credentials.json' })); } else { console.error(chalk.red('✗ OAuth credentials not found')); console.log(chalk.dim('Run "claude /login" to authenticate with your Claude subscription')); } @@ if (!process.env.E2B_API_KEY) { if (options.json) { console.log(JSON.stringify({ success: false, - error: 'E2B_API_KEY environment variable not set' + error: 'E2B_API_KEY environment variable not set', + hint: 'Set E2B_API_KEY to use E2B sandbox execution' })); } else { console.error(chalk.red('✗ E2B_API_KEY environment variable not set')); console.log(chalk.dim('Set E2B_API_KEY to use E2B sandbox execution')); }
Comment on lines
+2396
to
+2485
| // Step 2: Validate authentication | ||
| const authMethod = options.authMethod as 'api-key' | 'oauth'; | ||
| let oauthCredentials: string | undefined; | ||
|
|
||
| if (authMethod === 'api-key') { | ||
| if (!process.env.ANTHROPIC_API_KEY) { | ||
| if (options.json) { | ||
| console.log(JSON.stringify({ | ||
| success: false, | ||
| error: 'ANTHROPIC_API_KEY environment variable not set' | ||
| })); | ||
| } else { | ||
| console.error(chalk.red('✗ ANTHROPIC_API_KEY environment variable not set')); | ||
| console.log(chalk.dim('Set ANTHROPIC_API_KEY or use --auth-method oauth')); | ||
| } | ||
| process.exit(1); | ||
| } | ||
| } else if (authMethod === 'oauth') { | ||
| const credentialsPath = path.join(os.homedir(), '.claude', '.credentials.json'); | ||
| if (!existsSync(credentialsPath)) { | ||
| if (options.json) { | ||
| console.log(JSON.stringify({ | ||
| success: false, | ||
| error: 'OAuth credentials not found. Run "claude /login" first' | ||
| })); | ||
| } else { | ||
| console.error(chalk.red('✗ OAuth credentials not found')); | ||
| console.log(chalk.dim('Run "claude /login" to authenticate with your Claude subscription')); | ||
| } | ||
| process.exit(1); | ||
| } | ||
| oauthCredentials = await fs.readFile(credentialsPath, 'utf-8'); | ||
| } | ||
|
|
||
| // Step 3: Validate E2B API key | ||
| if (!process.env.E2B_API_KEY) { | ||
| if (options.json) { | ||
| console.log(JSON.stringify({ | ||
| success: false, | ||
| error: 'E2B_API_KEY environment variable not set' | ||
| })); | ||
| } else { | ||
| console.error(chalk.red('✗ E2B_API_KEY environment variable not set')); | ||
| console.log(chalk.dim('Set E2B_API_KEY to use E2B sandbox execution')); | ||
| } | ||
| process.exit(1); | ||
| } | ||
|
|
||
| // Step 4: Resolve repository path | ||
| const repoPath = path.resolve(options.repo); | ||
| if (!existsSync(repoPath)) { | ||
| if (options.json) { | ||
| console.log(JSON.stringify({ success: false, error: `Repository path not found: ${repoPath}` })); | ||
| } else { | ||
| console.error(chalk.red(`✗ Repository path not found: ${repoPath}`)); | ||
| } | ||
| process.exit(1); | ||
| } | ||
|
|
||
| // Step 5: Create output directory | ||
| const outputDir = path.resolve(options.outputDir || './parallel-results'); | ||
| await fs.mkdir(outputDir, { recursive: true }); | ||
|
|
||
| // Step 6: Create sandbox manager | ||
| const sandboxImage = options.template || | ||
| (process.env.E2B_TEMPLATE?.trim() || '') || | ||
| 'anthropic-claude-code'; | ||
| const sandboxManager = new SandboxManager(logger, { sandboxImage }); | ||
|
|
||
| // Step 7: Build configuration | ||
| const config = { | ||
| tasks, | ||
| maxConcurrent: parseInt(options.maxConcurrent || '3', 10), | ||
| failFast: options.failFast || false, | ||
| outputDir, | ||
| repoPath, | ||
| authMethod, | ||
| sandboxImage: options.template, | ||
| templateName: options.useTemplate, | ||
| branch: options.branch, | ||
| gitLive: options.gitLive || false, | ||
| targetBranch: options.targetBranch || 'main', | ||
| gitUser: options.gitUser, | ||
| gitEmail: options.gitEmail, | ||
| oauthCredentials, | ||
| budgetPerTask: options.budget ? parseFloat(options.budget) : undefined, | ||
| npmToken: options.npmToken || process.env.PARALLEL_CC_NPM_TOKEN, | ||
| npmRegistry: options.npmRegistry, | ||
| sshKeyPath: options.sshKey | ||
| }; |
There was a problem hiding this comment.
Validate --auth-method and --max-concurrent before building config.
An invalid auth method bypasses checks, and a non-numeric --max-concurrent (NaN) causes the limiter to never acquire a slot, stalling all tasks.
🛠️ Suggested fix
- const authMethod = options.authMethod as 'api-key' | 'oauth';
+ if (options.authMethod !== 'api-key' && options.authMethod !== 'oauth') {
+ const message = `Invalid --auth-method "${options.authMethod}". Use "api-key" or "oauth".`;
+ if (options.json) {
+ console.log(JSON.stringify({ success: false, error: message, hint: 'Example: --auth-method api-key' }));
+ } else {
+ console.error(chalk.red(`✗ ${message}`));
+ }
+ process.exit(1);
+ }
+ const authMethod = options.authMethod as 'api-key' | 'oauth';
@@
- const config = {
- tasks,
- maxConcurrent: parseInt(options.maxConcurrent || '3', 10),
+ const maxConcurrent = Number.parseInt(options.maxConcurrent || '3', 10);
+ if (!Number.isFinite(maxConcurrent) || maxConcurrent < 1) {
+ const message = `Invalid --max-concurrent "${options.maxConcurrent}". Must be a positive integer.`;
+ if (options.json) {
+ console.log(JSON.stringify({ success: false, error: message, hint: 'Example: --max-concurrent 3' }));
+ } else {
+ console.error(chalk.red(`✗ ${message}`));
+ }
+ process.exit(1);
+ }
+
+ const config = {
+ tasks,
+ maxConcurrent,
failFast: options.failFast || false,
outputDir,
repoPath,
authMethod,📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| // Step 2: Validate authentication | |
| const authMethod = options.authMethod as 'api-key' | 'oauth'; | |
| let oauthCredentials: string | undefined; | |
| if (authMethod === 'api-key') { | |
| if (!process.env.ANTHROPIC_API_KEY) { | |
| if (options.json) { | |
| console.log(JSON.stringify({ | |
| success: false, | |
| error: 'ANTHROPIC_API_KEY environment variable not set' | |
| })); | |
| } else { | |
| console.error(chalk.red('✗ ANTHROPIC_API_KEY environment variable not set')); | |
| console.log(chalk.dim('Set ANTHROPIC_API_KEY or use --auth-method oauth')); | |
| } | |
| process.exit(1); | |
| } | |
| } else if (authMethod === 'oauth') { | |
| const credentialsPath = path.join(os.homedir(), '.claude', '.credentials.json'); | |
| if (!existsSync(credentialsPath)) { | |
| if (options.json) { | |
| console.log(JSON.stringify({ | |
| success: false, | |
| error: 'OAuth credentials not found. Run "claude /login" first' | |
| })); | |
| } else { | |
| console.error(chalk.red('✗ OAuth credentials not found')); | |
| console.log(chalk.dim('Run "claude /login" to authenticate with your Claude subscription')); | |
| } | |
| process.exit(1); | |
| } | |
| oauthCredentials = await fs.readFile(credentialsPath, 'utf-8'); | |
| } | |
| // Step 3: Validate E2B API key | |
| if (!process.env.E2B_API_KEY) { | |
| if (options.json) { | |
| console.log(JSON.stringify({ | |
| success: false, | |
| error: 'E2B_API_KEY environment variable not set' | |
| })); | |
| } else { | |
| console.error(chalk.red('✗ E2B_API_KEY environment variable not set')); | |
| console.log(chalk.dim('Set E2B_API_KEY to use E2B sandbox execution')); | |
| } | |
| process.exit(1); | |
| } | |
| // Step 4: Resolve repository path | |
| const repoPath = path.resolve(options.repo); | |
| if (!existsSync(repoPath)) { | |
| if (options.json) { | |
| console.log(JSON.stringify({ success: false, error: `Repository path not found: ${repoPath}` })); | |
| } else { | |
| console.error(chalk.red(`✗ Repository path not found: ${repoPath}`)); | |
| } | |
| process.exit(1); | |
| } | |
| // Step 5: Create output directory | |
| const outputDir = path.resolve(options.outputDir || './parallel-results'); | |
| await fs.mkdir(outputDir, { recursive: true }); | |
| // Step 6: Create sandbox manager | |
| const sandboxImage = options.template || | |
| (process.env.E2B_TEMPLATE?.trim() || '') || | |
| 'anthropic-claude-code'; | |
| const sandboxManager = new SandboxManager(logger, { sandboxImage }); | |
| // Step 7: Build configuration | |
| const config = { | |
| tasks, | |
| maxConcurrent: parseInt(options.maxConcurrent || '3', 10), | |
| failFast: options.failFast || false, | |
| outputDir, | |
| repoPath, | |
| authMethod, | |
| sandboxImage: options.template, | |
| templateName: options.useTemplate, | |
| branch: options.branch, | |
| gitLive: options.gitLive || false, | |
| targetBranch: options.targetBranch || 'main', | |
| gitUser: options.gitUser, | |
| gitEmail: options.gitEmail, | |
| oauthCredentials, | |
| budgetPerTask: options.budget ? parseFloat(options.budget) : undefined, | |
| npmToken: options.npmToken || process.env.PARALLEL_CC_NPM_TOKEN, | |
| npmRegistry: options.npmRegistry, | |
| sshKeyPath: options.sshKey | |
| }; | |
| // Step 2: Validate authentication | |
| if (options.authMethod !== 'api-key' && options.authMethod !== 'oauth') { | |
| const message = `Invalid --auth-method "${options.authMethod}". Use "api-key" or "oauth".`; | |
| if (options.json) { | |
| console.log(JSON.stringify({ success: false, error: message, hint: 'Example: --auth-method api-key' })); | |
| } else { | |
| console.error(chalk.red(`✗ ${message}`)); | |
| } | |
| process.exit(1); | |
| } | |
| const authMethod = options.authMethod as 'api-key' | 'oauth'; | |
| let oauthCredentials: string | undefined; | |
| if (authMethod === 'api-key') { | |
| if (!process.env.ANTHROPIC_API_KEY) { | |
| if (options.json) { | |
| console.log(JSON.stringify({ | |
| success: false, | |
| error: 'ANTHROPIC_API_KEY environment variable not set' | |
| })); | |
| } else { | |
| console.error(chalk.red('✗ ANTHROPIC_API_KEY environment variable not set')); | |
| console.log(chalk.dim('Set ANTHROPIC_API_KEY or use --auth-method oauth')); | |
| } | |
| process.exit(1); | |
| } | |
| } else if (authMethod === 'oauth') { | |
| const credentialsPath = path.join(os.homedir(), '.claude', '.credentials.json'); | |
| if (!existsSync(credentialsPath)) { | |
| if (options.json) { | |
| console.log(JSON.stringify({ | |
| success: false, | |
| error: 'OAuth credentials not found. Run "claude /login" first' | |
| })); | |
| } else { | |
| console.error(chalk.red('✗ OAuth credentials not found')); | |
| console.log(chalk.dim('Run "claude /login" to authenticate with your Claude subscription')); | |
| } | |
| process.exit(1); | |
| } | |
| oauthCredentials = await fs.readFile(credentialsPath, 'utf-8'); | |
| } | |
| // Step 3: Validate E2B API key | |
| if (!process.env.E2B_API_KEY) { | |
| if (options.json) { | |
| console.log(JSON.stringify({ | |
| success: false, | |
| error: 'E2B_API_KEY environment variable not set' | |
| })); | |
| } else { | |
| console.error(chalk.red('✗ E2B_API_KEY environment variable not set')); | |
| console.log(chalk.dim('Set E2B_API_KEY to use E2B sandbox execution')); | |
| } | |
| process.exit(1); | |
| } | |
| // Step 4: Resolve repository path | |
| const repoPath = path.resolve(options.repo); | |
| if (!existsSync(repoPath)) { | |
| if (options.json) { | |
| console.log(JSON.stringify({ success: false, error: `Repository path not found: ${repoPath}` })); | |
| } else { | |
| console.error(chalk.red(`✗ Repository path not found: ${repoPath}`)); | |
| } | |
| process.exit(1); | |
| } | |
| // Step 5: Create output directory | |
| const outputDir = path.resolve(options.outputDir || './parallel-results'); | |
| await fs.mkdir(outputDir, { recursive: true }); | |
| // Step 6: Create sandbox manager | |
| const sandboxImage = options.template || | |
| (process.env.E2B_TEMPLATE?.trim() || '') || | |
| 'anthropic-claude-code'; | |
| const sandboxManager = new SandboxManager(logger, { sandboxImage }); | |
| // Step 7: Build configuration | |
| const maxConcurrent = Number.parseInt(options.maxConcurrent || '3', 10); | |
| if (!Number.isFinite(maxConcurrent) || maxConcurrent < 1) { | |
| const message = `Invalid --max-concurrent "${options.maxConcurrent}". Must be a positive integer.`; | |
| if (options.json) { | |
| console.log(JSON.stringify({ success: false, error: message, hint: 'Example: --max-concurrent 3' })); | |
| } else { | |
| console.error(chalk.red(`✗ ${message}`)); | |
| } | |
| process.exit(1); | |
| } | |
| const config = { | |
| tasks, | |
| maxConcurrent, | |
| failFast: options.failFast || false, | |
| outputDir, | |
| repoPath, | |
| authMethod, | |
| sandboxImage: options.template, | |
| templateName: options.useTemplate, | |
| branch: options.branch, | |
| gitLive: options.gitLive || false, | |
| targetBranch: options.targetBranch || 'main', | |
| gitUser: options.gitUser, | |
| gitEmail: options.gitEmail, | |
| oauthCredentials, | |
| budgetPerTask: options.budget ? parseFloat(options.budget) : undefined, | |
| npmToken: options.npmToken || process.env.PARALLEL_CC_NPM_TOKEN, | |
| npmRegistry: options.npmRegistry, | |
| sshKeyPath: options.sshKey | |
| }; |
🤖 Prompt for AI Agents
In `@src/cli.ts` around lines 2396 - 2485, The code does not validate
options.authMethod and the parsed maxConcurrent, so an invalid auth method can
bypass checks and NaN maxConcurrent stalls the limiter; update the validation
around authMethod (the authMethod variable derived from options.authMethod) to
explicitly reject values other than 'api-key' or 'oauth' with the same
JSON/console error + process.exit(1) flow used elsewhere, and validate
maxConcurrent by parsing parseInt(options.maxConcurrent || '3', 10), checking
Number.isFinite(parsed) and falling back to a safe default (e.g., 3) or erroring
if invalid before building the config (the config.maxConcurrent property),
ensuring the limiter always receives a valid numeric concurrency.
Comment on lines
+383
to
+413
| // Step 4: Download changed files | ||
| this.logger.info(`[${taskId}] Downloading changed files`); | ||
| const downloadPath = path.join(outputPath, 'changed-files'); | ||
| await fs.mkdir(downloadPath, { recursive: true }); | ||
|
|
||
| const downloadResult = await downloadChangedFiles(sandbox, '/workspace', downloadPath); | ||
|
|
||
| // Step 5: Save execution log | ||
| const logPath = path.join(outputPath, 'execution.log'); | ||
| const logContent = executionResult.fullOutput || executionResult.output || ''; | ||
| await fs.writeFile(logPath, logContent); | ||
|
|
||
| // Step 6: Save metadata | ||
| const metadataPath = path.join(outputPath, 'metadata.json'); | ||
| await fs.writeFile(metadataPath, JSON.stringify({ | ||
| taskId, | ||
| prompt, | ||
| exitCode: executionResult.exitCode, | ||
| executionTime: executionResult.executionTime, | ||
| filesDownloaded: downloadResult.filesDownloaded, | ||
| success: executionResult.success, | ||
| state: executionResult.state, | ||
| error: executionResult.error | ||
| }, null, 2)); | ||
|
|
||
| return { | ||
| success: executionResult.success, | ||
| exitCode: executionResult.exitCode, | ||
| filesChanged: downloadResult.filesDownloaded, | ||
| error: executionResult.error | ||
| }; |
There was a problem hiding this comment.
Handle failed downloads so task status reflects missing outputs.
downloadChangedFiles can fail, but the result is ignored, so tasks can report success with empty outputs. Check downloadResult.success and surface the error.
🛠️ Suggested fix
- const downloadResult = await downloadChangedFiles(sandbox, '/workspace', downloadPath);
+ const downloadResult = await downloadChangedFiles(sandbox, '/workspace', downloadPath);
+ const downloadError = !downloadResult.success
+ ? `Download failed: ${downloadResult.error || 'Unknown error'}`
+ : undefined;
@@
await fs.writeFile(metadataPath, JSON.stringify({
taskId,
prompt,
exitCode: executionResult.exitCode,
executionTime: executionResult.executionTime,
filesDownloaded: downloadResult.filesDownloaded,
success: executionResult.success,
state: executionResult.state,
- error: executionResult.error
+ error: downloadError ?? executionResult.error
}, null, 2));
return {
- success: executionResult.success,
+ success: executionResult.success && !downloadError,
exitCode: executionResult.exitCode,
- filesChanged: downloadResult.filesDownloaded,
- error: executionResult.error
+ filesChanged: downloadResult.success ? downloadResult.filesDownloaded : 0,
+ error: downloadError ?? executionResult.error
};🤖 Prompt for AI Agents
In `@src/e2b/parallel-executor.ts` around lines 383 - 413, The code currently
ignores failures from downloadChangedFiles; update the block after calling
downloadChangedFiles(sandbox, '/workspace', downloadPath) to check
downloadResult.success and, if false, set executionResult.success = false (or a
local success flag), merge downloadResult.error into the metadata error field,
include a clear error message in the metadata.json and return payload (the
returned object from this function) with success: false, error:
downloadResult.error (and filesChanged: downloadResult.filesDownloaded if
present); ensure the log (execution.log) is still written but that metadata and
the returned object reflect the download failure so callers do not treat the
task as successful.
Comment on lines
+12
to
+37
| import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; | ||
| import { execSync, spawn } from 'child_process'; | ||
| import * as fs from 'fs/promises'; | ||
| import * as path from 'path'; | ||
| import * as os from 'os'; | ||
|
|
||
| // Helper to run CLI command and capture output | ||
| function runCli(args: string[], options: { env?: Record<string, string> } = {}): { | ||
| stdout: string; | ||
| stderr: string; | ||
| exitCode: number | null; | ||
| } { | ||
| try { | ||
| const stdout = execSync(`node dist/cli.js ${args.join(' ')}`, { | ||
| encoding: 'utf-8', | ||
| env: { ...process.env, ...options.env }, | ||
| timeout: 5000 | ||
| }); | ||
| return { stdout, stderr: '', exitCode: 0 }; | ||
| } catch (error: any) { | ||
| return { | ||
| stdout: error.stdout || '', | ||
| stderr: error.stderr || '', | ||
| exitCode: error.status || 1 | ||
| }; | ||
| } |
There was a problem hiding this comment.
Avoid shell-splitting CLI args so tasks with spaces are preserved.
execSync with a joined string splits "Task 1" into separate tokens, which can make the tests less representative. Use spawnSync/execFileSync with an args array instead.
🛠️ Suggested fix
-import { execSync, spawn } from 'child_process';
+import { spawnSync } from 'child_process';
@@
-function runCli(args: string[], options: { env?: Record<string, string> } = {}): {
+function runCli(args: string[], options: { env?: Record<string, string> } = {}): {
stdout: string;
stderr: string;
exitCode: number | null;
} {
- try {
- const stdout = execSync(`node dist/cli.js ${args.join(' ')}`, {
- encoding: 'utf-8',
- env: { ...process.env, ...options.env },
- timeout: 5000
- });
- return { stdout, stderr: '', exitCode: 0 };
- } catch (error: any) {
- return {
- stdout: error.stdout || '',
- stderr: error.stderr || '',
- exitCode: error.status || 1
- };
- }
+ const result = spawnSync('node', ['dist/cli.js', ...args], {
+ encoding: 'utf-8',
+ env: { ...process.env, ...options.env },
+ timeout: 5000
+ });
+ return {
+ stdout: result.stdout ?? '',
+ stderr: result.stderr ?? (result.error ? String(result.error) : ''),
+ exitCode: result.status ?? 1
+ };
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; | |
| import { execSync, spawn } from 'child_process'; | |
| import * as fs from 'fs/promises'; | |
| import * as path from 'path'; | |
| import * as os from 'os'; | |
| // Helper to run CLI command and capture output | |
| function runCli(args: string[], options: { env?: Record<string, string> } = {}): { | |
| stdout: string; | |
| stderr: string; | |
| exitCode: number | null; | |
| } { | |
| try { | |
| const stdout = execSync(`node dist/cli.js ${args.join(' ')}`, { | |
| encoding: 'utf-8', | |
| env: { ...process.env, ...options.env }, | |
| timeout: 5000 | |
| }); | |
| return { stdout, stderr: '', exitCode: 0 }; | |
| } catch (error: any) { | |
| return { | |
| stdout: error.stdout || '', | |
| stderr: error.stderr || '', | |
| exitCode: error.status || 1 | |
| }; | |
| } | |
| import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; | |
| import { spawnSync } from 'child_process'; | |
| import * as fs from 'fs/promises'; | |
| import * as path from 'path'; | |
| import * as os from 'os'; | |
| // Helper to run CLI command and capture output | |
| function runCli(args: string[], options: { env?: Record<string, string> } = {}): { | |
| stdout: string; | |
| stderr: string; | |
| exitCode: number | null; | |
| } { | |
| const result = spawnSync('node', ['dist/cli.js', ...args], { | |
| encoding: 'utf-8', | |
| env: { ...process.env, ...options.env }, | |
| timeout: 5000 | |
| }); | |
| return { | |
| stdout: result.stdout ?? '', | |
| stderr: result.stderr ?? (result.error ? String(result.error) : ''), | |
| exitCode: result.status ?? 1 | |
| }; | |
| } |
🤖 Prompt for AI Agents
In `@tests/cli-sandbox-run-multi.test.ts` around lines 12 - 37, The runCli helper
currently shells the joined args string through execSync which breaks quoted
arguments like "Task 1"; change runCli to call a child-process API that accepts
an args array (e.g., spawnSync or execFileSync) instead of execSync with a
joined string so spaces are preserved; invoke the binary as ['dist/cli.js',
...args] (or spawnSync('node', ['dist/cli.js', ...args], ...)) and update the
result/error handling to read stdout, stderr, and status from the sync call’s
return value (result.stdout/result.stderr/result.status) rather than relying on
thrown error.stdout/error.stderr so tests keep arguments with spaces intact.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
ParallelExecutorclass orchestrates concurrent task execution with configurable limitssandbox runcommand with--multimode and supporting optionsNew CLI Options
--multi--task <text...>--task-file <path>--max-concurrent <n>--fail-fast--output-dir <path>Usage Examples
New Files
src/e2b/parallel-executor.tssrc/utils/concurrency.tstests/e2b/parallel-executor.test.tstests/cli-sandbox-run-multi.test.tsTest plan
--multioptionsSummary by CodeRabbit
Release Notes
New Features
Tests
Chores