From c1de2281c81298f070774fa877c3572ec199d1e0 Mon Sep 17 00:00:00 2001 From: kami Date: Thu, 2 Jul 2026 12:23:29 +0400 Subject: [PATCH] fix(tools): file_write to a directory is recoverable, not fatal MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Writing to an existing directory path threw an IOException marked recoverable=false, which the orchestrator turns into a FATAL non-retryable stage failure — dead-ending the workflow on a correctable model mistake. Guard the directory case up front with a clear message and make write IO errors recoverable so the retry-with-feedback loop can self-correct. --- .../tools/filesystem/FileWriteTool.kt | 21 +++++++++++++++++-- .../tools/filesystem/FileWriteToolTest.kt | 13 ++++++++++++ 2 files changed, 32 insertions(+), 2 deletions(-) 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 6079f8a2..b6068e32 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 @@ -135,6 +135,20 @@ class FileWriteTool( val pathString = request.parameters["path"] as String val content = request.parameters["content"] as String val path = resolvePath(pathString) + + // A path that names an existing directory is a correctable model mistake (it meant a file + // inside it), not a fatal IO error. Caught here it flows back as a recoverable ERROR the + // model can fix on the next round, instead of the atomic move throwing a bare-path + // IOException that aborts the whole stage non-retryably. + if (Files.isDirectory(path)) { + return@withContext ToolResult.Failure( + invocationId = request.invocationId, + reason = "Path '$pathString' is a directory — write to a file inside it instead " + + "(e.g. '$pathString/').", + recoverable = true, + ) + } + val originalContent = runCatching { Files.readString(path) }.getOrDefault("") val result = runCatching { @@ -168,10 +182,13 @@ class FileWriteTool( pathString: String, ): ToolResult = when (e) { is CancellationException -> throw e + // Write IO errors are recoverable: the model can correct the path/content and retry within + // the stage's tool loop (bounded by MAX_TOOL_ROUNDS). Marking them fatal turned a fixable + // mistake — e.g. writing to a directory — into a terminal workflow failure. is IOException -> ToolResult.Failure( invocationId = invocationId, - reason = "IO error: ${e.message}", - recoverable = false, + reason = "IO error writing '$pathString': ${e.message ?: e.javaClass.simpleName}", + recoverable = true, ) is SecurityException -> ToolResult.Failure( diff --git a/infrastructure/tools/filesystem/src/test/kotlin/com/correx/infrastructure/tools/filesystem/FileWriteToolTest.kt b/infrastructure/tools/filesystem/src/test/kotlin/com/correx/infrastructure/tools/filesystem/FileWriteToolTest.kt index 2a99fa34..db1aa6e3 100644 --- a/infrastructure/tools/filesystem/src/test/kotlin/com/correx/infrastructure/tools/filesystem/FileWriteToolTest.kt +++ b/infrastructure/tools/filesystem/src/test/kotlin/com/correx/infrastructure/tools/filesystem/FileWriteToolTest.kt @@ -37,6 +37,19 @@ class FileWriteToolTest { assertEquals(ValidationResult.Valid, tool.validateRequest(request)) } + @Test + fun `execute fails recoverably when path is a directory`(): Unit = runBlocking { + val tempDir = Files.createTempDirectory("file_write_test") + val subDir = Files.createDirectory(tempDir.resolve("views")) + val tool = FileWriteTool(allowedPaths = setOf(tempDir)) + val result = tool.execute(createRequest(mapOf("path" to subDir.toString(), "content" to "x"))) + assertTrue(result is ToolResult.Failure) + result as ToolResult.Failure + // Recoverable so the stage's tool loop feeds it back instead of dead-ending the workflow. + assertTrue(result.recoverable) + assertTrue(result.reason.contains("is a directory")) + } + @Test fun `validateRequest returns Invalid for disallowed path`(): Unit = runBlocking { val tempDir = Files.createTempDirectory("file_write_test")