Completions follow-ups #1318
Description
Follow-ups from the review of #1316. Each item can go in its own small PR.
-
A
refwithouttypereturns an internal error instead of invalid params.CompletionsFeature.handlelooks upbyRefType.get(ref.get("type")).byRefTypecomes fromMap.copyOf, andget(null)on an immutable map throws an NPE, so a request like{"ref": {"name": "x"}, ...}gets -32603 instead of -32602. -
McpFeature.completionRefTypes()can't serve a new reference type.CompletionsFeaturedecodes every request intoCompleteRequestbefore callingfeature.complete(...).CompleteReferenceonly listsref/promptandref/resourcein@JsonSubTypes, so a feature that declaresref/customis advertised and routed to, but decoding fails with -32602 "Malformed CompleteRequest" beforecomplete()runs. Today a custom feature can only take over one of the two built-in types. Either remove the extension point until there's a use case, or letcomplete()take the raw params, ashandle()does. -
ref/resourcewith a concrete resource URI is rejected.ResourcesFeature.completeonly resolvesref/resourcethrough an exactfindTemplatematch, so a reference to an existing resource such astest://static-textgets -32602 "Unknown resource template". The legacyMcpAsyncServerreturns an empty completion here, so the two server stacks now differ. Confirm if this is intentional
Activity
media-ctrl commentedon Nov 9, 2026
Item 1 reproduces from the source, and the fix belongs at the call site rather than in the map type.
In mcp-core/src/main/java/io/modelcontextprotocol/modern/server/CompletionsFeature.java, handle() does:
and byRefType is built in McpServer.java as Map.copyOf(byRefType) before the CompletionsFeature constructor receives it. A ref map with no "type" therefore probes an immutable map with a null key, which throws instead of returning null, so the null branch on the next line that would have produced the intended "Unsupported ref type" invalid params error is never reached. The -32603 follows from the runtime exception, not from a decision anyone made.
A guard above the lookup keeps the intended error path, and it is testable without starting a server:
Swapping Map.copyOf for a HashMap would turn the NPE into a null return and land on -32602 anyway, but it would also blur the difference between a malformed request and an unknown reference type, so I would keep the immutable map and fix the boundary.
On item 3, the disagreement between the two server stacks is the part I would settle first. Template or exact resolution, empty completion for a ref that matches nothing, and a conformance test that sends the same ref/resource request through both stacks would stop them drifting apart again.