18 KiB
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.
- Add
prepareGlobEntrybeside the selector. Take the filesystem access, base path, parentselectedImportCandidate,fs.DirEntry, pattern segment,lastflag, andresolvedImportPolicy. Return a candidate, a boolean meaning selected versus ordinary skip, and a nullable*Diagnostic. Use the parent's existingsite, which is propagated from the selector's root candidate. No candidate-set, accounting, or traversal-state mutation belongs here. - 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;importEntryTargetand 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. - 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. - 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.
- Retain engine
ImportDefaultswith its existing five exported fields. Remove privateimportDefaults, 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 asloadRequest.importDefaultscan keep their names. Do not rename the distinct fields ofimportDeclarationSpecorresolvedImportPolicyjust because they have similar meanings. - Expose
ImportDefaults ImportDefaultson engineOptions[T]andLayeredOptions[T]. RemoveOptionsWithImportDefaults,LayeredOptionsWithImportDefaults, andprivateImportDefaults. Stage 3 changes root callers; a temporary compile failure is preferable to an interim bridge. - Adapt
snapshotImportDefaultsto the canonical type, retaining nil boolean pointers and nil versus non-nil empty pattern. KeepcloneBoolandcloneStringswhere they still have real consumers. Keep current snapshot call sites until Stage 3 establishes their final ownership placement. - 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: not started.
- Set engine
ImportDefaultsfields directly in roottoEngineOptionsandtoEngineLayeredOptions. RetaintoEngineImportDefaultsas the explicit root-to-engine type mapping, but map its booleans/pattern without copies. Remove rootcloneImportDefaultBoolandcloneImportDefaultStringsafter confirming no other caller. Keep public root types and declaration contracts. - Keep
snapshotImportDefaultsinloadSettingsas the load-entry ownership operation for both APIs. Remove repeated snapshots in engineLoadLayeredrequest construction,executeLoadRequestacquisition-policy construction, andresolveImportPolicy. 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. - 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.
- Replace the private conversion-snapshot fragment in
TestImportDefaultsPreserveEmptyPatternAndRequiredness, which converts options, mutates inputs, and calls engineLoad. It should no longer demand a snapshot from a private conversion helper. Keep that test's public nil/empty, requiredness, and inspection-evidence checks. - 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*.cfgto*.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. - 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: not started.
- Rename the existing engine enum and its constants to
PlacementMode,PlacementModeMapping, andPlacementModeSequenceItem, preserving underlyinguint8values 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. - Expose engine
Options[T].Destinationand.PlacementMode, andInput.PlacementMode. NormalizedloadInputand other private owner fields can retain their names. Populate the normalized input from those fields. RemoveOptionsWithPlacementandInputWithPlacementand migrate all callers, including the direct engine construction in rootrecords_test.go. - In root
toEngineOptionsandtoEngineInput, set destination using the existingtoEnginePathconversion and placement usingengine.PlacementMode(options.PlacementMode)or its input equivalent. Preserve invalid enum values fornormalizeInputto diagnose before reads. Do not add validation/defaulting to the facade or a new setter/cast function. - Preserve normalization of literal/parsed destination keys, root-item rejection,
wrapper accounting, mounted/item source coordinates, and version projection.
Root
Pathis immutable andtoEnginePathcarries 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. - 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.
- 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.
- 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.
- Update
docs/internal/imports.mdfor the selector/preparation split anddocs/internal/overview.mdfor 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. - Replace C03/F07 detail in the first two
docs/roadmap/future.mdentries with short pointers tointernal-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: usebb5540b:docs/roadmap/implementation.mdfor that historical plan. Do not reopen the prior audit or retire unrelated planning records in this sprint. - 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.