Skip to content

Make finish wait and clean up by default #545

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
NagyVikt merged 1 commit into from
May 11, 2026
+192 −20
Merged

Make finish wait and clean up by default #545

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 22 additions & 8 deletions scripts/agent-branch-finish.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,8 @@ MERGE_MODE="auto"
GH_BIN="${GUARDEX_GH_BIN:-gh}"
NODE_BIN="${GUARDEX_NODE_BIN:-node}"
CLI_ENTRY="${GUARDEX_CLI_ENTRY:-}"
CLEANUP_AFTER_MERGE_RAW="${GUARDEX_FINISH_CLEANUP:-false}"
WAIT_FOR_MERGE_RAW="${GUARDEX_FINISH_WAIT_FOR_MERGE:-false}"
CLEANUP_AFTER_MERGE_RAW="${GUARDEX_FINISH_CLEANUP:-true}"
WAIT_FOR_MERGE_RAW="${GUARDEX_FINISH_WAIT_FOR_MERGE:-true}"
WAIT_TIMEOUT_SECONDS_RAW="${GUARDEX_FINISH_WAIT_TIMEOUT_SECONDS:-1800}"
WAIT_POLL_SECONDS_RAW="${GUARDEX_FINISH_WAIT_POLL_SECONDS:-10}"
PARENT_GITLINK_AUTO_COMMIT_RAW="${GUARDEX_FINISH_PARENT_GITLINK_AUTO_COMMIT:-true}"
Expand Down Expand Up @@ -64,8 +64,8 @@ normalize_int() {
printf '%s' "$value"
}

CLEANUP_AFTER_MERGE="$(normalize_bool "$CLEANUP_AFTER_MERGE_RAW" "0")"
WAIT_FOR_MERGE="$(normalize_bool "$WAIT_FOR_MERGE_RAW" "0")"
CLEANUP_AFTER_MERGE="$(normalize_bool "$CLEANUP_AFTER_MERGE_RAW" "1")"
WAIT_FOR_MERGE="$(normalize_bool "$WAIT_FOR_MERGE_RAW" "1")"
WAIT_TIMEOUT_SECONDS="$(normalize_int "$WAIT_TIMEOUT_SECONDS_RAW" "1800" "30")"
WAIT_POLL_SECONDS="$(normalize_int "$WAIT_POLL_SECONDS_RAW" "10" "0")"
PARENT_GITLINK_AUTO_COMMIT="$(normalize_bool "$PARENT_GITLINK_AUTO_COMMIT_RAW" "1")"
Expand Down Expand Up @@ -372,6 +372,22 @@ is_clean_worktree() {
&& git -C "$wt" diff --cached --quiet -- . ":(exclude).omx/state/agent-file-locks.json"
}

refresh_clean_base_worktree() {
local wt="$1"
[[ -z "$wt" || "$PUSH_ENABLED" -ne 1 ]] && return 0

if ! is_clean_worktree "$wt"; then
echo "[agent-branch-finish] Warning: local ${BASE_BRANCH} worktree is dirty; skipping 'git pull --ff-only origin ${BASE_BRANCH}' for ${wt}." >&2
return 0
fi

if GUARDEX_DISABLE_POST_MERGE_CLEANUP=1 GUARDEX_PRUNE_ACTIVE_CWD="$finish_active_cwd" git -C "$wt" pull --ff-only origin "$BASE_BRANCH" >/dev/null; then
echo "[agent-branch-finish] Refreshed local ${BASE_BRANCH} worktree with 'git pull --ff-only origin ${BASE_BRANCH}': ${wt}"
else
echo "[agent-branch-finish] Warning: failed to refresh local ${BASE_BRANCH} worktree with 'git pull --ff-only origin ${BASE_BRANCH}': ${wt}" >&2
fi
}

remove_stale_source_probe_worktrees "$SOURCE_BRANCH"
source_worktree="$(get_worktree_for_branch "$SOURCE_BRANCH")"
created_source_probe=0
Expand Down Expand Up @@ -957,9 +973,7 @@ fi
run_guardex_cli locks release --branch "$SOURCE_BRANCH" >/dev/null 2>&1 || true

base_worktree="$(get_worktree_for_branch "$BASE_BRANCH")"
if [[ -n "$base_worktree" ]] && is_clean_worktree "$base_worktree" && [[ "$PUSH_ENABLED" -eq 1 ]]; then
git -C "$base_worktree" pull --ff-only origin "$BASE_BRANCH" >/dev/null 2>&1 || true
fi
refresh_clean_base_worktree "$base_worktree"
maybe_auto_commit_parent_gitlink "$base_worktree"

# Pivot out of the agent worktree before prune calls that may remove it.
Expand All @@ -985,7 +999,7 @@ if [[ "$CLEANUP_AFTER_MERGE" -eq 1 ]]; then
git -C "$source_worktree" checkout --detach >/dev/null 2>&1 || true
fi
if [[ "$switched_to_base" -eq 1 && "$PUSH_ENABLED" -eq 1 ]] && git -C "$repo_root" show-ref --verify --quiet "refs/remotes/origin/${BASE_BRANCH}"; then
git -C "$source_worktree" pull --ff-only origin "$BASE_BRANCH" >/dev/null 2>&1 || true
refresh_clean_base_worktree "$source_worktree"
fi
fi
elif [[ "$source_worktree" == "$current_worktree" && "$source_worktree" == "${agent_worktree_root}"/* ]]; then
Expand Down
2 changes: 1 addition & 1 deletion src/cli/main.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3606,7 +3606,7 @@ function worktree(rawArgs) {
const { target, passthrough } = extractTargetedArgs(rest);
invokePackageAsset('worktreePrune', passthrough, {
cwd: resolveRepoRoot(target),
env: { GUARDEX_PRUNE_ACTIVE_CWD: activeCwd },
env: { GUARDEX_PRUNE_ACTIVE_CWD: process.env.GUARDEX_PRUNE_ACTIVE_CWD || activeCwd },
});
return;
}
Expand Down
30 changes: 22 additions & 8 deletions templates/scripts/agent-branch-finish.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,8 @@ MERGE_MODE="auto"
GH_BIN="${GUARDEX_GH_BIN:-gh}"
NODE_BIN="${GUARDEX_NODE_BIN:-node}"
CLI_ENTRY="${GUARDEX_CLI_ENTRY:-}"
CLEANUP_AFTER_MERGE_RAW="${GUARDEX_FINISH_CLEANUP:-false}"
WAIT_FOR_MERGE_RAW="${GUARDEX_FINISH_WAIT_FOR_MERGE:-false}"
CLEANUP_AFTER_MERGE_RAW="${GUARDEX_FINISH_CLEANUP:-true}"
WAIT_FOR_MERGE_RAW="${GUARDEX_FINISH_WAIT_FOR_MERGE:-true}"
WAIT_TIMEOUT_SECONDS_RAW="${GUARDEX_FINISH_WAIT_TIMEOUT_SECONDS:-1800}"
WAIT_POLL_SECONDS_RAW="${GUARDEX_FINISH_WAIT_POLL_SECONDS:-10}"
PARENT_GITLINK_AUTO_COMMIT_RAW="${GUARDEX_FINISH_PARENT_GITLINK_AUTO_COMMIT:-true}"
Expand Down Expand Up @@ -64,8 +64,8 @@ normalize_int() {
printf '%s' "$value"
}

CLEANUP_AFTER_MERGE="$(normalize_bool "$CLEANUP_AFTER_MERGE_RAW" "0")"
WAIT_FOR_MERGE="$(normalize_bool "$WAIT_FOR_MERGE_RAW" "0")"
CLEANUP_AFTER_MERGE="$(normalize_bool "$CLEANUP_AFTER_MERGE_RAW" "1")"
WAIT_FOR_MERGE="$(normalize_bool "$WAIT_FOR_MERGE_RAW" "1")"
WAIT_TIMEOUT_SECONDS="$(normalize_int "$WAIT_TIMEOUT_SECONDS_RAW" "1800" "30")"
WAIT_POLL_SECONDS="$(normalize_int "$WAIT_POLL_SECONDS_RAW" "10" "0")"
PARENT_GITLINK_AUTO_COMMIT="$(normalize_bool "$PARENT_GITLINK_AUTO_COMMIT_RAW" "1")"
Expand Down Expand Up @@ -372,6 +372,22 @@ is_clean_worktree() {
&& git -C "$wt" diff --cached --quiet -- . ":(exclude).omx/state/agent-file-locks.json"
}

refresh_clean_base_worktree() {
local wt="$1"
[[ -z "$wt" || "$PUSH_ENABLED" -ne 1 ]] && return 0

if ! is_clean_worktree "$wt"; then
echo "[agent-branch-finish] Warning: local ${BASE_BRANCH} worktree is dirty; skipping 'git pull --ff-only origin ${BASE_BRANCH}' for ${wt}." >&2
return 0
fi

if GUARDEX_DISABLE_POST_MERGE_CLEANUP=1 GUARDEX_PRUNE_ACTIVE_CWD="$finish_active_cwd" git -C "$wt" pull --ff-only origin "$BASE_BRANCH" >/dev/null; then
echo "[agent-branch-finish] Refreshed local ${BASE_BRANCH} worktree with 'git pull --ff-only origin ${BASE_BRANCH}': ${wt}"
else
echo "[agent-branch-finish] Warning: failed to refresh local ${BASE_BRANCH} worktree with 'git pull --ff-only origin ${BASE_BRANCH}': ${wt}" >&2
fi
}

remove_stale_source_probe_worktrees "$SOURCE_BRANCH"
source_worktree="$(get_worktree_for_branch "$SOURCE_BRANCH")"
created_source_probe=0
Expand Down Expand Up @@ -957,9 +973,7 @@ fi
run_guardex_cli locks release --branch "$SOURCE_BRANCH" >/dev/null 2>&1 || true

base_worktree="$(get_worktree_for_branch "$BASE_BRANCH")"
if [[ -n "$base_worktree" ]] && is_clean_worktree "$base_worktree" && [[ "$PUSH_ENABLED" -eq 1 ]]; then
git -C "$base_worktree" pull --ff-only origin "$BASE_BRANCH" >/dev/null 2>&1 || true
fi
refresh_clean_base_worktree "$base_worktree"
maybe_auto_commit_parent_gitlink "$base_worktree"

# Pivot out of the agent worktree before prune calls that may remove it.
Expand All @@ -985,7 +999,7 @@ if [[ "$CLEANUP_AFTER_MERGE" -eq 1 ]]; then
git -C "$source_worktree" checkout --detach >/dev/null 2>&1 || true
fi
if [[ "$switched_to_base" -eq 1 && "$PUSH_ENABLED" -eq 1 ]] && git -C "$repo_root" show-ref --verify --quiet "refs/remotes/origin/${BASE_BRANCH}"; then
git -C "$source_worktree" pull --ff-only origin "$BASE_BRANCH" >/dev/null 2>&1 || true
refresh_clean_base_worktree "$source_worktree"
fi
fi
elif [[ "$source_worktree" == "$current_worktree" && "$source_worktree" == "${agent_worktree_root}"/* ]]; then
Expand Down
150 changes: 147 additions & 3 deletions test/finish.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ test('agent-branch-finish handles Claude-root worktrees when inferring base from
result = runCmd('git', ['worktree', 'add', auxWorktree, 'main'], repoDir);
assert.equal(result.status, 0, result.stderr || result.stdout);

const finish = runBranchFinish(['--branch', agentBranch], repoDir);
const finish = runBranchFinish(['--branch', agentBranch, '--no-cleanup'], repoDir);
assert.equal(finish.status, 0, finish.stderr || finish.stdout);
assert.match(finish.stdout, new RegExp(`Merged '${escapeRegexLiteral(agentBranch)}' into 'main'`));

Expand Down Expand Up @@ -325,7 +325,7 @@ test('agent-branch-finish auto-syncs source branch when behind origin/dev', () =
result = runCmd('git', ['checkout', 'agent/test-finish-sync-guard'], repoDir);
assert.equal(result.status, 0, result.stderr);

const finish = runBranchFinish(['--branch', 'agent/test-finish-sync-guard'], repoDir);
const finish = runBranchFinish(['--branch', 'agent/test-finish-sync-guard', '--no-cleanup'], repoDir);
assert.equal(finish.status, 0, finish.stderr || finish.stdout);
assert.match(finish.stderr, /agent-sync-guard/);
assert.match(finish.stderr, /Auto-syncing 'agent\/test-finish-sync-guard' onto origin\/dev before finish/);
Expand Down Expand Up @@ -373,7 +373,7 @@ test('agent-branch-finish removes stale source-probe worktrees before creating a
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);
const finish = runBranchFinish(['--branch', 'agent/test-stale-source-probe', '--no-cleanup'], 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');
Expand Down Expand Up @@ -1078,6 +1078,150 @@ exit 1
});


test('agent-branch-finish defaults bare PR finish to wait, cleanup, and base refresh', () => {
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 = runCmd('git', ['push', 'origin', 'dev'], repoDir);
assert.equal(result.status, 0, result.stderr);

result = runCmd('git', ['checkout', '-b', 'agent/test-pr-default-wait-cleanup'], repoDir);
assert.equal(result.status, 0, result.stderr);
commitFile(repoDir, 'agent-pr-default.txt', 'agent default wait cleanup\n', 'agent default wait cleanup change');

const auxWorktree = path.join(path.dirname(repoDir), 'aux-default-pr-dev');
result = runCmd('git', ['worktree', 'add', auxWorktree, 'dev'], repoDir);
assert.equal(result.status, 0, result.stderr || result.stdout);

const ghMergeState = path.join(repoDir, '.finish-gh-default-merge-attempts');
const { fakePath: fakeGhPath } = createFakeGhScript(`
if [[ "$1" == "pr" && "$2" == "create" ]]; then
exit 0
fi
if [[ "$1" == "pr" && "$2" == "view" ]]; then
if [[ " $* " == *" --json url "* ]]; then
echo "https://example.test/pr/default"
exit 0
fi
if [[ " $* " == *" --json state,mergedAt,url "* ]]; then
attempts=0
if [[ -f "${'${GUARDEX_TEST_GH_MERGE_STATE}'}" ]]; then
attempts="$(cat "${'${GUARDEX_TEST_GH_MERGE_STATE}'}")"
fi
if [[ "$attempts" -ge 2 ]]; then
echo -e "MERGED\\x1f2026-05-11T00:00:00Z\\x1fhttps://example.test/pr/default"
else
echo -e "OPEN\\x1f\\x1fhttps://example.test/pr/default"
fi
exit 0
fi
echo "unexpected gh pr view args: $*" >&2
exit 1
fi
if [[ "$1" == "pr" && "$2" == "merge" ]]; then
attempts=0
if [[ -f "${'${GUARDEX_TEST_GH_MERGE_STATE}'}" ]]; then
attempts="$(cat "${'${GUARDEX_TEST_GH_MERGE_STATE}'}")"
fi
attempts=$((attempts + 1))
echo "$attempts" > "${'${GUARDEX_TEST_GH_MERGE_STATE}'}"
if [[ "$attempts" -lt 2 ]]; then
echo "Required status check \\"test (node 22)\\" is expected." >&2
exit 1
fi
git push origin "$3:dev" >/dev/null 2>&1
exit 0
fi
echo "unexpected gh args: $*" >&2
exit 1
`);

const finish = runBranchFinish(
[
'--branch',
'agent/test-pr-default-wait-cleanup',
'--mode',
'pr',
'--wait-timeout-seconds',
'60',
'--wait-poll-seconds',
'0',
],
repoDir,
{
GUARDEX_GH_BIN: fakeGhPath,
GUARDEX_TEST_GH_MERGE_STATE: ghMergeState,
},
);
assert.equal(finish.status, 0, finish.stderr || finish.stdout);
assert.equal(fs.readFileSync(ghMergeState, 'utf8').trim(), '2', 'bare PR finish should wait and retry merge by default');
assert.match(finish.stdout, /Merged 'agent\/test-pr-default-wait-cleanup' into 'dev' via pr flow and cleaned source branch\/worktree\./);
assert.match(finish.stdout, /Refreshed local dev worktree with 'git pull --ff-only origin dev': /);
assert.equal(fs.existsSync(auxWorktree), true, 'default cleanup should keep the checked-out dev worktree');
assert.equal(
fs.existsSync(path.join(repoDir, 'agent-pr-default.txt')),
true,
'clean local dev checkout should be refreshed after PR merge',
);

result = runCmd('git', ['show-ref', '--verify', '--quiet', 'refs/heads/agent/test-pr-default-wait-cleanup'], repoDir);
assert.notEqual(result.status, 0, 'default cleanup should delete the local agent branch');
result = runCmd('git', ['ls-remote', '--heads', 'origin', 'agent/test-pr-default-wait-cleanup'], repoDir);
assert.equal(result.stdout.trim(), '', 'default cleanup should delete the remote agent branch');
});


test('agent-branch-finish warns instead of pulling dirty local base worktree', () => {
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 = runCmd('git', ['push', 'origin', 'dev'], repoDir);
assert.equal(result.status, 0, result.stderr);

result = runCmd('git', ['checkout', '-b', 'agent/test-dirty-base-refresh-warning'], repoDir);
assert.equal(result.status, 0, result.stderr);
commitFile(repoDir, 'agent-dirty-base-refresh.txt', 'agent dirty base refresh\n', 'agent dirty base refresh change');

const auxWorktree = path.join(path.dirname(repoDir), 'aux-dirty-dev');
result = runCmd('git', ['worktree', 'add', auxWorktree, 'dev'], repoDir);
assert.equal(result.status, 0, result.stderr || result.stdout);
fs.writeFileSync(path.join(auxWorktree, 'package.json'), '{"dirty":true}\n', 'utf8');

const finish = runBranchFinish(
['--branch', 'agent/test-dirty-base-refresh-warning', '--base', 'dev', '--direct-only', '--no-cleanup'],
repoDir,
);
assert.equal(finish.status, 0, finish.stderr || finish.stdout);
assert.match(
finish.stderr,
/Warning: local dev worktree is dirty; skipping 'git pull --ff-only origin dev' for /,
);
assert.equal(
fs.existsSync(path.join(auxWorktree, 'agent-dirty-base-refresh.txt')),
false,
'dirty local dev worktree should not be pulled implicitly',
);
});


test('cleanup command removes merged agent branch/worktree and remote ref', () => {
const repoDir = initRepo();
seedCommit(repoDir);
Expand Down
Loading