You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
/tmp/go-apt123/root/usr/lib/go-1.23/bin/go test ./internal/config -run 'TestResolveEnvVarSearchesHomeVixDotEnv|TestResolveProviderCredentialBaseURLFromDotEnv|TestMiMoTokenPlanCredential'
/tmp/go-apt123/root/usr/lib/go-1.23/bin/go test ./internal/config
Thanks for tackling #29 and #30. A few things to address before I can merge:
Security hole: a .env file in any project folder can steal the user's API key. Your change lets a .env file in the current folder change where requests are sent (ANTHROPIC_BASE_URL). But the actual API key still comes from the secure keychain. So if someone clones a shady repo that contains a .env pointing at their own server, vix will happily send the user's real key to that server. Fix: only read the base URL setting from trusted places (~/.vix/.env and the file next to the program), never from the current project folder.
Too magical: it quietly turns on a URL override for every provider. Your code assumes "if the key variable is FOO_API_KEY, then FOO_BASE_URL overrides the URL" — for all providers, even ones meant to always use a fixed address (like OpenRouter), with no way to turn it off. The project already has a proper, explicit way to declare these overrides in the config. Use that existing mechanism instead of guessing from the variable name.
Add a test for the bad case. Right now you only test that the override works. Also add a test proving a provider that shouldn't be overridable ignores a stray *_BASE_URL. An end-to-end test would be nice too.
Use the right Go version. Your test output shows Go 1.23, but this project needs Go 1.26+. Please re-run on the correct version.
Wrong co-author (blocking). Your commit lists bridge as a co-author. Our rules say AI helpers must be credited as vix, not anything else. Please change the commit's co-author line to Co-authored-by: vix <290354907+vix-agent@users.noreply.github.com>, remove the bridge one, and force-push.
@zerone0x thanks again for your PR. Do you plan to address my feedback? I would like to get this merged, but if you don't plan on continuing working on it I'll close the PR. Thank you
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
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
~/.vix/.envin credential env-file lookup*_BASE_URLoverrides through the same env-file search path.envwith config testsFixes #29
Fixes #30
Tests
/tmp/go-apt123/root/usr/lib/go-1.23/bin/go test ./internal/config -run 'TestResolveEnvVarSearchesHomeVixDotEnv|TestResolveProviderCredentialBaseURLFromDotEnv|TestMiMoTokenPlanCredential'/tmp/go-apt123/root/usr/lib/go-1.23/bin/go test ./internal/config