fix(automerge): surface merge-rejection reason + back off, don't retry forever (closes #322) #323

Merged

joshtronic merged 1 commit from fix/automerge-backoff into master

2026-07-02 17:50:31 +00:00

igor commented

2026-07-02 17:42:19 +00:00

Collaborator

Closes #322.

Root cause

do_automerge_tick retried a rejected merge every tick with a bare merge API failed. On igor.bot#290 the Forgejo API returns:

HTTP 405: {"message":"User not allowed to merge PR"}

The bot lacks merge permission on igor.bot (almost certainly the #319 branch-protection lockdown — the bot can't even read the protection rules). A 405 won't self-heal by re-POSTing, so the harness hammered it forever and the hourly logwatch pass filed #322. automerge_do_merge swallowed the error (>/dev/null 2>&1), which is why the warning was opaque.

Fix (harness robustness — exactly what #322 asked for)

Surface the reason. automerge_do_merge now returns HTTP <code>: <message> via a new _fj_merge helper that keeps the error body _fj's curl -f discards.
Back off, don't hammer. A rejected head is recorded under .automerge_block and skipped for a cooldown (AUTOMERGE_BLOCK_COOLDOWN_SECS = 1h). The log shows why once, then goes quiet. It self-heals: a config fix is picked up after the cooldown, a new commit (different head) clears the block immediately, and a successful merge clears it.

Not in scope (your call)

This fixes the harness. The actual igor.bot#290 merge still needs a decision: grant the bot merge rights on igor.bot (add it to the branch-protection merge whitelist) if you want auto-merge there, or keep it manual — in which case its agent.json .smoke.url (what makes it auto-merge-eligible) should be dropped so the harness stops trying. Meanwhile you can merge #290 yourself to unblock the post.

Tests

bin/check-sync.sh green — added coverage for the reason surfacing, the merge payload, and the block-state machine (record / same-head-skip / different-head-retry / cooldown-elapsed / clear).

🤖 Generated with Claude Code

Closes #322. ## Root cause `do_automerge_tick` retried a rejected merge **every tick** with a bare `merge API failed`. On **igor.bot#290** the Forgejo API returns: ``` HTTP 405: {"message":"User not allowed to merge PR"} ``` The bot lacks merge permission on igor.bot (almost certainly the #319 branch-protection lockdown — the bot can't even read the protection rules). A 405 won't self-heal by re-POSTing, so the harness hammered it forever and the hourly logwatch pass filed #322. `automerge_do_merge` swallowed the error (`>/dev/null 2>&1`), which is why the warning was opaque. ## Fix (harness robustness — exactly what #322 asked for) - **Surface the reason.** `automerge_do_merge` now returns `HTTP <code>: <message>` via a new `_fj_merge` helper that keeps the error body `_fj`'s `curl -f` discards. - **Back off, don't hammer.** A rejected head is recorded under `.automerge_block` and skipped for a cooldown (`AUTOMERGE_BLOCK_COOLDOWN_SECS = 1h`). The log shows *why* once, then goes quiet. It **self-heals**: a config fix is picked up after the cooldown, a new commit (different head) clears the block immediately, and a successful merge clears it. ## Not in scope (your call) This fixes the *harness*. The actual igor.bot#290 merge still needs a decision: **grant the bot merge rights** on igor.bot (add it to the branch-protection merge whitelist) if you want auto-merge there, **or** keep it manual — in which case its `agent.json` `.smoke.url` (what makes it auto-merge-eligible) should be dropped so the harness stops trying. Meanwhile you can merge #290 yourself to unblock the post. ## Tests `bin/check-sync.sh` green — added coverage for the reason surfacing, the merge payload, and the block-state machine (record / same-head-skip / different-head-retry / cooldown-elapsed / clear). 🤖 Generated with [Claude Code](https://claude.com/claude-code)

igor added 1 commit

2026-07-02 17:42:19 +00:00

fix(automerge): surface the merge-rejection reason + back off, don't retry forever

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

Details

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

Details

426cf1a5e5

Closes #322.

do_automerge_tick retried a rejected merge every tick with a bare "merge API
failed". On igor.bot#290 the API returns 405 "User not allowed to merge PR" -- a
permission issue that won't self-heal by re-POSTing -- so it hammered forever and
the hourly logwatch pass filed #322.

- automerge_do_merge now returns the REASON ("HTTP <code>: <message>") via a new
  _fj_merge helper that keeps the error body _fj's `curl -f` discards.
- A rejected head is recorded under .automerge_block and skipped for a cooldown
  (AUTOMERGE_BLOCK_COOLDOWN_SECS = 1h): the log shows WHY once, then stops. It
  self-heals -- a config fix is picked up after the cooldown, a new commit
  (different head) clears the block immediately, and a success clears it.
- test-automerge.sh: reason surfacing, the merge payload, and the block state machine.

Scope: this fixes the HARNESS behavior (opaque + infinite retry). The underlying
igor.bot#290 merge still needs the bot granted merge rights (branch-protection
whitelist) or a manual merge -- a Forgejo config decision, separate from this PR.

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

igor commented

2026-07-02 17:44:09 +00:00

Author

Collaborator

🤖 Review — `COMMENT` (automated)

CI for 426cf1a5: success

Harness fix: automerge_do_merge now surfaces the Forgejo merge-rejection reason and do_automerge_tick records/backs-off rejected heads instead of re-POSTing every tick. The logic and unit coverage look sound; my reservation is one unverifiable variable in the integration path.

Must verify (blocking-ish, can't confirm from diff):

do_automerge_tick now references $sf in three new spots (automerge_block_active "$sf" ..., automerge_block_clear "$sf" ..., automerge_block_record "$sf" ...), but the diff does not show sf being assigned anywhere in that function. If sf isn't already a defined local/global earlier in do_automerge_tick, this passes an empty first arg to the block helpers (blocks silently never activate/record), or under set -u errors out the tick entirely. The new unit tests exercise the block helpers with an explicit state file, so they would NOT catch a missing $sf in the tick. Please confirm sf is defined and points at the same state file the block state should live in.

Looks correct:

_fj_merge code/body split (${out##*$'\n'} / ${out%$'\n'*}) and .message // empty extraction are right; deliberately bypassing _fj's curl -f to keep the error body is the correct call.
automerge_do_merge: 2?? glob → sha+rc0, else HTTP <code>: <msg>+rc1. curl timeout/failure (http_code=000) falls through to the reject branch and gets backed off — reasonable.
Block state machine (record / same-head-skip / different-head-retry / cooldown-elapsed / clear) matches the tests, and the tick clears the block on success and on a fresh head — self-heal story checks out.
Scope is tight, checklist is honest (reason-surfacing, merge payload, block state machine all present in the diff), CI green.

Coverage gap: no test drives `do_automerge_tick` itself through the reject→record→skip→clear flow, which is exactly why the `$sf` question above is invisible to CI. An integration-level assertion there would close the loop.

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

### 🤖 Review — `COMMENT` _(automated)_ CI for `426cf1a5`: **success** Harness fix: `automerge_do_merge` now surfaces the Forgejo merge-rejection reason and `do_automerge_tick` records/backs-off rejected heads instead of re-POSTing every tick. The logic and unit coverage look sound; my reservation is one unverifiable variable in the integration path. **Must verify (blocking-ish, can't confirm from diff):** - `do_automerge_tick` now references `$sf` in three new spots (`automerge_block_active "$sf" ...`, `automerge_block_clear "$sf" ...`, `automerge_block_record "$sf" ...`), but the diff does not show `sf` being assigned anywhere in that function. If `sf` isn't already a defined local/global earlier in `do_automerge_tick`, this passes an empty first arg to the block helpers (blocks silently never activate/record), or under `set -u` errors out the tick entirely. The new unit tests exercise the block helpers with an *explicit* state file, so they would NOT catch a missing `$sf` in the tick. Please confirm `sf` is defined and points at the same state file the block state should live in. **Looks correct:** - `_fj_merge` code/body split (`${out##*$'\n'}` / `${out%$'\n'*}`) and `.message // empty` extraction are right; deliberately bypassing `_fj`'s `curl -f` to keep the error body is the correct call. - `automerge_do_merge`: `2??` glob → sha+rc0, else `HTTP <code>: <msg>`+rc1. curl timeout/failure (`http_code=000`) falls through to the reject branch and gets backed off — reasonable. - Block state machine (record / same-head-skip / different-head-retry / cooldown-elapsed / clear) matches the tests, and the tick clears the block on success and on a fresh head — self-heal story checks out. - Scope is tight, checklist is honest (reason-surfacing, merge payload, block state machine all present in the diff), CI green. **Coverage gap:** no test drives `do_automerge_tick` itself through the reject→record→skip→clear flow, which is exactly why the `$sf` question above is invisible to CI. An integration-level assertion there would close the loop. ==== --- <sub>Independent review by the harness on `claude-opus-4-8` (effort: high). The human reviewer is requested once Igor has reviewed; a human still merges.</sub>

igor added spent time

2026-07-02 17:44:09 +00:00

44 seconds

igor requested review from joshtronic

2026-07-02 17:44:09 +00:00