[FIX] Research recipe valid=False on every backend blocks open_kitchen

src/autoskillit/recipes/diagrams/research.md

@@ -1,4 +1,4 @@
-<!-- autoskillit-recipe-hash: sha256:1d5fac42c5bb63012f0407cb60737d61eca2eb680d3e461a56f588852875fabb -->
+<!-- autoskillit-recipe-hash: sha256:1d61758ec0da78599ff3979d01bad2cedffd1eb616aee634e84d90374f37e12b -->
 <!-- autoskillit-diagram-format: v7 -->
 
 ## research
@@ -49,6 +49,16 @@ decompose_phases
 |
 +----+
 
+audit_impl
+|    x fail [-> remediate]
+|
+remediate (route)
+|
+check_audit_retry_loop (max 2)
+|    x exceeded [-> escalate_stop]
+|
+pre_remediation_cleanup
+|
 run_experiment <-> [adjust_experiment] (optional)
 |    x fail [-> troubleshoot_run_failure]
 |    x exhausted [-> ensure_results]

src/autoskillit/recipes/research.json

@@ -683,13 +683,60 @@
           "route": "run_experiment"
         },
         {
+          "when": "result.error",
           "route": "escalate_stop"
+        },
+        {
+          "route": "remediate"
         }
       ],
       "on_failure": "escalate_stop",
       "on_context_limit": "escalate_stop",
       "skip_when_false": "inputs.audit_impl",
-      "note": "Post-implementation quality gate. Diffs impl_base_sha..HEAD and verifies all decomposed phases from group_files have corresponding implementation commits. Catches silent group abandonment when on_context_limit fires mid-iteration — reports exactly which phases were completed vs. skipped. On GO, proceeds to run_experiment. On NO GO, escalates with a remediation report detailing which phases are missing. If false (inputs.audit_impl=false), skip directly to run_experiment.\n"
+      "note": "Post-implementation quality gate. Diffs impl_base_sha..HEAD and verifies all decomposed phases from group_files have corresponding implementation commits. Catches silent group abandonment when on_context_limit fires mid-iteration — reports exactly which phases were completed vs. skipped. On GO, proceeds to run_experiment. On error, escalates immediately. On NO GO, routes to remediate which re-enters plan_phase via check_audit_retry_loop (max 2 cycles). If false (inputs.audit_impl=false), skip directly to run_experiment.\n"
+    },
+    "remediate": {
+      "action": "route",
+      "with": {
+        "remediation_path": "${{ context.remediation_path }}"
+      },
+      "on_success": "check_audit_retry_loop",
+      "note": "Pure routing step carrying the remediation file path from audit_impl's capture into the retry cycle. The remediation_path contains the list of missing phases identified by audit-impl.\n"
+    },
+    "check_audit_retry_loop": {
+      "tool": "run_python",
+      "with": {
+        "callable": "autoskillit.smoke_utils.check_loop_iteration",
+        "current_iteration": "${{ context.audit_retry_count }}",
+        "max_iterations": "2"
+      },
+      "capture": {
+        "audit_retry_count": "${{ result.next_iteration }}"
+      },
+      "on_result": [
+        {
+          "when": "${{ result.max_exceeded }} == true",
+          "route": "escalate_stop"
+        },
+        {
+          "route": "pre_remediation_cleanup"
+        }
+      ],
+      "on_failure": "escalate_stop",
+      "optional_context_refs": [
+        "audit_retry_count"
+      ],
+      "note": "Guards the audit_impl → remediate → plan_phase remediation cycle. Allows up to 2 remediation attempts before escalating. This prevents unbounded re-planning when audit_impl repeatedly returns NO GO.\n"
+    },
+    "pre_remediation_cleanup": {
+      "tool": "run_cmd",
+      "with": {
+        "cmd": "git worktree remove --force \"${{ context.worktree_path }}\"",
+        "step_name": "pre_remediation_cleanup"
+      },
+      "on_success": "plan_phase",
+      "on_failure": "plan_phase",
+      "note": "Removes the old worktree before the remediation cycle creates a new one via implement_phase. on_failure routes to plan_phase regardless — if the worktree is already gone (e.g., context_limit cleanup), the new worktree created by the next implement_phase will work correctly.\n"
     },
     "run_experiment": {
       "tool": "run_skill",

src/autoskillit/recipes/research.yaml

@@ -665,7 +665,9 @@ steps:
     on_result:
     - when: "${{ result.verdict }} == GO"
       route: run_experiment
-    - route: escalate_stop
+    - when: "result.error"
+      route: escalate_stop
+    - route: remediate
     on_failure: escalate_stop
     on_context_limit: escalate_stop
     skip_when_false: "inputs.audit_impl"
@@ -674,9 +676,53 @@ steps:
       all decomposed phases from group_files have corresponding implementation
       commits. Catches silent group abandonment when on_context_limit fires
       mid-iteration — reports exactly which phases were completed vs. skipped.
-      On GO, proceeds to run_experiment. On NO GO, escalates with a remediation
-      report detailing which phases are missing. If false (inputs.audit_impl=false),
-      skip directly to run_experiment.
+      On GO, proceeds to run_experiment. On error, escalates immediately.
+      On NO GO, routes to remediate which re-enters plan_phase via
+      check_audit_retry_loop (max 2 cycles).
+      If false (inputs.audit_impl=false), skip directly to run_experiment.
+
+  remediate:
+    action: route
+    with:
+      remediation_path: "${{ context.remediation_path }}"
+    on_success: check_audit_retry_loop
+    note: >
+      Pure routing step carrying the remediation file path from audit_impl's
+      capture into the retry cycle. The remediation_path contains the list of
+      missing phases identified by audit-impl.
+
+  check_audit_retry_loop:
+    tool: run_python
+    with:
+      callable: "autoskillit.smoke_utils.check_loop_iteration"
+      current_iteration: "${{ context.audit_retry_count }}"
+      max_iterations: "2"
+    capture:
+      audit_retry_count: "${{ result.next_iteration }}"
+    on_result:
+    - when: "${{ result.max_exceeded }} == true"
+      route: escalate_stop
+    - route: pre_remediation_cleanup
+    on_failure: escalate_stop
+    optional_context_refs:
+    - audit_retry_count
+    note: >
+      Guards the audit_impl → remediate → plan_phase remediation cycle.
+      Allows up to 2 remediation attempts before escalating. This prevents
+      unbounded re-planning when audit_impl repeatedly returns NO GO.
+
+  pre_remediation_cleanup:
+    tool: run_cmd
+    with:
+      cmd: "git worktree remove --force \"${{ context.worktree_path }}\""
+      step_name: pre_remediation_cleanup
+    on_success: plan_phase
+    on_failure: plan_phase
+    note: >
+      Removes the old worktree before the remediation cycle creates a new one
+      via implement_phase. on_failure routes to plan_phase regardless — if the
+      worktree is already gone (e.g., context_limit cleanup), the new worktree
+      created by the next implement_phase will work correctly.
 
   # --- EXPERIMENT + REPORT PHASE ---
   run_experiment:

src/autoskillit/skills_extended/resolve-review/SKILL.md

@@ -504,7 +504,7 @@ classified `REJECT` with `category: "arch_violation"`.
 | Subagent filter guard | `test_subagent_filter_guard.py` | NDJSON assistant-record consumers missing `_is_parent_assistant_record` or `_is_parent_assistant` predicate — subagent records contaminate parent metrics |
 | Env-var-set constant consumption | `test_canonical_constant_consumption.py` | `*_ENV_FORWARD_VARS` or `*_REQUIRED_ENV` constant with zero production importers — every canonical env-var-set must be consumed |
 | MCP env forward coverage | `test_mcp_env_forward_coverage.py` | `mcp_env_forward_vars` values missing from `CmdSpec.env` in any cmd-builder (skill, food-truck, headless, resume, interactive) |
-| Rule severity consistency | `test_rule_severity_consistency.py` | Direct `RuleFinding()` construction in `@semantic_rule`/`@block_rule` bodies — must use `make_finding()`/`make_block_finding()` |
+| Rule severity consistency | `test_rule_severity_consistency.py` | Direct `RuleFinding()` construction in `@semantic_rule`/`@block_rule` bodies — must use `make_finding()`/`make_block_finding()`; `_KNOWN_NON_CONFORMING_RULES` entries without `# tracking: #NNNN` comments |
 | No direct swap_labels in fleet | `test_swap_labels_guard.py` | Direct `swap_labels` calls in `fleet/` outside `_label_cleanup.py` — must route through `cleanup_orphaned_labels` |
 | Pipeline ordering | `test_pipeline_ordering.py` | Moving `run_semantic_rules` before `_prune_skipped_steps` in `load_and_validate` — semantic rules must run on post-prune recipe |
 

tests/arch/AGENTS.md

@@ -76,7 +76,7 @@ AST enforcement, sub-package layer contracts, and architectural invariant tests.
 | `test_python_no_hardcoded_temp.py` | Architectural invariant: no literal `.autoskillit/temp` outside the whitelist |
 | `test_quota_capability_isolation.py` | AST guard: quota modules must not reference BackendCapabilities fields |
 | `test_recipe_rule_registration.py` | REQ-RECIPE-001: every recipe/rules_*.py file must be imported by recipe/__init__.py |
-| `test_rule_severity_consistency.py` | AST guards: rule functions must use make_finding()/make_block_finding() and RuleFinding severity must match @semantic_rule decorator severity |
+| `test_rule_severity_consistency.py` | AST guards: rule functions must use make_finding()/make_block_finding(); RuleFinding severity must match @semantic_rule decorator severity; _KNOWN_NON_CONFORMING_RULES entries must carry tracking comments |
 | `test_regex_guards.py` | Arch guard: keyword regexes in cmd-scanning rules must use path-safe lookbehind guards |
 | `test_regex_import.py` | Structural guard: src/ must use `import regex as re`, not bare `import re` (hooks/ exempt) |
 | `test_registry.py` | Symbolic rule registry tests (RuleDescriptor, RULES, Violation) |

tests/arch/test_rule_severity_consistency.py

@@ -15,6 +15,7 @@
 from __future__ import annotations
 
 import ast
+import re as _re
 from pathlib import Path
 
 import pytest
@@ -156,7 +157,7 @@ def test_rule_findings_match_rule_def_severity() -> None:
     Path(__file__).resolve().parents[1] / "recipe" / "test_bundled_recipes_dispatch_ready.py"
 )
 
-_ALLOWLIST_CAP = 5
+_ALLOWLIST_CAP = 4
 
 
 def _collect_allowlist_rule_names() -> set[str]:
@@ -205,6 +206,39 @@ def test_dispatch_readiness_allowlist_size_cap() -> None:
     )
 
 
+def test_known_non_conforming_entries_have_tracking_comments() -> None:
+    """Every _KNOWN_NON_CONFORMING_RULES entry must reference a tracking issue."""
+    source = _DISPATCH_READY_TEST.read_text()
+    lines = source.splitlines()
+    tree = ast.parse(source)
+
+    missing: list[str] = []
+    for node in ast.walk(tree):
+        if isinstance(node, ast.AnnAssign):
+            target = node.target
+            value = node.value
+        elif isinstance(node, ast.Assign):
+            target = node.targets[0] if node.targets else None
+            value = node.value
+        else:
+            continue
+        if not isinstance(target, ast.Name) or target.id != "_KNOWN_NON_CONFORMING_RULES":
+            continue
+        if not isinstance(value, ast.Dict):
+            continue
+        for key in value.keys:
+            if isinstance(key, ast.Constant) and isinstance(key.value, str):
+                line = lines[key.lineno - 1]
+                if not _re.search(r"#\s*tracking:\s*#\d+", line):
+                    missing.append(f"{key.value!r} (line {key.lineno})")
+
+    assert not missing, (
+        "Entries in _KNOWN_NON_CONFORMING_RULES missing tracking comments: "
+        + ", ".join(missing)
+        + ". Add '# tracking: #NNNN' with the relevant GitHub issue number."
+    )
+
+
 def _decorator_rule_name(dec: ast.Call) -> str | None:
     """Extract the ``name=`` keyword string value from a decorator call."""
     for kw in dec.keywords:
@@ -238,7 +272,10 @@ def _collect_error_severity_rules() -> set[str]:
 
 @pytest.mark.xfail(
     strict=True,
-    reason="3 ERROR-severity rules still in allowlist — fix recipes before removing xfail",
+    reason=(
+        "2 ERROR-severity rules still in allowlist — fix agent-eval and skill-eval "
+        "before removing xfail"
+    ),
 )
 def test_error_severity_rules_have_no_dispatch_ready_exemptions() -> None:
     """Every ERROR-severity rule must have zero entries in the dispatch-ready allowlist.

tests/recipe/test_backend_reachability.py

@@ -21,7 +21,7 @@
 
 pytestmark = [pytest.mark.layer("recipe"), pytest.mark.small]
 
-_RECIPE_NAMES = ["implementation", "implementation-groups", "remediation"]
+_RECIPE_NAMES = ["implementation", "implementation-groups", "remediation", "research"]
 _BACKEND_NAMES = sorted(BACKEND_REGISTRY.keys())
 
 # Steps guarded by inputs.open_pr rather than backend_supports_git_write.

tests/recipe/test_bundled_recipes_dispatch_ready.py

@@ -29,12 +29,11 @@
 
 
 _KNOWN_NON_CONFORMING_RULES: dict[str, set[str]] = {
-    "research": {"audit-impl-remediation-route"},
-    "agent-eval": {
+    "agent-eval": {  # tracking: #4069
         "all-dispatchable-stops-have-sentinel",
         "dead-output",
     },
-    "skill-eval": {
+    "skill-eval": {  # tracking: #4069
         "all-dispatchable-stops-have-sentinel",
         "dead-output",
     },

tests/recipe/test_research_audit_impl.py

@@ -236,6 +236,85 @@ def test_check_audit_retry_loop_on_failure_escalates(self, recipe) -> None:
         assert step.on_failure == "escalate_stop"
 
 
+class TestResearchRemediationLoop:
+    """Tests for the audit_impl → remediate → plan_phase retry cycle in research.yaml."""
+
+    @pytest.fixture(scope="class")
+    def recipe(self):
+        return load_recipe(builtin_recipes_dir() / "research.yaml")
+
+    def test_audit_impl_nogo_routes_to_remediate(self, recipe) -> None:
+        """research.yaml audit_impl default (NO GO) route must target remediate."""
+        step = recipe.steps["audit_impl"]
+        conditions = step.on_result.conditions
+        default_cond = next((c for c in conditions if c.when is None), None)
+        assert default_cond is not None, "audit_impl must have a default route"
+        assert default_cond.route == "remediate", (
+            f"audit_impl default route must be 'remediate', got '{default_cond.route}'"
+        )
+
+    def test_audit_impl_error_routes_to_escalate_stop(self, recipe) -> None:
+        step = recipe.steps["audit_impl"]
+        conditions = step.on_result.conditions
+        error_cond = next((c for c in conditions if c.when and "result.error" in c.when), None)
+        assert error_cond is not None
+        assert error_cond.route == "escalate_stop"
+
+    def test_has_remediate_step(self, recipe) -> None:
+        assert "remediate" in recipe.steps
+        assert recipe.steps["remediate"].action == "route"
+
+    def test_remediate_carries_remediation_path(self, recipe) -> None:
+        step = recipe.steps["remediate"]
+        assert "context.remediation_path" in step.with_args["remediation_path"]
+
+    def test_remediate_routes_to_check_audit_retry_loop(self, recipe) -> None:
+        step = recipe.steps["remediate"]
+        assert step.on_success == "check_audit_retry_loop"
+
+    def test_has_check_audit_retry_loop_step(self, recipe) -> None:
+        step = recipe.steps["check_audit_retry_loop"]
+        assert step.tool == "run_python"
+        assert step.with_args["callable"] == "autoskillit.smoke_utils.check_loop_iteration"
+
+    def test_check_audit_retry_loop_max_iterations(self, recipe) -> None:
+        step = recipe.steps["check_audit_retry_loop"]
+        assert step.with_args["max_iterations"] == "2"
+
+    def test_check_audit_retry_loop_max_exceeded_routes_to_escalate_stop(self, recipe) -> None:
+        step = recipe.steps["check_audit_retry_loop"]
+        conditions = step.on_result.conditions
+        max_cond = next(
+            (c for c in conditions if c.when and "max_exceeded" in c.when and "== true" in c.when),
+            None,
+        )
+        assert max_cond is not None
+        assert max_cond.route == "escalate_stop"
+
+    def test_check_audit_retry_loop_default_routes_to_pre_remediation_cleanup(
+        self, recipe
+    ) -> None:
+        step = recipe.steps["check_audit_retry_loop"]
+        conditions = step.on_result.conditions
+        default_cond = next((c for c in conditions if c.when is None), None)
+        assert default_cond is not None
+        assert default_cond.route == "pre_remediation_cleanup"
+
+    def test_check_audit_retry_loop_on_failure_escalates(self, recipe) -> None:
+        step = recipe.steps["check_audit_retry_loop"]
+        assert step.on_failure == "escalate_stop"
+
+    def test_has_pre_remediation_cleanup_step(self, recipe) -> None:
+        step = recipe.steps["pre_remediation_cleanup"]
+        assert step.tool == "run_cmd"
+        assert "worktree remove" in step.with_args["cmd"]
+
+    def test_pre_remediation_cleanup_routes_to_plan_phase(self, recipe) -> None:
+        step = recipe.steps["pre_remediation_cleanup"]
+        assert step.on_success == "plan_phase"
+        assert step.on_failure == "plan_phase"
+
+
 class TestResearchCampaignAuditIngredient:
     """Tests for audit ingredient in research-campaign.yaml."""
 

tests/recipe/test_research_context_tracking.py

@@ -111,16 +111,7 @@ def test_research_recipe_passes_semantic_rules(recipe):
     errors = validate_recipe_structure(recipe)
     assert not errors, f"Structural validation errors: {errors}"
     findings = run_semantic_rules(recipe)
-    _KNOWN_NON_CONFORMING = {"audit-impl-remediation-route"}
-    fired_rules = {f.rule for f in findings if f.severity == Severity.ERROR}
-    for rule_name in _KNOWN_NON_CONFORMING:
-        assert rule_name in fired_rules, (
-            f"Exclusion for '{rule_name}' is stale — rule no longer fires. "
-            f"Remove from _KNOWN_NON_CONFORMING."
-        )
-    error_findings = [
-        f for f in findings if f.severity == Severity.ERROR and f.rule not in _KNOWN_NON_CONFORMING
-    ]
+    error_findings = [f for f in findings if f.severity == Severity.ERROR]
     assert not error_findings, (
         f"Semantic rule errors: {[f'{f.rule}: {f.message}' for f in error_findings]}"
     )

tests/recipe/test_rules_registry.py

@@ -93,9 +93,7 @@ def test_bundled_workflows_pass_semantic_rules() -> None:
     yaml_files = list(wf_dir.glob("*.yaml"))
     assert yaml_files
 
-    _KNOWN_NON_CONFORMING: dict[str, set[str]] = {
-        "research.yaml": {"audit-impl-remediation-route"},
-    }
+    _KNOWN_NON_CONFORMING: dict[str, set[str]] = {}
     for path in yaml_files:
         wf = load_recipe(path)
         findings = run_semantic_rules(wf)