Skip to content

Commit 8734c63

Browse files
Trecekclaude
andauthored
[FIX] Pre-Prune Validity Computation Breaks Codex open_kitchen (#4068)
## Summary The recipe loading pipeline in `load_and_validate()` computes validity from semantic rule findings that were generated against the **pre-prune** recipe. When the Codex backend is used, the `backend-incompatible-skill` rule fires ERROR-severity findings against steps that have `skip_when_false` guards — steps that would be pruned immediately after. The stale pre-prune findings poison `compute_recipe_validity()`, marking the recipe as `valid=False`. The error response function then reads the empty `errors[]` list (structural errors only) and falls back to "unknown structural error", hiding the true cause entirely. Part A fixes the core pipeline ordering bug in `load_and_validate()` and surfaces semantic findings in the validation error response. Part B will cover adding pruning support to `validate_from_path` and its full caller chain — implement as a separate task. ## Requirements `open_kitchen(name="implementation")` fails on the first orchestrator message of every Codex-backend session in v0.10.752 with: > `Recipe 'implementation' failed structural validation: unknown structural error` (`stage=recipe_validation`, `errors=[]`, suggestions contain only the usual warnings) The fail-closed guard added by PR #3995 (`61802dce4`) gates on `result["valid"]`, but `load_and_validate` computes `valid` from semantic-rule findings evaluated on the **unpruned** recipe. With the Codex capability override (`backend_supports_git_write="false"`), the `backend-incompatible-skill` rule emits **13 ERROR-severity findings** — 9 on steps that are pruned immediately afterward (`skip_when_false: inputs.backend_supports_git_write`) and 4 on `inputs.open_pr`-route-guarded steps that the conformance test explicitly exempts via `_ROUTE_GUARDED_COMPAT_EXCEPTIONS` but `compute_recipe_validity` does not. So `valid=False` even though the served (pruned) content is structurally sound. ## Root cause chain (all empirically confirmed on installed v0.10.752) 1. `CodexBackend` declares `git_metadata_writable=False` (`execution/backends/codex.py:521`); `_backend_capability_overrides` yields `{"backend_supports_git_write": "false"}`, merged last/unoverridable (`tools_kitchen.py:560-561` via `_promote_capability_keys`). 2. **Sequencing defect (the root):** in `recipe/_api.py` (`load_and_validate`): `run_semantic_rules` runs at line 397 on the unpruned recipe → `_prune_skipped_steps` at line 408 → `compute_recipe_validity` at line 457 consumes the **pre-prune** findings. 3. `backend-incompatible-skill` (`rules/rules_backend_compat.py:27-68`) has no awareness of `skip_when_false` guards or route-guard exemptions → 13 ERROR findings for codex. 4. `compute_recipe_validity` (`registry.py:245-254`) folds any ERROR-severity semantic/contract finding into `valid` → `valid=False`, `content` non-empty, `errors=[]`. 5. #3995's guard trips on `not result.get("valid", False)`; the envelope renders only the structural `errors` list → misleading "unknown structural error". ## Scope of fix 1. **Compute validity from the post-prune recipe.** Move semantic-rule evaluation (or at minimum the validity-relevant findings derivation) after `_prune_skipped_steps`. The `ValidationContext` must be **rebuilt** with the pruned recipe — not reused from the pre-prune build. 2. **Single source of truth for route-guard exemptions.** Promote `_ROUTE_GUARDED_COMPAT_EXCEPTIONS` (currently test-only) into a production constant consumed by both the validity computation path and the conformance test. 3. **Fleet parity.** `fleet/_api.py` dispatch validation must pass the backend capability overrides so fleet's pruning matches what would actually be served. 4. **Regression tests through the real producer.** Bundled-recipe × backend validity assertions through the real `load_and_validate` with real capability overrides. ## Related work - #3992 — predecessor bug (silent empty-content delivery) that #3995 rectified - #3948 — prior incident of the same class: an ERROR-severity finding promotion blocked dispatch for 10 bundled recipes - **#4026 is hard-blocked by this issue** — asserts `valid=True` for codex × implementation/planner via `load_and_validate` with no xfail machinery - **#4008** can land first; its `xfail(strict=True)` markers must be removed after this fix lands Closes #4059 ## Implementation Plan Plan file: `/home/talon/projects/autoskillit-runs/remediation-20260611-063523-547628/.autoskillit/temp/rectify/rectify_pre_prune_validity_2026-06-11_063523_part_a.md` 🤖 Generated with [Claude Code](https://claude.com/claude-code) via AutoSkillit <!-- autoskillit:pipeline-signature steps=prepare_pr,run_arch_lenses,compose_pr,annotate_pr_diff,review_pr --> ## Token Usage Summary | Step | Model | count | uncached | output | cache_read | peak_ctx | turns | cache_write | time | |------|-------|-------|----------|--------|------------|----------|-------|-------------|------| | run_bem* | sonnet | 1 | 45.1k | 48.1k | 677.2k | 130.5k | 35 | 112.5k | 17m 18s | | dispatch:bf3ba4d0-338f-46e2-b744-1cc5af0cb74f* | sonnet | 1 | 66 | 6.1k | 324.1k | 48.4k | 17 | 29.4k | 18m 48s | | plan* | opus[1m] | 3 | 3.6k | 45.5k | 4.0M | 103.4k | 125 | 233.9k | 29m 52s | | verify* | sonnet | 3 | 993 | 27.7k | 1.9M | 84.4k | 107 | 142.7k | 11m 40s | | implement* | MiniMax-M3 | 3 | 4.2M | 23.3k | 532.7k | 64.8k | 173 | 0 | 16m 35s | | audit_impl* | sonnet | 3 | 1.1k | 21.1k | 620.0k | 45.8k | 45 | 89.0k | 10m 59s | | prepare_pr* | MiniMax-M3 | 3 | 460.3k | 6.1k | 85.0k | 44.1k | 38 | 0 | 3m 8s | | compose_pr* | MiniMax-M3 | 3 | 564.7k | 4.0k | 0 | 0 | 36 | 0 | 2m 22s | | review_pr* | sonnet | 4 | 522 | 54.9k | 2.9M | 75.1k | 150 | 204.4k | 16m 2s | | resolve_review* | sonnet | 1 | 133 | 9.9k | 833.6k | 66.4k | 37 | 46.2k | 5m 37s | | dispatch:a43619bc-64d1-412e-9f66-144a95109fd1* | sonnet | 1 | 330 | 10.2k | 3.0M | 83.5k | 86 | 137.5k | 1h 5m | | dispatch:76e5c3a5-b134-41b9-a511-2d686a4069c8* | sonnet | 1 | 362 | 14.6k | 3.4M | 88.0k | 93 | 63.6k | 38m 32s | | **Total** | | | 5.3M | 271.5k | 18.3M | 130.5k | | 1.1M | 3h 56m | \* *Step used a non-Anthropic provider; caching behavior may differ.* ## Token Efficiency | Step | LoC Changed | cache_read/LoC | cache_write/LoC | output/LoC | |------|-------------|----------------|-----------------|------------| | run_bem | 0 | — | — | — | | dispatch:bf3ba4d0-338f-46e2-b744-1cc5af0cb74f | 0 | — | — | — | | plan | 0 | — | — | — | | verify | 0 | — | — | — | | implement | 217 | 2454.8 | 0.0 | 107.5 | | audit_impl | 0 | — | — | — | | prepare_pr | 0 | — | — | — | | compose_pr | 0 | — | — | — | | review_pr | 0 | — | — | — | | resolve_review | 0 | — | — | — | | dispatch:a43619bc-64d1-412e-9f66-144a95109fd1 | 0 | — | — | — | | dispatch:76e5c3a5-b134-41b9-a511-2d686a4069c8 | 554 | 6178.5 | 114.8 | 26.3 | | **Total** | **771** | 23758.1 | 1373.7 | 352.1 | ## Model Usage Breakdown | Model | steps | uncached | output | cache_read | cache_write | time | |-------|-------|----------|--------|------------|-------------|------| | sonnet | 8 | 48.6k | 192.6k | 13.7M | 825.1k | 3h 4m | | opus[1m] | 1 | 3.6k | 45.5k | 4.0M | 233.9k | 29m 52s | | MiniMax-M3 | 3 | 5.2M | 33.4k | 617.7k | 0 | 22m 5s | --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 9bb70b0 commit 8734c63

23 files changed

Lines changed: 515 additions & 51 deletions

src/autoskillit/core/types/_type_protocols_recipe.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,7 @@ def validate_from_path(
5151
temp_dir_relpath: str = ".autoskillit/temp",
5252
*,
5353
backend_name: str | None = None,
54+
ingredient_overrides: dict[str, str] | None = None,
5455
) -> dict[str, Any]: ...
5556

5657
def list_all(

src/autoskillit/recipe/_api.py

Lines changed: 39 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,7 @@
6363
from autoskillit.recipe._rule_helpers import (
6464
_is_failure_sentinel_value,
6565
extract_sentinel_json_blocks,
66+
filter_pruning_false_positives,
6667
)
6768
from autoskillit.recipe.contracts import (
6869
check_contract_staleness,
@@ -361,7 +362,38 @@ def load_and_validate(
361362
errors.extend(f"[combined] {e}" for e in combined_errors)
362363
t0 = _t("validate_recipe_structure", t0, name)
363364

364-
# Stage: semantic rules (builds ValidationContext once — shared computation)
365+
# Stage: skip_when_false pruning (Python-side evaluation)
366+
# MUST run before semantic rules so pruned steps are never seen by
367+
# rules like backend-incompatible-skill. See test_semantic_rules_run_after_pruning.
368+
# Snapshot pre-prune semantic findings: graph-aware rules (dead-output,
369+
# capture-inversion) produce false positives when pruning changes step
370+
# graph reachability. Only findings that ALSO appear pre-prune are kept.
371+
_pre_prune_val_ctx = make_validation_context(active_recipe, backend_name=backend_name)
372+
_pre_prune_findings = run_semantic_rules(_pre_prune_val_ctx)
373+
_pre_prune_steps = dict(active_recipe.steps)
374+
active_recipe, _skip_resolutions = _prune_skipped_steps(
375+
active_recipe, ingredient_overrides, defer_unresolved
376+
)
377+
if _skip_resolutions:
378+
raw = _resolve_skip_guards_in_content(raw, _skip_resolutions, _pre_prune_steps)
379+
_assert_content_integrity(raw, _skip_resolutions, _pre_prune_steps)
380+
# Post-prune: validate that no surviving step routes to a removed step.
381+
# Must run inside try so active_recipe and errors are both in scope.
382+
_dangling_errors = _validate_no_dangling_routes(active_recipe)
383+
if _dangling_errors:
384+
errors.extend(f"[post-prune] dangling route: {e}" for e in _dangling_errors)
385+
raw = ""
386+
# Cross-check: raw YAML route refs must match the Python model exactly.
387+
# Catches any refs that the model repaired but the YAML repair pass missed.
388+
_route_consistency_errors = _validate_route_consistency(raw, active_recipe)
389+
if _route_consistency_errors:
390+
errors.extend(
391+
f"[post-prune] route consistency: {e}" for e in _route_consistency_errors
392+
)
393+
raw = ""
394+
t0 = _t("prune_skipped_steps", t0, name)
395+
396+
# Stage: semantic rules (builds ValidationContext from post-prune recipe)
365397
if lister is None:
366398
from autoskillit.workspace import DefaultSkillResolver # noqa: PLC0415
367399

@@ -398,35 +430,17 @@ def load_and_validate(
398430
semantic_suggestions = findings_to_dicts(semantic_findings)
399431
t0 = _t("semantic_rules", t0, name)
400432

433+
if _skip_resolutions and any(v is False for v in _skip_resolutions.values()):
434+
semantic_findings = filter_pruning_false_positives(
435+
semantic_findings, _pre_prune_findings
436+
)
437+
semantic_suggestions = findings_to_dicts(semantic_findings)
438+
401439
_suppressed = suppressed or []
402440
if name in _suppressed:
403441
semantic_suggestions = filter_version_rule(semantic_suggestions)
404442
suggestions.extend(semantic_suggestions)
405443

406-
# Stage: skip_when_false pruning (Python-side evaluation)
407-
_pre_prune_steps = dict(active_recipe.steps)
408-
active_recipe, _skip_resolutions = _prune_skipped_steps(
409-
active_recipe, ingredient_overrides, defer_unresolved
410-
)
411-
if _skip_resolutions:
412-
raw = _resolve_skip_guards_in_content(raw, _skip_resolutions, _pre_prune_steps)
413-
_assert_content_integrity(raw, _skip_resolutions, _pre_prune_steps)
414-
# Post-prune: validate that no surviving step routes to a removed step.
415-
# Must run inside try so active_recipe and errors are both in scope.
416-
_dangling_errors = _validate_no_dangling_routes(active_recipe)
417-
if _dangling_errors:
418-
errors.extend(f"[post-prune] dangling route: {e}" for e in _dangling_errors)
419-
raw = ""
420-
# Cross-check: raw YAML route refs must match the Python model exactly.
421-
# Catches any refs that the model repaired but the YAML repair pass missed.
422-
_route_consistency_errors = _validate_route_consistency(raw, active_recipe)
423-
if _route_consistency_errors:
424-
errors.extend(
425-
f"[post-prune] route consistency: {e}" for e in _route_consistency_errors
426-
)
427-
raw = ""
428-
t0 = _t("prune_skipped_steps", t0, name)
429-
430444
# Stage: hidden ingredient interpolation
431445
raw = _resolve_hidden_inputs_in_content(raw, active_recipe, ingredient_overrides)
432446
t0 = _t("resolve_hidden_inputs", t0, name)

src/autoskillit/recipe/_api_listing.py

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,14 +7,17 @@
77

88
from autoskillit.core import LoadResult, SkillLister, YAMLError, get_logger, load_yaml
99
from autoskillit.recipe._analysis import make_validation_context
10+
from autoskillit.recipe._recipe_composition import _prune_skipped_steps
1011
from autoskillit.recipe._recipe_ingredients import RecipeListItem
12+
from autoskillit.recipe._rule_helpers import filter_pruning_false_positives
1113
from autoskillit.recipe.contracts import load_recipe_card, validate_recipe_cards
1214
from autoskillit.recipe.io import (
1315
RecipeInfo,
1416
_parse_recipe,
1517
list_recipes,
1618
substitute_temp_placeholder,
1719
)
20+
from autoskillit.recipe.registry import RuleFinding
1821
from autoskillit.recipe.validator import (
1922
build_quality_dict,
2023
compute_recipe_validity,
@@ -74,6 +77,7 @@ def validate_from_path(
7477
*,
7578
lister: SkillLister | None = None,
7679
backend_name: str | None = None,
80+
ingredient_overrides: dict[str, str] | None = None,
7781
) -> dict[str, Any]:
7882
"""Validate a recipe YAML file at the given path.
7983
@@ -120,6 +124,19 @@ def validate_from_path(
120124

121125
recipe = _parse_recipe(data)
122126
errors = validate_recipe_structure(recipe)
127+
_skip_resolutions: dict[str, bool | None] = {}
128+
_pre_prune_findings: list[RuleFinding] = []
129+
if ingredient_overrides:
130+
pre_prune_ctx = make_validation_context(
131+
recipe,
132+
available_skills=frozenset(s.name for s in lister.list_all()),
133+
skill_resolver=_skill_resolver,
134+
backend_name=backend_name,
135+
)
136+
_pre_prune_findings = run_semantic_rules(pre_prune_ctx)
137+
recipe, _skip_resolutions = _prune_skipped_steps(
138+
recipe, ingredient_overrides, defer_unresolved=False
139+
)
123140
known_skills = frozenset(s.name for s in lister.list_all())
124141
ctx = make_validation_context(
125142
recipe,
@@ -129,6 +146,8 @@ def validate_from_path(
129146
)
130147
report = ctx.dataflow
131148
semantic_findings = run_semantic_rules(ctx)
149+
if _skip_resolutions and any(v is False for v in _skip_resolutions.values()):
150+
semantic_findings = filter_pruning_false_positives(semantic_findings, _pre_prune_findings)
132151

133152
quality = build_quality_dict(report)
134153
semantic = findings_to_dicts(semantic_findings)

src/autoskillit/recipe/_recipe_composition.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -347,7 +347,7 @@ def _prune_skipped_steps(
347347

348348
if is_truthy:
349349
new_steps = dict(working.steps)
350-
new_steps[step_name] = dataclasses.replace(step, skip_when_false=None)
350+
new_steps[step_name] = dataclasses.replace(step, skip_when_false=None, optional=False)
351351
working = dataclasses.replace(working, steps=new_steps)
352352
else:
353353
# Redirect all routes pointing to the pruned step; guard against None redirect.

src/autoskillit/recipe/_rule_helpers.py

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515

1616
if TYPE_CHECKING:
1717
from autoskillit.recipe._contracts_types import SkillContract
18+
from autoskillit.recipe.registry import RuleFinding
1819
from autoskillit.recipe.schema import CampaignDispatch, Recipe, RecipeStep
1920

2021
logger = get_logger(__name__)
@@ -268,3 +269,19 @@ def _identify_optional_output_fields(contract: SkillContract) -> set[str]:
268269
if m and m.group(1) in output_names:
269270
optional.add(m.group(1))
270271
return optional
272+
273+
274+
def filter_pruning_false_positives(
275+
post_prune_findings: list[RuleFinding],
276+
pre_prune_findings: list[RuleFinding],
277+
) -> list[RuleFinding]:
278+
"""Keep only post-prune findings that also appeared pre-prune.
279+
280+
Graph-aware rules (dead-output, capture-inversion) produce false positives
281+
when pruning changes step-graph reachability. Intersecting with the
282+
pre-prune baseline suppresses findings introduced by pruning.
283+
"""
284+
pre_prune_keys: frozenset[tuple[str, str, str]] = frozenset(
285+
(f.rule, f.step_name, f.message) for f in pre_prune_findings
286+
)
287+
return [f for f in post_prune_findings if (f.rule, f.step_name, f.message) in pre_prune_keys]

src/autoskillit/recipe/repository.py

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -123,9 +123,13 @@ def validate_from_path(
123123
temp_dir_relpath: str = ".autoskillit/temp",
124124
*,
125125
backend_name: str | None = None,
126+
ingredient_overrides: dict[str, str] | None = None,
126127
) -> dict[str, Any]:
127128
return _api.validate_from_path(
128-
script_path, temp_dir_relpath=temp_dir_relpath, backend_name=backend_name
129+
script_path,
130+
temp_dir_relpath=temp_dir_relpath,
131+
backend_name=backend_name,
132+
ingredient_overrides=ingredient_overrides,
129133
)
130134

131135
def list_all(

src/autoskillit/server/tools/tools_kitchen.py

Lines changed: 10 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -98,27 +98,25 @@ def _kitchen_failure_envelope(
9898

9999

100100
def _recipe_validation_error_response(name: str, result: dict[str, Any]) -> str:
101-
_errs: list[str] = result.get("errors", [])
102-
_error_findings = [
103-
s
104-
for s in result.get("suggestions", [])
105-
if isinstance(s, dict) and s.get("severity") == "error"
106-
]
107-
_finding_strs = [f"[{f.get('rule', '')}] {f.get('message', '')}" for f in _error_findings]
108-
if _errs and _finding_strs:
109-
_error_parts = _errs[:2] + _finding_strs[:1]
101+
_structural_errs: list[str] = result.get("errors", [])
102+
if _structural_errs:
103+
_error_parts = _structural_errs[:3]
110104
else:
111-
_error_parts = (_errs + _finding_strs)[:3]
105+
_error_parts = [
106+
f"[{s.get('rule', 'unknown-rule')}] {s.get('message', '')}"
107+
for s in result.get("suggestions", [])
108+
if isinstance(s, dict) and s.get("severity") == "error"
109+
][:3]
112110
_error_detail = "; ".join(_error_parts) if _error_parts else "unknown structural error"
113-
_label = "structural validation" if (_errs and not _error_findings) else "validation"
111+
_label = "structural validation" if _structural_errs else "validation"
114112
return json.dumps(
115113
{
116114
"success": False,
117115
"kitchen": "failed",
118116
"user_visible_message": (f"Recipe '{name}' failed {_label}: {_error_detail}"),
119117
"error": f"Recipe '{name}' failed validation: {_error_detail}",
120118
"stage": "recipe_validation",
121-
"errors": _errs,
119+
"errors": _structural_errs,
122120
"suggestions": result.get("suggestions", []),
123121
}
124122
)

src/autoskillit/server/tools/tools_recipe.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -280,6 +280,7 @@ async def validate_recipe(script_path: str) -> str:
280280
Path(script_path),
281281
temp_dir_relpath=temp_dir_display_str(tool_ctx.config.workspace.temp_dir),
282282
backend_name=tool_ctx.backend.name if tool_ctx.backend else None,
283+
ingredient_overrides=_backend_capability_overrides(tool_ctx.backend),
283284
)
284285
return json.dumps(result)
285286
except Exception as exc:

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -506,6 +506,7 @@ classified `REJECT` with `category: "arch_violation"`.
506506
| 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) |
507507
| Rule severity consistency | `test_rule_severity_consistency.py` | Direct `RuleFinding()` construction in `@semantic_rule`/`@block_rule` bodies — must use `make_finding()`/`make_block_finding()` |
508508
| 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` |
509+
| 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 |
509510

510511
When a reviewer suggestion would cause a change matching any row above, classify
511512
the finding as `REJECT` with `category: "arch_violation"` and `evidence` referencing

tests/arch/AGENTS.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,7 @@ AST enforcement, sub-package layer contracts, and architectural invariant tests.
5959
| `test_kitchen_id_assignment.py` | AST guard: ctx.kitchen_id assignment only via resolve_kitchen_id() |
6060
| `test_layer_enforcement.py` | MCP tool registry + import layer contracts + cross-package rules |
6161
| `test_pyi_stub_completeness.py` | Stub-symbol completeness: verifies core submodule public symbols appear in __init__.pyi |
62+
| `test_pipeline_ordering.py` | Structural enforcement: semantic rules must run after _prune_skipped_steps in load_and_validate |
6263
| `test_layer_markers.py` | Enforce pytestmark layer markers on all in-scope test files |
6364
| `test_conftest_env_coverage.py` | Structural guard: root conftest _clear_private_env fixture must reference AUTOSKILLIT_PRIVATE_ENV_VARS and _HEADLESS_EXCLUSIVE_VARS programmatically |
6465
| `test_make_context_env_boundary.py` | AST guard: _factory.py functions must not read AUTOSKILLIT_PRIVATE_ENV_VARS from os.environ |

0 commit comments

Comments
 (0)