From e9a52f80f7a749f3cf07f19703acfb7d3a4cad2b Mon Sep 17 00:00:00 2001 From: Trecek Date: Fri, 14 Aug 2026 16:35:46 -0700 Subject: [PATCH 01/18] feat: define unresolved skill input defaults --- src/autoskillit/recipe/_contracts_card.py | 15 ++- src/autoskillit/recipe/_contracts_manifest.py | 45 ++++++-- src/autoskillit/recipe/_contracts_types.py | 3 +- tests/recipe/test_contracts.py | 39 +++++++ tests/recipe/test_contracts_manifest.py | 109 ++++++++++++++++++ tests/server/test_tools_execution_results.py | 30 +++++ 6 files changed, 225 insertions(+), 16 deletions(-) diff --git a/src/autoskillit/recipe/_contracts_card.py b/src/autoskillit/recipe/_contracts_card.py index f57c93b439..f4fd52a538 100644 --- a/src/autoskillit/recipe/_contracts_card.py +++ b/src/autoskillit/recipe/_contracts_card.py @@ -157,12 +157,17 @@ def generate_recipe_card( skill_entry: dict[str, Any] = { "inputs": [ { - "name": i.name, - "type": i.type, - "required": i.required, - "recommended": i.recommended, + "name": item.name, + "type": item.type, + "required": item.required, + "recommended": item.recommended, + **( + {"unresolved_default": item.unresolved_default} + if item.unresolved_default is not None + else {} + ), } - for i in contract.inputs + for item in contract.inputs ], "outputs": [{"name": o.name, "type": o.type} for o in contract.outputs], "expected_output_patterns": contract.expected_output_patterns, diff --git a/src/autoskillit/recipe/_contracts_manifest.py b/src/autoskillit/recipe/_contracts_manifest.py index fe740a60e4..c91090cc9a 100644 --- a/src/autoskillit/recipe/_contracts_manifest.py +++ b/src/autoskillit/recipe/_contracts_manifest.py @@ -11,6 +11,7 @@ from autoskillit.core import ( VALID_INPUT_SPEC_TYPES, + BoundScalar, InputSpec, InputSpecType, get_logger, @@ -48,22 +49,45 @@ def load_bundled_manifest() -> dict[str, Any]: return _MANIFEST_CACHE.get_or_load(manifest_path, load_yaml) +def _parse_skill_input(skill_name: str, raw: Mapping[str, Any]) -> SkillInput: + input_def = SkillInput( + name=raw["name"], + type=raw["type"], + # Skill inputs default to optional — skills are permissive by design. + required=raw.get("required", False), + recommended=raw.get("recommended", False), + ) + if "unresolved_default" not in raw: + return input_def + unresolved_default = raw["unresolved_default"] + if type(unresolved_default) not in (str, int, bool): + raise ValueError( + f"unresolved_default for skill '{skill_name}' input " + f"'{input_def.name}' must be a strict string, integer, or boolean" + ) + if input_def.required: + raise ValueError( + f"required input '{input_def.name}' for skill '{skill_name}' " + "cannot declare unresolved_default" + ) + if not input_def.accepts(unresolved_default): + raise ValueError( + f"unresolved_default for skill '{skill_name}' input " + f"'{input_def.name}' does not satisfy type '{input_def.type}'" + ) + return dataclasses.replace( + input_def, + unresolved_default=cast(BoundScalar, unresolved_default), + ) + + def get_skill_contract(skill_name: str, manifest: dict[str, Any]) -> SkillContract | None: """Look up a skill in the manifest and return a SkillContract.""" skills = manifest.get("skills", {}) skill_data = skills.get(skill_name) if skill_data is None: return None - inputs = tuple( - SkillInput( - name=inp["name"], - type=inp["type"], - # Skill inputs default to optional — skills are permissive by design. - required=inp.get("required", False), - recommended=inp.get("recommended", False), - ) - for inp in skill_data.get("inputs", []) - ) + inputs = tuple(_parse_skill_input(skill_name, inp) for inp in skill_data.get("inputs", [])) outputs = [ SkillOutput( name=out["name"], @@ -294,6 +318,7 @@ def compute_skill_contract_identity( "nullable": item.nullable, "required": item.required, "type": item.type, + "unresolved_default": item.unresolved_default, } for item in contract.inputs ], diff --git a/src/autoskillit/recipe/_contracts_types.py b/src/autoskillit/recipe/_contracts_types.py index ac7c8b835b..df74537f6c 100644 --- a/src/autoskillit/recipe/_contracts_types.py +++ b/src/autoskillit/recipe/_contracts_types.py @@ -7,7 +7,7 @@ import regex as re -from autoskillit.core import PreflightKind +from autoskillit.core import BoundScalar, PreflightKind _CONTEXT_REF_RE = re.compile(r"\$\{\{\s*context\.(\w+)\s*\}\}") INPUT_REF_RE = re.compile(r"\$\{\{\s*inputs\.(\w+)\s*\}\}") @@ -22,6 +22,7 @@ class SkillInput: required: bool recommended: bool = False nullable: bool = True + unresolved_default: BoundScalar | None = None def accepts(self, value: object) -> bool: normalized = self.type diff --git a/tests/recipe/test_contracts.py b/tests/recipe/test_contracts.py index e61c9a6d8d..8642902358 100644 --- a/tests/recipe/test_contracts.py +++ b/tests/recipe/test_contracts.py @@ -256,6 +256,45 @@ def test_generate_recipe_card(tmp_path: Path) -> None: assert "dataflow" in contract +def test_generate_recipe_card_emits_only_declared_unresolved_defaults( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + from autoskillit.recipe import _contracts_card + + manifest = { + "version": "test", + "skills": { + "implement-worktree-no-merge": { + "inputs": [ + {"name": "plan_path", "type": "file_path", "required": True}, + { + "name": "audit_cycle_path", + "type": "file_path", + "required": False, + "unresolved_default": "", + }, + {"name": "mode", "type": "str", "required": False}, + ], + "outputs": [], + } + }, + } + monkeypatch.setattr(_contracts_card, "load_bundled_manifest", lambda: manifest) + recipes_dir = tmp_path / ".autoskillit" / "scripts" + recipes_dir.mkdir(parents=True) + pipeline = recipes_dir / "test-pipeline.yaml" + pipeline.write_text(SAMPLE_PIPELINE_YAML) + + card = generate_recipe_card(pipeline, recipes_dir) + + inputs = card["skills"]["implement-worktree-no-merge"]["inputs"] + by_name = {item["name"]: item for item in inputs} + assert by_name["audit_cycle_path"]["unresolved_default"] == "" + assert "unresolved_default" not in by_name["plan_path"] + assert "unresolved_default" not in by_name["mode"] + + def test_generate_recipe_card_returns_dict(tmp_path: Path) -> None: """generate_recipe_card returns a dict directly (not a Path).""" recipes_dir = tmp_path / ".autoskillit" / "scripts" diff --git a/tests/recipe/test_contracts_manifest.py b/tests/recipe/test_contracts_manifest.py index e5484d6412..8739975135 100644 --- a/tests/recipe/test_contracts_manifest.py +++ b/tests/recipe/test_contracts_manifest.py @@ -10,6 +10,7 @@ import pytest from autoskillit.recipe._contracts_manifest import ( + compute_skill_contract_identity, get_callable_contract, get_skill_contract, load_bundled_manifest, @@ -31,6 +32,114 @@ def test_get_skill_contract_rejects_non_boolean_scope_discipline(value: object) get_skill_contract("demo-skill", manifest) +@pytest.mark.parametrize( + ("input_type", "value"), + [ + ("str", None), + ("str", 1.5), + ("str", []), + ("str", {}), + ("str", 0), + ("integer", False), + ("boolean", 0), + ], +) +def test_get_skill_contract_rejects_invalid_unresolved_default( + input_type: str, + value: object, +) -> None: + manifest = { + "skills": { + "demo-skill": { + "inputs": [ + { + "name": "value", + "type": input_type, + "required": False, + "unresolved_default": value, + } + ] + } + } + } + + with pytest.raises(ValueError, match="unresolved_default"): + get_skill_contract("demo-skill", manifest) + + +@pytest.mark.parametrize( + ("input_type", "value"), + [("str", ""), ("integer", 0), ("boolean", False)], +) +def test_get_skill_contract_preserves_falsey_unresolved_default( + input_type: str, + value: str | int | bool, +) -> None: + manifest = { + "skills": { + "demo-skill": { + "inputs": [ + { + "name": "value", + "type": input_type, + "required": False, + "unresolved_default": value, + } + ] + } + } + } + + contract = get_skill_contract("demo-skill", manifest) + + assert contract is not None + restored = contract.inputs[0].unresolved_default + assert restored == value + assert type(restored) is type(value) + + +def test_get_skill_contract_rejects_required_unresolved_default() -> None: + manifest = { + "skills": { + "demo-skill": { + "inputs": [ + { + "name": "value", + "type": "str", + "required": True, + "unresolved_default": "", + } + ] + } + } + } + + with pytest.raises(ValueError, match="required input"): + get_skill_contract("demo-skill", manifest) + + +def test_unresolved_default_changes_skill_contract_identity() -> None: + def manifest(default: str) -> dict[str, object]: + return { + "skills": { + "demo-skill": { + "inputs": [ + { + "name": "value", + "type": "str", + "required": False, + "unresolved_default": default, + } + ] + } + } + } + + assert compute_skill_contract_identity( + "demo-skill", manifest=manifest("") + ) != compute_skill_contract_identity("demo-skill", manifest=manifest("unavailable")) + + def test_get_callable_contract_promotes_allowed_values_for_commit_guard() -> None: """get_callable_contract must parse `allowed_values` from YAML into SkillOutput.""" contract = get_callable_contract("autoskillit.recipe._cmd_rpc.commit_guard") diff --git a/tests/server/test_tools_execution_results.py b/tests/server/test_tools_execution_results.py index e091b67074..b4aa3e566f 100644 --- a/tests/server/test_tools_execution_results.py +++ b/tests/server/test_tools_execution_results.py @@ -164,6 +164,36 @@ def test_persisted_audit_contract_preserves_selected_output_mode() -> None: } +@pytest.mark.parametrize( + ("input_type", "value"), + [("str", ""), ("integer", 0), ("boolean", False)], +) +def test_persisted_skill_contract_preserves_falsey_unresolved_default( + input_type: str, + value: str | int | bool, +) -> None: + from autoskillit.server.tools import _execution_helpers as helpers + + selected = helpers.SkillContract( + inputs=( + helpers.SkillInput( + name="value", + type=input_type, + required=False, + unresolved_default=value, + ), + ), + outputs=[], + ) + + restored = helpers.deserialize_skill_contract(helpers.serialize_skill_contract(selected)) + + assert restored is not None + restored_default = restored.inputs[0].unresolved_default + assert restored_default == value + assert type(restored_default) is type(value) + + class TestGateErrorSchemaNormalization: """Gate errors use the standard 9-field response schema.""" From d6e4def9a51b66b21ee4d2a9d450fc40582f83fd Mon Sep 17 00:00:00 2001 From: Trecek Date: Fri, 14 Aug 2026 16:39:10 -0700 Subject: [PATCH 02/18] feat: compile optional context defaults into bindings --- .../core/types/_type_recipe_binding.py | 9 ++ .../core/types/_type_recipe_execution.py | 1 + src/autoskillit/recipe/_binding.py | 8 ++ .../rules/dataflow/rules_dataflow_callable.py | 62 ++++++++- tests/recipe/test_rules_dataflow_nullable.py | 120 ++++++++++++++++++ tests/recipe/test_skill_invocation_binding.py | 59 +++++++++ 6 files changed, 253 insertions(+), 6 deletions(-) diff --git a/src/autoskillit/core/types/_type_recipe_binding.py b/src/autoskillit/core/types/_type_recipe_binding.py index 284abeabcb..1771346a7d 100644 --- a/src/autoskillit/core/types/_type_recipe_binding.py +++ b/src/autoskillit/core/types/_type_recipe_binding.py @@ -197,15 +197,24 @@ class BoundValue: context_dependencies: tuple[str, ...] = () input_dependencies: tuple[str, ...] = () template_dependencies: tuple[str, ...] = () + unresolved_default: BoundScalar | None = None def __post_init__(self) -> None: if not isinstance(self.state, BoundValueState): raise ValueError("BoundValue.state must be a BoundValueState") if not isinstance(self.origin, BoundValueOrigin): raise ValueError("BoundValue.origin must be a BoundValueOrigin") + if self.unresolved_default is not None and type(self.unresolved_default) not in ( + str, + int, + bool, + ): + raise ValueError("BoundValue.unresolved_default must be a strict scalar or None") declared_absent = isinstance(self.declared_value, AbsentBoundValue) effective_absent = isinstance(self.effective_value, AbsentBoundValue) if self.state is BoundValueState.ABSENT: + if self.unresolved_default is not None: + raise ValueError("absent bound values cannot declare unresolved_default") if ( not declared_absent or not effective_absent diff --git a/src/autoskillit/core/types/_type_recipe_execution.py b/src/autoskillit/core/types/_type_recipe_execution.py index 8be6df0e73..fb56156c72 100644 --- a/src/autoskillit/core/types/_type_recipe_execution.py +++ b/src/autoskillit/core/types/_type_recipe_execution.py @@ -80,6 +80,7 @@ def _bound_value_payload(value: BoundValue) -> dict[str, object]: "origin": value.origin.value, "state": value.state.value, "template_dependencies": list(value.template_dependencies), + "unresolved_default": value.unresolved_default, } diff --git a/src/autoskillit/recipe/_binding.py b/src/autoskillit/recipe/_binding.py index f4c5933e07..4890b65cb3 100644 --- a/src/autoskillit/recipe/_binding.py +++ b/src/autoskillit/recipe/_binding.py @@ -3,6 +3,7 @@ from __future__ import annotations from collections.abc import Mapping +from dataclasses import replace from typing import Any, Final, TypeGuard import regex as re @@ -453,6 +454,7 @@ def _structured_skill_inputs( contract: SkillContract, hidden_inputs: frozenset[str], ingredient_values: Mapping[str, BoundScalar], + optional_context_refs: frozenset[str], ) -> tuple[tuple[BoundValue, ...], tuple[BindingFailure, ...]]: input_by_name = {input_def.name: input_def for input_def in contract.inputs} failures: list[BindingFailure] = [] @@ -501,6 +503,11 @@ def _structured_skill_inputs( ingredient_values=ingredient_values, ) value = _bound_value(input_def.name, declared, resolved) + if ( + not input_def.required + and frozenset(value.context_dependencies) & optional_context_refs + ): + value = replace(value, unresolved_default=input_def.unresolved_default) bound.append(value) unresolved = bool(value.context_dependencies or value.input_dependencies) if not unresolved and not _skill_value_is_valid(resolved, input_def): @@ -783,6 +790,7 @@ def bind_step_invocation( contract=contract, hidden_inputs=hidden_inputs, ingredient_values=ingredients, + optional_context_refs=frozenset(step.optional_context_refs), ) failures.extend(skill_failures) else: diff --git a/src/autoskillit/recipe/rules/dataflow/rules_dataflow_callable.py b/src/autoskillit/recipe/rules/dataflow/rules_dataflow_callable.py index 19bd16f5a5..f1d4018104 100644 --- a/src/autoskillit/recipe/rules/dataflow/rules_dataflow_callable.py +++ b/src/autoskillit/recipe/rules/dataflow/rules_dataflow_callable.py @@ -4,14 +4,22 @@ import importlib import inspect +from collections.abc import Mapping import regex as re -from autoskillit.core import RUN_PYTHON_SENTINEL_KEYS, SKILL_TOOLS, Severity, get_logger +from autoskillit.core import ( + RUN_PYTHON_SENTINEL_KEYS, + SKILL_TOOLS, + Severity, + get_logger, + resolve_skill_name, +) from autoskillit.recipe._analysis import ValidationContext from autoskillit.recipe.contracts import ( _CONTEXT_REF_RE, get_callable_contract, + get_skill_contract, load_bundled_manifest, ) from autoskillit.recipe.registry import RuleFinding, make_finding, semantic_rule @@ -186,8 +194,8 @@ def _check_downstream_context_completeness(ctx: ValidationContext) -> list[RuleF @semantic_rule( name="nullable-optional-context-ref", description=( - "run_python steps must not pass optional_context_refs values to non-nullable " - "callable inputs without null coercion in the callable" + "run_python nullable inputs and run_skill unresolved defaults must safely cover " + "values declared in optional_context_refs" ), severity=Severity.ERROR, ) @@ -195,6 +203,51 @@ def _check_nullable_optional_context_ref(ctx: ValidationContext) -> list[RuleFin findings = [] manifest = load_bundled_manifest() for step_name, step in ctx.recipe.steps.items(): + optional_refs = set(step.optional_context_refs) + if not optional_refs: + continue + if step.tool == "run_skill": + skill_inputs = step.with_args.get("skill_inputs") + skill_command = step.with_args.get("skill_command", "") + if not isinstance(skill_inputs, Mapping) or not isinstance(skill_command, str): + continue + skill_name = resolve_skill_name(skill_command) + if not skill_name: + continue + contract = get_skill_contract(skill_name, manifest) + if contract is None: + continue + input_by_name = {item.name: item for item in contract.inputs} + for input_name, value in skill_inputs.items(): + input_def = input_by_name.get(input_name) + if input_def is None or not isinstance(value, str): + continue + dependencies = optional_refs.intersection(_CONTEXT_REF_RE.findall(value)) + if not dependencies: + continue + dependency = sorted(dependencies)[0] + if input_def.required: + message = ( + f"Step '{step_name}' required input '{input_name}' for skill " + f"'{skill_name}' depends on optional context ref '{dependency}'. " + "Capture or gate the value before dispatch." + ) + elif input_def.unresolved_default is None: + message = ( + f"Step '{step_name}' optional input '{input_name}' for skill " + f"'{skill_name}' depends on optional context ref '{dependency}' " + "without a contract unresolved_default." + ) + else: + continue + findings.append( + make_finding( + rule_name="nullable-optional-context-ref", + step_name=step_name, + message=message, + ) + ) + continue if step.tool != "run_python": continue callable_path = step.with_args.get("callable", "") @@ -206,9 +259,6 @@ def _check_nullable_optional_context_ref(ctx: ValidationContext) -> list[RuleFin non_nullable = {inp.name for inp in contract.inputs if not inp.nullable} if not non_nullable: continue - optional_refs = set(step.optional_context_refs) - if not optional_refs: - continue args_values = _get_args_values(step.with_args) for inp_name in sorted(non_nullable): arg_val = args_values.get(inp_name) diff --git a/tests/recipe/test_rules_dataflow_nullable.py b/tests/recipe/test_rules_dataflow_nullable.py index 92fcd05251..948d1b00c4 100644 --- a/tests/recipe/test_rules_dataflow_nullable.py +++ b/tests/recipe/test_rules_dataflow_nullable.py @@ -4,6 +4,7 @@ import pytest +import autoskillit.recipe.rules.dataflow.rules_dataflow_callable as callable_rules from autoskillit.core.types import Severity from autoskillit.recipe.validator import run_semantic_rules from tests.recipe.conftest import _make_workflow @@ -58,3 +59,122 @@ def test_non_optional_context_ref_does_not_trigger_rule(self) -> None: assert not any(f.severity == Severity.ERROR for f in nullable_findings), ( f"Should not flag when optional_context_refs is absent, got: {nullable_findings}" ) + + @staticmethod + def _run_skill_recipe( + *, + value: str, + optional_refs: list[str], + required: bool, + unresolved_default: str | None, + monkeypatch: pytest.MonkeyPatch, + ): + input_def: dict[str, object] = { + "name": "value", + "type": "str", + "required": required, + } + if unresolved_default is not None: + input_def["unresolved_default"] = unresolved_default + manifest = { + "skills": { + "demo-skill": { + "inputs": [input_def], + "outputs": [], + } + } + } + monkeypatch.setattr(callable_rules, "load_bundled_manifest", lambda: manifest) + return _make_workflow( + { + "invoke": { + "tool": "run_skill", + "with": { + "skill_command": "/autoskillit:demo-skill", + "skill_inputs": {"value": value}, + }, + "optional_context_refs": optional_refs, + "on_success": "done", + }, + "done": {"action": "stop", "message": "done"}, + } + ) + + def test_optional_run_skill_input_requires_unresolved_default( + self, + monkeypatch: pytest.MonkeyPatch, + ) -> None: + recipe = self._run_skill_recipe( + value="${{ context.optional_value }}", + optional_refs=["optional_value"], + required=False, + unresolved_default=None, + monkeypatch=monkeypatch, + ) + + findings = run_semantic_rules(recipe) + + assert any( + finding.rule == "nullable-optional-context-ref" + and "unresolved_default" in finding.message + for finding in findings + ) + + def test_optional_run_skill_input_accepts_declared_default( + self, + monkeypatch: pytest.MonkeyPatch, + ) -> None: + recipe = self._run_skill_recipe( + value="${{ context.optional_value }}", + optional_refs=["optional_value"], + required=False, + unresolved_default="", + monkeypatch=monkeypatch, + ) + + findings = run_semantic_rules(recipe) + + assert not any(finding.rule == "nullable-optional-context-ref" for finding in findings) + + @pytest.mark.parametrize( + "value", + [ + "${{ context.optional_value }}", + "/receipts/${{ context.optional_value }}/review.json", + ], + ) + def test_required_run_skill_input_rejects_optional_context_dependency( + self, + value: str, + monkeypatch: pytest.MonkeyPatch, + ) -> None: + recipe = self._run_skill_recipe( + value=value, + optional_refs=["optional_value"], + required=True, + unresolved_default=None, + monkeypatch=monkeypatch, + ) + + findings = run_semantic_rules(recipe) + + assert any( + finding.rule == "nullable-optional-context-ref" and "required input" in finding.message + for finding in findings + ) + + def test_unlisted_context_dependency_does_not_project_default_rule( + self, + monkeypatch: pytest.MonkeyPatch, + ) -> None: + recipe = self._run_skill_recipe( + value="${{ context.optional_value }}", + optional_refs=["different_value"], + required=False, + unresolved_default=None, + monkeypatch=monkeypatch, + ) + + findings = run_semantic_rules(recipe) + + assert not any(finding.rule == "nullable-optional-context-ref" for finding in findings) diff --git a/tests/recipe/test_skill_invocation_binding.py b/tests/recipe/test_skill_invocation_binding.py index 1d8d980c2c..0c9272e510 100644 --- a/tests/recipe/test_skill_invocation_binding.py +++ b/tests/recipe/test_skill_invocation_binding.py @@ -2,6 +2,7 @@ from __future__ import annotations +import dataclasses import json import os from pathlib import Path @@ -18,6 +19,7 @@ BoundValue, BoundValueOrigin, BoundValueState, + compute_invocation_template_digest, ) from autoskillit.recipe._binding import bind_recipe, bind_step_invocation from autoskillit.recipe._contracts_manifest import get_skill_contract @@ -52,6 +54,8 @@ def _manifest() -> dict[str, object]: def _step( skill_inputs: dict[str, str | int | bool], + *, + optional_context_refs: list[str] | None = None, **extra: str, ) -> RecipeStep: return RecipeStep( @@ -63,6 +67,7 @@ def _step( "skill_inputs": skill_inputs, **extra, }, + optional_context_refs=optional_context_refs or [], ) @@ -135,6 +140,23 @@ def test_bound_value_rejects_raw_enum_values( BoundValue(**kwargs) # type: ignore[arg-type] +def test_bound_value_rejects_non_scalar_unresolved_default() -> None: + with pytest.raises(ValueError, match="unresolved_default"): + BoundValue( + name="value", + declared_value="declared", + effective_value="effective", + state=BoundValueState.PRESENT, + origin=BoundValueOrigin.LITERAL, + unresolved_default=1.5, # type: ignore[arg-type] + ) + + +def test_absent_bound_value_rejects_unresolved_default() -> None: + with pytest.raises(ValueError, match="unresolved_default"): + dataclasses.replace(BoundValue.absent("value"), unresolved_default="") + + def test_bound_step_invocation_freezes_collection_inputs() -> None: value = BoundValue( name="value", @@ -205,6 +227,43 @@ def test_structured_binding_uses_contract_order_not_mapping_order() -> None: ) +def test_structured_binding_projects_step_local_unresolved_default_into_digest() -> None: + manifest = _manifest() + optional_note = manifest["skills"]["dry-walkthrough"]["inputs"][2] # type: ignore[index] + optional_note["unresolved_default"] = "" # type: ignore[index] + values = {**_required_inputs(), "optional_note": "${{ context.note }}"} + + with_default = bind_step_invocation( + "verify", + _step(values, optional_context_refs=["note"]), + manifest=manifest, + ) + without_default = bind_step_invocation( + "verify", + _step(values), + manifest=manifest, + ) + + projected = with_default.skill_input("optional_note") + unprojected = without_default.skill_input("optional_note") + omitted = with_default.skill_input("enabled") + assert projected is not None and projected.unresolved_default == "" + assert unprojected is not None and unprojected.unresolved_default is None + assert omitted is not None and omitted.state is BoundValueState.ABSENT + + digest_kwargs = { + "execution_id": "exec-1", + "recipe_name": "demo", + "content_hash": "sha256:" + "1" * 64, + "composite_hash": "sha256:" + "2" * 64, + "tool_contract_identity": "sha256:" + "3" * 64, + "skill_contract_identity": "sha256:" + "4" * 64, + } + assert compute_invocation_template_digest( + invocation=with_default, **digest_kwargs + ) != compute_invocation_template_digest(invocation=without_default, **digest_kwargs) + + def test_explicit_empty_manifest_is_not_replaced( monkeypatch: pytest.MonkeyPatch, ) -> None: From 8720195a09e3a3cbfb6933f7f240fdc74d5376e6 Mon Sep 17 00:00:00 2001 From: Trecek Date: Fri, 14 Aug 2026 16:43:32 -0700 Subject: [PATCH 03/18] feat: publish attested skill input shapes --- src/autoskillit/cli/_prompts.py | 8 +- src/autoskillit/core/types/_type_constants.py | 5 +- .../core/types/_type_recipe_execution.py | 16 +++ src/autoskillit/skills/sous-chef/SKILL.md | 6 + tests/cli/test_mcp_startup_recovery_policy.py | 16 +++ tests/cli/test_sous_chef_content.py | 16 +++ .../test_completion_receipt_fitness.py | 1 + .../core/test_recipe_execution_credential.py | 112 ++++++++++++++++++ .../test_attestation_delivery_reachability.py | 4 + tests/server/test_recipe_initialization.py | 1 + .../test_tools_execution_recipe_execution.py | 13 ++ 11 files changed, 196 insertions(+), 2 deletions(-) create mode 100644 tests/core/test_recipe_execution_credential.py diff --git a/src/autoskillit/cli/_prompts.py b/src/autoskillit/cli/_prompts.py index 894aa7cf5a..a983894203 100644 --- a/src/autoskillit/cli/_prompts.py +++ b/src/autoskillit/cli/_prompts.py @@ -169,7 +169,13 @@ def render(self) -> str: "the unchanged recipe_pull. The overview is only a table of contents. " "If subtype=recipe_segment_post_effect_delivery_failure, the operation " "already ran; do not repeat it. Do not assume startup contains " - "full-horizon invocation_template_digests." + "full-horizon invocation_template_digests. For structured child inputs, " + "select " + "recipe_execution.skill_input_shapes[step_name], initialize skill_inputs " + "with exactly its ordered keys, and replace available values in place. " + "For unavailable context, copy only that key's advertised " + 'unresolved_defaults entry by key presence, so "", 0, and False remain ' + "verbatim; never delete or invent a key." ), ), _McpStartupRecoveryClause( diff --git a/src/autoskillit/core/types/_type_constants.py b/src/autoskillit/core/types/_type_constants.py index 90ef492f20..aaa9bcc22d 100644 --- a/src/autoskillit/core/types/_type_constants.py +++ b/src/autoskillit/core/types/_type_constants.py @@ -797,7 +797,10 @@ class SkillFamilyDef(NamedTuple): "an active recipe requires recipe_execution_id and invocation_template_digest; " "take both from the recipe_execution block of the complete_recipe_initialization " "receipt (bounded delivery) or of the open_kitchen response (inline delivery), " - "using invocation_template_digests[step_name] for this step" + "using invocation_template_digests[step_name] for this step; structured calls must " + "initialize skill_inputs from skill_input_shapes[step_name] ordered keys, replace " + "available values in place, copy only advertised unresolved_defaults by key presence " + 'so "", 0, and False remain verbatim, and never delete or invent a key' ) RECIPE_EXECUTION_INACTIVE_MESSAGE: str = ( diff --git a/src/autoskillit/core/types/_type_recipe_execution.py b/src/autoskillit/core/types/_type_recipe_execution.py index fb56156c72..5bb00f1897 100644 --- a/src/autoskillit/core/types/_type_recipe_execution.py +++ b/src/autoskillit/core/types/_type_recipe_execution.py @@ -217,11 +217,15 @@ class RecipeExecutionCredential: execution_id: str snapshot_digest: str invocation_template_digests: Mapping[str, str] + skill_input_shapes: Mapping[str, Mapping[str, object]] def as_wire_block(self) -> dict[str, Any]: return { "execution_id": self.execution_id, "invocation_template_digests": dict(self.invocation_template_digests), + "skill_input_shapes": { + step_name: dict(shape) for step_name, shape in self.skill_input_shapes.items() + }, "snapshot_digest": self.snapshot_digest, } @@ -235,10 +239,22 @@ def build_recipe_execution_credential( snapshot: RecipeExecutionSnapshot, ) -> RecipeExecutionCredential: """Project the sole caller-visible credential for an execution snapshot.""" + skill_input_shapes: dict[str, Mapping[str, object]] = {} + for step_name, template in snapshot.templates.items(): + present = tuple(value for value in template.invocation.skill_inputs if value.is_present) + skill_input_shapes[step_name] = { + "keys": [value.name for value in present], + "unresolved_defaults": { + value.name: value.unresolved_default + for value in present + if value.unresolved_default is not None + }, + } return RecipeExecutionCredential( execution_id=snapshot.execution_id, snapshot_digest=snapshot.snapshot_digest, invocation_template_digests=dict(snapshot.template_digests), + skill_input_shapes=skill_input_shapes, ) diff --git a/src/autoskillit/skills/sous-chef/SKILL.md b/src/autoskillit/skills/sous-chef/SKILL.md index 7a565e0e75..59395adfbc 100644 --- a/src/autoskillit/skills/sous-chef/SKILL.md +++ b/src/autoskillit/skills/sous-chef/SKILL.md @@ -1078,6 +1078,12 @@ delivered in the response will cause `recipe_execution_attestation_missing` or Segmented delivery narrows this lookup to the latest delivered carrier. Do not assume the startup credential contains invocation digests for the full recipe horizon. +For structured child inputs, select `recipe_execution.skill_input_shapes[step_name]` and +initialize `skill_inputs` with exactly its ordered keys. Replace available values in place; +for unavailable context, copy a value only from that key's advertised +`unresolved_defaults` entry. Test key presence rather than truthiness so `""`, `0`, and +`False` are forwarded verbatim. Never delete or invent a key. + Similarly, NEVER read SKILL.md files directly from the filesystem. Use the Skill tool to load skill instructions — it applies runtime transformations (namespace rewriting, temp directory substitution, disable-model-invocation injection) that raw files lack. diff --git a/tests/cli/test_mcp_startup_recovery_policy.py b/tests/cli/test_mcp_startup_recovery_policy.py index 47a547eece..03a4886702 100644 --- a/tests/cli/test_mcp_startup_recovery_policy.py +++ b/tests/cli/test_mcp_startup_recovery_policy.py @@ -49,3 +49,19 @@ def test_every_clause_is_rendered_once_in_its_declared_phase() -> None: def test_canonical_instruction_is_rendered_from_the_policy() -> None: assert _prompts._MCP_RETRY_INSTRUCTION == _prompts._MCP_STARTUP_RECOVERY_SPEC.render() + + +def test_startup_policy_preserves_attested_skill_input_shape() -> None: + rendered = _prompts._MCP_STARTUP_RECOVERY_SPEC.render() + + for required in ( + "skill_input_shapes[step_name]", + "ordered keys", + "unresolved_defaults", + "replace available values in place", + "never delete or invent a key", + '""', + "0", + "False", + ): + assert required in rendered diff --git a/tests/cli/test_sous_chef_content.py b/tests/cli/test_sous_chef_content.py index bef14a07b1..e01cc82be4 100644 --- a/tests/cli/test_sous_chef_content.py +++ b/tests/cli/test_sous_chef_content.py @@ -83,3 +83,19 @@ def test_sous_chef_requires_progressive_segment_consumption() -> None: "full recipe horizon", ): assert required in content + + +def test_sous_chef_preserves_attested_skill_input_shape_and_falsey_defaults() -> None: + content = _read_full_sous_chef() + + for required in ( + "skill_input_shapes[step_name]", + "ordered keys", + "unresolved_defaults", + "replace available values in place", + "never delete or invent a key", + '""', + "0", + "False", + ): + assert required in content diff --git a/tests/contracts/test_completion_receipt_fitness.py b/tests/contracts/test_completion_receipt_fitness.py index 28f29403b7..d047a80a6e 100644 --- a/tests/contracts/test_completion_receipt_fitness.py +++ b/tests/contracts/test_completion_receipt_fitness.py @@ -105,3 +105,4 @@ def test_completion_receipt_fits_every_delivery_bound( assert measured["invocation_template_digests"] == dict( prepared.execution_snapshot.template_digests ), f"{recipe_name}: the measured receipt must carry the real credential" + assert measured["skill_input_shapes"] == credential.as_wire_block()["skill_input_shapes"] diff --git a/tests/core/test_recipe_execution_credential.py b/tests/core/test_recipe_execution_credential.py new file mode 100644 index 0000000000..012f1b245e --- /dev/null +++ b/tests/core/test_recipe_execution_credential.py @@ -0,0 +1,112 @@ +"""Recipe execution credential projections.""" + +from __future__ import annotations + +import pytest + +from autoskillit.core import ( + BindingMode, + BoundStepInvocation, + BoundValue, + BoundValueOrigin, + BoundValueState, + InvocationTemplate, + RecipeExecutionSnapshot, + build_recipe_execution_credential, + compute_invocation_template_digest, + compute_recipe_execution_snapshot_digest, +) + +pytestmark = [pytest.mark.layer("core"), pytest.mark.small] + + +def test_recipe_execution_credential_projects_ordered_keys_and_falsey_defaults() -> None: + values = ( + BoundValue( + name="empty", + declared_value="${{ context.empty }}", + effective_value="${{ context.empty }}", + state=BoundValueState.PRESENT, + origin=BoundValueOrigin.CONTEXT, + context_dependencies=("empty",), + unresolved_default="", + ), + BoundValue( + name="zero", + declared_value="${{ context.zero }}", + effective_value="${{ context.zero }}", + state=BoundValueState.PRESENT, + origin=BoundValueOrigin.CONTEXT, + context_dependencies=("zero",), + unresolved_default=0, + ), + BoundValue( + name="disabled", + declared_value="${{ context.disabled }}", + effective_value="${{ context.disabled }}", + state=BoundValueState.PRESENT, + origin=BoundValueOrigin.CONTEXT, + context_dependencies=("disabled",), + unresolved_default=False, + ), + BoundValue( + name="resolved", + declared_value="ready", + effective_value="ready", + state=BoundValueState.PRESENT, + origin=BoundValueOrigin.LITERAL, + ), + BoundValue.absent("undeclared"), + ) + invocation = BoundStepInvocation( + step_name="invoke", + tool_name="run_skill", + mode=BindingMode.RECIPE, + skill_name="demo-skill", + mcp_kwargs=(), + skill_inputs=values, + ) + execution_id = "execution-1" + content_hash = "sha256:" + "c" * 64 + composite_hash = "sha256:" + "d" * 64 + tool_identity = "tool-v1" + skill_identity = "skill-v1" + template_digest = compute_invocation_template_digest( + execution_id=execution_id, + recipe_name="demo", + content_hash=content_hash, + composite_hash=composite_hash, + invocation=invocation, + tool_contract_identity=tool_identity, + skill_contract_identity=skill_identity, + ) + template = InvocationTemplate( + invocation=invocation, + tool_contract_identity=tool_identity, + skill_contract_identity=skill_identity, + template_digest=template_digest, + ) + templates = {"invoke": template} + snapshot = RecipeExecutionSnapshot( + execution_id=execution_id, + recipe_name="demo", + content_hash=content_hash, + composite_hash=composite_hash, + templates=templates, + snapshot_digest=compute_recipe_execution_snapshot_digest( + execution_id=execution_id, + recipe_name="demo", + content_hash=content_hash, + composite_hash=composite_hash, + templates=templates, + ), + ) + + wire = build_recipe_execution_credential(snapshot).as_wire_block() + + assert wire["skill_input_shapes"] == { + "invoke": { + "keys": ["empty", "zero", "disabled", "resolved"], + "unresolved_defaults": {"empty": "", "zero": 0, "disabled": False}, + } + } diff --git a/tests/server/test_attestation_delivery_reachability.py b/tests/server/test_attestation_delivery_reachability.py index d0cf4ce053..9678445f82 100644 --- a/tests/server/test_attestation_delivery_reachability.py +++ b/tests/server/test_attestation_delivery_reachability.py @@ -82,6 +82,10 @@ async def test_bounded_initialization_delivers_attestation_credential( assert len(value) == len("sha256:") + 64 assert value.removeprefix("sha256:").islower() assert _ATTESTED_STEP in digests + shapes = credential["skill_input_shapes"] + assert _ATTESTED_STEP in shapes + assert shapes[_ATTESTED_STEP]["keys"] == list(ready.with_args["skill_inputs"]) + assert isinstance(shapes[_ATTESTED_STEP]["unresolved_defaults"], dict) assert credential["execution_id"] assert isinstance(ready.tool_ctx.recipe_initialization_state, ReadyRecipe) diff --git a/tests/server/test_recipe_initialization.py b/tests/server/test_recipe_initialization.py index c38a079461..835f187a23 100644 --- a/tests/server/test_recipe_initialization.py +++ b/tests/server/test_recipe_initialization.py @@ -164,6 +164,7 @@ def test_completion_is_server_owned_and_commits_ready_only_after_enforcement( assert set(parsed_initial["recipe_execution"].keys()) == { "execution_id", "invocation_template_digests", + "skill_input_shapes", "snapshot_digest", } assert parsed_initial["recipe_execution"] == parsed_replay["recipe_execution"] diff --git a/tests/server/test_tools_execution_recipe_execution.py b/tests/server/test_tools_execution_recipe_execution.py index 4674edee07..c83c4fbf42 100644 --- a/tests/server/test_tools_execution_recipe_execution.py +++ b/tests/server/test_tools_execution_recipe_execution.py @@ -29,6 +29,19 @@ def test_attestation_missing_message_names_all_required_params(self) -> None: def test_attestation_missing_message_names_remedy_tool(self) -> None: assert "complete_recipe_initialization" in RECIPE_EXECUTION_ATTESTATION_MISSING_MESSAGE + def test_attestation_missing_message_preserves_delivered_skill_input_shape(self) -> None: + for required in ( + "skill_input_shapes[step_name]", + "ordered keys", + "unresolved_defaults", + "replace available values in place", + "never delete or invent a key", + '""', + "0", + "False", + ): + assert required in RECIPE_EXECUTION_ATTESTATION_MISSING_MESSAGE + def test_inactive_message_does_not_say_standalone_mode(self) -> None: assert "standalone mode" not in RECIPE_EXECUTION_INACTIVE_MESSAGE.lower() From 5368c2ed43cb5a132a718bd08454c9bd1bd7501d Mon Sep 17 00:00:00 2001 From: Trecek Date: Fri, 14 Aug 2026 16:53:13 -0700 Subject: [PATCH 04/18] fix: remediate optional context skill inputs --- src/autoskillit/recipe/skill_contracts.yaml | 14 +++ .../recipes/implementation-groups.json | 3 +- .../recipes/implementation-groups.yaml | 3 +- src/autoskillit/recipes/implementation.json | 3 +- src/autoskillit/recipes/implementation.yaml | 3 +- src/autoskillit/recipes/merge-prs.json | 42 +++++-- src/autoskillit/recipes/merge-prs.yaml | 26 +++-- src/autoskillit/recipes/remediation.json | 3 +- src/autoskillit/recipes/remediation.yaml | 3 +- src/autoskillit/recipes/research-design.json | 3 +- src/autoskillit/recipes/research-design.yaml | 4 +- src/autoskillit/recipes/research-review.json | 29 ++--- src/autoskillit/recipes/research-review.yaml | 25 ++--- src/autoskillit/recipes/research.json | 2 - src/autoskillit/recipes/research.yaml | 2 - .../test_completion_receipt_fitness.py | 62 +++++++++++ tests/recipe/test_bundled_recipes_general.py | 103 ++++++++++++++++++ .../test_review_loop_routing_invariant.py | 10 ++ .../test_runtime_skill_invocation_binding.py | 69 +++++++++++- 19 files changed, 332 insertions(+), 77 deletions(-) diff --git a/src/autoskillit/recipe/skill_contracts.yaml b/src/autoskillit/recipe/skill_contracts.yaml index 7ca81d9b5c..b59ace353b 100644 --- a/src/autoskillit/recipe/skill_contracts.yaml +++ b/src/autoskillit/recipe/skill_contracts.yaml @@ -44,6 +44,7 @@ skills: - name: event type: string required: false + unresolved_default: "" outputs: - name: diagnosis_path type: file_path @@ -83,6 +84,7 @@ skills: - name: revision_guidance type: file_path required: false + unresolved_default: "" outputs: - name: resolution type: string @@ -110,12 +112,14 @@ skills: - name: ci_conclusion type: string required: false + unresolved_default: "" - name: ci_failed_jobs type: string required: false - name: diagnosis_path type: file_path required: false + unresolved_default: "" outputs: - name: verdict type: string @@ -176,6 +180,7 @@ skills: - name: mode type: string required: false + unresolved_default: "" - name: repository type: string required: false @@ -252,12 +257,15 @@ skills: - name: review_path type: file_path required: false + unresolved_default: "" - name: audit_cycle_path type: file_path required: false + unresolved_default: "" - name: plan_disposition_path type: file_path required: false + unresolved_default: "" outputs: [] write_behavior: always input_preflight: audit_cycle_inventory @@ -343,6 +351,7 @@ skills: - name: audit_cycle_path type: file_path required: false + unresolved_default: "" outputs: - name: verdict type: string @@ -513,9 +522,11 @@ skills: - name: deviation_manifest_path type: file_path required: false + unresolved_default: "" - name: prior_audit_cycle_path type: file_path required: false + unresolved_default: "" outputs: - name: verdict type: string @@ -805,6 +816,7 @@ skills: - name: mode type: string required: false + unresolved_default: "" - name: repository type: string required: true @@ -1559,9 +1571,11 @@ skills: - name: scope_directions_path type: file_path required: false + unresolved_default: "" - name: revision_guidance type: file_path required: false + unresolved_default: "" outputs: - name: experiment_plan type: file_path diff --git a/src/autoskillit/recipes/implementation-groups.json b/src/autoskillit/recipes/implementation-groups.json index 886fe43761..148f330d18 100644 --- a/src/autoskillit/recipes/implementation-groups.json +++ b/src/autoskillit/recipes/implementation-groups.json @@ -1231,7 +1231,7 @@ "review_loop_count": "${{ result.value }}" }, "on_success": "clear_review_annotation_context", - "on_failure": "clear_review_annotation_context", + "on_failure": "release_issue_failure", "optional_context_refs": [ "review_loop_count" ] @@ -1348,7 +1348,6 @@ "skip_when_false": "inputs.open_pr", "on_skip": "release_issue_success", "optional_context_refs": [ - "review_loop_count", "review_mode" ], "note": "Runs after compose_pr creates the PR. Invokes the review-pr skill as a headless session to leave inline review comments on the PR. The skill finds the open PR by feature branch name (context.merge_target). Captures verdict as review_verdict and routes via on_result: changes_requested/approved_with_comments → resolve_review; all other verdicts (including approved, needs_human) → check_review_posted (effect verification gate) → check_review_loop (mandatory waypoint to increment review_loop_count). On tool-level failure (MCP error, gh unavailable) or context exhaustion, routes to release_issue_failure because no authoritative review receipt exists. Skipped when open_pr=false.\n" diff --git a/src/autoskillit/recipes/implementation-groups.yaml b/src/autoskillit/recipes/implementation-groups.yaml index ca7e40c359..0a9297b5f8 100644 --- a/src/autoskillit/recipes/implementation-groups.yaml +++ b/src/autoskillit/recipes/implementation-groups.yaml @@ -1067,7 +1067,7 @@ steps: capture: review_loop_count: ${{ result.value }} on_success: clear_review_annotation_context - on_failure: clear_review_annotation_context + on_failure: release_issue_failure optional_context_refs: - review_loop_count clear_review_annotation_context: @@ -1164,7 +1164,6 @@ steps: skip_when_false: inputs.open_pr on_skip: release_issue_success optional_context_refs: - - review_loop_count - review_mode note: > Runs after compose_pr creates the PR. Invokes the review-pr skill as a diff --git a/src/autoskillit/recipes/implementation.json b/src/autoskillit/recipes/implementation.json index 438f353ade..7f24b63c47 100644 --- a/src/autoskillit/recipes/implementation.json +++ b/src/autoskillit/recipes/implementation.json @@ -1564,7 +1564,7 @@ "review_loop_count": "${{ result.value }}" }, "on_success": "clear_review_annotation_context", - "on_failure": "clear_review_annotation_context", + "on_failure": "release_issue_failure", "optional_context_refs": [ "review_loop_count" ] @@ -1682,7 +1682,6 @@ "skip_when_false": "inputs.open_pr", "on_skip": "release_issue_success", "optional_context_refs": [ - "review_loop_count", "review_mode" ], "note": "Runs after compose_pr creates the PR. Invokes the review-pr skill as a headless session to leave inline review comments on the PR. The skill finds the open PR by feature branch name (context.merge_target). Captures verdict as review_verdict and routes via on_result: changes_requested/approved_with_comments → enrich_diff_context → resolve_review; all other verdicts (including approved, needs_human) → check_review_posted (effect verification gate) → check_review_loop (mandatory waypoint to increment review_loop_count). On tool-level failure or context exhaustion, routes to release_issue_failure because no authoritative review receipt exists. Skipped when open_pr=false.\n" diff --git a/src/autoskillit/recipes/implementation.yaml b/src/autoskillit/recipes/implementation.yaml index 87337713e0..ec9599d5ae 100644 --- a/src/autoskillit/recipes/implementation.yaml +++ b/src/autoskillit/recipes/implementation.yaml @@ -1296,7 +1296,7 @@ steps: capture: review_loop_count: ${{ result.value }} on_success: clear_review_annotation_context - on_failure: clear_review_annotation_context + on_failure: release_issue_failure optional_context_refs: - review_loop_count clear_review_annotation_context: @@ -1395,7 +1395,6 @@ steps: skip_when_false: inputs.open_pr on_skip: release_issue_success optional_context_refs: - - review_loop_count - review_mode note: 'Runs after compose_pr creates the PR. Invokes the review-pr skill as a headless session to leave inline review comments on the PR. The skill finds diff --git a/src/autoskillit/recipes/merge-prs.json b/src/autoskillit/recipes/merge-prs.json index 099e1c6ec4..2d21fbec6c 100644 --- a/src/autoskillit/recipes/merge-prs.json +++ b/src/autoskillit/recipes/merge-prs.json @@ -435,12 +435,27 @@ "route": "push_ejected_fix" }, { - "route": "resolve_ejected_conflicts" + "route": "gate_ejected_conflict_plan" } ], - "on_failure": "resolve_ejected_conflicts", + "on_failure": "gate_ejected_conflict_plan", "note": "Attempts a trivial rebase before invoking the full resolve-merge-conflicts skill. Uses ejected_pr_branch captured by get_ejected_pr_branch. On clean rebase, skips the skill and proceeds directly to push_ejected_fix. On conflict, aborts the rebase and routes to resolve_ejected_conflicts for goal-aware resolution.\n" }, + "gate_ejected_conflict_plan": { + "action": "route", + "on_result": [ + { + "when": "${{ context.pr_plan_path }} != ''", + "route": "resolve_ejected_conflicts" + }, + { + "route": "register_clone_failure" + } + ], + "optional_context_refs": [ + "pr_plan_path" + ] + }, "resolve_ejected_conflicts": { "tool": "run_skill", "model": "", @@ -461,9 +476,6 @@ "capture_list": { "all_conflict_report_paths": "${{ result.conflict_report_path }}" }, - "optional_context_refs": [ - "pr_plan_path" - ], "on_result": [ { "when": "${{ result.escalation_required }} == true", @@ -746,12 +758,27 @@ "route": "push_rebased_next_pr" }, { - "route": "resolve_proactive_rebase_conflicts" + "route": "gate_proactive_rebase_conflict_plan" } ], "on_failure": "get_current_pr_branch", "note": "Attempts a proactive rebase of the next PR branch onto the current base_branch HEAD before submitting to the merge queue. On clean rebase, skips conflict resolution entirely and proceeds to push. On conflict, routes to resolve_proactive_rebase_conflicts. On git command failure, falls through to get_current_pr_branch to enforce the CI gate.\n" }, + "gate_proactive_rebase_conflict_plan": { + "action": "route", + "on_result": [ + { + "when": "${{ context.pr_plan_path }} != ''", + "route": "resolve_proactive_rebase_conflicts" + }, + { + "route": "register_clone_failure" + } + ], + "optional_context_refs": [ + "pr_plan_path" + ] + }, "resolve_proactive_rebase_conflicts": { "tool": "run_skill", "model": "", @@ -773,9 +800,6 @@ "capture_list": { "all_conflict_report_paths": "${{ result.conflict_report_path }}" }, - "optional_context_refs": [ - "pr_plan_path" - ], "on_result": [ { "when": "${{ result.escalation_required }} == true", diff --git a/src/autoskillit/recipes/merge-prs.yaml b/src/autoskillit/recipes/merge-prs.yaml index 3d777e2f2e..6bcc09ecfe 100644 --- a/src/autoskillit/recipes/merge-prs.yaml +++ b/src/autoskillit/recipes/merge-prs.yaml @@ -418,13 +418,21 @@ steps: on_result: - when: ${{ result.status }} == clean route: push_ejected_fix - - route: resolve_ejected_conflicts - on_failure: resolve_ejected_conflicts + - route: gate_ejected_conflict_plan + on_failure: gate_ejected_conflict_plan note: 'Attempts a trivial rebase before invoking the full resolve-merge-conflicts skill. Uses ejected_pr_branch captured by get_ejected_pr_branch. On clean rebase, skips the skill and proceeds directly to push_ejected_fix. On conflict, aborts the rebase and routes to resolve_ejected_conflicts for goal-aware resolution. ' + gate_ejected_conflict_plan: + action: route + on_result: + - when: ${{ context.pr_plan_path }} != '' + route: resolve_ejected_conflicts + - route: register_clone_failure + optional_context_refs: + - pr_plan_path resolve_ejected_conflicts: tool: run_skill model: '' @@ -441,8 +449,6 @@ steps: conflict_escalation_required: ${{ result.escalation_required }} capture_list: all_conflict_report_paths: ${{ result.conflict_report_path }} - optional_context_refs: - - pr_plan_path on_result: - when: ${{ result.escalation_required }} == true route: register_clone_failure @@ -658,13 +664,21 @@ steps: on_result: - when: ${{ result.status }} == clean route: push_rebased_next_pr - - route: resolve_proactive_rebase_conflicts + - route: gate_proactive_rebase_conflict_plan on_failure: get_current_pr_branch note: 'Attempts a proactive rebase of the next PR branch onto the current base_branch HEAD before submitting to the merge queue. On clean rebase, skips conflict resolution entirely and proceeds to push. On conflict, routes to resolve_proactive_rebase_conflicts. On git command failure, falls through to get_current_pr_branch to enforce the CI gate. ' + gate_proactive_rebase_conflict_plan: + action: route + on_result: + - when: ${{ context.pr_plan_path }} != '' + route: resolve_proactive_rebase_conflicts + - route: register_clone_failure + optional_context_refs: + - pr_plan_path resolve_proactive_rebase_conflicts: tool: run_skill model: '' @@ -682,8 +696,6 @@ steps: conflict_escalation_reason: ${{ result.escalation_reason }} capture_list: all_conflict_report_paths: ${{ result.conflict_report_path }} - optional_context_refs: - - pr_plan_path on_result: - when: ${{ result.escalation_required }} == true route: register_clone_failure diff --git a/src/autoskillit/recipes/remediation.json b/src/autoskillit/recipes/remediation.json index 41d419f7a1..2a1799d706 100644 --- a/src/autoskillit/recipes/remediation.json +++ b/src/autoskillit/recipes/remediation.json @@ -1899,7 +1899,7 @@ "review_loop_count": "${{ result.value }}" }, "on_success": "clear_review_annotation_context", - "on_failure": "clear_review_annotation_context", + "on_failure": "release_issue_failure", "optional_context_refs": [ "review_loop_count" ] @@ -2017,7 +2017,6 @@ "skip_when_false": "inputs.open_pr", "on_skip": "release_issue_success", "optional_context_refs": [ - "review_loop_count", "review_mode" ], "note": "Runs after compose_pr creates the PR. Invokes the review-pr skill as a headless session to leave inline review comments on the PR. The skill finds the open PR by feature branch name (context.merge_target). Captures verdict as review_verdict and routes via on_result: changes_requested/approved_with_comments → resolve_review; all other verdicts (including approved, needs_human) → check_review_posted (effect verification gate) → check_review_loop (mandatory waypoint to increment review_loop_count). On tool-level failure (MCP error, gh unavailable) or context exhaustion, routes to release_issue_failure because no authoritative review receipt exists. Skipped when open_pr=false.\n" diff --git a/src/autoskillit/recipes/remediation.yaml b/src/autoskillit/recipes/remediation.yaml index 2a8f3aba1d..289a803c3a 100644 --- a/src/autoskillit/recipes/remediation.yaml +++ b/src/autoskillit/recipes/remediation.yaml @@ -1586,7 +1586,7 @@ steps: capture: review_loop_count: ${{ result.value }} on_success: clear_review_annotation_context - on_failure: clear_review_annotation_context + on_failure: release_issue_failure optional_context_refs: - review_loop_count clear_review_annotation_context: @@ -1685,7 +1685,6 @@ steps: skip_when_false: inputs.open_pr on_skip: release_issue_success optional_context_refs: - - review_loop_count - review_mode note: 'Runs after compose_pr creates the PR. Invokes the review-pr skill as a headless session to leave inline review comments on the PR. The skill finds diff --git a/src/autoskillit/recipes/research-design.json b/src/autoskillit/recipes/research-design.json index 0dcc7e1e53..0b5155f832 100644 --- a/src/autoskillit/recipes/research-design.json +++ b/src/autoskillit/recipes/research-design.json @@ -115,7 +115,7 @@ "classification_timestamp" ], "skip_when_false": "inputs.review_design", - "on_skip": "select_review_dimensions", + "on_skip": "vis_dial", "on_context_limit": "synthesize", "on_success": "select_review_dimensions", "on_failure": "escalate_stop" @@ -159,7 +159,6 @@ }, "optional_context_refs": [ "is_silent_type", - "classification_timestamp", "design_review_count" ], "skip_when_true": "context.is_silent_type", diff --git a/src/autoskillit/recipes/research-design.yaml b/src/autoskillit/recipes/research-design.yaml index fb619577e2..29fc659c06 100644 --- a/src/autoskillit/recipes/research-design.yaml +++ b/src/autoskillit/recipes/research-design.yaml @@ -111,7 +111,7 @@ steps: classification_timestamp: "${{ result.classification_timestamp }}" pass_through: [experiment_type, is_silent_type, classification_timestamp] skip_when_false: inputs.review_design - on_skip: select_review_dimensions + on_skip: vis_dial on_context_limit: synthesize on_success: select_review_dimensions on_failure: escalate_stop @@ -148,7 +148,7 @@ steps: capture: findings_manifest_path: "${{ result.findings_manifest_path }}" evaluation_dashboard: "${{ result.evaluation_dashboard_path }}" - optional_context_refs: [is_silent_type, classification_timestamp, design_review_count] + optional_context_refs: [is_silent_type, design_review_count] skip_when_true: context.is_silent_type retries: 2 on_exhausted: synthesize diff --git a/src/autoskillit/recipes/research-review.json b/src/autoskillit/recipes/research-review.json index f3c79dd5eb..275c6c9455 100644 --- a/src/autoskillit/recipes/research-review.json +++ b/src/autoskillit/recipes/research-review.json @@ -258,13 +258,10 @@ "review_research_pr": { "tool": "run_skill", "model": "", - "optional_context_refs": [ - "worktree_path" - ], "with": { "skill_command": "/autoskillit:review-research-pr", "skill_inputs": { - "worktree_path": "${{ context.worktree_path }}", + "worktree_path": "${{ inputs.worktree_path }}", "base_branch": "${{ inputs.base_branch }}", "pr_url": "${{ context.pr_url }}", "repository": "${{ context.review_repository }}", @@ -277,7 +274,7 @@ "hunk_ranges_path": "${{ context.hunk_ranges_path }}", "valid_lines_path": "${{ context.valid_lines_path }}" }, - "cwd": "${{ context.worktree_path }}", + "cwd": "${{ inputs.worktree_path }}", "step_name": "review_research_pr", "output_dir": "{{AUTOSKILLIT_TEMP}}/review-research-pr/research-review" }, @@ -340,13 +337,10 @@ "audit_claims": { "tool": "run_skill", "model": "", - "optional_context_refs": [ - "worktree_path" - ], "with": { "skill_command": "/autoskillit:audit-claims", "skill_inputs": { - "worktree_path": "${{ context.worktree_path }}", + "worktree_path": "${{ inputs.worktree_path }}", "base_branch": "${{ inputs.base_branch }}", "pr_url": "${{ context.pr_url }}", "repository": "${{ context.review_repository }}", @@ -355,7 +349,7 @@ "logical_iteration": "audit-claims:research-review", "receipt_path": "{{AUTOSKILLIT_TEMP}}/audit-claims/batch_review_response_${{ context.pr_number }}.json" }, - "cwd": "${{ context.worktree_path }}", + "cwd": "${{ inputs.worktree_path }}", "step_name": "audit_claims", "output_dir": "{{AUTOSKILLIT_TEMP}}/audit-claims" }, @@ -507,16 +501,13 @@ "model": "", "stale_threshold": 2400, "idle_output_timeout": 0, - "optional_context_refs": [ - "worktree_path" - ], "with": { "skill_command": "/autoskillit:run-experiment", "skill_inputs": { - "worktree_path": "${{ context.worktree_path }}", + "worktree_path": "${{ inputs.worktree_path }}", "adjust": true }, - "cwd": "${{ context.worktree_path }}", + "cwd": "${{ inputs.worktree_path }}", "step_name": "re_run_experiment", "output_dir": "." }, @@ -546,19 +537,15 @@ "re_generate_report": { "tool": "run_skill", "model": "", - "optional_context_refs": [ - "worktree_path", - "experiment_results" - ], "with": { "skill_command": "/autoskillit:generate-report", "skill_inputs": { - "worktree_path": "${{ context.worktree_path }}", + "worktree_path": "${{ inputs.worktree_path }}", "results_path": "${{ context.experiment_results }}", "output_mode": "${{ inputs.output_mode }}", "issue_url": "${{ inputs.issue_url }}" }, - "cwd": "${{ context.worktree_path }}", + "cwd": "${{ inputs.worktree_path }}", "step_name": "re_generate_report", "output_dir": "{{AUTOSKILLIT_TEMP}}/generate-report" }, diff --git a/src/autoskillit/recipes/research-review.yaml b/src/autoskillit/recipes/research-review.yaml index 1d088b26c2..ab2c1252e7 100644 --- a/src/autoskillit/recipes/research-review.yaml +++ b/src/autoskillit/recipes/research-review.yaml @@ -243,12 +243,10 @@ steps: review_research_pr: tool: run_skill model: '' - optional_context_refs: - - worktree_path with: skill_command: /autoskillit:review-research-pr skill_inputs: - worktree_path: "${{ context.worktree_path }}" + worktree_path: "${{ inputs.worktree_path }}" base_branch: "${{ inputs.base_branch }}" pr_url: "${{ context.pr_url }}" repository: "${{ context.review_repository }}" @@ -260,7 +258,7 @@ steps: annotated_diff_path: "${{ context.annotated_diff_path }}" hunk_ranges_path: "${{ context.hunk_ranges_path }}" valid_lines_path: "${{ context.valid_lines_path }}" - cwd: "${{ context.worktree_path }}" + cwd: "${{ inputs.worktree_path }}" step_name: review_research_pr output_dir: "{{AUTOSKILLIT_TEMP}}/review-research-pr/research-review" capture: @@ -314,12 +312,10 @@ steps: audit_claims: tool: run_skill model: '' - optional_context_refs: - - worktree_path with: skill_command: "/autoskillit:audit-claims" skill_inputs: - worktree_path: "${{ context.worktree_path }}" + worktree_path: "${{ inputs.worktree_path }}" base_branch: "${{ inputs.base_branch }}" pr_url: "${{ context.pr_url }}" repository: "${{ context.review_repository }}" @@ -327,7 +323,7 @@ steps: pr_head_sha: "${{ context.pr_head_sha }}" logical_iteration: "audit-claims:research-review" receipt_path: "{{AUTOSKILLIT_TEMP}}/audit-claims/batch_review_response_${{ context.pr_number }}.json" - cwd: "${{ context.worktree_path }}" + cwd: "${{ inputs.worktree_path }}" step_name: audit_claims output_dir: '{{AUTOSKILLIT_TEMP}}/audit-claims' capture: @@ -454,14 +450,12 @@ steps: model: '' stale_threshold: 2400 idle_output_timeout: 0 - optional_context_refs: - - worktree_path with: skill_command: /autoskillit:run-experiment skill_inputs: - worktree_path: ${{ context.worktree_path }} + worktree_path: ${{ inputs.worktree_path }} adjust: true - cwd: ${{ context.worktree_path }} + cwd: ${{ inputs.worktree_path }} step_name: re_run_experiment output_dir: . capture: @@ -486,17 +480,14 @@ steps: re_generate_report: tool: run_skill model: '' - optional_context_refs: - - worktree_path - - experiment_results with: skill_command: /autoskillit:generate-report skill_inputs: - worktree_path: ${{ context.worktree_path }} + worktree_path: ${{ inputs.worktree_path }} results_path: ${{ context.experiment_results }} output_mode: ${{ inputs.output_mode }} issue_url: ${{ inputs.issue_url }} - cwd: ${{ context.worktree_path }} + cwd: ${{ inputs.worktree_path }} step_name: re_generate_report output_dir: '{{AUTOSKILLIT_TEMP}}/generate-report' capture: diff --git a/src/autoskillit/recipes/research.json b/src/autoskillit/recipes/research.json index 7ef79a60a8..96bde9790a 100644 --- a/src/autoskillit/recipes/research.json +++ b/src/autoskillit/recipes/research.json @@ -380,8 +380,6 @@ "output_dir": "{{AUTOSKILLIT_TEMP}}/resolve-design-review/iter_${{ context.design_review_count }}" }, "optional_context_refs": [ - "evaluation_dashboard", - "experiment_plan", "revision_guidance" ], "capture": { diff --git a/src/autoskillit/recipes/research.yaml b/src/autoskillit/recipes/research.yaml index dfb07ae434..92d64f0059 100644 --- a/src/autoskillit/recipes/research.yaml +++ b/src/autoskillit/recipes/research.yaml @@ -351,8 +351,6 @@ steps: step_name: resolve_design_review output_dir: '{{AUTOSKILLIT_TEMP}}/resolve-design-review/iter_${{ context.design_review_count }}' optional_context_refs: - - evaluation_dashboard - - experiment_plan - revision_guidance capture: revision_guidance: ${{ result.revision_guidance }} diff --git a/tests/contracts/test_completion_receipt_fitness.py b/tests/contracts/test_completion_receipt_fitness.py index d047a80a6e..113b60559e 100644 --- a/tests/contracts/test_completion_receipt_fitness.py +++ b/tests/contracts/test_completion_receipt_fitness.py @@ -38,6 +38,68 @@ pytestmark = [pytest.mark.layer("contracts"), pytest.mark.small] +def test_implementation_plan_shape_supplies_unavailable_audit_context( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + from autoskillit.recipe import _api_cache + from autoskillit.recipe._api_cache import LoadCache + from autoskillit.recipe._binding import bind_runtime_skill_invocation + from autoskillit.server import _recipe_generation + + monkeypatch.setattr(_api_cache, "_LOAD_CACHE", LoadCache()) + monkeypatch.setattr(_recipe_generation, "_RECIPE_GENERATION_STORE", RecipeGenerationStore()) + payload, projection = _full_open_kitchen_generation("implementation") + tool_ctx = cast( + Any, + SimpleNamespace( + backend=None, + kitchen_id="implementation-plan-shape", + temp_dir=tmp_path, + ), + ) + prepared = prepare_recipe_delivery_generation( + payload, + recipe_name="implementation", + tool_ctx=tool_ctx, + finalized_projection=projection, + ) + credential = build_recipe_execution_credential(prepared.execution_snapshot) + shape = credential.as_wire_block()["skill_input_shapes"]["plan"] + assert shape == { + "keys": ["task", "issue_url", "adversarial_review_level", "audit_cycle_path"], + "unresolved_defaults": {"audit_cycle_path": ""}, + } + + resolved = { + "task": "test task", + "issue_url": "https://github.com/test/test/issues/1", + "adversarial_review_level": "standard", + } + defaults = cast(dict[str, str | int | bool], shape["unresolved_defaults"]) + assembled = { + name: resolved[name] if name in resolved else defaults[name] + for name in cast(list[str], shape["keys"]) + } + template = prepared.execution_snapshot.templates["plan"] + actual_mcp_kwargs = { + value.name: value.effective_value + for value in template.invocation.mcp_kwargs + if isinstance(value.effective_value, (str, int, bool)) + } + + bound = bind_runtime_skill_invocation( + template, + execution_id=prepared.execution_snapshot.execution_id, + step_name="plan", + skill_command="/autoskillit:make-plan", + skill_inputs=assembled, + actual_mcp_kwargs=actual_mcp_kwargs, + ) + + assert dict(bound) == assembled + + @pytest.mark.parametrize("recipe_name", _delivery_recipe_names(), ids=lambda n: n) def test_completion_receipt_fits_every_delivery_bound( recipe_name: str, diff --git a/tests/recipe/test_bundled_recipes_general.py b/tests/recipe/test_bundled_recipes_general.py index 7a6bdd9705..34fd62a9c3 100644 --- a/tests/recipe/test_bundled_recipes_general.py +++ b/tests/recipe/test_bundled_recipes_general.py @@ -33,6 +33,109 @@ def _resolve_recipe_path(name: str) -> Path: return builtin_recipes_dir() / f"{name}.yaml" +def test_optional_context_structured_skill_input_inventory_is_explicit() -> None: + from autoskillit.recipe._binding import bind_recipe + from autoskillit.recipe.contracts import get_skill_contract + + recipe_names = ( + "implementation", + "implementation-groups", + "merge-prs", + "remediation", + "research", + "research-design", + "research-implement", + "research-review", + ) + expected_pairs = { + ("audit-impl", "deviation_manifest_path"), + ("audit-impl", "prior_audit_cycle_path"), + ("diagnose-ci", "event"), + ("dry-walkthrough", "audit_cycle_path"), + ("dry-walkthrough", "plan_disposition_path"), + ("dry-walkthrough", "review_path"), + ("make-plan", "audit_cycle_path"), + ("plan-experiment", "revision_guidance"), + ("plan-experiment", "scope_directions_path"), + ("resolve-design-review", "revision_guidance"), + ("resolve-failures", "ci_conclusion"), + ("resolve-failures", "diagnosis_path"), + ("resolve-review", "mode"), + ("review-pr", "mode"), + } + manifest = load_bundled_manifest() + occurrences: list[tuple[str, str, str, str]] = [] + required_occurrences: list[tuple[str, str, str, str]] = [] + for recipe_name in recipe_names: + recipe = load_recipe(builtin_recipes_dir() / f"{recipe_name}.yaml") + projection = bind_recipe(recipe, manifest=manifest) + for step_name, step in recipe.steps.items(): + invocation = projection.for_step(step_name) + if invocation is None or invocation.skill_name is None: + continue + optional_refs = frozenset(step.optional_context_refs) + contract = get_skill_contract(invocation.skill_name, manifest) + assert contract is not None + input_by_name = {item.name: item for item in contract.inputs} + for value in invocation.skill_inputs: + if not value.is_present or not ( + optional_refs & frozenset(value.context_dependencies) + ): + continue + occurrence = ( + recipe_name, + step_name, + invocation.skill_name, + value.name, + ) + if input_by_name[value.name].required: + required_occurrences.append(occurrence) + else: + occurrences.append(occurrence) + + actual_pairs = {(skill, input_name) for _, _, skill, input_name in occurrences} + assert len(occurrences) == 49 + assert actual_pairs == expected_pairs + assert not required_occurrences + for skill_name, input_name in expected_pairs: + contract = get_skill_contract(skill_name, manifest) + assert contract is not None + input_def = next(item for item in contract.inputs if item.name == input_name) + assert input_def.unresolved_default == "" + + +def test_required_optional_context_routes_are_gated_or_guaranteed() -> None: + merge_prs = load_recipe(builtin_recipes_dir() / "merge-prs.yaml") + for prefix in ("ejected", "proactive_rebase"): + gate = merge_prs.steps[f"gate_{prefix}_conflict_plan"] + routes = [condition.route for condition in gate.on_result.conditions] + assert routes[-1] == "register_clone_failure" + resolve = merge_prs.steps[f"resolve_{prefix}_conflicts"] + assert "pr_plan_path" not in resolve.optional_context_refs + + research = load_recipe(builtin_recipes_dir() / "research.yaml") + assert research.steps["resolve_design_review"].optional_context_refs == ["revision_guidance"] + + research_design = load_recipe(builtin_recipes_dir() / "research-design.yaml") + assert research_design.steps["dial"].on_skip == "vis_dial" + assert "classification_timestamp" not in research_design.steps["apply"].optional_context_refs + + research_review = load_recipe(builtin_recipes_dir() / "research-review.yaml") + for step_name in ( + "review_research_pr", + "audit_claims", + "re_run_experiment", + "re_generate_report", + ): + step = research_review.steps[step_name] + assert step.with_args["skill_inputs"]["worktree_path"] == "${{ inputs.worktree_path }}" + assert "worktree_path" not in step.optional_context_refs + assert ( + "experiment_results" + not in research_review.steps["re_generate_report"].optional_context_refs + ) + + _ALL_CLONE_RECIPE_PATHS: list[Path] = [] for _p in _ALL_RECIPE_PATHS: _wf = load_recipe(_p) diff --git a/tests/recipe/test_review_loop_routing_invariant.py b/tests/recipe/test_review_loop_routing_invariant.py index a3a0740119..8fa13d6c12 100644 --- a/tests/recipe/test_review_loop_routing_invariant.py +++ b/tests/recipe/test_review_loop_routing_invariant.py @@ -20,6 +20,16 @@ REVIEW_LOOP_RECIPES = ["implementation", "remediation", "implementation-groups"] +@pytest.mark.parametrize("recipe_name", REVIEW_LOOP_RECIPES) +def test_review_counter_initialization_failure_cannot_reach_review_pr( + recipe_name: str, +) -> None: + recipe = load_recipe(builtin_recipes_dir() / f"{recipe_name}.yaml") + + assert recipe.steps["init_review_loop_count"].on_failure == "release_issue_failure" + assert "review_loop_count" not in recipe.steps["review_pr"].optional_context_refs + + @pytest.mark.parametrize("recipe_name", REVIEW_LOOP_RECIPES) def test_approved_verdict_must_not_route_directly_to_ci(recipe_name: str) -> None: """The catch-all on_result may not route directly to check_repo_ci_event. diff --git a/tests/recipe/test_runtime_skill_invocation_binding.py b/tests/recipe/test_runtime_skill_invocation_binding.py index 314cb07e1a..25aaa7946e 100644 --- a/tests/recipe/test_runtime_skill_invocation_binding.py +++ b/tests/recipe/test_runtime_skill_invocation_binding.py @@ -10,6 +10,8 @@ from __future__ import annotations +from collections.abc import Mapping + import pytest from autoskillit.core import ( @@ -34,6 +36,12 @@ "dry-walkthrough": { "inputs": [ {"name": "plan_path", "type": "file_path", "required": True}, + { + "name": "audit_cycle_path", + "type": "file_path", + "required": False, + "unresolved_default": "", + }, ] } } @@ -42,7 +50,11 @@ _STEP_NAME = "verify" -def _template(**extra_with_args: object) -> InvocationTemplate: +def _template( + *, + optional_context_refs: list[str] | None = None, + **extra_with_args: object, +) -> InvocationTemplate: """Build a compiled InvocationTemplate the way the production initialization envelope does (see server/_recipe_execution.py:build_recipe_execution_snapshot), without driving the full server stack.""" @@ -56,6 +68,7 @@ def _template(**extra_with_args: object) -> InvocationTemplate: "step_name": _STEP_NAME, **extra_with_args, }, + optional_context_refs=optional_context_refs or [], ) invocation = bind_step_invocation(_STEP_NAME, step, manifest=_MANIFEST) assert invocation.is_valid, invocation.failures @@ -81,7 +94,12 @@ def _template(**extra_with_args: object) -> InvocationTemplate: ) -def _bind(template: InvocationTemplate, **overrides: BoundScalar): +def _bind( + template: InvocationTemplate, + *, + skill_inputs: Mapping[str, BoundScalar] | None = None, + **overrides: BoundScalar, +): """Bind against ``template``, matching production's shape: the caller always supplies actual values for every compiled (with:-declared) param — see ``_build_actual_mcp_kwargs`` in tools_execution.py, which @@ -100,11 +118,56 @@ def _bind(template: InvocationTemplate, **overrides: BoundScalar): execution_id=_EXECUTION_ID, step_name=_STEP_NAME, skill_command="/autoskillit:dry-walkthrough", - skill_inputs={"plan_path": "/tmp/plan.md"}, + skill_inputs=skill_inputs or {"plan_path": "/tmp/plan.md"}, actual_mcp_kwargs=actual_mcp_kwargs, ) +def _template_with_unresolved_default() -> InvocationTemplate: + return _template( + optional_context_refs=["audit_cycle_path"], + skill_inputs={ + "plan_path": "/tmp/plan.md", + "audit_cycle_path": "${{ context.audit_cycle_path }}", + }, + ) + + +@pytest.mark.parametrize( + "skill_inputs", + [ + {"plan_path": "/tmp/plan.md"}, + { + "plan_path": "/tmp/plan.md", + "audit_cycle_path": "", + "fabricated": "value", + }, + ], +) +def test_runtime_binding_rejects_missing_or_fabricated_skill_input_keys( + skill_inputs: Mapping[str, BoundScalar], +) -> None: + template = _template_with_unresolved_default() + + with pytest.raises(RuntimeBindingError) as excinfo: + _bind(template, skill_inputs=skill_inputs) + + assert excinfo.value.code == "recipe_execution_input_shape" + + +def test_runtime_binding_admits_explicit_advertised_default_without_auto_fill() -> None: + template = _template_with_unresolved_default() + default = template.invocation.skill_input("audit_cycle_path") + assert default is not None and default.unresolved_default == "" + + bound = _bind( + template, + skill_inputs={"plan_path": "/tmp/plan.md", "audit_cycle_path": ""}, + ) + + assert bound == (("plan_path", "/tmp/plan.md"), ("audit_cycle_path", "")) + + def test_undeclared_non_empty_value_is_denied() -> None: """(a) characterization — passes today; unaffected by #4402.""" template = _template() From 1aa49b8ba0d7467f614d83c5303b4d487084db27 Mon Sep 17 00:00:00 2001 From: Trecek Date: Fri, 14 Aug 2026 18:23:02 -0700 Subject: [PATCH 05/18] test: cover optional context delivery paths --- .../test_completion_receipt_fitness.py | 71 +++++++++++++ .../test_attestation_delivery_reachability.py | 100 ++++++++++++++++++ 2 files changed, 171 insertions(+) diff --git a/tests/contracts/test_completion_receipt_fitness.py b/tests/contracts/test_completion_receipt_fitness.py index 113b60559e..65bf5ec199 100644 --- a/tests/contracts/test_completion_receipt_fitness.py +++ b/tests/contracts/test_completion_receipt_fitness.py @@ -100,6 +100,77 @@ def test_implementation_plan_shape_supplies_unavailable_audit_context( assert dict(bound) == assembled +def test_implementation_fix_shape_supplies_unavailable_failure_context( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + from autoskillit.recipe import _api_cache + from autoskillit.recipe._api_cache import LoadCache + from autoskillit.recipe._binding import bind_runtime_skill_invocation + from autoskillit.server import _recipe_generation + + monkeypatch.setattr(_api_cache, "_LOAD_CACHE", LoadCache()) + monkeypatch.setattr(_recipe_generation, "_RECIPE_GENERATION_STORE", RecipeGenerationStore()) + payload, projection = _full_open_kitchen_generation("implementation") + tool_ctx = cast( + Any, + SimpleNamespace( + backend=None, + kitchen_id="implementation-fix-shape", + temp_dir=tmp_path, + ), + ) + prepared = prepare_recipe_delivery_generation( + payload, + recipe_name="implementation", + tool_ctx=tool_ctx, + finalized_projection=projection, + ) + credential = build_recipe_execution_credential(prepared.execution_snapshot) + shape = credential.as_wire_block()["skill_input_shapes"]["fix"] + assert shape == { + "keys": [ + "worktree_path", + "plan_path", + "base_branch", + "ci_conclusion", + "diagnosis_path", + ], + "unresolved_defaults": { + "ci_conclusion": "", + "diagnosis_path": "", + }, + } + + resolved = { + "worktree_path": "/tmp/worktree", + "plan_path": "/tmp/plan.md", + "base_branch": "develop", + } + defaults = cast(dict[str, str | int | bool], shape["unresolved_defaults"]) + assembled = { + name: resolved[name] if name in resolved else defaults[name] + for name in cast(list[str], shape["keys"]) + } + template = prepared.execution_snapshot.templates["fix"] + actual_mcp_kwargs = { + value.name: value.effective_value + for value in template.invocation.mcp_kwargs + if isinstance(value.effective_value, (str, int, bool)) + } + + bound = bind_runtime_skill_invocation( + template, + execution_id=prepared.execution_snapshot.execution_id, + step_name="fix", + skill_command="/autoskillit:resolve-failures", + skill_inputs=assembled, + actual_mcp_kwargs=actual_mcp_kwargs, + ) + + assert dict(bound) == assembled + + @pytest.mark.parametrize("recipe_name", _delivery_recipe_names(), ids=lambda n: n) def test_completion_receipt_fits_every_delivery_bound( recipe_name: str, diff --git a/tests/server/test_attestation_delivery_reachability.py b/tests/server/test_attestation_delivery_reachability.py index 9678445f82..6f666492db 100644 --- a/tests/server/test_attestation_delivery_reachability.py +++ b/tests/server/test_attestation_delivery_reachability.py @@ -12,6 +12,7 @@ import json from collections.abc import Mapping from dataclasses import replace +from types import SimpleNamespace from unittest.mock import Mock import pytest @@ -21,13 +22,28 @@ RECIPE_EXECUTION_CREDENTIAL_WIRE_FIELDS, RECIPE_EXECUTION_CREDENTIAL_WIRE_KEY, RecipeDeliveryMode, + build_recipe_execution_credential, ) +from autoskillit.execution import CODEX_RECIPE_DELIVERY_BUDGET from autoskillit.pipeline import ReadyRecipe +from autoskillit.server._recipe_delivery import ( + load_recipe_artifact, + persist_recipe_artifact, + prepare_recipe_delivery_generation, +) +from autoskillit.server._recipe_delivery_helpers import _attested_render from autoskillit.server._recipe_execution import ( RecipeExecutionAdmissionError, install_recipe_execution, ) +from autoskillit.server._recipe_generation import RecipeGenerationStore +from autoskillit.server._recipe_initialization import ( + _render_completion_receipt, + build_embedded_completion_response, + recipe_initialization_receipt, +) from tests.conftest import _make_result +from tests.contracts.test_delivery_bound_fitness import _full_open_kitchen_generation from tests.server._pipeline_test_helpers import _write_tracker from tests.server.test_tools_recipe_pull import ( _NOW, @@ -404,3 +420,87 @@ async def test_no_delivery_mode_omits_the_attestation_credential( pytest.fail(f"unhandled delivery mode: {unreachable!r}") assert set(block) == RECIPE_EXECUTION_CREDENTIAL_WIRE_FIELDS + + +async def test_delivery_modes_preserve_one_snapshot_skill_input_shapes( + monkeypatch: pytest.MonkeyPatch, + tmp_path, +) -> None: + from autoskillit.recipe import _api_cache + from autoskillit.recipe._api_cache import LoadCache + from autoskillit.server import _recipe_generation + + monkeypatch.setattr(_api_cache, "_LOAD_CACHE", LoadCache()) + monkeypatch.setattr(_recipe_generation, "_RECIPE_GENERATION_STORE", RecipeGenerationStore()) + payload, projection = _full_open_kitchen_generation("implementation") + tool_ctx = SimpleNamespace( + backend=None, + kitchen_id="same-snapshot-delivery", + temp_dir=tmp_path, + ) + prepared = prepare_recipe_delivery_generation( + payload, + recipe_name="implementation", + tool_ctx=tool_ctx, + finalized_projection=projection, + ) + generation = persist_recipe_artifact( + tmp_path, + kitchen_id=tool_ctx.kitchen_id, + producer_tool="open_kitchen", + recipe_name="implementation", + payload=prepared.canonical_artifact_payload, + flow_generation=prepared.flow_generation, + ) + credential = build_recipe_execution_credential(prepared.execution_snapshot) + + ordinary = prepared.canonical_artifact_payload[RECIPE_EXECUTION_CREDENTIAL_WIRE_KEY] + attested_text = _attested_render( + prepared.canonical_artifact_payload, + generation, + budget=CODEX_RECIPE_DELIVERY_BUDGET, + evidence_identity="sha256:" + "a" * 64, + ) + attested_control = json.loads(attested_text.split(RECIPE_BODY_START, 1)[0]) + attested = attested_control["recipe_delivery"]["payload_metadata"][ + RECIPE_EXECUTION_CREDENTIAL_WIRE_KEY + ] + spilled = load_recipe_artifact( + tmp_path, + kitchen_id=tool_ctx.kitchen_id, + identity=generation, + )[RECIPE_EXECUTION_CREDENTIAL_WIRE_KEY] + embedded_completion = build_embedded_completion_response( + initialization_id="same-snapshot", + recipe_name="implementation", + artifact_generation=generation, + flow_generation=prepared.flow_generation, + snapshot=prepared.execution_snapshot, + )[RECIPE_EXECUTION_CREDENTIAL_WIRE_KEY] + completion = json.loads( + _render_completion_receipt( + initialization_id="same-snapshot", + completion_receipt=recipe_initialization_receipt("same-snapshot", generation), + recipe_name="implementation", + artifact_generation=generation, + flow_generation=prepared.flow_generation, + credential=credential, + ) + )[RECIPE_EXECUTION_CREDENTIAL_WIRE_KEY] + + serialized_shapes = { + mode: json.dumps( + block["skill_input_shapes"], + ensure_ascii=False, + separators=(",", ":"), + sort_keys=True, + ).encode() + for mode, block in { + "ordinary_inline": ordinary, + "attested_inline": attested, + "spill_artifact": spilled, + "spill_embedded_completion": embedded_completion, + "completion": completion, + }.items() + } + assert len(set(serialized_shapes.values())) == 1, serialized_shapes From 710ae8ce92a3f9b8c51f50746dc8a304878c65cc Mon Sep 17 00:00:00 2001 From: Trecek Date: Fri, 14 Aug 2026 18:32:56 -0700 Subject: [PATCH 06/18] fix: preserve required recipe input provenance --- src/autoskillit/recipe/skill_contracts.yaml | 4 +- .../recipes/diagrams/implementation-groups.md | 2 +- .../recipes/diagrams/implementation.md | 2 +- src/autoskillit/recipes/diagrams/merge-prs.md | 4 +- .../recipes/diagrams/remediation.md | 2 +- src/autoskillit/recipes/diagrams/research.md | 2 +- src/autoskillit/recipes/merge-prs.json | 30 ++++++++--- src/autoskillit/recipes/merge-prs.yaml | 26 +++++++--- src/autoskillit/recipes/research-design.json | 30 +++++++++-- src/autoskillit/recipes/research-design.yaml | 21 ++++++-- src/autoskillit/recipes/research-review.json | 51 +++++-------------- src/autoskillit/recipes/research-review.yaml | 46 +++++------------ src/autoskillit/skills/sous-chef/SKILL.md | 2 +- tests/infra/test_pretty_output_recipe.py | 6 +-- tests/recipe/test_bundled_recipes_general.py | 8 ++- tests/recipe/test_merge_prs_queue_pmp.py | 4 +- 16 files changed, 138 insertions(+), 102 deletions(-) diff --git a/src/autoskillit/recipe/skill_contracts.yaml b/src/autoskillit/recipe/skill_contracts.yaml index b59ace353b..f7680dc0fe 100644 --- a/src/autoskillit/recipe/skill_contracts.yaml +++ b/src/autoskillit/recipe/skill_contracts.yaml @@ -2526,9 +2526,11 @@ skills: - name: all_diagram_paths type: string required: false + unresolved_default: "" - name: visualization_plan_path type: file_path required: false + unresolved_default: "" outputs: - name: html_path type: file_path @@ -3363,7 +3365,7 @@ callable_contracts: inputs: - name: plan_parts type: str - required: true + required: false - name: audit_cycle_path type: str required: false diff --git a/src/autoskillit/recipes/diagrams/implementation-groups.md b/src/autoskillit/recipes/diagrams/implementation-groups.md index 10344d5499..bb96ee5290 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 9141b0f73c..a108f650e3 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/merge-prs.md b/src/autoskillit/recipes/diagrams/merge-prs.md index 5342e4d0d5..a07757bace 100644 --- a/src/autoskillit/recipes/diagrams/merge-prs.md +++ b/src/autoskillit/recipes/diagrams/merge-prs.md @@ -1,4 +1,4 @@ - + ## merge-prs Merge multiple PRs into an integration branch with conflict resolution and CI gates. @@ -12,7 +12,7 @@ fetch_merge_queue_data → analyze_prs → route_by_queue_mode | +-- [queue mode]: | enqueue → wait → advance → next PR -| → resolve ejected conflicts on failure +| → validate conflict plan → resolve ejected conflicts or stop on failure | +-- [integration mode]: | create_batch_branch → publish → check_pr_merge_loop diff --git a/src/autoskillit/recipes/diagrams/remediation.md b/src/autoskillit/recipes/diagrams/remediation.md index 355a5acd5b..e7c6759610 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/diagrams/research.md b/src/autoskillit/recipes/diagrams/research.md index d35fd5a46d..a4fb52b103 100644 --- a/src/autoskillit/recipes/diagrams/research.md +++ b/src/autoskillit/recipes/diagrams/research.md @@ -1,4 +1,4 @@ - + ## research diff --git a/src/autoskillit/recipes/merge-prs.json b/src/autoskillit/recipes/merge-prs.json index 2d21fbec6c..18b5f3347c 100644 --- a/src/autoskillit/recipes/merge-prs.json +++ b/src/autoskillit/recipes/merge-prs.json @@ -442,16 +442,25 @@ "note": "Attempts a trivial rebase before invoking the full resolve-merge-conflicts skill. Uses ejected_pr_branch captured by get_ejected_pr_branch. On clean rebase, skips the skill and proceeds directly to push_ejected_fix. On conflict, aborts the rebase and routes to resolve_ejected_conflicts for goal-aware resolution.\n" }, "gate_ejected_conflict_plan": { - "action": "route", + "tool": "run_python", + "with": { + "callable": "autoskillit.recipe._cmd_rpc.verify_plan_artifacts", + "plan_parts": "${{ context.pr_plan_path }}", + "audit_cycle_path": "" + }, + "capture": { + "conflict_plan_path": "${{ result.plan_path }}" + }, "on_result": [ { - "when": "${{ context.pr_plan_path }} != ''", + "when": "${{ result.verdict }} == salvaged", "route": "resolve_ejected_conflicts" }, { "route": "register_clone_failure" } ], + "on_failure": "register_clone_failure", "optional_context_refs": [ "pr_plan_path" ] @@ -463,7 +472,7 @@ "skill_command": "/autoskillit:resolve-merge-conflicts", "skill_inputs": { "worktree_path": "${{ context.ejected_pr_branch }}", - "plan_path": "${{ context.pr_plan_path }}", + "plan_path": "${{ context.conflict_plan_path }}", "base_branch": "${{ inputs.base_branch }}" }, "cwd": "${{ context.work_dir }}", @@ -765,16 +774,25 @@ "note": "Attempts a proactive rebase of the next PR branch onto the current base_branch HEAD before submitting to the merge queue. On clean rebase, skips conflict resolution entirely and proceeds to push. On conflict, routes to resolve_proactive_rebase_conflicts. On git command failure, falls through to get_current_pr_branch to enforce the CI gate.\n" }, "gate_proactive_rebase_conflict_plan": { - "action": "route", + "tool": "run_python", + "with": { + "callable": "autoskillit.recipe._cmd_rpc.verify_plan_artifacts", + "plan_parts": "${{ context.pr_plan_path }}", + "audit_cycle_path": "" + }, + "capture": { + "conflict_plan_path": "${{ result.plan_path }}" + }, "on_result": [ { - "when": "${{ context.pr_plan_path }} != ''", + "when": "${{ result.verdict }} == salvaged", "route": "resolve_proactive_rebase_conflicts" }, { "route": "register_clone_failure" } ], + "on_failure": "register_clone_failure", "optional_context_refs": [ "pr_plan_path" ] @@ -786,7 +804,7 @@ "skill_command": "/autoskillit:resolve-merge-conflicts", "skill_inputs": { "worktree_path": "${{ context.next_pr_branch }}", - "plan_path": "${{ context.pr_plan_path }}", + "plan_path": "${{ context.conflict_plan_path }}", "base_branch": "${{ inputs.base_branch }}" }, "cwd": "${{ context.work_dir }}", diff --git a/src/autoskillit/recipes/merge-prs.yaml b/src/autoskillit/recipes/merge-prs.yaml index 6bcc09ecfe..965efcba38 100644 --- a/src/autoskillit/recipes/merge-prs.yaml +++ b/src/autoskillit/recipes/merge-prs.yaml @@ -426,11 +426,18 @@ steps: ' gate_ejected_conflict_plan: - action: route + tool: run_python + with: + callable: autoskillit.recipe._cmd_rpc.verify_plan_artifacts + plan_parts: ${{ context.pr_plan_path }} + audit_cycle_path: '' + capture: + conflict_plan_path: ${{ result.plan_path }} on_result: - - when: ${{ context.pr_plan_path }} != '' + - when: ${{ result.verdict }} == salvaged route: resolve_ejected_conflicts - route: register_clone_failure + on_failure: register_clone_failure optional_context_refs: - pr_plan_path resolve_ejected_conflicts: @@ -440,7 +447,7 @@ steps: skill_command: /autoskillit:resolve-merge-conflicts skill_inputs: worktree_path: ${{ context.ejected_pr_branch }} - plan_path: ${{ context.pr_plan_path }} + plan_path: ${{ context.conflict_plan_path }} base_branch: ${{ inputs.base_branch }} cwd: ${{ context.work_dir }} output_dir: . @@ -672,11 +679,18 @@ steps: ' gate_proactive_rebase_conflict_plan: - action: route + tool: run_python + with: + callable: autoskillit.recipe._cmd_rpc.verify_plan_artifacts + plan_parts: ${{ context.pr_plan_path }} + audit_cycle_path: '' + capture: + conflict_plan_path: ${{ result.plan_path }} on_result: - - when: ${{ context.pr_plan_path }} != '' + - when: ${{ result.verdict }} == salvaged route: resolve_proactive_rebase_conflicts - route: register_clone_failure + on_failure: register_clone_failure optional_context_refs: - pr_plan_path resolve_proactive_rebase_conflicts: @@ -686,7 +700,7 @@ steps: skill_command: /autoskillit:resolve-merge-conflicts skill_inputs: worktree_path: ${{ context.next_pr_branch }} - plan_path: ${{ context.pr_plan_path }} + plan_path: ${{ context.conflict_plan_path }} base_branch: ${{ inputs.base_branch }} cwd: ${{ context.work_dir }} output_dir: . diff --git a/src/autoskillit/recipes/research-design.json b/src/autoskillit/recipes/research-design.json index 0b5155f832..d6e9a4ad11 100644 --- a/src/autoskillit/recipes/research-design.json +++ b/src/autoskillit/recipes/research-design.json @@ -199,7 +199,7 @@ }, { "when": "${{ result.verdict }} == STOP", - "route": "resolve_design_review" + "route": "gate_design_review_inputs" }, { "route": "vis_dial" @@ -302,14 +302,12 @@ "tool": "run_skill", "model": "", "with": { - "skill_command": "/autoskillit:resolve-design-review ${{ context.evaluation_dashboard }} ${{ context.experiment_plan }} ${{ context.revision_guidance }}", + "skill_command": "/autoskillit:resolve-design-review ${{ context.required_evaluation_dashboard }} ${{ context.experiment_plan }} ${{ context.revision_guidance }}", "cwd": "${{ inputs.source_dir }}", "step_name": "resolve_design_review", "output_dir": "{{AUTOSKILLIT_TEMP}}/resolve-design-review/iter_${{ context.design_review_count }}" }, "optional_context_refs": [ - "evaluation_dashboard", - "experiment_plan", "revision_guidance", "findings_manifest_path" ], @@ -334,6 +332,30 @@ ], "note": "Triages STOP-verdict findings from review_design. ADDRESSABLE/DISCUSS findings generate revision_guidance and loop back. Only all-STRUCTURAL findings halt at design_rejected.\n" }, + "gate_design_review_inputs": { + "tool": "run_python", + "with": { + "callable": "autoskillit.recipe._cmd_rpc.verify_plan_artifacts", + "plan_parts": "${{ context.evaluation_dashboard }}", + "audit_cycle_path": "" + }, + "capture": { + "required_evaluation_dashboard": "${{ result.plan_path }}" + }, + "on_result": [ + { + "when": "${{ result.verdict }} == salvaged", + "route": "resolve_design_review" + }, + { + "route": "design_rejected" + } + ], + "on_failure": "design_rejected", + "optional_context_refs": [ + "evaluation_dashboard" + ] + }, "design_rejected": { "action": "stop", "message": "Research pipeline halted: experiment design rejected (verdict=STOP, resolution=failed). All stop-trigger findings are structural — revision cannot address the identified flaws. Check {{AUTOSKILLIT_TEMP}}/resolve-design-review/ for the triage report and {{AUTOSKILLIT_TEMP}}/apply-review-dimensions/ for the evaluation dashboard. Emit the L3 result sentinel JSON block now with success=false. Example sentinel: {\"success\": false, \"reason\": \"experiment design rejected\"}\n" diff --git a/src/autoskillit/recipes/research-design.yaml b/src/autoskillit/recipes/research-design.yaml index 29fc659c06..5ced026629 100644 --- a/src/autoskillit/recipes/research-design.yaml +++ b/src/autoskillit/recipes/research-design.yaml @@ -177,7 +177,7 @@ steps: - when: "${{ result.verdict }} == REVISE" route: revise_design - when: "${{ result.verdict }} == STOP" - route: resolve_design_review + route: gate_design_review_inputs - route: vis_dial vis_dial: @@ -260,11 +260,11 @@ steps: tool: run_skill model: "" with: - skill_command: "/autoskillit:resolve-design-review ${{ context.evaluation_dashboard }} ${{ context.experiment_plan }} ${{ context.revision_guidance }}" + skill_command: "/autoskillit:resolve-design-review ${{ context.required_evaluation_dashboard }} ${{ context.experiment_plan }} ${{ context.revision_guidance }}" cwd: "${{ inputs.source_dir }}" step_name: resolve_design_review output_dir: "{{AUTOSKILLIT_TEMP}}/resolve-design-review/iter_${{ context.design_review_count }}" - optional_context_refs: [evaluation_dashboard, experiment_plan, revision_guidance, findings_manifest_path] + optional_context_refs: [revision_guidance, findings_manifest_path] capture: revision_guidance: "${{ result.revision_guidance }}" retries: 1 @@ -281,6 +281,21 @@ steps: generate revision_guidance and loop back. Only all-STRUCTURAL findings halt at design_rejected. + gate_design_review_inputs: + tool: run_python + with: + callable: autoskillit.recipe._cmd_rpc.verify_plan_artifacts + plan_parts: ${{ context.evaluation_dashboard }} + audit_cycle_path: "" + capture: + required_evaluation_dashboard: ${{ result.plan_path }} + on_result: + - when: "${{ result.verdict }} == salvaged" + route: resolve_design_review + - route: design_rejected + on_failure: design_rejected + optional_context_refs: [evaluation_dashboard] + design_rejected: action: stop message: > diff --git a/src/autoskillit/recipes/research-review.json b/src/autoskillit/recipes/research-review.json index 275c6c9455..d19b3eb131 100644 --- a/src/autoskillit/recipes/research-review.json +++ b/src/autoskillit/recipes/research-review.json @@ -93,18 +93,14 @@ "tool": "run_skill", "model": "", "optional_context_refs": [ - "worktree_path", - "research_dir", - "report_path", - "experiment_plan", "experiment_results", "experiment_type", "scope_report", "visualization_plan_path" ], "with": { - "skill_command": "/autoskillit:prepare-research-pr ${{ context.report_path }} ${{ context.experiment_plan }} ${{ context.worktree_path }} ${{ inputs.base_branch }}", - "cwd": "${{ context.worktree_path }}", + "skill_command": "/autoskillit:prepare-research-pr ${{ inputs.report_path }} ${{ inputs.experiment_plan }} ${{ inputs.worktree_path }} ${{ inputs.base_branch }}", + "cwd": "${{ inputs.worktree_path }}", "step_name": "prepare_research_pr", "output_dir": "." }, @@ -119,13 +115,9 @@ "run_experiment_lenses": { "tool": "run_skill", "model": "", - "optional_context_refs": [ - "worktree_path", - "experiment_plan" - ], "with": { - "skill_command": "/autoskillit:exp-lens-{slug} {context_path} ${{ context.experiment_plan }}", - "cwd": "${{ context.worktree_path }}", + "skill_command": "/autoskillit:exp-lens-{slug} {context_path} ${{ inputs.experiment_plan }}", + "cwd": "${{ inputs.worktree_path }}", "step_name": "run_experiment_lenses" }, "capture_list": { @@ -137,13 +129,9 @@ }, "stage_bundle": { "tool": "run_cmd", - "optional_context_refs": [ - "research_dir", - "worktree_path" - ], "with": { - "cmd": "bash {{AUTOSKILLIT_SCRIPTS}}/stage_bundle.sh '${{ context.research_dir }}' '${{ context.worktree_path }}' '{{AUTOSKILLIT_TEMP}}'", - "cwd": "${{ context.worktree_path }}", + "cmd": "bash {{AUTOSKILLIT_SCRIPTS}}/stage_bundle.sh '${{ inputs.research_dir }}' '${{ inputs.worktree_path }}' '{{AUTOSKILLIT_TEMP}}'", + "cwd": "${{ inputs.worktree_path }}", "step_name": "stage_bundle" }, "on_success": "route_pr_or_local", @@ -166,12 +154,9 @@ "compose_research_pr": { "tool": "run_skill", "model": "", - "optional_context_refs": [ - "worktree_path" - ], "with": { - "skill_command": "/autoskillit:compose-research-pr ${{ context.prep_path }} \"${{ context.all_diagram_paths }}\" ${{ context.worktree_path }} ${{ inputs.base_branch }}", - "cwd": "${{ context.worktree_path }}", + "skill_command": "/autoskillit:compose-research-pr ${{ context.prep_path }} \"${{ context.all_diagram_paths }}\" ${{ inputs.worktree_path }} ${{ inputs.base_branch }}", + "cwd": "${{ inputs.worktree_path }}", "step_name": "compose_research_pr", "output_dir": "." }, @@ -427,12 +412,9 @@ "resolve_research_review": { "tool": "run_skill", "model": "", - "optional_context_refs": [ - "worktree_path" - ], "with": { - "skill_command": "/autoskillit:resolve-research-review ${{ context.worktree_path }} ${{ inputs.base_branch }}", - "cwd": "${{ context.worktree_path }}", + "skill_command": "/autoskillit:resolve-research-review ${{ inputs.worktree_path }} ${{ inputs.base_branch }}", + "cwd": "${{ inputs.worktree_path }}", "step_name": "resolve_research_review", "output_dir": "." }, @@ -461,12 +443,9 @@ "resolve_claims_review": { "tool": "run_skill", "model": "", - "optional_context_refs": [ - "worktree_path" - ], "with": { - "skill_command": "/autoskillit:resolve-claims-review ${{ context.worktree_path }} ${{ inputs.base_branch }}", - "cwd": "${{ context.worktree_path }}", + "skill_command": "/autoskillit:resolve-claims-review ${{ inputs.worktree_path }} ${{ inputs.base_branch }}", + "cwd": "${{ inputs.worktree_path }}", "step_name": "resolve_claims_review", "output_dir": "." }, @@ -619,14 +598,12 @@ "tool": "run_skill", "model": "", "optional_context_refs": [ - "research_dir", - "worktree_path", "all_diagram_paths", "visualization_plan_path" ], "with": { - "skill_command": "/autoskillit:bundle-local-report ${{ context.research_dir }} ${{ context.report_path_after_finalize }} ${{ context.all_diagram_paths }} ${{ context.visualization_plan_path }}", - "cwd": "${{ context.worktree_path }}", + "skill_command": "/autoskillit:bundle-local-report ${{ inputs.research_dir }} ${{ context.report_path_after_finalize }} ${{ context.all_diagram_paths }} ${{ context.visualization_plan_path }}", + "cwd": "${{ inputs.worktree_path }}", "step_name": "finalize_bundle_render", "output_dir": "." }, diff --git a/src/autoskillit/recipes/research-review.yaml b/src/autoskillit/recipes/research-review.yaml index ab2c1252e7..96f362f9ee 100644 --- a/src/autoskillit/recipes/research-review.yaml +++ b/src/autoskillit/recipes/research-review.yaml @@ -96,18 +96,14 @@ steps: tool: run_skill model: '' optional_context_refs: - - worktree_path - - research_dir - - report_path - - experiment_plan - experiment_results - experiment_type - scope_report - visualization_plan_path with: - skill_command: /autoskillit:prepare-research-pr ${{ context.report_path }} ${{ context.experiment_plan }} ${{ context.worktree_path + skill_command: /autoskillit:prepare-research-pr ${{ inputs.report_path }} ${{ inputs.experiment_plan }} ${{ inputs.worktree_path }} ${{ inputs.base_branch }} - cwd: ${{ context.worktree_path }} + cwd: ${{ inputs.worktree_path }} step_name: prepare_research_pr output_dir: . capture: @@ -119,12 +115,9 @@ steps: run_experiment_lenses: tool: run_skill model: '' - optional_context_refs: - - worktree_path - - experiment_plan with: - skill_command: /autoskillit:exp-lens-{slug} {context_path} ${{ context.experiment_plan }} - cwd: ${{ context.worktree_path }} + skill_command: /autoskillit:exp-lens-{slug} {context_path} ${{ inputs.experiment_plan }} + cwd: ${{ inputs.worktree_path }} step_name: run_experiment_lenses capture_list: all_diagram_paths: ${{ result.diagram_path }} @@ -133,12 +126,9 @@ steps: on_failure: stage_bundle stage_bundle: tool: run_cmd - optional_context_refs: - - research_dir - - worktree_path with: - cmd: bash {{AUTOSKILLIT_SCRIPTS}}/stage_bundle.sh '${{ context.research_dir }}' '${{ context.worktree_path }}' '{{AUTOSKILLIT_TEMP}}' - cwd: ${{ context.worktree_path }} + cmd: bash {{AUTOSKILLIT_SCRIPTS}}/stage_bundle.sh '${{ inputs.research_dir }}' '${{ inputs.worktree_path }}' '{{AUTOSKILLIT_TEMP}}' + cwd: ${{ inputs.worktree_path }} step_name: stage_bundle on_success: route_pr_or_local on_failure: route_pr_or_local @@ -165,12 +155,10 @@ steps: compose_research_pr: tool: run_skill model: '' - optional_context_refs: - - worktree_path with: - skill_command: /autoskillit:compose-research-pr ${{ context.prep_path }} "${{ context.all_diagram_paths }}" ${{ context.worktree_path + skill_command: /autoskillit:compose-research-pr ${{ context.prep_path }} "${{ context.all_diagram_paths }}" ${{ inputs.worktree_path }} ${{ inputs.base_branch }} - cwd: ${{ context.worktree_path }} + cwd: ${{ inputs.worktree_path }} step_name: compose_research_pr output_dir: . capture: @@ -386,11 +374,9 @@ steps: resolve_research_review: tool: run_skill model: '' - optional_context_refs: - - worktree_path with: - skill_command: /autoskillit:resolve-research-review ${{ context.worktree_path }} ${{ inputs.base_branch }} - cwd: ${{ context.worktree_path }} + skill_command: /autoskillit:resolve-research-review ${{ inputs.worktree_path }} ${{ inputs.base_branch }} + cwd: ${{ inputs.worktree_path }} step_name: resolve_research_review output_dir: . capture: @@ -415,11 +401,9 @@ steps: resolve_claims_review: tool: run_skill model: '' - optional_context_refs: - - worktree_path with: - skill_command: /autoskillit:resolve-claims-review ${{ context.worktree_path }} ${{ inputs.base_branch }} - cwd: ${{ context.worktree_path }} + skill_command: /autoskillit:resolve-claims-review ${{ inputs.worktree_path }} ${{ inputs.base_branch }} + cwd: ${{ inputs.worktree_path }} step_name: resolve_claims_review output_dir: . capture: @@ -562,14 +546,12 @@ steps: tool: run_skill model: '' optional_context_refs: - - research_dir - - worktree_path - all_diagram_paths - visualization_plan_path with: - skill_command: /autoskillit:bundle-local-report ${{ context.research_dir }} ${{ context.report_path_after_finalize }} + skill_command: /autoskillit:bundle-local-report ${{ inputs.research_dir }} ${{ context.report_path_after_finalize }} ${{ context.all_diagram_paths }} ${{ context.visualization_plan_path }} - cwd: ${{ context.worktree_path }} + cwd: ${{ inputs.worktree_path }} step_name: finalize_bundle_render output_dir: . capture: diff --git a/src/autoskillit/skills/sous-chef/SKILL.md b/src/autoskillit/skills/sous-chef/SKILL.md index 59395adfbc..86202f36e3 100644 --- a/src/autoskillit/skills/sous-chef/SKILL.md +++ b/src/autoskillit/skills/sous-chef/SKILL.md @@ -1079,7 +1079,7 @@ Segmented delivery narrows this lookup to the latest delivered carrier. Do not a startup credential contains invocation digests for the full recipe horizon. For structured child inputs, select `recipe_execution.skill_input_shapes[step_name]` and -initialize `skill_inputs` with exactly its ordered keys. Replace available values in place; +initialize `skill_inputs` with exactly its ordered keys; replace available values in place; for unavailable context, copy a value only from that key's advertised `unresolved_defaults` entry. Test key presence rather than truthiness so `""`, `0`, and `False` are forwarded verbatim. Never delete or invent a key. diff --git a/tests/infra/test_pretty_output_recipe.py b/tests/infra/test_pretty_output_recipe.py index 7a7e6fbb8e..42351e20d9 100644 --- a/tests/infra/test_pretty_output_recipe.py +++ b/tests/infra/test_pretty_output_recipe.py @@ -1164,9 +1164,9 @@ def test_canonical_recipe_responses_fit_independent_registry_ceilings(tmp_path, for ingredients_only in (False, True) } assert maxima == { - "get_recipe_section": (179_549, "remediation", "all_truthy"), - "load_recipe": (179_549, "remediation", "all_truthy"), - "open_kitchen": (179_602, "remediation", "all_truthy"), + "get_recipe_section": (152_582, "implementation-groups", "all_truthy"), + "load_recipe": (152_582, "implementation-groups", "all_truthy"), + "open_kitchen": (152_635, "implementation-groups", "all_truthy"), } diff --git a/tests/recipe/test_bundled_recipes_general.py b/tests/recipe/test_bundled_recipes_general.py index 34fd62a9c3..7a785deca0 100644 --- a/tests/recipe/test_bundled_recipes_general.py +++ b/tests/recipe/test_bundled_recipes_general.py @@ -50,6 +50,8 @@ def test_optional_context_structured_skill_input_inventory_is_explicit() -> None expected_pairs = { ("audit-impl", "deviation_manifest_path"), ("audit-impl", "prior_audit_cycle_path"), + ("bundle-local-report", "all_diagram_paths"), + ("bundle-local-report", "visualization_plan_path"), ("diagnose-ci", "event"), ("dry-walkthrough", "audit_cycle_path"), ("dry-walkthrough", "plan_disposition_path"), @@ -94,7 +96,7 @@ def test_optional_context_structured_skill_input_inventory_is_explicit() -> None occurrences.append(occurrence) actual_pairs = {(skill, input_name) for _, _, skill, input_name in occurrences} - assert len(occurrences) == 49 + assert len(occurrences) == 54 assert actual_pairs == expected_pairs assert not required_occurrences for skill_name, input_name in expected_pairs: @@ -110,7 +112,11 @@ def test_required_optional_context_routes_are_gated_or_guaranteed() -> None: gate = merge_prs.steps[f"gate_{prefix}_conflict_plan"] routes = [condition.route for condition in gate.on_result.conditions] assert routes[-1] == "register_clone_failure" + assert gate.capture == {"conflict_plan_path": "${{ result.plan_path }}"} resolve = merge_prs.steps[f"resolve_{prefix}_conflicts"] + assert resolve.with_args["skill_inputs"]["plan_path"] == ( + "${{ context.conflict_plan_path }}" + ) assert "pr_plan_path" not in resolve.optional_context_refs research = load_recipe(builtin_recipes_dir() / "research.yaml") diff --git a/tests/recipe/test_merge_prs_queue_pmp.py b/tests/recipe/test_merge_prs_queue_pmp.py index 90e8d7840d..33cc1efcbf 100644 --- a/tests/recipe/test_merge_prs_queue_pmp.py +++ b/tests/recipe/test_merge_prs_queue_pmp.py @@ -169,7 +169,7 @@ def test_merge_prs_attempt_cheap_rebase_routing(pmp_recipe) -> None: assert clean_routes[0].route == "push_ejected_fix" fallback = [c for c in conditions if c.when is None] assert fallback, "must have a fallback condition" - assert fallback[0].route == "resolve_ejected_conflicts" + assert fallback[0].route == "gate_ejected_conflict_plan" def test_merge_prs_get_ejected_routes_to_cheap_rebase(pmp_recipe) -> None: @@ -246,7 +246,7 @@ def test_merge_prs_proactive_rebase_next_pr_routing(pmp_recipe) -> None: (c.route for c in step.on_result.conditions if c.when is None), None, ) - assert fallback_route == "resolve_proactive_rebase_conflicts" + assert fallback_route == "gate_proactive_rebase_conflict_plan" def test_merge_prs_proactive_rebase_next_pr_on_failure_routes_through_ci_gate(pmp_recipe) -> None: From 6445554d26f5bd530ed445c23628cf9417fb4edd Mon Sep 17 00:00:00 2001 From: Trecek Date: Fri, 14 Aug 2026 18:42:42 -0700 Subject: [PATCH 07/18] fix: route design review through captured inputs --- src/autoskillit/recipes/research-design.json | 48 +++++++++---------- src/autoskillit/recipes/research-design.yaml | 30 ++++++------ src/autoskillit/skills/sous-chef/SKILL.md | 2 +- tests/recipe/test_bundled_recipes_general.py | 2 +- .../test_bundled_recipes_research_design.py | 5 +- tests/recipe/test_research_review_recipe.py | 6 +-- 6 files changed, 47 insertions(+), 46 deletions(-) diff --git a/src/autoskillit/recipes/research-design.json b/src/autoskillit/recipes/research-design.json index d6e9a4ad11..f875381c3c 100644 --- a/src/autoskillit/recipes/research-design.json +++ b/src/autoskillit/recipes/research-design.json @@ -298,6 +298,30 @@ ], "note": "Iteration guard for the apply → synthesize REVISE → revise_design → check_design_review_loop → apply cycle. Re-entry skips re-dialing since experiment_type is stable across revision cycles. Caps design revision loops at 3 iterations. On exhaustion, proceeds to vis_dial with the best available design.\n" }, + "gate_design_review_inputs": { + "tool": "run_python", + "with": { + "callable": "autoskillit.recipe._cmd_rpc.verify_plan_artifacts", + "plan_parts": "${{ context.evaluation_dashboard }}", + "audit_cycle_path": "" + }, + "capture": { + "required_evaluation_dashboard": "${{ result.plan_path }}" + }, + "on_result": [ + { + "when": "${{ result.verdict }} == salvaged", + "route": "resolve_design_review" + }, + { + "route": "design_rejected" + } + ], + "on_failure": "design_rejected", + "optional_context_refs": [ + "evaluation_dashboard" + ] + }, "resolve_design_review": { "tool": "run_skill", "model": "", @@ -332,30 +356,6 @@ ], "note": "Triages STOP-verdict findings from review_design. ADDRESSABLE/DISCUSS findings generate revision_guidance and loop back. Only all-STRUCTURAL findings halt at design_rejected.\n" }, - "gate_design_review_inputs": { - "tool": "run_python", - "with": { - "callable": "autoskillit.recipe._cmd_rpc.verify_plan_artifacts", - "plan_parts": "${{ context.evaluation_dashboard }}", - "audit_cycle_path": "" - }, - "capture": { - "required_evaluation_dashboard": "${{ result.plan_path }}" - }, - "on_result": [ - { - "when": "${{ result.verdict }} == salvaged", - "route": "resolve_design_review" - }, - { - "route": "design_rejected" - } - ], - "on_failure": "design_rejected", - "optional_context_refs": [ - "evaluation_dashboard" - ] - }, "design_rejected": { "action": "stop", "message": "Research pipeline halted: experiment design rejected (verdict=STOP, resolution=failed). All stop-trigger findings are structural — revision cannot address the identified flaws. Check {{AUTOSKILLIT_TEMP}}/resolve-design-review/ for the triage report and {{AUTOSKILLIT_TEMP}}/apply-review-dimensions/ for the evaluation dashboard. Emit the L3 result sentinel JSON block now with success=false. Example sentinel: {\"success\": false, \"reason\": \"experiment design rejected\"}\n" diff --git a/src/autoskillit/recipes/research-design.yaml b/src/autoskillit/recipes/research-design.yaml index 5ced026629..087b372ab3 100644 --- a/src/autoskillit/recipes/research-design.yaml +++ b/src/autoskillit/recipes/research-design.yaml @@ -256,6 +256,21 @@ steps: Caps design revision loops at 3 iterations. On exhaustion, proceeds to vis_dial with the best available design. + gate_design_review_inputs: + tool: run_python + with: + callable: autoskillit.recipe._cmd_rpc.verify_plan_artifacts + plan_parts: ${{ context.evaluation_dashboard }} + audit_cycle_path: "" + capture: + required_evaluation_dashboard: ${{ result.plan_path }} + on_result: + - when: "${{ result.verdict }} == salvaged" + route: resolve_design_review + - route: design_rejected + on_failure: design_rejected + optional_context_refs: [evaluation_dashboard] + resolve_design_review: tool: run_skill model: "" @@ -281,21 +296,6 @@ steps: generate revision_guidance and loop back. Only all-STRUCTURAL findings halt at design_rejected. - gate_design_review_inputs: - tool: run_python - with: - callable: autoskillit.recipe._cmd_rpc.verify_plan_artifacts - plan_parts: ${{ context.evaluation_dashboard }} - audit_cycle_path: "" - capture: - required_evaluation_dashboard: ${{ result.plan_path }} - on_result: - - when: "${{ result.verdict }} == salvaged" - route: resolve_design_review - - route: design_rejected - on_failure: design_rejected - optional_context_refs: [evaluation_dashboard] - design_rejected: action: stop message: > diff --git a/src/autoskillit/skills/sous-chef/SKILL.md b/src/autoskillit/skills/sous-chef/SKILL.md index 86202f36e3..b6067ba552 100644 --- a/src/autoskillit/skills/sous-chef/SKILL.md +++ b/src/autoskillit/skills/sous-chef/SKILL.md @@ -1082,7 +1082,7 @@ For structured child inputs, select `recipe_execution.skill_input_shapes[step_na initialize `skill_inputs` with exactly its ordered keys; replace available values in place; for unavailable context, copy a value only from that key's advertised `unresolved_defaults` entry. Test key presence rather than truthiness so `""`, `0`, and -`False` are forwarded verbatim. Never delete or invent a key. +`False` are forwarded verbatim; never delete or invent a key. Similarly, NEVER read SKILL.md files directly from the filesystem. Use the Skill tool to load skill instructions — it applies runtime transformations (namespace rewriting, diff --git a/tests/recipe/test_bundled_recipes_general.py b/tests/recipe/test_bundled_recipes_general.py index 7a785deca0..e38472eb6b 100644 --- a/tests/recipe/test_bundled_recipes_general.py +++ b/tests/recipe/test_bundled_recipes_general.py @@ -112,7 +112,7 @@ def test_required_optional_context_routes_are_gated_or_guaranteed() -> None: gate = merge_prs.steps[f"gate_{prefix}_conflict_plan"] routes = [condition.route for condition in gate.on_result.conditions] assert routes[-1] == "register_clone_failure" - assert gate.capture == {"conflict_plan_path": "${{ result.plan_path }}"} + assert gate.capture["conflict_plan_path"].from_ == "${{ result.plan_path }}" resolve = merge_prs.steps[f"resolve_{prefix}_conflicts"] assert resolve.with_args["skill_inputs"]["plan_path"] == ( "${{ context.conflict_plan_path }}" diff --git a/tests/recipe/test_bundled_recipes_research_design.py b/tests/recipe/test_bundled_recipes_research_design.py index 372ccc6918..bf32f96b9c 100644 --- a/tests/recipe/test_bundled_recipes_research_design.py +++ b/tests/recipe/test_bundled_recipes_research_design.py @@ -58,7 +58,7 @@ def test_issue_url_ingredient_optional(self, recipe) -> None: assert recipe.ingredients["issue_url"].required is False def test_step_count(self, recipe) -> None: - assert len(recipe.steps) == 17 + assert len(recipe.steps) == 18 def test_step_names(self, recipe) -> None: expected = { @@ -75,6 +75,7 @@ def test_step_names(self, recipe) -> None: "create_worktree", "revise_design", "check_design_review_loop", + "gate_design_review_inputs", "resolve_design_review", "design_rejected", "design_complete", @@ -195,7 +196,7 @@ def test_synthesize_on_result_stop(self, recipe) -> None: (c for c in step.on_result.conditions if c.when and "STOP" in c.when), None ) assert stop_cond is not None, "Missing STOP route" - assert stop_cond.route == "resolve_design_review" + assert stop_cond.route == "gate_design_review_inputs" def test_synthesize_on_result_fallback(self, recipe) -> None: step = recipe.steps["synthesize"] diff --git a/tests/recipe/test_research_review_recipe.py b/tests/recipe/test_research_review_recipe.py index e242495027..9eef3979a3 100644 --- a/tests/recipe/test_research_review_recipe.py +++ b/tests/recipe/test_research_review_recipe.py @@ -117,10 +117,10 @@ def test_terminal_stops_present(self, recipe) -> None: assert recipe.steps["escalate_stop"].action == "stop" # --- Key routing adaptations --- - def test_prepare_research_pr_uses_context_worktree_path(self, recipe) -> None: + def test_prepare_research_pr_uses_required_worktree_ingredient(self, recipe) -> None: step = recipe.steps["prepare_research_pr"] - assert "context.worktree_path" in step.with_args.get("skill_command", "") - assert step.with_args.get("cwd") == "${{ context.worktree_path }}" + assert "inputs.worktree_path" in step.with_args.get("skill_command", "") + assert step.with_args.get("cwd") == "${{ inputs.worktree_path }}" def test_finalize_bundle_routes_to_finalize_bundle_render(self, recipe) -> None: step = recipe.steps["finalize_bundle"] From f5353c8ced8579ed6e55e69d3b645084c419a430 Mon Sep 17 00:00:00 2001 From: Trecek Date: Fri, 14 Aug 2026 20:45:53 -0700 Subject: [PATCH 08/18] fix(review): type and isolate skill input shapes --- .../core/types/_type_recipe_execution.py | 17 +++++++++++++---- tests/core/test_recipe_execution_credential.py | 11 +++++++++-- 2 files changed, 22 insertions(+), 6 deletions(-) diff --git a/src/autoskillit/core/types/_type_recipe_execution.py b/src/autoskillit/core/types/_type_recipe_execution.py index 5bb00f1897..a4f4d71ffa 100644 --- a/src/autoskillit/core/types/_type_recipe_execution.py +++ b/src/autoskillit/core/types/_type_recipe_execution.py @@ -14,7 +14,7 @@ from enum import StrEnum from pathlib import Path from types import MappingProxyType -from typing import Any, Protocol, runtime_checkable +from typing import Any, Protocol, TypedDict, runtime_checkable from ..closure_hashing import HASH_RE, compute_canonical_hash from ._type_audit_admission import InstallationVersion @@ -210,6 +210,11 @@ def template_digests(self) -> Mapping[str, str]: ) +class _SkillInputShape(TypedDict): + keys: list[str] + unresolved_defaults: dict[str, BoundScalar] + + @dataclass(frozen=True, slots=True) class RecipeExecutionCredential: """The caller-visible identity of one installed recipe execution.""" @@ -217,14 +222,18 @@ class RecipeExecutionCredential: execution_id: str snapshot_digest: str invocation_template_digests: Mapping[str, str] - skill_input_shapes: Mapping[str, Mapping[str, object]] + skill_input_shapes: Mapping[str, _SkillInputShape] def as_wire_block(self) -> dict[str, Any]: return { "execution_id": self.execution_id, "invocation_template_digests": dict(self.invocation_template_digests), "skill_input_shapes": { - step_name: dict(shape) for step_name, shape in self.skill_input_shapes.items() + step_name: { + "keys": list(shape["keys"]), + "unresolved_defaults": dict(shape["unresolved_defaults"]), + } + for step_name, shape in self.skill_input_shapes.items() }, "snapshot_digest": self.snapshot_digest, } @@ -239,7 +248,7 @@ def build_recipe_execution_credential( snapshot: RecipeExecutionSnapshot, ) -> RecipeExecutionCredential: """Project the sole caller-visible credential for an execution snapshot.""" - skill_input_shapes: dict[str, Mapping[str, object]] = {} + skill_input_shapes: dict[str, _SkillInputShape] = {} for step_name, template in snapshot.templates.items(): present = tuple(value for value in template.invocation.skill_inputs if value.is_present) skill_input_shapes[step_name] = { diff --git a/tests/core/test_recipe_execution_credential.py b/tests/core/test_recipe_execution_credential.py index 012f1b245e..9f90596f60 100644 --- a/tests/core/test_recipe_execution_credential.py +++ b/tests/core/test_recipe_execution_credential.py @@ -102,11 +102,18 @@ def test_recipe_execution_credential_projects_ordered_keys_and_falsey_defaults() ), ) - wire = build_recipe_execution_credential(snapshot).as_wire_block() + credential = build_recipe_execution_credential(snapshot) + wire = credential.as_wire_block() - assert wire["skill_input_shapes"] == { + expected_shapes = { "invoke": { "keys": ["empty", "zero", "disabled", "resolved"], "unresolved_defaults": {"empty": "", "zero": 0, "disabled": False}, } } + assert wire["skill_input_shapes"] == expected_shapes + + wire["skill_input_shapes"]["invoke"]["keys"].append("mutated") + wire["skill_input_shapes"]["invoke"]["unresolved_defaults"]["empty"] = "mutated" + + assert credential.as_wire_block()["skill_input_shapes"] == expected_shapes From bdf728249459e23317ab1859c094e6e9f406a602 Mon Sep 17 00:00:00 2001 From: Trecek Date: Fri, 14 Aug 2026 20:46:49 -0700 Subject: [PATCH 09/18] fix(review): parametrize receipt shape scenarios --- .../test_completion_receipt_fitness.py | 137 +++++++----------- 1 file changed, 52 insertions(+), 85 deletions(-) diff --git a/tests/contracts/test_completion_receipt_fitness.py b/tests/contracts/test_completion_receipt_fitness.py index 65bf5ec199..f3ae5f58b8 100644 --- a/tests/contracts/test_completion_receipt_fitness.py +++ b/tests/contracts/test_completion_receipt_fitness.py @@ -38,69 +38,53 @@ pytestmark = [pytest.mark.layer("contracts"), pytest.mark.small] -def test_implementation_plan_shape_supplies_unavailable_audit_context( - tmp_path: Path, - monkeypatch: pytest.MonkeyPatch, -) -> None: - from autoskillit.recipe import _api_cache - from autoskillit.recipe._api_cache import LoadCache - from autoskillit.recipe._binding import bind_runtime_skill_invocation - from autoskillit.server import _recipe_generation - - monkeypatch.setattr(_api_cache, "_LOAD_CACHE", LoadCache()) - monkeypatch.setattr(_recipe_generation, "_RECIPE_GENERATION_STORE", RecipeGenerationStore()) - payload, projection = _full_open_kitchen_generation("implementation") - tool_ctx = cast( - Any, - SimpleNamespace( - backend=None, - kitchen_id="implementation-plan-shape", - temp_dir=tmp_path, +@pytest.mark.parametrize( + ("step_name", "skill_command", "resolved", "expected_shape"), + [ + pytest.param( + "plan", + "/autoskillit:make-plan", + { + "task": "test task", + "issue_url": "https://github.com/test/test/issues/1", + "adversarial_review_level": "standard", + }, + { + "keys": ["task", "issue_url", "adversarial_review_level", "audit_cycle_path"], + "unresolved_defaults": {"audit_cycle_path": ""}, + }, + id="plan", ), - ) - prepared = prepare_recipe_delivery_generation( - payload, - recipe_name="implementation", - tool_ctx=tool_ctx, - finalized_projection=projection, - ) - credential = build_recipe_execution_credential(prepared.execution_snapshot) - shape = credential.as_wire_block()["skill_input_shapes"]["plan"] - assert shape == { - "keys": ["task", "issue_url", "adversarial_review_level", "audit_cycle_path"], - "unresolved_defaults": {"audit_cycle_path": ""}, - } - - resolved = { - "task": "test task", - "issue_url": "https://github.com/test/test/issues/1", - "adversarial_review_level": "standard", - } - defaults = cast(dict[str, str | int | bool], shape["unresolved_defaults"]) - assembled = { - name: resolved[name] if name in resolved else defaults[name] - for name in cast(list[str], shape["keys"]) - } - template = prepared.execution_snapshot.templates["plan"] - actual_mcp_kwargs = { - value.name: value.effective_value - for value in template.invocation.mcp_kwargs - if isinstance(value.effective_value, (str, int, bool)) - } - - bound = bind_runtime_skill_invocation( - template, - execution_id=prepared.execution_snapshot.execution_id, - step_name="plan", - skill_command="/autoskillit:make-plan", - skill_inputs=assembled, - actual_mcp_kwargs=actual_mcp_kwargs, - ) - - assert dict(bound) == assembled - - -def test_implementation_fix_shape_supplies_unavailable_failure_context( + pytest.param( + "fix", + "/autoskillit:resolve-failures", + { + "worktree_path": "/tmp/worktree", + "plan_path": "/tmp/plan.md", + "base_branch": "develop", + }, + { + "keys": [ + "worktree_path", + "plan_path", + "base_branch", + "ci_conclusion", + "diagnosis_path", + ], + "unresolved_defaults": { + "ci_conclusion": "", + "diagnosis_path": "", + }, + }, + id="fix", + ), + ], +) +def test_implementation_shape_supplies_unavailable_context( + step_name: str, + skill_command: str, + resolved: dict[str, str], + expected_shape: dict[str, object], tmp_path: Path, monkeypatch: pytest.MonkeyPatch, ) -> None: @@ -116,7 +100,7 @@ def test_implementation_fix_shape_supplies_unavailable_failure_context( Any, SimpleNamespace( backend=None, - kitchen_id="implementation-fix-shape", + kitchen_id=f"implementation-{step_name}-shape", temp_dir=tmp_path, ), ) @@ -127,32 +111,15 @@ def test_implementation_fix_shape_supplies_unavailable_failure_context( finalized_projection=projection, ) credential = build_recipe_execution_credential(prepared.execution_snapshot) - shape = credential.as_wire_block()["skill_input_shapes"]["fix"] - assert shape == { - "keys": [ - "worktree_path", - "plan_path", - "base_branch", - "ci_conclusion", - "diagnosis_path", - ], - "unresolved_defaults": { - "ci_conclusion": "", - "diagnosis_path": "", - }, - } + shape = credential.as_wire_block()["skill_input_shapes"][step_name] + assert shape == expected_shape - resolved = { - "worktree_path": "/tmp/worktree", - "plan_path": "/tmp/plan.md", - "base_branch": "develop", - } defaults = cast(dict[str, str | int | bool], shape["unresolved_defaults"]) assembled = { name: resolved[name] if name in resolved else defaults[name] for name in cast(list[str], shape["keys"]) } - template = prepared.execution_snapshot.templates["fix"] + template = prepared.execution_snapshot.templates[step_name] actual_mcp_kwargs = { value.name: value.effective_value for value in template.invocation.mcp_kwargs @@ -162,8 +129,8 @@ def test_implementation_fix_shape_supplies_unavailable_failure_context( bound = bind_runtime_skill_invocation( template, execution_id=prepared.execution_snapshot.execution_id, - step_name="fix", - skill_command="/autoskillit:resolve-failures", + step_name=step_name, + skill_command=skill_command, skill_inputs=assembled, actual_mcp_kwargs=actual_mcp_kwargs, ) From d492e426793a6095c05651e955dfce81cf9643b3 Mon Sep 17 00:00:00 2001 From: Trecek Date: Fri, 14 Aug 2026 20:47:44 -0700 Subject: [PATCH 10/18] fix(review): share attested shape assertions --- tests/_helpers.py | 11 +++++++++++ tests/cli/test_mcp_startup_recovery_policy.py | 12 ++---------- tests/cli/test_sous_chef_content.py | 12 ++---------- .../server/test_tools_execution_recipe_execution.py | 12 ++---------- 4 files changed, 17 insertions(+), 30 deletions(-) diff --git a/tests/_helpers.py b/tests/_helpers.py index 4c3f4c4a6e..9318d8dada 100644 --- a/tests/_helpers.py +++ b/tests/_helpers.py @@ -10,6 +10,17 @@ strip_markdown_code_regions as strip_markdown_code_regions, ) +ATTESTED_SKILL_INPUT_SHAPE_ATOMS = ( + "skill_input_shapes[step_name]", + "ordered keys", + "unresolved_defaults", + "replace available values in place", + "never delete or invent a key", + '""', + "0", + "False", +) + def seed_registry_owner(project_dir: Path, launch_id: str) -> None: """Seed stable owner identity fields into a test session registry row.""" diff --git a/tests/cli/test_mcp_startup_recovery_policy.py b/tests/cli/test_mcp_startup_recovery_policy.py index 03a4886702..c4ef268695 100644 --- a/tests/cli/test_mcp_startup_recovery_policy.py +++ b/tests/cli/test_mcp_startup_recovery_policy.py @@ -5,6 +5,7 @@ import pytest from autoskillit.cli import _prompts +from tests._helpers import ATTESTED_SKILL_INPUT_SHAPE_ATOMS pytestmark = [pytest.mark.layer("cli"), pytest.mark.small] @@ -54,14 +55,5 @@ def test_canonical_instruction_is_rendered_from_the_policy() -> None: def test_startup_policy_preserves_attested_skill_input_shape() -> None: rendered = _prompts._MCP_STARTUP_RECOVERY_SPEC.render() - for required in ( - "skill_input_shapes[step_name]", - "ordered keys", - "unresolved_defaults", - "replace available values in place", - "never delete or invent a key", - '""', - "0", - "False", - ): + for required in ATTESTED_SKILL_INPUT_SHAPE_ATOMS: assert required in rendered diff --git a/tests/cli/test_sous_chef_content.py b/tests/cli/test_sous_chef_content.py index e01cc82be4..3c61092a4e 100644 --- a/tests/cli/test_sous_chef_content.py +++ b/tests/cli/test_sous_chef_content.py @@ -5,6 +5,7 @@ import pytest from autoskillit.cli._prompts import _read_full_sous_chef +from tests._helpers import ATTESTED_SKILL_INPUT_SHAPE_ATOMS pytestmark = [pytest.mark.layer("cli"), pytest.mark.small] @@ -88,14 +89,5 @@ def test_sous_chef_requires_progressive_segment_consumption() -> None: def test_sous_chef_preserves_attested_skill_input_shape_and_falsey_defaults() -> None: content = _read_full_sous_chef() - for required in ( - "skill_input_shapes[step_name]", - "ordered keys", - "unresolved_defaults", - "replace available values in place", - "never delete or invent a key", - '""', - "0", - "False", - ): + for required in ATTESTED_SKILL_INPUT_SHAPE_ATOMS: assert required in content diff --git a/tests/server/test_tools_execution_recipe_execution.py b/tests/server/test_tools_execution_recipe_execution.py index c83c4fbf42..86f733d430 100644 --- a/tests/server/test_tools_execution_recipe_execution.py +++ b/tests/server/test_tools_execution_recipe_execution.py @@ -13,6 +13,7 @@ RECIPE_EXECUTION_INACTIVE_MESSAGE, RUN_SKILL_ATTESTATION_PARAMS, ) +from tests._helpers import ATTESTED_SKILL_INPUT_SHAPE_ATOMS pytestmark = [pytest.mark.layer("server"), pytest.mark.small] @@ -30,16 +31,7 @@ def test_attestation_missing_message_names_remedy_tool(self) -> None: assert "complete_recipe_initialization" in RECIPE_EXECUTION_ATTESTATION_MISSING_MESSAGE def test_attestation_missing_message_preserves_delivered_skill_input_shape(self) -> None: - for required in ( - "skill_input_shapes[step_name]", - "ordered keys", - "unresolved_defaults", - "replace available values in place", - "never delete or invent a key", - '""', - "0", - "False", - ): + for required in ATTESTED_SKILL_INPUT_SHAPE_ATOMS: assert required in RECIPE_EXECUTION_ATTESTATION_MISSING_MESSAGE def test_inactive_message_does_not_say_standalone_mode(self) -> None: From f598cbb4e912149ef20da6e56634f9df72760a1f Mon Sep 17 00:00:00 2001 From: Trecek Date: Fri, 14 Aug 2026 21:57:03 -0700 Subject: [PATCH 11/18] fix(review): assert startup skill input guidance context --- tests/cli/test_mcp_startup_recovery_policy.py | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/tests/cli/test_mcp_startup_recovery_policy.py b/tests/cli/test_mcp_startup_recovery_policy.py index c4ef268695..c2f4ca5e93 100644 --- a/tests/cli/test_mcp_startup_recovery_policy.py +++ b/tests/cli/test_mcp_startup_recovery_policy.py @@ -5,7 +5,6 @@ import pytest from autoskillit.cli import _prompts -from tests._helpers import ATTESTED_SKILL_INPUT_SHAPE_ATOMS pytestmark = [pytest.mark.layer("cli"), pytest.mark.small] @@ -55,5 +54,11 @@ def test_canonical_instruction_is_rendered_from_the_policy() -> None: def test_startup_policy_preserves_attested_skill_input_shape() -> None: rendered = _prompts._MCP_STARTUP_RECOVERY_SPEC.render() - for required in ATTESTED_SKILL_INPUT_SHAPE_ATOMS: - assert required in rendered + assert ( + "For structured child inputs, select " + "recipe_execution.skill_input_shapes[step_name], initialize skill_inputs " + "with exactly its ordered keys, and replace available values in place. " + "For unavailable context, copy only that key's advertised " + 'unresolved_defaults entry by key presence, so "", 0, and False remain ' + "verbatim; never delete or invent a key." + ) in rendered From 60167b52c7593ab55af8c8eb2de66e2acba0e09c Mon Sep 17 00:00:00 2001 From: Trecek Date: Fri, 14 Aug 2026 21:57:19 -0700 Subject: [PATCH 12/18] fix(review): assert sous-chef input guidance context --- tests/cli/test_sous_chef_content.py | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/tests/cli/test_sous_chef_content.py b/tests/cli/test_sous_chef_content.py index 3c61092a4e..f89d38a362 100644 --- a/tests/cli/test_sous_chef_content.py +++ b/tests/cli/test_sous_chef_content.py @@ -5,7 +5,6 @@ import pytest from autoskillit.cli._prompts import _read_full_sous_chef -from tests._helpers import ATTESTED_SKILL_INPUT_SHAPE_ATOMS pytestmark = [pytest.mark.layer("cli"), pytest.mark.small] @@ -89,5 +88,13 @@ def test_sous_chef_requires_progressive_segment_consumption() -> None: def test_sous_chef_preserves_attested_skill_input_shape_and_falsey_defaults() -> None: content = _read_full_sous_chef() - for required in ATTESTED_SKILL_INPUT_SHAPE_ATOMS: - assert required in content + assert ( + "For structured child inputs, select " + "`recipe_execution.skill_input_shapes[step_name]` and\n" + "initialize `skill_inputs` with exactly its ordered keys; replace available " + "values in place;\n" + "for unavailable context, copy a value only from that key's advertised\n" + "`unresolved_defaults` entry. Test key presence rather than truthiness so " + '`""`, `0`, and\n' + "`False` are forwarded verbatim; never delete or invent a key." + ) in content From ace6cbeee5eda9ef7fe51b12b312cd28b63f844a Mon Sep 17 00:00:00 2001 From: Trecek Date: Fri, 14 Aug 2026 21:57:35 -0700 Subject: [PATCH 13/18] fix(review): bind attestation guidance assertions --- tests/_helpers.py | 11 ----------- tests/server/test_tools_execution_recipe_execution.py | 10 +++++++--- 2 files changed, 7 insertions(+), 14 deletions(-) diff --git a/tests/_helpers.py b/tests/_helpers.py index 9318d8dada..4c3f4c4a6e 100644 --- a/tests/_helpers.py +++ b/tests/_helpers.py @@ -10,17 +10,6 @@ strip_markdown_code_regions as strip_markdown_code_regions, ) -ATTESTED_SKILL_INPUT_SHAPE_ATOMS = ( - "skill_input_shapes[step_name]", - "ordered keys", - "unresolved_defaults", - "replace available values in place", - "never delete or invent a key", - '""', - "0", - "False", -) - def seed_registry_owner(project_dir: Path, launch_id: str) -> None: """Seed stable owner identity fields into a test session registry row.""" diff --git a/tests/server/test_tools_execution_recipe_execution.py b/tests/server/test_tools_execution_recipe_execution.py index 86f733d430..e99dc1b18b 100644 --- a/tests/server/test_tools_execution_recipe_execution.py +++ b/tests/server/test_tools_execution_recipe_execution.py @@ -13,7 +13,6 @@ RECIPE_EXECUTION_INACTIVE_MESSAGE, RUN_SKILL_ATTESTATION_PARAMS, ) -from tests._helpers import ATTESTED_SKILL_INPUT_SHAPE_ATOMS pytestmark = [pytest.mark.layer("server"), pytest.mark.small] @@ -31,8 +30,13 @@ def test_attestation_missing_message_names_remedy_tool(self) -> None: assert "complete_recipe_initialization" in RECIPE_EXECUTION_ATTESTATION_MISSING_MESSAGE def test_attestation_missing_message_preserves_delivered_skill_input_shape(self) -> None: - for required in ATTESTED_SKILL_INPUT_SHAPE_ATOMS: - assert required in RECIPE_EXECUTION_ATTESTATION_MISSING_MESSAGE + assert ( + "structured calls must initialize skill_inputs from " + "skill_input_shapes[step_name] ordered keys, replace available values in place, " + "copy only advertised unresolved_defaults by key presence " + 'so "", 0, and False remain verbatim, and never delete or invent a key' + in RECIPE_EXECUTION_ATTESTATION_MISSING_MESSAGE + ) def test_inactive_message_does_not_say_standalone_mode(self) -> None: assert "standalone mode" not in RECIPE_EXECUTION_INACTIVE_MESSAGE.lower() From c81a84d11b06a01cf0c9ba52d8b9d368e6b3756c Mon Sep 17 00:00:00 2001 From: Trecek Date: Fri, 14 Aug 2026 21:58:13 -0700 Subject: [PATCH 14/18] fix(review): validate skill input unresolved defaults --- src/autoskillit/recipe/_contracts_types.py | 8 ++++++++ tests/recipe/test_contracts_types.py | 13 +++++++++++++ 2 files changed, 21 insertions(+) diff --git a/src/autoskillit/recipe/_contracts_types.py b/src/autoskillit/recipe/_contracts_types.py index df74537f6c..0381324e7c 100644 --- a/src/autoskillit/recipe/_contracts_types.py +++ b/src/autoskillit/recipe/_contracts_types.py @@ -24,6 +24,14 @@ class SkillInput: nullable: bool = True unresolved_default: BoundScalar | None = None + def __post_init__(self) -> None: + if self.unresolved_default is not None and type(self.unresolved_default) not in ( + str, + int, + bool, + ): + raise ValueError("SkillInput.unresolved_default must be a strict scalar or None") + def accepts(self, value: object) -> bool: normalized = self.type if normalized in { diff --git a/tests/recipe/test_contracts_types.py b/tests/recipe/test_contracts_types.py index e200e6e6b9..e1146f3575 100644 --- a/tests/recipe/test_contracts_types.py +++ b/tests/recipe/test_contracts_types.py @@ -22,6 +22,19 @@ def test_skill_input_rejects_noncanonical_float(input_type: str) -> None: assert not skill_input.accepts(1.5) +@pytest.mark.parametrize("unresolved_default", [1.5, {}, []]) +def test_skill_input_rejects_non_scalar_unresolved_default( + unresolved_default: object, +) -> None: + with pytest.raises(ValueError, match="unresolved_default must be a strict scalar"): + SkillInput( + name="value", + type="string", + required=False, + unresolved_default=unresolved_default, # type: ignore[arg-type] + ) + + def test_skill_contract_rejects_unknown_input_preflight() -> None: with pytest.raises(ValueError, match="unsupported input preflight"): SkillContract(inputs=(), outputs=[], input_preflight="unknown") From 566677e76141dabf73d89e62602c9a480a1db2e0 Mon Sep 17 00:00:00 2001 From: Trecek Date: Fri, 14 Aug 2026 23:19:58 -0700 Subject: [PATCH 15/18] fix: preserve segmented skill input shapes --- src/autoskillit/server/_recipe_initialization.py | 5 +++++ tests/server/test_recipe_initialization.py | 7 +++++++ 2 files changed, 12 insertions(+) diff --git a/src/autoskillit/server/_recipe_initialization.py b/src/autoskillit/server/_recipe_initialization.py index 5b232b0c15..b7b5309f58 100644 --- a/src/autoskillit/server/_recipe_initialization.py +++ b/src/autoskillit/server/_recipe_initialization.py @@ -553,6 +553,11 @@ def _public_completion_credential( for step_name, digest in credential.invocation_template_digests.items() if step_name in initial_steps }, + skill_input_shapes={ + step_name: shape + for step_name, shape in credential.skill_input_shapes.items() + if step_name in initial_steps + }, ) diff --git a/tests/server/test_recipe_initialization.py b/tests/server/test_recipe_initialization.py index 835f187a23..f301169571 100644 --- a/tests/server/test_recipe_initialization.py +++ b/tests/server/test_recipe_initialization.py @@ -185,6 +185,10 @@ def test_segmented_completion_credential_is_scoped_to_initial_bodies() -> None: execution_id="execution", snapshot_digest=_hash("snapshot"), invocation_template_digests={"initial": _hash("initial"), "future": _hash("future")}, + skill_input_shapes={ + "initial": {"keys": ["task"], "unresolved_defaults": {}}, + "future": {"keys": ["review_path"], "unresolved_defaults": {"review_path": ""}}, + }, ) public_credential = recipe_initialization._public_completion_credential(credential, projection) @@ -192,6 +196,9 @@ def test_segmented_completion_credential_is_scoped_to_initial_bodies() -> None: assert public_credential.execution_id == credential.execution_id assert public_credential.snapshot_digest == credential.snapshot_digest assert public_credential.invocation_template_digests == {"initial": _hash("initial")} + assert public_credential.skill_input_shapes == { + "initial": {"keys": ["task"], "unresolved_defaults": {}} + } def test_install_rejects_initializing_recipe_without_completion_receipt( From 94df65e784780b64626cb1efd9bcb8b7f3e972ee Mon Sep 17 00:00:00 2001 From: Trecek Date: Fri, 14 Aug 2026 23:53:45 -0700 Subject: [PATCH 16/18] fix: identify review counter delivery checkpoints --- src/autoskillit/recipes/implementation.json | 3 ++- src/autoskillit/recipes/implementation.yaml | 1 + src/autoskillit/recipes/remediation.json | 3 ++- src/autoskillit/recipes/remediation.yaml | 1 + tests/recipe/test_implementation.py | 3 ++- tests/recipe/test_remediation_recipe.py | 2 +- 6 files changed, 9 insertions(+), 4 deletions(-) diff --git a/src/autoskillit/recipes/implementation.json b/src/autoskillit/recipes/implementation.json index 7f24b63c47..8897e93739 100644 --- a/src/autoskillit/recipes/implementation.json +++ b/src/autoskillit/recipes/implementation.json @@ -1558,7 +1558,8 @@ "tool": "run_python", "with": { "callable": "autoskillit.smoke_utils.init_counter", - "counter_value": "${{ context.review_loop_count }}" + "counter_value": "${{ context.review_loop_count }}", + "step_name": "init_review_loop_count" }, "capture": { "review_loop_count": "${{ result.value }}" diff --git a/src/autoskillit/recipes/implementation.yaml b/src/autoskillit/recipes/implementation.yaml index ec9599d5ae..79476ba6a2 100644 --- a/src/autoskillit/recipes/implementation.yaml +++ b/src/autoskillit/recipes/implementation.yaml @@ -1293,6 +1293,7 @@ steps: with: callable: autoskillit.smoke_utils.init_counter counter_value: ${{ context.review_loop_count }} + step_name: init_review_loop_count capture: review_loop_count: ${{ result.value }} on_success: clear_review_annotation_context diff --git a/src/autoskillit/recipes/remediation.json b/src/autoskillit/recipes/remediation.json index 2a1799d706..9f063966fa 100644 --- a/src/autoskillit/recipes/remediation.json +++ b/src/autoskillit/recipes/remediation.json @@ -1893,7 +1893,8 @@ "tool": "run_python", "with": { "callable": "autoskillit.smoke_utils.init_counter", - "counter_value": "${{ context.review_loop_count }}" + "counter_value": "${{ context.review_loop_count }}", + "step_name": "init_review_loop_count" }, "capture": { "review_loop_count": "${{ result.value }}" diff --git a/src/autoskillit/recipes/remediation.yaml b/src/autoskillit/recipes/remediation.yaml index 289a803c3a..bf26d4043d 100644 --- a/src/autoskillit/recipes/remediation.yaml +++ b/src/autoskillit/recipes/remediation.yaml @@ -1583,6 +1583,7 @@ steps: with: callable: autoskillit.smoke_utils.init_counter counter_value: ${{ context.review_loop_count }} + step_name: init_review_loop_count capture: review_loop_count: ${{ result.value }} on_success: clear_review_annotation_context diff --git a/tests/recipe/test_implementation.py b/tests/recipe/test_implementation.py index 53ff595374..ecffd454e9 100644 --- a/tests/recipe/test_implementation.py +++ b/tests/recipe/test_implementation.py @@ -21,7 +21,7 @@ RECIPE_PATH = builtin_recipes_dir() / "implementation.yaml" _PRE_DELIVERY_STRUCTURE_SHA256 = ( - "sha256:c3e53626cbf7549b2bb225ff54fede2706ec85aef3296ce90678db4bcf4f8feb" + "sha256:d3ba8c051fb8a7f1decdf2ea84662bf820112efd265957117fb46ffba7f59909" ) _CHECKPOINT_HANDLER_TABLE = { @@ -34,6 +34,7 @@ "direct_merge": "run_cmd", "redirect_merge": "run_cmd", "immediate_merge": "run_cmd", + "init_review_loop_count": "run_python", "remerge_immediate": "run_cmd", "detect_ci_conflict": "run_cmd", } diff --git a/tests/recipe/test_remediation_recipe.py b/tests/recipe/test_remediation_recipe.py index bfea1afdca..d222e78f05 100644 --- a/tests/recipe/test_remediation_recipe.py +++ b/tests/recipe/test_remediation_recipe.py @@ -14,7 +14,7 @@ Path(__file__).parent.parent.parent / "src" / "autoskillit" / "recipes" / "remediation.yaml" ) _PRE_DELIVERY_STRUCTURE_SHA256 = ( - "sha256:2e7315d7256ce035110cd376ada4f5403c0a1ce574220ff488c004ec2cfea6e4" + "sha256:353517069e9715536b96428fe385a815ae580e69ce29f15065986d2be3e1fe15" ) From 64d2d9b2e034e7e0f856f7116fee05e6678f8573 Mon Sep 17 00:00:00 2001 From: Trecek Date: Sat, 15 Aug 2026 00:06:17 -0700 Subject: [PATCH 17/18] fix: scope segmented input shapes to delivered steps --- src/autoskillit/recipes/diagrams/implementation.md | 2 +- src/autoskillit/recipes/diagrams/remediation.md | 2 +- src/autoskillit/server/_recipe_segment_delivery.py | 5 +++++ tests/contracts/test_delivery_mode_ledger.py | 4 ++-- tests/server/test_recipe_segment_delivery.py | 3 +++ 5 files changed, 12 insertions(+), 4 deletions(-) diff --git a/src/autoskillit/recipes/diagrams/implementation.md b/src/autoskillit/recipes/diagrams/implementation.md index a108f650e3..3b6cd42803 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 e7c6759610..0993271710 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/server/_recipe_segment_delivery.py b/src/autoskillit/server/_recipe_segment_delivery.py index e0560b2d16..eed87d3eb4 100644 --- a/src/autoskillit/server/_recipe_segment_delivery.py +++ b/src/autoskillit/server/_recipe_segment_delivery.py @@ -149,6 +149,11 @@ def _segment_execution_credential( credential["invocation_template_digests"] = { step_name: digests[step_name] for step_name in ordered_step_names if step_name in digests } + shapes = credential["skill_input_shapes"] + assert isinstance(shapes, dict) + credential["skill_input_shapes"] = { + step_name: shapes[step_name] for step_name in ordered_step_names if step_name in shapes + } return credential diff --git a/tests/contracts/test_delivery_mode_ledger.py b/tests/contracts/test_delivery_mode_ledger.py index e3d64b9476..22c1949af0 100644 --- a/tests/contracts/test_delivery_mode_ledger.py +++ b/tests/contracts/test_delivery_mode_ledger.py @@ -182,8 +182,8 @@ def test_delivery_mode_is_pinned( ("consolidate-health-reports", "codex"): 12_000, ("full-audit", "claude-code"): 37_000, ("implement-findings", "claude-code"): 34_000, - ("implementation", "claude-code"): 10_000, - ("implementation", "codex"): 10_000, + ("implementation", "claude-code"): 11_600, + ("implementation", "codex"): 11_600, ("planner", "claude-code"): 80_000, ("promote-to-main-wrapper", "claude-code"): 19_000, ("remediation", "claude-code"): 10_000, diff --git a/tests/server/test_recipe_segment_delivery.py b/tests/server/test_recipe_segment_delivery.py index 48d50545ca..a043608ec8 100644 --- a/tests/server/test_recipe_segment_delivery.py +++ b/tests/server/test_recipe_segment_delivery.py @@ -168,6 +168,9 @@ async def test_checkpoint_delivery_reads_ready_exact_durable_artifact( assert prepared is not None assert prepared.success_carrier["source_step"] == "scope" assert [body["step"] for body in prepared.success_carrier["bodies"]] == ["select_directions"] + credential = prepared.success_carrier[RECIPE_EXECUTION_CREDENTIAL_WIRE_KEY] + assert set(credential["invocation_template_digests"]) == {"select_directions"} + assert set(credential["skill_input_shapes"]) == {"select_directions"} assert prepared.success_carrier["recipe_pull"] == state.artifact_generation.pull_identity() assert ( prepared.success_carrier["bodies"][0]["body"] From eb91e0cab6882889b41f64189c5d8db0ca2967bd Mon Sep 17 00:00:00 2001 From: Trecek Date: Sat, 15 Aug 2026 00:15:09 -0700 Subject: [PATCH 18/18] test: refresh recipe response size maxima --- tests/infra/test_pretty_output_recipe.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/infra/test_pretty_output_recipe.py b/tests/infra/test_pretty_output_recipe.py index 42351e20d9..911bbc042c 100644 --- a/tests/infra/test_pretty_output_recipe.py +++ b/tests/infra/test_pretty_output_recipe.py @@ -1164,9 +1164,9 @@ def test_canonical_recipe_responses_fit_independent_registry_ceilings(tmp_path, for ingredients_only in (False, True) } assert maxima == { - "get_recipe_section": (152_582, "implementation-groups", "all_truthy"), - "load_recipe": (152_582, "implementation-groups", "all_truthy"), - "open_kitchen": (152_635, "implementation-groups", "all_truthy"), + "get_recipe_section": (179_555, "remediation", "all_truthy"), + "load_recipe": (179_555, "remediation", "all_truthy"), + "open_kitchen": (179_608, "remediation", "all_truthy"), }