From 78e326039d145e593f8a1135961f5d4628ced5c6 Mon Sep 17 00:00:00 2001 From: Alex Clarke Date: Mon, 14 Sep 2026 04:12:56 -0600 Subject: [PATCH] feat(TASK-030): callers escalate on GAUNTLET_REVIEW_INCOMPLETE instead of findings-routing --- assets/agents/architect/README.md | 2 +- assets/agents/architect/config.yaml | 25 +++++++++++++-- assets/agents/sisyphus/README.md | 2 +- assets/agents/sisyphus/config.yaml | 26 +++++++++++++--- src/config/request_context.rs | 48 +++++++++++++++++++++++++++++ 5 files changed, 93 insertions(+), 10 deletions(-) diff --git a/assets/agents/architect/README.md b/assets/agents/architect/README.md index 96df479..b36f360 100644 --- a/assets/agents/architect/README.md +++ b/assets/agents/architect/README.md @@ -177,7 +177,7 @@ By default (`parallel_tasks: 0`) tasks run **sequentially** on the single run br | [`explore`](../explore/README.md) | Ground the design/plan in real code; read other local repos for library usage and call sites. | | [`librarian`](../librarian/README.md) | External docs / OSS examples for unfamiliar libraries. | | [`whetstone`](../whetstone/README.md) | Continuous improvement — spawned once before the final report when the run addressed PR review feedback from outside its own lanes; converts review misses into user-approved suite edits / ledger entries. Never blocks the run. | -| [`review-gauntlet`](../review-gauntlet/README.md) | Preferred per-task conformance gate — one spawn running adversary (+ probe for consumer-facing tasks) with a machine-parsed GAUNTLET: PASS/BLOCKED verdict; direct adversary/probe spawns remain the fallback. | +| [`review-gauntlet`](../review-gauntlet/README.md) | Preferred per-task conformance gate — one spawn running adversary (+ probe for consumer-facing tasks) with a machine-parsed GAUNTLET: PASS/BLOCKED verdict; a `GAUNTLET_REVIEW_INCOMPLETE:` line is an infrastructure fault escalated to the user (retry / accept NEEDS-HUMAN / abort), never a divergence; direct adversary/probe spawns remain the fallback. | ## Related skills diff --git a/assets/agents/architect/config.yaml b/assets/agents/architect/config.yaml index 4058e04..ea64257 100644 --- a/assets/agents/architect/config.yaml +++ b/assets/agents/architect/config.yaml @@ -341,9 +341,28 @@ instructions: | prompt also carries pasted plan text from the repo under review, and whatever is declared here is executed with a shell — a trust boundary the prose must not cross. - It returns `GAUNTLET: PASS/BLOCKED`. On BLOCKED apply the same fix-and-rerun rules as below - (resume the SAME Sisyphus session with the findings verbatim, then a FRESH gauntlet run). - The direct `adversary`/probe spawns below are the FALLBACK when the gauntlet is not installed. + It returns `GAUNTLET: PASS/BLOCKED`. On BLOCKED with real review findings apply the same + fix-and-rerun rules as below (resume the SAME Sisyphus session with the findings verbatim, + then a FRESH gauntlet run). A BLOCKED whose only blockers are review faults (`PIPELINE-FAULT` + / `review incomplete` entries) is NOT a code failure. A `GAUNTLET_REVIEW_INCOMPLETE: [, + ]` line directly under `GAUNTLET: BLOCKED` means the review could NOT be completed: the + named lanes still faulted after 3 attempts, or the gauntlet pipeline itself faulted + (`pipeline`). This is an infrastructure fault, not a code finding. + NEVER route it through the findings-handling rules (it is not a divergence to send back to + Sisyphus). NEVER claim done while it stands. + Escalate to the user via `user__select` with exactly these three options: + - "Retry the review again" + - "Accept NEEDS-HUMAN: proceed without the review (recorded in the report / PR body)" + - "Abort this task" + Retry → a FRESH gauntlet run. Accept → close the task and record the skipped review (lane + names or `pipeline`) in the task's `log.md`, the Phase F digest, and the PR body. Abort → + leave the task `in-progress`, log the abort in its `log.md`, and stop to report. Findings AND + incomplete on the same output → fix the findings first, then re-run the gauntlet (the re-run + retries the lane) — escalate only if the line persists. Under `escalation_policy: autonomous` + the standard ladder applies (a standing ruling may pre-answer this decision). + The direct `adversary`/probe spawns below are the FALLBACK when the gauntlet is not installed; + on that path an adversary/probe result that is ONLY a `PIPELINE-FAULT` (no verdict on the + code) is likewise an infrastructure fault to retry, never a divergence. - **Spawn `adversary`** with the diff base and the criteria pasted in: ``` diff --git a/assets/agents/sisyphus/README.md b/assets/agents/sisyphus/README.md index 8c91715..6aeb602 100644 --- a/assets/agents/sisyphus/README.md +++ b/assets/agents/sisyphus/README.md @@ -47,7 +47,7 @@ Spawnable sub-agents (from `config.yaml`): - **[code-reviewer](../code-reviewer/README.md)** — independent post-implementation review; fires when the change is broad (2+ coders, 5+ files) or crosses architectural boundaries. - **[adversary](../adversary/README.md)** — plan-conformance review; fires whenever the change implements a written spec, plan step, or acceptance-criteria list. Orthogonal to `code-reviewer` — both can run. - **[security-reviewer](../security-reviewer/README.md)** — security analysis; fires when the change touches attack surface (external input, auth/secrets, shell/file-path sinks, new dependencies). Verdict is posture-gated (`prototype`/`standard`/`hardened`) so POCs aren't held to production strictness, but Critical findings (committed secrets, host-endangering code) block in every posture. Orthogonal to both other reviewers — all three can run. -- **[review-gauntlet](../review-gauntlet/README.md)** — preferred single entry point for the post-implementation review passes: a graph that computes diff signals deterministically, runs the applicable lanes (code-reviewer / adversary / security-reviewer / probe) in parallel, and machine-parses their verdict sentinels into GAUNTLET: PASS/BLOCKED — skipping a lane or misreading a verdict becomes structurally impossible. The reviewer bullets above describe the lanes it runs and remain the spawn fallback when it is not installed. +- **[review-gauntlet](../review-gauntlet/README.md)** — preferred single entry point for the post-implementation review passes: a graph that computes diff signals deterministically, runs the applicable lanes (code-reviewer / adversary / security-reviewer / probe) in parallel, and machine-parses their verdict sentinels into GAUNTLET: PASS/BLOCKED — skipping a lane or misreading a verdict becomes structurally impossible. A `GAUNTLET_REVIEW_INCOMPLETE:` line under BLOCKED is an infrastructure fault (a lane exhausted its retries, or the pipeline faulted) that Sisyphus escalates to the user — retry / accept NEEDS-HUMAN / abort — never a finding to fix. The reviewer bullets above describe the lanes it runs and remain the spawn fallback when it is not installed. - **[step-runner](../step-runner/README.md)** — graph agent that executes one step of a phased plan repo. Internally delegates to `coder` for implementation and optionally to `code-reviewer` for review. - **[whetstone](../whetstone/README.md)** — continuous improvement; spawned ONCE at the end of a run that addressed review feedback from outside Sisyphus's own lanes (humans, or a teammate's independent review agent — never its own reviewers' output). Classifies each piece of feedback CATCH/PARTIAL/MISS against what the run's own reviewers flagged and converts the misses into user-approved suite improvements (skill edits, ledger entries). Never blocks run completion. diff --git a/assets/agents/sisyphus/config.yaml b/assets/agents/sisyphus/config.yaml index d5c5f6f..c81c5f5 100644 --- a/assets/agents/sisyphus/config.yaml +++ b/assets/agents/sisyphus/config.yaml @@ -329,11 +329,27 @@ instructions: | parallel verdict-independent, and returns a machine-parsed `GAUNTLET: PASS` or `GAUNTLET: BLOCKED` with a lane table, blockers, attention items, and every lane's full report. - - `GAUNTLET: BLOCKED` blocks completion. Route each blocker through the findings-handling rules in - the reviewer sections below (🔴 → coder fix in the SAME session; DIVERGES → the adversary rules; - security FAIL → the security rules; probe INCONCLUSIVE → fix the ENVIRONMENT per the plan's - local-run recipe first — it is never a code verdict). Then spawn a FRESH gauntlet run; lane - selection re-runs from the new diff. + - `GAUNTLET: BLOCKED` blocks completion. Route each blocker that is a real review finding through + the findings-handling rules in the reviewer sections below (🔴 → coder fix in the SAME session; + DIVERGES → the adversary rules; security FAIL → the security rules; probe INCONCLUSIVE → fix the + ENVIRONMENT per the plan's local-run recipe first — it is never a code verdict). Then spawn a + FRESH gauntlet run; lane selection re-runs from the new diff. A BLOCKED whose only blockers are + review faults (`PIPELINE-FAULT` / `review incomplete` entries) is NOT a code failure — the next + rule owns it. + - `GAUNTLET_REVIEW_INCOMPLETE: [, ]` (the line directly under `GAUNTLET: BLOCKED`) + means the review could NOT be completed: the named lanes still faulted after 3 attempts, or the + gauntlet pipeline itself faulted (`pipeline`). This is an infrastructure fault, not a code + finding. NEVER route it through the findings-handling rules. NEVER claim done while it stands. + Escalate to the user via `user__select` with exactly these three options: + - "Retry the review again" + - "Accept NEEDS-HUMAN: proceed without the review (recorded in the report / PR body)" + - "Abort this task" + Retry → spawn a FRESH gauntlet run. Accept → finish the work and record the skipped review + (lane names or `pipeline`) in your final report and in the PR body when you author one. Abort → + stop; leave the code in a working state and report. Findings AND incomplete on the same output → + fix the findings first, then re-run the gauntlet (the re-run retries the lane) — escalate only if + the line persists. Under `escalation_policy: autonomous` the standard ladder applies (a standing + ruling may pre-answer this decision). - `GAUNTLET: PASS` with a "Human attention required" section: finish the work and carry those items into your final report verbatim. - The reviewer sections below remain authoritative for HANDLING each lane's findings, and are the diff --git a/src/config/request_context.rs b/src/config/request_context.rs index 4262da2..b184cbe 100644 --- a/src/config/request_context.rs +++ b/src/config/request_context.rs @@ -8223,6 +8223,54 @@ mod tests { ); } + // A gauntlet output carrying the review-incomplete line is an infrastructure + // fault: both callers must escalate it with the exact same three options + // instead of routing it through their findings-handling rules. + #[test] + #[serial] + fn bundled_assets_pin_review_incomplete_escalation() { + let _guard = TestConfigDirGuard::new(); + Agent::install_builtin_agents(false).unwrap(); + + const SHARED_ANCHORS: [&str; 7] = [ + "GAUNTLET_REVIEW_INCOMPLETE:", + "\"Retry the review again\"", + "\"Accept NEEDS-HUMAN: proceed without the review (recorded in the report / PR body)\"", + "\"Abort this task\"", + "NEVER route it through the findings-handling rules", + "NEVER claim done while it stands", + "is NOT a code failure", + ]; + + let mut configs = std::collections::HashMap::new(); + for name in ["architect", "sisyphus"] { + let config = + read_to_string(paths::agents_data_dir().join(name).join("config.yaml")).unwrap(); + for anchor in SHARED_ANCHORS { + assert!( + config.contains(anchor), + "{name} config lost review-incomplete escalation anchor: {anchor:?}" + ); + } + assert!( + config.contains("user__select"), + "{name} config must escalate review-incomplete via user__select" + ); + configs.insert(name, config); + } + + // The architect also owns the no-gauntlet fallback path: a + // fault-only adversary/probe result is retried, never treated as + // a divergence. + let architect = &configs["architect"]; + assert!( + architect.contains( + "an adversary/probe result that is ONLY a `PIPELINE-FAULT` (no verdict on the\n code) is likewise an infrastructure fault to retry, never a divergence." + ), + "architect config lost the fallback adversary/probe pipeline-fault sentence" + ); + } + // Shell commands the adversary's run_checks stage executes must be // declared as a variable, never as prompt prose an LLM would re-extract. #[test]