narratio/docs/roadmap/remediation.md

ConfigKit Migration Review — Required Remediations

Reviewed 2026-09-07 at f7a40af, against the changes after 1558c9c, the feature roadmap, and the completed implementation plan.

The main migration is implemented. The following findings are reproducible gaps in the intended behavior, not proposals to restore retired compatibility or filesystem restrictions. P2 denotes a correctness issue to fix before release; P3 denotes an operator-facing completeness issue also required by the accepted plan. No application code was changed during this review.

1. Derive explicit publish destinations after family expansion

Status: Resolved — P2

Resolution: Missing destinations are now derived through one shared domain helper both during ConfigKit resolution and again after concrete family expansion. Explicit destinations remain unchanged, while unknown sources still fail read-only publish validation. Regression coverage verifies resolved runtime and effective inspection output for generated members, explicit overrides, and unknown sources.

Evidence: internal/config/pipeline_configkit.go:208 derives missing publish destinations during the ConfigKit resolver. A concrete family member does not exist yet, so the resolver ignores the failed derivation. After family expansion, internal/config/artifact_families.go:141 calls the now-read-only publish validator, which rejects the still-empty destination. The previous implementation could derive it during that later validation.

Reproduction: In an otherwise valid pipeline, define a family meta over party.characters, with output pattern artifacts/{character_id}.md. Select a valid campaign whose party contains alice, and add:

publish:
  outputs:
    - source: narratio.artifact.meta_alice

config validate --config <pipeline> --campaign-file <campaign> fails with pipeline.publish.outputs[0].dest is required when destination cannot be derived from "narratio.artifact.meta_alice". Adding dest: artifacts/alice.md makes the same configuration pass. The destination is therefore available and derivable; the lifecycle omits the required final normalization step.

Required change: Complete publish destination resolution after concrete artifacts exist, before publish validation and final projection/digest generation. Share the domain derivation logic and keep validation read-only. Preserve explicit destinations and generated family publish-rule collision checks. Do not require redundant dest fields or revive mutation inside validators.

Acceptance: Test an explicit publish source naming a generated member with omitted destination, an explicit destination override, and a genuinely unknown source. Validate both effective inspection and resolved runtime configuration.

2. Stop re-defaulting an explicitly empty Notarius working directory

Status: Resolved — P2

Resolution: Removed both legacy blank-value fallbacks after ConfigKit resolution. An omitted working directory is still derived by the presence-aware resolver, an explicit relative directory is still rooted at the pipeline, and explicit empty or whitespace-only values remain empty and fail enabled-Notarius validation. Existing relocation-invariant digest coverage remains green.

Evidence: ConfigKit's resolver correctly uses presence-aware defaulting at internal/config/pipeline_configkit.go:180. However, internal/config/load.go:454 subsequently replaces any blank working directory with the config file's directory, irrespective of input presence. internal/config/effective_digest.go also applies a blank-value fallback while capturing logical paths. These retained legacy fallbacks bypass the new rule that defaults apply only to absent fields.

Reproduction: Set notarius.enabled: true, a valid config path, pipeline ID and output map, and notarius.working_directory: "". With config_path: notarius/config.yml, config show succeeds and reports working_directory: notarius; validation never sees the explicit invalid blank.

Required change: Let the ConfigKit resolver own the absent-field default. The subsequent runtime and logical-path conversions must preserve explicit emptiness so enabled-Notarius validation can reject it. Remove duplicate value-based defaulting rather than adding another private presence mechanism.

Acceptance: Omission still derives the expected directory; explicit empty and whitespace-only values fail when Notarius is enabled; a configured relative directory resolves from the pipeline root. Preserve digest relocation invariance.

3. Enforce unique base definitions even when a profile removes them

Status: Resolved — P2

Resolution: Named-profile loading now validates the independently resolved base through the same ConfigKit definition and public metadata history before accepting the selected result. Duplicate ordinary or family definitions therefore fail even when an overlay nulls their container or all of Scriptorium. A single base owner with a legitimate profile override remains supported.

Evidence: rejectDuplicateBaseDefinitions at internal/config/pipeline_configkit.go:354 enumerates only artifact/family keys remaining in the final typed configuration. It inspects history for those keys but never examines base declarations removed by an overlay.

Reproduction: Import first.yml defining scriptorium.artifacts.recap.prompt_id and second.yml defining scriptorium.artifacts.recap.output_path. Base validation correctly rejects the two-file declaration. Add profiles/clear.yml containing:

scriptorium:
  artifacts: null

The same root with --profile clear passes. A selected profile can thus hide the base ownership conflict, contrary to the plan's unique base-definition rule. This concerns disjoint contributions to one artifact, not ConfigKit's generic conflicting-leaf checks, which still work.

Required change: Discover the complete base declaration set through public ConfigKit metadata/history, including declarations displaced by an overlay, and validate ownership independently of final effective-key survival. Keep legitimate profile overrides and profile-only definitions supported. Do not reparse source YAML or add a second composer.

Acceptance: Cover duplicate ordinary and family definitions hidden by a null artifact/family container or null Scriptorium section. Base and selected-profile loads must reject the same base conflict, while a single base declaration with a valid overlay remains accepted.

4. Preserve null versus empty collections in effective YAML

Status: Resolved — P2

Resolution: Effective YAML is now emitted from the credential-safe JSON value projection through an output-only YAML-node conversion. Explicit nulls, empty maps, empty slices, booleans, strings, and exact JSON numbers retain their semantic types. Public show/diff tests cover null and empty maps and slices, while digest, credential-redaction, deterministic output, and exact-integer coverage remain intact.

Evidence: internal/config/effective_pipeline.go:154 marshals the typed projection directly with yaml.Marshal. The encoder emits nil slices as [] and nil maps as {}, while the JSON projection used by diff and digest preserves them as null. The shared input type does not guarantee shared output semantics.

Reproduction: Set audita.modules: null in a valid base and audita.modules: [] in profiles/empty.yml. Both config show invocations emit modules: [], but config diff default empty reports:

changed audita.modules null []

The commands disagree about the effective value. The same renderer also supplies the resolved pipeline diagnostic written by prepare.

Required change: Render YAML from a credential-safe projection that explicitly preserves nulls and empty collections, consistently with the semantic projection. Keep exact integers and safe credential reference names. An output-only conversion is appropriate; do not restore YAML decoding or a configuration parser. This requires truthful inspection, not a guarantee that inspection output is reloadable.

Acceptance: Verify null versus empty maps and slices through the public show boundary, and agreement with diff/digest distinctions. Retain credential-leakage and exact-integer regression coverage.

5. Complete provenance for generated artifacts and inferred dependencies

Status: Open — P3

Two new reporting paths lose available declaration information:

• Profile contributions to family members: internal/config/artifact_catalog.go:52 fills a generated member's definitions from the single member.Source. That source is chosen by the first matching non-default family field in artifactFamilySource, not the full set of effective contributors. Define family meta in the base and override its prompt_id in profiles/testing.yml. config artifacts --profile testing reports only the base file for meta_alice, even though config sources correctly attributes that member's prompt to the testing overlay.
• Inferred ordinary dependencies: With second.inputs.prior.source set to narratio.artifact.first and no explicit dependency list, config sources reports scriptorium.artifacts.second.depends_on as default / default. finalizeArtifactDependencies stores the input-edge origin, but the effective ownership projection retains the pre-inference default attribution from internal/config/effective_digest.go:55 and does not consult that origin.

Impact: Operators inspecting a testing profile or following a dependency can be directed to the wrong file or told a user-derived value is a central default. Runtime ordering and generated artifact values were correct in these reproductions.

Required change: Use ConfigKit's contributing locations for family definition sets, as for ordinary artifacts, and add narrow generated attribution for inferred dependency lists using their retained edge origins. Preserve family/character/party identity and deterministic ordering. Avoid introducing a second general provenance store or choosing a file based on the first alphabetically visited field.

Acceptance: A profiled family catalog exposes both relevant files; an inferred edge points back to its input declaration; mixed explicit/inferred dependencies retain their relevant contributors. Verify catalog and sources agree after expansion.

6. Surface successful secrets bootstrap reports, including skipped empty variables

Status: Open — P3

Evidence: internal/app/secrets_bootstrap.go:36 discards the report whenever LoadSecretsDir returns no error. Reports are only rendered when directory bootstrap itself fails. This does not implement the roadmap's safe bootstrap reporting boundary or Stage 6's warning presentation requirement.

Reproduction: Place a synthetic key in secrets/NARRATIO_REVIEW_KEY, reference it from audita.llm_api_key, and invoke config validation with NARRATIO_REVIEW_KEY= in the environment. ConfigKit correctly skips the file because the variable already exists, then typed loading fails with secret.empty. Narratio shows only that load failure, omitting the bootstrap report's skipped_existing and ExistingEmpty information. A successful load likewise discards all bootstrap results.

Required change: Retain and render or summarize the safe report once per invocation through a diagnostic output channel, including when a later typed load fails. Make skipped-existing-empty outcomes explicit. Preserve process environment precedence and the existing partial report on failure; keep values out of all diagnostics and keep structured/YAML stdout uncontaminated. Do not add bootstrap rollback, retries, or repeated directory loading.

Acceptance: Command-level tests demonstrate safe success/report output and a useful skipped-empty diagnostic, without leaking a synthetic secret or polluting show output. A two-profile diff still bootstraps and reports only once.

Verification and scope

The following checks passed at the reviewed commit:

• go test ./...
• go test -race ./...
• go vet ./...
• go build ./...
• The release asset builder for v2.0.0, producing exactly Linux amd64, Linux arm64, and macOS arm64 assets; the Linux amd64 binary reported the expected version.

The existing full suites include documentation and maintained-example checks. The findings above were additionally reproduced through a freshly built CLI with isolated temporary YAML fixtures and synthetic credentials. No live downstream tools or cloud services were invoked. Linux arm64 and macOS arm64 were cross-built, not tested natively. No tag was created or pushed.

The migration removes a net 1,235 production Go lines and 843 test Go lines versus 1558c9c (physical diff lines, including comments and whitespace). ConfigKit owns generic loading/composition; Narratio retains domain resolution, orchestration, safe inspection, and adapter delivery. These findings require completing that design, not reversing the simplification.