Code review of 1185 (merge 0f4b0c95) found the retry subscriber's central
invariant did not hold: the MergeFailure->Merge bounce caused by its own
retry reset both its attempt counter and the block subscriber's counter,
so the shared merge_failure_block_threshold budget was unreachable and a
deterministic gates failure retried forever.
One subscriber now owns one counter driving both policies:
- Counter survives PipelineEvent::MergeRetryStarted bounces (finding 1);
a third consecutive failure blocks even with retries in between.
- Mixed failure kinds share the single budget (finding 5).
- Retries respect recovery: no counting or scheduling while a mergemaster
is active, and perform_auto_retry re-checks before firing (finding 2).
- perform_auto_retry applies the same eligibility gates as
assign_merge_stage (review hold, frozen, blocked, unmet deps) so freeze
now stops a retry loop (finding 4).
- Per-story scheduling generations invalidate stale sleeping timers
(finding 6).
- One-shot startup scan schedules a catch-up retry for stories already
parked in GatesFailed, so restarts no longer strand them (finding 3);
kept out of the periodic reconciler to avoid re-retrying exhausted
stories every tick.
- Chat is notified only after the merge actually starts; a failed trigger
logs instead of claiming a retry ran (finding 7).
- Config reads moved onto spawn_blocking (finding 8, bug 1170 class).
Deletes merge_failure_retry_subscriber.rs; notification plumbing
(WatcherEvent::MergeAutoRetry et al) is unchanged.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019fHdm92yjvguPi2LiXfLB9
The test asserted msg_count == 0 on a process-global broadcast channel
(TRANSITION_TX is a single OnceLock<Sender> shared across the test
binary), so any concurrent test calling apply_transition could land
events in our receiver between the drain and the post-reconcile check.
Observed failure: 3 stray transitions from parallel tests.
Drop the strict count check. The real "never floods" invariant is
captured by the Lagged check alone: 1000 seeded items must not overflow
the 256-slot channel, which can only hold if the reconcile path
bypasses the broadcast (AC4). The sibling test
`reconcile_pass_scales_to_1000_items_without_lagged_divergence` already
uses this Lagged-only pattern.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Merge 1061 added a replay_current_pipeline_state() call to the broadcast::Lagged
branch, but replay broadcasts one event per CRDT item (~997) into a 256-slot
channel, deterministically re-overflowing it and triggering another Lagged. The
loop pinned CPU and likely caused today's machine crash. Revert to the pre-1061
behaviour of logging and continuing.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1018's merge_failure_block_subscriber counted every MergeFailure transition
toward the 3-strike block threshold, but mergemaster's recovery iterations
(squash → fail → fix → retry) emit multiple MergeFailure transitions while
making real progress. Story 997 was blocked at 10:59:46 while mergemaster
was still resolving conflicts and would have succeeded a minute later.
Fix: pass the AgentPool to the subscriber. When a mergemaster agent is in
the pool for the story, MergeFailure transitions are recovery iterations
in progress and do NOT increment the consecutive-failure counter. Block
only fires for the genuinely-stuck case (no recovery agent attached and N
consecutive failures accumulate).
Tests:
- mergemaster_running_suppresses_block: 3 failures with recovery_running=true
→ counter stays empty, story stays in MergeFailure
- no_mergemaster_still_blocks_at_threshold: 3 failures with recovery_running=false
→ blocks (1018 behaviour preserved)
All 2938 tests pass.
Huskies Agent
TimmyandClaude Fable 5