From ed2fe351f2edba55fe8c6778484b04a083b5775f Mon Sep 17 00:00:00 2001 From: NagyVikt Date: Wed, 29 Apr 2026 21:40:36 +0200 Subject: [PATCH] Let agent lanes claim ownership at startup Agent startup now accepts repeated --claim flags, creates the branch/worktree first, then reuses the existing lock tool to claim the requested files for the created branch. If the lock tool rejects the paths, startup records a claim-failed session and prints the exact recovery command instead of continuing silently. Constraint: Existing gx agents start repo-bot mode must keep working when no task/agent/claim lane inputs are provided. Rejected: Reimplement lock writes in JavaScript | the existing lock tool already owns conflict detection and path normalization. Confidence: high Scope-risk: moderate Directive: Keep startup claims routed through the lock tool so claim semantics stay single-sourced. Tested: node --test test/agents.test.js test/cli-args-dispatch.test.js test/agents-start-claims.test.js Tested: openspec validate agent-codex-agents-start-claims-manual-2026-04-29-2131 --strict Tested: git diff --check Not-tested: npm test is not fully green; existing test/agents-launch.test.js prompt-mode cases fail with Unsupported prompt mode for : argument. --- .../proposal.md | 13 ++ .../specs/agent-lane-start/spec.md | 23 +++ .../tasks.md | 22 +++ src/agents/start.js | 155 +++++++++++++++++- src/cli/args.js | 20 ++- src/cli/main.js | 7 + test/agents-start-claims.test.js | 108 ++++++++++++ test/cli-args-dispatch.test.js | 17 ++ 8 files changed, 359 insertions(+), 6 deletions(-) create mode 100644 openspec/changes/agent-codex-agents-start-claims-manual-2026-04-29-2131/proposal.md create mode 100644 openspec/changes/agent-codex-agents-start-claims-manual-2026-04-29-2131/specs/agent-lane-start/spec.md create mode 100644 openspec/changes/agent-codex-agents-start-claims-manual-2026-04-29-2131/tasks.md create mode 100644 test/agents-start-claims.test.js diff --git a/openspec/changes/agent-codex-agents-start-claims-manual-2026-04-29-2131/proposal.md b/openspec/changes/agent-codex-agents-start-claims-manual-2026-04-29-2131/proposal.md new file mode 100644 index 00000000..1bdb098f --- /dev/null +++ b/openspec/changes/agent-codex-agents-start-claims-manual-2026-04-29-2131/proposal.md @@ -0,0 +1,13 @@ +# Add startup file claims to `gx agents start` + +## Problem + +Agent lane startup can create the branch/worktree, but callers cannot pre-claim the files they already know the lane will edit. Agents must run a second command and can miss the ownership step. + +## Scope + +- Parse repeated `--claim ` flags on `gx agents start --agent `. +- After branch/worktree creation, claim the requested files for the created branch. +- Preserve existing repo-bot `gx agents start --target ` behavior. +- Mark startup session state as `claim-failed` and print recovery commands when claims fail. + diff --git a/openspec/changes/agent-codex-agents-start-claims-manual-2026-04-29-2131/specs/agent-lane-start/spec.md b/openspec/changes/agent-codex-agents-start-claims-manual-2026-04-29-2131/specs/agent-lane-start/spec.md new file mode 100644 index 00000000..45bbc8bd --- /dev/null +++ b/openspec/changes/agent-codex-agents-start-claims-manual-2026-04-29-2131/specs/agent-lane-start/spec.md @@ -0,0 +1,23 @@ +## ADDED Requirements + +### Requirement: `gx agents start` can claim files during lane startup + +`gx agents start --agent --claim ` SHALL create the agent branch/worktree and then claim each requested file for the created branch using the existing lock system. + +#### Scenario: no startup claims + +- **WHEN** a user runs `gx agents start "fix auth" --agent codex` +- **THEN** Guardex creates the agent branch/worktree +- **AND** Guardex does not create file lock entries. + +#### Scenario: repeated startup claims + +- **WHEN** a user runs `gx agents start "fix auth" --agent codex --claim src/auth.js --claim test/auth.test.js` +- **THEN** Guardex claims both files for the created branch after branch/worktree creation. + +#### Scenario: claim failure + +- **WHEN** branch/worktree creation succeeds but startup claim fails +- **THEN** Guardex SHALL exit nonzero +- **AND** Guardex SHALL mark the session state as `claim-failed` +- **AND** Guardex SHALL print recovery instructions with the worktree path and `gx locks claim --branch ...` command. diff --git a/openspec/changes/agent-codex-agents-start-claims-manual-2026-04-29-2131/tasks.md b/openspec/changes/agent-codex-agents-start-claims-manual-2026-04-29-2131/tasks.md new file mode 100644 index 00000000..78868b5e --- /dev/null +++ b/openspec/changes/agent-codex-agents-start-claims-manual-2026-04-29-2131/tasks.md @@ -0,0 +1,22 @@ +# Tasks + +## 1. Spec + +- [x] Record optional startup claim behavior for agent lane starts. + +## 2. Tests + +- [x] Add coverage for no claims, one claim, repeated claims, and claim failure recovery. +- [x] Keep legacy `gx agents start` repo-bot parser/behavior covered. + +## 3. Implementation + +- [x] Parse repeated `--claim` flags. +- [x] Run existing lock claiming after branch/worktree creation. +- [x] Write `claim-failed` session state and print recovery instructions on claim failure. + +## 4. Cleanup + +- [x] Focused verification: `node --test test/agents.test.js test/cli-args-dispatch.test.js test/agents-start-claims.test.js` -> 23/23 pass. +- [ ] Finish via PR merge and sandbox cleanup. + diff --git a/src/agents/start.js b/src/agents/start.js index 8b1b1068..fe816708 100644 --- a/src/agents/start.js +++ b/src/agents/start.js @@ -1,8 +1,16 @@ -const { path } = require('../context'); +const { + fs, + path, + TOOL_NAME, + SHORT_TOOL_NAME, +} = require('../context'); +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'); + function sanitizeSlug(value, fallback = 'task') { const slug = String(value || '') .toLowerCase() @@ -86,9 +94,154 @@ function dryRunStart(options, repoRoot) { return renderDryRunPlan(buildStartPlan(options, repoRoot)); } +function isSpawnFailure(result) { + return Boolean(result?.error) && typeof result?.status !== 'number'; +} + +function extractAgentBranchStartMetadata(output) { + const outputText = String(output || ''); + const branchMatch = outputText.match(/^\[agent-branch-start\] (?:Created branch|Reusing existing branch): (.+)$/m); + const worktreeMatch = outputText.match(/^\[agent-branch-start\] Worktree: (.+)$/m); + return { + branch: branchMatch ? branchMatch[1].trim() : '', + worktreePath: worktreeMatch ? worktreeMatch[1].trim() : '', + }; +} + +function sanitizeBranchForFile(branch) { + return String(branch || 'session') + .replace(/[^a-zA-Z0-9._-]+/g, '__') + .replace(/^_+|_+$/g, '') || 'session'; +} + +function sessionFilePathForBranch(repoRoot, branch) { + return path.join( + path.resolve(repoRoot), + ACTIVE_SESSIONS_RELATIVE_DIR, + `${sanitizeBranchForFile(branch)}.json`, + ); +} + +function writeClaimFailedSession(repoRoot, options, metadata, claimResult) { + if (!metadata.branch || !metadata.worktreePath) { + return ''; + } + const targetPath = sessionFilePathForBranch(repoRoot, metadata.branch); + const now = new Date().toISOString(); + const record = { + schemaVersion: 1, + repoRoot: path.resolve(repoRoot), + 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', + 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; +} + +function buildBranchStartArgs(options) { + const args = ['--task', options.task, '--agent', options.agent || 'codex']; + if (options.base) { + args.push('--base', options.base); + } + return args; +} + +function buildRecoveryLines(metadata, claims, sessionPath) { + const quotedClaims = claims.map((claim) => JSON.stringify(claim)).join(' '); + const lines = [ + `[${TOOL_NAME}] Claim failed after branch/worktree creation.`, + `[${TOOL_NAME}] Session status: claim-failed`, + ]; + if (sessionPath) { + lines.push(`[${TOOL_NAME}] Session record: ${sessionPath}`); + } + if (metadata.worktreePath) { + lines.push(`[${TOOL_NAME}] Recovery: cd ${JSON.stringify(metadata.worktreePath)}`); + } + if (metadata.branch) { + lines.push(`[${TOOL_NAME}] Recovery: ${SHORT_TOOL_NAME} locks claim --branch ${JSON.stringify(metadata.branch)} ${quotedClaims}`); + } + lines.push(`[${TOOL_NAME}] Recovery: resolve the lock conflict or invalid path, then rerun the claim command above.`); + return `${lines.join('\n')}\n`; +} + +function startAgentLane(repoRoot, options) { + const startResult = runPackageAsset('branchStart', buildBranchStartArgs(options), { cwd: repoRoot }); + let stdout = String(startResult.stdout || ''); + let stderr = String(startResult.stderr || ''); + if (isSpawnFailure(startResult)) { + return { + status: 1, + stdout, + stderr: `${stderr}${startResult.error.message}\n`, + }; + } + if (startResult.status !== 0) { + return { + status: typeof startResult.status === 'number' ? startResult.status : 1, + stdout, + stderr, + }; + } + + const metadata = extractAgentBranchStartMetadata(stdout); + if (options.claims.length === 0) { + return { status: 0, stdout, stderr }; + } + + if (!metadata.branch || !metadata.worktreePath) { + return { + status: 1, + stdout, + stderr: `${stderr}[${TOOL_NAME}] Unable to claim files: branch start output did not include branch/worktree metadata.\n`, + }; + } + + const claimResult = runPackageAsset( + 'lockTool', + ['claim', '--branch', metadata.branch, ...options.claims], + { cwd: metadata.worktreePath }, + ); + stdout += String(claimResult.stdout || ''); + stderr += String(claimResult.stderr || ''); + if (!isSpawnFailure(claimResult) && claimResult.status === 0) { + return { status: 0, stdout, stderr }; + } + + if (isSpawnFailure(claimResult)) { + stderr += `${claimResult.error.message}\n`; + } + const sessionPath = writeClaimFailedSession(repoRoot, options, metadata, claimResult); + stdout += buildRecoveryLines(metadata, options.claims, sessionPath); + return { + status: typeof claimResult.status === 'number' ? claimResult.status : 1, + stdout, + stderr, + }; +} + module.exports = { + buildBranchStartArgs, buildStartPlan, + buildRecoveryLines, dryRunStart, + extractAgentBranchStartMetadata, renderDryRunPlan, sanitizeSlug, + sessionFilePathForBranch, + startAgentLane, }; diff --git a/src/cli/args.js b/src/cli/args.js index c3163e31..efa536de 100644 --- a/src/cli/args.js +++ b/src/cli/args.js @@ -271,6 +271,7 @@ function parseAgentsArgs(rawArgs) { task: '', agent: '', base: '', + claims: [], dryRun: false, reviewIntervalSeconds: 30, cleanupIntervalSeconds: 60, @@ -354,6 +355,15 @@ function parseAgentsArgs(rawArgs) { options.dryRun = true; continue; } + if (arg === '--claim') { + const next = rest[index + 1]; + if (!next || next.startsWith('-')) { + throw new Error('--claim requires a file path'); + } + options.claims.push(next); + index += 1; + continue; + } if (!arg.startsWith('-') && options.subcommand === 'start' && !options.task) { options.task = arg; continue; @@ -367,15 +377,15 @@ function parseAgentsArgs(rawArgs) { 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.subcommand !== 'start') { - throw new Error('--task, --agent, --base, and --dry-run are only supported with `gx agents start`'); - } - if (options.task && !options.dryRun) { - throw new Error('gx agents start is currently supported only with --dry-run'); + 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.dryRun && !options.task) { throw new Error('gx agents start --dry-run requires a task'); } + if (options.claims.length > 0 && !options.task) { + throw new Error('gx agents start --claim requires a task'); + } return options; } diff --git a/src/cli/main.js b/src/cli/main.js index 525cfc7a..f49dca66 100755 --- a/src/cli/main.js +++ b/src/cli/main.js @@ -2653,6 +2653,13 @@ function agents(rawArgs) { process.exitCode = 0; return; } + if (options.task) { + const result = agentsStart.startAgentLane(repoRoot, options); + if (result.stdout) process.stdout.write(result.stdout); + if (result.stderr) process.stderr.write(result.stderr); + process.exitCode = result.status; + return; + } const existingState = readAgentsState(repoRoot); const existingReviewPid = Number.parseInt(String(existingState?.review?.pid || ''), 10); diff --git a/test/agents-start-claims.test.js b/test/agents-start-claims.test.js new file mode 100644 index 00000000..d3179919 --- /dev/null +++ b/test/agents-start-claims.test.js @@ -0,0 +1,108 @@ +const { + test, + assert, + fs, + path, + runNode, + initRepo, + seedCommit, + extractCreatedBranch, + extractCreatedWorktree, + defineSpawnSuite, +} = require('./helpers/install-test-helpers'); + +function readLocks(worktreePath) { + const lockPath = path.join(worktreePath, '.omx', 'state', 'agent-file-locks.json'); + if (!fs.existsSync(lockPath)) { + return {}; + } + return JSON.parse(fs.readFileSync(lockPath, 'utf8')).locks || {}; +} + +defineSpawnSuite('agents start claim suite', () => { + test('agents start creates an agent lane without claims', () => { + const repoDir = initRepo(); + seedCommit(repoDir); + + const result = runNode(['agents', 'start', 'fix auth', '--agent', 'codex', '--target', repoDir], repoDir); + assert.equal(result.status, 0, result.stderr || result.stdout); + assert.match(result.stdout, /\[agent-branch-start\] Created branch: agent\/codex\//); + assert.doesNotMatch(result.stdout, /Claimed 1 file/); + + const worktreePath = extractCreatedWorktree(result.stdout); + assert.equal(fs.existsSync(path.join(worktreePath, '.omx', 'state', 'agent-file-locks.json')), false); + }); + + test('agents start claims one file after branch creation', () => { + const repoDir = initRepo(); + seedCommit(repoDir); + + const result = runNode( + ['agents', 'start', 'fix auth', '--agent', 'codex', '--claim', 'src/auth.js', '--target', repoDir], + repoDir, + ); + assert.equal(result.status, 0, result.stderr || result.stdout); + assert.match(result.stdout, /Claimed 1 file\(s\)/); + + const branch = extractCreatedBranch(result.stdout); + const worktreePath = extractCreatedWorktree(result.stdout); + assert.equal(readLocks(worktreePath)['src/auth.js'].branch, branch); + }); + + test('agents start supports repeated claim flags', () => { + const repoDir = initRepo(); + seedCommit(repoDir); + + const result = runNode( + [ + 'agents', + 'start', + 'fix auth', + '--agent', + 'codex', + '--claim', + 'src/auth.js', + '--claim', + 'test/auth.test.js', + '--target', + repoDir, + ], + repoDir, + ); + assert.equal(result.status, 0, result.stderr || result.stdout); + assert.match(result.stdout, /Claimed 2 file\(s\)/); + + const branch = extractCreatedBranch(result.stdout); + const locks = readLocks(extractCreatedWorktree(result.stdout)); + assert.equal(locks['src/auth.js'].branch, branch); + assert.equal(locks['test/auth.test.js'].branch, branch); + }); + + test('agents start marks claim-failed and prints recovery instructions when claim fails', () => { + const repoDir = initRepo(); + seedCommit(repoDir); + const outsidePath = path.join(path.dirname(repoDir), 'outside.js'); + + const result = runNode( + ['agents', 'start', 'fix auth', '--agent', 'codex', '--claim', outsidePath, '--target', repoDir], + repoDir, + ); + assert.notEqual(result.status, 0, 'claim failure should fail the command'); + assert.match(result.stderr, /Path is outside repository/); + assert.match(result.stdout, /Session status: claim-failed/); + assert.match(result.stdout, /Recovery: cd /); + 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'); + assert.deepEqual(session.claimFailure.claims, [outsidePath]); + }); +}); diff --git a/test/cli-args-dispatch.test.js b/test/cli-args-dispatch.test.js index 789a322d..7f88228f 100644 --- a/test/cli-args-dispatch.test.js +++ b/test/cli-args-dispatch.test.js @@ -108,6 +108,7 @@ test('parseAgentsArgs applies interval overrides and validates the subcommand', task: '', agent: '', base: '', + claims: [], dryRun: false, reviewIntervalSeconds: 15, cleanupIntervalSeconds: 45, @@ -129,6 +130,22 @@ test('parseAgentsArgs applies interval overrides and validates the subcommand', assert.equal(dryRunOptions.agent, 'codex'); assert.equal(dryRunOptions.base, 'main'); assert.equal(dryRunOptions.dryRun, true); + + const claimOptions = parseAgentsArgs([ + 'start', + 'fix auth tests', + '--agent', + 'codex', + '--claim', + 'src/auth.js', + '--claim', + 'test/auth.test.js', + ]); + + assert.equal(claimOptions.task, 'fix auth tests'); + assert.equal(claimOptions.agent, 'codex'); + assert.deepEqual(claimOptions.claims, ['src/auth.js', 'test/auth.test.js']); + assert.equal(claimOptions.dryRun, false); }); test('parseReportArgs accepts the session-severity flag set', () => {