Skip to content

Commit 507a69b

Browse files
NagyViktNagyVikt
andauthored
Prevent finish cleanup from deleting caller cwd (#426)
Finishing from a subdirectory inside an agent worktree made the active-worktree check compare the source worktree root against the physical subdirectory. That let cleanup remove the caller's cwd after a successful merge, which then surfaced as a false background-command failure. Constraint: Preserve active shell cwd even if this leaves the detached worktree for a later gx cleanup pass. Rejected: Always pivot prune to the primary checkout | it can delete the caller's active shell cwd and recreate the false failure. Confidence: high Scope-risk: narrow Directive: Active-cwd cleanup must compare worktree roots, not raw pwd values. Tested: node --test test/finish.test.js; node --test --test-name-pattern 'agent-branch-finish pivots' test/metadata.test.js; bash -n scripts/agent-branch-finish.sh; bash -n templates/scripts/agent-branch-finish.sh; git diff --check; openspec validate --specs Not-tested: Live Claude background shell prompt refresh after finish. Co-authored-by: NagyVikt <nagy.viktordp@gmail.com>
1 parent 1671131 commit 507a69b

4 files changed

Lines changed: 21 additions & 6 deletions

File tree

scripts/agent-branch-finish.sh

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -165,7 +165,11 @@ if ! git rev-parse --is-inside-work-tree >/dev/null 2>&1; then
165165
fi
166166

167167
repo_root="$(git rev-parse --show-toplevel)"
168-
current_worktree="$(pwd -P)"
168+
# The physical cwd may be a subdirectory inside the source worktree. Cleanup
169+
# decisions need the enclosing worktree root, otherwise finishing from `src/`
170+
# can delete the caller's cwd and turn a successful merge into a false shell
171+
# failure.
172+
current_worktree="$repo_root"
169173
common_git_dir_raw="$(git -C "$repo_root" rev-parse --git-common-dir)"
170174
if [[ "$common_git_dir_raw" == /* ]]; then
171175
common_git_dir="$common_git_dir_raw"
@@ -846,10 +850,12 @@ if [[ "$CLEANUP_AFTER_MERGE" -eq 1 ]]; then
846850
echo "[agent-branch-finish] You can run manual cleanup: gx cleanup --base ${BASE_BRANCH}" >&2
847851
fi
848852

849-
echo "[agent-branch-finish] Merged '${SOURCE_BRANCH}' into '${BASE_BRANCH}' via ${merge_status} flow and cleaned source branch/worktree."
850853
if [[ "$source_worktree" == "$current_worktree" && "$source_worktree" == "${agent_worktree_root}"/* && -d "$source_worktree" ]]; then
854+
echo "[agent-branch-finish] Merged '${SOURCE_BRANCH}' into '${BASE_BRANCH}' via ${merge_status} flow and cleaned source branch/remote."
851855
echo "[agent-branch-finish] Current worktree '${source_worktree}' still exists because it is the active shell cwd." >&2
852856
echo "[agent-branch-finish] Leave this directory, then run: gx cleanup --base ${BASE_BRANCH}" >&2
857+
else
858+
echo "[agent-branch-finish] Merged '${SOURCE_BRANCH}' into '${BASE_BRANCH}' via ${merge_status} flow and cleaned source branch/worktree."
853859
fi
854860
else
855861
pivot_to_repo_root_before_prune

templates/scripts/agent-branch-finish.sh

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -165,7 +165,11 @@ if ! git rev-parse --is-inside-work-tree >/dev/null 2>&1; then
165165
fi
166166

167167
repo_root="$(git rev-parse --show-toplevel)"
168-
current_worktree="$(pwd -P)"
168+
# The physical cwd may be a subdirectory inside the source worktree. Cleanup
169+
# decisions need the enclosing worktree root, otherwise finishing from `src/`
170+
# can delete the caller's cwd and turn a successful merge into a false shell
171+
# failure.
172+
current_worktree="$repo_root"
169173
common_git_dir_raw="$(git -C "$repo_root" rev-parse --git-common-dir)"
170174
if [[ "$common_git_dir_raw" == /* ]]; then
171175
common_git_dir="$common_git_dir_raw"
@@ -846,10 +850,12 @@ if [[ "$CLEANUP_AFTER_MERGE" -eq 1 ]]; then
846850
echo "[agent-branch-finish] You can run manual cleanup: gx cleanup --base ${BASE_BRANCH}" >&2
847851
fi
848852

849-
echo "[agent-branch-finish] Merged '${SOURCE_BRANCH}' into '${BASE_BRANCH}' via ${merge_status} flow and cleaned source branch/worktree."
850853
if [[ "$source_worktree" == "$current_worktree" && "$source_worktree" == "${agent_worktree_root}"/* && -d "$source_worktree" ]]; then
854+
echo "[agent-branch-finish] Merged '${SOURCE_BRANCH}' into '${BASE_BRANCH}' via ${merge_status} flow and cleaned source branch/remote."
851855
echo "[agent-branch-finish] Current worktree '${source_worktree}' still exists because it is the active shell cwd." >&2
852856
echo "[agent-branch-finish] Leave this directory, then run: gx cleanup --base ${BASE_BRANCH}" >&2
857+
else
858+
echo "[agent-branch-finish] Merged '${SOURCE_BRANCH}' into '${BASE_BRANCH}' via ${merge_status} flow and cleaned source branch/worktree."
853859
fi
854860
else
855861
pivot_to_repo_root_before_prune

test/finish.test.js

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -734,6 +734,8 @@ test('agent-branch-finish cleanup succeeds from active agent worktree when base
734734
assert.equal(result.status, 0, result.stderr || result.stdout);
735735
result = runCmd('git', ['push', '-u', 'origin', 'agent/test-active-worktree-cleanup'], agentWorktreePath);
736736
assert.equal(result.status, 0, result.stderr || result.stdout);
737+
const agentSubdir = path.join(agentWorktreePath, 'nested', 'cwd');
738+
fs.mkdirSync(agentSubdir, { recursive: true });
737739

738740
const { fakePath: fakeGhPath } = createFakeGhScript(`
739741
if [[ "$1" == "pr" && "$2" == "create" ]]; then
@@ -756,13 +758,13 @@ exit 1
756758

757759
const finish = runBranchFinish(
758760
['--branch', 'agent/test-active-worktree-cleanup', '--base', 'dev', '--mode', 'pr', '--cleanup'],
759-
agentWorktreePath,
761+
agentSubdir,
760762
{ GUARDEX_GH_BIN: fakeGhPath },
761763
);
762764
assert.equal(finish.status, 0, finish.stderr || finish.stdout);
763765
assert.match(
764766
finish.stdout,
765-
/Merged 'agent\/test-active-worktree-cleanup' into 'dev' via pr flow and cleaned source branch\/worktree\./,
767+
/Merged 'agent\/test-active-worktree-cleanup' into 'dev' via pr flow and cleaned source branch\/remote\./,
766768
);
767769
assert.match(finish.stderr, /Current worktree '.+' still exists because it is the active shell cwd/);
768770

test/metadata.test.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -235,6 +235,7 @@ test('critical runtime helper scripts and active-agents sources stay in sync wit
235235
test('agent-branch-finish pivots out of active agent cwd before every prune path', () => {
236236
const script = fs.readFileSync(path.join(repoRoot, 'scripts', 'agent-branch-finish.sh'), 'utf8');
237237

238+
assert.match(script, /current_worktree="\$repo_root"/);
238239
assert.match(script, /pivot_to_repo_root_before_prune\(\) \{\n\s+if \[\[ "\$current_worktree" == "\$source_worktree"/);
239240
assert.match(script, /cd "\$repo_root" 2>\/dev\/null \|\| true/);
240241
assert.match(

0 commit comments

Comments
 (0)