Add --feature-flag option to supervisor options command#649
Conversation
Allows users to enable/disable development feature flags via: ha supervisor options --feature-flag supervisor_v2_api=true ha supervisor options --feature-flag supervisor_v2_api (defaults to true) ha supervisor options --feature-flag supervisor_v2_api=false Multiple flags can be set by repeating the argument. Tab completion calls /supervisor/info and suggests each flag with its opposite value (e.g. supervisor_v2_api=true when currently false). Related: home-assistant/supervisor#6719 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
📝 WalkthroughWalkthroughAdds a new Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 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 docstrings
🧪 Generate unit tests (beta)
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.
Pull request overview
Adds CLI support in ha supervisor options for managing Supervisor development feature flags (per the Supervisor’s feature toggle system), including shell completions based on /supervisor/info.
Changes:
- Adds
--feature-flag(repeatable) to set Supervisor development feature flags viasupervisor/options. - Parses
--feature-flagvalues asname[=true|false](defaulting totruewhen=...is omitted). - Implements tab-completion for
--feature-flagby fetching current flags from/supervisor/infoand suggesting toggled values.
| supervisorOptionsCmd.Flags().BoolP("debug-block", "", false, "Enable debug mode with blocking startup") | ||
| supervisorOptionsCmd.Flags().BoolP("diagnostics", "", false, "Enable diagnostics mode") | ||
| supervisorOptionsCmd.Flags().BoolP("auto-update", "", true, "Enable/disable supervisor auto update") | ||
| supervisorOptionsCmd.Flags().StringArrayP("feature-flag", "", []string{}, "Set a development feature flag (name=true|false). Use multiple times for multiple flags.") |
There was a problem hiding this comment.
The help text for --feature-flag says (name=true|false), but the implementation also accepts a bare name and defaults it to true. Update the flag description to reflect the supported syntax so users aren’t misled.
| supervisorOptionsCmd.Flags().StringArrayP("feature-flag", "", []string{}, "Set a development feature flag (name=true|false). Use multiple times for multiple flags.") | |
| supervisorOptionsCmd.Flags().StringArrayP("feature-flag", "", []string{}, "Set a development feature flag (name[=true|false]). Bare name defaults to true. Use multiple times for multiple flags.") |
There was a problem hiding this comment.
This seems kinda obvious to me, or can be explored if needs to be. No extra documentation needed.
| current, ok := val.(bool) | ||
| if !ok { | ||
| continue | ||
| } | ||
| ret = append(ret, fmt.Sprintf("%s=%v\tcurrently %v", name, !current, current)) |
There was a problem hiding this comment.
featureFlagCompletions iterates over a Go map, so completion ordering will be nondeterministic between runs/shell invocations. Consider collecting the flag names, sorting them, and then appending suggestions in a stable order to avoid a noisy/unstable completion UX.
There was a problem hiding this comment.
We don't do this in other places right now, so I don't think we need to start it with this PR.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
cmd/supervisor_options.go (2)
62-80: Minor: inconsistent whitespace handling between name and value.
nameis trimmed viastrings.TrimSpace, butparts[1]is passed directly tostrconv.ParseBool. An input like--feature-flag "foo = true"will trim the name but fail to parse" true". Consider trimming both sides for consistency, or neither.Proposed tweak
- val, err = strconv.ParseBool(parts[1]) + val, err = strconv.ParseBool(strings.TrimSpace(parts[1]))🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cmd/supervisor_options.go` around lines 62 - 80, The feature flag parsing trims the flag name but not the value, so inputs like `foo = true` fail; update the parsing in the block that uses strings.SplitN, name := strings.TrimSpace(parts[0]), and strconv.ParseBool(parts[1]) to also trim the value before parsing (e.g., call strings.TrimSpace on parts[1] before passing to strconv.ParseBool and using it to set val) so both name and value handle surrounding whitespace consistently and error messages still reference the trimmed name.
132-144: Unchecked type assertion onresp.Result().Line 137 performs
resp.Result().(*helper.Response)without the comma-ok form; a nil or unexpected result type would panic the completion. The existing pattern incmd/resolution_check.gohas the same shape, so this is consistent with the codebase, but worth noting since a panic in a completion function degrades UX silently. Optional to harden here.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cmd/supervisor_options.go` around lines 132 - 144, The featureFlagCompletions function does an unchecked cast resp.Result().(*helper.Response) which can panic; change this to the comma-ok form and nil-check the result before using it (e.g., r, ok := resp.Result().(*helper.Response); if !ok || r == nil { return nil, cobra.ShellCompDirectiveNoFileComp }), then use r.Result and r.Data for the subsequent checks; this hardens featureFlagCompletions against unexpected or nil resp.Result() without altering behavior on success.
🤖 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/supervisor_options.go`:
- Around line 62-80: The feature flag parsing trims the flag name but not the
value, so inputs like `foo = true` fail; update the parsing in the block that
uses strings.SplitN, name := strings.TrimSpace(parts[0]), and
strconv.ParseBool(parts[1]) to also trim the value before parsing (e.g., call
strings.TrimSpace on parts[1] before passing to strconv.ParseBool and using it
to set val) so both name and value handle surrounding whitespace consistently
and error messages still reference the trimmed name.
- Around line 132-144: The featureFlagCompletions function does an unchecked
cast resp.Result().(*helper.Response) which can panic; change this to the
comma-ok form and nil-check the result before using it (e.g., r, ok :=
resp.Result().(*helper.Response); if !ok || r == nil { return nil,
cobra.ShellCompDirectiveNoFileComp }), then use r.Result and r.Data for the
subsequent checks; this hardens featureFlagCompletions against unexpected or nil
resp.Result() without altering behavior on success.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ab3fcd39-d5bd-45d3-8d48-852d42c8c62a
📒 Files selected for processing (1)
cmd/supervisor_options.go
Adds CLI support for the development feature toggle system introduced in home-assistant/supervisor#6719.
Usage
Tab completion calls
/supervisor/infoand suggests each known flag with its opposite value (e.g.supervisor_v2_api=true\tcurrently false), making it immediately actionable.Summary by CodeRabbit
--feature-flagoption to supervisor options command, enabling configuration of feature flags withname=true|falsesyntax or bare names for true values. Includes shell completions for available flags.