Skip to content

Split promote-to-main into Changelog PR Skill + Review-Promotion Skill#577

Merged
Trecek merged 5 commits intointegrationfrom
split-promote-to-main-into-changelog-pr-skill-review-promoti/576
Apr 2, 2026
Merged

Split promote-to-main into Changelog PR Skill + Review-Promotion Skill#577
Trecek merged 5 commits intointegrationfrom
split-promote-to-main-into-changelog-pr-skill-review-promoti/576

Conversation

@Trecek
Copy link
Copy Markdown
Collaborator

@Trecek Trecek commented Apr 2, 2026

Summary

The promote-to-main skill (.claude/skills/promote-to-main/SKILL.md) is a monolith that
conflates two distinct concerns: (1) generating a PR body that documents what changed
(changelog, release notes, architecture diagrams) and (2) reviewer-facing deep analysis
(domain risk scoring, breaking change audit, regression hotspots, quality assessment).

This plan splits it into two focused bundled skills:

  1. Streamlined promote-to-main — promoted from project-local (.claude/skills/) to
    bundled (skills_extended/). Retains Phases 0–2 (setup, pre-flight, change inventory)
    unchanged. Adds a condensed Phase 3 for architecture diagrams and a new Phase 4 for
    release notes synthesis + PR creation. Removes all domain analysis and quality assessment.

  2. New review-promotion — a new bundled skill in skills_extended/. Takes the domain
    analysis (old Phase 3), quality assessment (old Phase 4), and a reviewer-focused summary
    synthesis. Optionally posts as a GitHub PR review comment via --post-to-pr.

The local override .claude/skills/promote-to-main/ is deleted; the bundled skill replaces it.

Requirements

PROMOTE — Streamlined promote-to-main Skill

  • REQ-PROMOTE-001: The promote-to-main skill must produce a PR body containing only changelog content: executive summary, release notes, merged PR table, linked issues table, architecture diagrams, and closing references.
  • REQ-PROMOTE-002: The promote-to-main skill must not include domain risk scores, review guidance, breaking change audit tables, regression hotspot analysis, or test coverage deltas in the PR body or promotion report.
  • REQ-PROMOTE-003: The promote-to-main skill must invoke arch-lens skills via the Skill tool (not just write context files) and embed validated diagrams in the PR body.
  • REQ-PROMOTE-004: The promote-to-main skill must complete in 5 phases or fewer: Setup, Pre-flight, Change Inventory, Executive Summary + Release Notes, Architecture Diagrams, and Compose Report + Create PR.
  • REQ-PROMOTE-005: The promote-to-main skill must preserve existing pre-flight gate behavior (CI status, version consistency, outstanding reviews must all pass before proceeding).
  • REQ-PROMOTE-006: The promote-to-main skill must carry forward all Closes #N, Fixes #N, and Resolves #N references from merged PR bodies.

REVIEW — New review-promotion Skill

  • REQ-REVIEW-001: The review-promotion skill must accept an optional PR number or branch range as input for standalone or pipeline use.
  • REQ-REVIEW-002: The review-promotion skill must produce per-domain risk analysis with risk scores, key changes, and review guidance.
  • REQ-REVIEW-003: The review-promotion skill must produce a breaking change audit with severity ratings and affected downstream consumers.
  • REQ-REVIEW-004: The review-promotion skill must produce a regression risk assessment including conflict hotspots and rectify chains.
  • REQ-REVIEW-005: The review-promotion skill must produce a test coverage delta analysis.
  • REQ-REVIEW-006: The review-promotion skill must produce a cross-domain dependency analysis with integration confidence rating.
  • REQ-REVIEW-007: The review-promotion skill must write its output report to .autoskillit/temp/review-promotion/.

CONV — Skill Conventions

  • REQ-CONV-001: Both skills must follow the existing Tier 2/3 SKILL.md structural conventions used by other skills in src/autoskillit/skills_extended/.
  • REQ-CONV-002: Both skills must emit structured output tokens as their final lines for pipeline capture.
  • REQ-CONV-003: Both skills must use Task tool (model: sonnet) for all subagent spawning with the Subagent Autonomy Grant.

Architecture Impact

Process Flow Diagram

%%{init: {'flowchart': {'nodeSpacing': 40, 'rankSpacing': 55, 'curve': 'basis'}}}%%
flowchart TB
    %% CLASS DEFINITIONS %%
    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 output fill:#00695c,stroke:#4db6ac,stroke-width:2px,color:#fff;
    classDef detector fill:#b71c1c,stroke:#ef5350,stroke-width:2px,color:#fff;

    START_P2M([● promote-to-main START])
    END_P2M([● promote-to-main END])
    START_RP([★ review-promotion START])
    END_RP([★ review-promotion END])
    ERR_PREFLIGHT([ERROR: preflight failed])

    subgraph P2M ["● promote-to-main  (bundled — skills_extended/)"]
        direction TB

        subgraph P0 ["● Phase 0: Setup"]
            direction LR
            P0A["● Parse Args<br/>━━━━━━━━━━<br/>integration_branch<br/>base_branch · dry_run"]
            P0B["● Divergence Point<br/>━━━━━━━━━━<br/>merge_base_sha<br/>commit_count · date"]
            P0C["● Token Summary<br/>━━━━━━━━━━<br/>load from session logs<br/>kitchen_id filter"]
        end

        subgraph P1 ["● Phase 1: Pre-flight (parallel, blocking)"]
            direction LR
            P1A["● 1A: CI & Branch<br/>━━━━━━━━━━<br/>gh pr checks<br/>behind_base_by"]
            P1B["● 1B: Version Consistency<br/>━━━━━━━━━━<br/>pyproject vs plugin.json<br/>uv lock --check"]
            P1C["● 1C: Review Items<br/>━━━━━━━━━━<br/>CHANGES_REQUESTED PRs<br/>in-progress issues"]
        end

        GATE{"● all pass?"}

        subgraph P2 ["● Phase 2: Change Inventory (parallel)"]
            direction LR
            P2A["● 2A: Commit Categorization<br/>━━━━━━━━━━<br/>rectify · feature<br/>infra · test · docs"]
            P2B["● 2B: PR + Issue Linkage<br/>━━━━━━━━━━<br/>Closes/Fixes/Resolves<br/>traceability matrix"]
            P2C["● 2C: File Lifecycle<br/>━━━━━━━━━━<br/>added · modified<br/>deleted · renamed"]
            P2D["● 2D: Migration Detection<br/>━━━━━━━━━━<br/>schema · deps<br/>hooks · CI"]
        end

        subgraph P3 ["★ Phase 3: Architecture Diagrams"]
            direction LR
            P3A["★ Lens Selection<br/>━━━━━━━━━━<br/>1–3 lenses via subagent<br/>dev lens guard"]
            P3B["★ Diagram Generation<br/>━━━━━━━━━━<br/>Write context → Skill<br/>(anti-prose guard loop)"]
        end

        subgraph P4 ["★ Phase 4: Compose PR + Create PR"]
            direction TB
            P4A["★ Release Notes Synthesis<br/>━━━━━━━━━━<br/>Phase 2 data only<br/>exec summary · highlights"]
            P4B["★ Write PR Body<br/>━━━━━━━━━━<br/>changelog · release notes<br/>merged PRs · arch diagrams"]
            DRY{"dry_run?"}
            GH_AUTH{"gh auth ok?"}
            P4C["● Create PR<br/>━━━━━━━━━━<br/>gh pr create --body-file<br/>add 'promotion' label"]
        end

        OUT_P2M["● report_path · pr_url<br/>verdict · category_summary"]
    end

    subgraph RP ["★ review-promotion  (new skill — skills_extended/)"]
        direction TB

        subgraph RP0 ["★ Phase 0: Setup"]
            direction LR
            RP0A["★ Parse Args<br/>━━━━━━━━━━<br/>integration_branch<br/>base_branch · --post-to-pr"]
            RP0B["★ Divergence + Files<br/>━━━━━━━━━━<br/>merge_base_sha<br/>changed_files"]
            RP0C["★ Find PR (opt)<br/>━━━━━━━━━━<br/>gh pr list --head<br/>store pr_number"]
        end

        subgraph RP1 ["★ Phase 1: Domain Analysis"]
            direction TB
            RP1A["★ Partition by Domain<br/>━━━━━━━━━━<br/>partition_files_by_domain<br/>(autoskillit.execution.pr_analysis)"]
            RP1B["★ Domain Diffs + Commits<br/>━━━━━━━━━━<br/>git diff per domain (≤12k)<br/>git log per domain"]
            RP1C["★ Parallel Domain Agents<br/>━━━━━━━━━━<br/>risk_score · review_guidance<br/>breaking_changes · key_changes"]
            RP1D["★ Cross-Domain Analysis<br/>━━━━━━━━━━<br/>alignment · risks<br/>integration_confidence"]
        end

        subgraph RP2 ["★ Phase 2: Quality Assessment (parallel)"]
            direction LR
            RP2A["★ 2A: Test Coverage Delta<br/>━━━━━━━━━━<br/>new source vs test files<br/>coverage_assessment"]
            RP2B["★ 2B: Breaking Change Audit<br/>━━━━━━━━━━<br/>removed fns · sig changes<br/>Protocol/ABC mods"]
            RP2C["★ 2C: Regression Risk<br/>━━━━━━━━━━<br/>hotspot_files<br/>rectify_chains · overall_risk"]
        end

        subgraph RP3 ["★ Phase 3: Review Synthesis"]
            RP3A["★ Verdict Subagent<br/>━━━━━━━━━━<br/>review_ready · needs_attention<br/>blocking_issues"]
        end

        subgraph RP4_WRITE ["★ Phase 4: Write Review Report"]
            RP4["★ Write Report<br/>━━━━━━━━━━<br/>.autoskillit/temp/review-promotion/<br/>review_report_{timestamp}.md"]
        end

        POST_Q{"★ --post-to-pr<br/>AND pr_number?"}
        RP5["★ Post PR Comment<br/>━━━━━━━━━━<br/>gh pr comment --body-file<br/>(graceful fail)"]
        OUT_RP["★ report_path · verdict"]
    end

    %% PROMOTE-TO-MAIN FLOW
    START_P2M --> P0A --> P0B --> P0C
    P0C --> P1A & P1B & P1C
    P1A & P1B & P1C --> GATE
    GATE -->|"any fail"| ERR_PREFLIGHT
    GATE -->|"all pass"| P2A & P2B & P2C & P2D
    P2A & P2B & P2C & P2D --> P3A --> P3B
    P3B --> P4A --> P4B --> DRY
    DRY -->|"yes"| OUT_P2M
    DRY -->|"no"| GH_AUTH
    GH_AUTH -->|"fail"| OUT_P2M
    GH_AUTH -->|"pass"| P4C --> OUT_P2M --> END_P2M

    %% REVIEW-PROMOTION FLOW
    START_RP --> RP0A --> RP0B --> RP0C
    RP0C --> RP1A --> RP1B --> RP1C --> RP1D
    RP1D --> RP2A & RP2B & RP2C
    RP2A & RP2B & RP2C --> RP3A --> RP4
    RP4 --> POST_Q
    POST_Q -->|"yes"| RP5 --> OUT_RP --> END_RP
    POST_Q -->|"no"| OUT_RP

    %% CLASS ASSIGNMENTS
    class START_P2M,END_P2M,START_RP,END_RP terminal;
    class ERR_PREFLIGHT detector;
    class P0A,P0B,P0C,P1A,P1B,P1C,P2A,P2B,P2C,P2D handler;
    class P3A,P3B,P4A,P4B,P4C newComponent;
    class RP0A,RP0B,RP0C,RP1A,RP1B,RP1C,RP1D,RP2A,RP2B,RP2C,RP3A,RP4,RP5 newComponent;
    class GATE,DRY,GH_AUTH,POST_Q stateNode;
    class OUT_P2M,OUT_RP output;
Loading

Color Legend:

Color Category Description
Dark Blue Terminal START and END states
Orange Handler Unchanged phase nodes carried forward from original promote-to-main
Green New Component ★ New phases / nodes introduced by this PR
Teal State Decision points (gates, flags, auth checks)
Dark Teal Output Structured output tokens emitted
Red Detector Error terminal (preflight failure)

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 phase fill:#6a1b9a,stroke:#ba68c8,stroke-width:2px,color:#fff;
    classDef handler fill:#e65100,stroke:#ffb74d,stroke-width:2px,color:#fff;
    classDef stateNode fill:#004d40,stroke:#4db6ac,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;
    classDef terminal fill:#1a237e,stroke:#7986cb,stroke-width:2px,color:#fff;

    subgraph L0 ["L0 — CORE (zero autoskillit imports)"]
        direction LR
        PATHS["core/paths.py<br/>━━━━━━━━━━<br/>pkg_root()<br/>importlib.resources"]
        TYPES["core/types.py<br/>━━━━━━━━━━<br/>SkillSource · SkillInfo<br/>re-export hub"]
        YAML_IO["core/io.py<br/>━━━━━━━━━━<br/>load_yaml · atomic_write<br/>YAMLError"]
    end

    subgraph L1_WS ["L1 — WORKSPACE"]
        direction TB
        SKILLS_PY["workspace/skills.py<br/>━━━━━━━━━━<br/>SkillResolver · bundled_skills_dir<br/>bundled_skills_extended_dir<br/>detect_project_local_overrides"]
        SESSION_SKILLS["workspace/session_skills.py<br/>━━━━━━━━━━<br/>DefaultSessionSkillManager<br/>SkillsDirectoryProvider"]
    end

    subgraph L1_EX ["L1 — EXECUTION"]
        PR_ANALYSIS["execution/pr_analysis.py<br/>━━━━━━━━━━<br/>partition_files_by_domain<br/>DOMAIN_PATHS"]
    end

    subgraph L2_DATA ["L2 — SKILLS DATA (filesystem scan — not Python imports)"]
        direction LR
        PTM["★ skills_extended/<br/>promote-to-main/SKILL.md<br/>━━━━━━━━━━<br/>5 phases · bundled<br/>(renamed from .claude/skills/)"]
        RP["★ skills_extended/<br/>review-promotion/SKILL.md<br/>━━━━━━━━━━<br/>5 phases + post-to-pr<br/>(new skill)"]
        WR["● skills_extended/<br/>write-recipe/SKILL.md<br/>━━━━━━━━━━<br/>bundled skills list updated<br/>(modified)"]
    end

    subgraph L3_TESTS ["L3 — TEST SUITE"]
        direction LR
        T_SKILLS["● tests/workspace/test_skills.py<br/>━━━━━━━━━━<br/>BUNDLED_SKILLS contract<br/>count assertions updated"]
        T_PROMOTE["★ tests/skills/<br/>test_promote_split_contracts.py<br/>━━━━━━━━━━<br/>promote-to-main · review-promotion<br/>structural contracts"]
        T_PLACEHOLDER["● tests/skills/<br/>test_skill_placeholder_contracts.py<br/>━━━━━━━━━━<br/>allowlist entries added<br/>(modified)"]
    end

    %% DISCOVERY (filesystem scan — not Python import)
    SKILLS_PY -.->|"iterdir() scan<br/>pkg_root()/skills_extended/"| PTM
    SKILLS_PY -.->|"iterdir() scan"| RP
    SKILLS_PY -.->|"iterdir() scan"| WR

    %% RUNTIME INVOCATION from SKILL.md (shell python3 -c)
    RP -.->|"python3 -c runtime<br/>shell invocation"| PR_ANALYSIS

    %% WORKSPACE → CORE
    SKILLS_PY -->|"imports pkg_root<br/>SkillSource · YAMLError · load_yaml"| PATHS
    SKILLS_PY -->|"imports SkillSource"| TYPES
    SKILLS_PY -->|"imports load_yaml · YAMLError"| YAML_IO
    SESSION_SKILLS -->|"imports SkillInfo<br/>SkillResolver"| SKILLS_PY

    %% TESTS → WORKSPACE
    T_SKILLS -->|"imports SkillResolver<br/>bundled_skills_dir<br/>bundled_skills_extended_dir"| SKILLS_PY
    T_SKILLS -->|"imports SkillSource"| TYPES
    T_PROMOTE -->|"imports pkg_root (deferred)"| PATHS

    %% HIGH FAN-IN: pkg_root
    HI_FAN["HIGH FAN-IN: pkg_root()<br/>━━━━━━━━━━<br/>imported by workspace/skills.py<br/>and tests/skills/test_promote_split_contracts.py<br/>importlib.resources.files('autoskillit')"]

    PATHS --- HI_FAN

    %% CLASS ASSIGNMENTS
    class PATHS,TYPES,YAML_IO stateNode;
    class SKILLS_PY,SESSION_SKILLS phase;
    class PR_ANALYSIS handler;
    class PTM,RP newComponent;
    class WR,T_SKILLS,T_PLACEHOLDER output;
    class T_PROMOTE newComponent;
    class HI_FAN integration;
Loading

Color Legend:

Color Category Description
Teal L0 Core Foundation modules — high fan-in, zero autoskillit imports
Purple L1 Workspace Skill discovery and session management
Orange L1 Execution Runtime utilities (partition_files_by_domain)
Green New Components ★ New skills and new test contracts
Dark Teal Modified ● Modified files (write-recipe, test_skills, placeholder tests)
Red High Fan-In pkg_root() — anchor for all skills_extended discovery
Dashed Lines Non-import Filesystem scan (SkillResolver) and runtime shell invocation (SKILL.md)

Closes #576

Implementation Plan

Plan file: /home/talon/projects/autoskillit-runs/impl-20260402-152802-966153/.autoskillit/temp/make-plan/promote_split_plan_2026-04-02_120000.md

🤖 Generated with Claude Code via AutoSkillit

Token Usage Summary

Step input output cached count time
plan 41 28.5k 1.8M 1 8m 12s
verify 40 19.5k 2.5M 1 5m 54s
implement 64 23.0k 3.9M 1 8m 2s
fix 32 12.1k 1.4M 1 6m 22s
audit_impl 288 13.0k 304.1k 1 12m 38s
open_pr 36 32.1k 1.8M 1 15m 51s
Total 501 128.2k 11.6M 57m 1s

Trecek and others added 2 commits April 2, 2026 15:50
…n skill

Promotes promote-to-main from project-local (.claude/skills/) to bundled
(skills_extended/) with domain analysis and quality assessment removed (Phases 0-4
only). Adds new review-promotion skill with full domain risk scoring, breaking change
audit, regression risk assessment, and --post-to-pr support. Updates skill counts
from 61 to 63 throughout CLAUDE.md, docs, and test assertions.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…nces for promote-to-main and review-promotion
Copy link
Copy Markdown
Collaborator Author

@Trecek Trecek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AutoSkillit PR Review — Verdict: changes_requested (10 blocking findings; posting as COMMENT since reviewer is PR author)

Comment thread tests/workspace/test_skills.py
# Count top-level ### Phase N: headers
phase_headers = re.findall(r"^###\s+Phase\s+\d+", content, re.MULTILINE)
# Phase 0 counts, so 5 phases = Phase 0..4
assert len(phase_headers) <= 6, (
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[warning] bugs: Assertion/comment mismatch: inline comment says '5 phases = Phase 0..4' but assertion is <= 6 (allows 6 headers). Either change assertion to <= 5 to match the stated 5-phase limit, or update the docstring/comment/name to say '6'.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Valid observation — flagged for design decision. The comment says '5 phases = Phase 0..4' but the assertion is <= 6. The <= 6 slack is intentional (message says 'Phases 0-4 + sub-steps') but the comment and assertion are inconsistent. Human should decide: tighten to <= 5 or update the comment to describe the intentional slack.

content = (pkg_root() / "skills_extended" / "promote-to-main" / "SKILL.md").read_text()
assert "pr_url = " in content

def test_promote_to_main_arch_lens_loop_is_guarded(self):
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[warning] tests: Brittle string-match guard in test_promote_to_main_arch_lens_loop_is_guarded: uses re.search(r'do not output any prose', ...) which breaks on any rephrasing. No equivalent anti-prose guard test exists for review-promotion, which also spawns arch-lens diagram loops.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Investigated — this is intentional. (1) The test uses re.IGNORECASE and SKILL.md line 355 reads 'Do NOT output any prose status text between lens iterations' — the regex matches this literally and is a contractual phrase. (2) review-promotion has zero arch-lens references; the feature does not exist in that skill, so no equivalent test is needed.

("promote-to-main", "branch"), # per-branch loop var in git fetch origin {branch}:{branch}
("promote-to-main", "merge_base_sha"), # runtime-computed from git merge-base
("promote-to-main", "number"), # per-issue iteration in gh issue view {number}
("promote-to-main", "pr_title"), # runtime-computed PR title string
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[warning] tests: Suspicious ghost allowlist entry ('promote-to-main', 'pr_title'): {pr_title} does not appear in any bash block in the new promote-to-main SKILL.md — the PR title is constructed inline in prose at line L450, not as a bash variable. If this placeholder doesn't exist in the skill's bash blocks, this entry masks future real violations silently.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Investigated — this is intentional. {pr_title} IS in a bash block at promote-to-main/SKILL.md line 455: --title "{pr_title}" \\. The allowlist entry at test line 120 correctly reflects a real bash-block placeholder and is not a ghost entry.

<sub>Generated with Claude Code via AutoSkillit</sub>
```
Write to `.autoskillit/temp/promote-to-main/pr_body_{YYYY-MM-DD_HHMMSS}.md`
(relative to the current working directory).
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[warning] defense: Step 4.2 writes the PR body file using a relative path ('relative to the current working directory'), contradicting the ALWAYS constraint at line 53 which requires report_path to be an absolute path (prepend CWD). Use absolute path: {CWD}/.autoskillit/temp/promote-to-main/pr_body_{timestamp}.md.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Investigated — this is intentional. The ALWAYS constraint at line 53 applies specifically to the report_path output token, not to the prose write instruction. Line 471 correctly emits report_path = {absolute path ...} satisfying the constraint. The write description at line 424 saying 'relative to CWD' is standard orientation prose.


```
report_path = {absolute path to .autoskillit/temp/promote-to-main/promotion_report_{timestamp}.md}
report_path = {absolute path to .autoskillit/temp/promote-to-main/pr_body_{timestamp}.md}
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[warning] cohesion: report_path output token now points to pr_body_{timestamp}.md (a PR body file), not an analysis report. In review-promotion, report_path correctly points to review_report_{timestamp}.md. Rename to pr_body_path to differentiate and match what it actually holds.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Valid observation — flagged for design decision. In promote-to-main, report_path points to the PR body file; in review-promotion it points to an analysis report. Renaming to pr_body_path would improve clarity but downstream recipe consumers may depend on the report_path token name. Human should decide whether to rename.

Comment thread src/autoskillit/skills_extended/review-promotion/SKILL.md
Comment thread src/autoskillit/skills_extended/review-promotion/SKILL.md
### Phase 4: Write Review Report

```bash
mkdir -p .autoskillit/temp/review-promotion
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[warning] defense: Phase 4's mkdir -p .autoskillit/temp/review-promotion uses a relative path. The ALWAYS constraint (line 43) requires report_path to be absolute. The mkdir and subsequent file writes should use the absolute path (prepend CWD) to prevent silent creation in wrong directories if CWD shifts.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Investigated — this is intentional. The ALWAYS constraint at line 43 governs the output token report_path, not all file system operations. The relative mkdir at line 295 is standard shell practice; CWD is stable throughout the skill session. The output token at line 400 is correctly emitted as an absolute path.

### Phase 5: Post to PR (only if --post-to-pr AND pr_number found)

```bash
gh pr comment {pr_number} --body-file .autoskillit/temp/review-promotion/review_report_{timestamp}.md
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[critical] defense: Phase 5 gh pr comment --body-file .autoskillit/temp/review-promotion/review_report_{timestamp}.md uses a relative path, contradicting the NEVER constraint (line 40) and the output token at line 400 which correctly specifies absolute path. Use the absolute report_path captured in Phase 4 to ensure the file is found regardless of CWD.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Investigated — this is intentional. The NEVER constraint at line 40 says 'Use gh pr comment --body inline — always use --body-file'. Line 390 correctly uses --body-file, satisfying that constraint. The NEVER rule says nothing about relative vs absolute paths; the reviewer misattributed a different constraint.

Copy link
Copy Markdown
Collaborator Author

@Trecek Trecek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AutoSkillit review found 10 blocking issues. See inline comments.

@Trecek Trecek added this pull request to the merge queue Apr 2, 2026
Merged via the queue into integration with commit 47f7674 Apr 2, 2026
2 checks passed
@Trecek Trecek deleted the split-promote-to-main-into-changelog-pr-skill-review-promoti/576 branch April 2, 2026 23:53
github-merge-queue bot pushed a commit that referenced this pull request Apr 3, 2026
…ctly in Bundled Skills (#582)

## Summary

PR #577 placed `promote-to-main` and `review-promotion` in
`src/autoskillit/skills_extended/` (bundled package skills) instead of
`.claude/skills/` (project-local skills). These skills are specific to
the AutoSkillit repository itself and have no value when shipped to
external users.

The fix is a file relocation and accompanying count revert:
1. Move both SKILL.md files from `skills_extended/` to `.claude/skills/`
2. Delete the now-empty skill directories from `skills_extended/`
3. Delete `tests/skills/test_promote_split_contracts.py` (tests the
wrong invariant)
4. Revert all bundled skill counts: CLAUDE.md, docs, test assertions,
and write-recipe list

## Architecture Impact

### Module Dependency Diagram

```mermaid
%%{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 Consumers ["L3 — Consumers (server / cli / recipe)"]
        direction LR
        CLI["cli/app.py<br/>━━━━━━━━━━<br/>skills command"]
        Recipe["recipe/rules_skills.py<br/>━━━━━━━━━━<br/>skill resolvability rules"]
        Server["server/tools_execution.py<br/>━━━━━━━━━━<br/>run_skill handler"]
    end

    subgraph Workspace ["L2 — workspace/ (skill discovery)"]
        direction LR
        Resolver["● workspace/skills.py<br/>━━━━━━━━━━<br/>SkillResolver.list_all() → 60<br/>detect_project_local_overrides()"]
        SessionMgr["workspace/session_skills.py<br/>━━━━━━━━━━<br/>DefaultSessionSkillManager<br/>writes ephemeral skill dirs"]
    end

    subgraph Core ["L1 — core/ (types & paths)"]
        direction LR
        Enums["core/_type_enums.py<br/>━━━━━━━━━━<br/>SkillSource.BUNDLED<br/>SkillSource.BUNDLED_EXTENDED"]
        Paths["core/paths.py<br/>━━━━━━━━━━<br/>pkg_root()"]
    end

    subgraph Tier1 ["Tier 1 — skills/ (BUNDLED, 3 skills)"]
        direction LR
        T1["skills/<br/>━━━━━━━━━━<br/>open-kitchen<br/>close-kitchen<br/>sous-chef"]
    end

    subgraph Tier23 ["Tier 2+3 — skills_extended/ (BUNDLED_EXTENDED, 58 skills)"]
        direction LR
        T23["skills_extended/<br/>━━━━━━━━━━<br/>● write-recipe + 57 others<br/>(promote-to-main removed)<br/>(review-promotion removed)"]
    end

    subgraph ProjLocal ["Project-Local — .claude/skills/ (18 skills, not shipped)"]
        direction LR
        PTM["★ .claude/skills/promote-to-main/<br/>━━━━━━━━━━<br/>SKILL.md relocated here"]
        RP["★ .claude/skills/review-promotion/<br/>━━━━━━━━━━<br/>SKILL.md relocated here"]
        PLOther[".claude/skills/16 others<br/>━━━━━━━━━━<br/>audit-arch, make-req, etc."]
    end

    subgraph Tests ["Tests (● updated)"]
        direction LR
        TSkills["● tests/workspace/test_skills.py<br/>━━━━━━━━━━<br/>list_all() == 60<br/>skills_extended count == 58"]
        TContracts["● tests/skills/test_skill_placeholder_contracts.py<br/>━━━━━━━━━━<br/>allowlist entries removed"]
    end

    %% CONSUMER → WORKSPACE %%
    CLI -->|"imports SkillResolver"| Resolver
    Recipe -->|"imports SkillResolver"| Resolver
    Server -->|"imports DefaultSessionSkillManager"| SessionMgr

    %% WORKSPACE INTERNAL %%
    SessionMgr -->|"uses"| Resolver

    %% WORKSPACE → CORE %%
    Resolver -->|"imports SkillSource, pkg_root"| Enums
    Resolver -->|"imports"| Paths
    SessionMgr -->|"imports SkillSource"| Enums

    %% RESOLVER → SKILL DIRS %%
    Resolver -->|"scans BUNDLED"| T1
    Resolver -->|"scans BUNDLED_EXTENDED"| T23
    Resolver -.->|"detect_project_local_overrides()"| PTM
    Resolver -.->|"detect_project_local_overrides()"| RP
    Resolver -.->|"detect_project_local_overrides()"| PLOther

    %% SESSION MGR → EPHEMERAL %%
    SessionMgr -->|"skips BUNDLED<br/>(plugin-dir channel)"| T1
    SessionMgr -->|"writes Tier 2+3<br/>to /dev/shm/"| T23
    SessionMgr -.->|"skips when name<br/>in overrides"| PTM
    SessionMgr -.->|"skips when name<br/>in overrides"| RP

    %% CLASS ASSIGNMENTS %%
    class CLI,Recipe cli;
    class Server handler;
    class Resolver phase;
    class SessionMgr handler;
    class Enums,Paths stateNode;
    class T1,T23 output;
    class PTM,RP newComponent;
    class PLOther output;
    class TSkills,TContracts detector;
```

**Color Legend:**
| Color | Category | Description |
|-------|----------|-------------|
| Dark Blue | CLI/Recipe | Consumer layer — imports SkillResolver |
| Orange | Handler | Session manager, server tools |
| Purple | Phase/Resolver | SkillResolver — skill discovery and
resolution |
| Teal | State | Core types: SkillSource enum, pkg_root() |
| Dark Teal | Output | Skill directory tiers (Tier 1, Tier 2+3, other
project-local) |
| Green | ★ New Location | promote-to-main and review-promotion
relocated to .claude/skills/ |
| Red | Tests | Updated test assertions for new counts |
| Dashed Lines | Detection | detect_project_local_overrides() — override
scanning path |

Closes #579

## Implementation Plan

Plan file:
`/home/talon/projects/autoskillit-runs/impl-20260402-173309-328308/.autoskillit/temp/make-plan/bug_promote_review_relocation_plan_2026-04-02_000000.md`

🤖 Generated with [Claude Code](https://claude.com/claude-code) via
AutoSkillit

## Token Usage Summary

| Step | input | output | cached | count | time |
|------|-------|--------|--------|-------|------|
| plan | 31 | 16.3k | 1.3M | 1 | 5m 56s |
| verify | 28 | 10.0k | 1.1M | 1 | 3m 11s |
| implement | 55 | 14.0k | 2.6M | 1 | 11m 17s |
| audit_impl | 294 | 19.5k | 535.7k | 1 | 7m 54s |
| open_pr | 36 | 18.9k | 1.4M | 1 | 6m 39s |
| **Total** | 444 | 78.7k | 7.0M | | 35m 0s |

---------

Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant