fix(tools): file_write to a directory is recoverable, not fatal
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.
This commit is contained in:
+19
-2
@@ -135,6 +135,20 @@ class FileWriteTool(
|
|||||||
val pathString = request.parameters["path"] as String
|
val pathString = request.parameters["path"] as String
|
||||||
val content = request.parameters["content"] as String
|
val content = request.parameters["content"] as String
|
||||||
val path = resolvePath(pathString)
|
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/<filename>').",
|
||||||
|
recoverable = true,
|
||||||
|
)
|
||||||
|
}
|
||||||
|
|
||||||
val originalContent = runCatching { Files.readString(path) }.getOrDefault("")
|
val originalContent = runCatching { Files.readString(path) }.getOrDefault("")
|
||||||
|
|
||||||
val result = runCatching {
|
val result = runCatching {
|
||||||
@@ -168,10 +182,13 @@ class FileWriteTool(
|
|||||||
pathString: String,
|
pathString: String,
|
||||||
): ToolResult = when (e) {
|
): ToolResult = when (e) {
|
||||||
is CancellationException -> throw 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(
|
is IOException -> ToolResult.Failure(
|
||||||
invocationId = invocationId,
|
invocationId = invocationId,
|
||||||
reason = "IO error: ${e.message}",
|
reason = "IO error writing '$pathString': ${e.message ?: e.javaClass.simpleName}",
|
||||||
recoverable = false,
|
recoverable = true,
|
||||||
)
|
)
|
||||||
|
|
||||||
is SecurityException -> ToolResult.Failure(
|
is SecurityException -> ToolResult.Failure(
|
||||||
|
|||||||
+13
@@ -37,6 +37,19 @@ class FileWriteToolTest {
|
|||||||
assertEquals(ValidationResult.Valid, tool.validateRequest(request))
|
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
|
@Test
|
||||||
fun `validateRequest returns Invalid for disallowed path`(): Unit = runBlocking {
|
fun `validateRequest returns Invalid for disallowed path`(): Unit = runBlocking {
|
||||||
val tempDir = Files.createTempDirectory("file_write_test")
|
val tempDir = Files.createTempDirectory("file_write_test")
|
||||||
|
|||||||
Reference in New Issue
Block a user