ConfigKit/docs/roadmap/remediation.md

Pre-release Maintainability Remediation

Status and scope

Status: in progress; completed stages and their validation are recorded below. Audit baseline: 9b147bfd96aec25e0125dcae66e136e56dc7a670.

This roadmap owns the agreed maintainability cleanup before v0.1.0. It replaces the completed implementation-defect remediation record, whose findings, fixes, and validation evidence remain available in Git history. It does not reopen those defects or replace the original implementation plan.

The audit found worthwhile consolidation and ownership improvements, rather than a need to redesign ConfigKit. The offline default suite and vet passed at the audit baseline. That evidence is not validation of future changes or a release candidate.

Read the development guide, architecture policy, testing policy, and documentation policy before implementation. The configuration contract owns current semantics; the component overview owns implementation inventory. Release preparation remains governed by the release policy and maintainer procedure.

Execution rules and invariants

Implement the numbered stages in order. Each stage is one reviewable unit of work: inspect its current owners and tests, make the change, validate it, update its status and evidence here, and commit the nonempty unit with a short plain English message. Do not create empty commits. Record any discovered behavioral defect separately with a reproduction rather than silently changing semantics under a refactoring stage.

Keep the root public package and the existing internal/document and internal/yaml packages. Do not introduce dependencies, public API changes, provider interfaces, configurable merge policies, or a generic visitor framework. Private file splits do not require new packages or ADRs. Parser-specific types must remain behind the existing YAML adapter boundary.

Preserve the implemented contracts, particularly:

• Strict composition remains distinct from intentional profile/CLI overlay.
• Input precedence, final-only typed decoding, retained defaults, canonical decoding explanation events, and recorded domain resolution remain unchanged.
• Source identity, containment, optional-import rules, limits, diagnostic codes, source coordinates, ordering, and early callback termination remain intact.
• Semantic paths retain explicit key/index distinctions and historical shapes.
• Returned values and inspection snapshots remain detached; secret values remain absent from history, rendering, and library-generated diagnostics.
• Resolution runs before secret lookup and validation; ordinary loading never bootstraps environment variables implicitly.

Use focused existing behavioral tests during each stage and the baseline checks from the development guide before committing production changes. Add tests only for meaningful gaps exposed by extraction, not to enforce file locations or private call choreography. Update current owning documentation alongside each move. Planned filenames below are private implementation destinations, not new consumer contracts.

Stage 1 — Remove unused overlay value snapshots

Status: complete.

Completion: removed unread overlay before/after trees and their construction and cloning, including the now-unused CLI position lookup. Field-read review confirmed that resolution snapshots and history applications have separate live consumers; those and displaced origins remain intact. The full offline test suite, vet, build, and whitespace checks passed with Go 1.26.7 on Linux/amd64.

Evidence: overlayRecord in overlay.go retains before and after trees; profile and CLI processing construct them and cloneOverlayRecords clones them. Production consumers use the record's change and displaced origins, while explanation uses historyApplication instead.

Work:

1. Reconfirm all field reads before removal. Remove only the unused overlay snapshot fields and their construction/cloning in overlay.go and override.go.
2. Preserve change, displaced, input applications, and their ownership rules. Do not remove resolution snapshots or retained decoded trees: those have separate consumers and budget semantics.
3. Remove locals and imports made unnecessary by the deletion. Do not replace the removed state with a new cache or history representation.

Acceptance: profile/CLI provenance, explanation replay, composition independence, and redaction tests pass unchanged in observable behavior. Source inspection confirms no unused overlay snapshot construction remains. An allocation benchmark is not required to justify removing provably unread clones.

Stage 2 — Centralize parser issue translation

Status: pending.

Evidence: parse.go translates adapter issues inline, while importParseDiagnostics in imports.go repeats the translation for imports and profiles.

Work:

1. Define one private parserDiagnostics helper in parse.go that maps adapter issues to detached public diagnostics, including related locations.
2. Route selected-source, import, and profile parsing through it. Remove the import-specific translator and keep upstream parser errors private.
3. Return canonically ordered diagnostics from the shared translation boundary. Preserve existing codes, messages, primary/related coordinates, and source IDs. Keep per-source parsing budgets and graph accounting at their current owners.

Acceptance: malformed selected/imported/profile input retains safe, correctly attributed diagnostics. Use existing parser and import diagnostic tests; add a small cross-entry regression only if a source path lacks equivalent coverage.

Stage 3 — Share bounded regular-file read mechanics

Status: pending.

Evidence: readSource, importTraversal.readImport, and readProfileFile repeat preflight stat, open, post-open stat, regular-file checks, size checks, and bounded reads in source.go, imports.go, and profile.go.

Work:

1. Introduce private helpers in source_file.go for regular-file opening and bounded reading. Use narrow function parameters for stat/open operations so callers can supply os.Stat/os.Open or root-bound methods. Do not introduce an exported filesystem interface or substitute unrestricted opening for os.Root access.
2. Keep preflight before open and post-open verification. Close every opened file on all paths. Preserve the existing treatment of read-only file close errors; root-directory close diagnostics are a separate responsibility.
3. Return private classified failures with enough distinction for callers to map errors safely. Optional imports may suppress only the currently allowed missing-target failures at stat/open, not later read failures. Do not expose filesystem errors downstream through wrapping.
4. Keep source metadata, canonicalization, containment, per-source/aggregate budgets, byte accounting, and caller-specific limit diagnostics outside the shared mechanics. Pass the computed byte limit into the reader and retain both the size precheck and limit-plus-one read enforcement.
5. Reuse bounded read mechanics for reader/byte sources only where it simplifies existing code without changing single-use or ownership behavior. Keep secrets bootstrap independent: its symlink, environment, and reporting policies differ.

Acceptance: source, import, profile, FIFO, containment, symlink, optional-missing, exact-budget, and reader-ownership tests pass. Retain the deadline-isolated FIFO regression. Cover private failure classification only where public tests cannot reliably induce an important filesystem failure. Run race tests for this unit.

Stage 4 — Establish shared document traversal ownership

Status: pending.

Evidence: indexDocument, walkDocumentPaths, and projectedSensitivity repeat semantic tree traversal in load_metadata.go, metadata.go, and secret.go. General document lookup currently lives in load.go.

Work:

1. Create root-private document_walk.go with a nil-safe preorder walker taking a starting Path and a callback receiving both path and node. Visit mappings in stored order and sequences in index order; construct explicit Key/Index segments. It visits value nodes, not synthetic mapping-key nodes.
2. Move indexDocument and documentAtPath to this owner. Implement indexing, metadata shape collection, and projected sensitivity collection using the walker where their traversal semantics match.
3. Document callback borrowing: callers must clone paths or nodes when retaining mutable state. Preserve existing defensive copies and avoid sharing mutable metadata across results.
4. Keep path-bearing helpers in the root package so internal/document does not depend on the public package. Pure node representation and cloning stay in internal/document.
5. Leave specialized traversals explicit where they need inherited origins, pruning, node identity deduplication, or different counting. In particular, input limits count mapping keys and retained-history limits count shared nodes once; do not implement either using the new walker without those semantics.

Acceptance: numeric map keys versus sequence indices, escaped paths, unknown paths, historical shape changes, deleted descendants, dynamic secret tags, and metadata detachment remain protected by existing tests. No public path behavior or predicate coverage changes. Do not add tests that assert walker call counts.

Stage 5 — Separate typed projection, cloning, and comparison

Status: pending.

Evidence: projection.go combines roughly 622 lines of projection, detached cloning, dynamic schema handling, cycle/limit enforcement, and structural comparison. Value equality deliberately ignores sensitivity, which validation must check separately.

Work:

1. Keep canonical typed projection in projection.go; move cloning into clone.go, shared reflection operation state and cycle helpers into value_operation.go, and value-tree comparison into document_compare.go. Preserve the descriptor-driven approach and per-operation schema cache.
2. Rename the private equality/comparison entry points to make configuration-value semantics explicit. Document that location and sensitivity metadata are excluded, mapping order is immaterial, and sequences are compared atomically for change reporting. Update all callers without changing their semantics.
3. Give validator mutation comparison a named private owner in validation.go. It must combine projected value changes and sensitivity changes, while retaining private resolved-secret-state auditing as an additional check. Do not make resolver value comparison start treating sensitivity as a value.
4. Preserve the distinction between cloning unresolved public secret references and publication cloning of already validated private state. Name/document the clone mode explicitly rather than leaving an unexplained boolean at sensitive call sites.
5. Replace custom cmpString calls with the standard library comparator and remove that helper. Do not perform unrelated test-style rewrites.

Acceptance: projection/cloning tests cover nil versus empty, custom text codecs, inline fields, dynamic types, cycles, and limits. Validator mutation regressions must still reject equal-text sensitivity changes and secret-state changes. Callback ownership and secret lookup tests must prove publication does not share borrowed values or reread the environment. Run race tests for this unit.

Stage 6 — Clarify source assembly and override ownership

Status: pending.

Evidence: imports.go mixes filesystem context, directives, graph traversal, and diagnostics; override.go combines value construction/validation and application. Shared composition cloning is located inside the override feature, while profile overlay repeats it.

Work:

1. Keep import traversal and route diagnostics in imports.go. Move filesystem context validation/canonicalization/containment to filesystem.go, and directive structures/parsing/fragment restrictions to directives.go. Give source-independent node counting an appropriate shared document owner, with its mapping-key accounting documented.
2. Keep override validation and preparation in override.go, move parser-neutral value building to override_value.go, and application/displacement mechanics to override_apply.go.
3. Move cloneComposedDocument next to composedDocument in compose.go; use it for both profile overlay and overrides. Ensure every retained field is copied according to its existing ownership contract.
4. Keep strict composition, overlay, and history replay separate. Share small mechanics when identical; do not introduce merge strategy flags or one general merge engine. Preserve the existing profile-versus-CLI recording behavior.
5. Simplify profile root cleanup using deferred failure-path cleanup plus an explicit checked success-path close. Preserve primary failure diagnostics and the existing successful-read root-close error diagnostic; avoid double-close effects. Import traversal cleanup may remain explicit where already clear.

Acceptance: imports/globs, directive restrictions, source ordering, profiles, overrides, version restrictions, limits, and composition independence tests pass. No new cross-package dependencies appear. Root handles close on every path and profile/CLI provenance remains identical.

Stage 7 — Extract explicit loader lifecycle phases

Status: pending.

Evidence: loadWithSecretLookup in load.go coordinates roughly 200 lines of source assembly, decoding, callbacks, history, secrets, and result publication, including repeated creation of a composition handoff.

Work:

1. Keep the exported entry point and a readable linear coordinator in load.go. Move option selection to options.go, input assembly to load_input.go, and typed lifecycle/result publication to load_result.go. Move version checking beside the validation machinery rather than keeping it among document helpers.
2. Use small private handoff structs for assembled input and necessary phase outputs, not a mutable all-purpose loader object or a long list of output parameters. Reuse one composition initialization helper for profile and CLI processing, including NoSource override handling and source byte/node totals.
3. Preserve the exact existing order of option validation, parsing/import checks, schema validation, version checking, profile/CLI application, defaults factory, typed decoding, resolution, retained-history limit enforcement, secret lookup, validation, metadata/classification, and result publication.
4. Keep diagnostic/source accumulation explicit. Warnings survive successful phases; errors stop dependent work and never return a usable partial Result.
5. Preserve both ownership barriers: detach resolver-borrowed values before secret resolution and validation, then detach equal-valued validator replacements before publication while preserving validated secret state. Do not remove either clone as apparently redundant work.
6. Group Result's frozen inspection state only if it clarifies this handoff; do not add an additional history copy or change caller ownership of Config.

Acceptance: lifecycle ordering/early-stop tests, defaults-only loading, all input combinations, callback ownership, secrets, warning retention, and complete provenance agreement pass. Keep orchestration visible enough to verify phase ordering by reading the coordinator. Run the complete race suite.

Stage 8 — Separate explanation replay, snapshots, and rendering

Status: pending.

Evidence: explain.go combines approximately 576 lines of public records, replay algorithms, snapshot redaction, retained-history accounting, and human-readable output.

Work:

1. Keep public explanation records and Explain coordination in explain.go. Move application/snapshot replay and history budgeting to history.go, safe snapshot projection to snapshot.go, and WriteExplanation with its formatting helpers to explain_render.go.
2. Make effective rendering and explanation snapshots use the existing shared redaction implementation; do not create separate sensitivity rules.
3. Retain the canonical decoding reconciliation event between input replay and domain resolution, overlapping resolution intent coalescing, added/deleted paths, effective source attribution, and historical secret classification.
4. Document the differences between metadata origin records and value replay: they serve different query contracts but must agree on effective results. Do not replace them with a new event-store abstraction in this cleanup.
5. Preserve writer failure behavior and ownership of retained trees. Do not cache explanations or optimize full-tree replay without measured need.

Acceptance: explanation, effective rendering, history budget, provenance, secret redaction, and writer-failure tests pass without output changes. Container and leaf histories still reconcile with the frozen effective typed configuration.

Stage 9 — Complete ownership documentation and integration validation

Status: pending.

Work:

1. Review the final code for obsolete helpers, duplicate traversal/translation, unread retained state, stale comments, and imports left by earlier stages. Confirm file splits reflect discrete responsibilities rather than line quotas.
2. Update the component overview to describe the actual owners and fix its stale claim that metadata queries are empty. Keep detailed consumer semantics in their existing contract; repair navigation and moved-file links throughout documentation, including this roadmap's evidence links if necessary.
3. State the retained dependency direction clearly: public lifecycle coordination uses private source/document/schema/inspection helpers; the owned document representation has no dependency on the root package; only the private YAML adapter imports the upstream YAML package.
4. Run the full baseline and race suites, local documentation and dependency boundary checks, and the complete source-candidate checker described in the release procedure with available qualifying toolchains. Record exact versions, host platform, results, and any unavailable platform evidence. Cross-builds are not native execution evidence.
5. Review the accumulated diff for public API/configuration/diagnostic changes. Expected outcome: no consumer migration or dependency change. If any contract change is discovered, resolve it explicitly before closing this roadmap.
6. Record completion evidence and commit this final nonempty unit. Release preparation must subsequently retire completed roadmap material and validate the exact committed candidate under release policy. Completing this roadmap does not create a tag, push refs, or authorize publication.

Acceptance: all stages are complete with recorded validation, existing behavioral regressions remain protected, documentation matches the new ownership layout, and the worktree is clean after commits. Required release gates remain visible in the release procedure rather than being declared satisfied by this refactor.

Open Questions

None required to begin implementation. The package boundaries, compatibility constraints, ordered units, and acceptance criteria above are decided. Routine private naming adjustments are acceptable when they preserve those decisions; new behavioral discoveries require explicit evidence and disposition.