From ef9320d26747417220b09d65b67a428b768749e0 Mon Sep 17 00:00:00 2001 From: NagyVikt Date: Mon, 13 Apr 2026 23:04:05 +0200 Subject: [PATCH] Prevent gx doctor from hijacking the active checkout during sandbox startup Some repos may still carry a legacy or custom scripts/agent-branch-start.sh that performs in-place branch checkout. doctor now treats that as unsafe, restores the protected base branch when needed, and falls back to internal git worktree sandbox creation so the user-visible checkout stays on main. Constraint: doctor must preserve the protected base checkout while still repairing drift automatically Rejected: Trust repo-local agent-branch-start output unconditionally | can switch the base worktree to an agent branch Confidence: high Scope-risk: narrow Reversibility: clean Directive: Keep doctor sandbox startup tolerant of legacy starter scripts and always verify base checkout remained unchanged Tested: node --check bin/multiagent-safety.js; node --test test/install.test.js; npm test --- bin/multiagent-safety.js | 47 ++++++++++++++++++++++++++++++++++++--- test/install.test.js | 48 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 92 insertions(+), 3 deletions(-) diff --git a/bin/multiagent-safety.js b/bin/multiagent-safety.js index 977b22a..9ac521b 100755 --- a/bin/multiagent-safety.js +++ b/bin/multiagent-safety.js @@ -844,6 +844,33 @@ function isSpawnFailure(result) { return Boolean(result?.error) && typeof result?.status !== 'number'; } +function ensureRepoBranch(repoRoot, branch) { + const current = currentBranchName(repoRoot); + if (current === branch) { + return { ok: true, changed: false }; + } + + const checkoutResult = run('git', ['-C', repoRoot, 'checkout', branch], { timeout: 20_000 }); + if (isSpawnFailure(checkoutResult)) { + return { + ok: false, + changed: false, + stdout: checkoutResult.stdout || '', + stderr: checkoutResult.stderr || '', + }; + } + if (checkoutResult.status !== 0) { + return { + ok: false, + changed: false, + stdout: checkoutResult.stdout || '', + stderr: checkoutResult.stderr || '', + }; + } + + return { ok: true, changed: true }; +} + function doctorSandboxBranchPrefix() { const now = new Date(); const stamp = [ @@ -945,12 +972,26 @@ function startDoctorSandbox(blocked) { throw startResult.error; } if (startResult.status !== 0) { - throw new Error((startResult.stderr || startResult.stdout || 'failed to start doctor sandbox').trim()); + return startDoctorSandboxFallback(blocked); } const metadata = extractAgentBranchStartMetadata(startResult.stdout); - if (!metadata.worktreePath) { - throw new Error(`Failed to parse sandbox worktree from agent-branch-start output:\n${startResult.stdout}`); + const currentBranch = currentBranchName(blocked.repoRoot); + const worktreePath = metadata.worktreePath ? path.resolve(metadata.worktreePath) : ''; + const repoRootPath = path.resolve(blocked.repoRoot); + const hasSafeWorktree = Boolean(worktreePath) && worktreePath !== repoRootPath; + const branchChanged = Boolean(currentBranch) && currentBranch !== blocked.branch; + + if (!hasSafeWorktree || branchChanged) { + const restoreResult = ensureRepoBranch(blocked.repoRoot, blocked.branch); + if (!restoreResult.ok) { + const detail = [restoreResult.stderr, restoreResult.stdout].filter(Boolean).join('\n').trim(); + throw new Error( + `doctor sandbox startup switched protected base checkout and could not restore '${blocked.branch}'.` + + (detail ? `\n${detail}` : ''), + ); + } + return startDoctorSandboxFallback(blocked); } return { diff --git a/test/install.test.js b/test/install.test.js index d9f69cf..5c17a94 100644 --- a/test/install.test.js +++ b/test/install.test.js @@ -426,6 +426,54 @@ test('doctor on protected main auto-runs in a sandbox branch/worktree', () => { assert.equal(currentBranch.stdout.trim(), 'main'); }); +test('doctor keeps protected base checkout on main even if local starter script switches branches in-place', () => { + 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 = 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); + + const legacyStartScript = path.join(repoDir, 'scripts', 'agent-branch-start.sh'); + fs.writeFileSync( + legacyStartScript, + '#!/usr/bin/env bash\n' + + 'set -euo pipefail\n' + + 'branch_name="agent/legacy/doctor-in-place"\n' + + 'git checkout -B "$branch_name"\n' + + 'echo "[agent-branch-start] Created in-place branch: ${branch_name}"\n', + 'utf8', + ); + fs.chmodSync(legacyStartScript, 0o755); + + result = runCmd('git', ['add', '-f', 'scripts/agent-branch-start.sh'], repoDir); + assert.equal(result.status, 0, result.stderr || result.stdout); + result = runCmd('git', ['commit', '-m', 'simulate legacy in-place starter'], 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 = runNode(['doctor', '--target', repoDir], repoDir); + assert.equal(result.status, 0, result.stderr || result.stdout); + assert.match(result.stdout, /doctor detected protected branch 'main'/); + assert.match(extractCreatedBranch(result.stdout), /^agent\/gx\/.+-gx-doctor$/); + + const currentBranch = runCmd('git', ['branch', '--show-current'], repoDir); + assert.equal(currentBranch.status, 0, currentBranch.stderr || currentBranch.stdout); + assert.equal(currentBranch.stdout.trim(), 'main'); +}); + test('doctor on protected main syncs repaired stale lock state back to base workspace', () => { const repoDir = initRepoOnBranch('main'); seedCommit(repoDir);