297 lines
19 KiB
Markdown
297 lines
19 KiB
Markdown
# Import Discovery And Facade Simplification Implementation Plan
|
||
|
||
Status: complete.
|
||
Baseline: `bb5540b62e382106d59678e2edb7655acfd5c960`.
|
||
Audience: a Luna coding agent implementing the stages sequentially.
|
||
|
||
## Scope And Execution
|
||
|
||
Implement **C03** and **F07** from the
|
||
[internal-boundary roadmap](internal-boundaries.md). 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](../policy/agents.md), the [development
|
||
guide](../development.md), 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: **complete**. Validation
|
||
passed on Linux/amd64 (Go 1.26.10):
|
||
`env GOTOOLCHAIN=local GOPROXY=off GOTELEMETRY=off ./scripts/check.sh`,
|
||
`env GOTOOLCHAIN=local GOPROXY=off GOTELEMETRY=off go test -race ./...`, and
|
||
`git diff --check`. Existing integration coverage was reviewed; no additional
|
||
case was needed.
|
||
|
||
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:
|
||
|
||
```sh
|
||
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.
|