diff --git a/AGENTS.md b/AGENTS.md index 11b5405c1..bff6daad3 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -33,7 +33,7 @@ A coding-agent plugin that orchestrates automated skill-driven workflows using h - `*Def` — static definition of a registered entity (e.g., `HookDef`, `PackDef`, `FeatureDef`, `RuleDef`). Typically a `NamedTuple` or `@dataclass(frozen=True)`, used as elements in a registry or lookup table. Typically lives in `core/`; stdlib-only types importable from hook scripts may live at the package root (e.g., `HookDef` in `hook_registry.py`). - `*Spec` — behavioral specification or validation rule (e.g., `ExperimentTypeSpec`, `WriteBehaviorSpec`). Typically a `@dataclass` or `TypedDict` configuring a pipeline or validation stage. Typically lives in `recipe/` or domain layers; `*Spec` types used by IL-0 core protocols live in `core/` (e.g., `WriteBehaviorSpec` in `core/types/_type_results.py`). * **Commit discipline**: Always create NEW commits. Never use `git commit --amend`, `--fixup`, or `--squash` unless the active recipe or SKILL.md explicitly requires it. This applies to all session types including headless sessions. - * **Multi-part plan green-gate invariant**: Every part of a multi-part plan must independently pass `task test-check`. If a part's changes invalidate a pre-existing test, that test must be updated, removed, or marked `xfail(strict=True)` in the same part. The `xfail(strict=True)` bridge is the canonical mechanism: `check_test_passed` ignores xfailed counts; `strict=True` forces cleanup when the xfail condition is resolved. + * **Multi-part plan green-gate invariant**: Every part of a multi-part plan must independently pass `task test-check`. If a part's changes invalidate a pre-existing test, that test must be updated, removed, or marked `xfail(strict=True)` in the same part. The `xfail(strict=True)` bridge is the canonical mechanism: `check_test_passed` ignores xfailed counts; `strict=True` forces cleanup when the xfail condition is resolved. A bridge's `reason` must cite the open tracking issue (`#NNNN`) for the deferred work — enforced by an architectural guard. A bridge whose stated exit condition is satisfied within the same PR must be removed in that same PR. #### **3.1.a. Pre-commit Hooks** diff --git a/docs/cli.md b/docs/cli.md index 8ee9e6215..101e81ce2 100644 --- a/docs/cli.md +++ b/docs/cli.md @@ -82,14 +82,17 @@ Run health checks on your setup. **Flags:** - `--output-json` — Output results as JSON -Runs 28 checks (up to 33 with fleet enabled) enumerated by `run_doctor` in -`cli/_doctor.py` — 23 numbered plus lettered sub-checks `2b`, `2c`, `2d`, -`4b`, and `7b`. The checks cover stale MCP servers, plugin registration, PATH, +Runs 38 checks (up to 44 with fleet enabled) enumerated by `run_doctor` in +`cli/doctor/__init__.py` — numbered plus lettered sub-checks `2b`, `2c`, `2d`, +`2e`, `4b`, `7b`, `7c`, and `31b`. The checks cover stale MCP servers, plugin registration, PATH, project config, secrets placement, version consistency, hook health, hook registration, hook registry drift, recipe version health, gitignore completeness, secret-scanning hook, editable install source, stale entry points, source drift, quota cache schema, process state, install classification, -update dismissal state, ambient env leaks, and feature gate consistency. See +update dismissal state, ambient env leaks, feature gate consistency, codex version, +script binary, claude binary, codex MCP timeouts, codex graduation, CLI conformance, +codex NDJSON drift, codex model-alias staleness, standing backend pin feasibility, +and local recipe validity (including the 6 fleet checks 24-29). See [installation.md](installation.md#post-install-verification) for the full table. --- diff --git a/src/autoskillit/cli/doctor/__init__.py b/src/autoskillit/cli/doctor/__init__.py index 74cb9afa1..353fc54ec 100644 --- a/src/autoskillit/cli/doctor/__init__.py +++ b/src/autoskillit/cli/doctor/__init__.py @@ -13,8 +13,10 @@ from ._doctor_config import ( _check_config_layers_for_secrets, _check_gitignore_completeness, + _check_local_recipe_validity, _check_project_config, _check_secret_scanning_hook, + _check_standing_backend_pins_feasibility, ) from ._doctor_env import ( _check_ambient_campaign_id, @@ -143,22 +145,16 @@ def run_doctor(*, output_json: bool = False) -> None: # Check 11: Editable install source directory still exists results.append(_check_editable_install_source_exists()) - # Check 12: No stale autoskillit entry points outside ~/.local/bin results.append(_check_stale_entry_points()) - # Check 13: Source version drift (network, with disk-cache TTL fallback) results.append(_check_source_version_drift()) - # Check 14: Quota cache schema version results.append(_check_quota_cache_schema()) - # Check 15: claude process state breakdown results.append(_check_claude_process_state_breakdown(backend=_backend)) - # Check 16: Install classification from direct_url.json results.append(_check_install_classification()) - # Check 17: Update-prompt dismissal state results.append(_check_update_dismissal_state()) @@ -228,6 +224,11 @@ def run_doctor(*, output_json: bool = False) -> None: # Check 36: Codex model-alias staleness results.append(_check_codex_model_alias_staleness()) + # Check 37: Standing backend pin feasibility + results.extend(_check_standing_backend_pins_feasibility()) + # Check 38: Local recipe validity + results.extend(_check_local_recipe_validity()) + # Output if output_json: print( diff --git a/src/autoskillit/cli/doctor/_doctor_config.py b/src/autoskillit/cli/doctor/_doctor_config.py index e5df2e2eb..5095525f5 100644 --- a/src/autoskillit/cli/doctor/_doctor_config.py +++ b/src/autoskillit/cli/doctor/_doctor_config.py @@ -3,6 +3,7 @@ from __future__ import annotations from pathlib import Path +from typing import Any from autoskillit.core import Severity, get_logger @@ -128,3 +129,245 @@ def _check_secret_scanning_hook(project_dir: Path) -> DoctorResult: f"({scanners}). Add one to prevent credential leaks." ) return DoctorResult(Severity.ERROR, "secret_scanning_hook", msg) + + +def _iter_backend_pins(data: dict[str, Any]) -> list[tuple[str, str, str]]: + """Yield (recipe_name, step_name, backend_name) tuples for every pin in a config layer. + + ``recipe_name`` is empty for global ``step_overrides`` pins, which are not + scoped to a single recipe. + """ + agent_backend = data.get("agent_backend") + if not isinstance(agent_backend, dict): + return [] + pins: list[tuple[str, str, str]] = [] + recipe_overrides = agent_backend.get("recipe_overrides") + if isinstance(recipe_overrides, dict): + for recipe_name, step_map in recipe_overrides.items(): + if not isinstance(step_map, dict): + continue + for step_name, backend_name in step_map.items(): + if isinstance(backend_name, str): + pins.append((str(recipe_name), str(step_name), backend_name)) + step_overrides = agent_backend.get("step_overrides") + if isinstance(step_overrides, dict): + for step_name, backend_name in step_overrides.items(): + if isinstance(backend_name, str): + pins.append(("", str(step_name), backend_name)) + return pins + + +def _check_standing_backend_pins_feasibility( + project_dir: Path | None = None, +) -> list[DoctorResult]: + """Check that every standing agent_backend pin targets a capability-feasible backend. + + Re-reads each config.yaml layer individually (mirrors + ``_check_config_layers_for_secrets``) and resolves every ``recipe_overrides`` + and ``step_overrides`` pin against the step's skill capabilities. A pin that + forces a backend lacking a hard capability the step's skill requires is + reported as an ERROR since it would fail at dispatch time. + """ + from autoskillit.core import ( + YAMLError, + describe_capability_mismatches, + load_yaml, + unsatisfied_backend_capabilities, + ) + from autoskillit.execution import get_backend + from autoskillit.recipe import find_recipe_by_name, load_recipe + from autoskillit.workspace import DefaultSkillResolver + + root = project_dir or Path.cwd() + config_paths = [ + Path.home() / ".autoskillit" / "config.yaml", + root / ".autoskillit" / "config.yaml", + ] + resolver = DefaultSkillResolver() + results: list[DoctorResult] = [] + + for config_path in config_paths: + if not config_path.is_file(): + continue + try: + data = load_yaml(config_path) or {} + except YAMLError as exc: + results.append( + DoctorResult( + Severity.WARNING, + "standing_backend_pins_feasibility", + f"Could not parse {str(config_path)!r} as YAML: {exc}", + ) + ) + continue + if not isinstance(data, dict): + continue + + for recipe_name, step_name, backend_name in _iter_backend_pins(data): + is_recipe_pin = bool(recipe_name) + dotted_key = ( + f"agent_backend.recipe_overrides.{recipe_name}.{step_name}" + if is_recipe_pin + else f"agent_backend.step_overrides.{step_name}" + ) + + try: + backend = get_backend(backend_name) + except ValueError: + results.append( + DoctorResult( + Severity.WARNING, + "standing_backend_pins_feasibility", + f"{config_path}: {dotted_key} references unknown backend " + f"{backend_name!r}.", + ) + ) + continue + + if not is_recipe_pin: + # Global step_overrides pins are not tied to a single recipe's step + # definition, so there is no skill to resolve capabilities from. + continue + + recipe_info = find_recipe_by_name(recipe_name, root) + if recipe_info is None: + results.append( + DoctorResult( + Severity.WARNING, + "standing_backend_pins_feasibility", + f"{config_path}: {dotted_key} references unknown recipe {recipe_name!r}.", + ) + ) + continue + + try: + recipe = load_recipe(recipe_info.path) + except (OSError, ValueError, YAMLError) as exc: + results.append( + DoctorResult( + Severity.WARNING, + "standing_backend_pins_feasibility", + f"{config_path}: {dotted_key} could not load recipe " + f"{recipe_name!r}: {exc}", + ) + ) + continue + + step = recipe.steps.get(step_name) + if step_name != "*" and step is None: + results.append( + DoctorResult( + Severity.WARNING, + "standing_backend_pins_feasibility", + f"{config_path}: {dotted_key} references unknown step " + f"{step_name!r} in recipe {recipe_name!r}.", + ) + ) + continue + + steps_to_check = [step] if step is not None else list(recipe.steps.values()) + for target_step in steps_to_check: + skill_name = target_step.skill_name + if not skill_name: + continue + skill_info = resolver.resolve(skill_name) + if skill_info is None: + results.append( + DoctorResult( + Severity.WARNING, + "standing_backend_pins_feasibility", + f"{config_path}: {dotted_key} references skill " + f"{skill_name!r} (step {target_step.name!r}) which could " + "not be resolved.", + ) + ) + continue + + mismatches = unsatisfied_backend_capabilities( + skill_info.uses_capabilities, backend.capabilities + ) + if mismatches: + results.append( + DoctorResult( + Severity.ERROR, + "standing_backend_pins_feasibility", + f"{config_path}: {dotted_key} pins backend {backend_name!r} " + f"for step {target_step.name!r}, but " + f"{describe_capability_mismatches(mismatches)}. " + "Remove or update this pin, or choose a backend that " + "satisfies the step's required capabilities.", + ) + ) + + if not results: + results.append( + DoctorResult( + Severity.OK, + "standing_backend_pins_feasibility", + "All standing agent_backend pins are capability-feasible.", + ) + ) + return results + + +def _check_local_recipe_validity(project_dir: Path | None = None) -> list[DoctorResult]: + """Check every recipe in .autoskillit/recipes/ passes semantic validation. + + Local project recipes are not covered by the bundled recipe test suite, so + a broken local recipe would otherwise only surface when it is dispatched. + """ + from autoskillit.core import YAMLError + from autoskillit.recipe import load_recipe, run_semantic_rules + + root = project_dir or Path.cwd() + recipes_dir = root / ".autoskillit" / "recipes" + if not recipes_dir.is_dir(): + return [DoctorResult(Severity.OK, "local_recipe_validity", "No local recipes directory")] + + results: list[DoctorResult] = [] + for yaml_path in sorted(recipes_dir.glob("*.yaml")): + try: + recipe = load_recipe(yaml_path) + except (OSError, ValueError, YAMLError) as exc: + results.append( + DoctorResult( + Severity.ERROR, + "local_recipe_validity", + f"{yaml_path}: failed to load: {exc}", + ) + ) + continue + + try: + findings = run_semantic_rules(recipe) + except Exception as exc: # noqa: BLE001 - doctor must never crash + logger.warning("local_recipe_validation_error", path=str(yaml_path), error=str(exc)) + results.append( + DoctorResult( + Severity.WARNING, + "local_recipe_validity", + f"{yaml_path}: semantic validation could not run: {exc}", + ) + ) + continue + + error_findings = [f for f in findings if f.severity == Severity.ERROR] + if error_findings: + rule_names = ", ".join(sorted({f.rule for f in error_findings})) + results.append( + DoctorResult( + Severity.ERROR, + "local_recipe_validity", + f"{yaml_path}: failed semantic validation ({rule_names}).", + ) + ) + + if not results: + results.append( + DoctorResult( + Severity.OK, + "local_recipe_validity", + "All local recipes pass semantic validation.", + ) + ) + return results diff --git a/src/autoskillit/cli/fleet/_fleet_run.py b/src/autoskillit/cli/fleet/_fleet_run.py index cd06772ea..cebf8fb9a 100644 --- a/src/autoskillit/cli/fleet/_fleet_run.py +++ b/src/autoskillit/cli/fleet/_fleet_run.py @@ -81,7 +81,7 @@ async def _execute_fleet_run( skill_resolver=ctx.skill_resolver, config_backend=ctx.config.agent_backend, ) - _effective_backend_map = _compute_effective_backend_map( + _effective_backend_map, _backend_origin_map = _compute_effective_backend_map( _raw_steps, dispatch_backend.name if dispatch_backend else None, ctx.config.providers, diff --git a/src/autoskillit/core/__init__.pyi b/src/autoskillit/core/__init__.pyi index 486d74911..b841e5ca1 100644 --- a/src/autoskillit/core/__init__.pyi +++ b/src/autoskillit/core/__init__.pyi @@ -391,6 +391,7 @@ from .types import WriteExpectedResolver as WriteExpectedResolver from .types import assert_prompt_sentinel as assert_prompt_sentinel from .types import closure_authority_spec_from_args as closure_authority_spec_from_args from .types import compute_remaining as compute_remaining +from .types import describe_capability_mismatches as describe_capability_mismatches from .types import extract_path_arg as extract_path_arg from .types import extract_positional_args as extract_positional_args from .types import extract_skill_name as extract_skill_name diff --git a/src/autoskillit/core/types/_type_constants_registries.py b/src/autoskillit/core/types/_type_constants_registries.py index 76bdb33b2..0ffd65e6c 100644 --- a/src/autoskillit/core/types/_type_constants_registries.py +++ b/src/autoskillit/core/types/_type_constants_registries.py @@ -48,6 +48,7 @@ "SkillCapabilityDef", "HardCapabilityMismatch", "SKILL_CAPABILITY_REGISTRY", + "describe_capability_mismatches", "unsatisfied_backend_capabilities", ] @@ -485,6 +486,19 @@ class HardCapabilityMismatch(NamedTuple): ) +def describe_capability_mismatches( + mismatches: tuple[HardCapabilityMismatch, ...] | list[HardCapabilityMismatch], +) -> str: + """Format capability mismatches into a human-readable string.""" + parts = [] + for m in mismatches: + parts.append( + f"{m.property_name}=True required (via '{m.capability}') " + f"but backend has {m.property_name}={m.actual_value!r}" + ) + return "; ".join(parts) + + def unsatisfied_backend_capabilities( uses_capabilities: frozenset[str], backend_capabilities: BackendCapabilities, diff --git a/src/autoskillit/core/types/_type_protocols_recipe.py b/src/autoskillit/core/types/_type_protocols_recipe.py index 84d9f6154..31fbb6f81 100644 --- a/src/autoskillit/core/types/_type_protocols_recipe.py +++ b/src/autoskillit/core/types/_type_protocols_recipe.py @@ -67,6 +67,7 @@ def load_and_validate( backend_name: str | None = None, effective_backend_map: dict[str, str] | None = None, backend_capabilities_map: dict[str, BackendCapabilities] | None = None, + backend_origin_map: dict[str, str] | None = None, ) -> dict[str, Any]: """Load and validate a recipe. @@ -83,6 +84,7 @@ def validate_from_path( ingredient_overrides: dict[str, str] | None = None, effective_backend_map: dict[str, str] | None = None, backend_capabilities_map: dict[str, BackendCapabilities] | None = None, + backend_origin_map: dict[str, str] | None = None, ) -> dict[str, Any]: ... def list_all( diff --git a/src/autoskillit/recipe/_analysis.py b/src/autoskillit/recipe/_analysis.py index cf910ad90..d43f686eb 100644 --- a/src/autoskillit/recipe/_analysis.py +++ b/src/autoskillit/recipe/_analysis.py @@ -83,6 +83,7 @@ class ValidationContext: skill_resolver: SkillResolver | None = None effective_backend_map: dict[str, str] | None = None backend_capabilities_map: dict[str, BackendCapabilities] | None = None + backend_origin_map: dict[str, str] | None = None blocks: tuple[RecipeBlock, ...] = field(default_factory=tuple) predecessors: dict[str, set[str]] = field(default_factory=dict) @@ -137,6 +138,7 @@ def make_validation_context( skill_resolver: SkillResolver | None = None, effective_backend_map: dict[str, str] | None = None, backend_capabilities_map: dict[str, BackendCapabilities] | None = None, + backend_origin_map: dict[str, str] | None = None, ) -> ValidationContext: """Build a ``ValidationContext`` from a recipe. @@ -166,6 +168,7 @@ def make_validation_context( skill_resolver=skill_resolver, effective_backend_map=effective_backend_map, backend_capabilities_map=backend_capabilities_map, + backend_origin_map=backend_origin_map, blocks=extract_blocks(recipe, step_graph, predecessors=predecessors), predecessors=predecessors, ) diff --git a/src/autoskillit/recipe/_api.py b/src/autoskillit/recipe/_api.py index cb1554355..982ae394b 100644 --- a/src/autoskillit/recipe/_api.py +++ b/src/autoskillit/recipe/_api.py @@ -201,6 +201,7 @@ def load_and_validate( backend_name: str | None = None, effective_backend_map: dict[str, str] | None = None, backend_capabilities_map: dict[str, BackendCapabilities] | None = None, + backend_origin_map: dict[str, str] | None = None, ) -> LoadRecipeResult: """Load a recipe by name and run full validation. @@ -398,6 +399,7 @@ def load_and_validate( skill_resolver=_skill_resolver, effective_backend_map=effective_backend_map, backend_capabilities_map=backend_capabilities_map, + backend_origin_map=backend_origin_map, ) _pre_prune_findings = run_semantic_rules(_pre_prune_val_ctx) _pre_prune_steps = dict(active_recipe.steps) @@ -468,6 +470,7 @@ def load_and_validate( backend_name=backend_name, effective_backend_map=effective_backend_map, backend_capabilities_map=backend_capabilities_map, + backend_origin_map=backend_origin_map, ) semantic_findings = run_semantic_rules(val_ctx) semantic_suggestions = findings_to_dicts(semantic_findings) diff --git a/src/autoskillit/recipe/_api_listing.py b/src/autoskillit/recipe/_api_listing.py index e17a8bdba..888477fd0 100644 --- a/src/autoskillit/recipe/_api_listing.py +++ b/src/autoskillit/recipe/_api_listing.py @@ -87,6 +87,7 @@ def validate_from_path( ingredient_overrides: dict[str, str] | None = None, effective_backend_map: dict[str, str] | None = None, backend_capabilities_map: dict[str, BackendCapabilities] | None = None, + backend_origin_map: dict[str, str] | None = None, ) -> dict[str, Any]: """Validate a recipe YAML file at the given path. @@ -143,6 +144,7 @@ def validate_from_path( backend_name=backend_name, effective_backend_map=effective_backend_map, backend_capabilities_map=backend_capabilities_map, + backend_origin_map=backend_origin_map, ) _pre_prune_findings = run_semantic_rules(pre_prune_ctx) recipe, _skip_resolutions = _prune_skipped_steps( @@ -161,6 +163,7 @@ def validate_from_path( backend_name=backend_name, effective_backend_map=effective_backend_map, backend_capabilities_map=backend_capabilities_map, + backend_origin_map=backend_origin_map, ) report = ctx.dataflow semantic_findings = run_semantic_rules(ctx) diff --git a/src/autoskillit/recipe/registry.py b/src/autoskillit/recipe/registry.py index 24200abb2..f5d58ea6a 100644 --- a/src/autoskillit/recipe/registry.py +++ b/src/autoskillit/recipe/registry.py @@ -29,14 +29,21 @@ class RuleFinding: severity: Severity step_name: str message: str + origin: str | None = None + remedy: str | None = None def to_dict(self) -> dict[str, str]: - return { + d = { "rule": self.rule, "severity": self.severity.value, "step": self.step_name, "message": self.message, } + if self.origin is not None: + d["origin"] = self.origin + if self.remedy is not None: + d["remedy"] = self.remedy + return d @dataclass(frozen=True, slots=True) @@ -106,6 +113,8 @@ def make_finding( message: str, *, severity: Severity | None = None, + origin: str | None = None, + remedy: str | None = None, ) -> RuleFinding: """Create a RuleFinding using the registered severity for the named rule. @@ -121,6 +130,8 @@ def make_finding( severity=severity if severity is not None else rule_def.severity, step_name=step_name, message=message, + origin=origin, + remedy=remedy, ) diff --git a/src/autoskillit/recipe/repository.py b/src/autoskillit/recipe/repository.py index 7c897cf21..9a27b7eaa 100644 --- a/src/autoskillit/recipe/repository.py +++ b/src/autoskillit/recipe/repository.py @@ -98,6 +98,7 @@ def load_and_validate( backend_name: str | None = None, effective_backend_map: dict[str, str] | None = None, backend_capabilities_map: dict[str, BackendCapabilities] | None = None, + backend_origin_map: dict[str, str] | None = None, ) -> dict[str, Any]: project_dir = Path(project_dir) result = self._get_list(project_dir) @@ -118,6 +119,7 @@ def load_and_validate( backend_name=backend_name, effective_backend_map=effective_backend_map, backend_capabilities_map=backend_capabilities_map, + backend_origin_map=backend_origin_map, ), ) @@ -130,6 +132,7 @@ def validate_from_path( ingredient_overrides: dict[str, str] | None = None, effective_backend_map: dict[str, str] | None = None, backend_capabilities_map: dict[str, BackendCapabilities] | None = None, + backend_origin_map: dict[str, str] | None = None, ) -> dict[str, Any]: return _api.validate_from_path( script_path, @@ -138,6 +141,7 @@ def validate_from_path( ingredient_overrides=ingredient_overrides, effective_backend_map=effective_backend_map, backend_capabilities_map=backend_capabilities_map, + backend_origin_map=backend_origin_map, ) def list_all( diff --git a/src/autoskillit/recipe/rules/rules_backend_compat.py b/src/autoskillit/recipe/rules/rules_backend_compat.py index a82a143da..bab97bd05 100644 --- a/src/autoskillit/recipe/rules/rules_backend_compat.py +++ b/src/autoskillit/recipe/rules/rules_backend_compat.py @@ -7,6 +7,7 @@ SKILL_CAPABILITY_REGISTRY, SKILL_TOOLS, Severity, + describe_capability_mismatches, unsatisfied_backend_capabilities, ) from autoskillit.recipe._analysis import ValidationContext @@ -54,6 +55,16 @@ def _check_backend_incompatible_skill(ctx: ValidationContext) -> list[RuleFindin if step_backend is None: continue uses_caps: frozenset[str] = getattr(skill_info, "uses_capabilities", frozenset()) + _step_origin = (ctx.backend_origin_map or {}).get(step_name) + _step_remedy = ( + ( + f"Remove or change '{_step_origin}' in ~/.autoskillit/config.yaml " + "or /.autoskillit/config.yaml, or pin a backend with the " + "required capability." + ) + if _step_origin + else None + ) if skill_info.backend_requirements and step_backend not in skill_info.backend_requirements: cap_def = SKILL_CAPABILITY_REGISTRY.get(_GIT_METADATA_WRITE_CAP) _required = CLAUDE_CODE_CAPABILITIES.git_metadata_writable @@ -71,6 +82,8 @@ def _check_backend_incompatible_skill(ctx: ValidationContext) -> list[RuleFindin f"{sorted(skill_info.backend_requirements)} but recipe targets " f"backend '{step_backend}'.{git_detail}" ), + origin=_step_origin, + remedy=_step_remedy, ) ) # Independent hard-capability property check (sibling if, not elif) — @@ -80,17 +93,19 @@ def _check_backend_incompatible_skill(ctx: ValidationContext) -> list[RuleFindin # diagnostic from backend_requirements — both findings surface together. step_backend_caps = (ctx.backend_capabilities_map or {}).get(step_backend) if step_backend_caps and uses_caps: - for mismatch in unsatisfied_backend_capabilities(uses_caps, step_backend_caps): + mismatches = unsatisfied_backend_capabilities(uses_caps, step_backend_caps) + if mismatches: findings.append( make_finding( rule_name="backend-incompatible-skill", step_name=step_name, message=( - f"step '{step_name}': skill '{skill_name}' requires " - f"{mismatch.property_name}=True (via capability " - f"'{mismatch.capability}') but backend '{step_backend}' has " - f"{mismatch.property_name}={mismatch.actual_value!r}." + f"step '{step_name}': skill '{skill_name}' requires backend " + f"'{step_backend}' to satisfy: " + f"{describe_capability_mismatches(mismatches)}." ), + origin=_step_origin, + remedy=_step_remedy, ) ) return findings diff --git a/src/autoskillit/recipes/diagrams/implementation-groups.md b/src/autoskillit/recipes/diagrams/implementation-groups.md index c33b3e7f4..8a66dd7ac 100644 --- a/src/autoskillit/recipes/diagrams/implementation-groups.md +++ b/src/autoskillit/recipes/diagrams/implementation-groups.md @@ -1,4 +1,4 @@ - + ## implementation-groups diff --git a/src/autoskillit/recipes/diagrams/implementation.md b/src/autoskillit/recipes/diagrams/implementation.md index ddf3c71ed..793efa38c 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 bcaf029cb..11e97ed38 100644 --- a/src/autoskillit/recipes/diagrams/remediation.md +++ b/src/autoskillit/recipes/diagrams/remediation.md @@ -1,4 +1,4 @@ - + ## remediation diff --git a/src/autoskillit/recipes/full-audit.json b/src/autoskillit/recipes/full-audit.json index 2c737e6c3..134132378 100644 --- a/src/autoskillit/recipes/full-audit.json +++ b/src/autoskillit/recipes/full-audit.json @@ -39,7 +39,7 @@ "step_name": "checkout" }, "note": "Ensure the working directory is on the target branch before audits run. If the checkout fails (e.g., dirty working tree, unknown branch), route to escalate — audits cannot proceed on the wrong branch.\n", - "on_success": "run_audits", + "on_success": "init_audit_run", "on_failure": "escalate_stop", "retries": 0, "on_exhausted": "escalate_stop" diff --git a/src/autoskillit/recipes/full-audit.yaml b/src/autoskillit/recipes/full-audit.yaml index d086b9446..9e0609291 100644 --- a/src/autoskillit/recipes/full-audit.yaml +++ b/src/autoskillit/recipes/full-audit.yaml @@ -49,7 +49,7 @@ steps: Ensure the working directory is on the target branch before audits run. If the checkout fails (e.g., dirty working tree, unknown branch), route to escalate — audits cannot proceed on the wrong branch. - on_success: run_audits + on_success: init_audit_run on_failure: escalate_stop retries: 0 on_exhausted: escalate_stop diff --git a/src/autoskillit/recipes/implementation-groups.json b/src/autoskillit/recipes/implementation-groups.json index db3323f65..e8f9d2c7d 100644 --- a/src/autoskillit/recipes/implementation-groups.json +++ b/src/autoskillit/recipes/implementation-groups.json @@ -566,9 +566,22 @@ "capture": { "merge_test_fix_loop_count": "${{ result.value }}" }, + "on_success": "reset_merge_fix_counter", + "on_failure": "reset_merge_fix_counter", + "note": "Resets merge_test_fix_loop_count to 0 so each audit-remediation cycle gets a fresh merge-gate fix budget." + }, + "reset_merge_fix_counter": { + "tool": "run_python", + "with": { + "callable": "autoskillit.smoke_utils.init_counter", + "counter_value": "" + }, + "capture": { + "merge_fix_count": "${{ result.value }}" + }, "on_success": "reset_ref_push_counter", "on_failure": "reset_ref_push_counter", - "note": "Resets merge_test_fix_loop_count to 0 so each audit-remediation cycle gets a fresh merge-gate fix budget." + "note": "Resets merge_fix_count to 0 so each audit-remediation cycle gets a fresh merge-fix budget (dirty-tree, rebase, dirty-main retries)." }, "reset_ref_push_counter": { "tool": "run_python", diff --git a/src/autoskillit/recipes/implementation-groups.yaml b/src/autoskillit/recipes/implementation-groups.yaml index 2bc3bd187..81431b635 100644 --- a/src/autoskillit/recipes/implementation-groups.yaml +++ b/src/autoskillit/recipes/implementation-groups.yaml @@ -525,12 +525,25 @@ steps: counter_value: "" capture: merge_test_fix_loop_count: "${{ result.value }}" - on_success: reset_ref_push_counter - on_failure: reset_ref_push_counter + on_success: reset_merge_fix_counter + on_failure: reset_merge_fix_counter note: >- Resets merge_test_fix_loop_count to 0 so each audit-remediation cycle gets a fresh merge-gate fix budget. + reset_merge_fix_counter: + tool: run_python + with: + callable: "autoskillit.smoke_utils.init_counter" + counter_value: "" + capture: + merge_fix_count: "${{ result.value }}" + on_success: reset_ref_push_counter + on_failure: reset_ref_push_counter + note: >- + Resets merge_fix_count to 0 so each audit-remediation cycle gets a + fresh merge-fix budget (dirty-tree, rebase, dirty-main retries). + reset_ref_push_counter: tool: run_python with: diff --git a/src/autoskillit/recipes/implementation.json b/src/autoskillit/recipes/implementation.json index 8b88cf76c..9098bcca6 100644 --- a/src/autoskillit/recipes/implementation.json +++ b/src/autoskillit/recipes/implementation.json @@ -607,9 +607,22 @@ "capture": { "merge_test_fix_loop_count": "${{ result.value }}" }, + "on_success": "reset_merge_fix_counter", + "on_failure": "reset_merge_fix_counter", + "note": "Resets merge_test_fix_loop_count to 0 so each audit-remediation cycle gets a fresh merge-gate fix budget." + }, + "reset_merge_fix_counter": { + "tool": "run_python", + "with": { + "callable": "autoskillit.smoke_utils.init_counter", + "counter_value": "" + }, + "capture": { + "merge_fix_count": "${{ result.value }}" + }, "on_success": "reset_ref_push_counter", "on_failure": "reset_ref_push_counter", - "note": "Resets merge_test_fix_loop_count to 0 so each audit-remediation cycle gets a fresh merge-gate fix budget." + "note": "Resets merge_fix_count to 0 so each audit-remediation cycle gets a fresh merge-fix budget (dirty-tree, rebase, dirty-main retries)." }, "reset_ref_push_counter": { "tool": "run_python", diff --git a/src/autoskillit/recipes/implementation.yaml b/src/autoskillit/recipes/implementation.yaml index 5c4e26981..49b743ecd 100644 --- a/src/autoskillit/recipes/implementation.yaml +++ b/src/autoskillit/recipes/implementation.yaml @@ -578,12 +578,25 @@ steps: counter_value: '' capture: merge_test_fix_loop_count: ${{ result.value }} - on_success: reset_ref_push_counter - on_failure: reset_ref_push_counter + on_success: reset_merge_fix_counter + on_failure: reset_merge_fix_counter note: >- Resets merge_test_fix_loop_count to 0 so each audit-remediation cycle gets a fresh merge-gate fix budget. + reset_merge_fix_counter: + tool: run_python + with: + callable: autoskillit.smoke_utils.init_counter + counter_value: '' + capture: + merge_fix_count: ${{ result.value }} + on_success: reset_ref_push_counter + on_failure: reset_ref_push_counter + note: >- + Resets merge_fix_count to 0 so each audit-remediation cycle gets a + fresh merge-fix budget (dirty-tree, rebase, dirty-main retries). + reset_ref_push_counter: tool: run_python with: diff --git a/src/autoskillit/recipes/remediation.json b/src/autoskillit/recipes/remediation.json index 0bb465384..746e9de1e 100644 --- a/src/autoskillit/recipes/remediation.json +++ b/src/autoskillit/recipes/remediation.json @@ -646,9 +646,22 @@ "capture": { "merge_test_fix_loop_count": "${{ result.value }}" }, + "on_success": "reset_merge_fix_counter", + "on_failure": "reset_merge_fix_counter", + "note": "Resets merge_test_fix_loop_count to 0 so each audit-remediation cycle gets a fresh merge-gate fix budget." + }, + "reset_merge_fix_counter": { + "tool": "run_python", + "with": { + "callable": "autoskillit.smoke_utils.init_counter", + "counter_value": "" + }, + "capture": { + "merge_fix_count": "${{ result.value }}" + }, "on_success": "reset_ref_push_counter", "on_failure": "reset_ref_push_counter", - "note": "Resets merge_test_fix_loop_count to 0 so each audit-remediation cycle gets a fresh merge-gate fix budget." + "note": "Resets merge_fix_count to 0 so each audit-remediation cycle gets a fresh merge-fix budget (dirty-tree, rebase, dirty-main retries)." }, "reset_ref_push_counter": { "tool": "run_python", @@ -1162,7 +1175,7 @@ }, { "when": "result.failed_step == 'rebase'", - "route": "release_issue_failure" + "route": "check_merge_rebase_loop" }, { "when": "result.failed_step == 'dirty_main_repo'", diff --git a/src/autoskillit/recipes/remediation.yaml b/src/autoskillit/recipes/remediation.yaml index 7c86191e5..61986a6c0 100644 --- a/src/autoskillit/recipes/remediation.yaml +++ b/src/autoskillit/recipes/remediation.yaml @@ -622,12 +622,25 @@ steps: counter_value: '' capture: merge_test_fix_loop_count: ${{ result.value }} - on_success: reset_ref_push_counter - on_failure: reset_ref_push_counter + on_success: reset_merge_fix_counter + on_failure: reset_merge_fix_counter note: >- Resets merge_test_fix_loop_count to 0 so each audit-remediation cycle gets a fresh merge-gate fix budget. + reset_merge_fix_counter: + tool: run_python + with: + callable: autoskillit.smoke_utils.init_counter + counter_value: '' + capture: + merge_fix_count: ${{ result.value }} + on_success: reset_ref_push_counter + on_failure: reset_ref_push_counter + note: >- + Resets merge_fix_count to 0 so each audit-remediation cycle gets a + fresh merge-fix budget (dirty-tree, rebase, dirty-main retries). + reset_ref_push_counter: tool: run_python with: @@ -1059,7 +1072,7 @@ steps: - when: result.failed_step == 'post_rebase_test_gate' route: release_issue_failure - when: result.failed_step == 'rebase' - route: release_issue_failure + route: check_merge_rebase_loop - when: result.failed_step == 'dirty_main_repo' route: check_dirty_main_retry - when: result.failed_step == 'ref_coherence' and result.remote_is_ancestor == true diff --git a/src/autoskillit/server/_guards.py b/src/autoskillit/server/_guards.py index f0cca8570..08c451af9 100644 --- a/src/autoskillit/server/_guards.py +++ b/src/autoskillit/server/_guards.py @@ -4,7 +4,7 @@ import os from pathlib import Path -from typing import TYPE_CHECKING, Any, assert_never +from typing import TYPE_CHECKING, Any, NamedTuple, assert_never import regex as re @@ -464,15 +464,24 @@ def _resolve_provider_profile( return (name, _profile_to_env(profile)) +class BackendPinResolution(NamedTuple): + """Result of resolving an explicit backend override from config.""" + + backend: str + tier: str + key_path: str + + def _resolve_backend_override( step_name: str, recipe_name: str, config_backend: AgentBackendConfig, -) -> str | None: +) -> BackendPinResolution | None: """Resolve explicit backend override from config. - Returns the backend name if an explicit override matches, or ``None`` - to fall through to capability/provider routing. + Returns a ``BackendPinResolution`` with backend name, tier, and the + dotted config key path, or ``None`` to fall through to + capability/provider routing. Precedence (highest first): Tier 0: ``recipe_overrides[recipe][step]`` (exact) @@ -495,7 +504,11 @@ def _resolve_backend_override( step=step_name, backend=recipe_map[step_name], ) - return recipe_map[step_name] + return BackendPinResolution( + recipe_map[step_name], + "recipe_step", + f"agent_backend.recipe_overrides.{recipe_name}.{step_name}", + ) if "*" in recipe_map: logger.debug( "backend_override_resolved", @@ -504,7 +517,11 @@ def _resolve_backend_override( step=step_name, backend=recipe_map["*"], ) - return recipe_map["*"] + return BackendPinResolution( + recipe_map["*"], + "recipe_wildcard", + f"agent_backend.recipe_overrides.{recipe_name}.*", + ) if recipe_name and step_name and step_name in config_backend.step_overrides: logger.debug( @@ -513,7 +530,11 @@ def _resolve_backend_override( step=step_name, backend=config_backend.step_overrides[step_name], ) - return config_backend.step_overrides[step_name] + return BackendPinResolution( + config_backend.step_overrides[step_name], + "step_override", + f"agent_backend.step_overrides.{step_name}", + ) if recipe_name and "*" in config_backend.step_overrides: logger.debug( @@ -522,7 +543,11 @@ def _resolve_backend_override( step=step_name, backend=config_backend.step_overrides["*"], ) - return config_backend.step_overrides["*"] + return BackendPinResolution( + config_backend.step_overrides["*"], + "step_wildcard", + "agent_backend.step_overrides.*", + ) return None diff --git a/src/autoskillit/server/tools/_auto_overrides.py b/src/autoskillit/server/tools/_auto_overrides.py index c5b23b257..a7e009371 100644 --- a/src/autoskillit/server/tools/_auto_overrides.py +++ b/src/autoskillit/server/tools/_auto_overrides.py @@ -113,7 +113,10 @@ def _provider_aware_capability_overrides( if config_backend is not None: from autoskillit.server._guards import _resolve_backend_override # circular-break - _explicit_be = _resolve_backend_override(step_name, recipe_name, config_backend) + _explicit_resolution = _resolve_backend_override( + step_name, recipe_name, config_backend + ) + _explicit_be = _explicit_resolution.backend if _explicit_resolution else None if _explicit_be == AGENT_BACKEND_CLAUDE_CODE: cap_resolved = skill_resolver.resolve(skill_name) if cap_resolved: @@ -233,7 +236,7 @@ def _compute_effective_backend_map( *, skill_resolver: SkillResolver | None = None, config_backend: AgentBackendConfig | None = None, -) -> dict[str, str] | None: +) -> tuple[dict[str, str] | None, dict[str, str]]: """Build a per-step effective backend map mirroring ``tools_execution`` dispatch logic. For each ``run_skill`` step, returns the backend that ``run_skill`` would dispatch @@ -255,10 +258,11 @@ def _compute_effective_backend_map( (REQ-ADMIT-002). """ if backend_name is None or recipe_steps is None: - return None + return None, {} from autoskillit.server._guards import _resolve_provider_profile # circular-break result: dict[str, str] = {} + origin_map: dict[str, str] = {} for step_name, step in recipe_steps.items(): if getattr(step, "tool", None) not in SKILL_TOOLS: continue @@ -270,7 +274,8 @@ def _compute_effective_backend_map( _explicit = _resolve_backend_override(step_name, recipe_name, config_backend) if _explicit is not None: - result[step_name] = _explicit + result[step_name] = _explicit.backend + origin_map[step_name] = _explicit.key_path continue has_base_url = False @@ -307,4 +312,4 @@ def _compute_effective_backend_map( continue result[step_name] = backend_name - return result if result else None + return (result if result else None), origin_map diff --git a/src/autoskillit/server/tools/_preflight.py b/src/autoskillit/server/tools/_preflight.py index 33ae99beb..cbbf935b1 100644 --- a/src/autoskillit/server/tools/_preflight.py +++ b/src/autoskillit/server/tools/_preflight.py @@ -6,7 +6,11 @@ from pathlib import Path from typing import TYPE_CHECKING, Any -from autoskillit.core import CodingAgentBackend, unsatisfied_backend_capabilities +from autoskillit.core import ( + CodingAgentBackend, + describe_capability_mismatches, + unsatisfied_backend_capabilities, +) from autoskillit.hook_registry import HOOK_REGISTRY from autoskillit.server._misc import get_backend @@ -40,12 +44,7 @@ def check_hard_capability_feasibility( """ mismatches = unsatisfied_backend_capabilities(uses_capabilities, backend.capabilities) if mismatches: - mismatch = mismatches[0] - return ( - f"Capability '{mismatch.capability}' requires backend property " - f"'{mismatch.property_name}' to be True, but backend '{backend.name}' has " - f"{mismatch.property_name}={mismatch.actual_value!r}." - ) + return f"Backend '{backend.name}': {describe_capability_mismatches(mismatches)}." return None @@ -110,8 +109,9 @@ def _check_dispatch_feasibility( # for git_metadata_write skills), dispatch is infeasible and the # gate refuses the recipe. if config_backend is not None: - _explicit = _resolve_backend_override(step_name, recipe_name, config_backend) - if _explicit is not None: + _resolution = _resolve_backend_override(step_name, recipe_name, config_backend) + if _resolution is not None: + _explicit = _resolution.backend try: _pinned_backend = get_backend(_explicit) except (ValueError, KeyError): @@ -178,7 +178,13 @@ def _check_dispatch_feasibility( "stage": "dispatch_feasibility_preflight", "backend": _explicit, "step": step_name, - "override_source": "explicit_config", + "origin": _resolution.key_path, + "remedy": ( + f"Remove or change '{_resolution.key_path}' in " + "~/.autoskillit/config.yaml or " + "/.autoskillit/config.yaml, or pin a " + "backend with the required capability." + ), } ) continue diff --git a/src/autoskillit/server/tools/_serve_helpers.py b/src/autoskillit/server/tools/_serve_helpers.py index e8438a5d0..c77fbcf84 100644 --- a/src/autoskillit/server/tools/_serve_helpers.py +++ b/src/autoskillit/server/tools/_serve_helpers.py @@ -148,6 +148,7 @@ def serve_recipe( temp_dir: Path | str | None = None, temp_dir_relpath: str | None = None, backend_capabilities_map: dict[str, BackendCapabilities] | None = None, + backend_origin_map: dict[str, str] | None = None, ) -> dict[str, Any]: """Unified recipe serve path. Only legal caller of load_and_validate in server/tools/.""" ingredient_overrides = _build_serve_override_stack( @@ -169,6 +170,8 @@ def serve_recipe( kwargs["effective_backend_map"] = effective_backend_map if backend_capabilities_map is not None: kwargs["backend_capabilities_map"] = backend_capabilities_map + if backend_origin_map is not None: + kwargs["backend_origin_map"] = backend_origin_map if suppressed is not None: kwargs["suppressed"] = suppressed if backend_name is not None: diff --git a/src/autoskillit/server/tools/tools_execution.py b/src/autoskillit/server/tools/tools_execution.py index c96ac9312..278d038d8 100644 --- a/src/autoskillit/server/tools/tools_execution.py +++ b/src/autoskillit/server/tools/tools_execution.py @@ -910,11 +910,14 @@ async def run_skill( # capability-driven routing for this step (REQ-RES-001). from autoskillit.server._guards import _resolve_backend_override # circular-break - _explicit_backend_override: str | None = _resolve_backend_override( + _explicit_resolution = _resolve_backend_override( step_name or "", tool_ctx.recipe_name or "", _cfg.agent_backend, ) + _explicit_backend_override: str | None = ( + _explicit_resolution.backend if _explicit_resolution else None + ) _skill_caps: frozenset[str] = ( getattr(_skill_info, "uses_capabilities", frozenset()) @@ -999,8 +1002,8 @@ async def run_skill( ) _backend_override_source: str | None = None - if _explicit_backend_override is not None: - _backend_override_source = "explicit_config" + if _explicit_resolution is not None: + _backend_override_source = _explicit_resolution.key_path elif _skill_requires_claude: _backend_override_source = "skill_requirement" elif _provider_override: diff --git a/src/autoskillit/server/tools/tools_fleet_dispatch.py b/src/autoskillit/server/tools/tools_fleet_dispatch.py index 833897b80..55041e1e2 100755 --- a/src/autoskillit/server/tools/tools_fleet_dispatch.py +++ b/src/autoskillit/server/tools/tools_fleet_dispatch.py @@ -366,7 +366,7 @@ async def dispatch_food_truck( config_backend=tool_ctx.config.agent_backend, ) _merged_ingredients = {**(ingredients or {}), **_capability_overrides} - _effective_backend_map = _compute_effective_backend_map( + _effective_backend_map, _backend_origin_map = _compute_effective_backend_map( _preflight_raw_steps, _override_backend.name if _override_backend else None, tool_ctx.config.providers, diff --git a/src/autoskillit/server/tools/tools_kitchen.py b/src/autoskillit/server/tools/tools_kitchen.py index e55915db2..8825ac571 100644 --- a/src/autoskillit/server/tools/tools_kitchen.py +++ b/src/autoskillit/server/tools/tools_kitchen.py @@ -129,11 +129,15 @@ def _recipe_validation_error_response(name: str, result: dict[str, Any]) -> str: if len(_structural_errs) > 3: _error_parts.append(f"+{len(_structural_errs) - 3} more errors") else: - _all_errors = [ - f"[{s.get('rule', 'unknown-rule')}] {s.get('message', '')}" - for s in result.get("suggestions", []) - if isinstance(s, dict) and s.get("severity") == "error" - ] + _all_errors = [] + for s in result.get("suggestions", []): + if isinstance(s, dict) and s.get("severity") == "error": + _line = f"[{s.get('rule', 'unknown-rule')}] {s.get('message', '')}" + if s.get("origin"): + _line += f" (origin: {s['origin']})" + if s.get("remedy"): + _line += f" — remedy: {s['remedy']}" + _all_errors.append(_line) _error_parts = _all_errors[:3] if len(_all_errors) > 3: _error_parts.append(f"+{len(_all_errors) - 3} more errors") @@ -590,7 +594,7 @@ def get_recipe(name: str) -> str: skill_resolver=ctx.skill_resolver, config_backend=ctx.config.agent_backend, ) - _effective_backend_map = _compute_effective_backend_map( + _effective_backend_map, _backend_origin_map = _compute_effective_backend_map( _raw_recipe.steps, ctx.backend.name if ctx.backend else None, ctx.config.providers, @@ -614,6 +618,7 @@ def get_recipe(name: str) -> str: effective_backend_map=_effective_backend_map, backend_name=ctx.backend.name if ctx.backend else None, backend_capabilities_map=_backend_capabilities_map, + backend_origin_map=_backend_origin_map, ) except ProcessStaleError: logger.warning("get_recipe_failure", recipe=name, stage="process_stale", exc_info=True) @@ -835,7 +840,7 @@ async def open_kitchen( _config_layer = build_config_authoritative_layer(_defaults) _config_default = build_config_default_layer(_defaults) _promote_capability_keys(_config_layer, _session_overrides) - _effective_backend_map = _compute_effective_backend_map( + _effective_backend_map, _backend_origin_map = _compute_effective_backend_map( _raw_recipe.steps if _raw_recipe is not None else None, tool_ctx.backend.name if tool_ctx.backend else None, tool_ctx.config.providers, @@ -872,6 +877,7 @@ async def open_kitchen( backend_name=tool_ctx.backend.name if tool_ctx.backend else None, effective_backend_map=_effective_backend_map, backend_capabilities_map=_backend_capabilities_map, + backend_origin_map=_backend_origin_map, ) except ProcessStaleError as exc: logger.warning("open_kitchen_failure", stage="process_stale", exc_info=True) @@ -983,6 +989,7 @@ async def open_kitchen( backend_name=tool_ctx.backend.name if tool_ctx.backend else None, effective_backend_map=_effective_backend_map, backend_capabilities_map=_backend_capabilities_map, + backend_origin_map=_backend_origin_map, ) except ProcessStaleError as exc: logger.warning("open_kitchen_failure", stage="process_stale", exc_info=True) diff --git a/src/autoskillit/server/tools/tools_recipe.py b/src/autoskillit/server/tools/tools_recipe.py index 7e3ebaa7b..82cb6d777 100644 --- a/src/autoskillit/server/tools/tools_recipe.py +++ b/src/autoskillit/server/tools/tools_recipe.py @@ -247,7 +247,7 @@ async def load_recipe( _config_layer = build_config_authoritative_layer(_defaults) _config_default = build_config_default_layer(_defaults) _promote_capability_keys(_config_layer, _session_overrides) - _effective_backend_map = _compute_effective_backend_map( + _effective_backend_map, _backend_origin_map = _compute_effective_backend_map( _raw_recipe_obj.steps if _raw_recipe_obj is not None else None, tool_ctx.backend.name if tool_ctx.backend else None, tool_ctx.config.providers, @@ -272,6 +272,7 @@ async def load_recipe( backend_name=tool_ctx.backend.name if tool_ctx.backend else None, effective_backend_map=_effective_backend_map, backend_capabilities_map=_backend_capabilities_map, + backend_origin_map=_backend_origin_map, ) recipe_info = _recipe_info_pre result = await _apply_triage_gate(result, name, recipe_info=recipe_info) @@ -379,13 +380,15 @@ async def validate_recipe(script_path: str) -> str: skill_resolver=tool_ctx.skill_resolver, config_backend=tool_ctx.config.agent_backend, ) - _validate_effective_backend_map = _compute_effective_backend_map( - _validate_recipe_steps, - tool_ctx.backend.name if tool_ctx.backend else None, - tool_ctx.config.providers, - _validate_recipe_name, - skill_resolver=tool_ctx.skill_resolver, - config_backend=tool_ctx.config.agent_backend, + _validate_effective_backend_map, _validate_backend_origin_map = ( + _compute_effective_backend_map( + _validate_recipe_steps, + tool_ctx.backend.name if tool_ctx.backend else None, + tool_ctx.config.providers, + _validate_recipe_name, + skill_resolver=tool_ctx.skill_resolver, + config_backend=tool_ctx.config.agent_backend, + ) ) _validate_backend_capabilities_map = build_backend_capabilities_map( _validate_effective_backend_map, tool_ctx.backend @@ -397,6 +400,7 @@ async def validate_recipe(script_path: str) -> str: ingredient_overrides=_cap_overrides, effective_backend_map=_validate_effective_backend_map, backend_capabilities_map=_validate_backend_capabilities_map, + backend_origin_map=_validate_backend_origin_map, ) return json.dumps(result) except Exception as exc: diff --git a/src/autoskillit/skills_extended/make-plan/SKILL.md b/src/autoskillit/skills_extended/make-plan/SKILL.md index 9b2c1a62e..9ed27cae9 100644 --- a/src/autoskillit/skills_extended/make-plan/SKILL.md +++ b/src/autoskillit/skills_extended/make-plan/SKILL.md @@ -342,7 +342,7 @@ If the plan exceeds 500 lines, split it into multiple files (`_part_a`, `_part_b - The title of each part file MUST include `— PART A ONLY` (or B, C, etc.) so scope is immediately visible. - Each part file MUST open with the scope warning block shown in the multi-part template below. - **Every part MUST independently pass `task test-check`.** A part that registers a symbol, adds a route, or changes behavior that causes a pre-existing test to fail must also update, remove, or `xfail`-bridge that test in the same part. Split boundaries must keep gate-prerequisites with the code that triggers them. - - **`xfail(strict=True)` bridge:** When a test must temporarily fail during a multi-part implementation (e.g., a deletion-guard canary that will be removed in a later part), mark it with `pytest.mark.xfail(strict=True, reason=" registered in Part X; guard removed in Part Y")` in the current part. `check_test_passed` ignores xfailed counts, so the gate passes. `strict=True` ensures the xfail mark is removed when the guard is deleted — if the test starts passing unexpectedly (because the later part landed), CI breaks until the mark is cleaned up. + - **`xfail(strict=True)` bridge:** When a test must temporarily fail during a multi-part implementation (e.g., a deletion-guard canary that will be removed in a later part), mark it with `pytest.mark.xfail(strict=True, reason=" registered in Part X; guard removed in Part Y (#NNNN)")` in the current part. `check_test_passed` ignores xfailed counts, so the gate passes. `strict=True` ensures the xfail mark is removed when the guard is deleted — if the test starts passing unexpectedly (because the later part landed), CI breaks until the mark is cleaned up. The bridge's `reason` must cite the open tracking issue (`#NNNN`) for the deferred work — enforced by an architectural guard. A bridge whose stated exit condition is satisfied within the same PR must be removed in that same PR. - **Deletion-guard canaries** (tests asserting a name/symbol/command is absent) must be removed or xfail-bridged in the same part that re-registers the name. Never defer canary removal to a later part. Save the plan to: `{{AUTOSKILLIT_TEMP}}/make-plan/{task_name}_plan_{YYYY-MM-DD_HHMMSS}.md` (relative to the current working directory) diff --git a/src/autoskillit/skills_extended/resolve-review/SKILL.md b/src/autoskillit/skills_extended/resolve-review/SKILL.md index b41e5b66d..68f4da764 100644 --- a/src/autoskillit/skills_extended/resolve-review/SKILL.md +++ b/src/autoskillit/skills_extended/resolve-review/SKILL.md @@ -505,6 +505,7 @@ classified `REJECT` with `category: "arch_violation"`. | 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()`; `_KNOWN_NON_CONFORMING_RULES` entries without `# tracking: #NNNN` comments | +| Xfail bridge policy | `test_xfail_bridge_policy.py` | `xfail(strict=True)` decorators and `pytest.param` marks whose `reason` string does not cite a `#NNNN` tracking issue; `_XFAIL_POLICY_EXEMPT_FILES` entries without `# permanent:` or `# tracking:` rationale 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` | | Issue URL extraction guard | `test_issue_url_extraction_guard.py` | Raw `.get("issue_url")` / `.get("issue_urls")` in `fleet/` outside `_issue_url_helpers.py` and `state_types.py` — must use `extract_issue_urls()`; `fleet_claim_guard.py` must retain both key variants | | 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 | diff --git a/tests/_test_filter.py b/tests/_test_filter.py index 345ad5156..eee5891d6 100644 --- a/tests/_test_filter.py +++ b/tests/_test_filter.py @@ -538,6 +538,7 @@ class ImportContext(enum.StrEnum): ), "registry": frozenset( { + "cli", "recipe", "server/test_smoke_pipeline.py", "contracts/test_skill_yaml_validation.py", diff --git a/tests/arch/AGENTS.md b/tests/arch/AGENTS.md index a173eddfa..8ec3e30d0 100644 --- a/tests/arch/AGENTS.md +++ b/tests/arch/AGENTS.md @@ -82,6 +82,7 @@ AST enforcement, sub-package layer contracts, and architectural invariant tests. | `test_recipe_contract_freshness.py` | Parametrized JSON card freshness enforcement: contract cards must have non-stale .json companions with content parity | | `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(); RuleFinding severity must match @semantic_rule decorator severity; _KNOWN_NON_CONFORMING_RULES entries must carry tracking comments | +| `test_xfail_bridge_policy.py` | xfail(strict=True) bridge governance: reasons must cite tracking issue (#NNNN); size-capped exemption registry with rationale enforcement | | `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) | diff --git a/tests/arch/test_layer_enforcement.py b/tests/arch/test_layer_enforcement.py index 24909249b..1e6bd262f 100644 --- a/tests/arch/test_layer_enforcement.py +++ b/tests/arch/test_layer_enforcement.py @@ -1494,6 +1494,9 @@ def test_default_classes_only_instantiated_inside_factory_or_allowlist() -> None "DefaultWorkspaceManager", # signal guard cleanup }, Path("cli/app.py"): {"DefaultSkillResolver"}, # skill listing command + Path("cli/doctor/_doctor_config.py"): { + "DefaultSkillResolver" + }, # standing backend pin feasibility check Path("execution/recording.py"): {"DefaultSubprocessRunner"}, # lazy fallback Path("pipeline/context.py"): { # __post_init__ + "DefaultBackgroundSupervisor", # field default_factory diff --git a/tests/arch/test_pyright_suppression_allowlist.py b/tests/arch/test_pyright_suppression_allowlist.py index 391c6fc27..3d32f1cab 100644 --- a/tests/arch/test_pyright_suppression_allowlist.py +++ b/tests/arch/test_pyright_suppression_allowlist.py @@ -22,7 +22,7 @@ "recipe/__init__.py", 274, ): "lazy-registry: method added by _register_rule_module() side effects", - ("recipe/_api.py", 285): "lazy-registry: RULE_REGISTRY_HASH set by _finalize_registry()", + ("recipe/_api.py", 286): "lazy-registry: RULE_REGISTRY_HASH set by _finalize_registry()", } TEST_ALLOWLIST: dict[tuple[str, int], str] = { diff --git a/tests/arch/test_subpackage_structure.py b/tests/arch/test_subpackage_structure.py index 2a7b8fa7b..a167b881d 100644 --- a/tests/arch/test_subpackage_structure.py +++ b/tests/arch/test_subpackage_structure.py @@ -61,8 +61,8 @@ def test_type_constants_split_completeness(self): assert len(combined) == len(remaining) + len(env) + len(features) + len(registries), ( "Duplicate symbols across split modules" ) - assert len(combined) == 109, ( - f"Expected 109 symbols total, got {len(combined)} " + assert len(combined) == 110, ( + f"Expected 110 symbols total, got {len(combined)} " f"(remaining={len(remaining)}, env={len(env)}, " f"features={len(features)}, registries={len(registries)})" ) diff --git a/tests/arch/test_xfail_bridge_policy.py b/tests/arch/test_xfail_bridge_policy.py new file mode 100644 index 000000000..44a55f053 --- /dev/null +++ b/tests/arch/test_xfail_bridge_policy.py @@ -0,0 +1,162 @@ +"""Architectural guard: xfail(strict=True) bridges must cite a tracking issue. + +Every ``pytest.mark.xfail(strict=True, reason=...)`` decorator — both on function +definitions and inside ``pytest.param(..., marks=...)`` — must include a ``#NNNN`` +issue reference in its ``reason`` string so bridge exit conditions are trackable. +Files in ``_XFAIL_POLICY_EXEMPT_FILES`` are excluded when they have an alternative +self-governance mechanism (e.g. a companion meta-test that forces the count +monotonically non-increasing). +""" + +from __future__ import annotations + +import ast +import re +from pathlib import Path + +import pytest + +pytestmark = [pytest.mark.layer("arch"), pytest.mark.small] + +_TESTS_ROOT = Path(__file__).resolve().parent.parent + +_XFAIL_POLICY_EXEMPT_FILES: frozenset[str] = frozenset( + { + "arch/test_recipe_diagram_freshness.py", # permanent: shrink meta-test + } +) + +_EXEMPT_CAP = 2 + + +def _is_xfail_strict_true(node: ast.AST) -> bool: + if not isinstance(node, ast.Call): + return False + func = node.func + is_xfail = False + if isinstance(func, ast.Name) and func.id == "xfail": + is_xfail = True + elif isinstance(func, ast.Attribute) and func.attr == "xfail": + is_xfail = True + if not is_xfail: + return False + for kw in node.keywords: + if kw.arg == "strict" and isinstance(kw.value, ast.Constant) and kw.value.value is True: + return True + return False + + +def _extract_reason(node: ast.Call) -> str | None: + for kw in node.keywords: + if kw.arg == "reason": + if isinstance(kw.value, ast.Constant) and isinstance(kw.value.value, str): + return kw.value.value + if isinstance(kw.value, ast.JoinedStr): + parts = [] + for v in kw.value.values: + if isinstance(v, ast.Constant) and isinstance(v.value, str): + parts.append(v.value) + return "".join(parts) if parts else None + return None + return None + + +def _collect_xfail_strict_true_nodes(tree: ast.Module) -> list[ast.Call]: + results: list[ast.Call] = [] + for node in ast.walk(tree): + if isinstance(node, (ast.FunctionDef, ast.AsyncFunctionDef)): + for dec in node.decorator_list: + if isinstance(dec, ast.Call) and _is_xfail_strict_true(dec): + results.append(dec) + elif isinstance(dec, ast.Call): + for arg_node in ast.walk(dec): + if ( + isinstance(arg_node, ast.Call) + and arg_node is not dec + and _is_xfail_strict_true(arg_node) + ): + results.append(arg_node) + if isinstance(node, ast.Call): + func = node.func + is_param = False + if isinstance(func, ast.Attribute) and func.attr == "param": + is_param = True + elif isinstance(func, ast.Name) and func.id == "param": + is_param = True + if is_param: + for kw in node.keywords: + if kw.arg == "marks": + for marks_node in ast.walk(kw.value): + if isinstance(marks_node, ast.Call) and _is_xfail_strict_true( + marks_node + ): + results.append(marks_node) + return results + + +def test_strict_xfail_reasons_cite_tracking_issue() -> None: + """Every xfail(strict=True) reason must contain a #NNNN issue reference.""" + violations: list[str] = [] + for py_file in sorted(_TESTS_ROOT.rglob("test_*.py")): + rel = py_file.relative_to(_TESTS_ROOT).as_posix() + if rel in _XFAIL_POLICY_EXEMPT_FILES: + continue + try: + tree = ast.parse(py_file.read_text()) + except (SyntaxError, UnicodeDecodeError, OSError): + continue + for xfail_node in _collect_xfail_strict_true_nodes(tree): + reason = _extract_reason(xfail_node) + if reason is None: + violations.append( + f"{rel}:{xfail_node.lineno} — reason must be a string literal " + "so policy can be checked" + ) + elif not re.search(r"#\d+", reason): + violations.append( + f"{rel}:{xfail_node.lineno} — reason={reason!r} does not cite " + "a tracking issue; add #NNNN or resolve the defect and remove the xfail" + ) + assert not violations, "xfail(strict=True) without tracking issue reference:\n" + "\n".join( + f" {v}" for v in violations + ) + + +def test_exemption_entries_have_rationale_comments() -> None: + """Every _XFAIL_POLICY_EXEMPT_FILES entry must have a rationale comment.""" + src = Path(__file__).read_text() + tree = ast.parse(src) + lines = src.splitlines() + for node in ast.walk(tree): + if not isinstance(node, ast.Assign): + continue + for target in node.targets: + if isinstance(target, ast.Name) and target.id == "_XFAIL_POLICY_EXEMPT_FILES": + if isinstance(node.value, ast.Call) and node.value.args: + set_node = node.value.args[0] + if isinstance(set_node, ast.Set): + for elt in set_node.elts: + if isinstance(elt, ast.Constant): + line = lines[elt.lineno - 1] + assert re.search(r"#\s*(permanent|tracking):", line), ( + f"Exempt entry {elt.value!r} on line {elt.lineno} " + "must have a '# permanent:' or '# tracking:' comment" + ) + + +def test_exemption_registry_size_cap() -> None: + """Exemption registry must not exceed _EXEMPT_CAP.""" + assert len(_XFAIL_POLICY_EXEMPT_FILES) <= _EXEMPT_CAP, ( + f"_XFAIL_POLICY_EXEMPT_FILES has {len(_XFAIL_POLICY_EXEMPT_FILES)} entries " + f"but cap is {_EXEMPT_CAP}. Review whether all entries are still needed." + ) + + +def test_exempt_files_exist() -> None: + """Every exemption registry entry must resolve to an existing file.""" + missing = [ + entry + for entry in sorted(_XFAIL_POLICY_EXEMPT_FILES) + if not (_TESTS_ROOT / entry).is_file() + ] + assert not missing, f"Exempt entries point to non-existent files: {missing}" diff --git a/tests/cli/AGENTS.md b/tests/cli/AGENTS.md index 7bbdc8969..2b94b39d1 100644 --- a/tests/cli/AGENTS.md +++ b/tests/cli/AGENTS.md @@ -37,6 +37,8 @@ CLI command, subcommand, and interactive workflow tests. | `test_doctor_script.py` | Tests for the _check_script_binary doctor check — script(1) PTY binary availability | | `test_doctor_claude_binary.py` | Tests for the _check_claude_binary doctor check — claude CLI availability for capability-driven rerouting | | `test_doctor_runtime.py` | Tests for the _check_codex_model_alias_staleness doctor check — codex model alias staleness auditing (Check 36) | +| `test_doctor_standing_pins.py` | Tests for _check_standing_backend_pins_feasibility doctor check — standing backend pin capability feasibility (Check 37) | +| `test_doctor_local_recipes.py` | Tests for _check_local_recipe_validity doctor check — local recipe semantic validation (Check 38) | | `test_doctor_split.py` | Structural guards: test_doctor.py split into three files (P1-F02) | | `test_hook_drift_plugin_guard.py` | Tests for hook drift false-positive fix when plugin is marketplace-installed | | `test_features_cli.py` | Tests for the features CLI subcommand | diff --git a/tests/cli/test_doctor_local_recipes.py b/tests/cli/test_doctor_local_recipes.py new file mode 100644 index 000000000..e694cbe0d --- /dev/null +++ b/tests/cli/test_doctor_local_recipes.py @@ -0,0 +1,52 @@ +"""Tests for _check_local_recipe_validity doctor check.""" + +from __future__ import annotations + +from pathlib import Path + +import pytest + +pytestmark = [pytest.mark.layer("cli"), pytest.mark.small] + + +class TestLocalRecipeValidity: + def test_structurally_invalid_recipe_reports_error(self, tmp_path: Path) -> None: + from autoskillit.cli.doctor._doctor_config import _check_local_recipe_validity + from autoskillit.core import Severity + + recipes_dir = tmp_path / ".autoskillit" / "recipes" + recipes_dir.mkdir(parents=True) + (recipes_dir / "broken.yaml").write_text( + "name: broken\n" + "description: intentionally broken recipe\n" + 'autoskillit_version: "0.2.0"\n' + "steps:\n" + " step_a:\n" + " tool: run_skill\n", + encoding="utf-8", + ) + results = _check_local_recipe_validity(project_dir=tmp_path) + + errors = [r for r in results if r.severity == Severity.ERROR] + assert errors, f"Expected ERROR for broken recipe, got: {results}" + assert "broken.yaml" in errors[0].message + + def test_no_local_recipes_dir_reports_ok(self, tmp_path: Path) -> None: + from autoskillit.cli.doctor._doctor_config import _check_local_recipe_validity + from autoskillit.core import Severity + + results = _check_local_recipe_validity(project_dir=tmp_path) + + assert len(results) == 1 + assert results[0].severity == Severity.OK + + def test_empty_recipes_dir_reports_ok(self, tmp_path: Path) -> None: + from autoskillit.cli.doctor._doctor_config import _check_local_recipe_validity + from autoskillit.core import Severity + + recipes_dir = tmp_path / ".autoskillit" / "recipes" + recipes_dir.mkdir(parents=True) + results = _check_local_recipe_validity(project_dir=tmp_path) + + assert len(results) == 1 + assert results[0].severity == Severity.OK diff --git a/tests/cli/test_doctor_standing_pins.py b/tests/cli/test_doctor_standing_pins.py new file mode 100644 index 000000000..77e1f78b6 --- /dev/null +++ b/tests/cli/test_doctor_standing_pins.py @@ -0,0 +1,81 @@ +"""Tests for _check_standing_backend_pins_feasibility doctor check.""" + +from __future__ import annotations + +from pathlib import Path + +import pytest + +pytestmark = [pytest.mark.layer("cli"), pytest.mark.small] + + +def _write_config(path: Path, content: str) -> None: + path.parent.mkdir(parents=True, exist_ok=True) + path.write_text(content, encoding="utf-8") + + +class TestStandingBackendPinsFeasibility: + def test_infeasible_pin_reports_error( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch + ) -> None: + from autoskillit.cli.doctor._doctor_config import ( + _check_standing_backend_pins_feasibility, + ) + from autoskillit.core import Severity + + monkeypatch.setattr("pathlib.Path.home", lambda: tmp_path / "home") + config_dir = tmp_path / "home" / ".autoskillit" + _write_config( + config_dir / "config.yaml", + "agent_backend:\n recipe_overrides:\n remediation:\n resolve_review: codex\n", + ) + + project_dir = tmp_path / "project" + project_dir.mkdir() + results = _check_standing_backend_pins_feasibility(project_dir=project_dir) + + errors = [r for r in results if r.severity == Severity.ERROR] + assert errors, f"Expected at least one ERROR result, got: {results}" + msg = errors[0].message + assert "resolve_review" in msg and "codex" in msg + + def test_feasible_pin_reports_ok( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch + ) -> None: + from autoskillit.cli.doctor._doctor_config import ( + _check_standing_backend_pins_feasibility, + ) + from autoskillit.core import Severity + + monkeypatch.setattr("pathlib.Path.home", lambda: tmp_path / "home") + + project_dir = tmp_path / "project" + project_dir.mkdir() + results = _check_standing_backend_pins_feasibility(project_dir=project_dir) + + assert all(r.severity == Severity.OK for r in results) + + def test_unknown_backend_degrades_to_warning( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch + ) -> None: + from autoskillit.cli.doctor._doctor_config import ( + _check_standing_backend_pins_feasibility, + ) + from autoskillit.core import Severity + + monkeypatch.setattr("pathlib.Path.home", lambda: tmp_path / "home") + config_dir = tmp_path / "home" / ".autoskillit" + _write_config( + config_dir / "config.yaml", + "agent_backend:\n" + " recipe_overrides:\n" + " remediation:\n" + " resolve_review: nonexistent_backend_xyz\n", + ) + + project_dir = tmp_path / "project" + project_dir.mkdir() + results = _check_standing_backend_pins_feasibility(project_dir=project_dir) + + warnings = [r for r in results if r.severity == Severity.WARNING] + assert warnings, f"Expected WARNING for unknown backend, got: {results}" diff --git a/tests/cli/test_fleet_run_admission.py b/tests/cli/test_fleet_run_admission.py index c26ccbb04..8069eb978 100644 --- a/tests/cli/test_fleet_run_admission.py +++ b/tests/cli/test_fleet_run_admission.py @@ -383,7 +383,7 @@ async def spy_dispatch(**kwargs: object) -> DispatchResult: # config_providers, recipe_name, skill_resolver=skill_resolver) with the same recipe # steps that _make_mock_ctx returns (empty dict). recipe_steps: dict[str, Any] = {} - kitchen_map = _compute_effective_backend_map( + kitchen_map, _ = _compute_effective_backend_map( recipe_steps, backend_name, None, # config_providers diff --git a/tests/core/test_backend_capabilities.py b/tests/core/test_backend_capabilities.py index 56f0c8296..1225d1e1b 100644 --- a/tests/core/test_backend_capabilities.py +++ b/tests/core/test_backend_capabilities.py @@ -284,3 +284,39 @@ def test_cmd_spec_has_is_resume_bool_field(): hints = typing.get_type_hints(CmdSpec) assert "is_resume" in hints, "CmdSpec must have an is_resume field" assert hints["is_resume"] is bool + + +def test_describe_capability_mismatches_convergence(): + """describe_capability_mismatches() output appears in both consumer messages.""" + from autoskillit.core import ( + BackendCapabilities, + HardCapabilityMismatch, + describe_capability_mismatches, + unsatisfied_backend_capabilities, + ) + + caps = BackendCapabilities( + channel_b_capable=True, + pty_required=False, + session_resume_capable=False, + skill_injection_capable=False, + supports_thinking_blocks=False, + supports_claude_format_stdout=False, + exit_code_is_terminal=False, + mcp_config_capable=False, + food_truck_capable=False, + completion_record_types=frozenset(), + session_record_types=frozenset(), + git_metadata_writable=False, + ) + mismatches = unsatisfied_backend_capabilities(frozenset({"git_metadata_write"}), caps) + assert mismatches, "Expected at least one mismatch for git_metadata_write" + formatted = describe_capability_mismatches(mismatches) + assert "git_metadata_writable" in formatted + assert "True" in formatted + + single = [HardCapabilityMismatch("git_metadata_write", "git_metadata_writable", False)] + assert describe_capability_mismatches(single) == ( + "git_metadata_writable=True required (via 'git_metadata_write') " + "but backend has git_metadata_writable=False" + ) diff --git a/tests/docs/test_doc_counts.py b/tests/docs/test_doc_counts.py index b5fa8e0eb..6956403d7 100644 --- a/tests/docs/test_doc_counts.py +++ b/tests/docs/test_doc_counts.py @@ -237,16 +237,17 @@ def test_quota_thresholds_defaults() -> None: assert long_ == pytest.approx(95.0) -def test_doctor_check_count_is_44() -> None: - # 44 total = 23 numbered (1–17 base + 18–21 ambient env + 22–23 feature) +def test_doctor_check_count_is_46() -> None: + # 46 total = 23 numbered (1–17 base + 18–21 ambient env + 22–23 feature) # + 7 lettered sub-checks (2b–2e, 4b, 7b, 7c) - # + 6 gated fleet (24–29) + 8 backend runtime (30–37, including Check 31b - # claude CLI binary availability for capability-driven rerouting). + # + 6 gated fleet (24–29) + 8 backend runtime (30–36, including Check 31b + # claude CLI binary availability for capability-driven rerouting) + # + 2 new checks (37 standing backend pin feasibility, 38 local recipe validity). # Docs list 35 user-visible checks (23 numbered + 5 documented lettered + # 7 backend runtime); 2e and 7c are internal-only sub-checks not shown in docs. # Update both tests whenever a new doctor check is added. count = _count_doctor_checks() - assert count == 44, f"Expected 44 doctor checks; found {count}" + assert count == 46, f"Expected 46 doctor checks; found {count}" def test_bundled_recipe_count_is_15() -> None: diff --git a/tests/infra/test_pretty_output_recipe.py b/tests/infra/test_pretty_output_recipe.py index 677d2d881..ea45c0376 100644 --- a/tests/infra/test_pretty_output_recipe.py +++ b/tests/infra/test_pretty_output_recipe.py @@ -1052,8 +1052,8 @@ def test_canonical_recipe_responses_fit_independent_registry_ceilings(tmp_path, for ingredients_only in (False, True) } assert maxima == { - "load_recipe": (182_994, "remediation", "all_truthy"), - "open_kitchen": (183_053, "remediation", "all_truthy"), + "load_recipe": (182_916, "remediation", "all_truthy"), + "open_kitchen": (182_975, "remediation", "all_truthy"), } diff --git a/tests/infra/test_schema_version_convention.py b/tests/infra/test_schema_version_convention.py index 6731b4dac..46e6cb4b5 100644 --- a/tests/infra/test_schema_version_convention.py +++ b/tests/infra/test_schema_version_convention.py @@ -119,10 +119,10 @@ def _is_yaml_dump(node: ast.expr) -> bool: # _lifespan.py — hooks.json self-heal on startup drift (co-owned with Claude plugin system) ("src/autoskillit/server/_lifespan.py", 90), # tools_kitchen.py — hook config, quota guard, git_ops_policy, ingredient locks overlay - ("src/autoskillit/server/tools/tools_kitchen.py", 270), - ("src/autoskillit/server/tools/tools_kitchen.py", 289), - ("src/autoskillit/server/tools/tools_kitchen.py", 323), - ("src/autoskillit/server/tools/tools_kitchen.py", 1296), + ("src/autoskillit/server/tools/tools_kitchen.py", 274), + ("src/autoskillit/server/tools/tools_kitchen.py", 293), + ("src/autoskillit/server/tools/tools_kitchen.py", 327), + ("src/autoskillit/server/tools/tools_kitchen.py", 1303), # tools_pipeline_tracker.py — tracker_data dict ("src/autoskillit/server/tools/tools_pipeline_tracker.py", 166), # tools_status.py — mcp_data dict diff --git a/tests/recipe/AGENTS.md b/tests/recipe/AGENTS.md index 7a0c97589..3e15f4b59 100644 --- a/tests/recipe/AGENTS.md +++ b/tests/recipe/AGENTS.md @@ -29,6 +29,7 @@ Recipe I/O, validation, semantic rules, schema, and bundled recipe tests. | `test_bundled_recipes_general.py` | General structural tests for all bundled recipes | | `test_bundled_recipes_dispatch_ready.py` | Universal dispatch-readiness gate for all bundled recipes and contract cards | | `test_bundled_recipes_no_inversions.py` | Guard: no inversion step order violations in bundled recipes | +| `test_bundled_recipes_no_unreachable_steps.py` | Guard: no unreachable steps in any bundled recipe | | `test_bundled_recipes_all_truthy.py` | Regression guard: every bundled recipe validates cleanly under all-truthy and default ingredient configs (R1, R3) | | `test_bundled_recipes_pipeline_structure.py` | Pipeline structure tests for bundled recipes | | `test_bundled_recipes_research.py` | Tests for research bundled recipe structure | diff --git a/tests/recipe/test_api.py b/tests/recipe/test_api.py index ab26ddd6a..5f1ff7b9b 100644 --- a/tests/recipe/test_api.py +++ b/tests/recipe/test_api.py @@ -321,6 +321,7 @@ def test_load_and_validate_cache_key_includes_all_result_affecting_params(tmp_pa "resolved_defaults", "lister", "temp_dir", + "backend_origin_map", } ) @@ -476,6 +477,7 @@ def capturing_fn( defer_unresolved=False, backend_name=None, effective_backend_map=None, + backend_origin_map=None, ): captured["recipe_info"] = recipe_info captured["backend_capabilities_map"] = locals().get("backend_capabilities_map") @@ -494,6 +496,7 @@ def capturing_fn( backend_name=backend_name, effective_backend_map=effective_backend_map, backend_capabilities_map=backend_capabilities_map, + backend_origin_map=backend_origin_map, ) monkeypatch.setattr(api_mod, "load_and_validate", capturing_fn) @@ -1241,14 +1244,6 @@ def test_mid_process_yaml_only_update(tmp_path, monkeypatch): # --------------------------------------------------------------------------- -@pytest.mark.xfail( - strict=True, - reason=( - "Part A exposes latent defects in bundled implementation.yaml " - "(merge_fix_count without reset). Part B resolves the recipe defect; " - "remove xfail when Part B lands." - ), -) def test_load_and_validate_produces_valid_content_after_step_pruning(tmp_path: Path) -> None: """After structural repair (when=None catch-alls on resolve_review), pruning skip_when_false steps must produce a recipe with non-empty content and no @@ -1292,14 +1287,6 @@ def test_load_and_validate_produces_valid_content_after_step_pruning(tmp_path: P # --------------------------------------------------------------------------- -@pytest.mark.xfail( - strict=True, - reason=( - "Part A exposes latent defects in bundled implementation.yaml " - "(merge_fix_count without reset). Part B resolves the recipe defect; " - "remove xfail when Part B lands." - ), -) def test_validate_from_path_codex_backend_valid_after_pruning(tmp_path: Path) -> None: """validate_from_path must prune steps when ingredient_overrides are provided. diff --git a/tests/recipe/test_bundled_recipes_all_truthy.py b/tests/recipe/test_bundled_recipes_all_truthy.py index ea96d97ec..dc9e02932 100644 --- a/tests/recipe/test_bundled_recipes_all_truthy.py +++ b/tests/recipe/test_bundled_recipes_all_truthy.py @@ -48,40 +48,12 @@ def _default_overrides() -> dict[str, Any]: } -_PART_A_AFFECTED_RECIPES = frozenset({"remediation", "implementation", "implementation-groups"}) -"""Recipes whose semantic findings change as a result of Part A's rule corrections. - -Part A exposes latent defects (missing merge_fix_count reset and shared-counter- -cross-site-without-push-symmetry on remediation.yaml) which previously escaped -the old existential BFS. The xfail bridges on these tests survive until Part B -resolves the defects in the bundled recipes themselves. -""" - - def _recipe_names() -> list[str]: return sorted(all_validated_recipe_names(_PROJECT_ROOT)) -def _recipe_name_param(name: str): - """Return a pytest.param for ``name``, with strict xfail only on Part A affected recipes.""" - if name in _PART_A_AFFECTED_RECIPES: - return pytest.param( - name, - marks=pytest.mark.xfail( - strict=True, - reason=( - "Part A exposes latent defects in bundled recipes: " - "merge_fix_count without reset, and shared-counter-cross-site-" - "without-push-symmetry on remediation.yaml. Part B resolves " - "these defects; remove xfail when Part B lands." - ), - ), - ) - return pytest.param(name) - - def _parametrize_recipes() -> list: - return [_recipe_name_param(n) for n in _recipe_names()] + return [pytest.param(n) for n in _recipe_names()] @pytest.mark.parametrize("recipe_name", _parametrize_recipes(), ids=lambda n: n) diff --git a/tests/recipe/test_bundled_recipes_dispatch_ready.py b/tests/recipe/test_bundled_recipes_dispatch_ready.py index 548ce1522..e9ae8428b 100644 --- a/tests/recipe/test_bundled_recipes_dispatch_ready.py +++ b/tests/recipe/test_bundled_recipes_dispatch_ready.py @@ -37,38 +37,12 @@ _DISPATCH_GATE_STEMS = [s for s in _RECIPE_STEMS if s not in _RECIPES_WITH_OTHER_ERROR_RULES] -_PART_A_AFFECTED_RECIPES = frozenset({"remediation", "implementation", "implementation-groups"}) -"""Recipes that Part A's rule changes reveal latent defects in. - -The xfail bridges on these recipes survive until Part B resolves the defects -in the bundled recipes themselves. -""" - - -def _part_a_param(recipe_name: str): - """Return pytest.param for ``recipe_name`` with xfail on Part A recipes only.""" - if recipe_name in _PART_A_AFFECTED_RECIPES: - return pytest.param( - recipe_name, - marks=pytest.mark.xfail( - strict=True, - reason=( - "Part A exposes latent defects in bundled recipes: " - "merge_fix_count without reset, and shared-counter-cross-" - "site-without-push-symmetry on remediation.yaml. Part B " - "resolves these defects; remove xfail when Part B lands." - ), - ), - ) - return pytest.param(recipe_name) - - def _part_a_dispatch_gate_params() -> list: - return [_part_a_param(n) for n in _DISPATCH_GATE_STEMS] + return [pytest.param(n) for n in _DISPATCH_GATE_STEMS] def _part_a_recipe_params() -> list: - return [_part_a_param(n) for n in _RECIPE_STEMS] + return [pytest.param(n) for n in _RECIPE_STEMS] @pytest.mark.parametrize("recipe_name", _part_a_dispatch_gate_params(), ids=lambda n: n) @@ -315,14 +289,6 @@ def test_codex_implementation_dispatch_infeasible() -> None: assert "gate_backend_write" in result.get("infeasible_steps", []) -@pytest.mark.xfail( - strict=True, - reason=( - "Part A exposes latent defects in bundled implementation.yaml " - "(merge_fix_count without reset on the audit-remediation NO-GO path). " - "Part B adds the reset step; remove xfail when Part B lands." - ), -) def test_claude_implementation_dispatch_feasible() -> None: """Claude Code backend + implementation recipe must report dispatch_feasible=True.""" result = load_and_validate( diff --git a/tests/recipe/test_bundled_recipes_no_unreachable_steps.py b/tests/recipe/test_bundled_recipes_no_unreachable_steps.py new file mode 100644 index 000000000..3cd7772c8 --- /dev/null +++ b/tests/recipe/test_bundled_recipes_no_unreachable_steps.py @@ -0,0 +1,34 @@ +"""Bundled-recipe regression guard: no unreachable steps in any bundled recipe.""" + +from __future__ import annotations + +from pathlib import Path + +import pytest + +from autoskillit.core.types import Severity +from autoskillit.recipe.io import all_validated_recipe_names, builtin_recipes_dir, load_recipe +from autoskillit.recipe.validator import run_semantic_rules + +pytestmark = [pytest.mark.layer("recipe"), pytest.mark.small] + +_PROJECT_ROOT = Path(__file__).resolve().parent.parent.parent + + +def _bundled_recipe_names() -> list[str]: + names = sorted(all_validated_recipe_names(_PROJECT_ROOT)) + rd = builtin_recipes_dir() + return [n for n in names if (rd / f"{n}.yaml").exists()] + + +@pytest.mark.parametrize("recipe_name", _bundled_recipe_names()) +def test_bundled_recipe_has_no_unreachable_steps(recipe_name: str) -> None: + """Every step in a bundled recipe must be reachable from the entry point.""" + recipe = load_recipe(builtin_recipes_dir() / f"{recipe_name}.yaml") + findings = run_semantic_rules(recipe) + unreachable = [ + f for f in findings if f.rule == "unreachable-step" and f.severity >= Severity.WARNING + ] + assert unreachable == [], f"Recipe '{recipe_name}' has unreachable steps: " + ", ".join( + f"{f.step_name} ({f.message})" for f in unreachable + ) diff --git a/tests/recipe/test_content_integrity.py b/tests/recipe/test_content_integrity.py index 2c8f61fd0..23c42c9b1 100644 --- a/tests/recipe/test_content_integrity.py +++ b/tests/recipe/test_content_integrity.py @@ -42,32 +42,7 @@ def _make_params() -> list[tuple[str, str, list[str]]]: return params -_PART_A_AFFECTED_RECIPES = frozenset({"remediation", "implementation", "implementation-groups"}) - - -def _content_integrity_param(recipe_name: str, ing_name: str, guarded_steps: list[str]): - if recipe_name in _PART_A_AFFECTED_RECIPES: - return pytest.param( - recipe_name, - ing_name, - guarded_steps, - marks=pytest.mark.xfail( - strict=True, - reason=( - "Part A exposes latent defects (merge_fix_count without " - "reset, shared-counter-cross-site-without-push-symmetry). " - "Part B resolves them; remove xfail when Part B lands." - ), - ), - ) - return pytest.param(recipe_name, ing_name, guarded_steps) - - -def _xfail_params() -> list: - return [_content_integrity_param(*p) for p in _make_params()] - - -@pytest.mark.parametrize("recipe_name,ing_name,guarded_steps", _xfail_params()) +@pytest.mark.parametrize("recipe_name,ing_name,guarded_steps", _make_params()) def test_truthy_resolved_step_has_no_optional_signal_in_content( recipe_name: str, ing_name: str, guarded_steps: list[str] ) -> None: diff --git a/tests/recipe/test_full_audit_recipe.py b/tests/recipe/test_full_audit_recipe.py index 649a1096c..f038fd615 100644 --- a/tests/recipe/test_full_audit_recipe.py +++ b/tests/recipe/test_full_audit_recipe.py @@ -60,11 +60,12 @@ def test_full_audit_recipe_step_names() -> None: def test_full_audit_routing_chain() -> None: - """checkout → run_audits → validate_audits → create_issues → done.""" + """checkout → init_audit_run → run_audits → validate_audits → create_issues → done.""" from autoskillit.recipe.io import load_recipe recipe = load_recipe(RECIPE_PATH) - assert recipe.steps["checkout"].on_success == "run_audits" + assert recipe.steps["checkout"].on_success == "init_audit_run" + assert recipe.steps["init_audit_run"].on_success == "run_audits" assert recipe.steps["run_audits"].on_success == "validate_audits" assert recipe.steps["validate_audits"].on_success == "create_issues" assert recipe.steps["create_issues"].on_success == "done" diff --git a/tests/recipe/test_issue_url_pipeline.py b/tests/recipe/test_issue_url_pipeline.py index accf4dc87..982233435 100644 --- a/tests/recipe/test_issue_url_pipeline.py +++ b/tests/recipe/test_issue_url_pipeline.py @@ -20,15 +20,6 @@ def _recipe_path(name: str) -> Path: class TestImplementationPipelineIssueUrl: - @pytest.mark.xfail( - strict=True, - reason=( - "Part A's dominator fix to loop-counter-not-reset-on-outer-cycle " - "correctly exposes missing merge_fix_count reset on bundled " - "recipes. Part B adds the reset step; this xfail must be " - "removed when Part B lands." - ), - ) def test_recipe_validates_clean(self): """implementation must validate with no errors after adding issue_url.""" result = validate_from_path(_recipe_path("implementation")) @@ -118,15 +109,6 @@ def test_no_issue_content_dead_output(self): class TestInvestigateFirstIssueUrl: - @pytest.mark.xfail( - strict=True, - reason=( - "Part A's rule fix correctly exposes merge_fix_count without " - "reset and shared-counter-cross-site-without-push-symmetry on " - "remediation.yaml. Part B resolves both defects; remove xfail " - "when Part B lands." - ), - ) def test_recipe_validates_clean(self): result = validate_from_path(_recipe_path("remediation")) errors = [f for f in result.get("findings", []) if f.get("severity") == Severity.ERROR] @@ -214,15 +196,6 @@ def test_no_issue_content_dead_output(self): class TestImplementationGroupsIssueTitle: - @pytest.mark.xfail( - strict=True, - reason=( - "Part A's dominator fix to loop-counter-not-reset-on-outer-cycle " - "correctly exposes missing merge_fix_count reset on bundled " - "implementation-groups.yaml. Part B adds the reset step; remove " - "xfail when Part B lands." - ), - ) def test_recipe_validates_clean(self): result = validate_from_path(_recipe_path("implementation-groups")) errors = [f for f in result.get("findings", []) if f.get("severity") == Severity.ERROR] diff --git a/tests/recipe/test_recipe_backend_composition_matrix.py b/tests/recipe/test_recipe_backend_composition_matrix.py index 570babeb8..14df0f23f 100644 --- a/tests/recipe/test_recipe_backend_composition_matrix.py +++ b/tests/recipe/test_recipe_backend_composition_matrix.py @@ -41,31 +41,12 @@ ] -# -- Known-broken combos (xfail strict) ------------------------------------- -KNOWN_BROKEN: dict[tuple[str, str], str] = { - (recipe, backend): ( - "Part A exposes latent defects in bundled recipes (merge_fix_count " - "without reset on the audit-remediation NO-GO path; " - "ref_push_count shared with asymmetric push symmetry in remediation.yaml). " - "Part B adds reset steps and a push step to close these defects. " - "Remove this xfail entry when Part B lands." - ) - for recipe in ("remediation", "implementation", "implementation-groups") - for backend in _BACKEND_NAMES -} - _SKILL_RESOLVER = DefaultSkillResolver() def _apply_marks(matrix_ids: list[tuple[str, str]]) -> list[Any]: - """Wrap matrix tuples in pytest.param, attaching xfail marks for KNOWN_BROKEN.""" - params: list[Any] = [] - for r, b in matrix_ids: - marks: list[Any] = [] - if (r, b) in KNOWN_BROKEN: - marks.append(pytest.mark.xfail(strict=True, reason=KNOWN_BROKEN[(r, b)])) - params.append(pytest.param(r, b, marks=marks, id=f"{r}/{b}")) - return params + """Wrap matrix tuples in pytest.param.""" + return [pytest.param(r, b, id=f"{r}/{b}") for r, b in matrix_ids] @pytest.mark.parametrize("recipe_name,backend_name", _apply_marks(_MATRIX_IDS)) @@ -76,7 +57,7 @@ def test_recipe_backend_matrix_cell(recipe_name: str, backend_name: str, monkeyp ) backend = get_backend(backend_name) _raw = load_recipe(_RECIPE_PATHS[recipe_name]) - _eff_map = _compute_effective_backend_map( + _eff_map, _ = _compute_effective_backend_map( _raw.steps, backend_name, None, @@ -143,7 +124,7 @@ def test_dispatch_feasible_per_backend(recipe_name: str, backend_name: str, monk ) backend = get_backend(backend_name) _raw = load_recipe(_RECIPE_PATHS[recipe_name]) - _eff_map = _compute_effective_backend_map( + _eff_map, _ = _compute_effective_backend_map( _raw.steps, backend_name, None, @@ -200,14 +181,6 @@ def test_matrix_collection_count() -> None: ) -@pytest.mark.xfail( - strict=True, - reason=( - "Part A exposes latent defects in bundled implementation.yaml " - "(merge_fix_count without reset on the audit-remediation NO-GO path). " - "Part B adds the reset step; remove xfail when Part B lands." - ), -) def test_recipe_backend_matrix_codex_with_provider_override() -> None: """Codex with backend_supports_git_write=true: recipe is valid and feasible. diff --git a/tests/recipe/test_recipe_composition_vacuous_gate.py b/tests/recipe/test_recipe_composition_vacuous_gate.py index f0723ff6f..d35efb162 100644 --- a/tests/recipe/test_recipe_composition_vacuous_gate.py +++ b/tests/recipe/test_recipe_composition_vacuous_gate.py @@ -155,14 +155,6 @@ def test_is_vacuous_gate_returns_true_when_gate_unreachable_post_prune() -> None "recipe_name", ["implementation", "remediation", "implementation-groups"], ) -@pytest.mark.xfail( - strict=True, - reason=( - "Part A exposes latent defects in bundled recipes: " - "merge_fix_count without reset on the audit-remediation NO-GO path. " - "Part B adds the reset step; remove xfail when Part B lands." - ), -) def test_compute_capability_feasibility_returns_infeasible_for_codex_recipes( recipe_name: str, ) -> None: @@ -227,7 +219,7 @@ def test_compute_capability_feasibility_feasible_when_capability_route_active( _info = find_recipe_by_name(recipe_name, _PROJECT_ROOT) assert _info is not None, f"Recipe {recipe_name!r} not found" raw = load_recipe(_info.path) - eff_map = _compute_effective_backend_map( + eff_map, _ = _compute_effective_backend_map( raw.steps, "codex", None, @@ -301,7 +293,7 @@ def test_capability_route_binary_absent_fails_closed( assert overrides["backend_supports_git_write"] == "false" assert detail.resolution_path == "capability_route_no_binary" - eff_map = _compute_effective_backend_map( + eff_map, _ = _compute_effective_backend_map( raw.steps, "codex", None, diff --git a/tests/recipe/test_repository.py b/tests/recipe/test_repository.py index 25f52a869..5f5bd5643 100644 --- a/tests/recipe/test_repository.py +++ b/tests/recipe/test_repository.py @@ -177,6 +177,7 @@ def test_validate_from_path_delegates_to_api(tmp_path: Path) -> None: ingredient_overrides=None, effective_backend_map=None, backend_capabilities_map=None, + backend_origin_map=None, ) diff --git a/tests/recipe/test_rules_backend_compat.py b/tests/recipe/test_rules_backend_compat.py index 5afcfdf11..f454299eb 100644 --- a/tests/recipe/test_rules_backend_compat.py +++ b/tests/recipe/test_rules_backend_compat.py @@ -330,7 +330,7 @@ def test_codex_backend_fires_for_git_metadata_skills( recipe = load_recipe(builtin_recipes_dir() / f"{recipe_name}.yaml") resolver = DefaultSkillResolver() - eff_map = _compute_effective_backend_map( + eff_map, _ = _compute_effective_backend_map( recipe.steps, "codex", None, diff --git a/tests/recipe/test_rules_integration_predicate.py b/tests/recipe/test_rules_integration_predicate.py index ef872799d..f980cc694 100644 --- a/tests/recipe/test_rules_integration_predicate.py +++ b/tests/recipe/test_rules_integration_predicate.py @@ -56,7 +56,7 @@ def test_investigate_first_merge_step_has_predicate_on_result(self) -> None: cond3 = step.on_result.conditions[3] assert cond3.when == "result.failed_step == 'rebase'" - assert cond3.route == "release_issue_failure" + assert cond3.route == "check_merge_rebase_loop" cond4 = step.on_result.conditions[4] assert cond4.when == "result.failed_step == 'dirty_main_repo'" @@ -149,15 +149,6 @@ def test_both_recipes_validate_cleanly(self) -> None: ip_errors = validate_recipe_structure(self.ip_recipe) assert ip_errors == [], f"implementation.yaml has validation errors: {ip_errors}" - @pytest.mark.xfail( - strict=True, - reason=( - "Part A exposes latent defects in bundled recipes: " - "merge_fix_count without reset, and shared-counter-cross-site-" - "without-push-symmetry on remediation.yaml. Part B resolves " - "these defects; remove xfail when Part B lands." - ), - ) def test_all_recipes_no_error_semantic_findings(self) -> None: """All bundled implementation-family recipes have no ERROR-severity findings.""" for recipe, name in [ @@ -271,11 +262,7 @@ def test_all_merge_failure_arms_guarded(self, recipe_name: str) -> None: elif recipe_name == "remediation" and cond.when in ( "result.failed_step == 'test_gate'", "result.failed_step == 'post_rebase_test_gate'", - "result.failed_step == 'rebase'", ): - # After PART B step 5, remediation.yaml's pre_remediation_merge - # routes these to terminal escalation so live-worktree merge - # failures no longer orphan the next worktree creator. assert cond.route in terminal_steps, ( f"{cond.when} routes to {cond.route}, expected a terminal escalation" ) diff --git a/tests/recipe/test_rules_loop_counter.py b/tests/recipe/test_rules_loop_counter.py index 4aaf6d823..99a6d8c25 100644 --- a/tests/recipe/test_rules_loop_counter.py +++ b/tests/recipe/test_rules_loop_counter.py @@ -292,50 +292,21 @@ def test_wrapper_loop_exempt_counter_excluded(self) -> None: ("remediation", "implementation", "implementation-groups"), ) def test_bundled_recipes_no_missing_reset(self, recipe_name: str) -> None: - """After the dominator fix to ``loop-counter-not-reset-on-outer-cycle``, - this test asserts the rule fires on the bundled recipes for the - counters that lack a reset step on the audit-remediation NO-GO path. - - Before the fix this test asserted zero findings (the original - existential-path BFS mistakenly accepted parallel guards' ``capture`` - as resets). The fix correctly excludes ``check_loop_iteration`` guards - from the reset candidate list because their capture is an INCREMENT, - not a reset — so parallel guards sharing a counter no longer - masquerade as resets. - - The merged findings here represent latent defects in the bundled - recipes (``merge_fix_count`` is shared by ``check_merge_fix_loop``, - ``check_merge_rebase_loop``, ``check_dirty_main_retry`` with no - ``init_counter`` reset on the audit-remediation NO-GO path). Part B - will add the missing reset steps; once it lands this test will need - updating to assert zero findings again. + """Permanent regression gate: bundled recipes must have zero + ``loop-counter-not-reset-on-outer-cycle`` findings. + + Every counter shared by parallel guards (``check_merge_fix_loop``, + ``check_merge_rebase_loop``, ``check_dirty_main_retry``) must be + reset by an ``init_counter`` step on the audit-remediation NO-GO + path before reaching the guard. """ recipe = load_recipe(builtin_recipes_dir() / f"{recipe_name}.yaml") findings = run_semantic_rules(recipe) rule_findings = [f for f in findings if f.rule == "loop-counter-not-reset-on-outer-cycle"] - # Expected findings per recipe: counters shared by parallel guards with - # no reset step on the audit-remediation NO-GO path. - expected_inner_steps = { - "remediation": {"check_merge_fix_loop", "check_dirty_main_retry"}, - "implementation": { - "check_merge_fix_loop", - "check_merge_rebase_loop", - "check_dirty_main_retry", - }, - "implementation-groups": { - "check_merge_fix_loop", - "check_merge_rebase_loop", - "check_dirty_main_retry", - }, - } - expected = expected_inner_steps.get(recipe_name, set()) - - actual = {f.step_name for f in rule_findings} - assert expected <= actual, ( - f"{recipe_name}: expected at least {sorted(expected)}, " - f"got {sorted(actual)}. Findings: " - f"{[(f.step_name, f.message) for f in rule_findings]}" + assert rule_findings == [], ( + f"{recipe_name}: expected zero loop-counter-not-reset-on-outer-cycle " + f"findings, got: {[(f.step_name, f.message) for f in rule_findings]}" ) def test_branching_reentry_reset_on_only_one_branch_fires(self) -> None: diff --git a/tests/recipe/test_rules_registry.py b/tests/recipe/test_rules_registry.py index 30c6503cd..b81392b01 100644 --- a/tests/recipe/test_rules_registry.py +++ b/tests/recipe/test_rules_registry.py @@ -93,31 +93,10 @@ def test_bundled_workflows_pass_semantic_rules() -> None: yaml_files = list(wf_dir.glob("*.yaml")) assert yaml_files - _PART_A_NON_CONFORMING: dict[str, set[str]] = { - "remediation.yaml": { - "loop-counter-not-reset-on-outer-cycle", - }, - "implementation.yaml": {"loop-counter-not-reset-on-outer-cycle"}, - "implementation-groups.yaml": {"loop-counter-not-reset-on-outer-cycle"}, - } - """Part A exposes latent defects in these recipes. - - Excluded from the strict no-error-findings assertion. Part B resolves the - recipe-level defects (adds reset steps and a push step); once Part B lands - remove the entries to enforce the strict invariant again. - """ for path in yaml_files: wf = load_recipe(path) findings = run_semantic_rules(wf) - excluded = _PART_A_NON_CONFORMING.get(path.name, set()) - if excluded: - fired_rules = {f.rule for f in findings if f.severity == Severity.ERROR} - for rule_name in excluded: - assert rule_name in fired_rules, ( - f"Recipe '{path.name}': exclusion for '{rule_name}' is stale — " - f"rule no longer fires. Remove from _PART_A_NON_CONFORMING." - ) - errors = [f for f in findings if f.severity == Severity.ERROR and f.rule not in excluded] + errors = [f for f in findings if f.severity == Severity.ERROR] assert not errors, ( f"Bundled workflow {path.name} has error-severity semantic findings: {errors}" ) diff --git a/tests/server/test_admission_dispatch_agreement.py b/tests/server/test_admission_dispatch_agreement.py index 38bc77844..21a1d96c6 100644 --- a/tests/server/test_admission_dispatch_agreement.py +++ b/tests/server/test_admission_dispatch_agreement.py @@ -47,6 +47,14 @@ _BACKENDS: tuple[str, ...] = tuple(sorted(BACKEND_REGISTRY.keys())) _SKILL_RESOLVER = DefaultSkillResolver() +_EXPECTED_INFEASIBLE: frozenset[tuple[str, str]] = frozenset( + { + ("implementation", "codex"), # by-design: gate_backend_write reachable post-prune + ("implementation-groups", "codex"), # by-design: gate_backend_write reachable post-prune + ("remediation", "codex"), # by-design: gate_backend_write reachable post-prune + } +) + def _step_effective_backend( step: RecipeStep, @@ -104,7 +112,7 @@ def test_admission_dispatch_agreement( # Pre-load recipe to compute effective backend map (mirrors IL-3 call sites). raw_recipe = load_recipe_yaml(_BUILTIN_DIR / f"{recipe_name}.yaml") - effective_map = _compute_effective_backend_map( + effective_map, _ = _compute_effective_backend_map( raw_recipe.steps, backend_name, None, @@ -121,13 +129,20 @@ def test_admission_dispatch_agreement( lister=_SKILL_RESOLVER, ) - if not result.get("dispatch_feasible", True): - # If admission refuses feasibility, the contract is trivially satisfied. + if (recipe_name, backend_name) in _EXPECTED_INFEASIBLE: + assert result.get("dispatch_feasible") is False, ( + f"{recipe_name}/{backend_name} expected infeasible but got " + f"dispatch_feasible={result.get('dispatch_feasible')}" + ) return - if not result.get("valid", False): - # If admission produced errors, contract still holds (not feasible). - return + assert result.get("valid") is True, ( + f"{recipe_name}/{backend_name} expected valid=True but got valid={result.get('valid')}" + ) + assert result.get("dispatch_feasible") is True, ( + f"{recipe_name}/{backend_name} expected dispatch_feasible=True but got " + f"dispatch_feasible={result.get('dispatch_feasible')}" + ) # Admission says feasible + valid → dispatch must agree for every step. dispatch_backends = _dispatch_effective_backends(raw_recipe, backend_name, effective_map) @@ -213,7 +228,7 @@ def test_open_kitchen_capability_not_routing_eligible_in_admission() -> None: steps to claude-code — open_kitchen is not worker_routable.""" from autoskillit.config._config_dataclasses import ProvidersConfig - eff_map = _compute_effective_backend_map( + eff_map, _ = _compute_effective_backend_map( {"step": _mock_run_skill_step()}, "codex", ProvidersConfig(), @@ -231,7 +246,7 @@ def test_effective_backend_map_capability_route_unit() -> None: to claude-code when a skill_resolver is supplied.""" from autoskillit.config._config_dataclasses import ProvidersConfig - eff_map = _compute_effective_backend_map( + eff_map, _ = _compute_effective_backend_map( {"fix": _mock_run_skill_step()}, "codex", ProvidersConfig(), @@ -308,14 +323,6 @@ def test_provider_aware_overrides_capability_route_none_providers( assert detail.resolution_path == "capability_route" -@pytest.mark.xfail( - strict=True, - reason=( - "Part A exposes latent defects in bundled implementation.yaml " - "(merge_fix_count without reset). Part B resolves the recipe defect; " - "remove xfail when Part B lands." - ), -) def test_admission_dispatch_agreement_real_providers_config( monkeypatch: pytest.MonkeyPatch, ) -> None: @@ -346,7 +353,7 @@ def test_admission_dispatch_agreement_real_providers_config( ) assert overrides["backend_supports_git_write"] == "true" - effective_map = _compute_effective_backend_map( + effective_map, _ = _compute_effective_backend_map( raw_recipe.steps, "codex", providers, @@ -427,7 +434,7 @@ def test_make_plan_reroutes_on_codex() -> None: fake_backend.name = "codex" fake_backend.capabilities.anthropic_provider_capable = False - eff_map = _compute_effective_backend_map( + eff_map, _ = _compute_effective_backend_map( {"plan": step}, "codex", ProvidersConfig(), @@ -469,7 +476,7 @@ def test_investigate_reroutes_on_codex() -> None: fake_backend.name = "codex" fake_backend.capabilities.anthropic_provider_capable = False - eff_map = _compute_effective_backend_map( + eff_map, _ = _compute_effective_backend_map( {"inv": step}, "codex", ProvidersConfig(), @@ -508,7 +515,7 @@ def test_prepare_issue_stays_on_codex() -> None: fake_backend.name = "codex" fake_backend.capabilities.anthropic_provider_capable = False - eff_map = _compute_effective_backend_map( + eff_map, _ = _compute_effective_backend_map( {"prep": step}, "codex", ProvidersConfig(), @@ -691,7 +698,7 @@ def test_admission_dispatch_agreement_with_explicit_pin( # call sites use (_compute_effective_backend_map), instead of hand- # constructing the map, so this test proves the whole resolution chain — # not just the two downstream gates — agrees on the pinned backend. - effective_map = _compute_effective_backend_map( + effective_map, _origin_map = _compute_effective_backend_map( {step_name: step}, "claude-code", None, @@ -752,7 +759,7 @@ def test_admission_dispatch_agreement_with_explicit_pin( ) preflight_parsed = _json.loads(preflight_err) assert "git_metadata_writable" in preflight_parsed.get("error", "") - assert preflight_parsed.get("override_source") == "explicit_config" + assert preflight_parsed.get("origin") is not None # --- Leg 3: run_skill-time dispatch gate (_check_backend_compat) --- dispatch_err = _check_backend_compat( diff --git a/tests/server/test_backend_ingredient_injection.py b/tests/server/test_backend_ingredient_injection.py index a6cb877e9..b5b953f99 100644 --- a/tests/server/test_backend_ingredient_injection.py +++ b/tests/server/test_backend_ingredient_injection.py @@ -495,14 +495,6 @@ def _discover_git_write_steps() -> frozenset[str]: and step.tool == "run_skill" ) - @pytest.mark.xfail( - strict=True, - reason=( - "Part A exposes latent defects in bundled implementation.yaml " - "(merge_fix_count without reset). Part B resolves the recipe defect; " - "remove xfail when Part B lands." - ), - ) def test_codex_overrides_remove_guarded_steps_from_content(self, tmp_path: Path) -> None: from autoskillit.recipe._api import load_and_validate diff --git a/tests/server/test_capability_admission_e2e.py b/tests/server/test_capability_admission_e2e.py index a43e1e354..fde1fde1c 100644 --- a/tests/server/test_capability_admission_e2e.py +++ b/tests/server/test_capability_admission_e2e.py @@ -50,14 +50,6 @@ def test_codex_backend_reachable_gate_returns_infeasible() -> None: assert "gate_backend_write" in result.get("infeasible_steps", []) -@pytest.mark.xfail( - strict=True, - reason=( - "Part A exposes latent defects in bundled implementation.yaml " - "(merge_fix_count without reset). Part B resolves the recipe defect; " - "remove xfail when Part B lands." - ), -) def test_claude_code_backend_produces_feasible_recipe() -> None: """Claude Code + implementation recipe: backend_supports_git_write=true produces dispatch_feasible=True.""" diff --git a/tests/server/test_explicit_backend_override.py b/tests/server/test_explicit_backend_override.py index dba97a907..dc5744a60 100644 --- a/tests/server/test_explicit_backend_override.py +++ b/tests/server/test_explicit_backend_override.py @@ -49,7 +49,7 @@ def test_explicit_backend_override_admission_dispatch_agreement(self) -> None: recipe_overrides={"remediation": {"dry_walkthrough": "codex"}}, ) # Admission side - admission_map = _compute_effective_backend_map( + admission_map, _ = _compute_effective_backend_map( cast(Any, steps), "codex", None, "remediation", config_backend=cfg ) assert admission_map == {"dry_walkthrough": "codex"} @@ -97,7 +97,7 @@ def test_explicit_override_suppresses_capability_routing(self) -> None: recipe_overrides={"remediation": {"dry_walkthrough": "codex"}}, ) resolver = _make_resolver("dry-walkthrough") - admission_map = _compute_effective_backend_map( + admission_map, _ = _compute_effective_backend_map( cast(Any, steps), "codex", None, @@ -198,7 +198,9 @@ def test_codex_to_claude_explicit_override(self) -> None: backend="codex", step_overrides={"implement": "claude-code"}, ) - assert _resolve_backend_override("implement", "any_recipe", cfg) == "claude-code" + result = _resolve_backend_override("implement", "any_recipe", cfg) + assert result is not None + assert result.backend == "claude-code" def test_claude_to_codex_explicit_override(self) -> None: from autoskillit.server._guards import _resolve_backend_override @@ -207,7 +209,9 @@ def test_claude_to_codex_explicit_override(self) -> None: backend="claude-code", step_overrides={"implement": "codex"}, ) - assert _resolve_backend_override("implement", "any_recipe", cfg) == "codex" + result = _resolve_backend_override("implement", "any_recipe", cfg) + assert result is not None + assert result.backend == "codex" def test_dry_walkthrough_codex_pin(self) -> None: """The exact scenario from issue #4242: backend=codex + recipe override to codex + @@ -225,7 +229,7 @@ def test_dry_walkthrough_codex_pin(self) -> None: recipe_overrides={"remediation": {"dry_walkthrough": "codex"}}, ) resolver = _make_resolver("dry-walkthrough") - admission_map = _compute_effective_backend_map( + admission_map, _ = _compute_effective_backend_map( cast(Any, steps), "codex", providers, @@ -258,7 +262,7 @@ def test_explicit_codex_pin_skips_claude_binary_check(self, monkeypatch) -> None recipe_overrides={"remediation": {"dry_walkthrough": "codex"}}, ) resolver = _make_resolver("dry-walkthrough") - admission_map = _compute_effective_backend_map( + admission_map, _ = _compute_effective_backend_map( cast(Any, steps), "codex", None, diff --git a/tests/server/test_resolve_backend_override.py b/tests/server/test_resolve_backend_override.py index 5af70f7ab..9957b82fe 100644 --- a/tests/server/test_resolve_backend_override.py +++ b/tests/server/test_resolve_backend_override.py @@ -11,18 +11,24 @@ def _make_backend(**kwargs): return AgentBackendConfig(**kwargs) +def _backend(result): + """Extract .backend from a BackendPinResolution, or return None.""" + return result.backend if result is not None else None + + class TestResolveBackendOverride: def test_exact_recipe_step_match(self) -> None: from autoskillit.server._guards import _resolve_backend_override cfg = _make_backend(recipe_overrides={"remediation": {"dry_walkthrough": "codex"}}) - assert _resolve_backend_override("dry_walkthrough", "remediation", cfg) == "codex" + result = _resolve_backend_override("dry_walkthrough", "remediation", cfg) + assert _backend(result) == "codex" def test_recipe_wildcard(self) -> None: from autoskillit.server._guards import _resolve_backend_override cfg = _make_backend(recipe_overrides={"remediation": {"*": "codex"}}) - assert _resolve_backend_override("any_step", "remediation", cfg) == "codex" + assert _backend(_resolve_backend_override("any_step", "remediation", cfg)) == "codex" def test_exact_beats_wildcard(self) -> None: from autoskillit.server._guards import _resolve_backend_override @@ -30,13 +36,18 @@ def test_exact_beats_wildcard(self) -> None: cfg = _make_backend( recipe_overrides={"remediation": {"*": "codex", "dry_walkthrough": "claude-code"}} ) - assert _resolve_backend_override("dry_walkthrough", "remediation", cfg) == "claude-code" + assert ( + _backend(_resolve_backend_override("dry_walkthrough", "remediation", cfg)) + == "claude-code" + ) def test_step_override_with_recipe_context(self) -> None: from autoskillit.server._guards import _resolve_backend_override cfg = _make_backend(step_overrides={"dry_walkthrough": "codex"}) - assert _resolve_backend_override("dry_walkthrough", "remediation", cfg) == "codex" + assert ( + _backend(_resolve_backend_override("dry_walkthrough", "remediation", cfg)) == "codex" + ) def test_step_override_requires_recipe_context(self) -> None: from autoskillit.server._guards import _resolve_backend_override @@ -51,13 +62,16 @@ def test_recipe_override_beats_step_override(self) -> None: step_overrides={"dry_walkthrough": "codex"}, recipe_overrides={"remediation": {"dry_walkthrough": "claude-code"}}, ) - assert _resolve_backend_override("dry_walkthrough", "remediation", cfg) == "claude-code" + assert ( + _backend(_resolve_backend_override("dry_walkthrough", "remediation", cfg)) + == "claude-code" + ) def test_step_wildcard_with_recipe_context(self) -> None: from autoskillit.server._guards import _resolve_backend_override cfg = _make_backend(step_overrides={"*": "codex"}) - assert _resolve_backend_override("anything", "any_recipe", cfg) == "codex" + assert _backend(_resolve_backend_override("anything", "any_recipe", cfg)) == "codex" def test_step_wildcard_requires_recipe_context(self) -> None: from autoskillit.server._guards import _resolve_backend_override @@ -75,7 +89,7 @@ def test_empty_step_name_skips_recipe_lookup(self) -> None: from autoskillit.server._guards import _resolve_backend_override cfg = _make_backend(recipe_overrides={"remediation": {"*": "codex"}}) - assert _resolve_backend_override("", "remediation", cfg) == "codex" + assert _backend(_resolve_backend_override("", "remediation", cfg)) == "codex" def test_empty_recipe_name_skips_recipe_overrides(self) -> None: from autoskillit.server._guards import _resolve_backend_override diff --git a/tests/server/test_serve_idempotence.py b/tests/server/test_serve_idempotence.py index d0a5f566f..88218e6ba 100644 --- a/tests/server/test_serve_idempotence.py +++ b/tests/server/test_serve_idempotence.py @@ -63,15 +63,6 @@ async def _open_kitchen_patched(name, overrides, monkeypatch): ) -@pytest.mark.xfail( - strict=True, - reason=( - "Part A exposes latent defects in bundled remediation.yaml " - "(merge_fix_count without reset, shared-counter-cross-site-without-" - "push-symmetry). Part B resolves the recipe defects; remove xfail " - "when Part B lands." - ), -) async def test_load_recipe_after_open_kitchen_with_overrides_serves_identical_content( tool_ctx_kitchen_open, monkeypatch, @@ -104,13 +95,6 @@ async def test_load_recipe_after_open_kitchen_with_overrides_serves_identical_co ) -@pytest.mark.xfail( - strict=True, - reason=( - "Part A exposes latent defects in bundled remediation.yaml; Part B " - "resolves them. Remove xfail when Part B lands." - ), -) async def test_load_recipe_after_open_kitchen_without_overrides_serves_identical_content( tool_ctx_kitchen_open, monkeypatch, @@ -149,13 +133,6 @@ async def test_load_recipe_after_open_kitchen_without_overrides_serves_identical ) -@pytest.mark.xfail( - strict=True, - reason=( - "Part A exposes latent defects in bundled remediation.yaml; Part B " - "resolves them. Remove xfail when Part B lands." - ), -) async def test_deferred_recall_open_kitchen_serves_identical_to_first_serving( tool_ctx_kitchen_open, monkeypatch, @@ -193,13 +170,6 @@ async def test_deferred_recall_open_kitchen_serves_identical_to_first_serving( ) -@pytest.mark.xfail( - strict=True, - reason=( - "Part A exposes latent defects in bundled remediation.yaml; Part B " - "resolves them. Remove xfail when Part B lands." - ), -) async def test_session_serve_overrides_cleared_on_close_kitchen( tool_ctx_kitchen_open, monkeypatch, @@ -241,13 +211,6 @@ async def test_session_serve_overrides_cleared_on_close_kitchen( ) -@pytest.mark.xfail( - strict=True, - reason=( - "Part A exposes latent defects in bundled remediation.yaml; Part B " - "resolves them. Remove xfail when Part B lands." - ), -) async def test_explicit_load_recipe_overrides_layer_on_top_of_session_baseline( tool_ctx_kitchen_open, monkeypatch, @@ -284,13 +247,6 @@ async def test_explicit_load_recipe_overrides_layer_on_top_of_session_baseline( # ── New tests (Part B: get_recipe surface fix + parametric guard) ───────────── -@pytest.mark.xfail( - strict=True, - reason=( - "Part A exposes latent defects in bundled remediation.yaml; Part B " - "resolves them. Remove xfail when Part B lands." - ), -) async def test_get_recipe_content_matches_open_kitchen_with_overrides( tool_ctx_kitchen_open, monkeypatch, @@ -372,13 +328,6 @@ async def _call_re_serve_surface( raise ValueError(f"Unknown surface: {surface!r}") -@pytest.mark.xfail( - strict=True, - reason=( - "Part A exposes latent defects in bundled remediation.yaml; Part B " - "resolves them. Remove xfail when Part B lands." - ), -) @pytest.mark.parametrize("surface", _RE_SERVE_SURFACES) async def test_serve_surfaces_parametric_content_identity( surface: str, @@ -414,13 +363,6 @@ async def test_serve_surfaces_parametric_content_identity( ) -@pytest.mark.xfail( - strict=True, - reason=( - "Part A exposes latent defects in bundled remediation.yaml; Part B " - "resolves them. Remove xfail when Part B lands." - ), -) async def test_get_recipe_snapshot_lifecycle( tool_ctx_kitchen_open: object, monkeypatch: pytest.MonkeyPatch, @@ -484,13 +426,6 @@ async def test_get_recipe_snapshot_lifecycle( max_size=2, ) ) -@pytest.mark.xfail( - strict=True, - reason=( - "Part A exposes latent defects in bundled remediation.yaml; Part B " - "resolves them. Remove xfail when Part B lands." - ), -) async def test_load_recipe_routing_matches_open_kitchen_for_arbitrary_overrides( overrides: dict[str, str], tool_ctx_kitchen_open: ToolContext, diff --git a/tests/server/test_tools_kitchen_cache_poison.py b/tests/server/test_tools_kitchen_cache_poison.py index c1eddca83..3fa5e2009 100644 --- a/tests/server/test_tools_kitchen_cache_poison.py +++ b/tests/server/test_tools_kitchen_cache_poison.py @@ -12,14 +12,6 @@ pytestmark = [pytest.mark.layer("server"), pytest.mark.anyio, pytest.mark.medium] -@pytest.mark.xfail( - strict=True, - reason=( - "Part A exposes latent defects in bundled implementation.yaml " - "(merge_fix_count without reset). Part B resolves the recipe defect; " - "remove xfail when Part B lands." - ), -) async def test_open_kitchen_ingredients_only_does_not_poison_load_recipe( tool_ctx_kitchen_open, monkeypatch, diff --git a/tests/server/test_tools_kitchen_gate_features.py b/tests/server/test_tools_kitchen_gate_features.py index c5b7b6285..604c8b9d8 100644 --- a/tests/server/test_tools_kitchen_gate_features.py +++ b/tests/server/test_tools_kitchen_gate_features.py @@ -425,6 +425,7 @@ def test_recipe_resource_returns_composed_content(): defer_unresolved=True, resolved_defaults={}, backend_capabilities_map={}, + backend_origin_map={}, ) assert result == ("name: test-recipe\nsteps:\n stop:\n action: stop\n message: done\n") assert "optional: true" not in result diff --git a/tests/server/test_tools_kitchen_preflight.py b/tests/server/test_tools_kitchen_preflight.py index 1459e846c..4fd95a0c6 100644 --- a/tests/server/test_tools_kitchen_preflight.py +++ b/tests/server/test_tools_kitchen_preflight.py @@ -178,7 +178,7 @@ def test_dispatch_feasibility_rejects_pinned_step_to_incapable_backend(self) -> assert result is not None parsed = json.loads(result) assert "git_metadata_writable" in parsed.get("error", "") - assert parsed.get("override_source") == "explicit_config" + assert parsed.get("origin") == "agent_backend.recipe_overrides.test-recipe.resolve_review" def test_dispatch_feasibility_fails_closed_when_skill_resolver_missing_for_pinned_step( self,