From 6240fa0201adc17a381c307e05ae6e186570a8c4 Mon Sep 17 00:00:00 2001 From: NagyVikt Date: Wed, 29 Apr 2026 22:10:19 +0200 Subject: [PATCH] Unify gx agent session storage gx agents start, cockpit, and finish need one durable session source so started lanes can be observed and finished consistently. This routes start through the canonical .guardex agent session helper, keeps claim-failed starts in the same store, and makes cockpit read canonical sessions before legacy .omx active-session records. Constraint: Existing .omx/state/active-sessions records must remain visible while live older sessions age out. Rejected: Keep writing claim failures to .omx only | finish cannot resolve those sessions through the gx agents session store Confidence: high Scope-risk: moderate Directive: Do not add a second gx agents session writer; use src/agents/sessions.js for command-owned sessions. Tested: node --check src/agents/start.js; node --test test/agents-start-claims.test.js test/cockpit-render.test.js test/agents-finish.test.js Not-tested: full npm test suite --- src/agents/sessions.js | 3 ++ src/agents/start.js | 75 ++++++++++++++++++++------------ src/cockpit/state.js | 29 +++++++++++- test/agents-start-claims.test.js | 42 ++++++++++++++---- test/cockpit-render.test.js | 36 ++++++++++++++- 5 files changed, 146 insertions(+), 39 deletions(-) diff --git a/src/agents/sessions.js b/src/agents/sessions.js index 4ce91c7d..fc4818b8 100644 --- a/src/agents/sessions.js +++ b/src/agents/sessions.js @@ -10,6 +10,7 @@ const SESSION_FIELDS = [ 'worktreePath', 'base', 'status', + 'claimFailure', 'createdAt', 'updatedAt', ]; @@ -133,6 +134,8 @@ function removeAgentSession(repoRoot, sessionId) { } module.exports = { + sessionFilePath, + sessionsDir, createAgentSession, readAgentSession, updateAgentSession, diff --git a/src/agents/start.js b/src/agents/start.js index fe816708..05bdc483 100644 --- a/src/agents/start.js +++ b/src/agents/start.js @@ -1,5 +1,4 @@ const { - fs, path, TOOL_NAME, SHORT_TOOL_NAME, @@ -8,8 +7,12 @@ const { runPackageAsset } = require('../core/runtime'); const { currentBranchName } = require('../git'); const { buildAgentLaunchCommand } = require('./launch'); const { resolveAgent } = require('./registry'); - -const ACTIVE_SESSIONS_RELATIVE_DIR = path.join('.omx', 'state', 'active-sessions'); +const { + createAgentSession, + listAgentSessions, + sessionFilePath, + updateAgentSession, +} = require('./sessions'); function sanitizeSlug(value, fallback = 'task') { const slug = String(value || '') @@ -114,42 +117,55 @@ function sanitizeBranchForFile(branch) { .replace(/^_+|_+$/g, '') || 'session'; } -function sessionFilePathForBranch(repoRoot, branch) { - return path.join( - path.resolve(repoRoot), - ACTIVE_SESSIONS_RELATIVE_DIR, - `${sanitizeBranchForFile(branch)}.json`, - ); +function agentSessionIdForBranch(branch) { + return sanitizeBranchForFile(branch); } -function writeClaimFailedSession(repoRoot, options, metadata, claimResult) { +function buildSessionPayload(options, metadata, status, extra = {}) { if (!metadata.branch || !metadata.worktreePath) { - return ''; + return null; } - const targetPath = sessionFilePathForBranch(repoRoot, metadata.branch); - const now = new Date().toISOString(); - const record = { - schemaVersion: 1, - repoRoot: path.resolve(repoRoot), + + return { + id: extra.id || agentSessionIdForBranch(metadata.branch), + task: options.task, + agent: options.agent || 'codex', branch: metadata.branch, - taskName: options.task, - agentName: options.agent || 'codex', worktreePath: path.resolve(metadata.worktreePath), - pid: process.pid, - cliName: SHORT_TOOL_NAME, - startedAt: now, - lastHeartbeatAt: now, - state: 'claim-failed', + base: options.base || null, + status, + ...extra, + }; +} + +function findSessionByBranch(repoRoot, branch) { + return listAgentSessions(repoRoot).find((session) => session.branch === branch) || null; +} + +function writeAgentSession(repoRoot, options, metadata, status, extra = {}) { + const payload = buildSessionPayload(options, metadata, status, extra); + if (!payload) { + return null; + } + + const existing = findSessionByBranch(repoRoot, metadata.branch); + if (existing) { + return updateAgentSession(repoRoot, existing.id, payload); + } + + return createAgentSession(repoRoot, payload); +} + +function writeClaimFailedSession(repoRoot, options, metadata, claimResult) { + const session = writeAgentSession(repoRoot, options, metadata, 'claim-failed', { claimFailure: { claims: options.claims, exitCode: typeof claimResult.status === 'number' ? claimResult.status : 1, stderr: String(claimResult.stderr || '').trim(), stdout: String(claimResult.stdout || '').trim(), }, - }; - fs.mkdirSync(path.dirname(targetPath), { recursive: true }); - fs.writeFileSync(targetPath, `${JSON.stringify(record, null, 2)}\n`, 'utf8'); - return targetPath; + }); + return session ? sessionFilePath(repoRoot, session.id) : ''; } function buildBranchStartArgs(options) { @@ -200,6 +216,7 @@ function startAgentLane(repoRoot, options) { const metadata = extractAgentBranchStartMetadata(stdout); if (options.claims.length === 0) { + writeAgentSession(repoRoot, options, metadata, 'active'); return { status: 0, stdout, stderr }; } @@ -219,6 +236,7 @@ function startAgentLane(repoRoot, options) { stdout += String(claimResult.stdout || ''); stderr += String(claimResult.stderr || ''); if (!isSpawnFailure(claimResult) && claimResult.status === 0) { + writeAgentSession(repoRoot, options, metadata, 'active'); return { status: 0, stdout, stderr }; } @@ -240,8 +258,9 @@ module.exports = { buildRecoveryLines, dryRunStart, extractAgentBranchStartMetadata, + agentSessionIdForBranch, renderDryRunPlan, sanitizeSlug, - sessionFilePathForBranch, + writeAgentSession, startAgentLane, }; diff --git a/src/cockpit/state.js b/src/cockpit/state.js index 1872f7e8..7e61c94f 100644 --- a/src/cockpit/state.js +++ b/src/cockpit/state.js @@ -1,6 +1,10 @@ const fs = require('node:fs'); const path = require('node:path'); const cp = require('node:child_process'); +const { + listAgentSessions, + sessionFilePath, +} = require('../agents/sessions'); const ACTIVE_SESSIONS_DIR = path.join('.omx', 'state', 'active-sessions'); const LOCK_FILE = path.join('.omx', 'state', 'agent-file-locks.json'); @@ -67,7 +71,7 @@ function normalizeSession(input, filePath) { }; } -function readActiveSessions(repoPath) { +function readLegacyActiveSessions(repoPath) { const sessionsDir = path.join(repoPath, ACTIVE_SESSIONS_DIR); if (!fs.existsSync(sessionsDir)) { return []; @@ -80,6 +84,28 @@ function readActiveSessions(repoPath) { return normalizeSession(readJson(filePath), filePath); }) .filter(Boolean) +} + +function readCanonicalActiveSessions(repoPath) { + return listAgentSessions(repoPath) + .map((session) => normalizeSession(session, sessionFilePath(repoPath, session.id))) + .filter(Boolean); +} + +function sessionKey(session) { + return `${session.branch}\0${session.worktreePath}`; +} + +function readActiveSessions(repoPath) { + const byKey = new Map(); + for (const session of readLegacyActiveSessions(repoPath)) { + byKey.set(sessionKey(session), session); + } + for (const session of readCanonicalActiveSessions(repoPath)) { + byKey.set(sessionKey(session), session); + } + + return Array.from(byKey.values()) .sort((left, right) => left.branch.localeCompare(right.branch)); } @@ -128,6 +154,7 @@ module.exports = { LOCK_FILE, readCockpitState, readActiveSessions, + readLegacyActiveSessions, readBaseBranch, readLocksByBranch, }; diff --git a/test/agents-start-claims.test.js b/test/agents-start-claims.test.js index d3179919..1e2df45a 100644 --- a/test/agents-start-claims.test.js +++ b/test/agents-start-claims.test.js @@ -10,6 +10,8 @@ const { extractCreatedWorktree, defineSpawnSuite, } = require('./helpers/install-test-helpers'); +const { finishAgentSession } = require('../src/agents/finish'); +const { listAgentSessions, readAgentSession } = require('../src/agents/sessions'); function readLocks(worktreePath) { const lockPath = path.join(worktreePath, '.omx', 'state', 'agent-file-locks.json'); @@ -31,6 +33,13 @@ defineSpawnSuite('agents start claim suite', () => { const worktreePath = extractCreatedWorktree(result.stdout); assert.equal(fs.existsSync(path.join(worktreePath, '.omx', 'state', 'agent-file-locks.json')), false); + + const [session] = listAgentSessions(repoDir); + assert.equal(session.task, 'fix auth'); + assert.equal(session.agent, 'codex'); + assert.equal(session.branch, extractCreatedBranch(result.stdout)); + assert.equal(session.worktreePath, worktreePath); + assert.equal(session.status, 'active'); }); test('agents start claims one file after branch creation', () => { @@ -94,15 +103,30 @@ defineSpawnSuite('agents start claim suite', () => { assert.match(result.stdout, /Recovery: gx locks claim --branch /); const branch = extractCreatedBranch(result.stdout); - const sessionPath = path.join( - repoDir, - '.omx', - 'state', - 'active-sessions', - `${branch.replace(/[^a-zA-Z0-9._-]+/g, '__').replace(/^_+|_+$/g, '')}.json`, - ); - const session = JSON.parse(fs.readFileSync(sessionPath, 'utf8')); - assert.equal(session.state, 'claim-failed'); + const [session] = listAgentSessions(repoDir); + assert.equal(session.branch, branch); + assert.equal(session.status, 'claim-failed'); assert.deepEqual(session.claimFailure.claims, [outsidePath]); + assert.equal(fs.existsSync(path.join(repoDir, '.omx', 'state', 'active-sessions')), false); + }); + + test('agents finish resolves a canonical session created by agents start', () => { + const repoDir = initRepo(); + seedCommit(repoDir); + + const startResult = runNode(['agents', 'start', 'finish started session', '--agent', 'codex', '--target', repoDir], repoDir); + assert.equal(startResult.status, 0, startResult.stderr || startResult.stdout); + + const [session] = listAgentSessions(repoDir); + const calls = []; + const output = { write() {} }; + const result = finishAgentSession(repoDir, { sessionId: session.id, branch: '', finishArgs: ['--no-wait-for-merge'] }, { + output, + finishRunner(args) { calls.push(args); return { ok: true }; }, + }); + + assert.equal(result.status, 'pr-opened'); + assert.deepEqual(calls, [['--target', repoDir, '--branch', extractCreatedBranch(startResult.stdout), '--no-wait-for-merge']]); + assert.equal(readAgentSession(repoDir, session.id).status, 'pr-opened'); }); }); diff --git a/test/cockpit-render.test.js b/test/cockpit-render.test.js index ebca2f80..8a88e913 100644 --- a/test/cockpit-render.test.js +++ b/test/cockpit-render.test.js @@ -8,6 +8,7 @@ const cp = require('node:child_process'); const { renderCockpit } = require('../src/cockpit/render'); const { readCockpitState } = require('../src/cockpit/state'); const { render } = require('../src/cockpit'); +const { createAgentSession } = require('../src/agents/sessions'); function initRepo() { const repoPath = fs.mkdtempSync(path.join(os.tmpdir(), 'guardex-cockpit-')); @@ -43,7 +44,40 @@ test('renderCockpit returns a readable terminal string', () => { assert.match(output, /task: implement cockpit/); }); -test('readCockpitState reads active sessions and lock summaries', () => { +test('readCockpitState reads canonical sessions and lock summaries', () => { + const repoPath = initRepo(); + createAgentSession(repoPath, { + id: 'canonical-cockpit', + agent: 'codex', + branch: 'agent/codex/example', + worktreePath: path.join(repoPath, '.omx', 'agent-worktrees', 'example'), + status: 'working', + task: 'implement cockpit', + }); + fs.mkdirSync(path.join(repoPath, '.omx', 'state'), { recursive: true }); + fs.writeFileSync( + path.join(repoPath, '.omx', 'state', 'agent-file-locks.json'), + JSON.stringify({ + locks: { + 'src/cockpit/render.js': { branch: 'agent/codex/example' }, + 'src/cockpit/state.js': { branch: 'agent/codex/example' }, + 'README.md': { branch: 'agent/other/example' }, + }, + }), + 'utf8', + ); + + const state = readCockpitState(repoPath); + + assert.equal(state.repoPath, repoPath); + assert.equal(state.baseBranch, 'main'); + assert.equal(state.sessions.length, 1); + assert.equal(state.sessions[0].status, 'working'); + assert.equal(state.sessions[0].task, 'implement cockpit'); + assert.deepEqual(state.sessions[0].locks, ['src/cockpit/render.js', 'src/cockpit/state.js']); +}); + +test('readCockpitState still reads legacy .omx active sessions', () => { const repoPath = initRepo(); const sessionsDir = path.join(repoPath, '.omx', 'state', 'active-sessions'); fs.mkdirSync(sessionsDir, { recursive: true });