From bc5362daacc44657d7e0370b3bca3a79714b9d0c Mon Sep 17 00:00:00 2001 From: NagyVikt Date: Wed, 29 Apr 2026 22:30:55 +0200 Subject: [PATCH] Prune stale agent sessions from status surfaces Agent session metadata can outlive its branch, worktree, or terminal lifecycle state, so cockpit and gx agents status accumulate dead rows. Add a metadata-only cleanup command that identifies clear stale criteria and removes only session JSON records. Constraint: Cleanup must never delete branches or worktrees. Rejected: Fold cleanup into gx agents status | status must stay read-only and predictable. Confidence: high Scope-risk: narrow Directive: Keep cleanup-sessions metadata-only unless a future task explicitly widens branch/worktree cleanup semantics. Tested: node --test test/agents-cleanup-sessions.test.js test/agents-status.test.js test/cli-args-dispatch.test.js Tested: openspec validate --specs Not-tested: npm test baseline remains known red outside this slice. --- src/agents/cleanup-sessions.js | 126 +++++++++++++++++++++++ src/cli/args.js | 32 ++++-- src/cli/main.js | 7 ++ test/agents-cleanup-sessions.test.js | 143 +++++++++++++++++++++++++++ test/cli-args-dispatch.test.js | 5 + 5 files changed, 307 insertions(+), 6 deletions(-) create mode 100644 src/agents/cleanup-sessions.js create mode 100644 test/agents-cleanup-sessions.test.js diff --git a/src/agents/cleanup-sessions.js b/src/agents/cleanup-sessions.js new file mode 100644 index 00000000..8d2793d0 --- /dev/null +++ b/src/agents/cleanup-sessions.js @@ -0,0 +1,126 @@ +const fs = require('node:fs'); + +const { + listAgentSessions, + removeAgentSession, +} = require('./sessions'); +const { branchExists: defaultBranchExists } = require('../git'); +const { TOOL_NAME } = require('../context'); + +const DEFAULT_STALE_AGE_MINUTES = 24 * 60; +const TERMINAL_STATUSES = new Set(['finished', 'pr-opened', 'failed']); + +function parseTimestamp(value) { + if (!value) return null; + const timestamp = Date.parse(value); + return Number.isFinite(timestamp) ? timestamp : null; +} + +function sessionAgeMinutes(session, nowMs) { + const timestamp = parseTimestamp(session.updatedAt) ?? parseTimestamp(session.createdAt); + if (timestamp === null) return null; + return Math.max(0, Math.floor((nowMs - timestamp) / 60000)); +} + +function evaluateSession(session, repoRoot, options) { + const reasons = []; + const worktreePath = session.worktreePath || ''; + const branch = session.branch || ''; + + if (worktreePath && !options.existsSync(worktreePath)) { + reasons.push('missing-worktree'); + } + + if (branch && !options.branchExists(repoRoot, branch)) { + reasons.push('missing-branch'); + } + + const status = session.status || ''; + const ageMinutes = sessionAgeMinutes(session, options.nowMs); + if ( + TERMINAL_STATUSES.has(status) + && ageMinutes !== null + && ageMinutes >= options.staleAgeMinutes + ) { + reasons.push('terminal-status-old'); + } + + return { + ...session, + ageMinutes, + reasons, + stale: reasons.length > 0, + }; +} + +function cleanupAgentSessions(repoRoot, rawOptions = {}) { + const options = { + dryRun: Boolean(rawOptions.dryRun), + staleAgeMinutes: rawOptions.staleAgeMinutes ?? DEFAULT_STALE_AGE_MINUTES, + nowMs: rawOptions.nowMs ?? Date.now(), + existsSync: rawOptions.existsSync || fs.existsSync, + branchExists: rawOptions.branchExists || defaultBranchExists, + }; + + const sessions = listAgentSessions(repoRoot); + const candidates = sessions + .map((session) => evaluateSession(session, repoRoot, options)) + .filter((session) => session.stale); + + const removed = []; + if (!options.dryRun) { + for (const session of candidates) { + if (removeAgentSession(repoRoot, session.id)) { + removed.push(session.id); + } + } + } + + return { + schemaVersion: 1, + repoRoot, + dryRun: options.dryRun, + staleAgeMinutes: options.staleAgeMinutes, + inspected: sessions.length, + candidates, + removed, + }; +} + +function formatSessionLine(session, verb) { + const reasonText = session.reasons.join(','); + return `- ${verb} ${session.id} status=${session.status || '-'} branch=${session.branch || '-'} ` + + `worktree=${session.worktreePath || '-'} reasons=${reasonText}`; +} + +function renderCleanupSessionsResult(result, options = {}) { + if (options.json) return `${JSON.stringify(result, null, 2)}\n`; + + const action = result.dryRun ? 'would remove' : 'removed'; + const lines = [ + `[${TOOL_NAME}] Agent session cleanup: ${action} ${result.dryRun ? result.candidates.length : result.removed.length} ` + + `of ${result.inspected} (${result.repoRoot})`, + ]; + + if (result.candidates.length === 0) { + lines.push('- no stale session metadata found'); + } else { + for (const session of result.candidates) { + lines.push(formatSessionLine(session, action)); + } + } + + return `${lines.join('\n')}\n`; +} + +function runCleanupSessionsCommand(repoRoot, options = {}) { + return renderCleanupSessionsResult(cleanupAgentSessions(repoRoot, options), options); +} + +module.exports = { + DEFAULT_STALE_AGE_MINUTES, + TERMINAL_STATUSES, + cleanupAgentSessions, + renderCleanupSessionsResult, + runCleanupSessionsCommand, +}; diff --git a/src/cli/args.js b/src/cli/args.js index aea52225..15ecc509 100644 --- a/src/cli/args.js +++ b/src/cli/args.js @@ -276,6 +276,7 @@ function parseAgentsArgs(rawArgs) { reviewIntervalSeconds: 30, cleanupIntervalSeconds: 60, idleMinutes: DEFAULT_SHADOW_CLEANUP_IDLE_MINUTES, + staleAgeMinutes: 24 * 60, pid: null, branch: '', json: false, @@ -367,6 +368,19 @@ function parseAgentsArgs(rawArgs) { index += 1; continue; } + if (arg === '--older-than-minutes') { + const next = rest[index + 1]; + if (!next) { + throw new Error('--older-than-minutes requires an integer minutes value'); + } + const parsedValue = Number.parseInt(next, 10); + if (!Number.isInteger(parsedValue) || parsedValue < 1) { + throw new Error('--older-than-minutes must be an integer >= 1'); + } + options.staleAgeMinutes = parsedValue; + index += 1; + continue; + } if (arg === '--pid') { const next = rest[index + 1]; if (!next) { @@ -418,16 +432,19 @@ function parseAgentsArgs(rawArgs) { throw new Error(`Unknown option: ${arg}`); } - if (!['start', 'stop', 'status', 'files', 'diff', 'locks', 'finish'].includes(options.subcommand)) { + if (!['start', 'stop', 'status', 'files', 'diff', 'locks', 'finish', 'cleanup-sessions'].includes(options.subcommand)) { throw new Error(`Unknown agents subcommand: ${options.subcommand}`); } if (options.pid !== null && options.subcommand !== 'stop') { throw new Error('--pid is only supported with `gx agents stop`'); } - if ((options.task || options.agent || options.base || options.dryRun || options.claims.length > 0) && options.subcommand !== 'start') { - throw new Error('--task, --agent, --base, --dry-run, and --claim are only supported with `gx agents start`'); + if ((options.task || options.agent || options.base || options.claims.length > 0) && options.subcommand !== 'start') { + throw new Error('--task, --agent, --base, and --claim are only supported with `gx agents start`'); } - if (options.dryRun && !options.task) { + if (options.dryRun && !['start', 'cleanup-sessions'].includes(options.subcommand)) { + throw new Error('--dry-run is only supported with `gx agents start|cleanup-sessions`'); + } + if (options.subcommand === 'start' && options.dryRun && !options.task) { throw new Error('gx agents start --dry-run requires a task'); } if (options.claims.length > 0 && !options.task) { @@ -447,8 +464,11 @@ function parseAgentsArgs(rawArgs) { if (options.branch && !['files', 'diff', 'locks', 'finish'].includes(options.subcommand)) { throw new Error('--branch is only supported with `gx agents files|diff|locks|finish`'); } - if (options.json && !['status', 'files', 'diff', 'locks'].includes(options.subcommand)) { - throw new Error('--json is only supported with `gx agents status|files|diff|locks`'); + if (options.json && !['status', 'files', 'diff', 'locks', 'cleanup-sessions'].includes(options.subcommand)) { + throw new Error('--json is only supported with `gx agents status|files|diff|locks|cleanup-sessions`'); + } + if (options.staleAgeMinutes !== 24 * 60 && options.subcommand !== 'cleanup-sessions') { + throw new Error('--older-than-minutes is only supported with `gx agents cleanup-sessions`'); } return options; diff --git a/src/cli/main.js b/src/cli/main.js index 6b842c38..8a8c3fa8 100755 --- a/src/cli/main.js +++ b/src/cli/main.js @@ -7,6 +7,7 @@ const finishCommands = require('../finish'); const doctorModule = require('../doctor'); const agentInspect = require('../agents/inspect'); const agentStatus = require('../agents/status'); +const agentCleanupSessions = require('../agents/cleanup-sessions'); const { finishAgentSession } = require('../agents/finish'); const sessionSeverityReport = require('../report/session-severity'); const cockpitModule = require('../cockpit'); @@ -2662,6 +2663,12 @@ function agents(rawArgs) { return; } + if (options.subcommand === 'cleanup-sessions') { + process.stdout.write(agentCleanupSessions.runCleanupSessionsCommand(repoRoot, options)); + process.exitCode = 0; + return; + } + if (options.subcommand === 'start') { if (options.dryRun) { console.log(agentsStart.dryRunStart(options, repoRoot)); diff --git a/test/agents-cleanup-sessions.test.js b/test/agents-cleanup-sessions.test.js new file mode 100644 index 00000000..3a322f61 --- /dev/null +++ b/test/agents-cleanup-sessions.test.js @@ -0,0 +1,143 @@ +const { + test, + assert, + fs, + path, + runNode, + runCmd, + initRepo, + seedCommit, +} = require('./helpers/install-test-helpers'); +const { createAgentSession, readAgentSession } = require('../src/agents/sessions'); + +function makeRepo() { + const repoDir = initRepo(); + seedCommit(repoDir); + return repoDir; +} + +function createBranch(repoDir, branch) { + const result = runCmd('git', ['branch', branch], repoDir); + assert.equal(result.status, 0, result.stderr || result.stdout); +} + +function oldTimestamp(minutesAgo) { + return new Date(Date.now() - minutesAgo * 60000).toISOString(); +} + +function createSession(repoDir, overrides = {}) { + const worktreePath = overrides.worktreePath || path.join(repoDir, '.omx', 'agent-worktrees', overrides.id || 'session'); + if (overrides.createWorktree !== false) { + fs.mkdirSync(worktreePath, { recursive: true }); + } + const session = createAgentSession(repoDir, { + id: overrides.id || 'session', + agent: 'codex', + task: overrides.task || 'Cleanup session', + branch: overrides.branch || 'agent/codex/session', + base: 'main', + status: overrides.status || 'active', + worktreePath, + }); + if (overrides.updatedAt) { + const filePath = path.join(repoDir, '.guardex', 'agents', 'sessions', `${session.id}.json`); + const stored = JSON.parse(fs.readFileSync(filePath, 'utf8')); + stored.updatedAt = overrides.updatedAt; + fs.writeFileSync(filePath, `${JSON.stringify(stored, null, 2)}\n`, 'utf8'); + } + return session; +} + +test('agents cleanup-sessions removes a session with a missing worktree', () => { + const repoDir = makeRepo(); + createBranch(repoDir, 'agent/codex/missing-worktree'); + createSession(repoDir, { + id: 'missing-worktree', + branch: 'agent/codex/missing-worktree', + createWorktree: false, + }); + + const result = runNode(['agents', 'cleanup-sessions', '--target', repoDir], repoDir); + + assert.equal(result.status, 0, result.stderr || result.stdout); + assert.match(result.stdout, /removed missing-worktree/); + assert.equal(readAgentSession(repoDir, 'missing-worktree'), null); + const branch = runCmd('git', ['show-ref', '--verify', '--quiet', 'refs/heads/agent/codex/missing-worktree'], repoDir); + assert.equal(branch.status, 0, 'cleanup-sessions must not remove branches'); +}); + +test('agents cleanup-sessions removes a session with a missing branch', () => { + const repoDir = makeRepo(); + createSession(repoDir, { + id: 'missing-branch', + branch: 'agent/codex/missing-branch', + }); + + const result = runNode(['agents', 'cleanup-sessions', '--target', repoDir, '--json'], repoDir); + + assert.equal(result.status, 0, result.stderr || result.stdout); + const payload = JSON.parse(result.stdout); + assert.deepEqual(payload.removed, ['missing-branch']); + assert.deepEqual(payload.candidates[0].reasons, ['missing-branch']); + assert.equal(readAgentSession(repoDir, 'missing-branch'), null); + assert.equal(fs.existsSync(payload.candidates[0].worktreePath), true, 'cleanup-sessions must not remove worktrees'); +}); + +test('agents cleanup-sessions preserves active sessions with existing branch and worktree', () => { + const repoDir = makeRepo(); + createBranch(repoDir, 'agent/codex/active'); + const session = createSession(repoDir, { + id: 'active-session', + branch: 'agent/codex/active', + status: 'active', + }); + + const result = runNode(['agents', 'cleanup-sessions', '--target', repoDir], repoDir); + + assert.equal(result.status, 0, result.stderr || result.stdout); + assert.match(result.stdout, /removed 0 of 1/); + assert.deepEqual(readAgentSession(repoDir, 'active-session'), { + ...session, + status: 'active', + }); +}); + +test('agents cleanup-sessions removes old terminal sessions by configurable age', () => { + const repoDir = makeRepo(); + createBranch(repoDir, 'agent/codex/finished-old'); + createSession(repoDir, { + id: 'finished-old', + branch: 'agent/codex/finished-old', + status: 'finished', + updatedAt: oldTimestamp(90), + }); + + const result = runNode([ + 'agents', + 'cleanup-sessions', + '--target', + repoDir, + '--older-than-minutes', + '60', + ], repoDir); + + assert.equal(result.status, 0, result.stderr || result.stdout); + assert.match(result.stdout, /reasons=terminal-status-old/); + assert.equal(readAgentSession(repoDir, 'finished-old'), null); +}); + +test('agents cleanup-sessions --dry-run does not delete stale sessions', () => { + const repoDir = makeRepo(); + createBranch(repoDir, 'agent/codex/dry-run'); + createSession(repoDir, { + id: 'dry-run-session', + branch: 'agent/codex/dry-run', + createWorktree: false, + }); + + const result = runNode(['agents', 'cleanup-sessions', '--target', repoDir, '--dry-run'], repoDir); + + assert.equal(result.status, 0, result.stderr || result.stdout); + assert.match(result.stdout, /would remove dry-run-session/); + assert.notEqual(readAgentSession(repoDir, 'dry-run-session'), null); +}); diff --git a/test/cli-args-dispatch.test.js b/test/cli-args-dispatch.test.js index 7f88228f..4d7421a5 100644 --- a/test/cli-args-dispatch.test.js +++ b/test/cli-args-dispatch.test.js @@ -113,7 +113,12 @@ test('parseAgentsArgs applies interval overrides and validates the subcommand', reviewIntervalSeconds: 15, cleanupIntervalSeconds: 45, idleMinutes: 12, + staleAgeMinutes: 1440, pid: null, + branch: '', + json: false, + sessionId: '', + finishArgs: [], }); const dryRunOptions = parseAgentsArgs([