Rectify: Smoke-Test Workspace Isolation by Trecek · Pull Request #701 · TalonT-Org/AutoSkillit

Trecek · 2026-04-10T05:10:13Z

Summary

The recipe validation system treats cwd in with_args as an opaque string with zero semantic checks. No rule detects when a recipe operates directly on the user's source workspace instead of an isolated clone. The smoke-test recipe exploits this gap — it calls create_unique_branch and git worktree add with cwd: "${{ inputs.workspace }}", mutating the user's live checkout. This PR introduces a new semantic validation rule (source-isolation-violation) that makes this class of bug structurally impossible, rewrites the smoke-test recipe to use clone_repo, and updates all affected structural tests.

Architecture Impact

Process Flow Diagram

%%{init: {'flowchart': {'nodeSpacing': 50, 'rankSpacing': 60, 'curve': 'basis'}}}%%
flowchart TB
    %% CLASS DEFINITIONS %%
    classDef cli fill:#1a237e,stroke:#7986cb,stroke-width:2px,color:#fff;
    classDef stateNode fill:#004d40,stroke:#4db6ac,stroke-width:2px,color:#fff;
    classDef handler fill:#e65100,stroke:#ffb74d,stroke-width:2px,color:#fff;
    classDef phase fill:#6a1b9a,stroke:#ba68c8,stroke-width:2px,color:#fff;
    classDef newComponent fill:#2e7d32,stroke:#81c784,stroke-width:2px,color:#fff;
    classDef output fill:#00695c,stroke:#4db6ac,stroke-width:2px,color:#fff;
    classDef detector fill:#b71c1c,stroke:#ef5350,stroke-width:2px,color:#fff;
    classDef terminal fill:#1a237e,stroke:#7986cb,stroke-width:2px,color:#fff;

    %% TERMINALS %%
    START([START: Package Import])
    VALID([VALID: Recipe Approved])
    INVALID([INVALID: Recipe Rejected])

    subgraph Registration ["Rule Registration (import side-effect)"]
        direction TB
        REG_IMPORT["● recipe/__init__.py<br/>━━━━━━━━━━<br/>Imports all rules_*.py<br/>modules at package load"]
        REG_DECORATE["★ rules_isolation.py<br/>━━━━━━━━━━<br/>@semantic_rule decorators<br/>append RuleSpec to<br/>_RULE_REGISTRY"]
    end

    subgraph Structural ["Phase 1: Structural Validation"]
        direction TB
        STRUCT_ENTRY["● validate_recipe(recipe)<br/>━━━━━━━━━━<br/>Runs 9 sequential guards"]
        STRUCT_CHECKS{"Structural<br/>errors found?"}
        STRUCT_ERRS["Accumulate errors<br/>━━━━━━━━━━<br/>list of str messages"]
    end

    subgraph Context ["Phase 2: Context Building"]
        direction TB
        CTX_BUILD["make_validation_context<br/>━━━━━━━━━━<br/>Builds once per pass"]
        CTX_GRAPH["_build_step_graph<br/>━━━━━━━━━━<br/>Routing adjacency dict<br/>reads: recipe steps<br/>writes: step_graph"]
        CTX_DATAFLOW["analyze_dataflow<br/>━━━━━━━━━━<br/>BFS: dead outputs,<br/>implicit handoffs,<br/>ref invalidations"]
    end

    subgraph Dispatch ["Phase 3: Semantic Rule Dispatch"]
        direction TB
        DISPATCH_ENTRY["run_semantic_rules<br/>━━━━━━━━━━<br/>reads: _RULE_REGISTRY<br/>reads: ValidationContext"]
        DISPATCH_ITER{"More rules<br/>in registry?"}
        DISPATCH_CALL["spec.check(ctx)<br/>━━━━━━━━━━<br/>Invoke current rule<br/>writes: list of RuleFinding"]
    end

    subgraph Isolation ["Phase 3a: ★ Isolation Rule Checks"]
        direction TB
        ISO_SCAN["★ Scan all recipe steps<br/>━━━━━━━━━━<br/>reads: ctx.recipe.steps<br/>reads: step.cwd, step.tool"]
        ISO_CWD{"Step cwd<br/>matches<br/>inputs.*?"}
        ISO_TOOL{"Tool in<br/>_GIT_MUTATING_TOOLS<br/>or run_cmd with<br/>git mutation?"}
        ISO_FINDING["★ Emit RuleFinding<br/>━━━━━━━━━━<br/>source-isolation-violation: ERROR<br/>git-mutation-on-source: WARNING"]
        ISO_CLEAN["No finding<br/>━━━━━━━━━━<br/>Step uses context.* cwd<br/>or non-mutating tool"]
    end

    subgraph Verdict ["Phase 4: Validity Decision"]
        direction TB
        VERDICT_COMPUTE["compute_recipe_validity<br/>━━━━━━━━━━<br/>reads: structural errors<br/>reads: semantic findings<br/>reads: contract findings"]
        VERDICT_GATE{"Any ERROR-severity<br/>findings or<br/>structural errors?"}
    end

    %% FLOW: Registration %%
    START --> REG_IMPORT
    REG_IMPORT -->|"side-effect import"| REG_DECORATE

    %% FLOW: Structural %%
    REG_DECORATE -.->|"registry populated"| STRUCT_ENTRY
    STRUCT_ENTRY --> STRUCT_CHECKS
    STRUCT_CHECKS -->|"yes"| STRUCT_ERRS
    STRUCT_CHECKS -->|"no errors"| CTX_BUILD
    STRUCT_ERRS --> CTX_BUILD

    %% FLOW: Context %%
    CTX_BUILD --> CTX_GRAPH
    CTX_GRAPH --> CTX_DATAFLOW

    %% FLOW: Dispatch %%
    CTX_DATAFLOW -->|"ValidationContext ready"| DISPATCH_ENTRY
    DISPATCH_ENTRY --> DISPATCH_ITER
    DISPATCH_ITER -->|"yes"| DISPATCH_CALL
    DISPATCH_CALL -->|"next rule"| DISPATCH_ITER
    DISPATCH_ITER -->|"no: all rules checked"| VERDICT_COMPUTE

    %% FLOW: Isolation detail (expanded from DISPATCH_CALL) %%
    DISPATCH_CALL -.->|"when rule is isolation"| ISO_SCAN
    ISO_SCAN --> ISO_CWD
    ISO_CWD -->|"no: context.* or literal"| ISO_CLEAN
    ISO_CWD -->|"yes: inputs.* ref"| ISO_TOOL
    ISO_TOOL -->|"yes: git-mutating"| ISO_FINDING
    ISO_TOOL -->|"no: safe tool"| ISO_CLEAN
    ISO_FINDING -.->|"appended to findings"| DISPATCH_CALL
    ISO_CLEAN -.->|"next step"| ISO_SCAN

    %% FLOW: Verdict %%
    VERDICT_COMPUTE --> VERDICT_GATE
    VERDICT_GATE -->|"no: clean"| VALID
    VERDICT_GATE -->|"yes: ERROR found"| INVALID

    %% CLASS ASSIGNMENTS %%
    class START,VALID,INVALID terminal;
    class REG_IMPORT,REG_DECORATE newComponent;
    class STRUCT_ENTRY handler;
    class STRUCT_CHECKS,STRUCT_ERRS stateNode;
    class CTX_BUILD,CTX_GRAPH,CTX_DATAFLOW phase;
    class DISPATCH_ENTRY,DISPATCH_CALL handler;
    class DISPATCH_ITER stateNode;
    class ISO_SCAN,ISO_FINDING newComponent;
    class ISO_CWD,ISO_TOOL detector;
    class ISO_CLEAN stateNode;
    class VERDICT_COMPUTE handler;
    class VERDICT_GATE detector;

Development Diagram

%%{init: {'flowchart': {'nodeSpacing': 50, 'rankSpacing': 60, 'curve': 'basis'}}}%%
flowchart TB
    %% CLASS DEFINITIONS %%
    classDef cli fill:#1a237e,stroke:#7986cb,stroke-width:2px,color:#fff;
    classDef stateNode fill:#004d40,stroke:#4db6ac,stroke-width:2px,color:#fff;
    classDef handler fill:#e65100,stroke:#ffb74d,stroke-width:2px,color:#fff;
    classDef phase fill:#6a1b9a,stroke:#ba68c8,stroke-width:2px,color:#fff;
    classDef newComponent fill:#2e7d32,stroke:#81c784,stroke-width:2px,color:#fff;
    classDef output fill:#00695c,stroke:#4db6ac,stroke-width:2px,color:#fff;
    classDef detector fill:#b71c1c,stroke:#ef5350,stroke-width:2px,color:#fff;
    classDef gap fill:#ff6f00,stroke:#ffa726,stroke-width:2px,color:#000;
    classDef terminal fill:#1a237e,stroke:#7986cb,stroke-width:2px,color:#fff;

    subgraph Source ["SOURCE — recipe/ package"]
        direction TB
        INIT["● recipe/__init__.py<br/>━━━━━━━━━━<br/>Side-effect imports 17 rules_*<br/>Re-exports public API"]
        RULES_ISO["★ rules_isolation.py<br/>━━━━━━━━━━<br/>source-isolation-violation<br/>git-mutation-on-source"]
        RULES_OTHER["rules_*.py (16 existing)<br/>━━━━━━━━━━<br/>Semantic rule modules"]
        REGISTRY["registry.py<br/>━━━━━━━━━━<br/>@semantic_rule decorator<br/>_RULE_REGISTRY"]
    end

    subgraph Recipes ["RECIPES — smoke-test"]
        direction TB
        SMOKE_YAML["● smoke-test.yaml<br/>━━━━━━━━━━<br/>Smoke pipeline recipe"]
        SMOKE_CONTRACT["● contracts/smoke-test.yaml<br/>━━━━━━━━━━<br/>Generated contract card"]
        SMOKE_DIAG["● diagrams/smoke-test.md<br/>━━━━━━━━━━<br/>Generated flow diagram"]
    end

    subgraph Build ["BUILD TOOLING"]
        direction TB
        PYPROJECT["pyproject.toml<br/>━━━━━━━━━━<br/>hatchling backend<br/>uv package manager"]
        TASKFILE["● Taskfile.yml<br/>━━━━━━━━━━<br/>test-all, test-check,<br/>test-smoke, install-worktree"]
    end

    subgraph Quality ["CODE QUALITY GATES"]
        direction LR
        FORMAT["ruff format<br/>━━━━━━━━━━<br/>Auto-fix formatting"]
        LINT["ruff check<br/>━━━━━━━━━━<br/>Lint + auto-fix"]
        MYPY["mypy<br/>━━━━━━━━━━<br/>Type checking"]
        GITLEAKS["gitleaks<br/>━━━━━━━━━━<br/>Secret scanning"]
    end

    subgraph Testing ["TEST FRAMEWORK"]
        direction TB
        PYTEST["pytest + xdist<br/>━━━━━━━━━━<br/>-n 4 parallel<br/>asyncio_mode=auto"]
        T_ISO["★ test_rules_isolation.py<br/>━━━━━━━━━━<br/>7 tests: rule paths,<br/>bundled recipe sweep"]
        T_SMOKE["● test_smoke_pipeline.py<br/>━━━━━━━━━━<br/>18 structural tests<br/>+ skipped execution"]
        T_BUNDLED["● test_bundled_recipes.py<br/>━━━━━━━━━━<br/>Per-recipe invariants<br/>+ isolation compliance"]
        T_ARCH["● test_subpackage_isolation.py<br/>━━━━━━━━━━<br/>Layer contracts via AST<br/>__all__ completeness"]
        T_INSTR["● test_instruction_surface.py<br/>━━━━━━━━━━<br/>SOURCE ISOLATION sentinel<br/>Pipeline tool restrictions"]
        T_INFRA["● test_taskfile.py<br/>━━━━━━━━━━<br/>Taskfile structure<br/>pytest path assertions"]
    end

    subgraph Entry ["ENTRY POINTS"]
        direction LR
        CLI["autoskillit CLI<br/>━━━━━━━━━━<br/>autoskillit.cli:main"]
    end

    %% SOURCE FLOW %%
    RULES_ISO -->|registers via| REGISTRY
    RULES_OTHER -->|registers via| REGISTRY
    INIT -->|side-effect import| RULES_ISO
    INIT -->|side-effect import| RULES_OTHER

    %% RECIPE VALIDATION %%
    REGISTRY -->|validates| SMOKE_YAML
    SMOKE_YAML --> SMOKE_CONTRACT
    SMOKE_YAML --> SMOKE_DIAG

    %% BUILD FLOW %%
    PYPROJECT --> TASKFILE

    %% QUALITY PIPELINE %%
    TASKFILE -->|pre-commit| FORMAT
    FORMAT --> LINT
    LINT --> MYPY
    MYPY --> GITLEAKS

    %% TEST FLOW %%
    TASKFILE -->|task test-all| PYTEST
    TASKFILE -->|task test-smoke| T_SMOKE
    PYTEST --> T_ISO
    PYTEST --> T_BUNDLED
    PYTEST --> T_ARCH
    PYTEST --> T_INSTR
    PYTEST --> T_INFRA

    %% ENTRY %%
    PYPROJECT -->|console_scripts| CLI

    %% CLASS ASSIGNMENTS %%
    class INIT,RULES_OTHER,REGISTRY stateNode;
    class RULES_ISO,T_ISO newComponent;
    class SMOKE_YAML,SMOKE_CONTRACT,SMOKE_DIAG handler;
    class PYPROJECT phase;
    class TASKFILE phase;
    class FORMAT,LINT,MYPY,GITLEAKS detector;
    class PYTEST handler;
    class T_SMOKE,T_BUNDLED,T_ARCH,T_INSTR,T_INFRA handler;
    class CLI output;

Module Dependency Diagram

%%{init: {'flowchart': {'nodeSpacing': 50, 'rankSpacing': 70, 'curve': 'basis'}}}%%
graph TB
    %% CLASS DEFINITIONS %%
    classDef cli fill:#1a237e,stroke:#7986cb,stroke-width:2px,color:#fff;
    classDef stateNode fill:#004d40,stroke:#4db6ac,stroke-width:2px,color:#fff;
    classDef handler fill:#e65100,stroke:#ffb74d,stroke-width:2px,color:#fff;
    classDef phase fill:#6a1b9a,stroke:#ba68c8,stroke-width:2px,color:#fff;
    classDef newComponent fill:#2e7d32,stroke:#81c784,stroke-width:2px,color:#fff;
    classDef output fill:#00695c,stroke:#4db6ac,stroke-width:2px,color:#fff;
    classDef detector fill:#b71c1c,stroke:#ef5350,stroke-width:2px,color:#fff;
    classDef integration fill:#c62828,stroke:#ef9a9a,stroke-width:2px,color:#fff;

    subgraph L3_Tests ["L3 — TEST SUITES"]
        direction LR
        T_ISO["★ test_rules_isolation<br/>━━━━━━━━━━<br/>New isolation rule tests<br/>Fan-out: 2 pkgs"]
        T_BUNDLED["● test_bundled_recipes<br/>━━━━━━━━━━<br/>All-recipe validation<br/>Fan-out: 2 pkgs"]
        T_SMOKE["● test_smoke_pipeline<br/>━━━━━━━━━━<br/>Smoke-test recipe tests<br/>Fan-out: 2 pkgs"]
        T_ARCH["● test_subpackage_isolation<br/>━━━━━━━━━━<br/>AST-based layer guard<br/>Fan-out: 0 pkgs"]
        T_INST["● test_instruction_surface<br/>━━━━━━━━━━<br/>Contract surface tests<br/>Fan-out: 6 pkgs"]
        T_TASK["● test_taskfile<br/>━━━━━━━━━━<br/>Taskfile path assertions<br/>Fan-out: 0 pkgs"]
    end

    subgraph L2_API ["L2 — RECIPE PUBLIC API"]
        direction LR
        INIT["● recipe/__init__<br/>━━━━━━━━━━<br/>Re-export gateway<br/>Side-effect rule import<br/>Fan-in: 20"]
        VALIDATOR["recipe/validator<br/>━━━━━━━━━━<br/>Validation orchestrator<br/>Fan-in: 26"]
    end

    subgraph L1_Rules ["L1 — RECIPE RULE PLUGINS (self-registering)"]
        direction LR
        R_ISO["★ rules_isolation<br/>━━━━━━━━━━<br/>source-isolation-violation<br/>git-mutation-on-source<br/>Fan-in: 1"]
        R_OTHER["rules_{bypass,ci,clone,<br/>cmd,contracts,dataflow,<br/>graph,inputs,merge,<br/>packs,recipe,skill_content,<br/>skills,tools,verdict,<br/>worktree}<br/>━━━━━━━━━━<br/>15 existing rule modules<br/>Fan-in: 1 each"]
    end

    subgraph L1_Core_Recipe ["L1 — RECIPE CORE MODULES"]
        direction LR
        REGISTRY["recipe/registry<br/>━━━━━━━━━━<br/>_RULE_REGISTRY<br/>semantic_rule decorator<br/>run_semantic_rules<br/>Fan-in: 42"]
        ANALYSIS["recipe/_analysis<br/>━━━━━━━━━━<br/>ValidationContext<br/>Step graph builder<br/>Fan-in: 18"]
        SCHEMA["recipe/schema<br/>━━━━━━━━━━<br/>Recipe, RecipeStep<br/>Domain model leaf<br/>Fan-in: 43"]
        IO["recipe/io<br/>━━━━━━━━━━<br/>load_recipe<br/>iter_steps_with_context<br/>Fan-in: 12"]
        CONTRACTS["recipe/contracts<br/>━━━━━━━━━━<br/>Contract cards<br/>_INPUT_REF_RE<br/>Fan-in: 8"]
    end

    subgraph L0_Foundation ["L0 — CORE FOUNDATION"]
        direction LR
        CORE_TYPES["core/types<br/>━━━━━━━━━━<br/>Severity, RecipeSource<br/>SKILL_TOOLS, enums<br/>Fan-in: 208"]
        CORE_IO["core/io<br/>━━━━━━━━━━<br/>load_yaml, atomic_write<br/>Fan-in: 15"]
        CORE_LOG["core/logging<br/>━━━━━━━━━━<br/>get_logger<br/>Fan-in: 40"]
        CORE_PATHS["core/paths<br/>━━━━━━━━━━<br/>pkg_root<br/>Fan-in: 10"]
    end

    subgraph L0_External ["L0 — EXTERNAL"]
        direction LR
        EXT_IGRAPH["igraph<br/>━━━━━━━━━━<br/>Graph analysis"]
        EXT_PKG["packaging<br/>━━━━━━━━━━<br/>Version parsing"]
    end

    %% === TEST → RECIPE IMPORTS === %%
    T_ISO -->|"registry, io"| INIT
    T_ISO -->|"Severity"| CORE_TYPES
    T_BUNDLED -->|"validator, io, contracts"| INIT
    T_BUNDLED -->|"SKILL_TOOLS"| CORE_TYPES
    T_SMOKE -->|"io, validator"| INIT
    T_INST -->|"io"| INIT

    %% === L2 → L1 IMPORTS === %%
    INIT -->|"side-effect import"| R_ISO
    INIT -->|"side-effect import"| R_OTHER
    INIT -->|"re-exports"| VALIDATOR
    VALIDATOR -->|"orchestrates"| REGISTRY
    VALIDATOR -->|"ValidationContext"| ANALYSIS
    VALIDATOR -->|"Recipe"| SCHEMA

    %% === RULE PLUGIN → REGISTRY PATTERN === %%
    R_ISO -->|"semantic_rule, RuleFinding"| REGISTRY
    R_ISO -->|"ValidationContext"| ANALYSIS
    R_ISO -->|"_INPUT_REF_RE"| CONTRACTS
    R_ISO -->|"Severity, get_logger"| CORE_TYPES
    R_OTHER -->|"semantic_rule, RuleFinding"| REGISTRY
    R_OTHER -->|"ValidationContext"| ANALYSIS

    %% === L1 INTERNAL DEPENDENCIES === %%
    REGISTRY -->|"Severity"| CORE_TYPES
    REGISTRY -->|"Recipe, DataFlowReport"| SCHEMA
    REGISTRY -->|"ValidationContext"| ANALYSIS
    ANALYSIS -->|"Recipe, RecipeStep"| SCHEMA
    ANALYSIS -->|"SKILL_TOOLS"| CORE_TYPES
    ANALYSIS -->|"_CONTEXT_REF_RE"| CONTRACTS
    ANALYSIS -->|"iter_steps"| IO
    IO -->|"Recipe, RecipeStep"| SCHEMA
    IO -->|"load_yaml, pkg_root"| CORE_IO
    IO -->|"RecipeSource"| CORE_TYPES
    CONTRACTS -->|"load_yaml, atomic_write"| CORE_IO
    CONTRACTS -->|"Severity, SKILL_TOOLS"| CORE_TYPES

    %% === L1 → EXTERNAL === %%
    ANALYSIS -->|"graph ops"| EXT_IGRAPH

    %% === L0 CORE IS SELF-CONTAINED === %%
    CORE_TYPES ~~~ CORE_IO
    CORE_IO ~~~ CORE_LOG

    %% CLASS ASSIGNMENTS %%
    class T_ISO,R_ISO newComponent;
    class T_BUNDLED,T_SMOKE,T_ARCH,T_INST,T_TASK,INIT detector;
    class VALIDATOR,REGISTRY phase;
    class ANALYSIS,SCHEMA,IO,CONTRACTS handler;
    class R_OTHER handler;
    class CORE_TYPES,CORE_IO,CORE_LOG,CORE_PATHS stateNode;
    class EXT_IGRAPH,EXT_PKG integration;

Closes #697

Implementation Plan

Plan file: /home/talon/projects/autoskillit-runs/remediation-20260409-205816-838063/.autoskillit/temp/rectify/rectify_smoke-test-isolation_2026-04-09_213000_part_a.md

🤖 Generated with Claude Code via AutoSkillit

Token Usage Summary

Step	uncached	output	cache_read	cache_write	count	time
investigate	38	7.3k	356.7k	36.3k	1	5m 4s
rectify	10.5k	17.2k	1.2M	73.5k	1	8m 27s
dry_walkthrough	105	29.6k	2.2M	116.1k	2	20m 36s
implement	209	48.8k	7.9M	157.4k	2	24m 12s
prepare_pr	23	4.3k	132.0k	26.9k	1	1m 17s
run_arch_lenses	3.9k	15.8k	591.3k	84.5k	3	8m 9s
compose_pr	23	8.2k	181.4k	33.6k	1	2m 24s
Total	14.9k	131.2k	12.6M	528.2k		1h 10m

…tic rules New rules_isolation.py detects recipes that operate on the source repo instead of an isolated clone: create_unique_branch with inputs.* cwd triggers ERROR, run_skill without clone triggers WARNING, run_cmd with git-mutating commands on inputs.* triggers WARNING. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Replace worktree-based isolation with clone_repo pattern matching bundled recipes. Renames workspace ingredient to source_dir, uses push_to_remote instead of run_cmd git push, adds register_clone_status for cleanup, and adds SOURCE ISOLATION kitchen rule. Updates all structural tests to match the new step topology. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Contract card now reflects clone-based step topology. Diagram rewritten to show clone > create_branch > setup > implement_task > run_tests > push_branch > create_pr > close_pr flow with clone cleanup paths. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

- Use run_cmd with task test-check instead of test_check tool in smoke recipe to avoid clone-root-as-worktree semantic error (clone root is intentionally the test directory, not a sub-worktree) - Filter bundled recipe sweep for ERROR-level only (research.yaml legitimately operates on source without cloning) - Bump recipe subpackage file exemption to 30 for rules_isolation.py - Add rules_isolation.py to CLAUDE.md module listing Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

…n parametrized guards Adds _resolve_recipe_path helper and _ALL_CLONE_RECIPE_PATHS dynamic list for clone-aware recipe discovery across both bundled and project-local dirs. Includes smoke-test in three applicable parametrized guard tests. Adds new test_all_clone_recipes_use_context_cwd_after_clone sweep test. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Adds test_git_mutating_recipes_have_clone_step that validates any bundled recipe using MCP git-mutation tools (create_unique_branch, push_to_remote) also uses clone_repo for workspace isolation. Scoped to structured step.tool checks to avoid false positives from raw shell commands. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Fixes test-smoke target pointing to nonexistent tests/test_smoke_pipeline.py (correct path: tests/recipe/test_smoke_pipeline.py). Adds test_taskfile_pytest_paths_exist guard to prevent silent zero-test-collection from broken Taskfile targets. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Trecek

AutoSkillit PR Review — Verdict: changes_requested (9 actionable warnings, 2 decision items, 4 info)

Trecek · 2026-04-10T05:18:54Z

+)
+
+# Tools that inherently mutate git state when given a cwd
+_GIT_MUTATING_TOOLS = frozenset({"create_unique_branch"})


[warning] cohesion: _GIT_MUTATING_TOOLS contains only create_unique_branch, but tests/contracts/test_instruction_surface.py defines a local GIT_MUTATION_TOOLS = {"create_unique_branch", "push_to_remote"} that includes push_to_remote. The two sets diverge — the rule enforcer and the contract test use different tool sets to define git-mutating tools, breaking the single-source-of-truth pattern.

Trecek · 2026-04-10T05:18:54Z

      cmd: >-
-        git worktree remove "${{ context.worktree_path }}" --force 2>/dev/null || true &&
        git push origin --delete "${{ context.branch_name }}" 2>/dev/null || true &&
        git branch -D "${{ context.branch_name }}" 2>/dev/null || true


[warning] bugs: git branch -D may target the currently checked-out branch inside the clone, which will fail. The || true suppresses the error silently, so local branch cleanup may silently fail. Consider checking out a detached HEAD or the base branch before deleting.

Investigated — this is intentional. The || true is deliberate best-effort cleanup (REQ-GUARD-002 pattern). The clone directory is unconditionally deleted by the subsequent register_clone_status → remove_clone call chain, so the dangling local branch has zero practical consequence — it disappears with the clone directory. Adding git checkout --detach before git branch -D would add fragile complexity to a cleanup step whose output is discarded.

Trecek · 2026-04-10T05:18:54Z

      cmd: >-
-        git worktree remove "${{ context.worktree_path }}" --force 2>/dev/null || true &&
        git push origin --delete "${{ context.branch_name }}" 2>/dev/null || true &&
        git branch -D "${{ context.branch_name }}" 2>/dev/null || true


[warning] bugs: Same git branch -D issue as delete_remote_branch — may fail silently on the checked-out branch in the clone. The || true masks the failure.

Investigated — this is intentional. Same REQ-GUARD-002 best-effort cleanup pattern as delete_remote_branch. The clone directory is deleted by register_clone_failure → remove_clone immediately after, so the unsuppressed local branch deletion has no consequence.

Trecek · 2026-04-10T05:18:54Z

+    return builtin_recipes_dir() / f"{name}.yaml"
+
+
+_ALL_CLONE_RECIPE_PATHS: list[Path] = []


[warning] cohesion: _ALL_CLONE_RECIPE_PATHS is constructed at module import time using a side-effectful loop with file I/O (p.read_text()). Other cross-recipe parametrized tests in the same file use a static name list. This creates an asymmetric collection pattern within the same file.

Trecek · 2026-04-10T05:18:55Z

+
+
+# T_SP_NEW_17
+def test_no_step_uses_inputs_as_cwd_after_clone(smoke_recipe) -> None:


[info] tests: test_no_step_uses_inputs_as_cwd_after_clone is functionally identical to the cross-recipe parametrized test_all_clone_recipes_use_context_cwd_after_clone in test_bundled_recipes.py (L2243), which already covers smoke-test. Duplicated regex logic and invariant assertion.

Trecek · 2026-04-10T05:18:55Z

+
+
+# T_SP_NEW_18
+def test_smoke_recipe_passes_isolation_rules(smoke_recipe) -> None:


[info] tests: test_smoke_recipe_passes_isolation_rules duplicates coverage already provided by test_bundled_recipes_pass_isolation_rules in test_rules_isolation.py (L166), which iterates all bundled recipes including smoke-test.

Trecek · 2026-04-10T05:18:55Z

 SMOKE_RECIPE = PROJECT_ROOT / ".autoskillit" / "recipes" / "smoke-test.yaml"


+def _resolve_recipe_path(name: str) -> Path:


[info] cohesion: _resolve_recipe_path exists solely because smoke-test is project-local. Some parametrized tests include smoke-test and use this helper, while structurally identical tests exclude smoke-test and call builtin_recipes_dir() directly — asymmetric treatment.

Trecek · 2026-04-10T05:18:55Z

+# ---------------------------------------------------------------------------
+
+
+def test_run_cmd_git_checkout_after_clone_passes() -> None:


[info] cohesion: Test coverage is asymmetric — test_run_cmd_git_checkout_after_clone_passes tests the git-mutation-on-source rule with context.* cwd, but there is no corresponding test for run_skill with inputs.* cwd WITH a clone step passes (the has_clone suppression path).

Trecek · 2026-04-10T05:18:55Z

+# ---------------------------------------------------------------------------
+
+
+def test_bundled_recipes_pass_isolation_rules() -> None:


[info] tests: test_bundled_recipes_pass_isolation_rules only asserts no ERROR-severity isolation findings exist but silently ignores WARNING-severity findings. If a bundled recipe has a WARNING-level isolation issue, it will pass undetected.

The has_clone flag checks structural presence of clone_repo without considering skip_when_false conditions, which may suppress legitimate isolation warnings for conditional execution paths. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Replace raw text search 'clone_repo in p.read_text()' with parsed recipe step tool check to avoid false matches from comments or description fields. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Replace raw text 'clone_repo in raw' assertion with parsed step tool check since the loaded recipe is already available on the preceding line. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Account for recipes that invoke clone_repo via python: callables (e.g. merge-prs.yaml) in addition to tool: clone_repo steps. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Trecek and others added 7 commits April 9, 2026 21:24

Trecek commented Apr 10, 2026

View reviewed changes

Trecek and others added 4 commits April 9, 2026 22:26

fix(review): use parsed YAML to identify clone recipes

7d15ab0

Replace raw text search 'clone_repo in p.read_text()' with parsed recipe step tool check to avoid false matches from comments or description fields. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

fix(review): use parsed recipe for clone_repo assertion

4e5e944

Replace raw text 'clone_repo in raw' assertion with parsed step tool check since the loaded recipe is already available on the preceding line. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

fix(review): handle python: clone_repo in git-mutation assertion

f1bc67a

Account for recipes that invoke clone_repo via python: callables (e.g. merge-prs.yaml) in addition to tool: clone_repo steps. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Trecek added this pull request to the merge queue Apr 10, 2026

Merged via the queue into integration with commit 3f345bb Apr 10, 2026
2 checks passed

Trecek deleted the fix-smoke-test-recipe-to-use-isolated-worktree-instead-of-cu/697 branch April 10, 2026 05:42

Trecek mentioned this pull request Apr 14, 2026

Promote integration to main (198 PRs, 182 issues, 102 fixes, 107 features, 1 infra, 7 tests, 4 docs) #925

Merged

		return builtin_recipes_dir() / f"{name}.yaml"


		_ALL_CLONE_RECIPE_PATHS: list[Path] = []



		# T_SP_NEW_17
		def test_no_step_uses_inputs_as_cwd_after_clone(smoke_recipe) -> None:



		# T_SP_NEW_18
		def test_smoke_recipe_passes_isolation_rules(smoke_recipe) -> None:

		SMOKE_RECIPE = PROJECT_ROOT / ".autoskillit" / "recipes" / "smoke-test.yaml"


		def _resolve_recipe_path(name: str) -> Path:

		# ---------------------------------------------------------------------------


		def test_run_cmd_git_checkout_after_clone_passes() -> None:

		# ---------------------------------------------------------------------------


		def test_bundled_recipes_pass_isolation_rules() -> None:

Conversation

Trecek commented Apr 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Architecture Impact

Process Flow Diagram

Development Diagram

Module Dependency Diagram

Implementation Plan

Token Usage Summary

Uh oh!

Trecek left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Trecek commented Apr 10, 2026 •

edited

Loading