fix: rootCmd args parsed as directory path — breaks watch cache loading#109
Conversation
rootCmd accepted MaximumNArgs(1) and used args[0] as the working directory. Since watch is the default behavior on rootCmd (not a subcommand), running `supermodel` with any trailing token would use that token as the directory. This caused the daemon to create and read from a wrong directory (e.g. ./watch/.supermodel/shards.json instead of ./.supermodel/shards.json). Fix: switch to NoArgs and add an explicit --dir flag. This matches the pattern used by all other subcommands that accept a directory. Fixes supermodeltools#108
WalkthroughRoot CLI command no longer accepts a positional path. A new persistent Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
cmd/root.go (1)
34-43:⚠️ Potential issue | 🟡 Minor
Usestill advertises a positional path that is now rejected.Line 34 says
supermodel [path], but Line 43 enforcescobra.NoArgs. So--helpcurrently tells users to do something that fails.Suggested fix
- Use: "supermodel [path]", + Use: "supermodel",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cmd/root.go` around lines 34 - 43, The command metadata is inconsistent: Use is "supermodel [path]" but Args is set to cobra.NoArgs, causing HELP to advertise a positional that will be rejected; either remove the positional from Use or allow one arg. Fix by updating the command definition in root.go: if the command should accept an optional path, replace Args: cobra.NoArgs with Args: cobra.MaximumNArgs(1) (or cobra.ArbitraryArgs if multiple allowed) and adjust any code that reads the arg (refer to the root command initialization and any handlers that use os.Args or cmd.Args), otherwise change Use to "supermodel" to reflect no positional argument. Ensure the chosen change keeps help text and argument parsing consistent.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@cmd/root.go`:
- Around line 34-43: The command metadata is inconsistent: Use is "supermodel
[path]" but Args is set to cobra.NoArgs, causing HELP to advertise a positional
that will be rejected; either remove the positional from Use or allow one arg.
Fix by updating the command definition in root.go: if the command should accept
an optional path, replace Args: cobra.NoArgs with Args: cobra.MaximumNArgs(1)
(or cobra.ArbitraryArgs if multiple allowed) and adjust any code that reads the
arg (refer to the root command initialization and any handlers that use os.Args
or cmd.Args), otherwise change Use to "supermodel" to reflect no positional
argument. Ensure the chosen change keeps help text and argument parsing
consistent.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
cmd/root.go (1)
43-43: Optional: make the positional-arg error message migration-friendly.Right now users coming from
supermodel <path>will get a generic arg error. A targeted message like “use--dir” makes this transition smoother (example:supermodel watchtypo).💡 Suggested tweak
- Args: cobra.NoArgs, + Args: func(cmd *cobra.Command, args []string) error { + if len(args) == 0 { + return nil + } + return fmt.Errorf("unexpected argument %q; use --dir %q", args[0], args[0]) + },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cmd/root.go` at line 43, Replace the hard-coded cobra.NoArgs with a custom Args validation that detects when a positional argument is provided and returns a migration-friendly error directing users to use --dir; e.g., change the Args assignment (currently cobra.NoArgs in cmd/root.go) to a function that checks len(args)>0 and returns an error like "positional paths are not accepted; use --dir <path> instead" (keep this logic on the root command's Args field so callers like `supermodel <path>` get the targeted message).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@cmd/root.go`:
- Line 43: Replace the hard-coded cobra.NoArgs with a custom Args validation
that detects when a positional argument is provided and returns a
migration-friendly error directing users to use --dir; e.g., change the Args
assignment (currently cobra.NoArgs in cmd/root.go) to a function that checks
len(args)>0 and returns an error like "positional paths are not accepted; use
--dir <path> instead" (keep this logic on the root command's Args field so
callers like `supermodel <path>` get the targeted message).
Problem
rootCmdacceptedMaximumNArgs(1)and usedargs[0]as the working directory. Since watch is the default behavior on rootCmd (not a subcommand), any trailing token would be used as the directory path. The daemon would create and read from a wrong directory (e.g../watch/.supermodel/shards.jsoninstead of./.supermodel/shards.json), loading a stale 1-node cache instead of the 2000-node cache fromanalyze.Fix
Switch to
cobra.NoArgs+ explicit--dirflag. 4 lines changed.Production E2E results
Tested against production API v0.12.0 on the supermodeltools/cli repo:
Fixes #108
Summary by CodeRabbit
supermodel --dir /path/to/project(or omit to use the current directory).