From 226c2fca4e994c08abafaeb846ea552b96d022fe Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 10 May 2026 20:11:14 +0000 Subject: [PATCH 1/3] Initial plan From 0097671ad8dee73d5690958babe66328792bb556 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 10 May 2026 20:38:24 +0000 Subject: [PATCH 2/3] fix(plugin/external/sdk): register PluginService bridge in RegisterAllIaCProviderServices for GetContractRegistry support ServeIaCPlugin (used by DO plugin v1.0.0) calls RegisterAllIaCProviderServices which only registered typed IaC gRPC services. When wfctl's NewExternalPluginAdapter called GetContractRegistry via pb.NewPluginServiceClient, the plugin returned 'unknown service workflow.plugin.v1.PluginService', blocking the typedIaCAdapter load path in buildTypedIaCAdapterFrom. Fix: after registering IaC services, also register a minimal iacPluginServiceBridge as PluginServiceServer. The bridge's GetContractRegistry delegates to BuildContractRegistry(s) which enumerates all registered gRPC services on the server, returning the IaC service descriptors wfctl needs for optional-client construction. A guard prevents double-registration on mixed plugins that also run the SDK grpcServer. Tests: - TestRegisterAllIaCProviderServices_PluginServiceBridgeRegistered - TestRegisterAllIaCProviderServices_PluginServiceBridgeAnswersGetContractRegistry (end-to-end via live in-process gRPC client) - TestRegisterAllIaCProviderServices_PluginServiceAlreadyRegistered_NoPanic Agent-Logs-Url: https://github.com/GoCodeAlone/workflow/sessions/8995eb0b-2f92-48fc-ac39-7b3b9a8a4c00 Co-authored-by: intel352 <77607+intel352@users.noreply.github.com> --- plugin/external/sdk/iacserver.go | 38 ++++++++++++ plugin/external/sdk/iacserver_test.go | 83 +++++++++++++++++++++++++++ 2 files changed, 121 insertions(+) diff --git a/plugin/external/sdk/iacserver.go b/plugin/external/sdk/iacserver.go index 6ee5ce45..af972482 100644 --- a/plugin/external/sdk/iacserver.go +++ b/plugin/external/sdk/iacserver.go @@ -7,6 +7,7 @@ import ( goplugin "github.com/GoCodeAlone/go-plugin" "google.golang.org/grpc" + "google.golang.org/protobuf/types/known/emptypb" ext "github.com/GoCodeAlone/workflow/plugin/external" pb "github.com/GoCodeAlone/workflow/plugin/external/proto" @@ -95,9 +96,46 @@ func RegisterAllIaCProviderServices(s *grpc.Server, provider any) error { if v, ok := provider.(pb.ResourceDriverServer); ok { pb.RegisterResourceDriverServer(s, v) } + + // Register a minimal PluginService so the wfctl host can call + // GetContractRegistry to discover the typed IaC services registered + // above. Strict-cutover IaC plugins (e.g. DO v1.0.0) that use + // ServeIaCPlugin do NOT register the SDK grpcServer (which normally + // handles GetContractRegistry for non-IaC plugins). Without this + // bridge, wfctl's NewExternalPluginAdapter fails with + // "unknown service workflow.plugin.v1.PluginService" when it calls + // GetContractRegistry, blocking the typedIaCAdapter load path. + // + // Guard: skip registration if PluginService is already on the server + // (e.g. a mixed plugin that called sdk.Serve AND RegisterAllIaC). + // gRPC panics on double-registration; the guard prevents that. + if _, alreadyRegistered := s.GetServiceInfo()["workflow.plugin.v1.PluginService"]; !alreadyRegistered { + pb.RegisterPluginServiceServer(s, &iacPluginServiceBridge{grpcSrv: s}) + } return nil } +// iacPluginServiceBridge is a minimal pb.PluginServiceServer registered on +// the gRPC server by RegisterAllIaCProviderServices. It implements only +// GetContractRegistry, delegating to BuildContractRegistry so the wfctl +// host can discover which typed IaC services the plugin registered. +// +// All other PluginService methods (InvokeService, GetManifest, etc.) remain +// unimplemented (via UnimplementedPluginServiceServer) — strict-cutover IaC +// plugins do not support string-dispatch or module/step/trigger contracts. +type iacPluginServiceBridge struct { + pb.UnimplementedPluginServiceServer + grpcSrv *grpc.Server +} + +// GetContractRegistry returns the set of gRPC services registered on +// grpcSrv at call time, encoded as a *pb.ContractRegistry. wfctl uses +// this to gate optional typed-client construction (Enumerator, DriftDetector, +// etc.) after loading an IaC plugin via discoverAndLoadIaCProvider. +func (b *iacPluginServiceBridge) GetContractRegistry(_ context.Context, _ *emptypb.Empty) (*pb.ContractRegistry, error) { + return BuildContractRegistry(b.grpcSrv), nil +} + // IaCServeOptions configures the IaC plugin gRPC server entrypoint. // // Plugin authors typically zero-value this; ServeIaCPlugin then uses the diff --git a/plugin/external/sdk/iacserver_test.go b/plugin/external/sdk/iacserver_test.go index 90f12abb..83208949 100644 --- a/plugin/external/sdk/iacserver_test.go +++ b/plugin/external/sdk/iacserver_test.go @@ -1,10 +1,14 @@ package sdk_test import ( + "context" + "net" "strings" "testing" "google.golang.org/grpc" + "google.golang.org/grpc/credentials/insecure" + "google.golang.org/protobuf/types/known/emptypb" pb "github.com/GoCodeAlone/workflow/plugin/external/proto" "github.com/GoCodeAlone/workflow/plugin/external/sdk" @@ -147,3 +151,82 @@ type allCapabilitiesStub struct { // emptyStub satisfies no IaC interface; the helper must reject it. type emptyStub struct{} + +// TestRegisterAllIaCProviderServices_PluginServiceBridgeRegistered asserts +// that after calling RegisterAllIaCProviderServices, the server also exposes +// "workflow.plugin.v1.PluginService" so the wfctl host can call +// GetContractRegistry without getting "unknown service". This is the fix for +// the DO plugin v1.0.0 incompatibility where ServeIaCPlugin (which calls +// RegisterAllIaCProviderServices) didn't register PluginService, causing +// wfctl's NewExternalPluginAdapter to fail. +func TestRegisterAllIaCProviderServices_PluginServiceBridgeRegistered(t *testing.T) { + grpcSrv := grpc.NewServer() + if err := sdk.RegisterAllIaCProviderServices(grpcSrv, &fullProviderStub{}); err != nil { + t.Fatalf("unexpected error: %v", err) + } + if _, ok := grpcSrv.GetServiceInfo()["workflow.plugin.v1.PluginService"]; !ok { + t.Fatalf("PluginService bridge not registered; have: %v", serviceNames(grpcSrv.GetServiceInfo())) + } +} + +// TestRegisterAllIaCProviderServices_PluginServiceBridgeAnswersGetContractRegistry +// verifies the bridge returns a ContractRegistry containing the registered +// IaC services when GetContractRegistry is called via a live gRPC client. +// This exercises the end-to-end path that wfctl's NewExternalPluginAdapter +// takes when loading a DO v1.0.0-style plugin via discoverAndLoadIaCProvider. +func TestRegisterAllIaCProviderServices_PluginServiceBridgeAnswersGetContractRegistry(t *testing.T) { + t.Parallel() + + // Spin up an in-process gRPC server with the IaC services + bridge. + lis, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + t.Fatalf("net.Listen: %v", err) + } + grpcSrv := grpc.NewServer() + if err := sdk.RegisterAllIaCProviderServices(grpcSrv, &allCapabilitiesStub{}); err != nil { + t.Fatalf("register: %v", err) + } + go func() { _ = grpcSrv.Serve(lis) }() + t.Cleanup(func() { grpcSrv.Stop() }) + + conn, err := grpc.NewClient(lis.Addr().String(), grpc.WithTransportCredentials(insecure.NewCredentials())) + if err != nil { + t.Fatalf("grpc.NewClient: %v", err) + } + t.Cleanup(func() { _ = conn.Close() }) + + // Call GetContractRegistry via the PluginServiceClient — exactly what + // wfctl's NewExternalPluginAdapter does via pb.NewPluginServiceClient. + client := pb.NewPluginServiceClient(conn) + registry, err := client.GetContractRegistry(context.Background(), &emptypb.Empty{}) + if err != nil { + t.Fatalf("GetContractRegistry: %v — PluginService bridge did not answer (DO v1.0.0 incompatibility fix is broken)", err) + } + + services := map[string]bool{} + for _, c := range registry.GetContracts() { + if c.GetKind() == pb.ContractKind_CONTRACT_KIND_SERVICE { + services[c.GetServiceName()] = true + } + } + + // The IaCProviderRequired service MUST appear — this is what wfctl's + // buildTypedIaCAdapterFrom checks via registeredIaCServices(). + if !services["workflow.plugin.external.iac.IaCProviderRequired"] { + t.Errorf("GetContractRegistry did not include IaCProviderRequired; got services: %v", services) + } +} + +// TestRegisterAllIaCProviderServices_PluginServiceAlreadyRegistered_NoPanic +// asserts that calling RegisterAllIaCProviderServices on a server that already +// has PluginService registered (e.g. a mixed plugin using both sdk.Serve and +// RegisterAllIaCProviderServices) does NOT panic from double-registration. +func TestRegisterAllIaCProviderServices_PluginServiceAlreadyRegistered_NoPanic(t *testing.T) { + grpcSrv := grpc.NewServer() + // Pre-register PluginService (simulates a mixed sdk.Serve + IaC plugin). + pb.RegisterPluginServiceServer(grpcSrv, &pb.UnimplementedPluginServiceServer{}) + // RegisterAllIaCProviderServices must not panic on double-registration. + if err := sdk.RegisterAllIaCProviderServices(grpcSrv, &fullProviderStub{}); err != nil { + t.Fatalf("unexpected error: %v", err) + } +} From 7204a163f283747ac0ad367c83b9e1c19677a98b Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 10 May 2026 22:00:36 +0000 Subject: [PATCH 3/3] fix(plugin/external/sdk): use ServiceDesc.ServiceName constant and embedded-by-value stub - Replace hard-coded "workflow.plugin.v1.PluginService" string in double-registration guard with pb.PluginService_ServiceDesc.ServiceName so drift/typos if the proto package name changes are caught at compile time. - In the NoPanic test, replace &pb.UnimplementedPluginServiceServer{} with a local stub type that embeds UnimplementedPluginServiceServer by value, matching the pattern the generated gRPC code recommends. Agent-Logs-Url: https://github.com/GoCodeAlone/workflow/sessions/460fbd7b-0c6c-46f0-b681-e364ed8a8889 Co-authored-by: intel352 <77607+intel352@users.noreply.github.com> --- plugin/external/sdk/iacserver.go | 2 +- plugin/external/sdk/iacserver_test.go | 5 ++++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/plugin/external/sdk/iacserver.go b/plugin/external/sdk/iacserver.go index af972482..0c4c848d 100644 --- a/plugin/external/sdk/iacserver.go +++ b/plugin/external/sdk/iacserver.go @@ -109,7 +109,7 @@ func RegisterAllIaCProviderServices(s *grpc.Server, provider any) error { // Guard: skip registration if PluginService is already on the server // (e.g. a mixed plugin that called sdk.Serve AND RegisterAllIaC). // gRPC panics on double-registration; the guard prevents that. - if _, alreadyRegistered := s.GetServiceInfo()["workflow.plugin.v1.PluginService"]; !alreadyRegistered { + if _, alreadyRegistered := s.GetServiceInfo()[pb.PluginService_ServiceDesc.ServiceName]; !alreadyRegistered { pb.RegisterPluginServiceServer(s, &iacPluginServiceBridge{grpcSrv: s}) } return nil diff --git a/plugin/external/sdk/iacserver_test.go b/plugin/external/sdk/iacserver_test.go index 83208949..d14a9ebd 100644 --- a/plugin/external/sdk/iacserver_test.go +++ b/plugin/external/sdk/iacserver_test.go @@ -224,7 +224,10 @@ func TestRegisterAllIaCProviderServices_PluginServiceBridgeAnswersGetContractReg func TestRegisterAllIaCProviderServices_PluginServiceAlreadyRegistered_NoPanic(t *testing.T) { grpcSrv := grpc.NewServer() // Pre-register PluginService (simulates a mixed sdk.Serve + IaC plugin). - pb.RegisterPluginServiceServer(grpcSrv, &pb.UnimplementedPluginServiceServer{}) + // Use an embedded-by-value stub so the pattern is idiomatic Go and not + // a pointer-to-unimplemented (which the generated gRPC code warns against). + type minimalPluginSvc struct{ pb.UnimplementedPluginServiceServer } + pb.RegisterPluginServiceServer(grpcSrv, &minimalPluginSvc{}) // RegisterAllIaCProviderServices must not panic on double-registration. if err := sdk.RegisterAllIaCProviderServices(grpcSrv, &fullProviderStub{}); err != nil { t.Fatalf("unexpected error: %v", err)