5.3 KiB
ConfigKit Inheritance Review Remediation
Review Scope And Overall Result
Reviewed on 2026-10-04, comparing implementation commits ec99d48 through
6450994 with the baseline 9e74df0, the ConfigKit feature roadmap, and its
eight-stage implementation plan.
The architecture matches the intended design. ConfigKit v0.5.0 owns source composition and native sibling inheritance before decoding. Notarius retains its typed defaults, presence-sensitive validation, declaration-relative path normalization, detached runtime configuration, catalog-dependent resolution, and private redacted inspection boundary. The production configuration adapter needed no changes; the work adds the dependency update, integration tests, documentation, and the derived development pipeline example.
No production regression or inadvertently strengthened requirement was found.
In particular, the implementation adds no inheritance whitelist, new schema
version, custom merging, path-containment rule, template exemption, or release
gate. The newly reserved extends key is the expressly adopted upstream
compatibility change.
Two follow-ups were identified: a path-test blind spot and an incomplete documented command. Completion is recorded under each finding. This document records remediation work only; it does not authorize unrelated runtime refactoring.
Finding 1 — Make The Path Fixture Distinguish Declaring Directories
Priority: P2. Category: verification gap.
Status: addressed on 2026-10-04. The fixture now uses distinct parent-, child-,
and overlay-local paths for the source-base assertions and retains a separate
../ reference. GOWORK=off go test ./internal/core/config passed. No
production resolver change was needed.
Evidence: internal/core/config/inheritance_test.go:338–436,
TestInheritedReferencePathsKeepDeclaringSource.
The parent, child, and overlay files live in root/parents, root/children,
and root/overlays. Their resource paths all use ../fixtures/<name>, and
the expected paths all point into root/fixtures. Resolving one of these
paths from any of the three directories yields the same result. Thus, choosing
the child's directory for an inherited step/nested reference, or choosing the
parent's directory for an overlay replacement, could still pass this test.
TestInheritedInspectionRetainsOriginsRedactionAndOwnership does distinguish
the parent directory for a simple inherited ./shared.txt reference. It does
not close the blind spot for the nested, ordered-step, direct-child replacement,
and overlay cases in the larger path fixture.
Required change:
- Revise the existing path fixture to use directory-distinguishing relative
paths for these cases, such as
./shared.txt,./step.txt, and a child-local replacement. Assert inherited paths against the parent's directory, direct replacements against the child's directory, and overlay-supplied paths against the overlay's directory. - Retain a separate legitimate
../example to protect the existing host-path behavior. Avoid making every path converge on one shared sibling directory. - Keep this as a focused real-file configuration test. Change production resolution only if the improved assertions expose an actual defect.
Acceptance: choosing a parent, child, or overlay directory incorrectly produces a different absolute path and fails the relevant assertion. Nested inheritance, inherited ordered-step references, child replacements, and overlay replacements still pass with the actual resolver.
Validation: GOWORK=off go test ./internal/core/config.
Finding 2 — Supply Input In The Development Pipeline Command
Priority: P3. Category: documentation accuracy.
Status: addressed on 2026-10-04. The example now supplies the pipeline ID,
configuration path, and transcript input path, and links to the CLI run
reference. The flags were checked against that reference, and
GOWORK=off go test ./internal/cli passed without a live model request.
Evidence: docs/config.md:43–49, in Maintained Examples.
The new example says to select the development pipeline with
notarius run dnd-session-development --config /path/to/dnd-complete.config.yml.
The run command requires --input. Executing the equivalent command against
the maintained configuration returns notarius: run requires --input with CLI
exit status 2, before pipeline execution.
Required change: include a transcript input placeholder in the invocation,
for example --input /path/to/transcript.json, and retain the link to the CLI
reference for the full invocation contract. No CLI behavior change or new test
framework is needed.
Acceptance: the documented command supplies both required run inputs—the pipeline ID and input path—and uses the implemented flag names. Verify against the CLI reference and existing parser coverage without a live model request.
Checks Completed During Review
GOWORK=off go test ./...passed.GOWORK=off go vet ./...passed.GOWORK=off go buildof./cmd/notariusto a temporary destination passed.GOWORK=off go mod tidy -diffproduced no changes.- The selected ConfigKit module is v0.5.0; PromptKit remains v0.11.2.
- The implementation diff passed whitespace checks.
- The incomplete development invocation was reproduced without provider calls.
These checks describe the review baseline. Subsequent remediation and its focused validation are recorded under each finding.