Refactoring

---
name: refactoring
description: Use when restructuring existing code without changing behavior — "refactor", "clean up this module", "reduce tech debt/duplication", "split this god class/function", "improve code quality", "modernize" or migrate legacy code (strangler fig, framework/dependency upgrades, callback→async), or when planning a refactor before touching code. Two hats, test-guarded steps, revert-on-red. Do NOT use for new feature work, bug fixes with known symptoms (root-cause), styling or copy changes, the refactor step inside a user-requested TDD flow (tdd), or when the user asks to rewrite from scratch.
license: Behavior-preservation methodology synthesized from public sources (Fowler's Refactoring catalog, Tidy First?, and community agent skills by bienhoang, wondelai, mattpocock, jeffallan, vasilyu1983), audited 2026-09-12.
---

# Refactoring

Change the structure of code without changing what it does. Every observable behavior that exists before the refactoring — including the bugs — must exist after it. This skill exists because agents drift: without hard gates, "refactoring" silently becomes rewriting, and rewriting untested code is how behavior is lost.

**Without tests, you're not refactoring — you're editing.**

## Governing rules

1. **Two hats.** You are either adding function or restructuring, never both in the same change. Mixing them makes each unprovable. If new behavior is genuinely required, finish the structural change first, prove it, then change behavior in a separate step.
2. **The baseline is a gate, not advice.** Before the first edit: the test suite runs green on unmodified code, or you build a safety net first (see mode 3). Record what you ran and its result. A suite that was already red tells you nothing — fix or quarantine the noise before counting on it.
3. **One named transformation per step.** Each step applies exactly one refactoring (Extract Method, Move Function, Introduce Parameter…) and has a name you can state. If you cannot name it, it is not a refactoring — it is a rewrite wearing a costume.
4. **Red means revert, not debug.** After each step, run the smallest relevant suite. Green ⇒ commit (when per-step commits were agreed — see the loop's checkpoint), message names the transformation. Red ⇒ revert the step immediately (`git restore`/`git checkout` the touched files). A red intermediate means the step was too big or wrong; debugging a broken step costs more than re-slicing it.
5. **Never touch tests to get green.** In a pure refactor, modifying, loosening, or deleting an assertion is the cardinal sin — it destroys the only proof you have. If a test blocks legitimate structural change (asserts private internals), convert it to assert observable behavior *before* the transformation, as its own commit. Any unexplained test change in a refactor diff is a red flag.
6. **Smallest suite per step, full suite per finish.** Iterate fast on the touched area, but before declaring done: full test suite + typecheck + lint, on the same commands CI runs.
7. **Prove it, don't vibe it.** The closing question is not "is this good code" but "can I prove nothing observable changed?" Answer with evidence: suites run, before/after outputs, mutation spot-checks when the boundary is critical. Say plainly what you could not verify.
8. **Make the change easy, then make the easy change** (Beck). Preparatory refactoring — small structural setup that makes the coming feature trivial — is the highest-ROI kind. Do it, then stop.
9. **Public API surface is observable behavior.** Renaming or reshaping an exported symbol, endpoint, or file layout preserves runtime behavior but breaks consumers. Inside the module: refactor freely. Across a published boundary: deprecate-and-alias, never hard-break, unless the user chose the break.
10. **Edit sources, not generated output.** `dist/`, vendored, or generated files are regenerated by their toolchain — hand-editing them is a change that the next build erases. Change the generator or the source, then regenerate.

## Modes

**1. In-the-small** — tidy a module, extract a function, kill duplication while working. The execute loop below, inline, without ceremony. Keep litter-pickup bounded to files you are already touching; a boy-scout sweep across the repo is scope creep.

**2. Plan-first** — the user asks for a refactor plan, the change is architectural, or multiple approaches compete. Interview the problem, explore the repo to verify claims, fix scope as in/out lists, check test coverage (thin coverage is a question to the user, not an assumption), then break the work into a plan of tiny commits — each commit leaves the codebase working. File it where the user wants (issue, doc, or just the reply). Do NOT embed file paths or code snippets in the plan — they go stale before the work starts.

**3. Legacy / untested** — no tests, unrunnable suite, or a migration too large for one session. Do not proceed with the normal loop. Go to `references/legacy.md`: characterization tests capturing *actual* current behavior (bugs included — log them, don't silently fix them), seams, branch-by-abstraction, parallel change (expand–migrate–contract), strangler fig.

## Execute loop

1. **Baseline.** Run the suite on unmodified code; record green. If red or absent → mode 3. If the project has no VCS or the working tree is dirty with unrelated changes, say so and stop for direction — a dirty baseline destroys the revert safety.
2. **Commit checkpoint.** Refactoring is safest with per-step commits on a dedicated branch — ask the user once, up front: "I'll commit each verified step on a branch — good?" If they decline, keep steps small and separable and report the step list for review at the end. Never commit without authorization; without any VCS, or with unrelated uncommitted changes in the tree, say so and stop for direction — a dirty baseline destroys the revert safety.
3. **Pick one target.** Ranked by risk-adjusted value, not by how interesting it is: security → correctness → structure → duplication → naming. Hotspots first — files where churn (recent edit frequency) meets complexity. Smell catalog and metrics thresholds: `references/smells.md`.
4. **Apply one named transformation.** Full mechanics per transformation live in `references/smells.md`. Prefer language-aware tooling (IDE rename, AST codemods) over regex edits; at scale (>~10 files or >~500 lines), a codemod is the safe path and regex is the wrong one.
5. **Verify.** Smallest relevant suite → green ⇒ commit (message = transformation name) → next target. Red ⇒ rule 4 of the governing rules: revert, take a smaller step.
6. **Close.** Full suite + typecheck + lint, on the same commands CI runs — including every package that imports the code you touched, not just the one you edited (monorepos: respect build order). Report: transformations applied (named), before/after state, smells left and why, anything deferred. Agent-specific drift modes to check before closing: `references/agent-pitfalls.md`.

## Risk levels set the safety net

| Risk | Touching | Minimum net before editing |
|---|---|---|
| Low | Pure internals, no I/O | Unit tests + types |
| Medium | Module boundary, parsing, state | + contract tests at the seam |
| High | Money, auth, concurrency, migrations, data | + integration tests, a rollback plan, and small phases |

When unsure whether behavior could change: **do not apply — ask.**

## When NOT to refactor

- No failing demand: nothing to add, nothing hurting. Refactoring without a driver is gold-plating.
- The code is about to be deleted or replaced. Deleting is cheaper.
- You cannot run or construct any safety net and the risk is medium+. Report and stop.
- The rewrite instinct hits ("this is all wrong, let me start fresh"). That is a different conversation with the user, not a refactor — and big-bang rewrites lose the one thing refactoring keeps: working software at every step.

## References

- `references/smells.md` — smell catalog, metrics thresholds, prioritization, transformation mechanics
- `references/legacy.md` — characterization tests, seams, branch-by-abstraction, parallel change, strangler fig, per-phase checkpoints
- `references/agent-pitfalls.md` — how LLM agents specifically break "behavior-preserving", and the layered proof that catches it