378 lines
17 KiB
Markdown
378 lines
17 KiB
Markdown
# Configurable Output Directory Implementation Plan
|
|
|
|
Status: Complete.
|
|
|
|
## Purpose And Authority
|
|
|
|
This plan implements the accepted [Configurable Output Directory
|
|
Roadmap](output-directory.md). The feature roadmap is authoritative for user
|
|
intent, scope, policy choices, non-goals, and the desired end state. This
|
|
document owns implementation order, concrete changes, verification, and exit
|
|
gates.
|
|
|
|
The target is one optional `output.directory` configuration value that selects
|
|
the ordinary publication directory for both single reports and batches while
|
|
preserving explicit CLI overrides, current defaults, stateless execution,
|
|
atomic publication, and notification ordering.
|
|
|
|
This plan follows the repository's [architecture](../policy/architecture.md),
|
|
[documentation](../policy/documentation.md), and
|
|
[testing](../policy/testing.md) policies.
|
|
|
|
## Implementation Rules
|
|
|
|
1. Implement the stages in numeric order. Keep the repository buildable and
|
|
the default suite passing at every stage.
|
|
2. Use `GOWORK=off` for all Go validation so the sibling workspace cannot
|
|
supply unpublished dependency changes.
|
|
3. Keep this change within the configurable-output-directory scope. Do not
|
|
implement any part of the accepted
|
|
[LLM profile comparison roadmap](profile-comparison.md).
|
|
4. Preserve one operator-owned Markdown output per generated report. Do not
|
|
add a workspace, manifest, history, cache, retention policy, directory scan,
|
|
or managed cleanup behavior.
|
|
5. Preserve current report filenames, report periods, batch membership,
|
|
Promptkit inspection and execution, generated-text validation, rendering,
|
|
cancellation, atomic file publication, action summaries, and Distributor
|
|
notification ordering.
|
|
6. Keep CLI flag parsing and current-working-directory capture in
|
|
`internal/cli`, configuration loading and validation in `internal/config`,
|
|
and effective destination resolution and publication in `internal/app`.
|
|
7. Keep tests deterministic, offline, credential-free, and independent of
|
|
process-wide `os.Chdir` or machine-specific directories. Use `t.TempDir()`
|
|
and injected working directories.
|
|
8. Update current-state documentation only when the corresponding behavior is
|
|
implemented. Do not create release notes, select a release version, tag, or
|
|
publish a release in these stages.
|
|
9. Run `gofmt` on changed Go files and `git diff --check` before completing
|
|
every stage. Do not weaken unrelated tests or rewrite historical release
|
|
notes.
|
|
|
|
## Locked Product And Path Decisions
|
|
|
|
Implement exactly this optional configuration shape:
|
|
|
|
```yaml
|
|
output:
|
|
directory: /var/lib/weatherreporter/reports
|
|
```
|
|
|
|
The following decisions are fixed:
|
|
|
|
- Add `config.OutputConfig` with one `Directory string` field and add
|
|
`Config.Output` with YAML key `output`. Do not add other output settings.
|
|
- The built-in default is the empty string. Omitted and explicitly empty
|
|
values preserve current-directory behavior.
|
|
- A whitespace-only configured value is invalid. Preserve a nonblank value as
|
|
supplied during configuration loading; do not trim it into a different
|
|
filesystem path.
|
|
- Configuration loading performs no filesystem lookup and creates no
|
|
directory. Runtime output preflight owns path cleaning and filesystem
|
|
inspection.
|
|
- A relative configured directory resolves against the absolute working
|
|
directory captured for the invocation, never against the configuration-file
|
|
directory. There is no environment-variable or home-directory expansion.
|
|
- A nonexistent configured directory is valid and is created only as needed by
|
|
ordinary atomic report publication.
|
|
- An existing configured path that is not a directory, or cannot be inspected,
|
|
fails through the existing output-preflight boundary before prompt
|
|
inspection, weather collection, model execution, or output replacement.
|
|
- Single-report precedence is explicit `--out` path, then
|
|
`output.directory` plus the report's existing default filename, then the
|
|
working directory plus that filename.
|
|
- Batch precedence is explicit `--out-dir`, then `output.directory`, then the
|
|
working directory.
|
|
- An explicit `--out` is the complete file destination. A relative `--out`
|
|
remains relative to the invocation working directory and is never rebased
|
|
beneath `output.directory`.
|
|
- Configuration must still pass strict validation before any command runs.
|
|
After a valid configuration loads, explicit output flags completely ignore
|
|
the configured directory for runtime resolution and filesystem inspection.
|
|
They retain their current absolute and relative semantics.
|
|
- Successful result and summary paths remain cleaned absolute paths. Default
|
|
filenames and all output, cancellation, and notification safety guarantees
|
|
remain unchanged.
|
|
|
|
## Stage 1: Add The Configuration Contract
|
|
|
|
### Goal
|
|
|
|
Make `output.directory` a strict, validated configuration value without
|
|
changing runtime destination behavior or current-state documentation yet.
|
|
|
|
### Work
|
|
|
|
1. In `internal/config/config.go`, add:
|
|
- `Output OutputConfig` to `Config` with YAML key `output`; and
|
|
- `OutputConfig` containing only `Directory string` with YAML key
|
|
`directory`.
|
|
2. In `internal/config/defaults.go`, initialize `Output.Directory` to the empty
|
|
string explicitly so the public default is visible beside the other
|
|
configuration defaults.
|
|
3. In `internal/config/validate.go`, reject `Output.Directory` only when it is
|
|
nonempty and `strings.TrimSpace` finds no non-whitespace character. Use an
|
|
actionable error that identifies `output.directory`. Do not clean, make
|
|
absolute, stat, create, or otherwise mutate the path during validation.
|
|
4. Rely on the existing `yaml.Decoder.KnownFields(true)` boundary for strict
|
|
nested decoding. Do not introduce custom YAML unmarshalling or a second
|
|
unknown-field mechanism for `OutputConfig`.
|
|
5. Add focused configuration tests that prove:
|
|
- the exact empty default;
|
|
- omission and explicit `directory: ""` are accepted;
|
|
- representative absolute and relative values load without mutation;
|
|
- a whitespace-only value fails with `output.directory` context;
|
|
- an unknown field beneath `output` fails strict loading; and
|
|
- constructed and YAML-loaded configurations receive the same validation
|
|
result.
|
|
6. Keep the maintained example and current-state documentation unchanged in
|
|
this stage; they are updated after runtime behavior lands.
|
|
|
|
### Tests
|
|
|
|
Run:
|
|
|
|
```sh
|
|
GOWORK=off go test -count=1 ./internal/config
|
|
GOWORK=off go test -count=1 ./...
|
|
git diff --check
|
|
```
|
|
|
|
### Exit Gate
|
|
|
|
The configuration package exposes and validates the additive field, strict
|
|
loading rejects malformed output configuration, existing configurations remain
|
|
valid, and runtime output behavior is still unchanged.
|
|
|
|
## Stage 2: Centralize And Apply Effective Destination Resolution
|
|
|
|
### Goal
|
|
|
|
Apply the locked precedence to single-report and batch orchestration through
|
|
one coherent app-owned output-resolution boundary.
|
|
|
|
### Work
|
|
|
|
1. Create `internal/app/output.go` and move the existing app-owned output
|
|
helpers out of `app.go` without changing their established safety behavior:
|
|
`prepareBatchOutputs`, `plannedBatchOutputPath`,
|
|
`resolveReportOutputPath`, `resolveOutputDir`, `resolveOutputPath`,
|
|
`validateWorkingDir`, and `validateOutputPath`. Clean up imports in
|
|
`app.go`; do not export these helpers or introduce a new package.
|
|
2. Add one narrow directory-precedence helper in `output.go`. It must select a
|
|
nonempty explicit directory first, otherwise the configured directory, and
|
|
then pass that selected value to the existing directory resolver, whose
|
|
empty case returns the validated working directory. Do not inspect the
|
|
lower-precedence configured path when an explicit directory is present.
|
|
3. Extend the single-report resolver to accept the configured directory and
|
|
implement this exact branch:
|
|
- obtain the existing default output name from `report.Resolved`;
|
|
- when `GenerateRequest.OutputPath` is nonempty, resolve and validate that
|
|
complete explicit path against `WorkingDir` and ignore the configured
|
|
directory;
|
|
- otherwise resolve the configured directory, falling back to
|
|
`WorkingDir`, and place the existing default filename beneath it; and
|
|
- validate and return one cleaned absolute final file path.
|
|
4. Change `GenerateDetailed` to pass `req.Config.Output.Directory` into that
|
|
resolver before debug-writer construction, prompt inspection, weather
|
|
collection, or model execution. Preserve its current partial-result and
|
|
destination-preservation behavior on resolution failure.
|
|
5. Change `RunBatchDetailed` to resolve its base directory from
|
|
`req.OutputDir`, `req.Config.Output.Directory`, and `req.WorkingDir` in that
|
|
order before debug-writer construction, prompt inspection, or collection.
|
|
Preserve data-dependent batch planning and the later preflight of every
|
|
final report file.
|
|
6. Do not add output-directory fields to `GenerateRequest` or `BatchRequest`.
|
|
Their existing explicit override fields remain CLI/action inputs, while
|
|
the configured fallback remains in `Config`.
|
|
7. Add focused `internal/app` tests, using existing real filesystem behavior
|
|
and project-owned fakes, that prove:
|
|
- omitted configuration retains current-directory destinations for a
|
|
representative single report and batch;
|
|
- absolute and relative configured directories produce the expected
|
|
cleaned absolute destinations;
|
|
- single-report `OutputPath` and batch `OutputDir` overrides win and do not
|
|
inspect or use a lower-precedence configured path;
|
|
- a configured existing non-directory fails before prompt inspection,
|
|
collection, model execution, publication, or notification;
|
|
- a missing configured directory is created through successful ordinary
|
|
publication;
|
|
- existing atomic replacement and cancellation tests continue to pass; and
|
|
- result paths and Distributor source paths still identify only the final
|
|
selected Markdown files.
|
|
8. Prefer a compact resolver table plus representative assembled workflows.
|
|
Do not duplicate every path case at configuration, CLI, and app layers.
|
|
|
|
### Tests
|
|
|
|
Run:
|
|
|
|
```sh
|
|
GOWORK=off go test -count=1 ./internal/app ./internal/fileutil
|
|
GOWORK=off go test -count=1 ./...
|
|
git diff --check
|
|
```
|
|
|
|
### Exit Gate
|
|
|
|
Direct app callers, ordinary `generate`, and batches use the locked
|
|
CLI/configuration/working-directory precedence; paths are validated before
|
|
expensive work; and output, cancellation, and notification invariants remain
|
|
intact.
|
|
|
|
## Stage 3: Complete CLI Coverage And Current-State Documentation
|
|
|
|
### Goal
|
|
|
|
Protect the assembled CLI-to-app contract and make every canonical document
|
|
and maintained example describe the now-implemented behavior exactly once.
|
|
|
|
### Work
|
|
|
|
1. Keep `internal/cli.resolveOutputOverride` limited to explicit `--out` and
|
|
`--out-dir` values. It must continue resolving relative flag values against
|
|
the captured working directory. Do not resolve `output.directory` in the
|
|
CLI or replace the raw configured value in `Config`; the app owns final
|
|
precedence and runtime path validation.
|
|
2. Add or update focused `internal/cli` request-construction tests to prove:
|
|
- a configured relative or absolute directory survives configuration
|
|
loading and reaches the app request while the absent CLI override remains
|
|
empty;
|
|
- relative and absolute `--out` and `--out-dir` values retain their current
|
|
resolution; and
|
|
- an explicit flag coexists with the loaded configured value so app-owned
|
|
precedence can ignore the latter.
|
|
Keep these tests narrow; app tests own final path and publication outcomes.
|
|
3. Add this illustrative stanza to `examples/config.yml`:
|
|
|
|
```yaml
|
|
output:
|
|
directory: /var/lib/weatherreporter/reports
|
|
```
|
|
|
|
Keep `examples/minimal-config.yml` unchanged so it continues to demonstrate
|
|
the empty-default path. Extend `TestLoadExampleConfig` to assert the exact
|
|
maintained example value.
|
|
4. Update canonical current-state documentation according to
|
|
`docs/policy/documentation.md`:
|
|
- `docs/config.md` owns the field, empty default, whitespace validation,
|
|
absolute and relative semantics, lack of expansion, and configuration
|
|
versus flag precedence;
|
|
- `docs/cli.md` owns the unchanged flag syntax and the effective
|
|
flag/configuration/working-directory selection visible to users;
|
|
- `docs/operations.md` owns normal operation with a configured publication
|
|
directory, explicit one-command overrides, missing-directory creation,
|
|
atomic replacement, and failure handling;
|
|
- `README.md` updates only its short quickstart destination summary and
|
|
links to the canonical configuration/CLI references;
|
|
- `docs/policy/architecture.md` states that configuration or explicit CLI
|
|
input selects the operator-owned output destination without changing the
|
|
stateless boundary;
|
|
- `docs/internal/cli.md` describes capture and transport of the configured
|
|
fallback plus explicit overrides without duplicating the field reference;
|
|
and
|
|
- `docs/internal/app-orchestration.md` describes app-owned effective
|
|
destination precedence and the unchanged single and batch preflight
|
|
ordering.
|
|
5. Do not change historical release notes. Do not duplicate the complete field
|
|
contract outside `docs/config.md`, complete flag syntax outside
|
|
`docs/cli.md`, or operational procedures outside `docs/operations.md`.
|
|
6. Review all changed commands, values, paths, and examples against executable
|
|
code. Follow every added or changed repository-relative link and confirm its
|
|
target exists.
|
|
|
|
### Tests
|
|
|
|
Run:
|
|
|
|
```sh
|
|
GOWORK=off go test -count=1 ./internal/config ./internal/cli ./internal/app
|
|
GOWORK=off go run ./cmd/weatherreporter --help
|
|
GOWORK=off go test -count=1 ./...
|
|
git diff --check
|
|
```
|
|
|
|
Confirm the maintained full and minimal examples both load through the
|
|
configuration test suite and no current-state document still claims that the
|
|
working directory is always the no-flag destination.
|
|
|
|
### Exit Gate
|
|
|
|
The CLI-to-app boundary is protected without duplicating app behavior tests,
|
|
the maintained example is executable configuration, and all canonical
|
|
current-state documentation agrees with the implemented contract.
|
|
|
|
## Stage 4: Reconcile Roadmaps And Run The Final Gate
|
|
|
|
### Goal
|
|
|
|
Verify the complete feature in the same module-isolated environment used for
|
|
release preparation and leave roadmap status accurate for post-implementation
|
|
review.
|
|
|
|
### Work
|
|
|
|
1. Review the complete diff against the feature roadmap, this plan, and all
|
|
documents under `docs/policy/`. Remove incidental refactors, redundant
|
|
tests, duplicate documentation, and any behavior outside the accepted
|
|
scope.
|
|
2. Confirm repository hygiene:
|
|
- no tracked `go.work` or `go.work.sum`;
|
|
- no `vendor` directory or `go.mod` replacement;
|
|
- no generated binaries, test output, credentials, private paths, or
|
|
temporary filesystem artifacts; and
|
|
- no unintended changes to the profile-comparison feature.
|
|
3. After every implementation and documentation gate passes, update
|
|
`docs/roadmap/output-directory.md` from `Accepted; unimplemented` to
|
|
`Implemented; retained temporarily for post-implementation review`.
|
|
Preserve its target-state purpose and policy content until roadmap cleanup
|
|
is separately authorized.
|
|
4. Update the prerequisite section in
|
|
`docs/roadmap/profile-comparison.md` to state that configurable output
|
|
directories are implemented. Link its current behavior summary to
|
|
`docs/config.md` and `docs/operations.md` rather than duplicating their
|
|
contracts. Do not otherwise advance or implement profile comparison.
|
|
5. Mark this plan `Status: Complete` only after the final gate succeeds. Do not
|
|
delete either roadmap or this plan in this stage; repository roadmap cleanup
|
|
follows post-implementation review.
|
|
|
|
### Tests
|
|
|
|
Run from the repository root:
|
|
|
|
```sh
|
|
set -eu
|
|
test -z "$(git ls-files go.work go.work.sum)"
|
|
test ! -e vendor
|
|
if grep -Eq '^[[:space:]]*replace([[:space:]]|\()' go.mod; then
|
|
printf '%s\n' 'go.mod contains a replacement' >&2
|
|
exit 1
|
|
fi
|
|
GOWORK=off go test -count=1 ./...
|
|
GOWORK=off go test -race -count=1 ./...
|
|
GOWORK=off go vet ./...
|
|
GOWORK=off go build ./...
|
|
GOWORK=off go mod tidy -diff
|
|
unformatted="$(git ls-files '*.go' | while IFS= read -r file; do gofmt -l "$file"; done)"
|
|
test -z "$unformatted"
|
|
GOWORK=off go run ./cmd/weatherreporter --help >/dev/null
|
|
git diff --check
|
|
git diff --cached --check
|
|
```
|
|
|
|
Follow every added or changed local documentation link and verify the full and
|
|
minimal maintained configuration examples through their tests.
|
|
|
|
### Exit Gate
|
|
|
|
The complete repository passes normal, race-enabled, vet, build, module,
|
|
formatting, help, documentation, and hygiene checks with `GOWORK=off`; roadmap
|
|
status accurately distinguishes the implemented output-directory prerequisite
|
|
from the still-unimplemented profile-comparison feature; and no required work
|
|
remains.
|
|
|
|
## Open Questions
|
|
|
|
None. The feature roadmap and this plan define every product, architecture,
|
|
precedence, path, compatibility, testing, documentation, and completion choice
|
|
required for implementation.
|