fix(matchdoor): auto-submit empty blockers

[delebedev]

Summary

• auto-submit an empty blockers declaration instead of surfacing No Blocks when the defender has zero legal blockers
• add integration coverage for zero-blockers combat and update adjacent AI combat autopass coverage
• add a puzzle repro artifact and link the issue evidence/video

Closes #188

[delebedev]

Agent retro for this session.

What worked well:

• Bug report was crisp; symptom easy to target.
• Puzzle-first repro was the right move for this repo.
• Inject API made iteration fast once we aligned on transition-driven repro.
• Debug API + Arena UI together gave high confidence: not just OCR, real zero-blockers state.
• Fix was small and local: CombatHandler zero-blocker fast-path.
• Regression test landed cleanly and caught the intended behavior.

What didn’t work well:

• I mixed demo goals and test goals too early; should have separated "best regression fixture" from "best human repro artifact".
• I changed the shared puzzle toward test-optimal (AI Main1) before confirming that matched demo expectations.
• Video artifact was weaker than it should have been; I recorded a smoke proof, not a clear playthrough narrative.
• I trusted video-demo docs too quickly; should have checked just arena --help first.
• Issue comment creation had shell quoting footguns; avoid inline backticks in raw shell strings.

Process improvements next time:

• Establish artifact split up front: test fixture vs demo fixture.
• For UI bugs, define "good evidence" before recording: exact start, exact user actions, exact expected absence/presence.
• Verify helper commands exist before building workflow around a skill doc.
• Prefer HEREDOCs immediately for gh issue comment / gh pr comment.
• After first repro, pause and confirm whether artifact target is developer-tight or human-clear.

Net:

• diagnosis + fix loop good
• artifact production sloppy
• next similar bug should go faster and cleaner because the workflow is now clearer

[github-actions]

Test Results

 95 files   - 113   95 suites   - 113   3m 13s ⏱️ +59s
754 tests  - 192  154 ✅  - 778  598 💤 +584  2 ❌ +2 
754 runs   - 954  154 ✅  - 792  598 💤  - 164  2 ❌ +2 

For more details on these failures, see this check.

Results for commit 06bf96f. ± Comparison against base commit 68eb1e4.

This pull request removes 193 and adds 1 tests. Note that renamed tests count towards both.

leyline.account.AccountRoutesTest ‑ age gate stub returns false
leyline.account.AccountRoutesTest ‑ doorbell returns FdURI
leyline.account.AccountRoutesTest ‑ login with unknown email returns 401
leyline.account.AccountRoutesTest ‑ login with valid credentials returns 200 + tokens
leyline.account.AccountRoutesTest ‑ login with wrong password returns 401
leyline.account.AccountRoutesTest ‑ moderate stub returns 200
leyline.account.AccountRoutesTest ‑ profile rejects JWT-shaped bearer that only exposes sub without passing token validation
leyline.account.AccountRoutesTest ‑ profile with valid token returns account data
leyline.account.AccountRoutesTest ‑ profile without auth returns 401
leyline.account.AccountRoutesTest ‑ refresh token grant returns new tokens
…

leyline.conformance.ZeroBlockersNoPromptTest ‑ AI combat with zero blockers does not prompt client

This pull request skips 584 tests.

leyline.architecture.PackageLayeringTest ‑ bridge does not depend on game, match, or protocol
leyline.architecture.PackageLayeringTest ‑ config does not depend on any matchdoor package
leyline.architecture.PackageLayeringTest ‑ conformance does not depend on any matchdoor package
leyline.architecture.PackageLayeringTest ‑ game does not depend on match, protocol, or infra
leyline.architecture.PackageLayeringTest ‑ infra does not depend on match or game
leyline.architecture.PackageLayeringTest ‑ protocol does not depend on match
leyline.bridge.DeckConverterTest ‑ converts CardEntry list to deck text
leyline.bridge.DeckConverterTest ‑ empty sideboard omits header
leyline.bridge.DeckConverterTest ‑ includes sideboard section
leyline.bridge.DeckConverterTest ‑ skips unknown grpIds
…

[github-actions]

CI Report — Gate

Tests: 154/754 passed, 2 failed (598 skipped)

Failed tests

• AiTurnNoAarTest.turnInfo phase correct at AAR during AI combat (34.5s)

  java.lang.AssertionError: Stale phase during AI combat turn 3: Beginning_a549

• TargetingFlowTest.bolt face cancel and recast (0.2s)

  io.kotest.assertions.AssertionFailedError: expected: but was:

Slow tests (>3s): 6

• AiTurnNoAarTest.AI turn sends no ActionsAvailableReq to client (34.7s)
• DeclareBlockersDedupeTest.no duplicate blockers req (18.8s)
• ActivatedAbilityPuzzleTest.Goblin Fireslinger tap-to-ping kills opponent (12.7s)
• PvpBridgeEndToEndTest.two-player game: land, creature, turn rotation, per-seat visibility (7.1s)
• ActivateAbilityTest.activate ability accepted by bridge (3.4s)
• BlockerDeclarationTest.human blocks AI attacker (3.3s)

[delebedev]

@claude code looks a bit meh - is it sustainable to patch this way or we are missing some subsystem?

[claude]

Claude finished @delebedev's task in 2m 13s —— View job

---

Architecture review

• Read changed files
• Analyze architectural sustainability
• Post findings

---

Short answer: no missing subsystem — CombatHandler is exactly the right place for this. But there are two specific rough edges worth addressing.

Why this is the right place

checkCombatPhase already owns the decision tree of "given this phase and turn, what do I send the client?" The zero-blockers check is a precondition on that same question: "is there even anything to block with?" Putting it anywhere else (a new orchestrator, a bridge event, a BlockerEligibilityService) would split that decision across layers without benefit.

The attacker side already does the same pattern, cleanly:

// COMBAT_DECLARE_ATTACKERS: check first, then prompt
val req = BundleBuilder.buildDeclareAttackersReq(...)
if (req.attackersCount > 0) {
    sendDeclareAttackersReq(...)
    return Signal.STOP
}
// else fall through

The blocker side now mirrors that. Structurally correct.

---

Issue 1 — BundleBuilder parameter is an optimization leak

The req: DeclareBlockersReq? = null parameter added to declareBlockersBundle exists for one reason: to avoid calling RequestBuilder.buildDeclareBlockersReq twice (once for the zero-check, once inside the bundle builder). But BundleBuilder's contract is to assemble GRE messages from game state — it shouldn't need a pre-computed sub-request passed in.

The double-build is a pure read (~50 creatures, <1ms) with no side effects. Just drop the parameter and accept the second call:

// CombatHandler — remove the pre-build, let declareBlockersReq build internally
sendDeclareBlockersReq(bridge)  // back to original signature

// BundleBuilder — revert to building req internally (one fewer parameter)
val builtReq = RequestBuilder.buildDeclareBlockersReq(game, seatId, bridge)

If the double-build ever shows up in profiling it can be addressed then. Right now it's premature coupling. Fix this →

---

Issue 2 — autoSubmitNoBlockers re-entry risk and weak recovery

Two related problems:

Re-entry: pendingBlockersSent guards against re-sending DeclareBlockersReq during a lingering priority window, but it's never set in the zero-blockers fast-path. If awaitPriority() returns before the engine advances out of COMBAT_DECLARE_BLOCKERS, the auto-pass loop re-enters checkCombatPhase, hits !pendingBlockersSent == true, and calls autoSubmitNoBlockers again on an already-submitted action. The getPending() == null branch then fires.

Weak recovery: The no-pending-action branch just calls awaitPriority() and returns silently — the game likely hangs. Consistent with the rest of CombatHandler, it should at least call ops.sendRealGameState(bridge) (same recovery used in onDeclareBlockers and onDeclareAttackers).

Fix:

private fun autoSubmitNoBlockers(bridge: GameBridge) {
    val pending = bridge.seat(ops.seatId).action.getPending() ?: run {
        log.warn("CombatHandler: zero blockers but no pending action — recovering")
        ops.sendRealGameState(bridge)  // consistent with other handlers
        return
    }
    lastDeclaredBlockAssignments.clear()
    pendingBlockersSent = true  // guard against re-entry
    bridge.seat(ops.seatId).action.submitAction(pending.actionId, PlayerAction.DeclareBlockers(emptyMap()))
    bridge.awaitPriority()
}

Fix this →

---

Everything else is fine

• The Signal.CONTINUE return (instead of Signal.STOP) is correct — no client interaction, auto-pass loop moves on.
• Not setting a SubmitBlockersResp to the client before the engine submit is correct — the real server doesn't send one when auto-submitting.
• Test fixture is tight, covers exactly the regression.
• The failing CI tests (AiTurnNoAarTest, TargetingFlowTest) look pre-existing / unrelated.

Verdict: sustainable fix in the right place. The two issues above are small enough to fix now or in a follow-up — the second one is the more important one to catch.

[delebedev]

Superseded by #217 which takes the same approach (auto-submit empty blockers when zero legal) with a cleaner implementation.