diff --git a/docs/roadmap/remediation.md b/docs/roadmap/remediation.md new file mode 100644 index 0000000..657ec69 --- /dev/null +++ b/docs/roadmap/remediation.md @@ -0,0 +1,77 @@ +# Consolidation Review Findings + +Reviewed revision: `557e006a4ab98b666735a71b3845418c702f7cec`. +Comparison baseline: `8ab1568` (the committed implementation plan). + +The review compared the [consolidation target](consolidation.md) and +[implementation plan](implementation.md) with the completed sprint, including +public/private conversions, preflight and acquisition, import selection and +evidence, filesystem contexts, inheritance, projection, schema construction, +changed tests, and documentation. The development check (`./scripts/check.sh`) +and full race suite (`go test -race ./...`) passed with Go 1.26.10 on Linux. + +## 1. Complete The Non-Import Resolved-Context Handoff + +Status: open. Priority: medium. Category: implementation gap and maintainability. +No user-facing correctness regression has been established for this finding. + +**Target.** Roadmap internal target 2 and implementation Stages 7–9 select one +resolved filesystem description shared by validation/acquisition and passive +context construction. Stage 8 explicitly requires populating the description +at the existing non-import validation point and resolving only facts that are +still missing later. + +**Evidence.** In [acquisition.go](../../internal/engine/acquisition.go), +`acquireInput` calls `validateFilesystemContext` at line 181 for inputs without +import traversal and with an applicable explicit context. That validator returns +only a diagnostic. Its host/native branches derive a resolved description but +discard it. At lines 220–226, acquisition sees `input.resolvedFilesystem == nil` +and calls `resolveFilesystemContext` again before building passive views. + +For example, an ordinary `Load` of a byte or mapping source with explicit host +`FilesystemContext` and imports disabled resolves/canonicalizes the base and +root during validation, then derives those same facts again for its source +context. The retained description therefore does not complete the intended +validation-to-context handoff. + +Additionally, [filesystem.go](../../internal/engine/filesystem.go) still contains +`validateAbsentHostFilesystemContext` (line 40), with its own root/base +canonicalization, root-defaulting, and containment logic. That derivation +duplicates `resolveAbsentFilesystemContext` in +[filesystem_context.go](../../internal/engine/filesystem_context.go) (line 29). +The two paths already maintain separate containment-diagnostic messages. Graph +caller inspection confirms that the old validator is reachable from +`acquireInput`; it is not unused migration scaffolding. + +**Why address it.** The new model exists, but one important path still discards +and reconstructs its results, while absent-host context derivation has two +production implementations. This leaves part of audit finding 5 unresolved and +preserves the drift risk the consolidation was intended to remove. This is a +structural completion issue, not a claim of measured performance loss or a new +single-lookup guarantee. + +**Required revision.** + +- Have non-import context resolution/validation return or populate the resolved + description, and retain it on `assembledInput` at the existing validation + point. Later passive-view construction and ordinary-overlay handling should + reuse those applicable facts. +- Route absent-host derivation through the common resolved-context owner and + remove the duplicate derivation from the old validator. Keep the existing + directory/access checks and close-error handling at their appropriate points. +- Preserve deferred discovery/ordinary-overlay checks, root-only absent host + context, host/native distinctions, diagnostics, and borrowed filesystem + ownership. Independently required validation must not disappear merely to + reduce filesystem calls. +- Keep internal documentation aligned with the completed handoff. Describe + aggregate accounting as load-wide and repetition/namespace state as + tier-scoped; the new internal overview/import prose currently calls the latter + per-tier accounting despite its distinct role. + +**Verification.** Review the non-import call path to confirm that applicable +resolved facts are carried forward and absent-host derivation has one owner. +Reuse the existing host/native, discovery selected/absent, ordinary/layered +overlay, and passive-resource-context tests. Add a focused behavioral regression +only for a consequential gap exposed by the change; do not assert incidental +canonicalization/stat call counts. Run the development check and applicable +ownership race checks after remediation.