From 1ef845ed0ff9c6af912dac2463aff8133f9ddb29 Mon Sep 17 00:00:00 2001 From: claude Date: Sun, 2 Aug 2026 14:12:47 +0400 Subject: [PATCH] fix(retry): charge gate budget on any repeated failure fingerprint (#460) gateFailureFingerprints held one slot per gate, so the budget only charged when a failure repeated back to back. Whack-a-mole (fix A breaks B, fix B breaks A) alternates two fingerprints forever, every round read as progress, and the per-gate budget never triggered. Track the full set seen per gate and charge when the current fingerprint is a repeat. Co-Authored-By: Claude Opus 5 --- .../orchestration/OrchestrationState.kt | 9 ++++-- .../DefaultOrchestrationReducer.kt | 3 +- .../kernel/retry/DefaultRetryCoordinator.kt | 7 +++-- .../kotlin/DefaultRetryCoordinatorTest.kt | 31 ++++++++++++++++--- .../test/kotlin/OrchestrationReducerTest.kt | 4 +-- 5 files changed, 40 insertions(+), 14 deletions(-) diff --git a/core/events/src/main/kotlin/com/correx/core/events/orchestration/OrchestrationState.kt b/core/events/src/main/kotlin/com/correx/core/events/orchestration/OrchestrationState.kt index 4f4c4ae9..1609e8f5 100644 --- a/core/events/src/main/kotlin/com/correx/core/events/orchestration/OrchestrationState.kt +++ b/core/events/src/main/kotlin/com/correx/core/events/orchestration/OrchestrationState.kt @@ -19,9 +19,12 @@ data class OrchestrationState( // brief_grounding, brief_echo, contract, plan_compile, static_analysis, execution, review, ...) // exhausts its own budget rather than sharing retryCount session-wide. Attempts charged so far. val gateRetryBudgets: Map = emptyMap(), - // Last-seen failure fingerprint per gate, used to tell a no-progress retry (same fingerprint, - // charged) from a genuine-progress retry (changed fingerprint, free). - val gateFailureFingerprints: Map = emptyMap(), + // Every failure fingerprint seen per gate, used to tell a no-progress retry (a fingerprint + // already seen for this gate, charged) from a genuine-progress retry (an unseen fingerprint, + // free). A set, not a single slot: whack-a-mole (fix A breaks B, fix B breaks A) alternates two + // fingerprints forever and a single slot reads every round as progress, so nothing is ever + // charged and the budget never triggers. + val gateFailureFingerprints: Map> = emptyMap(), // Gates that have already spent their one hybrid-exhaustion salvage reset (review gate only, // see RetrySalvageDecidedEvent) — a second exhaustion for that gate is terminal. val gateSalvageUsed: Set = emptySet(), diff --git a/core/kernel/src/main/kotlin/com/correx/core/kernel/orchestration/DefaultOrchestrationReducer.kt b/core/kernel/src/main/kotlin/com/correx/core/kernel/orchestration/DefaultOrchestrationReducer.kt index e1aa403f..102ce745 100644 --- a/core/kernel/src/main/kotlin/com/correx/core/kernel/orchestration/DefaultOrchestrationReducer.kt +++ b/core/kernel/src/main/kotlin/com/correx/core/kernel/orchestration/DefaultOrchestrationReducer.kt @@ -83,7 +83,8 @@ class DefaultOrchestrationReducer : OrchestrationReducer { } else { state.gateRetryBudgets }, - gateFailureFingerprints = state.gateFailureFingerprints + (p.gate to p.fingerprint), + gateFailureFingerprints = state.gateFailureFingerprints + + (p.gate to ((state.gateFailureFingerprints[p.gate] ?: emptySet()) + p.fingerprint)), ) is RetrySalvageDecidedEvent -> if (p.decision == SalvageDecision.CONTINUE) { diff --git a/core/kernel/src/main/kotlin/com/correx/core/kernel/retry/DefaultRetryCoordinator.kt b/core/kernel/src/main/kotlin/com/correx/core/kernel/retry/DefaultRetryCoordinator.kt index 36e928bc..9ddff190 100644 --- a/core/kernel/src/main/kotlin/com/correx/core/kernel/retry/DefaultRetryCoordinator.kt +++ b/core/kernel/src/main/kotlin/com/correx/core/kernel/retry/DefaultRetryCoordinator.kt @@ -26,9 +26,10 @@ class DefaultRetryCoordinator( policy: RetryPolicy, ): RetryDecision { val fingerprint = FailureFingerprint.of(failureReason) - val prevFingerprint = state.gateFailureFingerprints[gate] - val progressed = prevFingerprint != fingerprint - val charged = !progressed + // Charge when this failure has been seen before for this gate — a repeat means the round + // trip produced no state the gate has not already rejected. Comparing against only the + // previous fingerprint misses cycles: A/B/A alternation reads as progress forever. + val charged = fingerprint in (state.gateFailureFingerprints[gate] ?: emptySet()) val currentCount = state.gateRetryBudgets[gate] ?: 0 val newCount = if (charged) currentCount + 1 else currentCount val maxAttempts = policy.maxAttemptsFor(gate) diff --git a/testing/kernel/src/test/kotlin/DefaultRetryCoordinatorTest.kt b/testing/kernel/src/test/kotlin/DefaultRetryCoordinatorTest.kt index 4df03d02..3ff987f6 100644 --- a/testing/kernel/src/test/kotlin/DefaultRetryCoordinatorTest.kt +++ b/testing/kernel/src/test/kotlin/DefaultRetryCoordinatorTest.kt @@ -179,7 +179,7 @@ class DefaultRetryCoordinatorTest { val policy = RetryPolicy(maxAttempts = 3, backoffMs = 0L) val state = OrchestrationState( gateRetryBudgets = mapOf("contract" to 1), - gateFailureFingerprints = mapOf("contract" to FailureFingerprint.of("same-reason")), + gateFailureFingerprints = mapOf("contract" to setOf(FailureFingerprint.of("same-reason"))), ) val decision = retryCoordinator.decide( @@ -200,7 +200,7 @@ class DefaultRetryCoordinatorTest { // changed fingerprint (progress) must still be free and retry. val state = OrchestrationState( gateRetryBudgets = mapOf("contract" to 1), - gateFailureFingerprints = mapOf("contract" to "old-reason"), + gateFailureFingerprints = mapOf("contract" to setOf("old-reason")), ) val decision = retryCoordinator.decide( @@ -218,7 +218,7 @@ class DefaultRetryCoordinatorTest { val policy = RetryPolicy(maxAttempts = 2, backoffMs = 0L) val state = OrchestrationState( gateRetryBudgets = mapOf("contract" to 2), - gateFailureFingerprints = mapOf("contract" to FailureFingerprint.of("same-reason")), + gateFailureFingerprints = mapOf("contract" to setOf(FailureFingerprint.of("same-reason"))), ) val decision = retryCoordinator.decide( @@ -234,7 +234,7 @@ class DefaultRetryCoordinatorTest { val policy = RetryPolicy(maxAttempts = 1, backoffMs = 0L) val state = OrchestrationState( gateRetryBudgets = mapOf("contract" to 1), - gateFailureFingerprints = mapOf("contract" to FailureFingerprint.of("same-reason")), + gateFailureFingerprints = mapOf("contract" to setOf(FailureFingerprint.of("same-reason"))), ) // "contract" is already exhausted at maxAttempts=1, but "review" has never charged. @@ -245,12 +245,33 @@ class DefaultRetryCoordinatorTest { assertEquals(RetryDecision.Retry, decision) } + @Test + fun `decide charges a whack-a-mole repeat — fingerprint seen earlier but not last round`() = runTest { + val policy = RetryPolicy(maxAttempts = 3, backoffMs = 0L) + // A/B/A: round 3 repeats round 1's failure. The previous fingerprint is B, so a single-slot + // comparison would read this as progress and never charge. + val state = OrchestrationState( + gateRetryBudgets = mapOf("contract" to 1), + gateFailureFingerprints = mapOf( + "contract" to setOf(FailureFingerprint.of("A"), FailureFingerprint.of("B")), + ), + ) + + retryCoordinator.decide( + sessionId, stageId, gate = "contract", failureReason = "A", state = state, policy = policy, + ) + + val event = eventStore.read(sessionId).single().payload as RetryAttemptedEvent + assertEquals(true, event.charged) + assertEquals(2, event.attemptNumber) + } + @Test fun `decide honours perGateMaxAttempts override`() = runTest { val policy = RetryPolicy(maxAttempts = 1, backoffMs = 0L, perGateMaxAttempts = mapOf("review" to 5)) val state = OrchestrationState( gateRetryBudgets = mapOf("review" to 4), - gateFailureFingerprints = mapOf("review" to "same-reason"), + gateFailureFingerprints = mapOf("review" to setOf(FailureFingerprint.of("same-reason"))), ) val decision = retryCoordinator.decide( diff --git a/testing/projections/src/test/kotlin/OrchestrationReducerTest.kt b/testing/projections/src/test/kotlin/OrchestrationReducerTest.kt index 7870fe63..9dbd0c04 100644 --- a/testing/projections/src/test/kotlin/OrchestrationReducerTest.kt +++ b/testing/projections/src/test/kotlin/OrchestrationReducerTest.kt @@ -167,7 +167,7 @@ class OrchestrationReducerTest { ), ) assertEquals(1, retried.gateRetryBudgets["contract"]) - assertEquals("fp1", retried.gateFailureFingerprints["contract"]) + assertEquals(setOf("fp1"), retried.gateFailureFingerprints["contract"]) } @Test @@ -191,7 +191,7 @@ class OrchestrationReducerTest { ), ) assertEquals(1, free.gateRetryBudgets["contract"]) - assertEquals("fp2", free.gateFailureFingerprints["contract"]) + assertEquals(setOf("fp1", "fp2"), free.gateFailureFingerprints["contract"]) } @Test