From 700f59ef0dc6b3b3819f078c60f0a4e18c3a10fb Mon Sep 17 00:00:00 2001 From: claude Date: Tue, 11 Aug 2026 19:56:43 +0400 Subject: [PATCH 1/6] feat(kernel): gate the DoD against discovery scope (#699) Session 954da1a9 asked for an eight-view web UI and shipped a Vite starter page. Discovery settled all eight items in brief.scope; the analyst emitted four criteria, all part="Project Foundation"; the architect planned against that DoD, so the run scaffolded Vite, Tailwind and TanStack Query and stopped. The plan-compile gate and the final reviewer both graded the shrunken DoD, so a plan delivering 5% of the request passed clean. Each DoD criterion now carries `covers`: the 0-based indexes into discovery brief.scope it proves. A post-stage scope_coverage gate fails the analyst retryably when an index has no criterion, handing back the dropped items verbatim. Pure function of two recorded artifacts, so replay recomputes it and no verdict event is needed. Ceiling is index bookkeeping, not semantics: a criterion claiming covers:[3] without really proving scope[3] still passes. It catches the silent collapse, not a weak criterion. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_013dVqqci5H5b3s6xzv6Lojq --- .../kernel/orchestration/ScopeCoverage.kt | 127 ++++++++++++++++++ .../orchestration/SessionOrchestratorGates.kt | 5 +- .../kernel/orchestration/ScopeCoverageTest.kt | 77 +++++++++++ docs/schemas/dod.json | 9 +- .../workflows/prompts/analyst_freestyle.md | 15 ++- 5 files changed, 226 insertions(+), 7 deletions(-) create mode 100644 core/kernel/src/main/kotlin/com/correx/core/kernel/orchestration/ScopeCoverage.kt create mode 100644 core/kernel/src/test/kotlin/com/correx/core/kernel/orchestration/ScopeCoverageTest.kt diff --git a/core/kernel/src/main/kotlin/com/correx/core/kernel/orchestration/ScopeCoverage.kt b/core/kernel/src/main/kotlin/com/correx/core/kernel/orchestration/ScopeCoverage.kt new file mode 100644 index 00000000..1a585820 --- /dev/null +++ b/core/kernel/src/main/kotlin/com/correx/core/kernel/orchestration/ScopeCoverage.kt @@ -0,0 +1,127 @@ +package com.correx.core.kernel.orchestration + +import com.correx.core.events.types.SessionId +import com.correx.core.events.types.StageId +import com.correx.core.transitions.execution.StageExecutionResult +import com.correx.core.transitions.graph.StageConfig +import kotlinx.serialization.json.Json +import kotlinx.serialization.json.JsonArray +import kotlinx.serialization.json.JsonObject +import kotlinx.serialization.json.contentOrNull +import kotlinx.serialization.json.intOrNull +import kotlinx.serialization.json.jsonPrimitive + +/** + * Pure deterministic check that a definition of done accounts for every in-scope item the + * discovery brief settled. + * + * Determinism / invariant #8: reads ONLY the two recorded artifact strings — no I/O, no external + * calls. Both are already in the event log (ArtifactCreatedEvent), so the result is recomputable + * on replay without emitting an observation event. + * + * The failure this closes: the analyst is the one-way funnel between the brief and every later + * stage. Session 954da1a9 turned an 8-item scope into four "Project Foundation" criteria, and + * because the architect, plan-compile gate and final reviewer all grade the *shrunken* DoD, a plan + * that delivered 5% of the request passed every gate. + * + * ponytail: index bookkeeping, not semantics — a criterion claiming `covers: [3]` without really + * proving scope[3] still passes. Forcing the analyst to name every index catches the silent + * collapse (a whole scope list dropped, unnoticed); judging whether a criterion is strong enough + * stays the reviewer's job. + */ +internal object ScopeCoverage { + + private val lenientJson = Json { ignoreUnknownKeys = true; isLenient = true } + + /** + * The discovery scope items no DoD criterion claims to cover, each rendered as + * `[index] item` so the retry feedback names the index the model must put in `covers`. + * + * Empty when the check does not apply: an unparseable discovery brief or an empty scope. An + * unparseable DoD, or one whose criteria declare no `covers` at all, reports the whole scope. + */ + fun uncoveredScope(discoveryJson: String, dodJson: String): List { + val scope = parse(discoveryJson) + ?.let { it["brief"] as? JsonObject } + ?.let { stringList(it, "scope") } + .orEmpty() + val covered = coveredIndexes(parse(dodJson)) + return scope.withIndex() + .filterNot { (index, _) -> index in covered } + .map { (index, item) -> "[$index] $item" } + } + + /** Every index listed in any criterion's `covers` array. */ + private fun coveredIndexes(dod: JsonObject?): Set = + (dod?.get("criteria") as? JsonArray) + ?.filterIsInstance() + ?.flatMap { criterion -> (criterion["covers"] as? JsonArray) ?: emptyList() } + ?.mapNotNull { runCatching { it.jsonPrimitive.intOrNull }.getOrNull() } + ?.toSet() + ?: emptySet() + + private fun parse(json: String): JsonObject? = + runCatching { lenientJson.parseToJsonElement(stripFence(json)) as? JsonObject }.getOrNull() + + private fun stringList(obj: JsonObject, key: String): List = + runCatching { + (obj[key] as? JsonArray)?.mapNotNull { it.jsonPrimitive.contentOrNull } ?: emptyList() + }.getOrElse { emptyList() } + + private fun stripFence(text: String): String { + val trimmed = text.trim() + if (!trimmed.startsWith("```") || !trimmed.endsWith("```")) return text + val withoutClose = trimmed.removeSuffix("```").trimEnd() + val firstNewline = withoutClose.indexOf('\n') + return if (firstNewline < 0) text else withoutClose.substring(firstNewline + 1) + } +} + +/** + * Scope-coverage gate: for a stage that produces a `dod` artifact and consumed the `discovery` + * brief, fail retryably when a settled scope item has no criterion claiming it. Both artifacts are + * already in the content cache (recorded via ArtifactCreatedEvent), so the verdict is a pure + * function of recorded data and needs no event of its own (invariants #8, #9). Unlike the + * plan-compile gate, nothing here leaves the process. + * + * Lives beside [ScopeCoverage] rather than in SessionOrchestratorGates.kt, which is already at its + * function budget. + */ +internal suspend fun SessionOrchestrator.runScopeCoverageGate( + sessionId: SessionId, + stageId: StageId, + stageConfig: StageConfig, +): StageExecutionResult { + val uncovered = uncoveredScopeItems(sessionId, stageConfig) + return if (uncovered.isEmpty()) { + StageExecutionResult.Success(emptyList()) + } else { + log.warn( + "[Orchestrator] scope-coverage gate failed session={} stage={} uncovered={}", + sessionId.value, stageId.value, uncovered.joinToString("; "), + ) + StageExecutionResult.Failure( + "stage ${stageId.value} produced a definition of done that drops settled in-scope " + + "work. These discovery scope items have no criterion:\n" + + uncovered.joinToString("\n") { "- $it" } + + "\nAdd a criterion for each and list its scope index in that criterion's `covers`.", + retryable = true, + gate = "scope_coverage", + ) + } +} + +/** Empty when the gate does not apply: no `dod` produced, or no `discovery` brief to compare. */ +private fun SessionOrchestrator.uncoveredScopeItems( + sessionId: SessionId, + stageConfig: StageConfig, +): List { + val dod = stageConfig.produces.firstOrNull { it.kind.id == "dod" } + ?.let { artifactContentCache["${sessionId.value}:${it.name.value}"] } + val discovery = artifactContentCache["${sessionId.value}:discovery"] + return if (dod == null || discovery == null) { + emptyList() + } else { + ScopeCoverage.uncoveredScope(discovery, dod) + } +} 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 ad3a0a86..9d0fb3d2 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 @@ -196,6 +196,7 @@ internal suspend fun SessionOrchestrator.runPostStageGates( val gates: List StageExecutionResult> = listOf( { groundBriefReferences(sessionId, stageId, stageConfig, effectives) }, { checkBriefEcho(sessionId, stageId, stageConfig) }, + { runScopeCoverageGate(sessionId, stageId, stageConfig) }, { runContractGate(sessionId, stageId, stageConfig, effectives) }, { runPlanCompileGate(sessionId, stageId, stageConfig) }, { runStaticAnalysis(sessionId, stageId, stageConfig, effectives) }, @@ -268,7 +269,9 @@ internal suspend fun SessionOrchestrator.evaluateStageContract( } /** The currently-failing assertions as (target, assertionId, evidence) triples for the checklist. */ -internal fun SessionOrchestrator.contractFailureItems(results: List): List> = +internal fun SessionOrchestrator.contractFailureItems( + results: List, +): List> = results.filterNot { it.passed }.map { Triple(it.target, it.assertionId, it.evidence) } internal suspend fun SessionOrchestrator.runContractGate( diff --git a/core/kernel/src/test/kotlin/com/correx/core/kernel/orchestration/ScopeCoverageTest.kt b/core/kernel/src/test/kotlin/com/correx/core/kernel/orchestration/ScopeCoverageTest.kt new file mode 100644 index 00000000..e9e73db0 --- /dev/null +++ b/core/kernel/src/test/kotlin/com/correx/core/kernel/orchestration/ScopeCoverageTest.kt @@ -0,0 +1,77 @@ +package com.correx.core.kernel.orchestration + +import org.junit.jupiter.api.Assertions.assertEquals +import org.junit.jupiter.api.Assertions.assertTrue +import org.junit.jupiter.api.Test + +class ScopeCoverageTest { + + private fun discovery(vararg scope: String): String { + val items = scope.joinToString(",") { "\"$it\"" } + return """{"brief":{"what":"a ui","scope":[$items],"non_goals":[]},"ready":true,"questions":[]}""" + } + + private fun dod(vararg covers: List): String { + val criteria = covers.mapIndexed { i, c -> + """{"id":"c${i + 1}","statement":"s","part":"p","verified_by":"gate","covers":[${c.joinToString(",")}]}""" + }.joinToString(",") + return """{"summary":"s","criteria":[$criteria],"out_of_scope":[]}""" + } + + @Test + fun `every scope index covered leaves nothing uncovered`() { + val uncovered = ScopeCoverage.uncoveredScope( + discovery("sessions list", "events viewer"), + dod(listOf(0), listOf(1)), + ) + assertTrue(uncovered.isEmpty(), "expected full coverage, got $uncovered") + } + + @Test + fun `one criterion may cover several scope items and a run-level criterion covers none`() { + val uncovered = ScopeCoverage.uncoveredScope( + discovery("sessions list", "events viewer", "artifacts viewer"), + dod(listOf(0, 1, 2), emptyList()), + ) + assertTrue(uncovered.isEmpty(), "expected full coverage, got $uncovered") + } + + @Test + fun `dropped scope items are reported with their index`() { + val uncovered = ScopeCoverage.uncoveredScope( + discovery("session driver", "sessions list", "workflows", "events"), + dod(listOf(0), listOf(1)), + ) + assertEquals(listOf("[2] workflows", "[3] events"), uncovered) + } + + /** The regression this gate exists for: session 954da1a9's foundation-only DoD. */ + @Test + fun `a DoD with no covers at all reports the whole scope`() { + val uncovered = ScopeCoverage.uncoveredScope( + discovery("session driver", "sessions list"), + """{"summary":"init the stack","criteria":[ + {"id":"c1","statement":"Vite and React initialized","part":"Project Foundation","verified_by":"gate"} + ],"out_of_scope":[]}""", + ) + assertEquals(listOf("[0] session driver", "[1] sessions list"), uncovered) + } + + @Test + fun `an unparseable DoD reports the whole scope`() { + val uncovered = ScopeCoverage.uncoveredScope(discovery("sessions list"), "not json at all") + assertEquals(listOf("[0] sessions list"), uncovered) + } + + @Test + fun `a fenced DoD is read through the fence`() { + val fenced = "```json\n" + dod(listOf(0)) + "\n```" + assertTrue(ScopeCoverage.uncoveredScope(discovery("sessions list"), fenced).isEmpty()) + } + + @Test + fun `the check does not apply without a usable discovery scope`() { + assertTrue(ScopeCoverage.uncoveredScope("not json", dod(listOf(0))).isEmpty()) + assertTrue(ScopeCoverage.uncoveredScope(discovery(), dod(listOf(0))).isEmpty()) + } +} diff --git a/docs/schemas/dod.json b/docs/schemas/dod.json index 48fdf61d..ae7e8403 100644 --- a/docs/schemas/dod.json +++ b/docs/schemas/dod.json @@ -11,9 +11,14 @@ "id": { "type": "string" }, "statement": { "type": "string" }, "part": { "type": "string" }, - "verified_by": { "type": "string", "description": "one of: gate, reviewer" } + "verified_by": { "type": "string", "description": "one of: gate, reviewer" }, + "covers": { + "type": "array", + "items": { "type": "integer" }, + "description": "0-based indexes into the discovery brief's scope[] that this criterion proves. Every scope index must appear in at least one criterion or the scope-coverage gate fails the stage." + } }, - "required": ["id", "statement", "part", "verified_by"], + "required": ["id", "statement", "part", "verified_by", "covers"], "additionalProperties": false } }, diff --git a/examples/workflows/prompts/analyst_freestyle.md b/examples/workflows/prompts/analyst_freestyle.md index 4ca985e1..835d9a61 100644 --- a/examples/workflows/prompts/analyst_freestyle.md +++ b/examples/workflows/prompts/analyst_freestyle.md @@ -41,20 +41,27 @@ Emit the `dod` artifact once. Its criteria are the complete acceptance contract - 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. +- Give every criterion a `covers` array: the 0-based indexes into discovery `brief.scope` it proves. + Walk `brief.scope` in order and account for every index. A scope-coverage gate fails this stage + and hands back the uncovered items verbatim. Two criteria proving one scope item repeat its index. + One criterion proving three items lists all three. A criterion that serves the run rather than a + scope item (the named task, a failure path) gets `[]`. - Include at least one criterion proving the named task is carried through to the implementation plan, and one criterion for each material failure or recovery path identified during discovery. 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]}`. +"verified_by": "gate" | "reviewer", "covers": [integer]}], "out_of_scope": [string]}`. -Example: +Example, for a discovery brief whose `scope` is +`["Bounded validation gate", "Operator sees the diagnostic"]`: ```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"} + {"id":"c1","statement":"The project typecheck passes before completion","part":"terminal gate","verified_by":"gate","covers":[0]}, + {"id":"c2","statement":"The operator sees the recorded diagnostic","part":"workflow UX","verified_by":"reviewer","covers":[1]}, + {"id":"c3","statement":"The implementation plan names task gate-42","part":"task threading","verified_by":"reviewer","covers":[]} ], "out_of_scope": ["Changing the workflow topology"] } -- 2.52.0 From d89258742041117b044bed163c5921309b6b7e48 Mon Sep 17 00:00:00 2001 From: claude Date: Tue, 11 Aug 2026 22:15:49 +0400 Subject: [PATCH 2/6] fix(kernel,workflow): five fixes from the 954da1a9 post-mortem (#705,#706,#709,#710,#712) The run died on a build gate running the wrong toolchain and burned 43% of its tool calls on repeats, rejections and failures. Five fixes, each traceable to a measured cost in docs/audits/2026-08-11-session-954da1a9-postmortem.md. #705 build gate toolchain scope. The gate is armed session-scoped but read the toolchain stage-scoped, so a reviewer stage that wrote nothing fell back to the flat `build` alias and ran ./gradlew assemble on an all-frontend session. It now falls back to the session's own manifest first. 32.5 min, 33% of that run. #706 action ledger. L2 keeps ten conversation entries, so a stage past round five has no memory of what it tried; 29% of tool calls were byte-identical repeats. One pinned line per call (tool, target, outcome) with repeats collapsed to a count, ~3k tokens for a whole run. #709 plan-compile lint for blocked runners. Plans prescribed `npx tailwindcss init -p`, rejected by the shell denylist at every attempt. Rejected at compile time now, where the architect can still rewrite the step. #710 near-greedy sampling on tool-call rounds. temperature 1.0 on argv emitted `./gradlew_`, `npm_prefix=frontend`, `create_vite@latest`. Prose rounds keep the operator's sampling. #712 auto-approve manifest-contained writes. 94 approvals, all APPROVED, no steering, 19 min. A write inside the declared manifest already proved its containment by getting past ManifestContainmentRule. DENY mode still denies. Tests: core:kernel 133, infrastructure:workflow 99, testing:integration 176, testing:deterministic 79, all green. detekt clean (no new findings). Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_013dVqqci5H5b3s6xzv6Lojq --- .../builder/DefaultContextPackBuilder.kt | 6 +- .../core/kernel/orchestration/ActionLedger.kt | 100 +++++ .../orchestration/BuildGateToolchain.kt | 10 + .../orchestration/SessionOrchestrator.kt | 19 +- .../SessionOrchestratorExecution.kt | 10 + .../SessionOrchestratorGates2.kt | 10 +- .../SessionOrchestratorToolExec.kt | 16 +- .../kernel/orchestration/ActionLedgerTest.kt | 76 ++++ .../BuildPrerequisiteDecisionTest.kt | 12 + .../2026-08-11-session-954da1a9-postmortem.md | 347 ++++++++++++++++++ .../workflow/ExecutionPlanCompiler.kt | 27 ++ .../workflow/ExecutionPlanCompilerTest.kt | 48 +++ 12 files changed, 674 insertions(+), 7 deletions(-) create mode 100644 core/kernel/src/main/kotlin/com/correx/core/kernel/orchestration/ActionLedger.kt create mode 100644 core/kernel/src/test/kotlin/com/correx/core/kernel/orchestration/ActionLedgerTest.kt create mode 100644 docs/audits/2026-08-11-session-954da1a9-postmortem.md diff --git a/core/context/src/main/kotlin/com/correx/core/context/builder/DefaultContextPackBuilder.kt b/core/context/src/main/kotlin/com/correx/core/context/builder/DefaultContextPackBuilder.kt index 57f411b4..c6217a38 100644 --- a/core/context/src/main/kotlin/com/correx/core/context/builder/DefaultContextPackBuilder.kt +++ b/core/context/src/main/kotlin/com/correx/core/context/builder/DefaultContextPackBuilder.kt @@ -59,7 +59,11 @@ class DefaultContextPackBuilder( // the model a progress signal that outlives the truncation which otherwise wipes its memory. // "retryFeedback" carries WHY the last attempt failed + the files already written this stage; if // truncation evicts it the model cold-starts on the same wrong idea every turn (the write-loop rot). - private val neverDropSourceTypes = setOf("steeringNote", "eventHistory", "factSheet", "remainingDelta", "retryFeedback") + // "actionLedger" is the stage's one-line-per-tool-call history (#706). L2 keeps only the last + // ten conversation entries, so without it a stage past round five re-issues calls it already + // made; the ledger is the memory that survives that eviction. + private val neverDropSourceTypes = + setOf("steeringNote", "eventHistory", "factSheet", "remainingDelta", "retryFeedback", "actionLedger") private companion object { const val CHARS_PER_TOKEN = 4 diff --git a/core/kernel/src/main/kotlin/com/correx/core/kernel/orchestration/ActionLedger.kt b/core/kernel/src/main/kotlin/com/correx/core/kernel/orchestration/ActionLedger.kt new file mode 100644 index 00000000..3e044220 --- /dev/null +++ b/core/kernel/src/main/kotlin/com/correx/core/kernel/orchestration/ActionLedger.kt @@ -0,0 +1,100 @@ +package com.correx.core.kernel.orchestration + +import com.correx.core.context.model.ContextEntry +import com.correx.core.context.model.ContextLayer +import com.correx.core.context.model.EntryRole +import com.correx.core.events.types.ContextEntryId +import com.correx.core.inference.ToolCallRequest +import java.util.UUID +import kotlinx.serialization.json.Json +import kotlinx.serialization.json.JsonPrimitive +import kotlinx.serialization.json.jsonObject + +/** + * Deterministic action ledger (#706, post-mortem of run 954da1a9). L2 holds the last ten + * conversation entries — five tool call/result pairs — so a stage past round five has no memory of + * what it already tried. 29% of that run's 258 tool calls were byte-identical repeats: `list_dir + * frontend` nine times, `./gradlew assemble` four times, each one failing the same way. + * + * The ledger is one line per call — tool, target, outcome — pinned so it never evicts. A whole run + * is roughly 3k tokens, cheaper than the duplicates it removes, and it carries the "already tried, + * same failure" signal no window size provides. Repeats collapse to a count, so a thrashing loop + * reads as `shell ./gradlew assemble -> exit 1 (x4)` rather than four separate lines. + */ +private const val LEDGER_MAX_LINES = 80 +private const val LEDGER_OUTCOME_CHARS = 90 +private const val LEDGER_TARGET_CHARS = 70 + +private val ledgerJson = Json { ignoreUnknownKeys = true } + +/** + * Folds this round's tool entries into `tool target -> outcome` lines. Pairs the `assistantToolCall` + * entry with its `toolResult` by sourceId (both are stamped with it in dispatchToolCalls), so this + * reads only what the loop already has — no event-store re-read. + */ +internal fun ledgerLinesFrom(entries: List): List { + val results = entries.filter { it.sourceType == "toolResult" }.associateBy { it.sourceId } + return entries.filter { it.sourceType == "assistantToolCall" }.mapNotNull { call -> + val request = runCatching { + ledgerJson.decodeFromString(ToolCallRequest.serializer(), call.content) + }.getOrNull() ?: return@mapNotNull null + val target = ledgerTarget(request.function.arguments) + val outcome = ledgerOutcome(results[call.sourceId]?.content) + listOfNotNull(request.function.name, target).joinToString(" ") + " -> " + outcome + } +} + +/** The one argument worth showing: the path/command a call acted on, else the first string value. */ +private fun ledgerTarget(arguments: String): String? { + val obj = runCatching { ledgerJson.parseToJsonElement(arguments).jsonObject }.getOrNull() ?: return null + val strings = obj.mapValues { (_, v) -> (v as? JsonPrimitive)?.takeIf { it.isString }?.content } + val picked = listOf("path", "command", "file_path", "query", "pattern") + .firstNotNullOfOrNull { strings[it] } + ?: strings.values.filterNotNull().firstOrNull() + return picked?.trim()?.take(LEDGER_TARGET_CHARS) +} + +private fun ledgerOutcome(result: String?): String = when { + result == null -> "no result" + result.startsWith("ERROR:") || result.startsWith("BLOCKED:") -> + result.lineSequence().first().take(LEDGER_OUTCOME_CHARS) + else -> "ok" +} + +/** + * Renders the pinned ledger entry. Identical lines collapse to one with a repeat count — that count + * IS the signal, so it must not be lost to dedup. Keeps the most recent [LEDGER_MAX_LINES] distinct + * lines and says how many it dropped, rather than silently truncating. + */ +internal fun buildActionLedgerEntry(lines: List): ContextEntry? { + if (lines.isEmpty()) return null + val counted = LinkedHashMap() + lines.forEach { counted[it] = (counted[it] ?: 0) + 1 } + val dropped = (counted.size - LEDGER_MAX_LINES).coerceAtLeast(0) + val content = buildString { + append("## Already done this stage\n") + append( + "Every tool call you have made in this stage, in order, with its outcome. Do NOT repeat " + + "a call listed here: it will return the same thing. A line marked (xN) is a call you " + + "have already retried N times without the result changing — try something different " + + "or move on.\n", + ) + if (dropped > 0) append("- ... $dropped earlier calls omitted\n") + counted.entries.drop(dropped).forEach { (line, count) -> + append("- ").append(line) + if (count > 1) append(" (x").append(count).append(")") + append("\n") + } + }.trimEnd() + return ContextEntry( + id = ContextEntryId(UUID.randomUUID().toString()), + layer = ContextLayer.L1, + content = content, + sourceType = "actionLedger", + sourceId = "action-ledger", + tokenEstimate = content.length / 4, + // Rebuilt every round, like remainingDelta — USER, so it never invalidates the cached + // system prefix and never competes with the stage's own instructions. + role = EntryRole.USER, + ) +} diff --git a/core/kernel/src/main/kotlin/com/correx/core/kernel/orchestration/BuildGateToolchain.kt b/core/kernel/src/main/kotlin/com/correx/core/kernel/orchestration/BuildGateToolchain.kt index 81154eee..87f713f2 100644 --- a/core/kernel/src/main/kotlin/com/correx/core/kernel/orchestration/BuildGateToolchain.kt +++ b/core/kernel/src/main/kotlin/com/correx/core/kernel/orchestration/BuildGateToolchain.kt @@ -14,6 +14,16 @@ internal fun SessionOrchestrator.stageProducedToolchain( stageId: StageId, ): KindContractTable.Toolchain? = toolchainForPaths(stageWrittenPaths(sessionId, stageId)) +/** + * The toolchain the execution gate runs for a stage: what this stage wrote, else what the session + * wrote. The gate is armed session-scoped, so a stage that wrote nothing (a reviewer) must not fall + * through to the profile's flat `build` alias and run some other stack's build (#705). + */ +internal fun resolveGateToolchain( + stagePaths: List, + sessionPaths: List, +): KindContractTable.Toolchain? = toolchainForPaths(stagePaths) ?: toolchainForPaths(sessionPaths) + internal fun toolchainForPaths(paths: List): KindContractTable.Toolchain? = paths.asReversed().firstNotNullOfOrNull { path -> KindInference.kindFor(path)?.let(KindContractTable::toolchainFor) 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 7cc4b953..aef7d0bb 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 @@ -106,6 +106,12 @@ internal const val TOOL_RESULT_HEAD_LINES = 60 internal const val TOOL_RESULT_TAIL_LINES = 60 internal const val TOOL_OUTPUT_TOOL = "tool_output" +// ponytail: near-greedy, not greedy (temperature 0) — a hard 0 makes a stuck model repeat the same +// failing call forever, and the repeat rate is already the problem (#706). Fixed constants, not +// config: they describe the field type (argv/path), not an operator preference. +internal const val TOOL_CALL_TEMPERATURE = 0.15 +internal const val TOOL_CALL_TOP_P = 0.9 + /** * Frame an over-cap tool output as `header` + head lines + a truncation marker (naming the * [tool_output] ref that retrieves the full text) + tail lines. Head and tail are each char-capped @@ -332,8 +338,9 @@ abstract class SessionOrchestrator( // A stage that grants tools needs a tool-calling model, so request ToolCalling on top of any // declared capabilities — the capability-aware strategy then ranks eligible providers by their // ToolCalling score and routes the stage to the best tool-caller. + val toolCallRound = withTools && stageConfig.allowedTools.isNotEmpty() val requiredCapabilities = stageConfig.requiredCapabilities + - if (withTools && stageConfig.allowedTools.isNotEmpty()) setOf(ModelCapability.ToolCalling) else emptySet() + if (toolCallRound) setOf(ModelCapability.ToolCalling) else emptySet() // Routing itself can fail transiently (provider mid-crash-recovery) — it must be retryable // like any other inference failure, not escape and kill the whole session (see #299). val provider = try { @@ -353,7 +360,15 @@ abstract class SessionOrchestrator( sessionId = sessionId, stageId = stageId, contextPack = contextPack, - generationConfig = stageConfig.generationConfig, + // A round that carries tools is answered with argv, paths and flags — fields where + // exactly one string is correct. Chat-temperature sampling there emits `./gradlew_`, + // `npm_prefix=frontend`, `create_vite@latest` (#710, run 954da1a9). Go near-greedy for + // those rounds and keep the operator's sampling for prose rounds (artifact + review). + generationConfig = if (toolCallRound) { + stageConfig.generationConfig.copy(temperature = TOOL_CALL_TEMPERATURE, topP = TOOL_CALL_TOP_P) + } else { + stageConfig.generationConfig + }, responseFormat = responseFormat, tools = if (!withTools) { emptyList() 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 da594f96..0b587dda 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 @@ -318,6 +318,10 @@ internal suspend fun SessionOrchestrator.executeStage( // already seen — a stage reading N distinct files before writing is legitimate context // gathering, not a loop, and shouldn't trip the same counter as re-reading the same file. val seenReadFingerprints = mutableSetOf() + // #706: every tool call this stage made, one line each, pinned so it outlives L2 eviction. + // Without it the model's memory is the last five call/result pairs and it re-issues calls it + // already made (29% of run 954da1a9's tool calls were byte-identical repeats). + val ledgerLines = mutableListOf() // Set when the model produces its artifact via the emit_artifact tool instead of a final // JSON message; overrides the post-loop capture of the (then-empty) assistant text. var llmArtifactOverride: String? = null @@ -473,6 +477,12 @@ internal suspend fun SessionOrchestrator.executeStage( // loop as tool-result context so the model can see the error and adapt (bounded by // MAX_TOOL_ROUNDS). Only FATAL: failures (handled above) abort the stage. accumulatedEntries = accumulatedEntries + toolEntries + // #706: fold this round into the pinned action ledger before any pushBack rebuilds the pack, + // so a nudged round already carries the "you have tried this N times" line. + ledgerLines += ledgerLinesFrom(toolEntries) + buildActionLedgerEntry(ledgerLines)?.let { ledger -> + accumulatedEntries = accumulatedEntries.filterNot { it.sourceType == "actionLedger" } + ledger + } // Read-loop breaker: this round called only read-only tools yet the stage still owes a // file_written artifact. Left alone the model keeps reading until MAX_TOOL_ROUNDS and // never writes (F-018 nudges only cover a prose turn or a premature stage_complete, not 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 cec7451b..099f8c96 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 @@ -174,7 +174,15 @@ internal suspend fun SessionOrchestrator.runExecutionGate( val runner = staticAnalysisRunner val workspaceRoot = effectives.policy?.workspaceRoot if (runner == null || workspaceRoot == null) return StageExecutionResult.Success(emptyList()) - val toolchain = stageProducedToolchain(sessionId, stageId)?.profileKey + // The gate is turned on session-scoped (sessionProducedBuildTarget) but the toolchain was read + // stage-scoped, so a stage that wrote nothing (a reviewer) resolved null and fell back to the + // flat `build` alias — run 954da1a9 ran `./gradlew assemble` on an all-`frontend/**` session and + // died on it (#705). Two lookups deciding one command must share a scope: fall back to the + // session's own manifest, the same one that armed the gate, before the flat alias. + val toolchain = resolveGateToolchain( + stageWrittenPaths(sessionId, stageId), + sessionWrittenPaths(sessionId), + )?.profileKey val command = expectation.commandFor(profileCommands, toolchain) if (command.isNullOrBlank()) { log.warn( diff --git a/core/kernel/src/main/kotlin/com/correx/core/kernel/orchestration/SessionOrchestratorToolExec.kt b/core/kernel/src/main/kotlin/com/correx/core/kernel/orchestration/SessionOrchestratorToolExec.kt index 43f5bb74..05e4705e 100644 --- a/core/kernel/src/main/kotlin/com/correx/core/kernel/orchestration/SessionOrchestratorToolExec.kt +++ b/core/kernel/src/main/kotlin/com/correx/core/kernel/orchestration/SessionOrchestratorToolExec.kt @@ -305,12 +305,22 @@ internal suspend fun SessionOrchestrator.dispatchToolCalls( assessment } val plane2Prompts = plane2Risk?.recommendedAction == RiskAction.PROMPT_USER + // #712: a write that lands inside the stage's declared manifest (or the claimed task's + // affected_paths) needs no interrupt — the operator already approved that path set when the + // plan was approved, and ManifestContainmentRule BLOCKs anything outside it above, so + // reaching here with a clean plane-2 verdict IS the containment proof. Run 954da1a9 spent + // 19 min over 94 prompts, every one APPROVED with no steering. DENY mode still denies. + val writeInsideManifest = plane2Risk?.recommendedAction == RiskAction.PROCEED && + approvalMode != ApprovalMode.DENY && + effectiveManifest.isNotEmpty() && + tool?.requiredCapabilities?.contains(ToolCapability.FILE_WRITE) == true // A steering note attached to a human approval is captured here and injected after the // tool result, so the same-stage loop re-infers with it and the model acts on the note. var approvalNote: String? = null - if ((tier.isAtMost(Tier.T1) && !plane2Prompts) || alreadyGranted) { - // no approval needed — either within the auto-approve tier, or this out-of-workspace - // read path was already approved earlier this session (this-path-this-session). + if ((tier.isAtMost(Tier.T1) && !plane2Prompts) || alreadyGranted || writeInsideManifest) { + // no approval needed — within the auto-approve tier, an out-of-workspace read path + // already approved earlier this session (this-path-this-session), or a write contained + // by the stage's declared manifest. } else { // Grants in effect = this session's own (SESSION/STAGE) unioned with the // cross-session ledger (PROJECT/GLOBAL). projectId is derived from the bound diff --git a/core/kernel/src/test/kotlin/com/correx/core/kernel/orchestration/ActionLedgerTest.kt b/core/kernel/src/test/kotlin/com/correx/core/kernel/orchestration/ActionLedgerTest.kt new file mode 100644 index 00000000..472e0913 --- /dev/null +++ b/core/kernel/src/test/kotlin/com/correx/core/kernel/orchestration/ActionLedgerTest.kt @@ -0,0 +1,76 @@ +package com.correx.core.kernel.orchestration + +import com.correx.core.context.model.ContextEntry +import com.correx.core.context.model.ContextLayer +import com.correx.core.context.model.EntryRole +import com.correx.core.events.types.ContextEntryId +import com.correx.core.inference.ToolCallFunction +import com.correx.core.inference.ToolCallRequest +import kotlinx.serialization.json.Json +import org.junit.jupiter.api.Assertions.assertEquals +import org.junit.jupiter.api.Assertions.assertFalse +import org.junit.jupiter.api.Assertions.assertNull +import org.junit.jupiter.api.Assertions.assertTrue +import org.junit.jupiter.api.Test + +class ActionLedgerTest { + + private fun round(id: String, tool: String, args: String, result: String): List = listOf( + entry(id, "assistantToolCall", Json.encodeToString( + ToolCallRequest.serializer(), + ToolCallRequest(id = id, function = ToolCallFunction(tool, args)), + ), EntryRole.ASSISTANT), + entry(id, "toolResult", result, EntryRole.TOOL), + ) + + private fun entry(sourceId: String, sourceType: String, content: String, role: EntryRole) = ContextEntry( + id = ContextEntryId(sourceId + sourceType), + layer = ContextLayer.L2, + content = content, + sourceType = sourceType, + sourceId = sourceId, + tokenEstimate = content.length / 4, + role = role, + ) + + @Test + fun `a call folds to one tool-target-outcome line`() { + val lines = ledgerLinesFrom(round("1", "file_read", """{"path":"frontend/package.json"}""", "{...}")) + assertEquals(listOf("file_read frontend/package.json -> ok"), lines) + } + + @Test + fun `a failed call keeps its first error line`() { + val lines = ledgerLinesFrom( + round("1", "shell", """{"command":"./gradlew assemble"}""", "ERROR: exit 1\nCould not resolve io.ktor"), + ) + assertEquals(listOf("shell ./gradlew assemble -> ERROR: exit 1"), lines) + } + + @Test + fun `repeats collapse to a count instead of N lines`() { + val line = "list_dir frontend -> ok" + val content = buildActionLedgerEntry(List(9) { line })!!.content + assertTrue(content.contains("$line (x9)"), content) + assertEquals(1, content.lines().count { it.contains("list_dir") }, content) + } + + @Test + fun `the ledger is empty until a call is made`() { + assertNull(buildActionLedgerEntry(emptyList())) + } + + @Test + fun `an overlong ledger drops the oldest lines and says so`() { + val content = buildActionLedgerEntry((1..100).map { "file_read f$it.kt -> ok" })!!.content + assertTrue(content.contains("20 earlier calls omitted"), content) + assertFalse(content.contains("f1.kt"), content) + assertTrue(content.contains("f100.kt"), content) + } + + @Test + fun `a call whose result never landed is still recorded`() { + val call = round("1", "file_write", """{"path":"a.kt","content":"x"}""", "ok").first() + assertEquals(listOf("file_write a.kt -> no result"), ledgerLinesFrom(listOf(call))) + } +} diff --git a/core/kernel/src/test/kotlin/com/correx/core/kernel/orchestration/BuildPrerequisiteDecisionTest.kt b/core/kernel/src/test/kotlin/com/correx/core/kernel/orchestration/BuildPrerequisiteDecisionTest.kt index c98014aa..4055e979 100644 --- a/core/kernel/src/test/kotlin/com/correx/core/kernel/orchestration/BuildPrerequisiteDecisionTest.kt +++ b/core/kernel/src/test/kotlin/com/correx/core/kernel/orchestration/BuildPrerequisiteDecisionTest.kt @@ -24,6 +24,18 @@ class BuildPrerequisiteDecisionTest { assertEquals(KindContractTable.Toolchain.JVM, toolchainForPaths(listOf("core/kernel/FooService.kt"))) } + @Test + fun `a stage that wrote nothing falls back to the session toolchain (#705)`() { + val session = listOf("frontend/package.json", "frontend/src/App.tsx", "README.md") + assertEquals(KindContractTable.Toolchain.NODE, resolveGateToolchain(emptyList(), session)) + // The stage's own writes still win when it has any. + assertEquals( + KindContractTable.Toolchain.JVM, + resolveGateToolchain(listOf("core/kernel/FooService.kt"), session), + ) + assertNull(resolveGateToolchain(emptyList(), listOf("README.md"))) + } + private val reason = "stage impl repeatedly referenced missing build prerequisite 'frontend/package.json' " + "(3 blocked attempts). Create or repair the project setup before continuing." diff --git a/docs/audits/2026-08-11-session-954da1a9-postmortem.md b/docs/audits/2026-08-11-session-954da1a9-postmortem.md new file mode 100644 index 00000000..d671594f --- /dev/null +++ b/docs/audits/2026-08-11-session-954da1a9-postmortem.md @@ -0,0 +1,347 @@ +# Session post-mortem: 954da1a9 (freestyle web-ui run) + +Date: 2026-08-11 +Branch: `master` +HEAD at analysis: `700f59ef` +Session: `954da1a9-56cb-48da-a7dc-3ef472c42bf6`, 11:52:14 to 13:31:10 UTC +Method: full event-log replay from `~/.config/correx/correx.db` plus CAS artifact reads +(`scripts/artread.py`). No inference was re-run. Every claim below cites a session sequence +number, an artifact hash, or a source line. + +## Executive summary + +The run did not fail at the end. It failed at minute 7. It then passed every gate for 92 +minutes and died on a build gate that was never its job. + +| | | +|---|---| +| Wall clock | 98.9 min | +| Inference | 73.8 min, 274 rounds, p50 12.9 s, max 190 s | +| Blocked on approvals | 19.0 min, 94 pauses, 100% approved, 0 steering | +| Tool calls | 258, of which **110 were waste** (43%) | +| Files written | 25 `FileWritten` events, **9 distinct files** | +| UI views delivered | 0 of 8 requested | + +`postcss.config.js` was written 5 times. `QueryClientProvider.tsx` was written 8 times. + +Eight findings. Seven are open. One (§1) was closed by #699 after the run ended. +Vikunja: #705, #706, #707, #708, #709, #710, #711, #712. + +## Timeline + +| seq | time | event | +|---|---|---| +| 2 | 11:52 | `InitialIntent` — eight-view web UI, React/Vite/Tailwind/Ethos/TanStack | +| 171 | 11:57 | discovery artifact: `brief.scope` holds all 8 items, `ready: true`, no questions | +| 236 | 11:59 | dod artifact: **4 criteria, all `part: "Project Foundation"`** | +| 249-256 | 12:02 | execution plan locked; plan-compile, plan-lint (score 0) and grounding all PASS | +| 609 | 12:14 | first retry — Tailwind v4 PostCSS plugin moved, `npm run build` exit 1 | +| 951 | 12:28 | `WorkspaceVerificationObserved` PROJECT `npm --prefix frontend run build` **passed** | +| 1510 | 12:41 | retry — `verbatimModuleSyntax` type-only import | +| 1697 | 12:49 | retry — `final_review` PROJECT gate runs **`./gradlew assemble`**, exit 1 | +| 1994 | 13:00 | reviewer correctly names `~/.gradle/init.d/offline.gradle` as the cause | +| 2306 | 13:12 | reviewer **retracts** the correct diagnosis: "was not found" | +| 2374 | 13:14 | `FailureTicketOpened` `stage_loop_break`, 6 identical failures, routes to recovery | +| 2584 | 13:23 | `RefinementIteration` recovery→final_review, back into the same wall | +| 2723-2731 | 13:30 | 4 retries in 40 s on `No provider satisfies capabilities [ToolCalling]` | +| 2733 | 13:31 | `WorkflowFailed` | + +## 1. Scope collapse at the analyst (closed by #699) + +Discovery settled 8 scope items (artifact `8053f0d7`): session driver, sessions list, +workflow listing, event log viewer, artifacts/timeline, ideas board, profiles, configuration. + +The analyst emitted 4 criteria (artifact `5d46f805`), every one `part: "Project Foundation"`. +Its summary reads *"Initialize the Correx web-ui project with the required tech stack."* The +architect planned against the shrunken DoD (`13d08806`): `scaffold_frontend`, +`install_styling_and_ethos`, `configure_tanstack_query`, `final_review`. Its `goal` field +never mentions a view. + +Nothing downstream sees discovery again. Implementer stages receive `neededArtifact: dod` and +nothing else, so the loss is total and silent from seq 236 onward. + +All three plan gates passed, because all three grade structure and the structure was sound. + +Closed by `ScopeCoverage` (#699, commit `700f59ef`, 19:56 the same day). Stated ceiling holds: +it catches a dropped index, not a weak criterion. + +## 2. The build gate ran the wrong toolchain (#705, open) + +[`SessionOrchestratorGates2.kt:156-178`](../../core/kernel/src/main/kotlin/com/correx/core/kernel/orchestration/SessionOrchestratorGates2.kt#L156-L178): + +- `sessionProducedBuildTarget(sessionId)` is **session-scoped** and turns the gate on. +- `stageProducedToolchain(sessionId, stageId)` is **stage-scoped** and returns null when the + stage wrote no files. +- `commandFor(profileCommands, null)` then falls back to the flat `build` alias. + +`final_review` is a reviewer. Its tools are `[file_read]` and the plan set +`build_expectation: "none"`. It wrote nothing, so the toolchain lookup returned null and the +gate ran `./gradlew assemble` against a run whose every write was `frontend/**`. The correct +command, `node.build` = `npm --prefix frontend run build`, had already passed at seq 951, 1286 +and 1683. + +The failure it produced was environmental: + +``` +> Could not resolve io.ktor:ktor-client-websockets:3.0.3. + > No cached version ... available for offline mode. +``` + +That is `~/.gradle/init.d/offline.gradle` on workpc, outside the workspace jail, predating the +session. + +Cost: 4 retries plus a recovery detour, seq 1694 to 2733. That is **32.5 min, 33% of wall +clock**, and the run's death. + +Fix: when the stage produced no toolchain, fall back to the session's toolchain (the same +manifest `sessionProducedBuildTarget` already reads) before the flat alias. Two lookups +deciding one command must share a scope. + +## 3. The harness destroyed a correct diagnosis (#707, open) + +Only 4 of 274 inference rounds returned any text at all (§5). Three of them are this finding. + +- **seq 1994**: *"The build failed because the Gradle environment is in offline mode... The + `org.gradle.offline=true` flag is present in `~/.gradle/init.d/offline.gradle`."* Correct + root cause, reached in about ten minutes. +- **seq 2306**: *"Since `~/.gradle/init.d/offline.gradle` was not found, I will check for global + Gradle properties..."* The model **retracted the correct answer**, because at seq 1793 + `file_read ~/.gradle/gradle.properties` returned `[REFERENCE_EXISTS] ... does not exist. Do + NOT keep retrying`. The tool layer does not expand `~`, and the path is outside the jail + regardless. The guard reported a true thing as false. +- **seq 2715**: *"the DoD criteria do not require a successful backend build... the frontend + build passed... I cannot fix the backend dependencies, it's out of scope. However, since the + PROJECT build gate is a hard gate, I must reject it."* + +Right three times, and wrong-footed each time by the harness. + +The structural gap: every verdict the reviewer can emit (`approved`, `changes_requested`, +`rejected`) routes back into the agent. There is no way to say *this failure is not +attributable to this run*. `FailureTicketOpened` fired correctly at seq 2374 after 6 identical +failures, routed to `recovery`, and recovery routed straight back (`RefinementIteration`, +seq 2584). + +The retry feedback closed with *"Do not re-discover unrelated files before it builds."* +The harness forbade the only move that would have worked. + +## 4. Ten-turn amnesia inside a mostly empty window (#706, open) + +176 `ContextTruncated` events, every one L2, `entriesDropped` climbing to 40+ and pinning. + +| stage | truncations | max dropped | +|---|---|---| +| discovery | 14 | 28 | +| scaffold_frontend | 40 | 40 | +| install_styling_and_ethos | 20 | 40 | +| configure_tanstack_query | 21 | 16 | +| final_review | 62 | 42 | +| recovery | 16 | 32 | + +[`DefaultContextPackBuilder.kt:380`](../../core/context/src/main/kotlin/com/correx/core/context/builder/DefaultContextPackBuilder.kt#L380) +uses `CompressionStrategy.Conversation()`. The default is `keepLast = 10` +([`CompressionStrategy.kt:7`](../../core/context/src/main/kotlin/com/correx/core/context/compression/CompressionStrategy.kt#L7)). +L2 holds **5 tool-call/result pairs**. The assembled pack at round 31 of `scaffold_frontend` +(seq 599) confirms it. It carries 4 L0 entries, 2 L1, 3 L3, and 10 L2, exactly the last ten. + +**The window is not full.** Those same packs run 1.7k to 13k tokens, median 5.3k. The cap is +entry count, not tokens, so the model is starved at 5k while a local model's context sits +mostly idle. + +Consequence, measured: **76 byte-identical repeat calls**, 29% of all tool invocations. + +| repeats | stage | call | +|---|---|---| +| 9 | final_review | `list_dir {"path":"frontend"}` | +| 7 | scaffold_frontend | `list_dir {"path":"frontend/"}` | +| 6 | final_review | `file_read {"path":"gradle.properties"}` | +| 5 | scaffold_frontend | `file_read {"path":"frontend/package.json"}` | +| 4 | final_review | `shell ./gradlew assemble --no-configuration-cache` | + +The `decisionJournal` is the only memory surviving eviction. Its complete content at the end of +the run: + +``` +- Goal: Write a web-ui for Correx. ... +- scaffold_frontend: 1 retry, resolved — stage completed +- configure_tanstack_query: 1 retry, resolved — stage completed +- final_review: 3 retries, resolved — stage completed +``` + +Bookkeeping, not knowledge. + +Preferred fix is not a larger `keepLast` (it costs local-inference latency and still forgets). +Add a deterministic **action ledger**. One line per tool call for the stage: tool, target, +exit, short result hash. Never evicted. 258 calls at roughly 12 tokens is about 3k tokens for +a whole run. That is cheaper than the duplicates it removes, and it carries the "already +tried, same failure" signal no window size provides. + +Related: the plan set `kind: "process_result"` on every stage, which silently disables the +`file_written` manifest injection +([`SessionOrchestratorContext.kt:458`](../../core/kernel/src/main/kotlin/com/correx/core/kernel/orchestration/SessionOrchestratorContext.kt#L458) +builds it only for `file_written` slots). A feature built to prevent this thrash was switched +off by one planner field. + +## 5. There was no chain of thought + +**270 of 274 inference rounds returned an empty response body** (blake3 +`af1349b9...`, the empty string). Per stage: discovery 25/25, analyst 9/9, architect 1/1, +scaffold_frontend 62/62, install_styling 31/31, configure_tanstack 32/33, final_review 83/86, +recovery 27/27. + +Every assistant message in the assembled prompts is a bare function call with no content. The +system prompt instructs *"make one coherent step per response."* + +The loop reduces to this. Context in, one tool call out, no reasoning, no plan, no self-check, +with a 10-turn memory (§4) at temperature 1.0 (§6). The 29% repeat rate is what that +combination produces. Surfacing reasoning is already tracked as #298. This finding is that +none was produced to surface. + +## 6. Temperature 1.0 on structured arguments (#710, open) + +[`Main.kt:435-451`](../../apps/server/src/main/kotlin/com/correx/apps/server/Main.kt#L435-L451) +sends `[sampling] temperature = 1.0, top_p = 0.95, top_k = 64` on **every** stage inference. +Chat-quality settings applied to fields where exactly one string is correct. + +| seq | emitted | intended | +|---|---|---| +| 277 | `create_vite@latest` | `create vite@latest` | +| 465 | `npm_prefix=frontend` | `npm --prefix frontend` | +| 1302 | `npm_install_package` | `npm install ` | +| 2060 | `./gradlew_` | `./gradlew` | +| 2708 | `gradlew` | `./gradlew` | + +Space-to-underscore substitution on an argv field is sampling noise. correx already sends a +per-request `GenerationConfig`, so the override is per-call. Keep 1.0 for artifact and review +prose. Go near-greedy when the response is constrained to tool calls. #303 repairs these after +the fact. Not emitting them is cheaper. + +**Same task, second half: files are copied through the token stream.** There is no `file_copy` +tool. `ethos-icons.svg` moved by a `file_read` of 6124 characters, then a `file_write` of 6845 +characters of tool-call arguments. A `READ_BEFORE_WRITE` rejection round came first (seq 540), +when the model tried to write before reading. About 6 rounds and 13k tokens for two static +assets, at temperature 1.0, with silent corruption possible. The project profile says *"COPY +design-system/ethos.tokens.css and ethos-icons.svg into the project and import them, do NOT +read their contents."* The toolset offers no way to obey that instruction. + +## 7. The plan contradicts itself, and the kernel sides against the model (#711, open) + +The architect emits a stage's `prompt` and its `writes` manifest independently. They disagreed +twice, and the kernel enforces `writes`. + +- `install_styling_and_ethos` prompt step 4: *"Copy ... into `frontend/src/assets/ethos/`."* + Manifest: `[tailwind.config.js, postcss.config.js, src/index.css]`. Result at seq 1086: + `[PATH_OUTSIDE_MANIFEST]`. +- `configure_tanstack_query` prompt step 2: *"Create a `QueryClient` instance and a wrapper + component."* Manifest: `[frontend/src/lib/query-client.ts]`. Blocked at seq 1321. + `QueryClientProvider.tsx` was then written 8 times. + +Both fields are already parsed by the plan-compile gate, so this is a string check over paths +the prompt itself spells out. + +## 8. The stage prompt never updates (#709, open) + +Verified against the assembled prompt for round 31 of `scaffold_frontend` (artifact +`47ebf7b0`). The system message still read: + +> Use the `shell` tool to initialize a new React + TypeScript project using Vite in the +> `frontend/` directory. Run `npm create vite@latest frontend -- --template react-ts` ... + +The project had existed for 30 rounds. The instruction is a fixed string from the execution +plan, and nothing recomputes it against what exists. Combined with §4, the model's strongest +signal every round is an order to redo step one. + +The plan's stage prompts are literal command transcripts written from training memory, and both +were wrong: + +- `npx tailwindcss init -p`. `npx` is **blocked by tool policy**, rejected at seq 329 and 988. + The policy message helpfully names the alternative. A plan-compile lint rejecting a plan that + names a forbidden executable is roughly ten lines. +- `npm install -D tailwindcss postcss autoprefixer` plus `tailwind.config.js` is the **Tailwind + v3** procedure. `npm install tailwindcss` now resolves v4, where the PostCSS plugin moved to + `@tailwindcss/postcss`. That is the build failure at seq 609 verbatim, and 22 rounds of + thrash followed it. + +## 9. Ninety-four approvals that carried no information (#712, open) + +Every `ApprovalDecisionResolved` in the run is `APPROVED` with `reason: null` and +`userSteering: null`. Pause durations: p50 13.6 s, p90 19.0 s, max 20.3 s. Total **19.0 min, +19% of wall clock**. + +By tool: `shell` T2 x63, `file_write` T2 x13, `file_edit` T3 x10, remainder task and delete +calls. + +Two problems. The gate is priced per tool call, so it charges 94 interrupts for one operator +intention. The attention also lands in the wrong place. All 94 chances to intervene were +spent on "may I run `npm install`". The decision that determined the outcome (seq 236, §1) +had no operator checkpoint at all. + +Auto-approving reads and writes inside the stage's declared `writes` manifest costs nothing in +safety, because `PATH_OUTSIDE_MANIFEST` already enforces that boundary. It returns most of the +19 minutes. + +## Waste breakdown + +258 tool calls. 110 wasted, counting each call once (repeat, failed, or policy-rejected). + +| stage | waste / total | | +|---|---|---| +| discovery | 2 / 24 | 8% | +| analyst | 1 / 8 | 12% | +| scaffold_frontend | 26 / 60 | 43% | +| install_styling_and_ethos | 11 / 30 | 37% | +| configure_tanstack_query | 9 / 31 | 29% | +| **final_review** | **55 / 79** | **70%** | +| recovery | 6 / 26 | 23% | + +Context composition across all 279 assemblies: `toolResult` 47.9%, `projectProfile` 8.4%, +`assistantToolCall` 8.1%, `decisionJournal` 6.8%, `retryFeedback` 5.4%. By layer: L2 56.0%, +L0 18.0%, L1 13.2%, L3 12.9%. + +## Ranked remediation + +inference, from this run's own timings: + +1. **Baseline every gate command at session start (#708).** One build at t=0. A command already + red before the agent touched anything reports `pre-existing` and never fails a stage. + Recovers `final_review` plus most of `recovery`. **~40 min.** +2. **Fix the toolchain scope mismatch (#705).** One line. Prevents the same 40 minutes by an + independent route. Do both. +3. **Deterministic action ledger (#706).** ~16 min of duplicate calls, plus the loops they feed. + Raising `keepLast` alone does not fix it. +4. **Auto-approve inside the declared manifest (#712).** **~17 min.** +5. **Split the sampling config (#710).** Near-greedy for tool-call rounds. + +Items 1, 3 and 4 account for roughly 70 of the 99 minutes. + +## The structural point + +Every gate in this run was local. Each asked whether a stage did its stage correctly. None +asked whether the run was still building the requested thing, until `final_review` at minute 90 +graded against the already-collapsed DoD. + +The path from intent to work is five lossy model summarisations: intent, discovery, DoD, plan, +stage prompt. Only the last link was ever checked. #699 now checks the DoD-to-discovery link, +which is the one that broke here. The general shape remains. + +The cheap version needs no model. The intent names eight views. After three of four stages, +`FileWritten` holds nine files and none is a view. That comparison is a set difference over +recorded events and costs nothing. It would have fired at minute 25, not failed silently at +minute 99. + +The deeper item, for #168: this run produced a genuinely valuable artifact and discarded it. A +local model correctly diagnosed a Gradle offline-mode misconfiguration in ten minutes from +build output alone. That knowledge existed at seq 1994 and was gone by seq 2306. A guard +reporting a real file as missing erased it. The event log still has it. Nothing reads it back. + +## Verification and scope + +- fact: every number above is derived from `events` rows for session `954da1a9` and from CAS + artifacts resolved through `~/.config/correx/artifacts/index.sqlite`. No inference re-run. +- fact: `ScopeCoverage` (#699) landed at 19:56 on 2026-08-11, after this session ended at + 13:31. It was not active during the run. +- inference: the `./gradlew assemble` failure is attributed to `~/.gradle/init.d/offline.gradle` + on workpc. The build output names offline mode and an uncached `io.ktor:ktor-client-websockets`. + The file itself was not read during this analysis. +- unknown: whether the 94 approvals were resolved by a human operator or by an auto-approver. + The tight 13-20 s clustering suggests a poll interval. No event records the decider. +- This post-mortem implemented no fixes. It filed Vikunja #705 through #712. 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 5cf737b8..c890bbb2 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 @@ -59,6 +59,10 @@ private const val RECOVERY_PROMPT = // leaves ample headroom for completion.) private const val DEFAULT_STAGE_TOKEN_BUDGET = 24576 +// Mirrors ShellTool.REMOTE_EXEC_RUNNERS (other module, not worth a dependency for three strings). +// Kept in sync by the message in RECOVERY_PROMPT, which names the same set. +private val BLOCKED_RUNNERS = listOf("npx", "bunx", "pnpx") + // A compiled freestyle stage must also lift its inference completion cap off StageConfig's 2048 // default, or the model is truncated (finishReason=length) mid-artifact — and a degenerating local // model burns the whole 2048 on garbage (e.g. a `<|channel>thought` repetition loop) before it can @@ -102,6 +106,7 @@ class ExecutionPlanCompiler( if (plan.stages.isEmpty()) throw WorkflowValidationException("execution_plan has no stages") validateTools(plan) validateScope(plan) + validatePromptCommands(plan) // Parse + validate every stage's declared build_expectation up front — also feeds the // deterministic build-gate guarantee below. @@ -264,6 +269,28 @@ class ExecutionPlanCompiler( } } + /** + * A stage prompt is a literal command transcript the architect writes from training memory, and + * the model obeys it verbatim for the whole stage. When it prescribes an executable the shell + * denylist blocks, every round burns on a call that can never run — run 954da1a9 spent rounds + * on `npx tailwindcss init -p`, rejected at seq 329 and 988 (#709). Reject at plan-compile time + * so the architect rewrites the step, rather than at seq 329 where nothing can rewrite it. + */ + private fun validatePromptCommands(plan: ExecutionPlanModel) { + val offenders = plan.stages.flatMap { s -> + BLOCKED_RUNNERS.filter { runner -> Regex("\\b$runner\\b").containsMatchIn(s.prompt) } + .map { s.id to it } + } + if (offenders.isEmpty()) return + val detail = offenders.joinToString(", ") { (stage, runner) -> "'$runner' in stage '$stage'" } + throw WorkflowValidationException( + "execution_plan prescribes blocked remote runner(s): $detail — the shell tool denies " + + "bare remote runners (${BLOCKED_RUNNERS.joinToString(", ")}). Rewrite the step to use " + + "the package manager directly (e.g. `npm create vite@latest -- --template " + + "react-ts`, `npm install ` then a local binary from node_modules/.bin).", + ) + } + /** * A field-equals edge can only ever fire if the producing stage's kind schema declares * that field — the kind's schema is also the LLM response format, so an undeclared field 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 6455eb13..3f2e3a6f 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 @@ -195,6 +195,54 @@ class ExecutionPlanCompilerTest { assertEquals(listOf("frontend/**"), graph.stages.getValue(StageId("impl_client")).touches) } + @Test + fun `a prompt prescribing a blocked remote runner is rejected at compile time (#709)`() { + val plan = """ + { + "goal": "add tailwind", + "stages": [ + { + "id": "install_styling", + "prompt": "Run npx tailwindcss init -p, then edit the generated config.", + "produces": "patch", + "needs": [], + "tools": ["shell"], + "writes": ["frontend/tailwind.config.js"] + } + ], + "edges": [ + { "from": "install_styling", "to": "done", "condition": { "type": "always_true" } } + ] + } + """.trimIndent() + val ex = assertThrows { compiler.compile(plan, "npx-workflow") } + assertTrue(ex.message!!.contains("npx"), "the rejection names the runner: ${ex.message}") + assertTrue(ex.message!!.contains("install_styling"), "and the stage: ${ex.message}") + } + + @Test + fun `a word merely containing a runner name does not trip the prompt lint (#709)`() { + val plan = """ + { + "goal": "document the sandbox", + "stages": [ + { + "id": "docs", + "prompt": "Explain why linux-sandboxing matters. Do not mention npxy tools.", + "produces": "patch", + "needs": [], + "tools": ["file_write"], + "writes": ["docs/sandbox.md"] + } + ], + "edges": [ + { "from": "docs", "to": "done", "condition": { "type": "always_true" } } + ] + } + """.trimIndent() + assertEquals(1, compiler.compile(plan, "docs-workflow").stages.size) + } + @Test fun `edge referencing unknown from-stage throws WorkflowValidationException`() { val bad = validPlan.replace("\"from\": \"analyse\"", "\"from\": \"nonexistent\"") -- 2.52.0 From 519290368f5d44bb06822f42c49dfc6186feade3 Mon Sep 17 00:00:00 2001 From: claude Date: Thu, 27 Aug 2026 01:33:16 +0400 Subject: [PATCH 3/6] fix(kernel,talkie,context): three instruction-corruption fixes from the 2026-08-26 context audit MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Each of the three lost or rewrote an instruction before the model saw it. 1. Orphan corrective nudges. pushBack() and the final tools-disabled emission built their nudge as a toolResult with a fresh sourceId, so it had no matching assistantToolCall and reconcileToolPairs() deleted it — the orchestrator believed it had corrected the model while the correction never reached the prompt. Affected the invalid-emit_artifact, premature-stage_complete, missing-write, read-loop, rejection-loop and final-JSON nudges. They are now USER turns (sourceType orchestratorCorrection, REQUIRED bucket, STRUCTURED in ContextClassifier so pruning cannot shred them), appended last so the builder's positional ordinal puts them at the end of the transcript. A superseded nudge is dropped rather than stacking stale demands. 2. Steering laundered through the router. The SteeringNoteAddedEvent carried the router model's paraphrase of the operator's message, not the message — negations, filenames, constraints and priority could change before the orchestrator saw them. It now carries the raw input; the router turn is still produced and shown as conversational acknowledgement, it is just not the mandate. Resolves the ponytail: note at TalkieFacade.kt:220. 3. Journal compaction erased its own history. compactIfNeeded() summarized only state.records while the reducer overwrote summaryArtifactId and dropped covered records, so the second compaction lost everything the first had preserved — and a low-salience-only batch replaced it with the "(no high-salience decisions)" fallback. Compaction is cumulative now, and a blank or fallback-only rewrite never replaces real history. Tests: rendered-prompt regression for (1) — verified to fail against the old toolResult shape — updated steering expectations for (2), two cumulative-compaction tests for (3). Full build green, 1831 tests, 0 failures. Co-Authored-By: Claude Opus 5 --- .../context/compression/ContextClassifier.kt | 5 ++ .../orchestration/JournalCompactionService.kt | 26 +++++++- .../SessionOrchestratorExecution.kt | 49 +++++++++------ .../JournalCompactionServiceTest.kt | 59 +++++++++++++++++++ .../com/correx/core/talkie/TalkieFacade.kt | 16 ++--- .../kotlin/DefaultContextPackBuilderTest.kt | 51 ++++++++++++++++ .../src/test/kotlin/TalkieFacadeTest.kt | 5 +- 7 files changed, 180 insertions(+), 31 deletions(-) diff --git a/core/context/src/main/kotlin/com/correx/core/context/compression/ContextClassifier.kt b/core/context/src/main/kotlin/com/correx/core/context/compression/ContextClassifier.kt index c41d547c..a1113306 100644 --- a/core/context/src/main/kotlin/com/correx/core/context/compression/ContextClassifier.kt +++ b/core/context/src/main/kotlin/com/correx/core/context/compression/ContextClassifier.kt @@ -35,8 +35,13 @@ class ContextClassifier { // shredded the JSON to a bare id + protected path, giving the model amnesia about what it // had already done → it re-issued the same call and looped. Structured = format-compress // ok, never pruned. + // "orchestratorCorrection" is an in-loop corrective USER turn authored by the + // orchestrator (invalid emit_artifact, premature stage_complete, read loop, missing write). + // Pruning it as freeform prose can shred the tool name or the negation that makes it + // actionable, which leaves the model with a vague complaint instead of an instruction. val STRUCTURED_SOURCES = setOf( "toolLog", "artifact", "config", "structured", "steeringNote", "assistantToolCall", + "orchestratorCorrection", ) } } diff --git a/core/kernel/src/main/kotlin/com/correx/core/kernel/orchestration/JournalCompactionService.kt b/core/kernel/src/main/kotlin/com/correx/core/kernel/orchestration/JournalCompactionService.kt index ae6210c0..d11b8f57 100644 --- a/core/kernel/src/main/kotlin/com/correx/core/kernel/orchestration/JournalCompactionService.kt +++ b/core/kernel/src/main/kotlin/com/correx/core/kernel/orchestration/JournalCompactionService.kt @@ -30,16 +30,38 @@ class JournalCompactionService( val highRecords = state.records.filter { it.kind.salience() == Salience.HIGH } val lowCount = state.records.count { it.kind.salience() == Salience.LOW } - val summaryText = if (highRecords.isEmpty()) { + // Compaction is CUMULATIVE. The reducer overwrites summaryArtifactId and drops every + // covered record, so a summary built from `state.records` alone erases the previous + // summary on the second compaction: the intent, approvals and steering it carried are + // neither an input here nor retained as a predecessor. Feed the prior summary back in. + val priorSummary = state.summaryArtifactId + ?.let { artifactStore.get(it) } + ?.toString(Charsets.UTF_8) + ?.takeIf { it.isNotBlank() } + + val summaryText = if (highRecords.isEmpty() && priorSummary == null) { "(no high-salience decisions to summarize)" + } else if (highRecords.isEmpty()) { + // Nothing new worth keeping — carry the prior summary forward untouched rather than + // replacing it with the fallback text. + priorSummary!! } else { val prompt = buildString { appendLine("Summarize the following key decisions concisely (≤200 words).") appendLine("Preserve all user intent, approvals, steering, and failures.") appendLine() + priorSummary?.let { + appendLine("Summary of earlier decisions (already compacted — preserve its content):") + appendLine(it) + appendLine() + appendLine("New decisions since then:") + } highRecords.forEach { appendLine("- [${it.kind}] ${it.summary}") } } - summarize(prompt) + // A blank or fallback-only rewrite must never replace real history. + summarize(prompt).takeIf { it.isNotBlank() } + ?: priorSummary + ?: "(no high-salience decisions to summarize)" } val summaryArtifactId = artifactStore.put(summaryText.toByteArray(Charsets.UTF_8)) 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 0b587dda..ab238a9d 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 @@ -2,6 +2,7 @@ package com.correx.core.kernel.orchestration import com.correx.core.context.builder.RequiredContextOverflowException import com.correx.core.context.model.ContextEntry +import com.correx.core.context.model.ContextBucket import com.correx.core.context.model.ContextLayer import com.correx.core.context.model.TokenBudget import com.correx.core.context.model.EntryRole @@ -351,18 +352,19 @@ internal suspend fun SessionOrchestrator.executeStage( return completedIds.isEmpty() } - // Append a corrective tool-result and re-run inference (bounded by MAX_TOOL_ROUNDS). + // Append a corrective USER turn and re-run inference (bounded by MAX_TOOL_ROUNDS). // Returns the new result rather than mutating inferenceResult, to preserve smart casts. + // + // NOT a toolResult: these nudges are orchestrator-authored, so they have no matching + // assistantToolCall, and reconcileToolPairs() drops every tool result whose call ID is absent + // (see its doc). The orchestrator believed it had corrected the model while the correction + // never reached the prompt. A USER turn is what this actually is — the operator side of the + // loop telling the model what to do next — and it survives reconciliation. Appended last so + // the builder's positional ordinal stamp puts it at the end of the transcript, and only one + // correction is ever live: a superseded nudge is dropped rather than stacking stale demands. suspend fun pushBack(nudge: String, forceWriteOnly: Boolean = false): InferenceResult { - accumulatedEntries = accumulatedEntries + ContextEntry( - id = ContextEntryId(UUID.randomUUID().toString()), - layer = ContextLayer.L2, - sourceType = "toolResult", - sourceId = UUID.randomUUID().toString(), - content = nudge, - tokenEstimate = estimateTokens(nudge), - role = EntryRole.TOOL, - ) + accumulatedEntries = accumulatedEntries.filterNot { it.sourceType == CORRECTION_SOURCE_TYPE } + + correctionEntry(nudge) currentContext = contextPackBuilder.build( id = ContextPackId(UUID.randomUUID().toString()), sessionId = sessionId, @@ -582,15 +584,8 @@ internal suspend fun SessionOrchestrator.executeStage( if (needsCleanEmission && !isCancelled(sessionId)) { val nudge = "Stop calling tools. Output the required '${llmEmittedSlots.first().name.value}' " + "artifact now as a single JSON object matching the schema — no tool calls, no commentary." - accumulatedEntries = accumulatedEntries + ContextEntry( - id = ContextEntryId(UUID.randomUUID().toString()), - layer = ContextLayer.L2, - sourceType = "toolResult", - sourceId = UUID.randomUUID().toString(), - content = nudge, - tokenEstimate = estimateTokens(nudge), - role = EntryRole.TOOL, - ) + accumulatedEntries = accumulatedEntries.filterNot { it.sourceType == CORRECTION_SOURCE_TYPE } + + correctionEntry(nudge) currentContext = contextPackBuilder.build( id = ContextPackId(UUID.randomUUID().toString()), sessionId = sessionId, @@ -687,3 +682,19 @@ internal suspend fun SessionOrchestrator.executeStage( } } } + +// Orchestrator-authored corrections. STRUCTURED in ContextClassifier (never token-pruned) and +// REQUIRED so a correction is never traded away for budget — the whole point of a nudge is that +// the next inference sees it. +internal const val CORRECTION_SOURCE_TYPE = "orchestratorCorrection" + +private suspend fun SessionOrchestrator.correctionEntry(nudge: String) = ContextEntry( + id = ContextEntryId(UUID.randomUUID().toString()), + layer = ContextLayer.L2, + sourceType = CORRECTION_SOURCE_TYPE, + sourceId = UUID.randomUUID().toString(), + content = nudge, + tokenEstimate = estimateTokens(nudge), + role = EntryRole.USER, + bucket = ContextBucket.REQUIRED, +) diff --git a/core/kernel/src/test/kotlin/com/correx/core/kernel/orchestration/JournalCompactionServiceTest.kt b/core/kernel/src/test/kotlin/com/correx/core/kernel/orchestration/JournalCompactionServiceTest.kt index a48615e4..3d1f4fdf 100644 --- a/core/kernel/src/test/kotlin/com/correx/core/kernel/orchestration/JournalCompactionServiceTest.kt +++ b/core/kernel/src/test/kotlin/com/correx/core/kernel/orchestration/JournalCompactionServiceTest.kt @@ -33,6 +33,65 @@ class JournalCompactionServiceTest { private fun makeRecord(seq: Long, kind: DecisionKind) = DecisionRecord(sequence = seq, kind = kind, summary = "summary of $kind") + // A store that really round-trips, so a second compaction can read back the first summary. + private fun recordingArtifactStore(): Pair> { + val blobs = mutableMapOf() + val store = object : ArtifactStore { + override suspend fun put(bytes: ByteArray): TypeId { + val id = TypeId("artifact-${blobs.size + 1}") + blobs[id] = bytes + return id + } + override suspend fun get(id: TypeId): ByteArray? = blobs[id] + override suspend fun flushBefore(commit: suspend () -> Unit) = commit() + } + return store to blobs + } + + @Test + fun `second compaction feeds the prior summary back into the prompt`(): Unit = runBlocking { + val (store, blobs) = recordingArtifactStore() + val prompts = mutableListOf() + val svc = JournalCompactionService(store, { prompts += it; "SUMMARY-${prompts.size}" }, { 100 }) + + val first = stateWithRecords(makeRecord(1, DecisionKind.INTENT)) + val emitted = mutableListOf() + assertTrue(svc.compactIfNeeded(SessionId("s1"), first, 500) { emitted += it }) + val firstId = (emitted.single() as JournalCompactedEvent).summaryArtifactId + + // Reducer semantics: covered records are gone, summaryArtifactId now points at SUMMARY-1. + val second = DecisionJournalState( + records = listOf(makeRecord(2, DecisionKind.INTENT)), + compactedThroughSequence = 1, + summaryArtifactId = firstId, + ) + emitted.clear() + assertTrue(svc.compactIfNeeded(SessionId("s1"), second, 500) { emitted += it }) + + assertTrue(prompts[1].contains("SUMMARY-1"), "prior summary must be an input: ${prompts[1]}") + val kept = blobs[(emitted.single() as JournalCompactedEvent).summaryArtifactId]!! + assertEquals("SUMMARY-2", kept.toString(Charsets.UTF_8)) + } + + @Test + fun `a low-salience-only batch carries the prior summary forward instead of erasing it`(): + Unit = runBlocking { + val (store, blobs) = recordingArtifactStore() + val priorId = store.put("EARLIER DECISIONS".toByteArray(Charsets.UTF_8)) + val svc = JournalCompactionService(store, { "should not be called" }, { 100 }) + + val state = DecisionJournalState( + records = listOf(makeRecord(2, DecisionKind.TRANSITION)), + compactedThroughSequence = 1, + summaryArtifactId = priorId, + ) + val emitted = mutableListOf() + assertTrue(svc.compactIfNeeded(SessionId("s1"), state, 500) { emitted += it }) + + val kept = blobs[(emitted.single() as JournalCompactedEvent).summaryArtifactId]!! + assertEquals("EARLIER DECISIONS", kept.toString(Charsets.UTF_8)) + } + @Test fun `returns false when token estimate is below threshold`(): Unit = runBlocking { val svc = JournalCompactionService(fakeArtifactStore(), { it }, tokenThreshold = { 2000 }) diff --git a/core/talkie/src/main/kotlin/com/correx/core/talkie/TalkieFacade.kt b/core/talkie/src/main/kotlin/com/correx/core/talkie/TalkieFacade.kt index a7a1dcbb..b785eade 100644 --- a/core/talkie/src/main/kotlin/com/correx/core/talkie/TalkieFacade.kt +++ b/core/talkie/src/main/kotlin/com/correx/core/talkie/TalkieFacade.kt @@ -217,16 +217,16 @@ class DefaultTalkieFacade( emitIdeasCaptured(sessionId, ideas.ideas) } - // ponytail: STEERING launders the user's text through the router LLM (the `content` above) - // before injecting it as a note. For a clear instruction that's an extra inference that can - // distort intent; the reformulation only earns its cost when the input is terse/context- - // dependent. Upgrade path: inject the raw (validated) input directly and skip the rewrite - // unless a heuristic flags the message as too short/ambiguous to stand alone. - val steeringEmitted = mode == ChatMode.STEERING && rawContent.isNotBlank() + // The steering note carries the operator's OWN text, not the router's paraphrase of it. + // Routing it through inference first let a lossy rewrite acquire the authority of a user + // directive — negations, filenames, constraints and priority could all change before the + // orchestrator saw them. The router turn above is still produced and shown to the operator + // as conversational acknowledgement; it is just not the mandate. + val steeringEmitted = mode == ChatMode.STEERING && input.isNotBlank() if (steeringEmitted) { - val validationError = validateSteering?.invoke(content) + val validationError = validateSteering?.invoke(input) if (validationError == null) { - emitSteeringNote(sessionId, content, effectiveStageId) + emitSteeringNote(sessionId, input, effectiveStageId) } } diff --git a/testing/deterministic/src/test/kotlin/DefaultContextPackBuilderTest.kt b/testing/deterministic/src/test/kotlin/DefaultContextPackBuilderTest.kt index de3d4449..06c654ba 100644 --- a/testing/deterministic/src/test/kotlin/DefaultContextPackBuilderTest.kt +++ b/testing/deterministic/src/test/kotlin/DefaultContextPackBuilderTest.kt @@ -5,8 +5,10 @@ import com.correx.core.context.builder.RequiredContextOverflowException import com.correx.core.context.model.ContextBucket import com.correx.core.context.model.ContextEntry import com.correx.core.context.model.ContextLayer +import com.correx.core.context.model.EntryRole import com.correx.core.context.model.TokenBudget import com.correx.core.events.types.ContextEntryId +import com.correx.core.inference.PromptRenderer import com.correx.core.events.types.ContextPackId import com.correx.core.events.types.SessionId import com.correx.core.events.types.StageId @@ -42,6 +44,55 @@ class DefaultContextPackBuilderTest { tokenEstimate = tokens ) + @Test + fun `an orchestrator correction reaches the rendered prompt as the last user turn`() = + kotlinx.coroutines.runBlocking { + // The nudge used to be built as a toolResult with a fresh sourceId, so it had no + // matching assistantToolCall and reconcileToolPairs() deleted it: the orchestrator + // "corrected" the model and the correction never reached the provider. Assert on the + // RENDERED prompt, not the entry list — that is where the defect was invisible. + val entries = listOf( + typedEntry("sys", ContextLayer.L0, 100, "systemPrompt"), + typedEntry("task", ContextLayer.L1, 50, "agentPrompt"), + ContextEntry( + id = ContextEntryId("call1"), + layer = ContextLayer.L2, + content = "{\"tool\":\"file_read\"}", + sourceType = "assistantToolCall", + sourceId = "inv-1", + tokenEstimate = 40, + role = EntryRole.ASSISTANT, + ), + ContextEntry( + id = ContextEntryId("res1"), + layer = ContextLayer.L2, + content = "file contents", + sourceType = "toolResult", + sourceId = "inv-1", + tokenEstimate = 40, + role = EntryRole.TOOL, + ), + ContextEntry( + id = ContextEntryId("nudge"), + layer = ContextLayer.L2, + content = "STOP reading. You MUST now call file_write.", + sourceType = "orchestratorCorrection", + sourceId = "corr-1", + tokenEstimate = 20, + role = EntryRole.USER, + bucket = ContextBucket.REQUIRED, + ), + ) + val pack = builder.build(packId, sessionId, stageId, entries, TokenBudget(limit = 4096)) + val messages = PromptRenderer.render(pack) + val last = messages.last() + assertEquals("user", last.role) + assertTrue( + last.content.contains("STOP reading"), + "correction must survive to the prompt; got: " + messages.map { it.role to it.content }, + ) + } + @Test fun `oversized tool results from a stage tool loop are trimmed to budget`() = kotlinx.coroutines.runBlocking { // Live repro (2026-06-11): analyst stage with three file_read results of ~5.6k/11.9k/10.7k diff --git a/testing/deterministic/src/test/kotlin/TalkieFacadeTest.kt b/testing/deterministic/src/test/kotlin/TalkieFacadeTest.kt index 8b71a8a4..77ccdfe7 100644 --- a/testing/deterministic/src/test/kotlin/TalkieFacadeTest.kt +++ b/testing/deterministic/src/test/kotlin/TalkieFacadeTest.kt @@ -129,7 +129,8 @@ class TalkieFacadeTest { assertEquals(com.correx.core.events.events.ChatTurnRole.USER, chatEvents[0].role) assertEquals("inference response", chatEvents[1].content) assertEquals(com.correx.core.events.events.ChatTurnRole.ROUTER, chatEvents[1].role) - assertEquals("inference response", steeringEvents[0].content) + // The steering note is the operator's raw text, never the router's paraphrase of it. + assertEquals("steer this way", steeringEvents[0].content) } @Test @@ -158,7 +159,7 @@ class TalkieFacadeTest { val payloads = mockStore.appendedEvents.map { it.payload } val steeringEvents = payloads.filterIsInstance() assertEquals(1, steeringEvents.size) - assertEquals("steering response", steeringEvents[0].content) + assertEquals("Hello!", steeringEvents[0].content) } // -------------------------------------------------------------------------- -- 2.52.0 From 6a8a7b31c1c8f13fd605a1dc82ebfb682909fb91 Mon Sep 17 00:00:00 2001 From: claude Date: Thu, 27 Aug 2026 11:57:49 +0400 Subject: [PATCH 4/6] fix(events,tools,toolintent): failure attribution, one path normalization rule, file_copy (#713) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three generic harness fixes from the web-ui postmortem dataset. Nothing here keys on a language, framework, build tool or task type. 1. Failure attribution. WorkflowFailedEvent carries one primary FailureAttribution (AGENT | HARNESS | WORKFLOW | ENVIRONMENT | PROVIDER | OPERATOR | UNKNOWN), defaulted to UNKNOWN so pre-field events replay unchanged. FailureAttributor is the deterministic reason->layer mapping, used both at emission and when classifying history, so the baseline and the live metric are one measurement. Emission sites set it: failWorkflow derives from the reason unless the caller knows the layer, cancellation is OPERATOR, the server catch-all falls back to HARNESS, a grounding-rejected plan is AGENT. Multi-cause chains stay on FailureTicketOpened — no second causal structure. GET /metrics/failure-attribution (FailureAttributionInspectionService, mirroring ToolReliabilityInspectionService) reports counts, share, UNKNOWN share, the preserved reasons and the ticket categories from the same sessions. Read-only: historical events are classified at READ time and reported as `inferred`, never written back over an append-only log. Baseline over the local log, 122 terminal failures: AGENT 51 (41.8%), OPERATOR 28 (23.0%), WORKFLOW 19 (15.6%), PROVIDER 15 (12.3%), HARNESS 6 (4.9%), ENVIRONMENT 3 (2.5%), UNKNOWN 0. 2. The `~` guard bug. ToolPath is now the ONE canonical normalization rule (expand a leading `~`/`~/`, keep absolutes, anchor relatives on the session working dir). Every filesystem tool, all six plane-2 path rules and the approval preview resolve through it, so policy and existence checks inspect the path the tool will operate on. `~/.gradle/init.d/offline.gradle` used to resolve to `/~/.gradle/...`: reported non-existent AND in-workspace, so the reference gate called a real file a hallucination and the out-of-workspace prompt never fired. Containment and external-read approval behaviour are unchanged — the expanded path is simply outside the workspace, where it always belonged. 3. file_copy (#713). A first-class tool with the writer's jail, tier, receipt, replay and CAS pre/post images; static and binary assets no longer move through the model's token stream. Needed one generic split: ParamRole.SOURCE_PATH marks a path a call reads FROM, so containment gates judge both params while write-target gates (read-before-write, stale-write, write scope, write manifest) judge the mutated one. ReadBeforeWriteRule exempts any call declaring a SOURCE_PATH: its content comes from disk, not from memory, and requiring a read of a binary is unsatisfiable. Existing tools declare no SOURCE_PATH, so their behaviour is byte-identical. Tests: ToolPathTest (9), FailureAttributionTest (10), PathNormalizationRuleTest (6), FileCopyToolTest (10), plus a home-relative FileReadTool read. ./gradlew check green. Co-Authored-By: Claude Opus 5 --- apps/server/AGENTS.md | 1 + .../com/correx/apps/server/Application.kt | 9 + .../com/correx/apps/server/ServerModule.kt | 6 + .../apps/server/freestyle/FreestyleDriver.kt | 3 + .../FailureAttributionInspectionService.kt | 140 ++++++++++++ core/events/AGENTS.md | 4 +- .../core/events/events/FailureAttribution.kt | 142 ++++++++++++ .../core/events/events/OrchestrationEvents.kt | 6 + .../events/events/FailureAttributionTest.kt | 140 ++++++++++++ .../SessionOrchestratorPreview.kt | 4 +- .../SessionOrchestratorWorkflow.kt | 16 +- .../SessionOrchestratorWorkspace.kt | 12 +- core/toolintent/AGENTS.md | 4 +- .../rules/ManifestContainmentRule.kt | 7 +- .../toolintent/rules/ParamValueExtractor.kt | 30 ++- .../toolintent/rules/PathContainmentRule.kt | 6 +- .../toolintent/rules/ReadBeforeWriteRule.kt | 15 +- .../toolintent/rules/ReferenceExistsRule.kt | 5 +- .../core/toolintent/rules/StaleWriteRule.kt | 8 +- .../core/toolintent/rules/WriteScopeRule.kt | 9 +- .../toolintent/PathNormalizationRuleTest.kt | 149 +++++++++++++ core/tools/AGENTS.md | 3 +- .../correx/core/tools/contract/ParamRole.kt | 11 +- .../correx/core/tools/contract/ToolPath.kt | 49 +++++ .../core/tools/contract/ToolPathTest.kt | 71 ++++++ infrastructure/tools/AGENTS.md | 2 + .../tools/filesystem/FileCopyTool.kt | 205 ++++++++++++++++++ .../tools/filesystem/FileDeleteTool.kt | 12 +- .../tools/filesystem/FileEditTool.kt | 11 +- .../tools/filesystem/FileReadTool.kt | 11 +- .../tools/filesystem/FileWriteTool.kt | 12 +- .../tools/filesystem/ListDirTool.kt | 11 +- .../tools/filesystem/WorkspaceSearchTools.kt | 8 +- .../tools/filesystem/FileCopyToolTest.kt | 162 ++++++++++++++ .../tools/filesystem/FileReadToolTest.kt | 18 ++ .../correx/infrastructure/tools/ToolConfig.kt | 9 + 36 files changed, 1232 insertions(+), 79 deletions(-) create mode 100644 apps/server/src/main/kotlin/com/correx/apps/server/metrics/FailureAttributionInspectionService.kt create mode 100644 core/events/src/main/kotlin/com/correx/core/events/events/FailureAttribution.kt create mode 100644 core/events/src/test/kotlin/com/correx/core/events/events/FailureAttributionTest.kt create mode 100644 core/toolintent/src/test/kotlin/com/correx/core/toolintent/PathNormalizationRuleTest.kt create mode 100644 core/tools/src/main/kotlin/com/correx/core/tools/contract/ToolPath.kt create mode 100644 core/tools/src/test/kotlin/com/correx/core/tools/contract/ToolPathTest.kt create mode 100644 infrastructure/tools/filesystem/src/main/kotlin/com/correx/infrastructure/tools/filesystem/FileCopyTool.kt create mode 100644 infrastructure/tools/filesystem/src/test/kotlin/com/correx/infrastructure/tools/filesystem/FileCopyToolTest.kt diff --git a/apps/server/AGENTS.md b/apps/server/AGENTS.md index bffc26b8..6b2d6eb0 100644 --- a/apps/server/AGENTS.md +++ b/apps/server/AGENTS.md @@ -19,6 +19,7 @@ All sources under `apps/server/src/`. - `GET /health` — health report (probes: event-store, llama-server, disk watermark) - `GET /stats` — metrics report (MetricsProjection) - `GET /metrics/tool-reliability` — per-model tool-call validity across the event log (`ToolReliabilityInspectionService`); groundwork for capability-aware routing +- `GET /metrics/failure-attribution` — terminal-failure attribution across the event log (`FailureAttributionInspectionService`): count and share per `FailureAttribution` layer, the UNKNOWN share, the preserved reasons behind each row, and the `FailureTicketOpened` categories from the same sessions. Read-only: events recorded before `WorkflowFailedEvent.attribution` existed are classified at read time by `FailureAttributor` and reported as `inferred`, never written back. - 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 default tool jail. Every session records its own resolved workspace binding; repo maps, project memory, profile/instruction snapshots, and git run branches use that binding and skip unbound sessions. `[project]` never supplies a workspace root. diff --git a/apps/server/src/main/kotlin/com/correx/apps/server/Application.kt b/apps/server/src/main/kotlin/com/correx/apps/server/Application.kt index 4355f037..6344ddb3 100644 --- a/apps/server/src/main/kotlin/com/correx/apps/server/Application.kt +++ b/apps/server/src/main/kotlin/com/correx/apps/server/Application.kt @@ -1,6 +1,7 @@ package com.correx.apps.server import com.correx.apps.server.health.HealthInspectionService +import com.correx.apps.server.metrics.FailureAttributionInspectionService import com.correx.apps.server.metrics.ToolReliabilityInspectionService import com.correx.apps.server.routes.providerRoutes import com.correx.apps.server.routes.sessionRoutes @@ -40,6 +41,7 @@ fun Application.configureServer(module: ServerModule) { val globalStreamHandler = GlobalStreamHandler(module) val healthInspection = HealthInspectionService(module.eventStore) val toolReliability = ToolReliabilityInspectionService(module.eventStore) + val failureAttribution = FailureAttributionInspectionService(module.eventStore) routing { get("/health") { @@ -59,6 +61,13 @@ fun Application.configureServer(module: ServerModule) { call.respond(toolReliability.inspect()) } + // Terminal-failure attribution across the whole event log: which layer each WorkflowFailed + // belongs to, and the UNKNOWN share. Read-only; events recorded before the attribution field + // are classified at read time and reported as `inferred`, never written back. + get("/metrics/failure-attribution") { + call.respond(failureAttribution.inspect()) + } + webSocket("/stream") { globalStreamHandler.handle(this) } 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 31f37a49..f7f72fcf 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 @@ -8,6 +8,8 @@ import com.correx.apps.server.registry.ProviderRegistry import com.correx.apps.server.registry.WorkflowRegistry import com.correx.apps.server.workspace.WorkspaceResolver import com.correx.apps.server.workspace.WorkspaceResolution +import com.correx.core.events.events.FailureAttribution +import com.correx.core.events.events.FailureAttributor import com.correx.core.events.events.SessionWorkspaceBoundEvent import com.correx.core.kernel.orchestration.WorkspaceContext import com.correx.core.approvals.ApprovalProjector @@ -478,6 +480,10 @@ class ServerModule( stageId = failingStageId, reason = reason, retryExhausted = retryExhausted, + // This is the catch-all for a throwable that escaped the orchestrator, so the + // default layer is correx itself; the reason text still wins when it names an + // outer layer (provider timeout, missing program, operator cancellation). + attribution = FailureAttributor.classify(reason, fallback = FailureAttribution.HARNESS), ), ), ) 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 bbb904e5..a2d3a133 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 @@ -7,6 +7,7 @@ import com.correx.core.events.events.CapabilityGapVerdict import com.correx.core.events.events.EventMetadata import com.correx.core.events.events.ExecutionPlanLockedEvent import com.correx.core.events.events.ExecutionPlanRejectedEvent +import com.correx.core.events.events.FailureAttribution import com.correx.core.events.events.NewEvent import com.correx.core.events.events.PlanGroundingEvaluatedEvent import com.correx.core.events.events.PlanGroundingVerdict @@ -371,6 +372,8 @@ class FreestyleDriver( stageId = StageId("architect"), reason = "execution plan rejected ($source): $reason", retryExhausted = false, + // The rejected plan is the model's own output; the harness evaluated it correctly. + attribution = FailureAttribution.AGENT, ), ), ) diff --git a/apps/server/src/main/kotlin/com/correx/apps/server/metrics/FailureAttributionInspectionService.kt b/apps/server/src/main/kotlin/com/correx/apps/server/metrics/FailureAttributionInspectionService.kt new file mode 100644 index 00000000..1e18782a --- /dev/null +++ b/apps/server/src/main/kotlin/com/correx/apps/server/metrics/FailureAttributionInspectionService.kt @@ -0,0 +1,140 @@ +package com.correx.apps.server.metrics + +import com.correx.core.events.events.FailureAttribution +import com.correx.core.events.events.FailureAttributor +import com.correx.core.events.events.FailureTicketOpenedEvent +import com.correx.core.events.events.WorkflowFailedEvent +import com.correx.core.events.stores.EventStore +import kotlinx.serialization.Serializable + +private const val PERCENT = 100.0 +private const val REASON_BUCKET_MAX = 110 +private const val TOP_REASONS = 10 + +@Serializable +data class AttributionRow( + val attribution: String, + val count: Long, + val sharePct: Double, + /** Failures whose event carried this attribution when it was recorded. */ + val recorded: Long, + /** Failures classified from the preserved reason at read time (the field was absent/UNKNOWN). */ + val inferred: Long, + /** The preserved [WorkflowFailedEvent.reason] texts behind this row, most frequent first. */ + val topReasons: List, + /** Categories of the [FailureTicketOpenedEvent]s in the same sessions — the causal chain when + * more than one layer contributed. Empty when no tickets were opened. */ + val contributingTicketCategories: List, +) + +@Serializable +data class FailureAttributionReport( + val totalFailures: Long, + val recordedFailures: Long, + val inferredFailures: Long, + val unknownCount: Long, + val unknownPct: Double, + val byAttribution: List, + /** Every reason that no marker matched, so UNKNOWN can never sit unexamined. */ + val unknownReasons: List, +) + +/** + * Failure attribution across the whole event log: how many terminal [WorkflowFailedEvent]s belong to + * each layer, and what share is still UNKNOWN. The denominator you need before changing execution + * behaviour — "correx failed N runs" is not actionable, "N of them were harness defects" is. + * + * Read-only and idempotent by construction: historical events recorded before + * [WorkflowFailedEvent.attribution] existed are classified at READ time by [FailureAttributor], the + * same function live emission uses. Nothing is written back — history is append-only, and a + * re-derived classification is a projection, not a fact (Hard Invariant #2). Each row separates what + * was `recorded` at emission from what this service `inferred`, so a backfilled baseline never + * masquerades as originally-recorded data. Re-running it over the same log yields the same numbers. + */ +class FailureAttributionInspectionService(private val eventStore: EventStore) { + + private class Agg { + var recorded: Long = 0 + var inferred: Long = 0 + val reasons: MutableMap = linkedMapOf() + val sessions: MutableSet = linkedSetOf() + } + + @Suppress("NestedBlockDepth") + fun inspect(): FailureAttributionReport { + val byAttribution = linkedMapOf() + val ticketsBySession = linkedMapOf>() + val unknownReasons = linkedMapOf() + + eventStore.allEvents().forEach { stored -> + when (val payload = stored.payload) { + is FailureTicketOpenedEvent -> { + val categories = ticketsBySession.getOrPut(payload.sessionId.value) { linkedMapOf() } + categories[payload.category] = (categories[payload.category] ?: 0) + 1 + } + + is WorkflowFailedEvent -> { + val wasRecorded = payload.attribution != FailureAttribution.UNKNOWN + val attribution = + if (wasRecorded) payload.attribution else FailureAttributor.classify(payload.reason) + val agg = byAttribution.getOrPut(attribution) { Agg() } + if (wasRecorded) agg.recorded++ else agg.inferred++ + val bucket = payload.reason.lineSequence().firstOrNull().orEmpty().take(REASON_BUCKET_MAX) + agg.reasons[bucket] = (agg.reasons[bucket] ?: 0) + 1 + agg.sessions += payload.sessionId.value + if (attribution == FailureAttribution.UNKNOWN) { + unknownReasons[bucket] = (unknownReasons[bucket] ?: 0) + 1 + } + } + + else -> Unit + } + } + + val total = byAttribution.values.sumOf { it.recorded + it.inferred } + val unknown = byAttribution[FailureAttribution.UNKNOWN]?.let { it.recorded + it.inferred } ?: 0 + val rows = byAttribution.entries + .sortedByDescending { it.value.recorded + it.value.inferred } + .map { (attribution, agg) -> row(attribution, agg, total, ticketsBySession) } + + return FailureAttributionReport( + totalFailures = total, + recordedFailures = byAttribution.values.sumOf { it.recorded }, + inferredFailures = byAttribution.values.sumOf { it.inferred }, + unknownCount = unknown, + unknownPct = share(unknown, total), + byAttribution = rows, + unknownReasons = topOf(unknownReasons), + ) + } + + private fun row( + attribution: FailureAttribution, + agg: Agg, + total: Long, + ticketsBySession: Map>, + ): AttributionRow { + val count = agg.recorded + agg.inferred + val tickets = linkedMapOf() + agg.sessions.forEach { sessionId -> + ticketsBySession[sessionId]?.forEach { (category, n) -> + tickets[category] = (tickets[category] ?: 0) + n + } + } + return AttributionRow( + attribution = attribution.name, + count = count, + sharePct = share(count, total), + recorded = agg.recorded, + inferred = agg.inferred, + topReasons = topOf(agg.reasons), + contributingTicketCategories = topOf(tickets), + ) + } + + private fun topOf(counts: Map): List = + counts.entries.sortedByDescending { it.value }.take(TOP_REASONS).map { ReasonCount(it.key, it.value) } + + private fun share(part: Long, total: Long): Double = + if (total == 0L) 0.0 else part.toDouble() / total * PERCENT +} diff --git a/core/events/AGENTS.md b/core/events/AGENTS.md index 5ac3843c..e4217d66 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. +- `FailureAttribution` / `FailureAttributor` — the terminal-failure taxonomy (`AGENT`, `HARNESS`, `WORKFLOW`, `ENVIRONMENT`, `PROVIDER`, `OPERATOR`, `UNKNOWN`) carried as `WorkflowFailedEvent.attribution`, plus the deterministic reason→layer mapping used both at emission and when classifying historical events. One primary attribution per terminal event; a multi-cause chain is the session's `FailureTicketOpenedEvent`s, not a second structure. - `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`. @@ -30,7 +31,8 @@ CORREX kernel team. This is the most cross-cutting module in the codebase — ch - Event classes are `@Serializable data class` with no mutable state. No methods beyond data accessors. - `RunBranchPushedEvent` records an optional server Git transport push only after it succeeds; its branch/base/head SHAs are observations, not values replay recalculates. - `RepoMapEntry.descriptor` is a bounded source-purpose observation recorded with the repo map and used when constructing semantic L3 embeddings. -- Do not add domain logic to events. They are records, not actors. +- Do not add domain logic to events. They are records, not actors. `FailureAttributor` is the one exception by design: a pure reason→layer function that must be identical for live emission and for historical classification, so it lives beside the enum it returns. +- `WorkflowFailedEvent.attribution` defaults to `UNKNOWN` so pre-field events replay unchanged. Classify those at READ time (see `FailureAttributionInspectionService`); never rewrite history to backfill them. - `EgressAllowlistProjection` — special projection kept in this module because it is used by both `core:toolintent` and `core:events` consumers; it is a shared cross-cutting projection. ## Verification diff --git a/core/events/src/main/kotlin/com/correx/core/events/events/FailureAttribution.kt b/core/events/src/main/kotlin/com/correx/core/events/events/FailureAttribution.kt new file mode 100644 index 00000000..c520e987 --- /dev/null +++ b/core/events/src/main/kotlin/com/correx/core/events/events/FailureAttribution.kt @@ -0,0 +1,142 @@ +package com.correx.core.events.events + +import kotlinx.serialization.Serializable + +/** + * WHOSE failure a terminal [WorkflowFailedEvent] was: the primary layer that has to change for the + * run to succeed. One value per terminal event. When several causes contributed, the causal chain is + * the session's [FailureTicketOpenedEvent]s — this enum does not model chains. + * + * The point is measurement: "correx failed 99 runs" is not actionable, "61 of them were harness + * defects" is. Read the layer, not the symptom. + */ +@Serializable +enum class FailureAttribution { + /** The model produced invalid work while the harness operated correctly. */ + AGENT, + + /** Correx's own runtime: a linkage error, a bug in a reducer/tool layer, a false observation + * handed to the agent, or an expectation correx could not evaluate for lack of instrumentation. */ + HARNESS, + + /** The workflow/graph definition: no transition matched, a condition referenced a field that + * cannot exist, a declared prompt or stage was never authored. */ + WORKFLOW, + + /** The machine the run executes on: a missing executable, permissions, disk, ports. */ + ENVIRONMENT, + + /** The inference provider: unavailable, timed out, or answered with a body correx cannot read. */ + PROVIDER, + + /** A human ended the run: cancellation, or a denied/rejected approval. */ + OPERATOR, + + /** Not classifiable from the recorded reason. A metric, not a bucket: a rising UNKNOWN share + * means the taxonomy or the reason text needs work, and every UNKNOWN is a defect to triage. */ + UNKNOWN, +} + +/** + * Deterministic mapping from a terminal failure reason to its [FailureAttribution]. + * + * Same function for live emission and for classifying historical events recorded before the field + * existed, so a backfilled baseline and a live metric are the same measurement. It is a pure + * function of the reason string: no clock, no I/O, no session lookup — safe to re-run over the whole + * event log any number of times. + * + * Markers are matched in layer order — OPERATOR, PROVIDER, ENVIRONMENT, WORKFLOW, HARNESS, AGENT — + * because the outermost cause wins: a provider timeout that surfaces as an artifact-validation + * failure is still a provider failure. Match on what the layer says about ITSELF (a provider being + * unavailable, a program that cannot be run), never on the domain of the run: nothing here may key + * on a language, framework, build tool or task type. + */ +object FailureAttributor { + + /** + * Classifies [reason]. [fallback] is returned when no marker matches — a call site that knows + * the layer from its position (e.g. a top-level catch-all in the correx runtime) supplies its + * own instead of leaving the failure [FailureAttribution.UNKNOWN]. + */ + fun classify(reason: String, fallback: FailureAttribution = FailureAttribution.UNKNOWN): FailureAttribution { + val text = reason.lowercase() + // A reason that is a bare JVM binary name with no prose is a linkage/classload error + // (NoClassDefFoundError.getMessage()), i.e. a correx runtime defect. + if (text.isNotBlank() && !text.contains(' ') && text.contains('/') && !text.contains('.')) { + return FailureAttribution.HARNESS + } + return MARKERS.firstOrNull { (_, markers) -> markers.any { it in text } }?.first ?: fallback + } + + private val MARKERS: List>> = listOf( + FailureAttribution.OPERATOR to listOf( + "cancelled", + "canceled", + "approval denied", + "approval rejected", + "rejected by operator", + ), + FailureAttribution.PROVIDER to listOf( + "is unavailable", + "health check failed", + "connection refused", + "request timeout has expired", + "no provider satisfies", + "returned 400", + "returned 401", + "returned 403", + "returned 404", + "returned 5", + "chatcompletionresponse", + "no completion returned", + "context window exceeded", + ), + FailureAttribution.ENVIRONMENT to listOf( + "cannot run program", + "exec failed", + "command not found", + "permission denied", + "no space left", + "address already in use", + ), + FailureAttribution.WORKFLOW to listOf( + "no transition condition matched", + "no matching transition", + "condition evaluation failed", + // A stage's declaration disagrees with reality: the prerequisite it names is unresolved + // or sits outside the scope it declared. Both are authoring defects in the definition. + "build prerequisite", + "declared prompt", + "unknown stage", + "no such stage", + ), + FailureAttribution.HARNESS to listOf( + "noclassdeffounderror", + "nosuchmethod", + "classnotfound", + "not supported in map", + "hex string must have even length", + "could not be evaluated", + "no instrumentation", + "unexpected orchestrator failure", + ), + FailureAttribution.AGENT to listOf( + "did not produce declared artifacts", + "did not satisfy its file contract", + "did not pass", + "failed semantic review", + "declared no artifacts", + "review loop exhausted", + // A call plane-2 denied: the harness evaluated policy correctly, the agent proposed it. + "blocked by tool-call policy", + "validation failed", + "artifact repair failed", + "repair ladder exhausted", + "recovery route budget exhausted", + "refinement loop", + "execution plan rejected", + "is stuck", + "failed to decode", + ), + ) +} diff --git a/core/events/src/main/kotlin/com/correx/core/events/events/OrchestrationEvents.kt b/core/events/src/main/kotlin/com/correx/core/events/events/OrchestrationEvents.kt index 5f403458..ad59b35b 100644 --- a/core/events/src/main/kotlin/com/correx/core/events/events/OrchestrationEvents.kt +++ b/core/events/src/main/kotlin/com/correx/core/events/events/OrchestrationEvents.kt @@ -34,6 +34,12 @@ data class WorkflowFailedEvent( val stageId: StageId, val reason: String, val retryExhausted: Boolean, + // WHOSE failure this was — the layer that must change for the run to succeed (see + // [FailureAttribution]). Set at emission by the site that knows the cause, or derived from + // [reason] by [FailureAttributor]; the original [reason] is always preserved alongside it. + // Defaulted to UNKNOWN so events recorded before this field replay unchanged: a classification + // for those is INFERRED at read time, never written back over history. + val attribution: FailureAttribution = FailureAttribution.UNKNOWN, ) : EventPayload /** diff --git a/core/events/src/test/kotlin/com/correx/core/events/events/FailureAttributionTest.kt b/core/events/src/test/kotlin/com/correx/core/events/events/FailureAttributionTest.kt new file mode 100644 index 00000000..47530ac7 --- /dev/null +++ b/core/events/src/test/kotlin/com/correx/core/events/events/FailureAttributionTest.kt @@ -0,0 +1,140 @@ +package com.correx.core.events.events + +import com.correx.core.events.types.SessionId +import com.correx.core.events.types.StageId +import kotlinx.serialization.json.Json +import kotlin.test.Test +import kotlin.test.assertEquals + +/** + * The taxonomy's contract. Cases are drawn from real `WorkflowFailed.reason` texts in the local + * event log so the mapping is checked against failures that actually happened, and every case keys + * on what a LAYER says about itself — never on a language, framework or build tool. + */ +class FailureAttributionTest { + + private fun assertLayer(expected: FailureAttribution, reason: String) = + assertEquals(expected, FailureAttributor.classify(reason), reason) + + @Test + fun `operator-ended runs`() { + assertLayer(FailureAttribution.OPERATOR, "CANCELLED") + assertLayer(FailureAttribution.OPERATOR, "approval denied") + assertLayer(FailureAttribution.OPERATOR, "approval rejected for stage architect") + } + + @Test + fun `provider failures`() { + assertLayer( + FailureAttribution.PROVIDER, + "Provider 'llama-cpp:default' is unavailable: Health check failed: Connection refused", + ) + assertLayer( + FailureAttribution.PROVIDER, + "Request timeout has expired [url=http://127.0.0.1:10000/v1/chat/completions, " + + "request_timeout=600000 ms]", + ) + assertLayer(FailureAttribution.PROVIDER, "No provider satisfies capabilities [] for stage 'routing'") + // A provider body correx cannot decode is a provider-communication failure, not a bad artifact. + assertLayer( + FailureAttribution.PROVIDER, + "Illegal input: Fields [id, choices, usage] are required for type with serial name " + + "'com.correx.infrastructure.inference.llama.cpp.ChatCompletionResponse', but they were missing", + ) + assertLayer(FailureAttribution.PROVIDER, "llama-server returned 400 Bad Request: {\"error\":{}}") + } + + @Test + fun `environment failures`() { + assertLayer( + FailureAttribution.ENVIRONMENT, + "Cannot run program \"cd\" (in directory \"/w\"): Exec failed, error: 2 (No such file or directory)", + ) + } + + @Test + fun `workflow-definition failures`() { + assertLayer(FailureAttribution.WORKFLOW, "no transition condition matched from stage analyst") + assertLayer( + FailureAttribution.WORKFLOW, + "condition evaluation failed on 'verify_completion->done': Field 'verdict' not found", + ) + assertLayer(FailureAttribution.WORKFLOW, "[SessionOrchestrator] stage=analyst: declared prompt 'x' missing") + assertLayer(FailureAttribution.WORKFLOW, "no matching transition from stage A") + assertLayer(FailureAttribution.WORKFLOW, "build prerequisite 'x' unresolved after bootstrap: missing") + } + + @Test + fun `harness failures`() { + // A bare JVM binary name with no prose is a linkage error inside correx itself. + assertLayer( + FailureAttribution.HARNESS, + "com/correx/core/kernel/orchestration/SessionOrchestrator\$failWorkflow\$1", + ) + assertLayer(FailureAttribution.HARNESS, "com/correx/core/approvals/GrantLedgerKt") + assertLayer(FailureAttribution.HARNESS, "null values are not supported in Map") + // An expectation correx could not evaluate for lack of instrumentation is ours, not the agent's. + assertLayer(FailureAttribution.HARNESS, "expected_result could not be evaluated: no instrumentation") + } + + @Test + fun `agent failures`() { + assertLayer(FailureAttribution.AGENT, "stage implementer did not produce declared artifacts: patch") + assertLayer(FailureAttribution.AGENT, "validation failed") + assertLayer(FailureAttribution.AGENT, "artifact repair failed (FORMATTING): could not extract a JSON object") + assertLayer(FailureAttribution.AGENT, "refinement loop 'implementer->reviewer' exceeded 2 iterations") + assertLayer(FailureAttribution.AGENT, "recovery route budget exhausted for stage ui_review (gate=execution)") + assertLayer(FailureAttribution.AGENT, "repair ladder exhausted for stage x (gate=stage_loop_break)") + assertLayer(FailureAttribution.AGENT, "execution plan rejected (grounding): plan failed grounding") + assertLayer(FailureAttribution.AGENT, "stage x did not satisfy its file contract. Fix these before review:") + assertLayer(FailureAttribution.AGENT, "stage x did not pass its PROJECT build gate") + assertLayer(FailureAttribution.AGENT, "stage x did not pass static analysis. Fix these before review:") + assertLayer(FailureAttribution.AGENT, "stage x failed semantic review — fix these correctness issues:") + assertLayer(FailureAttribution.AGENT, "stage x declared no artifacts and ran no tools") + assertLayer(FailureAttribution.AGENT, "review loop exhausted after exactly 3 cycles.") + assertLayer(FailureAttribution.AGENT, "blocked by tool-call policy") + } + + @Test + fun `an unmatched reason is UNKNOWN, and a call site may supply its own fallback`() { + assertLayer(FailureAttribution.UNKNOWN, "something nobody has seen before") + assertEquals( + FailureAttribution.HARNESS, + FailureAttributor.classify("something nobody has seen before", FailureAttribution.HARNESS), + ) + // The reason text still wins over a call site's fallback when it names an outer layer. + assertEquals( + FailureAttribution.OPERATOR, + FailureAttributor.classify("CANCELLED", FailureAttribution.HARNESS), + ) + } + + @Test + fun `classification is a pure function of the reason`() { + val reason = "no transition condition matched from stage analyst" + assertEquals(FailureAttributor.classify(reason), FailureAttributor.classify(reason)) + } + + @Test + fun `an event recorded before the field replays as UNKNOWN with its reason preserved`() { + val stored = """{"sessionId":"s","stageId":"st","reason":"CANCELLED","retryExhausted":false}""" + val event = Json.decodeFromString(stored) + assertEquals(FailureAttribution.UNKNOWN, event.attribution) + assertEquals("CANCELLED", event.reason) + // …and the historical baseline classifies it at read time, without rewriting history. + assertEquals(FailureAttribution.OPERATOR, FailureAttributor.classify(event.reason)) + } + + @Test + fun `a live event carries its attribution through a round-trip`() { + val event = WorkflowFailedEvent( + sessionId = SessionId("s"), + stageId = StageId("st"), + reason = "no transition condition matched from stage analyst", + retryExhausted = true, + attribution = FailureAttribution.WORKFLOW, + ) + val json = Json.encodeToString(WorkflowFailedEvent.serializer(), event) + assertEquals(event, Json.decodeFromString(WorkflowFailedEvent.serializer(), json)) + } +} diff --git a/core/kernel/src/main/kotlin/com/correx/core/kernel/orchestration/SessionOrchestratorPreview.kt b/core/kernel/src/main/kotlin/com/correx/core/kernel/orchestration/SessionOrchestratorPreview.kt index 2fc4c07d..62d43022 100644 --- a/core/kernel/src/main/kotlin/com/correx/core/kernel/orchestration/SessionOrchestratorPreview.kt +++ b/core/kernel/src/main/kotlin/com/correx/core/kernel/orchestration/SessionOrchestratorPreview.kt @@ -1,4 +1,5 @@ package com.correx.core.kernel.orchestration +import com.correx.core.tools.contract.ToolPath import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.withContext import kotlinx.serialization.json.Json @@ -46,8 +47,7 @@ internal suspend fun readFileIfExists(path: String, workspaceRoot: java.nio.file // Resolve relative paths against the session's workspace root, same as the tools do — // resolving against the daemon CWD showed the operator the wrong file (or nothing) when // server CWD ≠ workspace_root. - val raw = java.nio.file.Paths.get(path) - val filePath = if (raw.isAbsolute || workspaceRoot == null) raw else workspaceRoot.resolve(raw) + val filePath = ToolPath.resolve(path, workspaceRoot) if (java.nio.file.Files.exists(filePath)) { java.nio.file.Files.readString(filePath) } else null diff --git a/core/kernel/src/main/kotlin/com/correx/core/kernel/orchestration/SessionOrchestratorWorkflow.kt b/core/kernel/src/main/kotlin/com/correx/core/kernel/orchestration/SessionOrchestratorWorkflow.kt index d083a4a6..810ffd61 100644 --- a/core/kernel/src/main/kotlin/com/correx/core/kernel/orchestration/SessionOrchestratorWorkflow.kt +++ b/core/kernel/src/main/kotlin/com/correx/core/kernel/orchestration/SessionOrchestratorWorkflow.kt @@ -5,6 +5,8 @@ import com.correx.core.events.events.CritiqueFindingsRecordedEvent import com.correx.core.events.events.CritiqueOutcomeCorrelatedEvent import com.correx.core.events.events.EventMetadata import com.correx.core.events.events.EventPayload +import com.correx.core.events.events.FailureAttribution +import com.correx.core.events.events.FailureAttributor import com.correx.core.events.events.NewEvent import com.correx.core.events.events.TransitionExecutedEvent import com.correx.core.events.events.WorkflowCompletedEvent @@ -108,6 +110,9 @@ internal suspend fun SessionOrchestrator.failWorkflow( stageId: StageId, reason: String, retryExhausted: Boolean, + // Null ⇒ derive the attribution from [reason] (FailureAttributor). A caller that knows the layer + // from its own position passes it explicitly instead of relying on the reason text. + attribution: FailureAttribution? = null, ): WorkflowResult.Failed { log.warn( "[Orchestrator] FAILED session={} stage={} reason={} retryExhausted={}", @@ -134,7 +139,16 @@ internal suspend fun SessionOrchestrator.failWorkflow( // (e.g. a serialization edge), we cannot record the failure at all — log it loudly with the // full throwable so it is never silent, then still return a clean Failed result. runCatching { - emit(sessionId, WorkflowFailedEvent(sessionId, stageId, reason, retryExhausted)) + emit( + sessionId, + WorkflowFailedEvent( + sessionId, + stageId, + reason, + retryExhausted, + attribution ?: FailureAttributor.classify(reason), + ), + ) }.onFailure { e -> log.error( "[Orchestrator] failWorkflow: FAILED to record terminal WorkflowFailedEvent — event " + diff --git a/core/kernel/src/main/kotlin/com/correx/core/kernel/orchestration/SessionOrchestratorWorkspace.kt b/core/kernel/src/main/kotlin/com/correx/core/kernel/orchestration/SessionOrchestratorWorkspace.kt index 9a1a6eef..ee47c922 100644 --- a/core/kernel/src/main/kotlin/com/correx/core/kernel/orchestration/SessionOrchestratorWorkspace.kt +++ b/core/kernel/src/main/kotlin/com/correx/core/kernel/orchestration/SessionOrchestratorWorkspace.kt @@ -1,5 +1,6 @@ package com.correx.core.kernel.orchestration +import com.correx.core.events.events.FailureAttribution import com.correx.core.events.events.ToolCallAssessedEvent import com.correx.core.events.events.RepoMapComputedEvent import com.correx.core.events.events.OutsidePathAccessGrantedEvent @@ -212,7 +213,16 @@ internal suspend fun SessionOrchestrator.handleCancellation( stageId: StageId, ): WorkflowResult.Cancelled { log.warn("[Orchestrator] CANCELLED session={} stage={}", sessionId.value, stageId.value) - emit(sessionId, WorkflowFailedEvent(sessionId, stageId, "CANCELLED", retryExhausted = false)) + emit( + sessionId, + WorkflowFailedEvent( + sessionId, + stageId, + "CANCELLED", + retryExhausted = false, + attribution = FailureAttribution.OPERATOR, + ), + ) cancellations.remove(sessionId) return WorkflowResult.Cancelled(sessionId) } diff --git a/core/toolintent/AGENTS.md b/core/toolintent/AGENTS.md index ca72f1c7..b759643d 100644 --- a/core/toolintent/AGENTS.md +++ b/core/toolintent/AGENTS.md @@ -24,7 +24,7 @@ CORREX kernel team. This module enforces Hard Invariant #9 for the tool-call pat - `WorkspacePolicy` — aggregates rules and configuration for a workspace. - `WorldProbe` — performs environment checks (filesystem, network) and records the observations as events immediately (Hard Invariant #9). Never call `WorldProbe` during replay. - `EgressAllowlist` — current egress allowlist; rebuilt from `EgressAllowlistProjection` (in `core:events`). -- `ParamValueExtractor` — extracts typed parameter values from tool call arguments. +- `ParamValueExtractor` — extracts typed parameter values from tool call arguments. `candidatePathStrings` = every path-like argument (`ParamRole.PATH` + `ParamRole.SOURCE_PATH`), used by the containment/existence gates; `writeTargetPathStrings` = only the paths a call MUTATES (`ParamRole.PATH`), used by the write-target gates. A tool declaring none of those roles falls back to sniffing path-like strings, so `shell` is unaffected. - `RiskMapping` — maps rule violations to risk levels for `core:risk`. - `SessionContext` — session-scoped context passed to rules during evaluation. @@ -33,6 +33,8 @@ CORREX kernel team. This module enforces Hard Invariant #9 for the tool-call pat - Hard Invariant #9: all `WorldProbe` calls record observations as events. Replay reads those recorded events — it must not call `WorldProbe` again. - Hard Invariant #5: every tool call must be assessed before execution. Assessment result is recorded as `ToolCallAssessmentEvents` in `core:events`. - New rules implement `ToolCallRule` and are registered in `WorkspacePolicy`. Do not add rule logic directly to `ToolCallAssessor`. +- Resolve every model-supplied path through `ToolPath.resolve` (`core:tools`) — the one canonical normalization rule, shared with the filesystem tools. A rule that resolves paths itself will judge a different path than the tool operates on (the `~` bug: `~/x` resolved to `/~/x`, so a real home-directory file was reported as a non-existent in-workspace file and the out-of-workspace prompt never fired). +- A call that declares a `SOURCE_PATH` takes its content from disk, not from the model, so `ReadBeforeWriteRule` exempts it: requiring a read of a copied file's bytes is unsatisfiable for binaries and defeats the purpose of the tool. ## Verification diff --git a/core/toolintent/src/main/kotlin/com/correx/core/toolintent/rules/ManifestContainmentRule.kt b/core/toolintent/src/main/kotlin/com/correx/core/toolintent/rules/ManifestContainmentRule.kt index 57ef6242..d5bdf1f9 100644 --- a/core/toolintent/src/main/kotlin/com/correx/core/toolintent/rules/ManifestContainmentRule.kt +++ b/core/toolintent/src/main/kotlin/com/correx/core/toolintent/rules/ManifestContainmentRule.kt @@ -7,6 +7,7 @@ import com.correx.core.toolintent.ToolCallAssessmentInput import com.correx.core.toolintent.ToolCallRule import com.correx.core.toolintent.maxAction import com.correx.core.tools.contract.ToolCapability +import com.correx.core.tools.contract.ToolPath import com.correx.core.validation.model.ValidationIssue import com.correx.core.validation.model.ValidationSeverity import java.nio.file.FileSystems @@ -52,10 +53,8 @@ class ManifestContainmentRule : ToolCallRule { val observations = mutableListOf() var disposition = RiskAction.PROCEED - for (raw in candidatePathStrings(input.paramRoles, input.request.parameters)) { - val candidate = Path.of(raw) - val resolvedInput = - if (candidate.isAbsolute) candidate else input.workspace.workspaceRoot.resolve(candidate) + for (raw in writeTargetPathStrings(input.paramRoles, input.request.parameters)) { + val resolvedInput = ToolPath.resolve(raw, input.workspace.workspaceRoot) val resolvedReal = input.probe.resolveReal(resolvedInput) val inWorkspace = resolvedReal.startsWith(workspaceReal) val relative = if (inWorkspace) workspaceReal.relativize(resolvedReal).toString() else raw diff --git a/core/toolintent/src/main/kotlin/com/correx/core/toolintent/rules/ParamValueExtractor.kt b/core/toolintent/src/main/kotlin/com/correx/core/toolintent/rules/ParamValueExtractor.kt index c8867618..08d0978d 100644 --- a/core/toolintent/src/main/kotlin/com/correx/core/toolintent/rules/ParamValueExtractor.kt +++ b/core/toolintent/src/main/kotlin/com/correx/core/toolintent/rules/ParamValueExtractor.kt @@ -14,13 +14,31 @@ internal fun extractParamStrings(value: Any?): List = when (value) { } /** - * The path-like argument strings of a tool call: the values of params declared - * [ParamRole.PATH], or — when none are declared — any string value that looks like a - * path. Shared by the path-containment and write-manifest rules so both judge exactly - * the same set of targets. + * Every path-like argument of a tool call — the call's own targets ([ParamRole.PATH]) and any path + * it merely reads from ([ParamRole.SOURCE_PATH]). Used by the containment/existence gates, which + * must judge both: a source outside the workspace is as much an escape as a destination outside it. */ -internal fun candidatePathStrings(paramRoles: Map, parameters: Map): List { - val declared = paramRoles.filterValues { it == ParamRole.PATH }.keys +internal fun candidatePathStrings(paramRoles: Map, parameters: Map): List = + pathStringsForRoles(paramRoles, parameters, setOf(ParamRole.PATH, ParamRole.SOURCE_PATH)) + +/** + * Only the paths a tool call MUTATES ([ParamRole.PATH]). Used by the write-target gates + * (read-before-write, stale-write, write scope, write manifest): blocking a copy because its + * SOURCE was never read, or charging a source against the task's write scope, would be wrong. + */ +internal fun writeTargetPathStrings(paramRoles: Map, parameters: Map): List = + pathStringsForRoles(paramRoles, parameters, setOf(ParamRole.PATH)) + +/** + * Values of the params declared with one of [roles] or — when the tool declares none of them (e.g. + * `shell`, whose param is an [ParamRole.EXEC_COMMAND]) — any string value that looks like a path. + */ +private fun pathStringsForRoles( + paramRoles: Map, + parameters: Map, + roles: Set, +): List { + val declared = paramRoles.filterValues { it in roles }.keys return if (declared.isNotEmpty()) { declared.flatMap { extractParamStrings(parameters[it]) } } else { diff --git a/core/toolintent/src/main/kotlin/com/correx/core/toolintent/rules/PathContainmentRule.kt b/core/toolintent/src/main/kotlin/com/correx/core/toolintent/rules/PathContainmentRule.kt index 52a35c7a..941cce11 100644 --- a/core/toolintent/src/main/kotlin/com/correx/core/toolintent/rules/PathContainmentRule.kt +++ b/core/toolintent/src/main/kotlin/com/correx/core/toolintent/rules/PathContainmentRule.kt @@ -7,9 +7,9 @@ import com.correx.core.toolintent.ToolCallAssessmentInput import com.correx.core.toolintent.ToolCallRule import com.correx.core.toolintent.maxAction import com.correx.core.tools.contract.ToolCapability +import com.correx.core.tools.contract.ToolPath import com.correx.core.validation.model.ValidationIssue import com.correx.core.validation.model.ValidationSeverity -import java.nio.file.Path /** * Effect-based path containment. Dispatches on FILE_READ / FILE_WRITE. For every @@ -34,9 +34,7 @@ class PathContainmentRule : ToolCallRule { var disposition = RiskAction.PROCEED for (raw in candidatePathStrings(input.paramRoles, input.request.parameters)) { - val candidate = Path.of(raw) - val resolvedInput = - if (candidate.isAbsolute) candidate else input.workspace.workspaceRoot.resolve(candidate) + val resolvedInput = ToolPath.resolve(raw, input.workspace.workspaceRoot) val resolvedReal = input.probe.resolveReal(resolvedInput) val exists = input.probe.exists(resolvedInput) val privileged = privilegedReal.any { resolvedReal.startsWith(it) } diff --git a/core/toolintent/src/main/kotlin/com/correx/core/toolintent/rules/ReadBeforeWriteRule.kt b/core/toolintent/src/main/kotlin/com/correx/core/toolintent/rules/ReadBeforeWriteRule.kt index 53a81ed1..08cc8e7d 100644 --- a/core/toolintent/src/main/kotlin/com/correx/core/toolintent/rules/ReadBeforeWriteRule.kt +++ b/core/toolintent/src/main/kotlin/com/correx/core/toolintent/rules/ReadBeforeWriteRule.kt @@ -6,7 +6,9 @@ import com.correx.core.toolintent.ToolCallAssessment import com.correx.core.toolintent.ToolCallAssessmentInput import com.correx.core.toolintent.ToolCallRule import com.correx.core.toolintent.maxAction +import com.correx.core.tools.contract.ParamRole import com.correx.core.tools.contract.ToolCapability +import com.correx.core.tools.contract.ToolPath import com.correx.core.validation.model.ValidationIssue import com.correx.core.validation.model.ValidationSeverity import java.nio.file.Path @@ -27,6 +29,12 @@ class ReadBeforeWriteRule : ToolCallRule { ToolCapability.FILE_WRITE in capabilities override fun assess(input: ToolCallAssessmentInput): ToolCallAssessment { + // A call that declares a SOURCE_PATH takes its content from a file on disk, not from the + // model's memory, so there is nothing for this gate to protect: demanding a file_read of the + // source (or of the target being replaced by it) would force the very bytes-through-context + // round-trip that such tools exist to avoid — and is unsatisfiable for a binary file. + if (input.paramRoles.containsValue(ParamRole.SOURCE_PATH)) return ToolCallAssessment() + val root = input.workspace.workspaceRoot val readReal = input.session.reads.map { realOf(input, root, it) }.toSet() @@ -34,7 +42,7 @@ class ReadBeforeWriteRule : ToolCallRule { val observations = mutableListOf() var disposition = RiskAction.PROCEED - for (raw in candidatePathStrings(input.paramRoles, input.request.parameters)) { + for (raw in writeTargetPathStrings(input.paramRoles, input.request.parameters)) { val resolvedInput = resolveInput(root, raw) val exists = input.probe.exists(resolvedInput) val read = input.probe.resolveReal(resolvedInput) in readReal @@ -58,10 +66,7 @@ class ReadBeforeWriteRule : ToolCallRule { return ToolCallAssessment(issues = issues, observations = observations, disposition = disposition) } - private fun resolveInput(root: Path, raw: String): Path { - val candidate = Path.of(raw) - return if (candidate.isAbsolute) candidate else root.resolve(candidate) - } + private fun resolveInput(root: Path, raw: String): Path = ToolPath.resolve(raw, root) private fun realOf(input: ToolCallAssessmentInput, root: Path, raw: String): Path = input.probe.resolveReal(resolveInput(root, raw)) diff --git a/core/toolintent/src/main/kotlin/com/correx/core/toolintent/rules/ReferenceExistsRule.kt b/core/toolintent/src/main/kotlin/com/correx/core/toolintent/rules/ReferenceExistsRule.kt index 6192a13d..544dba91 100644 --- a/core/toolintent/src/main/kotlin/com/correx/core/toolintent/rules/ReferenceExistsRule.kt +++ b/core/toolintent/src/main/kotlin/com/correx/core/toolintent/rules/ReferenceExistsRule.kt @@ -7,9 +7,9 @@ import com.correx.core.toolintent.ToolCallAssessmentInput import com.correx.core.toolintent.ToolCallRule import com.correx.core.toolintent.maxAction import com.correx.core.tools.contract.ToolCapability +import com.correx.core.tools.contract.ToolPath import com.correx.core.validation.model.ValidationIssue import com.correx.core.validation.model.ValidationSeverity -import java.nio.file.Path /** * Reference-must-exist gate (anti-hallucination). Dispatches on FILE_READ: a read of a path that is @@ -37,8 +37,7 @@ class ReferenceExistsRule : ToolCallRule { var disposition = RiskAction.PROCEED for (raw in candidatePathStrings(input.paramRoles, input.request.parameters)) { - val candidate = Path.of(raw) - val resolvedInput = if (candidate.isAbsolute) candidate else root.resolve(candidate) + val resolvedInput = ToolPath.resolve(raw, root) val exists = input.probe.exists(resolvedInput) val inWorkspace = input.probe.resolveReal(resolvedInput).startsWith(workspaceReal) diff --git a/core/toolintent/src/main/kotlin/com/correx/core/toolintent/rules/StaleWriteRule.kt b/core/toolintent/src/main/kotlin/com/correx/core/toolintent/rules/StaleWriteRule.kt index 880cbd6a..f096b69f 100644 --- a/core/toolintent/src/main/kotlin/com/correx/core/toolintent/rules/StaleWriteRule.kt +++ b/core/toolintent/src/main/kotlin/com/correx/core/toolintent/rules/StaleWriteRule.kt @@ -7,6 +7,7 @@ import com.correx.core.toolintent.ToolCallAssessmentInput import com.correx.core.toolintent.ToolCallRule import com.correx.core.toolintent.maxAction import com.correx.core.tools.contract.ToolCapability +import com.correx.core.tools.contract.ToolPath import com.correx.core.validation.model.ValidationIssue import com.correx.core.validation.model.ValidationSeverity import java.nio.file.Path @@ -35,7 +36,7 @@ class StaleWriteRule : ToolCallRule { val observations = mutableListOf() var disposition = RiskAction.PROCEED - for (raw in candidatePathStrings(input.paramRoles, input.request.parameters)) { + for (raw in writeTargetPathStrings(input.paramRoles, input.request.parameters)) { val resolved = resolveInput(root, raw) val real = input.probe.resolveReal(resolved) val recorded = hashByReal[real] @@ -60,10 +61,7 @@ class StaleWriteRule : ToolCallRule { return ToolCallAssessment(issues = issues, observations = observations, disposition = disposition) } - private fun resolveInput(root: Path, raw: String): Path { - val candidate = Path.of(raw) - return if (candidate.isAbsolute) candidate else root.resolve(candidate) - } + private fun resolveInput(root: Path, raw: String): Path = ToolPath.resolve(raw, root) private fun realOf(input: ToolCallAssessmentInput, root: Path, raw: String): Path = input.probe.resolveReal(resolveInput(root, raw)) diff --git a/core/toolintent/src/main/kotlin/com/correx/core/toolintent/rules/WriteScopeRule.kt b/core/toolintent/src/main/kotlin/com/correx/core/toolintent/rules/WriteScopeRule.kt index b1729845..e4f23296 100644 --- a/core/toolintent/src/main/kotlin/com/correx/core/toolintent/rules/WriteScopeRule.kt +++ b/core/toolintent/src/main/kotlin/com/correx/core/toolintent/rules/WriteScopeRule.kt @@ -7,10 +7,10 @@ import com.correx.core.toolintent.ToolCallAssessmentInput import com.correx.core.toolintent.ToolCallRule import com.correx.core.toolintent.maxAction import com.correx.core.tools.contract.ToolCapability +import com.correx.core.tools.contract.ToolPath import com.correx.core.validation.model.ValidationIssue import com.correx.core.validation.model.ValidationSeverity import java.nio.file.FileSystems -import java.nio.file.Path /** * Write-scope adherence. When the session has claimed a task that declared affected_paths, an @@ -37,11 +37,8 @@ class WriteScopeRule : ToolCallRule { val observations = mutableListOf() var disposition = RiskAction.PROCEED - for (raw in candidatePathStrings(input.paramRoles, input.request.parameters)) { - val candidate = Path.of(raw) - val resolvedReal = input.probe.resolveReal( - if (candidate.isAbsolute) candidate else input.workspace.workspaceRoot.resolve(candidate), - ) + for (raw in writeTargetPathStrings(input.paramRoles, input.request.parameters)) { + val resolvedReal = input.probe.resolveReal(ToolPath.resolve(raw, input.workspace.workspaceRoot)) if (!resolvedReal.startsWith(workspaceReal)) continue // out of workspace: not this gate val rel = workspaceReal.relativize(resolvedReal) val inScope = matchers.any { it.matches(rel) } diff --git a/core/toolintent/src/test/kotlin/com/correx/core/toolintent/PathNormalizationRuleTest.kt b/core/toolintent/src/test/kotlin/com/correx/core/toolintent/PathNormalizationRuleTest.kt new file mode 100644 index 00000000..1b3ec754 --- /dev/null +++ b/core/toolintent/src/test/kotlin/com/correx/core/toolintent/PathNormalizationRuleTest.kt @@ -0,0 +1,149 @@ +package com.correx.core.toolintent + +import com.correx.core.events.events.ToolRequest +import com.correx.core.events.risk.RiskAction +import com.correx.core.events.types.SessionId +import com.correx.core.events.types.StageId +import com.correx.core.events.types.ToolInvocationId +import com.correx.core.toolintent.rules.PathContainmentRule +import com.correx.core.toolintent.rules.ReadBeforeWriteRule +import com.correx.core.toolintent.rules.ReferenceExistsRule +import com.correx.core.toolintent.rules.WriteScopeRule +import com.correx.core.tools.contract.ParamRole +import com.correx.core.tools.contract.ToolCapability +import java.nio.file.Path +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertTrue + +/** + * Plane-2 must judge the SAME path the tool will operate on (ToolPath), and must judge a call's + * source path differently from its write target. + * + * The `~` cases pin the harness bug from the 2026-08 web-ui runs: a correct diagnosis of + * `~/.gradle/init.d/offline.gradle` was resolved to `/~/.gradle/…`, so the reference gate + * called a real file a hallucination and the out-of-workspace prompt never fired. + */ +class PathNormalizationRuleTest { + + private val workspace = Path.of("/work/project") + private val home: String = System.getProperty("user.home") + private val tildeTarget = "~/.gradle/init.d/offline.gradle" + + private class FakeProbe(private val existing: Set = emptySet()) : WorldProbe { + override fun exists(path: Path): Boolean = path.toAbsolutePath().normalize() in existing + override fun resolveReal(path: Path): Path = path.toAbsolutePath().normalize() + } + + private fun input( + parameters: Map, + capabilities: Set, + probe: WorldProbe, + paramRoles: Map = emptyMap(), + reads: Set = emptySet(), + tool: String = "file_read", + activeTask: ActiveTask? = null, + ) = ToolCallAssessmentInput( + request = ToolRequest(ToolInvocationId("i"), SessionId("s"), StageId("st"), tool, parameters), + capabilities = capabilities, + workspace = WorkspacePolicy(workspace, emptyList()), + probe = probe, + paramRoles = paramRoles, + session = SessionContext(reads = reads, activeTask = activeTask), + ) + + @Test + fun `a tilde path is not mistaken for a non-existent in-workspace file`() { + val real = Path.of(home, ".gradle/init.d/offline.gradle") + val r = ReferenceExistsRule().assess( + input( + mapOf("path" to tildeTarget), + setOf(ToolCapability.FILE_READ), + FakeProbe(existing = setOf(real)), + ), + ) + assertEquals(RiskAction.PROCEED, r.disposition) + assertTrue(r.issues.isEmpty(), "a real home-directory file must not be reported as a hallucination") + assertEquals("true", r.observations.single().facts["exists"]) + assertEquals("false", r.observations.single().facts["inWorkspace"]) + } + + @Test + fun `a tilde path still prompts as an out-of-workspace read`() { + // Containment behaviour is preserved: expansion moves the path OUT of the workspace, which is + // where it always belonged, so the operator approval fires instead of being silently skipped. + val r = PathContainmentRule().assess( + input(mapOf("path" to tildeTarget), setOf(ToolCapability.FILE_READ), FakeProbe()), + ) + assertEquals(RiskAction.PROMPT_USER, r.disposition) + assertEquals("PATH_OUTSIDE_WORKSPACE", r.issues.single().code) + assertEquals(Path.of(home, ".gradle/init.d/offline.gradle").toString(), r.observations.single().facts["resolved"]) + } + + @Test + fun `containment judges a source path as well as a write target`() { + val r = PathContainmentRule().assess( + input( + mapOf("source" to "/etc/hosts", "dest" to "public/hosts"), + setOf(ToolCapability.FILE_WRITE), + FakeProbe(), + paramRoles = mapOf("source" to ParamRole.SOURCE_PATH, "dest" to ParamRole.PATH), + tool = "file_copy", + ), + ) + assertEquals(RiskAction.PROMPT_USER, r.disposition) + assertEquals(2, r.observations.size) + } + + @Test + fun `the write-scope gate judges the write target only, not the source`() { + // A source path is read, not mutated, so it is not charged against the task's write scope — + // otherwise every copy of an in-repo asset would have to widen affected_paths to include it. + val r = WriteScopeRule().assess( + input( + mapOf("source" to "assets/logo.png", "dest" to "public/logo.png"), + setOf(ToolCapability.FILE_WRITE), + FakeProbe(), + paramRoles = mapOf("source" to ParamRole.SOURCE_PATH, "dest" to ParamRole.PATH), + tool = "file_copy", + activeTask = ActiveTask("42", listOf("public/**")), + ), + ) + assertEquals(RiskAction.PROCEED, r.disposition) + assertEquals(listOf("public/logo.png"), r.observations.map { it.facts["path"] }) + } + + @Test + fun `a content-from-disk call is exempt from read-before-write even when the dest exists`() { + // Overwriting an existing binary via a copy must not demand a file_read of it first — that + // read is impossible to satisfy usefully and is exactly the round-trip file_copy removes. + val dest = Path.of("/work/project/public/logo.png") + val r = ReadBeforeWriteRule().assess( + input( + mapOf("source" to "assets/logo.png", "dest" to "public/logo.png"), + setOf(ToolCapability.FILE_WRITE), + FakeProbe(existing = setOf(dest)), + paramRoles = mapOf("source" to ParamRole.SOURCE_PATH, "dest" to ParamRole.PATH), + tool = "file_copy", + ), + ) + assertEquals(RiskAction.PROCEED, r.disposition) + assertTrue(r.issues.isEmpty()) + } + + @Test + fun `a model-authored write still requires a prior read`() { + val target = "/work/project/src/A.kt" + val r = ReadBeforeWriteRule().assess( + input( + mapOf("path" to target), + setOf(ToolCapability.FILE_WRITE), + FakeProbe(existing = setOf(Path.of(target))), + paramRoles = mapOf("path" to ParamRole.PATH), + tool = "file_write", + ), + ) + assertEquals(RiskAction.BLOCK, r.disposition) + assertEquals("READ_BEFORE_WRITE", r.issues.single().code) + } +} diff --git a/core/tools/AGENTS.md b/core/tools/AGENTS.md index 4ef7994c..23863af3 100644 --- a/core/tools/AGENTS.md +++ b/core/tools/AGENTS.md @@ -20,7 +20,8 @@ CORREX kernel team. - `FileMutationRecord` — records file-affecting side effects. - `FileAffectingTool` — extended `Tool` interface for tools that write files; must declare `affectedPaths`. - `OutputCompressionSpec` / `ToolOutputCompressor` / `DeclarativeCompressor` — compress large tool outputs to fit token budgets (Hard Invariant #6: compressed output is informational; original events are preserved). -- `ParamRole` — annotates tool parameter semantic roles (input path, output path, etc.). +- `ParamRole` — annotates tool parameter semantic roles: `PATH` (the path a call acts on / mutates), `SOURCE_PATH` (a path it only reads from while acting on another target, e.g. `file_copy`), `EXEC_COMMAND`, `NETWORK_TARGET`. Plane-2 gates dispatch on these, never on tool names. +- `ToolPath` — the ONE canonical path normalization rule: expands a leading `~`/`~/` to the user home, keeps absolute paths, anchors relative paths on the session's working dir (never the JVM cwd). Every filesystem tool and every plane-2 path rule must resolve through it so policy checks and execution act on the same path. `~other/…` is deliberately NOT expanded. - `ValidationResult` — result from tool-level parameter validation (pre-execution check). - Hard Invariant #5: all tool side effects are captured in events. Silent execution is not allowed. diff --git a/core/tools/src/main/kotlin/com/correx/core/tools/contract/ParamRole.kt b/core/tools/src/main/kotlin/com/correx/core/tools/contract/ParamRole.kt index ca997487..a0592fac 100644 --- a/core/tools/src/main/kotlin/com/correx/core/tools/contract/ParamRole.kt +++ b/core/tools/src/main/kotlin/com/correx/core/tools/contract/ParamRole.kt @@ -1,3 +1,12 @@ package com.correx.core.tools.contract -enum class ParamRole { PATH, EXEC_COMMAND, NETWORK_TARGET } +/** + * The semantic role of a declared tool parameter, so plane-2 rules can judge a call by what each + * argument DOES rather than by the tool's name. + * + * [PATH] is the path a call acts ON (its write/read target). [SOURCE_PATH] is a path a call reads + * FROM while acting on some other target — e.g. `file_copy(source, dest)`. Containment and + * privileged-location gates cover both; write-target gates (read-before-write, stale-write, write + * scope, write manifest) cover only [PATH], because a source is not being mutated. + */ +enum class ParamRole { PATH, SOURCE_PATH, EXEC_COMMAND, NETWORK_TARGET } diff --git a/core/tools/src/main/kotlin/com/correx/core/tools/contract/ToolPath.kt b/core/tools/src/main/kotlin/com/correx/core/tools/contract/ToolPath.kt new file mode 100644 index 00000000..5bee7869 --- /dev/null +++ b/core/tools/src/main/kotlin/com/correx/core/tools/contract/ToolPath.kt @@ -0,0 +1,49 @@ +package com.correx.core.tools.contract + +import java.nio.file.Path +import java.nio.file.Paths + +/** + * The ONE canonical rule for turning a model-supplied path string into the absolute path a tool + * will actually operate on. Every filesystem tool and every plane-2 path rule must resolve through + * here, so the policy/existence check and the execution act on the same path. + * + * The bug this exists to prevent: `~/.gradle/init.d/offline.gradle` used to be treated as a + * RELATIVE path (it is not absolute per `Path.isAbsolute`), resolved to + * `/~/.gradle/init.d/offline.gradle`, reported as "does not exist" — and, worse, as + * *inside* the workspace, so the out-of-workspace approval prompt never fired. The agent was then + * told a real file it had correctly identified was a hallucination. + * + * Rules, in order: + * 1. A leading `~` or `~/` expands to the current user's home. `~other/…` is NOT expanded — only + * the shell knows other users' homes, and guessing one would widen the jail on a lookalike path. + * 2. An absolute path is normalized and used as-is. + * 3. A relative path resolves against [base] (a session's workspace/working dir), never the JVM + * process cwd; with no [base] it falls back to the process cwd. + * + * Symlink resolution and containment stay where they were (`PathJail` / the plane-2 world probe): + * this object only decides WHICH path is meant, not whether it is allowed. + */ +object ToolPath { + + /** Expands a leading `~`/`~/` in [raw] to [home]. Any other string is returned unchanged. */ + fun expandHome(raw: String, home: String? = System.getProperty("user.home")): String = when { + home.isNullOrEmpty() -> raw + raw == "~" -> home + raw.startsWith("~/") -> home + raw.substring(1) + else -> raw + } + + /** + * Resolves [raw] to the absolute, normalized path the tool will operate on: home-expanded, + * then anchored on [base] when relative. + */ + fun resolve(raw: String, base: Path?, home: String? = System.getProperty("user.home")): Path { + val expanded = Paths.get(expandHome(raw, home)) + return when { + expanded.isAbsolute -> expanded.normalize() + base != null -> base.resolve(expanded).normalize() + else -> expanded.toAbsolutePath().normalize() + } + } +} diff --git a/core/tools/src/test/kotlin/com/correx/core/tools/contract/ToolPathTest.kt b/core/tools/src/test/kotlin/com/correx/core/tools/contract/ToolPathTest.kt new file mode 100644 index 00000000..ecfc0f06 --- /dev/null +++ b/core/tools/src/test/kotlin/com/correx/core/tools/contract/ToolPathTest.kt @@ -0,0 +1,71 @@ +package com.correx.core.tools.contract + +import java.nio.file.Path +import kotlin.test.Test +import kotlin.test.assertEquals + +class ToolPathTest { + + private val home = "/home/tester" + private val base: Path = Path.of("/work/project") + + @Test + fun `a leading tilde expands to the user home`() { + // The bug this guards: `~/.gradle/init.d/offline.gradle` used to resolve to + // /~/.gradle/... — reported as non-existent AND as inside the workspace, so the + // agent was told a real file it had correctly identified did not exist. + assertEquals( + Path.of("/home/tester/.gradle/init.d/offline.gradle"), + ToolPath.resolve("~/.gradle/init.d/offline.gradle", base, home), + ) + } + + @Test + fun `a bare tilde is the home directory itself`() { + assertEquals(Path.of("/home/tester"), ToolPath.resolve("~", base, home)) + } + + @Test + fun `another users home is not expanded`() { + // Only the shell knows other users' homes; guessing would widen the jail on a lookalike path. + assertEquals(Path.of("/work/project/~other/notes.md"), ToolPath.resolve("~other/notes.md", base, home)) + } + + @Test + fun `an absolute path is normalized and kept`() { + assertEquals(Path.of("/etc/hosts"), ToolPath.resolve("/etc/./hosts", base, home)) + assertEquals(Path.of("/work/a.txt"), ToolPath.resolve("/work/project/../a.txt", base, home)) + } + + @Test + fun `a relative path anchors on the base, not the process cwd`() { + assertEquals(Path.of("/work/project/src/A.kt"), ToolPath.resolve("src/A.kt", base, home)) + assertEquals(Path.of("/work/project/src/A.kt"), ToolPath.resolve("./src/A.kt", base, home)) + } + + @Test + fun `a relative path escaping the base stays escaped for the containment check to see`() { + // Normalization must not silently clamp `..` back inside: the jail decides, not this object. + assertEquals(Path.of("/work/secrets.txt"), ToolPath.resolve("../secrets.txt", base, home)) + } + + @Test + fun `with no base a relative path falls back to the process cwd`() { + assertEquals( + Path.of("").toAbsolutePath().resolve("src/A.kt").normalize(), + ToolPath.resolve("src/A.kt", null, home), + ) + } + + @Test + fun `an unknown home leaves the tilde untouched`() { + assertEquals(Path.of("/work/project/~/x"), ToolPath.resolve("~/x", base, home = null)) + } + + @Test + fun `expandHome only rewrites a leading tilde`() { + assertEquals("/home/tester/x", ToolPath.expandHome("~/x", home)) + assertEquals("src/~/x", ToolPath.expandHome("src/~/x", home)) + assertEquals("/a/b", ToolPath.expandHome("/a/b", home)) + } +} diff --git a/infrastructure/tools/AGENTS.md b/infrastructure/tools/AGENTS.md index 7ca4cb53..58f3fcc7 100644 --- a/infrastructure/tools/AGENTS.md +++ b/infrastructure/tools/AGENTS.md @@ -16,6 +16,8 @@ Adapter for `core:tools`. Depends on `core:tools`, `core:events`, `core:approval - Web search and web fetch results are environment observations; they must be recorded as events by callers to preserve replay determinism (invariant #9). - `ToolConfig` is the only configuration surface; pass via `InfrastructureModule.createToolExecutor()`. - `buildTools()` extension on `ToolConfig` assembles the full tool list; add new tools there, not in the registry directly. +- `file_copy` copies one existing file to another path (`{source, dest}`) so static/binary assets never pass through the model's token stream. Same jail, anchor and `fileWrite.enabled` toggle as `file_write`; `dest` is the mutated target (`ParamRole.PATH`, the only affected path), `source` is read-only (`ParamRole.SOURCE_PATH`) and may sit under an operator-granted out-of-workspace path exactly as a `file_read` may. One regular file per call: no recursion, no globs. +- Every path parameter resolves through `ToolPath.resolve` (`core:tools`), which expands a leading `~` and anchors relatives on the bound workspace's working dir. Do not re-implement path resolution in a tool. - Filesystem mutation is split by intent: `file_write` only writes (`{path, content}`), `file_edit` edits, and `file_delete` only deletes (`{path}`) — deletion is a separately-named capability so a model can never delete by getting a write-mode parameter wrong. `file_delete` shares `file_write`'s path jail and `fileWrite.enabled` toggle and carries `ToolCapability.FILE_WRITE`. - `list_dir` is shallow by default, but collapses a non-symlink single-child directory chain (bounded depth) to the first branch point and explains that expansion in its output; recursive listings retain normal tree traversal. diff --git a/infrastructure/tools/filesystem/src/main/kotlin/com/correx/infrastructure/tools/filesystem/FileCopyTool.kt b/infrastructure/tools/filesystem/src/main/kotlin/com/correx/infrastructure/tools/filesystem/FileCopyTool.kt new file mode 100644 index 00000000..136256a6 --- /dev/null +++ b/infrastructure/tools/filesystem/src/main/kotlin/com/correx/infrastructure/tools/filesystem/FileCopyTool.kt @@ -0,0 +1,205 @@ +package com.correx.infrastructure.tools.filesystem + +import com.correx.core.approvals.Tier +import com.correx.core.events.events.ToolRequest +import com.correx.core.events.types.ToolInvocationId +import com.correx.core.tools.contract.FileAffectingTool +import com.correx.core.tools.contract.ParamRole +import com.correx.core.tools.contract.Tool +import com.correx.core.tools.contract.ToolCapability +import com.correx.core.tools.contract.ToolExecutor +import com.correx.core.tools.contract.ToolPath +import com.correx.core.tools.contract.ToolResult +import com.correx.core.tools.contract.ValidationResult +import kotlinx.coroutines.CancellationException +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.withContext +import kotlinx.serialization.json.JsonObject +import kotlinx.serialization.json.JsonPrimitive +import kotlinx.serialization.json.buildJsonArray +import kotlinx.serialization.json.buildJsonObject +import kotlinx.serialization.json.put +import kotlinx.serialization.json.putJsonObject +import java.io.IOException +import java.nio.file.Files +import java.nio.file.InvalidPathException +import java.nio.file.Path +import java.nio.file.Paths +import java.nio.file.StandardCopyOption + +/** + * Copies one file to another path (#713). Without it, moving an existing asset — a static file, an + * image, anything binary — has to go `file_read` → model context → `file_write`, which burns tool + * rounds, floods the context window and corrupts any byte the tokenizer cannot round-trip. A copy + * never puts the bytes in front of the model. + * + * Carries [ToolCapability.FILE_WRITE] (it mutates the filesystem) and shares [FileWriteTool]'s jail + * and toggle. `dest` is the mutated target ([ParamRole.PATH]); `source` is only read + * ([ParamRole.SOURCE_PATH]), so the plane-2 write-target gates (read-before-write, stale-write, + * write scope, write manifest) judge `dest` alone while containment judges both. `source` may + * additionally sit under an operator-approved out-of-workspace path (`grantedPaths`), exactly as a + * `file_read` may; `dest` never can. + * + * ponytail: single regular file per call — no recursive directory copy, no glob. Copying a tree is + * N calls; add recursion only if a real run shows N is the bottleneck. + */ +class FileCopyTool( + allowedPaths: Set = emptySet(), + private val workingDir: Path? = null, +) : Tool, FileAffectingTool, ToolExecutor { + + private val normalizedAllowedPaths: Set = allowedPaths.map { it.normalize().toAbsolutePath() }.toSet() + + override val name: String = "file_copy" + override val description: String = + "Copy the file at 'source' to 'dest' (creates missing parent directories, overwrites an " + + "existing dest). Use this for existing or binary files instead of reading and " + + "re-writing their contents." + override val parametersSchema: JsonObject = buildJsonObject { + put("type", "object") + putJsonObject("properties") { + putJsonObject("source") { + put("type", "string") + put("description", "Relative path of the existing file to copy from") + } + putJsonObject("dest") { + put("type", "string") + put("description", "Relative path to copy to") + } + } + put( + "required", + buildJsonArray { + add(JsonPrimitive("source")) + add(JsonPrimitive("dest")) + }, + ) + } + override val tier: Tier = Tier.T2 + override val requiredCapabilities: Set = setOf(ToolCapability.FILE_WRITE) + override val paramRoles: Map = + mapOf("source" to ParamRole.SOURCE_PATH, "dest" to ParamRole.PATH) + + /** Only `dest` is mutated, so only `dest` gets pre/post CAS images and undo coverage. */ + override fun affectedPaths(request: ToolRequest): Set { + val dest = request.parameters["dest"] as? String ?: return emptySet() + return setOf(ToolPath.resolve(dest, workingDir)) + } + + override fun validateRequest(request: ToolRequest): ValidationResult { + val source = request.parameters["source"] as? String + ?: return ValidationResult.Invalid( + "Missing 'source' parameter (string). Call file_copy with " + + """{"source": "", "dest": ""}.""", + ) + val dest = request.parameters["dest"] as? String + ?: return ValidationResult.Invalid( + """Missing 'dest' parameter (string). Call file_copy with {"source": "$source", "dest": ""}.""", + ) + if (normalizedAllowedPaths.isEmpty()) return ValidationResult.Invalid("No paths are allowed.") + + return runCatching { + // The source is a read, so it may also be an operator-approved out-of-workspace path + // (OutsidePathAccessGrantedEvent), mirroring file_read. The dest is a write: jail only. + val readRoots = normalizedAllowedPaths + request.grantedPaths.map { Paths.get(it) } + when { + !PathJail.isContained(ToolPath.resolve(source, workingDir), readRoots) -> + ValidationResult.Invalid("Path '$source' is not in the allowed list.") + + !PathJail.isContained(ToolPath.resolve(dest, workingDir), normalizedAllowedPaths) -> + ValidationResult.Invalid("Path '$dest' is not in the allowed list.") + + else -> ValidationResult.Valid + } + }.getOrElse { e -> mapExceptionToValidationResult(e) } + } + + private fun mapExceptionToValidationResult(e: Throwable): ValidationResult = + when (e) { + is InvalidPathException -> ValidationResult.Invalid("Invalid path format: ${e.message}") + is IOException -> ValidationResult.Invalid("IO error: ${e.message}") + is SecurityException -> ValidationResult.Invalid("Security error: ${e.message}") + else -> ValidationResult.Invalid(e.message ?: "Unknown error occurred") + } + + override suspend fun execute(request: ToolRequest): ToolResult = withContext(Dispatchers.IO) { + val validation = validateRequest(request) + if (validation is ValidationResult.Invalid) { + return@withContext ToolResult.Failure( + invocationId = request.invocationId, + reason = validation.reason, + recoverable = false, + ) + } + val sourceString = request.parameters["source"] as String + val destString = request.parameters["dest"] as String + val source = ToolPath.resolve(sourceString, workingDir) + val dest = ToolPath.resolve(destString, workingDir) + + // Every rejection below is recoverable: each one is a mistake the agent can correct on the + // next tool round (fix the path, name a file inside the directory) rather than a dead stage. + when { + !Files.exists(source) -> return@withContext ToolResult.Failure( + invocationId = request.invocationId, + reason = "Source file not found: $sourceString", + recoverable = true, + ) + + Files.isDirectory(source) -> return@withContext ToolResult.Failure( + invocationId = request.invocationId, + reason = "Source '$sourceString' is a directory — file_copy copies one file per call.", + recoverable = true, + ) + + Files.isDirectory(dest) -> return@withContext ToolResult.Failure( + invocationId = request.invocationId, + reason = "Dest '$destString' is a directory — name the file inside it " + + "(e.g. '$destString/${source.fileName}').", + recoverable = true, + ) + + source == dest -> return@withContext ToolResult.Failure( + invocationId = request.invocationId, + reason = "Source and dest resolve to the same file ($destString) — nothing to copy.", + recoverable = true, + ) + } + + runCatching { + dest.parent?.let { Files.createDirectories(it) } + Files.copy(source, dest, StandardCopyOption.REPLACE_EXISTING) + ToolResult.Success( + invocationId = request.invocationId, + output = "Copied $sourceString to $destString (${Files.size(dest)} bytes)", + metadata = mapOf("source" to sourceString, "dest" to destString), + ) + }.getOrElse { e -> handleExecutionException(e, request.invocationId, destString) } + } + + private fun handleExecutionException( + e: Throwable, + invocationId: ToolInvocationId, + destString: String, + ): ToolResult = when (e) { + is CancellationException -> throw e + // Recoverable for the same reason as file_write's IO errors: the agent can correct the + // path and retry inside the stage's tool loop. + is IOException -> ToolResult.Failure( + invocationId = invocationId, + reason = "IO error copying to '$destString': ${e.message ?: e.javaClass.simpleName}", + recoverable = true, + ) + + is SecurityException -> ToolResult.Failure( + invocationId = invocationId, + reason = "Access denied: $destString, ${e.message}", + recoverable = false, + ) + + else -> ToolResult.Failure( + invocationId = invocationId, + reason = e.message ?: "Unknown error occurred", + recoverable = false, + ) + } +} diff --git a/infrastructure/tools/filesystem/src/main/kotlin/com/correx/infrastructure/tools/filesystem/FileDeleteTool.kt b/infrastructure/tools/filesystem/src/main/kotlin/com/correx/infrastructure/tools/filesystem/FileDeleteTool.kt index 972fe80c..083bef49 100644 --- a/infrastructure/tools/filesystem/src/main/kotlin/com/correx/infrastructure/tools/filesystem/FileDeleteTool.kt +++ b/infrastructure/tools/filesystem/src/main/kotlin/com/correx/infrastructure/tools/filesystem/FileDeleteTool.kt @@ -6,6 +6,7 @@ import com.correx.core.events.types.ToolInvocationId import com.correx.core.tools.contract.FileAffectingTool import com.correx.core.tools.contract.ParamRole import com.correx.core.tools.contract.Tool +import com.correx.core.tools.contract.ToolPath import com.correx.core.tools.contract.ToolCapability import com.correx.core.tools.contract.ToolExecutor import com.correx.core.tools.contract.ToolResult @@ -23,7 +24,6 @@ import java.io.IOException import java.nio.file.Files import java.nio.file.InvalidPathException import java.nio.file.Path -import java.nio.file.Paths /** * Deletes a file. Split out of [FileWriteTool] so deletion is an explicitly-named capability a model @@ -59,12 +59,10 @@ class FileDeleteTool( } private fun resolvePath(pathString: String): Path { - val raw = Paths.get(pathString) - return when { - raw.isAbsolute -> raw.normalize() - workingDir != null -> workingDir.resolve(raw).normalize() - else -> raw.toAbsolutePath().normalize() - } + // ToolPath is the ONE normalization rule (expands a leading `~`, anchors relatives on the + // bound workspace's working dir, never the JVM cwd) so the jail check, the existence check + // and the operation itself all act on the same path. + return ToolPath.resolve(pathString, workingDir) } override fun validateRequest(request: ToolRequest): ValidationResult { 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 9d361449..1c016ea7 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 @@ -6,6 +6,7 @@ import com.correx.core.events.types.ToolInvocationId import com.correx.core.tools.contract.FileAffectingTool import com.correx.core.tools.contract.ParamRole import com.correx.core.tools.contract.Tool +import com.correx.core.tools.contract.ToolPath import com.correx.core.tools.contract.ToolCapability import com.correx.core.tools.contract.ToolExecutor import com.correx.core.tools.contract.ToolResult @@ -33,12 +34,10 @@ class FileEditTool( private val normalizedAllowedPaths: Set = allowedPaths.map { it.normalize().toAbsolutePath() }.toSet() private fun resolvePath(pathString: String): Path { - val raw = Paths.get(pathString) - return when { - raw.isAbsolute -> raw.normalize() - workingDir != null -> workingDir.resolve(raw).normalize() - else -> raw.toAbsolutePath().normalize() - } + // ToolPath is the ONE normalization rule (expands a leading `~`, anchors relatives on the + // bound workspace's working dir, never the JVM cwd) so the jail check, the existence check + // and the operation itself all act on the same path. + return ToolPath.resolve(pathString, workingDir) } override val name: String = "file_edit" diff --git a/infrastructure/tools/filesystem/src/main/kotlin/com/correx/infrastructure/tools/filesystem/FileReadTool.kt b/infrastructure/tools/filesystem/src/main/kotlin/com/correx/infrastructure/tools/filesystem/FileReadTool.kt index 9c5c6277..139a6e75 100644 --- a/infrastructure/tools/filesystem/src/main/kotlin/com/correx/infrastructure/tools/filesystem/FileReadTool.kt +++ b/infrastructure/tools/filesystem/src/main/kotlin/com/correx/infrastructure/tools/filesystem/FileReadTool.kt @@ -9,6 +9,7 @@ import com.correx.core.tools.compression.OutputCompressionSpec import com.correx.core.tools.compression.ToolOutputCompressor import com.correx.core.tools.contract.ParamRole import com.correx.core.tools.contract.Tool +import com.correx.core.tools.contract.ToolPath import com.correx.core.tools.contract.ToolCapability import com.correx.core.tools.contract.ToolExecutor import com.correx.core.tools.contract.ToolResult @@ -38,12 +39,10 @@ class FileReadTool( // process cwd. Mirrors FileWriteTool/FileEditTool so all three jail against the // same anchor and agree with the Plane-2 tool-call-intent containment check. private fun resolvePath(pathString: String): Path { - val raw = Paths.get(pathString) - return when { - raw.isAbsolute -> raw.normalize() - workingDir != null -> workingDir.resolve(raw).normalize() - else -> raw.toAbsolutePath().normalize() - } + // ToolPath is the ONE normalization rule (expands a leading `~`, anchors relatives on the + // bound workspace's working dir, never the JVM cwd) so the jail check, the existence check + // and the operation itself all act on the same path. + return ToolPath.resolve(pathString, workingDir) } override val name: String = "file_read" diff --git a/infrastructure/tools/filesystem/src/main/kotlin/com/correx/infrastructure/tools/filesystem/FileWriteTool.kt b/infrastructure/tools/filesystem/src/main/kotlin/com/correx/infrastructure/tools/filesystem/FileWriteTool.kt index db8b17f9..193eff8d 100644 --- a/infrastructure/tools/filesystem/src/main/kotlin/com/correx/infrastructure/tools/filesystem/FileWriteTool.kt +++ b/infrastructure/tools/filesystem/src/main/kotlin/com/correx/infrastructure/tools/filesystem/FileWriteTool.kt @@ -6,6 +6,7 @@ import com.correx.core.events.types.ToolInvocationId import com.correx.core.tools.contract.FileAffectingTool import com.correx.core.tools.contract.ParamRole import com.correx.core.tools.contract.Tool +import com.correx.core.tools.contract.ToolPath import com.correx.core.tools.contract.ToolCapability import com.correx.core.tools.contract.ToolExecutor import com.correx.core.tools.contract.ToolResult @@ -23,7 +24,6 @@ import java.io.IOException import java.nio.file.Files import java.nio.file.InvalidPathException import java.nio.file.Path -import java.nio.file.Paths /** * Writes content to a file. Write-only by design: deleting is the separate, explicitly-named @@ -73,12 +73,10 @@ class FileWriteTool( } private fun resolvePath(pathString: String): Path { - val raw = Paths.get(pathString) - return when { - raw.isAbsolute -> raw.normalize() - workingDir != null -> workingDir.resolve(raw).normalize() - else -> raw.toAbsolutePath().normalize() - } + // ToolPath is the ONE normalization rule (expands a leading `~`, anchors relatives on the + // bound workspace's working dir, never the JVM cwd) so the jail check, the existence check + // and the operation itself all act on the same path. + return ToolPath.resolve(pathString, workingDir) } override fun validateRequest(request: ToolRequest): ValidationResult { diff --git a/infrastructure/tools/filesystem/src/main/kotlin/com/correx/infrastructure/tools/filesystem/ListDirTool.kt b/infrastructure/tools/filesystem/src/main/kotlin/com/correx/infrastructure/tools/filesystem/ListDirTool.kt index 2589fcf8..88cdd58b 100644 --- a/infrastructure/tools/filesystem/src/main/kotlin/com/correx/infrastructure/tools/filesystem/ListDirTool.kt +++ b/infrastructure/tools/filesystem/src/main/kotlin/com/correx/infrastructure/tools/filesystem/ListDirTool.kt @@ -4,6 +4,7 @@ import com.correx.core.approvals.Tier import com.correx.core.events.events.ToolRequest import com.correx.core.tools.contract.ParamRole import com.correx.core.tools.contract.Tool +import com.correx.core.tools.contract.ToolPath import com.correx.core.tools.contract.ToolCapability import com.correx.core.tools.contract.ToolExecutor import com.correx.core.tools.contract.ToolResult @@ -43,12 +44,10 @@ class ListDirTool( ) : Tool, ToolExecutor { private fun resolvePath(pathString: String): Path { - val raw = Paths.get(pathString) - return when { - raw.isAbsolute -> raw.normalize() - workingDir != null -> workingDir.resolve(raw).normalize() - else -> raw.toAbsolutePath().normalize() - } + // ToolPath is the ONE normalization rule (expands a leading `~`, anchors relatives on the + // bound workspace's working dir, never the JVM cwd) so the jail check, the existence check + // and the operation itself all act on the same path. + return ToolPath.resolve(pathString, workingDir) } override val name: String = "list_dir" diff --git a/infrastructure/tools/filesystem/src/main/kotlin/com/correx/infrastructure/tools/filesystem/WorkspaceSearchTools.kt b/infrastructure/tools/filesystem/src/main/kotlin/com/correx/infrastructure/tools/filesystem/WorkspaceSearchTools.kt index 03b3f70a..b5c26daa 100644 --- a/infrastructure/tools/filesystem/src/main/kotlin/com/correx/infrastructure/tools/filesystem/WorkspaceSearchTools.kt +++ b/infrastructure/tools/filesystem/src/main/kotlin/com/correx/infrastructure/tools/filesystem/WorkspaceSearchTools.kt @@ -4,6 +4,7 @@ import com.correx.core.approvals.Tier import com.correx.core.events.events.ToolRequest import com.correx.core.tools.contract.ParamRole import com.correx.core.tools.contract.Tool +import com.correx.core.tools.contract.ToolPath import com.correx.core.tools.contract.ToolCapability import com.correx.core.tools.contract.ToolExecutor import com.correx.core.tools.contract.ToolResult @@ -224,12 +225,7 @@ class GrepTool( private fun resolveSearchPath(request: ToolRequest, workingDir: Path?): Path { val pathString = (request.parameters["path"] as? String)?.takeIf { it.isNotBlank() } ?: "." - val raw = Paths.get(pathString) - return when { - raw.isAbsolute -> raw.normalize() - workingDir != null -> workingDir.resolve(raw).normalize() - else -> raw.toAbsolutePath().normalize() - } + return ToolPath.resolve(pathString, workingDir) } private fun validateSearchPath(request: ToolRequest, allowedPaths: Set, workingDir: Path?): ValidationResult = diff --git a/infrastructure/tools/filesystem/src/test/kotlin/com/correx/infrastructure/tools/filesystem/FileCopyToolTest.kt b/infrastructure/tools/filesystem/src/test/kotlin/com/correx/infrastructure/tools/filesystem/FileCopyToolTest.kt new file mode 100644 index 00000000..97d7ff5c --- /dev/null +++ b/infrastructure/tools/filesystem/src/test/kotlin/com/correx/infrastructure/tools/filesystem/FileCopyToolTest.kt @@ -0,0 +1,162 @@ +package com.correx.infrastructure.tools.filesystem + +import com.correx.core.events.events.ToolRequest +import com.correx.core.events.types.SessionId +import com.correx.core.events.types.StageId +import com.correx.core.events.types.ToolInvocationId +import com.correx.core.tools.contract.ToolResult +import com.correx.core.tools.contract.ValidationResult +import kotlinx.coroutines.runBlocking +import org.junit.jupiter.api.Assertions.assertArrayEquals +import org.junit.jupiter.api.Assertions.assertEquals +import org.junit.jupiter.api.Assertions.assertTrue +import org.junit.jupiter.api.Test +import java.nio.file.Files +import java.nio.file.Path +import java.util.UUID + +class FileCopyToolTest { + + private val invocationId = ToolInvocationId(UUID.randomUUID().toString()) + + private fun request(parameters: Map, grantedPaths: Set = emptySet()): ToolRequest = + ToolRequest( + invocationId = invocationId, + sessionId = SessionId(UUID.randomUUID().toString()), + stageId = StageId(UUID.randomUUID().toString()), + toolName = "file_copy", + parameters = parameters, + grantedPaths = grantedPaths, + ) + + private fun workspace(): Path = Files.createTempDirectory("file_copy_test") + + @Test + fun `copies bytes without routing them through the model`(): Unit = runBlocking { + val dir = workspace() + val bytes = byteArrayOf(0, 1, 2, -1, -128, 127) + Files.write(dir.resolve("logo.png"), bytes) + val tool = FileCopyTool(allowedPaths = setOf(dir), workingDir = dir) + + val result = tool.execute(request(mapOf("source" to "logo.png", "dest" to "public/assets/logo.png"))) + + assertTrue(result is ToolResult.Success, (result as? ToolResult.Failure)?.reason) + // Parent directories are created, and every byte survives — the point of the tool. + assertArrayEquals(bytes, Files.readAllBytes(dir.resolve("public/assets/logo.png"))) + } + + @Test + fun `only dest is reported as an affected path`(): Unit = runBlocking { + val dir = workspace() + val tool = FileCopyTool(allowedPaths = setOf(dir), workingDir = dir) + assertEquals( + setOf(dir.resolve("b.txt")), + tool.affectedPaths(request(mapOf("source" to "a.txt", "dest" to "b.txt"))), + ) + } + + @Test + fun `overwrites an existing dest`(): Unit = runBlocking { + val dir = workspace() + Files.writeString(dir.resolve("a.txt"), "new") + Files.writeString(dir.resolve("b.txt"), "old") + val tool = FileCopyTool(allowedPaths = setOf(dir), workingDir = dir) + + assertTrue(tool.execute(request(mapOf("source" to "a.txt", "dest" to "b.txt"))) is ToolResult.Success) + assertEquals("new", Files.readString(dir.resolve("b.txt"))) + } + + @Test + fun `a missing source is a recoverable failure`(): Unit = runBlocking { + val dir = workspace() + val tool = FileCopyTool(allowedPaths = setOf(dir), workingDir = dir) + val result = tool.execute(request(mapOf("source" to "ghost.txt", "dest" to "b.txt"))) + assertTrue(result is ToolResult.Failure) + assertTrue((result as ToolResult.Failure).recoverable) + assertTrue(result.reason.contains("Source file not found")) + } + + @Test + fun `a directory source or dest is rejected with the corrective move`(): Unit = runBlocking { + val dir = workspace() + Files.createDirectories(dir.resolve("assets")) + Files.writeString(dir.resolve("a.txt"), "x") + val tool = FileCopyTool(allowedPaths = setOf(dir), workingDir = dir) + + val fromDir = tool.execute(request(mapOf("source" to "assets", "dest" to "b.txt"))) + assertTrue((fromDir as ToolResult.Failure).reason.contains("one file per call")) + + val toDir = tool.execute(request(mapOf("source" to "a.txt", "dest" to "assets"))) + assertTrue((toDir as ToolResult.Failure).reason.contains("assets/a.txt")) + } + + @Test + fun `copying a file onto itself is rejected`(): Unit = runBlocking { + val dir = workspace() + Files.writeString(dir.resolve("a.txt"), "x") + val tool = FileCopyTool(allowedPaths = setOf(dir), workingDir = dir) + val result = tool.execute(request(mapOf("source" to "a.txt", "dest" to "./a.txt"))) + assertTrue((result as ToolResult.Failure).reason.contains("nothing to copy")) + } + + @Test + fun `both source and dest are jailed`(): Unit = runBlocking { + val dir = workspace() + val outside = Files.createTempDirectory("file_copy_outside") + Files.writeString(outside.resolve("other.txt"), "s") + Files.writeString(dir.resolve("a.txt"), "x") + val tool = FileCopyTool(allowedPaths = setOf(dir), workingDir = dir) + + val readEscape = tool.validateRequest( + request(mapOf("source" to outside.resolve("other.txt").toString(), "dest" to "a.txt")), + ) + assertTrue(readEscape is ValidationResult.Invalid) + + val writeEscape = tool.validateRequest( + request(mapOf("source" to "a.txt", "dest" to outside.resolve("copied.txt").toString())), + ) + assertTrue(writeEscape is ValidationResult.Invalid) + } + + @Test + fun `an operator-granted path widens the source jail but never the dest`(): Unit = runBlocking { + val dir = workspace() + val granted = Files.createTempDirectory("file_copy_granted") + Files.writeString(granted.resolve("asset.bin"), "a") + val tool = FileCopyTool(allowedPaths = setOf(dir), workingDir = dir) + val grants = setOf(granted.toString()) + + // Mirrors file_read: an approved out-of-workspace path may be READ from… + assertEquals( + ValidationResult.Valid, + tool.validateRequest( + request(mapOf("source" to granted.resolve("asset.bin").toString(), "dest" to "asset.bin"), grants), + ), + ) + // …but a write target never escapes the workspace, approved or not. + assertTrue( + tool.validateRequest( + request(mapOf("source" to "asset.bin", "dest" to granted.resolve("out.bin").toString()), grants), + ) is ValidationResult.Invalid, + ) + } + + @Test + fun `a tilde path resolves to the user home instead of a phantom workspace path`(): Unit = runBlocking { + val home = Path.of(System.getProperty("user.home")) + val dir = workspace() + val tool = FileCopyTool(allowedPaths = setOf(dir, home), workingDir = dir) + // The canonical normalization applies to every path param, not just file_read's. + assertEquals(setOf(home.resolve("x.bin")), tool.affectedPaths(request(mapOf("dest" to "~/x.bin")))) + } + + @Test + fun `missing parameters name the exact call shape`(): Unit = runBlocking { + val dir = workspace() + val tool = FileCopyTool(allowedPaths = setOf(dir), workingDir = dir) + val noSource = tool.validateRequest(request(mapOf("dest" to "b.txt"))) + assertTrue((noSource as ValidationResult.Invalid).reason.contains("Missing 'source'")) + val noDest = tool.validateRequest(request(mapOf("source" to "a.txt"))) + assertTrue((noDest as ValidationResult.Invalid).reason.contains("Missing 'dest'")) + } +} diff --git a/infrastructure/tools/filesystem/src/test/kotlin/com/correx/infrastructure/tools/filesystem/FileReadToolTest.kt b/infrastructure/tools/filesystem/src/test/kotlin/com/correx/infrastructure/tools/filesystem/FileReadToolTest.kt index 1110784e..fed83fe1 100644 --- a/infrastructure/tools/filesystem/src/test/kotlin/com/correx/infrastructure/tools/filesystem/FileReadToolTest.kt +++ b/infrastructure/tools/filesystem/src/test/kotlin/com/correx/infrastructure/tools/filesystem/FileReadToolTest.kt @@ -200,4 +200,22 @@ class FileReadToolTest { val compressed = tool.outputCompressor.compress(raw, ToolOutputContext(exitCode = 0)) assertEquals("fun main() {\nprintln()\n}", compressed) } + + @Test + fun `a home-relative path is read, not reported as missing`(): Unit = runBlocking { + // The 2026-08 harness bug: `~/…` was treated as relative, resolved under the workspace, and + // reported as "File not found" — telling the agent a real file it had correctly identified + // did not exist. Every path param normalizes through ToolPath now. + val home = java.nio.file.Path.of(System.getProperty("user.home")) + val marker = Files.createTempFile(home, "correx_tilde_read", ".txt") + try { + Files.writeString(marker, "real content") + val tool = FileReadTool(allowedPaths = setOf(home), workingDir = Files.createTempDirectory("ws")) + val result = tool.execute(createRequest("~/${marker.fileName}")) + assertTrue(result is ToolResult.Success, (result as? ToolResult.Failure)?.reason) + assertEquals("real content", (result as ToolResult.Success).output) + } finally { + Files.deleteIfExists(marker) + } + } } diff --git a/infrastructure/tools/src/main/kotlin/com/correx/infrastructure/tools/ToolConfig.kt b/infrastructure/tools/src/main/kotlin/com/correx/infrastructure/tools/ToolConfig.kt index d1a53945..8b2100b0 100644 --- a/infrastructure/tools/src/main/kotlin/com/correx/infrastructure/tools/ToolConfig.kt +++ b/infrastructure/tools/src/main/kotlin/com/correx/infrastructure/tools/ToolConfig.kt @@ -1,6 +1,7 @@ package com.correx.infrastructure.tools import com.correx.core.tools.contract.Tool +import com.correx.infrastructure.tools.filesystem.FileCopyTool import com.correx.infrastructure.tools.filesystem.FileDeleteTool import com.correx.infrastructure.tools.filesystem.FileEditTool import com.correx.infrastructure.tools.filesystem.FileReadTool @@ -104,6 +105,14 @@ fun ToolConfig.buildTools(): List = buildList { workingDir = fileWrite.workingDir, ), ) + // file_copy moves existing/binary files without routing their bytes through the model's + // context (#713); same jail, anchor and toggle as the writer. + add( + FileCopyTool( + allowedPaths = fileWrite.allowedPaths, + workingDir = fileWrite.workingDir, + ), + ) } if (fileEdit.enabled) { add( -- 2.52.0 From d18075925d863e9da45a455fda5516b8461c715e Mon Sep 17 00:00:00 2001 From: claude Date: Thu, 27 Aug 2026 12:04:04 +0400 Subject: [PATCH 5/6] fix(toolintent): key the read-before-write exemption on content provenance, not parameter shape The exemption added in 6a8a7b31 was broader than the invariant it stood on. "Tool declares a SOURCE_PATH" is a claim about the parameter list; the safe property is "every byte written derives from an existing source object rather than from model-supplied content". A future transform or import tool could name a source and still write model-controlled output, and would have inherited the exemption. ToolCapability.CONTENT_FROM_SOURCE now carries that provenance claim explicitly. file_copy declares it; ReadBeforeWriteRule.appliesTo stands down only for calls that do, so ToolCallAssessor skips the rule rather than the rule skipping itself. The capability is recorded on the invocation event like every other one, so replay classifies a call by what it actually claimed instead of re-deriving it from parameters. Tool availability is by declared tool name, not capability-set containment, so the extra capability does not narrow which stages can reach file_copy. Tests: the exemption is asserted through ToolCallAssessor, plus a source-naming tool WITHOUT the provenance capability that stays gated. ./gradlew check green. Co-Authored-By: Claude Opus 5 --- core/events/AGENTS.md | 1 + .../core/tools/contract/ToolCapability.kt | 12 ++++++++ core/toolintent/AGENTS.md | 2 +- .../toolintent/rules/ReadBeforeWriteRule.kt | 16 +++++----- .../toolintent/PathNormalizationRuleTest.kt | 30 +++++++++++++++++-- infrastructure/tools/AGENTS.md | 2 +- .../tools/filesystem/FileCopyTool.kt | 6 +++- 7 files changed, 55 insertions(+), 14 deletions(-) diff --git a/core/events/AGENTS.md b/core/events/AGENTS.md index e4217d66..2e5d0378 100644 --- a/core/events/AGENTS.md +++ b/core/events/AGENTS.md @@ -22,6 +22,7 @@ CORREX kernel team. This is the most cross-cutting module in the codebase — ch - Domain event files: `ApprovalEvents`, `ArtifactEvents`, `ContextEvents`, `InferenceEvents`, `OrchestrationEvents`, `RouterEvents`, `SessionEvents`, `TaskEvents`, `ToolEvents`, `IntentEvents`, `RiskAssessedEvent`, `JournalCompactedEvent`, and many more — all payload definitions live here. - `FailureAttribution` / `FailureAttributor` — the terminal-failure taxonomy (`AGENT`, `HARNESS`, `WORKFLOW`, `ENVIRONMENT`, `PROVIDER`, `OPERATOR`, `UNKNOWN`) carried as `WorkflowFailedEvent.attribution`, plus the deterministic reason→layer mapping used both at emission and when classifying historical events. One primary attribution per terminal event; a multi-cause chain is the session's `FailureTicketOpenedEvent`s, not a second structure. - `LspDiagnosticsCompletedEvent` records pulled language-server diagnostics or a graceful skip reason; replay consumes this observation and never contacts the server. +- `ToolCapability.CONTENT_FROM_SOURCE` — content-provenance claim: the bytes a call writes derive entirely from an existing source object it names, never from model output. Recorded on the invocation event like every other capability, so replay classifies the call by what it actually claimed. Only declare it on a tool whose output is a faithful reproduction of its source. - 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/tools/contract/ToolCapability.kt b/core/events/src/main/kotlin/com/correx/core/tools/contract/ToolCapability.kt index 42df5d2c..2fcf0e4b 100644 --- a/core/events/src/main/kotlin/com/correx/core/tools/contract/ToolCapability.kt +++ b/core/events/src/main/kotlin/com/correx/core/tools/contract/ToolCapability.kt @@ -23,6 +23,18 @@ enum class ToolCapability { */ DIRECTORY_LIST, FILE_WRITE, + + /** + * Content provenance: every byte this call writes is derived from an existing source object the + * call names (a file on disk, a stored artifact), never from model-supplied content. It is a + * claim about WHERE the bytes come from, not about the shape of the parameter list — a transform + * or import tool that mixes in model-authored output must NOT declare it. + * + * Carried alongside [FILE_WRITE] (such a call still mutates the filesystem) so the gates that + * exist to stop a model writing from memory can stand down: requiring a prior `file_read` of a + * copied file's bytes is unsatisfiable for a binary and defeats the point of copying it. + */ + CONTENT_FROM_SOURCE, NETWORK_ACCESS, SHELL_EXEC, PROCESS_SPAWN, diff --git a/core/toolintent/AGENTS.md b/core/toolintent/AGENTS.md index b759643d..4042b52f 100644 --- a/core/toolintent/AGENTS.md +++ b/core/toolintent/AGENTS.md @@ -34,7 +34,7 @@ CORREX kernel team. This module enforces Hard Invariant #9 for the tool-call pat - Hard Invariant #5: every tool call must be assessed before execution. Assessment result is recorded as `ToolCallAssessmentEvents` in `core:events`. - New rules implement `ToolCallRule` and are registered in `WorkspacePolicy`. Do not add rule logic directly to `ToolCallAssessor`. - Resolve every model-supplied path through `ToolPath.resolve` (`core:tools`) — the one canonical normalization rule, shared with the filesystem tools. A rule that resolves paths itself will judge a different path than the tool operates on (the `~` bug: `~/x` resolved to `/~/x`, so a real home-directory file was reported as a non-existent in-workspace file and the out-of-workspace prompt never fired). -- A call that declares a `SOURCE_PATH` takes its content from disk, not from the model, so `ReadBeforeWriteRule` exempts it: requiring a read of a copied file's bytes is unsatisfiable for binaries and defeats the purpose of the tool. +- `ReadBeforeWriteRule` exempts calls declaring `ToolCapability.CONTENT_FROM_SOURCE` — every byte written comes from an existing source object, so there is no model-authored content to clobber with, and requiring a read of a copied binary is unsatisfiable. The exemption keys on that declared PROVENANCE, never on the presence of a `SOURCE_PATH` parameter: a transform or import tool may name a source and still write model-controlled output, and must stay gated. It lives in `appliesTo`, so `ToolCallAssessor` skips the rule entirely. ## Verification diff --git a/core/toolintent/src/main/kotlin/com/correx/core/toolintent/rules/ReadBeforeWriteRule.kt b/core/toolintent/src/main/kotlin/com/correx/core/toolintent/rules/ReadBeforeWriteRule.kt index 08cc8e7d..c54dcefd 100644 --- a/core/toolintent/src/main/kotlin/com/correx/core/toolintent/rules/ReadBeforeWriteRule.kt +++ b/core/toolintent/src/main/kotlin/com/correx/core/toolintent/rules/ReadBeforeWriteRule.kt @@ -6,7 +6,6 @@ import com.correx.core.toolintent.ToolCallAssessment import com.correx.core.toolintent.ToolCallAssessmentInput import com.correx.core.toolintent.ToolCallRule import com.correx.core.toolintent.maxAction -import com.correx.core.tools.contract.ParamRole import com.correx.core.tools.contract.ToolCapability import com.correx.core.tools.contract.ToolPath import com.correx.core.validation.model.ValidationIssue @@ -26,15 +25,16 @@ import java.nio.file.Path class ReadBeforeWriteRule : ToolCallRule { override fun appliesTo(capabilities: Set): Boolean = - ToolCapability.FILE_WRITE in capabilities + ToolCapability.FILE_WRITE in capabilities && + // A call whose bytes come entirely from an existing source object + // ([ToolCapability.CONTENT_FROM_SOURCE]) has nothing for this gate to protect: there is + // no model-authored content to clobber the file with. The exemption keys on that + // declared provenance, NOT on the presence of a source-path parameter — a future + // transform or import tool could name a source and still write model-controlled output, + // and must stay gated. + ToolCapability.CONTENT_FROM_SOURCE !in capabilities override fun assess(input: ToolCallAssessmentInput): ToolCallAssessment { - // A call that declares a SOURCE_PATH takes its content from a file on disk, not from the - // model's memory, so there is nothing for this gate to protect: demanding a file_read of the - // source (or of the target being replaced by it) would force the very bytes-through-context - // round-trip that such tools exist to avoid — and is unsatisfiable for a binary file. - if (input.paramRoles.containsValue(ParamRole.SOURCE_PATH)) return ToolCallAssessment() - val root = input.workspace.workspaceRoot val readReal = input.session.reads.map { realOf(input, root, it) }.toSet() diff --git a/core/toolintent/src/test/kotlin/com/correx/core/toolintent/PathNormalizationRuleTest.kt b/core/toolintent/src/test/kotlin/com/correx/core/toolintent/PathNormalizationRuleTest.kt index 1b3ec754..0dbed063 100644 --- a/core/toolintent/src/test/kotlin/com/correx/core/toolintent/PathNormalizationRuleTest.kt +++ b/core/toolintent/src/test/kotlin/com/correx/core/toolintent/PathNormalizationRuleTest.kt @@ -14,6 +14,7 @@ import com.correx.core.tools.contract.ToolCapability import java.nio.file.Path import kotlin.test.Test import kotlin.test.assertEquals +import kotlin.test.assertFalse import kotlin.test.assertTrue /** @@ -114,14 +115,17 @@ class PathNormalizationRuleTest { } @Test - fun `a content-from-disk call is exempt from read-before-write even when the dest exists`() { + fun `a content-from-source call is exempt from read-before-write`() { // Overwriting an existing binary via a copy must not demand a file_read of it first — that // read is impossible to satisfy usefully and is exactly the round-trip file_copy removes. + // The exemption lives in appliesTo, so ToolCallAssessor never runs the gate for such a call. + val exempt = setOf(ToolCapability.FILE_WRITE, ToolCapability.CONTENT_FROM_SOURCE) + assertFalse(ReadBeforeWriteRule().appliesTo(exempt)) val dest = Path.of("/work/project/public/logo.png") - val r = ReadBeforeWriteRule().assess( + val r = ToolCallAssessor(listOf(ReadBeforeWriteRule())).assess( input( mapOf("source" to "assets/logo.png", "dest" to "public/logo.png"), - setOf(ToolCapability.FILE_WRITE), + exempt, FakeProbe(existing = setOf(dest)), paramRoles = mapOf("source" to ParamRole.SOURCE_PATH, "dest" to ParamRole.PATH), tool = "file_copy", @@ -131,6 +135,26 @@ class PathNormalizationRuleTest { assertTrue(r.issues.isEmpty()) } + @Test + fun `naming a source does not by itself earn the exemption`() { + // The invariant is content provenance, not parameter shape: a hypothetical transform tool + // that reads a source AND writes model-authored output stays gated. + val target = "/work/project/src/A.kt" + val capabilities = setOf(ToolCapability.FILE_WRITE) + assertTrue(ReadBeforeWriteRule().appliesTo(capabilities)) + val r = ReadBeforeWriteRule().assess( + input( + mapOf("source" to "template.kt", "path" to target), + capabilities, + FakeProbe(existing = setOf(Path.of(target))), + paramRoles = mapOf("source" to ParamRole.SOURCE_PATH, "path" to ParamRole.PATH), + tool = "file_transform", + ), + ) + assertEquals(RiskAction.BLOCK, r.disposition) + assertEquals("READ_BEFORE_WRITE", r.issues.single().code) + } + @Test fun `a model-authored write still requires a prior read`() { val target = "/work/project/src/A.kt" diff --git a/infrastructure/tools/AGENTS.md b/infrastructure/tools/AGENTS.md index 58f3fcc7..1e1366ba 100644 --- a/infrastructure/tools/AGENTS.md +++ b/infrastructure/tools/AGENTS.md @@ -16,7 +16,7 @@ Adapter for `core:tools`. Depends on `core:tools`, `core:events`, `core:approval - Web search and web fetch results are environment observations; they must be recorded as events by callers to preserve replay determinism (invariant #9). - `ToolConfig` is the only configuration surface; pass via `InfrastructureModule.createToolExecutor()`. - `buildTools()` extension on `ToolConfig` assembles the full tool list; add new tools there, not in the registry directly. -- `file_copy` copies one existing file to another path (`{source, dest}`) so static/binary assets never pass through the model's token stream. Same jail, anchor and `fileWrite.enabled` toggle as `file_write`; `dest` is the mutated target (`ParamRole.PATH`, the only affected path), `source` is read-only (`ParamRole.SOURCE_PATH`) and may sit under an operator-granted out-of-workspace path exactly as a `file_read` may. One regular file per call: no recursion, no globs. +- `file_copy` copies one existing file to another path (`{source, dest}`) so static/binary assets never pass through the model's token stream. Same jail, anchor and `fileWrite.enabled` toggle as `file_write`; It declares `ToolCapability.CONTENT_FROM_SOURCE` (its bytes are a faithful copy of `source`), which is what exempts it from the read-before-write gate — a tool that mixes model-authored output into its result must not declare it. `dest` is the mutated target (`ParamRole.PATH`, the only affected path), `source` is read-only (`ParamRole.SOURCE_PATH`) and may sit under an operator-granted out-of-workspace path exactly as a `file_read` may. One regular file per call: no recursion, no globs. - Every path parameter resolves through `ToolPath.resolve` (`core:tools`), which expands a leading `~` and anchors relatives on the bound workspace's working dir. Do not re-implement path resolution in a tool. - Filesystem mutation is split by intent: `file_write` only writes (`{path, content}`), `file_edit` edits, and `file_delete` only deletes (`{path}`) — deletion is a separately-named capability so a model can never delete by getting a write-mode parameter wrong. `file_delete` shares `file_write`'s path jail and `fileWrite.enabled` toggle and carries `ToolCapability.FILE_WRITE`. - `list_dir` is shallow by default, but collapses a non-symlink single-child directory chain (bounded depth) to the first branch point and explains that expansion in its output; recursive listings retain normal tree traversal. diff --git a/infrastructure/tools/filesystem/src/main/kotlin/com/correx/infrastructure/tools/filesystem/FileCopyTool.kt b/infrastructure/tools/filesystem/src/main/kotlin/com/correx/infrastructure/tools/filesystem/FileCopyTool.kt index 136256a6..100bc001 100644 --- a/infrastructure/tools/filesystem/src/main/kotlin/com/correx/infrastructure/tools/filesystem/FileCopyTool.kt +++ b/infrastructure/tools/filesystem/src/main/kotlin/com/correx/infrastructure/tools/filesystem/FileCopyTool.kt @@ -76,7 +76,11 @@ class FileCopyTool( ) } override val tier: Tier = Tier.T2 - override val requiredCapabilities: Set = setOf(ToolCapability.FILE_WRITE) + // CONTENT_FROM_SOURCE is the provenance claim: the bytes written come entirely from the file + // named by `source`. Declared here, recorded on the invocation event, and replayed — not + // re-derived from the parameter list. + override val requiredCapabilities: Set = + setOf(ToolCapability.FILE_WRITE, ToolCapability.CONTENT_FROM_SOURCE) override val paramRoles: Map = mapOf("source" to ParamRole.SOURCE_PATH, "dest" to ParamRole.PATH) -- 2.52.0 From 496c447d9bbc31a7d555a92857bd0e6f76143457 Mon Sep 17 00:00:00 2001 From: claude Date: Thu, 27 Aug 2026 13:37:40 +0400 Subject: [PATCH 6/6] chore: ignore frontend/, the untracked Vite QA client The web-UI QA app at the repo root is a live-QA surface, not a tracked module. Without the rule every run of the experiment or the stack leaves the working tree dirty, which blocks the PR helper. Co-Authored-By: Claude Opus 5 --- .gitignore | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.gitignore b/.gitignore index 8169a110..fb9f0afe 100644 --- a/.gitignore +++ b/.gitignore @@ -82,3 +82,6 @@ apps/server/logs/ # local QA scratch workspace (nested git repo) /qa/ testing/integration/logs/ + +# web-UI QA client (untracked Vite app) +frontend/ -- 2.52.0