From 7d116f9e61e2e6970882185d9ec5baca7c5ca3d1 Mon Sep 17 00:00:00 2001 From: NagyVikt Date: Wed, 22 Apr 2026 15:40:24 +0200 Subject: [PATCH] Keep doctor source probes disposable instead of sticky Stale __source-probe worktrees were being reused as if they were the branch's real worktree, which let leaked probes block finish and surface as fake repos in editor repo scans. This patch removes stale probes before finish, treats probe paths as temporary during prune even when they track agent branches, and rewrites doctor conflict guidance so operators rebase the real branch in a normal worktree. Constraint: gx doctor and gx branch finish still need a temporary probe when no live branch worktree exists Rejected: Keep telling users to continue rebases inside __source-probe worktrees | conflicts with the cleanup contract and leaves fake repos behind Confidence: high Scope-risk: moderate Reversibility: clean Directive: Treat __source-probe-* paths as disposable plumbing; do not promote them to operator-facing workflow state again Tested: bash -n scripts/agent-branch-finish.sh scripts/agent-worktree-prune.sh templates/scripts/agent-branch-finish.sh templates/scripts/agent-worktree-prune.sh; node --test test/doctor.test.js test/worktree.test.js test/finish.test.js Not-tested: gx branch finish --via-pr --wait-for-merge against live GitHub --- .../notes.md | 21 +++++++++ scripts/agent-branch-finish.sh | 43 ++++++++++++++++-- scripts/agent-worktree-prune.sh | 16 ++++++- src/output/index.js | 9 +++- templates/scripts/agent-branch-finish.sh | 43 ++++++++++++++++-- templates/scripts/agent-worktree-prune.sh | 16 ++++++- test/doctor.test.js | 9 ++-- test/finish.test.js | 44 +++++++++++++++++++ test/worktree.test.js | 32 ++++++++++++++ 9 files changed, 219 insertions(+), 14 deletions(-) create mode 100644 openspec/changes/agent-codex-fix-doctor-source-probe-stale-worktree-2026-04-22-15-33/notes.md diff --git a/openspec/changes/agent-codex-fix-doctor-source-probe-stale-worktree-2026-04-22-15-33/notes.md b/openspec/changes/agent-codex-fix-doctor-source-probe-stale-worktree-2026-04-22-15-33/notes.md new file mode 100644 index 0000000..a6457f5 --- /dev/null +++ b/openspec/changes/agent-codex-fix-doctor-source-probe-stale-worktree-2026-04-22-15-33/notes.md @@ -0,0 +1,21 @@ +# agent-codex-fix-doctor-source-probe-stale-worktree-2026-04-22-15-33 (minimal / T1) + +Branch: `agent/codex/fix-doctor-source-probe-stale-worktree-2026-04-22-15-33` + +`gx doctor` can surface or reuse leaked `__source-probe-*` worktrees during the auto-finish sweep. That makes VS Code Source Control show a fake extra repo, can block `gx branch finish` on a dirty probe, and the compact doctor copy wrongly implies the throwaway probe is the place to keep rebasing. + +Scope: +- Treat `__source-probe-*` paths as temporary worktrees even when they track an `agent/*` branch. +- Remove stale source-probe worktrees before `agent-branch-finish.sh` creates a fresh probe for the branch. +- Update doctor conflict copy so the operator rebases the real branch in a normal worktree instead of treating the probe as durable state. +- Add focused regressions for doctor output, stale-probe finish recovery, and worktree prune behavior. + +Verification: +- `bash -n scripts/agent-branch-finish.sh scripts/agent-worktree-prune.sh templates/scripts/agent-branch-finish.sh templates/scripts/agent-worktree-prune.sh` +- `node --test test/doctor.test.js test/worktree.test.js test/finish.test.js` + +## Cleanup + +- [ ] Run `gx branch finish --branch agent/codex/fix-doctor-source-probe-stale-worktree-2026-04-22-15-33 --base main --via-pr --wait-for-merge --cleanup` +- [ ] Record PR URL + `MERGED` state in the completion handoff. +- [ ] Confirm sandbox worktree is gone (`git worktree list`, `git branch -a`). diff --git a/scripts/agent-branch-finish.sh b/scripts/agent-branch-finish.sh index fa67866..d086624 100755 --- a/scripts/agent-branch-finish.sh +++ b/scripts/agent-branch-finish.sh @@ -218,18 +218,48 @@ fi get_worktree_for_branch() { local branch="$1" - git -C "$repo_root" worktree list --porcelain | awk -v target="refs/heads/${branch}" ' + git -C "$repo_root" worktree list --porcelain | awk -v target="refs/heads/${branch}" -v probe_prefix="${agent_worktree_root}/__source-probe-" ' $1 == "worktree" { wt = $2 } - $1 == "branch" && $2 == target { print wt; exit } + $1 == "branch" && $2 == target { + if (index(wt, probe_prefix) != 1) { + print wt + exit + } + } ' } +remove_stale_source_probe_worktrees() { + local branch="$1" + local stale_probe="" + + while IFS= read -r stale_probe; do + [[ -z "$stale_probe" ]] && continue + [[ "$stale_probe" == "$current_worktree" ]] && continue + + echo "[agent-branch-finish] Removing stale source-probe worktree for '${branch}': ${stale_probe}" >&2 + git -C "$stale_probe" rebase --abort >/dev/null 2>&1 || true + git -C "$stale_probe" merge --abort >/dev/null 2>&1 || true + git -C "$repo_root" worktree remove "$stale_probe" --force >/dev/null 2>&1 || true + done < <( + git -C "$repo_root" worktree list --porcelain | awk -v target="refs/heads/${branch}" -v probe_prefix="${agent_worktree_root}/__source-probe-" ' + $1 == "worktree" { wt = $2 } + $1 == "branch" && $2 == target { + if (index(wt, probe_prefix) == 1) { + print wt + } + } + ' + ) +} + is_clean_worktree() { local wt="$1" git -C "$wt" diff --quiet -- . ":(exclude).omx/state/agent-file-locks.json" \ && git -C "$wt" diff --cached --quiet -- . ":(exclude).omx/state/agent-file-locks.json" } +remove_stale_source_probe_worktrees "$SOURCE_BRANCH" source_worktree="$(get_worktree_for_branch "$SOURCE_BRANCH")" created_source_probe=0 source_probe_path="" @@ -295,8 +325,13 @@ if [[ "$should_require_sync" -eq 1 ]] && git -C "$repo_root" show-ref --verify - echo "[agent-sync-guard] Auto-sync failed while rebasing '${SOURCE_BRANCH}' onto origin/${BASE_BRANCH}." >&2 if [[ "$rebase_active" -eq 1 ]]; then - echo "[agent-sync-guard] Resolve conflicts, then run: git -C \"$source_worktree\" rebase --continue" >&2 - echo "[agent-sync-guard] Or abort: git -C \"$source_worktree\" rebase --abort" >&2 + if [[ "$created_source_probe" -eq 1 ]]; then + echo "[agent-sync-guard] Temporary source-probe worktree will be cleaned up on exit." >&2 + echo "[agent-sync-guard] Reattach '${SOURCE_BRANCH}' in a regular worktree, then rebase it onto origin/${BASE_BRANCH} manually." >&2 + else + echo "[agent-sync-guard] Resolve conflicts, then run: git -C \"$source_worktree\" rebase --continue" >&2 + echo "[agent-sync-guard] Or abort: git -C \"$source_worktree\" rebase --abort" >&2 + fi fi exit 1 fi diff --git a/scripts/agent-worktree-prune.sh b/scripts/agent-worktree-prune.sh index c967140..1145fdc 100755 --- a/scripts/agent-worktree-prune.sh +++ b/scripts/agent-worktree-prune.sh @@ -111,6 +111,13 @@ is_managed_worktree_path() { return 1 } +is_temporary_worktree_path() { + local entry="$1" + local name + name="$(basename "$entry")" + [[ "$name" == __agent_integrate-* || "$name" == __source-probe-* ]] +} + resolve_base_branch() { local configured="" local current="" @@ -425,7 +432,9 @@ process_entry() { local remove_reason="" local branch_delete_mode="safe" - if [[ -z "$branch_ref" ]]; then + if is_temporary_worktree_path "$wt"; then + remove_reason="temporary-worktree" + elif [[ -z "$branch_ref" ]]; then remove_reason="detached-worktree" elif ! git -C "$repo_root" show-ref --verify --quiet "refs/heads/${branch}"; then remove_reason="missing-branch" @@ -452,6 +461,11 @@ process_entry() { return fi + if [[ "$remove_reason" == "temporary-worktree" ]]; then + git -C "$wt" rebase --abort >/dev/null 2>&1 || true + git -C "$wt" merge --abort >/dev/null 2>&1 || true + fi + if [[ "$FORCE_DIRTY" -ne 1 ]] && ! is_clean_worktree "$wt"; then skipped_dirty=$((skipped_dirty + 1)) echo "[agent-worktree-prune] Skipping dirty worktree (${remove_reason}): ${wt}" diff --git a/src/output/index.js b/src/output/index.js index 8d43f57..133a99e 100644 --- a/src/output/index.js +++ b/src/output/index.js @@ -281,7 +281,14 @@ function detectRecoverableAutoFinishConflict(message) { if (/rebase --continue/i.test(text) && /rebase --abort/i.test(text)) { return { rawLabel: 'auto-finish requires manual rebase.', - summary: 'manual rebase required in the source-probe worktree; run rebase --continue or rebase --abort', + summary: 'manual rebase required on the branch before auto-finish can continue', + }; + } + + if (/Reattach '.+' in a regular worktree, then rebase it onto origin\/.+ manually\./i.test(text)) { + return { + rawLabel: 'auto-finish requires manual rebase.', + summary: 'manual rebase required on the branch before auto-finish can continue', }; } diff --git a/templates/scripts/agent-branch-finish.sh b/templates/scripts/agent-branch-finish.sh index fa67866..d086624 100755 --- a/templates/scripts/agent-branch-finish.sh +++ b/templates/scripts/agent-branch-finish.sh @@ -218,18 +218,48 @@ fi get_worktree_for_branch() { local branch="$1" - git -C "$repo_root" worktree list --porcelain | awk -v target="refs/heads/${branch}" ' + git -C "$repo_root" worktree list --porcelain | awk -v target="refs/heads/${branch}" -v probe_prefix="${agent_worktree_root}/__source-probe-" ' $1 == "worktree" { wt = $2 } - $1 == "branch" && $2 == target { print wt; exit } + $1 == "branch" && $2 == target { + if (index(wt, probe_prefix) != 1) { + print wt + exit + } + } ' } +remove_stale_source_probe_worktrees() { + local branch="$1" + local stale_probe="" + + while IFS= read -r stale_probe; do + [[ -z "$stale_probe" ]] && continue + [[ "$stale_probe" == "$current_worktree" ]] && continue + + echo "[agent-branch-finish] Removing stale source-probe worktree for '${branch}': ${stale_probe}" >&2 + git -C "$stale_probe" rebase --abort >/dev/null 2>&1 || true + git -C "$stale_probe" merge --abort >/dev/null 2>&1 || true + git -C "$repo_root" worktree remove "$stale_probe" --force >/dev/null 2>&1 || true + done < <( + git -C "$repo_root" worktree list --porcelain | awk -v target="refs/heads/${branch}" -v probe_prefix="${agent_worktree_root}/__source-probe-" ' + $1 == "worktree" { wt = $2 } + $1 == "branch" && $2 == target { + if (index(wt, probe_prefix) == 1) { + print wt + } + } + ' + ) +} + is_clean_worktree() { local wt="$1" git -C "$wt" diff --quiet -- . ":(exclude).omx/state/agent-file-locks.json" \ && git -C "$wt" diff --cached --quiet -- . ":(exclude).omx/state/agent-file-locks.json" } +remove_stale_source_probe_worktrees "$SOURCE_BRANCH" source_worktree="$(get_worktree_for_branch "$SOURCE_BRANCH")" created_source_probe=0 source_probe_path="" @@ -295,8 +325,13 @@ if [[ "$should_require_sync" -eq 1 ]] && git -C "$repo_root" show-ref --verify - echo "[agent-sync-guard] Auto-sync failed while rebasing '${SOURCE_BRANCH}' onto origin/${BASE_BRANCH}." >&2 if [[ "$rebase_active" -eq 1 ]]; then - echo "[agent-sync-guard] Resolve conflicts, then run: git -C \"$source_worktree\" rebase --continue" >&2 - echo "[agent-sync-guard] Or abort: git -C \"$source_worktree\" rebase --abort" >&2 + if [[ "$created_source_probe" -eq 1 ]]; then + echo "[agent-sync-guard] Temporary source-probe worktree will be cleaned up on exit." >&2 + echo "[agent-sync-guard] Reattach '${SOURCE_BRANCH}' in a regular worktree, then rebase it onto origin/${BASE_BRANCH} manually." >&2 + else + echo "[agent-sync-guard] Resolve conflicts, then run: git -C \"$source_worktree\" rebase --continue" >&2 + echo "[agent-sync-guard] Or abort: git -C \"$source_worktree\" rebase --abort" >&2 + fi fi exit 1 fi diff --git a/templates/scripts/agent-worktree-prune.sh b/templates/scripts/agent-worktree-prune.sh index c967140..1145fdc 100644 --- a/templates/scripts/agent-worktree-prune.sh +++ b/templates/scripts/agent-worktree-prune.sh @@ -111,6 +111,13 @@ is_managed_worktree_path() { return 1 } +is_temporary_worktree_path() { + local entry="$1" + local name + name="$(basename "$entry")" + [[ "$name" == __agent_integrate-* || "$name" == __source-probe-* ]] +} + resolve_base_branch() { local configured="" local current="" @@ -425,7 +432,9 @@ process_entry() { local remove_reason="" local branch_delete_mode="safe" - if [[ -z "$branch_ref" ]]; then + if is_temporary_worktree_path "$wt"; then + remove_reason="temporary-worktree" + elif [[ -z "$branch_ref" ]]; then remove_reason="detached-worktree" elif ! git -C "$repo_root" show-ref --verify --quiet "refs/heads/${branch}"; then remove_reason="missing-branch" @@ -452,6 +461,11 @@ process_entry() { return fi + if [[ "$remove_reason" == "temporary-worktree" ]]; then + git -C "$wt" rebase --abort >/dev/null 2>&1 || true + git -C "$wt" merge --abort >/dev/null 2>&1 || true + fi + if [[ "$FORCE_DIRTY" -ne 1 ]] && ! is_clean_worktree "$wt"; then skipped_dirty=$((skipped_dirty + 1)) echo "[agent-worktree-prune] Skipping dirty worktree (${remove_reason}): ${wt}" diff --git a/test/doctor.test.js b/test/doctor.test.js index 1db8497..44a071f 100644 --- a/test/doctor.test.js +++ b/test/doctor.test.js @@ -616,7 +616,7 @@ exit 1 assert.match( compactOutput, new RegExp( - `\\[skip\\] ${escapeRegexLiteral(readyBranch)}: manual rebase required in the source-probe worktree; run rebase --continue or rebase --abort`, + `\\[skip\\] ${escapeRegexLiteral(readyBranch)}: manual rebase required on the branch before auto-finish can continue`, ), ); assert.doesNotMatch(compactOutput, /git -C "\/tmp\/very\/long\/path\/for\/source-probe-agent-worktree/); @@ -629,7 +629,10 @@ exit 1 assert.equal(result.status, 0, result.stderr || result.stdout); const verboseOutput = `${result.stdout}\n${result.stderr}`; assert.match(verboseOutput, new RegExp(`\\[skip\\] ${escapeRegexLiteral(readyBranch)}: auto-finish requires manual rebase\\.`)); - assert.match(verboseOutput, /git -C ".+rebase --continue/); + assert.match( + verboseOutput, + new RegExp(`Reattach '${escapeRegexLiteral(readyBranch)}' in a regular worktree, then rebase it onto origin/main manually\\.`), + ); }); @@ -680,7 +683,7 @@ exit 1 assert.match( ansiOutput, new RegExp( - `\\u001B\\[33m\\[gitguardex\\]\\s+\\[skip\\] ${escapeRegexLiteral(readyBranch)}: manual rebase required in the source-probe worktree; run rebase --continue or rebase --abort\\u001B\\[0m`, + `\\u001B\\[33m\\[gitguardex\\]\\s+\\[skip\\] ${escapeRegexLiteral(readyBranch)}: manual rebase required on the branch before auto-finish can continue\\u001B\\[0m`, ), ); assert.match(ansiOutput, /\u001B\[32m\[gitguardex\] ✅ Repo is fully safe\.\u001B\[0m/); diff --git a/test/finish.test.js b/test/finish.test.js index 4fae6f7..871fc5f 100644 --- a/test/finish.test.js +++ b/test/finish.test.js @@ -198,6 +198,50 @@ test('agent-branch-finish auto-syncs source branch when behind origin/dev', () = }); +test('agent-branch-finish removes stale source-probe worktrees before creating a fresh probe', () => { + const repoDir = initRepo(); + seedCommit(repoDir); + attachOriginRemote(repoDir); + + 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', 'dev'], repoDir); + assert.equal(result.status, 0, result.stderr || result.stdout); + + result = runCmd('git', ['checkout', '-b', 'agent/test-stale-source-probe'], repoDir); + assert.equal(result.status, 0, result.stderr || result.stdout); + commitFile(repoDir, 'agent-stale-source-probe.txt', 'agent branch change\n', 'agent branch change'); + + result = runCmd('git', ['checkout', 'dev'], repoDir); + assert.equal(result.status, 0, result.stderr || result.stdout); + + const sourceProbePath = path.join( + repoDir, + '.omx', + 'agent-worktrees', + '__source-probe-agent__test-stale-source-probe-20260422-153300', + ); + result = runCmd('git', ['worktree', 'add', sourceProbePath, 'agent/test-stale-source-probe'], repoDir); + assert.equal(result.status, 0, result.stderr || result.stdout); + fs.writeFileSync(path.join(sourceProbePath, 'agent-stale-source-probe.txt'), 'stale probe dirty change\n', 'utf8'); + + const finish = runBranchFinish(['--branch', 'agent/test-stale-source-probe'], repoDir); + assert.equal(finish.status, 0, finish.stderr || finish.stdout); + assert.match(finish.stderr, /Removing stale source-probe worktree for 'agent\/test-stale-source-probe'/); + assert.equal(fs.existsSync(sourceProbePath), false, 'stale source-probe worktree should be removed before finish continues'); + assert.match( + finish.stdout, + /Merged 'agent\/test-stale-source-probe' into 'dev' via direct flow and kept source branch\/worktree\./, + ); +}); + + test('agent-branch-finish pr mode continues cleanup when gh merge only fails local branch deletion', () => { const repoDir = initRepo(); seedCommit(repoDir); diff --git a/test/worktree.test.js b/test/worktree.test.js index 31fa8b3..c91830a 100644 --- a/test/worktree.test.js +++ b/test/worktree.test.js @@ -135,6 +135,38 @@ test('worktree prune --only-dirty-worktrees removes clean agent worktrees but ke }); +test('worktree prune removes __source-probe worktrees even when they track agent branches', () => { + const repoDir = initRepo(); + let result = runNode(['setup', '--target', repoDir, '--no-global-install'], repoDir); + assert.equal(result.status, 0, result.stderr || result.stdout); + seedCommit(repoDir); + + result = runCmd('git', ['checkout', '-b', 'agent/test-source-probe-prune'], repoDir); + assert.equal(result.status, 0, result.stderr || result.stdout); + commitFile(repoDir, 'source-probe-prune.txt', 'agent branch change\n', 'agent branch change'); + + result = runCmd('git', ['checkout', 'dev'], repoDir); + assert.equal(result.status, 0, result.stderr || result.stdout); + + const sourceProbePath = path.join( + repoDir, + '.omx', + 'agent-worktrees', + '__source-probe-agent__test-source-probe-prune-20260422-153300', + ); + result = runCmd('git', ['worktree', 'add', sourceProbePath, 'agent/test-source-probe-prune'], repoDir); + assert.equal(result.status, 0, result.stderr || result.stdout); + assert.equal(fs.existsSync(sourceProbePath), true); + + result = runWorktreePrune([], repoDir); + assert.equal(result.status, 0, result.stderr || result.stdout); + assert.equal(fs.existsSync(sourceProbePath), false, 'temporary source-probe worktree should be removed'); + + const branchResult = runCmd('git', ['show-ref', '--verify', '--quiet', 'refs/heads/agent/test-source-probe-prune'], repoDir); + assert.equal(branchResult.status, 0, 'agent branch ref should remain after pruning only the temporary worktree'); +}); + + test('worktree prune reroutes foreign worktrees to the owning repo .omx root', () => { const repoDir = initRepo(); let result = runNode(['setup', '--target', repoDir, '--no-global-install'], repoDir);