diff --git a/docs/roadmap/implementation.md b/docs/roadmap/implementation.md index 806ddf4..bde86a5 100644 --- a/docs/roadmap/implementation.md +++ b/docs/roadmap/implementation.md @@ -1,416 +1,162 @@ # Stateless Execution Implementation Plan -Status: Complete. +Status: Ready for remediation implementation (Stages 14-18). ## Purpose This plan implements the accepted [Stateless Execution -Roadmap](ephemeral-state.md). That roadmap is authoritative for product intent, -policy choices, and the desired end state. This document owns implementation -order, concrete code changes, and verification gates. +Roadmap](ephemeral-state.md). The roadmap is authoritative for product intent, +policy choices, and the desired end state. This document records the completed +migration and defines the remaining remediation work discovered during +post-implementation review. -The target is a Weatherreporter process whose ordinary invocations require no -prior application state and leave only operator-owned Markdown outputs. Local -Recent Changes, the managed workspace, historical artifacts and metadata, and -the `inspect` command family are removed. Explicit secure Promptkit debug -capture remains operator-owned. +Stages 1-13 are complete and are summarized below. Implement Stages 14-18 in +numeric order. Each remaining stage is intentionally scoped for one focused +implementation prompt. ## Implementation Rules -1. Implement the stages in numeric order. Do not merge or release an - intermediate stage as a completed stateless-execution feature. -2. Keep the repository buildable and `go test ./...` passing at every stage. - Use focused tests while iterating, then run the stage's listed commands. -3. Follow the architecture, documentation, and testing policies under - `docs/policy/`. Delete tests that protect intentionally removed contracts; - do not mechanically rewrite them to preserve obsolete structures. -4. Do not retain compatibility shims for `workspace`, `recent_change`, - historical metadata, inspection commands, prompt artifact paths, or the - `recent_changes` prompt field. Strict configuration loading should reject - removed fields. -5. Preserve prompt inspection before weather collection, batch-wide prompt - inspection before collection, exact profile selection, structured-output - validation, repository-owned rendering, deterministic report periods, - batch membership, safe error classification, and Distributor bundle paths. -6. Preserve the existing `--llm-debug-dir PATH` interface and security - properties. Do not introduce an implicit debug location or a general - logging subsystem. -7. Do not contact live Weather API, Promptkit provider, or Distributor - services in repository tests. -8. Do not create release notes until a release version is selected. The - durable documentation stage must record the compatibility and operator - actions that future release notes need to summarize. +1. Keep the repository buildable and `go test ./...` passing after every stage. +2. Follow all policies under `docs/policy/`, especially the architecture, + documentation, and risk-based testing requirements. +3. Preserve the accepted stateless contracts: no application-owned durable + workspace, no local Recent Changes comparison, one operator-owned output per + successful report, atomic publication, and explicit-only prompt debug + capture. +4. Preserve prompt and profile inspection before weather collection, exact + prompt versions, structured-output validation, repository-owned rendering, + deterministic report periods, batch membership, and Distributor upload of + published Markdown files. +5. Keep default tests deterministic and offline. Do not contact live Weather + API, Promptkit provider, or Distributor services. +6. Use behavior-focused regression tests at the narrowest stable boundary. + Avoid tests that merely encode private helper structure or call order. +7. Update canonical documentation in the same stage as any user-visible or + architectural behavior change. Do not create release notes until a release + version is selected. +8. Do not modify or reinterpret the Accepted stateless-execution ADR. A changed + architectural decision would require a new ADR; the remaining work does not + require one. -## Target Contracts +## Fixed Remediation Decisions -These decisions are fixed for implementation: +The following decisions are complete and require no further product input: -- `generate` always writes exactly one operator-owned Markdown file. -- `--out PATH` selects that file. Without `--out`, the destination is in the - working directory captured at invocation start: - `daily-YYYY-MM-DD.md`, `today.md`, `tomorrow.md`, or `hourly.md`. -- `run` writes each successful report beneath `--out-dir PATH`, or beneath the - captured working directory when the flag is omitted. Daily batch filenames - remain date-qualified. -- CLI-resolved output paths are cleaned absolute paths. App-level action - requests receive an absolute working directory plus an optional operator - override and reject invalid or empty resolved destinations before weather - collection. -- A generation publishes its selected output atomically. Failure or - cancellation before publication leaves an existing destination unchanged. -- Single-report notification occurs after local output publication. Batch - notification occurs only after every planned report succeeds. Distributor - reads the selected output files; there is no second managed copy. -- Action summaries retain identity, valid period, safe effective - profile/backend/model information, source warnings, final output path, - optional debug path, notification status, and a safe error. They contain no - historical or transient artifact paths. -- RunIDs remain active correlation and idempotency values only. -- The prompt data-package schema becomes - `weatherreporter.data_package.v4`, with no `recent_changes` member. All four - embedded prompts become exact version `2.0.0`. -- Ordinary execution creates no `workspace` tree, metadata, receipts, - snapshots, data packages, generated-text intermediates, render-context - files, notification artifacts, or run index. +- A canceled generation must not publish or replace its selected output if + cancellation is observable before atomic publication begins. +- A batch must validate every planned report destination before starting any + report's Promptkit execution or output publication. Dynamic Daily + destinations may be validated after weather collection and batch planning, + because those dates are not known earlier. +- `BatchResult.Total`, `Succeeded`, and `Failed` count reports only. A batch + notification failure changes overall batch status and exit behavior through + the top-level notification result; it does not increment `Failed`. +- Batch report items do not expose per-report notification fields because batch + reports deliberately suppress per-report notification. +- Completed roadmap prose must distinguish the former persistent architecture + from current stateless behavior, and repository hygiene must no longer hide + an accidentally recreated root-level `workspace` directory. -## Stage 1: Record The Stateless Architecture Decision +## Completed Stages + +### Stage 1: Record The Stateless Architecture Decision — Complete + +Added the Accepted ADR recording the stateless transformation pipeline, +operator-owned output boundary, removal of local comparison and historical +inspection, atomic publication, and explicit debug-capture exception. + +### Stage 2: Remove Recent Changes From The Prompt Contract — Complete + +Removed Recent Changes from prompt input, advanced the data package to +`weatherreporter.data_package.v4`, and advanced all four embedded prompts to +exact version `2.0.0`. + +### Stage 3: Delete Dormant Local Comparison Policy — Complete + +Removed the local comparison implementation and `recent_change` configuration; +strict configuration loading now rejects the obsolete field. + +### Stage 4: Establish The Operator-Owned Output Contract — Complete + +Made every successful report publish one atomic Markdown output, added +current-working-directory defaults and explicit destination overrides, and +centralized report output naming. + +### Stage 5: Separate Explicit Debug Capture From State — Complete + +Moved secure opt-in Promptkit diagnostics into `internal/promptdebug` without +introducing implicit diagnostics or ordinary artifact persistence. + +### Stage 6: Remove Notification Persistence — Complete + +Removed notification receipts and changed Distributor delivery to consume the +published operator-owned output from the active workflow. + +### Stage 7: Replace The Persisted Generation Workflow — Complete + +Converted single and batch generation to in-memory orchestration and removed +ordinary persistence of snapshots, prompt packages, generated text, render +contexts, metadata, and managed reports. + +### Stage 8: Remove Historical Inspection And Prior Compatibility — Complete + +Removed the `inspect` command family, historical lookup, prior-run selection, +and metadata compatibility surfaces. + +### Stage 9: Delete The Workspace And State Subsystem — Complete + +Removed `internal/state`, workspace configuration, state-only helpers, and +legacy artifact models. Strict loading rejects the obsolete `workspace` stanza. + +### Stage 10: Consolidate Stateless Behavioral Coverage — Complete + +Replaced state-oriented fixtures with focused offline coverage of generation, +batching, atomic output, partial success, notification ordering, prompt +inspection, summaries, and explicit debug capture. + +### Stage 11: Publish User, Operator, And Policy Documentation — Complete + +Updated the CLI, operations, architecture, configuration, and documentation +policy owners for stateless operation and manual legacy-workspace cleanup. + +### Stage 12: Reconcile Internal And Integration Documentation — Complete + +Updated focused internal and integration documents to describe in-memory +orchestration, published-output notification sources, and the absence of +historical inspection. + +### Stage 13: Run The Repository Exit Gate — Complete + +Ran the complete offline test, race, vet, build, help, formatting, stale-string, +and documentation review gates and marked the initial migration implemented. + +## Stage 14: Prevent Publication After Cancellation ### Goal -Create the durable decision record before changing the architecture, without -describing unimplemented behavior as current behavior elsewhere. +Close the cancellation window between successful Promptkit execution and +atomic output publication. ### Work -1. Create `docs/adr/` if it does not exist and add - `docs/adr/0001-stateless-execution.md` as an Accepted ADR using the format - required by `docs/policy/documentation.md`. -2. Record: - - why run-addressed provenance conflicts with ephemeral weather reports; - - removal of local Recent Changes rather than retention of state for it; - - the stateless transformation pipeline and operator-owned output boundary; - - atomic output and notification ordering; - - explicit debug capture as the only retained diagnostic-file exception; - - removal of inspection and backward-compatible workspace decoding; - - the upstream Weather API path for future forecast comparison; and - - alternatives: the former bounded-current-state design, time-based - retention, and bounded run history. -3. Link the ADR to the feature roadmap for scope, while keeping the ADR focused - on durable rationale rather than implementation stages. -4. Do not update current-state architecture or user documentation in this - stage. - -### Tests - -Run: - -```sh -git diff --check -``` - -Verify every repository-relative ADR link resolves. - -### Exit Gate - -An Accepted ADR records the exact decision and consequences; no production -behavior or current-state documentation has changed. - -## Stage 2: Remove Recent Changes From The Prompt Contract - -### Goal - -Stop generating or sending local comparison results while the existing -workspace remains temporarily available for unrelated artifacts. - -### Work - -1. Remove `RecentChanges` from `promptinput.BuildRequest` and - `promptinput.Package`, delete the prompt-input `RecentChanges` wrapper, and - remove the `internal/changes` dependency from `internal/promptinput`. -2. Advance `promptinput.SchemaVersion` from - `weatherreporter.data_package.v3` to `weatherreporter.data_package.v4`. - Update marshal, load, validation, round-trip, ordering, and fixture tests so - v4 has no `recent_changes` key and v3 is rejected. -3. In app orchestration, stop finding a prior snapshot, loading it, invoking a - comparator, or placing Recent Changes in `ReportResult` or prompt input. - Continue building the current module snapshot because it is still needed - in memory for prompt input and rendering, and may still be persisted by the - transitional workflow. -4. Remove Recent Changes fields and assertions from app workflow fakes and - tests. Delete prior-run workflow cases whose only purpose was local change - detection. -5. Change all four report-definition prompt versions and embedded prompt YAML - versions from `1.1.0` to `2.0.0`. -6. Remove the common system-prompt reference to supplied recent changes. - Review every prompt body to ensure none instructs the model to infer or - discuss changes from an absent comparison field. -7. Update prompt-asset and adapter integration fixtures to use exact version - `2.0.0`. Do not change generated-text output schemas or report templates; - neither directly depends on Recent Changes. - -### Tests - -Run: - -```sh -go test ./internal/promptinput ./internal/promptassets ./internal/report -go test ./internal/app ./internal/adapters/promptkit -go test ./... -git diff --check -``` - -Add or retain focused tests proving that serialized v4 packages omit -`recent_changes` entirely and every embedded prompt resolves at `2.0.0`. - -### Exit Gate - -No generated prompt package or active app workflow contains Recent Changes, -while report generation and the still-transitional persistence workflow -continue to function. - -## Stage 3: Delete Dormant Local Comparison Policy - -### Goal - -Remove comparison code and configuration that no active report workflow uses. - -### Work - -1. Delete `internal/changes` and its tests. -2. Remove `RecentChangeConfig`, `Config.RecentChange`, its defaults, - validation, YAML handling, test fixtures, and maintained example values. -3. Add a strict-loading regression test showing that a top-level - `recent_change:` stanza is now rejected as unknown. Do not silently ignore - the obsolete field. -4. Remove stale Recent Changes imports, helpers, test builders, and comments - throughout app, config, prompt input, facts, forecast, briefing, generated - text, and report code. -5. Leave report comparison declarations and state prior-lookup code only where - the still-supported historical `inspect prior` path requires them. They are - removed with inspection in Stage 8; do not invent a new consumer. - -### Tests - -Run: - -```sh -go test ./internal/config ./internal/app ./internal/promptinput -go test ./... -git diff --check -``` - -### Exit Gate - -No active generation code or configuration surface implements local change -detection, and the legacy field fails strict configuration loading. - -## Stage 4: Establish The Operator-Owned Output Contract - -### Goal - -Make every successful action select and atomically write its final output even -while the old managed workspace still exists behind the workflow. - -### Work - -1. Add an injectable `WorkingDir string` to `cli.Runner`. When empty, - production calls `os.Getwd` once per action; tests supply an absolute - temporary directory without calling `os.Chdir`. -2. Pass the captured absolute working directory through `GenerateRequest` and - `BatchRequest`. Resolve relative `--out` and `--out-dir` values against it - and clean the resulting absolute paths. -3. Centralize output filenames rather than duplicating them in CLI and app: - - Daily uses `daily-YYYY-MM-DD.md` with the resolved valid-period start in - the effective report timezone; - - Today, Tomorrow, and Hourly use `today.md`, `tomorrow.md`, and - `hourly.md`; - - dynamic Daily batch items use the same date-qualified rule. - Rename `report.Definition.BatchOutputName` to `OutputName` and rename - app-internal “output copy” fields/helpers to “output” terminology. Preserve - the external Distributor template variable `batch_output_name` and populate - it from the selected output filename. -4. When `--out` is absent, resolve the single-report default after the report - period is known. When `--out-dir` is absent, use the captured working - directory for all batch items. -5. Require a non-empty absolute final output path before collection. Reject a - filesystem root or a destination that resolves to a directory. Let atomic - publication create missing parent directories for a valid file path. -6. Continue using `fileutil.WriteFileAtomic` or an equivalently narrow helper. - An existing destination may be replaced only after the complete new report - has been written and closed successfully. -7. Change help and flag descriptions so `--out` and `--out-dir` select report - destinations rather than “extra copies.” -8. Update app and CLI tests for defaults, explicit absolute and relative - overrides, Daily date naming, batch naming, existing-file preservation on - failure, and absolute summary paths. - -### Tests - -Run: - -```sh -go test ./internal/report ./internal/fileutil -go test ./internal/app ./internal/cli -go test ./... -git diff --check -``` - -### Exit Gate - -Every successful single or batch item has one selected operator output path, -omitted flags use the captured working directory, and failed publication -cannot corrupt an existing destination. - -## Stage 5: Separate Explicit Debug Capture From State - -### Goal - -Preserve secure opt-in Promptkit diagnostics without retaining a dependency on -the package that owns the obsolete workspace. - -### Work - -1. Move `internal/state/debug_writer.go` and its focused tests to - `internal/promptdebug`. Move rather than duplicate the implementation. -2. Preserve: - - empty-path disablement without filesystem access; - - absolute non-root destination validation; - - safe report/date/RunID path segments; - - directory mode `0700` and file mode `0600`; - - endpoint sanitization and parameter redaction; - - credential exclusion; and - - preparation-callback failure preventing provider execution. -3. Update app imports and debug tests to use the new package. Do not move - ordinary artifacts into the debug package. -4. Leave the transitional `DataPackagePath` execution fields intact while - persisted receipts still validate them. Stage 7 removes those fields with - their final consumer. - -### Tests - -Run: - -```sh -go test ./internal/promptdebug ./internal/promptexec -go test ./internal/adapters/promptkit ./internal/app ./internal/cli -go test ./... -git diff --check -``` - -### Exit Gate - -Debug capture is independent of `internal/state`, and its security and -preparation-callback behavior are unchanged. - -## Stage 6: Remove Notification Persistence - -### Goal - -Make Distributor delivery an active-workflow result that consumes final output -files and writes no Weatherreporter receipt. - -### Work - -1. Refactor `notifyReport` and `notifyBatch` so they do not accept a - `state.Store` and do not call any `SaveDistributorNotification` operation. - Delete notification-artifact conversion and persistence helpers from app. -2. For single reports, build the notification request from the resolved report - identity, active RunID/timestamps, and selected `OutputPath`. Remove - “managed report” terminology from code and safe errors. -3. For batches, use each successful item's `OutputPath` as the Distributor - source. Preserve bundle-path rendering, duplicate bundle-path rejection, - batch inclusion metadata, all-success gating, and one batch upload. -4. Remove `NotificationPath` and batch notification `Path` fields from app - results and CLI summaries. Preserve remote run, pipeline, bundle, - idempotency, status, timing, and safe-error fields. -5. Preserve ordering: - - local atomic publication precedes a single notification; - - a notification failure leaves the output and returns a failure; - - batch notification is skipped after any report failure; - - successful batch items remain successful when only batch notification - fails. -6. Delete tests that assert notification receipt files. Replace them with - focused interaction tests proving source paths, ordering, skip behavior, - one-call behavior, and failure propagation. - -### Tests - -Run: - -```sh -go test ./internal/app ./internal/adapters/distributor -go test ./internal/cli -go test ./... -git diff --check -``` - -### Exit Gate - -Distributor behavior uses operator outputs and returns active status without -creating notification artifacts or depending on state. - -## Stage 7: Replace The Persisted Generation Workflow - -### Goal - -Convert the shared single-report generation path into a state-free in-memory -pipeline used by both `generate` and `run`. - -### Work - -1. Replace the persistence-oriented `promptReportWorkflow` with a cohesive - workflow that: - - initializes a safe partial `ReportResult` from `report.Resolved`; - - builds collected and derived facts; - - builds the module snapshot in memory; - - builds `briefing.Metadata` and the v4 prompt package in memory; - - marshals YAML in memory; - - executes Promptkit with the existing preparation callback; - - writes requested preparation and execution debug captures only; - - requires a completed passed Promptkit validation; - - validates and decodes generated text; - - builds the render context and renders Markdown in memory; - - atomically writes the selected output; and - - notifies only after publication when notification is enabled. -2. Delete all ordinary calls that save module snapshots, data packages, - prompt preparation/execution, raw output, validated output, render context, - metadata, or managed reports. -3. Remove `DataPackagePath` from `promptexec.ExecuteRequest`, - `promptexec.Preparation`, `promptexec.Execution`, and debug artifact - mappings. Change the Promptkit adapter input from `InlineWithURI` using a - filesystem path to `promptkit.Inline`; the input remains copied inline and - its hashes remain available. Advance the changed debug schema identifiers - from `weatherreporter.prompt_preparation_debug.v1` and - `weatherreporter.prompt_execution_debug.v1` to their corresponding `v2` - identifiers without adding compatibility readers. -4. Preserve failure classification and context. A debug-write error remains an - invalid-configuration failure and the preparation callback must still stop - the provider when it fails. Operational provider failure and completed - validation rejection remain distinct. -5. Redesign `ReportResult` as an active result rather than an artifact index. - Retain only: - - report ID/name, prompt ID/version, RunID, generation time, timezone, and - valid period; - - selected profile ID, backend ID, and model name once inspected; - - source warnings; - - validation status when execution reached validation; - - final `OutputPath` once publication succeeds; - - `LLMDebugPath` when explicitly created; and - - notification result. - Do not retain module snapshots, prompt packages, generated bodies, render - contexts, state metadata, or transient paths in the result. -6. Make the CLI generate summary map directly from this active result and - remove `ReportPath`, `MetadataPath`, `DataPackagePath`, - `PreparationPath`, `ExecutionPath`, `GeneratedTextRawPath`, - `GeneratedTextPath`, `RenderContextPath`, and `NotificationPath`. -7. Apply the same safe identity/profile/warning/output fields to - `BatchReportResult`. Keep batch failure accounting and sequential execution. -8. Remove `Store` from `GenerateRequest` and `BatchRequest`. Remove - `defaultStore` and all persistence-only finalization helpers in this stage. -9. Keep app tests behavioral. Replace receipt/checkpoint assertions with a - compact matrix covering success, each consequential failure boundary, - partial results, inspection-before-collection, output atomicity, effective - profile propagation, debug capture, and notification sequencing. +1. In the report workflow, check the active context after validation and + rendering have completed and immediately before calling the atomic file + writer. +2. If `ctx.Err()` is `context.Canceled`, wrap it in a + `promptexec.Canceled` error; if it is `context.DeadlineExceeded`, wrap it in + a `promptexec.DeadlineExceeded` error. Return that classified error through + the existing report-error surface. Preserve `errors.Is` behavior for the + underlying context error; do not collapse either case into a generic + rendering or output error. +3. Do not remove an output that was published before a later cancellation. + Cancellation observed after publication remains subject to the existing + notification and result behavior. +4. Add a focused app regression test whose fake executor returns valid output + but cancels the context before returning. Prove that generation fails and a + pre-existing destination remains byte-for-byte unchanged. This test must not + depend on the fake executor voluntarily returning a cancellation error. +5. Ensure the same workflow check protects every single-report and batch item; + do not add duplicate cancellation logic in CLI or batch orchestration. ### Tests @@ -418,142 +164,98 @@ Run: ```sh go test ./internal/app -go test ./internal/cli ./internal/adapters/promptkit go test ./... +go test -race ./internal/app git diff --check ``` ### Exit Gate -Single and batch generation share a state-free pipeline, ordinary success -leaves only selected outputs, and no app production path imports -`internal/state`. +Cancellation observable before publication prevents the atomic write, the +selected destination remains unchanged, and cancellation identity is retained. -## Stage 8: Remove Historical Inspection And Prior Compatibility +## Stage 15: Preflight Every Planned Batch Output ### Goal -Delete public and internal surfaces whose only purpose is reading prior runs. +Ensure a structural destination error cannot appear midway through a batch +after earlier reports have already been published. ### Work -1. Delete `internal/app/inspect.go` and all inspection request/result types and - focused tests. -2. Remove the top-level `inspect` dispatch, parsers, `--limit` handling, - command tables, help text, and CLI tests for: - `reports`, `metadata`, `modules`, `data-package`, `prior`, and `sources`. - `inspect` becomes an unknown command; do not retain a tombstone command. -3. Remove report `ComparisonStrategy`, comparison constants, - `CompatiblePriorIDs`, compatibility helpers, and registry tests. -4. Remove `FindPriorSnapshot`, `PriorSnapshot`, prior compatibility helpers, - and their tests from the still-transitional state package so it continues - to compile until full deletion. -5. Remove state load/list methods that existed only for CLI inspection where - doing so is clean and local. Do not spend effort preserving a smaller - historical reader that Stage 9 will delete. -6. Ensure `generate` and `run` parsing, help, summaries, and error behavior - remain intact. +1. After weather collection and `planBatchRun` have produced the complete + dynamic report set, resolve and validate the output path for every planned + report before executing the first report prompt. +2. Store each validated absolute output path with its planned report for use by + the generation loop. Do not recompute or revalidate destinations inside the + loop. +3. Treat any invalid path, including an existing directory at a report's final + filename, as a batch preflight error. Return before Promptkit execution and + before publication of any batch item. Weather collection may already have + occurred because eligible Daily dates depend on collected coverage. +4. Keep missing parent-directory creation in the atomic publication helper; + preflight must not create report files or introduce a new managed directory + lifecycle. +5. Add a focused app test using a real temporary output directory where a later + planned filename, such as `tomorrow.md`, already exists as a directory. + Assert that prompt inspection still occurs before collection, the executor's + prompt-execution method is never called, and no earlier report output is + created or replaced. +6. Retain the existing independent report-failure behavior after successful + preflight: a provider, validation, rendering, or publication failure for one + report remains an item failure, later items continue, and successful outputs + remain available. ### Tests Run: ```sh -go test ./internal/report ./internal/app ./internal/cli ./internal/state -go test ./... -go run ./cmd/weatherreporter --help -git diff --check -``` - -Assert that `inspect` is rejected as an unknown command and no inspection -subcommand appears in help. - -### Exit Gate - -No public or app-level historical inspection contract remains, and report -definitions contain no prior-run compatibility policy. - -## Stage 9: Delete The Workspace And State Subsystem - -### Goal - -Remove the now-unreferenced durable-state implementation and configuration. - -### Work - -1. Delete the remaining `internal/state` package and all of its tests. Do not - preserve metadata structs, artifact schemas, validators, path builders, - filesystem stores, or compatibility readers in another package. -2. Remove `WorkspaceConfig`, `Config.Workspace`, workspace defaults, - validation, YAML fixtures, and test helpers. -3. Add strict-loading coverage showing that a top-level `workspace:` stanza is - rejected as unknown. Existing legacy configuration is intentionally not - accepted. -4. Remove workspace and Recent Changes sections from maintained examples. - Keep every example complete, secret-free, and accepted by the production - loader. -5. Remove dead app code exposed by the deletion, including `FetchBundle`, - `FetchAndSaveBundle`, store helpers, artifact conversion helpers, copy - helpers, imports, and persistence-only test infrastructure. Neither fetch - helper currently has a production caller. -6. Keep state-independent atomic output helpers in `internal/fileutil`, update - their package comments to describe operator-owned outputs, and delete the - now-unused `CopyFileAtomic` helper. -7. Verify production Go code contains no import of `internal/state` and no - construction of a directory named `workspace`. - -### Tests - -Run: - -```sh -go test ./internal/config ./internal/fileutil ./internal/app ./internal/cli +go test ./internal/app +go test ./internal/fileutil go test ./... git diff --check ``` ### Exit Gate -`internal/state` and `workspace` configuration no longer exist, legacy fields -fail strict loading, and normal execution has no application-owned durable -state mechanism. +Every selected batch destination is validated before any batch report executes, +and a destination collision cannot yield an unreported partial batch. -## Stage 10: Consolidate Stateless Behavioral Coverage +## Stage 16: Separate Report Counts From Batch Notification Status ### Goal -Review the rewritten suite as a whole and retain a lean set of tests that -protect the new risks without preserving deleted implementation choreography. +Restore coherent batch counters while preserving failed status and non-zero +exit behavior when the batch notification fails. ### Work -1. Audit app and CLI tests that were rewritten in earlier stages. Consolidate - overlapping cases and delete helpers whose only value was constructing - stores, metadata, artifact paths, or prior snapshots. -2. Ensure durable offline coverage at the narrowest stable boundary for: - - default and explicit absolute output selection; - - relative override resolution from an injected working directory; - - Daily valid-date filenames and distinct multi-day batch outputs; - - preservation of an existing destination on generation, rendering, - output-write, or cancellation before publication; - - retention of the newly published output when notification fails; - - no output on pre-publication failure; - - successful items surviving partial batch failure; - - batch notification only after all outputs exist; - - Distributor source and bundle paths; - - inspection/profile validation before weather collection; - - safe effective profile/backend/model and source warnings in summaries; - - absence of historical/transient paths in JSON summaries; - - no ordinary debug directory creation; - - secure explicit debug capture and credential redaction; and - - no default workspace or intermediate artifacts after repeated successes - and failures. -3. Use real temporary directories and real internal rendering/file helpers. - Fake only Weather API collection, Promptkit/provider execution, clocks, and - Distributor delivery. -4. Avoid tests of private phase ordering unless the ordering is a stated - external requirement. Do not replace deleted state tests with broad - filesystem snapshots. +1. Define and enforce the invariant + `Total == Succeeded + Failed == len(Reports)` after report execution. + `Succeeded` and `Failed` count only report-item statuses. +2. Remove the increment of `BatchResult.Failed` when `notifyBatch` returns an + error. Preserve the failed top-level `BatchNotificationResult`, including + its safe error and identity fields. +3. Update the `RunBatch` wrapper to return `BatchError` when either a report + failed or the top-level batch notification failed. Keep + `RunBatchDetailed` returning the populated result according to its existing + detailed-result contract. +4. Retain CLI behavior in which the batch summary status is failed and the + command exits unsuccessfully for a batch notification failure, even though + all report counters show success. +5. Make `BatchError` use report counts only for report-failure wording and its + existing notification-specific wording when the reports succeeded but the + notification failed. +6. Add focused app and CLI tests for a batch whose reports all publish + successfully but whose batch notifier fails. Assert: + - `Total == Succeeded == len(Reports)` and `Failed == 0`; + - every output remains present; + - the top-level notification status is `failed`; + - the summary status is `failed`; + - the returned error describes notification failure rather than claiming a + report failed; and + - the action exits unsuccessfully. ### Tests @@ -561,156 +263,95 @@ Run: ```sh go test ./internal/app ./internal/cli -go test ./internal/fileutil ./internal/promptdebug go test ./... git diff --check ``` ### Exit Gate -The suite protects stateless product behavior and consequential failure -boundaries without retaining obsolete state-oriented fixtures or redundant -mock choreography. +Batch report counters are internally consistent, while notification failure +still produces a failed summary, safe diagnostic, retained outputs, and +non-zero command result. -## Stage 11: Publish User, Operator, And Policy Documentation +## Stage 17: Remove Impossible Per-Report Notification State ### Goal -Publish the user-visible, operational, and normative stateless contracts in -their canonical owners. +Align batch result types and stderr output with the architecture in which +per-report notification is suppressed and delivery is represented once at the +batch level. ### Work -1. Update `README.md` so the shortest useful command relies on the documented - current-directory output and links to canonical CLI and operations details. -2. Update `docs/cli.md` with: - - removal of `inspect`; - - default and explicit output behavior; - - absolute `outputPath` summaries; - - retained identity/profile/model/warning/debug/notification fields; and - - removal of historical artifact path fields. -3. Update `docs/config.md` and maintained examples to remove `workspace` and - `recent_change`. Keep Promptkit debug and notification configuration in - their existing canonical owners. -4. Rewrite `docs/operations.md` around operator-owned outputs, atomic - replacement, batch partial success, Distributor ordering, explicit debug - capture, and precise manual legacy-workspace cleanup. Remove all inspection, - metadata, receipt, recovery, and managed-workspace procedures. -5. Update `docs/policy/architecture.md` to make stateless execution, - in-memory processing, operator-owned atomic outputs, and state-free - notification normative. Remove `internal/state` and prior-comparison - ownership and persistence invariants. -6. Update `docs/policy/documentation.md` so Operations owns output lifecycle, - diagnosis, legacy cleanup, and explicit debug handling rather than physical - workspace and inspection contracts. -7. Update `docs/policy/testing.md` where its examples assign behavior to state - tests or name persistence/inspection as current app workflow contracts. - Preserve its general risk-based guidance. -8. Update `docs/development.md` package inventory and task guide for the - stateless package layout, removed inspection workflow, and operator-output - boundary. Route detailed subsystem work to its canonical internal or - integration document rather than duplicating it. -9. Leave `docs/releases/v0.9.0.md` unchanged as a historical record. Do not - create a new release note without a selected version. Confirm that the - future note must call out removed commands and fields, default outputs, - prompt contract changes, and manual legacy cleanup. - -### Tests - -Run: - -```sh -go test ./... -go run ./cmd/weatherreporter --help -git diff --check -``` - -Check every changed repository-relative link, maintained YAML example, command, -flag, field, and output filename against executable sources. - -### Exit Gate - -User, operator, policy, development, and example documentation describe the -implemented stateless contract, and historical release notes remain unchanged. - -## Stage 12: Reconcile Internal And Integration Documentation - -### Goal - -Bring maintainer-facing subsystem and external-contract documentation into -line with the implemented package boundaries without duplicating user-facing -contracts. - -### Work - -1. Delete `docs/internal/state.md` and `docs/internal/changes.md` and repair all - incoming links. -2. Update app orchestration internals for the in-memory workflow, partial - active results, atomic output, notification order, and batch isolation. -3. Update CLI internals for working-directory capture, destination resolution, - stateless summaries, and removal of inspection dispatch. -4. Update prompt-input internals for v4 without `recent_changes`, report - registry internals for output naming without comparison policy, and module - and generated-text internals only where their actual contracts changed. -5. Update Promptkit adapter internals and - `docs/integrations/promptkit.md` for inline input without a filesystem URI, - exact prompt version `2.0.0`, active safe provenance, and explicit debug - capture. -6. Update Distributor adapter and integration documents so source files are - operator outputs and notification state is not retained. Preserve the - remote API and bundle-path contract. -7. Review weather-data, collection, template, and other internal/integration - documents for stale references, but edit only documents whose owned - contracts changed. -8. Do not mark either roadmap implemented in this stage; Stage 13 owns that - status transition after the repository-wide gate. Do not turn either - roadmap into a second current-state reference. +1. Remove `NotificationStatus`, `NotificationRunID`, + `NotificationPipelineID`, and `NotificationError` from + `BatchReportResult`. +2. Remove the unreachable copying of `ReportResult.Notification` into a batch + item. Rename `copyBatchReportPaths` to reflect that it copies the current + safe report result fields rather than only paths, or replace it with an + equally clear narrow helper. +3. Remove per-report notification formatting from `writeBatchStatus`. Keep the + top-level `batchNotification` status line and the batch summary + `notification` object unchanged. +4. Search tests and documentation for the removed per-report fields. Delete + stale assertions or descriptions rather than adding compatibility fields; + no backward compatibility is required for this pre-release result cleanup. +5. Retain single-report notification fields and behavior. This stage changes + only batch report items. ### Tests Run: ```sh +go test ./internal/app ./internal/cli go test ./... git diff --check ``` -Validate every changed repository-relative link and verify each named package, -schema, prompt version, field, and integration path against executable sources. - ### Exit Gate -No current internal or integration document describes local comparison, -historical state, inspection, managed-report sources, or durable prompt -provenance, and roadmap status accurately reflects completion. +Batch items expose only report-generation facts, and all batch delivery state +is represented by the single top-level notification result. -## Stage 13: Run The Repository Exit Gate +## Stage 18: Reconcile Documentation And Run The Remediation Exit Gate ### Goal -Verify the complete refactor as one coherent change and remove any remaining -legacy coupling before declaring implementation complete. +Remove the remaining documentation and repository-hygiene traces of the former +workspace architecture and verify the remediated implementation as a whole. ### Work -1. Run `gofmt` on every changed Go file and review the complete diff for stale - compatibility code, unnecessary abstractions, accidental content capture, - or user-owned unrelated changes. -2. Search production code, tests, examples, and current-state documentation - for obsolete `internal/state` imports, `workspace` and `recent_change` - configuration, `recent_changes` prompt input, comparison strategy fields, - inspection commands, managed-report terminology, and removed artifact-path - summary fields. Historical `v0.9.0` release notes and roadmap discussion of - removed behavior are allowed. -3. Verify the built help contains only `generate` and `run` action families - plus top-level help/version behavior. -4. Verify from focused offline CLI tests that commands run in a clean temporary - working directory and leave only expected Markdown outputs unless a debug - directory was explicitly supplied. -5. Confirm no test, example, or default path depends on the developer machine, - live credentials, or external services. -6. Mark the roadmap and plan complete only after all gates pass. Do not create - or tag a release in this implementation plan. +1. In `docs/roadmap/ephemeral-state.md`, rename `Current State` to + `Former State`, convert its description of persistence and Recent Changes to + past tense, and reconcile other implementation-future phrasing with the + roadmap's Implemented status. Preserve the roadmap's product intent and + desired end state; do not turn it back into a staged plan. +2. Remove the root-level `/workspace` ignore rule from `.gitignore` and update + its adjacent comment. Legacy workspace cleanup remains an explicit operator + procedure in `docs/operations.md`; removing the ignore rule must not delete + any operator data or add automated cleanup. +3. Review the canonical CLI, operations, architecture, app-orchestration, and + testing documentation for the Stage 14-17 behavior. Update only documents + whose owned contract changed, avoiding duplicate definitions. +4. Search production code, tests, examples, and current-state documentation + for: + - stale per-report batch notification fields; + - report counters that include notification failures; + - output validation performed inside the batch execution loop; + - claims that cancellation can publish an output; + - present-tense descriptions of the removed workspace; and + - active ignore rules or defaults that conceal a workspace tree. +5. Confirm ordinary generation and batch tests use isolated temporary + directories and leave only the selected Markdown outputs unless explicit + prompt debug capture is requested. +6. Run formatting and all repository validation gates. Review the complete + remediation diff for unrelated changes, content leakage, compatibility + shims, and unnecessary abstractions. +7. When every exit gate passes, mark Stages 14-18 and this plan Complete. Keep + the feature roadmap marked Implemented. Do not create, tag, or publish a + release in this plan. ### Tests @@ -726,14 +367,17 @@ go run ./cmd/weatherreporter --help git diff --check ``` +Verify all changed repository-relative documentation links resolve and inspect +`git status --short` for an accidentally created root-level `workspace` tree. + ### Exit Gate -Weatherreporter builds and passes its deterministic offline suite; ordinary -actions are stateless and leave only selected outputs; debug and Distributor -boundaries remain safe; documentation matches implementation; and no required -work remains. +The remediated stateless workflow honors cancellation before publication, +preflights complete batch destinations, reports coherent batch counts, exposes +only reachable notification state, and has documentation and repository +hygiene consistent with the implemented architecture. ## Open Questions -None. The roadmap supplies all product and policy decisions required to -implement these stages. +None. The roadmap, accepted ADR, and remediation decisions above provide all +product and architectural choices required to implement Stages 14-18. diff --git a/internal/app/generation_test.go b/internal/app/generation_test.go index 3d978b3..522fc1d 100644 --- a/internal/app/generation_test.go +++ b/internal/app/generation_test.go @@ -28,13 +28,13 @@ func (c *generationCollector) Run(context.Context, collect.Request) (*collect.Re } type generationExecutor struct { - called bool - inspectErr error - executeErr error - respectCancellation bool - validation promptexec.ValidationStatus - rawOutput []byte - failedPrompt string + called bool + inspectErr error + executeErr error + cancelBeforeReturn context.CancelFunc + validation promptexec.ValidationStatus + rawOutput []byte + failedPrompt string } func (e generationExecutor) InspectPrompt(_ context.Context, id, version string) (promptexec.PromptInspection, error) { @@ -47,10 +47,7 @@ func (e generationExecutor) InspectPrompt(_ context.Context, id, version string) func (generationExecutor) InspectProfile(_ context.Context, id string) (promptexec.ProfileInspection, error) { return promptexec.ProfileInspection{ProfileID: id, BackendID: "fixture", ModelName: "fixture-model"}, nil } -func (e *generationExecutor) Execute(ctx context.Context, req promptexec.ExecuteRequest, callback promptexec.PreparationCallback) (*promptexec.Execution, error) { - if e.respectCancellation && ctx.Err() != nil { - return nil, ctx.Err() - } +func (e *generationExecutor) Execute(_ context.Context, req promptexec.ExecuteRequest, callback promptexec.PreparationCallback) (*promptexec.Execution, error) { stamp := time.Date(2026, 5, 29, 15, 0, 0, 0, time.UTC) if err := callback(promptexec.Preparation{PromptID: req.PromptID, PromptVersion: req.PromptVersion, PromptHash: "prompt-hash", RenderedPromptHash: "rendered-hash", ProfileID: req.ProfileID, BackendID: "fixture", ModelName: "fixture-model", StartedAt: stamp, EndedAt: stamp}, nil); err != nil { return nil, err @@ -70,6 +67,9 @@ func (e *generationExecutor) Execute(ctx context.Context, req promptexec.Execute if rawOutput == nil { rawOutput = []byte(`{"summary":"Showers are possible during the selected day.","forecast_discussion":["A front will keep rain chances in the forecast."],"precipitation_timing":"Rain is most likely during the afternoon."}`) } + if e.cancelBeforeReturn != nil { + e.cancelBeforeReturn() + } return &promptexec.Execution{RunID: "provider-run", PromptID: req.PromptID, PromptVersion: req.PromptVersion, PromptHash: "prompt-hash", RenderedPromptHash: "rendered-hash", ProfileID: req.ProfileID, BackendID: "fixture", ModelName: "fixture-model", StartedAt: stamp, EndedAt: stamp, RawOutput: rawOutput, Validation: promptexec.NewValidation(status, "json_schema", generationDefinitionForPrompt(req.PromptID).GeneratedTextSchemaID+".generated_text.schema.json", nil)}, nil } @@ -137,25 +137,17 @@ func TestGenerateDetailedPreservesDestinationBeforePublish(t *testing.T) { for _, scenario := range []struct { name string executor generationExecutor - cancel bool }{ {name: "generation", executor: generationExecutor{executeErr: errors.New("provider unavailable")}}, {name: "render", executor: generationExecutor{rawOutput: []byte(`{"summary":""}`)}}, - {name: "cancellation", executor: generationExecutor{respectCancellation: true}, cancel: true}, } { t.Run(scenario.name, func(t *testing.T) { outputPath := filepath.Join(t.TempDir(), "daily.md") if err := os.WriteFile(outputPath, []byte("previous report"), 0o600); err != nil { t.Fatal(err) } - ctx := context.Background() - if scenario.cancel { - var cancel context.CancelFunc - ctx, cancel = context.WithCancel(ctx) - cancel() - } bundle := generationBundle(t) - result, err := GenerateDetailed(ctx, 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: &generationCollector{bundle: &bundle}, Executor: &scenario.executor}) + 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: &generationCollector{bundle: &bundle}, Executor: &scenario.executor}) data, readErr := os.ReadFile(outputPath) if err == nil || result == nil || readErr != nil || string(data) != "previous report" { t.Fatalf("GenerateDetailed() result/error/output = %#v/%v/%q (%v)", result, err, data, readErr) @@ -164,6 +156,45 @@ func TestGenerateDetailedPreservesDestinationBeforePublish(t *testing.T) { } } +func TestGenerateDetailedPreservesDestinationWhenContextCancelsBeforePublication(t *testing.T) { + outputPath := filepath.Join(t.TempDir(), "daily.md") + const previousReport = "previous report" + if err := os.WriteFile(outputPath, []byte(previousReport), 0o600); err != nil { + t.Fatal(err) + } + ctx, cancel := context.WithCancel(context.Background()) + bundle := generationBundle(t) + result, err := GenerateDetailed(ctx, 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: &generationCollector{bundle: &bundle}, Executor: &generationExecutor{cancelBeforeReturn: cancel}, + }) + data, readErr := os.ReadFile(outputPath) + if !errors.Is(err, context.Canceled) || promptexec.CategoryOf(err) != promptexec.Canceled || result == nil || result.OutputPath != "" || readErr != nil || string(data) != previousReport { + t.Fatalf("GenerateDetailed() result/error/output = %#v/%v/%q (%v)", result, err, data, readErr) + } +} + +func TestGenerateDetailedPreservesDestinationWhenContextDeadlineExpiresBeforePublication(t *testing.T) { + outputPath := filepath.Join(t.TempDir(), "daily.md") + const previousReport = "previous report" + if err := os.WriteFile(outputPath, []byte(previousReport), 0o600); err != nil { + t.Fatal(err) + } + ctx, cancel := context.WithDeadline(context.Background(), time.Unix(0, 0)) + defer cancel() + bundle := generationBundle(t) + result, err := GenerateDetailed(ctx, 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: &generationCollector{bundle: &bundle}, Executor: &generationExecutor{}, + }) + data, readErr := os.ReadFile(outputPath) + if !errors.Is(err, context.DeadlineExceeded) || promptexec.CategoryOf(err) != promptexec.DeadlineExceeded || result == nil || result.OutputPath != "" || readErr != nil || string(data) != previousReport { + t.Fatalf("GenerateDetailed() result/error/output = %#v/%v/%q (%v)", result, err, data, readErr) + } +} + func TestGenerateDetailedRetainsPublishedOutputWhenNotificationFails(t *testing.T) { cfg := generationConfig() cfg.Notify.Distributor.Enabled = true diff --git a/internal/app/prompt_generate.go b/internal/app/prompt_generate.go index 71d909c..c1310af 100644 --- a/internal/app/prompt_generate.go +++ b/internal/app/prompt_generate.go @@ -2,6 +2,7 @@ package app import ( "context" + "errors" "fmt" "gitea.maximumdirect.net/eric/weatherreporter/internal/briefing" @@ -169,6 +170,9 @@ func (w *promptReportWorkflow) renderAndPublish(raw []byte) (*ReportResult, erro if err != nil { return w.result, w.reportError("render template", err) } + if err := publicationContextError(w.ctx); err != nil { + return w.result, w.reportError("publish report", err) + } if err := fileutil.WriteFileAtomic(w.req.OutputPath, rendered); err != nil { return w.result, err } @@ -195,6 +199,16 @@ func classifiedPromptError(operation string, err error) error { return promptexec.NewError(promptexec.Generation, operation, err) } +func publicationContextError(ctx context.Context) error { + if err := ctx.Err(); err != nil { + if errors.Is(err, context.DeadlineExceeded) { + return promptexec.NewError(promptexec.DeadlineExceeded, "context expired before output publication", err) + } + return promptexec.NewError(promptexec.Canceled, "context canceled before output publication", err) + } + return nil +} + func promptDebugWriteError(err error) error { return promptexec.NewError(promptexec.InvalidConfiguration, "write requested prompt debug artifact", err) }