feat(recovery,context): tier-2 intent-holder arbiter + remaining-delta pinning + surfaced signal events

- recovery: two-tier repair ladder — owner then arbiter (recovery stage recast
  as intent-holder, holds initial intent, reconciles cross-file contract disputes)
  with independent INTENT_ROUTE_BUDGET; RecoveryRoutingTest coverage
- context: remaining-delta checklist pinning (RemainingDelta{Pinning,Entry}Test)
- surface write-only events (session name, quality signals) into stats/browse
- parseToolArguments lenient-Json fallback for bare-key drift
- tools: ChildProcess seam
This commit is contained in:
2026-07-11 23:56:52 +04:00
parent 3d5e05c1fb
commit 15248cae8a
55 changed files with 1932 additions and 231 deletions
@@ -79,7 +79,30 @@ class FileEditTool(
add(JsonPrimitive("path"))
add(JsonPrimitive("operation"))
})
// Per-operation requirements. `required` above can't express these (they depend on the
// chosen operation), so a plain schema lets a constrained decoder finish a 'replace' call
// with no target/replacement — a schema-valid but useless edit the runtime then has to
// reject. The if/then conditionals make the operand params required once the operation is
// fixed, so a schema-aware decoder emits them.
put("allOf", buildJsonArray {
add(conditionalRequirement("replace", "target", "replacement"))
add(conditionalRequirement("append", "content"))
add(conditionalRequirement("patch", "patch"))
})
}
private fun conditionalRequirement(operation: String, vararg required: String): JsonObject =
buildJsonObject {
putJsonObject("if") {
putJsonObject("properties") {
putJsonObject("operation") { put("const", operation) }
}
put("required", buildJsonArray { add(JsonPrimitive("operation")) })
}
putJsonObject("then") {
put("required", buildJsonArray { required.forEach { add(JsonPrimitive(it)) } })
}
}
override val tier: Tier = Tier.T3
override val requiredCapabilities: Set<ToolCapability> = setOf(ToolCapability.FILE_WRITE)
override val paramRoles: Map<String, ParamRole> = mapOf("path" to ParamRole.PATH)
@@ -232,27 +255,76 @@ class FileEditTool(
val currentContent = Files.readString(path)
val occurrences = currentContent.split(target).size - 1
return if (occurrences == 1) {
val newContent = currentContent.replace(target, replacement)
Files.writeString(path, newContent)
ToolResult.Success(
return when (occurrences) {
1 -> {
val newContent = currentContent.replace(target, replacement)
Files.writeString(path, newContent)
ToolResult.Success(
invocationId = request.invocationId,
output = "Target replaced in $pathString",
)
}
// Don't echo the (often whole-file) target back into context — it bloats the turn and
// teaches nothing. Instead point at the closest actual line, since a miss is almost
// always whitespace/content drift the model can correct from a nudge.
0 -> ToolResult.Failure(
invocationId = request.invocationId,
output = "Target replaced in $pathString",
)
} else {
ToolResult.Failure(
invocationId = request.invocationId,
reason = if (occurrences == 0) {
"Target '$target' not found in $pathString"
} else {
"Target '$target' found $occurrences times, expected exactly once"
reason = buildString {
append("Target not found in $pathString")
nearestLine(currentContent, target)?.let {
append(". Did you mean (whitespace/content may differ): $it ?")
}
},
// Recoverable: the model can adjust the target string and retry.
recoverable = true,
)
else -> ToolResult.Failure(
invocationId = request.invocationId,
reason = "Target found $occurrences times in $pathString, expected exactly once — " +
"extend it with surrounding lines to make it unique.",
recoverable = true,
)
}
}
/**
* Best-effort "did you mean" for a not-found replace target: anchor on the target's first
* non-blank line and return the file line most similar to it (levenshtein ratio), or null when
* nothing is close enough to be a useful suggestion. Mirrors ListDirTool's fuzzy directory hint.
*/
private fun nearestLine(content: String, target: String): String? {
val anchor = target.lineSequence().firstOrNull { it.isNotBlank() }?.trim()?.take(MAX_CMP)
if (anchor == null || anchor.length < MIN_ANCHOR) return null
return content.lineSequence()
.filter { it.isNotBlank() }
.map { it.trim() to similarity(anchor, it.trim().take(MAX_CMP)) }
.maxByOrNull { it.second }
?.takeIf { it.second >= MIN_SIMILARITY }
?.first
}
private fun similarity(a: String, b: String): Double {
val maxLen = maxOf(a.length, b.length)
return if (maxLen == 0) 1.0 else 1.0 - levenshtein(a, b).toDouble() / maxLen
}
private fun levenshtein(a: String, b: String): Int {
val dp = IntArray(b.length + 1) { it }
for (i in 1..a.length) {
var prev = dp[0]
dp[0] = i
for (j in 1..b.length) {
val tmp = dp[j]
dp[j] = minOf(
dp[j] + 1,
dp[j - 1] + 1,
prev + if (a[i - 1].lowercaseChar() == b[j - 1].lowercaseChar()) 0 else 1,
)
prev = tmp
}
}
return dp[b.length]
}
private fun patch(path: Path, pathString: String, request: ToolRequest): ToolResult {
val patchContent = request.parameters["patch"] as String
val parentDir = path.parent ?: Paths.get(".")
@@ -312,4 +384,10 @@ class FileEditTool(
recoverable = false,
)
}
private companion object {
const val MIN_ANCHOR = 3
const val MAX_CMP = 400
const val MIN_SIMILARITY = 0.5
}
}
@@ -93,7 +93,10 @@ class FileReadTool(
return runCatching {
val path = resolvePath(pathString)
if (!PathJail.isContained(path, allowedPaths))
// Widen the jail with any operator-approved out-of-workspace paths carried on the request
// (OutsidePathAccessGrantedEvent). Reads may escape once approved; writes never see this.
val effectiveRoots = allowedPaths + request.grantedPaths.map { Paths.get(it) }
if (!PathJail.isContained(path, effectiveRoots))
ValidationResult.Invalid("Path '$pathString' is not in the allowed list")
else
ValidationResult.Valid
@@ -78,7 +78,9 @@ class ListDirTool(
val pathString = (request.parameters["path"] as? String) ?: "."
return runCatching {
val path = resolvePath(pathString)
if (!PathJail.isContained(path, allowedPaths))
// Operator-approved out-of-workspace paths (OutsidePathAccessGrantedEvent) widen the jail.
val effectiveRoots = allowedPaths + request.grantedPaths.map { Paths.get(it) }
if (!PathJail.isContained(path, effectiveRoots))
ValidationResult.Invalid("Path '$pathString' is not in the allowed list")
else ValidationResult.Valid
}.getOrElse {
@@ -179,6 +179,28 @@ class FileEditToolTest {
assertTrue(failure.reason.contains("not found"))
}
@Test
fun `execute replace not-found suggests nearest line`(): Unit = runBlocking {
val tempDir = Files.createTempDirectory("file_edit_replace_suggest")
val filePath = tempDir.resolve("test.txt")
Files.writeString(filePath, "import React from 'react';\nconst greeting = 'hello world';\n")
val tool = FileEditTool(allowedPaths = setOf(tempDir))
val request = createRequest(
mapOf(
"operation" to "replace",
"path" to filePath.toString(),
// whitespace/quote drift from the real line — the classic near-miss
"target" to "const greeting = \"hello world\"",
"replacement" to "const greeting = 'hi';",
),
)
val failure = tool.execute(request) as ToolResult.Failure
assertTrue(failure.reason.contains("not found"))
assertTrue(failure.reason.contains("Did you mean"))
assertTrue(failure.reason.contains("const greeting = 'hello world';"))
}
@Test
fun `execute replace failure multiple matches`(): Unit = runBlocking {
val tempDir = Files.createTempDirectory("file_edit_replace_fail_multi")
@@ -49,6 +49,21 @@ class FileReadToolTest {
assertTrue((result as ValidationResult.Invalid).reason.contains("not in the allowed list"))
}
@Test
fun `validateRequest admits an out-of-jail path carried on request grantedPaths`(): Unit = runBlocking {
// A file the static jail does NOT cover (an unrelated temp dir), mirroring an out-of-workspace
// reference the operator approved. Without the grant it's rejected; with it, admitted.
val jailDir = Files.createTempDirectory("jail")
val outside = Files.createTempFile("outside_ref", ".md")
val tool = FileReadTool(allowedPaths = setOf(jailDir))
val ungranted = createRequest(outside.toString())
assertTrue(tool.validateRequest(ungranted) is ValidationResult.Invalid)
val granted = createRequest(outside.toString()).copy(grantedPaths = setOf(outside.toString()))
assertEquals(ValidationResult.Valid, tool.validateRequest(granted))
}
@Test
fun `validateRequest returns Invalid for missing path parameter`(): Unit = runBlocking {
val tool = FileReadTool()
@@ -14,6 +14,7 @@ import com.correx.core.tools.contract.ToolCapability
import com.correx.core.tools.contract.ToolExecutor
import com.correx.core.tools.contract.ToolResult
import com.correx.core.tools.contract.ValidationResult
import com.correx.core.tools.process.ChildProcess
import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.TimeoutCancellationException
import kotlinx.coroutines.async
@@ -83,28 +84,20 @@ class ShellTool(
}
}
// Return a rejection reason if argv is a fetch-and-execute-remote-code or interactive-scaffolder
// invocation, else null. Matched by shape, not just argv[0], because the danger for package
// managers is the *subcommand* (`npm install` is fine; `npm create`/`npm init` scaffolds and
// prompts). Deterministic, allowlist-independent — see [validateRequest].
// Return a rejection reason if argv fetches and runs an arbitrary *remote* package in one shot
// (npx/bunx/pnpx), else null. That's remote code execution — a distinct threat we still deny.
// Package-manager scaffolders (`npm create`, `npm init`, `npm install`) are NO LONGER blocked:
// the hang they used to guard against (stdin prompt with no TTY) is now closed at the process
// factory — every child gets stdin from /dev/null and a non-interactive env overlay
// ([ChildProcess]). Matched by shape, not just argv[0], because a runner can hide inside a shell
// command line run via `sh -c` (`cd web && npx create-foo`), where argv[0] is `cd`.
private fun deniedReason(argv: List<String>): String? {
// Scan every token, not just argv[0]: a scaffolder can hide inside a shell command line that
// runs via `sh -c` (`cd web && npm create vite`), where argv[0] is `cd`.
// Scan every token, not just argv[0]: a runner can hide behind a shell builtin at argv[0].
val progs = argv.map { it.substringAfterLast('/') }
val redirect = "Scaffold by writing files directly with file_write (package.json, configs, " +
"sources) — do not shell out to a network installer; it needs a TTY and will hang unattended."
val runner = progs.firstOrNull { it in REMOTE_EXEC_RUNNERS }
val scaffolder = progs.firstOrNull { it.startsWith("create-") }
val pmIdx = progs.indexOfFirst { it in PACKAGE_MANAGERS }
return when {
runner != null ->
"'$runner' fetches and runs remote code with no TTY, which hangs an unattended run. $redirect"
scaffolder != null ->
"'$scaffolder' is an interactive project scaffolder that prompts on stdin. $redirect"
pmIdx >= 0 && progs.getOrNull(pmIdx + 1) in SCAFFOLD_SUBCOMMANDS ->
"'${progs[pmIdx]} ${progs[pmIdx + 1]}' scaffolds a project interactively " +
"(prompts on stdin). $redirect"
else -> null
return progs.firstOrNull { it in REMOTE_EXEC_RUNNERS }?.let { runner ->
"'$runner' downloads and runs an arbitrary remote package in one shot (remote code " +
"execution). Use the package manager's own scaffold command instead — e.g. " +
"`npm create vite@latest <dir> -- --template react-ts` — or write the files with file_write."
}
}
@@ -170,7 +163,7 @@ class ShellTool(
// shell API. A still-unrunnable program returns RECOVERABLE (model adapts, not fatal).
val command = if (looksLikeShellCommand(argv)) listOf("sh", "-c", argv.joinToString(" ")) else argv
val process = runCatching {
ProcessBuilder(command).apply { workingDir?.let { directory(it.toFile()) } }.start()
ChildProcess.builder(command, workingDir?.toFile()).start()
}.getOrElse {
return@let ToolResult.Failure(
invocationId = request.invocationId,
@@ -213,10 +206,29 @@ class ShellTool(
val SHELL_OPERATORS = setOf("&&", "||", "|", ">", ">>", "<", ";", "&", "2>", "2>&1")
// Runners that download and execute an arbitrary remote package in one shot.
val REMOTE_EXEC_RUNNERS = setOf("npx", "bunx", "pnpx")
val PACKAGE_MANAGERS = setOf("npm", "yarn", "pnpm", "bun")
// Package-manager subcommands that generate a project by pulling+running a remote scaffolder
// and prompting on stdin (npm create vite, yarn create next-app, pnpm dlx, …).
val SCAFFOLD_SUBCOMMANDS = setOf("create", "init", "dlx")
}
// Combines a failed process's stdout and stderr into one labelled, head/tail-bounded block for
// the model's failure feedback. Labels each stream (only when non-blank) so the model can tell
// compiler diagnostics on stdout from runner errors on stderr; the same HEAD/TAIL window as the
// success-path compressor keeps a large build log from flooding the next context window.
private fun boundFailureOutput(stdout: String, stderr: String): String {
val sections = buildList {
if (stdout.isNotBlank()) add("stdout:\n${stdout.trimEnd()}")
if (stderr.isNotBlank()) add("stderr:\n${stderr.trimEnd()}")
}
return boundLines(sections.joinToString("\n\n"))
}
// Keeps the first HEAD_LINES and last TAIL_LINES of a multi-line block, dropping the middle with
// an elision marker — build errors cluster at both ends (first failures, final summary), so a
// head+tail window preserves the actionable lines while bounding size.
private fun boundLines(text: String): String {
val lines = text.lines()
if (lines.size <= HEAD_LINES + TAIL_LINES) return text
val omitted = lines.size - HEAD_LINES - TAIL_LINES
return (lines.take(HEAD_LINES) + "… ($omitted lines omitted) …" + lines.takeLast(TAIL_LINES))
.joinToString("\n")
}
private suspend fun runCmd(request: ToolRequest, process: Process): ToolResult = withTimeout(timeoutMs) {
@@ -230,11 +242,18 @@ class ShellTool(
val metadata = if (stderr.isNotBlank()) mapOf("stderr" to stderr) else emptyMap()
if (exitCode != 0) {
// A non-zero exit is normal command behaviour (missing path, no grep match, git
// --exit-code, test, diff). Surface it recoverably so the model sees the code + stderr
// --exit-code, test, diff). Surface it recoverably so the model sees the code + output
// and adapts, rather than aborting the stage on the first failed probe.
//
// BOTH streams, not just stderr: compilers and test runners (tsc, vite, jest, pytest…)
// print their diagnostics to STDOUT, so a stderr-only failure message returns a bare
// "exited with code 2" and the model fixes blind — it can see the build failed but not
// which file/line. Combine stdout+stderr, head/tail-bounded so a large build log can't
// flood the next context window.
val detail = boundFailureOutput(stdout, stderr)
ToolResult.Failure(
invocationId = request.invocationId,
reason = "Process exited with code $exitCode${if (stderr.isNotBlank()) ": $stderr" else ""}",
reason = "Process exited with code $exitCode${if (detail.isNotBlank()) ":\n$detail" else ""}",
recoverable = true,
)
} else {
@@ -102,6 +102,24 @@ class ShellToolTest {
assertTrue(failure.recoverable, "non-zero exit must be recoverable so the model can adapt")
}
@Test
fun `execute surfaces stdout in the failure reason for a non-zero exit`(): Unit = runBlocking {
// Compilers/test runners (tsc, vite, jest) print diagnostics to STDOUT and exit non-zero.
// The failure reason must carry that output, else the model sees only "exited with code 2"
// and fixes blind. Regression for session 6f9a9f08 recovery loop.
val tool = ShellTool()
val request = createRequest(listOf("sh", "-c", "echo 'hooks.ts(57,3): error TS1005'; exit 2"))
val result = tool.execute(request)
assertTrue(result is ToolResult.Failure)
val failure = result as ToolResult.Failure
assertTrue(failure.reason.contains("exited with code 2"))
assertTrue(
failure.reason.contains("TS1005"),
"stdout diagnostics must reach the model's failure feedback, got: ${failure.reason}",
)
}
@Test
fun `execute returns Failure on timeout`(): Unit = runBlocking {
// Using sleep command to simulate timeout
@@ -182,15 +200,12 @@ class ShellToolTest {
}
@Test
fun `denylist blocks npm create even in allow-all mode`() {
// Repro: gemma ran `npm create vite@latest` — a network scaffolder that prompts on stdin.
// Empty allowedExecutables = allow-all, yet this must still be rejected.
fun `denylist allows npm create — hang closed at the process factory`() {
// `npm create vite@latest` used to be blocked as an interactive scaffolder. The stdin-prompt
// hang is now closed at ChildProcess (stdin=/dev/null + non-interactive env), so the idiomatic
// scaffold path is allowed again. Empty allowedExecutables = allow-all.
val tool = ShellTool(allowedExecutables = emptySet())
val result = tool.validateRequest(createRequest(listOf("npm", "create", "vite")))
assertTrue(result is ValidationResult.Invalid)
val reason = (result as ValidationResult.Invalid).reason
assertTrue(reason.contains("npm create"), reason)
assertTrue(reason.contains("file_write"), reason)
assertEquals(ValidationResult.Valid, tool.validateRequest(createRequest(listOf("npm", "create", "vite"))))
}
@Test
@@ -201,32 +216,24 @@ class ShellToolTest {
}
@Test
fun `denylist blocks create- scaffolder run directly`() {
val result = ShellTool().validateRequest(createRequest(listOf("create-next-app", "app")))
fun `denylist catches remote runner hidden in a shell command line`() {
// argv[0] is `cd`, but `npx` rides along and would run via sh -c.
val result = ShellTool().validateRequest(createRequest(listOf("cd", "web", "&&", "npx", "create-foo")))
assertTrue(result is ValidationResult.Invalid)
assertTrue((result as ValidationResult.Invalid).reason.contains("scaffolder"), result.reason)
}
@Test
fun `denylist catches scaffolder hidden in a shell command line`() {
// argv[0] is `cd`, but `npm create` rides along and would run via sh -c.
val result = ShellTool().validateRequest(createRequest(listOf("cd", "web", "&&", "npm", "create", "vite")))
assertTrue(result is ValidationResult.Invalid)
assertTrue((result as ValidationResult.Invalid).reason.contains("npm create"), result.reason)
assertTrue((result as ValidationResult.Invalid).reason.contains("remote code"), result.reason)
}
@Test
fun `denylist allows npm install`() {
// The subcommand is what's dangerous — `npm install` on existing files is fine.
assertEquals(ValidationResult.Valid, ShellTool().validateRequest(createRequest(listOf("npm", "install"))))
}
@Test
fun `denylist takes precedence over the allowlist`() {
// Even if npm is explicitly allowed, `npm create` stays blocked.
val tool = ShellTool(allowedExecutables = setOf("npm"))
val result = tool.validateRequest(createRequest(listOf("npm", "create", "vite")))
fun `denylist takes precedence over the allowlist for remote runners`() {
// Even if npx is explicitly allowed, remote-exec stays blocked.
val tool = ShellTool(allowedExecutables = setOf("npx"))
val result = tool.validateRequest(createRequest(listOf("npx", "create-react-app", "app")))
assertTrue(result is ValidationResult.Invalid)
assertTrue((result as ValidationResult.Invalid).reason.contains("file_write"), result.reason)
assertTrue((result as ValidationResult.Invalid).reason.contains("remote code"), result.reason)
}
}
@@ -27,9 +27,25 @@ private const val RECOVERY_PROMPT =
"You are the recovery stage. An upstream stage failed a verification gate it could not fix " +
"(it lacked write access). The failure ticket — the failing stage, the gate, and its exact " +
"output — is in your context under \"## Recovery ticket\". Read the gate output, inspect the " +
"relevant files (file_read / list_dir), and make the minimal edits that fix the reported " +
"cause. Do not re-scaffold or rewrite working code. When done, control returns to the stage " +
"that failed so its gate re-runs."
"relevant files (file_read / list_dir), then FIX the reported cause. You MUST make the " +
"change before finishing — investigation alone does not resolve the ticket, and handing " +
"back without an edit only re-triggers the same failure.\n" +
"Fix the actual cause the gate reports, whatever form it takes:\n" +
"- Broken code / config → edit it with file_write.\n" +
"- A MISSING file the build needs (e.g. package.json, tsconfig.json, a build/entry file " +
"the gate can't find) → produce it. A build gate failing with \"no such file\" is fixed by " +
"creating that file. If a whole project skeleton is missing, prefer the package manager's " +
"own generator via shell — e.g. `npm create vite@latest <dir> -- --template react-ts` — " +
"which writes a correct, complete file set (always pass the template/preset arg it " +
"requires; the run is non-interactive so there's no prompt to fall back on). Fall back to " +
"writing the files directly with file_write when no generator fits. `npm install` is " +
"available to fetch deps once package.json exists. Only bare remote runners (`npx`, " +
"`bunx`, `pnpx`) are blocked; use `npm create` instead.\n" +
"Do not gratuitously rewrite code that already works — touch only what the gate blames. But " +
"\"minimal\" means minimal to actually pass the gate, not zero.\n" +
"When the cause is repaired, finish with a short resolution summary that mirrors the ticket: " +
"the failing stage and gate, the root cause you found, and the exact files created/edited " +
"and commands run. Then control returns to the stage that failed so its gate re-runs."
// Freestyle plans don't specify a per-stage token budget, so without this every compiled stage
// inherits StageConfig's 4096 default — 1/8th of what the static role_pipeline execution stages
@@ -118,7 +134,13 @@ class ExecutionPlanCompiler(
StageId(s.id) to StageConfig(
produces = listOf(TypedArtifactSlot(name = ArtifactId(s.produces), kind = kind)),
needs = s.needs.map { ArtifactId(it) }.toSet(),
allowedTools = s.tools.toSet(),
// Every stage gets the full registered tool universe. The architect's per-stage
// `tools` pick was advisory and mis-scoped stages (e.g. an edit-shaped stage granted
// file_write but not file_edit → forced full-file rewrites; a repair stage unable to
// touch a file it must fix). writeManifest still jails WHICH paths a stage may write,
// so widening the tool grant does not widen write authority. Falls back to the
// architect's pick only when no universe was supplied (bare-constructor unit tests).
allowedTools = knownTools.ifEmpty { s.tools.toSet() },
writeManifest = writeManifest,
// Option A: the concrete (non-glob) subset of the declared writes are expected files
// the contract gate will require to exist. Globs (src/**) are write-permission scopes,
@@ -144,7 +166,7 @@ class ExecutionPlanCompiler(
null
} else {
StageId(RECOVERY_STAGE) to StageConfig(
allowedTools = setOf("file_write", "shell"),
allowedTools = knownTools.ifEmpty { setOf("file_write", "file_edit", "shell") },
tokenBudget = DEFAULT_STAGE_TOKEN_BUDGET,
generationConfig = DEFAULT_STAGE_GENERATION,
metadata = mapOf("role" to "recovery", "promptInline" to RECOVERY_PROMPT),