feat(sdk+wfctl): contract-diff extension for verify-capabilities (workflow#767)#773
Merged
Conversation
…low#767) Adds capabilities.iacServices schema to PluginManifest + BuildContractRegistryForPlugin SDK helper + extends verify-capabilities subcommand to walk GetContractRegistry. Sweeps 4 IaC plugins. Closes deferred contract-diff from #765 cycle-3 review.
…helpers) Cycle 1 FAIL: 2 Critical (workflow.iac.v1 namespace never existed on wire — actual workflow.plugin.external.iac per proto pkg decl; duplicated existing registeredIaCServices + iacServiceRequired helpers without citing). + 5 Important. Cycle 2: - Derive namespace prefix programmatically from pb.IaCProviderRequired_ServiceDesc.ServiceName (single source of truth keyed to .proto pkg). - Cite + reuse registeredIaCServices (deploy_providers.go:344) + iacServiceRequired const (iac_typed_adapter.go:52). - Directional diff (FAIL missing-from-binary; WARN extra-in-binary) per IMPORTANT-1. - Use cached adapter.ContractRegistry() — no redundant RPC (IMPORTANT-2). - Sweep-target SDK pin assumption explicit (IMPORTANT-4). - IaCStateBackend orthogonality documented (IMPORTANT-5). - Non-goal: embedded plugin.json verify (IMPORTANT-3).
Cycle 2 PASS with 4 Important to fold. Cycle 3: - ContractRegistryError() check ahead of diff (surface RPC errors verbatim, no synthetic FAILs). - iacserver.go:302 added to §Files so bridge calls BuildContractRegistryForPlugin (SDK helper not dead code for sweep targets). - Fixture construction recipes spelled out (iac-good/iac-missing-service/iac-extra-service stub providers). - Hard-cite #765 as sequencing prerequisite in §Assumptions.
6 tasks, 1 PR. Pairs with cycle-3-PASS design. Task 5 is no-op acknowledgement of cycle-3 reviewer Option B (don't factor existing registeredIaCServices helper this PR).
…nline-spawn pattern) #765 PR #769 + v0.63.2 landed since cycle 3 paused. Worktree rebased onto current main with verify-capabilities.go present. Replan: direct pbClient.GetContractRegistry(ctx, Empty) after existing GetManifest call (line 137); explicit codes.Unimplemented branch maps to empty registry. Drops cycle-3's adapter-based hypothesis (#765 ships inline-spawn, NOT adapter).
…wer) Cycle 4 PASS with 3 Important. Apply inline: - I-2: drop iac_contract_filter.go NEW proposal; reuse registeredIaCServices in-place (both package main) - I-1: add git-grep audit confirming iacserver.go:302 rebinding is safe (4 existing consumers all already filter) - I-3: reword Unimplemented branch to distinguish empty-LHS (skip) vs non-empty-LHS (FAIL on every declared) cases
…ent pattern) 5 tasks, 1 PR. Mirrors design cycle 4 PASS. Direct pbClient.GetContractRegistry after existing GetManifest call (line 131 inline-spawn pattern). Explicit codes.Unimplemented branch. Reuses #765 fixture pattern + IaCStateBackends UnmarshalJSON precedent.
…r from adversarial cycle 1 Critical: registeredIaCServices→serviceNamesFromRegistry unconditional rename; commit go.sum in Task 5 fixtures. Important: client-side namespace filter (defense-in-depth); dedup test for both-top-and-nested. Minors: dead Finalize methods removed; per-task rollback notes → PR-level revert.
C1: iac-missing-service fixture must NOT embed UnimplementedIaCProviderFinalizerServer (Unimplemented satisfies the interface via mustEmbed sentinel → SDK registers it → false PASS). C2: bash pipeline $? reads tee exit not wfctl exit — capture WFCTL_EXIT=$? before any pipe in Final verification 3b.
…from design + plan Resolves alignment-check MISSING finding: design cited decisions/NNNN-verify-capabilities-iac-namespace.md but no ADR existed. ADR documents the proto-descriptor TrimSuffix single-source-of-truth pattern that Tasks 2/3/4 derive from. Design + plan both updated to cite 0042 in place of NNNN.
…orkflow#767 Task 1)
…(workflow#767 Task 2)
…kflow#767 Task 3)
…) (workflow#767 Task 4)
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
⏱ Benchmark Results✅ No significant performance regressions detected. benchstat comparison (baseline → PR)
|
This was referenced May 24, 2026
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
1 participant
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
Extends
wfctl plugin verify-capabilities(shipped at v0.63.2 via #765/#769) with a typed-IaC service diff. Closes #767.PluginManifest.IaCServices []stringfield with nested-promotion (mirrorsIaCStateBackendsprecedent).sdk.BuildContractRegistryForPlugin(grpcSrv, namespacePrefix)filtering helper.iacPluginServiceBridge.GetContractRegistryrewired to use the namespace-filtered helper (strips go-plugin infra services).runPluginVerifyCapabilitiesnow issuespbClient.GetContractRegistryafterGetManifestand computes a directional diff againstplugin.json.iacServices: missing-from-binary → FAIL, extra-in-binary → WARN.Design
See:
docs/plans/2026-05-24-contract-diff-design.mdImplementation Plan
See:
docs/plans/2026-05-24-contract-diff.md(Status: Locked 2026-05-24T13:25:37Z; sha256=0924488cfddf…)ADR
decisions/0042-verify-capabilities-iac-namespace.md— namespace prefix derivation single-source-of-truth pattern (derive frompb.IaCProviderRequired_ServiceDesc.ServiceNamevia TrimSuffix; both SDK bridge and client-side filter use the same derivation).Scope Manifest
Changes
7dfbaecab):plugin/manifest.go— addsIaCServices []string \json:"iacServices,omitempty"`field;UnmarshalJSONlegacy-object branch extended for nestedcapabilities.iacServicespromotion viaappendUnique`. 4 unit tests including dedup across top-level + nested forms.a14de6ab9):plugin/external/sdk/contracts.go— addsBuildContractRegistryForPlugin(grpcSrv, namespacePrefix)that enumerates registered services and returns only those matching the prefix, sorted, as SERVICE-kind STRICT_PROTO ContractDescriptors. ExistingBuildContractRegistry(full-surface) retained.5d87fba11):plugin/external/sdk/iacserver.go:302—iacPluginServiceBridge.GetContractRegistryderives prefix viastrings.TrimSuffix(pb.IaCProviderRequired_ServiceDesc.ServiceName, \".IaCProviderRequired\") + \".\"(ADR 0042) and delegates to the filtered helper. Internal regression test confirms PluginService + health are excluded.6031a29d9):cmd/wfctl/plugin_verify_capabilities.go— addsserviceNamesFromRegistry(reg, prefix)helper (kind + prefix filter; named distinct fromdeploy_providers.go'sregisteredIaCServicesto avoidpackage mainsymbol clash) anddiffIaCServices(declared, advertised)directional set-difference. NewpbClient.GetContractRegistryRPC issued afterGetManifest;codes.Unimplementedmapped to empty registry; missing-from-binary appended tofailures, extra-in-binary emitted as WARN to stderr.ac48371e3): 3 fixture scenarios + 3 integration tests:iac-good: declares + registers bothIaCProviderRequiredandIaCProviderFinalizer→ PASS.iac-missing-service: declares both but binary ONLY embedsUnimplementedIaCProviderRequiredServer(NO Finalizer — the embed would satisfy the interface viamustEmbed*sentinel) → FAIL on Finalizer.iac-extra-service: registers both but declares only Required → WARN on Finalizer (exit 0).Test Plan
GOWORK=off go test ./...— all packages PASS (plugin, plugin/external/sdk, cmd/wfctl, plus 14 others)GOWORK=off go vet ./...— 0 issuesgolangci-lint run ./cmd/wfctl/... ./plugin/...— 0 issuesTestPluginConformanceregression (Task 3 touched SDK bridge) — PASS (54s)wfctl plugin verify-capabilities --binary /tmp/iac-good cmd/wfctl/testdata/verify_capabilities/iac-good→ exit 0,OK verify-iac-good(PASS path)wfctl plugin verify-capabilities --binary /tmp/iac-missing cmd/wfctl/testdata/verify_capabilities/iac-missing-service→ exit 1, stderr containsiacServices:+IaCProviderFinalizer(FAIL path)origin/feat/767-contract-diffOut of scope (deferred to follow-up PRs)
iacServicesinplugin.json— separate per-repo PRs after this lands and v0.64+ ships.validate-contractstatic enforcement of non-emptyiacServicesfortype:\"iac\"plugins.workflow.plugin.external.iac.*.iacServicesfrom runtime introspection.Process record
registeredIaCServicesname-clash +iac-missing-serviceFinalizer-embed false-PASS + bash pipeline\$?bug + missinggo.sumin fixtures before any code was written.🤖 Generated with Claude Code