From 5bbae242fad2dd27c3b0576c72982321cd0d7d71 Mon Sep 17 00:00:00 2001 From: Jon Gallant <2163001+jongio@users.noreply.github.com> Date: Tue, 4 Aug 2026 16:39:56 -0700 Subject: [PATCH] fix(cli): make --check a consistent exit-code gate across commands status --check and clean --check gate the exit code and leave output alone. doctor --check did the opposite: it suppressed the report but returned an error with or without the flag, so it was not a gate at all and its own flag help was wrong. doctor now always prints its report and only fails when --check is passed. Without --check it is a diagnostic that exits 0, which keeps it usable mid-pipeline and is what makes --check meaningful. BREAKING: "grut doctor" alone no longer exits non-zero on a failed required check. Add --check to restore the gate behavior. Also reject unknown names in "theme show". theme.Load falls back to the default theme so a stale config value cannot stop the TUI from launching, but that made "grut theme show grubvox" print the default theme and exit 0, silently hiding the typo. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: c5a9c45a-0341-46e5-8458-f4060ddd58c7 --- README.md | 2 +- cmd/doctor.go | 17 +++++++++-------- cmd/doctor_test.go | 20 +++++++++++++------- cmd/theme.go | 17 ++++++++++++++--- cmd/theme_test.go | 27 +++++++++++++++++++++++++-- docs/configuration.md | 2 +- 6 files changed, 63 insertions(+), 22 deletions(-) diff --git a/README.md b/README.md index c0b54fa..92b5bb1 100644 --- a/README.md +++ b/README.md @@ -168,7 +168,7 @@ grut is primarily a TUI, but it also exposes subcommands for scripting and setup | `grut [path]` | Launch the TUI in the current or given directory | | `grut version` | Print the version of grut | | `grut update` | Update grut to the latest release | -| `grut doctor` | Check environment health (`--json` for reports, `--check` for quiet gates) | +| `grut doctor` | Check environment health (`--json` for reports, `--check` to fail on required-check failures) | | `grut status` | Print a summary of the working tree status | | `grut config` | Inspect configuration (`check`, `get `, `defaults`) | | `grut theme` | Inspect themes (`theme list`) | diff --git a/cmd/doctor.go b/cmd/doctor.go index 56535af..bcdd071 100644 --- a/cmd/doctor.go +++ b/cmd/doctor.go @@ -86,17 +86,18 @@ func newDoctorCmdWithDeps(deps doctorCommandDeps) *cobra.Command { SilenceUsage: true, RunE: func(cmd *cobra.Command, _ []string) error { report := runDoctor(cmd.Context(), deps) - check, _ := cmd.Flags().GetBool("check") - var err error if asJSON { - err = writeDoctorJSON(cmd.OutOrStdout(), report) - } else if !check { + if err := writeDoctorJSON(cmd.OutOrStdout(), report); err != nil { + return err + } + } else { writeDoctorText(cmd.OutOrStdout(), report) } - if err != nil { - return err - } - if !report.OK { + // --check turns the report into an exit-code gate, matching + // "status --check" and "clean --check". Without it, doctor is a + // diagnostic that always succeeds so it stays usable mid-pipeline. + check, _ := cmd.Flags().GetBool("check") + if check && !report.OK { return errDoctorRequiredChecksFailed } return nil diff --git a/cmd/doctor_test.go b/cmd/doctor_test.go index 3ce0334..52cc358 100644 --- a/cmd/doctor_test.go +++ b/cmd/doctor_test.go @@ -42,7 +42,7 @@ func TestDoctorInvalidConfigFailsRequiredCheck(t *testing.T) { out, err := runDoctorTest(t, deps, []string{"--json"}) - require.Error(t, err) + require.NoError(t, err) var report doctorReport require.NoError(t, json.Unmarshal([]byte(out), &report)) assert.False(t, report.OK) @@ -78,7 +78,7 @@ func TestDoctorMissingGitHubAuthWarnsWithoutFailing(t *testing.T) { assert.Contains(t, out, "All required checks passed.") } -func TestDoctorRequiredFailureExitsNonZero(t *testing.T) { +func TestDoctorRequiredFailureReportsFailure(t *testing.T) { deps := doctorTestDeps(&config.Config{ Theme: config.ThemeConfig{Name: "default"}, AI: config.AIConfig{Enabled: false, Provider: "none"}, @@ -92,23 +92,25 @@ func TestDoctorRequiredFailureExitsNonZero(t *testing.T) { out, err := runDoctorTest(t, deps, nil) - require.Error(t, err) + require.NoError(t, err) assert.Contains(t, out, "Git") assert.Contains(t, out, "FAIL") assert.Contains(t, out, "One or more required checks failed.") } -func TestDoctorCheckPassingRequiredChecksPrintsNothing(t *testing.T) { +// --check is an exit-code gate, matching "status --check" and "clean --check". +// The report still prints; only the exit code changes. +func TestDoctorCheckPassingRequiredChecksPrintsReport(t *testing.T) { out, err := runDoctorTest(t, doctorTestDeps(&config.Config{ Theme: config.ThemeConfig{Name: "default"}, AI: config.AIConfig{Enabled: false, Provider: "none"}, }), []string{"--check"}) require.NoError(t, err) - assert.Empty(t, out) + assert.Contains(t, out, "All required checks passed.") } -func TestDoctorCheckRequiredFailureExitsNonZeroAndPrintsNothing(t *testing.T) { +func TestDoctorCheckRequiredFailureExitsNonZeroAndStillPrints(t *testing.T) { deps := doctorTestDeps(&config.Config{ Theme: config.ThemeConfig{Name: "default"}, AI: config.AIConfig{Enabled: false, Provider: "none"}, @@ -123,9 +125,13 @@ func TestDoctorCheckRequiredFailureExitsNonZeroAndPrintsNothing(t *testing.T) { out, err := runDoctorTest(t, deps, []string{"--check"}) require.ErrorIs(t, err, errDoctorRequiredChecksFailed) - assert.Empty(t, out) + assert.Contains(t, out, "One or more required checks failed.") } +// Without --check, doctor is a diagnostic: it reports failures but exits 0 so +// it stays usable mid-pipeline. Covered by +// TestDoctorRequiredFailureReportsFailure above. + func TestDoctorCheckJSONStillPrintsJSON(t *testing.T) { out, err := runDoctorTest(t, doctorTestDeps(&config.Config{ Theme: config.ThemeConfig{Name: "default"}, diff --git a/cmd/theme.go b/cmd/theme.go index b80402e..5d0418f 100644 --- a/cmd/theme.go +++ b/cmd/theme.go @@ -3,6 +3,7 @@ package cmd import ( "encoding/json" "fmt" + "slices" "strings" "github.com/jongio/grut/internal/theme" @@ -17,7 +18,7 @@ func newThemeCmd() *cobra.Command { Short: "Inspect grut themes", } themeCmd.AddCommand(newThemeListCmd(theme.ListThemes)) - themeCmd.AddCommand(newThemeShowCmd(theme.Load)) + themeCmd.AddCommand(newThemeShowCmd(theme.Load, theme.ListThemes)) return themeCmd } @@ -70,14 +71,24 @@ type themeShowReport struct { Colors theme.Colors `json:"colors"` } -func newThemeShowCmd(load themeLoadFunc) *cobra.Command { +func newThemeShowCmd(load themeLoadFunc, list themeListFunc) *cobra.Command { var asJSON bool showCmd := &cobra.Command{ Use: "show ", Short: "Show resolved theme details", Args: cobra.ExactArgs(1), RunE: func(cmd *cobra.Command, args []string) error { - resolved, err := load(args[0]) + // theme.Load falls back to the default theme for an unknown name so + // a stale config value can't stop the TUI from launching. That is + // wrong for "show", where silently reporting the default theme + // hides the typo, so reject unknown names up front. + name := args[0] + available := list() + if !slices.Contains(available, name) { + return fmt.Errorf("unknown theme %q (available: %s)", + name, strings.Join(available, ", ")) + } + resolved, err := load(name) if err != nil { return err } diff --git a/cmd/theme_test.go b/cmd/theme_test.go index fa17a50..fbafc69 100644 --- a/cmd/theme_test.go +++ b/cmd/theme_test.go @@ -101,7 +101,7 @@ func TestThemeListCommandFilterNoMatchesJSON(t *testing.T) { func TestThemeShowCommandPrintsText(t *testing.T) { cmd := newThemeShowCmd(func(string) (*theme.Theme, error) { return &theme.Theme{Name: "default", Variant: "dark", Mode: theme.ModeColor, Colors: theme.Colors{Background: "#000000"}}, nil - }) + }, func() []string { return []string{"default", "gruvbox"} }) cmd.SetArgs([]string{"default"}) var out bytes.Buffer cmd.SetOut(&out) @@ -117,7 +117,7 @@ func TestThemeShowCommandPrintsText(t *testing.T) { func TestThemeShowCommandPrintsJSON(t *testing.T) { cmd := newThemeShowCmd(func(string) (*theme.Theme, error) { return &theme.Theme{Name: "gruvbox", Variant: "dark", Mode: theme.ModeColor, Colors: theme.Colors{Background: "#282828"}}, nil - }) + }, func() []string { return []string{"default", "gruvbox"} }) cmd.SetArgs([]string{"gruvbox", "--json"}) var out bytes.Buffer cmd.SetOut(&out) @@ -133,6 +133,29 @@ func TestThemeShowCommandPrintsJSON(t *testing.T) { assert.Equal(t, "#282828", report.Colors.Background) } +// theme.Load falls back to the default theme for an unknown name, which would +// make "theme show typo" print the default theme and exit 0. Assert the +// command rejects the name before ever calling load. +func TestThemeShowCommandUnknownThemeErrors(t *testing.T) { + loadCalled := false + cmd := newThemeShowCmd(func(string) (*theme.Theme, error) { + loadCalled = true + return &theme.Theme{Name: "default", Variant: "dark", Mode: theme.ModeColor}, nil + }, func() []string { return []string{"default", "gruvbox"} }) + cmd.SetArgs([]string{"grubvox"}) + var out bytes.Buffer + cmd.SetOut(&out) + cmd.SetErr(&out) + cmd.SilenceUsage = true + + err := cmd.Execute() + + require.Error(t, err) + assert.Contains(t, err.Error(), `unknown theme "grubvox"`) + assert.Contains(t, err.Error(), "gruvbox") + assert.False(t, loadCalled, "load should not run for an unknown theme") +} + func TestRootRegistersThemeListCommand(t *testing.T) { root, cleanup := newRootCommand() defer cleanup() diff --git a/docs/configuration.md b/docs/configuration.md index b434c9e..e619650 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -20,7 +20,7 @@ Config: C:\Users\me\AppData\Roaming\grut\config.toml Data: C:\Users\me\AppData\Local\grut ``` -Run `grut doctor` to validate the active config, selected theme, terminal compatibility, Git/GitHub auth, AI provider readiness, and config/data directory writability. Use `grut doctor --json` to attach a stable machine-readable report to bug reports or CI logs. Use `grut doctor --check` for quiet shell gates that only need an exit code. +Run `grut doctor` to validate the active config, selected theme, terminal compatibility, Git/GitHub auth, AI provider readiness, and config/data directory writability. Use `grut doctor --json` to attach a stable machine-readable report to bug reports or CI logs. Use `grut doctor --check` for shell gates: it prints the same report but exits non-zero when a required check fails, matching `grut status --check` and `grut clean --check`. ---