micro-fix: shell config handling and add antigravity option#6844
micro-fix: shell config handling and add antigravity option#6844sundaram2021 wants to merge 4 commits intoaden-hive:mainfrom
Conversation
📝 WalkthroughWalkthroughAdds Antigravity subscription option and Google OAuth flow to the interactive quickstart, plus platform- and login-shell-aware shell config detection and selection logic with associated tests. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant Quickstart as "quickstart.ps1"
participant AuthScript as "core/antigravity_auth.py"
participant Google as "Google OAuth"
participant Config as "config.json / quickstart state"
User->>Quickstart: select Antigravity option
Quickstart->>AuthScript: execute antigravity_auth.py
AuthScript->>Google: initiate OAuth flow
Google-->>AuthScript: return token/credentials
AuthScript->>Config: write credentials
Quickstart->>Config: set SubscriptionMode="antigravity" and use_antigravity_subscription=true
Quickstart->>User: display Antigravity subscription status
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (2)
tools/tests/test_shell_config.py (1)
83-90: Consider adding a test for fresh system scenario (no config files exist).The current tests all create at least one config file. Adding a test for the case when no config files exist would verify the fallback behavior and catch any inconsistencies with
quickstart.sh:Suggested additional test
def test_get_shell_config_path_returns_first_candidate_when_none_exist(monkeypatch, tmp_path): """On Windows bash with no existing files, should return .bash_profile (first candidate).""" _mock_home(monkeypatch, tmp_path) monkeypatch.setenv("SHELL", "/usr/bin/bash") monkeypatch.setattr(shell_config.platform, "system", lambda: "Windows") # No config files created result = shell_config.get_shell_config_path() # Should return first candidate (.bash_profile) even though it doesn't exist assert result == tmp_path / ".bash_profile"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tools/tests/test_shell_config.py` around lines 83 - 90, Add a new unit test (e.g., test_get_shell_config_path_returns_first_candidate_when_none_exist) that uses _mock_home(monkeypatch, tmp_path), sets monkeypatch.setenv("SHELL", "/usr/bin/bash") and monkeypatch.setattr(shell_config.platform, "system", lambda: "Windows"), does NOT create any config files in tmp_path, calls shell_config.get_shell_config_path(), and asserts the result equals tmp_path / ".bash_profile" to verify the function returns the first candidate when none exist.quickstart.sh (1)
676-693: Minor inconsistency in fallback behavior when no config files exist.The shell script's fallback differs from the Python implementation:
- Shell (line 685): Returns
.profilewhen neither.bash_profilenor.bashrcexist- Python (
shell_config.py): Returns.bash_profile(first candidate) when none existThis could cause issues on a fresh Windows Git Bash system where no shell config files exist:
quickstart.shwould write API keys to.profile- A later call to
add_env_var_to_shell_config()would create.bash_profile- Different vars would end up in different files
For consistency, consider returning
.bash_profileas the fallback on Windows:Proposed fix to align fallback behavior
if [ -n "$MSYSTEM" ] || [ -n "$MINGW_PREFIX" ]; then if [ -f "$HOME/.bash_profile" ]; then echo "$HOME/.bash_profile" elif [ -f "$HOME/.bashrc" ]; then echo "$HOME/.bashrc" else - echo "$HOME/.profile" + echo "$HOME/.bash_profile" fi🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@quickstart.sh` around lines 676 - 693, The Windows/Git-Bash branch in quickstart.sh currently falls back to echoing "$HOME/.profile" when neither "$HOME/.bash_profile" nor "$HOME/.bashrc" exist, which mismatches shell_config.py and can split environment variables between files; update the MSYSTEM/MINGW_PREFIX conditional so that when no .bash_profile or .bashrc is found it echoes "$HOME/.bash_profile" (not .profile) to align behavior with shell_config.py and the add_env_var_to_shell_config() flow.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@quickstart.sh`:
- Around line 676-693: The Windows/Git-Bash branch in quickstart.sh currently
falls back to echoing "$HOME/.profile" when neither "$HOME/.bash_profile" nor
"$HOME/.bashrc" exist, which mismatches shell_config.py and can split
environment variables between files; update the MSYSTEM/MINGW_PREFIX conditional
so that when no .bash_profile or .bashrc is found it echoes
"$HOME/.bash_profile" (not .profile) to align behavior with shell_config.py and
the add_env_var_to_shell_config() flow.
In `@tools/tests/test_shell_config.py`:
- Around line 83-90: Add a new unit test (e.g.,
test_get_shell_config_path_returns_first_candidate_when_none_exist) that uses
_mock_home(monkeypatch, tmp_path), sets monkeypatch.setenv("SHELL",
"/usr/bin/bash") and monkeypatch.setattr(shell_config.platform, "system",
lambda: "Windows"), does NOT create any config files in tmp_path, calls
shell_config.get_shell_config_path(), and asserts the result equals tmp_path /
".bash_profile" to verify the function returns the first candidate when none
exist.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 01939e57-e4fc-4380-81e3-cf8b42d08a7e
📒 Files selected for processing (4)
quickstart.ps1quickstart.shtools/src/aden_tools/credentials/shell_config.pytools/tests/test_shell_config.py
There was a problem hiding this comment.
🧹 Nitpick comments (1)
quickstart.ps1 (1)
1107-1114: Consider removing remaining hardcoded menu numbers to prevent future index drift.Line 1107, Line 1179, Line 1377, and Line 1459 still repeat literal boundaries. Deriving these from shared variables would make future provider insertions safer.
Suggested refactor
+$SubscriptionChoiceCount = 7 +$ApiProviderStart = $SubscriptionChoiceCount + 1 +$ApiProviderEnd = $ApiProviderStart + $ProviderMenuEnvVars.Count - 1 +$OllamaChoice = $ApiProviderEnd + 1 +$SkipChoice = $OllamaChoice + 1 ... - $num = $idx + 8 + $num = $idx + $ApiProviderStart ... -Write-Color -Text "14" -Color Cyan -NoNewline +Write-Color -Text "$OllamaChoice" -Color Cyan -NoNewline ... -{ $_ -ge 8 -and $_ -le 13 } { - $provIdx = $num - 8 +{ $_ -ge $ApiProviderStart -and $_ -le $ApiProviderEnd } { + $provIdx = $num - $ApiProviderStart ... -14 { +$OllamaChoice {Also applies to: 1179-1200, 1377-1379, 1459-1459
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@quickstart.ps1` around lines 1107 - 1114, The switch cases set $DefaultChoice using hardcoded numeric literals (e.g., "anthropic", "openai", "minimax" assigning values to $DefaultChoice) which will break when providers are inserted; instead derive $DefaultChoice programmatically from a single authoritative providers list or mapping (e.g., compute index via an ordered array of provider names or a Hashtable of provider=>menuIndex and assign $DefaultChoice = ($providers.IndexOf('minimax') + <menu-offset>) or $providerIndexMap['minimax']), and replace all remaining literal assignments (the other similar switch blocks) to reference that shared source so menu numbers stay consistent.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@quickstart.ps1`:
- Around line 1107-1114: The switch cases set $DefaultChoice using hardcoded
numeric literals (e.g., "anthropic", "openai", "minimax" assigning values to
$DefaultChoice) which will break when providers are inserted; instead derive
$DefaultChoice programmatically from a single authoritative providers list or
mapping (e.g., compute index via an ordered array of provider names or a
Hashtable of provider=>menuIndex and assign $DefaultChoice =
($providers.IndexOf('minimax') + <menu-offset>) or
$providerIndexMap['minimax']), and replace all remaining literal assignments
(the other similar switch blocks) to reference that shared source so menu
numbers stay consistent.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 71460812-66a7-4e72-8726-3de4ddedcb0e
📒 Files selected for processing (2)
quickstart.ps1quickstart.sh
✅ Files skipped from review due to trivial changes (1)
- quickstart.sh
Description
Restores Windows quickstart parity with the Bash flow.
This PR fixes two Windows-specific quickstart issues:
queenmodel.quickstart.ps1was missing the Antigravity subscription option even though the underlying support already existed.Type of Change
Changes Made
.bash_profile,.bashrc,.profile) for persisted credentials likeHIVE_API_KEYtools/tests/test_shell_config.pyquickstart.ps1, including credential detection, previous-selection defaulting, menu entry, selection handling, config persistence, and success summary outputTesting
Describe the tests you ran to verify your changes:
cd core && pytest tests/)cd core && ruff check .)Manual / focused verification run:
cd tools && uv run pytest tests/test_shell_config.pycd core && uv run pytest tests/test_credential_bootstrap.pyquickstart.ps1bash -n quickstart.shChecklist
Summary by CodeRabbit
New Features
Improvements
Tests