Detect failed merges and hold write protection

[ajtmccarty]

Why

When a task worker is killed mid-merge (e.g. SIGKILL), the source branch is left stuck in MERGING with a partially-merged graph and the global merge lock still held by the dead worker. Nothing detected this state, so the default-branch write-protection either lingered indefinitely or — worse — writes could resume against a half-merged default branch, and users got no signal that anything was wrong.

Goal: deterministically detect a dead merge, record it durably as MERGE_FAILED, and keep the default branch write-blocked with a distinct, non-retryable error that tells the operator recovery is required — while never mis-flagging a healthy in-progress merge.

Non-goals (follow-ups):

• The recovery itself — infrahub recover / range rollback / branch + proposed-change reset (US3, separate PR).
• The Python SDK BranchStatus enum mirror (separate submodule PR).
• Structured detection/recovery logging and richer visibility (US4).

Part of the merge-failure-recovery epic IFC-2559; this is the US2 (detection) slice of umbrella bug IFC-2437 and advances IFC-2565 (surface a crashed-merge error to the operator). It does not close IFC-2437 — recovery lands in a follow-up.

Closes IFC-2565

What changed

Behavioral changes (what you'll observe differently):

• New durable branch status MERGE_FAILED, auto-exposed in the GraphQL BranchStatus enum. It is set only by detection and cleared only by recovery (not yet in this PR).
• A recurring MERGE_WATCHER workflow (1-minute cron, single-flighted across workers) plus a best-effort check at API-server startup flip a branch that is stuck in MERGING whose merge-lock holder is no longer a live worker (after a grace period) to MERGE_FAILED, and hold the shared write-protection.
• Writes blocked by a failed merge now return the distinct MERGE_RECOVERY_REQUIRED GraphQL error code (HTTP 423) with a message naming infrahub recover and "contact an administrator" — separate from the transient MERGE_IN_PROGRESS used during a healthy merge. Payload: {branch_name, merging_branch}.
• A healthy in-progress merge (live lock holder, or younger than the grace period) is never flagged.
• New config setting INFRAHUB_MERGE_FAILURE_GRACE_PERIOD_SECONDS (default 180).

Implementation notes:

• Liveness reuses the existing merge-lock token ({timestamp}::{worker_id}) compared against component.list_workers()' active set — the same pattern as clean_up_deadlocks. The decision is a side-effect-free predicate (is_failed_merge) that is unit-tested without a DB.
• MergeFailureRecovery.scan() runs detection then reconciliation in one pass. Reconciliation re-aligns the volatile merge:protected cache key with the durable MERGING/MERGE_FAILED branch status, so protection self-heals after a restart or cache flush.
• MergeRecoveryRequiredError is deliberately a sibling of MergeInProgressError, not a subclass — the error-catalogue resolver walks the MRO, so subclassing would collapse it onto the retryable MERGE_IN_PROGRESS code.
• Lock-token parsing now lives in lock.py beside the token format (a get_worker_id_from_lock_token helper), and a multi-status filter (BranchListFilters.statuses → n.status IN …) lets reconcile query protected branches in one round-trip.

What stayed the same: no database migration; the healthy-merge happy path is unchanged; the recovery flow and SDK enum mirror are out of scope.

Suggested review order (5 commits)

1. bc0bf2ee5 — regenerate error-catalogue bindings + tests for MERGE_RECOVERY_REQUIRED (completes the already-merged error-code source).
2. 913dac109 — add the MERGE_FAILED status (+ GraphQL schema regen).
3. 68598a3ad — detection primitives (grace config, lock-token parser, merge-lock read, multi-status filter).
4. 675b6c02b — the detector, watcher cron, startup scan, and the write gate (the meat + tests).
5. 2ec412ed6 — docs + spec alignment.

How to review

• Focus: backend/infrahub/core/merge/failure_recovery.py (the predicate + scan), and backend/infrahub/branch/status_checker.py (the MERGE_FAILED gate raising MergeRecoveryRequiredError).
• Worth extra scrutiny: the failure predicate's conditions (why an absent lock is deliberately not auto-flagged), and the sibling-not-subclass choice for MergeRecoveryRequiredError.
• Mechanical / generated (skim for drift only): schema/schema.graphql, schema/error-catalogue.json, frontend/app/src/shared/api/**/generated, docs/docs/reference/error-catalogue.mdx.

Impact & rollout

• Backward compatibility: additive only — MERGE_FAILED is a new BranchStatus enum value and MERGE_RECOVERY_REQUIRED a new catalogue code (marked evolving). Clients that exhaustively switch on either enum should handle the new value. No migration.
• ⚠️ Operational caveat: recovery (infrahub recover) is not in this PR. Until the recovery PR lands, a branch that reaches MERGE_FAILED stays write-blocked with no in-product way to clear it (manual DB intervention only). Land the recovery follow-up close behind this.
• Performance: the write gate is unchanged (one cache GET per top-level mutation). Detection adds one parameterized branch query + one cache read per worker per minute.
• Config/env changes: new INFRAHUB_MERGE_FAILURE_GRACE_PERIOD_SECONDS (default 180).
• Deployment notes: safe to deploy; the MERGE_WATCHER cron deployment is created automatically once the workflow definition is present.

Checklist

• Tests added/updated
• Changelog entry — intentionally omitted; to be folded into the larger merge-failure-recovery docs update once recovery lands
• External docs updated (generated error-catalogue reference reflects MERGE_RECOVERY_REQUIRED)
• Internal .md docs updated (dev/knowledge/backend/branch-status.md, dev/specs/ifc-2437-merge-failure-recovery/*)
• I have reviewed AI generated content

---

Summary by cubic

Detects and guards against failed merges to keep the default branch consistent. Adds MERGE_FAILED and returns MERGE_RECOVERY_REQUIRED (HTTP 423), runs a 1‑min merge‑watcher plus a startup scan, and surfaces a grace‑period setting across API/docs/Docker/frontend (IFC-2437).

• New Features

  • Detect crashed merges and flip MERGING to MERGE_FAILED after a grace period if the lock holder is dead; keep shared write protection and reconcile it on each scan.
  • Write gates: raise MergeInProgressError for MERGING and MergeRecoveryRequiredError (HTTP 423) for MERGE_FAILED with payload {branch_name, merging_branch}.
  • Watcher + startup: recurring merge‑watcher workflow (1‑min, single‑flight) and a best‑effort API‑server startup scan.
  • Contracts/tooling: extend error catalogue and GraphQL schema; regenerate frontend error bindings and GraphQL types; regenerate frontend REST OpenAPI types for the new setting; add multi‑status branch filtering and a lock‑token worker‑id parser; DB fallback now filters MERGE_FAILED/MERGING server‑side to avoid misses on large installs; bump python_sdk submodule to include MERGE_FAILED in the BranchStatus enum.
  • Tests: make the SIGKILL merge‑detection integration test fast and deterministic by using a ~1s grace period in the test stack, waiting for MERGING before SIGKILL, and tightening polling.

• Migration

  • No breaking changes. Clients may see MERGE_FAILED and MERGE_RECOVERY_REQUIRED.
  • New setting merge_failure_grace_period_seconds (default 180s); surfaced in OpenAPI/config docs and exposed via Docker Compose as INFRAHUB_MERGE_FAILURE_GRACE_PERIOD_SECONDS.
  • Ensure workers run the internal MERGE_WATCHER workflow (auto‑registered).

^{Written for commit 3e3706f. Summary will update on new commits.}

[cubic-dev-ai]

3 issues found and verified against the latest diff

Confidence score: 3/5

• In schema/error-catalogue.json, the SDK BranchStatus enum missing MERGE_FAILED can leave clients unable to represent or handle a backend-reported merge state, which risks broken consumer logic when MERGE_RECOVERY_REQUIRED is returned — add MERGE_FAILED to the SDK enum before merging.
• In backend/infrahub/branch/status_checker.py, the cache-fallback path uses Branch.get_list without a status filter or explicit limit, so branches beyond the default page size may bypass merge gating and allow writes during active/failed merges — tighten the fallback query (status + pagination/limit handling) before merge.
• In backend/tests/functional/merge/test_failed_merge_protection.py, inconsistent assertions across the three error-response blocks make it easier for regressions in error contract fields to slip through unnoticed — align the assertions so each path verifies the same key fields (message, code, http_status, and data) for reliable protection.

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="backend/tests/functional/merge/test_failed_merge_protection.py">

<violation number="1" location="backend/tests/functional/merge/test_failed_merge_protection.py:85">
P3: The three error-response assertion blocks in this test are inconsistent in what they verify. The default-branch gate checks `message`, `code` (`MERGE_RECOVERY_REQUIRED`), `http_status` (`423`), and `data`; the source-branch gate skips `http_status`; and the restart/post-recovery gate only checks `code`. A partial regression in `http_status`, `data`, or the error message after recovery would go undetected. Align all three gates to verify the same set of fields so the error contract is uniformly validated, especially after the `scan_for_failed_merges` recovery path.</violation>
</file>

<file name="schema/error-catalogue.json">

<violation number="1" location="schema/error-catalogue.json:304">
P1: SDK `BranchStatus` enum is missing `MERGE_FAILED`. The backend already has this value (`backend/infrahub/core/branch/enums.py:8`), and the new `MERGE_RECOVERY_REQUIRED` error code communicates this state to consumers. But the SDK's `BranchStatus` enum at `opsmill/infrahub-sdk-python/infrahub_sdk/branch.py:5` is missing it, so any `branch.get()` or `branch.all()` call that encounters a branch with `status: "MERGE_FAILED"` will crash with a Pydantic validation error. Add `MERGE_FAILED = "MERGE_FAILED"` to the SDK's `BranchStatus` enum so SDK consumers can query and handle failed-merge branches.</violation>
</file>

<file name="backend/infrahub/branch/status_checker.py">

<violation number="1" location="backend/infrahub/branch/status_checker.py:110">
P2: Cache-fallback merge gating can allow writes during an active/failed merge when branch count exceeds the default page size. The fallback query uses `Branch.get_list` without status filter or explicit limit, so it only inspects up to 1000 branches before deciding no block is needed.</violation>
</file>

<sub>Shadow auto-approve: would not auto-approve because issues were found.

Re-trigger cubic</sub>

[cubic-dev-ai]

P1: SDK BranchStatus enum is missing MERGE_FAILED. The backend already has this value (backend/infrahub/core/branch/enums.py:8), and the new MERGE_RECOVERY_REQUIRED error code communicates this state to consumers. But the SDK's BranchStatus enum at opsmill/infrahub-sdk-python/infrahub_sdk/branch.py:5 is missing it, so any branch.get() or branch.all() call that encounters a branch with status: "MERGE_FAILED" will crash with a Pydantic validation error. Add MERGE_FAILED = "MERGE_FAILED" to the SDK's BranchStatus enum so SDK consumers can query and handle failed-merge branches.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At schema/error-catalogue.json, line 304:

<comment>SDK `BranchStatus` enum is missing `MERGE_FAILED`. The backend already has this value (`backend/infrahub/core/branch/enums.py:8`), and the new `MERGE_RECOVERY_REQUIRED` error code communicates this state to consumers. But the SDK's `BranchStatus` enum at `opsmill/infrahub-sdk-python/infrahub_sdk/branch.py:5` is missing it, so any `branch.get()` or `branch.all()` call that encounters a branch with `status: "MERGE_FAILED"` will crash with a Pydantic validation error. Add `MERGE_FAILED = "MERGE_FAILED"` to the SDK's `BranchStatus` enum so SDK consumers can query and handle failed-merge branches.</comment>

<file context>
@@ -238,6 +238,92 @@
+      }
+    },
+    "MERGE_RECOVERY_REQUIRED": {
+      "description": "The write was rejected because a previous branch merge failed and left the default branch protected. Recovery is required: an administrator must run `infrahub recover`. Unlike MERGE_IN_PROGRESS this is not retryable.",
+      "stability": "evolving",
+      "http_status": 423,
</file context>

[cubic-dev-ai]

2 issues found across 34 files

Confidence score: 4/5

• In docs/docs/reference/error-catalogue.mdx, the recovery guidance points operators to infrahub recover, which is not present here, so on-call responders could follow a non-existent command during an incident and lose time; update the catalogue to the actual supported recovery path before merging.
• In dev/specs/ifc-2437-merge-failure-recovery/contracts/internal-api.md, the predicate text still depends on merge_locker.get_merge_lock_holder_worker_id(cache) even though merge_locker is no longer injected, which leaves the contract internally inconsistent and can cause incorrect implementations; revise the spec to document the new source of lock-holder worker ID before merge.

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="schema/error-catalogue.json">

<violation number="1" location="schema/error-catalogue.json:304">
P1: SDK `BranchStatus` enum is missing `MERGE_FAILED`. The backend already has this value (`backend/infrahub/core/branch/enums.py:8`), and the new `MERGE_RECOVERY_REQUIRED` error code communicates this state to consumers. But the SDK's `BranchStatus` enum at `opsmill/infrahub-sdk-python/infrahub_sdk/branch.py:5` is missing it, so any `branch.get()` or `branch.all()` call that encounters a branch with `status: "MERGE_FAILED"` will crash with a Pydantic validation error. Add `MERGE_FAILED = "MERGE_FAILED"` to the SDK's `BranchStatus` enum so SDK consumers can query and handle failed-merge branches.</violation>
</file>

<file name="docs/docs/reference/error-catalogue.mdx">

<violation number="1" location="docs/docs/reference/error-catalogue.mdx:289">
P2: Custom agent: **Flag AI Slop and Fabricated Changes**

The error catalogue claims that operators should run `infrahub recover`, but that CLI command does not exist in this PR. The PR description explicitly states that "No recovery path included in this PR; the recovery flow lands in a follow-up." Documentation that describes behavior not yet implemented is misleading and should be deferred until the recovery command actually exists, or at minimum should not prescribe a specific command that does not exist.</violation>
</file>

<file name="dev/specs/ifc-2437-merge-failure-recovery/contracts/internal-api.md">

<violation number="1" location="dev/specs/ifc-2437-merge-failure-recovery/contracts/internal-api.md:193">
P2: The constructor no longer injects `merge_locker`, but the predicate description still references `merge_locker.get_merge_lock_holder_worker_id(cache)` to read the lock-holder worker id. The spec needs to clarify how the lock-holder token is now parsed — either the reading/parsing logic is folded into `MergeWriteBlocker` (which currently owns the `merge:protected` key, not the merge-lock token) or is handled through a standalone helper. As written, an implementer following this spec has no way to get the lock-holder worker id.</violation>
</file>

<sub>Shadow auto-approve: would not auto-approve because issues were found.

Re-trigger cubic</sub>

[cubic-dev-ai]

P2: Custom agent: Flag AI Slop and Fabricated Changes

The error catalogue claims that operators should run infrahub recover, but that CLI command does not exist in this PR. The PR description explicitly states that "No recovery path included in this PR; the recovery flow lands in a follow-up." Documentation that describes behavior not yet implemented is misleading and should be deferred until the recovery command actually exists, or at minimum should not prescribe a specific command that does not exist.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At docs/docs/reference/error-catalogue.mdx, line 289:

<comment>The error catalogue claims that operators should run `infrahub recover`, but that CLI command does not exist in this PR. The PR description explicitly states that "No recovery path included in this PR; the recovery flow lands in a follow-up." Documentation that describes behavior not yet implemented is misleading and should be deferred until the recovery command actually exists, or at minimum should not prescribe a specific command that does not exist.</comment>

<file context>
@@ -284,6 +284,41 @@ The write was rejected because a branch merge is in progress. The block is trans
 
+### MERGE_RECOVERY_REQUIRED
+
+The write was rejected because a previous branch merge failed and left the default branch protected. Recovery is required: an administrator must run `infrahub recover`. Unlike MERGE_IN_PROGRESS this is not retryable.
+
+- **Stability**: `evolving`
</file context>

[cubic-dev-ai]

P2: The constructor no longer injects merge_locker, but the predicate description still references merge_locker.get_merge_lock_holder_worker_id(cache) to read the lock-holder worker id. The spec needs to clarify how the lock-holder token is now parsed — either the reading/parsing logic is folded into MergeWriteBlocker (which currently owns the merge:protected key, not the merge-lock token) or is handled through a standalone helper. As written, an implementer following this spec has no way to get the lock-holder worker id.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At dev/specs/ifc-2437-merge-failure-recovery/contracts/internal-api.md, line 193:

<comment>The constructor no longer injects `merge_locker`, but the predicate description still references `merge_locker.get_merge_lock_holder_worker_id(cache)` to read the lock-holder worker id. The spec needs to clarify how the lock-holder token is now parsed — either the reading/parsing logic is folded into `MergeWriteBlocker` (which currently owns the `merge:protected` key, not the merge-lock token) or is handled through a standalone helper. As written, an implementer following this spec has no way to get the lock-holder worker id.</comment>

<file context>
@@ -179,42 +179,48 @@ deleted/reopened; the `updated_at >= merge_at` filter excludes already-restored
-        merge_locker: MergeLocker,     # read-side helper: current merge-lock holder worker_id (or None)
+        cache: InfrahubCache,                    # read the merge-lock holder token
+        component: InfrahubComponent,            # list_workers() -> active-worker set for the liveness check
+        merge_write_blocker: MergeWriteBlocker,  # owns the merge:protected key (set/get/delete)
+        # recovery (PR-5) additionally injects: diff_merger: DiffMerger (the range rollback query)
     ) -> None: ...
</file context>

[cubic-dev-ai]

0 issues found across 4 files (changes from recent commits).

<sub>Shadow auto-approve: would not auto-approve. Auto-approval blocked by 3 unresolved issues from previous reviews.

Re-trigger cubic</sub>

[cubic-dev-ai]

0 issues found across 1 file (changes from recent commits).

<sub>Shadow auto-approve: would not auto-approve. Auto-approval blocked by 3 unresolved issues from previous reviews.

Re-trigger cubic</sub>

[cubic-dev-ai]

0 issues found across 2 files (changes from recent commits).

<sub>Shadow auto-approve: would not auto-approve. Auto-approval blocked by 3 unresolved issues from previous reviews.

Re-trigger cubic</sub>

[codspeed-hq]

Merging this PR will not alter performance

✅ 12 untouched benchmarks

---

_{Comparing ifc-2437-merge-failure-recovery-detect-failed (3e3706f) with develop (bc9a3f6)}

[cubic-dev-ai]

1 issue found across 2 files (changes from recent commits).

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="backend/tests/integration_docker/test_merge_kill_recovery.py">

<violation number="1" location="backend/tests/integration_docker/test_merge_kill_recovery.py:60">
P2: The merge mutation fires as a background task via `asyncio.create_task`, but its potential exception is never collected. If the GraphQL call raises an error for any reason unrelated to the SIGKILL (API timeout, schema validation, transient network issue), Python will emit a "Task exception was never retrieved" warning at garbage-collection time and the root cause will be silently lost, potentially masking CI flakiness or real failures. Consider short-circuiting the exception retrieval in the `finally` block so the test makes the failure explicit rather than letting it go unobserved.</violation>
</file>

<sub>Shadow auto-approve: would not auto-approve because issues were found.
Tip: Review your code locally with the cubic CLI to iterate faster.

Re-trigger cubic</sub>

[cubic-dev-ai]

P2: The merge mutation fires as a background task via asyncio.create_task, but its potential exception is never collected. If the GraphQL call raises an error for any reason unrelated to the SIGKILL (API timeout, schema validation, transient network issue), Python will emit a "Task exception was never retrieved" warning at garbage-collection time and the root cause will be silently lost, potentially masking CI flakiness or real failures. Consider short-circuiting the exception retrieval in the finally block so the test makes the failure explicit rather than letting it go unobserved.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At backend/tests/integration_docker/test_merge_kill_recovery.py, line 60:

<comment>The merge mutation fires as a background task via `asyncio.create_task`, but its potential exception is never collected. If the GraphQL call raises an error for any reason unrelated to the SIGKILL (API timeout, schema validation, transient network issue), Python will emit a "Task exception was never retrieved" warning at garbage-collection time and the root cause will be silently lost, potentially masking CI flakiness or real failures. Consider short-circuiting the exception retrieval in the `finally` block so the test makes the failure explicit rather than letting it go unobserved.</comment>

<file context>
@@ -37,32 +49,38 @@ async def test_killed_merge_is_flagged_failed_while_idle(
+
+        # Fire the merge without awaiting completion (it runs on a task worker); we kill the worker
+        # while it is in flight, so awaiting here would hang.
+        merge_task = asyncio.create_task(
+            client.execute_graphql(
+                query=f'mutation {{ BranchMerge(data: {{name: "{branch_name}"}}) {{ ok }} }}',
</file context>

[cubic-dev-ai]

0 issues found across 1 file (changes from recent commits).

<sub>Shadow auto-approve: would not auto-approve. Auto-approval blocked by 4 unresolved issues from previous reviews.

Re-trigger cubic</sub>