feat(config): Phase 3 — gnoma doctor diagnostic command
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 <path> 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.
This commit is contained in:
@@ -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
|
||||
@@ -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)
|
||||
}
|
||||
}
|
||||
@@ -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:]))
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -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
|
||||
}
|
||||
@@ -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)
|
||||
}
|
||||
}
|
||||
}
|
||||
+12
-3
@@ -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",
|
||||
|
||||
Reference in New Issue
Block a user