From 7d7ac64ffa814b8f8d063be293767bc98a70bd31 Mon Sep 17 00:00:00 2001 From: NagyVikt Date: Mon, 13 Apr 2026 23:28:14 +0200 Subject: [PATCH] Prevent finished agent branches from lingering without PR merge Setup and doctor now run an auto-finish sweep for clean pending agent/* branches against the current local base branch (including protected-main doctor sandbox runs). This closes the common gap where branch worktrees stayed local/visible in VS Code after work was done. Added regression coverage for protected-main doctor flow and a metadata parity guard so runtime codex-agent helper stays aligned with templates. Constraint: Keep protected-main write policy intact while enabling post-doctor auto-finish via sandbox-safe path Rejected: Force-commit dirty pending branches automatically | risks shipping partial/unreviewed work Confidence: medium Scope-risk: moderate Reversibility: clean Directive: If auto-finish sweep behavior changes, update doctor/setup output and test fixtures together Tested: node --check bin/multiagent-safety.js; node --test test/install.test.js; node --test test/metadata.test.js; npm test Not-tested: Live GitHub required-check wait behavior in a real protected remote --- README.md | 2 + bin/multiagent-safety.js | 255 +++++++++++++++++++++++++++++++++++++++ test/install.test.js | 80 ++++++++++++ test/metadata.test.js | 16 +++ 4 files changed, 353 insertions(+) diff --git a/README.md b/README.md index 6f854d2..a952753 100644 --- a/README.md +++ b/README.md @@ -69,6 +69,7 @@ gx cleanup --branch "$(git rev-parse --abbrev-ref HEAD)" ``` If you use `scripts/codex-agent.sh`, the finish flow is auto-run after the Codex session exits. +It auto-commits sandbox changes, retries once after syncing if the branch moved behind base during the run, then pushes/opens PR merge flow against the current base branch. ## Visual workflow @@ -143,6 +144,7 @@ Note: the monitor dispatches Codex through explicit `--task/--agent/--base` flag - `gx setup` checks GitHub CLI (`gh`) and prints install guidance if missing. - Interactive self-update prompt defaults to **No** (`[y/N]`). - In initialized repos, `setup`/`install`/`fix` block protected-base writes unless explicitly overridden. +- `gx setup` and `gx doctor` auto-finish clean pending `agent/*` branches (push + PR merge cleanup) against the current local base branch. - Direct commits/pushes to protected branches are blocked by default (including VS Code Source Control). - Optional repo override for manual VS Code protected-branch writes: `git config multiagent.allowVscodeProtectedBranchWrites true`. - Codex/agent sessions stay blocked on protected branches and must use `agent/*` branch + PR workflow. diff --git a/bin/multiagent-safety.js b/bin/multiagent-safety.js index 977b22a..4b2b7f4 100755 --- a/bin/multiagent-safety.js +++ b/bin/multiagent-safety.js @@ -350,6 +350,7 @@ NOTES - ${TOOL_NAME} setup asks for Y/N approval before global installs - ${TOOL_NAME} setup checks GitHub CLI (gh) and prints install guidance if missing - In initialized repos, setup/install/fix block in-place writes on protected main by default + - setup/doctor auto-finish clean pending agent/* branches via PR flow into the current local base branch - doctor auto-runs in a sandbox agent branch/worktree on protected main and tries auto-finish PR flow - agent-branch-finish merges by default and keeps agent branches/worktrees until explicit cleanup - use '${SHORT_TOOL_NAME} cleanup' to remove merged agent branches/worktrees (optionally remote refs too) @@ -1197,6 +1198,14 @@ function runDoctorInSandbox(options, blocked) { status: 'skipped', note: 'sandbox doctor did not complete successfully', }; + let postSandboxAutoFinishSummary = { + enabled: false, + attempted: 0, + completed: 0, + skipped: 0, + failed: 0, + details: ['Skipped auto-finish sweep (sandbox doctor did not complete successfully).'], + }; if (nestedResult.status === 0) { if (!options.dryRun) { autoCommitResult = autoCommitDoctorSandboxChanges(metadata); @@ -1253,6 +1262,12 @@ function runDoctorInSandbox(options, blocked) { }; } } + + postSandboxAutoFinishSummary = autoFinishReadyAgentBranches(blocked.repoRoot, { + baseBranch: blocked.branch, + dryRun: options.dryRun, + excludeBranches: [metadata.branch], + }); } if (options.json) { @@ -1267,6 +1282,7 @@ function runDoctorInSandbox(options, blocked) { sandboxLockSync: lockSyncResult, sandboxAutoCommit: autoCommitResult, sandboxFinish: finishResult, + autoFinish: postSandboxAutoFinishSummary, }, null, 2, @@ -1332,6 +1348,17 @@ function runDoctorInSandbox(options, blocked) { } 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}`, + ); + for (const detail of postSandboxAutoFinishSummary.details) { + console.log(`[${TOOL_NAME}] ${detail}`); + } + } else if (postSandboxAutoFinishSummary.details.length > 0) { + console.log(`[${TOOL_NAME}] ${postSandboxAutoFinishSummary.details[0]}`); + } } } @@ -1628,6 +1655,203 @@ function listLocalUserBranches(repoRoot) { return [branchName]; } +function listLocalAgentBranches(repoRoot) { + const result = gitRun( + repoRoot, + ['for-each-ref', '--format=%(refname:short)', 'refs/heads/agent/'], + { allowFailure: true }, + ); + if (result.status !== 0) { + return []; + } + return uniquePreserveOrder( + String(result.stdout || '') + .split('\n') + .map((item) => item.trim()) + .filter(Boolean), + ); +} + +function mapWorktreePathsByBranch(repoRoot) { + const result = gitRun(repoRoot, ['worktree', 'list', '--porcelain'], { allowFailure: true }); + const map = new Map(); + if (result.status !== 0) { + return map; + } + + const lines = String(result.stdout || '').split('\n'); + let currentWorktree = ''; + for (const line of lines) { + if (line.startsWith('worktree ')) { + currentWorktree = line.slice('worktree '.length).trim(); + continue; + } + if (line.startsWith('branch refs/heads/')) { + const branchName = line.slice('branch refs/heads/'.length).trim(); + if (currentWorktree && branchName) { + map.set(branchName, currentWorktree); + } + } + } + return map; +} + +function hasSignificantWorkingTreeChanges(worktreePath) { + const result = run('git', ['-C', worktreePath, 'status', '--porcelain']); + if (result.status !== 0) { + return true; + } + + const lines = String(result.stdout || '') + .split('\n') + .map((line) => line.trimEnd()) + .filter((line) => line.length > 0); + + for (const line of lines) { + const pathPart = (line.length > 3 ? line.slice(3) : '').trim(); + if (!pathPart) continue; + if (pathPart === LOCK_FILE_RELATIVE) continue; + if (pathPart.startsWith(`${LOCK_FILE_RELATIVE} -> `)) continue; + if (pathPart.endsWith(` -> ${LOCK_FILE_RELATIVE}`)) continue; + return true; + } + return false; +} + +function autoFinishReadyAgentBranches(repoRoot, options = {}) { + const baseBranch = String(options.baseBranch || '').trim(); + const dryRun = Boolean(options.dryRun); + const excludedBranches = new Set( + Array.isArray(options.excludeBranches) + ? options.excludeBranches.map((branch) => String(branch || '').trim()).filter(Boolean) + : [], + ); + + const summary = { + enabled: true, + baseBranch, + attempted: 0, + completed: 0, + skipped: 0, + failed: 0, + details: [], + }; + + if (!baseBranch || baseBranch === 'HEAD' || baseBranch.startsWith('agent/')) { + summary.enabled = false; + summary.details.push('Skipped auto-finish sweep (base branch is missing or not a non-agent local branch).'); + return summary; + } + + if (String(process.env.MUSAFETY_DOCTOR_SANDBOX || '') === '1') { + summary.enabled = false; + summary.details.push('Skipped auto-finish sweep inside doctor sandbox pass.'); + return summary; + } + + if (String(process.env.MUSAFETY_SKIP_AUTO_FINISH_READY_BRANCHES || '') === '1') { + summary.enabled = false; + summary.details.push('Skipped auto-finish sweep (MUSAFETY_SKIP_AUTO_FINISH_READY_BRANCHES=1).'); + return summary; + } + + if (dryRun) { + summary.enabled = false; + summary.details.push('Skipped auto-finish sweep in dry-run mode.'); + return summary; + } + + const finishScript = path.join(repoRoot, 'scripts', 'agent-branch-finish.sh'); + if (!fs.existsSync(finishScript)) { + summary.enabled = false; + summary.details.push(`Skipped auto-finish sweep (missing ${path.relative(repoRoot, finishScript)}).`); + return summary; + } + + const hasOrigin = gitRun(repoRoot, ['remote', 'get-url', 'origin'], { allowFailure: true }).status === 0; + if (!hasOrigin) { + summary.enabled = false; + summary.details.push('Skipped auto-finish sweep (origin remote missing).'); + return summary; + } + + const ghBin = process.env.MUSAFETY_GH_BIN || 'gh'; + if (run(ghBin, ['--version']).status !== 0) { + summary.enabled = false; + summary.details.push(`Skipped auto-finish sweep (${ghBin} not available).`); + return summary; + } + + const branchWorktrees = mapWorktreePathsByBranch(repoRoot); + const agentBranches = listLocalAgentBranches(repoRoot); + if (agentBranches.length === 0) { + summary.enabled = false; + summary.details.push('No local agent branches found for auto-finish sweep.'); + return summary; + } + + for (const branch of agentBranches) { + if (excludedBranches.has(branch)) { + summary.skipped += 1; + summary.details.push(`[skip] ${branch}: excluded from this auto-finish sweep.`); + continue; + } + + if (branch === baseBranch) { + summary.skipped += 1; + summary.details.push(`[skip] ${branch}: source branch equals base branch.`); + continue; + } + + let counts; + try { + counts = aheadBehind(repoRoot, branch, baseBranch); + } catch (error) { + summary.failed += 1; + summary.details.push(`[fail] ${branch}: unable to compute ahead/behind (${error.message}).`); + continue; + } + + if (counts.ahead <= 0) { + summary.skipped += 1; + summary.details.push(`[skip] ${branch}: already merged into ${baseBranch}.`); + continue; + } + + const branchWorktree = branchWorktrees.get(branch) || ''; + if (branchWorktree && hasSignificantWorkingTreeChanges(branchWorktree)) { + summary.skipped += 1; + summary.details.push(`[skip] ${branch}: dirty worktree (${branchWorktree}).`); + continue; + } + + summary.attempted += 1; + const finishArgs = [ + finishScript, + '--branch', + branch, + '--base', + baseBranch, + '--via-pr', + '--cleanup', + ]; + const finishResult = run('bash', finishArgs, { cwd: repoRoot }); + const combinedOutput = [finishResult.stdout || '', finishResult.stderr || ''].join('\n').trim(); + + if (finishResult.status === 0) { + summary.completed += 1; + summary.details.push(`[done] ${branch}: auto-finish completed.`); + continue; + } + + summary.failed += 1; + const tail = combinedOutput ? ` ${combinedOutput.split('\n').slice(-2).join(' | ')}` : ''; + summary.details.push(`[fail] ${branch}: auto-finish failed.${tail}`); + } + + return summary; +} + function ensureSetupProtectedBranches(repoRoot, dryRun) { const localUserBranches = listLocalUserBranches(repoRoot); if (localUserBranches.length === 0) { @@ -2755,6 +2979,11 @@ function doctor(rawArgs) { assertProtectedMainWriteAllowed(options, 'doctor'); const fixPayload = runFixInternal(options); const scanResult = runScanInternal({ target: options.target, json: false }); + const currentBaseBranch = currentBranchName(scanResult.repoRoot); + const autoFinishSummary = autoFinishReadyAgentBranches(scanResult.repoRoot, { + baseBranch: currentBaseBranch, + dryRun: options.dryRun, + }); const safe = scanResult.errors === 0 && scanResult.warnings === 0; const musafe = safe; @@ -2776,6 +3005,7 @@ function doctor(rawArgs) { warnings: scanResult.warnings, findings: scanResult.findings, }, + autoFinish: autoFinishSummary, }, null, 2, @@ -2787,6 +3017,16 @@ function doctor(rawArgs) { printOperations('Doctor/fix', fixPayload, options.dryRun); printScanResult(scanResult, false); + if (autoFinishSummary.enabled) { + console.log( + `[${TOOL_NAME}] Auto-finish sweep (base=${currentBaseBranch}): attempted=${autoFinishSummary.attempted}, completed=${autoFinishSummary.completed}, skipped=${autoFinishSummary.skipped}, failed=${autoFinishSummary.failed}`, + ); + for (const detail of autoFinishSummary.details) { + console.log(`[${TOOL_NAME}] ${detail}`); + } + } else if (autoFinishSummary.details.length > 0) { + console.log(`[${TOOL_NAME}] ${autoFinishSummary.details[0]}`); + } if (safe) { console.log(`[${TOOL_NAME}] ✅ Repo is fully safe.`); } else { @@ -2955,7 +3195,22 @@ function setup(rawArgs) { } const scanResult = runScanInternal({ target: options.target, json: false }); + const currentBaseBranch = currentBranchName(scanResult.repoRoot); + const autoFinishSummary = autoFinishReadyAgentBranches(scanResult.repoRoot, { + baseBranch: currentBaseBranch, + dryRun: options.dryRun, + }); printScanResult(scanResult, false); + if (autoFinishSummary.enabled) { + console.log( + `[${TOOL_NAME}] Auto-finish sweep (base=${currentBaseBranch}): attempted=${autoFinishSummary.attempted}, completed=${autoFinishSummary.completed}, skipped=${autoFinishSummary.skipped}, failed=${autoFinishSummary.failed}`, + ); + for (const detail of autoFinishSummary.details) { + console.log(`[${TOOL_NAME}] ${detail}`); + } + } else if (autoFinishSummary.details.length > 0) { + console.log(`[${TOOL_NAME}] ${autoFinishSummary.details[0]}`); + } if (scanResult.errors === 0 && scanResult.warnings === 0) { console.log(`[${TOOL_NAME}] ✅ Setup complete.`); diff --git a/test/install.test.js b/test/install.test.js index d9f69cf..6ced0b6 100644 --- a/test/install.test.js +++ b/test/install.test.js @@ -636,6 +636,86 @@ exit 1 assert.match(combinedOutput, /Merge pending review\/check policy/); }); +test('doctor auto-finishes clean pending agent branches against the current local base branch', () => { + const repoDir = initRepoOnBranch('main'); + seedCommit(repoDir); + attachOriginRemoteForBranch(repoDir, 'main'); + + let result = runNode(['setup', '--target', repoDir, '--no-global-install'], repoDir); + assert.equal(result.status, 0, result.stderr || result.stdout); + result = runCmd('git', ['add', '.'], repoDir); + assert.equal(result.status, 0, result.stderr || result.stdout); + result = runCmd('git', ['commit', '-m', 'apply gx setup'], repoDir, { + ALLOW_COMMIT_ON_PROTECTED_BRANCH: '1', + }); + assert.equal(result.status, 0, result.stderr || result.stdout); + result = runCmd('git', ['push', 'origin', 'main'], repoDir); + assert.equal(result.status, 0, result.stderr || result.stdout); + + result = runCmd( + 'bash', + ['scripts/agent-branch-start.sh', 'doctor-ready-finish', 'planner', 'main'], + repoDir, + ); + assert.equal(result.status, 0, result.stderr || result.stdout); + const readyBranch = extractCreatedBranch(result.stdout); + const readyWorktree = extractCreatedWorktree(result.stdout); + + fs.writeFileSync(path.join(readyWorktree, 'doctor-ready-finish.txt'), 'ready for finish\n', 'utf8'); + result = runCmd('git', ['add', 'doctor-ready-finish.txt'], readyWorktree); + assert.equal(result.status, 0, result.stderr || result.stdout); + result = runCmd('git', ['commit', '--no-verify', '-m', 'doctor ready branch change'], readyWorktree); + assert.equal(result.status, 0, result.stderr || result.stdout); + + const ghLogPath = path.join(repoDir, '.doctor-auto-finish-gh.log'); + const { fakePath: fakeGhPath } = createFakeGhScript(` +LOG_PATH="${ghLogPath}" +echo "$*" >> "$LOG_PATH" +if [[ "$1" == "--version" ]]; then + echo "gh version 2.0.0" + 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/doctor-auto-finish-ready" + exit 0 + fi + if [[ " $* " == *" --json state,mergedAt,url "* ]]; then + printf "OPEN\\t\\t%s\\n" "https://example.test/pr/doctor-auto-finish-ready" + 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 +`); + + result = runNodeWithEnv(['doctor', '--target', repoDir], repoDir, { + MUSAFETY_GH_BIN: fakeGhPath, + }); + assert.equal(result.status, 0, result.stderr || result.stdout); + + 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, /\[done\] agent\/planner\/.*doctor-ready-finish.*: auto-finish completed\./); + + const ghCalls = fs.readFileSync(ghLogPath, 'utf8'); + assert.match(ghCalls, /pr create/); + assert.match(ghCalls, /pr merge/); + + result = runCmd('git', ['show-ref', '--verify', '--quiet', `refs/heads/${readyBranch}`], repoDir); + assert.notEqual(result.status, 0, 'doctor auto-finish should remove local ready branch'); + result = runCmd('git', ['ls-remote', '--heads', 'origin', readyBranch], repoDir); + assert.equal(result.stdout.trim(), '', 'doctor auto-finish should remove remote ready branch'); +}); + test('setup pre-commit blocks codex session commits on non-agent branches by default', () => { const repoDir = initRepo(); diff --git a/test/metadata.test.js b/test/metadata.test.js index c83f89c..79abd59 100644 --- a/test/metadata.test.js +++ b/test/metadata.test.js @@ -58,3 +58,19 @@ test('security workflows are present and use pinned GitHub Actions SHAs', () => } } }); + +test('critical runtime helper scripts stay in sync with templates', () => { + const pairs = [ + ['templates/scripts/codex-agent.sh', 'scripts/codex-agent.sh'], + ]; + + for (const [templatePath, runtimePath] of pairs) { + const template = fs.readFileSync(path.join(repoRoot, templatePath), 'utf8'); + const runtime = fs.readFileSync(path.join(repoRoot, runtimePath), 'utf8'); + assert.equal( + runtime, + template, + `${runtimePath} diverged from ${templatePath}; run gx setup/doctor parity repair`, + ); + } +});