From 1b28870fb7e4c20a00bf158220054d1d328a9aa4 Mon Sep 17 00:00:00 2001 From: NagyVikt Date: Tue, 21 Apr 2026 13:32:26 +0200 Subject: [PATCH] Align Guardex finish-path regressions with the current workflow contract This change finishes the remaining workflow-regression cleanup around doctor sandbox repair sync, agent role naming, and codex-agent auto-finish gating. Doctor no longer treats its own .omx/agent-worktrees sandbox path as unrelated protected-branch drift, finish runs now allocate unique integration refs, and the focused install regressions were updated to the current protected-main and local-remote behavior. The OpenSpec change docs were also backfilled so the branch is reviewable instead of carrying placeholder scaffolding. Constraint: Doctor and auto-finish can invoke multiple finish flows inside the same repo within one timestamp window Rejected: Keep the stale install expectations and only patch runtime output | that would leave real doctor sync and integration-ref collisions hidden Confidence: high Scope-risk: moderate Directive: Treat doctor sandbox worktrees as internal workflow state during protected-base repair sync, and keep temporary integration refs unique across consecutive finish runs Tested: node --test test/install.test.js; bash scripts/test-agent-naming.sh; openspec validate fix-regression-finish-flows --type change --strict; openspec validate --specs Not-tested: Live GitHub merge behavior outside the scripted harness --- bin/multiagent-safety.js | 696 +++++++++--------- .../.openspec.yaml | 2 + .../fix-regression-finish-flows/proposal.md | 24 + .../specs/workflow-guardrails/spec.md | 31 + .../fix-regression-finish-flows/tasks.md | 21 + scripts/agent-branch-finish.sh | 29 +- scripts/agent-branch-start.sh | 22 +- scripts/codex-agent.sh | 33 +- scripts/test-agent-naming.sh | 9 +- templates/scripts/agent-branch-finish.sh | 29 +- templates/scripts/agent-branch-start.sh | 22 +- templates/scripts/codex-agent.sh | 33 +- test/install.test.js | 266 +++---- 13 files changed, 668 insertions(+), 549 deletions(-) create mode 100644 openspec/changes/fix-regression-finish-flows/.openspec.yaml create mode 100644 openspec/changes/fix-regression-finish-flows/proposal.md create mode 100644 openspec/changes/fix-regression-finish-flows/specs/workflow-guardrails/spec.md create mode 100644 openspec/changes/fix-regression-finish-flows/tasks.md diff --git a/bin/multiagent-safety.js b/bin/multiagent-safety.js index 3672e28..acfbee6 100755 --- a/bin/multiagent-safety.js +++ b/bin/multiagent-safety.js @@ -187,6 +187,8 @@ const MANAGED_GITIGNORE_PATHS = [ '.omx/', '.omc/', 'scripts/*', + 'scripts/agent-branch-start.sh', + 'scripts/agent-file-locks.py', '.githooks', 'oh-my-codex/', '.codex/skills/gitguardex/SKILL.md', @@ -488,45 +490,6 @@ function gitRun(repoRoot, args, { allowFailure = false } = {}) { return result; } -function trackedStatusByPath(repoRoot) { - const result = gitRun(repoRoot, ['status', '--short', '--untracked-files=no', '--porcelain']); - const statuses = new Map(); - const lines = String(result.stdout || '') - .split('\n') - .map((line) => line.trimEnd()) - .filter(Boolean); - for (const line of lines) { - if (line.length < 4) { - continue; - } - statuses.set(line.slice(3), line.slice(0, 2)); - } - return statuses; -} - -function restoreTrackedWorktreePaths(repoRoot, relativePaths) { - const uniquePaths = [...new Set(relativePaths.filter(Boolean))]; - if (uniquePaths.length === 0) { - return; - } - - let result = gitRun( - repoRoot, - ['restore', '--worktree', '--source=HEAD', '--', ...uniquePaths], - { allowFailure: true }, - ); - if (result.status === 0) { - return; - } - - result = gitRun(repoRoot, ['checkout', '--', ...uniquePaths], { allowFailure: true }); - if (result.status !== 0) { - throw new Error( - `Unable to restore tracked protected-base paths: ${(result.stderr || '').trim() || (result.stdout || '').trim()}`, - ); - } -} - function resolveRepoRoot(targetPath) { const resolvedTarget = path.resolve(targetPath || process.cwd()); const result = run('git', ['-C', resolvedTarget, 'rev-parse', '--show-toplevel']); @@ -1310,32 +1273,11 @@ function resolveSandboxTarget(repoRoot, worktreePath, targetPath) { function buildSandboxSetupArgs(options, sandboxTarget) { const args = ['setup', '--target', sandboxTarget, '--no-global-install', '--no-recursive']; - if (options.dryRun) args.push('--dry-run'); - if (options.force) args.push('--force'); - if (options.skipAgents) args.push('--skip-agents'); - if (options.skipPackageJson) args.push('--skip-package-json'); - if (options.skipGitignore) args.push('--no-gitignore'); - if (options.parentWorkspaceView) args.push('--parent-workspace-view'); - return args; -} - -function buildSandboxInstallArgs(options, sandboxTarget) { - const args = ['install', '--target', sandboxTarget]; - if (options.dryRun) args.push('--dry-run'); if (options.force) args.push('--force'); if (options.skipAgents) args.push('--skip-agents'); if (options.skipPackageJson) args.push('--skip-package-json'); if (options.skipGitignore) args.push('--no-gitignore'); - return args; -} - -function buildSandboxFixArgs(options, sandboxTarget) { - const args = ['fix', '--target', sandboxTarget]; if (options.dryRun) args.push('--dry-run'); - if (options.skipAgents) args.push('--skip-agents'); - if (options.skipPackageJson) args.push('--skip-package-json'); - if (options.skipGitignore) args.push('--no-gitignore'); - if (!options.dropStaleLocks) args.push('--keep-stale-locks'); return args; } @@ -1416,7 +1358,7 @@ function resolveProtectedBaseSandboxStartRef(repoRoot, baseBranch) { if (currentBranchName(repoRoot) === baseBranch) { return null; } - throw new Error(`Unable to find base ref for protected-base sandbox: ${baseBranch}`); + throw new Error(`Unable to find base ref for sandbox bootstrap: ${baseBranch}`); } function startProtectedBaseSandboxFallback(blocked, sandboxSuffix) { @@ -1440,7 +1382,7 @@ function startProtectedBaseSandboxFallback(blocked, sandboxSuffix) { } if (!selectedBranch || !selectedWorktreePath) { - throw new Error('Unable to allocate unique protected-base sandbox branch/worktree'); + throw new Error('Unable to allocate unique sandbox branch/worktree'); } fs.mkdirSync(path.dirname(selectedWorktreePath), { recursive: true }); @@ -1453,7 +1395,7 @@ function startProtectedBaseSandboxFallback(blocked, sandboxSuffix) { throw addResult.error; } if (addResult.status !== 0) { - throw new Error((addResult.stderr || addResult.stdout || 'failed to create protected-base sandbox').trim()); + throw new Error((addResult.stderr || addResult.stdout || 'failed to create sandbox').trim()); } if (!startRef) { @@ -1485,6 +1427,10 @@ function startProtectedBaseSandboxFallback(blocked, sandboxSuffix) { } function startProtectedBaseSandbox(blocked, { taskName, sandboxSuffix }) { + if (sandboxSuffix === 'gx-doctor') { + return startProtectedBaseSandboxFallback(blocked, sandboxSuffix); + } + const startScript = path.join(blocked.repoRoot, 'scripts', 'agent-branch-start.sh'); if (!fs.existsSync(startScript)) { return startProtectedBaseSandboxFallback(blocked, sandboxSuffix); @@ -1518,7 +1464,7 @@ function startProtectedBaseSandbox(blocked, { taskName, sandboxSuffix }) { if (!restoreResult.ok) { const detail = [restoreResult.stderr, restoreResult.stdout].filter(Boolean).join('\n').trim(); throw new Error( - `protected-base sandbox startup switched '${blocked.branch}' and could not restore it.` + + `sandbox startup switched protected base checkout and could not restore '${blocked.branch}'.` + (detail ? `\n${detail}` : ''), ); } @@ -1536,10 +1482,10 @@ function cleanupProtectedBaseSandbox(repoRoot, metadata) { const result = { worktree: 'skipped', branch: 'skipped', - note: '', + note: 'missing sandbox metadata', }; + if (!metadata?.worktreePath || !metadata?.branch) { - result.note = 'missing sandbox metadata'; return result; } @@ -1563,15 +1509,17 @@ function cleanupProtectedBaseSandbox(repoRoot, metadata) { } if (gitRefExists(repoRoot, `refs/heads/${metadata.branch}`)) { - const deleteResult = run('git', ['-C', repoRoot, 'branch', '-D', metadata.branch], { - timeout: 20_000, - }); - if (isSpawnFailure(deleteResult)) { - throw deleteResult.error; + const branchDeleteResult = run( + 'git', + ['-C', repoRoot, 'branch', '-D', metadata.branch], + { timeout: 20_000 }, + ); + if (isSpawnFailure(branchDeleteResult)) { + throw branchDeleteResult.error; } - if (deleteResult.status !== 0) { + if (branchDeleteResult.status !== 0) { throw new Error( - (deleteResult.stderr || deleteResult.stdout || 'failed to delete sandbox branch').trim(), + (branchDeleteResult.stderr || branchDeleteResult.stdout || 'failed to delete sandbox branch').trim(), ); } result.branch = 'deleted'; @@ -1579,7 +1527,7 @@ function cleanupProtectedBaseSandbox(repoRoot, metadata) { result.branch = 'missing'; } - result.note = 'sandbox branch/worktree pruned'; + result.note = 'sandbox worktree pruned'; return result; } @@ -1590,7 +1538,7 @@ function parseGitPathList(output) { .filter((line) => line && line !== LOCK_FILE_RELATIVE); } -function collectSandboxChangedPaths(worktreePath) { +function collectDoctorChangedPaths(worktreePath) { const changed = new Set(); const commands = [ ['diff', '--name-only'], @@ -1606,7 +1554,7 @@ function collectSandboxChangedPaths(worktreePath) { return Array.from(changed); } -function collectSandboxDeletedPaths(worktreePath) { +function collectDoctorDeletedPaths(worktreePath) { const deleted = new Set(); const commands = [ ['diff', '--name-only', '--diff-filter=D'], @@ -1621,7 +1569,60 @@ function collectSandboxDeletedPaths(worktreePath) { return Array.from(deleted); } -function claimSandboxChangedLocks(metadata, noteLabel) { +function collectWorktreeDirtyPaths(worktreePath) { + const dirty = new Set(); + const commands = [ + ['diff', '--name-only'], + ['diff', '--cached', '--name-only'], + ['ls-files', '--others', '--exclude-standard'], + ]; + for (const gitArgs of commands) { + const result = run('git', ['-C', worktreePath, ...gitArgs], { timeout: 20_000 }); + for (const filePath of parseGitPathList(result.stdout)) { + dirty.add(filePath); + } + } + return Array.from(dirty); +} + +function collectDoctorForceAddPaths(worktreePath) { + return TEMPLATE_FILES + .map((entry) => toDestinationPath(entry)) + .filter((relativePath) => relativePath.startsWith('scripts/') || relativePath.startsWith('.githooks/')) + .filter((relativePath) => fs.existsSync(path.join(worktreePath, relativePath))); +} + +function stripDoctorSandboxLocks(rawContent, branchName) { + if (!rawContent || !branchName) { + return rawContent; + } + try { + const parsed = JSON.parse(rawContent); + const locks = parsed && typeof parsed === 'object' && parsed.locks && typeof parsed.locks === 'object' + ? parsed.locks + : null; + if (!locks) { + return rawContent; + } + let changed = false; + const filteredLocks = {}; + for (const [filePath, lockInfo] of Object.entries(locks)) { + if (lockInfo && lockInfo.branch === branchName) { + changed = true; + continue; + } + filteredLocks[filePath] = lockInfo; + } + if (!changed) { + return rawContent; + } + return `${JSON.stringify({ ...parsed, locks: filteredLocks }, null, 2)}\n`; + } catch { + return rawContent; + } +} + +function claimDoctorChangedLocks(metadata) { const lockScript = path.join(metadata.worktreePath, 'scripts', 'agent-file-locks.py'); if (!fs.existsSync(lockScript) || !metadata.branch) { return { @@ -1632,8 +1633,11 @@ function claimSandboxChangedLocks(metadata, noteLabel) { }; } - const changedPaths = collectSandboxChangedPaths(metadata.worktreePath); - const deletedPaths = collectSandboxDeletedPaths(metadata.worktreePath); + const changedPaths = Array.from(new Set([ + ...collectDoctorChangedPaths(metadata.worktreePath), + ...collectDoctorForceAddPaths(metadata.worktreePath), + ])); + const deletedPaths = collectDoctorDeletedPaths(metadata.worktreePath); if (changedPaths.length > 0) { run('python3', [lockScript, 'claim', '--branch', metadata.branch, ...changedPaths], { cwd: metadata.worktreePath, @@ -1649,13 +1653,13 @@ function claimSandboxChangedLocks(metadata, noteLabel) { return { status: 'claimed', - note: `claimed locks for ${noteLabel}`, + note: 'claimed locks for doctor auto-commit', changedCount: changedPaths.length, deletedCount: deletedPaths.length, }; } -function autoCommitSandboxChanges(metadata, { noteLabel, commitMessage }) { +function autoCommitDoctorSandboxChanges(metadata) { if (!metadata.worktreePath || !metadata.branch) { return { status: 'skipped', @@ -1663,8 +1667,20 @@ function autoCommitSandboxChanges(metadata, { noteLabel, commitMessage }) { }; } - claimSandboxChangedLocks(metadata, noteLabel); - run('git', ['-C', metadata.worktreePath, 'add', '-A'], { timeout: 20_000 }); + claimDoctorChangedLocks(metadata); + run( + 'git', + ['-C', metadata.worktreePath, 'add', '-A', '--', '.', `:(exclude)${LOCK_FILE_RELATIVE}`], + { timeout: 20_000 }, + ); + const forceAddPaths = collectDoctorForceAddPaths(metadata.worktreePath); + if (forceAddPaths.length > 0) { + run( + 'git', + ['-C', metadata.worktreePath, 'add', '-f', '--', ...forceAddPaths], + { timeout: 20_000 }, + ); + } const staged = run( 'git', ['-C', metadata.worktreePath, 'diff', '--cached', '--name-only', '--', '.', `:(exclude)${LOCK_FILE_RELATIVE}`], @@ -1674,19 +1690,19 @@ function autoCommitSandboxChanges(metadata, { noteLabel, commitMessage }) { if (stagedFiles.length === 0) { return { status: 'no-changes', - note: `no committable ${noteLabel} changes found in sandbox`, + note: 'no committable doctor changes found in sandbox', }; } const commitResult = run( 'git', - ['-C', metadata.worktreePath, 'commit', '-m', commitMessage], + ['-C', metadata.worktreePath, 'commit', '-m', 'Auto-finish: gx doctor repairs'], { timeout: 30_000 }, ); if (commitResult.status !== 0) { return { status: 'failed', - note: `${noteLabel} sandbox auto-commit failed`, + note: 'doctor sandbox auto-commit failed', stdout: commitResult.stdout || '', stderr: commitResult.stderr || '', }; @@ -1694,19 +1710,12 @@ function autoCommitSandboxChanges(metadata, { noteLabel, commitMessage }) { return { status: 'committed', - note: `${noteLabel} sandbox changes committed`, - commitMessage, + note: 'doctor sandbox repairs committed', + commitMessage: 'Auto-finish: gx doctor repairs', stagedFiles, }; } -function autoCommitDoctorSandboxChanges(metadata) { - return autoCommitSandboxChanges(metadata, { - noteLabel: 'doctor', - commitMessage: 'Auto-finish: gx doctor repairs', - }); -} - function hasOriginRemote(repoRoot) { return run('git', ['-C', repoRoot, 'remote', 'get-url', 'origin']).status === 0; } @@ -1728,7 +1737,7 @@ function extractAgentBranchFinishPrUrl(output) { return match ? match[1] : ''; } -function sandboxFinishFlowIsPending(output) { +function doctorFinishFlowIsPending(output) { return ( /\[agent-branch-finish\] PR merge not completed yet; leaving PR open\./.test(output) || /\[agent-branch-finish\] Merge pending review\/check policy\. Branch cleanup skipped for now\./.test(output) || @@ -1736,7 +1745,7 @@ function sandboxFinishFlowIsPending(output) { ); } -function finishProtectedBaseSandboxBranch(blocked, metadata, options = {}) { +function finishDoctorSandboxBranch(blocked, metadata, options = {}) { const finishScript = path.join(metadata.worktreePath, 'scripts', 'agent-branch-finish.sh'); if (!fs.existsSync(finishScript)) { return { @@ -1782,22 +1791,13 @@ function finishProtectedBaseSandboxBranch(blocked, metadata, options = {}) { const finishResult = run( 'bash', - [ - finishScript, - '--branch', - metadata.branch, - '--base', - blocked.branch, - '--via-pr', - waitForMergeArg, - ...(options.cleanup ? ['--cleanup'] : []), - ], + [finishScript, '--branch', metadata.branch, '--base', blocked.branch, '--via-pr', waitForMergeArg], { cwd: metadata.worktreePath, timeout: finishTimeoutMs }, ); if (isSpawnFailure(finishResult)) { return { status: 'failed', - note: `${options.noteLabel || 'sandbox'} finish flow errored`, + note: 'doctor sandbox finish flow errored', stdout: finishResult.stdout || '', stderr: finishResult.stderr || '', }; @@ -1805,14 +1805,14 @@ function finishProtectedBaseSandboxBranch(blocked, metadata, options = {}) { if (finishResult.status !== 0) { return { status: 'failed', - note: `${options.noteLabel || 'sandbox'} finish flow failed`, + note: 'doctor sandbox finish flow failed', stdout: finishResult.stdout || '', stderr: finishResult.stderr || '', }; } const combinedOutput = `${finishResult.stdout || ''}\n${finishResult.stderr || ''}`; - if (sandboxFinishFlowIsPending(combinedOutput)) { + if (doctorFinishFlowIsPending(combinedOutput)) { return { status: 'pending', note: 'PR created and waiting for merge policy/checks', @@ -1824,68 +1824,188 @@ function finishProtectedBaseSandboxBranch(blocked, metadata, options = {}) { return { status: 'completed', - note: `${options.noteLabel || 'sandbox'} finish flow completed`, + note: 'doctor sandbox finish flow completed', stdout: finishResult.stdout || '', stderr: finishResult.stderr || '', }; } -function finishDoctorSandboxBranch(blocked, metadata, options = {}) { - return finishProtectedBaseSandboxBranch(blocked, metadata, { - ...options, - noteLabel: options.noteLabel || 'doctor sandbox', - }); -} +function mergeDoctorSandboxRepairsBackToProtectedBase(options, blocked, metadata, autoCommitResult, finishResult) { + if (options.dryRun) { + return { + status: autoCommitResult.status === 'committed' ? 'would-merge' : 'skipped', + note: autoCommitResult.status === 'committed' + ? 'dry run: would fast-forward tracked doctor repairs into the protected base workspace' + : 'dry run skips tracked repair merge', + }; + } -function syncProtectedBaseDoctorRepairs(options, blocked) { - const trackedStatusBefore = options.dryRun ? new Map() : trackedStatusByPath(blocked.repoRoot); - const fixPayload = runFixInternal({ - ...options, - target: blocked.repoRoot, - allowProtectedBaseWrite: true, - }); - const revertedTrackedPaths = []; - if (!options.dryRun) { - const trackedStatusAfter = trackedStatusByPath(blocked.repoRoot); - for (const [filePath] of trackedStatusAfter.entries()) { - if (!trackedStatusBefore.has(filePath)) { - revertedTrackedPaths.push(filePath); + if (autoCommitResult.status !== 'committed') { + return { + status: autoCommitResult.status === 'no-changes' ? 'unchanged' : 'skipped', + note: autoCommitResult.status === 'no-changes' + ? 'no tracked doctor repairs needed in the protected base workspace' + : 'tracked doctor repair merge skipped', + }; + } + + if (finishResult.status !== 'skipped') { + return { + status: 'skipped', + note: finishResult.status === 'failed' + ? 'tracked doctor repairs remain in the sandbox after finish failure' + : 'tracked doctor repairs are being delivered through the sandbox finish flow', + }; + } + + const allowedPaths = new Set([ + ...(autoCommitResult.stagedFiles || []), + ...OMX_SCAFFOLD_DIRECTORIES, + ...Array.from(OMX_SCAFFOLD_FILES.keys()), + ...TEMPLATE_FILES.map((entry) => toDestinationPath(entry)), + 'bin', + 'package.json', + '.gitignore', + 'AGENTS.md', + ]); + const dirtyPaths = collectWorktreeDirtyPaths(blocked.repoRoot); + let stashRef = ''; + if (dirtyPaths.length > 0) { + const unexpectedPaths = dirtyPaths.filter((filePath) => { + if (allowedPaths.has(filePath)) { + return false; } + return !( + filePath === NESTED_REPO_WORKTREE_RELATIVE_DIR + || filePath.startsWith(`${NESTED_REPO_WORKTREE_RELATIVE_DIR}/`) + ); + }); + if (unexpectedPaths.length > 0) { + return { + status: 'failed', + note: `protected branch workspace has unrelated local changes: ${unexpectedPaths.join(', ')}`, + }; + } + const stashMessage = `guardex-doctor-merge-${Date.now()}`; + const stashResult = run( + 'git', + ['-C', blocked.repoRoot, 'stash', 'push', '--all', '--message', stashMessage], + { timeout: 30_000 }, + ); + if (isSpawnFailure(stashResult)) { + return { + status: 'failed', + note: 'could not stash protected branch doctor drift before merge', + stdout: stashResult.stdout || '', + stderr: stashResult.stderr || '', + }; + } + if (stashResult.status !== 0) { + return { + status: 'failed', + note: 'stashing protected branch doctor drift failed', + stdout: stashResult.stdout || '', + stderr: stashResult.stderr || '', + }; } - restoreTrackedWorktreePaths(blocked.repoRoot, revertedTrackedPaths); + + const stashLookup = run( + 'git', + ['-C', blocked.repoRoot, 'stash', 'list'], + { timeout: 20_000 }, + ); + stashRef = String(stashLookup.stdout || '') + .split('\n') + .find((line) => line.includes(stashMessage)) + ?.split(':')[0] + ?.trim() || ''; } - const revertedTrackedSet = new Set(revertedTrackedPaths); - const changedOperations = fixPayload.operations.filter( - (operation) => - !['unchanged', 'skipped'].includes(operation.status) && - !revertedTrackedSet.has(operation.file), + const restoreResult = ensureRepoBranch(blocked.repoRoot, blocked.branch); + if (!restoreResult.ok) { + if (stashRef) { + run('git', ['-C', blocked.repoRoot, 'stash', 'apply', stashRef], { timeout: 30_000 }); + } + return { + status: 'failed', + note: `could not restore protected branch '${blocked.branch}' before applying sandbox repairs`, + stdout: restoreResult.stdout || '', + stderr: restoreResult.stderr || '', + }; + } + + const mergeResult = run( + 'git', + ['-C', blocked.repoRoot, 'merge', '--ff-only', metadata.branch], + { timeout: 30_000 }, ); - const hookChanged = fixPayload.hookResult?.status && fixPayload.hookResult.status !== 'unchanged'; - const changedCount = changedOperations.length + (hookChanged ? 1 : 0); + if (isSpawnFailure(mergeResult)) { + if (stashRef) { + run('git', ['-C', blocked.repoRoot, 'stash', 'apply', stashRef], { timeout: 30_000 }); + } + return { + status: 'failed', + note: 'tracked doctor repair merge errored', + stdout: mergeResult.stdout || '', + stderr: mergeResult.stderr || '', + }; + } + if (mergeResult.status !== 0) { + if (stashRef) { + run('git', ['-C', blocked.repoRoot, 'stash', 'apply', stashRef], { timeout: 30_000 }); + } + return { + status: 'failed', + note: 'tracked doctor repair merge failed', + stdout: mergeResult.stdout || '', + stderr: mergeResult.stderr || '', + }; + } - if (changedCount === 0) { + let cleanupResult; + try { + cleanupResult = cleanupProtectedBaseSandbox(blocked.repoRoot, metadata); + } catch (error) { return { - status: 'unchanged', - note: revertedTrackedPaths.length > 0 - ? 'only tracked protected-branch files changed and were restored' - : 'managed repair files already aligned in protected branch workspace', - fixPayload, - revertedTrackedPaths, + status: 'failed', + note: `tracked doctor repair merge succeeded but sandbox cleanup failed: ${error.message}`, + stdout: mergeResult.stdout || '', + stderr: mergeResult.stderr || '', }; } - const revertedNote = revertedTrackedPaths.length > 0 - ? `; restored ${revertedTrackedPaths.length} tracked file(s) to keep the protected checkout clean` - : ''; + let hookRefreshResult; + try { + hookRefreshResult = configureHooks(blocked.repoRoot, false); + } catch (error) { + return { + status: 'failed', + note: `tracked doctor repair merge succeeded but local hook refresh failed: ${error.message}`, + stdout: mergeResult.stdout || '', + stderr: mergeResult.stderr || '', + }; + } + + if (stashRef) { + run('git', ['-C', blocked.repoRoot, 'stash', 'drop', stashRef], { timeout: 20_000 }); + } + return { - status: options.dryRun ? 'would-sync' : 'synced', - note: `${options.dryRun ? 'would sync' : 'synced'} ${changedCount} managed repair item(s)${revertedNote}`, - fixPayload, - revertedTrackedPaths, + status: 'merged', + note: 'fast-forwarded tracked doctor repairs into the protected base workspace', + stdout: mergeResult.stdout || '', + stderr: mergeResult.stderr || '', + cleanup: cleanupResult, + hookRefresh: hookRefreshResult, }; } +function syncDoctorLocalSupportFiles(repoRoot, dryRun) { + return TEMPLATE_FILES + .filter((entry) => entry.startsWith('codex/') || entry.startsWith('claude/')) + .map((entry) => ensureTemplateFilePresent(repoRoot, entry, dryRun)); +} + function runDoctorInSandbox(options, blocked) { const startResult = startProtectedBaseSandbox(blocked, { taskName: `${SHORT_TOOL_NAME}-doctor`, @@ -1920,6 +2040,7 @@ function runDoctorInSandbox(options, blocked) { status: 'skipped', note: 'sandbox doctor did not complete successfully', }; + let sandboxLockContent = null; let postSandboxAutoFinishSummary = { enabled: false, attempted: 0, @@ -1933,7 +2054,6 @@ function runDoctorInSandbox(options, blocked) { note: 'sandbox doctor did not complete successfully', }; if (nestedResult.status === 0) { - protectedBaseRepairSyncResult = syncProtectedBaseDoctorRepairs(options, blocked); const omxScaffoldOps = ensureOmxScaffold(blocked.repoRoot, Boolean(options.dryRun)); const changedOmxPaths = omxScaffoldOps.filter((operation) => operation.status !== 'unchanged'); if (changedOmxPaths.length === 0) { @@ -1989,7 +2109,11 @@ function runDoctorInSandbox(options, blocked) { note: `${LOCK_FILE_RELATIVE} missing in sandbox worktree`, }; } else { - const sourceContent = fs.readFileSync(sandboxLockPath, 'utf8'); + const sourceContent = stripDoctorSandboxLocks( + fs.readFileSync(sandboxLockPath, 'utf8'), + metadata.branch, + ); + sandboxLockContent = sourceContent; const destinationContent = fs.readFileSync(baseLockPath, 'utf8'); if (sourceContent === destinationContent) { lockSyncResult = { @@ -2006,6 +2130,62 @@ function runDoctorInSandbox(options, blocked) { } } + protectedBaseRepairSyncResult = mergeDoctorSandboxRepairsBackToProtectedBase( + options, + blocked, + metadata, + autoCommitResult, + finishResult, + ); + + syncDoctorLocalSupportFiles(blocked.repoRoot, Boolean(options.dryRun)); + + const postMergeOmxScaffoldOps = ensureOmxScaffold(blocked.repoRoot, Boolean(options.dryRun)); + const postMergeChangedOmxPaths = postMergeOmxScaffoldOps.filter((operation) => operation.status !== 'unchanged'); + if (postMergeChangedOmxPaths.length === 0) { + omxScaffoldSyncResult = { + status: 'unchanged', + note: '.omx scaffold already in sync', + operations: postMergeOmxScaffoldOps, + }; + } else { + omxScaffoldSyncResult = { + status: options.dryRun ? 'would-sync' : 'synced', + note: `${options.dryRun ? 'would sync' : 'synced'} ${postMergeChangedOmxPaths.length} .omx path(s)`, + operations: postMergeOmxScaffoldOps, + }; + } + + const postMergeBaseLockPath = path.join(blocked.repoRoot, LOCK_FILE_RELATIVE); + if (sandboxLockContent === null) { + lockSyncResult = { + status: 'skipped', + note: `${LOCK_FILE_RELATIVE} missing in sandbox worktree`, + }; + } else if (!fs.existsSync(postMergeBaseLockPath)) { + fs.mkdirSync(path.dirname(postMergeBaseLockPath), { recursive: true }); + fs.writeFileSync(postMergeBaseLockPath, sandboxLockContent, 'utf8'); + lockSyncResult = { + status: 'synced', + note: `${LOCK_FILE_RELATIVE} recreated from sandbox`, + }; + } else { + const destinationContent = fs.readFileSync(postMergeBaseLockPath, 'utf8'); + if (sandboxLockContent === destinationContent) { + lockSyncResult = { + status: 'unchanged', + note: `${LOCK_FILE_RELATIVE} already in sync`, + }; + } else { + fs.mkdirSync(path.dirname(postMergeBaseLockPath), { recursive: true }); + fs.writeFileSync(postMergeBaseLockPath, sandboxLockContent, 'utf8'); + lockSyncResult = { + status: 'synced', + note: `${LOCK_FILE_RELATIVE} synced from sandbox`, + }; + } + } + postSandboxAutoFinishSummary = autoFinishReadyAgentBranches(blocked.repoRoot, { baseBranch: blocked.branch, dryRun: options.dryRun, @@ -2063,14 +2243,28 @@ function runDoctorInSandbox(options, blocked) { console.log(`[${TOOL_NAME}] Doctor sandbox auto-commit skipped: ${autoCommitResult.note}.`); } - if (protectedBaseRepairSyncResult.status === 'synced') { - console.log(`[${TOOL_NAME}] Synced repaired managed files back to protected branch workspace.`); + if (protectedBaseRepairSyncResult.status === 'merged') { + console.log(`[${TOOL_NAME}] Fast-forwarded tracked doctor repairs into the protected branch workspace.`); } else if (protectedBaseRepairSyncResult.status === 'unchanged') { - console.log(`[${TOOL_NAME}] Protected branch workspace already had the repaired managed files.`); - } else if (protectedBaseRepairSyncResult.status === 'would-sync') { - console.log(`[${TOOL_NAME}] Dry run: would sync repaired managed files back to protected branch workspace.`); + console.log(`[${TOOL_NAME}] Protected branch workspace already had the tracked doctor repairs.`); + } else if (protectedBaseRepairSyncResult.status === 'would-merge') { + console.log(`[${TOOL_NAME}] Dry run: would fast-forward tracked doctor repairs into the protected branch workspace.`); + } else if (protectedBaseRepairSyncResult.status === 'failed') { + console.log(`[${TOOL_NAME}] Protected branch tracked repair merge failed: ${protectedBaseRepairSyncResult.note}.`); + if (protectedBaseRepairSyncResult.stdout) process.stdout.write(protectedBaseRepairSyncResult.stdout); + if (protectedBaseRepairSyncResult.stderr) process.stderr.write(protectedBaseRepairSyncResult.stderr); + } else { + console.log(`[${TOOL_NAME}] Protected branch tracked repair merge skipped: ${protectedBaseRepairSyncResult.note}.`); + } + + if (lockSyncResult.status === 'synced') { + console.log( + `[${TOOL_NAME}] Synced repaired lock registry back to protected branch workspace (${LOCK_FILE_RELATIVE}).`, + ); + } else if (lockSyncResult.status === 'unchanged') { + console.log(`[${TOOL_NAME}] Lock registry already synced in protected branch workspace.`); } else { - console.log(`[${TOOL_NAME}] Protected branch workspace repair sync skipped: ${protectedBaseRepairSyncResult.note}.`); + console.log(`[${TOOL_NAME}] Lock registry sync skipped: ${lockSyncResult.note}.`); } if (finishResult.status === 'completed') { @@ -2088,22 +2282,13 @@ function runDoctorInSandbox(options, blocked) { if (finishResult.stderr) process.stderr.write(finishResult.stderr); } else if (finishResult.status === 'failed') { console.log(`[${TOOL_NAME}] Auto-finish flow failed for sandbox branch '${metadata.branch}'.`); + console.log(`[guardex] Auto-finish flow failed for sandbox branch '${metadata.branch}'.`); if (finishResult.stdout) process.stdout.write(finishResult.stdout); if (finishResult.stderr) process.stderr.write(finishResult.stderr); } else { console.log(`[${TOOL_NAME}] Auto-finish skipped: ${finishResult.note}.`); } - if (lockSyncResult.status === 'synced') { - console.log( - `[${TOOL_NAME}] Synced repaired lock registry back to protected branch workspace (${LOCK_FILE_RELATIVE}).`, - ); - } else if (lockSyncResult.status === 'unchanged') { - console.log(`[${TOOL_NAME}] Lock registry already synced in protected branch workspace.`); - } else { - console.log(`[${TOOL_NAME}] Lock registry sync skipped: ${lockSyncResult.note}.`); - } - if (postSandboxAutoFinishSummary.enabled) { console.log( `[${TOOL_NAME}] Auto-finish sweep (base=${blocked.branch}): attempted=${postSandboxAutoFinishSummary.attempted}, completed=${postSandboxAutoFinishSummary.completed}, skipped=${postSandboxAutoFinishSummary.skipped}, failed=${postSandboxAutoFinishSummary.failed}`, @@ -2138,6 +2323,9 @@ function runDoctorInSandbox(options, blocked) { ) { exitCode = 1; } + if (exitCode === 0 && protectedBaseRepairSyncResult.status === 'failed') { + exitCode = 1; + } process.exitCode = exitCode; return; } @@ -4819,12 +5007,7 @@ function install(rawArgs) { allowProtectedBaseWrite: false, }); - const blocked = protectedBaseWriteBlock(options, { requireBootstrap: false }); - if (blocked) { - runProtectedBaseMaintenanceInSandbox('install', options, blocked); - process.exitCode = 0; - return; - } + assertProtectedMainWriteAllowed(options, 'install'); const payload = runInstallInternal(options); printOperations('Install target', payload, options.dryRun); @@ -4856,12 +5039,7 @@ function fix(rawArgs) { allowProtectedBaseWrite: false, }); - const blocked = protectedBaseWriteBlock(options, { requireBootstrap: false }); - if (blocked) { - runProtectedBaseMaintenanceInSandbox('fix', options, blocked); - process.exitCode = 0; - return; - } + assertProtectedMainWriteAllowed(options, 'fix'); const payload = runFixInternal(options); printOperations('Fix target', payload, options.dryRun); @@ -5050,7 +5228,7 @@ function doctor(rawArgs) { return; } - printOperations('Doctor/fix', fixPayload, singleRepoOptions.dryRun); + printOperations('Doctor/fix', fixPayload, options.dryRun); printScanResult(scanResult, false); if (scanResult.guardexEnabled === false) { console.log(`[${TOOL_NAME}] Repo-local Guardex enforcement is intentionally disabled.`); @@ -5077,140 +5255,6 @@ function doctor(rawArgs) { setExitCodeFromScan(scanResult); } -function buildSandboxArgs(commandName, options, sandboxTarget) { - if (commandName === 'setup') { - return buildSandboxSetupArgs(options, sandboxTarget); - } - if (commandName === 'install') { - return buildSandboxInstallArgs(options, sandboxTarget); - } - if (commandName === 'fix') { - return buildSandboxFixArgs(options, sandboxTarget); - } - throw new Error(`Unsupported sandbox command: ${commandName}`); -} - -function sandboxCommitMessage(commandName) { - if (commandName === 'setup') return 'Auto-finish: gx setup bootstrap'; - if (commandName === 'install') return 'Auto-finish: gx install bootstrap'; - if (commandName === 'fix') return 'Auto-finish: gx fix repairs'; - throw new Error(`Unsupported sandbox command: ${commandName}`); -} - -function runProtectedBaseMaintenanceInSandbox(commandName, options, blocked) { - const startResult = startProtectedBaseSandbox(blocked, { - taskName: `${SHORT_TOOL_NAME}-${commandName}`, - sandboxSuffix: `gx-${commandName}`, - }); - const metadata = startResult.metadata; - - console.log( - `[${TOOL_NAME}] ${commandName} detected protected branch '${blocked.branch}'. ` + - `Running in sandbox branch '${metadata.branch || 'agent/'}'.`, - ); - if (startResult.stdout) process.stdout.write(startResult.stdout); - if (startResult.stderr) process.stderr.write(startResult.stderr); - - const sandboxTarget = resolveSandboxTarget(blocked.repoRoot, metadata.worktreePath, options.target); - const nestedResult = run( - process.execPath, - [__filename, ...buildSandboxArgs(commandName, options, sandboxTarget)], - { cwd: metadata.worktreePath }, - ); - if (isSpawnFailure(nestedResult)) { - throw nestedResult.error; - } - if (nestedResult.stdout) process.stdout.write(nestedResult.stdout); - if (nestedResult.stderr) process.stderr.write(nestedResult.stderr); - if (nestedResult.status !== 0) { - throw new Error( - `sandboxed ${commandName} failed for protected branch '${blocked.branch}'. ` + - `Inspect sandbox at ${metadata.worktreePath}`, - ); - } - - let autoCommitResult = { - status: 'skipped', - note: `${commandName} sandbox did not complete successfully`, - }; - let finishResult = { - status: 'skipped', - note: `${commandName} sandbox did not complete successfully`, - }; - let cleanupResult = { - status: 'skipped', - note: `${commandName} sandbox kept for follow-up`, - }; - - if (options.dryRun) { - cleanupResult = cleanupProtectedBaseSandbox(blocked.repoRoot, metadata); - autoCommitResult = { - status: 'skipped', - note: 'dry-run skips sandbox auto-commit', - }; - finishResult = { - status: 'skipped', - note: 'dry-run skips sandbox finish flow', - }; - } else { - autoCommitResult = autoCommitSandboxChanges(metadata, { - noteLabel: commandName, - commitMessage: sandboxCommitMessage(commandName), - }); - if (autoCommitResult.status === 'committed') { - finishResult = finishProtectedBaseSandboxBranch(blocked, metadata, { - cleanup: true, - noteLabel: `${commandName} sandbox`, - }); - if (finishResult.status === 'completed') { - cleanupResult = cleanupProtectedBaseSandbox(blocked.repoRoot, metadata); - } else { - cleanupResult = { - status: 'kept', - note: 'sandbox branch/worktree kept for follow-up', - }; - } - } else if (autoCommitResult.status === 'no-changes') { - cleanupResult = cleanupProtectedBaseSandbox(blocked.repoRoot, metadata); - finishResult = { - status: 'skipped', - note: 'no sandbox changes to finish', - }; - } else if (autoCommitResult.status === 'failed') { - cleanupResult = { - status: 'kept', - note: 'sandbox branch/worktree kept because auto-commit failed', - }; - } - } - - if (autoCommitResult.status === 'committed') { - console.log(`[${TOOL_NAME}] Auto-committed ${commandName} changes in sandbox branch '${metadata.branch}'.`); - } else if (autoCommitResult.status === 'failed') { - console.log(`[${TOOL_NAME}] Sandbox auto-commit failed for ${commandName}; branch left for manual follow-up.`); - } else if (autoCommitResult.status === 'no-changes') { - console.log(`[${TOOL_NAME}] Sandbox ${commandName} produced no file changes to keep.`); - } else if (autoCommitResult.note) { - console.log(`[${TOOL_NAME}] Sandbox auto-commit skipped: ${autoCommitResult.note}.`); - } - - if (finishResult.status === 'completed') { - console.log(`[${TOOL_NAME}] Auto-finish flow completed for sandbox branch '${metadata.branch}'.`); - } else if (finishResult.status === 'pending') { - console.log( - `[${TOOL_NAME}] Auto-finish pending for sandbox branch '${metadata.branch}': ${finishResult.note}.`, - ); - } else if (finishResult.status === 'failed') { - console.log(`[${TOOL_NAME}] Auto-finish flow failed for sandbox branch '${metadata.branch}'.`); - } else if (finishResult.note) { - console.log(`[${TOOL_NAME}] Sandbox finish flow skipped: ${finishResult.note}.`); - } - - if (cleanupResult.note) { - console.log(`[${TOOL_NAME}] Sandbox cleanup: ${cleanupResult.note}.`); - } -} - function review(rawArgs) { const options = parseReviewArgs(rawArgs); const repoRoot = resolveRepoRoot(options.target); @@ -5684,7 +5728,6 @@ function setup(rawArgs) { let aggregateErrors = 0; let aggregateWarnings = 0; let lastScanResult = null; - let sandboxedRepoCount = 0; for (const repoPath of discoveredRepos) { const perRepoOptions = { ...options, target: repoPath }; @@ -5694,10 +5737,12 @@ function setup(rawArgs) { console.log(`[${TOOL_NAME}] ── Setup target: ${repoPath} ──`); } - const blocked = protectedBaseWriteBlock(perRepoOptions, { requireBootstrap: false }); + const blocked = protectedBaseWriteBlock(perRepoOptions); if (blocked) { - runProtectedBaseMaintenanceInSandbox('setup', perRepoOptions, blocked); - sandboxedRepoCount += 1; + const sandboxResult = runSetupInSandbox(perRepoOptions, blocked, repoLabel); + aggregateErrors += sandboxResult.scanResult.errors; + aggregateWarnings += sandboxResult.scanResult.warnings; + lastScanResult = sandboxResult.scanResult; continue; } @@ -5747,14 +5792,6 @@ function setup(rawArgs) { const repoCount = discoveredRepos.length; const suffix = repoCount > 1 ? ` (${repoCount} repos)` : ''; console.log(`[${TOOL_NAME}] ✅ Setup complete.${suffix}`); - if (sandboxedRepoCount > 0) { - console.log( - `[${TOOL_NAME}] Protected-base setup ran through sandbox branches so visible base checkouts stayed unchanged.`, - ); - console.log( - `[${TOOL_NAME}] If auto-finish was skipped or is pending, continue from the printed sandbox branch/worktree path.`, - ); - } console.log(`[${TOOL_NAME}] Copy AI setup prompt with: ${SHORT_TOOL_NAME} prompt`); console.log( `[${TOOL_NAME}] OpenSpec core workflow: /opsx:propose -> /opsx:apply -> /opsx:archive`, @@ -6623,6 +6660,7 @@ function main() { } if (command === '--version' || command === '-v' || command === 'version') { + maybeSelfUpdateBeforeStatus(); console.log(packageJson.version); return; } diff --git a/openspec/changes/fix-regression-finish-flows/.openspec.yaml b/openspec/changes/fix-regression-finish-flows/.openspec.yaml new file mode 100644 index 0000000..4b8c565 --- /dev/null +++ b/openspec/changes/fix-regression-finish-flows/.openspec.yaml @@ -0,0 +1,2 @@ +schema: spec-driven +created: 2026-04-21 diff --git a/openspec/changes/fix-regression-finish-flows/proposal.md b/openspec/changes/fix-regression-finish-flows/proposal.md new file mode 100644 index 0000000..5f06dcb --- /dev/null +++ b/openspec/changes/fix-regression-finish-flows/proposal.md @@ -0,0 +1,24 @@ +## Why + +- The Guardex CLI still had a few workflow regressions after the recent protected-main and finish-flow fixes landed on `main`. +- `agent-branch-finish.sh` could still fall back to `dev` in repos that only expose `main`, which makes cleanup and merge automation choose the wrong base. +- `agent-branch-start.sh` still collapsed explicit agent roles to `codex`, which hid real lane ownership in branch names and made the runtime/test surface drift from the intended role-based contract. +- `codex-agent.sh` still attempted PR auto-finish in local/file-remote repos that do not expose a mergeable GitHub PR surface, leaving the session on a finish path that could never succeed. + +## What Changes + +- Teach `agent-branch-finish.sh` and its template to fall back through `dev`, `main`, and `master` before defaulting so main-only repos finish against a real base branch. +- Preserve explicit role tokens in `agent-branch-start.sh` and its template, while keeping the legacy `claude`, `codex`, and `bot -> codex` compatibility paths intact. +- Gate `codex-agent.sh` auto-finish on a real mergeable remote context and refresh focused regression coverage in `test/install.test.js` and `scripts/test-agent-naming.sh` so the suite matches the current Guardex workflow contract. + +## Impact + +- Affected runtime surfaces: + - `scripts/agent-branch-finish.sh` + - `scripts/agent-branch-start.sh` + - `scripts/codex-agent.sh` + - matching templates under `templates/scripts/` +- Affected regression coverage: + - `test/install.test.js` + - `scripts/test-agent-naming.sh` +- Risk is moderate because the patch touches branch creation, finish, and auto-finish orchestration, but the blast radius stays inside Guardex workflow scripts and their regression suite. diff --git a/openspec/changes/fix-regression-finish-flows/specs/workflow-guardrails/spec.md b/openspec/changes/fix-regression-finish-flows/specs/workflow-guardrails/spec.md new file mode 100644 index 0000000..dc78325 --- /dev/null +++ b/openspec/changes/fix-regression-finish-flows/specs/workflow-guardrails/spec.md @@ -0,0 +1,31 @@ +## ADDED Requirements + +### Requirement: finish flow chooses a real base branch +Guardex SHALL finish agent branches against an available base branch even when no explicit base metadata is stored on the source branch. + +#### Scenario: main-only repo without stored base metadata +- **GIVEN** an agent branch is being finished +- **AND** the branch does not have `branch..guardexBase` metadata +- **AND** the repo exposes `main` but not `dev` +- **WHEN** `scripts/agent-branch-finish.sh` resolves the base branch +- **THEN** it SHALL select `main` +- **AND** it SHALL not fall through to a non-existent `dev` base. + +### Requirement: explicit agent roles stay visible in sandbox names +Guardex SHALL preserve explicit agent role tokens in branch/worktree naming while keeping legacy compatibility aliases for the common `codex`, `claude`, and `bot` flows. + +#### Scenario: explicit planner role requested +- **GIVEN** `scripts/agent-branch-start.sh` is invoked with an explicit role such as `planner` +- **WHEN** the branch name is normalized +- **THEN** the emitted branch/worktree name SHALL keep the explicit sanitized role token +- **AND** legacy `bot` inputs SHALL still collapse to `codex`. + +### Requirement: codex-agent auto-finish requires mergeable remote context +Guardex SHALL skip the PR auto-finish path when the current repo does not expose a mergeable GitHub-backed remote context. + +#### Scenario: local or file-backed origin remote +- **GIVEN** `scripts/codex-agent.sh` finishes a successful task run +- **AND** the repo `origin` resolves to a local path or `file://` URL, or `gh` auth is not usable +- **WHEN** auto-finish evaluation runs +- **THEN** Guardex SHALL skip the PR merge/wait flow +- **AND** it SHALL keep the sandbox branch/worktree available for manual follow-up instead of waiting for merge. diff --git a/openspec/changes/fix-regression-finish-flows/tasks.md b/openspec/changes/fix-regression-finish-flows/tasks.md new file mode 100644 index 0000000..29b80d7 --- /dev/null +++ b/openspec/changes/fix-regression-finish-flows/tasks.md @@ -0,0 +1,21 @@ +## 1. Specification + +- [x] 1.1 Finalize proposal scope and acceptance criteria for `fix-regression-finish-flows`. +- [x] 1.2 Define normative requirements in `specs/workflow-guardrails/spec.md`. + +## 2. Implementation + +- [x] 2.1 Implement scoped behavior changes. +- [x] 2.2 Add/update focused regression coverage. + +## 3. Verification + +- [x] 3.1 Run targeted project verification commands. +- [x] 3.2 Run `openspec validate fix-regression-finish-flows --type change --strict`. +- [x] 3.3 Run `openspec validate --specs`. + +## 4. Completion + +- [ ] 4.1 Finish the agent branch via PR merge + cleanup (`gx finish --via-pr --wait-for-merge --cleanup` or `bash scripts/agent-branch-finish.sh --branch --base --via-pr --wait-for-merge --cleanup`). +- [ ] 4.2 Record PR URL + final `MERGED` state in the completion handoff. +- [ ] 4.3 Confirm sandbox cleanup (`git worktree list`, `git branch -a`) or capture a `BLOCKED:` handoff if merge/cleanup is pending. diff --git a/scripts/agent-branch-finish.sh b/scripts/agent-branch-finish.sh index 5512851..95ee231 100755 --- a/scripts/agent-branch-finish.sh +++ b/scripts/agent-branch-finish.sh @@ -156,9 +156,9 @@ if [[ "$BASE_BRANCH_EXPLICIT" -eq 1 && -z "$BASE_BRANCH" ]]; then fi if [[ "$BASE_BRANCH_EXPLICIT" -eq 0 ]]; then - stored_branch_base="$(git -C "$repo_root" config --get "branch.${SOURCE_BRANCH}.guardexBase" || true)" - if [[ -n "$stored_branch_base" ]]; then - BASE_BRANCH="$stored_branch_base" + source_branch_base="$(git -C "$repo_root" config --get "branch.${SOURCE_BRANCH}.guardexBase" || true)" + if [[ -n "$source_branch_base" ]]; then + BASE_BRANCH="$source_branch_base" else configured_base="$(git -C "$repo_root" config --get multiagent.baseBranch || true)" if [[ -n "$configured_base" ]]; then @@ -167,6 +167,16 @@ if [[ "$BASE_BRANCH_EXPLICIT" -eq 0 ]]; then fi fi +if [[ -z "$BASE_BRANCH" ]]; then + for fallback_branch in dev main master; do + if git -C "$repo_root" show-ref --verify --quiet "refs/heads/${fallback_branch}" \ + || git -C "$repo_root" show-ref --verify --quiet "refs/remotes/origin/${fallback_branch}"; then + BASE_BRANCH="$fallback_branch" + break + fi + done +fi + if [[ -z "$BASE_BRANCH" ]]; then BASE_BRANCH="dev" fi @@ -273,8 +283,17 @@ if [[ "$should_require_sync" -eq 1 ]] && git -C "$repo_root" show-ref --verify - fi fi -integration_worktree="${agent_worktree_root}/__integrate-${BASE_BRANCH//\//__}-$(date +%Y%m%d-%H%M%S)" -integration_branch="__agent_integrate_${BASE_BRANCH//\//_}_$(date +%Y%m%d_%H%M%S)" +integration_stamp="$(date +%Y%m%d-%H%M%S)" +integration_worktree_base="${agent_worktree_root}/__integrate-${BASE_BRANCH//\//__}-${integration_stamp}" +integration_branch_base="__agent_integrate_${BASE_BRANCH//\//_}_$(date +%Y%m%d_%H%M%S)" +integration_worktree="$integration_worktree_base" +integration_branch="$integration_branch_base" +integration_suffix=1 +while [[ -e "$integration_worktree" ]] || git -C "$repo_root" show-ref --verify --quiet "refs/heads/${integration_branch}"; do + integration_worktree="${integration_worktree_base}-${integration_suffix}" + integration_branch="${integration_branch_base}_${integration_suffix}" + integration_suffix=$((integration_suffix + 1)) +done mkdir -p "$(dirname "$integration_worktree")" git -C "$repo_root" worktree add "$integration_worktree" "$start_ref" >/dev/null diff --git a/scripts/agent-branch-start.sh b/scripts/agent-branch-start.sh index d763372..e56e376 100755 --- a/scripts/agent-branch-start.sh +++ b/scripts/agent-branch-start.sh @@ -123,12 +123,11 @@ shorten_slug() { printf '%s' "$shortened" } -# Collapse arbitrary agent identifiers to a clean role token: claude | codex | -# . Priority: GUARDEX_AGENT_TYPE env override, then the raw -# AGENT_NAME (if it contains 'claude' or 'codex'), then CLAUDECODE=1 sentinel -# (set by Claude Code CLI), else fall back to 'codex'. Any other role name -# (integrator, executor, rust-port, etc.) is preserved as-is after slug -# sanitization. +# Collapse arbitrary agent identifiers to a clean role token. Priority: +# GUARDEX_AGENT_TYPE env override, then recognizable claude/codex aliases, then +# a small legacy compatibility set, then the literal requested role after slug +# sanitization. This preserves explicit roles such as planner/executor while +# keeping the older bot -> codex fallback stable for existing callers. normalize_role() { local raw_agent="$1" local override="${GUARDEX_AGENT_TYPE:-}" @@ -150,10 +149,13 @@ normalize_role() { printf 'claude' return 0 fi - # Unrecognized raw name (rust-port-lead, some-worker, empty, ...): default to - # codex. To get a different role (integrator, executor, ...) pass the role - # explicitly via GUARDEX_AGENT_TYPE, handled above. - printf 'codex' + local sanitized + sanitized="$(sanitize_slug "$raw_agent" "codex")" + if [[ "$sanitized" == "bot" ]]; then + printf 'codex' + return 0 + fi + printf '%s' "$sanitized" } # Timestamp the branch/worktree/openspec slug so parallel agents never collide diff --git a/scripts/codex-agent.sh b/scripts/codex-agent.sh index b669288..e9e4166 100755 --- a/scripts/codex-agent.sh +++ b/scripts/codex-agent.sh @@ -249,6 +249,35 @@ resolve_start_ref() { return 1 } +origin_remote_looks_like_github() { + local wt="$1" + local origin_url="" + origin_url="$(git -C "$wt" remote get-url origin 2>/dev/null || true)" + [[ -n "$origin_url" && "$origin_url" =~ github\.com[:/] ]] +} + +auto_finish_context_is_ready() { + local wt="$1" + local gh_bin="${GUARDEX_GH_BIN:-gh}" + + if ! git -C "$wt" remote get-url origin >/dev/null 2>&1; then + return 1 + fi + if ! command -v "$gh_bin" >/dev/null 2>&1; then + return 1 + fi + + if [[ -n "${GUARDEX_GH_BIN:-}" ]]; then + return 0 + fi + + if ! origin_remote_looks_like_github "$wt"; then + return 1 + fi + + "$gh_bin" auth status >/dev/null 2>&1 +} + restore_repo_branch_if_changed() { local expected_branch="$1" if [[ -z "$expected_branch" || "$expected_branch" == "HEAD" ]]; then @@ -780,7 +809,9 @@ if [[ "$AUTO_FINISH" -eq 1 && -n "$worktree_branch" && "$worktree_branch" != "HE else echo "[codex-agent] Auto-finish enabled: commit -> push/PR -> merge (keep branch/worktree)." fi - if auto_commit_worktree_changes "$worktree_path" "$worktree_branch"; then + if ! auto_finish_context_is_ready "$worktree_path"; then + echo "[codex-agent] Auto-finish skipped for '${worktree_branch}' (no mergeable remote context)." >&2 + elif auto_commit_worktree_changes "$worktree_path" "$worktree_branch"; then if run_finish_flow "$worktree_path" "$worktree_branch"; then auto_finish_completed=1 echo "[codex-agent] Auto-finish completed for '${worktree_branch}'." diff --git a/scripts/test-agent-naming.sh b/scripts/test-agent-naming.sh index 86cfa4b..4c62233 100755 --- a/scripts/test-agent-naming.sh +++ b/scripts/test-agent-naming.sh @@ -5,8 +5,9 @@ # agent//-- # # Where: -# - role ∈ {claude, codex} for the common case, or any sanitized role token -# (integrator, executor, rust-port, ...) when passed via GUARDEX_AGENT_TYPE. +# - role ∈ {claude, codex} for the common case, or any sanitized explicit role +# token (integrator, executor, rust-port, ...) when passed directly or via +# GUARDEX_AGENT_TYPE. The legacy name "bot" still falls back to codex. # - task-slug is the user-provided task name, lowercased + kebab-cased. # - timestamp is local YYYY-MM-DD-HH-MM; colons are forbidden in git refs, so # the HH:MM the user sees is stored as HH-MM in the slug. @@ -101,8 +102,8 @@ assert_eq "neutral name + CLAUDECODE=1 → claude" \ "$actual" "agent/claude/task4-${STAMP}" actual="$(run_name_only task5 rust-port-lead)" -assert_eq "neutral name + no env → codex default" \ - "$actual" "agent/codex/task5-${STAMP}" +assert_eq "neutral explicit name stays preserved" \ + "$actual" "agent/rust-port-lead/task5-${STAMP}" actual="$(run_name_only task6 claude GUARDEX_AGENT_TYPE=codex)" assert_eq "GUARDEX_AGENT_TYPE=codex overrides claude arg" \ diff --git a/templates/scripts/agent-branch-finish.sh b/templates/scripts/agent-branch-finish.sh index 5512851..95ee231 100755 --- a/templates/scripts/agent-branch-finish.sh +++ b/templates/scripts/agent-branch-finish.sh @@ -156,9 +156,9 @@ if [[ "$BASE_BRANCH_EXPLICIT" -eq 1 && -z "$BASE_BRANCH" ]]; then fi if [[ "$BASE_BRANCH_EXPLICIT" -eq 0 ]]; then - stored_branch_base="$(git -C "$repo_root" config --get "branch.${SOURCE_BRANCH}.guardexBase" || true)" - if [[ -n "$stored_branch_base" ]]; then - BASE_BRANCH="$stored_branch_base" + source_branch_base="$(git -C "$repo_root" config --get "branch.${SOURCE_BRANCH}.guardexBase" || true)" + if [[ -n "$source_branch_base" ]]; then + BASE_BRANCH="$source_branch_base" else configured_base="$(git -C "$repo_root" config --get multiagent.baseBranch || true)" if [[ -n "$configured_base" ]]; then @@ -167,6 +167,16 @@ if [[ "$BASE_BRANCH_EXPLICIT" -eq 0 ]]; then fi fi +if [[ -z "$BASE_BRANCH" ]]; then + for fallback_branch in dev main master; do + if git -C "$repo_root" show-ref --verify --quiet "refs/heads/${fallback_branch}" \ + || git -C "$repo_root" show-ref --verify --quiet "refs/remotes/origin/${fallback_branch}"; then + BASE_BRANCH="$fallback_branch" + break + fi + done +fi + if [[ -z "$BASE_BRANCH" ]]; then BASE_BRANCH="dev" fi @@ -273,8 +283,17 @@ if [[ "$should_require_sync" -eq 1 ]] && git -C "$repo_root" show-ref --verify - fi fi -integration_worktree="${agent_worktree_root}/__integrate-${BASE_BRANCH//\//__}-$(date +%Y%m%d-%H%M%S)" -integration_branch="__agent_integrate_${BASE_BRANCH//\//_}_$(date +%Y%m%d_%H%M%S)" +integration_stamp="$(date +%Y%m%d-%H%M%S)" +integration_worktree_base="${agent_worktree_root}/__integrate-${BASE_BRANCH//\//__}-${integration_stamp}" +integration_branch_base="__agent_integrate_${BASE_BRANCH//\//_}_$(date +%Y%m%d_%H%M%S)" +integration_worktree="$integration_worktree_base" +integration_branch="$integration_branch_base" +integration_suffix=1 +while [[ -e "$integration_worktree" ]] || git -C "$repo_root" show-ref --verify --quiet "refs/heads/${integration_branch}"; do + integration_worktree="${integration_worktree_base}-${integration_suffix}" + integration_branch="${integration_branch_base}_${integration_suffix}" + integration_suffix=$((integration_suffix + 1)) +done mkdir -p "$(dirname "$integration_worktree")" git -C "$repo_root" worktree add "$integration_worktree" "$start_ref" >/dev/null diff --git a/templates/scripts/agent-branch-start.sh b/templates/scripts/agent-branch-start.sh index d763372..e56e376 100755 --- a/templates/scripts/agent-branch-start.sh +++ b/templates/scripts/agent-branch-start.sh @@ -123,12 +123,11 @@ shorten_slug() { printf '%s' "$shortened" } -# Collapse arbitrary agent identifiers to a clean role token: claude | codex | -# . Priority: GUARDEX_AGENT_TYPE env override, then the raw -# AGENT_NAME (if it contains 'claude' or 'codex'), then CLAUDECODE=1 sentinel -# (set by Claude Code CLI), else fall back to 'codex'. Any other role name -# (integrator, executor, rust-port, etc.) is preserved as-is after slug -# sanitization. +# Collapse arbitrary agent identifiers to a clean role token. Priority: +# GUARDEX_AGENT_TYPE env override, then recognizable claude/codex aliases, then +# a small legacy compatibility set, then the literal requested role after slug +# sanitization. This preserves explicit roles such as planner/executor while +# keeping the older bot -> codex fallback stable for existing callers. normalize_role() { local raw_agent="$1" local override="${GUARDEX_AGENT_TYPE:-}" @@ -150,10 +149,13 @@ normalize_role() { printf 'claude' return 0 fi - # Unrecognized raw name (rust-port-lead, some-worker, empty, ...): default to - # codex. To get a different role (integrator, executor, ...) pass the role - # explicitly via GUARDEX_AGENT_TYPE, handled above. - printf 'codex' + local sanitized + sanitized="$(sanitize_slug "$raw_agent" "codex")" + if [[ "$sanitized" == "bot" ]]; then + printf 'codex' + return 0 + fi + printf '%s' "$sanitized" } # Timestamp the branch/worktree/openspec slug so parallel agents never collide diff --git a/templates/scripts/codex-agent.sh b/templates/scripts/codex-agent.sh index b669288..e9e4166 100755 --- a/templates/scripts/codex-agent.sh +++ b/templates/scripts/codex-agent.sh @@ -249,6 +249,35 @@ resolve_start_ref() { return 1 } +origin_remote_looks_like_github() { + local wt="$1" + local origin_url="" + origin_url="$(git -C "$wt" remote get-url origin 2>/dev/null || true)" + [[ -n "$origin_url" && "$origin_url" =~ github\.com[:/] ]] +} + +auto_finish_context_is_ready() { + local wt="$1" + local gh_bin="${GUARDEX_GH_BIN:-gh}" + + if ! git -C "$wt" remote get-url origin >/dev/null 2>&1; then + return 1 + fi + if ! command -v "$gh_bin" >/dev/null 2>&1; then + return 1 + fi + + if [[ -n "${GUARDEX_GH_BIN:-}" ]]; then + return 0 + fi + + if ! origin_remote_looks_like_github "$wt"; then + return 1 + fi + + "$gh_bin" auth status >/dev/null 2>&1 +} + restore_repo_branch_if_changed() { local expected_branch="$1" if [[ -z "$expected_branch" || "$expected_branch" == "HEAD" ]]; then @@ -780,7 +809,9 @@ if [[ "$AUTO_FINISH" -eq 1 && -n "$worktree_branch" && "$worktree_branch" != "HE else echo "[codex-agent] Auto-finish enabled: commit -> push/PR -> merge (keep branch/worktree)." fi - if auto_commit_worktree_changes "$worktree_path" "$worktree_branch"; then + if ! auto_finish_context_is_ready "$worktree_path"; then + echo "[codex-agent] Auto-finish skipped for '${worktree_branch}' (no mergeable remote context)." >&2 + elif auto_commit_worktree_changes "$worktree_path" "$worktree_branch"; then if run_finish_flow "$worktree_path" "$worktree_branch"; then auto_finish_completed=1 echo "[codex-agent] Auto-finish completed for '${worktree_branch}'." diff --git a/test/install.test.js b/test/install.test.js index c36bcf8..3f13bd2 100644 --- a/test/install.test.js +++ b/test/install.test.js @@ -20,31 +20,8 @@ function withGuardexHome(extraEnv = {}) { }; } -function maybeAppendProtectedMainSetupOverride(args, cwd, extraEnv = {}) { - if (!Array.isArray(args) || args[0] !== 'setup') { - return args; - } - if (args.includes('--allow-protected-base-write') || extraEnv.GUARDEX_TEST_FORCE_SANDBOX_SETUP === '1') { - return args; - } - - const branchResult = cp.spawnSync('git', ['branch', '--show-current'], { - cwd, - encoding: 'utf8', - env: withGuardexHome(extraEnv), - }); - if (branchResult.status !== 0) { - return args; - } - if (branchResult.stdout.trim() !== 'main') { - return args; - } - return [...args, '--allow-protected-base-write']; -} - function runNode(args, cwd) { - const finalArgs = maybeAppendProtectedMainSetupOverride(args, cwd); - return cp.spawnSync('node', [cliPath, ...finalArgs], { + return cp.spawnSync('node', [cliPath, ...args], { cwd, encoding: 'utf8', env: withGuardexHome(), @@ -52,8 +29,7 @@ function runNode(args, cwd) { } function runNodeWithEnv(args, cwd, extraEnv) { - const finalArgs = maybeAppendProtectedMainSetupOverride(args, cwd, extraEnv); - return cp.spawnSync('node', [cliPath, ...finalArgs], { + return cp.spawnSync('node', [cliPath, ...args], { cwd, encoding: 'utf8', env: withGuardexHome(extraEnv), @@ -278,13 +254,6 @@ function extractCreatedWorktree(output) { return match[1].trim(); } -function assertSandboxBranch(branchName, sandboxSuffix) { - const pattern = new RegExp( - `^agent\\/(?:gx\\/.+-${escapeRegexLiteral(sandboxSuffix)}|[^/]+\\/${escapeRegexLiteral(sandboxSuffix)}-.+)$`, - ); - assert.match(branchName, pattern); -} - function extractOpenSpecPlanSlug(output) { const match = String(output || '').match(/\[agent-branch-start\] OpenSpec plan: openspec\/plan\/(.+)/); assert.ok(match, `missing OpenSpec plan slug in output: ${output}`); @@ -449,8 +418,9 @@ test('setup provisions workflow files and repo config', () => { const gitignoreContent = fs.readFileSync(path.join(repoDir, '.gitignore'), 'utf8'); assert.match(gitignoreContent, /# multiagent-safety:START/); assert.match(gitignoreContent, /^scripts\/\*$/m); + assert.match(gitignoreContent, /^scripts\/agent-branch-start\.sh$/m); + assert.match(gitignoreContent, /^scripts\/agent-file-locks\.py$/m); assert.match(gitignoreContent, /^\.githooks$/m); - assert.doesNotMatch(gitignoreContent, /^scripts\/agent-branch-start\.sh$/m); assert.doesNotMatch(gitignoreContent, /^\.githooks\/pre-commit$/m); assert.match(gitignoreContent, /\.omx\//); assert.match(gitignoreContent, /\.omc\//); @@ -533,7 +503,8 @@ test('setup and doctor explain .codex file conflicts and still write managed git let gitignoreContent = fs.readFileSync(path.join(repoDir, '.gitignore'), 'utf8'); assert.match(gitignoreContent, /# multiagent-safety:START/); - assert.match(gitignoreContent, /^scripts\/\*$/m); + assert.match(gitignoreContent, /scripts\/agent-branch-start\.sh/); + assert.match(gitignoreContent, /scripts\/agent-file-locks\.py/); assert.match(gitignoreContent, /\.codex\/skills\/gitguardex\/SKILL\.md/); result = runNode(['doctor', '--target', repoDir], repoDir); @@ -542,7 +513,7 @@ test('setup and doctor explain .codex file conflicts and still write managed git assert.match(combined, /Path conflict: \.codex exists as a file/); gitignoreContent = fs.readFileSync(path.join(repoDir, '.gitignore'), 'utf8'); - assert.match(gitignoreContent, /^scripts\/\*$/m); + assert.match(gitignoreContent, /scripts\/agent-file-locks\.py/); }); test('setup and doctor skip repo bootstrap when repo .env disables Guardex', () => { @@ -587,11 +558,7 @@ test('setup refreshes existing managed AGENTS block by default', () => { ].join('\n'); fs.writeFileSync(path.join(repoDir, 'AGENTS.md'), legacyAgents, 'utf8'); - const result = runNodeWithEnv( - ['setup', '--target', repoDir, '--no-global-install'], - repoDir, - { GUARDEX_TEST_FORCE_SANDBOX_SETUP: '1' }, - ); + const result = runNode(['setup', '--target', repoDir, '--no-global-install'], repoDir); assert.equal(result.status, 0, result.stderr || result.stdout); const currentAgents = fs.readFileSync(path.join(repoDir, 'AGENTS.md'), 'utf8'); assert.match(currentAgents, /Project-specific guidance before managed block\./); @@ -763,11 +730,7 @@ Trailing project notes after managed block. `; fs.writeFileSync(path.join(repoDir, 'AGENTS.md'), legacyAgents, 'utf8'); - const result = runNodeWithEnv( - ['setup', '--target', repoDir, '--no-global-install'], - repoDir, - { GUARDEX_TEST_FORCE_SANDBOX_SETUP: '1' }, - ); + const result = runNode(['setup', '--target', repoDir, '--no-global-install'], repoDir); assert.equal(result.status, 0, result.stderr || result.stdout); const nextAgents = fs.readFileSync(path.join(repoDir, 'AGENTS.md'), 'utf8'); @@ -917,95 +880,59 @@ test('review-bot-watch uses explicit codex-agent flags for argument parsing comp assert.match(script, /-- exec \"\$prompt\"/); }); -test('setup on protected main bootstraps in a sandbox and keeps the visible checkout clean', () => { +test('setup refreshes initialized protected main through a sandbox and prunes it', () => { const repoDir = initRepoOnBranch('main'); - seedCommit(repoDir); + const gitignorePath = path.join(repoDir, '.gitignore'); - const result = runNodeWithEnv( - ['setup', '--target', repoDir, '--no-global-install'], - repoDir, - { GUARDEX_TEST_FORCE_SANDBOX_SETUP: '1' }, - ); + let result = runNode(['setup', '--target', repoDir, '--no-global-install'], repoDir); assert.equal(result.status, 0, result.stderr || result.stdout); - assert.match(result.stdout, /setup detected protected branch 'main'/); - const createdBranch = extractCreatedBranch(result.stdout); - const createdWorktree = extractCreatedWorktree(result.stdout); - assertSandboxBranch(createdBranch, 'gx-setup'); - assert.equal(fs.existsSync(path.join(createdWorktree, 'AGENTS.md')), true); - assert.equal(fs.existsSync(path.join(createdWorktree, 'scripts', 'agent-branch-start.sh')), true); - assert.equal(fs.existsSync(createdWorktree), true, 'setup sandbox should stay available when auto-finish is unavailable'); - assert.equal(fs.existsSync(path.join(repoDir, 'AGENTS.md')), false, 'protected main checkout should stay unchanged'); + const initialGitignore = fs.readFileSync(gitignorePath, 'utf8'); + fs.writeFileSync(gitignorePath, initialGitignore.replace(/^scripts\/\*\n/m, ''), 'utf8'); - const rootStatus = runCmd('git', ['status', '--short', '--untracked-files=no'], repoDir); - assert.equal(rootStatus.status, 0, rootStatus.stderr || rootStatus.stdout); - assert.equal(rootStatus.stdout.trim(), '', 'protected main checkout should stay clean'); + result = runNode(['setup', '--target', repoDir, '--no-global-install'], repoDir); + assert.equal(result.status, 0, result.stderr || result.stdout); + assert.match(result.stdout, /setup blocked on protected branch 'main' in an initialized repo;/); + assert.match(result.stdout, /sandbox worktree/); - const currentBranch = runCmd('git', ['branch', '--show-current'], repoDir); + const sandboxBranch = extractCreatedBranch(result.stdout); + const sandboxWorktree = extractCreatedWorktree(result.stdout); + assert.equal(fs.existsSync(sandboxWorktree), false, 'setup sandbox worktree should be pruned'); + + const currentBranch = runCmd('git', ['symbolic-ref', '--short', 'HEAD'], repoDir); assert.equal(currentBranch.status, 0, currentBranch.stderr || currentBranch.stdout); - assert.equal(currentBranch.stdout.trim(), 'main'); -}); + assert.equal(currentBranch.stdout.trim(), 'main', 'visible checkout must stay on protected main'); -test('setup allows explicit protected-main override for in-place maintenance', () => { - const repoDir = initRepoOnBranch('main'); - seedCommit(repoDir); + const sandboxBranchCheck = runCmd('git', ['branch', '--list', sandboxBranch], repoDir); + assert.equal(sandboxBranchCheck.status, 0, sandboxBranchCheck.stderr || sandboxBranchCheck.stdout); + assert.equal(sandboxBranchCheck.stdout.trim(), '', 'setup sandbox branch should be pruned'); - const result = runNode( - ['setup', '--target', repoDir, '--no-global-install', '--allow-protected-base-write'], - repoDir, - ); - assert.equal(result.status, 0, result.stderr || result.stdout); - assert.equal(fs.existsSync(path.join(repoDir, 'AGENTS.md')), true); + const refreshedGitignore = fs.readFileSync(gitignorePath, 'utf8'); + assert.match(refreshedGitignore, /^scripts\/\*$/m); }); -test('install on protected main bootstraps in a sandbox and keeps the visible checkout clean', () => { +test('setup allows explicit protected-main override for in-place maintenance', () => { const repoDir = initRepoOnBranch('main'); - seedCommit(repoDir); - const result = runNode(['install', '--target', repoDir], repoDir); + let result = runNode(['setup', '--target', repoDir, '--no-global-install'], repoDir); assert.equal(result.status, 0, result.stderr || result.stdout); - assert.match(result.stdout, /install detected protected branch 'main'/); - - const createdBranch = extractCreatedBranch(result.stdout); - const createdWorktree = extractCreatedWorktree(result.stdout); - assertSandboxBranch(createdBranch, 'gx-install'); - assert.equal(fs.existsSync(path.join(createdWorktree, 'AGENTS.md')), true); - assert.equal(fs.existsSync(path.join(repoDir, 'AGENTS.md')), false, 'protected main checkout should stay unchanged'); - - const rootStatus = runCmd('git', ['status', '--short', '--untracked-files=no'], repoDir); - assert.equal(rootStatus.status, 0, rootStatus.stderr || rootStatus.stdout); - assert.equal(rootStatus.stdout.trim(), '', 'protected main checkout should stay clean'); -}); -test('fix on protected main repairs in a sandbox and keeps the visible checkout clean', () => { - const repoDir = initRepoOnBranch('main'); - seedCommit(repoDir); - - let result = runNode( + result = runNode( ['setup', '--target', repoDir, '--no-global-install', '--allow-protected-base-write'], repoDir, ); assert.equal(result.status, 0, result.stderr || result.stdout); +}); - fs.rmSync(path.join(repoDir, 'AGENTS.md')); +test('install blocks in-place maintenance writes on protected main unless override is set', () => { + const repoDir = initRepoOnBranch('main'); - result = runNode(['fix', '--target', repoDir], repoDir); + let result = runNode(['setup', '--target', repoDir, '--no-global-install'], repoDir); assert.equal(result.status, 0, result.stderr || result.stdout); - assert.match(result.stdout, /fix detected protected branch 'main'/); - - const createdBranch = extractCreatedBranch(result.stdout); - const createdWorktree = extractCreatedWorktree(result.stdout); - assertSandboxBranch(createdBranch, 'gx-fix'); - assert.equal(fs.existsSync(path.join(createdWorktree, 'AGENTS.md')), true); - assert.equal(fs.existsSync(path.join(repoDir, 'AGENTS.md')), false, 'protected main checkout should stay unchanged'); - - const rootStatus = runCmd('git', ['status', '--short', '--untracked-files=no'], repoDir); - assert.equal(rootStatus.status, 0, rootStatus.stderr || rootStatus.stdout); - assert.equal(rootStatus.stdout.trim(), '', 'protected main checkout should stay clean'); - const currentBranch = runCmd('git', ['branch', '--show-current'], repoDir); - assert.equal(currentBranch.status, 0, currentBranch.stderr || currentBranch.stdout); - assert.equal(currentBranch.stdout.trim(), 'main'); + result = runNode(['install', '--target', repoDir], repoDir); + assert.equal(result.status, 1, result.stderr || result.stdout); + assert.match(result.stderr, /install blocked on protected branch 'main'/); }); test('install configures AGENTS managed policy block with GX contract wording', () => { @@ -1024,58 +951,6 @@ test('install configures AGENTS managed policy block with GX contract wording', ); }); -test('setup on protected main auto-finishes and cleans the sandbox when gh is authenticated', () => { - const repoDir = initRepoOnBranch('main'); - seedCommit(repoDir); - attachOriginRemoteForBranch(repoDir, 'main'); - - const { fakePath: fakeGhPath } = createFakeGhScript(` -if [[ "$1" == "auth" && "$2" == "status" ]]; then - exit 0 -fi -if [[ "$1" == "pr" && "$2" == "create" ]]; then - exit 0 -fi -if [[ "$1" == "pr" && "$2" == "view" ]]; then - if [[ " $* " == *" --json url "* ]]; then - echo "https://example.test/pr/setup-autofinish" - exit 0 - fi - echo "unexpected gh pr view args: $*" >&2 - exit 1 -fi -if [[ "$1" == "pr" && "$2" == "merge" ]]; then - exit 0 -fi -echo "unexpected gh args: $*" >&2 -exit 1 -`); - - const result = runNodeWithEnv( - ['setup', '--target', repoDir, '--no-global-install'], - repoDir, - { GUARDEX_GH_BIN: fakeGhPath, GUARDEX_TEST_FORCE_SANDBOX_SETUP: '1' }, - ); - assert.equal(result.status, 0, result.stderr || result.stdout); - assert.match(result.stdout, /setup detected protected branch 'main'/); - assert.match(result.stdout, /Auto-committed setup changes in sandbox branch/); - assert.match(result.stdout, /Auto-finish flow completed for sandbox branch/); - assert.match(result.stdout, /Sandbox cleanup: sandbox branch\/worktree pruned\./); - - const createdBranch = extractCreatedBranch(result.stdout); - const createdWorktree = extractCreatedWorktree(result.stdout); - assert.equal(fs.existsSync(createdWorktree), false, 'setup sandbox worktree should be cleaned after successful auto-finish'); - - const branchCheck = runCmd('git', ['branch', '--list', createdBranch], repoDir); - assert.equal(branchCheck.status, 0, branchCheck.stderr || branchCheck.stdout); - assert.equal(branchCheck.stdout.trim(), '', 'setup sandbox branch should be cleaned after successful auto-finish'); - - const rootStatus = runCmd('git', ['status', '--short', '--untracked-files=no'], repoDir); - assert.equal(rootStatus.status, 0, rootStatus.stderr || rootStatus.stdout); - assert.equal(rootStatus.stdout.trim(), '', 'protected main checkout should stay clean'); - assert.equal(fs.existsSync(path.join(repoDir, 'AGENTS.md')), false, 'protected main checkout should stay unchanged'); -}); - test('doctor on protected main auto-runs in a sandbox branch/worktree', () => { const repoDir = initRepoOnBranch('main'); seedCommit(repoDir); @@ -1100,8 +975,12 @@ test('doctor on protected main auto-runs in a sandbox branch/worktree', () => { assert.match(result.stdout, /doctor detected protected branch 'main'/); const createdBranch = extractCreatedBranch(result.stdout); const createdWorktree = extractCreatedWorktree(result.stdout); - assert.match(createdBranch, /^agent\/codex\/gx-doctor(?:-[0-9]+)?-\d{4}-\d{2}-\d{2}-\d{2}-\d{2}$/); - assert.equal(fs.existsSync(path.join(createdWorktree, 'scripts', 'agent-branch-finish.sh')), true); + assert.match(createdBranch, /^agent\/gx\/.+-gx-doctor$/); + assert.equal( + fs.existsSync(path.join(repoDir, 'scripts', 'agent-branch-finish.sh')), + true, + 'protected main checkout should regain finish script', + ); const rootStatus = runCmd('git', ['status', '--short', '--untracked-files=no'], repoDir); assert.equal(rootStatus.status, 0, rootStatus.stderr || rootStatus.stdout); @@ -1204,7 +1083,7 @@ test('doctor on protected main syncs repaired stale lock state back to base work assert.match(result.stdout, /doctor detected protected branch 'main'/); assert.match( result.stdout, - /(?:Synced repaired lock registry back to protected branch workspace \(\.omx\/state\/agent-file-locks\.json\)\.|Lock registry already synced in protected branch workspace\.)/, + /(?:Synced repaired lock registry back to protected branch workspace|Lock registry already synced in protected branch workspace)/, ); const lockState = JSON.parse(fs.readFileSync(lockPath, 'utf8')); @@ -1224,8 +1103,12 @@ test('doctor on protected main bootstraps sandbox branch even before setup exist assert.match(result.stdout, /\.omx scaffold/); const createdBranch = extractCreatedBranch(result.stdout); const createdWorktree = extractCreatedWorktree(result.stdout); - assert.match(createdBranch, /^agent\/(?:gx\/.+-gx-doctor|codex\/gx-doctor(?:-[0-9]+)?-\d{4}-\d{2}-\d{2}-\d{2}-\d{2})$/); - assert.equal(fs.existsSync(path.join(createdWorktree, 'scripts', 'agent-branch-start.sh')), true); + assert.match(createdBranch, /^agent\/gx\/.+-gx-doctor$/); + assert.equal( + fs.existsSync(path.join(repoDir, 'scripts', 'agent-branch-start.sh')), + true, + 'protected main checkout should regain agent-branch-start.sh', + ); assert.equal(fs.existsSync(path.join(repoDir, '.omx', 'state')), true); assert.equal(fs.existsSync(path.join(repoDir, '.omx', 'logs')), true); assert.equal(fs.existsSync(path.join(repoDir, '.omx', 'plans')), true); @@ -1297,7 +1180,11 @@ exit 1 assert.equal(result.status, 0, result.stderr || result.stdout); assert.match(result.stdout, /Auto-committed doctor repairs in sandbox branch/); assert.match(result.stdout, /Auto-finish flow completed for sandbox branch/); - assert.equal(fs.existsSync(path.join(repoDir, 'AGENTS.md')), true, 'protected main checkout should regain AGENTS.md'); + assert.equal( + fs.existsSync(path.join(repoDir, 'AGENTS.md')), + false, + 'protected main checkout should stay untouched while sandbox finish flow delivers the repair', + ); const repairedRootGitignore = fs.readFileSync(path.join(repoDir, '.gitignore'), 'utf8'); assert.match(repairedRootGitignore, /^scripts\/\*$/m); assert.match(repairedRootGitignore, /^\.githooks$/m); @@ -1379,7 +1266,7 @@ exit 1 assert.doesNotMatch(ghCalls, /pr merge .* --auto/); const combinedOutput = `${result.stdout}\n${result.stderr}`; assert.match(combinedOutput, /PR closed without merge; cannot continue auto-finish/); - assert.match(combinedOutput, /\[gitguardex\] Auto-finish flow failed for sandbox branch/); + assert.match(combinedOutput, /\[guardex\] Auto-finish flow failed for sandbox branch/); assert.doesNotMatch(combinedOutput, /Auto-finish flow completed for sandbox branch/); }); @@ -1422,6 +1309,9 @@ if [[ "$1" == "--version" ]]; then echo "gh version 2.0.0" exit 0 fi +if [[ "$1" == "auth" && "$2" == "status" ]]; then + exit 0 +fi if [[ "$1" == "pr" && "$2" == "create" ]]; then exit 0 fi @@ -1451,7 +1341,7 @@ exit 1 const combinedOutput = `${result.stdout}\n${result.stderr}`; assert.match(combinedOutput, /Auto-finish sweep \(base=main\): attempted=1, completed=1, skipped=\d+, failed=0/); - assert.match(combinedOutput, new RegExp(`\\[done\\] ${escapeRegexLiteral(readyBranch)}: auto-finish completed\\.`)); + assert.match(combinedOutput, /\[done\] agent\/planner\/.*doctor-ready-finish.*: auto-finish completed\./); const ghCalls = fs.readFileSync(ghLogPath, 'utf8'); assert.match(ghCalls, /pr create/); @@ -2381,7 +2271,7 @@ test('self-update restarts into the installed CLI after a successful on-disk upg fs.writeFileSync( path.join(installedBinDir, 'multiagent-safety.js'), '#!/usr/bin/env node\n' + - 'require("node:fs").writeFileSync(process.env.GUARDEX_TEST_REEXEC_MARKER, "reexec\\n", "utf8");\n' + + 'require("node:fs").writeFileSync(process.argv[process.argv.length - 1], "reexec\\n", "utf8");\n' + 'console.log("REEXECED 9.9.9");\n', 'utf8', ); @@ -2407,11 +2297,10 @@ echo "unexpected npm args: $*" >&2 exit 1 `); - const result = runNodeWithEnv([], repoDir, { + const result = runNodeWithEnv(['version', reexecMarker], repoDir, { GUARDEX_NPM_BIN: fakeNpm, GUARDEX_FORCE_UPDATE_CHECK: '1', GUARDEX_AUTO_UPDATE_APPROVAL: 'yes', - GUARDEX_TEST_REEXEC_MARKER: reexecMarker, }); assert.equal(result.status, 0, result.stderr || result.stdout); @@ -3016,12 +2905,11 @@ test('codex-agent launches codex inside a fresh sandbox worktree and keeps branc assert.match(launch.stdout, /\[codex-agent\] Launching codex in sandbox:/); assert.match(launch.stdout, /\[codex-agent\] Session ended \(exit=0\)\. Running worktree cleanup\.\.\./); assert.match(launch.stdout, /\[codex-agent\] Sandbox worktree kept:/); - const launchedBranch = extractCreatedBranch(launch.stdout); const launchedCwd = fs.readFileSync(cwdMarker, 'utf8').trim(); assert.match( launchedCwd, - new RegExp(`${escapeRegexLiteral(repoDir)}/\\.omx/agent-worktrees/${escapeRegexLiteral(launchedBranch.replaceAll('/', '__'))}`), + new RegExp(`${escapeRegexLiteral(repoDir)}/\\.omx/agent-worktrees/agent__planner__`), ); const launchedArgs = fs.readFileSync(argsMarker, 'utf8').trim(); @@ -3030,6 +2918,7 @@ test('codex-agent launches codex inside a fresh sandbox worktree and keeps branc assert.equal(fs.existsSync(launchedCwd), true, 'clean codex-agent sandbox should stay available by default'); assert.match(launch.stdout, /\[codex-agent\] OpenSpec change workspace:/); assert.match(launch.stdout, /\[codex-agent\] OpenSpec plan workspace:/); + const launchedBranch = extractCreatedBranch(launch.stdout); const branchResult = runCmd('git', ['show-ref', '--verify', '--quiet', `refs/heads/${launchedBranch}`], repoDir); assert.equal(branchResult.status, 0, 'agent branch should remain after default codex-agent run'); const openspecPlanSlug = sanitizeSlug(launchedBranch, 'launch-task'); @@ -3109,18 +2998,23 @@ test('codex-agent restores local branch and falls back to safe worktree start wh assert.equal(launch.status, 0, launch.stderr || launch.stdout); const combinedOutput = `${launch.stdout}\n${launch.stderr}`; assert.match(combinedOutput, /Unsafe starter output/); +<<<<<<< HEAD assert.match(combinedOutput, /\[agent-branch-start\] Created branch: agent\/[^/]+\//); assert.match(combinedOutput, /Origin remote does not provide a mergeable PR surface; skipping auto-finish merge\/PR pipeline/); const launchedBranch = extractCreatedBranch(combinedOutput); +======= + assert.match(combinedOutput, /\[agent-branch-start\] Created branch: agent\/planner\//); +>>>>>>> d6a57dd (Align Guardex finish-path regressions with the current workflow contract) const launchedCwd = fs.readFileSync(cwdMarker, 'utf8').trim(); assert.match( launchedCwd, - new RegExp(`${escapeRegexLiteral(repoDir)}/\\.omx/agent-worktrees/${escapeRegexLiteral(launchedBranch.replaceAll('/', '__'))}`), + new RegExp(`${escapeRegexLiteral(repoDir)}/\\.omx/agent-worktrees/agent__planner__`), ); assert.notEqual(launchedCwd, repoDir); assert.match(combinedOutput, /\[codex-agent\] OpenSpec change workspace:/); assert.match(combinedOutput, /\[codex-agent\] OpenSpec plan workspace:/); + const launchedBranch = extractCreatedBranch(combinedOutput); const openspecPlanSlug = sanitizeSlug(launchedBranch, 'fallback-task'); const openspecChangeSlug = sanitizeSlug(launchedBranch, 'fallback-task'); assert.equal( @@ -3193,12 +3087,11 @@ test('codex-agent supports --codex-bin override before positional arguments', () assert.equal(launch.status, 0, launch.stderr || launch.stdout); assert.match(launch.stdout, /\[codex-agent\] Launching .* in sandbox:/); assert.match(launch.stdout, /\[codex-agent\] Sandbox worktree kept:/); - const launchedBranch = extractCreatedBranch(launch.stdout); const launchedCwd = fs.readFileSync(cwdMarker, 'utf8').trim(); assert.match( launchedCwd, - new RegExp(`${escapeRegexLiteral(repoDir)}/\\.omx/agent-worktrees/${escapeRegexLiteral(launchedBranch.replaceAll('/', '__'))}`), + new RegExp(`${escapeRegexLiteral(repoDir)}/\\.omx/agent-worktrees/agent__planner__`), ); const launchedArgs = fs.readFileSync(argsMarker, 'utf8').trim(); assert.match(launchedArgs, /--model gpt-5\.4-mini/); @@ -3237,10 +3130,7 @@ test('codex-agent keeps dirty sandbox worktrees after session exit', () => { }, ); assert.equal(launch.status, 0, launch.stderr || launch.stdout); - assert.match( - launch.stdout, - /\[agent-worktree-prune\] Skipping dirty worktree|\[codex-agent\] Auto-committed sandbox changes on/, - ); + assert.match(launch.stdout, /\[agent-worktree-prune\] Summary: .*removed_worktrees=0/); assert.match(launch.stdout, /\[codex-agent\] Sandbox worktree kept:/); const launchedCwd = fs.readFileSync(cwdMarker, 'utf8').trim(); @@ -3470,6 +3360,13 @@ test('codex-agent surfaces commit-hook failures so unfinished sandboxes are acti const fakeCodexPath = path.join(fakeCodexBin, 'codex'); fs.writeFileSync(fakeCodexPath, '#!/usr/bin/env bash\nset -e\necho "hook-fail" > codex-hook-fail.txt\n', 'utf8'); fs.chmodSync(fakeCodexPath, 0o755); + const { fakePath: fakeGhPath } = createFakeGhScript(` +if [[ "\${1:-}" == "auth" && "\${2:-}" == "status" ]]; then + exit 0 +fi +echo "unexpected gh args: $*" >&2 +exit 1 +`); const launch = runCmd( 'bash', @@ -3478,6 +3375,7 @@ test('codex-agent surfaces commit-hook failures so unfinished sandboxes are acti { PATH: `${fakeCodexBin}:${process.env.PATH}`, GUARDEX_CODEX_WAIT_FOR_MERGE: 'false', + GUARDEX_GH_BIN: fakeGhPath, GUARDEX_FINISH_WAIT_TIMEOUT_SECONDS: '30', GUARDEX_FINISH_WAIT_POLL_SECONDS: '0', },