TASK.md

@@ -0,0 +1,54 @@
+# Vikunja: #460 — Retry budget: track all seen gate fingerprints, charge on repeat (cycle detection)
+
+**Problem**
+
+`DefaultRetryCoordinator.decide` compares the current failure fingerprint against only the *previous* one (`core/kernel/src/main/kotlin/com/correx/core/kernel/retry/DefaultRetryCoordinator.kt:29`):
+
+```kotlin
+val prevFingerprint = state.gateFailureFingerprints[gate]
+val progressed = prevFingerprint != fingerprint
+val charged = !progressed
+```
+
+A single slot cannot see a cycle. Whack-a-mole (fix A breaks B, fix B breaks A) alternates two fingerprints forever, every round reads as progress, nothing is ever charged, and the per-gate budget never triggers. `DefaultSessionOrchestratorStep.kt:296` already acknowledges this ("can be fooled into treating as still progressing indefinitely") but guards only inside the recovery stage. A normal implementer stage has no equivalent guard.
+
+**Fix**
+
+Make `OrchestrationState.gateFailureFingerprints[gate]` a `Set<String>` of every fingerprint seen for that gate, and charge when the current one is a repeat:
+
+```kotlin
+val seen = state.gateFailureFingerprints[gate]
+val charged = fingerprint in seen
+```
+
+Touches: `OrchestrationState` field type, the reducer that writes it, `DefaultRetryCoordinator.decide`.
+
+**Why not count-based progress**
+
+Rejected: "charge unless the failing-item count went down". A syntax error masks later errors — fix `'}' expected` and tsc parses further, reporting 5 real type errors where there was 1. Count goes 1 to 5 on the most productive edit of the run, and the guard would charge the budget for genuine progress.
+
+**Cases**
+
+- Unmasking, `{syntax}` to `{5 type errors}`: new fingerprint, unseen, free. Correct.
+- Grinding down, 3 to 2 to 1 errors: each state distinct, all free. Correct.
+- Whack-a-mole, A/B/A: round 3 repeats round 1, charged. This is the case the single slot misses today.
+
+**Known limit**
+
+Detects cycles, not progress. A stage emitting a fresh distinct failure every round is still unbounded; only `stageCount` catches it. Worth measuring with `scripts/artread.py` over a few failed sessions before assuming that pattern matters.
+
+## Acceptance criteria
+
+- [ ] (fill in before starting)
+
+## Quality gate
+
+```sh
+# the command that must pass, e.g. make check
+```
+
+## Rules
+
+- Do not edit this file.
+- One task, one session, one PR. Keep the diff under ~300 lines.
+- Finish with `task pr`.

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<String, Int> = 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<String, String> = 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<String, Set<String>> = 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<String> = emptySet(),

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) {

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)

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(

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