diff --git a/internal/app/app.go b/internal/app/app.go index c179ff2..c7cfc79 100644 --- a/internal/app/app.go +++ b/internal/app/app.go @@ -381,7 +381,11 @@ func ResolveBatch(req BatchRequest, now time.Time) ([]report.Resolved, error) { } func reportRegistry(cfg config.Config) (report.Registry, error) { - registry, err := report.DefaultRegistry().WithModuleOverrides(cfg.ReportModuleOverrides()) + overrides, err := cfg.ReportModuleOverrides() + if err != nil { + return report.Registry{}, err + } + registry, err := report.DefaultRegistry().WithModuleOverrides(overrides) if err != nil { return report.Registry{}, err } diff --git a/internal/app/app_test.go b/internal/app/app_test.go index eab6538..beb9bb2 100644 --- a/internal/app/app_test.go +++ b/internal/app/app_test.go @@ -1895,6 +1895,24 @@ reports: } } +func TestResolveGenerateRejectsInvalidProgrammaticReportOverrides(t *testing.T) { + cfg := config.Defaults() + cfg.Reports = map[string]config.ReportConfig{ + "moon": {}, + } + + _, err := ResolveGenerate(GenerateRequest{ + Config: cfg, + Report: ReportDaily, + }, mustParse("2026-05-29T05:00:00-05:00")) + if err == nil { + t.Fatal("ResolveGenerate() error = nil, want report override error") + } + if !strings.Contains(err.Error(), "reports.moon") { + t.Fatalf("ResolveGenerate() error = %q, want report override context", err.Error()) + } +} + func moduleIDsForTest(ids []module.ID) []string { out := make([]string, 0, len(ids)) for _, id := range ids { diff --git a/internal/config/config_test.go b/internal/config/config_test.go index 850ba89..b6b3852 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -3,6 +3,7 @@ package config import ( "os" "path/filepath" + "reflect" "strings" "testing" "time" @@ -91,7 +92,10 @@ func TestLoadExampleConfig(t *testing.T) { if len(cfg.Notify.Distributor.ReportPathTemplates) != 1 { t.Fatalf("ReportPathTemplates = %#v, want example archive path", cfg.Notify.Distributor.ReportPathTemplates) } - overrides := cfg.ReportModuleOverrides() + overrides, err := cfg.ReportModuleOverrides() + if err != nil { + t.Fatalf("ReportModuleOverrides() error = %v", err) + } hourly := overrides[report.Hourly] if len(hourly) != 9 { t.Fatalf("hourly example override length = %d, want 9", len(hourly)) @@ -155,7 +159,10 @@ reports: if err != nil { t.Fatalf("LoadFile() error = %v", err) } - overrides := cfg.ReportModuleOverrides() + overrides, err := cfg.ReportModuleOverrides() + if err != nil { + t.Fatalf("ReportModuleOverrides() error = %v", err) + } items := overrides[report.DailyToday] if len(items) != 6 { t.Fatalf("daily override length = %d, want 6", len(items)) @@ -192,7 +199,10 @@ reports: if err != nil { t.Fatalf("LoadFile() error = %v", err) } - overrides := cfg.ReportModuleOverrides() + overrides, err := cfg.ReportModuleOverrides() + if err != nil { + t.Fatalf("ReportModuleOverrides() error = %v", err) + } items := overrides[report.Hourly] if len(items) != 4 { t.Fatalf("hourly override length = %d, want 4", len(items)) @@ -223,7 +233,10 @@ reports: if err != nil { t.Fatalf("LoadFile() error = %v", err) } - overrides := cfg.ReportModuleOverrides() + overrides, err := cfg.ReportModuleOverrides() + if err != nil { + t.Fatalf("ReportModuleOverrides() error = %v", err) + } if len(overrides[report.ThreeDay]) != 1 || overrides[report.ThreeDay][0].ID != module.Metadata { t.Fatalf("three-day alias override = %#v, want metadata override", overrides[report.ThreeDay]) } @@ -235,6 +248,84 @@ reports: } } +func TestValidateReportModuleKeysWithoutMutatingOptions(t *testing.T) { + cfg := Defaults() + rawOptions := map[string]any{ + "sections": []any{"short_term"}, + } + cfg.Reports = map[string]ReportConfig{ + "daily": { + DeterministicModules: []ModuleConfigItem{ + {ID: module.Metadata}, + {ID: module.AreaForecastDiscussion, Options: rawOptions}, + }, + deterministicModulesSet: true, + }, + } + + if err := Validate(cfg); err != nil { + t.Fatalf("Validate() error = %v", err) + } + got, ok := cfg.Reports["daily"].DeterministicModules[1].Options.(map[string]any) + if !ok || !reflect.DeepEqual(got, rawOptions) { + t.Fatalf("Options after Validate = %#v, want original raw map", cfg.Reports["daily"].DeterministicModules[1].Options) + } +} + +func TestValidateReportModuleAliasesDirectly(t *testing.T) { + tests := []struct { + name string + reports map[string]ReportConfig + wantErr string + }{ + { + name: "DuplicateAlias", + reports: map[string]ReportConfig{ + "daily": {}, + "daily_today": {}, + }, + wantErr: "duplicates report override", + }, + { + name: "UnknownReport", + reports: map[string]ReportConfig{ + "moon": {}, + }, + wantErr: "reports.moon", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + cfg := Defaults() + cfg.Reports = tt.reports + err := Validate(cfg) + if err == nil { + t.Fatal("Validate() error = nil, want report key error") + } + if !strings.Contains(err.Error(), tt.wantErr) { + t.Fatalf("Validate() error = %q, want %q", err.Error(), tt.wantErr) + } + }) + } +} + +func TestReportModuleOverridesRejectsInvalidReportKeys(t *testing.T) { + cfg := Defaults() + cfg.Reports = map[string]ReportConfig{ + "moon": { + DeterministicModules: []ModuleConfigItem{{ID: module.Metadata}}, + deterministicModulesSet: true, + }, + } + _, err := cfg.ReportModuleOverrides() + if err == nil { + t.Fatal("ReportModuleOverrides() error = nil, want invalid report key") + } + if !strings.Contains(err.Error(), "reports.moon") { + t.Fatalf("ReportModuleOverrides() error = %q, want report key context", err.Error()) + } +} + func TestReportModuleOverrideValidation(t *testing.T) { tests := []struct { name string diff --git a/internal/config/reports.go b/internal/config/reports.go index a901170..b2c37da 100644 --- a/internal/config/reports.go +++ b/internal/config/reports.go @@ -11,23 +11,24 @@ import ( "gopkg.in/yaml.v3" ) -func (cfg Config) ReportModuleOverrides() map[report.ID][]module.ConfigItem { +func (cfg Config) ReportModuleOverrides() (map[report.ID][]module.ConfigItem, error) { overrides := map[report.ID][]module.ConfigItem{} + seenReports := map[report.ID]string{} for key, reportCfg := range cfg.Reports { + id, err := report.IDForConfigKey(key) + if err != nil { + return nil, fmt.Errorf("reports.%s: %w", key, err) + } + if previous, ok := seenReports[id]; ok { + return nil, fmt.Errorf("reports.%s duplicates report override %q", key, previous) + } + seenReports[id] = key if !reportCfg.deterministicModulesSet { continue } - id, err := report.IDForConfigKey(key) - if err != nil { - continue - } - items := make([]module.ConfigItem, 0, len(reportCfg.DeterministicModules)) - for _, item := range reportCfg.DeterministicModules { - items = append(items, module.ConfigItem{ID: item.ID, Options: item.Options}) - } - overrides[id] = items + overrides[id] = moduleItemsFromConfig(reportCfg.DeterministicModules) } - return overrides + return overrides, nil } func normalizeReportModules(cfg *Config) error { @@ -55,15 +56,14 @@ func normalizeReportModules(cfg *Config) error { if !reportCfg.deterministicModulesSet { continue } - items := make([]module.ConfigItem, 0, len(reportCfg.DeterministicModules)) for i, rawItem := range reportCfg.DeterministicModules { options, err := normalizeModuleOptions(moduleRegistry, rawItem.ID, rawItem.Options) if err != nil { return fmt.Errorf("reports.%s.deterministic_modules[%d]: %w", key, i, err) } reportCfg.DeterministicModules[i].Options = options - items = append(items, module.ConfigItem{ID: rawItem.ID, Options: options}) } + items := moduleItemsFromConfig(reportCfg.DeterministicModules) if err := moduleRegistry.ValidateComposition(reportID, items); err != nil { return fmt.Errorf("reports.%s.deterministic_modules: %w", key, err) } @@ -72,6 +72,58 @@ func normalizeReportModules(cfg *Config) error { return nil } +func validateReportModules(cfg Config) error { + if cfg.Reports == nil { + return nil + } + moduleRegistry, err := briefing.DefaultModuleRegistry() + if err != nil { + return fmt.Errorf("initialize module registry: %w", err) + } + reportRegistry := report.DefaultRegistry() + seenReports := map[report.ID]string{} + for key, reportCfg := range cfg.Reports { + reportID, err := report.IDForConfigKey(key) + if err != nil { + return fmt.Errorf("reports.%s: %w", key, err) + } + if previous, ok := seenReports[reportID]; ok { + return fmt.Errorf("reports.%s duplicates report override %q", key, previous) + } + seenReports[reportID] = key + if _, err := reportRegistry.Lookup(reportID); err != nil { + return fmt.Errorf("reports.%s: %w", key, err) + } + if !reportCfg.deterministicModulesSet { + continue + } + items := make([]module.ConfigItem, 0, len(reportCfg.DeterministicModules)) + for i, rawItem := range reportCfg.DeterministicModules { + options := rawItem.Options + if options != nil { + normalized, err := normalizeModuleOptions(moduleRegistry, rawItem.ID, options) + if err != nil { + return fmt.Errorf("reports.%s.deterministic_modules[%d]: %w", key, i, err) + } + options = normalized + } + items = append(items, module.ConfigItem{ID: rawItem.ID, Options: options}) + } + if err := moduleRegistry.ValidateComposition(reportID, items); err != nil { + return fmt.Errorf("reports.%s.deterministic_modules: %w", key, err) + } + } + return nil +} + +func moduleItemsFromConfig(items []ModuleConfigItem) []module.ConfigItem { + out := make([]module.ConfigItem, 0, len(items)) + for _, item := range items { + out = append(out, module.ConfigItem{ID: item.ID, Options: item.Options}) + } + return out +} + func normalizeModuleOptions(registry briefing.ModuleRegistry, id module.ID, raw any) (any, error) { if raw == nil { return nil, nil @@ -83,6 +135,9 @@ func normalizeModuleOptions(registry briefing.ModuleRegistry, id module.ID, raw if definition.DefaultOptions == nil { return nil, fmt.Errorf("module %q does not accept options", id) } + if err := definition.ValidateOptions(raw); err == nil { + return raw, nil + } optionType := reflect.TypeOf(definition.DefaultOptions) normalized, err := decodeKnownOptions(raw, optionType) if err != nil { diff --git a/internal/config/validate.go b/internal/config/validate.go index 4964329..37d5bbd 100644 --- a/internal/config/validate.go +++ b/internal/config/validate.go @@ -9,7 +9,7 @@ import ( ) func Validate(cfg Config) error { - if err := normalizeReportModules(&cfg); err != nil { + if err := validateReportModules(cfg); err != nil { return err } if cfg.WeatherAPI.BaseURL != "" {