From 4e4236c70ccac97523f387ac57348071f40f6c06 Mon Sep 17 00:00:00 2001 From: NagyVikt Date: Mon, 13 Apr 2026 20:55:54 +0200 Subject: [PATCH] Report pending gx doctor PR state accurately When doctor runs on protected main, sandbox auto-finish can return success while leaving the PR open due branch policy. The CLI previously labeled this as "Auto-finish flow completed", which is misleading and caused confusion. This change inspects agent-branch-finish output for pending markers, reports a pending status with PR URL, and adds a regression test for blocked merge policy behavior. Constraint: Keep doctor exit behavior non-fatal when PR is created but merge is pending policy gates Rejected: Force --wait-for-merge for all doctor runs | risks long hangs and timeouts on review-gated repositories Confidence: high Scope-risk: narrow Reversibility: clean Directive: Treat agent-branch-finish pending markers as non-complete states in higher-level status messaging Tested: node --check bin/multiagent-safety.js Tested: node --test test/install.test.js Tested: npm test --- bin/multiagent-safety.js | 33 +++++++++++++++++++ test/install.test.js | 71 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 104 insertions(+) diff --git a/bin/multiagent-safety.js b/bin/multiagent-safety.js index fa37e79..8ae36aa 100755 --- a/bin/multiagent-safety.js +++ b/bin/multiagent-safety.js @@ -1069,6 +1069,19 @@ function isCommandAvailable(commandName) { return run('which', [commandName]).status === 0; } +function extractAgentBranchFinishPrUrl(output) { + const match = String(output || '').match(/\[agent-branch-finish\] PR:\s*(\S+)/); + return match ? match[1] : ''; +} + +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) || + /\[agent-branch-finish\] PR auto-merge enabled; waiting for required checks\/reviews\./.test(output) + ); +} + function finishDoctorSandboxBranch(blocked, metadata) { const finishScript = path.join(metadata.worktreePath, 'scripts', 'agent-branch-finish.sh'); if (!fs.existsSync(finishScript)) { @@ -1122,6 +1135,17 @@ function finishDoctorSandboxBranch(blocked, metadata) { }; } + const combinedOutput = `${finishResult.stdout || ''}\n${finishResult.stderr || ''}`; + if (doctorFinishFlowIsPending(combinedOutput)) { + return { + status: 'pending', + note: 'PR created and waiting for merge policy/checks', + prUrl: extractAgentBranchFinishPrUrl(combinedOutput), + stdout: finishResult.stdout || '', + stderr: finishResult.stderr || '', + }; + } + return { status: 'completed', note: 'doctor sandbox finish flow completed', @@ -1266,6 +1290,15 @@ function runDoctorInSandbox(options, blocked) { console.log(`[${TOOL_NAME}] Auto-finish flow completed for sandbox branch '${metadata.branch}'.`); if (finishResult.stdout) process.stdout.write(finishResult.stdout); if (finishResult.stderr) process.stderr.write(finishResult.stderr); + } else if (finishResult.status === 'pending') { + console.log( + `[${TOOL_NAME}] Auto-finish pending for sandbox branch '${metadata.branch}': ${finishResult.note}.`, + ); + if (finishResult.prUrl) { + console.log(`[${TOOL_NAME}] PR: ${finishResult.prUrl}`); + } + if (finishResult.stdout) process.stdout.write(finishResult.stdout); + 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}'.`); if (finishResult.stdout) process.stdout.write(finishResult.stdout); diff --git a/test/install.test.js b/test/install.test.js index 14b56d3..0ae0631 100644 --- a/test/install.test.js +++ b/test/install.test.js @@ -533,6 +533,77 @@ exit 1 assert.equal(rootStatus.stdout.trim(), '', 'protected main checkout should stay clean'); }); +test('doctor on protected main reports auto-finish pending when PR merge policy blocks immediate merge', () => { + 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); + + fs.rmSync(path.join(repoDir, 'AGENTS.md')); + result = runCmd('git', ['add', '-A'], repoDir, { + ALLOW_COMMIT_ON_PROTECTED_BRANCH: '1', + }); + assert.equal(result.status, 0, result.stderr || result.stdout); + result = runCmd('git', ['commit', '-m', 'simulate drift remove agents'], 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); + + const ghLogPath = path.join(repoDir, 'gh-calls-pending.log'); + const { fakePath: fakeGhPath } = createFakeGhScript(` +echo "$*" >> "${ghLogPath}" +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/doctor-autofinish-pending" + exit 0 + fi + echo "unexpected gh pr view args: $*" >&2 + exit 1 +fi +if [[ "$1" == "pr" && "$2" == "merge" ]]; then + if [[ " $* " == *" --auto "* ]]; then + echo "GraphQL: Pull request Auto merge is not allowed for this repository (enablePullRequestAutoMerge)" >&2 + exit 1 + fi + echo "X Pull request recodeecom/musafety#999 is not mergeable: the base branch policy prohibits the merge." >&2 + echo "To have the pull request merged after all the requirements have been met, add the --auto flag." >&2 + exit 1 +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 ghCalls = fs.readFileSync(ghLogPath, 'utf8'); + assert.match(ghCalls, /pr merge/); + assert.match(ghCalls, /pr merge .* --auto/); + const combinedOutput = `${result.stdout}\n${result.stderr}`; + assert.match(combinedOutput, /\[guardex\] Auto-finish pending for sandbox branch/); + assert.match(combinedOutput, /PR: https:\/\/example\.test\/pr\/doctor-autofinish-pending/); + assert.doesNotMatch(combinedOutput, /Auto-finish flow completed for sandbox branch/); + assert.match(combinedOutput, /Merge pending review\/check policy/); +}); + test('setup pre-commit blocks codex session commits on non-agent branches by default', () => { const repoDir = initRepo();