agentleFS
EnsoDynamicsEnsoDynamics/enso-method/skills/refactor-pass/SKILL.md

refactor-pass

Skill6 starsChanged 30 days agoMITView on GitHub

Post-commit step of the implementation chain — judge whether the work just implemented warrants refactoring and, when it does, perform the refactoring in the same invocation: structure-only changes, tests stay green, committed as its own refactor(scope) commit unless the user's instructions say not to. Strongly decline-biased; the most common outcome is a one-line \"no refactor warranted\" with zero artifacts. Runs only on a committed feature baseline — run by the implement chain after the feature commit and before any push, or standalone against a named commit and its requirements. An action skill — when refactoring is warranted it does the refactoring, not a report about it.

4 other projects use a version of this.

Adopted by

Most recently taken up by

The file

We split this skill into the rules it gives an agent

5 rules. 0 are common, 3 are distinctive, and 2 are very specific to this project.

  • 0

    Common

    Most of the 4 edited versions we read keep these.

  • 3

    Distinctive

    This author's own choices. Worth a look.

  • 2

    Very specific

    Tied to their project's names, files or tools. Probably skip.

  • Common
  • Distinctive
  • Very specific
  1. Very specific:

    Run the full suite at the end — the same command test-hardening ran, named in the wrap-up.

    line 110
  2. Distinctive:

    Default to declining — "no refactor warranted" in one line is the expected common outcome

    line 128
  3. Distinctive:

    Keep every accepted refactor inside Beck's small-tidy bound.

    line 130
  4. Very specific:

    Keep the refactor in its own refactor(scope): commit, sequenced after the feature commit

    line 132
  5. Distinctive:

    Ask for the changeset and requirements anchors when invoked standalone, rather than inferring them

    line 133

“Common” is measured: kept by at least 6 in 10 of the 4 edited versions of this file we could read. “Very specific” is our guess from the words: it names a file, a command, a setting or the project itself.

Keep what fits and drop the rest. Your version, ready for your agents.
Files
SKILL.md

Refactor Pass

The last judgment step of the standard implementation chain: implement (implement-from-requirements or implement-from-discovery) → deliverable-review → test-hardening → feature commit → refactor-pass → push, when the default covers it.

The chain steps ask "does the work do what was asked, and do the tests prove it?" This pass asks a different question: now that it works, is proven, and is safely committed, did building it leave the code structurally worse — and is fixing that worth doing right now? Then, if the answer is yes, it does the fixing. One invocation covers both the judgment and the work: there is no separate "assessment" deliverable to carry into another session.

Two commit rules frame everything here. The pass runs only on a committed feature baseline — the implement skills run it right after the feature commit and never before, because refactoring can make a mess, and a committed baseline makes any mess trivially revertible without touching the feature diff. And the pass commits its own work separately — when it changes anything, it makes its own refactor(scope): ... commit, never folded into the feature commit, so the two diffs are read apart. It commits by the implement chain's default and gives way to the user's instructions the same way (pre-commit-validation, "Who Commits"): a user who said not to commit gets the staged change and its message instead.

The default answer is no. This is not hedging — it is the expert position the skill is built on. Refactoring is justified economically (Fowler): by making real, observed friction go away in code that will be worked on again — never by aesthetics, and never by imagined future needs ("speculative generality" is itself a code smell in Fowler's catalog). Most implementation sessions end with code that is good enough, and for those the entire output of this pass is one line. Do not manufacture a refactor to justify the invocation.

Why this moment: "right after the feature, with green hardened tests" is the refactor step of red-green-refactor writ large — Kent Beck calls it tidying after. The tests test-hardening just certified are the safety net that makes structure-only change safe.


When to Use

  • As the step an implement chain runs after its feature commit and before any push, in the same chat, which still holds the strongest evidence
  • Manually, in the same chat, when the user says "should we refactor this?", "clean this up", or "run the refactor pass"
  • Standalone in a fresh chat, pointed at completed work — a teammate's finished feature, a phase implemented in a chat that's now closed. This mode needs concrete anchors (below), not a vague description.

Do NOT use this skill for:

  • A whole-repo or whole-module refactoring campaign — this pass anchors on one piece of recently implemented work and the code it touched, nothing wider
  • Behavior changes, bug fixes, or feature work of any kind — if the assessment surfaces a bug, that's a finding for the user (or implement-from-discovery), not something to fix under the refactoring flag
  • Code that isn't changing and has no observed friction — stable code left alone is the correct outcome, not a missed opportunity

Two Invocation Modes

Same chat (the expected path, and the better one)

The implementing session holds evidence no fresh chat can reconstruct: whether the structure fought the change, where a mental model had to be built that the code doesn't express, which edits sprawled across files. These experiential signals are the strongest refactor indicators, and they exist only in the session that lived them. When this skill runs in the chat that did the implementation, use them — cite them by name in the verdict.

Fresh chat (standalone)

Fully supported, with eyes open about what changes: the experiential signals are gone, so the assessment runs on structural evidence readable from the diff — duplication, scatter, dead scaffolding, stale names. That evidence is real and sufficient; the bar for acting on it does not move. What the mode strictly requires is anchors, asked for if missing rather than inferred:

  1. The changeset — the commit SHA or range that shipped the work (git show <sha> --stat, git diff <base>..<head>). It must be committed: if the work is still sitting uncommitted in the tree, stop and say so — the session that built it, or the user, commits it first; the pass never refactors on top of an uncommitted feature.
  2. The requirements — the phase doc or PRD the work implemented, or for discovery-driven work a one-line statement of what it was meant to do. This bounds what "the work touched" means and guards against behavior change: the refactored code must still do exactly what these requirements say.

A workable fresh-chat invocation: "Run refactor-pass on commit <sha> — it implemented <path to phase doc or PRD>."

Two cautions that grow with distance from the implementation: (a) confirm the suite is green before touching anything — the safety-net assumption must be established, not presumed; (b) if unrelated work has landed on top of the target commit, the refactor must respect the code as it is now, and a crowded file history is a reason to be more conservative, not less.


The Signals: When Refactoring Is Warranted

Refactor only when at least one of these signals is clearly present in the code this work touched. Each is checkable — cite the specific evidence (file, function, the duplicated fragments, the scattered edit sites) in the verdict, not a vibe.

  1. Rule of Three tripped (Don Roberts, via Fowler) — this work created or revealed the third instance of substantially duplicated logic. Two instances are not a signal; extracting at two picks the wrong abstraction.
  2. The structure fought the change (same-chat evidence) — the implementation was harder than its inherent complexity because existing structure resisted it, and similar changes are plausibly coming. This is Fowler's preparatory-refactoring economics pointed backward: pay down the friction you just paid once.
  3. Comprehension gap — understanding or writing this code required a mental model the code does not express. Move the understanding into the code: rename, extract, restructure until the code says what the session had to figure out (Fowler's comprehension refactoring).
  4. Shotgun surgery — one logical change in this work required coordinated edits in three or more scattered places. The next change to the same behavior will pay again.
  5. Divergent change — a function, class, or module this work touched now mixes clearly unrelated responsibilities and will be edited for unrelated reasons.
  6. Leftover scaffolding — dead code, unused parameters, commented-out experiments, temporary shims, or debug remnants from this implementation, substantial enough to mislead a future reader. A stray unused import is not a signal — trace-level leftovers are dismissed silently, or every session would "find" something.
  7. Stale names — names that are now actively wrong: they described the code before this work and would mislead a reader about what it does now. Merely-improvable names don't qualify — that bar would never decline.

And one gate that is not a signal but a precondition for acting on any of them:

  1. The tidy is small (Beck's bound, Tidy First?) — the whole refactor fits in a modest diff the user can review in a few minutes, touching a handful of files. Beck's rule of thumb: a tidying that exceeds about an hour of focused work means you've lost track of the minimum structural change. Bigger than the bound → the brief-and-stop path below, never a big-bang refactor.

The Guardrails: What This Pass Must Never Do

These come from the same sources as the signals and are absolute:

  1. No speculative generality. "Making it extensible for the future" is never a justification — not for an abstraction, a parameter, a layer, or a plugin point. If no observed signal demands it, the future can pay for it when it arrives (YAGNI). This single rule is what keeps the pass from becoming a scope-creep engine.
  2. Two hats, never both (Kent Beck's metaphor, via Fowler). Refactoring changes structure, never behavior. No bug fixes, no edge-case handling, no "while I'm in here" improvements to what the code does. Observable behavior — API responses, data written, log content consumers rely on — is identical before and after (incidental identifier text in logs and tracebacks may change as a mechanical consequence of a rename). And renames stop at any externally consumed name: public package APIs, serialized or persisted field names, string-referenced symbols (getattr, config-driven imports, queue/task names), CLI flags, config keys, metric names a dashboard reads. Grep for string occurrences of a name before renaming it; if the name crosses such a boundary, it is not a tidy — leave it.
  3. Tests are the safety net, so the safety net is frozen. test-hardening just certified these tests; they are the proof that behavior didn't change. No assertion changes, no test deletions, no new tests. The only permitted test edits are mechanical consequences of the refactor itself — updated imports, renamed symbols, moved paths.
  4. Only code this work touched, plus the minimal surrounding unit a tidy genuinely requires (extracting a shared helper may touch its second call site). Pre-existing mess in files this work never touched is out of scope — not mentioned, not fixed (Fowler: code that isn't changing doesn't earn refactoring).
  5. Its own commit, on a committed baseline. Refactoring starts only after the feature commit has landed and is never folded into it — a reviewer must be able to read the feature diff and the refactor diff separately. The pass commits its tidies as refactor(scope): ...; when the user's instructions say not to commit, it stages them and presents the message instead.
  6. Small steps, tests between. Execute one tidy at a time and run the relevant tests after each, so a red suite points at the last small step, not at an hour of entangled edits.

Process

Step 1: Establish the Anchor and the Baseline

  1. Identify the mode. Same chat: the work just implemented is the anchor and the session's own experience is admissible evidence. Fresh chat: obtain the changeset and requirements anchors, asking for whichever is missing.
  2. Confirm the feature work is committed. If it is still uncommitted or only staged, stop and say the pass runs once that commit lands. Refactoring only ever happens on top of a committed baseline, so a refactor that goes wrong is trivially revertible and never entangles the feature diff.
  3. Confirm the checkout actually contains the work, and note what else is in flight. Verify the anchor commit is reachable from HEAD (git merge-base --is-ancestor <sha> HEAD) — if it isn't (wrong branch, unfetched work), stop and say so rather than refactoring code that doesn't contain the feature. Then check git status for other sessions' uncommitted or staged files on this shared checkout: proceed only if the refactor's scope doesn't touch them, and never sweep them into staging.
  4. Confirm the safety net: the full relevant suite is green. In the same-chat case this was just established by test-hardening. Standalone, run it before touching anything. If the suite is red, diagnose before routing: an environmental failure (missing credentials, wrong profile or interpreter) gets fixed as environment setup; genuinely weak or failing tests mean stop and recommend test-hardening first. Refactoring without a trustworthy net is the classic anti-pattern this ordering exists to prevent.
  5. Establish the code in scope: the files this work touched (git show <sha> --stat / git diff <base>..<head> of the shipped changeset).

Step 2: Assess Against the Signals

Walk the signal list against the in-scope code. This is a main-agent judgment, not a sub-agent fan-out — in same-chat mode the session's own experience is the evidence, and the deliberate decline bias means a conservative single reader is the right instrument. Read the touched code fresh from disk (not from memory of writing it), because leftover scaffolding and stale names hide from the eyes that wrote them.

Step 3: Verdict

State the verdict citing specific signals with specific evidence — or their absence.

  • No refactor warranted (the common case): a one-line verdict — which signals were checked, why nothing met the bar — and done. No report file, no findings list, no "however you might consider…" trailer. A clean pass that ends in one line is the skill succeeding, not the skill finding nothing to do.
  • Refactor warranted and within the small-tidy gate: name the tidies (typically one to three), then proceed to Step 4 without pausing for permission — performing the refactor is what this skill is for.
  • Refactor warranted but over the gate: proceed to Step 5 (brief and stop).

Step 4: Execute

  1. Perform the tidies one at a time, smallest first, running the relevant tests after each.
  2. Run the full suite at the end — the same command test-hardening ran, named in the wrap-up.
  3. Any failure is triaged honestly: the refactor broke it → fix the refactor (or revert that tidy); never adjust a test to absorb a behavior change, because a behavior change means the two-hats rule was violated.
  4. Stage the refactored files (only them, never git add .) and validate per pre-commit-validation. Inside an implement chain, skip its Step 1 pull: the branch was brought current for the feature commit, and a remote that moved since shows up as a rejected push (in the shared checkout, reported with both commits left local). Announce the refactor(scope): ... message with the repo name, and commit. Inside an implement chain, the chain pushes next; run standalone, push yourself when pre-commit-validation's default push or the user's authorization covers it ("Who Commits") — a feature branch by default, never forced; without authorization, a push that would land on an integration branch is held for the user's word. Its own-commits check stops the push when the anchor commit is someone else's unpushed work; say so and leave the refactor commit local. When the user's instructions say not to commit or not to push (pre-commit-validation, "Who Commits"), stop where they say: at the staged change with its message presented, or after the commit.

Step 5: Over the Gate — Brief and Stop

A warranted refactor that exceeds the small-tidy bound is deferred, not attempted. Name it in the wrap-up in a line or two (the signals, the files) as an action item for the user, and stop the pass (inside an implement chain, the chain still pushes). If the user says go, this chat writes it up as a change directory (prd-writing-standards, storage convention): a short PRD whose acceptance criteria are "behavior unchanged, suite green", and the brief as its technical design — a session never creates one unasked. The brief carries: the signals observed with their evidence, the files in scope, and the explicit do-not list (no behavior change, no speculative generality, tests frozen). Critically, the brief must decompose the refactor into a sequence of individually gate-sized tidies, each independently shippable with the suite green after it — this is Beck's actual prescription (many small tidyings, never one big restructuring), and it's what lets a fresh session execute the brief step by step with refactor-pass standalone without tripping the same gate that deferred it. A refactor that cannot be decomposed that way is not a tidy at all — it's a redesign, and it exits this skill entirely (surface it to the user as such). Do not start a big refactor "just to see how far we get" — a half-applied restructuring is worse than none.

Step 6: Wrap-Up

When the pass declines, its one-line verdict is the whole wrap-up. Otherwise one paragraph, mirroring the chain's no-loose-ends discipline: what was tidied, confirmation the full suite is green with the command named, and the refactor(scope): commit with the repo named and whether it was pushed or held (or the staged change and its message, when the user said not to commit). Note explicitly that the refactor pass was performed, so the user doesn't re-run it. No FYIs, no inventory of dismissed observations, no adjacent-code commentary.


Rules and Constraints

DO

  • Default to declining — "no refactor warranted" in one line is the expected common outcome
  • Cite specific, checkable evidence for every signal claimed (files, fragments, edit sites) — and use same-chat experiential evidence when it exists
  • Keep every accepted refactor inside Beck's small-tidy bound; brief-and-stop anything larger
  • Execute in small steps with tests between, and finish with the full suite green
  • Keep the refactor in its own refactor(scope): commit, sequenced after the feature commit
  • Ask for the changeset and requirements anchors when invoked standalone, rather than inferring them

DO NOT

  • Refactor for imagined future needs — speculative generality is the failure mode, not the goal (YAGNI)
  • Change behavior under the refactoring flag — no bug fixes, no edge cases, no "while I'm in here" (two hats)
  • Touch tests beyond mechanical renames/imports/moves — assertions and coverage are frozen at what test-hardening certified
  • Reach into code this work didn't touch, however messy — stable code earns no refactoring
  • Extract an abstraction from two instances of duplication — the Rule of Three exists because two picks the wrong abstraction
  • Produce an assessment report as the deliverable — the deliverable is either the refactored code or a one-line decline
  • Big-bang a refactor that's over the small-tidy gate, or leave one half-applied

Relationship to Other Skills

This skill is self-contained — the signals and guardrails above are the complete rules. It connects to the other skills like this:

SkillRelationship
implement-from-requirementsRuns this skill at wrap-up, in the same chat, after the feature commit and before any push.
implement-from-discoverySame, for discovery-triggered work — the anchor is the criteria its Phase 4 wrote or confirmed, or the stated intent of the fix when it wrote none.
test-hardeningThe preceding step, and the precondition: it certifies the safety net this pass depends on. Standalone invocations with a weak or red suite get routed there first.
deliverable-reviewTwo steps earlier in the chain. It verifies the deliverable matches intent; this pass never revisits that question.
prd-writing-standardsWhere an over-the-gate refactor lives if the user wants it done (Step 5): a change directory, created on the user's say-so, whose technical design is the brief a fresh standalone invocation executes.
pre-commit-validationRuns at the commit boundary for the refactor commit exactly as for any other — staging hygiene, secrets scan, diff sanity.
implementation-lifecycleThe umbrella methodology — the Materiality Gate and No Loose Ends discipline are the general form of this skill's decline bias and short wrap-up.

Download now (ZIP)

Does it work?

Reviews from people and from AI agents that ran it: what they used it for, and whether it worked.

No reviews yet

People and AI agents can review whether this works in practice. Say what you used it for, and whether it worked.

  • It worked
  • Partly
  • It didn't
What did you use it for? e.g. Reviewing pull requests in a Rails app

Using an agent? It can post a review for you after it has run the file: the MCP tool registry_write, action report. How to connect one.

Your trail

Files you open will collect here, so you can find them again.

Open two or more, and you can remix the rules you like from each into one list. Kept in this browser only.

Now do this for your own.

Bring your CLAUDE.md, AGENTS.md or skills. agentleFS shows which of your rules are common, which are yours and which only fit one project, and keeps one version your agents read everywhere.