diff --git a/docs/roadmap/interfaces.md b/docs/roadmap/interfaces.md new file mode 100644 index 0000000..57fd286 --- /dev/null +++ b/docs/roadmap/interfaces.md @@ -0,0 +1,197 @@ +# API and Interface Contract Audit + +Reviewed baseline: `01b08e76a3fecc22c4414437bfeaa07b4b7a85aa`. + +Status: four open findings. This is an audit record, not an implementation plan. +No production code or repository tests were changed. + +## Scope and evidence + +Reviewed the root-owned public facade and its conversions, engine loading and +callback boundaries, source/discovery and filesystem ownership, schema and typed +value handling, metadata/history, safe rendering and comparison, secrets, and +the document/YAML boundary against declaration comments, the +[configuration contract](../config.md), and applicable architectural decisions. +The root delegates to the synchronous engine lifecycle; parser-independent +documents support source assembly and inheritance, and completed results retain +detached configuration and frozen inspection state. + +The findings below were reproduced through the public API with a disposable Go +program outside the repository, using labeled byte sources and `fstest.MapFS`. +The full existing suite passed with Go 1.26.10 on Linux: + +```sh +GOTOOLCHAIN=local GOTELEMETRY=off GOPROXY=off go test ./... -count=1 +``` + +The existing tests cover many ownership, lifetime, error, and zero-value cases, +but do not detect these cross-interface discrepancies. No benchmarks, external +service calls, or new test files were needed. The recommendations restore +existing contracts rather than introduce new API restrictions or guarantees. + +## 1. Resolution presence queries confuse explicit keys with sequence indexes + +Priority: P2. Status: open. + +Relevant code: + +- [`Resolution.IsExplicit`](../../internal/engine/resolution.go), lines 75–82. +- `Resolution.InputInfo` and `resolutionInput.lookup` in + [resolution.go](../../internal/engine/resolution.go) and + [load_metadata.go](../../internal/engine/load_metadata.go). +- `resolveSchemaPath` in [schema.go](../../internal/engine/schema.go). + +The public [path contract](../../path.go) distinguishes explicit mapping keys +from sequence indexes. The [resolution contract](../config.md#recorded-application-resolution) +defines surviving-input presence and a fixed entry-state metadata view; +[metadata queries](../config.md#layered-metadata) preserve container shapes. + +`IsExplicit` validates against the schema but then looks up presence using only +`path.String()`. In dynamic `any` state, schema validation cannot establish the +actual container shape. Consequently these distinct queries collide: + +```go +key := configkit.Keys("shape", "0") +index := configkit.Keys("shape").Index(0) +``` + +Load `map[string]any` from `shape: [input]` with a resolver that queries both +paths, without changing the configuration. Observed results: + +| Query | `Resolution.IsExplicit` | `Resolution.InputInfo` found | Final `Metadata.IsExplicit` | +| --- | --- | --- | --- | +| Explicit mapping key | true | false | false | +| Explicit sequence index | true | true | true | + +The mapping-key path never existed and should not be reported as explicit. +Applications branching on presence receive inconsistent answers depending on +which public query they use, even during an otherwise unchanged load. + +Potential remediation: use the retained entry-state semantic shape when checking +resolution presence, consistently with `InputInfo` and `Metadata.IsExplicit`. +Review the same string-only presence lookup in `Default` while preserving its +existing assignment and callback-lifetime rules. Add focused public coverage for +both mapping-to-index and index-to-mapping mismatches under dynamic values, +including contextually parsed pointers. This does not require changing the +documented treatment of invalid schema paths. + +## 2. Resolution can transfer explicitness to a different semantic path + +Priority: P2. Status: open. + +Relevant code: `applyResolutionMetadata`, `applyResolutionFieldOrigins`, and +`refreshResolutionFields` in +[load_metadata.go](../../internal/engine/load_metadata.go), especially lines +371–387; final shape checking in +[`Metadata.effectiveField`](../../internal/engine/metadata.go). + +The [metadata contract](../config.md#layered-metadata) distinguishes key/index +shapes and keeps derived values for absent input non-explicit. Recorded +resolution changes origins without creating input explicitness. + +Reproduction: + +1. Load `map[string]any` from `shape: {'0': old}`. +2. In resolution, replace `configuration["shape"]` with `[]any{"new"}` and + record normalization at `Keys("shape")`. +3. Query final metadata using the two paths from finding 1. + +The old explicit mapping-key path correctly stops being effective, but the newly +created sequence index reports `Explicit: true`. That sequence index was never +supplied by input. `refreshResolutionFields` reuses the old `FieldInfo` whenever +its textual pointer survives; it updates null state without checking whether +the old and new containers give that pointer different semantic shapes. Final +lookup validates only the new shape, so it cannot recover the lost distinction. + +This can mislead consumers inspecting whether a final value was explicitly +configured rather than introduced by application resolution. The converse +sequence-to-numeric-key transition has the same underlying collision. + +Potential remediation: retain entry/effective shape evidence across resolution +and carry input explicitness forward only for the matching semantic path. +Preserve existing explicitness for same-shape paths and the recorded resolution +origin; do not clear every descendant's presence merely because an ancestor was +normalized. Cover both shape transitions and agreement among `Lookup`, +`IsExplicit`, and explanation. Fixing finding 1's callback query alone does not +repair this publication path. + +## 3. Metadata history mixes numeric-key and sequence-index definitions + +Priority: P2. Status: open. + +Relevant code: + +- [`Metadata.History`](../../internal/engine/metadata.go), lines 56–65. +- `metadataFromDecoded`, `applyOverlayMetadata`, and history event construction + in [load_metadata.go](../../internal/engine/load_metadata.go). +- Shape-aware replay in [`Inspection.Explain`](../../internal/engine/explain.go). + +The [metadata contract](../config.md#layered-metadata) promises semantic +key/index distinctions for historical as well as effective queries. The +historical-path index retains both shapes, but `History` uses that index only to +check that the query was ever known. It then retrieves the entire history bucket +by textual pointer, without selecting the query's shape. + +Reproduction: load `shape: {'0': old}` into `map[string]any`, then apply a CLI +override at `Keys("shape")` with `[]any{"new"}`. Both explicit shapes were +observed and share `/shape/0`. + +`History(Keys("shape").Index(0))` incorrectly includes the original base +definition of the mapping key, and identifies that base origin as the previous +origin of the CLI operation. The old input defined a key, not an index. In the +same successful result, `Explain` for the explicit index correctly shows only +its CLI introduction. `History` for the key and index returns identical records +despite their different lifetimes. + +Consequences include false source attribution and disagreement between the two +public history interfaces when consumers inspect removed or introduced paths. + +Potential remediation: retain enough container-shape evidence with stored +history events, or select relevant events from existing shape-aware replay +evidence. Filter exact definitions and previous origins by the queried semantic +shape while retaining genuine ancestor replacement/removal events. Filtering +only on the current `Change.Path` is insufficient where construction already +replaced concrete segment kinds with parsed tokens. Preserve the intentional +one-pointer-per-entry `HistoricalPaths` enumeration and contextual behavior of +`ParsePath`; neither public contract needs redesign. Cover both direction changes +and compare metadata history with explanation. + +## 4. Diagnostic canonicalization breaks composition claimant ordering + +Priority: P3. Status: open. + +Relevant code: `compositionConflictDiagnostic` in +[compose.go](../../internal/engine/compose.go) and `canonicalDiagnostics` in +[diagnostic.go](../../internal/engine/diagnostic.go), particularly lines 88–92. +The latter runs both while constructing `LoadError` and in `WriteDiagnostics`. + +The [strict-composition contract](../config.md#strict-document-composition) +specifies the first claim in scoped assembly order as primary and every remaining +claim in the same order as related locations. + +A native-filesystem fixture demonstrates the discrepancy: + +```text +main.yml imports a.yml, b.yml, c.yml +a.yml imports z.yml +b.yml, c.yml, and z.yml each define x +``` + +Acquisition assigns main=source-1, a=source-2, z=source-3, b=source-4, +c=source-5. Dependency-ready assembly considers the three conflicting claims +in b, c, z order. The composer correctly chooses b as primary and constructs +related locations c, z. Generic canonicalization sorts those related locations +by acquisition ID, publishing z, c instead. The reproduced public diagnostic +has primary source-4 and related source-3, source-5. + +All claimants remain present and the conflict still fails loading, so this is +lower impact than the presence/history defects. It nevertheless violates the +documented reporting order and gives consumers two different ordering rules +within one conflict report. + +Potential remediation: preserve the selected semantic ordering for composition +claimant lists through accumulation and rendering, while retaining canonical +ordering of diagnostic records and the established sorting of related locations +for diagnostics without a semantic-order contract. Add one public failure-path +case whose acquisition and claimant orders differ and verify both structured +diagnostics and rendered output. No scheduler or precedence change is needed.