feat: auto-launch setup wizard on first run of bare 'supermodel'#152
feat: auto-launch setup wizard on first run of bare 'supermodel'#152greynewell merged 2 commits intomainfrom
Conversation
When a user runs 'supermodel' with no API key configured, decide what
to do based on whether stdin is interactive:
- TTY: hand off to setup.Run (the wizard already ends in
shards.Watch, so onboarding flows straight into watch).
- Non-TTY: return a clean 'not authenticated' error so CI scripts
can't hang on a browser-auth prompt.
Together this collapses the canonical first-run flow from three
commands to one:
curl -fsSL https://supermodeltools.com/install.sh | sh
cd /path/to/repo
supermodel # ← was: supermodel setup
'supermodel setup' remains as the explicit reconfigure entry point.
Subcommands keep their existing pre-run gate.
The decision is extracted as pickRootAction(hasAPIKey, interactive)
so the matrix is unit-testable without invoking the wizard or the
daemon. cmd/root_test.go covers all four cases.
Refs #151.
WalkthroughRoot command startup now dispatches based on whether an API key exists and whether stdin is a TTY, choosing to run the watch daemon, launch the interactive setup, or return an unauthenticated error. Subcommand auth gating is centralized in a new persistentPreRunE that exempts no-config commands. Tests cover the pickRootAction logic. Changes
Sequence Diagram(s)sequenceDiagram
participant CLI as "CLI (supermodel)"
participant Root as "root command logic\n(pickRootAction)"
participant Config as "Config Loader"
participant Watch as "Watch daemon"
participant Setup as "Setup wizard"
CLI->>Root: start
Root->>Config: load (if not exempt)
alt cfg.APIKey present
Root->>Watch: runWatch()
Watch-->>CLI: starts watching
else cfg.APIKey missing
Root->>Root: stdinIsTerminal?
alt interactive (TTY)
Root->>Setup: runSetup()
Setup-->>CLI: interactive setup flow
else non-interactive
Root-->>CLI: errNotAuthenticated
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Review rate limit: 4/5 reviews remaining, refill in 12 minutes. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
cmd/root.go (1)
62-70: Prefer returning errors here instead of callingos.Exitinside pre-run.This keeps command flow consistent and testable (single exit point in
Execute()), and avoids abrupt termination from deep in the call stack.Refactor sketch
cfg, err := config.Load() if err != nil { - fmt.Fprintf(os.Stderr, "Error loading config: %v\n", err) - os.Exit(1) + return fmt.Errorf("error loading config: %w", err) } if cfg.APIKey == "" { - fmt.Fprintln(os.Stderr, "Run 'supermodel setup' to get started.") - os.Exit(1) + return fmt.Errorf("run 'supermodel setup' to get started") } return nil }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cmd/root.go` around lines 62 - 70, The pre-run in cmd/root.go currently calls os.Exit on config.Load() failure and when cfg.APIKey is empty; change this to return errors instead so the command flow is testable—replace the fmt.Fprintf(os.Stderr, "...")/fmt.Fprintln(...) + os.Exit calls in the pre-run with returned errors (e.g., wrap the original err from config.Load() with fmt.Errorf and return it, and return a descriptive error when cfg.APIKey == ""), update the pre-run function signature if needed to return error, and let the top-level Execute()/main handle the process exit and printing so tests can assert failures without terminating the process.
🤖 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/root.go`:
- Around line 54-56: The current check only inspects the leaf command name
(noConfigCommands[cmd.Name()]) so nested commands like "completion bash" miss a
parent-level exemption; change the logic to walk up the command chain and check
each ancestor's name instead (e.g. for c := cmd; c != nil; c = c.Parent() { if
noConfigCommands[c.Name()] { return nil } }), referencing noConfigCommands and
cmd.Name()/cmd.Parent() to locate where to replace the single-name check.
---
Nitpick comments:
In `@cmd/root.go`:
- Around line 62-70: The pre-run in cmd/root.go currently calls os.Exit on
config.Load() failure and when cfg.APIKey is empty; change this to return errors
instead so the command flow is testable—replace the fmt.Fprintf(os.Stderr,
"...")/fmt.Fprintln(...) + os.Exit calls in the pre-run with returned errors
(e.g., wrap the original err from config.Load() with fmt.Errorf and return it,
and return a descriptive error when cfg.APIKey == ""), update the pre-run
function signature if needed to return error, and let the top-level
Execute()/main handle the process exit and printing so tests can assert failures
without terminating the process.
🪄 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: 739801ef-a206-4f6b-bcc3-c6322aa482dd
📒 Files selected for processing (2)
cmd/root.gocmd/root_test.go
- Walk full command ancestry for noConfigCommands check so nested subcommands like 'completion bash' are correctly exempted - Return errors from persistentPreRunE instead of calling os.Exit - Update README quickstart to single-command flow (supermodel auto- launches setup wizard on first run) - Update install URLs: /install.sh → /install Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
🧹 Nitpick comments (1)
README.md (1)
100-100: 💤 Low valueConsider clarifying what happens after the first run.
The comment "first run: launches setup wizard, then watches for changes" is accurate for the initial case (no API key + terminal). But for someone reading this who already has an API key configured, it might not be immediately clear that subsequent runs skip the wizard and go straight to watching.
Maybe something like:
supermodel # first run: launches setup wizard, then watches for changes # subsequent runs: watches for changesThis is totally optional though—"first run" is pretty self-explanatory!
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@README.md` at line 100, Clarify the README entry for the "supermodel" CLI so readers know behavior differs between an initial run and subsequent runs: update the line containing "supermodel # first run: launches setup wizard, then watches for changes" to explicitly state that the first run launches the setup wizard and subsequent runs skip the wizard and immediately watch for changes (e.g., add a second comment like "# subsequent runs: watches for changes") so users with an existing API key understand the behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@README.md`:
- Line 100: Clarify the README entry for the "supermodel" CLI so readers know
behavior differs between an initial run and subsequent runs: update the line
containing "supermodel # first run: launches setup wizard, then
watches for changes" to explicitly state that the first run launches the setup
wizard and subsequent runs skip the wizard and immediately watch for changes
(e.g., add a second comment like "# subsequent runs: watches for changes") so
users with an existing API key understand the behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 7f450a4e-24c2-4ea2-af04-cb72acc59312
📒 Files selected for processing (2)
README.mdcmd/root.go
🚧 Files skipped from review as they are similar to previous changes (1)
- cmd/root.go
* test: failing tests for install.sh wizard removal (#153, #158) * fix: remove install-time setup wizard, add getting-started message The wizard now auto-launches from bare 'supermodel' when run in a project directory (PR #152). Running it at install time starts watch in the wrong directory. Closes #153. Closes #158. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * refactor(test): use regex assertions in TestInstallScript Address CodeRabbit review: replace brittle substring checks with regex-based assertions that match command execution shape (not arbitrary text mentions) and accept valid guidance variations case-insensitively. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
* test: integration tests for first-run dispatch flow (#157) Adds integration tests covering the non-PTY dispatch matrix defined in issue #151 and implemented in PR #152: - non-TTY + no key → exits 1 with "not authenticated" error - non-TTY + key → enters watch path (error is NOT "not authenticated") - version subcommand → exits 0, prints version (noConfigCommand bypass) - completion bash → exits 0, prints completion script (noConfigCommand) - analyze without key → exits 1 with "run 'supermodel setup'" error - config with empty api_key field → treated same as no key PTY-based tests (wizard launch) require hardware and are left for manual testing. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(test): add .exe suffix on Windows for integration test binary (#157) The binary built by buildBinary must have a .exe extension on Windows to be executable. Use runtime.GOOS to choose the correct name. Also skip `completion bash` on Windows (bash is not available by default) and use os.DevNull for cross-platform /dev/null handling. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(test): add context timeout to prevent CI hangs in integration tests Address CodeRabbit review: wrap all exec.Command calls in exec.CommandContext with a 20s hard timeout so that if the watch daemon behaviour changes, CI cannot hang indefinitely. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
Refs #151. Drafting because the README/docs updates from the tracking
checklist are still pending — this PR covers items 1–3 (dispatch logic
Behavior
supermodel setup\\or set SUPERMODEL_API_KEY`The canonical first-run flow drops from:
```bash
curl -fsSL https://supermodeltools.com/install.sh | sh
cd /path/to/repo
supermodel setup
```
to:
```bash
curl -fsSL https://supermodeltools.com/install.sh | sh
cd /path/to/repo
supermodel
```
`supermodel setup` is unchanged and still works as the explicit
reconfigure entry point.
Implementation
`cmd/root.go`:
returns one of `runWatch`, `runSetup`, `errNotAuthenticated`. Trivially
unit-testable.
function and now bails out early for `cmd.Parent() == nil` (the bare
root command), letting `RunE` own the dispatch.
dep, used in `internal/auth`).
Tests
`cmd/root_test.go` covers the full 4-cell decision matrix. Full suite
is green:
```
$ go test ./...
ok github.com/supermodeltools/cli/cmd 0.233s
ok github.com/supermodeltools/cli/internal/auth 2.062s
[…all packages pass…]
```
Manual demo (matches the matrix above)
```
$ SUPERMODEL_API_KEY= ./supermodel < /dev/null
Error: not authenticated — run `supermodel setup` or set SUPERMODEL_API_KEY
$ SUPERMODEL_API_KEY=smsk_live_demo ./supermodel --dir /tmp/repo < /dev/null
[supermodel] [step:1] Building code graph
…went straight to watch path, no wizard…
$ SUPERMODEL_API_KEY= ./supermodel analyze
Run 'supermodel setup' to get started.
```
The TTY-no-key case (drop into wizard) is exercised by the unit test;
covering it in a shell test would require pseudo-terminal allocation.
Tracking checklist (mirrors #151)
The unchecked items should land in follow-up PRs once this dispatch
change is approved. Marking this draft until those land or you give me
the go-ahead to merge as-is.
Summary by CodeRabbit
New Features
Bug Fixes
Tests
Documentation