ConfigKit/docs/roadmap/remediation.md

Configuration Discovery Review Findings

Status: complete. All five findings have been addressed in separate commits.

Reviewed commit: e50fee2, including the discovery implementation commits after 424c897. The review compared the implementation with the discovery roadmap, implementation plan, ADR 0019, and the existing configuration and architecture contracts.

1. Defer Absence-Only Context Requirements Until The Search Outcome Is Known

Priority: P2. Status: complete.

Completed: search prevalidation now defers absence-only root requirements until selection. Ordinary, layered-main, and underlay regression cases compare searched files with explicit files under an empty context and retain errors for absent inputs. The regression failed before the fix. Focused context/absence tests and ./scripts/check.sh passed with Go 1.26.10 on Linux.

Location: filesystem.go, lines 77–123, and the outcome-specific validation in load_input.go, lines 103–122.

The sourceSearch branch calls absentFilesystemOptionDiagnostics before selection. That helper requires a nonempty host Root, even though a selected host file can derive both its base and default root from its own path.

Confirmed reproduction: with an existing file containing value: selected and Options[Config]{Filesystem: &FilesystemContext{}}, ordinary File(path) loading succeeds. Replacing only the source with SearchConfig("", Paths(path)) fails with options.invalid and the message "absent-input filesystem root and base directories must be absolute host paths". No candidate is acquired. The same source distinction affects layered inputs because their validation uses the same helper.

This applies an absent-input requirement to a selected-file workflow and unintentionally strengthens its contract. The roadmap requires selected files to retain normal file-context behavior; the plan explicitly separates static syntax checks from outcome-dependent compatibility checks.

Required correction: validate only outcome-independent context syntax before discovery. Once the outcome is known, apply normal host-file rules to selection and absent-input rules to exhaustion. Preserve the required explicit root for an absent input with a supplied host context, selected-file rejection of BaseDir or native FS, containment checks, and terminal context failures.

Verification: add ordinary and layered selected-file equivalence cases with an explicit empty context. Also verify that optional exhaustion with that same context still fails, and that the existing root-only selected/absent overlay cases continue to pass. Do not change File to reject its currently valid context or require callers to omit an otherwise valid options value.

2. Preserve The Failing Candidate's Identity In Access Diagnostics

Priority: P2. Status: complete.

Completed: terminal probe failures retain the candidate descriptor and gain normal file-source evidence at the acquisition boundary. Regression tests compare ordinary and later-underlay failures with explicit-file errors, including source IDs, placement, layers, and skipped missing candidates. A controlled permission probe checks the diagnostic-to-file link. The load regression failed before the fix; focused discovery/failure tests and ./scripts/check.sh passed on Linux.

Location: discovery.go, lines 517–528, and the early resolver-error return in load_input.go, lines 105–109.

When symlink resolution or stat fails for a reached candidate with an error other than absence, the resolver returns a diagnostic with an empty source ID and discards the candidate. The acquisition boundary returns no source evidence. Consequently neither LoadError.Sources() nor the diagnostic identifies which candidate failed. The permission-probe test in discovery_test.go also asserts a nil primary location, preserving this omission rather than the intended normal source diagnostics.

Confirmed reproduction: create a regular app.yml, then search paths app.yml/child.yml followed by the valid app.yml. The first path fails with ENOTDIR. Explicit File(badPath) returns source.read with a primary source location and a file record identifying badPath. The equivalent search correctly stops, but returns source.read with no primary location and an empty source list. An operator cannot locate the problem from the structured error.

The roadmap calls for normal candidate access diagnostics, and the plan requires normal candidate records for file failures. The decision to omit a public attempt trace does not require discarding the one failing file's identity.

Required correction: retain the failing candidate and expose its normal file-source diagnostic identity at the acquisition boundary, with the appropriate source ID, safe path/label, and input placement/layer. Keep missing attempts absent from records and budgets. Required exhaustion and directory-helper errors still have no invented file location. Preserve terminal access errors and avoid adding a public attempt history, raw OS error strings, or YAML values.

Verification: cover a deterministic real candidate-resolution failure and a controlled permission/stat failure. Check the diagnostic's location against the returned file record, including an underlay after earlier acquired sources. Assert that no later candidate is loaded and that skipped missing candidates still produce no records. Revise the existing nil-primary assertion accordingly.

3. Isolate User-Configuration Preset Tests Using The Platform's Go Convention

Priority: P2. Status: complete.

Completed: the environment-change test selects the environment variable used by Go on Windows, macOS/iOS, Plan 9, or other Unix hosts, and obtains both fixture bases through os.UserConfigDir. It remains nonparallel and restores the environment. The redundant relative-XDG failure assumption was removed; existing private-helper tests retain portable failure/short-circuit coverage. Focused tests and ./scripts/check.sh passed on Linux. The macOS branch was checked against Go's helper implementation; no native macOS gate was added.

Location: discovery_load_test.go, TestSearchSourceObservesUserConfigEnvironmentChanges, lines 231–279.

The new test treats every non-Windows platform as using XDG_CONFIG_HOME, writes both fixtures beneath that variable, and leaves the actual home environment unchanged. On macOS, os.UserConfigDir uses $HOME/Library/Application Support and ignores XDG_CONFIG_HOME. The first load therefore searches the real user's configuration directory instead of the fixture, and the environment change cannot select the second fixture. This makes the test fail on an ordinary macOS checkout and potentially depend on local files.

This is confirmed by inspection of the selected Go 1.26.10 standard library's os.UserConfigDir implementation and the test. A native macOS run was not performed or required. Production preset resolution correctly delegates to Go; the defect is in the test setup.

Required correction: control the environment consumed by the platform's Go helper, then obtain fixture bases through os.UserConfigDir rather than assuming the XDG layout. On macOS, use a temporary home and its derived configuration directory; retain appropriate isolation on Linux and Windows. Keep environment changes restored and the test nonparallel. Use the existing private helper seam for portable directory-resolution failure tests rather than assuming that a relative environment value fails on every platform.

Verification: the environment-change test must observe both controlled fixtures and avoid the real home/config directory. Review the macOS branch against Go's helper behavior. Keep macOS best-effort support and the existing release procedure; do not introduce native macOS validation as a gate.

4. Use Platform-Aware Windows Component Validation

Priority: P3. Status: complete. Algorithmic audit: correctness finding.

Completed: Windows validation now delegates reserved-name decisions to filepath.IsLocal while preserving the selected component/character checks. The manual device table was removed. Tests cover console names, platform-dependent extension behavior, and invalid later search components; Unix tests retain their own filename rules. Focused tests and ./scripts/check.sh passed on Linux. The Windows engine test binary cross-compiled successfully; it was not run natively.

Location: discovery.go, validWindowsFilenameComponent, lines 266–281.

The hand-maintained device-name switch omits CONIN$ and CONOUT$. Both are Windows console device names, not ordinary filename components. The helper accepts them, so on Windows a slug or custom filename using one can pass static validation. That conflicts with the selected usable-host-component rule and can defer an invalid description until acquisition or hide it behind an earlier successful candidate.

Evidence: an external Go test overlay calling the current private helper failed for both names: validWindowsFilenameComponent returned true. The Go 1.26.10 Windows implementation of filepath.IsLocal recognizes both as reserved, and its own tests include both spellings. The overlay ran on Linux and exercised the helper directly; it was not a native Windows load test. No repository test file was added.

Complexity and refactoring opportunity: the current predicate takes O(L) time for a component of length L, with O(L) temporary storage for case conversion. No asymptotic optimization is needed. Prefer the standard library's platform-aware component/local-path checks within the Windows branch, retaining the selected single-component and invalid-character checks as needed. This avoids maintaining an incomplete device table. Go's Windows reserved-name logic also accounts for OS-dependent extension behavior; do not replace it with a broader blanket naming prohibition.

Verification: add appropriate Windows-specific cases for console names and ordinary names, including invalid later components behind an earlier match. Keep those tests consistent with the host Go helpers rather than imposing their Windows behavior on Unix. This is a correction to existing platform-specific code, not a new Windows support promise or native-platform release gate.

5. Preserve Drive-Root-Relative Windows Path Meaning

Priority: P3. Status: complete. Algorithmic audit: correctness finding.

Completed: drive-root-relative Windows paths now use filepath.Abs alongside drive-relative paths, preserving native host interpretation. Windows regression tests compare candidates and loaded files with explicit File for relative, drive-root-relative (both separators), drive-relative, and absolute paths, through both Paths and SearchDirectory. Existing Unix path/short-circuit tests and ./scripts/check.sh passed. The Windows engine test binary cross-compiled; no native Windows execution or new release gate was introduced.

Location: discovery.go, absoluteCandidate, lines 380–391; also used for literal directory searches.

On Windows, a path such as \config.yml has no volume name and is not absolute according to filepath.IsAbs, but it is rooted at the current drive's root. The helper treats it as an ordinary relative path and joins it to the whole working directory. With a working directory of C:\work, discovery therefore probes C:\work\config.yml instead of C:\config.yml. The same issue affects slash-rooted Windows spellings and custom directory searches.

This can select a different file from explicit File loading, which uses filepath.Abs. The roadmap requires literal host paths consistent with explicit file loading; it does not select an alternate interpretation of drive-root paths.

Evidence: static tracing against the selected Go 1.26.10 Windows IsAbs, VolumeName, Join, and Abs implementations establishes the branch and result. The first two checks route this path into Join, which appends it beneath the working directory; Windows Abs delegates to the native full-path operation. A native Windows execution was not performed or required.

Complexity and refactoring opportunity: both current normalization and the correct standard-library route are linear in the path spelling length, apart from OS helper work. This is a correctness refactoring, not a scaling problem. Route Windows drive-root-relative paths through filepath.Abs, as the helper already does for volume-qualified relative paths, or use a narrow private absolute-path helper that delegates to Go in production. Preserve lazy base resolution and the ordinary-relative-path test seam. No new public API or hand-written Windows path parser is needed.

Verification: on Windows, compare discovery's candidate interpretation with filepath.Abs and explicit File for ordinary relative, drive-root-relative, drive-relative, and absolute paths. Cover both Paths and a literal SearchDirectory, and retain the existing Unix literal-path tests. Do not add a new native Windows validation requirement to the release procedure.

Algorithmic Audit Scope And Conclusions

The additional audit reviewed new production code from 424c897 through e50fee2; the current code at 68cf5c4 is unchanged from that implementation. It preserved the expressly planned flat snapshots, ordered first-match search, directory-first expansion, whole-description static validation, and reuse of ordinary file acquisition. It did not benchmark or reopen those choices.

• Let S be the description data processed by an operation, including entries, filename groups, custom names, retained issues, and supplied option lists. Snapshot/concatenation construction is O(S) per construction operation with O(S) owned storage. The nested clone loops visit each stored group/name once; they are not repeated scans of the same list for each candidate. Repeated immutable prefix rebuilding by a caller follows the explicitly planned flat-snapshot representation and was not treated as a new algorithm finding.
• Validation is O(S) plus the existing canonical diagnostic ordering cost, O(E log E) comparisons for E errors. Validation occurs a fixed number of times per consumed description, not once per candidate. The audit found no new repeated sorting or quadratic validation pass.
• Iteration advances persistent entry/group/name indexes. Across the search it visits each reached group and candidate a constant number of times, with no restart of earlier candidate lists. Traversal is O(S + C) in descriptor and candidate counts, plus generated path bytes and ordinary filesystem work, with O(1) iterator state beyond the description and current path storage. Descriptions are not expanded into a complete candidate product in memory.
• Each attempt performs a bounded number of standard path/probe operations; selection stops on success or a terminal error. Selected-file acquisition and the final lifecycle run once. The added absent-context handling checks a fixed number of directories and does not introduce scans over prior configuration contributions.

No unplanned quadratic algorithm or other actionable asymptotic regression was identified. Findings 4 and 5 record the additional ordinary correctness issues found while examining those algorithms; findings 1–3 were carried forward from the earlier review. Existing configuration-tree algorithms outside the discovery changes were not re-audited as new feature work.

Review Validation

• ./scripts/check.sh passed with Go 1.26.10 on Linux: formatting/whitespace, all tests, vet, and build.
• go test -race ./... passed on the same toolchain.
• An external temporary Go program reproduced findings 1 and 2 through the public API, comparing explicit file loading with discovery. No production or test files were modified for these reproductions.
• Findings 1–3 remained open at the original review despite the suites passing. The broader discovery architecture is implemented: immutable root-owned descriptions, incremental ordered selection during shared source acquisition, concrete selected-source propagation, and shared absent-input handling before the existing composition, inheritance, decoding, resolution, and publication lifecycle. No additional release gates or new composition semantics were found.
• The algorithmic audit used source-level complexity analysis and the temporary helper regression overlay described in finding 4. It collected no timing or allocation measurements. Findings 4 and 5 are platform-specific correctness findings, not claims of a native Windows validation run.

Completed Remediation Validation

All five fixes include focused regression coverage and per-issue completion markers above. The final ./scripts/check.sh and go test -race ./... passed with Go 1.26.10 on Linux. The Windows engine tests also cross-compiled with GOOS=windows GOARCH=amd64 CGO_ENABLED=0 go test -c, with the binary kept outside the repository. No native Windows or macOS run was performed or made a gate. The implementation preserves the public API, candidate ordering, optional absence, and existing composition/version/ownership contracts.