From 2432a33d0c2c9aa85a7f59bbc191c90efb683b70 Mon Sep 17 00:00:00 2001 From: lin2qwer1-cloud <296501427+lin2qwer1-cloud@users.noreply.github.com> Date: Thu, 16 Jul 2026 16:31:00 +0900 Subject: [PATCH 1/2] =?UTF-8?q?fix(mcp):=20route=20read=20for=20SRVB=20?= =?UTF-8?q?=E2=80=94=20advertised=20but=20dropped=20by=20the=20switch?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit GetSource's own tool description lists "SRVB (service binding)" as a supported object_type, and pkg/adt already implements it end to end (workflows_source.go `case "SRVB"` -> Client.GetSRVB -> JSON). But the MCP router's allow-list in routeSourceAction omits "SRVB", so every `read SRVB ` is rejected with: No handler found for action="read" target="SRVB" This makes a created service binding unverifiable through the MCP surface: callers can create an SRVB but cannot read back which OData version/category they actually got. Add "SRVB" to the read allow-list. No new capability — this only lets the existing, working implementation be reached, matching the tool's documented contract. Verified against a live S/4HANA 2021 (7.56) system: before: No handler found for action="read" target="SRVB" after : {"name":"YUI_FLIGHT_O4","bindingType":"ODATA", "bindingVersion":"V4","published":false, "serviceDefName":"YUI_FLIGHT_O4"} --- internal/mcp/handlers_source.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/mcp/handlers_source.go b/internal/mcp/handlers_source.go index 40b5043c..ab58efc1 100644 --- a/internal/mcp/handlers_source.go +++ b/internal/mcp/handlers_source.go @@ -16,9 +16,9 @@ import ( // routeSourceAction routes "read" for GetSource and "edit" for WriteSource/EditSource. func (s *Server) routeSourceAction(ctx context.Context, action, objectType, objectName string, params map[string]any) (*mcp.CallToolResult, bool, error) { if action == "read" { - // GetSource covers: CLAS, PROG, INTF, FUNC, FUGR, INCL, DDLS, BDEF, SRVD, MSAG, VIEW + // GetSource covers: CLAS, PROG, INTF, FUNC, FUGR, INCL, DDLS, BDEF, SRVD, SRVB, MSAG, VIEW switch objectType { - case "CLAS", "PROG", "INTF", "FUNC", "FUGR", "INCL", "DDLS", "BDEF", "SRVD", "MSAG", "VIEW": + case "CLAS", "PROG", "INTF", "FUNC", "FUGR", "INCL", "DDLS", "BDEF", "SRVD", "SRVB", "MSAG", "VIEW": args := map[string]any{ "object_type": objectType, "name": objectName, From 44dd292b5b06f8e6467c4dd274094e2634053300 Mon Sep 17 00:00:00 2001 From: lin2qwer1-cloud <296501427+lin2qwer1-cloud@users.noreply.github.com> Date: Thu, 16 Jul 2026 16:31:37 +0900 Subject: [PATCH 2/2] fix(srvb): correct inverted binding_category docs (0=UI, 1=A2X) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The binding_category values were documented backwards. Per SAP's own domain SRVB_BND_CATEGORY (verified on S/4HANA 2021 via DD07L/DD07T): 0 = UI ( User Interface ) 1 = A2X ( Application to X users ) i.e. Web API but CreateObjectOptions, buildCreateObjectBody and the CreateObject tool schema all said the opposite ("0" for Web API, "1" for UI). A caller who wants a UI binding and follows the documentation passes "1" and silently gets an A2X/Web API binding instead. The emitted default ("0") was already correct in practice — it produces a UI binding — only its label was wrong, so this is a docs/semantics fix with no behavior change. Also clarify binding_version: it silently defaults to "V2", which yields a binding that cannot drive a Fiori Elements V4 app. Callers who need V4 must pass it explicitly; the schema now says so. Add regression tests pinning the ADT payload for category (default/UI/A2X), version (default/V2/V4) and the bound service definition. --- internal/mcp/tools_register.go | 4 +- pkg/adt/crud.go | 5 +- pkg/adt/crud_srvb_test.go | 93 ++++++++++++++++++++++++++++++++++ 3 files changed, 98 insertions(+), 4 deletions(-) create mode 100644 pkg/adt/crud_srvb_test.go diff --git a/internal/mcp/tools_register.go b/internal/mcp/tools_register.go index e79542ed..64b0c079 100644 --- a/internal/mcp/tools_register.go +++ b/internal/mcp/tools_register.go @@ -964,10 +964,10 @@ func (s *Server) registerCRUDTools(shouldRegister func(string) bool) { mcp.Description("For SRVB: the service definition name to bind"), ), mcp.WithString("binding_version", - mcp.Description("For SRVB: OData version 'V2' or 'V4' (default: V2)"), + mcp.Description("For SRVB: OData version 'V2' or 'V4'. Defaults to 'V2' - pass 'V4' explicitly for Fiori Elements V4 apps, otherwise a V2 binding is created silently."), ), mcp.WithString("binding_category", - mcp.Description("For SRVB: '0' for Web API, '1' for UI (default: 0)"), + mcp.Description("For SRVB: '0' = UI (User Interface), '1' = A2X (Web API). Default: '0' (UI). Values follow SAP domain SRVB_BND_CATEGORY."), ), ), s.handleCreateObject) } diff --git a/pkg/adt/crud.go b/pkg/adt/crud.go index 78ed82be..5e819861 100755 --- a/pkg/adt/crud.go +++ b/pkg/adt/crud.go @@ -217,7 +217,8 @@ type CreateObjectOptions struct { BindingType string `json:"bindingType,omitempty"` // For SRVB: binding version ("V2" or "V4") BindingVersion string `json:"bindingVersion,omitempty"` - // For SRVB: category ("0" for Web API, "1" for UI) + // For SRVB: category per SAP domain SRVB_BND_CATEGORY: + // "0" = UI (User Interface), "1" = A2X (Application to X users, i.e. Web API) BindingCategory string `json:"bindingCategory,omitempty"` // For BDEF: source code (required for creation - ADT API embeds source in creation request) @@ -765,7 +766,7 @@ func buildCreateObjectBody(opts CreateObjectOptions, typeInfo objectTypeInfo, de } bindingCategory := opts.BindingCategory if bindingCategory == "" { - bindingCategory = "0" // Web API + bindingCategory = "0" // UI (SRVB_BND_CATEGORY: 0=UI, 1=A2X/Web API) } return fmt.Sprintf(` <%s %s xmlns:adtcore="http://www.sap.com/adt/core" diff --git a/pkg/adt/crud_srvb_test.go b/pkg/adt/crud_srvb_test.go new file mode 100644 index 00000000..1f17a118 --- /dev/null +++ b/pkg/adt/crud_srvb_test.go @@ -0,0 +1,93 @@ +package adt + +import ( + "strings" + "testing" +) + +// srvbTypeInfo mirrors what CreateObject resolves for ObjectTypeSRVB. +func srvbTestTypeInfo(t *testing.T) objectTypeInfo { + t.Helper() + ti, ok := objectTypes[ObjectTypeSRVB] + if !ok { + t.Fatalf("ObjectTypeSRVB not present in objectTypes registry") + } + return ti +} + +func buildSRVB(t *testing.T, opts CreateObjectOptions) string { + t.Helper() + opts.ObjectType = ObjectTypeSRVB + if opts.Name == "" { + opts.Name = "ZUI_TEST_O4" + } + if opts.PackageName == "" { + opts.PackageName = "$TMP" + } + if opts.ServiceDefinition == "" { + opts.ServiceDefinition = "ZUI_TEST" + } + return buildCreateObjectBody(opts, srvbTestTypeInfo(t), "DEVELOPER") +} + +// The binding category values come from SAP domain SRVB_BND_CATEGORY: +// +// 0 = UI (User Interface) +// 1 = A2X (Application to X users) i.e. Web API +// +// The default must be UI ("0"), and an explicitly requested category must survive. +func TestBuildCreateObjectBody_SRVBCategory(t *testing.T) { + tests := []struct { + name string + category string + wantCategory string + }{ + {name: "default is UI", category: "", wantCategory: `srvb:category="0"`}, + {name: "explicit UI", category: "0", wantCategory: `srvb:category="0"`}, + {name: "explicit A2X/Web API", category: "1", wantCategory: `srvb:category="1"`}, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + body := buildSRVB(t, CreateObjectOptions{BindingCategory: tc.category}) + if !strings.Contains(body, tc.wantCategory) { + t.Errorf("want %s in body, got:\n%s", tc.wantCategory, body) + } + }) + } +} + +// Regression: passing binding_version must actually reach the ADT payload. +// A silently-defaulted V2 binding for a caller that asked for V4 produces a +// service that cannot drive a Fiori Elements V4 app. +func TestBuildCreateObjectBody_SRVBVersion(t *testing.T) { + tests := []struct { + name string + version string + wantVersion string + }{ + {name: "default is V2", version: "", wantVersion: `srvb:version="V2"`}, + {name: "explicit V2", version: "V2", wantVersion: `srvb:version="V2"`}, + {name: "explicit V4", version: "V4", wantVersion: `srvb:version="V4"`}, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + body := buildSRVB(t, CreateObjectOptions{BindingVersion: tc.version}) + if !strings.Contains(body, tc.wantVersion) { + t.Errorf("want %s in body, got:\n%s", tc.wantVersion, body) + } + }) + } +} + +// The bound service definition must be upper-cased and present. +func TestBuildCreateObjectBody_SRVBServiceDefinition(t *testing.T) { + body := buildSRVB(t, CreateObjectOptions{ServiceDefinition: "zui_flight_o4"}) + if !strings.Contains(body, ``) { + t.Errorf("service definition not bound/upper-cased, got:\n%s", body) + } + if !strings.Contains(body, `srvb:type="ODATA"`) { + t.Errorf(`want srvb:type="ODATA", got:\n%s`, body) + } +}