base-layer/configkit
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases 3 Wiki Activity

Record main-relative loading review findings

Browse Source
This commit is contained in:
Eric Rakestraw
2026-11-09 20:56:01 +00:00
parent 18af2d2130
commit d2608e8d56
1 changed files with 108 additions and 0 deletions
Download Patch File Download Diff File Expand all files Collapse all files

108
docs/roadmap/remediation.md Normal file
View File

@@ -0,0 +1,108 @@
# Main-relative companion implementation review
Reviewed the eight implementation commits from `ae0c94d` through `18af2d2`
against the [implementation plan](implementation.md) and
[feature roadmap](main-relative-inputs.md).
| Stage | Review result |
| --- | --- |
| 1 | The root facade copies binding and fallback descriptions. Preflight checks active binding scope and static context shape. Concrete overrides and inactive overlays take precedence over shadowed settings. |
| 2 | The acquired main's resolved context feeds underlay acquisition. Derived inputs retain the namespace and containment root, clear the main's physical path, and use receiving-tier host identity. Fallback selection and namespace anchors use the chosen context. |
| 3 | Bound traversal and contextless imports share declaration policy and placement preparation, with source-local diagnostic sites. |
| 4 | Without a base, the loader retains payload and version processing, honors requiredness and the empty named-path exception, and emits no fabricated selection evidence. Root assembly has a second implementation. See Finding 2. |
| 5 | Layered overlays select explicit, main-derived, then fallback context. Lookup base and containment root are separate. Native link handling leaves a gap in containment and duplicate detection. See Finding 1. |
| 6 | The executable example covers discovery, embedded defaults, mounted file and recursive directory imports, transitive imports, whole-record upsert, and the selected overlay. A companion test rejects duplicate peers. |
| 7 | Public and engine tests cover discovery, source and version evidence, optional resources, budgets, and cleanup. The exhaustion and lazy fallback branches need focused regression coverage. See Finding 3. |
| 8 | The configuration contract, public comments, architecture policy, and ADR describe deferred binding. Two supporting descriptions misstate ownership. See Finding 4. |
## Finding 1 — Native overlay symlinks bypass containment and duplicate checks
**Priority P2.** The layered branch passes the inherited root and lookup base
to the shared overlay loader. That loader calls `filesystemAccess.canonical`.
For a native filesystem, the function stats the path and returns its lexical
spelling even when the filesystem implements `fs.ReadLinkFS`. The root and
duplicate checks then compare that spelling, while the subsequent read can
follow the link. The same limitation predates this sprint in the shared
loader. Automatic derivation makes it reachable without an explicit overlay
filesystem, and the new contract describes the inherited root as the
containment boundary. [Layered overlay selection](../../internal/engine/load_request.go#L192)
· [Overlay checks](../../internal/engine/selected_overlay.go#L74)
· [Native path lookup](../../internal/engine/filesystem_access.go#L94)
· [Overlay contract](../config.md#selected-overlays)
Reproduced through `LoadLayered` on Go 1.27.3 using
`NewFilesystem("native", os.DirFS(dir))`, which exposes `fs.ReadLinkFS`.
The main is `scope/main.yml` with a context rooted at `scope`. There is no
explicit overlay context.
- `scope/overlays/escape.yml` points to `../../outside.yml`. The load
succeeds and takes `count: 9` from outside the root.
- `scope/overlays/repeat.yml` points to `../main.yml`. The load succeeds
and reports the main content a second time under the overlay alias.
Apply capability-aware link resolution to native overlay targets, using the
existing native link resolver where applicable. Use the resolved path for
containment, repetition, and reading. Check the resolved identity against
already acquired files in the same namespace, including aliases where those
files retain lexical paths. Add focused derived-overlay regressions for the
escape and repeat cases. Preserve root-relative lookup for ordinary and
explicit overlays and document the limit for native filesystems that cannot
expose link targets. This requires no general sandbox guarantee for caller
filesystems. [Native link resolver](../../internal/engine/filesystem_links.go#L227)
## Finding 2 — Contextless imports duplicate root contribution assembly
**Priority P3.** Stage 4 calls for sharing the root contribution assembly
and accounting. The new contextless path has its own source charging,
mapping placement, wrapper charging, pending sequence-item scope
construction, and placement reservation. Bound imports perform those same
steps independently. Both paths pass today’s tests, but changes to placement
or accounting now need to be made twice for the same declaring payload.
The initial limit diagnostics already differ in specificity.
[Contextless path](../../internal/engine/contextless_imports.go#L51)
· [Bound import assembly](../../internal/engine/import_acquisition.go#L115)
Share the preparation and finalization of the declaring root contribution
where the paths have the same semantics. Keep the contextless requiredness
decision and bound traversal separate. Preserve their existing evidence and
the contextless path's lack of filesystem access. Use existing mapping,
sequence-item, and budget checks to verify the extraction.
## Finding 3 — Exhaustion and lazy fallback lack focused regression coverage
**Priority P3.** The context-selection test uses an existing fallback when
checking that main derivation wins. It would still pass if the loader
prematurely opened that fallback. The no-base tests use `NoSource()`, and
the discovery integration covers a successful selection or a selected
parse failure. These cases leave the optional-search exhaustion handoff to
both companion and overlay selection unprotected.
[Context selection tests](../../facade_main_relative_test.go#L39)
· [Discovery integration tests](../../facade_main_relative_integration_test.go#L12)
A disposable public API probe confirmed the current behavior. Exhausted
optional search without context retained the declaring payload with no empty
selection evidence. The same search with a root-only main context loaded a
companion and overlay despite a well-shaped but nonexistent fallback. With
the main context removed, the selected missing fallback failed. Add a small
public regression table for those outcomes. Include an overlay-only failure
case if the shared fixture would otherwise stop at the companion error
before reaching overlay selection.
## Finding 4 — Supporting documentation misstates binding ownership
**Priority P3.** The consumer guide locates main-relative bindings in
`LayeredOptions`, but a companion binds through `Input.MainRelative`.
The import internals guide says passing the main context replaces tier-local
identity state. The engine preserves the receiving tier's bookkeeping and
sets the derived host token to that tier's token. Correct these descriptions
and leave the observable rules with the configuration contract.
[Consumer guide](../consumers/README.md#choosing-a-loading-entry-point)
· [Import internals](../internal/imports.md#discovery-and-acquisition)
· [Receiving tier](../../internal/engine/load_request.go#L276)
## Verification
The everyday checker and the full race suite passed on Linux with Go 1.27.3.
Disposable public API probes reproduced Finding 1 and checked the exhaustion
and fallback behavior in Finding 3. The review found no additional contract
strengthening beyond the native containment wording discussed in Finding 1.