ci(wiki-maintenance): verify the PR merged instead of trusting the merge status #895

2026-09-21T08:06:41Z

claude-bot commented

2026-09-21 08:06:41 +00:00

Run 10254 (manual, the picker test): checkout, distill and audit succeeded, PR #2 on k.renner/notes was created (201) and is merged (verified: state merged at 10:05, and wiki-index synced the resulting main at 08:05 UTC with 4 changed notes, 1727 total). But POST .../pulls/2/merge with delete_branch_after_merge:true answered HTTP 500 with an empty body, the step failed, and the branch wiki-maintenance/2026-09-21-10254 stayed behind (deleted by hand now, HTTP 204).

The step now merges with {"Do":"merge"} only, reads the PR back and requires merged=true, then deletes the branch as a separate best-effort DELETE /branches/{name}. A merge that did not land still fails the run and leaves the PR open. Lint passes.

🤖 Generated with Claude Code

Run 10254 (manual, the picker test): checkout, distill and audit succeeded, PR #2 on k.renner/notes was created (201) and is merged (verified: state merged at 10:05, and wiki-index synced the resulting main at 08:05 UTC with 4 changed notes, 1727 total). But POST .../pulls/2/merge with delete_branch_after_merge:true answered HTTP 500 with an empty body, the step failed, and the branch wiki-maintenance/2026-09-21-10254 stayed behind (deleted by hand now, HTTP 204). The step now merges with {"Do":"merge"} only, reads the PR back and requires merged=true, then deletes the branch as a separate best-effort DELETE /branches/{name}. A merge that did not land still fails the run and leaves the PR open. Lint passes. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

claude-bot added 1 commit 2026-09-21 08:06:41 +00:00

ci(wiki-maintenance): verify the PR merged instead of trusting the merge status

ci/woodpecker/pr/ci-scripts-test Pipeline was successful

Details

ci/woodpecker/pr/ci-manifests-test Pipeline was successful

Details

ci/woodpecker/pr/pr-review Pipeline was successful

Details

0279210595

Run 10254: POST .../merge with delete_branch_after_merge returned
HTTP 500 although PR #2 on k.renner/notes was merged, so the step
failed and left the branch behind. The merge call no longer asks Gitea
to delete the branch; the step reads the PR back and requires
merged=true, then deletes the branch as a separate best-effort call.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

review-bot reviewed

2026-09-21 08:09:12 +00:00

review-bot left a comment

🤖 Automated review — model claude-sonnet-5 (complexity: normal), commit 027921059571ac0c6fbe6487f616ebb4a647ad6f.

Advisory review by the review-bot pipeline. It never approves; a human merges. Push new commits to get a fresh review.

Summary

Single-file change to .woodpecker/wiki-maintenance.yml's push-notes step: instead of trusting the HTTP status of the POST .../pulls/{n}/merge call, it now re-fetches the PR and checks merged == true, and moves branch deletion out of the merge call into a separate best-effort DELETE .../branches/{name} afterwards. This directly targets the documented failure (run 10254: merge landed but the combined merge+delete-branch call returned HTTP 500, leaving a stale branch and a failed run). The logic is sound and no blocking issues were found.

🟡 Suggestions

.woodpecker/wiki-maintenance.yml:135 — The merged-state check (curl .../pulls/$$NUM | jq -r '.merged') runs exactly once, immediately after the merge POST returns. The fix assumes the merge is fully reflected by the time this GET fires. That assumption is plausible (the previous 500 looks like it came from response generation after the merge itself completed, e.g. from the now-removed delete_branch_after_merge:true), but nothing in the diff or PR body confirms Gitea's merge endpoint is synchronous with respect to a follow-up GET. If it ever isn't, this step would falsely report merged: false, exit 1, and skip branch deletion on a merge that would have landed moments later — trading the old false-positive risk for a smaller false-negative one. A one-shot retry (e.g. check, sleep a couple seconds, check again) before giving up would close this gap cheaply. Not blocking since the evidence in the PR body supports the synchronous-merge theory.

Tests

No automated tests apply to this Woodpecker pipeline YAML (none exist elsewhere in the file for similar steps either). The PR body documents manual verification via a real triggered run (#10254) plus a stated Lint passes, which is the normal verification bar for this kind of change.

Recommendation

Looks reasonable — no blocking issues; comment above is optional.

🤖 **Automated review** — model `claude-sonnet-5` (complexity: normal), commit `027921059571ac0c6fbe6487f616ebb4a647ad6f`. > Advisory review by the `review-bot` pipeline. It never approves; a human merges. Push new commits to get a fresh review. --- ## Summary Single-file change to `.woodpecker/wiki-maintenance.yml`'s `push-notes` step: instead of trusting the HTTP status of the `POST .../pulls/{n}/merge` call, it now re-fetches the PR and checks `merged == true`, and moves branch deletion out of the merge call into a separate best-effort `DELETE .../branches/{name}` afterwards. This directly targets the documented failure (run 10254: merge landed but the combined merge+delete-branch call returned HTTP 500, leaving a stale branch and a failed run). The logic is sound and no blocking issues were found. ## 🟡 Suggestions - `.woodpecker/wiki-maintenance.yml:135` — The merged-state check (`curl .../pulls/$$NUM | jq -r '.merged'`) runs exactly once, immediately after the merge POST returns. The fix assumes the merge is fully reflected by the time this GET fires. That assumption is plausible (the previous 500 looks like it came from response generation *after* the merge itself completed, e.g. from the now-removed `delete_branch_after_merge:true`), but nothing in the diff or PR body confirms Gitea's merge endpoint is synchronous with respect to a follow-up GET. If it ever isn't, this step would falsely report `merged: false`, `exit 1`, and skip branch deletion on a merge that would have landed moments later — trading the old false-positive risk for a smaller false-negative one. A one-shot retry (e.g. check, sleep a couple seconds, check again) before giving up would close this gap cheaply. Not blocking since the evidence in the PR body supports the synchronous-merge theory. ## Tests No automated tests apply to this Woodpecker pipeline YAML (none exist elsewhere in the file for similar steps either). The PR body documents manual verification via a real triggered run (#10254) plus a stated `Lint passes`, which is the normal verification bar for this kind of change. ## Recommendation Looks reasonable — no blocking issues; comment above is optional.

k.renner merged commit 63701fbebb into master

2026-09-21 08:49:11 +00:00

k.renner referenced this issue from a commit

2026-09-21 08:49:12 +00:00

Merge pull request 'ci(wiki-maintenance): verify the PR merged instead of trusting the merge status' (#895) from fix/wiki-maintenance-merge-verify into master

Sign in to join this conversation.

2 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: konairius/homelab#895