fix: log the exception the MCP layer refuses to show, and report its code - #160
Merged
Björn Meyer (BrocksiNet) merged 5 commits intoAug 5, 2026
Merged
Conversation
…messages Two defects that between them made a conformant UCP agent unable to buy anything, both found by the functional store suite. **No address could be set.** The guest-address resolver read only `fulfillment.shipping_address`, which is not a property of `checkout.create`, `checkout.update` or `checkout.complete` in any UCP version. The protocol puts the address in `fulfillment.methods[].destinations[]`, and the plugin never looked there — `grep -rn destinations src/` found nothing — so completion always refused with "Checkout session is missing fulfillment.shipping_address" no matter what the agent sent. It now reads a destination, preferring `selected_destination_id` over the first entry, accepting both branches of the oneOf (a shipping_destination carries the postal address inline, a retail_location nests it under `address`) and mapping schema.org's names — `street_address`, `postal_code`, `address_locality`, `address_country` — onto the Shopware address. The old shape still works. The violation paths named `$.checkout_session.fulfillment.shipping_address` too, so the one message that says what is missing pointed at a field nothing could fill. They now name the property an agent can set. **A cart message failed the response schema.** `mapCartMessages()` emitted `type: cart_error`. `types/message.json` is a oneOf whose three branches pin `type` with a const of `error`, `warning` or `info`, so a fourth spelling matched no branch and the WHOLE response failed with `$ must match exactly one allowed schema` — which the executor then reports to the agent as a server error even though its request was fine and the write had succeeded. Not a corner case: a successful `discount.apply` leaves `promotion-discount-added` on the cart (`PromotionCartAddedInformationError`, LEVEL_NOTICE, persistent), so applying a valid code failed by construction, and every later cart or checkout response carried the same poison. Shopware's three error levels map onto UCP's three types, so the level is the mapping; `getMessageKey()` stays the code, since error_code, warning_code and info_code are all freeform. `severity` is set for `error` only, where the schema requires it, and `recoverable` is the honest value: the platform can change the cart and retry. Measured against GeneratedSchemaValidator: `cart_error` and an `error` without a severity are rejected; `info`, `warning` and `error` + `severity` pass. UcpResponseSchemaTest's cart fixture now carries the promotion notice a real cart would, so all seven cart and checkout operations cover this rather than `discount.apply` alone — the fixture being error-free is why `cart_error` survived a test suite that validates against the real schemas.
Lifted from #131 by @relativvv, which found this from the other side while running an agent-only shop: an address that was present but malformed returned `null`, fell through to the stored session address, and resurfaced two steps later as a message about a different field. The agent had no way to learn that what it sent was wrong. `postal_address` marks nothing required, so an incomplete one is schema-valid — this layer is the only place that can catch it. The distinction that matters is **attempted** versus **absent**, and it is not "is the key there". A destination carrying only an `id` is not a broken address: `shipping_destination` requires `id` and nothing else, so selecting a destination the business already offered looks exactly like that, and it must keep falling through to the stored address. So any postal field present is read as intent, and then the missing ones are named. `address_region` and `extended_address` count as evidence too — a destination naming those and no street is unambiguously a botched address rather than a selection. Violations name the element actually read rather than assuming `[0]`, since `selected_destination_id` can point anywhere in the list: $.fulfillment.methods[1].destinations[1].street_address is required $.fulfillment.methods[1].destinations[1].postal_code is required and a retail location gets the nested path, `destinations[0].address.*`. The legacy `shipping_address` container is rejected on presence alone — that key exists for nothing but an address, so no field-level evidence is needed — and its message points at `fulfillment.methods[].destinations[]`, which is where UCP actually puts the address. #131's own error text taught the off-spec shape instead.
… its code
`failure()` answered any non-UcpException with
`{"type":"internal","message":"The tool call failed unexpectedly."}` and logged
nothing anywhere. The generic response is deliberate — PR #154 chose not to leak
internals to an unauthenticated MCP client — but the exception vanished with it.
The only way to see a cause was to call the same operation over REST, where the
SDK's ExceptionListener does not swallow it: that is how an unreachable platform
profile was eventually found, out of `var/log/prod-*.log`, after the MCP
transport had reported `internal` for a dozen runs.
Every throwable is now logged with itself attached, so Monolog renders the class,
the message and the file:line. The response body is unchanged for internal
faults.
The body also carries `code` and `severity` now, taken from the SDK's
UcpErrorDescriptor — the same mapping the HTTP listener reads, so the two
transports cannot describe one exception differently. They used to: an
unreachable profile answered 424 `agent_profile_unreachable` `recoverable` over
REST and a bare `internal` over MCP. `types/message_error.json` requires both
fields, and an agent that has to parse prose to decide whether to retry has no
reason to get it right.
Shopware's own exceptions get the same treatment. They are
`HttpExceptionInterface`, not `UcpException`, so they were reported as internal
with their message hidden: `order.get` on a guest order answered "The tool call
failed unexpectedly." over MCP while REST answered 403 "Customer is not logged
in." A 4xx message is written for the caller and the SDK's listener already
passes it through, so the MCP transport now agrees. 5xx stays generic — that
message is written for an operator, and this client is unauthenticated.
The private errorType() match is gone; it was the fragment of that mapping this
class had reimplemented, and it knew about seven of the ten exception types.
Björn Meyer (BrocksiNet)
force-pushed
the
fix/mcp-error-code-severity-and-logging
branch
from
August 5, 2026 09:19
7a2ae0a to
7d6ce8e
Compare
Björn Meyer (BrocksiNet)
marked this pull request as ready for review
August 5, 2026 09:27
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7d6ce8e10b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "Codex (@codex) review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "Codex (@codex) address that feedback".
Lukas Rump (lukasrump)
approved these changes
Aug 5, 2026
This was referenced Aug 5, 2026
`UcpMcpToolContext` uses `Ucp\Sdk\Model\Common\UcpErrorDescriptor`, added in
ucp-php-sdk 0.0.4. The constraint was `>=0.0.2 <0.1.0`, so a plain
`composer install` could resolve 0.0.2 or 0.0.3 from Packagist and every MCP tool
failure path would fatal on a missing class — on the failure path, where it is
least likely to be noticed before a customer finds it.
The CI version override moves with it, and that is not optional. `ci.yml` declares
the SDK path packages' versions, because the packages carry no `version` field of
their own:
"versions":{"ucp-php-sdk/core":"0.0.2"}
Left at 0.0.2 while the requirement says `>=0.0.4`, Composer cannot resolve and CI
fails for a reason that has nothing to do with the code. The two have to move in
the same commit.
This also stops CI measuring a version nobody ships: the pin has been declaring
0.0.2 while checking out the SDK's default branch, so the plugin's own pipeline
has never exercised the release its constraint exists to admit.
`shopware-matrix` failed on the previous commit: the constraint moved to `>=0.0.4` but `bin/ci-smoke.sh` declares its own copy of the path-repo versions, and it still said 0.0.2. So Composer could not resolve inside the smoke stack. That is two places declaring the same fiction — `ci.yml` for php-quality and ci-smoke.sh for the deployed stack — and I only found the first by grepping the workflow. Both now say 0.0.4, and there are no `0.0.2` pins left anywhere under bin/ or .github/. Worth remembering: a version this repo declares in more than one file is a version it can disagree with itself about.
Base automatically changed from
fix/ucp-fulfillment-destinations-and-cart-messages
to
main
August 5, 2026 12:57
Björn Meyer (BrocksiNet)
deleted the
fix/mcp-error-code-severity-and-logging
branch
August 5, 2026 12:59
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
2 participants
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.
php-qualityfails on all four lanes with seven PHPStan errors, all one missing class:That class is added in ucp-php-sdk#111, which is not merged yet.
validation-gatecarries onlyProcess completed with exit code 1.— the step-exit marker reacting to the above, not a second failure. The base PR #159 is green on all fourphp-qualitylanes, so nothing in the shared code is at fault.It self-heals with no change to this PR.
ci.ymlresolves the SDK from a path repository on a sibling checkout atUCP_SDK_REF(defaultmain), version-pinned to"0.0.2"regardless of what the files actually contain. So the moment #111 lands on the SDK'smain, this pipeline goes green.To prove it before then, without committing a pin:
Correcting an earlier version of this note, which said this needs SDK 0.0.4 and a constraint bump to merge. For the pipeline it does not — see above. For shipping it does, and the distinction matters:
main>=0.0.4 <0.1.0The plugin currently requires
>=0.0.2 <0.1.0(#157). A realcomposer installresolves that from Packagist and would get 0.0.3 — noUcpErrorDescriptor— so every MCP failure path would fatal at runtime while CI was green. That gap is exactly the shape of #157's own finding: CI measuring a different version than the one that matters. Bump the constraint as the last commit before merge, not now —>=0.0.4cannot resolve until 0.0.4 exists and would turn the pipeline red for an unrelated reason.Merge order: #111 → release 0.0.4 → bump the constraint here → merge #159 → retarget this to
main→ merge.Why is this change necessary?
UcpMcpToolContext::failure()answered any non-UcpExceptionwith{"type":"internal","message":"The tool call failed unexpectedly."}and logged nothing anywhere.
The generic response is deliberate — #154 chose not to leak internals to an unauthenticated MCP client, and that judgement stands. The problem is that the exception vanished with it. The only way to see a cause was to call the same operation over REST, where the SDK's
ExceptionListenerdoes not swallow it. That is how an unreachable platform profile was eventually found, read out ofvar/log/prod-*.log, after the MCP transport had reportedinternalfor a dozen runs. Logging server-side leaks nothing to the client and is the whole difference between a five-minute diagnosis and an afternoon.The body was also missing the two fields that make a failure machine-readable. Same SDK, same request, one operation, two transports:
424,code: agent_profile_unreachable,severity: recoverable, message naming the URI and port{"type":"internal","message":"The tool call failed unexpectedly."}types/message_error.jsonrequirescodeandseverity. This class had reimplemented a fragment of the SDK's mapping in a privateerrorType()match that knew about seven of the ten exception types and produced neither field.What does this change do?
Logs every throwable before flattening it, with the throwable itself attached, so Monolog renders the class, the message and the
file:line. The response body for internal faults is byte-identical to before.Takes
codeandseverityfromUcpErrorDescriptor— the same mapping the SDK's HTTP listener reads, so the two transports can no longer describe one exception differently.errorType()is deleted; it becomes$descriptor->type.Passes through the message of a Shopware client error. Shopware's own exceptions are
HttpExceptionInterface, notUcpException, sofromThrowable()alone calls them internal and hides what they say. That is howorder.geton a guest order answered "The tool call failed unexpectedly." over MCP while REST answered403 Customer is not logged in.A 4xx message is written for the caller and the SDK's listener already passes it through, so the MCP transport now agrees.5xx stays generic on purpose: that message is written for an operator, and this client is unauthenticated.
How to test
UcpMcpToolContextTestcovers the descriptor mapping per exception type, the agent-profile case specifically, a 4xx message passing through, a 5xx one staying hidden, and — via a collecting logger — that a\RuntimeExceptionstill answers generically and reaches the log with the throwable in theexceptionkey.Verified live on a trunk lane. Before, this exact call answered a bare
internal:and the log now carries it:
In the store suite the last failing check went from
internal: The tool call failed unexpectedly.torequest: Customer is not logged in.— which is how theorder.getguest-order defect got diagnosed at all.Worth knowing when reading a lane's log
Some failures are logged at
errorby this code and still do not reachvar/log. Core'sErrorCodeLogLevelHandlerdowngrades a long list of error codes tonoticeviashopware.logger.error_code_log_levels, andCHECKOUT__ORDER_CUSTOMER_NOT_LOGGED_INis on it, so in prod it falls below the file handler's threshold. That is core behaving as configured, not this change failing — the agent-profile line above proves the path works.Not addressed here
codeandseverityare absent from the tool's success bodies, which wrap the operation payload rather than the UCP envelope. Only the error shape changes here.order.getguest-order failure itself — this change only makes it legible. See fix: read the shipping address where UCP puts it, and type cart messages #159's "Not addressed here". Independently observed from the other end in fix: never report unpaid orders as completed; advertise delegated payment handlers #130, whose own follow-up notes that thecheckout.completereplay path also needs a Shopware context token a pure-UCP agent does not hold.