feat: interactive setup wizard with file mode onboarding#48
Merged
greynewell merged 4 commits intomainfrom Apr 7, 2026
Merged
Conversation
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
Caution Review failedPull request was closed or merged during review 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (3)
internal/setup/wizard.go (3)
171-171: This line is unnecessary.
hookInstalledis already used on lines 159-160 in the conditional, so this blank assignment is dead code. Probably leftover from an earlier iteration?🧹 Proposed fix
- _ = hookInstalled - if confirmYN("Run 'supermodel analyze' now?", true) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/setup/wizard.go` at line 171, Remove the redundant blank assignment "_ = hookInstalled" — the variable hookInstalled is already referenced in the conditional earlier (the check using hookInstalled), so delete the dead statement to clean up the code; locate the assignment of hookInstalled and the trailing "_ = hookInstalled" and remove the latter.
362-363: This blank assignment silences the unused warning forselectMenu.If you're not using
selectMenuanywhere yet, you could either remove it entirely or add a TODO comment explaining when you plan to use it. Right now it's defined but only kept alive by thisvar _ = selectMenutrick.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/setup/wizard.go` around lines 362 - 363, The blank assignment var _ = selectMenu is masking that the selectMenu symbol is unused; either remove the selectMenu declaration if not needed, or keep it but replace the var _ = selectMenu trick with an explicit TODO comment explaining intended future use (or add a real usage). Locate the selectMenu declaration in wizard.go and either delete it, or add a clear TODO comment adjacent to the selectMenu function/name and remove the blank assignment, or implement the planned usage so the symbol is genuinely referenced.
301-307: Heads up:os.Exit(0)in library code can be surprising.Calling
os.Exit(0)here means the wizard can't be easily tested in unit tests (since the process just dies) and can't be composed into larger workflows. For a CLI wizard this is probably fine, but if you ever need to test this or run it programmatically, you might want to return an error likepromptui.ErrInterruptinstead and let the caller decide what to do.Not a blocker, just something to keep in mind.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/setup/wizard.go` around lines 301 - 307, The code currently calls os.Exit(0) inside the error branch after sel.Run(), which kills the process; instead make the function return the error to the caller (e.g., change the function signature to return (int, error) and replace os.Exit(0) with return 0, promptui.ErrInterrupt or the wrapped err), so callers can decide how to handle interrupts; update any callers of this function to handle the returned error accordingly and remove the direct process termination from this module.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@internal/setup/wizard.go`:
- Around line 3-15: Reformat the import block in wizard.go so standard library
imports (context, encoding/json, fmt, os, os/exec, path/filepath, strings) are
grouped together, followed by a blank line and then external imports
(github.com/manifoldco/promptui,
github.com/supermodeltools/cli/internal/analyze,
github.com/supermodeltools/cli/internal/config); alternatively run goimports -w
on the file to automatically fix the grouping and ordering of imports.
- Around line 119-130: The if-else chain that handles the result of
installHook(repoDir) should be rewritten as a switch on the error/installed
result to satisfy gocritic; replace the current if err != nil / else if
installed / else block with a switch that first checks the error case (case err
!= nil) then a case for installed == true and a default for installed == false,
and inside each case set hookInstalled and hookNote and print the same messages
as before (use installHook, repoDir, hookInstalled and hookNote to locate the
code).
- Around line 189-197: The function findGitRoot currently always returns a
non-empty string which makes the caller's repoDir != "" check meaningless;
change findGitRoot to return a discriminant (either (string, bool) or (string,
error)) so it can signal "git not found" explicitly (e.g., return "", false or
return "", err). Update all callers (the code that currently does if repoDir !=
"") to use the new return form (check the bool or error) and only use cwd as a
fallback when the function indicates git wasn't found. Ensure the function still
returns the trimmed git top-level path when git succeeds (e.g., return root,
true) and returns an empty string with false/error when exec.Command fails.
- Around line 61-72: The code discards the error returned by filepath.Abs when
normalizing repoDir which can mask failures (e.g., deleted cwd) or handle empty
input poorly; update the logic around repoDir, filepath.Abs and the
promptText/confirmYN flow to capture and check the error from filepath.Abs,
validate that the resolved path is non-empty and exists (os.Stat) and reprompt
or return an error if resolution fails, ensuring any failure is logged/handed
back to the caller instead of silently continuing with an invalid repoDir.
---
Nitpick comments:
In `@internal/setup/wizard.go`:
- Line 171: Remove the redundant blank assignment "_ = hookInstalled" — the
variable hookInstalled is already referenced in the conditional earlier (the
check using hookInstalled), so delete the dead statement to clean up the code;
locate the assignment of hookInstalled and the trailing "_ = hookInstalled" and
remove the latter.
- Around line 362-363: The blank assignment var _ = selectMenu is masking that
the selectMenu symbol is unused; either remove the selectMenu declaration if not
needed, or keep it but replace the var _ = selectMenu trick with an explicit
TODO comment explaining intended future use (or add a real usage). Locate the
selectMenu declaration in wizard.go and either delete it, or add a clear TODO
comment adjacent to the selectMenu function/name and remove the blank
assignment, or implement the planned usage so the symbol is genuinely
referenced.
- Around line 301-307: The code currently calls os.Exit(0) inside the error
branch after sel.Run(), which kills the process; instead make the function
return the error to the caller (e.g., change the function signature to return
(int, error) and replace os.Exit(0) with return 0, promptui.ErrInterrupt or the
wrapped err), so callers can decide how to handle interrupts; update any callers
of this function to handle the returned error accordingly and remove the direct
process termination from this module.
🪄 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: 29a183d5-5bf9-4857-b48e-6c66e013f8fd
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (3)
cmd/setup.gogo.modinternal/setup/wizard.go
Comment on lines
+61
to
+72
| repoDir := findGitRoot() | ||
| if repoDir != "" { | ||
| fmt.Printf(" %sDetected:%s %s\n", dim, reset, repoDir) | ||
| fmt.Println() | ||
| if !confirmYN("Use this directory?", true) { | ||
| repoDir = promptText("Path to repository", "") | ||
| } | ||
| } else { | ||
| repoDir = promptText("Path to repository", ".") | ||
| } | ||
| repoDir, _ = filepath.Abs(repoDir) | ||
| fmt.Printf(" %s✓%s Repository\n", green, reset) |
There was a problem hiding this comment.
Don't discard the error from filepath.Abs.
Line 71 throws away the error with _. While filepath.Abs rarely fails, when it does (e.g., the working directory was deleted), you'd get a weird empty path and downstream confusion. Also, if the user enters an empty string, you'd end up with an absolute path to... somewhere unexpected.
🛡️ Proposed fix
- repoDir, _ = filepath.Abs(repoDir)
+ if repoDir == "" {
+ repoDir = "."
+ }
+ absDir, err := filepath.Abs(repoDir)
+ if err != nil {
+ return fmt.Errorf("resolve repository path: %w", err)
+ }
+ repoDir = absDir🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@internal/setup/wizard.go` around lines 61 - 72, The code discards the error
returned by filepath.Abs when normalizing repoDir which can mask failures (e.g.,
deleted cwd) or handle empty input poorly; update the logic around repoDir,
filepath.Abs and the promptText/confirmYN flow to capture and check the error
from filepath.Abs, validate that the resolved path is non-empty and exists
(os.Stat) and reprompt or return an error if resolution fails, ensuring any
failure is logged/handed back to the caller instead of silently continuing with
an invalid repoDir.
Comment on lines
+189
to
+197
| // findGitRoot detects the git root from the current working directory. | ||
| func findGitRoot() string { | ||
| cwd, _ := os.Getwd() | ||
| out, err := exec.Command("git", "-C", cwd, "rev-parse", "--show-toplevel").Output() | ||
| if err == nil { | ||
| return strings.TrimSpace(string(out)) | ||
| } | ||
| return cwd | ||
| } |
There was a problem hiding this comment.
This function never returns empty string, which affects caller logic.
findGitRoot() always returns either the git root OR the current working directory - never an empty string. But look at line 61-62: it checks if repoDir != "", which will always be true.
If you want to distinguish "found a git repo" from "didn't find one", consider returning ("", false) or an error when git isn't detected, rather than silently falling back to cwd.
💡 One possible approach
-// findGitRoot detects the git root from the current working directory.
-func findGitRoot() string {
- cwd, _ := os.Getwd()
+// findGitRoot detects the git root from the current working directory.
+// Returns empty string if not in a git repository.
+func findGitRoot() string {
out, err := exec.Command("git", "-C", cwd, "rev-parse", "--show-toplevel").Output()
- if err == nil {
- return strings.TrimSpace(string(out))
+ if err != nil {
+ return ""
}
- return cwd
+ return strings.TrimSpace(string(out))
}Then the caller's if repoDir != "" check would actually be meaningful.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| // findGitRoot detects the git root from the current working directory. | |
| func findGitRoot() string { | |
| cwd, _ := os.Getwd() | |
| out, err := exec.Command("git", "-C", cwd, "rev-parse", "--show-toplevel").Output() | |
| if err == nil { | |
| return strings.TrimSpace(string(out)) | |
| } | |
| return cwd | |
| } | |
| // findGitRoot detects the git root from the current working directory. | |
| // Returns empty string if not in a git repository. | |
| func findGitRoot() string { | |
| cwd, _ := os.Getwd() | |
| out, err := exec.Command("git", "-C", cwd, "rev-parse", "--show-toplevel").Output() | |
| if err != nil { | |
| return "" | |
| } | |
| return strings.TrimSpace(string(out)) | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@internal/setup/wizard.go` around lines 189 - 197, The function findGitRoot
currently always returns a non-empty string which makes the caller's repoDir !=
"" check meaningless; change findGitRoot to return a discriminant (either
(string, bool) or (string, error)) so it can signal "git not found" explicitly
(e.g., return "", false or return "", err). Update all callers (the code that
currently does if repoDir != "") to use the new return form (check the bool or
error) and only use cwd as a fallback when the function indicates git wasn't
found. Ensure the function still returns the trimmed git top-level path when git
succeeds (e.g., return root, true) and returns an empty string with false/error
when exec.Command fails.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
5 tasks
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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
supermodel setupcommand — a 4-step interactive onboarding wizard modeled on the graph-fusion setup wizardconfig.Load()), showing masked key or directing user tosupermodel login~/.supermodel/config.yamlexec.LookPath("claude")/~/.claudedir, installs PostToolUse hook into.claude/settings.json, or shows manual instructions if not detectedsupermodel analyzeTest plan
supermodel setupwithout being logged in — should print "Run 'supermodel login' first" and exit cleanlysupermodel setupaftersupermodel login— all 4 steps should appearfiles: falsein~/.supermodel/config.yaml.claude/settings.jsonin repo dir.claude/settings.jsonshows a warning and skips hook installgo build ./...andgo vet ./...pass cleanly🤖 Generated with Claude Code
Summary by CodeRabbit