Refactor Safely

Refactoring Safely

In a private codebase a refactor is a Tuesday. In a public one it is a coordination problem with strangers who did not agree to be coordinated.

First: should you?

The Second System Effect is real and rewrites kill projects. Before agreeing:

| Reason to refactor | Verdict | |---|---| | A specific bug class keeps recurring here | Yes — targeted refactor | | New feature is genuinely blocked by the structure | Yes — refactor exactly enough to unblock | | Performance ceiling hit, profiled and proven | Yes | | Onboarding contributors repeatedly stall in this file | Yes | | "The code is ugly" | No — write tests instead, and reconsider in a month | | "I'd write it differently now" | No | | "Let's move to " | Only with a user-facing reason | | "Full rewrite, v2, from scratch" | Almost never — see below |

The full rewrite trap. A from-scratch v2 means: shipping nothing for months, maintaining v1 anyway, re-discovering every edge case that the ugly code in v1 was silently handling, and a migration your users may simply decline. Projects have died here. If the user wants a rewrite, propose the strangler-fig alternative first, and only lose that argument once.

Rewriting is genuinely correct when the original's core assumption is wrong — single-threaded when it must be concurrent, synchronous when it must stream, a data model that cannot express the domain. Incremental refactoring cannot fix an axiom.

Preconditions

Do not start until all of these hold:

1. Characterization tests exist. Before changing anything, write tests that pin

current behavior — including behavior you think is wrong. Bugs get depended upon; those tests tell you which ones.

1. The public API surface is snapshotted and asserted in CI (see api-design).
2. The refactor is sequenced into reviewable PRs. Nobody can review 5,000 lines,

including you in three weeks.

1. main stays releasable at every commit. A long-lived refactor branch

accumulates conflicts and blocks everyone else's work.

# Establish a behavioral baseline before touching anything
make test                                   # must be green
git tag pre-refactor-baseline
cargo public-api > api-baseline.txt         # or api-extractor / dir() snapshot

The strangler fig

The pattern that lets you replace a system while it stays in production.

1. Put a seam around the old implementation — an interface, a facade, a module

boundary. This is a pure-mechanical, zero-behavior-change PR. Merge it alone.

1. Build the new implementation behind the same seam. It does not have to be

complete; it only has to be correct for the slice it claims.

1. Route a slice of traffic/calls to the new path, behind a flag or an env var.
2. Verify equivalence. Run both and compare outputs where feasible.
3. Migrate slices one at a time, each its own PR, each independently revertible.
4. Delete the old path once nothing routes to it — and actually delete it. A dead

branch left "just in case" is the thing future contributors trip over.

Each step ships. Each step is revertible. At no point is main broken. This is slower in wall-clock time than a rewrite and dramatically faster in time-to-working-software.

Moving and renaming public symbols

The rule: never break the old path in the same release that introduces the new one.

// v3.1 — new home, old path still works
export { parseConfig } from './config/parse.js';

/** @deprecated Import from `pkg/config` instead. Removed in v4.0.0. */
export function parse(opts: Options) {
  warnOnce('parse() moved to parseConfig() in pkg/config. Removed in v4.0.0.');
  return parseConfig(opts);
}

Sequence: v3.1 adds the new path and deprecates the old → v3.x keeps both, warning once per process → v4.0 removes the old path, with a migration guide and a codemod.

For renames inside a package, keep the change mechanical and separate:

git mv src/utils.ts src/text/normalize.ts
# rename-only commit, no logic changes — reviewable in 30 seconds

Mixing a rename with a behavior change produces a diff where the behavior change is invisible. Reviewers will miss it. This is one of the most reliable ways to ship a bug.

Ship a codemod

If a migration requires more than about ten mechanical edits per user, write the codemod. The adoption difference is enormous — a one-command migration gets run; a 20-step guide gets deferred until the user is on an unsupported version filing bugs.

npx jscodeshift -t ./codemods/v4-rename-parse.js src/     # JS/TS
python -m libcst.tool codemod v4_rename src/               # Python
comby 'parse(:[args])' 'parseConfig(:[args])' -i -f .go    # language-agnostic
cargo fix --edition                                         # Rust editions

Ship it in the repo under codemods/, test it against your own codebase first, and link it from the migration guide and the deprecation warning message.

Sequencing into PRs

A good refactor PR series, in order:

1. Tests only. Characterization tests for current behavior. Merges instantly.
2. Mechanical moves. Renames, file moves, extractions. No logic changes. Verify

with git diff -M --stat showing pure renames.

1. Seam introduction. Interface/facade added; old code unchanged behind it.
2. New implementation, unused or flag-gated.
3. Switch the default. One line. Trivially revertible — this is the point.
4. Delete the old path.
5. Cleanup. Now that both paths are gone, simplify what remained.

Each PR states in its description: what changed, what did not, and how to verify. Label the series (refactor/parser) and track it in a single meta-issue so contributors know which files are moving and can avoid conflicts.

Communicating with users and contributors

• Announce before starting, in an issue: what, why, what will break, when. Give

people the chance to object before you have spent the effort.

• Freeze the affected area for other contributors, or you will create conflicts

that make their PRs unmergeable — which is a good way to lose them.

• Publish the migration guide with the release, not after (see docs-architecture).
• Support the previous major for a stated window. Backport security fixes to it.

"We support the previous major for 12 months" is a sentence that buys enormous goodwill and should live in your README.

Verifying you didn't break anything

# API surface diff — the highest-signal check
cargo public-api diff pre-refactor-baseline
npx api-extractor run --local && git diff etc/

# Behavioral equivalence on real inputs
for f in fixtures/*; do diff <(old-bin "$f") <(new-bin "$f") || echo "DIFF: $f"; done

# Downstream smoke test: run your top dependents' suites against the new version
# (npm: `npm pack` + install into their repo; Python: `pip install -e .`)

For widely-depended-upon libraries, test against real downstream consumers before release. Rust's crater and Go's module proxy analysis are the industrial versions; the manual version — cloning your five biggest dependents and running their tests — catches most of it and takes an afternoon.

Anti-patterns

• The v2 branch that never merges. If it has been open six months, it is dead;

extract what is salvageable and close it.

• Refactor mixed with features. Reviewers cannot separate them, so they approve

neither carefully.

• Renaming and changing logic in one commit.
• Removing a deprecated API earlier than announced. You published a date; honor it.
• No deprecation period because "nobody uses that." You cannot know. Check

GitHub code search and package download stats before asserting it.

• Reformatting the whole repo inside a refactor PR. Do formatting in its own

commit, add it to .git-blame-ignore-revs, and never do it again.

• Breaking changes in a minor release because "it was technically a bug." The

users' upgrade did not care about your reasoning.

Source & license

This open-source skill is cataloged on AgentStack and links to its original source — we do not rehost the code.

• Author: the-open-agent
• Source: the-open-agent/oss-skills
• License: Apache-2.0

Install and usage instructions live in the source repository linked above.