fix(forgejo): auto-merge attempt_merge signals via return code, not stdout #45

Merged

navigator merged 1 commit from fix/auto-merge-result-return into main

2026-05-25 14:33:25 -03:00

navigator commented

2026-05-25 14:31:56 -03:00

Owner

Summary

Follow-up to #44. The previous refactor captured attempt_merge's stdout into RESULT and branched on the string. On the success path (HTTP 200), the function echoed "[auto-merge] PR #N merged via squash" to stdout, which made RESULT non-empty and tripped the "Unexpected merge result" branch — so the script exited 5 after a clean merge, even though the merge had actually landed.

This was harmless when the API succeeded (PR really did merge, the bogus exit just looked like failure) but it confuses pr-approver, which would see exit 5 and kick the loop again — exactly the spam path the rebase work was meant to close.

Change

attempt_merge keeps all log lines on stderr and signals state purely via return code: 0 = merged, MERGE_BEHIND_BASE (100) = needs rebase, hard exit 5 otherwise.
Caller uses set +e; attempt_merge ...; RESULT=$?; set -e so a non-zero return doesn't trip set -e, then branches on the integer.

Test plan

bash -n infra/forgejo/auto-merge.sh
Verify on the next "behind base" PR that the script correctly returns 0 on merge and triggers rebase + retry on 405

Notes

infra/forgejo/** off-limits per ADR-017 — merged manually.

## Summary Follow-up to #44. The previous refactor captured `attempt_merge`'s stdout into `RESULT` and branched on the string. On the success path (HTTP 200), the function echoed `"[auto-merge] PR #N merged via squash"` to stdout, which made `RESULT` non-empty and tripped the "Unexpected merge result" branch — so the script exited 5 after a clean merge, even though the merge had actually landed. This was harmless when the API succeeded (PR really did merge, the bogus exit just looked like failure) but it confuses `pr-approver`, which would see exit 5 and kick the loop again — exactly the spam path the rebase work was meant to close. ## Change - `attempt_merge` keeps all log lines on stderr and signals state purely via return code: `0` = merged, `MERGE_BEHIND_BASE` (100) = needs rebase, hard `exit 5` otherwise. - Caller uses `set +e; attempt_merge ...; RESULT=$?; set -e` so a non-zero return doesn't trip `set -e`, then branches on the integer. ## Test plan - [x] `bash -n infra/forgejo/auto-merge.sh` - [ ] Verify on the next "behind base" PR that the script correctly returns 0 on merge and triggers rebase + retry on 405 ## Notes `infra/forgejo/**` off-limits per ADR-017 — merged manually.

navigator added 1 commit

2026-05-25 14:31:56 -03:00

fix(forgejo): auto-merge attempt_merge signals via return code, not stdout

build / scalafmt-check (pull_request) Successful in 4s

Details

build / sbt-compile (pull_request) Successful in 3s

Details

build / shell-lint (pull_request) Successful in 1m4s

Details

28e7b77076

The previous refactor captured the function's stdout into RESULT and
keyed control flow off the captured string. attempt_merge echoed
"PR #N merged via squash" to stdout on the success path, which made
RESULT non-empty and tripped the "Unexpected merge result" branch
even though the merge actually succeeded — exiting 5 after a clean
merge and confusing the pr-approver loop.

Now attempt_merge keeps all human-readable lines on stderr and signals
state purely via return code: 0 for merged, MERGE_BEHIND_BASE (100)
for the rebase-needed case, exit 5 for any other failure. The caller
uses `set +e; attempt_merge ...; RESULT=$?; set -e` so a non-zero
return doesn't trip `set -e`, then branches on the integer code.