fix(events,tools,toolintent): failure attribution, one path normalization rule, file_copy (#713)
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 `<workspace>/~/.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 <noreply@anthropic.com>
This commit is contained in:
@@ -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.
|
||||
|
||||
|
||||
+205
@@ -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<Path> = emptySet(),
|
||||
private val workingDir: Path? = null,
|
||||
) : Tool, FileAffectingTool, ToolExecutor {
|
||||
|
||||
private val normalizedAllowedPaths: Set<Path> = 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<ToolCapability> = setOf(ToolCapability.FILE_WRITE)
|
||||
override val paramRoles: Map<String, ParamRole> =
|
||||
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<Path> {
|
||||
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": "<existing path>", "dest": "<path to copy to>"}.""",
|
||||
)
|
||||
val dest = request.parameters["dest"] as? String
|
||||
?: return ValidationResult.Invalid(
|
||||
"""Missing 'dest' parameter (string). Call file_copy with {"source": "$source", "dest": "<path>"}.""",
|
||||
)
|
||||
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,
|
||||
)
|
||||
}
|
||||
}
|
||||
+5
-7
@@ -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 {
|
||||
|
||||
+5
-6
@@ -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<Path> = 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"
|
||||
|
||||
+5
-6
@@ -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"
|
||||
|
||||
+5
-7
@@ -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 {
|
||||
|
||||
+5
-6
@@ -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"
|
||||
|
||||
+2
-6
@@ -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<Path>, workingDir: Path?): ValidationResult =
|
||||
|
||||
+162
@@ -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<String, String>, grantedPaths: Set<String> = 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'"))
|
||||
}
|
||||
}
|
||||
+18
@@ -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)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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<Tool> = 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(
|
||||
|
||||
Reference in New Issue
Block a user