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

Consolidate repository audit findings for review

Browse Source
This commit is contained in:
Eric Rakestraw
2026-10-25 20:07:46 +00:00
parent ecd5d5d460
commit aab79d9508
11 changed files with 930 additions and 1957 deletions
Download Patch File Download Diff File Expand all files Collapse all files

926
docs/roadmap/audit.md Normal file
View File

@@ -0,0 +1,926 @@
# Consolidated Audit Review
Status: review pending; implementation scope has not been selected.
Consolidated: 2026-10-25. Baseline: `ecd5d5d`.
This document consolidates nine sequential audits for maintainer review. It owns
their open findings, supporting evidence, proposed remedies, and relationships.
It is not an implementation plan, an acceptance of every recommendation, or a
new set of release gates. The [configuration contract](../config.md), public Go
declarations, and [architecture policy](../policy/architecture.md) continue to
own behavior and guarantees.
There are nine distinct reported defects: four from correctness, three from
resources, one from error handling, and one from security. Concurrency adds
evidence for an existing callback-lifetime defect. Other entries are cleanup,
efficiency, maintainability, or test improvements. Regression recommendations
are attached to the defects they protect rather than counted as additional
product fixes.
The 35 original numbered findings map to 31 review entries after deduplication
and integration of related test work. All entries remain open and unselected.
Their medium/low priorities retain the original assessments; those labels do not
imply severity certification or implementation order. IDs remain stable as work
is selected or completed.
## Evidence And Provenance
All nine audits were dated 2026-10-25. Their sequential commits changed only the
audit documents, so their production baseline is the same. This consolidation
does not independently reproduce the findings or claim new runtime evidence.
| Original report | Reviewed revision | Evidence contributed |
| --- | --- | --- |
| `correctness.md` | `47944d2` | Source review and standalone public-API reproductions of four defects. |
| `efficiency.md` | `5098b56` | Source-derived scaling cases and proposed algorithm changes; no benchmarks. |
| `complexity.md` | `320d0c5` | Responsibility/caller review, graph complexity estimates, and maintainability candidates. |
| `concurrency.md` | `a802734` | Ownership review, race checks, and corroboration of callback lifetime after panic. |
| `resources.md` | `0cca633` | Bounded allocation, retained-node, and host-handle diagnostics. |
| `error-handling.md` | `5925066` | Directory read/close fault injection through the public API. |
| `security.md` | `b30c9f1` | Trust-boundary review, bounded symlink operation counts, and a historical dependency spot-check. |
| `testing.md` | `eb5a82f` | Aggregate statement coverage, assertion review, and regression/test-cleanup recommendations. |
| `deadcode.md` | `b1c278b` | Type-aware reference inventory, graph/caller checks, and source review of private cleanup candidates. |
The full original reports, including exact probes, commands, source line
references, and validation limitations, are preserved at `ecd5d5d`. For example:
```sh
git show ecd5d5d:docs/roadmap/correctness.md
git show ecd5d5d:docs/roadmap/security.md
```
The [original-finding map](#original-finding-map) accounts for every numbered
finding, including duplicates and test recommendations integrated elsewhere.
Source links below identify implementation owners; historical line numbers are
left in the originals because later edits will move them.
### Recorded Validation And Its Limits
The audits report successful development checks, ordinary tests, full race tests,
`go vet`, and focused suites as specified in their individual records, using
Go 1.26.10 on Linux amd64. Concurrency additionally repeated six relevant engine
tests with race instrumentation, count 20, and CPUs 1, 4, and 8. Testing ran the
suite with shuffle enabled three times. These results did not detect the newly
reported defects and are not evidence that those defects have been fixed.
Testing recorded 771 ordinary test functions, three fuzz targets, and 29 executable
examples. Aggregate cross-package coverage was 90.3%: root 81.3%, engine 91.0%,
document 100%, YAML 89.9%, and maintained consumers 79.2%. Those figures locate
gaps; they are not targets or a measure of assertion strength. Only checked-in
fuzz seeds ran, not an extended campaign or mutation analysis.
Standalone diagnostics stayed outside the repository. Some potentially fatal
cases used disposable processes, small limits, or operation ceilings; larger
scaling examples were derived rather than executed. The dead-code inventory
considered Linux, Darwin, and Windows build configurations, but no audit reports
native macOS or Windows execution. Graph scores and reachability results were
triage aids, not proof that a function is unnecessary or a numerical refactoring
threshold. Cached analysis tooling was used outside the repository; no new
project tools or dependencies were introduced.
The security report's module-path advisory spot-check and successful module
integrity verification are historical evidence only. It recorded no listed
advisories for the selected YAML v4, ReleaseKit, or Goldmark v2 module paths, and
recorded the selected `x/mod` version as fixed for the two reports it found. It
did not perform complete toolchain/transitive reachability analysis or supply-
chain attestation; dedicated vulnerability, security, and secret scanners were
unavailable. This consolidation neither refreshes nor broadens that conclusion.
## Review Priorities And Dependencies
Recommended review order is reported defects with their regression coverage,
bounded cleanup, targeted efficiency changes, and then broader structural
refactoring. Small prerequisite cleanups can accompany a defect fix. The order
of the groups below is for navigation, not an execution sequence.
| Relationship | Consequence for scope selection |
| --- | --- |
| A02 and A05 | Make descriptor construction safe before reusing successful dynamic-type eligibility results. |
| B01 and B02 | Panic cleanup must close callback state and the owned host root independently. Neither fix substitutes for the other. |
| C01 and its filesystem tests | Strengthen error/handle assertions while adding failure-phase regressions, using one shared test owner. |
| C02 and C03 | Link expansion correctness is independent of optional glob readability extraction. Do not make the latter a prerequisite. |
| D01, D02, and D03 | Remove confirmed unused copies where selected, then account for the complete retained graph. Removing one unused tree does not fix the remaining omissions. |
| E01, E02, and E03 | Consider policy-state costs and responsibility boundaries together, but preserve separate scaling and behavioral acceptance cases. |
| F01 and F02 | Removing the test-only acquisition mode does not by itself simplify active lifecycle handoffs. |
| F04, F09, and F12 | Replace useful facade ownership/query coverage before deleting disconnected or representation-coupled scaffolding. |
| F05 and F07 | Removing unused placement bridges is a bounded cleanup independent of the wider transport refactor. |
## A. Schema And Typed Values
### A01. Terminate Pointer-Only Recursive Descriptor Traversal
Type: reported defect. Priority: medium. Source: correctness 1; testing 1.
**Problem and evidence.** In [schema.go](../../internal/engine/schema.go), a legal
type such as `type Loop *Loop` produces a pointer descriptor whose element is
itself. [decode.go](../../internal/engine/decode.go) unwraps that descriptor
without consuming a non-null input node; `resolveSchemaPath` likewise assumes
pointer traversal eventually reaches another kind. A public load with a `Loop`
field and `value: {}` overflowed the stack in a process with a small stack limit.
`value: null` succeeded, but resolving a normalization path for that field hung
until the diagnostic process was stopped. The [schema
contract](../config.md#application-schema-binding) permits recursive types with
acyclic values; the inline-field restriction is not a blanket pointer restriction.
**Recommendation and acceptance.** Detect repeated descriptors while unwrapping
pointer-only chains in decoding and schema-path resolution, and review analogous
helpers. Incompatible non-null input must fail safely; path traversal must
terminate and reject unaddressable descendants. Preserve supported nil/null
values and ordinary recursive structs. Do not add a blanket schema rejection or
arbitrary depth cap. Add bounded regression coverage for both decoding and helper
path resolution, keeping fatal/hanging probes out of the default test process.
### A02. Make Failed Dynamic Descriptor Construction Transactional
Type: reported defect. Priority: medium. Source: correctness 2; testing 1.
**Problem and evidence.** `dynamicDescriptor` in
[value_operation.go](../../internal/engine/value_operation.go) passes the load's
descriptor cache to `schemaBuilder`. Failed construction leaves cached container
placeholders. For a dynamic struct containing an unsupported channel field, two
`Normalize` calls in one resolver, first with a typed nil pointer and then a
non-nil pointer to that type, reproduced a nil-pointer panic on the second call.
The first call reported a diagnostic but left a reusable partial descriptor.
The application callback itself did not panic.
**Recommendation and acceptance.** Stage new descriptors and commit only on
successful construction, or roll back all additions from a failed build. Include
recursive references among newly built descriptors; clearing only the immediate
failed container or adding a projection nil guard is insufficient. Preserve
previously valid descriptors, load-local ownership, and safe path-specific
diagnostics under [typed projection](../config.md#typed-projection-and-ownership)
and [resolution](../config.md#recorded-application-resolution). Test repeated
operations after a failed build in the same load. Coordinate A05 only after this
failure-state boundary is sound.
### A03. Distinguish Acyclic Slice Views In Cycle Detection
Type: reported defect. Priority: medium. Source: correctness 3; testing 1.
**Problem and evidence.** `valueVisit` / `visitFor` in
[value_operation.go](../../internal/engine/value_operation.go) identify slices by
type, kind, and starting pointer, omitting length. The finite value built by
`shared := []any{"leaf", nil}; shared[1] = shared[:1]` was rejected through mapping
sources, typed defaults, and CLI overrides, while an equivalent independently
allocated value succeeded. The nested view contains only the leaf, not its
enclosing view. The same helper serves projection/cloning and
[value.go](../../internal/engine/value.go).
**Recommendation and acceptance.** Include visible slice length in active-visit
identity, or use an equivalent distinction. Preserve rejection of actual self
and mutual cycles, nil/empty behavior, and detached output; preserving alias
identity is not required. Regressions should cover the shared source, override,
projection, and cloning paths under the existing supported-acyclic-value contract,
alongside true-cycle rejection.
### A04. Enforce Existing Node Budgets Before Container Reservations
Type: reported defect. Priority: medium. Source: resources 1; testing 1.
**Problem and evidence.** `projectSequence` and `projectMap` in
[projection.go](../../internal/engine/projection.go) reserve space for the full
container, and maps collect all keys, before child charging in
[value_operation.go](../../internal/engine/value_operation.go). Defaults containing
a maximum-length slice of zero-sized structs caused a `makeslice: cap out of range`
panic instead of `schema.projection_limit`; that diagnostic did not allocate a
huge input backing array. Less extreme cases can allocate excessive temporary
memory before rejection. Application resolution can reach these paths too.
**Recommendation and acceptance.** Check the known minimum node cost against the
remaining existing allowance with overflow-safe arithmetic, or bound initial
capacity and grow as charging succeeds. Review map reservation/key collection as
well as sequences. Preserve node/depth semantics, inline handling, and useful
diagnostic paths. Use bounded regressions, including small controlled allowances,
without enormous allocations. This enforces an existing budget, not a new length
cap or universal heap-size guarantee.
### A05. Reuse Successful Dynamic-Type Eligibility Checks Locally
Type: optimization candidate. Priority: low. Source: efficiency 6.
**Problem and evidence.** Each dynamic value in projection/cloning invokes
`descriptorContainsSecret` with a new visited map, even when its descriptor is
cached. N values of one concrete type with descriptor-graph size S therefore cost
O(N × S) eligibility work. N interface values containing nil pointers to one wide
supported struct can produce only N null values while repeating the full type
walk. This is a source-derived bound, not measured runtime.
**Recommendation and acceptance.** After A02, consider remembering successfully
approved concrete descriptors within `valueOperation`, reducing repeated-type
eligibility work to O(S + N). Cache only completed successful eligibility checks,
not traversal-local visits, partial descriptors, or path-bearing failures.
Preserve rejection of dynamic secret-reference types, including nil pointers and
recursive descriptor cases. This does not select cross-load schema caching or a
clone traversal redesign.
## B. Callback And Filesystem Lifetime
### B01. Deactivate Callback Contexts During Panic Unwinding
Type: reported defect. Priority: low. Sources: correctness 4, concurrency 1;
testing 1; corroborated by error-handling and security reviews.
**Problem and evidence.** `runResolution` in
[resolution.go](../../internal/engine/resolution.go) and `runValidation` /
`reportVersion` in [validation.go](../../internal/engine/validation.go) deactivate
contexts only after normal callback return. Public probes retained the context,
panicked, recovered outside `Load`, and then successfully queried resolution or
wrote validation/version diagnostics. The concurrency audit reproduced this
with race instrumentation without reporting a data race. The defect violates
callback lifetime, not a newly supported concurrent-use scenario.
**Recommendation and acceptance.** Defer deactivation within each callback
invocation, closing immediately at normal return and on unwinding, before later
projection or metadata work. Preserve the original panic and prohibit retained
context use through the established closed-context behavior. Test all three
callback kinds with external recovery. No recovery service, lock, atomic flag,
or broader callback thread-safety guarantee is needed. B02 separately owns root
handle cleanup.
### B02. Close The Owned Import Root When A Version Callback Panics
Type: reported defect. Priority: medium. Source: resources 3; testing 1.
**Problem and evidence.** Host import acquisition in
[import_acquisition.go](../../internal/engine/import_acquisition.go) opens an
`os.Root` through [filesystem_access.go](../../internal/engine/filesystem_access.go)
but relies on explicit ordinary-path closes. An imported version-check panic
bypasses them. A disposable Linux probe disabled GC, recovered outside `Load`,
and observed one remaining root descriptor through `/proc/self/fd`; the normal
control returned to zero. The imported file's own deferred close still ran.
**Recommendation and acceptance.** Install deferred fallback cleanup at the
owning boundary, preserving explicit normal close timing and diagnostics, as
selected-overlay acquisition already does. Avoid double-close confusion, leave
borrowed filesystems untouched, and propagate the original panic. A regression
must observe the owned host root, not merely a native file-close counter, and
document platform prerequisites. Context deactivation in B01 is a separate
requirement. This restores load-scope cleanup without adding panic recovery.
## C. Import Selection And Link Resolution
### C01. Preserve Directory Failure Phase And Strengthen Its Tests
Type: reported defect with test improvements. Priority: medium.
Sources: error-handling 1, testing 3.
**Problem and evidence.** `filesystemAccess.readDirectory` in
[filesystem_access.go](../../internal/engine/filesystem_access.go) returns open,
read, and close errors through one unqualified error channel.
[import_selection.go](../../internal/engine/import_selection.go) and
[glob.go](../../internal/engine/glob.go) treat any `fs.ErrNotExist` as absence.
A borrowed filesystem probe selected a failing directory plus a good directory,
using both `dir` and `glob` with `required: true`. Injecting `fs.ErrNotExist`
alongside entries from `ReadDir`, or from `Close`, let loads succeed with the bad
directory's contribution omitted. Permission-classified controls failed. All
eight selector/phase/error cases ran. This violates the existing distinction
between optional absence and failed acquisition.
**Recommendation and acceptance.** Preserve missing-on-open versus post-open
failure, for example with a small private failure record analogous to regular
file acquisition. Keep genuine missing-target rules, entry budgets, first-error
precedence, borrowed filesystem ownership, and safe existing traversal diagnostics.
Review both batched `fs.ReadDirFile` and standard fallback paths. A successful
second directory must not hide enumeration/close failure; raw backing error text
must not leak. No retries or filesystem transaction are proposed.
**Test improvements.** The shared fixture in
[native_filesystem_test.go](../../internal/engine/native_filesystem_test.go) and
[resource_context_test.go](../../internal/engine/resource_context_test.go) currently
checks aggregate closes and often only non-nil errors. One leaked handle and
one double-close can cancel out. Its canary check examines the terse error
summary, not structured diagnostics. Track per-handle closure, assert nil result,
diagnostic code and source, and inspect diagnostic messages/rendering for canaries.
Add entries/data-plus-error and close-phase cases through narrow failure seams.
Preserve intentionally ignored read-only regular-file close errors; this does
not impose a universal close-error policy.
### C02. Avoid Reexpanding Completed Finite Symlink Dependencies
Type: reported availability defect. Priority: medium. Source: security 1;
testing 1.
**Problem and evidence.** Host/native resolvers in
[filesystem_links.go](../../internal/engine/filesystem_links.go) track active links
but reuse no completed resolution. A finite contained graph `s00 -> .`,
`s01 -> s00/s00`, `s02 -> s01/s01`, and so on repeats completed dependencies.
Its work follows T(N) = 2T(N-1) + O(1), despite linear graph size. Directory
selection resolves discovered links before deciding on recursive descent, so
`recursive: false` does not avoid it. Existing source/entry/node/import-depth
budgets do not account for the repeated expansion.
A real-link fixture exposed through a native `fs.ReadLinkFS` wrapper recorded
32, 128, 512, 2,048, and 8,192 `Lstat` calls for 3, 5, 7, 9, and 11 entries;
corresponding `ReadLink` counts were 11, 57, 247, 1,013, and 4,083. Thirteen
entries reached a diagnostic ceiling of 12,000 `Lstat` calls. With following
disabled, that fixture used 17 `Lstat` calls and no `ReadLink`. The host resolver's
matching logic was reviewed, not separately operation-count tested.
**Recommendation and acceptance.** Reuse fully resolved targets within the
resolution operation, keyed for the rooted namespace and full link path, or use
equivalent dependency evaluation. Retain active-path cycle detection, containment,
finite reuse, relative/absolute host semantics, and the stable-input assumption.
A global visited-means-cycle rule would incorrectly reject valid inputs. Cover
both resolvers and directory/glob entry points with small fixtures and bounded
operation observation, not timing thresholds or exhaustion tests. No arbitrary
link cap, public cache, or default-follow change is selected.
The availability risk requires influence over imported filesystem data; it is
not an anonymous remote vulnerability by itself. No concurrent mutation or
outside-root target is needed, and accidental graphs can cause the same work.
### C03. Separate Glob Entry Preparation From Traversal
Type: optional refactor. Priority: medium. Source: complexity 4.
**Problem and evidence.** The patterned branch of `selectGlobImportCandidates`
in [glob.go](../../internal/engine/glob.go) interleaves component traversal and
union/sorting with hidden/match rules, lexical/access paths, containment, links,
directory eligibility, namespace differences, labels, and candidate construction.
Directory selection already has a named entry-preparation operation.
**Recommendation and acceptance.** Consider a glob-specific preparation helper
returning an admitted candidate, ordinary skip, or diagnostic. Keep literal
segments, component advancement, union/sorting, exclusions, and requiredness in
the selector. Preserve inspection/error order, budgets, native spelling, and
selected-file validation. Share low-level facts only where semantics agree;
no general visitor framework, discovery-algorithm change, or caching is implied.
C01/C02 can be fixed independently of this readability change.
## D. Retained History And Unused Evidence
### D01. Account For The Complete Published History Graph
Type: reported defect. Priority: medium. Source: resources 2; testing 1.
**Problem and evidence.** The checkpoint in [load.go](../../internal/engine/load.go)
and [history.go](../../internal/engine/history.go) counts a different root set
from [load_result.go](../../internal/engine/load_result.go) publication. Separately
cloned post-CLI input, `structuralInput`, `preCLI`, and later inheritance copies
are not all represented. A read-only diagnostic of a small successful load with
a no-op resolver counted 14 distinct nodes before including the two published
replay roots and 22 with them. Source inspection found that the checkpoint uses
a different, same-sized decoded-final instance and also counts 14 in this case.
Generalized to N scalar items, the reported counts were 3N + 8 versus 5N + 12;
800,000 items would pass the checkpoint yet exceed the existing 4,000,000-node
history allowance. That large case was derived, not allocated or executed.
**Recommendation and acceptance.** Use pointer-aware accounting for every document
root actually retained at publication, including replay and copied inheritance
history, counting shared nodes once. Where practical, reserve/charge copies before
allocation. Coordinate removal of unused trees under D03 and recalculate the
retained set afterward; that removal alone does not establish complete accounting.
Reuse already-owned immutable snapshots only where equivalence and ownership
are demonstrated. This need not redesign public metadata copying.
Use a small controlled allowance to test publication integration and the existing
`explain.limit_exceeded` outcome, as well as count-once sharing. Helper-only tests
cannot detect roots omitted by callers. Preserve [historical
inspection](../config.md#historical-value-explanation), detached ownership, and
the existing quota; no smaller limit or universal heap-size bound is proposed.
### D02. Remove The Unconsumed Override-Result Pre-CLI Copy
Type: cleanup. Priority: medium. Source: deadcode 1.
**Problem and evidence.** `decodedOverrideResult.preCLI` in
[override_decode.go](../../internal/engine/override_decode.go) clones the entire
decoded final tree even without CLI assignments. Its readers are private tests;
production independently clones `before.final` into the active context in
[override_context.go](../../internal/engine/override_context.go).
**Recommendation and acceptance.** Remove that result member, its clone, and
assertions solely about it. Preserve the independently owned pre-CLI context
that actually feeds resolution, publication, and explanation. Retain behavioral
CLI/history/ownership coverage. The change removes a redundant traversal, not
the active replay snapshot or its detachment boundary.
### D03. Remove The Unqueried Published Structural-Input Copy
Type: cleanup. Priority: medium. Source: deadcode 2.
**Problem and evidence.** `Inspection.structuralInput` in
[load.go](../../internal/engine/load.go) is cloned and indexed at publication,
but has no production reader. [explain.go](../../internal/engine/explain.go)
uses source/inheritance replay, `preCLI`, and `input`. The immutable lookup is
addressed by supplied node pointers; indexing this otherwise unused root does
not make it useful and keeps its mappings alive.
**Recommendation and acceptance.** Remove the inspection member, final clone,
and its `retainedTrees` inclusion. Keep earlier `resolved.structuralInput` uses
for path finalization and sensitivity. Check explanation, historical paths,
redaction, and ownership through active outputs. Coordinate D01 for all remaining
retained roots; do not infer that structural evidence is globally unused.
### D04. Stop Constructing Unconsumed Combination Evidence
Type: cleanup. Priority: medium. Source: deadcode 5.
**Problem and evidence.** `sequenceCombinationResult.discardedLower` in
[sequence_combine.go](../../internal/engine/sequence_combine.go) allocates a list
of all replaced lower indexes with no production reader. Three tests inspect it.
`inheritanceReplacement.path`, `previous`, and `effective` in
[inheritance.go](../../internal/engine/inheritance.go) are also unread, but their
writers perform path/origin copies and displaced-subtree walks. Active inheritance
history uses operations and projected changes elsewhere. Replacement `before`
and `after` trees still feed node accounting.
**Recommendation and acceptance.** Remove only unread members and work solely
feeding them. Keep replacement trees, item projections, replacement mappings,
`appendedHigher`, active provenance/history, and node accounting. Keep
`sequenceIndexes` for append and `displacedOrigins` for overlay callers. Replace
private unused-evidence assertions with existing behavior-level replacement,
removed-descendant, and history protection where necessary. Coordinate D01 and
E03 without deleting active evidence merely because another representation exists.
## E. Collection Processing And Efficiency
All complexity estimates here describe identified internal work with bounded-
depth examples and expected hash lookup costs. Actual path processing, copied
payload, and expanded output remain costs; no end-to-end complexity or timing
guarantee is proposed.
### E01. Avoid Scanning Every Policy For Each Replacement
Type: optimization candidate. Priority: medium. Source: efficiency 1.
**Problem and evidence.** `retireCollectionPoliciesAtOrBelow` in
[collection_policy_state.go](../../internal/engine/collection_policy_state.go)
scans all P policies for each of Q ordinary replacements in
[overlay.go](../../internal/engine/overlay.go), even if the changed scalars are
unrelated to every collection. P = Q = N gives O(N²) visits for linear-sized
input and output within one tier.
**Recommendation and acceptance.** Consider an operation-local semantic-path
index with descendant queries, maintained as policies change, or batch retirement
where timing remains equivalent. The disjoint case can approach O(P + Q), with
additional work charged to affected entries/path lengths. Preserve explicit-mode
persistence, record ownership, and the moment old identity retires; delayed
retirement must not clear newly established higher-tier policy. Evaluate existing
inheritance indexing without requiring identical interfaces. Retain a distinct
disjoint-replacement case even if implementation is coordinated with E02/E03.
### E02. Avoid Recopying Unrelated Policy Across Small Tiers
Type: optimization candidate. Priority: medium. Source: efficiency 2.
**Problem and evidence.** `lowerCollectionPoliciesForClaims` in
[collection_retirement.go](../../internal/engine/collection_retirement.go) clones
and scans lower policy, and `resolveStrictCollectionPolicies` in
[compose.go](../../internal/engine/compose.go) clones it again. Overlay preparation
repeats full-state work and checks retained destinations. P initial policies
followed by T scalar-only tiers cost O(T × P) work/allocation although only O(P + T)
material exists. Copies include record order and nested record policy; these
are transient private copies, not independently published snapshots.
**Recommendation and acceptance.** Separate read-only lower-policy lookup from
owned current-tier changes, localize work to incoming claims/declarations, and
apply changes to the assembly owner. Keep copies at actual mutable ownership
boundaries. Removing the immediate duplicate copy helps but does not alone fix
O(T × P) growth. Preserve pending declarations, replacement eligibility, and
independent nested record policies. This case remains distinct from E01 and the
deferred public metadata-copy experiment.
### E03. Clarify Policy Preparation And Merge Evidence Application
Type: optional refactor. Priority: medium. Source: complexity 3.
**Problem and evidence.** [overlay.go](../../internal/engine/overlay.go) combines
claim/declaration preparation, operation resolution, callbacks, diagnostics,
origins/history, retirement, record policy rebuilding, and declaration-only
finalization. Overlay and [inheritance](../../internal/engine/inheritance_collection.go)
separately translate sequence results into history descriptors and reconstruct
record order. `combineSequence` already owns shared value combination.
**Recommendation and acceptance.** Consider explicit preparation, application,
and finalization operations with a small result/owner for evidence and diagnostics.
Share straightforward history-descriptor construction; assess projected-record
selection sharing while keeping precedence and inheritance policy ownership
explicit. Do not unify distinct origin or mode-lifetime rules behind a generic
strategy framework. Coordinate E01/E02 so extraction does not preserve avoidable
full-table work. Preserve pending declarations, replacement ordering, retained
versus incoming record owners, and stable history. D04 can remove unused evidence
without requiring this broader refactor.
### E04. Index Tier-History Projection Lookups
Type: optimization candidate. Priority: medium. Source: efficiency 3.
**Problem and evidence.** `projectTierHistoryApplications` in
[sequence_history_projection.go](../../internal/engine/sequence_history_projection.go)
calls unindexed `documentAtPath` for each mapped sequence. N sibling collections
selected by one wildcard identity registration incur O(N²) mapping comparisons
while projecting linear-sized source/history material.
**Recommendation and acceptance.** Lazily prepare one read-only `documentLookup`
per original application value with mapped contributions, or collect needed nodes
in one traversal. With V nodes, S contributions, depth D, and I projected items,
that portion can be O(V + S × D + I). Preserve authored coordinates, sparse item
projections, replacement evidence, and independent output storage. This is load-
time coordinate projection, not branch-local explanation replay.
### E05. Restore Compound Identity Without Repeated Mapping Scans
Type: optimization candidate. Priority: medium. Source: efficiency 4.
**Problem and evidence.** `preserveInheritanceIdentityFields` in
[inheritance_scope.go](../../internal/engine/inheritance_scope.go) searches child
and merged mappings for each identity field. K fields and C/M child/merged
entries cost O(K × (C + M)) comparisons plus required clones. A wide compound
identity can make this quadratic without expanding record output.
**Recommendation and acceptance.** Index merged positions and walk child entries
against an identity-field set, or use equivalent local mapping indexes, for
O(K + C + M) lookup work plus copies. Preserve child-owned identity values and
locations, mapping order, and detached nodes. Do not change identity equality or
parent selection. Removing the restoration step instead requires a separate
equivalence review; apparent duplication alone is insufficient.
## F. Remaining Maintenance And Test Improvements
### F01. Remove The Test-Only Eager Acquisition Mode
Type: cleanup. Priority: medium. Sources: complexity 1, deadcode 8.
**Problem and evidence.** Both production constructors of the request declared in
[acquisition.go](../../internal/engine/acquisition.go) set
`inputAcquisitionRequest.deferAssembly` to true. Only private
test helpers select the eager composition branch in
[import_acquisition.go](../../internal/engine/import_acquisition.go). Acquisition
maintains two return protocols and tests can bypass the production phase handoff.
**Recommendation and acceptance.** Return retained scopes consistently and remove
the unused production mode. Tests needing composition should invoke the active
composition phase through a small test-only helper; acquisition-only tests should
remain acquisition-only. Preserve closure, pending-item reservation, accounting,
diagnostics, and meaningful composition coverage. This does not retire public
loading workflows or imply that every nil-state/directive context is unused.
### F02. Clarify Active Acquisition And Coordinator Handoffs
Type: optional refactor. Priority: medium. Source: complexity 2.
**Problem and evidence.** `acquireInput` in
[acquisition.go](../../internal/engine/acquisition.go) combines discovery, parsing,
directive/version branches, placement/accounting, dispatch, and passive contexts.
Version branches repeat source-ID/failure/result handling, and layered processing
extracts and restores directives. [load.go](../../internal/engine/load.go) contains
a substantial ordinary-load context fallback; [load_request.go](../../internal/engine/load_request.go)
repeats tier declaration/application/projection handoffs. Parsed, local, composed,
inherited, and version-path representations are difficult to follow together.
**Recommendation and acceptance.** Consider cohesive parsed-input directive/version
processing and standalone placement/registration operations, plus explicit context,
structural-expansion, and repeated tier handoffs. Reuse existing state/results;
helpers that merely share the same mutable surrounding locals do not resolve
ownership. Preserve visible phase order, ordinary/layered overlay differences,
default/enabled version timing, warnings, exactly-once callbacks, and load-wide
versus tier-local state. F01 removes a different, inactive protocol; neither task
requires a new context hierarchy or public request model.
### F03. Retire The Parallel Test-Only Import Directive Parser
Type: cleanup. Priority: low. Source: deadcode 7.
**Problem and evidence.** `parseImportSpecificationDirective` in
[import_spec.go](../../internal/engine/import_spec.go) is called only by the
`parseImportSpecsYAML` test fixture. It duplicates mapping/field dispatch already
owned by `parseRootDirectiveBlock` in [directives.go](../../internal/engine/directives.go).
**Recommendation and acceptance.** Use the active parser with the appropriate
context, or parse the declaration list directly when that is the tested boundary,
then remove the obsolete implementation. Preserve malformed-declaration and
selector coverage. Keep the active imports-only context used in layered version
processing; thin test helpers delegating to production are not inherently dead.
### F04. Remove The Disconnected Root Record-Clone Family
Type: cleanup. Priority: low. Source: deadcode 3; related testing 6.
**Problem and evidence.** The private family in root [records.go](../../records.go)
has no production facade entry: `cloneLocation`, `cloneLocations`, `cloneSourceInfo`,
`cloneImportSelections`, `cloneSourceVersionCheck`, `cloneSourceInfos`, `cloneOrigin`,
`cloneFieldInfo`, `cloneChange`, `cloneDiagnostic`, and `cloneDiagnostics`.
Helpers refer to each other; only tests call `cloneSourceInfo`. Active root-owned
records come through [conversion.go](../../conversion.go).
**Recommendation and acceptance.** Delete the disconnected family and tests solely
protecting it. Keep conversion assertions and useful returned-record ownership
coverage through repeated public queries and mutation of mutable nested results.
Replacing an entire `Destination` field on a copied struct does not demonstrate
nested detachment. Keep active engine clones and `Path.clone`; do not expose
opaque path internals or invent a stronger copy guarantee. Coordinate F12.
### F05. Remove Uncalled Engine Helpers
Type: cleanup. Priority: low. Source: deadcode 4; placement subset also complexity 5.
The type-aware audit found no callers or function-value uses in the inspected
production/test build configurations for the following helpers:
| Candidate | Active counterpart to preserve |
| --- | --- |
| `mergeDisjointMappings` in [compose.go](../../internal/engine/compose.go) | `mergeDisjointMappingsAtPath` and shared recursive merging; the old helper references only itself. |
| `mergeHistoryDocument` in [history.go](../../internal/engine/history.go) | `mergeHistoryDocumentAtPath` with sequence evidence. |
| `inheritanceDirectiveLocation` in [inheritance.go](../../internal/engine/inheritance.go) | Preparation's directive entry/location handling. |
| `addSecretEvidencePath` in [secret.go](../../internal/engine/secret.go) | Projected version sensitivity evidence and the active path accumulator. |
| `PlacementModeFromFacade`, `PlacementModeForFacade` in [destination.go](../../internal/engine/destination.go) | Input/options conversion and `SourceInfoPlacementModeForFacade`. |
**Recommendation and acceptance.** Remove these definitions and imports/comments
solely supporting them after confirming references at implementation time. Preserve
active counterparts and public root APIs. The placement pair can be removed
without selecting the broader transport work in F07.
### F06. Trim Unread Private Result And Transport Members
Type: cleanup. Priority: low. Source: deadcode 6.
The audit identified the following populated members without production readers.
Some private tests inspect them. Remove only these slots and computations solely
feeding them, preserving validation and active outputs:
| Owner and candidate members | Boundary to preserve |
| --- | --- |
| `collectionPolicyResolution.declarations`, `declarationsPresent`, `identityDeclarationPresent`, `applicationIdentityFields` in [collection_policy.go](../../internal/engine/collection_policy.go) | Local declaration normalization, `modeDeclarationPresent`, and `applicationIdentityPresent`. |
| `inputPresence.null` in [decode.go](../../internal/engine/decode.go) | Presence/location and effective-node null reporting. |
| `rootDirectiveExtraction.importsPresent`, `sequencesPresent` in [directives.go](../../internal/engine/directives.go) | Parsed declarations, diagnostics, and directive validation. |
| `importSelection.outcome` in [import_selection.go](../../internal/engine/import_selection.go) and glob selection | Candidate/admission behavior and the distinct, active public `importSelectionEvidence.outcome`. |
| `importDeclarationSpec.excludeSet` in [import_spec.go](../../internal/engine/import_spec.go) | Active `exclude` policy. |
| `schemaPath.field`, `dynamic` in [schema.go](../../internal/engine/schema.go) | Descriptor and success result used by path consumers; coordinate A01. |
| `selectedOverlayRequest.filesystem` in [selected_overlay.go](../../internal/engine/selected_overlay.go) | Explicit filesystem-access argument and borrowed ownership. |
| `sequenceDeclarationSpec.destinationLocation` in [sequence_declaration.go](../../internal/engine/sequence_declaration.go) | Other active declaration/field diagnostic locations. |
| `sourceVersionEvidence.absence`, `checked`; `sourceVersionAncestor.kind` in [source_version.go](../../internal/engine/source_version.go) | Actual checks, version/presence/location evidence, ancestor paths, and sensitivity. |
| `postDecodeOverrideContext.sources` in [override_context.go](../../internal/engine/override_context.go) | Local source list used to build callback resolution input. |
**Acceptance.** Preserve public presence, null, selection, version, diagnostic,
and ownership behavior. Rework assertions on inactive annotations only where
they protect a meaningful supported outcome. A lack of selector reads alone is
not a general removal rule: map keys, embedded records, and interfaces require
separate analysis.
### F07. Simplify Redundant Facade-To-Engine Transport
Type: optional refactor. Priority: low. Source: complexity 5, excluding F05's
bounded placement-bridge removal.
**Problem and evidence.** Root conversion, engine facade-transport records, engine
private defaults, decorators, and load normalization provide several mapping
sites for the same import-default fields. Placement likewise uses a record plus
a setter-style bridge. See [conversion.go](../../conversion.go),
[lifecycle.go](../../lifecycle.go), [layered.go](../../layered.go),
[import_spec.go](../../internal/engine/import_spec.go), and
[destination.go](../../internal/engine/destination.go).
**Recommendation and acceptance.** Consider one engine representation for
import-default input and straightforward construction on existing option/input
records, only if this reduces total concepts and mapping sites. Keep domain-rule
and ownership conversions and separate registration compilation. Preserve
root-owned types/signatures, both loading APIs, invalid-enum validation,
nil-versus-empty patterns, and pointer/slice snapshots. No universal request DTO,
generated mapper, public internal aliases, or adapter framework is proposed.
### F08. Index Secret-Failure Location Queries Lazily
Type: optimization candidate. Priority: low. Source: efficiency 5.
**Problem and evidence.** `secretDiagnostic` in
[secret_reference.go](../../internal/engine/secret_reference.go) calls unindexed
nearest-location traversal for each failed reference. N missing references in a
wide mapping incur O(N²) comparisons, even when they use one cached environment
name. Normal deterministic traversal/sorting does not require these scans.
**Recommendation and acceptance.** Consider a lazy read-only `documentLookup`
shared during secret resolution, or carry relevant location state through the
walk. With V nodes, F failures, and depth D, attribution can be O(V + F × D),
apart from sorting. Keep the successful path free of an unnecessary eager index.
Preserve all diagnostics, ancestor fallback, source coordinates, and safe messages;
failing fast would change behavior rather than optimize it.
### F09. Cover Public Historical-Path Enumeration
Type: test improvement. Priority: medium. Source: testing 2; related testing 6.
**Problem and evidence.** Root `Metadata.HistoricalPaths` in
[metadata.go](../../metadata.go) had no executed statements in the aggregate
profile. Engine tests cover it thoroughly, but not root path conversion. An empty
or incorrectly converted public result could leave engine tests green.
**Recommendation and acceptance.** Add one external-package test enumerating
effective and removed paths, using returned paths with `Lookup` and `History`,
including a numeric-key/sequence shape change. Cover sorted unique pointer
spelling, zero metadata, and replacement of a returned slice element without
affecting later queries. Keep detailed path mutation/shape tests in the engine.
This can replace F12's private segment-representation assertion without copying
the complete internal matrix or exposing private segments.
### F10. Strengthen Fuzz Detachment Assertions
Type: test improvement. Priority: low. Source: testing 4.
**Problem and evidence.** `FuzzCompositionAndOverlay` in
[composition_fuzz_test.go](../../internal/engine/composition_fuzz_test.go) appends
to the result root then compares inputs. A shallow root copy sharing existing
descendants can pass because append changes neither the input's slice header nor
elements within its old length. Existing deterministic merge tests already
exercise meaningful nested mutation.
**Recommendation and acceptance.** Mutate an existing result descendant or
in-length element within the bounded fuzz target, then compare both inputs.
Handle empty containers separately and retain input immutability/commutativity
properties. Improve the existing assertion rather than adding another ownership
matrix or claiming all ownership coverage is absent.
### F11. Replace Exact Stat-Call Choreography With Observable Ordering
Type: test cleanup. Priority: low. Source: testing 5.
**Problem and evidence.** `TestSelectNamedImportsChecksNormalizedPathsLexically`
in [import_selection_test.go](../../internal/engine/import_selection_test.go)
requires `a.yaml,a.yaml,z.yaml,z.yaml` stat calls but checks candidate count only.
Lexical handling matters; exactly two stats per file is not an established contract.
**Recommendation and acceptance.** Assert ordered candidate paths and, where
needed, the first diagnostic site using failing targets. Consolidate with existing
selection tests and delete the standalone case if its protection is subsumed.
Keep meaningful observations such as not opening excluded contents. Do not turn
incidental helper call counts into an efficiency or behavior guarantee.
### F12. Consolidate Facade Scaffolding Around Public Behavior
Type: test cleanup. Priority: low. Source: testing 6; related deadcode 3.
**Evidence and recommendations.**
- Remove the duplicate six-type ownership loop in
[facade_compatibility_test.go](../../facade_compatibility_test.go), retaining the
comprehensive owner in [facade_candidate_test.go](../../facade_candidate_test.go),
compile-time interfaces, public construction, and distinct lifecycle checks.
A literal assignment equality assertion on a new `Result.Config` adds no
protection beyond compilation and ordinary Go assignment.
- Replace `TestWrappedPathsRetainEngineSegmentIntent` in
[facade_internal_test.go](../../facade_internal_test.go), which compares private
representations, with public contextual mapping/index behavior under F09.
Keep engine segment-intent coverage.
- Move `TestWrappedSourcesPreserveEngineOwnership` from direct `engine.Load`
through `source.toEngine()` to public `Load` and root error assertions.
Consolidate reader cases with [facade_equivalence_test.go](../../facade_equivalence_test.go)
while preserving explicit copied-reader and byte-source mutation coverage.
- F04 owns removal of the disconnected clone family and the ineffective whole-
destination assignment assertion in [records_test.go](../../records_test.go).
Keep useful conversion and returned-record ownership protection there or in
existing public source-snapshot tests.
**Acceptance.** Remove scaffolding only after preserving its distinct behavioral
risks. Root integration tests still protect conversion, error typing, callbacks,
source identity, disclosure, and ownership; engine coverage does not substitute
for them. Static public/dependency-boundary tests remain useful.
## Boundaries To Preserve
The audits found the architecture coherent and did not establish a need for new
public features, a replacement loading API, package proliferation, or a general
visitor/strategy framework. Root-owned conversions, synchronous load-local state,
shared dependency scheduling, indexed combination, separated inheritance phases,
and passive resource contexts remain useful owners. The clone traversal was
already reviewed and retained at `34cbc0d:docs/roadmap/consolidation.md`; a high
complexity estimate does not reopen it.
No additional supported-use data race, deadlock, background-work leak, credential
disclosure, command injection, authority bypass, or host containment escape was
established. These are limited audit observations, not absence guarantees.
Caller-owned mutable objects, blocking operations, safe callback messages, and
borrowed filesystem behavior retain their documented responsibilities. No new
locking, cancellation, retry, authentication, permission, or environment-variable
restriction follows from these findings.
Preserve raw-error suppression, structured safe diagnostics, original application
panic propagation, intentional bootstrap partial writes, allowed partial writer
output, and documented read-only regular-file close handling. Regression tests
must enforce existing contracts rather than broaden them. Optional absence must
remain distinct from acquisition failure, and genuine finite reuse from cycles.
Public APIs are not dead merely because repository examples omit them. Interface
dispatch, reflection, build constraints, identity tokens, map-key equality, and
uninstantiated generics require more than caller counts. In particular,
`decodeInlineMapEntry` and import-cycle diagnostics remain active.
`Inspection.resolution` retains recorded snapshots under the existing resolution
contract even though replay uses a coalesced view; it is not a cleanup candidate.
Selected stable Kahn ordering, deterministic sorting, exact numeric identity,
and the expressly selected registration-overlap check are not reconsidered here.
Required detached copies and genuinely expanding output remain legitimate costs.
The broader replay, metadata-handoff, cross-load schema-cache, and contributor-
trace experiments stay in [future.md](future.md#3-performance-experiments).
Local fixes and removal of proven unused work do not select those experiments.
## Validation Approach For Selected Work
Use the existing [development guide](../development.md#baseline-validation) and
[testing policy](../policy/testing.md). Add meaningful regressions with the
selected fixes, at the narrowest stable boundary that detects the reported
failure. Extend existing owners rather than duplicating internal matrices at
every layer. Assert nil results and safe structured failures where relevant,
and verify retained state/handles independently when they have distinct owners.
Keep termination, allocation, and link-expansion probes bounded. Use small
controlled budgets, disposable processes where necessary, and filesystem
observation where it detects the specific defect. No wall-clock thresholds,
coverage percentages, complexity-score limits, exact incidental syscall traces,
new resource caps, or extra release gates are selected. Algorithmic evidence is
not a measured speedup. Performance experiments need separate scope and suitable
measurement conditions; no benchmarks were run for this consolidation.
Once selected work changes behavior or structure, update its canonical contract
or internal owner as appropriate. This review document remains the finding/status
owner until an implementation plan is prepared; completed records can retire
to Git history under the normal documentation lifecycle.
## Original-Finding Map
Original files are retrieved from `ecd5d5d:docs/roadmap/<filename>`. Every numbered
finding is mapped below; repeated citations are corroboration or shared coverage,
not extra fixes.
| Original finding | Consolidated owner |
| --- | --- |
| correctness 1 | [A01](#a01-terminate-pointer-only-recursive-descriptor-traversal) |
| correctness 2 | [A02](#a02-make-failed-dynamic-descriptor-construction-transactional) |
| correctness 3 | [A03](#a03-distinguish-acyclic-slice-views-in-cycle-detection) |
| correctness 4 | [B01](#b01-deactivate-callback-contexts-during-panic-unwinding) |
| efficiency 1 | [E01](#e01-avoid-scanning-every-policy-for-each-replacement) |
| efficiency 2 | [E02](#e02-avoid-recopying-unrelated-policy-across-small-tiers) |
| efficiency 3 | [E04](#e04-index-tier-history-projection-lookups) |
| efficiency 4 | [E05](#e05-restore-compound-identity-without-repeated-mapping-scans) |
| efficiency 5 | [F08](#f08-index-secret-failure-location-queries-lazily) |
| efficiency 6 | [A05](#a05-reuse-successful-dynamic-type-eligibility-checks-locally) |
| complexity 1 | [F01](#f01-remove-the-test-only-eager-acquisition-mode) |
| complexity 2 | [F02](#f02-clarify-active-acquisition-and-coordinator-handoffs) |
| complexity 3 | [E03](#e03-clarify-policy-preparation-and-merge-evidence-application) |
| complexity 4 | [C03](#c03-separate-glob-entry-preparation-from-traversal) |
| complexity 5 | [F07](#f07-simplify-redundant-facade-to-engine-transport), with bounded bridge removal in [F05](#f05-remove-uncalled-engine-helpers) |
| concurrency 1 | [B01](#b01-deactivate-callback-contexts-during-panic-unwinding), corroboration |
| resources 1 | [A04](#a04-enforce-existing-node-budgets-before-container-reservations) |
| resources 2 | [D01](#d01-account-for-the-complete-published-history-graph) |
| resources 3 | [B02](#b02-close-the-owned-import-root-when-a-version-callback-panics) |
| error-handling 1 | [C01](#c01-preserve-directory-failure-phase-and-strengthen-its-tests) |
| security 1 | [C02](#c02-avoid-reexpanding-completed-finite-symlink-dependencies) |
| testing 1 | Regression acceptance under A01–A04, B01–B02, C02, and D01 |
| testing 2 | [F09](#f09-cover-public-historical-path-enumeration) |
| testing 3 | [C01](#c01-preserve-directory-failure-phase-and-strengthen-its-tests), including shared fixture assertions |
| testing 4 | [F10](#f10-strengthen-fuzz-detachment-assertions) |
| testing 5 | [F11](#f11-replace-exact-stat-call-choreography-with-observable-ordering) |
| testing 6 | [F12](#f12-consolidate-facade-scaffolding-around-public-behavior), with F04 and F09 |
| deadcode 1 | [D02](#d02-remove-the-unconsumed-override-result-pre-cli-copy) |
| deadcode 2 | [D03](#d03-remove-the-unqueried-published-structural-input-copy) |
| deadcode 3 | [F04](#f04-remove-the-disconnected-root-record-clone-family) |
| deadcode 4 | [F05](#f05-remove-uncalled-engine-helpers) |
| deadcode 5 | [D04](#d04-stop-constructing-unconsumed-combination-evidence) |
| deadcode 6 | [F06](#f06-trim-unread-private-result-and-transport-members) |
| deadcode 7 | [F03](#f03-retire-the-parallel-test-only-import-directive-parser) |
| deadcode 8 | [F01](#f01-remove-the-test-only-eager-acquisition-mode), duplicate of complexity 1 |
## Open Questions
### OQ-1. What Should The First Implementation Scope Include?
**Recommendation:** select all nine reported defects with their regression
coverage, plus the bounded cleanup and test improvements where their evidence
holds on the implementation baseline. The unused history copies deserve early
coordination with D01, and callback context/root closure deserve independent
acceptance within the same work area. Keep fixes reviewable and distinct even
when delivered in one sprint.
**Alternative:** deliver defect fixes first, then handle cleanup and independent
test improvements in a separate effort. This produces a smaller initial scope
and simpler attribution, but retains known redundant work and can revisit the
same files/tests. Cleanup should not delay a consequential fix merely to achieve
a tidier overall structure.
### OQ-2. Which Efficiency Candidates Should Accompany Remediation?
**Recommendation:** consider E04/E05 as narrow local improvements, and A05/F08
as lower-priority options after their correctness/diagnostic boundaries are sound.
Evaluate E01/E02 together with E03 before committing to a policy-state redesign:
their distinct scaling cases need preserved acceptance even if they share an
implementation. These candidates have source-level evidence of avoidable work,
but no new measured speedup claim.
**Alternative:** defer all six efficiency candidates until representative consumer
profiling establishes their practical cost. This concentrates the first effort
on correctness and cleanup, at the cost of retaining demonstrated growth patterns.
It does not block C02's availability fix or A04/D01's enforcement of existing
resource budgets. Broader experiments in `future.md` remain separately deferred.
### OQ-3. How Much Structural Refactoring Should Be Selected?
**Recommendation:** accept F01's removal of the test-only protocol as bounded
cleanup; assess active acquisition handoffs (F02) and policy/evidence ownership
(E03) with their affected fixes before selecting a wider extraction. Leave glob
entry extraction (C03) and transport simplification (F07) optional unless a
concrete design reduces concepts and clarifies ownership.
**Alternative:** include all four structural refactors in the same effort for a
more uniform maintenance result. This offers broader readability improvements
but increases review scope and the chance of moving complexity rather than
removing it. Neither approach calls for a new public API, generic framework, or
changes to the established configuration model.

283
docs/roadmap/complexity.md
View File

@@ -1,283 +0,0 @@
# Maintainability And Complexity Audit
Reviewed revision: `320d0c5f439a059a7081cf7532c9b5d65891f5c7`.
Audit date: 2026-10-25.
## Scope And Method
This audit examines readability, responsibility boundaries, state handoffs, and
interface duplication. It covers the root facade, engine subsystems, document
and YAML boundaries, and relevant tests, documentation, and prior refactoring
decisions. It does not propose new feature behavior or performance guarantees.
The codebase-memory graph was refreshed and its function spans, cyclomatic and
cognitive complexity estimates, parameter counts, and caller traces were used
to select code for inspection. The graph contains 907 non-test functions/methods
and 940 test-file functions/methods. Its index excludes examples, documentation,
and scripts; tracked-file inventory and direct inspection supplemented it.
Standalone `gocyclo`, `gocognit`, and `golangci-lint` were unavailable. No tool
installation, benchmarks, or empirical performance testing was needed.
The tracked production inventory, excluding examples, contains 16 root Go files,
97 engine files, one document file, and two YAML adapter files: approximately
22,612 physical lines overall. The largest production files are `compose.go`
(599 lines), `history.go` (568), `overlay.go` (564), and `discovery.go` (540).
These measurements locate responsibilities; they are not refactoring thresholds.
Representative graph measurements:
| Function | File | Function span | Cognitive estimate |
| --- | --- | ---: | ---: |
| `valueOperation.clone` | `internal/engine/clone.go` | 130 | 104 |
| `acquireInput` | `internal/engine/acquisition.go` | 202 | 99 |
| `valueBuilder.build` | `internal/engine/value.go` | 104 | 93 |
| `decodeValue` | `internal/engine/decode.go` | 115 | 85 |
| `selectGlobImportCandidates` | `internal/engine/glob.go` | 112 | 73 |
| `assembleLayeredInput` | `internal/engine/load_request.go` | 146 | 63 |
| `executeLoadRequest` | `internal/engine/load.go` | 198 | 56 |
| `mergeOverlayMappingsWithCollectionPolicy` | `internal/engine/overlay.go` | 163 | 46 |
Spans include comments and blank lines; scores are tool estimates for comparative
triage, not standardized quality grades. Source review, rather than a numerical
cutoff, determines the findings below.
## Architectural Assessment
The [implemented architecture](../internal/overview.md) remains coherent: the
root owns the supported public types and explicit conversions; the engine owns
loading and frozen inspection; the document package owns the ordered tree; and
the YAML adapter isolates the upstream parser. Ordinary and layered loads share
one normalized request and execution lifecycle. Imports and inheritance retain
their separate scopes and use the common dependency scheduler.
The recent consolidation improved import ownership and file boundaries, separated
inheritance preparation/combination/finalization, shared filesystem facts, and
clarified schema and projection responsibilities. The findings concern remaining
maintenance opportunities, not a recommendation to repeat that refactor or split
the engine into a package per subsystem.
The existing [correctness](correctness.md) and [efficiency](efficiency.md) audits
remain separate owners of their findings. No additional runtime defect was
established in this pass. Related risks are referenced rather than duplicated.
## 1. Remove The Test-Only Eager Acquisition Mode
Status: open. Priority: medium. Scope: engine acquisition and its test helpers.
**Relevant code.** `inputAcquisitionRequest.deferAssembly` in
[acquisition.go](../../internal/engine/acquisition.go), and the final branch of
`acquireImportedWithAccess` in
[import_acquisition.go](../../internal/engine/import_acquisition.go), lines
206–224. Production construction sites in `executeLoadRequest`
([load.go](../../internal/engine/load.go), lines 210–214) and
`acquireUnderlayGroupScopes`
([load_request.go](../../internal/engine/load_request.go), lines 243–246) both
set this flag to true.
**Complexity.** Caller tracing found no production caller selecting eager
composition. The false branch remains exercised by private test entry helpers,
including `testImportRequest`, `testAcquireImport`, and `testAcquireInput` in
[imports_test.go](../../internal/engine/imports_test.go). Acquisition therefore
maintains two return protocols: an acquired scope with a nil composed root, or
an already composed document. The eager branch reconstructs `assembledInput`
and reenters composition from the acquisition function, while the published
loading paths perform that phase later. Tests can consequently exercise a
different handoff from the one consumers use.
**Potential refactoring.** Make acquisition return retained scopes consistently
and remove the unused production mode. Tests needing composed values can invoke
the real composition phase explicitly through a small test-only convenience
helper. Keep acquisition-only tests acquisition-only. Preserve handle closure,
pending-item placement reservation, accounting, diagnostics, and meaningful
composition coverage. This removes a private alternative path, not support for
any public loading workflow. Do not infer that other nil-state or directive
contexts are unused without separately tracing their callers.
## 2. Clarify Acquisition And Load-Coordinator Phase Handoffs
Status: open. Priority: medium. Scope: engine, across acquisition, version,
assembly, and lifecycle files.
**Relevant code.** `acquireInput` in
[acquisition.go](../../internal/engine/acquisition.go), `executeLoadRequest` in
[load.go](../../internal/engine/load.go), and `assembleLayeredInput` in
[load_request.go](../../internal/engine/load_request.go).
**Complexity.** `acquireInput` still combines search outcome handling, parsing,
disabled-directive checks, three directive/version-processing branches,
import-versus-standalone dispatch, placement/accounting, and passive context
construction. Its version-processing section repeats source-ID extraction,
failure handling, and `mainProcessing` updates, while the layered branch must
extract directives separately and restore them after version processing.
The coordinators also manipulate several intentionally distinct representations:
parsed source, local payload, acquired scope, composed root, inherited root,
local/protected version paths, and effective version evidence. For example,
`executeLoadRequest` includes a substantial ordinary-load resource-context
fallback between assembly and inheritance, while `assembleLayeredInput` repeats
declaration gathering, tier application, and destination projection for underlays
and main. Understanding a source-version or placement change requires tracing
both those mechanics and the overall phase order through long functions.
**Potential refactoring.** Extract cohesive operations for processing one parsed
input's directives/version evidence and for placing/registering a standalone
contribution. Reuse the existing processing result and acquisition state rather
than inventing another context hierarchy. Isolate the substantial context and
structural-expansion operations from the top-level lifecycle, and consider one
helper for the repeated tier application/projection handoff. Keep phase order,
error exits, and source acquisition order readable in the coordinators.
Extraction is useful only when it gives one operation a clear owner and result;
moving blocks into helpers that still share the same mutable surrounding locals
would not solve the problem. Preserve the deliberate ordinary/layered overlay
differences, default versus enabled version-check timing, warning accumulation,
exactly-once callbacks, and load-wide versus tier-local state. This is separate
from finding 1: removing the eager branch does not simplify these active paths.
## 3. Separate Collection-Policy Preparation From Merge Evidence Application
Status: open. Priority: medium. Scope: engine precedence and inheritance.
**Relevant code.** `mergeOverlayMappingsWithCollectionPolicy` and
`applyOverlaySequenceEvidence` in
[overlay.go](../../internal/engine/overlay.go);
`resolveStrictCollectionPolicies` in
[compose.go](../../internal/engine/compose.go);
`combineInheritanceTemplate` in
[inheritance_combine.go](../../internal/engine/inheritance_combine.go); and the
policy/origin helpers in
[inheritance_collection.go](../../internal/engine/inheritance_collection.go).
**Complexity.** The overlay coordinator prepares claims and declaration groups,
discovers applicable destinations, resolves operations, configures recursive
merge callbacks, accumulates diagnostics/history, reconciles declaration-only
policies after mutation, and sorts the output. Its callbacks close over several
maps and mutate the assembly while reporting failures through an outer slice.
`applyOverlaySequenceEvidence` then mixes policy retirement, displaced origins,
replacement records, copied incoming origins, and record-policy reconstruction.
There is also concrete duplicated bookkeeping: overlay and inheritance callbacks
both translate a sequence result into replace-versus-mapped history descriptors;
overlay evidence and `applyInheritanceSequencePolicy` both rebuild record policy
order from lower/higher item projections. The shared `combineSequence` is already
the correct value-combination owner, but maintainers must separately coordinate
these sidecar translations when its result representation changes.
**Potential refactoring.** Give overlay preparation, value/evidence application,
and declaration-only finalization explicit local boundaries. Let a small
operation owner or result carry the diagnostic and evidence collections currently
captured by the large callbacks. Share the straightforward history-descriptor
construction; evaluate a common projected-record selection helper while retaining
named precedence and inheritance ownership rules.
Do not merge the distinct origin semantics or hide mode-lifetime differences
behind a generic strategy framework. Keep `combineSequence` and the recursive
mapping merge shared, and consume their result rather than recomputing values.
Coordinate this work with collection-policy findings 1–2 in the
[efficiency audit](efficiency.md) so clearer preparation does not preserve or
introduce repeated whole-table copying/scanning. Preserve replacement ordering,
pending declarations, new versus retained record owners, and stable history.
## 4. Extract Patterned Glob Entry Preparation From Traversal Orchestration
Status: open. Priority: medium. Scope: engine import discovery.
**Relevant code.** `selectGlobImportCandidates` in
[glob.go](../../internal/engine/glob.go), especially its patterned-entry branch.
The corresponding directory-search entry operation already has a named owner,
`prepareImportDirectoryEntry` in
[import_selection.go](../../internal/engine/import_selection.go).
**Complexity.** The glob function nests pattern-segment traversal, parent
candidates, and directory entries. Its innermost branch handles matching,
hidden-entry policy, lexical and access paths, containment, link inspection,
follow behavior, intermediate-directory eligibility, native/host identity
differences, canonicalization, labels, and candidate creation. Set-union and
component-advancement mechanics are interleaved with those admission decisions,
making the important lexical-versus-canonical distinction difficult to follow.
**Potential refactoring.** Extract a glob-specific entry preparation operation
returning an admitted candidate, an ordinary skip, or a diagnostic. Leave pattern
component advancement, literal segments, per-component union/sorting, and final
exclusion/requiredness visible in the selector. Reuse low-level filesystem facts
where semantics coincide; directory and glob admission need not become one
general visitor interface. Preserve inspection/error order, entry accounting,
native namespace spelling, and selected-file validation. This is a readability
change, not a proposal to change the selected discovery algorithm or add caching.
## 5. Simplify Redundant Facade-To-Engine Input Transport
Status: open. Priority: low. Scope: root facade and engine, across packages.
**Relevant code.** Root `toEngineImportDefaults` in
[conversion.go](../../conversion.go), `toEngineOptions` in
[lifecycle.go](../../lifecycle.go), and `toEngineLayeredOptions` in
[layered.go](../../layered.go); engine transport/private defaults and option
decorators in [import_spec.go](../../internal/engine/import_spec.go), lines
46–87; and placement transport helpers in
[destination.go](../../internal/engine/destination.go), lines 23–39.
**Complexity.** Import defaults have a root-owned public representation, an
engine facade-transport representation, and a second engine-private representation
with the same five fields. Conversion constructs the transport, an option
decorator converts it to private defaults, and load normalization snapshots it
again. Placement similarly requires constructing an engine record and then
passing it through a separate setter-style helper. Even a simple control change
has multiple mechanical mapping sites across the package boundary.
Caller and text-reference checks also found no uses of the standalone
`PlacementModeFromFacade` and `PlacementModeForFacade` functions beyond their
declarations. The similarly named `SourceInfoPlacementModeForFacade` is used by
root source-record conversion and is not dead code. The unused pair adds a
second apparent placement conversion route for maintainers to investigate.
Root type ownership and explicit conversion are deliberate and useful; the
opportunity is the extra engine-internal representation/construction protocol,
not eliminating the facade. The public wrappers do not need to change.
**Potential refactoring.** Remove the two unused enum bridges. Then consider one
engine representation of import-default input values and a straightforward
construction surface on the existing engine option/input records. Review inert
placement decorators as part of the same bounded cleanup. Keep helpers that
actually encode domain rules or ownership conversion, and keep
definition/registration compilation separate from transport.
Avoid adding another universal request DTO, code generator, reflective mapper,
or adapter framework to remove a few mechanical functions.
Preserve root-owned named types and signatures, invalid-enum validation,
nil-versus-empty patterns, pointer/slice snapshots, and the two public loading
entry points. Exported declarations inside Go's `internal` boundary need not
become public root aliases or bridge methods. Pursue this low-priority cleanup
only if it reduces total concepts and mapping sites, rather than relocating them.
## Areas To Retain And Review Boundaries
- The clone traversal was explicitly reviewed and retained in the completed
consolidation record at `34cbc0d:docs/roadmap/consolidation.md`. Its high score
is not new evidence for reopening that work. `valueBuilder.build`, typed
decoding, and the YAML translator also contain substantial but recognizable
type dispatch; no repository-wide visitor framework is recommended.
- The cohesive engine package and explicit root record conversions follow
[ADR 0006](../adr/0006-public-facade.md). Function/file counts alone do not
justify package extraction, public API consolidation, or generated conversions.
- The shared scheduler, mapping/sequence primitives, recently separated
inheritance phases, schema field assembly, and resolved filesystem facts remain
useful common owners. Preserve those improvements.
- `history.go` and the larger integration-test files have several substantial
routines, but file size alone did not establish another prioritized refactor.
The deferred replay, copy-handoff, caching, and contributor-index experiments
remain in [future work](future.md#3-performance-experiments).
These are opportunities for further consideration, not a staged implementation
plan or new release blockers. Favor the concrete unused-mode removal and active
acquisition/collection boundaries before lower-value transport cleanup. A later
implementation should preserve the established contracts and relevant behavioral
coverage; complexity scores, helper counts, and private call sequences should
not become test thresholds or repository guarantees.
## Validation
This audit changes only this findings document. No production code, tests,
dependencies, policies, or public contracts are changed. Validation consists of
the existing local documentation-link test and `git diff --check`; runtime and
performance measurements are not part of this review.

108
docs/roadmap/concurrency.md
View File

@@ -1,108 +0,0 @@
# Concurrency And Lifetime Audit
Reviewed revision: `a80273433e87ca955e5a5d9e99b616d8da10f185`.
Audit date: 2026-10-25.
## Scope And Assessment
This audit reviewed source reuse, concurrent independent loads, schema and
acquisition state, callback lifetime, publication, frozen metadata/explanation/
diff/rendering, resource contexts, and secrets bootstrap. Knowledge-graph searches
and caller traces were supplemented by source inspection and existing tests.
No new data race, deadlock, unsafe synchronization, goroutine leak, channel
management defect, or cancellation/shutdown defect was established under the
documented usage contracts. One previously reported callback-lifetime defect
remains reproducible and relevant to teardown; it is referenced below rather
than assigned a second independent remediation task.
The [implemented architecture](../internal/overview.md) provides a straightforward
ownership model for concurrency:
- Loading runs synchronously through one engine lifecycle. Production code
launches no library-owned goroutines and has no channel, worker, timer, watcher,
or background shutdown protocol to manage.
- Acquisition, version evidence, schema caches, reflection traversal, composition,
and callback records belong to a load or operation, rather than a shared global
cache. The root facade delegates without introducing a second shared state owner.
- Reader-source copies share a pointer to `readerSource`, whose atomic one-use
claim precedes reading. Byte and mapping sources retain constructor snapshots.
- Publication builds the retained inspection indexes before returning. Query,
explanation, diff, and rendering paths read frozen state and allocate their
own mutable results or replay trees. Typed configuration and returned metadata
are detached from the retained inspection snapshot.
- Returned resource contexts contain passive root information or a borrowed
filesystem. Host resource opens acquire their own handles, with ownership
transferred to the returned file, rather than sharing the acquisition handle.
These observations support the existing model; they are not new blanket
thread-safety guarantees for application callbacks, borrowed objects, or I/O.
## 1. Callback Lifetime Guards Survive Panic Unwinding — Previously Reported
Status: open, tracked by
[correctness finding 4](correctness.md#4-callback-contexts-remain-active-after-panic-unwinding).
Priority: low. Category: callback teardown across the facade/engine boundary.
**Relevant code.** `runResolution` in
[resolution.go](../../internal/engine/resolution.go), and `runValidation` /
`reportVersion` in [validation.go](../../internal/engine/validation.go), deactivate
the callback state only after a normal return. The root callback wrappers in
[lifecycle.go](../../lifecycle.go) expose that same engine context.
**Circumstances and consequence.** If a callback retains its context and panics,
and application code recovers outside the load, the retained context still
accepts queries or diagnostic writes after callback exit. It should instead
produce the established closed-context failure. The abandoned per-load state
therefore remains accessible beyond its intended lifetime. Reusing the existing
external diagnostic probe reproduced this for resolution, semantic validation,
and version-check contexts at the reviewed revision.
**Potential remediation.** Use invocation-scoped deferred deactivation, as
described in the linked correctness finding, preserving immediate closure on
normal return and unchanged propagation of application panics. There is no need
to add context locks, atomic lifecycle flags, panic recovery, or background-work
management. Concurrent use of a live callback context is already outside its
supported contract; this finding does not establish an additional supported-use
race or justify broadening that contract.
## Validation
The following passed with Go 1.26.10 on Linux amd64:
```sh
go test -race ./... -count=1
go vet ./...
```
The six existing engine concurrency tests for shared definitions, copied-reader
claims, concurrent inspection, selected-overlay loads, and resolution-input
isolation also passed with `-race -count=20 -cpu=1,4,8`. These tests live in
[independence_test.go](../../internal/engine/independence_test.go),
[source_test.go](../../internal/engine/source_test.go),
[explain_test.go](../../internal/engine/explain_test.go), and
[provenance_test.go](../../internal/engine/provenance_test.go).
The previously created standalone callback-panic probe was rerun outside the
repository with race instrumentation. It confirmed the known lifetime defect
without reporting a data race; that probe performs sequential post-panic calls,
not unsupported concurrent callback-context use. No new tests or production
changes were made. Race detection covers executed paths and is not a proof of
all possible interleavings. No native macOS or Windows execution was performed.
## Contract Boundaries Preserved
- Read-only shared definitions and independent result ownership remain the
existing model. Callers remain responsible for synchronization of their own
mutable options, callback effects, readers, filesystems, and output writers.
- Secrets bootstrap is explicitly a startup operation with no environment
compare-and-set guarantee; its check-then-set behavior is not a new finding.
See the [bootstrap contract](../config.md#explicit-secrets-directory-bootstrap).
- Synchronous loading has no cancellation API or background refresh service.
Caller-supplied blocking operations are not evidence of a leaked ConfigKit
goroutine or a missing promised shutdown mechanism. See
[completion and resource lifetime](../config.md#completion-and-resource-lifetime).
This report adds no synchronization requirements, new API, or release gates.
The existing [correctness](correctness.md), [efficiency](efficiency.md), and
[maintainability](complexity.md) findings retain their own scopes and status.

191
docs/roadmap/correctness.md
View File

@@ -1,191 +0,0 @@
# Functional Correctness Audit
Reviewed revision: `47944d22d1ad45a7f537780effb3d05f998eef14`
(Update ReleaseKit to v0.4.2).
Audit date: 2026-10-25.
## Scope And Evidence
This audit reviewed the root facade and its engine handoffs, source/discovery
and filesystem handling, composition and collection rules, inheritance, typed
decoding and ownership, application callbacks, secrets, metadata, explanation,
and effective-value comparison against the configuration contract and relevant
implementation/tests. It was a source-level functional review with targeted
diagnostics, not an exhaustive proof or a performance/security certification.
The existing development check (`./scripts/check.sh`) and full race-enabled
suite (`go test -race ./...`) passed using Go 1.26.10 on Linux amd64. Standalone
public-API probes outside the repository reproduced the findings below. No
production code or repository tests were added or changed. No native macOS or
Windows validation was performed, and no new release gates are proposed.
## 1. Pointer-Only Recursive Types Can Overflow The Stack Or Hang
Status: open. Priority: medium. Category: schema/decoder termination.
**Relevant code.** `schemaBuilder.describe` in
[schema.go](../../internal/engine/schema.go), lines 119–140, caches a placeholder
before describing pointer elements. A legal Go type such as `type Loop *Loop`
therefore produces a pointer descriptor whose element is itself.
`decodeValue` in [decode.go](../../internal/engine/decode.go), lines 93–106,
recursively unwraps pointers without consuming the input node. The pointer
unwrapping loops in `resolveSchemaPath` (`schema.go`, lines 347–350 and 387–390)
also assume that a non-pointer descriptor will eventually be reached.
**Reproduction.** With the public package imported as `configkit`:
```go
type Loop *Loop
type Config struct {
Value Loop `yaml:"value"`
}
configkit.Load[Config](configkit.Bytes("input", []byte("value: {}\n")))
```
This does not return a decode error. It repeatedly calls `decodeValue` with the
same input and descriptor until the process reports a fatal stack overflow.
A disposable probe using `debug.SetMaxStack(64 << 10)` demonstrated the failure
without allowing the process to grow a large stack. Explicit `value: null`
loads successfully. With that null input, adding a resolver that calls
`Normalize` on `/value` with `Loop(nil)` instead hangs in `resolveSchemaPath`;
the separate diagnostic process was stopped by a three-second timeout.
**Expected behavior.** Incompatible non-null input must fail with a safe
diagnostic, and schema-path resolution must terminate. The
[schema contract](../config.md#application-schema-binding) supports recursive
types with acyclic values; the existing explicit pointer-only restriction is
for inline fields, not all pointer fields. This is not a request to recover
application panics or reject ordinary recursive structs.
**Potential remediation.** Detect repeated descriptors while unwrapping
pointer-only chains in decoding and schema-path traversal. Preserve supported
nil/null values and finite acyclic values, return an appropriate decode error
for an incompatible non-null node, and reject descendants that cannot be
addressed. Review analogous pointer-unwrapping helpers for the same assumption.
A blanket schema rejection of pointer-only recursive types would instead be a
contract decision, since it would also reject currently valid nil values; do
not introduce it or an arbitrary pointer-depth cap merely to stop the loop.
## 2. Failed Dynamic Schema Construction Leaves A Reusable Partial Descriptor
Status: open. Priority: medium. Category: error handling and shared operation state.
**Relevant code.** `valueOperation.dynamicDescriptor` in
[value_operation.go](../../internal/engine/value_operation.go), lines 36–48,
passes the load's existing descriptor cache directly to a new schema builder.
`schemaBuilder.describe` in [schema.go](../../internal/engine/schema.go), lines
119–140 and 167–191, caches container placeholders but returns child errors
without removing those placeholders. A subsequent lookup returns the partial
descriptor as if construction had succeeded.
**Reproduction.** Use a configuration with `Value any` and this dynamic type:
```go
type Invalid struct {
Channel chan int `yaml:"channel"`
}
```
In one resolver, execute these two assignments in order:
```go
configkit.Normalize(r, configkit.Keys("value"), &c.Value,
any((*Invalid)(nil)), "first unsupported value")
configkit.Normalize(r, configkit.Keys("value"), &c.Value,
any(&Invalid{}), "second unsupported value")
```
The first helper correctly records an invalid-value diagnostic because channels
are unsupported. It nevertheless leaves the `*Invalid` descriptor cached with
a nil element. The second helper reuses it, reaches projection with that nil
element, and panics with a nil-pointer dereference instead of returning a
`LoadError`. This was reproduced through ordinary public `Load` and `Normalize`;
the application callback itself does not panic.
**Expected behavior.** Unsupported dynamic/helper values must produce safe
schema/resolution diagnostics under the
[typed projection](../config.md#typed-projection-and-ownership) and
[resolution contracts](../config.md#recorded-application-resolution).
One reported validation failure must not corrupt the descriptor cache used by
later operations in the same callback.
**Potential remediation.** Make dynamic descriptor construction transactional:
stage newly discovered descriptors and commit them only after a successful
build, or track and roll back all cache additions belonging to a failed build.
Account for recursive descriptors that refer to other partially built types,
not just the immediate failed container. Preserve previously valid descriptors
and the load-local cache ownership. A nil guard in projection alone would not
repair invalid cache entries being treated as successful schema descriptions.
## 3. Cycle Detection Rejects Acyclic Slice Views Sharing A Starting Address
Status: open. Priority: medium. Category: supported-value correctness.
**Relevant code.** `valueVisit` and `visitFor` in
[value_operation.go](../../internal/engine/value_operation.go), lines 20–24
and 97–116, identify a slice by its type, kind, and starting pointer, but not its
length. Both reflective projection/cloning and the parser-neutral value builder
in [value.go](../../internal/engine/value.go) use that identity for active-path
cycle detection.
**Reproduction.** This value is finite and acyclic: its nested one-element view
contains only the string, not the enclosing two-element view.
```go
shared := []any{"leaf", nil}
shared[1] = shared[:1]
```
Place it at `data` in a `Mapping` source, return it from typed defaults for
`Data []any`, or supply it as a CLI override. Observed results at `/data/1` were,
respectively, `source.invalid`, `schema.cycle`, and `override.invalid_value`.
The equivalent independently allocated value
`[]any{"leaf", []any{"leaf"}}` loads and renders successfully. A true
self-referential slice was also checked and remains correctly rejected.
**Expected behavior.** The
[mapping source](../config.md#sources),
[projection/ownership](../config.md#typed-projection-and-ownership), and
[override](../config.md#explicit-cli-overrides) contracts permit supported
acyclic values. Shared backing storage alone is not a cycle, and detaching it
does not require preserving alias identity in the returned configuration.
**Potential remediation.** Include the visible slice extent (its length) in the
active-visit identity, or use an equivalent identity that distinguishes these
views. Preserve detection of actual self/mutual cycles, empty/nil distinctions,
and independent ownership of retained values. Cover the shared helper's source,
override, projection, and cloning callers rather than fixing only one entry point.
## 4. Callback Contexts Remain Active After Panic Unwinding
Status: open. Priority: low. Category: callback-lifetime contract.
**Relevant code.** `runResolution` in
[resolution.go](../../internal/engine/resolution.go), lines 283–291, and
`runValidation`/`reportVersion` in
[validation.go](../../internal/engine/validation.go), lines 99–106 and 250–257,
set `state.active = false` only after the application callback returns normally.
The root facade forwards the context without an additional lifetime guard.
**Reproduction.** Retain the supplied context in a resolver, semantic validator,
or version checker, then panic in that callback. Recover the application panic
outside `Load` and invoke the retained context. Public-API probes showed that
`Resolution.IsExplicit` still returns a value and `Validation.Warning` still
accepts a report for both validation and version contexts; none produces the
documented closed-context panic.
**Expected behavior.** The
[resolution](../config.md#recorded-application-resolution) and
[validation](../config.md#semantic-validation) contracts make these contexts
callback-scoped and require retained contexts to reject use after callback exit.
Application panics remain programming failures and should propagate unchanged;
closing the context during unwinding does not require ConfigKit to recover them
or return a partial result. This finding concerns only the lifetime guard after
an application chooses to recover its own panic.
**Potential remediation.** Deactivate the context with a defer scoped to each
callback invocation, retaining immediate deactivation on normal return as well
as on panic unwinding. Do not leave the context active through later projection
or metadata work, and do not convert or suppress the callback panic. Apply the
same lifetime handling to resolution, semantic validation, and version checks.

307
docs/roadmap/deadcode.md
View File

@@ -1,307 +0,0 @@
# Dead Code And Obsolete Functionality Audit
Reviewed revision: `b1c278b38bead1fceeb80b99d2f336b0cb6fe2fd`.
Audit date: 2026-10-25.
## Scope And Method
This audit covers the public facade, engine, document and YAML packages,
consumer examples, and development/release entry points. It identifies private
implementation cleanup, not public features to retire or new release gates.
All findings below are open opportunities for further consideration.
Knowledge-graph discovery and caller tracing were supplemented by a temporary,
type-aware reference inventory built outside the repository with the locally
cached `golang.org/x/tools` v0.49.0. It distinguishes declarations, production
references, test references, and direct field writes. Static inventories were
run for Linux, Darwin, and Windows; only Linux tests were executed. A separate
SSA/Rapid Type Analysis pass over test executables supplied additional leads.
Neither absent graph edges nor an unreachable result from the selected test
programs was treated as proof of dead library code. In particular, uninstantiated
generic declarations and unexercised public APIs require caller/source review.
The standalone `deadcode`, `unused`, and `staticcheck` commands were not installed.
Building the cached standalone `deadcode` command offline encountered an uncached
telemetry dependency; the temporary analysis instead used cached packages/SSA/RTA
libraries directly. Repository dependencies and tooling were not changed.
Candidates were checked for interface dispatch, reflection, whole-record use,
map-key equality, build constraints, public consumers, and documented retention.
The existing Linux suite and `go vet ./...` pass with Go 1.26.10. No benchmarks,
new tests, or production changes were made.
## Architectural Assessment
The [implemented architecture](../internal/overview.md) remains coherent. The
root owns the supported API and explicit conversions; `internal/engine` owns
synchronous acquisition, composition, inheritance, decoding, CLI application,
resolution, validation, and frozen inspection. `internal/document` owns the
ordered tree, and `internal/yaml` owns parser/dumper integration. Ordinary and
layered loading share one lifecycle.
Most findings are remnants beside that shared lifecycle: unused snapshots,
private clone families disconnected from the facade, obsolete helper entry
points, and bookkeeping whose results no longer reach an active consumer.
No supported public feature or deliberate compatibility mechanism was established
as obsolete. Existing correctness and resource findings remain owned by their
separate audit documents.
## 1. Remove The Unconsumed Override-Result Pre-CLI Snapshot
Priority: medium. Scope: engine loading and override handoff.
**Relevant code.** `decodedOverrideResult.preCLI` and `applyDecodedOverrides`
in [override_decode.go](../../internal/engine/override_decode.go), lines 9–18,
33, and 79; `preparePostDecodeOverrideContext` in
[override_context.go](../../internal/engine/override_context.go), lines 20–84.
**Evidence.** `applyDecodedOverrides` clones the complete `decoded.final` tree
and returns it in `preCLI`. Type-aware references identify no production read
of that particular field. Its readers are assertions in `override_test.go` and
`override_context_test.go`. The production recipient instead independently clones
`before.final` into `postDecodeOverrideContext.preCLI`.
This performs an extra traversal and allocation proportional to the decoded
tree on successful ordinary and layered loads, including loads without CLI
assignments. It does not supply the pre-CLI snapshot used by explanation.
**Potential cleanup.** Remove the unused result member and its clone, and
update assertions tied to that member. Keep the independently owned pre-CLI
state that actually feeds resolution/publication and explanation. This is not a
proposal to share mutable trees or redesign the remaining ownership handoffs.
## 2. Retire The Unqueried Published Structural-Input Clone
Priority: medium. Scope: engine publication and immutable lookup.
**Relevant code.** `Inspection.structuralInput` in
[load.go](../../internal/engine/load.go), line 96, and `publishLoadResult` in
[load_result.go](../../internal/engine/load_result.go), lines 112, 119, and 131.
Lookup ownership is in
[document_lookup.go](../../internal/engine/document_lookup.go); active replay is
in [explain.go](../../internal/engine/explain.go).
**Evidence.** Publication clones `resolved.structuralInput`, stores the clone
on `Inspection`, and indexes it as an additional immutable tree. No production
reader accesses `Inspection.structuralInput`; one private test populates it.
Explanation replays source applications and inheritance operations, then uses
`preCLI` and `input`. It does not query this separately cloned structural root.
The lookup is addressed by a supplied node pointer, not by searching all indexed
roots, so indexing the otherwise unreachable clone does not make it useful.
Its mapping-node keys also keep the tree alive.
**Potential cleanup.** Remove the final publication clone, unused inspection
member, and that clone's inclusion in `retainedTrees`. Preserve
`resolved.structuralInput` where it is still used for path finalization and
sensitivity classification before publication. Do not indiscriminately remove
earlier structural evidence or active source/application history.
Coordinate this bounded removal with finding 2 in the
[resource audit](resources.md): removing an unused tree does not by itself fix
accounting for the other retained replay trees.
## 3. Remove The Facade's Disconnected Record-Clone Family
Priority: low. Scope: root facade, across the root/engine conversion boundary.
**Relevant code.** The private helper block in
[records.go](../../records.go), lines 317–430:
`cloneLocation`, `cloneLocations`, `cloneSourceInfo`, `cloneImportSelections`,
`cloneSourceVersionCheck`, `cloneSourceInfos`, `cloneOrigin`, `cloneFieldInfo`,
`cloneChange`, `cloneDiagnostic`, and `cloneDiagnostics`.
**Evidence.** None is reachable from a production facade entry point.
`cloneSourceInfos`, `cloneFieldInfo`, `cloneChange`, and `cloneDiagnostics`
have no references at all. Their supporting helpers refer to one another;
the only external callers of this family are private `records_test.go` calls
to `cloneSourceInfo`. Public results already acquire root-owned records through
`fromEngineSourceInfo`, `fromEngineOrigin`, `fromEngineFieldInfo`,
`fromEngineChange`, and diagnostic conversions in
[conversion.go](../../conversion.go). The engine's separate clone helpers are
active and are not removal candidates.
**Potential cleanup.** Delete the disconnected root helper family. Keep relevant
conversion checks, and express ownership checks through repeated public record
queries and mutation of returned nested data rather than testing an unused
clone API. This complements the facade-test observations in the
[testing audit](testing.md); it does not eliminate explicit conversions,
root-owned types, or active `Path.clone` callers.
## 4. Remove Uncalled Engine Helper Remnants
Priority: low. Scope: individual engine helpers and facade transport.
The following definitions have no callers or function-value references in
production or tests, across the inspected build configurations:
| Definition | Relevant code | Evidence and active replacement |
| --- | --- | --- |
| `mergeDisjointMappings` | [compose.go](../../internal/engine/compose.go), line 486 | Its only reference is its own recursion. Strict composition uses `mergeDisjointMappingsAtPath` and the shared recursive merge machinery. |
| `mergeHistoryDocument` | [history.go](../../internal/engine/history.go), line 240 | No caller remains. Replay calls `mergeHistoryDocumentAtPath` directly with sequence evidence. |
| `inheritanceDirectiveLocation` | [inheritance.go](../../internal/engine/inheritance.go), line 176 | No caller remains. Active preparation uses the directive entry and its location directly. |
| `addSecretEvidencePath` | [secret.go](../../internal/engine/secret.go), line 137 | No caller remains. Classification obtains projected sensitivity paths from version evidence and feeds the active path accumulator. |
| `PlacementModeFromFacade`, `PlacementModeForFacade` | [destination.go](../../internal/engine/destination.go), lines 23 and 26 | Neither is used by the facade. Input/options decorators perform the incoming conversion; `SourceInfoPlacementModeForFacade` supplies outgoing evidence. |
**Potential cleanup.** Delete these definitions and any imports/comments left
solely for them, keeping their active counterparts. These are private functions
or internal-package exports, not supported root API. The unused placement pair
was already identified in finding 5 of the [complexity audit](complexity.md);
this audit confirms that bounded removal without expanding its broader transport
refactor.
## 5. Stop Constructing Unconsumed Combination Evidence
Priority: medium. Scope: sequence combination and inheritance bookkeeping.
**Relevant code and evidence.**
- `sequenceCombinationResult.discardedLower` in
[sequence_combine.go](../../internal/engine/sequence_combine.go), lines 26
and 72, is populated with a newly allocated list of every lower sequence
index during replacement. It has no production reader; three test assertions
inspect it. Overlay and inheritance owners already handle whole replacement
from the selected operation and lower value. The result therefore allocates
an unused `O(n)` index slice for an `n`-item lower sequence.
- `inheritanceReplacement.path`, `.previous`, and `.effective` in
[inheritance.go](../../internal/engine/inheritance.go), lines 135–141, are
written by [inheritance_combine.go](../../internal/engine/inheritance_combine.go)
and [inheritance_collection.go](../../internal/engine/inheritance_collection.go)
but never read by production. Populating them performs path/origin copies
and `displacedOrigins` subtree walks whose results are discarded. The active
history path uses `inheritanceOperation`, its origin maps, and
`projectedInheritanceChanges` in
[inheritance_history.go](../../internal/engine/inheritance_history.go).
The replacement's `before` and `after` trees, however, are read by retained-node
accounting in [inheritance_finalize.go](../../internal/engine/inheritance_finalize.go)
and [history.go](../../internal/engine/history.go).
**Potential cleanup.** Remove the unread members and computations exclusively
feeding them. Preserve higher/lower item projections, replacement mappings,
`appendedHigher`, active provenance/history, and replacement node accounting.
Keep `sequenceIndexes` where append still uses it, and keep `displacedOrigins`
where overlay processing needs it. Adjust private evidence assertions while
retaining behavior-level replacement, removed-descendant, and history tests.
This consolidates duplicate evidence ownership rather than removing inspection
functionality or changing collection semantics.
## 6. Trim Write-Only Private Result And Transport Metadata
Priority: low. Scope: engine phase handoffs and private result shapes.
The following members are populated but have no production reader. Some are
inspected by private tests; none is exposed as a root record or callback field.
They add apparent responsibilities to result shapes that active consumers do not
actually use:
| Private record and members | Relevant code and evidence |
| --- | --- |
| `collectionPolicyResolution.declarations`, `declarationsPresent`, `identityDeclarationPresent`, `applicationIdentityFields` | [collection_policy.go](../../internal/engine/collection_policy.go), lines 14–24. The resolver uses local normalized declarations/fields to compute the active policy; these returned copies/flags are not consumed. Preserve `modeDeclarationPresent` and `applicationIdentityPresent`, which are consumed. |
| `inputPresence.null` | [decode.go](../../internal/engine/decode.go), line 25. Presence collection writes it, but current metadata null reporting uses the effective node's kind. The presence entry and its location remain active. |
| `rootDirectiveExtraction.importsPresent`, `sequencesPresent` | [directives.go](../../internal/engine/directives.go), lines 25–26. Parsing sets them, but callers use the parsed declaration slices and diagnostics. |
| `importSelection.outcome` | [import_selection.go](../../internal/engine/import_selection.go), line 29, and [glob.go](../../internal/engine/glob.go). Selectors set it, but traversal uses candidate lists and admitted-source counts. The separate `importSelectionEvidence.outcome` is consumed by public source-record conversion and must remain. |
| `importDeclarationSpec.excludeSet` | [import_spec.go](../../internal/engine/import_spec.go), line 38. Parsing sets it; active policy resolution consumes `exclude`, not this flag. |
| `schemaPath.field`, `dynamic` | [schema.go](../../internal/engine/schema.go), lines 335–339. Path resolution tracks and returns them, but callers consume only the descriptor and success result. |
| `selectedOverlayRequest.filesystem` | [selected_overlay.go](../../internal/engine/selected_overlay.go), line 33. Callers populate it, but the loader uses its explicit filesystem-access argument. |
| `sequenceDeclarationSpec.destinationLocation` | [sequence_declaration.go](../../internal/engine/sequence_declaration.go), line 17. The parser stores it, but later diagnostics use other retained declaration/field locations. |
| `sourceVersionEvidence.absence`, `checked`; `sourceVersionAncestor.kind` | [source_version.go](../../internal/engine/source_version.go), lines 27–43. These annotations have no production consumers. Actual checking, version/presence/location evidence, and ancestor paths are consumed separately. |
| `postDecodeOverrideContext.sources` | [override_context.go](../../internal/engine/override_context.go), line 14. The local source list is used to build resolution input, but returning it again in this member serves only tests. |
**Potential cleanup.** Remove these unused result slots and any bookkeeping
whose sole purpose is filling them. Keep computations that validate or produce
the active result, and keep public presence, null, selection, version, ownership,
and diagnostic behavior unchanged. In particular, do not remove the source list
used for callback input, public empty-selection evidence, declaration validation,
or filesystem access. Rework assertions on inactive annotations to check the
actual supported outcome where they protect a meaningful risk.
This is a bounded handoff cleanup, not a requirement to remove every field with
no explicit selector read: map keys, embedded records, and interface contracts
need separate treatment.
## 7. Consolidate The Test-Only Import Directive Parser
Priority: low. Scope: engine import parsing and test fixtures.
**Relevant code.** `parseImportSpecificationDirective` in
[import_spec.go](../../internal/engine/import_spec.go), lines 183–201;
`parseImportSpecsYAML` in
[import_spec_test.go](../../internal/engine/import_spec_test.go); and
`parseRootDirectiveBlock` in [directives.go](../../internal/engine/directives.go).
**Evidence.** The older parser has exactly one direct caller, the test fixture
helper. That helper feeds many selector/declaration tests, but no supported load
uses this parser. The active root parser owns the same mapping/field dispatch
and delegates import declarations to the same `parseImportDeclarations` function,
while also supporting the appropriate sequence-declaration context. This is a
parallel directive-validation implementation, not merely a thin test adapter.
**Potential cleanup.** Have the fixture helper use the active root-directive
parser with the appropriate context, or parse the import declaration list
directly when that is the test's actual boundary. Remove the obsolete production
parser after preserving relevant malformed-declaration and selector coverage.
Do not retire the active imports-only context wholesale: layered version
processing still uses it after extracting the outer directives.
## 8. Remove The Test-Only Eager Acquisition Mode
Priority: medium. Scope: engine acquisition. Previously recorded in finding 1
of the [complexity audit](complexity.md); not a second independent defect.
**Relevant code.** `inputAcquisitionRequest.deferAssembly` in
[acquisition.go](../../internal/engine/acquisition.go), and the final branch of
`acquireImportedWithAccess` in
[import_acquisition.go](../../internal/engine/import_acquisition.go), lines 206–224.
**Evidence.** Both production request construction sites, `executeLoadRequest`
in [load.go](../../internal/engine/load.go) and `acquireUnderlayGroupScopes`
in [load_request.go](../../internal/engine/load_request.go), set the flag to
true. Only private test entry helpers choose the eager path. Its assembly call
and alternate return protocol are therefore disconnected from supported loading.
**Potential cleanup.** Make acquisition consistently return retained scopes.
Tests needing composition can explicitly invoke the active composition phase
through a test helper. Preserve pending-item reservation, resource closure,
accounting, diagnostics, and composition coverage. Thin test conveniences that
delegate to active implementations are not generally dead functionality; the
problem here is the alternate production phase protocol maintained solely for
tests.
## Retained Functionality And Review Limits
- Public exports remain supported even when current in-repository programs do
not call them. Discovery presets, `Metadata.HistoricalPaths`, generic APIs,
diagnostic codes, and optional controls are not removal recommendations.
- Heap methods, filesystem adapters, error/string/format methods, writer
methods, reflection descriptors, callback bindings, and platform-specific
implementations may be live without direct calls. They were not classified
as dead on reference counts alone. Nonzero-size identity tokens and
whole-struct map keys also have meaningful behavior without field reads.
- `decodeInlineMapEntry` and import cycle diagnostics have real production
callers despite gaps in current execution coverage. They remain necessary.
- `Inspection.resolution` has no production selector reader, but the
[recorded-resolution contract](../config.md#recorded-application-resolution)
documents retaining each recorded before/after snapshot. This audit does not
propose dropping that retention merely because explanation uses a coalesced
replay view. Any separate retention redesign must respect that contract.
- The retained inclusion graph, historical paths, sensitivity evidence, and
passive contexts serve supported behavior. Deferred experiments in
[future work](future.md#3-performance-experiments) are not obsolete features
to remove. Neither are deliberate overlay/default/version semantics.
- This audit establishes cleanup candidates, not measured performance gains or
an exhaustive proof that all unsupported input states are unreachable.
## Validation
The following checks passed against unchanged production code:
```sh
env PATH=/snap/go/current/bin:$PATH GOTOOLCHAIN=local GOTELEMETRY=off \
GOPROXY=off GOCACHE=/tmp/configkit-review-cache-shared \
go test ./... -count=1
env PATH=/snap/go/current/bin:$PATH GOTOOLCHAIN=local GOTELEMETRY=off \
GOPROXY=off GOCACHE=/tmp/configkit-review-cache-shared go vet ./...
```
The audit document is additionally checked by the existing local documentation
link test and whitespace checks. No production code, tests, dependencies,
policies, public contracts, or release machinery are changed by this audit.

245
docs/roadmap/efficiency.md
View File

@@ -1,245 +0,0 @@
# Algorithmic Efficiency Audit
Reviewed revision: `5098b56655bd383e22a57d6cf09d99db3e92090f`.
Audit date: 2026-10-25.
## Scope And Assessment
This source-level audit reviewed the public facade, acquisition and discovery,
strict composition, precedence assembly, collection policy, inheritance, schema
and typed-value operations, callbacks, secrets, inspection, and comparison.
Knowledge-graph searches and caller traces guided source inspection; the bounds
below follow from the actual loops and handoffs, not timing measurements.
The implementation already uses indexed mapping merges, tuple-indexed sequence
matching, sparse item projections, scoped import adjacency, inheritance policy
localization, and worklist sensitivity propagation. The findings are remaining
batch-processing costs, not a recommendation to replace those mechanisms.
The review excludes expressly selected algorithms, including deterministic
sorting, stable Kahn scheduling, exact numeric identity normalization, and the
pairwise registration-overlap check expressly selected in the completed sequence
plan at `d0a605a:docs/roadmap/implementation.md`. It also leaves the
[deferred performance experiments](future.md#3-performance-experiments) deferred.
Required detached copies and genuinely expanding configuration/history output
are not treated as avoidable overhead.
Complexities describe the identified internal work, not an end-to-end runtime
bound or a new consumer guarantee. Examples use bounded-depth paths and
constant-sized records unless noted; hash-index alternatives use expected lookup
costs, with string processing and actual copied payload accounted for separately.
Existing resource limits bound realizable inputs but do not remove these growth
patterns. No new limits, public caches, dependencies, or release gates are proposed.
No additional functional defect was established during this pass. The four open
findings in the separate [functional correctness audit](correctness.md) remain
applicable and are not duplicated here.
## 1. Policy Retirement Scans Every Collection For Every Replacement
Status: open. Priority: medium.
**Relevant code.** `collectionPolicyScope.retireCollectionPoliciesAtOrBelow` in
[collection_policy_state.go](../../internal/engine/collection_policy_state.go),
lines 163–174, scans the complete policy table. The replacement hook in
`mergeOverlayMappingsWithCollectionPolicy`
([overlay.go](../../internal/engine/overlay.go), lines 182–185) calls it for each
ordinary replaced value. `applyOverlaySequenceEvidence` also calls it for
whole-collection replacement.
**Scaling case.** A lower tier has P independently declared collection policies
and Q ordinary scalar settings elsewhere. A higher tier changes those Q scalar
settings without touching a collection. Each scalar replacement scans all P
policies even though none is affected: O(P × Q) policy visits. Taking P = Q = N
gives quadratic work for linear-sized inputs, effective values, and policy state.
This occurs within one precedence merge; it does not depend on many tiers,
registration-overlap checking, or expanding inheritance output.
**Potential refactoring.** Prepare an operation-local semantic path index over
the policy table and use descendant queries for retirement, maintaining it as
policies change. Alternatively, batch retirement against the old policy state
where its timing can be preserved. For the disjoint example, preparation plus
queries can be O(P + Q); generally charge additional work to affected entries
and path lengths. Preserve explicit-mode persistence, record ownership, and the
point at which old identity is retired. A delayed blanket retirement must not
clear newly established higher-tier policy. Existing inheritance policy indexing
provides a related mechanism to evaluate, not a requirement to unify its interface.
## 2. Small Tiers Recopy Unrelated Accumulated Collection Policy
Status: open. Priority: medium.
**Relevant code.** `resolveStrictCollectionPolicies` in
[compose.go](../../internal/engine/compose.go), lines 330–334, first obtains
`effectiveLower` through `lowerCollectionPoliciesForClaims`, then clones the
result. The latter helper
([collection_retirement.go](../../internal/engine/collection_retirement.go),
lines 65–71) already deep-clones and scans the entire lower scope.
`cloneCollectionPolicyScope` / `cloneCollectionPolicy` in
[collection_policy_state.go](../../internal/engine/collection_policy_state.go)
also copy record-order and nested record-policy state.
`assembleLayeredInput` in [load_request.go](../../internal/engine/load_request.go)
supplies the accumulated lower policy when composing each underlay group and
when applying that group to the accumulator. The overlay resolver repeats this
whole-table preparation and additionally checks all retained policy destinations
against the incoming tree (`overlay.go`, lines 145–150).
**Scaling case.** An initial underlay establishes P constant-sized collection
policies. Each of T later underlays contributes only one unrelated scalar, with
no declarations or collection changes. Despite having no relevant collection
work, each tier copies/scans all P policies: O(T × P) work and cumulative
allocation. P = T = N yields quadratic bookkeeping for O(P + T) input and final
policy/value sizes. These are transient copies of unchanged private policy, not
T separately published snapshots required by the public ownership contract.
**Potential refactoring.** Separate read-only lower-policy lookup from the owned
policy changes produced by the current tier. Resolve only policies intersecting
incoming claims/declarations, and apply those changes to the owned assembly
state. A path index or localized view can avoid scanning unrelated entries;
retain copies where state actually crosses a mutable ownership boundary. At
minimum, avoid the immediate duplicate deep copy, but removing only that copy
does not change the O(T × P) growth. Aim for work proportional to initial state,
incoming material, and affected policy rather than the complete accumulated
table per tier. Preserve pending declarations, replacement eligibility, and
independent nested record-policy ownership.
This is distinct from finding 1: one tier can avoid repeated retirement scans
and still recopy the full table at every subsequent tier. It is also distinct
from the deferred metadata-copy/public-record handoff experiment.
## 3. Tier-History Projection Repeatedly Searches Wide Mappings
Status: open. Priority: medium.
**Relevant code.** `projectTierHistoryApplications` in
[sequence_history_projection.go](../../internal/engine/sequence_history_projection.go),
lines 17–30, calls unindexed `documentAtPath` for each mapped sequence in a source
application. That helper delegates to `documentAtPathWithLookup` with no lookup;
[document_walk.go](../../internal/engine/document_walk.go), lines 51–58, then
linearly searches each mapping on the path.
**Scaling case.** A source has N sibling collections under `collections`, each
with a constant number of records. A single wildcard registration can identify
all of them. When this tier upserts/appends over lower collections, the source
application contains N sequence contributions requiring projection. Locating
each `/collections/<key>` searches the same N-entry mapping, costing
1 + 2 + ... + N entry comparisons: O(N²), despite linear-sized source material
and projection output. This does not require N application registrations.
**Potential refactoring.** Lazily build one read-only `documentLookup` for each
original application value with active mapped contributions and reuse it, or
collect the relevant sequence nodes in one traversal. With V input nodes, S
contributions, maximum depth D, and I projected items, the lookup/projection
portion can be O(V + S × D + I), rather than repeated wide scans. Preserve
authored coordinates, noncontiguous item projections, replacement evidence,
and independent output storage. This concerns load-time coordinate projection,
not the deferred branch-local explanation-replay experiment.
## 4. Compound Identity Restoration Rescans Both Records Per Field
Status: open. Priority: medium.
**Relevant code.** `preserveInheritanceIdentityFields` in
[inheritance_scope.go](../../internal/engine/inheritance_scope.go), lines 143–161,
searches the child's mapping for each identity field, then searches the merged
mapping again. `combineInheritanceTemplate` invokes it after merging the parent
payload with the child in
[inheritance_combine.go](../../internal/engine/inheritance_combine.go), line 104.
**Scaling case.** For K identity fields, C child entries, and M merged entries,
the searches cost O(K × (C + M)), plus the required identity-value clones.
A single parent/child relationship with K = C = M = N direct scalar identity
fields performs quadratic comparisons for linear-sized records and identity
metadata. There is no declared small bound on compound-identity width. The
shared identity validator and parent-identity exclusion already use mapping or
membership indexes; this restoration step does not.
**Potential refactoring.** Index merged entry positions once and walk child
entries against an identity-field set, or use equivalent operation-local mapping
indexes. This can be O(K + C + M), plus required copies. Preserve child-owned
identity values and locations, existing mapping order, and detached nodes; do
not change parent lookup or identity equality. If the step proves redundant
after the established merge, removal needs a separate equivalence review rather
than an assumption that identity preservation is unnecessary.
## 5. Secret Failure Reporting Repeats Unindexed Location Searches
Status: open. Priority: low; affects batches of failures.
**Relevant code.** `resolveSecretValue` reports each failed reference through
`secretDiagnostic` in
[secret_reference.go](../../internal/engine/secret_reference.go), lines 53–78 and
112–117. That helper calls unindexed `nearestDocumentLocation`; the mapping
branch of `nearestDocumentLocationWithLookup` in
[validation.go](../../internal/engine/validation.go), lines 144–155, falls back
to a linear search when no lookup is supplied.
**Scaling case.** An N-entry mapping of secret references whose environment
values are all absent produces N diagnostics. Each location lookup starts at
the root and scans the mapping for its key, yielding O(N²) comparisons for
O(N) references and diagnostics. Reusing the same environment name still
reproduces the lookup pattern even though environment lookup itself is cached.
Deterministic traversal and diagnostic sorting add their ordinary O(N log N)
costs; they do not require the quadratic location work.
**Potential refactoring.** Share a lazily initialized read-only `documentLookup`
for the duration of secret resolution, using `nearestDocumentLocationWithLookup`
for failures, or carry matching document/location state through traversal. For
V input nodes and F failed references at depth D, attribution can be
O(V + F × D), before existing diagnostic sorting. Keep the success path free
of an eager index if it is unnecessary. Preserve all diagnostics, nearest-known
ancestor fallback, source coordinates, and safe messages; failing fast instead
would change behavior rather than optimize it.
## 6. Dynamic Values Repeat A Type-Graph Eligibility Walk
Status: open. Priority: low; most relevant to repeated dynamic container types.
**Relevant code.** `valueOperation.dynamicDescriptor` in
[value_operation.go](../../internal/engine/value_operation.go), lines 36–48,
uses the schema descriptor cache but still calls `descriptorContainsSecret`
with a fresh visited map on every value. That helper (lines 51–68) traverses
the reachable descriptor graph. Dynamic branches in projection and cloning
invoke it for every non-nil interface value, before handling the concrete value.
**Scaling case.** Let N dynamic values have the same concrete pointer type whose
descriptor graph has size S (nodes plus member/element edges). Each call rechecks
the same graph, costing O(N × S), even after descriptor construction is cached.
For example, typed defaults can supply N interface entries holding nil pointers
to one wide, supported, secret-free struct type. Actual values and projected
output contain only N nulls, while the struct has S fields. Taking N = S makes
the repeated eligibility walk quadratic in the combined type/value description,
without requiring quadratic value expansion.
**Potential refactoring.** Remember successfully approved dynamic descriptors
by concrete type within `valueOperation`, after complete construction and the
secret-semantics check. The repeated-type case becomes O(S + N) eligibility
work, with one entry per approved concrete type. Do not treat a traversal-local
visited marker as a completed negative result for a cyclic descriptor graph.
Do not cache partial descriptors or path-bearing failures; coordinate with
finding 2 of the [correctness audit](correctness.md), which identifies failed
construction cache poisoning. Preserve rejection of dynamic secret-reference
types, including nil pointers to them. This is operation-local reuse, not the
deferred cross-load schema cache or a redesign of typed cloning.
## Validation And Recommended Order
Existing focused tests passed on Go 1.26.10, Linux amd64:
```sh
go test ./internal/engine -run 'Collection|Sequence|Inheritance|Secret|Dynamic' -count=1
go test . -run '^TestLocalDocumentationLinks$' -count=1
git diff --check
```
No benchmarks, timing comparisons, or new tests were needed to establish the
loop bounds. Passing tests do not establish efficient growth. Production code
and existing correctness findings are unchanged by this audit.
Consider findings 1 and 2 together when selecting a policy-state refactoring,
while keeping their acceptance cases distinct. Findings 3 and 4 are narrower
independent changes; findings 5 and 6 are lower-priority follow-ups. These are
opportunities for consideration, not an implementation sequence or new release
blockers. Any later refactoring should preserve the existing contracts and use
focused behavioral coverage without adding performance thresholds.

158
docs/roadmap/error-handling.md
View File

@@ -1,158 +0,0 @@
# Error-Handling Audit
Reviewed revision: `5925066d35795b2dc1d31d069a1fb23024a62ab8`.
Audit date: 2026-10-25.
## Scope And Architecture
The audit traced source construction and discovery, host/native filesystem
acquisition, import selection, grouped composition, inheritance, typed decoding
and codecs, application callbacks, secrets bootstrap, diagnostics, inspection,
and rendering/diff failure paths. It reviewed error classification, propagation,
partial state, exceptional cleanup, and the root facade's conversions against
the [configuration contract](../config.md) and existing tests.
The [implemented architecture](../internal/overview.md) separates public wrappers
from one synchronous private engine lifecycle. The document package provides the
owned tree; the YAML adapter isolates parser/dumper failures. Engine loading
collects safe structured diagnostics and returns a nil result on failure; the
facade translates engine load errors into root-owned `LoadError` values. Secrets
bootstrap is a separate operation with its own partial report and error type.
Published inspections read frozen state rather than reloading input.
One new finding warrants remediation. Previously recorded exceptional-path
defects are cross-referenced below rather than duplicated as new work.
## 1. Directory Read And Close Failures Can Be Misclassified As Absence
Status: open. Priority: medium. Category: filesystem/selection error propagation.
**Relevant code.** `filesystemAccess.readDirectory` in
[filesystem_access.go](../../internal/engine/filesystem_access.go), lines 144–184,
returns errors from opening a directory, reading its entries, and closing it
through the same unqualified error result. `visitBoundedImportDirectory` in
[directory_walk.go](../../internal/engine/directory_walk.go) forwards that error.
Both `importDirectoryEntries` in
[import_selection.go](../../internal/engine/import_selection.go), lines 295–306,
and `readGlobEntries` in [glob.go](../../internal/engine/glob.go), lines 187–198,
interpret any resulting `fs.ErrNotExist` as an absent directory and discard its
entries without a diagnostic.
**Failure condition.** A directory has been successfully inspected and opened,
but its `ReadDir` fails with an error wrapping `fs.ErrNotExist`, possibly together
with entries already read. The same issue occurs if enumeration succeeds but
`Close` returns that classification. A caller-supplied `fs.FS` can legitimately
encounter such failures during its backing operations. The helper loses the
distinction between an absent target at lookup/open and a failure after opening.
**Expected behavior.** Preserve the existing
[absence and requiredness rules](../config.md#explicit-imports) for genuinely
missing targets, while reporting a safe load failure when enumeration fails
after opening. A close failure that the directory helper elects to report must
not become an empty successful selection just because of its error identity.
This follows the existing distinction between absence and failed acquisition;
the regular-file helper in
[source_file.go](../../internal/engine/source_file.go) already restricts its
`missingTarget` flag to preflight/open failures rather than subsequent reads.
**Actual behavior and consequence.** The failed directory is treated as though
it contributed no files. Optional loading can succeed with an incomplete
configuration. Required directory/glob selection can also succeed if another
directory supplies a match, since requiredness is checked against the surviving
combined selection. If it was the only directory, the outcome is either a
successful optional empty selection or a misleading `import.missing` error,
rather than the acquisition failure.
**Evidence.** A standalone public-API diagnostic used a borrowed filesystem
backed by `fstest.MapFS` with these files:
```text
main.yml
bad/b.yml # b: 2
good/a.yml # a: 1
```
The main input selected either `dir: [bad, good]` or `glob: '*/*.yml'`, with
`required: true`. The filesystem kept metadata and file opens working and
wrapped only the opened `bad` directory. Its wrapper returned an injected error
either alongside its `ReadDir` entries or from `Close` after enumeration.
For both selectors and both failure phases, `fs.ErrNotExist` produced a
successful load with `a=1, b=0`. Replacing only the injected error with
`fs.ErrPermission` failed loading (`import.invalid` for directory search and
`import.permission` for glob search). All eight small cases were executed; no
repository test or production code was changed. The filesystem still contained
the omitted file, so this was not a missing-target fixture.
**Potential remediation.** Preserve failure phase across the shared directory
boundary, for example with a small private result/failure record distinguishing
missing-on-open from read/close failure, analogous to `sourceReadFailure`.
Keep the existing preflight and open-time missing-directory behavior, entry
budgets, first-error precedence, and caller-owned filesystem lifetime. Map later
failures to the existing safe traversal diagnostics without exposing raw backing
error text. Review the standard `fs.ReadDir` fallback as well as the batched
`fs.ReadDirFile` path. No retry policy, filesystem transaction, rollback service,
or new restriction on supported filesystems is needed.
## Existing Findings Relevant To Error Handling
These remain open in their original owners; this audit creates no duplicate
remediation requirement:
- [Correctness finding 1](correctness.md#1-pointer-only-recursive-types-can-overflow-the-stack-or-hang):
pointer-only recursive descriptors can turn incompatible input or a helper
query into stack exhaustion/nontermination instead of a safe failure.
- [Correctness finding 2](correctness.md#2-failed-dynamic-schema-construction-leaves-a-reusable-partial-descriptor):
failed dynamic descriptor construction leaves partial cache state; a later
helper in the same callback can panic instead of returning the recorded load
diagnostics. Error-state rollback belongs to that finding.
- [Correctness finding 4](correctness.md#4-callback-contexts-remain-active-after-panic-unwinding)
and [resource finding 3](resources.md#3-an-imported-version-check-panic-leaves-the-host-root-open):
panic unwinding bypasses callback deactivation and imported host-root cleanup.
Application panics should still propagate; neither finding requests recovery
or publication of partial results.
- [Resource findings 1 and 2](resources.md): oversized typed-container allocation
can panic before a quota diagnostic, and incomplete history accounting can
omit an established limit failure.
## Deliberate Behaviors Preserved
- Raw parser, reader, codec, filesystem, and returned callback error text is
intentionally suppressed at the loading boundary. Stable codes, safe messages,
semantic paths, and available locations provide context; adding raw error
chains would conflict with the [safe-failure contract](../config.md#safe-failures).
- Version/resolution callbacks that return an error without reporting one receive
a generic hook-failed diagnostic. Warnings alone do not suppress that failure.
Application callback panics remain programming failures.
- Read-only regular-file close errors are explicitly ignored by the shared
helper. Cleanup failures during an existing primary failure do not need to
replace that primary failure. These choices are not findings by themselves.
- Secrets bootstrap stages reads before writes and documents that an apply
failure leaves earlier writes in place, reports the failed entry, and leaves
later entries unapplied. That partial-state reporting is intentional, not a
missing rollback guarantee.
- Effective rendering, diagnostic rendering, explanation output, and bootstrap
reports propagate writer failures, including short writes; partial output is
permitted. Internal snapshot/diff serialization uses safe errors or redacted
fallback snapshots rather than exposing internal serialization details.
- ConfigKit owns no background cancellation, timeout, retry, or shutdown service.
Synchronous application readers/callbacks can block; this audit does not invent
a cancellation guarantee or move their resource ownership into ConfigKit.
## Validation
The following passed with Go 1.26.10 on Linux amd64, using cached dependencies
with network/toolchain acquisition disabled:
```sh
go test ./... -count=1
go vet ./...
go test ./internal/engine -run 'Failure|Failures|Error|Errors|Preflight|Propagates|HookOrder' -count=1
go test ./internal/yaml -run 'Safe|Writer|Malformed' -count=1
```
The standalone fault-injection diagnostic ran outside the repository. No
benchmarks, new repository tests, production changes, or native macOS/Windows
validation were performed. Passing existing tests does not cover the newly
identified phase-classification case. This report adds no public contract,
security hardening requirement, or release gate.

189
docs/roadmap/resources.md
View File

@@ -1,189 +0,0 @@
# Resource Management Audit
Reviewed revision: `0cca633a67abd3d0fdcb7db6e83260dcaf1eaac8`.
Audit date: 2026-10-25.
## Scope And Architecture
The audit covered input snapshots and readers, discovery, filesystem acquisition,
imports and selected overlays, typed projection and cloning, composition and
inheritance, callback teardown, retained inspection history, rendering/diff, and
secrets bootstrap. Knowledge-graph discovery and caller tracing were supplemented
by source inspection, existing tests, and small standalone diagnostics outside
the repository. No benchmarks or large-memory stress tests were conducted.
The [implemented architecture](../internal/overview.md) has a mostly clear
resource-ownership model:
- The public facade delegates to a synchronous engine lifecycle, with private
document and YAML packages. Loading creates no library-owned background
goroutines, timers, network connections, or shutdown service.
- Ordinary file reads close owned handles, including read/stat failures. Host
import and overlay acquisition own their root handles; caller-supplied readers
and filesystems remain borrowed.
- Published inspection retains immutable documents, history, indexes, and passive
resource contexts. A later explicit resource open transfers ownership of that
returned file to the caller; publication does not normally retain acquisition
handles.
- Retained history and detached query results are intentional ownership choices,
not leaks by themselves. The findings below concern enforcing existing limits
and closing owned resources, rather than removing that functionality.
Three new findings warrant remediation. All remain open.
## 1. Typed Containers Allocate Before Their Node Budget Is Enforced
Priority: medium. Category: peak memory and failure handling.
**Relevant code.** `projectSequence` and `projectMap` in
[projection.go](../../internal/engine/projection.go), `valueOperation.charge` in
[value_operation.go](../../internal/engine/value_operation.go), and the defaults
projection in [decode.go](../../internal/engine/decode.go).
**Circumstances.** Sequence projection reserves `value.Len()` document pointers
before visiting any children. Map projection similarly reserves the complete
entry array and obtains all map keys before charging individual entries. The
existing node limit therefore rejects an oversized typed value only after a
potentially much larger allocation has already occurred. Defaults and values
returned by application resolution can reach these paths.
**Evidence.** A standalone public-API diagnostic loaded `NoSource()` with defaults
containing an `Items []struct{}` field initialized with
`make([]struct{}, int(^uint(0)>>1))`. Zero-sized elements make the input slice
inexpensive, but projection immediately panicked with
`runtime error: makeslice: cap out of range`. It did not return the existing
`schema.projection_limit` diagnostic. No enormous backing array was successfully
allocated by this diagnostic.
**Consequence.** Less extreme oversized containers can cause excessive temporary
memory allocation before rejection; sufficiently large lengths panic or exhaust
memory instead of reaching the intended limit failure. This is a library-created
allocation, distinct from memory an application has already allocated itself.
**Potential remediation.** Before bulk reservations, compare the known minimum
node cost of the container against the operation's remaining existing allowance,
using overflow-safe arithmetic. Alternatively, bound initial capacity and grow
only as successfully charged elements are accepted. Review the analogous map
reservation/key collection. Preserve current node/depth semantics, inline-field
behavior, and useful diagnostic paths; do not introduce an unrelated container
length cap or promise a universal heap-size bound.
## 2. Retained-History Accounting Omits Published Replay Trees
Priority: medium. Category: retained memory and lifecycle integration.
**Relevant code.** The history check in `executeLoadRequest` in
[load.go](../../internal/engine/load.go), `historyNodesWithin` in
[history.go](../../internal/engine/history.go), `preparePostDecodeOverrideContext`
in [override_context.go](../../internal/engine/override_context.go), and
`resolveLoadInput` / `publishLoadResult` in
[load_result.go](../../internal/engine/load_result.go).
**Expected behavior.** The
[retained-history contract](../config.md#historical-value-explanation) limits retained
history to 4,000,000 distinct document nodes, counting shared nodes once and
reporting `explain.limit_exceeded` when exceeded.
**Actual behavior.** The load check visits the original decoded baseline and
pre-CLI final tree, the resolved final tree, application trees, resolution deltas,
and the acquisition inheritance scope. Publication retains a different root set:
it includes a separately cloned post-CLI decoded tree as inspection input, plus
independent `structuralInput` and `preCLI` replay snapshots. The replay snapshots
are cloned again during publication and included in its immutable lookup. They
are not passed to the budget check. Inheritance operation copies are also made
after that check. Counting the earlier root set does not bound the distinct
nodes actually kept alive by the result.
**Evidence.** A successful public load of `items: [1, 2]`, decoded into
`struct { Items []int }` with a no-op resolver, was inspected by a read-only
standalone reflection diagnostic. Its baseline/input/final/application roots
contained 14 distinct nodes; including the two published replay roots yielded
22. The checkpoint uses a different decoded-final instance of the same size in
this example. Source inspection establishes that its count is also 14.
For this fixture generalized to N scalar items, the checkpoint counts `3N + 8`
nodes, while the published roots retain `5N + 12`. Thus 800,000 items would pass
the history checkpoint with 2,400,008 nodes yet retain 4,000,012. Such a source
also fits the existing per-source byte and node allowances. This larger case is
a structural derivation, not an executed allocation test.
**Consequence.** Successful loads can exceed the established retained-history
limit, with additional retained lookup/index overhead. The quota currently gives
an incomplete account of the memory-owning snapshot graph.
**Potential remediation.** Apply one pointer-aware accounting operation to the
complete set of document roots actually retained at publication, including replay
snapshots and copied inheritance history. Preserve count-once semantics for
shared nodes. Where practical, charge or reserve these copies before creating
them so failure does not first incur avoidable peak allocation. Reusing an
already-owned immutable snapshot can be considered where equivalent, but this
finding does not require the broader metadata-copy redesign deferred in
[future experiments](future.md#3-performance-experiments), weaker detachment, a
smaller limit, or removal of historical inspection.
## 3. An Imported Version-Check Panic Leaves The Host Root Open
Priority: medium. Category: file-descriptor cleanup during unwinding.
**Relevant code.** `acquireImportedInput` / `acquireImportedWithAccess` and imported
version processing in
[import_acquisition.go](../../internal/engine/import_acquisition.go), together with
`openHostAccess` / `filesystemAccess.close` in
[filesystem_access.go](../../internal/engine/filesystem_access.go). Compare the
deferred fallback cleanup in
[selected_overlay.go](../../internal/engine/selected_overlay.go) and deferred file
cleanup in [source_file.go](../../internal/engine/source_file.go).
**Circumstances.** Host import acquisition opens an `os.Root`, then closes it
explicitly on ordinary failure paths and after import traversal. It has no
deferred fallback closure. A user version-check callback runs for imported
sources while that root is open. If it panics and the application recovers outside
`Load`, unwinding bypasses all those explicit root closes. The imported file's
own deferred close still runs; the outstanding resource is the directory/root
handle.
**Evidence.** A standalone Linux diagnostic created a temporary main file and one
import, both with a version, enabled source-version checks, and panicked only on
the imported source's check. With automatic GC disabled only in that disposable
diagnostic process, the number of `/proc/self/fd` entries targeting the temporary
root went from zero to one after external recovery. The non-panicking control
ran both checks and returned to zero. One outstanding root was sufficient to
demonstrate the issue; no descriptor-exhaustion workload was run.
**Consequence.** Library-owned root cleanup depends on eventual runtime cleanup
instead of load-scope unwinding. Repeated recovered callback failures can retain
descriptors long enough to exhaust them. This does not imply that ConfigKit must
recover application panics or successfully return a configuration after one.
**Potential remediation.** Install a deferred fallback close at the owning
acquisition boundary, while preserving normal explicit close timing and close
error diagnostics, as the selected-overlay path already does. Avoid double-close
confusion and leave borrowed native filesystems untouched. Preserve panic
propagation. This is separate from
[correctness finding 4](correctness.md#4-callback-contexts-remain-active-after-panic-unwinding):
deactivating a callback context alone does not close the root, and closing the
root alone does not deactivate that context.
## Validation And Boundaries
The following passed with Go 1.26.10 on Linux amd64, using cached dependencies
with automatic toolchain/network acquisition disabled:
```sh
go test ./internal/engine -run 'Closes|Close|Budget|Limits|Limit|SecretsDir' -count=1
go test ./... -count=1
```
The three standalone diagnostics described above were run outside the repository;
they added no production code or repository tests. The history diagnostic only
read private fields to count pointers; it did not mutate inspection state. No
native macOS or Windows checks were performed.
No additional network, timer, goroutine, buffer-pool, or ordinary-return handle
leak was established. Borrowed blocking readers/callbacks do not create a new
cancellation guarantee. Previously reported recursive-schema stack exhaustion
and callback-state retention remain owned by the
[correctness audit](correctness.md), rather than duplicated here. Existing
algorithmic and maintainability findings remain owned by their respective
audits. This report adds no API, release gate, new resource quota, or background
management requirement.

194
docs/roadmap/security.md
View File

@@ -1,194 +0,0 @@
# Security Audit
Reviewed revision: `b30c9f167dba16a9192d9bfde4aeb2b9c597ce0e`.
Audit date: 2026-10-25.
## Scope And Trust Model
This review examined input validation and budgets, import/discovery containment,
link resolution, collection authority, application callbacks, secret references
and bootstrap, historical sensitivity, diagnostics, effective rendering and
diffs, and the runtime/development dependency boundary. Knowledge-graph searches
and caller traces were supplemented by source inspection, existing tests, a
bounded standalone diagnostic, and an advisory-database spot-check.
The [architecture](../policy/architecture.md) and
[configuration contract](../config.md) establish these boundaries:
- ConfigKit is a synchronous startup library, not a network service or an
authentication/authorization framework. Applications select inputs, roots,
filesystem implementations, callbacks, and credential use. Applications own
remote acquisition and authentication of that data.
- The public facade delegates to a private engine and private document/YAML
packages. Input composition and inspection do not execute shell commands or
launch a background service. Ordinary loading does not change the process
environment; explicit secrets-directory bootstrap is the narrow exception.
- Host imports use canonical containment checks and owned rooted access under
the documented stable-input assumption. Borrowed native filesystems retain
their own behavior and are explicitly not a general sandbox guarantee.
- Secret references resolve after composition and application resolution. The
trusted returned configuration can expose credentials through `Value()`;
inspection retains names rather than credentials. Tags, a path predicate,
inheritance, and recorded dependencies govern redaction of other values.
Application-supplied messages and unrecorded secret transformations remain
application responsibilities.
One new availability finding warrants remediation. No new credential disclosure,
command injection, application-authority bypass, or host containment escape was
established. This is a source-level audit with bounded checks, not a certification
that every input, dependency, platform, or deployment is secure.
## 1. Finite Symlink Reuse Can Cause Exponential Import-Resolution Work
Status: open. Priority: medium. Category: availability / input amplification.
**Relevant code.** `resolveHostImportPath` and `resolveNativeImportPath` in
[filesystem_links.go](../../internal/engine/filesystem_links.go), lines 129–242,
expand link-target components with a frame stack and track only currently active
links. Completion removes a link from that set; a later use resolves its entire
target again. No completed-target result is reused.
`importEntryTarget` invokes these resolvers for discovered links. Both
[directory selection](../../internal/engine/import_selection.go) and
[glob selection](../../internal/engine/glob.go) reach it. Directory selection
resolves the target before deciding whether recursive descent is enabled, so
`recursive: false` does not avoid this work. Native resolution is used when the
borrowed filesystem provides `fs.ReadLinkFS`; host resolution uses the analogous
rooted implementation.
**Plausible scenario.** An application loads a configuration tree whose imported
assets can be supplied by a less-trusted user or configuration provider. That
provider controls link entries inside an allowed import directory. The
application's own main file need only select that directory with link following
enabled, which is the current default. No concurrent filesystem mutation,
outside-root access, executable callback, or malicious filesystem implementation
is required. This is conditional on influence over imported filesystem data;
it is not an anonymous remote vulnerability in ConfigKit by itself.
Inside an ordinary `links` directory, create this finite graph:
```text
s00 -> .
s01 -> s00/s00
s02 -> s01/s01
s03 -> s02/s02
...
```
Each link resolves to the same contained directory. It has no active link cycle:
the first reference to a lower link finishes before the second begins. Resolving
level N nevertheless repeats the level N-1 expansion twice, giving a recurrence
of `T(N) = 2T(N-1) + O(1)` component work for this fixed-name fixture. Directory
enumeration contains only N+1 entries, and no imported configuration nodes or
new import edges account for those repeated expansions. Consequently, source,
entry, input-node, and active-import-depth budgets do not bound this work by the
size of the selected graph. This is repeated resolution of a compact graph, not
genuinely expanding configuration output or ordinary deterministic sorting.
**Evidence.** A standalone public-API diagnostic created real temporary symlinks
and exposed them through `NewFilesystem` backed by `os.DirFS`. A thin
`fs.ReadLinkFS` wrapper counted metadata/link operations and imposed a diagnostic
ceiling of 12,000 `Lstat` calls. The main input selected `dir: links` with link
following enabled and default nonrecursive discovery. Successful loads produced
these counts:
| Link entries | `Lstat` calls | `ReadLink` calls |
| --- | ---: | ---: |
| 3 | 32 | 11 |
| 5 | 128 | 57 |
| 7 | 512 | 247 |
| 9 | 2,048 | 1,013 |
| 11 | 8,192 | 4,083 |
A 13-entry fixture reached the diagnostic ceiling and returned a safe load
failure when the wrapper stopped it. The same fixture with `follow: false`
completed with 17 `Lstat` calls and zero `ReadLink` calls. The operation counts
substantiate amplification in the native resolver; the host resolver's matching
frame-expansion logic was verified by source inspection and caller tracing,
not a separate host operation-count test. No timings, large attack graph, or
external-system exploitation were used.
**Impact.** A small imported asset tree can monopolize startup/load execution for
an impractically long period. Applications that expose configuration validation
or repeated loading to less-trusted providers can also tie up their own workers.
The normal trusted-operator-only deployment reduces the adversarial exposure,
but accidental alias graphs can cause the same availability failure.
**Potential remediation.** Reuse fully resolved link-target results within the
resolution operation, keyed by the appropriate rooted namespace and full link
path, while retaining active-path cycle detection and containment checks.
Equivalent dependency-based evaluation is another option. Preserve legitimate
finite reuse, relative/absolute host-link semantics, and the stable-input model;
do not replace the active set with a global "already visited means cycle" rule.
Review both resolvers and directory/glob entry points. The objective is to avoid
re-expanding completed dependencies, not to introduce a public cache or reject
valid graphs with an arbitrary link-count cap. Consumers can already opt out of
following discovered links where appropriate; changing that default is not part
of this recommendation.
## Previously Reported Risks
The [correctness audit](correctness.md) already records pointer-only recursive
schema nontermination, failed dynamic-descriptor cache state, and callback guards
left active during panic unwinding. The [resource audit](resources.md) owns
pre-budget typed-container allocation, incomplete retained-history accounting,
and an import-root cleanup gap after callback panic. These can affect availability
when their documented preconditions hold; they are not newly discovered remote
exploits or duplicate remediation tasks here.
The [error-handling audit](error-handling.md) separately owns silently omitted
directory contributions after certain enumeration/close failures. The
[efficiency audit](efficiency.md) owns previously identified avoidable batch costs.
This review does not reclassify every expensive operation or application
programming error as a security vulnerability.
## Dependency Spot-Check
The ordinary package's non-standard-library dependency graph contains only
`go.yaml.in/yaml/v4 v4.0.0-rc.6`; ReleaseKit, Goldmark, and `x/mod` are maintainer
tool dependencies rather than ConfigKit runtime imports. Cached module integrity
verification passed.
The current [Go vulnerability database module index](https://vuln.go.dev/index/modules.json)
contained no entries for the selected YAML v4, ReleaseKit, or Goldmark v2 module
paths. Its two `golang.org/x/mod` reports,
[GO-2026-6179](https://pkg.go.dev/vuln/GO-2026-6179) and
[GO-2026-6180](https://pkg.go.dev/vuln/GO-2026-6180), identify v0.40.0 as fixed,
which is the version selected here. The related toolchain fixes also precede the
Go 1.26.10 toolchain used for this audit. The database metadata reported a modified
time of `2026-10-21T00:27:29Z` when checked.
This was a module-path/advisory spot-check, not a complete standard-library,
toolchain, transitive-reachability, or supply-chain attestation. `govulncheck`,
`gosec`, and `gitleaks` were not installed; no scanners were added to the project.
Absence of an advisory is not proof of absence of vulnerabilities, and
`go mod verify` does not audit dependency behavior or establish that upstream
content is safe.
The [database API documentation](https://go.dev/doc/security/vuln/database)
describes the advisory source used for this check.
## Validation And Preserved Boundaries
The following passed with Go 1.26.10 on Linux amd64 using cached dependencies and
disabled automatic dependency/toolchain acquisition:
```sh
go test -race ./... -count=1
go test ./internal/engine -run 'Secret|Sensitive|Sensitivity|Contain|Escap|Outside|Symlink|Nonregular|Budget|Limit|Diagnostic' -count=1
go vet ./...
go mod verify
```
These checks include existing canary-based non-disclosure, historical and
bilateral diff sensitivity, safe diagnostic, parser-policy, containment, and
nonregular-file cases. The bounded link diagnostic ran outside the repository.
No production code or repository tests were added or changed; no native
macOS/Windows run, active external exploitation, or exhaustive fuzzing was done.
This report does not propose authentication in the library, arbitrary environment
name restrictions, secret-file permission requirements, retries, memory erasure,
or protection against privileged concurrent filesystem mutation. Those would
need their own concrete requirement and threat analysis. It preserves borrowed
filesystem ownership, application-owned safe callback messages, public secret
reference names, explicit partial bootstrap semantics, and the current release
gates.

281
docs/roadmap/testing.md
View File

@@ -1,281 +0,0 @@
# Test Coverage And Effectiveness Audit
Reviewed revision: `eb5a82f1f3a3286506c2d527e7980db7ccae5398`.
Audit date: 2026-10-25.
## Assessment And Architecture
The suite broadly follows the [testing policy](../policy/testing.md): real
parsing, composition, decoding, and filesystem fixtures dominate; narrow doubles
control I/O and environment failures. Engine tests own mechanisms and semantics,
root tests protect public conversions and integration, and maintained consumers
exercise representative workflows. The private document tree and YAML adapter
support the synchronous engine behind the root-owned public facade; see the
[component overview](../internal/overview.md).
Strengths include presence/default/zero distinctions, strict composition versus
precedence, collection policy, inheritance/history agreement, safe rendering and
bilateral diff sensitivity, independent loads, reader claims, callback ordering,
bootstrap partial failures, and realistic source/containment behavior. Parser and
path fuzz targets have deterministic seeds in the ordinary suite. Architecture,
documentation, and development-check tests protect explicit repository contracts.
Six targeted findings follow: four coverage/assertion improvements and two areas
for removal or consolidation. They do not justify a wholesale test purge, a
percentage target, or another test for every private helper. No new production
defect was established during this pass; the existing audits own their findings.
## Coverage Evidence
On Go 1.26.10, Linux amd64, the ordinary suite listed 771 `Test` functions, three
fuzz targets, and 29 executable examples. These counts exclude Windows-only
compiled tests and do not count table cases as separate test functions.
Cross-package coverage, aggregated by source block across all test binaries, was:
| Package | Covered statements | Coverage |
| --- | ---: | ---: |
| Root public package | 348 / 428 | 81.3% |
| `internal/engine` | 9,089 / 9,987 | 91.0% |
| `internal/document` | 13 / 13 | 100.0% |
| `internal/yaml` | 151 / 168 | 89.9% |
| Maintained consumer examples | 259 / 327 | 79.2% |
| Total | 9,860 / 10,923 | 90.3% |
The document package has no standalone tests, but its code is covered through
real collaborators. The per-test-binary percentages printed by `-coverpkg=./...`
have a different denominator and must not be mistaken for these aggregate
package percentages. Statement coverage measures execution, not assertion
strength or state-space coverage: the current suite passes despite the defects
already recorded in the correctness, resource, error-handling, and security audits.
Some zero-coverage private functions have no production caller; trivial public
preset forwards are also unexecuted. Neither observation alone warrants adding
tests. Prioritize the behavioral risks below over filling those percentages.
## 1. Confirmed Exceptional Cases Lack Durable Regression Coverage
Status: open. Priority: medium. Category: gaps in state transitions and boundaries.
The earlier audits reproduced defects outside the checked-in suite. Relevant
tests exercise nearby ordinary cases but do not yet protect these transitions:
- **Schema and dynamic-value failure state.**
[schema_test.go](../../internal/engine/schema_test.go) covers ordinary recursive
structs; [projection_test.go](../../internal/engine/projection_test.go) checks
independent invalid values and cycles. Neither covers pointer-only recursive
descriptors, acyclic slice views sharing a starting pointer, or a second
dynamic/helper operation after a failed descriptor build in the same load.
Add the narrowly scoped regressions described by
[correctness findings 1–3](correctness.md), retaining null/acyclic success cases
and asserting safe classification and termination. Avoid an unbounded
stack-overflow/hang fixture in the default test process.
- **Exceptional callback exit.** The retained-context tests in
[resolution_test.go](../../internal/engine/resolution_test.go) and
[facade_context_test.go](../../facade_context_test.go) verify normal callback
return. File-close tests cover ordinary load failures. They do not exercise
external recovery after version/resolution/validation callback panic or an
imported version check while a host root is owned. Extend the existing owners
when fixing [correctness finding 4](correctness.md#4-callback-contexts-remain-active-after-panic-unwinding)
and [resource finding 3](resources.md#3-an-imported-version-check-panic-leaves-the-host-root-open).
Preserve propagation of the original panic and verify closure of the callback
state and the actually owned root independently; a native file-close counter
alone cannot verify an `os.Root` lifetime.
- **Budgets before allocation and through publication.**
`TestProjectionRejectsNonFiniteLimitsAndCodecFailuresSafely` checks a small
scalar tree against supplied limits. `TestHistoryNodeBudgetCountsSharedTreesOnce`
and the inheritance-history budget tests verify the accounting helper with
explicitly supplied roots. They cannot detect a container reservation before
charging, or roots omitted by the publication caller. Add bounded regressions
with [resource findings 1–2](resources.md): oversized zero-sized-element input
can avoid a huge input allocation, and a small controlled history allowance can
test the complete retained-root boundary. Keep count-once sharing coverage;
do not allocate millions of nodes just to cross the production default.
- **Nested finite link expansion.**
[filesystem_links_test.go](../../internal/engine/filesystem_links_test.go) and
the public finite-reuse test cover simple reuse and actual cycles, but not the
compact repeated-dependency graph in [security finding 1](security.md#1-finite-symlink-reuse-can-cause-exponential-import-resolution-work).
When that remediation is selected, use a small real link fixture and a bounded
operation observer at the filesystem boundary. Protect finite reuse and
nonrecursive discovery without running an exhaustion workload or adding a
wall-clock performance threshold.
These are test accompaniments to the original remediation owners, not duplicate
product fixes or a request for a broad new cross-product matrix. The existing
tests should remain where they protect distinct ordinary behavior.
## 2. Public Historical-Path Enumeration Is Not Exercised
Status: open. Priority: medium. Category: public facade integration.
**Relevant code/tests.** `Metadata.HistoricalPaths` in
[metadata.go](../../metadata.go), lines 49–56, has zero executed statements in
the aggregate profile. Every checked-in `HistoricalPaths` test call is in the
engine package. `TestMetadataHistoricalPaths` in
[metadata_test.go](../../internal/engine/metadata_test.go) thoroughly covers the
engine method, including removed descendants, escapes, contextual numeric
tokens, and detached results, but does not cross the public conversion.
**Risk.** Returning no paths or incorrectly converting the numeric segment intent
at the root boundary could leave that engine suite green while breaking the
documented consumer enumeration/query workflow. Coverage of `History` and
`Lookup` separately does not exercise enumeration's result conversion.
**Improvement.** Add one small external-package public-API test that enumerates
effective and removed paths, then uses the returned paths with `Lookup` and
`History`, including a numeric-key/sequence shape change. Check sorted unique
pointer spelling, zero metadata, and replacement of a returned slice element
without affecting later queries. Keep detailed path mutation and engine shape
cases with their existing owner; do not duplicate the complete internal matrix
or expose private segments to test the facade.
## 3. Native-Filesystem Failure Tests Observe Too Little
Status: open. Priority: medium. Category: doubles and inadequate assertions.
**Relevant tests.** `TestNativeFilesystemClosesFilesOnSuccessAndFailure`,
`observedFilesystem`, and `observedFile` in
[native_filesystem_test.go](../../internal/engine/native_filesystem_test.go),
especially lines 150–162 and 278–319. The fixture is also used by
`TestLoadingClosesNativeFilesAcrossPipelineOutcomes` in
[resource_context_test.go](../../internal/engine/resource_context_test.go).
**Deficiencies and plausible survivors.**
- Failure cases require only a non-nil error, not a nil result, the expected
diagnostic classification, or evidence identifying the intended failed
source. An unrelated early failure could satisfy the assertion.
- The non-disclosure check searches only `err.Error()` for the fixture canary.
`LoadError.Error` deliberately renders an error count, not diagnostic messages.
Accidentally forwarding the raw filesystem error into `Diagnostics()` could
therefore pass this check. The parser/callback canary tests cover different
failure-producing boundaries.
- `assertClosed` checks only total opens versus total `Close` calls. The counter
increments even for a repeated close, so one leaked handle and one double-close
can cancel out. It does not prove that each acquired handle was released.
- The double supplies immediate generic read/directory errors and successful
closes. It cannot model entries/data plus an error, or a close error whose
identity resembles absence. This leaves the
[directory failure-phase defect](error-handling.md#1-directory-read-and-close-failures-can-be-misclassified-as-absence)
unprotected on both the batched and fallback paths.
**Improvement.** Keep the real in-memory filesystem backing and small controlled
failure seams. Track each returned handle's close state, assert the relevant
structured load error and nil result, and inspect diagnostics/rendered diagnostics
for the canary. Add only the read/close phase cases needed by the existing
error-handling remediation, including a second successful directory so required
selection cannot mask the failed one. Preserve intentionally ignored read-only
regular-file close errors; do not invent a universal close-error contract.
## 4. Fuzz Detachment Assertions Miss Shared Descendants
Status: open. Priority: low. Category: ineffective ownership assertion.
**Relevant target.** `FuzzCompositionAndOverlay` in
[composition_fuzz_test.go](../../internal/engine/composition_fuzz_test.go),
lines 46–49 and 63–66, appends a new root mapping entry and then compares inputs
against their earlier copies to establish result detachment.
**Deficiency.** A shallow-copied root can still share existing entries, descendant
nodes, or sequence backing storage. Appending through the copied root's slice
header changes neither the input's header nor its existing elements; even reuse
of the backing array writes beyond the input's old length. The comparison can
pass while the result still shares mutable descendants. It primarily detects
aliasing the root object itself, not the deeper ownership property in its error
message.
**Improvement.** Within the existing bounded target, mutate an existing result
descendant or an element inside the original slice length, then check both
inputs. Keep empty-container cases separate and retain the existing input
immutability and commutativity properties. Deterministic tests in
[compose_test.go](../../internal/engine/compose_test.go),
[overlay_test.go](../../internal/engine/overlay_test.go), and
[recursive_merge_test.go](../../internal/engine/recursive_merge_test.go) already
give meaningful nested-mutation protection; improve the fuzz assertion rather
than adding a parallel matrix or claiming all ownership coverage is absent.
## 5. Remove Exact Stat-Call Choreography From The Ordering Test
Status: open. Priority: low. Category: removal/rewrite candidate.
`TestSelectNamedImportsChecksNormalizedPathsLexically` in
[import_selection_test.go](../../internal/engine/import_selection_test.go),
lines 151–166, asserts the exact trace `a.yaml,a.yaml,z.yaml,z.yaml`.
Lexically ordered target handling is meaningful. Performing exactly two `Stat`
calls per target is not an established requirement: reusing successful preflight
metadata or changing a filesystem helper could preserve selection, safe failure,
and ordering while failing this test. The selected-candidate assertion meanwhile
checks only the count, not the order or paths.
Remove the duplicated-stat trace expectation. Check ordered candidate paths and,
if first-failure order needs separate protection, the selected diagnostic site
using two failing targets. Consolidate that case into the existing selection
tests and delete this standalone test if its distinct protection is then
subsumed. Keep meaningful interaction assertions such as not opening excluded
file contents; those protect an actual resource/selection behavior.
## 6. Consolidate Redundant And Representation-Coupled Facade Scaffolding
Status: open. Priority: low. Category: removal/consolidation candidates.
- In [facade_compatibility_test.go](../../facade_compatibility_test.go), the
six-type package-ownership loop in `TestFacadeCompatibility` is a subset of
the comprehensive `TestFacadeValuesRemainRootOwned` loop in
[facade_candidate_test.go](../../facade_candidate_test.go). Remove the duplicate
loop while retaining the compile-time interface checks, public construction,
and any distinct lifecycle behavior. The runtime assertion that a freshly
constructed `Result.Config` still equals its literal field value adds no
protection beyond ordinary Go assignment and compilation.
- `TestWrappedPathsRetainEngineSegmentIntent` in
[facade_internal_test.go](../../facade_internal_test.go) compares private
`.value` representations for inequality. Distinct representations are not the
consumer contract; correct contextual mapping/index lookup is. Replace its
protection with public query behavior, coordinated with finding 2, then remove
the private-representation test. Keep the engine's own segment-intent tests.
- In that same file, `TestWrappedSourcesPreserveEngineOwnership` invokes
`engine.Load` directly through `source.toEngine()`. Its byte snapshot and
copied-reader risks are legitimate, but the facade test should use public
`Load` and root-owned errors. Consolidate overlapping reader behavior with
`TestRepeatedImportsAcrossTiersAndReaderConsumption` in
[facade_equivalence_test.go](../../facade_equivalence_test.go), preserving
explicit copied-source and byte-mutation coverage before deleting scaffolding.
- The clone portion of `TestSourceInfoDestinationConversionsAndCopies` in
[records_test.go](../../records_test.go), lines 39–42, replaces the whole copied
`Destination` field. A shallow struct copy passes this without proving nested
detachment. Remove that non-protective assertion or consolidate the relevant
returned-record ownership check into existing public source-snapshot tests;
retain conversion checks that protect fields at the facade boundary. `Path`
is opaque and immutable through its public builders, so exposing internals or
adding a new deep-copy guarantee is not justified by this test.
This is a small cleanup, not a recommendation to remove root integration tests
because the engine has tests. Public conversion, error typing, callback bridging,
source namespace identity, disclosure, and returned-record ownership have real
cross-package risks. The static public/dependency-boundary checks also protect
the expressly selected architecture and should remain.
## Validation And Limits Of This Review
The following passed using cached dependencies with automatic network/toolchain
acquisition disabled:
```sh
go test ./... -coverpkg=./... -covermode=atomic -coverprofile=/tmp/configkit-testing-audit.ZNsVuW/coverage.out -count=1
go tool cover -func=/tmp/configkit-testing-audit.ZNsVuW/coverage.out
go test ./... -shuffle=on -count=3
```
The profile stayed outside the repository. Coverage blocks were combined across
test binaries before calculating package totals. A separate JSON test run found
no individual skipped test/subtest on this Linux host; `internal/document` was
reported as having no standalone tests. This is not native macOS/Windows evidence.
Only the checked-in fuzz seeds were run; no extended fuzz campaign or mutation
testing was performed. Assertion weaknesses above are grounded in source-level
counterexamples, not a claimed measured mutation score.
No production code, tests, fixtures, dependencies, or release gates were changed.
Recommended additions should accompany the relevant fixes and existing test
owners. Use small controlled budgets and independent outcomes, retain selected
stable formatting/code/path contracts, and avoid making implementation details
or speculative guarantees into new policy through assertions.