docs(18.1): re-verify after quick 261016-myi and 261016-nqk - human_needed (release go-ahead)
This commit is contained in:
Jakub Zych
@@ -1,8 +1,8 @@
|
||||
---
|
||||
phase: 18.1-payment-gateway-subscriptions
|
||||
verified: 2026-10-16T14:45:00Z
|
||||
verified: 2026-10-16T15:45:06Z
|
||||
status: human_needed
|
||||
score: 5/5 roadmap success criteria verified (plan must-haves 64/65 verified, 1 present-behavior-unverified)
|
||||
score: 5/5 roadmap success criteria verified (plan must-haves 65/65 verified, 4 of them as amended by user-accepted fixes)
|
||||
covered_files:
|
||||
- .planning/phases/18.1-payment-gateway-subscriptions/18.1-01-PLAN.md
|
||||
- .planning/phases/18.1-payment-gateway-subscriptions/18.1-01-SUMMARY.md
|
||||
@@ -22,67 +22,71 @@ covered_files:
|
||||
- .planning/phases/18.1-payment-gateway-subscriptions/18.1-08-SUMMARY.md
|
||||
- .planning/phases/18.1-payment-gateway-subscriptions/18.1-09-PLAN.md
|
||||
- .planning/phases/18.1-payment-gateway-subscriptions/18.1-09-SUMMARY.md
|
||||
- .planning/quick/261016-myi-fix-18-1-review-warnings/261016-myi-PLAN.md
|
||||
- .planning/quick/261016-myi-fix-18-1-review-warnings/261016-myi-SUMMARY.md
|
||||
- .planning/quick/261016-nqk-close-uat-gaps-g-18-1-7-and-g-18-1-4/261016-nqk-PLAN.md
|
||||
- .planning/quick/261016-nqk-close-uat-gaps-g-18-1-7-and-g-18-1-4/261016-nqk-SUMMARY.md
|
||||
- admin/src/components/form/fields/TaglistField.vue
|
||||
- admin/src/views/FormView.vue
|
||||
- admin/src/views/PreviewView.vue
|
||||
- admin/src/views/ReorderView.vue
|
||||
- modules/cabana/bulk_detached_test.go
|
||||
- modules/cabana/crud.go
|
||||
- modules/cabana/field_taglist.go
|
||||
- modules/cabana/form_schema.go
|
||||
- modules/cabana/list_schema.go
|
||||
- modules/cabana/phase181_records_test.go
|
||||
- modules/cabana/query.go
|
||||
- modules/cabana/relation_field.go
|
||||
- modules/cabana/reorder.go
|
||||
- modules/pact/capabilities.go
|
||||
- scripts/check-phase10.sh
|
||||
- scripts/check-phase18.1.sh
|
||||
covered_digest: "v2:sha256:32f47883441c7cf001b0c0f1e2ca7a9d1d0c7086072759828956104046006979"
|
||||
behavior_unverified: 1
|
||||
covered_digest: "v2:sha256:4d5014a3f2a0a899ec2e3ad3896b5355c8fd446b212d578575869422cd4560e8"
|
||||
behavior_unverified: 0
|
||||
overrides_applied: 0
|
||||
re_verification:
|
||||
previous_status: human_needed
|
||||
previous_score: "5/5 roadmap SCs, 41/41 plan must-haves (plans 01-06)"
|
||||
previous_score: "5/5 roadmap SCs; plan must-haves 64/65 verified, 1 present-behavior-unverified (WR-07 component wiring)"
|
||||
gaps_closed:
|
||||
- "Human item 1 (decide CR-01 and CR-02): decided by the user on 2026-10-14 (18.1-CONTEXT.md, D-21 amendment) and fixed by gap-closure plans 18.1-07 to 18.1-09"
|
||||
- "CR-01 stale webhook snapshots: created/updated jobs retrieve Stripe's current state under a per-subscription session advisory lock; deletion before creation retried (ErrSubscriptionNotStored)"
|
||||
- "CR-02 cross-currency checkout: CheckoutPriceID has no cross-currency fallback; API answers 422, gateway answers ErrNoPriceForCurrency; jpy dropped"
|
||||
- "WR-01 Stripe calls in the bulk transaction: pact.AdminBulkAction.Detached; sync actions hold no transaction across Stripe calls"
|
||||
- "WR-02 duplicate Stripe customers: first-write-wins customer id plus idempotency key"
|
||||
- "WR-03 already-subscribed guard: ScopeOpen, CheckoutGuarded under SubscriberLockKey, checkout idempotency key (partially: see review WR-04/WR-07)"
|
||||
- "WR-04 archived prices: plan resolution by price, product, plan_code"
|
||||
- "WR-06 SyncSubscription lost update: locked re-read, gateway-owned columns only"
|
||||
- "WR-07 buttons stuck after Back: watchPageRestore wired in the three components"
|
||||
- "Previous human item 1 (disposition of re-review WR-01..WR-07): UAT row 5 passed; all seven fixed in quick 261016-myi (pgsub 2c9e5ef, pgstripe 2e44508), each with a named test that ran green here"
|
||||
- "Previous human item 2 (live Stripe checkout): UAT row 6 passed (source=stripe logged, activated once, second subscribe 409)"
|
||||
- "Previous human item 3 (portal cancel_at question): UAT row 7 first failed (G-18.1-7), fixed in pgstripe a5b3314, retest passed"
|
||||
- "Previous human item 4 (browser pass, incl. WR-07 Back button): UAT row 4 passed after the G-18.1-4 fix (pgsub 6a3f2c4 d5579cd, framework bc5685e 013890c 17398fe)"
|
||||
- "WR-07 component wiring (previously PRESENT_BEHAVIOR_UNVERIFIED): Back from Checkout observed in a real browser in UAT row 4; layer code unchanged since (v0.2.1, 22368c5)"
|
||||
- "CR-01 sequence 3 coincidental reliance (Service.Cancel without the subscription lock): Cancel now runs under SubscriptionLockKey; TestCancelTakesSubscriptionLock passed here"
|
||||
- "G-18.1-8 (pgsub could not boot in a summer-built app): bootService goes through lagoon.OnDatabase; TestBootBeforeDatabase passed here; UAT row 8 passed"
|
||||
gaps_remaining: []
|
||||
regressions: []
|
||||
behavior_unverified_items:
|
||||
- truth: "WR-07: after a redirect to Stripe Checkout or the billing portal, a back/forward-cache restore re-enables PgSubscribeButton, PgManageSubscriptionButton and PgMySubscription's actions; each component removes its pageshow listener on unmount"
|
||||
test: "In a real browser, click Subscribe (and Manage subscription, and the portal link in My subscription), let the page leave for Stripe, then press Back"
|
||||
expected: "The page comes back from the bfcache with the button enabled (not 'Subscribing...'), no error shown; PgMySubscription also refreshes its data"
|
||||
why_human: "flow.restore() and watchPageRestore are unit-tested, but the component wiring is only asserted as source text (subscriptionComponents.test.ts, review IN-04); no test mounts a component and dispatches pageshow persisted=true, and node --test cannot exercise the real bfcache"
|
||||
coincidental_reliance_items:
|
||||
- truth: "CR-01 sequence 3: an older queued customer.subscription.updated after Service.Cancel fires no SubscriptionReactivated"
|
||||
reason: incidental-ordering
|
||||
harden: "TestEventOrdering/stale_cancel_flag_no_reactivated runs the job after Cancel has finished. Nothing orders Cancel against a job that already retrieved Stripe's state: Service.Cancel (classes/service.go:373-395) writes cancel_at_period_end without SubscriptionLockKey, contrary to lock.go's own rule. Take WithSessionLock(SubscriptionLockKey) around the gateway call and the local write (review WR-01)."
|
||||
human_verification:
|
||||
human_verified:
|
||||
- test: "Disposition of the re-review warnings (18.1-REVIEW.md, 2026-10-16; 18.1-REVIEW-DISPOSITION.md lists all open) before the Horoskopia cutover"
|
||||
expected: "Each of WR-01 to WR-07 is accepted (backlog item or deferred-items entry) or scheduled as a quick task or decimal phase. The money- and listener-relevant ones: WR-02 SubscriptionCanceled fires twice in the deleted-before-created sequence (ordering_test.go never counts canceled); WR-01 Service.Cancel race; WR-03 concurrent plan syncs can create duplicate Stripe products (row lock lost with Detached); WR-04 unpaid open Checkout Sessions not refused; WR-06 checkout bills the last-synced amount, not the shown one; WR-05 gbp/cad/aud/chf plans become unbuyable after a sync; WR-07 incomplete blocks checkout for about 23 h"
|
||||
why_human: "None defeats a success criterion or a plan must-have as written, and Horoskopia's own flow (pln, synced by plan sync) is unaffected by WR-05 and WR-06's currency side. Whether they block the cutover is a product decision."
|
||||
uat_row: 5
|
||||
recorded_at: "2026-10-16T15:34:36.000Z"
|
||||
- test: "Live Stripe test-mode checkout end to end: test-mode keys and the subscription webhook secret in pgstripe settings, `stripe listen --forward-to <host>/webhook/stripe/subscriptions`, checkout from the layer playground (PgPlans -> PgSubscribeButton) with the user subscriber"
|
||||
expected: "Checkout opens on the plan's price in the shown currency; after payment the webhook answers 200 'Webhook received'; the job logs source=stripe (fresh retrieval, not snapshot), stores the subscription active; golem15.pgsub.subscription.activated fires once even when created(incomplete) and updated(active) arrive together; GET /subscription answers has_access true. A second Subscribe answers 409"
|
||||
why_human: "The Stripe fake cannot prove hosted Checkout, real delivery order, or that a real V1Subscriptions.Retrieve body on the pinned API version decodes through decodeSubscription (the new CR-01 path)"
|
||||
uat_row: 6
|
||||
recorded_at: "2026-10-16T15:34:36.000Z"
|
||||
- test: "Billing-portal cancellation (D-21 cancel_at question): open the portal through PgManageSubscriptionButton, cancel the subscription, inspect the delivered customer.subscription.updated and the retrieved subscription"
|
||||
expected: "If Stripe reports cancel_at_period_end true, the row gets it and PgMySubscription shows the will-cancel notice. If it reports only cancel_at, the row keeps cancel_at_period_end false and the cancellation is invisible; that needs a follow-up mapping"
|
||||
why_human: "Depends on the Stripe account's API version and portal configuration"
|
||||
uat_row: 7
|
||||
recorded_at: "2026-10-16T15:34:36.000Z"
|
||||
- test: "Browser pass through the admin screens and the layer components"
|
||||
expected: "Taglist chips (Enter, comma, remove, suggestions, read-only) on the plan form; flat drag reorder writes sort_order; bulk sync_to_stripe and toolbar sync_all_to_stripe toasts with per-plan failures; Subscriptions list filters and preview subscriber partial; the layer's plans table, subscribe redirect, my-subscription cancel through PgConfirm, portal redirect; Back from Stripe Checkout and the portal re-enables the buttons (WR-07, now fixed: retest)"
|
||||
why_human: "vitest and SSR specs do not drive a real browser or the back/forward cache"
|
||||
retest_reason: "UAT row 4 is still pending; the layer components changed in 34cf926 (WR-07 fix), so the Back-button part now checks the fix rather than the defect"
|
||||
uat_row: 4
|
||||
recorded_at: "2026-10-16T15:34:36.000Z"
|
||||
- test: "pgsub boots in an application built with `summer build`"
|
||||
uat_row: 8
|
||||
recorded_at: "2026-10-16T15:34:36.000Z"
|
||||
human_verification:
|
||||
- test: "Release go-ahead for the post-gate fixes: framework tag, plugin require bumps and tags, pushes"
|
||||
expected: "The user approves (or defers with a recorded reason) the release order from the quick-task summaries: push summercms.go master and tag v0.2.2 (carries pact.ListExtendRecords, pact.FormRecordTitle and meta.title); bump sm-paymentgatewaysubscriptions-plugin's require to v0.2.2, push main, tag (v0.1.1); bump sm-pgstripe-plugin's requires to v0.2.2 and the new pgsub tag, push main, tag (v0.1.1). After that a consumer pinned to the tags gets the G-18.1-8 boot fix, WR-01..WR-07 and G-18.1-7"
|
||||
why_human: "Project rule: nothing is pushed or tagged without the user's go-ahead. Today the released pgsub v0.1.0 lacks the G-18.1-8 fix, so an app built with `summer build` on the tagged plugin fails every command with 'database is not published' (the UAT 8 blocker). pgsub HEAD calls pact.ListExtendRecords and pact.FormRecordTitle, which no framework tag has; its go.mod requires summercms v0.2.1 and builds only through its replace ../summercms.go, which a consuming module ignores. Unpushed: framework 13 commits (3 code), pgsub 4, pgstripe 2"
|
||||
---
|
||||
|
||||
# Phase 18.1: Payment Gateway Subscriptions Verification Report
|
||||
|
||||
**Phase goal:** The generic core of `Golem15.PaymentGatewaySubscriptions` and pgstripe's subscription gateway and webhook run on SummerCMS. Horoskopia's subscription flow (plans, checkout, Stripe webhook, `golem15.pgsub.*` events, cancel and portal) works as it does on PHP. Event ordering is the exception: the user's 2026-10-14 decision amended D-21 (18.1-CONTEXT.md § Amendments).
|
||||
**Mode:** mvp. The ROADMAP goal is not in user-story form, so this report verifies against the goal text and its five success criteria, as the first verification did.
|
||||
**Verified:** 2026-10-16T14:45:00Z
|
||||
**Status:** human_needed
|
||||
**Re-verification:** Yes. This run follows gap-closure plans 18.1-07, 08 and 09. The previous run (2026-10-14) ended human_needed with no `gaps:`. Its human item 1 (CR-01/CR-02) was decided as "fix", and this run verifies those fixes in full. Plans 01 to 06 got a regression check through the full gate.
|
||||
**Phase goal:** The generic core of `Golem15.PaymentGatewaySubscriptions` and pgstripe's subscription gateway and webhook run on SummerCMS. Horoskopia's subscription flow (plans, checkout, Stripe webhook, `golem15.pgsub.*` events, cancel and portal) works as it does on PHP. Exceptions are the user's recorded amendments: event ordering (D-21 amendment, 2026-10-14), the portal `cancel_at` mapping (D-21 amendment, 2026-10-16), and the re-review fixes the user accepted in UAT row 5 (`canceled` fires once on a deletion, `incomplete` no longer blocks checkout).
|
||||
**Mode:** mvp. The ROADMAP goal is not in user-story form, so this report checks the goal text and its five success criteria, as the earlier verifications did.
|
||||
**Verified:** 2026-10-16T15:45:06Z
|
||||
**Status:** human_needed (one item: the release go-ahead)
|
||||
**Re-verification:** Yes. The previous report (2026-10-16T14:45Z) had no `gaps:`. It went stale (covered_changed) after two quick tasks: 261016-myi (WR-01 to WR-07 and G-18.1-8) and 261016-nqk (G-18.1-7 and G-18.1-4). This run verifies both in full and regression-checks plans 01 to 09 through the full gate.
|
||||
|
||||
## Goal Achievement
|
||||
|
||||
@@ -90,177 +94,168 @@ human_verification:
|
||||
|
||||
| # | Truth | Status | Evidence |
|
||||
|---|-------|--------|----------|
|
||||
| 1 | SC-1: squashed idempotent baseline, build-time polymorphic subscribers, built-in `user` subscribable | VERIFIED | Unchanged since the first verification. TestBaselineSchema and TestUserSubscribable passed in this run's postgres stage. `user_subscribable.go` SetCustomerID now writes only when the column is empty (`UPDATE users SET stripe_customer_id = ? WHERE id = ? AND (stripe_customer_id IS NULL OR stripe_customer_id = '')`). It still writes only `stripe_customer_id`. |
|
||||
| 2 | SC-2: Stripe gateway incl. syncPlanToGateway; both webhook routes verify, enqueue, process idempotently, fire six events | VERIFIED | `subscriptions/job.go` `work` sends created/updated to `applySubscription`. That path answers from the marker first (`EventProcessed`), routes a foreign app to claim-and-skip with no Stripe call, and otherwise takes `classes.WithSessionLock(SubscriptionLockKey)`. Under the lock it calls `RetrieveSubscription` outside any transaction (the 404 path falls back to the snapshot) and then claims the marker and calls `Apply` in a single `lagoon.Transaction`. Deletions run under the same lock. TestEventOrdering, TestFreshStateSmoke, TestSubscriptionJob, TestSubscriptionWebhookSmoke, TestRetrieveSubscription and TestHostSubscriptionFlow passed. |
|
||||
| 3 | SC-3: admin screens, Go service API, headless customer API, layer components | VERIFIED | The admin `sync_to_stripe` declares `Detached: true` (`controllers/subscriptionplans.go:55`). cabana's `detachedBulkAction` (`crud.go:412-435`) loads the selected plans through `loadScoped(..., false)` with no transaction. `SyncPlan(ctx, nil, plan)` saves the ids with `context.WithoutCancel`. In the service API, `CheckoutGuarded`, `Open` and a locked `SyncSubscription` are new. In the customer API, the 409 uses `Open` (`api/subscription.go:92-98`) and the 422 uses `CheckoutPriceID` (`:128-131`), both before any gateway call. In the layer, `watchPageRestore` is wired in SubscribeButton, ManageSubscriptionButton and MySubscription, and package.json is 0.2.1. TestAdminSyncDetached, TestCustomerAPIGuards, TestCustomerAPISmoke and TestCustomerAPIAuthz passed, and the layer ran 112/112 TAP (43 `subscriptions:`). |
|
||||
| 4 | SC-4: host test proves checkout, activation, invoice.paid, past_due grace, cancel, duplicate idempotency against a fake | VERIFIED | TestHostSubscriptionFlow passed in this run, now against the fake's retrieve endpoint. TestEventOrdering adds the checkout race in both orders and concurrently, deleted-before-created, the stale cancel flag, a retrieval failure retried with no marker, and duplicate or foreign events making no Stripe call. |
|
||||
| 5 | SC-5: unit tests delivered in the phase's last plan | VERIFIED | Plan 09 is the last plan and adds the named tests for every plan 07 and 08 behaviour: cabana `bulk_detached_test.go`; pgsub `checkout_price_test.go`, `plan_resolution_test.go`, `sync_subscription_test.go`, `checkout_guard_test.go` and `admin_sync_detached_test.go`; pgstripe `ordering_test.go` and `gateway_hardening_test.go`; the layer's `subscribe.test.ts` and `subscriptionComponents.test.ts`. The gate requires each of the 13 new Go names by stage and package (grep count 1 each in `scripts/check-phase18.1.sh`). |
|
||||
| 1 | SC-1: Subscription and SubscriptionPlan on a squashed, idempotent v2.4.0 baseline; subscribers registered at build time; built-in `user` subscribable | VERIFIED | TestBaselineSchema and TestUserSubscribable passed in this run's postgres stage. G-18.1-8: `boot.go` `bootService` now builds and publishes `*classes.Service` through `lagoon.OnDatabase`, so plugins can activate before a command opens the database (generated main.go order). TestBootBeforeDatabase passed here. UAT row 8 passed on a demo app built with `summer build`. The user resolver gained an optional `ResolveMany` (`contracts.SubscriberBatchResolver`). It still writes only `stripe_customer_id`. |
|
||||
| 2 | SC-2: Stripe implementation of the gateway incl. `syncPlanToGateway`; both webhook routes verify, enqueue, process idempotently, fire the six events | VERIFIED | `subscriptions/job.go` `applySubscription` is unchanged: marker check, then foreign-app claim, then `WithSessionLock(SubscriptionLockKey)`, then retrieve outside a transaction, then claim and Apply in one transaction. G-18.1-7: every mapping path goes through `subscriptionObject.args()`: the snapshot (`decode.go:222-226`), the fresh retrieve (`gateway.go:468-475`) and `Gateway.SyncSubscription` (`gateway.go:415-422`). `pendingCancel()` (`decode.go:162-167`) maps Stripe's flag, or `cancel_at` on an active or trialing subscription, to `cancel_at_period_end`. WR-02: `ApplyDeleted` fires `canceled` only when the row was not already canceled (`apply.go:241-243`). Passed here: TestFreshStateSmoke incl. `portal_cancel_at_only_then_uncancel` (reactivated exactly once) and `deleted_before_created` (canceled == 1); TestEventOrdering incl. `deleted_before_created` (canceled == 1, `ordering_test.go:152`); TestDecodeSubscription/a_cancel_at_date…; TestHostSubscriptionFlow; TestSubscriptionWebhookSmoke; TestSubscriptionJob. UAT rows 6 and 7 passed against real Stripe test mode. |
|
||||
| 3 | SC-3: admin screens, Go service API, headless customer API, layer components | VERIFIED | Admin: `models/subscription/columns.yaml` gains a `subscriber` column, filled per page by `subscriptionsAdminController.ListExtendRecords`. Type and id stay as invisible searchable columns. The preview title comes from `FormRecordTitle`, and the label element reads its `value` property (`assets/js/fields.js`). Framework: `pact.ListExtendRecords` and `pact.FormRecordTitle` (`capabilities.go:737-763`); the SPA uses `meta.title`; dist rebuilt and matching. Service API: `Cancel` takes `SubscriptionLockKey` (`service.go:471-499`). `CheckoutGuarded` refuses on `CheckoutBlocker`, cancels incomplete subscriptions and expires open sessions inside `SubscriberLockKey` (`service.go:351-437`). `SyncPlan` with a nil tx runs under `PlanLockKey` on a re-read plan (`service.go:693-712`). Customer API: 409 on `CheckoutBlocker`; 422 on `CheckoutPriceID == ""` and on `contracts.ErrPriceUnavailable` (`api/subscription.go:93-171`). Layer: unchanged at v0.2.1; 112/112 TAP. UAT row 4 passed in a browser, covering admin and layer, including Back from Checkout and the subscriber e-mail and title. |
|
||||
| 4 | SC-4: host test proves checkout, activation, invoice.paid, past_due grace, cancel, duplicate idempotency against a Stripe fake | VERIFIED | TestHostSubscriptionFlow passed in this run (postgres stage, pgstripe `./subscriptions`, 12 required tests). |
|
||||
| 5 | SC-5: unit tests delivered in the phase's last plan | VERIFIED | Plan 09 is the last plan, and the gate requires its names by stage and package. The quick tasks are outside the phase's plan sequence. Each carries named tests in its fix commits: pgsub TestCheckoutIncompleteAndOpenSessions, TestCancelTakesSubscriptionLock, TestSyncPlanSerialised, TestCheckoutPriceUnavailable, TestBootBeforeDatabase, TestLabelElementValueProperty; pgstripe TestReviewFixesGateway (4 subtests) and the cancel_at subtests; cabana TestListUnstoredColumn, TestListExtendRecords, TestFormRecordTitle. All passed when run by name here, with 0 skips. The gate does not pin these names (see Anti-Patterns, INFO). |
|
||||
|
||||
**Score:** 5/5 roadmap truths verified (0 present, behavior-unverified at SC level).
|
||||
**Score:** 5/5 roadmap truths verified (0 present, behavior-unverified).
|
||||
|
||||
### Plan must-haves of the gap closure (plans 07 to 09)
|
||||
### Plan must-haves: changes since the previous report
|
||||
|
||||
The previous report verified plans 01 to 09 (65 truths). This run re-checked all of them through the full gate (below). The rows here are the truths whose status or wording changed.
|
||||
|
||||
| Plan | Truth | Status | Evidence |
|
||||
|------|-------|--------|----------|
|
||||
| 07 | CR-01 seq 1: fresh state outside any tx under SubscriptionLockKey; stale event cannot regress an active row | VERIFIED | `job.go applySubscription`; TestFreshStateSmoke/stale_updated_after_active, TestEventOrdering/checkout_race_* |
|
||||
| 07 | CR-01 seq 2: ApplyDeleted on an unstored id wraps ErrSubscriptionNotStored, marker rolled back; late created stores canceled, no activated | VERIFIED | `apply.go` ApplyDeleted `len(rows)==0` branch; TestApplyDeletedNotStored, TestEventOrdering/deleted_before_created (asserts canceled row, 0 activated, 0 then 1 marker) |
|
||||
| 07 | CR-01 seq 3: older queued updated after Service.Cancel fires no reactivated | VERIFIED (coincidental-reliance) | TestEventOrdering/stale_cancel_flag_no_reactivated. It holds when the job runs after Cancel; the race with a job already past its retrieve is open (review WR-01), see `coincidental_reliance_items` |
|
||||
| 07 | Claim-first kept; duplicates and foreign apps make no Stripe call; failed retrieve or Apply leaves no marker; 404 falls back to the snapshot with Warn | VERIFIED | `applySubscription` steps 1-3; TestEventOrdering/duplicate_makes_no_stripe_call, other_app_makes_no_stripe_call, retrieve_error_retries; `RetrieveSubscription` maps 404/resource_missing to ErrSubscriptionMissing |
|
||||
| 07 | WR-04: plan resolution by price, stripe_prices, product, plan_code (inactive incl.); Update/SubscriptionArgs carry product_id/plan_code; job args PII-free | VERIFIED | `apply.go` resolvePlan; TestPlanResolution; TestJobArgsMinimal passed (go stage) |
|
||||
| 07 | WR-06: SyncSubscription retrieves outside tx, locked re-read, gateway-owned columns only, plan from price on retriever path, same lock, no events | VERIFIED | `service.go` SyncSubscription/gatewayFields (code read above); TestSyncSubscriptionFresh incl. the held-lock wait |
|
||||
| 07 | D-21 amended for ordering in READMEs and CONTEXT | VERIFIED | pgsub README:275, pgstripe subscriptions README:141, CONTEXT § Amendments line 68 |
|
||||
| 07 | Compatibility: no route/event/table/column/config/response change; Gateway keeps 8 methods (retriever optional); D-06; go vet/test green | VERIFIED | `contracts.SubscriptionRetriever` is a separate interface; `go list -deps ./...` of pgsub names 0 stripe-go packages (run here); `go test ./...` green in both plugin repos (run here, with Docker) |
|
||||
| 08 | CR-02 API: 422 without a gateway call when CheckoutPriceID is empty; synced map decides alone; unsynced stripe_price_id only in the own currency | VERIFIED | `subscription_plan.go:129-144`; `api/subscription.go:128-131`; TestCheckoutPriceID, TestCustomerAPIGuards |
|
||||
| 08 | CR-02 gateway: CreateCheckoutSession uses CheckoutPriceID, ErrNoPriceForCurrency before any Stripe call; jpy out; StripePriceIDFor keeps PHP fallback | VERIFIED | `gateway.go:199-202`, `:44`; TestGatewayStrictPrice |
|
||||
| 08 | WR-02: customer idempotency key; first-write-wins stored id; WithoutCancel store | VERIFIED | `gateway.go:164`; `user_subscribable.go` SetCustomerID; `service.go` ensureCustomer; TestCustomerIDFirstWriteWins, TestGatewayIdempotencyKeys |
|
||||
| 08 | WR-03: 409 for any non-terminal status; CheckoutGuarded under SubscriberLockKey; session idempotency key per subscriber/plan/currency/params in a 10-min window | VERIFIED | `models/subscription.go` ScopeOpen/TerminalStatuses; `service.go:319-344`; `gateway.go:246`; TestCheckoutGuarded. Residuals in review WR-04 (sessions that are open but unpaid) and WR-07 (incomplete lockout) |
|
||||
| 08 | WR-01: Detached bulk action without tx or row locks; ids saved right after calls, cancellation-proof; sync_all opens no tx; failed save is a failure line | VERIFIED | `pact/capabilities.go:292-300`, `cabana/crud.go:370-373, 412-435`, `subscriptionplans.go` syncOne/runSyncAll; TestDetachedBulkAction (NOWAIT probe), TestAdminSyncDetached (idle-in-transaction probe). Residual: lost per-plan serialisation (review WR-03) |
|
||||
| 08 | WR-07: bfcache restore re-enables the three components; throwing navigation re-enables with generic error; listener removed on unmount | PRESENT_BEHAVIOR_UNVERIFIED | `subscribe.ts` watchPageRestore/navigate/resetState are unit-tested. The component wiring (onMounted/onBeforeUnmount in all three `.vue` files) is present but only source-asserted (review IN-04). Routed to human item 4 |
|
||||
| 08 | Compatibility: shapes unchanged, Service.Checkout unchanged, non-detached bulk actions unchanged, D-06, tests green | VERIFIED | Non-detached branch of `BulkAction` is the prior code path; TestDetachedBulkAction asserts the non-detached case unchanged; framework `go vet ./...` and `go test -short ./...` green (run here) |
|
||||
| 09 | SC-5 named tests in four repos; go vet/test green; layer # fail 0 | VERIFIED | Gate output: go, plugins, postgres and layers stages PASS; `# tests 112 # pass 112 # fail 0` |
|
||||
| 09 | CR-01 proof (TestEventOrdering) | VERIFIED | 7 subtests listed in `ordering_test.go`; passed in postgres stage |
|
||||
| 09 | WR-04/WR-06 proof | VERIFIED | TestPlanResolution, TestApplyDeletedNotStored, TestSyncSubscriptionFresh passed |
|
||||
| 09 | CR-02 proof | VERIFIED | TestCheckoutPriceID (go stage), TestGatewayStrictPrice, TestCustomerAPIGuards passed |
|
||||
| 09 | WR-02/WR-03 proof | VERIFIED | TestCustomerIDFirstWriteWins, TestCheckoutGuarded, TestGatewayIdempotencyKeys, TestCustomerAPIGuards passed |
|
||||
| 09 | WR-01 proof | VERIFIED | TestDetachedBulkAction, TestAdminSyncDetached passed |
|
||||
| 09 | WR-07 proof (layer specs) | VERIFIED | 43 `subscriptions:` TAP tests (minimum raised to 43). The specs cover the flow; the component wiring is text-only, which is the truth row above |
|
||||
| 09 | Gate requires every new name; self-test; PHASE181_REQUIRE_POSTGRES=1 --all green; VALIDATION rows for 07-09, validated, nyquist_compliant | VERIFIED | Run here: EXIT=0, "Phase 18.1 gate passed", 0 skip lines; VALIDATION.md `status: validated`, `nyquist_compliant: true`, 17 references to 18.1-07/08/09 |
|
||||
| 09 | D-22 release in the stated order after the human's reply | VERIFIED | The human's reply was "push-all-with-plugin-tags, v0.2.1", so the tag is v0.2.1 instead of the plan's v0.1.27, which already existed on origin. `v0.2.1` → `f64ae79` and contains `fc2a397` (Detached). pgsub `v0.1.0` → `41b68f6` (requires summercms v0.2.1), pgstripe `v0.1.0` → `2a427fd` (requires summercms v0.2.1 and pgsub v0.1.0), layer `v0.2.1` → `22368c5` (package.json 0.2.1). All tags are on origin (`git ls-remote`); all three sibling repos are `main...origin/main` clean |
|
||||
| 07 | CR-01 seq 3: an older queued updated after Service.Cancel fires no reactivated | VERIFIED | Was `coincidental-reliance`. `Service.Cancel` now calls the gateway and writes the flag inside `WithSessionLock(SubscriptionLockKey)`, so a job that has already retrieved Stripe's state finishes before the cancel starts. TestCancelTakesSubscriptionLock passed: Cancel blocks while another session holds the lock and makes no gateway call until the lock is released. TestFreshStateSmoke/stale_update_after_cancel_no_reactivated and TestEventOrdering/stale_cancel_flag_no_reactivated also passed |
|
||||
| 08 | WR-07: bfcache restore re-enables the three components; listener removed on unmount | VERIFIED (human UAT, row 4) | Was PRESENT_BEHAVIOR_UNVERIFIED. UAT row 4: "playground subscribe, Back from Checkout and 409 pass". The layer's last commit (22368c5, 13:35+02:00) predates the UAT record (15:34Z) |
|
||||
| 02 | ApplyDeleted sets canceled and ended_at and fires canceled unconditionally | VERIFIED (amended) | Changed by the user-accepted fix of review WR-02 (UAT row 5 note: "WR-02 now fires canceled once (deviation from PHP)"). It now fires only on the transition into canceled (`apply.go:241`). On the paths PHP could reach, the listener count is still one. The pgsub README states the deviation (README:36) |
|
||||
| 03 | D-21: PHP's cancel_at_period_end mapping kept; prices for usd, eur and pln | VERIFIED (amended) | D-21 amendment of 2026-10-16 in 18.1-CONTEXT.md:69: `cancel_at` on active or trialing maps to the flag. COVERAGE.md row "subscription cancel_at" is INTEGRATE. Sync currencies are now usd, eur and pln plus every supported currency the plan prices (`sync_plan.go:24-41`, WR-05), a superset of the original. TestReviewFixesGateway/WR-05 passed |
|
||||
| 04 | Subscriptions list with subscriber type, subscriber id, plan, status, period end… | VERIFIED (amended) | G-18.1-4: type and id are still columns, now invisible but searchable, and the visible `subscriber` column shows the e-mail. UAT row 4 retest passed ("subscriber e-mail column, title 'Subscription #1 (Pro)', Gateway/Status shown") |
|
||||
| 08 | WR-03: 409 for any status but canceled/incomplete_expired (incl. incomplete); session idempotency key per subscriber/plan/currency | VERIFIED (amended) | Changed by the user-accepted fixes of review WR-07 and WR-04 (UAT row 5). `incomplete` no longer blocks checkout (`models/subscription.go:133-135`); the guarded checkout cancels it at the gateway first. The session idempotency key was replaced by expiring the customer's open subscription sessions inside the subscriber lock (`gateway.go` `ExpireOpenCheckoutSessions`), which is stronger: at most one payable session. TestCheckoutIncompleteAndOpenSessions, TestCheckoutGuarded and TestReviewFixesGateway/WR-04 passed |
|
||||
| 08 | WR-01 residual (lost per-plan serialisation, review WR-03) | VERIFIED | `SyncPlan` with a nil tx holds `PlanLockKey` and re-reads the plan; `V1Products.Create` carries an idempotency key. TestSyncPlanSerialised (one product from two concurrent syncs) and TestReviewFixesGateway/WR-03 passed |
|
||||
| 08 | CR-02 amount residual (review WR-06) | VERIFIED | `checkPrice` (`gateway.go`) retrieves the price before the session and refuses a missing, inactive, other-currency or other-amount price with `ErrNoPriceForCurrency`, which wraps `contracts.ErrPriceUnavailable` (`gateway.go:33`). The API answers that with 422. TestCheckoutPriceUnavailable and TestReviewFixesGateway/WR-06 passed |
|
||||
| 09 | D-22 release | VERIFIED (for plan 09's scope) | The tags plan 09 recorded are still on origin. The quick-task commits that followed are unreleased: see the human item and the Anti-Patterns WARNING |
|
||||
|
||||
Plans 01 to 06 (41 truths) got a regression check. Every test the first verification relied on passed again in this run's gate. No file the first run verified lost its wiring, and the only changes to those files are the plan 07 and 08 edits verified above.
|
||||
The amended rows follow the precedent set when D-21 was amended for ordering. A user decision recorded in UAT or CONTEXT changes the contract, and the code is checked against the amended contract. Two amendments are recorded only in UAT row 5 and the pgsub README, not in 18.1-CONTEXT.md § Amendments: WR-02 (canceled once) and WR-07 (incomplete no longer blocks). `apply.go:205` says the deviation is "recorded next to the D-21 amendment", but CONTEXT does not record it (INFO).
|
||||
|
||||
**Plan must-have score:** 64/65 verified, 1 present but behavior-unverified (WR-07 component wiring).
|
||||
**Plan must-have score:** 65/65 verified (4 as amended, 1 by human UAT). 0 present-behavior-unverified.
|
||||
|
||||
### Requirements Coverage (phase-local IDs; REQUIREMENTS.md maps no IDs to 18.1)
|
||||
|
||||
| ID | Plans | Status | Evidence |
|
||||
|----|-------|--------|----------|
|
||||
| ID | Plans / tasks | Status | Evidence |
|
||||
|----|---------------|--------|----------|
|
||||
| SC-1 to SC-5 | 01-09 | SATISFIED | Truths 1-5 |
|
||||
| D-01 to D-20, D-22 | 01-09 | SATISFIED | As in the first verification. D-06 re-run: 0 stripe-go deps. D-22: release record above |
|
||||
| D-21 | 03-07 | SATISFIED (amended) | Ordering amended by the user's decision. The cancel_at_period_end mapping, both payload shapes and the latest subscription with flags are unchanged. The portal `cancel_at` question remains a human item |
|
||||
| CR-01, CR-02 | 07, 08, 09 | SATISFIED | Gap-closure truths above |
|
||||
| WR-01, WR-02, WR-03, WR-04, WR-06, WR-07 | 07, 08, 09 | SATISFIED (WR-07 behaviour pending browser check) | Gap-closure truths above |
|
||||
| WR-05 (first review) | none | DEFERRED by user scope | Grace period from the advanced period end, kept PHP-identical (`deferred-items.md`) |
|
||||
| D-01 to D-20, D-22 | 01-09 | SATISFIED | Decision coverage gate: 24/24 honored. D-06 re-run: `go list -deps ./...` in pgsub has 0 stripe-go packages |
|
||||
| D-21 | 03, 07, quick nqk | SATISFIED (amended twice) | Ordering (2026-10-14) and `cancel_at` (2026-10-16), both in CONTEXT § Amendments |
|
||||
| CR-01, CR-02 | 07-09 | SATISFIED | Unchanged; regression-checked by the gate |
|
||||
| Re-review WR-01 to WR-07 | quick 261016-myi | SATISFIED | 18.1-REVIEW-DISPOSITION.md marks all "fixed"; code read above; named tests passed here |
|
||||
| UAT G-18.1-4, G-18.1-7, G-18.1-8 | quick nqk, quick myi | SATISFIED | 18.1-UAT.md `## Gaps`: all `resolved`, with retest passes in rows 4, 7 and 8 |
|
||||
| First-review WR-05 (grace period from the advanced period end) | none | DEFERRED by user scope | `deferred-items.md`, unchanged |
|
||||
|
||||
No orphaned requirements.
|
||||
|
||||
### Required Artifacts (gap closure)
|
||||
### Required Artifacts (changed since the previous report)
|
||||
|
||||
| Artifact | Status | Details |
|
||||
|----------|--------|---------|
|
||||
| `sm-paymentgatewaysubscriptions-plugin/classes/lock.go` | VERIFIED | `pg_advisory_lock(hashtextextended(?,0))` on a pinned connection; WithoutCancel unlock; discards the connection on a lock or unlock failure |
|
||||
| `sm-pgstripe-plugin/subscriptions/gateway.go` | VERIFIED | RetrieveSubscription, ErrSubscriptionMissing, ErrNoPriceForCurrency, SetIdempotencyKey on customer and checkout creates, Gateway.Now |
|
||||
| `sm-pgstripe-plugin/subscriptions/job.go` | VERIFIED | applySubscription under SubscriptionLockKey; deletion under the same lock |
|
||||
| `sm-paymentgatewaysubscriptions-plugin/classes/apply.go` | VERIFIED | ErrSubscriptionNotStored; price, product and plan_code resolution |
|
||||
| `sm-paymentgatewaysubscriptions-plugin/models/subscription_plan.go` | VERIFIED | `func (p SubscriptionPlan) CheckoutPriceID` |
|
||||
| `sm-paymentgatewaysubscriptions-plugin/classes/service.go` | VERIFIED | CheckoutGuarded, Open, locked SyncSubscription, detached SyncPlan save |
|
||||
| `modules/pact/capabilities.go`, `modules/cabana/crud.go` | VERIFIED | `Detached bool json:"-"`; detachedBulkAction branch |
|
||||
| `vue-paymentgateway-layer/app/utils/subscribe.ts` | VERIFIED | `pageshow` with persisted; restore on both flows |
|
||||
| `sm-pgstripe-plugin/subscriptions/ordering_test.go`, `fresh_state_smoke_test.go`, `gateway_hardening_test.go` | VERIFIED | TestEventOrdering, TestFreshStateSmoke, TestGatewayStrictPrice, TestGatewayIdempotencyKeys |
|
||||
| `sm-paymentgatewaysubscriptions-plugin/checkout_guard_test.go` and siblings | VERIFIED | TestCheckoutGuarded etc. |
|
||||
| `modules/cabana/bulk_detached_test.go` | VERIFIED | TestDetachedBulkAction |
|
||||
| `scripts/check-phase18.1.sh` | VERIFIED | requires every new name; ran green |
|
||||
| `18.1-VALIDATION.md` | VERIFIED | validated, nyquist_compliant, rows for 07-09 |
|
||||
| `sm-paymentgatewaysubscriptions-plugin/classes/service.go` | VERIFIED | Cancel under the subscription lock; CheckoutBlocker; cancelIncomplete; expireOpenSessions; SyncPlan under PlanLockKey with a re-read |
|
||||
| `sm-paymentgatewaysubscriptions-plugin/classes/apply.go` | VERIFIED | Transition-only canceled in ApplyDeleted; warnDuplicateOpen at Error |
|
||||
| `sm-paymentgatewaysubscriptions-plugin/boot.go` | VERIFIED | `lagoon.OnDatabase`; grace_period_days still validated at Boot |
|
||||
| `sm-paymentgatewaysubscriptions-plugin/contracts/gateway.go`, `contracts/subscriber.go` | VERIFIED | `CheckoutSessionExpirer`, `ErrPriceUnavailable`, `SubscriberBatchResolver` (optional interfaces; `Gateway` keeps its methods) |
|
||||
| `sm-paymentgatewaysubscriptions-plugin/controllers/subscriptions.go`, `models/subscription/columns.yaml`, `assets/js/fields.js` | VERIFIED | ListExtendRecords, FormRecordTitle, subscriber column, label `value` property |
|
||||
| `sm-pgstripe-plugin/subscriptions/decode.go` | VERIFIED | `CancelAt` decoded, `pendingCancel()` used by `args()` |
|
||||
| `sm-pgstripe-plugin/subscriptions/gateway.go`, `sync_plan.go` | VERIFIED | checkPrice, ExpireOpenCheckoutSessions, product idempotency key, planCurrencies |
|
||||
| `summercms.go/modules/pact/capabilities.go`, `modules/cabana/{list_schema,query,crud,relation_field}.go`, `admin/src/views/{FormView,PreviewView}.vue` | VERIFIED | Generic hooks; unstored columns compile unsortable; `meta.title`; OpenAPI and dist checks clean |
|
||||
|
||||
### Key Link Verification (gap closure)
|
||||
### Key Link Verification (new links)
|
||||
|
||||
| From | To | Via | Status |
|
||||
|------|----|-----|--------|
|
||||
| `subscriptions/job.go` | `subscriptions/gateway.go` | `NewGateway(p.app).RetrieveSubscription` inside the lock, before `lagoon.Transaction` | WIRED |
|
||||
| `subscriptions/job.go` | `classes/lock.go` | `classes.WithSessionLock(ctx, db, classes.SubscriptionLockKey(id), ...)` (created/updated and deleted) | WIRED |
|
||||
| `classes/service.go` SyncSubscription | `contracts/gateway.go` | `gw.(contracts.SubscriptionRetriever)` | WIRED |
|
||||
| `api/subscription.go` Checkout | `classes/service.go` | `d.svc.CheckoutGuarded`, `errors.Is(err, classes.ErrAlreadySubscribed)` → 409 | WIRED |
|
||||
| `subscriptions/gateway.go` CreateCheckoutSession | `models/subscription_plan.go` | `plan.CheckoutPriceID(currency)` | WIRED |
|
||||
| `controllers/subscriptionplans.go` | `modules/cabana/crud.go` | `Detached: true` → `detachedBulkAction` | WIRED |
|
||||
| layer `SubscribeButton.vue`, `ManageSubscriptionButton.vue`, `MySubscription.vue` | `utils/subscribe.ts` | `onMounted` `watchPageRestore(window, ...)`, `onBeforeUnmount(stopRestore)` | WIRED (behaviour unexercised at component level) |
|
||||
| `scripts/check-phase18.1.sh` | `ordering_test.go`, `bulk_detached_test.go` | required names in the postgres stage | WIRED |
|
||||
| plugin `go.mod` files | summercms tag | `git.golem15.com/golem15/summercms v0.2.1` (the plan's pattern said v0.1.27; the human chose v0.2.1) | WIRED |
|
||||
| `api/subscription.go` Checkout | `classes/service.go` | `d.svc.CheckoutBlocker` (409) then `CheckoutGuarded`; `errors.Is(err, contracts.ErrPriceUnavailable)` → 422 | WIRED |
|
||||
| `classes/service.go` CheckoutGuarded | pgstripe `Gateway.ExpireOpenCheckoutSessions` | `gw.(contracts.CheckoutSessionExpirer)` | WIRED (TestReviewFixesGateway/WR-04 through the fake) |
|
||||
| pgstripe `ErrNoPriceForCurrency` | `contracts.ErrPriceUnavailable` | `fmt.Errorf("...: %w", contracts.ErrPriceUnavailable)` (`gateway.go:33`) | WIRED |
|
||||
| `decode.go` `args()` | snapshot, fresh retrieve, legacy SyncSubscription | `pendingCancel()` | WIRED (all three paths share `args()`) |
|
||||
| `controllers/subscriptions.go` | framework cabana | `var _ pact.ListExtendRecords`, `var _ pact.FormRecordTitle` | WIRED (TestAdminSubscriptionsSmoke, TestListExtendRecords, TestFormRecordTitle) |
|
||||
| `classes/user_subscribable.go` | `controllers/subscriptions.go` | `contracts.SubscriberBatchResolver.ResolveMany` | WIRED (TestUserSubscribable) |
|
||||
| `boot.go` bootService | `lagoon.OnDatabase` | callback publishes `*classes.Service` | WIRED (TestBootBeforeDatabase, UAT 8) |
|
||||
|
||||
### Data-Flow Trace (Level 4)
|
||||
|
||||
| Artifact | Data | Source | Real data | Status |
|
||||
|----------|------|--------|-----------|--------|
|
||||
| Webhook job → row | subscription state | `V1Subscriptions.Retrieve` → `decodeSubscription` → `Apply` (the snapshot only on a 404) | yes | FLOWING |
|
||||
| POST /subscription/checkout | Stripe price id | `stripe_prices[currency]`, or `stripe_price_id` in the plan's own currency → session create | yes. The amount is the one fixed at the last sync, not `currency_prices` (review WR-06) | FLOWING |
|
||||
| GET /plans, GET /subscription | plans, latest subscription | GORM queries (unchanged) | yes | FLOWING |
|
||||
| Webhook job → row | subscription state incl. pending cancel | `V1Subscriptions.Retrieve` → `decodeSubscription` → `args()` → `Apply` | yes (UAT 6, 7 live) | FLOWING |
|
||||
| POST /subscription/checkout | Stripe price | `CheckoutPriceID` → `V1Prices.Retrieve` check → session create | yes | FLOWING |
|
||||
| Admin subscriptions list | subscriber e-mail | `ResolveMany` (one query per alias) → `Subscription.Subscriber` (`gorm:"-"`) | yes (UAT 4 retest, demo API showed `customer@example.test`) | FLOWING |
|
||||
| Admin preview | `meta.title` | `FormRecordTitle` → plan name query | yes | FLOWING |
|
||||
|
||||
### Behavioral Spot-Checks and Probe Execution
|
||||
|
||||
| Behavior | Command | Result | Status |
|
||||
|----------|---------|--------|--------|
|
||||
| Full phase gate, Postgres required | `PHASE181_REQUIRE_POSTGRES=1 bash scripts/check-phase18.1.sh --all` | EXIT=0. PASS: self-test, go, plugins (vet both plugins), admin (vitest 1429), layers (112/112, 43 subscriptions), docs (docs:build clean, admin dist matches), postgres (cabana 4, pgsub 21, pgstripe root 4, subscriptions 12). "Phase 18.1 gate passed"; 0 skip lines | PASS |
|
||||
| Plugin suites with Docker | `go test ./...` in pgsub and pgstripe | all ok | PASS |
|
||||
| Framework | `go vet ./...`; `go test -short ./...` | no output except ok lines | PASS |
|
||||
| D-06 | `go list -deps ./...` in pgsub, grep stripe-go | 0 | PASS |
|
||||
| Release tags | `git ls-remote --tags origin` in four repos; `merge-base --is-ancestor fc2a397 v0.2.1` | v0.2.1, v0.1.0, v0.1.0, v0.2.1 present; Detached contained | PASS |
|
||||
| Full phase gate, Postgres required | `PHASE181_REQUIRE_POSTGRES=1 bash scripts/check-phase18.1.sh --all` | EXIT=0. PASS self-test, go, plugins, admin (vitest 1431 in 93 files), layers (112/112, 43 `subscriptions:`), docs (docs:build clean, OpenAPI and admin dist match), postgres (cabana 4, pgsub 21, pgstripe root 4, subscriptions 12). "Phase 18.1 gate passed" | PASS |
|
||||
| Quick-task tests, pgsub (Docker) | `go test -count=1 -v -run '^(TestCheckoutIncompleteAndOpenSessions\|TestCancelTakesSubscriptionLock\|TestSyncPlanSerialised\|TestCheckoutPriceUnavailable\|TestBootBeforeDatabase\|TestLabelElementValueProperty\|TestUserSubscribable\|TestAdminSubscriptionsSmoke\|TestApplyEvents)$' .` | 9/9 PASS, 0 SKIP | PASS |
|
||||
| Quick-task tests, pgstripe (Docker) | `go test -count=1 -v -run '^(TestReviewFixesGateway\|TestDecodeSubscription\|TestFreshStateSmoke\|TestEventOrdering\|TestJobArgsMinimal)$' ./subscriptions` | all PASS incl. portal_cancel_at_only_then_uncancel, the 4 WR subtests, deleted_before_created; 0 SKIP | PASS |
|
||||
| Framework hooks (Docker) | `go test -count=1 -v -run '^(TestListUnstoredColumn\|TestListExtendRecords\|TestFormRecordTitle)$' ./modules/cabana` | 3/3 PASS | PASS |
|
||||
| Framework | `go vet ./...`; `go test -short ./...` | both exit 0, no failures | PASS |
|
||||
| Plugins vet | `go vet ./...` in pgsub and pgstripe | clean | PASS |
|
||||
| D-06 | `go list -deps ./...` in pgsub, count stripe-go | 0 | PASS |
|
||||
| Plugin build outside the workspace | `GOWORK=off GOPROXY=off go build ./...` in pgsub | builds only because pgsub's own go.mod has `replace .../summercms => ../summercms.go`; `go list -m` resolves summercms "v0.2.1" to the local checkout | INFO (see Anti-Patterns) |
|
||||
|
||||
### Code Review Findings Weighed (18.1-REVIEW.md, re-review 2026-10-16: 0 critical, 7 warnings, 5 info)
|
||||
### Test Quality Audit (new tests)
|
||||
|
||||
The review confirms that both blockers from the first review are closed. I read the code paths it cites. Each warning below is a residual of, or a side effect of, a fix. None makes a roadmap SC or a plan must-have false as worded.
|
||||
| Test | Linked item | Skipped | Assertion level | Verdict |
|
||||
|------|-------------|---------|-----------------|---------|
|
||||
| TestCancelTakesSubscriptionLock | WR-01 | no | behavioral (blocks on held lock, no gateway call before release, flag stored) | OK |
|
||||
| TestCheckoutIncompleteAndOpenSessions | WR-04, WR-07 | no | behavioral | OK |
|
||||
| TestSyncPlanSerialised | WR-03 | no | behavioral (one product from two syncs) | OK |
|
||||
| TestCheckoutPriceUnavailable, TestReviewFixesGateway/WR-06 | WR-06 | no | value | OK |
|
||||
| ordering/fresh-state `canceled == 1` | WR-02 | no | value (event counts) | OK |
|
||||
| TestFreshStateSmoke/portal_cancel_at_only_then_uncancel | G-18.1-7 | no | behavioral (no event on cancel, reactivated once on un-cancel) | OK |
|
||||
| TestBootBeforeDatabase | G-18.1-8 | no | behavioral (activation before DB publish) | OK |
|
||||
| TestLabelElementValueProperty | G-18.1-4 | skips only without `node`; ran here | value | OK |
|
||||
|
||||
- **WR-01 (Cancel without the subscription lock).** Confirmed: `Service.Cancel` (`service.go:373-395`) writes the flag through `s.db` with no lock. This is a narrow race in the CR-01 sequence 3 truth, recorded under `coincidental_reliance_items`. The sequential case is tested and holds.
|
||||
- **WR-02 (SubscriptionCanceled twice).** Confirmed by reading:
|
||||
- the late created event stores `canceled` from no previous status, so `fireStateChangeEvents` fires canceled;
|
||||
- the retried `ApplyDeleted` then fires unconditionally.
|
||||
|
||||
`deleted_before_created` asserts 0 activated but never counts canceled. PHP never reached this state. On PHP the late created would have stored the snapshot (active), so Go is strictly safer on access, but a listener runs twice. WARNING.
|
||||
- **WR-03 (plan sync serialisation lost).** Confirmed: the detached path loads with `loadScoped(..., false)`, and `SyncPlan` takes no lock. WARNING.
|
||||
- **WR-04 (unpaid sessions) and WR-07 (incomplete lockout).** Both follow from the plan 08 guard design that the must-have spells out ("incomplete" is listed as open). They are product trade-offs. WARNING.
|
||||
- **WR-05 (non-sync currencies unbuyable after a sync) and WR-06 (amount drift).** Both are in the new customer API's money path. Horoskopia's checkout is `pln`, a sync currency. WARNING.
|
||||
- **IN-01 to IN-05.** Informational only.
|
||||
|
||||
They are collected as human item 1 for a disposition before the cutover.
|
||||
No disabled tests, no circular expected values. The Stripe fake's responses are hand-written, and UAT rows 6 and 7 back the mapping with real Stripe bodies.
|
||||
|
||||
### Anti-Patterns Found
|
||||
|
||||
| File | Line | Pattern | Severity | Impact |
|
||||
|------|------|---------|----------|--------|
|
||||
| sm-paymentgatewaysubscriptions-plugin/classes/service.go | 373-395 | gateway-state write without SubscriptionLockKey (review WR-01) | WARNING | spurious `reactivated` reachable by a race |
|
||||
| sm-paymentgatewaysubscriptions-plugin/classes/apply.go | 208-211 | unconditional SubscriptionCanceled after a fresh-state canceled apply (review WR-02) | WARNING | listener runs twice |
|
||||
| sm-paymentgatewaysubscriptions-plugin/classes/service.go | 584-611 | SyncPlan unserialised on the detached path (review WR-03) | WARNING | duplicate Stripe products/prices on concurrent syncs |
|
||||
| sm-paymentgatewaysubscriptions-plugin/models/subscription_plan.go | 129-144 | strict map vs fixed usd/eur/pln sync list; no amount check (review WR-05, WR-06) | WARNING | gbp/cad/aud/chf unsellable after a sync; stale amount billed |
|
||||
| vue-paymentgateway-layer/tests/subscriptionComponents.test.ts | 423-440 | wiring asserted as source text (review IN-04) | INFO | component bfcache behaviour unexercised (human item 4) |
|
||||
| sm-paymentgatewaysubscriptions-plugin/go.mod | 7, 149 | requires `summercms v0.2.1`, but HEAD calls `pact.ListExtendRecords` and `pact.FormRecordTitle`, which are in no framework tag; it builds only through its own `replace ../summercms.go`, which a consuming module ignores | WARNING | pgsub HEAD cannot be consumed by tag or pseudo-version until the framework is tagged and the require bumped. This is the human release item |
|
||||
| sm-paymentgatewaysubscriptions-plugin, tag v0.1.0 | n/a | released plugin lacks G-18.1-8 | WARNING | an app built with `summer build` on pgsub v0.1.0 fails every command ("database is not published"). Fixed on main (ab46d6f), unreleased |
|
||||
| scripts/check-phase18.1.sh | required names | the quick-task tests (above) are not in the gate's required lists; 18.1-VALIDATION.md has no rows for them | INFO | the gate would not notice if they were skipped or removed. They passed here by name |
|
||||
| .planning/phases/18.1-…/18.1-CONTEXT.md | § Amendments | WR-02 (canceled once) and WR-07 (incomplete no longer blocks) deviations not recorded; `apply.go:205` claims they are "recorded next to the D-21 amendment" | INFO | decision trail sits in UAT row 5 and the README only |
|
||||
| docs/backend/forms.md | FormRecordTitle paragraph | the example title "Subscription #12 (Pro)" | INFO | generic concept, no application named; the framework rule holds |
|
||||
|
||||
No TBD, FIXME, XXX, TODO or HACK markers in the phase's non-test source files I scanned (pgsub classes, api, models and controllers; pgstripe subscriptions; layer subscribe.ts; cabana crud.go; pact capabilities.go).
|
||||
No TBD, FIXME, XXX, TODO or HACK markers in the files these quick tasks changed: pgsub classes, api, models, contracts, controllers, boot and assets; pgstripe subscriptions gateway, decode, sync_plan and job; cabana list_schema, query, crud and relation_field; pact capabilities; FormView and PreviewView.
|
||||
|
||||
IN-01 to IN-05 of the re-review stay open as info. The disposition records them. IN-03 (502 instead of 422 on a gateway price refusal) is now closed in effect: the API maps `ErrPriceUnavailable` to 422.
|
||||
|
||||
### Prohibitions (judgment tier, autonomous LLM verdict, non-authoritative)
|
||||
|
||||
| Plan | Prohibition | Verdict |
|
||||
|------|-------------|---------|
|
||||
| 07 | No Stripe call while a database transaction is open in the job or SyncSubscription | holds by reading: the retrieve runs inside WithSessionLock, before lagoon.Transaction, in both places. Event listeners inside Apply are outside this plan's code |
|
||||
| 07 | pgsub imports no stripe-go (D-06) | holds (0 deps, run here) |
|
||||
| 07, 08 | Nothing pushed or tagged by these plans | holds: the tags sit on plan 09's commits. Flagged as unverifiable from git timestamps alone |
|
||||
| 08 | No customer API body, status for an existing case, route or event name changes | holds: the 409 and 422 reuse `errAlreadySubscribed` and `writeValidation`. The 409 now also covers past_due, unpaid, incomplete and paused, which the must-have specifies. Flagged |
|
||||
| 08 | Framework Detached addition names no app, provider or subscription concept | holds: the `fc2a397` and `fa91e2a` diffs contain none of stripe, subscri, payment or app names |
|
||||
| 08 | Another session's edits never staged or overwritten | not independently verifiable. The current summercms.go working tree holds other sessions' uncommitted STATE.md and state.json changes, untouched. Flagged |
|
||||
| 09 | No test calls the network; test-only secrets | holds per the Stripe fake (BackendOverride). Not audited line by line. Flagged |
|
||||
| 09 | Gate never green on a skip or missing Docker | holds: PHASE181_REQUIRE_POSTGRES=1, the self-test passed, 0 skip lines |
|
||||
| 09 | Nothing pushed or tagged without the human's reply; no force-push | the reply is recorded verbatim in the 09 SUMMARY. Old tags v0.1.27 and v0.2.0 are untouched. Flagged |
|
||||
| Source | Prohibition | Verdict |
|
||||
|--------|-------------|---------|
|
||||
| 02, 04, 07 | pgsub imports no stripe-go (D-06) | holds (0 deps, run here) |
|
||||
| 01, 08 | Framework code and docs name no consuming application; generic additions only | holds. The bc5685e and 013890c diffs name no app or provider. One docs example uses "Subscription #12 (Pro)" as a sample title (INFO) |
|
||||
| 07 | No Stripe call while a DB transaction is open in the job or SyncSubscription | holds by reading. The new Cancel and CheckoutGuarded Stripe calls also run under session locks with no transaction open |
|
||||
| 02 | Built-in user subscribable writes no users column other than stripe_customer_id | holds. `ResolveMany` only reads (TestUserSubscribable) |
|
||||
| 03, 06, 09 | No test calls the network; test-only secrets | holds per the fake (BackendOverride); the new subtests use the same fake. Not audited line by line. Flagged |
|
||||
| 06, 09 | Gate never green on a skip or missing Docker | holds: PHASE181_REQUIRE_POSTGRES=1, 0 skip lines |
|
||||
| 09, quick tasks | Nothing pushed or tagged without the user's reply | holds: sibling repos are ahead of origin (pgsub 4, pgstripe 2), and no new tags exist. Hence the human release item |
|
||||
| 08 | Another session's uncommitted edits never staged | the working tree still holds other sessions' STATE.md and state.json edits, untouched. Flagged |
|
||||
|
||||
### Decision Coverage
|
||||
|
||||
All trackable CONTEXT.md decisions are honored by shipped artifacts (24/24).
|
||||
|
||||
### Human Verification Required
|
||||
|
||||
UAT evidence (Step 8b): `18.1-UAT.md` (2026-10-14) has all four rows pending, so no row settles an item. Row 1, the CR-01/CR-02 decision, was answered by the user's decision recorded in 18.1-CONTEXT.md § Amendments, and the gap closure implemented that decision, so it is dropped here. The UAT file is left as is.
|
||||
UAT evidence (Step 8b): `18.1-UAT.md` is `complete`, recorded at 2026-10-16T15:34:36Z. `verification.uat-evidence` reports `changed_since_uat: []` for the covered framework files. The sibling repos' last commits are also older than the record: pgsub 17:22+02, pgstripe 17:07+02, layer 13:35+02. Their working trees are clean. Rows 4, 5, 6, 7 and 8 passed on that code and settle every human item of the previous report (see `human_verified`). Rows 2 and 3 are skipped duplicates of 6 and 7. No row needs a retest.
|
||||
|
||||
1. **Disposition of the re-review warnings.** Decide WR-01 to WR-07 before the Horoskopia cutover. The most relevant are WR-02 (canceled fires twice), WR-01 (cancel race), WR-03 (duplicate products) and WR-06 (stale amount).
|
||||
2. **Live Stripe test-mode checkout.** Confirm the fresh-retrieve path against real Stripe (`source=stripe` in the job log), a single `activated` and the 409 on a second subscribe.
|
||||
3. **Billing-portal cancellation.** Check whether Stripe reports `cancel_at_period_end` or `cancel_at` (D-21).
|
||||
4. **Browser pass.** Cover the admin screens and the layer components, and retest Back from Checkout and the portal now that WR-07 is fixed. This item also covers the one present-but-behavior-unverified truth.
|
||||
One new item remains:
|
||||
|
||||
1. **Release go-ahead.** Tag the framework v0.2.2, bump and tag pgsub, then pgstripe, and push all three. Until then the tagged pgsub v0.1.0 does not boot in a `summer build` app, and pgsub main cannot be required by a consumer. This needs the user's go-ahead under the project rule. If the user defers it, record the reason (for example "released together with the Horoskopia port").
|
||||
|
||||
### Gaps Summary
|
||||
|
||||
No success criterion or plan must-have failed. The gap closure did what it set out to do:
|
||||
- **CR-01:** webhook jobs apply Stripe's current state under a per-subscription session lock, outside any transaction, and a deletion that arrives before creation is retried.
|
||||
- **CR-02:** checkout never bills another currency's price.
|
||||
- **WR-01, WR-02, WR-03, WR-04, WR-06 and WR-07:** each has code and a named test, and the gate requires all of them.
|
||||
No success criterion and no plan must-have failed. The two quick tasks did what they claim:
|
||||
- **WR-01:** Cancel is serialised with the webhook job.
|
||||
- **WR-02:** `canceled` fires once.
|
||||
- **WR-03:** plan syncs are serialised, and the product create is idempotent.
|
||||
- **WR-04:** open checkout sessions are expired.
|
||||
- **WR-05:** non-sync currencies stay sellable.
|
||||
- **WR-06:** a stale price is refused with 422.
|
||||
- **WR-07:** `incomplete` no longer locks a user out.
|
||||
- **G-18.1-7:** a portal `cancel_at` shows as will-cancel.
|
||||
- **G-18.1-4:** the admin shows the subscriber's e-mail and a record title.
|
||||
- **G-18.1-8:** pgsub boots in a summer-built app.
|
||||
|
||||
The full gate, with Postgres required, passed in this run, and so did both plugin suites and the framework's vet and short tests. The release (summercms v0.2.1, pgsub v0.1.0, pgstripe v0.1.0, layer v0.2.1) is on origin, and the plugin requires point at it.
|
||||
Each fix has a named test that passed in this run, and the live UAT passed on the same code.
|
||||
|
||||
The status is `human_needed`, not `passed`, for three reasons:
|
||||
- **Live and browser items.** The three manual checks from the first verification are still open: live Stripe checkout, portal `cancel_at`, and the browser pass, which now also confirms the WR-07 fix.
|
||||
- **Component wiring untested.** The WR-07 component wiring is present but not exercised by any behavioural test.
|
||||
- **Re-review warnings.** The review found seven new warnings. They are side effects or residuals of the fixes, and some touch money or event listeners (double `canceled`, the cancel race, duplicate Stripe products on concurrent syncs, stale amounts). They do not defeat the goal as worded, but deciding whether they block the Horoskopia cutover is a product call.
|
||||
The status is `human_needed` only because the fixes are not released. The tagged plugins still carry the boot blocker, and pgsub main depends on framework hooks that no tag has. Releasing needs the user's go-ahead.
|
||||
|
||||
---
|
||||
|
||||
_Verified: 2026-10-16T14:45:00Z_
|
||||
_Verified: 2026-10-16T15:45:06Z_
|
||||
_Verifier: Claude (msd-verifier)_
|
||||
|
||||
Reference in New Issue
Block a user