eric/ConfigKit
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Retire completed plans and outline named inheritance

Browse Source
This commit is contained in:
Eric Rakestraw
2026-09-29 20:11:42 +00:00
parent b604aa167b
commit 0a4b774bfe
9 changed files with 301 additions and 1318 deletions
Download Patch File Download Diff File Expand all files Collapse all files

141
docs/roadmap/remediation.md
View File

@@ -1,141 +0,0 @@
# Filesystem Sources and Underlays Review Findings
Reviewed implementation: `cbf031a`, covering the implementation commits after
`27df838`. These findings concern the agreed source, precedence, and provenance
contracts in the [roadmap](sources-and-layering.md) and
[implementation plan](implementation.md). No implementation fixes are included
in this review.
## Resolution status
All three findings were addressed on 2026-09-29:
- Composition now fills sequence-descendant origins separately from atomic
conflict claims, so initial underlay attribution reaches every surviving node.
- History reconciliation checks contributing origins and recognizes ancestor
replacement events without duplicating their descendant contributions.
- Native contexts use `fs.ValidPath`; host contexts use `filepath.Localize`
before the existing canonicalization and containment checks.
`facade_underlay_origins_test.go` covers imported sequence descendants in first
and later tiers, resolver/result/explanation agreement, and later mapping
contributors. `TestResourceContextsPreserveLiteralBackslashes` covers native
and host resource reads and directory listing. The new tests reproduced all
three failures before the fixes. The findings below retain the original review
evidence and acceptance requirements.
Post-fix validation passed: `./scripts/check.sh` and
`GOPROXY=off go test -race ./...`. Existing sequence replacement, provenance,
history, and resource-containment tests remain passing.
## 1. Preserve underlay attribution for sequence descendants (P2)
**Owners:** `internal/engine/load_request.go:174`,
`internal/engine/compose.go:113`, and `internal/engine/load_result.go:39`.
The first nonempty underlay seeds the accumulator using a composed document.
Composition claims intentionally stop at sequences because sequences are atomic
for strict conflict checking. Consequently its origin index contains the sequence
itself but not sequence elements or nested fields inside them. Relabeling that
index as `LayerUnderlay` leaves those descendants with the decoder's default
`LayerBase` attribution. Later overlay insertion recursively collects origins,
so attribution also depends on whether the same input is the initial tier.
**Reproduction:** load `NoSource()` as main and an underlay mapping containing
`items: [{name: first}]`. The load succeeds. `Metadata.Lookup` reports
`LayerUnderlay` for `/items`, but `LayerBase` for `/items/0` and `/items/0/name`,
all referring to the same underlay source ID. `Metadata.History` repeats the
incorrect base attribution for the descendants.
**Required correction:** cover every surviving input node when assigning origins,
including descendants of atomic sequence values. Keep strict-composition claims
and sequence replacement semantics unchanged. Ensure resolver entry metadata,
returned metadata, and explanation agree with each acquired source's role.
**Regression coverage:** a first-tier sequence containing nested mappings,
including an imported contribution, retained beneath main input; compare with
equivalent later-tier insertion. Assert source ID and underlay role for elements
and their fields as well as the sequence container.
## 2. Preserve later same-role mapping contributions in history (P2)
**Owner:** `internal/engine/load_metadata.go:128` (`applyOverlayMetadata`).
The final-history reconciliation skips an effective origin when its layer and
operation match the last recorded event. Multiple underlay tiers share
`LayerUnderlay`, so those two fields do not establish that the recorded event
contains the effective source contributors. The comparison cannot distinguish
contributions from repeated tiers of the same role.
**Reproduction:** with no main document, supply three ordered underlays:
```yaml
# Tier 1
x: 1
# Tier 2
x: {}
# Tier 3
x: {y: 2}
```
The final `/x` origin correctly includes sources 2 and 3. Its metadata history
contains only source 1 and the replacement by source 2; source 3's contribution
is omitted. The returned effective provenance and chronological history disagree.
**Required correction:** determine whether the effective provenance is already
represented using contributing origins, not merely layer/operation equality.
Preserve meaningful same-tier-role contributions without duplicating events
already represented. Keep existing replacement, equal-value, and displaced-path
history behavior.
**Regression coverage:** scalar-to-mapping replacement followed by a recursive
mapping contribution from a later underlay; assert the final contributor appears
in history and that metadata and explanation describe consistent source origins.
## 3. Honor standard filesystem names in resource contexts (P2)
**Owner:** `internal/engine/resource_context.go:38` and `:77`.
Both resource adapters reject any name containing a backslash, beyond
`fs.ValidPath` validation. A backslash is a legal literal filename character in
an `io/fs` namespace and on the Linux host filesystem. Native source acquisition
accepts such paths, but resource-context access rejects them. This introduces an
unrequested path restriction and applies host-style separator assumptions to a
generic filesystem.
**Reproduction:** supply `fstest.MapFS` with `config.yml` and a resource named
`asset\name`. Loading `config.yml` succeeds; the resource name passes
`fs.ValidPath` and can be read directly from the supplied filesystem.
The equivalent read through the returned context fails with `fs.ErrInvalid`:
```go
context, ok := result.SourceContext(sourceID) // ok is true
_, err := fs.ReadFile(context.FS, name) // rejects the valid name
```
**Required correction:** use standard slash-path validation for native contexts
and preserve literal characters accepted by the supplied filesystem. For host
contexts, use platform-appropriate conversion while preserving the existing
contained-root behavior; native namespace semantics must not inherit Windows
separator restrictions. This is not a request to broaden root traversal or
change symlink guarantees.
**Regression coverage:** access a literal-backslash resource through both native
Open/ReadDir paths where applicable and a Linux host context; retain invalid-path
and outside-root coverage. Avoid rejecting valid native names on other platforms
merely because the corresponding host path would be invalid there.
## Review validation
- `./scripts/check.sh` passed (tests, vet, build, formatting, and whitespace).
- `GOPROXY=off go test -race ./...` passed.
- Broad-catalog and many-small-tier benchmarks ran with `-benchmem` and 50 ms
samples. Allocated bytes scaled approximately with tested input sizes; these
measurements are review evidence, not timing gates.
- A temporary external Go program reproduced all three findings through the
public API; no diagnostic/test code was added to the repository.
The intended facade, shared synchronous lifecycle, strict within-tier composition,
recursive precedence, mapping snapshotting, and passive resource-context design
are present. The findings above are correctness gaps within that design, not a
recommendation to expand its scope or change its selected combination semantics.