From 95b16a5047f7f60f0ad9362376cee6e1fe6594e7 Mon Sep 17 00:00:00 2001 From: kami Date: Sun, 19 Jul 2026 22:33:01 +0400 Subject: [PATCH] fix(weak-model-gates): stop three gates from stalling weak stage models MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Diagnosed from session 508c8d58 (frontend freestyle run, WorkflowFailed): three independent weak-model-hostile gates, none a model-capability problem. - shell: split a collapsed single-string command line (["npm create vite …"]) into tokens instead of rejecting it as a "collapsed array". The model reliably re-emits this shape; rejecting looped bootstrap_frontend until stage_loop_break. JSON-escape mangles (quotes/commas in argv[0]) stay rejected. - recovery: a stage_loop_break route now gets a "Stuck-loop ticket", not the "Contract arbitration ticket". The arbitration prompt told the model to read and reconcile "the files named below" — but a tool-syntax loop names zero files, sending the recovery agent grepping the repo for 40+ turns until the repair ladder exhausted. - plan lint: H3 (unreferenced_prompt_artifact) demoted from hard failure to soft finding, and seeds excluded from it. It word-matches artifact IDs in free prose and cannot tell a forgotten dep from a descriptive mention, so as a hard gate it burned architect retries on words it couldn't reword away (analysis/dod). Hard tier is now deterministic graph facts only (H1/H2). Tests: ShellToolTest, PlanLinterTest, RecoveryRoutingTest green; detekt clean. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_015Ly2mMnt9TCZbvhcC1JfuV --- .../kernel/orchestration/ContextFeedback.kt | 18 ++++++++++-- .../infrastructure/tools/shell/ShellTool.kt | 26 +++++++++++++---- .../tools/shell/ShellToolTest.kt | 9 ++++++ .../infrastructure/workflow/PlanLinter.kt | 28 +++++++++++++------ .../infrastructure/workflow/PlanLinterTest.kt | 22 +++++++++++++-- 5 files changed, 84 insertions(+), 19 deletions(-) diff --git a/core/kernel/src/main/kotlin/com/correx/core/kernel/orchestration/ContextFeedback.kt b/core/kernel/src/main/kotlin/com/correx/core/kernel/orchestration/ContextFeedback.kt index 278bc272..bf1d5ef9 100644 --- a/core/kernel/src/main/kotlin/com/correx/core/kernel/orchestration/ContextFeedback.kt +++ b/core/kernel/src/main/kotlin/com/correx/core/kernel/orchestration/ContextFeedback.kt @@ -82,10 +82,24 @@ fun buildRecoveryTicketEntry(events: List, stageId: StageId): Conte val ticket = events .mapNotNull { it.payload as? FailureTicketOpenedEvent } .lastOrNull { it.routeTo == stageId } ?: return null - val content = if (ticket.escalated) { + val intent = events.mapNotNull { it.payload as? InitialIntentEvent }.lastOrNull()?.intent + val content = if (ticket.gate == STAGE_LOOP_BREAK_GATE) { + // A stuck-loop route is NOT a cross-file contract dispute — the gate output names no files, it + // is the SAME action (often a malformed tool call) repeated until the loop-break tripped. The + // arbitration prompt (read/reconcile the named files) sends the model hunting for files that + // don't exist. Tell it plainly: the last action is futile, take a materially different one. + "## Stuck-loop ticket\n" + + "Stage '${ticket.stageId.value}' repeated the SAME failing action until it tripped the " + + "'${ticket.gate}' gate. Retrying that action again is futile — it will fail identically. The " + + "exact failure is below; it is a single stuck step, NOT a dispute between files, so do not go " + + "looking for files to reconcile. If it is a malformed tool call, fix the call's shape and " + + "continue the task; otherwise take a materially different route to the same goal. Serve the " + + "intent" + (intent?.let { " below" } ?: "") + ", then control returns to verification." + + (intent?.let { "\n### Intent (authoritative)\n$it" } ?: "") + + "\n### Gate output (the failing action)\n${ticket.evidence}" + } else if (ticket.escalated) { // Tier 2: the owner loop couldn't settle it — a cross-file contract dispute. The arbiter holds // the intent and reconciles ALL sides in one pass. - val intent = events.mapNotNull { it.payload as? InitialIntentEvent }.lastOrNull()?.intent "## Contract arbitration ticket\n" + "Stage '${ticket.stageId.value}' keeps failing the '${ticket.gate}' gate even after the " + "file owners repaired their own layers — so this is NOT a bug in one file. The files named " + diff --git a/infrastructure/tools/src/main/kotlin/com/correx/infrastructure/tools/shell/ShellTool.kt b/infrastructure/tools/src/main/kotlin/com/correx/infrastructure/tools/shell/ShellTool.kt index 123f947e..09f14107 100644 --- a/infrastructure/tools/src/main/kotlin/com/correx/infrastructure/tools/shell/ShellTool.kt +++ b/infrastructure/tools/src/main/kotlin/com/correx/infrastructure/tools/shell/ShellTool.kt @@ -164,15 +164,30 @@ class ShellTool( } } - // argv[0] is the program name; embedded quotes/commas/whitespace mean the array collapsed into - // one string element and there is no real executable to run. - private fun checkExecutable(argv: List): ArgvParse = - if (argv[0].any { it == '"' || it == ',' || it.isWhitespace() }) { + // argv[0] is the program name. A single element carrying whitespace is the weak-model "whole + // command line in one string" collapse (["npm create vite@latest frontend"]) — split it into + // tokens so the denylist, allowlist and executor all see a real argv, instead of rejecting it and + // looping the stage on a call the model reliably re-emits. A JSON-escape mangle collapses + // differently — it leaves quotes/commas in argv[0] (["npm\",\"-v"]) — and stays rejected: there is + // no runnable program to recover. (Splitting on whitespace loses quoting of args with spaces — + // same ceiling as the `sh -c` join below; fine for agent shell use, not a general shell API.) + private fun checkExecutable(raw: List): ArgvParse { + val argv = if (raw.size == 1 && raw[0].any { it.isWhitespace() } && + raw[0].none { it == '"' || it == ',' } + ) { + raw[0].trim().split(WHITESPACE_RE) + } else { + raw + } + return if (argv[0].any { it == '"' || it == ',' || it.isWhitespace() }) { ArgvParse.Bad( "argv[0] `${argv[0]}` is not a valid executable name — it looks like a collapsed array. " + "Emit each token as a separate string element, e.g. [\"npm\", \"-v\"].", ) - } else ArgvParse.Ok(argv) + } else { + ArgvParse.Ok(argv) + } + } override suspend fun execute(request: ToolRequest): ToolResult = withContext(Dispatchers.IO) { validateRequest(request).run { @@ -221,6 +236,7 @@ class ShellTool( // (line 250 of 500) survives the head/tail window instead of being silently dropped. const val SALIENCE_PATTERN = "error|fail|exception|panic|traceback|✗" const val EMPTY_MSG = "Missing or empty 'argv' parameter. Expected a JSON array of strings." + val WHITESPACE_RE = Regex("\\s+") val SHELL_BUILTINS = setOf("cd", "export", "source", ".", "pushd", "popd", "umask", "ulimit") val SHELL_OPERATORS = setOf("&&", "||", "|", ">", ">>", "<", ";", "&", "2>", "2>&1") // Operators that chain a new command (its following token is a command position); the rest diff --git a/infrastructure/tools/src/test/kotlin/com/correx/infrastructure/tools/shell/ShellToolTest.kt b/infrastructure/tools/src/test/kotlin/com/correx/infrastructure/tools/shell/ShellToolTest.kt index 3d6bd8f9..57d1fb00 100644 --- a/infrastructure/tools/src/test/kotlin/com/correx/infrastructure/tools/shell/ShellToolTest.kt +++ b/infrastructure/tools/src/test/kotlin/com/correx/infrastructure/tools/shell/ShellToolTest.kt @@ -204,6 +204,15 @@ class ShellToolTest { assertTrue((result as ValidationResult.Invalid).reason.contains("collapsed array"), result.reason) } + @Test + fun `validateRequest splits a collapsed single-string command line into tokens`() { + // Repro: weak model emitted ["npm create vite@latest frontend"] — the whole command line as + // one whitespace-carrying element (no quotes/commas). Now split into tokens instead of + // rejected as a collapsed array, so the stage doesn't loop on a call the model keeps re-emitting. + val result = ShellTool().validateRequest(rawArgvRequest(listOf("npm create vite@latest frontend"))) + assertTrue(result is ValidationResult.Valid, (result as? ValidationResult.Invalid)?.reason ?: "") + } + @Test fun `execute runs a shell command line via sh -c`(): Unit = runBlocking { // Repro: gemma called shell with a builtin/command-line ("cd apps && ..."). Now honoured diff --git a/infrastructure/workflow/src/main/kotlin/com/correx/infrastructure/workflow/PlanLinter.kt b/infrastructure/workflow/src/main/kotlin/com/correx/infrastructure/workflow/PlanLinter.kt index c946cb56..dfd661b8 100644 --- a/infrastructure/workflow/src/main/kotlin/com/correx/infrastructure/workflow/PlanLinter.kt +++ b/infrastructure/workflow/src/main/kotlin/com/correx/infrastructure/workflow/PlanLinter.kt @@ -41,10 +41,13 @@ object PlanLinter { fun lint(graph: WorkflowGraph, seeds: Set = seedArtifacts): PlanLintResult = PlanLintResult( - hardFailures = unproducedNeeds(graph, seeds) + - trapStates(graph) + + // HARD = deterministic graph facts, provably broken regardless of wording. SOFT = heuristics + // (incl. H3's prose word-match, which cannot tell a forgotten dep from a descriptive mention) + // — they score the plan but must NOT block it, or a weak architect burns retries it can't + // reword its way out of. + hardFailures = unproducedNeeds(graph, seeds) + trapStates(graph), + softFindings = stageCount(graph) + fanOut(graph) + emptyBriefs(graph) + duplicateBriefs(graph) + unreferencedPromptArtifacts(graph, seeds), - softFindings = stageCount(graph) + fanOut(graph) + emptyBriefs(graph) + duplicateBriefs(graph), ) /** H1: a stage `needs` an artifact that neither a plan stage `produces` nor a seed provides. */ @@ -92,14 +95,21 @@ object PlanLinter { } /** - * H3: a stage's inline prompt mentions an artifact ID that is produced by the plan or - * provided by a seed, but the stage does not declare it in its `needs`. The stage will - * not have access to that artifact at runtime, so any reference in the prompt is a dangling - * assumption that will confuse or stall the model — force the architect to either add the - * artifact to `needs` or reword the prompt. + * H3 (SOFT): a stage's inline prompt word-matches a plan-produced artifact ID the stage does not + * declare in `needs` — possibly a dangling reference the model can't resolve at runtime. This is a + * prose heuristic, not a graph fact: it cannot distinguish a forgotten dependency from a + * descriptive mention ("unlike the `report` stage"), so it only penalises the plan's score and + * never blocks it. Seeds are excluded (see below); a genuine missing consumption is still caught + * structurally by H1 when the stage actually declares the `needs`. */ private fun unreferencedPromptArtifacts(graph: WorkflowGraph, seeds: Set): List { - val produced = graph.stages.values.flatMap { it.produces }.map { it.name.value }.toSet() + seeds + // Only plan-PRODUCED artifacts are checked — a prompt that references a distinctly-named one + // (e.g. `server_api_analysis`) but omits it from `needs` is a real dangling dependency. Seed + // IDs (`analysis`, `dod`) are EXCLUDED: they double as ordinary words, so a prose mention + // ("refine the analysis", "meets the DoD") is not a dependency signal — flagging it just churns + // architect retries on a false positive the model cannot word around. (A stage that genuinely + // consumes a seed still declares it in `needs`; H1 keeps that path honest.) + val produced = graph.stages.values.flatMap { it.produces }.map { it.name.value }.toSet() - seeds return graph.stages.entries.flatMap { (id, stage) -> val prompt = briefOf(stage) val needIds = stage.needs.map { it.value }.toSet() diff --git a/infrastructure/workflow/src/test/kotlin/com/correx/infrastructure/workflow/PlanLinterTest.kt b/infrastructure/workflow/src/test/kotlin/com/correx/infrastructure/workflow/PlanLinterTest.kt index a39436a9..0ecf87fc 100644 --- a/infrastructure/workflow/src/test/kotlin/com/correx/infrastructure/workflow/PlanLinterTest.kt +++ b/infrastructure/workflow/src/test/kotlin/com/correx/infrastructure/workflow/PlanLinterTest.kt @@ -107,7 +107,23 @@ class PlanLinterTest { } @Test - fun `a prompt mentioning another stage's artifact not in needs is a hard failure`() { + fun `a prompt using a seed name as an ordinary word is not a failure`() { + // Repro: architect burned 3 retries because briefs mentioning "analysis"/"dod" as prose kept + // tripping H3 on the seed IDs — words the model cannot reword away. + val g = graph( + stages = mapOf( + "a" to stage(produces = listOf("scope"), prompt = "refine the analysis and meet the dod"), + "b" to stage(needs = listOf("scope"), prompt = "build"), + ), + edges = listOf("a" to "b", "b" to "done"), + start = "a", + ) + val result = PlanLinter.lint(g) + assertTrue(result.passed, "seed names in prose must not fire H3, hard=${result.hardFailures}") + } + + @Test + fun `a prompt mentioning another stage's artifact not in needs is a soft finding, not a block`() { val g = graph( stages = mapOf( "a" to stage(produces = listOf("plan"), prompt = "write the plan artifact"), @@ -117,8 +133,8 @@ class PlanLinterTest { start = "a", ) val result = PlanLinter.lint(g) - assertFalse(result.passed) - val h = result.hardFailures.single { it.code == "unreferenced_prompt_artifact" } + assertTrue(result.passed, "H3 is a prose heuristic — it scores but must not block the plan") + val h = result.softFindings.single { it.code == "unreferenced_prompt_artifact" } assertEquals("b", h.stageId) assertTrue(h.detail.contains("plan")) }