feat(kernel): static-first reviewer gate (role-reliability §5)

Run operator-configured static-analysis commands (compiler/detekt/formatters) as a deterministic harness step on the producing stage, before the LLM reviewer. A stage declares `static_analysis = [commands]`; after it produces its artifact, each command runs in the workspace root and the results are recorded in a StaticAnalysisCompletedEvent (invariant #9 env observation — recorded live, folded on replay, never re-run). A non-clean command fails the stage retryably with its output fed back verbatim (§2: compiler/test output is ground truth), so the implementer fixes it and only static-clean code ever reaches the reviewer. This makes §5's "reviewer context excludes static findings" mechanical — the findings are resolved upstream, so the reviewer can't waste inference on what detekt catches for free; no output-parsing, no context plumbing. - StaticAnalysisCompletedEvent + StaticAnalysisFinding (core:events) + registration. - StaticAnalysisRunner seam + ProcessStaticAnalysisRunner (whitespace-split argv, stderr merged, timeout→nonzero exit) in core:kernel; wired (nullable) into OrchestratorEngines and the server. Null runner / no workspace root → logged no-op, never blocks the run. - StageConfig.staticAnalysis + `static_analysis` TOML field; runStaticAnalysis folded into a runPostStageGates sequence (produces → grounding → echo → static analysis). - Documented `static_analysis` on the role_pipeline implementer stage (commented; commands are workspace-specific). The reviewer prompt half shipped in 3467826.
2026-06-14 23:51:49 +04:00
parent 0d8df4a130
commit 365eb8a279
13 changed files with 354 additions and 9 deletions
@@ -28,4 +28,7 @@ data class OrchestratorEngines(
    val workspacePolicy: WorkspacePolicy? = null,
    val worldProbe: WorldProbe = FileSystemWorldProbe(),
    val workspaceToolRegistryProvider: WorkspaceToolRegistryProvider? = null,
+    // Runs operator-configured static-analysis commands as a harness step (role-reliability §5).
+    // Null = no static-first step; a stage declaring `static_analysis` then no-ops with a warning.
+    val staticAnalysisRunner: StaticAnalysisRunner? = null,
 )
@@ -0,0 +1,55 @@
+package com.correx.core.kernel.orchestration
+
+import kotlinx.coroutines.Dispatchers
+import kotlinx.coroutines.TimeoutCancellationException
+import kotlinx.coroutines.async
+import kotlinx.coroutines.withContext
+import kotlinx.coroutines.withTimeout
+import java.nio.file.Path
+
+/**
+ * Process-spawning [StaticAnalysisRunner]: splits each operator-configured command on whitespace
+ * into argv (no shell), runs it in the workspace root with stderr merged into stdout, and returns
+ * its exit code with the combined output. A spawn failure or timeout is reported as a non-zero exit
+ * so the gate treats a hung or unrunnable check as a failure rather than silently passing.
+ *
+ * Commands come from the operator's workflow config (trusted, like prompt files), not from the LLM,
+ * so they bypass the tool-approval pipeline by design. Mirrors the streaming/timeout shape of
+ * ShellTool to avoid pipe-buffer deadlock on large output.
+ */
+class ProcessStaticAnalysisRunner(
+    private val timeoutMs: Long = DEFAULT_TIMEOUT_MS,
+) : StaticAnalysisRunner {
+
+    override suspend fun run(workingDir: Path, command: String): StaticAnalysisRunResult =
+        withContext(Dispatchers.IO) {
+            val argv = command.trim().split(WHITESPACE).filter { it.isNotEmpty() }
+            if (argv.isEmpty()) return@withContext StaticAnalysisRunResult(EXIT_NOT_RUN, "empty command")
+            val process = runCatching {
+                ProcessBuilder(argv).directory(workingDir.toFile()).redirectErrorStream(true).start()
+            }.getOrElse {
+                return@withContext StaticAnalysisRunResult(EXIT_NOT_RUN, "failed to start '$command': ${it.message}")
+            }
+            runCatching {
+                withTimeout(timeoutMs) {
+                    val outputDeferred = async { process.inputStream.bufferedReader().use { it.readText() } }
+                    val exit = process.waitFor()
+                    StaticAnalysisRunResult(exit, outputDeferred.await())
+                }
+            }.getOrElse {
+                process.destroyForcibly()
+                val reason = if (it is TimeoutCancellationException) {
+                    "static analysis '$command' timed out after ${timeoutMs}ms"
+                } else {
+                    it.message ?: "error running '$command'"
+                }
+                StaticAnalysisRunResult(EXIT_NOT_RUN, reason)
+            }
+        }
+
+    private companion object {
+        const val DEFAULT_TIMEOUT_MS = 300_000L
+        const val EXIT_NOT_RUN = -1
+        val WHITESPACE = Regex("\\s+")
+    }
+}
@@ -30,6 +30,8 @@ import com.correx.core.events.events.ArtifactContentStoredEvent
 import com.correx.core.events.events.ArtifactCreatedEvent
 import com.correx.core.events.events.BriefEchoMismatchEvent
 import com.correx.core.events.events.BriefGroundingCheckedEvent
+import com.correx.core.events.events.StaticAnalysisCompletedEvent
+import com.correx.core.events.events.StaticAnalysisFinding
 import com.correx.core.events.events.ContextTruncatedEvent
 import com.correx.core.events.events.ClarificationAnswer
 import com.correx.core.events.events.ClarificationAnsweredEvent
@@ -151,6 +153,11 @@ private const val HTTP_TOO_MANY_REQUESTS = 429
 // Cap on clarification rounds per stage, so a stage that keeps re-asking eventually proceeds.
 private const val MAX_CLARIFICATION_ROUNDS = 3

+// Static-analysis output caps: the tail retained in the recorded event vs. the (larger) slice fed
+// back to the model so it can fix the failure. Tails, because the error summary sits at the end.
+private const val STATIC_ANALYSIS_SUMMARY_CAP = 2_000
+private const val STATIC_ANALYSIS_FEEDBACK_CAP = 6_000
+
@SuppressWarnings(
    "ForbiddenComment",
    "UnusedParameter",
@@ -182,6 +189,7 @@ abstract class SessionOrchestrator(
    private val toolCallAssessor: ToolCallAssessor? = engines.toolCallAssessor
    private val workspacePolicy: WorkspacePolicy? = engines.workspacePolicy
    private val worldProbe: WorldProbe = engines.worldProbe
+    private val staticAnalysisRunner: StaticAnalysisRunner? = engines.staticAnalysisRunner
    private val workspaceToolRegistryProvider: WorkspaceToolRegistryProvider? = engines.workspaceToolRegistryProvider
    private val inferenceRepository: InferenceRepository = repositories.inferenceRepository
    internal val orchestrationRepository: OrchestrationRepository = repositories.orchestrationRepository
@@ -549,15 +557,7 @@ abstract class SessionOrchestrator(
                    is StageExecutionResult.Success -> {
                        emitToolArtifacts(sessionId, stageId, stageConfig)
                        emitLlmArtifacts(sessionId, stageId, stageConfig)
-                        when (val produced = verifyProduces(sessionId, stageId, stageConfig)) {
-                            is StageExecutionResult.Success ->
-                                when (val grounded = groundBriefReferences(sessionId, stageId, stageConfig, effectives)) {
-                                    is StageExecutionResult.Success ->
-                                        checkBriefEcho(sessionId, stageId, stageConfig)
-                                    is StageExecutionResult.Failure -> grounded
-                                }
-                            is StageExecutionResult.Failure -> produced
-                        }
+                        runPostStageGates(sessionId, stageId, stageConfig, effectives)
                    }

                    is StageExecutionResult.Failure -> outcome
@@ -1468,6 +1468,88 @@ abstract class SessionOrchestrator(
        }
    }

+    /**
+     * Post-validation stage gates, run in order; the first failure short-circuits and is returned.
+     * Cheap deterministic checks run first (produces presence, brief grounding, brief echo), then
+     * the heavier process-spawning static-analysis step last, so it only runs once a stage is
+     * otherwise sound.
+     */
+    private suspend fun runPostStageGates(
+        sessionId: SessionId,
+        stageId: StageId,
+        stageConfig: StageConfig,
+        effectives: RunEffectives,
+    ): StageExecutionResult {
+        val gates: List<suspend () -> StageExecutionResult> = listOf(
+            { verifyProduces(sessionId, stageId, stageConfig) },
+            { groundBriefReferences(sessionId, stageId, stageConfig, effectives) },
+            { checkBriefEcho(sessionId, stageId, stageConfig) },
+            { runStaticAnalysis(sessionId, stageId, stageConfig, effectives) },
+        )
+        for (gate in gates) {
+            val result = gate()
+            if (result is StageExecutionResult.Failure) return result
+        }
+        return StageExecutionResult.Success(emptyList())
+    }
+
+    /**
+     * Static-first reviewer gate (role-reliability §5): for a stage that declares `static_analysis`
+     * commands, run them (compiler / detekt / formatters) against its just-produced output in the
+     * workspace root. Every result is recorded in a [StaticAnalysisCompletedEvent] — an environment
+     * observation captured at run time (invariant #9), so replay reads it back and never re-runs.
+     *
+     * A non-clean command fails the stage retryably with its output fed back verbatim (§2: compiler/
+     * test output is ground truth, not LLM noise), so the producing stage fixes the issue and only
+     * static-clean output ever reaches the downstream LLM reviewer — its context thereby excludes
+     * anything static tools already catch, with no parsing or context plumbing.
+     */
+    private suspend fun runStaticAnalysis(
+        sessionId: SessionId,
+        stageId: StageId,
+        stageConfig: StageConfig,
+        effectives: RunEffectives,
+    ): StageExecutionResult {
+        if (stageConfig.staticAnalysis.isEmpty()) return StageExecutionResult.Success(emptyList())
+        val runner = staticAnalysisRunner
+        val workspaceRoot = effectives.policy?.workspaceRoot
+        if (runner == null || workspaceRoot == null) {
+            log.warn(
+                "[Orchestrator] stage declares static_analysis but no {} is wired — skipping " +
+                    "session={} stage={}",
+                if (runner == null) "runner" else "workspace root",
+                sessionId.value, stageId.value,
+            )
+            return StageExecutionResult.Success(emptyList())
+        }
+
+        val results = stageConfig.staticAnalysis.map { command ->
+            val run = runner.run(workspaceRoot, command)
+            StaticAnalysisFinding(
+                command = command,
+                exitCode = run.exitCode,
+                summary = run.output.takeLast(STATIC_ANALYSIS_SUMMARY_CAP),
+            ) to run.output
+        }
+        emit(sessionId, StaticAnalysisCompletedEvent(sessionId, stageId, results.map { it.first }))
+
+        val failures = results.filterNot { it.first.clean }
+        if (failures.isEmpty()) return StageExecutionResult.Success(emptyList())
+
+        log.warn(
+            "[Orchestrator] static analysis failed session={} stage={} commands={}",
+            sessionId.value, stageId.value,
+            failures.joinToString(", ") { it.first.command },
+        )
+        val detail = failures.joinToString("\n\n") { (finding, output) ->
+            "$ ${finding.command}  (exit ${finding.exitCode})\n${output.takeLast(STATIC_ANALYSIS_FEEDBACK_CAP)}"
+        }
+        return StageExecutionResult.Failure(
+            "stage ${stageId.value} did not pass static analysis. Fix these before review:\n\n$detail",
+            retryable = true,
+        )
+    }
+
    private suspend fun emitProcessResultEvents(
        sessionId: SessionId,
        stageId: StageId,
@@ -0,0 +1,22 @@
+package com.correx.core.kernel.orchestration
+
+import java.nio.file.Path
+
+/** Result of running one static-analysis command: its exit code and combined stdout+stderr. */
+data class StaticAnalysisRunResult(
+    val exitCode: Int,
+    val output: String,
+)
+
+/**
+ * Seam for running operator-configured static-analysis commands (compiler / detekt / formatters)
+ * as a deterministic harness step (role-reliability §5 static-first ordering). Implemented by a
+ * process-spawning adapter in infrastructure and injected, so the deterministic core never spawns
+ * processes itself and the gate stays unit-testable with a fake.
+ *
+ * The orchestrator records each result as a [com.correx.core.events.events.StaticAnalysisCompletedEvent]
+ * (invariant #9 environment observation) — replay reads the recorded facts and never re-runs the command.
+ */
+fun interface StaticAnalysisRunner {
+    suspend fun run(workingDir: Path, command: String): StaticAnalysisRunResult
+}
@@ -0,0 +1,54 @@
+package com.correx.core.kernel.orchestration
+
+import kotlinx.coroutines.runBlocking
+import org.junit.jupiter.api.Assertions.assertEquals
+import org.junit.jupiter.api.Assertions.assertTrue
+import org.junit.jupiter.api.Test
+import java.nio.file.Path
+import kotlin.io.path.createTempDirectory
+import kotlin.io.path.writeText
+
+class ProcessStaticAnalysisRunnerTest {
+
+    private val runner = ProcessStaticAnalysisRunner()
+
+    private fun script(dir: Path, name: String, body: String): String {
+        val file = dir.resolve(name)
+        file.writeText("#!/bin/sh\n$body\n")
+        file.toFile().setExecutable(true)
+        return "./$name"
+    }
+
+    @Test
+    fun `clean command reports exit zero and captures output`() = runBlocking {
+        val dir = createTempDirectory("sa-ok")
+        val cmd = script(dir, "ok.sh", "echo all-good")
+        val result = runner.run(dir, cmd)
+        assertEquals(0, result.exitCode)
+        assertTrue(result.output.contains("all-good"), "output: ${result.output}")
+    }
+
+    @Test
+    fun `failing command reports nonzero exit and captures stderr`() = runBlocking {
+        val dir = createTempDirectory("sa-fail")
+        // stderr is merged into the captured output (redirectErrorStream).
+        val cmd = script(dir, "bad.sh", "echo boom 1>&2\nexit 2")
+        val result = runner.run(dir, cmd)
+        assertEquals(2, result.exitCode)
+        assertTrue(result.output.contains("boom"), "output: ${result.output}")
+    }
+
+    @Test
+    fun `unrunnable command is a failure, not a silent pass`() = runBlocking {
+        val dir = createTempDirectory("sa-missing")
+        val result = runner.run(dir, "./does-not-exist-xyz")
+        assertTrue(result.exitCode != 0, "exit: ${result.exitCode}")
+    }
+
+    @Test
+    fun `blank command is a failure`() = runBlocking {
+        val dir = createTempDirectory("sa-blank")
+        val result = runner.run(dir, "   ")
+        assertTrue(result.exitCode != 0, "exit: ${result.exitCode}")
+    }
+}