docs(18): phase re-verification report after gap closure 18-08 (human_needed, 11/11)
This commit is contained in:
@@ -1,8 +1,8 @@
|
||||
---
|
||||
phase: 18-payment-gateway
|
||||
verified: 2026-10-13T16:08:51Z
|
||||
status: gaps_found
|
||||
score: 10/11 must-haves verified
|
||||
verified: 2026-10-13T22:41:19Z
|
||||
status: human_needed
|
||||
score: 11/11 must-haves verified
|
||||
covered_files:
|
||||
- .planning/phases/18-payment-gateway/18-01-PLAN.md
|
||||
- .planning/phases/18-payment-gateway/18-01-SUMMARY.md
|
||||
@@ -18,6 +18,8 @@ covered_files:
|
||||
- .planning/phases/18-payment-gateway/18-06-SUMMARY.md
|
||||
- .planning/phases/18-payment-gateway/18-07-PLAN.md
|
||||
- .planning/phases/18-payment-gateway/18-07-SUMMARY.md
|
||||
- .planning/phases/18-payment-gateway/18-08-PLAN.md
|
||||
- .planning/phases/18-payment-gateway/18-08-SUMMARY.md
|
||||
- admin/src/app/money.ts
|
||||
- admin/src/components/form/PreviewField.vue
|
||||
- admin/src/components/form/RecordActions.vue
|
||||
@@ -45,38 +47,42 @@ covered_files:
|
||||
- modules/cabana/settings_secret.go
|
||||
- modules/pact/capabilities.go
|
||||
- scripts/check-phase18.sh
|
||||
covered_digest: "v2:sha256:3f01e32ce16c66fd8767b087a9dd61f4d00e4c6d9f0d57ed0f45017f5b591bed"
|
||||
covered_digest: "v2:sha256:f3a47f626fb8473c48b3b0edd9882b5e3b8aa0d524950787e3c591f8a0ca43e7"
|
||||
sibling_heads: # outside the project root, so not in covered_digest; verified at these commits
|
||||
sm-paymentgateway-plugin: f55320a
|
||||
sm-pgstripe-plugin: d488834
|
||||
vue-paymentgateway-layer: ac2b960 (tag v0.1.0 is one test-only commit behind)
|
||||
summercms.go: a591932 (framework tag v0.1.25 at 2f5e8e3, pushed)
|
||||
sm-paymentgateway-plugin: f55320a (clean tree, unchanged since the previous verification)
|
||||
sm-pgstripe-plugin: 0313820 (main, equal to origin/main by ls-remote; no local or remote tag)
|
||||
vue-paymentgateway-layer: ac2b960 (unchanged; tag v0.1.0 is one test-only commit behind)
|
||||
summercms.go: e691b83 (only .planning changed since a591932; framework tag v0.1.25 at 2f5e8e3)
|
||||
behavior_unverified: 0
|
||||
overrides_applied: 0
|
||||
re_verification:
|
||||
previous_status: gaps_found
|
||||
previous_score: 10/11
|
||||
previous_verified: 2026-10-13T16:08:51Z
|
||||
gaps_closed:
|
||||
- "SC-2 / goal 'take payments': the Stripe operator takes a payment reliably through the notification route; as on PHP, a failed card attempt never makes the payment unpayable"
|
||||
gaps_remaining: []
|
||||
regressions: []
|
||||
decision_coverage:
|
||||
honored: 17
|
||||
total: 17
|
||||
not_honored: []
|
||||
gaps:
|
||||
- truth: "SC-2 / goal 'take payments': the Stripe operator takes a payment reliably through the notification route; as on PHP, a failed card attempt never makes the payment unpayable"
|
||||
status: failed
|
||||
reason: "webhook.go maps payment_intent.payment_failed to Reject. Stripe sends that event on every declined attempt while the PaymentIntent returns to requires_payment_method and the customer may retry on the same Checkout page or client secret. Reject is final (Accept only from Created/Sent), so the later checkout.session.completed / payment_intent.succeeded finds Can(Accept)=false and operator.accept returns nil with no log (A-CR-02). The customer is charged; the order stays AwaitingPayment and the cron cancels it. Checkout is the default mode, the README tells operators to subscribe to payment_intent.payment_failed, and the event_table subtest asserts a requires_payment_method intent becomes Rejected, so the defect is the documented production behaviour. The PHP Stripe operator never rejected from a notification (processNotification only accepts when isPaidInStripe && can(accept))."
|
||||
artifacts:
|
||||
- path: "../sm-pgstripe-plugin/webhook.go"
|
||||
issue: "lines 161-162: case EventPaymentIntentPaymentFailed -> o.reject (final state on a retryable signal)"
|
||||
- path: "../sm-pgstripe-plugin/operator.go"
|
||||
issue: "accept (about line 439): returns nil silently when Can(Accept) is false, without telling a replay (state Accepted) from a paid-at-Stripe payment in Rejected/Canceled"
|
||||
- path: "../sm-pgstripe-plugin/host_test.go"
|
||||
issue: "event_table (about line 905): asserts payment_intent.payment_failed with status requires_payment_method moves the payment to Rejected; the test encodes the defect"
|
||||
- path: "../sm-pgstripe-plugin/README.md"
|
||||
issue: "lines 33 and 62 document payment_failed: Reject and tell operators to subscribe to it"
|
||||
missing:
|
||||
- "Treat payment_intent.payment_failed as non-terminal: log it and leave the payment Sent (reject only on terminal signals such as checkout.session.async_payment_failed, and optionally checkout.session.expired / payment_intent.canceled)"
|
||||
- "In accept, when Can(Accept) is false and the state is not Accepted, log at Error level with payment id, event id and intent id (and optionally fire an event for a manual refund)"
|
||||
- "Change event_table to assert payment_failed leaves the payment Sent, and add a decline-then-succeed subtest to TestHostStripeFlow that ends Accepted / order Paid"
|
||||
- "Update the README event table"
|
||||
advisory: []
|
||||
advisory:
|
||||
- finding: "A-WR-11: in operator Checkout mode, a payment whose asynchronous (delayed-notification) method settles after payment_sent_to_canceled_timeout is canceled by pg:cancel-sent-payments, and the later checkout.session.async_payment_succeeded is refused (Error line 'refuses Accept'); the customer is charged for a canceled order"
|
||||
category: architectural
|
||||
reason: "Not a failed must-have: no success criterion, plan truth or item of the previous gap covers it; the sweep is the PHP sweep unchanged; the PHP Stripe operator had no hosted Checkout and never accepted from event content, so PHP would not have kept this payment; the Horoskopia flow (path b) records, sends and accepts in one transaction and never waits in Sent; the outcome is logged with every id; operators can raise the timeout (1 to 30 days) or limit the methods on the Stripe account. Resolve before an application runs operator Checkout mode on an account with delayed methods, and before the Horoskopia port drops its own Checkout code (D-16): mark the payment on completed-unpaid (which also restarts the sweep clock) and document the timeout rule in the README"
|
||||
evidence_status: "mechanism confirmed by reading (webhook.go:178-181, console/pg_cancel_sent_payments.go) and by passing subtests that show each half (event_table: completed-unpaid leaves the payment Sent with no write; paid_after_final_state part 1: a paid signal for a Canceled payment is refused and logged). No failing test exists: this is the behaviour plan decisions 3 to 5 chose. Stripe's settlement times are from memory, not checked against its documentation"
|
||||
- finding: "A-CR-02, remainder: nothing expires the Checkout Session or cancels the PaymentIntent when a payment is canceled; the Error log line is the only signal (no event, admin notice or refund); a customer cancel through the API leaves the Stripe payment Sent, so a later paid signal accepts a payment on a Canceled order (B-WR-06, B-CR-02)"
|
||||
category: architectural
|
||||
reason: "Not a failed must-have: the previous gap asked for the Error log line (the event was optional) and 18-08 delivers and proves it; 18-08's own must-have excludes a transition, event, data write or refund on that path; PHP was silent in the same situation and had no session to expire. The customer-cancel case lives in the unchanged gateway (f55320a), was a warning on the previous pass and is logged at Error level by the bridge (orders/bridge.go). Fix before an application exposes customer cancel together with an online operator"
|
||||
evidence_status: "none provided (reading only; no failing test)"
|
||||
human_verification:
|
||||
- test: "Live Stripe test mode, declined card then a successful retry. On an instance with both plugins mounted, test keys and the test signing secret stored on the pgstripe settings page, and the webhook endpoint reachable (public URL or `stripe listen --forward-to`): (a) Checkout mode: start a payment, pay with 4000 0000 0000 0002, then on the same Checkout page with 4242 4242 4242 4242; (b) PaymentIntent mode: the same two cards through the card form with the same client secret"
|
||||
expected: "After the decline: the payment stays Sent, the order AwaitingPayment, no rejected mail, one Warn line 'payment attempt failed' with code and decline_code and no decline text. After the retry: payment Accepted, order Paid, one OrderPaid and one paid mail. No 'refuses Accept' Error line"
|
||||
why_human: "External service. The proof host uses a fake Stripe and hand-built event bodies; that Stripe really sends payment_intent.payment_failed with the payment_uuid metadata on a Checkout decline, and the order of its events, can only be observed against Stripe"
|
||||
- test: "Read the README of sm-pgstripe-plugin: the Webhook events list, the paragraph on a paid signal that cannot be accepted, and step 2 of Stripe Dashboard (18-08 coverage item D6, human_judgment true)"
|
||||
expected: "The text matches the behaviour and is clear for an operator. Known inaccuracies to accept or have fixed: 'one Error log line' is one line per signal (A-IN-10); the settings page comment (lang webhook_comment, en and pl) still lists three events and omits checkout.session.async_payment_failed, the only event that rejects (A-IN-15)"
|
||||
why_human: "Accuracy against the code was checked; clarity for operators is a reading judgment"
|
||||
- test: "Mail wording: send each of the 27 templates (PL unsuffixed, EN -en) through the memory or log driver, or read views/mail/**.htm in sm-paymentgateway-plugin"
|
||||
expected: "Polish copy reads naturally and matches the PHP mails in meaning; variables (order code, totals, items, names, orderUrl) render; the bank transfer block is readable as plain lines"
|
||||
why_human: "Language quality and tone cannot be checked by grep or tests"
|
||||
@@ -92,28 +98,51 @@ human_verification:
|
||||
- test: "Horoskopia consumer parity: compare the acme.books fixture (sm-pgstripe-plugin/fixture_plugin_test.go) with horoscope BookApiController::purchase/pay and BookFulfilmentService::transitionToPaid, and decide whether the pgstripe operator will be enabled on Horoskopia after cutover"
|
||||
expected: "The fixture makes the same calls; the Stripe operator is enabled on Horoskopia (or RecordExternal is changed), because Go guards Send/Accept with the operator's enabled setting while PHP's guard was always true for a directly constructed StripeOperator. With the operator disabled, the fixture-style webhook skips Send/Accept and answers 200, leaving the payment Created and the order Placed with no error (B-WR-04)"
|
||||
why_human: "The real Horoskopia port is a later seed; whether its PHP settings have the Stripe operator enabled is production configuration, not code"
|
||||
- test: "Review the judgment-tier prohibitions table below (non-authoritative LLM verdicts) and accept or reject each"
|
||||
expected: "Every prohibition is accepted as held, or turned into a gap"
|
||||
- test: "Review the judgment-tier prohibitions table below (non-authoritative LLM verdicts, 19 rows including the four of plan 18-08) and accept or reject each"
|
||||
expected: "Every prohibition is accepted as held, or turned into a gap. Two need a decision: the push go-aheads cannot be checked from code, and 'logs never carry the decline message' holds for the webhook lines 18-08 added but not for the older purchase-time path (A-IN-11)"
|
||||
why_human: "Judgment-tier prohibitions need explicit human resolution in interactive verification"
|
||||
---
|
||||
|
||||
# Phase 18: Payment Gateway Verification Report
|
||||
|
||||
**Phase Goal:** `Golem15.PaymentGateway` and its Stripe operator run on SummerCMS as shared plugins whose contracts let Horoskopia's horoscope plugin create orders, take payments and react to payment events, as it does on PHP.
|
||||
**Verified:** 2026-10-13T16:08:51Z
|
||||
**Status:** gaps_found
|
||||
**Re-verification:** No, initial verification
|
||||
**Verified:** 2026-10-13T22:41:19Z
|
||||
**Status:** human_needed
|
||||
**Re-verification:** Yes, after gap closure 18-08 (previous report: gaps_found, 10/11, verified 2026-10-13T16:08:51Z)
|
||||
|
||||
**Mode note:** ROADMAP marks Phase 18 `mode: mvp`, but `user-story.validate` returns `valid: false` because the goal is not written as "As a ..., I want to ..., so that ...". The orchestrator asked for verification, so this report uses standard goal-backward verification and adds a User Flow Coverage table derived from the Horoskopia flow. To make the mode consistent, either run `/msd mvp-phase 18` to restate the goal or drop `mode: mvp`.
|
||||
**Mode note:** ROADMAP marks Phase 18 `mode: mvp`, but `user-story.validate` still returns `valid: false` because the goal is not written as "As a ..., I want to ..., so that ...". As in the first pass, this report uses standard goal-backward verification with a User Flow Coverage table derived from the Horoskopia flow. To make the mode consistent, either run `/msd mvp-phase 18` to restate the goal or drop `mode: mvp`.
|
||||
|
||||
**Scope of change since the previous pass:** only `sm-pgstripe-plugin` changed (`d488834..0313820`, four commits, seven files, +650/-33). `sm-paymentgateway-plugin` (`f55320a`), `vue-paymentgateway-layer` (`ac2b960`) and all non-planning files of `summercms.go` are byte-identical to what the previous pass verified (`git log --since` and `git diff a591932..HEAD -- . ':!.planning'` are empty). The full gate was still re-run for every truth.
|
||||
|
||||
## Re-verification of the previous gap
|
||||
|
||||
| Missing item of gap 1 | State now | Evidence |
|
||||
|---|---|---|
|
||||
| 1. `payment_intent.payment_failed` is non-terminal; reject only on terminal signals | closed | `webhook.go:189-191` calls `attemptFailed` (log only, `webhook.go:200-216`) and returns nil. `o.reject(` has one call site left, `checkout.session.async_payment_failed` (`webhook.go:185-186`). `checkout.session.expired` and `payment_intent.canceled` are 200 no-ops by plan decision 3 (the gap text made them optional) |
|
||||
| 2. Error log when a paid signal finds a payment that refuses Accept and was never accepted | closed | `operator.go` `accept` calls `refusedAccept`; silent for Accepted and Refunded, Error line `refuses Accept` with payment, uuid, state, event or `browser_return`, intent and session otherwise. The optional event was not added, by plan decision 5 |
|
||||
| 3. `event_table` asserts Sent; a decline-then-succeed subtest ends Accepted and Paid | closed | `host_test.go`: `event_table` asserts payment Sent and order AwaitingPayment for a `requires_payment_method` intent; `decline_then_succeed` (Checkout) and `decline_then_succeed_payment_intent` end in `assertPaid` (payment Accepted, order Paid, `successful_payment_id`, one OrderPaid, one paid mail). All passed in my run |
|
||||
| 4. README event table | closed | `README.md:30-41` and `:68` list the mapping as implemented; `PaymentError` is in the API reference (`:84`) |
|
||||
|
||||
Truths of the gap-closure plan 18-08 (all seven are part of truth 3 below):
|
||||
|
||||
| # | 18-08 truth (short) | Status | Evidence |
|
||||
|---|---|---|---|
|
||||
| 08-1 | Signed decline for a Sent payment: 200, nothing changes (state, updated_at, events, mail), one Warn line with ids and codes, never the message or body | ✓ VERIFIED | `decline_then_succeed` compares a snapshot (payment and order state, both updated_at, OrderPaid count, mail count) before and after, and counts PaymentRejected (0). `TestStripeOperator/a_declined_attempt_is_logged_and_never_rejected` asserts exactly one Warn record with payment, uuid, event, intent, code, decline_code, no Error record and no decline text in any message or attribute. `PaymentError` has no message field (`json:"message"` count in webhook.go: 0) |
|
||||
| 08-2 | Decline then succeed pays the order in both modes; a late redelivery of the decline changes nothing | ✓ VERIFIED | both subtests passed; the late decline is re-posted after acceptance and the snapshot is unchanged |
|
||||
| 08-3 | `async_payment_failed` rejects; expired and canceled stay Sent | ✓ VERIFIED | `event_table` and `TestStripeOperator/expired_sessions_and_canceled_intents_change_nothing` passed |
|
||||
| 08-4 | Refused paid signal: nothing changes, 200 or 303 as before, one Error line; Accepted and Refunded silent | ✓ VERIFIED | `paid_after_final_state`: Canceled (event), Rejected (event), Canceled (browser return, 303), then Accepted and Refunded replays; exactly three Error records with the expected attributes. The branch "Created or Sent with the operator's guard refusing" has no test of its own (A-IN-14); it is the same statement as the tested states, so this is info |
|
||||
| 08-5 | Payment machine, gateway and purchase-time mapping unchanged | ✓ VERIFIED | `sm-paymentgateway-plugin` is at `f55320a` with a clean tree; Accept stays `From: Created, Sent` (`payments/machine.go:48-49`); `refusedAccept` has no write, transition or Stripe call; `go.mod` and `go.sum` of pgstripe untouched |
|
||||
| 08-6 | README documents the mapping and `PaymentError` | ✓ VERIFIED (wording is human item 2) | read against `handleEvent`; consistent |
|
||||
| 08-7 | vet and `-short` green, 17 subtests, gate stages `plugins` and `postgres`, pushed after the reply, no tag | ✓ VERIFIED (the reply itself is not verifiable from code) | run by me, see Behavioral Spot-Checks; `ls-remote origin refs/heads/main` = `0313820f...`; 0 local and 0 remote tags |
|
||||
|
||||
## User Flow Coverage (derived; the goal is not a user story)
|
||||
|
||||
| Step | Expected | Evidence | Status |
|
||||
|------|----------|----------|--------|
|
||||
| Create order | Consumer builds Item/Cart/Details and calls `orders.Service.Make`, gets a Placed order with `hash_id` | `orders/service.go` Make; `fixture_plugin_test.go` MakeOrder; demo lists 6 orders | VERIFIED |
|
||||
| Take payment, path (b) (Horoskopia today) | Own Checkout Session plus own webhook, then `RecordExternal`, `Send`, `Accept`; order Paid | `TestHostStripeFlow/consumer_driven_path_b` passed in this run | VERIFIED (with the operator enabled; see human item 5) |
|
||||
| Take payment, path (a) (operator, D-16) | Operator Checkout/PaymentIntent, signed webhook, Accepted, Paid | `checkout_operator_path` and `payment_intent_operator_path` passed. A declined attempt followed by a successful retry ends Rejected with the money captured | FAILED (gap 1) |
|
||||
| React to payment events | One `OrderPaid` event, paid mail after commit, replay is a no-op | `assertPaid` and `replay_and_race` in TestHostStripeFlow; TestMailTracer | VERIFIED |
|
||||
| Create order | Consumer builds Item/Cart/Details and calls `orders.Service.Make`, gets a Placed order with `hash_id` | `orders/service.go` Make; `fixture_plugin_test.go` MakeOrder | VERIFIED |
|
||||
| Take payment, path (b) (Horoskopia today) | Own Checkout Session plus own webhook, then `RecordExternal`, `Send`, `Accept` in one transaction; order Paid | `TestHostStripeFlow/consumer_driven_path_b` passed in this run | VERIFIED (with the operator enabled; human item 7) |
|
||||
| Take payment, path (a) (operator, D-16) | Operator Checkout/PaymentIntent, signed webhook, Accepted, Paid; a declined attempt followed by a retry is paid | `checkout_operator_path`, `payment_intent_operator_path`, `decline_then_succeed`, `decline_then_succeed_payment_intent` passed in this run | VERIFIED against the fake Stripe; live confirmation is human item 1 |
|
||||
| React to payment events | One `OrderPaid` event, paid mail after commit, replay is a no-op | `assertPaid` and `replay_and_race` in TestHostStripeFlow; TestMailTracer in the gate's postgres stage | VERIFIED |
|
||||
|
||||
## Goal Achievement
|
||||
|
||||
@@ -121,75 +150,131 @@ human_verification:
|
||||
|
||||
| # | Truth | Status | Evidence |
|
||||
|---|-------|--------|----------|
|
||||
| 1 | SC-1: the Payment machine (6 states) and Order machine (12 states, 19 transitions, all refund paths) port the PHP transitions, and money is exact decimal | ✓ VERIFIED | Diffed `orders/machine.go` and `payments/machine.go` line by line against PHP `core/Order.php` and `core/Operator.php` stateMachineConfig: from-sets, targets, initial and final states, the Ship guard (`isShippingEnabled`) and enabled guards (Cancel unguarded) all match. State strings match the PHP constants. `TestOrderMatrix` (228 pairs) and `TestPaymentMatrix` (30 pairs) check against an independent hand-copied PHP table, not the machine itself. Money is `Amount4` int64×10⁴ with an exact Scan/Value; the only `float64` in production is a display switch for JSON detail values (`controllers/orders_partials.go:382`), not money. TestMoneyVectors, TestAmount4 and TestRefundBridge passed in this run. Warnings: B-CR-02, B-WR-01 |
|
||||
| 2 | SC-2: operators register at build time through a Go interface that replaces `core/Operator` and the `paymentgateway.booted` push | ✓ VERIFIED | `operators.HasOperators`; `boot.go` scans `party.Activated` and publishes `*operators.Registry`; a duplicate code fails (`ErrDuplicateCode`, TestOperatorRegistry); `sm-pgstripe-plugin/plugin.go` has `PaymentOperators()` and `Requires golem15.paymentgateway` |
|
||||
| 3 | SC-2 / goal: Cash, manual transfer and Stripe operators are implemented, and the Stripe operator takes payments without losing one that PHP would have kept | ✗ FAILED | Cash, ManualTransfer and Disabled ship with the PHP codes. The Stripe operator works on the happy path, but `payment_intent.payment_failed` moves the payment to Rejected, a final state, on a retryable decline, and a later success is dropped silently (A-CR-01 with A-CR-02). See Gaps |
|
||||
| 4 | SC-2: the `_paymentgateway/{operator}/{mode?}/{result?}` route keeps its PHP behaviour | ✓ VERIFIED | `routes.go`: 3 patterns × GET/POST/PUT/PATCH/DELETE under the notify throttle only; `payments/notification.go` caps the body at 1 MiB, runs `VerifyNotification` before `ExtractUUID`/`FindByUUID`, and only registered and enabled operators resolve. TestNotificationRoute, TestNotificationSecurity and TestNotificationRouteSmoke passed |
|
||||
| 5 | SC-3: the Order/Payment events, their mail templates, the admin screens (orders, payments, settings) and the scheduled cancel/finish/update commands are ported, and contracts (tables, event names, config keys) stay non-breaking | ✓ VERIFIED | 24 events named with the namespace `Golem15\PaymentGateway\Events\` plus the PHP class names (set-equal to PHP `events/`). 55 `.htm` mail files (27 codes × PL/EN plus the layout). `schedule.go` has the same five commands and cadences as PHP `registerSchedule`; `pg:update-orders`/`pg:update-metadata` (unregistered in PHP) are documented non-ports. Table names equal the PHP `updates/` set plus the settings singletons (PHP `system_settings` keys are mapped by `import-winter`). The demo admin serves orders (localized state labels), payments, shippings, quotes, quote items and the three settings pages; pgstripe secrets show only `meta.secrets` states. Admin, mail and command suites passed on Postgres |
|
||||
| 6 | SC-4: a host test proves the order-create, Stripe-pay and order-paid flow Horoskopia relies on | ✓ VERIFIED | `TestHostStripeFlow` (14 subtests: both pay paths, wrong secret, missing header or secret, older api_version, replay and race, browser return, refund) passed on testcontainers Postgres in this run. The happy-path flow is proven; the decline-retry defect is truth 3. Path (b) runs with the operator enabled (human item 5) |
|
||||
| 7 | SC-5: the new code has unit tests, delivered in the last plan | ✓ VERIFIED | `PHASE18_REQUIRE_POSTGRES=1 bash scripts/check-phase18.sh --all` exited 0: self-test, go, plugins, admin (1377 vitest), layers (69 TAP, 0 fail), docs (docs checker, OpenAPI, dist match), postgres (9 + 22 + 3 suites). Framework `go vet ./...` and `go test -short ./...` are green (run once in this verification). The 72/74 PHP test map is recorded in 18-VALIDATION.md |
|
||||
| 8 | D-07/D-13 framework: cabana `money` field and column (exact decimal string, 422 keys, no rounding), write-only `lagoon.Encrypted` settings secrets with env fallback, richeditor and tabs on settings, danger actions, preview Edit rule, table kit | ✓ VERIFIED | `field_money.go`, `settings_secret.go`, `registry.ts` maps `money` to MoneyField. TestMoneyField*, TestSettingsSecret*, TestSettingsTabs, TestRecordActionDanger and the Postgres smokes passed; v0.1.25 is tagged and pushed (ls-remote). Warning: A-CR-03 |
|
||||
| 9 | D-11: v2 rows carry `"v": 2`; the permanent legacy JMS reader decodes golden fixtures; the typed extension registry replaces setCartClass/setDetailsClass | ✓ VERIFIED (with warning) | TestLegacyCartGoldens, TestLegacyFixtures and TestCartRegistry passed. Warning: `cart.RegisterDetailsParams` stores types that nothing reads (recorded in deferred-items.md). No PHP consumer in the reference trees uses setDetailsClass, so no stated criterion depends on it |
|
||||
| 10 | D-06/D-17: the customer order API `/_paymentgateway/api/v1` (12 endpoints, owner / guest_code / guest_token authz, 404 for forbidden, `process` returns `{redirect_url}` or the requires_action bundle) and `vue-paymentgateway-layer` | ✓ VERIFIED (with warnings) | `api/*.go`; TestCustomerAPI, TestCustomerAPITracer and TestCustomerAPIAuthz passed; layer tag v0.1.0 pushed and 69 layer tests passed, including the checkout and orderPage backstops. Warnings: B-CR-01, A-WR-09, B-WR-03, B-WR-06 |
|
||||
| 11 | D-08: settings pages and mail: the PG and Mailer settings pages, 27 templates under the PHP codes, four mailer groups sent after commit | ✓ VERIFIED | TestPGSettingsSmoke, TestMailerSettingsSmoke, TestMailTracer (rollback sends nothing) and TestMailersMatrixSmoke passed; TestMailCatalog checks every registered name has a file. Wording is a human item |
|
||||
| 1 | SC-1: the Payment machine (6 states) and Order machine (12 states, 19 transitions, all refund paths) port the PHP transitions, and money is exact decimal | ✓ VERIFIED | Regression check: gateway unchanged at `f55320a`; `orders/machine.go`, `payments/machine.go`, `money/*.go`, `fsm/fsm.go` present; the only `float64` in production code is still the display switch at `controllers/orders_partials.go:382`. The gate's `plugins` stage re-ran the money, fsm, orders and payments suites by name. Warnings carried: B-CR-02, B-WR-01 |
|
||||
| 2 | SC-2: operators register at build time through a Go interface that replaces `core/Operator` and the `paymentgateway.booted` push | ✓ VERIFIED | `operators.HasOperators` (`operators/operator.go:73`), `boot.go:96-111` collects it from activated plugins; `sm-pgstripe-plugin/plugin.go:55,69` (`Requires golem15.paymentgateway`, `PaymentOperators`) |
|
||||
| 3 | SC-2 / goal: Cash, manual transfer and Stripe operators are implemented, and the Stripe operator takes payments without losing one that PHP would have kept | ✓ VERIFIED (was FAILED) | Cash, ManualTransfer and Disabled unchanged. The failed mapping is corrected: a declined attempt is logged and leaves the payment Sent, the retry is accepted in both modes, and a paid signal that cannot be accepted is an Error line instead of a silent drop. Behavioural proof run by me: TestHostStripeFlow 17/17 subtests on Postgres. Details in the 18-08 table above. Proof is against a fake Stripe (human item 1). Remaining review findings are judged below |
|
||||
| 4 | SC-2: the `_paymentgateway/{operator}/{mode?}/{result?}` route keeps its PHP behaviour | ✓ VERIFIED | `routes.go:16-18,36-40` (three patterns under the notify throttle); `payments/notification.go` caps the body (`MaxNotificationBytes`), runs `VerifyNotification` (`:97`) before `ExtractUUID` (`:104`) and `FindByUUID` (`:127`). TestNotificationRouteSmoke passed in the gate; `wrong_secret`, `missing_header`, `missing_secret` passed in TestHostStripeFlow |
|
||||
| 5 | SC-3: the Order/Payment events, their mail templates, the admin screens (orders, payments, settings) and the scheduled cancel/finish/update commands are ported, and contracts (tables, event names, config keys) stay non-breaking | ✓ VERIFIED | Unchanged code. 55 `.htm` mail files, `schedule.go:17-21` (five commands, PHP cadences), controllers for orders, payments, shippings, quotes, quoteables. Admin, mail, command, migration and import suites (22 top-level tests) passed in the gate's postgres stage |
|
||||
| 6 | SC-4: a host test proves the order-create, Stripe-pay and order-paid flow Horoskopia relies on | ✓ VERIFIED | `TestHostStripeFlow` passed with 17 subtests, 0 skipped, on testcontainers Postgres in my run (both pay paths, security cases, replay and race, browser return, refund, the event table, the two decline-then-succeed proofs and paid_after_final_state) |
|
||||
| 7 | SC-5: the new code has unit tests, delivered in the last plan | ✓ VERIFIED | `PHASE18_REQUIRE_POSTGRES=1 bash scripts/check-phase18.sh --all` exited 0 in this run: self-test, go, plugins, admin, layers (69 TAP, 0 fail), docs (docs checker, OpenAPI, dist match, phase 10 hygiene), postgres (9 + 22 + 3). 18-08 is a gap-closure plan that carries its own tests, so tests are still the last thing delivered |
|
||||
| 8 | D-07/D-13 framework: cabana `money` field and column, write-only `lagoon.Encrypted` settings secrets with env fallback, richeditor and tabs on settings, danger actions, preview Edit rule, table kit | ✓ VERIFIED | Unchanged files (covered_digest lists them); `registry.ts:95` maps `money`; the gate's go, admin and postgres stages passed; tag v0.1.25 at `2f5e8e3`. Warning carried: A-CR-03 |
|
||||
| 9 | D-11: v2 rows carry `"v": 2`; the permanent legacy JMS reader decodes golden fixtures; the typed extension registry replaces setCartClass/setDetailsClass | ✓ VERIFIED (with warning) | Unchanged; cart suites passed in the `plugins` stage. Warning carried: `cart.RegisterDetailsParams` has no reader (deferred-items.md) |
|
||||
| 10 | D-06/D-17: the customer order API `/_paymentgateway/api/v1` and `vue-paymentgateway-layer` | ✓ VERIFIED (with warnings) | Unchanged; TestCustomerAPI and TestCustomerAPITracer passed in the postgres stage, the API suites in `plugins`, 69 layer tests in `layers`. Warnings carried: B-CR-01, A-WR-09, B-WR-03, B-WR-06 |
|
||||
| 11 | D-08: settings pages and mail: the PG and Mailer settings pages, 27 templates under the PHP codes, four mailer groups sent after commit | ✓ VERIFIED | Unchanged; TestPGSettingsSmoke, TestMailerSettingsSmoke, TestMailTracer and TestMailersMatrixSmoke passed in the postgres stage. Wording is human item 3 |
|
||||
|
||||
**Score:** 10/11 truths verified (0 present but behavior-unverified). The behaviour-dependent truths (replay and race idempotency, after-commit mail, transaction rollback) are covered by named tests that passed in this run.
|
||||
**Score:** 11/11 truths verified (0 present but behavior-unverified). The behaviour-dependent truths (the non-terminal decline, decline then succeed, the refused paid signal, replay and race idempotency, after-commit mail) are each exercised by a named test that passed in this run.
|
||||
|
||||
### Re-review findings judged against the must-haves
|
||||
|
||||
The orchestrator asked for a ruling on the new findings of the 18-08 re-review. Each was checked in code.
|
||||
|
||||
**A-WR-11 (asynchronous Checkout payment canceled by the sweep, later success refused): follow-up work, not a failed must-have.**
|
||||
|
||||
Confirmed by reading: `checkout.session.completed` with `payment_status` unpaid returns nil and writes nothing (`webhook.go:178-181`, unchanged from `d488834`), so `updated_at` stays at purchase time; `pg:cancel-sent-payments` cancels every Sent payment whose `updated_at` is older than `payment_sent_to_canceled_timeout` days (default 7, rule 1 to 30); `purchaseCheckout` sets no `payment_method_types`; `checkout_mode` defaults to `checkout`. A delayed method that settles after the timeout ends as a charged customer, a Canceled payment and order, and the `refuses Accept` Error line.
|
||||
|
||||
Why it is not a failed must-have:
|
||||
|
||||
1. **No contract item covers it.** None of the five success criteria, no plan truth and no "missing" item of the previous gap mentions delayed-notification methods. D-16 asks that the handler accepts `checkout.session.async_payment_succeeded`, which it does (`event_table`).
|
||||
2. **It is not a loss relative to PHP.** The PHP Stripe operator had no hosted Checkout, confirmed card PaymentIntents only, and its `processNotification` accepted only when `isPaidInStripe` had been set at purchase. PHP would not have taken this payment at all. The sweep is the PHP sweep unchanged (`CancelSentPayments.php`: state Sent and `updated_at < deadline`).
|
||||
3. **The Horoskopia flow does not pass through it.** Path (b) records, sends and accepts the payment in one transaction when the consumer's own webhook reports it paid (`fixture_plugin_test.go:265-282`), so the payment never waits in Sent. Horoskopia's PHP webhook handles `async_payment_succeeded` itself.
|
||||
4. **It needs a non-default combination and has an operator-side remedy.** A delayed method must be enabled on the Stripe account and settle later than the timeout; the timeout can be raised to 30 days and the methods limited in the Dashboard.
|
||||
5. **It is no longer silent.** The outcome is an Error line with every id (proven by `paid_after_final_state` part 1, the same state combination).
|
||||
|
||||
Compare with the gap that did block: every card decline followed by a retry, in the default mode, on the event the README told operators to subscribe to, with no log line, and new relative to PHP.
|
||||
|
||||
It should not be left open for long. It becomes goal-relevant the moment an application runs operator Checkout mode on an account with delayed methods, and D-16 exists so the Horoskopia port can drop its own Checkout code. Recommended as a quick task before the first tag of `sm-pgstripe-plugin`: store a marker on completed-unpaid (which also restarts the sweep clock) and add the timeout rule to the README. If the developer reads "take payments" as covering delayed methods in operator Checkout mode now, this converts to a gap:
|
||||
|
||||
```yaml
|
||||
gaps:
|
||||
- truth: "SC-2 / D-16: an asynchronous Checkout payment that settles after payment_sent_to_canceled_timeout is still accepted"
|
||||
status: failed
|
||||
reason: "completed-unpaid records nothing; pg:cancel-sent-payments cancels the payment; async_payment_succeeded is then refused (A-WR-11)"
|
||||
artifacts:
|
||||
- path: "../sm-pgstripe-plugin/webhook.go"
|
||||
issue: "lines 178-181: completed with payment_status unpaid returns nil without a marker or an updated_at change"
|
||||
missing:
|
||||
- "Record an async-pending marker with SaveData when a Sent payment's session completes unpaid"
|
||||
- "README: payment_sent_to_canceled_timeout must exceed the settlement time of the delayed methods enabled on the account"
|
||||
```
|
||||
|
||||
**A-CR-02, remaining part: follow-up work, not a failed must-have.**
|
||||
|
||||
The previous gap asked for an Error log line when `Can(Accept)` is false and the payment was never accepted, with the event marked optional. That is delivered and proven. 18-08's own must-have states that the path adds no transition, event, data write or refund. What remains:
|
||||
|
||||
1. Nothing expires the Checkout Session or cancels the PaymentIntent when a payment is canceled (no `V1CheckoutSessions.Expire`, no PaymentCanceled listener). PHP had no session to expire and was silent in the same situation.
|
||||
2. The log line is the only signal, written per signal (A-IN-10).
|
||||
3. `POST /orders/{hash}/cancel` skips payments of online operators (`api/actions.go`, confirmed), so the order is Canceled while the Stripe payment stays Sent; a later paid signal accepts the payment, the bridge writes `successful_payment_id` and logs `order cannot accept the payment` at Error level (`orders/bridge.go`, confirmed). This is B-WR-06 with B-CR-02, in the unchanged gateway. It was a warning on the previous pass; under the re-verification evidence gate a finding on an unmodified file that was not a gap needs a failing test to block, and there is none.
|
||||
4. A replay is inferred from the state alone (closing it is the id check of A-WR-02).
|
||||
|
||||
Item 3 is the one with money at risk on a new public API (D-06). Fix it before an application exposes customer cancel together with an online operator.
|
||||
|
||||
**Other new findings (A-IN-09 to A-IN-15): info.** Confirmed where checked: the Warn line has no `state` attribute (A-IN-09); `PaymentError.Type` is decoded and unused (A-IN-12); `completedEvent` hardcodes `cs_test_1`/`pi_test_1` (A-IN-13); the settings comment omits `checkout.session.async_payment_failed` (A-IN-15, in human item 2); the purchase-time decline still logs Stripe's message through `payments/service.go:285` (A-IN-11, in the prohibitions table).
|
||||
|
||||
### Advisory (New Scope, Unevidenced)
|
||||
|
||||
New-scope findings with no deterministic failing evidence: reported, not blocking, and they do not revert the closed gap.
|
||||
|
||||
| # | Finding | Category | Why Advisory |
|
||||
|---|---------|----------|--------------|
|
||||
| 1 | A-WR-11: delayed Checkout method settles after the sweep's timeout; the success is refused | architectural | Not a must-have (see above). `webhook.go` was modified by 18-08, so a blocker on it would need no further evidence; it is classified a warning because it does not prevent the goal. The mechanism is confirmed by reading and by passing tests of each half; no failing test |
|
||||
| 2 | A-CR-02 remainder: no session expiry on cancel, log-only signal, customer cancel leaves the Stripe payment Sent | architectural | Outside the previous gap's missing items; item 3 is in an unmodified file and has no failing test |
|
||||
|
||||
### Required Artifacts
|
||||
|
||||
| Artifact | Expected | Status | Details |
|
||||
|----------|----------|--------|---------|
|
||||
| `modules/cabana/field_money.go` | money field and column compile, validation | ✓ VERIFIED | wired via `compileMoneyKeys` in form_schema.go |
|
||||
| `modules/cabana/settings_secret.go` | write-only secrets, SecretFallbacks | ✓ VERIFIED | used by pgstripe settings; demo shows `meta.secrets` |
|
||||
| `admin/src/components/form/fields/MoneyField.vue`, `admin/src/app/money.ts` | decimal input, parse, format | ✓ VERIFIED (warning A-CR-03) | registered in registry.ts |
|
||||
| `modules/pact/capabilities.go` | `AdminRecordAction.Danger` | ✓ VERIFIED | |
|
||||
| `../sm-paymentgateway-plugin/money/{money,amount}.go` | exact money | ✓ VERIFIED | |
|
||||
| `../sm-paymentgateway-plugin/fsm/fsm.go`, `orders/machine.go`, `payments/machine.go` | PHP machines | ✓ VERIFIED | equal to PHP |
|
||||
| `../sm-paymentgateway-plugin/operators/operator.go`, `registry.go` | Operator, HasOperators, Registry | ✓ VERIFIED | |
|
||||
| `../sm-paymentgateway-plugin/orders/service.go`, `orders/bridge.go` | services and bridge | ✓ VERIFIED (warning B-CR-02) | |
|
||||
| `../sm-paymentgateway-plugin/payments/service.go` incl. `RecordExternal` | payments API | ✓ VERIFIED (warning B-WR-04) | |
|
||||
| `../sm-paymentgateway-plugin/cart/legacy.go` | legacy JMS reader | ✓ VERIFIED | `RegisterDetailsParams` is hollow (warning) |
|
||||
| `../sm-paymentgateway-plugin/routes.go` | notification route and API | ✓ VERIFIED | |
|
||||
| `../sm-paymentgateway-plugin/controllers/*`, `assets/js/quote-cart.js`, `lang/*` | admin | ✓ VERIFIED | serves on the demo |
|
||||
| `../sm-paymentgateway-plugin/mail.go`, `events/mailers.go`, `views/mail/**` | mail | ✓ VERIFIED | |
|
||||
| `../sm-pgstripe-plugin/operator.go`, `webhook.go` | Stripe operator | ✗ DEFECT | exists and is wired; event mapping defect (gap 1) |
|
||||
| `../sm-pgstripe-plugin/client.go`, `models/settings.go`, `console/import_winter.go` | client seam, settings, import | ✓ VERIFIED | |
|
||||
| `../sm-pgstripe-plugin/host_test.go` | D-03/D-04 proof host | ✓ VERIFIED | encodes the gap-1 behaviour in event_table |
|
||||
| `../vue-paymentgateway-layer/app/**` | layer | ✓ VERIFIED | |
|
||||
| `../sm-pgstripe-plugin/webhook.go` | non-terminal decline branch, `PaymentError` decode | ✓ VERIFIED | `last_payment_error` decoded; `attemptFailed`; wired from `handleEvent`; was DEFECT |
|
||||
| `../sm-pgstripe-plugin/operator.go` | `accept` tells a replay from a refused paid payment | ✓ VERIFIED | `refuses Accept` in `refusedAccept`, called from `accept` |
|
||||
| `../sm-pgstripe-plugin/browser_return.go` | browser return passes its trigger | ✓ VERIFIED | `browserReturnTrigger` passed at `:71` |
|
||||
| `../sm-pgstripe-plugin/host_test.go` | D-03/D-04 proof host with the decline and refused-accept proofs | ✓ VERIFIED | 17 subtests passed; no longer encodes the defect |
|
||||
| `../sm-pgstripe-plugin/operator_test.go`, `webhook_test.go` | Docker-free decline and decode coverage | ✓ VERIFIED | three new subtests passed |
|
||||
| `../sm-pgstripe-plugin/README.md` | corrected event table and Dashboard events | ✓ VERIFIED | wording is human item 2 |
|
||||
| `../sm-pgstripe-plugin/client.go`, `models/settings.go`, `console/import_winter.go` | client seam, settings, import | ✓ VERIFIED | unchanged; settings and import smokes passed in the gate |
|
||||
| `modules/cabana/field_money.go`, `settings_secret.go`, `modules/pact/capabilities.go`, `admin/src/**` (covered files) | framework money field, secrets, danger actions | ✓ VERIFIED | unchanged since the previous pass; gate stages go, admin, docs, postgres passed |
|
||||
| `../sm-paymentgateway-plugin/**` (money, fsm, machines, operators, services, bridge, cart, routes, api, controllers, mail, console) | gateway | ✓ VERIFIED | unchanged at `f55320a`, clean tree; gate stages plugins and postgres passed. Warnings carried: B-CR-02, B-WR-04, hollow `RegisterDetailsParams` |
|
||||
| `../vue-paymentgateway-layer/app/**` | layer | ✓ VERIFIED | unchanged at `ac2b960`; 69 tests passed |
|
||||
| `scripts/check-phase18.sh` | fail-closed gate | ✓ VERIFIED | self-test passed; `--all` green with Postgres required |
|
||||
| `.planning/notes/2026-10-13-paymentgateway-hashids-go-hashids.md` | decision note | ✓ VERIFIED | |
|
||||
|
||||
`verify.artifacts` on 18-08-PLAN.md: 6/6 passed.
|
||||
|
||||
### Key Link Verification
|
||||
|
||||
`verify.key-links` rejects the 18-08 links because their sources are outside the project directory, so they were checked by grep.
|
||||
|
||||
| From | To | Via | Status |
|
||||
|------|----|-----|--------|
|
||||
| cabana form_schema.go | field_money.go | `compileMoneyKeys` | WIRED |
|
||||
| pgstripe webhook.go | operator.go | `o.accept(ctx, pc, ev.ID` (3 call sites) | WIRED |
|
||||
| pgstripe browser_return.go | operator.go | `o.accept(ctx, pc, browserReturnTrigger, extra)` | WIRED |
|
||||
| pgstripe host_test.go | gateway payments/notification.go | signed POSTs of `EventPaymentIntentPaymentFailed` and the retry to the notification route | WIRED (tests ran) |
|
||||
| pgstripe operator.go | gateway payments/machine.go | `svc.Can(Accept)`, then the state picks the silent or the Error branch (`PaymentStateRefunded`) | WIRED |
|
||||
| cabana form_schema.go | field_money.go | `compileMoneyKeys` | WIRED (unchanged) |
|
||||
| admin registry.ts | MoneyField.vue | `['money', MoneyField]` | WIRED |
|
||||
| paymentgateway boot.go | operators/registry.go | `party.Activated` scan for `HasOperators`, `app.Publish` | WIRED |
|
||||
| orders/bridge.go | events | `app.Events.Listen` on the five payment events | WIRED |
|
||||
| paymentgateway boot.go | operators/registry.go | scan for `HasOperators` | WIRED |
|
||||
| payments/notification.go | operator | `VerifyNotification` before `ExtractUUID`/`FindByUUID` | WIRED |
|
||||
| pgstripe plugin.go | paymentgateway registry | `PaymentOperators()` | WIRED |
|
||||
| pgstripe operator.go | webhook.go | `ValidatePayload` on the raw body | WIRED |
|
||||
| host_test.go | notification route | signed POST to `/_paymentgateway/Z29sZW0x...` | WIRED |
|
||||
| routes.go | api/*.go | `r.Group(APIPrefix, throttle)` | WIRED |
|
||||
| layer useOrder.ts | client.ts | `createPaymentGatewayClient` | WIRED |
|
||||
| events/mailers.go | postcard | `postcard.Message` via `lagoon.AfterCommit` | WIRED |
|
||||
| orders/bridge.go, events/mailers.go, layer useOrder.ts | events, postcard, client.ts | as in the previous pass | WIRED (unchanged code; suites passed) |
|
||||
|
||||
### Data-Flow Trace (Level 4)
|
||||
|
||||
| Artifact | Data | Source | Real data | Status |
|
||||
|----------|------|--------|-----------|--------|
|
||||
| Orders/Payments admin lists | rows, money columns | GORM over `golem15_paymentgateway_*` | demo returns 6 orders and 4 payments with exact `"151.4300"` strings | ✓ FLOWING |
|
||||
| pgstripe settings GET | secret states | `settings_secret.go` projection | `meta.secrets` states, no values | ✓ FLOWING |
|
||||
| Stripe amount | `unit_amount` | `TotalGross().Minor()` from the stored cart | 4900 asserted in the host test | ✓ FLOWING |
|
||||
| Decline Warn line | code, decline_code | `ev.Data.Object.LastPaymentError` decoded from the signed body | asserted `card_declined` / `insufficient_funds` in the unit test | ✓ FLOWING |
|
||||
| Refused-accept Error line | state, intent, session | locked payment row and the event | asserted per record in `paid_after_final_state` | ✓ FLOWING |
|
||||
| Stripe amount | `unit_amount` | `TotalGross().Minor()` from the stored cart | asserted in `checkout_operator_path` (passed) | ✓ FLOWING |
|
||||
| Orders/Payments admin lists, pgstripe settings secrets | rows, secret states | GORM, `settings_secret.go` | admin and settings smokes passed in the gate; the demo instance was not queried again | ✓ FLOWING (by test) |
|
||||
|
||||
### Behavioral Spot-Checks
|
||||
|
||||
All run by the verifier at the heads listed in the frontmatter.
|
||||
|
||||
| Behavior | Command | Result | Status |
|
||||
|----------|---------|--------|--------|
|
||||
| Full phase gate with Postgres | `PHASE18_REQUIRE_POSTGRES=1 bash scripts/check-phase18.sh --all` | all 7 stages PASS, exit 0 | ✓ PASS |
|
||||
| Framework regressions | `go vet ./... && go test -short -count=1 ./...` | exit 0 | ✓ PASS |
|
||||
| Admin screens live | demo `/backend/api/v1/golem15/paymentgateway/{orders,payments,shippings,quotes,quoteables}`, `/settings/{payment_gateway,payment_gateway_mailer,pgstripe}` (GET only) | real rows; localized states; secrets redacted | ✓ PASS |
|
||||
| Full phase gate, Postgres required | `PHASE18_REQUIRE_POSTGRES=1 bash scripts/check-phase18.sh --all` | self-test, go, plugins, admin, layers, docs, postgres all PASS; "Phase 18 gate passed"; exit 0 | ✓ PASS |
|
||||
| The gap truth, end to end | `go test -count=1 -timeout 20m -run '^TestHostStripeFlow$' -v .` in sm-pgstripe-plugin | 17 subtests PASS, 0 FAIL, 0 SKIP, including `decline_then_succeed`, `decline_then_succeed_payment_intent`, `paid_after_final_state`, `event_table` | ✓ PASS |
|
||||
| Docker-free decline coverage | `go test -count=1 -short -run '^(TestStripeOperator|TestStripeWebhook)$' -v .` | the three new subtests PASS | ✓ PASS |
|
||||
| Static check | `go vet ./...` in sm-pgstripe-plugin | exit 0 | ✓ PASS |
|
||||
| Published head | `git ls-remote origin refs/heads/main`, `ls-remote --tags` | `0313820f...`; no tags | ✓ PASS |
|
||||
| Decision coverage | `check.decision-coverage-verify` | 17/17 honored | ✓ PASS |
|
||||
|
||||
### Probe Execution
|
||||
@@ -198,105 +283,137 @@ Not applicable: no probe is declared in the phase and none exists under `scripts
|
||||
|
||||
### Requirements Coverage
|
||||
|
||||
REQUIREMENTS.md maps no IDs to Phase 18 (the roadmap says "Requirements: TBD"). Coverage is by the roadmap SCs and CONTEXT decisions:
|
||||
REQUIREMENTS.md maps no IDs to Phase 18 (the roadmap says "Requirements: TBD"); its few `D-nn` strings belong to other phases. The plans use phase-local IDs. Every ID in any plan's `requirements` field is accounted for:
|
||||
|
||||
| ID | Status | Evidence |
|
||||
|----|--------|----------|
|
||||
| SC-1 | ✓ SATISFIED | truth 1 |
|
||||
| SC-2 | ✗ BLOCKED (partial) | truths 2 and 4 verified; truth 3 failed |
|
||||
| SC-3 | ✓ SATISFIED | truth 5 |
|
||||
| SC-4 | ✓ SATISFIED | truth 6 |
|
||||
| SC-5 | ✓ SATISFIED | truth 7 |
|
||||
| D-01..D-17 | ✓ 17/17 honored | decision-coverage gate |
|
||||
| ID | Claimed by plans | Status | Evidence |
|
||||
|----|------------------|--------|----------|
|
||||
| SC-1 | 02, 07 | ✓ SATISFIED | truth 1 |
|
||||
| SC-2 | 02, 05, 07, 08 | ✓ SATISFIED (was BLOCKED) | truths 2, 3, 4 |
|
||||
| SC-3 | 01, 02, 03, 04, 06, 07 | ✓ SATISFIED | truth 5 |
|
||||
| SC-4 | 05, 07, 08 | ✓ SATISFIED | truth 6 |
|
||||
| SC-5 | 07 | ✓ SATISFIED | truth 7 |
|
||||
| D-02 | 02, 03, 04, 05, 06, 07 | ✓ honored | frozen port contract; parity with the consumer is human item 7 |
|
||||
| D-03, D-04 | 05 (D-04 also 02) | ✓ honored | proof host with both pay paths (truth 6) |
|
||||
| D-05 | 02, 03, 04 | ✓ honored | truth 5 |
|
||||
| D-06 | 06 | ✓ honored | truth 10 |
|
||||
| D-07 | 01, 03, 07 | ✓ honored | truth 8 |
|
||||
| D-08 | 01, 04 | ✓ honored | truth 11 |
|
||||
| D-09, D-10 | 01, 02, 07 | ✓ honored | truth 1 |
|
||||
| D-11 | 02, 06, 07 | ✓ honored | truth 9 |
|
||||
| D-12 | 05 | ✓ honored | stripe-go v87, TestSDKPin in the `plugins` stage |
|
||||
| D-13 | 01, 04, 05, 07 | ✓ honored | truths 8 and 11, pgstripe settings smokes |
|
||||
| D-14 | 02, 05, 07, 08 | ✓ honored | signature-only verification; truths 3 and 4 |
|
||||
| D-15 | 02, 03 | ✓ honored | truth 2 |
|
||||
| D-16 | 05, 08 | ✓ honored | Checkout mode and both accepting events; limitation A-WR-11 is advisory |
|
||||
| D-17 | 05, 06 | ✓ honored | process response bundles; truth 10 |
|
||||
| D-01 | none (scope split to 18.1) | ✓ honored | decision-coverage gate |
|
||||
|
||||
No orphaned requirements.
|
||||
No orphaned requirements: REQUIREMENTS.md expects nothing of Phase 18.
|
||||
|
||||
### Test Quality Audit
|
||||
|
||||
| Test | Linked | Skipped | Circular | Assertion level | Verdict |
|
||||
|------|--------|---------|----------|-----------------|---------|
|
||||
| TestOrderMatrix, TestPaymentMatrix | SC-1 | 0 | no (independent PHP table) | value | OK |
|
||||
| TestMoneyVectors | SC-1 | 0 | no (PHP MoneyRight vectors) | value | OK |
|
||||
| TestHostStripeFlow | SC-4 | 0 (the gate fails on skip) | no | behavioral | OK, but `event_table` asserts the defective payment_failed → Rejected mapping |
|
||||
| TestCustomerAPI* | D-06 | 0 | no | behavioral | OK |
|
||||
| layer checkout/orderPage | D-06/D-17 | 0 | no | behavioral | OK |
|
||||
| TestHostStripeFlow/decline_then_succeed, /decline_then_succeed_payment_intent | SC-2, D-16 | 0 | no | behavioral (snapshot equality, event counts, `assertPaid`) | OK |
|
||||
| TestHostStripeFlow/paid_after_final_state | SC-2, D-14 | 0 | no | behavioral plus log attributes | OK with notes: part 1's comment says the customer cancels the order while the test cancels the payment through the service (A-IN-14); part 4 posts an event naming the first payment's session and intent for the third payment and passes only because object ids are not matched (A-IN-13, A-WR-02) |
|
||||
| TestHostStripeFlow/event_table | SC-2 | 0 | no | value | OK; now asserts the corrected mapping |
|
||||
| TestStripeOperator (two new subtests), TestStripeWebhook (one) | SC-2 | 0 | no | value | OK; the state checks cannot fail on their own, the control case and the error check carry the proof (A-IN-14) |
|
||||
| TestOrderMatrix, TestPaymentMatrix, TestMoneyVectors, TestCustomerAPI*, layer suites | SC-1, D-06, D-17 | 0 | no | value / behavioral | OK (unchanged; ran in the gate) |
|
||||
|
||||
Disabled tests on requirements: 0. Circular patterns: 0. The only `t.Skip` calls are the `-short` guards in the testcontainers TestMain.
|
||||
Disabled tests on requirements: 0. Circular patterns: 0. The only `t.Skip` is the `-short` guard in the testcontainers TestMain, and the gate fails on a skip. Provenance of the Stripe event bodies: hand-built from Stripe's documented shapes, not captured from Stripe (human item 1).
|
||||
|
||||
Disconfirmation pass: (1) partially met: operator Checkout with delayed methods (A-WR-11); (2) a test that passes without testing what its comment says: part 1 of `paid_after_final_state`; (3) error paths without coverage: the guard-refusing branch of `refusedAccept`, and the browser return answering 500 when `accept` errors (A-IN-04).
|
||||
|
||||
### Anti-Patterns Found
|
||||
|
||||
| File | Line | Pattern | Severity | Impact |
|
||||
|------|------|---------|----------|--------|
|
||||
| (phase files) | - | TBD/FIXME/XXX | none found | - |
|
||||
| `../sm-paymentgateway-plugin/cart/registry.go` | 112 | `RegisterDetailsParams` stores types that nothing reads | ⚠️ Warning | exported, documented API with no effect |
|
||||
| `../sm-pgstripe-plugin/webhook.go` | 161 | retryable failure mapped to a final state | 🛑 Blocker | gap 1 |
|
||||
| `../sm-paymentgateway-plugin/orders/bridge.go` | 98-110 | `successful_payment_id` written before `Can(Pay)` | ⚠️ Warning | B-CR-02 |
|
||||
| `../sm-paymentgateway-plugin/currency/fixer.go` | 50, 66-69 | access key in the query string and logged inside `*url.Error` | ⚠️ Warning | B-CR-04 |
|
||||
| the seven files of 18-08 | - | TBD/FIXME/XXX/TODO/HACK/PLACEHOLDER | none found | - |
|
||||
| `../sm-pgstripe-plugin/webhook.go` | 178-181 | completed-unpaid records nothing; the sweep can close a payment that is still settling | Warning | A-WR-11, advisory |
|
||||
| `../sm-pgstripe-plugin/lang/{en,pl}/lang.yaml` | 30 | settings comment lists three webhook events, omits the only rejecting one | Warning | A-IN-15; an admin following the page leaves a failed async payment Sent until the sweep |
|
||||
| `../sm-pgstripe-plugin/webhook.go` | 79-83 | `PaymentError.Type` exported, never read | Info | A-IN-12; decide before the first tag |
|
||||
| `../sm-paymentgateway-plugin/orders/bridge.go` | 98-110 | `successful_payment_id` written before `Can(Pay)` | Warning | B-CR-02 (carried) |
|
||||
| `../sm-paymentgateway-plugin/api/actions.go` | 361-365 | customer cancel leaves an online payment Sent | Warning | B-WR-06 (carried), see A-CR-02 remainder |
|
||||
| `../sm-paymentgateway-plugin/cart/registry.go` | 112 | `RegisterDetailsParams` stores types nothing reads | Warning | carried; deferred-items.md |
|
||||
| `../sm-paymentgateway-plugin/currency/fixer.go` | 50, 66-69 | access key in the query string and in a logged `*url.Error` | Warning | B-CR-04 (carried) |
|
||||
|
||||
### Code Review Critical Findings Against the Criteria
|
||||
No blocker. The blocker of the previous pass (`webhook.go:161`, a retryable failure mapped to a final state) is gone.
|
||||
|
||||
The review was advisory and run after the push. Each critical finding was checked in code and weighed against the stated criteria:
|
||||
|
||||
| Finding | Confirmed in code | Breaks a stated criterion? | Classification |
|
||||
|---------|-------------------|----------------------------|----------------|
|
||||
| A-CR-01 payment_failed rejects | yes (`webhook.go:161`; the test asserts it; the README tells operators to subscribe) | **Yes**: the Stripe operator does not "take payments" reliably, and the change is new relative to PHP, whose Stripe notification never rejected | 🛑 Blocker (gap 1) |
|
||||
| A-CR-02 paid signal on Rejected/Canceled dropped silently | yes (`operator.go` accept) | Not on its own: PHP processNotification was equally silent. It turns A-CR-01 from visible into silent loss, so it belongs in the same fix | grouped with gap 1 |
|
||||
| B-CR-02 bridge persists successful_payment_id before Can(Pay) | yes (`bridge.go:98-110`); PHP only assigned it in memory | Not for the proven flows (normal Pay, replay). It diverges from PHP when a second payment is accepted on a Paid/Canceled/Finished order, where refund paths would then target the wrong payment | ⚠️ Warning (fix recommended; small) |
|
||||
| B-CR-01 forgeable guest codes on legacy orders | yes (`LegacySalt` and the deterministic `guest_code`) | No SC covers the customer API; PHP had the same weakness. It is a real exposure on a new public API (read/cancel/refund-request of legacy guest orders) | ⚠️ Warning, security; decide before any app with legacy guest orders enables the API |
|
||||
| A-CR-03 comma parsed as a decimal in `en` | yes (`money.ts` parseMoneyInput is locale-blind) | Not an SC; an admin data-entry hazard (1000× error) | ⚠️ Warning |
|
||||
| B-CR-03 default_currency editable | yes (README admits "does not convert stored amounts") | Not an SC; PHP locked it via the wizard | ⚠️ Warning |
|
||||
| B-CR-04 Fixer key in logs | yes | Not an SC; violates the spirit of the secrets prohibitions | ⚠️ Warning, security |
|
||||
|
||||
Other confirmed warnings relevant to the Horoskopia contract:
|
||||
- **B-WR-04**: `RecordExternal` says "need not be enabled", but Send/Accept are guarded by the operator's enabled setting. PHP's guard was effectively always true for Horoskopia's directly constructed `StripeOperator`, which "bypasses PaymentFactory enabled-check". If Horoskopia keeps the Stripe operator disabled, path (b) stalls silently (human item 5).
|
||||
- **Release hygiene (info):** both plugin repositories have no tags; `sm-pgstripe-plugin` requires `sm-paymentgateway-plugin v0.0.0` through a local replace, and both `go.mod` files require `summercms v0.1.24` although they rely on v0.1.25 changes (21c5618 settings dropdown provider, eb60c32 money trailing zeros). This works through the submodule plus `go.work` layout, but an app must itself require v0.1.25 or newer.
|
||||
Carried warnings that do not break a stated criterion, unchanged from the previous pass: B-CR-01 (forgeable guest codes on legacy orders), A-CR-03 (comma read as a decimal in `en`), B-CR-03 (`default_currency` editable), B-WR-04 (`RecordExternal` with a disabled operator, human item 7). Release hygiene (info): neither plugin repository has a tag; both `go.mod` files still require `summercms v0.1.24` while relying on v0.1.25 changes, and pgstripe requires the gateway at `v0.0.0` through a local replace, so an application must itself require v0.1.25 or newer.
|
||||
|
||||
### Prohibitions (judgment tier, non-authoritative LLM verdicts)
|
||||
|
||||
unverified-prohibition: human review recommended (human item 8).
|
||||
|
||||
| Plan | Prohibition (short) | Verdict | Evidence |
|
||||
|------|--------------------|---------|----------|
|
||||
| 01 | framework never names a consuming app or carries payment-specific types | held | grep over the phase-18 framework files is clean |
|
||||
| 01 | framework never names a consuming app or carries payment-specific types | held | unchanged framework files; grep clean on the previous pass |
|
||||
| 01, 02 | no float for money | held | grep; the one `float64` is a JSON display switch |
|
||||
| 01, 05 | secrets never returned or logged | held for the settings secrets (demo GET shows only states) | B-CR-04 leaks the Fixer key, which is outside the literal scope |
|
||||
| 01, 06, 07 | nothing pushed or tagged without go-ahead | not verifiable from code | recorded go-aheads are in the SUMMARYs |
|
||||
| 02 | legacy rows never bulk-rewritten | held | no migration or command rewrites cart/details; TestLegacyFixtures |
|
||||
| 01, 05 | secrets never returned or logged | held for the settings secrets | B-CR-04 leaks the Fixer key, outside the literal scope |
|
||||
| 01, 06, 07 | nothing pushed or tagged without go-ahead | not verifiable from code | go-aheads are recorded in the SUMMARYs |
|
||||
| 02 | legacy rows never bulk-rewritten | held | no migration or command rewrites cart/details |
|
||||
| 02, 05 | unsigned input never changes state; verify before lookup | held | notification.go ordering; browser return re-checks at Stripe |
|
||||
| 02, 04 | PHP names not renamed | held | events, tables, commands and state strings compared |
|
||||
| 02 | redactor on every logged body | held | TestRedactor; dashed api-key fix |
|
||||
| 02, 04 | PHP names not renamed | held | events, tables, commands and state strings compared on the previous pass; code unchanged |
|
||||
| 02 | redactor on every logged body | held | TestRedactor in the gate |
|
||||
| 03 | partials are curated view models; state changes only through services | held (spot check) | TestOrdersAdminSmoke |
|
||||
| 04 | mail only after commit | held | TestMailTracer rollback case |
|
||||
| 05 | amount computed server-side | held | 4900 asserted |
|
||||
| 05, 07 | no network in tests | held | grep finds no external hosts in `_test.go` |
|
||||
| 06 | API never accepts amount, currency or state | held (spot check) | TestCustomerAPIAuthz |
|
||||
| 05 | amount computed server-side | held | asserted in `checkout_operator_path` |
|
||||
| 05, 07 | no network in tests | held | fake Stripe on a local listener; no external host in `_test.go` |
|
||||
| 06 | API never accepts amount, currency or state | held (spot check) | TestCustomerAPI authz suites |
|
||||
| 06 | guest code grants exactly one order | held literally; weak in strength (B-CR-01) | |
|
||||
| 07 | no test weakened to go green | flagged | `event_table` asserts the defective mapping; written to the plan's must_have, not weakened |
|
||||
| 07 | no test weakened to go green | held (was flagged) | `event_table` now asserts the corrected mapping |
|
||||
| 08 | no test weakened or skipped | held | the diff `d488834..0313820` removes only the old decline assertion (replaced by the corrected one) and the string-only log capture (replaced by a superset keeping `has` and `lines`); 17 subtests, 0 skipped |
|
||||
| 08 | logs never carry the body, the decline message, the e-mail or key material | held for the lines 18-08 added; not held literally on the older purchase-time path | the unit test asserts no decline text in any record; `payments/service.go:285` still logs the operator's message for a decline confirmed at Purchase (A-IN-11). Stripe's decline texts are not secrets; the rule and the behaviour differ. Decide: narrow the wording or log the code instead |
|
||||
| 08 | nothing pushed before the reply; no tag | pushed state consistent, reply not verifiable from code | `origin/main` = `0313820`, no tags; the SUMMARY quotes the reply `push` |
|
||||
| 08 | the gateway and its payment machine are not changed | held | `f55320a`, clean tree, empty diff |
|
||||
|
||||
### Human Verification Required
|
||||
|
||||
These are listed now so they are not lost; they become actionable once gap 1 is closed.
|
||||
No `*-UAT.md` exists in the phase directory, so no item is settled by recorded UAT. The plans carry no deferred `<human-check>` blocks.
|
||||
|
||||
1. **Mail wording (PL/EN)**: read or send the 27 templates. Expected: natural Polish, PHP meaning kept, variables render. Why human: language quality.
|
||||
2. **Quote cart widget, light and dark**: add, edit, refuse, remove, process. Expected: busy row, restore with a toast, read-only after placing. Why human: visual rendering of a custom element.
|
||||
3. **Settings pages, light and dark** (payment_gateway, mailer, pgstripe secrets). Expected: tabs, chips, status lines without values, wrapping. Why human: visual.
|
||||
4. **Layer playground**: order → checkout redirect → polling → list. Expected: UI-SPEC L1-L8. Why human: visual; no DOM in tests.
|
||||
5. **Horoskopia consumer parity and the operator enabled flag**: compare the acme.books fixture with BookApiController and BookFulfilmentService, and confirm the Stripe operator will be enabled on Horoskopia (B-WR-04). Why human: production configuration and a later port.
|
||||
6. **Prohibition verdicts**: accept or reject the table above.
|
||||
### 1. Live Stripe test mode: declined card, then a successful retry
|
||||
|
||||
**Test:** On an instance with both plugins mounted, test keys and the test signing secret on the pgstripe settings page and a reachable webhook endpoint (public URL or `stripe listen --forward-to`): in Checkout mode pay with 4000 0000 0000 0002, then with 4242 4242 4242 4242 on the same Checkout page; repeat in PaymentIntent mode through the card form with the same client secret.
|
||||
**Expected:** After the decline the payment stays Sent, the order AwaitingPayment, no rejected mail, one Warn line `payment attempt failed` with `code` and `decline_code` and no decline text. After the retry the payment is Accepted, the order Paid, one OrderPaid, one paid mail. No `refuses Accept` line.
|
||||
**Why human:** External service. The proof uses a fake Stripe and hand-built events; that Stripe sends `payment_intent.payment_failed` with the `payment_uuid` metadata on a Checkout decline, and in which order, can only be observed against Stripe.
|
||||
|
||||
### 2. README of sm-pgstripe-plugin (18-08 coverage item D6)
|
||||
|
||||
**Test:** Read the Webhook events list, the paragraph on a paid signal that cannot be accepted, and step 2 of Stripe Dashboard.
|
||||
**Expected:** Accurate and clear for an operator. Decide on two known points: "one Error log line" is one per signal (A-IN-10); the settings page comment omits `checkout.session.async_payment_failed` (A-IN-15).
|
||||
**Why human:** Clarity is a reading judgment.
|
||||
|
||||
### 3. Mail wording (PL/EN)
|
||||
|
||||
**Test:** Read or send the 27 templates. **Expected:** natural Polish, PHP meaning kept, variables render. **Why human:** language quality.
|
||||
|
||||
### 4. Quote cart widget, light and dark
|
||||
|
||||
**Test:** Add, edit, refuse (12.34567, qty 0), remove, process. **Expected:** busy row, restore with a toast, read-only after placing. **Why human:** visual rendering of a custom element.
|
||||
|
||||
### 5. Settings pages, light and dark
|
||||
|
||||
**Test:** payment_gateway, mailer, pgstripe; set, reload, remove and undo a secret. **Expected:** tabs, chips, status lines without values, wrapping. **Why human:** visual.
|
||||
|
||||
### 6. Layer playground
|
||||
|
||||
**Test:** order, checkout redirect, polling, list, narrow screen. **Expected:** UI-SPEC L1-L8. **Why human:** visual; no DOM in tests.
|
||||
|
||||
### 7. Horoskopia consumer parity and the operator enabled flag
|
||||
|
||||
**Test:** Compare the acme.books fixture with BookApiController and BookFulfilmentService, and confirm the Stripe operator will be enabled on Horoskopia (B-WR-04). **Expected:** the same calls; operator enabled, or `RecordExternal` changed. **Why human:** production configuration and a later port.
|
||||
|
||||
### 8. Prohibition verdicts
|
||||
|
||||
**Test:** Accept or reject each row of the table above. **Expected:** every prohibition accepted as held or turned into a gap. **Why human:** judgment-tier prohibitions need explicit resolution.
|
||||
|
||||
### Gaps Summary
|
||||
|
||||
One root cause blocks the goal: the Stripe operator's webhook mapping treats `payment_intent.payment_failed` as terminal. Stripe sends that event on every declined attempt and lets the customer retry, so in the default Checkout mode a single decline followed by a successful retry leaves the customer charged while the payment stays Rejected, and the success event is dropped without a log line (A-CR-01 with A-CR-02). The PHP Stripe operator never rejected a payment from a notification, so this is a regression in "take payments as on PHP", not a carried-over quirk. The fix is small and local to `sm-pgstripe-plugin`: log instead of rejecting on payment_failed, log at Error level for a paid signal on a non-Accepted final payment, flip the `event_table` assertion, add a decline-then-succeed subtest, and update the README. It needs a pgstripe patch commit. No framework re-tag is needed.
|
||||
No gaps. The one failed truth of the previous pass is closed: `payment_intent.payment_failed` no longer moves the payment to a final state, the retry is accepted in Checkout and PaymentIntent modes, and a paid signal that the payment machine refuses is logged at Error level with the ids needed for a refund. I ran the proof myself: TestHostStripeFlow passes with 17 subtests on Postgres, the Docker-free decline tests pass, and the full gate with Postgres required exits 0. Nothing that passed before regressed; apart from the seven pgstripe files, the code is identical to what the previous pass verified.
|
||||
|
||||
Everything else holds: the PHP state machines are reproduced exactly, money is exact, build-time operator registration, the notification route, events, mail, admin, commands and the customer API work, and the full gate (including Postgres) and the framework suite are green. The other confirmed critical findings (B-CR-02, B-CR-01, A-CR-03, B-CR-03, B-CR-04) do not break a stated success criterion, but B-CR-02 (a one-block reorder) and B-CR-04 are cheap to fix in the same gap-closure round, and B-CR-01 should be decided before any app with legacy guest orders exposes the customer API.
|
||||
The status is `human_needed`, not `passed`, for two reasons. The Stripe behaviour is proven against a fake, so one live test-mode run of decline then retry is still owed. And the items the previous report parked until the gap closed (mail wording, three visual checks, consumer parity, prohibition verdicts) are now due.
|
||||
|
||||
**If the developer judges the operator path (a) outside the Horoskopia contract** (Horoskopia uses path (b) today), the gap can be accepted with an override. This is not recommended, because D-16 exists so the Horoskopia port can drop its own Checkout code:
|
||||
|
||||
```yaml
|
||||
overrides:
|
||||
- must_have: "SC-2 / goal 'take payments': the Stripe operator takes a payment reliably through the notification route"
|
||||
reason: "<why a rejected-then-paid Stripe payment is acceptable until a named follow-up>"
|
||||
accepted_by: "<name>"
|
||||
accepted_at: "<ISO timestamp>"
|
||||
```
|
||||
Two review findings were judged follow-up work, not failed must-haves, and are recorded as advisory with the reasons above: A-WR-11 (a delayed Checkout method that settles after the sweep's timeout) and the remainder of A-CR-02 (no session expiry on cancel, a log-only signal, and customer cancel leaving an online payment Sent). Neither touches the Horoskopia flow as it runs on PHP. Both should be closed before an application relies on operator Checkout mode with delayed methods or exposes customer cancel with an online operator, and before the first tag of `sm-pgstripe-plugin`.
|
||||
|
||||
### Decision Coverage
|
||||
|
||||
@@ -304,5 +421,5 @@ All trackable CONTEXT.md decisions are honored by shipped artifacts (17/17).
|
||||
|
||||
---
|
||||
|
||||
_Verified: 2026-10-13T16:08:51Z_
|
||||
_Verified: 2026-10-13T22:41:19Z_
|
||||
_Verifier: Claude (msd-verifier)_
|
||||
|
||||
Reference in New Issue
Block a user