Skip to content

engine: isolate component inputs and commit enrichment only on success #416

Description

@bomly-guy

Summary

Make matcher and analyzer execution transactional at the host boundary: each component receives mutation-isolated inputs, and only a successful result is committed to the canonical registry.

Current mismatch

internal/engine/engine.go:Match and Analyze pass requests containing mutable graph, registry, and target pointers directly to embedded components. A component can mutate those pointers and then return an error or observe cancellation; the engine continues with changed host state.

Managed plugins behave differently because requests cross the JSON/gRPC boundary. In internal/plugin/registry.go, returned PackageUpdates are applied only after a successful RPC. A failing managed plugin cannot mutate the caller's registry through request pointers.

The same SDK module should not have different rollback behavior solely because it runs embedded versus managed.

Dependency

Blocked on the deep snapshot primitives in bomly-dev/bomly-sdk#27. The final CLI change must pin a released SDK version; do not commit a module replace.

Required behavior

For every matcher and analyzer:

  1. Build component-visible request data from an SDK deep snapshot, without a JSON round trip.
  2. Do not expose canonical host pointers to Ready, Applicable, Match, or Analyze.
  3. On a nil error, commit exactly one result form:
    • a non-nil full Registry wins; otherwise
    • merge PackageUpdates through sdk.ApplyPackageUpdates.
  4. On error or cancellation, discard all changes made to request snapshots and result-local state.
  5. Preserve successful changes from earlier components if a later component fails; rollback is per invocation, not whole-pipeline.
  6. Keep managed and embedded behavior observably equivalent.

Fresh snapshots for separate Ready, Applicable, and action calls are preferred because managed RPC serialization already prevents mutation from leaking between those calls. If a different scope is chosen, document and test the parity reasoning in the ADR.

Acceptance criteria

  • A failing embedded matcher that mutates registry, graph, target, maps, and nested slices leaves canonical host state unchanged.
  • The same rollback test exists for analyzers.
  • Cancellation before/during a component call commits no partial changes from that component.
  • Successful full-registry and package-delta results both commit correctly, with full registry taking precedence when both are returned.
  • Updates are never applied twice by the external adapter and engine.
  • Successful earlier component output remains available after a later component error.
  • Embedded and managed fixtures produce equivalent canonical results for success and failure.
  • Independent/disjoint component order permutations produce equivalent results; conflicting writes have explicit expected precedence.
  • No production snapshot path uses JSON marshal/unmarshal.
  • Existing matcher/analyzer stats, run labels, filtering, and error aggregation remain intact.

Architecture and compatibility

Add or update an ADR under dev-docs/adr/ because this tightens component execution semantics and may affect components that relied on mutating request inputs instead of returning results. The wire contract remains unchanged: full registries stay the v1 baseline and PackageUpdates remains optional/additive.

Shared copying and merge rules belong in the SDK; orchestration and commit timing belong here.

Out of scope

  • Parallel matcher/analyzer execution.
  • Changing merge precedence.
  • Removing the full-registry response.
  • Making every SDK model immutable.

Verification

Run:

make test
make build-lite

If the SDK pin changes generated documentation, run make generate and commit the drift.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    enhancementNew feature or request

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions