fix(config): [global] and [docker] unknown-key warnings carry 'did you mean?' (#678) (#695)

## Summary

Brings the \`[global]\` and \`[docker]\` unknown-key warnings into
parity with job sections so operator typos now suggest the nearest
mapstructure key. Pre-fix:

\`\`\`
WARN Unknown configuration key 'webhook-defauls-preset' in [global]
section (typo?)
\`\`\`

Post-fix:

\`\`\`
WARN Unknown configuration key 'webhook-defauls-preset' in [global]
section (did you mean 'webhook-default-preset'?)
\`\`\`

Closes [#678](#678).

## Approach

Extracted a \`logSectionUnknownKeyWarnings(logger, section, unknownKeys,
knownKeys, filename)\` helper that mirrors the filename / suggestion
matrix already used by \`logJobUnknownKeyWarnings\`, then routed both
\`[global]\` and \`[docker]\` paths through it from both call sites
(file-based \`logUnknownKeyWarnings\` and string-based
\`BuildFromString\`).

Known keys come from \`extractMapstructureKeys(Config{}.Global)\` and
\`extractMapstructureKeys(DockerConfig{})\` via thin \`globalKnownKeys\`
/ \`dockerKnownKeys\` helpers, so the suggestion list cannot drift from
the actual decoded struct — the same drift-resistance pattern as
\`getKnownKeysForJobType\`.

## Tests

Three new tests in \`cli/config_decode_test.go\`:

1. **\`TestGlobalSectionUnknownKeyWarning_DidYouMean\`** — the exact
mistype called out in the issue body (\`webhook-defauls-preset\` →
\`webhook-default-preset\`).
2. **\`TestGlobalSectionUnknownKeyWarning_NoSuggestion\`** — locks the
fallback to \`(typo?)\` when no close match exists (asymmetry against
the suggestion case is what makes the parity fix meaningful).
3. **\`TestDockerSectionUnknownKeyWarning_DidYouMean\`** — same parity
assertion for the \`[docker]\` path so a future code split can't
silently drop suggestions on one half.

## Test plan

- [x] \`go test ./...\` passes (full repo, 14 packages, ~58s)
- [x] \`golangci-lint run\` clean (intentional typos in test carry
\`//nolint:misspell\`)
- [x] \`go vet ./...\` clean
- [ ] CI green

## References

- Surfaced in: [#677](#677)
(parallel-reviewer pass)
- Tracks against:
[#621](#621) (global-keys
drift detection family)

Author: CybotTM

cli/config.go

@@ -241,19 +241,61 @@ 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)
+	if len(res.unknownGlobal) > 0 {
+		logSectionUnknownKeyWarnings(logger, "global", res.unknownGlobal, globalKnownKeys(), filename)
 	}
-	for _, key := range res.unknownDocker {
-		logger.Warn(fmt.Sprintf("Unknown configuration key '%s' in [docker] section (typo?)", key),
-			"key", key, "file", filename)
+	if len(res.unknownDocker) > 0 {
+		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)
+		}
+		// Drop the "file" structured attr for string-based configs (filename
+		// is empty) so JSON logs don't carry a noisy file="" field for the
+		// BuildFromString path.
+		if filename != "" {
+			logger.Warn(msg, "key", key, "file", filename)
+		} else {
+			logger.Warn(msg, "key", key)
+		}
+	}
+}
+
+// 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,12 +364,12 @@ 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))
+		// Log warnings for unknown keys (empty filename for string-based config)
+		if len(parseRes.unknownGlobal) > 0 {
+			logSectionUnknownKeyWarnings(logger, "global", parseRes.unknownGlobal, globalKnownKeys(), "")
 		}
-		for _, key := range parseRes.unknownDocker {
-			logger.Warn(fmt.Sprintf("Unknown configuration key '%s' in [docker] section (typo?)", key))
+		if len(parseRes.unknownDocker) > 0 {
+			logSectionUnknownKeyWarnings(logger, "docker", parseRes.unknownDocker, dockerKnownKeys(), "")
 		}
 
 		// Log warnings for unknown keys in job sections (empty filename for string-based config)

cli/config_decode_test.go

@@ -4,6 +4,8 @@
 package cli
 
 import (
+	"os"
+	"path/filepath"
 	"testing"
 	"time"
 
@@ -470,6 +472,154 @@ 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. Also pins cross-section
+// isolation: a [docker] typo must NOT be misattributed to [global].
+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.False(t, handler.HasWarning("[global] section"),
+		"Should NOT misattribute a [docker] typo to the [global] section")
+	assert.True(t, handler.HasWarning("did you mean 'include-stopped'"),
+		"Should suggest 'include-stopped' for 'inculde-stopped'")
+}
+
+// TestGlobalSectionUnknownKeyWarning_NonSquashedKey complements the
+// _DidYouMean test (which targets `webhook-default-preset` inside the
+// squashed WebhookGlobalConfig) by exercising a key declared DIRECTLY on
+// the anonymous Global struct. Together they lock both halves of
+// extractMapstructureKeysFromType: squash recursion AND direct-field
+// enumeration. Without this, a refactor that breaks one path but not the
+// other could pass the existing tests.
+func TestGlobalSectionUnknownKeyWarning_NonSquashedKey(t *testing.T) {
+	t.Parallel()
+
+	// "enabel-pprof" swaps the el on "enable-pprof", a key declared
+	// directly on Config{}.Global (not via squash).
+	configStr := `
+[global]
+enabel-pprof = 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 'enabel-pprof'"),
+		"Should warn about 'enabel-pprof'")
+	assert.True(t, handler.HasWarning("did you mean 'enable-pprof'"),
+		"Should suggest 'enable-pprof' for 'enabel-pprof'")
+}
+
+// TestGlobalSectionUnknownKeyWarning_FileBased pins the suggestion
+// behavior on the BuildFromFile path (logUnknownKeyWarnings), the
+// production code path the issue reporter actually hits. The other
+// _DidYouMean tests cover BuildFromString (filename == ""); this one
+// covers the four-arm switch's filename != "" branch and asserts the
+// "of <filename>" substring lands in the message.
+func TestGlobalSectionUnknownKeyWarning_FileBased(t *testing.T) {
+	t.Parallel()
+
+	tmpDir := t.TempDir()
+	configPath := filepath.Join(tmpDir, "config.ini")
+	//nolint:misspell // intentional typo for did-you-mean assertion
+	content := `[global]
+webhook-defauls-preset = json-post
+`
+	require.NoError(t, os.WriteFile(configPath, []byte(content), 0o644))
+
+	logger, handler := test.NewTestLoggerWithHandler()
+	_, err := BuildFromFile(configPath, 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 the typo")
+	assert.True(t, handler.HasWarning("[global] section"),
+		"Should name the [global] section")
+	assert.True(t, handler.HasWarning("of "+configPath),
+		"Should include the filename (file-based path's message variant)")
+	assert.True(t, handler.HasWarning("did you mean 'webhook-default-preset'"),
+		"Should suggest 'webhook-default-preset' even on the file-based path")
+}
+
 // Phase 8: Additional coverage tests for config_decode.go
 
 func TestWeakDecodeConsistent_NilInput(t *testing.T) {