fix(config): [global] and [docker] unknown-key warnings carry "did you mean?"

Pre-fix, the unknown-key warning was asymmetric across INI section
types: job sections (job-exec, job-run, etc.) printed
'(did you mean "..."?)' via findClosestMatch, but [global] and [docker]
printed bare '(typo?)' with no suggestion — leaving the operator to
read the line and guess.

Surfaced in PR #677's parallel-reviewer pass: the new
webhook-default-preset global key is easy to mistype
(webhook-defauls-preset swaps t/s) and the typo logged a [global]
warning with no hint at the right spelling.

Fix extracts a logSectionUnknownKeyWarnings helper that mirrors
logJobUnknownKeyWarnings's filename/suggestion matrix and call it from
both [global] and [docker] paths in logUnknownKeyWarnings (file-based)
and BuildFromString (string-based). Known keys come from
extractMapstructureKeys(Config{}.Global) and
extractMapstructureKeys(DockerConfig{}) via two thin helpers
(globalKnownKeys / dockerKnownKeys) so the suggestion list cannot drift
from the actual decoded struct.

Three new tests:
- TestGlobalSectionUnknownKeyWarning_DidYouMean: the issue's exact
  example — webhook-defauls-preset suggests webhook-default-preset.
- TestGlobalSectionUnknownKeyWarning_NoSuggestion: locks the fallback
  to '(typo?)' when no close match exists.
- TestDockerSectionUnknownKeyWarning_DidYouMean: same parity assertion
  for the [docker] path so a future code split can't silently drop
  suggestions on one half.

Closes #678. Tracks against #621 (global-keys drift detection family).

Signed-off-by: Sebastian Mendel <info@sebastianmendel.de>

Author: CybotTM

cli/config.go

@@ -241,19 +241,50 @@ func logUnknownKeyWarnings(logger *slog.Logger, filename string, res *parseResul
 		return
 	}
 
-	for _, key := range res.unknownGlobal {
-		logger.Warn(fmt.Sprintf("Unknown configuration key '%s' in [global] section (typo?)", key),
-			"key", key, "file", filename)
-	}
-	for _, key := range res.unknownDocker {
-		logger.Warn(fmt.Sprintf("Unknown configuration key '%s' in [docker] section (typo?)", key),
-			"key", key, "file", filename)
-	}
+	logSectionUnknownKeyWarnings(logger, "global", res.unknownGlobal, globalKnownKeys(), filename)
+	logSectionUnknownKeyWarnings(logger, "docker", res.unknownDocker, dockerKnownKeys(), filename)
 
 	// Log warnings for unknown keys in job sections
 	logJobUnknownKeyWarnings(logger, res.unknownJobs, filename)
 }
 
+// logSectionUnknownKeyWarnings emits a "Unknown configuration key … in [section]"
+// warning for each key, with a "did you mean?" suggestion when a close match is
+// found in knownKeys. Used by [global] and [docker] sections so the suggestion
+// behavior is at parity with the job-section path (issue #678).
+func logSectionUnknownKeyWarnings(logger *slog.Logger, section string, unknownKeys, knownKeys []string, filename string) {
+	for _, key := range unknownKeys {
+		suggestion := findClosestMatch(key, knownKeys)
+		var msg string
+		switch {
+		case suggestion != "" && filename != "":
+			msg = fmt.Sprintf("Unknown configuration key '%s' in [%s] section of %s (did you mean '%s'?)",
+				key, section, filename, suggestion)
+		case suggestion != "":
+			msg = fmt.Sprintf("Unknown configuration key '%s' in [%s] section (did you mean '%s'?)", key, section, suggestion)
+		case filename != "":
+			msg = fmt.Sprintf("Unknown configuration key '%s' in [%s] section of %s (typo?)", key, section, filename)
+		default:
+			msg = fmt.Sprintf("Unknown configuration key '%s' in [%s] section (typo?)", key, section)
+		}
+		logger.Warn(msg, "key", key, "file", filename)
+	}
+}
+
+// globalKnownKeys returns the list of valid mapstructure keys for the [global]
+// INI section. Derived from Config{}.Global so the suggestion list cannot
+// drift from the actual decoded struct.
+func globalKnownKeys() []string {
+	return extractMapstructureKeys(Config{}.Global)
+}
+
+// dockerKnownKeys returns the list of valid mapstructure keys for the [docker]
+// INI section. Derived from DockerConfig{} so the suggestion list cannot drift
+// from the actual decoded struct.
+func dockerKnownKeys() []string {
+	return extractMapstructureKeys(DockerConfig{})
+}
+
 // logJobUnknownKeyWarnings logs warnings for unknown keys in job sections with
 // "did you mean?" suggestions. If filename is non-empty, it is included in the message.
 func logJobUnknownKeyWarnings(logger *slog.Logger, unknownJobs []jobUnknownKeys, filename string) {
@@ -322,13 +353,9 @@ func BuildFromString(configStr string, logger *slog.Logger) (*Config, error) {
 	if parseRes != nil {
 		usedKeys = parseRes.usedKeys
 
-		// Log warnings for unknown keys
-		for _, key := range parseRes.unknownGlobal {
-			logger.Warn(fmt.Sprintf("Unknown configuration key '%s' in [global] section (typo?)", key))
-		}
-		for _, key := range parseRes.unknownDocker {
-			logger.Warn(fmt.Sprintf("Unknown configuration key '%s' in [docker] section (typo?)", key))
-		}
+		// Log warnings for unknown keys (empty filename for string-based config)
+		logSectionUnknownKeyWarnings(logger, "global", parseRes.unknownGlobal, globalKnownKeys(), "")
+		logSectionUnknownKeyWarnings(logger, "docker", parseRes.unknownDocker, dockerKnownKeys(), "")
 
 		// Log warnings for unknown keys in job sections (empty filename for string-based config)
 		logJobUnknownKeyWarnings(logger, parseRes.unknownJobs, "")

cli/config_decode_test.go

@@ -470,6 +470,90 @@ typo2 = value2
 		"Should have warning for job2")
 }
 
+// TestGlobalSectionUnknownKeyWarning_DidYouMean pins the parity fix from
+// issue #678: a typo on any [global] mapstructure-tagged key should produce
+// a "did you mean?" line citing the nearest match within Levenshtein
+// threshold. Pre-fix, only job-section warnings carried the suggestion;
+// [global] just emitted "(typo?)" and left the operator to guess.
+func TestGlobalSectionUnknownKeyWarning_DidYouMean(t *testing.T) {
+	t.Parallel()
+
+	// "webhook-defauls-preset" swaps t/s on "webhook-default-preset" — the
+	// exact mistype called out in the issue body.
+	//nolint:misspell // intentional typo for did-you-mean assertion
+	configStr := `
+[global]
+webhook-defauls-preset = json-post
+`
+
+	logger, handler := test.NewTestLoggerWithHandler()
+	_, err := BuildFromString(configStr, logger)
+	require.NoError(t, err)
+
+	assert.Equal(t, 1, handler.WarningCount(), "Expected 1 warning for unknown key")
+	//nolint:misspell // intentional typo for did-you-mean assertion
+	assert.True(t, handler.HasWarning("Unknown configuration key 'webhook-defauls-preset'"),
+		"Should warn about 'webhook-defauls-preset'")
+	assert.True(t, handler.HasWarning("[global] section"),
+		"Should name the [global] section")
+	//nolint:misspell // intentional typo for did-you-mean assertion
+	assert.True(t, handler.HasWarning("did you mean 'webhook-default-preset'"),
+		"Should suggest 'webhook-default-preset' for 'webhook-defauls-preset'")
+}
+
+// TestGlobalSectionUnknownKeyWarning_NoSuggestion pins that the [global]
+// path still falls back to "(typo?)" — same shape as the existing job-section
+// no-suggestion test — when no close match exists. Asymmetry against the
+// suggestion case is what makes the parity fix meaningful.
+func TestGlobalSectionUnknownKeyWarning_NoSuggestion(t *testing.T) {
+	t.Parallel()
+
+	configStr := `
+[global]
+zzz-totally-unrelated-key = value
+`
+
+	logger, handler := test.NewTestLoggerWithHandler()
+	_, err := BuildFromString(configStr, logger)
+	require.NoError(t, err)
+
+	assert.Equal(t, 1, handler.WarningCount(), "Expected 1 warning for unknown key")
+	assert.True(t, handler.HasWarning("Unknown configuration key 'zzz-totally-unrelated-key'"),
+		"Should warn about 'zzz-totally-unrelated-key'")
+	assert.True(t, handler.HasWarning("[global] section"),
+		"Should name the [global] section")
+	assert.True(t, handler.HasWarning("typo?"),
+		"Should fall back to '(typo?)' when no close match found")
+	assert.False(t, handler.HasWarning("did you mean"),
+		"Should not suggest when no close match")
+}
+
+// TestDockerSectionUnknownKeyWarning_DidYouMean confirms the same parity fix
+// also applies to the [docker] section path (#678). The two sections share
+// the same code path, but explicitly asserting both prevents a future split
+// from silently dropping suggestions on one half.
+func TestDockerSectionUnknownKeyWarning_DidYouMean(t *testing.T) {
+	t.Parallel()
+
+	// "inculde-stopped" swaps u/n on "include-stopped".
+	configStr := `
+[docker]
+inculde-stopped = true
+`
+
+	logger, handler := test.NewTestLoggerWithHandler()
+	_, err := BuildFromString(configStr, logger)
+	require.NoError(t, err)
+
+	assert.Equal(t, 1, handler.WarningCount(), "Expected 1 warning for unknown key")
+	assert.True(t, handler.HasWarning("Unknown configuration key 'inculde-stopped'"),
+		"Should warn about 'inculde-stopped'")
+	assert.True(t, handler.HasWarning("[docker] section"),
+		"Should name the [docker] section")
+	assert.True(t, handler.HasWarning("did you mean 'include-stopped'"),
+		"Should suggest 'include-stopped' for 'inculde-stopped'")
+}
+
 // Phase 8: Additional coverage tests for config_decode.go
 
 func TestWeakDecodeConsistent_NilInput(t *testing.T) {