fix: add actionable config and state errors

docs/architecture.md

@@ -149,6 +149,9 @@ Schema 10 preserves current state across starts. Older, future and unrecognized
 schemas are refused; opening a database never resets it. Durable server volumes
 require an explicit reset decision: stop the owner, back up its state directory,
 then select a fresh database path. No automatic schema migration is implemented.
+State errors identify the database and backup directory, name the compatible
+schema, and show the `--state` override for explicitly selecting fresh state.
+Fresh state has no saved reviews or server accounts.
 Upgrade server and collectors together.
 Versioned producer directories ignore old manual/hook pending payloads.
 Stop old binaries before upgrading.
@@ -168,4 +171,7 @@ hooks. Mode is chosen by the command, never by a config field: local commands
 reject remote flags, and sync, hooks and `remote status` fail without a complete
 remote destination. Hooks check that destination before discovery, so an
 unconfigured machine never enrolls identities or collects checkouts. Flat
-pre-namespace config files are rejected with each key's replacement.
+pre-namespace config files are rejected with each key's replacement and a nested
+JSON example. Misplaced keys name their target section and field; invalid values
+include accepted examples. Diagnostics never echo tokens or credential-bearing
+URLs, and rejected files remain unchanged.

internal/config/config.go

@@ -73,9 +73,40 @@ func (values *Values) field(key string) (interface{}, error) {
 		return &values.Remote.Token, nil
 	}
 	if replacement, ok := legacyKeys[key]; ok {
-		return nil, fmt.Errorf("config key %q moved to %q", key, replacement)
+		return nil, fmt.Errorf("config key %q moved to %q; use %s with your existing value", key, replacement, keyExample(replacement))
 	}
-	return nil, fmt.Errorf("unknown config key %q", key)
+	if section, name, ok := strings.Cut(key, "."); ok && (section == "local" || section == "remote") {
+		replacement := legacyKeys[name]
+		if name == "url" {
+			replacement = "remote.url"
+		}
+		if replacement != "" {
+			return nil, fmt.Errorf("unknown config key %q; move its value to %q; use %s with your existing value", key, replacement, keyExample(replacement))
+		}
+	}
+	return nil, fmt.Errorf("unknown config key %q; remove or rename it; valid keys: %s", key, strings.Join(Keys, ", "))
+}
+
+// Examples never interpolate config values: errors may be logged, including
+// when a token or credentials in a URL caused validation to fail.
+func keyExample(key string) string {
+	section, name, _ := strings.Cut(key, ".")
+	value := `""`
+	switch key {
+	case "local.host":
+		value = `"127.0.0.1"`
+	case "local.port":
+		value = "null"
+	case "local.state":
+		value = `"/path/to/state.db"`
+	case "local.retentionDays":
+		value = "7"
+	case "remote.url":
+		value = `"https://reviews.example.com"`
+	case "remote.token":
+		value = `"<existing token>"`
+	}
+	return fmt.Sprintf(`{%q: {%q: %s}}`, section, name, value)
 }
 
 // Get returns one persisted setting by dotted path.
@@ -163,14 +194,14 @@ func LoadFile(path string) (Values, error) {
 	if raw, exists := os.LookupEnv("DIFFX_LOCAL_PORT"); exists {
 		port, err := strconv.Atoi(raw)
 		if err != nil {
-			return values, fmt.Errorf("invalid DIFFX_LOCAL_PORT: %w", err)
+			return values, errors.New("invalid DIFFX_LOCAL_PORT: set a whole number between 0 and 65535, or unset it for automatic port selection")
 		}
 		values.Local.Port = &port
 	}
 	if raw, exists := os.LookupEnv("DIFFX_LOCAL_RETENTION_DAYS"); exists {
 		days, err := strconv.Atoi(raw)
 		if err != nil {
-			return values, fmt.Errorf("invalid DIFFX_LOCAL_RETENTION_DAYS: %w", err)
+			return values, errors.New("invalid DIFFX_LOCAL_RETENTION_DAYS: set a whole number between 1 and 106751; use 7 for one week")
 		}
 		values.Local.RetentionDays = days
 	}
@@ -194,7 +225,7 @@ func load(path string) (Values, error) {
 	}
 	values, err = Merge(values, string(raw))
 	if err != nil {
-		return values, fmt.Errorf("config file %s: %w; edit it or delete it to recreate defaults", path, err)
+		return values, fmt.Errorf("config file %s: %w\n  Edit this file, then retry. Omitted keys use defaults.", path, err)
 	}
 	return values, nil
 }
@@ -203,13 +234,13 @@ func decodeObject(raw []byte, name string) (map[string]json.RawMessage, error) {
 	var fields map[string]json.RawMessage
 	decoder := json.NewDecoder(bytes.NewReader(raw))
 	if err := decoder.Decode(&fields); err != nil {
-		return nil, fmt.Errorf("invalid %s JSON: %w", name, err)
+		return nil, fmt.Errorf("invalid %s JSON: %w; use a JSON object with double-quoted keys and no trailing commas", name, err)
 	}
 	if fields == nil {
-		return nil, fmt.Errorf("%s must be a JSON object", name)
+		return nil, fmt.Errorf("%s must be a JSON object; use {} for defaults", name)
 	}
 	if decoder.Decode(&struct{}{}) != io.EOF {
-		return nil, fmt.Errorf("%s must contain one JSON object", name)
+		return nil, fmt.Errorf("%s must contain one JSON object; combine settings inside a single pair of braces", name)
 	}
 	return fields, nil
 }
@@ -227,27 +258,33 @@ func Merge(values Values, raw string) (Values, error) {
 	}
 	if len(moves) > 0 {
 		sort.Strings(moves)
-		return values, fmt.Errorf(`uses the retired flat format; move each key into its "local" or "remote" section (%s)`, strings.Join(moves, ", "))
+		return values, fmt.Errorf(`uses the retired flat format; move each key into its "local" or "remote" section (%s).
+  Use nested JSON, keeping your existing values (omit settings you do not need):
+  {
+    "local": {"host": "127.0.0.1", "port": null, "state": "/path/to/state.db", "webDir": "", "retentionDays": 7},
+    "remote": {"url": "https://reviews.example.com", "token": "<existing token>"}
+  }
+  "server" becomes "url" inside "remote"; "token" also belongs inside "remote".`, strings.Join(moves, ", "))
 	}
 	for section, body := range sections {
 		if section != "local" && section != "remote" {
-			return values, fmt.Errorf(`unknown config section %q; expected "local" and "remote"`, section)
+			return values, fmt.Errorf(`unknown config section %q; remove or rename it; expected "local" and "remote", like {"local": {"port": null}, "remote": {"url": "https://reviews.example.com"}}`, section)
 		}
 		var fields map[string]json.RawMessage
 		if err := json.Unmarshal(body, &fields); err != nil || fields == nil {
-			return values, fmt.Errorf("%s must be a JSON object, like {%q: {...}}", section, section)
+			return values, fmt.Errorf("%s must be a JSON object; use {%q: {}} for defaults, then put settings inside that object", section, section)
 		}
 		for name, field := range fields {
 			key := section + "." + name
 			target, err := values.field(key)
 			if err != nil {
-				return values, fmt.Errorf("unknown config key %q; valid keys: %s", key, strings.Join(Keys, ", "))
+				return values, err
 			}
 			if string(field) == "null" && key != "local.port" {
-				return values, fmt.Errorf("%s cannot be null", key)
+				return values, fmt.Errorf("%s cannot be null; omit it to use the default, or use %s", key, keyExample(key))
 			}
 			if err := json.Unmarshal(field, target); err != nil {
-				return values, fmt.Errorf("%s must be %s", key, expectedType(target))
+				return values, fmt.Errorf("%s must be %s; use %s", key, expectedType(target), keyExample(key))
 			}
 		}
 	}
@@ -271,21 +308,21 @@ func expectedType(target interface{}) string {
 func (values Values) Validate() error {
 	local := values.Local
 	if net.ParseIP(local.Host) == nil {
-		return errors.New("local.host must be an IP address")
+		return errors.New(`local.host must be an IP address; use "127.0.0.1" for this machine or "0.0.0.0" to listen on all IPv4 interfaces`)
 	}
 	if local.Port != nil && (*local.Port < 0 || *local.Port > 65535) {
-		return errors.New("local.port must be null (automatic) or between 0 and 65535")
+		return errors.New("local.port must be null (automatic) or between 0 and 65535; use 0 for an ephemeral port, or omit the setting for automatic selection")
 	}
 	if local.State == "" {
-		return errors.New("local.state must be a database path or memory")
+		return errors.New(`local.state must be a database path or memory; omit it to use the default persistent database, or use "memory" for temporary state`)
 	}
 	if local.RetentionDays <= 0 || local.RetentionDays > 106751 {
-		return errors.New("local.retentionDays must be between 1 and 106751")
+		return errors.New("local.retentionDays must be between 1 and 106751; use 7 for one week")
 	}
 	if values.Remote.URL != "" {
 		endpoint, err := url.Parse(values.Remote.URL)
 		if err != nil || endpoint.Host == "" || (endpoint.Scheme != "http" && endpoint.Scheme != "https") || endpoint.User != nil || endpoint.RawQuery != "" || endpoint.Fragment != "" || (endpoint.Path != "" && endpoint.Path != "/") {
-			return errors.New("remote.url must be an HTTP(S) origin without credentials, path, query, or fragment")
+			return errors.New(`remote.url must be an HTTP(S) origin without credentials, path, query, or fragment; use "https://reviews.example.com" and put the API token in remote.token`)
 		}
 	}
 	return nil

internal/config/config_test.go

@@ -1,6 +1,7 @@
 package config
 
 import (
+	"encoding/json"
 	"os"
 	"path/filepath"
 	"runtime"
@@ -257,8 +258,19 @@ func TestInvalidConfigNeverChangesSavedSettings(t *testing.T) {
 func TestMalformedConfigFileErrorsNameFileAndFix(t *testing.T) {
 	for raw, want := range map[string][]string{
 		`{"host":"127.0.0.1","port":null,"server":"","token":""}`: {"retired flat format", "host → local.host", "port → local.port", "server → remote.url", "token → remote.token"},
+		`{"local":{"server":"https://example.com"}}`:              {`unknown config key "local.server"`, `move its value to "remote.url"`, `{"remote": {"url": "https://reviews.example.com"}}`},
+		`{"remote":{"server":"https://example.com"}}`:             {`move its value to "remote.url"`},
+		`{"local":{"token":"secret-token"}}`:                      {`move its value to "remote.token"`, `{"remote": {"token": "<existing token>"}}`},
+		`{"local":{"url":"https://example.com"}}`:                 {`move its value to "remote.url"`},
+		`{"remote":{"port":4000}}`:                                {`move its value to "local.port"`},
 		`{"local":{"prot":4000}}`:                                 {`unknown config key "local.prot"`, "local.port"},
-		`{"local":{"port":"4000"}}`:                               {"local.port must be a whole number or null"},
+		`{"local":{"port":"4000"}}`:                               {"local.port must be a whole number or null", `{"local": {"port": null}}`},
+		`{"local":{"port":65536}}`:                                {"use 0 for an ephemeral port"},
+		`{"local":{"host":null}}`:                                 {"local.host cannot be null", "omit it to use the default", `{"local": {"host": "127.0.0.1"}}`},
+		`{"local":{"host":"localhost"}}`:                          {"local.host must be an IP address", `use "127.0.0.1"`},
+		`{"local":{"state":""}}`:                                  {"omit it to use the default persistent database"},
+		`{"local":{"retentionDays":0}}`:                           {"use 7 for one week"},
+		`{"remote":{"url":"https://user:password@example.com"}}`:  {`use "https://reviews.example.com"`, "remote.token"},
 		`{"remote":"https://example.com"}`:                        {"remote must be a JSON object"},
 		`{"servers":{}}`:                                          {`unknown config section "servers"`},
 		`{"local":`:                                               {"invalid config JSON"},
@@ -271,10 +283,50 @@ func TestMalformedConfigFileErrorsNameFileAndFix(t *testing.T) {
 		if err == nil {
 			t.Fatalf("accepted %s", raw)
 		}
-		for _, text := range append(want, path, "delete it to recreate defaults") {
+		for _, text := range append(want, path, "Edit this file, then retry") {
 			if !strings.Contains(err.Error(), text) {
 				t.Fatalf("%s: error %q lacks %q", raw, err, text)
 			}
 		}
+		for _, secret := range []string{"secret-token", "user:password"} {
+			if strings.Contains(err.Error(), secret) {
+				t.Fatalf("error exposes credentials: %v", err)
+			}
+		}
+		after, err := os.ReadFile(path)
+		if err != nil || string(after) != raw {
+			t.Fatalf("rejected config changed: %q, %v", after, err)
+		}
+	}
+}
+
+func TestFlatConfigErrorProvidesValidNestedExample(t *testing.T) {
+	values, err := Default()
+	if err != nil {
+		t.Fatal(err)
+	}
+	_, err = Merge(values, `{"host":"127.0.0.1","server":"https://user:password@example.com","token":"secret-token"}`)
+	if err == nil {
+		t.Fatal("accepted flat config")
+	}
+	message := err.Error()
+	start := strings.Index(message, "{")
+	end := strings.LastIndex(message, "}")
+	if start < 0 || end < start {
+		t.Fatalf("missing JSON example: %s", message)
+	}
+	example := message[start : end+1]
+	var document Values
+	if err := json.Unmarshal([]byte(example), &document); err != nil {
+		t.Fatalf("invalid JSON example: %v", err)
+	}
+	if _, err := Merge(values, example); err != nil {
+		t.Fatalf("example rejected by config loader: %v", err)
+	}
+	if document.Local.Host != "127.0.0.1" || document.Remote.URL == "" || document.Remote.Token == "" {
+		t.Fatalf("example missing local/remote structure: %+v", document)
+	}
+	if strings.Contains(message, "secret-token") || strings.Contains(message, "user:password") {
+		t.Fatalf("error exposes credentials: %s", message)
 	}
 }

internal/reviewstore/catalog_test.go

@@ -4,6 +4,7 @@ import (
 	"errors"
 	"os"
 	"path/filepath"
+	"strings"
 	"testing"
 
 	"github.com/flexdinesh/diffx/internal/processlock"
@@ -36,6 +37,8 @@ func TestUnsupportedLegacyFilePreserved(t *testing.T) {
 	}
 	if _, err := Open(path); err == nil {
 		t.Fatal("unsupported legacy state opened")
+	} else if !strings.Contains(err.Error(), path) || !strings.Contains(err.Error(), "--state /path/to/new-state.db") {
+		t.Fatalf("missing state path or recovery guidance: %v", err)
 	}
 	preserved, err := os.ReadFile(path)
 	if err != nil || string(preserved) != string(raw) {

internal/reviewstore/reset_test.go

@@ -13,7 +13,7 @@ func TestOlderSchemaRefusedWithoutReset(t *testing.T) {
 	if err != nil {
 		t.Fatal(err)
 	}
-	_, err = db.Exec("CREATE TABLE legacy(value TEXT); INSERT INTO legacy VALUES('retain'); PRAGMA user_version=8")
+	_, err = db.Exec("CREATE TABLE legacy(value TEXT); INSERT INTO legacy VALUES('retain'); PRAGMA user_version=9")
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -22,8 +22,12 @@ func TestOlderSchemaRefusedWithoutReset(t *testing.T) {
 		if store, err := Open(path); err == nil {
 			store.Close()
 			t.Fatal("accepted older schema")
-		} else if !strings.Contains(err.Error(), "automatic reset is disabled") {
-			t.Fatalf("missing recovery guidance: %v", err)
+		} else {
+			for _, text := range []string{path, "schema 9 is older than supported schema 10", "supporting schema 9", "automatic reset is disabled", "back up", "SQLite sidecar files", "--state /path/to/new-state.db", "no saved reviews or server accounts"} {
+				if !strings.Contains(err.Error(), text) {
+					t.Fatalf("missing recovery guidance %q: %v", text, err)
+				}
+			}
 		}
 	}
 	db, err = sql.Open("sqlite", path)
@@ -36,7 +40,7 @@ func TestOlderSchemaRefusedWithoutReset(t *testing.T) {
 		t.Fatalf("older data changed: %q %v", value, err)
 	}
 	var version int
-	if err := db.QueryRow("PRAGMA user_version").Scan(&version); err != nil || version != 8 {
+	if err := db.QueryRow("PRAGMA user_version").Scan(&version); err != nil || version != 9 {
 		t.Fatalf("older schema version changed: %d %v", version, err)
 	}
 }
@@ -67,6 +71,12 @@ func TestCurrentSchemaSurvivesRestartAndFutureVersionRefused(t *testing.T) {
 	if newer, err := Open(path); err == nil {
 		newer.Close()
 		t.Fatal("accepted future schema")
+	} else {
+		for _, text := range []string{path, "supporting schema 999", "--state /path/to/new-state.db"} {
+			if !strings.Contains(err.Error(), text) {
+				t.Fatalf("missing recovery guidance %q: %v", text, err)
+			}
+		}
 	}
 	db, err := sql.Open("sqlite", path)
 	if err != nil {
@@ -77,4 +87,35 @@ func TestCurrentSchemaSurvivesRestartAndFutureVersionRefused(t *testing.T) {
 	if err := db.QueryRow("SELECT COUNT(*) FROM users").Scan(&count); err != nil || count != 1 {
 		t.Fatalf("future state changed: %d %v", count, err)
 	}
+	var version int
+	if err := db.QueryRow("PRAGMA user_version").Scan(&version); err != nil || version != 999 {
+		t.Fatalf("future schema version changed: %d %v", version, err)
+	}
+}
+
+func TestUnversionedDatabaseRefusedWithRecoveryGuidance(t *testing.T) {
+	path := filepath.Join(t.TempDir(), "state.db")
+	db, err := sql.Open("sqlite", path)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if _, err := db.Exec("CREATE TABLE unrelated(value TEXT); INSERT INTO unrelated VALUES('retain')"); err != nil {
+		t.Fatal(err)
+	}
+	db.Close()
+	if store, err := Open(path); err == nil {
+		store.Close()
+		t.Fatal("accepted unversioned database")
+	} else if !strings.Contains(err.Error(), path) || !strings.Contains(err.Error(), "--state /path/to/new-state.db") {
+		t.Fatalf("missing recovery guidance: %v", err)
+	}
+	db, err = sql.Open("sqlite", path)
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer db.Close()
+	var value string
+	if err := db.QueryRow("SELECT value FROM unrelated").Scan(&value); err != nil || value != "retain" {
+		t.Fatalf("unversioned data changed: %q %v", value, err)
+	}
 }

internal/reviewstore/store.go

@@ -106,7 +106,7 @@ func OpenWithRetention(path string, retention time.Duration) (*Store, error) {
 	db.SetMaxOpenConns(1)
 	store := &Store{db: db, lock: lock, retention: retention}
 	if _, err = db.Exec("PRAGMA foreign_keys = ON; PRAGMA busy_timeout = 5000"); err == nil {
-		err = store.initialize()
+		err = store.initialize(name)
 	}
 	if err != nil {
 		_ = db.Close()
@@ -139,19 +139,23 @@ func validateStateFile(path string) error {
 	if count == 0 || string(header) == "SQLite format 3\x00" {
 		return nil
 	}
-	return errors.New("unrecognized or unsupported diffx state file; existing data preserved")
+	return stateRecoveryError(path, "unrecognized or unsupported diffx state file; check that --state selects the intended SQLite database")
 }
 
-func (store *Store) initialize() error {
+func stateRecoveryError(path, reason string) error {
+	return fmt.Errorf("state database %q: %s\n  Existing data preserved; automatic reset is disabled.\n  To start fresh: stop processes using this database, back up %q (including SQLite sidecar files), then retry your command with --state /path/to/new-state.db (choose an unused path).\n  Fresh state has no saved reviews or server accounts.", path, reason, filepath.Dir(path))
+}
+
+func (store *Store) initialize(path string) error {
 	var version int
 	if err := store.db.QueryRow("PRAGMA user_version").Scan(&version); err != nil {
 		return err
 	}
 	if version > schemaVersion {
-		return fmt.Errorf("diffx state schema %d is newer than supported schema %d", version, schemaVersion)
+		return stateRecoveryError(path, fmt.Sprintf("diffx state schema %d is newer than supported schema %d; to keep existing data, use a diffx binary supporting schema %d", version, schemaVersion, version))
 	}
 	if version > 0 && version < schemaVersion {
-		return fmt.Errorf("diffx state schema %d is older than supported schema %d; automatic reset is disabled: use a compatible binary or back up the state directory and explicitly choose a fresh database path", version, schemaVersion)
+		return stateRecoveryError(path, fmt.Sprintf("diffx state schema %d is older than supported schema %d; to keep existing data, use a diffx binary supporting schema %d", version, schemaVersion, version))
 	}
 	if _, err := store.db.Exec("PRAGMA foreign_keys=OFF"); err != nil {
 		return err
@@ -166,7 +170,7 @@ func (store *Store) initialize() error {
 		var existing string
 		err := transaction.QueryRow(`SELECT name FROM sqlite_master WHERE type='table' AND name NOT LIKE 'sqlite_%' LIMIT 1`).Scan(&existing)
 		if err == nil {
-			return errors.New("unversioned state database")
+			return stateRecoveryError(path, "unversioned state database; check that --state selects the intended diffx database")
 		}
 		if !errors.Is(err, sql.ErrNoRows) {
 			return err