Commit 45c1b00
authored
feat(mcp): carry union keywords to the provider instead of skipping the tool (#853)
Implements the decision on #848: carry the union, do not skip the tool.
**The rule that was there is sound, but it answered a different
question.** Refusing `anyOf` protects against *dropping* it — a stored
schema without the union permits more than the server does, so a model
can produce arguments the server then rejects. Carrying the keyword
unchanged does not do that: `ToolSpec::toArray()` puts the stored schema
into the provider payload verbatim, and the providers this extension
talks to accept a union in a function schema. The narrowing the server
declared survives end to end.
`anyOf`, `oneOf`, `allOf` and `not` are therefore carried now, and added
to the retained top-level keys so a union declared at the root survives
the filter too (inside a property it rides along with the property
schema, which is copied whole).
**What stays refused, and why the line runs exactly there:**
- `$ref`, `$dynamicRef`, `$defs`, `definitions` — a reference points
into a definition block this filter does not carry, so carrying the
pointer alone would hand the provider a dangling reference. Refusing is
the honest answer until the definition block is carried too.
- `if`/`then`/`else`, `dependentSchemas`, `dependentRequired`,
`unevaluatedProperties`, `patternProperties`, `propertyNames` — the
draft-2019/2020 applicators have no dependable support across providers.
Carrying them would trade an import-time refusal, which names the
keyword since #849, for a call-time failure with a worse message.
**The case that prompted this.** `https://mcp.deepwiki.com/mcp`
advertises three tools; `ask_question` declares `repoName` as
`anyOf[string, array<string>]` and was the one skipped, while its two
siblings imported. That is now the test: the schema is carried verbatim
from the real server's response, and the assertion checks the `anyOf`
block arrives unchanged rather than merely that the tool imports.
**Tests:** the real-world union imports and keeps its `anyOf`; a
top-level union survives the key filter; a `$ref` schema is still
refused and its reason names the keyword. Two existing data-provider
cases asserted the old behaviour for `allOf` and nested `oneOf` — they
are replaced by cases for the applicators that remain refused, so the
provider still pins a real boundary rather than shrinking.
Gates run locally: cgl, PHPStan level 10, the normaliser unit suite, and
the functional `McpImport` suite on sqlite. CI covers the eight-cell
matrix.
Closes #848.
_Assisted by claude-code:claude-fable-5 —
[Session](https://claude.ai/code/session_0144iD1P22LotW8rxmxrNGro)_4 files changed
Lines changed: 89 additions & 22 deletions
File tree
- Classes/Service/Tool/Mcp
- Tests/Unit/Service/Tool/Mcp
- Conformance
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
8 | 8 | | |
9 | 9 | | |
10 | 10 | | |
| 11 | + | |
11 | 12 | | |
12 | 13 | | |
13 | 14 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
49 | 49 | | |
50 | 50 | | |
51 | 51 | | |
| 52 | + | |
| 53 | + | |
| 54 | + | |
| 55 | + | |
52 | 56 | | |
53 | 57 | | |
54 | 58 | | |
55 | 59 | | |
56 | 60 | | |
57 | 61 | | |
58 | 62 | | |
| 63 | + | |
| 64 | + | |
| 65 | + | |
| 66 | + | |
59 | 67 | | |
60 | 68 | | |
61 | 69 | | |
62 | | - | |
63 | | - | |
64 | | - | |
65 | | - | |
66 | | - | |
| 70 | + | |
| 71 | + | |
| 72 | + | |
| 73 | + | |
| 74 | + | |
| 75 | + | |
| 76 | + | |
| 77 | + | |
| 78 | + | |
| 79 | + | |
| 80 | + | |
| 81 | + | |
67 | 82 | | |
68 | 83 | | |
69 | 84 | | |
| |||
74 | 89 | | |
75 | 90 | | |
76 | 91 | | |
77 | | - | |
78 | | - | |
79 | | - | |
80 | | - | |
81 | 92 | | |
82 | 93 | | |
83 | 94 | | |
| |||
Lines changed: 6 additions & 2 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
238 | 238 | | |
239 | 239 | | |
240 | 240 | | |
241 | | - | |
242 | | - | |
| 241 | + | |
| 242 | + | |
| 243 | + | |
| 244 | + | |
| 245 | + | |
| 246 | + | |
243 | 247 | | |
244 | 248 | | |
245 | 249 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
102 | 102 | | |
103 | 103 | | |
104 | 104 | | |
105 | | - | |
106 | | - | |
107 | | - | |
108 | | - | |
109 | | - | |
110 | | - | |
111 | 105 | | |
112 | 106 | | |
113 | 107 | | |
| |||
119 | 113 | | |
120 | 114 | | |
121 | 115 | | |
122 | | - | |
| 116 | + | |
| 117 | + | |
| 118 | + | |
| 119 | + | |
| 120 | + | |
123 | 121 | | |
124 | | - | |
| 122 | + | |
| 123 | + | |
| 124 | + | |
| 125 | + | |
| 126 | + | |
| 127 | + | |
| 128 | + | |
125 | 129 | | |
126 | 130 | | |
127 | 131 | | |
| |||
236 | 240 | | |
237 | 241 | | |
238 | 242 | | |
239 | | - | |
| 243 | + | |
240 | 244 | | |
241 | 245 | | |
242 | 246 | | |
| |||
263 | 267 | | |
264 | 268 | | |
265 | 269 | | |
266 | | - | |
| 270 | + | |
| 271 | + | |
| 272 | + | |
| 273 | + | |
| 274 | + | |
| 275 | + | |
| 276 | + | |
| 277 | + | |
| 278 | + | |
| 279 | + | |
| 280 | + | |
| 281 | + | |
| 282 | + | |
| 283 | + | |
| 284 | + | |
| 285 | + | |
| 286 | + | |
| 287 | + | |
| 288 | + | |
| 289 | + | |
| 290 | + | |
| 291 | + | |
| 292 | + | |
| 293 | + | |
| 294 | + | |
| 295 | + | |
| 296 | + | |
| 297 | + | |
| 298 | + | |
| 299 | + | |
| 300 | + | |
| 301 | + | |
| 302 | + | |
| 303 | + | |
| 304 | + | |
| 305 | + | |
| 306 | + | |
| 307 | + | |
| 308 | + | |
| 309 | + | |
| 310 | + | |
| 311 | + | |
| 312 | + | |
| 313 | + | |
| 314 | + | |
| 315 | + | |
| 316 | + | |
267 | 317 | | |
| 318 | + | |
268 | 319 | | |
269 | 320 | | |
270 | | - | |
| 321 | + | |
271 | 322 | | |
272 | 323 | | |
273 | 324 | | |
| |||
0 commit comments