Skip to content

fix(langgraph): stop concatenating getServerName()+getName() into connectorType#198

Merged
saurabhjain1592 merged 5 commits into
mainfrom
fix/2909-langgraph-server-tool-split
Jul 17, 2026
Merged

fix(langgraph): stop concatenating getServerName()+getName() into connectorType#198
saurabhjain1592 merged 5 commits into
mainfrom
fix/2909-langgraph-server-tool-split

Conversation

@gzak

@gzak gzak commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

Summary

LangGraphAdapter.mcpToolInterceptor() derived a single connectorType as "{serverName}.{toolName}" because the MCP check-input/check-output wire contract only had one identity field to put it in. The platform has since finalized a genuine two-field (server, tool) identity contract for that request (epic #2905, sub-issue #2904 — R3-approved, CI-green, schema finalized though not yet merged to axonflow-enterprise): a new optional tool field alongside the existing connector_type.

This PR threads server and tool identity through as two separate fields instead of collapsing them into one opaque string, in both branches of mcpToolInterceptor(options) (default and caller-supplied MCPInterceptorOptions).

Changes

  • MCPCheckInputRequest / MCPCheckOutputRequest: add an optional tool field (@JsonProperty("tool")), with new all-args constructors; existing constructors are preserved and default tool to null for backward compatibility.
  • AxonFlow.mcpCheckInput / mcpCheckOutput: read a new "tool" key out of the existing options map and forward it on the wire alongside connectorType.
  • MCPInterceptorOptions: adds a toolFn (Function<MCPToolRequest, String>) alongside the existing connectorTypeFn, so callers can customize tool-name derivation independently of connector-type derivation.
  • MCPToolInterceptor: now takes both a connectorTypeFn and a toolFn; sends tool in the options map passed to mcpCheckInput/mcpCheckOutput for both the pre-check and post-check calls.
  • LangGraphAdapter.mcpToolInterceptor(options): default connectorTypeFn is now MCPToolRequest::getServerName (was serverName + "." + name), default toolFn is MCPToolRequest::getName. Applied in both the options != null and options == null branches — same root cause, same fix.
  • tests/fixtures/wire-shape-baseline.json: regenerated via scripts/wire_shape/refresh.py against the pinned OpenAPI spec commit. The new tool field shows up as expected sdk_only drift on MCPCheckInputRequest/MCPCheckOutputRequest (same pattern as the pre-existing content_type field), since the platform's spec pin predates #2904 merging into axonflow-enterprise. Once the platform spec ships tool and the pin is bumped, that baseline entry should be removed as part of the burndown.
  • CHANGELOG.md: [Unreleased] entry.
  • Tests: updated LangGraphAdapterTest (MCPToolInterceptorTests) to assert connectorType and tool are sent as separate values (not concatenated), plus a new case for a custom toolFn; added tool-field coverage to MoreTypesTest (MCPCheckInputRequest/MCPCheckOutputRequest) and AxonFlowTest (wire-level assertions via WireMock that connector_type and tool are sent as distinct JSON fields).

Not in scope

  • GovernedTool/GovernedTool.Builder.connectorTypeFn uses a single-tool-name Function<String, String> (no MCP server concept) — unrelated pattern, untouched.
  • axonflow-docs/main-tree/docs/orchestration/wcp/langgraph-wrapper.md mentions mcp_tool_interceptor() (~line 454), but that's describing the Python SDK's snake_case equivalent, not this Java concatenation bug — spot-checked, no Java-specific staleness found. If the Python SDK has an analogous single-field concatenation issue, that would need its own follow-up in axonflow-docs/axonflow-sdk-python, tracked separately.

Refs #2909, epic #2905.

Test plan

  • ./mvnw test — 1321 tests, 0 failures/errors
  • ./mvnw verify — BUILD SUCCESS (jacoco coverage gate passes)
  • scripts/wire_shape/validate.py run locally against the pinned OpenAPI spec commit — passes with the regenerated baseline
  • New/updated unit tests assert connectorType and tool are sent as two distinct values rather than a concatenated string, in both the default and custom-MCPInterceptorOptions interceptor paths

…nectorType

LangGraphAdapter.mcpToolInterceptor() derived a single connectorType as
"{serverName}.{toolName}" because the MCP check-input/check-output wire
contract only had one identity field. The platform now accepts a
separate `tool` field alongside `connector_type` (epic #2905, #2904),
so thread server and tool identity through as two distinct fields
instead of collapsing them into one opaque string.

- MCPCheckInputRequest / MCPCheckOutputRequest gain an optional `tool`
  field (sdk_only pending platform-spec sync, tracked in the wire-shape
  baseline like `content_type` before it).
- AxonFlow.mcpCheckInput/mcpCheckOutput read "tool" out of the options
  map and forward it on the wire.
- MCPInterceptorOptions gains a toolFn alongside connectorTypeFn.
- LangGraphAdapter.mcpToolInterceptor() now defaults connectorType to
  MCPToolRequest::getServerName and tool to MCPToolRequest::getName,
  in both the custom-options and default branches.
- Add runtime-e2e/mcp_server_tool_split/, run against a live agent:
  proves the interceptor's split server+tool identity round-trips
  through check-input/check-output, a direct mcpCheckInput call with an
  explicit "tool" option is accepted, and the pre-#2909 two-argument
  mcpCheckInput(connectorType, statement) overload still works
  unchanged.

Refs #2909, epic #2905.

Signed-off-by: Gregory Zak <gzak@getaxonflow.com>
@gzak
gzak force-pushed the fix/2909-langgraph-server-tool-split branch from 42d4af6 to 95cfad6 Compare July 16, 2026 22:38
@saurabhjain1592

Copy link
Copy Markdown
Member

R3 review (master + independent hostile pass). Verdict: MERGEABLE with conditions. CI green; additive constructors preserve the public API; Jackson NON_NULL omission + two-field serialization pinned in MoreTypesTest; WireMock pins the wire body on both endpoints; adapter tests cover default/custom connectorTypeFn/custom toolFn.

MEDIUM — toolFn is a Java-only surface (cluster drift)

No other SDK lets callers override the tool identity, and it allows reintroducing concatenation or arbitrary identities into the audit trail. Either port it to py/ts/go under the epic or drop it here for coherence — at minimum record the divergence decision on epic #2905.

MEDIUM — changelog gaps

Add the policy re-scope migration note (Go #189 has the model text: policies matching the old concatenated value need re-scoping; connectorTypeFn is the compat path) + pin the minimum platform version at release time (enterprise #2916 merged but unreleased; platforms ≤v9.9.0 silently drop tool).

LOW

  • MCPCheckOutputRequest.tool javadoc says the field is "mirrored on check-output", implying server-side consumption — the platform has no such field on ANY version (verified on enterprise main). Add the py/ts-style "not yet consumed server-side" caveat; platform gap tracked via epic #2905.
  • runtime-e2e asserts acceptance only (no wire capture) — weakest live proof of the four; acceptable given the WireMock body assertions, but worth an upgrade when convenient.

@saurabhjain1592

Copy link
Copy Markdown
Member

Delta note — semver classification (net-new, decision-forcing). This de-concatenation changes the wire value of connector_type from server.tool to bare server. For any default-config user, existing customer policies that match the concatenated value (e.g. "filesystem.read_file") silently stop matching after upgrade — silent policy non-enforcement in a governance SDK. Nothing is removed from the API surface, but that is a breaking behavioral change for consumers.

Honest call: this warrants a MAJOR SDK bump (SDK semver is decoupled from the platform, so the platform stays on its own track). The connectorTypeFn escape hatch is the migration path, not a reason to call it minor. If the team decides the derived connector_type default is non-contractual and keeps this minor, that ruling must be recorded on epic #2905 and this changelog needs a prominent Breaking/Migration section (currently it's under ### Fixed, which materially understates it) plus a min-platform pin: check-input tool shipped in released v9.10.0; check-output tool is still unconsumed on every version while #2955 is open.

(All prior-round findings on this PR are still open at the current head — no post-review commits landed.)

Apply epic #2905 RULING 3 and mark the de-concatenation as a BREAKING change:

- RULING 3: DROP the Java-only MCPInterceptorOptions.toolFn. The tool identity
  is always MCPToolRequest#getName(); connectorTypeFn remains the escape hatch
  for the server/connector dimension only. A separate tool-identity override
  let a caller write an arbitrary identity into the audit trail (integrity
  foot-gun) and had no analog in the py/ts/go adapters. Removed the field,
  getter, builder method, and constructor param; the shouldUseCustomToolFn
  test is replaced with one asserting the tool name is always getName().
- Fix the MCPCheckOutputRequest.tool javadoc overclaim ("mirrored on
  check-output" implied server-side consumption): check-output has NO tool
  field on any released version (tracked by #2955). Also pin check-input Tool
  consumption to v9.10.0+.
- Add the missing-server edge test: empty serverName -> connector_type=""
  -> platform 400 -> fail-closed; wire body pinned.
- Changelog: move the entry to a new "Changed (BREAKING)" section, require a
  major SDK bump, add the policy re-scope note, the missing-server edge, and
  the platform-version pins; drop the stale "gains a toolFn" line.

Refs #2909, epic #2905.

Signed-off-by: Saurabh Jain <saurabh.jain@getaxonflow.com>
The prior commit's wire-shape validate run left a scripts/wire_shape/
__pycache__/*.pyc build artifact staged. Remove it and add __pycache__/ +
*.pyc to .gitignore so it cannot recur.

Signed-off-by: Saurabh Jain <saurabh.jain@getaxonflow.com>
…rver-tool-split

Signed-off-by: Saurabh Jain <saurabh.jain@getaxonflow.com>

# Conflicts:
#	CHANGELOG.md
@saurabhjain1592

Copy link
Copy Markdown
Member

Follow-up commits (WS-DECONCAT): applied epic #2905 RULING 3 — DROPPED the Java-only MCPInterceptorOptions.toolFn (field, getter, builder method, constructor param). Tool identity is always MCPToolRequest.getName(); connectorTypeFn remains the escape hatch for the server dimension only. (A separate tool-identity override let a caller write an arbitrary identity into the audit trail and had no analog in py/ts/go.) Fixed the MCPCheckOutputRequest.tool javadoc overclaim ("mirrored on check-output" → check-output consumes it on no released version, tracked by #2955) and pinned check-input to v9.10.0+. Added the missing-server edge test. Changelog is now Changed (BREAKING) (major bump, re-scope note, min-platform pins); dropped the stale "gains a toolFn" line. Merged origin/main (caller_name) in. CI green, MERGEABLE/CLEAN, master-R3'd (2 rounds). Rulings on #2905.

@saurabhjain1592

Copy link
Copy Markdown
Member

Master independent R3: CLEAR-FOR-SET (code). Two-field split coherent with the other SDKs (connector_type=server, tool=tool name, omitted when empty), statement derives from the resolved connector type, missing-server edge tested, breaking changelog proper. CI green, non-destructive origin/main merge (caller_name + tool coexist). Version-file major bump lands at the operator release-cut (consistent with how the caller_name legs merged at 8.5.1). Held for the operator SDK-major cut.

RULING 3 verified: no residual toolFn (grep shows only the deliberate-absence doc comments); tool identity is always getName(), pinned by toolNameIsAlwaysRequestName. LOW (acknowledged design): @JsonInclude(NON_NULL) omits tool only when null, not empty-string — reachable only at the pathological empty-tool-name edge; matches the accepted per-language mechanism.

Bump pom.xml to 9.0.0 and finalize the tool-identity content under
## [9.0.0] - 2026-07-18 (banner + internal sweep intro dropped). Changelog
scrubbed of internal issue/PR numbers and internal review terminology;
minimum platform stated as versions (check-input tool = platform v9.10.0+,
response-plane tool = platform v9.11.0+).

Signed-off-by: Saurabh Jain <saurabh.jain@getaxonflow.com>
@saurabhjain1592

Copy link
Copy Markdown
Member

Release-ready follow-up: bumped the version to 9.0.0 and finalized the tool-identity content under a real ## [9.0.0] - 2026-07-18 changelog header (dropped the "must be a major bump" banner — the header is the bump). Scrubbed the [9.0.0] changelog of internal issue/PR numbers and internal review terminology; minimum platform is now stated as versions (check-input tool = AxonFlow platform v9.10.0+, response-plane tool = v9.11.0+). Version-alignment gate green. CI green, MERGEABLE/CLEAN. Master-R3'd (2 rounds, incl. an independent finalization pass).

@saurabhjain1592

Copy link
Copy Markdown
Member

Master re-R3 of the v9.0.0 finalize delta: CLEAR-FOR-SET. Version file 8.5.1 → 9.0.0; correct Keep-a-Changelog shape (empty [Unreleased] above ## [9.0.0], BREAKING content under [9.0.0], release-workflow preflight will match). Public [9.0.0] block scrubbed clean — no enterprise issue/PR numbers, no RULING language, min-platform stated as versions (v9.10.0+/v9.11.0+). Finalize commit is additive; de-concat logic (two-field split + rulings) untouched. CI green.

Scrub also removed a leaked partner name (BukuWarung) from the public changelog, plus the enterprise/RULING refs. py#217 companion note: RULING-1 body rewrite confirmed on that PR.

@saurabhjain1592
saurabhjain1592 merged commit b2a3a30 into main Jul 17, 2026
18 checks passed
@saurabhjain1592
saurabhjain1592 deleted the fix/2909-langgraph-server-tool-split branch July 17, 2026 23:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants