Skip to content

fix(purchase): always send tax in ConfirmPayment, including 0 - #79

Merged
dinosoeren merged 1 commit into
mainfrom
soeren/confirm-tax-always-sent
Oct 29, 2026
Merged

dinosoeren merged 1 commit into
mainfrom
soeren/confirm-tax-always-sent

Conversation

@dinosoeren

@dinosoeren dinosoeren commented Oct 29, 2026 •

Copy link
Copy Markdown
Collaborator

Why

A game backend that validates the published ConfirmPayment schema declined test purchases that had no tax, answering HTTP 400 "tax missing or invalid": the schema lists tax as required and says it can be 0, but Stash dropped the key whenever the value was 0. This PR makes the contract able to carry an explicit 0, so the sender can keep the promise the docs already make. Ticket: DIS-199 ("Purchase confirmation drops a zero tax, so the game backend declines the purchase"), https://linear.app/stashgg/issue/DIS-199

Intent

tax on ConfirmPaymentRequest becomes optional uint32, so it has explicit presence and protojson emits it whenever it is set, including the literal 0. This is the same change #23 made for decimal_places on Register, Confirm and Cancel.

flowchart LR
  P["ConfirmPaymentRequest<br/>tax = 5<br/>new: optional uint32"]:::this
  D["swagger.v1.json<br/>tax description<br/>new: always sent"]:::this
  G["Go stubs<br/>Tax becomes *uint32"]:::this
  S["pie-mono clients<br/>set Tax explicitly"]:::open
  J{{"protojson<br/>emits any set field,<br/>including 0"}}:::existing
  B(["Game backend<br/>confirm endpoint"]):::existing
  P --> D
  P --> G
  G --> S
  S --> J
  J -- "tax: 0 present" --> B
  classDef done fill:#d3f9d8,stroke:#2f9e44,stroke-width:1px,color:#111
  classDef this fill:#ffd8a8,stroke:#e8590c,stroke-width:4px,color:#111
  classDef open fill:#e5dbff,stroke:#7048e8,stroke-width:1px,color:#111
  classDef todo fill:#f1f3f5,stroke:#868e96,stroke-width:1px,stroke-dasharray:5 5,color:#333
  classDef existing fill:#dbe4ff,stroke:#4263eb,stroke-width:1px,color:#111
Loading

Legend: orange = changed here, purple = the open consumer PR that sets the field, blue = existing behavior; hexagon = rule, rounded = outside service.

Big Picture

flowchart LR
  subgraph S1["Contract"]
    direction TB
    A1["public-api 79<br/>tax optional"]:::this
  end
  subgraph S2["Wire fix"]
    direction TB
    A2["pie-mono 19541<br/>bump + set tax"]:::open
  end
  subgraph S3["Verify"]
    direction TB
    A3["Deploy pie-mono<br/>test shop 0-tax<br/>confirm shows tax 0"]:::todo
  end
  subgraph S4["Precedent"]
    direction TB
    A0["public-api 074a2fe<br/>decimal_places optional"]:::done
  end
  S4 -- "same pattern" --> S1
  S1 -- "regenerated stubs" --> S2
  S2 -- "deployed" --> S3
  subgraph Legend
    direction TB
    L1["merged"]:::done
    L2["this PR"]:::this
    L3["other open PR"]:::open
    L4["not done yet"]:::todo
  end
  classDef done fill:#d3f9d8,stroke:#2f9e44,stroke-width:1px,color:#111
  classDef this fill:#ffd8a8,stroke:#e8590c,stroke-width:4px,color:#111
  classDef open fill:#e5dbff,stroke:#7048e8,stroke-width:1px,color:#111
  classDef todo fill:#f1f3f5,stroke:#868e96,stroke-width:1px,stroke-dasharray:5 5,color:#333
  classDef existing fill:#dbe4ff,stroke:#4263eb,stroke-width:1px,color:#111
Loading
  • Precedent: 074a2fe "fix(purchase): emit decimal_places for zero-decimal currencies" (fix(purchase): emit decimal_places for zero-decimal currencies #23)
  • Contract: this PR
  • Wire fix: stashgg/pie-mono#19541 "send tax on every ConfirmPayment, including 0" (bump proto/public, set tax on both confirm builders, tests)
  • Verify: a 0-tax confirm after the consumer deploys carries "tax":0

This PR goes first; the consumer PR points its submodule at this PR's merge commit on main and is what changes the bytes partners receive. Nothing is removed or superseded later. The JSON name, field number and type are unchanged, and the OpenAPI required list is unchanged; only the tax description gains "Always sent, including the literal 0 when the purchase has no tax."

Details

Breaking-change check (expected). buf breaking reports field 5 tax on ConfirmPaymentRequest changing from implicit to explicit presence (FIELD_SAME_CARDINALITY), so the Check job fails by design. Per this repo's rule, there is no buf.yaml exception. Why it is safe for game backends: on the wire the field keeps its number, type and JSON name, so every existing reader decodes the same values; the only observable change is that a 0 tax now arrives as "tax": 0 instead of no key, which the schema already required. In generated Go code the field becomes *uint32 (GetTax() still returns uint32); the only Go consumer is updated in the companion PR.

Changed files. server/egress/shop/purchase/v1/service.proto (field and description) and the regenerated docs/gen/swagger.v1.json. A structural diff of the regenerated swagger shows exactly two leaves changed, both tax.description.

Audit of the other request messages. Every request Stash sends under server/egress/** was checked for the same pattern (required or documented always-sent, non-optional scalar or enum, legitimately zero or empty). tax is the only one. The other required scalars either forbid zero or empty by validation (total, time_millis, line-item quantity and cents, bonus quantity, ids and currency) or are already optional (decimal_places).

part of DIS-199

🤖 Generated with Claude Code


Caution

💥 Blast Radius — Critical Risk

Intent: Enable valid zero-tax purchase confirmations to satisfy the published schema and avoid rejection by game backends.

The acknowledged protobuf presence change breaks regenerated Go callers that assign scalar Tax values, requiring coordinated consumer adoption and explicit owner sign-off.

Overview
server/egress/shop/purchase/v1/service.proto changes ConfirmPaymentRequest.tax to optional uint32 and documents explicit zero emission. One withheld generated file is accounted for without inspection.

Shop reach: all consumers of the shared ConfirmPayment contract; no shop gate exists on this field, and this checkout provides no deployed-shop inventory.

Data/proto: Field number 5, wire type, JSON name, required-schema membership, and minor-unit meaning remain unchanged. Generated Go Tax changes from uint32 to *uint32; the PR explicitly acknowledges the expected FIELD_SAME_CARDINALITY failure without changing the breaking-check configuration.

Payments path: With an updated sender, confirmation builder → ConfirmPaymentRequest.tax → protojson → /api/v1/purchase/confirm → game backend carries "tax": 0 where zero previously disappeared. Nonzero numeric values retain their meaning; an unset optional field remains omitted.

Rollout: No sender implementation exists in this checkout. The companion consumer update must populate Tax explicitly; this schema change alone does not establish that deployed confirmations emit zero.

Validation: Source and consumer searches confirm the scoped contract change. buf and protoc are unavailable locally, so their checks were not run.

Scope is the shared confirmation contract and one generated artifact; registration, cancellation, confirmation error statuses, and decimal_places remain unchanged.

Not reviewed: 1 generated file(s) — marked linguist-generated in .gitattributes: docs/gen/swagger.v1.json.

Ripley Review v0.1.0 · 4b10e24 · Report Issue

ConfirmPaymentRequest lists tax as required and documents that it can be 0,
but it was a plain proto3 uint32 with implicit presence. protojson omits an
implicit-presence zero, so a purchase with no tax reached the game backend
with no tax key at all, and a backend that validates the published schema
rejected the confirm as missing tax.

Make tax `optional uint32` so presence is explicit and the sender always
serializes the literal 0, the same change made for decimal_places in #23.
Field number, type and JSON name are unchanged, so the wire and JSON stay
compatible. buf breaking reports the implicit-to-explicit presence change on
this field; that is expected and acknowledged in the PR rather than
silenced in buf.yaml.

part of DIS-199

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@dinosoeren
dinosoeren merged commit e97d19f into main Oct 29, 2026
2 of 3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant