Rectify: stdlib-only Contract for SKILL.md Python Blocks

[Trecek]

Summary

SKILL.md bash blocks can embed python3 -c or python3 - <<'EOF' blocks that import from the autoskillit package. These blocks execute inside headless Claude Code sessions where the Python interpreter is the user's system Python — not the project's virtual environment. The package may not be installed there, causing silent or visible failures. The architecture has no structural mechanism preventing this pattern.

This part adds a semantic validation rule that makes the pattern statically unacceptable: any SKILL.md that imports autoskillit in a python3 bash block fails validation at validate_recipe() time, before the skill can be executed. Once merged, CI rejects future violations at commit time rather than surfacing them silently at runtime.

Part B adds a deterministic PostToolUse hook (token_summary_appender) replacing the LLM-dependent Step 0b token summary injection. Part C fixes the remaining four SKILL.md import violations caught by the new rule.

Architecture Impact

Module Dependency Diagram

%%{init: {'flowchart': {'nodeSpacing': 50, 'rankSpacing': 70, 'curve': 'basis'}}}%%
graph TB
    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;

    subgraph L3_Hooks ["L3 — HOOKS (PostToolUse)"]
        direction LR
        TSA["★ token_summary_appender.py<br/>━━━━━━━━━━<br/>stdlib-only PostToolUse hook<br/>Appends token table to PR body"]
        HREG["● hook_registry.py<br/>━━━━━━━━━━<br/>HookDef, HOOK_REGISTRY<br/>imports: core.pkg_root"]
    end

    subgraph L2_Recipe ["L2 — RECIPE (Semantic Validation)"]
        direction LR
        RSC["● rules_skill_content.py<br/>━━━━━━━━━━<br/>+no-autoskillit-import rule<br/>imports: core, recipe._analysis,<br/>recipe._skill_placeholder_parser,<br/>recipe.contracts, recipe.registry"]
        REG["recipe/registry.py<br/>━━━━━━━━━━<br/>@semantic_rule, RuleFinding<br/>imports: core, recipe._analysis,<br/>recipe.schema"]
        SPP["recipe/_skill_placeholder_parser.py<br/>━━━━━━━━━━<br/>extract_bash_blocks()<br/>imports: stdlib only"]
        CON["● recipe/skill_contracts.yaml<br/>━━━━━━━━━━<br/>Contract definitions"]
    end

    subgraph L1_Exec ["L1 — EXECUTION"]
        direction LR
        EINIT["● execution/__init__.py<br/>━━━━━━━━━━<br/>Re-export hub<br/>+parse_hunk_ranges"]
    end

    subgraph L0_Core ["L0 — CORE (Foundation)"]
        direction LR
        CORE["core/<br/>━━━━━━━━━━<br/>Severity, pkg_root,<br/>SkillResult, types"]
    end

    subgraph Skills ["SKILL.md FILES (bash blocks — not Python imports)"]
        direction LR
        OPR["● open-pr/SKILL.md<br/>━━━━━━━━━━<br/>Removed python3 heredoc<br/>importing autoskillit"]
        OIP["● open-integration-pr/SKILL.md<br/>━━━━━━━━━━<br/>Replaced autoskillit import<br/>with stdlib inline"]
        RPR["● review-pr/SKILL.md<br/>━━━━━━━━━━<br/>Replaced autoskillit import<br/>with stdlib inline"]
        APR["● analyze-prs/SKILL.md<br/>━━━━━━━━━━<br/>Replaced __import__ autoskillit<br/>with stdlib inline"]
    end

    subgraph Tests ["TEST COVERAGE"]
        direction LR
        TTSA["★ test_token_summary_appender.py<br/>━━━━━━━━━━<br/>TSA-1 through TSA-8"]
        TRSC["● test_rules_skill_content.py<br/>━━━━━━━━━━<br/>SC-PKG-1 through SC-PKG-7"]
        TRVD["★ test_review_pr_diff_annotation.py"]
        THEX["● test_hook_executability.py"]
        THRC["● test_hook_registration_coverage.py"]
    end

    %% L2 internal dependencies %%
    RSC -->|"imports"| REG
    RSC -->|"imports"| SPP
    RSC -->|"imports"| CON

    %% Downward dependencies (valid) %%
    RSC -->|"imports Severity"| CORE
    REG -->|"imports Severity"| CORE
    HREG -->|"imports pkg_root"| CORE
    EINIT -->|"re-exports from"| CORE

    %% TSA is stdlib-only — no package dependencies %%
    TSA -.-x|"NO autoskillit imports<br/>(stdlib-only by design)"| CORE

    %% Semantic rule scans SKILL.md files %%
    RSC -.->|"validates bash blocks"| OPR
    RSC -.->|"validates bash blocks"| OIP
    RSC -.->|"validates bash blocks"| RPR
    RSC -.->|"validates bash blocks"| APR

    %% Test dependencies %%
    TTSA -.->|"tests"| TSA
    TRSC -.->|"tests"| RSC
    THEX -.->|"tests"| HREG
    THRC -.->|"tests"| HREG

    %% Class assignments %%
    class TSA,TTSA,TRVD newComponent;
    class RSC,REG,SPP,CON phase;
    class HREG handler;
    class EINIT handler;
    class CORE stateNode;
    class OPR,OIP,RPR,APR gap;
    class TRSC,THEX,THRC output;

Loading

Process Flow Diagram

%%{init: {'flowchart': {'nodeSpacing': 50, 'rankSpacing': 60, 'curve': 'basis'}}}%%
flowchart TB
    classDef terminal 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 detector fill:#b71c1c,stroke:#ef5350,stroke-width:2px,color:#fff;
    classDef gap fill:#ff6f00,stroke:#ffa726,stroke-width:2px,color:#000;

    START([Pipeline Start])

    subgraph Validation ["★ SEMANTIC VALIDATION (no-autoskillit-import rule)"]
        direction TB
        VR["● validate_recipe / load_recipe<br/>━━━━━━━━━━<br/>MCP tool entry point<br/>calls run_semantic_rules()"]
        VS["● run_semantic_rules()<br/>━━━━━━━━━━<br/>Iterates _RULE_REGISTRY<br/>calls each @semantic_rule"]
        VI{"★ _check_no_autoskillit_import<br/>━━━━━━━━━━<br/>step.tool == run_skill?"}
        VB["★ extract_bash_blocks()<br/>━━━━━━━━━━<br/>Parse SKILL.md content<br/>extract python3 blocks"]
        VM{"★ Pattern match?<br/>━━━━━━━━━━<br/>from autoskillit...<br/>import autoskillit...<br/>__import__ autoskillit"}
        VF["★ RuleFinding(ERROR)<br/>━━━━━━━━━━<br/>Skill X bash block contains<br/>autoskillit import"]
        VP["Pass<br/>━━━━━━━━━━<br/>No violation found"]
    end

    subgraph Hook ["★ POSTTOOLUSE HOOK (token_summary_appender)"]
        direction TB
        HT["★ PostToolUse fires<br/>━━━━━━━━━━<br/>on run_skill response"]
        HP{"★ PR URL in response?<br/>━━━━━━━━━━<br/>regex: github.com/.../pull/N"}
        HS["★ Read session logs<br/>━━━━━━━━━━<br/>sessions.jsonl + token_usage.json<br/>filter by cwd"]
        HF["★ Format token table<br/>━━━━━━━━━━<br/>stdlib-only markdown formatter"]
        HA["★ gh pr edit --body-file<br/>━━━━━━━━━━<br/>Append Token Usage Summary<br/>to PR body"]
        HN["Skip<br/>━━━━━━━━━━<br/>No PR URL or no data"]
    end

    subgraph SkillFix ["● SKILL.MD FIXES (bash block cleanup)"]
        direction TB
        SF["● 4 SKILL.md files<br/>━━━━━━━━━━<br/>open-pr, open-integration-pr,<br/>review-pr, analyze-prs"]
        SR["Replaced autoskillit imports<br/>━━━━━━━━━━<br/>with stdlib-only inline code<br/>or PostToolUse hook delegation"]
    end

    COMPLETE([Pipeline Complete])

    %% Validation flow %%
    START -->|"recipe loaded"| VR
    VR -->|"iterates rules"| VS
    VS -->|"invokes new rule"| VI
    VI -->|"yes: run_skill step"| VB
    VI -->|"no: skip step"| VP
    VB -->|"for each bash block"| VM
    VM -->|"match found"| VF
    VM -->|"no match"| VP

    %% Hook flow %%
    START -->|"run_skill completes"| HT
    HT --> HP
    HP -->|"yes"| HS
    HP -->|"no"| HN
    HS -->|"tokens found"| HF
    HS -->|"no data"| HN
    HF --> HA
    HA --> COMPLETE

    %% Skill fix flow %%
    SF --> SR
    SR -.->|"now passes"| VI

    class START,COMPLETE terminal;
    class VR,VS phase;
    class VI,VM,HP stateNode;
    class VB,HT,HS,HF,HA newComponent;
    class VF detector;
    class VP,HN handler;
    class SF,SR gap;

Loading

Closes #506

Implementation Plan

Plan file: /home/talon/projects/autoskillit-runs/remediation-20260325-123256-483611/.autoskillit/temp/rectify/rectify_skill-python-autoskillit-import-contract_2026-03-25_160000_part_a.md

🤖 Generated with Claude Code via AutoSkillit

[Trecek]

AutoSkillit PR Review — Verdict: changes_requested (10 inline findings: 1 critical, 9 warning)

[Trecek]

[warning] bugs: info["owner"]["login"] uses bare dict access. If gh repo view returns unexpected JSON schema, KeyError or TypeError propagates uncaught. Wrap in try/except with descriptive error or validate keys.

[Trecek]

Investigated — this is intentional. The subprocess at L109-115 uses check=True, so CalledProcessError is raised before json.loads if gh exits non-zero. The gh repo view --json owner,name command returns a stable schema where owner.login is always present on success.

[Trecek]

[warning] cohesion: fetch_merge_queue_data uses check=True for gh repo view but manually checks returncode for gh api graphql. Inconsistent error handling strategy within the same function.

[Trecek]

Investigated — this is intentional. check=True for gh repo view is a hard prerequisite (function cannot proceed without owner/name). Manual returncode check for GraphQL is a soft dependency — empty entries list is a valid degraded state (no PRs in queue). This dual-mode error handling reflects different criticality levels.

[Trecek]

[warning] bugs: When fetch_merge_queue_data fails and routes to analyze_prs via on_failure, context.merge_queue_data_path is empty string. The skill_command passes merge_queue_data_path= (empty value). Verify analyze-prs handles empty-string path correctly via its bash guard.

[Trecek]

Investigated — this is intentional. The step note at lines 179-183 documents the failure path. The analyze-prs SKILL.md bash guard uses [ -n "${merge_queue_data_path:-}" ] to handle empty/unset values, falling back to QUEUE_ENTRIES="[]". This is the designed fallback (QUEUE_MODE=false).

[Trecek]

[warning] cohesion: annotated_diff_path and hunk_ranges_path use type string while merge_queue_data_path and domain_partitions_path use type file_path. All four are pre-staged file paths. Use consistent type file_path for all.

[Trecek]

Investigated — this is intentional. The type 'path' does not exist in skill_contracts.yaml (vocabulary: string, file_path, directory_path, file_path_list). The change from file_path to string for annotated_diff_path/hunk_ranges_path was deliberate (commit ee3b894) — file_path triggered false-positive uncaptured-handoff-consumer semantic rule violations.

[Trecek]

AutoSkillit review found 10 blocking issues (1 critical, 9 warning). See inline comments.

Critical

• token_summary_appender.py:69 — re.search receives non-string result_text when inner JSON result field is a dict/list/int. TypeError at runtime.

Warnings (source)

• token_summary_appender.py:31 — _log_root() ignores XDG_DATA_HOME, diverging from resolve_log_dir(). Silent zero-session result in CI.
• token_summary_appender.py:111 — TOCTOU: read_text() not guarded after exists() check.
• token_summary_appender.py:232 — Empty gh pr view stdout causes PR body replacement with only the token table.
• token_summary_appender.py:234 — CalledProcessError caught by broad except Exception with generic message.
• smoke_utils.py:117 — Bare info["owner"]["login"] dict access; KeyError/TypeError on unexpected JSON.
• smoke_utils.py:135 — json.loads on GraphQL stdout not guarded against JSONDecodeError.
• smoke_utils.py:110 — Asymmetric error handling: check=True for gh repo view but manual returncode for gh api graphql.
• merge-prs.yaml:188 — Empty context.merge_queue_data_path passed as merge_queue_data_path= on failure path.
• skill_contracts.yaml:389 — annotated_diff_path/hunk_ranges_path use type string while sibling inputs use file_path.

Additional info-level findings (non-blocking)

• Redundant mkdir in annotate_pr_diff (smoke_utils.py:86)
• "Parts B and C" implementation-phase comments in 5 test files
• Weak assertions in TSA-2 and TSA-5 tests
• post_only = all_scripts - pre_registered logic flaw in hook registration coverage test