Files
ConfigKit/docs/roadmap/audit.md

56 KiB
Raw Blame History

Consolidated Audit Review

Status: initial scope selected; efficiency and structural-refactoring scope remain under review. Implementation has not begun. 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, public Go declarations, and architecture policy 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 findings remain unresolved; the settled scope decision identifies the selected work. OQ-2 and OQ-3 remain open. 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:

git show ecd5d5d:docs/roadmap/correctness.md
git show ecd5d5d:docs/roadmap/security.md

The 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 the confirmed unused copies, 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, a legal type such as type Loop *Loop produces a pointer descriptor whose element is itself. 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 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 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 and 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 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.

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 reserve space for the full container, and maps collect all keys, before child charging in 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 and runValidation / reportVersion in 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 opens an os.Root through 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.

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 returns open, read, and close errors through one unqualified error channel. import_selection.go and 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 and 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.

Type: reported availability defect. Priority: medium. Source: security 1; testing 1.

Problem and evidence. Host/native resolvers in 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 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 and history.go counts a different root set from 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, 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 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.

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 is cloned and indexed at publication, but has no production reader. 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 allocates a list of all replaced lower indexes with no production reader. Three tests inspect it. inheritanceReplacement.path, previous, and effective in 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 scans all P policies for each of Q ordinary replacements in 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 clones and scans lower policy, and resolveStrictCollectionPolicies in 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 combines claim/declaration preparation, operation resolution, callbacks, diagnostics, origins/history, retirement, record policy rebuilding, and declaration-only finalization. Overlay and inheritance 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 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 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 set inputAcquisitionRequest.deferAssembly to true. Only private test helpers select the eager composition branch in 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 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 contains a substantial ordinary-load context fallback; 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 is called only by the parseImportSpecsYAML test fixture. It duplicates mapping/field dispatch already owned by parseRootDirectiveBlock in 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 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.

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 mergeDisjointMappingsAtPath and shared recursive merging; the old helper references only itself.
mergeHistoryDocument in history.go mergeHistoryDocumentAtPath with sequence evidence.
inheritanceDirectiveLocation in inheritance.go Preparation's directive entry/location handling.
addSecretEvidencePath in secret.go Projected version sensitivity evidence and the active path accumulator.
PlacementModeFromFacade, PlacementModeForFacade in 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 Local declaration normalization, modeDeclarationPresent, and applicationIdentityPresent.
inputPresence.null in decode.go Presence/location and effective-node null reporting.
rootDirectiveExtraction.importsPresent, sequencesPresent in directives.go Parsed declarations, diagnostics, and directive validation.
importSelection.outcome in import_selection.go and glob selection Candidate/admission behavior and the distinct, active public importSelectionEvidence.outcome.
importDeclarationSpec.excludeSet in import_spec.go Active exclude policy.
schemaPath.field, dynamic in schema.go Descriptor and success result used by path consumers; coordinate A01.
selectedOverlayRequest.filesystem in selected_overlay.go Explicit filesystem-access argument and borrowed ownership.
sequenceDeclarationSpec.destinationLocation in sequence_declaration.go Other active declaration/field diagnostic locations.
sourceVersionEvidence.absence, checked; sourceVersionAncestor.kind in source_version.go Actual checks, version/presence/location evidence, ancestor paths, and sensitivity.
postDecodeOverrideContext.sources in 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, lifecycle.go, layered.go, import_spec.go, and 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 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 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 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 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, retaining the comprehensive owner in 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, 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 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. 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. Local fixes and removal of proven unused work do not select those experiments.

Validation Approach For Selected Work

Use the existing development guide and testing policy. 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
correctness 2 A02
correctness 3 A03
correctness 4 B01
efficiency 1 E01
efficiency 2 E02
efficiency 3 E04
efficiency 4 E05
efficiency 5 F08
efficiency 6 A05
complexity 1 F01
complexity 2 F02
complexity 3 E03
complexity 4 C03
complexity 5 F07, with bounded bridge removal in F05
concurrency 1 B01, corroboration
resources 1 A04
resources 2 D01
resources 3 B02
error-handling 1 C01
security 1 C02
testing 1 Regression acceptance under A01–A04, B01–B02, C02, and D01
testing 2 F09
testing 3 C01, including shared fixture assertions
testing 4 F10
testing 5 F11
testing 6 F12, with F04 and F09
deadcode 1 D02
deadcode 2 D03
deadcode 3 F04
deadcode 4 F05
deadcode 5 D04
deadcode 6 F06
deadcode 7 F03
deadcode 8 F01, duplicate of complexity 1

Settled Scope Decisions

OQ-1. What Should The First Implementation Scope Include?

Settled: include all nine reported defects with their regression coverage, the bounded cleanup, and the identified independent test improvements.

Selected work Finding IDs
Reported defects and accompanying regression coverage A01–A04, B01–B02, C01–C02, D01
Removal of unused copies, evidence, helpers, fields, and test-only production paths D02–D04, F01, F03–F06
Public coverage and stronger or less redundant test assertions F09–F12

This selects 21 review entries. Reconfirm cleanup evidence on the implementation baseline before removal, and preserve distinct behavioral protection when replacing tests. Coordinate unused history copies with D01's retained-graph accounting; verify callback-state closure and host-root closure independently. Keep fixes and cleanup in focused, reviewable commits, even if delivered in one sprint. Cleanup must not delay a consequential fix.

The combined scope restores existing contracts and removes demonstrated maintenance burden while avoiding repeated work at the same ownership and test boundaries. Delivering defects first and cleanup separately would reduce the initial review surface, but would retain redundant work and revisit those boundaries. The combined approach is selected.

The six efficiency candidates and four broader structural refactors remain unselected pending OQ-2 and OQ-3. This decision does not select new public behavior or establish an implementation sequence.

Open Questions

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?

F01's removal of the test-only protocol is already selected under OQ-1.

Recommendation: 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.