Conversation
Add service.BindEnvToCommand(cmd) to the root command so all CLI flags can be configured via environment variables. Also fixes go.mod issues (duplicate dependency, missing go-jose replace directive) via go mod tidy. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
WalkthroughAdded an import for Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 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 docstrings
🧪 Generate unit tests (beta)
📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
cmd/root.go (1)
106-107: Move env binding into NewRootCommand constructor to ensure consistent env resolution across all code paths.
NewRootCommand()is called from two contexts:Execute()in cmd/root.go and directly from cmd/prompt.go (lines 110, 143). Currently,service.BindEnvToCommand()is only applied inExecute(), leaving prompt-initiated commands without env binding. Both paths then fall back to manualos.Getenv()calls in pkg/flags.go, creating inconsistent precedence between bound flags and direct env reads.Moving
BindEnvToCommand()into theNewRootCommand()constructor ensures all command instances receive consistent env binding behavior.♻️ Proposed refactor
func NewRootCommand() *cobra.Command { homedir, err := os.UserHomeDir() if err != nil { panic(err) } + service.BindEnvToCommand(cmd) return cmd } func Execute() { cmd := NewRootCommand() - service.BindEnvToCommand(cmd) if err := cmd.ExecuteContext(ctx); err != nil {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cmd/root.go` around lines 106 - 107, Move the environment binding call into the root command constructor so all command instances get consistent env binding: remove the standalone service.BindEnvToCommand(cmd) call from Execute() and instead invoke service.BindEnvToCommand(...) inside NewRootCommand() (the constructor that returns *cobra.Command used by Execute() and cmd/prompt.go) so both Execute() and direct callers (like prompt.go) receive the same env-bound flags; update NewRootCommand to accept or obtain the service needed to call BindEnvToCommand and ensure no remaining manual os.Getenv() fallbacks in pkg/flags.go are relied on for precedence.
🤖 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`:
- Around line 106-107: Move the environment binding call into the root command
constructor so all command instances get consistent env binding: remove the
standalone service.BindEnvToCommand(cmd) call from Execute() and instead invoke
service.BindEnvToCommand(...) inside NewRootCommand() (the constructor that
returns *cobra.Command used by Execute() and cmd/prompt.go) so both Execute()
and direct callers (like prompt.go) receive the same env-bound flags; update
NewRootCommand to accept or obtain the service needed to call BindEnvToCommand
and ensure no remaining manual os.Getenv() fallbacks in pkg/flags.go are relied
on for precedence.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: fc49a5e6-8d7a-427d-a730-4a809a52a882
⛔ Files ignored due to path filters (2)
go.modis excluded by!**/*.modgo.sumis excluded by!**/*.sum,!**/*.sum
📒 Files selected for processing (1)
cmd/root.go
Summary
service.BindEnvToCommand(cmd)so all CLI flags can be configured via environment variablesgo.modissues (duplicatego-libs/v4dependency line, missinggopkg.in/go-jose/go-jose.v4replace directive) viago mod tidyChanges
go-libs/v3/serviceimport tocmd/root.goservice.BindEnvToCommand(cmd)call afterNewRootCommand()inExecute()go.modandgo.sumviago mod tidyTest plan
PROFILE=myprofilecorrectly binds to--profileflag🤖 Generated with Claude Code