Skip to content

refactor: remove migrate_env_credentials and MigrationResult#2774

Open
tusharmath wants to merge 2 commits intomainfrom
drop-cred-migration
Open

refactor: remove migrate_env_credentials and MigrationResult#2774
tusharmath wants to merge 2 commits intomainfrom
drop-cred-migration

Conversation

@tusharmath
Copy link
Copy Markdown
Collaborator

@tusharmath tusharmath commented Apr 1, 2026

Summary

Remove the migrate_env_credentials one-time migration path that existed to move API keys from environment variables to the file-based credentials store, along with the MigrationResult domain type and all associated plumbing.

Context

When file-based credential storage was first introduced, a migration helper was added so that users with existing OPENAI_API_KEY/ANTHROPIC_API_KEY environment variables would have those credentials automatically written to the credentials file on the first run. That migration has now served its purpose — any user who was affected has already been migrated — making this code dead weight. Keeping it in the API trait, service layer, repository layer, and UI creates unnecessary surface area and maintenance burden.

Changes

  • Deleted forge_domain/src/migration.rs — removes the MigrationResult struct and its module entirely.
  • Removed migrate_env_credentials from ProviderRepository trait (forge_domain/src/repo.rs) and all implementations/mocks across forge_repo, forge_app, forge_services, and forge_api.
  • Removed migrate_env_to_file from ForgeProviderRepository (forge_repo/src/provider/provider_repo.rs) along with the now-dead UrlParamVarConfig::param_name helper and env-scanning logic (~100 lines).
  • Removed handle_migrate_credentials from UI (forge_main/src/ui.rs) and its call-site in init_state; the Info command now correctly calls on_new() to initialise the model instead of the lighter init_state(false).
  • Simplified generate_commit_message (forge_app/src/git_app.rs) — dropped the retry wrapper and the Clone derive on DiffContext; also removed the now-redundant empty-message error guard.
  • Fixed total_lines reporting in operation.rs — the attribute now reflects the number of lines actually returned to the caller rather than the total line count of the full file, correcting snapshot tests accordingly.

Key Implementation Details

The migration function iterated environment variables, matched known provider patterns, and wrote them to a JSON credentials file if the file did not yet exist. All of this logic lived inside ForgeProviderRepository::migrate_env_to_file. Removing it required stripping the method from every layer: domain trait → repository implementation → service wrapper → API trait → UI call-site. Mock implementations in tests were updated to drop the stub.

Testing

# Run all tests with snapshot acceptance
cargo insta test --accept

# Quick type-check to confirm no dangling references
cargo check

Links

@github-actions github-actions bot added the type: refactor Code refactoring and restructuring. label Apr 1, 2026
@tusharmath tusharmath changed the title refactor(api): remove migrate_env_credentials from API trait and implementation refactor: remove migrate_env_credentials and MigrationResult Apr 1, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

type: refactor Code refactoring and restructuring.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant