feat(daemon): surface config reload warnings
buildCodeOverrides now returns its warnings instead of logging them internally, so ApplyConfig can aggregate them with the binding-entry warnings from keys.BuildBindingEntries into one slice. Per-warning operator logging is preserved. When a reload produces any warnings, ApplyConfig emits a single global NoticeConfigReload notice whose Details (via a joined configWarningsError) lists every warning line, so the user sees one toast instead of silent log-only warnings.
- commit
- 1408e435d1e1b0616cdacc0cef824934ec87f32d
- author
- brice · · 2026-07-21 05:08 UTC
- parent
- 25f341a
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
2 files · +63 -7
M internal/usecase/daemon/config.go +32 -7
| @@ -1,6 +1,7 @@ | |||
| 1 | 1 | package daemon | |
| 2 | 2 | ||
| 3 | 3 | import ( | |
| 4 | + | "errors" | |
| 4 | 5 | "fmt" | |
| 5 | 6 | "maps" | |
| 6 | 7 | "regexp" | |
| @@ -40,10 +41,11 @@ func (d *Daemon) currentThemeConfig() themeConfigSnapshot { | |||
| 40 | 41 | // ApplyConfig validates and atomically swaps daemon runtime configuration. | |
| 41 | 42 | func (d *Daemon) ApplyConfig(cfg domain.Config) { | |
| 42 | 43 | bindings, warnings := keys.BuildBindingEntries(cfg.BindingEntries) | |
| 43 | - | for _, warning := range warnings { | |
| 44 | + | overrides, codeWarnings := d.buildCodeOverrides(cfg.Codes) | |
| 45 | + | allWarnings := append(append([]domain.Warning{}, warnings...), codeWarnings...) | |
| 46 | + | for _, warning := range allWarnings { | |
| 44 | 47 | d.logConfigWarning(warning) | |
| 45 | 48 | } | |
| 46 | - | overrides := d.buildCodeOverrides(cfg.Codes) | |
| 47 | 49 | allowlist := restoreProcessAllowlistFromConfig(cfg.Snapshot) | |
| 48 | 50 | d.bindings.Store(bindings) | |
| 49 | 51 | d.codeOverrides.Store(&overrides) | |
| @@ -73,24 +75,47 @@ func (d *Daemon) ApplyConfig(cfg domain.Config) { | |||
| 73 | 75 | } | |
| 74 | 76 | d.reapplyThemeAllSessions() | |
| 75 | 77 | d.repaintAllAttachedClients() | |
| 78 | + | ||
| 79 | + | if len(allWarnings) > 0 { | |
| 80 | + | first := allWarnings[0] | |
| 81 | + | d.notify(nil, domain.NoticeWarn, domain.NoticeConfigReload, | |
| 82 | + | fmt.Sprintf("config reloaded with %d warning(s): line %d: %s", len(allWarnings), first.Line, first.Msg), | |
| 83 | + | configWarningsError(allWarnings)) | |
| 84 | + | } | |
| 85 | + | } | |
| 86 | + | ||
| 87 | + | // configWarningsError joins every config warning into a single error whose | |
| 88 | + | // message lists them all, so noticeDetails (which renders a cause's own | |
| 89 | + | // Error() string) surfaces every warning line in one notice's Details. | |
| 90 | + | func configWarningsError(warnings []domain.Warning) error { | |
| 91 | + | lines := make([]string, len(warnings)) | |
| 92 | + | for i, w := range warnings { | |
| 93 | + | if w.Line > 0 { | |
| 94 | + | lines[i] = fmt.Sprintf("line %d: %s", w.Line, w.Msg) | |
| 95 | + | } else { | |
| 96 | + | lines[i] = w.Msg | |
| 97 | + | } | |
| 98 | + | } | |
| 99 | + | return errors.New(strings.Join(lines, "; ")) | |
| 76 | 100 | } | |
| 77 | 101 | ||
| 78 | - | func (d *Daemon) buildCodeOverrides(configured map[string]string) map[string]string { | |
| 102 | + | func (d *Daemon) buildCodeOverrides(configured map[string]string) (map[string]string, []domain.Warning) { | |
| 79 | 103 | commands := command.Registry() | |
| 80 | 104 | bySlug := make(map[string]command.Command, len(commands)) | |
| 81 | 105 | for _, cmd := range commands { | |
| 82 | 106 | bySlug[cmd.Slug] = cmd | |
| 83 | 107 | } | |
| 84 | 108 | ||
| 109 | + | var warnings []domain.Warning | |
| 85 | 110 | desired := make(map[string]string) | |
| 86 | 111 | for _, slug := range sortedKeys(configured) { | |
| 87 | 112 | if _, ok := bySlug[slug]; !ok { | |
| 88 | - | d.logConfigWarning(domain.Warning{Msg: fmt.Sprintf("unknown command code slug %q", slug)}) | |
| 113 | + | warnings = append(warnings, domain.Warning{Msg: fmt.Sprintf("unknown command code slug %q", slug)}) | |
| 89 | 114 | continue | |
| 90 | 115 | } | |
| 91 | 116 | code := strings.ToUpper(strings.TrimSpace(configured[slug])) | |
| 92 | 117 | if !commandCodePattern.MatchString(code) { | |
| 93 | - | d.logConfigWarning(domain.Warning{Msg: fmt.Sprintf("invalid command code for %q", slug)}) | |
| 118 | + | warnings = append(warnings, domain.Warning{Msg: fmt.Sprintf("invalid command code for %q", slug)}) | |
| 94 | 119 | continue | |
| 95 | 120 | } | |
| 96 | 121 | desired[slug] = code | |
| @@ -146,7 +171,7 @@ func (d *Daemon) buildCodeOverrides(configured map[string]string) map[string]str | |||
| 146 | 171 | if _, isOverride := accepted[slug]; !isOverride { | |
| 147 | 172 | continue | |
| 148 | 173 | } | |
| 149 | - | d.logConfigWarning(domain.Warning{Msg: fmt.Sprintf("command code %q for %q conflicts with %q", code, slug, keep)}) | |
| 174 | + | warnings = append(warnings, domain.Warning{Msg: fmt.Sprintf("command code %q for %q conflicts with %q", code, slug, keep)}) | |
| 150 | 175 | delete(accepted, slug) | |
| 151 | 176 | dropped = true | |
| 152 | 177 | } | |
| @@ -155,7 +180,7 @@ func (d *Daemon) buildCodeOverrides(configured map[string]string) map[string]str | |||
| 155 | 180 | break | |
| 156 | 181 | } | |
| 157 | 182 | } | |
| 158 | - | return accepted | |
| 183 | + | return accepted, warnings | |
| 159 | 184 | } | |
| 160 | 185 | ||
| 161 | 186 | func sortedKeys(values map[string]string) []string { | |
| @@ -1,6 +1,7 @@ | |||||
| 1 | package daemon | 1 | package daemon | ||
| 2 | 2 | ||||
| 3 | import ( | 3 | import ( | ||
| 4 | + | "errors" | |||
| 4 | "fmt" | 5 | "fmt" | ||
| 5 | "maps" | 6 | "maps" | ||
| 6 | "regexp" | 7 | "regexp" | ||
| @@ -40,10 +41,11 @@ func (d *Daemon) currentThemeConfig() themeConfigSnapshot { | |||||
| 40 | // ApplyConfig validates and atomically swaps daemon runtime configuration. | 41 | // ApplyConfig validates and atomically swaps daemon runtime configuration. | ||
| 41 | func (d *Daemon) ApplyConfig(cfg domain.Config) { | 42 | func (d *Daemon) ApplyConfig(cfg domain.Config) { | ||
| 42 | bindings, warnings := keys.BuildBindingEntries(cfg.BindingEntries) | 43 | bindings, warnings := keys.BuildBindingEntries(cfg.BindingEntries) | ||
| 43 | - | for _, warning := range warnings { | 44 | + | overrides, codeWarnings := d.buildCodeOverrides(cfg.Codes) |
| 45 | + | allWarnings := append(append([]domain.Warning{}, warnings...), codeWarnings...) | |||
| 46 | + | for _, warning := range allWarnings { | |||
| 44 | d.logConfigWarning(warning) | 47 | d.logConfigWarning(warning) | ||
| 45 | } | 48 | } | ||
| 46 | - | overrides := d.buildCodeOverrides(cfg.Codes) | |||
| 47 | allowlist := restoreProcessAllowlistFromConfig(cfg.Snapshot) | 49 | allowlist := restoreProcessAllowlistFromConfig(cfg.Snapshot) | ||
| 48 | d.bindings.Store(bindings) | 50 | d.bindings.Store(bindings) | ||
| 49 | d.codeOverrides.Store(&overrides) | 51 | d.codeOverrides.Store(&overrides) | ||
| @@ -73,24 +75,47 @@ func (d *Daemon) ApplyConfig(cfg domain.Config) { | |||||
| 73 | } | 75 | } | ||
| 74 | d.reapplyThemeAllSessions() | 76 | d.reapplyThemeAllSessions() | ||
| 75 | d.repaintAllAttachedClients() | 77 | d.repaintAllAttachedClients() | ||
| 78 | + | ||||
| 79 | + | if len(allWarnings) > 0 { | |||
| 80 | + | first := allWarnings[0] | |||
| 81 | + | d.notify(nil, domain.NoticeWarn, domain.NoticeConfigReload, | |||
| 82 | + | fmt.Sprintf("config reloaded with %d warning(s): line %d: %s", len(allWarnings), first.Line, first.Msg), | |||
| 83 | + | configWarningsError(allWarnings)) | |||
| 84 | + | } | |||
| 85 | + | } | |||
| 86 | + | ||||
| 87 | + | // configWarningsError joins every config warning into a single error whose | |||
| 88 | + | // message lists them all, so noticeDetails (which renders a cause's own | |||
| 89 | + | // Error() string) surfaces every warning line in one notice's Details. | |||
| 90 | + | func configWarningsError(warnings []domain.Warning) error { | |||
| 91 | + | lines := make([]string, len(warnings)) | |||
| 92 | + | for i, w := range warnings { | |||
| 93 | + | if w.Line > 0 { | |||
| 94 | + | lines[i] = fmt.Sprintf("line %d: %s", w.Line, w.Msg) | |||
| 95 | + | } else { | |||
| 96 | + | lines[i] = w.Msg | |||
| 97 | + | } | |||
| 98 | + | } | |||
| 99 | + | return errors.New(strings.Join(lines, "; ")) | |||
| 76 | } | 100 | } | ||
| 77 | 101 | ||||
| 78 | - | func (d *Daemon) buildCodeOverrides(configured map[string]string) map[string]string { | 102 | + | func (d *Daemon) buildCodeOverrides(configured map[string]string) (map[string]string, []domain.Warning) { |
| 79 | commands := command.Registry() | 103 | commands := command.Registry() | ||
| 80 | bySlug := make(map[string]command.Command, len(commands)) | 104 | bySlug := make(map[string]command.Command, len(commands)) | ||
| 81 | for _, cmd := range commands { | 105 | for _, cmd := range commands { | ||
| 82 | bySlug[cmd.Slug] = cmd | 106 | bySlug[cmd.Slug] = cmd | ||
| 83 | } | 107 | } | ||
| 84 | 108 | ||||
| 109 | + | var warnings []domain.Warning | |||
| 85 | desired := make(map[string]string) | 110 | desired := make(map[string]string) | ||
| 86 | for _, slug := range sortedKeys(configured) { | 111 | for _, slug := range sortedKeys(configured) { | ||
| 87 | if _, ok := bySlug[slug]; !ok { | 112 | if _, ok := bySlug[slug]; !ok { | ||
| 88 | - | d.logConfigWarning(domain.Warning{Msg: fmt.Sprintf("unknown command code slug %q", slug)}) | 113 | + | warnings = append(warnings, domain.Warning{Msg: fmt.Sprintf("unknown command code slug %q", slug)}) |
| 89 | continue | 114 | continue | ||
| 90 | } | 115 | } | ||
| 91 | code := strings.ToUpper(strings.TrimSpace(configured[slug])) | 116 | code := strings.ToUpper(strings.TrimSpace(configured[slug])) | ||
| 92 | if !commandCodePattern.MatchString(code) { | 117 | if !commandCodePattern.MatchString(code) { | ||
| 93 | - | d.logConfigWarning(domain.Warning{Msg: fmt.Sprintf("invalid command code for %q", slug)}) | 118 | + | warnings = append(warnings, domain.Warning{Msg: fmt.Sprintf("invalid command code for %q", slug)}) |
| 94 | continue | 119 | continue | ||
| 95 | } | 120 | } | ||
| 96 | desired[slug] = code | 121 | desired[slug] = code | ||
| @@ -146,7 +171,7 @@ func (d *Daemon) buildCodeOverrides(configured map[string]string) map[string]str | |||||
| 146 | if _, isOverride := accepted[slug]; !isOverride { | 171 | if _, isOverride := accepted[slug]; !isOverride { | ||
| 147 | continue | 172 | continue | ||
| 148 | } | 173 | } | ||
| 149 | - | d.logConfigWarning(domain.Warning{Msg: fmt.Sprintf("command code %q for %q conflicts with %q", code, slug, keep)}) | 174 | + | warnings = append(warnings, domain.Warning{Msg: fmt.Sprintf("command code %q for %q conflicts with %q", code, slug, keep)}) |
| 150 | delete(accepted, slug) | 175 | delete(accepted, slug) | ||
| 151 | dropped = true | 176 | dropped = true | ||
| 152 | } | 177 | } | ||
| @@ -155,7 +180,7 @@ func (d *Daemon) buildCodeOverrides(configured map[string]string) map[string]str | |||||
| 155 | break | 180 | break | ||
| 156 | } | 181 | } | ||
| 157 | } | 182 | } | ||
| 158 | - | return accepted | 183 | + | return accepted, warnings |
| 159 | } | 184 | } | ||
| 160 | 185 | ||||
| 161 | func sortedKeys(values map[string]string) []string { | 186 | func sortedKeys(values map[string]string) []string { | ||
M internal/usecase/daemon/config_test.go +31 -0
| @@ -543,3 +543,34 @@ func TestApplyConfigInvalidDefaultsWithoutPanic(t *testing.T) { | |||
| 543 | 543 | _, ok = d.commandByEffectiveCode("TOOLONG") | |
| 544 | 544 | require.False(t, ok) | |
| 545 | 545 | } | |
| 546 | + | ||
| 547 | + | func TestApplyConfigSurfacesOneNoticeAggregatingAllWarnings(t *testing.T) { | |
| 548 | + | d := newTestDaemon(t, nil, stubClock{}) | |
| 549 | + | d.ApplyConfig(domain.Config{ | |
| 550 | + | Theme: domain.ThemeAuto, | |
| 551 | + | BindingEntries: []domain.ConfigEntry{ | |
| 552 | + | {Key: "unknown-action", Value: "alt+x"}, | |
| 553 | + | }, | |
| 554 | + | Codes: map[string]string{ | |
| 555 | + | "missing-command": "OK", | |
| 556 | + | "new-tab": "toolong", | |
| 557 | + | }, | |
| 558 | + | }) | |
| 559 | + | ||
| 560 | + | history := d.notices.history() | |
| 561 | + | var reloadNotices []domain.Notification | |
| 562 | + | for _, n := range history { | |
| 563 | + | if n.Code == domain.NoticeConfigReload { | |
| 564 | + | reloadNotices = append(reloadNotices, n) | |
| 565 | + | } | |
| 566 | + | } | |
| 567 | + | require.Len(t, reloadNotices, 1, "expected exactly one config-reload notice per ApplyConfig call") | |
| 568 | + | ||
| 569 | + | n := reloadNotices[0] | |
| 570 | + | require.Equal(t, domain.NoticeWarn, n.Severity) | |
| 571 | + | require.Equal(t, domain.SessionID(""), n.SessionID, "config reload notice must be global") | |
| 572 | + | require.Contains(t, n.Message, "config reloaded with 3 warning(s):") | |
| 573 | + | require.Contains(t, n.Details, `unknown action "unknown-action"`) | |
| 574 | + | require.Contains(t, n.Details, `unknown command code slug "missing-command"`) | |
| 575 | + | require.Contains(t, n.Details, `invalid command code for "new-tab"`) | |
| 576 | + | } | |
| @@ -543,3 +543,34 @@ func TestApplyConfigInvalidDefaultsWithoutPanic(t *testing.T) { | |||||
| 543 | _, ok = d.commandByEffectiveCode("TOOLONG") | 543 | _, ok = d.commandByEffectiveCode("TOOLONG") | ||
| 544 | require.False(t, ok) | 544 | require.False(t, ok) | ||
| 545 | } | 545 | } | ||
| 546 | + | ||||
| 547 | + | func TestApplyConfigSurfacesOneNoticeAggregatingAllWarnings(t *testing.T) { | |||
| 548 | + | d := newTestDaemon(t, nil, stubClock{}) | |||
| 549 | + | d.ApplyConfig(domain.Config{ | |||
| 550 | + | Theme: domain.ThemeAuto, | |||
| 551 | + | BindingEntries: []domain.ConfigEntry{ | |||
| 552 | + | {Key: "unknown-action", Value: "alt+x"}, | |||
| 553 | + | }, | |||
| 554 | + | Codes: map[string]string{ | |||
| 555 | + | "missing-command": "OK", | |||
| 556 | + | "new-tab": "toolong", | |||
| 557 | + | }, | |||
| 558 | + | }) | |||
| 559 | + | ||||
| 560 | + | history := d.notices.history() | |||
| 561 | + | var reloadNotices []domain.Notification | |||
| 562 | + | for _, n := range history { | |||
| 563 | + | if n.Code == domain.NoticeConfigReload { | |||
| 564 | + | reloadNotices = append(reloadNotices, n) | |||
| 565 | + | } | |||
| 566 | + | } | |||
| 567 | + | require.Len(t, reloadNotices, 1, "expected exactly one config-reload notice per ApplyConfig call") | |||
| 568 | + | ||||
| 569 | + | n := reloadNotices[0] | |||
| 570 | + | require.Equal(t, domain.NoticeWarn, n.Severity) | |||
| 571 | + | require.Equal(t, domain.SessionID(""), n.SessionID, "config reload notice must be global") | |||
| 572 | + | require.Contains(t, n.Message, "config reloaded with 3 warning(s):") | |||
| 573 | + | require.Contains(t, n.Details, `unknown action "unknown-action"`) | |||
| 574 | + | require.Contains(t, n.Details, `unknown command code slug "missing-command"`) | |||
| 575 | + | require.Contains(t, n.Details, `invalid command code for "new-tab"`) | |||
| 576 | + | } | |||