Fabrik Issue State Machine

Every issue in Fabrik follows a defined lifecycle: from intake through a series of AI-driven stages (Specify → Research → Plan → Implement → Review → Validate → Done), with automated gates at key transitions. The diagram below shows the happy path at a glance.

Not an engineer? The diagram and the Pipeline Overview table are the fastest way to understand Fabrik’s workflow.

Engine contributor or debugger? The dense reference below covers every reachable state, every label mutation, every guard condition, and the visual state diagrams in §10. Use the State Enumeration section as the authoritative source when diagnosing unexpected engine behavior.

This document is the formal specification of Fabrik’s issue-level state machine: how an issue moves between states across multiple invocations of the engine. It covers every reachable state, every event that triggers a transition, every label mutation, and every guard condition.

Companion document: stage-lifecycle.md describes the per-invocation lifecycle (what happens before, during, and after a single Claude invocation). This document describes the cross-invocation state machine (how an issue progresses through the pipeline over time). They are complementary.

As-built specification: This document describes what the code actually does, not what it ideally should do. Discrepancies between intended and actual behavior are flagged with > **Bug?:** callout blocks.

Source of truth for: state enumeration, transition tables, label semantics, and guard conditions. Supersedes partial label references in CLAUDE.md.

Pipeline Overview

Issues traverse a linear pipeline of stages, each corresponding to a column on the GitHub Project board:

Specify → Research → Plan → Implement → Review → Validate → [Queued†] → Done

* All flags in this table reflect the default stage configuration shipped in .fabrik/stages/. Each flag is opt-in per stage YAML and may differ in custom configurations. wait_for_reviews is enabled for Review and Validate in the defaults.

† Queued is a holding stage (holding_stage: true) active only when merge_train: on. Items enter Queued when attemptMergeOnValidate routes to advanceToQueued instead of GitHub auto-merge. Per-item dispatch is suppressed (itemMayNeedWork / itemNeedsWork return false for holding stages); the batch handler handleMergeTrainBatch snapshots the Queued items once per poll cycle (via groupQueuedByRepo, which excludes closed and fabrik:paused members — see the poison-well guard under Merge-Train Red-Batch Bisection) and dispatches runMergeTrainWorker when a batch is ready. On a green trial-branch CI result, landMergeTrainBatch opens a landing integration PR, merges it, advances each member from Queued → Done via advanceToNextStage, and closes their individual PRs and issues (ADR-059 D3). The Queued column must exist on the GitHub Project board when merge_train: on; it is validated at startup.

1. State Enumeration

A state is defined by the tuple (BoardColumn, ControllingLabelSet). Not every label combination is a valid state — only reachable combinations are enumerated.

1.1 Controlling Labels

These labels define distinct states (their presence changes what the engine does with an item):

1.2 Modifier Labels (Guard Conditions)

These labels do not define distinct states but influence transition behavior:

1.3 Reachable States by Board Column

For each board column, the reachable sub-states are listed. States are written as Column + {labels}. An issue in a column with no controlling labels is in the Idle sub-state for that column.

Specify / Research / Plan / Implement / Review / Validate (Active Stages)

Each active stage column has the same set of reachable sub-states:

Note: The Cooldown sub-state is purely in-memory — there is no label for it. The engine uses LastAttemptAt[stageName] from itemstate.StageState (written by StageAttempted mutation) to enforce dispatch cooldown. On restart, cooldown state is lost and the item is retried immediately.

Distinct from Cooldown is Deferred Dispatch: an item whose dispatch was skipped in the current poll cycle solely because a worker from a prior cycle is still running. Deferred-Dispatch items still receive the CooldownAt("periodic-re-eval") stamp at end-of-poll — the cooldown avoids repeated deep-fetch evaluation (and the fallback GraphQL fetch when the cache is invalidated or disabled) for an item the dispatch guard (snap.Worker() != nil) would block anyway. Prompt re-dispatch after the prior worker exits is guaranteed by WorkerExited → WorkerLifecycleChanged, which is in wakeChFlags and wakes the poll loop immediately (#544). Note: WorkerLifecycleChanged is excluded from cycleSetFlags (Fix B, issue #576) so it does not bypass the cooldown gate via mayNeedWork. For successful workers, StatusChanged from advanceToNextStage adds the item to cycleSet (bypassing the cooldown) so the next stage dispatches promptly. For workers that ended without completing — whether preempted at the turn cap or genuinely errored — the cooldown expires naturally and the next ticker poll re-evaluates the item. See §3.2, §9.2, and §9.9.

Not listed above: fabrik:awaiting-placement is a related but distinct marker applied to a spawned child issue whose initial board placement failed — by construction it is sitting outside any of these active-stage columns (typically Backlog, which is declared unmanaged: true — see below — and therefore never dispatched to regardless), so it never appears as a sub-state of an active-stage column. See §6.9.

Backlog (Unmanaged Stage)

itemMayNeedWork and itemNeedsWork both return false for any stage with unmanaged: true, so these items are never dispatched to a per-item worker and never auto-advanced by the catch-up loop (same mechanism as holding_stage, see below). Unlike a holding stage, no batch handler ever acts on an unmanaged stage’s items either — it is a pure no-op declaration. stages/examples/backlog.yaml (name: Backlog, unmanaged: true) is the shipped default; the hardcoded "Backlog" recognition in checkStageColumnAlignment (engine/startup.go) remains as a compat net for installs that predate this file — see USER_GUIDE.md §Startup Board Validation.

unmanaged: true is not rejected at load time when combined with prompt/skill/cleanup_worktree/holding_stage (mirroring the codebase’s existing lack of mutual-exclusion validation between cleanup_worktree and holding_stage themselves), but the resulting configuration is safe, not just permissively invalid: every order-sensitive resolver that could otherwise pick an Unmanaged stage as “the” target for something, or hand it bookkeeping it never earned, skips it. cleanupStage/holdingStage (engine/stages.go) never return a stage with Unmanaged: true, even if it has the lowest Order or is HoldingStage: true — falling back to nil (same as “no such stage configured”) if no eligible non-unmanaged stage remains. NextStage (stages/stages.go) walks past any Unmanaged stage when computing the next stage in the pipeline, so an operator who accidentally gives a parking column (e.g. On Hold) an Order between two dispatched stages does not strand items there — advancement continues to the next real stage instead. The gate-checked completion-label fill loop in runValidatePRTerminalAdvance (engine/pr_terminal_advance.go) stops at the first CleanupWorktree stage that is not also Unmanaged, so a misconfigured unmanaged + cleanup_worktree stage at a low Order cannot short-circuit the fill before any real gate-checked stage is considered. And settleNoWorkNeeded’s skip-labeling loop (engine/no_work_needed_settle.go) both excludes Unmanaged stages from its doneOrder boundary computation and never adds a skip label or “skipped” comment for an Unmanaged stage it steps over — a parking column an operator placed mid-pipeline gets none of the bookkeeping a real intermediate stage would.

Done (Cleanup Stage)

Queued (Holding Stage — merge_train: on only)

This stage is a no-op for per-item dispatch. It is the universal landing rendezvous: a yolo (non-cruise) Validate completion under merge_train: on advances here via the dedicated attemptMergeOnValidate → advanceToQueued enqueue, and the Queued handler then picks the landing engine per repo (ADR-059 D6 — see Engine Selection below).

Reachable only via dedicated code (issue #1072). stages.NextStage skips any HoldingStage exactly as it already skips Unmanaged — a holding stage is never returned as “the next stage” by generic positional advancement. The only call site that intentionally lands an item here is advanceToQueued, gated on yoloActive && !cruise && merge_train: on (engine/stages.go). Every other terminal-advance path — runValidatePRTerminalAdvance (cruise, human-merge, or any other gate-label convergence) and advanceConvergedPRToDone (non-train yolo convergence) — resolves NextStage from Validate and now lands directly on Done, never on Queued, regardless of merge_train state. Before this fix, NextStage’s purely positional walk landed every such path in Queued whenever the column was configured, stranding items with no active drain (ADR-1072).

itemMayNeedWork and itemNeedsWork both return false for any stage with holding_stage: true, so these items are never dispatched to a per-item worker. processItem also short-circuits on stage.HoldingStage as a defense-in-depth guard. The catch-up loop skips holding stages for the same reason. A closed item that nonetheless ends up stranded in Queued (e.g. a human gh pr merge outside the train) is rescued by the settle-scan backstop described in §6.11.

The same holding-stage exclusion that keeps dispatch off Queued items also blacks out PR feedback processing while they sit there — review-reinvoke and comment processing never run on a Queued item either, since both live behind the identical HoldingStage admission gate. §6.16 describes the dedicated settle scan (settleQueuedReviewFindings, ADR-1208) that detects an unresolved review-thread finding arriving on a Queued member’s linked PR and ejects it back onto a stage the ordinary reinvoke path can reach, rather than widening this section’s dispatch exclusion itself.

Engine Selection: 058 Native Queue vs 059 Internal Train (ADR-059 D6)

Queued is one board column served by two landing engines. The single convergence owner handleMergeTrainBatch (ADR-056 — there is no parallel scanner) picks the engine per repo each poll, because isMergeQueueEnabled is a per-PR/per-repo signal and the Queued column may hold items from several repos at once.

Selection algorithm. handleMergeTrainBatch groups the current Queued snapshot by owner/repo (groupQueuedByRepo, preserving first-seen repo order and per-repo entry order), then routeQueuedGroup routes each group by the poll-native LinkedPRIsMergeQueueEnabled signal (populated by the GraphQL board query — never a REST pulls fetch, which always reports false for the queue flag, and never a webhook payload):

• Queue-enabled repo (MergeQueue != "off" && LinkedPRIsMergeQueueEnabled) → the ADR-058 enqueue path, invoked per item via enqueueForQueue (the same helper attemptMergeOnValidate uses on the merge_train: off path). It calls EnqueuePullRequest with the poll-native LinkedPRHeadSHA as the expected-OID guard and applies fabrik:auto-merge-enabled (idempotency + convergence anchor). checkAutoMergeConvergence then drains each enqueued item Queued → Done (it claims any fabrik:auto-merge-enabled item regardless of its column). Two guards: an item already carrying fabrik:auto-merge-enabled is mid-convergence and is not re-enqueued; an item whose LinkedPRNumber/LinkedPRHeadSHA is empty (cache miss) is skipped this poll and retried next (no REST fetch).
• Non-queue repo → the ADR-059 internal merge train: the group’s remaining items form one per-repo batch dispatched to a single dispatchMergeTrainWorker (already keyed owner/repo via mergeTrainInFlight), which lands members Queued → Done via landMergeTrainBatch.

Batch-snapshot logging is deduped by composition (#1151). routeQueuedGroup runs once per poll cycle for as long as any Queued items exist for a repo — including the full CI-wait duration of an in-flight train, which can span many poll cycles. Before dispatching, it computes an order-independent signature of the internal-train subset (mergeTrainBatchSignature: sorted item numbers, comma-joined) and compares it against Engine.mergeTrainBatchSnapshotSeen (a sync.Map keyed owner/repo). The batch snapshot for <repo>: N item(s) — ... line is only logged, and the stored signature updated, when the signature differs from the last-logged one for that repo — a membership or count change re-logs; a pure reordering of the same members (GraphQL item order is not guaranteed stable poll-to-poll) does not. Dispatch itself (dispatchMergeTrainWorker) always runs regardless of whether the log fired.

Both engines drain the same Queued column and advance their members to Done on land; only who batches differs (GitHub’s queue vs Fabrik’s trial branch). max_batch_size caps the internal-train subset per repo group, not the flat cross-repo batch — so a large repo A never starves repo B, and repo B’s items can never be shoved into repo A’s trial branch (the pre-D6 batch[0]-anchored latent multi-repo bug, now hardened by grouping).

Precedence (FR-3). For any item reaching a landing decision:

1. Native merge queue present on the repo (MergeQueue != "off" && LinkedPRIsMergeQueueEnabled) → 058 enqueue path, regardless of merge_train. The queue always wins: a direct or trial-branch merge on a queue-required branch returns HTTP 405. Under merge_train: off this fires inline in attemptMergeOnValidate; under merge_train: on it fires from the Queued handler.
2. Else merge_train: on → the internal train (from the Queued handler).
3. Else → legacy per-PR serial auto-merge (attemptMergeOnValidate’s native-auto-merge / direct-merge fallback), unchanged from pre-merge-train behavior.

Merge-Train Landing Lifecycle (ADR-059 D3)

When handleMergeTrainBatch has a non-empty batch of Queued items, it calls dispatchMergeTrainWorker, which starts a goroutine running runMergeTrainWorker. The goroutine owns the full assembly-to-landing sequence:

Two-PR model: The goroutine creates two PRs: (1) a draft CI PR (chore(merge-train): trial integration for #N…) that triggers checks on the trial branch immediately, and (2) a landing integration PR ([merge-train] batch: #N1, #N2, …) opened only after trial CI passes. The draft CI PR is closed implicitly when the trial branch is deleted at cleanup.

Trial assembly — merge conflicts (issue #1235): assembleTrialBranch merges each batch member’s head SHA into the trial worktree in sequence (git merge --no-ff --no-edit). A clean merge just continues to the next member. On conflict, it calls resolveTrainConflict, which first lists the conflicted paths (unmergedPaths, parsing git status --porcelain for the UU/AA/DD/AU/UD/UA/DU codes, returning each path paired with its two-letter status code as a conflictedPath) and classifies them against a single declared generated-path → regeneration-command mapping (generatedFiles in engine/generated_files.go; today’s only entry is docs/llms-full.txt → bash scripts/generate-llms-full.sh). If unmergedPaths itself errors (e.g. a transient git status failure), resolveTrainConflict cannot classify anything and falls back to dispatching Claude for the full, unscoped conflict exactly as it would pre-#1235 — a deliberate, narrow tradeoff: a git-level failure says nothing about whether the conflict is confined to a generated path, so failing open to the safe pre-existing behavior is preferable to guessing (TestResolveTrainConflict_UnmergedPathsErrorFallsBackToPlainClaude). Otherwise, three outcomes:

• No generated paths involved: unchanged pre-#1235 behavior — resolveConflictWithClaude dispatches Claude with a synthetic comment instructing it to resolve every conflict marker and commit.
• Confined entirely to declared generated path(s): Claude is never invoked. regenerateAndCommit runs each distinct regeneration command once (deduplicated by command, not by path, so two paths sharing one command don’t re-run it), bounded by a fixed regenerationCommandTimeout (5 minutes) via exec.CommandContext — unlike Claude dispatch, which is ctx-aware throughout, a hung regeneration command has no other circuit breaker, and a timed-out command fails the member closed (ejected with a diagnosable reason) rather than blocking the worker indefinitely. Staging follows the same scope as execution, not just the conflicted subset: for every command it runs, it stages every declared path (from the full generatedFiles mapping, not only the conflicted ones passed in) tied to that command, since running a shared command once regenerates all of its declared outputs as a side effect. A non-conflicted sibling path left unstaged here would surface as a dirty working-tree change in the next member’s merge. It then verifies the tree is fully resolved before committing.
• Mixed (a declared generated path conflicts alongside a normal path in the same member): Claude is dispatched for the non-generated part only — the synthetic comment names the generated path(s) as out of scope and instructs Claude to stage only what it resolved and not commit. regenerateAndCommit always runs after Claude returns, never before: a co-conflicted non-generated file can itself be one of the generator’s own source inputs (e.g. docs/state-machine.md, one of generate-llms-full.sh’s four ORDERED pages), so regenerating first would read stale/conflicted content. regenerateAndCommit performs the single commit that finalizes both parts once its own tree-wide git diff --check and unmerged-path checks pass.

classifyConflictedPaths treats a declared generated path as eligible for regeneration only when its conflict status carries no deletion intent. A status of DD (both sides deleted), UD (deleted by them), or DU (deleted by us) — deletionInvolvingStatus — routes the path into the non-generated bucket instead, so it reaches Claude like any other conflict rather than being silently regenerated: a contributor who deleted a declared generated file meant to remove it, and blindly rerunning the regeneration command would recreate content nobody wants (TestMergeTrainWorker_DeletionConflictOnGeneratedPathRoutesToClaude). AA/UU/AU/UA carry no deletion intent and remain eligible for regeneration as before. classifyConflictedPaths also returns this deletion-excluded subset separately (deletionExcluded, alongside matched and nonGenerated) precisely so the mixed-case call into regenerateAndCommit can pass it through as protectedPaths.

protectedPaths closes a further interaction between the two mechanisms above: if a matched path and a deletion-excluded sibling declare the same regeneration command, executing that command for the matched path also regenerates the sibling on disk as a side effect — even though the sibling isn’t in specs and Claude (not regeneration) owns its resolution. regenerateAndCommit never stages such a sibling; instead it discards the command’s side effect on that one path — restoring the working-tree file from the index via git checkout-index -f when Claude staged content for it, or removing the file entirely when Claude staged its deletion (git rm) — before staging and committing the rest as usual (TestMergeTrainWorker_SharedCommandDoesNotOverwriteDeletionExcludedSibling).

A regeneration failure (non-zero exit, inability to stage, conflict markers still present afterward, or — in the mixed case — Claude having committed prematurely despite being instructed not to) ejects the member via the existing ejectMember path with a diagnosable reason describing the specific failure — it never falls back to Claude. The premature-commit case is detected structurally, not by content: regenerateAndCommit requires MERGE_HEAD to still be present at entry (it is only ever called with the merge either still fully in conflict, or with Claude having resolved-and-staged but not committed the non-generated part) and fails closed immediately if it’s already gone, before running any regeneration command. This is deliberately not a git diff --cached content comparison after the fact — that would miss the case where Claude’s premature commit happens to write byte-identical content to what regeneration would have produced. This mirrors the existing unresolvable-conflict eject path (assembleTrialBranch’s abort-and-eject branch) and reuses the same three-way (resolved, reason, err) contract style as the pre-existing usage-limit sentinel handling described under ADR-1120 below: a non-nil err (the ADR-1120 usage-limit sentinel) is still a fatal assembly abort, never an ejection, in every one of these three outcomes.

resolveConflictWithClaude also guards against a false “resolved” verdict: buildTrainConflictComment’s fallback instructions tell Claude to run git merge --abort when it judges a conflict genuinely unresolvable, and an abort clears every conflict marker (and MERGE_HEAD) exactly as a real resolution would — unmergedPaths alone can’t tell the two apart. assembleTrialBranch captures the trial worktree’s HEAD SHA immediately before attempting each member’s merge (preMergeHEAD) and threads it through resolveTrainConflict into resolveConflictWithClaude; after confirming no non-generated conflict markers remain, it additionally requires either MERGE_HEAD still present (merge legitimately in progress) or HEAD having moved past preMergeHEAD (a real commit happened). A MERGE_HEAD-less worktree still sitting on preMergeHEAD is treated as unresolved — an abort, not a resolution — so the member is ejected instead of silently landing with its whole contribution missing. This applies to both the plain and mixed conflict paths.

assembleTrialBranch’s own eject-on-unresolvable cleanup reuses that same captured preMergeHEAD: since resolution can now fail after a commit already landed on wtDir (e.g. regenerateAndCommit’s premature-commit guard tripping), git merge --abort alone is not sufficient cleanup — with MERGE_HEAD already gone, it is a silent no-op, and the bad commit would remain as wtDir’s HEAD, contaminating every subsequent member’s merge attempt and the pushed trial branch. The eject path therefore unconditionally follows the best-effort abort with git reset --hard <preMergeHEAD>, which is a no-op when the abort already fully reverted things and the authoritative cleanup when it didn’t. git reset --hard only rewinds tracked content, so the eject path also unconditionally runs git clean -fd afterward to remove any untracked files a failed resolution attempt (Claude, or a regeneration command) may have left behind — without it, a stray untracked file would make the next member’s git merge fail with git’s own “untracked working tree file would be overwritten by merge” error, which resolveTrainConflict would misclassify as a plain conflict (no MERGE_HEAD, no unmerged paths) and dispatch Claude against a worktree with nothing to resolve.

Landing sequence (FR-1 through FR-5):

1. FR-1 — Open integration PR: landMergeTrainBatch calls CreatePR (non-draft) with title [merge-train] batch: #N1, #N2, … and a body containing the idempotency marker <!-- fabrik-merge-train-batch -->. The body lists all batch members (issue number + title) and a Closes #N line per member. The member PRs are closed-not-merged (the change lands via this integration PR), so a member PR’s own Closes #N never fires — the integration PR’s Closes #N is what restores issue↔landing-PR connectivity and auto-closes each member issue when the integration PR merges into the default branch. (For a non-default base, or on auto-close lag, FR-3’s explicit CloseIssue is the fallback.) The same Closes #N lines are carried on the trial draft CI PR body from assembly, since that draft PR IS the landing PR when reused (FR-5).

2. FR-2 — Poll and merge: pollForMergeable polls the integration PR with the same 30s interval as pollTrainCI, using FetchPRDetails (mergeable_state and head SHA in one call) and classifyLandingCI (ADR-1441, #1441) — the merge-train landing counterpart to pollTrainCI’s own check-run-aware classification, built from the same shared primitives (gh.ClassifyCheckRuns, e.classifyRequiredContexts, describeCheckRuns, gh.MergeableStateAccepted). mergeable_state == "dirty" rejects immediately with no check-run fetch; otherwise check runs on the head SHA are fetched and classified: a confirmed failure or required-context failure rejects; a check still pending keeps polling; an all-clear (plus required contexts satisfied) lands. Only with zero check runs at all does an accepted mergeable_state (clean/unstable, gh.MergeableStateAccepted) become load-bearing by itself — mirroring pollTrainCI’s own zero-check-runs fallback. Before ADR-1441, this step accepted mergeable_state ∈ {"clean", "unstable"} outright with no check-run awareness at all — the merge-train landing counterpart to the single-PR advance gate’s pre-ADR-1441 shortcut, left unfixed by ADR-1153 and explicitly flagged there as a “candidate fast-follow.” Once classifyLandingCI judges the PR landable, MergePR is called (no admin bypass). MergePR itself independently re-checks mergeable_state against the clean/unstable allowlist before merging (see “MergePR’s own CI precondition (ADR-933)” in §5, after §5.3) — normally a no-op here since pollForMergeable already judged the PR acceptable, but it closes a narrow TOCTOU window if the state flips between the two checks; this merge-side check is deliberately unchanged by ADR-1441 (its R3 decision: the merge path continues to defer to branch protection alone, per ADR-072’s operator note). On timeout, a warning comment is posted on the first batch member and the function returns without advancing members (they remain in Queued for the next train cycle). On merge API failure — including gh.ErrNotMergeableCI from that TOCTOU window — an error comment is posted and landing aborts, leaving members in Queued to retry on the next train cycle; this is not escalated to a pause and does not touch fabrik:rebase-needed.

3. FR-3 — Member lifecycle: For each surviving batch member: (a) advanceToNextStage moves the item from Queued → Done on the project board; (b) addLandedCommentWithRetry posts 🏭 **Fabrik merge-train** — Landed via batch PR #<N>. on the member PR — this is the sole cross-landing-path, member-scoped record of which PR actually landed the change (issue #1275), so a bare AddComment failure would silently and permanently lose it; the shared helper retries up to 3 attempts total with exponential backoff (landedCommentRetryDelay, base 200ms, doubling), gated on isTransientError (a non-transient error like a 404/422 short-circuits to the warn immediately, without wasting the backoff window) — on exhaustion it falls back to the pre-existing warn-and-continue behavior unchanged, never blocking or delaying landing; the identical helper is used by landSingleton’s equivalent comment post (🏭 **Fabrik merge-train** — Landed one-at-a-time via singleton PR #<N>.); (c) CloseIssue closes the member PR explicitly; (d) CloseIssue closes the member issue explicitly. Step (d) is the fallback that guarantees issue closure independent of the integration PR’s Closes #N (which only auto-closes on merge into the default branch) — without it a member landed via a non-default base, or before auto-close propagates, is left landed-but-open. It is idempotent (a no-op on an already-closed issue). Members whose Status == "Done" are silently skipped (restart safety). The same close-issue fallback runs in the one-at-a-time singleton landing path (landSingleton) — but only in landSingleton is a failure of step (d) retried: on failure, markMergeTrainMemberCloseOutstanding writes fabrik:awaiting-member-close, and settleMergeTrainMemberCloses retries it every poll thereafter, escalating to fabrik:paused after MaxRetries (§6.10, ADR-061). landMergeTrainBatch’s own step (d) has no such retry — a deliberate, narrowly-scoped-fix decision (ADR-061 §Sibling Audit); a follow-up issue is expected to extend the same machinery to this path.

4. FR-4 — Cleanup: CleanupTrainWorktree(name, deleteBranch: true) runs in a defer, removing the trial worktree directory and deleting both the local and remote trial branch (git push origin --delete). This is the single call site responsible for local worktree removal per trial lifecycle — assembleAndValidate no longer removes the worktree itself after opening the draft CI PR (pre-#1151, it did, making every downstream cleanup call a guaranteed no-op on the success path); the trial worktree now stays checked out for the full CI-poll window and is reclaimed exactly once, here (or in the equivalent failure-path cleanupTrialArtifacts call), regardless of outcome. All cleanup steps are best-effort (failures are logged, not propagated) — but only genuine failures are logged: a git worktree remove failing because the path is already not a working tree, or a git push --delete failing because the remote ref already doesn’t exist (e.g. removed by GitHub’s own “delete branch on merge” behavior once the integration PR — which reuses the trial branch — merges), is classified as a no-op success and does not emit a warn: line (#1151). The remote-branch-delete push also runs with an SSH LogLevel=ERROR override scoped to this one invocation, suppressing the “Permanently added … to the list of known hosts” notice without weakening StrictHostKeyChecking.

5. FR-5 — Restart idempotency: findIntegrationPR scans recent PRs via ListPRs (state=all) for this trial’s own PR — HeadRefName == trialBranch (fabrik/merge-train/<trialName>) is the sole, mandatory identity gate (#1615, R1). The shared idempotency marker <!-- fabrik-merge-train-batch --> is checked too, but only as non-fatal corroboration: a branch match is returned regardless of whether the marker is present (a warning is logged if it’s missing), and a marker match on a different branch is never returned, no matter how recently that PR was updated or how many repo-wide PRs ListPRs surfaces. Before #1615, the marker alone was sufficient — since every merge-train PR in the repo carries it, in any state, a later trial’s search could return an unrelated, already-merged PR from a completely different batch, and landMergeTrainBatch would trust that PR’s Merged field (alreadyMerged) to skip its own merge step and proceed straight to member advancement, closing the member PR and issue as though the batch had actually landed (reported in #1614). Once branch identity gates the match, the returned PR’s State/Merged are inspected to decide what happens next: open — FR-1 is skipped, FR-2 proceeds as normal (poll, then merge); merged — FR-2 is also skipped (alreadyMerged) and FR-3 proceeds directly, now provably safe since identity is confirmed; closed and unmerged — a failed trial, not a reusable integration PR (R2) — routes to the escalation path below instead of FR-3, never treated as “already landed” (R5). Member advancement is additionally guarded, per member, by two checks: Status == "Done" (restart safety, unchanged) and, new in #1615, whether the member’s issue number actually appears in the integration PR’s body (parseTrainMembers) — a batch that dropped a member during assembly must not be able to claim it (R4). A member failing either the closed-unmerged trial check or the per-member membership check is escalated (below) rather than silently skipped or, worse, advanced.

Landing-failure escalation (#1615, R4/R5). When findIntegrationPR returns this trial’s own PR closed and unmerged, escalateClosedUnmergedTrial escalates every survivor; when the matched (merged) integration PR’s body doesn’t list a specific member, that one member alone is escalated in place inside the FR-3 loop, leaving the rest of the batch to land normally. Both call the same escalateStrandedTrainMember, mirroring ejectRedSingleton’s reroute+comment+pause convention (§ “Merge-Train Red-Batch Bisection” below) rather than ejectMember’s counter/stay-in-Queued semantics — this is an infra-level landing failure (the integration PR itself, or the batch’s own composition), not a retryable member-level defect: rerouteQueuedMemberOffHolding moves the member off Queued to stageBeforeHolding first (a failed reroute leaves the member untouched for the next poll to retry, exactly as ejectRedSingleton’s identical failure path); on success, an explanatory 🏭 **Fabrik merge-train — landing failed** comment is posted (naming the closed PR, or the merged PR that dropped the member) and pauseMergeTrainMember applies fabrik:paused + fabrik:awaiting-input. The reentry guidance (fabrik:revalidate when the reroute target is literally named "Validate", or naming the real blocking stage:<target>:complete label otherwise) is the same reentryInstruction helper ejectRedSingleton uses, so both escalation paths give identical, correct instructions for the same reroute target. No “landed via” comment is ever posted for an escalated member (R5/AC4), and R6 — that comment may only be emitted after the batch PR is observed MERGED — holds structurally once R1–R4 do: the FR-3 loop only ever reaches the comment for a member that passed both the branch-identity and membership checks on a PR whose Merged field is true.

pollForMergeable’s own polling loop never inspects pr.State — if this trial’s PR is open when findIntegrationPR returns it but closes (unmerged) mid-poll, it is a narrower, deliberately out-of-scope variant of the same defect: the loop simply times out (unchanged pre-#1615 behavior, members remain in Queued for a fresh retry) rather than routing to the escalation path above, since it can’t misattribute a landing the way the fixed bug could.

State threading: board.ProjectID is threaded from handleMergeTrainBatch → dispatchMergeTrainWorker(ctx, batch, projectID) → mergeTrainWorkerState.projectID (immutable after dispatch, no mutex needed). Inside landMergeTrainBatch, a minimal &gh.ProjectBoard{ProjectID: projectID} is constructed for the advanceToNextStage call.

mergeTrainInFlight lifecycle: set at dispatchMergeTrainWorker entry (LoadOrStore), cleared by finishTrain — the single ADR-067-mandated clear point, called from prepareTrainWorker’s own-failure defer and runMergeTrainWorker’s top-level defer (both on success and failure). The goroutine’s entry in mergeTrainInFlight is therefore gone by the time the goroutine exits, and the next handleMergeTrainBatch poll cycle can dispatch a fresh train for any still-Queued members. finishTrain also clears the repo-scoped liveness registry inside itemstate.Store (Store.ExitRepoWorker, set by dispatchMergeTrainWorker alongside the LoadOrStore claim) — see §9.2, issue #1222. The two registries share the exact same set/clear lifecycle, but serve different purposes: mergeTrainInFlight is the atomic duplicate-launch claim plus the assembling/bisecting/CIResult/prNum/trialName sub-state; the Store registry is the single liveness answer the auto-upgrade idle guard and mergeTrainWorkerActive read.

Merge-Train Red-Batch Bisection (ADR-059 D4)

A batch is usually green: every member already passed Validate individually, so a batch fails only on a genuine cross-PR semantic conflict. runMergeTrainWorker is therefore structured as a re-form loop around a single combined validation per (re-formed) batch:

• Green path is a hard invariant (D-d): a green combined Validate costs exactly one validation and performs zero bisection — it lands immediately via landMergeTrainBatch. Bisection is strictly gated behind a red result.
• max_batch_size cap (FR-4): handleMergeTrainBatch caps the Queued snapshot to the first MaxBatchSize items (default 5) by entry order (capBatch), logging any truncation. This bounds the worst-case bisection depth.

Bisection on red (FR-1): when the combined Validate is red, handleRedBatch opens a per-episode cost budget (used = 1 for the initial red validation, capped at effectiveBisectCap() = MaxBisectValidations, default 2·⌈log₂(max_batch_size)⌉ + 1 ≈ 7) and calls bisect. bisect recursively halves the red member set in bors-ng order — validate half A; if red recurse into A; else validate half B; if red recurse into B — until a red singleton (the poisoner) is isolated. Every trial (initial and every sub-trial) is assembled by assembleAndValidate off the same base SHA pinned once at batch start (D-b, via EnsureTrainWorktreeAt), so redness is attributable to member composition, not a moving base branch (main-moved is D5, out of scope). Each sub-trial’s branch + worktree + draft CI PR is reaped immediately by cleanupTrialArtifacts.

Red singleton short-circuit (#1440, reroute added #1545): a red batch of exactly one member is never handed to handleRedBatch at all — runMergeTrainWorker’s TrainCIRed branch checks len(survivors) == 1 first and, when true, calls ejectRedSingleton directly. Bisection exists to isolate a poisoner among two or more members; a batch of one has nothing to isolate, so routing it through bisect (whose own len(red) == 1 base case is a pure pass-through) would only reach the same poisoner-isolation ejection wording — “isolated by halving bisection,” “retried in a future train with a different composition” — that is actively misleading for a batch that never had more than one member to begin with. ejectRedSingleton:

• Reroutes off Queued before doing anything else (#1545 R1/R2): calls rerouteQueuedMemberOffHolding — the same primitive and reroute-before-side-effects ordering §6.16 established for the structurally identical review-findings cause — to move the member’s board Status from Queued to stageBeforeHolding (normally Validate) before posting a comment or pausing. Before this fix the member was paused in place inside Queued, a HoldingStage column itemMayNeedWork/processItem/settleQueuedReviewFindings all structurally refuse to touch — the pause was permanently unreachable by any stage, and the “fix it, then remove fabrik:paused” instruction had an unstated precondition that only a human moving the board card by hand could satisfy. If the reroute fails, nothing is posted and the member is not paused — it looks like nothing happened, and the very next poll’s train re-forms the same singleton and re-hits this same disposition.
• Posts one comment stating plainly that the PR’s own combined Validate is failing (never “ejected… isolated by halving bisection,” never a promise of a differently-composed future train, never a conflict framing), still naming the failing check(s) via the shared renderDiagnosticBlock (#1420). The comment now names the reroute target stage and, only when that target is literally named "Validate", instructs applying fabrik:revalidate — not a bare fabrik:paused removal (#1545 R4): stage:Validate:complete is already set from this item’s original completion, so removing only fabrik:paused would silently no-op against itemNeedsWork’s completed-stage check. fabrik:revalidate’s existing handler (handleRevalidateLabel) clears stage:Validate:complete alongside fabrik:paused/fabrik:awaiting-input/etc. together, which is what actually makes Validate re-run. handleRevalidateLabel is itself hardcoded to that literal stage name, not generic over stageBeforeHolding’s (Order-derived) result — a Pruefer review finding on #1550 — so when the reroute target isn’t literally "Validate" (a config-only edge case the merge-train unit test fixture exercises via an "Implement" stage; production .fabrik/stages/*.yaml always has Validate precede Queued, but nothing enforces it), the comment instead names the item’s real blocking labels (stage:<target>:complete, fabrik:paused) directly and explains why fabrik:revalidate would not help there.
• Applies fabrik:paused + fabrik:awaiting-input immediately after the reroute succeeds, via the shared pauseMergeTrainMember helper, rather than accumulating toward ejectMember’s mergeTrainEjectionCounts 3-strike counter — every red-singleton disposition for the same member carries identical information, so counting to 3 would only measure retries of a deterministic outcome. R3 design choice — reroute + pause, not reroute-without-pause: unlike the review-findings cause (§6.16), a standalone combined-Validate failure has no external, persistent signal for the ordinary pipeline to re-detect once rerouted — the failure was only ever observed on the synthetic combined trial branch, never on the member’s own already-green PR, and the ejection comment itself is filtered out of “new comment” detection by findNewComments’ 🏭 **Fabrik prefix skip. Rerouting without pausing would therefore land the member inertly on Validate with nothing to pick it back up — a quieter stranding than the pre-#1545 bug (no fabrik:paused signal at all). Reroute + pause keeps the human gate the original design already relied on, now on a column where clearing it via fabrik:revalidate actually works. See ADR-1545.
• Since the member leaves Queued entirely, the poison-well guard below no longer needs to do any work for it — the reroute alone removes it from every future Queued batch snapshot, whether or not the pause is later manually cleared incorrectly (a member re-admitted to Queued must complete Validate again first).

handleRedBatch and bisect are otherwise unchanged; every batch with two or more members bisects exactly as described in the rest of this section, and the arity guard above means bisection itself never sees a true singleton. landOneAtATime’s own red-singleton-ejection branch (below, FR-5) validates its member completely alone — the same true-singleton scenario, reached via a different path (bisection degrading a genuinely multi-member batch rather than the top-level arity guard) — so it also calls ejectRedSingleton rather than ejectMember, getting the identical wording/no-counter/immediate-pause disposition instead of leaving the pre-#1440 misleading wording live in a second reachable path.

Eject + re-form + re-validate (FR-2/FR-3): the isolated poisoner is ejected via ejectMember (D-a — the same shared MaxMergeTrainEjections counter that assembly-conflict ejections increment; D-c — the ejection comment is the attention marker; cap→pause reuse: after MaxMergeTrainEjections ejections the member gets fabrik:paused + fabrik:awaiting-input). The poisoner remains in the Queued column — ejectMember never moves it off the board — so a still-unpaused poisoner is retried in a future differently-composed train. Poison-well guard: once a member is fabrik:paused (or its issue is closed), groupQueuedByRepo excludes it from every subsequent batch snapshot. Without this, a poisoner that reds the combined Validate even in isolation — paused but left in Queued — would be re-snapshotted into every future batch, reding and bisecting the train indefinitely and starving clean members from ever landing. A human removing fabrik:paused after resolving the underlying conflict re-admits the member. handleRedBatch returns the surviving members; the main loop re-forms and re-validates them (a survivor batch is not assumed green — a second poisoner or interaction may remain). Terminal states are clean: an empty survivor set completes the train with nothing to land (the existing zero-survivor path); each episode ejects ≥1 member or falls back, so the loop runs ≤ N episodes.

Ejection comment carries the combined-Validate diagnostic (#1420). Because a merge-train failure is, by construction, a failure that does not exist on any member’s own branch — it arises only from combining with a base that moved since the branch was cut — the combined-Validate run that observed it is the only place the failure output ever exists. pollTrainCI builds a trainCIDiagnostic (failing check names, output text/summary, and details/run URL for ordinary check-run failures; context names only for classic-commit-status RequiredContextsFailed failures, which carry no check-run output; a free-text Note for a dirty mergeable_state with no per-check signal at all) at the point of failure and threads it as a plain return value — never shared or mutable state — through assembleAndValidate → bisect/handleRedBatch/landOneAtATime → ejectMember/ejectRedSingleton (#1440: landOneAtATime’s red-singleton branch and the top-level arity-guard short-circuit above both terminate in ejectRedSingleton instead, for the true-singleton case), so the run that isolates the poisoner is always the diagnostic’s origin, and bisection’s later, unrelated re-validation of the reformed survivor batch cannot overwrite it. ejectMember/ejectRedSingleton render the diagnostic (bounded per-check inline excerpt, degrading to a head/tail truncation plus a details/run link when oversized, capped again at a whole-block hard limit well under GitHub’s comment size limit) into every ejection comment for that failure — the first as fully as the last, so an operator doesn’t pay for three uninformative cycles before learning there’s something to look at — alongside a sentence naming the other members riding in that train attempt (or stating explicitly that none were present, for a single-member train), so the operator knows before investigating that the fault does not exist on their own branch. The pause-after-MaxMergeTrainEjections comment names the same failing check(s)/context(s) and links the permalink of the ejection comment that carries the full diagnostic, rather than repeating “resolve the underlying conflict” with nothing to act on. The three ejection call sites outside this issue’s scope (unfetchable PR/head-SHA, unresolvable merge conflict) pass a nil diagnostic, leaving their ejection comments exactly as before. See ADR-1420.

One-at-a-time fallback (FR-5): when bisection cannot isolate a single culprit within the cost budget — either the budget is exhausted, or both halves validate green (a non-isolable cross-PR interaction: each half green alone, the union red — D-e, no bespoke interaction-detection in v1) — handleRedBatch logs the degrade clearly (never silent) and calls landOneAtATime, which validates and lands each remaining member as its own singleton batch. A green singleton lands via landSingleton; a red singleton (fails even in isolation) is disposed of via ejectRedSingleton (own-validation-failed wording, immediate pause, no counter — see above); a pending singleton stays in Queued. This dissolves any interaction by construction (no two members co-reside). Two non-obvious correctness points:

• Per-singleton base re-pin: unlike bisection (which pins the base once, D-b), landOneAtATime re-pins the base to the current origin/<base> before each singleton so a prior singleton’s land is visible to the next member’s validation — this is what actually dissolves a genuine {A,B} interaction (landing A, then validating B against main-with-A, which now goes red and ejects B rather than letting both land and poison main). This is a deliberate sequential base advance, distinct from the D5 concurrent main-moved race. Under the membership-keyed test seam this git step is skipped (the stub is stateless).
• landSingleton, not landMergeTrainBatch: landSingleton creates a marker-free integration PR. Reusing landMergeTrainBatch across sequential singletons would make a later singleton’s findIntegrationPR match an earlier singleton’s already-merged integration PR by the shared <!-- fabrik-merge-train-batch --> marker, skip the later singleton’s own merge, and advance it to Done without landing its code — a data-loss bug. This was true when written; since #1615, findIntegrationPR gates on trial-branch identity first (FR-5 above), so a marker match on a different branch — including an earlier singleton’s — is no longer possible on findIntegrationPR’s own side either. The reasoning above remains valid belt-and-suspenders documentation (and landSingleton’s marker-free design is unchanged), but the hazard it describes is no longer live even if a future change routed singletons through landMergeTrainBatch.

Dispatch-guard logging: state.bisecting (set for the duration of handleRedBatch) makes dispatchMergeTrainWorker log “bisecting red batch” for a re-dispatch attempt during bisection, rather than the misleading “CI red — needs attention”.

Runaway guard (ADR-059 D8): a composition-agnostic rate cap that fires when a repo creates ≥ MaxTrainTrialsPerWindow trial branches (default 20) with zero successful landings within a rolling TrainTrialWindowDuration window (default 60 min). The guard is distinct from the poison-well guard (which terminates the loop) and the bisect cost cap (which degrades to one-at-a-time within one goroutine run): it bounds the cross-poll-cycle burst that occurs when infra is broken and every trial fails.

Counter: assembleAndValidate is a thin wrapper around assembleAndValidateInner (the single site where all trial branches are created) that calls recordTrial(repoKey) only when the trial’s result is not TrainCIGreen (#1528). A green result is, by construction, either the landing attempt itself or a bisection sub-trial that just proved a sub-batch clean — never a “zero successful lands” event, which is the guard’s entire premise. Before this fix, every trial counted unconditionally, including the green survivor-validation trial that confirms a bisection isolated its poisoner and is about to land — so a successful bisection could trip the guard on its own green trial, pausing the clean survivors it had just vindicated one second before they would have landed (deterministically for any 3-member poisoned batch under the default MaxTrainTrialsPerWindow=6 test configuration, since bisecting a 3-member batch down to a single poisoner takes exactly 6 raw trials). The exclusion is by-outcome, not by-origin: a red bisection sub-trial still counts, exactly as before — it represents genuine “no progress,” the same as a red top-level trial. TrainCIPending results and assembly errors also still count unconditionally, unchanged. The counter itself is a rolling window (slice of time.Time) keyed owner/repo, protected by mergeTrainTrialsMu. resetTrialCounter(repoKey) deletes the entry; it is called from both landMergeTrainBatch and landSingleton after a successful merge, so any train where survivors do land never accumulates toward the cap.

Guard fires: when isRunawayTripped(repoKey) returns true (count ≥ N within M minutes with zero resets), fireRunawayGuard is called. It:

• Logs the event at the merge-train tag (never silent): repo key, trial count, window duration, and member count.
• Applies fabrik:paused + fabrik:awaiting-input to every member passed to it, and posts an alert comment explaining the trial count seen, the window, and operator instructions — atomically per member, and idempotently per episode (ADR-1533, #1533).

Atomicity and per-episode idempotency (ADR-1533): fireRunawayGuard is called from three independent sites — twice inside runMergeTrainWorker (Hook 1, the worker goroutine) and once from routeQueuedGroup (Hook 2, the poll goroutine) — and nothing prevents Hook 1 and Hook 2 from running concurrently for the same repoKey once the shared trial counter trips: the poll loop does not check whether a worker is mid-firing. Each call site constructs its own, possibly-overlapping member slice from whatever local state it holds. Before #1533, this meant a member appearing in two racing calls’ slices could receive a duplicate alert, and — independently — a member whose AddComment call happened to fail was left fabrik:paused with no comment and no retry path, since a paused member permanently drops out of groupQueuedByRepo’s Queued snapshot on every later poll.

The fix makes the whole pause+alert sequence for one fireRunawayGuard call a single critical section, serialized by mergeTrainRunawayMu, so two concurrent calls can never interleave their loops. Within that section, mergeTrainRunawayAlerted (an in-memory map[string]int keyed "owner/repo#N", recording the trial count at which the member was last alerted) records which members have already been alerted this episode; a call re-encountering an already-alerted member at the same or a lower count skips it entirely (no duplicate comment, no redundant label calls) — a strictly higher count means genuinely new trials ran since the last alert (only possible after an operator manually resumes a paused member without the episode having ended via a successful land), and falls through to a fresh alert rather than being skipped. resetTrialCounter — the guard’s only existing “this trip is over” signal, called on a successful land — also clears mergeTrainRunawayAlerted for the repo, so the next trip starts a fresh episode.

A member whose AddComment call fails is not marked alerted. Instead it is left with a new durable marker label, fabrik:awaiting-runaway-alert (the fabrik:paused application is unaffected — pausing never depends on the comment succeeding). settleRunawayGuardAlertScan (engine/runaway_alert_settle.go), a per-poll settle scan sourced directly from board.Items (not groupQueuedByRepo, which the member’s own fabrik:paused label already excludes it from), retries the alert every poll independent of any fireRunawayGuard call ever reaching that member again. After MaxRetries failed retries, escalateRunawayAlertFailure posts a fallback comment carrying the same explanation — but the marker is removed, and the member marked alerted, only if that fallback comment itself is confirmed posted; on a persistent AddComment outage that outlasts the fallback attempt too, the marker stays and the settle scan keeps retrying indefinitely, so the one remaining diagnostic signal is never erased out from under a member that still has zero delivered explanation (#1533 review, finding 1). See §6.18 and the Label Semantics Reference (§1.4) for fabrik:awaiting-runaway-alert’s full added-by/removed-by contract.

Two-hook dispatch: the guard is checked at two complementary points to cover both the active batch and any beyond-cap Queued members:

1. Hook 1 (worker-side, runMergeTrainWorker): after the initial re-form trial and after handleRedBatch returns runaway=true (propagated from bisect or landOneAtATime). Pauses the active batch members (membersToItems(survivors) — the survivors of the initial trial assembly, which are all members when the guard trips during bisect before any ejection).
2. Hook 2 (poll-side, routeQueuedGroup): before dispatchMergeTrainWorker is called. If the counter is already tripped (from a prior worker run), pauses all trainItems in the current Queued snapshot and skips dispatch. This handles beyond-cap members that Hook 1 could not reach.

Hook 1 and Hook 2 can run concurrently for the same repo — the “beyond-cap members cannot form their own batch while the worker goroutine is still active” argument bounds only whether a second train can form, not whether Hook 2’s own guard check can race Hook 1’s. Correctness for the alerting side comes from mergeTrainRunawayMu and mergeTrainRunawayAlerted (above), not from an assumption that the two hooks never overlap.

Operator resume: once the runaway guard fires, all affected members have fabrik:paused and fabrik:awaiting-input applied and are therefore excluded from groupQueuedByRepo on every subsequent poll. No re-formation occurs until a human manually removes both fabrik:paused and fabrik:awaiting-input from each affected member. Before doing so, the operator should investigate the infra root cause (GitHub Actions billing, required-check configuration, base-branch health). The in-memory trial counter resets to zero on engine restart, but fabrik:paused labels persist on GitHub — so a restart alone does not re-enable the train; the labels must be cleared explicitly.

fireRunawayGuard is deliberately exempted from the #1545 reroute fix (R5 audit, below): unlike ejectRedSingleton’s pre-#1545 defect, a Queued member paused by the runaway guard is not stranded — groupQueuedByRepo/routeQueuedGroup re-admit an unpaused Queued member directly from board.Items on the very next poll, entirely independent of itemMayNeedWork/processItem’s HoldingStage exclusion (that exclusion only blocks per-item stage dispatch, not the batch handler’s own board read). Rerouting every paused member to Validate instead would force each one through a full, redundant Validate re-run for a cause (infra-wide: billing, a broken base branch, all required checks erroring) no code stage can fix, and would make each member re-earn its way back into Queued via a fresh stage completion instead of simply resuming the batch it had already validated into.

Pause-in-Holding-Column Audit (#1545)

Any site that applies fabrik:paused to an item while its board Status is still a HoldingStage column (Queued) has the same structural defect ejectRedSingleton had before this issue: itemMayNeedWork excludes HoldingStage items from dispatch, processItem (the comment-unpause path) is never reached, and settleQueuedReviewFindings applies the identical closed/fabrik:paused exclusion — so nothing can ever act on the pause without a human manually moving the board card. Every such site was audited:

References: ADR-1545 (this issue’s R3 design record), ADR-1208: Queued Review-Finding Ejection (the reroute-before-side-effects precedent), ADR-1420: Merge-Train Ejection Diagnostics, issue #1545.

Merge-Train Serialization + Main-Moved Recovery (ADR-059 D5)

The batch validates a combined batch against main once; its correctness rests on the main the batch validated against being the main it lands on. Serializing the train (one batch in flight per repo) keeps main from moving under a validating batch; the only remaining mover is an external direct push by a human, which is detected and recovered. D5 adds two things without weakening the atomic LoadOrStore guard on mergeTrainInFlight.

Durable, restart-safe in-flight reconstruction (FR-1/FR-4). dispatchMergeTrainWorker keeps LoadOrStore as the sole race-critical, in-memory check (dispatch runs only from the single poll goroutine). Because that map is lost on restart, runMergeTrainWorker calls reconstructTrainState inside the already-guarded goroutine, before pinning the base, probing only durable artifacts — merge-train PRs via ListPRs and origin trial branches via WorktreeManager.ListTrainBranchesOnOrigin() (git ls-remote --heads origin refs/heads/fabrik/merge-train/*). A PR is recognized as a train PR by isTrainPR — structurally, by its fabrik/merge-train/* head branch (HeadRefName) alone (R7, #1615): every genuine trial PR, landing PR or unpromoted draft CI PR alike, is Fabrik-created on that branch. The batch marker (<!-- fabrik-merge-train-batch -->, carried by a landing PR, never by a draft CI PR) is corroboration only — reconstructTrainState logs when it’s absent on a branch-matched PR, but never treats its presence or absence as identity. Before #1615’s R7, isTrainPR accepted the marker or the branch — a PR whose body merely quoted the marker literal in prose (with nothing to do with any trial) was misidentified as a train PR; this is what let the sweep close a live, unrelated PR during reconstruction for a different batch, an incident that hit this very fix’s own PR (see R8 below).

Relevance filtering — the critical guard. ListPRs returns state=all, so it also surfaces merged/closed integration PRs from prior completed batches; those still carry the batch marker but have no members left in today’s Queued snapshot (members only leave Queued on a successful land). Reconstruction therefore selects the first train PR whose parsed members still intersect the current Queued batch (filterBatchByNumbers(batch, parseTrainMembers(body)) non-empty). A historical PR that fails this test is skipped — routing on it would wrongly abort today’s fresh batch (complete-deferred would find no still-Queued members and exit), permanently stalling the train after the first landing. A stale open train PR that fails the test is closed (and its branch cleaned) so it cannot later hijack findIntegrationPR during a fresh batch’s landing — but only after a second, deliberately redundant structural check immediately before the CloseIssue call (R8, #1615): closing a PR is destructive and irreversible, so the identity that authorizes it is re-confirmed at the point of the action itself, not only inherited from the isTrainPR filter above it in the loop. A PR that reaches this point without a train-branch head ref is skipped and logged instead of closed — ambiguity fails closed, never toward a destructive action. From the selected relevant PR (or none), it routes to:

• complete-deferred — a merged marker PR whose members are still Queued (checked first, so already-landed work is never misclassified as an orphan): completeDeferredLanding parses the member #Ns from the PR body (parseTrainMembers), intersects them with the still-Queued snapshot, and runs the idempotent landMergeTrainBatch (its alreadyMerged short-circuit skips the merge; its Status == "Done" guard prevents double-advance) to finish the member lifecycle; clears the in-flight marker;
• resume — an open train PR (with still-Queued members) backed by an origin trial branch: resumeTrain re-resolves the still-Queued members, re-polls CI on the existing trial head, and on green lands via landGreenBatch (with main-moved recovery); any non-green outcome (or no resolvable members) dissolves, so the next poll re-forms cleanly rather than re-entering bisection on resume; clears the in-flight marker;
• dissolve — an open train PR (with still-Queued members) without a backing trial branch: dissolveBatch (FR-5), commenting only on that PR’s own members; clears the in-flight marker;
• fresh — nothing relevant, or only orphaned remnants: any orphaned fabrik/merge-train/* origin branch without a relevant PR is a crash remnant and is cleaned up silently (never dissolveBatchd with today’s members — that would post confusing “batch dissolved” comments on unrelated fresh Queued issues), then reconstructTrainState returns false and the worker forms a new batch this poll (a fresh trial gets a new, unique branch name — no clash).

Reconstruction reads only durable state (never the map) and never launches a goroutine, so a restart with an empty map resumes/completes/dissolves an existing train rather than starting a duplicate. The ls-remote branch probe is gated under the trainValidateFn test seam (membership-keyed tests drive routing through listPRsFn).

Main-moved (behind) recovery at the landing gate (FR-2/FR-6). The green case calls landGreenBatch, which — before merging — checks whether the validated-green trial branch has fallen behind its base via trialBehind (FetchCommitsBehind(base, trialBranch) > 0; a probe error is treated as up-to-date, fail-safe). MergeableStateAccepted stays narrow — behind is not widened into it; it gets this dedicated branch. If up-to-date, landGreenBatch delegates to the unchanged landMergeTrainBatch. If behind (an external push advanced main), it re-pins the base to the current origin/<base> and re-assembles the survivors off the new base via assembleAndValidate (which reuses resolveConflictWithClaude for FR-6 rebase conflicts), re-runs the combined Validate, and loops back to the landing gate — bounded by MaxTrainRebaseCycles (default 3; effectiveMaxTrainRebaseCycles). On a green re-validation it loops back to the gate; on exhaustion, a non-green re-validation, or an assembly wipeout it dissolves the batch.

Dissolve semantics (FR-5). dissolveBatch closes the integration/CI PR if open (via CloseIssue — PRs are issues; there is no ClosePR), deletes the trial branch locally and on origin (cleanupTrialArtifacts), clears the mergeTrainInFlight entry (and, via finishTrain, the itemstate.Store repo-worker liveness marker), and posts an explanatory comment on each member so the outcome is observable. Members are left untouched in the Queued column — they only advance to Done on a successful land, so no status rollback is needed. It is idempotent (safe to re-run after a crash mid-dissolve: CloseIssue on an already-closed PR and cleanup on an already-deleted branch are best-effort no-ops; the comment may double-post — acceptable and observable). The next poll re-snapshots Queued and forms a fresh train.

Compose-not-duplicate with bisection. Main-moved rebase recovery lives only in landGreenBatch (the green landing path); a red re-validation after a rebase dissolves rather than entering bisection. The rebase-cycle budget (MaxTrainRebaseCycles) and the bisection cost cap (MaxBisectValidations) are therefore disjoint and never double-count — the next poll re-forms a fresh train that bisects cleanly.

Per-repo isolation (FR-3). Serialization is keyed strictly owner/repo, so trains for different repos run concurrently under the shared MaxConcurrent semaphore; the per-repo guard never cross-blocks distinct repos.

References: ADR-1615: Structural Identity for Destructive Merge-Train Actions.

1.4 Label Semantics Reference

2. Event Enumeration

Thirteen distinct event types drive state transitions (§2.1–2.11, §2.13, §2.14), plus one TUI display event (§2.12) that does not drive transitions:

2.1 Poll Tick

Trigger: The engine’s poll loop fires on a configurable interval (PollSeconds).

Code path: poll() → itemMayNeedWork() (shallow filter) → FetchItemDetails() (deep fetch) → itemNeedsWork() (full filter) → catch-up loop → dispatch loop → processItem()

Effect: The primary driver of all state transitions. Each poll cycle evaluates every item on the board through the filter chain and dispatches work for qualifying items.

2.2 New User Comment

Trigger: A user posts a comment on an issue or its linked PR. Detected by findNewComments() — filters out Fabrik-generated comments (prefix 🏭 **Fabrik) and already-processed comments (ROCKET reaction or CommentProcessed entry in itemstate.Store).

Code path: itemNeedsWork() detects new comments → processItem() routes to processComments() or triggers unpause/unblock

Effect: Can trigger three distinct behaviors:

1. Unpause: On a paused issue, a human comment (humanNewComments() — excludes bot logins via github.IsBotLogin; Fabrik’s own output is separately excluded upstream by findNewComments’ 🏭 **Fabrik body-prefix check) removes fabrik:paused (and clears failed state if present) and falls through to comment processing. A bot-authored comment does not unpause (#1083).
2. Unblock awaiting-input: On an awaiting-input issue, a human comment removes both fabrik:paused and fabrik:awaiting-input, then routes to processComments(). A bot-authored comment does not unblock.
3. Comment processing: On an active (non-paused) issue, any new comment (human or bot) routes directly to processComments() — unaffected by the human-only resume restriction above
4. Circuit breaker trip: Every processComments() invocation (from any of the three paths above, or from a review/CI-fix/rebase reinvoke, §6.2/§6.5/§6.6) records a comment-processing circuit-breaker invocation; if the issue accumulates N invocations within a rolling window with no forward progress, the engine applies fabrik:paused + fabrik:awaiting-input itself and suppresses further dispatch — see §4.6.

2.3 PR Review State Change

Trigger: A pull_request_review webhook event with action ∈ {submitted, edited, dismissed} arrives. All three actions carry the full review object (author, state, body, DatabaseID) and are routed through the same review-upsert path in applyPullRequestReviewDelta. The webhook action itself is not stored — only the review state (from the payload’s review.state field, normalised to uppercase) is recorded in item.LinkedPRReviews (upserted by DatabaseID).

• submitted — reviewer submitted a new review (APPROVED, CHANGES_REQUESTED, or COMMENTED).
• edited — reviewer edited an in-progress review (the only action some bot reviewers, e.g. GitHub Copilot, ever send).
• dismissed — a prior APPROVED or CHANGES_REQUESTED review was dismissed; the stored state becomes DISMISSED.

Code path: Delta applied by applyPullRequestReviewDelta → itemstate.PRReviewSubmitted upsert → catch-up loop in poll() re-evaluates checkReviewGate().

Effect: Can clear the review gate when len(outstanding) == 0 and at least one non-DISMISSED review exists. A DISMISSED review does not satisfy the hasReviews condition. Does not directly trigger a stage invocation.

The self-review escape hatch. hasReviews does not check the review’s author or state beyond “not DISMISSED” — a COMMENTED review submitted by the PR author itself satisfies it, even though GitHub forbids a PR author from approving their own PR. This is intentional, not an oversight: the advisory gate’s real question is “has a human looked at this?”, not “has a reviewer approved this?” (see §6.1.1 for the authoritative-mode distinction, which layers an approval requirement on top). On a repo with no reviewer ever requested — no CODEOWNERS, no review bot installed — a self-authored COMMENTED review is the supported manual way to clear an otherwise-unsatisfiable gate; wait_for_reviews: false is the supported stage-config setting for such a repo going forward. See docs/USER_GUIDE.md’s wait_for_reviews section for the operator-facing version of this note.

2.4 PR Review Threads with Feedback

Trigger: Reviewers leave inline code comments on the linked PR in unresolved review threads. These are real GitHub comments with DatabaseIDs.

Code path: Detected by buildReviewThreadComments() in the catch-up loop → dispatchReviewReinvoke() → async processComments() with synthetic comments

Effect: Triggers a review reinvocation cycle — the stage agent is re-invoked via processComments() with the review thread comments as input, allowing it to address reviewer feedback. This is a distinct event type from regular comment processing (see §6.2).

2.5 Blocking Issue Closed

Trigger: An issue listed in item.BlockedBy transitions to the CLOSED state, OR a dependent item’s BlockedBy slice is populated for the first time via deep-fetch while all listed blockers are already closed.

Primary code path (push-based): PushUnblockObserver

PushUnblockObserver is registered on engine.store and fires on two distinct events. The dependent unblocks within seconds regardless of which event arrives first.

Trigger 1 — StateChanged (blocker closes): When a blocker Y closes:

1. The observer calls store.All() to scan all known items.
2. For each item X that carries fabrik:blocked and has Y in its BlockedBy list, it checks every remaining blocker via store.Get() (fresher than dep.State from the last board fetch; falls back to dep.State == "CLOSED" if the blocker is not in the store).
3. If all of X’s blockers are now closed, the observer dispatches removeBlockedIfResolved on a goroutine, which removes fabrik:blocked via RemoveLabelFromIssue (3 attempts, exponential backoff, idempotent) and applies the cache write-through.

Trigger 2 — BlockedByChanged (dependent’s BlockedBy first populated via deep-fetch): BlockedBy is a deep-fetch-only field — after bootstrap every item has BlockedBy = nil until its first ItemDeepFetched mutation. If Trigger 1 fires before the dependent’s first deep-fetch, the dependent’s BlockedBy is empty and the scan silently skips it. When the dependent IS later deep-fetched, the Store emits BlockedByChanged, and the observer reacts:

1. It reads the post-mutation snapshot of the dependent item X directly (no store.All() scan).
2. If BlockedBy is empty after the mutation, it returns immediately (no-op — BlockedByChanged only fires when the slice actually changes, so this guards against a deep-fetch that clears or results in an empty dependency list).
3. If X carries fabrik:blocked and all listed blockers are closed in the store, it dispatches removeBlockedIfResolved on a goroutine.

Both triggers are idempotent — double-removal races are handled correctly (ErrNotFound is treated as success by removeBlockedIfResolved).

This path bypasses processItem and itemMayNeedWork entirely, so it works for items in any column — including Backlog, Done, or custom non-stage columns where itemMayNeedWork would return false. No comment is posted; the push path is a label-removal-only operation. Observer decisions (skipped, unblocked, or still-blocked reasons) are emitted under the [push-unblock] log tag in fabrik.log.

Defense-in-depth path (pull-based): itemNeedsWork exception + processItem() → checkDependencies()

processItem() applies CooldownRecorded{Reason: "dep-blocked"} each time checkDependencies() returns true (blocked). While the cooldown is active the pre-filter skips deep-fetch entirely (no goroutine, no GraphQL burn — #576 fix preserved). When the cooldown expires, itemNeedsWork detects the expiry via snap.CooldownAt("dep-blocked") and admits the item — bypassing the #576 pre-dispatch gate for this one re-check — so processItem → checkDependencies can re-evaluate: if still blocked it re-stamps the cooldown (resetting the 150s window); if resolved it removes fabrik:blocked. A missing store entry (cold-start or restart) also admits the item, since no active cooldown exists. This path fires once per 10 × PollSeconds for items in configured stage columns and serves as defense-in-depth for missed webhook events.

Note: within checkDependencies(), for each blocker the engine first consults store.Get(blocker).IsClosed() (the cache’s view) and only falls back to dep.State != "CLOSED" from the GraphQL deep-fetch if the blocker is not present in the store. This preference avoids false “still blocked” conclusions caused by GitHub indexer lag, which can delay dep.State updates by several seconds after a blocker actually closes.

Resume-from-blocked live re-read (ADR-1419, requirement 6). On the recheck path (alreadyBlocked — the item already carries fabrik:blocked), checkDependencies bypasses the cache entirely with a raw, uncached FetchItemDetails call before computing openDeps, rather than trusting the item snapshot’s own (possibly stale) BlockedBy list — mirroring recoverMissingPlanComment (engine/spawn.go) and verifyAndHealLinkage (engine/prcreate.go). This fires unconditionally on alreadyBlocked alone — not additionally gated on the cached item.BlockedBy being non-empty. A stale-empty cache is exactly the shape a bypassed engine spawn produces: a stage agent calling gh issue create directly instead of emitting FABRIK_SPAWN_CHILD never creates a blockedBy edge, so the cached snapshot has always shown zero blockers even while fabrik:blocked is present from some other source (or becomes stale after a genuine edge is added out-of-band). Gating the live read on a non-empty cached list — the pre-#1419 behavior — would skip it entirely in that shape and trust the wrong empty list, silently clearing the label on nothing. This is the check that closes the observed regression (repo-b#102): a parent resuming from fabrik:blocked must re-verify its dependency set is actually satisfied, not merely that the cached count says so.

Concurrency note: Neither StateChanged nor BlockedByChanged is in wakeChFlags or cycleSetFlags, so PushUnblockObserver fires do not wake the poll loop or populate mayNeedWork. Label removal is a direct side effect dispatched on a goroutine to avoid blocking the store notification call path. Double-removal races (two blockers closing within milliseconds, or both triggers firing for the same item) are handled correctly — ErrNotFound is treated as success by removeBlockedIfResolved.

2.6 Claude Output Markers

Trigger: Claude’s output contains one of the Fabrik markers. Checked after each stage invocation.

Markers and priority order (enforced by the if/else if dispatch chain in processItem()):

1. FABRIK_STAGE_COMPLETE + FABRIK_NO_WORK_NEEDED (both present) — highest priority among completed == true paths; completed && noWorkNeeded branch fires before the plain completed branch
2. FABRIK_STAGE_COMPLETE (alone) — next; handled by the plain completed branch
3. FABRIK_BLOCKED_ON_INPUT — checked last; only honored if completed is false and err == nil

FABRIK_NO_WORK_NEEDED is ignored unless FABRIK_STAGE_COMPLETE also appears in the same output — the no-work path requires the emitting stage to explicitly declare itself complete. The timeout/kill recovery path that scans buffered output for FABRIK_STAGE_COMPLETE does not trigger the no-work path (only FABRIK_STAGE_COMPLETE is scanned in that recovery path, not FABRIK_NO_WORK_NEEDED).

Code path: processItem() → outcome dispatch based on which marker is present

Effect:

• FABRIK_STAGE_COMPLETE + FABRIK_NO_WORK_NEEDED: handleNoWorkNeeded() — adds completion label for emitting stage; adds dummy stage:<name>:complete labels and one-line “skipped” comments for each subsequent non-cleanup stage; moves issue directly to Done; no PR created (see §6.8)
• FABRIK_STAGE_COMPLETE: handleStageComplete() — adds completion label, potentially advances to next stage
• FABRIK_BLOCKED_ON_INPUT: blockOnInput() — adds fabrik:paused + fabrik:awaiting-input
• None of the above: cooldown retry path; eventually escalateFailedStage() if MaxRetries exceeded

FABRIK_SPAWN_CHILD_BEGIN/END blocks are not processed inline — they are structured data emitted by the Plan stage and preserved in the Plan stage comment. The engine’s preImplement step reads them at Implement dispatch time to create child issues. See §6.7.

FABRIK_PR_CREATE_BEGIN/END blocks are processed inline — the engine parses the block during processItem(), creates the draft PR with Closes #N prepended as the first line, and records the PR number before posting output. See §5.6.

Invocation-level kill paths: The max_wall_time and inactivity timeout mechanisms (see §7.7) can terminate the Claude process before it writes a clean {"type":"result"} line. After such a kill, runClaude() retroactively scans the already-buffered output for FABRIK_STAGE_COMPLETE in intermediate {"type":"assistant"} NDJSON lines via extractTextFromAssistantTurns(). If found, completed=true is returned and the invocation is treated identically to a live FABRIK_STAGE_COMPLETE. If not found, completed=false is returned and the invocation routes to the cooldown/retry path. These kills are distinguished from engine-shutdown cancellation by the wasTimedOut flag, so they do not trigger the hard-error path.

Degenerate-output guard: Before any of the marker-dispatch branches above run, finalizeStageOutcome() checks the stripped/trimmed output (postOutput) against isDegenerateOutput() — a conservative check for a single-line body that is nothing but a bare @file reference or an absolute filesystem path (e.g. @/tmp/plan.md, /tmp/plan.md). This catches a model writing its real stage output to a file and returning a dangling reference instead of emitting it inline (issue #1065). If it trips, regardless of which marker was present: the output is not posted as a comment, and completed is forced to false before the branches above evaluate — so FABRIK_STAGE_COMPLETE (with or without FABRIK_NO_WORK_NEEDED) cannot advance the stage on a degenerate body. The invocation instead falls through to the “None of the above” cooldown-retry path and, on the first detection, an explanatory comment is posted immediately (rather than staying silent until MaxRetries); at the retry limit it escalates via the normal escalateFailedStage() path (see §7.2), whose pause comment names the offending reference. The detector requires an unambiguous signal (@-prefix or a leading absolute path with ≥2 segments) and only ever matches single-line output — ordinary short prose (N/A, TBD, a one-sentence completion note, or a sentence that merely mentions a path) is not flagged.

2.7 Manual Label Change

Trigger: A human adds or removes a label on the issue via the GitHub UI.

Code path: Detected on the next poll cycle when labels are fetched

Effect varies by label:

• Adding fabrik:paused → engine skips the item (unless a human comment arrives)
• Removing fabrik:paused from a failed issue → clearFailedStage() resets retry state
• Adding fabrik:yolo or fabrik:cruise → enables auto-advance (even mid-run, due to label re-fetch in handleStageComplete())
• Adding model:<name> or effort:<level> → takes effect on next Claude invocation

2.8 Issue Closed

Trigger: The issue is closed on GitHub (e.g., by PR merge with Closes #N).

Code path: itemMayNeedWork() and itemNeedsWork() check item.IsClosed

Effect (ADR-1387): A closed item is never dispatched to a Claude stage invocation. The closed-issue guard admits an item only if one of the following holds:

1. The current stage is a cleanup stage (CleanupWorktree: true) — cleanup can remove the worktree
2. The current stage has a stage:<X>:complete label — the catch-up loop can advance to the next stage (e.g., a PR merge closes an issue sitting in Validate with stage:Validate:complete; it needs to move to Done)

Everything else a closed item needs — advancing to Done, clearing stale labels, reaping a worktree, retrying a failed close — is board/label reconciliation performed by a board.Items-sourced settle scan, never by widening this admission gate. In particular, a closed item at a gate-checked stage (Validate) lacking stage:Validate:complete — carrying any gate label (fabrik:awaiting-review, fabrik:paused, fabrik:awaiting-ci) or none at all — is healed exclusively by settleClosedValidateAdvance (§6.15), independent of this admission gate. Before ADR-1387, this guard additionally admitted such items directly (a !stageIsGateChecked(stage) disjunct, plus explicit fabrik:awaiting-ci/fabrik:auto-merge-enabled admissions) on the theory that admission was the only way for the settle-owner to observe them — see §6.15 for why that coupling produced an unbounded post-close dispatch loop, and why removing it here is safe now that the settle-owner has its own independent feed.

Comment-triggered dispatch is closed on the same terms (R1, ADR-1387). Clause 2 above admits a closed, stage-complete item, so itemNeedsWork must also refuse every path that could turn a new comment on such an item into a Claude invocation. There are three: the fabrik:awaiting-input resume, the fabrik:paused unpause resume, and the plain new-comment fast path. A single item.IsClosed && !stage.CleanupWorktree guard sits ahead of all three, so a closed item carrying stage:<X>:complete together with fabrik:paused / fabrik:awaiting-input — reachable, since the pause paths apply those labels without touching the completion label — is not dispatched when a human comments. processItem carries the same guard ahead of its two resume branches as a redundant-but-explicit ownership boundary. The cleanup-stage exclusion is R1’s stated exception and covers worktree reaping only; the plain new-comment path additionally keeps its own !item.IsClosed condition so a closed item at a cleanup stage is still never routed into comment processing.

The stage:<X>:complete admission clause triggers a real FetchItemDetails deep fetch, pre-existing and unchanged by this ADR (raised on review, PR #1388). A closed item admitted solely because stage:<X>:complete is present enters deepFetchCandidates like any other admitted item — itemNeedsWork’s later “already completed this stage” check is what actually withholds dispatch, not itemMayNeedWork’s shallow filter. In the common case this is transient (the settle-owner or catch-up loop moves the item off its current column the same poll it heals it), but if the board move itself keeps failing (advanceToNextStage’s aerr/fillFailed paths in advanceValidateTerminalItem, or the equivalent in settleClosedItemsToDone), the item can sit indefinitely with stage:complete set and the board column unchanged, paying one FetchItemDetails call per poll. This clause is not new: it was present in the closed-issue guard before commit 7311a14e and is unmodified by R3’s simplification (R3 only removed the !stageIsGateChecked(stage), fabrik:awaiting-ci, and fabrik:auto-merge-enabled disjuncts) — so this ADR neither introduces nor changes this cost. A repeatedly-failing board move is itself an existing, unrelated failure mode (e.g. a missing status option, or a transient GitHub error) with no escalation path today; that gap is out of scope here, same as the “unresolved-PR polling cost” gap noted in §6.15.

Outbound case — engine-initiated close (ADR-1096): GitHub only honours Closes #N auto-close when the merged PR’s base branch is the repository’s default branch; on a base:<branch> repo where the PR targets a non-default base, the keyword is inert and nothing closes the issue, which stalls native dependency edges that unblock on close (not merge). Both terminal merge-advance paths — runValidatePRTerminalAdvance (cruise) and advanceConvergedPRToDone (non-train yolo) — call the shared helper closeIssueIfNonDefaultBase(item, prNumber) immediately after their board advance to Done, on a confirmed merge only (never for a PR closed without merging). The helper resolves the item’s base via the base:<branch> label (baseBranchForItem) and the repo’s actual default via wm.DefaultBaseBranch(); it calls CloseIssue only when they differ — when base equals default, GitHub’s own auto-close already handles it, and skipping here is what prevents a double-close. The close is best-effort and idempotent (already-closed / ErrNotFound treated as success, failures logged and never block the advance); a failed close is durably retried via the fabrik:awaiting-close settle scan — see §6.13, ADR-1097. The merge-train path (engine/merge_train.go) and the FABRIK_NO_WORK_NEEDED short-circuit (no_work_needed_settle.go) already close explicitly and are unaffected.

2.9 Review Reinvoke

Trigger: The catch-up loop Phase 1 detects unresolved PR review feedback — inline thread comments and/or an unaddressed review body from any review whose state is not DISMISSED or PENDING (Finding 4, #1375; widened from CHANGES_REQUESTED-only to any non-DISMISSED/PENDING state by #1045, since a bot reviewer that submits its findings exclusively as COMMENTED — e.g. Pruefer — was otherwise silently unaddressed; see §6.2) — on any stage:<X>:complete item (or fabrik:awaiting-ci item on a wait_for_ci: true stage) — regardless of whether the item has fabrik:yolo, fabrik:cruise, or any auto_advance config, and regardless of whether checkReviewGate currently reports the gate blocked (R1, #1375, amending ADR-1250 — see §6.1.1). Phase 1 runs unconditionally; only Phase 2 (stage advancement) is gated on those labels.

Code path: poll() catch-up loop Phase 1 → buildReviewFeedbackComments() (buildReviewThreadComments() + buildReviewBodyComments()) → cycle limit check → dispatchReviewReinvoke() → async goroutine → processComments() with synthetic comments

Distinct from regular comment processing because:

• Uses synthetic comments derived from PR review threads (LinkedPRReviewThreadComments) and review bodies (LinkedPRReviews[].Body, keyed on a synthetic "review-body:" + PRReview.DatabaseID ID — no GitHub reaction endpoint exists for a top-level review, so snap.CommentProcessed is the only dedup mechanism for these, R7), not issue comments
• Has cycle limits (MaxReviewCycles, default 5) — exceeding pauses the issue via pauseForReviewCycleLimit, the terminal fallback for a verdict that never converges (R5), reached whether the gate cleared naturally or (under review_authority: authoritative) is still blocked
• Has timeout integration (review wait timeout can also trigger pause, but only when there is genuinely nothing actionable to reinvoke on)
• Dispatches asynchronously via goroutine with semaphore slot
• The worker guard (snap.Worker() != nil) prevents double-dispatch across poll cycles
• Resolves review threads (marks them resolved on GitHub) after processing; a review-body comment has no thread to resolve, so only its CommentProcessed record is what prevents reprocessing

Yolo-merge guards against unresolved current-head review threads (#1207). Review Reinvoke’s own dispatch loop protects an item only while it is parked at a completed stage. With fabrik:yolo, Validate completing does not park anything — attemptMergeOnValidate (§2.10’s runValidatePRTerminalAdvance discussion and engine/stages.go) advances out of Validate or enables GitHub auto-merge in the same flow, and native auto-merge then leaves the PR pending for however long CI takes, with GitHub poised to merge the instant checks go green. A review finding arriving inside that window is invisible to both the Review Reinvoke trigger above (nothing is parked to reinvoke) and to §2.16’s SHA-invalidation scan (a review comment does not change the head SHA). Two guards close this, both built on currentHeadReviewThreadComments (which excludes threads GitHub marks isOutdated — superseded by a later push — so a stale-SHA thread never blocks indefinitely) rather than a parallel detection path — note the scope split: guard 1 (attemptMergeOnValidate) is scoped to inline thread comments only, unchanged by #1375; guard 2 (inside handleReviewGate) additionally covers unaddressed review bodies via currentHeadReviewFeedbackComments (currentHeadReviewThreadComments + all review-body comments, R8 — see §6.1.1/§6.2), since that guard sits directly alongside the wider Finding-4 reinvoke check it was already updated for:

• Guard 1 — attemptMergeOnValidate (engine/stages.go): before the merge-train/enqueue/enable logic, the function checks currentHeadReviewThreadComments(item). If non-empty, it logs "not advancing: N unresolved review thread(s) on <sha>" and returns (false, true, nil) — the new third return value, deferred, distinguishes this from the two pre-existing benign (false, nil) cases (cruise label, no linked PR). handleStageComplete folds deferred into shouldAdvance exactly like autoMergeEnabled. No new retry plumbing is needed: poll.go’s Phase 2 Validate branch already calls attemptMergeOnValidate on every poll while fabrik:auto-merge-enabled is absent, so once the review-reinvoke loop above resolves the thread, the very next poll proceeds normally.

  Freshness caveat at the primary call site. handleStageComplete’s own “Path 1” (§2.6/§2.9 above) is synchronous with the Claude invocation that just completed Validate, and reuses the pre-invocation item snapshot — the same staleness already documented for its wait_for_reviews handling a few lines below in engine/stages.go, and can be tens of minutes old on a long Validate run, not just a few seconds. A thread posted during that run is invisible to guard 1 at this call site, so Fabrik can proceed to enable auto-merge in this narrow window. This is deliberately not closed by re-fetching inside attemptMergeOnValidate — that would reintroduce the per-completion GraphQL round-trip Path 1 exists to avoid. It is closed instead by guard 2 on the very next poll: fabrik:auto-merge-enabled is now set, poll()’s deep-fetch (selectDeepFetchCandidates) has refreshed the item with current LinkedPRReviewThreadComments, and handleReviewGate disables auto-merge before GitHub can act on the miss. The resulting exposure is the same PollSeconds-bounded residual race already accepted for guard 2’s convergence-window disable (see below), not a new unbounded gap. poll.go’s own Phase 2 Validate retry — the other caller of attemptMergeOnValidate — always operates on deep-fetched, fresh data and is unaffected by this caveat.

  Exception: the direct-merge fallback has no convergence window, so it gets a live re-check instead of trusting the caveat above. The freshness caveat’s reasoning (“a miss here is closed by guard 2 on the next poll”) depends on there being a next poll to close it on — true for the native-auto-merge and merge-queue-enqueue paths, which leave the PR pending in a convergence window guard 2 monitors. It does not hold for the direct-merge fallback (taken when EnablePullRequestAutoMerge fails for any reason other than ErrAutoMergeNotEnabled — e.g. the PR is already in CLEAN or UNSTABLE status): that branch calls MergePR synchronously, in the same invocation, with no window and no later poll for guard 2 to act on. A thread that appeared during the staleness gap above would otherwise merge underneath both guards in one step — identified in review (Pruefer, PR #1211). Immediately before the MergePR call, attemptMergeOnValidate now does a raw, uncached FetchItemDetails re-read (the same “re-read immediately before an irreversible decision” idiom used by checkDependencies, recoverMissingPlanComment, and verifyAndHealLinkage) and re-checks currentHeadReviewThreadComments against the fresh data. A blocking thread defers ((false, true, nil), same as guard 1 above); a fetch failure surfaces as an error so the existing retry-next-dispatch path handles it, rather than proceeding to merge on stale data.

• Guard 2 — inside handleReviewGate, not checkAutoMergeConvergence (engine/catch_up_handlers.go): the naive placement — inside checkAutoMergeConvergence (engine/merge_gate.go), the function that polls repeatedly during the auto-merge-enabled window — does not work. catchUpPhase1Handlers (§6.2) is dependencies → reviewGate → autoMergeConvergence → mergeAndCIGates, and each handler claims and stops the chain on true. Whenever fresh unresolved feedback (a thread comment or a review body, Finding 4) appears on an item that already carries fabrik:auto-merge-enabled, handleReviewGate (Handler 2) finds it via buildReviewFeedbackComments and dispatches a reinvoke on that same poll — claiming the item and preventing handleAutoMergeConvergence (Handler 3) from ever running. A check placed only in checkAutoMergeConvergence would therefore never fire in exactly the scenario it exists to catch. Guard 2 instead lives inside handleReviewGate itself: immediately before the dispatchWithCycleLimit call, if the item carries fabrik:auto-merge-enabled and currentHeadReviewFeedbackComments(item) is non-empty, disableAutoMergeForReviewThreads fires — logging "disabling auto-merge: N unresolved review thread(s) on <sha>", then calling DequeuePullRequest (merge-queue-enabled PRs) or the new DisablePullRequestAutoMerge (native auto-merge PRs, mirroring reenableAutoMergeAfterRebase’s existing queue-aware branch), and only removing fabrik:auto-merge-enabled on mutation success. Mutation-before-label-removal is deliberate: on partial failure (mutation succeeds, label removal fails) the label stays, so the disable is retried next poll — the alternative order risks Fabrik going label-blind to a PR GitHub might still merge. Removing the label on success stops handleAutoMergeConvergence from running for this item at all (so it cannot fight the ejection-recovery ladder or misread the dequeue as a genuine ejection), and hands re-enablement to guard 1’s existing retry loop once the review-reinvoke dispatch (fired on the same poll, immediately after) resolves the thread — the same remove-then-reapply idiom pauseForMergeGroupStall already uses for the convergence-budget anchor. MaxReviewCycles is unaffected — the disable is additive to the existing dispatch/cycle-limit/pause path in handleReviewGate, so the cycle limit still bounds and escalates regardless of which guard triggered the poll.

Both guards are an explicit stopgap for fabrik:yolo specifically (see #1071, which is expected to supersede this mechanism with a decidable “hold a valid signature from every required signer for this SHA” check); fabrik:cruise is unaffected because it already stops at Validate for human merge, well before either guard point.

2.10 CI Check Completed

Trigger: CI check runs on the PR head SHA transition from pending to a terminal state (success, failure, etc.). Fabrik detects this by polling on each catch-up loop iteration — there are no webhooks.

Pending-over-failed precedence (#958): Both settlePRMergeState() and checkCIGate() classify check runs via the shared github.ClassifyCheckRuns() helper, which (1) reduces check runs to the latest run per check name (highest ID wins — a stale completed/failed run left behind by a rerun under a new ID is discarded) and (2) treats any pending (queued/in_progress) run as taking global precedence over any failed run, regardless of whether the failed run is a different check name or a superseded run of the same name. A single failed check-run coexisting with a pending check-run on the same head therefore classifies as WAIT (PRMergeUnsettled / ciBlocked=true, ciFailure=false), never FAIL — the engine waits for the pending check to reach a terminal state instead of dispatching a CI-fix reinvoke. Per ADR-1410, this wait is bounded by liveness (no observed check-run progress for CIWaitTimeout, tracked via LinkedPRState.LastCIProgressAt), not elapsed time — a suite that keeps transitioning check-run state waits indefinitely, up to the separate, much larger CIBackstopTimeout absolute cap. Both call sites use the same helper so they cannot drift out of agreement. See §6.4 rule 19 and §6.5.1 for the full classification rules.

Required-context awareness (ADR-933, #933): ClassifyCheckRuns() alone has no concept of “required” — skipped/neutral/absent check runs fall through to CheckRunsReady, which is correct for a non-required job but wrong for a required one that simply never ran (e.g. a repo whose required signal is a classic commit status posted out-of-band, with GitHub Actions disabled). settlePRMergeState() layers a required-context pre-filter (engine.classifyRequiredContexts() → github.ClassifyRequiredContexts()) in front of every point where check-run classification alone would otherwise resolve to PRMergeReady: the zero-check-runs “no CI configured” fallback (§6.4 rules 13 & 17) and the check-runs-present “all green” fallback (§6.4 rule 19). The pre-filter checks each name in the repo’s configured required_status_contexts (.fabrik/config.yaml, keyed by "owner/repo"; unconfigured repos are always a no-op — zero behavior change) against the union of check-run names and classic commit-status contexts (FetchCombinedStatus, fetched only when a required name isn’t already resolvable from check runs) observed on the exact head SHA. Only a confirmed success counts; a confirmed failure resolves to PRMergeBlocked, and anything else not-yet-confirmed (missing, pending, skipped, neutral) resolves to PRMergeUnsettled — never silently to PRMergeReady. checkCIGate() mirrors this with a matching pre-check, classifyCIFromRequiredContexts(), run ahead of classifyCIFromCheckRuns()/classifyCIFromMergeableState() — necessary because a required-context failure sourced solely from a classic commit status has no check-run footprint for those two functions’ checkRuns-only view to react to. The merge-train poller (pollTrainCI(), engine/merge_train.go — see §1.3’s FR-2 mention of its 30s poll interval) classifies check runs via the same shared github.ClassifyCheckRuns() helper (replacing a previously-separate inline duplicate) and applies the identical required-context pre-filter to its trial-branch SHA. See ADR-933 for the full required-context design.

pollTrainCI’s own mergeable_state shortcut — touched by ADR-1153, #1153. pollTrainCI no longer treats an accepted mergeable_state (clean/unstable) as sufficient for TrainCIGreen by itself. mergeable_state is computed by GitHub from required checks only, so it can read as accepted while a non-required check (e.g. the actual test suite, if left unmarked-required) is still queued/in_progress on the trial SHA — exactly what happened on integration PR #1150, where the train merged 20 seconds after the sole required check went green, 14 seconds before the non-required test job itself reported. The two signals now compose instead of racing:

• mergeable_state == "dirty" still returns TrainCIRed immediately — unambiguous, unchanged.
• An accepted mergeable_state is recorded but no longer returns green by itself; it is necessary but not sufficient.
• The check-run pass (gh.ClassifyCheckRuns + the required-context pre-filter above) is the actual green determinant on the common path: any run still queued/in_progress keeps polling; a confirmed failing run — required or not (Strict policy, ADR-1153) — returns TrainCIRed; all-clear plus required contexts satisfied returns TrainCIGreen.
• Zero check runs at all (e.g. GitHub Actions disabled — the #933 case): there is no per-check completeness signal to consult, so an accepted mergeable_state combined with RequiredContextsSatisfied is the one remaining basis for TrainCIGreen. A confirmed required-context failure still returns TrainCIRed immediately, exactly as before.

Every terminal pollTrainCI decision now logs the specific check runs (name, status/conclusion) it was based on. See ADR-1153 for the full design, the Strict non-required-failure policy rationale, and its relationship to ADR-072 (the single-PR MergePR self-gate, which still accepts unstable and is unaffected) and ADR-933 (whose required-context classifier is reused unmodified).

pollForMergeable’s identical shortcut — fixed by ADR-1441, #1441 (previously the unfixed sibling ADR-1153 flagged and left open). pollForMergeable (engine/merge_train.go, the merge-train landing-step poller — distinct from pollTrainCI’s CI-poll step above) used to treat an accepted mergeable_state as an unconditional green light for landing, with no check-run fallback at all — not even the partial classification settlePRMergeState had for other values. It now fetches FetchPRDetails (for HeadSHA alongside MergeableState) and classifies via a new classifyLandingCI helper that mirrors pollTrainCI’s composition exactly (same shared primitives — gh.ClassifyCheckRuns, e.classifyRequiredContexts, describeCheckRuns, gh.MergeableStateAccepted — same ordering, same zero-check-runs fallback), reusing TrainCIResult as its verdict type: mergeable_state == "dirty" → immediate reject, no check-run fetch; a confirmed check-run or required-context failure → reject; a check still pending → keep polling (bounded by CIBackstopTimeout, exactly as before); zero check runs → the mergeableAccepted && RequiredContextsSatisfied fallback. classifyLandingCI is a new function, not literally shared code with pollTrainCI (out of scope for #1441, already fixed) — see ADR-1441 for why “one shared mechanism” is satisfied at the primitive level rather than by refactoring pollTrainCI itself.

Code path: poll() catch-up loop Phase 1 → settlePRMergeState() (one combined REST pass for mergeable, mergeable_state, and check runs) → checkCIGate() interprets the PRSettleResult:

• PRMergeTerminal (pr.Merged == true): addCompleteLabelAndRemoveCI → gate clears, advance to Done.
• PRMergeTerminal (closed, not merged): pauseForPRClosedNotMerged() posts comment naming the PR, adds fabrik:paused + fabrik:awaiting-input, removes fabrik:awaiting-ci, fabrik:awaiting-review, and fabrik:rebase-needed (all three gate labels, not just fabrik:awaiting-ci — ADR-1387 R6: cleanupClosedIssueTransientLabels no longer sweeps these independently for a closed item at Validate, so this is the only remaining place that clears them on the closed-unmerged/pause path). Returns (false, false, false).
• PRMergeReady from the mergeable_state == "clean" fast path (ADR-033/ADR-1441): gate clears immediately (addCompleteLabelAndRemoveCI); per-check classification is skipped — GitHub has confirmed every check, required or not, passed. mergeable_state == "unstable" is no longer a shortcut (ADR-1441, #1441): it falls through, unmodified, into the same check-run/required-context classification chain described below that "blocked" and every other non-accepted value already use — a confirmed failure on a non-required check now blocks (PRMergeBlocked) instead of clearing the gate. PRMergeReady can still result for unstable PRs, but only when that classification chain itself finds nothing blocking (e.g. every observed check passed, or only skipped/neutral/cancelled conclusions are present).
• A confirmed required-context failure (ADR-933): classifyCIFromRequiredContexts() returns (true, true, false) unconditionally — a verdict, never a timeout, regardless of how long fabrik:awaiting-ci has been applied (ADR-1410, R3) — evaluated before the check-run/mergeable_state classification below, since the failure may be visible only via settle.RequiredContextsStatus/RequiredFailed, not settle.CheckRuns.
• PRMergeUnsettled or PRMergeBlocked with CheckRuns populated: classifyCIFromCheckRuns() classifies via gh.ClassifyCheckRuns first (ADR-1410):
  • Confirmed failure (CheckRunsFailed): returns (true, true, false) unconditionally — a verdict, never a timeout, regardless of elapsed time (R3, fixing the pre-ADR-1410 defect where a failure past the deadline was misreported as a timeout). Optionally dispatches dispatchCIFixReinvoke() → async goroutine → processComments() with synthetic CI failure comment.
  • Still pending: liveness-stall dwell (R1/R2) — a new ciProgressStalledSince() helper reads LinkedPRState.LastCIProgressAt from the itemstate store (set whenever a check run’s content actually changes, or the head SHA advances — see §6.14.5); if that timestamp is zero (no progress observed yet this process’s lifetime — cold start) or time.Since(...) < CIWaitTimeout, returns (true, false, false) (still blocked, re-evaluate next poll); once non-zero and elapsed ≥ CIWaitTimeout, removes fabrik:awaiting-ci and returns (false, false, true) (caller calls pauseForCITimeout).
  • R3 (empty CheckRuns arm, MergeableState == "blocked"): hadChecks == false; check FetchLabelAppliedAt dwell — unchanged by ADR-1410, since this case has no check-run signal to observe progress on at all; if < CIWaitTimeout → return (true, false, false) (dwell guard, not yet paused); if ≥ CIWaitTimeout → pauseForRequiredNeverRunningCheck() posts distinct comment, adds fabrik:paused + fabrik:awaiting-input, removes fabrik:awaiting-ci
  • New guard (empty CheckRuns arm, MergeableState ∉ {"", "unknown"}): hadChecks == false; checks FetchLabelAppliedAt inline — unchanged by ADR-1410, same “no signal to observe” reasoning — if fabrik:awaiting-ci has been present ≥ CIWaitTimeout, removes fabrik:awaiting-ci and returns (false, false, true) (caller calls pauseForCITimeout); otherwise returns (true, false, false) — mergeable_state actively blocking via a signal Fabrik cannot see via check_runs (e.g. a Commit Status / legacy Statuses API); re-evaluate on next poll.
  • Post-push dwell guard (empty CheckRuns arm, MergeableState ∈ {"", "unknown"}): when hadChecks == false AND lpr.LastHeadSHAUpdate is non-zero AND time.Since(lpr.LastHeadSHAUpdate) < PostPushDwell (default 90 s): return (true, false, false) — GitHub has not yet computed mergeability or started CI for the newly-pushed SHA; re-evaluate after the dwell window elapses. Zero LastHeadSHAUpdate (cold start / post-restart) falls through to R5 immediately (EC-1 preserved). LastHeadSHAUpdate is stamped in the Store’s PRHeadSHAUpdated handler only when the SHA actually changes. Configured via PostPushDwell in engine.Config; set from --post-push-dwell flag (seconds) or FABRIK_POST_PUSH_DWELL env var; default 90 s.
  • R5 (fallthrough): hadChecks == false + MergeableState ∈ {"", "unknown"} + dwell elapsed (or LastHeadSHAUpdate zero) + required contexts satisfied or unconfigured (ADR-933) → addCompleteLabelAndRemoveCI → gate clears as “no CI configured”; stage:<X>:complete added

Settling primitive consolidation (v0.0.53): settlePRMergeState() reads mergeable, mergeable_state, and check runs in one pass before both the merge-conflict gate and the CI gate. This eliminates the split-brain where two separate REST calls within one poll cycle could observe different GitHub state. See §6.4 for the full specification. MergeableState is intentionally omitted from PRSettleResult in the hadChecks, post-push dwell, and HeadSHA-empty cases so that checkCIGate’s R3 path (MergeableState == "blocked" + empty CheckRuns) cannot misfire on those cases. The mergeable_state field is null on the list endpoint used by FetchLinkedPR, so FetchPRMergeableFields() (a separate single-PR REST call) provides both mergeable and mergeable_state in one request.

(false, false, false) return semantics: checkCIGate returns (false, false, false) for all of the following terminal conditions, which it handles internally (the poll.go call site never sees ciTimedOut=true for these): R1 merged PR, R2 closed-not-merged PR, R3 required-never-running check, no linked PR, all-green checks. The caller only calls pauseForCITimeout when timedOut=true — since ADR-1410, this fires only for a genuine liveness-dwell-exceeded condition (check-run progress stalled, or the R3/mergeable-state elapsed dwell), never for a confirmed CI failure (which returns ciFailure=true instead, unconditionally).

Distinct from Review Reinvoke because:

• Triggered by check run status changes, not reviewer submissions
• Uses fabrik:awaiting-ci label (not fabrik:awaiting-review)
• Only active on stages with wait_for_ci: true
• fabrik:awaiting-ci is applied by handleStageComplete on FABRIK_STAGE_COMPLETE (the in-flight CI-await marker, present for both pending and failed checks); stage:X:complete is withheld until checkCIGate confirms CI is green (ADR 032)
• Timeout tracked two different ways depending on which dwell applies (ADR-1410): the R3/mergeable-state-blocked dwells and the settleAwaitingCIScan backstop use FetchLabelAppliedAt on fabrik:awaiting-ci (durable across restarts — see §6.14.5); the check-run-pending liveness-stall dwell uses in-memory LinkedPRState.LastCIProgressAt instead (not durable — a restart resets it to “no progress observed yet,” the safe cold-start default, never a blind escalation)
• CI-fix cycle counter is StageState.CIFixCycles[stageName] in itemstate.Store (written by CIFixCycleIncremented mutation; read via snap.CIFixCycles(stageName))

Single-owner Validate advance, split into an open-item and closed-item pair (runValidatePRTerminalAdvance / settleClosedValidateAdvance, R4 — ADR-056 D2, ADR-1387): Both functions delegate to the same shared per-item logic, advanceValidateTerminalItem, and differ only in what they iterate and where they’re called from. For each candidate item, e.client.FetchLinkedPR is called directly (not e.readClient — boardcache may have stale Merged/State), and the logic runs regardless of which gate label (fabrik:awaiting-ci, fabrik:awaiting-review, fabrik:rebase-needed, or any future label) is present — no disjointness maintained by label negation anywhere. Items already in advancedItems are skipped (no double-advance), and items carrying fabrik:auto-merge-enabled are excluded (that convergence flow is owned exclusively by checkAutoMergeConvergence, Phase 1). When pr.Merged == true: the shared logic iterates all pipeline stages in ascending Order from the stage after the highest already-complete stage up to (but not including) the cleanup-terminal stage, adding stage:<N>:complete for every WaitForCI or WaitForReviews gate-checked stage whose label is absent (with cache write-through; fail-fast on error for idempotent retry). After all completion labels are added, removeAwaitingCILabel, removeAwaitingReviewLabel, and removeRebaseNeededLabel are called as applicable, fabrik:paused + fabrik:awaiting-input are removed, and advanceToNextStage() is called. Immediately after the Done advance, closeIssueIfNonDefaultBase(item, pr.Number) runs (ADR-1096) and explicitly closes the issue when its resolved base:<branch> differs from the repo default — see §2.8. When pr.State == "closed" and !pr.Merged (closed without merging): pauseForPRClosedNotMerged() is called unless the item is already paused (to avoid duplicate comments). Neither function may ever dispatch workers or acquire e.sem (carries ADR-053 constraints; superseded by ADR-057).

• runValidatePRTerminalAdvance is the open-item owner: it iterates deepFetchCandidates (unchanged sourcing) and skips any item.IsClosed item — a redundant-but-explicit ownership boundary, since admission (§2.8) no longer lets a closed item reach deepFetchCandidates at a gate-checked stage in the first place.
• settleClosedValidateAdvance (§6.15) is the closed-item owner: it iterates board.Items directly, independent of dispatch admission entirely, so healing a closed item at Validate never depends on that item having been admitted to dispatch.

Note: Non-Validate-stage items with fabrik:paused and a merged PR are not handled by either function; they require manual label intervention to clear fabrik:paused, after which the normal catch-up loop advances them.

2.11 Base Branch Advanced

Trigger: The PR’s base branch moves forward (a different PR merges) while this branch is sitting in the post-stage:Validate:complete catch-up window. GitHub recomputes mergeable on the linked PR; if the new base conflicts with this branch, mergeable transitions from true (or null) to false.

Code path: poll() catch-up loop Phase 1 → settlePRMergeState() (combined REST pass: FetchLinkedPR + FetchPRMergeableFields for mergeable and mergeable_state) → checkMergeabilityGate() interprets PRSettleResult → evaluates the flag → optionally dispatches dispatchRebaseReinvoke() → async goroutine → processComments() with a synthetic rebase-required comment

Distinct from CI Check Completed because:

• Triggered by base-branch movement, not check run status changes
• Uses fabrik:rebase-needed label (not fabrik:awaiting-ci)
• Runs before the CI gate in catch-up Phase 1 — a PR that cannot merge has no reason to spin on CI-await
• Only active on stages with wait_for_ci: true (same opt-in as the CI gate — these are the stages admitted to the catch-up window via fabrik:awaiting-ci)
• fabrik:rebase-needed is only applied on confirmed conflict (mergeable == false), not on mergeable == null (GitHub still computing)
• Rebase cycle counter is StageState.RebaseCycles[stageName] in itemstate.Store (written by RebaseCycleIncremented mutation; read via snap.RebaseCycles(stageName))
• Resolution relies on Claude rebasing in the worktree (to handle semantic collisions like duplicated ADR numbers) rather than an engine-side git rebase

2.12 TurnProgressEvent (TUI Display Event)

Trigger: A {"type":"user"} NDJSON line is written to Claude’s stdout pipe during a Claude invocation. Each logical turn (one user→assistant cycle) begins with exactly one such line (either the initial prompt or a tool-result response), so this fires once per logical turn.

Code path: runClaude() stdout pipe → turnCountingWriter.Write() → detects type == "user" line → increments per-invocation counter → calls claudeTurnProgress(issueNumber, turnsUsed, maxTurns) → Engine.emit(TurnProgressEvent{...}) → TUI channel

Effect: Purely additive display — does not trigger any state transitions, label mutations, or issue processing. The TUI consumes TurnProgressEvent to update the live turn counter shown in:

• The In Progress pane row for the active issue (width-adaptive badge [N/M turns] / [N/M] / omitted)
• The detail panel for the selected active item (Turns: N/M)

TurnProgressEvent is only emitted in TUI mode (claudeTurnProgress is nil in plain-text mode and tests). It uses the non-blocking emit path (drop-if-full), so turn-progress updates are best-effort and may be dropped under backpressure. This does not affect engine behavior because the event is display-only; at most one event is produced per logical turn.

MaxTurns in the event carries the effective budget for the current invocation — effectiveBudget as computed in InvokeClaude() (which already accounts for opts.MaxTurnsOverride from the extension loop). This means:

• First invocation without fabrik:extend-turns: stage.MaxTurns
• First invocation with fabrik:extend-turns: 2 × stage.MaxTurns
• Extension loop second iteration: stage.MaxTurns (per-invocation limit, not cumulative)

2.13 Manual Assignee Change

Webhook event: issues.assigned / issues.unassigned

Detection: applyIssuesDelta (boardcache/delta.go:382) applies IssueAssigneesUpdated, which emits the AssigneesChanged flag. The mayNeedWorkObserver and wakeChObserver (engine/observers.go) include AssigneesChanged in wakeChFlags, so the assignment fires both a wake signal and a mayNeedWork cycleSet entry.

Effect: Dispatcher re-evaluates the item on the next poll cycle. The engine does not currently filter dispatch on assignee — assignee changes do not change what work happens, only that the item is re-considered. Future assignee-as-dispatch-filter work (planned, not yet filed) will give this event additional dispatch semantics.

Why: Assignment is a strong “please look at this” signal from the user, and is the mechanical underpinning of multi-user shared boards (each fabrik instance picking up only items assigned to its cfg.User).

2.14 Worker Lifecycle

Source: Engine-internal mutation, not a webhook.

Detection: The dispatch loop (and each reinvoke dispatcher — dispatchReviewReinvoke, dispatchCIFixReinvoke, dispatchRebaseReinvoke) applies WorkerEntered{Repo, Number, StageName, StartedAt} synchronously before the goroutine is launched. WorkerExited{Repo, Number} is deferred at the top of each goroutine so it fires on any exit path (context cancel, ensureRepoReady failure, normal return). Both mutations emit WorkerChanged | WorkerLifecycleChanged; WorkerLifecycleChanged (not the broader WorkerChanged) is the flag in wakeChFlags. WorkerHeartbeat and WorkerPIDSet emit only WorkerChanged and do not wake the poll loop — this prevents deep-fetch churn for active workers (30s heartbeat × N workers would otherwise trigger repeated deep-fetches for items that can’t be dispatched anyway).

Effect: WorkerExited adds the item to mayNeedWork and, when wakeCh != nil, fires the wakeChObserver so the dispatcher re-evaluates within milliseconds. In headless runs (--notui or any configuration without a wake channel), the mutation populates mayNeedWork for the next ticker-driven poll (within PollSeconds); there is no immediate wake. Either way, re-evaluation does not depend on cooldown expiry or external events. This eliminates the previous race where self-advance to the next stage would wait up to 150s (PollSeconds × 10) if the departing worker had not finished cleanup before the post-advance dispatch loop ran.

Dispatch guard: The dispatch loop uses snap.Worker() != nil (Store-backed) instead of the former e.inFlight.Load(iKey) (sync.Map). Because WorkerEntered is applied before wg.Add(1) and before the goroutine starts, snap.Worker() != nil is true from the instant the goroutine is scheduled — there is no window where a new dispatch cycle could race in and double-dispatch the item.

Why: Worker lifecycle is engine state the dispatcher must react to. Pre-Fix B (issue #544), it lived in e.inFlight (sync.Map) outside the Store — a known bypass that violated ADR 036’s single-owner reactive cache invariant. WorkerEntered/WorkerExited complete the migration begun by the Phase 5 F3 store unification.

2.15 Revalidate Label (fabrik:revalidate)

Source: Operator-applied GitHub label.

Detection: A dedicated revalidate-scan loop, settleRevalidateScan (engine/poll_settle.go; called from poll() in engine/poll.go), runs after the paused-item recovery loop and before the dispatch loop. It iterates all deepFetchCandidates unconditionally (paused items included — FR-5). Items that don’t carry fabrik:revalidate are skipped immediately. The loop is reached by stuck-Validate items because fabrik:revalidate is included in the hasAwaitingLabel pre-filter bypass at poll.go:1294, ensuring items with settled updatedAt are still deep-fetched on every poll.

Effect (Validate-stage item, no in-flight worker):

1. Removes stage:Validate:complete, stage:Validate:failed, fabrik:paused, fabrik:awaiting-input, fabrik:awaiting-ci, fabrik:auto-merge-enabled from GitHub (best-effort sequential REST calls; 404 responses are treated as already-absent and are not errors).
2. Removes fabrik:revalidate itself from GitHub.
3. For each successful removal: cacheImpl.ApplyLabelRemoved + webhookMgr.RegisterEcho (cache write-through and echo registration, same pattern as all other label mutations in the engine).
4. Applies four store resets: StageRetryCleared, EngineUnpaused, StageLastAttemptCleared, EngineCyclesCleared — all for StageName: "Validate". This is the same full-reset sequence used by clearFailedStage.
5. Validate dispatches on the next poll cycle: the revalidate scan does not update deepFetchCandidates in place. After label removal, the item re-enters the board on the next poll with no blocking labels and no LastAttemptAt cooldown, so itemNeedsWork returns true and the dispatch loop invokes processItem.

Effect (non-Validate item): Logs a warning, removes only fabrik:revalidate, takes no other action.

In-flight worker guard (FR-4): If snap.Worker() != nil (a Validate invocation is still active), the label is left in place and the revalidate scan defers to the next poll cycle. When the worker exits, WorkerExited populates mayNeedWork and the next poll processes the label normally.

Partial label-removal failure: Each REST call is independent. If a removal fails (non-404 error), the engine logs a warning and continues. On the next poll, the item is still deep-fetched (if fabrik:revalidate was not yet removed), and the revalidate scan retries the remaining removals idempotently.

Closed-issue cleanup: fabrik:revalidate is in transientLifecycleLabels, so cleanupClosedIssueTransientLabels sweeps it from closed issues defensively (FR-7).

Why: Recovery from a stuck-Validate state previously required knowing and clearing 4–5 individual labels. fabrik:revalidate collapses recovery to one action. It stays useful even after structural CI-gate fixes (#855) for operator-initiated re-runs when CI infrastructure has recovered out-of-band, a flaky test was resolved manually, or other non-engine causes.

2.16 SHA-Invalidation Scan

Source: Automatic — triggered by PRHeadSHAUpdated events from any source (webhook, reconcile, deep-fetch probe).

Detection: A SHA-invalidation scan, settleSHAInvalidationScan (engine/poll_settle.go; called from poll() in engine/poll.go), runs immediately after the revalidate scan (§2.15) and before the dispatch loop. It iterates all deepFetchCandidates. Items that don’t carry stage:Validate:complete are skipped immediately. For qualifying items it reads snap.ValidateCompletedSHA() (the HEAD SHA recorded when Validate last completed) and snap.LinkedPR().HeadSHA (the current PR HEAD). A mismatch triggers invalidation. Items are admitted to deepFetchCandidates via the standard PRHeadSHAUpdated → LinkedPRChanged → mayNeedWorkObserver → cycleSet observer pipeline — no bypass label is required.

Effect (SHA mismatch, no in-flight worker):

1. Removes fabrik:rebase-needed, fabrik:auto-merge-enabled, fabrik:awaiting-ci, fabrik:awaiting-review, stage:Validate:complete from GitHub (best-effort sequential REST calls; 404 responses are treated as already-absent and are not errors). fabrik:rebase-needed is included because a conflict determination tied to the invalidated completion SHA no longer describes reality (#1225).
2. For each successful removal: cacheImpl.ApplyLabelRemoved + webhookMgr.RegisterEcho (cache write-through and echo registration).
3. Applies ValidateCompletedAtSHACleared to zero LinkedPR.ValidateCompletedSHA in the Store.
4. Applies four store resets: StageRetryCleared, EngineUnpaused, StageLastAttemptCleared, EngineCyclesCleared — all for StageName: "Validate".
5. Validate dispatches on the next poll cycle: after label removal, the item re-enters the board with no blocking labels and no LastAttemptAt cooldown, so itemNeedsWork returns true and the dispatch loop invokes processItem.

Guards:

• FR-5 (empty completion SHA): If snap.ValidateCompletedSHA() == "" — pre-feature legacy item, or worktree HEAD was unavailable at completion time — the scan skips the item unconditionally. Operators use fabrik:revalidate (§2.15) for legacy items.
• FR-6 (in-flight worker): If snap.Worker() != nil (a Validate invocation is active), the scan defers to the next poll. When the worker exits, WorkerExited re-populates mayNeedWork and the SHA-invalidation scan re-evaluates on the next poll.
• FR-4 (idempotency): After the first mismatch triggers clearance, stage:Validate:complete is removed. Subsequent PRHeadSHAUpdated events for the same SHA find the label absent — the scan’s first guard (!hasLabel(item.Labels, "stage:Validate:complete")) causes an immediate skip.

Loop prevention: The SHA is recorded AFTER Claude’s final commit and AFTER the worktree HEAD is finalized (git rev-parse HEAD in the worktree). The subsequent PRHeadSHAUpdated event for Validate’s own commit arrives with SHA == ValidateCompletedSHA, so the mismatch check returns false and the scan is a no-op. The loop is architecturally closed.

Partial label-removal failure: If any REST removal fails (non-404 error), hasError is set and the function returns without applying Store mutations. stage:Validate:complete remains; the next poll re-runs the scan and retries idempotently.

Why: stage:Validate:complete validates a SHA, not an issue. A force-push, external commit, or operator rebase after Validate completes produces a new artifact that Fabrik has not validated. Without this scan, attemptMergeOnValidate’s auto-merge enablement would target the new SHA against a gate Fabrik never ran, risking auto-merge of unvalidated code. The scan closes this gap automatically for any SHA change, including ones the operator did not intend as a re-Validate signal.

2.17 Consecutive Resume Failure (Session Abandonment)

Source: Automatic — evaluated inline at the end of every Claude invocation, on both invocation paths: InvokeClaude (the primary stage run) and InvokeClaudeForComments (comment review), and transitively merge_train.go’s conflict-resolution invocation (which also goes through InvokeForComments). Unlike every other event in this section, it is not detected by a settle scan reading board/label state — it is classified structurally, in-process, from the CLI’s own result object.

Detection: interpretClaudeResult (engine/claude.go) is the single funnel all three invocation paths converge on. When an invocation fails for a reason none of the more specific classifiers already claim (stale session, turn-cap error_max_turns, usage-limit blocking_limit, transient api_error), and the failing invocation itself passed a non-empty --resume session ID, classifyResumeFailure reads a plain-text sidecar counter file colocated with the session pointer (<Stage>.session.resumefails, alongside <Stage>.session) and increments it. The counter is durable across an engine restart — deliberately a plain file, not itemstate.Store (confirmed fully in-memory, resets on restart), since every sibling counter’s resting place (SliceRetries, ReviewCycles, etc.) does not need to survive a restart but this one must. The counter is keyed purely to the session-pointer file path, which both InvokeClaude and InvokeClaudeForComments compute identically for a given (issue, stage) — so a failure from either path increments the same counter, and an alternating stage/comment-review failure sequence still reaches the threshold.

Effect (count reaches MaxResumeFailures, default 2):

1. Logs the abandonment (claudeLog, tag resume): session id, stage, consecutive-failure count, threshold, and the last error — the sole record of this event; see Guards below for why no comment or label is used.
2. Removes the session pointer file — the exact same os.Remove call the pre-existing stale-session path (§ “the errors[] substring No conversation found with session ID”) already uses.
3. Resets the sidecar counter to 0.
4. The invocation’s error is wrapped as *claudeResumeFailureError (Cause, SessionID, ConsecutiveFailures, Threshold, Abandoned: true) and returned to the caller — for every occurrence of this branch, not only the one that crosses the threshold, so a below-threshold failure is also visible to finalizeStageOutcome’s MaxRetries exemption (below).
5. No explicit “force cold start” signal is threaded anywhere: resolveResumeSessionID already treats an absent session file as a cold start identically for both InvokeClaude and InvokeClaudeForComments (logs a warning, returns ""). The file’s absence at step 2 is the cold-start signal for whichever invocation path runs next — guaranteed sequential, never concurrent, by the existing per-issue single-worker lock.

Effect (below threshold — count incremented but not yet at MaxResumeFailures): Same wrapped *claudeResumeFailureError is returned (with Abandoned: false), but the session pointer is left in place — only the sidecar counter is written. The next invocation still resumes normally; only reaching the threshold triggers abandonment.

Effect (finalizeStageOutcome, engine/item.go, stage path only): resumeFailed := errors.As(err, &resumeFailErr) is computed alongside the pre-existing turnLimited check. Unlike the usage-limit/api_error “did-not-run” family (which early-return before commitWIP/push), a resume failure does not short-circuit — a resumed session can break after real work happened, so commitWIP, the branch push, markCommentsSeenByStage, and InvocationRecorded all still run exactly as for any other incomplete run. StageAttempted is recorded unconditionally for any invocation that ran, satisfying the dispatch-cooldown requirement with no new call. In the final escalation block, resumeFailed is a third branch alongside turnLimited: it skips StageRetryIncremented entirely — for every consecutive resume failure up to MaxResumeFailures, not only the one that triggers abandonment, mirroring the fabrik:claude-limit precedent (ADR-1119). No label, no comment is applied from this branch.

Effect (engine/comments.go, comment-review path): No control-flow change. A resume failure is not excluded from checkCommentBreaker’s “no forward progress” counting the way a usage-limit hit is — it counts toward the comment-processing circuit breaker exactly like *claudeAPIErrorExit already does (ADR-1458), since the breaker is this path’s only bound and a resume failure, unlike an account-wide usage limit, is specific to this issue’s own session.

Guards:

• Reset on evidence of a healthy session: the sidecar counter resets to 0 on (a) a clean process exit (runErr == nil), (b) FABRIK_STAGE_COMPLETE found despite a trailing error, and (c) a turn-cap exit (*claudeTurnLimitError) — by construction a turn-cap exit consumed real turns and real cost, the strongest possible evidence the session is not the poisoned-session symptom this mechanism targets.
• Untouched on a did-not-run exit: a usage-limit (blocking_limit) or api_error exit neither increments nor resets the counter — the stage never ran, so the outcome says nothing about session health either way.
• Untouched on a cold start’s own failure: if the failing invocation’s own --resume session ID was empty (this call was itself a cold start), the failure is not attributable to a resumed session — the counter resets to 0 (clearing any stale leftover from a since-abandoned or since-pruned prior session lineage) and the plain, unwrapped error is returned. Normal MaxRetries accounting applies to this outcome exactly as before this mechanism existed.
• No label, no issue comment: unlike fabrik:claude-limit (which comments once per episode because the operator can act — wait, or clear the suspension), this condition is self-healing by construction: its entire purpose is that the next attempt succeeds without intervention. A comment or label here would be indistinguishable from the orphaned-durable-state leak §”fabrik:claude-limit” and ADR-1183 were written to clean up. If the guaranteed cold-start attempt also fails, that outcome has no --resume session ID attached (see the guard above) and is therefore a genuine, unexempted failure — it flows into the normal MaxRetries → stage:<name>:failed → fabrik:paused path, which already comments.
• Independent of MaxRetries, by design: every consecutive resume failure up to MaxResumeFailures is exempted from StageRetryIncremented, not only the abandoning one. If only the abandoning failure were exempted, the earlier ones would still burn MaxRetries budget and could pause the issue before the cold-start attempt this mechanism exists to produce is ever reached — reproducing, one layer up, the exact indefinite-retry bug this mechanism fixes.

Why: A --resume invocation resumes the same session every time, including when the session’s own transcript (grown too large, or otherwise structurally broken) is the cause of the failure — every retry re-triggers the identical condition. Because InvokeClaude and InvokeClaudeForComments compute the identical session-pointer path for a given (issue, stage), a transcript poisoned by a long stage run equally poisons the next comment-review invocation, and vice versa; the counter and the abandonment must therefore be shared across both paths rather than kept separately, or an alternating failure sequence between the two paths would never reach the threshold. See #1414, ADR-1414.

3. Transition Tables

3.1 Happy Path — Linear Stage Progression

This table shows the normal flow when an issue progresses through the pipeline without interruption.

shouldAdvance resolution (Path 1, handleStageComplete):

1. yoloActive = cfg.Yolo || hasYoloLabel(item) — re-fetches labels first to pick up mid-run changes
2. cruiseActive = !yoloActive && hasCruiseLabel(item) — false when yolo is active. This is not a precedence rule: it only avoids double-counting in step 3, where yoloActive already contributes the same true. Step 5 keys off the raw label, so cruise still wins at Validate.
3. shouldAdvance = yoloActive || cruiseActive
4. If stage.AutoAdvance != nil AND neither fabrik:yolo nor fabrik:cruise label is present: shouldAdvance = *stage.AutoAdvance — this means auto_advance: false in YAML overrides cfg.Yolo (the --yolo flag), but explicit yolo/cruise labels override auto_advance: false
5. If hasCruiseLabel(item) && stage.Name == "Validate": shouldAdvance = false — cruise stops at Validate. Note the raw label, not cruiseActive: this fires even when yolo is also present, which is what makes cruise win the stop-at-Validate decision.

Catch-up loop shouldAdvance resolution (Path 2): The catch-up loop first checks cfg.Yolo || hasYoloLabel(item) || hasCruiseLabel(item) — items without any of these are skipped entirely. Then: if neither yolo nor cruise LABEL is present and stage.AutoAdvance is explicitly false, the item is skipped. This produces the same behavior as Path 1.

Self-advance wake guarantee (Fix B, #544): When advanceToNextStage runs, two independent wake events fire: the new column status (LocalStatusUpdated → StatusChanged) and the worker exit (WorkerExited → WorkerLifecycleChanged). Both flags are in wakeChFlags. When wakeCh != nil (TUI mode or any wake-channel-enabled setup), the wakeChObserver fires and the dispatcher re-evaluates within milliseconds, without waiting for cooldown expiry. In headless runs (--notui or any configuration without a wake channel), re-evaluation occurs within PollSeconds via the next ticker poll. This eliminates the previous race where the departing worker was still alive when the post-advance dispatch loop ran, causing the item to receive a 150s CooldownAt("periodic-re-eval") stamp and wait the full cooldown window before the next stage was dispatched.

The same guarantee applies to the Review → Validate catch-up advance: when a review reinvoke worker exits after clearing the review gate, WorkerExited → WorkerLifecycleChanged wakes the poll loop and adds the item to mayNeedWork. The next poll’s catch-up loop sees stage:Review:complete and advances to Validate (typically within 15s). Pre-Fix B, this transition depended on external event noise — e.g., an unrelated check_run webhook for a different PR opportunistically waking the dispatcher — because CooldownAt("review-blocked") was still active from the gate-waiting period. The CooldownAt("review-blocked") retry timer (10 × PollSeconds) remains valid for non-responsive bot reviewers where no event fires, but it is no longer the primary re-admission path after the gate clears.

Validate → Done special cases:

3.2 Off-Path Transitions

Pause / Unpause

This table covers processItem’s own unpause gate — reachable only for a pause that fires before the stage is marked stage:<X>:complete (escalateFailedStage, escalatePRCreationFailure, handleBoundaryViolation, pauseForSliceLimit). A pause that fires after the stage is already complete (the four cycle-limit pause sites: review, CI-fix, rebase, enqueue — see §7.2 “Resumable Engine Pauses”) is resumed by a different mechanism, handleEngineUnpause in the catch-up loop’s Phase 1 handler chain (§6.2, Handler 0) — processItem is never dispatched for a stage-complete item with no new comment, so its gate above is structurally unreachable for those four sites. See ADR-1460.

Lock Conflict (Multi-Instance)

Dependency Blocking

Awaiting Input (FABRIK_BLOCKED_ON_INPUT)

TUI Manual Stop (s-key)

Daemon Clean-Stop Shutdown (SIGINT/SIGTERM)

Generalizes TUI Manual Stop to every issue with a live worker at once, on receipt of the first SIGINT/SIGTERM (bare-CLI Ctrl-C, or the TUI’s q/Ctrl-C quit path, which self-raises SIGTERM on its own PID — both routes converge on the same signal handler). See ADR-1393 for the full design record.

Any cancellation reaching finalizeStageOutcome’s early-out (ctx.Err() != nil — daemon shutdown, TUI stop, or any future cancellation reason) now commits and best-effort pushes any uncommitted worktree changes before releasing the lock (ADR-1393 R8) — see the TUI Manual Stop row above for what a resumed issue’s worktree looks like. This is a shared code path, not something specific to the daemon-shutdown case.

Startup recovery (ADR-1393 R7): a stage:<Name>:in_progress label that survives with neither a fabrik:locked:<any-user> label nor a stage:<Name>:complete/failed sibling — the shape a shutdown pause write’s own failure, a crash, or a force-quit can still produce despite the direct clear above — is healed by a third startup pass, runStartupBareInProgressScan (§9.7), alongside the two pre-existing conditional passes.

Awaiting Review (wait_for_reviews gate)

Awaiting CI (wait_for_ci gate)

In the conjunctive gate design (ADR 032), stage:X:complete is withheld until the CI gate actually clears. handleStageComplete adds fabrik:awaiting-ci as the durable in-flight marker; checkCIGate adds stage:X:complete once CI passes.

R4b — Convergence-paused recovery: Items paused by pauseForConvergenceFailed (convergence budget exhausted) carry fabrik:paused + stage:Validate:complete but hold no gate label (fabrik:awaiting-ci or fabrik:awaiting-review). The R4 row above admits these items via its “any gate label (or none), paused or not” guard. When their linked PR merges externally after the convergence pause, runValidatePRTerminalAdvance detects the merge, removes fabrik:paused + fabrik:awaiting-input, and advances to Done — no manual unpause required. This is distinct from the active convergence monitor path (checkAutoMergeConvergence), which is excluded from R4 by the fabrik:auto-merge-enabled guard: pauseForConvergenceFailed removes fabrik:auto-merge-enabled before pausing, so R4 correctly picks up the item on the next poll after the external merge.

Merge-conflict gate (wait_for_ci: true only; runs before the CI gate):

Cooldown Retry and Failed Stage Escalation

In-flight items and cooldown (#544): CooldownAt("periodic-re-eval") is stamped for in-flight items (those where a prior worker is still running at end-of-poll). Stamping is intentional: without it, once a prior expired cooldown ages out, the item would be re-admitted to the deep-fetch path on every poll cycle until the worker exits — causing repeated unnecessary deep-fetch evaluation work (and the fallback GraphQL fetch when the cache is invalidated or disabled) for items that can’t be dispatched anyway (snap.Worker() != nil blocks them). The prompt re-dispatch after the worker finishes is guaranteed by WorkerExited → WorkerLifecycleChanged, which is in wakeChFlags and wakes the poll loop immediately. Note: WorkerLifecycleChanged is excluded from cycleSetFlags (Fix B, issue #576), so it does not bypass the cooldown gate via mayNeedWork — but the cooldown is already expired (or was already bypassed by an entry in cycleSet from earlier in the worker’s lifecycle). See §2.14, §9.2, and §9.9.

Turn Limit Extension

The turn budget is a preemption mechanism, not a failure threshold. It exists for two reasons, neither of which is “the work went wrong”:

1. Runaway guard — bound an unbounded Claude loop.
2. Time-slicer — chop a large job so a single issue cannot starve every other issue of workers.

When the budget is exhausted, the engine commits and pushes partial work, and the next dispatch resumes the same Claude session (--resume) against the same worktree. Work continues from where it stopped; nothing is discarded and nothing restarts. A capped invocation is a slice boundary, and a job needing several slices is normal — cost typically rises across slices as each resumed session carries more accumulated context.

Treat “turn limit hit” throughout this section as this slice ended, never as this stage failed. The distinction is load-bearing: the CLI reports it structurally as subtype: "error_max_turns" / terminal_reason: "max_turns" (captured since #1178), separate from a genuine fault, and the TUI renders it as ↻ (turn limit) rather than ✗ (error).

Retry accounting matches this model (#1199). A turn-capped slice calls SliceRetryIncremented, not StageRetryIncremented — it is bounded by its own counter, SliceRetries/MaxSliceRetries (default 10), never by MaxRetries (default 3, unchanged). A job needing more slices than max_retries completes without pausing, as long as it stays within MaxSliceRetries. See §7.12 for the full detection/handling/recovery treatment, and #1191 for the operator-facing report this resolves.

When Claude exits a stage invocation due to max_turns (i.e., the per-invocation turn usage satisfies invUsage.TurnsUsed >= currentBudget and !completed && err == nil), the engine evaluates whether to extend before entering the cooldown/retry path.

Extension trigger condition: !completed && err == nil && stage.MaxTurns > 0 && invUsage.TurnsUsed >= currentBudget

Hard cap: 3× stage.MaxTurns total across all invocations. When totalMultiple >= 3, no further extension is attempted.

Per-stage progress signals:

The “baseline clean AND working tree dirty” guard for Implement prevents a pre-existing dirty worktree (e.g. from a prior interrupted session) from counting as progress. Only new uncommitted changes made during the invocation trigger extension.

Extension loop behavior (within a single processItem call — no poll-cycle gap):

1. At invocation start, a progressBaseline is snapshotted: git HEAD SHA (Implement, Review), working-tree dirty state (Implement), comment count (Validate), and resolved thread count (Review).
2. Claude is invoked with the current budget.
3. If the turn limit is hit AND totalMultiple < 3: call detectProgress. If progress → totalMultiple++, re-invoke with --resume. If no progress or progress check fails → proceed to cooldown/retry as today.
4. Output is accumulated across all invocations before posting as a single stage comment.
5. WIP commit and push are deferred to after the loop.

fabrik:extend-turns label: When present at invocation start, the first invocation receives 2 × stage.MaxTurns as its budget (pre-granted extension, no progress check required for the first turn-limit hit). That same first invocation also gets max_wall_time scaled by the identical 2× factor (scaledWallTime in engine/claude.go, §7.7, ADR-1206), so the extra turn budget has proportionate wall-clock headroom instead of being killed on a deadline sized for the un-extended case. Subsequent extensions beyond 2× still require the progress check, reset to the un-multiplied stage.MaxTurns budget, and — since each such extension re-invokes with --resume and thus gets its own fresh runClaude call — run under the unscaled max_wall_time (their own correctly-sized window starting at that call’s own spawn time, not a further-inflated one). The label persists across all intermediate stages — it is not removed on stage completion. It is removed only in the cleanup (Done) stage branch of processItem, after the stage:Done:complete label is added. ErrNotFound on removal is treated as success (user removed it manually). The label is a no-op when stage.MaxTurns == 0 (which also means no wall-time scaling occurs — scaledWallTime cannot scale against a zero baseline).

Log tag: [#N extend-turns] — emitted on every detectProgress call (pass or fail), reporting the evaluated signals and has_progress=true/false. When extension is granted, an additional line logs the new budget multiple and cumulative turns used.

Cleanup Stage

Review Reinvoke

CI Fix Reinvoke

Rebase Reinvoke

3.3 Comment Processing During Non-Pause Gate States

When a new user comment arrives while a non-pause gate label (fabrik:awaiting-ci, fabrik:awaiting-review, fabrik:rebase-needed, or fabrik:auto-merge-enabled) is active, the engine dispatches comment processing before evaluating the gate. This behavior is intentional: item.go checks for new comments (len(newComments) > 0 → processComments()) before reaching any gate check, and itemNeedsWork guard 7 admits any item with new comments regardless of gate labels.

Known limitation: If both the catch-up loop and a comment arrive at the same poll cycle before any worker is recorded, both dispatches can fire in the same cycle. The advancedItems map prevents double-advancement within a single poll cycle; the fabrik:editing label added by the comment goroutine at entry prevents a second goroutine from launching immediately. This is not structurally exclusive — it is a best-effort guard.

3.4 Operator-Level and System-Level Transitions

These transitions are driven by operator actions or background engine scans rather than stage-lifecycle events. They do not fit the “stage-running → stage-complete” grammar of §3.1/§3.2.

4. Comment Processing Lifecycle

When new comments are detected on an issue (or synthetic review comments on a PR), the engine processes them through processComments(). This is an 11-step flow.

4.1 Comment Detection

findNewComments() filters item.Comments to find unprocessed comments using four independent dedup signals:

1. In-memory CommentProcessed in itemstate.Store (session-scoped) — skip comments whose ID is recorded via snap.CommentProcessed(c.ID) (written by CommentProcessed mutation). Fast but lost on restart.
2. 🏭 **Fabrik body prefix (engine-authored output convention) — skip comments whose body starts with this header. Durable but requires the header to be present.
3. 🚀 ROCKET reaction (durable, cross-restart) — skip comments that already have a rocket reaction. Applied to user comments by processComments step 10 after processing; also applied by the engine to every comment it posts immediately after AddComment succeeds.
4. Bot service-notice pattern (content-based, isBotServiceNotice) — skip comments whose author matches github.IsBotLogin and whose body matches a known service-notice pattern from botServiceNoticePatterns (engine/comments.go), a flat, vendor-grouped list of literal case-insensitive substrings. Covers two vendors as of #1122, plus a third CodeRabbit marker added by #1141: Gemini’s quota/rate-limit/sunset/unsupported-file-type notices (e.g. “daily quota limit”, “you have reached your rate limit”, “the consumer version of gemini code assist on github has been sunset”), CodeRabbit’s rate-limit notice, matched primarily via its HTML comment marker (rate limited by coderabbit.ai — a structural, non-user-facing signal that won’t drift when CodeRabbit rewrites its banner copy) with prose fallbacks (“## review limit reached”, “you’ve reached your pr review limit, so we couldn’t start this review”) in case a future CodeRabbit variant omits the marker — the fallbacks are deliberately full-phrase rather than bare fragments, since a bare "review limit reached" substring would itself match a genuine bot review comment that quotes these very patterns (e.g. a diff review critiquing this pattern list) — and CodeRabbit’s auto-generated acknowledgement-reply marker (auto-generated reply by coderabbit, matching the HTML comment <!-- This is an auto-generated reply by CodeRabbit -->), distinct from the rate-limit marker above: it covers a content-free “acknowledged, no action taken” reply rather than a quota-exhaustion notice, and — like the rate-limit marker — is matched structurally so it holds regardless of which acknowledgement phrasing CodeRabbit uses. Unlike signals 1–3, this is a content classification rather than a watermark: it must hold on every poll’s raw fetch, so a matching comment never reaches processComments in the first place — no 👀 reaction, no worktree/Claude invocation, no reply. Because this signal excludes the comment before signal 1’s watermark would ever be recorded by the normal processComments flow, settleBotServiceNotices — an unconditional per-poll scan over the raw board snapshot, independent of dispatch state — separately applies the 🚀 reaction and CommentProcessed watermark so the comment doesn’t need to re-qualify via this pattern check on every future poll. Classification is per-comment, not per-author or per-length: a bot can post both a rate-limit notice and a genuine review on the same PR, and each comment is classified independently on its own body. Introduced to stop a runaway reply loop where a bot’s quota-exhaustion auto-reply triggered a Fabrik “not actionable” reply, which in turn re-triggered the bot (#1083/#1088); extended in #1122 once Gemini was suspended and CodeRabbit became the active review bot with its own unmatched rate-limit phrasing; extended again in #1141 to cover CodeRabbit’s acknowledgement-reply loop (see the outbound mention-neutralization invariant below — this marker was one half of a two-part fix, the other half being that the acknowledgement was also being re-triggered by Fabrik’s own reply naming the bot). Enforcement is now split across two points (#1221): this raw-fetch exclusion inside findNewComments still governs whether a worker is dispatched at all for a bot-notice-only item.Comments backlog (a notice excluded here never causes itemNeedsWork to see “new work”), while a second chokepoint inside processComments itself (§6.2) applies the same isBotServiceNotice classifier to the fully-assembled working slice just before Step 1 — covering comments merged in from item.LinkedPRReviewThreadComments and comments supplied by any of the three reinvoke dispatchers (dispatchReviewReinvoke, dispatchCIFixReinvoke, dispatchRebaseReinvoke), none of which route through this findNewComments exclusion.

Any single signal catching the comment is sufficient to skip it. The four signals are orthogonal — any combination can fail independently without triggering the self-review loop.

Dedup coverage by comment type:

• Engine-authored comments: carry signals (2) and (3) — the 🏭 **Fabrik prefix (when formatted via formatOutputComment) and a 🚀 reaction added by the engine at post time.
• User comments: carry signals (1) and (3) after processing — the CommentProcessed entry added to itemstate.Store during processComments, and the 🚀 reaction added at step 10.
• Bot service-notice comments: carry signal (4) immediately, then signals (1) and (3) once settleBotServiceNotices watermarks them on a later poll.

Invariant: every engine-emitted AddComment call must start with 🏭 **Fabrik — <context>**. This is an engine-wide convention enforced by TestAddCommentCompliance in engine/compliance_test.go, not just a detection heuristic.

Invariant (#1141): generated stage/comment output never renders a live GitHub mention of a bot login. neutralizeBotMentions (engine/mentions.go) rewrites any @<login> where github.IsBotLogin(login) is true into an inline code span — @coderabbitai becomes `@coderabbitai` — so GitHub does not notify the bot, regardless of what the surrounding prose claims about whether a reply was posted. Detection is code-span-aware (text already inside backticks is skipped), which makes the rewrite idempotent. Applied inside the four points Claude-derived freeform text is embedded into posted or persisted content: formatOutputComment, formatPRSummaryComment, formatReviewFeedbackComment (all engine/pr.go), updatePRVerification’s PR-body ## Verification rewrite (engine/pr.go — a UpdateIssueBody call outside the AddComment compliance funnel above, but equally capable of triggering a live mention), and buildAwaitingInputComment (engine/item.go). github.IsBotLogin gained a bare-literal "coderabbitai" match for this: CodeRabbit’s GitHub API login carries a [bot] suffix (already matched), but its @-mention surface is the bare form, which the suffix rule alone does not catch. Fixes the #933 incident, where Fabrik’s own “no action taken” comment-processing summary re-mentioned @coderabbitai, notifying the bot into another acknowledgement reply that Fabrik then processed again — see ADR-073.

4.2 The 11-Step Flow

4.3 Turn Limit Extension

When Claude exits a comment processing invocation due to comment_max_turns (i.e., invUsage.TurnsUsed >= currentBudget and !invCompleted && err == nil), the engine evaluates whether to extend before returning partial output.

Extension trigger condition: !invCompleted && err == nil && currentBudget > 0 && invUsage.TurnsUsed >= currentBudget

Note: currentBudget > 0 is only satisfied when fabrik:extend-turns is present (label absent → currentBudget = 0 → no extension possible). This differs from stage invocations (§3), where the progress-based extension loop fires whenever stage.MaxTurns > 0 is hit — the label only pre-grants the 2× first budget there. Comment processing is intentionally label-gated: extending comment-review turns is a new opt-in capability, and changing no-label behavior would silently extend comment budgets for all existing issues.

Hard cap: 3× commentMaxTurns(stage) total across all invocations. When totalMultiple >= 3, no further extension is attempted.

commentMaxTurns(stage): Returns CommentMaxTurns if set, else MaxTurns, else 50. This value is always > 0 (unlike stage.MaxTurns which can be 0 for unlimited).

Per-stage progress signals: Same signals as the stage invocation path — see §3 Turn Limit Extension table. For no-signal stages (Research, Specify, Plan, Done), detectProgress returns false immediately, so fabrik:extend-turns grants the 2× pre-budget for the first invocation but no further extension.

Extension loop behavior (within a single processComments call):

1. hadExtendTurnsLabel snapshotted before the loop. If present: currentBudget = 2 × commentMaxTurns(stage), totalMultiple = 2. If absent: currentBudget = 0, totalMultiple = 1.
2. snapshotBaseline called before the loop (same function as stage path).
3. InvokeForComments called with opts.MaxTurnsOverride = currentBudget. Session resume is handled internally by InvokeClaudeForComments — no loop-level session management needed.
4. If limit hit AND totalMultiple < 3: call detectProgress. If progress → totalMultiple++, currentBudget = commentMaxTurns(stage), re-invoke. If no progress or error → return partial output.
5. Output accumulated across all invocations before posting as a single comment.
6. Usage totals (tokens, turns) and InvocationRecorded store event applied once after loop completes.

fabrik:extend-turns label: When present, the first comment processing invocation receives 2 × commentMaxTurns(stage) as its budget (pre-granted, no progress check required for the first turn-limit hit). That same first invocation also gets max_wall_time scaled by the identical 2× factor (scaledWallTime in engine/claude.go, §7.7, ADR-1206) — the same mechanism as the stage-invocation path (§3.2). Subsequent extensions reset to the un-multiplied commentMaxTurns(stage) budget and, via their own fresh InvokeClaudeForComments/runClaude call, run under the unscaled deadline.

Log tag: [#N extend-turns] — same tag as stage path. Emitted on each detectProgress call. When extension is granted, an additional line logs the new budget multiple and cumulative turns used. [#N stats] emitted after loop with final accumulated usage.

4.4 Comment Processing Entry Points

Comments can trigger processing through three paths in processItem():

1. Awaiting-input unblock: isAwaitingInput(item) is true + new human comments (humanNewComments()) → unblockAwaitingInput() → processComments()
2. Paused unpause: fabrik:paused present + new human comments (humanNewComments()) → remove fabrik:paused, clearFailedStage() → fall through → processComments()
3. Normal comment processing: Item is not paused → findNewComments() finds comments (human or bot) → processComments()

In paths 1 and 2, humanNewComments() only gates whether to resume — once a human comment authorizes it, the raw (unfiltered) comment set from findNewComments() is what’s actually handed to processComments(), in the same processItem invocation. Any bot comments that accumulated while paused/awaiting-input are processed together with the resuming human comment, not deferred to a later poll (#1083) — except bot service-notice comments (§4.1 signal 4), which findNewComments() excludes from that raw set entirely regardless of pause state, since they are non-actionable either way.

All three paths ultimately source their candidate comments from findNewComments() (paths 1–2 via humanNewComments()’s wrapping, path 3 directly). But processItem()’s three paths are not the only route into processComments() — the three catch-up-loop reinvoke dispatchers (dispatchReviewReinvoke, dispatchCIFixReinvoke, dispatchRebaseReinvoke, §6.2) call it directly with comments built by buildReviewThreadComments() or a synthetic single-comment builder, neither of which goes through findNewComments(). A quota/rate-limit bot notice can therefore still reach processComments() as an entry in its working comments slice via one of these non-findNewComments() paths — the claim that it “never reaches processComments() via any path” was false, and as of #1221 that gap is closed differently: processComments() itself (§6.2) applies isBotServiceNotice as a single chokepoint to its fully-assembled working comments slice, immediately after the slice is assembled — covering the findNewComments()-sourced set (idempotently) as well as the LinkedPRReviewThreadComments merge and all three reinvoke dispatchers’ input — before any reaction, label, worktree, or invocation side effect. findNewComments() is the enforcement point for whether a worker is dispatched at all (§4.1 signal 4); processComments() is the enforcement point for whether a comment — however it arrived — is ever handed to Claude or otherwise acted upon.

4.5 markCommentsSeenByStage

After a stage invocation (not comment processing), markCommentsSeenByStage() adds ROCKET reactions to all pre-existing comments that were included in the prompt as context. This prevents those comments from triggering the awaiting-input unblock logic on subsequent polls.

4.6 Comment-Processing Circuit Breaker (#1089)

Fixes 1/4 (#1087, ADR-069) and 2/4 (#1088, ADR-070) close the two known pumps behind the #1083 incident: a bot comment can no longer silently lift an operator’s pause, and a known bot service-notice no longer spawns a worker or draws a reply. Neither fix, however, counts damage from a loop it doesn’t recognize — a new bot-notice variant, a webhook that re-triggers on Fabrik’s own reply some other way, or any other self-sustaining pattern not yet anticipated. The circuit breaker is a defense-in-depth backstop of last resort, not a replacement for fixes 1/4 or 2/4: if the same issue undergoes N comment-processing invocations within a rolling window T with no forward progress, the engine pauses the issue, posts an explanatory comment, emits a structural TUI event, and suppresses further dispatch. It should rarely if ever fire in practice.

Counter. Tracked per issue (not per stage — the #1083 incident stayed on one stage throughout, but a legitimate stage transition mid-window must not itself reset the count to a lower, easier-to-hit number) in itemstate.Store as ItemState.CommentBreaker: InvocationsAt []time.Time plus LastAuthor string (the author of the comment that triggered the most recent recorded invocation, surfaced in the trip comment). Two mutations drive it: CommentBreakerInvocationRecorded{Repo, Number, At, Author, Cutoff} (append, pruning any InvocationsAt entries older than Cutoff first when Cutoff is non-zero) and CommentBreakerReset{Repo, Number} (clear both fields; a no-op — no Change emitted — when already empty). The Store holds only raw timestamps; threshold/window business logic lives in the engine (engine/comment_breaker.go), mirroring the existing mergeTrainTrials runaway-guard precedent (ADR-059 D8). recordCommentBreakerInvocation() computes Cutoff from the current window and passes it on every append, so InvocationsAt is pruned at write time and doesn’t grow without bound for an issue that receives invocations sparser than the window; commentBreakerCount() additionally prunes-and-counts on every read against time.Now().Add(-window) as a cheap belt-and-suspenders pass that also stays correct if the configured window shrinks between writes.

Recording point. recordCommentBreakerInvocation() is called inside processComments (engine/comments.go) immediately after the working comment slice is finalized (once the item’s bot-service-notice filtering has run), before any setup side effect — the step 0 JobStartedEvent emission, the step 1 👀 reaction, the step 2 fabrik:editing label add, and step 3 worktree setup (§4.2) — and well before step 4’s InvokeForComments() call. Recording this early (#1413) means a failure in any of the three setup steps counts as a cycle, not just an invocation-time failure: each of the three setup-failure early-return paths also calls checkCommentBreaker() (see “Trip action” below) with a reason describing the failing step, so a persistently failing setup step — e.g. a fabrik:editing label add that fails on every poll under GraphQL budget pressure — trips the breaker within the configured window instead of looping unbounded (the #1382/#1386 failure mode this closes). Before #1413, the record call sat immediately before the Claude invocation, so all three setup steps could fail indefinitely without ever incrementing the counter; ADR-1089’s original rationale for that placement (“an early return before Claude runs… does not count as a wasted cycle”) is superseded by ADR-1413.

Threshold and window. Default N = 10 invocations within T = 30 minutes, both zero-means-default (mirroring MaxTrainTrialsPerWindow/TrainTrialWindowDuration): Config.MaxCommentCyclesPerWindow / Config.CommentCycleWindow, overridable via --max-comment-cycles-per-window / --comment-cycle-window flags, FABRIK_MAX_COMMENT_CYCLES_PER_WINDOW / FABRIK_COMMENT_CYCLE_WINDOW env vars, or max_comment_cycles_per_window / comment_cycle_window in config.yaml (flag > env > config.yaml > default 10 / 30m — same precedence order as every other zero-means-default engine setting).

Reset triggers (any one resets InvocationsAt to empty before the threshold is reached):

Trip action. checkCommentBreaker(item, reason string) is called from five points in processComments: each of the three setup-failure early-return paths (editing-label add, base-branch resolution, worktree setup — reason names the failing step and its error), the non-completing invocation-failure early-return path (reason == ""), and, when the invocation proceeds normally, after publishCommentOutput/finalizeComments (reason == "", so any reset those steps applied takes effect first). When commentBreakerCount() >= N, tripCommentBreaker(item, count, window, reason):

1. Applies fabrik:paused + fabrik:awaiting-input via the shared pauseIssue()/pauseOpts helper (ADR-065) — the same helper and same labels pauseForReviewCycleLimit and the merge-train runaway guard already use, so ADR-069’s honorable-pause guarantee (a subsequent bot comment cannot silently lift the pause, §1.4 fabrik:paused row) applies automatically. No new label is introduced.
2. Posts an explanatory 🏭 **Fabrik — comment-processing circuit breaker tripped** comment naming the invocation count, the window, the last comment author, and how to resume (remove fabrik:paused). When reason is non-empty (the tripping cycle was a setup failure, #1413), the comment gains an additional sentence naming the specific setup step and error that caused it — e.g. “The cycle that tripped this breaker failed during setup: the fabrik:editing label add failed: ." — so the pause is diagnosable rather than a bare "no forward progress" notice. `reason` is passed as a call parameter, not persisted in `itemstate` — it always describes the cycle that is tripping the breaker synchronously, in the same call, so no durable storage is needed (mirrors `LastAuthor`'s "only the triggering cycle's context matters" precedent).
3. Emits tui.CommentBreakerTrippedEvent{IssueNumber, Repo, Title, StageName, InvocationCount, Window, LastCommentAuthor} via emitStructural — the TUI’s active pane renders it as a blocked entry, mirroring IssueBlockedEvent (§9.8).
4. Suppresses further dispatch: once fabrik:paused + fabrik:awaiting-input are present, the item is gated by the same paused/awaiting-input machinery as every other pause (§1.4, §4.4) — only a human comment (humanNewComments()) can resume it, and per §4.4 a resuming human comment is processed together with any bot chatter that accumulated during the pause, which itself records a fresh invocation and is subject to the breaker again.

Scope note. The breaker’s suppression is a downstream consequence of the standard pause/awaiting-input labels, not a separate dispatch gate — it adds no new entry to itemNeedsWork/itemMayNeedWork (§8.1, Appendix C). This is unlike fabrik:awaiting-done or fabrik:blocked, which are engine-internal gates with their own dispatch-suppression wiring; the circuit breaker deliberately reuses the existing human-only pause semantics instead of introducing a fifth.

4.7 Success-Agnostic No-Op Comment Cycle Breaker (#1555)

handarbeit/fabrik#1254’s Validate stage redelivered the same stale, already-processed bot review body seven consecutive times — each a clean (err == nil) cycle: no commit, no issue-body update, no FABRIK_STAGE_COMPLETE. §4.6’s breaker records every invocation regardless of success, but its counter is a rolling 30-minute window, sized for #1083’s burst-loop shape (~995 invocations in rapid succession). A loop whose invocations are spaced sparser than 30 minutes — as this one was, driven by Fabrik’s own self-upgrade restart cadence (the redelivery’s actual root cause, fixed directly by §6.2’s “Durable review-ids-addressed marker”) — never accumulates enough entries within any single window to trip §4.6’s breaker, no matter how many total invocations occur over the issue’s lifetime. This section’s counter is the safety net for that redelivery, not its fix; it is a second, independent breaker that closes the windowing gap regardless of what causes a cycle to be a no-op.

Counter. StageState.NoOpCommentCycles map[string]int (internal/itemstate/itemstate.go) — per-stage, unlike §4.6’s item-scoped CommentBreaker, and never time-pruned, mirroring ReviewCycles/ReviewBlockedCycles’s shape (§6.2, ADR-1518) rather than CommentBreaker’s rolling-timestamp shape. Two mutations drive it: NoOpCommentCycleIncremented{Repo, Number, StageName} and NoOpCommentCycleReset{Repo, Number, StageName} (a no-op — no Change emitted — when already zero).

Recording point. checkNoOpCommentCycle(item, stage, progressed bool, lastAuthor string) (engine/comment_noop_breaker.go) is called from the same five mutually-exclusive exit points in processComments that call checkCommentBreaker (§4.6’s “Trip action”) — exactly one of the five executes per cycle. It is evaluated first at each site; only when it does not trip does checkCommentBreaker also run for that cycle:

if !e.checkNoOpCommentCycle(item, stage, progressed, lastCommentAuthor(comments)) {
    e.checkCommentBreaker(item, reason)
}

Both counters still record every invocation independently — NoOpCommentCycles increments even on a cycle where §4.6’s breaker is the one that ultimately trips. What’s mutually exclusive is the pause escalation: at most one fabrik:paused application and one trip comment per cycle.

progressed — R2’s three signals, computed once per cycle. progressed := headChanged || extractUpdatedBody(output) != "" || completed:

• headChanged — gitHeadSHA(workDir) compared immediately before/after runCommentExtensionLoop returns, the same comparison §4.6’s own commit-reset trigger already uses.
• extractUpdatedBody(output) != "" — the same issue-body-update signal §4.6 uses, evaluated against the caller’s own copy of output (publishCommentOutput takes output by value, so its internal marker-stripping never mutates the caller’s copy).
• completed — the same FABRIK_STAGE_COMPLETE-derived bool threaded through the rest of processComments.

The three setup-failure sites (editing-label add, base-branch resolution, worktree setup) pass progressed=false directly — no invocation ran, so none of the three signals can be true by construction.

No refund/decrement counterpart, unlike ReviewCycles. progressed=true unconditionally resets the counter to 0; there is no partial-credit decrement. A cycle that would qualify for #1045’s ReviewCycleDecremented refund in the review-gate path (a reinvoke dispatched while the gate was already clear, landing no commit) is, from this counter’s perspective, indistinguishable from any other no-progress cycle — it correctly increments here. This counter and §6.2/ADR-1045’s refund answer different questions: ADR-1045 asks “did this reinvoke change the review-gate verdict”; this counter asks “did this comment-processing cycle change anything observable on the issue.”

Threshold. Default 10, zero-means-default: Config.MaxNoOpCommentCycles, overridable via --max-no-op-comment-cycles flag, FABRIK_MAX_NO_OP_COMMENT_CYCLES env var, or max_no_op_comment_cycles in config.yaml — the same flag > env > config.yaml > default precedence as §4.6’s own threshold.

Deliberately not equal to MaxReviewCycles’s default (5). Both counters observe the same processComments funnel (see below), but only ReviewCycles is ever refunded — a shared default would make NoOpCommentCycles the one that always trips first, silently nullifying ADR-1518’s tolerate-forever guarantee at whatever count the shared value happened to be. An early version of this fix did ship both at 5 and isolated the collision out of the review-reinvoke tests with an inflated per-test override (eng.cfg.MaxNoOpCommentCycles = 100) rather than fixing the default — that hid, rather than resolved, the fact that TestHandleReviewGate_FiveNoOpReinvokes_DoNotExhaustBudget_SixthGenuineFindingAddressed’s own guarantee was false under the shipped default config. Live evidence on this repo during review (a healthy, mergeable PR accumulating 4 consecutive no-op comment-processing cycles from ordinary duplicate bot delivery) showed 5 was too tight regardless of the test-isolation question — normal operation was one round from tripping it. 10 was chosen as comfortably clear of that observed ordinary-churn ceiling while still being a hard bound. See ADR-1555’s Consequences.

Interaction with §6.2/ADR-1045’s no-op refund on the review-reinvoke path. Because checkNoOpCommentCycle runs at the same processComments funnel every dispatch source shares (§4.6), it also counts cycles dispatched via dispatchReviewReinvoke. ReviewCycles’ refund (ReviewCycleDecremented) was deliberately built to forgive an unbounded run of no-op reinvokes as long as the review gate itself keeps returning a clear verdict (ADR-1518); this counter does not forgive the same cycles, so at the default threshold, ten consecutive no-op review-reinvoke rounds now pause the issue even where ReviewCycles alone would have tolerated more. This narrows ADR-1518’s “forgive forever” guarantee to “forgive up to MaxNoOpCommentCycles consecutive rounds” in practice — deliberately, with enough headroom above both MaxReviewCycles’s own default and observed ordinary-operation churn that it should not be reached by a review bot behaving normally. An operator whose review bots need more than 10 consecutive no-op rounds before converging should raise --max-no-op-comment-cycles.

Reset triggers are narrower than §4.6’s five — no PRStateChanged reset, no new itemstate.Store observer. Only R2’s three signals above reset the counter, plus a manual unpause (below). If a future incident shows this is too narrow, that is a follow-up, not a blocking gap — this counter is a safety net, not the primary defense (the durable-marker fix in §6.2 is).

Manual unpause — from either pause route. EngineCyclesCleared’s store.go apply case deletes StageState.NoOpCommentCycles[stageName] alongside its existing deletes for ReviewCycles/ReviewBlockedCycles/CIFixCycles/RebaseCycles/EnqueueCycles. This breaker’s own trip action (below) pauses via fabrik:paused + fabrik:awaiting-input — the resume path §1.4’s table routes through unblockAwaitingInput (isAwaitingInput(item) true + a human comment), not clearFailedStage (which handles the separate stage:<name>:failed + fabrik:paused shape). Both clearFailedStage and unblockAwaitingInput apply EngineCyclesCleared and reset §4.6’s CommentBreaker, so the counter is reset regardless of which of the two pause shapes an item is resumed from. (unblockAwaitingInput gained this call as part of #1555 — before that fix it cleared retry/pause state but not the cycle counters, so NoOpCommentCycles survived an awaiting-input resume at its tripped value and could re-trip on the very next cycle; see ADR-1555.)

Trip action. tripNoOpCommentCycleBreaker reuses pauseIssue/pauseOpts exactly like §4.6’s breaker — fabrik:paused + fabrik:awaiting-input, ADR-069’s honorable-pause guarantee. The trip comment (🏭 **Fabrik — no-op comment-processing circuit breaker tripped** — a distinct header from §4.6’s 🏭 **Fabrik — comment-processing circuit breaker tripped**, so an operator always knows which mechanism fired) names the stage, the consecutive no-progress count, and the last comment’s author.

See ADR-1555, which also covers the durable review-ids-addressed marker fix (§6.2) for the redelivery this counter is a safety net for.

5. PR Lifecycle Integration

5.1 Draft PR Creation

When: After a stage signals FABRIK_STAGE_COMPLETE, if the stage has create_draft_pr: true. An early-guard path also runs before output is posted when both create_draft_pr: true and post_to_pr: true are set, so the PR exists before postOutputToPR runs.

Code path: processItem() → ensureDraftPR()

Flow (idempotent, up to 3 attempts with exponential backoff):

1. Check for an existing open PR via FetchLinkedPR() — if found open and not merged, ensure body contains Closes #N and return its number. Closed or merged PRs are ignored; a new PR will be created.
2. Push the issue branch via PushBranch()
3. Build a seed body from .fabrik-context/ files (issue summary, plan approach, verification placeholder)
4. Create draft PR via CreateDraftPR() with title from issue, targeting baseBranch, body ending with Closes #N

Transient errors (network errors, 5xx) are retried with backoff (base delay 500ms, doubled each attempt). Non-transient errors (4xx including 422) return immediately without retry. Returns (prNumber, nil) on success, (0, error) on failure.

Success logging: [#N pr] created draft PR #<num> (branch: fabrik/issue-N)

Failure logging: [#N pr] failed to create draft PR for branch fabrik/issue-N: <error>

5.5 PR Creation Failure Path

Trigger: Stage with create_draft_pr: true emits FABRIK_STAGE_COMPLETE, but ensureDraftPR returns (0, error) after exhausting its in-process retry loop.

Critical invariant: handleStageComplete is NOT called when PR creation fails. The stage does not advance. stage:<X>:complete is not added.

Retry counting: Each PR creation failure increments the same per-stage Attempts counter used by StageRetryIncremented. The StageRetryCleared mutation (which resets the counter) fires only on PR creation success, not on failure. This means PR creation failures count against MaxRetries just like Claude failures.

Code path (completion block in processItem):

FABRIK_STAGE_COMPLETE received
  → if stage.CreateDraftPR and prNumber == 0:
      prErr = ensureDraftPR(item, baseBranch)
      if prErr != nil:
          if MaxRetries > 0:
              StageRetryIncremented
              if Attempts >= MaxRetries:
                  escalatePRCreationFailure()  → fabrik:paused, stage:<X>:failed, comment
                  releaseLock()
                  return
          PRCreationFailedRecorded            → sets in-memory flag for R5
          releaseLock()
          return                               → NO handleStageComplete
      else:
          updatePRVerification(prNumber, summary)
  releaseLock()
  StageRetryCleared, EngineUnpaused
  handleStageComplete()                        → stage:X:complete, optional advance

Escalation comment (posted by escalatePRCreationFailure): Names PR creation as the failure cause (not Claude), includes the manual workaround command using the actual base branch: `gh pr create --head fabrik/issue-N --base <baseBranch> --body "Closes #N"`, and instructs the user to remove fabrik:paused to resume.

PRCreationFailed in-memory flag (R5): StageState.PRCreationFailed map[string]bool records that Claude completed a stage but the draft PR could not be created. This flag does not survive engine restarts. It is cleared by StageRetryCleared (on PR creation success). On restart, the item re-runs Claude, which is safe and conservative — Claude’s commits are idempotent.

R5 skip-Claude retry path: On the next poll cycle for an item with PRCreationFailed[stageName] == true, the engine checks this flag early in runStage (before the Claude invocation). If ensureDraftPR now succeeds, the engine calls handleStageComplete directly — no Claude re-invocation needed, since the worktree already has the commits from the prior run. If ensureDraftPR still fails, the retry counter is incremented and the escalation path applies when MaxRetries is reached. Exception: for a fabrik:children-spawned Implement item with zero commits ahead of base (an empty coordinator, §6.7.1), this branch instead routes directly to handleNoWorkNeeded before ever attempting ensureDraftPR — see §6.7.1 for why this case self-heals to Done rather than retrying a doomed PR creation.

5.2 Mark PR Ready

When: After a stage signals FABRIK_STAGE_COMPLETE, if the stage has mark_pr_ready_on_complete: true.

Code path: processItem() → markPRReady()

Flow:

1. Push the issue branch
2. Find PR number (uses knownPR from ensureDraftPR if available, else FindPRForIssue())
3. MarkPRReady() transitions draft → ready-for-review; retries up to 3 times on transient 5xx errors with exponential backoff (500ms / 1s / 2s); non-transient errors (4xx, including 429) are logged immediately without retry

Note: This triggers external review bots and populates LinkedPRReviewRequests, which is why the review gate in handleStageComplete() (Path 1) is always optimistic — reviewer data is stale at that point.

5.3 Linked PR Discovery

Fabrik discovers PR comments through the closedByPullRequestsReferences GraphQL field, which traverses issue → linked PRs → PR comments. The Closes #N keyword in the PR body creates this linkage.

MergePR’s own CI precondition (ADR-933). Before this addition, github.Client.MergePR gated only on GitHub’s mergeable field (merge-conflict status), which stays true while required status checks are still pending or failing — a gap that branch protection (enforce_admins) normally masked, but which was live and unmitigated on handarbeit/fabrik (enforce_admins: false, engine token is an org admin). MergePR now self-gates instead of relying on branch protection: after the existing mergeable check passes, it calls FetchPRMergeableFields a second time and refuses to merge — without ever calling the merge endpoint — unless the returned mergeable_state satisfies gh.MergeableStateAccepted ({clean, unstable}). A refusal returns the sentinel gh.ErrNotMergeableCI, deliberately distinct from gh.ErrNotMergeable (the conflict/dirty case) so errors.Is lets callers tell “CI not green” apart from “merge conflict” — a CI refusal must never apply fabrik:rebase-needed or consume a rebase cycle. This self-gate is deliberately unchanged by ADR-1441 (#1441, R3): the merge path continues to defer to mergeable_state alone, per ADR-072’s operator note that a check that should refuse an engine merge belongs in branch protection’s required-checks list, not in MergePR. Only the CI advance gate (checkCIGate, below) tightened — checkCIGate no longer shares this {clean, unstable} allowlist; its own fast path narrowed to mergeable_state == "clean" only (see §2.10, §6.4 rule 9).

This precondition runs identically inside MergePR regardless of caller, so all four call sites inherit it:

• attemptMergeOnValidate’s direct-merge fallback (§5.4) — the only call site with no independent mergeable_state gate upstream of MergePR (reached only when wait_for_ci: false), so this is where the precondition closes a real, previously-live gap.
• dispatchRebaseReinvoke’s already-clean-after-rebase fallback (engine/merge_gate.go, reenableAutoMergeAfterRebase) — narrows a TOCTOU window between EnablePullRequestAutoMerge returning ErrAutoMergeAlreadyClean and the direct MergePR call.
• landSingleton and landMergeTrainBatch (the merge-train landing sequence, §1.3 Queued, FR-2) — both already gated by pollForMergeable’s classifyLandingCI classification (ADR-1441), whose mergeable_state component uses the identical {clean, unstable} allowlist for its dirty-check and zero-check-runs fallback, so MergePR’s own check is normally a no-op there; it only fires in a narrow TOCTOU window between pollForMergeable’s last poll and the MergePR call.

None of the four call sites currently branch on ErrNotMergeableCI to escalate — each already logs the failure and lets the item retry on its existing cadence (a full Validate re-invocation for attemptMergeOnValidate, since that call site has no lighter-weight poll-only retry; the next poll’s convergence pass for the rebase-reinvoke fallback; the next merge-train cycle, with members left in Queued, for the landing paths). This is unchanged behavior, not new machinery — fabrik:rebase-needed is applied earlier and independently by checkMergeabilityGate from settle.Status == PRMergeConflicting, never from MergePR’s own return value, so a CI refusal was already incapable of reaching the rebase-cycle path before this change.

5.4 Auto-Merge on Validate (yolo issues)

When: Validate stage completes and the issue has fabrik:yolo (or global cfg.Yolo) and does NOT have fabrik:cruise.

Code path: handleStageComplete() → attemptMergeOnValidate() (Path 1); or catch-up loop Phase 2 → attemptMergeOnValidate() (Path 2 — when fabrik:auto-merge-enabled is absent)

Flow:

1. If fabrik:auto-merge-enabled is already present on the issue: return nil (idempotent — auto-merge was already enabled in a previous invocation).
2. Fetch linked PR via FetchLinkedPR().
3. Call EnablePullRequestAutoMerge(owner, repo, pr.Number, AutoMergeStrategy). The strategy defaults to MERGE and is configurable via FABRIK_AUTO_MERGE_STRATEGY. On failure, two branches apply:
   • ErrAutoMergeNotEnabled (repo-level setting disabled): log a guidance message pointing to Settings → General → Allow auto-merge; return a retriable error (next poll retries). No direct-merge attempt.
   • Any other error (PR in a terminal GitHub state — e.g. CLEAN, UNSTABLE, or future variants where GitHub refuses to queue auto-merge): log the specific error, then attempt a direct MergePR call as a fallback. This is the only call site with no independent mergeable_state gate upstream of MergePR (reached whenever wait_for_ci: false), so MergePR’s own CI precondition (“MergePR’s own CI precondition (ADR-933)” above) does real work here. If MergePR succeeds: apply fabrik:auto-merge-enabled and return (true, nil). If MergePR fails with ErrNotMergeable (DIRTY PR — a conflict): return (false, err) so existing rebase/CI-fix gates can act on it. If MergePR fails with ErrNotMergeableCI (a required check is blocked, or GitHub is still computing): return (false, err) as well, but this is not a conflict — fabrik:rebase-needed is never applied and no rebase cycle is consumed. Since handleStageComplete does not add stage:Validate:complete on either error, the whole Validate stage simply re-dispatches on the next poll.
4. On EnablePullRequestAutoMerge success: apply fabrik:auto-merge-enabled label. GitHub now owns the merge decision atomically — no Fabrik-side MergePR call. Done advancement is deferred to checkAutoMergeConvergence in Phase 1 of the catch-up loop.

cruise > yolo precedence: If fabrik:cruise is present on the issue, attemptMergeOnValidate returns immediately without calling EnablePullRequestAutoMerge. Cruise items keep the current stage:Validate:complete behavior: the branch is maintained (rebase reinvoke, CI-fix reinvoke) but merging is left to the user.

See section 5.5 for how Fabrik monitors the convergence flow after auto-merge is enabled.

Non-yolo issues: Items without fabrik:yolo and without fabrik:cruise follow the same behavior as cruise for purposes of merge tracking — checkMergeabilityGate and checkCIGate continue to operate using MaxRebaseCycles / MaxCiFixCycles per-gate cycle limits. EnablePullRequestAutoMerge is never called for these issues. This is unchanged from pre-5.4 behavior.

5.5 Post-Validate Convergence Monitor (yolo issues)

After fabrik:auto-merge-enabled is applied (section 5.4), every Phase 1 iteration of the catch-up loop calls checkAutoMergeConvergence() instead of the legacy checkMergeabilityGate / checkCIGate path. All merge decisions for the issue are delegated to GitHub’s server-side auto-merge logic (legacy native auto-merge) or to GitHub’s merge queue (ADR-058). handleAutoMergeConvergence calls settlePRMergeState() before forwarding to checkAutoMergeConvergence(); the settle result is consumed for merge/CI state interpretation (eliminating the split-brain), though the merge/CI gates remain bypassed — see §6.4. The function is the single mutation point for the convergence/ejection lifecycle (ADR-056 — no parallel scanner is added); the merge-queue ejection classifier (ADR-058 D4) is composed into the same branch ladder.

Convergence budget:

• The budget starts when fabrik:auto-merge-enabled is applied. The start timestamp is stored durably in GitHub’s issue event log (FetchLabelAppliedAt("fabrik:auto-merge-enabled")), so it survives engine restarts.
• The budget is configured via FABRIK_CONVERGENCE_BUDGET (Go duration syntax, e.g., 30m). Default: 30 minutes. Set to 0 to disable the budget — Fabrik waits indefinitely for auto-merge to complete (or for the user to disable auto-merge in the GitHub UI). MaxCiFixCycles is not consulted while fabrik:auto-merge-enabled is present. MaxRebaseCycles IS consulted: the convergence path’s rebase dispatch is bounded by the same MaxRebaseCycles gate used by handleMergeAndCIGates, preventing an unbounded rebase loop when a conflict is unresolvable (see RebaseCycles vs. budget below).
• On each Phase 1 iteration: elapsed = time.Since(budgetStart). If elapsed > budget, pauseForConvergenceFailed() fires.

checkAutoMergeConvergence() decision tree (ordered; first match wins):

Ejection-recovery ladder (steps ⑦–⑧) — merge-queue repos only (mergeQueueEnabled == true):

Poll-native ejection detection (the “left the queue” edge): leftQueue = priorInQueue (settle.Status != PRMergeQueued is guaranteed past step ②). priorInQueue is the item’s previous-poll LinkedPRState.IsInMergeQueue, captured in poll.go from the pre-fetch store snapshot before ItemDeepFetched overwrites the store with the current value, then threaded through phase1Ctx → handleAutoMergeConvergence → checkAutoMergeConvergence (ADR-058 D4 OQ-3). Reading “prior” from e.store inside the classifier would yield the already-overwritten current value, silently losing the edge. No webhook is required; dequeued.reason is never consulted for correctness.

reEnqueueOrPause — bounded fresh re-enqueue (FR-3):

1. Merged-first re-confirmation at the mutation point (the #913 trap): FetchPRMerged (authoritative single-PR endpoint) — if merged → advanceConvergedPRToDone; if the call errors → wait (never re-enqueue on an unconfirmed state). The REST list endpoint reports merged == false for several seconds after a queue merge, so this guard runs right before the dangerous mutation.
2. Worker-in-flight guard (snap.Worker() != nil) → skip.
3. snap.EnqueueCycles(stage) >= MaxEnqueueCycles → pauseForEnqueueCycleLimit() (queue-thrash exhaustion).
4. Otherwise EnqueuePullRequest(owner, repo, prNum, pr.HeadSHA) at the fresh head SHA (optimistic concurrency fails safe on a stale SHA; skipped if HeadSHA is empty), then apply EnqueueCycleIncremented + PREnqueueRecorded. Re-enqueue is always fresh after off-queue resolution, never re-enqueue-in-place, so conflict-heavy PRs do not starve the queue. An enqueue failure does not increment the cycle.

Rebase reinvoke + auto-merge re-enable: GitHub disables auto-merge on every push. After dispatchRebaseReinvoke() completes successfully (Claude resolved conflicts, pushed), EnablePullRequestAutoMerge is called again to re-arm auto-merge — but only on non-merge-queue repos (!item.LinkedPRIsMergeQueueEnabled, ADR-058 D4). On a merge-queue repo the recovery path is re-enqueue, not native auto-merge, so the convergence monitor re-enqueues the resolved PR once it re-derives clean (step ⑧); re-enabling native auto-merge there would fight the queue model. This is the only scenario where auto-merge is re-enabled without going through attemptMergeOnValidate.

pauseForConvergenceFailed() — convergence budget exhausted: Uses settle.PR (from the pre-fetched PRSettleResult) for PR diagnostic fields, FetchCommitsBehind for the commits-behind count, settle.CheckRuns for the CI summary, and the store for the current rebase cycle count — no independent FetchLinkedPR or FetchCheckRuns calls. Posts a structured pause comment containing:

• Total wall-clock elapsed time and configured budget
• Number of rebase reinvokes dispatched
• Commits-behind-base count
• Current mergeable_state
• Latest CI check run summary (passed / failed / pending)
• Three named user options: (1) manual rebase + re-yolo, (2) switch to cruise, (3) leave as-is

Then: applies fabrik:paused + fabrik:awaiting-input; removes fabrik:auto-merge-enabled. Does NOT add fabrik:awaiting-ci (CI may be healthy; the convergence failure is about the merge window, not CI).

RebaseCycles vs. budget: Under the convergence flow, RebaseCycles is incremented on every rebase dispatch. MaxRebaseCycles IS consulted as a gate — the same three-step guard used by handleMergeAndCIGates (in-flight guard → cycle-limit check → dispatch or pauseForRebaseCycleLimit) applies here. When FABRIK_CONVERGENCE_BUDGET=0, the time-based budget check is skipped, but MaxRebaseCycles still bounds rebase reinvokes — so an unresolvable conflict never produces an unbounded rebase loop.

Cap composition (ADR-058 D4 FR-3): Four bounded paths pause independently, first-to-trip wins, no double-pause (each pause returns immediately after applying fabrik:paused, and a paused item is skipped on the next poll):

EnqueueCycles is dedicated and independent of RebaseCycles/CIFixCycles: the sub-paths increment their own counter when they fire, while EnqueueCycles counts trips through the queue. All four counters are cleared by EngineCyclesCleared (applied by clearFailedStage on unpause/success), so a resumed issue starts fresh. LastEnqueuedSHA (on LinkedPRState) records the head SHA at the last enqueue (recorded both at the in-queue hand-off ② and at each re-enqueue ⑧), making the step-⑧ post-enqueue-window suppression SHA-precise.

State transitions for yolo Validate convergence:

5.6 Engine-Mechanized PR Creation (FABRIK_PR_CREATE Marker)

Motivation: If the Implement skill creates the draft PR directly via gh pr create, it can silently omit the Closes #N closing keyword. Without that keyword, GitHub’s closingIssuesReferences / closedByPullRequestsReferences fields return empty, breaking every downstream gate (review gate, CI gate, auto-merge). To eliminate this failure class, PR creation is mechanized: the skill emits a structured marker block, the engine creates the PR with Closes #N guaranteed as the first line.

Marker format:

FABRIK_PR_CREATE_BEGIN [owner/repo]
TITLE: <single-line PR title>

<PR body content — no closing keyword>
FABRIK_PR_CREATE_END

• BEGIN and END must each be on their own line.
• Optional owner/repo on the BEGIN line (cross-repo target; v1: only same-repo supported).
• First non-empty line inside the block must be TITLE: <title>.
• Body is everything after the title line. Must be non-empty.
• The skill MUST NOT write any closing keyword (Closes, Fixes, Resolves + #N); the engine prepends one.

Code path: processItem() → ParsePRCreateBlock() → processPRCreateMarker()

Processing (runs when stage.CreateDraftPR and a FABRIK_PR_CREATE_BEGIN/END block is found in output, before output is posted):

1. Cross-repo guard: If the BEGIN line specifies a different owner/repo, the engine pauses the issue with fabrik:paused and a “cross-repo PR creation not supported in v1” comment. Does not create a PR.
2. Idempotency: Call FetchLinkedPR() — if an open PR already exists on fabrik/issue-N, use it (record in Store, skip create).
3. Push branch: Call PushBranch() (non-fatal warning if push fails — mirrors ensureDraftPR).
4. Compose body: finalBody = "Closes #N\n\n" + block.Body. This is the mechanized guarantee.
5. Create PR: Call CreateDraftPR() with the composed body and verbatim title. Up to 3 attempts with exponential backoff for transient errors.
6. On success: Cache write-through (RecordPRLinkage), RegisterEcho("pull_request", "opened"), post acknowledgement comment "🏭 **Fabrik** — opened PR #M" on the issue.
7. On failure: Apply fabrik:paused, post error comment, return error. handleStageComplete is NOT called — the stage does not advance.

After the marker block is processed, the FABRIK_PR_CREATE_BEGIN/END content is stripped from the output string before posting the stage comment.

Malformed marker (missing TITLE, empty body, missing END, invalid owner/repo format): ParsePRCreateBlock returns an error. The engine pauses the issue with fabrik:paused and a comment naming the malformation. Does not create a PR.

Fallback to ensureDraftPR: If no FABRIK_PR_CREATE marker is found and stage.CreateDraftPR is true, the engine falls back to the legacy ensureDraftPR path (§5.1). The post-Implement linkage backstop (§5.7) runs in either case.

PRCreationFailed in-memory flag and retry path: When the marker path fails to create a PR, the same PRCreationFailed and R5 retry path as §5.5 applies.

5.7 Post-Implement Linkage Backstop

Motivation: Belt-and-braces for the case where a PR exists but lacks the Closes #N keyword — either because a legacy skill called gh pr create directly, because a gh pr edit overwrote the body, or because the engine-mechanized path (§5.6) failed and a fallback PR was opened manually.

Trigger: After the Implement stage signals FABRIK_STAGE_COMPLETE (via either the marker path or the ensureDraftPR fallback), before handleStageComplete() is called. Only runs when stage.CreateDraftPR is true.

Code path: processItem() → verifyAndHealLinkage()

Flow:

1. If prNumber == 0 (no PR exists): skip, return true (no linkage to verify).
2. Fast path (default-branch items only, !itemHasBaseLabel(item)): call FetchItemDetails(&item) (GraphQL closedByPullRequestsReferences) to get fresh item.LinkedPRNumber.
   • If item.LinkedPRNumber != 0: linkage confirmed — return true. No further calls.
3. Otherwise — item.LinkedPRNumber == 0, or the item carries a base:<branch> label and skipped the fast path entirely — delegate unconditionally to verifyAndHealLinkageByBody (see below). item.LinkedPRNumber == 0 is never itself treated as “linkage missing” — it is read only as “the fast path didn’t confirm; fall through to the authoritative body check” (#1598, R1/R3). closedByPullRequestsReferences is populated asynchronously, so a read taken within seconds of the write that should have set it (PR creation, or a heal that just edited the body) can legitimately still be empty; the old flow trusted that emptiness as definitive and paused a correctly-linked issue.
4. verifyAndHealLinkageByBody: call FetchLinkedPR() to find the PR by branch name (fabrik/issue-N).
   • If no PR found: user has diverged from the fabrik/issue-N naming convention; log warning and return true (skip heal — do not pause).
5. Call FetchPRClosingIssues() (a direct REST fetch + regex parse of the PR body for Closes/Fixes/Resolves #N — synchronous, no propagation lag).
   • If the parsed set already contains the issue number: linkage confirmed — return true. No body edit.
6. Otherwise call attemptLinkageHeal:
   • R2 guard: fetch the current body (GetIssueBody) and re-check it with gh.ParseClosingIssues before writing anything. If the keyword is already present for this issue — the caller’s own presence check (step 5, or the fast path’s item.LinkedPRNumber) raced a lagging read — log and return success without writing the body and without touching the idempotency guard. This is what stops the historical duplicate-Closes #N write (#1598, R2).
   • Body-length guard (FR-015): if len(currentBody) + len("Closes #N\n\n") > 65,300, pause with fabrik:paused + “body too long for auto-heal” comment. Return false.
   • Idempotency guard: check Snapshot.LinkageHealAttempted(stage.Name, prSHA). If true (a genuine heal — one that actually wrote the body — was already attempted once for this PR head SHA): pause with fabrik:paused + copy-paste gh pr edit recovery command. Return false.
   • Record LinkageHealAttempted{Repo, Number, StageName, PRSHA} mutation in Store (in-memory only), then heal: balanceFences(currentBody), prepend "Closes #N\n\n", call UpdateIssueBody().
7. Re-verify (only when a write happened in step 6): call FetchPRClosingIssues() again (never FetchItemDetails — re-reading the async derived field a second time is exactly the race this fix removes).
   • If the parsed set now contains the issue number: post "🏭 **Fabrik** — PR body auto-corrected: Closes #N prepended" comment, return true.
   • If still absent: call pauseForBrokenLinkage() — fabrik:paused + copy-paste recovery command, with a reason string that is now unambiguous ("auto-heal completed but PR body still lacks the closing keyword") because the body was actually checked, not inferred from a timeout (R4). This condition can now only mean the write itself didn’t take (e.g. a further concurrent body edit) — the derived-field-lag case is structurally excluded by never consulting that field here.
8. If heal returns false: handleStageComplete is NOT called — stage:Implement:complete is NOT applied.

LinkageHealAttempted in-memory flag: StageState.LinkageHealAttempted map[string]string maps stage name to PR head SHA. In-memory only — does not survive engine restart. On restart, one more heal attempt is permitted (conservative / safe). Keyed by SHA so a force-push clears the guard naturally. Not recorded when step 6’s R2 guard finds the keyword already present and skips the write — no attempt occurred, so nothing should count against the guard.

pauseForBrokenLinkage comment: Names PR number, issue number, failure reason, and provides a copy-paste gh pr edit ... --body-file ... command the user can run to manually add the closing keyword. After running the command, the user removes fabrik:paused to resume.

Non-default-base branch (base:<branch>) exception — now the shared path, not a separate implementation: GitHub only populates closingIssuesReferences / closedByPullRequestsReferences for a PR targeting the repository’s default branch — the field is structurally empty for a PR targeting any other branch, regardless of body content (#1046). Before #1598, the default-branch flow (steps 2–9 above) and the base:<branch> flow were two parallel implementations of the same heal/re-verify shape, one trusting the derived field throughout and one trusting the PR body throughout. #1598 collapsed them: verifyAndHealLinkage now only ever owns the cheap item.LinkedPRNumber fast path (step 2, skipped entirely when itemHasBaseLabel(item) is true), and both a base:<branch> item and a default-branch item whose fast path didn’t confirm linkage fall through to the same verifyAndHealLinkageByBody — the body-based confirm/heal/re-verify logic described in steps 4–7. attemptLinkageHeal (step 6) is shared unchanged by both callers. The only remaining default-branch-specific behavior is the fast path itself; everything past it is now identical for both kinds of item. A default-branch item’s log lines read verifyAndHealLinkageByBody: ... whenever the fast path doesn’t confirm — a cosmetic consequence of the shared implementation, not a functional difference.

Review-gate data feed (closed — #1050): This section’s fix addresses linkage verification only. The broader review-gate data feed (LinkedPRReviewRequests, LinkedPRReviews — see §6.1) is nested inside the same closedByPullRequestsReferences GraphQL field and remains structurally empty for base:<branch> repos. That gap is closed by a separate, additive REST-fetch branch inside checkReviewGate (§6.1) — handleBrokenReviewLinkage above now also returns the PR number it resolves, which checkReviewGate reuses to fetch reviews/requested-reviewers directly via FetchPRReviews/FetchPRReviewRequests. LinkedPRReviewThreadComments (the inline per-line comments consumed by review reinvoke) is unaffected — that field remains structurally empty for base:<branch> repos and is out of scope here.

5.8 Merge-Queue Awareness (ADR-058 D3)

When the linked PR’s repository has GitHub’s merge queue enabled, Fabrik’s preemptive rebasing and branch mutations fight the queue: the queue already enforces “up-to-date at merge time,” and any push, rebase, or base-branch change to a PR currently in the queue ejects it. Two guards make every engine-initiated git/PR mutation queue-aware, across all paths (yolo, cruise, manual). Both guards source their signal exclusively from the GraphQL-populated ProjectItem fields — never from e.client.FetchLinkedPR (the REST pulls endpoint does not carry these flags, so it always reports false). The convergence-owner conflict-dispatch site no longer reads settle.PR.IsInMergeQueue inline: D4 (§5.5) consolidates queue membership into the PRMergeQueued settle status, which intercepts an in-queue PR before the conflict branch is reached.

Signal fields:

• ProjectItem.LinkedPRIsInMergeQueue — the linked PR is currently in the merge queue (per-PR).
• ProjectItem.LinkedPRIsMergeQueueEnabled — the repository requires a merge queue (per-repo).

Guard 1 — per-PR in-queue skip (FR-1). When LinkedPRIsInMergeQueue is true, every branch/PR mutation is skipped. Fires on the in-queue signal alone, regardless of the merge_queue config kill-switch — a PR physically in the queue is queued no matter the config (it may have been queued before the operator flipped the switch). Helper: prInMergeQueue(item).

Guard 2 — per-repo stop-preemptive-rebase (FR-2). When LinkedPRIsMergeQueueEnabled is true and cfg.MergeQueue != "off", the preemptive rebase (behind-but-clean) in updateWorktreeFromMain is suppressed — the queue enforces up-to-date at merge time, so it is redundant. The merge_queue: off kill-switch restores legacy preemptive rebasing (mirrors the D2 enqueue kill-switch). Helper: (*Engine).suppressPreemptiveRebase(item).

FR-2 boundary — preemptive rebase vs. conflict resolution. Preemptive rebasing (behind-but-clean) lives exclusively in updateWorktreeFromMain, which git rebase --aborts on any conflict and never resolves it. Genuine conflict resolution lives exclusively in dispatchRebaseReinvoke, gated on settle.Status == PRMergeConflicting (⟺ mergeable_state == "dirty"). A behind-but-clean PR is "behind", never PRMergeConflicting, so suppressing the former (Guard 2) never suppresses the latter. Conflict resolution is therefore preserved and still runs when a PR is actually conflicting and not in the queue; it is skipped (via Guard 1) only when the PR is in the queue.

Guarded mutation sites (completeness — FR-1):

Non-mutating (no guard needed): engine/pr_terminal_advance.go rebase-needed handling is removeRebaseNeededLabel only (label cleanup, no git/PR mutation) and runs only after a PR is already merged/closed.

Skill-side complement (ADR-1364): the guards above cover only engine-initiated mutations. The fabrik-validate skill’s own Pre-Completion Gate rebase is skill-initiated — it runs inside the worker subprocess via gh/git, with no access to the engine’s ProjectItem snapshot — so none of the guards in the table above reach it. ADR-1364 extends the same queue-awareness principle to that site with an in-session gh api graphql check (plus an up-to-date check and a CI-freshness check comparing the base branch’s commit time against the earliest start time among the PR’s required-check runs, requiring every one of them to be currently successful — start time, not completion time, since a long-running check can finish after the base has moved again, and earliest rather than most recent, since one fresh required check does not vouch for a sibling required check that hasn’t rerun), closing a gap where CI-fix re-invocation or fabrik:revalidate could otherwise push an already-enqueued PR out of its queue.

Scope boundary. These guards cover engine-initiated mutations. Claude-driven git push/rebase inside a stage (the default tool allowlist includes Bash(git:*)) is outside engine control. Stages rarely run while a PR is in the queue (queue entry happens at Validate completion), so the realistic exposure is the rebase-reinvoke synthetic comment — which is guarded at dispatch.

WIP-preservation trade-off. Skipping the post-stage push (item.go) when the PR is in the queue means in-flight WIP is not pushed for that cycle. This is acceptable: queue entry happens at Validate completion, so a stage rarely runs while the PR is queued.

Backward-compat (FR-3). Both signal fields are false-by-default, so on a non-queue repo (LinkedPRIsMergeQueueEnabled == false, LinkedPRIsInMergeQueue == false) every guard is a no-op and all rebase/mutation behavior is byte-for-byte unchanged (the ADR-058 D1 guarantee).

6. Review Gate and Review Reinvoke

6.1 Two-Phase Review Gate

The review gate has three paths that handle different timing scenarios:

Path 1: handleStageComplete() (inside worker goroutine)

• Runs immediately after a stage completes
• Reviewer data is STALE (reviewers are assigned only after MarkPRReady, which just ran)
• Optimistically applies fabrik:awaiting-review label only when wait_for_ci: false (stages with wait_for_ci: true skip Path 1 seeding — Path 2 handles the gate after CI clears, avoiding spurious re-application of fabrik:awaiting-review from stale data; #617)
• Returns without advancing — defers to Path 2

Path 2: Catch-up loop in poll() (in poll goroutine)

• Runs on subsequent poll cycles for items with stage:<X>:complete
• Has FRESH reviewer data from FetchItemDetails() (both LinkedPRReviewRequests and LinkedPRReviews)
• LinkedPRReviewRequests is kept current via two independent paths: (1) an explicit pull_request.review_request_removed webhook fires and applyPullRequestDelta applies PRReviewRequestRemoved; (2) any pull_request_review event (action=submitted, edited, or dismissed) is received — applyPullRequestReviewDelta applies PRReviewRequestRemoved as a side effect, even when the review_request_removed side-effect webhook does not arrive (observed reliably for Copilot edited-action reviews). This means the cache can reflect the cleared reviewer list within seconds of a review arriving, without waiting for the next deep-fetch cycle.
• Calls checkReviewGate() for the real gate evaluation
• Gate clears only when len(outstanding) == 0 AND at least one non-DISMISSED review exists. This means: no requested reviewers are outstanding AND at least one review with State != "DISMISSED" has been submitted. Waiting on submitted reviews (not just requests) is what catches bot reviewers like Copilot and Gemini that self-trigger via webhooks without ever appearing in the formal requested-reviewer list. DISMISSED reviews are excluded from this check because a dismissed review indicates the reviewer’s prior response was revoked — the gate must re-block until a new non-dismissed review arrives or the reviewer re-submits. This filter is necessary because the dismissed webhook action is now processed (not silently dropped), and there is a race window between a pull_request_review.dismissed event and the subsequent pull_request.review_requested event that re-adds the reviewer to outstanding. reviewGateOutstanding(reviewRequests, reviews) is a pure function over explicit slices (not gh.ProjectItem) — checkReviewGate selects which slices to pass, per the base: branch below. This clearing condition itself is unchanged by `expected_reviewers` (below); only the *waiting* behavior that precedes it changes.
• FR-2 fast path — expected_reviewers: []. Immediately after computing outstanding/hasReviews, checkReviewGate (and, identically, reviewGateBlocksLanding in Path 3) calls the pure function reviewGateFastAdvance(outstanding, hasReviews, expected) bool, where expected is e.effectiveExpectedReviewers(item, stage) — the per-issue expected-reviewers:<mode> label override (#1304) resolved on top of stage.ExpectedReviewers; see the “Per-issue override” bullet below. It returns true — and the gate clears with (false, false, false) exactly as “cleared naturally” below — only when outstanding is empty, hasReviews is false, and the stage explicitly declared expected_reviewers: [] (see “Declared unrequested reviewers” below). A reviewer actually requested on the PR (outstanding non-empty) or an already-submitted review always takes precedence over the declaration, so this narrows waiting for unrequested reviewers only — it is not equivalent to wait_for_reviews: false, which ignores a requested reviewer outright. Placed after handleBrokenReviewLinkage/the base: REST-fallback resolution in both call sites, so it never fires on unconfirmed PR linkage. The advance is recorded via a distinct `e.logf(..., "awaiting-review", "expected_reviewers declared none expected and nothing was requested — advancing immediately\n")` line (the trail is `fabrik.log`/the TUI, not a new PR comment — every other "gate cleared naturally" path already posts nothing).
• On a default-branch item, the slices passed to reviewGateOutstanding/reviewGateAllBots are item.LinkedPRReviewRequests/item.LinkedPRReviews — the GraphQL-sourced fields, kept current via two independent paths: (1) an explicit pull_request.review_request_removed webhook fires and applyPullRequestDelta applies PRReviewRequestRemoved; (2) any pull_request_review event (action=submitted, edited, or dismissed) is received — applyPullRequestReviewDelta applies PRReviewRequestRemoved as a side effect, even when the review_request_removed side-effect webhook does not arrive (observed reliably for Copilot edited-action reviews). This means the cache can reflect the cleared reviewer list within seconds of a review arriving, without waiting for the next deep-fetch cycle.
• On a base:<branch> item (#1050): closedByPullRequestsReferences — and everything nested inside it, including reviewRequests/latestReviews — is structurally empty there (GitHub only populates it for PRs targeting the repository default branch), so item.LinkedPRReviewRequests/item.LinkedPRReviews are always empty regardless of the PR’s actual review state. When itemHasBaseLabel(item) is true and handleBrokenReviewLinkage (§5.7) has resolved a PR number > 0, checkReviewGate instead calls e.readClient.FetchPRReviews(owner, repo, prNumber) and e.readClient.FetchPRReviewRequests(owner, repo, prNumber) — REST endpoints keyed on PR number, unaffected by the PR’s base branch — and passes those slices to reviewGateOutstanding/reviewGateAllBots instead. FetchPRReviewRequests’s IsBot reproduces the GraphQL path’s dual signal (REST user.type == "Bot", with the same login-pattern fallback). If either REST call errors, the poll is treated conservatively as no-data (nil slices, gate stays blocked) rather than trusting a partial success — a warning is logged and the fetch retries on the next poll. Because item.LinkedPRNumber is always 0 on a base:<branch> repo, this PR-number resolution is not a one-time linkage-repair path — it is the steady-state lookup handleBrokenReviewLinkage performs on every poll while the gate is open. Partially out of scope, updated by #1375: LinkedPRReviewThreadComments (the inline per-line comments consumed by review reinvoke, §6.2) is unaffected by this fix and remains structurally empty for base:<branch> repos — buildReviewThreadComments has no REST fallback and this stays an accepted, pre-existing gap. buildReviewBodyComments (§6.2, Finding 4), however, now has its own REST fallback mirroring this one exactly: it resolves the PR number from item.LinkedPRNumber or, when zero, a side-effect-free FetchLinkedPR call (safe because handleReviewGate only reaches it after checkReviewGate’s own linkage check already ran this same poll without terminating), then calls FetchPRReviews directly — so a CHANGES_REQUESTED review’s body is picked up on a base:<branch> repo too, closing the gap a Pruefer review comment identified during #1375: without this, the reinvoke-before-block fix (Finding 1) would have been silently unreachable for base:<branch> stages, since buildReviewFeedbackComments would always see an empty item.LinkedPRReviews.
• ReviewRequest.IsBot is populated from requestedReviewer.__typename == "Bot" in the GraphQL query, with a login-pattern fallback (*[bot], *-bot, copilot-*, dependabot, gemini-code-assist). This drives the bot-aware escalation ladder.
• Broken-linkage guard (FR-013/FR-014): Before evaluating reviewers, checkReviewGate() checks whether item.LinkedPRNumber == 0. If so, it calls FetchLinkedPR() to look for a PR on the fabrik/issue-N branch. If a PR is found open and unmerged: on a default-branch item, the issue is paused with fabrik:paused and a comment — "🏭 **Fabrik** — PR #M exists on branch fabrik/issue-N but is not linked to this issue via a closing keyword. Add Closes #N to the PR body and remove fabrik:paused to resume." — and (false, false, true) is returned. fabrik:awaiting-review is NOT applied (FR-014). The terminated=true value (ADR-1223) signals that processing was already terminated via a direct pauseIssue call, distinct from “gate cleared naturally” — handleReviewGate checks it first and claims the item so Phase 2 does not advance it in the same poll pass. On a base:<branch> item (itemHasBaseLabel(item) true — closedByPullRequestsReferences is structurally empty there regardless of the PR body, so item.LinkedPRNumber == 0 alone does not indicate broken linkage), handleBrokenReviewLinkage first calls FetchPRClosingIssues to parse the PR body directly: if it already contains Closes #N, linkage is confirmed and the function returns (false, prNumber) so the gate falls through to normal reviewer evaluation using the REST branch above (no pause); only if the body genuinely lacks the closing keyword does it pause, with wording that explains the base-branch limitation instead of implying closingIssuesReferences alone is diagnostic. If no PR is found on the branch (either case): gate falls through to the normal reviewer evaluation path with prNumber == 0, so the REST branch above is skipped and the (empty) GraphQL-sourced slices are used — same behavior as before #1050 in that specific sub-case.
• checkReviewGate() returns (blocked, timedOut, terminated bool) (ADR-1223). Four outcomes:
  • (blocked=true, timedOut=false, terminated=false) — still waiting; fabrik:awaiting-review maintained. Either outstanding requested reviewers remain, or no reviews submitted yet (bots may still be processing). Also returned after Phase 1 (bot re-prompt fired, waiting for Phase 2 window).
  • (blocked=false, timedOut=false, terminated=false) — gate cleared naturally; fabrik:awaiting-review removed; fabrik:bot-reprompted label cleaned if present; advance or reinvoke.
  • (blocked=false, timedOut=true, terminated=false) — gate cleared by timeout; fabrik:awaiting-review removed; pauseForReviewTimeout() pauses issue. Fires at 1× ReviewWaitTimeout for mixed/pure-human outstanding (existing path) or at 2× ReviewWaitTimeout for pure-bot outstanding (Phase 2 — after the re-prompt window expired).
  • (blocked=false, timedOut=false, terminated=true) — processing already terminated via a direct pauseIssue call inside handleBrokenReviewLinkage (see broken-linkage guard above). Before ADR-1223 this branch returned the same all-false tuple as “gate cleared naturally,” so handleReviewGate did not claim the item and could fall through to buildReviewThreadComments/Phase 2, potentially dispatching a review reinvoke or advancing an item that had just been paused in the same poll pass. handleReviewGate now checks terminated first, ahead of blocked/timedOut, and claims (return true) immediately when it is set.

Path 3: reviewGateBlocksLanding() inside attemptMergeOnValidate (landing decision)

• Runs immediately before any landing action for a Validate item: auto-merge enable, merge-queue enqueue, direct merge, or advance-to-Queued. Positioned ahead of the merge_train fork, so both modes are gated identically — turning the merge train on cannot weaken the gate.
• Exists because Paths 1 and 2 leave a hole whenever wait_for_ci defers completion: within the single poll pass that clears CI, Path 2 has already skipped itself on the frozen pctx.hasComplete, and Phase 2 reaches the landing decision in that same iteration. See §6.6.6 for the full mechanism (#1216).
• Re-fetches review state live (FetchPRReviews/FetchPRReviewRequests) rather than reading item.LinkedPRReviewRequests/LinkedPRReviews, because attemptMergeOnValidate’s two callers have different freshness guarantees — and because a reviewer requested during the CI-await window must still block. PR number comes from item.LinkedPRNumber, falling back to FetchLinkedPR only when it is 0.
• Uses the same reviewGateOutstanding clearing condition as Path 2, so the two sites cannot disagree. Blocks conservatively on any fetch error — the review fetches (both slices discarded) and the FetchLinkedPR fallback alike. Only a PR that is definitively absent lets the landing through.
• Blocks and labels only — applies fabrik:awaiting-review idempotently and returns (false, nil). It never runs the escalation ladder or the timeout: once it blocks, stage:X:complete is present, so Path 2 claims the item on the next poll and owns escalation from there.

Declared unrequested reviewers (expected_reviewers, #1283). Every real-world review bot — Pruefer, Gemini, CodeRabbit, Copilot — is unrequested and asynchronous: it polls or receives a webhook and posts a review without ever being formally requested, so it never appears in ReviewRequest/outstanding. Before #1283, reviewGateAllBots(reviewRequests, outstanding) computed allBots := len(outstanding) > 0, which is unconditionally false whenever nothing was formally requested — the escalation ladder below was therefore unreachable dead code for every self-submitting bot, the exact case it exists to handle; fabrik:bot-reprompted had been applied to zero issues, ever, in production. GitHub has no representation for “a reviewer will turn up here unrequested” (requesting these bots as reviewers is not possible; CODEOWNERS requires collaborator permissions a GitHub App does not hold; inferring from history fails open on a cold-started repo) — the information exists only in the operator’s head, so it must be declared.

• Declaration (stages.Stage.ExpectedReviewers *[]string, YAML expected_reviewers). Mirrors the codebase’s *bool tri-state idiom (WaitForReviews, WaitForCI): nil (key absent) — undeclared, unchanged default behavior (below); &[]string{} (expected_reviewers: []) — explicitly no unrequested reviewer is expected, enabling the FR-2 fast path above; non-empty — one or more declared identities, satisfied when any one responds (matches the existing any-author hasReviews clearing condition — requiring all would turn N declared bots into N independent points of failure, recreating the SPOF tracked in #1071). Only meaningful alongside wait_for_reviews: true; inert otherwise, mirroring review_authority’s precedent.
• Load-time validation (validateExpectedReviewers, stages/stages.go, FR-8). Each declared identity is trimmed and must be a bare, mention-resolvable handle: non-empty, no leading @, no trailing [bot]/[Bot]/[BOT] suffix (case-insensitive). A malformed identity fails LoadAll and stage load entirely — startup fails rather than silently never notifying anyone. This rejection is not academic: Fabrik’s own first-party bot Pruefer’s actual submitted-review author is <slug>[bot] (confirmed for handarbeit/fabrik’s own installation via both GraphQL, where Bot.login omits the suffix, and REST, where user.login carries it) — the declared form must be the suffix-free one, which is also GitHub’s live @-mention surface.
• Identity matching (reviewerIdentityMatches, stripBotSuffix, declaredReviewersOutstanding, engine/reviews.go). A declared identity is matched against a live PRReview.Author by stripping a trailing [bot] from the author side, applying the existing botMentionHandle copilot-collapse to both sides, then comparing case-insensitively — tolerating both the suffixed (REST) and unsuffixed (GraphQL) forms the same login can arrive in. declaredReviewersOutstanding(declared, reviews) []string returns the declared names not yet matched to a non-DISMISSED review; this is the declared analogue of outstanding, computed independently of it (a declared identity is never a member of ReviewRequest, so it cannot be represented inside outstanding itself).
• reviewGateAllBots extended (FR-3). Signature is now reviewGateAllBots(reviewRequests []gh.ReviewRequest, outstanding, declaredOutstanding []string) bool. When outstanding is non-empty, behavior is byte-for-byte unchanged — declared reviewers never interfere with formally-requested-reviewer classification. When outstanding is empty, it now returns len(declaredOutstanding) > 0 instead of unconditional false — the one-line fix that makes the ladder below reachable for a declared self-submitting bot.
• A declaration is not evidence the reviewer exists (FR-6). A declared-but-uninstalled bot is indistinguishable, at the gate, from a slow one: it still runs the same escalation ladder and still times out, naming the bot that never appeared.
• fabrik:bot-reprompted idempotency guard is unrelaxed. The label remains the single fixed, bot-agnostic guard described below; adding the declared-reviewer re-prompt path does not introduce a second label or a per-declaration guard — this is what prevents the runaway-mention class tracked in #1083/#1088.
• Per-issue override — the expected-reviewers:<mode> label (#1304, extending #1283). expected_reviewers is a stage-YAML-only setting by default, applying repo-wide to every issue on that stage. expected-reviewers:none/expected-reviewers:declared lets a single issue opt into (or out of) a declared value without editing stage config, mirroring review-authority:<mode>’s per-issue override idiom (§6.1.1). extractExpectedReviewersOverride(issueNumber int, labels []string) *[]string (engine/reviews.go) scans item.Labels for the expected-reviewers: prefix: none resolves to &[]string{}; declared resolves to &[]string{expectedReviewersSyntheticName} ("e2e-synthetic-declared-reviewer" — a fixed testing/e2e identity that never posts a real review; applying it to a production issue runs out the full re-prompt ladder before pausing). Precedence follows extractReviewAuthorityOverride’s “pick deterministically, don’t arbitrate” convention: no label → stage config governs (nil stays nil — FR-5 default preserved); exactly one recognized label → it overrides the stage config for this issue only; both expected-reviewers:none and expected-reviewers:declared present → resolves to declared (the more restrictive mode — it imposes waiting/re-prompt-ladder obligation, unlike none’s immediate fast-advance), with a logged warning; a label whose suffix is neither exactly none nor declared (typo, casing, unknown value) → ignored with a logged warning, falls back to stage config — never a hard failure, never a silent escalation to declared. effectiveExpectedReviewers(item gh.ProjectItem, stage *stages.Stage) *[]string composes the extractor with the stage fallback and is the single resolution point checkReviewGate, reviewGateBlocksLanding, and pauseForReviewTimeout’s FR-4 status message all consult — none of the three reads stage.ExpectedReviewers directly anymore, so the advance gate, the landing gate, and the message describing declared-reviewer status can never disagree about which value applies to a given issue. The override changes nothing about whether the gate applies (wait_for_reviews: true on the stage is still required) — only which reviewers are expected once the gate is active.

Bot-aware escalation ladder:

The ladder engages whenever reviewGateAllBots is true and item.LinkedPRNumber > 0 — either every outstanding requested reviewer is a bot (detected via ReviewRequest.IsBot, unchanged from before #1283), or nothing was requested and at least one declared expected_reviewers identity has not yet responded (declaredOutstanding, new in #1283). checkReviewGate applies a two-phase escalation instead of immediately pausing:

A declared bot never preempts an authoritative human escalation (Finding 2, #1375). At the checkReviewGate call site, the ladder’s allBots is computed as reviewGateAllBots(reviewRequests, outstanding, declaredOutstanding) && authorityReason == "" — not reviewGateAllBots(...) alone. authorityReason (§6.1.1) is only ever non-empty once a requested human has already responded but with a verdict authoritative mode does not accept (outstanding is empty by the time it’s set), so this cannot change behavior for the “human hasn’t responded yet” case — outstanding non-empty already forces allBots=false via the per-reviewer loop below. It only closes the gap where a human has responded with a blocking verdict and a stage-declared bot is separately still outstanding: before this fix, reviewGateAllBots’s declared-reviewer branch (len(outstanding)==0) saw only the bot and returned true, routing the item into the bot re-prompt ladder instead of the human-escalation timeout path — deferring the human’s CHANGES_REQUESTED verdict behind a full ReviewWaitTimeout bot-reprompt cycle that cannot possibly resolve it. reviewGateAllBots’s own signature and pure-function contract are unchanged; the gate is applied at the call site, preserving ADR-1283’s “shared function, minimal call-site divergence” discipline.

• Phase 1 (fires at 1× ReviewWaitTimeout from fabrik:awaiting-review): For each outstanding requested bot reviewer: deletes then re-adds the formal review request (DELETE+POST to requested_reviewers — the delete-then-add cycle is required to re-trigger the bot’s webhook; a plain POST is a silent no-op if the reviewer is already listed), posts an @<login> just checking in comment on the linked PR, using botMentionHandle(login) for the mention surface. For each declared-but-unrequested reviewer still outstanding (skipping any name already re-prompted above via the formal-request path, so a reviewer that is both requested and declared isn’t mentioned twice): posts the same @<name> just checking in comment directly, using the declared identity verbatim (already FR-8-validated to be mention-resolvable) — no DeleteReviewRequest/AddReviewRequest call is made, since there is no GitHub review request to mutate for a reviewer that was never formally requested. This direct-mention path does not conflict with neutralizeBotMentions/#1141 (engine/mentions.go), which is scoped to Claude’s freeform stage output, not this engine-authored re-prompt — Phase 1 already posted a live mention for requested bots before #1283. After processing both groups, applies the single fixed label fabrik:bot-reprompted (idempotency guard — ≤50 chars, GitHub REST API limit) exactly once. Returns (true, false) — still blocked. Phase 1 fires once per gate cycle (idempotency enforced by presence of fabrik:bot-reprompted) regardless of which group(s) triggered it.

• Phase 2 (fires at 1× ReviewWaitTimeout from fabrik:bot-reprompted): If no bot — requested or declared — has responded after a full additional ReviewWaitTimeout window, fabrik:bot-reprompted is removed, fabrik:awaiting-review is removed, and (false, true) is returned. The caller fires pauseForReviewTimeout(), which detects Phase 2 context from the pre-cleanup item.Labels snapshot and posts a named, contextual pause comment explaining that a re-prompt was already attempted. Bot logins in the Phase 2 comment are derived from LinkedPRReviewRequests (requested bots that haven’t responded are still in that list) plus declaredOutstanding (declared-but-unrequested bots that never appear there at all) — folding both sources together is what lets the Phase 2 message flavor fire for a purely-declared reviewer, not just a formally requested one. Human-in-the-loop is preserved — the engine never auto-advances past a wait_for_reviews: true gate.

Mixed bot+human outstanding: Phase 1 does NOT fire. The gate falls through to the existing pauseForReviewTimeout() at 1× ReviewWaitTimeout from fabrik:awaiting-review, unchanged. Re-prompting humans is not appropriate (they have inbox notifications; webhooks don’t apply).

pauseForReviewTimeout() messaging (FR-4): In all timeout paths (Phase 2, mixed, pure-human), the pause comment names all requested outstanding reviewers and tags each as (bot) or (human) for easy triage, as before #1283. When the stage declares expected_reviewers (non-empty), the message additionally appends a per-declared-reviewer status line — e.g. Expected reviewers: `handarbeit-pruefer` reviewed; `gemini-code-assist` did not respond — built from live review data (resolvedReviewData, the same REST-fallback helper the authoritative-mode verdict messaging uses) via declaredReviewersOutstanding, so a partial response across multiple declared reviewers is diagnosable. This line is unconditional on which timeout branch fired (Phase 2 / mixed / generic), and replaces the old, pre-#1283 message’s generic “no reviewers were requested” framing — which was true but told the operator nothing about why nobody was requested. In Phase 2, the comment additionally notes that a re-prompt was already attempted and provides four recovery options.

ReviewWaitTimeout semantics (depends on outstanding-reviewer mix):

Label lifecycle for bot escalation:

• fabrik:bot-reprompted — single fixed label (22 chars, well under GitHub’s 50-char REST API limit); applied once after Phase 1 finishes re-prompting all outstanding bots (requested and/or declared); removed when Phase 2 fires (as part of Phase 2 cleanup), when the gate clears naturally via removeAwaitingReviewLabel (all reviewers submitted), or when the issue is closed (defensive sweep by cleanupClosedIssueTransientLabels). Only exists while the bot escalation is in progress within a gate cycle.
• fabrik:awaiting-review — applied on first block; removed on natural gate clear (including the FR-2 fast-advance path) or when Phase 2 fires; persists while the issue is paused (mixed/human/Phase-2 timeout paths — pauseForReviewTimeout does not remove it).

Startup notice for under-specified stages (stages.WarnUndeclaredReviewers, FR-7). On every startup, alongside the existing stage-drift check, Fabrik emits a one-line [startup] notice: for each wait_for_reviews: true stage with ExpectedReviewers == nil (undeclared). This is deliberately not part of drift detection (WarnStageDrift/FilterNoOpKeys), which stays silent by design here — an omitted declaration is a behaviorally-identical no-op under FR-5, so drift correctly has nothing to say about it. The notice answers a different question (“is this config under-specified?”, not “is this config outdated?”) and uses the same warnings.Record/warnings.Clear self-limiting idiom, keyed "undeclared_reviewers:" + stage.Name: informational only, never blocking, and it disappears the moment a declaration — including an explicit expected_reviewers: [] — is added for that stage.

6.1.1 review_authority: Verdict-Aware Clearing (Authoritative Mode)

Everything in §6.1 describes advisory mode — the default, and the only mode that existed before ADR-1250. Advisory clears the gate on existence: len(outstanding) == 0 && hasReviews, regardless of what any review actually says. A CHANGES_REQUESTED review does not, by itself, block advancement; a required reviewer only needs to submit, not approve. This is why advisory mode’s self-review escape hatch (§2.3) works at all: a COMMENTED review from the PR author is existence, not approval, and GitHub permits self-COMMENT while forbidding self-approval. Authoritative mode (below) layers an approval requirement on top of existence — on a repo with no reviewer and no branch-protection review requirement, a self-review still clears authoritative mode’s own-repo fallback verdict check (the “Fabrik’s own fallback” bullet below), since that check only fails on an active CHANGES_REQUESTED, not on the reviewer’s identity.

review_authority: authoritative is a per-stage YAML opt-in (stages.Stage.ReviewAuthority, {"", "advisory", "authoritative"}, default ""/advisory) that makes the same clearing branch additionally verdict-aware. It is orthogonal to Fabrik’s autonomy controls (yolo/cruise/manual) — autonomy answers “does Fabrik proceed without a human clicking go?”; authority answers “what must be true before proceeding is allowed?” See ADR-1250.

Where the check lives. reviewGateAuthorityVerdict(reviewDecision string, reviews []gh.PRReview) (satisfied bool, reason string) (engine/reviews.go) is a pure function called from inside the existing len(outstanding) == 0 && hasReviews branch of both checkReviewGate (Path 2, the advance gate) and reviewGateBlocksLanding (Path 3, the landing gate) — never as a parallel or replacement check. This is strictly additive: a PR with zero reviews stays blocked by the outer advisory condition regardless of mode, and advisory mode’s existing behavior is completely untouched (the new check is gated on effectiveReviewAuthority(item, stage) == "authoritative" and never runs otherwise — zero extra API calls, zero behavior change for advisory/default repos).

Per-issue override — the review-authority:<mode> label (#1261, extending ADR-1250). review_authority is a stage-YAML-only setting by default, applying repo-wide to every issue on that stage. review-authority:advisory/review-authority:authoritative lets a single issue opt into (or out of) authoritative mode without editing stage config, mirroring the established model:<name>/effort:<level>/base:<branch> per-issue override idiom. extractReviewAuthorityOverride(issueNumber int, labels []string) string (engine/reviews.go) scans item.Labels for the review-authority: prefix and validates the suffix against the same {"advisory", "authoritative"} value set stages.go’s validReviewAuthorities enforces at YAML-load time. Precedence follows extractEffortOverride’s “pick deterministically, don’t arbitrate” convention (rank array, prefer the higher-ranked/more-restrictive value), not extractModelOverride’s “first wins” convention: no label → stage config governs; exactly one recognized label → it overrides the stage config for this issue only; both review-authority:advisory and review-authority:authoritative present → resolves to authoritative, with a logged warning; a label whose suffix is neither exactly advisory nor authoritative (typo, casing, unknown value) → ignored with a logged warning, falls back to stage config — never a hard failure, never a silent escalation to authoritative. effectiveReviewAuthority(item gh.ProjectItem, stage *stages.Stage) string composes the extractor with the stage fallback (override, else stage.ReviewAuthority) and is the single resolution point checkReviewGate, reviewGateBlocksLanding, and pauseForReviewTimeout’s authoritative-mode pause message all consult — none of the three reads stage.ReviewAuthority directly anymore, so the advance gate, the landing gate, and the message describing why a pause happened can never disagree about which mode applies to a given issue. The override changes nothing about whether the gate applies (wait_for_reviews: true on the stage is still required for either gate to engage at all) — only the verdict-strictness once the gate is active. See ADR-1261.

Verdict source, preferred then fallback:

1. GitHub’s reviewDecision, when it reflects a real branch-protection review requirement — one of APPROVED, CHANGES_REQUESTED, REVIEW_REQUIRED (computed server-side, including CODEOWNERS-required approvals and required-approval-count rules — Fabrik does no CODEOWNERS parsing of its own). Satisfied only on APPROVED. REVIEW_REQUIRED covers both “zero reviews yet” and “some, but not enough, approvals” (e.g. a required-approval-count of 2 with only 1 approval so far) — GitHub reports the same value for both, and reviewGateAuthorityVerdict is only reached once hasReviews is already true, so in practice this branch represents the not-enough-approvals case.
2. Fabrik’s own fallback, when reviewDecision is "" (no branch-protection review requirement configured on the repo) — satisfied unless any review in the caller’s reviews slice has State == "CHANGES_REQUESTED". This exists so authoritative mode is never a silent no-op on a repo without branch protection; without it, opting in on such a repo would be indistinguishable from advisory. On the base:<branch> REST fallback path, reviews comes from FetchPRReviews (github/prs.go), which collapses each author’s full review history down to one entry — critically, a COMMENTED follow-up submission never overwrites a prior formal verdict (APPROVED/CHANGES_REQUESTED/DISMISSED) from the same author during that collapse, so a reviewer who requests changes and later leaves a comment-only follow-up still reads as an active CHANGES_REQUESTED here, matching GitHub’s own review-decision semantics (COMMENTED is informational, not a state transition).
3. Any other non-empty reviewDecision (an undocumented future GitHub enum value) blocks conservatively rather than falling through to the fallback in (2) — GitHub did report a real verdict, just not one Fabrik recognizes yet, so treating it as “no requirement configured” could wrongly satisfy the gate.

Fetching reviewDecision. FetchPRReviewDecision(owner, repo string, prNumber int) (string, error) (github/prs.go) is a GraphQL-by-PR-number query (repository(owner,name){ pullRequest(number){ reviewDecision } }), mirroring prNodeID’s shape rather than nesting inside closedByPullRequestsReferences — REST has no equivalent field, and this shape works identically for default-branch and base:<branch> items with no ProjectItem schema change, no webhook-delta freshness plumbing, and no caching (boardcache.CacheImpl.FetchPRReviewDecision always delegates, mirroring FetchPRReviews/FetchPRReviewRequests’ “review state is highly volatile” rationale). The call is made only when stage.ReviewAuthority == "authoritative" and the gate has already reached the “would otherwise clear” branch, so the extra GraphQL round-trip is paid only by stages that opt in, and only on the poll where it would matter.

Fetch failure blocks conservatively. A FetchPRReviewDecision error at either gate site does not clear the gate and does not silently fall back to the no-branch-protection computation — it is treated as unknown state, mirroring the existing FetchPRReviews/FetchPRReviewRequests failure handling in both functions. At checkReviewGate this falls through into the existing still-waiting/escalation logic with the reason "review verdict unreadable (fetch failed), blocking conservatively"; at reviewGateBlocksLanding it returns from holdLandingForReview with an equivalent message.

No PR number resolved. If checkReviewGate’s prNumber <= 0 at the point authoritative mode would consult it (linkage not yet resolved), the gate blocks with reason "review verdict unreadable — no PR number resolved" rather than skipping the check.

Non-clear → reinvoke first, pause only at MaxReviewCycles (R1, #1375, amending ADR-1250). review_authority governs merging, never working. A verdict that does not clear (most commonly an unresolved CHANGES_REQUESTED) does not make Fabrik sit idle waiting for a human — handleReviewGate (§6.2) computes buildReviewFeedbackComments() (unresolved inline thread comments plus unaddressed review bodies) before ever consuming checkReviewGate’s blocked/timedOut return, and dispatches a bounded reinvoke whenever that set is non-empty, regardless of blocked state. Per GitHub’s REST contract, a CHANGES_REQUESTED review always carries a non-empty body, so there is always something actionable to reinvoke on — the reinvoke, not a pause, is the primary response to a change request. The reinvoke loop is bounded by the pre-existing MaxReviewCycles/dispatchWithCycleLimit/pauseForReviewCycleLimit machinery (§6.2), plus a non-convergence terminal check independent of any single poll’s dedup outcome (R1/R2, ADR-1518): the cycle-limit pause fires whenever the gate is still blocked/timedOut and max(ReviewCycles, ReviewBlockedCycles) has reached MaxReviewCycles — including on a poll where the currently-outstanding feedback happens to have already been deduped as processed (snap.CommentProcessed), which is exactly the steady state immediately after the MaxReviewCycles-th reinvoke and was previously unreachable (#1518). pauseForReviewTimeout remains reserved for the genuinely different case: nothing actionable to reinvoke on at all — a plain “nobody has responded yet” block, which can only have a structurally-zero cycle count (a reinvoke can only ever fire in response to actual review content), so the two pause reasons cannot collide. See ADR-1375, which amends this ADR-1250 consequence, and ADR-1518 for the terminal-check/ReviewBlockedCycles mechanism.

Messaging distinguishes “no reviews yet” from “reviews exist but the verdict blocks.” checkAwaitingReviewTimeout takes an authorityReason parameter (non-empty only when the authoritative check blocked) and uses it verbatim as both the wait-log reason and the timeout-pause reason, instead of the generic "no reviews submitted yet" / "pending reviewers" text — which would otherwise misleadingly suggest nobody has reviewed at all. pauseForReviewTimeout’s standard (non-Phase-2) message additionally live-fetches the verdict and reuses reviewGateAuthorityVerdict itself when authoritative mode is on, appending its reason string verbatim (e.g. "no branch-protection review requirement; bob requested changes" — the reviewer’s login comes from reviewGateAuthorityVerdict’s own fallback-branch message, not a separate helper) — so this line can never disagree with why the gate is actually blocking, and covers REVIEW_REQUIRED/fetch-failure cases too, not just an active CHANGES_REQUESTED review. item.LinkedPRNumber/item.LinkedPRReviews are structurally empty for base:<branch> items (same gap as the existing pendingLine), so this resolves the PR and its reviews via the same FetchLinkedPR/FetchPRReviews REST fallback checkReviewGate/reviewGateBlocksLanding use, rather than omitting the line there.

yolo/cruise never bypass the gate — no new branching needed. Because the check lives inside the shared clearing predicate rather than at attemptMergeOnValidate/handleStageComplete’s yolo/cruise decision points, authoritative + yolo “waits” for free: attemptMergeOnValidate simply never reaches its merge/enqueue logic while reviewGateBlocksLanding returns true, exactly like an outstanding-reviewer block today. Once the verdict clears (a fix pushed → stale review dismissed on push → re-review → APPROVED, or a required approval lands), yolo merges immediately with no human click required — the timing is still automatic, only the gate has tightened. cruise composes identically at both layers: hasCruiseLabel is checked before reviewGateBlocksLanding is ever consulted (§1’s cruise-wins-over-yolo precedent — cruise is checked on the raw label, not cruiseActive — applies unchanged here), so a cruise item never even reaches the authoritative check — cruise’s “leave the PR for human merge” behavior is unaffected by review_authority.

Unit tests: TestReviewGateAuthorityVerdict (engine/reviews_test.go) is the pure-function table (APPROVED/CHANGES_REQUESTED/REVIEW_REQUIRED/empty-with-clean-reviews/empty-with-CHANGES_REQUESTED/empty-with-DISMISSED-CHANGES_REQUESTED). TestCheckReviewGate_Authoritative_* and TestCheckReviewGate_NonDefaultBase_Authoritative_* cover the advance gate across reviewDecision × branch-protection-present/absent × default-branch/base:<branch> × fetch-error. TestAttemptMergeOnValidate_Authoritative_* covers the identical matrix at the landing gate, plus TestAttemptMergeOnValidate_Authoritative_CruiseSkipsAutoMerge (cruise composition) — attemptMergeOnValidate’s own yolo-vs-advisory tests (TestAttemptMergeOnValidate_YoloEnablesAutoMerge etc.) already exercise the yolo path unconditionally, since the function has no yolo-specific branching to begin with. TestCheckReviewGate_Advisory_ChangesRequestedReview_StillClears and TestAttemptMergeOnValidate_Advisory_ChangesRequestedReview_StillMerges are explicit non-regression controls: advisory mode clears/merges regardless of a CHANGES_REQUESTED verdict, and FetchPRReviewDecision is asserted never called. For the per-issue label override (#1261): TestExtractReviewAuthorityOverride and TestEffectiveReviewAuthority (engine/reviews_test.go) cover the extractor and resolution-helper precedence matrix directly (no label, single label, both labels, malformed suffix). TestCheckReviewGate_LabelOverride_* and TestAttemptMergeOnValidate_LabelOverride_* (engine/reviews_test.go/engine/stages_test.go) re-run the label-authoritative-on-advisory-stage and label-advisory-on-authoritative-stage cases at both gate sites, plus TestAttemptMergeOnValidate_LabelOverride_Authoritative_CruiseSkipsAutoMerge/_YoloWaitsForGate confirming yolo/cruise compose with a label-resolved gate exactly as they do with a stage-YAML-resolved one.

6.2 Review Reinvoke Mechanics

The catch-up loop in poll() is split into two phases for every non-paused non-cleanup item that has either stage:<X>:complete OR fabrik:awaiting-ci (on a wait_for_ci: true stage):

Phase 1 — unconditional, ordered handler list (catchUpPhase1Handlers):

Phase 1 is implemented as an explicit ordered slice of named handler methods (catchUpPhase1Handlers in engine/catch_up_handlers.go). Each handler returns true to claim the item — Phase 2 is skipped for this item this cycle — or false to pass through to the next handler. Ordering is structurally enforced by slice position (ADR-056 D3); a future reorder requires a deliberate code change and triggers a test failure, rather than occurring as an accidental side effect of a continue insertion.

Handler 0: handleEngineUnpause (#1460 — numbered 0, not renumbering Handlers 1-4 below, to avoid an invasive cross-reference cascade) — runs first, unconditionally, ahead of every other Phase 1 handler. It is the actual resume trigger for the four cycle-limit pause sites (pauseForReviewCycleLimit, pauseForCIFixCycleLimit, pauseForRebaseCycleLimit, pauseForEnqueueCycleLimit; see §7.2 “Resumable Engine Pauses”): checks snap.PausedByEngine(stage.Name); if true — meaning the item reached this handler chain (so fabrik:paused is currently absent, per both the main catch-up loop’s and settleAwaitingCIScan’s admission filters) while still carrying a stale itemstate.EnginePaused record from before the label was removed — it calls clearFailedStage(), the same reset processItem’s own unpause gate already uses (zeroing ReviewCycles/CIFixCycles/RebaseCycles/EnqueueCycles, clearing Attempts, resetting LastAttemptAt, and resetting the comment circuit breaker). Always returns false — it only resets state, never claims — so the rest of the chain evaluates the just-reset counters in the same pass, letting a genuine resume converge in one poll rather than two. See ADR-1460 for why this handler exists at all (processItem’s pre-existing unpause gate is structurally unreachable for these four sites — they only ever fire post-stage-completion, where itemNeedsWork never re-dispatches processItem).

Handler 1: handleDependencies — thin wrapper around checkDependencies(). If the item has unresolved blocking dependencies, claims the item (returns true); otherwise passes through. checkDependencies’s own bool return is the Phase 1 claim signal directly (no translation layer, unlike the review/CI gates below) — so its cycle-detection branch (a bounded BFS finds the item is itself a transitive blocker of one of its own blockers) also returns true when it pauses the issue via pauseIssue (ADR-1223), rather than reusing the false value returned for “no open dependencies.” Before this fix the cycle-detection pause fell through unclaimed, the same overloaded-return bug present in the review/CI gates.

Handler 2: handleReviewGate — only active when stage:<X>:complete is present (pctx.hasComplete == true; skipped during the CI-await window when fabrik:awaiting-ci is present but stage:X:complete is absent — prevents spurious fabrik:awaiting-review re-application from stale board data; #617):

• checkReviewGate() runs first (side effects — label application, the bot-reprompt ladder — happen regardless of what the caller does with the return value), yielding (blocked, timedOut, terminated). terminated (a direct pause already issued by handleBrokenReviewLinkage, ADR-1223) is claimed immediately, exactly as before.
• Reinvoke-before-block (R1, #1375, amending ADR-1250): buildReviewFeedbackComments() — unresolved inline review-thread comments (buildReviewThreadComments(), unmodified) plus unaddressed review-body comments (buildReviewBodyComments(), new) — is computed unconditionally next, before blocked/timedOut are consumed. If that combined set is non-empty, the reinvoke below dispatches regardless of whether the gate is currently blocked — this is what makes the reinvoke reachable at all under review_authority: authoritative, where an unresolved CHANGES_REQUESTED would otherwise keep checkReviewGate returning blocked=true indefinitely (Finding 1). Only when this set is empty does the handler fall through to the pre-existing tail: if blocked, records CooldownRecorded{Reason: "review-blocked"} so itemMayNeedWork’s expiry path re-evaluates the item every 10 × PollSeconds, then claims; if timedOut, calls pauseForReviewTimeout() and claims.
  • buildReviewBodyComments() turns each unaddressed non-DISMISSED/non-PENDING review’s top-level body into a synthetic gh.Comment (ID: "review-body:" + PRReview.DatabaseID, DatabaseID: 0) — as of #1045, widened from CHANGES_REQUESTED-only. DISMISSED (no longer an active verdict) and PENDING (not yet submitted) are the only states skipped, plus empty bodies and DatabaseID == 0 reviews (no stable ID to dedup on). CHANGES_REQUESTED, COMMENTED, and APPROVED bodies are all now treated as potentially actionable. Superseded rationale (#1375, kept for history): the original version skipped COMMENTED/APPROVED outright, reasoning that automated reviewers like Copilot/Gemini routinely submit a COMMENTED review whose body is a generic summary, not actionable feedback, and that admitting it would trigger a reinvoke on every wait_for_reviews stage far beyond a CHANGES_REQUESTED verdict’s scope. That reasoning held only while every COMMENTED reviewer submitted generic summaries; it stopped holding once a reviewer that submits its entire finding set as COMMENTED (Pruefer, severity-gated raise-only REQUEST_CHANGES, #1251) started running on handarbeit/fabrik — the gate cleared normally on those reviews (a non-DISMISSED review from an author satisfies hasReviews), but the findings were never routed to the fixer, reproducing #1207’s “merge past unresolved feedback” failure shape through a filter instead of a race. #1045’s fix is to widen actionability rather than add an author-based discriminator (an expected_reviewers-keyed approach was proposed and withdrawn — it would have relocated #1407’s “silently drops an undeclared identity’s findings” failure into the gate) and pay the reopened cost down via the no-op cycle exemption immediately below, rather than filtering on state. reviewGateAuthorityVerdict (§6.1.1, the separate merge/landing decision) is unaffected — it still treats only CHANGES_REQUESTED as blocking; that split is unchanged by #1045. Dedup is via snap.CommentProcessed(id) only — GitHub’s REST reactions API has no endpoint for a top-level review body, so there is no ROCKET-reaction backstop the way a real thread comment gets (R7); this is what bounds the reinvoke to one dispatch per genuinely new review, not one per poll (the #1083 runaway shape). The synthetic comment’s DatabaseID: 0 routes it through the pre-existing “synthetic comment, skip reaction calls” branches in acknowledgeComments/finalizeComments automatically. On a base:<branch> item, item.LinkedPRReviews is structurally empty (§6.1’s base: note) — `buildReviewBodyComments()` REST-falls-back exactly like `checkReviewGate()` does (resolve PR number, then `FetchPRReviews`), so this reinvoke path is reachable there too. `buildReviewThreadComments()` has no equivalent fallback and stays an accepted gap for that repo shape.
  • Durable review-ids-addressed marker (R3, #1555). snap.CommentProcessed(id) above is an in-memory record — wiped by every self-upgrade restart (syscall.Exec, §4.7’s root cause discussion). Because a review body has no reaction-endpoint backstop (R7, above), that restart-wipe is a single point of failure: handarbeit/fabrik#1254 observed the same already-addressed review body redelivered seven consecutive times, each restart re-admitting it as “new.” buildReviewBodyCommentsFromReviews (engine/reviews.go) now consults a second, durable, GitHub-sourced fallback for any candidate not already snap.CommentProcessed: every review-feedback PR comment formatReviewFeedbackComment posts already lists which review bodies it addressed, so the fix embeds a machine-readable trailer — <!-- fabrik:review-ids-addressed: N,N --> — in that existing artifact. durablyAddressedReviewIDs resolves the linked PR and fetches its comments via GitHubClient.FetchIssueComments (a new interface method, deliberately not added to boardcache.ReadClient — this is a one-shot, restart-triggered lookup, not a per-poll cache-worthy read), scanning for the marker. A hit backfills snap.CommentProcessed so every subsequent call in the same process takes the fast, no-fetch path again. Lazy, not unconditional: the fetch only fires when at least one candidate isn’t already known-processed in memory — at steady state (nothing outstanding) this costs zero extra API calls; immediately after a restart it costs exactly one fetch per PR with an outstanding review body, not per poll. Accepted residual gap: the marker only exists once a review-feedback PR comment is actually posted — a cycle that lands no PR output (e.g. every candidate filtered upstream, output == "") still depends on the in-memory record alone for that narrower case; §4.7’s NoOpCommentCycles counter is the safety net for it. See ADR-1555.
  • No-op cycle exemption (#1045). handleReviewGate still increments ReviewCycles synchronously before dispatch, exactly as before — dispatchWithCycleLimit (below) is unchanged and shared with rebase/CI-fix cycle dispatch. dispatchReviewReinvoke (engine/reviews.go) additionally snapshots HEAD via gitHeadSHA(workDir) in its build() hook, before processComments() runs, and compares it again in a new after hook once processComments() returns. When the two match — and only when the “before” snapshot was itself readable (headBefore != "") — it applies itemstate.ReviewCycleDecremented (floored at 0), netting that dispatch back to unchanged. This mirrors dispatchCIFixReinvoke’s existing gitHeadSHA before/after pattern (below), with one deliberate divergence: CI-fix’s no-op record only short-circuits future dispatches for the same head SHA, it does not stop the counter incrementing for the no-op dispatch itself; the review-cycle exemption prevents the increment from sticking at all. The check is scoped to “no new commit” only, uniformly — it does not distinguish “Claude ran and did nothing” from “the batch was filtered to empty before Claude ran” (the #1221 chokepoint below). It does not reach the #1221 case: processComments()’s notice filter runs before wm.EnsureWorktree, so when every candidate comment is filtered out, the worktree is never touched that cycle and gitHeadSHA fails identically in build() and after() — there is no HEAD to compare, headBefore == "", and no decrement fires. TestCatchUpLoop_ReviewReinvoke_AllNoticeThread_NoInvocation’s ReviewCycles == 1 assertion is therefore unchanged by #1045 — see the #1221 paragraph below. See ADR-1045.
  • ReviewBlockedCycles — the never-refunded non-convergence counter (R3, #1518, ADR-1518). #1045’s refund above is deliberately unconditional for its own scenario — five, or an unbounded number of, consecutive no-op reinvokes on non-blocking feedback (advisory-mode COMMENTED junk overviews that never held up the gate at all) must keep being forgiven forever (TestHandleReviewGate_FiveNoOpReinvokes_DoNotExhaustBudget_SixthGenuineFindingAddressed). But that same unconditional refund can also mask a genuinely non-converging loop: if every reinvoke against an unresolved authoritative verdict happens to be a no-op on HEAD, ReviewCycles never accumulates past 1, and the cycle-limit check below never fires. The two scenarios are structurally distinguishable at dispatch time — checkReviewGate’s own blocked/timedOut return (computed before buildReviewFeedbackComments() is even built) is true only when the gate itself is still failing to clear. handleReviewGate therefore also applies itemstate.ReviewBlockedCycleIncremented — alongside the existing pre-dispatch ReviewCycleIncremented, never in place of it — exactly when a reinvoke is about to be dispatched and blocked || timedOut is true at that moment. It has no decrement counterpart: a no-op reinvoke dispatched while the gate was genuinely blocked is non-convergence evidence regardless of whether it happened to land a commit. A reinvoke dispatched while !blocked && !timedOut (#1045’s own shape) never increments this counter, so that refund stays unconditional exactly as #1045 requires.
  • Terminal check independent of this poll’s dedup outcome (R1/R2, #1518, ADR-1518). Both the cycle-limit check below (inside the buildReviewFeedbackComments() > 0 branch) and a new check in the pre-existing blocked/timedOut fallback tail now compare against max(snap.ReviewCycles(stage.Name), snap.ReviewBlockedCycles(stage.Name)), not ReviewCycles alone — closing the gap ReviewBlockedCycles exists to close. The fallback-tail check fires whenever blocked || timedOut is true and that max has already reached MaxReviewCycles, calling pauseForReviewCycleLimit() directly and claiming the item before the tail’s own CooldownRecorded/pauseForReviewTimeout branches are reached — so a gate that is still failing to clear cannot fall through to the timeout pause merely because the currently-outstanding feedback happens to already be deduped as processed on this particular poll. Gated on blocked || timedOut, so the naturally-cleared case (buildReviewFeedbackComments() empty and the gate itself cleared) is untouched; a genuine “nobody has ever reviewed” block can only have a structurally-zero cycle count (no reinvoke has ever had content to fire on), so it can never satisfy the >= comparison and R4 (pauseForReviewTimeout for that case) holds without an explicit special case.
• Yolo-merge guard 2 (#1207): if the item carries fabrik:auto-merge-enabled and currentHeadReviewFeedbackComments() (current-head thread comments, via currentHeadReviewThreadComments()’s existing isOutdated filtering, plus all review-body comments — a review body has no thread and therefore no isOutdated signal; every body comment is treated as always-current-head, an explicit, documented R8 simplification, not an oversight) is non-empty, disableAutoMergeForReviewThreads() runs here — before the worker/cycle-limit checks below, on the same poll. This has to live inside Handler 2 rather than inside Handler 3 (handleAutoMergeConvergence/checkAutoMergeConvergence): Handler 2 always claims the item and stops the Phase 1 chain whenever buildReviewFeedbackComments() is non-empty, so a check placed only in Handler 3 would never run in that same cycle. See §2.9 for the full guard-1/guard-2 design.
• Worker guard (snap.Worker() != nil): if a reinvoke goroutine from a previous poll cycle is still running, claims the item without dispatching or incrementing the cycle count.
• Cycle limit check (max(snap.ReviewCycles(stage.Name), snap.ReviewBlockedCycles(stage.Name)) vs MaxReviewCycles, default 5 — the max, not ReviewCycles alone, is #1518/ADR-1518’s fix for the refund-masking gap above): if exceeded, pauseForReviewCycleLimit() adds fabrik:paused + fabrik:awaiting-input and claims — this is the terminal fallback (R5) for a verdict that never converges, reached whether the gate cleared naturally or is still authoritatively blocked. If not exceeded: increment count via ReviewCycleIncremented (plus ReviewBlockedCycleIncremented when blocked || timedOut at that moment, per the bullet above), dispatch dispatchReviewReinvoke() (applies WorkerEntered, acquires semaphore, calls processComments() asynchronously with the combined thread+body comment set), set advancedItems[key] = true, claim.
• isReviewReinvoke() (engine/comments.go, consulted by publishCommentOutput when the reinvoke’s processComments() call completes) recognizes a comment as review-reinvoke-eligible when ReviewThreadID != "" or its ID carries the "review-body:" prefix — so a reinvoke batch that is mixed (thread comments + a review body) or consists solely of review bodies still posts the PR feedback summary comment, not just a pure-thread-comment batch.

Handler 3: handleAutoMergeConvergence — if the item does not have fabrik:auto-merge-enabled, passes through. If it does: calls settlePRMergeState() to build a PRSettleResult, then forwards to checkAutoMergeConvergence() which consumes the settle result for PRMergeUnsettled/PRMergeConflicting branch decisions. Claims the item (returns true). The merge/CI gates remain bypassed — GitHub owns the merge decision for these items — but checkAutoMergeConvergence() no longer independently interprets mergeable_state or calls FetchPRMergeableFields/FetchCheckRuns.