Install
openclaw skills install bookforge-legacy-code-change-algorithmGuide safe modification of legacy code (untested production code) using Feathers' 5-step Legacy Code Change Algorithm. Use this skill whenever a developer needs to change code that lacks test coverage — adding a feature, fixing a bug, refactoring, or optimizing — and wants to avoid regressions. Activates for 'I need to change this code but there are no tests', 'how do I safely modify legacy code', 'inherited codebase', 'untested code', 'legacy system change', 'code without tests', 'refactor without regressions', 'make this code testable', 'cover and modify', 'don't break anything', 'risky change to old code', 'no test coverage', 'adding feature to old code', 'fixing bug in untested code', 'legacy codebase change safely'.
openclaw skills install bookforge-legacy-code-change-algorithmYou are about to change code that has little or no test coverage. The code may be code you wrote, code you inherited, or code that was written before the team adopted testing practices. Any of these situations apply:
Before executing this skill, verify:
If the test suite cannot run at all (build is broken), resolve that first. This algorithm assumes you can get to a green baseline, even if it's zero tests.
Before executing, determine: "Do I have enough context to identify the change?"
Change description: A one-sentence description of what must change and why. -> Check prompt for: "I need to", "add X", "fix Y", "change Z" -> If still missing, ask: "What exactly needs to change, and what behavior should it produce after the change?"
Entry point in codebase: The file(s) or class(es) where the change will happen. -> Check prompt for: file names, class names, method names -> Check environment for: matching files via Grep/Glob on mentioned names -> If still missing, ask: "Which file or class is the starting point for this change?"
Test suite status: Are there any existing tests? Which test runner?
-> Look for: test/, tests/, spec/, *Test.java, *_test.py, *.test.ts, pytest.ini, build.gradle, package.json scripts section
-> If no tests found: default to "starting from zero tests"
Language and build system: Determines which dependency-breaking techniques are available.
-> Look for: file extensions, pom.xml, Makefile, package.json, requirements.txt, go.mod
-> If unavailable: note "language unknown" and proceed — the algorithm is language-agnostic
Dependency profile: Does the code under change have constructor dependencies, global state, or direct I/O?
-> Look for: new DatabaseConnection(), Singleton.getInstance(), System.out, direct file reads in the target file
-> Default: assume dependencies exist until proven otherwise
SUFFICIENT: Change description known + entry point file(s) identified + can run build command (even if no tests) PROCEED WITH DEFAULTS: Change description known + rough area of codebase known MUST ASK: Change goal is entirely unknown — cannot proceed without user input
ACTION: Read the change description and classify it into one of four types: (A) adding a feature, (B) fixing a bug, (C) refactoring (improving design), or (D) optimizing (improving performance/resource use).
WHY: The change type determines your safety posture. Feature addition and bug-fixing both alter behavior — you must write tests that verify the new behavior plus tests that preserve existing behavior. Refactoring and optimization must not change behavior at all — every test you write should still pass before and after. Knowing the type up front prevents you from accidentally testing the wrong thing or under-testing the right thing.
IF the change is a refactor or optimization -> your single constraint is "all behavior must be identical after the change." Set a mental flag: zero behavioral change permitted. IF the change is a feature addition -> identify the new behavior you want to add. The old behavior is off-limits for modification (unless the requirement explicitly says otherwise). IF the change is a bug fix -> identify the one slice of behavior that is wrong and needs to change. Everything else is off-limits. IF the intent is ambiguous -> default to "feature addition" (safest posture). Ask for clarification in parallel.
ARTIFACT: Write a one-sentence declaration:
"This is a [feature addition / bug fix / refactor / optimization]: [change description]."
Save this as the header of change-plan.md.
ACTION: Locate the specific files, classes, and methods that will need to be modified to implement the change. Trace from the entry point outward using Grep and Read. Document each change point by file path + method/function name.
WHY: You cannot write effective tests or plan dependency-breaking until you know exactly where the code will change. "Change points" are the precise locations — not the entire module or service. Precision here prevents over-testing (wasting time on code that won't change) and under-testing (missing code that will).
HOW:
IF the codebase is very large and you can't find the right location -> invoke legacy-code-symptom-router or use scratch refactoring (Chapter 16) to explore before committing.
IF the architecture is clear and the change point is obvious -> proceed immediately.
ARTIFACT: Append to change-plan.md:
## Change Points
- [file:line] [ClassName.methodName] — reason: [why this changes]
- [file:line] [ClassName.methodName] — reason: [why this changes]
ACTION: For each change point identified in Step 1, determine where tests can be placed to detect both correct new behavior (for feature/bug) and regressions (for all types). Trace the effect of the change outward to find interception points where test assertions are both meaningful and achievable.
WHY: Test points are not the same as change points. The method you change may be deep inside a dependency chain; you may need to test it from a calling method one level up, or from a public interface that routes to it. The goal is to find the narrowest entry point that (a) you can exercise from a test harness and (b) will fail detectably if the change has an unintended side effect.
HOW:
IF test harness construction is blocked (class cannot be instantiated in tests) -> this is a dependency problem. Note it and proceed to Step 3.
IF you cannot find any reachable test point -> invoke test-harness-entry-diagnostics for the 4-category triage.
ARTIFACT: Append to change-plan.md:
## Test Points
- [ClassName.testableMethod] — covers change points: [...] — assertion: [what the test will assert]
- NOTE: [ClassName] cannot be instantiated in harness because: [reason] — dependency-breaking needed
ACTION: For each dependency that blocks test harness construction (identified in Step 2), apply a conservative dependency-breaking technique to make the code testable. Work without tests at this stage — apply strict safety constraints: Preserve Signatures (copy-paste method signatures exactly, no creative changes), Single-Goal Editing (one change at a time, note others for later), and use the compiler as your safety net.
WHY: This is the step most people skip, and it is why legacy code stays untestable. Dependencies — hardcoded constructor calls, global singletons, direct I/O, tightly coupled classes — make it structurally impossible to instantiate objects in isolation. Before you can write any test, you need a seam (a place where you can alter behavior without editing in place). Breaking dependencies creates seams. The reason to be conservative here is that you are refactoring without tests: every change carries risk of introducing a bug. Minimizing what you change minimizes that risk.
HOW:
new, a singleton, a global variable, a direct DB call).dependency-breaking-technique-executor)IF the change required multiple dependency breaks, apply them one at a time, verifying compile between each. IF a dependency-breaking change itself looks risky -> document it in the log and consider Lean on the Compiler (deliberately introduce a type error to let the compiler find all affected locations).
ARTIFACT: Create dependency-break-log.md:
## Dependency Breaks Applied
1. [Technique name]: [what changed] — file: [path] — Signatures preserved: yes
2. [Technique name]: [what changed] — file: [path] — Signatures preserved: yes
ACTION: Write characterization tests (tests that document the actual current behavior of the code, not what it should do) for the test points identified in Step 2. Use the test harness you have now made accessible via Step 3. Run the tests. They must pass on the current (unchanged) code before you proceed.
WHY: Characterization tests are written to detect change — not to find bugs. Their purpose is to act as a vise: they lock existing behavior in place so that Step 5 changes only what you intend. If you discover a bug while writing characterization tests, document it but do not fix it now (fixing it would be a behavior change you haven't planned for, and it would make the test suite misleading). The rule is: "What the system does is more important than what it is supposed to do" — at this stage. You are building your safety net, not auditing correctness.
HOW:
assertEquals("", result)), run the test, and let the failure message tell you the actual value. Then update the assertion to match the actual value.IF tests are hard to write because the methods have side effects (DB writes, network calls) -> go back to Step 3 and break those dependencies too. IF you discover the code does something obviously wrong while writing tests -> document it; do not change behavior until Step 5, and only if it's within scope of the current change. IF you are adding a feature or fixing a bug -> the new-behavior test should fail at the end of this step (you haven't implemented the change yet).
ARTIFACT: A passing test suite that characterizes current behavior, plus (for feature/bug changes) one or more failing tests that describe the target behavior.
ACTION: With tests in place, implement the change. Run tests after every small edit. When all target-behavior tests pass and no characterization tests regress, the change is done. Then — and only then — refactor the code you touched to improve its structure.
WHY: The sequence matters: change first, then refactor. If you interleave them, a test failure won't tell you which caused the regression. Running tests after every small edit keeps the feedback loop tight (ideally under 10 seconds for the local subset). Refactoring after the change is what makes the codebase progressively better — you are not leaving legacy code worse than you found it, you are leaving it with tests and cleaner structure in the area you touched.
HOW:
IF a characterization test fails during Step 5 -> stop. This is a regression. Roll back the last edit and try a smaller step. IF the code is too tangled to refactor safely now -> leave it. Document a follow-up ticket. The tests are in place; future work is safer. IF you want to apply TDD for the feature implementation -> the failing tests from Step 4 are your red state; proceed with the red-green-refactor cycle.
ARTIFACT: Committed code with all tests passing, including at least one new test per changed behavior.
| Input | Required | Description |
|---|---|---|
| Source code files | Yes | The legacy codebase, readable via the file system |
| Change description | Yes | What the developer needs the code to do after the change |
| Test runner command | Recommended | How to execute the test suite (e.g., pytest, ./gradlew test, npm test) |
| Existing tests (if any) | Optional | Current test suite — algorithm works with zero tests |
All outputs are written to the working directory unless a specific path is provided.
| Output | Description |
|---|---|
change-plan.md | Running document capturing change type, change points, and test points |
dependency-break-log.md | Record of each dependency-breaking technique applied |
| New or modified source files | The actual change implementation |
| New test files | Characterization tests + new-behavior tests |
# Change Plan
## Change Classification
Type: [feature addition | bug fix | refactor | optimization]
Description: [one sentence]
Safety posture: [behavioral change expected: yes/no | behavioral change permitted: yes/no]
## Change Points
- [file:line] [ClassName.methodName] — [why it changes]
## Test Points
- [ClassName.testableMethod] — covers: [...] — assertion: [what fails on regression]
- BLOCKED: [ClassName] needs dependency breaking because: [reason]
## Status
- [ ] Step 0: Change classified
- [ ] Step 1: Change points identified
- [ ] Step 2: Test points identified
- [ ] Step 3: Dependencies broken
- [ ] Step 4: Tests written and passing on current behavior
- [ ] Step 5: Change implemented, all tests passing
Cover and Modify, not Edit and Pray. Edit and Pray (carefully plan, then change, then poke around) feels professional but provides no systematic safety — care alone is not a substitute for a feedback mechanism. Cover and Modify wraps the change in tests that detect any unintended effect automatically. WHY: without test coverage, every change is a bet that you haven't missed an edge case. With coverage, regressions surface in seconds.
Change type determines safety posture. Refactoring and optimization must produce zero behavioral change. Feature addition and bug-fixing produce intentional behavioral change that must be verified by tests. Conflating these types — for example, refactoring while fixing a bug in the same commit — removes your ability to tell which change caused a test failure. WHY: keeping change types separate makes failures diagnostic rather than confusing.
Step 3 (break dependencies) comes before Step 4 (write tests) — not after. This ordering resolves the Legacy Code Dilemma: "when we change code, we should have tests in place; to put tests in place, we often have to change code." The algorithm breaks the cycle by permitting conservative, pre-test dependency-breaking refactorings. Dependencies often make it structurally impossible to call code from a test harness — you cannot write a test for a class that won't compile in isolation. WHY: attempting tests before dependency-breaking wastes time writing tests against unreachable code and demoralizes developers.
Every dependency break must be conservative: Preserve Signatures, Single-Goal Editing. When breaking dependencies without tests, every unnecessary change is an opportunity to introduce a bug. Preserve Signatures means copy-pasting method signatures verbatim — no renaming, no reformatting, no "while I'm here" improvements. Single-Goal Editing means completing each dependency-breaking refactoring fully before starting the next one. WHY: these constraints trade speed for safety in the window between "no tests" and "some tests."
Every step produces a reversible intermediate artifact. change-plan.md, dependency-break-log.md, and a passing test suite are all checkpoints. If Step 5 produces an unexpected regression, you can roll back to the last green test state and try a smaller step. WHY: artifacts make the process auditable and make rollback decisions trivial — you always know exactly what was done and what the last known-good state was.
Scenario: Adding retry logic to a Java order-processing method with a remote billing API dependency
Trigger: "I inherited a Java microservice with no tests. I need to add retry logic to the order-processing method. It calls a remote billing API synchronously."
Process:
OrderProcessor.process() calls BillingApiClient.charge() directly via new BillingApiClient(). Change point: process().process() is public and takes an Order object. Test point: call it directly. Blocked: BillingApiClient is a concrete class with a live network connection.BillingApiClient → create IBillingClient. Apply Parameterize Constructor to OrderProcessor to accept an IBillingClient. Build passes.FakeBillingClient implementing IBillingClient. Write characterization test verifying that process() calls charge() exactly once on a successful order. Tests pass. Write failing test: process() retries up to 3 times when charge() throws TransientException.Output: OrderProcessor now has retry logic, a FakeBillingClient test double, two new tests, and an IBillingClient interface that future tests can reuse.
Scenario: Fixing a null pointer bug in a C++ legacy module with no test harness
Trigger: "Production keeps crashing with a null dereference in ConfigLoader::resolve(). No tests exist. The class uses global state."
Process:
ConfigLoader::resolve() reads from a global ConfigStore*. Change point: the null check path inside resolve().resolve() is a member function of ConfigLoader. Test point: call it directly. Blocked: ConfigStore is a global pointer initialized at startup, cannot be replaced in test.ConfigStore* in a class. Apply Replace Global Reference with Getter in ConfigLoader — override the getter in a test subclass to return a fake ConfigStore.resolve() returns the current value when ConfigStore is populated. Write failing test: resolve() returns a default value when ConfigStore is null.Output: Bug fixed with test coverage, no regressions, global state encapsulated.
Scenario: Optimizing a Python query function in a monolith
Trigger: "This function runs a database query that's taking 4 seconds. I want to add query-result caching. Nothing has tests."
Process:
ReportService.fetch_report() queries the DB directly. Change point: the query call.fetch_report() can be called if the DB dependency is broken. Blocked: psycopg2.connect() called at module level.db_conn as a parameter instead of creating it internally. Create FakeDbConn that returns fixture data.lru_cache or a dict-based cache). Run characterization tests — they pass (same values returned). Note: cache eviction policy is out of scope for this change.Output: Caching added, behavior preserved and verified, no regression.
No supplementary reference files for this skill. The algorithm is self-contained.
For deeper sub-procedures invoked at specific steps, see the Related BookForge Skills section below.
This skill is licensed under CC-BY-SA-4.0. Source: BookForge — Working Effectively with Legacy Code by Michael C. Feathers (2004, Prentice Hall).
These skills handle specific sub-procedures invoked at steps of this algorithm:
legacy-code-symptom-router — Step 1: when you don't know where your change point is; routes you to the relevant Part II chapter for your situation. IF not installed → ask the user to describe their situation and apply the 19-symptom routing from Part II manually.test-harness-entry-diagnostics — Step 2/3: when a class cannot be instantiated in a test harness; 4-category diagnostic triage. IF not installed → ask: "What happens when you try to construct this class in a test?"characterization-test-writing — Step 4: the 5-step characterization test algorithm in detail. IF not installed → use the inline Step 4 guidance above (write a failing assertion, let the failure tell you the actual value, update the test).safe-legacy-editing-discipline — Step 3: the 4 safety constraints for editing without tests (Hyperaware Editing, Single-Goal Editing, Preserve Signatures, Lean on the Compiler). IF not installed → apply the constraints described in Step 3 above.dependency-breaking-technique-executor — Step 3: the full catalog of 24 dependency-breaking techniques with step-by-step mechanics and language guidance. IF not installed → use the abbreviated technique descriptions in Step 3 above and refer to Chapter 25 of the source book.Install the full book skill set from GitHub: bookforge-skills — working-effectively-with-legacy-code