refactor(migrations)!: empty run-once migration registry for 2.0 - #263
Merged
Conversation
Remove all 20 1.x migration entries from MIGRATIONS (and the registry-only helpers: moveFile, moveDirContents, MEMORY_LEGACY_SKIP_FILES, CONSOLIDATE_STALE_GITIGNORE_ENTRIES, cleanStaleGitignoreEntries, migrateMemoryDir, seedFeatureConfigFromDream). Drop the learning-queue-cleanup and feature-config imports; keep only getMemoryDir from project-paths. Remove dead rendererPath? field from PerProjectMigrationContext. Registry is now an empty readonly array — the framework (types, runner, marker-file I/O) stays intact so 2.x migrations can be appended. Test suite: delete 14 entry-specific describe blocks, convert 5 registry-coupled tests to registryOverride stubs, add toEqual([]) lock-in test, fix stale harness comment. Drop vi import (no more vi.spyOn uses). Delete tests/decisions/decisions-ledger-migration.test.ts (its render-decisions-index-v1 registration block was broken by the empty registry; module itself deleted in the next commit). Co-Authored-By: Claude <noreply@anthropic.com>
Delete 4 production modules whose only TS consumers were the 1.x migration entries removed in the previous commit: - src/cli/utils/legacy-decisions-purge.ts - src/cli/utils/decisions-ledger-migration.ts - src/cli/utils/marketplace-cleanup.ts - src/cli/utils/mkdir-lock.ts (transitively dead via the first two) Delete corresponding test files: - tests/legacy-decisions-purge.test.ts - tests/mkdir-lock.test.ts (tests/decisions/decisions-ledger-migration.test.ts was deleted in the previous commit as it was already broken by the empty MIGRATIONS registry) Delete dead export: stripDevflowTeammateMode (async file wrapper) from teammate-mode-cleanup.ts — production-dead now that the two teammate migrations are gone; uninstall.ts uses only stripDevflowTeammateModeFromJson. Prune the wrapper's tests from tests/teammate-mode-cleanup.test.ts. Comment-only fixes in fs-atomic.ts, observations.ts, flags.ts, feature-config.ts, tests/decisions/json-helper-write-exclusive.test.ts, and tests/learning-config.test.ts to remove stale references to the deleted modules. Co-Authored-By: Claude <noreply@anthropic.com>
Remove all 1.x migration IDs from CLAUDE.md, KNOWLEDGE.md files, and CHANGELOG.md. Add a Removed entry for the retired helper modules.
- migrations.ts: drop "as of 2.0 / no 1.x upgrade path / 2.x" framing from module docblock, MIGRATIONS docblock, and runMigrations docblock - tests/migrations.test.ts: remove dead homeDevflowDir scaffolding, strip claudeDir excess properties, reword end-state comment and test title - learning-queue-cleanup.ts: rewrite header to name actual callers only (learning.ts --reset / --clear / --disable); remove migration reference - json-helper.cjs: remove migrateDecisionsLedger mention from D002 comment - .devflow/features/index.md: drop marketplace-cleanup.ts from installer-shadowing directory list and keyword - CHANGELOG.md: drop purge-v3 migration bullet (never shipped), remove Self-learning reconciler bullet (self-learning removed), strip "auto-purge on migration" clause from Learning validation bullet - tests/decisions/json-helper-write-exclusive.test.ts: fix header path (was tests/learning/, is tests/decisions/)
Owner
Author
Code Review FindingsAll reviewers APPROVED this PR with no blocking issues. This is a high-quality, clean deletion sweep (133 insertions / 6557 deletions, 8-10/10 scores across all 10 focuses). Summary of all findings: Inline Comments (≥80% confidence)The following require inline fixes:
Summary Comments (60-79% or pre-existing)
Test Coverage Note
RecommendationAPPROVED — subject to the three fixes above This PR is an exemplary cleanup: framework-vs-registry separation is clean per ADR-012, all deletions are verified safe (TypeScript passes, tests pass, zero regressions), and CLAUDE.md/CHANGELOG are accurate end-state descriptions. The three "should-fix" findings are minor ADR-003 residue polish — metadata pointers and comments describing the now-deleted migration architecture that survived the machinery sweep. Claude Code Review | /code-review |
dean0x
commented
Jul 17, 2026
| @@ -195,988 +72,10 @@ export interface Migration<S extends MigrationScope = MigrationScope> { | |||
| * | |||
| * The `scope` field distinguishes global (one run per machine, no project context | |||
| * needed) from per-project (sweeps every discovered Claude-enabled project root). | |||
Owner
Author
There was a problem hiding this comment.
SHOULD FIX — Dead Exported Type (Confidence: 90%)
export type MigrationContext = GlobalMigrationContext | PerProjectMigrationContext;This union is never imported or consumed anywhere. The Migration.run signature uses the conditional type directly, and the runner constructs concrete context objects directly.
Per ADR-003, this PR sweeps migration residue. A dead union type is exactly the kind of leftover the end-state should remove.
Fix: Delete this line.
Claude Code Review | /code-review
Two JSDoc lines in the LedgerRow interface comment (D202) referenced the now-deleted decisions-ledger-migration.ts. Reword to describe the end-state: rows are written by assign-anchor / retire-anchor, and the renderer spread-merges them — no migration involved. applies ADR-003
…ateModeFromJson "both functions" was stale — the async wrapper was deleted, only one function remains. "cause the migration to record a failure, retrying on every devflow init" was also stale — there is no migration involved; the sole caller is uninstall.ts. Reworded to "surface as an unhandled error during uninstall" to match the framing already used in flags.ts. The avoids PF-004 citation is preserved — the non-object root tolerance is a real, live invariant. applies ADR-003.
…omments ISSUE-4: Delete the exported MigrationContext union alias — Migration.run already uses a conditional type over GlobalMigrationContext / PerProjectMigrationContext directly; the alias had no callers. Retarget D38 to describe the two concrete context types explicitly (applies ADR-003). ISSUE-6: Reword D37 vacuous-truth comment to describe the empty-registry no-op generically, removing the "no legacy entries to purge" sentence that referenced the deleted 1.x purge migration (applies ADR-003). ISSUE-8: Replace the brittle init.ts:962-974 line-number reference in D35 with the symbol name `installClaudeignore` — avoids doc drift when line numbers shift (applies ADR-003). Co-Authored-By: Claude <noreply@anthropic.com>
The reworked runMigrations tests used void-returning stubs exclusively, leaving the non-void branch of normaliseRunResult (migrations.ts:213) and the infos/warnings aggregation loops (lines 311-313, 362-363, 371-372) uncovered. Add one focused test that injects global and per-project stub migrations whose run() returns MigrationRunResult, asserting the infos and warnings are aggregated into the RunMigrationsResult (ISSUE-1). Co-Authored-By: Claude <noreply@anthropic.com>
Shorten the test name to match the concise style of other tests in the runMigrations describe block. Remove the // Covers: annotation with implementation line numbers — no other test in the file uses this pattern and line numbers drift as the source file evolves.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
MIGRATIONSinsrc/cli/utils/migrations.ts; keep the full migration framework (types, runner, marker-file I/O) for future 2.x entrieslegacy-decisions-purge.ts,decisions-ledger-migration.ts,marketplace-cleanup.ts,mkdir-lock.tsand their test filestests/migrations.test.tsto useregistryOverridestubs — no entry-specific tests remain; the framework behavior tests (D33 non-fatal failures, D37 vacuous-truth, per-project failure) all passChanges
Commit 1 —
refactor(migrations)!: empty run-once migration registry for 2.0src/cli/utils/migrations.ts: stripped all 20 migration entry consts + registry-only helpers;MIGRATIONS = []; droppedrendererPath?fromPerProjectMigrationContexttests/migrations.test.ts: rewritten —registryOverridestubs for all framework tests; 14 entry-specific describe blocks deletedtests/decisions/decisions-ledger-migration.test.ts: deleted (referenced migration entries that no longer exist)Commit 2 —
refactor(utils): delete dead 1.x migration helper modulessrc/cli/utils/legacy-decisions-purge.ts,decisions-ledger-migration.ts,marketplace-cleanup.ts,mkdir-lock.tstests/legacy-decisions-purge.test.ts,tests/mkdir-lock.test.tsteammate-mode-cleanup.ts: removedstripDevflowTeammateModeasync wrapper (no longer called); keptstripDevflowTeammateModeFromJson(used by uninstall)fs-atomic.ts,observations.ts,flags.ts,feature-config.ts, and test files to remove stale referencesCommit 3 —
docs: update migration docs for the empty 2.0 registryCLAUDE.md: Migrations paragraph rewritten — no 1.x IDs, updated framework descriptionCHANGELOG.md: Added### Removedbullet for the retired entries and moduleslearning-capture-system/KNOWLEDGE.md: removed### Migrationssubsection and D37 recovery bulletinstaller-shadowing/KNOWLEDGE.md: removed marketplace-cleanup referencesBreaking Changes
All 20 registered 1.x upgrade migrations are removed. Users upgrading from a pre-2.0 devflow to 2.0 will not run any of these migrations. This is intentional — all migrations are run-once-per-machine; the 2.0 release closes the upgrade window.
Reviewer Focus Areas
src/cli/utils/migrations.tslines 1–60: verify framework intact,MIGRATIONS = [],PerProjectMigrationContexthas norendererPath?tests/migrations.test.ts: all five reworkedrunMigrationstests useregistryOverride(noMIGRATIONScoupling); three framework-behavior tests (D33/D37/per-project) pass verbatimgrep -r "legacy-decisions-purge\|decisions-ledger-migration\|marketplace-cleanup\|mkdir-lock" src/)