Approved, green PRs are orphaned when their worker exits before auto-merge can succeed: nothing retries the merge

[ryanrborn]

Summary

This is round 2 for the orphaned approved-merge fix. The first version ([bd-a370ak]) shipped in v0.1.72 and failed post-merge verification on emricare/tonic !292 and !293. The sweeper did detect both orphaned merges. The worker-less retry then gave up on the first red pipeline: it paged "approved merge abandoned (orphaned PR: ci_failed)" and latched the stamp as escalated. Nothing re-armed it, so when a later pipeline went green, !292 sat green and mergeable until a human resumed the worker.

There were two defects, both confirmed from the live DB (worker_runs and messages):

1. Red CI was treated as a verdict, when it is only a wait. detached_outcome/3 called give_up_retry(state, :ci_failed). Now detached_ci_red/1 waits on red CI the same way it waits on a draft or on pending CI, and it merges once a re-run or a new pipeline goes green.
   • The wait stays bounded by max_retry_wait_ms (48h), measured from the stamp's since. After that it pages once, as wait_exhausted, and latches.
   • The coordinator gets one merge_blocked(:ci_failed) notice per pending merge, not an "abandoned" page. The notice is latched on the stamp through the new PendingMerge.note_block/2 (notified_block), so a server restart or a re-armed retry doesn't repeat it. A fresh stamp from a live Watchdog starts a new episode.
2. The retry was started while a fix pass was still running. In both incidents the sweeper started the retry while <task>:fixpass was still working the red pipeline: acqboi's fix pass ended 13s after the give-up, and 2oyzcn's ran for another ~3 min. live_merge_owner/1 only checked the task's own registry key. It now returns {:subordinate, key} for any active :fixpass or :conflict pass, found through the new Worker.active_subordinate/1, which reuses the single-active-worker guard's probe. When that happens the sweeper skips the task and a running retry stands down. Once the pass exits, a later sweep re-arms the retry.

The merge guards on the retry path are unchanged. A head pushed after the approval (for example by a fix pass) still has to pass the stale-reviewed-SHA and coverage decision, and the zero-net-diff guard, before anything merges.

On the GitLab awaiting_approval question: that status comes from the MergeQueue's own item after it adopts the MR. advance_ready_ladder/3 reads only the forge's approved flag, which an in-process ReviewGate approval never sets, so on a ReviewGate lane the item idles at awaiting_approval by design, and the Watchdog or the retry does the merge. GitLab approval is not required on tonic: !292 was merged by the Watchdog at 03:37Z without a GitLab approval. If a project did require one, the retry would see block_reason: :needs_approval or :needs_nonauthor_approval and page once. It would not merge.

W21 in the guard registry and in docs/review-coverage-and-guard-policy.md now names the new sites. I re-anchored the doc's watchdog.ex citations with an exact line remap.

Post-deploy verification (bd-a370ak, verify_after_deploy)

After the restart, what to look for:

• An approved PR whose worker has exited and whose CI is red gets exactly one mailbox item, <task> merge blocked — required CI checks are failing. It should not get approved merge abandoned (orphaned PR: ci_failed).
• While it waits, the task's issues.pending_merge JSON has "notified_block": "ci_failed", "reason": "ci_failed", and "escalated_at": null. The retry is still running, so Watchdog.retry_whereis(task_id) returns a pid.
• When a pipeline on the reviewed head goes green, the server log shows merge_retry auto-merged orphaned approved MR … (pinned to <reviewed sha>) within one retry interval (2 min). The task then closes or parks at awaiting-verification.
• While a <task>:fixpass or <task>:conflict worker is running, the sweeper does not log started a worker-less retry for that task.

A working result means an approved PR that goes red and later green, for example after an infra fix on main plus a pipeline re-run, merges by itself without anyone resuming the worker.

Test plan

• New tests in apps/arbiter/test/arbiter/workflows/pending_merge_sweeper_test.exs: I wrote them first, and all 6 failed against the old code:
  • red → re-run green: keeps watching, sends one notice, merges pinned to the reviewed SHA, no "abandoned" page
  • the red-CI notice is sent once across a cleared notice, a stopped retry, and a fresh sweep (a simulated restart)
  • red, then an unreviewed new head goes green: the stale-SHA guard still refuses and it escalates as stale_reviewed_sha
  • red that never recovers: gives up once after max_wait_ms, as wait_exhausted
  • a live :fixpass blocks the sweeper (:live_worker); a :conflict pass that starts mid-retry makes the retry stand down without merging
• All 31 sweeper tests pass, repeated 3×
• Existing neighbours pass: all watchdog*_test.exs, worker_test.exs, merge_queue_test.exs, concurrent_task_workers_test.exs, merged_pr_finalizer*_test.exs, review_coverage_design_test.exs, guard_registry_test.exs, circuit_breaker_adoption_test.exs
• mix precommit exits 0: 7398 + 1711 + 887 tests, 0 failures. I ran it with the worker's ARB_* env vars unset, which the known ProvisioningTest environment failures need.
• mix format --check-formatted is clean, and mix credo --strict on the changed files is clean
• mix dialyzer: its only warning is arbiter_cli/cmd/doctor/checks.ex:177. This branch doesn't touch that file, and .dialyzer_ignore.exs documents the warning as depending on how the repo was cloned.
• Scratch run of the real sweeper → retry path with StubMerger: while red, the stamp had notified_block: "ci_failed", escalated_at: nil, and the retry was alive. After green, the task was :closed with merge {"!292", <reviewed sha>} and exactly one "merge blocked — required CI checks are failing" message.
• Not verified: a live GitLab forge. The post-deploy verification above covers it.

References

• Task: bd-a370ak (second attempt, after the v0.1.72 post-merge verification failed on emricare/tonic !292 / !293, tasks emr-acqboi and emr-2oyzcn)
• Original incidents: Provider accounts P4: destructive step — remove moved keys from worker_env; delete ConfigDir's old fallbacks #1947, agy parity T7 — agy workers can't resume, be nudged, or auto-resume: implement splice_prompt/2 #1966, worker_list reports zero workers while a worker is actively running, leading the coordinator to stop live work #1932

Closes #2002

🤖 Generated with Claude Code