Skip to content

Conversation

@andyrewlee
Copy link
Owner

@andyrewlee andyrewlee commented Jan 26, 2026

Copy link

@devin-ai-integration devin-ai-integration bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devin Review found 2 potential issues.

View issues and 5 additional flags in Devin Review.

Open in Devin Review

Copy link

@devin-ai-integration devin-ai-integration bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devin Review found 1 new potential issue.

View issue and 7 additional flags in Devin Review.

Open in Devin Review

Comment on lines +225 to +229
legacyWorkspacesRoot := filepath.Join(cfg.Paths.Home, "worktrees")
if info, err := os.Stat(legacyWorkspacesRoot); err == nil && info.IsDir() {
logging.Warn("Falling back to legacy workspaces path: %s", legacyWorkspacesRoot)
cfg.Paths.WorkspacesRoot = legacyWorkspacesRoot
}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 Fallback logic overwrites successfully migrated paths when partial migration fails

When workspaces migration succeeds but metadata migration fails, the fallback logic incorrectly reverts BOTH paths to legacy locations, causing data loss.

Click to expand

Scenario

  1. RunMigrations() successfully copies workspaces (MigratedWorkspacesRoot = true)
  2. Metadata migration fails (e.g., permission error), setting result.Error
  3. In app.go:223-236, when migrationResult.Error != nil, the fallback logic checks if legacy paths exist and unconditionally falls back to them
  4. Even though workspaces were successfully migrated to the new location, cfg.Paths.WorkspacesRoot is reset to the legacy path

Code flow:

// migrate.go:41 - workspaces migration succeeds
result.MigratedWorkspacesRoot = migrated  // true

// migrate.go:48-50 - metadata migration fails
if err != nil {
    result.Error = err
    return result  // MigratedWorkspacesRoot is still true
}
// app.go:223-229 - fallback ignores successful migration
if migrationResult.Error != nil {
    // Falls back BOTH paths, even though workspaces succeeded
    if info, err := os.Stat(legacyWorkspacesRoot); err == nil && info.IsDir() {
        cfg.Paths.WorkspacesRoot = legacyWorkspacesRoot  // WRONG!
    }
}

Impact

The application will read from the legacy workspaces directory instead of the newly migrated one, potentially causing:

  • Users see stale/old workspace data
  • New changes are written to wrong location
  • Data inconsistency between workspaces and metadata

Recommendation: Check migrationResult.MigratedWorkspacesRoot before falling back: only fall back to legacy workspaces path if the workspaces migration did NOT succeed. Similarly for metadata.

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

@andyrewlee andyrewlee merged commit 4e06005 into main Jan 26, 2026
1 check passed
@andyrewlee andyrewlee deleted the migration branch January 26, 2026 18:01
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.

2 participants