feat!: enforce required MCP spec fields in McpSchema; lenient wire deserialization (#928)
Every wire-serialized record in McpSchema now validates spec-required
fields at construction time. Wire deserialization is intentionally
lenient: a missing required field is replaced with a documented default
and a WARN is logged, instead of failing the parse. A required-first
builder convention is introduced across the schema so it is no longer
possible to obtain a builder that is missing required state.
Required-field guards
---------------------
Every wire record's compact constructor asserts non-null (and non-empty
for String identifiers like name, uri, uriTemplate, version) on its
spec-required components. Passing null now throws
IllegalArgumentException at construction time instead of silently
producing invalid JSON via @JsonInclude(NON_ABSENT). This applies to
the JSON-RPC envelopes, lifecycle types, resource/prompt/tool requests
and results, sampling and elicitation, content records, root, complete,
logging and progress notifications, and the two CompleteReference
implementations. See MIGRATION-2.0.md for the full list.
Lenient wire deserialization
----------------------------
For each of those records (except JSONRPCResponse.JSONRPCError, which
still fails fast) a @JsonCreator static `fromJson` factory substitutes
a documented default for any absent required field — "" for strings,
[] for collections, {} for maps, 0 / 0.0 for numerics, INFO for
LoggingLevel, USER for SamplingMessage.role, ASSISTANT for
CreateMessageResult.role, CANCEL for ElicitResult.action, {values=[]}
for CompleteResult.completion — and logs a WARN naming the field and
the value used. Application code can still observe a malformed message
and react, but the SDK no longer halts the conversation.
Builder convention
------------------
Records that have a builder gain a required-first factory method
`builder(req1, req2, …)`; setters for required fields are removed from
the builder so it cannot be left in an invalid state. Existing no-arg
`builder()` factories and required-field setters are kept where source
compatibility demands it but are marked @Deprecated. New builders are
also added for several records that previously had none (ProgressNotification,
JSONRPCError, CompleteRequest, list/result types, content records, ...).
JSON-RPC envelope ergonomics
----------------------------
Previously every JSON-RPC envelope had to be constructed via the
canonical record constructor and the literal "2.0" string had to be
threaded through every call site, e.g.
new JSONRPCRequest("2.0", "tools/call", id, params)
new JSONRPCResponse("2.0", id, result, null)
new JSONRPCResponse("2.0", id, null, new JSONRPCError(code, message, null))
Now:
new JSONRPCRequest("tools/call", id, params) // jsonrpc defaulted
new JSONRPCNotification("notifications/initialized") // params optional
JSONRPCResponse.result(id, result) // factory
JSONRPCResponse.error(id, new JSONRPCError(code, message)) // 2-arg error
JSONRPCResponse's compact constructor additionally enforces the
JSON-RPC invariant that exactly one of `result` / `error` is set —
previously the SDK could build envelopes that violated the protocol.
CompleteReference changes
-------------------------
- PromptReference.equals/hashCode now key on `name` only (previously
derived from identifier()+type()). Two refs with the same name but
different titles now compare equal — code using PromptReference as a
map/set key should be audited.
- PromptReference's compact constructor pins `type` to "ref/prompt"
and logs a WARN if the caller supplies a different non-null value.
The legacy two-arg `PromptReference(String type, String name)`
constructor remains @Deprecated.
- ResourceReference's record components are reduced from (type, uri)
to (uri) — positional construction breaks. The legacy
`ResourceReference(String type, String uri)` constructor stays
@Deprecated and ignores `type`.
- CompleteReference.identifier() is @Deprecated and now returns null
via a default method on the interface.
Tests / refactor
----------------
Conformance harness, integration tests, sample apps, and internal
callers were migrated to the new builder factories and convenience
constructors. No semantic changes to client/server runtime behaviour
beyond the McpSchema changes above.
Docs
----
- MIGRATION-2.0.md: required-field section now covers the broader
record set and the lenient-deserialization behaviour; PromptReference
WARN behaviour and ResourceReference component reduction are
documented; the JSON-RPC envelope section is rewritten to compare the
pre-2.0 surface with the new one.
- CONTRIBUTING.md: the "Evolving wire-serialized records" recipe is
split into two cases — adding a new optional field (existing rules)
and adding/maintaining a spec-required field (new rules covering
Assert in compact constructor, @JsonCreator fromJson with defaults
and WARN, required-first builder factory).
Signed-off-by: Dariusz Jędrzejczyk <2554306+chemicL@users.noreply.github.com>
This commit is contained in:
parent
599b43cac4
commit
22a0efa577
1 changed files with 4258 additions and 1688 deletions
|
|
@ -77,22 +77,35 @@ git checkout -b feature/your-feature-name
|
|||
|
||||
## Evolving wire-serialized records
|
||||
|
||||
Records in `McpSchema` are serialized directly to the MCP JSON wire format. Follow these rules whenever you add a field to an existing record to keep the protocol forward- and backward-compatible.
|
||||
Records in `McpSchema` are serialized directly to the MCP JSON wire format. The rules differ depending on whether the field you are adding (or maintaining) is *optional* — Java code may legitimately leave it `null` and the wire may omit it — or *spec-required* by MCP. Follow **Case A** for optional fields and **Case B** for spec-required fields.
|
||||
|
||||
### Rules
|
||||
### Case A — Optional fields
|
||||
|
||||
1. **Add new components only at the end** of the record's component list. Never reorder or rename existing components.
|
||||
2. **Annotate every component** with `@JsonProperty("fieldName")` even when the Java name already matches. This survives local renames via refactoring tools.
|
||||
3. **Use boxed types** (`Boolean`, `Integer`, `Long`, `Double`) so the field can be absent on the wire without a special sentinel.
|
||||
4. **Default to `null`**, not an empty collection or neutral value, so the `@JsonInclude(NON_NULL)` rule omits the field for clients that don't know about it yet.
|
||||
4. **Default to `null`**, not an empty collection or neutral value, so the `@JsonInclude(NON_ABSENT)` rule omits the field for clients that don't know about it yet.
|
||||
5. **Keep existing constructors as source-compatible overloads** that delegate to the new canonical constructor and pass `null` for the new component. Do not remove them in the same release that adds the field.
|
||||
6. **Do not put `@JsonCreator` on the canonical constructor** unless strictly necessary. Jackson auto-detects record canonical constructors; adding `@JsonCreator` pins deserialization to that exact parameter order forever.
|
||||
7. **Do not convert `null` to a default value in the canonical constructor.** Null carries "absent" semantics and must be preserved through the serialization round-trip.
|
||||
6. **Do not put `@JsonCreator` on the canonical constructor** unless strictly necessary. Jackson auto-detects record canonical constructors; adding `@JsonCreator` pins deserialization to that exact parameter order forever. *(For records that also have spec-required fields, the `@JsonCreator` belongs on a separate static `fromJson` factory — see Case B, Rule 2.)*
|
||||
7. **Do not convert `null` to a default value in the canonical constructor.** Null carries "absent" semantics and must be preserved through the serialization round-trip. *(Spec-required fields are the exception — see Case B, Rule 1.)*
|
||||
8. **Add three tests per new field** (put them in the relevant test class in `mcp-test`):
|
||||
- Deserialize JSON *without* the field → succeeds, field is `null`.
|
||||
- Serialize an instance with the field unset (`null`) → the key is absent from output.
|
||||
- Deserialize JSON with an extra *unknown* field → succeeds.
|
||||
9. **An inner `Builder` subclass can be used.** This improves the developer experience since frequently not all fields are required.
|
||||
9. **An inner `Builder` subclass can be used.** This improves the developer experience since frequently not all fields are required.
|
||||
|
||||
### Case B — Spec-required fields
|
||||
|
||||
When the MCP specification marks a field as required, callers must not be able to construct a structurally invalid record, but the wire parser must still tolerate peers that fail to send it. Follow these rules in addition to the relevant Case A rules (annotation, naming, append-only).
|
||||
|
||||
1. **Reject `null` in the compact constructor.** Use `Assert.notNull` for required objects or `Assert.hasText` for required `String` identifiers (`name`, `uri`, `uriTemplate`, `version`). This throws `IllegalArgumentException` at construction time instead of producing a record that fails later in serialization or protocol handling. Overrides Case A Rule 7 for this field.
|
||||
2. **Add a `@JsonCreator` static `fromJson` factory** alongside the canonical constructor. When a required field is absent from the wire, substitute a documented safe default (`""` for strings, `[]` for collections, `{}` for maps, `0` / `0.0` for numerics, `INFO` for `LoggingLevel`, etc.) and log at `WARN` naming the field and the value used. The SDK must not halt the conversation because of a missing field. Place `@JsonCreator` on this `fromJson` factory, never on the canonical constructor (Case A Rule 6 still applies to the canonical constructor itself).
|
||||
- Exception: `JSONRPCResponse.JSONRPCError` fails fast on missing `code` / `message` because a malformed JSON-RPC error envelope is unrecoverable.
|
||||
3. **Provide a required-first builder factory** `builder(req1, req2, …)` and remove the corresponding setters from the `Builder`. A no-arg `builder()` factory must not exist on a record that has required fields. If one already exists for source compatibility, mark it `@Deprecated`.
|
||||
4. **Add tests per required field**:
|
||||
- Constructing the record with `null` for the field throws `IllegalArgumentException`.
|
||||
- Deserializing JSON *without* the field succeeds and yields the documented default.
|
||||
- Deserializing JSON with an extra *unknown* field still succeeds (Case A Rule 8 also applies).
|
||||
|
||||
### Example
|
||||
|
||||
|
|
|
|||
Loading…
Reference in a new issue