From d518400b5fae138f94300b5edf31ad5725c05627 Mon Sep 17 00:00:00 2001 From: kami Date: Mon, 8 Jun 2026 17:26:34 +0400 Subject: [PATCH] fix(validation): resolve F-007 artifact validator CAS-hash conflation ArtifactPayloadValidator passed the logical slot name into the content-hash-keyed CAS store, crashing every workflow with a typed produces slot. Validator now reads payloads from a content map threaded through ValidationContext (populated from the orchestrator's per-session artifact cache across all stages) and no longer depends on ArtifactStore. Also records the slot->CAS-hash mapping as a new ArtifactContentStoredEvent (registered for serialization), emitted at both CAS write sites, so artifact content is recoverable from CAS by hash after a cold start. --- .../kotlin/com/correx/apps/server/Main.kt | 2 +- .../core/events/events/ArtifactEvents.kt | 9 + .../events/serialization/Serialization.kt | 2 + .../orchestration/SessionOrchestrator.kt | 32 +++- .../artifact/ArtifactPayloadValidator.kt | 12 +- .../validation/model/ValidationContext.kt | 1 + qa/audit-report-2026-06-08.md | 161 ++++++++++++++++++ 7 files changed, 206 insertions(+), 13 deletions(-) create mode 100644 qa/audit-report-2026-06-08.md 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 4cc07d0b..b3b7736b 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 @@ -225,7 +225,7 @@ fun main() { inferenceRouter = inferenceRouter, validationPipeline = ValidationPipeline( validators = listOf( - ArtifactPayloadValidator(artifactStore), + ArtifactPayloadValidator(), SemanticValidator( rules = listOf( CycleExitRule(requirePolicyForCycles = false), diff --git a/core/events/src/main/kotlin/com/correx/core/events/events/ArtifactEvents.kt b/core/events/src/main/kotlin/com/correx/core/events/events/ArtifactEvents.kt index 1bd4e645..8c271307 100644 --- a/core/events/src/main/kotlin/com/correx/core/events/events/ArtifactEvents.kt +++ b/core/events/src/main/kotlin/com/correx/core/events/events/ArtifactEvents.kt @@ -30,3 +30,12 @@ data class ArtifactCreatedEvent( val stageId: StageId, val schemaVersion: Int, ) : EventPayload + +@Serializable +@SerialName("ArtifactContentStored") +data class ArtifactContentStoredEvent( + val artifactId: ArtifactId, + val contentHash: ArtifactId, // CAS content hash returned by ArtifactStore.put(...) + val sessionId: SessionId, + val stageId: StageId, +) : 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 92d6d593..6552ebe6 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 @@ -4,6 +4,7 @@ import com.correx.core.events.events.ApprovalDecisionResolvedEvent import com.correx.core.events.events.ApprovalGrantCreatedEvent import com.correx.core.events.events.ApprovalGrantExpiredEvent import com.correx.core.events.events.ApprovalRequestedEvent +import com.correx.core.events.events.ArtifactContentStoredEvent import com.correx.core.events.events.ArtifactCreatedEvent import com.correx.core.events.events.ArtifactValidatedEvent import com.correx.core.events.events.ArtifactValidatingEvent @@ -70,6 +71,7 @@ val eventModule = SerializersModule { subclass(ApprovalDecisionResolvedEvent::class) subclass(ApprovalGrantCreatedEvent::class) subclass(ApprovalGrantExpiredEvent::class) + subclass(ArtifactContentStoredEvent::class) subclass(ArtifactCreatedEvent::class) subclass(ArtifactValidatingEvent::class) subclass(ArtifactValidatedEvent::class) 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 e735cdd3..2ecef17f 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 @@ -22,6 +22,7 @@ import com.correx.core.context.model.TokenBudget import com.correx.core.context.model.EntryRole import com.correx.core.events.events.ApprovalDecisionResolvedEvent import com.correx.core.events.events.ApprovalRequestedEvent +import com.correx.core.events.events.ArtifactContentStoredEvent import com.correx.core.events.events.ArtifactCreatedEvent import com.correx.core.events.events.ContextTruncatedEvent import com.correx.core.events.events.ToolCallAssessedEvent @@ -418,11 +419,25 @@ abstract class SessionOrchestrator( llmEmittedSlots.firstOrNull()?.let { slot -> artifactContentCache["${sessionId.value}:${slot.name.value}"] = inferenceResult.response.text + inferenceResult.responseArtifactId?.let { hash -> + emit( + sessionId, + ArtifactContentStoredEvent( + artifactId = slot.name, + contentHash = hash, + sessionId = sessionId, + stageId = stageId, + ), + ) + } } val validationCtx = ValidationContext( graph = graph, sessionState = session.state, - availableTools = effectives.registry?.all()?.map { it.name }?.toSet() + availableTools = effectives.registry?.all()?.map { it.name }?.toSet(), + producedArtifactContent = graph.stages.values.flatMap { it.produces }.mapNotNull { slot -> + artifactContentCache["${sessionId.value}:${slot.name.value}"]?.let { slot.name.value to it } + }.toMap(), ) when (val outcome = mapValidationOutcome(sessionId, stageId, validationCtx)) { is StageExecutionResult.Success -> { @@ -702,8 +717,17 @@ abstract class SessionOrchestrator( ) } val jsonContent = Json.encodeToString(ProcessResultArtifact.serializer(), processResult) - artifactStore.put(jsonContent.toByteArray()) + val storedHash = artifactStore.put(jsonContent.toByteArray()) artifactContentCache["${sessionId.value}:${slot.name.value}"] = jsonContent + emit( + sessionId, + ArtifactContentStoredEvent( + artifactId = slot.name, + contentHash = storedHash, + sessionId = sessionId, + stageId = stageId, + ), + ) } val sourceId = toolCall.id ?: invocationId.value @@ -1073,7 +1097,7 @@ abstract class SessionOrchestrator( ), ) - InferenceResult.Success(response) + InferenceResult.Success(response, responseArtifactId) } catch (e: TimeoutCancellationException) { emit( sessionId, @@ -1456,7 +1480,7 @@ private fun buildDiffString(path: String, existingContent: String?, proposedCont } internal sealed interface InferenceResult { - data class Success(val response: InferenceResponse) : InferenceResult + data class Success(val response: InferenceResponse, val responseArtifactId: ArtifactId? = null) : InferenceResult data class Failed(val reason: String) : InferenceResult data object Cancelled : InferenceResult } diff --git a/core/validation/src/main/kotlin/com/correx/core/validation/artifact/ArtifactPayloadValidator.kt b/core/validation/src/main/kotlin/com/correx/core/validation/artifact/ArtifactPayloadValidator.kt index 02477063..4c16a477 100644 --- a/core/validation/src/main/kotlin/com/correx/core/validation/artifact/ArtifactPayloadValidator.kt +++ b/core/validation/src/main/kotlin/com/correx/core/validation/artifact/ArtifactPayloadValidator.kt @@ -3,7 +3,6 @@ package com.correx.core.validation.artifact import com.correx.core.artifacts.kind.FileWrittenArtifact import com.correx.core.artifacts.kind.ProcessResultArtifact import com.correx.core.artifacts.kind.TypedArtifactSlot -import com.correx.core.artifactstore.ArtifactStore import com.correx.core.events.types.ArtifactId import com.correx.core.validation.model.ValidationContext import com.correx.core.validation.model.ValidationIssue @@ -13,16 +12,14 @@ import com.correx.core.validation.pipeline.Validator import kotlinx.serialization.json.Json import java.nio.file.Path -class ArtifactPayloadValidator( - private val artifactStore: ArtifactStore, -) : Validator { +class ArtifactPayloadValidator : Validator { override suspend fun validate(context: ValidationContext): ValidationSection { val issues = mutableListOf() for ((_, stageConfig) in context.graph.stages) { for (slot in stageConfig.produces) { - validateSlot(slot, issues) + validateSlot(slot, context.producedArtifactContent, issues) } } @@ -31,11 +28,10 @@ class ArtifactPayloadValidator( private suspend fun validateSlot( slot: TypedArtifactSlot, + artifactContent: Map, issues: MutableList, ) { - val bytes = artifactStore.get(slot.name) ?: return - - val payloadStr = bytes.toString(Charsets.UTF_8) + val payloadStr = artifactContent[slot.name.value] ?: return when (slot.kind.id) { "file_written" -> diff --git a/core/validation/src/main/kotlin/com/correx/core/validation/model/ValidationContext.kt b/core/validation/src/main/kotlin/com/correx/core/validation/model/ValidationContext.kt index 07fab289..b14f8236 100644 --- a/core/validation/src/main/kotlin/com/correx/core/validation/model/ValidationContext.kt +++ b/core/validation/src/main/kotlin/com/correx/core/validation/model/ValidationContext.kt @@ -11,4 +11,5 @@ data class ValidationContext( val cyclePolicies: Set = emptySet(), val sessionState: SessionState? = null, val availableTools: Set? = null, + val producedArtifactContent: Map = emptyMap(), ) diff --git a/qa/audit-report-2026-06-08.md b/qa/audit-report-2026-06-08.md new file mode 100644 index 00000000..e71ac973 --- /dev/null +++ b/qa/audit-report-2026-06-08.md @@ -0,0 +1,161 @@ +# correx QA / Audit Report — 2026-06-08 + +Live operator session (Phase 3). Provider: local llama-server via env-var fallback +(`llama-cpp:default` @ 127.0.0.1:10000). Target: `qa/sample-repo/` (tag `clean`). + +## Run context / repro prompt + +Workflow: `role_pipeline` (analyst→architect→planner→implementer⇄reviewer). +Target workspace (TUI `Hello` workingDir): `/home/kami/Programs/correx/qa/sample-repo` +(reset between runs with `git -C qa/sample-repo reset --hard clean`). +Provider: env-var fallback `llama-cpp:default` @ 127.0.0.1:10000 (start llama-server first). + +Operator prompt used for every run so far: + +``` +Fix the failing test in this repository. Running `python3 -m pytest` shows +tests/test_calc.py::test_average_empty failing: calc.average([]) raises +ZeroDivisionError, but it should return 0.0 for an empty list. Diagnose the +cause in src/sampleapp/calc.py, implement the fix, and verify all tests pass. +``` + +Patches applied this session (rebuild + restart server to load): +- F-001: `LlamaCppInferenceProvider.kt` — drop grammar when tools present. +- F-005: `FileReadConfig`/`Main` workspace builder/`FileReadTool` — workingDir-relative + path resolution. +- F-007: validator content-map (drops CAS dep) + `ArtifactContentStoredEvent` slot→hash + recording. No server restart needed for validation correctness. + +## Findings + +### F-001 — grammar + tools sent together → llama.cpp 400, every tool+artifact stage dies +- **invariant/contract:** functional (workflows run end-to-end) / #7 path (artifact constraint) +- **subsystem:** infrastructure/inference (llama_cpp) +- **severity:** blocker +- **evidence:** `logs/current.log` analyst stage: `llama-server returned 400 Bad Request: + "Cannot use custom grammar constraints with tools."`. Request body carries both + `"grammar": "root ::= ..."` and `"tools":[file_read, stage_complete]`. +- **repro:** `correx run role_pipeline` (any prompt) against sample-repo. Analyst declares + `allowed_tools=[file_read, ShellTool]` AND `produces=[analysis]`. Fails immediately. +- **root cause:** `LlamaCppInferenceProvider.kt:103-104` sets `grammar` and `tools` + unconditionally. llama.cpp's server forbids the combination. ANY stage that both has + tools and a JSON `responseFormat` is unrunnable on llama.cpp. role_pipeline analyst + + implementer both do → pipeline cannot start. +- **fix direction:** when tools are present, drop the GBNF grammar and rely on + post-hoc schema validation + retry (invariant #7 still holds — output is validated); + OR gate the artifact emission into a separate tool-free final inference turn. +- **status:** PATCHED (workaround) — `LlamaCppInferenceProvider.kt`: grammar suppressed + when tools are present; artifact now constrained by validation+retry only. Requires + server restart to take effect. Proper fix tracked below. +- **follow-up TODO (proper fix):** add a first-class artifact-emission tool — the LLM + calls `emit_artifact(path, content, description)` like it calls file_read/file_write, + making artifact production an explicit schema-checked tool call instead of a grammar + constraint that can't coexist with tools. TODO left in code at the patch site. + +### F-002 — non-retryable 400 is retried to exhaustion +- **invariant/contract:** functional (retry accounting) +- **subsystem:** core/kernel (orchestration retry policy) +- **severity:** minor +- **evidence:** orchestrator marks the 400 `retryable=true`; retries 1/3, 2/3, 3/3 all + identical 400s within ~80ms, then `WorkflowFailedEvent retryExhausted=true`. +- **suspected cause:** retry classifier treats all provider failures as retryable. A 400 + `invalid_request_error` is deterministic request-construction failure → should be terminal. +- **status:** confirmed + +### F-003 — `~` not expanded in default_system prompt path +- **subsystem:** core/kernel (prompt loading) / core/config +- **severity:** minor +- **evidence:** `failed to load prompt '~/.config/correx/prompts/default_system.md': + Prompt not found (searched: .../prompts/~/.config/correx/prompts/default_system.md)` — + tilde concatenated literally onto the config dir. Logged ERROR on every attempt. + Non-fatal (run proceeds) but noisy + the default system prompt is silently absent. +- **status:** confirmed + +### F-005 — file_read jails relative paths against process cwd, not the workspace +- **invariant/contract:** functional (tool jailing) / two-plane consistency (#9) +- **subsystem:** infrastructure/tools/filesystem (FileReadTool) + apps/server wiring +- **severity:** major (blocks any file_read of a relative path unless server launched + from inside the workspace) +- **evidence:** `logs/current.log` session c0eab7ba: analyst emitted + `file_read(path=tests/test_calc.py)`. Plane-2 PATH_CONTAINMENT PROCEEDed + (`resolved=.../qa/sample-repo/tests/test_calc.py, inWorkspace=true`), but the tool's own + `validateRequest` rejected it: `Path 'tests/test_calc.py' is not in the allowed list` + → stage produced no artifact → surfaced as the misleading `no transition condition + matched from stage analyst`. +- **root cause:** `FileReadTool` resolved the relative path via `Paths.get(p).toAbsolutePath()` + = JVM process cwd (the correx repo root, where the server was launched), not the bound + workspace. `FileReadConfig` had no `workingDir` field at all, unlike FileWrite/FileEdit + which carry `workingDir` and call `resolvePath()`. So the two file-access planes anchored + relative paths to different roots. +- **status:** PATCHED — added `workingDir` to `FileReadConfig`, wired + `workspace.workingDir` in Main's per-workspace tool builder, and gave `FileReadTool` a + `resolvePath()` mirroring FileWriteTool. Requires server restart. +- **note:** the operator-facing failure reason (`no transition condition matched`) masked + the real cause (tool denial). See F-004 family — failure-reason surfacing is lossy. + +### F-007 — ArtifactPayloadValidator conflates slot name with CAS content-hash → crash +- **invariant/contract:** functional (artifacts validate) / #7 (LLM output untrusted + until validated — the validation step itself is broken) +- **subsystem:** core/validation + core/artifacts-store contract vs infrastructure CAS +- **severity:** BLOCKER (every workflow with a typed `produces` slot crashes at the first + post-stage validation against the real CAS store) +- **evidence:** `logs/current.log` session 28c818c1: analyst COMPLETED + (responseArtifactId=82da21d5…, a real blake3 hash), then + `runSession ... failed: hex string must have even length` — + `CasArtifactStore.hexToBytes` at `CasArtifactStore.kt:102`, called from + `ArtifactPayloadValidator.validateSlot:36`. +- **root cause:** `ArtifactPayloadValidator.validateSlot` calls + `artifactStore.get(slot.name)`. `slot.name` is the LOGICAL slot id ("analysis"), but + `CasArtifactStore.get` requires `ArtifactId.value` to be a hex content-hash + (`id.value.hexToBytes()`). "analysis" → odd-length → `require` throws. The validator has + no slot-name→content-hash mapping: `ValidationContext` carries only the static graph + + `SessionState`, and `SessionState` holds NO produced-artifact map. So the validator + cannot locate the bytes it is supposed to validate. +- **deeper implication:** this path has apparently never been exercised end-to-end against + the real content-addressed CAS — it only "works" if the ArtifactStore is keyed by literal + slot name (an in-memory test double). The `ArtifactStore.get(ArtifactId)` contract is + ambiguous: the validator treats ArtifactId as a logical name, the CAS treats it as a hash. +- **fix direction (NOT a one-liner):** the run's produced-artifact mapping + (slot name → ArtifactId(hash)) must be derived from the event log (an ArtifactProduced-type + event projection per invariant #1) and threaded into `ValidationContext`, so the validator + resolves slot.name → stored hash before `get`. Cross-module change + (events → projection → context builder → validator). Audit-only: NOT patched. +- **status:** FIXED. +- **fix applied:** two changes (validation crash + durable mapping). + 1. *Validator no longer touches the CAS.* `ValidationContext` gained + `producedArtifactContent: Map` (slot.name.value → payload). The + orchestrator populates it from the in-memory `artifactContentCache` for every produced + slot across `graph.stages` (scoped to the session). `ArtifactPayloadValidator` reads + from that map and **dropped its `ArtifactStore` dependency entirely** — the slot-name + vs content-hash conflation is gone. `Main.kt` wiring updated. Validation stays live-only + (never runs during replay) so invariant #8 is unaffected. + 2. *Slot→CAS-hash mapping now recorded as an event* (invariant #9 — env observation + recorded at the moment it runs). New `ArtifactContentStoredEvent(artifactId=logical + slot, contentHash=CAS blake3 hash, sessionId, stageId)`, registered in + `Serialization.kt`, emitted at both CAS write sites (LLM-emitted slot path + tool/ + process_result path; the tool path previously discarded the `put()` return value). This + is the durable record that lets content be recovered from CAS by hash after a cold start. +- **remaining follow-up (not in this fix):** the validator/`needs`-rehydration still read the + *in-memory* `artifactContentCache`, which is lost on restart (the F-001 rehydration-gap + suspect). Closing that means resolving slot→hash→CAS from the new + `ArtifactContentStoredEvent` on cold start / re-validation. Foundation is now in place. + +### F-006 — TUI input pane render glitch +- **subsystem:** apps/tui-go +- **severity:** minor (cosmetic) +- **evidence:** operator screenshot 16:07 — a garbled/smeared rectangular artifact in the + bottom-right of the input pane (uncleared region / stale cell buffer) while the prompt + text rendered normally on the left. +- **repro:** operator confirms it appears **only on multi-line paste** into the input box; + single-line / typed input renders clean. Likely the input widget doesn't repaint the full + region when a pasted block changes line count. +- **status:** confirmed, repro narrowed (multi-line paste only) + +### F-004 — InferenceFailedEvent / RetryAttemptedEvent unmapped to TUI bridge +- **invariant/contract:** functional (operator observability) +- **subsystem:** apps/server (DomainEventMapper) +- **severity:** minor +- **evidence:** `DomainEventMapper: unmapped payload type=InferenceFailedEvent` and + `=RetryAttemptedEvent` repeated. Operator sees failure only via router narration, not a + structured failure/retry surface in the TUI. +- **status:** confirmed