diff --git a/.planning/phases/18-payment-gateway/18-REVIEW-DISPOSITION.md b/.planning/phases/18-payment-gateway/18-REVIEW-DISPOSITION.md index dbd4ba9..2255250 100644 --- a/.planning/phases/18-payment-gateway/18-REVIEW-DISPOSITION.md +++ b/.planning/phases/18-payment-gateway/18-REVIEW-DISPOSITION.md @@ -2,10 +2,11 @@ phase: 18 review: 18-REVIEW.md findings: -open: 48 -total: 48 +open: 54 +fixed: 1 +total: 55 unparsed: 0 -recorded: 2026-10-13T16:01:01.576Z +recorded: 2026-10-13T22:34:31Z --- # Phase 18: Code Review Disposition @@ -14,8 +15,8 @@ Written by hand: the review uses part-prefixed IDs (A- for framework, pgstripe a | Finding | Severity | Disposition | Source | |---------|----------|-------------|--------| -| A-CR-01 | critical | open | `payment_intent.payment_failed` rejects a payment the customer can still complete, so a paid order stays Rejected | -| A-CR-02 | critical | open | A "paid" signal for a Rejected or Canceled payment is dropped silently (no log, no refund, Checkout Session left open after a cancel) | +| A-CR-01 | critical | fixed | `payment_intent.payment_failed` rejects a payment the customer can still complete, so a paid order stays Rejected. Fixed by plan 18-08, sm-pgstripe-plugin `0c4fde7` | +| A-CR-02 | critical | open | A "paid" signal for a Rejected or Canceled payment is dropped silently (no log, no refund, Checkout Session left open after a cancel). Partly resolved by plan 18-08 (`f5c10e8`: the refused paid signal is now an Error log line); still open: the Checkout Session is not expired on cancel, and there is no event, notice or refund | | A-CR-03 | critical | open | The admin money input reads a comma as the decimal separator in every locale, so `1,234` in `en` is saved as `1.234` | | A-WR-01 | warning | open | Webhook verification, refunds and Stripe lookups always use the currently active mode's keys; the payment does not record its own mode | | A-WR-02 | warning | open | Accepting an event does not check the event's amount, currency or session/intent id against the payment | @@ -27,6 +28,7 @@ Written by hand: the review uses part-prefixed IDs (A- for framework, pgstripe a | A-WR-08 | warning | open | A money field without `max` has no upper bound, so overflow is an opaque 500 instead of a 422 | | A-WR-09 | warning | open | The layer navigates to any `redirect_url` without checking the scheme | | A-WR-10 | warning | open | The layer's `formatMoney` throws on an unknown currency code and breaks the whole render | +| A-WR-11 | warning | open | A payment whose asynchronous Checkout payment is still processing is canceled by the sweep, and the later `async_payment_succeeded` is refused (18-08 re-review) | | A-IN-01 | info | open | An authentic event for a payment that is not yet visible is answered 200 "ignored", so Stripe never retries it | | A-IN-02 | info | open | `checkout.session.expired` is not handled | | A-IN-03 | info | open | Method-backed dropdowns dispatch on the field name and ignore the declared method; submitted values are not checked against the options | @@ -35,6 +37,13 @@ Written by hand: the review uses part-prefixed IDs (A- for framework, pgstripe a | A-IN-06 | info | open | Enabling the operator without a secret key turns every checkout into a 500 | | A-IN-07 | info | open | `readWinterFields` reports every read error as "File not found" | | A-IN-08 | info | open | `isZeroMoney` treats `""`, `"-"` and `"."` as zero | +| A-IN-09 | info | open | The decline Warn line says "the payment stays open for a retry" whatever the payment's state (18-08 re-review) | +| A-IN-10 | info | open | The refused-accept Error line is written per signal, and the success URL can repeat it (18-08 re-review) | +| A-IN-11 | info | open | The decline text is still logged on the purchase-time decline path (18-08 re-review) | +| A-IN-12 | info | open | `PaymentError.Type` is decoded and exported but nothing reads it (18-08 re-review) | +| A-IN-13 | info | open | `completedEvent` names the first payment's session and intent, and `paid_after_final_state` uses it for the third purchase (18-08 re-review) | +| A-IN-14 | info | open | Test-quality notes on the 18-08 tests (18-08 re-review) | +| A-IN-15 | info | open | The README and the settings page list different webhook events (18-08 re-review) | | B-CR-01 | critical | open | Guest access to an order is forgeable. `guest_code` is a deterministic hashids of the order id, and legacy rows used the public salt `KEIOS.PAYMENTGATEWAY` | | B-CR-02 | critical | open | The PaymentAccepted bridge writes `successful_payment_id` before checking `Can(Pay)`, overwriting the real successful payment on Paid, Canceled or Finished orders (PHP did not persist it) | | B-CR-03 | critical | open | `default_currency` can be changed at any time, which silently re-bases every stored exchange ratio and native amount |