feat(automerge): confirm-deploy PR comment + delete branch on merge #257

Merged

joshtronic merged 1 commit from feat/automerge-confirm-and-delete into master

2026-06-26 01:50:41 +00:00

igor commented

2026-06-26 01:24:15 +00:00

Collaborator

Two bits of polish from watching the first live run.

1. Confirm-deploy comment on the PR. The deploy barrier now posts a comment when it finishes — a ✅ confirm on verify ("deploy verified, site is live") and a ⚠️ failure note (alongside the email) on a failed CI/smoke. Visible record on the PR, not just buried in the logs.

2. Delete the branch on merge. You were right it's not a Forgejo bug: the repo's "Delete branch after merge by default" only pre-checks the box in the UI merge dialog. An API merge has to send delete_branch_after_merge: true, which automerge_do_merge wasn't. Added — so auto-merged branches get cleaned up like your manual merges do. (Also deleted the leftover docs/readme-agent-index branch from the #60 run.)

Tests: merge body now asserts Do=merge + delete_branch_after_merge; the barrier asserts a confirm comment on healthy and a failure comment on CI-failure. bash -n + check-sync green. No reviewer.

Two bits of polish from watching the first live run. **1. Confirm-deploy comment on the PR.** The deploy barrier now posts a comment when it finishes — a ✅ confirm on verify (*"deploy verified, site is live"*) and a ⚠️ failure note (alongside the email) on a failed CI/smoke. Visible record on the PR, not just buried in the logs. **2. Delete the branch on merge.** You were right it's not a Forgejo bug: the repo's *"Delete branch after merge by default"* only pre-checks the box in the **UI** merge dialog. An **API** merge has to send `delete_branch_after_merge: true`, which `automerge_do_merge` wasn't. Added — so auto-merged branches get cleaned up like your manual merges do. (Also deleted the leftover `docs/readme-agent-index` branch from the #60 run.) Tests: merge body now asserts `Do=merge` + `delete_branch_after_merge`; the barrier asserts a confirm comment on healthy and a failure comment on CI-failure. `bash -n` + `check-sync` green. No reviewer.

igor added 1 commit

2026-06-26 01:24:15 +00:00

feat(automerge): confirm-deploy PR comment + delete branch on merge

Lint / check-sync (push) Successful in 4s

Details

Lint / check-sync (pull_request) Successful in 4s

Details

84a558d037

Two polish items from watching the first live run:

- The deploy barrier now leaves a COMMENT on the PR -- a confirm on verify
  ("deploy verified, site is live") and a failure note (alongside the email) on
  a failed CI/smoke. Gives a visible record on the PR itself, not just the logs.

- automerge_do_merge now passes delete_branch_after_merge:true. The repo's
  "Delete branch after merge by default" is only a UI-form default (it pre-checks
  the box in the merge dialog); an API merge must opt in explicitly, so the
  auto-merge was leaving head branches behind. Now they're cleaned up.

Tests: merge body asserts Do=merge + delete_branch_after_merge; barrier asserts a
confirm comment on healthy and a failure comment on CI-failure. check-sync green.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011KXPafSYJPY6XkWHHk1Lfs

igor commented

2026-06-26 01:26:52 +00:00

Author

Collaborator

🤖 Review — `COMMENT` (automated)

CI for 84a558d0: success

Small, focused two-part change to the auto-merge deploy barrier: (1) post a PR comment on deploy verify/failure, (2) send delete_branch_after_merge:true on the API merge. Code is in scope, the checklist matches the diff, and CI is green. The one thing I can't confirm from the diff blocks a clean APPROVE.

Findings

Can't verify forgejo_comment is real (the main risk). Both new feature calls — _deploy_alert (lib/automerge.sh:109) and do_deploy_barrier (lib/automerge.sh:166) — invoke forgejo_comment "$repo" "$pr" "<body>", but the function isn't defined anywhere in this diff. The tests stub it (forgejo_comment() { COMMENTS=...; COMMENT_BODY="$3"; }), so the unit suite passes regardless of whether the production helper exists or takes this exact (repo, pr, body) signature. bash -n won't catch a runtime "command not found" either. And both call sites swallow failure (2>/dev/null || true / || log warning), so if the function is missing the whole feature silently no-ops with no test catching it. The name matches the sibling forgejo_commit_status convention, so it's likely a real helper — but please confirm forgejo_comment exists in the production source tree and that its arg order is (repo, pr, body).
Merge-body change is correct and tested. automerge_do_merge now sends {"Do":"merge","delete_branch_after_merge":true} (lib/automerge.sh:79); the new test asserts both fields and that the merge SHA is still returned. The in-shell > "$TMP/merge_sha" (not $()) trick to let MERGE_BODY persist out of the _fj stub is sound.
Comment assertions line up with the bodies. Failure path body contains "did NOT verify" and success path contains "verified", matching the new has assertions.

Notes / nits (non-blocking)

The test's local _fj() override (returns {"merge_commit_sha":"deadbeef"} for any non-merge call) isn't restored before the following do_automerge_tick block; CI green suggests no interference, but worth a glance if later tests get flaky.
Inconsistent failure handling between the two call sites (alert path silently || true, confirm path logs a warning) — intentional given the alert path already emails, just noting it.

Flagging as COMMENT rather than APPROVE solely because the existence/signature of forgejo_comment can't be verified from the diff and the tests can't catch its absence. One quick repo check by the human clears it.
===BODY===

_{Independent review by the harness on claude-opus-4-8. The human reviewer is requested once Igor has reviewed; a human still merges.}

### 🤖 Review — `COMMENT` _(automated)_ CI for `84a558d0`: **success** Small, focused two-part change to the auto-merge deploy barrier: (1) post a PR comment on deploy verify/failure, (2) send `delete_branch_after_merge:true` on the API merge. Code is in scope, the checklist matches the diff, and CI is green. The one thing I can't confirm from the diff blocks a clean APPROVE. **Findings** - **Can't verify `forgejo_comment` is real (the main risk).** Both new feature calls — `_deploy_alert` (lib/automerge.sh:109) and `do_deploy_barrier` (lib/automerge.sh:166) — invoke `forgejo_comment "$repo" "$pr" "<body>"`, but the function isn't defined anywhere in this diff. The tests *stub* it (`forgejo_comment() { COMMENTS=...; COMMENT_BODY="$3"; }`), so the unit suite passes regardless of whether the production helper exists or takes this exact `(repo, pr, body)` signature. `bash -n` won't catch a runtime "command not found" either. And both call sites swallow failure (`2>/dev/null || true` / `|| log warning`), so if the function is missing the whole feature silently no-ops with no test catching it. The name matches the sibling `forgejo_commit_status` convention, so it's *likely* a real helper — but please confirm `forgejo_comment` exists in the production source tree and that its arg order is `(repo, pr, body)`. - **Merge-body change is correct and tested.** `automerge_do_merge` now sends `{"Do":"merge","delete_branch_after_merge":true}` (lib/automerge.sh:79); the new test asserts both fields and that the merge SHA is still returned. The in-shell `> "$TMP/merge_sha"` (not `$()`) trick to let `MERGE_BODY` persist out of the `_fj` stub is sound. - **Comment assertions line up with the bodies.** Failure path body contains "did NOT verify" and success path contains "verified", matching the new `has` assertions. **Notes / nits (non-blocking)** - The test's local `_fj()` override (returns `{"merge_commit_sha":"deadbeef"}` for any non-merge call) isn't restored before the following `do_automerge_tick` block; CI green suggests no interference, but worth a glance if later tests get flaky. - Inconsistent failure handling between the two call sites (alert path silently `|| true`, confirm path logs a warning) — intentional given the alert path already emails, just noting it. Flagging as COMMENT rather than APPROVE solely because the existence/signature of `forgejo_comment` can't be verified from the diff and the tests can't catch its absence. One quick repo check by the human clears it. ===BODY=== --- <sub>Independent review by the harness on `claude-opus-4-8`. The human reviewer is requested once Igor has reviewed; a human still merges.</sub>

igor added spent time

2026-06-26 01:26:52 +00:00

59 seconds

igor requested review from joshtronic

2026-06-26 01:26:52 +00:00