diff --git a/internal/cli/command_center.go b/internal/cli/command_center.go index a6fab33ec..393742f36 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()) @@ -462,8 +486,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..b8a91a117 --- /dev/null +++ b/internal/cli/config_notify.go @@ -0,0 +1,169 @@ +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 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 { + return writeExecUsageError(stderr, err.Error()) + } + if help { + if err := writeConfigNotifyHelp(stdout); err != nil { + return exitCrash + } + return exitSuccess + } + + configPath, err := deps.userConfigPath() + if err != nil { + return writeAppError(stderr, err.Error(), exitCrash) + } + + if options.mode != "" || options.focus != "" || options.reset { + // 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) + } + notify := current + 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) + } + } + + // 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": current.Mode, + "focusMode": current.FocusMode, + }); err != nil { + return exitCrash + } + return exitSuccess + } + lines := []string{ + "Notify", + "mode: " + displayCLIValue(current.Mode, "(default)"), + "focusMode: " + displayCLIValue(current.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; 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"+ + " 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..0b3e868a1 --- /dev/null +++ b/internal/cli/config_notify_test.go @@ -0,0 +1,480 @@ +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: 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 + // 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: (default)") { + t.Errorf("stdout should show unconfigured mode as (default), 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. 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", + "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"] != "" { + t.Errorf("mode = %v, want empty (unconfigured)", payload["mode"]) + } + if payload["focusMode"] != "" { + t.Errorf("focusMode = %v, want empty (unconfigured)", 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" + }], + "notify": {"mode": "both", "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", "--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) + } + // 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) + } +} + +// 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") + 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) + } +} + +// 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 + 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 a277d553b..cd00c1ebf 100644 --- a/internal/cli/exec_test.go +++ b/internal/cli/exec_test.go @@ -970,10 +970,10 @@ func TestRunExecUsesProjectConfigAndOpenAICompatibleProvider(t *testing.T) { "name": "local", "provider_kind": "openai-compatible", "base_url": "` + server.URL + `", - "api_key": "sk-local", - "model": "local-model" - }] - }` + "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_test.go b/internal/config/resolver_test.go index 13038664e..6de99934b 100644 --- a/internal/config/resolver_test.go +++ b/internal/config/resolver_test.go @@ -1864,14 +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{}}) +// 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 != "" || resolved.Notify.FocusMode != "" { - t.Fatalf("unset notify should be empty, got %+v", resolved.Notify) + t.Fatalf("no config file: notify = %+v, want empty (resolver must not default)", resolved.Notify) + } + + 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 e3b6846f2..20724e839 100644 --- a/internal/config/writer.go +++ b/internal/config/writer.go @@ -8,6 +8,7 @@ import ( "sort" "strings" + "github.com/Gitlawb/zero/internal/notify" "github.com/Gitlawb/zero/internal/providercatalog" ) @@ -591,6 +592,77 @@ 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 — 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 == "" { + 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 +} + // SetPet persists only the terminal-pet preference while preserving every // unrelated user setting through the config writer's atomic replace path. func SetPet(path string, pet string) (FileConfig, error) { diff --git a/internal/config/writer_test.go b/internal/config/writer_test.go index c66fc26ba..69a06a488 100644 --- a/internal/config/writer_test.go +++ b/internal/config/writer_test.go @@ -369,6 +369,101 @@ 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 built-in + // defaults" signal — SetNotify must not reject blanks, and they must round + // trip unchanged. + 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) + } +} + +// 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/notify/notify.go b/internal/notify/notify.go index 79906d5ac..3c9d39ed6 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 @@ -102,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 54ba06aef..2a81720c2 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) @@ -109,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") diff --git a/internal/tui/commands.go b/internal/tui/commands.go index 5eea59d21..a82786127 100644 --- a/internal/tui/commands.go +++ b/internal/tui/commands.go @@ -36,6 +36,7 @@ const ( commandFast commandStyle commandTheme + commandNotify commandTranscript commandBash commandImage @@ -400,6 +401,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 9473de06a..c9f07e89c 100644 --- a/internal/tui/model.go +++ b/internal/tui/model.go @@ -212,16 +212,21 @@ type model struct { keyBindings keyBindings themeMode themeMode // palette preference: system (default) or named palette hasDarkBg bool // last terminal background-detection result, if one is delivered - 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. @@ -876,6 +881,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() @@ -947,7 +966,7 @@ func newModel(ctx context.Context, options Options) model { runSpinner.Spinner.FPS = activeAnimationFrameInterval 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 @@ -1009,6 +1028,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(), @@ -4524,6 +4545,14 @@ func (m model) choosePicker() (tea.Model, tea.Cmd) { return m.showTransientNotice(m.themeAppliedNotice(), transientNoticeSuccess) } m.transcript = reduceTranscript(m.transcript, transcriptAction{kind: actionAppendSystem, text: text}) + 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 } @@ -4932,6 +4961,18 @@ func (m model) dispatchCommand(command parsedCommand) (tea.Model, tea.Cmd) { } m.transcript = reduceTranscript(m.transcript, transcriptAction{kind: actionAppendSystem, text: text}) 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 b81f3a6cc..6da03498c 100644 --- a/internal/tui/model_test.go +++ b/internal/tui/model_test.go @@ -2935,6 +2935,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 TestComposerBlinkStaysSolidWhileTyping(t *testing.T) { base := time.Date(2026, 7, 7, 12, 0, 0, 0, time.UTC) now := base diff --git a/internal/tui/notify_select.go b/internal/tui/notify_select.go new file mode 100644 index 000000000..d9df97180 --- /dev/null +++ b/internal/tui/notify_select.go @@ -0,0 +1,191 @@ +package tui + +import ( + "strings" + + "github.com/Gitlawb/zero/internal/config" + "github.com/Gitlawb/zero/internal/notify" +) + +// notifyChoice is one row in the /notify picker: a (mode, focusMode) pair and +// the label the user reads. +type notifyChoice struct { + label string + mode string + focusMode string +} + +// 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 +// 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" { + 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)" + } + 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 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. 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), + FocusMode: notify.FocusMode(focus), + }) + } + lines := []string{ + "Notify", + "active mode: " + mode + ", focus: " + effectiveFocusLabel(focus), + } + if note := m.persistNotifyPreference(mode, focus); note != "" { + lines = append(lines, note) + } + 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). +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: 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 { + stored, _ := m.storedNotify() + mode := stored.Mode + if strings.TrimSpace(mode) == "" { + mode = string(effectiveTUINotifyMode(mode)) + } + sections := []commandSection{{ + Title: "State", + Lines: []string{ + "active mode: " + mode, + "active focus: " + effectiveFocusLabel(stored.FocusMode), + }, + }} + rows := make([]string, 0, 12) + for _, c := range notifyPickerChoices() { + 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"}, + }) +} + +// 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..a38694401 --- /dev/null +++ b/internal/tui/notify_select_test.go @@ -0,0 +1,293 @@ +package tui + +import ( + "bytes" + "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, +// 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 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 always") + 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" || cfg.Notify.FocusMode != "always" { + t.Fatalf("notify = %+v, want mode=off focusMode=always", cfg.Notify) + } + + // 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 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") + persisted = readNotifyBlock(t, blankPath) + if persisted.Mode != "off" { + t.Errorf("persisted mode = %q, want off", persisted.Mode) + } + if persisted.FocusMode != "" { + t.Errorf("persisted focusMode = %q, want blank (unspecified stays unspecified)", persisted.FocusMode) + } +} + +// `/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) + } +} + +// 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 + 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 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 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()) + } +} + +// `/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) { + 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` fails validation before persisting, so neither field +// changes. +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) + } +} + +// 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 { + 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) + } + // (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") + } + persisted := readNotifyBlock(t, cfgPath) + if persisted.Mode != "off" || persisted.FocusMode != "always" { + t.Fatalf("Enter changed the setting: got %+v, want off/always unchanged", persisted) + } +} + +// 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() + 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 { + 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 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: bell") { + t.Errorf("state should show stored mode, got: %s", state) + } + if !strings.Contains(state, "active focus: always") { + t.Errorf("state should show stored focus, got: %s", state) + } +} + +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) + } + 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 563dffe62..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" @@ -29,6 +30,7 @@ const ( pickerSTTModel pickerSTTDownload pickerPet + pickerNotify ) // pickerItem is one selectable row: Label is shown, Value is passed to the @@ -1066,6 +1068,39 @@ func (m model) newThemePicker() *commandPicker { return &commandPicker{kind: pickerTheme, title: "Choose a theme", items: items, allItems: append([]pickerItem{}, items...), selected: selected} } +// 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. 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 { + choices := notifyPickerChoices() + items := make([]pickerItem, 0, len(choices)) + selected := 0 + 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(), + }) + 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. Theme candidates render // only in the picker preview; their active palette is applied only after Enter. // Safe to call with no picker open. Callers mutate through m.picker (a pointer),