From 772a668c36cc5af03ecad3a63e8c2e09934161e8 Mon Sep 17 00:00:00 2001 From: NagyVikt Date: Sun, 12 Apr 2026 02:50:07 +0200 Subject: [PATCH] Make Codex auto-finish wait for merge and clean merged sandboxes This changes the codex-agent default finish flow from "merge attempt then keep" to "wait/retry until merge succeeds, then clean merged branch/worktree". agent-branch-finish now supports wait controls (, timeout, poll interval) and retries PR merges until checks clear or timeout. The template codex-agent passes wait+cleanup by default, and documentation was updated to match the new default behavior. Regression coverage now verifies both direct finish waiting behavior and codex-agent end-to-end retry/cleanup behavior. Constraint: Keep manual agent-branch-finish behavior backward compatible unless wait flags are explicit Rejected: Force waiting for every finish invocation | would break manual async review workflows Confidence: high Scope-risk: moderate Reversibility: clean Directive: Codex automation should treat unresolved PR checks as incomplete work, not successful finish Tested: npm test; node --check bin/multiagent-safety.js; npm pack --dry-run Not-tested: Real GitHub required-check timing in external repos --- AGENTS.md | 6 +- scripts/agent-branch-finish.sh | 149 +++++++++++++++- templates/AGENTS.multiagent-safety.md | 6 +- templates/scripts/agent-branch-finish.sh | 149 +++++++++++++++- templates/scripts/codex-agent.sh | 25 ++- test/install.test.js | 211 ++++++++++++++++++++++- 6 files changed, 514 insertions(+), 32 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index a25f7ee..aeada97 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -95,9 +95,9 @@ OMX runtime state typically lives under `.omx/`: - If the current local branch already contains accidental edits, move them to an agent branch/worktree first, then continue implementation. - Treat the base branch (`main` or the user's current local base branch) as read-only while the agent branch is active. - Agent completion defaults to `scripts/codex-agent.sh`, which auto-finishes the branch (auto-commit changed files, push/create PR, attempt merge, and pull the local base branch after merge). -- Auto-finish keeps the sandbox branch/worktree by default so conflict follow-ups and audits stay reproducible. -- Use explicit cleanup when done: `gx cleanup --branch ""` (or `gx cleanup` for all merged agent branches). -- If codex-agent auto-finish cannot complete, immediately run `scripts/agent-branch-finish.sh --branch "" --via-pr` and keep the branch open until checks/review pass. +- Auto-finish now waits for required checks/merge and then cleans merged sandbox branch/worktree by default. +- Use `--no-cleanup` only when you explicitly need to keep a merged sandbox for audit/debug follow-up. +- If codex-agent auto-finish cannot complete, immediately run `scripts/agent-branch-finish.sh --branch "" --via-pr --wait-for-merge` and keep the branch open until checks/review pass. - If merge/rebase conflicts block auto-finish, run a conflict-resolution review pass in that sandbox branch, then rerun `agent-branch-finish.sh --via-pr` until merged. - Completion is not valid until these are true: commit exists on the agent branch, branch is pushed to `origin`, and PR/merge status is produced by `agent-branch-finish.sh` or `codex-agent`. - Per-message loop is mandatory: for every new user message/task, start a fresh agent branch/worktree, claim ownership locks, implement and verify, finish via PR/merge cleanup, then repeat for the next message/task. diff --git a/scripts/agent-branch-finish.sh b/scripts/agent-branch-finish.sh index 17aa2f2..c151729 100755 --- a/scripts/agent-branch-finish.sh +++ b/scripts/agent-branch-finish.sh @@ -10,6 +10,9 @@ DELETE_REMOTE_BRANCH_EXPLICIT=0 MERGE_MODE="auto" GH_BIN="${MUSAFETY_GH_BIN:-gh}" CLEANUP_AFTER_MERGE_RAW="${MUSAFETY_FINISH_CLEANUP:-false}" +WAIT_FOR_MERGE_RAW="${MUSAFETY_FINISH_WAIT_FOR_MERGE:-false}" +WAIT_TIMEOUT_SECONDS_RAW="${MUSAFETY_FINISH_WAIT_TIMEOUT_SECONDS:-1800}" +WAIT_POLL_SECONDS_RAW="${MUSAFETY_FINISH_WAIT_POLL_SECONDS:-10}" normalize_bool() { local raw="${1:-}" @@ -24,7 +27,27 @@ normalize_bool() { esac } +normalize_int() { + local raw="${1:-}" + local fallback="${2:-0}" + local min_value="${3:-0}" + local value="$raw" + + if [[ -z "$value" || ! "$value" =~ ^[0-9]+$ ]]; then + value="$fallback" + fi + + if (( value < min_value )); then + value="$min_value" + fi + + printf '%s' "$value" +} + CLEANUP_AFTER_MERGE="$(normalize_bool "$CLEANUP_AFTER_MERGE_RAW" "0")" +WAIT_FOR_MERGE="$(normalize_bool "$WAIT_FOR_MERGE_RAW" "0")" +WAIT_TIMEOUT_SECONDS="$(normalize_int "$WAIT_TIMEOUT_SECONDS_RAW" "1800" "30")" +WAIT_POLL_SECONDS="$(normalize_int "$WAIT_POLL_SECONDS_RAW" "10" "0")" while [[ $# -gt 0 ]]; do case "$1" in @@ -59,6 +82,22 @@ while [[ $# -gt 0 ]]; do CLEANUP_AFTER_MERGE=0 shift ;; + --wait-for-merge) + WAIT_FOR_MERGE=1 + shift + ;; + --no-wait-for-merge) + WAIT_FOR_MERGE=0 + shift + ;; + --wait-timeout-seconds) + WAIT_TIMEOUT_SECONDS="$(normalize_int "${2:-}" "1800" "30")" + shift 2 + ;; + --wait-poll-seconds) + WAIT_POLL_SECONDS="$(normalize_int "${2:-}" "10" "0")" + shift 2 + ;; --mode) MERGE_MODE="${2:-auto}" shift 2 @@ -73,7 +112,7 @@ while [[ $# -gt 0 ]]; do ;; *) echo "[agent-branch-finish] Unknown argument: $1" >&2 - echo "Usage: $0 [--base ] [--branch ] [--no-push] [--cleanup|--no-cleanup] [--keep-remote-branch|--delete-remote-branch] [--mode auto|direct|pr|--via-pr|--direct-only]" >&2 + echo "Usage: $0 [--base ] [--branch ] [--no-push] [--cleanup|--no-cleanup] [--wait-for-merge|--no-wait-for-merge] [--wait-timeout-seconds ] [--wait-poll-seconds ] [--keep-remote-branch|--delete-remote-branch] [--mode auto|direct|pr|--via-pr|--direct-only]" >&2 exit 1 ;; esac @@ -98,6 +137,14 @@ fi repo_root="$(git rev-parse --show-toplevel)" current_worktree="$(pwd -P)" +common_git_dir_raw="$(git -C "$repo_root" rev-parse --git-common-dir)" +if [[ "$common_git_dir_raw" == /* ]]; then + common_git_dir="$common_git_dir_raw" +else + common_git_dir="$(cd "$repo_root/$common_git_dir_raw" && pwd -P)" +fi +repo_common_root="$(cd "$common_git_dir/.." && pwd -P)" +agent_worktree_root="${repo_common_root}/.omx/agent-worktrees" if [[ -z "$SOURCE_BRANCH" ]]; then SOURCE_BRANCH="$(git rev-parse --abbrev-ref HEAD)" @@ -171,7 +218,7 @@ created_source_probe=0 source_probe_path="" if [[ -z "$source_worktree" ]]; then - source_probe_path="${repo_root}/.omx/agent-worktrees/__source-probe-${SOURCE_BRANCH//\//__}-$(date +%Y%m%d-%H%M%S)" + source_probe_path="${agent_worktree_root}/__source-probe-${SOURCE_BRANCH//\//__}-$(date +%Y%m%d-%H%M%S)" mkdir -p "$(dirname "$source_probe_path")" git -C "$repo_root" worktree add "$source_probe_path" "$SOURCE_BRANCH" >/dev/null source_worktree="$source_probe_path" @@ -229,7 +276,7 @@ if [[ "$should_require_sync" -eq 1 ]] && git -C "$repo_root" show-ref --verify - fi fi -integration_worktree="${repo_root}/.omx/agent-worktrees/__integrate-${BASE_BRANCH//\//__}-$(date +%Y%m%d-%H%M%S)" +integration_worktree="${agent_worktree_root}/__integrate-${BASE_BRANCH//\//__}-$(date +%Y%m%d-%H%M%S)" integration_branch="__agent_integrate_${BASE_BRANCH//\//_}_$(date +%Y%m%d_%H%M%S)" mkdir -p "$(dirname "$integration_worktree")" @@ -289,6 +336,78 @@ is_local_branch_delete_error() { return 1 } +read_pr_state() { + local state_line + state_line="$("$GH_BIN" pr view "$SOURCE_BRANCH" --json state,mergedAt,url --jq '[.state, (.mergedAt // ""), (.url // "")] | @tsv' 2>/dev/null || true)" + if [[ -z "$state_line" ]]; then + return 1 + fi + + local parsed_state="" + local parsed_merged_at="" + local parsed_url="" + IFS=$'\t' read -r parsed_state parsed_merged_at parsed_url <<< "$state_line" + PR_STATE="$parsed_state" + PR_MERGED_AT="$parsed_merged_at" + if [[ -n "$parsed_url" ]]; then + pr_url="$parsed_url" + fi + return 0 +} + +wait_for_pr_merge() { + local deadline + deadline=$(( $(date +%s) + WAIT_TIMEOUT_SECONDS )) + local wait_notice_printed=0 + local merge_output="" + + while true; do + if merge_output="$("$GH_BIN" pr merge "$SOURCE_BRANCH" --squash --delete-branch 2>&1)"; then + return 0 + fi + if is_local_branch_delete_error "$merge_output"; then + echo "[agent-branch-finish] PR merged but gh could not delete the local branch (active worktree); continuing local cleanup." >&2 + return 0 + fi + + PR_STATE="" + PR_MERGED_AT="" + if read_pr_state; then + if [[ "$PR_STATE" == "MERGED" || -n "$PR_MERGED_AT" ]]; then + return 0 + fi + if [[ "$PR_STATE" == "CLOSED" ]]; then + echo "[agent-branch-finish] PR closed without merge; cannot continue auto-finish." >&2 + if [[ -n "$pr_url" ]]; then + echo "[agent-branch-finish] PR: ${pr_url}" >&2 + fi + if [[ -n "$merge_output" ]]; then + echo "$merge_output" >&2 + fi + return 1 + fi + fi + + if [[ "$wait_notice_printed" -eq 0 ]]; then + echo "[agent-branch-finish] Waiting for required checks/reviews, then retrying merge automatically (timeout ${WAIT_TIMEOUT_SECONDS}s)." >&2 + if [[ -n "$pr_url" ]]; then + echo "[agent-branch-finish] PR: ${pr_url}" >&2 + fi + wait_notice_printed=1 + fi + + if (( $(date +%s) >= deadline )); then + echo "[agent-branch-finish] Timed out waiting for PR merge after ${WAIT_TIMEOUT_SECONDS}s." >&2 + if [[ -n "$merge_output" ]]; then + echo "$merge_output" >&2 + fi + return 2 + fi + + sleep "$WAIT_POLL_SECONDS" + done +} + run_pr_flow() { if ! command -v "$GH_BIN" >/dev/null 2>&1; then echo "[agent-branch-finish] PR fallback requested but GitHub CLI not found: ${GH_BIN}" >&2 @@ -320,6 +439,11 @@ run_pr_flow() { return 0 fi + if [[ "$WAIT_FOR_MERGE" -eq 1 ]]; then + wait_for_pr_merge + return $? + fi + auto_output="" if auto_output="$("$GH_BIN" pr merge "$SOURCE_BRANCH" --squash --delete-branch --auto 2>&1)"; then echo "[agent-branch-finish] PR auto-merge enabled; waiting for required checks/reviews." >&2 @@ -365,6 +489,10 @@ if [[ "$PUSH_ENABLED" -eq 1 ]]; then if [[ -n "$pr_url" ]]; then echo "[agent-branch-finish] PR: ${pr_url}" >&2 fi + if [[ "$WAIT_FOR_MERGE" -eq 1 ]]; then + echo "[agent-branch-finish] Merge did not complete within wait window; keeping branch open." >&2 + exit 1 + fi echo "[agent-branch-finish] Merge pending review/check policy. Branch cleanup skipped for now." >&2 exit 0 fi @@ -390,16 +518,21 @@ fi if [[ "$CLEANUP_AFTER_MERGE" -eq 1 ]]; then if [[ "$source_worktree" == "$repo_root" ]]; then if is_clean_worktree "$source_worktree"; then - git -C "$source_worktree" checkout "$BASE_BRANCH" >/dev/null 2>&1 || true - if [[ "$PUSH_ENABLED" -eq 1 ]] && git -C "$repo_root" show-ref --verify --quiet "refs/remotes/origin/${BASE_BRANCH}"; then + switched_to_base=0 + if git -C "$source_worktree" checkout "$BASE_BRANCH" >/dev/null 2>&1; then + switched_to_base=1 + else + 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 fi fi - elif [[ "$source_worktree" == "$current_worktree" && "$source_worktree" == "${repo_root}/.omx/agent-worktrees"/* ]]; then + elif [[ "$source_worktree" == "$current_worktree" && "$source_worktree" == "${agent_worktree_root}"/* ]]; then git -C "$source_worktree" checkout --detach >/dev/null 2>&1 || true fi - if [[ "$source_worktree" != "$current_worktree" && "$source_worktree" == "${repo_root}/.omx/agent-worktrees"/* ]]; then + if [[ "$source_worktree" != "$current_worktree" && "$source_worktree" == "${agent_worktree_root}"/* ]]; then git -C "$repo_root" worktree remove "$source_worktree" --force >/dev/null 2>&1 || true fi @@ -423,7 +556,7 @@ if [[ "$CLEANUP_AFTER_MERGE" -eq 1 ]]; then fi echo "[agent-branch-finish] Merged '${SOURCE_BRANCH}' into '${BASE_BRANCH}' via ${merge_status} flow and cleaned source branch/worktree." - if [[ "$source_worktree" == "$current_worktree" && "$source_worktree" == "${repo_root}/.omx/agent-worktrees"/* ]]; then + if [[ "$source_worktree" == "$current_worktree" && "$source_worktree" == "${agent_worktree_root}"/* ]]; then echo "[agent-branch-finish] Current worktree '${source_worktree}' still exists because it is the active shell cwd." >&2 echo "[agent-branch-finish] Leave this directory, then run: bash scripts/agent-worktree-prune.sh --base ${BASE_BRANCH} --delete-branches" >&2 fi diff --git a/templates/AGENTS.multiagent-safety.md b/templates/AGENTS.multiagent-safety.md index 0a73664..c39b81c 100644 --- a/templates/AGENTS.multiagent-safety.md +++ b/templates/AGENTS.multiagent-safety.md @@ -12,9 +12,9 @@ - For git isolation, each agent must start on a dedicated branch via `scripts/agent-branch-start.sh "" ""`. - Treat the base branch (`main` or the user's current local base branch) as read-only while the agent branch is active. - Agent completion defaults to `scripts/codex-agent.sh`, which auto-finishes the branch (auto-commit changed files, push/create PR, attempt merge, and pull the local base branch after merge). -- Auto-finish keeps the sandbox branch/worktree by default so conflict follow-ups and audits stay reproducible. -- Use explicit cleanup when done: `gx cleanup --branch ""` (or `gx cleanup` for all merged agent branches). -- If codex-agent auto-finish cannot complete, immediately run `scripts/agent-branch-finish.sh --branch "" --via-pr` and keep the branch open until checks/review pass. +- Auto-finish now waits for required checks/merge and then cleans merged sandbox branch/worktree by default. +- Use `--no-cleanup` only when you explicitly need to keep a merged sandbox for audit/debug follow-up. +- If codex-agent auto-finish cannot complete, immediately run `scripts/agent-branch-finish.sh --branch "" --via-pr --wait-for-merge` and keep the branch open until checks/review pass. - If merge/rebase conflicts block auto-finish, run a conflict-resolution review pass in that sandbox branch, then rerun `agent-branch-finish.sh --via-pr` until merged. - Completion is not valid until these are true: commit exists on the agent branch, branch is pushed to `origin`, and PR/merge status is produced by `agent-branch-finish.sh` or `codex-agent`. - Per-message loop is mandatory: for every new user message/task, start a fresh agent branch/worktree, claim ownership locks, implement and verify, finish via PR/merge cleanup, then repeat for the next message/task. diff --git a/templates/scripts/agent-branch-finish.sh b/templates/scripts/agent-branch-finish.sh index 17aa2f2..c151729 100755 --- a/templates/scripts/agent-branch-finish.sh +++ b/templates/scripts/agent-branch-finish.sh @@ -10,6 +10,9 @@ DELETE_REMOTE_BRANCH_EXPLICIT=0 MERGE_MODE="auto" GH_BIN="${MUSAFETY_GH_BIN:-gh}" CLEANUP_AFTER_MERGE_RAW="${MUSAFETY_FINISH_CLEANUP:-false}" +WAIT_FOR_MERGE_RAW="${MUSAFETY_FINISH_WAIT_FOR_MERGE:-false}" +WAIT_TIMEOUT_SECONDS_RAW="${MUSAFETY_FINISH_WAIT_TIMEOUT_SECONDS:-1800}" +WAIT_POLL_SECONDS_RAW="${MUSAFETY_FINISH_WAIT_POLL_SECONDS:-10}" normalize_bool() { local raw="${1:-}" @@ -24,7 +27,27 @@ normalize_bool() { esac } +normalize_int() { + local raw="${1:-}" + local fallback="${2:-0}" + local min_value="${3:-0}" + local value="$raw" + + if [[ -z "$value" || ! "$value" =~ ^[0-9]+$ ]]; then + value="$fallback" + fi + + if (( value < min_value )); then + value="$min_value" + fi + + printf '%s' "$value" +} + CLEANUP_AFTER_MERGE="$(normalize_bool "$CLEANUP_AFTER_MERGE_RAW" "0")" +WAIT_FOR_MERGE="$(normalize_bool "$WAIT_FOR_MERGE_RAW" "0")" +WAIT_TIMEOUT_SECONDS="$(normalize_int "$WAIT_TIMEOUT_SECONDS_RAW" "1800" "30")" +WAIT_POLL_SECONDS="$(normalize_int "$WAIT_POLL_SECONDS_RAW" "10" "0")" while [[ $# -gt 0 ]]; do case "$1" in @@ -59,6 +82,22 @@ while [[ $# -gt 0 ]]; do CLEANUP_AFTER_MERGE=0 shift ;; + --wait-for-merge) + WAIT_FOR_MERGE=1 + shift + ;; + --no-wait-for-merge) + WAIT_FOR_MERGE=0 + shift + ;; + --wait-timeout-seconds) + WAIT_TIMEOUT_SECONDS="$(normalize_int "${2:-}" "1800" "30")" + shift 2 + ;; + --wait-poll-seconds) + WAIT_POLL_SECONDS="$(normalize_int "${2:-}" "10" "0")" + shift 2 + ;; --mode) MERGE_MODE="${2:-auto}" shift 2 @@ -73,7 +112,7 @@ while [[ $# -gt 0 ]]; do ;; *) echo "[agent-branch-finish] Unknown argument: $1" >&2 - echo "Usage: $0 [--base ] [--branch ] [--no-push] [--cleanup|--no-cleanup] [--keep-remote-branch|--delete-remote-branch] [--mode auto|direct|pr|--via-pr|--direct-only]" >&2 + echo "Usage: $0 [--base ] [--branch ] [--no-push] [--cleanup|--no-cleanup] [--wait-for-merge|--no-wait-for-merge] [--wait-timeout-seconds ] [--wait-poll-seconds ] [--keep-remote-branch|--delete-remote-branch] [--mode auto|direct|pr|--via-pr|--direct-only]" >&2 exit 1 ;; esac @@ -98,6 +137,14 @@ fi repo_root="$(git rev-parse --show-toplevel)" current_worktree="$(pwd -P)" +common_git_dir_raw="$(git -C "$repo_root" rev-parse --git-common-dir)" +if [[ "$common_git_dir_raw" == /* ]]; then + common_git_dir="$common_git_dir_raw" +else + common_git_dir="$(cd "$repo_root/$common_git_dir_raw" && pwd -P)" +fi +repo_common_root="$(cd "$common_git_dir/.." && pwd -P)" +agent_worktree_root="${repo_common_root}/.omx/agent-worktrees" if [[ -z "$SOURCE_BRANCH" ]]; then SOURCE_BRANCH="$(git rev-parse --abbrev-ref HEAD)" @@ -171,7 +218,7 @@ created_source_probe=0 source_probe_path="" if [[ -z "$source_worktree" ]]; then - source_probe_path="${repo_root}/.omx/agent-worktrees/__source-probe-${SOURCE_BRANCH//\//__}-$(date +%Y%m%d-%H%M%S)" + source_probe_path="${agent_worktree_root}/__source-probe-${SOURCE_BRANCH//\//__}-$(date +%Y%m%d-%H%M%S)" mkdir -p "$(dirname "$source_probe_path")" git -C "$repo_root" worktree add "$source_probe_path" "$SOURCE_BRANCH" >/dev/null source_worktree="$source_probe_path" @@ -229,7 +276,7 @@ if [[ "$should_require_sync" -eq 1 ]] && git -C "$repo_root" show-ref --verify - fi fi -integration_worktree="${repo_root}/.omx/agent-worktrees/__integrate-${BASE_BRANCH//\//__}-$(date +%Y%m%d-%H%M%S)" +integration_worktree="${agent_worktree_root}/__integrate-${BASE_BRANCH//\//__}-$(date +%Y%m%d-%H%M%S)" integration_branch="__agent_integrate_${BASE_BRANCH//\//_}_$(date +%Y%m%d_%H%M%S)" mkdir -p "$(dirname "$integration_worktree")" @@ -289,6 +336,78 @@ is_local_branch_delete_error() { return 1 } +read_pr_state() { + local state_line + state_line="$("$GH_BIN" pr view "$SOURCE_BRANCH" --json state,mergedAt,url --jq '[.state, (.mergedAt // ""), (.url // "")] | @tsv' 2>/dev/null || true)" + if [[ -z "$state_line" ]]; then + return 1 + fi + + local parsed_state="" + local parsed_merged_at="" + local parsed_url="" + IFS=$'\t' read -r parsed_state parsed_merged_at parsed_url <<< "$state_line" + PR_STATE="$parsed_state" + PR_MERGED_AT="$parsed_merged_at" + if [[ -n "$parsed_url" ]]; then + pr_url="$parsed_url" + fi + return 0 +} + +wait_for_pr_merge() { + local deadline + deadline=$(( $(date +%s) + WAIT_TIMEOUT_SECONDS )) + local wait_notice_printed=0 + local merge_output="" + + while true; do + if merge_output="$("$GH_BIN" pr merge "$SOURCE_BRANCH" --squash --delete-branch 2>&1)"; then + return 0 + fi + if is_local_branch_delete_error "$merge_output"; then + echo "[agent-branch-finish] PR merged but gh could not delete the local branch (active worktree); continuing local cleanup." >&2 + return 0 + fi + + PR_STATE="" + PR_MERGED_AT="" + if read_pr_state; then + if [[ "$PR_STATE" == "MERGED" || -n "$PR_MERGED_AT" ]]; then + return 0 + fi + if [[ "$PR_STATE" == "CLOSED" ]]; then + echo "[agent-branch-finish] PR closed without merge; cannot continue auto-finish." >&2 + if [[ -n "$pr_url" ]]; then + echo "[agent-branch-finish] PR: ${pr_url}" >&2 + fi + if [[ -n "$merge_output" ]]; then + echo "$merge_output" >&2 + fi + return 1 + fi + fi + + if [[ "$wait_notice_printed" -eq 0 ]]; then + echo "[agent-branch-finish] Waiting for required checks/reviews, then retrying merge automatically (timeout ${WAIT_TIMEOUT_SECONDS}s)." >&2 + if [[ -n "$pr_url" ]]; then + echo "[agent-branch-finish] PR: ${pr_url}" >&2 + fi + wait_notice_printed=1 + fi + + if (( $(date +%s) >= deadline )); then + echo "[agent-branch-finish] Timed out waiting for PR merge after ${WAIT_TIMEOUT_SECONDS}s." >&2 + if [[ -n "$merge_output" ]]; then + echo "$merge_output" >&2 + fi + return 2 + fi + + sleep "$WAIT_POLL_SECONDS" + done +} + run_pr_flow() { if ! command -v "$GH_BIN" >/dev/null 2>&1; then echo "[agent-branch-finish] PR fallback requested but GitHub CLI not found: ${GH_BIN}" >&2 @@ -320,6 +439,11 @@ run_pr_flow() { return 0 fi + if [[ "$WAIT_FOR_MERGE" -eq 1 ]]; then + wait_for_pr_merge + return $? + fi + auto_output="" if auto_output="$("$GH_BIN" pr merge "$SOURCE_BRANCH" --squash --delete-branch --auto 2>&1)"; then echo "[agent-branch-finish] PR auto-merge enabled; waiting for required checks/reviews." >&2 @@ -365,6 +489,10 @@ if [[ "$PUSH_ENABLED" -eq 1 ]]; then if [[ -n "$pr_url" ]]; then echo "[agent-branch-finish] PR: ${pr_url}" >&2 fi + if [[ "$WAIT_FOR_MERGE" -eq 1 ]]; then + echo "[agent-branch-finish] Merge did not complete within wait window; keeping branch open." >&2 + exit 1 + fi echo "[agent-branch-finish] Merge pending review/check policy. Branch cleanup skipped for now." >&2 exit 0 fi @@ -390,16 +518,21 @@ fi if [[ "$CLEANUP_AFTER_MERGE" -eq 1 ]]; then if [[ "$source_worktree" == "$repo_root" ]]; then if is_clean_worktree "$source_worktree"; then - git -C "$source_worktree" checkout "$BASE_BRANCH" >/dev/null 2>&1 || true - if [[ "$PUSH_ENABLED" -eq 1 ]] && git -C "$repo_root" show-ref --verify --quiet "refs/remotes/origin/${BASE_BRANCH}"; then + switched_to_base=0 + if git -C "$source_worktree" checkout "$BASE_BRANCH" >/dev/null 2>&1; then + switched_to_base=1 + else + 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 fi fi - elif [[ "$source_worktree" == "$current_worktree" && "$source_worktree" == "${repo_root}/.omx/agent-worktrees"/* ]]; then + elif [[ "$source_worktree" == "$current_worktree" && "$source_worktree" == "${agent_worktree_root}"/* ]]; then git -C "$source_worktree" checkout --detach >/dev/null 2>&1 || true fi - if [[ "$source_worktree" != "$current_worktree" && "$source_worktree" == "${repo_root}/.omx/agent-worktrees"/* ]]; then + if [[ "$source_worktree" != "$current_worktree" && "$source_worktree" == "${agent_worktree_root}"/* ]]; then git -C "$repo_root" worktree remove "$source_worktree" --force >/dev/null 2>&1 || true fi @@ -423,7 +556,7 @@ if [[ "$CLEANUP_AFTER_MERGE" -eq 1 ]]; then fi echo "[agent-branch-finish] Merged '${SOURCE_BRANCH}' into '${BASE_BRANCH}' via ${merge_status} flow and cleaned source branch/worktree." - if [[ "$source_worktree" == "$current_worktree" && "$source_worktree" == "${repo_root}/.omx/agent-worktrees"/* ]]; then + if [[ "$source_worktree" == "$current_worktree" && "$source_worktree" == "${agent_worktree_root}"/* ]]; then echo "[agent-branch-finish] Current worktree '${source_worktree}' still exists because it is the active shell cwd." >&2 echo "[agent-branch-finish] Leave this directory, then run: bash scripts/agent-worktree-prune.sh --base ${BASE_BRANCH} --delete-branches" >&2 fi diff --git a/templates/scripts/codex-agent.sh b/templates/scripts/codex-agent.sh index 136c59c..a7d9f1c 100755 --- a/templates/scripts/codex-agent.sh +++ b/templates/scripts/codex-agent.sh @@ -8,7 +8,8 @@ BASE_BRANCH_EXPLICIT=0 CODEX_BIN="${MUSAFETY_CODEX_BIN:-codex}" AUTO_FINISH_RAW="${MUSAFETY_CODEX_AUTO_FINISH:-true}" AUTO_REVIEW_ON_CONFLICT_RAW="${MUSAFETY_CODEX_AUTO_REVIEW_ON_CONFLICT:-true}" -AUTO_CLEANUP_RAW="${MUSAFETY_CODEX_AUTO_CLEANUP:-false}" +AUTO_CLEANUP_RAW="${MUSAFETY_CODEX_AUTO_CLEANUP:-true}" +AUTO_WAIT_FOR_MERGE_RAW="${MUSAFETY_CODEX_WAIT_FOR_MERGE:-true}" normalize_bool() { local raw="${1:-}" @@ -25,7 +26,8 @@ normalize_bool() { AUTO_FINISH="$(normalize_bool "$AUTO_FINISH_RAW" "1")" AUTO_REVIEW_ON_CONFLICT="$(normalize_bool "$AUTO_REVIEW_ON_CONFLICT_RAW" "1")" -AUTO_CLEANUP="$(normalize_bool "$AUTO_CLEANUP_RAW" "0")" +AUTO_CLEANUP="$(normalize_bool "$AUTO_CLEANUP_RAW" "1")" +AUTO_WAIT_FOR_MERGE="$(normalize_bool "$AUTO_WAIT_FOR_MERGE_RAW" "1")" if [[ -n "$BASE_BRANCH" ]]; then BASE_BRANCH_EXPLICIT=1 @@ -74,6 +76,14 @@ while [[ $# -gt 0 ]]; do AUTO_CLEANUP=0 shift ;; + --wait-for-merge) + AUTO_WAIT_FOR_MERGE=1 + shift + ;; + --no-wait-for-merge) + AUTO_WAIT_FOR_MERGE=0 + shift + ;; --) shift break @@ -306,6 +316,9 @@ run_finish_flow() { if [[ "$AUTO_CLEANUP" -eq 1 ]]; then finish_args+=(--cleanup) fi + if [[ "$AUTO_WAIT_FOR_MERGE" -eq 1 ]]; then + finish_args+=(--wait-for-merge) + fi if has_origin_remote; then if command -v gh >/dev/null 2>&1 || command -v "${MUSAFETY_GH_BIN:-gh}" >/dev/null 2>&1; then @@ -368,7 +381,11 @@ auto_finish_completed=0 worktree_branch="$(git -C "$worktree_path" rev-parse --abbrev-ref HEAD 2>/dev/null || true)" if [[ "$AUTO_FINISH" -eq 1 && -n "$worktree_branch" && "$worktree_branch" != "HEAD" ]]; then - if [[ "$AUTO_CLEANUP" -eq 1 ]]; then + if [[ "$AUTO_WAIT_FOR_MERGE" -eq 1 && "$AUTO_CLEANUP" -eq 1 ]]; then + echo "[codex-agent] Auto-finish enabled: commit -> push/PR -> wait for merge -> cleanup." + elif [[ "$AUTO_WAIT_FOR_MERGE" -eq 1 ]]; then + echo "[codex-agent] Auto-finish enabled: commit -> push/PR -> wait for merge (keep branch/worktree)." + elif [[ "$AUTO_CLEANUP" -eq 1 ]]; then echo "[codex-agent] Auto-finish enabled: commit -> push/PR -> merge -> cleanup." else echo "[codex-agent] Auto-finish enabled: commit -> push/PR -> merge (keep branch/worktree)." @@ -401,7 +418,7 @@ if [[ -x "${repo_root}/scripts/agent-worktree-prune.sh" ]]; then if [[ "$BASE_BRANCH_EXPLICIT" -eq 1 ]]; then prune_args+=(--base "$BASE_BRANCH") fi - if [[ "$AUTO_CLEANUP" -eq 1 ]]; then + if [[ "$AUTO_CLEANUP" -eq 1 && "$auto_finish_completed" -eq 1 ]]; then prune_args+=(--delete-branches --delete-remote-branches) fi if ! bash "${repo_root}/scripts/agent-worktree-prune.sh" "${prune_args[@]}"; then diff --git a/test/install.test.js b/test/install.test.js index 7ae8a1a..9942ccb 100644 --- a/test/install.test.js +++ b/test/install.test.js @@ -1085,7 +1085,7 @@ test('codex-agent keeps dirty sandbox worktrees after session exit', () => { assert.equal(fs.existsSync(path.join(launchedCwd, 'codex-dirty.txt')), true); }); -test('codex-agent auto-finishes dirty sandbox branches via PR flow when origin is configured', () => { +test('codex-agent waits for PR merge completion and cleans merged sandbox branch/worktree by default', () => { const repoDir = initRepo(); seedCommit(repoDir); attachOriginRemote(repoDir); @@ -1113,6 +1113,8 @@ test('codex-agent auto-finishes dirty sandbox branches via PR flow when origin i ); fs.chmodSync(fakeCodexPath, 0o755); + const ghMergeState = path.join(repoDir, '.codex-agent-gh-merge-attempts'); + const { fakePath: fakeGhPath } = createFakeGhScript(` if [[ "$1" == "pr" && "$2" == "create" ]]; then exit 0 @@ -1126,6 +1128,16 @@ if [[ "$1" == "pr" && "$2" == "view" ]]; then exit 1 fi if [[ "$1" == "pr" && "$2" == "merge" ]]; then + attempts=0 + if [[ -f "${'${MUSAFETY_TEST_GH_MERGE_STATE}'}" ]]; then + attempts="$(cat "${'${MUSAFETY_TEST_GH_MERGE_STATE}'}")" + fi + attempts=$((attempts + 1)) + echo "$attempts" > "${'${MUSAFETY_TEST_GH_MERGE_STATE}'}" + if [[ "$attempts" -lt 2 ]]; then + echo "Required status check \\"test (node 22)\\" is expected." >&2 + exit 1 + fi exit 0 fi echo "unexpected gh args: $*" >&2 @@ -1142,21 +1154,25 @@ exit 1 PATH: `${fakeCodexBin}:${process.env.PATH}`, MUSAFETY_TEST_CODEX_CWD: cwdMarker, MUSAFETY_TEST_CODEX_ARGS: argsMarker, + MUSAFETY_TEST_GH_MERGE_STATE: ghMergeState, MUSAFETY_GH_BIN: fakeGhPath, + MUSAFETY_FINISH_WAIT_TIMEOUT_SECONDS: '60', + MUSAFETY_FINISH_WAIT_POLL_SECONDS: '0', }, ); assert.equal(launch.status, 0, launch.stderr || launch.stdout); - assert.match(launch.stdout, /\[codex-agent\] Auto-finish enabled: commit -> push\/PR -> merge \(keep branch\/worktree\)\./); + assert.match(launch.stdout, /\[codex-agent\] Auto-finish enabled: commit -> push\/PR -> wait for merge -> cleanup\./); assert.match(launch.stdout, /\[codex-agent\] Auto-finish completed for/); - assert.match(launch.stdout, /\[codex-agent\] Sandbox worktree kept:/); + assert.match(launch.stdout, /\[codex-agent\] Auto-cleaned sandbox worktree:/); + assert.equal(fs.readFileSync(ghMergeState, 'utf8').trim(), '2', 'finish flow should retry merge until checks are ready'); const launchedCwd = fs.readFileSync(cwdMarker, 'utf8').trim(); - assert.equal(fs.existsSync(launchedCwd), true, 'auto-finished sandbox should stay until explicit cleanup'); + assert.equal(fs.existsSync(launchedCwd), false, 'auto-finished sandbox should be cleaned by default'); const launchedBranch = extractCreatedBranch(launch.stdout); result = runCmd('git', ['show-ref', '--verify', '--quiet', `refs/heads/${launchedBranch}`], repoDir); - assert.equal(result.status, 0, 'auto-finished branch should remain locally by default'); + assert.notEqual(result.status, 0, 'auto-finished branch should be removed locally by default'); result = runCmd('git', ['ls-remote', '--heads', 'origin', launchedBranch], repoDir); - assert.match(result.stdout, /refs\/heads\//, 'auto-finished branch should remain on origin by default'); + assert.equal(result.stdout.trim(), '', 'auto-finished branch should be removed on origin by default'); const launchedArgs = fs.readFileSync(argsMarker, 'utf8').trim(); assert.match(launchedArgs, /--model gpt-5\.4-mini/); @@ -1413,6 +1429,189 @@ exit 1 assert.equal(result.stdout.trim(), '', 'agent branch should be deleted on origin'); }); +test('agent-branch-finish cleanup succeeds from active agent worktree when base branch is checked out elsewhere', () => { + 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); + + const agentWorktreePath = path.join(repoDir, '.omx', 'agent-worktrees', 'agent__active-cleanup'); + result = runCmd( + 'git', + ['worktree', 'add', '-b', 'agent/test-active-worktree-cleanup', agentWorktreePath, 'dev'], + repoDir, + ); + assert.equal(result.status, 0, result.stderr || result.stdout); + + fs.writeFileSync(path.join(agentWorktreePath, 'active-worktree-cleanup.txt'), 'cleanup from active worktree\n', 'utf8'); + result = runCmd( + 'git', + ['add', 'active-worktree-cleanup.txt'], + agentWorktreePath, + ); + assert.equal(result.status, 0, result.stderr); + result = runCmd('git', ['commit', '--no-verify', '-m', 'active worktree cleanup change'], agentWorktreePath); + assert.equal(result.status, 0, result.stderr || result.stdout); + result = runCmd('git', ['push', '-u', 'origin', 'agent/test-active-worktree-cleanup'], agentWorktreePath); + assert.equal(result.status, 0, result.stderr || result.stdout); + + 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/active-cleanup" + exit 0 + fi + echo "unexpected gh pr view args: $*" >&2 + exit 1 +fi +if [[ "$1" == "pr" && "$2" == "merge" ]]; then + exit 0 +fi +echo "unexpected gh args: $*" >&2 +exit 1 +`); + + const finish = runCmd( + 'bash', + [ + path.join(repoDir, 'scripts', 'agent-branch-finish.sh'), + '--branch', + 'agent/test-active-worktree-cleanup', + '--base', + 'dev', + '--mode', + 'pr', + '--cleanup', + ], + agentWorktreePath, + { MUSAFETY_GH_BIN: fakeGhPath }, + ); + assert.equal(finish.status, 0, finish.stderr || finish.stdout); + assert.match( + finish.stdout, + /Merged 'agent\/test-active-worktree-cleanup' into 'dev' via pr flow and cleaned source branch\/worktree\./, + ); + assert.match(finish.stderr, /Current worktree '.+' still exists because it is the active shell cwd/); + + result = runCmd('git', ['show-ref', '--verify', '--quiet', 'refs/heads/agent/test-active-worktree-cleanup'], repoDir); + assert.notEqual(result.status, 0, 'agent branch should be deleted locally'); + result = runCmd('git', ['ls-remote', '--heads', 'origin', 'agent/test-active-worktree-cleanup'], repoDir); + assert.equal(result.stdout.trim(), '', 'agent branch should be deleted on origin'); + assert.equal(fs.existsSync(agentWorktreePath), true, 'active cwd worktree should remain until manual prune'); + result = runCmd('git', ['rev-parse', '--abbrev-ref', 'HEAD'], agentWorktreePath); + assert.equal(result.status, 0, result.stderr || result.stdout); + assert.equal(result.stdout.trim(), 'HEAD', 'active worktree should detach before local branch deletion'); +}); + +test('agent-branch-finish waits for required checks in PR mode and merges when ready', () => { + 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-wait-merge'], repoDir); + assert.equal(result.status, 0, result.stderr); + commitFile(repoDir, 'agent-pr-wait.txt', 'agent wait merge\n', 'agent wait merge change'); + + const ghMergeState = path.join(repoDir, '.finish-gh-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/2" + exit 0 + fi + if [[ " $* " == *" --json state,mergedAt,url "* ]]; then + attempts=0 + if [[ -f "${'${MUSAFETY_TEST_GH_MERGE_STATE}'}" ]]; then + attempts="$(cat "${'${MUSAFETY_TEST_GH_MERGE_STATE}'}")" + fi + if [[ "$attempts" -ge 2 ]]; then + echo -e "MERGED\\t2026-04-12T00:00:00Z\\thttps://example.test/pr/2" + else + echo -e "OPEN\\t\\thttps://example.test/pr/2" + fi + exit 0 + fi + echo "unexpected gh pr view args: $*" >&2 + exit 1 +fi +if [[ "$1" == "pr" && "$2" == "merge" ]]; then + attempts=0 + if [[ -f "${'${MUSAFETY_TEST_GH_MERGE_STATE}'}" ]]; then + attempts="$(cat "${'${MUSAFETY_TEST_GH_MERGE_STATE}'}")" + fi + attempts=$((attempts + 1)) + echo "$attempts" > "${'${MUSAFETY_TEST_GH_MERGE_STATE}'}" + if [[ "$attempts" -lt 2 ]]; then + echo "Required status check \\"test (node 22)\\" is expected." >&2 + exit 1 + fi + exit 0 +fi +echo "unexpected gh args: $*" >&2 +exit 1 +`); + + const finish = runCmd( + 'bash', + [ + 'scripts/agent-branch-finish.sh', + '--branch', + 'agent/test-pr-wait-merge', + '--mode', + 'pr', + '--cleanup', + '--wait-for-merge', + '--wait-timeout-seconds', + '60', + '--wait-poll-seconds', + '0', + ], + repoDir, + { + MUSAFETY_GH_BIN: fakeGhPath, + MUSAFETY_TEST_GH_MERGE_STATE: ghMergeState, + }, + ); + assert.equal(finish.status, 0, finish.stderr || finish.stdout); + assert.equal(fs.readFileSync(ghMergeState, 'utf8').trim(), '2', 'finish flow should retry merge until checks are ready'); + assert.match( + finish.stdout, + /Merged 'agent\/test-pr-wait-merge' into 'dev' via pr flow and cleaned source branch\/worktree\./, + ); + + result = runCmd('git', ['show-ref', '--verify', '--quiet', 'refs/heads/agent/test-pr-wait-merge'], repoDir); + assert.notEqual(result.status, 0, 'agent branch should be deleted locally after wait+merge cleanup'); + result = runCmd('git', ['ls-remote', '--heads', 'origin', 'agent/test-pr-wait-merge'], repoDir); + assert.equal(result.stdout.trim(), '', 'agent branch should be deleted on origin after wait+merge cleanup'); +}); + test('OpenSpec plan workspace scaffold creates expected role/task structure', () => { const repoDir = initRepo();