diff --git a/internal/app/app.go b/internal/app/app.go index d8f9bed..e53961b 100644 --- a/internal/app/app.go +++ b/internal/app/app.go @@ -87,7 +87,7 @@ type ReportRequest struct { Config config.Config Resolved report.Resolved OutputPath string - Collector Collector + Collection collect.Result Renderer Renderer Store state.Store Notifier Notifier @@ -228,6 +228,10 @@ func Generate(ctx context.Context, req GenerateRequest) error { if now.IsZero() { now = time.Now() } + collection, err := collectWeather(ctx, req.Config, req.Collector) + if err != nil { + return err + } resolved, err := ResolveGenerate(req, now) if err != nil { return err @@ -237,7 +241,7 @@ func Generate(ctx context.Context, req GenerateRequest) error { Config: req.Config, Resolved: resolved, OutputPath: req.OutputPath, - Collector: req.Collector, + Collection: *collection, Notifier: req.Notifier, }) return err @@ -289,12 +293,20 @@ func RunBatchDetailed(ctx context.Context, req BatchRequest) (*BatchResult, erro item.ReportPath = paths.RenderedReport item.MetadataPath = paths.Metadata } + collection, err := collectWeather(ctx, req.Config, req.Collector) + if err != nil { + item.Status = "failed" + item.Error = err.Error() + result.Failed++ + result.Reports = append(result.Reports, item) + continue + } outputPath := batchOutputPath(req.OutputDir, resolved.Definition) reportResult, err := GenerateReport(ctx, ReportRequest{ Config: req.Config, Resolved: resolved, OutputPath: outputPath, - Collector: req.Collector, + Collection: *collection, Renderer: req.Renderer, Store: store, Notifier: req.Notifier, @@ -409,10 +421,14 @@ func reportRegistry(cfg config.Config) (report.Registry, error) { } func FetchBundle(ctx context.Context, req FetchBundleRequest) (*weatherdata.Bundle, error) { - return collectBundle(ctx, req.Config, nil) + result, err := collectWeather(ctx, req.Config, nil) + if err != nil { + return nil, err + } + return result.Bundle, nil } -func collectBundle(ctx context.Context, cfg config.Config, collector Collector) (*weatherdata.Bundle, error) { +func collectWeather(ctx context.Context, cfg config.Config, collector Collector) (*collect.Result, error) { if collector == nil { collector = defaultCollector{} } @@ -426,7 +442,7 @@ func collectBundle(ctx context.Context, cfg config.Config, collector Collector) if result.Bundle == nil { return nil, fmt.Errorf("collect weather bundle: collector returned nil bundle") } - return result.Bundle, nil + return result, nil } func FetchAndSaveBundle(ctx context.Context, req FetchBundleRequest) (*weatherdata.Bundle, error) { @@ -444,6 +460,11 @@ func FetchAndSaveBundle(ctx context.Context, req FetchBundleRequest) (*weatherda } func GenerateReport(ctx context.Context, req ReportRequest) (*ReportResult, error) { + bundle := req.Collection.Bundle + if bundle == nil { + return nil, fmt.Errorf("collected weather bundle is required") + } + store := req.Store if store == nil { defaultStore, err := defaultStore(req.Config) @@ -461,10 +482,6 @@ func GenerateReport(ctx context.Context, req ReportRequest) (*ReportResult, erro return nil, err } - bundle, err := collectBundle(ctx, req.Config, req.Collector) - if err != nil { - return nil, err - } reportFacts, err := BuildReportFacts(ModuleSnapshotRequest{ Config: req.Config, Resolved: req.Resolved, diff --git a/internal/app/app_test.go b/internal/app/app_test.go index 4ecb7a0..7029127 100644 --- a/internal/app/app_test.go +++ b/internal/app/app_test.go @@ -109,6 +109,37 @@ func TestGenerateUsesProvidedCollector(t *testing.T) { } } +func TestGenerateCollectsOnceForSingleReport(t *testing.T) { + server := dailyBundleServer(t) + cfg := dailyWorkspaceConfig(t, server) + collection := collectionForTest(t, cfg) + collector := &recordingCollector{result: &collection} + markerPath := filepath.Join(t.TempDir(), "scriptorium-called") + binaryPath := filepath.Join(t.TempDir(), "scriptorium") + script := fmt.Sprintf("#!/bin/sh\nprintf called > %q\nprintf render failed >&2\nexit 1\n", markerPath) + if err := os.WriteFile(binaryPath, []byte(script), 0o755); err != nil { + t.Fatalf("write scriptorium marker script: %v", err) + } + cfg.Scriptorium.Binary = binaryPath + + err := Generate(context.Background(), GenerateRequest{ + Config: cfg, + Report: ReportDaily, + Date: mustParse("2026-05-29T12:00:00-05:00"), + Now: mustParse("2026-05-29T05:00:00-05:00"), + Collector: collector, + }) + if err == nil { + t.Fatal("Generate() error = nil, want render error") + } + if len(collector.requests) != 1 { + t.Fatalf("collector requests = %d, want 1", len(collector.requests)) + } + if _, statErr := os.Stat(markerPath); statErr != nil { + t.Fatalf("scriptorium marker stat error = %v, want report execution reached renderer", statErr) + } +} + func TestGenerateCollectionFailureStopsBeforeReportExecution(t *testing.T) { cfg := config.Defaults() cfg.WeatherAPI.BaseURL = "" @@ -168,6 +199,7 @@ func TestGenerateReportWritesReportAndPreflight(t *testing.T) { result, err := GenerateReport(context.Background(), ReportRequest{ Config: cfg, + Collection: collectionForTest(t, cfg), Resolved: resolved, OutputPath: outputPath, Renderer: renderer, @@ -397,9 +429,10 @@ func TestGeneratedTemplateReportsUseRichArtifactsAndCuratedDataPackages(t *testi renderer := successfulGeneratedTextRenderer("") result, err := GenerateReport(context.Background(), ReportRequest{ - Config: cfg, - Resolved: resolved, - Renderer: renderer, + Config: cfg, + Collection: collectionForTest(t, cfg), + Resolved: resolved, + Renderer: renderer, }) if err != nil { t.Fatalf("GenerateReport() error = %v", err) @@ -542,10 +575,11 @@ func TestGenerateHourlyReportUsesGeneratedTextTemplateWorkflow(t *testing.T) { } result, err := GenerateReport(context.Background(), ReportRequest{ - Config: cfg, - Resolved: resolved, - Renderer: renderer, - Store: store, + Config: cfg, + Collection: collectionForTest(t, cfg), + Resolved: resolved, + Renderer: renderer, + Store: store, }) if err != nil { t.Fatalf("GenerateReport() error = %v", err) @@ -670,6 +704,7 @@ func TestGenerateHourlyReportCopiesOutputAndNotifiesManagedReport(t *testing.T) result, err := GenerateReport(context.Background(), ReportRequest{ Config: cfg, + Collection: collectionForTest(t, cfg), Resolved: resolved, OutputPath: outputPath, Renderer: renderer, @@ -756,6 +791,7 @@ func TestGenerateTodayReportCopiesOutputAndNotifiesTodayTemplateValues(t *testin result, err := GenerateReport(context.Background(), ReportRequest{ Config: cfg, + Collection: collectionForTest(t, cfg), Resolved: resolved, OutputPath: outputPath, Renderer: renderer, @@ -832,6 +868,7 @@ func TestGenerateHourlyReportNotificationFailureFailsReport(t *testing.T) { _, err := GenerateReport(context.Background(), ReportRequest{ Config: cfg, + Collection: collectionForTest(t, cfg), Resolved: resolved, OutputPath: outputPath, Renderer: renderer, @@ -883,9 +920,10 @@ func TestGenerateReportSavesFinalMetadataForMarkdownAndGeneratedTextReports(t *t } result, err := GenerateReport(context.Background(), ReportRequest{ - Config: cfg, - Resolved: resolved, - Renderer: successfulRenderer("# 3-Day Outlook\n"), + Config: cfg, + Collection: collectionForTest(t, cfg), + Resolved: resolved, + Renderer: successfulRenderer("# 3-Day Outlook\n"), }) if err != nil { t.Fatalf("GenerateReport() error = %v", err) @@ -912,10 +950,11 @@ func TestGenerateReportSavesFinalMetadataForMarkdownAndGeneratedTextReports(t *t } result, err := GenerateReport(context.Background(), ReportRequest{ - Config: cfg, - Resolved: resolved, - Renderer: renderer, - Store: store, + Config: cfg, + Collection: collectionForTest(t, cfg), + Resolved: resolved, + Renderer: renderer, + Store: store, }) if err != nil { t.Fatalf("GenerateReport() error = %v", err) @@ -950,10 +989,11 @@ func TestGenerateTomorrowReportNotificationUsesTomorrowTemplateValues(t *testing renderer := successfulGeneratedTextRenderer(validTomorrowGeneratedTextJSON()) result, err := GenerateReport(context.Background(), ReportRequest{ - Config: cfg, - Resolved: resolved, - Renderer: renderer, - Notifier: notifier, + Config: cfg, + Collection: collectionForTest(t, cfg), + Resolved: resolved, + Renderer: renderer, + Notifier: notifier, }) if err != nil { t.Fatalf("GenerateReport() error = %v", err) @@ -993,6 +1033,7 @@ func TestGenerateHourlyReportPersistsPreflightFailure(t *testing.T) { _, err := GenerateReport(context.Background(), ReportRequest{ Config: cfg, + Collection: collectionForTest(t, cfg), Resolved: resolved, OutputPath: outputPath, Renderer: renderer, @@ -1039,6 +1080,7 @@ func TestGenerateHourlyReportPersistsStructuredRunFailure(t *testing.T) { _, err := GenerateReport(context.Background(), ReportRequest{ Config: cfg, + Collection: collectionForTest(t, cfg), Resolved: resolved, OutputPath: outputPath, Renderer: renderer, @@ -1073,6 +1115,7 @@ func TestGenerateHourlyReportPreservesRawTextOnValidationFailure(t *testing.T) { _, err := GenerateReport(context.Background(), ReportRequest{ Config: cfg, + Collection: collectionForTest(t, cfg), Resolved: resolved, OutputPath: outputPath, Renderer: renderer, @@ -1101,6 +1144,7 @@ func TestGenerateHourlyReportRejectsUnsupportedTemplateBeforeStructuredRun(t *te _, err := GenerateReport(context.Background(), ReportRequest{ Config: cfg, + Collection: collectionForTest(t, cfg), Resolved: resolved, OutputPath: outputPath, Renderer: renderer, @@ -1135,10 +1179,11 @@ func TestGenerateReportDisabledNotificationDoesNotCallNotifier(t *testing.T) { notifier := &recordingNotifier{} _, err := GenerateReport(context.Background(), ReportRequest{ - Config: cfg, - Resolved: resolved, - Renderer: successfulRenderer("# Daily Report\n"), - Notifier: notifier, + Config: cfg, + Collection: collectionForTest(t, cfg), + Resolved: resolved, + Renderer: successfulRenderer("# Daily Report\n"), + Notifier: notifier, }) if err != nil { t.Fatalf("GenerateReport() error = %v", err) @@ -1162,6 +1207,7 @@ func TestGenerateReportNotifiesManagedReportPath(t *testing.T) { result, err := GenerateReport(context.Background(), ReportRequest{ Config: cfg, + Collection: collectionForTest(t, cfg), Resolved: resolved, OutputPath: outputPath, Renderer: successfulRenderer("# Daily Report\n"), @@ -1234,11 +1280,12 @@ func TestGenerateReportNotificationFailureFailsReport(t *testing.T) { store := recordingFilesystemStore(t, cfg) _, err := GenerateReport(context.Background(), ReportRequest{ - Config: cfg, - Resolved: resolved, - Renderer: successfulRenderer("# Daily Report\n"), - Store: store, - Notifier: notifier, + Config: cfg, + Collection: collectionForTest(t, cfg), + Resolved: resolved, + Renderer: successfulRenderer("# Daily Report\n"), + Store: store, + Notifier: notifier, }) if err == nil { t.Fatal("GenerateReport() error = nil, want notification error") @@ -1299,10 +1346,11 @@ func TestGenerateReportDoesNotNotifyAfterRenderOrRunFailure(t *testing.T) { notifier := &recordingNotifier{} _, err := GenerateReport(context.Background(), ReportRequest{ - Config: cfg, - Resolved: resolved, - Renderer: tt.renderer, - Notifier: notifier, + Config: cfg, + Collection: collectionForTest(t, cfg), + Resolved: resolved, + Renderer: tt.renderer, + Notifier: notifier, }) if err == nil { t.Fatal("GenerateReport() error = nil, want generation error") @@ -1314,7 +1362,7 @@ func TestGenerateReportDoesNotNotifyAfterRenderOrRunFailure(t *testing.T) { } } -func TestGenerateReportDoesNotNotifyAfterFetchFailure(t *testing.T) { +func TestGenerateReportRequiresCollectedBundleBeforeStateWrites(t *testing.T) { cfg := config.Defaults() cfg.WeatherAPI.BaseURL = "" cfg.WeatherAPI.Timezone = "America/Chicago" @@ -1326,18 +1374,26 @@ func TestGenerateReportDoesNotNotifyAfterFetchFailure(t *testing.T) { Date: mustParse("2026-05-29T12:00:00-05:00"), }, "2026-05-29T05:00:00-05:00") notifier := &recordingNotifier{} + store := recordingFilesystemStore(t, cfg) _, err := GenerateReport(context.Background(), ReportRequest{ Config: cfg, Resolved: resolved, Renderer: successfulRenderer("# Daily Report\n"), + Store: store, Notifier: notifier, }) if err == nil { - t.Fatal("GenerateReport() error = nil, want fetch setup error") + t.Fatal("GenerateReport() error = nil, want collected bundle error") + } + if !strings.Contains(err.Error(), "collected weather bundle is required") { + t.Fatalf("GenerateReport() error = %q, want collected bundle context", err.Error()) } if len(notifier.requests) != 0 { - t.Fatalf("notification requests = %#v, want none after fetch failure", notifier.requests) + t.Fatalf("notification requests = %#v, want none without collected data", notifier.requests) + } + if len(store.calls) != 0 { + t.Fatalf("state calls = %#v, want no state writes without collected data", store.calls) } } @@ -1358,9 +1414,10 @@ func TestGenerateReportPersistsFailedPreflight(t *testing.T) { } _, err := GenerateReport(context.Background(), ReportRequest{ - Config: cfg, - Resolved: resolved, - Renderer: renderer, + Config: cfg, + Collection: collectionForTest(t, cfg), + Resolved: resolved, + Renderer: renderer, }) if err == nil { t.Fatal("GenerateReport() error = nil, want render error") @@ -1403,9 +1460,10 @@ func TestGenerateReportReturnsRunErrorAfterPreflight(t *testing.T) { } _, err := GenerateReport(context.Background(), ReportRequest{ - Config: cfg, - Resolved: resolved, - Renderer: renderer, + Config: cfg, + Collection: collectionForTest(t, cfg), + Resolved: resolved, + Renderer: renderer, }) if err == nil { t.Fatal("GenerateReport() error = nil, want run error") @@ -1450,10 +1508,11 @@ func TestGenerateReportIncludesRecentChangesFromPriorSnapshot(t *testing.T) { } result, err := GenerateReport(context.Background(), ReportRequest{ - Config: cfg, - Resolved: currentResolved, - Renderer: renderer, - Store: store, + Config: cfg, + Collection: collectionForTest(t, cfg), + Resolved: currentResolved, + Renderer: renderer, + Store: store, }) if err != nil { t.Fatalf("GenerateReport() error = %v", err) @@ -1487,10 +1546,11 @@ func TestGenerateTodayReportUsesTodayIdentityAndRecentChanges(t *testing.T) { renderer := successfulGeneratedTextRenderer(validTodayGeneratedTextJSON()) result, err := GenerateReport(context.Background(), ReportRequest{ - Config: cfg, - Resolved: currentResolved, - Renderer: renderer, - Store: store, + Config: cfg, + Collection: collectionForTest(t, cfg), + Resolved: currentResolved, + Renderer: renderer, + Store: store, }) if err != nil { t.Fatalf("GenerateReport() error = %v", err) @@ -1548,9 +1608,10 @@ func TestGenerateTomorrowReportUsesTomorrowBriefingDate(t *testing.T) { renderer := successfulGeneratedTextRenderer(validTomorrowGeneratedTextJSON()) result, err := GenerateReport(context.Background(), ReportRequest{ - Config: cfg, - Resolved: resolved, - Renderer: renderer, + Config: cfg, + Collection: collectionForTest(t, cfg), + Resolved: resolved, + Renderer: renderer, }) if err != nil { t.Fatalf("GenerateReport() error = %v", err) @@ -1620,10 +1681,11 @@ func TestTomorrowReportCanCompareAgainstPriorTomorrowSnapshot(t *testing.T) { renderer := successfulGeneratedTextRenderer(validTomorrowGeneratedTextJSON()) result, err := GenerateReport(context.Background(), ReportRequest{ - Config: cfg, - Resolved: currentResolved, - Renderer: renderer, - Store: store, + Config: cfg, + Collection: collectionForTest(t, cfg), + Resolved: currentResolved, + Renderer: renderer, + Store: store, }) if err != nil { t.Fatalf("GenerateReport() error = %v", err) @@ -1650,10 +1712,11 @@ func TestDailyReportIgnoresPriorTomorrowSnapshot(t *testing.T) { Date: mustParse("2026-05-29T12:00:00-05:00"), }, "2026-05-29T05:00:00-05:00") result, err := GenerateReport(context.Background(), ReportRequest{ - Config: cfg, - Resolved: currentResolved, - Renderer: successfulRenderer("# Daily Report\n"), - Store: store, + Config: cfg, + Collection: collectionForTest(t, cfg), + Resolved: currentResolved, + Renderer: successfulRenderer("# Daily Report\n"), + Store: store, }) if err != nil { t.Fatalf("GenerateReport() error = %v", err) @@ -1684,10 +1747,11 @@ func TestGenerateThreeDayReportWritesReportAndRecentChanges(t *testing.T) { } result, err := GenerateReport(context.Background(), ReportRequest{ - Config: cfg, - Resolved: currentResolved, - Renderer: renderer, - Store: store, + Config: cfg, + Collection: collectionForTest(t, cfg), + Resolved: currentResolved, + Renderer: renderer, + Store: store, }) if err != nil { t.Fatalf("GenerateReport() error = %v", err) @@ -1729,10 +1793,11 @@ func TestGenerateWeekendReportWritesReportAndRecentChanges(t *testing.T) { } result, err := GenerateReport(context.Background(), ReportRequest{ - Config: cfg, - Resolved: currentResolved, - Renderer: renderer, - Store: store, + Config: cfg, + Collection: collectionForTest(t, cfg), + Resolved: currentResolved, + Renderer: renderer, + Store: store, }) if err != nil { t.Fatalf("GenerateReport() error = %v", err) @@ -1773,6 +1838,7 @@ func TestGenerateStormReportWritesReport(t *testing.T) { result, err := GenerateReport(context.Background(), ReportRequest{ Config: cfg, + Collection: collectionForTest(t, cfg), Resolved: resolved, OutputPath: outputPath, Renderer: renderer, @@ -1812,9 +1878,10 @@ func TestInspectGeneratedReportArtifacts(t *testing.T) { runBody: "# Daily Report\n", } result, err := GenerateReport(context.Background(), ReportRequest{ - Config: cfg, - Resolved: resolved, - Renderer: renderer, + Config: cfg, + Collection: collectionForTest(t, cfg), + Resolved: resolved, + Renderer: renderer, }) if err != nil { t.Fatalf("GenerateReport() error = %v", err) @@ -1877,10 +1944,10 @@ func TestInspectPriorSnapshot(t *testing.T) { runResult: &scriptorium.RunResult{ExitCode: 0}, runBody: "# Daily Report\n", } - if _, err := GenerateReport(context.Background(), ReportRequest{Config: cfg, Resolved: priorResolved, Renderer: renderer, Store: store}); err != nil { + if _, err := GenerateReport(context.Background(), ReportRequest{Config: cfg, Collection: collectionForTest(t, cfg), Resolved: priorResolved, Renderer: renderer, Store: store}); err != nil { t.Fatalf("GenerateReport(prior) error = %v", err) } - current, err := GenerateReport(context.Background(), ReportRequest{Config: cfg, Resolved: currentResolved, Renderer: renderer, Store: store}) + current, err := GenerateReport(context.Background(), ReportRequest{Config: cfg, Collection: collectionForTest(t, cfg), Resolved: currentResolved, Renderer: renderer, Store: store}) if err != nil { t.Fatalf("GenerateReport(current) error = %v", err) } @@ -2438,6 +2505,15 @@ func dailyNotificationConfig(t *testing.T, server *httptest.Server) config.Confi return cfg } +func collectionForTest(t *testing.T, cfg config.Config) collect.Result { + t.Helper() + bundle, err := FetchBundle(context.Background(), FetchBundleRequest{Config: cfg}) + if err != nil { + t.Fatalf("FetchBundle() error = %v", err) + } + return collect.Result{Bundle: bundle} +} + func resolveGenerateForTest(t *testing.T, cfg config.Config, req GenerateRequest, now string) report.Resolved { t.Helper() req.Config = cfg @@ -2568,9 +2644,10 @@ func generateDailyReportForTest(t *testing.T, cfg config.Config) *ReportResult { t.Fatalf("ResolveGenerate() error = %v", err) } result, err := GenerateReport(context.Background(), ReportRequest{ - Config: cfg, - Resolved: resolved, - Renderer: successfulRenderer("# Daily Report\n"), + Config: cfg, + Collection: collectionForTest(t, cfg), + Resolved: resolved, + Renderer: successfulRenderer("# Daily Report\n"), }) if err != nil { t.Fatalf("GenerateReport() error = %v", err)