Skip to content

fix(config): prevent base_path from flipping to ~/.forge when ~/forge still holds user data#2925

Merged
tusharmath merged 3 commits intomainfrom
change-bin
Apr 10, 2026
Merged

fix(config): prevent base_path from flipping to ~/.forge when ~/forge still holds user data#2925
tusharmath merged 3 commits intomainfrom
change-bin

Conversation

@tusharmath
Copy link
Copy Markdown
Collaborator

@tusharmath tusharmath commented Apr 10, 2026

Summary

Fix ProviderNotAvailable errors caused by base_path() silently switching from ~/forge to ~/.forge when an empty ~/.forge directory was created as a side effect of the shell plugin's config-edit action.

Context

After adding the forge config migrate command (which teaches users to move from ~/forge to ~/.forge), the shell plugin's _forge_action_config_edit function was updated to call forge config path dynamically and then run mkdir -p <config_dir> to ensure the config directory exists.

The problem: forge config path returns ~/.forge/.forge.toml as the new default (since 26badbaac changed base_path() to prefer ~/.forge). So for users who still have their data in ~/forge, the shell plugin would eagerly create an empty ~/.forge directory. This caused base_path() to flip:

Before mkdir:  ~/forge exists, ~/.forge does not → base_path() = ~/forge  ✓
After mkdir:   ~/forge exists, ~/.forge exists   → base_path() = ~/.forge ✗

With base_path() now returning ~/.forge, credentials_path() pointed to ~/.forge/.credentials.json — a file that doesn't exist because all credentials are in ~/forge/.credentials.json. read_credentials() returned an empty list, every provider appeared unconfigured, and every provider lookup raised ProviderNotAvailable.

Changes

  • crates/forge_config/src/reader.rsbase_path() now returns ~/forge whenever that directory exists, regardless of whether ~/.forge also exists. The legacy path takes priority until forge config migrate removes it, at which point the fallback stops applying and ~/.forge is used correctly.
  • shell-plugin/lib/actions/config.zsh — fixed hardcoded forge binary name to use $FORGE_BIN variable (so the correct binary is invoked in environments where forge is installed under a different name or path).
  • Tests updated to accept either ~/.forge or ~/forge as valid base_path results depending on the test machine's state.

Testing

Reproduce and verify the fix:

# 1. Set up the broken state: have ~/forge with credentials, and an empty ~/.forge
mkdir -p ~/forge
echo '[]' > ~/forge/.credentials.json
mkdir -p ~/.forge   # simulates what the shell plugin's mkdir was doing

# 2. Before fix: forge fails to find credentials
forge info  # → ProviderNotAvailable

# 3. Apply the fix and rebuild
cargo build -p forge_config

# 4. After fix: base_path stays on ~/forge, credentials found correctly
forge info  # → shows provider correctly

# 5. Cleanup
rm -rf ~/.forge

@tusharmath tusharmath changed the title change bin fix(config): prevent base_path from flipping to ~/.forge when ~/forge still holds user data Apr 10, 2026
@github-actions github-actions bot added the type: fix Iterations on existing features or infrastructure. label Apr 10, 2026
Comment on lines +205 to +212
// Without FORGE_CONFIG set the path must be either ".forge" (new) or
// "forge" (legacy fallback when ~/forge exists on this machine).
let name = actual.file_name().unwrap();
assert!(
name == ".forge" || name == "forge",
"Expected base_path to end with '.forge' or 'forge', got: {:?}",
name
);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Test is now non-deterministic and depends on the host filesystem state. If ~/forge exists on the CI/CD runner or developer's machine, the test validates different behavior than if it doesn't exist. This makes test results non-reproducible and can hide regressions.

Impact: Different environments will test different code paths, reducing test reliability.

Fix: Use filesystem isolation for deterministic testing:

#[test]
fn test_base_path_prefers_legacy_when_it_exists() {
    let temp = TempDir::new().unwrap();
    env::set_var("HOME", temp.path());
    
    fs::create_dir(temp.path().join("forge")).unwrap();
    let actual = ConfigReader::base_path();
    assert_eq!(actual.file_name().unwrap(), "forge");
}

Spotted by Graphite

Fix in Graphite


Is this helpful? React 👍 or 👎 to let us know.

@tusharmath tusharmath merged commit 8241bb8 into main Apr 10, 2026
10 checks passed
@tusharmath tusharmath deleted the change-bin branch April 10, 2026 09:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

type: fix Iterations on existing features or infrastructure.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant