fix(weak-model-gates): stop three gates from stalling weak stage models
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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015Ly2mMnt9TCZbvhcC1JfuV
This commit is contained in:
+16
-2
@@ -82,10 +82,24 @@ fun buildRecoveryTicketEntry(events: List<StoredEvent>, 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 " +
|
||||
|
||||
+21
-5
@@ -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<String>): 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<String>): 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
|
||||
|
||||
+9
@@ -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
|
||||
|
||||
+19
-9
@@ -41,10 +41,13 @@ object PlanLinter {
|
||||
|
||||
fun lint(graph: WorkflowGraph, seeds: Set<String> = 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<String>): List<PlanLintFinding> {
|
||||
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()
|
||||
|
||||
+19
-3
@@ -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"))
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user