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 7bd27e3b..ad3a0a86 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 @@ -364,18 +364,6 @@ internal fun SessionOrchestrator.sessionProducedBuildTarget(sessionId: SessionId KindContractTable.assertionsFor(kind, path).any { it.id == "imports_resolve" } } -internal fun SessionOrchestrator.stageWrittenPaths(sessionId: SessionId, stageId: StageId): List { - val events = eventStore.read(sessionId) - val invocationIds = events.mapNotNull { it.payload as? ToolInvocationRequestedEvent } - .filter { it.stageId == stageId } - .map { it.invocationId } - .toSet() - return events.mapNotNull { it.payload as? FileWrittenEvent } - .filter { it.invocationId in invocationIds && it.postImageHash != null } - .map { it.path } - .distinct() -} - /** * Static-first reviewer gate (role-reliability §5): for a stage that declares `static_analysis` * commands, run them (compiler / detekt / formatters) against its just-produced output in the diff --git a/core/kernel/src/main/kotlin/com/correx/core/kernel/orchestration/StageWrittenPaths.kt b/core/kernel/src/main/kotlin/com/correx/core/kernel/orchestration/StageWrittenPaths.kt new file mode 100644 index 00000000..20c1e3f9 --- /dev/null +++ b/core/kernel/src/main/kotlin/com/correx/core/kernel/orchestration/StageWrittenPaths.kt @@ -0,0 +1,33 @@ +package com.correx.core.kernel.orchestration + +import com.correx.core.events.events.FileWrittenEvent +import com.correx.core.events.events.StoredEvent +import com.correx.core.events.events.ToolInvocationRequestedEvent +import com.correx.core.events.types.SessionId +import com.correx.core.events.types.StageId + +internal fun SessionOrchestrator.stageWrittenPaths(sessionId: SessionId, stageId: StageId): List = + stageWrittenPathsFrom(eventStore.read(sessionId), stageId) + +/** + * The files [stageId] wrote that still exist — the stage's live output manifest. + * + * Keeps only paths whose LAST mutation still has content. A deletion is a [FileWrittenEvent] with a + * null `postImageHash`, so filtering per-event rather than per-path leaves a written-then-deleted file + * in the manifest forever. Callers treat the manifest as ground truth: the contract gate stamps + * `file_exists` on every entry, which makes deleting — or renaming, which is delete plus write — a + * permanent contract violation, deadlocking against a build gate that demands one (observed live: + * "rename it to use the '.cjs' file extension" against `file_exists` on the `.js`). + */ +internal fun stageWrittenPathsFrom(events: List, stageId: StageId): List { + val invocationIds = events.mapNotNull { it.payload as? ToolInvocationRequestedEvent } + .filter { it.stageId == stageId } + .map { it.invocationId } + .toSet() + return events.mapNotNull { it.payload as? FileWrittenEvent } + .filter { it.invocationId in invocationIds } + .associateBy { it.path } // last write per path wins + .filterValues { it.postImageHash != null } + .keys + .toList() +} diff --git a/core/kernel/src/test/kotlin/com/correx/core/kernel/orchestration/StageWrittenPathsTest.kt b/core/kernel/src/test/kotlin/com/correx/core/kernel/orchestration/StageWrittenPathsTest.kt new file mode 100644 index 00000000..4db32271 --- /dev/null +++ b/core/kernel/src/test/kotlin/com/correx/core/kernel/orchestration/StageWrittenPathsTest.kt @@ -0,0 +1,155 @@ +package com.correx.core.kernel.orchestration + +import com.correx.core.approvals.Tier +import com.correx.core.events.events.EventMetadata +import com.correx.core.events.events.EventPayload +import com.correx.core.events.events.FileWrittenEvent +import com.correx.core.events.events.StoredEvent +import com.correx.core.events.events.ToolInvocationRequestedEvent +import com.correx.core.events.events.ToolRequest +import com.correx.core.events.types.EventId +import com.correx.core.events.types.SessionId +import com.correx.core.events.types.StageId +import com.correx.core.events.types.ToolInvocationId +import kotlinx.datetime.Instant +import org.junit.jupiter.api.Assertions.assertEquals +import org.junit.jupiter.api.Assertions.assertTrue +import org.junit.jupiter.api.Test + +/** + * The stage output manifest must track deletions. Callers treat it as ground truth — the contract gate + * stamps `file_exists` on every entry — so a written-then-deleted path that survives here makes + * deleting, and therefore renaming, a permanent contract violation. Live session fced377e deadlocked + * exactly there: the build gate ordered "rename it to use the '.cjs' file extension", the agent + * complied, and the contract gate then failed `file_exists` on the `.js` it had just been told to + * remove — 89 turns without converging. + */ +class StageWrittenPathsTest { + + private val stage = StageId("scaffold_frontend") + private val other = StageId("review_ui") + + @Test + fun `a written-then-deleted path drops out of the manifest`() { + val events = listOf( + invoked("inv1", stage), + wrote("inv1", "frontend/postcss.config.js", "h1"), + invoked("inv2", stage), + deleted("inv2", "frontend/postcss.config.js"), + ) + assertEquals(emptyList(), stageWrittenPathsFrom(events, stage)) + } + + @Test + fun `a rename leaves only the new path`() { + val events = listOf( + invoked("inv1", stage), + wrote("inv1", "frontend/postcss.config.js", "h1"), + invoked("inv2", stage), + wrote("inv2", "frontend/postcss.config.cjs", "h1"), + invoked("inv3", stage), + deleted("inv3", "frontend/postcss.config.js"), + ) + assertEquals(listOf("frontend/postcss.config.cjs"), stageWrittenPathsFrom(events, stage)) + } + + @Test + fun `a path deleted and then rewritten is back in the manifest`() { + val events = listOf( + invoked("inv1", stage), + wrote("inv1", "frontend/vite.config.ts", "h1"), + invoked("inv2", stage), + deleted("inv2", "frontend/vite.config.ts"), + invoked("inv3", stage), + wrote("inv3", "frontend/vite.config.ts", "h2"), + ) + assertEquals(listOf("frontend/vite.config.ts"), stageWrittenPathsFrom(events, stage)) + } + + @Test + fun `surviving writes are unaffected by a sibling deletion`() { + val events = listOf( + invoked("inv1", stage), + wrote("inv1", "frontend/src/App.tsx", "h1"), + invoked("inv2", stage), + wrote("inv2", "frontend/tailwind.config.js", "h2"), + invoked("inv3", stage), + deleted("inv3", "frontend/src/App.css"), + ) + assertEquals( + listOf("frontend/src/App.tsx", "frontend/tailwind.config.js"), + stageWrittenPathsFrom(events, stage), + ) + } + + @Test + fun `another stage's writes stay out of this stage's manifest`() { + val events = listOf( + invoked("inv1", stage), + wrote("inv1", "frontend/src/App.tsx", "h1"), + invoked("inv2", other), + wrote("inv2", "frontend/src/Sessions.tsx", "h2"), + ) + assertEquals(listOf("frontend/src/App.tsx"), stageWrittenPathsFrom(events, stage)) + assertTrue(stageWrittenPathsFrom(events, other) == listOf("frontend/src/Sessions.tsx")) + } + + private var seq = 0L + + private fun stored(payload: EventPayload): StoredEvent { + seq++ + return StoredEvent( + metadata = EventMetadata( + eventId = EventId("e$seq"), + sessionId = SessionId("s1"), + timestamp = Instant.parse("2026-01-01T00:00:00Z"), + schemaVersion = 1, + causationId = null, + correlationId = null, + ), + sequence = seq, + sessionSequence = seq, + payload = payload, + ) + } + + private fun invoked(invocationId: String, stageId: StageId) = stored( + ToolInvocationRequestedEvent( + invocationId = ToolInvocationId(invocationId), + sessionId = SessionId("s1"), + stageId = stageId, + toolName = "file_write", + tier = Tier.T3, + request = ToolRequest( + invocationId = ToolInvocationId(invocationId), + sessionId = SessionId("s1"), + stageId = stageId, + toolName = "file_write", + parameters = emptyMap(), + ), + ), + ) + + private fun wrote(invocationId: String, path: String, hash: String) = stored( + FileWrittenEvent( + invocationId = ToolInvocationId(invocationId), + sessionId = SessionId("s1"), + path = path, + postImageHash = hash, + preExisted = false, + timestampMs = 1L, + ), + ) + + /** A deletion is a [FileWrittenEvent] with no post-image. */ + private fun deleted(invocationId: String, path: String) = stored( + FileWrittenEvent( + invocationId = ToolInvocationId(invocationId), + sessionId = SessionId("s1"), + path = path, + postImageHash = null, + preExisted = true, + timestampMs = 1L, + ), + ) +} 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 57e71423..9d361449 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 @@ -201,20 +201,30 @@ class FileEditTool( } /** - * Locate [target] in [content] ignoring each line's leading/trailing whitespace — small models - * routinely drop or misjudge indentation, so an exact-string miss is almost always an indent - * mismatch, not a wrong edit. Returns the matched file-line range iff exactly one block matches. - * ponytail: line-trim match only; mixed tab/space or a target spanning blank-line drift may miss. + * Locate [target] in [content] ignoring each line's leading/trailing whitespace AND blank lines — + * small models reconstruct the target from memory, so an exact-string miss is almost always + * indentation drift or a dropped blank line, not a wrong edit. Comparing only the non-blank lines + * survives both: blank-line drift shifts every later index, so a positional walk over raw lines + * misses the whole block over one absent empty line (observed live on vite.config.ts, main.tsx, + * index.css). Returns the matched file-line range iff exactly one block matches; interior blank + * lines of the file fall inside the range and are consumed by the replacement, which is the + * intent — the caller sent a replacement for that whole block. + * ponytail: mixed tab/space inside a line still has to match after trim(). */ private fun flexibleMatch(content: String, target: String): IntRange? { val fileLines = content.split("\n") - val targetLines = target.split("\n").dropLastWhile { it.isBlank() } - if (targetLines.isEmpty() || targetLines.size > fileLines.size) return null - val normTarget = targetLines.map { it.trim() } - val starts = (0..fileLines.size - targetLines.size).filter { start -> - normTarget.indices.all { fileLines[start + it].trim() == normTarget[it] } + // (originalIndex, trimmed) for the file's non-blank lines only. + val fileNonBlank = fileLines.withIndex().filter { it.value.isNotBlank() } + .map { it.index to it.value.trim() } + val normTarget = target.split("\n").filter { it.isNotBlank() }.map { it.trim() } + if (normTarget.isEmpty() || normTarget.size > fileNonBlank.size) return null + val hits = (0..fileNonBlank.size - normTarget.size).filter { start -> + normTarget.indices.all { fileNonBlank[start + it].second == normTarget[it] } } - return if (starts.size == 1) starts[0] until (starts[0] + targetLines.size) else null + if (hits.size != 1) return null + val start = fileNonBlank[hits[0]].first + val end = fileNonBlank[hits[0] + normTarget.size - 1].first + return start..end } /** Rebase [replacement]'s indentation onto [baseIndent], preserving its own relative structure. */ diff --git a/infrastructure/tools/filesystem/src/test/kotlin/com/correx/infrastructure/tools/filesystem/FileEditToolTest.kt b/infrastructure/tools/filesystem/src/test/kotlin/com/correx/infrastructure/tools/filesystem/FileEditToolTest.kt index 6e5d9fd0..652468cf 100644 --- a/infrastructure/tools/filesystem/src/test/kotlin/com/correx/infrastructure/tools/filesystem/FileEditToolTest.kt +++ b/infrastructure/tools/filesystem/src/test/kotlin/com/correx/infrastructure/tools/filesystem/FileEditToolTest.kt @@ -216,6 +216,75 @@ class FileEditToolTest { ) } + @Test + fun `replace tolerates a dropped blank line in the target`(): Unit = runBlocking { + // Verbatim from live session fced377e: the model reconstructed vite.config.ts from memory and + // omitted the blank line before the comment. A positional walk over raw lines shifts every + // later index and misses the whole block, so 4-of-6 file_edit calls failed on drift like this. + val tempDir = Files.createTempDirectory("file_edit_blankline") + val filePath = tempDir.resolve("vite.config.ts") + Files.writeString( + filePath, + "import { defineConfig } from 'vite'\n" + + "import react from '@vitejs/plugin-react'\n" + + "\n" + // the blank line the model dropped + "// https://vite.dev/config/\n" + + "export default defineConfig({\n" + + " plugins: [react()],\n" + + "})\n", + ) + val tool = FileEditTool(allowedPaths = setOf(tempDir)) + val request = createRequest( + mapOf( + "operation" to "replace", + "path" to filePath.toString(), + "target" to "import { defineConfig } from 'vite'\n" + + "import react from '@vitejs/plugin-react'\n" + + "// https://vite.dev/config/\n" + + "export default defineConfig({\n" + + " plugins: [react()],\n" + + "})", + "replacement" to "import { defineConfig } from 'vite'\n" + + "import react from '@vitejs/plugin-react'\n" + + "export default defineConfig({\n" + + " plugins: [react()],\n" + + " server: { port: 5173 },\n" + + "})", + ), + ) + + val result = tool.execute(request) + assertTrue(result is ToolResult.Success, "blank-line-drift replace should succeed") + assertTrue( + Files.readString(filePath).contains("server: { port: 5173 }"), + "the replacement should have been applied", + ) + } + + @Test + fun `replace still refuses a target that is ambiguous once blank lines are ignored`(): Unit = runBlocking { + // Ignoring blank lines must not turn a genuinely ambiguous edit into a silent wrong one. + val tempDir = Files.createTempDirectory("file_edit_blank_ambiguous") + val filePath = tempDir.resolve("dup.ts") + // Both sites are blank-separated, so there is no exact match to short-circuit on — the + // blank-line-insensitive walk is what has to reject this. + Files.writeString(filePath, "call()\n\nother()\n\ncall()\n\nother()\n") + val tool = FileEditTool(allowedPaths = setOf(tempDir)) + val request = createRequest( + mapOf( + "operation" to "replace", + "path" to filePath.toString(), + "target" to "call()\nother()", + "replacement" to "call2()\nother2()", + ), + ) + + assertTrue( + tool.validateRequest(request) is ValidationResult.Invalid, + "two blank-line-normalized matches must stay ambiguous, not pick one", + ) + } + @Test fun `replace accepts content as an alias for replacement`(): Unit = runBlocking { val tempDir = Files.createTempDirectory("file_edit_alias")