From 86ae142dfeff6aacbb5b770ae9876dcc736e866b Mon Sep 17 00:00:00 2001 From: vikingowl Date: Thu, 4 Jun 2026 13:26:01 +0200 Subject: [PATCH] fix(upgrade-config): friendly "no such file" + add --global flag First-run UX fix. `gnoma upgrade-config --dry-run` in a directory with no `.gnoma/config.toml` used to error with: error: read config: open .../.gnoma/config.toml: no such file or directory That's a hard error for what's actually a non-event. The cleanest user experience: tell the user there's no project config to upgrade, hint that they can pass an explicit path or use --global, and exit 0. Changes: 1. `cmd/gnoma/upgrade_config_cmd.go::runUpgradeConfigCommand` now stats the target before calling `gnomacfg.Upgrade`. For the implicit project/global targets, a missing file produces a friendly exit-0 message. An explicit path the user typed is still a hard error (caller asked for that specific file, didn't get it). 2. New `--global` flag, symmetric with `gnoma config set --global`. The user-level config is where zero-spam actually accumulates over time (most users never have a project config) so this is the more useful default target in practice. `--global ` is rejected as mutually-exclusive. 3. Rewrote the flag-parsing loop to avoid a Go slice-aliasing bug discovered while writing the tests. The original implementation did `pathArgs = append(args[:i], args[i+1:]...)` inside a `for i, a := range args` loop, which aliases the underlying array and overwrites earlier `a` values on subsequent iterations. With `--global --dry-run` the `--dry-run` overwrote `args[0]`, so the second iteration read `--dry-run` as `a` for both the `--dry-run` and `--global` cases. The new code walks `args` once and accumulates into a fresh `pathArgs` slice, no aliasing. Tests added in upgrade_config_cmd_test.go: - TestRunUpgradeConfig_MissingProjectConfigIsFriendly - TestRunUpgradeConfig_MissingGlobalConfigIsFriendly - TestRunUpgradeConfig_GlobalFlagUpgradesGlobalConfig - TestRunUpgradeConfig_GlobalWithExplicitPathIsError End-to-end check on the user's actual environment: $ gnoma upgrade-config --dry-run /home/.../gnoma/.gnoma/config.toml: no such file, nothing to upgrade hint: pass an explicit path, or use --global for the user-level config exit: 0 $ gnoma upgrade-config --global --dry-run /home/.../.config/gnoma/config.toml: already clean, nothing to do (dry run) exit: 0 --- cmd/gnoma/upgrade_config_cmd.go | 60 ++++++++++++++++----- cmd/gnoma/upgrade_config_cmd_test.go | 78 ++++++++++++++++++++++++++++ 2 files changed, 126 insertions(+), 12 deletions(-) diff --git a/cmd/gnoma/upgrade_config_cmd.go b/cmd/gnoma/upgrade_config_cmd.go index b8f785b..c8de1b4 100644 --- a/cmd/gnoma/upgrade_config_cmd.go +++ b/cmd/gnoma/upgrade_config_cmd.go @@ -15,31 +15,67 @@ import ( // // Single-file mode only. `--all-projects` is deferred to the // project-registry work in the 2026-05-24 config-migration plan. +// +// Target selection: +// - `gnoma upgrade-config` (no args) → project config +// - `gnoma upgrade-config --global` → global config +// - `gnoma upgrade-config ` → the given path +// - `gnoma upgrade-config --global ` → error (mutually exclusive) +// +// If the default target (project or global config) doesn't exist, +// print a friendly "nothing to upgrade" message and exit 0 — not +// a hard error. The user can pass an explicit path to upgrade a +// different file. func runUpgradeConfigCommand(args []string) int { + // Walk args in a single pass, building pathArgs into a fresh + // slice. Using args[:i] / args[i+1:] in-place would alias the + // underlying array and corrupt subsequent iterations' `a` + // reads (a known Go slice footgun). The fresh-slice approach + // keeps the parsing correct regardless of flag ordering. + var pathArgs []string dryRun := false - pathArgs := args - for i, a := range args { - if a == "--dry-run" { + global := false + for _, a := range args { + switch a { + case "--dry-run": dryRun = true - pathArgs = append(args[:i], args[i+1:]...) - break + case "--global": + global = true + default: + pathArgs = append(pathArgs, a) } } - // Default to the project config if no path given — matches - // the convention of `gnoma config set` writing to the project - // file by default. + // --global and an explicit path are mutually exclusive. + if global && len(pathArgs) > 0 { + fmt.Fprintln(os.Stderr, "usage: gnoma upgrade-config [--dry-run] [--global | ]") + return 1 + } target := "" - switch len(pathArgs) { - case 0: + switch { + case global: + target = gnomacfg.GlobalConfigPath() + case len(pathArgs) == 0: target = gnomacfg.ProjectConfigPath() - case 1: + case len(pathArgs) == 1: target = pathArgs[0] default: - fmt.Fprintln(os.Stderr, "usage: gnoma upgrade-config [--dry-run] [path]") + fmt.Fprintln(os.Stderr, "usage: gnoma upgrade-config [--dry-run] [--global | ]") return 1 } + // Friendly "nothing to upgrade" when the default target + // doesn't exist. We only do this for the default targets + // (project/global); an explicit path the user typed that + // doesn't exist is a real error surfaced by Upgrade() below. + if global || len(pathArgs) == 0 { + if _, err := os.Stat(target); os.IsNotExist(err) { + fmt.Printf("%s: no such file, nothing to upgrade\n", target) + fmt.Println("hint: pass an explicit path, or use --global for the user-level config") + return 0 + } + } + if dryRun { return runUpgradeConfigDryRun(target) } diff --git a/cmd/gnoma/upgrade_config_cmd_test.go b/cmd/gnoma/upgrade_config_cmd_test.go index 5b2a410..cf73546 100644 --- a/cmd/gnoma/upgrade_config_cmd_test.go +++ b/cmd/gnoma/upgrade_config_cmd_test.go @@ -140,3 +140,81 @@ func TestRunUpgradeConfig_AlreadyCleanIsNoOp(t *testing.T) { } } } + +// TestRunUpgradeConfig_MissingProjectConfigIsFriendly verifies the +// user-experience fix for the 2026-06-04 follow-up: when the +// project .gnoma/config.toml doesn't exist, print a friendly +// "nothing to upgrade" message and exit 0 instead of a hard +// "no such file or directory" error. The user can pass an +// explicit path or use --global. +func TestRunUpgradeConfig_MissingProjectConfigIsFriendly(t *testing.T) { + dir := t.TempDir() + t.Setenv("XDG_CONFIG_HOME", dir) + + origDir, _ := os.Getwd() + projectDir := filepath.Join(dir, "project") + if err := os.MkdirAll(projectDir, 0o755); err != nil { + t.Fatalf("mkdir: %v", err) + } + if err := os.Chdir(projectDir); err != nil { + t.Fatalf("chdir: %v", err) + } + t.Cleanup(func() { _ = os.Chdir(origDir) }) + + // No .gnoma/ dir at all — Upgrade() would error. + if rc := runUpgradeConfigCommand(nil); rc != 0 { + t.Errorf("rc = %d, want 0 for missing project config (friendly exit)", rc) + } +} + +// TestRunUpgradeConfig_MissingGlobalConfigIsFriendly mirrors +// the above for --global. The user-level config not existing +// is also "nothing to upgrade", not an error. +func TestRunUpgradeConfig_MissingGlobalConfigIsFriendly(t *testing.T) { + dir := t.TempDir() + t.Setenv("XDG_CONFIG_HOME", dir) + // Don't create the global config dir either. + + if rc := runUpgradeConfigCommand([]string{"--global"}); rc != 0 { + t.Errorf("rc = %d, want 0 for missing global config (friendly exit)", rc) + } +} + +// TestRunUpgradeConfig_GlobalFlagUpgradesGlobalConfig verifies +// the --global flag actually points at the global config and +// upgrades it. +func TestRunUpgradeConfig_GlobalFlagUpgradesGlobalConfig(t *testing.T) { + dir := t.TempDir() + t.Setenv("XDG_CONFIG_HOME", dir) + + // Seed a global config with a default-equivalent field. + globalDir := filepath.Join(dir, "gnoma") + if err := os.MkdirAll(globalDir, 0o755); err != nil { + t.Fatalf("mkdir: %v", err) + } + globalPath := filepath.Join(globalDir, "config.toml") + if err := os.WriteFile(globalPath, []byte("[provider]\nmax_tokens = 8192\n"), 0o644); err != nil { + t.Fatalf("seed: %v", err) + } + + if rc := runUpgradeConfigCommand([]string{"--global"}); rc != 0 { + t.Errorf("rc = %d, want 0", rc) + } + + got, _ := os.ReadFile(globalPath) + if strings.Contains(string(got), "max_tokens") { + t.Errorf("max_tokens at default not dropped from global config, got:\n%s", got) + } +} + +// TestRunUpgradeConfig_GlobalWithExplicitPathIsError verifies +// the mutually-exclusive-flag handling: --global and an +// explicit path can't both be supplied. +func TestRunUpgradeConfig_GlobalWithExplicitPathIsError(t *testing.T) { + dir := t.TempDir() + t.Setenv("XDG_CONFIG_HOME", dir) + + if rc := runUpgradeConfigCommand([]string{"--global", "/tmp/somewhere/config.toml"}); rc != 1 { + t.Errorf("rc = %d, want 1 for --global + explicit path", rc) + } +}