Add implementation plan for CLI and configuration test coverage
This commit is contained in:
642
docs/roadmap/implementation.md
Normal file
642
docs/roadmap/implementation.md
Normal file
@@ -0,0 +1,642 @@
|
||||
# CLI and Configuration Contract Test Plan
|
||||
|
||||
This plan adds deliberate contract and risk-based test coverage for
|
||||
`internal/cli` and `internal/core/config`. It does not pursue a statement
|
||||
coverage target. Coverage reports are diagnostic input: they may reveal an
|
||||
important untested branch, but a percentage alone does not justify a test.
|
||||
|
||||
The audience is a relatively small LLM coding agent. Implement one stage per
|
||||
prompt, in order. Each stage must leave the repository compiling and passing
|
||||
all tests.
|
||||
|
||||
## Scope and fixed decisions
|
||||
|
||||
Tests added by this plan protect:
|
||||
|
||||
- external contracts documented in `docs/cli.md`, `docs/config.md`,
|
||||
`docs/operations.md`, and maintained examples;
|
||||
- internal contracts around configuration loading, precedence, validation,
|
||||
resolution, cloning, and redaction;
|
||||
- high-risk CLI composition paths involving run selection, references, LLM
|
||||
profiles, output, cache, resume, and debug state; and
|
||||
- production composition and the shortest maintained end-to-end workflows.
|
||||
|
||||
The following decisions are fixed:
|
||||
|
||||
- Do not set or enforce a repository-wide coverage percentage.
|
||||
- Do not restore deleted test files wholesale. The former suite predates
|
||||
ADR-0006 and includes obsolete workspace and diagnostics behavior. Historical
|
||||
tests immediately before commit `26142f0` may be consulted for fixture and
|
||||
harness ideas only; every restored assertion must be checked against current
|
||||
code and canonical documentation.
|
||||
- Prefer behavior tests at the narrowest useful boundary. Pure configuration
|
||||
rules belong in `internal/core/config`; CLI tests should prove parsing,
|
||||
composition, process-facing behavior, and cross-component wiring rather than
|
||||
repeat every configuration permutation.
|
||||
- Test stable observable facts: exit code, output stream, selected collaborator,
|
||||
logical artifact, path, or resolved field. Avoid exact whole-error strings,
|
||||
entire JSON snapshots, timestamps, random identifiers, and internal call
|
||||
sequences unless those are the contract under test.
|
||||
- Tests must be deterministic, offline, and secret-free. Inject catalogs,
|
||||
registries, clocks, run-ID generators, cache collaborators, and fake LLM
|
||||
clients. Use `t.TempDir`, `t.Setenv`, and `t.Chdir` where appropriate. Never
|
||||
use a real provider, credential, user cache directory, or shared output root.
|
||||
- Do not weaken a test to match an apparent defect. If current behavior
|
||||
conflicts with a canonical contract, stop the stage and report the conflict.
|
||||
Production fixes are outside this test-coverage plan unless separately
|
||||
authorized.
|
||||
- Preserve the focused ADR-0006 state tests already present. Consolidate
|
||||
helpers only when it materially reduces duplication and does not obscure the
|
||||
contract being tested.
|
||||
- Checkpoint wire compatibility is already owned by
|
||||
`internal/framework/checkpoint` tests. Do not add a second legacy checkpoint
|
||||
fixture in CLI tests unless a distinct CLI compatibility boundary is found.
|
||||
|
||||
## Execution rules for every stage
|
||||
|
||||
Before editing:
|
||||
|
||||
1. Read `docs/development.md` and follow its task-specific reading guide.
|
||||
2. Read both files under `docs/policy/`.
|
||||
3. Read this plan and all current source, tests, and canonical documents named
|
||||
by the stage.
|
||||
4. Run `git status --short`; preserve all existing changes.
|
||||
5. Run the stage's package tests once to establish a baseline.
|
||||
|
||||
While editing:
|
||||
|
||||
- Add tests only for the cases listed in the stage. Do not expand into adjacent
|
||||
subsystem redesign.
|
||||
- Use table-driven tests for validation and syntax matrices when the setup and
|
||||
assertion are genuinely shared.
|
||||
- Give failures enough context to identify the contract case.
|
||||
- Assert both the positive outcome and important negative side effects, such as
|
||||
a root not being resolved or a file not being created.
|
||||
- Do not run environment- or working-directory-mutating tests in parallel.
|
||||
|
||||
At the end of each stage:
|
||||
|
||||
1. Run the focused tests named by that stage.
|
||||
2. Run `go test ./...`.
|
||||
3. Run `go vet ./...`.
|
||||
4. Run `go build ./cmd/notarius`.
|
||||
5. Run `git diff --check`.
|
||||
6. Review the diff for assertions tied to obsolete implementation details.
|
||||
|
||||
Do not mark a stage complete until all checks pass.
|
||||
|
||||
## Stage 1: File schema, defaults, and decoding contracts
|
||||
|
||||
**Status:** Not started
|
||||
|
||||
### Objective
|
||||
|
||||
Protect the version 3 file format and the translation from YAML into the
|
||||
runtime configuration model.
|
||||
|
||||
### Read first
|
||||
|
||||
- `docs/config.md`, through **Module Bindings**
|
||||
- `internal/core/config/config.go`
|
||||
- `internal/core/config/file_config.go`
|
||||
- `internal/core/config/v3_test.go`
|
||||
|
||||
### Implement
|
||||
|
||||
Add focused tests in `internal/core/config/file_config_contract_test.go`. Reuse
|
||||
small helper constructors within that file; do not reproduce complete example
|
||||
files for unit cases.
|
||||
|
||||
Cover these contracts:
|
||||
|
||||
1. `Default` returns the documented concurrency, output, cache-family, debug,
|
||||
and empty-pipeline defaults. Mutating maps in one returned configuration
|
||||
must not affect a later `Default` result.
|
||||
2. A minimal `version: 3` file parses and applies over defaults.
|
||||
3. Missing, version 2, and unsupported versions fail with actionable version
|
||||
context before ordinary field decoding.
|
||||
4. Unknown top-level, pipeline, lane, and module-binding fields are rejected.
|
||||
Include removed `workspace`, `diagnostics`, and `llm_profiles` fields as
|
||||
representative compatibility failures, without testing every old field.
|
||||
5. Shorthand and object module bindings preserve module, profile, retries,
|
||||
options, references, and the distinction between omitted validators and an
|
||||
explicit empty validator chain.
|
||||
6. Pipeline, target-local, and lane compatibility reference maps apply with the
|
||||
documented precedence, including `extract.references` overriding the
|
||||
lane-level alias.
|
||||
7. Chunk, extract, merge, and normalize stage-local validator bindings retain
|
||||
configured order and fields.
|
||||
8. Scriptorium source, concurrency, output, chunk-plan cache, checkpoint cache,
|
||||
and debug sections apply without coupling their roots.
|
||||
9. Keys that collide after trimming—pipeline IDs, lane IDs, and representative
|
||||
reference slots—are rejected. Do not enumerate the same normalization rule
|
||||
at every possible reference location.
|
||||
10. `LoadFileConfig` reports both missing-file and malformed-YAML context.
|
||||
|
||||
Where `v3_test.go` already proves a case completely, either leave it there or
|
||||
move it without duplicating it.
|
||||
|
||||
### Validation
|
||||
|
||||
```sh
|
||||
go test ./internal/core/config -run 'Test(FileConfig|Default|Version3)'
|
||||
```
|
||||
|
||||
### Exit criteria
|
||||
|
||||
The maintained version 3 schema, defaults, binding forms, state sections, and
|
||||
normalization rules can be refactored without silently changing their contract.
|
||||
|
||||
## Stage 2: Precedence and validation matrices
|
||||
|
||||
**Status:** Not started
|
||||
|
||||
### Objective
|
||||
|
||||
Protect configuration precedence and high-risk invalid combinations after all
|
||||
sources have been applied.
|
||||
|
||||
### Read first
|
||||
|
||||
- `docs/config.md`, especially **Discovery**, **Environment Overrides**,
|
||||
**Concurrency**, **Module Bindings**, and **State Surfaces**
|
||||
- `internal/core/config/env.go`
|
||||
- `internal/core/config/validation.go`
|
||||
- current tests from Stage 1
|
||||
|
||||
### Implement
|
||||
|
||||
Add `internal/core/config/env_contract_test.go` and
|
||||
`internal/core/config/validation_contract_test.go`.
|
||||
|
||||
Precedence tests must prove:
|
||||
|
||||
1. File values override built-in defaults.
|
||||
2. Every current `NOTARIUS_*` operational variable overrides the corresponding
|
||||
file value: total LLM concurrency, extract workers, output directory,
|
||||
chunk-plan mode and directory, checkpoint directory, and debug directory.
|
||||
3. Extract workers default to the final effective total when not explicitly
|
||||
configured, but an explicit file or environment worker value is retained.
|
||||
4. Empty cache-directory fields in a file remain valid and select deferred
|
||||
per-user defaults; empty directory environment overrides are errors.
|
||||
5. Invalid integers and chunk-cache modes report the responsible environment
|
||||
variable and do not panic.
|
||||
6. Removed provider environment variables are ignored. Do not place credential
|
||||
values in assertions or failure output.
|
||||
|
||||
Validation tables must cover one representative case for each rule family:
|
||||
|
||||
- non-positive total concurrency; extract worker below one, above total, and at
|
||||
both valid boundaries; unknown and blank worker keys;
|
||||
- mutually exclusive Scriptorium profile sources;
|
||||
- blank required output/debug roots, NUL in every physical root family, and an
|
||||
invalid chunk-plan mode;
|
||||
- empty and trim-duplicated pipeline/lane/reference identifiers;
|
||||
- negative retries and whitespace-only explicit LLM profiles;
|
||||
- references on unsupported input/output bindings;
|
||||
- invalid validator bindings: empty module, retries, references, nested
|
||||
validators, and validator chains on unsupported stages; and
|
||||
- deprecated non-empty lane-level validators rejected while stage-local omitted,
|
||||
empty, and non-empty overrides remain valid structurally.
|
||||
|
||||
Assert stable contextual fragments rather than complete error text.
|
||||
|
||||
### Validation
|
||||
|
||||
```sh
|
||||
go test ./internal/core/config -run 'Test(Env|Precedence|Validate)'
|
||||
```
|
||||
|
||||
### Exit criteria
|
||||
|
||||
Every documented precedence edge and every materially different validation
|
||||
rule family has a direct, readable regression test.
|
||||
|
||||
## Stage 3: Effective configuration, cloning, and redaction
|
||||
|
||||
**Status:** Not started
|
||||
|
||||
### Objective
|
||||
|
||||
Protect the internal contract that validated configuration resolves into an
|
||||
isolated, auditable pipeline without leaking sensitive options.
|
||||
|
||||
### Read first
|
||||
|
||||
- `docs/policy/architecture.md`, especially **Pipeline Composition And
|
||||
Ownership**, **Configuration And Provenance**, and **State, Output, And
|
||||
Safety**
|
||||
- `internal/core/config/effective_config.go`
|
||||
- `internal/core/config/redaction.go`
|
||||
- `internal/core/config/redaction_test.go`
|
||||
- `internal/framework/pipeline/profile.go`
|
||||
|
||||
### Implement
|
||||
|
||||
Add `internal/core/config/effective_config_contract_test.go` and extend
|
||||
`redaction_test.go` only for missing cases.
|
||||
|
||||
Use a minimal fake `pipeline.ModuleCatalog` with enough variants and
|
||||
capabilities to make each resolution outcome explicit. Cover:
|
||||
|
||||
1. Empty and unknown pipeline IDs fail; a map key with harmless surrounding
|
||||
whitespace resolves to its normalized ID.
|
||||
2. `Only` selects exactly the requested lanes, rejects unknown lanes, and does
|
||||
not mutate the source configuration.
|
||||
3. Default chunk, merge, normalize, and output modules are materialized through
|
||||
the catalog.
|
||||
4. Unknown modules, missing capabilities, missing artifact variants, invalid
|
||||
module options, and invalid validator options retain pipeline/lane/stage
|
||||
context.
|
||||
5. An LLM-profile override applies to LLM-capable pipeline module bindings
|
||||
before digest calculation, does not override validator-specific profiles,
|
||||
and changes the digest when effective behavior changes.
|
||||
6. Omitted, explicitly empty, and configured validator chains resolve
|
||||
distinctly and preserve configured order.
|
||||
7. Returned `EffectiveConfig`, resolved bindings, nested options, reference
|
||||
slices/content, and validator chains do not alias the input configuration or
|
||||
resolution inputs.
|
||||
8. Both redacted summary payloads cover every binding and recursively redact
|
||||
sensitive keys in untyped and typed/aliased option containers while
|
||||
preserving safe neighbors and excluding materialized reference content.
|
||||
|
||||
Do not duplicate detailed reference-selector behavior owned by Stage 6.
|
||||
|
||||
### Validation
|
||||
|
||||
```sh
|
||||
go test ./internal/core/config -run 'Test(Resolve|Effective|Redacted)'
|
||||
```
|
||||
|
||||
### Exit criteria
|
||||
|
||||
Resolution defaults, selection, compatibility failures, profile overrides,
|
||||
digests, deep-copy isolation, and redaction are directly protected.
|
||||
|
||||
## Stage 4: CLI command and configuration-loading contracts
|
||||
|
||||
**Status:** Not started
|
||||
|
||||
### Objective
|
||||
|
||||
Protect the public command surface, exit-code classification, configuration
|
||||
discovery, validation command, and pipeline listing.
|
||||
|
||||
### Read first
|
||||
|
||||
- `docs/cli.md`
|
||||
- `docs/config.md#discovery`
|
||||
- command dispatch, `runConfigValidate`, `runPipelinesList`, and configuration
|
||||
loading in `internal/cli/run.go`
|
||||
- `internal/cli/state_surfaces_test.go`
|
||||
|
||||
### Implement
|
||||
|
||||
Add `internal/cli/command_contract_test.go`. Use injected options and temporary
|
||||
files; no case may require production credentials or provider access.
|
||||
|
||||
Cover:
|
||||
|
||||
1. No arguments and each documented help spelling write usage to stdout and
|
||||
return `0`.
|
||||
2. Unknown commands/subcommands and malformed command syntax write to stderr
|
||||
and return `2`.
|
||||
3. Configuration discovery precedence is explicit `--config`, then non-empty
|
||||
`NOTARIUS_CONFIG`, then the compiled default path. Test the first two with
|
||||
temporary files. For the compiled path, inspect its host state first: assert
|
||||
selection when it is a regular file, or the documented not-found error when
|
||||
it is absent. Never create, replace, or remove the system path.
|
||||
4. Missing and malformed configuration files return `1` with actionable path
|
||||
or parse context.
|
||||
5. `config validate` succeeds for a valid file, resolves a requested pipeline
|
||||
against an injected catalog, rejects an unknown pipeline/lane, requires
|
||||
`--pipeline` with `--only`, and returns `2` for malformed `--only` syntax.
|
||||
6. `pipelines list` sorts normalized IDs in text and JSON forms; JSON is decoded
|
||||
and compared structurally rather than as raw formatting.
|
||||
7. Removed structural/state flags such as `--diagnostics-dir` are rejected with
|
||||
exit `2` instead of being silently ignored.
|
||||
8. Representative malformed run flags and valid-runtime failures establish the
|
||||
documented `2` versus `1` boundary.
|
||||
|
||||
### Validation
|
||||
|
||||
```sh
|
||||
go test ./internal/cli -run 'Test(Command|Help|ConfigDiscovery|ConfigValidate|PipelinesList|ExitCode)'
|
||||
```
|
||||
|
||||
### Exit criteria
|
||||
|
||||
The documented command grammar, streams, discovery order, listing behavior,
|
||||
and exit-code classes are protected without invoking a real pipeline.
|
||||
|
||||
## Stage 5: Run controls, profile selection, and process-facing results
|
||||
|
||||
**Status:** Not started
|
||||
|
||||
### Objective
|
||||
|
||||
Protect the public run controls and the CLI-to-pipeline composition boundary.
|
||||
|
||||
### Read first
|
||||
|
||||
- `docs/cli.md#run`
|
||||
- `internal/cli/run.go`
|
||||
- `internal/cli/scriptorium_profiles.go`
|
||||
- `internal/cli/state_hardening_test.go` and its fake harness
|
||||
|
||||
### Implement
|
||||
|
||||
Add `internal/cli/run_contract_test.go`. Reuse or minimally generalize the
|
||||
existing deterministic state-test harness. Keep helper implementations at the
|
||||
bottom of a test file or in one clearly named `test_helpers_test.go` file.
|
||||
|
||||
Cover:
|
||||
|
||||
1. Missing pipeline ID, missing `--input`, unknown flags, blank values, and
|
||||
multiple positional IDs return `2` without allocating output or debug state.
|
||||
2. Unknown pipeline/lane and unreadable input are valid invocations that return
|
||||
`1`; when debug is requested after allocation, its path is reported.
|
||||
3. A successful run writes the logical output files, reports normalized and
|
||||
rejected counts, and passes the same deterministic run ID and start time to
|
||||
the manifest and debug bundle.
|
||||
4. `--only` executes and reports only selected lanes.
|
||||
5. `--output-dir` and `--debug-dir` override environment, file, and default
|
||||
roots; `--debug-dir` without `--debug` returns `2` and creates nothing.
|
||||
6. `--llm-profile` reaches every effective LLM-capable pipeline module binding,
|
||||
leaves validator-specific profiles unchanged, selects the expected factory
|
||||
profile when exactly one is effective, and validates explicit Scriptorium
|
||||
profile IDs without making provider calls.
|
||||
7. `effectiveLLMProfileIDs` ignores deterministic stages and returns stable,
|
||||
deduplicated ordering across chunk, lane stages, and LLM-backed validators.
|
||||
8. `--session-id` rejects missing/blank values and passes a trimmed explicit
|
||||
value through prompt-facing requests. With no override, the parsed source
|
||||
document ID is used as the session identifier.
|
||||
9. LLM factory, preparation, pipeline execution, output persistence, and debug
|
||||
persistence failures return `1`, preserve the primary error, and do not print
|
||||
a success message.
|
||||
10. Successful warnings remain exit `0`, are counted on stderr, and appear in
|
||||
durable output and requested debug summary.
|
||||
|
||||
Do not repeat the output-collision and terminalization matrices already covered
|
||||
by `run_id_test.go` and `state_hardening_test.go`.
|
||||
|
||||
### Validation
|
||||
|
||||
```sh
|
||||
go test ./internal/cli -run 'TestRun'
|
||||
```
|
||||
|
||||
### Exit criteria
|
||||
|
||||
Every public non-reference run flag and every major CLI-to-runner handoff has a
|
||||
contract test, including representative failures.
|
||||
|
||||
## Stage 6: CLI reference selector and override contracts
|
||||
|
||||
**Status:** Not started
|
||||
|
||||
### Objective
|
||||
|
||||
Protect the high-risk selector grammar and target-resolution behavior without
|
||||
duplicating reference materialization internals.
|
||||
|
||||
### Read first
|
||||
|
||||
- reference sections of `docs/cli.md` and `docs/config.md`
|
||||
- reference parsing and resolution helpers in `internal/cli/run.go`
|
||||
- reference resolution/materialization tests under `internal/framework/pipeline`
|
||||
|
||||
### Implement
|
||||
|
||||
Add `internal/cli/reference_contract_test.go`. Build a compact catalog with
|
||||
chunk, extractor, merger, and normalizer specs declaring distinct, shared,
|
||||
optional, and required slots, including at least two artifact kinds where
|
||||
variant lookup matters.
|
||||
|
||||
Cover:
|
||||
|
||||
1. Parse and apply every documented selector form: flat, `chunk.slot`,
|
||||
`merge.slot`, `lane.slot`, and explicit lane extract/merge/normalize.
|
||||
2. Flat and lane selectors succeed only with one eligible selected target.
|
||||
Ambiguity errors identify viable explicit selectors.
|
||||
3. Selected lanes constrain reference discovery; selectors for unselected or
|
||||
unknown lanes fail before reference file materialization.
|
||||
4. Malformed bindings—missing selector, path, separator, or excess selector
|
||||
segments—and malformed unbind selectors return `2`.
|
||||
5. Repeated overrides use the final explicit binding for that exact target;
|
||||
target-specific overrides do not alter same-named slots on other targets.
|
||||
6. `--without-reference` removes optional configured bindings and fails when a
|
||||
required chunk, extract, merge, or normalize slot would remain unbound.
|
||||
7. CLI paths resolve from the working directory while config paths resolve from
|
||||
the config file directory. Assert recorded provenance, not private helper
|
||||
call order.
|
||||
8. Target lookup uses the selected artifact-kind variant and returns useful
|
||||
context when the requested merger/normalizer/extractor variant is absent.
|
||||
|
||||
Use small UTF-8 temporary reference files. Media-type and malformed-content
|
||||
details already proven by pipeline materialization tests need only one CLI
|
||||
smoke case.
|
||||
|
||||
### Validation
|
||||
|
||||
```sh
|
||||
go test ./internal/cli -run 'Test.*Reference'
|
||||
```
|
||||
|
||||
### Exit criteria
|
||||
|
||||
All public selector forms, ambiguity boundaries, lane selection, binding
|
||||
precedence, unbinding, and path-origin rules are protected.
|
||||
|
||||
## Stage 7: Cache, resume, output, and debug integration risks
|
||||
|
||||
**Status:** Not started
|
||||
|
||||
### Objective
|
||||
|
||||
Fill state-integration gaps without duplicating the focused ADR-0005 and
|
||||
ADR-0006 tests already present.
|
||||
|
||||
### Read first
|
||||
|
||||
- `docs/operations.md`
|
||||
- `docs/internal/state.md`
|
||||
- ADR-0005 and ADR-0006
|
||||
- `internal/cli/run_id_test.go`
|
||||
- `internal/cli/state_surfaces_test.go`
|
||||
- `internal/cli/state_hardening_test.go`
|
||||
- tests under `internal/framework/chunkplan` and
|
||||
`internal/framework/checkpoint`
|
||||
|
||||
### Implement
|
||||
|
||||
Add `internal/cli/cache_contract_test.go` only for uncovered CLI composition
|
||||
contracts. Extend an existing state test when it already owns the behavior.
|
||||
|
||||
Cover or confirm existing coverage for:
|
||||
|
||||
1. Chunk-plan mode precedence: CLI flag, environment, file, default.
|
||||
Invalid CLI syntax returns `2`; invalid environment/file modes return `1`.
|
||||
2. An explicit chunk-plan root is passed through exactly. An empty configured
|
||||
root selects `<UserCacheDir>/notarius/chunk-plans`. `bypass` never calls
|
||||
`UserCacheDir`, constructs a store, or creates a root.
|
||||
3. Root-resolution and store-construction failures preserve context and create
|
||||
no durable output.
|
||||
4. `auto` reuses a plan across independent invocations with the same source
|
||||
digest even when pipeline, chunk module settings, references, or selected
|
||||
lanes differ. `refresh` replaces only after valid chunking; `bypass` does no
|
||||
cache I/O. Reuse details below the CLI remain owned by chunk-plan package
|
||||
tests.
|
||||
5. `--resume` alone constructs checkpoint loader and recorder under the exact
|
||||
configured or per-user checkpoint root. Without it, neither root resolution
|
||||
nor checkpoint I/O occurs.
|
||||
6. Chunk-plan reuse and checkpoint resume remain independent across the
|
||||
relevant mode matrix. Reuse a compact table rather than duplicating every
|
||||
existing state-surface assertion.
|
||||
7. Existing tests continue to prove exclusive output allocation, atomic files,
|
||||
retained new partial output, shared run identity, debug opt-in, restrictive
|
||||
debug permissions, redacted summaries, and exactly-once terminal reports.
|
||||
Add a case only if one of these facts is not directly asserted.
|
||||
8. Config-oriented commands do not resolve or create output, cache, or debug
|
||||
state.
|
||||
|
||||
Do not introduce automatic cleanup tests: Notarius intentionally retains output
|
||||
and requested debug bundles.
|
||||
|
||||
### Validation
|
||||
|
||||
```sh
|
||||
go test ./internal/cli -run 'Test.*(Cache|ChunkPlan|Checkpoint|Resume|State|Output|Debug|RunID)'
|
||||
go test ./internal/framework/chunkplan ./internal/framework/checkpoint
|
||||
```
|
||||
|
||||
### Exit criteria
|
||||
|
||||
CLI state-root selection, opt-in I/O, cache mode precedence, cross-run reuse,
|
||||
and cache-family independence are covered at their composition boundary.
|
||||
|
||||
## Stage 8: Production composition and maintained examples
|
||||
|
||||
**Status:** Not started
|
||||
|
||||
### Objective
|
||||
|
||||
Protect the small number of production and end-to-end contracts most likely to
|
||||
break while unit tests continue to pass.
|
||||
|
||||
### Read first
|
||||
|
||||
- `README.md`
|
||||
- maintained files under `examples/`
|
||||
- `docs/config.md#implemented-production-modules`
|
||||
- `internal/cli/catalog.go`
|
||||
- `internal/modules/integration` tests and helpers
|
||||
|
||||
### Implement
|
||||
|
||||
Add `internal/cli/production_contract_test.go` and
|
||||
`internal/cli/example_contract_test.go`. Reuse production registries and prompt
|
||||
assets, but always inject a deterministic structured fake LLM client.
|
||||
|
||||
Production composition tests must prove:
|
||||
|
||||
1. The production catalog exposes every module, artifact codec, validator,
|
||||
default validator chain, and prompt asset named by current configuration and
|
||||
maintained examples.
|
||||
2. Catalog/registry conversion preserves typed artifact codec variants and
|
||||
validator-chain registrations.
|
||||
3. Production prompt assets allow the configured D&D scene chunker and spell
|
||||
extractor to prepare without reading provider credentials.
|
||||
4. `config validate --pipeline` accepts the maintained production module graph
|
||||
and rejects representative unknown modules, unknown validators, invalid
|
||||
artifact variants, and deterministic validators with LLM profiles.
|
||||
5. A small test configuration selecting `dnd/scenes` records that chunker and
|
||||
its expected warning/provenance fields during a fake-LLM run.
|
||||
|
||||
Example tests must prove:
|
||||
|
||||
1. Every maintained example configuration parses, validates, resolves its
|
||||
documented pipeline, and appears correctly in `pipelines list`.
|
||||
2. The README/minimal example invocation runs with a fake LLM and produces the
|
||||
documented logical JSON bundle. Decode manifest, index, warning, rejection,
|
||||
and artifact files and assert contract fields; do not snapshot whitespace or
|
||||
incidental timestamps.
|
||||
3. The documented `--only spells` invocation selects exactly the `spells` lane.
|
||||
4. One malformed input fixture returns `1`, produces no successful output
|
||||
bundle, and records a failure report only when debug is explicitly enabled.
|
||||
|
||||
If an example requires a provider profile, supply a test-only fake through
|
||||
`cli.Options`; never modify the example to embed credentials.
|
||||
|
||||
### Validation
|
||||
|
||||
```sh
|
||||
go test ./internal/cli -run 'Test(Production|Example|Maintained)'
|
||||
go test ./internal/modules/integration
|
||||
```
|
||||
|
||||
### Exit criteria
|
||||
|
||||
Production registration, prompts, maintained configuration, and the shortest
|
||||
end-to-end user workflow are protected by offline tests.
|
||||
|
||||
## Stage 9: Coverage audit and plan closure
|
||||
|
||||
**Status:** Not started
|
||||
|
||||
### Objective
|
||||
|
||||
Confirm that contract and high-risk coverage is complete without converting
|
||||
coverage percentage into a goal.
|
||||
|
||||
### Read first
|
||||
|
||||
- all tests added by Stages 1–8
|
||||
- current canonical CLI, configuration, operations, integration, and internal
|
||||
state documents
|
||||
- `docs/policy/documentation.md`
|
||||
|
||||
### Implement
|
||||
|
||||
1. Build a temporary checklist mapping every current CLI command/flag and every
|
||||
current configuration section/source to at least one owning test. Do not
|
||||
commit the checklist if the test names themselves make ownership clear.
|
||||
2. Run statement coverage for `internal/cli` and `internal/core/config`. Inspect
|
||||
uncovered functions and branches. Add a test only when the uncovered code is
|
||||
a documented contract, safety boundary, destructive/stateful path, complex
|
||||
selector/precedence branch, or failure path likely to regress.
|
||||
3. Remove redundant cases that prove no additional contract or risk. Keep a
|
||||
small end-to-end layer and more numerous narrow unit tests.
|
||||
4. Confirm tests do not access the network, depend on host credentials, use the
|
||||
real user cache directory, write outside temporary roots, or depend on test
|
||||
ordering.
|
||||
5. Run race-enabled tests for the two changed packages. Fix test races; report
|
||||
production races separately unless authorized to change production code.
|
||||
6. Update `docs/development.md` only if a stable testing procedure beyond its
|
||||
existing validation commands is now necessary. Do not document a coverage
|
||||
threshold.
|
||||
7. Once all checks pass, delete this completed implementation plan. The tests
|
||||
and canonical current-behavior documents remain authoritative.
|
||||
|
||||
### Validation
|
||||
|
||||
```sh
|
||||
go test -coverprofile=/tmp/notarius-cli.cover ./internal/cli
|
||||
go test -coverprofile=/tmp/notarius-config.cover ./internal/core/config
|
||||
go tool cover -func=/tmp/notarius-cli.cover
|
||||
go tool cover -func=/tmp/notarius-config.cover
|
||||
go test -race ./internal/cli ./internal/core/config
|
||||
go test ./...
|
||||
go vet ./...
|
||||
go build ./cmd/notarius
|
||||
git diff --check
|
||||
```
|
||||
|
||||
The `/tmp` coverage profiles are inspection artifacts and must not be committed.
|
||||
|
||||
### Exit criteria
|
||||
|
||||
Every current CLI and configuration contract has an identifiable owning test;
|
||||
high-risk state, safety, precedence, selector, and failure paths are covered;
|
||||
the suite remains deterministic and maintainable; and no numeric coverage target
|
||||
has been introduced.
|
||||
@@ -1,6 +1,7 @@
|
||||
package config
|
||||
|
||||
import (
|
||||
"reflect"
|
||||
"strings"
|
||||
|
||||
"gitea.maximumdirect.net/eric/notarius/internal/framework/pipeline"
|
||||
@@ -126,17 +127,47 @@ func redactOptions(values map[string]any) map[string]any {
|
||||
}
|
||||
|
||||
func redactOptionValue(value any) any {
|
||||
switch typed := value.(type) {
|
||||
case map[string]any:
|
||||
return redactOptions(typed)
|
||||
case []any:
|
||||
items := make([]any, len(typed))
|
||||
for i, item := range typed {
|
||||
items[i] = redactOptionValue(item)
|
||||
if value == nil {
|
||||
return nil
|
||||
}
|
||||
reflected := reflect.ValueOf(value)
|
||||
switch reflected.Kind() {
|
||||
case reflect.Map:
|
||||
if reflected.Type().Key().Kind() != reflect.String {
|
||||
return value
|
||||
}
|
||||
if reflected.IsNil() {
|
||||
return nil
|
||||
}
|
||||
out := make(map[string]any, reflected.Len())
|
||||
iterator := reflected.MapRange()
|
||||
for iterator.Next() {
|
||||
key := iterator.Key().String()
|
||||
if sensitiveConfigKey(key) {
|
||||
out[key] = "[REDACTED]"
|
||||
continue
|
||||
}
|
||||
out[key] = redactOptionValue(iterator.Value().Interface())
|
||||
}
|
||||
return out
|
||||
case reflect.Slice:
|
||||
if reflected.IsNil() {
|
||||
return nil
|
||||
}
|
||||
if reflected.Type().Elem().Kind() == reflect.Uint8 {
|
||||
out := reflect.MakeSlice(reflected.Type(), reflected.Len(), reflected.Len())
|
||||
reflect.Copy(out, reflected)
|
||||
return out.Interface()
|
||||
}
|
||||
fallthrough
|
||||
case reflect.Array:
|
||||
items := make([]any, reflected.Len())
|
||||
for i := 0; i < reflected.Len(); i++ {
|
||||
items[i] = redactOptionValue(reflected.Index(i).Interface())
|
||||
}
|
||||
return items
|
||||
default:
|
||||
return typed
|
||||
return value
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -117,6 +117,40 @@ func TestRedactedEffectiveConfigPayloadDoesNotAliasSource(t *testing.T) {
|
||||
assertRedactionTestBindingUnchanged(t, effective.ResolvedPipeline.Input, "effective")
|
||||
}
|
||||
|
||||
func TestRedactedResolvedPipelinePayloadHandlesTypedOptionContainers(t *testing.T) {
|
||||
type optionMap map[string]string
|
||||
type optionList []optionMap
|
||||
|
||||
typed := optionList{{
|
||||
"api_key": "typed-container-secret",
|
||||
"safe": "typed-container-safe",
|
||||
}}
|
||||
effective := EffectiveConfig{ResolvedPipeline: pipeline.ResolvedPipeline{
|
||||
Input: pipeline.ModuleBinding{Options: map[string]any{"nested": typed}},
|
||||
}}
|
||||
|
||||
payload := effective.RedactedResolvedPipelinePayload()
|
||||
nested, ok := payload.Input.Options["nested"].([]any)
|
||||
if !ok || len(nested) != 1 {
|
||||
t.Fatalf("redacted typed list = %#v", payload.Input.Options["nested"])
|
||||
}
|
||||
item, ok := nested[0].(map[string]any)
|
||||
if !ok {
|
||||
t.Fatalf("redacted typed map = %#v", nested[0])
|
||||
}
|
||||
if got := item["api_key"]; got != "[REDACTED]" {
|
||||
t.Fatalf("redacted api_key = %v", got)
|
||||
}
|
||||
if got := item["safe"]; got != "typed-container-safe" {
|
||||
t.Fatalf("safe option = %v", got)
|
||||
}
|
||||
|
||||
item["safe"] = "mutated"
|
||||
if got := typed[0]["safe"]; got != "typed-container-safe" {
|
||||
t.Fatalf("source typed map mutated through redacted payload: %q", got)
|
||||
}
|
||||
}
|
||||
|
||||
func redactionTestBinding(name string) pipeline.ModuleBinding {
|
||||
return pipeline.ModuleBinding{
|
||||
Module: "safe-" + name,
|
||||
|
||||
Reference in New Issue
Block a user