fix(config): show precise malformed config diagnostics (#2415)

* fix(config): show precise malformed config diagnostics

* fix lint

* fix test
This commit is contained in:
Mauro
2026-04-27 03:45:52 +02:00
committed by GitHub
parent 39dec35408
commit ed687d62ae
6 changed files with 619 additions and 34 deletions
+47 -1
View File
@@ -996,7 +996,9 @@ func LoadConfig(path string) (*Config, error) {
Version int `json:"version"`
}
if e := json.Unmarshal(data, &versionInfo); e != nil {
return nil, fmt.Errorf("failed to detect config version: %w", e)
e = wrapJSONError(data, e, "config.json")
logger.ErrorCF("config", formatDiagnosticLogMessage("Malformed config file", e), map[string]any{"path": path})
return nil, e
}
if len(data) <= 10 {
logger.Warn(fmt.Sprintf("content is [%s]", string(data)))
@@ -1011,10 +1013,23 @@ func LoadConfig(path string) (*Config, error) {
"config migrate start",
map[string]any{"from": versionInfo.Version, "to": CurrentVersion},
)
if err = validateLegacyConfigDiagnostics(data); err != nil {
logger.ErrorCF(
"config",
formatDiagnosticLogMessage("Failed to load config", err),
map[string]any{"path": path},
)
return nil, err
}
var m map[string]any
m, err = loadConfigMap(path)
if err != nil {
logger.ErrorCF(
"config",
formatDiagnosticLogMessage("Failed to load config", err),
map[string]any{"path": path},
)
return nil, err
}
@@ -1056,10 +1071,23 @@ func LoadConfig(path string) (*Config, error) {
"config migrate start",
map[string]any{"from": versionInfo.Version, "to": CurrentVersion},
)
if err = validateLegacyConfigDiagnostics(data); err != nil {
logger.ErrorCF(
"config",
formatDiagnosticLogMessage("Failed to load config", err),
map[string]any{"path": path},
)
return nil, err
}
var m map[string]any
m, err = loadConfigMap(path)
if err != nil {
logger.ErrorCF(
"config",
formatDiagnosticLogMessage("Failed to load config", err),
map[string]any{"path": path},
)
return nil, err
}
@@ -1101,9 +1129,22 @@ func LoadConfig(path string) (*Config, error) {
"config migrate start",
map[string]any{"from": versionInfo.Version, "to": CurrentVersion},
)
if err = validateLegacyConfigDiagnostics(data); err != nil {
logger.ErrorCF(
"config",
formatDiagnosticLogMessage("Failed to load config", err),
map[string]any{"path": path},
)
return nil, err
}
var m map[string]any
m, err = loadConfigMap(path)
if err != nil {
logger.ErrorCF(
"config",
formatDiagnosticLogMessage("Failed to load config", err),
map[string]any{"path": path},
)
return nil, err
}
migrateErr := migrateV2ToV3(m)
@@ -1138,6 +1179,11 @@ func LoadConfig(path string) (*Config, error) {
// Current version
cfg, err = loadConfig(data)
if err != nil {
logger.ErrorCF(
"config",
formatDiagnosticLogMessage("Failed to load config", err),
map[string]any{"path": path},
)
return nil, err
}
// Load security configuration
+68 -15
View File
@@ -847,6 +847,72 @@ func TestLoadConfig_WebPreferNativeCanBeDisabled(t *testing.T) {
}
}
func TestLoadConfig_SyntaxErrorReportsLineAndColumn(t *testing.T) {
dir := t.TempDir()
configPath := filepath.Join(dir, "config.json")
raw := "{\n \"version\": 2,\n \"tools\": {\n \"web\": {\n \"enabled\": true,,\n \"format\": \"markdown\"\n }\n }\n}\n"
if err := os.WriteFile(configPath, []byte(raw), 0o600); err != nil {
t.Fatalf("WriteFile() error: %v", err)
}
_, err := LoadConfig(configPath)
if err == nil {
t.Fatal("expected syntax error, got nil")
}
if !strings.Contains(err.Error(), "syntax error at line 5, column 23") {
t.Fatalf("expected line/column diagnostic, got %q", err.Error())
}
if !strings.Contains(err.Error(), "\"enabled\": true,,") {
t.Fatalf("expected source snippet in diagnostic, got %q", err.Error())
}
if !strings.Contains(err.Error(), "^") {
t.Fatalf("expected caret marker in diagnostic, got %q", err.Error())
}
}
func TestLoadConfig_TypeErrorReportsFieldPath(t *testing.T) {
dir := t.TempDir()
configPath := filepath.Join(dir, "config.json")
raw := "{\n \"version\": 2,\n \"tools\": {\n \"web\": {\n \"fetch_limit_bytes\": \"oops\"\n }\n }\n}\n"
if err := os.WriteFile(configPath, []byte(raw), 0o600); err != nil {
t.Fatalf("WriteFile() error: %v", err)
}
_, err := LoadConfig(configPath)
if err == nil {
t.Fatal("expected type error, got nil")
}
if !strings.Contains(err.Error(), "type error at line 5, column 33") {
t.Fatalf("expected line/column diagnostic, got %q", err.Error())
}
if !strings.Contains(err.Error(), "fetch_limit_bytes") {
t.Fatalf("expected field name in diagnostic, got %q", err.Error())
}
if !strings.Contains(err.Error(), "\"fetch_limit_bytes\": \"oops\"") {
t.Fatalf("expected source snippet in diagnostic, got %q", err.Error())
}
if !strings.Contains(err.Error(), "^") {
t.Fatalf("expected caret marker in diagnostic, got %q", err.Error())
}
}
func TestLoadConfig_UnknownFieldsReportsExactPaths(t *testing.T) {
dir := t.TempDir()
configPath := filepath.Join(dir, "config.json")
raw := "{\n \"version\": 2,\n \"tools\": {\n \"weeb\": {\n \"enabled\": true\n },\n \"web\": {\n \"fatch_limit_bytes\": 123\n }\n }\n}\n"
if err := os.WriteFile(configPath, []byte(raw), 0o600); err != nil {
t.Fatalf("WriteFile() error: %v", err)
}
_, err := LoadConfig(configPath)
if err == nil {
t.Fatal("expected unknown field error, got nil")
}
if !strings.Contains(err.Error(), "tools.weeb") || !strings.Contains(err.Error(), "tools.web.fatch_limit_bytes") {
t.Fatalf("expected exact unknown field paths, got %q", err.Error())
}
}
func TestDefaultConfig_ExecAllowRemoteEnabled(t *testing.T) {
cfg := DefaultConfig()
if !cfg.Tools.Exec.AllowRemote {
@@ -1355,25 +1421,12 @@ func TestLoadConfig_TelegramPlaceholderTextAcceptsSingleString(t *testing.T) {
}
// TestLoadConfig_WarnsForPlaintextAPIKey verifies that LoadConfig resolves a plaintext
// api_key into memory but does NOT rewrite the config file. File writes are the sole
// api_keys entry into memory but does NOT rewrite the config file. File writes are the sole
// responsibility of SaveConfig.
func TestLoadConfig_WarnsForPlaintextAPIKey(t *testing.T) {
dir := t.TempDir()
cfgPath := filepath.Join(dir, "config.json")
const original = `{"version":1,"model_list":[{"model_name":"test","model":"openai/gpt-4","api_key":"sk-plaintext"}]}`
if err := os.WriteFile(cfgPath, []byte(original), 0o600); err != nil {
t.Fatalf("setup: %v", err)
}
secPath := filepath.Join(dir, SecurityConfigFile)
const securityConfig = `
model_list:
test:0:
api_keys:
- "sk-plaintext"
`
if err := os.WriteFile(secPath, []byte(securityConfig), 0o600); err != nil {
t.Fatalf("setup: %v", err)
}
const original = `{"version":2,"model_list":[{"model_name":"test","model":"openai/gpt-4","api_keys":["sk-plaintext"]}]}`
if err := os.WriteFile(cfgPath, []byte(original), 0o600); err != nil {
t.Fatalf("setup: %v", err)
}
+441
View File
@@ -0,0 +1,441 @@
package config
import (
"encoding/json"
"fmt"
"os"
"reflect"
"sort"
"strings"
"unicode/utf8"
"golang.org/x/term"
)
func decodeJSONWithDiagnostics(data []byte, target any, label string) error {
var raw any
if err := json.Unmarshal(data, &raw); err != nil {
return wrapJSONError(data, err, label)
}
unknownFields := collectUnknownJSONFields(raw, reflect.TypeOf(target), "")
if len(unknownFields) > 0 {
sort.Strings(unknownFields)
return fmt.Errorf(
"%s contains unknown field(s): %s",
label,
strings.Join(unknownFields, ", "),
)
}
if err := json.Unmarshal(data, target); err != nil {
return wrapJSONError(data, err, label)
}
return nil
}
func DiagnosticSummary(err error) string {
if err == nil {
return ""
}
summary, _ := splitDiagnosticError(err.Error())
return stripANSISequences(summary)
}
func formatDiagnosticLogMessage(prefix string, err error) string {
if err == nil {
return prefix
}
summary, preview := splitDiagnosticError(err.Error())
summary = stripANSISequences(summary)
if preview == "" {
if summary == "" {
return prefix
}
return prefix + ": " + summary
}
if summary == "" {
return prefix + "\n" + preview
}
return prefix + ": " + summary + "\n" + preview
}
func wrapJSONError(data []byte, err error, label string) error {
switch e := err.(type) {
case *json.SyntaxError:
line, column := lineAndColumnForOffset(data, e.Offset)
preview := diagnosticPreviewForOffset(data, e.Offset)
if preview != "" {
return fmt.Errorf(
"%s syntax error at line %d, column %d: %w\n%s",
label,
line,
column,
err,
preview,
)
}
return fmt.Errorf("%s syntax error at line %d, column %d: %w", label, line, column, err)
case *json.UnmarshalTypeError:
line, column := lineAndColumnForOffset(data, e.Offset)
preview := diagnosticPreviewForOffset(data, e.Offset)
field := strings.TrimSpace(e.Field)
if field != "" {
if preview != "" {
return fmt.Errorf(
"%s type error at line %d, column %d for field %q: expected %s but got %s\n%s",
label,
line,
column,
field,
e.Type.String(),
e.Value,
preview,
)
}
return fmt.Errorf(
"%s type error at line %d, column %d for field %q: expected %s but got %s",
label,
line,
column,
field,
e.Type.String(),
e.Value,
)
}
if preview != "" {
return fmt.Errorf(
"%s type error at line %d, column %d: expected %s but got %s\n%s",
label,
line,
column,
e.Type.String(),
e.Value,
preview,
)
}
return fmt.Errorf(
"%s type error at line %d, column %d: expected %s but got %s",
label,
line,
column,
e.Type.String(),
e.Value,
)
default:
return fmt.Errorf("failed to parse %s: %w", label, err)
}
}
func splitDiagnosticError(message string) (string, string) {
if idx := strings.IndexByte(message, '\n'); idx >= 0 {
return message[:idx], message[idx+1:]
}
return message, ""
}
func stripANSISequences(s string) string {
if s == "" {
return ""
}
var b strings.Builder
b.Grow(len(s))
for i := 0; i < len(s); i++ {
if s[i] != 0x1b {
b.WriteByte(s[i])
continue
}
if i+1 >= len(s) || s[i+1] != '[' {
continue
}
i += 2
for i < len(s) {
c := s[i]
if c >= '@' && c <= '~' {
break
}
i++
}
}
return b.String()
}
func diagnosticPreviewForOffset(data []byte, offset int64) string {
if len(data) == 0 {
return ""
}
start, end := lineBoundsForOffset(data, offset)
if start >= end {
return ""
}
lineNumber, column := lineAndColumnForOffset(data, offset)
line := strings.TrimRight(string(data[start:end]), "\r\n")
if strings.TrimSpace(line) == "" {
return ""
}
trimmedLine, trimOffset := trimDiagnosticLine(line, column)
if trimmedLine == "" {
return ""
}
prefix := fmt.Sprintf("%4d | ", lineNumber)
caretColumn := column - trimOffset
if caretColumn < 1 {
caretColumn = 1
}
if diagnosticsUseColor() {
linePrefix := "\x1b[2m" + prefix + "\x1b[0m"
caretPrefix := "\x1b[2m" + strings.Repeat(" ", len(fmt.Sprintf("%4d", lineNumber))) + " | " + "\x1b[0m"
highlighted := highlightDiagnosticColumn(trimmedLine, caretColumn)
caretPad := strings.Repeat(" ", maxRuneCount(trimmedLine, caretColumn-1))
return fmt.Sprintf(
" %s%s\n %s%s\x1b[1;31m^\x1b[0m",
linePrefix,
highlighted,
caretPrefix,
caretPad,
)
}
caretPrefix := strings.Repeat(" ", len(prefix))
caretPad := strings.Repeat(" ", maxRuneCount(trimmedLine, caretColumn-1))
return fmt.Sprintf(
" %s%s\n %s%s^",
prefix,
trimmedLine,
caretPrefix,
caretPad,
)
}
func lineAndColumnForOffset(data []byte, offset int64) (int, int) {
if offset <= 0 {
return 1, 1
}
if offset > int64(len(data)) {
offset = int64(len(data))
}
line := 1
column := 1
for i := int64(0); i < offset-1; i++ {
if data[i] == '\n' {
line++
column = 1
continue
}
column++
}
return line, column
}
func lineBoundsForOffset(data []byte, offset int64) (int, int) {
if len(data) == 0 {
return 0, 0
}
if offset <= 0 {
offset = 1
}
if offset > int64(len(data)) {
offset = int64(len(data))
}
index := int(offset - 1)
if index < 0 {
index = 0
}
if index >= len(data) {
index = len(data) - 1
}
start := index
for start > 0 && data[start-1] != '\n' {
start--
}
end := index
for end < len(data) && data[end] != '\n' {
end++
}
return start, end
}
func trimDiagnosticLine(line string, column int) (string, int) {
runes := []rune(line)
if len(runes) == 0 {
return "", 0
}
if len(runes) <= 160 {
return line, 0
}
const contextBefore = 60
const maxWidth = 160
start := column - 1 - contextBefore
if start < 0 {
start = 0
}
if start > len(runes)-maxWidth {
start = len(runes) - maxWidth
}
if start < 0 {
start = 0
}
end := start + maxWidth
if end > len(runes) {
end = len(runes)
}
trimmed := string(runes[start:end])
trimOffset := start
if start > 0 {
trimmed = "..." + trimmed
trimOffset -= 3
}
if end < len(runes) {
trimmed += "..."
}
return trimmed, trimOffset
}
func diagnosticsUseColor() bool {
return term.IsTerminal(int(os.Stdout.Fd()))
}
func highlightDiagnosticColumn(line string, column int) string {
runes := []rune(line)
if column < 1 || column > len(runes) {
return line
}
index := column - 1
return string(runes[:index]) + "\x1b[31m" + string(runes[index]) + "\x1b[0m" + string(runes[index+1:])
}
func maxRuneCount(s string, count int) int {
if count <= 0 {
return 0
}
runes := []rune(s)
if count > len(runes) {
count = len(runes)
}
return utf8.RuneCountInString(string(runes[:count]))
}
func collectUnknownJSONFields(raw any, targetType reflect.Type, path string) []string {
targetType = derefType(targetType)
if targetType == nil {
return nil
}
switch targetType.Kind() {
case reflect.Struct:
obj, ok := raw.(map[string]any)
if !ok {
return nil
}
fieldMap := jsonFieldTypeMap(targetType)
var issues []string
for key, value := range obj {
fieldType, exists := fieldMap[key]
fieldPath := appendJSONPath(path, key)
if !exists {
issues = append(issues, fieldPath)
continue
}
issues = append(issues, collectUnknownJSONFields(value, fieldType, fieldPath)...)
}
return issues
case reflect.Slice, reflect.Array:
items, ok := raw.([]any)
if !ok {
return nil
}
var issues []string
elemType := targetType.Elem()
for i, item := range items {
itemPath := fmt.Sprintf("%s[%d]", path, i)
issues = append(issues, collectUnknownJSONFields(item, elemType, itemPath)...)
}
return issues
case reflect.Map:
obj, ok := raw.(map[string]any)
if !ok {
return nil
}
var issues []string
elemType := targetType.Elem()
for key, value := range obj {
fieldPath := appendJSONPath(path, key)
issues = append(issues, collectUnknownJSONFields(value, elemType, fieldPath)...)
}
return issues
default:
return nil
}
}
func jsonFieldTypeMap(t reflect.Type) map[string]reflect.Type {
result := make(map[string]reflect.Type)
populateJSONFieldTypeMap(result, derefType(t))
return result
}
func populateJSONFieldTypeMap(result map[string]reflect.Type, t reflect.Type) {
if t == nil || t.Kind() != reflect.Struct {
return
}
for i := 0; i < t.NumField(); i++ {
field := t.Field(i)
if !field.IsExported() {
continue
}
tag := field.Tag.Get("json")
name := strings.Split(tag, ",")[0]
if name == "-" {
continue
}
if field.Anonymous && name == "" {
populateJSONFieldTypeMap(result, derefType(field.Type))
continue
}
if name == "" {
name = field.Name
}
result[name] = field.Type
}
}
func derefType(t reflect.Type) reflect.Type {
for t != nil && t.Kind() == reflect.Pointer {
t = t.Elem()
}
return t
}
func appendJSONPath(path, segment string) string {
if path == "" {
return segment
}
return path + "." + segment
}
+60 -14
View File
@@ -27,6 +27,59 @@ func buildModelWithProtocol(protocol, model string) string {
return protocol + "/" + model
}
type legacyDiagnosticConfig struct {
Version int `json:"version"`
Isolation IsolationConfig `json:"isolation,omitempty"`
Agents legacyDiagnosticAgents `json:"agents,omitempty"`
Session SessionConfig `json:"session,omitempty"`
Channels map[string]any `json:"channels,omitempty"`
ChannelList ChannelsConfig `json:"channel_list,omitempty"`
ModelList []map[string]any `json:"model_list,omitempty"`
Gateway GatewayConfig `json:"gateway,omitempty"`
Hooks HooksConfig `json:"hooks,omitempty"`
Tools ToolsConfig `json:"tools,omitempty"`
Heartbeat HeartbeatConfig `json:"heartbeat,omitempty"`
Devices DevicesConfig `json:"devices,omitempty"`
Voice VoiceConfig `json:"voice,omitempty"`
Bindings json.RawMessage `json:"bindings,omitempty"`
Providers json.RawMessage `json:"providers,omitempty"`
}
type legacyDiagnosticAgents struct {
Defaults legacyDiagnosticAgentDefaults `json:"defaults,omitempty"`
List []AgentConfig `json:"list,omitempty"`
Dispatch *DispatchConfig `json:"dispatch,omitempty"`
}
type legacyDiagnosticAgentDefaults struct {
AgentDefaults
LegacyModel string `json:"model,omitempty"`
}
func validateLegacyConfigDiagnostics(data []byte) error {
var cfg legacyDiagnosticConfig
return decodeJSONWithDiagnostics(data, &cfg, "config.json")
}
func migrateLegacyAgentDefaultsModel(m map[string]any) {
agents, ok := m["agents"].(map[string]any)
if !ok {
return
}
defaults, ok := agents["defaults"].(map[string]any)
if !ok {
return
}
model, hasModel := defaults["model"]
if !hasModel {
return
}
if _, hasModelName := defaults["model_name"]; !hasModelName {
defaults["model_name"] = model
}
delete(defaults, "model")
}
// loadConfigV1 loads a version 1 config (current schema)
func loadConfig(data []byte) (*Config, error) {
cfg := DefaultConfig()
@@ -38,14 +91,14 @@ func loadConfig(data []byte) (*Config, error) {
// index position. We only reset cfg.ModelList when the user actually provides
// entries; when count is 0 we keep DefaultConfig's built-in list as fallback.
var tmp Config
if err := json.Unmarshal(data, &tmp); err != nil {
if err := decodeJSONWithDiagnostics(data, &tmp, "config.json"); err != nil {
return nil, err
}
if len(tmp.ModelList) > 0 {
cfg.ModelList = nil
}
if err := json.Unmarshal(data, cfg); err != nil {
if err := decodeJSONWithDiagnostics(data, cfg, "config.json"); err != nil {
return nil, err
}
return cfg, nil
@@ -96,17 +149,7 @@ func migrateV0ToV1(m map[string]any) error {
return fmt.Errorf("migrateV0ToV1: expected version 0, got %v", m["version"])
}
// Migrate agents.defaults.model → agents.defaults.model_name
if agents, ok := m["agents"].(map[string]any); ok {
if defaults, ok := agents["defaults"].(map[string]any); ok {
if model, hasModel := defaults["model"]; hasModel {
if _, hasModelName := defaults["model_name"]; !hasModelName {
defaults["model_name"] = model
}
delete(defaults, "model")
}
}
}
migrateLegacyAgentDefaultsModel(m)
// Migrate legacy providers to model_list if no model_list exists
if _, hasModelList := m["model_list"]; !hasModelList {
@@ -275,6 +318,9 @@ func migrateV2ToV3(m map[string]any) error {
return fmt.Errorf("migrateV2ToV3: expected version 2, got %v", m["version"])
}
migrateLegacyAgentDefaultsModel(m)
delete(m, "bindings")
// Rename channels → channel_list
if channels, ok := m["channels"]; ok {
delete(m, "channels")
@@ -334,7 +380,7 @@ func loadConfigMap(path string) (map[string]any, error) {
return nil, fmt.Errorf("failed to read config: %w", err)
}
if err = json.Unmarshal(data, &m1); err != nil {
return nil, fmt.Errorf("failed to parse config: %w", err)
return nil, wrapJSONError(data, err, "config.json")
}
secPath := securityPath(path)
data, err = os.ReadFile(secPath)
+1 -1
View File
@@ -75,7 +75,7 @@ func loadSecurityConfig(cfg *Config, securityPath string) error {
// Unmarshal non-channel fields from security.yml
// This will resolve encrypted values for model_list, tools, etc.
if err := yaml.Unmarshal(data, cfg); err != nil {
return fmt.Errorf("failed to parse security config: %w", err)
return fmt.Errorf("failed to parse security config %s: %w", securityPath, err)
}
if err := applyLegacySkillsSecurityConfig(cfg, data); err != nil {
return fmt.Errorf("failed to parse legacy skills security config: %w", err)
+2 -3
View File
@@ -43,11 +43,10 @@ func TestSecurityConfigIntegration(t *testing.T) {
t.Run("Full workflow with security references", func(t *testing.T) {
tmpDir := t.TempDir()
// Create config.json with direct security values (not ref: references)
// These values should take precedence over .security.yml
// Create config.json with direct security values using the current schema.
configPath := filepath.Join(tmpDir, "config.json")
configContent := `{
"version": 1,
"version": 2,
"model_list": [
{
"model_name": "test-model",