From 0912912e744e1d3eb6a8f73f88d1e87b49dbcb1d Mon Sep 17 00:00:00 2001 From: Trecek Date: Fri, 9 Oct 2026 21:08:07 -0700 Subject: [PATCH 01/12] feat: add shared output_dir boundary predicate to the write-scope authority output_dir_widens_scope decides whether a dispatch output_dir widens a skill's declared write scope, matching the scope kind before calling bounded_scope_contains. widened_output_dir_message is the diagnostic that names the skill, the requested value, and the declared directories. Co-Authored-By: Claude Opus 5.5 --- src/autoskillit/hooks/AGENTS.md | 2 +- src/autoskillit/hooks/_write_scope.py | 26 ++++++++++++++++-- tests/hooks/test_write_scope.py | 38 +++++++++++++++++++++++++++ 3 files changed, 63 insertions(+), 3 deletions(-) diff --git a/src/autoskillit/hooks/AGENTS.md b/src/autoskillit/hooks/AGENTS.md index fe8a1dc105..dba53548ea 100644 --- a/src/autoskillit/hooks/AGENTS.md +++ b/src/autoskillit/hooks/AGENTS.md @@ -74,6 +74,6 @@ Files with no registration support their folder; files with several registration - `_dispatch.py` — Support module: Stable hook dispatcher — NEVER RENAME THIS FILE. - `_join_ledger.py` — Support module: Stdlib-only durable authority for declared fixed-set join batches. - `_session_binding.py` — Support module: Shared authority for the cross-process session-binding channel. -- `_write_scope.py` — Support module: Stdlib-only authority for typed skill write scopes, their expansion and containment, and the session fold. +- `_write_scope.py` — Support module: Stdlib-only authority for typed skill write scopes, their expansion and containment, the session fold, and the `output_dir` boundary predicate. - `session_lifetime_notice_hook.py` — Groups 2 registrations: Interactive lifetime notice on PostToolUse and Stop. - `skill_load_post_hook.py` — Groups 2 registrations: PostToolUse Skill and UserPromptExpansion slash-command binding writer. diff --git a/src/autoskillit/hooks/_write_scope.py b/src/autoskillit/hooks/_write_scope.py index 57934d3938..2a4fdb0c02 100644 --- a/src/autoskillit/hooks/_write_scope.py +++ b/src/autoskillit/hooks/_write_scope.py @@ -7,8 +7,9 @@ * ``INHERIT`` — the literal ``inherit``: the skill has no writes of its own. This module owns the single decoder and encoder for that declaration, the one -expansion of both temp spellings, the temp-root containment check, and the -session fold that composes loaded skills' scopes by union. It is imported both as +expansion of both temp spellings, the temp-root containment check, the session +fold that composes loaded skills' scopes by union, and the ``output_dir`` boundary +predicate shared by ``run_skill`` admission and the recipe rule. It is imported both as ``_write_scope`` by hook subprocesses and as ``autoskillit.hooks._write_scope`` by in-venv callers. """ @@ -210,6 +211,25 @@ def bounded_scope_contains(scope: WriteScope, path: str, project_dir: str) -> bo ) +def output_dir_widens_scope(scope: WriteScope, output_dir: str, project_dir: str) -> bool: + """Return whether a dispatch ``output_dir`` ("" if omitted) widens the skill's write scope.""" + match scope.kind: + case WriteScopeKind.BOUNDED: + return bool(output_dir) and not bounded_scope_contains(scope, output_dir, project_dir) + case WriteScopeKind.UNRESTRICTED | WriteScopeKind.INHERIT: + return False + case _ as unreachable: + assert_never(unreachable) + + +def widened_output_dir_message(skill_name: str, scope: WriteScope, output_dir: str) -> str: + return ( + f"output_dir {output_dir!r} is outside the declared write scope of skill {skill_name!r} " + f"(write_paths: {', '.join(scope.paths)}); omit output_dir to use the skill's own temp " + "directory, or pass a directory inside write_paths" + ) + + __all__ = [ "TEMP_PLACEHOLDER", "WRITE_SCOPE_INHERIT", @@ -224,6 +244,8 @@ def bounded_scope_contains(scope: WriteScope, path: str, project_dir: str) -> bo "encode_write_scope", "expand_write_path", "fold_session_write_scopes", + "output_dir_widens_scope", "temp_root", "temp_root_escape", + "widened_output_dir_message", ] diff --git a/tests/hooks/test_write_scope.py b/tests/hooks/test_write_scope.py index cb9861fbf6..a50d07023e 100644 --- a/tests/hooks/test_write_scope.py +++ b/tests/hooks/test_write_scope.py @@ -18,7 +18,9 @@ encode_write_scope, expand_write_path, fold_session_write_scopes, + output_dir_widens_scope, temp_root_escape, + widened_output_dir_message, ) pytestmark = [pytest.mark.layer("infra"), pytest.mark.small] @@ -178,3 +180,39 @@ def test_bounded_scope_contains_rejects_unbounded_scopes( ) -> None: with pytest.raises(WriteScopeError): bounded_scope_contains(scope, "{{AUTOSKILLIT_TEMP}}/a/", str(tmp_path)) + + +@pytest.mark.parametrize( + ("output_dir", "widens"), + [ + ("", False), + (".", True), + ("/", True), + (".autoskillit/temp", True), + (".autoskillit/temp/b", True), + (".autoskillit/temp/a/iter_1", False), + ], +) +def test_output_dir_widens_bounded_scope(tmp_path: Path, output_dir: str, widens: bool) -> None: + assert output_dir_widens_scope(_A[1], output_dir, str(tmp_path)) is widens + + +def test_output_dir_widens_bounded_scope_judges_absolute_runtime_paths(tmp_path: Path) -> None: + project = str(tmp_path) + + assert output_dir_widens_scope(_A[1], str(tmp_path / ".autoskillit/temp/a"), project) is False + assert output_dir_widens_scope(_A[1], project, project) is True + + +@pytest.mark.parametrize("scope", [_INHERIT[1], _UNRESTRICTED[1]]) +def test_output_dir_never_widens_unbounded_scopes(tmp_path: Path, scope: WriteScope) -> None: + assert output_dir_widens_scope(scope, ".", str(tmp_path)) is False + + +def test_widened_output_dir_message_names_skill_value_and_declared_paths() -> None: + scope = _bounded("{{AUTOSKILLIT_TEMP}}/make-plan/", "{{AUTOSKILLIT_TEMP}}/other/") + + message = widened_output_dir_message("make-plan", scope, ".") + + for fragment in ("'make-plan'", "'.'", *scope.paths): + assert fragment in message From 1b1141e48d6ac111e7506d54a0645dc679689129 Mon Sep 17 00:00:00 2001 From: Trecek Date: Fri, 9 Oct 2026 21:08:49 -0700 Subject: [PATCH 02/12] fix: name the skill, value and scope when run_skill rejects output_dir _extend_closure_write_scope now decides through output_dir_widens_scope and reports widened_output_dir_message, so a rejected dispatch says which skill, which output_dir, and which declared directories were involved. Admission semantics and the closure union extension are unchanged. Co-Authored-By: Claude Opus 5.5 --- .../tools_execution/_run_skill_session.py | 28 ++++++++----------- .../test_tools_execution_write_prefix.py | 18 +++++++++++- 2 files changed, 28 insertions(+), 18 deletions(-) diff --git a/src/autoskillit/server/tools/tools_execution/_run_skill_session.py b/src/autoskillit/server/tools/tools_execution/_run_skill_session.py index 0c4c29eb2b..5455522aea 100644 --- a/src/autoskillit/server/tools/tools_execution/_run_skill_session.py +++ b/src/autoskillit/server/tools/tools_execution/_run_skill_session.py @@ -13,7 +13,7 @@ from collections.abc import Mapping from dataclasses import replace from pathlib import Path -from typing import TYPE_CHECKING, assert_never +from typing import TYPE_CHECKING from uuid import uuid4 from autoskillit.core import ( @@ -33,7 +33,7 @@ resolve_skill_temp_dir as _resolve_skill_temp_dir, ) from autoskillit.execution import ReplayingSubprocessRunner -from autoskillit.hooks._write_scope import WriteScopeKind, bounded_scope_contains +from autoskillit.hooks._write_scope import output_dir_widens_scope, widened_output_dir_message from autoskillit.pipeline import canonical_step_name as _canonical_step_name from autoskillit.pipeline import gate_error_result from autoskillit.server._explorer_projection import _build_requested_execution_identity @@ -611,20 +611,14 @@ def _extend_closure_write_scope(state: _RunSkillDispatchState) -> str | None: return _write_scope_failure(str(exc)) root_scope = state.invocation.root.write_scope assert root_scope is not None - match root_scope.kind: - case WriteScopeKind.BOUNDED: - # `_resolve_dispatch_paths` populates write_watch_dirs from state.output_dir - # before this runs, so [0] is the requested output_dir when supplied, - # or the default temp floor otherwise. The default floor is not narrowed. - if state.output_dir and not bounded_scope_contains( - root_scope, str(state.write_watch_dirs[0]), state.cwd - ): - return _write_scope_failure( - "run_skill output_dir is outside the skill's declared write scope" - ) - case WriteScopeKind.UNRESTRICTED | WriteScopeKind.INHERIT: - pass - case _ as unreachable: - assert_never(unreachable) + # `_resolve_dispatch_paths` populates write_watch_dirs from state.output_dir + # before this runs, so [0] is the granted output_dir prefix when supplied, + # or the default temp floor otherwise. The default floor is not narrowed. + if output_dir_widens_scope( + root_scope, str(state.write_watch_dirs[0]) if state.output_dir else "", state.cwd + ): + return _write_scope_failure( + widened_output_dir_message(state.invocation.root.name, root_scope, state.output_dir) + ) state.write_watch_dirs.extend(closure_dirs) return None diff --git a/tests/server/test_tools_execution_write_prefix.py b/tests/server/test_tools_execution_write_prefix.py index fecac2a47e..5bc532aa3e 100644 --- a/tests/server/test_tools_execution_write_prefix.py +++ b/tests/server/test_tools_execution_write_prefix.py @@ -375,10 +375,26 @@ def test_bounded_root_rejects_output_dir_outside_its_scope(tmp_path: Path) -> No assert state.write_watch_dirs == original_dirs assert failure is not None - assert failure["error"] == "run_skill output_dir is outside the skill's declared write scope" + for fragment in ("'widget'", "'.autoskillit/temp/other'", "{{AUTOSKILLIT_TEMP}}/widgets/"): + assert fragment in failure["error"] assert failure["stage"] == "validate_args:run_skill" +def test_bounded_root_rejects_worktree_root_output_dir(tool_ctx, tmp_path: Path) -> None: + from autoskillit.core import SkillExecutionRole + + invocation = tool_ctx.skill_resolver.resolve_invocation( + "make-plan", tool_ctx.project_dir, SkillExecutionRole.SESSION + ) + + failure = _extend(_scope_state(tmp_path, invocation.root, output_dir=".")) + + assert failure is not None + assert failure["stage"] == "validate_args:run_skill" + for fragment in ("'make-plan'", "'.'", "{{AUTOSKILLIT_TEMP}}/make-plan/"): + assert fragment in failure["error"] + + def test_bounded_root_admits_output_dir_inside_its_scope(tmp_path: Path) -> None: root = _scoped_skill(tmp_path, "widget", "['{{AUTOSKILLIT_TEMP}}/widgets/']") state = _scope_state(tmp_path, root, output_dir=".autoskillit/temp/widgets/run-1") From 4ec453ffd34c7391a7e4f83ef31deee625efe86e Mon Sep 17 00:00:00 2001 From: Trecek Date: Fri, 9 Oct 2026 21:10:43 -0700 Subject: [PATCH 03/12] refactor: resolve a skill's declared write scope through one recipe helper skill_write_scope resolves a SKILL.md, reads it once, and decodes its write_paths declaration. The alignment rule now reads SKILL.md through it; its findings are unchanged. Co-Authored-By: Claude Opus 5.5 --- src/autoskillit/recipe/_skill_helpers.py | 2 + src/autoskillit/recipe/helpers/__init__.py | 3 ++ .../recipe/helpers/_skill_helpers.py | 23 +++++++++++ .../rules/rules_skill_write_path_alignment.py | 39 +++++++------------ 4 files changed, 42 insertions(+), 25 deletions(-) diff --git a/src/autoskillit/recipe/_skill_helpers.py b/src/autoskillit/recipe/_skill_helpers.py index d460a93b9b..c549ccd2c0 100644 --- a/src/autoskillit/recipe/_skill_helpers.py +++ b/src/autoskillit/recipe/_skill_helpers.py @@ -25,6 +25,7 @@ get_logger, logger, pkg_root, + skill_write_scope, ) __all__ = [ @@ -46,4 +47,5 @@ "get_logger", "logger", "pkg_root", + "skill_write_scope", ] diff --git a/src/autoskillit/recipe/helpers/__init__.py b/src/autoskillit/recipe/helpers/__init__.py index e141077576..7302d0688a 100644 --- a/src/autoskillit/recipe/helpers/__init__.py +++ b/src/autoskillit/recipe/helpers/__init__.py @@ -97,6 +97,7 @@ _resolve_skill_md, bound_skill_name, get_allowed_values_for_skill, + skill_write_scope, ) from ._skill_placeholder_parser import ( _BANNED_CONTENT_SUFFIX_RE, @@ -232,6 +233,7 @@ def __getattr__(name: str): "_resolve_skill_md": "_skill_helpers", "bound_skill_name": "_skill_helpers", "get_allowed_values_for_skill": "_skill_helpers", + "skill_write_scope": "_skill_helpers", "_BANNED_CONTENT_SUFFIX_RE": "_skill_placeholder_parser", "_CONTENT_VAR_SIGNAL_RE": "_skill_placeholder_parser", "_DYNAMIC_WRITE_VAR_RE": "_skill_placeholder_parser", @@ -363,6 +365,7 @@ def __getattr__(name: str): "pkg_root", "push_reachable", "shell_vars_assigned", + "skill_write_scope", "substitute_scripts_placeholder", "substitute_temp_placeholder", ] diff --git a/src/autoskillit/recipe/helpers/_skill_helpers.py b/src/autoskillit/recipe/helpers/_skill_helpers.py index 76124c3e30..9af724ab5a 100644 --- a/src/autoskillit/recipe/helpers/_skill_helpers.py +++ b/src/autoskillit/recipe/helpers/_skill_helpers.py @@ -13,6 +13,7 @@ if TYPE_CHECKING: from autoskillit.core import SkillResolver + from autoskillit.hooks._write_scope import WriteScope from autoskillit.recipe.analysis._analysis import ValidationContext @@ -92,6 +93,28 @@ def _resolve_skill_md( return skill_path if isinstance(skill_path, Path) else None +def skill_write_scope( + ctx: ValidationContext, skill_name: str +) -> tuple[Path, str, WriteScope | None] | None: + """Return a skill's SKILL.md path, text, and declared write scope (None if undeclared). + + Returns None when the SKILL.md cannot be resolved or read. + """ + skill_md_path = _resolve_skill_md( + skill_name, project_root=ctx.project_dir, resolver=ctx.skill_resolver + ) + if skill_md_path is None: + return None + try: + content = skill_md_path.read_text(encoding="utf-8") + except OSError: + logger.debug("Could not read SKILL.md for %s at %s", skill_name, skill_md_path) + return None + from autoskillit.workspace import parse_frontmatter_content + + return skill_md_path, content, parse_frontmatter_content(content).write_scope + + def _has_dynamic_skill_name(skill_cmd: str) -> bool: """Return True if the skill name portion contains template expressions. diff --git a/src/autoskillit/recipe/rules/rules_skill_write_path_alignment.py b/src/autoskillit/recipe/rules/rules_skill_write_path_alignment.py index d652e2ff98..6812c0f80a 100644 --- a/src/autoskillit/recipe/rules/rules_skill_write_path_alignment.py +++ b/src/autoskillit/recipe/rules/rules_skill_write_path_alignment.py @@ -13,10 +13,10 @@ import regex as re -from autoskillit.core import Severity, get_logger -from autoskillit.hooks._write_scope import WriteScopeKind, bounded_scope_contains +from autoskillit.core import Severity +from autoskillit.hooks._write_scope import WriteScope, WriteScopeKind, bounded_scope_contains from autoskillit.recipe._analysis import ValidationContext -from autoskillit.recipe._skill_helpers import _resolve_skill_md +from autoskillit.recipe._skill_helpers import skill_write_scope from autoskillit.recipe._skill_placeholder_parser import ( extract_write_path_declarations, has_dynamic_write_path, @@ -24,9 +24,6 @@ from autoskillit.recipe.contracts import resolve_skill_name from autoskillit.recipe.registry import RuleFinding, make_finding, semantic_rule from autoskillit.recipe.schema import RecipeStep -from autoskillit.workspace import parse_frontmatter_content - -logger = get_logger(__name__) _CONTEXT_TEMPLATE_RE = re.compile(r"\$\{\{\s*context\.[^}]+\}\}") _AUTOSKILLIT_TEMP_RE = re.compile(r"\{\{AUTOSKILLIT_TEMP\}\}") @@ -78,11 +75,12 @@ def _first_misaligned_write_path(content: str, output_dir: str) -> str | None: return None -def _declared_boundary_error(content: str, output_dir: str, project_dir: Path) -> str | None: +def _declared_boundary_error( + write_scope: WriteScope | None, output_dir: str, project_dir: Path +) -> str | None: static_base = _static_base_prefix(output_dir) if not static_base or "${{" in static_base: return None - write_scope = parse_frontmatter_content(content).write_scope if write_scope is None: return "cannot verify output_dir: skill lacks a valid write_paths declaration" match write_scope.kind: @@ -96,7 +94,7 @@ def _declared_boundary_error(content: str, output_dir: str, project_dir: Path) - assert_never(unreachable) -def _eligible_skill_path(ctx: ValidationContext, step: RecipeStep) -> tuple[str, str, Path] | None: +def _eligible_skill_path(step: RecipeStep) -> tuple[str, str] | None: if step.tool != "run_skill": return None output_dir = (step.with_args or {}).get("output_dir", "") or "" @@ -111,14 +109,7 @@ def _eligible_skill_path(ctx: ValidationContext, step: RecipeStep) -> tuple[str, skill_name = resolve_skill_name(skill_cmd) if skill_name is None: return None - skill_md_path = _resolve_skill_md( - skill_name, - project_root=ctx.project_dir, - resolver=ctx.skill_resolver, - ) - if skill_md_path is None: - return None - return output_dir, skill_name, skill_md_path + return output_dir, skill_name @semantic_rule( @@ -135,19 +126,17 @@ def _check_skill_write_path_alignment(ctx: ValidationContext) -> list[RuleFindin findings: list[RuleFinding] = [] for step_name, step in ctx.recipe.steps.items(): - eligible = _eligible_skill_path(ctx, step) + eligible = _eligible_skill_path(step) if eligible is None: continue - output_dir, skill_name, skill_md_path = eligible - - try: - content = skill_md_path.read_text(encoding="utf-8") - except OSError: - logger.debug("Could not read SKILL.md for %s at %s", skill_name, skill_md_path) + output_dir, skill_name = eligible + resolved = skill_write_scope(ctx, skill_name) + if resolved is None: continue + skill_md_path, content, write_scope = resolved boundary_error = _declared_boundary_error( - content, output_dir, ctx.project_dir or skill_md_path.parent.parent + write_scope, output_dir, ctx.project_dir or skill_md_path.parent.parent ) if boundary_error is not None: findings.append( From 14083714a4b3b6cd8a673874103919790402353b Mon Sep 17 00:00:00 2001 From: Trecek Date: Fri, 9 Oct 2026 21:12:38 -0700 Subject: [PATCH 04/12] fix: require output_dir only for UNRESTRICTED write skills write-skill-requires-source-output-dir keyed on write_behavior and demanded an output_dir from BOUNDED skills too, recommending a worktree-root value that run_skill rejects for them. It now fires only when the skill declares write_paths: unrestricted and the step omits output_dir; a BOUNDED skill's temp floor is already its declared scope. The duplicate test_write_skill_steps_have_output_dir is removed; the real rule's bundled sweep in test_rules_contracts covers it. Co-Authored-By: Claude Opus 5.5 --- .../recipe/rules/rules_contracts.py | 29 ++++++--- tests/arch/test_import_paths.py | 1 + tests/arch/test_layer_enforcement.py | 4 ++ tests/recipe/test_planner_contracts.py | 62 +------------------ tests/recipe/test_rules_contracts.py | 21 ++++++- 5 files changed, 46 insertions(+), 71 deletions(-) diff --git a/src/autoskillit/recipe/rules/rules_contracts.py b/src/autoskillit/recipe/rules/rules_contracts.py index 2c4d2e0881..8f298802a5 100644 --- a/src/autoskillit/recipe/rules/rules_contracts.py +++ b/src/autoskillit/recipe/rules/rules_contracts.py @@ -12,14 +12,16 @@ get_logger, pkg_root, ) +from autoskillit.hooks._write_scope import WriteScopeKind from autoskillit.recipe._analysis import ValidationContext from autoskillit.recipe._contracts_types import VALID_EXTERNAL_EFFECTS -from autoskillit.recipe._skill_helpers import bound_skill_name +from autoskillit.recipe._skill_helpers import bound_skill_name, skill_write_scope from autoskillit.recipe.contracts import ( get_skill_contract, load_bundled_manifest, ) from autoskillit.recipe.registry import RuleFinding, make_finding, semantic_rule +from autoskillit.recipe.schema import RecipeStep # Must match _INTENTIONALLY_EXCLUDED_PATH_TOKENS in _headless_path_tokens.py. # Cannot import directly: recipe (IL-2) cannot depend on execution (IL-1). @@ -709,17 +711,24 @@ def _check_path_output_recovery_coverage(ctx: ValidationContext) -> list[RuleFin return findings +def _unrestricted_without_output_dir(ctx: ValidationContext, name: str, step: RecipeStep) -> bool: + resolved = skill_write_scope(ctx, name) + scope = resolved[2] if resolved is not None else None + unrestricted = scope is not None and scope.kind is WriteScopeKind.UNRESTRICTED + return unrestricted and not (step.with_args or {}).get("output_dir") + + @semantic_rule( name="write-skill-requires-source-output-dir", description=( - "run_skill steps with write_behavior in (always, conditional) and not read_only " - "must declare output_dir. Without it, the write guard defaults to a temp-dir " - "prefix that blocks source-file edits." + "run_skill steps whose skill declares write_paths: unrestricted and write_behavior in " + "(always, conditional) must declare output_dir; without it the write guard confines " + "the skill to its temp floor." ), severity=Severity.ERROR, ) def _check_write_skill_requires_source_output_dir(ctx: ValidationContext) -> list[RuleFinding]: - """Fire when a write-behavior skill step lacks output_dir.""" + """Fire when an UNRESTRICTED write-behavior skill step lacks output_dir.""" try: manifest = load_bundled_manifest() except Exception: @@ -742,16 +751,16 @@ def _check_write_skill_requires_source_output_dir(ctx: ValidationContext) -> lis continue if contract.write_behavior not in ("always", "conditional") or contract.read_only: continue - if "output_dir" not in (step.with_args or {}): + if _unrestricted_without_output_dir(ctx, name, step): findings.append( make_finding( rule_name="write-skill-requires-source-output-dir", step_name=step_name, message=( - f"Step '{step_name}' invokes skill '{name}' " - f"(write_behavior={contract.write_behavior!r}) but declares " - f"no output_dir. The write guard defaults to a temp-dir " - f"prefix that blocks source-file edits. Add " + f"Step '{step_name}' invokes skill '{name}' (write_paths: unrestricted, " + f"write_behavior={contract.write_behavior!r}) but declares no " + f"output_dir. The write guard confines the skill to its temp floor, " + f"which blocks source-file edits. Add " f"output_dir: ${{{{ context.work_dir }}}} " f"(or equivalent) to this step." ), diff --git a/tests/arch/test_import_paths.py b/tests/arch/test_import_paths.py index 150b42f14d..6cf9dfd70b 100644 --- a/tests/arch/test_import_paths.py +++ b/tests/arch/test_import_paths.py @@ -133,6 +133,7 @@ def test_req_imp_002_no_core_submodule_imports() -> None: ("workspace/_projected_artifact/_validation.py", "autoskillit.hooks._write_scope"), ("workspace/session_skills/_provider.py", "autoskillit.hooks._write_scope"), ("recipe/rules/rules_skill_write_path_alignment.py", "autoskillit.hooks._write_scope"), + ("recipe/rules/rules_contracts.py", "autoskillit.hooks._write_scope"), ("migration/adapters_skill.py", "autoskillit.hooks._write_scope"), } ) diff --git a/tests/arch/test_layer_enforcement.py b/tests/arch/test_layer_enforcement.py index 7ea486551d..0fdabf1f51 100644 --- a/tests/arch/test_layer_enforcement.py +++ b/tests/arch/test_layer_enforcement.py @@ -1192,6 +1192,10 @@ def test_migration_no_forbidden_imports() -> None: "recipe/rules/rules_skill_write_path_alignment.py", "autoskillit.hooks._write_scope", ), + ( + "recipe/rules/rules_contracts.py", + "autoskillit.hooks._write_scope", + ), ( "migration/adapters_skill.py", "autoskillit.hooks._write_scope", diff --git a/tests/recipe/test_planner_contracts.py b/tests/recipe/test_planner_contracts.py index 11202f9f2e..568ebc6542 100644 --- a/tests/recipe/test_planner_contracts.py +++ b/tests/recipe/test_planner_contracts.py @@ -1,8 +1,6 @@ -"""Contract-level tests for write_behavior + output_dir coherence across bundled recipes. +"""Contract-level tests for planner.yaml write isolation. -Enforces two invariants for every run_skill step: -- write_behavior=always: output_dir must be declared unconditionally. -- write_behavior=conditional: output_dir must be declared unconditionally (same as always). +Every planner run_skill output_dir must be rooted at the planner run directory. """ from __future__ import annotations @@ -11,17 +9,11 @@ import pytest -from autoskillit.core import SKILL_TOOLS from autoskillit.core.io import load_yaml -from autoskillit.core.types import RecipeSource -from autoskillit.recipe.contracts import load_bundled_manifest, resolve_skill_name -from autoskillit.recipe.io import load_recipe -from tests._tracked_recipes import tracked_recipe_paths pytestmark = [pytest.mark.layer("recipe"), pytest.mark.medium] _RECIPE_DIR = Path(__file__).parent.parent.parent / "src" / "autoskillit" / "recipes" -_PROJECT_ROOT = Path(__file__).resolve().parent.parent.parent @pytest.fixture(scope="module") @@ -29,56 +21,6 @@ def planner_yaml() -> dict: return load_yaml(_RECIPE_DIR / "planner.yaml") -_ALL_BUNDLED_RECIPE_PATHS = sorted( - tracked_recipe_paths( - _PROJECT_ROOT, - source=RecipeSource.BUILTIN, - scan_dirs=(".",), - ) -) -assert _ALL_BUNDLED_RECIPE_PATHS - - -@pytest.mark.parametrize("recipe_yaml", _ALL_BUNDLED_RECIPE_PATHS, ids=lambda p: p.stem) -def test_write_skill_steps_have_output_dir(recipe_yaml: Path) -> None: - """Every run_skill step with write-capable behavior must declare output_dir. - - write_behavior=always: output_dir required unconditionally. - write_behavior=conditional: output_dir required unconditionally. - """ - recipe = load_recipe(recipe_yaml) - manifest = load_bundled_manifest() - skills = manifest.get("skills", {}) - - violations: list[str] = [] - for step_name, step in recipe.steps.items(): - if step.tool not in SKILL_TOOLS: - continue - skill_cmd = str((step.with_args or {}).get("skill_command", "")) - skill = resolve_skill_name(skill_cmd) - if skill is None: - continue - skill_data = skills.get(skill, {}) - write_behavior = skill_data.get("write_behavior") - output_dir = (step.with_args or {}).get("output_dir") - - if write_behavior == "always": - if not output_dir: - violations.append( - f"{step_name} ({skill}): write_behavior=always but no output_dir" - ) - elif write_behavior == "conditional": - if not output_dir: - violations.append( - f"{step_name} ({skill}): write_behavior=conditional but no output_dir" - ) - - assert not violations, ( - f"{recipe_yaml.stem}: run_skill steps missing required output_dir:\n" - + "\n".join(f" {v}" for v in violations) - ) - - def test_output_dir_is_under_planner_dir(planner_yaml: dict) -> None: """Every output_dir in a planner.yaml run_skill step must be under context.planner_dir. diff --git a/tests/recipe/test_rules_contracts.py b/tests/recipe/test_rules_contracts.py index f17ef52682..6d81bf1322 100644 --- a/tests/recipe/test_rules_contracts.py +++ b/tests/recipe/test_rules_contracts.py @@ -792,10 +792,11 @@ def _make_write_push_recipe( *, output_dir: str | None = None, routes_to_push: bool = True, + skill_command: str = "/autoskillit:resolve-review branch base", ) -> Recipe: """Minimal recipe: run_skill → push_to_remote (or stop).""" with_args: dict[str, str] = { - "skill_command": "/autoskillit:resolve-review branch base", + "skill_command": skill_command, "cwd": "/tmp", } if output_dir is not None: @@ -919,6 +920,24 @@ def test_read_only_skill_without_output_dir_not_flagged() -> None: assert not hits, "write-skill-requires-source-output-dir must not fire for read_only skills" +def test_bounded_write_skill_without_output_dir_not_flagged() -> None: + """Rule does NOT fire for a BOUNDED write skill: its temp floor is its write scope.""" + recipe = _make_write_push_recipe(output_dir=None, skill_command="/autoskillit:make-plan task") + findings = run_semantic_rules(recipe) + hits = [f for f in findings if f.rule == "write-skill-requires-source-output-dir"] + assert not hits, "write-skill-requires-source-output-dir must not fire for BOUNDED skills" + + +def test_write_skill_rule_message_names_unrestricted_scope() -> None: + recipe = _make_write_push_recipe(output_dir=None) + contract = _make_write_contract(write_behavior="conditional") + with patch.object(_rc, "get_skill_contract", return_value=contract): + findings = run_semantic_rules(recipe) + hits = [f for f in findings if f.rule == "write-skill-requires-source-output-dir"] + assert hits + assert "write_paths: unrestricted" in hits[0].message + + def test_all_bundled_recipes_pass_write_skill_output_dir_rule() -> None: """All bundled recipes must have zero write-skill-requires-source-output-dir findings.""" recipes_dir = pkg_root() / "recipes" From 8c5b33a07b94de3d0396a0adb136c957016cfd0e Mon Sep 17 00:00:00 2001 From: Trecek Date: Fri, 9 Oct 2026 21:16:04 -0700 Subject: [PATCH 05/12] fix: let vis-lens apply use each lens's own scope and synthesize its outputs vis_apply passed output_dir run-vis-lenses to vis-lens-{slug} skills, each of which may write only to its own vis-lens-/ directory, so run_skill rejected every lens dispatch; vis_synthesize then read run-vis-lenses, a directory no skill writes. vis_apply now passes no output_dir, and synthesize-vis-plan takes the captured vis_lens_output_paths list (lens_output_paths, optional) instead of a capture directory. research-design declares the capture optional because vis_apply routes failures to vis_synthesize. Co-Authored-By: Claude Opus 5.5 --- docs/phoropter/execution-contract.md | 4 ++-- .../recipe-blocks/multi-family-prefixed.yaml | 3 ++- src/autoskillit/recipe/skill_contracts.yaml | 7 ++++--- src/autoskillit/recipes/diagrams/research.md | 2 +- src/autoskillit/recipes/research-design.json | 8 +++++--- src/autoskillit/recipes/research-design.yaml | 4 ++-- src/autoskillit/recipes/research.json | 7 +++---- src/autoskillit/recipes/research.yaml | 7 +++---- .../synthesize-vis-plan/SKILL.md | 14 ++++++++------ .../test_synthesize_vis_plan_contracts.py | 4 ++-- tests/recipe/test_bundled_recipes_general.py | 3 ++- .../test_bundled_recipes_research_design.py | 19 +++++++++++++++++++ 12 files changed, 53 insertions(+), 29 deletions(-) diff --git a/docs/phoropter/execution-contract.md b/docs/phoropter/execution-contract.md index f1bacba7f9..dbb8fed56f 100644 --- a/docs/phoropter/execution-contract.md +++ b/docs/phoropter/execution-contract.md @@ -53,10 +53,10 @@ Each selected lens runs against the target, producing per-lens output files (dia ### Synthesize — Aggregation and Verdict -Reads all per-lens outputs from the capture directory and produces a unified result. The synthesis strategy (§6) determines how conflicts between lens outputs are resolved. +Reads all per-lens outputs from the capture directory (for `vis-lens`, from the captured list of lens output paths) and produces a unified result. The synthesis strategy (§6) determines how conflicts between lens outputs are resolved. - Synthesize is the only phase that emits the family's final output tokens (e.g., `verdict`, `visualization_plan_path`, `architecture_impact`). -- The synthesis skill is invoked with the capture directory containing all per-lens output files. +- The synthesis skill is invoked with the capture directory containing all per-lens output files; `vis-lens` synthesis instead receives the apply step's captured path list (`vis_lens_output_paths`). ## §4. Configuration Knobs diff --git a/docs/phoropter/recipe-blocks/multi-family-prefixed.yaml b/docs/phoropter/recipe-blocks/multi-family-prefixed.yaml index 9c77d0c9aa..48a54b0290 100644 --- a/docs/phoropter/recipe-blocks/multi-family-prefixed.yaml +++ b/docs/phoropter/recipe-blocks/multi-family-prefixed.yaml @@ -93,10 +93,11 @@ vis_synthesize: /autoskillit:synthesize-vis-plan ${{ inputs.source_dir }} ${{ context.experiment_plan }} - {{AUTOSKILLIT_TEMP}}/run-vis-lenses + "${{ context.vis_lens_output_paths }}" capture: visualization_plan_path: "${{ result.visualization_plan_path }}" report_plan_path: "${{ result.report_plan_path }}" visualization_plan_trace_path: "${{ result.visualization_plan_trace_path }}" + optional_context_refs: [vis_lens_output_paths] on_success: next_stage on_failure: escalate_stop diff --git a/src/autoskillit/recipe/skill_contracts.yaml b/src/autoskillit/recipe/skill_contracts.yaml index 18a22bbb65..80b2b5fe89 100644 --- a/src/autoskillit/recipe/skill_contracts.yaml +++ b/src/autoskillit/recipe/skill_contracts.yaml @@ -1722,9 +1722,10 @@ skills: - name: experiment_plan_path type: file_path required: true - - name: capture_dir - type: directory_path - required: true + - name: lens_output_paths + type: string + required: false + absence_value: "" - name: tier_c_lens type: string required: false diff --git a/src/autoskillit/recipes/diagrams/research.md b/src/autoskillit/recipes/diagrams/research.md index d9710a884e..ec5d52c845 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/research-design.json b/src/autoskillit/recipes/research-design.json index 73aea5741f..46ead2280e 100644 --- a/src/autoskillit/recipes/research-design.json +++ b/src/autoskillit/recipes/research-design.json @@ -263,8 +263,7 @@ "with": { "skill_command": "/autoskillit:vis-lens-{slug} {context_path} ${{ context.experiment_plan }}", "cwd": "${{ inputs.source_dir }}", - "step_name": "vis_apply", - "output_dir": "{{AUTOSKILLIT_TEMP}}/run-vis-lenses" + "step_name": "vis_apply" }, "capture_list": { "vis_lens_output_paths": "${{ result.diagram_path }}" @@ -282,7 +281,7 @@ "model": "", "phoropter_family": "vis-lens", "with": { - "skill_command": "/autoskillit:synthesize-vis-plan ${{ inputs.source_dir }} ${{ context.experiment_plan }} {{AUTOSKILLIT_TEMP}}/run-vis-lenses --tier-c-lens=${{ context.tier_c_lens }} --methodology-tradition=${{ context.methodology_tradition }} --disambiguation-rule-applied=${{ context.disambiguation_rule_applied }}", + "skill_command": "/autoskillit:synthesize-vis-plan ${{ inputs.source_dir }} ${{ context.experiment_plan }} \"${{ context.vis_lens_output_paths }}\" --tier-c-lens=${{ context.tier_c_lens }} --methodology-tradition=${{ context.methodology_tradition }} --disambiguation-rule-applied=${{ context.disambiguation_rule_applied }}", "cwd": "${{ inputs.source_dir }}", "step_name": "vis_synthesize", "output_dir": "{{AUTOSKILLIT_TEMP}}/synthesize-vis-plan" @@ -292,6 +291,9 @@ "report_plan_path": "${{ result.report_plan_path }}", "visualization_plan_trace_path": "${{ result.visualization_plan_trace_path }}" }, + "optional_context_refs": [ + "vis_lens_output_paths" + ], "on_success": "create_worktree", "on_failure": "escalate_stop" }, diff --git a/src/autoskillit/recipes/research-design.yaml b/src/autoskillit/recipes/research-design.yaml index 74e7d02e1d..b3917a7be4 100644 --- a/src/autoskillit/recipes/research-design.yaml +++ b/src/autoskillit/recipes/research-design.yaml @@ -225,7 +225,6 @@ steps: skill_command: "/autoskillit:vis-lens-{slug} {context_path} ${{ context.experiment_plan }}" cwd: "${{ inputs.source_dir }}" step_name: vis_apply - output_dir: "{{AUTOSKILLIT_TEMP}}/run-vis-lenses" capture_list: vis_lens_output_paths: "${{ result.diagram_path }}" optional_context_refs: [selected_lenses, lens_context_paths] @@ -238,7 +237,7 @@ steps: model: "" phoropter_family: vis-lens with: - skill_command: "/autoskillit:synthesize-vis-plan ${{ inputs.source_dir }} ${{ context.experiment_plan }} {{AUTOSKILLIT_TEMP}}/run-vis-lenses --tier-c-lens=${{ context.tier_c_lens }} --methodology-tradition=${{ context.methodology_tradition }} --disambiguation-rule-applied=${{ context.disambiguation_rule_applied }}" + skill_command: "/autoskillit:synthesize-vis-plan ${{ inputs.source_dir }} ${{ context.experiment_plan }} \"${{ context.vis_lens_output_paths }}\" --tier-c-lens=${{ context.tier_c_lens }} --methodology-tradition=${{ context.methodology_tradition }} --disambiguation-rule-applied=${{ context.disambiguation_rule_applied }}" cwd: "${{ inputs.source_dir }}" step_name: vis_synthesize output_dir: "{{AUTOSKILLIT_TEMP}}/synthesize-vis-plan" @@ -246,6 +245,7 @@ steps: visualization_plan_path: "${{ result.visualization_plan_path }}" report_plan_path: "${{ result.report_plan_path }}" visualization_plan_trace_path: "${{ result.visualization_plan_trace_path }}" + optional_context_refs: [vis_lens_output_paths] on_success: create_worktree on_failure: escalate_stop diff --git a/src/autoskillit/recipes/research.json b/src/autoskillit/recipes/research.json index 2f02ca3ba8..ca0353cfd6 100644 --- a/src/autoskillit/recipes/research.json +++ b/src/autoskillit/recipes/research.json @@ -303,8 +303,7 @@ "context_path": "{context_path}" }, "cwd": "${{ inputs.source_dir }}", - "step_name": "vis_apply", - "output_dir": "{{AUTOSKILLIT_TEMP}}/run-vis-lenses" + "step_name": "vis_apply" }, "capture_list": { "vis_lens_output_paths": "${{ result.diagram_path }}" @@ -312,7 +311,7 @@ "retries": 0, "on_success": "vis_synthesize", "on_failure": "escalate_stop", - "note": "LENS ITERATION: context.selected_lenses contains comma-separated vis-lens slugs (e.g. \"always-on,chart-select\"). context.lens_context_paths contains the corresponding comma-separated context file paths (same order as slugs). For each lens slug, run:\n /autoskillit:vis-lens-{slug} {matching_context_path}\nRun all selected lenses in parallel (all run_skill calls in a single message). Accumulate each diagram_path value into context.vis_lens_output_paths via capture_list. If a lens fails, continue with remaining lenses.\n" + "note": "LENS ITERATION: context.selected_lenses contains comma-separated vis-lens slugs (e.g. \"always-on,chart-select\"). context.lens_context_paths contains the corresponding comma-separated context file paths (same order as slugs). For each lens slug, run:\n /autoskillit:vis-lens-{slug} {matching_context_path}\nRun all selected lenses in parallel (all run_skill calls in a single message). Accumulate each diagram_path value into context.vis_lens_output_paths using comma separation via capture_list. If a lens fails, continue with remaining lenses.\n" }, "vis_synthesize": { "tool": "run_skill", @@ -326,7 +325,7 @@ "experiment_plan_path": "${{ context.experiment_plan }}", "tier_c_lens": "${{ context.tier_c_lens }}", "source_dir": "${{ inputs.source_dir }}", - "capture_dir": "{{AUTOSKILLIT_TEMP}}/run-vis-lenses" + "lens_output_paths": "${{ context.vis_lens_output_paths }}" }, "cwd": "${{ inputs.source_dir }}", "step_name": "vis_synthesize", diff --git a/src/autoskillit/recipes/research.yaml b/src/autoskillit/recipes/research.yaml index 989cf476b6..43eddc2930 100644 --- a/src/autoskillit/recipes/research.yaml +++ b/src/autoskillit/recipes/research.yaml @@ -287,7 +287,6 @@ steps: context_path: '{context_path}' cwd: ${{ inputs.source_dir }} step_name: vis_apply - output_dir: '{{AUTOSKILLIT_TEMP}}/run-vis-lenses' capture_list: vis_lens_output_paths: ${{ result.diagram_path }} retries: 0 @@ -296,8 +295,8 @@ steps: note: "LENS ITERATION: context.selected_lenses contains comma-separated vis-lens slugs (e.g. \"always-on,chart-select\"\ ). context.lens_context_paths contains the corresponding comma-separated context file paths (same order as slugs). For\ \ each lens slug, run:\n /autoskillit:vis-lens-{slug} {matching_context_path}\nRun all selected lenses in parallel\ - \ (all run_skill calls in a single message). Accumulate each diagram_path value into context.vis_lens_output_paths via\ - \ capture_list. If a lens fails, continue with remaining lenses.\n" + \ (all run_skill calls in a single message). Accumulate each diagram_path value into context.vis_lens_output_paths using\ + \ comma separation via capture_list. If a lens fails, continue with remaining lenses.\n" vis_synthesize: tool: run_skill model: '' @@ -310,7 +309,7 @@ steps: experiment_plan_path: ${{ context.experiment_plan }} tier_c_lens: ${{ context.tier_c_lens }} source_dir: ${{ inputs.source_dir }} - capture_dir: '{{AUTOSKILLIT_TEMP}}/run-vis-lenses' + lens_output_paths: ${{ context.vis_lens_output_paths }} cwd: ${{ inputs.source_dir }} step_name: vis_synthesize output_dir: '{{AUTOSKILLIT_TEMP}}/synthesize-vis-plan' diff --git a/src/autoskillit/skills_extended/synthesize-vis-plan/SKILL.md b/src/autoskillit/skills_extended/synthesize-vis-plan/SKILL.md index fd8345d41b..fbf74abbfc 100644 --- a/src/autoskillit/skills_extended/synthesize-vis-plan/SKILL.md +++ b/src/autoskillit/skills_extended/synthesize-vis-plan/SKILL.md @@ -41,15 +41,16 @@ hierarchy and token-based input, use `phoropter-priority-synthesis` instead. ## Arguments ``` -/autoskillit:synthesize-vis-plan {source_dir} {experiment_plan_path} {capture_dir} --tier-c-lens={tier_c_lens} --methodology-tradition={methodology_tradition} --disambiguation-rule-applied={disambiguation_rule_applied} --applied-union-rules={applied_union_rules} --precedence-trace={precedence_trace} +/autoskillit:synthesize-vis-plan {source_dir} {experiment_plan_path} {lens_output_paths} --tier-c-lens={tier_c_lens} --methodology-tradition={methodology_tradition} --disambiguation-rule-applied={disambiguation_rule_applied} --applied-union-rules={applied_union_rules} --precedence-trace={precedence_trace} ``` **Positional arguments:** - `{source_dir}` — Absolute path to the source repo (the CWD before worktree creation) - `{experiment_plan_path}` — Absolute path to the finalized experiment plan markdown -- `{capture_dir}` — Absolute path to the directory containing lens output files (vis-lens - markdown files with `yaml:figure-spec` fenced blocks) +- `{lens_output_paths}` — Comma-separated absolute paths of this run's lens output files + (`vis_lens_output_paths`: vis-lens markdown files with `yaml:figure-spec` fenced blocks); + may be empty **Tier-C routing fields** (named arguments, passed from the `select-vis-lenses` context): @@ -94,7 +95,7 @@ hierarchy and token-based input, use `phoropter-priority-synthesis` instead. Extract positional arguments: - `source_dir` — the source repository path - `experiment_plan_path` — path to the experiment plan -- `capture_dir` — directory containing lens output files +- `lens_output_paths` — comma-separated lens output file paths; may be empty Extract Tier-C routing fields from named arguments: - `tier_c_lens` @@ -105,7 +106,7 @@ Extract Tier-C routing fields from named arguments: - `precedence_trace` — sourced from the methodology-norms context file passed as arguments from `select-vis-lenses`, not from recipe context -Read all files matching `capture_dir/**/*.md` and extract `yaml:figure-spec` fenced +Read each file listed in `lens_output_paths` and extract `yaml:figure-spec` fenced blocks from each file. Parse each block as YAML to produce a list of figure specification objects. @@ -122,7 +123,8 @@ chart_type: grouped_bar **Empty plan handling:** If no `yaml:figure-spec` blocks are found in any lens output file, record zero figures and proceed to Step 1 with an empty figure list. All three -output files are still written (with empty tables). +output files are still written (with empty tables). An empty `lens_output_paths`, or a +listed file that cannot be read, contributes zero figures. ### Step 1 — Resolve Conflicts diff --git a/tests/contracts/test_synthesize_vis_plan_contracts.py b/tests/contracts/test_synthesize_vis_plan_contracts.py index e5349010bc..45a9ac570f 100644 --- a/tests/contracts/test_synthesize_vis_plan_contracts.py +++ b/tests/contracts/test_synthesize_vis_plan_contracts.py @@ -64,7 +64,7 @@ def test_frontmatter_description_mentions_synthesize() -> None: def test_arguments_positional_args() -> None: text = _text() - for arg in ("source_dir", "experiment_plan_path", "capture_dir"): + for arg in ("source_dir", "experiment_plan_path", "lens_output_paths"): assert arg in text, f"Arguments must document {arg}" @@ -130,7 +130,7 @@ def test_three_output_tokens() -> None: def test_yaml_figure_spec_parsing() -> None: text = _text() assert "yaml:figure-spec" in text - assert "capture_dir" in text + assert "lens_output_paths" in text def test_visualization_plan_content_structure() -> None: diff --git a/tests/recipe/test_bundled_recipes_general.py b/tests/recipe/test_bundled_recipes_general.py index 3753625ed6..76d7ef9a1b 100644 --- a/tests/recipe/test_bundled_recipes_general.py +++ b/tests/recipe/test_bundled_recipes_general.py @@ -95,6 +95,7 @@ def test_optional_context_structured_skill_input_inventory_is_explicit() -> None ("resolve-failures", "diagnosis_path"), ("resolve-review", "mode"), ("review-pr", "mode"), + ("synthesize-vis-plan", "lens_output_paths"), } manifest = load_bundled_manifest() occurrences: list[tuple[str, str, str, str]] = [] @@ -138,7 +139,7 @@ def test_optional_context_structured_skill_input_inventory_is_explicit() -> None for recipe, step, skill, input_name in occurrences if (recipe, step, skill, input_name) != standalone_optional } - assert len(occurrences) == 111 + assert len(occurrences) == 112 assert actual_pairs == expected_pairs assert not required_occurrences for skill_name, input_name in expected_pairs: diff --git a/tests/recipe/test_bundled_recipes_research_design.py b/tests/recipe/test_bundled_recipes_research_design.py index 364cb5f2fd..d96b4d23ee 100644 --- a/tests/recipe/test_bundled_recipes_research_design.py +++ b/tests/recipe/test_bundled_recipes_research_design.py @@ -419,3 +419,22 @@ def test_kitchen_rule_food_truck_sentinel(self, recipe) -> None: rule = recipe.kitchen_rules[1].lower() assert "food truck" in rule, "Kitchen rule 2 must mention food truck" assert "sentinel" in rule, "Kitchen rule 2 must mention sentinel emission" + + +@pytest.mark.parametrize("recipe_name", ["research", "research-design"]) +def test_vis_synthesize_consumes_vis_apply_outputs(recipe_name: str) -> None: + from autoskillit.core import BoundValueOrigin + from autoskillit.recipe._analysis import make_validation_context + + recipe = load_recipe(builtin_recipes_dir() / f"{recipe_name}.yaml") + apply_args = recipe.steps["vis_apply"].with_args + synthesize_args = recipe.steps["vis_synthesize"].with_args + + assert "output_dir" not in apply_args + binding = make_validation_context(recipe).binding_projection.for_step("vis_synthesize") + assert binding is not None + lens_outputs = binding.skill_input("lens_output_paths") + assert lens_outputs is not None + assert lens_outputs.origin is BoundValueOrigin.CONTEXT + assert lens_outputs.context_dependencies == ("vis_lens_output_paths",) + assert "run-vis-lenses" not in repr((apply_args, synthesize_args)) From bf8e0bd6e7ab51d31817827b462673bf5bc370b2 Mon Sep 17 00:00:00 2001 From: Trecek Date: Fri, 9 Oct 2026 21:18:28 -0700 Subject: [PATCH 06/12] fix: stop widening BOUNDED planning and PR-prep dispatches to the worktree Seventeen bundled steps passed output_dir: . to make-plan, prepare-pr, prepare-research-pr and compose-research-pr. Those skills declare BOUNDED write scopes, so run_skill rejects a worktree-root output_dir for them. With output_dir omitted, the default floor /.autoskillit/temp/ equals each skill's declared scope; captures, plan placement and attestation are unchanged. Regenerates the compiled recipes, the diagram recipe hashes and the pinned pre-delivery structure digests. Co-Authored-By: Claude Opus 5.5 --- .../recipes/diagrams/implementation-groups.md | 2 +- src/autoskillit/recipes/diagrams/implementation.md | 2 +- src/autoskillit/recipes/diagrams/merge-prs.md | 2 +- src/autoskillit/recipes/diagrams/remediation.md | 2 +- src/autoskillit/recipes/diagrams/research.md | 2 +- src/autoskillit/recipes/implementation-groups.json | 7 ++----- src/autoskillit/recipes/implementation-groups.yaml | 3 --- src/autoskillit/recipes/implementation.json | 4 +--- src/autoskillit/recipes/implementation.yaml | 2 -- src/autoskillit/recipes/merge-prs.json | 8 ++------ src/autoskillit/recipes/merge-prs.yaml | 4 ---- src/autoskillit/recipes/remediation.json | 4 +--- src/autoskillit/recipes/remediation.yaml | 2 -- src/autoskillit/recipes/research-implement.json | 3 +-- src/autoskillit/recipes/research-implement.yaml | 1 - src/autoskillit/recipes/research-review.json | 6 ++---- src/autoskillit/recipes/research-review.yaml | 2 -- src/autoskillit/recipes/research.json | 9 +++------ src/autoskillit/recipes/research.yaml | 3 --- tests/recipe/test_implementation.py | 2 +- tests/recipe/test_remediation_recipe.py | 2 +- 21 files changed, 19 insertions(+), 53 deletions(-) diff --git a/src/autoskillit/recipes/diagrams/implementation-groups.md b/src/autoskillit/recipes/diagrams/implementation-groups.md index 593acd2f99..7dad121550 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 fff80796b5..52cd41d81c 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 968f6d25ae..ad26f866bb 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. diff --git a/src/autoskillit/recipes/diagrams/remediation.md b/src/autoskillit/recipes/diagrams/remediation.md index eaff5649ca..7a3ad79f51 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 ec5d52c845..9463481686 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/implementation-groups.json b/src/autoskillit/recipes/implementation-groups.json index 0b1bb2f85d..cab0d3507b 100644 --- a/src/autoskillit/recipes/implementation-groups.json +++ b/src/autoskillit/recipes/implementation-groups.json @@ -232,8 +232,7 @@ "audit_cycle_path": "${{ context.audit_cycle_path }}" }, "cwd": "${{ context.work_dir }}", - "step_name": "plan", - "output_dir": "." + "step_name": "plan" }, "capture": { "plan_path": "${{ result.plan_path }}", @@ -427,8 +426,7 @@ "audit_cycle_path": "${{ context.audit_cycle_path }}" }, "cwd": "${{ context.work_dir }}", - "step_name": "gap_replan", - "output_dir": "." + "step_name": "gap_replan" }, "capture": { "plan_path": "${{ result.plan_path }}", @@ -1414,7 +1412,6 @@ "run_name": "${{ inputs.run_name }}" }, "cwd": "${{ context.work_dir }}", - "output_dir": ".", "step_name": "prepare_pr" }, "capture": { diff --git a/src/autoskillit/recipes/implementation-groups.yaml b/src/autoskillit/recipes/implementation-groups.yaml index f7eb3ea207..925411eea1 100644 --- a/src/autoskillit/recipes/implementation-groups.yaml +++ b/src/autoskillit/recipes/implementation-groups.yaml @@ -238,7 +238,6 @@ steps: audit_cycle_path: ${{ context.audit_cycle_path }} cwd: ${{ context.work_dir }} step_name: plan - output_dir: . capture: plan_path: ${{ result.plan_path }} all_plan_paths: ${{ result.plan_path }} @@ -394,7 +393,6 @@ steps: audit_cycle_path: ${{ context.audit_cycle_path }} cwd: ${{ context.work_dir }} step_name: gap_replan - output_dir: . capture: plan_path: ${{ result.plan_path }} plan_disposition_path: ${{ result.plan_disposition_path }} @@ -1163,7 +1161,6 @@ steps: base_branch: ${{ inputs.base_branch }} run_name: ${{ inputs.run_name }} cwd: ${{ context.work_dir }} - output_dir: . step_name: prepare_pr capture: prep_path: ${{ result.prep_path }} diff --git a/src/autoskillit/recipes/implementation.json b/src/autoskillit/recipes/implementation.json index 9cd7a68fef..0847f38d44 100644 --- a/src/autoskillit/recipes/implementation.json +++ b/src/autoskillit/recipes/implementation.json @@ -499,8 +499,7 @@ "plan_set_coverage_gaps": "${{ context.plan_set_coverage_gaps }}" }, "cwd": "${{ context.work_dir }}", - "step_name": "plan", - "output_dir": "." + "step_name": "plan" }, "capture": { "plan_path": "${{ result.plan_path }}", @@ -1643,7 +1642,6 @@ "run_name": "${{ inputs.run_name }}" }, "cwd": "${{ context.work_dir }}", - "output_dir": ".", "step_name": "prepare_pr" }, "capture": { diff --git a/src/autoskillit/recipes/implementation.yaml b/src/autoskillit/recipes/implementation.yaml index dfd04a8825..492628cce9 100644 --- a/src/autoskillit/recipes/implementation.yaml +++ b/src/autoskillit/recipes/implementation.yaml @@ -434,7 +434,6 @@ steps: plan_set_coverage_gaps: ${{ context.plan_set_coverage_gaps }} cwd: ${{ context.work_dir }} step_name: plan - output_dir: . capture: plan_path: ${{ result.plan_path }} all_plan_paths: ${{ result.plan_path }} @@ -1331,7 +1330,6 @@ steps: base_branch: ${{ inputs.base_branch }} run_name: ${{ inputs.run_name }} cwd: ${{ context.work_dir }} - output_dir: . step_name: prepare_pr capture: prep_path: ${{ result.prep_path }} diff --git a/src/autoskillit/recipes/merge-prs.json b/src/autoskillit/recipes/merge-prs.json index 7a1ff9b674..08de98b8b8 100644 --- a/src/autoskillit/recipes/merge-prs.json +++ b/src/autoskillit/recipes/merge-prs.json @@ -470,7 +470,6 @@ "audit_cycle_path": "${{ context.audit_cycle_path }}" }, "cwd": "${{ context.work_dir }}", - "output_dir": ".", "step_name": "plan_ejected_rebase_conflicts" }, "capture": { @@ -848,7 +847,6 @@ "audit_cycle_path": "${{ context.audit_cycle_path }}" }, "cwd": "${{ context.work_dir }}", - "output_dir": ".", "step_name": "plan_proactive_rebase_conflicts" }, "capture": { @@ -1065,8 +1063,7 @@ "audit_cycle_path": "${{ context.audit_cycle_path }}" }, "cwd": "${{ context.work_dir }}", - "step_name": "plan", - "output_dir": "." + "step_name": "plan" }, "capture": { "pr_plan_path": "${{ result.plan_path }}", @@ -1806,8 +1803,7 @@ "audit_cycle_path": "${{ context.audit_cycle_path }}" }, "cwd": "${{ context.work_dir }}", - "step_name": "plan_remediation", - "output_dir": "." + "step_name": "plan_remediation" }, "capture": { "pr_plan_path": "${{ result.plan_path }}", diff --git a/src/autoskillit/recipes/merge-prs.yaml b/src/autoskillit/recipes/merge-prs.yaml index 7c0973e58a..5bdddb152f 100644 --- a/src/autoskillit/recipes/merge-prs.yaml +++ b/src/autoskillit/recipes/merge-prs.yaml @@ -448,7 +448,6 @@ steps: task: ${{ context.ejected_rebase_conflict_report_path }} audit_cycle_path: ${{ context.audit_cycle_path }} cwd: ${{ context.work_dir }} - output_dir: . step_name: plan_ejected_rebase_conflicts capture: ejected_rebase_plan_path: ${{ result.plan_path }} @@ -735,7 +734,6 @@ steps: task: ${{ context.proactive_rebase_conflict_report_path }} audit_cycle_path: ${{ context.audit_cycle_path }} cwd: ${{ context.work_dir }} - output_dir: . step_name: plan_proactive_rebase_conflicts capture: proactive_rebase_plan_path: ${{ result.plan_path }} @@ -926,7 +924,6 @@ steps: audit_cycle_path: ${{ context.audit_cycle_path }} cwd: ${{ context.work_dir }} step_name: plan - output_dir: . capture: pr_plan_path: ${{ result.plan_path }} verdict: ${{ result.verdict }} @@ -1554,7 +1551,6 @@ steps: audit_cycle_path: ${{ context.audit_cycle_path }} cwd: ${{ context.work_dir }} step_name: plan_remediation - output_dir: . capture: pr_plan_path: ${{ result.plan_path }} verdict: ${{ result.verdict }} diff --git a/src/autoskillit/recipes/remediation.json b/src/autoskillit/recipes/remediation.json index cada4021a0..75d3fc48f8 100644 --- a/src/autoskillit/recipes/remediation.json +++ b/src/autoskillit/recipes/remediation.json @@ -1924,8 +1924,7 @@ "plan_set_coverage_gaps": "${{ context.plan_set_coverage_gaps }}" }, "cwd": "${{ context.work_dir }}", - "step_name": "make_plan", - "output_dir": "." + "step_name": "make_plan" }, "capture": { "plan_path": "${{ result.plan_path }}", @@ -2176,7 +2175,6 @@ "run_name": "${{ inputs.run_name }}" }, "cwd": "${{ context.work_dir }}", - "output_dir": ".", "step_name": "prepare_pr" }, "capture": { diff --git a/src/autoskillit/recipes/remediation.yaml b/src/autoskillit/recipes/remediation.yaml index cccbec4757..9d7dfb829b 100644 --- a/src/autoskillit/recipes/remediation.yaml +++ b/src/autoskillit/recipes/remediation.yaml @@ -1475,7 +1475,6 @@ steps: plan_set_coverage_gaps: ${{ context.plan_set_coverage_gaps }} cwd: ${{ context.work_dir }} step_name: make_plan - output_dir: . capture: plan_path: ${{ result.plan_path }} verdict: ${{ result.verdict }} @@ -1657,7 +1656,6 @@ steps: base_branch: ${{ inputs.base_branch }} run_name: ${{ inputs.run_name }} cwd: ${{ context.work_dir }} - output_dir: . step_name: prepare_pr capture: prep_path: ${{ result.prep_path }} diff --git a/src/autoskillit/recipes/research-implement.json b/src/autoskillit/recipes/research-implement.json index 615187d304..d6faf2f411 100644 --- a/src/autoskillit/recipes/research-implement.json +++ b/src/autoskillit/recipes/research-implement.json @@ -185,8 +185,7 @@ "audit_cycle_path": "${{ context.audit_cycle_path }}" }, "cwd": "${{ inputs.worktree_path }}", - "step_name": "plan_phase", - "output_dir": "." + "step_name": "plan_phase" }, "capture": { "phase_plan_path": "${{ result.plan_path }}", diff --git a/src/autoskillit/recipes/research-implement.yaml b/src/autoskillit/recipes/research-implement.yaml index 4a416857ec..8ca237853f 100644 --- a/src/autoskillit/recipes/research-implement.yaml +++ b/src/autoskillit/recipes/research-implement.yaml @@ -187,7 +187,6 @@ steps: audit_cycle_path: ${{ context.audit_cycle_path }} cwd: ${{ inputs.worktree_path }} step_name: plan_phase - output_dir: . capture: phase_plan_path: ${{ result.plan_path }} verdict: ${{ result.verdict }} diff --git a/src/autoskillit/recipes/research-review.json b/src/autoskillit/recipes/research-review.json index c849d394c9..ffa2efa959 100644 --- a/src/autoskillit/recipes/research-review.json +++ b/src/autoskillit/recipes/research-review.json @@ -96,8 +96,7 @@ "with": { "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": "." + "step_name": "prepare_research_pr" }, "capture": { "prep_path": "${{ result.prep_path }}", @@ -165,8 +164,7 @@ "with": { "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": "." + "step_name": "compose_research_pr" }, "capture": { "pr_url": { diff --git a/src/autoskillit/recipes/research-review.yaml b/src/autoskillit/recipes/research-review.yaml index b1a9429133..9ceaa9c749 100644 --- a/src/autoskillit/recipes/research-review.yaml +++ b/src/autoskillit/recipes/research-review.yaml @@ -101,7 +101,6 @@ steps: }} ${{ inputs.base_branch }} cwd: ${{ inputs.worktree_path }} step_name: prepare_research_pr - output_dir: . capture: prep_path: ${{ result.prep_path }} selected_lenses: ${{ result.selected_lenses }} @@ -166,7 +165,6 @@ steps: }} ${{ inputs.base_branch }} cwd: ${{ inputs.worktree_path }} step_name: compose_research_pr - output_dir: . capture: pr_url: from: ${{ result.pr_url }} diff --git a/src/autoskillit/recipes/research.json b/src/autoskillit/recipes/research.json index ca0353cfd6..ae4cc24d7e 100644 --- a/src/autoskillit/recipes/research.json +++ b/src/autoskillit/recipes/research.json @@ -561,8 +561,7 @@ "audit_cycle_path": "${{ context.audit_cycle_path }}" }, "cwd": "${{ context.worktree_path }}", - "step_name": "plan_phase", - "output_dir": "." + "step_name": "plan_phase" }, "capture": { "phase_plan_path": "${{ result.plan_path }}", @@ -1342,8 +1341,7 @@ "experiment_plan_path": "${{ context.experiment_plan }}" }, "cwd": "${{ context.worktree_path }}", - "step_name": "prepare_research_pr", - "output_dir": "." + "step_name": "prepare_research_pr" }, "capture": { "prep_path": "${{ result.prep_path }}", @@ -1422,8 +1420,7 @@ "prep_path": "${{ context.prep_path }}" }, "cwd": "${{ context.worktree_path }}", - "step_name": "compose_research_pr", - "output_dir": "." + "step_name": "compose_research_pr" }, "capture": { "pr_url": { diff --git a/src/autoskillit/recipes/research.yaml b/src/autoskillit/recipes/research.yaml index 43eddc2930..4920ddd21c 100644 --- a/src/autoskillit/recipes/research.yaml +++ b/src/autoskillit/recipes/research.yaml @@ -528,7 +528,6 @@ steps: audit_cycle_path: ${{ context.audit_cycle_path }} cwd: ${{ context.worktree_path }} step_name: plan_phase - output_dir: . capture: phase_plan_path: ${{ result.plan_path }} verdict: ${{ result.verdict }} @@ -1135,7 +1134,6 @@ steps: experiment_plan_path: ${{ context.experiment_plan }} cwd: ${{ context.worktree_path }} step_name: prepare_research_pr - output_dir: . capture: prep_path: ${{ result.prep_path }} selected_lenses: ${{ result.selected_lenses }} @@ -1210,7 +1208,6 @@ steps: prep_path: ${{ context.prep_path }} cwd: ${{ context.worktree_path }} step_name: compose_research_pr - output_dir: . capture: pr_url: from: ${{ result.pr_url }} diff --git a/tests/recipe/test_implementation.py b/tests/recipe/test_implementation.py index a3ec6e5804..3e8f4c2f01 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:50b3afcb1c897d1e7d0b592813911f75385edb80b83f96d31d00d81957b2f666" + "sha256:03057194113eef6e9902d5b26ed974c8da839a66f17aa8b20f70b8f8ebdbb375" ) _CHECKPOINT_HANDLER_TABLE = { diff --git a/tests/recipe/test_remediation_recipe.py b/tests/recipe/test_remediation_recipe.py index 1fbde40a44..cfd31877e6 100644 --- a/tests/recipe/test_remediation_recipe.py +++ b/tests/recipe/test_remediation_recipe.py @@ -22,7 +22,7 @@ Path(__file__).parent.parent.parent / "src" / "autoskillit" / "recipes" / "remediation.yaml" ) _PRE_DELIVERY_STRUCTURE_SHA256 = ( - "sha256:7e1b5010bdb3ce0b486543d5693c160cb062edb96c1cfbc6405615f16a4a75d1" + "sha256:ecd6705d9e08441738c914d0853a975c292bdd341e1bf7957e35256d4ca11777" ) From fe252f0e99da7aa0e9c6067f23e09299893bec7b Mon Sep 17 00:00:00 2001 From: Trecek Date: Fri, 9 Oct 2026 21:20:24 -0700 Subject: [PATCH 07/12] fix: judge worktree-root output_dir values in the write-path alignment rule skill-write-path-recipe-alignment pre-filtered '.', '${{ context.work_dir }}' and slash-only values before its boundary check, so it stayed silent on the exact dispatches run_skill rejects. The rule now resolves each value to the path run_skill will judge -- '.', slash-only, or a value rooted at the step's cwd -- and decides through output_dir_widens_scope, reporting the runtime's own diagnostic. Only statically undecidable values (another template root, an absolute path) stay silent. bounded_scope_contains is registered as a single-enforcement-point policy function: output_dir_widens_scope is now its only caller. Co-Authored-By: Claude Opus 5.5 --- scripts/check_single_enforcement_point.py | 1 + .../rules/rules_skill_write_path_alignment.py | 91 ++++++++++++------ .../test_rules_skill_write_path_alignment.py | 95 +++++++++++++++++-- 3 files changed, 149 insertions(+), 38 deletions(-) diff --git a/scripts/check_single_enforcement_point.py b/scripts/check_single_enforcement_point.py index 36ffd4cd24..a3c13eb7bc 100644 --- a/scripts/check_single_enforcement_point.py +++ b/scripts/check_single_enforcement_point.py @@ -44,6 +44,7 @@ ("_interactive_invocation_environment_policy", "execution/backends/_claude/environment.py"), ("admit_hook_session_scope", "hooks/_runtime/_hook_settings.py"), ("admit_tool_session_scope", "server/lifecycle/_session_scope.py"), + ("bounded_scope_contains", "hooks/_write_scope.py"), ) #: Policy functions permitted exactly two call sites, iff the second site is diff --git a/src/autoskillit/recipe/rules/rules_skill_write_path_alignment.py b/src/autoskillit/recipe/rules/rules_skill_write_path_alignment.py index 6812c0f80a..12426246d7 100644 --- a/src/autoskillit/recipe/rules/rules_skill_write_path_alignment.py +++ b/src/autoskillit/recipe/rules/rules_skill_write_path_alignment.py @@ -1,7 +1,11 @@ """Semantic rule: SKILL.md declared write scope must align with recipe output_dir. -A BOUNDED write scope bounds a recipe output_dir; UNRESTRICTED and INHERIT scopes -do not narrow it, and a skill without a valid declaration cannot be verified. An +A recipe output_dir is resolved to the path run_skill will judge -- ``.``, a +slash-only value, or a value rooted at the step's ``cwd`` -- and checked with the +boundary predicate run_skill admission uses: it must stay inside a BOUNDED write +scope, while UNRESTRICTED and INHERIT scopes do not narrow it. A value whose runtime +path is statically unknown (another template root, or an absolute path) is not +judged, and a skill without a valid declaration cannot be verified. An iteration-scoped output_dir also requires the skill's prose write instructions to use the dynamic write prefix so the agent can target the narrowed directory. """ @@ -9,12 +13,15 @@ from __future__ import annotations from pathlib import Path -from typing import assert_never import regex as re from autoskillit.core import Severity -from autoskillit.hooks._write_scope import WriteScope, WriteScopeKind, bounded_scope_contains +from autoskillit.hooks._write_scope import ( + WriteScope, + output_dir_widens_scope, + widened_output_dir_message, +) from autoskillit.recipe._analysis import ValidationContext from autoskillit.recipe._skill_helpers import skill_write_scope from autoskillit.recipe._skill_placeholder_parser import ( @@ -29,6 +36,7 @@ _AUTOSKILLIT_TEMP_RE = re.compile(r"\{\{AUTOSKILLIT_TEMP\}\}") _TEMP_DIR_SUFFIX = "autoskillit/temp" _AUTOSKILLIT_TEMP_LITERAL_RE = re.compile(rf"\.{_TEMP_DIR_SUFFIX}(?=/|$)") +_TEMPLATE_SPACING_RE = re.compile(r"\$\{\{\s*(.*?)\s*\}\}") def _static_base_prefix(output_dir: str) -> str: @@ -75,41 +83,60 @@ def _first_misaligned_write_path(content: str, output_dir: str) -> str | None: return None +def _normalise_templates(value: str) -> str: + return _TEMPLATE_SPACING_RE.sub(r"${{ \1 }}", value) + + +def _strip_cwd_prefix(output_dir: str, cwd: str) -> str: + base = _normalise_templates(cwd) + if base and (output_dir == base or output_dir.startswith(base + "/")): + return "." + output_dir[len(base) :] + return output_dir + + +def _static_output_dir(output_dir: str, cwd: str) -> str | None: + """Return the path run_skill will judge for ``output_dir``, or None when undecidable.""" + value = _normalise_templates(output_dir) + if value.strip("/") == "": + return "/" + value = _strip_cwd_prefix(value, cwd) + if Path(value).is_absolute(): + return None + if "${{" not in value: + return value + static_base = _static_base_prefix(value) + if static_base in ("", ".") or "${{" in static_base: + return None + return static_base + + def _declared_boundary_error( - write_scope: WriteScope | None, output_dir: str, project_dir: Path + skill_name: str, + write_scope: WriteScope | None, + static_path: str | None, + output_dir: str, + project_dir: Path, ) -> str | None: - static_base = _static_base_prefix(output_dir) - if not static_base or "${{" in static_base: + if static_path is None: return None if write_scope is None: return "cannot verify output_dir: skill lacks a valid write_paths declaration" - match write_scope.kind: - case WriteScopeKind.BOUNDED: - if bounded_scope_contains(write_scope, static_base, str(project_dir)): - return None - return f"recipe output_dir {output_dir!r} is outside the skill's declared write scope" - case WriteScopeKind.UNRESTRICTED | WriteScopeKind.INHERIT: - return None - case _ as unreachable: - assert_never(unreachable) - - -def _eligible_skill_path(step: RecipeStep) -> tuple[str, str] | None: + if output_dir_widens_scope(write_scope, static_path, str(project_dir)): + return widened_output_dir_message(skill_name, write_scope, output_dir) + return None + + +def _eligible_skill_path(step: RecipeStep) -> tuple[str, str, str] | None: if step.tool != "run_skill": return None - output_dir = (step.with_args or {}).get("output_dir", "") or "" + with_args = step.with_args or {} + output_dir = with_args.get("output_dir", "") or "" if not output_dir: return None - # Whole-worktree and work_dir destinations do not narrow the skill's write scope. - if output_dir in (".", "${{ context.work_dir }}") or output_dir.strip("/") == "": - return None - skill_cmd = (step.with_args or {}).get("skill_command", "") or "" - if not skill_cmd: - return None - skill_name = resolve_skill_name(skill_cmd) + skill_name = resolve_skill_name(with_args.get("skill_command", "") or "") if skill_name is None: return None - return output_dir, skill_name + return output_dir, with_args.get("cwd", "") or "", skill_name @semantic_rule( @@ -129,14 +156,18 @@ def _check_skill_write_path_alignment(ctx: ValidationContext) -> list[RuleFindin eligible = _eligible_skill_path(step) if eligible is None: continue - output_dir, skill_name = eligible + output_dir, cwd, skill_name = eligible resolved = skill_write_scope(ctx, skill_name) if resolved is None: continue skill_md_path, content, write_scope = resolved boundary_error = _declared_boundary_error( - write_scope, output_dir, ctx.project_dir or skill_md_path.parent.parent + skill_name, + write_scope, + _static_output_dir(output_dir, cwd), + output_dir, + ctx.project_dir or skill_md_path.parent.parent, ) if boundary_error is not None: findings.append( diff --git a/tests/recipe/test_rules_skill_write_path_alignment.py b/tests/recipe/test_rules_skill_write_path_alignment.py index 9adfd0f730..9b01045868 100644 --- a/tests/recipe/test_rules_skill_write_path_alignment.py +++ b/tests/recipe/test_rules_skill_write_path_alignment.py @@ -19,11 +19,13 @@ pytestmark = [pytest.mark.layer("recipe"), pytest.mark.small] _RULE_NAME = "skill-write-path-recipe-alignment" +_SCOPED_BOUNDED = "write_paths: ['{{AUTOSKILLIT_TEMP}}/scoped/']\n" _PROJECT_ROOT = Path(__file__).resolve().parent.parent.parent -def _make_recipe_yaml(skill_name: str, output_dir: str) -> str: +def _make_recipe_yaml(skill_name: str, output_dir: str, *, cwd: str = "") -> str: + cwd_line = f'\n cwd: "{cwd}"' if cwd else "" return textwrap.dedent( f"""\ name: test-{skill_name} @@ -34,7 +36,7 @@ def _make_recipe_yaml(skill_name: str, output_dir: str) -> str: tool: run_skill with: skill_command: "/autoskillit:{skill_name} branch" - output_dir: "{output_dir}" + output_dir: "{output_dir}"{cwd_line} on_success: done done: tool: run_cmd @@ -233,7 +235,9 @@ def test_rule_fires_on_synthetic_pre_fix_divergence(tmp_path: Path) -> None: assert "NEVER block declares write scope 'review-pr/'" in rule_findings[0].message -def _alignment_findings(tmp_path: Path, frontmatter: str, output_dir: str) -> list[str]: +def _alignment_findings( + tmp_path: Path, frontmatter: str, output_dir: str, *, cwd: str = "" +) -> list[str]: skill_dir = tmp_path / "scoped" skill_dir.mkdir() (skill_dir / "SKILL.md").write_text( @@ -241,7 +245,7 @@ def _alignment_findings(tmp_path: Path, frontmatter: str, output_dir: str) -> li encoding="utf-8", ) recipe_path = tmp_path / "recipe.yaml" - recipe_path.write_text(_make_recipe_yaml("scoped", output_dir), encoding="utf-8") + recipe_path.write_text(_make_recipe_yaml("scoped", output_dir, cwd=cwd), encoding="utf-8") with patch.object(_sh, "SKILL_SEARCH_DIRS", [tmp_path]): findings = run_semantic_rules(load_recipe(recipe_path)) return [finding.message for finding in findings if finding.rule == _RULE_NAME] @@ -263,12 +267,58 @@ def test_unbounded_scopes_do_not_narrow_output_dir(tmp_path: Path, frontmatter: def test_bounded_scope_violation_names_the_declared_write_scope(tmp_path: Path) -> None: - messages = _alignment_findings( - tmp_path, "write_paths: ['{{AUTOSKILLIT_TEMP}}/scoped/']\n", "{{AUTOSKILLIT_TEMP}}/other/" - ) + messages = _alignment_findings(tmp_path, _SCOPED_BOUNDED, "{{AUTOSKILLIT_TEMP}}/other/") + + assert len(messages) == 1 + assert "is outside the declared write scope of skill" in messages[0] + + +_WORKTREE_ROOT_OUTPUT_DIRS = [ + (".", ""), + ("/", ""), + ("${{ context.work_dir }}", "${{ context.work_dir }}"), + ("${{context.work_dir}}/.autoskillit/temp", "${{ context.work_dir }}"), +] + + +@pytest.mark.parametrize(("output_dir", "cwd"), _WORKTREE_ROOT_OUTPUT_DIRS) +def test_worktree_root_output_dir_fires_for_bounded_skill( + tmp_path: Path, output_dir: str, cwd: str +) -> None: + messages = _alignment_findings(tmp_path, _SCOPED_BOUNDED, output_dir, cwd=cwd) assert len(messages) == 1 - assert "is outside the skill's declared write scope" in messages[0] + for fragment in ("is outside the declared write scope of skill", "'scoped'", output_dir): + assert fragment in messages[0] + + +def test_cwd_prefixed_output_dir_inside_scope_is_silent(tmp_path: Path) -> None: + output_dir = "${{ context.work_dir }}/.autoskillit/temp/scoped/iter_${{ context.n }}" + + assert ( + _alignment_findings(tmp_path, _SCOPED_BOUNDED, output_dir, cwd="${{ context.work_dir }}") + == [] + ) + + +@pytest.mark.parametrize( + ("output_dir", "cwd"), + [ + ("${{ context.planner_dir }}", "${{ context.work_dir }}"), + ("${{ context.work_dir }}/${{ context.sub }}/x", "${{ context.work_dir }}"), + ("/abs/proj/.autoskillit/temp/other", ""), + ], +) +def test_undecidable_output_dir_is_silent(tmp_path: Path, output_dir: str, cwd: str) -> None: + assert _alignment_findings(tmp_path, _SCOPED_BOUNDED, output_dir, cwd=cwd) == [] + + +@pytest.mark.parametrize("frontmatter", ["write_paths: unrestricted\n", "write_paths: inherit\n"]) +@pytest.mark.parametrize(("output_dir", "cwd"), _WORKTREE_ROOT_OUTPUT_DIRS) +def test_worktree_root_output_dir_silent_for_unrestricted_and_inherit( + tmp_path: Path, frontmatter: str, output_dir: str, cwd: str +) -> None: + assert _alignment_findings(tmp_path, frontmatter, output_dir, cwd=cwd) == [] def _bundled_recipe_paths() -> list[Path]: @@ -300,3 +350,32 @@ def test_rule_silent_on_fixed_bundled_recipes(recipe_path: Path) -> None: f"Recipe {recipe_path.name} has skill-write-path-recipe-alignment findings: " + "\n".join(f" step={f.step_name}: {f.message}" for f in rule_findings) ) + + +def test_rule_is_exercised_not_vacuously_silent() -> None: + """Proves the silence in test_rule_silent_on_fixed_bundled_recipes is earned — + the rule's own selection path reaches BOUNDED skills with a judged output_dir.""" + from autoskillit.recipe._analysis import make_validation_context + from autoskillit.recipe.rules.rules_skill_write_path_alignment import ( + _eligible_skill_path, + _static_output_dir, + ) + + evaluated = 0 + for recipe_path in _bundled_recipe_paths(): + ctx = make_validation_context(load_recipe(recipe_path)) + for step in ctx.recipe.steps.values(): + eligible = _eligible_skill_path(step) + if eligible is None: + continue + output_dir, cwd, skill_name = eligible + resolved = _sh.skill_write_scope(ctx, skill_name) + if _static_output_dir(output_dir, cwd) is None or resolved is None: + continue + scope = resolved[2] + if scope is not None and scope.kind == "bounded": + evaluated += 1 + assert evaluated >= 1, ( + "precondition: no bundled step reached the boundary check with a BOUNDED skill — " + "rule would be vacuously silent (dead matcher?)" + ) From a9a8ed99499da1f914ceb09141a6c686b441514e Mon Sep 17 00:00:00 2001 From: Trecek Date: Fri, 9 Oct 2026 21:21:37 -0700 Subject: [PATCH 08/12] test: guard bundled run_skill dispatches against write-scope drift test_bundled_dispatch_write_scope_admission dry-runs every tracked bundled run_skill step through the real _resolve_dispatch_paths and _extend_closure_write_scope, asserting each is admitted and that the alignment rule flags exactly what the runtime would reject. Planner steps run through a reviewed probe for ${{ context.planner_dir }}, templated-skill dispatches must pass no output_dir, and positive controls keep the sweep non-vacuous. test_attested_dispatch_write_scope_admission drives the real attested run_skill for implementation:plan and implementation:prepare_pr and pins the narrowed write-watch directory. ADR 0015 names the shared predicate and the drift guard. Co-Authored-By: Claude Opus 5.5 --- docs/decisions/0015-write-containment.md | 6 +- tests/_test_filter.py | 1 + ...attested_dispatch_write_scope_admission.py | 65 ++++++ ..._bundled_dispatch_write_scope_admission.py | 198 ++++++++++++++++++ 4 files changed, 269 insertions(+), 1 deletion(-) create mode 100644 tests/server/test_attested_dispatch_write_scope_admission.py create mode 100644 tests/server/test_bundled_dispatch_write_scope_admission.py diff --git a/docs/decisions/0015-write-containment.md b/docs/decisions/0015-write-containment.md index d23f97d752..89b5e24074 100644 --- a/docs/decisions/0015-write-containment.md +++ b/docs/decisions/0015-write-containment.md @@ -24,7 +24,11 @@ Cross-skill intersection was retired. Declared BOUNDED scopes are sibling directories, so no loaded skill's scope nests inside another's in practice, and narrowing one sibling by another only denied legitimate chains such as `investigate → prepare-issue`. Narrowing survives where it means something: a -single skill's scope against one dispatch's `output_dir`. +single skill's scope against one dispatch's `output_dir`. `run_skill` admission and +the `skill-write-path-recipe-alignment` recipe rule both decide that through the one +predicate `output_dir_widens_scope`, and +`tests/server/test_bundled_dispatch_write_scope_admission.py` guards every bundled +dispatch against drift between the two. A skill the projection manifest does not know — another plugin's skill or a built-in command — is bound as foreign: it abstains from containment and from join diff --git a/tests/_test_filter.py b/tests/_test_filter.py index 2e31b45846..5a8cfcb557 100644 --- a/tests/_test_filter.py +++ b/tests/_test_filter.py @@ -1327,6 +1327,7 @@ class SourceMapRejection(enum.StrEnum): "migration", # Server file-level entries importing autoskillit.recipe: "server/test_run_skill_execution_tuning_fallbacks.py", + "server/test_bundled_dispatch_write_scope_admission.py", "server/test_serve_idempotence.py", "server/test_attestation_delivery_reachability.py", "server/test_tools_status_kitchen.py", diff --git a/tests/server/test_attested_dispatch_write_scope_admission.py b/tests/server/test_attested_dispatch_write_scope_admission.py new file mode 100644 index 0000000000..847851f9bb --- /dev/null +++ b/tests/server/test_attested_dispatch_write_scope_admission.py @@ -0,0 +1,65 @@ +"""Attested bundled dispatches of BOUNDED skills reach the executor inside their own scope.""" + +from __future__ import annotations + +import json +from pathlib import Path + +import pytest + +pytestmark = [pytest.mark.layer("server"), pytest.mark.anyio, pytest.mark.medium] + +_IMPLEMENTATION_OVERRIDES = { + "issue_url": "https://github.com/TalonT-Org/AutoSkillit/issues/5256", + "task": "test task", +} + + +@pytest.mark.parametrize( + ("tool_ctx_ready_recipe", "skill"), + [ + (("implementation", "plan", _IMPLEMENTATION_OVERRIDES), "make-plan"), + (("implementation", "prepare_pr", _IMPLEMENTATION_OVERRIDES), "prepare-pr"), + ], + indirect=["tool_ctx_ready_recipe"], +) +async def test_attested_bounded_dispatch_is_admitted_inside_the_skill_scope( + tool_ctx_ready_recipe, git_checkout: Path, skill: str +) -> None: + from autoskillit.server.tools.tools_execution import run_skill + from tests.fakes import InMemoryHeadlessExecutor + from tests.server._helpers import _ready_recipe_segment_step + from tests.server._pipeline_test_helpers import _write_tracker + + ready = tool_ctx_ready_recipe + executor = InMemoryHeadlessExecutor() + ready.tool_ctx.executor = executor + step, credential = _ready_recipe_segment_step(ready.tool_ctx, ready.step_name) + with_args = step["with"] + _write_tracker( + ready.tool_ctx.project_dir, + "AB", + {ready.step_name: {"status": "pending"}}, + {}, + kitchen_id=ready.tool_ctx.kitchen_id, + ) + + result = json.loads( + await run_skill( + skill_command=with_args["skill_command"], + cwd=str(git_checkout), + step_name=ready.step_name, + output_dir=with_args.get("output_dir", ""), + recipe_execution_id=credential["execution_id"], + invocation_template_digest=credential["invocation_template_digests"][ready.step_name], + skill_inputs={name: "probe value" for name in with_args["skill_inputs"]}, + ) + ) + + assert result.get("stage") != "validate_args:run_skill", result + assert "outside the declared write scope" not in json.dumps(result) + assert len(executor.calls) == 1, result + watch_dirs = {Path(entry) for entry in executor.calls[0].write_watch_dirs} + checkout = git_checkout.resolve() + assert checkout / ".autoskillit" / "temp" / skill in watch_dirs + assert checkout not in watch_dirs diff --git a/tests/server/test_bundled_dispatch_write_scope_admission.py b/tests/server/test_bundled_dispatch_write_scope_admission.py new file mode 100644 index 0000000000..1642f2e5b0 --- /dev/null +++ b/tests/server/test_bundled_dispatch_write_scope_admission.py @@ -0,0 +1,198 @@ +"""Bundled recipe dispatches stay inside their skills' declared write scopes. + +Every tracked bundled ``run_skill`` dispatch is admitted by the real runtime admission +code (``_resolve_dispatch_paths`` then ``_extend_closure_write_scope``), and the +``skill-write-path-recipe-alignment`` rule flags exactly the dispatches that code would +reject. A templated-skill dispatch such as ``/autoskillit:vis-lens-{slug}`` passes no +``output_dir``: each family member declares its own scope, so no single directory can be +checked against all of them. +""" + +from __future__ import annotations + +import re +import textwrap +from dataclasses import dataclass, field +from pathlib import Path +from types import SimpleNamespace +from typing import Any + +import pytest + +from autoskillit.core import RecipeSource, SkillExecutionRole, resolve_skill_name +from autoskillit.recipe import load_recipe, run_semantic_rules +from tests._tracked_recipes import tracked_recipe_paths + +pytestmark = [pytest.mark.layer("server"), pytest.mark.medium] + +_PROJECT_ROOT = Path(__file__).resolve().parents[2] +_RULE_NAME = "skill-write-path-recipe-alignment" +_WIDENS_MARKER = "is outside the declared write scope of skill" +_TEMPLATE_RE = re.compile(r"\$\{\{.*?\}\}") + +# Each probe mirrors the directory its producer creates: planner.create_run_dir. +REVIEWED_TEMPLATE_ROOTS = {"${{ context.planner_dir }}": ".autoskillit/temp/planner/run-probe"} + + +def _flagged_steps(recipe) -> frozenset[str]: + return frozenset( + finding.step_name + for finding in run_semantic_rules(recipe) + if finding.rule == _RULE_NAME and _WIDENS_MARKER in finding.message + ) + + +def _dry_run(tool_ctx, skill_command: str, output_dir: str, cwd: Path) -> tuple[bool, str]: + from autoskillit.server.tools.tools_execution._run_skill_session import ( + _extend_closure_write_scope, + _resolve_dispatch_paths, + ) + + skill_name = resolve_skill_name(skill_command) + assert skill_name is not None + invocation = tool_ctx.skill_resolver.resolve_invocation( + skill_name, tool_ctx.project_dir, SkillExecutionRole.SESSION + ) + state = SimpleNamespace( + output_dir=output_dir, + closure_spec=None, + skill_command=skill_command, + cwd=str(cwd), + invocation=invocation, + ) + terminal = _resolve_dispatch_paths(state, base_cwd=cwd) or _extend_closure_write_scope(state) + return terminal is None, str(invocation.root.write_scope.kind) + + +@dataclass +class _Sweep: + tool_ctx: Any + cwd: Path + violations: list[str] = field(default_factory=list) + reached: set[str] = field(default_factory=set) + controls: set[str] = field(default_factory=set) + + def concrete(self, output_dir: str) -> tuple[str | None, bool]: + for root, probe in REVIEWED_TEMPLATE_ROOTS.items(): + if output_dir.startswith(root): + self.reached.add(root) + return probe + _TEMPLATE_RE.sub("probe", output_dir[len(root) :]), True + if "${{" in output_dir.split("/", 1)[0]: + return None, False + return _TEMPLATE_RE.sub("probe", output_dir), False + + def step(self, label: str, with_args: dict[str, Any], flagged: bool) -> None: + output_dir = with_args.get("output_dir", "") or "" + skill_command = with_args.get("skill_command", "") or "" + if resolve_skill_name(skill_command) is None: + if output_dir: + self.violations.append(f"{label}: templated skill dispatch passes output_dir") + return + concrete, reviewed = self.concrete(output_dir) + if concrete is None: + self.violations.append(f"{label}: unreviewed template root in {output_dir!r}") + return + admitted, kind = _dry_run(self.tool_ctx, skill_command, concrete, self.cwd) + if not admitted: + self.violations.append(f"{label}: runtime rejects output_dir {output_dir!r}") + if not reviewed and flagged == admitted: + self.violations.append(f"{label}: rule flagged={flagged}, runtime admitted={admitted}") + if admitted and kind == "bounded": + self.controls.add("explicit" if output_dir else "omitted") + + +def test_bundled_dispatches_are_admitted_and_agree_with_the_rule(tool_ctx, tmp_path: Path) -> None: + sweep = _Sweep(tool_ctx, tmp_path / "project") + paths = tracked_recipe_paths(_PROJECT_ROOT, source=RecipeSource.BUILTIN, scan_dirs=(".",)) + assert paths + for path in paths: + recipe = load_recipe(path) + flagged = _flagged_steps(recipe) + for step_name, step in recipe.steps.items(): + if step.tool == "run_skill": + sweep.step(f"{path.stem}.{step_name}", step.with_args or {}, step_name in flagged) + + assert not sweep.violations, "\n".join(sweep.violations) + assert sweep.controls == {"explicit", "omitted"} + assert sweep.reached == set(REVIEWED_TEMPLATE_ROOTS), "stale REVIEWED_TEMPLATE_ROOTS key" + + +def _one_step_recipe(tmp_path: Path, skill: str, output_dir: str | None, cwd: str | None) -> Path: + with_lines = "".join( + f'\n {key}: "{value}"' + for key, value in (("output_dir", output_dir), ("cwd", cwd)) + if value is not None + ) + path = tmp_path / "control.yaml" + path.write_text( + textwrap.dedent( + """\ + name: control + kitchen_rules: + - "test" + steps: + dispatch: + tool: run_skill + with: + skill_command: "/autoskillit:{skill} probe"{with_lines} + on_success: done + done: + action: stop + message: done + """ + ).format(skill=skill, with_lines=with_lines), + encoding="utf-8", + ) + return path + + +@pytest.mark.parametrize( + ("skill", "output_dir", "cwd", "concrete", "rejected"), + [ + ("make-plan", ".", None, ".", True), + ("make-plan", ".autoskillit/temp", None, ".autoskillit/temp", True), + ("make-plan", "/", None, "/", True), + ("make-plan", "${{ context.work_dir }}", "${{ context.work_dir }}", ".", True), + ( + "make-plan", + "${{ context.work_dir }}/.autoskillit/temp", + "${{ context.work_dir }}", + ".autoskillit/temp", + True, + ), + ("make-plan", None, None, "", False), + ( + "make-plan", + ".autoskillit/temp/make-plan/iter_${{ context.n }}", + None, + ".autoskillit/temp/make-plan/iter_probe", + False, + ), + ("resolve-review", ".", None, ".", False), + ("vis-lens-always-on", None, None, "", False), + ( + "vis-lens-always-on", + ".autoskillit/temp/run-vis-lenses", + None, + ".autoskillit/temp/run-vis-lenses", + True, + ), + ], +) +def test_rule_and_runtime_agree_on_controls( + tool_ctx, + tmp_path: Path, + skill: str, + output_dir: str | None, + cwd: str | None, + concrete: str, + rejected: bool, +) -> None: + recipe = load_recipe(_one_step_recipe(tmp_path, skill, output_dir, cwd)) + + admitted, _kind = _dry_run( + tool_ctx, f"/autoskillit:{skill} probe", concrete, tmp_path / "project" + ) + + assert ("dispatch" in _flagged_steps(recipe)) is rejected + assert admitted is not rejected From cac04eb2050e2d6924d5f51d24e53c0df6afc3ac Mon Sep 17 00:00:00 2001 From: Trecek Date: Fri, 9 Oct 2026 21:29:44 -0700 Subject: [PATCH 09/12] test: re-measure pretty-output recipe baseline maxima Dropping output_dir: . from remediation's make_plan and prepare_pr steps shrinks the rendered remediation recipe; the canonical baseline maxima are re-measured. Co-Authored-By: Claude Opus 5.5 --- 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 23f7de5e42..02fe9a80bc 100644 --- a/tests/infra/test_pretty_output_recipe.py +++ b/tests/infra/test_pretty_output_recipe.py @@ -1350,9 +1350,9 @@ def test_pretty_output_recipe_grid_preserves_semantics_and_budgets(tmp_path, mon f"ingredients_only=both; actual={measured_modes!r}; limit=all registry modes" ) assert maxima == { - "get_recipe_section": (186_880, "remediation", "all_truthy"), - "load_recipe": (186_880, "remediation", "all_truthy"), - "open_kitchen": (186_934, "remediation", "all_truthy"), + "get_recipe_section": (186_838, "remediation", "all_truthy"), + "load_recipe": (186_838, "remediation", "all_truthy"), + "open_kitchen": (186_892, "remediation", "all_truthy"), }, ( "property=canonical baseline maxima; recipe=all; mode=all; tool=all; " f"ingredients_only=both; actual={maxima!r}; limit=measured current baseline" From 8a9c1762662185ce1c8177359c641658d4d74297 Mon Sep 17 00:00:00 2001 From: Trecek Date: Sat, 10 Oct 2026 00:47:20 -0700 Subject: [PATCH 10/12] fix(review): destructure the resolved skill write scope consistently --- src/autoskillit/recipe/rules/rules_contracts.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/autoskillit/recipe/rules/rules_contracts.py b/src/autoskillit/recipe/rules/rules_contracts.py index 8f298802a5..430ccd38a8 100644 --- a/src/autoskillit/recipe/rules/rules_contracts.py +++ b/src/autoskillit/recipe/rules/rules_contracts.py @@ -713,7 +713,9 @@ def _check_path_output_recovery_coverage(ctx: ValidationContext) -> list[RuleFin def _unrestricted_without_output_dir(ctx: ValidationContext, name: str, step: RecipeStep) -> bool: resolved = skill_write_scope(ctx, name) - scope = resolved[2] if resolved is not None else None + scope = None + if resolved is not None: + _, _, scope = resolved unrestricted = scope is not None and scope.kind is WriteScopeKind.UNRESTRICTED return unrestricted and not (step.with_args or {}).get("output_dir") From 89de0878ba48612e7cbaec9aaf7c785f86c01829 Mon Sep 17 00:00:00 2001 From: Trecek Date: Sat, 10 Oct 2026 00:48:43 -0700 Subject: [PATCH 11/12] fix(review): inline the single-use unrestricted output directory predicate --- .../recipe/rules/rules_contracts.py | 20 +++++++++---------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/src/autoskillit/recipe/rules/rules_contracts.py b/src/autoskillit/recipe/rules/rules_contracts.py index 430ccd38a8..14dbef6b59 100644 --- a/src/autoskillit/recipe/rules/rules_contracts.py +++ b/src/autoskillit/recipe/rules/rules_contracts.py @@ -21,7 +21,6 @@ load_bundled_manifest, ) from autoskillit.recipe.registry import RuleFinding, make_finding, semantic_rule -from autoskillit.recipe.schema import RecipeStep # Must match _INTENTIONALLY_EXCLUDED_PATH_TOKENS in _headless_path_tokens.py. # Cannot import directly: recipe (IL-2) cannot depend on execution (IL-1). @@ -711,15 +710,6 @@ def _check_path_output_recovery_coverage(ctx: ValidationContext) -> list[RuleFin return findings -def _unrestricted_without_output_dir(ctx: ValidationContext, name: str, step: RecipeStep) -> bool: - resolved = skill_write_scope(ctx, name) - scope = None - if resolved is not None: - _, _, scope = resolved - unrestricted = scope is not None and scope.kind is WriteScopeKind.UNRESTRICTED - return unrestricted and not (step.with_args or {}).get("output_dir") - - @semantic_rule( name="write-skill-requires-source-output-dir", description=( @@ -753,7 +743,15 @@ def _check_write_skill_requires_source_output_dir(ctx: ValidationContext) -> lis continue if contract.write_behavior not in ("always", "conditional") or contract.read_only: continue - if _unrestricted_without_output_dir(ctx, name, step): + resolved = skill_write_scope(ctx, name) + if resolved is None: + continue + _, _, scope = resolved + if ( + scope is not None + and scope.kind is WriteScopeKind.UNRESTRICTED + and not (step.with_args or {}).get("output_dir") + ): findings.append( make_finding( rule_name="write-skill-requires-source-output-dir", From 72b7238d629edb0df73dac24012c9d688da4dd7c Mon Sep 17 00:00:00 2001 From: Trecek Date: Sat, 10 Oct 2026 09:53:15 -0700 Subject: [PATCH 12/12] test: refresh bounded recipe delivery costs after narrowing write scopes --- tests/server/test_delivery_e2e_verification.py | 18 ++++++------------ 1 file changed, 6 insertions(+), 12 deletions(-) diff --git a/tests/server/test_delivery_e2e_verification.py b/tests/server/test_delivery_e2e_verification.py index ed780339e8..e346a0380e 100644 --- a/tests/server/test_delivery_e2e_verification.py +++ b/tests/server/test_delivery_e2e_verification.py @@ -248,18 +248,12 @@ async def test_implementation_bounded_path_counts_automatic_and_advertised_deliv part=record.part, ) totals = normalized_counter.totals() - # Drift baseline, not a production-safe threshold. Updated to the - # values produced by the current implementation after the - # YAML-safe placeholder substitution fix landed - # (commit 4adc2f4cc) — literal " characters in the worktree directory - # name JSON-escape in serialized records, producing +92 chars per - # placeholder occurrence (× 2 = 184 bytes total) at the - # implementation recipe paths. Pin the refreshed values so this - # test stays a stable drift signal. + # Pin normalized delivery costs to detect drift in the bundled recipe. + # These measured totals are not production-safe thresholds. assert totals == { - "raw_chars": 18_572, - "utf8_bytes": 18_574, - "client_serialized_chars": 20_663, - "estimated_tokens": 4_641, + "raw_chars": 18_551, + "utf8_bytes": 18_553, + "client_serialized_chars": 20_634, + "estimated_tokens": 4_636, "responses": 5, }