fix: accept standard UCP postal-address fields and reject malformed addresses loudly by relativvv · Pull Request #131 · shopware/agentic-commerce

Robin Schulte (relativvv) · 2026-07-22T11:24:19Z

Draft — extracted from the agent-shop integration project for upstream review.

Guest fulfillment/billing address handling on the UCP checkout path. Discovered while running an agent-only shop: the agent could not learn the required address shape and ended up fabricating an address to get past a misleading error.

Problem

Completion required fulfillment.shipping_address in a shape not present in the advertised schema.
A malformed/partial address was silently dropped and resurfaced later as a misleading "shipping_address is required".
Only the shop's non-standard field names under shipping_address were accepted; an agent trusting the published UCP postal_address schema (street_address / address_locality) failed validation.

Root cause

CheckoutGuestAddressPayloadResolver::normalize() read only fulfillment.extra.shipping_address with the shop-specific field names (street/zipcode/city) and returned null on a partial address — collapsing "malformed" into "missing." The address is in fact used as the guest's billing address for registration, so the "shipping" framing is misleading.

What changed

src/Ucp/Checkout/CheckoutGuestAddressPayloadResolver.php
- renamed normalize() → extractAddress();
- accept the address under shipping_address or billing_address;
- tolerate standard field-name aliases: line1 and the canonical UCP street_address for street; postal_code for zipcode; locality and the canonical address_locality for city; country for country code;
- throw a specific ValidationException naming the missing fields and the accepted shape when an address object is present but incomplete/malformed;
- return null only when no address object is present (legitimate fall-through to session metadata).
src/Ucp/Customer/GuestCustomerAddressResolver.php — the "no address at all" error message now names the accepted keys/shape.
tests/Unit/CheckoutGuestAddressPayloadResolverTest.php (new) — shop shape, standard-alias billing_address, canonical street_address/address_locality, malformed ⇒ throws naming fields, no-address ⇒ null.

Testing

php -l clean on all changed/added files (PHP 8.3).
New unit tests cover every accepted alias, the loud-rejection path, and the null fall-through.
Exercised live against a running agent-only shop: an agent supplying a canonical UCP postal_address now validates; a partial address returns a shape-documenting error instead of a downstream "required" message.
CI in this repo runs the full PHPUnit suite.

Notes for reviewers

Replayed onto current main; CheckoutGuestAddressPayloadResolver there still used normalize(), read only shipping_address with street/zipcode/city, and returned null on partial — the bug — so the fix applied cleanly with no conflicts.
Two upstream framings: (a) accept a standard postal-address shape / a billing_address for digital carts instead of only a non-standard shipping_address; (b) reject malformed fulfillment loudly at the point of entry with a shape-documenting error, instead of silently dropping it and failing later with a misleading message.
A published fulfillment schema / a formal checkout-requirements descriptor would supersede the "error documents the shape" stopgap — worth tracking separately.

…malformed fulfillment loudly The address resolver silently dropped a malformed address, which resurfaced as a misleading 'shipping_address is required' at the next step. Now accept the address under shipping_address OR billing_address (it is used as the guest billing address) with standard field-name aliases (line1/postal_code/locality/country), and throw a specific ValidationException naming the missing fields + accepted shape. The error itself documents the required shape.

…ress/address_locality)

Drop the redundant case/format variants (line_1, postalCode, zip, countryCode); keep shop-native (street/zipcode/city/country_code), canonical UCP (street_address/postal_code/address_locality/country) and the documented standard aliases (line1/locality). Matches the accepted-field list named in the validation errors; no test change needed.

@relativvv

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.

Björn Meyer (BrocksiNet) · 2026-08-05T09:20:26Z

Closing this in favour of #159, which fixes the same defect at its root — but your diagnosis of the silent drop was right, and one commit here is yours, so let me be precise about what moved and what did not.

What #159 does differently

You found that a malformed address is silently dropped and resurfaces later as a misleading "shipping_address is required". Same finding, and we hit it from the other end: the store suite in shopware/shopware-mcp-evals could reach checkout.complete for the first time and got that exact message.

The difference is where the address is read from. Checked against the 2026-04-08 schemas:

concept	UCP location
shipping address	`fulfillment.methods[].destinations[]`
billing address	`payment.instruments[].billing_address`
`shipping_address`	does not exist anywhere in the schemas
`fulfillment.billing_address`	does not exist

fulfillment.shipping_address, fulfillment.billing_address and fulfillment.extra.shipping_address all validate — I ran them through GeneratedSchemaValidator — but only because fulfillment does not restrict additionalProperties, so an off-spec key is accepted and ignored. That is the same mechanism that silently swallowed a top-level fulfillment_address on our side. Validating and being in the protocol are not the same thing.

The practical consequence is that this PR's error text — "Provide fulfillment.extra.shipping_address (or billing_address)…" — teaches an agent a shape that no published schema describes, and its violations still point at $.checkout_session.fulfillment.shipping_address.*, a path with no property behind it. That misleading path is the thing that cost us a day: it reads as proof that UCP has no usable address channel, when in fact destinations[] was there the whole time and the plugin simply never looked at it (grep -rn destinations src/ returned nothing).

Worth recording, since it fooled us too: the reason destinations looked unusable is that branch 0 of the oneOf (shipping_destination) is an allOf of postal_address and {required: ["id"]}. A bare address matches neither branch; anything carrying name matches both. Exactly one shape validates — {id, …postal address}, no name.

Lifted from here, with credit

The loud rejection, as f4191863 on #159. postal_address marks nothing required, so an incomplete one is schema-valid and this layer is the only place that can catch it — your point stands regardless of which container the address arrives in.

One thing we had to handle that presence-of-key does not cover: 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 the trigger is any postal field present rather than the key exists, with address_region / extended_address counting as evidence too. Violations name the element actually read, 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

The legacy shipping_address container is still accepted for compatibility, is now rejected loudly when partial, and its message points at fulfillment.methods[].destinations[].

Your billing-address point is correct, and currently unimplementable

This is the part I want to flag rather than quietly drop. Your rationale — the address is used as the guest's billing address, and a digital cart has no shipping destination — is right, and UCP agrees: billing_address exists as a postal_address on payment.instruments[]. For a cart with nothing to ship, that is the only address the protocol offers.

We could not lift it, because the SDK throws it away:

final class PaymentInstrument
{
    public function __construct(
        public readonly string $type,
        public readonly string $handlerId,
        public readonly array $credential = [],
    ) {}
}

No billing_address, and the adapter only ever sees this typed model — there is no raw-payload path like FulfillmentSelection::$extra. So an agent can send a spec-conformant billing address today and the plugin cannot see it. That is the same class of bug as the one this PR is about, one layer down, and it needs an SDK change before the plugin side is worth writing. Tracked; I'd rather fix it there than add a fourth off-spec container here.

Not adopted

fulfillment.shipping_address / fulfillment.billing_address as containers — off-spec, see above.
line1, locality, country aliases — those are not postal_address field names either, so they add a third vocabulary rather than converging on the spec's.

Thanks for this — the silent-drop finding was the valuable half and it is in. If you disagree on the container question I am happy to reopen and argue it against the schemas; the shape decision should follow the protocol, not either of our preferences.

@relativvv

…ges (#159) * fix(ucp): read the shipping address where UCP puts it, and type cart 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. * fix(ucp): reject a partial address instead of dropping it 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.

@relativvv

…code (#160) * fix(ucp): read the shipping address where UCP puts it, and type cart 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. * fix(ucp): reject a partial address instead of dropping it 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. * fix(ucp): log the exception the MCP layer refuses to show, and report 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. * fix: require the SDK release that carries UcpErrorDescriptor `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. * fix: bump the SDK version pin the smoke stack declares too `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.

Robin Schulte (relativvv) added 4 commits July 22, 2026 13:23

fix(ucp): accept canonical UCP postal_address field names (street_add…

48d4a45

…ress/address_locality)

fix: satisfy phpstan return-type on extractAddress

0624947

Robin Schulte (relativvv) self-assigned this Jul 23, 2026

Robin Schulte (relativvv) marked this pull request as ready for review July 23, 2026 07:33

Robin Schulte (relativvv) requested review from Björn Meyer (BrocksiNet) and Lukas Rump (lukasrump) July 23, 2026 07:33

Merge branch 'main' into fix/ucp-guest-address-handling

1203699

Björn Meyer (BrocksiNet) closed this Aug 5, 2026

This was referenced Aug 5, 2026

fix: read the shipping address where UCP puts it, and type cart messages #159

Merged

fix(ucp): register the billing and shipping address UCP actually states #161

Draft

Navigation Menu

Uh oh!

fix: accept standard UCP postal-address fields and reject malformed addresses loudly - #131

fix: accept standard UCP postal-address fields and reject malformed addresses loudly#131
Robin Schulte (relativvv) wants to merge 5 commits into
mainfrom
fix/ucp-guest-address-handling

Robin Schulte (relativvv) commented Jul 22, 2026

Uh oh!

Björn Meyer (BrocksiNet) commented Aug 5, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

Robin Schulte (relativvv) commented Jul 22, 2026

Problem

Root cause

What changed

Testing

Notes for reviewers

Uh oh!

Björn Meyer (BrocksiNet) commented Aug 5, 2026

What #159 does differently

Lifted from here, with credit

Your billing-address point is correct, and currently unimplementable

Not adopted

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants