Cloud-SDK Plan #2 — PR 1: IaCServeOptions.Modules + .Steps + mapBackedProvider adapter#683
Merged
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR extends the external plugin IaC SDK entrypoint (sdk.ServeIaCPlugin) so typed-IaC plugins can additionally expose module and step factories via the existing PluginService module/step lifecycle RPCs (without proto changes), using a thin mapBackedProvider adapter and an optional delegate inside the IaC PluginService bridge.
Changes:
- Added
IaCServeOptions.ModulesandIaCServeOptions.Stepsto allow serving module/step factories from IaC plugins. - Updated
iacPluginServiceBridgeto optionally forward module/step RPCs togrpc_server.go’s existinggrpcServerimplementation. - Added unit + bufconn tests covering delegation behavior, backward-compat (Unimplemented on zero-value), and a regression guard around message broker plumbing.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| plugin/external/sdk/iacserver.go | Adds IaCServeOptions.Modules/Steps, mapBackedProvider, and delegate-forwarding methods on the IaC PluginService bridge. |
| plugin/external/sdk/iacserver_modules_test.go | Unit tests verifying delegate wiring, zero-value backward compatibility, and a nil-broker regression guard. |
| plugin/external/sdk/iac_modules_e2e_test.go | Bufconn end-to-end test exercising pb.PluginServiceClient GetModuleTypes/CreateModule round-trip through the IaC bridge. |
Comments suppressed due to low confidence (1)
plugin/external/sdk/iacserver.go:378
- mapBackedProvider.StepTypes ranges over a Go map, so the returned list order is nondeterministic. To avoid flaky behavior and keep type listings stable across runs, sort the collected keys before returning.
// StepTypes returns the keys of the steps map.
func (p *mapBackedProvider) StepTypes() []string {
out := make([]string, 0, len(p.steps))
for name := range p.steps {
out = append(out, name)
}
return out
Comment on lines
+354
to
+360
| // ModuleTypes returns the keys of the modules map. | ||
| func (p *mapBackedProvider) ModuleTypes() []string { | ||
| out := make([]string, 0, len(p.modules)) | ||
| for name := range p.modules { | ||
| out = append(out, name) | ||
| } | ||
| return out |
Contributor
Author
There was a problem hiding this comment.
Fixed in 0bca646 — sort.Strings on mapBackedProvider.ModuleTypes/StepTypes (real determinism bug); new TestIaCBridge_ModuleStepTypes_Deterministic regression test.
Comment on lines
+92
to
+99
| go func() { _ = s.Serve(lis) }() | ||
| t.Cleanup(s.Stop) | ||
| conn, err := grpc.NewClient("passthrough://bufnet", | ||
| grpc.WithContextDialer(func(ctx context.Context, _ string) (net.Conn, error) { | ||
| return lis.DialContext(ctx) | ||
| }), | ||
| grpc.WithTransportCredentials(insecure.NewCredentials()), | ||
| ) |
Contributor
Author
There was a problem hiding this comment.
Fixed in 0bca646 — bufconn dial target normalized to passthrough:///bufnet (canonical gRPC URI form, aligns with in-tree convention).
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
⏱ Benchmark Results✅ No significant performance regressions detected. benchstat comparison (baseline → PR)
|
Addresses 2 Copilot findings on PR #683 (plan-2 Tasks 1+2): 1. mapBackedProvider.ModuleTypes / StepTypes — Go map iteration is randomized, so the previous unsorted slice would differ run-to-run, breaking cache keys + any caller that compares as an ordered list. Sort the keys lexicographically before returning. 2. Bufconn dial target — the iacserver_modules_test.go dial used `passthrough://bufnet` (double-slash, wrong); the rest of the sdk bufconn tests use `passthrough:///bufnet` (triple-slash, gRPC URI form). Aligned with the in-tree convention. Added TestIaCBridge_ModuleStepTypes_Deterministic: 3 entries inserted non-alphabetically, expects alphabetical back, asserts across 5 iterations to catch a non-sorted impl that happens to win on a single race. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
This was referenced May 15, 2026
Merged
intel352
added a commit
that referenced
this pull request
May 16, 2026
…affolding (#689) Records the resolution of two interlocking decisions made mid-execution of plan-2 (plugin-modules-on-iac): 1. (c1) for plan-2 plugin shipping: aws + gcp v1.1.0 carry strict descriptors backing legacy ModuleProvider Go impls, satisfying wfctl plugin audit without forcing an eager typed-Provider migration. 2. SDK Typed-fields scaffolding (PR #686 #683): IaCServeOptions now carries TypedModules + TypedSteps alongside Modules + Steps; mapBackedProvider implements both contracts; grpc_server's Typed-first → legacy-fallback path is reused. (c2) — relaxing the validator — rejected as eroding the strict-cutover value (decisions/0024). (B1) — eager typed migration — rejected as blocking the Phase B/C deletions per the user's "just get the remaining plugins finished up" mandate. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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
sdk.ServeIaCPluginto also serve module + step factories: addsIaCServeOptions.Modules map[string]sdk.ModuleProvider+Steps map[string]sdk.StepProvider; thinmapBackedProvideradapter (~30 LOC) implementsPluginProvider+ModuleProvider+StepProviderby delegating to the supplied maps;iacPluginServiceBridgeconstructsnewGRPCServer(mapBackedProvider)when those fields are non-nil sogrpc_server.go's existing PluginService Module/Step lifecycle is reused unchanged.pb.PluginServiceServer; no proto change.docs/plans/2026-05-14-cloud-sdk-extraction-bcd.md): plugins served viaServeIaCPlugincouldn't register module/step factories. Perdecisions/0038.Plan 2 PR 1 of
docs/plans/2026-05-15-plugin-modules-on-iac.md(locked sha256 8964f744d0ca, 5 PRs / 19 tasks; PR 2/3 unblock after this merges + workflow v0.53.0 tag is cut).Test plan
iacserver_modules_test.go: delegate-path round-trip; zero-valueUnimplementedinvariant;NilBroker_NoMessagePublisherCallregression guard locking the v1 Non-Goal in codeiac_modules_e2e_test.go:pb.PluginServiceClient.GetModuleTypes+CreateModuleround-trip — same gRPC dispatch the engine's adapter usesGOWORK=off go build ./... && GOWORK=off go test ./...green (no regression)🤖 Generated with Claude Code