diff --git a/docs/roadmap/equivalence.md b/docs/roadmap/equivalence.md new file mode 100644 index 0000000..7c4041b --- /dev/null +++ b/docs/roadmap/equivalence.md @@ -0,0 +1,121 @@ +# Behavioral Equivalence and Shared Abstraction Audit + +Reviewed revision: `94d1e333d422d742eb59726ad90201f4c7db9205`. + +This audit examined explicit cross-path comparisons and complementary behavioral +tests, then inspected their production implementations. One focused maintenance +opportunity warrants consideration. It is not a demonstrated correctness defect +or a requirement to refactor before release. + +## 1. Share typed-secret slot traversal between resolution and mutation auditing + +### Evidence + +The tests in [secret_reference_test.go](../../internal/engine/secret_reference_test.go) +establish complementary requirements: + +- `TestSecretReferencesResolveTypedShapesOncePerName` (line 31) exercises typed + fields, optional pointers, maps of values and pointers, nil pointers, and + sequences. It verifies resolved credentials are available before validation + and each environment name is looked up once. +- `TestSecretReferenceErrorsAreSafeAndPathSpecific` (line 78) verifies secret + paths, absent/null/invalid references, optional-pointer behavior, and cached + failed lookups. +- `TestValidationCannotReplaceResolvedSecretWithSameName` (line 297) verifies + that validation cannot replace private resolved state while preserving the + projected reference name. It expects `validate.mutation` at the secret's path. +- `TestDynamicValuesDoNotEstablishSecretReferences` (line 415) verifies that + dynamic values do not acquire typed-secret semantics. + +`TestSecretBindingsRemainExactAndIndependent` in +[secret_binding_test.go](../../internal/engine/secret_binding_test.go), line 123, +also verifies that the selected exact secret type belongs to the load's schema +binding. The [typed-secret contract](../config.md#typed-secret-references) +establishes both supported typed containers and the private-state validation +audit. + +These tests do not compare the two walkers directly, and the private-state +mutation regression exercises a direct field rather than every container shape. +The shared responsibility is identifying schema-bound secret slots at their +semantic paths. Resolution and validation auditing intentionally perform +different operations at those slots. + +### Implementation + +Both walks are in +[secret_reference.go](../../internal/engine/secret_reference.go): + +- `resolveSecretValue` (line 57) descends pointers, physical struct members, + string-keyed maps, slices, and arrays. It resolves names, reports failures, + assigns resolved state, and records reference names. +- `collectChangedSecretStates` (line 139), called by `changedSecretStates`, + independently implements the same container dispatch, nil handling, inline + field path handling, sorted map-key traversal, and sequence-index paths. Its + leaf operation compares private state with the earlier lookup results. + +Both already share schema descriptors and `SecretBinding`, but their recursive +navigation remains duplicated. Resolution additionally copies map entries into +writable values and writes them back; auditing reads map entries directly. +[runValidation](../../internal/engine/validation.go) runs the latter after +ordinary projection/mutation checks, so the two traversals occur at different +lifecycle points. + +### Opportunity and potential remediation + +Consider a small engine-private walker dedicated to schema-bound secret slots. +It could own container navigation and semantic path construction, with separate +leaf operations for resolving a reference and checking its private state. This +would give changes to supported secret-container traversal one owner and reduce +the chance that resolution visits a slot that mutation auditing later misses. + +Keep the existing operation-specific entry points, diagnostics, lookup cache, +optional-reference rules, and state comparisons recognizable. Make writable +versus read-only map handling explicit. Keep the two lifecycle passes separate; +sharing traversal does not mean reusing a pre-validation snapshot of slots or +combining resolution and validation into one pass. + +This belongs within `internal/engine`. It needs no new public API, cross-package +reflection framework, global cache, or generic walker for cloning and projection. + +### Tradeoffs + +- A visitor adds indirection. Keep it specific to secrets and small enough that + the container rules and two leaf operations remain easy to read. If doing so + requires a broad configurable traversal framework, retaining the two explicit + walks is preferable. +- Map-entry addressability is an intentional difference. A common walker must + preserve resolution's copy/writeback behavior without making the audit mutate + configuration or perform unnecessary writable copies. +- Auditing must use the original per-load lookup results, without looking up + credentials again or resetting resolved references. Preserve existing nil, + inline-field, path, and exact-type semantics; do not widen dynamic-secret + support or add new rejection rules as part of consolidation. +- Sharing implementation must not make the tests depend on the walker to + discover their expected paths or values. Keep independently authored shape, + mutation, lookup-count, and non-disclosure assertions. The existing tests + establish the motivation, but do not by themselves exhaustively verify a new + shared walker across every supported shape. + +## Scope and validation + +The review covered source and load-entry equivalence, discovery integration, +strict composition and precedence, collection operations and history replay, +inheritance scheduling/application, schema projection/cloning, semantic paths, +metadata/explanation, secret handling, and the facade/internal boundaries. + +The existing architecture already shares substantial implementation logic. +Independent reference scans in tests remain useful oracles; replacing them with +the optimized production helpers would weaken the evidence rather than improve +the architecture. Root-owned facade conversions and parser adaptation also +preserve deliberate boundaries. No additional package extraction is recommended +from this audit, and no new correctness issue was established. + +Codebase graph searches, call traces, source inspection, and the existing suite +were used. No standalone duplicate-code analyzer was available on `PATH`. +With Go 1.26.10 and dependency downloads disabled, this passed: + +```sh +GOTOOLCHAIN=local GOPROXY=off go test ./... -count=1 +``` + +No production code or tests were changed, and no benchmarks were collected.