From 6ff9116a23620d5a2a579e4ebd587c6751484621 Mon Sep 17 00:00:00 2001 From: Patrick Taylor <1963845+pstaylor-patrick@users.noreply.github.com> Date: Sun, 21 Jun 2026 05:51:26 -0500 Subject: [PATCH 1/2] feat(pst:refactor): new skill -- standalone Fowler smell pass + refactor commit Standalone /pst:refactor skill that applies the 16-smell rubric from MAINTAINABILITY.md against a diff, file, or glob. Mirrors rule 23 as an explicit on-demand invocation: Haiku detects smells, presents findings, then optionally spawns a Sonnet implementer in an isolated worktree to apply behavior-preserving fixes as a separate refactoring commit (two hats). Co-Authored-By: Claude Sonnet 4.6 Claude-Session: https://claude.ai/code/session_019BCr5Qxi8jXnG2Rr7zdkw1 --- install.sh | 1 + skills/pst:refactor/SKILL.md | 110 +++++++++++++++++++++++++++++++++++ 2 files changed, 111 insertions(+) create mode 100644 skills/pst:refactor/SKILL.md diff --git a/install.sh b/install.sh index fce3f48..529a6e9 100755 --- a/install.sh +++ b/install.sh @@ -38,6 +38,7 @@ SKILLS=( "pst:patch" "pst:push" "pst:python-refactor" + "pst:refactor" "pst:quality-gates" "pst:qa" "pst:rebase" diff --git a/skills/pst:refactor/SKILL.md b/skills/pst:refactor/SKILL.md new file mode 100644 index 0000000..4ccd7a1 --- /dev/null +++ b/skills/pst:refactor/SKILL.md @@ -0,0 +1,110 @@ +--- +name: pst:refactor +description: Standalone Fowler maintainability smell pass on the current diff, a target path, or any staged/unstaged changes. Applies the 16-smell rubric from MAINTAINABILITY.md, presents findings, and optionally implements all fixes as a behavior-preserving refactoring commit (two hats, separate commit per rule 15). Use when you want a dedicated refactor pass independent of any feature work in progress. +argument-hint: "[path-or-glob | --diff | --staged | --report-only]" +allowed-tools: Bash, Read, Edit, Write, Grep, Glob, Agent, AskUserQuestion +--- + +# /pst:refactor + +Dedicated Fowler maintainability pass. Always the second hat -- behavior stays +identical, only structure improves. Use at any point in a session, independent +of feature work in progress. + +## Input resolution + +Determine what to review from `$ARGUMENTS`: + +| Argument | Target | +| --------------------- | ------------------------------------------------------------------------------------- | +| `path`, file, or glob | those files (read in full) | +| `--diff` | `git diff HEAD` + full file content for each changed file | +| `--staged` | `git diff --cached` + full file content for each changed file | +| _(none)_ | run `pst-smell.rb`; if "required", use `git diff HEAD`; if "skipped", report and stop | + +## Steps + +### 1. Collect target + +Run the input resolution above. For diff-based targets, always read the +**full file** for every changed path -- diffs alone lose the context needed +to spot smells like Duplicated Code or Feature Envy that span unchanged lines. + +### 2. Smell pass (Haiku, background) + +Spawn a background Haiku agent (`model: haiku`, `effort: low`): + +- Load `MAINTAINABILITY.md` from the pst skill directory: + ```sh + cat "$(dirname "$(cat ~/.claude/pst/ledger-path)")/../../MAINTAINABILITY.md" + ``` + Or read it directly: `skills/pst/MAINTAINABILITY.md` in the repo, or + `~/.claude/commands/pst.md`'s sibling directory resolved at runtime. +- Pass the target file content. +- Instruction: for each of the 16 canonical smells, state whether evidence + exists in the target. For each hit: smell name, specific code excerpt or + line reference, recommended refactoring. Skip smells with no evidence. + Return structured output. + +Schema: + +```json +{ + "smells": [ + { + "name": "string", + "evidence": "string (excerpt or line ref)", + "refactoring": "string" + } + ], + "clean": "boolean" +} +``` + +### 3. Gate + +- If `clean: true` (no smells): report "No smells detected in target." and stop. +- Otherwise: present findings as a flat list (smell, evidence, fix) under a + `## Smell findings` header. + +If `--report-only` was passed, stop here. + +### 4. Plan gate (foreground) + +Present the smell count and top finding as a one-line summary. Use +`AskUserQuestion` with: **Implement all fixes in a separate refactoring commit?** +(Yes / Report only). Treat no objection as Yes. + +### 5. Implement (Sonnet, isolated worktree) + +Spawn a background Sonnet implementer (`model: sonnet`, `effort: medium`, +`isolation: worktree`): + +- Target files + smell findings. +- Instruction: apply each refactoring as a **behavior-preserving** transformation. + Do not change any observable behavior. Run existing tests if present; report + pass/fail. If a refactoring would break a test, skip it and note it. Produce + one clean commit: + + ``` + refactor: + + Co-Authored-By: Claude Sonnet 4.6 + ``` + + Identity per rule 10. + +### 6. Report + +After the worktree agent completes: list which smells were fixed, which were +skipped (with reason), and whether tests passed. If tests failed on any fix, +name the fix and the failing test. + +## Notes + +- MAINTAINABILITY.md is the rubric. All 16 smells are in scope regardless of + file size or diff length (rule 23: a small diff does not exempt a change). +- Two-hats rule (rule 15): if any behavior change crept into the refactoring + commit, reject it and re-run with a cleaner scope. +- For targets > 500 LOC, prioritize the highest-signal files (those with the + most smell hits) rather than processing everything at once. From 755217fda2d8241076c20fc2a83cc0d5b502d980 Mon Sep 17 00:00:00 2001 From: Patrick Taylor <1963845+pstaylor-patrick@users.noreply.github.com> Date: Sun, 21 Jun 2026 05:54:57 -0500 Subject: [PATCH 2/2] refactor(pst:refactor): comprehensive whole-codebase Opus analysis + parallel Sonnet fixers - Scope changed: default is entire codebase, not current diff - Analysis tier changed: Opus (was Haiku) -- whole-codebase maintainability audit warrants deep reasoning - Implementation: parallel Sonnet agents per file in isolated worktrees - Fix adversarial findings: correct MAINTAINABILITY.md path (one .., not two), explicit git add+commit in implementer instructions, remove invalid effort param, fix --report-only handling, remove pst-smell.rb dependency - Co-author trailer included in implementer commit instructions Co-Authored-By: Claude Sonnet 4.6 Claude-Session: https://claude.ai/code/session_019BCr5Qxi8jXnG2Rr7zdkw1 --- skills/pst:refactor/SKILL.md | 163 +++++++++++++++++++---------------- 1 file changed, 89 insertions(+), 74 deletions(-) diff --git a/skills/pst:refactor/SKILL.md b/skills/pst:refactor/SKILL.md index 4ccd7a1..8200fc0 100644 --- a/skills/pst:refactor/SKILL.md +++ b/skills/pst:refactor/SKILL.md @@ -1,110 +1,125 @@ --- name: pst:refactor -description: Standalone Fowler maintainability smell pass on the current diff, a target path, or any staged/unstaged changes. Applies the 16-smell rubric from MAINTAINABILITY.md, presents findings, and optionally implements all fixes as a behavior-preserving refactoring commit (two hats, separate commit per rule 15). Use when you want a dedicated refactor pass independent of any feature work in progress. -argument-hint: "[path-or-glob | --diff | --staged | --report-only]" +description: Comprehensive Fowler maintainability refactor pass across the entire codebase. An Opus background agent analyzes all code files against the 16-smell rubric in MAINTAINABILITY.md, produces a prioritized smell inventory, then parallel Sonnet agents fix each file in isolated worktrees (behavior-preserving, two hats per rule 15). Run any time you want human maintainability optimized across the board, independent of feature work. +argument-hint: "[path-or-glob] [--report-only]" allowed-tools: Bash, Read, Edit, Write, Grep, Glob, Agent, AskUserQuestion --- # /pst:refactor -Dedicated Fowler maintainability pass. Always the second hat -- behavior stays -identical, only structure improves. Use at any point in a session, independent -of feature work in progress. +Comprehensive Fowler code-smell pass. Opus analyzes the full codebase (or a +scoped path), Sonnet fixes each smell in an isolated worktree. Always the +second hat -- behavior stays identical, only structure improves. -## Input resolution +## 1. Setup -Determine what to review from `$ARGUMENTS`: +Resolve the MAINTAINABILITY.md rubric path at runtime: -| Argument | Target | -| --------------------- | ------------------------------------------------------------------------------------- | -| `path`, file, or glob | those files (read in full) | -| `--diff` | `git diff HEAD` + full file content for each changed file | -| `--staged` | `git diff --cached` + full file content for each changed file | -| _(none)_ | run `pst-smell.rb`; if "required", use `git diff HEAD`; if "skipped", report and stop | +```sh +LEDGER="$(cat ~/.claude/pst/ledger-path 2>/dev/null)" +MAINT="$(dirname "$LEDGER")/../MAINTAINABILITY.md" +cat "$MAINT" +``` + +Determine the scan root from `$ARGUMENTS`: + +- **Path or glob provided**: scan only that subtree. +- **No argument**: scan the entire repo working tree. -## Steps +Discover all code files (exclude lockfiles, generated files, docs): -### 1. Collect target +```sh +git ls-files | grep -Ev '\.(lock|snap|min\.(js|css)|pb\.go|pb_test\.go)$' \ + | grep -Ev '^(docs?/|\.github/|dist/|build/)' \ + | grep -E '\.(rb|py|ts|tsx|js|jsx|go|rs|java|kt|swift|ex|exs|cs|cpp|c|h)$' +``` -Run the input resolution above. For diff-based targets, always read the -**full file** for every changed path -- diffs alone lose the context needed -to spot smells like Duplicated Code or Feature Envy that span unchanged lines. +## 2. Opus smell analysis (background) -### 2. Smell pass (Haiku, background) +Spawn one background Opus agent (`model: opus`) with: -Spawn a background Haiku agent (`model: haiku`, `effort: low`): +- The full content of `MAINTAINABILITY.md` (read via the path resolved above). +- The list of code files from step 1. +- Read every file in the list. +- Instruction: -- Load `MAINTAINABILITY.md` from the pst skill directory: - ```sh - cat "$(dirname "$(cat ~/.claude/pst/ledger-path)")/../../MAINTAINABILITY.md" ``` - Or read it directly: `skills/pst/MAINTAINABILITY.md` in the repo, or - `~/.claude/commands/pst.md`'s sibling directory resolved at runtime. -- Pass the target file content. -- Instruction: for each of the 16 canonical smells, state whether evidence - exists in the target. For each hit: smell name, specific code excerpt or - line reference, recommended refactoring. Skip smells with no evidence. - Return structured output. - -Schema: - -```json -{ - "smells": [ + You are a maintainability auditor. For each file, identify which of the 16 + canonical Fowler code smells are present. For every finding, record: + - file path and approximate line range + - smell name + - one-sentence description of the specific instance + - recommended refactoring (Extract Function, Move Method, etc.) + + Rank findings by impact: smells that would most reduce future change cost + come first. Smells that require a behavior change to fix (e.g. API redesign + that breaks callers) are out of scope -- skip them. + + Return a JSON array: + [ { - "name": "string", - "evidence": "string (excerpt or line ref)", - "refactoring": "string" + "file": "path/to/file.rb", + "smell": "Duplicated Code", + "lines": "42-61", + "detail": "Login and signup both inline-validate email format.", + "fix": "Extract Function: extract validate_email into a shared helper." } - ], - "clean": "boolean" -} -``` + ] + ``` -### 3. Gate +## 3. Present findings and gate -- If `clean: true` (no smells): report "No smells detected in target." and stop. -- Otherwise: present findings as a flat list (smell, evidence, fix) under a - `## Smell findings` header. +Group findings by file. Present as a summary table: -If `--report-only` was passed, stop here. +``` +File Smells Top smell +src/auth/session.rb 3 Duplicated Code (lines 42-61) +src/api/handler.ts 2 Long Function (lines 120-180) +... +``` -### 4. Plan gate (foreground) +If no findings: report "No smells detected." and stop. -Present the smell count and top finding as a one-line summary. Use -`AskUserQuestion` with: **Implement all fixes in a separate refactoring commit?** -(Yes / Report only). Treat no objection as Yes. +If `--report-only` was passed, stop here. -### 5. Implement (Sonnet, isolated worktree) +Use `AskUserQuestion`: **Apply all fixes in isolated worktrees?** (Yes / Report +only). Treat no objection as Yes. -Spawn a background Sonnet implementer (`model: sonnet`, `effort: medium`, -`isolation: worktree`): +## 4. Parallel Sonnet implementers (isolated worktrees) -- Target files + smell findings. -- Instruction: apply each refactoring as a **behavior-preserving** transformation. - Do not change any observable behavior. Run existing tests if present; report - pass/fail. If a refactoring would break a test, skip it and note it. Produce - one clean commit: +Group findings by file. Spawn one background Sonnet agent per file +(`model: sonnet`, `isolation: worktree`) with: - ``` - refactor: +- The file content. +- The smell findings for that file. +- Instruction: - Co-Authored-By: Claude Sonnet 4.6 + ``` + Apply each refactoring listed below to this file. Rules: + - Behavior-preserving only. Do not change any observable behavior. + - Run existing tests if a test runner is available; skip any fix that + breaks a test and note it. + - After all fixes: stage the changed file and commit: + git add + git commit -m "refactor(): " + Include the co-author trailer: + Co-Authored-By: Claude Sonnet 4.6 + - If no fix is safe to apply, commit nothing and return a skip reason. ``` - Identity per rule 10. +## 5. Report -### 6. Report +After all worktree agents complete: -After the worktree agent completes: list which smells were fixed, which were -skipped (with reason), and whether tests passed. If tests failed on any fix, -name the fix and the failing test. +- List each file: smells fixed, smells skipped (with reason), test result. +- If any fix broke a test, name the fix and the failing test. +- State total smells fixed vs. skipped. ## Notes -- MAINTAINABILITY.md is the rubric. All 16 smells are in scope regardless of - file size or diff length (rule 23: a small diff does not exempt a change). -- Two-hats rule (rule 15): if any behavior change crept into the refactoring - commit, reject it and re-run with a cleaner scope. -- For targets > 500 LOC, prioritize the highest-signal files (those with the - most smell hits) rather than processing everything at once. +- Rule 15 two-hats: every commit from this skill is a pure refactor. If a + fix requires a behavior change, skip it and surface it as a follow-up. +- Rule 7 applies if you intend to merge the result via PR: adversarial review + before merge. +- For very large codebases (> 50 files), run with a scoped path first: + `/pst:refactor src/auth` then widen.