notarius/docs/roadmap/implementation.md

# ADR-0006 Findings 1–3 Remediation Plan

This document is the executable implementation plan for correcting Findings 1,
2, and 3 from the post-implementation review of ADR-0006:

1. unredacted resolved-pipeline data in the debug summary;
2. collision-prone output run directories; and
3. missing failure `run-report.json` artifacts.

Finding 4—the broad loss of CLI and configuration regression coverage—is
explicitly deferred. The focused regression tests required to prove these three
fixes are in scope; restoring or redesigning the general test suites is not.

The audience is an LLM coding agent. Implement the stages in order. Each stage
must finish with a compiling, tested repository.

## Execution Rules

Before every stage:

1. read [Development](../development.md), both documents under `docs/policy/`,
   [ADR-0006](../adr/0006-separate-output-cache-and-debug-state.md), this plan,
   and every stage-specific document listed below;
2. inspect `git status` and preserve user changes;
3. inspect the focused implementation and tests before editing; and
4. confirm all earlier stages are complete.

During every stage:

- keep the CLI as the run-identity and physical-path composition root;
- preserve durable logical output schemas, checkpoint identity and wire
  compatibility, chunk-plan behavior, debug layout, exit codes, and current
  configuration contracts unless this plan expressly changes them;
- never allow a secondary debug persistence error to hide the primary command
  error;
- add focused tests for the changed behavior without expanding into Finding 4;
  and
- update a canonical current-behavior document only if the implemented contract
  changes or its existing statement is inaccurate.

At the end of every stage:

1. run the focused tests listed for that stage;
2. run `go test ./...`;
3. run `go vet ./...`;
4. run `go build ./cmd/notarius`;
5. run `git diff --check`; and
6. mark the stage complete here only after all checks pass.

If repository reality conflicts with this plan, update the plan before making a
materially different design choice. Do not silently weaken redaction, output
exclusivity, or failure reporting.

## Fixed Decisions

### Redacted summary boundary

- Every file under `summary/` is a redacted artifact. The fact that the adjacent
  `trace/` may contain application data does not relax this boundary.
- `resolved-pipeline.json` retains its current logical shape and useful module,
  validator, digest, capability, and reference-provenance data, but all module
  and validator option maps pass through the same recursive sensitive-key
  redaction used by `effective-config.json`.
- The debug-bundle writer must not accept an arbitrary resolved pipeline for a
  method whose name promises redacted summary output. Introduce a narrow
  payload interface, analogous to `RedactedSummaryPayload`, that can only be
  satisfied by a producer responsible for redaction. `config.EffectiveConfig`
  implements it and returns a deep-cloned, redacted
  `pipeline.ResolvedPipeline`.
- The CLI passes the effective configuration to that method, not
  `effective.ResolvedPipeline` directly. Do not expose the private redaction
  helper merely to keep the unsafe call shape.
- Redaction must cover input, chunk, output, lane extract/merge/normalize,
  resolved validator-chain bindings, deprecated lane validator bindings, and
  recursively nested option maps and lists. Safe option values remain intact.
- Reference contents remain excluded through the existing `json:"-"`
  `ReferenceSet` contract. Reference provenance and configured paths may remain
  in the summary.
- Credentials and sensitive values must not be copied into errors while testing
  this behavior.

### Run identity and output allocation

- The CLI owns one run ID before allocating debug or executing the pipeline.
  Output, debug, manifests, metadata, stdout, and stderr use that same ID.
- The default ID format becomes:

  ```text
  run-<started-at-unix-nanoseconds>-<32-lowercase-hex-characters>
  ```

  The suffix is 16 bytes from `crypto/rand`. Time preserves useful ordering;
  randomness makes IDs collision-resistant across processes and hosts sharing a
  root.
- Add an injectable `RunIDGenerator func(time.Time) (string, error)` to
  `internal/cli.Options`. The production default uses `crypto/rand.Reader`.
  Tests inject deterministic valid IDs. Do not retain package-global clocks or
  random sources for this path.
- Generate the ID after configuration and CLI validation but before debug
  allocation and pipeline resolution. A generation failure is a normal command
  failure with exit code `1` and cannot have a debug bundle because no run
  identity exists yet.
- Change debug allocation to accept the CLI-generated run ID and `startedAt`.
  It validates that the ID is one safe path component and creates that exact
  bundle with `os.Mkdir`, failing on an existing entry. Debug no longer creates
  a different ID from its own clock or retries by changing identity.
- Durable output allocation is exclusive. Validate every logical output name
  first, create the output parent as needed, then create
  `<output-root>/<run-id>` with `os.Mkdir`, not `os.MkdirAll`. An existing run
  directory is an error; Notarius never writes into it.
- Nested logical output directories may still use `MkdirAll` after the exclusive
  run directory is owned by the invocation. File writes remain atomic.
- If output allocation or a later output write fails, retain any newly created
  partial run directory for inspection. Never remove or overwrite an existing
  run directory. Document this only if current Operations wording needs
  clarification.
- Keep output directory permissions and logical file permissions unchanged;
  this remediation addresses identity and exclusivity, not the public output
  permission contract.

### Terminal debug reports

- Every debug-enabled invocation whose bundle was successfully allocated makes
  one best-effort attempt to write a terminal `run-report.json`, whether the
  command succeeds or fails.
- A failure report uses `Succeeded: false`. Populate fields only from state that
  is known at the failure point:
  - `run_id`, requested `pipeline_id`, and `debug_path` are always known after
    bundle allocation;
  - `output_path` is included once the effective output root and run ID are
    known;
  - output, rejection, warning, and validation fields are populated when a
    `pipeline.RunOutput` is available; otherwise their zero values are retained.
- `error.log` remains the human-readable primary error artifact. Do not add the
  error string to `RunReport`; this avoids a second unstructured error surface
  and preserves the existing report schema.
- Introduce one CLI-owned run-state value that accumulates terminal-report data
  as resolution and execution progress. Success and failure reporting both
  derive `debugbundle.RunReport` from this value rather than constructing
  unrelated literals.
- Add a single summary terminalization operation that attempts
  `run-report.json` and, on failure, `error.log`. On a successful command it
  writes only the success report. On a failed command it writes the failure
  report and error log, attempting both even if one write fails and joining
  secondary persistence errors for stderr reporting.
- Terminalization is invoked at most once. If the primary failure is itself a
  summary persistence failure, do not recursively retry the failed operation;
  make one terminalization attempt that excludes the artifact already known to
  have failed. In particular, a failure to write `run-report.json` must never
  call itself again.
- Stderr prints the primary error first, then any terminalization error, then
  the allocated debug path. Exit code remains `1`. A secondary error never
  replaces or wraps away the primary error.
- Runs without debug perform no summary work and retain current concise stderr
  behavior.

## Stage 1: Enforce redaction for resolved-pipeline summaries

**Status:** Complete

### Objective

Close the summary credential leak and make the redacted-payload boundary
difficult to bypass accidentally.

### Read first

- `internal/core/config/redaction.go`
- `internal/core/config/effective_config.go`
- `internal/core/debugbundle/summary.go`
- the summary-writing section of `internal/cli/run.go`
- `internal/core/config/v3_test.go`
- `internal/cli/state_hardening_test.go`
- [Architecture: State, Output, And Safety](../policy/architecture.md#state-output-and-safety)

### Implement

1. Add the resolved-pipeline redacted payload interface to `debugbundle` and
   change `SummaryWriter.WriteResolvedPipeline` to accept it rather than `any`.
2. Implement the interface on `config.EffectiveConfig` using the existing deep
   clone/redaction path.
3. Audit recursive option redaction. Extend it only where nested list/map values
   can currently retain a value under a sensitive key.
4. Change the CLI call to pass `effective` through the redacted interface.
5. Keep `effective-config.json` behavior and `resolved-pipeline.json` logical
   structure unchanged apart from required redaction.

### Focused tests

- Unit-test a resolved pipeline containing distinct sentinel secrets in every
  module and validator binding listed under Fixed Decisions.
- Include nested maps and lists, safe neighboring values, and materialized
  reference content. Assert secrets and reference contents are absent, redaction
  markers are present, and safe provenance remains.
- Run a debug-enabled CLI invocation whose accepted fake module options include
  sensitive keys. Inspect both `effective-config.json` and
  `resolved-pipeline.json` and prove neither contains the sentinels.
- Assert mutation of a redacted payload cannot alter the effective configuration
  or resolved pipeline.

Run at minimum:

```sh
go test ./internal/core/config ./internal/core/debugbundle ./internal/cli
```

### Exit criteria

No arbitrary resolved pipeline can be passed to the redacted summary writer;
all resolved bindings are recursively redacted; focused leak tests fail against
the old implementation and pass against the new one; and all repository checks
pass.

## Stage 2: Make run identity collision-resistant and output exclusive

**Status:** Not started

### Objective

Ensure separate invocations can never silently share or overwrite one durable
output run directory.

### Read first

- run initialization and output writing in `internal/cli/run.go`
- `internal/core/debugbundle/bundle.go`
- `internal/core/debugbundle/bundle_test.go`
- `internal/cli/state_surfaces_test.go`
- `internal/cli/state_hardening_test.go`
- [Operations: Output](../operations.md#output)

### Implement

1. Add the default cryptographic run-ID generator and inject it through
   `cli.Options` as specified above. Validate generated IDs before any path use.
2. Generate one ID from `startedAt` before debug allocation. Remove the debug
   allocator's package-global clock and identity generation.
3. Refactor `debugbundle.Allocate` to accept the run ID and start time and
   exclusively create that exact bundle.
4. Refactor output writing so the run directory is created exclusively after
   all logical file names validate. Return a contextual collision error without
   touching the existing directory.
5. Preserve atomic per-file writes, partial-new-directory behavior on later
   write failures, and the single ID across manifests and reported paths.
6. Update existing focused tests to inject deterministic run IDs rather than
   relying on timestamps or the debug allocator's global clock.

### Focused tests

- Verify the production generator's format and validate many generated IDs for
  uniqueness and path safety without asserting random bytes.
- Inject a deterministic ID and prove debug and output use the exact same run
  directory name and manifest run ID.
- Pre-create the target output run directory with sentinel files. Run the
  command and assert exit `1`, a collision error, and byte-for-byte preservation
  of the existing tree.
- Execute two invocations with the same injected ID and root. The first may
  succeed; the second must fail without changing the first output.
- Pre-create the target debug bundle and assert allocation fails without
  changing it.
- Prove nested logical output paths still work and unsafe logical paths are
  rejected before the run directory is created.
- Prove a later output-file failure retains only the newly allocated partial
  directory and never affects a sibling run.

Run at minimum:

```sh
go test ./internal/core/debugbundle ./internal/cli
```

### Exit criteria

Every invocation has one collision-resistant ID, output and debug allocation
are exclusive, an existing run directory is never reused or overwritten, and
all repository checks pass.

## Stage 3: Write terminal run reports for failures

**Status:** Not started

### Objective

Make every allocated debug summary record a structured terminal outcome while
preserving primary errors and preventing recursive persistence failures.

### Read first

- `internal/core/debugbundle/summary.go`
- failure and summary paths in `internal/cli/run.go`
- `internal/cli/state_hardening_test.go`
- [Run State Internals](../internal/state.md)
- [Operations: Debug Bundles](../operations.md#debug-bundles)
- [Operations: Failures And Warnings](../operations.md#failures-and-warnings)

### Implement

1. Add the CLI-owned run-state/report builder described under Fixed Decisions.
   Initialize it immediately after run-ID generation and update it as pipeline
   identity, output path, and `RunOutput` become available.
2. Add a debug-summary terminalization method or focused helper that writes the
   success report, or the failure report plus error log, exactly once.
3. Route every post-allocation failure through the common terminalization path:
   catalog and reference resolution, pipeline resolution, profile validation,
   reference materialization, registry and LLM construction, preparation, input
   read, cache/checkpoint construction, pipeline execution, summary writes,
   output allocation, and output writes.
4. Replace the existing success-only report literal with the same report
   builder and terminalization path.
5. Preserve failures before run-ID generation or debug allocation as stderr-only
   failures.
6. Keep explicit guards for failures caused by summary terminalization itself so
   the command never recursively rewrites `run-report.json` or `error.log`.

### Focused tests

- For resolution, pipeline, and output-write failures, assert both
  `run-report.json` and `error.log` exist, `succeeded` is false, and every known
  field is correct.
- For a pipeline failure with partial `RunOutput`, assert available counts,
  validation status, manifest, warnings, checkpoint events, and chunk-plan
  summary are retained.
- Assert a success report uses `succeeded: true` and contains the same paths and
  counts printed by the CLI.
- Inject independent failures for the run-report writer and error-log writer.
  Assert each operation is attempted no more than once, the primary error is
  printed first, the secondary error is reported, the debug path is printed,
  and exit code is `1`.
- Assert a pre-allocation configuration failure creates no report or debug root.
- Assert a non-debug failure performs no summary writes.

Run at minimum:

```sh
go test ./internal/core/debugbundle ./internal/cli
```

### Exit criteria

Every successfully allocated debug bundle has a best-effort terminal report;
ordinary failures retain both structured and textual outcomes; summary failures
cannot recurse or mask the primary error; and all repository checks pass.

## Stage 4: Final audit and close the remediation plan

**Status:** Not started

### Objective

Verify the three findings are closed, align any affected current-behavior
documentation, and remove this completed planning artifact.

### Read first

- the final code and tests from Stages 1–3
- [Documentation Policy](../policy/documentation.md)
- [Architecture Policy](../policy/architecture.md)
- [CLI Reference](../cli.md)
- [Operations](../operations.md)
- [Run State Internals](../internal/state.md)

### Implement

1. Audit every `summary/` writer call and confirm each accepts or constructs a
   redacted payload. Search for direct serialization of raw effective pipeline
   bindings into summary files.
2. Audit every output path and confirm no code writes into a pre-existing run
   directory. Confirm debug and output use one CLI-owned ID.
3. Audit every post-allocation return path and confirm it reaches terminal
   reporting exactly once or is itself an explicitly guarded terminalization
   failure.
4. Update Operations or Run State Internals only where the final implementation
   adds a useful current-behavior clarification about run IDs, exclusive output
   allocation, partial output after failure, or best-effort failure reports.
   Do not duplicate volatile CLI or configuration contracts.
5. Confirm Finding 4 remains deferred. Do not restore unrelated deleted tests,
   introduce a coverage threshold, or add a legacy-checkpoint fixture in this
   remediation.
6. After all validation succeeds and documentation is current, delete this
   implementation plan. The accepted ADR and canonical current-behavior
   documents remain authoritative.

### Validation

Run:

```sh
go test ./...
go vet ./...
go build ./cmd/notarius
git diff --check
```

Manually inspect the changed debug artifacts from one success, one pipeline
failure, and one output collision. Confirm no test fixture contains real
credentials or private data.

### Exit criteria

Findings 1–3 are closed with focused regression tests; durable output cannot be
overwritten by a run-ID collision; failure bundles have terminal reports;
summary redaction is enforced by contract; documentation is accurate; Finding
4 remains deferred; the plan is removed; and all checks pass.