wip(freestyle/acr): grounding & edit-tool fixes + ACR-compiler experiment

This branch's uncommitted WIP, committed together (entangled at file level). Distinct pieces of work: Freestyle QA fixes (this session): - FileEditTool: pre-validate replace anchor in validateRequest — reject a missing/ambiguous target BEFORE the approval gate, mirroring read/write's file-not-found / read-before-write pre-checks. Shared not-found/ambiguous messages between validate and execute so they can't drift. - PlanGrounder: add `scanned` flag; when no RepoMapComputedEvent was recorded, repoMapPaths is "unknown" not "empty workspace" — skip scope grounding (which proves a path ABSENT) so real paths (apps/server/**) aren't falsely rejected. Build-manifest check still runs. - FreestyleDriver: wire scanned=(repoMap!=null); on plan rejection emit a session-terminal WorkflowFailedEvent so a rejected run reads FAILED, not the COMPLETED-lie (last verdict was the planning-phase WorkflowCompleted). - ServerModule: resolve project-memory workspace root from the session's bound workspace (sessionWorkspaceRoot) instead of boot-static pm.repoRoot(), fixing the workspace-binding divergence (correx vs empty scratch dir). Retire tracked in Vikunja #266. - LaunchRegistrationRaceTest: join registered jobs before asserting launchCount — computeIfAbsent returns the Job immediately but the fire-and-forget launch body lagged awaitAll (the 49-vs-50 flake). ACR concept-compiler experiment (pre-existing WIP on this branch): - ExecutionPlanCompiler/Model/PlanLinter, #264 needs-seam (sessionArtifacts), LSP diagnostics subsystem (LspDiagnosticEvents/Runner/Lsp4j), BootWorkspace, config surface, workflow prompts/schemas, orchestrator advance-don't-rerun. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-07-19 01:20:37 +04:00
parent 7b90944b61
commit 1b58bc325e
57 changed files with 1205 additions and 115 deletions
@@ -2,7 +2,7 @@

 ## Purpose

-End-to-end integration tests that wire multiple core modules together without live LLM or network. Covers: `SessionOrchestrator` full flows, tool-call gate, freestyle approval gate, clarification loop, steering preemption, graph reroute, artifact injection, workspace-scoped tool registry, validation pipeline integration, critique calibration wiring, session rehydration, repo-map reuse, and subagent runner seam.
+End-to-end integration tests that wire multiple core modules together without live LLM or network. Covers: `SessionOrchestrator` full flows, tool-call gate, freestyle approval gate, terminal-review gate ordering, bounded review-loop recovery, clarification loop, steering preemption, graph reroute, artifact injection, workspace-scoped tool registry, validation pipeline integration, critique calibration wiring, session rehydration, repo-map reuse, and subagent runner seam.

 ## Ownership

@@ -52,14 +52,14 @@ import kotlinx.coroutines.withTimeout
 import kotlinx.coroutines.yield
 import org.junit.jupiter.api.Assertions.assertEquals
 import org.junit.jupiter.api.Assertions.assertNotNull
-import org.junit.jupiter.api.Assertions.assertTrue
 import org.junit.jupiter.api.Test

 /**
- * Integration test for the producer-exit clarification loop: a stage whose produced artifact
+ * Integration test for the producer-exit clarification handoff: a stage whose produced artifact
 * carries an unanswered `questions` array parks its run, surfaces a [ClarificationRequestedEvent],
- * and on operator answer re-runs the same stage with the answers in context — without consuming the
- * failure-retry budget (retry maxAttempts is 1 here). Bounded at three rounds.
+ * and on operator answer the run ADVANCES to the next stage with the answers in context — the
+ * questioning stage is not re-run (a re-run would restart inference with no prior reasoning and
+ * make the stage re-explore; 2026-07-18). The answers reach later stages as session-wide events.
 */
 class ClarificationLoopTest {

@@ -147,7 +147,7 @@ class ClarificationLoopTest {
    private val config = OrchestrationConfig(retryPolicy = RetryPolicy(maxAttempts = 1, backoffMs = 0))

    @Test
-    fun `open questions park the stage, the answer re-runs it, then the workflow completes`():
+    fun `open questions park the stage, the answer advances to the next stage`():
        Unit = runBlocking {
        val sessionId = SessionId("clarify-happy")
        val orchestrator = orchestrator(listOf(withQuestions, clean))
@@ -177,14 +177,15 @@ class ClarificationLoopTest {
        )
        assertNotNull(
            events.find { it.payload is WorkflowCompletedEvent },
-            "Workflow should complete after the clarification re-run produced a clean analysis",
+            "Workflow should complete once the operator answered and the run advanced",
        )
    }

    @Test
-    fun `clarification rounds are capped so a stuck stage still proceeds`(): Unit = runBlocking {
-        val sessionId = SessionId("clarify-cap")
-        // Always returns questions: without a cap this would loop forever.
+    fun `a stage that emits questions parks once and is never re-run`(): Unit = runBlocking {
+        val sessionId = SessionId("clarify-no-rerun")
+        // Provider ALWAYS returns questions. Under the old re-run loop this would park up to the
+        // round cap; under the advance-don't-re-run contract it parks exactly once, then proceeds.
        val orchestrator = orchestrator(listOf(withQuestions))
        val runJob = launch { orchestrator.run(sessionId, graph(), config) }

@@ -207,10 +208,10 @@ class ClarificationLoopTest {
        runJob.join()

        val rounds = eventStore.read(sessionId).count { it.payload is ClarificationRequestedEvent }
-        assertEquals(3, rounds, "Clarification must be capped at three rounds")
+        assertEquals(1, rounds, "Questioning stage must park exactly once (no re-run)")
        assertNotNull(
            eventStore.read(sessionId).find { it.payload is WorkflowCompletedEvent },
-            "Workflow should proceed once the clarification cap is reached",
+            "Workflow should advance to completion after the single answered clarification",
        )
    }
 }
@@ -11,6 +11,9 @@ import com.correx.core.approvals.model.DomainApprovalRequest
 import com.correx.core.artifacts.DefaultArtifactReducer
 import com.correx.core.events.EventDispatcher
 import com.correx.core.events.events.FailureTicketOpenedEvent
+import com.correx.core.events.events.StaticAnalysisCompletedEvent
+import com.correx.core.events.events.TransitionExecutedEvent
+import com.correx.core.events.events.WorkflowCompletedEvent
 import com.correx.core.events.events.WorkflowFailedEvent
 import com.correx.core.events.execution.RetryPolicy
 import com.correx.core.events.types.ProviderId
@@ -424,6 +427,7 @@ class RecoveryRoutingTest {
    private fun buildMultiToolOrchestrator(
        staticOutput: String = "App.tsx: TS2322 type mismatch",
        provider: InferenceProvider = StageRoutingProvider(),
+        staticExitCode: Int = 1,
    ): Pair<DefaultSessionOrchestrator, InMemoryEventStore> {
        val eventStore = InMemoryEventStore()
        val shell = ShellTool(allowedExecutables = setOf("true"), workingDir = workspace)
@@ -471,7 +475,7 @@ class RecoveryRoutingTest {
            workspacePolicy = WorkspacePolicy(workspace),
            // Gate names the file impl wrote, so route-to-owner can resolve the author from the manifest.
            staticAnalysisRunner = StaticAnalysisRunner { _, _ ->
-                StaticAnalysisRunResult(exitCode = 1, output = staticOutput)
+                StaticAnalysisRunResult(exitCode = staticExitCode, output = staticOutput)
            },
            approvalEngine = object : ApprovalEngine {
                override fun evaluate(
@@ -498,6 +502,84 @@ class RecoveryRoutingTest {
        return orchestrator to eventStore
    }

+    @Test
+    fun `non-writing review terminal records static analysis before workflow completion`(): Unit = runBlocking {
+        val (orchestrator, eventStore) = buildMultiToolOrchestrator(
+            staticOutput = "static floor clean",
+            staticExitCode = 0,
+        )
+        val sessionId = SessionId("review-terminal-static-floor")
+        val graph = WorkflowGraph(
+            id = "freestyle-review-terminal",
+            stages = mapOf(
+                StageId("impl") to StageConfig(
+                    allowedTools = setOf("file_write"),
+                    writeManifest = listOf("App.tsx"),
+                ),
+                StageId("review") to StageConfig(
+                    allowedTools = setOf("shell"),
+                    staticAnalysis = listOf("correx-static-floor -- App.tsx"),
+                    autoBuildGate = true,
+                    metadata = mapOf("role" to "reviewer"),
+                ),
+            ),
+            transitions = setOf(
+                TransitionEdge(TransitionId("impl->review"), StageId("impl"), StageId("review")) { true },
+                TransitionEdge(TransitionId("review->done"), StageId("review"), StageId("done")) { true },
+            ),
+            start = StageId("impl"),
+        )
+
+        orchestrator.run(sessionId, graph, OrchestrationConfig())
+
+        val payloads = eventStore.read(sessionId).map { it.payload }
+        val staticIndex = payloads.indexOfFirst {
+            it is StaticAnalysisCompletedEvent && it.stageId == StageId("review")
+        }
+        val completedIndex = payloads.indexOfFirst { it is WorkflowCompletedEvent }
+        assertTrue(staticIndex >= 0, "the non-writing review terminal must execute its static floor")
+        assertTrue(completedIndex > staticIndex, "static analysis must be durably recorded before completion")
+    }
+
+    @Test
+    fun `review loop routes to recovery after exactly three back edges and never emits a fourth`(): Unit =
+        runBlocking {
+            val (orchestrator, eventStore) = buildMultiToolOrchestrator()
+            val sessionId = SessionId("review-loop-convergence")
+            val graph = WorkflowGraph(
+                id = "review-loop-convergence",
+                stages = mapOf(
+                    StageId("impl") to StageConfig(allowedTools = setOf("file_write")),
+                    StageId("review") to StageConfig(
+                        allowedTools = setOf("shell"),
+                        metadata = mapOf("role" to "reviewer"),
+                    ),
+                    StageId("recovery") to StageConfig(
+                        allowedTools = setOf("shell", "file_write"),
+                        metadata = mapOf("role" to "recovery"),
+                    ),
+                ),
+                transitions = setOf(
+                    TransitionEdge(TransitionId("impl->review"), StageId("impl"), StageId("review")) { true },
+                    TransitionEdge(TransitionId("review->impl"), StageId("review"), StageId("impl")) { true },
+                ),
+                start = StageId("impl"),
+            )
+
+            val result = orchestrator.run(sessionId, graph, OrchestrationConfig())
+
+            assertTrue(result is WorkflowResult.Failed)
+            val payloads = eventStore.read(sessionId).map { it.payload }
+            val reviewBackEdges = payloads.filterIsInstance<TransitionExecutedEvent>()
+                .filter { it.from == StageId("review") && it.to == StageId("impl") }
+            assertEquals(3, reviewBackEdges.size, "the fourth rejected review must not enter rework")
+            val ticket = payloads.filterIsInstance<FailureTicketOpenedEvent>()
+                .single { it.gate == "review_loop" }
+            assertEquals(StageId("review"), ticket.stageId)
+            assertEquals(StageId("recovery"), ticket.routeTo)
+            assertTrue(payloads.any { it is WorkflowFailedEvent })
+        }
+
    @Test
    fun `write-less gate failure routes the ticket to the file's author (route-to-owner)`(): Unit = runBlocking {
        val (orchestrator, eventStore) = buildMultiToolOrchestrator()
@@ -45,6 +45,10 @@ class LaunchRegistrationRaceTest {
            }.awaitAll()
        }

+        // computeIfAbsent returns the Job as soon as it's created; the launched body (which
+        // increments launchCount) runs independently on `scope`. Join the registered jobs so the
+        // increments have happened before we assert — otherwise launchCount races the launch bodies.
+        registry.values.forEach { it.join() }
        assertEquals(1, launchCount.get(), "mapping lambda must execute exactly once")
        assertEquals(1, registry.size, "registry must contain exactly one entry")
        scope.cancel()
@@ -69,6 +73,9 @@ class LaunchRegistrationRaceTest {
            }.awaitAll()
        }

+        // Join the launched bodies before asserting: computeIfAbsent returns the Job immediately,
+        // but the increment runs on `scope` independently and may lag awaitAll (the 49-vs-50 flake).
+        registry.values.forEach { it.join() }
        assertEquals(50, launchCount.get(), "each distinct key must execute its lambda")
        assertEquals(50, registry.size, "registry must contain 50 entries")
        scope.cancel()