feat(iac): capability extension — canonical_keys + compute_plan_version (ADR-0029)#619
Merged
Merged
Conversation
…on on CapabilitiesResponse Closes the SupportedCanonicalKeys + ComputePlanVersionDeclarer regressions left open by the strict-contracts force-cutover (ADRs 0024-0028) per the deferred option-d follow-up: Proto: adds 2 optional fields to CapabilitiesResponse — - canonical_keys (repeated string): provider-level override of interfaces.CanonicalKeys() default. DO plugin override path restored. - compute_plan_version (string): provider-level apply-time dispatch version. typedIaCAdapter now satisfies wfctlhelpers.ComputePlanVersionDeclarer so DispatchVersionFor reads the plugin's declaration instead of silently defaulting to v1. Adapter: caches CapabilitiesResponse on first access; subsequent SupportedCanonicalKeys / ComputePlanVersion / Capabilities calls reuse the cached value (capabilities are advertised once at plugin startup + don't change during a wfctl invocation; cache avoids RPC thrash on the apply-time dispatch hot path). Tests: 5 new cases on *typedIaCAdapter cover override path, default fallback (set-based comparison since CanonicalKeys() iteration order isn't guaranteed), apply-version declaration, empty-declaration default, and cache-reuse invariant. Full -short suite + -race PASS. ADR-0029 documents the decision, alternatives rejected (per-type canonical_keys, dedicated SupportedCanonicalKeys RPC, separate ComputePlanVersionDeclarer service), and plugin-author migration shape. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Extends the typed IaC plugin capability surface to restore two regressions from the strict-contracts cutover: provider-specific canonical key overrides and apply-time dispatch version selection, both delivered via additive fields on CapabilitiesResponse (ADR-0029).
Changes:
- Added
canonical_keysandcompute_plan_versionto the IaC external pluginCapabilitiesResponseproto (and regeneratediac.pb.go). - Updated
*typedIaCAdapterto read these fields and to cacheCapabilitiesResponseafter first fetch. - Added focused unit/in-process gRPC tests covering canonical key override behavior, plan version dispatch behavior, and capability caching.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| plugin/external/proto/iac.proto | Adds the new canonical_keys and compute_plan_version fields to CapabilitiesResponse. |
| plugin/external/proto/iac.pb.go | Regenerated protobuf output reflecting the new capability fields. |
| decisions/0029-capability-extension-canonical-keys-and-compute-plan-version.md | Documents ADR-0029 rationale, decision, and migration guidance for plugin authors. |
| cmd/wfctl/iac_typed_adapter.go | Implements capability caching and exposes SupportedCanonicalKeys() + ComputePlanVersion() via cached capabilities. |
| cmd/wfctl/iac_typed_adapter_test.go | Adds tests validating the new behavior and the intended caching invariant. |
Comment on lines
+294
to
+310
| // fetchCapabilities returns the plugin's CapabilitiesResponse, caching the | ||
| // first result for the adapter's lifetime. RPC errors are also cached so | ||
| // repeated accesses don't repeatedly fail against an unreachable plugin. | ||
| // Capabilities are advertised at plugin startup and don't change during | ||
| // a wfctl invocation; caching is correct + cheap. | ||
| func (a *typedIaCAdapter) fetchCapabilities() (*pb.CapabilitiesResponse, error) { | ||
| if a.capsFetch { | ||
| return a.cachedCaps, a.capsErr | ||
| } | ||
| a.capsFetch = true | ||
| resp, err := a.required.Capabilities(context.Background(), &pb.CapabilitiesRequest{}) | ||
| if err != nil { | ||
| a.capsErr = err | ||
| return nil, err | ||
| } | ||
| a.cachedCaps = resp | ||
| return resp, nil |
⏱ Benchmark Results✅ No significant performance regressions detected. benchstat comparison (baseline → PR)
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Closes the
SupportedCanonicalKeys+ComputePlanVersionDeclarerregressions left open by the strict-contracts force-cutover (ADRs 0024-0028) per the deferred option-d follow-up agreed during Task 30 + Task 17 reviews.Proto — adds 2 optional fields to
CapabilitiesResponse:canonical_keys(repeated string): provider-level override ofinterfaces.CanonicalKeys()default. DO plugin override path restored.compute_plan_version(string): provider-level apply-time dispatch version.*typedIaCAdapternow satisfieswfctlhelpers.ComputePlanVersionDeclarersoDispatchVersionForreads the plugin's declaration instead of silently defaulting to v1.Adapter — caches
CapabilitiesResponseon first access; subsequentSupportedCanonicalKeys/ComputePlanVersion/Capabilitiescalls reuse the cached value. Capabilities are advertised once at plugin startup + don't change during a wfctl invocation; cache avoids RPC thrash on the apply-time dispatch hot path.Tests
5 new cases on
*typedIaCAdapter:_SupportedCanonicalKeys_PluginOverride— strict-subset declaration honored_SupportedCanonicalKeys_FallbackToDefault— empty list →interfaces.CanonicalKeys()default (set-based comparison)_ComputePlanVersion_PluginDeclares—\"v2\"declaration surfaces throughDispatchVersionFor_ComputePlanVersion_EmptyMeansV1— empty string default →\"v1\"viaDispatchVersionFor_CapabilitiesCacheReusedAcrossCalls— RPC fired exactly once across 5×3 accessor callsLocal:
GOWORK=off go test ./cmd/wfctl/ -count=1 -short+-raceboth PASS.ADR
decisions/0029-capability-extension-canonical-keys-and-compute-plan-version.mddocuments the decision, alternatives rejected (per-type canonical_keys, dedicated SupportedCanonicalKeys RPC, separate ComputePlanVersionDeclarer service), plugin-author migration shape.Wire compat
Additive proto field numbers (2, 3); old plugin binaries return empty values; new wfctl interprets empty as "use defaults". No flag-day required. AWS/GCP/Azure plugins (when they cut over) populate as needed; backward-compatible.
Hard constraints
NO compat shim, NO fallback path, NO build-tag dual-path. Empty proto fields default to legacy/wfctl-side behavior — fix-forward to a typed gRPC field, not a compat surface.
🤖 Generated with Claude Code