From 1b58bc325ec3c66a94910a90c7453f73c8047cf2 Mon Sep 17 00:00:00 2001 From: kami Date: Sun, 19 Jul 2026 01:20:37 +0400 Subject: [PATCH] wip(freestyle/acr): grounding & edit-tool fixes + ACR-compiler experiment MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- apps/server/AGENTS.md | 1 + .../com/correx/apps/server/BootWorkspace.kt | 35 ++++ .../kotlin/com/correx/apps/server/Main.kt | 3 + .../com/correx/apps/server/ServerModule.kt | 20 ++- .../apps/server/freestyle/FreestyleDriver.kt | 39 ++++- .../correx/apps/server/BootWorkspaceTest.kt | 61 +++++++ core/config/AGENTS.md | 1 + .../com/correx/core/config/ConfigLoader.kt | 3 + .../com/correx/core/config/CorrexConfig.kt | 2 + .../correx/core/config/CorrexConfigWriter.kt | 1 + core/events/AGENTS.md | 1 + .../core/events/events/LspDiagnosticEvents.kt | 27 ++++ .../events/serialization/Serialization.kt | 2 + ...gnosticsCompletedEventSerializationTest.kt | 23 +++ core/kernel/AGENTS.md | 2 + .../DefaultSessionOrchestrator.kt | 16 +- .../DefaultSessionOrchestratorStep.kt | 51 +++++- .../orchestration/LspDiagnosticsRunner.kt | 20 +++ .../orchestration/OrchestrationTuning.kt | 2 + .../orchestration/OrchestratorEngines.kt | 1 + .../ProcessStaticAnalysisRunner.kt | 58 +++++-- .../orchestration/SessionOrchestrator.kt | 1 + .../SessionOrchestratorArtifacts.kt | 11 +- .../SessionOrchestratorContext.kt | 10 +- .../SessionOrchestratorExecution.kt | 25 ++- .../orchestration/SessionOrchestratorGates.kt | 1 + .../SessionOrchestratorGates2.kt | 56 +++++++ .../ProcessStaticAnalysisRunnerTest.kt | 17 ++ docs/AGENTS.md | 2 +- docs/WHATS-NEW-since-epic-14.md | 150 ++++++++++++++++++ docs/sample-config.toml | 9 ++ docs/schemas/discovery.json | 16 +- docs/schemas/dod.json | 24 +++ docs/schemas/execution_plan.json | 4 + examples/AGENTS.md | 1 + examples/workflows/artifacts.config.toml | 5 + examples/workflows/freestyle_planning.toml | 9 +- .../workflows/prompts/analyst_freestyle.md | 56 +++---- .../workflows/prompts/architect_freestyle.md | 13 +- examples/workflows/prompts/discovery.md | 45 +++++- .../tools/filesystem/FileEditTool.kt | 39 ++++- .../tools/filesystem/FileEditToolTest.kt | 28 ++++ infrastructure/workflow/AGENTS.md | 4 +- infrastructure/workflow/build.gradle | 2 + .../workflow/ExecutionPlanCompiler.kt | 48 +++++- .../workflow/ExecutionPlanModel.kt | 1 + .../workflow/Lsp4jDiagnosticsRunner.kt | 142 +++++++++++++++++ .../infrastructure/workflow/PlanGrounder.kt | 25 ++- .../infrastructure/workflow/PlanLinter.kt | 2 +- .../workflow/ExecutionPlanCompilerTest.kt | 57 +++++++ .../workflow/FreestylePlanningWorkflowTest.kt | 3 +- .../workflow/LspServerRegistryTest.kt | 22 +++ .../workflow/PlanGrounderTest.kt | 7 + testing/integration/AGENTS.md | 2 +- .../src/test/kotlin/ClarificationLoopTest.kt | 23 +-- .../src/test/kotlin/RecoveryRoutingTest.kt | 84 +++++++++- .../test/kotlin/LaunchRegistrationRaceTest.kt | 7 + 57 files changed, 1205 insertions(+), 115 deletions(-) create mode 100644 apps/server/src/main/kotlin/com/correx/apps/server/BootWorkspace.kt create mode 100644 apps/server/src/test/kotlin/com/correx/apps/server/BootWorkspaceTest.kt create mode 100644 core/events/src/main/kotlin/com/correx/core/events/events/LspDiagnosticEvents.kt create mode 100644 core/events/src/test/kotlin/com/correx/core/events/serialization/LspDiagnosticsCompletedEventSerializationTest.kt create mode 100644 core/kernel/src/main/kotlin/com/correx/core/kernel/orchestration/LspDiagnosticsRunner.kt create mode 100644 docs/WHATS-NEW-since-epic-14.md create mode 100644 docs/schemas/dod.json create mode 100644 infrastructure/workflow/src/main/kotlin/com/correx/infrastructure/workflow/Lsp4jDiagnosticsRunner.kt create mode 100644 infrastructure/workflow/src/test/kotlin/com/correx/infrastructure/workflow/LspServerRegistryTest.kt diff --git a/apps/server/AGENTS.md b/apps/server/AGENTS.md index eaa8dcd6..fd5a36c0 100644 --- a/apps/server/AGENTS.md +++ b/apps/server/AGENTS.md @@ -21,6 +21,7 @@ All sources under `apps/server/src/`. - `GET /metrics/tool-reliability` — per-model tool-call validity across the event log (`ToolReliabilityInspectionService`); groundwork for capability-aware routing - Optional `[git]` transport creates `run/` from a server-local checkout and pushes it at terminal state; clients review with ordinary Git and never supply a remote URL as `cwd`. - Repo-map L3 embeddings use bounded, recorded source descriptors (module/package, imports, leading purpose comment, symbols); raw file bodies are never embedded. Their versioned `repomap:v2` namespace forces a one-time re-embed when the semantic document format changes. +- At boot, `tools.workspace_root` is the authoritative tool jail and project-observation root. A configured `tools.working_dir` may only remain distinct when it is contained by that root; an outside value is clamped to `workspace_root`. Project memory and repo-map indexing are also rebound to `workspace_root`, so a stale `[project].root` cannot inject files from outside the session workspace. ### WebSocket protocol (`/ws`) - **ServerMessage** (server → client): sealed hierarchy — `SessionMessage` (event-derived, carries `sequence` + `sessionSequence`) and `NonEventMessage` (control/infra). Variants include session lifecycle, approval requests, clarification requests, narration, proposed workflows, health/metrics pushes. diff --git a/apps/server/src/main/kotlin/com/correx/apps/server/BootWorkspace.kt b/apps/server/src/main/kotlin/com/correx/apps/server/BootWorkspace.kt new file mode 100644 index 00000000..7cf22703 --- /dev/null +++ b/apps/server/src/main/kotlin/com/correx/apps/server/BootWorkspace.kt @@ -0,0 +1,35 @@ +package com.correx.apps.server + +import com.correx.core.config.ProjectConfig +import java.nio.file.Path + +internal data class BootWorkspace( + val workspaceRoot: Path, + val workingDir: Path, + val workingDirWasClamped: Boolean, +) + +internal fun resolveBootWorkspace( + explicitWorkspaceRoot: Path?, + explicitWorkingDir: Path?, + processWorkingDir: Path, +): BootWorkspace { + val workspaceRoot = (explicitWorkspaceRoot ?: explicitWorkingDir ?: processWorkingDir) + .toAbsolutePath() + .normalize() + val requestedWorkingDir = (explicitWorkingDir ?: workspaceRoot) + .toAbsolutePath() + .normalize() + val workingDirIsContained = requestedWorkingDir.startsWith(workspaceRoot) + + return BootWorkspace( + workspaceRoot = workspaceRoot, + workingDir = requestedWorkingDir.takeIf { workingDirIsContained } ?: workspaceRoot, + workingDirWasClamped = !workingDirIsContained, + ) +} + +/** Keep repo-map observation and L3 project memory inside the authoritative boot workspace. */ +internal fun ProjectConfig.boundToWorkspace(workspaceRoot: Path): ProjectConfig = copy( + root = workspaceRoot.toAbsolutePath().normalize().toString(), +) diff --git a/apps/server/src/main/kotlin/com/correx/apps/server/Main.kt b/apps/server/src/main/kotlin/com/correx/apps/server/Main.kt index 8a7b1998..c341680e 100644 --- a/apps/server/src/main/kotlin/com/correx/apps/server/Main.kt +++ b/apps/server/src/main/kotlin/com/correx/apps/server/Main.kt @@ -83,6 +83,7 @@ import com.correx.core.events.types.SessionId import com.correx.infrastructure.InfrastructureModule import com.correx.infrastructure.inference.DefaultProviderRegistry import com.correx.infrastructure.workflow.ExecutionPlanCompiler +import com.correx.infrastructure.workflow.Lsp4jDiagnosticsRunner import com.correx.infrastructure.workflow.PlanLinter import com.correx.infrastructure.inference.CapabilityAwareRoutingStrategy import com.correx.infrastructure.inference.commons.ManagedInferenceRouter @@ -384,6 +385,7 @@ fun main() { workspacePolicy = workspacePolicy, workspaceToolRegistryProvider = wsToolRegistryProvider, staticAnalysisRunner = ProcessStaticAnalysisRunner(), + lspDiagnosticsRunner = Lsp4jDiagnosticsRunner(), contractAssertionEvaluator = FileSystemContractEvaluator(), semanticReviewer = SemanticReviewerImpl(inferenceRouter), ) @@ -464,6 +466,7 @@ fun main() { maxClarificationRounds = maxClarificationRounds, reviewBlockMinConfidence = reviewBlockMinConfidence, reviewBlockRetryCap = reviewBlockRetryCap, + reviewLoopMaxCycles = reviewLoopMaxCycles, defaultMaxRefinement = defaultMaxRefinement, recoveryRouteBudget = recoveryRouteBudget, intentRouteBudget = intentRouteBudget, diff --git a/apps/server/src/main/kotlin/com/correx/apps/server/ServerModule.kt b/apps/server/src/main/kotlin/com/correx/apps/server/ServerModule.kt index f27e638c..6ac36c11 100644 --- a/apps/server/src/main/kotlin/com/correx/apps/server/ServerModule.kt +++ b/apps/server/src/main/kotlin/com/correx/apps/server/ServerModule.kt @@ -380,10 +380,11 @@ class ServerModule( // Record the repo map + seed prior-session memory before the run so stages // see both in context. projectMemory?.let { pm -> + val root = sessionWorkspaceRoot(sessionId) runCatching { - pm.observeAndRecord(sessionId, pm.repoRoot()) - pm.indexAndRecord(sessionId, pm.repoRoot()) - pm.retrieveAndSeed(sessionId, pm.repoRoot()) + pm.observeAndRecord(sessionId, root) + pm.indexAndRecord(sessionId, root) + pm.retrieveAndSeed(sessionId, root) } } // Bind operator profile snapshot as an event so replay reads the recorded @@ -416,7 +417,7 @@ class ServerModule( val result = orchestrator.run(sessionId, graph, sessionConfig) freestyleHandoff(sessionId, graph, result) // Distil this run's decisions into durable project memory on completion. - projectMemory?.let { pm -> pm.persist(sessionId, pm.repoRoot()) } + projectMemory?.let { pm -> pm.persist(sessionId, sessionWorkspaceRoot(sessionId)) } // Propose learned profile adaptations based on session journal (opt-in, never auto-applied). operatorProfile?.let { profile -> profileAdaptationService?.let { svc -> @@ -498,6 +499,17 @@ class ServerModule( * router chat triage, and replay read the recorded snapshot, never the live file * (invariants #8/#9). Shared by the workflow path and the chat-session path. */ + /** + * The session's bound workspace root — the same one the tool jail and [SessionWorkspaceBoundEvent] + * use. The repo-map/index/L3-memory pipeline MUST key off this, not [ProjectMemoryService.repoRoot] + * (a session-independent config/cwd default): when they diverge the repo map is computed for a + * different tree than the session operates in, so grounding "proves" real paths absent + * (session 5fe538f5, 2026-07-19). Falls back to the server default only when unbound. + */ + private fun sessionWorkspaceRoot(sessionId: SessionId): String = + runCatching { sessionRepository.getSession(sessionId).state.boundWorkspace?.workspaceRoot } + .getOrNull() ?: projectMemory?.repoRoot() ?: "." + suspend fun bindProjectProfile(sessionId: SessionId) { val workspaceRoot = runCatching { sessionRepository.getSession(sessionId).state.boundWorkspace?.workspaceRoot diff --git a/apps/server/src/main/kotlin/com/correx/apps/server/freestyle/FreestyleDriver.kt b/apps/server/src/main/kotlin/com/correx/apps/server/freestyle/FreestyleDriver.kt index 25c7f8a5..99b442cf 100644 --- a/apps/server/src/main/kotlin/com/correx/apps/server/freestyle/FreestyleDriver.kt +++ b/apps/server/src/main/kotlin/com/correx/apps/server/freestyle/FreestyleDriver.kt @@ -1,5 +1,6 @@ package com.correx.apps.server.freestyle +import com.correx.core.events.events.ArtifactValidatedEvent import com.correx.core.events.events.CapabilityGapDetectedEvent import com.correx.core.events.events.CapabilityGapReflectedEvent import com.correx.core.events.events.CapabilityGapVerdict @@ -12,6 +13,7 @@ import com.correx.core.events.events.PlanGroundingVerdict import com.correx.core.events.events.PlanLintCompletedEvent import com.correx.core.events.events.ProjectProfileBoundEvent import com.correx.core.events.events.RepoMapComputedEvent +import com.correx.core.events.events.WorkflowFailedEvent import com.correx.infrastructure.workflow.PlanGrounder import com.correx.core.tools.contract.ToolCapability import com.correx.infrastructure.workflow.CapabilityGap @@ -62,7 +64,7 @@ class FreestyleDriver( emitRejected(sessionId, "no execution_plan content produced by planning phase", "missing_content") return } - val graph = runCatching { compiler.compile(json, "freestyle-${sessionId.value}") } + val graph = runCatching { compiler.compile(json, "freestyle-${sessionId.value}", sessionArtifacts(sessionId)) } .getOrElse { log.error("freestyle: plan failed to compile: {}", it.message) emitRejected(sessionId, "plan failed to compile: ${it.message}", "compile") @@ -137,11 +139,17 @@ class FreestyleDriver( */ fun compiledGraph(sessionId: SessionId): WorkflowGraph? { val json = planContent(sessionId) ?: return null - return runCatching { compiler.compile(json, "freestyle-${sessionId.value}") } + return runCatching { compiler.compile(json, "freestyle-${sessionId.value}", sessionArtifacts(sessionId)) } .onFailure { log.error("freestyle: plan recompile for resume failed: {}", it.message) } .getOrNull() } + /** Artifact ids validated earlier in the session (e.g. planning-phase `dod`) for the #264 needs seam. */ + private fun sessionArtifacts(sessionId: SessionId): Set = + eventStore.read(sessionId) + .mapNotNull { (it.payload as? ArtifactValidatedEvent)?.artifactId?.value } + .toSet() + /** * Grounds [graph] against recorded workspace facts and emits [PlanGroundingEvaluatedEvent]. * Returns true if the plan was rejected (caller must stop before lock). Missing repo map/profile @@ -153,7 +161,10 @@ class FreestyleDriver( val repoMap = events.mapNotNull { it.payload as? RepoMapComputedEvent }.lastOrNull() val profile = events.mapNotNull { it.payload as? ProjectProfileBoundEvent }.lastOrNull() val paths = repoMap?.entries?.map { it.path }?.toSet().orEmpty() - val result = PlanGrounder.ground(graph, paths, profile?.commands.orEmpty()) + // No RepoMapComputedEvent = no scan ran, so `paths` is unknown, not "empty workspace". + // Tell the grounder not to prove paths absent from a set it never observed (the false + // "apps/server/** doesn't exist" reject); the build-manifest check still runs. + val result = PlanGrounder.ground(graph, paths, profile?.commands.orEmpty(), scanned = repoMap != null) eventStore.append( NewEvent( metadata = EventMetadata( @@ -307,6 +318,28 @@ class FreestyleDriver( ), ), ) + // A rejected plan ends the session — but the last workflow verdict on record was the + // planning phase's WorkflowCompleted, so the session read as SUCCESS (the "COMPLETED-lie"). + // Emit a session-terminal WorkflowFailed so the run's true outcome is on the log. stageId is + // architect: the plan's producer and where a fix (or future return-to-architect loop) lands. + eventStore.append( + NewEvent( + metadata = EventMetadata( + eventId = EventId(UUID.randomUUID().toString()), + sessionId = sessionId, + timestamp = Clock.System.now(), + schemaVersion = 1, + causationId = null, + correlationId = null, + ), + payload = WorkflowFailedEvent( + sessionId = sessionId, + stageId = StageId("architect"), + reason = "execution plan rejected ($source): $reason", + retryExhausted = false, + ), + ), + ) } companion object { diff --git a/apps/server/src/test/kotlin/com/correx/apps/server/BootWorkspaceTest.kt b/apps/server/src/test/kotlin/com/correx/apps/server/BootWorkspaceTest.kt new file mode 100644 index 00000000..b0d785cf --- /dev/null +++ b/apps/server/src/test/kotlin/com/correx/apps/server/BootWorkspaceTest.kt @@ -0,0 +1,61 @@ +package com.correx.apps.server + +import com.correx.core.config.ProjectConfig +import java.nio.file.Path +import kotlin.test.assertEquals +import kotlin.test.assertFalse +import kotlin.test.assertTrue +import org.junit.jupiter.api.Test + +class BootWorkspaceTest { + @Test + fun `workspace root clamps an outside configured working directory`() { + val resolved = resolveBootWorkspace( + explicitWorkspaceRoot = Path.of("/tmp/audition"), + explicitWorkingDir = Path.of("/home/user/repo"), + processWorkingDir = Path.of("/home/user/repo"), + ) + + assertEquals(Path.of("/tmp/audition"), resolved.workspaceRoot) + assertEquals(Path.of("/tmp/audition"), resolved.workingDir) + assertTrue(resolved.workingDirWasClamped) + } + + @Test + fun `working directory inside workspace root remains intact`() { + val resolved = resolveBootWorkspace( + explicitWorkspaceRoot = Path.of("/tmp/audition"), + explicitWorkingDir = Path.of("/tmp/audition/frontend"), + processWorkingDir = Path.of("/home/user/repo"), + ) + + assertEquals(Path.of("/tmp/audition"), resolved.workspaceRoot) + assertEquals(Path.of("/tmp/audition/frontend"), resolved.workingDir) + assertFalse(resolved.workingDirWasClamped) + } + + @Test + fun `configured working directory supplies the root when workspace root is absent`() { + val resolved = resolveBootWorkspace( + explicitWorkspaceRoot = null, + explicitWorkingDir = Path.of("/tmp/configured"), + processWorkingDir = Path.of("/home/user/repo"), + ) + + assertEquals(Path.of("/tmp/configured"), resolved.workspaceRoot) + assertEquals(Path.of("/tmp/configured"), resolved.workingDir) + assertFalse(resolved.workingDirWasClamped) + } + + @Test + fun `project memory root follows the authoritative workspace root`() { + val configured = ProjectConfig( + enabled = true, + root = "/home/user/repo", + ) + + val resolved = configured.boundToWorkspace(Path.of("/tmp/audition/../audition")) + + assertEquals("/tmp/audition", resolved.root) + } +} diff --git a/core/config/AGENTS.md b/core/config/AGENTS.md index 3db5a65c..de837e49 100644 --- a/core/config/AGENTS.md +++ b/core/config/AGENTS.md @@ -24,6 +24,7 @@ CORREX kernel team. Config schema changes affect all consumers — coordinate wi - `ConfigHolder` is the shared mutable reference injected into consumers. Never read the file directly in domain code — always go through `ConfigHolder`. - `CorrexConfigWriter` regenerates TOML from the in-memory model; round-tripping loses comments by design. - `[git]` is opt-in server transport configuration (`enabled`, `remote`, `base_branch`, optional `author`); it is off by default. +- `[orchestration].review_loop_max_cycles` bounds review→rework cycles before recovery escalation; default `3`. - `ModelConfig.contextSize` defaults to 24,576 tokens; explicit `context_size` remains the operator override for smaller local-model windows. ## Verification diff --git a/core/config/src/main/kotlin/com/correx/core/config/ConfigLoader.kt b/core/config/src/main/kotlin/com/correx/core/config/ConfigLoader.kt index 5ecaf9a4..b3e17da5 100644 --- a/core/config/src/main/kotlin/com/correx/core/config/ConfigLoader.kt +++ b/core/config/src/main/kotlin/com/correx/core/config/ConfigLoader.kt @@ -217,6 +217,7 @@ object ConfigLoader { private const val DEFAULT_MAX_CLARIFICATION_ROUNDS = 3 private const val DEFAULT_REVIEW_BLOCK_MIN_CONFIDENCE = 0.7 private const val DEFAULT_REVIEW_BLOCK_RETRY_CAP = 20 + private const val DEFAULT_REVIEW_LOOP_MAX_CYCLES = 3 private const val DEFAULT_MAX_REFINEMENT = 3 private const val DEFAULT_RECOVERY_ROUTE_BUDGET = 2 private const val DEFAULT_INTENT_ROUTE_BUDGET = 2 @@ -716,6 +717,8 @@ object ConfigLoader { reviewBlockMinConfidence = asDouble(orchestrationSection["review_block_min_confidence"], DEFAULT_REVIEW_BLOCK_MIN_CONFIDENCE), reviewBlockRetryCap = asInt(orchestrationSection["review_block_retry_cap"], DEFAULT_REVIEW_BLOCK_RETRY_CAP), + reviewLoopMaxCycles = + asInt(orchestrationSection["review_loop_max_cycles"], DEFAULT_REVIEW_LOOP_MAX_CYCLES), defaultMaxRefinement = asInt(orchestrationSection["default_max_refinement"], DEFAULT_MAX_REFINEMENT), recoveryRouteBudget = asInt(orchestrationSection["recovery_route_budget"], DEFAULT_RECOVERY_ROUTE_BUDGET), intentRouteBudget = asInt(orchestrationSection["intent_route_budget"], DEFAULT_INTENT_ROUTE_BUDGET), diff --git a/core/config/src/main/kotlin/com/correx/core/config/CorrexConfig.kt b/core/config/src/main/kotlin/com/correx/core/config/CorrexConfig.kt index aa6845de..2bfa809f 100644 --- a/core/config/src/main/kotlin/com/correx/core/config/CorrexConfig.kt +++ b/core/config/src/main/kotlin/com/correx/core/config/CorrexConfig.kt @@ -130,6 +130,8 @@ data class OrchestrationKnobs( val maxClarificationRounds: Int = 3, val reviewBlockMinConfidence: Double = 0.7, val reviewBlockRetryCap: Int = 20, + /** Review→rework cycles before deterministic escalation to the recovery stage. */ + val reviewLoopMaxCycles: Int = 3, val defaultMaxRefinement: Int = 3, val recoveryRouteBudget: Int = 2, val intentRouteBudget: Int = 2, diff --git a/core/config/src/main/kotlin/com/correx/core/config/CorrexConfigWriter.kt b/core/config/src/main/kotlin/com/correx/core/config/CorrexConfigWriter.kt index 7fec7c78..941de847 100644 --- a/core/config/src/main/kotlin/com/correx/core/config/CorrexConfigWriter.kt +++ b/core/config/src/main/kotlin/com/correx/core/config/CorrexConfigWriter.kt @@ -98,6 +98,7 @@ object CorrexConfigWriter { b.kv("max_clarification_rounds", cfg.orchestration.maxClarificationRounds) b.kv("review_block_min_confidence", cfg.orchestration.reviewBlockMinConfidence) b.kv("review_block_retry_cap", cfg.orchestration.reviewBlockRetryCap) + b.kv("review_loop_max_cycles", cfg.orchestration.reviewLoopMaxCycles) b.kv("default_max_refinement", cfg.orchestration.defaultMaxRefinement) b.kv("recovery_route_budget", cfg.orchestration.recoveryRouteBudget) b.kv("intent_route_budget", cfg.orchestration.intentRouteBudget) diff --git a/core/events/AGENTS.md b/core/events/AGENTS.md index 3a1466b1..5ac3843c 100644 --- a/core/events/AGENTS.md +++ b/core/events/AGENTS.md @@ -20,6 +20,7 @@ CORREX kernel team. This is the most cross-cutting module in the codebase — ch - `JsonEventSerializer` / `EventSerializer` — serialize/deserialize `StoredEvent` to JSON. - `EventDispatcher` — broadcasts events to in-process listeners. - Domain event files: `ApprovalEvents`, `ArtifactEvents`, `ContextEvents`, `InferenceEvents`, `OrchestrationEvents`, `RouterEvents`, `SessionEvents`, `TaskEvents`, `ToolEvents`, `IntentEvents`, `RiskAssessedEvent`, `JournalCompactedEvent`, and many more — all payload definitions live here. +- `LspDiagnosticsCompletedEvent` records pulled language-server diagnostics or a graceful skip reason; replay consumes this observation and never contacts the server. - Shared vocabulary: `IdentityTypes` (SessionId, TaskId, etc.), `Tier`, `TokenUsage`, `ToolReceipt`, `ToolRequest`, `RiskLevel`, `RetryPolicy`, `GrantScope`, `GrantLedger`. ## Work Guidance diff --git a/core/events/src/main/kotlin/com/correx/core/events/events/LspDiagnosticEvents.kt b/core/events/src/main/kotlin/com/correx/core/events/events/LspDiagnosticEvents.kt new file mode 100644 index 00000000..c9a9de32 --- /dev/null +++ b/core/events/src/main/kotlin/com/correx/core/events/events/LspDiagnosticEvents.kt @@ -0,0 +1,27 @@ +package com.correx.core.events.events + +import com.correx.core.events.types.SessionId +import com.correx.core.events.types.StageId +import kotlinx.serialization.SerialName +import kotlinx.serialization.Serializable + +@Serializable +data class LspDiagnostic( + val path: String, + val line: Int, + val character: Int, + val severity: String, + val code: String? = null, + val message: String, +) + +/** Recorded LSP 3.17 pull-diagnostic observation; replay never re-queries a language server. */ +@Serializable +@SerialName("LspDiagnosticsCompleted") +data class LspDiagnosticsCompletedEvent( + val sessionId: SessionId, + val stageId: StageId, + val server: String?, + val diagnostics: List, + val skippedReason: String? = null, +) : EventPayload diff --git a/core/events/src/main/kotlin/com/correx/core/events/serialization/Serialization.kt b/core/events/src/main/kotlin/com/correx/core/events/serialization/Serialization.kt index 2f3119ec..9c64ea5b 100644 --- a/core/events/src/main/kotlin/com/correx/core/events/serialization/Serialization.kt +++ b/core/events/src/main/kotlin/com/correx/core/events/serialization/Serialization.kt @@ -16,6 +16,7 @@ import com.correx.core.events.events.BriefEchoMismatchEvent import com.correx.core.events.events.BriefGroundingCheckedEvent import com.correx.core.events.events.ConceptPromotedEvent import com.correx.core.events.events.StaticAnalysisCompletedEvent +import com.correx.core.events.events.LspDiagnosticsCompletedEvent import com.correx.core.events.events.ContractGateEvaluatedEvent import com.correx.core.events.events.PlanLintCompletedEvent import com.correx.core.events.events.ChatSessionStartedEvent @@ -173,6 +174,7 @@ val eventModule = SerializersModule { subclass(BriefGroundingCheckedEvent::class) subclass(BriefEchoMismatchEvent::class) subclass(StaticAnalysisCompletedEvent::class) + subclass(LspDiagnosticsCompletedEvent::class) subclass(ContractGateEvaluatedEvent::class) subclass(PlanLintCompletedEvent::class) subclass(RiskAssessedEvent::class) diff --git a/core/events/src/test/kotlin/com/correx/core/events/serialization/LspDiagnosticsCompletedEventSerializationTest.kt b/core/events/src/test/kotlin/com/correx/core/events/serialization/LspDiagnosticsCompletedEventSerializationTest.kt new file mode 100644 index 00000000..633ca4a0 --- /dev/null +++ b/core/events/src/test/kotlin/com/correx/core/events/serialization/LspDiagnosticsCompletedEventSerializationTest.kt @@ -0,0 +1,23 @@ +package com.correx.core.events.serialization + +import com.correx.core.events.events.LspDiagnostic +import com.correx.core.events.events.LspDiagnosticsCompletedEvent +import com.correx.core.events.types.SessionId +import com.correx.core.events.types.StageId +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertIs + +class LspDiagnosticsCompletedEventSerializationTest { + @Test + fun `round-trips as polymorphic EventPayload`() { + val event = LspDiagnosticsCompletedEvent( + SessionId("s"), StageId("impl"), "tsserver", + listOf(LspDiagnostic("src/App.tsx", 1, 2, "error", "2322", "not assignable")), + ) + val encoded = eventJson.encodeToString(com.correx.core.events.events.EventPayload.serializer(), event) + val decoded = eventJson.decodeFromString(com.correx.core.events.events.EventPayload.serializer(), encoded) + assertIs(decoded) + assertEquals("src/App.tsx", decoded.diagnostics.single().path) + } +} diff --git a/core/kernel/AGENTS.md b/core/kernel/AGENTS.md index 9a3fec0a..df356a8b 100644 --- a/core/kernel/AGENTS.md +++ b/core/kernel/AGENTS.md @@ -19,6 +19,8 @@ CORREX kernel team. This is the integration point for all other `core/` modules. - `ReplayOrchestrator` / `ReplayInferenceProvider` / `ReplayStrategy` — deterministic replay of a session from its event log. `ReplayInferenceProvider` returns recorded responses — no live LLM (Hard Invariant #8). - `SubagentRunner` / `InSessionSubagentRunner` — runs sub-agent invocations within an active session. - `StaticAnalysisRunner` / `ProcessStaticAnalysisRunner` — runs static analysis tools and records results as events. +- `LspDiagnosticsRunner` — injected pull-diagnostics seam; diagnostics are filtered to stage-written files, recorded, and enforced before build/review. +- Review→rework loops use the configured three-cycle default, then route accumulated notes to recovery once and fail if the fixed DoD still cannot be approved. - `StageCheckpointReconciler` — reconciles checkpoint state across stage transitions. - Capability-gated failures first retry in place when the stage holds the required tool; an unchanged-fingerprint gate-budget exhaustion routes to the recovery/intent-holder stage when one is available, so capability possession alone cannot cause a frozen owner loop to fail the workflow. - Three repeated `REFERENCE_EXISTS` blocks for the same build prerequisite within one stage become a `workspace_precondition` gate failure, which is eligible for file-write recovery rather than remaining disconnected tool-call noise. diff --git a/core/kernel/src/main/kotlin/com/correx/core/kernel/orchestration/DefaultSessionOrchestrator.kt b/core/kernel/src/main/kotlin/com/correx/core/kernel/orchestration/DefaultSessionOrchestrator.kt index 2ad993e8..1a194818 100644 --- a/core/kernel/src/main/kotlin/com/correx/core/kernel/orchestration/DefaultSessionOrchestrator.kt +++ b/core/kernel/src/main/kotlin/com/correx/core/kernel/orchestration/DefaultSessionOrchestrator.kt @@ -15,6 +15,7 @@ import com.correx.core.events.events.OrchestrationResumedEvent import com.correx.core.events.events.SteeringNoteAddedEvent import com.correx.core.events.events.StoredEvent import com.correx.core.events.events.TransitionExecutedEvent +import com.correx.core.events.events.WorkflowStartedEvent import com.correx.core.events.orchestration.OrchestrationState import com.correx.core.events.types.ApprovalDecisionId import com.correx.core.events.types.ApprovalRequestId @@ -226,8 +227,8 @@ class DefaultSessionOrchestrator( /** * Delivers the operator's answers to a pending clarification raised by a stage. The waiting - * stage (parked in [requestClarificationIfNeeded]) completes and re-runs with the answers in - * context. If the server restarted while the clarification was pending there is no live + * stage (parked in [requestClarificationIfNeeded]) unparks and the run advances to the next + * stage with the answers in context. If the server restarted while the clarification was pending there is no live * coroutine to complete, so the answers are recorded directly and the session resumed. */ suspend fun submitClarification( @@ -280,10 +281,17 @@ class DefaultSessionOrchestrator( ) } -// A back-edge re-enters a stage already transitioned into this run (e.g. reviewer→implementer). +// A back-edge re-enters any stage already visited this run (e.g. reviewer→implementer). The start +// stage is visited via WorkflowStarted rather than TransitionExecuted, so it must participate too. // Top-level (not a member) to keep the orchestrator off the TooManyFunctions threshold. internal fun isBackEdge(events: List, target: StageId): Boolean = - events.any { (it.payload as? TransitionExecutedEvent)?.to == target } + events.any { + when (val payload = it.payload) { + is WorkflowStartedEvent -> payload.startStageId == target + is TransitionExecutedEvent -> payload.to == target + else -> false + } + } internal sealed class StepResult { data class Continue(val ctx: EnrichedExecutionContext) : StepResult() diff --git a/core/kernel/src/main/kotlin/com/correx/core/kernel/orchestration/DefaultSessionOrchestratorStep.kt b/core/kernel/src/main/kotlin/com/correx/core/kernel/orchestration/DefaultSessionOrchestratorStep.kt index 263c6bb9..6aa8b423 100644 --- a/core/kernel/src/main/kotlin/com/correx/core/kernel/orchestration/DefaultSessionOrchestratorStep.kt +++ b/core/kernel/src/main/kotlin/com/correx/core/kernel/orchestration/DefaultSessionOrchestratorStep.kt @@ -5,6 +5,7 @@ import com.correx.core.artifacts.ArtifactState import com.correx.core.events.events.ApprovalDecisionResolvedEvent import com.correx.core.events.events.ApprovalRequestedEvent import com.correx.core.events.events.ArtifactValidatedEvent +import com.correx.core.events.events.FailureTicketOpenedEvent import com.correx.core.events.events.RefinementIterationEvent import com.correx.core.events.events.RetrySalvageDecidedEvent import com.correx.core.events.events.SalvageDecision @@ -135,6 +136,7 @@ internal tailrec suspend fun DefaultSessionOrchestrator.step(ctx: EnrichedExecut * re-execute the same stage if attempts remain, else fail terminally with retryExhausted=true. */ +@Suppress("LongMethod", "ReturnCount", "NestedBlockDepth") internal suspend fun DefaultSessionOrchestrator.executeMove( ctx: EnrichedExecutionContext, decision: TransitionDecision.Move, @@ -154,10 +156,43 @@ internal suspend fun DefaultSessionOrchestrator.executeMove( // terminal failure instead of looping forever. if (isBackEdge(repositories.eventStore.read(ctx.sessionId), nextStageId)) { val cycleKey = "${ctx.currentStageId.value}->${nextStageId.value}" - val maxIterations = ctx.graph.stages[nextStageId]?.maxRetries ?: tuning.defaultMaxRefinement + val reviewerRole = ctx.graph.stages[ctx.currentStageId]?.metadata?.get("role")?.lowercase() + val reviewLoop = reviewerRole in setOf("review", "reviewer") + val maxIterations = if (reviewLoop) { + tuning.reviewLoopMaxCycles + } else { + ctx.graph.stages[nextStageId]?.maxRetries ?: tuning.defaultMaxRefinement + } val iteration = (orchestrationRepository.getState(ctx.sessionId).refinementIterations[cycleKey] ?: 0) + 1 emit(ctx.sessionId, RefinementIterationEvent(ctx.sessionId, cycleKey, iteration, maxIterations)) if (iteration > maxIterations) { + if (reviewLoop) { + val events = repositories.eventStore.read(ctx.sessionId) + val alreadyRecovered = events.mapNotNull { it.payload as? FailureTicketOpenedEvent } + .any { it.gate == REVIEW_LOOP_GATE && it.stageId == ctx.currentStageId } + val notes = ctx.graph.stages[ctx.currentStageId]?.produces + ?.mapNotNull { artifactContentCache["${ctx.sessionId.value}:${it.name.value}"] } + ?.joinToString("\n\n") + .orEmpty() + val reason = "review loop exhausted after exactly $maxIterations cycles. " + + "The fixed DoD was not approved. Accumulated review notes:\n" + + notes.ifBlank { "(review stage emitted no retained notes)" } + if (!alreadyRecovered) { + return routeToRecovery( + ctx, + ctx.currentStageId, + REVIEW_LOOP_GATE, + "review_convergence", + reason, + orchestrationRepository.getState(ctx.sessionId), + ) ?: StepResult.Terminal( + failWorkflow(ctx.sessionId, ctx.currentStageId, reason, retryExhausted = true), + ) + } + return StepResult.Terminal( + failWorkflow(ctx.sessionId, ctx.currentStageId, reason, retryExhausted = true), + ) + } return StepResult.Terminal( failWorkflow( ctx.sessionId, @@ -179,6 +214,8 @@ internal suspend fun DefaultSessionOrchestrator.executeMove( } } +private const val REVIEW_LOOP_GATE = "review_loop" + @Suppress("ReturnCount") internal suspend fun DefaultSessionOrchestrator.enterStage( ctx: EnrichedExecutionContext, @@ -224,11 +261,13 @@ internal suspend fun DefaultSessionOrchestrator.enterStage( ).outcome when (result) { is StageExecutionResult.Success -> { - if (requestClarificationIfNeeded(ctx.sessionId, stageId, ctx.graph)) { - // The stage raised open questions and the operator answered them; loop to - // re-run the stage with the answers injected (no failure-retry budget spent). - continue - } + // The stage may emit open questions in its artifact (discovery does). Park, let the + // operator answer, and record the answers — then ADVANCE, do not re-run the stage. + // A re-run restarts inference with no prior CoT, so the stage re-explores instead of + // converging (2026-07-18). The answers are recorded as ClarificationAnsweredEvents + // and injected into every later stage's L0 context (buildClarificationAnswerEntries), + // so the next stage (e.g. analyst) sees them without discovery running again. + requestClarificationIfNeeded(ctx.sessionId, stageId, ctx.graph) compactionService?.let { svc -> val journalState = decisionJournalRepository.getJournal(ctx.sessionId) val journalText = DecisionJournalRenderer().render(journalState) diff --git a/core/kernel/src/main/kotlin/com/correx/core/kernel/orchestration/LspDiagnosticsRunner.kt b/core/kernel/src/main/kotlin/com/correx/core/kernel/orchestration/LspDiagnosticsRunner.kt new file mode 100644 index 00000000..22e4c5b7 --- /dev/null +++ b/core/kernel/src/main/kotlin/com/correx/core/kernel/orchestration/LspDiagnosticsRunner.kt @@ -0,0 +1,20 @@ +package com.correx.core.kernel.orchestration + +import com.correx.core.events.events.LspDiagnostic +import java.nio.file.Path + +data class LspDiagnosticsRequest( + val workspaceRoot: Path, + val paths: List, +) + +data class LspDiagnosticsResult( + val server: String? = null, + val diagnostics: List = emptyList(), + val skippedReason: String? = null, +) + +/** Nondeterministic LSP boundary. The orchestrator records its result before using it. */ +fun interface LspDiagnosticsRunner { + suspend fun pull(request: LspDiagnosticsRequest): LspDiagnosticsResult +} diff --git a/core/kernel/src/main/kotlin/com/correx/core/kernel/orchestration/OrchestrationTuning.kt b/core/kernel/src/main/kotlin/com/correx/core/kernel/orchestration/OrchestrationTuning.kt index 173c2a88..48559435 100644 --- a/core/kernel/src/main/kotlin/com/correx/core/kernel/orchestration/OrchestrationTuning.kt +++ b/core/kernel/src/main/kotlin/com/correx/core/kernel/orchestration/OrchestrationTuning.kt @@ -35,6 +35,8 @@ data class OrchestrationTuning( val reviewBlockMinConfidence: Double = 0.7, /** Pathological backstop: max review-driven retries before the stage is let through. */ val reviewBlockRetryCap: Int = 20, + /** Review→rework cycles before deterministic escalation to recovery. */ + val reviewLoopMaxCycles: Int = 3, /** Max review→refine cycles for a stage (freestyle default refinement budget). */ val defaultMaxRefinement: Int = 3, /** Budget for routing a failed write-less stage to a recovery stage. */ diff --git a/core/kernel/src/main/kotlin/com/correx/core/kernel/orchestration/OrchestratorEngines.kt b/core/kernel/src/main/kotlin/com/correx/core/kernel/orchestration/OrchestratorEngines.kt index a00b2ae5..192bb011 100644 --- a/core/kernel/src/main/kotlin/com/correx/core/kernel/orchestration/OrchestratorEngines.kt +++ b/core/kernel/src/main/kotlin/com/correx/core/kernel/orchestration/OrchestratorEngines.kt @@ -31,6 +31,7 @@ data class OrchestratorEngines( // 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, + val lspDiagnosticsRunner: LspDiagnosticsRunner? = null, // Evaluates Gate 2 contract assertions against a stage's produced files (design §1). Null = no // contract gate; the deterministic funnel then rests on produces-presence + static analysis only. val contractAssertionEvaluator: ContractAssertionEvaluator? = null, diff --git a/core/kernel/src/main/kotlin/com/correx/core/kernel/orchestration/ProcessStaticAnalysisRunner.kt b/core/kernel/src/main/kotlin/com/correx/core/kernel/orchestration/ProcessStaticAnalysisRunner.kt index 4c86727d..7105dcb1 100644 --- a/core/kernel/src/main/kotlin/com/correx/core/kernel/orchestration/ProcessStaticAnalysisRunner.kt +++ b/core/kernel/src/main/kotlin/com/correx/core/kernel/orchestration/ProcessStaticAnalysisRunner.kt @@ -4,6 +4,7 @@ import com.correx.core.tools.process.ChildProcess import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.TimeoutCancellationException import kotlinx.coroutines.async +import kotlinx.coroutines.coroutineScope import kotlinx.coroutines.withContext import kotlinx.coroutines.withTimeout import java.nio.file.Path @@ -26,31 +27,70 @@ class ProcessStaticAnalysisRunner( withContext(Dispatchers.IO) { val argv = command.trim().split(WHITESPACE).filter { it.isNotEmpty() } if (argv.isEmpty()) return@withContext StaticAnalysisRunResult(EXIT_NOT_RUN, "empty command") + if (argv.first() == STATIC_FLOOR_COMMAND) return@withContext runStaticFloor(workingDir, argv) + runProcess(workingDir, argv, command) + } + + @Suppress("ReturnCount") + private suspend fun runStaticFloor(workingDir: Path, argv: List): StaticAnalysisRunResult { + val paths = argv.dropWhile { it != "--" }.drop(1) + if (paths.isEmpty()) return StaticAnalysisRunResult(0, "static floor skipped: no concrete files") + val checks = paths.mapNotNull { path -> checkerFor(path)?.let { it + path } } + if (checks.isEmpty()) { + return StaticAnalysisRunResult(0, "static floor skipped: no resolvable checker for ${paths.joinToString()}") + } + val outputs = mutableListOf() + for (check in checks) { + val result = runProcess(workingDir, check, check.joinToString(" ")) + outputs += "${check.joinToString(" ")}: ${result.output}".trim() + if (result.exitCode != 0) return StaticAnalysisRunResult(result.exitCode, outputs.joinToString("\n")) + } + val skipped = paths.size - checks.size + if (skipped > 0) outputs += "static floor skipped $skipped file(s) without a checker" + return StaticAnalysisRunResult(0, outputs.joinToString("\n")) + } + + private fun checkerFor(path: String): List? = when (path.substringAfterLast('.', "").lowercase()) { + "js", "mjs", "cjs" -> listOf("node", "--check") + "py" -> listOf("python3", "-m", "py_compile") + "sh", "bash" -> listOf("bash", "-n") + "rb" -> listOf("ruby", "-c") + else -> null + } + + private suspend fun runProcess( + workingDir: Path, + argv: List, + displayCommand: String, + ): StaticAnalysisRunResult { val process = runCatching { ChildProcess.builder(argv, workingDir.toFile()).redirectErrorStream(true).start() }.getOrElse { - return@withContext StaticAnalysisRunResult(EXIT_NOT_RUN, "failed to start '$command': ${it.message}") + return StaticAnalysisRunResult(EXIT_NOT_RUN, "failed to start '$displayCommand': ${it.message}") } - runCatching { - withTimeout(timeoutMs) { - val outputDeferred = async { process.inputStream.bufferedReader().use { it.readText() } } - val exit = process.waitFor() - StaticAnalysisRunResult(exit, outputDeferred.await()) + return runCatching { + coroutineScope { + 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" + "static analysis '$displayCommand' timed out after ${timeoutMs}ms" } else { - it.message ?: "error running '$command'" + it.message ?: "error running '$displayCommand'" } StaticAnalysisRunResult(EXIT_NOT_RUN, reason) } - } + } private companion object { const val DEFAULT_TIMEOUT_MS = 300_000L const val EXIT_NOT_RUN = -1 + const val STATIC_FLOOR_COMMAND = "correx-static-floor" val WHITESPACE = Regex("\\s+") } } diff --git a/core/kernel/src/main/kotlin/com/correx/core/kernel/orchestration/SessionOrchestrator.kt b/core/kernel/src/main/kotlin/com/correx/core/kernel/orchestration/SessionOrchestrator.kt index a928bf21..76439c49 100644 --- a/core/kernel/src/main/kotlin/com/correx/core/kernel/orchestration/SessionOrchestrator.kt +++ b/core/kernel/src/main/kotlin/com/correx/core/kernel/orchestration/SessionOrchestrator.kt @@ -212,6 +212,7 @@ abstract class SessionOrchestrator( internal val workspacePolicy: WorkspacePolicy? = engines.workspacePolicy internal val worldProbe: WorldProbe = engines.worldProbe internal val staticAnalysisRunner: StaticAnalysisRunner? = engines.staticAnalysisRunner + internal val lspDiagnosticsRunner: LspDiagnosticsRunner? = engines.lspDiagnosticsRunner internal val contractAssertionEvaluator: ContractAssertionEvaluator? = engines.contractAssertionEvaluator internal val planCompilationCheck: PlanCompilationCheck? = engines.planCompilationCheck internal val semanticReviewer: SemanticReviewer? = engines.semanticReviewer diff --git a/core/kernel/src/main/kotlin/com/correx/core/kernel/orchestration/SessionOrchestratorArtifacts.kt b/core/kernel/src/main/kotlin/com/correx/core/kernel/orchestration/SessionOrchestratorArtifacts.kt index 6bbce781..1fcda1ec 100644 --- a/core/kernel/src/main/kotlin/com/correx/core/kernel/orchestration/SessionOrchestratorArtifacts.kt +++ b/core/kernel/src/main/kotlin/com/correx/core/kernel/orchestration/SessionOrchestratorArtifacts.kt @@ -76,7 +76,7 @@ internal suspend fun SessionOrchestrator.repairArtifact( if (eligible && bestCandidate != null && !isCancelled(sessionId)) { emitArtifactRepairAttempted(sessionId, stageId, slot, unresolved.classification, "LLM") val repaired = runArtifactRepairInference( - sessionId, stageId, slot, bestCandidate, stageConfig, effectives, timeoutMs, + sessionId, stageId, slot, bestCandidate, unresolved.detail, stageConfig, effectives, timeoutMs, ) val reRun = repaired?.let { artifactExtractionPipeline.run(it, schema) } if (reRun is ArtifactExtractionPipeline.ExtractionResult.Resolved) { @@ -115,15 +115,18 @@ internal suspend fun SessionOrchestrator.runArtifactRepairInference( stageId: StageId, slot: TypedArtifactSlot, bestCandidate: String, + validationError: String, stageConfig: StageConfig, effectives: RunEffectives, timeoutMs: Long, ): String? { val schema = slot.kind.deriveJsonSchema() val schemaJson = Json.encodeToString(JsonSchema.serializer(), schema) - val prompt = "The previous output for the '${slot.name.value}' artifact was malformed and did not " + - "match the required schema.\n\nMalformed output:\n$bestCandidate\n\nReturn ONLY a single JSON " + - "object matching this schema. Do not add fields, prose, or code fences.\nSchema: $schemaJson" + val prompt = "The previous output for the '${slot.name.value}' artifact failed schema validation.\n\n" + + "Validation error:\n$validationError\n\nMalformed output:\n$bestCandidate\n\nFix ONLY what the " + + "validation error names — every field must sit at the level the schema defines; do not nest a " + + "top-level field inside another object. Return ONLY a single JSON object matching this schema. " + + "Do not add fields, prose, or code fences.\nSchema: $schemaJson" val entry = ContextEntry( id = ContextEntryId(UUID.randomUUID().toString()), layer = ContextLayer.L2, diff --git a/core/kernel/src/main/kotlin/com/correx/core/kernel/orchestration/SessionOrchestratorContext.kt b/core/kernel/src/main/kotlin/com/correx/core/kernel/orchestration/SessionOrchestratorContext.kt index 91c198f1..eae644d0 100644 --- a/core/kernel/src/main/kotlin/com/correx/core/kernel/orchestration/SessionOrchestratorContext.kt +++ b/core/kernel/src/main/kotlin/com/correx/core/kernel/orchestration/SessionOrchestratorContext.kt @@ -190,9 +190,13 @@ internal suspend fun SessionOrchestrator.buildClarificationAnswerEntries(session /** * If [stageId] just produced an LLM artifact carrying a non-empty `questions` array, park the * run: record the questions (invariant #9 — observed at parse time), await the operator's - * answers, record them, and return true so the caller re-runs the stage with the answers in - * context. Bounded by [MAX_CLARIFICATION_ROUNDS] so a stage that keeps re-asking eventually - * proceeds. Returns false when there are no questions or the round budget is spent. + * answers, and record them. The caller then ADVANCES to the next stage — the stage is not + * re-run. The answers are session-wide events, injected into every later stage's L0 context + * (see [buildClarificationAnswerEntries]), so the next stage acts on them directly; re-running + * the questioning stage only restarts inference with no prior reasoning and makes it re-explore. + * Bounded by [OrchestrationTuning.maxClarificationRounds] so retries can't re-park unboundedly. + * Returns true when it parked and collected answers, false when there are no questions or the + * round budget is spent. */ internal suspend fun SessionOrchestrator.requestClarificationIfNeeded( 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 902c5966..337e388e 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 @@ -374,12 +374,29 @@ internal suspend fun SessionOrchestrator.executeStage( continue } // emit_artifact is a meta-tool: the LLM calls it with the artifact's fields as arguments. - // Capture those arguments as the artifact content and exit the loop straight to validation - // (the post-loop capture honours this override instead of the empty assistant text). + // Validate those arguments INLINE, like any other tool call: a valid artifact captures and + // exits the loop; an invalid one is handed straight back into the SAME conversation as a + // tool-result error, so the model fixes it with its full context (and CoT) intact. Breaking + // the loop on a bad artifact would instead drop into a cold repair/retry that re-explores + // from scratch and tends to re-make the identical schema mistake (2026-07-18). val emitCall = inferenceResult.response.toolCalls.firstOrNull { it.function.name == EMIT_ARTIFACT_TOOL } if (emitCall != null && llmEmittedSlots.isNotEmpty()) { - llmArtifactOverride = emitCall.function.arguments - break + val emitSlot = llmEmittedSlots.first() + when (val res = artifactExtractionPipeline.run(emitCall.function.arguments, emitSlot.kind.deriveJsonSchema())) { + is ArtifactExtractionPipeline.ExtractionResult.Resolved -> { + llmArtifactOverride = res.canonicalJson.toString() + break + } + is ArtifactExtractionPipeline.ExtractionResult.Unresolved -> { + inferenceResult = pushBack( + "ERROR: your emit_artifact call for '${emitSlot.name.value}' did not match the " + + "schema: ${res.detail}. Call emit_artifact again with only that corrected. Every " + + "field must sit at the level the schema defines — do not nest a top-level field " + + "(e.g. ready, questions) inside another object.", + ) + continue + } + } } // stage_complete is a meta-tool: the LLM calls it to signal the stage's goal is met. // Skip tool dispatch — the loop exits and normal validation/transition follows diff --git a/core/kernel/src/main/kotlin/com/correx/core/kernel/orchestration/SessionOrchestratorGates.kt b/core/kernel/src/main/kotlin/com/correx/core/kernel/orchestration/SessionOrchestratorGates.kt index fb0e2a36..7bd27e3b 100644 --- a/core/kernel/src/main/kotlin/com/correx/core/kernel/orchestration/SessionOrchestratorGates.kt +++ b/core/kernel/src/main/kotlin/com/correx/core/kernel/orchestration/SessionOrchestratorGates.kt @@ -199,6 +199,7 @@ internal suspend fun SessionOrchestrator.runPostStageGates( { runContractGate(sessionId, stageId, stageConfig, effectives) }, { runPlanCompileGate(sessionId, stageId, stageConfig) }, { runStaticAnalysis(sessionId, stageId, stageConfig, effectives) }, + { runLspDiagnostics(sessionId, stageId, stageConfig, effectives) }, { runExecutionGate(sessionId, stageId, stageConfig, effectives, profileCommands) }, { runReviewGate(sessionId, stageId, stageConfig, effectives) }, ) 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 8d1bb944..9a005d59 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 @@ -5,6 +5,7 @@ import com.correx.core.events.events.ReviewVerdict import com.correx.core.events.events.ArtifactCreatedEvent import com.correx.core.events.events.StaticAnalysisCompletedEvent import com.correx.core.events.events.StaticAnalysisFinding +import com.correx.core.events.events.LspDiagnosticsCompletedEvent import com.correx.core.events.events.ArtifactValidatedEvent import com.correx.core.events.events.ArtifactValidatingEvent import com.correx.core.events.types.SessionId @@ -66,6 +67,55 @@ internal suspend fun SessionOrchestrator.runStaticAnalysis( ) } +@Suppress("ReturnCount") +internal suspend fun SessionOrchestrator.runLspDiagnostics( + sessionId: SessionId, + stageId: StageId, + stageConfig: StageConfig, + effectives: RunEffectives, +): StageExecutionResult { + val runner = lspDiagnosticsRunner ?: return StageExecutionResult.Success(emptyList()) + val workspaceRoot = effectives.policy?.workspaceRoot ?: return StageExecutionResult.Success(emptyList()) + val stagePaths = stageWrittenPaths(sessionId, stageId) + val paths = if (stagePaths.isNotEmpty()) { + stagePaths + } else if (stageConfig.autoBuildGate) { + sessionWrittenPaths(sessionId) + } else { + emptyList() + } + if (paths.isEmpty()) return StageExecutionResult.Success(emptyList()) + + val result = runner.pull(LspDiagnosticsRequest(workspaceRoot, paths)) + val plannedButUnwritten = stageConfig.writeManifest.filterNot { it in paths } + val diagnostics = result.diagnostics.filter { diagnostic -> + diagnostic.path in paths && plannedButUnwritten.none { planned -> + diagnostic.message.contains(planned.substringAfterLast('/'), ignoreCase = true) + } + } + emit( + sessionId, + LspDiagnosticsCompletedEvent(sessionId, stageId, result.server, diagnostics, result.skippedReason), + ) + val errors = diagnostics.filter { it.severity.equals("error", ignoreCase = true) } + if (errors.isEmpty()) return StageExecutionResult.Success(emptyList()) + val detail = errors.joinToString("\n") { + "- ${it.path}:${it.line + 1}:${it.character + 1} ${it.code.orEmpty()} ${it.message}".trim() + } + return StageExecutionResult.Failure( + "stage ${stageId.value} has LSP diagnostics in files it wrote. Fix these before proceeding:\n$detail", + retryable = true, + gate = "lsp_diagnostics", + ) +} + +internal fun SessionOrchestrator.sessionWrittenPaths(sessionId: SessionId): List = + eventStore.read(sessionId) + .mapNotNull { it.payload as? com.correx.core.events.events.FileWrittenEvent } + .filter { it.postImageHash != null } + .map { it.path } + .distinct() + /** * Gate 4 — execution (staged-verification §1/§2). A stage's [StageConfig.buildExpectation] * (none/module/project/tests) resolves to the bound project profile's typecheck/build/test @@ -76,6 +126,7 @@ internal suspend fun SessionOrchestrator.runStaticAnalysis( * [StaticAnalysisCompletedEvent], invariant #9, so replay never re-runs it). */ +@Suppress("ReturnCount") internal suspend fun SessionOrchestrator.runExecutionGate( sessionId: SessionId, stageId: StageId, @@ -93,6 +144,11 @@ internal suspend fun SessionOrchestrator.runExecutionGate( stageConfig.autoBuildGate && sessionProducedBuildTarget(sessionId) -> BuildExpectation.PROJECT else -> null } + // LSP pull diagnostics are the compiler-front-end/typecheck gate. Keep PROJECT bundlers and + // TESTS as real commands, but do not invoke the profile's flat `typecheck` alias as well. + if (expectation == BuildExpectation.MODULE && lspDiagnosticsRunner != null) { + return StageExecutionResult.Success(emptyList()) + } val alias = expectation?.commandAlias ?: return StageExecutionResult.Success(emptyList()) val runner = staticAnalysisRunner val workspaceRoot = effectives.policy?.workspaceRoot diff --git a/core/kernel/src/test/kotlin/com/correx/core/kernel/orchestration/ProcessStaticAnalysisRunnerTest.kt b/core/kernel/src/test/kotlin/com/correx/core/kernel/orchestration/ProcessStaticAnalysisRunnerTest.kt index 37cdc32c..2161631c 100644 --- a/core/kernel/src/test/kotlin/com/correx/core/kernel/orchestration/ProcessStaticAnalysisRunnerTest.kt +++ b/core/kernel/src/test/kotlin/com/correx/core/kernel/orchestration/ProcessStaticAnalysisRunnerTest.kt @@ -51,4 +51,21 @@ class ProcessStaticAnalysisRunnerTest { val result = runner.run(dir, " ") assertTrue(result.exitCode != 0, "exit: ${result.exitCode}") } + + @Test + fun `static floor skips an unsupported filetype without failing`() = runBlocking { + val dir = createTempDirectory("sa-skip") + dir.resolve("Main.kt").writeText("fun main() = Unit") + val result = runner.run(dir, "correx-static-floor -- Main.kt") + assertEquals(0, result.exitCode) + assertTrue(result.output.contains("skipped"), "output: ${result.output}") + } + + @Test + fun `static floor checks javascript syntax without project dependencies`() = runBlocking { + val dir = createTempDirectory("sa-js") + dir.resolve("broken.js").writeText("const = ;") + val result = runner.run(dir, "correx-static-floor -- broken.js") + assertTrue(result.exitCode != 0, "exit: ${result.exitCode}") + } } diff --git a/docs/AGENTS.md b/docs/AGENTS.md index aa8bf5bf..555a14b4 100644 --- a/docs/AGENTS.md +++ b/docs/AGENTS.md @@ -14,7 +14,7 @@ Maintained alongside the features they describe. Any agent shipping a significan - `decisions/` — ADRs (adr-NNNN-*.md). Append-only. Never delete or retroactively alter a decided ADR; write a superseding one instead. - `qa/` — QA run plans (QA-*.md). Each maps to a shipped feature or an explicitly pending controlled audition. `TEMPLATE.md` is the canonical shape. `ENV.md` describes the required live environment. `README.md` explains the QA process. - `specs/` — feature specs by date-slug. Inputs for planned or in-flight work. -- `schemas/` — canonical JSON schemas for structured outputs (analysis, brief_echo, design, execution_plan, impl_plan). Shared across the router and validator. +- `schemas/` — canonical JSON schemas for structured outputs (analysis, discovery brief, DoD, brief_echo, design, execution_plan, impl_plan). Shared across the router and validator. - `epics/`, `modules/`, `diagrams/`, `design/`, `reviews/`, `visual/` — supporting reference material. **⚠️ STALE / DO NOT TRUST FOR STATUS:** diff --git a/docs/WHATS-NEW-since-epic-14.md b/docs/WHATS-NEW-since-epic-14.md new file mode 100644 index 00000000..4d2255d7 --- /dev/null +++ b/docs/WHATS-NEW-since-epic-14.md @@ -0,0 +1,150 @@ +# Correx — Catch-up for a ~2-month-old memory + +**If your last memory of Correx is the epic-13/epic-14 era (mid-May 2026), read this.** + +Back then the project was a *skeleton*: epics 0–14 had landed the module structure +(events → sessions → transitions → validation → approvals → artifacts → context → +tools → inference → kernel → infra → interfaces → router). It compiled and replayed, +but the orchestration loop was thin and mostly unproven against real model runs. + +Two months and ~250 feature commits later, the architecture is the *same* (all 9 Hard +Invariants still hold; the event log is still the only source of truth), but nearly the +entire *behavioral* layer — how a session actually drives a model through real work — +was built and hardened by live QA. This document groups the major additions. It is a +map, not a changelog; the dated files in `docs/plans/` are the real changelog. + +Work is no longer tracked as "epics." It's tracked as **plans** (`docs/plans/`) and a +**Vikunja backlog** (project_id 4). + +--- + +## The single biggest correction: the TUI is now Go + +The Kotlin TUI (Mosaic, then tamboui) was **retired**. The interactive terminal UI is +now a **Go / Bubble Tea** app at `apps/tui-go` — a WS client of the server. `apps/cli` +(Clikt, Kotlin) and `apps/server` (Ktor) are the only Gradle app modules; the old +`apps/desktop` / `apps/worker` stubs were deleted. If you remember tamboui, forget it. + +## New whole subsystems (did not meaningfully exist at epic-14) + +**Native task tracking (`core:tasks`).** A dependency-aware work graph: `task_decompose` +splits a goal into a DEPENDS_ON graph in one approval; tasks have claim gates +(dependency-hard-gate, cite-before-claim, duplicate-title guard), per-task history, +markdown export, a full REST surface, a `correx task` CLI, a TUI task board with +readiness, and **git-driven status** (a commit mention → IN_PROGRESS, a closing keyword +→ DONE, idempotent). The execution loop is now *tasked* — it replaced the old linear +role_pipeline. + +**Plane-2 tool-call intent validation (`core:toolintent`).** An anti-hallucination / +safety layer that assesses every tool call *before* it runs and records +`ToolCallAssessedEvent`: path-containment, read-before-write, reference-must-exist, +write-scope adherence, stale-write, exec-interpreter and network-host rules, and a +per-session egress allowlist. This is the layer that stops an agent citing files it +never read or writing outside its declared scope. + +**Compression pipeline (`infrastructure:compression`).** A staged, replay-safe context +compressor: level policy → fact sheet + strict allocation → tier summarizer → token +pruning + embedding relevance + ToMe merge → static-doc pruning (CLAUDE.md/AGENTS.md). +Distinct from the required/optional *bucket* rework in `core:context`. + +**Staged verification & review gates.** Stage output passes a gate ladder instead of +being trusted: build gate, plan-compile gate, contract/execution gate, and a +semantic-review gate (`core:critique`, with structured `CritiqueFinding` + per-model +calibration). Per-gate retry budgets with progress-aware charging and hybrid salvage; +static-first reviewer framing; capability-gap reflection; brief echo-back gate. + +**Research workflow.** `WebSearchTool` (local SearXNG), `WebFetchTool` (bounded fetch), +deterministic HTML→markdown extraction, a research workflow graph, batch source-fetch +approval, and dedicated `SourceFetched` / `LowQualityExtraction` events. On by default. + +**Cross-session repo memory (L3 + repo-map + project profile).** `RepoMapIndexer` +(symbols for Kotlin, GDScript, C#/Godot-Mono, markdown headings), `RepoKnowledgeRetriever` +with recency fallback, L3 ANN retrieval (`in_memory` or `turbovec` sidecar) recorded as +environment observations, `ProjectMemoryService`/`Distiller`, and `ProjectProfile` +(per-repo standing context at `.correx/project.toml`) injected as L0. Note: this is +*repo-scoped* memory — the "cross-session memory" anti-feature (conversational recall) +is still deliberately out. + +**Git run-branch transport (the "Gitea transport") — SHIPPED.** The server owns a Git +checkout; each run pushes to a run branch (`GitRunBranchTransport`, `RunBranchPushedEvent`); +`GitTaskSync`/`GitCommandCommitReader` drive task status from commits. "gitea" is just +the configured remote name; the transport is plain Git. + +**Observability & replay tooling (Epic 15, largely shipped).** Event-stream inspector +(`correx events`, `/sessions/{id}/events`), session replay + determinism digest, +causation-graph DOT export, metrics as a replayable projection (`correx stats`), +event-sourced health checks (EventStore latency, llama-server liveness, disk watermark, +ROCm/VRAM probe), and correlation-structured MDC logging. + +**Model lifecycle management (`[[models]]`).** Correx spawns/owns the llama-server +process: per-stage model selection, manual swap + pin, resource telemetry, VRAM gauge. +(The autonomous *residency scheduler* is still unbuilt — see below.) + +**Workspace scoping & undo.** `WorkspaceResolver` trust pipeline + `allowed_workspace_roots`, +per-session workspace-scoped tools/policy, client→server workspace handshake, crash-safe +atomic file writes with pre/post-image captured to CAS (`FileWrittenEvent`), and a +`FileMutationReverser` undo primitive (server endpoint + CLI). Agents can also READ +outside `workspace_root` via an operator approval prompt (writes stay jailed). + +**Approvals & grants.** Cross-session grant scopes (PROJECT/GLOBAL) + revoke, +grant-aware approval engine, risk rationale surfaced to the operator; approval gates now +block indefinitely instead of auto-rejecting on timeout, and steering actually affects +the run. + +**Router interaction layer.** Beyond CHAT+STEERING: grounded system prompts, triage +directive (no bare refusals), structured **workflow proposals** from triage, a +**clarification-question loop** (analyst asks, operator answers, producer-exit rehydrate +on reconnect), **rubber-duck idea board** (capture/feed/promote to project profile), and +grounded **narration** (pinned narration model, latency/token metrics surfaced). + +**Freestyle / execution-plan engine.** Two-phase planning→execution driver, +`ExecutionPlanCompiler` + `ExecutionPlanLockedEvent`, post-plan operator approval gate, +freestyle graph re-routing (deterministic override), soft-lock steering preemption, +plan grounding + positive-pattern mining + a stage-prompt audition rig. + +**Decision journal (`core:journal`).** Decision-journal projection injected into stage +context, salience-aware compaction (`JournalCompactionService`, CAS-rendered). + +**Personalization (`core:config`).** `OperatorProfile` + `ProfileLoader` bound at start +and surfaced into L0; propose-only `ProfileAdaptationService`. + +**Native MCP host.** The server mounts external stdio MCP servers as first-class Correx +tools (inherit tier / receipt / replay). `codebase-memory-mcp` is installed and granted +per code-intel stage. + +**Recovery loop.** Failed write-less stages route to recovery instead of futile retry; +tier-2 intent-holder arbiter; remaining-delta pinning; failure-ticket routing + +review-gate RECOVER + return-to-sender; structured package-404 recovery evidence; +stage-global repeated-failure loop breaker. + +**Godot / game-dev support.** GDScript + C#/Godot-Mono symbol extraction in the repo +map, and a structural `.tscn`/`.tres` scene validator. + +## Refactors & hardening you'll notice + +- `DefaultSessionOrchestrator` god-class decomposed into per-concern extensions; + `DomainEventMapper`'s god-`when` split per-domain. Detekt baseline ratcheted 120 → 90. +- Persistence hardened: sequences assigned atomically inside INSERT, reads serialized + with the append transaction, slow WS clients no longer stall the kernel. +- Tool results bounded + framed, full output spilled to CAS with a `tool_output` + retrieval tool; `glob`/`grep` search tools; web_fetch SSRF fix; interruptible shell + timeouts with process-tree kill; shell allowlist validates every command position. +- Config: orchestration loop/threshold/budget constants moved to `[orchestration]`; + operator-tunable sampling knobs (top_k/min_p/repeat_penalty) per stage. + +## Still planned / deliberately NOT built (don't assume these exist) + +- Full **remote always-on / thin-client** deployment topology. Its Git *transport* + shipped (above); the hosting topology itself is not yet stood up. +- Full router **context isolation** (persistent conversation history, session-scoped + boundaries). CHAT+STEERING works; the isolation layer is deferred. +- Autonomous GPU **residency scheduling** (lifecycle *is* built; idle-eviction policy + is not). +- Still explicitly out: parallel agent execution, conversational cross-session memory, + three-critic ensemble, fine-tuning, streaming inference. + +## Where to look now + +- `CLAUDE.md` — current session guide, Module Map, invariants. +- `docs/plans/` — the real changelog of intent since epic-14 (dated files). +- Vikunja project 4 — live backlog. `scripts/ctx.py ` — ranked file/symbol lookup. diff --git a/docs/sample-config.toml b/docs/sample-config.toml index 07f868a5..59afce39 100644 --- a/docs/sample-config.toml +++ b/docs/sample-config.toml @@ -135,6 +135,10 @@ backend = "in_memory" # or "turbovec" # schema_path = "schemas/review_report.json" # llm_emitted = true # +# Review loops escalate to the synthesized recovery stage after this many review→rework cycles. +# [orchestration] +# review_loop_max_cycles = 3 +# # The bundled examples/workflows/review_loop.toml uses this kind: the reviewer emits a # review_report whose `verdict` field gates the implementer↔reviewer loop via # artifact_field_equals transitions. See docs/schemas/review_report.json for the schema. @@ -143,3 +147,8 @@ backend = "in_memory" # or "turbovec" id = "execution_plan" schema_path = "schemas/execution_plan.json" llm_emitted = true + +[[artifacts]] +id = "dod" +schema_path = "schemas/dod.json" +llm_emitted = true diff --git a/docs/schemas/discovery.json b/docs/schemas/discovery.json index 860847bc..e0d475c6 100644 --- a/docs/schemas/discovery.json +++ b/docs/schemas/discovery.json @@ -1,6 +1,20 @@ { "type": "object", "properties": { + "brief": { + "type": "object", + "properties": { + "what": { "type": "string" }, + "why": { "type": "string" }, + "who": { "type": "array", "items": { "type": "string" } }, + "scope": { "type": "array", "items": { "type": "string" } }, + "non_goals": { "type": "array", "items": { "type": "string" } }, + "constraints": { "type": "array", "items": { "type": "string" } }, + "assumptions": { "type": "array", "items": { "type": "string" } } + }, + "required": ["what", "why", "who", "scope", "non_goals", "constraints", "assumptions"], + "additionalProperties": false + }, "ready": { "type": "boolean", "description": "true when the request is clear and grounded enough to plan; false when you are raising questions" @@ -33,6 +47,6 @@ } } }, - "required": ["ready", "questions"], + "required": ["brief", "ready", "questions"], "additionalProperties": false } diff --git a/docs/schemas/dod.json b/docs/schemas/dod.json new file mode 100644 index 00000000..48fdf61d --- /dev/null +++ b/docs/schemas/dod.json @@ -0,0 +1,24 @@ +{ + "type": "object", + "properties": { + "summary": { "type": "string" }, + "criteria": { + "type": "array", + "minItems": 1, + "items": { + "type": "object", + "properties": { + "id": { "type": "string" }, + "statement": { "type": "string" }, + "part": { "type": "string" }, + "verified_by": { "type": "string", "description": "one of: gate, reviewer" } + }, + "required": ["id", "statement", "part", "verified_by"], + "additionalProperties": false + } + }, + "out_of_scope": { "type": "array", "items": { "type": "string" } } + }, + "required": ["summary", "criteria", "out_of_scope"], + "additionalProperties": false +} diff --git a/docs/schemas/execution_plan.json b/docs/schemas/execution_plan.json index 72034c1d..ec2a5012 100644 --- a/docs/schemas/execution_plan.json +++ b/docs/schemas/execution_plan.json @@ -14,6 +14,10 @@ "produces": { "type": "string" }, "kind": { "type": "string" }, "tools": { "type": "array", "items": { "type": "string" } } + ,"writes": { "type": "array", "items": { "type": "string" } } + ,"touches": { "type": "array", "items": { "type": "string" } } + ,"build_expectation": { "type": "string", "description": "one of: none, module, project, tests" } + ,"semantic_review": { "type": "boolean" } }, "required": ["id", "prompt", "produces"] } diff --git a/examples/AGENTS.md b/examples/AGENTS.md index 2820fcb4..49bea466 100644 --- a/examples/AGENTS.md +++ b/examples/AGENTS.md @@ -22,6 +22,7 @@ Each TOML file in `workflows/` is a valid workflow loadable by the server. Keep - Add a new workflow example when a major new workflow type ships. - Prompts referenced by TOML files go in `workflows/prompts/`. - Freestyle architect prompts specify stage constraints, boundaries, and verification goals; they do not prescribe an exact resulting file list when the authoritative intent leaves implementation details open. +- Freestyle discovery emits a structured comprehension brief; the analyst emits the addressable `dod` artifact used as the fixed implementation/review rubric. - Do not add configs/plugins/stages stubs speculatively — populate when there is real content. ## Verification diff --git a/examples/workflows/artifacts.config.toml b/examples/workflows/artifacts.config.toml index c2dcbfd7..578757e6 100644 --- a/examples/workflows/artifacts.config.toml +++ b/examples/workflows/artifacts.config.toml @@ -19,6 +19,11 @@ id = "analysis" schema_path = "schemas/analysis.json" llm_emitted = true +[[artifacts]] +id = "dod" +schema_path = "schemas/dod.json" +llm_emitted = true + [[artifacts]] id = "design" schema_path = "schemas/design.json" diff --git a/examples/workflows/freestyle_planning.toml b/examples/workflows/freestyle_planning.toml index 5dd3fa43..407b99b8 100644 --- a/examples/workflows/freestyle_planning.toml +++ b/examples/workflows/freestyle_planning.toml @@ -12,7 +12,7 @@ allowed_tools = ["file_read", "list_dir", "shell"] token_budget = 16384 max_retries = 2 -# analyst writes no files, but it owns task framing: task_search/task_context (read-only) find +# analyst writes no files, but it owns task framing and the fixed definition of done: # existing work; task_create (T2, approval-gated — a task is an event-log entry, not a file write) # opens a single task; task_decompose (T2, one approval for the whole graph) splits a goal with # dependency seams into parent + DEPENDS_ON-linked children. Either way the analysis names the task @@ -20,7 +20,8 @@ max_retries = 2 [[stages]] id = "analyst" prompt = "prompts/analyst_freestyle.md" -produces = [{ name = "analysis", kind = "analysis" }] +needs = ["discovery"] +produces = [{ name = "dod", kind = "dod" }] allowed_tools = ["file_read", "list_dir", "shell", "task_search", "task_context", "task_create", "task_decompose"] token_budget = 16384 max_retries = 2 @@ -30,7 +31,7 @@ id = "architect" requires_approval = true inject_artifact_kinds = true prompt = "prompts/architect_freestyle.md" -needs = ["analysis"] +needs = ["dod"] produces = [{ name = "execution_plan", kind = "execution_plan" }] token_budget = 16384 max_retries = 2 @@ -47,7 +48,7 @@ id = "analyst-to-architect" from = "analyst" to = "architect" condition_type = "artifact_validated" -condition_artifact_id = "analysis" +condition_artifact_id = "dod" [[transitions]] id = "architect-to-done" diff --git a/examples/workflows/prompts/analyst_freestyle.md b/examples/workflows/prompts/analyst_freestyle.md index 603eb5df..7f73d06c 100644 --- a/examples/workflows/prompts/analyst_freestyle.md +++ b/examples/workflows/prompts/analyst_freestyle.md @@ -1,33 +1,35 @@ -You are the **Analyst** in freestyle mode. Understand the user's goal (in the decision history -above) and the code it touches. Read-only: `file_read` (also lists a directory's entries when -given a directory path), `ls`, `grep`, `cat`, `find`. +You are the **Analyst** in freestyle mode. Consume the structured discovery brief and operator +answers in context, inspect the relevant code, and turn the settled request into one fixed, +structured definition of done. Read-only tools: `file_read`, `list_dir`, `shell`, `task_search`, +and `task_context`. -Before deriving requirements, check for existing work: `task_search` for related, duplicate, or -blocking tasks and `task_context` to load any the goal names. Fold what you find into the -analysis rather than re-deriving it; flag a duplicate instead of restating it. +Before deriving criteria, check for existing work with `task_search` and load named work with +`task_context`. Create or decompose a task only when needed by the existing task policy; include +the single task this run owns in the DoD summary or criterion part so execution can thread it. -Then frame the work as a task (per the task policy): -- If a task already covers this work, name its id (e.g. `auth-142`) in the analysis. -- If the goal is a single coherent unit one run can carry to review, `task_create` one and name its - id. -- If the goal has **dependency seams** (a thing that must land before another) or **independent - review/handoff points** (a piece worth shipping or reviewing on its own), `task_decompose` it into - a parent epic + `DEPENDS_ON`-linked children — one approval for the whole graph. A session works - one task at a time, so the children are claimed by *later* runs as they unblock; don't over-split. -- After decomposing, **name in the analysis the single task this run will work** — the one already - ready (no unmet dependency, e.g. the scaffold). Leave the blocked siblings for future runs. +Emit the `dod` artifact once. Its criteria are the complete acceptance contract for this run: -Either way later stages thread the named task through the plan; the rest wait to be claimed. +- Give every criterion a stable id (`c1`, `c2`, …), a checkable statement, and its feature area. +- Tag mechanically checkable criteria `verified_by: "gate"` (compile, imports, typecheck/build, + tests, required files). The reviewer must not adjudicate these. +- Tag semantic or UX criteria `verified_by: "reviewer"`. +- Copy discovery `brief.non_goals` into `out_of_scope`; this is a hard review boundary. +- Cover the entire in-scope brief now. Later stages may not silently add criteria. -Produce the `analysis` artifact by calling the **`emit_artifact`** tool with these fields: -- `summary`: the goal in your own words. -- `requirements`: concrete, checkable requirements, one per line. -- `affected_areas`: files/modules likely involved, one per line. +Call `emit_artifact` with a JSON object matching this shape: +`{"summary": string, "criteria": [{"id": string, "statement": string, "part": string, +"verified_by": "gate" | "reviewer"}], "out_of_scope": [string]}`. -Call `emit_artifact` once you have read enough — do not write the JSON as a plain message. +Example: +```json +{ + "summary": "Deliver the bounded validation gate for task gate-42.", + "criteria": [ + {"id":"c1","statement":"The project typecheck passes before completion","part":"terminal gate","verified_by":"gate"}, + {"id":"c2","statement":"The operator sees the recorded diagnostic","part":"workflow UX","verified_by":"reviewer"} + ], + "out_of_scope": ["Changing the workflow topology"] +} +``` -Always produce the analysis — this is your single exit. Open questions and operator forks are the -**Discovery** stage's job, and it has already run before you: any ambiguity or contradiction the -user needed to resolve was raised and answered upstream, and those answers are in the decision -history above. Treat the request as settled, ground your requirements in what you actually read, -and do not ask the user anything. Do not design or plan yet. +Do not ask questions; discovery owns clarification. Do not design or implement. diff --git a/examples/workflows/prompts/architect_freestyle.md b/examples/workflows/prompts/architect_freestyle.md index 713bce68..aa2000f6 100644 --- a/examples/workflows/prompts/architect_freestyle.md +++ b/examples/workflows/prompts/architect_freestyle.md @@ -7,8 +7,8 @@ message; call `emit_artifact` and nothing else. ## Inputs available to you -- `analysis` artifact — structured findings from the analyst stage (goal, constraints, - risks, open questions). +- `dod` artifact — the fixed acceptance contract from the analyst. Every implementation and + review stage must consume it and must not widen it. - Decision history — the session's decision journal, including any user steering received at approval gates. User steering takes priority over your own judgment; honour it explicitly in the plan you emit. @@ -48,7 +48,7 @@ Emit a JSON object that validates against the `execution_plan` schema: ## Rules -**goal** — one sentence, derived from the analysis artifact and any user steering. +**goal** — one sentence, derived from the DoD artifact and any user steering. **stages** — ordered list; each stage must: - Have a unique `id` in `snake_case`. @@ -60,6 +60,8 @@ Emit a JSON object that validates against the `execution_plan` schema: llm-emitted kind. - Declare `needs`: every upstream artifact id the stage's prompt references. Every id in `needs` must be `produces`d by a strictly earlier stage. +- Every implementation and review stage must include the session-scoped `dod` artifact in + `needs`. The compiler also enforces this seam. - Include `tools` per stage as it needs them, using only names from this set: `file_read`, `file_write`, `file_edit`, `list_dir`, `shell`, `task_context`, `task_update`, `task_search`. Stages that write or edit files take the file set @@ -141,12 +143,15 @@ Emit a JSON object that validates against the `execution_plan` schema: `field`: `"verdict"`, `value`: `"approved"`, `operator`: `"eq"` → `to: "done"` - `"type": "artifact_field_equals"`, `artifact_id`: reviewer's produces id, `field`: `"verdict"`, `value`: `"approved"`, `operator`: `"neq"` → `to: ""` +- A reviewer prompt must use only DoD rows tagged `verified_by: reviewer`: approve iff every such + row is met and no `out_of_scope` item was introduced. It must cite failed criterion ids and may + not invent new requirements. Criteria tagged `gate` are already decided upstream. - Every stage except the first must have an inbound edge from an earlier stage; every stage must have an outbound edge. Unreachable stages fail to compile. ## Constraints -- Do not add stages, roles, or tools not justified by the analysis artifact. +- Do not add stages, roles, or tools not justified by the DoD artifact. - Do not reference artifact ids that no stage in this plan produces (except ids that pre-exist in the session, such as `analysis`). - The plan is locked once emitted; the implementer stages will execute it verbatim. Be diff --git a/examples/workflows/prompts/discovery.md b/examples/workflows/prompts/discovery.md index afaa041b..5ddb3ba1 100644 --- a/examples/workflows/prompts/discovery.md +++ b/examples/workflows/prompts/discovery.md @@ -22,24 +22,47 @@ Two checks, both grounded in what you actually read: endpoint. Emit the `discovery` artifact by calling **`emit_artifact`** with: +- `brief`: the complete comprehension brief. Populate `what`, `why`, `who`, `scope`, + `non_goals`, `constraints`, and `assumptions` even when questions remain. Use assumptions for + reasonable, visible defaults instead of parking on micro-decisions. - `ready`: `true` when the request is clear and grounded enough to hand to the analyst; `false` when you are raising questions. - `questions`: the open questions (empty when `ready` is true). Batch **all** of them into this one list — do not ask one at a time. Each entry is an object: - `prompt` (required): the question, in full. - - `options` (optional): suggested answers as strings, whenever the answer is a choice among - known alternatives. + - `options` (**required whenever the answer is a choice among known alternatives** — and it + almost always is: stack, library, endpoint, layout, priority are all choices among things you + can name). Provide 2–4 concrete prefilled answers as strings. An open-ended question with no + `options` is only acceptable when no candidate set exists at all. Empty `options` on a + choice-question is a defect — enumerate the real candidates you found in the repo. - `multiSelect` (optional, default false): true if more than one option may apply. - `header` (optional): a 1–2 word label (e.g. "Scope", "Stack", "Endpoint"). -**Default to proceeding.** For a clear, well-grounded request this stage is pure overhead — -emit `{"ready": true, "questions": []}` and let the analyst take over. Only ask when a genuine +**Default to proceeding.** First inspect enough of the repository to enumerate the whole question +surface, then ask every genuine operator-only question in one batch. For a clear, well-grounded +request, proceed with visible assumptions. Do not raise one question, re-enter, and discover +another question that the same initial inspection could have exposed. + +**Converge once answered.** If the decision history above already contains the operator's answers +to your questions, you are done vetting — emit the brief with `ready: true` and empty `questions`. +Do NOT re-explore the repo hunting for new questions after the operator has answered; fold their +answers into the brief and hand off. You get **one** clarification round: ask everything up front, +then commit. Endless re-inspection is a failure, not diligence. fork or contradiction blocks planning. Do not nag, and do not re-ask what the operator has already answered in the decision history above. Example (needs input): ```json { + "brief": { + "what": "Build a browser client for the existing session API.", + "why": "Operators need a visual session surface.", + "who": ["operators"], + "scope": ["browser session client"], + "non_goals": ["server protocol redesign"], + "constraints": ["reuse the existing endpoint"], + "assumptions": [] + }, "ready": false, "questions": [ {"prompt": "Which frontend stack should the UI target?", @@ -52,5 +75,17 @@ Example (needs input): Example (clear — the common case): ```json -{ "ready": true, "questions": [] } +{ + "brief": { + "what": "Add the requested deterministic validation gate.", + "why": "Prevent invalid output from reaching review.", + "who": ["workflow authors", "operators"], + "scope": ["gate execution and recorded verdict"], + "non_goals": ["workflow topology redesign"], + "constraints": ["replay uses recorded observations"], + "assumptions": ["existing event-store contracts remain authoritative"] + }, + "ready": true, + "questions": [] +} ``` diff --git a/infrastructure/tools/filesystem/src/main/kotlin/com/correx/infrastructure/tools/filesystem/FileEditTool.kt b/infrastructure/tools/filesystem/src/main/kotlin/com/correx/infrastructure/tools/filesystem/FileEditTool.kt index cfe0d179..870bfd2b 100644 --- a/infrastructure/tools/filesystem/src/main/kotlin/com/correx/infrastructure/tools/filesystem/FileEditTool.kt +++ b/infrastructure/tools/filesystem/src/main/kotlin/com/correx/infrastructure/tools/filesystem/FileEditTool.kt @@ -154,6 +154,12 @@ class FileEditTool( ) -> ValidationResult.Invalid("Missing 'target' or 'replacement' parameter for 'replace' operation.") + // Anchor validity is checkable now, so reject a bad target BEFORE the approval gate + // fires — mirroring read/write's file-exists / read-before-write pre-checks. Without + // this the operator is prompted to approve an edit that then dies "Target not found" + // at execute time. Recoverable: the model corrects the anchor and retries. + operation == "replace" -> validateReplaceAnchor(path, pathString, request) + operation == "patch" && !request.parameters.containsKey("patch") -> ValidationResult.Invalid("Missing 'patch' parameter for 'patch' operation.") @@ -177,6 +183,29 @@ class FileEditTool( private fun isPathAllowed(path: Path): Boolean = PathJail.isContained(path, normalizedAllowedPaths) + /** Valid iff the replace target occurs exactly once. Same 0/>1 messaging as [replace] at execute. */ + private fun validateReplaceAnchor(path: Path, pathString: String, request: ToolRequest): ValidationResult { + val target = request.parameters["target"] as? String ?: return ValidationResult.Valid + val content = Files.readString(path) + return when (val occurrences = content.split(target).size - 1) { + 1 -> ValidationResult.Valid + 0 -> ValidationResult.Invalid(targetNotFoundMessage(pathString, content, target)) + else -> ValidationResult.Invalid(targetAmbiguousMessage(pathString, occurrences)) + } + } + + private fun targetNotFoundMessage(pathString: String, content: String, target: String): String = + buildString { + append("Target not found in $pathString") + nearestLine(content, target)?.let { + append(". Did you mean (whitespace/content may differ): $it ?") + } + } + + private fun targetAmbiguousMessage(pathString: String, occurrences: Int): String = + "Target found $occurrences times in $pathString, expected exactly once — " + + "extend it with surrounding lines to make it unique." + private fun mapExceptionToValidationResult(e: Throwable): ValidationResult = when (e) { is InvalidPathException -> ValidationResult.Invalid("Invalid path format: ${e.message}") @@ -270,18 +299,12 @@ class FileEditTool( // always whitespace/content drift the model can correct from a nudge. 0 -> ToolResult.Failure( invocationId = request.invocationId, - reason = buildString { - append("Target not found in $pathString") - nearestLine(currentContent, target)?.let { - append(". Did you mean (whitespace/content may differ): $it ?") - } - }, + reason = targetNotFoundMessage(pathString, currentContent, target), recoverable = true, ) else -> ToolResult.Failure( invocationId = request.invocationId, - reason = "Target found $occurrences times in $pathString, expected exactly once — " + - "extend it with surrounding lines to make it unique.", + reason = targetAmbiguousMessage(pathString, occurrences), recoverable = true, ) } diff --git a/infrastructure/tools/filesystem/src/test/kotlin/com/correx/infrastructure/tools/filesystem/FileEditToolTest.kt b/infrastructure/tools/filesystem/src/test/kotlin/com/correx/infrastructure/tools/filesystem/FileEditToolTest.kt index 58dae2c0..be0d0d06 100644 --- a/infrastructure/tools/filesystem/src/test/kotlin/com/correx/infrastructure/tools/filesystem/FileEditToolTest.kt +++ b/infrastructure/tools/filesystem/src/test/kotlin/com/correx/infrastructure/tools/filesystem/FileEditToolTest.kt @@ -95,6 +95,34 @@ class FileEditToolTest { assertTrue((result as ValidationResult.Invalid).reason.contains("File not found")) } + @Test + fun `validateRequest rejects a replace whose target is absent or ambiguous, before execute`(): Unit = runBlocking { + val tempDir = Files.createTempDirectory("file_edit_anchor") + val filePath = tempDir.resolve("test.txt") + Files.writeString(filePath, "alpha\nbeta\nbeta\n") + val tool = FileEditTool(allowedPaths = setOf(tempDir)) + + val absent = tool.validateRequest( + createRequest(mapOf("operation" to "replace", "path" to filePath.toString(), + "target" to "gamma", "replacement" to "x")), + ) + assertTrue(absent is ValidationResult.Invalid) + assertTrue((absent as ValidationResult.Invalid).reason.contains("Target not found")) + + val ambiguous = tool.validateRequest( + createRequest(mapOf("operation" to "replace", "path" to filePath.toString(), + "target" to "beta", "replacement" to "x")), + ) + assertTrue(ambiguous is ValidationResult.Invalid) + assertTrue((ambiguous as ValidationResult.Invalid).reason.contains("expected exactly once")) + + val unique = tool.validateRequest( + createRequest(mapOf("operation" to "replace", "path" to filePath.toString(), + "target" to "alpha", "replacement" to "x")), + ) + assertEquals(ValidationResult.Valid, unique) + } + @Test fun `validateRequest returns Invalid for missing parameters`(): Unit = runBlocking { val tempDir = Files.createTempDirectory("file_edit_test") diff --git a/infrastructure/workflow/AGENTS.md b/infrastructure/workflow/AGENTS.md index ff20fb66..24a4a031 100644 --- a/infrastructure/workflow/AGENTS.md +++ b/infrastructure/workflow/AGENTS.md @@ -15,7 +15,9 @@ Adapter for `core:transitions` and `core:inference` workflow interfaces. Depends - `ExecutionPlanCompiler` converts the parsed `ExecutionPlanModel` into a runnable plan for `core:transitions`; compilation is deterministic given the same input model. - `PlanLinter` enforces structural rules (e.g. unreachable stages are rejected); linting failures throw `WorkflowValidationException`. - `PlanDerivedManifest` exposes the write manifest derived from a plan. -- For a plan with no explicit build expectation, `ExecutionPlanCompiler` marks every write-declaring stage for the runtime auto build gate; plans with no declared writes are not represented as compiler-verified. +- For a plan with no explicit build expectation, `ExecutionPlanCompiler` marks every write-declaring stage and the graph-terminal stage for the runtime auto build gate; plans with no declared writes are not represented as compiler-verified. +- When a plan declares a `dod` producer, freestyle implementation/review stages consume that session-scoped artifact; reviewer prompts are compiler-bound to its reviewer-owned criteria. +- `Lsp4jDiagnosticsRunner` uses LSP 3.17 pull diagnostics for registered languages and degrades to the static one-shot floor when a server is unavailable. ## Work Guidance diff --git a/infrastructure/workflow/build.gradle b/infrastructure/workflow/build.gradle index 40351109..b5d39b7b 100644 --- a/infrastructure/workflow/build.gradle +++ b/infrastructure/workflow/build.gradle @@ -9,6 +9,8 @@ dependencies { implementation(project(":core:events")) implementation(project(":core:artifacts")) implementation(project(":core:tools")) + implementation(project(":core:kernel")) + implementation("org.eclipse.lsp4j:org.eclipse.lsp4j:0.23.1") implementation("com.fasterxml.jackson.dataformat:jackson-dataformat-toml:2.17.0") implementation("com.fasterxml.jackson.module:jackson-module-kotlin:2.17.0") testImplementation "org.junit.jupiter:junit-jupiter" diff --git a/infrastructure/workflow/src/main/kotlin/com/correx/infrastructure/workflow/ExecutionPlanCompiler.kt b/infrastructure/workflow/src/main/kotlin/com/correx/infrastructure/workflow/ExecutionPlanCompiler.kt index 0d7391ab..1f3a0f1a 100644 --- a/infrastructure/workflow/src/main/kotlin/com/correx/infrastructure/workflow/ExecutionPlanCompiler.kt +++ b/infrastructure/workflow/src/main/kotlin/com/correx/infrastructure/workflow/ExecutionPlanCompiler.kt @@ -19,6 +19,8 @@ import java.nio.file.Path private const val TERMINAL = "done" private const val RECOVERY_STAGE = "recovery" +private const val DOD_ARTIFACT = "dod" +private const val STATIC_FLOOR_COMMAND = "correx-static-floor" // Synthesized recovery stage prompt. Freestyle plans are LLM-emitted and never declare a recovery // stage, but the kernel's retry-agency guard routes a write-less stage's unfixable gate failure to @@ -85,7 +87,16 @@ class ExecutionPlanCompiler( .disable(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES) .build() - fun compile(planJson: String, workflowId: String): WorkflowGraph { + /** + * @param sessionArtifactIds artifact ids already produced earlier in the same freestyle session + * (e.g. the planning-phase `dod`). The execution plan never re-declares these, so they must be + * passed in for the cross-workflow `needs` seam (#264) to thread the DoD into impl/review stages. + */ + fun compile( + planJson: String, + workflowId: String, + sessionArtifactIds: Set = emptySet(), + ): WorkflowGraph { val plan = runCatching { mapper.readValue(planJson) } .getOrElse { throw WorkflowValidationException("Failed to parse execution_plan JSON: ${it.message}") } if (plan.stages.isEmpty()) throw WorkflowValidationException("execution_plan has no stages") @@ -115,10 +126,13 @@ class ExecutionPlanCompiler( // FileWritten manifest — the plan's declared kinds don't reveal code-ness, but the written // paths do — so a docs-only plan is left alone and a code plan is always gated. val autoGateStages: Set = if (declaredExpectations.values.all { it == BuildExpectation.NONE }) { - plan.stages.filter { it.writes.any { path -> path.isNotBlank() } }.map { it.id }.toSet() + val writing = plan.stages.filter { it.writes.any { path -> path.isNotBlank() } }.map { it.id }.toSet() + if (writing.isEmpty()) emptySet() else writing + terminalStageId(plan) } else { emptySet() } + val sessionArtifacts = + plan.stages.mapNotNull { it.produces.takeIf(String::isNotBlank) }.toSet() + sessionArtifactIds val stageMap = plan.stages.associate { s -> // `produces` is the unique slot name referenced by needs/edges; `kind` selects the @@ -137,7 +151,7 @@ class ExecutionPlanCompiler( ).sorted() StageId(s.id) to StageConfig( produces = listOf(TypedArtifactSlot(name = ArtifactId(s.produces), kind = kind)), - needs = s.needs.map { ArtifactId(it) }.toSet(), + needs = (s.needs + requiredSessionArtifacts(s, sessionArtifacts)).map { ArtifactId(it) }.toSet(), // Every stage gets the full registered tool universe. The architect's per-stage // `tools` pick was advisory and mis-scoped stages (e.g. an edit-shaped stage granted // file_write but not file_edit → forced full-file rewrites; a repair stage unable to @@ -146,6 +160,7 @@ class ExecutionPlanCompiler( // architect's pick only when no universe was supplied (bare-constructor unit tests). allowedTools = knownTools.ifEmpty { s.tools.toSet() }, writeManifest = writeManifest, + staticAnalysis = staticFloorCommands(writeManifest), // Option A: the concrete (non-glob) subset of the declared writes are expected files // the contract gate will require to exist. Globs (src/**) are write-permission scopes, // not existence guarantees, so they are excluded here. @@ -156,7 +171,7 @@ class ExecutionPlanCompiler( semanticReview = s.semanticReview, tokenBudget = DEFAULT_STAGE_TOKEN_BUDGET, generationConfig = defaultStageGeneration, - metadata = mapOf("promptInline" to s.prompt), + metadata = mapOf("role" to s.role, "promptInline" to reviewerBoundPrompt(s, DOD_ARTIFACT in sessionArtifacts)), ) } // Inject a recovery stage unless the plan already declares one. Reached only by the kernel's @@ -291,6 +306,31 @@ class ExecutionPlanCompiler( private fun terminalStageId(plan: ExecutionPlanModel): String = plan.edges.firstOrNull { it.to == TERMINAL }?.from ?: plan.stages.last().id + private fun requiredSessionArtifacts(stage: PlanStage, sessionArtifacts: Set): List = + if ( + DOD_ARTIFACT in sessionArtifacts && + stage.role.lowercase() in setOf("impl", "implementer", "review", "reviewer") + ) { + listOf(DOD_ARTIFACT) + } else { + emptyList() + } + + private fun reviewerBoundPrompt(stage: PlanStage, dodPresent: Boolean): String { + if (!dodPresent || stage.role.lowercase() !in setOf("review", "reviewer")) return stage.prompt + return stage.prompt + "\n\n" + + "The `dod` input artifact is your sole acceptance rubric. Judge only criteria tagged " + + "`verified_by: reviewer`; deterministic gates own `verified_by: gate`. Approve if and only if " + + "every reviewer criterion is met and no `out_of_scope` item was introduced. On rejection, " + + "cite the unmet criterion ids. Never add or imply criteria outside the DoD." + } + + private fun staticFloorCommands(writeManifest: List): List { + return writeManifest.takeIf { it.isNotEmpty() } + ?.let { listOf("$STATIC_FLOOR_COMMAND -- ${it.joinToString(" ")}") } + ?: emptyList() + } + private fun isGlob(path: String): Boolean = path.any { it == '*' || it == '?' || it == '[' } private fun validateReachability( diff --git a/infrastructure/workflow/src/main/kotlin/com/correx/infrastructure/workflow/ExecutionPlanModel.kt b/infrastructure/workflow/src/main/kotlin/com/correx/infrastructure/workflow/ExecutionPlanModel.kt index e20d6a80..07d26099 100644 --- a/infrastructure/workflow/src/main/kotlin/com/correx/infrastructure/workflow/ExecutionPlanModel.kt +++ b/infrastructure/workflow/src/main/kotlin/com/correx/infrastructure/workflow/ExecutionPlanModel.kt @@ -10,6 +10,7 @@ data class ExecutionPlanModel( data class PlanStage( val id: String = "", + val role: String = "", val prompt: String = "", val produces: String = "", val kind: String? = null, diff --git a/infrastructure/workflow/src/main/kotlin/com/correx/infrastructure/workflow/Lsp4jDiagnosticsRunner.kt b/infrastructure/workflow/src/main/kotlin/com/correx/infrastructure/workflow/Lsp4jDiagnosticsRunner.kt new file mode 100644 index 00000000..e70dbdc7 --- /dev/null +++ b/infrastructure/workflow/src/main/kotlin/com/correx/infrastructure/workflow/Lsp4jDiagnosticsRunner.kt @@ -0,0 +1,142 @@ +package com.correx.infrastructure.workflow + +import com.correx.core.events.events.LspDiagnostic +import com.correx.core.kernel.orchestration.LspDiagnosticsRequest +import com.correx.core.kernel.orchestration.LspDiagnosticsResult +import com.correx.core.kernel.orchestration.LspDiagnosticsRunner +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.withContext +import org.eclipse.lsp4j.ClientCapabilities +import org.eclipse.lsp4j.DidOpenTextDocumentParams +import org.eclipse.lsp4j.DocumentDiagnosticParams +import org.eclipse.lsp4j.InitializeParams +import org.eclipse.lsp4j.InitializedParams +import org.eclipse.lsp4j.MessageActionItem +import org.eclipse.lsp4j.MessageParams +import org.eclipse.lsp4j.PublishDiagnosticsParams +import org.eclipse.lsp4j.ShowMessageRequestParams +import org.eclipse.lsp4j.TextDocumentIdentifier +import org.eclipse.lsp4j.TextDocumentItem +import org.eclipse.lsp4j.launch.LSPLauncher +import org.eclipse.lsp4j.services.LanguageClient +import java.nio.file.Files +import java.util.concurrent.CompletableFuture +import java.util.concurrent.TimeUnit +import kotlin.coroutines.cancellation.CancellationException + +data class LspServerSpec(val id: String, val command: List, val languageId: String) + +class LspServerRegistry( + private val byExtension: Map = defaults, +) { + fun serverFor(path: String): LspServerSpec? = byExtension[path.substringAfterLast('.', "").lowercase()] + + companion object { + private val typescript = LspServerSpec( + "tsserver", + listOf("typescript-language-server", "--stdio"), + "typescriptreact", + ) + private val rust = LspServerSpec("rust-analyzer", listOf("rust-analyzer"), "rust") + private val go = LspServerSpec("gopls", listOf("gopls"), "go") + private val clangd = LspServerSpec("clangd", listOf("clangd"), "cpp") + val defaults = mapOf( + "ts" to typescript.copy(languageId = "typescript"), + "tsx" to typescript, + "js" to typescript.copy(languageId = "javascript"), + "jsx" to typescript.copy(languageId = "javascriptreact"), + "rs" to rust, + "go" to go, + "c" to clangd.copy(languageId = "c"), + "cc" to clangd, + "cpp" to clangd, + "h" to clangd.copy(languageId = "c"), + "hpp" to clangd, + ) + } +} + +/** LSP 3.17 pull-diagnostics adapter. Missing servers degrade to the static one-shot floor. */ +class Lsp4jDiagnosticsRunner( + private val registry: LspServerRegistry = LspServerRegistry(), + private val timeoutSeconds: Long = 30, +) : LspDiagnosticsRunner { + override suspend fun pull(request: LspDiagnosticsRequest): LspDiagnosticsResult = withContext(Dispatchers.IO) { + val groups = request.paths + .mapNotNull { path -> registry.serverFor(path)?.let { it to path } } + .groupBy({ it.first }, { it.second }) + if (groups.isEmpty()) { + return@withContext LspDiagnosticsResult(skippedReason = "no language server registered for touched files") + } + val diagnostics = mutableListOf() + val skipped = mutableListOf() + groups.forEach { (server, paths) -> + runCatching { diagnostics += pullFromServer(request, server, paths) } + .onFailure { + if (it is CancellationException) throw it + skipped += "${server.id}: ${it.message ?: it::class.simpleName}" + } + } + LspDiagnosticsResult( + server = groups.keys.joinToString(",") { it.id }, + diagnostics = diagnostics, + skippedReason = skipped.takeIf { it.isNotEmpty() }?.joinToString("; "), + ) + } + + private fun pullFromServer( + request: LspDiagnosticsRequest, + spec: LspServerSpec, + paths: List, + ): List { + val process = ProcessBuilder(spec.command).directory(request.workspaceRoot.toFile()).start() + val stderrDrain = Thread.ofVirtual().start { process.errorStream.bufferedReader().use { it.readText() } } + val launcher = LSPLauncher.createClientLauncher(QuietLanguageClient, process.inputStream, process.outputStream) + val listening = launcher.startListening() + val server = launcher.remoteProxy + return try { + server.initialize( + InitializeParams().apply { + rootUri = request.workspaceRoot.toUri().toString() + capabilities = ClientCapabilities() + }, + ).get(timeoutSeconds, TimeUnit.SECONDS) + server.initialized(InitializedParams()) + paths.flatMap { path -> + val file = request.workspaceRoot.resolve(path).normalize() + val uri = file.toUri().toString() + server.textDocumentService.didOpen( + DidOpenTextDocumentParams(TextDocumentItem(uri, spec.languageId, 1, Files.readString(file))), + ) + val report = server.textDocumentService.diagnostic( + DocumentDiagnosticParams(TextDocumentIdentifier(uri)), + ).get(timeoutSeconds, TimeUnit.SECONDS) + report.relatedFullDocumentDiagnosticReport.items.orEmpty().map { diagnostic -> + LspDiagnostic( + path = path, + line = diagnostic.range.start.line, + character = diagnostic.range.start.character, + severity = diagnostic.severity?.name?.lowercase() ?: "error", + code = diagnostic.code?.let { if (it.isLeft) it.left else it.right.toString() }, + message = diagnostic.message, + ) + } + } + } finally { + runCatching { server.shutdown().get(timeoutSeconds, TimeUnit.SECONDS) } + server.exit() + listening.cancel(true) + process.destroy() + stderrDrain.join(TimeUnit.SECONDS.toMillis(1)) + } + } + + private object QuietLanguageClient : LanguageClient { + override fun telemetryEvent(`object`: Any?) = Unit + override fun publishDiagnostics(diagnostics: PublishDiagnosticsParams) = Unit + override fun showMessage(messageParams: MessageParams) = Unit + override fun showMessageRequest(requestParams: ShowMessageRequestParams): CompletableFuture = + CompletableFuture.completedFuture(null) + override fun logMessage(message: MessageParams) = Unit + } +} diff --git a/infrastructure/workflow/src/main/kotlin/com/correx/infrastructure/workflow/PlanGrounder.kt b/infrastructure/workflow/src/main/kotlin/com/correx/infrastructure/workflow/PlanGrounder.kt index a23860f1..d635d592 100644 --- a/infrastructure/workflow/src/main/kotlin/com/correx/infrastructure/workflow/PlanGrounder.kt +++ b/infrastructure/workflow/src/main/kotlin/com/correx/infrastructure/workflow/PlanGrounder.kt @@ -35,6 +35,13 @@ object PlanGrounder { graph: WorkflowGraph, repoMapPaths: Set, profileCommands: Map, + // False = no repo scan was recorded for this session, so [repoMapPaths] is "unknown", not + // "empty workspace". Scope grounding proves a path ABSENT, which an unknown set can't do — + // running it against a missing scan flags every real path as nonexistent (the apps/server + // "doesn't exist?!" false reject). Skip scope grounding then; the build-manifest check still + // runs (it degrades safely via manifestProducedByPlan). A recorded scan with 0 entries is a + // genuine greenfield workspace — scanned=true, so scope grounding still catches a missing scaffold. + scanned: Boolean = true, ): PlanGroundingResult { val manifestInWorkspace = repoMapPaths.any(::isBuildManifest) val manifestProducedByPlan = graph.stages.values @@ -60,14 +67,16 @@ object PlanGrounder { // scope that matches NOTHING in the recorded repo map AND that no plan stage creates a file // under is grounded on a directory that will never exist — the exact session-a35cf1e3 // "scoped to EXISTING frontend/ that isn't there" failure. Caught at stage 0, not stage 11. - cfg.touches - .filter { glob -> !scopeSatisfied(glob, repoMapPaths, createdPaths) } - .forEach { glob -> - add( - "stage ${id.value}: scoped to '$glob' but nothing under it exists in the repo map " + - "and no plan stage creates a file there — scaffold that path first or fix the scope.", - ) - } + if (scanned) { + cfg.touches + .filter { glob -> !scopeSatisfied(glob, repoMapPaths, createdPaths) } + .forEach { glob -> + add( + "stage ${id.value}: scoped to '$glob' but nothing under it exists in the repo map " + + "and no plan stage creates a file there — scaffold that path first or fix the scope.", + ) + } + } } } diff --git a/infrastructure/workflow/src/main/kotlin/com/correx/infrastructure/workflow/PlanLinter.kt b/infrastructure/workflow/src/main/kotlin/com/correx/infrastructure/workflow/PlanLinter.kt index 9d61860e..c946cb56 100644 --- a/infrastructure/workflow/src/main/kotlin/com/correx/infrastructure/workflow/PlanLinter.kt +++ b/infrastructure/workflow/src/main/kotlin/com/correx/infrastructure/workflow/PlanLinter.kt @@ -37,7 +37,7 @@ object PlanLinter { * planning phase, not by any plan stage. The architect prompt explicitly allows `needs` to * reference these (notably `analysis`), so the linter must treat them as available producers. */ - private val seedArtifacts = setOf("analysis") + private val seedArtifacts = setOf("analysis", "dod") fun lint(graph: WorkflowGraph, seeds: Set = seedArtifacts): PlanLintResult = PlanLintResult( diff --git a/infrastructure/workflow/src/test/kotlin/com/correx/infrastructure/workflow/ExecutionPlanCompilerTest.kt b/infrastructure/workflow/src/test/kotlin/com/correx/infrastructure/workflow/ExecutionPlanCompilerTest.kt index 7cb50b10..ee6d029c 100644 --- a/infrastructure/workflow/src/test/kotlin/com/correx/infrastructure/workflow/ExecutionPlanCompilerTest.kt +++ b/infrastructure/workflow/src/test/kotlin/com/correx/infrastructure/workflow/ExecutionPlanCompilerTest.kt @@ -499,6 +499,63 @@ class ExecutionPlanCompilerTest { graph.stages[StageId("entry")]!!.autoBuildGate, "implementation stages must not defer compiler coverage to a terminal verifier", ) + assertTrue(graph.stages[StageId("entry")]!!.staticAnalysis.single().contains("src/main.tsx")) + assertTrue(graph.stages[StageId("views")]!!.staticAnalysis.single().contains("src/App.tsx")) + } + + @Test + fun `non-writing review terminal receives the auto build gate`() { + val planned = """ + { + "goal": "implement then review", + "stages": [ + { "id": "impl", "role": "implementer", "prompt": "write code", "produces": "code", + "kind": "react_entry", "writes": ["src/main.tsx"] }, + { "id": "review", "role": "reviewer", "prompt": "review against dod", "produces": "verdict", + "kind": "react_component", "needs": ["code"] } + ], + "edges": [ + { "from": "impl", "to": "review", "condition": { "type": "always_true" } }, + { "from": "review", "to": "done", "condition": { "type": "always_true" } } + ] + } + """.trimIndent() + val graph = codeCompiler.compile(planned, "wf") + assertTrue(graph.stages.getValue(StageId("review")).autoBuildGate) + // No dod in session → nothing to thread (execution plan never re-produces it). + assertTrue(graph.stages.getValue(StageId("review")).needs.none { it.value == "dod" }) + + // Planning-phase dod present in the session → threaded into impl + review needs (#264 seam). + val withDod = codeCompiler.compile(planned, "wf", sessionArtifactIds = setOf("dod")) + assertTrue(withDod.stages.getValue(StageId("review")).needs.any { it.value == "dod" }) + assertTrue(withDod.stages.getValue(StageId("impl")).needs.any { it.value == "dod" }) + } + + @Test + fun `implementation and review consume dod only when the plan produces it`() { + val planned = """ + { + "goal": "define, implement, and review", + "stages": [ + { "id": "analyst", "role": "analyst", "prompt": "define dod", "produces": "dod", + "kind": "react_component" }, + { "id": "impl", "role": "implementer", "prompt": "write code", "produces": "code", + "kind": "react_entry", "writes": ["src/main.tsx"] }, + { "id": "review", "role": "reviewer", "prompt": "review", "produces": "verdict", + "kind": "react_component", "needs": ["code"] } + ], + "edges": [ + { "from": "analyst", "to": "impl", "condition": { "type": "always_true" } }, + { "from": "impl", "to": "review", "condition": { "type": "always_true" } }, + { "from": "review", "to": "done", "condition": { "type": "always_true" } } + ] + } + """.trimIndent() + + val graph = codeCompiler.compile(planned, "wf-with-dod") + + assertTrue(graph.stages.getValue(StageId("impl")).needs.any { it.value == "dod" }) + assertTrue(graph.stages.getValue(StageId("review")).needs.any { it.value == "dod" }) } @Test diff --git a/infrastructure/workflow/src/test/kotlin/com/correx/infrastructure/workflow/FreestylePlanningWorkflowTest.kt b/infrastructure/workflow/src/test/kotlin/com/correx/infrastructure/workflow/FreestylePlanningWorkflowTest.kt index 142f7c67..e182d171 100644 --- a/infrastructure/workflow/src/test/kotlin/com/correx/infrastructure/workflow/FreestylePlanningWorkflowTest.kt +++ b/infrastructure/workflow/src/test/kotlin/com/correx/infrastructure/workflow/FreestylePlanningWorkflowTest.kt @@ -28,6 +28,7 @@ class FreestylePlanningWorkflowTest { private val registry = DefaultArtifactKindRegistry().apply { register(llmKind("discovery")) register(llmKind("analysis")) + register(llmKind("dod")) register(llmKind("execution_plan")) } @@ -64,7 +65,7 @@ class FreestylePlanningWorkflowTest { "architect must produce execution_plan", ) - assertTrue(architect.needs.map { it.value }.contains("analysis"), "architect needs analysis") + assertTrue(architect.needs.map { it.value }.contains("dod"), "architect needs the fixed DoD") val analystToArchitect = graph.transitions.first { it.from == StageId("analyst") } assertEquals(StageId("architect"), analystToArchitect.to) diff --git a/infrastructure/workflow/src/test/kotlin/com/correx/infrastructure/workflow/LspServerRegistryTest.kt b/infrastructure/workflow/src/test/kotlin/com/correx/infrastructure/workflow/LspServerRegistryTest.kt new file mode 100644 index 00000000..ee5d6b18 --- /dev/null +++ b/infrastructure/workflow/src/test/kotlin/com/correx/infrastructure/workflow/LspServerRegistryTest.kt @@ -0,0 +1,22 @@ +package com.correx.infrastructure.workflow + +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertNull + +class LspServerRegistryTest { + private val registry = LspServerRegistry() + + @Test + fun `registry maps language extensions to servers`() { + assertEquals("tsserver", registry.serverFor("src/App.tsx")?.id) + assertEquals("rust-analyzer", registry.serverFor("src/main.rs")?.id) + assertEquals("gopls", registry.serverFor("cmd/main.go")?.id) + assertEquals("clangd", registry.serverFor("src/main.cpp")?.id) + } + + @Test + fun `unknown extension falls through`() { + assertNull(registry.serverFor("README.md")) + } +} diff --git a/infrastructure/workflow/src/test/kotlin/com/correx/infrastructure/workflow/PlanGrounderTest.kt b/infrastructure/workflow/src/test/kotlin/com/correx/infrastructure/workflow/PlanGrounderTest.kt index ced0bf4c..9bc4ff3c 100644 --- a/infrastructure/workflow/src/test/kotlin/com/correx/infrastructure/workflow/PlanGrounderTest.kt +++ b/infrastructure/workflow/src/test/kotlin/com/correx/infrastructure/workflow/PlanGrounderTest.kt @@ -65,6 +65,13 @@ class PlanGrounderTest { assertTrue(r.findings.single().contains("frontend/**")) } + @Test + fun `scanned=false skips scope grounding — a missing scan can't prove a path absent`() { + val g = graph("impl" to StageConfig(touches = listOf("frontend/**", "apps/server/**"))) + val r = PlanGrounder.ground(g, repoMapPaths = emptySet(), profileCommands = cmds, scanned = false) + assertEquals(PlanGroundingVerdict.PASS, r.verdict) + } + @Test fun `a scope satisfied by an existing repo path grounds`() { val g = graph("impl" to StageConfig(touches = listOf("frontend/**"))) diff --git a/testing/integration/AGENTS.md b/testing/integration/AGENTS.md index 82a28b17..f6f3307e 100644 --- a/testing/integration/AGENTS.md +++ b/testing/integration/AGENTS.md @@ -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 diff --git a/testing/integration/src/test/kotlin/ClarificationLoopTest.kt b/testing/integration/src/test/kotlin/ClarificationLoopTest.kt index 6bf3b0d6..21a82e81 100644 --- a/testing/integration/src/test/kotlin/ClarificationLoopTest.kt +++ b/testing/integration/src/test/kotlin/ClarificationLoopTest.kt @@ -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", ) } } diff --git a/testing/integration/src/test/kotlin/RecoveryRoutingTest.kt b/testing/integration/src/test/kotlin/RecoveryRoutingTest.kt index 8123b8ba..1e9a3146 100644 --- a/testing/integration/src/test/kotlin/RecoveryRoutingTest.kt +++ b/testing/integration/src/test/kotlin/RecoveryRoutingTest.kt @@ -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 { 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() + .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() + .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() diff --git a/testing/kernel/src/test/kotlin/LaunchRegistrationRaceTest.kt b/testing/kernel/src/test/kotlin/LaunchRegistrationRaceTest.kt index caa24bb7..268336ee 100644 --- a/testing/kernel/src/test/kotlin/LaunchRegistrationRaceTest.kt +++ b/testing/kernel/src/test/kotlin/LaunchRegistrationRaceTest.kt @@ -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()