Prevent gx doctor sandbox branches from lingering after merge

openspec/changes/agent-codex-fix-doctor-sandbox-cleanup-2026-04-23-17-49/notes.md

@@ -0,0 +1,14 @@
+# agent-codex-fix-doctor-sandbox-cleanup-2026-04-23-17-49 (minimal / T1)
+
+Branch: `agent/codex/fix-doctor-sandbox-cleanup-2026-04-23-17-49`
+
+`gx doctor` already merges protected-branch sandbox repairs through `gx branch finish --cleanup`, but the doctor flow runs that finisher from inside the sandbox worktree. That leaves the just-merged doctor sandbox attached as the active cwd, so the finisher cannot prune the worktree even though the merge succeeded.
+
+Scope:
+- Run the doctor sandbox finish flow from the protected repo root instead of the sandbox cwd.
+- Verify successful doctor auto-finish leaves no surviving local sandbox branch/worktree.
+- Add a focused regression that proves the sandbox worktree path disappears after merge cleanup.
+
+Verification:
+- `node --test test/doctor.test.js --test-name-pattern "doctor on protected main auto-commits sandbox repairs and runs PR finish flow when gh is authenticated"`
+- `node --test test/doctor.test.js --test-name-pattern "doctor on protected main fails when sandbox PR is not merged"`

src/doctor/index.js

@@ -314,6 +314,118 @@ function doctorFinishFlowIsPending(output) {
   );
 }
 
+function verifyDoctorSandboxCleanup(repoRoot, metadata) {
+  const branchExists = Boolean(metadata.branch) && gitRefExists(repoRoot, `refs/heads/${metadata.branch}`);
+  const worktreeExists = Boolean(metadata.worktreePath) && fs.existsSync(metadata.worktreePath);
+
+  if (!branchExists && !worktreeExists) {
+    return {
+      status: 'verified',
+      note: 'doctor sandbox cleanup verified',
+    };
+  }
+
+  const cleanup = cleanupProtectedBaseSandbox(repoRoot, metadata);
+  const branchStillExists = Boolean(metadata.branch) && gitRefExists(repoRoot, `refs/heads/${metadata.branch}`);
+  const worktreeStillExists = Boolean(metadata.worktreePath) && fs.existsSync(metadata.worktreePath);
+  if (branchStillExists || worktreeStillExists) {
+    return {
+      status: 'failed',
+      note:
+        'doctor sandbox cleanup incomplete ' +
+        `(branch=${branchStillExists ? 'present' : 'missing'}, worktree=${worktreeStillExists ? 'present' : 'missing'})`,
+      cleanup,
+    };
+  }
+
+  return {
+    status: 'verified',
+    note: 'doctor sandbox cleanup verified',
+    cleanup,
+  };
+}
+
+function verifyDoctorSandboxRemoteCleanup(repoRoot, metadata) {
+  if (!metadata.branch || !hasOriginRemote(repoRoot)) {
+    return {
+      status: 'skipped',
+      note: 'doctor sandbox remote cleanup skipped',
+    };
+  }
+
+  const remoteBefore = run(
+    'git',
+    ['-C', repoRoot, 'ls-remote', '--heads', 'origin', metadata.branch],
+    { timeout: 20_000 },
+  );
+  if (isSpawnFailure(remoteBefore)) {
+    throw remoteBefore.error;
+  }
+  if (remoteBefore.status !== 0) {
+    return {
+      status: 'failed',
+      note: 'doctor sandbox remote branch inspection failed',
+      stdout: remoteBefore.stdout || '',
+      stderr: remoteBefore.stderr || '',
+    };
+  }
+  if (!String(remoteBefore.stdout || '').trim()) {
+    return {
+      status: 'verified',
+      note: 'doctor sandbox remote cleanup verified',
+    };
+  }
+
+  const deleteResult = run(
+    'git',
+    ['-C', repoRoot, 'push', 'origin', '--delete', metadata.branch],
+    { timeout: 30_000 },
+  );
+  if (isSpawnFailure(deleteResult)) {
+    throw deleteResult.error;
+  }
+  if (deleteResult.status !== 0) {
+    return {
+      status: 'failed',
+      note: 'doctor sandbox remote branch cleanup failed',
+      stdout: deleteResult.stdout || '',
+      stderr: deleteResult.stderr || '',
+    };
+  }
+
+  const remoteAfter = run(
+    'git',
+    ['-C', repoRoot, 'ls-remote', '--heads', 'origin', metadata.branch],
+    { timeout: 20_000 },
+  );
+  if (isSpawnFailure(remoteAfter)) {
+    throw remoteAfter.error;
+  }
+  if (remoteAfter.status !== 0) {
+    return {
+      status: 'failed',
+      note: 'doctor sandbox remote cleanup recheck failed',
+      stdout: remoteAfter.stdout || '',
+      stderr: remoteAfter.stderr || '',
+    };
+  }
+  if (String(remoteAfter.stdout || '').trim()) {
+    return {
+      status: 'failed',
+      note: 'doctor sandbox remote branch still present after cleanup',
+      stdout: remoteAfter.stdout || '',
+      stderr: remoteAfter.stderr || '',
+    };
+  }
+
+  return {
+    status: 'verified',
+    note: 'doctor sandbox remote cleanup verified',
+    stdout: deleteResult.stdout || '',
+    stderr: deleteResult.stderr || '',
+  };
+}
+
 function finishDoctorSandboxBranch(blocked, metadata, options = {}) {
   if (!hasOriginRemote(blocked.repoRoot)) {
     return {
@@ -353,8 +465,8 @@ function finishDoctorSandboxBranch(blocked, metadata, options = {}) {
 
   const finishResult = runPackageAsset(
     'branchFinish',
-    ['--branch', metadata.branch, '--base', blocked.branch, '--via-pr', waitForMergeArg, '--cleanup'],
-    { cwd: metadata.worktreePath, timeout: finishTimeoutMs },
+    ['--branch', metadata.branch, '--base', blocked.branch, '--via-pr', waitForMergeArg, '--no-cleanup'],
+    { cwd: blocked.repoRoot, timeout: finishTimeoutMs },
   );
   if (isSpawnFailure(finishResult)) {
     return {
@@ -384,11 +496,52 @@ function finishDoctorSandboxBranch(blocked, metadata, options = {}) {
     };
   }
 
+  let cleanupVerification;
+  try {
+    cleanupVerification = verifyDoctorSandboxCleanup(blocked.repoRoot, metadata);
+  } catch (error) {
+    return {
+      status: 'failed',
+      note: `doctor sandbox cleanup verification failed: ${error.message}`,
+      stdout: finishResult.stdout || '',
+      stderr: finishResult.stderr || '',
+    };
+  }
+  if (cleanupVerification.status === 'failed') {
+    return {
+      status: 'failed',
+      note: cleanupVerification.note,
+      stdout: finishResult.stdout || '',
+      stderr: finishResult.stderr || '',
+    };
+  }
+
+  let remoteCleanupVerification;
+  try {
+    remoteCleanupVerification = verifyDoctorSandboxRemoteCleanup(blocked.repoRoot, metadata);
+  } catch (error) {
+    return {
+      status: 'failed',
+      note: `doctor sandbox remote cleanup verification failed: ${error.message}`,
+      stdout: finishResult.stdout || '',
+      stderr: finishResult.stderr || '',
+    };
+  }
+  if (remoteCleanupVerification.status === 'failed') {
+    return {
+      status: 'failed',
+      note: remoteCleanupVerification.note,
+      stdout: [finishResult.stdout || '', remoteCleanupVerification.stdout || ''].filter(Boolean).join('\n'),
+      stderr: [finishResult.stderr || '', remoteCleanupVerification.stderr || ''].filter(Boolean).join('\n'),
+    };
+  }
+
   return {
     status: 'completed',
-    note: 'doctor sandbox finish flow completed',
-    stdout: finishResult.stdout || '',
-    stderr: finishResult.stderr || '',
+    note: 'doctor sandbox finish flow completed and cleanup verified',
+    stdout: [finishResult.stdout || '', remoteCleanupVerification.stdout || ''].filter(Boolean).join('\n'),
+    stderr: [finishResult.stderr || '', remoteCleanupVerification.stderr || ''].filter(Boolean).join('\n'),
+    cleanup: cleanupVerification.cleanup,
   };
 }
 

test/doctor.test.js

@@ -353,6 +353,7 @@ exit 1
 
   result = runNodeWithEnv(['doctor', '--target', repoDir], repoDir, { GUARDEX_GH_BIN: fakeGhPath });
   assert.equal(result.status, 0, result.stderr || result.stdout);
+  const doctorOutput = `${result.stdout}\n${result.stderr}`;
   assert.match(result.stdout, /Auto-committed doctor repairs in sandbox branch/);
   assert.match(result.stdout, /Auto-finish flow completed for sandbox branch/);
   assert.equal(
@@ -364,10 +365,28 @@ exit 1
   assertZeroCopyManagedGitignore(repairedRootGitignore);
 
   const createdBranch = extractCreatedBranch(result.stdout);
+  const createdWorktree = extractCreatedWorktree(result.stdout);
   result = runCmd('git', ['show-ref', '--verify', '--quiet', `refs/heads/${createdBranch}`], repoDir);
   assert.notEqual(result.status, 0, 'doctor auto-finish should clean up the merged sandbox branch locally by default');
   result = runCmd('git', ['ls-remote', '--heads', 'origin', createdBranch], repoDir);
   assert.equal(result.stdout.trim(), '', 'doctor auto-finish should clean up the merged sandbox branch remotely by default');
+  assert.equal(
+    fs.existsSync(createdWorktree),
+    false,
+    'doctor auto-finish should remove the sandbox worktree after merge cleanup succeeds',
+  );
+  const worktreeList = runCmd('git', ['worktree', 'list', '--porcelain'], repoDir);
+  assert.equal(worktreeList.status, 0, worktreeList.stderr || worktreeList.stdout);
+  assert.doesNotMatch(
+    worktreeList.stdout,
+    new RegExp(`worktree ${escapeRegexLiteral(createdWorktree)}`),
+    'doctor auto-finish should not leave the sandbox worktree registered after cleanup',
+  );
+  assert.doesNotMatch(
+    doctorOutput,
+    /Current worktree '.*' still exists because it is the active shell cwd/,
+    'doctor should not finish from inside the sandbox cwd anymore',
+  );
 
   const rootStatus = runCmd('git', ['status', '--short', '--untracked-files=no'], repoDir);
   assert.equal(rootStatus.status, 0, rootStatus.stderr || rootStatus.stdout);