diff --git a/docs/roadmap/diff.md b/docs/roadmap/diff.md index f0ffe91..e9ab84f 100644 --- a/docs/roadmap/diff.md +++ b/docs/roadmap/diff.md @@ -2,8 +2,8 @@ Status: design agreed; implementation has not started. This document owns the targeted end state, selected contract, and work needed from the current baseline. -A future staged implementation plan will own execution order. No diff API is -available yet. +The [implementation plan](implementation.md) owns execution order. No diff API +is available yet. ## Targeted end state diff --git a/docs/roadmap/implementation.md b/docs/roadmap/implementation.md new file mode 100644 index 0000000..655a55e --- /dev/null +++ b/docs/roadmap/implementation.md @@ -0,0 +1,358 @@ +# Implement effective configuration diffs + +Status: planned. Audience: a Luna coding agent implementing one stage per prompt, +in numerical order. The [diff roadmap](diff.md) owns the agreed target contract +and scope. This document owns implementation steps and completion evidence. + +Planning baseline: `e82b1c90543a6ab92138ee8c0c2a4cd4daea8af0`. +No diff API exists at this baseline. This plan adds comparison of successful +loads' frozen canonical inspection values; it does not change loading rules. + +## Working instructions + +- Begin each stage with [development.md](../development.md), applicable policies, + working-tree inspection, and the owning implementation and tests. Follow + `AGENTS.md`'s graph-first code discovery guidance. Preserve unrelated work. +- Implement only the current stage and its relevant tests/documentation. Update + its completion status and evidence here, then commit the validated nonempty + unit with a short plain-English message. Keep subsequent stages planned. +- Use the roadmap's same-type `Diff`, root-owned `Difference`, existing + `ValueSnapshot`, atomic sequences, cross-protected redaction, and reference-only + credential boundary. Do not replace these with an easier partial feature. +- Keep loading, callback auditing, secret classification, existing rendering, + and historical explanation behavior unchanged. No dependencies, toolchain + changes, new resource caps, callback identity checks, release gates, renderer, + sequence alignment, credential comparison tokens, or background work are + needed. These are this implementation's scope boundaries, not new repository + prohibitions on unrelated future designs. +- New diff state is operation-local. Read retained trees/indexes without mutation; + do not add load-time caches, retain paired documents after comparison, or share + mutable state between calls. Build safe output before exposing records. +- Run each stage's focused checks and the everyday `./scripts/check.sh` before + committing production changes. Documentation-only stages need local-link and + whitespace checks. Run engine race tests after the integrated engine stage + and the full race suite during final verification. Reuse successful checks for + unchanged content. These are development checks, not new release gates. +- Tests remain offline and use disposable canaries, temporary files, and + controlled environment variables. Environment-mutating tests are not parallel. + Reuse existing loading tests rather than reproduce the loader. Benchmarks are + optional; no machine-specific thresholds or public complexity promises. +- Do not push, tag, publish, upgrade consumer dependencies, or retire unfinished + roadmaps. Release preparation and inclusion in a particular version are + separate decisions; do not assume this feature belongs in the prepared v0.6.0 + note. + +## Implementation decisions + +These decisions complete the roadmap's implementation details without extending +its public contract. + +1. Root `diff.go` defines `Difference` and + `Diff[T any](left, right *Result[T]) ([]Difference, error)`. Engine `diff.go` + owns `Diff(left, right *Inspection) ([]Difference, error)` and an internal- + facing `Difference` record. Root conversion remains explicit, never an alias. +2. A usable result has a non-nil retained inspection with a non-nil final tree. + A valid defaults-only load is usable. Nil/zero results fail with a value-free + error and no records. Do not validate `Result.Config`, options identity, + source identity, or matching defaults. Do not add a public error hierarchy or + diagnostic codes just for this feature; exact error wording is not a promise. +3. Obtain regions with `compareConfigurationValues(left.final, right.final)`. + Resolve their nodes with the existing shape-aware document lookup and each + inspection's immutable lookup. Preserve the comparator's pointer ordering, + canonical scalar equality, atomic sequences, and owned paths. Do not alter + comparison used by resolution or validation. +4. Combine true entries from both `Inspection.secrets` maps once per operation. + Those maps already include historical and dependency-derived classification; + do not rerun predicates or propagation. Canonical pointer classification + retains its existing meaning. Actual document traversal determines whether + a token is a mapping key or sequence index; matching text alone does not prove + structural correspondence. +5. Use a small private diff-local sensitivity lookup for ancestor/equal masking + and strict-descendant classification. Reuse `pathPrefixIndex` with owner-held + IDs where useful. Do not scan the entire classification map for each visited + node or each difference, or build a second general path framework. Retain + historical-only selectors even when absent from both current trees. +6. Prepare safe paired subtrees by traversing the union of mapping keys and + corresponding sequence positions. Position pairing is solely for disclosure + and hidden-change detection; the public delta remains the whole sequence, + with no element alignment. Absence is a nil node; explicit null is a present + `document.Null` node. +7. If a selector masks the current path by equality or ancestry, replace each + present node there with the ordinary redaction marker. Otherwise recurse + through compatible mappings/sequences. A missing key/index or entirely absent + side is absence, not a kind mismatch: mask present sensitive descendants and + preserve public siblings. +8. A kind mismatch between two present nodes requires complete reported-region + masking if there is a classified strict descendant of that mismatch path. + A sensitive ancestor/equal selector already masks it under item 7. Check + nested mismatches within atomic sequence/addition/replacement regions, not + only the reported roots. On such a mismatch, mask both present roots of the + original reported region; leave an absent side absent. An unrelated sensitive + sibling does not trigger fallback for a public-only mismatch. Missing or + incompatible historical descendant selectors still require this fallback. +9. Compute `HiddenChanged` from raw value/presence differences at masking + frontiers. At a masked frontier use `configurationValuesEqual` on the two + raw nodes; otherwise OR the children's flags. A classified but unchanged + subtree yields false even alongside a public change. Complete-region fallback + yields true for an emitted changed region. Do not compare redacted YAML, use + `Redacted` as a change flag, or expose child identities/counts through this flag. +10. Render only already-safe documents through the private YAML adapter, using + existing deterministic snapshot formatting. An absent side is the zero + `ValueSnapshot`. Unexpected serialization failure returns a fixed value-free + error with no usable partial diff, rather than silently substituting a marker + and misreporting `HiddenChanged`. Existing explanation snapshot fallback + behavior remains unchanged. No raw dumper/codec error text is exposed. +11. No-difference results contain zero records; do not promise a distinction + between nil and allocated empty slices. Return detached paths and immutable + YAML strings. Sensitivity differences alone do not emit records for equal + effective values. Frozen snapshots include secret reference names, never + credentials, even when the caller changes the environment after loading. + +## Stage 1 — Establish evidence and record the architectural decision + +Status: planned. Scope: orientation, baseline evidence, and one ADR; no feature +code or current-behavior documentation changes. + +1. Confirm the baseline and inspect the roadmap and relevant code/test owners: + `document_compare.go`, `snapshot.go`, `render.go`, `load.go`, `load_result.go`, + `document_walk.go`, `document_lookup.go`, root `lifecycle.go`, `inspection.go`, + `conversion.go`, and `path.go`. +2. Run `./scripts/check.sh`; record revision, local Go version, results, and any + environmental limitation here. Identify existing coverage for canonical + comparison, snapshot presence/redaction, immutable inspection, and load failure. +3. Write `docs/adr/0010-effective-configuration-diff.md` using the repository's + ADR format and the agreed roadmap. Check the next unused ADR number before + creating it. Record the accepted architectural choice and supporting reasons: + completed-load inspection, canonical equality, shared composition model, + cross-protected snapshots, application-owned presentation, and the credential + boundary. Put rejected choices only in Alternatives Considered. Do not discuss + downstream-specific conventions or use negative parallelism outside that + section. The roadmap continues to own implementation status. + +Done: baseline evidence and an accepted decision are recorded. Existing public +behavior remains unchanged; the deferred inventory is reconciled when the API +lands in Stage 10. + +## Stage 2 — Prepare canonical changed regions + +Status: planned. Scope: a private helper and structural tests only. + +1. Add `internal/engine/diff.go` with private preparation over two inspections. + Reject unavailable inspection/final state with safe errors and no regions. +2. Reuse `compareConfigurationValues`; package each delta's owned path with + borrowed immutable left/right nodes found through existing lookup helpers. + Do not clone metadata, inspect sources, replay history, project mutable Go + values, or write to retained lookup maps. +3. Add `internal/engine/diff_test.go` coverage for equal roots/mapping order, + additions/removals, null versus absence, empty containers, scalar kinds, + atomic sequences, root changes, lexical pointer order, escaped/empty/numeric + keys, and unavailable inspections. Reuse comparator fixtures where suitable. + Private node fixtures may exercise structural cases not reachable from one + typed schema; full-load tests belong in later stages. + +Validate: `go test ./internal/engine -run 'Diff|CompareConfiguration|Path'` and +the everyday check. Done: raw regions match existing structural semantics, and +no consumer-facing operation exposes them yet. + +## Stage 3 — Combine sensitivity into a local lookup + +Status: planned. Scope: classification lookup, independently of safe rendering. + +1. Add `internal/engine/diff_sensitivity.go` and focused tests. Union true + classifications from both inspections without mutating either map. +2. Implement ancestor/equal masking and strict-descendant lookup with separator- + aware canonical tokens. Preserve root versus the empty-key pointer, escaping, + exact terminal boundaries, and `/a` versus `/ab`. +3. Exercise either-side classification, false map entries, duplicates, root, + ancestors/descendants, and historical-only paths. Verify both input maps are + unchanged. Test queries by their results, not the chosen trie layout. + +Validate: `go test ./internal/engine -run 'Diff|PathPrefix'` and the everyday +check. Done: later disclosure can query combined sensitivity without repeated +whole-map scans or new persistent inspection state. + +## Stage 4 — Build paired safe snapshots for compatible shapes + +Status: planned. Scope: compatible mapping/sequence/scalar traversal and absence. + +1. Add private paired traversal in `internal/engine/diff_snapshot.go`, passing + the reported region path and Stage 3's lookup. Build safe owned containers + for each present side. Reuse the existing marker and safe snapshot conventions. +2. Mask ancestor/equal-selected nodes before exposing bytes. Visit the union of + mapping keys and sequence positions, preserving each side's own content and + length. Do not fill missing keys or indices with explicit null. +3. Preserve public siblings and deterministic YAML ordering. Borrow immutable + scalar nodes only where safe; never modify retained node structs or slices. +4. Test aligned sensitive/public mappings, sequences with unchanged masked + members, absent subtrees, added/removed sensitive descendants, classified root + and containers, one-sided classification, and literal marker-looking strings. + Keep this helper private and unexposed; Stage 5 completes incompatible shapes + before Stage 7 exposes the engine operation. Unsupported intermediate branches + must not return unprotected values. + +Validate: `go test ./internal/engine -run 'Diff|Effective|Snapshot'` and the +everyday check. Done: compatible paired masking preserves presence and public +content, while existing standalone rendering remains unchanged. + +## Stage 5 — Complete structural-replacement masking + +Status: planned. Scope: the fallback rule and its shape-sensitive regressions. + +1. Extend the paired traversal to detect incompatible present kinds at the root + or nested positions. Use strict-descendant classification under the mismatch + path to request complete masking of the original reported region. +2. On fallback discard partial safe trees for that region, mask each present + reported root, and preserve any absent side. Without applicable sensitivity, + ordinary public kind replacements retain their values. Do not infer shape + from pointer spelling or ignore a historical selector that is now absent. +3. Test both directions of mapping/scalar, mapping/sequence with numeric keys, + and null/container replacements; nested mismatches inside atomic sequences; + and historical-only descendant classification. Contrast public mismatches + with unrelated unchanged sensitive siblings, and ordinary missing keys/indices + that permit targeted redaction rather than complete masking. + +Validate: focused `Diff` tests, existing rendering tests, and the everyday +check. Done: every shape branch has defined disclosure behavior without blanket +masking of otherwise compatible public regions. + +## Stage 6 — Distinguish hidden changes from unchanged masked content + +Status: planned. Scope: one flag threaded through the existing paired traversal. + +1. Have paired preparation return `HiddenChanged` alongside the safe nodes and + each side's redaction state. At directly masked frontiers compare raw nodes + with `configurationValuesEqual`; include presence. OR flags through unmasked + containers. Full-region fallback on a delta sets true. +2. Do not use safe YAML equality to infer raw equality. Do not set the flag simply + because either snapshot is redacted. Do not align sequences or emit new + descendant delta records; hidden detection uses the same positional disclosure + traversal as Stage 4. +3. Test wholly masked changes, identical rendered snapshots with differing raw + values, mixed public/hidden changes, public-only changes with unchanged masked + content (false), masked additions/removals, changed sequence lengths, and + fallback (true). Include raw equality under different classifications to show + that classification alone does not create a delta. + +Validate: `go test ./internal/engine -run 'Diff|Secret|Sensitivity'` and the +everyday check. Done: the flag communicates withheld change without claiming +that every masked child changed or revealing additional change details. + +## Stage 7 — Assemble the complete engine diff operation + +Status: planned. Scope: internal-facing records, serialization, and real loads. + +1. Define engine `Difference` with `Path`, `Left`, `Right`, and `HiddenChanged`, + and the operation over two `*Inspection` values. Wire Stages 2–6 together: + prepare raw regions; build the local classification lookup once; make safe + pairs; serialize safe nodes; append records in comparator order. +2. Match existing `ValueSnapshot` YAML/presence/redaction behavior on ordinary + successful serialization. Return safe errors with no records for unavailable + inputs or unexpected serialization failure. Do not reuse explanation's + silent rendering-failure fallback for the diff flag. Do not expose internal + node pointers in the result. +3. Add full-load tests using typed/dynamic fields, defaults, recorded resolution, + dependency-derived sensitivity, and historical removal/replacement. Confirm + actual `Inspection.final` values and `Inspection.secrets` drive comparison. +4. Load typed secrets with disposable environment values: rotation behind the + same reference produces no diff; changing reference names does. Exercise + optional secret absence and classified reference names. Change the environment + after loading to prove diff does not reread it. +5. Exercise unchanged standalone effective rendering, explanations, metadata, + and independent loads after comparison. Repeated diff calls remain detached. + +Validate: `go test ./internal/engine -run 'Diff|Explain|History|Secret|Independent'`, +`go test -race ./internal/engine`, and the everyday check. Done: complete safe +comparison works through the engine with no loading or existing inspection +regression. Do not expose a partial public API earlier. + +## Stage 8 — Add the root-owned public facade + +Status: planned. Scope: public declarations, conversion, and external tests. + +1. Add root `diff.go` with the exact roadmap record and generic entry point. + Validate nil/zero `Result` state without inspecting caller-owned `Config`; + delegate through the private inspection pointer. Convert engine records with + existing path and `fromEngineValueSnapshot` conventions. Ensure engine paths + are owned before wrapping them; root signatures expose only root types. +2. Add Go documentation for direction, presence, granularity, ordering, frozen + state, combined sensitivity, `HiddenChanged`, equal-result behavior, errors, + and credential limitation. No extra options, method-only alternative, + exported bridge, generic value tree, or second supported import package. +3. Add `facade_diff_test.go` in the external test package for representative + results, nil/zero inputs, defaults-only loads, equal values from different + sources/defaults/resolvers, explicit zero versus omitted/defaulted zero, + and null/absence. Mutate `Result.Config` and returned records, then confirm + repeated diffs and existing inspection still use frozen values. +4. Confirm callbacks run during loads only. A temporary source file may be + removed after both loads and comparison still works. Test invalid left and + right loads retain the normal nil-result/LoadError behavior; callers must + handle those errors before diffing. Do not add a new load-failure aggregator. + +Validate: `go test . -run 'Diff|FacadeInspection|PublicFacadeBoundary|DependencyImportBoundary'` +and the everyday check. Done: the selected API is available through the stable +root facade without weakened ownership or changed load contracts. + +## Stage 9 — Demonstrate both consumer workflows + +Status: planned. Scope: maintained examples and their behavior tests. + +1. Add a small complete example under `examples/consumers` that loads two caller- + selected files with one definition, handles each load error, calls `Diff`, and + presents safe changes using consumer-owned formatting. Tests use temporary + files; documentation contains no real application configuration or credentials. +2. Add a base-versus-base-plus-overlay example using normal loading options and + the selected overlay. Include recorded resolution and a classified nested + value, showing the difference after resolution rather than diffing overlay + source text. Recreate one-shot sources where necessary under existing reuse + rules. Demonstrate retained lower-priority siblings. +3. Use `Present` for absence, record membership for change, and `HiddenChanged` + for withheld change indicators. Do not parse marker text. Do not supplement + records with separately rendered full contexts that lack combined sensitivity. +4. Add an executable root `ExampleDiff` using minimal mapping inputs if this + provides useful API discovery without duplicating the complete workflows. + Verify example output is deterministic and secret-free. + +Validate: `go test ./examples/consumers`, `go test . -run '^ExampleDiff$'` when +added, and the everyday check. Done: applications can implement the motivating +workflows without internal imports or their own structural comparison. + +## Stage 10 — Reconcile documentation and audit the complete feature + +Status: planned. Scope: current-behavior documentation, integration audit, and +completion evidence; no release publication. + +1. Add an effective-diff section to `docs/config.md` for shared equality, + granularity, cross-redaction, and credential semantics. Symbol details remain + beside declarations. Add task guidance/example links in + `docs/consumers/README.md` and concise navigation in the root README. +2. Update `docs/internal/overview.md` for implemented ownership; add the new ADR + and focused diff test commands to `docs/development.md`. Update architecture + policy only to recognize the approved inspection boundary and link to owners; + preserve existing loading, composition, confidentiality, and ownership rules. +3. In `docs/roadmap/deferred.md`, remove configuration diffs from the excluded + list, keep fingerprints and unrelated exclusions, and link implemented diff + behavior to its owner. No unrelated policy provenance, licensing, dependencies, + prepared release notes, or release machinery changes are required. +4. Review the complete diff against the planning baseline and every roadmap + contract. Check all structural/redaction branches, error disclosure, the + root/private boundary, detached records, and unaffected standalone inspection. + Remove abandoned temporary branches/helpers from earlier stages; do not keep + alternate production comparators or unsafe public entry points. +5. Run `./scripts/check.sh` and `GOTOOLCHAIN=local GOPROXY=off go test -race ./...`. + Confirm tests cover both workflows, aligned and incompatible secret shapes, + hidden-change false/true cases, reference rotation, invalid results, ownership, + and the existing public/import boundaries. Document actual results, revisions, + and limitations here without claiming unmeasured speedups. +6. Mark stages and roadmap complete only once the selected outcomes and owning + documentation agree. Keep planned/deferred work accurate. Retire completed + planning records later during separately authorized release preparation. + +Done: the agreed feature is implemented, validated, and documented without +stronger unrelated guarantees or changes to the publication contract. + +## Open Questions + +None required for this scope. The roadmap's four choices are approved; the +shape fallback, `HiddenChanged` calculation, error handling, and sequencing are +resolved above. Any new requirement found during implementation should be +reported with its impact rather than inferred as an additional contract.