From a269245aee94734afbbcf8a51107d876f9d04ee8 Mon Sep 17 00:00:00 2001 From: Gaurav Bhatia Date: Tue, 1 Sep 2026 16:47:51 +0400 Subject: [PATCH 1/5] feat(notify): make permission-prompt alerts on by default and discoverable The notify system existed but was silent unless the user hand-edited config.json, with no UI surface to discover or change it. - resolver: fall back to mode=both, focusMode=unfocused when the notify block is missing or empty (Fixes #579) - tui: add /notify slash command with popup picker, mirroring /theme; explicit choices persist via config.SetNotify - cli: add `zero config notify` to read/update/reset the preference (--mode, --focus, --reset, --json) - config: add SetNotify writer using the existing atomic-write helper, validating against the same vocab the resolver accepts The TUI effectiveTUINotifyMode default (empty -> both) now matches the resolver. exec_test.go seeds notify.mode=off where a test asserted silent stderr, which the old empty-default implicitly provided. --- internal/cli/command_center.go | 29 +++- internal/cli/config_notify.go | 148 +++++++++++++++++ internal/cli/config_notify_test.go | 255 +++++++++++++++++++++++++++++ internal/cli/exec_test.go | 3 +- internal/config/resolver.go | 24 +++ internal/config/resolver_test.go | 55 ++++++- internal/config/writer.go | 44 +++++ internal/config/writer_test.go | 62 +++++++ internal/tui/commands.go | 8 + internal/tui/model.go | 63 +++++-- internal/tui/model_test.go | 23 +++ internal/tui/notify_select.go | 158 ++++++++++++++++++ internal/tui/notify_select_test.go | 184 +++++++++++++++++++++ internal/tui/picker.go | 27 +++ 14 files changed, 1068 insertions(+), 15 deletions(-) create mode 100644 internal/cli/config_notify.go create mode 100644 internal/cli/config_notify_test.go create mode 100644 internal/tui/notify_select.go create mode 100644 internal/tui/notify_select_test.go diff --git a/internal/cli/command_center.go b/internal/cli/command_center.go index e6c4f5dac..3cd05f4fa 100644 --- a/internal/cli/command_center.go +++ b/internal/cli/command_center.go @@ -26,6 +26,30 @@ type modelSummary = zerocommands.ModelSnapshot type providerCatalogSummary = zerocommands.ProviderCatalogSnapshot func runConfig(args []string, stdout io.Writer, stderr io.Writer, deps appDeps) int { + // The first non-flag argument is a subcommand. With no positional argument + // (or only flag arguments), the read-only summary path runs — this keeps + // `zero config` and `zero config --json` working unchanged. + command := "summary" + rest := args + if len(args) > 0 && !strings.HasPrefix(args[0], "-") { + command = strings.ToLower(strings.TrimSpace(args[0])) + rest = args[1:] + } + switch command { + case "summary": + return runConfigSummary(rest, stdout, stderr, deps) + case "notify": + return runConfigNotify(rest, stdout, stderr, deps) + case "help": + if err := writeConfigHelp(stdout); err != nil { + return exitCrash + } + return exitSuccess + } + return writeExecUsageError(stderr, fmt.Sprintf("unknown config command %q", command)) +} + +func runConfigSummary(args []string, stdout io.Writer, stderr io.Writer, deps appDeps) int { options, help, err := parseCommandCenterArgs(args, false, false) if err != nil { return writeExecUsageError(stderr, err.Error()) @@ -433,8 +457,11 @@ func formatProviderCatalogValue(value string, fallback string) string { func writeConfigHelp(w io.Writer) error { _, err := fmt.Fprint(w, `Usage: zero config [flags] + zero config notify [flags] -Inspects resolved Go configuration without printing secrets. +Inspects resolved Go configuration without printing secrets. The notify +subcommand reads or updates the permission-prompt alert preference — +run "zero config notify --help" for details. Flags: --json Print JSON summary diff --git a/internal/cli/config_notify.go b/internal/cli/config_notify.go new file mode 100644 index 000000000..3715870f9 --- /dev/null +++ b/internal/cli/config_notify.go @@ -0,0 +1,148 @@ +package cli + +import ( + "fmt" + "io" + "strings" + + "github.com/Gitlawb/zero/internal/config" +) + +// runConfigNotify implements `zero config notify`: with no flags it prints the +// current mode/focusMode; --mode/--focus update them via the same +// config.SetNotify writer the TUI /notify command uses, so all surfaces stay +// in lockstep; --reset blanks both fields so the resolver defaults apply. +func runConfigNotify(args []string, stdout io.Writer, stderr io.Writer, deps appDeps) int { + options, help, err := parseConfigNotifyArgs(args) + if err != nil { + return writeExecUsageError(stderr, err.Error()) + } + if help { + if err := writeConfigNotifyHelp(stdout); err != nil { + return exitCrash + } + return exitSuccess + } + + resolved, exitCode := resolveCommandCenterConfig(stderr, deps) + if exitCode != exitSuccess { + return exitCode + } + + if options.mode != "" || options.focus != "" || options.reset { + configPath, err := deps.userConfigPath() + if err != nil { + return writeAppError(stderr, err.Error(), exitCrash) + } + notify := config.NotifyConfig{Mode: options.mode, FocusMode: options.focus} + if options.reset { + notify = config.NotifyConfig{} + } + if _, err := config.SetNotify(configPath, notify); err != nil { + return writeAppError(stderr, err.Error(), exitUsage) + } + // Re-resolve so the printed value reflects what the next launch will + // actually use (e.g. a reset shows the built-in defaults). + resolved, exitCode = resolveCommandCenterConfig(stderr, deps) + if exitCode != exitSuccess { + return exitCode + } + } + + if options.json { + if err := writePrettyJSON(stdout, map[string]any{ + "mode": resolved.Notify.Mode, + "focusMode": resolved.Notify.FocusMode, + }); err != nil { + return exitCrash + } + return exitSuccess + } + lines := []string{ + "Notify", + "mode: " + displayCLIValue(resolved.Notify.Mode, "(default)"), + "focusMode: " + displayCLIValue(resolved.Notify.FocusMode, "(default)"), + } + if _, err := fmt.Fprintln(stdout, strings.Join(lines, "\n")); err != nil { + return exitCrash + } + return exitSuccess +} + +type configNotifyOptions struct { + mode string + focus string + reset bool + json bool +} + +func parseConfigNotifyArgs(args []string) (configNotifyOptions, bool, error) { + options := configNotifyOptions{} + for index := 0; index < len(args); index++ { + arg := args[index] + switch { + case arg == "-h" || arg == "--help" || arg == "help": + return options, true, nil + case arg == "--json": + options.json = true + case arg == "--reset": + options.reset = true + case arg == "--mode": + value, next, err := nextFlagValue(args, index, arg) + if err != nil { + return options, false, err + } + options.mode = value + index = next + case strings.HasPrefix(arg, "--mode="): + value, err := requiredInlineFlagValue(arg, "--mode") + if err != nil { + return options, false, err + } + options.mode = value + case arg == "--focus": + value, next, err := nextFlagValue(args, index, arg) + if err != nil { + return options, false, err + } + options.focus = value + index = next + case strings.HasPrefix(arg, "--focus="): + value, err := requiredInlineFlagValue(arg, "--focus") + if err != nil { + return options, false, err + } + options.focus = value + case strings.HasPrefix(arg, "-"): + return options, false, execUsageError{fmt.Sprintf("unknown flag %q", arg)} + default: + return options, false, execUsageError{fmt.Sprintf("unexpected argument %q", arg)} + } + } + return options, false, nil +} + +func writeConfigNotifyHelp(w io.Writer) error { + _, err := fmt.Fprint(w, "Usage:\n"+ + " zero config notify [flags]\n"+ + "\n"+ + "Print or update the permission-prompt notify preference.\n"+ + "\n"+ + "When run with no flag, prints the current mode and focusMode (the resolver\n"+ + "defaults to \"both\" and \"unfocused\" when the config block is empty).\n"+ + "\n"+ + "Examples:\n"+ + " zero config notify\n"+ + " zero config notify --json\n"+ + " zero config notify --mode both --focus unfocused\n"+ + " zero config notify --mode off\n"+ + " zero config notify --reset # clear config so the resolver defaults apply\n"+ + "\n"+ + "Flags:\n"+ + " --mode Notification mechanism\n"+ + " --focus When the alert fires\n"+ + " --reset Clear both fields so the resolver defaults apply\n"+ + " --json Machine-readable output\n"+ + " -h, --help Show this help\n") + return err +} diff --git a/internal/cli/config_notify_test.go b/internal/cli/config_notify_test.go new file mode 100644 index 000000000..c0bd9aa7f --- /dev/null +++ b/internal/cli/config_notify_test.go @@ -0,0 +1,255 @@ +package cli + +import ( + "bytes" + "encoding/json" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/Gitlawb/zero/internal/config" +) + +// `zero config notify` with no flag and a fresh config: the resolver applies +// the built-in defaults (mode=both, focusMode=unfocused) and the command +// reports them. This is the "just works" case a new user lands in. We use a +// real on-disk config (not the synthetic commandCenterDeps fixture, which +// returns an empty Notify field) because the resolver-default behavior is the +// whole point of this test. +func TestRunConfigNotifyPrintsResolverDefaults(t *testing.T) { + configPath := filepath.Join(t.TempDir(), "config.json") + // A valid openai profile so the resolver does not error with + // ErrNoActiveProvider. The notify defaults are applied independently of + // the provider resolution path. + seed := `{ + "activeProvider": "openai", + "providers": [{ + "name": "openai", + "providerKind": "openai", + "baseUrl": "https://api.openai.com/v1", + "model": "gpt-4.1", + "apiKeyEnv": "OPENAI_API_KEY" + }] + }` + if err := os.WriteFile(configPath, []byte(seed), 0o600); err != nil { + t.Fatalf("seed config: %v", err) + } + deps := commandCenterDeps(t) + deps.userConfigPath = func() (string, error) { return configPath, nil } + deps.resolveConfig = func(workspaceRoot string, overrides config.Overrides) (config.ResolvedConfig, error) { + return config.Resolve(config.ResolveOptions{UserConfigPath: configPath, Env: map[string]string{"OPENAI_API_KEY": "sk-test"}}) + } + + var stdout bytes.Buffer + var stderr bytes.Buffer + exitCode := runWithDeps([]string{"config", "notify"}, &stdout, &stderr, deps) + if exitCode != exitSuccess { + t.Fatalf("exit = %d, want %d: %s", exitCode, exitSuccess, stderr.String()) + } + if !strings.Contains(stdout.String(), "mode: both") { + t.Errorf("stdout should show the default mode, got: %s", stdout.String()) + } + if !strings.Contains(stdout.String(), "focusMode: unfocused") { + t.Errorf("stdout should show the default focus, got: %s", stdout.String()) + } +} + +// `zero config notify --json` emits a machine-readable payload so scripts can +// read the resolved preference without parsing prose. Same real-resolver +// fixture as the print-defaults test, since the JSON path reads the same +// `resolved.Notify` that the print path does. +func TestRunConfigNotifyPrintsJSON(t *testing.T) { + configPath := filepath.Join(t.TempDir(), "config.json") + seed := `{ + "activeProvider": "openai", + "providers": [{ + "name": "openai", + "providerKind": "openai", + "baseUrl": "https://api.openai.com/v1", + "model": "gpt-4.1", + "apiKeyEnv": "OPENAI_API_KEY" + }] + }` + if err := os.WriteFile(configPath, []byte(seed), 0o600); err != nil { + t.Fatalf("seed config: %v", err) + } + deps := commandCenterDeps(t) + deps.userConfigPath = func() (string, error) { return configPath, nil } + deps.resolveConfig = func(workspaceRoot string, overrides config.Overrides) (config.ResolvedConfig, error) { + return config.Resolve(config.ResolveOptions{UserConfigPath: configPath, Env: map[string]string{"OPENAI_API_KEY": "sk-test"}}) + } + + var stdout bytes.Buffer + var stderr bytes.Buffer + exitCode := runWithDeps([]string{"config", "notify", "--json"}, &stdout, &stderr, deps) + if exitCode != exitSuccess { + t.Fatalf("exit = %d, want %d: %s", exitCode, exitSuccess, stderr.String()) + } + var payload map[string]any + if err := json.Unmarshal(stdout.Bytes(), &payload); err != nil { + t.Fatalf("output is not valid JSON: %v\n%s", err, stdout.String()) + } + if payload["mode"] != "both" { + t.Errorf("mode = %v, want both", payload["mode"]) + } + if payload["focusMode"] != "unfocused" { + t.Errorf("focusMode = %v, want unfocused", payload["focusMode"]) + } +} + +// `zero config notify --mode off` writes the new value to disk and prints +// confirmation. The user can read it back by running the command again. +func TestRunConfigNotifyWritesModeChange(t *testing.T) { + configPath := filepath.Join(t.TempDir(), "config.json") + seed := `{ + "activeProvider": "openai", + "providers": [{ + "name": "openai", + "providerKind": "openai", + "baseUrl": "https://api.openai.com/v1", + "model": "gpt-4.1", + "apiKeyEnv": "OPENAI_API_KEY" + }] + }` + if err := os.WriteFile(configPath, []byte(seed), 0o600); err != nil { + t.Fatalf("seed config: %v", err) + } + deps := commandCenterDeps(t) + deps.userConfigPath = func() (string, error) { return configPath, nil } + deps.resolveConfig = func(workspaceRoot string, overrides config.Overrides) (config.ResolvedConfig, error) { + return config.Resolve(config.ResolveOptions{UserConfigPath: configPath, Env: map[string]string{"OPENAI_API_KEY": "sk-test"}}) + } + + var stdout bytes.Buffer + var stderr bytes.Buffer + exitCode := runWithDeps([]string{"config", "notify", "--mode", "off"}, &stdout, &stderr, deps) + if exitCode != exitSuccess { + t.Fatalf("exit = %d, want %d: %s", exitCode, exitSuccess, stderr.String()) + } + cfg := readFileConfig(t, configPath) + if cfg.Notify.Mode != "off" { + t.Errorf("Notify.Mode = %q, want off", cfg.Notify.Mode) + } + if !strings.Contains(stdout.String(), "mode: off") { + t.Errorf("stdout should confirm the change, got: %s", stdout.String()) + } +} + +// `--mode` and `--focus` together update both fields in one call. +func TestRunConfigNotifyWritesModeAndFocus(t *testing.T) { + configPath := filepath.Join(t.TempDir(), "config.json") + seed := `{ + "activeProvider": "openai", + "providers": [{ + "name": "openai", + "providerKind": "openai", + "baseUrl": "https://api.openai.com/v1", + "model": "gpt-4.1", + "apiKeyEnv": "OPENAI_API_KEY" + }] + }` + if err := os.WriteFile(configPath, []byte(seed), 0o600); err != nil { + t.Fatalf("seed config: %v", err) + } + deps := commandCenterDeps(t) + deps.userConfigPath = func() (string, error) { return configPath, nil } + deps.resolveConfig = func(workspaceRoot string, overrides config.Overrides) (config.ResolvedConfig, error) { + return config.Resolve(config.ResolveOptions{UserConfigPath: configPath, Env: map[string]string{"OPENAI_API_KEY": "sk-test"}}) + } + + var stdout bytes.Buffer + var stderr bytes.Buffer + exitCode := runWithDeps([]string{"config", "notify", "--mode", "both", "--focus", "unfocused"}, &stdout, &stderr, deps) + if exitCode != exitSuccess { + t.Fatalf("exit = %d, want %d: %s", exitCode, exitSuccess, stderr.String()) + } + cfg := readFileConfig(t, configPath) + if cfg.Notify.Mode != "both" || cfg.Notify.FocusMode != "unfocused" { + t.Errorf("Notify = %+v, want mode=both focusMode=unfocused", cfg.Notify) + } +} + +// `--mode loud` is a usage error. The config must not be mutated. +func TestRunConfigNotifyRejectsInvalidMode(t *testing.T) { + configPath := filepath.Join(t.TempDir(), "config.json") + seed := `{ + "activeProvider": "openai", + "providers": [{ + "name": "openai", + "providerKind": "openai", + "baseUrl": "https://api.openai.com/v1", + "model": "gpt-4.1", + "apiKeyEnv": "OPENAI_API_KEY" + }], + "notify": {"mode": "off"} + }` + if err := os.WriteFile(configPath, []byte(seed), 0o600); err != nil { + t.Fatalf("seed config: %v", err) + } + deps := commandCenterDeps(t) + deps.userConfigPath = func() (string, error) { return configPath, nil } + deps.resolveConfig = func(workspaceRoot string, overrides config.Overrides) (config.ResolvedConfig, error) { + return config.Resolve(config.ResolveOptions{UserConfigPath: configPath, Env: map[string]string{"OPENAI_API_KEY": "sk-test"}}) + } + + var stdout bytes.Buffer + var stderr bytes.Buffer + exitCode := runWithDeps([]string{"config", "notify", "--mode", "loud"}, &stdout, &stderr, deps) + if exitCode == exitSuccess { + t.Fatalf("expected failure for invalid mode, got success; stdout=%s", stdout.String()) + } + cfg := readFileConfig(t, configPath) + if cfg.Notify.Mode != "off" { + t.Errorf("Notify.Mode = %q, want off (unchanged after failed write)", cfg.Notify.Mode) + } +} + +// `--reset` blanks both fields so the resolver defaults apply on the next +// resolve. Useful for "go back to the recommended setup" after a custom value. +func TestRunConfigNotifyResetClearsStoredValues(t *testing.T) { + configPath := filepath.Join(t.TempDir(), "config.json") + seed := `{ + "activeProvider": "openai", + "providers": [{ + "name": "openai", + "providerKind": "openai", + "baseUrl": "https://api.openai.com/v1", + "model": "gpt-4.1", + "apiKeyEnv": "OPENAI_API_KEY" + }], + "notify": {"mode": "off", "focusMode": "always"} + }` + if err := os.WriteFile(configPath, []byte(seed), 0o600); err != nil { + t.Fatalf("seed config: %v", err) + } + deps := commandCenterDeps(t) + deps.userConfigPath = func() (string, error) { return configPath, nil } + deps.resolveConfig = func(workspaceRoot string, overrides config.Overrides) (config.ResolvedConfig, error) { + return config.Resolve(config.ResolveOptions{UserConfigPath: configPath, Env: map[string]string{"OPENAI_API_KEY": "sk-test"}}) + } + + var stdout bytes.Buffer + var stderr bytes.Buffer + exitCode := runWithDeps([]string{"config", "notify", "--reset"}, &stdout, &stderr, deps) + if exitCode != exitSuccess { + t.Fatalf("exit = %d, want %d: %s", exitCode, exitSuccess, stderr.String()) + } + cfg := readFileConfig(t, configPath) + if cfg.Notify.Mode != "" || cfg.Notify.FocusMode != "" { + t.Errorf("Notify after reset = %+v, want empty (defaults apply)", cfg.Notify) + } +} + +// `zero config` (no subcommand) still works after the dispatch change. +func TestRunConfigSummaryStillWorks(t *testing.T) { + var stdout bytes.Buffer + var stderr bytes.Buffer + exitCode := runWithDeps([]string{"config"}, &stdout, &stderr, commandCenterDeps(t)) + if exitCode != exitSuccess { + t.Fatalf("exit = %d, want %d: %s", exitCode, exitSuccess, stderr.String()) + } + if !strings.Contains(stdout.String(), "Config") { + t.Errorf("stdout should show the config summary, got: %s", stdout.String()) + } +} diff --git a/internal/cli/exec_test.go b/internal/cli/exec_test.go index 3cc4f8fe8..64a0a0895 100644 --- a/internal/cli/exec_test.go +++ b/internal/cli/exec_test.go @@ -968,7 +968,8 @@ func TestRunExecUsesProjectConfigAndOpenAICompatibleProvider(t *testing.T) { "base_url": "` + server.URL + `", "api_key": "sk-local", "model": "local-model" - }] + }], + "notify": {"mode": "off"} }` if err := os.WriteFile(filepath.Join(configDir, "config.json"), []byte(writeConfig), 0o600); err != nil { t.Fatal(err) diff --git a/internal/config/resolver.go b/internal/config/resolver.go index 713cc7c83..846759282 100644 --- a/internal/config/resolver.go +++ b/internal/config/resolver.go @@ -61,6 +61,18 @@ const MaxTurnsCeiling = 500 // (set 0 to always advertise every schema, e.g. for a model without tool_search). const defaultDeferThreshold = 3 +// defaultNotifyMode and defaultNotifyFocus are the fallback values used when +// config.json is missing, has no notify block, or has an empty notify block. +// both = terminal bell + OSC-9 desktop notification; unfocused = fire only +// when the TUI window is not the active window so users looking at the prompt +// are not spammed. The defaults make the permission-prompt alert "just work" +// for new users; the TUI /notify command and `zero config notify` let users +// change or opt out. +const ( + defaultNotifyMode = "both" + defaultNotifyFocus = "unfocused" +) + func Resolve(options ResolveOptions) (ResolvedConfig, error) { cfg := FileConfig{ MaxTurns: defaultMaxTurns, @@ -95,6 +107,18 @@ func Resolve(options ResolveOptions) (ResolvedConfig, error) { applyOverrides(&cfg, options.Overrides) + // Notify defaults: when the user has not configured notify (no block, or + // an empty block), apply the built-in defaults so the permission-prompt + // alert works out of the box. A user who explicitly sets notify.mode=off + // or notify.focusMode=focused still wins because their value is + // non-empty after the trim in the validation step below. + if strings.TrimSpace(cfg.Notify.Mode) == "" { + cfg.Notify.Mode = defaultNotifyMode + } + if strings.TrimSpace(cfg.Notify.FocusMode) == "" { + cfg.Notify.FocusMode = defaultNotifyFocus + } + if !cfg.Tools.deferThresholdSet && cfg.Tools.DeferThreshold == 0 { cfg.Tools.DeferThreshold = defaultDeferThreshold } diff --git a/internal/config/resolver_test.go b/internal/config/resolver_test.go index 6daa691a8..10981c982 100644 --- a/internal/config/resolver_test.go +++ b/internal/config/resolver_test.go @@ -1515,8 +1515,59 @@ func TestResolveNotifyDefaultEmpty(t *testing.T) { if err != nil { t.Fatalf("Resolve: %v", err) } - if resolved.Notify.Mode != "" || resolved.Notify.FocusMode != "" { - t.Fatalf("unset notify should be empty, got %+v", resolved.Notify) + // Missing notify block falls back to the built-in defaults so the + // permission-prompt alert works for users who never ran setup. + if resolved.Notify.Mode != "both" { + t.Errorf("unset notify.mode should default to %q, got %q", "both", resolved.Notify.Mode) + } + if resolved.Notify.FocusMode != "unfocused" { + t.Errorf("unset notify.focusMode should default to %q, got %q", "unfocused", resolved.Notify.FocusMode) + } +} + +func TestResolveNotifyDefaultEmptyBlock(t *testing.T) { + // An explicit empty notify block should behave the same as a missing one: + // fall back to the built-in defaults. + path := writeConfig(t, `{"notify":{}}`) + resolved, err := Resolve(ResolveOptions{UserConfigPath: path, Env: map[string]string{}}) + if err != nil { + t.Fatalf("Resolve: %v", err) + } + if resolved.Notify.Mode != "both" { + t.Errorf("empty notify.mode should default to %q, got %q", "both", resolved.Notify.Mode) + } + if resolved.Notify.FocusMode != "unfocused" { + t.Errorf("empty notify.focusMode should default to %q, got %q", "unfocused", resolved.Notify.FocusMode) + } +} + +func TestResolveNotifyDefaultPartialEmpty(t *testing.T) { + // Only one field is set; the other should still get the default. + path := writeConfig(t, `{"notify":{"mode":"off"}}`) + resolved, err := Resolve(ResolveOptions{UserConfigPath: path, Env: map[string]string{}}) + if err != nil { + t.Fatalf("Resolve: %v", err) + } + if resolved.Notify.Mode != "off" { + t.Errorf("notify.mode should be preserved as %q, got %q", "off", resolved.Notify.Mode) + } + if resolved.Notify.FocusMode != "unfocused" { + t.Errorf("empty notify.focusMode should default to %q, got %q", "unfocused", resolved.Notify.FocusMode) + } +} + +func TestResolveNotifyDefaultNoConfigFile(t *testing.T) { + // No config file at all: defaults should still apply so the + // permission-prompt alert is on for first-run users. + resolved, err := Resolve(ResolveOptions{UserConfigPath: "", Env: map[string]string{}}) + if err != nil { + t.Fatalf("Resolve: %v", err) + } + if resolved.Notify.Mode != "both" { + t.Errorf("missing config: notify.mode should default to %q, got %q", "both", resolved.Notify.Mode) + } + if resolved.Notify.FocusMode != "unfocused" { + t.Errorf("missing config: notify.focusMode should default to %q, got %q", "unfocused", resolved.Notify.FocusMode) } } diff --git a/internal/config/writer.go b/internal/config/writer.go index f27740a01..6b35bf018 100644 --- a/internal/config/writer.go +++ b/internal/config/writer.go @@ -7,6 +7,8 @@ import ( "path/filepath" "sort" "strings" + + "github.com/Gitlawb/zero/internal/notify" ) func UpsertProvider(path string, profile ProviderProfile, setActive bool) (FileConfig, error) { @@ -176,6 +178,48 @@ func SetTheme(path string, theme string) (FileConfig, error) { return cfg, nil } +// SetNotify persists the TUI notification preference. Both fields are trimmed +// and validated against the accepted vocab (mode in {off,bell,notify,both}; +// focusMode in {unfocused,always,focused}) so a bad caller cannot write a value +// the resolver would later reject at startup. An empty Mode or FocusMode is +// stored as-is — the resolver applies the built-in defaults at read time, so a +// blank value means "use defaults" rather than "no notify" or "no focus rule". +func SetNotify(path string, value NotifyConfig) (FileConfig, error) { + path = strings.TrimSpace(path) + if path == "" { + return FileConfig{}, fmt.Errorf("config path is required") + } + value.Mode = strings.TrimSpace(value.Mode) + value.FocusMode = strings.TrimSpace(value.FocusMode) + if mode := value.Mode; mode != "" { + switch notify.Mode(mode) { + case notify.ModeOff, notify.ModeBell, notify.ModeNotify, notify.ModeBoth: + default: + return FileConfig{}, fmt.Errorf("invalid notify.mode %q: expected off, bell, notify, or both", mode) + } + } + if focus := value.FocusMode; focus != "" { + switch notify.FocusMode(focus) { + case notify.FocusUnfocused, notify.FocusAlways, notify.FocusFocused: + default: + return FileConfig{}, fmt.Errorf("invalid notify.focusMode %q: expected unfocused, always, or focused", focus) + } + } + cfg := FileConfig{} + if data, err := os.ReadFile(path); err == nil { + if err := json.Unmarshal(data, &cfg); err != nil { + return FileConfig{}, fmt.Errorf("invalid config JSON %s: %w", path, err) + } + } else if !os.IsNotExist(err) { + return FileConfig{}, fmt.Errorf("read config %s: %w", path, err) + } + cfg.Notify = value + if err := writeConfigFile(path, cfg); err != nil { + return FileConfig{}, err + } + return cfg, nil +} + func normalizeFavoriteModels(models []string) []string { seen := map[string]bool{} favorites := make([]string, 0, len(models)) diff --git a/internal/config/writer_test.go b/internal/config/writer_test.go index 93a849706..6600b4ecd 100644 --- a/internal/config/writer_test.go +++ b/internal/config/writer_test.go @@ -305,6 +305,68 @@ func TestSetThemePersistsUserPreference(t *testing.T) { } } +func TestSetNotifyPersistsValidValues(t *testing.T) { + path := filepath.Join(t.TempDir(), "zero.json") + writeConfigFixture(t, path, FileConfig{ + ActiveProvider: "openai", + Providers: []ProviderProfile{ + {Name: "openai", ProviderKind: ProviderKindOpenAI, Model: "gpt-4.1"}, + }, + }, 0o600) + + cfg, err := SetNotify(path, NotifyConfig{Mode: " both ", FocusMode: " unfocused "}) + if err != nil { + t.Fatalf("SetNotify() error = %v", err) + } + if cfg.Notify.Mode != "both" || cfg.Notify.FocusMode != "unfocused" { + t.Fatalf("Notify = %+v, want mode=both focusMode=unfocused (trimmed)", cfg.Notify) + } + persisted := readConfigFixture(t, path) + if persisted.Notify.Mode != "both" || persisted.Notify.FocusMode != "unfocused" { + t.Fatalf("persisted Notify = %+v, want mode=both focusMode=unfocused", persisted.Notify) + } + if persisted.ActiveProvider != "openai" || len(persisted.Providers) != 1 { + t.Fatalf("provider config was not preserved by SetNotify: %#v", persisted) + } +} + +func TestSetNotifyRejectsInvalidMode(t *testing.T) { + path := filepath.Join(t.TempDir(), "zero.json") + writeConfigFixture(t, path, FileConfig{ActiveProvider: "openai"}, 0o600) + if _, err := SetNotify(path, NotifyConfig{Mode: "loud", FocusMode: "unfocused"}); err == nil { + t.Fatal("expected error for invalid notify.mode") + } +} + +func TestSetNotifyRejectsInvalidFocusMode(t *testing.T) { + path := filepath.Join(t.TempDir(), "zero.json") + writeConfigFixture(t, path, FileConfig{ActiveProvider: "openai"}, 0o600) + if _, err := SetNotify(path, NotifyConfig{Mode: "off", FocusMode: "sideways"}); err == nil { + t.Fatal("expected error for invalid notify.focusMode") + } +} + +func TestSetNotifyRejectsEmptyConfigPath(t *testing.T) { + if _, err := SetNotify("", NotifyConfig{Mode: "off"}); err == nil { + t.Fatal("expected error for empty config path") + } +} + +func TestSetNotifyBlankValuesPreservedAsDefaults(t *testing.T) { + // An empty mode/focusMode stored on disk is a valid "use the resolver + // defaults" signal — SetNotify must not reject blanks, and they must round + // trip unchanged so the resolver can apply its built-in fallback. + path := filepath.Join(t.TempDir(), "zero.json") + writeConfigFixture(t, path, FileConfig{ActiveProvider: "openai"}, 0o600) + if _, err := SetNotify(path, NotifyConfig{}); err != nil { + t.Fatalf("SetNotify({}) should accept blank values, got error: %v", err) + } + persisted := readConfigFixture(t, path) + if persisted.Notify.Mode != "" || persisted.Notify.FocusMode != "" { + t.Fatalf("blank notify values should round-trip, got %+v", persisted.Notify) + } +} + func TestRecapsPreferenceRoundTrips(t *testing.T) { // Default (unset) is ON. if !(PreferencesConfig{}).RecapsEnabled() { diff --git a/internal/tui/commands.go b/internal/tui/commands.go index e98665117..b8db8467d 100644 --- a/internal/tui/commands.go +++ b/internal/tui/commands.go @@ -35,6 +35,7 @@ const ( commandEffort commandStyle commandTheme + commandNotify commandTranscript commandBash commandImage @@ -342,6 +343,13 @@ var commandDefinitions = []commandDefinition{ description: "Pick a color theme (no arg opens the picker; auto detects the terminal background).", kind: commandTheme, }, + { + name: "/notify", + usage: "/notify [list|off|bell|notify|both [unfocused|always]]", + group: commandGroupSession, + description: "Pick when Zero alerts you it needs input. No arg opens the picker.", + kind: commandNotify, + }, { name: "/exit", aliases: []string{"/quit"}, diff --git a/internal/tui/model.go b/internal/tui/model.go index a6cc1c6a3..11698b154 100644 --- a/internal/tui/model.go +++ b/internal/tui/model.go @@ -130,16 +130,21 @@ type model struct { keyBindings keyBindings themeMode themeMode // palette preference: auto (default), dark, light hasDarkBg bool // last terminal background-detection result (auto mode) - userAgent string - compactRequests int - compactInFlight bool - compactFrame int - lastCompactResult *CompactResult - lastCompactError string - unpricedRequests int - unpricedTokens int - lastUsage usage.Normalized - lastUsageSeen bool + // notifyMode and notifyFocusMode track the user's in-session notify + // preference (from options.Notify, updated by /notify). The notifier built + // at startup is independent, so changes apply on the NEXT permission prompt. + notifyMode string + notifyFocusMode string + userAgent string + compactRequests int + compactInFlight bool + compactFrame int + lastCompactResult *CompactResult + lastCompactError string + unpricedRequests int + unpricedTokens int + lastUsage usage.Normalized + lastUsageSeen bool // turnLatencySum / turnLatencyCount accumulate completed-run wall time so // /context can show a rolling average turn latency (the "is it slow?" signal). // Reset by /new. @@ -673,6 +678,20 @@ type tuiAgentRunOptions struct { specDraft bool } +// effectiveTUINotifyMode returns the notification mode the TUI should use. An +// empty/unconfigured mode falls back to the resolver default ("both": terminal +// bell + OSC-9 desktop notification) so the permission-prompt alert works for +// new users without requiring them to hand-edit config.json. The /notify +// command persists explicit choices; the resolver applies the same default at +// read time, so this function and the resolver always agree. +func effectiveTUINotifyMode(mode string) notify.Mode { + m := notify.Mode(strings.TrimSpace(mode)) + if m == "" { + return notify.ModeBoth + } + return m +} + func newModel(ctx context.Context, options Options) model { if ctx == nil { ctx = context.Background() @@ -736,7 +755,7 @@ func newModel(ctx context.Context, options Options) model { runSpinner := spinner.New(spinner.WithSpinner(spinner.MiniDot)) notifier := notify.New(os.Stderr, notify.Config{ - Mode: notify.Mode(strings.TrimSpace(options.Notify.Mode)), + Mode: effectiveTUINotifyMode(options.Notify.Mode), FocusMode: notify.FocusMode(strings.TrimSpace(options.Notify.FocusMode)), }) // Opt-in webhook fan-out (ZERO_NOTIFY_WEBHOOK_URL). Delivery failures stay @@ -788,6 +807,8 @@ func newModel(ctx context.Context, options Options) model { keyBindings: resolvedKeyBindings, themeMode: resolveThemeMode(options.Theme, os.Getenv("ZERO_THEME"), options.SavedTheme), hasDarkBg: true, + notifyMode: string(effectiveTUINotifyMode(options.Notify.Mode)), + notifyFocusMode: strings.TrimSpace(options.Notify.FocusMode), userAgent: options.UserAgent, usageTracker: usageTracker, transcript: initialTranscript(), @@ -3796,6 +3817,14 @@ func (m model) choosePicker() (tea.Model, tea.Cmd) { // text /theme dispatch (M17). return m, tea.RequestBackgroundColor } + case pickerNotify: + // The picker item's Value is " "; reusing the text handler + // keeps validation, persistence, and the user-facing message in one + // place. There is no live preview for notify, so no follow-up command + // is needed here. + text := "" + m, text = m.handleNotifyCommand(item.Value) + m.transcript = reduceTranscript(m.transcript, transcriptAction{kind: actionAppendSystem, text: text}) } return m, cmd } @@ -4120,6 +4149,18 @@ func (m model) handleSubmit() (tea.Model, tea.Cmd) { return m, tea.RequestBackgroundColor } return m, nil + case commandNotify: + // Bare `/notify` opens the popup picker so the user can pick mode + focus + // with arrow keys, matching /model and /theme. An explicit + // `/notify off|bell|notify|both [unfocused|always]` runs the text handler. + if strings.TrimSpace(command.text) == "" { + m.picker = m.newNotifyPicker() + return m, nil + } + text := "" + m, text = m.handleNotifyCommand(command.text) + m.transcript = reduceTranscript(m.transcript, transcriptAction{kind: actionAppendSystem, text: text}) + return m, nil case commandImage: m = m.handleImageCommand(command.text) return m, nil diff --git a/internal/tui/model_test.go b/internal/tui/model_test.go index 4c4efbfdb..f3f54c8c2 100644 --- a/internal/tui/model_test.go +++ b/internal/tui/model_test.go @@ -2619,6 +2619,29 @@ func TestModelNotifierFocusAndCompletion(t *testing.T) { } } +func TestEffectiveTUINotifyMode(t *testing.T) { + cases := []struct { + in string + want notify.Mode + }{ + // Empty input falls through to the resolver default ("both": bell + + // OSC-9 desktop notification) so the permission-prompt alert works + // for users who never configured notify. + {"", notify.ModeBoth}, + {" ", notify.ModeBoth}, + {"off", notify.ModeOff}, + {"bell", notify.ModeBell}, + {"notify", notify.ModeNotify}, + {"both", notify.ModeBoth}, + {" bell ", notify.ModeBell}, + } + for _, c := range cases { + if got := effectiveTUINotifyMode(c.in); got != c.want { + t.Errorf("effectiveTUINotifyMode(%q) = %q, want %q", c.in, got, c.want) + } + } +} + func TestScrimViewportLine(t *testing.T) { // Blank lines are left untouched (no scrim). if got := scrimViewportLine(" ", 10); got != " " { diff --git a/internal/tui/notify_select.go b/internal/tui/notify_select.go new file mode 100644 index 000000000..d3c403139 --- /dev/null +++ b/internal/tui/notify_select.go @@ -0,0 +1,158 @@ +package tui + +import ( + "strings" + + "github.com/Gitlawb/zero/internal/config" + "github.com/Gitlawb/zero/internal/notify" +) + +// notifyChoice is one row in the /notify picker. The mode and focusMode pair is +// the on-disk shape; the label is what the user reads. +type notifyChoice struct { + label string + subtitle string + mode string + focusMode string +} + +// notifyChoices is the ordered list shown by the /notify picker. "Unfocused + +// both" (the resolver default) is first because it is the most useful option +// for users who do not already have a strong opinion. "Silent" is last so the +// recommended path is also the visually-defaulted one. Adding a new (mode, +// focus) pair is enough to extend the picker and the /notify state list. +var notifyChoices = []notifyChoice{ + { + label: "Notify when unfocused (recommended)", + subtitle: "Sound + desktop notification when the terminal is in the background.", + mode: string(notify.ModeBoth), + focusMode: string(notify.FocusUnfocused), + }, + { + label: "Always notify", + subtitle: "Sound + desktop notification every time Zero needs your input.", + mode: string(notify.ModeBoth), + focusMode: string(notify.FocusAlways), + }, + { + label: "Bell only", + subtitle: "Terminal bell (no desktop notification) every time Zero needs your input.", + mode: string(notify.ModeBell), + focusMode: string(notify.FocusAlways), + }, + { + label: "Silent", + subtitle: "Show prompts in the TUI only — no extra sound or notification.", + mode: string(notify.ModeOff), + focusMode: string(notify.FocusUnfocused), + }, +} + +// handleNotifyCommand implements /notify [list|off|bell|notify|both [focus]]. +// Bare `/notify` opens the picker at the dispatch layer; a mode-only argument +// keeps the existing focusMode. Mirrors handleThemeCommand. +func (m model) handleNotifyCommand(args string) (model, string) { + tokens := strings.Fields(strings.TrimSpace(args)) + if len(tokens) == 0 || tokens[0] == "list" { + return m, m.notifyStateText() + } + mode := strings.ToLower(strings.TrimSpace(tokens[0])) + if !isValidNotifyMode(mode) { + return m, "Notify\nUnknown mode: " + tokens[0] + " (expected off, bell, notify, or both; run /notify with no argument to pick from the list)" + } + focus := "" + if len(tokens) > 1 { + focus = strings.ToLower(strings.TrimSpace(tokens[1])) + if !isValidNotifyFocusMode(focus) { + return m, "Notify\nUnknown focus mode: " + tokens[1] + " (expected unfocused, always, or focused)" + } + } else { + focus = m.notifyCurrentFocusMode() + } + m.notifyMode = mode + m.notifyFocusMode = focus + lines := []string{ + "Notify", + "active mode: " + mode + ", focus: " + focus, + "Changes apply on the next permission prompt in this session.", + } + if note := m.persistNotifyPreference(mode, focus); note != "" { + lines = append(lines, note) + } + return m, strings.Join(lines, "\n") +} + +// persistNotifyPreference writes the choice to user config so it survives a +// restart. Best-effort: returns a short note to surface on failure, or "" on +// success / when there is no config path (e.g. tests). +func (m model) persistNotifyPreference(mode string, focus string) string { + if strings.TrimSpace(m.userConfigPath) == "" { + return "" + } + if _, err := config.SetNotify(m.userConfigPath, config.NotifyConfig{ + Mode: mode, + FocusMode: focus, + }); err != nil { + return "note: could not save notify preference (" + err.Error() + ")" + } + return "" +} + +// notifyStateText renders the /notify state view: current mode + focus + the +// picker rows, so the user has the same information whether they ran +// `/notify list` or just opened the picker. +func (m model) notifyStateText() string { + activeMode := m.notifyCurrentMode() + activeFocus := m.notifyCurrentFocusMode() + sections := []commandSection{{ + Title: "State", + Lines: []string{ + "active mode: " + activeMode, + "active focus: " + activeFocus, + }, + }} + rows := make([]string, 0, len(notifyChoices)) + for _, c := range notifyChoices { + rows = append(rows, c.label) + } + sections = append(sections, commandSection{ + Title: "Available", + Lines: rows, + }) + return renderCommandOutput(commandOutput{ + Title: "Notify", + Status: commandStatusOK, + Sections: sections, + Hints: []string{"run /notify with no argument to open the picker, or /notify [focus] to change directly"}, + }) +} + +// notifyCurrentMode and notifyCurrentFocusMode return the in-session notify +// preference. newModel populates both from options.Notify via +// effectiveTUINotifyMode (which never returns ""), so no empty fallback is +// needed here. +func (m model) notifyCurrentMode() string { + return m.notifyMode +} + +func (m model) notifyCurrentFocusMode() string { + return m.notifyFocusMode +} + +// isValidNotifyMode reports whether s names one of the four notification modes. +func isValidNotifyMode(s string) bool { + switch s { + case string(notify.ModeOff), string(notify.ModeBell), string(notify.ModeNotify), string(notify.ModeBoth): + return true + } + return false +} + +// isValidNotifyFocusMode reports whether s names one of the three focus modes. +func isValidNotifyFocusMode(s string) bool { + switch s { + case string(notify.FocusUnfocused), string(notify.FocusAlways), string(notify.FocusFocused): + return true + } + return false +} diff --git a/internal/tui/notify_select_test.go b/internal/tui/notify_select_test.go new file mode 100644 index 000000000..9a8e84113 --- /dev/null +++ b/internal/tui/notify_select_test.go @@ -0,0 +1,184 @@ +package tui + +import ( + "context" + "encoding/json" + "os" + "path/filepath" + "strings" + "testing" + + tea "charm.land/bubbletea/v2" + + "github.com/Gitlawb/zero/internal/config" + "github.com/Gitlawb/zero/internal/notify" +) + +// A committed /notify choice is written to user config and reloaded at startup +// (via the resolver's defaults + the notifyMode/notifyFocusMode fields on the +// model), so a /notify choice survives restart, just like /theme. +func TestNotifyChoicePersistsAcrossRestart(t *testing.T) { + cfgPath := filepath.Join(t.TempDir(), "config.json") + + // First session: pick a non-default notify pair via the text handler (same + // commit path the picker uses via choosePicker). + m := newModel(context.Background(), Options{UserConfigPath: cfgPath}) + m, out := m.handleNotifyCommand("off") + if m.notifyMode != "off" { + t.Fatalf("notifyMode = %q, want off", m.notifyMode) + } + if !strings.Contains(out, "Notify") { + t.Fatalf("output should announce the change, got: %s", out) + } + data, err := os.ReadFile(cfgPath) + if err != nil { + t.Fatalf("notify commit should have written config: %v", err) + } + var cfg struct { + Notify config.NotifyConfig `json:"notify"` + } + if err := json.Unmarshal(data, &cfg); err != nil { + t.Fatalf("config is not valid JSON: %v", err) + } + if cfg.Notify.Mode != "off" { + t.Fatalf("notify.mode = %q, want off", cfg.Notify.Mode) + } + + // Second session: the persisted notify block seeds the model fields so the + // /notify state line is correct and a permission prompt uses the right + // notifier (the runtime notifier is built from options.Notify, which is + // populated by the resolver from the same file). + restarted := newModel(context.Background(), Options{UserConfigPath: cfgPath, Notify: config.NotifyConfig{Mode: "off"}}) + if restarted.notifyMode != "off" { + t.Fatalf("restarted notifyMode = %q, want off (from saved config)", restarted.notifyMode) + } +} + +// `/notify` with a mode-only arg keeps the existing focusMode. A common mistake +// would be to reset the focus rule on every mode change. +func TestNotifyCommandPreservesFocusOnModeOnly(t *testing.T) { + m := newModel(context.Background(), Options{}) + m.notifyFocusMode = string(notify.FocusAlways) + m, _ = m.handleNotifyCommand("off") + if m.notifyMode != "off" { + t.Errorf("notifyMode = %q, want off", m.notifyMode) + } + if m.notifyFocusMode != string(notify.FocusAlways) { + t.Errorf("notifyFocusMode = %q, want preserved %q", m.notifyFocusMode, notify.FocusAlways) + } +} + +// `/notify bell unfocused` updates both fields in one call. +func TestNotifyCommandSetsModeAndFocus(t *testing.T) { + m := newModel(context.Background(), Options{}) + m, _ = m.handleNotifyCommand("bell unfocused") + if m.notifyMode != "bell" { + t.Errorf("notifyMode = %q, want bell", m.notifyMode) + } + if m.notifyFocusMode != "unfocused" { + t.Errorf("notifyFocusMode = %q, want unfocused", m.notifyFocusMode) + } +} + +// `/notify loud` (invalid) returns an error message; the model's notifyMode +// is NOT mutated, so a typo cannot accidentally turn the alert off. +func TestNotifyCommandRejectsInvalidMode(t *testing.T) { + m := newModel(context.Background(), Options{}) + m.notifyMode = "both" + m, out := m.handleNotifyCommand("loud") + if m.notifyMode != "both" { + t.Errorf("invalid mode should not mutate state, got %q", m.notifyMode) + } + if !strings.Contains(out, "Unknown mode") { + t.Errorf("output should explain the error, got: %s", out) + } +} + +// `/notify bell sideways` rejects the focus mode but the call also failed +// validation before persisting, so neither field should change. +func TestNotifyCommandRejectsInvalidFocus(t *testing.T) { + m := newModel(context.Background(), Options{}) + m.notifyMode = "bell" + m.notifyFocusMode = "always" + m, out := m.handleNotifyCommand("bell sideways") + if m.notifyMode != "bell" || m.notifyFocusMode != "always" { + t.Errorf("invalid focus should not mutate state, got mode=%q focus=%q", m.notifyMode, m.notifyFocusMode) + } + if !strings.Contains(out, "Unknown focus mode") { + t.Errorf("output should explain the error, got: %s", out) + } +} + +// `/notify` with no argument opens the picker, just like /theme and /model. +func TestNotifyPickerOpensOnBareNotify(t *testing.T) { + m := newModel(context.Background(), Options{Notify: config.NotifyConfig{Mode: "off", FocusMode: "unfocused"}}) + m.input.SetValue("/notify") + + updated, cmd := m.Update(testKey(tea.KeyEnter)) + m = updated.(model) + if cmd != nil { + t.Fatalf("opening the notify picker should not emit a cmd, got %T", cmd) + } + if m.picker == nil || m.picker.kind != pickerNotify { + t.Fatalf("expected the notify picker to open, got %#v", m.picker) + } + if len(m.picker.items) != len(notifyChoices) { + t.Fatalf("picker has %d items, want %d", len(m.picker.items), len(notifyChoices)) + } + // The preselected row should match the active (mode, focus) pair. + sel := m.picker.items[m.picker.selected] + if sel.Value != "off unfocused" { + t.Errorf("preselected value = %q, want the active pair %q", sel.Value, "off unfocused") + } +} + +// The picker's Value strings are the same " " form the text +// handler accepts, so the commit path can be shared. This is the contract that +// lets choosePicker dispatch to handleNotifyCommand without translation. +func TestNotifyPickerValuesAreValidCommandArgs(t *testing.T) { + m := newModel(context.Background(), Options{}) + picker := m.newNotifyPicker() + for _, item := range picker.items { + tokens := strings.Fields(item.Value) + if len(tokens) != 2 { + t.Errorf("item %q has %d tokens, want 2 (mode focus)", item.Value, len(tokens)) + continue + } + if !isValidNotifyMode(tokens[0]) { + t.Errorf("item %q: mode %q is not a valid notify mode", item.Value, tokens[0]) + } + if !isValidNotifyFocusMode(tokens[1]) { + t.Errorf("item %q: focus %q is not a valid focus mode", item.Value, tokens[1]) + } + } +} + +// The /notify state view shows the current mode and focus so users can see +// the value before opening the picker. +func TestNotifyStateTextShowsActivePair(t *testing.T) { + m := newModel(context.Background(), Options{}) + m.notifyMode = "both" + m.notifyFocusMode = "unfocused" + state := m.notifyStateText() + if !strings.Contains(state, "active mode: both") { + t.Errorf("state should show active mode, got: %s", state) + } + if !strings.Contains(state, "active focus: unfocused") { + t.Errorf("state should show active focus, got: %s", state) + } +} + +// notifyCurrentMode / notifyCurrentFocusMode surface the in-session fields +// that newModel populates from options.Notify, so /notify reads the same +// value the runtime notifier uses. +func TestNotifyCurrentReflectsModelFields(t *testing.T) { + m := newModel(context.Background(), Options{}) + m.notifyMode = "bell" + m.notifyFocusMode = "always" + if got := m.notifyCurrentMode(); got != "bell" { + t.Errorf("notifyCurrentMode = %q, want bell", got) + } + if got := m.notifyCurrentFocusMode(); got != "always" { + t.Errorf("notifyCurrentFocusMode = %q, want always", got) + } +} diff --git a/internal/tui/picker.go b/internal/tui/picker.go index 39384b2a2..d1381874a 100644 --- a/internal/tui/picker.go +++ b/internal/tui/picker.go @@ -25,6 +25,7 @@ const ( pickerSession pickerTheme pickerSkill + pickerNotify ) // pickerItem is one selectable row: Label is shown, Value is passed to the @@ -907,6 +908,32 @@ func (m model) newThemePicker() *commandPicker { return &commandPicker{kind: pickerTheme, title: "select theme", items: items, allItems: append([]pickerItem{}, items...), selected: selected} } +// newNotifyPicker lists the four (mode, focus) pairs from notifyChoices. Each +// row's Value is the same synthetic string the text /notify handler accepts +// (" "), so /notify with no arg and the picker share one commit +// path through handleNotifyCommand. The currently active pair is preselected so +// the user can press Enter to keep it. There is no live preview — notify +// affects the next permission prompt, not the current view — so the picker +// does not call a preview function on move. +func (m model) newNotifyPicker() *commandPicker { + items := make([]pickerItem, 0, len(notifyChoices)) + selected := 0 + activeMode := m.notifyCurrentMode() + activeFocus := m.notifyCurrentFocusMode() + for _, c := range notifyChoices { + items = append(items, pickerItem{ + Group: "When Zero needs your input", + Label: c.label, + Value: c.mode + " " + c.focusMode, + Meta: c.subtitle, + }) + if c.mode == activeMode && c.focusMode == activeFocus { + selected = len(items) - 1 + } + } + return &commandPicker{kind: pickerNotify, title: "select notify mode", items: items, allItems: append([]pickerItem{}, items...), selected: selected} +} + // pickerMoved advances the open picker's cursor by delta and live-previews the new // selection where the picker supports it — stepping through the /theme popup // repaints the UI in the hovered palette. Safe to call with no picker open. Callers From 334c688e2d5d1e31809946c8f45121a1a8cec5f5 Mon Sep 17 00:00:00 2001 From: Gaurav Bhatia Date: Tue, 1 Sep 2026 17:46:21 +0400 Subject: [PATCH 2/5] fix(notify): apply review feedback from CodeRabbit - cli: omitted --mode/--focus flags now preserve the current resolved value instead of wiping it (--reset remains the only clearing path); aligns the CLI with the TUI's mode-only preservation behavior - tui: reject /notify inputs with more than two tokens instead of silently accepting them - tui: apply /notify choices to the live notifier via the new notify.Notifier.Configure, so the change takes effect on the next permission prompt in the same session (the previous message claimed this but only the persisted value was updated) - notify: add Notifier.Configure (mutex-guarded policy swap that preserves sinks, focus state, and the writer) - tests: mode-only/focus-only CLI preservation, live-notifier apply, trailing-argument rejection, Configure immediate-effect + sink retention --- internal/cli/config_notify.go | 15 +++++++++- internal/cli/config_notify_test.go | 45 +++++++++++++++++++++++++++++- internal/notify/notify.go | 12 +++++++- internal/notify/notify_test.go | 38 +++++++++++++++++++++++++ internal/tui/notify_select.go | 12 +++++++- internal/tui/notify_select_test.go | 38 +++++++++++++++++++++++++ 6 files changed, 156 insertions(+), 4 deletions(-) diff --git a/internal/cli/config_notify.go b/internal/cli/config_notify.go index 3715870f9..ffa15de7f 100644 --- a/internal/cli/config_notify.go +++ b/internal/cli/config_notify.go @@ -34,9 +34,22 @@ func runConfigNotify(args []string, stdout io.Writer, stderr io.Writer, deps app if err != nil { return writeAppError(stderr, err.Error(), exitCrash) } - notify := config.NotifyConfig{Mode: options.mode, FocusMode: options.focus} + // Omitted flags preserve the current value — a full replace would let + // `--mode bell` silently wipe a configured focusMode. --reset is the + // only path that clears both fields. + notify := config.NotifyConfig{ + Mode: resolved.Notify.Mode, + FocusMode: resolved.Notify.FocusMode, + } if options.reset { notify = config.NotifyConfig{} + } else { + if options.mode != "" { + notify.Mode = options.mode + } + if options.focus != "" { + notify.FocusMode = options.focus + } } if _, err := config.SetNotify(configPath, notify); err != nil { return writeAppError(stderr, err.Error(), exitUsage) diff --git a/internal/cli/config_notify_test.go b/internal/cli/config_notify_test.go index c0bd9aa7f..e36ccd05c 100644 --- a/internal/cli/config_notify_test.go +++ b/internal/cli/config_notify_test.go @@ -110,7 +110,8 @@ func TestRunConfigNotifyWritesModeChange(t *testing.T) { "baseUrl": "https://api.openai.com/v1", "model": "gpt-4.1", "apiKeyEnv": "OPENAI_API_KEY" - }] + }], + "notify": {"mode": "both", "focusMode": "always"} }` if err := os.WriteFile(configPath, []byte(seed), 0o600); err != nil { t.Fatalf("seed config: %v", err) @@ -131,11 +132,53 @@ func TestRunConfigNotifyWritesModeChange(t *testing.T) { if cfg.Notify.Mode != "off" { t.Errorf("Notify.Mode = %q, want off", cfg.Notify.Mode) } + // A mode-only update must preserve the configured focusMode, not wipe it. + if cfg.Notify.FocusMode != "always" { + t.Errorf("Notify.FocusMode = %q, want preserved %q", cfg.Notify.FocusMode, "always") + } if !strings.Contains(stdout.String(), "mode: off") { t.Errorf("stdout should confirm the change, got: %s", stdout.String()) } } +// A focus-only update preserves the configured mode. +func TestRunConfigNotifyFocusOnlyPreservesMode(t *testing.T) { + configPath := filepath.Join(t.TempDir(), "config.json") + seed := `{ + "activeProvider": "openai", + "providers": [{ + "name": "openai", + "providerKind": "openai", + "baseUrl": "https://api.openai.com/v1", + "model": "gpt-4.1", + "apiKeyEnv": "OPENAI_API_KEY" + }], + "notify": {"mode": "bell", "focusMode": "unfocused"} + }` + if err := os.WriteFile(configPath, []byte(seed), 0o600); err != nil { + t.Fatalf("seed config: %v", err) + } + deps := commandCenterDeps(t) + deps.userConfigPath = func() (string, error) { return configPath, nil } + deps.resolveConfig = func(workspaceRoot string, overrides config.Overrides) (config.ResolvedConfig, error) { + return config.Resolve(config.ResolveOptions{UserConfigPath: configPath, Env: map[string]string{"OPENAI_API_KEY": "sk-test"}}) + } + + var stdout bytes.Buffer + var stderr bytes.Buffer + exitCode := runWithDeps([]string{"config", "notify", "--focus", "always"}, &stdout, &stderr, deps) + if exitCode != exitSuccess { + t.Fatalf("exit = %d, want %d: %s", exitCode, exitSuccess, stderr.String()) + } + cfg := readFileConfig(t, configPath) + if cfg.Notify.Mode != "bell" { + t.Errorf("Notify.Mode = %q, want preserved %q", cfg.Notify.Mode, "bell") + } + if cfg.Notify.FocusMode != "always" { + t.Errorf("Notify.FocusMode = %q, want always", cfg.Notify.FocusMode) + } +} + // `--mode` and `--focus` together update both fields in one call. func TestRunConfigNotifyWritesModeAndFocus(t *testing.T) { configPath := filepath.Join(t.TempDir(), "config.json") diff --git a/internal/notify/notify.go b/internal/notify/notify.go index 79906d5ac..1376ac426 100644 --- a/internal/notify/notify.go +++ b/internal/notify/notify.go @@ -57,7 +57,7 @@ type Sink interface { // attached Sinks. Safe for concurrent use. type Notifier struct { w io.Writer - cfg Config // immutable after New; reads outside the lock are safe + cfg Config // swap at runtime via Configure; reads outside the lock are safe mu sync.Mutex focused bool @@ -71,6 +71,16 @@ func New(w io.Writer, cfg Config) *Notifier { return &Notifier{w: w, cfg: cfg} } +// Configure swaps the mode/focus policy at runtime. Sinks, focus state, and the +// writer are preserved, so an in-session preference change (e.g. the TUI's +// /notify command) applies from the next Notify call. Safe to call concurrently +// with Notify. +func (n *Notifier) Configure(cfg Config) { + n.mu.Lock() + n.cfg = cfg + n.mu.Unlock() +} + // AddSink registers an additional destination that receives every eligible // event (subject to the same mode/focus policy as the terminal). Sinks fire // even when the Notifier has no terminal writer, so a headless CI run can still diff --git a/internal/notify/notify_test.go b/internal/notify/notify_test.go index 54ba06aef..a565345b8 100644 --- a/internal/notify/notify_test.go +++ b/internal/notify/notify_test.go @@ -50,6 +50,44 @@ func TestSequence(t *testing.T) { } } +// Configure swaps the policy at runtime: after turning a silent notifier on, +// the next Notify emits; after turning a noisy one off, it stays silent. Sinks +// survive the swap (the TUI relies on this when /notify reconfigures mid-run). +func TestConfigureAppliesImmediatelyAndKeepsSinks(t *testing.T) { + var buf bytes.Buffer + n := New(&buf, Config{Mode: ModeOff}) + n.SetFocused(true) + + n.Notify(Completion, "x") + if buf.Len() != 0 { + t.Fatalf("off should be silent, got %q", buf.String()) + } + + sink := &recordingSink{} + n.AddSink(sink) + n.Configure(Config{Mode: ModeBell, FocusMode: FocusAlways}) + n.Notify(Completion, "x") + if buf.String() != "\x07" { + t.Fatalf("after Configure(bell) should bell, got %q", buf.String()) + } + sink.mu.Lock() + got := len(sink.events) + sink.mu.Unlock() + if got != 1 { + t.Fatalf("sink should still receive events after Configure, got %d", got) + } + + n.Configure(Config{Mode: ModeOff}) + buf.Reset() + n.Notify(Completion, "x") + sink.mu.Lock() + got = len(sink.events) + sink.mu.Unlock() + if buf.Len() != 0 || got != 1 { + t.Fatalf("after Configure(off) should be fully silent, buf=%q events=%d", buf.String(), got) + } +} + func TestSanitizeMessage(t *testing.T) { if got := sanitizeMessage("ok\x1b]0;evil\x07more\nx"); got != "ok]0;evilmorex" { t.Fatalf("sanitize=%q", got) diff --git a/internal/tui/notify_select.go b/internal/tui/notify_select.go index d3c403139..6aa7e2d84 100644 --- a/internal/tui/notify_select.go +++ b/internal/tui/notify_select.go @@ -56,6 +56,9 @@ func (m model) handleNotifyCommand(args string) (model, string) { if len(tokens) == 0 || tokens[0] == "list" { return m, m.notifyStateText() } + if len(tokens) > 2 { + return m, "Notify\nToo many arguments: " + args + " (usage: /notify [unfocused|always|focused])" + } mode := strings.ToLower(strings.TrimSpace(tokens[0])) if !isValidNotifyMode(mode) { return m, "Notify\nUnknown mode: " + tokens[0] + " (expected off, bell, notify, or both; run /notify with no argument to pick from the list)" @@ -71,10 +74,17 @@ func (m model) handleNotifyCommand(args string) (model, string) { } m.notifyMode = mode m.notifyFocusMode = focus + // Apply to the live notifier so the change takes effect on the next + // permission prompt in this session, not just after a restart. + if m.notifier != nil { + m.notifier.Configure(notify.Config{ + Mode: notify.Mode(mode), + FocusMode: notify.FocusMode(focus), + }) + } lines := []string{ "Notify", "active mode: " + mode + ", focus: " + focus, - "Changes apply on the next permission prompt in this session.", } if note := m.persistNotifyPreference(mode, focus); note != "" { lines = append(lines, note) diff --git a/internal/tui/notify_select_test.go b/internal/tui/notify_select_test.go index 9a8e84113..bbcedc564 100644 --- a/internal/tui/notify_select_test.go +++ b/internal/tui/notify_select_test.go @@ -1,6 +1,7 @@ package tui import ( + "bytes" "context" "encoding/json" "os" @@ -80,6 +81,43 @@ func TestNotifyCommandSetsModeAndFocus(t *testing.T) { } } +// The choice reaches the LIVE notifier immediately, so the change applies on +// the next permission prompt in this session (not only after a restart). +func TestNotifyCommandAppliesToLiveNotifier(t *testing.T) { + var buf bytes.Buffer + // Construct through newModel so both fields are populated the way the real + // session does; then swap in a buffer-backed notifier to observe output. + m := newModel(context.Background(), Options{Notify: config.NotifyConfig{Mode: "off", FocusMode: "always"}}) + m.notifier = notify.New(&buf, notify.Config{Mode: notify.ModeOff, FocusMode: notify.FocusAlways}) + m.notifier.SetFocused(true) + + m, _ = m.handleNotifyCommand("bell") + m.notifier.Notify(notify.Completion, "x") + if buf.String() != "\x07" { + t.Fatalf("live notifier should bell after /notify bell, got %q", buf.String()) + } + + m, _ = m.handleNotifyCommand("off") + m.notifier.Notify(notify.Completion, "x") + if buf.String() != "\x07" { + t.Fatalf("live notifier should go silent after /notify off, got %q", buf.String()) + } +} + +// `/notify off always typo` (more than two tokens) is rejected, not silently +// accepted as a successful change. +func TestNotifyCommandRejectsTrailingArguments(t *testing.T) { + m := newModel(context.Background(), Options{}) + m.notifyMode = "both" + m, out := m.handleNotifyCommand("off always typo") + if m.notifyMode != "both" { + t.Errorf("trailing args should not mutate state, got %q", m.notifyMode) + } + if !strings.Contains(out, "Too many arguments") { + t.Errorf("output should explain the rejection, got: %s", out) + } +} + // `/notify loud` (invalid) returns an error message; the model's notifyMode // is NOT mutated, so a typo cannot accidentally turn the alert off. func TestNotifyCommandRejectsInvalidMode(t *testing.T) { From 15929bf4581d3bb611f1bf02bee9958abdcc6541 Mon Sep 17 00:00:00 2001 From: Gaurav Bhatia Date: Tue, 1 Sep 2026 17:56:14 +0400 Subject: [PATCH 3/5] fix(notify): read cfg under the lock in Notify MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Configure (334c688) made cfg mutable at runtime, but Notify still read n.cfg.Mode before acquiring n.mu — a data race with a concurrent Configure. Move the mode check inside the critical section and copy cfg to a local for all reads. Regression test TestConfigureConcurrentWithNotify runs Configure concurrently with Notify; verified it reports DATA RACE on the unfixed code and passes after the fix (go test -race -count=5). --- internal/notify/notify.go | 13 ++++++++----- internal/notify/notify_test.go | 15 +++++++++++++++ 2 files changed, 23 insertions(+), 5 deletions(-) diff --git a/internal/notify/notify.go b/internal/notify/notify.go index 1376ac426..3c9d39ed6 100644 --- a/internal/notify/notify.go +++ b/internal/notify/notify.go @@ -112,18 +112,21 @@ func (n *Notifier) SetFocused(focused bool) { // Sinks are invoked outside the lock so a slow/blocking sink cannot stall a // concurrent Notify or SetFocused. func (n *Notifier) Notify(event Event, message string) { - if n.cfg.Mode == ModeOff || n.cfg.Mode == "" { + n.mu.Lock() + // cfg is mutable at runtime (Configure), so every read happens under the + // lock; shouldEmit/sequence work on the local copy. + cfg := n.cfg + if cfg.Mode == ModeOff || cfg.Mode == "" { + n.mu.Unlock() return } - - n.mu.Lock() - eligible := shouldEmit(n.cfg, event, n.focused) + eligible := shouldEmit(cfg, event, n.focused) var sinks []Sink if eligible && len(n.sinks) > 0 { sinks = append(sinks, n.sinks...) } if eligible && n.w != nil { - if seq := sequence(n.cfg.Mode, message); seq != "" { + if seq := sequence(cfg.Mode, message); seq != "" { _, _ = io.WriteString(n.w, seq) } } diff --git a/internal/notify/notify_test.go b/internal/notify/notify_test.go index a565345b8..2a81720c2 100644 --- a/internal/notify/notify_test.go +++ b/internal/notify/notify_test.go @@ -147,6 +147,21 @@ func TestNotifyRaceSafe(t *testing.T) { wg.Wait() } +// Configure mutates cfg under the lock while Notify reads it; this pair must +// be race-clean (run under -race). Regression for the unsynchronized cfg read +// Notify used to perform before acquiring the lock. +func TestConfigureConcurrentWithNotify(t *testing.T) { + n := New(&bytes.Buffer{}, Config{Mode: ModeBell, FocusMode: FocusAlways}) + var wg sync.WaitGroup + for i := 0; i < 50; i++ { + wg.Add(3) + go func() { defer wg.Done(); n.Configure(Config{Mode: ModeBoth, FocusMode: FocusAlways}) }() + go func() { defer wg.Done(); n.Configure(Config{Mode: ModeOff}) }() + go func() { defer wg.Done(); n.Notify(AwaitingInput, "x") }() + } + wg.Wait() +} + func TestDefaultMessage(t *testing.T) { if DefaultMessage(Completion) != "Zero: ready" { t.Fatal("completion message") From 18e851af66d6b70d03b2367bbec45424ba6a6f79 Mon Sep 17 00:00:00 2001 From: Gaurav Bhatia Date: Thu, 3 Sep 2026 13:53:06 +0400 Subject: [PATCH 4/5] fix(notify): keep defaults out of the resolver; preserve the user's own values MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses the maintainer review (Vasanthdev2004) on PR #1001. All three findings share one root cause: the resolved config was treated as if it were the user's choice. - resolver: no longer defaults notify.mode/focusMode. The TUI's effectiveTUINotifyMode already maps empty -> both on its own, so the permission-prompt alert still works out of the box, while headless `zero exec` stays byte-identical to base (no BEL/OSC-9 on stderr under -o json), the exec empty-stderr fixture is restored, and the ZERO_NOTIFY_WEBHOOK_URL sink is not armed by an implicit default. - cli: `zero config notify` seeds omitted fields from the user's own file (new config.UserNotify), never from the resolved view — a project config's mode:off can no longer be copied into the user's global config, and blank stays blank instead of pinning today's default as an explicit choice. --reset remains the only clearing path. - tui: /notify mode-only changes preserve the focus stored in the user's own file (blank stays blank); the /notify picker enumerates the full 4x3 mode x focus space so every valid pair is a row, the current pair is always preselected, and Enter can never commit a setting the user did not choose. State view reads the stored pair. - tests: the three regressions from the review — exec writes nothing to stderr on a clean run (fixture restored + resolver empty-default test), a ProjectConfigPath test proving project notify cannot leak into the user file, and Enter on an open picker from a pair outside the old curated list (off, always) keeps the setting unchanged. --- internal/cli/config_notify.go | 24 ++-- internal/cli/config_notify_test.go | 153 ++++++++++++++++++++--- internal/cli/exec_test.go | 9 +- internal/config/resolver.go | 24 ---- internal/config/resolver_test.go | 86 +++++-------- internal/config/writer.go | 33 ++++- internal/config/writer_test.go | 37 +++++- internal/tui/notify_select.go | 141 ++++++++++++--------- internal/tui/notify_select_test.go | 193 ++++++++++++++++++++--------- internal/tui/picker.go | 28 +++-- 10 files changed, 484 insertions(+), 244 deletions(-) diff --git a/internal/cli/config_notify.go b/internal/cli/config_notify.go index ffa15de7f..9c8216971 100644 --- a/internal/cli/config_notify.go +++ b/internal/cli/config_notify.go @@ -34,13 +34,18 @@ func runConfigNotify(args []string, stdout io.Writer, stderr io.Writer, deps app if err != nil { return writeAppError(stderr, err.Error(), exitCrash) } - // Omitted flags preserve the current value — a full replace would let - // `--mode bell` silently wipe a configured focusMode. --reset is the - // only path that clears both fields. - notify := config.NotifyConfig{ - Mode: resolved.Notify.Mode, - FocusMode: resolved.Notify.FocusMode, + // Seed omitted fields from the USER'S OWN file, never from the + // resolved view: resolved merges project config (so a repo's + // mode:off would be copied into the user's global settings) and + // carries no defaults here, but seeding from it would also pin + // defaults as explicit choices. Blank stays blank — blank means + // "use the built-in defaults". --reset is the only path that + // clears both fields. + current, err := config.UserNotify(configPath) + if err != nil { + return writeAppError(stderr, err.Error(), exitUsage) } + notify := current if options.reset { notify = config.NotifyConfig{} } else { @@ -141,8 +146,11 @@ func writeConfigNotifyHelp(w io.Writer) error { "\n"+ "Print or update the permission-prompt notify preference.\n"+ "\n"+ - "When run with no flag, prints the current mode and focusMode (the resolver\n"+ - "defaults to \"both\" and \"unfocused\" when the config block is empty).\n"+ + "When run with no flag, prints the current mode and focusMode; a field you\n"+ + "never set shows as (default) — the TUI alerts with bell + notification,\n"+ + "firing only when the terminal is unfocused. Omitted flags preserve the\n"+ + "values stored in YOUR config file; --reset clears both so the defaults\n"+ + "apply again.\n"+ "\n"+ "Examples:\n"+ " zero config notify\n"+ diff --git a/internal/cli/config_notify_test.go b/internal/cli/config_notify_test.go index e36ccd05c..eec1ec6d8 100644 --- a/internal/cli/config_notify_test.go +++ b/internal/cli/config_notify_test.go @@ -11,13 +11,11 @@ import ( "github.com/Gitlawb/zero/internal/config" ) -// `zero config notify` with no flag and a fresh config: the resolver applies -// the built-in defaults (mode=both, focusMode=unfocused) and the command -// reports them. This is the "just works" case a new user lands in. We use a -// real on-disk config (not the synthetic commandCenterDeps fixture, which -// returns an empty Notify field) because the resolver-default behavior is the -// whole point of this test. -func TestRunConfigNotifyPrintsResolverDefaults(t *testing.T) { +// `zero config notify` with no flag and a fresh config: nothing is configured, +// the resolver leaves the fields blank (defaults deliberately live in the TUI, +// not the resolver — maintainer review, PR #1001), and the command reports +// "(default)" for both. +func TestRunConfigNotifyPrintsUnconfiguredAsDefault(t *testing.T) { configPath := filepath.Join(t.TempDir(), "config.json") // A valid openai profile so the resolver does not error with // ErrNoActiveProvider. The notify defaults are applied independently of @@ -47,19 +45,60 @@ func TestRunConfigNotifyPrintsResolverDefaults(t *testing.T) { if exitCode != exitSuccess { t.Fatalf("exit = %d, want %d: %s", exitCode, exitSuccess, stderr.String()) } - if !strings.Contains(stdout.String(), "mode: both") { - t.Errorf("stdout should show the default mode, got: %s", stdout.String()) + if !strings.Contains(stdout.String(), "mode: (default)") { + t.Errorf("stdout should show unconfigured mode as (default), got: %s", stdout.String()) } - if !strings.Contains(stdout.String(), "focusMode: unfocused") { - t.Errorf("stdout should show the default focus, got: %s", stdout.String()) + if !strings.Contains(stdout.String(), "focusMode: (default)") { + t.Errorf("stdout should show unconfigured focus as (default), got: %s", stdout.String()) } } -// `zero config notify --json` emits a machine-readable payload so scripts can -// read the resolved preference without parsing prose. Same real-resolver -// fixture as the print-defaults test, since the JSON path reads the same -// `resolved.Notify` that the print path does. +// `zero config notify --json` emits a machine-readable payload. A configured +// pair round-trips; unconfigured fields are empty strings (never defaults +// filled in), so scripts can distinguish "user chose" from "default". func TestRunConfigNotifyPrintsJSON(t *testing.T) { + configPath := filepath.Join(t.TempDir(), "config.json") + seed := `{ + "activeProvider": "openai", + "providers": [{ + "name": "openai", + "providerKind": "openai", + "baseUrl": "https://api.openai.com/v1", + "model": "gpt-4.1", + "apiKeyEnv": "OPENAI_API_KEY" + }], + "notify": {"mode": "bell", "focusMode": "always"} + }` + if err := os.WriteFile(configPath, []byte(seed), 0o600); err != nil { + t.Fatalf("seed config: %v", err) + } + deps := commandCenterDeps(t) + deps.userConfigPath = func() (string, error) { return configPath, nil } + deps.resolveConfig = func(workspaceRoot string, overrides config.Overrides) (config.ResolvedConfig, error) { + return config.Resolve(config.ResolveOptions{UserConfigPath: configPath, Env: map[string]string{"OPENAI_API_KEY": "sk-test"}}) + } + + var stdout bytes.Buffer + var stderr bytes.Buffer + exitCode := runWithDeps([]string{"config", "notify", "--json"}, &stdout, &stderr, deps) + if exitCode != exitSuccess { + t.Fatalf("exit = %d, want %d: %s", exitCode, exitSuccess, stderr.String()) + } + var payload map[string]any + if err := json.Unmarshal(stdout.Bytes(), &payload); err != nil { + t.Fatalf("output is not valid JSON: %v\n%s", err, stdout.String()) + } + if payload["mode"] != "bell" { + t.Errorf("mode = %v, want bell", payload["mode"]) + } + if payload["focusMode"] != "always" { + t.Errorf("focusMode = %v, want always", payload["focusMode"]) + } +} + +// The unconfigured JSON shape: fields are empty strings, never defaults +// filled in, so scripts can tell "user chose" from "default". +func TestRunConfigNotifyPrintsJSONUnconfigured(t *testing.T) { configPath := filepath.Join(t.TempDir(), "config.json") seed := `{ "activeProvider": "openai", @@ -90,11 +129,11 @@ func TestRunConfigNotifyPrintsJSON(t *testing.T) { if err := json.Unmarshal(stdout.Bytes(), &payload); err != nil { t.Fatalf("output is not valid JSON: %v\n%s", err, stdout.String()) } - if payload["mode"] != "both" { - t.Errorf("mode = %v, want both", payload["mode"]) + if payload["mode"] != "" { + t.Errorf("mode = %v, want empty (unconfigured)", payload["mode"]) } - if payload["focusMode"] != "unfocused" { - t.Errorf("focusMode = %v, want unfocused", payload["focusMode"]) + if payload["focusMode"] != "" { + t.Errorf("focusMode = %v, want empty (unconfigured)", payload["focusMode"]) } } @@ -179,6 +218,82 @@ func TestRunConfigNotifyFocusOnlyPreservesMode(t *testing.T) { } } +// Maintainer regression (PR #1001): a partial update must seed from the USER'S +// OWN file, never from the resolved view. A project .zero/config.json setting +// mode=off resolves into the session, but `--focus always` inside that repo +// must NOT copy the project's off into the user's global config. +func TestRunConfigNotifyDoesNotCopyProjectNotifyIntoUserConfig(t *testing.T) { + configPath := filepath.Join(t.TempDir(), "user.json") + projectPath := filepath.Join(t.TempDir(), "project.json") + if err := os.WriteFile(configPath, []byte(`{"activeProvider": "openai"}`), 0o600); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(projectPath, []byte(`{"notify": {"mode": "off"}}`), 0o600); err != nil { + t.Fatal(err) + } + deps := commandCenterDeps(t) + deps.userConfigPath = func() (string, error) { return configPath, nil } + // Resolve EXACTLY the way production does: user config + project config + // merged, so resolved.Notify.Mode is the project's "off". + deps.resolveConfig = func(workspaceRoot string, overrides config.Overrides) (config.ResolvedConfig, error) { + return config.Resolve(config.ResolveOptions{ + UserConfigPath: configPath, + ProjectConfigPath: projectPath, + Env: map[string]string{"OPENAI_API_KEY": "sk-test"}, + }) + } + + var stdout bytes.Buffer + var stderr bytes.Buffer + exitCode := runWithDeps([]string{"config", "notify", "--focus", "always"}, &stdout, &stderr, deps) + if exitCode != exitSuccess { + t.Fatalf("exit = %d, want %d: %s", exitCode, exitSuccess, stderr.String()) + } + cfg := readFileConfig(t, configPath) + // The user's file must keep an unspecified mode unspecified — the + // project's "off" stays in the project file where it belongs. + if cfg.Notify.Mode != "" { + t.Errorf("user Notify.Mode = %q, want blank (project's off must not leak into the user file)", cfg.Notify.Mode) + } + if cfg.Notify.FocusMode != "always" { + t.Errorf("Notify.FocusMode = %q, want always", cfg.Notify.FocusMode) + } + // The project file is untouched. + project := readFileConfig(t, projectPath) + if project.Notify.Mode != "off" { + t.Errorf("project Notify.Mode = %q, want untouched off", project.Notify.Mode) + } +} + +// Maintainer regression (PR #1001): with a clean config, `--mode off` must not +// also pin focusMode as an explicit choice — blank means "use the built-in +// default", and a partial update keeps it that way. +func TestRunConfigNotifyDoesNotPinDefaultsAsExplicitChoices(t *testing.T) { + configPath := filepath.Join(t.TempDir(), "config.json") + if err := os.WriteFile(configPath, []byte(`{"activeProvider": "openai"}`), 0o600); err != nil { + t.Fatal(err) + } + deps := commandCenterDeps(t) + deps.userConfigPath = func() (string, error) { return configPath, nil } + deps.resolveConfig = func(workspaceRoot string, overrides config.Overrides) (config.ResolvedConfig, error) { + return config.Resolve(config.ResolveOptions{UserConfigPath: configPath, Env: map[string]string{"OPENAI_API_KEY": "sk-test"}}) + } + + var stdout bytes.Buffer + var stderr bytes.Buffer + exitCode := runWithDeps([]string{"config", "notify", "--mode", "off"}, &stdout, &stderr, deps) + if exitCode != exitSuccess { + t.Fatalf("exit = %d, want %d: %s", exitCode, exitSuccess, stderr.String()) + } + cfg := readFileConfig(t, configPath) + if cfg.Notify.Mode != "off" { + t.Errorf("Notify.Mode = %q, want off", cfg.Notify.Mode) + } + if cfg.Notify.FocusMode != "" { + t.Errorf("Notify.FocusMode = %q, want blank (unspecified stays unspecified; the default must not be pinned)", cfg.Notify.FocusMode) + } +} + // `--mode` and `--focus` together update both fields in one call. func TestRunConfigNotifyWritesModeAndFocus(t *testing.T) { configPath := filepath.Join(t.TempDir(), "config.json") diff --git a/internal/cli/exec_test.go b/internal/cli/exec_test.go index 63bf0d709..cd00c1ebf 100644 --- a/internal/cli/exec_test.go +++ b/internal/cli/exec_test.go @@ -970,11 +970,10 @@ func TestRunExecUsesProjectConfigAndOpenAICompatibleProvider(t *testing.T) { "name": "local", "provider_kind": "openai-compatible", "base_url": "` + server.URL + `", - "api_key": "sk-local", - "model": "local-model" - }], - "notify": {"mode": "off"} - }` + "api_key": "sk-local", + "model": "local-model" + }] +}` if err := os.WriteFile(filepath.Join(configDir, "config.json"), []byte(writeConfig), 0o600); err != nil { t.Fatal(err) } diff --git a/internal/config/resolver.go b/internal/config/resolver.go index d34939e12..16936874d 100644 --- a/internal/config/resolver.go +++ b/internal/config/resolver.go @@ -64,18 +64,6 @@ const MaxTurnsCeiling = 500 // (set 0 to always advertise every schema, e.g. for a model without tool_search). const defaultDeferThreshold = 3 -// defaultNotifyMode and defaultNotifyFocus are the fallback values used when -// config.json is missing, has no notify block, or has an empty notify block. -// both = terminal bell + OSC-9 desktop notification; unfocused = fire only -// when the TUI window is not the active window so users looking at the prompt -// are not spammed. The defaults make the permission-prompt alert "just work" -// for new users; the TUI /notify command and `zero config notify` let users -// change or opt out. -const ( - defaultNotifyMode = "both" - defaultNotifyFocus = "unfocused" -) - func Resolve(options ResolveOptions) (ResolvedConfig, error) { cfg := FileConfig{ MaxTurns: defaultMaxTurns, @@ -121,18 +109,6 @@ func Resolve(options ResolveOptions) (ResolvedConfig, error) { applyOverrides(&cfg, options.Overrides) - // Notify defaults: when the user has not configured notify (no block, or - // an empty block), apply the built-in defaults so the permission-prompt - // alert works out of the box. A user who explicitly sets notify.mode=off - // or notify.focusMode=focused still wins because their value is - // non-empty after the trim in the validation step below. - if strings.TrimSpace(cfg.Notify.Mode) == "" { - cfg.Notify.Mode = defaultNotifyMode - } - if strings.TrimSpace(cfg.Notify.FocusMode) == "" { - cfg.Notify.FocusMode = defaultNotifyFocus - } - if !cfg.Tools.deferThresholdSet && cfg.Tools.DeferThreshold == 0 { cfg.Tools.DeferThreshold = defaultDeferThreshold } diff --git a/internal/config/resolver_test.go b/internal/config/resolver_test.go index dd8ff0a99..6de99934b 100644 --- a/internal/config/resolver_test.go +++ b/internal/config/resolver_test.go @@ -1864,65 +1864,43 @@ func TestResolveNotifyInvalidFocusMode(t *testing.T) { } } -func TestResolveNotifyDefaultEmpty(t *testing.T) { - path := writeConfig(t, `{}`) - resolved, err := Resolve(ResolveOptions{UserConfigPath: path, Env: map[string]string{}}) - if err != nil { - t.Fatalf("Resolve: %v", err) - } - // Missing notify block falls back to the built-in defaults so the - // permission-prompt alert works for users who never ran setup. - if resolved.Notify.Mode != "both" { - t.Errorf("unset notify.mode should default to %q, got %q", "both", resolved.Notify.Mode) - } - if resolved.Notify.FocusMode != "unfocused" { - t.Errorf("unset notify.focusMode should default to %q, got %q", "unfocused", resolved.Notify.FocusMode) - } -} - -func TestResolveNotifyDefaultEmptyBlock(t *testing.T) { - // An explicit empty notify block should behave the same as a missing one: - // fall back to the built-in defaults. - path := writeConfig(t, `{"notify":{}}`) - resolved, err := Resolve(ResolveOptions{UserConfigPath: path, Env: map[string]string{}}) - if err != nil { - t.Fatalf("Resolve: %v", err) - } - if resolved.Notify.Mode != "both" { - t.Errorf("empty notify.mode should default to %q, got %q", "both", resolved.Notify.Mode) - } - if resolved.Notify.FocusMode != "unfocused" { - t.Errorf("empty notify.focusMode should default to %q, got %q", "unfocused", resolved.Notify.FocusMode) - } -} - -func TestResolveNotifyDefaultPartialEmpty(t *testing.T) { - // Only one field is set; the other should still get the default. - path := writeConfig(t, `{"notify":{"mode":"off"}}`) - resolved, err := Resolve(ResolveOptions{UserConfigPath: path, Env: map[string]string{}}) +// An unconfigured notify block must resolve EMPTY. Defaults live in the TUI +// (effectiveTUINotifyMode) and in `zero config notify`'s display, not in the +// resolver: a filled-in default here leaks into headless `zero exec` (BEL + +// OSC-9 bytes on stderr under -o json) and into the CLI/TUI preserve paths, +// which must treat "user never chose" differently from "user chose both" +// (maintainer review, PR #1001). +func TestResolveNotifyUnconfiguredStaysEmpty(t *testing.T) { + // No config file at all. + resolved, err := Resolve(ResolveOptions{Env: map[string]string{}}) if err != nil { t.Fatalf("Resolve: %v", err) } - if resolved.Notify.Mode != "off" { - t.Errorf("notify.mode should be preserved as %q, got %q", "off", resolved.Notify.Mode) + if resolved.Notify.Mode != "" || resolved.Notify.FocusMode != "" { + t.Fatalf("no config file: notify = %+v, want empty (resolver must not default)", resolved.Notify) } - if resolved.Notify.FocusMode != "unfocused" { - t.Errorf("empty notify.focusMode should default to %q, got %q", "unfocused", resolved.Notify.FocusMode) - } -} -func TestResolveNotifyDefaultNoConfigFile(t *testing.T) { - // No config file at all: defaults should still apply so the - // permission-prompt alert is on for first-run users. - resolved, err := Resolve(ResolveOptions{UserConfigPath: "", Env: map[string]string{}}) - if err != nil { - t.Fatalf("Resolve: %v", err) - } - if resolved.Notify.Mode != "both" { - t.Errorf("missing config: notify.mode should default to %q, got %q", "both", resolved.Notify.Mode) - } - if resolved.Notify.FocusMode != "unfocused" { - t.Errorf("missing config: notify.focusMode should default to %q, got %q", "unfocused", resolved.Notify.FocusMode) + for name, body := range map[string]string{ + "empty config": `{}`, + "empty block": `{"notify":{}}`, + "mode only": `{"notify":{"mode":"off"}}`, + } { + t.Run(name, func(t *testing.T) { + resolved, err := Resolve(ResolveOptions{UserConfigPath: writeConfig(t, body), Env: map[string]string{}}) + if err != nil { + t.Fatalf("Resolve: %v", err) + } + if name == "mode only" { + if resolved.Notify.Mode != "off" { + t.Errorf("notify.mode = %q, want preserved off", resolved.Notify.Mode) + } + } else if resolved.Notify.Mode != "" { + t.Errorf("notify.mode = %q, want empty (resolver must not default)", resolved.Notify.Mode) + } + if resolved.Notify.FocusMode != "" { + t.Errorf("notify.focusMode = %q, want empty (resolver must not default)", resolved.Notify.FocusMode) + } + }) } } diff --git a/internal/config/writer.go b/internal/config/writer.go index 33f3f42cc..20724e839 100644 --- a/internal/config/writer.go +++ b/internal/config/writer.go @@ -592,12 +592,41 @@ func SetTheme(path string, theme string) (FileConfig, error) { return cfg, nil } +// UserNotify returns the notify block stored in the user's own config file at +// path, trimmed. A missing file or missing/empty block returns the zero value: +// callers that need to preserve "whatever the user already chose" on a partial +// update must seed from THIS value, not from the resolved view — the resolver +// merges project config and the TUI applies its own defaults, so seeding from +// resolved copies choices the user never made into their global file (a +// project's mode: off, or a pinned default) and across every other project. +func UserNotify(path string) (NotifyConfig, error) { + path = strings.TrimSpace(path) + if path == "" { + return NotifyConfig{}, nil + } + data, err := os.ReadFile(path) + if err != nil { + if os.IsNotExist(err) { + return NotifyConfig{}, nil + } + return NotifyConfig{}, fmt.Errorf("read config %s: %w", path, err) + } + var cfg FileConfig + if err := json.Unmarshal(data, &cfg); err != nil { + return NotifyConfig{}, fmt.Errorf("invalid config JSON %s: %w", path, err) + } + cfg.Notify.Mode = strings.TrimSpace(cfg.Notify.Mode) + cfg.Notify.FocusMode = strings.TrimSpace(cfg.Notify.FocusMode) + return cfg.Notify, nil +} + // SetNotify persists the TUI notification preference. Both fields are trimmed // and validated against the accepted vocab (mode in {off,bell,notify,both}; // focusMode in {unfocused,always,focused}) so a bad caller cannot write a value // the resolver would later reject at startup. An empty Mode or FocusMode is -// stored as-is — the resolver applies the built-in defaults at read time, so a -// blank value means "use defaults" rather than "no notify" or "no focus rule". +// stored as-is — blank means "use the built-in defaults" (the TUI's +// effectiveTUINotifyMode maps an empty mode to both; the notify package treats +// an empty focusMode as unfocused), not "off". func SetNotify(path string, value NotifyConfig) (FileConfig, error) { path = strings.TrimSpace(path) if path == "" { diff --git a/internal/config/writer_test.go b/internal/config/writer_test.go index da3c2ac0b..69a06a488 100644 --- a/internal/config/writer_test.go +++ b/internal/config/writer_test.go @@ -417,9 +417,9 @@ func TestSetNotifyRejectsEmptyConfigPath(t *testing.T) { } func TestSetNotifyBlankValuesPreservedAsDefaults(t *testing.T) { - // An empty mode/focusMode stored on disk is a valid "use the resolver + // An empty mode/focusMode stored on disk is a valid "use the built-in // defaults" signal — SetNotify must not reject blanks, and they must round - // trip unchanged so the resolver can apply its built-in fallback. + // trip unchanged. path := filepath.Join(t.TempDir(), "zero.json") writeConfigFixture(t, path, FileConfig{ActiveProvider: "openai"}, 0o600) if _, err := SetNotify(path, NotifyConfig{}); err != nil { @@ -431,6 +431,39 @@ func TestSetNotifyBlankValuesPreservedAsDefaults(t *testing.T) { } } +// UserNotify reads the notify block from the user's own file. Partial updates +// seed from this value so they preserve what the USER chose (blank included) +// instead of copying a project config's setting or a pinned default into the +// global file (maintainer review, PR #1001). +func TestUserNotify(t *testing.T) { + dir := t.TempDir() + + // Missing file: zero value, no error. + got, err := UserNotify(filepath.Join(dir, "missing.json")) + if err != nil { + t.Fatalf("missing file should not error: %v", err) + } + if got.Mode != "" || got.FocusMode != "" { + t.Fatalf("missing file = %+v, want zero value", got) + } + + // Present block: trimmed values returned. + path := filepath.Join(dir, "zero.json") + writeConfigFixture(t, path, FileConfig{Notify: NotifyConfig{Mode: " bell ", FocusMode: " always "}}, 0o600) + got, err = UserNotify(path) + if err != nil { + t.Fatalf("UserNotify: %v", err) + } + if got.Mode != "bell" || got.FocusMode != "always" { + t.Fatalf("UserNotify = %+v, want bell/always (trimmed)", got) + } + + // Blank path: zero value, no error. + if got, err = UserNotify(""); err != nil || got.Mode != "" || got.FocusMode != "" { + t.Fatalf("blank path = %+v err=%v, want zero value", got, err) + } +} + func TestRecapsPreferenceRoundTrips(t *testing.T) { // Default (unset) is ON. if !(PreferencesConfig{}).RecapsEnabled() { diff --git a/internal/tui/notify_select.go b/internal/tui/notify_select.go index 6aa7e2d84..d9df97180 100644 --- a/internal/tui/notify_select.go +++ b/internal/tui/notify_select.go @@ -7,50 +7,60 @@ import ( "github.com/Gitlawb/zero/internal/notify" ) -// notifyChoice is one row in the /notify picker. The mode and focusMode pair is -// the on-disk shape; the label is what the user reads. +// notifyChoice is one row in the /notify picker: a (mode, focusMode) pair and +// the label the user reads. type notifyChoice struct { label string - subtitle string mode string focusMode string } -// notifyChoices is the ordered list shown by the /notify picker. "Unfocused + -// both" (the resolver default) is first because it is the most useful option -// for users who do not already have a strong opinion. "Silent" is last so the -// recommended path is also the visually-defaulted one. Adding a new (mode, -// focus) pair is enough to extend the picker and the /notify state list. -var notifyChoices = []notifyChoice{ - { - label: "Notify when unfocused (recommended)", - subtitle: "Sound + desktop notification when the terminal is in the background.", - mode: string(notify.ModeBoth), - focusMode: string(notify.FocusUnfocused), - }, - { - label: "Always notify", - subtitle: "Sound + desktop notification every time Zero needs your input.", - mode: string(notify.ModeBoth), - focusMode: string(notify.FocusAlways), - }, - { - label: "Bell only", - subtitle: "Terminal bell (no desktop notification) every time Zero needs your input.", - mode: string(notify.ModeBell), - focusMode: string(notify.FocusAlways), - }, - { - label: "Silent", - subtitle: "Show prompts in the TUI only — no extra sound or notification.", - mode: string(notify.ModeOff), - focusMode: string(notify.FocusUnfocused), - }, +// notifyChoiceSubtitle renders the human explanation shown as the picker row's +// Meta text: " — ". +func (c notifyChoice) subtitle() string { + modeDescriptions := map[string]string{ + string(notify.ModeOff): "silent", + string(notify.ModeBell): "terminal bell only", + string(notify.ModeNotify): "desktop notification only", + string(notify.ModeBoth): "terminal bell + desktop notification", + } + focusDescriptions := map[string]string{ + string(notify.FocusUnfocused): "only when the terminal is in the background", + string(notify.FocusAlways): "every time", + string(notify.FocusFocused): "only while the terminal is focused", + } + return modeDescriptions[c.mode] + " — " + focusDescriptions[c.focusMode] +} + +// notifyPickerChoices enumerates the FULL mode x focus space (4 modes x 3 +// focus modes = 12 rows), the way newThemePicker enumerates every theme. The +// earlier 4-row curated list could not represent the other 8 valid pairs, so +// opening the picker on one of them fell through to row 0 and Enter silently +// committed a different setting than the user's current one (maintainer +// review, PR #1001). Every valid pair must have a row so Enter always keeps +// (or explicitly changes) the user's actual setting. +func notifyPickerChoices() []notifyChoice { + modes := []string{string(notify.ModeBoth), string(notify.ModeBell), string(notify.ModeNotify), string(notify.ModeOff)} + foci := []string{string(notify.FocusUnfocused), string(notify.FocusAlways), string(notify.FocusFocused)} + choices := make([]notifyChoice, 0, len(modes)*len(foci)) + for _, mode := range modes { + for _, focus := range foci { + choices = append(choices, notifyChoice{ + label: mode + " · " + focus, + mode: mode, + focusMode: focus, + }) + } + } + return choices } // handleNotifyCommand implements /notify [list|off|bell|notify|both [focus]]. -// Bare `/notify` opens the picker at the dispatch layer; a mode-only argument -// keeps the existing focusMode. Mirrors handleThemeCommand. +// Bare `/notify` opens the picker at the dispatch layer. A mode-only argument +// preserves the focusMode stored in the USER'S OWN config (blank stays blank); +// seeding from the model's in-session value would copy a project config's +// choice, or a pinned default, into the user's global file. Mirrors +// handleThemeCommand. func (m model) handleNotifyCommand(args string) (model, string) { tokens := strings.Fields(strings.TrimSpace(args)) if len(tokens) == 0 || tokens[0] == "list" { @@ -69,13 +79,16 @@ func (m model) handleNotifyCommand(args string) (model, string) { if !isValidNotifyFocusMode(focus) { return m, "Notify\nUnknown focus mode: " + tokens[1] + " (expected unfocused, always, or focused)" } - } else { - focus = m.notifyCurrentFocusMode() + } else if stored, err := m.storedNotify(); err == nil { + // Preserve what the USER chose (blank included); never the resolved + // view, which merges project config and in-session defaults. + focus = stored.FocusMode } m.notifyMode = mode m.notifyFocusMode = focus // Apply to the live notifier so the change takes effect on the next - // permission prompt in this session, not just after a restart. + // permission prompt in this session, not just after a restart. A blank + // focusMode is fine here: the notifier treats blank as "unfocused". if m.notifier != nil { m.notifier.Configure(notify.Config{ Mode: notify.Mode(mode), @@ -84,7 +97,7 @@ func (m model) handleNotifyCommand(args string) (model, string) { } lines := []string{ "Notify", - "active mode: " + mode + ", focus: " + focus, + "active mode: " + mode + ", focus: " + effectiveFocusLabel(focus), } if note := m.persistNotifyPreference(mode, focus); note != "" { lines = append(lines, note) @@ -92,6 +105,25 @@ func (m model) handleNotifyCommand(args string) (model, string) { return m, strings.Join(lines, "\n") } +// storedNotify reads the notify block from the user's own config file. Missing +// file or read error returns the zero value (best-effort, like the rest of the +// preference persistence). +func (m model) storedNotify() (config.NotifyConfig, error) { + if strings.TrimSpace(m.userConfigPath) == "" { + return config.NotifyConfig{}, nil + } + return config.UserNotify(m.userConfigPath) +} + +// effectiveFocusLabel renders a focus value for the state line: blank means the +// built-in "unfocused" default, so say so instead of showing an empty string. +func effectiveFocusLabel(focus string) string { + if strings.TrimSpace(focus) == "" { + return "unfocused (default)" + } + return focus +} + // persistNotifyPreference writes the choice to user config so it survives a // restart. Best-effort: returns a short note to surface on failure, or "" on // success / when there is no config path (e.g. tests). @@ -108,21 +140,24 @@ func (m model) persistNotifyPreference(mode string, focus string) string { return "" } -// notifyStateText renders the /notify state view: current mode + focus + the -// picker rows, so the user has the same information whether they ran +// notifyStateText renders the /notify state view: the stored preference plus +// every valid pair, so the user has the same information whether they ran // `/notify list` or just opened the picker. func (m model) notifyStateText() string { - activeMode := m.notifyCurrentMode() - activeFocus := m.notifyCurrentFocusMode() + stored, _ := m.storedNotify() + mode := stored.Mode + if strings.TrimSpace(mode) == "" { + mode = string(effectiveTUINotifyMode(mode)) + } sections := []commandSection{{ Title: "State", Lines: []string{ - "active mode: " + activeMode, - "active focus: " + activeFocus, + "active mode: " + mode, + "active focus: " + effectiveFocusLabel(stored.FocusMode), }, }} - rows := make([]string, 0, len(notifyChoices)) - for _, c := range notifyChoices { + rows := make([]string, 0, 12) + for _, c := range notifyPickerChoices() { rows = append(rows, c.label) } sections = append(sections, commandSection{ @@ -137,18 +172,6 @@ func (m model) notifyStateText() string { }) } -// notifyCurrentMode and notifyCurrentFocusMode return the in-session notify -// preference. newModel populates both from options.Notify via -// effectiveTUINotifyMode (which never returns ""), so no empty fallback is -// needed here. -func (m model) notifyCurrentMode() string { - return m.notifyMode -} - -func (m model) notifyCurrentFocusMode() string { - return m.notifyFocusMode -} - // isValidNotifyMode reports whether s names one of the four notification modes. func isValidNotifyMode(s string) bool { switch s { diff --git a/internal/tui/notify_select_test.go b/internal/tui/notify_select_test.go index bbcedc564..a38694401 100644 --- a/internal/tui/notify_select_test.go +++ b/internal/tui/notify_select_test.go @@ -15,16 +15,15 @@ import ( "github.com/Gitlawb/zero/internal/notify" ) -// A committed /notify choice is written to user config and reloaded at startup -// (via the resolver's defaults + the notifyMode/notifyFocusMode fields on the -// model), so a /notify choice survives restart, just like /theme. +// A committed /notify choice is written to user config and reloaded at startup, +// so a /notify choice survives restart, just like /theme. func TestNotifyChoicePersistsAcrossRestart(t *testing.T) { cfgPath := filepath.Join(t.TempDir(), "config.json") - // First session: pick a non-default notify pair via the text handler (same - // commit path the picker uses via choosePicker). + // First session: pick a notify pair via the text handler (the same commit + // path the picker uses via choosePicker). m := newModel(context.Background(), Options{UserConfigPath: cfgPath}) - m, out := m.handleNotifyCommand("off") + m, out := m.handleNotifyCommand("off always") if m.notifyMode != "off" { t.Fatalf("notifyMode = %q, want off", m.notifyMode) } @@ -41,31 +40,55 @@ func TestNotifyChoicePersistsAcrossRestart(t *testing.T) { if err := json.Unmarshal(data, &cfg); err != nil { t.Fatalf("config is not valid JSON: %v", err) } - if cfg.Notify.Mode != "off" { - t.Fatalf("notify.mode = %q, want off", cfg.Notify.Mode) + if cfg.Notify.Mode != "off" || cfg.Notify.FocusMode != "always" { + t.Fatalf("notify = %+v, want mode=off focusMode=always", cfg.Notify) } - // Second session: the persisted notify block seeds the model fields so the - // /notify state line is correct and a permission prompt uses the right - // notifier (the runtime notifier is built from options.Notify, which is - // populated by the resolver from the same file). - restarted := newModel(context.Background(), Options{UserConfigPath: cfgPath, Notify: config.NotifyConfig{Mode: "off"}}) - if restarted.notifyMode != "off" { - t.Fatalf("restarted notifyMode = %q, want off (from saved config)", restarted.notifyMode) + // Second session: the persisted notify block seeds startup (options.Notify + // is populated by the resolver from the same file). + restarted := newModel(context.Background(), Options{UserConfigPath: cfgPath, Notify: config.NotifyConfig{Mode: "off", FocusMode: "always"}}) + if restarted.notifyMode != "off" || restarted.notifyFocusMode != "always" { + t.Fatalf("restarted = mode %q focus %q, want off/always (from saved config)", restarted.notifyMode, restarted.notifyFocusMode) } } -// `/notify` with a mode-only arg keeps the existing focusMode. A common mistake -// would be to reset the focus rule on every mode change. -func TestNotifyCommandPreservesFocusOnModeOnly(t *testing.T) { - m := newModel(context.Background(), Options{}) - m.notifyFocusMode = string(notify.FocusAlways) +// `/notify bell` (mode-only) preserves the focusMode stored in the USER'S OWN +// file — not the resolved view. A project config's choice must not be copied +// into the user's global file, and a blank focus must stay blank (blank means +// "use the built-in default"), so nothing is pinned as an explicit choice the +// user never made. Maintainer review, PR #1001. +func TestNotifyModeOnlyPreservesStoredFocusNotResolved(t *testing.T) { + cfgPath := filepath.Join(t.TempDir(), "config.json") + if err := os.WriteFile(cfgPath, []byte(`{"notify":{"focusMode":"focused"}}`), 0o600); err != nil { + t.Fatal(err) + } + + // The in-session (resolved) focus disagrees with the user's file; the + // write must take the user's file. + m := newModel(context.Background(), Options{ + UserConfigPath: cfgPath, + Notify: config.NotifyConfig{Mode: "both", FocusMode: "unfocused"}, + }) + m, _ = m.handleNotifyCommand("bell") + persisted := readNotifyBlock(t, cfgPath) + if persisted.Mode != "bell" { + t.Errorf("persisted mode = %q, want bell", persisted.Mode) + } + if persisted.FocusMode != "focused" { + t.Errorf("persisted focusMode = %q, want the user-file value focused (not the resolved unfocused)", persisted.FocusMode) + } + + // Blank stays blank: with nothing stored, a mode-only change must not pin + // the default focus as an explicit choice. + blankPath := filepath.Join(t.TempDir(), "config.json") + m = newModel(context.Background(), Options{UserConfigPath: blankPath}) m, _ = m.handleNotifyCommand("off") - if m.notifyMode != "off" { - t.Errorf("notifyMode = %q, want off", m.notifyMode) + persisted = readNotifyBlock(t, blankPath) + if persisted.Mode != "off" { + t.Errorf("persisted mode = %q, want off", persisted.Mode) } - if m.notifyFocusMode != string(notify.FocusAlways) { - t.Errorf("notifyFocusMode = %q, want preserved %q", m.notifyFocusMode, notify.FocusAlways) + if persisted.FocusMode != "" { + t.Errorf("persisted focusMode = %q, want blank (unspecified stays unspecified)", persisted.FocusMode) } } @@ -85,19 +108,17 @@ func TestNotifyCommandSetsModeAndFocus(t *testing.T) { // the next permission prompt in this session (not only after a restart). func TestNotifyCommandAppliesToLiveNotifier(t *testing.T) { var buf bytes.Buffer - // Construct through newModel so both fields are populated the way the real - // session does; then swap in a buffer-backed notifier to observe output. m := newModel(context.Background(), Options{Notify: config.NotifyConfig{Mode: "off", FocusMode: "always"}}) m.notifier = notify.New(&buf, notify.Config{Mode: notify.ModeOff, FocusMode: notify.FocusAlways}) m.notifier.SetFocused(true) - m, _ = m.handleNotifyCommand("bell") + m, _ = m.handleNotifyCommand("bell always") m.notifier.Notify(notify.Completion, "x") if buf.String() != "\x07" { t.Fatalf("live notifier should bell after /notify bell, got %q", buf.String()) } - m, _ = m.handleNotifyCommand("off") + m, _ = m.handleNotifyCommand("off always") m.notifier.Notify(notify.Completion, "x") if buf.String() != "\x07" { t.Fatalf("live notifier should go silent after /notify off, got %q", buf.String()) @@ -132,8 +153,8 @@ func TestNotifyCommandRejectsInvalidMode(t *testing.T) { } } -// `/notify bell sideways` rejects the focus mode but the call also failed -// validation before persisting, so neither field should change. +// `/notify bell sideways` fails validation before persisting, so neither field +// changes. func TestNotifyCommandRejectsInvalidFocus(t *testing.T) { m := newModel(context.Background(), Options{}) m.notifyMode = "bell" @@ -147,11 +168,48 @@ func TestNotifyCommandRejectsInvalidFocus(t *testing.T) { } } -// `/notify` with no argument opens the picker, just like /theme and /model. -func TestNotifyPickerOpensOnBareNotify(t *testing.T) { - m := newModel(context.Background(), Options{Notify: config.NotifyConfig{Mode: "off", FocusMode: "unfocused"}}) - m.input.SetValue("/notify") +// The picker enumerates the FULL mode x focus space (12 rows), so every valid +// pair is representable and Enter can never silently commit a different pair +// than the user's current one. Maintainer review, PR #1001: with a 4-row +// curated list, opening the picker on (off, always) preselected row 0 +// (both, unfocused) and Enter changed the setting. +func TestNotifyPickerEnumeratesFullSpace(t *testing.T) { + m := newModel(context.Background(), Options{}) + picker := m.newNotifyPicker() + if len(picker.items) != 12 { + t.Fatalf("picker has %d items, want 12 (4 modes x 3 focus modes)", len(picker.items)) + } + seen := map[string]bool{} + for _, item := range picker.items { + if seen[item.Value] { + t.Errorf("duplicate picker row %q", item.Value) + } + seen[item.Value] = true + } + for _, mode := range []string{"off", "bell", "notify", "both"} { + for _, focus := range []string{"unfocused", "always", "focused"} { + if !seen[mode+" "+focus] { + t.Errorf("picker is missing row for valid pair %q %q", mode, focus) + } + } + } +} +// Every pair the picker preselects must also be a row: Enter on an open +// picker must keep (or explicitly change) the user's actual setting. This is +// the maintainer's suggested regression: send Enter to an open picker from a +// pair that is NOT in a 4-row curated list — with full enumeration the +// preselected row IS the current pair and the commit is a no-op change. +func TestNotifyPickerEnterOnUnlistedPairKeepsSetting(t *testing.T) { + cfgPath := filepath.Join(t.TempDir(), "config.json") + if err := os.WriteFile(cfgPath, []byte(`{"notify":{"mode":"off","focusMode":"always"}}`), 0o600); err != nil { + t.Fatal(err) + } + m := newModel(context.Background(), Options{ + UserConfigPath: cfgPath, + Notify: config.NotifyConfig{Mode: "off", FocusMode: "always"}, + }) + m.input.SetValue("/notify") updated, cmd := m.Update(testKey(tea.KeyEnter)) m = updated.(model) if cmd != nil { @@ -160,13 +218,21 @@ func TestNotifyPickerOpensOnBareNotify(t *testing.T) { if m.picker == nil || m.picker.kind != pickerNotify { t.Fatalf("expected the notify picker to open, got %#v", m.picker) } - if len(m.picker.items) != len(notifyChoices) { - t.Fatalf("picker has %d items, want %d", len(m.picker.items), len(notifyChoices)) + // (off, always) must be preselected — it is a valid pair even though the + // old curated list could not represent it. + if sel := m.picker.items[m.picker.selected]; sel.Value != "off always" { + t.Fatalf("preselected = %q, want the current pair %q", sel.Value, "off always") + } + + // Enter commits the preselected row: the setting is unchanged. + updated, _ = m.Update(testKey(tea.KeyEnter)) + m = updated.(model) + if m.picker != nil { + t.Fatal("picker should close on Enter") } - // The preselected row should match the active (mode, focus) pair. - sel := m.picker.items[m.picker.selected] - if sel.Value != "off unfocused" { - t.Errorf("preselected value = %q, want the active pair %q", sel.Value, "off unfocused") + persisted := readNotifyBlock(t, cfgPath) + if persisted.Mode != "off" || persisted.FocusMode != "always" { + t.Fatalf("Enter changed the setting: got %+v, want off/always unchanged", persisted) } } @@ -176,6 +242,9 @@ func TestNotifyPickerOpensOnBareNotify(t *testing.T) { func TestNotifyPickerValuesAreValidCommandArgs(t *testing.T) { m := newModel(context.Background(), Options{}) picker := m.newNotifyPicker() + if len(picker.items) == 0 { + t.Fatal("picker has no items") + } for _, item := range picker.items { tokens := strings.Fields(item.Value) if len(tokens) != 2 { @@ -191,32 +260,34 @@ func TestNotifyPickerValuesAreValidCommandArgs(t *testing.T) { } } -// The /notify state view shows the current mode and focus so users can see -// the value before opening the picker. -func TestNotifyStateTextShowsActivePair(t *testing.T) { - m := newModel(context.Background(), Options{}) - m.notifyMode = "both" - m.notifyFocusMode = "unfocused" +// The /notify state view shows the stored mode and focus (and labels a blank +// focus as the default) so users see the real value before opening the picker. +func TestNotifyStateTextShowsStoredPair(t *testing.T) { + cfgPath := filepath.Join(t.TempDir(), "config.json") + if err := os.WriteFile(cfgPath, []byte(`{"notify":{"mode":"bell","focusMode":"always"}}`), 0o600); err != nil { + t.Fatal(err) + } + m := newModel(context.Background(), Options{UserConfigPath: cfgPath}) state := m.notifyStateText() - if !strings.Contains(state, "active mode: both") { - t.Errorf("state should show active mode, got: %s", state) + if !strings.Contains(state, "active mode: bell") { + t.Errorf("state should show stored mode, got: %s", state) } - if !strings.Contains(state, "active focus: unfocused") { - t.Errorf("state should show active focus, got: %s", state) + if !strings.Contains(state, "active focus: always") { + t.Errorf("state should show stored focus, got: %s", state) } } -// notifyCurrentMode / notifyCurrentFocusMode surface the in-session fields -// that newModel populates from options.Notify, so /notify reads the same -// value the runtime notifier uses. -func TestNotifyCurrentReflectsModelFields(t *testing.T) { - m := newModel(context.Background(), Options{}) - m.notifyMode = "bell" - m.notifyFocusMode = "always" - if got := m.notifyCurrentMode(); got != "bell" { - t.Errorf("notifyCurrentMode = %q, want bell", got) +func readNotifyBlock(t *testing.T, path string) config.NotifyConfig { + t.Helper() + data, err := os.ReadFile(path) + if err != nil { + t.Fatalf("read config %s: %v", path, err) } - if got := m.notifyCurrentFocusMode(); got != "always" { - t.Errorf("notifyCurrentFocusMode = %q, want always", got) + var cfg struct { + Notify config.NotifyConfig `json:"notify"` + } + if err := json.Unmarshal(data, &cfg); err != nil { + t.Fatalf("decode config %s: %v", path, err) } + return cfg.Notify } diff --git a/internal/tui/picker.go b/internal/tui/picker.go index e885f925d..1a72b2656 100644 --- a/internal/tui/picker.go +++ b/internal/tui/picker.go @@ -11,6 +11,7 @@ import ( "github.com/Gitlawb/zero/internal/config" "github.com/Gitlawb/zero/internal/modelregistry" + "github.com/Gitlawb/zero/internal/notify" "github.com/Gitlawb/zero/internal/providercatalog" "github.com/Gitlawb/zero/internal/providermodelcatalog" "github.com/Gitlawb/zero/internal/providermodeldiscovery" @@ -1067,24 +1068,31 @@ func (m model) newThemePicker() *commandPicker { return &commandPicker{kind: pickerTheme, title: "Choose a theme", items: items, allItems: append([]pickerItem{}, items...), selected: selected} } -// newNotifyPicker lists the four (mode, focus) pairs from notifyChoices. Each +// newNotifyPicker lists the FULL (mode, focus) space from notifyPickerChoices +// (every valid pair has a row, so a stored setting like (off, always) is always +// preselectable and Enter can never silently commit a different pair). Each // row's Value is the same synthetic string the text /notify handler accepts // (" "), so /notify with no arg and the picker share one commit -// path through handleNotifyCommand. The currently active pair is preselected so -// the user can press Enter to keep it. There is no live preview — notify -// affects the next permission prompt, not the current view — so the picker -// does not call a preview function on move. +// path through handleNotifyCommand. A blank stored field resolves to its +// effective default for preselection only (both / unfocused — what actually +// fires today); committing any row writes an explicit pair. There is no live +// preview — notify affects the next permission prompt, not the current view. func (m model) newNotifyPicker() *commandPicker { - items := make([]pickerItem, 0, len(notifyChoices)) + choices := notifyPickerChoices() + items := make([]pickerItem, 0, len(choices)) selected := 0 - activeMode := m.notifyCurrentMode() - activeFocus := m.notifyCurrentFocusMode() - for _, c := range notifyChoices { + stored, _ := m.storedNotify() + activeMode := string(effectiveTUINotifyMode(stored.Mode)) + activeFocus := stored.FocusMode + if strings.TrimSpace(activeFocus) == "" { + activeFocus = string(notify.FocusUnfocused) + } + for _, c := range choices { items = append(items, pickerItem{ Group: "When Zero needs your input", Label: c.label, Value: c.mode + " " + c.focusMode, - Meta: c.subtitle, + Meta: c.subtitle(), }) if c.mode == activeMode && c.focusMode == activeFocus { selected = len(items) - 1 From 703d9f5c3c7bbe2a5fcb20cdcf950b046e3ec9e9 Mon Sep 17 00:00:00 2001 From: Gaurav Bhatia Date: Thu, 3 Sep 2026 14:17:14 +0400 Subject: [PATCH 5/5] fix(cli): zero config notify no longer requires a configured provider MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The command resolved the full config (providers included) before touching notification settings, so a fresh user with no provider hit ErrNoActiveProvider and could not read, set, or reset their notify preference — the exact first-run user this feature targets (CodeRabbit review, PR #1001). The command manages a user preference, so it now talks only to the user's own config file (config.UserNotify / config.SetNotify) and never runs config resolution. Display reports the user's stored values — a project config that overrides notify for one repo is not shown here, matching the write path's source-of-truth from the maintainer review. --- internal/cli/config_notify.go | 50 +++++++++++----------- internal/cli/config_notify_test.go | 67 ++++++++++++++++++++++++++++++ 2 files changed, 92 insertions(+), 25 deletions(-) diff --git a/internal/cli/config_notify.go b/internal/cli/config_notify.go index 9c8216971..b8a91a117 100644 --- a/internal/cli/config_notify.go +++ b/internal/cli/config_notify.go @@ -11,7 +11,16 @@ import ( // runConfigNotify implements `zero config notify`: with no flags it prints the // current mode/focusMode; --mode/--focus update them via the same // config.SetNotify writer the TUI /notify command uses, so all surfaces stay -// in lockstep; --reset blanks both fields so the resolver defaults apply. +// in lockstep; --reset blanks both fields so the built-in defaults apply. +// +// The command manages a user preference, so it talks ONLY to the user's own +// config file (config.UserNotify / config.SetNotify) and never runs the full +// config resolution: resolving providers would fail with ErrNoActiveProvider +// for a brand-new user, locking them out of setting notifications before they +// have even configured a provider (CodeRabbit review, PR #1001). The display +// therefore reports the USER'S stored values — a project config that +// overrides notify for one repo is not shown here, by the same logic the +// maintainer applied to the write path. func runConfigNotify(args []string, stdout io.Writer, stderr io.Writer, deps appDeps) int { options, help, err := parseConfigNotifyArgs(args) if err != nil { @@ -24,23 +33,15 @@ func runConfigNotify(args []string, stdout io.Writer, stderr io.Writer, deps app return exitSuccess } - resolved, exitCode := resolveCommandCenterConfig(stderr, deps) - if exitCode != exitSuccess { - return exitCode + configPath, err := deps.userConfigPath() + if err != nil { + return writeAppError(stderr, err.Error(), exitCrash) } if options.mode != "" || options.focus != "" || options.reset { - configPath, err := deps.userConfigPath() - if err != nil { - return writeAppError(stderr, err.Error(), exitCrash) - } - // Seed omitted fields from the USER'S OWN file, never from the - // resolved view: resolved merges project config (so a repo's - // mode:off would be copied into the user's global settings) and - // carries no defaults here, but seeding from it would also pin - // defaults as explicit choices. Blank stays blank — blank means - // "use the built-in defaults". --reset is the only path that - // clears both fields. + // Seed omitted fields from the USER'S OWN file. Blank stays blank — + // blank means "use the built-in defaults"; --reset is the only path + // that clears both fields. current, err := config.UserNotify(configPath) if err != nil { return writeAppError(stderr, err.Error(), exitUsage) @@ -59,18 +60,17 @@ func runConfigNotify(args []string, stdout io.Writer, stderr io.Writer, deps app if _, err := config.SetNotify(configPath, notify); err != nil { return writeAppError(stderr, err.Error(), exitUsage) } - // Re-resolve so the printed value reflects what the next launch will - // actually use (e.g. a reset shows the built-in defaults). - resolved, exitCode = resolveCommandCenterConfig(stderr, deps) - if exitCode != exitSuccess { - return exitCode - } } + // Report the stored values (blank renders as "(default)"). + current, err := config.UserNotify(configPath) + if err != nil { + return writeAppError(stderr, err.Error(), exitUsage) + } if options.json { if err := writePrettyJSON(stdout, map[string]any{ - "mode": resolved.Notify.Mode, - "focusMode": resolved.Notify.FocusMode, + "mode": current.Mode, + "focusMode": current.FocusMode, }); err != nil { return exitCrash } @@ -78,8 +78,8 @@ func runConfigNotify(args []string, stdout io.Writer, stderr io.Writer, deps app } lines := []string{ "Notify", - "mode: " + displayCLIValue(resolved.Notify.Mode, "(default)"), - "focusMode: " + displayCLIValue(resolved.Notify.FocusMode, "(default)"), + "mode: " + displayCLIValue(current.Mode, "(default)"), + "focusMode: " + displayCLIValue(current.FocusMode, "(default)"), } if _, err := fmt.Fprintln(stdout, strings.Join(lines, "\n")); err != nil { return exitCrash diff --git a/internal/cli/config_notify_test.go b/internal/cli/config_notify_test.go index eec1ec6d8..0b3e868a1 100644 --- a/internal/cli/config_notify_test.go +++ b/internal/cli/config_notify_test.go @@ -399,6 +399,73 @@ func TestRunConfigNotifyResetClearsStoredValues(t *testing.T) { } } +// CodeRabbit regression (PR #1001): the command manages a user preference and +// must not require provider resolution. A brand-new user with NO provider +// configured (the resolver would return ErrNoActiveProvider) can still read, +// set, and reset their notification preference. +func TestRunConfigNotifyWorksWithoutAnyProviderConfigured(t *testing.T) { + configPath := filepath.Join(t.TempDir(), "config.json") + if err := os.WriteFile(configPath, []byte(`{}`), 0o600); err != nil { + t.Fatal(err) + } + // resolveConfig fails the way the real resolver does for a fresh user — + // the command must never call it, so a panic-free stub is enough to prove + // the point; use the failing resolver to catch any regression to the + // resolve-first shape. + deps := commandCenterDeps(t) + deps.userConfigPath = func() (string, error) { return configPath, nil } + deps.resolveConfig = func(workspaceRoot string, overrides config.Overrides) (config.ResolvedConfig, error) { + return config.Resolve(config.ResolveOptions{UserConfigPath: configPath, Env: map[string]string{}}) + } + + // Read works and shows defaults. + var stdout bytes.Buffer + var stderr bytes.Buffer + exitCode := runWithDeps([]string{"config", "notify"}, &stdout, &stderr, deps) + if exitCode != exitSuccess { + t.Fatalf("read: exit = %d, want %d: %s", exitCode, exitSuccess, stderr.String()) + } + if !strings.Contains(stdout.String(), "mode: (default)") { + t.Errorf("read should show (default), got: %s", stdout.String()) + } + + // Write works. + stdout.Reset() + exitCode = runWithDeps([]string{"config", "notify", "--mode", "bell", "--focus", "always"}, &stdout, &stderr, deps) + if exitCode != exitSuccess { + t.Fatalf("write: exit = %d, want %d: %s", exitCode, exitSuccess, stderr.String()) + } + cfg := readFileConfig(t, configPath) + if cfg.Notify.Mode != "bell" || cfg.Notify.FocusMode != "always" { + t.Fatalf("write: Notify = %+v, want bell/always", cfg.Notify) + } + + // JSON read reflects the stored pair. + stdout.Reset() + exitCode = runWithDeps([]string{"config", "notify", "--json"}, &stdout, &stderr, deps) + if exitCode != exitSuccess { + t.Fatalf("json: exit = %d, want %d: %s", exitCode, exitSuccess, stderr.String()) + } + var payload map[string]any + if err := json.Unmarshal(stdout.Bytes(), &payload); err != nil { + t.Fatalf("json output invalid: %v\n%s", err, stdout.String()) + } + if payload["mode"] != "bell" || payload["focusMode"] != "always" { + t.Errorf("json = mode %v focus %v, want bell/always", payload["mode"], payload["focusMode"]) + } + + // Reset works. + stdout.Reset() + exitCode = runWithDeps([]string{"config", "notify", "--reset"}, &stdout, &stderr, deps) + if exitCode != exitSuccess { + t.Fatalf("reset: exit = %d, want %d: %s", exitCode, exitSuccess, stderr.String()) + } + cfg = readFileConfig(t, configPath) + if cfg.Notify.Mode != "" || cfg.Notify.FocusMode != "" { + t.Errorf("reset: Notify = %+v, want empty", cfg.Notify) + } +} + // `zero config` (no subcommand) still works after the dispatch change. func TestRunConfigSummaryStillWorks(t *testing.T) { var stdout bytes.Buffer