From 70cad789ea4b1ffa5afcaf771c452dc02dc93017 Mon Sep 17 00:00:00 2001 From: Eric Rakestraw Date: Thu, 13 Aug 2026 03:02:59 +0000 Subject: [PATCH] Preserve batch cancellation outcomes --- docs/cli.md | 11 ++++- docs/internal/app-orchestration.md | 7 +++ docs/operations.md | 5 +- internal/app/app.go | 67 +++++++++++++++++++++++++-- internal/app/batch_generation_test.go | 61 +++++++++++++++++++++++- internal/app/batch_notification.go | 6 +++ internal/app/generation_test.go | 11 +++-- internal/cli/output.go | 6 ++- internal/cli/result.go | 8 ++-- internal/cli/root.go | 5 +- internal/cli/run_test.go | 37 +++++++++++++++ 11 files changed, 207 insertions(+), 17 deletions(-) diff --git a/docs/cli.md b/docs/cli.md index 517a20f..995c3cb 100644 --- a/docs/cli.md +++ b/docs/cli.md @@ -70,7 +70,10 @@ remains available. `SIGINT` and `SIGTERM` cancel an active action. Weatherreporter lets that cancellation reach the action before exiting; when the action has a result, it -emits the usual failed summary and exits nonzero. +emits the usual failed summary and exits nonzero. A canceled batch retains any +reports that were already published, marks interrupted and unstarted reports +as `canceled`, skips batch notification, and identifies cancellation separately +from report failures. Action commands (`generate`, `run`, and `compare`) write a JSON summary to stdout unless `--quiet` is set. `run` also writes compact per-report and batch @@ -119,11 +122,15 @@ The `total`, `succeeded`, and `failed` counters describe report items only, so a failed batch notification can leave `failed` at `0` while the top-level notification and action status are `failed`. +When cancellation stops a batch, the summary also includes a nonzero +`canceled` count. Canceled reports have `"status": "canceled"`; they are not +included in `failed`, and the action still has failed status and exits nonzero. + Without `--quiet`, batch status lines use this form: ```text report=today status=succeeded output="/srv/weather/reports/today.md" -batch=morning total=2 succeeded=2 failed=0 +batch=morning total=2 succeeded=2 failed=0 canceled=0 ``` ### Compare Summary diff --git a/docs/internal/app-orchestration.md b/docs/internal/app-orchestration.md index 7dcf8ec..fff9128 100644 --- a/docs/internal/app-orchestration.md +++ b/docs/internal/app-orchestration.md @@ -19,6 +19,13 @@ Failures return an active partial result with safe identity, profile, warning, v Each item has an independent result. A failed item does not stop later items; successful items retain their published output paths. Per-report notification is suppressed during a batch. Batch notification runs only after every planned report has published successfully. It is skipped when any item failed. Batch result counters count report items only; a batch notification failure is represented by the top-level notification result and still produces a failed batch outcome. +Cancellation and deadline expiry stop the sequential loop before another report +starts. Completed report results and published paths remain successful; the +interrupted and unstarted planned reports have `canceled` status and are counted +separately from failed reports. The batch notification result records that +delivery was skipped, and the returned error retains the original context cause +for callers and CLI projection. + ## Comparisons `CompareDetailed` validates ordered explicit profile IDs, resolves the report, diff --git a/docs/operations.md b/docs/operations.md index cd0f9e7..cd04a7c 100644 --- a/docs/operations.md +++ b/docs/operations.md @@ -69,7 +69,10 @@ final output destination before executing its first report prompt. A destination collision, such as a directory named `tomorrow.md`, stops the batch before any report output is created or replaced. After successful validation, each selected report processes independently and successful outputs remain available if -another report fails. +another report fails. If cancellation or a deadline is observed during the +sequence, Weatherreporter stops before starting another report. It retains +already published files, marks interrupted and unstarted reports as canceled in +the result, and skips batch notification. When `notify.distributor.enabled` and batch notification are enabled, Weatherreporter sends one Distributor upload only after every selected output diff --git a/internal/app/app.go b/internal/app/app.go index 5cc202d..0bf309b 100644 --- a/internal/app/app.go +++ b/internal/app/app.go @@ -3,6 +3,7 @@ package app import ( "context" + "errors" "fmt" "path/filepath" "time" @@ -100,6 +101,7 @@ type BatchResult struct { Total int `json:"total"` Succeeded int `json:"succeeded"` Failed int `json:"failed"` + Canceled int `json:"canceled,omitempty"` Notification *BatchNotificationResult `json:"notification,omitempty"` Reports []BatchReportResult `json:"reports"` } @@ -143,12 +145,19 @@ type BatchReportResult struct { type BatchError struct { Result *BatchResult + Cause error } func (e BatchError) Error() string { if e.Result == nil { return "batch failed" } + if errors.Is(e.Cause, context.DeadlineExceeded) { + return fmt.Sprintf("batch %s deadline exceeded", e.Result.Batch) + } + if errors.Is(e.Cause, context.Canceled) || e.Result.Canceled > 0 { + return fmt.Sprintf("batch %s canceled", e.Result.Batch) + } failedReports := batchReportFailures(e.Result) if batchNotificationFailed(e.Result) && failedReports == 0 { if e.Result.Notification.Error != "" { @@ -159,6 +168,10 @@ func (e BatchError) Error() string { return fmt.Sprintf("batch %s failed: %d of %d reports failed", e.Result.Batch, failedReports, len(e.Result.Reports)) } +func (e BatchError) Unwrap() error { + return e.Cause +} + func batchNotificationFailed(result *BatchResult) bool { return result != nil && result.Notification != nil && result.Notification.Status == "failed" } @@ -344,7 +357,12 @@ func RunBatchDetailed(ctx context.Context, req BatchRequest) (*BatchResult, erro if req.Batch == BatchEvening || req.Batch == BatchMorning { startedAt := now result := &BatchResult{Batch: req.Batch, StartedAt: startedAt} - for _, planned := range plannedReports { + var cancellation error + for index, planned := range plannedReports { + if cancellation = batchCancellationCause(ctx, nil); cancellation != nil { + appendCanceledBatchReports(result, plannedReports[index:]) + break + } resolved := planned.Resolved item := batchReportResult(planned) reportResult, err := generatePromptReport(ctx, promptReportRequest{ @@ -364,14 +382,26 @@ func RunBatchDetailed(ctx context.Context, req BatchRequest) (*BatchResult, erro copyBatchReportDetails(&item, reportResult) } if err != nil { - item.Status = "failed" - item.Error = err.Error() - result.Failed++ + if cancellation = batchCancellationCause(ctx, err); cancellation != nil { + item.Status = "canceled" + result.Canceled++ + } else { + item.Status = "failed" + item.Error = err.Error() + result.Failed++ + } } else { item.Status = "succeeded" result.Succeeded++ } result.Reports = append(result.Reports, item) + if cancellation == nil { + cancellation = batchCancellationCause(ctx, nil) + } + if cancellation != nil { + appendCanceledBatchReports(result, plannedReports[index+1:]) + break + } } result.Total = len(result.Reports) batchNotification := notifyBatch(ctx, req.Config, req.Batch, batchRunID(startedAt, req.Batch), startedAt, result, plannedReports, req.Notifier) @@ -379,11 +409,38 @@ func RunBatchDetailed(ctx context.Context, req BatchRequest) (*BatchResult, erro result.Notification = batchNotification } result.FinishedAt = time.Now() - return result, nil + return result, cancellation } return nil, fmt.Errorf("run is not implemented") } +func batchCancellationCause(ctx context.Context, err error) error { + if ctx != nil { + if contextErr := ctx.Err(); contextErr != nil { + return contextErr + } + } + if errors.Is(err, context.Canceled) { + return context.Canceled + } + if errors.Is(err, context.DeadlineExceeded) { + return context.DeadlineExceeded + } + return nil +} + +func appendCanceledBatchReports(result *BatchResult, plannedReports []plannedBatchReport) { + if result == nil { + return + } + for _, planned := range plannedReports { + item := batchReportResult(planned) + item.Status = "canceled" + result.Reports = append(result.Reports, item) + result.Canceled++ + } +} + func copyBatchReportDetails(item *BatchReportResult, result *ReportResult) { item.LLMDebugPath = result.LLMDebugPath item.OutputPath = result.OutputPath diff --git a/internal/app/batch_generation_test.go b/internal/app/batch_generation_test.go index 517439b..bba053e 100644 --- a/internal/app/batch_generation_test.go +++ b/internal/app/batch_generation_test.go @@ -21,7 +21,7 @@ func TestRunBatchDetailedKeepsSuccessfulOutputAndSkipsNotificationAfterPartialFa Now: generationTime("2026-05-29T08:30:00-05:00"), WorkingDir: t.TempDir(), OutputDir: t.TempDir(), Collector: &generationCollector{bundle: &bundle}, Executor: executor, Notifier: notifier, }) - if err != nil || result == nil || result.Total != 2 || result.Succeeded != 1 || result.Failed != 1 || result.Notification == nil || result.Notification.Status != "skipped" || notifier.batchCalls != 0 { + if err != nil || result == nil || result.Total != 2 || result.Succeeded != 1 || result.Failed != 1 || result.Canceled != 0 || result.Notification == nil || result.Notification.Status != "skipped" || notifier.batchCalls != 0 { t.Fatalf("RunBatchDetailed() result/error/notifier = %#v/%v/%#v", result, err, notifier) } if result.Reports[0].Status != "succeeded" || result.Reports[0].OutputPath == "" || result.Reports[1].Status != "failed" || result.Reports[1].OutputPath != "" { @@ -32,6 +32,65 @@ func TestRunBatchDetailedKeepsSuccessfulOutputAndSkipsNotificationAfterPartialFa } } +func TestRunBatchDetailedStopsAfterReportCancellation(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + bundle := generationBundle(t) + bundle.Hourly.Periods = bundle.Hourly.Periods[:1] + notifier := &generationNotifier{} + executor := &generationExecutor{cancelBeforeReturn: cancel} + + result, err := RunBatchDetailed(ctx, BatchRequest{ + Config: generationDistributorConfig(), Batch: BatchMorning, + Now: generationTime("2026-05-29T08:30:00-05:00"), WorkingDir: t.TempDir(), OutputDir: t.TempDir(), + Collector: &generationCollector{bundle: &bundle}, Executor: executor, Notifier: notifier, + }) + if !errors.Is(err, context.Canceled) || result == nil || result.Total != 2 || result.Succeeded != 0 || result.Failed != 0 || result.Canceled != 2 || executor.executeCalls != 1 || notifier.batchCalls != 0 || result.Notification == nil || result.Notification.Status != "skipped" || result.Notification.Reason != "batch canceled" { + t.Fatalf("RunBatchDetailed() result/error/executor/notifier = %#v/%v/%#v/%#v", result, err, executor, notifier) + } + for _, item := range result.Reports { + if item.Status != "canceled" || item.OutputPath != "" { + t.Fatalf("canceled report = %#v", item) + } + } +} + +func TestRunBatchDetailedRetainsPublishedReportBeforeCancellation(t *testing.T) { + for _, cause := range []error{context.Canceled, context.DeadlineExceeded} { + t.Run(cause.Error(), func(t *testing.T) { + bundle := generationBundle(t) + bundle.Hourly.Periods = bundle.Hourly.Periods[:1] + notifier := &generationNotifier{} + ctx := &publicationGateContext{Context: context.Background(), err: cause, afterChecks: 4} + + result, err := RunBatchDetailed(ctx, BatchRequest{ + Config: generationDistributorConfig(), Batch: BatchMorning, + Now: generationTime("2026-05-29T08:30:00-05:00"), WorkingDir: t.TempDir(), OutputDir: t.TempDir(), + Collector: &generationCollector{bundle: &bundle}, Executor: &generationExecutor{}, Notifier: notifier, + }) + if !errors.Is(err, cause) || result == nil || result.Total != 2 || result.Succeeded != 1 || result.Failed != 0 || result.Canceled != 1 || len(result.Reports) != 2 || result.Reports[0].Status != "succeeded" || result.Reports[0].OutputPath == "" || result.Reports[1].Status != "canceled" || result.Reports[1].OutputPath != "" || notifier.batchCalls != 0 || result.Notification == nil || result.Notification.Status != "skipped" || result.Notification.Reason != "batch canceled" { + t.Fatalf("RunBatchDetailed() result/error/notifier = %#v/%v/%#v", result, err, notifier) + } + if _, statErr := os.Stat(result.Reports[0].OutputPath); statErr != nil { + t.Fatalf("published report %q: %v", result.Reports[0].OutputPath, statErr) + } + }) + } +} + +func TestRunBatchPreservesCancellationCause(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + bundle := generationBundle(t) + bundle.Hourly.Periods = bundle.Hourly.Periods[:1] + err := RunBatch(ctx, BatchRequest{ + Config: generationDistributorConfig(), Batch: BatchMorning, + Now: generationTime("2026-05-29T08:30:00-05:00"), WorkingDir: t.TempDir(), OutputDir: t.TempDir(), + Collector: &generationCollector{bundle: &bundle}, Executor: &generationExecutor{cancelBeforeReturn: cancel}, Notifier: &generationNotifier{}, + }) + if !errors.Is(err, context.Canceled) { + t.Fatalf("RunBatch() error = %v", err) + } +} + func TestRunBatchDetailedNotifiesOnlyAfterAllOutputsExist(t *testing.T) { bundle := generationBundle(t) bundle.Hourly.Periods = bundle.Hourly.Periods[:1] diff --git a/internal/app/batch_notification.go b/internal/app/batch_notification.go index 861e502..a1a45af 100644 --- a/internal/app/batch_notification.go +++ b/internal/app/batch_notification.go @@ -56,6 +56,12 @@ func notifyBatch(ctx context.Context, cfg config.Config, batch BatchKind, runID if result == nil { return failedBatchNotificationResult(batchNotificationRequest{}, fmt.Errorf("batch result is required")) } + if result.Canceled > 0 { + return &BatchNotificationResult{ + Status: "skipped", + Reason: "batch canceled", + } + } if result.Failed > 0 { return &BatchNotificationResult{ Status: "skipped", diff --git a/internal/app/generation_test.go b/internal/app/generation_test.go index 487389c..273bf9c 100644 --- a/internal/app/generation_test.go +++ b/internal/app/generation_test.go @@ -28,13 +28,18 @@ type generationCollector struct { type publicationGateContext struct { context.Context - err error - checks int + err error + checks int + afterChecks int } func (c *publicationGateContext) Err() error { c.checks++ - if c.checks >= 2 { + afterChecks := c.afterChecks + if afterChecks == 0 { + afterChecks = 2 + } + if c.checks >= afterChecks { return c.err } return nil diff --git a/internal/cli/output.go b/internal/cli/output.go index c293669..ece25eb 100644 --- a/internal/cli/output.go +++ b/internal/cli/output.go @@ -37,6 +37,10 @@ func writeBatchStatus(stderr io.Writer, result *app.BatchResult) { _, _ = fmt.Fprintf(stderr, "report=%s status=failed error=%q\n", item.ReportID, item.Error) continue } + if item.Status == "canceled" { + _, _ = fmt.Fprintf(stderr, "report=%s status=canceled\n", item.ReportID) + continue + } _, _ = fmt.Fprintf(stderr, "report=%s status=succeeded output=%q\n", item.ReportID, item.OutputPath) } if result.Notification != nil { @@ -58,5 +62,5 @@ func writeBatchStatus(stderr io.Writer, result *app.BatchResult) { } _, _ = fmt.Fprintln(stderr) } - _, _ = fmt.Fprintf(stderr, "batch=%s total=%d succeeded=%d failed=%d\n", result.Batch, result.Total, result.Succeeded, result.Failed) + _, _ = fmt.Fprintf(stderr, "batch=%s total=%d succeeded=%d failed=%d canceled=%d\n", result.Batch, result.Total, result.Succeeded, result.Failed, result.Canceled) } diff --git a/internal/cli/result.go b/internal/cli/result.go index aeaeec9..1903e2b 100644 --- a/internal/cli/result.go +++ b/internal/cli/result.go @@ -70,6 +70,7 @@ type batchSummary struct { Total int `json:"total"` Succeeded int `json:"succeeded"` Failed int `json:"failed"` + Canceled int `json:"canceled,omitempty"` Notification *app.BatchNotificationResult `json:"notification,omitempty"` Reports []app.BatchReportResult `json:"reports"` Error string `json:"error,omitempty"` @@ -161,7 +162,7 @@ func newGenerateNotificationSummary(result *app.NotificationResult) *generateNot return summary } -func newBatchSummary(result *app.BatchResult) batchSummary { +func newBatchSummary(result *app.BatchResult, err error) batchSummary { summary := batchSummary{Command: commandRun} if result == nil { return summary @@ -174,10 +175,11 @@ func newBatchSummary(result *app.BatchResult) batchSummary { summary.Total = result.Total summary.Succeeded = result.Succeeded summary.Failed = result.Failed + summary.Canceled = result.Canceled summary.Notification = result.Notification summary.Reports = append([]app.BatchReportResult(nil), result.Reports...) if summary.Status == summaryStatusFailed { - summary.Error = app.BatchError{Result: result}.Error() + summary.Error = app.BatchError{Result: result, Cause: err}.Error() } return summary } @@ -267,7 +269,7 @@ func batchSummaryStatus(result *app.BatchResult) string { if result == nil { return "" } - if result.Failed > 0 || (result.Notification != nil && result.Notification.Status == summaryStatusFailed) { + if result.Failed > 0 || result.Canceled > 0 || (result.Notification != nil && result.Notification.Status == summaryStatusFailed) { return summaryStatusFailed } return summaryStatusSucceeded diff --git a/internal/cli/root.go b/internal/cli/root.go index 71e9734..9cd60cc 100644 --- a/internal/cli/root.go +++ b/internal/cli/root.go @@ -111,12 +111,15 @@ func (r Runner) Run(ctx context.Context, args []string, stdout io.Writer, stderr } result, err := runBatchDetailed(ctx, req) if result != nil { - summary := newBatchSummary(result) + summary := newBatchSummary(result, err) if encodeErr := writeActionResult(stdout, stderr, summary, outputOptions{Quiet: opts.Quiet}, func(w io.Writer) { writeBatchStatus(w, result) }); encodeErr != nil { return encodeErr } + if err != nil { + return err + } if summary.Status == summaryStatusFailed { return app.BatchError{Result: result} } diff --git a/internal/cli/run_test.go b/internal/cli/run_test.go index 57feed0..4b86501 100644 --- a/internal/cli/run_test.go +++ b/internal/cli/run_test.go @@ -193,6 +193,43 @@ func TestRunActionReturnsFailureForBatchNotificationFailure(t *testing.T) { } } +func TestRunActionPreservesBatchCancellation(t *testing.T) { + configPath := actionConfigPath(t) + for _, cause := range []error{context.Canceled, context.DeadlineExceeded} { + t.Run(cause.Error(), func(t *testing.T) { + result := &app.BatchResult{ + Batch: app.BatchMorning, Total: 2, Succeeded: 1, Canceled: 1, + Reports: []app.BatchReportResult{ + {ReportID: "today", Status: "succeeded", OutputPath: "/reports/today.md"}, + {ReportID: "tomorrow", Status: "canceled"}, + }, + Notification: &app.BatchNotificationResult{Status: "skipped", Reason: "batch canceled"}, + } + var stdout, stderr bytes.Buffer + runner := Runner{ + Clock: timeutil.FixedClock{Time: time.Date(2026, 5, 29, 8, 0, 0, 0, time.UTC)}, + ExecutorFactory: func(PromptExecutorConfig) (promptexec.Executor, error) { + return &factoryExecutor{}, nil + }, + runBatchDetailed: func(context.Context, app.BatchRequest) (*app.BatchResult, error) { + return result, cause + }, + } + err := runner.Run(context.Background(), []string{"run", "morning", "--config", configPath}, &stdout, &stderr) + if !errors.Is(err, cause) { + t.Fatalf("Run() error = %v", err) + } + var summary batchSummary + if decodeErr := json.Unmarshal(stdout.Bytes(), &summary); decodeErr != nil { + t.Fatal(decodeErr) + } + if summary.Status != summaryStatusFailed || summary.Total != 2 || summary.Succeeded != 1 || summary.Failed != 0 || summary.Canceled != 1 || summary.Error == "" || len(summary.Reports) != 2 || summary.Reports[1].Status != "canceled" || !strings.Contains(stderr.String(), "report=tomorrow status=canceled") || !strings.Contains(stderr.String(), "canceled=1") { + t.Fatalf("summary/stderr = %#v/%q", summary, stderr.String()) + } + }) + } +} + func TestRunCommandProjectsSuccessAndReportFailure(t *testing.T) { configPath := actionConfigPath(t) for _, tt := range []struct {