diff --git a/bin/multiagent-safety.js b/bin/multiagent-safety.js index d66db9b..419b8c2 100755 --- a/bin/multiagent-safety.js +++ b/bin/multiagent-safety.js @@ -3967,37 +3967,60 @@ function agents(rawArgs) { return; } - if (reviewRunning) { - stopAgentProcessByPid(existingReviewPid, 'review-bot-watch.sh'); - } - if (cleanupRunning) { - stopAgentProcessByPid(existingCleanupPid, `${path.basename(__filename)} cleanup`); - } - const reviewLogPath = path.join(repoRoot, '.omx', 'logs', 'agent-review.log'); const cleanupLogPath = path.join(repoRoot, '.omx', 'logs', 'agent-cleanup.log'); - const reviewPid = spawnDetachedAgentProcess({ - command: 'bash', - args: [reviewScriptPath, '--interval', String(options.reviewIntervalSeconds)], - cwd: repoRoot, - logPath: reviewLogPath, - }); - const cleanupPid = spawnDetachedAgentProcess({ - command: process.execPath, - args: [ - path.resolve(__filename), - 'cleanup', - '--target', - repoRoot, - '--watch', - '--interval', - String(options.cleanupIntervalSeconds), - '--idle-minutes', - String(options.idleMinutes), - ], - cwd: repoRoot, - logPath: cleanupLogPath, - }); + + let reviewPid = existingReviewPid; + let cleanupPid = existingCleanupPid; + let startedAny = false; + let reusedAny = false; + + if (!reviewRunning) { + reviewPid = spawnDetachedAgentProcess({ + command: 'bash', + args: [reviewScriptPath, '--interval', String(options.reviewIntervalSeconds)], + cwd: repoRoot, + logPath: reviewLogPath, + }); + startedAny = true; + } else { + reusedAny = true; + } + + if (!cleanupRunning) { + cleanupPid = spawnDetachedAgentProcess({ + command: process.execPath, + args: [ + path.resolve(__filename), + 'cleanup', + '--target', + repoRoot, + '--watch', + '--interval', + String(options.cleanupIntervalSeconds), + '--idle-minutes', + String(options.idleMinutes), + ], + cwd: repoRoot, + logPath: cleanupLogPath, + }); + startedAny = true; + } else { + reusedAny = true; + } + + const priorReviewInterval = Number.parseInt(String(existingState?.review?.intervalSeconds || ''), 10); + const priorCleanupInterval = Number.parseInt(String(existingState?.cleanup?.intervalSeconds || ''), 10); + const priorIdleMinutes = Number.parseInt(String(existingState?.cleanup?.idleMinutes || ''), 10); + const reviewIntervalSeconds = reviewRunning && Number.isInteger(priorReviewInterval) && priorReviewInterval >= 5 + ? priorReviewInterval + : options.reviewIntervalSeconds; + const cleanupIntervalSeconds = cleanupRunning && Number.isInteger(priorCleanupInterval) && priorCleanupInterval >= 5 + ? priorCleanupInterval + : options.cleanupIntervalSeconds; + const idleMinutes = cleanupRunning && Number.isInteger(priorIdleMinutes) && priorIdleMinutes >= 1 + ? priorIdleMinutes + : options.idleMinutes; writeAgentsState(repoRoot, { schemaVersion: 1, @@ -4005,14 +4028,14 @@ function agents(rawArgs) { startedAt: new Date().toISOString(), review: { pid: reviewPid, - intervalSeconds: options.reviewIntervalSeconds, + intervalSeconds: reviewIntervalSeconds, script: reviewScriptPath, logPath: reviewLogPath, }, cleanup: { pid: cleanupPid, - intervalSeconds: options.cleanupIntervalSeconds, - idleMinutes: options.idleMinutes, + intervalSeconds: cleanupIntervalSeconds, + idleMinutes, script: path.resolve(__filename), logPath: cleanupLogPath, }, @@ -4021,6 +4044,9 @@ function agents(rawArgs) { console.log( `[${TOOL_NAME}] Started repo agents in ${repoRoot} (review pid=${reviewPid}, cleanup pid=${cleanupPid}).`, ); + if (reusedAny && startedAny) { + console.log(`[${TOOL_NAME}] Reused healthy bot process(es) and started only missing ones.`); + } console.log(`[${TOOL_NAME}] Logs: ${reviewLogPath}, ${cleanupLogPath}`); process.exitCode = 0; return; diff --git a/test/install.test.js b/test/install.test.js index fdfd880..fb3b342 100644 --- a/test/install.test.js +++ b/test/install.test.js @@ -1386,6 +1386,68 @@ test('agents command starts review+cleanup bots for the target repo and stops th assert.equal(fs.existsSync(statePath), false, 'agents stop should remove state file'); }); +test('agents start reuses running review bot when only cleanup bot is missing', () => { + const repoDir = initRepo(); + seedCommit(repoDir); + const scriptsDir = path.join(repoDir, 'scripts'); + fs.mkdirSync(scriptsDir, { recursive: true }); + + const reviewScriptPath = path.join(scriptsDir, 'review-bot-watch.sh'); + fs.writeFileSync( + reviewScriptPath, + '#!/usr/bin/env bash\n' + + 'set -euo pipefail\n' + + 'while true; do sleep 60; done\n', + 'utf8', + ); + fs.chmodSync(reviewScriptPath, 0o755); + + const pruneScriptPath = path.join(scriptsDir, 'agent-worktree-prune.sh'); + fs.writeFileSync( + pruneScriptPath, + '#!/usr/bin/env bash\n' + + 'set -euo pipefail\n' + + 'exit 0\n', + 'utf8', + ); + fs.chmodSync(pruneScriptPath, 0o755); + + let result = runNode( + ['agents', 'start', '--target', repoDir, '--review-interval', '31', '--cleanup-interval', '47', '--idle-minutes', '12'], + repoDir, + ); + assert.equal(result.status, 0, result.stderr || result.stdout); + + const statePath = path.join(repoDir, '.omx', 'state', 'agents-bots.json'); + const firstState = JSON.parse(fs.readFileSync(statePath, 'utf8')); + const firstReviewPid = firstState.review.pid; + const firstCleanupPid = firstState.cleanup.pid; + assert.equal(isPidAlive(firstReviewPid), true, 'review bot should be alive after initial start'); + assert.equal(isPidAlive(firstCleanupPid), true, 'cleanup bot should be alive after initial start'); + + process.kill(firstCleanupPid, 'SIGTERM'); + assert.equal(waitForPidExit(firstCleanupPid), true, 'cleanup bot should stop during simulation'); + assert.equal(isPidAlive(firstReviewPid), true, 'review bot should remain alive before restart'); + + result = runNode( + ['agents', 'start', '--target', repoDir, '--review-interval', '30', '--cleanup-interval', '60', '--idle-minutes', '60'], + repoDir, + ); + assert.equal(result.status, 0, result.stderr || result.stdout); + assert.match(result.stdout, /Reused healthy bot process\(es\) and started only missing ones\./); + + const secondState = JSON.parse(fs.readFileSync(statePath, 'utf8')); + assert.equal(secondState.review.pid, firstReviewPid, 'running review bot should be reused'); + assert.notEqual(secondState.cleanup.pid, firstCleanupPid, 'missing cleanup bot should be restarted'); + assert.equal(isPidAlive(secondState.review.pid), true, 'reused review bot should stay alive'); + assert.equal(isPidAlive(secondState.cleanup.pid), true, 'new cleanup bot should be alive'); + + result = runNode(['agents', 'stop', '--target', repoDir], repoDir); + assert.equal(result.status, 0, result.stderr || result.stdout); + assert.equal(waitForPidExit(secondState.review.pid), true, 'review bot pid should exit after stop'); + assert.equal(waitForPidExit(secondState.cleanup.pid), true, 'cleanup bot pid should exit after stop'); +}); + test('agents cleanup bot defaults to a 60-minute idle threshold', () => { const repoDir = initRepo(); seedCommit(repoDir);