Make finish wait and clean up by default (#545)

NagyVikt · NagyVikt · web-flow · commit ab32d5bbdd0c · 2026-05-11T10:19:27.000+02:00
Claude finish flows could strand work at PR-created because the lower-level branch finish script defaulted wait and cleanup off while higher-level gx finish defaulted them on. Align the script and template defaults, keep explicit --no-wait-for-merge and --no-cleanup escape hatches, and make post-merge base refresh visible without pulling dirty local base worktrees. Preserve forwarded active-cwd pruning context when branch finish calls gx worktree prune through the CLI wrapper.

Constraint: Claude AGENTS flow requires gx branch finish --via-pr to wait for merge and cleanup by default. Constraint: Local base refresh must not overwrite dirty user worktrees. Rejected: Require every Claude prompt to include --wait-for-merge --cleanup | the script defaults still permit stranded PR-created lanes. Confidence: high. Scope-risk: moderate. Directive: Do not make PR finish pending-by-default again; use --no-wait-for-merge or --no-cleanup for explicit opt-out tests and workflows. Tested: bash -n scripts/agent-branch-finish.sh templates/scripts/agent-branch-finish.sh; node --test test/finish.test.js; git diff --check; openspec validate --specs. Not-tested: Live GitHub branch protection timing against a real protected remote.

Co-authored-by: NagyVikt &lt;nagy.viktordp@gmail.com&gt;
diff --git a/scripts/agent-branch-finish.sh b/scripts/agent-branch-finish.sh
@@ -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}"
@@ -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")"
@@ -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
@@ -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.
@@ -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
diff --git a/src/cli/main.js b/src/cli/main.js
@@ -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;
   }
diff --git a/templates/scripts/agent-branch-finish.sh b/templates/scripts/agent-branch-finish.sh
@@ -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}"
@@ -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")"
@@ -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
@@ -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.
@@ -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
diff --git a/test/finish.test.js b/test/finish.test.js
@@ -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'`));
 
@@ -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/);
@@ -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');
@@ -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);

Original file line number	Diff line number	Diff line change
`@@ -3606,7 +3606,7 @@ function worktree(rawArgs) {`
`3606`	`3606`	`const { target, passthrough } = extractTargetedArgs(rest);`
`3607`	`3607`	`invokePackageAsset('worktreePrune', passthrough, {`
`3608`	`3608`	`cwd: resolveRepoRoot(target),`
`3609`		`- env: { GUARDEX_PRUNE_ACTIVE_CWD: activeCwd },`
	`3609`	`+ env: { GUARDEX_PRUNE_ACTIVE_CWD: process.env.GUARDEX_PRUNE_ACTIVE_CWD \|\| activeCwd },`
`3610`	`3610`	`});`
`3611`	`3611`	`return;`
`3612`	`3612`	`}`