diff --git a/skills/dispatch-worktree-task/SKILL.md b/skills/dispatch-worktree-task/SKILL.md new file mode 100644 index 0000000..17a0935 --- /dev/null +++ b/skills/dispatch-worktree-task/SKILL.md @@ -0,0 +1,131 @@ +--- +name: dispatch-worktree-task +description: "Use when a task has a clear implementation plan and should be executed by a subagent in an isolated git worktree. Triggers on phrases like: dispatch this, send to a subagent, work on this in a worktree, dispatch to worktree, background task, isolated execution." +--- + +# Dispatch Worktree Task + +## Overview + +Packages a task with a clear plan, creates an isolated git worktree, and dispatches a subagent to implement, validate, verify, and commit in the background. The main session reviews, pushes, and creates the PR. + +**Core principle:** Plan in the main session, execute in isolation, verify before pushing. + +## When to Use + +- Implementation plan is written and approved +- Task is self-contained (no back-and-forth needed) +- You want background execution while continuing other work + +**When NOT to use:** +- Task requires interactive clarification mid-implementation +- Changes span multiple repos or need coordination with other branches +- Quick single-file fix (just do it inline) + +## Workflow + +```dot +digraph dispatch { + rankdir=TB; + node [shape=box]; + + plan [label="1. Prepare implementation plan"]; + worktree [label="2. Create git worktree\non new branch"]; + dispatch [label="3. Dispatch subagent\n(implement, verify, commit)"]; + review [label="4. Review diff,\npush + PR"]; + cleanup [label="5. Remove worktree"]; + + plan -> worktree -> dispatch -> review -> cleanup; +} +``` + +### 1. Prepare the implementation plan + +Write a detailed plan that includes: +- **Context** — why this change, what was tried/rejected +- **Step-by-step plan** — numbered, specific files and code patterns +- **Pre-commit validation** — exact commands to run (read from project's CLAUDE.md) +- **Reproduction command** — a command that exercises the original scenario (bug or feature). Adapt paths, use temp dirs, etc., but exercise the same behavior. The subagent must run it after implementing and confirm the fix works. +- **What NOT to do** — anti-patterns to avoid + +### 1b. Verify plan completeness + +Before proceeding, confirm the plan includes all required sections: + +- [ ] **Context** — why this change, what was tried/rejected +- [ ] **Step-by-step plan** — numbered, specific files and code patterns +- [ ] **Pre-commit validation** — exact commands from project's CLAUDE.md +- [ ] **Reproduction command** — a command that exercises the original scenario. For refactorings or structural changes where there's no behavioral scenario, reproduction = "existing tests pass and restructured code compiles" — state this explicitly rather than omitting reproduction. +- [ ] **Anti-patterns** — what NOT to do + +If any section is missing or vague, fill it in before creating the worktree. + +### 2. Create worktree + +```bash +git worktree add .worktrees/ -b main +``` + +Use `.worktrees/` directory. Check that `.worktrees/` is in `.gitignore`. If not, add it before creating the worktree. Branch name should match the work (e.g., `fix/cache-lock-fs4`). + +> **Stale worktree/branch:** If the branch or worktree already exists from a previous attempt, remove them first (`git worktree remove .worktrees/` and `git branch -D `) or reuse after verifying the worktree state is clean (`git status` in the worktree). + +> **Why manual worktrees instead of `isolation: "worktree"`?** The branch is the deliverable — it gets pushed and PR'd. You need a named branch you control. For fan-out/fan-in where you cherry-pick temporary commits back, see `review-and-fix`. + +### 3. Dispatch subagent + +Use the Task tool with these settings: + +| Parameter | Value | +|-----------|-------| +| `subagent_type` | `general-purpose` | +| `mode` | `bypassPermissions` | +| `run_in_background` | `true` | + +**Prompt must include:** +- Worktree path and branch name +- **Explicit `cd ` as the subagent's first action.** The Task tool starts subagents in the parent's working directory, not the worktree. +- Full implementation plan +- Pre-commit validation commands (from project CLAUDE.md) +- **Reproduction command** — a command that exercises the same scenario as the original report. Adapt for the worktree context (temp output dirs, etc.) but verify the same behavior. +- Commit message to use (conventional commits) +- Project conventions (no co-authored-by lines, etc.) + +**Subagent execution order:** +1. Implement the change +2. Run pre-commit validation (from project CLAUDE.md) +3. **Reproduce the original scenario** — build and run a command that exercises the same behavior from the original report. Adapt for safety (temp dirs, non-destructive variants) but verify the same scenario. +4. Commit (do NOT push — the main session handles push after review) + +**On failure:** If the implementation is incomplete, the plan doesn't match reality, or validation/reproduction fails — the subagent must NOT commit. Instead, report what happened: what was attempted, what failed, and what state the worktree is in. The main session will decide next steps. + +If reproduction fails but the fix is otherwise correct, the subagent should fix the issue and re-verify. If it cannot fix it, do not commit — report the failure. + +### 4. Review, push, and create PR + +When the agent reports back: +- Show `git log main.. --oneline` and `git diff --stat main..` +- Read the actual diff for the files listed in the implementation plan +- Confirm the reproduction command passed in the agent's output +- Push the branch: `git push -u origin ` +- Create the PR (or remind the user) + +### 5. Clean up worktree + +After the PR is created, remove the worktree: + +```bash +git worktree remove .worktrees/ +``` + +## Common Mistakes + +| Mistake | Fix | +|---------|-----| +| No reproduction command in prompt | Always include a command that exercises the original scenario | +| Reproduction command is just "run tests" | Tests verify correctness; reproduction verifies the original complaint is resolved | +| Reproduction command is destructive | Adapt for safety — use temp dirs, non-destructive variants — but verify the same behavior | +| Vague subagent prompt | Paste the full plan — subagents have no prior context | +| Not verifying the diff | Always read the key file diffs before creating the PR | +| Skipping pre-commit in prompt | Agent won't know to run validation unless told | +| Forgetting the PR | Always create or remind about PR after reviewing | diff --git a/skills/iterative-review-fix/SKILL.md b/skills/iterative-review-fix/SKILL.md new file mode 100644 index 0000000..546820f --- /dev/null +++ b/skills/iterative-review-fix/SKILL.md @@ -0,0 +1,190 @@ +--- +name: iterative-review-fix +description: "Use when code needs repeated review-fix cycles until clean. Dispatches a reviewer agent, then a fixer agent, commits, and repeats until findings converge to zero or a cap is reached. Triggers on: iterate until clean, review fix loop, converge, repeated review, keep fixing until done, iterative review." +--- + +# Iterative Review-Fix Loop + +## Overview + +Serial review-fix loop: dispatch a read-only reviewer, then a fixer, commit, repeat until findings hit zero, plateau, or an iteration cap. Designed for convergence, not throughput. + +**Core principle:** One reviewer, one fixer, per iteration. Each pass commits directly on the current branch. Commits are revert points. + +## When to Use + +- Code needs multiple rounds of review to converge on clean +- User says "iterate until clean", "keep fixing", or "review fix loop" +- A single review-and-fix pass is insufficient (findings cascade) + +**When NOT to use:** +- Single-pass review with parallel dispatch — use `review-and-fix` +- Research-backed review — use `review-with-research` +- One specific bug fix — just fix it inline + +## Workflow + +### 1. Capture Inputs + +Determine three things before starting the loop: + +**Review focus** — what to look for. The user provides criteria inline or as a file path. Read the file if a path is given. + +**Scope** — what to review. Default: branch diff vs main. +```bash +git diff main --stat +git log main..HEAD --oneline +``` +If the user specifies a directory or file list, use that instead. + +**Iteration cap** — default 5. The user can override. + +Read the project's `CLAUDE.md` for pre-commit validation commands — the fixer needs these. + +### 2. Loop (iterations 1..cap) + +#### 2a. Dispatch reviewer + +Launch a read-only subagent: + +| Parameter | Value | +|-----------|-------| +| `subagent_type` | `Explore` | +| `run_in_background` | `true` | + +**Reviewer prompt:** + +``` +You are performing review pass {N} of an iterative review-fix cycle. + +## Review Focus +{criteria — inline text or file contents} + +## Scope +{branch diff output, directory listing, or file list} + +## Output Format +For each issue found, produce: + +### Finding: [stable title — same title if same issue persists across passes] +**Severity:** high | medium | low +**File:** `path/to/file:line` +**Description:** [what's wrong and why] +**Fix approach:** [specific steps to fix] + +If no issues found, respond with exactly: NO_FINDINGS +``` + +**Stable titles are critical.** The main session compares findings across passes to detect plateaus. If the reviewer uses a different title for the same issue, plateau detection breaks. Instruct the reviewer to title findings by the problem, not the pass number. + +#### 2b. Parse findings + +Three outcomes: + +1. **Zero findings** (`NO_FINDINGS`) — break the loop, report clean. +2. **Same findings as previous pass** — plateau detected. The fixer is stuck on these issues. Break the loop, report the stuck findings. +3. **New or different findings** — proceed to fixer. + +**Plateau detection:** Compare finding titles (not full descriptions) between consecutive passes. If the set of titles is identical, it's a plateau. If a subset overlaps but new findings appeared, it's not a plateau — the fixer made partial progress. + +#### 2c. Dispatch fixer + +Launch a general-purpose subagent: + +| Parameter | Value | +|-----------|-------| +| `subagent_type` | `general-purpose` | +| `mode` | `bypassPermissions` | +| `run_in_background` | `true` | + +**Fixer prompt:** + +``` +You are fixing issues found during review pass {N}. + +## Findings +{paste all findings from reviewer — full text, not just titles} + +## Validation +After making all fixes, run these commands and confirm they pass: +{pre-commit validation commands from project CLAUDE.md} + +## Commit +If all fixes are made and validation passes, create a single commit: +fix: address review findings (pass {N}) + +Do NOT add co-authored-by lines. +Do NOT modify files beyond what the findings require. +Do NOT commit if validation fails — report the failure instead. +``` + +**If the fixer reports validation failure:** stop the loop immediately. Report the failure to the user with the fixer's output. Do not continue to the next pass — a broken build state will cascade. + +#### 2d. Increment and continue + +After a successful fixer pass, increment the counter and loop back to 2a. + +### 3. Report + +When the loop exits (clean, plateau, or cap), produce a summary: + +**Clean exit:** +``` +Review-fix loop completed in {N} passes. +Pass 1: {count} findings — fixed and committed +Pass 2: {count} findings — fixed and committed +Pass 3: clean — no findings +``` + +**Plateau exit:** +``` +Review-fix loop plateaued after {N} passes. +Pass 1: {count} findings — fixed and committed +Pass 2: {count} findings — fixed and committed (partial overlap with pass 1) +Pass 3: {count} findings — same as pass 2 (plateau) + +Stuck findings: +- [Finding title]: [brief description] +- [Finding title]: [brief description] + +These issues may require manual intervention or a different approach. +``` + +**Cap exit:** +``` +Review-fix loop hit iteration cap ({cap}) with {count} findings remaining. +[Per-pass summary as above] + +Remaining findings: +[List findings from the last pass] +``` + +## Key Conventions + +- **Serial, not parallel.** One reviewer, one fixer per iteration. Simplicity over throughput. +- **Direct on branch.** No worktrees. Each iteration commits directly. Commits are revert points. +- **Fully autonomous.** No user gate between iterations. Runs until a stopping condition. +- **Fixer gets full findings each pass.** Not deltas. The fixer doesn't know what previous passes did. +- **One commit per pass.** Keeps history clean and gives one revert point per iteration. +- **Pre-commit validation is the fixer's job.** If validation fails, the fixer does NOT commit. +- **No co-authored-by lines.** Encode this in every fixer prompt. + +## Stopping Conditions + +| Condition | Action | +|-----------|--------| +| Reviewer returns `NO_FINDINGS` | Break — report clean | +| Finding titles identical to previous pass | Break — report plateau with stuck findings | +| Iteration counter reaches cap | Break — report cap hit with remaining findings | +| Fixer reports validation failure | Break — report failure with fixer output | + +## Common Mistakes + +| Mistake | Fix | +|---------|-----| +| Reviewer uses different titles for same issue across passes | Prompt reviewer to use stable titles based on the problem | +| Fixer modifies unrelated code | Prompt explicitly: only fix what's in the findings | +| Skipping plateau detection | Always compare finding titles between consecutive passes | +| Continuing after validation failure | Stop immediately — broken builds cascade | +| Not reading project CLAUDE.md first | Pre-commit commands vary per project | +| Running in parallel with other work | This skill commits directly on the branch — no concurrent modifications | diff --git a/skills/review-and-fix/SKILL.md b/skills/review-and-fix/SKILL.md new file mode 100644 index 0000000..dd83827 --- /dev/null +++ b/skills/review-and-fix/SKILL.md @@ -0,0 +1,185 @@ +--- +name: review-and-fix +description: "Use when the user wants to review existing work on a branch, find issues, and dispatch parallel fixes that get cherry-picked back. Triggers include: review and fix, review and fix this branch, find and fix issues, dispatch fixes, parallel fix, review then fix, fix all issues, code review with fixes." +--- + +# Review and Fix: Parallel Code Review with Automated Dispatch + +Review code on a branch using a domain skill, document all findings, dispatch each fix to a parallel worktree subagent, cherry-pick results back, verify, and push. + +--- + +## Workflow + +### 1. Understand the Scope + +Determine what to review. Default is the current branch's diff against `main`. + +```bash +git log main..HEAD --oneline +git diff main --stat +``` + +Check if the user wants to review: +- The full branch diff (default) +- A specific module or directory +- Only recent commits + +Read the project's `CLAUDE.md` for pre-commit validation commands — you'll need these later. + +### 2. Review with a Domain Skill + +Apply domain knowledge from the relevant skill's reference files (e.g., the rust skill's anti-patterns and style references). Do not invoke the Skill tool — stay in this workflow. + +- **Rust** — use the `rust` skill's reference files +- **TypeScript** — use general review or a TS skill's reference files if available +- **Other** — do a thorough general code review + +Focus the review on the branch diff, not the entire codebase. Look for: +- Bugs and correctness issues +- Performance problems +- Design issues (wrong abstraction, coupling, missing error handling) +- Minor issues (naming, style, redundancy) + +Produce a structured list of findings. + +**If the review finds no issues, report a clean review to the user and stop.** Do not proceed to dispatch. + +### 3. Document Findings + +Format each finding as: + +```markdown +## Finding N: [Title] + +**Severity:** bug | perf | design | minor +**Files:** `path/to/file.rs:42`, `path/to/other.rs:17` + +**Description:** +[What the issue is and why it matters] + +**Fix approach:** +[Specific steps to fix. Include before/after code snippets when possible] +``` + +Present the findings to the user. Ask if they want to: +- Fix all findings +- Fix only specific findings (by number) +- Skip the dispatch and keep the review document + +If the user chooses to **skip dispatch**, write findings to `_review-findings.md` in the repo root and stop. Otherwise, keep findings in context (the agent already has them in memory) and continue. + +### 4. Create Task List + +Create one task per finding using `TaskCreate`. Include the finding number, title, severity, and file list in each task description. Step 5 uses this as the dispatch manifest — tasks are marked in-progress as agents launch and completed as cherry-picks land. + +### 5. Dispatch Parallel Worktree Agents + +Launch one `general-purpose` subagent per fix. Use `isolation: "worktree"`, `run_in_background: true`, and `mode: "bypassPermissions"`. + +> **Why `isolation: "worktree"` instead of manual worktrees?** These worktrees are temporary scaffolding — you cherry-pick the commits back by SHA and the worktree is discarded. You don't need named branches. For backgrounding a single task where the branch is the deliverable, see `dispatch-worktree-task`. + +**Dispatch order:** Least-coupled fixes first. If two fixes touch the same file, they cannot run in parallel — cherry-pick the first fix BEFORE creating the second worktree so the second agent branches from the updated state. + +**Wave strategy:** For more than 4 findings, dispatch in waves of 3–4. Complete the wave, cherry-pick all results, then dispatch the next wave. This gives the next wave a more complete starting state and reduces conflicts. Mark tasks in-progress as agents launch. + +**Agent prompt template:** + +``` +You are fixing a specific issue found during code review. + +## Task +[Finding title and description from the findings doc] + +## Files to Modify +[Exact file paths and line numbers] + +## Fix Approach +[Step-by-step fix instructions, with before/after code when available] + +## Validation +After making the fix, run these commands and ensure they all pass: +[Pre-commit validation commands from project CLAUDE.md] + +## Commit +Create a single commit with this message: +[Conventional commit message, e.g., fix(module): description] + +Do NOT add any co-authored-by lines. +Do NOT modify any files beyond what is needed for this fix. +Do NOT add comments, docstrings, or formatting changes to code you didn't change. +``` + +### 6. Cherry-Pick Results + +As each agent completes (or after each wave): + +1. The Task tool result includes the worktree branch name. Use it to find the commit SHA: + ```bash + git log --oneline -1 + ``` +2. Cherry-pick onto the working branch: + ```bash + git cherry-pick + ``` +3. If the cherry-pick conflicts: + - Check if the conflict is trivial (overlapping context lines) — resolve manually. + - If the conflict is substantive (two fixes changed the same logic), re-dispatch one of them with the other's changes already applied. +4. Mark the corresponding task as completed. + +**Cherry-pick in dependency order:** If fixes were dispatched with noted dependencies, cherry-pick the prerequisite first. + +### 7. Verify + +Run pre-commit validation after each cherry-pick (not just at the end) to catch failures early: + +```bash +# Example for Rust — use whatever the project's CLAUDE.md specifies +cargo fmt --check +cargo clippy -- -D warnings +cargo test +``` + +If a check fails after a cherry-pick: +1. Revert the last cherry-pick: `git cherry-pick --abort` or `git reset --hard HEAD~1` +2. Identify the conflict between the fix and the current branch state +3. Re-dispatch that specific fix with the current branch state as context + +After all cherry-picks land, run the full validation suite once more to confirm the integrated result passes. + +### 8. Push and Clean Up + +1. Show the user what will be pushed: + ```bash + git log main..HEAD --oneline + ``` + Along with validation results. Ask the user to confirm before pushing. +2. Push the branch: + ```bash + git push + ``` +3. If a PR exists for this branch, update its description with a "Code review fixes" section listing each finding and its resolution. + +--- + +## Key Conventions + +- **Always read the project's CLAUDE.md first.** Pre-commit validation commands vary per project. +- **Use `mode: "bypassPermissions"` for worktree agents.** They're isolated and can't affect the main tree. +- **Cherry-pick in dependency order.** Least-coupled first, then fixes that depend on earlier ones. +- **If cherry-pick conflicts, resolve or re-dispatch.** Don't force through broken merges. +- **No co-authored-by lines.** Per user preference — encode this in every agent prompt. +- **Clean up after yourself.** Mark tasks complete, report final status. + +--- + +## Output Quality Checklist + +Before reporting completion to the user: + +- [ ] All dispatched fixes have been cherry-picked +- [ ] Pre-commit validation passes on the integrated result +- [ ] User confirmed push after seeing `git log` and validation results +- [ ] Branch has been pushed +- [ ] PR description updated (if a PR exists) +- [ ] Task list shows all findings resolved diff --git a/skills/review-with-research/SKILL.md b/skills/review-with-research/SKILL.md new file mode 100644 index 0000000..8698177 --- /dev/null +++ b/skills/review-with-research/SKILL.md @@ -0,0 +1,140 @@ +--- +name: review-with-research +description: "Use when asked to review a branch, PR, or implementation where the quality of the approach itself is in question — not just code style, but whether the technique or design is sound. Also use when the user expresses suspicion about an approach and wants independent verification, or says: look up, research, what do others do, good practice, validate approach." +--- + +# Review With Research + +## Overview + +Reviews a branch by dispatching a code-review subagent in a worktree alongside a research subagent, then synthesizes their findings into an evidence-based verdict. + +**Core principle:** Review in isolation (worktree), research the domain, compare against what established tools do. Never rubber-stamp, never just echo the user's hunch. + +## When to Use + +- User questions whether an approach is sound +- Review involves a technique worth verifying (locking, caching, auth, crypto, concurrency) +- User explicitly asks to research best practices + +**Skip for:** pure style reviews or quick thumbs-up on small changes. + +## Workflow + +```dot +digraph review { + rankdir=TB; + node [shape=box]; + + worktree [label="0. Create worktree\nfor the branch"]; + review [label="1. Dispatch review subagent\n(read diff + baseline in worktree)"]; + research [label="2. Dispatch research subagent\n(ecosystem, libraries, how major tools do it)"]; + parallel [label="Run 1 and 2\nin parallel" shape=diamond]; + synthesize [label="3. Synthesize + verdict:\ncompare impl vs best practices"]; + cleanup [label="4. Remove worktree"]; + + worktree -> parallel; + parallel -> review; + parallel -> research; + review -> synthesize; + research -> synthesize; + synthesize -> cleanup; +} +``` + +### 0. Create a worktree + +```bash +git worktree add --detach .worktrees/review- +``` + +> **Why `--detach`?** If `` is already checked out (e.g., it's the current branch), `git worktree add` will refuse. Using `--detach` creates the worktree in detached HEAD state, avoiding the "already checked out" error. The worktree still has the branch's content. + +Check that `.worktrees/` is in `.gitignore`. If not, add it before creating the worktree. This keeps the main session on its current branch. Clean up after the review. + +### 0.5. Identify research topic + +Before dispatching subagents, skim the diff briefly to identify the technique or domain to research: + +```bash +git diff main.. --stat +git diff main.. | head -200 +``` + +This is a quick read, not the full review — just enough to know the research topic (e.g., "file locking with advisory locks", "JWT refresh token rotation", "concurrent HashMap sharding"). If the diff is empty, short-circuit: report "nothing to review" and stop. + +### 1. Dispatch review subagent (parallel with step 2) + +Dispatch an `Explore` subagent (read-only) into the worktree. + +| Parameter | Value | +|-----------|-------| +| `subagent_type` | `Explore` | +| `run_in_background` | `true` | + +The subagent should: +- `cd ` as its first action (the Task tool starts subagents in the parent's working directory, not the worktree) +- Read the full diff (`git diff main..`) +- Use `git show main:` to read baseline files on main for context +- Read files directly from the worktree path for the branch version +- Summarize what the branch does and flag any concerns + +**Subagent prompt template:** +> You are reviewing branch ``. First, `cd `. Then read `git diff main..` and examine the changed files. Use `git show main:` to read baseline files on main. Read files directly from the worktree for the branch version. Produce a detailed review: what the branch does, any correctness/design concerns with line numbers, and questions about the approach. Do NOT write code. + +### 2. Dispatch research subagent (parallel with step 1) + +| Parameter | Value | +|-----------|-------| +| `subagent_type` | `general-purpose` | +| `model` | `"sonnet"` | +| `run_in_background` | `true` | + +Dispatch a `general-purpose` subagent to: +- Find established libraries/packages for the problem domain +- Check how major tools in the ecosystem solve it +- Identify known failure modes of the approach used + +**Subagent prompt template:** +> Research best practices for [topic] in [language/ecosystem]. Find: (1) common libraries, (2) tradeoffs between approaches, (3) what well-known tools use, (4) known failure modes of [technique]. Do NOT write code. + +### 3. Synthesize and deliver verdict + +Compare the review subagent's findings against the research and produce a verdict with this structure: + +- **What the branch does** — neutral summary +- **What the ecosystem does** — researched evidence +- **Specific problems** — line numbers and failure modes +- **Recommendation** — accept, reject, or revise with concrete alternative + +**Reconciliation guidance:** +- If review and research **agree**, state the convergence and cite both. +- If review and research **contradict**, present both perspectives with specific evidence. Don't silently pick one — the user needs to see the tension. +- If research is **inconclusive** (topic too niche, conflicting sources), weight code review more heavily, evaluate on first principles, and note the gap explicitly. + +**Handoff:** If the recommendation is "revise" and the user wants to proceed with fixes, hand off to `review-and-fix`. + +### 4. Clean up + +```bash +git worktree remove .worktrees/review- +``` + +## Key Rules + +| Rule | Why | +|------|-----| +| Don't just confirm user suspicions | They want independent verification, not validation | +| Don't just contradict either | If the approach is fine, say so with evidence | +| Cite specifics, not vibes | "PID reuse is a known failure mode" beats "PIDs seem unreliable" | +| Always suggest the alternative | Rejection without a path forward isn't useful | +| Review in a worktree, not main | Don't switch branches on the main session | + +## Common Mistakes + +| Mistake | Fix | +|---------|-----| +| Reviewing on the main thread's branch | Always create a worktree — keep main session clean | +| Reviewing from memory, skipping research | Always dispatch research subagent — training data may be stale | +| Only reading the diff, not main | Need baseline context to judge what changed | +| Agreeing with user before researching | Research first, form opinion second |