fix: print auth URL and prompt for token when browser open fails#162
fix: print auth URL and prompt for token when browser open fails#162greynewell merged 2 commits intomainfrom
Conversation
|
Warning Rate limit exceeded
To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
WalkthroughThree new test files validate that installation scripts provide proper user guidance and that authentication gracefully falls back to manual token entry when browser opening fails in headless environments. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Review rate limit: 0/5 reviews remaining, refill in 3 minutes and 32 seconds. Comment |
…ronments When the browser cannot be opened (headless/SSH/container environments), Login now prints the CLI auth URL (with port and state) so the user can visit it from another machine, then prompts them to paste their API key. Three package-level vars make the behaviour testable without exec or os coupling: openBrowserFunc, stdinReader, and loginOut. Closes #155. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
greynewell
force-pushed
the
fix/headless-auth-fallback
branch
from
7444247 to
17500d3
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@cmd/install_script_test.go`:
- Around line 16-29: The test expects install.sh to NOT run the setup subcommand
and to include a getting-started message; update install.sh to match this by
removing or guarding the literal invocation of "$INSTALL_DIR/$BINARY" setup (the
current invocation that runs setup) and instead print a clear getting-started
message that includes either "Run 'supermodel'" or the phrase "get started" (so
the test's checks in the t.Run blocks for absence of `" setup"`/`"supermodel
setup"` and presence of "run 'supermodel'" or "get started" pass). Ensure the
printed message appears during install and that no direct call to the setup
subcommand is executed.
In `@cmd/npm_package_test.go`:
- Around line 27-33: The test t.Run("includes getting-started message") expects
npm/install.js to print a getting-started message containing either "run
'supermodel'" or "get started"; update npm/install.js (the script that logs
download/install status around the current block near Lines 110-128) to append a
concise post-install message that includes one of those exact phrases (e.g.,
"Get started: run 'supermodel' in your project directory") so the contains
checks in the test (hasRunSupermodel / hasGetStarted) pass. Ensure the message
is emitted to the same output stream inspected by the test.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: cab95002-5897-4eee-860e-ef7d3d2af91c
📒 Files selected for processing (3)
cmd/install_script_test.gocmd/npm_package_test.gointernal/auth/handler_test.go
| t.Run("does not run setup wizard at install time", func(t *testing.T) { | ||
| // The script uses $BINARY variable, so match the literal subcommand argument | ||
| if strings.Contains(content, `" setup`) || strings.Contains(content, "supermodel setup") { | ||
| t.Error("install.sh must not run the setup subcommand at install time — the wizard now auto-launches from bare 'supermodel' in a project directory (PR #152)") | ||
| } | ||
| }) | ||
|
|
||
| t.Run("includes getting-started message", func(t *testing.T) { | ||
| lower := strings.ToLower(content) | ||
| hasRunSupermodel := strings.Contains(lower, "run 'supermodel'") || strings.Contains(lower, "run \"supermodel\"") | ||
| hasGetStarted := strings.Contains(lower, "get started") | ||
| if !hasRunSupermodel && !hasGetStarted { | ||
| t.Error("install.sh must include a getting-started message directing users to run 'supermodel' in their project directory") | ||
| } |
There was a problem hiding this comment.
Test contradicts current install.sh behavior
Line 16-Line 20 expects no setup invocation, but install.sh currently invokes setup ("$INSTALL_DIR/$BINARY" setup around Line 81 in that file). Line 23-Line 29 also expects a getting-started message that isn’t currently present. This test will fail unless install.sh is updated accordingly.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@cmd/install_script_test.go` around lines 16 - 29, The test expects install.sh
to NOT run the setup subcommand and to include a getting-started message; update
install.sh to match this by removing or guarding the literal invocation of
"$INSTALL_DIR/$BINARY" setup (the current invocation that runs setup) and
instead print a clear getting-started message that includes either "Run
'supermodel'" or the phrase "get started" (so the test's checks in the t.Run
blocks for absence of `" setup"`/`"supermodel setup"` and presence of "run
'supermodel'" or "get started" pass). Ensure the printed message appears during
install and that no direct call to the setup subcommand is executed.
| t.Run("includes getting-started message", func(t *testing.T) { | ||
| lower := strings.ToLower(content) | ||
| hasRunSupermodel := strings.Contains(lower, "run 'supermodel'") || strings.Contains(lower, `run "supermodel"`) | ||
| hasGetStarted := strings.Contains(lower, "get started") | ||
| if !hasRunSupermodel && !hasGetStarted { | ||
| t.Error("npm/install.js must print a getting-started message directing users to run 'supermodel' in their project directory") | ||
| } |
There was a problem hiding this comment.
Expectation doesn’t match current npm installer output
Line 27-Line 33 require "run 'supermodel'" or "get started" in npm/install.js, but current installer output (see npm/install.js around Lines 110-128) only logs download/install status. This test will fail unless the script is updated in the same PR.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@cmd/npm_package_test.go` around lines 27 - 33, The test t.Run("includes
getting-started message") expects npm/install.js to print a getting-started
message containing either "run 'supermodel'" or "get started"; update
npm/install.js (the script that logs download/install status around the current
block near Lines 110-128) to append a concise post-install message that includes
one of those exact phrases (e.g., "Get started: run 'supermodel' in your project
directory") so the contains checks in the test (hasRunSupermodel /
hasGetStarted) pass. Ensure the message is emitted to the same output stream
inspected by the test.
1 participant
Closes #155.
In headless/SSH/container environments, the browser-based OAuth flow fails silently, leaving users stuck. Fix: detect browser open failure and fall back to printing the CLI auth URL and prompting the user to paste their API key.
TDD: failing test first.
🤖 Generated with Claude Code
Summary by CodeRabbit