diff --git a/core/kernel/src/main/kotlin/com/correx/core/kernel/orchestration/LspMandateRefresh.kt b/core/kernel/src/main/kotlin/com/correx/core/kernel/orchestration/LspMandateRefresh.kt new file mode 100644 index 00000000..f6aca9e5 --- /dev/null +++ b/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): 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) +} diff --git a/core/kernel/src/main/kotlin/com/correx/core/kernel/orchestration/SessionOrchestratorExecution.kt b/core/kernel/src/main/kotlin/com/correx/core/kernel/orchestration/SessionOrchestratorExecution.kt index 83b46a10..da594f96 100644 --- a/core/kernel/src/main/kotlin/com/correx/core/kernel/orchestration/SessionOrchestratorExecution.kt +++ b/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()), diff --git a/core/kernel/src/main/kotlin/com/correx/core/kernel/orchestration/SessionOrchestratorGates2.kt b/core/kernel/src/main/kotlin/com/correx/core/kernel/orchestration/SessionOrchestratorGates2.kt index bd590d2b..cec7451b 100644 --- a/core/kernel/src/main/kotlin/com/correx/core/kernel/orchestration/SessionOrchestratorGates2.kt +++ b/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, ) } diff --git a/core/kernel/src/test/kotlin/com/correx/core/kernel/orchestration/LspMandateRefreshTest.kt b/core/kernel/src/test/kotlin/com/correx/core/kernel/orchestration/LspMandateRefreshTest.kt new file mode 100644 index 00000000..c2dd77d3 --- /dev/null +++ b/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"))) + } +}