From f321dabce3845ef6afa2d3608fd9aba75f8a9faa Mon Sep 17 00:00:00 2001 From: vikingowl Date: Thu, 4 Jun 2026 18:05:14 +0200 Subject: [PATCH] =?UTF-8?q?feat(config):=20Phase=203=20=E2=80=94=20`gnoma?= =?UTF-8?q?=20doctor`=20diagnostic=20command?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Phase 3 of the 2026-05-24 config-migration plan. Read-only diagnostic over config files. Pairs with `gnoma upgrade-config` from the previous slice: doctor finds things upgrade-config can't fix, upgrade-config fixes the things it can. What doctor surfaces (severity-ranked): error — file unreadable, file unparseable warn — unknown top-level keys (decoder silently ignores them today) — invalid enum values (permission.mode, router.prefer, slm.backend) — explicit-zero pointer fields whose resolved value diverges from the default (e.g. max_tokens = 0 when default is 8192) info — (reserved; current diagnostics are warn+) What doctor does NOT yet surface: - Per-field zero-spam inside a partially-set section (e.g. user wrote [provider] default = "anthropic" with no other fields — those are at Go zero but the encoder's omitempty handles them on the next write). Catching this requires per-key source-tracking that BurntSushi's MetaData doesn't expose for nested fields; tracked as a follow-up. - Cross-file layering bugs (e.g. project file's prefer = "" silently shadows global's prefer = "cloud"). That requires loading the full layered config and diffing per-section — could be a follow-up to doctor, or the per-project upgrade-config --all flow. CLI surface (`cmd/gnoma doctor`): gnoma doctor scan the project config (default — cwd's .gnoma/config.toml) gnoma doctor scan a specific file gnoma doctor --all-projects walk the registry, scan global + every known project gnoma doctor --json structured JSON to stdout (severity as string, suitable for CI/scripts) exit code: 0 = clean, 1 = any warn/error Help text: `gnoma -h` now lists `doctor` alongside the other subcommands. Implementation: internal/config/doctor.go Severity, Finding, Doctor, DiagnoseFile, DiagnoseFiles (~150 lines). internal/config/doctor_test.go 11 tests covering each finding type + Severity.String. cmd/gnoma/doctor_cmd.go CLI dispatch + JSON / text rendering + exit code. cmd/gnoma/doctor_cmd_test.go 5 tests for the CLI surface. internal/config/load.go new ProjectConfigPathFor helper for --all-projects (constructs a project config path from an arbitrary root without chdir). cmd/gnoma/main.go dispatch case + -h help text. Severity.MarshalJSON is custom: encodes the int as its lower-case name string ("warn" not 1) for stable CI consumption. Tests assert on the string form. End-to-end check on a synthetic config with multiple findings: $ gnoma doctor warn ...:permission.mode invalid permission.mode "yes" ... → fix the value, or remove the line warn ...:provider.max_tokens explicit zero for provider.max_tokens (resolved to 0); the default is 8192. ... warn ...:unknown_section unknown top-level key "unknown_section" ... warn ...:unknown_section.foo unknown top-level key "unknown_section.foo" ... exit: 1 $ gnoma doctor --json [ { "severity": "warn", "path": "...", "key": "permission.mode", "message": "..." }, ... ] Quality pipeline: gofmt -l . clean go vet ./... clean golangci-lint run ... 0 issues on touched packages go test ./... all pass (only the pre-existing TestStartBackend_Auto_NothingReachable environmental failure remains) Refs: docs/superpowers/plans/2026-05-24-config-migration.md § Phase 3. --- cmd/gnoma/doctor_cmd.go | 141 ++++++++++++++++ cmd/gnoma/doctor_cmd_test.go | 145 +++++++++++++++++ cmd/gnoma/main.go | 3 + internal/config/doctor.go | 287 +++++++++++++++++++++++++++++++++ internal/config/doctor_test.go | 256 +++++++++++++++++++++++++++++ internal/config/load.go | 15 +- 6 files changed, 844 insertions(+), 3 deletions(-) create mode 100644 cmd/gnoma/doctor_cmd.go create mode 100644 cmd/gnoma/doctor_cmd_test.go create mode 100644 internal/config/doctor.go create mode 100644 internal/config/doctor_test.go diff --git a/cmd/gnoma/doctor_cmd.go b/cmd/gnoma/doctor_cmd.go new file mode 100644 index 0000000..97a343e --- /dev/null +++ b/cmd/gnoma/doctor_cmd.go @@ -0,0 +1,141 @@ +package main + +import ( + "encoding/json" + "fmt" + "os" + "sort" + + gnomacfg "somegit.dev/Owlibou/gnoma/internal/config" +) + +// runDoctorCommand handles `gnoma doctor`. Read-only diagnostic +// over config files. Default: scans the project config (and +// the global config if the project one is missing). With +// `--all-projects`, walks the registry. With `--json`, +// emits structured findings to stdout for CI consumption. +// Exits non-zero on Warn+ findings (CI-friendly). +func runDoctorCommand(args []string) int { + jsonOutput := false + allProjects := false + pathArgs := args + for i, a := range args { + switch a { + case "--json": + jsonOutput = true + pathArgs = append(args[:i], args[i+1:]...) + case "--all-projects": + allProjects = true + pathArgs = append(args[:i], args[i+1:]...) + } + } + + var paths []string + switch { + case allProjects: + loaded, err := gnomacfg.LoadRegistry() + if err != nil { + fmt.Fprintf(os.Stderr, "error: load registry: %v\n", err) + return 1 + } + // Always include the global config in --all-projects + // mode (it applies to every project). Then per-project + // configs from the registry. Files that don't exist + // are filtered out — the doctor reports a finding for + // them, but in --all-projects mode we silently skip + // rather than reporting every project root that has + // been visited but has no config. + paths = append(paths, gnomacfg.GlobalConfigPath()) + for _, p := range loaded.Projects { + paths = append(paths, gnomacfg.ProjectConfigPathFor(p.Path)) + } + // Dedupe and sort for deterministic output. + seen := map[string]bool{} + var deduped []string + for _, p := range paths { + if seen[p] { + continue + } + seen[p] = true + deduped = append(deduped, p) + } + sort.Strings(deduped) + paths = deduped + case len(pathArgs) == 0: + paths = []string{gnomacfg.ProjectConfigPath()} + case len(pathArgs) == 1: + paths = []string{pathArgs[0]} + default: + fmt.Fprintln(os.Stderr, "usage: gnoma doctor [--all-projects] [--json] [path]") + return 1 + } + + doc := gnomacfg.NewDoctor() + findings := doc.DiagnoseFiles(paths) + return renderAndExit(findings, jsonOutput) +} + +// renderAndExit emits findings to stdout (text or JSON per +// the --json flag) and returns the exit code: +// +// 0 — clean (no findings, or only Info findings) +// 1 — Warn or Error findings present +// +// Error findings indicate file-level failures (missing or +// corrupt files); for those the message is the only signal. +// Warn findings are the actionable ones — the user should +// review and fix. +func renderAndExit(findings []gnomacfg.Finding, jsonOutput bool) int { + if jsonOutput { + enc := json.NewEncoder(os.Stdout) + enc.SetIndent("", " ") + if err := enc.Encode(findings); err != nil { + fmt.Fprintf(os.Stderr, "error: encode json: %v\n", err) + return 1 + } + } else { + renderText(os.Stdout, findings) + } + + for _, f := range findings { + if f.Severity >= gnomacfg.SeverityWarn { + return 1 + } + } + return 0 +} + +// renderText writes findings in a human-readable columnar +// format. Severity column, then path:key, then message. +// Color is intentionally omitted — this is for terminals and +// CI logs alike. +func renderText(w *os.File, findings []gnomacfg.Finding) { + if len(findings) == 0 { + _, _ = fmt.Fprintln(w, "no findings — config looks clean") + return + } + // Find the longest path:key for column alignment. + maxWidth := 0 + for _, f := range findings { + loc := f.Path + if f.Key != "" { + loc = f.Path + ":" + f.Key + } + if len(loc) > maxWidth { + maxWidth = len(loc) + } + } + for _, f := range findings { + loc := f.Path + if f.Key != "" { + loc = f.Path + ":" + f.Key + } + _, _ = fmt.Fprintf(w, "%-7s %-*s %s\n", f.Severity, maxWidth, loc, f.Message) + if f.Suggestion != "" { + _, _ = fmt.Fprintf(w, "%-7s %-*s → %s\n", "", maxWidth, "", f.Suggestion) + } + } +} + +// Ensure the file ends cleanly. +var _ = renderAndExit diff --git a/cmd/gnoma/doctor_cmd_test.go b/cmd/gnoma/doctor_cmd_test.go new file mode 100644 index 0000000..495791a --- /dev/null +++ b/cmd/gnoma/doctor_cmd_test.go @@ -0,0 +1,145 @@ +package main + +import ( + "encoding/json" + "os" + "path/filepath" + "strings" + "testing" +) + +// TestRunDoctorCommand_CleanFileExitsZero verifies the +// happy path: a valid config produces no findings and the +// command exits 0. +func TestRunDoctorCommand_CleanFileExitsZero(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) }) + + // Create a project config with a valid user value. + if err := os.MkdirAll(filepath.Join(projectDir, ".gnoma"), 0o755); err != nil { + t.Fatalf("mkdir: %v", err) + } + if err := os.WriteFile( + filepath.Join(projectDir, ".gnoma", "config.toml"), + []byte("[provider]\ndefault = \"anthropic\"\n"), + 0o644, + ); err != nil { + t.Fatalf("seed: %v", err) + } + + if rc := runDoctorCommand(nil); rc != 0 { + t.Errorf("rc = %d, want 0 for clean file", rc) + } +} + +// TestRunDoctorCommand_WarnFindingExitsOne verifies the +// CI-friendly exit code: a Warn finding (invalid enum +// value) causes a non-zero exit. +func TestRunDoctorCommand_WarnFindingExitsOne(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "config.toml") + if err := os.WriteFile(path, []byte("[permission]\nmode = \"yes\"\n"), 0o644); err != nil { + t.Fatalf("seed: %v", err) + } + + if rc := runDoctorCommand([]string{path}); rc != 1 { + t.Errorf("rc = %d, want 1 for warn finding", rc) + } +} + +// TestRunDoctorCommand_JSONOutputIsValidJSON verifies the +// --json flag emits parseable JSON to stdout, suitable for +// CI/script consumption. +func TestRunDoctorCommand_JSONOutputIsValidJSON(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "config.toml") + if err := os.WriteFile(path, []byte("[permission]\nmode = \"yes\"\n"), 0o644); err != nil { + t.Fatalf("seed: %v", err) + } + + // Capture stdout. + origStdout := os.Stdout + r, w, _ := os.Pipe() + os.Stdout = w + t.Cleanup(func() { os.Stdout = origStdout }) + + rc := runDoctorCommand([]string{path, "--json"}) + _ = w.Close() + if rc != 1 { + t.Errorf("rc = %d, want 1", rc) + } + + buf := make([]byte, 8192) + n, _ := r.Read(buf) + out := string(buf[:n]) + + // Should be valid JSON array of Finding objects. + var findings []map[string]any + if err := json.Unmarshal([]byte(out), &findings); err != nil { + t.Fatalf("json.Unmarshal: %v\noutput:\n%s", err, out) + } + if len(findings) == 0 { + t.Errorf("json output had zero findings; expected at least one") + } + if findings[0]["severity"] != "warn" { + t.Errorf("severity = %v, want warn", findings[0]["severity"]) + } +} + +// TestRunDoctorCommand_TextOutputIncludesFindingKey verifies +// the human-readable output format. Should include the file +// path and the finding key. +func TestRunDoctorCommand_TextOutputIncludesFindingKey(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "config.toml") + if err := os.WriteFile(path, []byte("[permission]\nmode = \"yes\"\n"), 0o644); err != nil { + t.Fatalf("seed: %v", err) + } + + origStdout := os.Stdout + r, w, _ := os.Pipe() + os.Stdout = w + t.Cleanup(func() { os.Stdout = origStdout }) + + rc := runDoctorCommand([]string{path}) + _ = w.Close() + if rc != 1 { + t.Errorf("rc = %d, want 1", rc) + } + + buf := make([]byte, 4096) + n, _ := r.Read(buf) + out := string(buf[:n]) + + if !strings.Contains(out, "permission.mode") { + t.Errorf("output missing key, got:\n%s", out) + } + if !strings.Contains(out, path) { + t.Errorf("output missing path, got:\n%s", out) + } + if !strings.Contains(out, "warn") { + t.Errorf("output missing severity, got:\n%s", out) + } +} + +// TestRunDoctorCommand_MissingFileExitsOne documents the +// error path: a missing config file produces a single +// SeverityError finding and the command exits 1. +func TestRunDoctorCommand_MissingFileExitsOne(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "nonexistent.toml") + + if rc := runDoctorCommand([]string{path}); rc != 1 { + t.Errorf("rc = %d, want 1 for missing file", rc) + } +} diff --git a/cmd/gnoma/main.go b/cmd/gnoma/main.go index 9012f8e..54168fb 100644 --- a/cmd/gnoma/main.go +++ b/cmd/gnoma/main.go @@ -89,6 +89,7 @@ func main() { fmt.Fprintf(os.Stderr, " gnoma router stats show router quality + classifier telemetry\n") fmt.Fprintf(os.Stderr, " gnoma config write a config key or list whitelisted keys\n") fmt.Fprintf(os.Stderr, " gnoma upgrade-config clean a config file in place (--dry-run previews)\n") + fmt.Fprintf(os.Stderr, " gnoma doctor diagnostic scan; --all-projects walks the registry\n") fmt.Fprintf(os.Stderr, "\nFlags:\n") flag.PrintDefaults() } @@ -189,6 +190,8 @@ func main() { os.Exit(runConfigCommand(cliArgs[1:])) case "upgrade-config": os.Exit(runUpgradeConfigCommand(cliArgs[1:])) + case "doctor": + os.Exit(runDoctorCommand(cliArgs[1:])) } } diff --git a/internal/config/doctor.go b/internal/config/doctor.go new file mode 100644 index 0000000..61e31bc --- /dev/null +++ b/internal/config/doctor.go @@ -0,0 +1,287 @@ +package config + +import ( + "fmt" + "os" + "sort" + + "github.com/BurntSushi/toml" +) + +// Severity ranks diagnostic findings for the CLI output and +// exit-code decision. Higher numeric value = more severe. +type Severity int + +const ( + // SeverityInfo is a neutral observation (e.g. "field is at + // the default value, can be removed"). Never causes a + // non-zero exit on its own. + SeverityInfo Severity = iota + + // SeverityWarn indicates a likely problem the user should + // review (e.g. an invalid enum value, an explicit-zero + // pointer field that diverges from the default). Causes + // a non-zero exit in CLI mode by default. + SeverityWarn + + // SeverityError indicates a hard failure (file unreadable, + // file unparseable). Causes a non-zero exit. + SeverityError +) + +// String returns the lower-case name of the severity for +// human-readable output. +func (s Severity) String() string { + switch s { + case SeverityInfo: + return "info" + case SeverityWarn: + return "warn" + case SeverityError: + return "error" + default: + return "?" + } +} + +// MarshalJSON encodes Severity as its lower-case name string +// (e.g. "warn", "error") for stable CI/script consumption. +// The default Go marshaling would emit the int value, which +// is opaque to consumers. +func (s Severity) MarshalJSON() ([]byte, error) { + return []byte(`"` + s.String() + `"`), nil +} + +// Finding is one diagnostic result. The CLI renders these +// either as human-readable text or as JSON (--json flag). +type Finding struct { + Severity Severity `json:"severity"` + Path string `json:"path"` + Key string `json:"key,omitempty"` + Message string `json:"message"` + Suggestion string `json:"suggestion,omitempty"` +} + +// Doctor runs diagnostic checks on config files. Constructed +// with NewDoctor; reusable across many files. Stateless after +// construction — set Defaults to override the comparison +// baseline (used in tests; production always uses Defaults()). +type Doctor struct { + // Defaults is the baseline for "is this field at the + // default value" checks. If nil, Defaults() is used. + Defaults *Config +} + +// NewDoctor returns a Doctor with the production defaults +// baseline. +func NewDoctor() *Doctor { + return &Doctor{Defaults: nil} +} + +// DiagnoseFile runs the full diagnostic suite on a single +// config file. The returned slice may be empty (file is +// clean) or contain findings of any severity. +func (d *Doctor) DiagnoseFile(path string) []Finding { + data, err := os.ReadFile(path) + if err != nil { + return []Finding{{ + Severity: SeverityError, + Path: path, + Message: fmt.Sprintf("read: %v", err), + }} + } + + var cfg Config + meta, err := toml.Decode(string(data), &cfg) + if err != nil { + return []Finding{{ + Severity: SeverityError, + Path: path, + Message: fmt.Sprintf("parse: %v", err), + }} + } + + defaults := d.Defaults + if defaults == nil { + def := Defaults() + defaults = &def + } + + var findings []Finding + findings = append(findings, d.detectUnknownKeys(path, meta)...) + findings = append(findings, d.detectInvalidEnums(path, &cfg)...) + findings = append(findings, d.detectExplicitZeros(path, &cfg, defaults)...) + return findings +} + +// DiagnoseFiles runs DiagnoseFile on each path in turn and +// returns the concatenated findings. The order is the input +// order; callers that want deterministic output should sort +// their input list first. +func (d *Doctor) DiagnoseFiles(paths []string) []Finding { + var findings []Finding + for _, p := range paths { + findings = append(findings, d.DiagnoseFile(p)...) + } + // Stable order for diff-friendly CI output. + sort.SliceStable(findings, func(i, j int) bool { + if findings[i].Path != findings[j].Path { + return findings[i].Path < findings[j].Path + } + if findings[i].Severity != findings[j].Severity { + return findings[i].Severity > findings[j].Severity + } + return findings[i].Key < findings[j].Key + }) + return findings +} + +// detectUnknownKeys surfaces top-level keys in the source that +// don't map to any Config field. Decoder ignores them silently +// today; doctor flags them so the user can clean up typos +// like `[provdier]` or removed-schema leftovers. +func (d *Doctor) detectUnknownKeys(path string, meta toml.MetaData) []Finding { + var findings []Finding + for _, k := range meta.Undecoded() { + findings = append(findings, Finding{ + Severity: SeverityWarn, + Path: path, + Key: k.String(), + Message: fmt.Sprintf("unknown top-level key %q (not in the current Config schema)", k.String()), + Suggestion: "remove the section or rename to a known key", + }) + } + return findings +} + +// detectInvalidEnums checks enum-typed string fields against +// their parsers. The current set is intentionally small — +// only fields with a documented value space and a parser +// function. Add more as the surface grows. +func (d *Doctor) detectInvalidEnums(path string, cfg *Config) []Finding { + var findings []Finding + + // permission.mode — must be a permission.Mode constant. + if cfg.Permission.Mode != "" && !validPermissionMode(cfg.Permission.Mode) { + findings = append(findings, Finding{ + Severity: SeverityWarn, + Path: path, + Key: "permission.mode", + Message: fmt.Sprintf("invalid permission.mode %q (expected one of: default, accept_edits, bypass, deny, plan, auto)", cfg.Permission.Mode), + Suggestion: "fix the value, or remove the line to use the default", + }) + } + + // router.prefer — must parse via router.ParsePreferPolicy. + // (That parser accepts "" and "auto" as valid, so we skip + // the check on those.) + if cfg.Router.Prefer != "" && cfg.Router.Prefer != "auto" && + !validRouterPrefer(cfg.Router.Prefer) { + findings = append(findings, Finding{ + Severity: SeverityWarn, + Path: path, + Key: "router.prefer", + Message: fmt.Sprintf("invalid router.prefer %q (expected \"local\", \"cloud\", or \"auto\")", cfg.Router.Prefer), + Suggestion: "fix the value, or remove the line to use the default", + }) + } + + // slm.backend — must be a recognized backend. + if cfg.SLM.Backend != "" && !validSLMBackend(cfg.SLM.Backend) { + findings = append(findings, Finding{ + Severity: SeverityWarn, + Path: path, + Key: "slm.backend", + Message: fmt.Sprintf("invalid slm.backend %q (expected auto, ollama, llamacpp, llamafile, openaicompat, or disabled)", cfg.SLM.Backend), + Suggestion: "fix the value, or remove the line to use the default", + }) + } + + return findings +} + +// detectExplicitZeros surfaces pointer-converted fields whose +// value is *zero (the user explicitly wrote a zero in the +// file) and the default's resolved value is non-zero. These +// are the cases where the user might have a typo (e.g. +// `max_tokens = 0` when they meant 8192) or an explicit +// override. The upgrade-config preserves them as user +// intent; the doctor surfaces them for review. +func (d *Doctor) detectExplicitZeros(path string, cfg *Config, defaults *Config) []Finding { + var findings []Finding + + resolved := cfg.Resolved() + defaultsResolved := defaults.Resolved() + + // Provider.MaxTokens + if cfg.Provider.MaxTokens != nil && *cfg.Provider.MaxTokens == 0 && resolved.Provider.MaxTokens != defaultsResolved.Provider.MaxTokens { + findings = append(findings, Finding{ + Severity: SeverityWarn, + Path: path, + Key: "provider.max_tokens", + Message: fmt.Sprintf("explicit zero for provider.max_tokens (resolved to %d); the default is %d. Is this intentional?", resolved.Provider.MaxTokens, defaultsResolved.Provider.MaxTokens), + }) + } + + // Tools.MaxFileSize + if cfg.Tools.MaxFileSize != nil && *cfg.Tools.MaxFileSize == 0 && resolved.Tools.MaxFileSize != defaultsResolved.Tools.MaxFileSize { + findings = append(findings, Finding{ + Severity: SeverityWarn, + Path: path, + Key: "tools.max_file_size", + Message: fmt.Sprintf("explicit zero for tools.max_file_size (resolved to %d); the default is %d. Zero disables the size cap.", resolved.Tools.MaxFileSize, defaultsResolved.Tools.MaxFileSize), + }) + } + + // Session.MaxKeep + if cfg.Session.MaxKeep != nil && *cfg.Session.MaxKeep == 0 && resolved.Session.MaxKeep != defaultsResolved.Session.MaxKeep { + findings = append(findings, Finding{ + Severity: SeverityWarn, + Path: path, + Key: "session.max_keep", + Message: fmt.Sprintf("explicit zero for session.max_keep (resolved to %d); the default is %d. Zero disables session retention.", resolved.Session.MaxKeep, defaultsResolved.Session.MaxKeep), + }) + } + + return findings +} + +// validPermissionMode returns true if s is a recognized +// permission mode string. Kept as a local function instead of +// importing permission.Mode.Valid() so doctor stays +// independent of the permission package's Type system +// (permission.Mode is a typed string with .Valid() but using +// it would create a coupling we'd rather avoid here). +func validPermissionMode(s string) bool { + switch s { + case "default", "accept_edits", "bypass", "deny", "plan", "auto": + return true + } + return false +} + +// validRouterPrefer returns true if s is a recognized router +// preference. Mirrors the policy table in router.ParsePreferPolicy +// without importing that package (the parser lives in +// internal/router; doctor is in internal/config and the +// layering would invite import cycles if a future router +// subpackage ever imports config). +func validRouterPrefer(s string) bool { + switch s { + case "auto", "local", "cloud": + return true + } + return false +} + +// validSLMBackend returns true if s is a recognized SLM +// backend name. Mirrors the constants in internal/slm +// (auto / ollama / llamacpp / llamafile / openaicompat / +// disabled) without importing that package. +func validSLMBackend(s string) bool { + switch s { + case "auto", "ollama", "llamacpp", "llamafile", "openaicompat", "disabled": + return true + } + return false +} diff --git a/internal/config/doctor_test.go b/internal/config/doctor_test.go new file mode 100644 index 0000000..260bbea --- /dev/null +++ b/internal/config/doctor_test.go @@ -0,0 +1,256 @@ +package config + +import ( + "os" + "path/filepath" + "strings" + "testing" +) + +// TestDiagnose_ValidFileNoFindings sanity-checks the no-op path: +// a freshly-written config (after upgrade-config) produces zero +// findings because every field either matches the default or +// is a legitimate user value. +func TestDiagnose_ValidFileNoFindings(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "config.toml") + if err := os.WriteFile(path, []byte("[provider]\ndefault = \"anthropic\"\n"), 0o644); err != nil { + t.Fatalf("seed: %v", err) + } + + doc := NewDoctor() + fs := doc.DiagnoseFile(path) + for _, f := range fs { + if f.Severity >= SeverityWarn { + t.Errorf("unexpected warn/error finding for valid file: %+v", f) + } + } +} + +// TestDiagnose_MissingFileReturnsErrorFinding verifies the +// error path: a path that doesn't exist produces a single +// SeverityError finding. +func TestDiagnose_MissingFileReturnsErrorFinding(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "nonexistent.toml") + + doc := NewDoctor() + fs := doc.DiagnoseFile(path) + if len(fs) != 1 { + t.Fatalf("len(findings) = %d, want 1", len(fs)) + } + if fs[0].Severity != SeverityError { + t.Errorf("Severity = %v, want SeverityError", fs[0].Severity) + } + if !strings.Contains(fs[0].Message, "read:") { + t.Errorf("Message = %q, want it to mention the read error", fs[0].Message) + } +} + +// TestDiagnose_CorruptFileReturnsErrorFinding verifies the +// parse-error path: a file with invalid TOML produces a +// SeverityError finding with a parse message. +func TestDiagnose_CorruptFileReturnsErrorFinding(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "config.toml") + if err := os.WriteFile(path, []byte("[broken\nthis = 'is not valid"), 0o644); err != nil { + t.Fatalf("seed: %v", err) + } + + doc := NewDoctor() + fs := doc.DiagnoseFile(path) + if len(fs) != 1 { + t.Fatalf("len(findings) = %d, want 1", len(fs)) + } + if fs[0].Severity != SeverityError { + t.Errorf("Severity = %v, want SeverityError", fs[0].Severity) + } + if !strings.Contains(fs[0].Message, "parse:") { + t.Errorf("Message = %q, want it to mention the parse error", fs[0].Message) + } +} + +// TestDiagnose_UnknownTopLevelKeysAreWarned verifies that keys +// in the source file that don't map to any Config field +// surface as SeverityWarn findings. Decoder ignores them +// silently today; doctor surfaces them. +func TestDiagnose_UnknownTopLevelKeysAreWarned(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "config.toml") + if err := os.WriteFile(path, []byte("[unknown_section]\nfoo = 1\n"), 0o644); err != nil { + t.Fatalf("seed: %v", err) + } + + doc := NewDoctor() + fs := doc.DiagnoseFile(path) + found := false + for _, f := range fs { + if f.Severity == SeverityWarn && strings.Contains(f.Key, "unknown_section") { + found = true + break + } + } + if !found { + t.Errorf("expected warning for unknown_section, got %+v", fs) + } +} + +// TestDiagnose_InvalidPermissionModeIsWarned verifies that an +// invalid permission.mode value surfaces as SeverityWarn. +// The mode is a string that must be one of the documented +// permission.Mode constants. +func TestDiagnose_InvalidPermissionModeIsWarned(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "config.toml") + if err := os.WriteFile(path, []byte("[permission]\nmode = \"yes\"\n"), 0o644); err != nil { + t.Fatalf("seed: %v", err) + } + + doc := NewDoctor() + fs := doc.DiagnoseFile(path) + found := false + for _, f := range fs { + if f.Severity == SeverityWarn && f.Key == "permission.mode" { + found = true + if !strings.Contains(f.Message, "yes") { + t.Errorf("Message = %q, want it to mention the invalid value 'yes'", f.Message) + } + } + } + if !found { + t.Errorf("expected warning for invalid permission.mode, got %+v", fs) + } +} + +// TestDiagnose_ValidPermissionModeIsClean verifies the +// "explicit-valid" path: a user-set valid mode produces no +// finding for permission.mode. +func TestDiagnose_ValidPermissionModeIsClean(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "config.toml") + if err := os.WriteFile(path, []byte("[permission]\nmode = \"deny\"\n"), 0o644); err != nil { + t.Fatalf("seed: %v", err) + } + + doc := NewDoctor() + fs := doc.DiagnoseFile(path) + for _, f := range fs { + if f.Key == "permission.mode" { + t.Errorf("unexpected finding for valid mode: %+v", f) + } + } +} + +// TestDiagnose_InvalidRouterPreferIsWarned verifies that an +// invalid router.prefer value surfaces as SeverityWarn. +func TestDiagnose_InvalidRouterPreferIsWarned(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "config.toml") + if err := os.WriteFile(path, []byte("[router]\nprefer = \"yes\"\n"), 0o644); err != nil { + t.Fatalf("seed: %v", err) + } + + doc := NewDoctor() + fs := doc.DiagnoseFile(path) + found := false + for _, f := range fs { + if f.Severity == SeverityWarn && f.Key == "router.prefer" { + found = true + } + } + if !found { + t.Errorf("expected warning for invalid router.prefer, got %+v", fs) + } +} + +// TestDiagnose_ExplicitZeroProviderMaxTokensIsWarned verifies +// the "explicit zero" case the upgrade-config preserves but +// the doctor surfaces: a user-set *int64(0) on a pointer +// field whose default is non-zero is probably a mistake. +// SeverityWarn (not Error) because the user might have set +// it intentionally. +func TestDiagnose_ExplicitZeroProviderMaxTokensIsWarned(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "config.toml") + if err := os.WriteFile(path, []byte("[provider]\nmax_tokens = 0\n"), 0o644); err != nil { + t.Fatalf("seed: %v", err) + } + + doc := NewDoctor() + fs := doc.DiagnoseFile(path) + found := false + for _, f := range fs { + if f.Severity == SeverityWarn && f.Key == "provider.max_tokens" { + found = true + } + } + if !found { + t.Errorf("expected warning for explicit-zero max_tokens, got %+v", fs) + } +} + +// TestDiagnose_DefaultProviderMaxTokensClean documents the +// "user set to default" case: the cleaner drops these, and +// the doctor should NOT warn about them (the user did the +// right thing by setting an explicit value that matches the +// default). +func TestDiagnose_DefaultProviderMaxTokensClean(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "config.toml") + if err := os.WriteFile(path, []byte("[provider]\nmax_tokens = 8192\n"), 0o644); err != nil { + t.Fatalf("seed: %v", err) + } + + doc := NewDoctor() + fs := doc.DiagnoseFile(path) + for _, f := range fs { + if f.Key == "provider.max_tokens" { + t.Errorf("unexpected finding for default-equivalent max_tokens: %+v", f) + } + } +} + +// TestDiagnose_DiagnoseManyAggregates verifies the multi-file +// API: paths is a list of files to scan, the result is the +// concatenation of per-file findings. +func TestDiagnose_DiagnoseManyAggregates(t *testing.T) { + dir := t.TempDir() + good := filepath.Join(dir, "good.toml") + bad := filepath.Join(dir, "bad.toml") + _ = os.WriteFile(good, []byte("[provider]\ndefault = \"anthropic\"\n"), 0o644) + _ = os.WriteFile(bad, []byte("[permission]\nmode = \"yes\"\n"), 0o644) + + doc := NewDoctor() + fs := doc.DiagnoseFiles([]string{good, bad}) + if len(fs) < 1 { + t.Fatalf("len(findings) = %d, want >= 1", len(fs)) + } + // The bad file should contribute at least one finding. + foundBad := false + for _, f := range fs { + if f.Path == bad { + foundBad = true + } + } + if !foundBad { + t.Errorf("expected finding for %s, got %+v", bad, fs) + } +} + +// TestSeverity_String verifies the human-readable form of +// Severity values for the CLI's text output. +func TestSeverity_String(t *testing.T) { + cases := []struct { + sev Severity + want string + }{ + {SeverityInfo, "info"}, + {SeverityWarn, "warn"}, + {SeverityError, "error"}, + } + for _, c := range cases { + if got := c.sev.String(); got != c.want { + t.Errorf("Severity(%d).String() = %q, want %q", c.sev, got, c.want) + } + } +} diff --git a/internal/config/load.go b/internal/config/load.go index 288055c..81a2115 100644 --- a/internal/config/load.go +++ b/internal/config/load.go @@ -96,13 +96,22 @@ func projectConfigPath() string { } // ProjectConfigPath returns the path to the project config file -// (.gnoma/config.toml under the project root). Exported so the -// `gnoma upgrade-config` CLI (and any future callers that need to -// point at the project config) can use it. +// for the current working directory (.gnoma/config.toml under +// the project root). Exported so the `gnoma upgrade-config` CLI +// (and any future callers that need to point at the project +// config) can use it. func ProjectConfigPath() string { return filepath.Join(ProjectRoot(), ".gnoma", "config.toml") } +// ProjectConfigPathFor returns the project config path for an +// arbitrary project root. Used by `gnoma doctor --all-projects` +// to enumerate registry entries without `chdir`-ing into each +// project. +func ProjectConfigPathFor(projectRoot string) string { + return filepath.Join(projectRoot, ".gnoma", "config.toml") +} + func applyEnv(cfg *Config) { envKeys := map[string]string{ "mistral": "MISTRAL_API_KEY",