From cd3b588eaef36739bf4e3ae8a351df4cc2c67a02 Mon Sep 17 00:00:00 2001 From: Trecek Date: Fri, 26 Jun 2026 10:50:54 -0700 Subject: [PATCH 1/3] fix: route is_fixable=false env/unknown failures to ci_only_failure When the diagnosis file emits is_fixable=false for env/unknown failure subtypes, the verdict must route to human escalation instead of flake_suspected, preventing infinite re-push loops on unfixable failures. --- .../recipes/implementation-groups.json | 32 ++++++- .../recipes/implementation-groups.yaml | 31 ++++++- src/autoskillit/recipes/implementation.json | 32 ++++++- src/autoskillit/recipes/implementation.yaml | 33 ++++++- src/autoskillit/recipes/remediation.json | 32 ++++++- src/autoskillit/recipes/remediation.yaml | 33 ++++++- .../skills_extended/resolve-failures/SKILL.md | 24 ++--- .../test_resolve_ci_routing_invariant.py | 52 ++++++++++- .../recipe/test_rules_flake_loop_deadlock.py | 30 ++++++- .../skills/test_resolve_failures_ci_aware.py | 90 +++++++++++++++---- 10 files changed, 334 insertions(+), 55 deletions(-) diff --git a/src/autoskillit/recipes/implementation-groups.json b/src/autoskillit/recipes/implementation-groups.json index 44cb3e75ea..cc56e3a2c2 100644 --- a/src/autoskillit/recipes/implementation-groups.json +++ b/src/autoskillit/recipes/implementation-groups.json @@ -1487,7 +1487,8 @@ "output_dir": "{{AUTOSKILLIT_TEMP}}/diagnose-ci" }, "capture": { - "diagnosis_path": "${{ result.diagnosis_path }}" + "diagnosis_path": "${{ result.diagnosis_path }}", + "is_fixable": "${{ result.is_fixable }}" }, "on_success": "resolve_ci", "on_failure": "resolve_ci", @@ -1527,7 +1528,7 @@ }, { "when": "${{ result.verdict }} == 'flake_suspected'", - "route": "re_push" + "route": "check_flake_loop" }, { "when": "${{ result.verdict }} == 'ci_only_failure'", @@ -1542,7 +1543,7 @@ } ], "on_failure": "release_issue_failure", - "note": "Runs when ci_watch reports a CI failure. Receives ci_failed_jobs from wait_for_ci structured output to avoid redundant CI failure re-investigation. Routes via on_result: verdict dispatch — real_fix pushes, already_green rebases and re-diagnoses (sibling fix may have landed), flake_suspected retries via re_push (bounded by retries: 2 / on_exhausted: release_issue_failure), ci_only_failure escalates to human. Retries up to 2 times (3 total attempts) before exhausting to release_issue_failure.\n" + "note": "Runs when ci_watch reports a CI failure. Receives ci_failed_jobs from wait_for_ci structured output to avoid redundant CI failure re-investigation. Routes via on_result: verdict dispatch — real_fix pushes, already_green rebases and re-diagnoses (sibling fix may have landed), flake_suspected routes to check_flake_loop (bounded by ci_flake_count <= 3, shared across all diagnose_ci callers; escalates to release_issue_failure when exceeded), ci_only_failure escalates to human. Retries up to 2 times (3 total attempts) before exhausting to release_issue_failure.\n" }, "pre_resolve_rebase": { "tool": "run_python", @@ -1618,6 +1619,31 @@ ], "note": "Guards the resolve_ci(already_green) → pre_resolve_rebase → diagnose_ci cycle. Caps at 3 rebase-rediagnose iterations. If CI keeps reporting already_green after 3 rebases, a structural issue requires human intervention.\n" }, + "check_flake_loop": { + "tool": "run_python", + "with": { + "callable": "autoskillit.smoke_utils.check_loop_iteration", + "current_iteration": "${{ context.ci_flake_count }}", + "max_iterations": "3" + }, + "capture": { + "ci_flake_count": "${{ result.next_iteration }}" + }, + "on_result": [ + { + "when": "${{ result.max_exceeded }} == true", + "route": "release_issue_failure" + }, + { + "route": "re_push" + } + ], + "on_failure": "release_issue_failure", + "optional_context_refs": [ + "ci_flake_count" + ], + "note": "Guards ALL resolve_ci(flake_suspected) → re_push cycles. The ci_flake_count counter is shared across all entry points into diagnose_ci, including CI watch retries, merge queue ejections, dropped-CI-group paths, and conflict resolution escalations. Caps at 3 total flake_suspected verdicts per pipeline run. If CI keeps failing with the same flake_suspected verdict after 3 iterations from any combination of callers, the failure requires human intervention.\n" + }, "re_push": { "tool": "push_to_remote", "with": { diff --git a/src/autoskillit/recipes/implementation-groups.yaml b/src/autoskillit/recipes/implementation-groups.yaml index 43c6e68dab..995dbc7f0c 100644 --- a/src/autoskillit/recipes/implementation-groups.yaml +++ b/src/autoskillit/recipes/implementation-groups.yaml @@ -1340,6 +1340,7 @@ steps: output_dir: "{{AUTOSKILLIT_TEMP}}/diagnose-ci" capture: diagnosis_path: "${{ result.diagnosis_path }}" + is_fixable: "${{ result.is_fixable }}" on_success: resolve_ci on_failure: resolve_ci on_context_limit: resolve_ci @@ -1373,7 +1374,7 @@ steps: - when: "${{ result.verdict }} == 'already_green'" route: pre_resolve_rebase - when: "${{ result.verdict }} == 'flake_suspected'" - route: re_push + route: check_flake_loop - when: "${{ result.verdict }} == 'ci_only_failure'" route: release_issue_failure - when: "${{ result.verdict }} == 'no_test_infrastructure'" @@ -1385,8 +1386,9 @@ steps: Runs when ci_watch reports a CI failure. Receives ci_failed_jobs from wait_for_ci structured output to avoid redundant CI failure re-investigation. Routes via on_result: verdict dispatch — real_fix pushes, already_green rebases and re-diagnoses - (sibling fix may have landed), flake_suspected retries via re_push (bounded by - retries: 2 / on_exhausted: release_issue_failure), ci_only_failure escalates to human. + (sibling fix may have landed), flake_suspected routes to check_flake_loop (bounded + by ci_flake_count <= 3, shared across all diagnose_ci callers; escalates to + release_issue_failure when exceeded), ci_only_failure escalates to human. Retries up to 2 times (3 total attempts) before exhausting to release_issue_failure. pre_resolve_rebase: @@ -1449,6 +1451,29 @@ steps: Caps at 3 rebase-rediagnose iterations. If CI keeps reporting already_green after 3 rebases, a structural issue requires human intervention. + check_flake_loop: + tool: run_python + with: + callable: "autoskillit.smoke_utils.check_loop_iteration" + current_iteration: "${{ context.ci_flake_count }}" + max_iterations: "3" + capture: + ci_flake_count: "${{ result.next_iteration }}" + on_result: + - when: "${{ result.max_exceeded }} == true" + route: release_issue_failure + - route: re_push + on_failure: release_issue_failure + optional_context_refs: + - ci_flake_count + note: > + Guards ALL resolve_ci(flake_suspected) → re_push cycles. The ci_flake_count + counter is shared across all entry points into diagnose_ci, including CI watch + retries, merge queue ejections, dropped-CI-group paths, and conflict resolution + escalations. Caps at 3 total flake_suspected verdicts per pipeline run. If CI + keeps failing with the same flake_suspected verdict after 3 iterations from any + combination of callers, the failure requires human intervention. + re_push: tool: push_to_remote with: diff --git a/src/autoskillit/recipes/implementation.json b/src/autoskillit/recipes/implementation.json index e68d5c274f..2d2c565f43 100644 --- a/src/autoskillit/recipes/implementation.json +++ b/src/autoskillit/recipes/implementation.json @@ -2299,7 +2299,8 @@ "output_dir": "{{AUTOSKILLIT_TEMP}}/diagnose-ci" }, "capture": { - "diagnosis_path": "${{ result.diagnosis_path }}" + "diagnosis_path": "${{ result.diagnosis_path }}", + "is_fixable": "${{ result.is_fixable }}" }, "on_success": "resolve_ci", "on_failure": "resolve_ci", @@ -2336,7 +2337,7 @@ }, { "when": "${{ result.verdict }} == 'flake_suspected'", - "route": "re_push" + "route": "check_flake_loop" }, { "when": "${{ result.verdict }} == 'ci_only_failure'", @@ -2351,7 +2352,7 @@ } ], "on_failure": "release_issue_failure", - "note": "Runs when ci_watch reports a CI failure. Receives ci_failed_jobs from wait_for_ci structured output to avoid redundant CI failure re-investigation. Routes via on_result: verdict dispatch — real_fix pushes, already_green rebases and re-diagnoses (sibling fix may have landed), flake_suspected retries via re_push (bounded by retries: 2 / on_exhausted: release_issue_failure), ci_only_failure escalates to human. Retries up to 2 times (3 total attempts) before exhausting to release_issue_failure.\n" + "note": "Runs when ci_watch reports a CI failure. Receives ci_failed_jobs from wait_for_ci structured output to avoid redundant CI failure re-investigation. Routes via on_result: verdict dispatch — real_fix pushes, already_green rebases and re-diagnoses (sibling fix may have landed), flake_suspected routes to check_flake_loop (bounded by ci_flake_count <= 3, shared across all diagnose_ci callers; escalates to release_issue_failure when exceeded), ci_only_failure escalates to human. Retries up to 2 times (3 total attempts) before exhausting to release_issue_failure.\n" }, "pre_resolve_rebase": { "tool": "run_python", @@ -2427,6 +2428,31 @@ ], "note": "Guards the resolve_ci(already_green) → pre_resolve_rebase → diagnose_ci cycle. Caps at 3 rebase-rediagnose iterations. If CI keeps reporting already_green after 3 rebases, a structural issue requires human intervention.\n" }, + "check_flake_loop": { + "tool": "run_python", + "with": { + "callable": "autoskillit.smoke_utils.check_loop_iteration", + "current_iteration": "${{ context.ci_flake_count }}", + "max_iterations": "3" + }, + "capture": { + "ci_flake_count": "${{ result.next_iteration }}" + }, + "on_result": [ + { + "when": "${{ result.max_exceeded }} == true", + "route": "release_issue_failure" + }, + { + "route": "re_push" + } + ], + "on_failure": "release_issue_failure", + "optional_context_refs": [ + "ci_flake_count" + ], + "note": "Guards ALL resolve_ci(flake_suspected) → re_push cycles. The ci_flake_count counter is shared across all entry points into diagnose_ci, including CI watch retries, merge queue ejections, dropped-CI-group paths, and conflict resolution escalations. Caps at 3 total flake_suspected verdicts per pipeline run. If CI keeps failing with the same flake_suspected verdict after 3 iterations from any combination of callers, the failure requires human intervention.\n" + }, "re_push": { "tool": "push_to_remote", "with": { diff --git a/src/autoskillit/recipes/implementation.yaml b/src/autoskillit/recipes/implementation.yaml index 47b7d2930a..4c447712f9 100644 --- a/src/autoskillit/recipes/implementation.yaml +++ b/src/autoskillit/recipes/implementation.yaml @@ -2068,6 +2068,7 @@ steps: output_dir: '{{AUTOSKILLIT_TEMP}}/diagnose-ci' capture: diagnosis_path: ${{ result.diagnosis_path }} + is_fixable: ${{ result.is_fixable }} on_success: resolve_ci on_failure: resolve_ci on_context_limit: resolve_ci @@ -2103,7 +2104,7 @@ steps: - when: ${{ result.verdict }} == 'already_green' route: pre_resolve_rebase - when: ${{ result.verdict }} == 'flake_suspected' - route: re_push + route: check_flake_loop - when: ${{ result.verdict }} == 'ci_only_failure' route: release_issue_failure - when: ${{ result.verdict }} == 'no_test_infrastructure' @@ -2114,9 +2115,10 @@ steps: note: 'Runs when ci_watch reports a CI failure. Receives ci_failed_jobs from wait_for_ci structured output to avoid redundant CI failure re-investigation. Routes via on_result: verdict dispatch — real_fix pushes, already_green rebases and re-diagnoses - (sibling fix may have landed), flake_suspected retries via re_push (bounded - by retries: 2 / on_exhausted: release_issue_failure), ci_only_failure escalates - to human. Retries up to 2 times (3 total attempts) before exhausting to release_issue_failure. + (sibling fix may have landed), flake_suspected routes to check_flake_loop (bounded + by ci_flake_count <= 3, shared across all diagnose_ci callers; escalates to + release_issue_failure when exceeded), ci_only_failure escalates to human. + Retries up to 2 times (3 total attempts) before exhausting to release_issue_failure. ' pre_resolve_rebase: @@ -2178,6 +2180,29 @@ steps: cycle. Caps at 3 rebase-rediagnose iterations. If CI keeps reporting already_green after 3 rebases, a structural issue requires human intervention. + ' + check_flake_loop: + tool: run_python + with: + callable: autoskillit.smoke_utils.check_loop_iteration + current_iteration: ${{ context.ci_flake_count }} + max_iterations: '3' + capture: + ci_flake_count: ${{ result.next_iteration }} + on_result: + - when: ${{ result.max_exceeded }} == true + route: release_issue_failure + - route: re_push + on_failure: release_issue_failure + optional_context_refs: + - ci_flake_count + note: 'Guards ALL resolve_ci(flake_suspected) → re_push cycles. The ci_flake_count + counter is shared across all entry points into diagnose_ci, including CI watch + retries, merge queue ejections, dropped-CI-group paths, and conflict resolution + escalations. Caps at 3 total flake_suspected verdicts per pipeline run. If CI + keeps failing with the same flake_suspected verdict after 3 iterations from any + combination of callers, the failure requires human intervention. + ' re_push: tool: push_to_remote diff --git a/src/autoskillit/recipes/remediation.json b/src/autoskillit/recipes/remediation.json index 87d988c067..411f7e15f5 100644 --- a/src/autoskillit/recipes/remediation.json +++ b/src/autoskillit/recipes/remediation.json @@ -2435,7 +2435,8 @@ "output_dir": "{{AUTOSKILLIT_TEMP}}/diagnose-ci" }, "capture": { - "diagnosis_path": "${{ result.diagnosis_path }}" + "diagnosis_path": "${{ result.diagnosis_path }}", + "is_fixable": "${{ result.is_fixable }}" }, "on_success": "resolve_ci", "on_failure": "resolve_ci", @@ -2472,7 +2473,7 @@ }, { "when": "${{ result.verdict }} == 'flake_suspected'", - "route": "re_push" + "route": "check_flake_loop" }, { "when": "${{ result.verdict }} == 'ci_only_failure'", @@ -2487,7 +2488,7 @@ } ], "on_failure": "release_issue_failure", - "note": "Runs when ci_watch reports a CI failure. Receives ci_failed_jobs from wait_for_ci structured output to avoid redundant CI failure re-investigation. Routes via on_result: verdict dispatch — real_fix pushes, already_green rebases and re-diagnoses (sibling fix may have landed), flake_suspected retries via re_push (bounded by retries: 2 / on_exhausted: release_issue_failure), ci_only_failure escalates to human. Retries up to 2 times (3 total attempts) before exhausting to release_issue_failure.\n" + "note": "Runs when ci_watch reports a CI failure. Receives ci_failed_jobs from wait_for_ci structured output to avoid redundant CI failure re-investigation. Routes via on_result: verdict dispatch — real_fix pushes, already_green rebases and re-diagnoses (sibling fix may have landed), flake_suspected routes to check_flake_loop (bounded by ci_flake_count <= 3, shared across all diagnose_ci callers; escalates to release_issue_failure when exceeded), ci_only_failure escalates to human. Retries up to 2 times (3 total attempts) before exhausting to release_issue_failure.\n" }, "pre_resolve_rebase": { "tool": "run_python", @@ -2563,6 +2564,31 @@ ], "note": "Guards the resolve_ci(already_green) → pre_resolve_rebase → diagnose_ci cycle. Caps at 3 rebase-rediagnose iterations. If CI keeps reporting already_green after 3 rebases, a structural issue requires human intervention.\n" }, + "check_flake_loop": { + "tool": "run_python", + "with": { + "callable": "autoskillit.smoke_utils.check_loop_iteration", + "current_iteration": "${{ context.ci_flake_count }}", + "max_iterations": "3" + }, + "capture": { + "ci_flake_count": "${{ result.next_iteration }}" + }, + "on_result": [ + { + "when": "${{ result.max_exceeded }} == true", + "route": "release_issue_failure" + }, + { + "route": "re_push" + } + ], + "on_failure": "release_issue_failure", + "optional_context_refs": [ + "ci_flake_count" + ], + "note": "Guards ALL resolve_ci(flake_suspected) → re_push cycles. The ci_flake_count counter is shared across all entry points into diagnose_ci, including CI watch retries, merge queue ejections, dropped-CI-group paths, and conflict resolution escalations. Caps at 3 total flake_suspected verdicts per pipeline run. If CI keeps failing with the same flake_suspected verdict after 3 iterations from any combination of callers, the failure requires human intervention.\n" + }, "re_push": { "tool": "push_to_remote", "with": { diff --git a/src/autoskillit/recipes/remediation.yaml b/src/autoskillit/recipes/remediation.yaml index 9cd9d24c07..05c513aea4 100644 --- a/src/autoskillit/recipes/remediation.yaml +++ b/src/autoskillit/recipes/remediation.yaml @@ -2174,6 +2174,7 @@ steps: output_dir: '{{AUTOSKILLIT_TEMP}}/diagnose-ci' capture: diagnosis_path: ${{ result.diagnosis_path }} + is_fixable: ${{ result.is_fixable }} on_success: resolve_ci on_failure: resolve_ci on_context_limit: resolve_ci @@ -2209,7 +2210,7 @@ steps: - when: ${{ result.verdict }} == 'already_green' route: pre_resolve_rebase - when: ${{ result.verdict }} == 'flake_suspected' - route: re_push + route: check_flake_loop - when: ${{ result.verdict }} == 'ci_only_failure' route: release_issue_failure - when: ${{ result.verdict }} == 'no_test_infrastructure' @@ -2219,9 +2220,10 @@ steps: note: 'Runs when ci_watch reports a CI failure. Receives ci_failed_jobs from wait_for_ci structured output to avoid redundant CI failure re-investigation. Routes via on_result: verdict dispatch — real_fix pushes, already_green rebases and re-diagnoses - (sibling fix may have landed), flake_suspected retries via re_push (bounded - by retries: 2 / on_exhausted: release_issue_failure), ci_only_failure escalates - to human. Retries up to 2 times (3 total attempts) before exhausting to release_issue_failure. + (sibling fix may have landed), flake_suspected routes to check_flake_loop (bounded + by ci_flake_count <= 3, shared across all diagnose_ci callers; escalates to + release_issue_failure when exceeded), ci_only_failure escalates to human. + Retries up to 2 times (3 total attempts) before exhausting to release_issue_failure. ' pre_resolve_rebase: @@ -2282,6 +2284,29 @@ steps: cycle. Caps at 3 rebase-rediagnose iterations. If CI keeps reporting already_green after 3 rebases, a structural issue requires human intervention. + ' + check_flake_loop: + tool: run_python + with: + callable: autoskillit.smoke_utils.check_loop_iteration + current_iteration: ${{ context.ci_flake_count }} + max_iterations: '3' + capture: + ci_flake_count: ${{ result.next_iteration }} + on_result: + - when: ${{ result.max_exceeded }} == true + route: release_issue_failure + - route: re_push + on_failure: release_issue_failure + optional_context_refs: + - ci_flake_count + note: 'Guards ALL resolve_ci(flake_suspected) → re_push cycles. The ci_flake_count + counter is shared across all entry points into diagnose_ci, including CI watch + retries, merge queue ejections, dropped-CI-group paths, and conflict resolution + escalations. Caps at 3 total flake_suspected verdicts per pipeline run. If CI + keeps failing with the same flake_suspected verdict after 3 iterations from any + combination of callers, the failure requires human intervention. + ' re_push: tool: push_to_remote diff --git a/src/autoskillit/skills_extended/resolve-failures/SKILL.md b/src/autoskillit/skills_extended/resolve-failures/SKILL.md index 337108d37c..a65b92e90e 100644 --- a/src/autoskillit/skills_extended/resolve-failures/SKILL.md +++ b/src/autoskillit/skills_extended/resolve-failures/SKILL.md @@ -138,10 +138,13 @@ If `diagnosis_path` was provided and the file exists: 1. Open `diagnosis_path` and read its content 2. Find the "Structured Output" section or scan for the line matching `failure_subtype = {value}` 3. Extract the `failure_subtype` value (e.g., `flaky`, `deterministic`, `timing_race`, etc.) -4. Store as `{failure_subtype}` for use in the Verdict Decision Tree +4. Find the line matching `Is Fixable:` header or `is_fixable = {value}` in the Structured Output section +5. Extract the `is_fixable` value (`true` or `false`) +6. Store as `{failure_subtype}` and `{is_fixable}` for use in the Verdict Decision Tree If `diagnosis_path` is absent or the file does not exist: - Set `{failure_subtype} = unknown` +- Set `{is_fixable} = true` (default to fixable — preserves existing behavior for legacy diagnosis files) ### Step 2b: Reproduce CI Pre-Test Steps @@ -181,15 +184,16 @@ and the table is never consulted. **Applies ONLY when no fix was applied (fixes_applied == 0).** If the fix loop was entered and a commit was made, skip this table — verdict is already `real_fix` per Step 2c. -Using `{local_result}` from Step 2 and `{failure_subtype}` from Step 2a, determine `{verdict}`: +Using `{local_result}` from Step 2, `{failure_subtype}` and `{is_fixable}` from Step 2a, determine `{verdict}`: -| Local result | `failure_subtype` | Verdict | -|---|---|---| -| PASS | `flaky` or `timing_race` | `flake_suspected` | -| PASS | `deterministic` | `ci_only_failure` | -| PASS | `fixture` or `import` | `flake_suspected` | -| PASS | `env` or `unknown` | `flake_suspected` | -| PASS | `no_failure` | `ci_only_failure` | +| Local result | `failure_subtype` | `is_fixable` | Verdict | +|---|---|---|---| +| PASS | `flaky` or `timing_race` | any | `flake_suspected` | +| PASS | `deterministic` | any | `ci_only_failure` | +| PASS | `fixture` or `import` | any | `flake_suspected` | +| PASS | `env` or `unknown` | `false` | `ci_only_failure` | +| PASS | `env` or `unknown` | `true` (or unset) | `flake_suspected` | +| PASS | `no_failure` | any | `ci_only_failure` | **Note on `already_green`:** This verdict is reserved for the `pre_resolve_rebase` re-entry path — when a sibling pipeline's fix has already landed on integration and @@ -322,7 +326,7 @@ Where: Return control to the orchestrator. The recipe's `on_result:` routing dispatches on `verdict`: - `real_fix` → `re_push` (fix landed, push to remote) -- `flake_suspected` → `re_push` (retry via CI, bounded by retries: 2 / on_exhausted: release_issue_failure) +- `flake_suspected` → `check_flake_loop` (loop guard with ci_flake_count counter shared across all diagnose_ci callers; escalates to release_issue_failure when max_iterations=3 exceeded, otherwise retries via re_push) - `ci_only_failure` → `release_issue_failure` (human escalation) - `no_test_infrastructure` → `release_issue_failure` / `escalate_stop` / `register_clone_failure` (depending on recipe) diff --git a/tests/recipe/test_resolve_ci_routing_invariant.py b/tests/recipe/test_resolve_ci_routing_invariant.py index 82504fce43..ba48ef1195 100644 --- a/tests/recipe/test_resolve_ci_routing_invariant.py +++ b/tests/recipe/test_resolve_ci_routing_invariant.py @@ -181,8 +181,8 @@ def test_resolve_ci_on_result_routes_escalation_verdicts_to_failure( @pytest.mark.parametrize("recipe_name", _PIPELINE_RECIPES) -def test_resolve_ci_routes_flake_suspected_to_re_push(recipe_name: str) -> None: - """flake_suspected in resolve_ci must route to re_push, not release_issue_failure.""" +def test_resolve_ci_routes_flake_suspected_to_check_flake_loop(recipe_name: str) -> None: + """flake_suspected in resolve_ci must route to check_flake_loop, not directly to re_push.""" recipe_path = _RECIPES_DIR / recipe_name recipe = load_recipe(recipe_path) resolve_steps = _find_resolve_steps_reaching_push(recipe) @@ -196,9 +196,9 @@ def test_resolve_ci_routes_flake_suspected_to_re_push(recipe_name: str) -> None: ] if not routes: continue - assert any("re_push" in r or "pre_review_rebase" in r for r in routes), ( + assert any("check_flake" in r for r in routes), ( f"{recipe_name}/{step_name}: verdict=flake_suspected must route to " - f"a re_push step or pre_review_rebase (retry path), got routes: {routes}" + f"check_flake_loop (bounded retry path), got routes: {routes}" ) @@ -257,3 +257,47 @@ def test_pre_resolve_rebase_step_exists(recipe_name: str) -> None: assert "resolve_pre_resolve_conflicts" in conflict_routes, ( f"{recipe_name}: pre_resolve_rebase must route conflicts to resolve_pre_resolve_conflicts" ) + + +@pytest.mark.parametrize("recipe_name", _PIPELINE_RECIPES) +def test_check_flake_loop_step_exists(recipe_name: str) -> None: + """check_flake_loop must use run_python, check_loop_iteration, and route max_exceeded.""" + recipe_path = _RECIPES_DIR / recipe_name + recipe = load_recipe(recipe_path) + assert "check_flake_loop" in recipe.steps, ( + f"{recipe_name}: missing 'check_flake_loop' step. " + "This step is required to bound the flake_suspected → re_push retry loop." + ) + step = recipe.steps["check_flake_loop"] + assert step.tool == "run_python", ( + f"{recipe_name}: check_flake_loop must use run_python, got {step.tool!r}" + ) + assert step.with_args["callable"] == "autoskillit.smoke_utils.check_loop_iteration", ( + f"{recipe_name}: check_flake_loop must use check_loop_iteration callable" + ) + assert step.on_result is not None, ( + f"{recipe_name}: check_flake_loop must have on_result routing" + ) + max_routes = [ + c.route + for c in step.on_result.conditions + if c.when and "max_exceeded" in c.when and "true" in c.when + ] + assert any("failure" in r or "escalat" in r for r in max_routes), ( + f"{recipe_name}: check_flake_loop max_exceeded=true must route to " + f"a failure/escalation step, got routes: {max_routes}" + ) + + +@pytest.mark.parametrize("recipe_name", _PIPELINE_RECIPES) +def test_diagnose_ci_captures_is_fixable(recipe_name: str) -> None: + """diagnose_ci must capture is_fixable for observability and future routing.""" + recipe_path = _RECIPES_DIR / recipe_name + recipe = load_recipe(recipe_path) + assert "diagnose_ci" in recipe.steps, f"{recipe_name}: missing 'diagnose_ci' step" + step = recipe.steps["diagnose_ci"] + assert "is_fixable" in (step.capture or {}), ( + f"{recipe_name}: diagnose_ci must capture is_fixable. " + "Currently only captures diagnosis_path, leaving is_fixable invisible " + "to the routing chain." + ) diff --git a/tests/recipe/test_rules_flake_loop_deadlock.py b/tests/recipe/test_rules_flake_loop_deadlock.py index 10d6553e84..71445a94a7 100644 --- a/tests/recipe/test_rules_flake_loop_deadlock.py +++ b/tests/recipe/test_rules_flake_loop_deadlock.py @@ -189,7 +189,7 @@ def test_flake_suspected_to_merge_with_context_is_clean() -> None: def test_flake_suspected_not_in_merge_cycle_is_clean() -> None: - """resolve_ci routes flake_suspected → re_push (no merge step in cycle) → no finding.""" + """flake_suspected → check_flake_loop → re_push (no merge step in cycle) → no finding.""" recipe = _make_recipe( { "resolve_ci": RecipeStep( @@ -209,13 +209,36 @@ def test_flake_suspected_not_in_merge_cycle_is_clean() -> None: on_result=StepResultRoute( conditions=[ StepResultCondition( - when="${{ result.verdict }} == 'flake_suspected'", route="re_push" + when="${{ result.verdict }} == 'flake_suspected'", + route="check_flake_loop", ), StepResultCondition(when=None, route="escalate"), ] ), on_failure="escalate", ), + "check_flake_loop": RecipeStep( + tool="run_python", + with_args={ + "callable": "autoskillit.smoke_utils.check_loop_iteration", + "current_iteration": "${{ context.ci_flake_count }}", + "max_iterations": "3", + }, + capture={ + "ci_flake_count": "${{ result.next_iteration }}", + }, + on_result=StepResultRoute( + conditions=[ + StepResultCondition( + when="${{ result.max_exceeded }} == true", + route="release_issue_failure", + ), + StepResultCondition(when=None, route="re_push"), + ] + ), + on_failure="release_issue_failure", + optional_context_refs=["ci_flake_count"], + ), "re_push": RecipeStep( tool="push_to_remote", with_args={"clone_path": "${{ context.work_dir }}", "remote_url": "origin"}, @@ -224,6 +247,9 @@ def test_flake_suspected_not_in_merge_cycle_is_clean() -> None: ), "done": RecipeStep(action="stop", with_args={}, message="done"), "escalate": RecipeStep(action="stop", with_args={}, message="escalate"), + "release_issue_failure": RecipeStep( + action="stop", with_args={}, message="release_issue_failure" + ), } ) findings = run_semantic_rules(recipe) diff --git a/tests/skills/test_resolve_failures_ci_aware.py b/tests/skills/test_resolve_failures_ci_aware.py index 275769d7ba..9679dd81ba 100644 --- a/tests/skills/test_resolve_failures_ci_aware.py +++ b/tests/skills/test_resolve_failures_ci_aware.py @@ -148,23 +148,30 @@ def _find_table_row_verdict(skill_text: str, subtype: str) -> str | None: Scans the markdown table in the Verdict Decision Tree section for a row containing the subtype, and extracts the verdict value from that row. + Returns the FIRST matching row's verdict. """ in_table = False + subtype_col = -1 + verdict_col = -1 for line in skill_text.splitlines(): if "Local result" in line and "failure_subtype" in line and "Verdict" in line: in_table = True + header_cells = [c.strip() for c in line.split("|")] + for i, cell in enumerate(header_cells): + if "failure_subtype" in cell: + subtype_col = i + if "Verdict" in cell: + verdict_col = i continue if in_table and line.strip().startswith("|---"): continue if in_table and "|" in line: cells = [c.strip() for c in line.split("|")] - # cells[0] is empty (before first |), cells[-1] is empty (after last |) - if len(cells) < 4: + if len(cells) <= max(subtype_col, verdict_col): continue - subtype_cell = cells[2] # failure_subtype column - verdict_cell = cells[3] # Verdict column + subtype_cell = cells[subtype_col] + verdict_cell = cells[verdict_col] if subtype in subtype_cell: - # Extract verdict token (backtick-wrapped) match = re.search(r"`(\w+)`", verdict_cell) if match: return match.group(1) @@ -173,27 +180,72 @@ def _find_table_row_verdict(skill_text: str, subtype: str) -> str | None: return None +def _find_all_table_row_verdicts(skill_text: str, subtype: str) -> list[str]: + """Find ALL verdicts for rows matching a given failure_subtype.""" + verdicts: list[str] = [] + in_table = False + subtype_col = -1 + verdict_col = -1 + for line in skill_text.splitlines(): + if "Local result" in line and "failure_subtype" in line and "Verdict" in line: + in_table = True + header_cells = [c.strip() for c in line.split("|")] + for i, cell in enumerate(header_cells): + if "failure_subtype" in cell: + subtype_col = i + if "Verdict" in cell: + verdict_col = i + continue + if in_table and line.strip().startswith("|---"): + continue + if in_table and "|" in line: + cells = [c.strip() for c in line.split("|")] + if len(cells) <= max(subtype_col, verdict_col): + continue + subtype_cell = cells[subtype_col] + verdict_cell = cells[verdict_col] + if subtype in subtype_cell: + match = re.search(r"`(\w+)`", verdict_cell) + if match: + verdicts.append(match.group(1)) + elif in_table and line.strip() == "": + break + return verdicts + + def test_unknown_subtype_maps_to_flake_suspected_not_ci_only(skill_text: str) -> None: - """The 'unknown' failure_subtype must map to flake_suspected, not ci_only_failure.""" - verdict = _find_table_row_verdict(skill_text, "unknown") - assert verdict is not None, ( - "resolve-failures SKILL.md verdict decision table must contain a row for 'unknown'" + """The 'unknown' failure_subtype must map to flake_suspected when is_fixable=true, + and ci_only_failure when is_fixable=false.""" + verdicts = _find_all_table_row_verdicts(skill_text, "unknown") + assert len(verdicts) >= 2, ( + f"resolve-failures SKILL.md verdict decision table must contain at least 2 rows " + f"for 'unknown' (split by is_fixable), got {len(verdicts)} row(s)" ) - assert verdict == "flake_suspected", ( - f"'unknown' subtype must map to 'flake_suspected', got '{verdict}'. " - "Ambiguous subtypes should not be routed to abort." + assert "flake_suspected" in verdicts, ( + f"'unknown' subtype must have a 'flake_suspected' row (for is_fixable=true), " + f"got verdicts: {verdicts}" + ) + assert "ci_only_failure" in verdicts, ( + f"'unknown' subtype must have a 'ci_only_failure' row (for is_fixable=false), " + f"got verdicts: {verdicts}" ) def test_env_subtype_maps_to_flake_suspected_not_ci_only(skill_text: str) -> None: - """The 'env' failure_subtype must map to flake_suspected, not ci_only_failure.""" - verdict = _find_table_row_verdict(skill_text, "env") - assert verdict is not None, ( - "resolve-failures SKILL.md verdict decision table must contain a row for 'env'" + """The 'env' failure_subtype must map to flake_suspected when is_fixable=true, + and ci_only_failure when is_fixable=false.""" + verdicts = _find_all_table_row_verdicts(skill_text, "env") + assert len(verdicts) >= 2, ( + f"resolve-failures SKILL.md verdict decision table must contain at least 2 rows " + f"for 'env' (split by is_fixable), got {len(verdicts)} row(s)" + ) + assert "flake_suspected" in verdicts, ( + f"'env' subtype must have a 'flake_suspected' row (for is_fixable=true), " + f"got verdicts: {verdicts}" ) - assert verdict == "flake_suspected", ( - f"'env' subtype must map to 'flake_suspected', got '{verdict}'. " - "Ambiguous subtypes should not be routed to abort." + assert "ci_only_failure" in verdicts, ( + f"'env' subtype must have a 'ci_only_failure' row (for is_fixable=false), " + f"got verdicts: {verdicts}" ) From 2dc9d3594183424ced588cda4a45093a77893756 Mon Sep 17 00:00:00 2001 From: Trecek Date: Fri, 26 Jun 2026 11:01:34 -0700 Subject: [PATCH 2/3] fix: remove dead is_fixable capture and narrow flake_suspected routing test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Remove is_fixable from diagnose_ci.capture in all 3 pipeline recipes. The dead-output semantic rule fires because resolve-failures reads is_fixable from the diagnosis FILE (via diagnosis_path), not from recipe context — context.is_fixable is never referenced downstream. Restrict test_resolve_ci_routes_flake_suspected_to_check_flake_loop to only check resolve-failures-invoking steps. The resolve_review step uses resolve-review skill which legitimately routes flake_suspected to pre_review_rebase, not through check_flake_loop. Update diagram hashes for all 3 recipes after YAML changes. --- .../recipes/diagrams/implementation-groups.md | 2 +- .../recipes/diagrams/implementation.md | 2 +- src/autoskillit/recipes/diagrams/remediation.md | 2 +- .../recipes/implementation-groups.json | 3 +-- .../recipes/implementation-groups.yaml | 1 - src/autoskillit/recipes/implementation.json | 3 +-- src/autoskillit/recipes/implementation.yaml | 1 - src/autoskillit/recipes/remediation.json | 3 +-- src/autoskillit/recipes/remediation.yaml | 1 - .../recipe/test_resolve_ci_routing_invariant.py | 17 +++-------------- 10 files changed, 9 insertions(+), 26 deletions(-) diff --git a/src/autoskillit/recipes/diagrams/implementation-groups.md b/src/autoskillit/recipes/diagrams/implementation-groups.md index ac9e6b0a3c..aaad7c60f7 100644 --- a/src/autoskillit/recipes/diagrams/implementation-groups.md +++ b/src/autoskillit/recipes/diagrams/implementation-groups.md @@ -1,4 +1,4 @@ - + ## implementation-groups Group-based implementation with per-group plan/implement/test cycles and PR gates. diff --git a/src/autoskillit/recipes/diagrams/implementation.md b/src/autoskillit/recipes/diagrams/implementation.md index 90bd328b12..5417ad1358 100644 --- a/src/autoskillit/recipes/diagrams/implementation.md +++ b/src/autoskillit/recipes/diagrams/implementation.md @@ -1,4 +1,4 @@ - + # implementation diff --git a/src/autoskillit/recipes/diagrams/remediation.md b/src/autoskillit/recipes/diagrams/remediation.md index cfbd15ad98..d710373053 100644 --- a/src/autoskillit/recipes/diagrams/remediation.md +++ b/src/autoskillit/recipes/diagrams/remediation.md @@ -1,4 +1,4 @@ - + ## remediation Investigate, rectify, implement, and merge a bug fix with CI and PR gates. diff --git a/src/autoskillit/recipes/implementation-groups.json b/src/autoskillit/recipes/implementation-groups.json index cc56e3a2c2..a4817457b5 100644 --- a/src/autoskillit/recipes/implementation-groups.json +++ b/src/autoskillit/recipes/implementation-groups.json @@ -1487,8 +1487,7 @@ "output_dir": "{{AUTOSKILLIT_TEMP}}/diagnose-ci" }, "capture": { - "diagnosis_path": "${{ result.diagnosis_path }}", - "is_fixable": "${{ result.is_fixable }}" + "diagnosis_path": "${{ result.diagnosis_path }}" }, "on_success": "resolve_ci", "on_failure": "resolve_ci", diff --git a/src/autoskillit/recipes/implementation-groups.yaml b/src/autoskillit/recipes/implementation-groups.yaml index 995dbc7f0c..aa6d23eb27 100644 --- a/src/autoskillit/recipes/implementation-groups.yaml +++ b/src/autoskillit/recipes/implementation-groups.yaml @@ -1340,7 +1340,6 @@ steps: output_dir: "{{AUTOSKILLIT_TEMP}}/diagnose-ci" capture: diagnosis_path: "${{ result.diagnosis_path }}" - is_fixable: "${{ result.is_fixable }}" on_success: resolve_ci on_failure: resolve_ci on_context_limit: resolve_ci diff --git a/src/autoskillit/recipes/implementation.json b/src/autoskillit/recipes/implementation.json index 2d2c565f43..f0ef050836 100644 --- a/src/autoskillit/recipes/implementation.json +++ b/src/autoskillit/recipes/implementation.json @@ -2299,8 +2299,7 @@ "output_dir": "{{AUTOSKILLIT_TEMP}}/diagnose-ci" }, "capture": { - "diagnosis_path": "${{ result.diagnosis_path }}", - "is_fixable": "${{ result.is_fixable }}" + "diagnosis_path": "${{ result.diagnosis_path }}" }, "on_success": "resolve_ci", "on_failure": "resolve_ci", diff --git a/src/autoskillit/recipes/implementation.yaml b/src/autoskillit/recipes/implementation.yaml index 4c447712f9..b47529a8a5 100644 --- a/src/autoskillit/recipes/implementation.yaml +++ b/src/autoskillit/recipes/implementation.yaml @@ -2068,7 +2068,6 @@ steps: output_dir: '{{AUTOSKILLIT_TEMP}}/diagnose-ci' capture: diagnosis_path: ${{ result.diagnosis_path }} - is_fixable: ${{ result.is_fixable }} on_success: resolve_ci on_failure: resolve_ci on_context_limit: resolve_ci diff --git a/src/autoskillit/recipes/remediation.json b/src/autoskillit/recipes/remediation.json index 411f7e15f5..d259454203 100644 --- a/src/autoskillit/recipes/remediation.json +++ b/src/autoskillit/recipes/remediation.json @@ -2435,8 +2435,7 @@ "output_dir": "{{AUTOSKILLIT_TEMP}}/diagnose-ci" }, "capture": { - "diagnosis_path": "${{ result.diagnosis_path }}", - "is_fixable": "${{ result.is_fixable }}" + "diagnosis_path": "${{ result.diagnosis_path }}" }, "on_success": "resolve_ci", "on_failure": "resolve_ci", diff --git a/src/autoskillit/recipes/remediation.yaml b/src/autoskillit/recipes/remediation.yaml index 05c513aea4..d20af042e0 100644 --- a/src/autoskillit/recipes/remediation.yaml +++ b/src/autoskillit/recipes/remediation.yaml @@ -2174,7 +2174,6 @@ steps: output_dir: '{{AUTOSKILLIT_TEMP}}/diagnose-ci' capture: diagnosis_path: ${{ result.diagnosis_path }} - is_fixable: ${{ result.is_fixable }} on_success: resolve_ci on_failure: resolve_ci on_context_limit: resolve_ci diff --git a/tests/recipe/test_resolve_ci_routing_invariant.py b/tests/recipe/test_resolve_ci_routing_invariant.py index ba48ef1195..10cd307b72 100644 --- a/tests/recipe/test_resolve_ci_routing_invariant.py +++ b/tests/recipe/test_resolve_ci_routing_invariant.py @@ -187,6 +187,9 @@ def test_resolve_ci_routes_flake_suspected_to_check_flake_loop(recipe_name: str) recipe = load_recipe(recipe_path) resolve_steps = _find_resolve_steps_reaching_push(recipe) for step_name, step in resolve_steps: + cmd = (step.with_args or {}).get("skill_command", "") + if "resolve-failures" not in cmd: + continue if step.on_result is None: continue routes = [ @@ -287,17 +290,3 @@ def test_check_flake_loop_step_exists(recipe_name: str) -> None: f"{recipe_name}: check_flake_loop max_exceeded=true must route to " f"a failure/escalation step, got routes: {max_routes}" ) - - -@pytest.mark.parametrize("recipe_name", _PIPELINE_RECIPES) -def test_diagnose_ci_captures_is_fixable(recipe_name: str) -> None: - """diagnose_ci must capture is_fixable for observability and future routing.""" - recipe_path = _RECIPES_DIR / recipe_name - recipe = load_recipe(recipe_path) - assert "diagnose_ci" in recipe.steps, f"{recipe_name}: missing 'diagnose_ci' step" - step = recipe.steps["diagnose_ci"] - assert "is_fixable" in (step.capture or {}), ( - f"{recipe_name}: diagnose_ci must capture is_fixable. " - "Currently only captures diagnosis_path, leaving is_fixable invisible " - "to the routing chain." - ) From 669c984e4b83da4f3d157dd99b2a438c67fdc626 Mon Sep 17 00:00:00 2001 From: Trecek Date: Fri, 26 Jun 2026 11:35:59 -0700 Subject: [PATCH 3/3] fix(review): guard step.with_args before subscripting in test_check_flake_loop_step_exists If a run_python step has no with: block, step.with_args is None and subscripting it raises TypeError instead of a clear AssertionError. Co-Authored-By: Claude Sonnet 4.6 --- tests/recipe/test_resolve_ci_routing_invariant.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/recipe/test_resolve_ci_routing_invariant.py b/tests/recipe/test_resolve_ci_routing_invariant.py index 10cd307b72..7d0242833b 100644 --- a/tests/recipe/test_resolve_ci_routing_invariant.py +++ b/tests/recipe/test_resolve_ci_routing_invariant.py @@ -275,6 +275,9 @@ def test_check_flake_loop_step_exists(recipe_name: str) -> None: assert step.tool == "run_python", ( f"{recipe_name}: check_flake_loop must use run_python, got {step.tool!r}" ) + assert step.with_args is not None, ( + f"{recipe_name}: check_flake_loop run_python step has no with_args block" + ) assert step.with_args["callable"] == "autoskillit.smoke_utils.check_loop_iteration", ( f"{recipe_name}: check_flake_loop must use check_loop_iteration callable" )