From c573cd5b4d6fb6aaedd795525d17b8bbc7921cd1 Mon Sep 17 00:00:00 2001 From: Eric Rakestraw Date: Sun, 7 Jun 2026 23:42:27 +0000 Subject: [PATCH] Report distributor notification outcomes in batches --- docs/cli.md | 4 +- docs/operations.md | 13 +-- internal/app/app.go | 65 ++++++++++---- internal/app/app_test.go | 9 ++ internal/cli/root.go | 14 ++- internal/cli/root_test.go | 179 ++++++++++++++++++++++++++++++++++++++ 6 files changed, 261 insertions(+), 23 deletions(-) diff --git a/docs/cli.md b/docs/cli.md index eaa4a13..f031ea5 100644 --- a/docs/cli.md +++ b/docs/cli.md @@ -40,7 +40,9 @@ explicit event-window bounds with `--start` and `--end`. except on Sunday. `run evening` generates the Tomorrow Planning Brief. Batch runs continue independent reports after a failure, print a JSON summary to stdout, write compact status lines to stderr, and return nonzero when any report -failed. +failed. When notification is configured, batch summaries and status lines include +notification status, accepted distributor run ID, or notification error fields +for each attempted report. `inspect` commands read existing workspace artifacts and emit JSON to stdout. They do not fetch weather data or invoke `scriptorium`. diff --git a/docs/operations.md b/docs/operations.md index 3271f66..6607ecf 100644 --- a/docs/operations.md +++ b/docs/operations.md @@ -32,9 +32,11 @@ weatherreporter run evening except on Sunday. `run evening` generates the Tomorrow Planning Brief. Batch commands print a JSON summary to stdout, write compact per-report status lines to stderr, continue independent reports after one report fails, and return -nonzero when any report failed. `--out-dir PATH` writes extra Markdown copies -using report default filenames such as `daily.md`, `three-day.md`, -`weekend.md`, and `tomorrow.md`. +nonzero when any report failed. When notification is configured, the summary and +status lines include notification status, accepted distributor run ID, or +notification error fields for each attempted report. `--out-dir PATH` writes +extra Markdown copies using report default filenames such as `daily.md`, +`three-day.md`, `weekend.md`, and `tomorrow.md`. ## Filesystem Layout @@ -117,8 +119,9 @@ Each generated report writes metadata that links: - preflight output path - managed Markdown report path -Batch summaries include report status, error text when applicable, valid -period, and known artifact paths for each attempted report. +Batch summaries include report status, error text when applicable, notification +outcome when attempted, valid period, and known artifact paths for each +attempted report. ## Inspection diff --git a/internal/app/app.go b/internal/app/app.go index 88d8e66..1040738 100644 --- a/internal/app/app.go +++ b/internal/app/app.go @@ -3,6 +3,7 @@ package app import ( "context" + "errors" "fmt" "path/filepath" "time" @@ -112,20 +113,23 @@ type BatchResult struct { } type BatchReportResult struct { - ReportID report.ID `json:"reportId"` - ReportName string `json:"reportName"` - PromptID string `json:"promptId"` - RunID string `json:"runId"` - Status string `json:"status"` - Error string `json:"error,omitempty"` - GeneratedAt time.Time `json:"generatedAt"` - ValidPeriod timeutil.Period `json:"validPeriod"` - BriefingPath string `json:"briefingPath,omitempty"` - DataPackagePath string `json:"dataPackagePath,omitempty"` - PreflightPath string `json:"preflightPath,omitempty"` - ReportPath string `json:"reportPath,omitempty"` - OutputPath string `json:"outputPath,omitempty"` - MetadataPath string `json:"metadataPath,omitempty"` + ReportID report.ID `json:"reportId"` + ReportName string `json:"reportName"` + PromptID string `json:"promptId"` + RunID string `json:"runId"` + Status string `json:"status"` + Error string `json:"error,omitempty"` + NotificationStatus string `json:"notificationStatus,omitempty"` + NotificationRunID string `json:"notificationRunId,omitempty"` + NotificationError string `json:"notificationError,omitempty"` + GeneratedAt time.Time `json:"generatedAt"` + ValidPeriod timeutil.Period `json:"validPeriod"` + BriefingPath string `json:"briefingPath,omitempty"` + DataPackagePath string `json:"dataPackagePath,omitempty"` + PreflightPath string `json:"preflightPath,omitempty"` + ReportPath string `json:"reportPath,omitempty"` + OutputPath string `json:"outputPath,omitempty"` + MetadataPath string `json:"metadataPath,omitempty"` } type BatchError struct { @@ -164,6 +168,25 @@ type NotificationResult struct { Status string } +type NotificationError struct { + Request NotificationRequest + Err error +} + +func (e *NotificationError) Error() string { + if e == nil || e.Err == nil { + return "notification failed" + } + return e.Err.Error() +} + +func (e *NotificationError) Unwrap() error { + if e == nil { + return nil + } + return e.Err +} + func Generate(ctx context.Context, req GenerateRequest) error { now := req.Now if now.IsZero() { @@ -242,6 +265,11 @@ func RunBatchDetailed(ctx context.Context, req BatchRequest) (*BatchResult, erro if err != nil { item.Status = "failed" item.Error = err.Error() + var notificationErr *NotificationError + if errors.As(err, ¬ificationErr) { + item.NotificationStatus = "failed" + item.NotificationError = notificationErr.Error() + } result.Failed++ } else { item.Status = "succeeded" @@ -251,6 +279,10 @@ func RunBatchDetailed(ctx context.Context, req BatchRequest) (*BatchResult, erro item.ReportPath = reportResult.ReportPath item.OutputPath = reportResult.OutputPath item.MetadataPath = reportResult.MetadataPath + if reportResult.Notification != nil { + item.NotificationStatus = reportResult.Notification.Status + item.NotificationRunID = reportResult.Notification.RunID + } result.Succeeded++ } result.Reports = append(result.Reports, item) @@ -542,7 +574,10 @@ func notifyReport(ctx context.Context, cfg config.Config, resolved report.Resolv } result, err := notifier.Notify(ctx, notificationRequest) if err != nil { - return nil, fmt.Errorf("notify report %q run %q from managed report %q: %w", resolved.Definition.ID, metadata.RunID, reportPath, err) + return nil, &NotificationError{ + Request: notificationRequest, + Err: fmt.Errorf("notify report %q run %q from managed report %q: %w", resolved.Definition.ID, metadata.RunID, reportPath, err), + } } return result, nil } diff --git a/internal/app/app_test.go b/internal/app/app_test.go index 871f381..d5f9d4f 100644 --- a/internal/app/app_test.go +++ b/internal/app/app_test.go @@ -1247,11 +1247,20 @@ func TestRunBatchContinuesAfterNotificationFailure(t *testing.T) { if item.Status == "failed" && strings.Contains(item.Error, "notify report") && strings.Contains(item.Error, "distributor unavailable") { failedThreeDay = true } + if item.NotificationStatus != "failed" { + t.Fatalf("3-day notification status = %q, want failed", item.NotificationStatus) + } + if !strings.Contains(item.NotificationError, "distributor unavailable") { + t.Fatalf("3-day 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 !failedThreeDay { t.Fatalf("reports = %#v, want notification failure on 3-day item", result.Reports) diff --git a/internal/cli/root.go b/internal/cli/root.go index a274062..5924fa5 100644 --- a/internal/cli/root.go +++ b/internal/cli/root.go @@ -352,11 +352,21 @@ func writeRunLogs(stderr io.Writer, result *app.BatchResult) { return } for _, item := range result.Reports { + notificationFields := "" + if item.NotificationStatus != "" { + notificationFields += fmt.Sprintf(" notificationStatus=%q", item.NotificationStatus) + } + if item.NotificationRunID != "" { + notificationFields += fmt.Sprintf(" notificationRunId=%q", item.NotificationRunID) + } + if item.NotificationError != "" { + notificationFields += fmt.Sprintf(" notificationError=%q", item.NotificationError) + } if item.Status == "failed" { - _, _ = fmt.Fprintf(stderr, "report=%s status=failed error=%q\n", item.ReportID, item.Error) + _, _ = fmt.Fprintf(stderr, "report=%s status=failed error=%q%s\n", item.ReportID, item.Error, notificationFields) continue } - _, _ = fmt.Fprintf(stderr, "report=%s status=succeeded output=%q\n", item.ReportID, item.OutputPath) + _, _ = fmt.Fprintf(stderr, "report=%s status=succeeded output=%q%s\n", item.ReportID, item.OutputPath, notificationFields) } _, _ = fmt.Fprintf(stderr, "batch=%s total=%d succeeded=%d failed=%d\n", result.Batch, result.Total, result.Succeeded, result.Failed) } diff --git a/internal/cli/root_test.go b/internal/cli/root_test.go index 0554d2c..12d674c 100644 --- a/internal/cli/root_test.go +++ b/internal/cli/root_test.go @@ -346,6 +346,88 @@ func TestRunMorningReportsPartialFailureAndContinues(t *testing.T) { } } +func TestBatchOutputIncludesNotificationDetails(t *testing.T) { + result := &app.BatchResult{ + Batch: app.BatchMorning, + Total: 2, + Succeeded: 1, + Failed: 1, + Reports: []app.BatchReportResult{ + { + ReportID: "daily_today", + Status: "succeeded", + OutputPath: "/tmp/daily.md", + NotificationStatus: "accepted", + NotificationRunID: "distributor-run-1", + }, + { + ReportID: "three_day", + Status: "failed", + Error: "notify report three_day: upload failed", + NotificationStatus: "failed", + NotificationError: "notify report three_day: upload failed", + }, + }, + } + var stdout bytes.Buffer + var stderr bytes.Buffer + + if err := writeJSON(&stdout, result); err != nil { + t.Fatalf("writeJSON() error = %v", err) + } + writeRunLogs(&stderr, result) + + var decoded app.BatchResult + if err := json.Unmarshal(stdout.Bytes(), &decoded); err != nil { + t.Fatalf("decode batch JSON: %v\n%s", err, stdout.String()) + } + if decoded.Reports[0].NotificationStatus != "accepted" || decoded.Reports[0].NotificationRunID != "distributor-run-1" { + t.Fatalf("success notification fields = %#v", decoded.Reports[0]) + } + if decoded.Reports[1].NotificationStatus != "failed" || !strings.Contains(decoded.Reports[1].NotificationError, "upload failed") { + t.Fatalf("failure notification fields = %#v", decoded.Reports[1]) + } + if !strings.Contains(stderr.String(), `notificationStatus="accepted"`) || !strings.Contains(stderr.String(), `notificationRunId="distributor-run-1"`) { + t.Fatalf("stderr missing success notification fields:\n%s", stderr.String()) + } + if !strings.Contains(stderr.String(), `notificationStatus="failed"`) || !strings.Contains(stderr.String(), `notificationError="notify report three_day: upload failed"`) { + t.Fatalf("stderr missing failure notification fields:\n%s", stderr.String()) + } +} + +func TestBatchOutputDoesNotExposeSecretLikeNotificationErrors(t *testing.T) { + result := &app.BatchResult{ + Batch: app.BatchMorning, + Total: 1, + Failed: 1, + Reports: []app.BatchReportResult{ + { + ReportID: "daily_today", + Status: "failed", + Error: "notify report daily_today: upload failed: [redacted]", + NotificationStatus: "failed", + NotificationError: "notify report daily_today: upload failed: [redacted]", + }, + }, + } + var stdout bytes.Buffer + var stderr bytes.Buffer + + if err := writeJSON(&stdout, result); err != nil { + t.Fatalf("writeJSON() error = %v", err) + } + writeRunLogs(&stderr, result) + + for _, output := range []string{stdout.String(), stderr.String()} { + if strings.Contains(output, "DISTRIBUTOR_SECRET_TOKEN") { + t.Fatalf("output contains token value:\n%s", output) + } + if !strings.Contains(output, "[redacted]") { + t.Fatalf("output missing redacted marker:\n%s", output) + } + } +} + func TestRunEveningUsesOutputDirectoryAndSummary(t *testing.T) { server := dailyServer(t) tempDir := t.TempDir() @@ -384,6 +466,103 @@ func TestRunEveningUsesOutputDirectoryAndSummary(t *testing.T) { } } +func TestRunEveningReportsNotificationSuccess(t *testing.T) { + server := dailyServer(t) + distributorServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path != "/upload" { + http.NotFound(w, r) + return + } + w.WriteHeader(http.StatusAccepted) + _, _ = w.Write([]byte(`{"run_id":"distributor-run-1","status":"accepted"}`)) + })) + t.Cleanup(distributorServer.Close) + tempDir := t.TempDir() + scriptoriumPath := writeFakeScriptorium(t, tempDir) + configPath := filepath.Join(tempDir, "config.yml") + workspaceRoot := filepath.Join(tempDir, "workspace") + configBody := "weather_api:\n base_url: " + server.URL + "/\n timezone: America/Chicago\nscriptorium:\n binary: " + scriptoriumPath + "\nworkspace:\n root: " + workspaceRoot + "\nnotify:\n distributor:\n enabled: true\n endpoint: " + distributorServer.URL + "\n token_env: CLI_DISTRIBUTOR_TOKEN\n" + if err := os.WriteFile(configPath, []byte(configBody), 0o600); err != nil { + t.Fatalf("write config: %v", err) + } + t.Setenv("CLI_DISTRIBUTOR_TOKEN", "cli-secret-token") + var stdout bytes.Buffer + var stderr bytes.Buffer + runner := Runner{Clock: fixedClock()} + + err := runner.Run(context.Background(), []string{ + "run", "evening", + "--config", configPath, + }, &stdout, &stderr) + 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 { + t.Fatalf("reports = %#v, want one report", summary.Reports) + } + if summary.Reports[0].NotificationStatus != "accepted" || summary.Reports[0].NotificationRunID != "distributor-run-1" { + t.Fatalf("notification fields = %#v", summary.Reports[0]) + } + if !strings.Contains(stderr.String(), `notificationStatus="accepted"`) || !strings.Contains(stderr.String(), `notificationRunId="distributor-run-1"`) { + t.Fatalf("stderr missing 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) { + 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.Cleanup(distributorServer.Close) + tempDir := t.TempDir() + scriptoriumPath := writeFakeScriptorium(t, tempDir) + configPath := filepath.Join(tempDir, "config.yml") + workspaceRoot := filepath.Join(tempDir, "workspace") + configBody := "weather_api:\n base_url: " + server.URL + "/\n timezone: America/Chicago\nscriptorium:\n binary: " + scriptoriumPath + "\nworkspace:\n root: " + workspaceRoot + "\nnotify:\n distributor:\n enabled: true\n endpoint: " + distributorServer.URL + "\n token_env: CLI_DISTRIBUTOR_TOKEN\n" + if err := os.WriteFile(configPath, []byte(configBody), 0o600); err != nil { + t.Fatalf("write config: %v", err) + } + t.Setenv("CLI_DISTRIBUTOR_TOKEN", "cli-secret-token") + var stdout bytes.Buffer + var stderr bytes.Buffer + runner := Runner{Clock: fixedClock()} + + err := runner.Run(context.Background(), []string{ + "run", "evening", + "--config", configPath, + }, &stdout, &stderr) + if err == nil { + t.Fatal("Run() error = nil, want notification failure") + } + + 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) + } + 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) + } + } +} + func TestRunMorningGeneratesDailyAndThreeDayOnSunday(t *testing.T) { server := dailyServer(t) tempDir := t.TempDir()