diff --git a/openspec/changes/agent-codex-prevent-finish-rerun-from-opening-duplic-2026-04-22-23-58/.openspec.yaml b/openspec/changes/agent-codex-prevent-finish-rerun-from-opening-duplic-2026-04-22-23-58/.openspec.yaml new file mode 100644 index 0000000..25345f4 --- /dev/null +++ b/openspec/changes/agent-codex-prevent-finish-rerun-from-opening-duplic-2026-04-22-23-58/.openspec.yaml @@ -0,0 +1,2 @@ +schema: spec-driven +created: 2026-04-22 diff --git a/openspec/changes/agent-codex-prevent-finish-rerun-from-opening-duplic-2026-04-22-23-58/notes.md b/openspec/changes/agent-codex-prevent-finish-rerun-from-opening-duplic-2026-04-22-23-58/notes.md new file mode 100644 index 0000000..fa555ff --- /dev/null +++ b/openspec/changes/agent-codex-prevent-finish-rerun-from-opening-duplic-2026-04-22-23-58/notes.md @@ -0,0 +1,20 @@ +# agent-codex-prevent-finish-rerun-from-opening-duplic-2026-04-22-23-58 (minimal / T1) + +Branch: `agent/codex/prevent-finish-rerun-from-opening-duplic-2026-04-22-23-58` + +`gx branch finish --via-pr --wait-for-merge --cleanup` can reopen a fresh PR when the same source branch head already shipped in an earlier merged PR but the local branch/worktree cleanup was left behind. That turns a cleanup rerun into a new merge-wait loop instead of the bounded cleanup pass the user expected. + +Scope: +- Before PR create/merge, detect whether the current source branch HEAD already matches a merged PR for the same branch/base pair. +- If that exact head already landed, skip new PR creation and continue straight to local cleanup while preserving merged PR context in the logs. +- Add a focused finish regression that fails if a rerun opens or merges a duplicate PR for an already-merged head. + +Verification: +- `bash -n scripts/agent-branch-finish.sh templates/scripts/agent-branch-finish.sh` +- `node --test test/finish.test.js` + +## Cleanup + +- [ ] Run `gx branch finish --branch agent/codex/prevent-finish-rerun-from-opening-duplic-2026-04-22-23-58 --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 3e7e149..456abae 100755 --- a/scripts/agent-branch-finish.sh +++ b/scripts/agent-branch-finish.sh @@ -455,6 +455,44 @@ read_pr_state() { return 0 } +read_merged_pr_for_head() { + local head_sha="${1:-}" + local state_line="" + local parsed_state="" + local parsed_merged_at="" + local parsed_url="" + + if [[ -z "$head_sha" ]]; then + return 1 + fi + + state_line="$("$GH_BIN" pr list \ + --state merged \ + --head "$SOURCE_BRANCH" \ + --base "$BASE_BRANCH" \ + --json state,mergedAt,url,headRefOid \ + --jq "map(select(.headRefOid == \"$head_sha\")) | sort_by(.mergedAt // \"\") | reverse | (.[0] // {}) | [(.state // \"\"), (.mergedAt // \"\"), (.url // \"\")] | join(\"\u001f\")" \ + 2>/dev/null || true)" + if [[ -z "$state_line" ]]; then + return 1 + fi + + IFS=$'\x1f' read -r parsed_state parsed_merged_at parsed_url <<< "$state_line" + if [[ -z "$parsed_state" && -z "$parsed_merged_at" && -z "$parsed_url" ]]; then + return 1 + fi + if [[ "$parsed_state" != "MERGED" && -z "$parsed_merged_at" ]]; then + return 1 + fi + + 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 )) @@ -509,11 +547,22 @@ wait_for_pr_merge() { } run_pr_flow() { + local source_head_sha="" + 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 return 1 fi + source_head_sha="$(git -C "$repo_root" rev-parse "$SOURCE_BRANCH" 2>/dev/null || true)" + if read_merged_pr_for_head "$source_head_sha"; then + echo "[agent-branch-finish] Source branch head already landed in a merged PR; skipping new PR creation and continuing cleanup." >&2 + if [[ -n "$pr_url" ]]; then + echo "[agent-branch-finish] Merged PR: ${pr_url}" >&2 + fi + return 0 + fi + git -C "$source_worktree" push -u origin "$SOURCE_BRANCH" pr_title="$(git -C "$repo_root" log -1 --pretty=%s "$SOURCE_BRANCH" 2>/dev/null || true)" diff --git a/templates/scripts/agent-branch-finish.sh b/templates/scripts/agent-branch-finish.sh index 3e7e149..456abae 100755 --- a/templates/scripts/agent-branch-finish.sh +++ b/templates/scripts/agent-branch-finish.sh @@ -455,6 +455,44 @@ read_pr_state() { return 0 } +read_merged_pr_for_head() { + local head_sha="${1:-}" + local state_line="" + local parsed_state="" + local parsed_merged_at="" + local parsed_url="" + + if [[ -z "$head_sha" ]]; then + return 1 + fi + + state_line="$("$GH_BIN" pr list \ + --state merged \ + --head "$SOURCE_BRANCH" \ + --base "$BASE_BRANCH" \ + --json state,mergedAt,url,headRefOid \ + --jq "map(select(.headRefOid == \"$head_sha\")) | sort_by(.mergedAt // \"\") | reverse | (.[0] // {}) | [(.state // \"\"), (.mergedAt // \"\"), (.url // \"\")] | join(\"\u001f\")" \ + 2>/dev/null || true)" + if [[ -z "$state_line" ]]; then + return 1 + fi + + IFS=$'\x1f' read -r parsed_state parsed_merged_at parsed_url <<< "$state_line" + if [[ -z "$parsed_state" && -z "$parsed_merged_at" && -z "$parsed_url" ]]; then + return 1 + fi + if [[ "$parsed_state" != "MERGED" && -z "$parsed_merged_at" ]]; then + return 1 + fi + + 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 )) @@ -509,11 +547,22 @@ wait_for_pr_merge() { } run_pr_flow() { + local source_head_sha="" + 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 return 1 fi + source_head_sha="$(git -C "$repo_root" rev-parse "$SOURCE_BRANCH" 2>/dev/null || true)" + if read_merged_pr_for_head "$source_head_sha"; then + echo "[agent-branch-finish] Source branch head already landed in a merged PR; skipping new PR creation and continuing cleanup." >&2 + if [[ -n "$pr_url" ]]; then + echo "[agent-branch-finish] Merged PR: ${pr_url}" >&2 + fi + return 0 + fi + git -C "$source_worktree" push -u origin "$SOURCE_BRANCH" pr_title="$(git -C "$repo_root" log -1 --pretty=%s "$SOURCE_BRANCH" 2>/dev/null || true)" diff --git a/test/finish.test.js b/test/finish.test.js index a03eaf5..07b3959 100644 --- a/test/finish.test.js +++ b/test/finish.test.js @@ -466,6 +466,85 @@ exit 1 assert.equal(result.stdout.trim(), '', 'agent branch should be deleted on origin'); }); +test('agent-branch-finish cleanup skips duplicate PR creation when the current head already landed in a merged PR', () => { + 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-merged-head-rerun'], repoDir); + assert.equal(result.status, 0, result.stderr); + commitFile(repoDir, 'agent-pr-merged-head-rerun.txt', 'merged head rerun\n', 'merged head rerun change'); + result = runCmd('git', ['push', '-u', 'origin', 'agent/test-pr-merged-head-rerun'], repoDir); + assert.equal(result.status, 0, result.stderr || result.stdout); + + result = runCmd('git', ['rev-parse', 'HEAD'], repoDir); + assert.equal(result.status, 0, result.stderr || result.stdout); + const headSha = result.stdout.trim(); + + const { fakePath: fakeGhPath } = createFakeGhScript(` +if [[ "$1" == "pr" && "$2" == "list" ]]; then + if [[ " $* " == *" --state merged "* ]] && [[ " $* " == *" --json state,mergedAt,url,headRefOid "* ]]; then + printf 'MERGED\\x1f2026-04-22T21:37:13Z\\x1fhttps://example.test/pr/already-merged-head\\n' + exit 0 + fi + echo "unexpected gh pr list args: $*" >&2 + exit 1 +fi +if [[ "$1" == "pr" && "$2" == "create" ]]; then + echo "duplicate PR creation should have been skipped" >&2 + exit 1 +fi +if [[ "$1" == "pr" && "$2" == "merge" ]]; then + echo "duplicate PR merge should have been skipped" >&2 + exit 1 +fi +if [[ "$1" == "pr" && "$2" == "view" ]]; then + echo "unexpected gh pr view args: $*" >&2 + exit 1 +fi +echo "unexpected gh args: $*" >&2 +exit 1 +`); + + const finish = runBranchFinish( + ['--branch', 'agent/test-pr-merged-head-rerun', '--base', 'dev', '--mode', 'pr', '--cleanup'], + repoDir, + { + GUARDEX_GH_BIN: fakeGhPath, + GUARDEX_TEST_HEAD_SHA: headSha, + }, + ); + assert.equal(finish.status, 0, finish.stderr || finish.stdout); + assert.match( + finish.stderr, + /Source branch head already landed in a merged PR; skipping new PR creation and continuing cleanup\./, + ); + assert.match( + finish.stderr, + /Merged PR: https:\/\/example\.test\/pr\/already-merged-head/, + ); + assert.match( + finish.stdout, + /Merged 'agent\/test-pr-merged-head-rerun' into 'dev' via pr flow and cleaned source branch\/worktree\./, + ); + + result = runCmd('git', ['show-ref', '--verify', '--quiet', 'refs/heads/agent/test-pr-merged-head-rerun'], repoDir); + assert.notEqual(result.status, 0, 'agent branch should be deleted locally'); + result = runCmd('git', ['ls-remote', '--heads', 'origin', 'agent/test-pr-merged-head-rerun'], repoDir); + 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();