diff --git a/.aiox-core/core/execution/build-orchestrator.js b/.aiox-core/core/execution/build-orchestrator.js index e8d7b52fe6..abcb36f242 100644 --- a/.aiox-core/core/execution/build-orchestrator.js +++ b/.aiox-core/core/execution/build-orchestrator.js @@ -501,9 +501,18 @@ The subtask is complete only when verification passes. } /** - * Run Claude CLI with prompt + * Run Claude CLI with prompt delivered via stdin. + * + * @param {string} prompt - The prompt to execute + * @param {string} workDir - Working directory for execution + * @param {Object} config - Orchestrator configuration + * @returns {Promise} - Execution result with stdout and exit code */ async runClaudeCLI(prompt, workDir, config) { + if (!prompt || typeof prompt !== 'string') { + throw new Error('runClaudeCLI requires a non-empty string prompt'); + } + return new Promise((resolve, reject) => { const args = [ '--print', // Non-interactive mode @@ -514,21 +523,46 @@ The subtask is complete only when verification passes. args.push('--model', config.claudeModel); } - // Escape prompt for shell - const escapedPrompt = prompt.replace(/'/g, "'\\''"); - - const fullCommand = `echo '${escapedPrompt}' | claude ${args.join(' ')}`; - this.log(`Running Claude CLI in ${workDir}`, 'debug'); - const child = spawn('sh', ['-c', fullCommand], { + const child = spawn('claude', args, { cwd: workDir, env: { ...process.env }, timeout: config.subtaskTimeout, + stdio: ['pipe', 'pipe', 'pipe'], }); let stdout = ''; let stderr = ''; + let stdinError = null; + let hasError = false; + + child.on('error', (error) => { + hasError = true; + reject(error); + }); + + // Prevent unhandled stream errors if the pipe breaks or process exits early + child.stdin.on('error', (err) => { + stdinError = err; + this.log(`Claude stdin stream error: ${err.message}`, 'debug'); + }); + + // Write prompt via stdin to avoid shell-related issues and command injection + if (child.stdin.writable) { + child.stdin.write(prompt); + child.stdin.end(); + } else { + // If stdin is not writable, the process might have failed to start or exited immediately + // We wait a bit for 'error' event to fire if it's a spawn error (like ENOENT) + setImmediate(() => { + if (!hasError) { + if (child.kill) child.kill(); + reject(new Error('Claude stdin was not writable immediately after spawn')); + } + }); + return; + } child.stdout.on('data', (data) => { stdout += data.toString(); @@ -544,17 +578,19 @@ The subtask is complete only when verification passes. } }); - child.on('close', (code) => { - if (code === 0) { + child.on('close', (code, signal) => { + if (hasError) return; // Already rejected in 'error' handler + + if (stdinError) { + reject(new Error(`Claude CLI stdin write failed (prompt not delivered): ${stdinError.message}`)); + } else if (code === 0) { resolve({ stdout, stderr, code }); + } else if (signal) { + reject(new Error(`Claude CLI killed by signal ${signal} (timeout or external kill): ${stderr}`)); } else { reject(new Error(`Claude CLI exited with code ${code}: ${stderr}`)); } }); - - child.on('error', (error) => { - reject(error); - }); }); } diff --git a/.aiox-core/core/execution/subagent-dispatcher.js b/.aiox-core/core/execution/subagent-dispatcher.js index dc5bc26780..9ad6664747 100644 --- a/.aiox-core/core/execution/subagent-dispatcher.js +++ b/.aiox-core/core/execution/subagent-dispatcher.js @@ -94,6 +94,9 @@ class SubagentDispatcher extends EventEmitter { this.maxRetries = config.maxRetries || 2; this.retryDelay = config.retryDelay || 2000; + // Timeout configuration + this.claudeTimeout = config.claudeTimeout || 10 * 60 * 1000; // 10 minutes + // Dependencies this.memoryQuery = config.memoryQuery || (MemoryQuery ? new MemoryQuery() : null); this.gotchasMemory = config.gotchasMemory || (GotchasMemory ? new GotchasMemory() : null); @@ -642,24 +645,58 @@ class SubagentDispatcher extends EventEmitter { } /** - * Execute prompt via Claude CLI - * @param {string} prompt - Prompt to execute - * @returns {Promise} - Execution result + * Execute prompt via Claude CLI using stdin for robust handling. + * This approach avoids shell-related parsing issues and command injection. + * + * @param {string} prompt - The prompt content to send to Claude + * @returns {Promise} - Execution result with success flag and output */ executeClaude(prompt) { + if (!prompt || typeof prompt !== 'string') { + return Promise.reject(new Error('executeClaude requires a non-empty string prompt')); + } + return new Promise((resolve, reject) => { const args = ['--print', '--dangerously-skip-permissions']; - const escapedPrompt = prompt.replace(/'/g, "'\\''"); - const fullCommand = `echo '${escapedPrompt}' | claude ${args.join(' ')}`; - const child = spawn('sh', ['-c', fullCommand], { + const child = spawn('claude', args, { cwd: this.rootPath, env: { ...process.env }, stdio: ['pipe', 'pipe', 'pipe'], + timeout: this.claudeTimeout, }); let stdout = ''; let stderr = ''; + let stdinError = null; + let hasError = false; + + child.on('error', (error) => { + hasError = true; + reject(error); + }); + + // Prevent unhandled stream errors if the pipe breaks or process exits early + child.stdin.on('error', (err) => { + stdinError = err; + this.log('stdin_write_error', { error: err.message, code: err.code }); + }); + + // Write prompt via stdin to avoid shell-related issues and command injection + if (child.stdin.writable) { + child.stdin.write(prompt); + child.stdin.end(); + } else { + // If stdin is not writable, the process might have failed to start or exited immediately + // We wait a bit for 'error' event to fire if it's a spawn error (like ENOENT) + setImmediate(() => { + if (!hasError) { + if (child.kill) child.kill(); + reject(new Error('Claude stdin was not writable immediately after spawn')); + } + }); + return; + } child.stdout.on('data', (data) => { stdout += data.toString(); @@ -669,21 +706,23 @@ class SubagentDispatcher extends EventEmitter { stderr += data.toString(); }); - child.on('close', (code) => { - if (code === 0) { + child.on('close', (code, signal) => { + if (hasError) return; // Already rejected in 'error' handler + + if (stdinError) { + reject(new Error(`Claude CLI stdin write failed (prompt not delivered): ${stdinError.message}`)); + } else if (code === 0) { resolve({ success: true, output: stdout, filesModified: this.extractModifiedFiles(stdout), }); + } else if (signal) { + reject(new Error(`Claude CLI killed by signal ${signal} (timeout or external kill): ${stderr || stdout}`)); } else { reject(new Error(`Claude CLI exited with code ${code}: ${stderr || stdout}`)); } }); - - child.on('error', (error) => { - reject(error); - }); }); } diff --git a/.aiox-core/install-manifest.yaml b/.aiox-core/install-manifest.yaml index 05816fa4fd..1b59b50ded 100644 --- a/.aiox-core/install-manifest.yaml +++ b/.aiox-core/install-manifest.yaml @@ -8,7 +8,7 @@ # - File types for categorization # version: 5.0.3 -generated_at: "2026-03-11T15:04:09.395Z" +generated_at: "2026-04-18T04:12:10.295Z" generator: scripts/generate-install-manifest.js file_count: 1090 files: @@ -401,9 +401,9 @@ files: type: core size: 34050 - path: core/execution/build-orchestrator.js - hash: sha256:3698635011a845dfc43c46dfd7f15c0aa3ab488938ea3bd281de29157f5c4687 + hash: sha256:24878fba391fe2ac055695fbc7fbc6cdc62fcc200b6e8e865e72757c0cbf03c4 type: core - size: 31780 + size: 33328 - path: core/execution/build-state-manager.js hash: sha256:415fca3ad3d750db1f41f14f33971cf661ee9d6073a570175d695bb3c1be655c type: core @@ -433,9 +433,9 @@ files: type: core size: 51556 - path: core/execution/subagent-dispatcher.js - hash: sha256:7affbc04de9be2bc53427670009a885f0b35e1cc183f82c2e044abf9611344b6 + hash: sha256:e7c803cab96b837cba5e8ca16790e4a9fd6c0b1137ae492dd8ab5cfe03bc4b9e type: core - size: 25738 + size: 27350 - path: core/execution/wave-executor.js hash: sha256:4e2324edb37ae0729062b5ac029f2891e050e7efd3a48d0f4a1dc4f227a6716b type: core diff --git a/package-lock.json b/package-lock.json index 19110122d5..2ffa428d24 100644 --- a/package-lock.json +++ b/package-lock.json @@ -9301,25 +9301,6 @@ "node": "^20.17.0 || >=22.9.0" } }, - "node_modules/npm/node_modules/@isaacs/balanced-match": { - "version": "4.0.1", - "dev": true, - "license": "MIT", - "engines": { - "node": "20 || >=22" - } - }, - "node_modules/npm/node_modules/@isaacs/brace-expansion": { - "version": "5.0.1", - "dev": true, - "license": "MIT", - "dependencies": { - "@isaacs/balanced-match": "^4.0.1" - }, - "engines": { - "node": "20 || >=22" - } - }, "node_modules/npm/node_modules/@isaacs/fs-minipass": { "version": "4.0.1", "dev": true, @@ -9881,15 +9862,6 @@ "node": ">=0.3.1" } }, - "node_modules/npm/node_modules/encoding": { - "version": "0.1.13", - "dev": true, - "license": "MIT", - "optional": true, - "dependencies": { - "iconv-lite": "^0.6.2" - } - }, "node_modules/npm/node_modules/env-paths": { "version": "2.2.1", "dev": true, @@ -9899,11 +9871,6 @@ "node": ">=6" } }, - "node_modules/npm/node_modules/err-code": { - "version": "2.0.3", - "dev": true, - "license": "MIT" - }, "node_modules/npm/node_modules/exponential-backoff": { "version": "3.1.3", "dev": true, @@ -10027,14 +9994,6 @@ "node": "^20.17.0 || >=22.9.0" } }, - "node_modules/npm/node_modules/imurmurhash": { - "version": "0.1.4", - "dev": true, - "license": "MIT", - "engines": { - "node": ">=0.8.19" - } - }, "node_modules/npm/node_modules/ini": { "version": "6.0.0", "dev": true, @@ -10765,18 +10724,6 @@ "url": "https://github.com/sponsors/isaacs" } }, - "node_modules/npm/node_modules/promise-retry": { - "version": "2.0.1", - "dev": true, - "license": "MIT", - "dependencies": { - "err-code": "^2.0.2", - "retry": "^0.12.0" - }, - "engines": { - "node": ">=10" - } - }, "node_modules/npm/node_modules/promzard": { "version": "3.0.1", "dev": true, @@ -10818,14 +10765,6 @@ "node": "^20.17.0 || >=22.9.0" } }, - "node_modules/npm/node_modules/retry": { - "version": "0.12.0", - "dev": true, - "license": "MIT", - "engines": { - "node": ">= 4" - } - }, "node_modules/npm/node_modules/safer-buffer": { "version": "2.1.2", "dev": true, @@ -10912,24 +10851,6 @@ "node": ">= 14" } }, - "node_modules/npm/node_modules/spdx-correct": { - "version": "3.2.0", - "dev": true, - "license": "Apache-2.0", - "dependencies": { - "spdx-expression-parse": "^3.0.0", - "spdx-license-ids": "^3.0.0" - } - }, - "node_modules/npm/node_modules/spdx-correct/node_modules/spdx-expression-parse": { - "version": "3.0.1", - "dev": true, - "license": "MIT", - "dependencies": { - "spdx-exceptions": "^2.1.0", - "spdx-license-ids": "^3.0.0" - } - }, "node_modules/npm/node_modules/spdx-exceptions": { "version": "2.5.0", "dev": true, @@ -11072,52 +10993,12 @@ "node": "^20.17.0 || >=22.9.0" } }, - "node_modules/npm/node_modules/unique-filename": { - "version": "5.0.0", - "dev": true, - "license": "ISC", - "dependencies": { - "unique-slug": "^6.0.0" - }, - "engines": { - "node": "^20.17.0 || >=22.9.0" - } - }, - "node_modules/npm/node_modules/unique-slug": { - "version": "6.0.0", - "dev": true, - "license": "ISC", - "dependencies": { - "imurmurhash": "^0.1.4" - }, - "engines": { - "node": "^20.17.0 || >=22.9.0" - } - }, "node_modules/npm/node_modules/util-deprecate": { "version": "1.0.2", "dev": true, "inBundle": true, "license": "MIT" }, - "node_modules/npm/node_modules/validate-npm-package-license": { - "version": "3.0.4", - "dev": true, - "license": "Apache-2.0", - "dependencies": { - "spdx-correct": "^3.0.0", - "spdx-expression-parse": "^3.0.0" - } - }, - "node_modules/npm/node_modules/validate-npm-package-license/node_modules/spdx-expression-parse": { - "version": "3.0.1", - "dev": true, - "license": "MIT", - "dependencies": { - "spdx-exceptions": "^2.1.0", - "spdx-license-ids": "^3.0.0" - } - }, "node_modules/npm/node_modules/validate-npm-package-name": { "version": "7.0.2", "dev": true, diff --git a/tests/core/claude-execution-robustness.test.js b/tests/core/claude-execution-robustness.test.js new file mode 100644 index 0000000000..838cbe1f2b --- /dev/null +++ b/tests/core/claude-execution-robustness.test.js @@ -0,0 +1,125 @@ +/** + * Claude Execution Robustness - Test Suite + * Verifies that prompts are handled safely via stdin instead of shell pipes. + * Covers success scenarios, error handling, and input validation. + */ + +const { SubagentDispatcher } = require('../../.aiox-core/core/execution/subagent-dispatcher'); +const { BuildOrchestrator } = require('../../.aiox-core/core/execution/build-orchestrator'); +const { mockChildProcess, createMockMemoryQuery, createMockGotchasMemory } = require('./execution-test-helpers'); +const child_process = require('child_process'); + +// Mock child_process.spawn +jest.mock('child_process', () => ({ + spawn: jest.fn() +})); + +describe('Claude Execution Robustness', () => { + let mockSpawn; + let mockMQ, mockGM; + + beforeEach(() => { + jest.clearAllMocks(); + mockSpawn = mockChildProcess('simulated output', '', 0); + // Add stdin mock to the process + mockSpawn.process.stdin = { + write: jest.fn(), + end: jest.fn(), + on: jest.fn(), + writable: true + }; + child_process.spawn.mockReturnValue(mockSpawn.process); + + // Create mock dependencies to pass to constructor + mockMQ = createMockMemoryQuery(); + mockGM = createMockGotchasMemory(); + }); + + describe('SubagentDispatcher.executeClaude', () => { + let sd; + beforeEach(() => { + sd = new SubagentDispatcher({ + rootPath: '/tmp', + memoryQuery: mockMQ, + gotchasMemory: mockGM + }); + }); + + it('should pass the prompt via stdin instead of shell pipes', async () => { + const prompt = "Test prompt with 'quotes' and | pipes"; + const promise = sd.executeClaude(prompt); + mockSpawn.emitData(); + const result = await promise; + + expect(child_process.spawn).toHaveBeenCalledWith( + 'claude', + ['--print', '--dangerously-skip-permissions'], + expect.objectContaining({ cwd: '/tmp' }) + ); + expect(mockSpawn.process.stdin.write).toHaveBeenCalledWith(prompt); + expect(result.success).toBe(true); + }); + + it('should reject if prompt is invalid', async () => { + await expect(sd.executeClaude(null)).rejects.toThrow('non-empty string prompt'); + await expect(sd.executeClaude('')).rejects.toThrow('non-empty string prompt'); + await expect(sd.executeClaude(123)).rejects.toThrow('non-empty string prompt'); + }); + + it('should handle non-zero exit codes', async () => { + mockSpawn = mockChildProcess('', 'command error', 1); + mockSpawn.process.stdin = { write: jest.fn(), end: jest.fn(), on: jest.fn(), writable: true }; + child_process.spawn.mockReturnValue(mockSpawn.process); + + const promise = sd.executeClaude("test"); + mockSpawn.emitData(); + await expect(promise).rejects.toThrow('Claude CLI exited with code 1'); + }); + }); + + describe('BuildOrchestrator.runClaudeCLI', () => { + let bo; + beforeEach(() => { + bo = new BuildOrchestrator({ rootPath: '/tmp' }); + }); + + it('should pass the prompt via stdin and handle model override', async () => { + const prompt = "Build this component"; + const config = { claudeModel: 'claude-3-opus', subtaskTimeout: 5000 }; + const promise = bo.runClaudeCLI(prompt, '/tmp/work', config); + + mockSpawn.emitData(); + const result = await promise; + + expect(child_process.spawn).toHaveBeenCalledWith( + 'claude', + ['--print', '--dangerously-skip-permissions', '--model', 'claude-3-opus'], + expect.objectContaining({ cwd: '/tmp/work' }) + ); + expect(mockSpawn.process.stdin.write).toHaveBeenCalledWith(prompt); + expect(result.stdout).toBe('simulated output'); + }); + + it('should throw if prompt is invalid', async () => { + await expect(bo.runClaudeCLI(null, '/tmp')).rejects.toThrow('non-empty string prompt'); + }); + + it('should handle spawn errors (e.g. command not found)', async () => { + const EventEmitter = require('events'); + const errProcess = new EventEmitter(); + // Important: provide a dummy handler for stdout/stderr to avoid errors when the code tries to access them + errProcess.stdout = new EventEmitter(); + errProcess.stderr = new EventEmitter(); + errProcess.stdin = { on: jest.fn(), write: jest.fn(), end: jest.fn() }; + + child_process.spawn.mockReturnValue(errProcess); + + const promise = bo.runClaudeCLI("test", "/tmp", {}); + + // Emit error after the handler is attached in runClaudeCLI + process.nextTick(() => errProcess.emit('error', new Error('spawn ENOENT'))); + + await expect(promise).rejects.toThrow('spawn ENOENT'); + }); + }); +});