Files
ConfigKit/docs/roadmap/implementation.md

19 KiB
Raw Blame History

Import Discovery And Facade Simplification Implementation Plan

Status: in progress. Baseline: bb5540b62e382106d59678e2edb7655acfd5c960. Audience: a Luna coding agent implementing the stages sequentially.

Scope And Execution

Implement C03 and F07 from the internal-boundary roadmap. That document owns the target state and preservation requirements; this plan owns concrete private choices, steps, validation, and progress. The preceding 29-finding remediation sprint is complete. Its implementation record remains at bb5540b:docs/roadmap/implementation.md; this plan does not reopen it.

Read AGENTS.md, agent policy, the development guide, and applicable architecture, testing, and documentation policies. Use graph-first discovery and verify actual callers before deleting helpers. Preserve unrelated work.

This is one end-to-end sprint. Intermediate stages may fail to compile, especially the coordinated engine/facade cutover in Stages 2–3. Do not create adapters, aliases, duplicate paths, or extra abstractions solely to make intermediate stages green. Run focused checks once their affected surface is buildable; record pending checks and finish them before final acceptance. Commit focused nonempty units under repository policy and record stage status, actual validation, and known commit identifiers here. Completion requires the integrated result, not just checked-off stages.

Boundaries

  • Preserve public root-owned declarations and signatures, YAML syntax, loading semantics, ordinary/layered differences, callback order, diagnostics, source identity, inspection, confidentiality, and ownership.
  • Preserve glob literal-versus-discovered rules and directory/glob differences. Keep completed-link reuse, directory failure phases, containment, and existing accounting. This is not a discovery algorithm or permission-policy change.
  • Keep resolvedImportPolicy, registration compilation, normalized request responsibilities, and conversions with real ownership or domain meaning. Simplification is limited to defaults transport and placement construction.
  • Add no dependencies, caller controls, caches, limits, background work, release gates, or performance promises. Do not publish a release, update consumers, or revise accepted ADR decisions.

Selected Ownership Model

Use the existing engine ImportDefaults name and its exported Required, Follow, Hidden, Recursive, and Pattern fields as the sole engine input-defaults type. Root ImportDefaults stays separately defined. Root conversion maps the fields without cloning them; the public loading wrappers immediately enter engine preflight. loadSettings takes the per-load snapshot before acquisition or callbacks, as it already does. The normalized request and acquisition policy then borrow that load-owned read-only value.

Declaration resolution reads the owned defaults and retains its existing copies of selected pattern/exclusion slices and destinations in resolvedImportPolicy. This separates presence-aware load input from independently owned effective declaration policy. No public copy-count or thread-safety guarantee follows.

For placement, expose the engine's existing enum as PlacementMode uint8 with PlacementModeMapping and PlacementModeSequenceItem, and expose placement fields on engine input/options records for direct construction. These remain internal-package implementation surfaces. Root types stay root-owned; direct casts preserve unknown values for existing preflight validation.

Validation Conventions

Run from the repository root with an installed compatible Go toolchain and dependencies already available. Use GOTOOLCHAIN=local, GOPROXY=off, and GOTELEMETRY=off. Keep disposable artifacts outside the repository and do not change dependencies for tooling. Focused expressions identify behavior owners; include a newly named regression explicitly if needed. Reuse unchanged successful checks. No timing, allocation, exact syscall, or helper-shape assertions are acceptance criteria.

Stage 1 — Extract Patterned Glob Entry Preparation

Finding: C03. Owners: internal/engine/glob.go, existing glob selection, link, and directory failure tests. Status: complete. Validation passed: env GOTOOLCHAIN=local GOPROXY=off GOTELEMETRY=off go test ./internal/engine -run 'Glob|Directory|Link|Filesystem|Resource' and git diff --check.

  1. Add prepareGlobEntry beside the selector. Take the filesystem access, base path, parent selectedImportCandidate, fs.DirEntry, pattern segment, last flag, and resolvedImportPolicy. Return a candidate, a boolean meaning selected versus ordinary skip, and a nullable *Diagnostic. Use the parent's existing site, which is propagated from the selector's root candidate. No candidate-set, accounting, or traversal-state mutation belongs here.
  2. Move only the patterned branch's per-entry body into this helper. Preserve its order: path.Match; match/hidden filtering; lexical/access path creation and containment; importEntryTarget and existing diagnostic translation; follow filtering; intermediate-directory eligibility; host/native physical path selection and canonicalization fallback; candidate label/site creation. Return existing safe codes, paths, and sites. Do not turn inspection errors into skips or skip validation merely because exclusion will later match.
  3. Replace that inline body with the helper call and candidate insertion. Leave pattern validation, explicitGlobCandidate, directory enumeration, per-segment union/sorting, final-file validation, exclusions, requiredness, and source-admission timing in the selector.
  4. Keep directory entry preparation separate. Reuse existing filesystem/link primitives; do not introduce a generic entry strategy or unify differing missing-target, literal-link, filtering, or recursion rules. Remove imports only when genuinely unused after extraction.

Acceptance: the selector now expresses traversal without embedding preparation details. Reuse glob_selection_test.go, glob_test.go, filesystem-link tests, and the preceding sprint's directory-phase regressions. They must still cover literal hidden/link components versus wildcard filtering, aliases and native spelling, escaping/cyclic links, matched-directory errors before exclusion, selection order, and bounded accounting. Add a behavioral case only for a gap identified during extraction, not a unit test asserting the new helper exists.

Focused check: go test ./internal/engine -run 'Glob|Directory|Link|Filesystem|Resource'.

Stage 2 — Consolidate The Engine Defaults Representation

Finding: F07, engine representation. Owners: internal/engine/import_spec.go, load.go, layered.go, load_request.go, acquisition.go, options.go, and their callers/fixtures. Status: complete. Validation passed: env GOTOOLCHAIN=local GOPROXY=off GOTELEMETRY=off go test ./internal/engine -run 'Import|Glob|Directory|SourceVersion' and git diff --check. Root facade callers remain pending Stage 3.

  1. Retain engine ImportDefaults with its existing five exported fields. Remove private importDefaults, without a compatibility alias. Update engine option/request/acquisition fields, helper signatures, readers, and fixture literals to use the canonical type and exported default-field names. Private containing fields such as loadRequest.importDefaults can keep their names. Do not rename the distinct fields of importDeclarationSpec or resolvedImportPolicy just because they have similar meanings.
  2. Expose ImportDefaults ImportDefaults on engine Options[T] and LayeredOptions[T]. Remove OptionsWithImportDefaults, LayeredOptionsWithImportDefaults, and privateImportDefaults. Stage 3 changes root callers; a temporary compile failure is preferable to an interim bridge.
  3. Adapt snapshotImportDefaults to the canonical type, retaining nil boolean pointers and nil versus non-nil empty pattern. Keep cloneBool and cloneStrings where they still have real consumers. Keep current snapshot call sites until Stage 3 establishes their final ownership placement.
  4. Keep effective declaration policy resolution and validation semantics intact: explicit false beats defaults, each control resolves independently, and selector-specific applicability and depth checks stay unchanged. Do not combine defaults with resolved policy or registration compilation.

Acceptance: one engine input-defaults type reaches every production path and fixture; no equivalent private record or alias remains. Inspect ordinary, layered, transitive-import, directory/glob, and test-helper call sites. The root cutover and removal of redundant snapshots are explicitly pending until Stage 3.

Run the import/default focused tests after Stage 3 if the root cutover prevents building the package here; do not report that deferred check as passed.

Stage 3 — Simplify Facade Defaults Construction And Snapshot Ownership

Finding: F07, defaults handoff. Owners: root conversion.go, lifecycle.go, layered.go; engine defaults/preflight/coordinator owners; facade_import_defaults_test.go and internal/engine/import_spec_test.go. Status: complete. Validation passed: env GOTOOLCHAIN=local GOPROXY=off GOTELEMETRY=off go test ./internal/engine -run 'Import|Glob|Directory|SourceVersion', env GOTOOLCHAIN=local GOPROXY=off GOTELEMETRY=off go test . -run 'ImportDefaults|FacadeImport|SourceVersion', env GOTOOLCHAIN=local GOPROXY=off GOTELEMETRY=off go test ./..., env GOTOOLCHAIN=local GOPROXY=off GOTELEMETRY=off go test -race ./..., and git diff --check.

  1. Set engine ImportDefaults fields directly in root toEngineOptions and toEngineLayeredOptions. Retain toEngineImportDefaults as the explicit root-to-engine type mapping, but map its booleans/pattern without copies. Remove root cloneImportDefaultBool and cloneImportDefaultStrings after confirming no other caller. Keep public root types and declaration contracts.
  2. Keep snapshotImportDefaults in loadSettings as the load-entry ownership operation for both APIs. Remove repeated snapshots in engine LoadLayered request construction, executeLoadRequest acquisition-policy construction, and resolveImportPolicy. Carry the owned defaults read-only thereafter. Review all callers of those helpers, including test setup; direct declaration resolution can read its input because it returns its own concrete policy.
  3. Keep per-policy copies of the selected pattern, exclusions, and destination. Do not borrow caller-owned storage beyond preflight, mutate the shared owned defaults, or remove copying from unrelated option/record paths. Each new load snapshots its own current inputs; add no reuse cache or ownership flag.
  4. Replace the private conversion-snapshot fragment in TestImportDefaultsPreserveEmptyPatternAndRequiredness, which converts options, mutates inputs, and calls engine Load. It should no longer demand a snapshot from a private conversion helper. Keep that test's public nil/empty, requiredness, and inspection-evidence checks.
  5. Protect the public snapshot contract with a compact ordinary/layered table: use a main document with a version and imports, enable SourceVersionModeIfPresent, and use a configuration type with version and imported-value fields. In the main version-check callback, mutate the caller's recursive default from true to false and its existing pattern element from *.cfg to *.skip. This callback runs after preflight but before imported discovery. Verify the original recursive/pattern selection still supplies a value from a nested imported file. Use existing fixture helpers and no concurrent mutation. A subsequent load with those changed defaults must omit that imported contribution rather than reuse the prior snapshot. Keep fixtures reusable or create fresh readers for each load.
  6. Reuse or extend the small snapshot/policy tests for all four optional booleans, explicit false, pattern mutation, nil, and non-nil empty pattern. Preserve existing invalid-pattern-before-read/hook tests and declaration override, transitive, underlay, and selected-overlay coverage. Do not assert exact copy counts, pointer identities, or the private conversion boundary as public policy.

Acceptance: the engine/facade cutover builds; public defaults remain detached before acquisition/callbacks, reused loads remain independent, and only genuine ownership copies remain. Confirm the absence of full defaults snapshots at the three removed downstream sites by source review, not production instrumentation.

Focused checks: go test ./internal/engine -run 'Import|Glob|Directory|SourceVersion' and go test . -run 'ImportDefaults|FacadeImport|SourceVersion'.

Stage 4 — Construct Placement Directly Across The Facade

Finding: F07, placement handoff. Owners: engine destination.go, load.go, layered.go, normalized input and placement consumers; root lifecycle.go, layered.go, and placement fixtures. Status: complete. Validation passed: env GOTOOLCHAIN=local GOPROXY=off GOTELEMETRY=off go test ./internal/engine -run 'Destination|Placement|Item|SourceVersion', env GOTOOLCHAIN=local GOPROXY=off GOTELEMETRY=off go test . -run 'Placement|Destination|SourceInfo|SourceVersion|PublicFacadeBoundary|DependencyImportBoundary', env GOTOOLCHAIN=local GOPROXY=off GOTELEMETRY=off go test ./..., and git diff --check.

  1. Rename the existing engine enum and its constants to PlacementMode, PlacementModeMapping, and PlacementModeSequenceItem, preserving underlying uint8 values and zero semantics. Use the same type throughout engine placement parsing, normalization, acquisition, source evidence, and version projection. Do not add a second raw-mode record or type alias.
  2. Expose engine Options[T].Destination and .PlacementMode, and Input.PlacementMode. Normalized loadInput and other private owner fields can retain their names. Populate the normalized input from those fields. Remove OptionsWithPlacement and InputWithPlacement and migrate all callers, including the direct engine construction in root records_test.go.
  3. In root toEngineOptions and toEngineInput, set destination using the existing toEnginePath conversion and placement using engine.PlacementMode(options.PlacementMode) or its input equivalent. Preserve invalid enum values for normalizeInput to diagnose before reads. Do not add validation/defaulting to the facade or a new setter/cast function.
  4. Preserve normalization of literal/parsed destination keys, root-item rejection, wrapper accounting, mounted/item source coordinates, and version projection. Root Path is immutable and toEnginePath carries its private engine value; keep normalization and actual mutable ownership copies. Do not redesign path conversion or add redundant clones solely to replace the deleted decorator.
  5. Leave source-result transport such as SourceInfoPlacementModeForFacade, source-version conversion, sequence registration preparation, and opaque records alone. They are not input setters targeted by this step. Adapt tests mechanically to new engine construction without weakening public assertions.

Acceptance: both public loading APIs retain zero/mapping and sequence-item placement, errors before input consumption, final values, source metadata, and history. Reuse the public placement tests plus destination/accounting, item-import, mounted-version, and invalid-option cases. Root signatures still contain no engine types, and engine exports remain below the internal boundary.

Focused checks: go test ./internal/engine -run 'Destination|Placement|Item|SourceVersion' and go test . -run 'Placement|Destination|SourceInfo|SourceVersion|PublicFacadeBoundary|DependencyImportBoundary'.

Stage 5 — Integrate, Document, And Verify The Result

Findings: final acceptance for C03, F07. Status: not started.

  1. Review both paths together: defaults resolving wildcard discovery in ordinary, layered, and transitive imports; placement carrying the resulting sources through composition and inspection; and failures stopping before later hooks. Existing integration tests own these interactions. Add only a concrete missing case rather than a second copy of the complete matrix or a new fixture framework.
  2. Inspect the resulting handoffs. Confirm the glob selector no longer embeds per-entry preparation, the engine has one defaults input type, direct fields replaced the four input decorators, and the load snapshot is not repeated at each private representation. Keep domain-specific policy/registration and root conversion boundaries, necessary ownership copies, and existing budgets.
  3. Update docs/internal/imports.md for the selector/preparation split and docs/internal/overview.md for defaults conversion, preflight snapshot ownership, and direct placement construction. Keep algorithms private and avoid public performance or copy-count guarantees. No ADR/policy change is needed.
  4. Replace C03/F07 detail in the first two docs/roadmap/future.md entries with short pointers to internal-boundaries.md. Retain their heading anchors for existing audit links; leave other future candidates unchanged. Correct old roadmap references that would otherwise mistake this replacement plan for the previous sprint's completed record: use bb5540b:docs/roadmap/implementation.md for that historical plan. Do not reopen the prior audit or retire unrelated planning records in this sprint.
  5. Format changed Go files, finish deferred checks, run the final checks below, and inspect the complete diff for API, dependency, semantics, or scope changes. Record actual results and platform limitations here. Commit focused nonempty units and hand off a clean workspace; do not create empty validation commits.

Final validation, following the existing development guide:

env GOTOOLCHAIN=local GOPROXY=off GOTELEMETRY=off ./scripts/check.sh
env GOTOOLCHAIN=local GOPROXY=off GOTELEMETRY=off go test -race ./...
git diff --check

The everyday check includes consumers, documentation links, and public/dependency boundary tests. Confirm those existing tests ran rather than adding redundant final invocations. Report filesystem/platform skips accurately; express macOS validation, CI completion, release publication, extended fuzz campaigns, and benchmarks are not additional acceptance requirements.

No product or architectural question remains open. The concrete choices above complete the plan without changing the roadmap's public contracts; private formatting and file organization can follow local idiom.