From f4e3a6f26ce02c5d8622c53c722186536f3d253e Mon Sep 17 00:00:00 2001 From: Eric Rakestraw Date: Thu, 13 Aug 2026 02:33:32 +0000 Subject: [PATCH] Preflight report output filenames --- docs/internal/app-orchestration.md | 2 +- docs/operations.md | 6 ++++++ internal/app/generation_test.go | 21 +++++++++++++++++++++ internal/app/output.go | 4 ++++ internal/fileutil/fileutil.go | 19 ++++++++++++++++++- internal/fileutil/fileutil_test.go | 27 ++++++++++++++++++++++++++- 6 files changed, 76 insertions(+), 3 deletions(-) diff --git a/docs/internal/app-orchestration.md b/docs/internal/app-orchestration.md index 348d330..484c6cf 100644 --- a/docs/internal/app-orchestration.md +++ b/docs/internal/app-orchestration.md @@ -7,7 +7,7 @@ is owned by the [CLI reference](../cli.md) and [operations guide](../operations. ## Single-Report Flow -`GenerateDetailed` resolves the requested report and output destination before initializing an optional explicit debug writer. An explicit output file wins; otherwise the configured output directory is used, falling back to the captured working directory. It validates the report's generated-text catalog binding, exact Promptkit prompt, and selected profile before collecting weather data. The resolved profile, backend, and model are carried in the active result. +`GenerateDetailed` resolves the requested report and output destination before initializing an optional explicit debug writer. An explicit output file wins; otherwise the configured output directory is used, falling back to the captured working directory. Output preflight validates the final filename and the bounded same-directory temporary form without creating a missing parent. It then validates the report's generated-text catalog binding, exact Promptkit prompt, and selected profile before collecting weather data. The resolved profile, backend, and model are carried in the active result. The workflow builds facts, a module snapshot, briefing metadata, and the YAML prompt package in memory. It executes Promptkit only against the inspected prompt and profile, reconciles the preparation callback and completed result with that identity and the prepared report schema, validates the returned generated text, builds a render context, and renders Markdown. `fileutil` atomically writes the completed Markdown to the selected output path. Only after that write succeeds does single-report notification run. diff --git a/docs/operations.md b/docs/operations.md index 2158134..2b0ef80 100644 --- a/docs/operations.md +++ b/docs/operations.md @@ -36,6 +36,12 @@ absolute output path and active profile, backend, model, warnings, validation, debug, and notification information; see the [CLI reference](cli.md) for its exact fields. +Weatherreporter validates the final output filename before prompt inspection or +weather collection. A valid long filename is published through a short, +same-directory temporary sibling, so temporary naming does not shorten the +operator-selected destination. A rejected filename does not create a missing +parent directory. + `SIGINT` and `SIGTERM` request orderly cancellation of an active action. The command lets cancellation and related cleanup finish before it exits; use the usual failed result or error to determine whether an output was published. diff --git a/internal/app/generation_test.go b/internal/app/generation_test.go index 3ee9bc8..ba21538 100644 --- a/internal/app/generation_test.go +++ b/internal/app/generation_test.go @@ -6,6 +6,7 @@ import ( "errors" "os" "path/filepath" + "strings" "sync" "testing" "time" @@ -276,6 +277,26 @@ func TestGenerateDetailedRejectsConfiguredNonDirectoryBeforeWork(t *testing.T) { } } +func TestGenerateDetailedRejectsOverlongOutputBeforeWork(t *testing.T) { + missingDirectory := filepath.Join(t.TempDir(), "missing") + outputPath := filepath.Join(missingDirectory, strings.Repeat("a", 253)+".md") + bundle := generationBundle(t) + collector := &generationCollector{bundle: &bundle} + executor := &generationExecutor{} + + result, err := GenerateDetailed(context.Background(), GenerateRequest{ + Config: generationConfig(), Report: ReportDaily, + Date: generationTime("2026-05-29T12:00:00-05:00"), Now: generationTime("2026-05-29T08:30:00-05:00"), + WorkingDir: t.TempDir(), OutputPath: outputPath, Collector: collector, Executor: executor, + }) + if err == nil || result == nil || collector.called || executor.promptInspections != 0 || executor.called { + t.Fatalf("GenerateDetailed() result/error/collector/executor = %#v/%v/%t/%#v", result, err, collector.called, executor) + } + if _, statErr := os.Stat(missingDirectory); !os.IsNotExist(statErr) { + t.Fatalf("missing output directory exists after preflight failure: %v", statErr) + } +} + func TestGenerateDetailedReturnsResolvedResultWhenCollectionFails(t *testing.T) { cfg := config.Defaults() cfg.WeatherAPI.Timezone, cfg.Location.ID = "America/Chicago", "home" diff --git a/internal/app/output.go b/internal/app/output.go index ca44f3e..52602b8 100644 --- a/internal/app/output.go +++ b/internal/app/output.go @@ -7,6 +7,7 @@ import ( "strings" "gitea.maximumdirect.net/eric/weatherreporter/internal/comparison" + "gitea.maximumdirect.net/eric/weatherreporter/internal/fileutil" "gitea.maximumdirect.net/eric/weatherreporter/internal/report" ) @@ -184,6 +185,9 @@ func validateOutputPath(path string) (string, error) { if filepath.Dir(path) == path { return "", fmt.Errorf("final output path %q must not be a filesystem root", path) } + if err := fileutil.ValidateAtomicPath(path); err != nil { + return "", fmt.Errorf("validate final output path %q: %w", path, err) + } if info, err := os.Stat(path); err == nil && info.IsDir() { return "", fmt.Errorf("final output path %q is a directory", path) } else if err != nil && !os.IsNotExist(err) { diff --git a/internal/fileutil/fileutil.go b/internal/fileutil/fileutil.go index c7e803e..ab4c06f 100644 --- a/internal/fileutil/fileutil.go +++ b/internal/fileutil/fileutil.go @@ -8,11 +8,28 @@ import ( "path/filepath" ) +const ( + maxFileNameBytes = 255 + temporaryFilePattern = ".weatherreporter-*.tmp" +) + +// ValidateAtomicPath verifies that the final path can be safely used with this +// package's same-directory atomic-write implementation. +func ValidateAtomicPath(path string) error { + if len(filepath.Base(path)) > maxFileNameBytes { + return fmt.Errorf("final file name exceeds the %d-byte limit", maxFileNameBytes) + } + return nil +} + func WriteFileAtomic(path string, data []byte) error { + if err := ValidateAtomicPath(path); err != nil { + return err + } if err := os.MkdirAll(filepath.Dir(path), 0o755); err != nil { return fmt.Errorf("create directory %q: %w", filepath.Dir(path), err) } - tmp, err := os.CreateTemp(filepath.Dir(path), "."+filepath.Base(path)+".*.tmp") + tmp, err := os.CreateTemp(filepath.Dir(path), temporaryFilePattern) if err != nil { return fmt.Errorf("create temporary file for %q: %w", path, err) } diff --git a/internal/fileutil/fileutil_test.go b/internal/fileutil/fileutil_test.go index 96b839f..6f0df82 100644 --- a/internal/fileutil/fileutil_test.go +++ b/internal/fileutil/fileutil_test.go @@ -42,6 +42,31 @@ func TestWriteFileAtomicOverwritesTarget(t *testing.T) { } } +func TestWriteFileAtomicSupportsLongestFileName(t *testing.T) { + directory := t.TempDir() + name := strings.Repeat("a", maxFileNameBytes-len(".md")) + ".md" + path := filepath.Join(directory, name) + if err := os.WriteFile(path, []byte("old"), 0o600); err != nil { + t.Fatal(err) + } + + if err := WriteFileAtomic(path, []byte("new")); err != nil { + t.Fatalf("WriteFileAtomic() error = %v", err) + } + data, err := os.ReadFile(path) + if err != nil || string(data) != "new" { + t.Fatalf("output/error = %q/%v", data, err) + } + info, err := os.Stat(path) + if err != nil || info.Mode().Perm() != 0o600 { + t.Fatalf("output mode/error = %o/%v", info.Mode().Perm(), err) + } + matches, err := filepath.Glob(filepath.Join(directory, ".weatherreporter-*.tmp")) + if err != nil || len(matches) != 0 { + t.Fatalf("temporary files/error = %v/%v", matches, err) + } +} + func TestWriteFileAtomicCleansTemporaryFileAfterRenameError(t *testing.T) { dir := t.TempDir() target := filepath.Join(dir, "target") @@ -56,7 +81,7 @@ func TestWriteFileAtomicCleansTemporaryFileAfterRenameError(t *testing.T) { if !strings.Contains(err.Error(), "save") { t.Fatalf("error = %q, want save context", err.Error()) } - matches, err := filepath.Glob(filepath.Join(dir, ".target.*.tmp")) + matches, err := filepath.Glob(filepath.Join(dir, ".weatherreporter-*.tmp")) if err != nil { t.Fatalf("Glob() error = %v", err) }