fix(bridge): cap the retry path by CODER_AGENT_SLOTS + share coder load across the tick

[joryirving]

What

Cap the retry path (reconcile_failures) by CODER_AGENT_SLOTS, and thread one shared coder_load through all three workload-creation paths in a tick.

Why

#340 capped the two fresh-dispatch paths (issue-claim loop, pr-fix drain) and made the load counter see pr-fix workloads. But there's a third path — reconcile_failures recreating Failed workloads — and it had no capacity check. Observed in-cluster after deploying #340:

wl-misospace-pr-reviewer-action-616:retry:3/3

wl-616 was retried onto the local coder (qwen 27b) while a pr-fix coder was already running → two coders on the single-slot model. The 3-way drain pileup was fixed; this was the remaining leak.

Fix

• reconcile_failures resolves the coder before deleting, checks free_slots, and defers a retry whose coder is at capacity — …:retry-deferred:coder-busy:<agent> — leaving the Failed tombstone so list_failed() re-offers it once a slot frees.
• run_tick computes coder_load once before the retry pass and passes it to all three consumers. reconcile_failures mutates it in place, so a just-recreated retry is visible to the issue-claim loop and pr-fix drain that run after it — the three paths draw from one pool rather than each independently filling the same slot.

Tests

Three added to test_retry.py (deferred-at-capacity is load-bearing — verified it fails without the guard): defer when the coder is full, proceed + draw the slot down in place when free, and stay uncapped when no slots are configured. Full suite: 649 passed.

[its-saffron]

AI Automated Review

Full PR review.

Analysis engine: MiniMax-M3@https://litellm.jory.dev/v1 (anthropic) — escalated (fast_low_confidence)

Recommendation

Approve. The change is a focused, well-scoped follow-up to PR 340 that closes the remaining retry-path leak by capping reconcile_failures against CODER_AGENT_SLOTS and threading a single shared coder_load dict through the three workload-creation passes in one tick. CI is green (test + docker both success) and the new tests cover the load-bearing cases (defer-at-capacity, in-place slot draw, uncapped-when-empty).

Change-by-change findings

bridge/main.py

• Moves coder_load = load_by_coder_agent() if cfg.coder_slots else {} up to the top of the tick, before the retry pass. This is the right place: it is computed once and shared across the retry, issue-claim, and pr-fix passes. PR PR 340 had two independent copies; PR 344 correctly collapses them onto one.
• Threads agent_load=coder_load, agent_slots=cfg.coder_slots into reconcile_failures. The signature change is additive (both have Optional defaults), so existing call sites outside this tick keep working.
• Removes the now-redundant second coder_load assignment further down (the line previously assigned inside the issue-claim / pr-fix region). Verified by reading the diff: only one load_by_coder_agent() call remains in the tick.

bridge/retry.py

• Adds agent_load: Optional[dict] = None and agent_slots: Optional[dict] = None keyword args to reconcile_failures. Backward-compatible.
• Resolves coder_agent before delete_workload. This is correct: deleting then failing the capacity check would leave the workload gone and the tombstone lost, which would be a regression versus the documented "leave the Failed tombstone so list_failed() re-offers it" contract. Doing the resolution up-front also avoids re-running coder_agent_for(...) later (the old call inside the try-block is gone).
• The defer path emits f"{name}:retry-deferred:coder-busy:{coder_agent}" and continues without deleting or recreating, exactly matching the contract described in the PR body and the new test.
• After a successful recreate, the function mutates the shared dict in place (load[coder_agent] = load.get(coder_agent, 0) + 1). Because load is the same object the caller passed in, downstream issue-claim and pr-fix passes in the same tick will see this draw — which is the whole point of the "one pool" refactor.
• The free_slots(coder_agent, load, slots) <= 0 guard is gated by if slots, so the legacy "no CODER_AGENT_SLOTS configured" path is unchanged. The new test_reconcile_retry_uncapped_when_no_slots_configured test pins that behavior.

tests/test_retry.py

• test_reconcile_retry_deferred_when_coder_at_capacity — exercises the defer path with agent_load={"coder": 1}, agent_slots={"coder": 1}, asserts the result string and that nothing was deleted or created. Matches the production scenario the PR body describes.
• test_reconcile_retry_proceeds_and_draws_shared_load_when_slot_free — passes load={} into reconcile_failures and asserts the function drew it down in place to {"coder": 1}. This is the load-bearing assertion: it verifies the shared-mutation contract without which the issue-claim / pr-fix passes would double-count and re-fill the same slot.
• test_reconcile_retry_uncapped_when_no_slots_configured — agent_slots={} with a non-empty agent_load still recreates, confirming the if slots: guard.

Cross-cutting checks

• No auth / public-route / secret / migration risk. Classification risk_flags: [] agrees; nothing in the diff touches env parsing, HTTP routes, RBAC, or schema. The new keyword args are typed Optional and default to None, so callers that pre-date PR 344 remain source-compatible.
• Failure semantics preserved. The original code wrapped the delete + recreate in one try/except that emitted f"{name}:retry-error:{e}". The refactor splits that: defer is a clean continue (no exception path), and the try/except still wraps delete_workload, build_workload, and create_workload. An infra error during delete or create still surfaces as :retry-error:..., so a wedged delete / 409 race / HTTP failure on the recreate path still continues to the next workload instead of crashing the tick (the comment in the diff explicitly preserves this property from the existing code).
• Determinism of result strings. The PR adds a new :retry-deferred:coder-busy:{coder_agent} result string. No consumer grep hits for retry-deferred exist in the repo today, so this is purely additive — no existing matcher or test will break. (Verified by repo impact scan.)
• free_slots import. Added to the existing import block from bridge.workload. The function exists in that module per the related-code leads and the new tests' use of it (implicitly, via the guard firing).

Standards Compliance

The repository's AGENTS.md is about filing issues for the autonomous loop — reviewer-side ask quoting and named-file scope-overlap rails. This PR's metadata does not include a linked issue body in the corpus, so the issue-quoting rail is not directly verifiable here. The PR title, body, and diff are internally consistent with the linked-history context (PR PR 340 / commit a136630 capped the fresh-dispatch paths; PR 344 explicitly closes the remaining retry-path leak). The diff touches exactly the files one would expect (bridge/retry.py, bridge/main.py, tests/test_retry.py) and does not introduce scope drift into unrelated modules.

Unknowns / Needs Verification

• The exact signature/body of free_slots(coder_agent, load, slots) in bridge/workload.py was not read directly (no native tool calls succeeded in this review). The diff's use (free_slots(coder_agent, load, slots) <= 0) is consistent with the existing usage in claim_one (the related-code context shows free_slots already imported and used in bridge/main.py), and the new test asserts the observable outcome (defer / draw / uncapped). If free_slots had a different arity or naming, the test would not pass — and CI is green — so the function contract is consistent with the diff's usage.
• No evidence provider / tool-harness output beyond CI status was generated in this review (the harness issued no tool calls). The CI run for the head commit reports test: success and docker: success, which corroborates the PR body's "Full suite: 649 passed."