Skip to content

fix!: discriminate destinations at the method level - #689

Merged
igrigorik merged 2 commits into
feat/typed-fulfillment-destinationsfrom
lr/response-only-destination-types
Aug 6, 2026
Merged

fix!: discriminate destinations at the method level#689
igrigorik merged 2 commits into
feat/typed-fulfillment-destinationsfrom
lr/response-only-destination-types

Conversation

@richmolj

@richmolj richmolj commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

Counter-proposal for the request side of #688; response-side typing is unchanged. Context: Slack thread.

Problem

#688 discriminates destinations per-object in both directions. Requiring type on request destinations invalidates every deployed Platform integration — requests that validate today fail the moment the requirement lands. Response-side type is additive: producers add a field; consumers ignore unknown fields under the open-world model. The two directions carry incomparable migration costs.

The per-object tag also leaves the agreed authority boundary unenforced. { "type": "business_location", "id": ... } remains a schema-valid Platform-written destination, so a written destination and selected_destination_id can select different locations with no precedence rule — the conflict #688 sets out to remove.

Solution

Move the discriminator up one level: a fulfillment method's type selects the shape of its entire subtree, destinations included. A shipping method has Shipping Destinations; a pickup method has Business Location Destinations; extension-defined method types define their own. Polymorphism resolves at the parent, so request destinations need no per-object tag, and the request wire format Platforms send today remains valid unchanged.

// fulfillment_method.json — illustrative
"dependentRequired": { "destinations": ["type"] },
"allOf": [
  { "if":   { "properties": { "type": { "const": "shipping" } }, "required": ["type"] },
    "then": { "properties": { "destinations": { "items": { "$ref": "shipping_destination.json" } } } } },
  { "if":   { "properties": { "type": { "const": "pickup" } }, "required": ["type"] },
    "then": { "properties": { "destinations": {
      "ucp_request": "omit",
      "items": { "$ref": "location_destination.json" } } } } }
]

Plain draft-2020 if/then + $ref — no direction- or context-specific request schemas.

Design decisions:

  • Pickup destinations are response-only, schema-enforced. ucp_request: "omit" inside the pickup branch removes destinations from the request projection; under strict resolution a Platform-written location is rejected as unevaluated. selected_destination_id is the sole selection channel and accepts any stable, Business-scoped Location ID the Business recognizes for the method, including IDs not yet enumerated in destinations[] — the feat!: Introduce Location Search + Lookup capabilities #589 handoff. One channel, one authority.
  • dependentRequired: {"destinations": ["type"]}. Method type is update-optional (target by id), so a request that writes destinations[] must carry the method type for branch dispatch. Updates touching only selected_destination_id or selected_option_id are unaffected.
  • Responses are unchanged from fix!: make destination types explicit #688. Every response destination carries a required, open type and remains self-describing. type stays optional in requests for reference disambiguation (e.g., an id-only destination naming one of several address sources).
  • Extension-defined method types match no core branch; the negotiated extension's schema defines destination shape and writability.

Before / After — strict request validation

Case #688 this PR
Untyped inline shipping address (today's clients) rejected valid
Unknown field on a shipping address rejected rejected
Platform writes a business location into destinations[] valid (blessed shape) rejected
destinations[] without method.type n/a rejected (dependentRequired)
Update touching only selection fields valid valid

Resolver change

ucp-schema --strict seals item schemas referenced from base properties but does not descend into if/then/else; branch item schemas are never sealed. One match arm in close_additional_properties_inner (resolver.rs):

"then" | "else" => {
    // Conditional branches apply in-place alongside siblings: don't seal
    // the branch itself, but descend so nested properties/items get sealed.
    close_additional_properties_inner(child, true);
}

Verified against a patched build: the full matrix above holds on unmodified resolver output; resolver unit tests pass (153/153). Diagnostic trade-off: a failed then contributes no annotations, so some rejections surface as a root-level unevaluated-destinations error rather than an item-level message — verdicts unchanged, messages less precise.

Trade-offs

  • Pickup write-ban enforcement applies under strict resolution; open validation remains permissive and the MUST NOT prose governs that regime. Strictly stronger than fix!: make destination types explicit #688, where the written location is schema-blessed in both regimes.
  • The schema_fields macro does not collect branch-scoped properties, so destinations no longer renders in the Fulfillment Method field table (covered in the Destinations section). Either the macro learns then.properties, or destinations are documented per method type.
  • fulfillment_destination.json is no longer schema-referenced; retained as the response-side documentation entity, or foldable into docs.

richmolj and others added 2 commits August 5, 2026 15:46
…ly in requests

A fulfillment method's type selects the shape of its entire subtree,
destinations included: a shipping method has shipping-address
destinations, a pickup method has business-location destinations, and
extension-defined method types define their own. Polymorphism is
resolved at the parent, so request destinations need no per-object
discriminator.

- fulfillment_method branches per method type; the generic
  fulfillment_destination union is no longer referenced by schemas
  (kept for response documentation).
- Destination type is required in responses, optional in requests.
- destinations under pickup is response-only (ucp_request omit inside
  the pickup branch): under strict resolution the Platform cannot write
  business locations. selected_destination_id is the sole selection
  channel and accepts any Business-scoped Location ID the Business
  recognizes for the method, including IDs not yet enumerated (#589
  handoff).
- dependentRequired: a request that writes destinations[] must carry
  the method's type.
- Removed explicit additionalProperties:true from fulfillment_method
  (behavior-neutral in open validation; lets strict sealing work).
- Existing request wire shapes are unchanged; responses gain the
  required type field.

Assisted-By: devx/296664b9-53b6-409a-989a-ace9d3348247
   Fulfillment responses always self-describe with a required destination type,
   while Platform requests follow the enclosing method's authorship contract.

   Clarify that untyped destinations under the well-known shipping method default
   to Shipping Destination, pickup destinations are Business-authored and selected
   through selected_destination_id, and other method types define their own request
   shape and Platform writability.

   Remove the unsupported suggestion that an alternate destination type can be
   selected under core shipping. An ID-only saved or provider-held address remains
   a Shipping Destination; provider provenance or additional fields require a
   negotiated extension contract.
@igrigorik
igrigorik marked this pull request as ready for review August 5, 2026 21:52
@damaz91 damaz91 added the status:needs-triage Signal that the PR is ready for human triage label Aug 5, 2026
@igrigorik
igrigorik merged commit 455ab32 into feat/typed-fulfillment-destinations Aug 6, 2026
4 of 5 checks passed
@igrigorik
igrigorik deleted the lr/response-only-destination-types branch August 6, 2026 04:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

status:needs-triage Signal that the PR is ready for human triage

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants