feat(sdk): ServeIaCPlugin high-level entrypoint with GRPCServer callback (Task 29)#600
Merged
Conversation
4 tasks
intel352
force-pushed
the
feat/iac-sdk-auto-register-task4
branch
from
31cfd5a to
a88917b
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
Adds a higher-level SDK entrypoint for serving typed IaC plugins over go-plugin gRPC, ensuring service registration occurs inside the framework-owned GRPCServer callback (per the strict-contracts cutover plan). The PR also includes the typed IaC proto contract + generated gRPC bindings and unit tests covering service auto-registration and handshake resolution.
Changes:
- Introduces
sdk.ServeIaCPlugin(and related option types) that wrapsgoplugin.Serveand auto-registers IaC services withinGRPCServer. - Adds/extends unit tests to validate GRPCServer registration behavior and handshake override/default logic.
- Adds the typed IaC gRPC proto (
iac.proto) and generated Go gRPC bindings plus compile-time guard tests for interface surfaces.
Reviewed changes
Copilot reviewed 6 out of 7 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| plugin/external/sdk/iacserver.go | Adds ServeIaCPlugin, handshake resolution, and iacGRPCPlugin that registers typed IaC services inside the go-plugin callback. |
| plugin/external/sdk/iacserver_test.go | Black-box tests for RegisterAllIaCProviderServices registration behavior and error surfacing. |
| plugin/external/sdk/iacserver_serve_test.go | Internal-package tests exercising iacGRPCPlugin directly + handshake resolution tests. |
| plugin/external/proto/iac.proto | Defines the typed IaCProvider/ResourceDriver gRPC contract. |
| plugin/external/proto/iac_proto_test.go | Compile-time guard tests ensuring generated server interfaces contain expected methods. |
| plugin/external/proto/iac_grpc.pb.go | Generated gRPC bindings for the typed IaC proto contract. |
Comment on lines
+62
to
+63
| "pb.IaCProviderRequiredServer (missing methods); see "+ | ||
| "docs/plans/2026-05-10-strict-contracts-force-cutover-design.md", |
Comment on lines
+3
to
+4
| // Design: docs/plans/2026-05-10-strict-contracts-force-cutover-design.md | ||
| // Plan: docs/plans/2026-05-10-strict-contracts-force-cutover.md (Task 3) |
…rver callback
Task 29 of the strict-contracts force-cutover plan
(docs/plans/2026-05-10-strict-contracts-force-cutover.md, rev5).
Adds the high-level plugin-author API on top of Task 4's
RegisterAllIaCProviderServices:
func main() {
sdk.ServeIaCPlugin(&doProvider{}, sdk.IaCServeOptions{})
}
Per cycle 3 I-1 of the design, service registration happens INSIDE
go-plugin's GRPCServer callback (iacGRPCPlugin.GRPCServer) — the
framework owns *grpc.Server lifecycle, so plugin authors cannot
pre-create a server and forget to register a typed service on it.
API surface (all in plugin/external/sdk/iacserver.go):
- IaCServeOptions{ PluginInfo *PluginInfo } — caller-side options.
- PluginInfo{ HandshakeConfig goplugin.HandshakeConfig } — extension
point for future Name/Version metadata; defaults to ext.Handshake
(the canonical wfctl<->plugin handshake) when zero-valued.
- iacGRPCPlugin{provider any} — implements goplugin.Plugin
(GRPCServer + GRPCClient). The GoCodeAlone fork of go-plugin v1.7.0
is gRPC-only and exposes only the canonical Plugin interface; there
is no GRPCPlugin alias or NetRPCUnsupportedPlugin embed to use.
- ServeIaCPlugin(provider, opts) — wraps goplugin.Serve with the
resolved handshake + a single iacGRPCPlugin entry under the "iac"
key.
- resolveServeHandshake(opts) — extracted helper so the override-vs-
default rule is unit-testable without invoking the blocking
goplugin.Serve loop.
Tests (iacserver_serve_test.go) cover six cases via internal-package
tests (so the unexported plugin type is exercisable without a real
subprocess; subprocess-level coverage lands in Task 6's typed-IaC E2E
test):
- iacGRPCPlugin.GRPCServer registers all satisfied services on the
framework-managed *grpc.Server (Required + Enumerator + ResourceDriver
for the all-stub).
- iacGRPCPlugin.GRPCServer propagates the auto-register error for an
empty stub — go-plugin aborts plugin startup with an actionable
message.
- iacGRPCPlugin.GRPCClient is a no-op (host builds typed clients
directly).
- iacGRPCPlugin satisfies goplugin.Plugin at compile time (refactor
guard).
- ServeIaCPlugin defaults to ext.Handshake when PluginInfo is nil.
- ServeIaCPlugin honors a non-zero override handshake when provided.
Stacked on feat/iac-sdk-auto-register-task4 (Task 4 PR #599 provides
RegisterAllIaCProviderServices, which the GRPCServer callback delegates
to).
Verification: GOWORK=off go test -race ./plugin/external/sdk/... PASS;
GOWORK=off go build ./plugin/... ./cmd/... ./module/... clean;
GOWORK=off go vet ./plugin/external/... clean.
Rollback: revert this commit; plugin authors can fall back to
manually constructing goplugin.Serve + Plugins map referencing
RegisterAllIaCProviderServices in their own GRPCServer callback.
Per cycle 4 code-review PR 600 IMPORTANT (Copilot finding):
The previous resolveServeHandshake guard only checked
`opts.PluginInfo.HandshakeConfig.MagicCookieKey != ""`. Caller setting
ProtocolVersion or MagicCookieValue alone (partial config) was
silently coerced to ext.Handshake with the partial fields ignored —
the misconfig hid until dial time when the host rejected the cookie.
Tightens the rule:
- nil PluginInfo → ext.Handshake (unchanged)
- zero-valued HandshakeConfig (PluginInfo{}) → ext.Handshake (was: only
MagicCookieKey check; now whole-struct zero-value via == comparison)
- Any non-zero field BUT missing MagicCookieKey or MagicCookieValue →
typed error naming the partial fields. ServeIaCPlugin propagates the
error via panic at startup so the plugin author sees the misconfig
immediately rather than at first dial.
- Both MagicCookieKey + MagicCookieValue set → use override (unchanged)
Signature change: resolveServeHandshake now returns
(goplugin.HandshakeConfig, error). ServeIaCPlugin handles the error
via panic (plugin-startup misconfig is a programmer error, not a
runtime condition; panic gives the caller the immediate stack trace).
Tests:
- ZeroValuePluginInfo: new — confirms PluginInfo{} treated identically
to nil PluginInfo
- PartialHandshakeOverride_ReturnsError: new — table-driven across 5
partial-override shapes (only ProtocolVersion, only MagicCookieKey,
only MagicCookieValue, Key+ProtocolVersion, Value+ProtocolVersion);
each must produce a typed error naming "partial"
- Existing 2 tests (DefaultsToWorkflowHandshake, HonorsOverride)
updated for the (HandshakeConfig, error) signature
Verification: GOWORK=off go test -race ./plugin/external/sdk/...
PASS; gofmt clean; golangci-lint run → 0 issues.
intel352
force-pushed
the
feat/iac-sdk-serve-task29
branch
from
79d86b2 to
3af176b
Compare
Comment on lines
+197
to
+206
| // supplied a PARTIAL override (any non-zero field but missing | ||
| // MagicCookieKey or MagicCookieValue) — partial overrides produce a | ||
| // broken handshake at dial time, so the misconfig is rejected early | ||
| // rather than silently coerced to defaults. | ||
| // | ||
| // Per cycle 4 PR 600 IMPORTANT review (Copilot finding): the previous | ||
| // guard only checked MagicCookieKey != "", which silently accepted a | ||
| // caller setting ProtocolVersion or MagicCookieValue alone — fields | ||
| // that look intentional but cannot produce a valid handshake. | ||
| // |
Comment on lines
+150
to
+153
| // rather than silently coerced to ext.Handshake. Per cycle 4 PR 600 | ||
| // IMPORTANT review (Copilot finding): partial overrides look | ||
| // intentional but cannot produce a valid handshake; falling back to | ||
| // defaults silently swallows the misconfig until dial-time when the |
⏱ Benchmark Results✅ No significant performance regressions detected. benchstat comparison (baseline → PR)
|
intel352
added a commit
that referenced
this pull request
May 10, 2026
Task 5 of the strict-contracts force-cutover plan (docs/plans/2026-05-10-strict-contracts-force-cutover.md, rev5). Adds plugin/external/sdk/contracts.go with the BuildContractRegistry helper that enumerates grpc.Server.GetServiceInfo() and emits a SERVICE-kind ContractDescriptor for each registered service. ContractMode is set to STRICT_PROTO so the host can distinguish typed IaC services from the legacy structpb-mode contracts produced by Module/Step/Trigger ContractProvider implementations. Per cycle 3 I-1 of the design: wfctl needs a single mechanism to discover "is the optional service registered on this plugin handle?". Reusing the existing ContractRegistry shape keeps Module/Step/Trigger and IaC capability discovery on the same wire surface — no new gRPC server-reflection dependency required. Service descriptors are emitted in deterministic alphabetical order so callers can rely on stable output for diff/compare operations and the wftest BDD test in Task 15. The helper is safe to call with a nil server (returns an empty but non-nil ContractRegistry) so callers that may construct it before the gRPC server exists do not panic. Tests (contracts_iac_test.go) cover three cases — all pass: - AdvertisesRegisteredIaCServices: a Required + Enumerator + DriftDetector stub yields exactly those service descriptors. - ServiceContractsUseStrictProtoMode: every emitted descriptor is Kind=SERVICE + Mode=STRICT_PROTO (host-side discriminator). - NilServer_ReturnsEmpty: defensive contract for nil input. Stacked on feat/iac-sdk-serve-task29 (Task 29 PR #600 provides ServeIaCPlugin which IaC plugins use to register the services this helper enumerates). Verification: GOWORK=off go test -race ./plugin/external/sdk/... PASS; GOWORK=off go build ./plugin/... ./cmd/... ./module/... clean; GOWORK=off go vet ./plugin/external/... clean. Rollback: revert this commit; ContractRegistry returns the prior shape (Module/Step/Trigger only via the existing ContractProvider hook in grpc_server.go).
intel352
added a commit
that referenced
this pull request
May 10, 2026
…602) Task 5 of the strict-contracts force-cutover plan (docs/plans/2026-05-10-strict-contracts-force-cutover.md, rev5). Adds plugin/external/sdk/contracts.go with the BuildContractRegistry helper that enumerates grpc.Server.GetServiceInfo() and emits a SERVICE-kind ContractDescriptor for each registered service. ContractMode is set to STRICT_PROTO so the host can distinguish typed IaC services from the legacy structpb-mode contracts produced by Module/Step/Trigger ContractProvider implementations. Per cycle 3 I-1 of the design: wfctl needs a single mechanism to discover "is the optional service registered on this plugin handle?". Reusing the existing ContractRegistry shape keeps Module/Step/Trigger and IaC capability discovery on the same wire surface — no new gRPC server-reflection dependency required. Service descriptors are emitted in deterministic alphabetical order so callers can rely on stable output for diff/compare operations and the wftest BDD test in Task 15. The helper is safe to call with a nil server (returns an empty but non-nil ContractRegistry) so callers that may construct it before the gRPC server exists do not panic. Tests (contracts_iac_test.go) cover three cases — all pass: - AdvertisesRegisteredIaCServices: a Required + Enumerator + DriftDetector stub yields exactly those service descriptors. - ServiceContractsUseStrictProtoMode: every emitted descriptor is Kind=SERVICE + Mode=STRICT_PROTO (host-side discriminator). - NilServer_ReturnsEmpty: defensive contract for nil input. Stacked on feat/iac-sdk-serve-task29 (Task 29 PR #600 provides ServeIaCPlugin which IaC plugins use to register the services this helper enumerates). Verification: GOWORK=off go test -race ./plugin/external/sdk/... PASS; GOWORK=off go build ./plugin/... ./cmd/... ./module/... clean; GOWORK=off go vet ./plugin/external/... clean. Rollback: revert this commit; ContractRegistry returns the prior shape (Module/Step/Trigger only via the existing ContractProvider hook in grpc_server.go).
intel352
added a commit
that referenced
this pull request
May 10, 2026
…e (Task 6) (#603) * feat(proto): add iac.proto with IaCProviderRequired + 6 optional services + ResourceDriver Task 3 of the strict-contracts force-cutover plan (docs/plans/2026-05-10-strict-contracts-force-cutover.md, rev5). Adds plugin/external/proto/iac.proto defining the typed gRPC contract that supersedes the legacy InvokeService/structpb dispatch path for IaCProvider + ResourceDriver: - service IaCProviderRequired: 11 RPCs every IaC plugin MUST implement (Initialize, Name, Version, Capabilities, Plan, Apply, Destroy, Status, Import, ResolveSizing, BootstrapStateBackend). Compile-time enforced via the SDK type-assert in Task 4. - 6 optional services — providers register only the ones they support: IaCProviderEnumerator (EnumerateAll, EnumerateByTag), IaCProviderDriftDetector (DetectDrift, DetectDriftWithSpecs), IaCProviderCredentialRevoker (RevokeProviderCredential), IaCProviderMigrationRepairer (RepairDirtyMigration), IaCProviderValidator (ValidatePlan), IaCProviderDriftConfigDetector (DetectDriftConfig). Absence of registration IS the negative signal — no NotSupported field on any optional response (per design §Optional services). - service ResourceDriver: 9 RPCs for per-resource-type CRUD dispatch (Create, Read, Update, Delete, Diff, Scale, HealthCheck, SensitiveKeys, Troubleshoot), each carrying resource_type so a single server can route to the per-type driver implementation. Hard invariants honored: - NO google.protobuf.Struct, NO google.protobuf.Any anywhere. - Free-form per-resource Config/Outputs payloads cross the wire as bytes <name>_json (the plugin owns json.Marshal/Unmarshal); this eliminates the structpb conversion surface that previously dropped map[string]bool entries silently (T3.9 finding). - ResourceOutput.sensitive uses typed map<string, bool> per design. Generated iac.pb.go + iac_grpc.pb.go via protoc v34.1 + protoc-gen-go v1.36.11 + protoc-gen-go-grpc v1.6.1. Failing test (plugin/external/proto/iac_proto_test.go) asserts the generated server interfaces exist and have the methods the design requires — drops in iac.proto cause the test file to fail to compile. Verification: GOWORK=off go test ./plugin/external/proto/... PASSES; GOWORK=off go build ./plugin/... ./cmd/... ./module/... clean. Rollback: revert this commit; legacy InvokeService dispatch in plugin.proto remains functional; the additive-only nature of this PR means no consumer is affected until subsequent tasks wire callers. * feat(sdk): RegisterAllIaCProviderServices auto-registration helper Task 4 of the strict-contracts force-cutover plan (docs/plans/2026-05-10-strict-contracts-force-cutover.md, rev5). Adds plugin/external/sdk/iacserver.go: a single helper that uses Go type-assertion to register every typed IaC gRPC service the provider satisfies, in one call. REQUIRED service: pb.IaCProviderRequiredServer — surfaced as a clear startup-time error if the provider type doesn't satisfy it (rather than failing at the first RPC dispatch with a generic "unimplemented" status). OPTIONAL services (auto-detected): IaCProviderEnumerator, IaCProviderDriftDetector, IaCProviderCredentialRevoker, IaCProviderMigrationRepairer, IaCProviderValidator, IaCProviderDriftConfigDetector. Plus ResourceDriver. Per cycle 3 I-1 of the design: plugin authors write ONE call; they cannot omit registration for a capability they implemented. This removes the registration-omission bug class (the same shape as the legacy InvokeService case-string-typo bug) by removing the manual step entirely. Tests cover four cases: - required-satisfied → required service registered + advertised by grpcSrv.GetServiceInfo(). - enumerator-only → only the optional Enumerator service registered; other optionals stay absent (auto-detection precision). - empty-stub → returns an error naming the unsatisfied required interface, with a docs pointer. - all-capabilities-stub → all 8 typed services (Required + 6 optional + ResourceDriver) registered. Stacked on feat/iac-proto-task3 (Task 3 PR #598 provides the generated server interfaces this helper consumes). Verification: GOWORK=off go test -race ./plugin/external/sdk/... PASS; GOWORK=off go build ./plugin/... ./cmd/... ./module/... clean; GOWORK=off go vet ./plugin/external/... clean. Rollback: revert this commit; SDK consumers can still register services manually via the per-service Register* helpers protoc generated. * feat(sdk): ServeIaCPlugin high-level entrypoint with go-plugin GRPCServer callback Task 29 of the strict-contracts force-cutover plan (docs/plans/2026-05-10-strict-contracts-force-cutover.md, rev5). Adds the high-level plugin-author API on top of Task 4's RegisterAllIaCProviderServices: func main() { sdk.ServeIaCPlugin(&doProvider{}, sdk.IaCServeOptions{}) } Per cycle 3 I-1 of the design, service registration happens INSIDE go-plugin's GRPCServer callback (iacGRPCPlugin.GRPCServer) — the framework owns *grpc.Server lifecycle, so plugin authors cannot pre-create a server and forget to register a typed service on it. API surface (all in plugin/external/sdk/iacserver.go): - IaCServeOptions{ PluginInfo *PluginInfo } — caller-side options. - PluginInfo{ HandshakeConfig goplugin.HandshakeConfig } — extension point for future Name/Version metadata; defaults to ext.Handshake (the canonical wfctl<->plugin handshake) when zero-valued. - iacGRPCPlugin{provider any} — implements goplugin.Plugin (GRPCServer + GRPCClient). The GoCodeAlone fork of go-plugin v1.7.0 is gRPC-only and exposes only the canonical Plugin interface; there is no GRPCPlugin alias or NetRPCUnsupportedPlugin embed to use. - ServeIaCPlugin(provider, opts) — wraps goplugin.Serve with the resolved handshake + a single iacGRPCPlugin entry under the "iac" key. - resolveServeHandshake(opts) — extracted helper so the override-vs- default rule is unit-testable without invoking the blocking goplugin.Serve loop. Tests (iacserver_serve_test.go) cover six cases via internal-package tests (so the unexported plugin type is exercisable without a real subprocess; subprocess-level coverage lands in Task 6's typed-IaC E2E test): - iacGRPCPlugin.GRPCServer registers all satisfied services on the framework-managed *grpc.Server (Required + Enumerator + ResourceDriver for the all-stub). - iacGRPCPlugin.GRPCServer propagates the auto-register error for an empty stub — go-plugin aborts plugin startup with an actionable message. - iacGRPCPlugin.GRPCClient is a no-op (host builds typed clients directly). - iacGRPCPlugin satisfies goplugin.Plugin at compile time (refactor guard). - ServeIaCPlugin defaults to ext.Handshake when PluginInfo is nil. - ServeIaCPlugin honors a non-zero override handshake when provided. Stacked on feat/iac-sdk-auto-register-task4 (Task 4 PR #599 provides RegisterAllIaCProviderServices, which the GRPCServer callback delegates to). Verification: GOWORK=off go test -race ./plugin/external/sdk/... PASS; GOWORK=off go build ./plugin/... ./cmd/... ./module/... clean; GOWORK=off go vet ./plugin/external/... clean. Rollback: revert this commit; plugin authors can fall back to manually constructing goplugin.Serve + Plugins map referencing RegisterAllIaCProviderServices in their own GRPCServer callback. * feat(sdk): BuildContractRegistry advertises registered IaC services Task 5 of the strict-contracts force-cutover plan (docs/plans/2026-05-10-strict-contracts-force-cutover.md, rev5). Adds plugin/external/sdk/contracts.go with the BuildContractRegistry helper that enumerates grpc.Server.GetServiceInfo() and emits a SERVICE-kind ContractDescriptor for each registered service. ContractMode is set to STRICT_PROTO so the host can distinguish typed IaC services from the legacy structpb-mode contracts produced by Module/Step/Trigger ContractProvider implementations. Per cycle 3 I-1 of the design: wfctl needs a single mechanism to discover "is the optional service registered on this plugin handle?". Reusing the existing ContractRegistry shape keeps Module/Step/Trigger and IaC capability discovery on the same wire surface — no new gRPC server-reflection dependency required. Service descriptors are emitted in deterministic alphabetical order so callers can rely on stable output for diff/compare operations and the wftest BDD test in Task 15. The helper is safe to call with a nil server (returns an empty but non-nil ContractRegistry) so callers that may construct it before the gRPC server exists do not panic. Tests (contracts_iac_test.go) cover three cases — all pass: - AdvertisesRegisteredIaCServices: a Required + Enumerator + DriftDetector stub yields exactly those service descriptors. - ServiceContractsUseStrictProtoMode: every emitted descriptor is Kind=SERVICE + Mode=STRICT_PROTO (host-side discriminator). - NilServer_ReturnsEmpty: defensive contract for nil input. Stacked on feat/iac-sdk-serve-task29 (Task 29 PR #600 provides ServeIaCPlugin which IaC plugins use to register the services this helper enumerates). Verification: GOWORK=off go test -race ./plugin/external/sdk/... PASS; GOWORK=off go build ./plugin/... ./cmd/... ./module/... clean; GOWORK=off go vet ./plugin/external/... clean. Rollback: revert this commit; ContractRegistry returns the prior shape (Module/Step/Trigger only via the existing ContractProvider hook in grpc_server.go). * test(sdk): typed-IaC E2E integration test + cross-plugin-build CI gate Task 6 of the strict-contracts force-cutover plan (docs/plans/2026-05-10-strict-contracts-force-cutover.md, rev5). Adds plugin/external/sdk/iac_e2e_test.go (build tag `integration`) — the canonical workflow-side smoke test for the typed IaC contract. Uses bufconn for in-process gRPC, registers a fake provider via sdk.RegisterAllIaCProviderServices, dials the server through a real gRPC channel, and exercises typed RPCs on both Required (Name, Version) and the Enumerator optional (EnumerateAll). Critical assertion: ResourceOutput.Sensitive (typed map<string,bool>) survives the roundtrip with value=true. The pre-cutover structpb path silently dropped this map (T3.9 finding); this E2E test guards the regression. Second case asserts that when a provider satisfies Required ONLY (no Enumerator embed), the auto-registration helper SKIPS the optional service registration — and a typed enumerator client receives a gRPC-layer Unimplemented error rather than a NotSupported flag in a response body. This is the "absence of registration IS the negative signal" contract from the design. CI integration (.github/workflows/cross-plugin-build-test.yml): - Adds an `iac-typed-e2e` job that runs the tests under -tags=integration on every IaC-touching PR. Per cycle 1 I-2 + cycle 2 I-1-NEW, `go build` alone leaves wire incompat between workflow and plugin grpc-go versions undetected; this job catches that bug class. - Extends the path filters to gate on plugin/external/**, so changes to the typed sdk helpers + iac.proto trigger this workflow rather than only the AWS/GCP/Azure compile-compat job. - The subprocess wire-test variant against the real DO plugin v1.0.0 binary is added once that plugin ships (per plan §PR 3 / Task 7+). Stacked on feat/iac-sdk-contracts-task5 (Task 5 PR #602 provides BuildContractRegistry; the E2E test exercises the surface from Tasks 3–5 + 29 end-to-end through gRPC). Verification: - GOWORK=off go test -tags=integration -race \ ./plugin/external/sdk/... -run TestIaC_EndToEnd → PASS (2/2) - GOWORK=off go test ./plugin/external/sdk/... → PASS (no regression in non-integration tests) - GOWORK=off go vet -tags=integration ./plugin/external/... → clean - actionlint .github/workflows/cross-plugin-build-test.yml → clean - python yaml.safe_load(...) → parses Rollback: revert this commit; no production code or contract is affected (test + CI YAML only). * test(sdk): pin OptionalNotRegistered E2E test to codes.Unimplemented Per cycle 4 code-review PR 603 MINOR-1: the previous assertion in TestIaC_EndToEnd_OptionalNotRegistered_ClientFailsTyped only checked err != nil — any error (network flake, deadline, transport-layer behavior change) would satisfy it, masking real Unimplemented-vs- other regressions in the absence-of-registration signal. Tightens the assertion to status.Code(err) == codes.Unimplemented so the test specifically pins the design's "absence of registration IS the negative signal" contract end-to-end at the gRPC layer. Verification: GOWORK=off go test -tags=integration -race ./plugin/external/sdk/... -run TestIaC_EndToEnd → PASS (2/2); gofmt clean. * test(sdk): bufconn cleanup + RPC deadline + clarified path filter (PR 603) Per cycle 4 code-review PR 603 (Copilot 4 Important + 1 MINOR): IMPORTANT-2/3 — bufconn listener leak: Both TestIaC_EndToEnd_* tests called bufconn.Listen but never closed the listener. server.Stop in t.Cleanup tears down the *grpc.Server but leaves the listener's accept goroutine alive until -race's GC pressure trips it. Adds `t.Cleanup(func() { _ = listener.Close() })` after each bufconn.Listen call. IMPORTANT-4/5 — RPC deadline: RPCs used context.Background() with no deadline → CI worker hangs until suite-wide timeout on transport failure. Replaces with `ctx, cancel := context.WithTimeout(context.Background(), e2eRPCDeadline)` (5s) + t.Cleanup(cancel). Both tests now bound their RPC time even if the gRPC layer wedges. e2eRPCDeadline lives at package scope alongside e2eBufSize so the per-test allocation reads cleanly and a future timeout bump is one line. MINOR-6 — path-filter intent comment: cross-plugin-build-test.yml `plugin/external/**` filter is broad on purpose — typed contract + sdk helpers + downstream IaC dispatch + remote-plugin orchestration code all live under this dir, and ALL of them affect the iac-typed-e2e job. Comment rewrites to document the intent (was: comment said only sdk helpers, suggesting a narrower path). Verification: GOWORK=off go test -tags=integration -race ./plugin/external/sdk/... -run TestIaC_EndToEnd → PASS (2/2); gofmt clean; actionlint clean. Rollback: revert this commit; bufconn listener leaks return + RPC unbounded; cross-plugin-build path filter intent comment returns to misleading wording.
intel352
added a commit
that referenced
this pull request
May 10, 2026
…d-braces guard (Task 15) (#606) * feat(proto): add iac.proto with IaCProviderRequired + 6 optional services + ResourceDriver Task 3 of the strict-contracts force-cutover plan (docs/plans/2026-05-10-strict-contracts-force-cutover.md, rev5). Adds plugin/external/proto/iac.proto defining the typed gRPC contract that supersedes the legacy InvokeService/structpb dispatch path for IaCProvider + ResourceDriver: - service IaCProviderRequired: 11 RPCs every IaC plugin MUST implement (Initialize, Name, Version, Capabilities, Plan, Apply, Destroy, Status, Import, ResolveSizing, BootstrapStateBackend). Compile-time enforced via the SDK type-assert in Task 4. - 6 optional services — providers register only the ones they support: IaCProviderEnumerator (EnumerateAll, EnumerateByTag), IaCProviderDriftDetector (DetectDrift, DetectDriftWithSpecs), IaCProviderCredentialRevoker (RevokeProviderCredential), IaCProviderMigrationRepairer (RepairDirtyMigration), IaCProviderValidator (ValidatePlan), IaCProviderDriftConfigDetector (DetectDriftConfig). Absence of registration IS the negative signal — no NotSupported field on any optional response (per design §Optional services). - service ResourceDriver: 9 RPCs for per-resource-type CRUD dispatch (Create, Read, Update, Delete, Diff, Scale, HealthCheck, SensitiveKeys, Troubleshoot), each carrying resource_type so a single server can route to the per-type driver implementation. Hard invariants honored: - NO google.protobuf.Struct, NO google.protobuf.Any anywhere. - Free-form per-resource Config/Outputs payloads cross the wire as bytes <name>_json (the plugin owns json.Marshal/Unmarshal); this eliminates the structpb conversion surface that previously dropped map[string]bool entries silently (T3.9 finding). - ResourceOutput.sensitive uses typed map<string, bool> per design. Generated iac.pb.go + iac_grpc.pb.go via protoc v34.1 + protoc-gen-go v1.36.11 + protoc-gen-go-grpc v1.6.1. Failing test (plugin/external/proto/iac_proto_test.go) asserts the generated server interfaces exist and have the methods the design requires — drops in iac.proto cause the test file to fail to compile. Verification: GOWORK=off go test ./plugin/external/proto/... PASSES; GOWORK=off go build ./plugin/... ./cmd/... ./module/... clean. Rollback: revert this commit; legacy InvokeService dispatch in plugin.proto remains functional; the additive-only nature of this PR means no consumer is affected until subsequent tasks wire callers. * feat(sdk): RegisterAllIaCProviderServices auto-registration helper Task 4 of the strict-contracts force-cutover plan (docs/plans/2026-05-10-strict-contracts-force-cutover.md, rev5). Adds plugin/external/sdk/iacserver.go: a single helper that uses Go type-assertion to register every typed IaC gRPC service the provider satisfies, in one call. REQUIRED service: pb.IaCProviderRequiredServer — surfaced as a clear startup-time error if the provider type doesn't satisfy it (rather than failing at the first RPC dispatch with a generic "unimplemented" status). OPTIONAL services (auto-detected): IaCProviderEnumerator, IaCProviderDriftDetector, IaCProviderCredentialRevoker, IaCProviderMigrationRepairer, IaCProviderValidator, IaCProviderDriftConfigDetector. Plus ResourceDriver. Per cycle 3 I-1 of the design: plugin authors write ONE call; they cannot omit registration for a capability they implemented. This removes the registration-omission bug class (the same shape as the legacy InvokeService case-string-typo bug) by removing the manual step entirely. Tests cover four cases: - required-satisfied → required service registered + advertised by grpcSrv.GetServiceInfo(). - enumerator-only → only the optional Enumerator service registered; other optionals stay absent (auto-detection precision). - empty-stub → returns an error naming the unsatisfied required interface, with a docs pointer. - all-capabilities-stub → all 8 typed services (Required + 6 optional + ResourceDriver) registered. Stacked on feat/iac-proto-task3 (Task 3 PR #598 provides the generated server interfaces this helper consumes). Verification: GOWORK=off go test -race ./plugin/external/sdk/... PASS; GOWORK=off go build ./plugin/... ./cmd/... ./module/... clean; GOWORK=off go vet ./plugin/external/... clean. Rollback: revert this commit; SDK consumers can still register services manually via the per-service Register* helpers protoc generated. * feat(sdk): ServeIaCPlugin high-level entrypoint with go-plugin GRPCServer callback Task 29 of the strict-contracts force-cutover plan (docs/plans/2026-05-10-strict-contracts-force-cutover.md, rev5). Adds the high-level plugin-author API on top of Task 4's RegisterAllIaCProviderServices: func main() { sdk.ServeIaCPlugin(&doProvider{}, sdk.IaCServeOptions{}) } Per cycle 3 I-1 of the design, service registration happens INSIDE go-plugin's GRPCServer callback (iacGRPCPlugin.GRPCServer) — the framework owns *grpc.Server lifecycle, so plugin authors cannot pre-create a server and forget to register a typed service on it. API surface (all in plugin/external/sdk/iacserver.go): - IaCServeOptions{ PluginInfo *PluginInfo } — caller-side options. - PluginInfo{ HandshakeConfig goplugin.HandshakeConfig } — extension point for future Name/Version metadata; defaults to ext.Handshake (the canonical wfctl<->plugin handshake) when zero-valued. - iacGRPCPlugin{provider any} — implements goplugin.Plugin (GRPCServer + GRPCClient). The GoCodeAlone fork of go-plugin v1.7.0 is gRPC-only and exposes only the canonical Plugin interface; there is no GRPCPlugin alias or NetRPCUnsupportedPlugin embed to use. - ServeIaCPlugin(provider, opts) — wraps goplugin.Serve with the resolved handshake + a single iacGRPCPlugin entry under the "iac" key. - resolveServeHandshake(opts) — extracted helper so the override-vs- default rule is unit-testable without invoking the blocking goplugin.Serve loop. Tests (iacserver_serve_test.go) cover six cases via internal-package tests (so the unexported plugin type is exercisable without a real subprocess; subprocess-level coverage lands in Task 6's typed-IaC E2E test): - iacGRPCPlugin.GRPCServer registers all satisfied services on the framework-managed *grpc.Server (Required + Enumerator + ResourceDriver for the all-stub). - iacGRPCPlugin.GRPCServer propagates the auto-register error for an empty stub — go-plugin aborts plugin startup with an actionable message. - iacGRPCPlugin.GRPCClient is a no-op (host builds typed clients directly). - iacGRPCPlugin satisfies goplugin.Plugin at compile time (refactor guard). - ServeIaCPlugin defaults to ext.Handshake when PluginInfo is nil. - ServeIaCPlugin honors a non-zero override handshake when provided. Stacked on feat/iac-sdk-auto-register-task4 (Task 4 PR #599 provides RegisterAllIaCProviderServices, which the GRPCServer callback delegates to). Verification: GOWORK=off go test -race ./plugin/external/sdk/... PASS; GOWORK=off go build ./plugin/... ./cmd/... ./module/... clean; GOWORK=off go vet ./plugin/external/... clean. Rollback: revert this commit; plugin authors can fall back to manually constructing goplugin.Serve + Plugins map referencing RegisterAllIaCProviderServices in their own GRPCServer callback. * feat(sdk): BuildContractRegistry advertises registered IaC services Task 5 of the strict-contracts force-cutover plan (docs/plans/2026-05-10-strict-contracts-force-cutover.md, rev5). Adds plugin/external/sdk/contracts.go with the BuildContractRegistry helper that enumerates grpc.Server.GetServiceInfo() and emits a SERVICE-kind ContractDescriptor for each registered service. ContractMode is set to STRICT_PROTO so the host can distinguish typed IaC services from the legacy structpb-mode contracts produced by Module/Step/Trigger ContractProvider implementations. Per cycle 3 I-1 of the design: wfctl needs a single mechanism to discover "is the optional service registered on this plugin handle?". Reusing the existing ContractRegistry shape keeps Module/Step/Trigger and IaC capability discovery on the same wire surface — no new gRPC server-reflection dependency required. Service descriptors are emitted in deterministic alphabetical order so callers can rely on stable output for diff/compare operations and the wftest BDD test in Task 15. The helper is safe to call with a nil server (returns an empty but non-nil ContractRegistry) so callers that may construct it before the gRPC server exists do not panic. Tests (contracts_iac_test.go) cover three cases — all pass: - AdvertisesRegisteredIaCServices: a Required + Enumerator + DriftDetector stub yields exactly those service descriptors. - ServiceContractsUseStrictProtoMode: every emitted descriptor is Kind=SERVICE + Mode=STRICT_PROTO (host-side discriminator). - NilServer_ReturnsEmpty: defensive contract for nil input. Stacked on feat/iac-sdk-serve-task29 (Task 29 PR #600 provides ServeIaCPlugin which IaC plugins use to register the services this helper enumerates). Verification: GOWORK=off go test -race ./plugin/external/sdk/... PASS; GOWORK=off go build ./plugin/... ./cmd/... ./module/... clean; GOWORK=off go vet ./plugin/external/... clean. Rollback: revert this commit; ContractRegistry returns the prior shape (Module/Step/Trigger only via the existing ContractProvider hook in grpc_server.go). * test(wftest/bdd): AssertProviderCapabilitiesMatchRegistration belt-and-braces guard Task 15 of the strict-contracts force-cutover plan (docs/plans/2026-05-10-strict-contracts-force-cutover.md, rev5). Adds wftest/bdd/strict_iac.go with the AssertProviderCapabilitiesMatchRegistration helper. Given a Go provider implementation + the gRPC server with the plugin's service registrations, the helper asserts: - Pass 1: every typed IaC service interface satisfied by the provider's Go type IS registered on the gRPC server. - Pass 2: no IaC service is registered on the gRPC server that the provider's Go type does NOT satisfy (catches the "manual Register* call binds a different Go impl" failure mode). Per cycle 4 belt-and-braces of the design: the canonical registration path is sdk.RegisterAllIaCProviderServices, which uses Go type-assertion to auto-detect every interface and cannot omit a registration. The per-service Register* helpers are still exposed for advanced use cases; this helper is the test-time guard that catches the manual-registration omission failure mode. iacServiceChecks lists every typed IaC service the helper knows about (Required + 6 optional + ResourceDriver). New optional services added to iac.proto must be appended here. Helper takes a strictIaCT interface (Errorf + Fatalf + Helper) rather than *testing.T directly so the failure path itself is unit-testable via a recordingT double — the four tests in strict_iac_test.go cover the four behavior axes: - AutoRegisteredAllOK: silent pass when sdk.RegisterAllIaCProviderServices was used as designed. - ManuallyRegisteredMissingOptional_Fails: provider satisfies every optional + ResourceDriver but author registered only Required + Enumerator manually → helper names every missing service in the error messages. - ProviderMissingRequired_Fails: broken test fixture (provider doesn't satisfy IaCProviderRequiredServer) → fatal-class failure with the missing interface named. - RegisteredButProviderDoesntSatisfy_Fails: server has Enumerator registered but provider passed to assert is requiredOnlyStub → over-registration named in error. Stacked on feat/iac-sdk-contracts-task5 (Task 5 PR #602 provides the typed proto interfaces this helper inspects via reflection- free type-assert chain). Verification: GOWORK=off go test -race ./wftest/... PASS; GOWORK=off go build ./... clean; GOWORK=off go vet ./wftest/... clean. Rollback: revert this commit; canonical registration path (sdk.RegisterAllIaCProviderServices) is unaffected. * test(wftest/bdd): protoreflect coverage check for iacServiceChecks Per cycle 4 code-review PR 606 MINOR-1: iacServiceChecks in wftest/bdd/strict_iac.go is a manually-maintained list of typed IaC services. If iac.proto adds a new optional service later without appending to iacServiceChecks, AssertProviderCapabilitiesMatchRegistration silently passes for the new service (the satisfies check is never invoked for unlisted services), leaving a hole in the cycle 4 belt- and-braces invariant. Adds wftest/bdd/strict_iac_internal_test.go (package bdd, internal test so iacServiceChecks is in scope) with TestIaCServiceChecks_CoversEveryProtoService — walks pb.File_plugin_external_proto_iac_proto.Services() at runtime and asserts: - Forward: every IaC service name in the proto (prefix-matched on iacServicePrefix) appears in iacServiceChecks. Missing entries produce a test failure naming the service(s) the proto declares but the helper doesn't cover. - Inverse: every iacServiceCheck row names a service the proto actually declares. Catches the rename-without-cleanup failure mode (proto renames a service; iacServiceChecks still references the old name). Tightens the manual-maintenance surface into a compile-time- discoverable test failure rather than a silent test-passing regression. Verification: GOWORK=off go test ./wftest/bdd/... -run "TestIaCServiceChecks|TestAssertProviderCapabilities" -count=1 → PASS (5/5); gofmt clean. Rollback: revert this commit; the cycle 4 belt-and-braces guard returns to the manual-maintenance form (still works, just no test-time coverage check on the helper itself). * fix(wftest/bdd): use generated File_iac_proto descriptor (post-cascade-merge) Generated proto descriptor variable is File_iac_proto (per iac.proto's go_package option), not File_plugin_external_proto_iac_proto. Reference fixed; all 5 BDD strict-IaC tests PASS. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
intel352
added a commit
that referenced
this pull request
May 10, 2026
…tover runbook (Task 18) (#610) * feat(proto): add iac.proto with IaCProviderRequired + 6 optional services + ResourceDriver Task 3 of the strict-contracts force-cutover plan (docs/plans/2026-05-10-strict-contracts-force-cutover.md, rev5). Adds plugin/external/proto/iac.proto defining the typed gRPC contract that supersedes the legacy InvokeService/structpb dispatch path for IaCProvider + ResourceDriver: - service IaCProviderRequired: 11 RPCs every IaC plugin MUST implement (Initialize, Name, Version, Capabilities, Plan, Apply, Destroy, Status, Import, ResolveSizing, BootstrapStateBackend). Compile-time enforced via the SDK type-assert in Task 4. - 6 optional services — providers register only the ones they support: IaCProviderEnumerator (EnumerateAll, EnumerateByTag), IaCProviderDriftDetector (DetectDrift, DetectDriftWithSpecs), IaCProviderCredentialRevoker (RevokeProviderCredential), IaCProviderMigrationRepairer (RepairDirtyMigration), IaCProviderValidator (ValidatePlan), IaCProviderDriftConfigDetector (DetectDriftConfig). Absence of registration IS the negative signal — no NotSupported field on any optional response (per design §Optional services). - service ResourceDriver: 9 RPCs for per-resource-type CRUD dispatch (Create, Read, Update, Delete, Diff, Scale, HealthCheck, SensitiveKeys, Troubleshoot), each carrying resource_type so a single server can route to the per-type driver implementation. Hard invariants honored: - NO google.protobuf.Struct, NO google.protobuf.Any anywhere. - Free-form per-resource Config/Outputs payloads cross the wire as bytes <name>_json (the plugin owns json.Marshal/Unmarshal); this eliminates the structpb conversion surface that previously dropped map[string]bool entries silently (T3.9 finding). - ResourceOutput.sensitive uses typed map<string, bool> per design. Generated iac.pb.go + iac_grpc.pb.go via protoc v34.1 + protoc-gen-go v1.36.11 + protoc-gen-go-grpc v1.6.1. Failing test (plugin/external/proto/iac_proto_test.go) asserts the generated server interfaces exist and have the methods the design requires — drops in iac.proto cause the test file to fail to compile. Verification: GOWORK=off go test ./plugin/external/proto/... PASSES; GOWORK=off go build ./plugin/... ./cmd/... ./module/... clean. Rollback: revert this commit; legacy InvokeService dispatch in plugin.proto remains functional; the additive-only nature of this PR means no consumer is affected until subsequent tasks wire callers. * feat(sdk): RegisterAllIaCProviderServices auto-registration helper Task 4 of the strict-contracts force-cutover plan (docs/plans/2026-05-10-strict-contracts-force-cutover.md, rev5). Adds plugin/external/sdk/iacserver.go: a single helper that uses Go type-assertion to register every typed IaC gRPC service the provider satisfies, in one call. REQUIRED service: pb.IaCProviderRequiredServer — surfaced as a clear startup-time error if the provider type doesn't satisfy it (rather than failing at the first RPC dispatch with a generic "unimplemented" status). OPTIONAL services (auto-detected): IaCProviderEnumerator, IaCProviderDriftDetector, IaCProviderCredentialRevoker, IaCProviderMigrationRepairer, IaCProviderValidator, IaCProviderDriftConfigDetector. Plus ResourceDriver. Per cycle 3 I-1 of the design: plugin authors write ONE call; they cannot omit registration for a capability they implemented. This removes the registration-omission bug class (the same shape as the legacy InvokeService case-string-typo bug) by removing the manual step entirely. Tests cover four cases: - required-satisfied → required service registered + advertised by grpcSrv.GetServiceInfo(). - enumerator-only → only the optional Enumerator service registered; other optionals stay absent (auto-detection precision). - empty-stub → returns an error naming the unsatisfied required interface, with a docs pointer. - all-capabilities-stub → all 8 typed services (Required + 6 optional + ResourceDriver) registered. Stacked on feat/iac-proto-task3 (Task 3 PR #598 provides the generated server interfaces this helper consumes). Verification: GOWORK=off go test -race ./plugin/external/sdk/... PASS; GOWORK=off go build ./plugin/... ./cmd/... ./module/... clean; GOWORK=off go vet ./plugin/external/... clean. Rollback: revert this commit; SDK consumers can still register services manually via the per-service Register* helpers protoc generated. * feat(sdk): ServeIaCPlugin high-level entrypoint with go-plugin GRPCServer callback Task 29 of the strict-contracts force-cutover plan (docs/plans/2026-05-10-strict-contracts-force-cutover.md, rev5). Adds the high-level plugin-author API on top of Task 4's RegisterAllIaCProviderServices: func main() { sdk.ServeIaCPlugin(&doProvider{}, sdk.IaCServeOptions{}) } Per cycle 3 I-1 of the design, service registration happens INSIDE go-plugin's GRPCServer callback (iacGRPCPlugin.GRPCServer) — the framework owns *grpc.Server lifecycle, so plugin authors cannot pre-create a server and forget to register a typed service on it. API surface (all in plugin/external/sdk/iacserver.go): - IaCServeOptions{ PluginInfo *PluginInfo } — caller-side options. - PluginInfo{ HandshakeConfig goplugin.HandshakeConfig } — extension point for future Name/Version metadata; defaults to ext.Handshake (the canonical wfctl<->plugin handshake) when zero-valued. - iacGRPCPlugin{provider any} — implements goplugin.Plugin (GRPCServer + GRPCClient). The GoCodeAlone fork of go-plugin v1.7.0 is gRPC-only and exposes only the canonical Plugin interface; there is no GRPCPlugin alias or NetRPCUnsupportedPlugin embed to use. - ServeIaCPlugin(provider, opts) — wraps goplugin.Serve with the resolved handshake + a single iacGRPCPlugin entry under the "iac" key. - resolveServeHandshake(opts) — extracted helper so the override-vs- default rule is unit-testable without invoking the blocking goplugin.Serve loop. Tests (iacserver_serve_test.go) cover six cases via internal-package tests (so the unexported plugin type is exercisable without a real subprocess; subprocess-level coverage lands in Task 6's typed-IaC E2E test): - iacGRPCPlugin.GRPCServer registers all satisfied services on the framework-managed *grpc.Server (Required + Enumerator + ResourceDriver for the all-stub). - iacGRPCPlugin.GRPCServer propagates the auto-register error for an empty stub — go-plugin aborts plugin startup with an actionable message. - iacGRPCPlugin.GRPCClient is a no-op (host builds typed clients directly). - iacGRPCPlugin satisfies goplugin.Plugin at compile time (refactor guard). - ServeIaCPlugin defaults to ext.Handshake when PluginInfo is nil. - ServeIaCPlugin honors a non-zero override handshake when provided. Stacked on feat/iac-sdk-auto-register-task4 (Task 4 PR #599 provides RegisterAllIaCProviderServices, which the GRPCServer callback delegates to). Verification: GOWORK=off go test -race ./plugin/external/sdk/... PASS; GOWORK=off go build ./plugin/... ./cmd/... ./module/... clean; GOWORK=off go vet ./plugin/external/... clean. Rollback: revert this commit; plugin authors can fall back to manually constructing goplugin.Serve + Plugins map referencing RegisterAllIaCProviderServices in their own GRPCServer callback. * feat(sdk): BuildContractRegistry advertises registered IaC services Task 5 of the strict-contracts force-cutover plan (docs/plans/2026-05-10-strict-contracts-force-cutover.md, rev5). Adds plugin/external/sdk/contracts.go with the BuildContractRegistry helper that enumerates grpc.Server.GetServiceInfo() and emits a SERVICE-kind ContractDescriptor for each registered service. ContractMode is set to STRICT_PROTO so the host can distinguish typed IaC services from the legacy structpb-mode contracts produced by Module/Step/Trigger ContractProvider implementations. Per cycle 3 I-1 of the design: wfctl needs a single mechanism to discover "is the optional service registered on this plugin handle?". Reusing the existing ContractRegistry shape keeps Module/Step/Trigger and IaC capability discovery on the same wire surface — no new gRPC server-reflection dependency required. Service descriptors are emitted in deterministic alphabetical order so callers can rely on stable output for diff/compare operations and the wftest BDD test in Task 15. The helper is safe to call with a nil server (returns an empty but non-nil ContractRegistry) so callers that may construct it before the gRPC server exists do not panic. Tests (contracts_iac_test.go) cover three cases — all pass: - AdvertisesRegisteredIaCServices: a Required + Enumerator + DriftDetector stub yields exactly those service descriptors. - ServiceContractsUseStrictProtoMode: every emitted descriptor is Kind=SERVICE + Mode=STRICT_PROTO (host-side discriminator). - NilServer_ReturnsEmpty: defensive contract for nil input. Stacked on feat/iac-sdk-serve-task29 (Task 29 PR #600 provides ServeIaCPlugin which IaC plugins use to register the services this helper enumerates). Verification: GOWORK=off go test -race ./plugin/external/sdk/... PASS; GOWORK=off go build ./plugin/... ./cmd/... ./module/... clean; GOWORK=off go vet ./plugin/external/... clean. Rollback: revert this commit; ContractRegistry returns the prior shape (Module/Step/Trigger only via the existing ContractProvider hook in grpc_server.go). * feat(wfctl): IaC plugin pre-flight gate building block + iac-typed-cutover runbook Task 18 of the strict-contracts force-cutover plan (docs/plans/2026-05-10-strict-contracts-force-cutover.md, rev5). Adds two pieces: 1. cmd/wfctl/iac_loader_gate.go — the pre-flight gate building block. AssertIaCPluginAdvertisesRequiredService(name, version, registry) inspects a *pb.ContractRegistry response from GetContractRegistry and returns nil iff the plugin advertises workflow.plugin.external.iac.IaCProviderRequired as a CONTRACT_KIND_SERVICE descriptor. On failure: an actionable error naming the offending plugin + version, citing .wfctl-lock.yaml as the migration target, and pointing to docs/runbooks/iac-typed-cutover.md, wrapping errLegacyIaCPlugin for IsLegacyIaCPluginErr dispatch. The function is the load-bearing predicate the deploy / infra-plan / infra-apply call sites will invoke after their GetContractRegistry RPC succeeds (wiring left for the typed- client cutover already in progress in Task 16; this commit ships the testable predicate so the wiring is one-line drop-in). 2. docs/runbooks/iac-typed-cutover.md — operator-facing runbook for the cutover. Covers: - Compatibility matrix (wfctl × DO plugin version combinations, pre-flight expected outcomes). - Upgrade order: plugins first, then wfctl. Documents WHY this ordering matters (legacy wfctl + v1.0.0 plugin works; v1.0.0 wfctl + legacy plugin refuses). - .wfctl-lock.yaml migration (lockfile shape unchanged; only the version pin needs editing). - Troubleshooting: legacy-plugin pre-flight error, EnumeratorAll-style messages from pre-cutover wfctl, state-file decode regressions, advertised-but-buggy plugins. - Backout plan: re-pin legacy plugin + wfctl while in the rc1 window. Tests (cmd/wfctl/iac_loader_gate_test.go) — 7 cases, all PASS: - TypedRegistryAccepts: SERVICE-kind descriptor for IaCProviderRequired. - LegacyRegistryRejects: ContractRegistry without the required service → wrapped error names plugin + version + runbook. - NilRegistryRejects: defensive contract for nil registry input. - EmptyContractsRejects: no descriptors at all → still legacy. - WrongKindRejects: ServiceName matches but Kind != SERVICE. - EmptyMetadataDefaults: name/version absent → graceful "<unknown>". - IsLegacyIaCPluginErr_NoFalsePositives: sentinel match is typed (not string-based) so dispatch sites can errors.Is cleanly. Stacked on feat/iac-sdk-contracts-task5 (Task 5 PR #602 provides BuildContractRegistry — the wfctl-side gate inspects what plugins emit via that helper, so this PR builds on it). Verification: GOWORK=off go test -race ./cmd/wfctl/... PASS; GOWORK=off go vet ./cmd/wfctl/ clean; gofmt clean. Rollback: revert this commit. The pre-flight gate is a standalone predicate function — no production code currently depends on it (dispatch wiring lands in the typed-client cutover Task 16). The runbook is documentation; no operational impact from removal. * docs(runbook): correct cutover model — wfctl-side rc1 adapter, not plugin compat shim Per spec-reviewer's PR 610 IMPORTANT-1 + team-lead's ruling on the correct cutover model (which I as ADR 0024 author should have known already): The runbook previously claimed "v1.0.0 plugins also expose the legacy InvokeService surface during the rc1 window for backward compatibility" and "backout fully supported through rc1 window (workflow v1.0.0-rc1 + DO plugin v1.0.0)". Both wrong. Contradicts ADR 0024 (force-cutover, no compat shim, no build-tag dual-path), Task 9 spec literal ("DELETE module_instance.go"), and workspace memory feedback_force_strict_contracts_no_compat. Corrected cutover model: - workflow v1.0.0-rc1 ships the wfctl-SIDE typed-client adapter alongside the existing wfctl remoteIaCProvider. Plugins are unchanged at v0.14.x; the typed adapter is exercised only when a typed-aware plugin is loaded. This is rc1's role: wfctl-side additive, not plugin-side compat shim. - DO plugin v1.0.0 ships typed-only (Task 9 deletes the legacy internal/module_instance.go switch dispatcher entirely; ResourceDriver Task 11 follows the same pattern). - workflow v1.0.0 final ships typed-only on the wfctl side too (Task 20 removes wfctl-side remoteIaCProvider). Both legacy paths are gone. Specific revisions applied: 1. Compatibility matrix: added the missing v0.27.x × v1.0.0 row marked "BROKEN by design" — operators MUST run wfctl rc1+ to consume DO v1.0.0 because the legacy wfctl binary cannot dispatch through a typed-only plugin. Added explicit Step 4 troubleshooting block for operators who skipped wfctl rc1. 2. Upgrade order rewrite: 5 steps (was 3). Order is wfctl rc1 first (test against v0.14.x plugin set), then DO plugin v1.0.0, then wfctl v1.0.0 final. Each step has its smoke-test command and rollback escape. 3. Backout plan rewrite: explicit "rollback BOTH wfctl AND plugin together" for the post-v1.0.0-final case. Single-side rollback is broken — wfctl v1.0.0 final binary cannot dispatch v0.14.x plugin (legacy adapter deleted in Task 20); legacy v0.27.x wfctl cannot dispatch typed-only DO v1.0.0 (no typed adapter on the legacy binary). Documents the partial-rollback option that exists during the rc1 window only. 4. Top-of-runbook callout: explicit statement that this is a hard cutover with no plugin-side compat shim, citing ADR 0024 and feedback_force_strict_contracts_no_compat. No code changes; documentation only. Verification: markdown rendered locally; cross-references to ADRs + plan + design + state_compat_test.go all resolve. Followups (per spec-reviewer + team-lead Finding 2): - PR description gets a "Wiring deferred to Task 16 (PR 609)" callout with predicate symbol + sentinel + call-site location - Coordination DM to implementer-2 with the wiring contract * refactor(wfctl): rename iacRequiredServiceName → iacServiceRequired Cross-task naming coordination with PR #605/#609 (implementer-2, Tasks 30/16). Their iac_typed_adapter.go declares 8 sibling consts naming every typed IaC service: iacServiceRequired iacServiceEnumerator iacServiceDriftDetector iacServiceCredentialRevoker iacServiceMigrationRepairer iacServiceValidator iacServiceDriftConfigDetect iacServiceResourceDriver PR #610 originally introduced its own const iacRequiredServiceName for the same Required service FQN. Spec-reviewer flagged the inconsistency and asked us to coordinate. Implementer-2 + I agreed their convention wins (8 siblings establish the pattern; my single const matches). Pure rename; no behavior change. Tests still pass. Verification: GOWORK=off go test ./cmd/wfctl/ -run \ "TestAssertIaCPluginAdvertises|TestIsLegacyIaCPluginErr" \ -count=1 → PASS (7/7); gofmt clean. Once PR #610 merges, implementer-2's PR #609 rebase can drop their duplicate const and import iacServiceRequired from here. * fix(wfctl): drop duplicate iacServiceRequired const (now lives in iac_typed_adapter.go on main) PR #605 merged iac_typed_adapter.go to main with iacServiceRequired const. PR #610's iac_loader_gate.go declared it independently; collision after cascade-merge. Removed the duplicate; use canonical const from iac_typed_adapter.go directly. Build clean. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
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
Task 29 of the strict-contracts force-cutover plan (docs/plans/2026-05-10-strict-contracts-force-cutover.md, rev5).
Adds the high-level plugin-author API on top of Task 4's
RegisterAllIaCProviderServices:Per cycle 3 I-1, service registration happens INSIDE go-plugin's
GRPCServercallback (iacGRPCPlugin.GRPCServer) — the framework owns*grpc.Serverlifecycle, so plugin authors cannot pre-create a server and forget to register a typed service on it.Stacked on PR #599 (Task 4). Base branch is
feat/iac-sdk-auto-register-task4.API surface (
plugin/external/sdk/iacserver.go)IaCServeOptions{ PluginInfo *PluginInfo }— caller-side optionsPluginInfo{ HandshakeConfig goplugin.HandshakeConfig }— extension point for future Name/Version metadata; defaults toext.Handshakewhen zero-valuediacGRPCPlugin{provider any}— implementsgoplugin.Plugin(GRPCServer + GRPCClient). The GoCodeAlone fork of go-plugin v1.7.0 is gRPC-only and exposes only the canonicalPlugininterface (noGRPCPluginalias /NetRPCUnsupportedPluginembed)ServeIaCPlugin(provider, opts)— wrapsgoplugin.Servewith the resolved handshake + a singleiacGRPCPluginentry under the"iac"keyresolveServeHandshake(opts)— extracted helper so the override-vs-default rule is unit-testable without invoking the blockinggoplugin.ServeloopTests (
iacserver_serve_test.go, internal-package)Six cases — all PASS:
GRPCServer_RegistersAllServices— auto-registers Required + Enumerator + ResourceDriver on the framework-managed gRPC serverGRPCServer_PropagatesAutoRegisterError— empty stub yields a startup-aborting errorGRPCClient_NoOp— plugin-side client adapter returns(nil, nil)SatisfiesGoPluginPlugin— compile-time refactor guardDefaultsToWorkflowHandshake_WhenPluginInfoNil— default isext.HandshakeHonorsOverrideHandshake_WhenProvided— caller can override (e.g., for non-workflow hosts)Subprocess-level coverage of
ServeIaCPluginend-to-end lands in Task 6's typed-IaC E2E test.Verification
GOWORK=off go test -race ./plugin/external/sdk/...→ PASSGOWORK=off go build ./plugin/... ./cmd/... ./module/...→ cleanGOWORK=off go vet ./plugin/external/...→ cleanRollback
Revert this commit; plugin authors can fall back to manually constructing
goplugin.Serve+Pluginsmap referencingRegisterAllIaCProviderServicesin their ownGRPCServercallback.Test plan
-v -run TestIaCGRPCPlugin|TestServeIaCPlugin)🤖 Generated with Claude Code