Merge pull request 'Refresh the failure mandate in-loop when a write lands on a file the failure names' (#2) from task/461-refresh-the-failure-mandate-in-loop-when into master

core/kernel/src/main/kotlin/com/correx/core/kernel/orchestration/LspMandateRefresh.kt

@@ -0,0 +1,80 @@
+package com.correx.core.kernel.orchestration
+
+// Split into its own file (same reason as RecoveryFileLoopBreak.kt) to keep
+// SessionOrchestratorGates2.kt under detekt's per-file function-count cap.
+
+import com.correx.core.context.model.ContextEntry
+import com.correx.core.events.events.LspDiagnosticsCompletedEvent
+import com.correx.core.events.events.RetryAttemptedEvent
+import com.correx.core.events.types.SessionId
+import com.correx.core.events.types.StageId
+
+internal const val LSP_DIAGNOSTICS_GATE = "lsp_diagnostics"
+
+/**
+ * True when [failureReason] names at least one of [writtenPaths] (Vikunja #461). The frozen mandate
+ * quotes diagnostics as `src/App.tsx:12:5 TS2322 ...`, so the path token is matched with the same
+ * suffix rule [resolveTicketOwner] uses on ticket evidence — the failure may name `App.tsx` while
+ * the write manifest holds the workspace-relative `src/App.tsx`.
+ */
+internal fun failureNamesWrittenPath(failureReason: String, writtenPaths: List<String>): Boolean {
+    val named = EVIDENCE_PATH_RE.findAll(failureReason)
+        .map { it.value.substringBefore(':').replace('\\', '/') }
+        .filter { it.length >= MIN_EVIDENCE_TOKEN }
+        .toSet()
+    if (named.isEmpty()) return false
+    return writtenPaths.any { written ->
+        val norm = written.replace('\\', '/')
+        named.any { norm == it || norm.endsWith("/$it") }
+    }
+}
+
+/**
+ * In-loop refresh of a stale `lsp_diagnostics` repair mandate (Vikunja #461). On a gate-repair retry
+ * the failure text is frozen for the whole tool loop: the agent edits the offending file, is told
+ * "written successfully", and keeps editing against a diagnostic it may already have cleared — it
+ * only finds out after `stage_complete`, when [runPostStageGates] re-runs from the top. Called from
+ * the existing `wroteThisRound` hook, this re-pulls diagnostics and records them, so
+ * [buildRetryFeedbackEntry]'s per-file ledger flips to "done, leave it" in-loop. Rebuilding from the
+ * recorded event (invariant #9) is what keeps the fresh truth replayable, and is why there is no new
+ * message format here.
+ *
+ * Scoped to LSP by design: a `tsc` / `npm run build` re-run per write is too expensive. Fires only
+ * when the write landed on a path the frozen failure actually names, so an unrelated write in the
+ * same loop costs nothing.
+ *
+ * Returns the rebuilt `retryFeedback` entry, or null when nothing applies (no runner, wrong gate, no
+ * overlap, or the pull was skipped). Returns null on a skipped pull deliberately: an empty
+ * diagnostics list from a server that never started reads as "clean" to the ledger, and telling the
+ * model to leave a still-broken file alone is worse than leaving the stale text in place.
+ */
+@Suppress("ReturnCount")
+internal suspend fun SessionOrchestrator.refreshLspRetryMandate(
+    sessionId: SessionId,
+    stageId: StageId,
+    effectives: RunEffectives,
+): ContextEntry? {
+    val runner = lspDiagnosticsRunner ?: return null
+    val workspaceRoot = effectives.policy?.workspaceRoot ?: return null
+    val pending = eventStore.read(sessionId)
+        .mapNotNull { it.payload as? RetryAttemptedEvent }
+        .lastOrNull { it.stageId == stageId }
+        ?: return null
+    if (pending.gate != LSP_DIAGNOSTICS_GATE) return null
+
+    // Pull for the stage's WHOLE written set, not just the paths the failure names, even though the
+    // overlap is what triggers the refresh: the recorded event is read back per path, and a path
+    // absent from it reads as clean. A partial pull would mark every other file the stage wrote
+    // "done, leave it" on no evidence.
+    val paths = stageWrittenPaths(sessionId, stageId)
+    if (!failureNamesWrittenPath(pending.failureReason, paths)) return null
+
+    val result = runner.pull(LspDiagnosticsRequest(workspaceRoot, paths))
+    if (result.skippedReason != null) return null
+    val diagnostics = result.diagnostics.filter { it.path in paths }
+    emit(
+        sessionId,
+        LspDiagnosticsCompletedEvent(sessionId, stageId, result.server, diagnostics, result.skippedReason),
+    )
+    return buildRetryFeedbackEntry(eventStore.read(sessionId), stageId)
+}

core/kernel/src/main/kotlin/com/correx/core/kernel/orchestration/SessionOrchestratorExecution.kt

@@ -538,6 +538,13 @@ internal suspend fun SessionOrchestrator.executeStage(
             val refreshed = remainingDeltaResults?.let { buildRemainingDeltaEntry(contractFailureItems(it)) }
             accumulatedEntries = accumulatedEntries.filterNot { it.sourceType == "remainingDelta" } +
                 listOfNotNull(refreshed)
+            // #461: the same cache-until-write logic applied to a frozen lsp_diagnostics repair
+            // mandate. Without it the agent keeps editing against a diagnostic it may already have
+            // cleared and only learns otherwise after stage_complete re-runs the gate.
+            refreshLspRetryMandate(sessionId, stageId, effectives)?.let { mandate ->
+                accumulatedEntries = accumulatedEntries.filterNot { it.sourceType == "retryFeedback" } +
+                    mandate
+            }
         }
         currentContext = contextPackBuilder.build(
             id = ContextPackId(UUID.randomUUID().toString()),

core/kernel/src/main/kotlin/com/correx/core/kernel/orchestration/SessionOrchestratorGates2.kt

@@ -107,7 +107,7 @@ internal suspend fun SessionOrchestrator.runLspDiagnostics(
     return StageExecutionResult.Failure(
         "stage ${stageId.value} has LSP diagnostics in files it wrote. Fix these before proceeding:\n$detail",
         retryable = true,
-        gate = "lsp_diagnostics",
+        gate = LSP_DIAGNOSTICS_GATE,
     )
 }
 

core/kernel/src/test/kotlin/com/correx/core/kernel/orchestration/LspMandateRefreshTest.kt

@@ -0,0 +1,42 @@
+package com.correx.core.kernel.orchestration
+
+import org.junit.jupiter.api.Assertions.assertFalse
+import org.junit.jupiter.api.Assertions.assertTrue
+import org.junit.jupiter.api.Test
+
+/**
+ * The trigger predicate for the in-loop mandate refresh (#461). It decides whether an LSP re-pull
+ * fires at all, so a false negative leaves the agent editing against a stale diagnostic and a false
+ * positive re-pulls on every unrelated write.
+ */
+class LspMandateRefreshTest {
+
+    private val failure = "stage build has LSP diagnostics in files it wrote. Fix these before proceeding:\n" +
+        "- src/api/queries.ts:39:3 TS1005 '}' expected"
+
+    @Test
+    fun `write on the path the failure names triggers a refresh`() {
+        assertTrue(failureNamesWrittenPath(failure, listOf("src/api/queries.ts")))
+    }
+
+    @Test
+    fun `a failure naming a bare filename still matches the workspace-relative write`() {
+        assertTrue(failureNamesWrittenPath("queries.ts(39,3): '}' expected", listOf("src/api/queries.ts")))
+    }
+
+    @Test
+    fun `an unrelated write does not trigger a refresh`() {
+        assertFalse(failureNamesWrittenPath(failure, listOf("src/api/other.ts", "README.md")))
+    }
+
+    @Test
+    fun `a suffix that is not a path boundary does not match`() {
+        // "notqueries.ts" ends with the named token as a substring but is a different file.
+        assertFalse(failureNamesWrittenPath(failure, listOf("src/api/notqueries.ts")))
+    }
+
+    @Test
+    fun `a failure naming no path never triggers a refresh`() {
+        assertFalse(failureNamesWrittenPath("stage build failed: server exited", listOf("src/api/queries.ts")))
+    }
+}