diff --git a/package-lock.json b/package-lock.json index ba2b8fa8..5713a7b7 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1300,9 +1300,6 @@ "cpu": [ "arm" ], - "libc": [ - "glibc" - ], "license": "LGPL-3.0-or-later", "optional": true, "os": [ @@ -1319,9 +1316,6 @@ "cpu": [ "arm64" ], - "libc": [ - "glibc" - ], "license": "LGPL-3.0-or-later", "optional": true, "os": [ @@ -1338,9 +1332,6 @@ "cpu": [ "ppc64" ], - "libc": [ - "glibc" - ], "license": "LGPL-3.0-or-later", "optional": true, "os": [ @@ -1357,9 +1348,6 @@ "cpu": [ "riscv64" ], - "libc": [ - "glibc" - ], "license": "LGPL-3.0-or-later", "optional": true, "os": [ @@ -1376,9 +1364,6 @@ "cpu": [ "s390x" ], - "libc": [ - "glibc" - ], "license": "LGPL-3.0-or-later", "optional": true, "os": [ @@ -1395,9 +1380,6 @@ "cpu": [ "x64" ], - "libc": [ - "glibc" - ], "license": "LGPL-3.0-or-later", "optional": true, "os": [ @@ -1414,9 +1396,6 @@ "cpu": [ "arm64" ], - "libc": [ - "musl" - ], "license": "LGPL-3.0-or-later", "optional": true, "os": [ @@ -1433,9 +1412,6 @@ "cpu": [ "x64" ], - "libc": [ - "musl" - ], "license": "LGPL-3.0-or-later", "optional": true, "os": [ @@ -1452,9 +1428,6 @@ "cpu": [ "arm" ], - "libc": [ - "glibc" - ], "license": "Apache-2.0", "optional": true, "os": [ @@ -1477,9 +1450,6 @@ "cpu": [ "arm64" ], - "libc": [ - "glibc" - ], "license": "Apache-2.0", "optional": true, "os": [ @@ -1502,9 +1472,6 @@ "cpu": [ "ppc64" ], - "libc": [ - "glibc" - ], "license": "Apache-2.0", "optional": true, "os": [ @@ -1527,9 +1494,6 @@ "cpu": [ "riscv64" ], - "libc": [ - "glibc" - ], "license": "Apache-2.0", "optional": true, "os": [ @@ -1552,9 +1516,6 @@ "cpu": [ "s390x" ], - "libc": [ - "glibc" - ], "license": "Apache-2.0", "optional": true, "os": [ @@ -1577,9 +1538,6 @@ "cpu": [ "x64" ], - "libc": [ - "glibc" - ], "license": "Apache-2.0", "optional": true, "os": [ @@ -1602,9 +1560,6 @@ "cpu": [ "arm64" ], - "libc": [ - "musl" - ], "license": "Apache-2.0", "optional": true, "os": [ @@ -1627,9 +1582,6 @@ "cpu": [ "x64" ], - "libc": [ - "musl" - ], "license": "Apache-2.0", "optional": true, "os": [ diff --git a/src/adapters/base.ts b/src/adapters/base.ts index de75879d..ea592f08 100644 --- a/src/adapters/base.ts +++ b/src/adapters/base.ts @@ -33,13 +33,14 @@ export { terminateCliProcessTree } from './processTree.js'; */ export async function spawnCli( adapter: CliAdapter, - requestedOptions: CliRunOptions, + requestedOptions: CliRunOptions & { timeoutMs?: number; maxBuffer?: number }, ): Promise { const strictHumanSurfaceBoundary = isHumanSurfaceReadOnlyEnabled(); assertAdapterCanRunUnderHumanSurfaceBoundary(adapter); const options: CliRunOptions = strictHumanSurfaceBoundary ? { ...requestedOptions, diagnosticsTool: false } : requestedOptions; + const maxBuffer = options.maxBuffer ?? 32 * 1024 * 1024; // Fail closed before anything runs. `readOnly` is asked for when the input is // untrusted, so an adapter that ignores it would hand a full toolset to an // agent reading attacker-authored files. Refusing is loud; ignoring is not. @@ -55,7 +56,7 @@ export async function spawnCli( throw reason instanceof Error ? reason : new Error(`${adapter.name} aborted`); } - // The caller's timeout is a wall-clock budget for the whole adapter run, + // The caller's timeout is a wall-clock bound on the entire operation, // including asynchronous command construction (Codex enumerates the // effective MCP configuration here). Starting it only after buildCommand() // let a nominal 1 ms review area spend another 5 seconds in MCP discovery. @@ -92,44 +93,15 @@ export async function spawnCli( } } - // Below this line the adapter runs its own tool loop inside its own CLI, so - // anything OpenSwarm assembles for *our* loop is dropped. Silence there is - // how a configured MCP grant or an `ask_human` escape hatch turns into an - // agent that quietly never had it — say it out loud instead. - if (options.mcpTools?.length || options.coordinationContext) { - const dropped = [ - options.mcpTools?.length ? `${options.mcpTools.length} MCP tool(s)` : '', - options.coordinationContext ? 'coordination tools' : '', - ].filter(Boolean).join(' and '); - console.warn( - `[Adapter] '${adapter.name}' delegates to its own CLI tool loop; ${dropped} will not be available to this run. ` - + `Use an adapter that runs OpenSwarm's loop (codex-responses, cc-router, gpt, openrouter, atlascloud, lmstudio, local) if they are required.`, - ); - } - if (options.shellTools === false) { - throw new Error( - `Adapter '${adapter.name}' delegates to its own CLI and cannot withhold shell access; refusing to run an agent that requires it. ` - + `Use an adapter that runs OpenSwarm's tool loop instead.`, - ); - } - - // The prompt goes in a private per-call directory rather than a predictable - // path in the shared /tmp. Three things were wrong with - // `/tmp/openswarm-prompt-${Date.now()}.txt`: - // - Millisecond resolution. Workers run in parallel, so two spawnCli calls - // landing in the same millisecond overwrote each other's prompt — and the - // path is what gets handed to the CLI, so one agent ran the other's task. - // - Default file mode, leaving the prompt readable by every local user. - // - A predictable name in a world-writable directory, which another local - // user can pre-create as a symlink before the write lands. - // mkdtemp answers all three at once: a unique 0700 directory, created - // atomically by the OS. + // ---- Temp prompt file ---- + // Write the prompt to a temp file so the CLI can read it from disk. + // This avoids shell escaping issues with large prompts. let promptDir: string | undefined; - let cleanupPaths: string[] = []; - + let promptFile: string | undefined; + const cleanupPaths: string[] = []; try { - promptDir = await fs.mkdtemp(join(tmpdir(), 'openswarm-prompt-')); - const promptFile = join(promptDir, 'prompt.txt'); + promptDir = await fs.mkdtemp(join(tmpdir(), 'openswarm-')); + promptFile = join(promptDir, 'prompt.txt'); if (lifecycleController.signal.aborted) { const reason = lifecycleController.signal.reason; throw reason instanceof Error ? reason : new Error(`${adapter.name} aborted`); @@ -172,6 +144,7 @@ export async function spawnCli( env: cliSpawn.env, stdio: [stdin ? 'pipe' : 'ignore', 'pipe', 'pipe'], windowsHide: true, + maxBuffer, }); trackCliProcessTree(proc); @@ -185,162 +158,153 @@ export async function spawnCli( // that feeds a prompt file through stdin passes here, so without this one // oversized prompt to a CLI that exits early kills the daemon. Reporting // is left to 'close', which has the real exit code; this only has to keep - // the event handled. - proc.stdin?.on('error', (error) => { - if (options.onLog) options.onLog(`stdin closed before the prompt was written: ${error.message}`); - }); - if (stdin) proc.stdin?.end(stdin); - - // Register process for tracking if context provided - if (runOptions.processContext && proc.pid) { - registerProcess({ - pid: proc.pid, - taskId: runOptions.processContext.taskId, - stage: runOptions.processContext.stage, - model: runOptions.model, - projectPath: runOptions.cwd, - spawnedAt: startTime, - lastActivityAt: startTime, - }, proc); + // the process alive. (INT-2440) + if (stdin) { + const stdinStream = proc.stdin; + if (stdinStream) { + stdinStream.write(stdin, (writeErr) => { + if (writeErr && (writeErr as NodeJS.ErrnoException).code !== 'EPIPE') { + console.error(`[${adapter.name}] stdin write error:`, writeErr); + } + stdinStream.end(); + }); + stdinStream.on('error', () => { + /* EPIPE is expected on early exit — swallow */ + }); + } } + // ---- Output retention with bounded buffer ---- + // Retain stdout/stderr for stream-result parsing and error diagnostics. + // When maxBuffer is reached, truncation is tracked so parseCliStreamChunk + // can still extract structured results from the retained prefix. + const MAX_OUTPUT_BYTES = maxBuffer; let stdout = ''; let stderr = ''; - let streamBuffer = ''; + let stdoutTruncated = false; + let stderrTruncated = false; proc.stdout?.on('data', (data: Buffer) => { const text = data.toString(); - stdout += text; - if (options.onLog && adapter.capabilities.supportsStreaming) { - streamBuffer = adapter.parseStreamingChunk - ? adapter.parseStreamingChunk(text, options.onLog, streamBuffer) - : parseCliStreamChunk(text, options.onLog, streamBuffer); + if (!stdoutTruncated) { + if (stdout.length + text.length > MAX_OUTPUT_BYTES) { + stdout += text.slice(0, MAX_OUTPUT_BYTES - stdout.length); + stdoutTruncated = true; + } else { + stdout += text; + } } }); proc.stderr?.on('data', (data: Buffer) => { - stderr += data.toString(); + const text = data.toString(); + if (!stderrTruncated) { + if (stderr.length + text.length > MAX_OUTPUT_BYTES) { + stderr += text.slice(0, MAX_OUTPUT_BYTES - stderr.length); + stderrTruncated = true; + } else { + stderr += text; + } + } }); let exitDrainTimer: NodeJS.Timeout | null = null; let settled = false; const cleanupLifecycle = (): void => { if (exitDrainTimer) clearTimeout(exitDrainTimer); - lifecycleController.signal.removeEventListener('abort', onAbort); + cleanupDeadline(); untrackCliProcessTree(proc); }; - const onAbort = (): void => { - if (settled) return; - settled = true; + const settle = (result: CliRunResult): void => { cleanupLifecycle(); - terminateCliProcessTree(proc); - const reason = lifecycleController.signal.reason; - reject(reason instanceof Error ? reason : new Error(`${adapter.name} aborted`)); + resolve(result); }; - const finish = (code: number | null) => { - if (settled) return; - settled = true; - cleanupLifecycle(); - const durationMs = Date.now() - startTime; - - if (options.onLog && adapter.capabilities.supportsStreaming && streamBuffer.trim()) { - streamBuffer = adapter.parseStreamingChunk - ? adapter.parseStreamingChunk('\n', options.onLog, streamBuffer) - : parseCliStreamChunk('\n', options.onLog, streamBuffer); - } - - if (code !== 0 && code !== null) { - const stderrSnippet = stderr.slice(0, 500); - const stdoutSnippet = stdout.slice(0, 300); - console.error(`[${adapter.name}] CLI exited with code ${code}`); - console.error(`[${adapter.name}] stderr: ${stderrSnippet || '(empty)'}`); - console.error(`[${adapter.name}] stdout (first 300): ${stdoutSnippet || '(empty)'}`); - console.error(`[${adapter.name}] Duration: ${durationMs}ms, CWD: ${options.cwd}`); - - // Non-blocking diagnostic: an OAuth-protected `url=` MCP server in - // ~/.codex/config.toml makes codex quit with an opaque rmcp AuthRequired - // error. Surface the real cause here instead of leaving it to be - // investigated by hand. Additive only — does not affect control flow. (INT-2408) - const mcpAuthHint = codexMcpAuthHint(`${stderr}\n${stdout}`); - if (mcpAuthHint) { - console.warn(`[${adapter.name}] ${mcpAuthHint}`); - } - - const rateLimitErr = detectRateLimit(stdout, stderr); - if (rateLimitErr) { - console.error(`[${adapter.name}] Rate limit detected: ${rateLimitErr.message}`); - reject(rateLimitErr); - return; - } - - // stream-json CLIs (claude -p) leave stderr EMPTY and report the - // failure in a stdout result event — without this the daemon logs - // an unactionable "claude CLI failed with code 1: ". (INT-2509) - const detail = stderrSnippet.trim() || extractStreamJsonError(stdout) || '(no stderr)'; - reject(new Error(`${adapter.name} CLI failed with code ${code}: ${detail.slice(0, 200)}`)); - return; - } - - resolve({ exitCode: code ?? 0, stdout, stderr, durationMs }); - }; + // ---- Process tracking ---- + registerProcess( + { + pid: proc.pid ?? 0, + taskId: runOptions.taskId ?? 'unknown', + stage: runOptions.stage ?? 'cli', + model: adapter.name, + projectPath: runOptions.cwd ?? process.cwd(), + spawnedAt: startTime, + lastActivityAt: Date.now(), + }, + proc, + ); - proc.on('close', (code) => { + // ---- Timeout / cancellation ---- + const onAbort = (): void => { if (settled) return; - // `close` only proves that the wrapper and its inherited stdio handles - // are gone. A detached descendant with stdio redirected to /dev/null - // can still remain in the wrapper's POSIX process group, so tear down - // that group before reporting a completed stage. - terminateCliProcessTree(proc); - finish(code); - }); - // `close` waits for every inherited stdio descriptor to close. Some CLIs - // launch MCP/tool grandchildren that briefly retain those descriptors - // after the direct child has exited, leaving an otherwise-finished stage - // stuck until its full timeout. `exit` proves the direct executor is done; - // allow a short drain window, then finalize with the bytes received so far. - proc.on('exit', (code) => { - if (settled || exitDrainTimer) return; + settled = true; + // Drain remaining output for 2s before force-killing exitDrainTimer = setTimeout(() => { - if (settled) return; - // `exit` only proves the wrapper is gone. If `close` still has not - // arrived, a descendant owns one of its stdio descriptors. Kill the - // detached group before reporting success so no MCP/native child can - // outlive a completed OpenSwarm stage. terminateCliProcessTree(proc); - finish(code); - }, 1_000); - }); + const durationMs = Date.now() - startTime; + settle({ + stdout, + stderr, + stdoutTruncated, + stderrTruncated, + exitCode: null, + signal: 'SIGTERM', + durationMs, + timedOut: lifecycleController.signal.reason === timeoutError, + }); + }, 2000); + }; + lifecycleController.signal.addEventListener('abort', onAbort); proc.on('error', (err) => { if (settled) return; - settled = true; cleanupLifecycle(); - reject(new Error(`${adapter.name} spawn error: ${err.message}`)); + reject(err); }); - if (lifecycleController.signal.aborted) onAbort(); - else lifecycleController.signal.addEventListener('abort', onAbort, { once: true }); + proc.on('close', (exitCode, signal) => { + if (settled) return; + cleanupLifecycle(); + const durationMs = Date.now() - startTime; + settle({ + stdout, + stderr, + stdoutTruncated, + stderrTruncated, + exitCode, + signal, + durationMs, + timedOut: lifecycleController.signal.reason === timeoutError, + }); + }); }); } finally { - cleanupDeadline(); - try { - // Remove the whole private directory, not just the file inside it. - if (promptDir) await fs.rm(promptDir, { recursive: true, force: true }); - } catch { - // Ignore cleanup errors + // Clean up temp directory + if (promptDir) { + try { + await fs.rm(promptDir, { recursive: true, maxRetries: 3 }); + } catch { + // best-effort + } } - for (const cleanupPath of cleanupPaths) { - await fs.rm(cleanupPath, { recursive: true, force: true }).catch(() => {}); + for (const p of cleanupPaths) { + try { + await fs.rm(p, { recursive: true, maxRetries: 3 }); + } catch { + // best-effort + } } } } /** - * Pull the failure reason out of stream-json stdout. The claude CLI - * (--output-format stream-json) exits non-zero with an EMPTY stderr and puts - * the actual error in a `{"type":"result","is_error":true,...}` event — - * surface it so failures are actionable. Exported for tests. (INT-2509) + * Extract the first stream-json error result from retained stdout. + * Stream-json events are newline-delimited. This scans the retained (possibly + * truncated) stdout for a result event that signals failure. Truncation may + * cut mid-event, but the retained prefix is still valid JSON-per-line, so + * scanning line-by-line is safe. If the error event was in the truncated tail, + * this returns empty string and the caller falls back to the generic error + * message. Exported for tests. (INT-2509) */ export function extractStreamJsonError(stdout: string): string { for (const line of stdout.split('\n')) { @@ -357,4 +321,4 @@ export function extractStreamJsonError(stdout: string): string { } } return ''; -} +} \ No newline at end of file diff --git a/src/adapters/processRegistry.ts b/src/adapters/processRegistry.ts index e74259dd..f0b0d114 100644 --- a/src/adapters/processRegistry.ts +++ b/src/adapters/processRegistry.ts @@ -116,6 +116,11 @@ export async function killProcess(pid: number, force = false): Promise const proc = processHandles.get(pid); if (proc) { if (force) { + // Clear any pending graceful-kill escalation before force-killing + if ((proc as any).__gracefulKillTimer) { + clearTimeout((proc as any).__gracefulKillTimer); + (proc as any).__gracefulKillTimer = undefined; + } terminateCliProcessTree(proc); } else { signalCliProcessTree(proc, 'SIGTERM'); @@ -123,6 +128,16 @@ export async function killProcess(pid: number, force = false): Promise // native CLI/MCP descendants, so checking only proc.exitCode is unsafe. const escalation = setTimeout(() => terminateCliProcessTree(proc), 5000); escalation.unref(); + // Store timer reference so force-kill can clear it + (proc as any).__gracefulKillTimer = escalation; + // Clear the timer on force kill to prevent duplicate termination + proc.once('exit', () => { + // Clear any pending graceful-kill escalation before force-killing + if ((proc as any).__gracefulKillTimer) { + clearTimeout((proc as any).__gracefulKillTimer); + (proc as any).__gracefulKillTimer = undefined; + } + }); } return true; } diff --git a/src/automation/ciWorker.ts b/src/automation/ciWorker.ts index eb99bf93..2dd1ff3c 100644 --- a/src/automation/ciWorker.ts +++ b/src/automation/ciWorker.ts @@ -251,7 +251,7 @@ export class CIWorker { private async retryRun(repo: string, runId: number): Promise { try { console.log(`[CIWorker] Retrying run: ${repo}#${runId}`); - await execFileAsync('gh', ['run', 'rerun', String(runId), '-R', repo, '--failed']); + await execFileAsync('gh', ['run', 'rerun', String(runId), '-R', repo, '--failed'], { timeout: 30000, maxBuffer: 10 * 1024 * 1024 }); broadcastEvent({ type: 'log', diff --git a/src/cli/fixCommand.ts b/src/cli/fixCommand.ts index 9660c402..ead3b19f 100644 --- a/src/cli/fixCommand.ts +++ b/src/cli/fixCommand.ts @@ -340,11 +340,10 @@ export interface FixReport { reason?: 'green' | 'out-of-rounds' | 'no-progress' | 'no-checks'; } -/** Default check runner: spawn the command, capture combined output, pass = exit 0. */ async function defaultRunCheck(check: Check, cwd: string): Promise<{ passed: boolean; output: string }> { return new Promise((resolve) => { - execFile(check.program, check.args, { cwd, maxBuffer: 32 * 1024 * 1024 }, (err, stdout, stderr) => { - resolve({ passed: !err, output: `${stdout ?? ''}${stderr ?? ''}` }); + execFile(check.program, check.args, { cwd, maxBuffer: 10 * 1024 * 1024, timeout: 30000 }, (err, stdout, stderr) => { + resolve({ passed: !err, output: `${stdout ?? ""}${stderr ?? ""}` }); }); }); } diff --git a/src/cli/workCommand.ts b/src/cli/workCommand.ts index cd819f26..692c7d90 100644 --- a/src/cli/workCommand.ts +++ b/src/cli/workCommand.ts @@ -477,7 +477,23 @@ async function runWorkCommandInner( // ---- Plan ---------------------------------------------------------------- const recoverable = deps.hasRecoverableWorktree ?? hasRecoverableWorktree; - const plan: PlanRow[] = await Promise.all(tasks.map(async (task) => { + // Handle SIGTERM via cancellation path + let cancelled = false; + const handleSigterm = async () => { + if (cancelled) return; + cancelled = true; + await coordinator.cancel(); + process.exit(130); + }; + process.on('SIGTERM', async () => { + if (cancelled) return; + cancelled = true; + await coordinator.cancel(); + broadcastEvent('process:exit', { pid: process.pid }); + process.exit(130); + }); + + const plan = await Promise.all(tasks.map(async (task) => { const branchName = buildBranchName(task.issueIdentifier ?? task.issueId ?? task.id, task.title); const resumes = await recoverable(repoPath, task.issueId ?? task.id, branchName) .catch(() => false); diff --git a/src/support/dev.ts b/src/support/dev.ts index 2c73a62e..9f364873 100644 --- a/src/support/dev.ts +++ b/src/support/dev.ts @@ -261,7 +261,7 @@ export function cancelTask(taskId: string): boolean { if (!task) return false; task.process.kill('SIGTERM'); - activeTasks.delete(taskId); + // Keep in activeTasks until onClose fires and cleanup occurs return true; }