Tighten workspace artifact path handling

docs/roadmap/implementation.md

@@ -1,194 +0,0 @@
-# Workspace Layout Implementation Plan
-
-## Purpose
-
-This document is the staged implementation plan for
-[workspace.md](workspace.md). It is written for an LLM coding agent that will
-implement the workspace artifact layout refactor in order.
-
-The feature is complete when every managed local run artifact uses the
-kind-first, date-scoped layout defined in `docs/roadmap/workspace.md`, metadata
-and inspection link those paths correctly, and distributor upload behavior is
-unchanged.
-
-## Ground Rules
-
-- Review `docs/policy/architecture.md`, `docs/policy/development.md`, and
-  `docs/policy/documentation.md` before editing code.
-- Keep path construction owned by `internal/state`.
-- Do not add user-visible workspace path template configuration.
-- Do not use distributor bundle paths for managed workspace paths.
-- Do not implement cleanup, migration tooling, or a local publication mirror in
-  this work.
-- Keep optional `--out` and `--out-dir` copies separate from managed workspace
-  artifacts.
-
-## Target Path Contract
-
-State-managed run artifacts must use these relative path shapes under
-`workspace.root`:
-
-```text
-reports/<artifact_group>/<valid_start_date>/report.<run_id>.md
-
-snapshots/<artifact_group>/<valid_start_date>/modules.<run_id>.json
-snapshots/<artifact_group>/<valid_start_date>/metadata.<run_id>.json
-snapshots/<artifact_group>/<valid_start_date>/generated_text_raw.<run_id>.json
-snapshots/<artifact_group>/<valid_start_date>/generated_text_result.<run_id>.json
-snapshots/<artifact_group>/<valid_start_date>/generated_text.<run_id>.json
-snapshots/<artifact_group>/<valid_start_date>/render_context.<run_id>.json
-
-data-packages/<artifact_group>/<valid_start_date>/data_package.<run_id>.yaml
-preflight/<artifact_group>/<valid_start_date>/render.<run_id>.json
-
-notifications/<artifact_group>/<valid_start_date>/distributor.<run_id>.json
-notifications/batches/<batch>/<batch_start_date>/distributor.<batch_run_id>.json
-```
-
-`valid_start_date` and `batch_start_date` are local dates in the effective
-report timezone already carried by the resolved report or batch reference.
-
-## Stage 1: State Path Construction
-
-Goal: switch the canonical managed path builder to the target layout.
-
-Implementation:
-
-- Update `internal/state.FilesystemStore.Paths` to build all `ArtifactPaths`
-  using the target path contract above.
-- Keep existing validation for workspace subdirectories, RunID, and report
-  artifact group.
-- Keep `PrepareRenderedReport` responsible for creating the parent report
-  directory before Scriptorium or template rendering writes Markdown.
-- Update `BatchDistributorNotificationPath` to use
-  `distributor.<batch_run_id>.json`.
-
-Tests:
-
-- Update state path tests to assert the exact new relative paths for at least
-  `daily`, `today`, `tomorrow`, and `hourly`.
-- Update batch notification path tests for
-  `notifications/batches/<batch>/<date>/distributor.<batch_run_id>.json`.
-- Run:
-
-```bash
-go test ./internal/state
-```
-
-Completion criteria:
-
-- `FilesystemStore.Paths` returns only target-layout managed paths.
-- Existing save methods write to the new paths through the existing
-  `ArtifactPaths` fields.
-
-## Stage 2: Metadata Discovery And Prior Lookup
-
-Goal: make inspection and prior lookup discover the new metadata filenames.
-
-Implementation:
-
-- Replace metadata filename checks that currently rely on the old
-  `.metadata.json` suffix with a helper that matches only
-  `metadata.<run_id>.json`.
-- Use that helper in `ListReports` and `FindPriorSnapshot`.
-- Do not add legacy metadata filename fallback in this refactor; migration is
-  explicitly deferred by the feature roadmap.
-- Keep `LoadMetadataByRunID` behavior based on `ListReports`.
-
-Tests:
-
-- Update report listing tests to create/read `metadata.<run_id>.json`.
-- Update prior lookup tests to confirm comparable metadata is found under the
-  new filename.
-- Add or adjust one negative test so non-metadata JSON in the snapshot tree is
-  ignored.
-- Run:
-
-```bash
-go test ./internal/state
-```
-
-Completion criteria:
-
-- Inspection and prior lookup work with new metadata names.
-- Generated-text, module, and render-context JSON files are not mistaken for
-  metadata.
-
-## Stage 3: App Workflow Expectations
-
-Goal: update app-level tests and any path assumptions outside `internal/state`.
-
-Implementation:
-
-- Do not duplicate path-building logic in `internal/app`; continue asking the
-  state store for managed paths.
-- Update app tests that assert managed report paths, data package paths,
-  generated-text paths, render context paths, notification source paths, batch
-  included report source paths, and result JSON paths.
-- Preserve the invariant that distributor uploads use the managed Markdown
-  report path as the upload source and map it to separate distributor bundle
-  paths.
-- Preserve `--out` and `--out-dir` behavior as extra copies outside the managed
-  report path.
-
-Tests:
-
-- Run:
-
-```bash
-go test ./internal/app ./internal/cli
-```
-
-Completion criteria:
-
-- App workflows still generate, save metadata, notify distributor, and report
-  paths using state-owned managed paths.
-- No app code constructs the new managed path layout manually.
-
-## Stage 4: Documentation
-
-Goal: move the implemented layout into maintained docs after code behavior has
-changed.
-
-Documentation changes:
-
-- Update `docs/internal/state.md` with the new target managed layout and
-  filename convention.
-- Update `docs/operations.md` filesystem layout examples and artifact wording.
-- Update `docs/config.md` only if workspace field descriptions need
-  clarification.
-- Keep cleanup, migration, and local publication mirror details in roadmap
-  docs until those features are implemented.
-
-Validation:
-
-```bash
-go test ./internal/state ./internal/app ./internal/config
-go test ./...
-go run ./cmd/weatherreporter --help
-git diff --check
-```
-
-Completion criteria:
-
-- Implemented docs match the new behavior.
-- Non-roadmap docs do not describe deferred cleanup or local publication mirror
-  behavior as implemented.
-
-## Final Verification
-
-Before considering the feature complete, run:
-
-```bash
-go test ./...
-go run ./cmd/weatherreporter --help
-git diff --check
-rg "\\.metadata\\.json|\\.modules\\.json|\\.data_package\\.yaml|\\.render\\.json|\\.distributor\\.json|generated_text\\.raw|generated_text\\.run" internal docs examples --glob '!docs/roadmap/**'
-```
-
-The final `rg` should find no old managed filename patterns in implemented
-code, maintained docs, or examples.
-
-## Open Questions
-
-None. The feature roadmap decisions are sufficient for implementation.

docs/roadmap/workspace.md

@@ -1,119 +0,0 @@
-# Workspace Artifact Layout Roadmap
-
-## Purpose
-
-This roadmap defines the target local workspace layout for artifacts generated
-by weatherreporter.
-
-Weatherreporter currently writes managed local artifacts for each run and also
-maps the managed Markdown report into distributor bundle paths. Distributor
-bundle paths are publication paths and may include overwrite-oriented aliases
-such as `hourly/index.md`, `today/index.md`, and
-`daily/<valid_start_date>/index.md`. Local workspace paths have a different
-job: they preserve run-specific source artifacts for inspection, troubleshooting,
-retry diagnosis, and future cleanup.
-
-## Locked Decisions
-
-- Keep managed local workspace paths separate from distributor bundle paths.
-- Do not use distributor-style `index.md` aliases as the canonical managed
-  local artifact paths.
-- Store managed local artifacts as immutable run artifacts.
-- Group every durable run artifact by artifact family, report artifact group,
-  and local `valid_start_date`.
-- Use `valid_start_date` in the effective report timezone as the directory key.
-- Include the RunID in every durable run artifact filename.
-- Prefer artifact-kind-first filenames: `<kind>.<run_id>.<extension>`.
-- Keep optional `--out` and `--out-dir` copies separate from managed workspace
-  artifacts.
-- Future cleanup should delete old managed artifact date directories by
-  `valid_start_date`; cleanup should be narrow and opt-in.
-- If a local publication preview is added later, put it in a separate mirror
-  tree and treat it as rebuildable output, not canonical run state.
-
-## Target Managed Layout
-
-The target managed workspace layout is:
-
-```text
-<workspace.root>/
-  reports/<artifact_group>/<valid_start_date>/report.<run_id>.md
-
-  snapshots/<artifact_group>/<valid_start_date>/modules.<run_id>.json
-  snapshots/<artifact_group>/<valid_start_date>/metadata.<run_id>.json
-  snapshots/<artifact_group>/<valid_start_date>/generated_text_raw.<run_id>.json
-  snapshots/<artifact_group>/<valid_start_date>/generated_text_result.<run_id>.json
-  snapshots/<artifact_group>/<valid_start_date>/generated_text.<run_id>.json
-  snapshots/<artifact_group>/<valid_start_date>/render_context.<run_id>.json
-
-  data-packages/<artifact_group>/<valid_start_date>/data_package.<run_id>.yaml
-  preflight/<artifact_group>/<valid_start_date>/render.<run_id>.json
-
-  notifications/<artifact_group>/<valid_start_date>/distributor.<run_id>.json
-  notifications/batches/<batch>/<batch_start_date>/distributor.<batch_run_id>.json
-```
-
-`artifact_group` comes from the report definition. `valid_start_date` is the
-resolved report valid period start date in the effective report timezone.
-`batch_start_date` is the batch start date in the effective report timezone.
-
-Generated-text-template reports use the generated-text artifacts. Markdown-path
-reports omit generated-text artifacts they do not produce.
-
-## Cleanup Compatibility
-
-The target layout is designed to support a future cleanup command that removes
-managed local artifacts where `valid_start_date` is more than two days before
-the current local date.
-
-Cleanup can operate by deleting matching date directories across managed
-artifact families:
-
-```text
-reports/<artifact_group>/<old_date>/
-snapshots/<artifact_group>/<old_date>/
-data-packages/<artifact_group>/<old_date>/
-preflight/<artifact_group>/<old_date>/
-notifications/<artifact_group>/<old_date>/
-```
-
-The command should not infer dates from RunIDs when the directory date is
-available. It should use the effective configured report timezone for "current
-date" and for interpreting `valid_start_date` directories.
-
-Batch notification cleanup should use `batch_start_date` under
-`notifications/batches/<batch>/`. A future local publication mirror, if added,
-should be ignored by cleanup or rebuilt after cleanup; it should not be treated
-as run history.
-
-## Intended Final State
-
-`internal/state` owns the managed workspace layout and path construction. The
-app layer continues to ask the state store for managed paths and records those
-paths in metadata and command results.
-
-Metadata links every artifact produced for a run using the new managed paths.
-Inspection and prior-snapshot lookup continue to read metadata rather than
-rendered Markdown text. Distributor uploads continue to use the managed
-Markdown report as the source file and map it to separately resolved
-distributor bundle paths.
-
-Existing workspace configuration fields remain meaningful:
-
-- `workspace.root`
-- `workspace.snapshots_dir`
-- `workspace.reports_dir`
-- `workspace.data_packages_dir`
-- `workspace.preflight_dir`
-- `workspace.notifications_dir`
-
-This roadmap does not require a new user-visible workspace path template config
-surface.
-
-## Deferred Work
-
-- A cleanup command for managed local artifacts.
-- Cleanup dry-run output, retention configuration, and deletion confirmation
-  behavior.
-- A rebuildable local publication mirror that uses distributor-style paths.
-- Migration tooling for existing pre-refactor workspace directories.

internal/app/app_test.go

@@ -3346,15 +3346,22 @@ func plannedBatchNotificationReports(t *testing.T, cfg config.Config, batch Batc
 	if err != nil {
 		t.Fatalf("planBatchRun() error = %v", err)
 	}
-	reportDir := filepath.Join(t.TempDir(), "managed-reports")
+	store, err := state.NewFilesystemStore(cfg.Workspace)
+	if err != nil {
+		t.Fatalf("NewFilesystemStore() error = %v", err)
+	}
 	results := make([]BatchReportResult, 0, len(planned))
 	for _, item := range planned {
 		metadata := item.Resolved.Metadata()
+		paths, err := store.Paths(item.Resolved)
+		if err != nil {
+			t.Fatalf("Paths(%s) error = %v", item.Resolved.Definition.ID, err)
+		}
 		results = append(results, BatchReportResult{
 			ReportID:   item.Resolved.Definition.ID,
 			RunID:      metadata.RunID,
 			Status:     "succeeded",
-			ReportPath: filepath.Join(reportDir, string(item.Resolved.Definition.ID), metadata.RunID+".md"),
+			ReportPath: paths.RenderedReport,
 		})
 	}
 	return planned, results

internal/state/filesystem.go

@@ -83,8 +83,8 @@ func (s *FilesystemStore) Paths(resolved report.Resolved) (ArtifactPaths, error)
 		return ArtifactPaths{}, fmt.Errorf("state store is required")
 	}
 	metadata := resolved.Metadata()
-	if metadata.RunID == "" {
+	if err := validatePathSegment("run id", metadata.RunID); err != nil {
-		return ArtifactPaths{}, fmt.Errorf("run id is required")
+		return ArtifactPaths{}, err
 	}
 	group := resolved.Definition.ArtifactGroup
 	if group == "" {

internal/state/filesystem_test.go

@@ -69,6 +69,20 @@ func TestDailyPathsUseRunIDValidDateDisambiguator(t *testing.T) {
 	}
 }
 
+func TestPathsRejectRunIDPathSeparators(t *testing.T) {
+	store := newTestStore(t)
+	resolved := resolveDailyAt(t, "2026-05-29T05:00:00-05:00")
+	resolved.Definition.ID = report.ID("daily/bad")
+
+	_, err := store.Paths(resolved)
+	if err == nil {
+		t.Fatal("Paths() error = nil, want invalid run id error")
+	}
+	if !strings.Contains(err.Error(), "run id must not contain path separators") {
+		t.Fatalf("error = %q, want run id path separator context", err.Error())
+	}
+}
+
 func TestBatchDistributorNotificationPathUsesWorkspaceBatchDateAndRunID(t *testing.T) {
 	store := newTestStore(t)
 	location := mustLoadStateLocation(t, "America/Chicago")