diff --git a/internal/app/app.go b/internal/app/app.go index 2e830a2..83285ef 100644 --- a/internal/app/app.go +++ b/internal/app/app.go @@ -91,6 +91,7 @@ type ReportRequest struct { Renderer Renderer Store state.Store Notifier Notifier + noNotify bool } type ReportResult struct { @@ -331,6 +332,7 @@ func RunBatchDetailed(ctx context.Context, req BatchRequest) (*BatchResult, erro Renderer: req.Renderer, Store: store, Notifier: req.Notifier, + noNotify: true, }) if err != nil { item.Status = "failed" @@ -619,6 +621,7 @@ func GenerateReport(ctx context.Context, req ReportRequest) (*ReportResult, erro OutputPath: req.OutputPath, Notifier: req.Notifier, GenerationErr: runErr, + noNotify: req.noNotify, }) if err != nil { return nil, err @@ -741,6 +744,7 @@ func generateTextTemplateReport(ctx context.Context, req generatedReportRequest) ManagedReportPath: reportPath, OutputPath: req.OutputPath, Notifier: req.Notifier, + noNotify: req.noNotify, }) if err != nil { return nil, err @@ -778,6 +782,7 @@ type finalizeRenderedReportRequest struct { OutputPath string Notifier Notifier GenerationErr error + noNotify bool } type finalizeRenderedReportResult struct { @@ -820,6 +825,9 @@ func finalizeRenderedReport(ctx context.Context, req finalizeRenderedReportReque if req.GenerationErr != nil { return result, req.GenerationErr } + if req.noNotify { + return result, nil + } notification, notificationPath, err := notifyReport(ctx, req.Config, req.Resolved, req.ManagedReportPath, metadata, req.Notifier, req.Store) if notificationPath != "" { diff --git a/internal/app/app_test.go b/internal/app/app_test.go index 669c1d1..a7c7512 100644 --- a/internal/app/app_test.go +++ b/internal/app/app_test.go @@ -2497,92 +2497,54 @@ func TestRunBatchContinuesAfterReportFailure(t *testing.T) { } } -func TestRunBatchContinuesAfterNotificationFailure(t *testing.T) { +func TestRunBatchSuppressesPerReportNotification(t *testing.T) { server := dailyBundleServer(t) cfg := dailyNotificationConfig(t, server) collection := collectionWithFutureDailyForTest(t, cfg, "2026-05-31") cfg.WeatherAPI.BaseURL = "" collector := &recordingCollector{result: &collection} - notifier := &recordingNotifier{ - errByReport: map[report.ID]error{ - report.Tomorrow: errors.New("distributor unavailable"), - }, - } + store := recordingFilesystemStore(t, cfg) + notifier := &recordingNotifier{err: errors.New("distributor unavailable")} result, err := RunBatchDetailed(context.Background(), BatchRequest{ Config: cfg, - Batch: BatchMorning, - Now: mustParse("2026-05-29T05:00:00-05:00"), + Batch: BatchEvening, + Now: mustParse("2026-05-29T18:00:00-05:00"), Collector: collector, Renderer: &selectiveRenderer{runBody: "# Batch Report\n"}, + Store: store, Notifier: notifier, }) if err != nil { t.Fatalf("RunBatchDetailed() error = %v", err) } - if result.Total != 3 || result.Succeeded != 2 || result.Failed != 1 { - t.Fatalf("summary total/succeeded/failed = %d/%d/%d, want 3/2/1", result.Total, result.Succeeded, result.Failed) + if result.Total != 2 || result.Succeeded != 2 || result.Failed != 0 { + t.Fatalf("summary total/succeeded/failed = %d/%d/%d, want 2/2/0", result.Total, result.Succeeded, result.Failed) } if len(collector.requests) != 1 { t.Fatalf("collector requests = %d, want one collection for batch", len(collector.requests)) } - if len(notifier.requests) != 3 { - t.Fatalf("notification requests = %d, want one per generated report", len(notifier.requests)) + if len(notifier.requests) != 0 { + t.Fatalf("notification requests = %#v, want none for batch-generated reports", notifier.requests) } - var failedTomorrow bool for _, item := range result.Reports { - if item.ReportID == report.Tomorrow { - if item.Status == "failed" && strings.Contains(item.Error, "notify report") && strings.Contains(item.Error, "distributor unavailable") { - failedTomorrow = true - } - if item.NotificationStatus != "failed" { - t.Fatalf("Tomorrow notification status = %q, want failed", item.NotificationStatus) - } - if item.NotificationPipelineID != "weatherreporter.tomorrow" { - t.Fatalf("Tomorrow notification pipeline = %q, want weatherreporter.tomorrow", item.NotificationPipelineID) - } - if !strings.Contains(item.NotificationError, "distributor unavailable") { - t.Fatalf("Tomorrow notification error = %q, want distributor unavailable", item.NotificationError) - } - continue - } if item.Status != "succeeded" { t.Fatalf("report %s status = %s, want succeeded", item.ReportID, item.Status) } - if item.NotificationStatus != "accepted" { - t.Fatalf("report %s notification status = %q, want accepted", item.ReportID, item.NotificationStatus) + if item.NotificationStatus != "" || item.NotificationRunID != "" || item.NotificationPipelineID != "" || item.NotificationError != "" || item.NotificationPath != "" { + t.Fatalf("report %s notification fields = %#v, want empty per-report notification fields", item.ReportID, item) } - if item.NotificationPipelineID == "" { - t.Fatalf("report %s notification pipeline is empty", item.ReportID) + metadata := readMetadataForTest(t, item.MetadataPath) + if metadata.NotificationPath != "" { + t.Fatalf("report %s metadata NotificationPath = %q, want empty", item.ReportID, metadata.NotificationPath) } } - if !failedTomorrow { - t.Fatalf("reports = %#v, want notification failure on Tomorrow item", result.Reports) - } - - cfg.Workspace.Root = t.TempDir() - collector = &recordingCollector{result: &collection} - err = RunBatch(context.Background(), BatchRequest{ - Config: cfg, - Batch: BatchMorning, - Now: mustParse("2026-05-29T05:00:00-05:00"), - Collector: collector, - Renderer: &selectiveRenderer{ - runBody: "# Batch Report\n", - }, - Notifier: &recordingNotifier{ - errByReport: map[report.ID]error{ - report.Tomorrow: errors.New("distributor unavailable"), - }, - }, - }) - var batchErr BatchError - if !errors.As(err, &batchErr) { - t.Fatalf("RunBatch() error = %T %v, want BatchError", err, err) - } - if batchErr.Result == nil || batchErr.Result.Failed != 1 || batchErr.Result.Succeeded != 2 { - t.Fatalf("RunBatch() result = %#v, want notification failure aggregate", batchErr.Result) + notificationDir := filepath.Join(cfg.Workspace.Root, cfg.Workspace.NotificationsDir) + if _, err := os.Stat(notificationDir); err == nil { + t.Fatalf("notification directory %q exists, want no per-report notification artifacts", notificationDir) + } else if !os.IsNotExist(err) { + t.Fatalf("stat notification directory %q: %v", notificationDir, err) } } @@ -2664,8 +2626,8 @@ func TestRunBatchDynamicDailyReportsHaveDistinctIdentity(t *testing.T) { if result.Failed != 0 || len(result.Reports) != 3 { t.Fatalf("summary = %#v, want three successful reports", result) } - if len(notifier.requests) != 3 { - t.Fatalf("notification requests = %d, want one per report", len(notifier.requests)) + if len(notifier.requests) != 0 { + t.Fatalf("notification requests = %#v, want none for batch-generated reports", notifier.requests) } dailyByDate := map[string]BatchReportResult{} @@ -2713,22 +2675,6 @@ func TestRunBatchDynamicDailyReportsHaveDistinctIdentity(t *testing.T) { } } - dailyKeys := map[string]struct{}{} - for _, req := range notifier.requests { - if req.ReportID != report.Daily { - continue - } - if req.IdempotencyKey != req.BundleID+"."+req.RunID { - t.Fatalf("Daily idempotency key = %q, want bundle id plus run id", req.IdempotencyKey) - } - if _, ok := dailyKeys[req.IdempotencyKey]; ok { - t.Fatalf("duplicate Daily idempotency key: %q", req.IdempotencyKey) - } - dailyKeys[req.IdempotencyKey] = struct{}{} - } - if len(dailyKeys) != 2 { - t.Fatalf("Daily notification keys = %#v, want two distinct keys", dailyKeys) - } } func TestRunBatchMorningUsesTodayOutputName(t *testing.T) { diff --git a/internal/cli/root_test.go b/internal/cli/root_test.go index a2cf6e0..d5a4668 100644 --- a/internal/cli/root_test.go +++ b/internal/cli/root_test.go @@ -321,20 +321,10 @@ func TestRunEveningUsesOutputDirectoryAndSummary(t *testing.T) { } } -func TestRunEveningReportsNotificationSuccess(t *testing.T) { +func TestRunEveningReportsOmitsPerReportNotification(t *testing.T) { server := dailyServer(t) distributorServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - if r.URL.Path == "/runs/distributor-run-1" { - w.Header().Set("Content-Type", "application/json") - _, _ = w.Write([]byte(`{"run_id":"distributor-run-1","pipeline_id":"weatherreporter.tomorrow","status":"succeeded","report":{"actions":[{"action":"replace_older"}]}}`)) - return - } - if r.URL.Path != "/v1/pipelines/weatherreporter.tomorrow/upload" { - http.NotFound(w, r) - return - } - w.WriteHeader(http.StatusAccepted) - _, _ = w.Write([]byte(`{"run_id":"distributor-run-1","status":"accepted"}`)) + t.Fatalf("unexpected distributor request %s", r.URL.Path) })) t.Cleanup(distributorServer.Close) tempDir := t.TempDir() @@ -361,29 +351,27 @@ func TestRunEveningReportsNotificationSuccess(t *testing.T) { if len(summary.Reports) != 1 { t.Fatalf("reports = %#v, want one report", summary.Reports) } - if summary.Reports[0].NotificationStatus != "succeeded" || summary.Reports[0].NotificationRunID != "distributor-run-1" || summary.Reports[0].NotificationPipelineID != "weatherreporter.tomorrow" { - t.Fatalf("notification fields = %#v", summary.Reports[0]) + if summary.Reports[0].NotificationStatus != "" || summary.Reports[0].NotificationRunID != "" || summary.Reports[0].NotificationPipelineID != "" || summary.Reports[0].NotificationError != "" || summary.Reports[0].NotificationPath != "" { + t.Fatalf("notification fields = %#v, want empty per-report notification fields", summary.Reports[0]) } - if !strings.Contains(stderr.String(), `notificationStatus="succeeded"`) || !strings.Contains(stderr.String(), `notificationRunId="distributor-run-1"`) { - t.Fatalf("stderr missing notification fields:\n%s", stderr.String()) + if strings.Contains(stderr.String(), "notificationStatus") || strings.Contains(stderr.String(), "notificationRunId") { + t.Fatalf("stderr includes per-report notification fields:\n%s", stderr.String()) } if strings.Contains(stdout.String(), "cli-secret-token") || strings.Contains(stderr.String(), "cli-secret-token") { t.Fatalf("output contains token value\nstdout=%s\nstderr=%s", stdout.String(), stderr.String()) } } -func TestRunEveningReportsNotificationFailureWithoutToken(t *testing.T) { +func TestRunEveningReportsDoesNotRequirePerReportDistributorToken(t *testing.T) { server := dailyServer(t) distributorServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - w.WriteHeader(http.StatusBadRequest) - _, _ = w.Write([]byte(`{"error":"rejected cli-secret-token","retryable":false}`)) + t.Fatalf("unexpected distributor request %s", r.URL.Path) })) t.Cleanup(distributorServer.Close) tempDir := t.TempDir() scriptoriumPath := writeFakeScriptorium(t, tempDir) workspaceRoot := filepath.Join(tempDir, "workspace") configPath := writeTestConfigWithDistributor(t, server, scriptoriumPath, workspaceRoot, distributorServer.URL) - t.Setenv("CLI_DISTRIBUTOR_TOKEN", "cli-secret-token") var stdout bytes.Buffer var stderr bytes.Buffer runner := Runner{Clock: fixedClock()} @@ -392,26 +380,19 @@ func TestRunEveningReportsNotificationFailureWithoutToken(t *testing.T) { "run", "evening", "--config", configPath, }, &stdout, &stderr) - if err == nil { - t.Fatal("Run() error = nil, want notification failure") + if err != nil { + t.Fatalf("Run() error = %v", err) } var summary app.BatchResult if decodeErr := json.Unmarshal(stdout.Bytes(), &summary); decodeErr != nil { t.Fatalf("decode summary: %v\n%s", decodeErr, stdout.String()) } - if len(summary.Reports) != 1 || summary.Reports[0].NotificationStatus != "failed" { - t.Fatalf("summary reports = %#v, want failed notification", summary.Reports) + if len(summary.Reports) != 1 { + t.Fatalf("summary reports = %#v, want one report", summary.Reports) } - for _, output := range []string{stdout.String(), stderr.String(), err.Error()} { - if strings.Contains(output, "cli-secret-token") { - t.Fatalf("output contains token value:\n%s", output) - } - } - for _, output := range []string{stdout.String(), stderr.String()} { - if !strings.Contains(output, "[redacted]") { - t.Fatalf("output missing redaction marker:\n%s", output) - } + if summary.Reports[0].NotificationStatus != "" || summary.Reports[0].NotificationError != "" { + t.Fatalf("notification fields = %#v, want empty per-report notification fields", summary.Reports[0]) } }