diff --git a/docs/development.md b/docs/development.md index 2770d18..1f82550 100644 --- a/docs/development.md +++ b/docs/development.md @@ -160,6 +160,10 @@ git show 5747ec4:docs/roadmap/implementation.md git show 5747ec4:docs/roadmap/remediation.md ``` +The subsequent import-integration audit and completed remediation record are +preserved at `6903c9e:docs/roadmap/remediation.md`; retrieve them with +`git show 6903c9e:docs/roadmap/remediation.md`. + Potential enhancements are tracked in [future features](roadmap/future.md). Install the Go version required by `go.mod` or a compatible newer toolchain. diff --git a/docs/roadmap/remediation.md b/docs/roadmap/remediation.md deleted file mode 100644 index 052d874..0000000 --- a/docs/roadmap/remediation.md +++ /dev/null @@ -1,163 +0,0 @@ -# Import Integration Correctness Audit - -Status: complete; all three findings addressed in separate validated units. - -Reviewed tree: `91db99b` (including the six completed fixes through `5747ec4`). -Scope: import discovery/defaults and shared dependency-ready scheduling, including -their interaction with existing strict composition, precedence tiers, sequence -placement, source-version checking, inheritance, provenance/history, passive -resource contexts, and maintained consumer examples. The completed design and -implementation plan remain available at `5747ec4:docs/roadmap/imports.md` and -`5747ec4:docs/roadmap/implementation.md`. - -The audit made no production code or contract changes. Preserve the -accepted syntax, defaults, scheduling, containment, and namespace decisions when -addressing these findings. - -## 1. [x] P2 — Accept contained absolute symlinks in directory-root prefixes - -Owners: [`importEntryTarget`](../../internal/engine/filesystem_links.go), -especially the initial host `a.root.Lstat(relative)` around line 30, and -[`selectDirectoryImportCandidates`](../../internal/engine/import_selection.go), -around lines 114–119. - -A declared directory root below a contained absolute symlink fails even though -the same source is accessible through the other selectors. This contradicts the -[import contract](../config.md#explicit-imports): explicit roots retain normal -access behavior, and contained relative or absolute host symlink targets are -accepted. - -Reproduction on Linux with Go 1.26.9: - -- A configuration root contains `real/parts/part.yml` with `value: included`. -- `absolute` is a symlink to the absolute path of that root's `real` directory. -- `main.yml` enables imports and declares `dir: absolute/parts` with - `pattern: '*.yml'`. -- Loading fails with `import.invalid` at `/configkit/imports/0/dir`, reporting - `directory entry could not be inspected`. -- `path: absolute/parts/part.yml`, `glob: 'absolute/parts/*.yml'`, and - `dir: absolute` with recursive discovery all succeed and load the same file. - Replacing the absolute symlink target with relative `real` also succeeds. - -Named-target validation already accepts the contained canonical directory. -Discovery then inspects its original spelling through `os.Root.Lstat`, which -rejects an absolute symlink in an intermediate component before the existing -component-wise resolver can handle it. Resolving the link as the final component -does not encounter that failure, explaining the inconsistent results. - -Remediation: make explicit directory-root inspection handle contained symlinks -in intermediate components as well as final components. Keep candidate spelling -separate from canonical host access/source identity, and preserve containment, -cycle detection, native identity, and discovered-entry `follow` behavior. Do not -solve this by prohibiting absolute symlinks or relaxing root containment. Add a -focused public regression for a directory root below an absolute contained link; -reuse existing escaping-link and cycle coverage. - -Completed: host entry inspection resolves intermediate parent components through -the existing containment-aware resolver before inspecting the final entry. Native -handling and the final entry's `follow` policy are unchanged. Added a public -regression for directory roots below an absolute contained link with both -`follow` settings, including canonical source identity. Confirmed that the test -fails before the fix and passes afterward. Focused facade and engine directory, -glob, symlink, and cycle tests passed, as did `./scripts/check.sh`, -`go test -race -count=1 ./...`, and `git diff --check`. The original public -reproduction now succeeds for all selectors. - -## 2. [x] P2 — Describe scheduled provenance and history order consistently - -Owner: [configuration contract](../config.md), especially layered metadata -around lines 431–439, historical explanation around lines 501–504, and strict -composition diagnostics around lines 821–825. - -These sections still promise source/declaration traversal order for composite -origin locations, base-source history, and conflict locations. The updated import -section correctly distinguishes authored acquisition from dependency-ready -assembly. Current inspection and diagnostics follow assembly order within a -strict scope, not source-record acquisition order. - -Public reproduction: - -- `main.yml` contributes `settings: {main: main}` and imports `z.yml`, then - `a.yml`; each import contributes a distinct child under `settings`. -- `Sources()` returns `main.yml`, `z.yml`, `a.yml`, with IDs `source-1`, - `source-2`, `source-3`. -- The composite origin and `Explain(/settings)` input history use `a.yml`, - `z.yml`, `main.yml` (`source-3`, `source-2`, `source-1`). -- In a separate main/child scalar conflict, the imported child's claim is - primary and the main claim is related, despite the main source being acquired - first. - -This is the intended scheduler behavior, not a production ordering defect. -Remediation: update the older contract sections to distinguish acquisition order -from scoped assembly/replay order and link to the existing ordering definition. -Keep source IDs and source records in acquisition order, precedence tiers in -precedence order, and item-local scope boundaries intact. Do not restore the old -DFS behavior or introduce an independent provenance ordering rule. Existing -scheduled-placement and import-order tests already cover the main mechanics; -check the revised wording against the public cases above. - -Completed: composite-origin, explanation-history, and conflict-location wording -now links to scoped assembly order and distinguishes it from source acquisition. -The descriptions retain precedence-tier order and item-local scope boundaries. -No scheduler or provenance implementation changed. Verified the public composite -origin/history and conflict reproductions, existing scheduled-placement and main/ -underlay ordering tests, local documentation links, and `git diff --check`. - -## 3. [x] P3 — Qualify the empty-path inspection exception by requiredness - -Owner: [import inspection contract](../config.md#explicit-imports), around lines -1010–1011. - -The contract says that empty `path: []` succeeds but does not produce an -optional-empty record. That is true only when the declaration is effectively -required. With omitted/default-false `required`, the implementation returns one -`ImportSelections` record with selector `path`, outcome `none`, and the -source-local declaration path. Adding `required: true` succeeds without that -record. - -The accepted roadmap explicitly includes deliberate empty lists among optional -no-selection outcomes. The implementation plan singles out **required** empty -paths for the exception. The -[engine evidence tests](../../internal/engine/import_evidence_test.go) already -exercise both forms and intentionally expect evidence for the optional form; -the [public record comment](../../records.go) agrees with that behavior. - -Remediation: qualify the documentation exception as applying to effectively -required empty path lists. Retain optional empty-list evidence and existing -tests rather than changing production behavior to match the inaccurate sentence. -Preserve the distinction between no selection and missing-file evidence. - -Completed: the import inspection contract now states that empty path lists -succeed regardless of requiredness, recording `none` only when effectively -optional. Production behavior and existing tests are unchanged. Verified both -forms through the public API and the existing optional-empty evidence test; -`./scripts/check.sh`, local documentation links, and `git diff --check` passed. - -## Validation and remaining architecture - -The ordinary development check and an uncached full race run passed on the -reviewed production tree: - -```sh -PATH=/snap/go/current/bin:$PATH GOTOOLCHAIN=local GOTELEMETRY=off GOPROXY=off \ - GOCACHE=/tmp/configkit-review-cache-shared ./scripts/check.sh -GOTOOLCHAIN=local GOTELEMETRY=off GOPROXY=off \ - GOCACHE=/tmp/configkit-review-cache-shared \ - /snap/go/current/bin/go test -race -count=1 ./... -``` - -The suite includes executable public examples and maintained consumer fixtures. -Additional disposable public-API probes outside the repository confirmed the -three findings and exercised scheduled item/helper placement in ordinary and -underlay loads, nested sequence values, projected source destinations, consumed -version-header sensitivity, and agreement of effective values with inspection. -The filesystem reproduction used real temporary files and symlinks. - -The root-owned facade, private engine, parser-independent document tree, and YAML -adapter remain separated. Discovery retains authored acquisition order; the -shared scheduler governs scoped assembly and sibling dependencies. Strict -composition within tiers, recursive precedence between tiers, destination-local -inheritance, and the subsequent typed lifecycle retain their existing roles. -No further correctness issue was confirmed in the reviewed interactions. These -checks do not provide macOS or external downstream-application validation, and -do not constitute release publication.