fix(srvb): unblock read for SRVB + correct inverted binding_category docs#149
Open
lin2qwer1-cloud wants to merge 2 commits into
Open
fix(srvb): unblock read for SRVB + correct inverted binding_category docs#149lin2qwer1-cloud wants to merge 2 commits into
lin2qwer1-cloud wants to merge 2 commits into
Conversation
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 <name>` 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"}
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.
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.
Two independent SRVB defects found while building a RAP OData V4 service through the MCP surface against a live S/4HANA 2021 (SAP_BASIS 7.56) system. Both are small; the first one is what let the second one bite.
1.
read SRVBis advertised but unreachableGetSource's own tool description already listsSRVB (service binding)as a supportedobject_type, andpkg/adtimplements it end to end (workflows_source.gocase "SRVB"→Client.GetSRVB→ JSON). But the MCP router's allow-list inrouteSourceActionomits"SRVB", so the call is rejected:Consequence: a caller can create a service binding but cannot read back what it actually got. This is what made defect 2 silent.
Fix: add
"SRVB"to the read allow-list — no new capability, it just reaches the existing implementation and matches the documented contract.Verified live:
2.
binding_categoryis documented backwardsPer SAP's domain
SRVB_BND_CATEGORY(verified on 7.56 viaDD07L/DD07T):01CreateObjectOptions,buildCreateObjectBodyand theCreateObjecttool schema all state 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.The emitted default (
"0") was already correct in practice — it produces a UI binding — only the label was wrong. Docs/semantics only, no behavior change.Also clarified
binding_version: it silently defaults toV2, producing a binding that cannot drive a Fiori Elements V4 app. Callers needing V4 must pass it explicitly; the schema now says so. (This is exactly how a binding named..._O4ended up as V2 for us, undetected because of defect 1.)Tests
Added
pkg/adt/crud_srvb_test.gopinning the ADT payload:srvb:category="0", explicit UI0, explicit A2X1srvb:version="V2", explicitV2/V4srvb:type="ODATA"go vetclean.internal/mcpandpkg/adtsuites pass.Note: the
Recording*tests inpkg/adtappear flaky on this machine independently of this change — they pass in isolation on both this branch and cleanmain, and across repeated full-suite runs differentRecording*tests fail on unchanged code (2 of 3 runs fully green). Unrelated to this PR, flagging in case it is useful.