From 542d54ba90d913cc16babd4c9a0a18cb53278143 Mon Sep 17 00:00:00 2001 From: Jon Langevin Date: Tue, 12 May 2026 11:06:53 -0400 Subject: [PATCH 1/2] fix(plugin/external): handle empty ConfigMessage for input-only STRICT_PROTO step contracts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Steps that declare STRICT_PROTO mode + InputMessage + OutputMessage but no ConfigMessage (e.g., step.eventbus.ack, step.eventbus.publish) failed engine initialization with: STRICT_PROTO contract for config message "" cannot use legacy Struct fallback: missing protobuf message name The step has no per-instance config schema — data flows through the input message. Engine now treats empty ConfigMessage as "no typed config", encodes cfg as legacy *structpb.Struct, returns nil typed payload. Plugin's typed factory reads from InputMessage as designed. Caught by BMW PR #278 image-launch smoke against v0.51.3 + eventbus v0.3.0 (steps.eventbus.{ack,publish,consume} have empty ConfigMessage). Test: TestCreateTypedConfigRequestEmptyConfigMessageStrictProto. --- plugin/external/adapter.go | 12 ++++++++++++ plugin/external/adapter_test.go | 27 +++++++++++++++++++++++++++ 2 files changed, 39 insertions(+) diff --git a/plugin/external/adapter.go b/plugin/external/adapter.go index 3c304cc9..28efac30 100644 --- a/plugin/external/adapter.go +++ b/plugin/external/adapter.go @@ -303,6 +303,18 @@ func createTypedConfigRequest(descriptor *pb.ContractDescriptor, cfg map[string] } return s, nil, nil } + // Steps with STRICT_PROTO mode but no ConfigMessage are input-only + // (eventbus.ack, eventbus.publish, etc.) — they declare InputMessage + + // OutputMessage but no per-instance config schema. Encode cfg as legacy + // struct only; typed payload is nil. The plugin's typed factory reads + // data from the input message, not from the config. + if descriptor.ConfigMessage == "" { + s, err := mapToStruct(cfg) + if err != nil { + return nil, nil, fmt.Errorf("encode config as Struct (input-only typed contract): %w", err) + } + return s, nil, nil + } // Strip engine-internal "_"-prefix keys before proto decode. STRICT_PROTO // and PROTO_WITH_LEGACY_STRUCT modules use protojson with DiscardUnknown // = false (convert.go:62), which rejects engine internals like diff --git a/plugin/external/adapter_test.go b/plugin/external/adapter_test.go index 8f1f54e4..fda138a8 100644 --- a/plugin/external/adapter_test.go +++ b/plugin/external/adapter_test.go @@ -1022,6 +1022,33 @@ func TestCreateTypedConfigRequestStripsInternalKeysForStrictProtoStep(t *testing } } +// TestCreateTypedConfigRequestEmptyConfigMessageStrictProto covers step +// contracts that declare STRICT_PROTO with InputMessage + OutputMessage but +// no ConfigMessage (input-only steps like step.eventbus.ack / +// step.eventbus.publish). The engine must encode cfg as a legacy +// *structpb.Struct, NOT attempt to encode an unnamed typed proto. +func TestCreateTypedConfigRequestEmptyConfigMessageStrictProto(t *testing.T) { + descriptor := &pb.ContractDescriptor{ + Kind: pb.ContractKind_CONTRACT_KIND_STEP, + StepType: "step.eventbus.ack", + Mode: pb.ContractMode_CONTRACT_MODE_STRICT_PROTO, + InputMessage: "workflow.plugin.eventbus.v1.AckRequest", + OutputMessage: "workflow.plugin.eventbus.v1.AckResponse", + // ConfigMessage intentionally empty — step has no per-instance + // config schema; data flows via the input message. + } + legacy, typed, err := createTypedConfigRequest(descriptor, nil, nil) + if err != nil { + t.Fatalf("createTypedConfigRequest with empty ConfigMessage: %v", err) + } + if typed != nil { + t.Fatalf("expected nil typed *anypb.Any for input-only step contract; got %v", typed) + } + if legacy != nil && len(legacy.Fields) != 0 { + t.Fatalf("expected empty legacy struct for nil cfg; got %v", legacy.Fields) + } +} + // TestCreateTypedConfigRequestRetainsInternalKeysInLegacyStruct asserts the // legacy-struct path keeps "_"-prefix keys on its *structpb.Struct payload. // Legacy modules consume "_config_dir" at the plugin side to resolve filesystem- From d2b75290bd4fe6e9e25497a0f9f7c58712835275 Mon Sep 17 00:00:00 2001 From: Jon Langevin Date: Tue, 12 May 2026 11:30:31 -0400 Subject: [PATCH 2/2] =?UTF-8?q?fix:=20address=20Copilot=20review=20?= =?UTF-8?q?=E2=80=94=20comment=20scope=20+=20test=20asserts=20both=20nil?= =?UTF-8?q?=20+=20non-nil=20cfg=20paths?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- plugin/external/adapter.go | 15 +++++++++------ plugin/external/adapter_test.go | 27 +++++++++++++++++++++------ 2 files changed, 30 insertions(+), 12 deletions(-) diff --git a/plugin/external/adapter.go b/plugin/external/adapter.go index 28efac30..82a0a4e0 100644 --- a/plugin/external/adapter.go +++ b/plugin/external/adapter.go @@ -303,15 +303,18 @@ func createTypedConfigRequest(descriptor *pb.ContractDescriptor, cfg map[string] } return s, nil, nil } - // Steps with STRICT_PROTO mode but no ConfigMessage are input-only - // (eventbus.ack, eventbus.publish, etc.) — they declare InputMessage + - // OutputMessage but no per-instance config schema. Encode cfg as legacy - // struct only; typed payload is nil. The plugin's typed factory reads - // data from the input message, not from the config. + // Contracts that declare a typed Mode (STRICT_PROTO or + // PROTO_WITH_LEGACY_STRUCT) but leave ConfigMessage empty have no + // per-instance config schema — primarily input-only steps like + // step.eventbus.ack/publish/consume where data flows through the + // InputMessage proto, but also applies to any contract Kind that + // legitimately omits a config schema. Encode cfg as legacy struct + // only; typed payload is nil. The plugin's typed factory reads data + // from the input message (or other typed payload), not from config. if descriptor.ConfigMessage == "" { s, err := mapToStruct(cfg) if err != nil { - return nil, nil, fmt.Errorf("encode config as Struct (input-only typed contract): %w", err) + return nil, nil, fmt.Errorf("encode config as Struct (no typed config schema): %w", err) } return s, nil, nil } diff --git a/plugin/external/adapter_test.go b/plugin/external/adapter_test.go index fda138a8..2d723f81 100644 --- a/plugin/external/adapter_test.go +++ b/plugin/external/adapter_test.go @@ -1022,11 +1022,13 @@ func TestCreateTypedConfigRequestStripsInternalKeysForStrictProtoStep(t *testing } } -// TestCreateTypedConfigRequestEmptyConfigMessageStrictProto covers step +// TestCreateTypedConfigRequestEmptyConfigMessageStrictProto covers // contracts that declare STRICT_PROTO with InputMessage + OutputMessage but // no ConfigMessage (input-only steps like step.eventbus.ack / -// step.eventbus.publish). The engine must encode cfg as a legacy -// *structpb.Struct, NOT attempt to encode an unnamed typed proto. +// step.eventbus.publish). The engine must NOT attempt to encode an +// unnamed typed proto; typed payload is nil, legacy struct mirrors cfg +// (nil cfg → nil legacy via mapToStruct(nil); non-nil cfg → populated +// struct). func TestCreateTypedConfigRequestEmptyConfigMessageStrictProto(t *testing.T) { descriptor := &pb.ContractDescriptor{ Kind: pb.ContractKind_CONTRACT_KIND_STEP, @@ -1037,15 +1039,28 @@ func TestCreateTypedConfigRequestEmptyConfigMessageStrictProto(t *testing.T) { // ConfigMessage intentionally empty — step has no per-instance // config schema; data flows via the input message. } + // nil cfg — mapToStruct(nil) returns nil; legacy is permitted to be nil. legacy, typed, err := createTypedConfigRequest(descriptor, nil, nil) if err != nil { - t.Fatalf("createTypedConfigRequest with empty ConfigMessage: %v", err) + t.Fatalf("createTypedConfigRequest with nil cfg + empty ConfigMessage: %v", err) } if typed != nil { t.Fatalf("expected nil typed *anypb.Any for input-only step contract; got %v", typed) } - if legacy != nil && len(legacy.Fields) != 0 { - t.Fatalf("expected empty legacy struct for nil cfg; got %v", legacy.Fields) + if legacy != nil { + t.Fatalf("expected nil legacy struct for nil cfg; got %v", legacy.Fields) + } + // Non-nil cfg — fields populated into legacy struct; typed still nil. + cfg := map[string]any{"timeout_ms": float64(5000)} + legacy2, typed2, err := createTypedConfigRequest(descriptor, cfg, nil) + if err != nil { + t.Fatalf("createTypedConfigRequest with cfg + empty ConfigMessage: %v", err) + } + if typed2 != nil { + t.Fatalf("expected nil typed *anypb.Any for input-only step contract; got %v", typed2) + } + if legacy2 == nil || legacy2.Fields["timeout_ms"] == nil { + t.Fatalf("expected legacy struct with timeout_ms populated; got %v", legacy2) } }