Skip to content

feat(migration): auto-migrate configs after CLI upgrade#25

Merged
josephgoksu merged 2 commits intomainfrom
feat/post-upgrade-auto-migration
Mar 9, 2026
Merged

feat(migration): auto-migrate configs after CLI upgrade#25
josephgoksu merged 2 commits intomainfrom
feat/post-upgrade-auto-migration

Conversation

@josephgoksu
Copy link
Owner

@josephgoksu josephgoksu commented Mar 9, 2026

Summary

  • After brew upgrade, users have stale configs (old tw-* slash command files, legacy taskwing-mcp MCP entries). This adds a lightweight version check to PersistentPreRunE that auto-repairs local configs silently on the next command after upgrade.
  • Stamps .taskwing/version during bootstrap; on version mismatch, regenerates slash commands for all managed AIs (pruning stale tw-* files) and warns about legacy global MCP server names.
  • Happy path is sub-millisecond (1 stat + 1 file read + string compare). Migration path ~100ms, runs once per version per project. Skipped for version, help, mcp commands and dev builds.

Files changed

File Change
internal/migration/upgrade.go NewCheckAndMigrate() core logic
internal/migration/upgrade_test.go New — 7 tests (skip cases, migration, idempotency, warnings)
cmd/root.go Hook maybeRunPostUpgradeMigration into PersistentPreRunE
cmd/bootstrap.go Pass CLI version to service for stamping
internal/bootstrap/initializer.go Version field, AIHelperByName() accessor, version stamp in createStructure()
internal/bootstrap/service.go SetVersion() method

Test plan

  • go test ./... -count=1 -short — 193 tests pass across 32 packages
  • go build ./... — clean build
  • Manual: echo "1.21.4" > .taskwing/version && ./bin/taskwing doctor → verify old tw-* files removed, new taskwing/ commands created, version updated
  • Manual: verify no output on repeated run (idempotent)
  • Manual: verify taskwing mcp (stdio server) is not affected

Greptile Summary

This PR adds a lightweight post-upgrade migration system that detects CLI version changes after brew upgrade and automatically regenerates stale AI slash command files and warns about legacy MCP server names. The design intent is solid — version-stamp at bootstrap, compare on every command invocation, migrate once and re-stamp. The happy path is correctly sub-millisecond and the migration is non-fatal throughout.

Key issues found:

  1. Broken mcp subcommand skip (cmd/root.go): cmd.Name() returns the leaf command name, so any subcommand under mcp (e.g., mcp install) has cmd.Name() != "mcp", bypassing the guard. The migration will run for subcommands when it shouldn't. Fix requires walking up the parent chain to detect the entire mcp subtree.

  2. Silent infinite re-migration loop (internal/migration/upgrade.go): Both version stamp writes silently discard errors with _ = . On a read-only or full filesystem, the version is never updated, causing the migration to re-run on every command indefinitely with no user feedback — a hidden performance regression.

  3. init shadows Go built-in (internal/migration/upgrade.go): The local variable init := bootstrap.NewInitializer(...) shadows Go's reserved init identifier. golangci-lint will flag this; rename to initializer.

  4. CreateSlashCommands errors silently discarded (internal/migration/upgrade.go): Failed slash command regeneration (e.g., permissions) is swallowed — the broken state is marked as successfully migrated and never retried, with zero diagnostic feedback.

The implementation approach is sound, but these four correctness gaps should be addressed: the mcp skip guard doesn't protect subcommands, silent write failures create a performance regression, lint violations, and missing error visibility.

Confidence Score: 3/5

  • Mergeable with known correctness issues — migration is non-fatal so no user-blocking bugs, but the mcp skip guard, silent write failures, and error handling gaps should be fixed before production use at scale.
  • Four verified logic and style issues present. The mcp skip guard doesn't protect subcommands (allowing migration to run unexpectedly), both version write operations silently ignore errors (creating permanent re-migration loops on filesystem failures), the init variable shadows Go's built-in (lint failure), and CreateSlashCommands errors are discarded (hiding broken states). The silent write-failure issue is particularly concerning because it turns the happy-path ~1ms check into a permanent per-command loop on any filesystem error. However, the migration is non-fatal and correctness gaps are fixable without redesign.
  • internal/migration/upgrade.go (3 issues: init shadowing, write error handling, CreateSlashCommands errors) and cmd/root.go (mcp skip guard incomplete) require the most attention.

Sequence Diagram

sequenceDiagram
    participant User
    participant RootCmd as cmd/root.go (PersistentPreRunE)
    participant Migration as migration.CheckAndMigrate
    participant FS as Filesystem (.taskwing/version)
    participant Bootstrap as bootstrap.Initializer
    participant MCPCfg as mcpcfg.IsLegacyServerName

    User->>RootCmd: taskwing <any-command>
    RootCmd->>RootCmd: initTelemetry()
    RootCmd->>RootCmd: maybeRunPostUpgradeMigration(cmd)
    Note over RootCmd: Skip if cmd.Name() == "version"|"help"|"mcp"<br/>(⚠️ does NOT match mcp subcommands)
    RootCmd->>Migration: CheckAndMigrate(cwd, version)
    Migration->>FS: os.Stat(.taskwing/)
    alt Not bootstrapped
        Migration-->>RootCmd: nil, nil (no-op)
    else Bootstrapped
        Migration->>FS: os.ReadFile(.taskwing/version)
        alt Version file missing
            Migration->>FS: os.WriteFile(version) ⚠️ error silently dropped
            Migration-->>RootCmd: nil, nil
        else Version matches
            Migration-->>RootCmd: nil, nil (happy path, ~sub-ms)
        else currentVersion == "dev"
            Migration-->>RootCmd: nil, nil (skip dev builds)
        else Version mismatch
            Migration->>Bootstrap: migrateLocalConfigs(projectDir)
            loop For each managed AI
                Bootstrap->>FS: Check marker file
                Bootstrap->>Bootstrap: NewInitializer(projectDir)<br/>⚠️ named "init" (shadows built-in)
                Bootstrap->>Bootstrap: CreateSlashCommands(aiName)<br/>⚠️ error silently dropped
            end
            Migration->>MCPCfg: checkGlobalMCPLegacy()
            MCPCfg-->>Migration: []warnings
            Migration->>FS: os.WriteFile(version) ⚠️ error silently dropped
            Migration-->>RootCmd: warnings, nil
        end
    end
    RootCmd->>User: fmt.Fprintf(stderr, warnings...)
Loading

Fix All in Claude Code

Last reviewed commit: d6a52c6

Greptile also left 4 inline comments on this PR.

After `brew upgrade`, users have stale configs (old tw-* slash command
files and legacy taskwing-mcp MCP entries). This adds a lightweight
version check to PersistentPreRunE that detects version mismatches
and silently regenerates local configs on the next command.

- Add internal/migration package with CheckAndMigrate()
- Hook into root command PersistentPreRunE (skips version/help/mcp)
- Stamp .taskwing/version during bootstrap for change detection
- Silently regenerate slash commands for managed AIs on version change
- Warn (stderr) about legacy global MCP server names
- Sub-millisecond happy path (1 stat + 1 read + string compare)
- 7 tests covering skip cases, migration, idempotency, and warnings
- Rename `init` variable to `initializer` (reserved identifier)
- Walk parent chain to skip entire mcp subtree, not just leaf command
- Warn on stderr when version stamp write fails (prevents silent retry loop)
- Warn on stderr when slash command regeneration fails (visibility)
@josephgoksu josephgoksu merged commit d199e62 into main Mar 9, 2026
9 checks passed
@josephgoksu josephgoksu deleted the feat/post-upgrade-auto-migration branch March 9, 2026 11:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant