From 2323bac8574badec6eed31cbd5d3b1dc076b7e57 Mon Sep 17 00:00:00 2001 From: ptt <1928627998@qq.com> Date: Sat, 29 Aug 2026 15:25:30 +0800 Subject: [PATCH] fix(land-and-deploy): check fork branch in head repo MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Wave-amended: gh leaves .headRepository.nameWithOwner empty (verified live against gh 2.83) — owner/name now composed from headRepositoryOwner.login + headRepository.name so reconciliation is not a permanent no-op; fork branches get report-not-delete (maintainers lack fork push rights); pins updated --- land-and-deploy/sections/merge-and-deploy.md | 16 +++++++++++++--- .../sections/merge-and-deploy.md.tmpl | 16 +++++++++++++--- test/land-and-deploy-postfail.test.ts | 18 ++++++++++++++---- 3 files changed, 40 insertions(+), 10 deletions(-) diff --git a/land-and-deploy/sections/merge-and-deploy.md b/land-and-deploy/sections/merge-and-deploy.md index 1ca7083d3..5259dea94 100644 --- a/land-and-deploy/sections/merge-and-deploy.md +++ b/land-and-deploy/sections/merge-and-deploy.md @@ -78,14 +78,24 @@ Identify candidates: a worktree is stale if (a) it is checked out on the base br Remote-branch reconciliation — the failed `gh pr merge` carried `--delete-branch`, and this recovery path must not silently drop that half. The success path above says "The branch has been cleaned up"; this path states the branch outcome explicitly instead of staying silent: ```bash -BRANCH=$(gh pr view --json headRefName -q .headRefName) -git ls-remote --heads origin "$BRANCH" +# NB: gh leaves .headRepository.nameWithOwner EMPTY (verified against gh +# 2.83); compose owner/name from headRepositoryOwner.login + headRepository.name. +gh pr view --json headRepositoryOwner,headRepository,headRefName \ + --jq '"\(.headRepositoryOwner.login)/\(.headRepository.name)\t\(.headRefName)"' +git ls-remote --heads "https://github.com/.git" "" ``` +Record the first field as `` (`owner/name`) and the second as +``, then substitute both into `git ls-remote`. The PR head repository is +the authoritative branch location: for same-repository PRs it is the base repository; +for fork PRs it is the fork. Do not substitute the checkout's `origin`. If the metadata +lookup fails or either field is empty or contains a bare `/`, treat the branch state as +unknown and do not run the deletion path. + Three outcomes — never read a failed check as a clean branch: - **Exit 0, empty output** — the remote branch is already gone (GitHub's post-merge deletion or a concurrent actor got there). Tell the user: "The remote branch has already been cleaned up." This makes re-runs of the recovery idempotent. -- **Exit 0, one ref line** — the branch survived: the failed merge command never reached its `--delete-branch` half. OFFER deletion, confirm-first (matching the worktree-cleanup posture above): "The remote branch `` still exists — the failed merge never ran its --delete-branch half. Delete it?" Only on confirmation: `git push origin --delete "$BRANCH"`. If a local branch of the same name exists, offer `git branch -d "$BRANCH"` alongside (`-d`, never `-D` — a non-fast-forwarded local branch is the user's call). +- **Exit 0, one ref line** — the branch survived: the failed merge command never reached its `--delete-branch` half. If `` is the BASE repository, OFFER deletion, confirm-first (matching the worktree-cleanup posture above): "The remote branch `` still exists in `` — the failed merge never ran its --delete-branch half. Delete it?" Only on confirmation: `git push "https://github.com/.git" --delete ""`. If `` is a FORK, do not offer deletion — the branch belongs to the contributor and the maintainer typically has no push rights there; report instead: "The branch lives on the contributor's fork `` — leaving it to them." If a local branch of the same name exists, offer `git branch -d ""` alongside (`-d`, never `-D` — a non-fast-forwarded local branch is the user's call). - **Non-zero exit** — the check ITSELF failed (network, auth). Tell the user: "Couldn't verify remote branch state — leaving it alone." and skip the deletion offer entirely; a failed check is unknown state, not a clean branch. Record `MERGE_PATH=direct`, then continue to §4a (CI auto-deploy detection). diff --git a/land-and-deploy/sections/merge-and-deploy.md.tmpl b/land-and-deploy/sections/merge-and-deploy.md.tmpl index 6c82d2d0e..22841505a 100644 --- a/land-and-deploy/sections/merge-and-deploy.md.tmpl +++ b/land-and-deploy/sections/merge-and-deploy.md.tmpl @@ -76,14 +76,24 @@ Identify candidates: a worktree is stale if (a) it is checked out on the base br Remote-branch reconciliation — the failed `gh pr merge` carried `--delete-branch`, and this recovery path must not silently drop that half. The success path above says "The branch has been cleaned up"; this path states the branch outcome explicitly instead of staying silent: ```bash -BRANCH=$(gh pr view --json headRefName -q .headRefName) -git ls-remote --heads origin "$BRANCH" +# NB: gh leaves .headRepository.nameWithOwner EMPTY (verified against gh +# 2.83); compose owner/name from headRepositoryOwner.login + headRepository.name. +gh pr view --json headRepositoryOwner,headRepository,headRefName \ + --jq '"\(.headRepositoryOwner.login)/\(.headRepository.name)\t\(.headRefName)"' +git ls-remote --heads "https://github.com/.git" "" ``` +Record the first field as `` (`owner/name`) and the second as +``, then substitute both into `git ls-remote`. The PR head repository is +the authoritative branch location: for same-repository PRs it is the base repository; +for fork PRs it is the fork. Do not substitute the checkout's `origin`. If the metadata +lookup fails or either field is empty or contains a bare `/`, treat the branch state as +unknown and do not run the deletion path. + Three outcomes — never read a failed check as a clean branch: - **Exit 0, empty output** — the remote branch is already gone (GitHub's post-merge deletion or a concurrent actor got there). Tell the user: "The remote branch has already been cleaned up." This makes re-runs of the recovery idempotent. -- **Exit 0, one ref line** — the branch survived: the failed merge command never reached its `--delete-branch` half. OFFER deletion, confirm-first (matching the worktree-cleanup posture above): "The remote branch `` still exists — the failed merge never ran its --delete-branch half. Delete it?" Only on confirmation: `git push origin --delete "$BRANCH"`. If a local branch of the same name exists, offer `git branch -d "$BRANCH"` alongside (`-d`, never `-D` — a non-fast-forwarded local branch is the user's call). +- **Exit 0, one ref line** — the branch survived: the failed merge command never reached its `--delete-branch` half. If `` is the BASE repository, OFFER deletion, confirm-first (matching the worktree-cleanup posture above): "The remote branch `` still exists in `` — the failed merge never ran its --delete-branch half. Delete it?" Only on confirmation: `git push "https://github.com/.git" --delete ""`. If `` is a FORK, do not offer deletion — the branch belongs to the contributor and the maintainer typically has no push rights there; report instead: "The branch lives on the contributor's fork `` — leaving it to them." If a local branch of the same name exists, offer `git branch -d ""` alongside (`-d`, never `-D` — a non-fast-forwarded local branch is the user's call). - **Non-zero exit** — the check ITSELF failed (network, auth). Tell the user: "Couldn't verify remote branch state — leaving it alone." and skip the deletion offer entirely; a failed check is unknown state, not a clean branch. Record `MERGE_PATH=direct`, then continue to §4a (CI auto-deploy detection). diff --git a/test/land-and-deploy-postfail.test.ts b/test/land-and-deploy-postfail.test.ts index f2f65ea53..451aa7faa 100644 --- a/test/land-and-deploy-postfail.test.ts +++ b/test/land-and-deploy-postfail.test.ts @@ -92,11 +92,19 @@ describe("PR #1620 §4a-postfail in land-and-deploy template", () => { // #2656: the failed merge carried --delete-branch; the recovery path must // reconcile the remote branch instead of silently dropping that half. - test("MERGED branch reconciles the remote branch (ls-remote, confirm-first delete)", () => { + // #2696: that reconciliation must target the PR head repository, not the + // base checkout's origin, because fork branches do not exist in origin. + test("MERGED branch reconciles the PR head repository (ls-remote, confirm-first delete)", () => { const body = readTmpl(); - expect(body).toMatch(/git ls-remote --heads origin "\$BRANCH"/); - expect(body).toMatch(/gh pr view --json headRefName -q \.headRefName/); - expect(body).toMatch(/git push origin --delete "\$BRANCH"/); + expect(body).toMatch(/gh pr view --json headRepositoryOwner,headRepository,headRefName/); + // gh leaves .headRepository.nameWithOwner empty (verified live, gh 2.83) — + // owner/name is composed from headRepositoryOwner.login + headRepository.name. + expect(body).toMatch(/headRepositoryOwner\.login/); + expect(body).not.toMatch(/\[\.headRepository\.nameWithOwner/); + expect(body).toMatch(/git ls-remote --heads "https:\/\/github\.com\/\.git" ""/); + expect(body).toMatch(/git push "https:\/\/github\.com\/\.git" --delete ""/); + expect(body).not.toMatch(/git ls-remote --heads origin/); + expect(body).not.toMatch(/git push origin --delete/); // Confirm-first: deletion is offered, never unilateral. expect(body).toMatch(/Delete it\?/); }); @@ -130,5 +138,7 @@ describe("PR #1620 §4a-postfail in land-and-deploy template", () => { const md = readMd(); expect(md).toMatch(/### 4a-postfail: Post-failure PR-state check/); expect(md).toMatch(/state == "MERGED"/); + expect(md).toMatch(/headRepositoryOwner\.login/); + expect(md).not.toMatch(/git ls-remote --heads origin/); }); });