docs(18.1): add code review report

This commit is contained in:
Jakub Zych
2026-10-14 21:24:17 +02:00
parent a9dd5656d9
commit 929cfd8669

View File

@@ -0,0 +1,434 @@
---
phase: 18.1-payment-gateway-subscriptions
reviewed: 2026-10-14T19:22:09Z
depth: standard
files_reviewed: 96
files_reviewed_list:
- summercms.go/admin/src/components/form/PreviewField.vue
- summercms.go/admin/src/components/form/fields/TaglistField.vue
- summercms.go/admin/src/components/form/registry.ts
- summercms.go/admin/src/components/reorder/ReorderBranch.vue
- summercms.go/admin/src/components/reorder/tree.ts
- summercms.go/admin/src/views/ReorderView.vue
- summercms.go/modules/cabana/admin_openapi.go
- summercms.go/modules/cabana/crud.go
- summercms.go/modules/cabana/field_file.go
- summercms.go/modules/cabana/field_taglist.go
- summercms.go/modules/cabana/form_schema.go
- summercms.go/modules/cabana/list_schema.go
- summercms.go/modules/cabana/registry.go
- summercms.go/modules/cabana/relation_child.go
- summercms.go/modules/cabana/relation_form.go
- summercms.go/modules/cabana/reorder.go
- summercms.go/modules/cabana/schema_types.go
- summercms.go/modules/cabana/settings.go
- summercms.go/scripts/check-phase18.1.sh
- sm-paymentgatewaysubscriptions-plugin/.gitignore
- sm-paymentgatewaysubscriptions-plugin/admin.go
- sm-paymentgatewaysubscriptions-plugin/admin_navigation.go
- sm-paymentgatewaysubscriptions-plugin/admin_permissions.go
- sm-paymentgatewaysubscriptions-plugin/api/errors.go
- sm-paymentgatewaysubscriptions-plugin/api/json.go
- sm-paymentgatewaysubscriptions-plugin/api/plans.go
- sm-paymentgatewaysubscriptions-plugin/api/subscription.go
- sm-paymentgatewaysubscriptions-plugin/api/urls.go
- sm-paymentgatewaysubscriptions-plugin/assets/js/fields.js
- sm-paymentgatewaysubscriptions-plugin/boot.go
- sm-paymentgatewaysubscriptions-plugin/classes/apply.go
- sm-paymentgatewaysubscriptions-plugin/classes/service.go
- sm-paymentgatewaysubscriptions-plugin/classes/user_subscribable.go
- sm-paymentgatewaysubscriptions-plugin/config/config.yaml
- sm-paymentgatewaysubscriptions-plugin/contracts/gateway.go
- sm-paymentgatewaysubscriptions-plugin/contracts/registry.go
- sm-paymentgatewaysubscriptions-plugin/contracts/subscriber.go
- sm-paymentgatewaysubscriptions-plugin/controllers/admin_registry.go
- sm-paymentgatewaysubscriptions-plugin/controllers/base.go
- sm-paymentgatewaysubscriptions-plugin/controllers/subscriptionplans.go
- sm-paymentgatewaysubscriptions-plugin/controllers/subscriptionplans/config_form.yaml
- sm-paymentgatewaysubscriptions-plugin/controllers/subscriptionplans/config_list.yaml
- sm-paymentgatewaysubscriptions-plugin/controllers/subscriptionplans/config_reorder.yaml
- sm-paymentgatewaysubscriptions-plugin/controllers/subscriptions.go
- sm-paymentgatewaysubscriptions-plugin/controllers/subscriptions/_subscription_plan.htm
- sm-paymentgatewaysubscriptions-plugin/controllers/subscriptions/_subscription_subscriber.htm
- sm-paymentgatewaysubscriptions-plugin/controllers/subscriptions/config_filter.yaml
- sm-paymentgatewaysubscriptions-plugin/controllers/subscriptions/config_form.yaml
- sm-paymentgatewaysubscriptions-plugin/controllers/subscriptions/config_list.yaml
- sm-paymentgatewaysubscriptions-plugin/events/events.go
- sm-paymentgatewaysubscriptions-plugin/field_types.go
- sm-paymentgatewaysubscriptions-plugin/go.mod
- sm-paymentgatewaysubscriptions-plugin/go.work
- sm-paymentgatewaysubscriptions-plugin/models/feature_list.go
- sm-paymentgatewaysubscriptions-plugin/models/filters.go
- sm-paymentgatewaysubscriptions-plugin/models/processed_webhook_event.go
- sm-paymentgatewaysubscriptions-plugin/models/registry.go
- sm-paymentgatewaysubscriptions-plugin/models/subscription.go
- sm-paymentgatewaysubscriptions-plugin/models/subscription/columns.yaml
- sm-paymentgatewaysubscriptions-plugin/models/subscription/fields.yaml
- sm-paymentgatewaysubscriptions-plugin/models/subscription_plan.go
- sm-paymentgatewaysubscriptions-plugin/models/subscriptionplan/columns.yaml
- sm-paymentgatewaysubscriptions-plugin/models/subscriptionplan/fields.yaml
- sm-paymentgatewaysubscriptions-plugin/plugin.go
- sm-paymentgatewaysubscriptions-plugin/routes.go
- sm-paymentgatewaysubscriptions-plugin/updates/202610150001_create_golem15_pgsub_baseline.go
- sm-paymentgatewaysubscriptions-plugin/updates/202610150002_add_stripe_customer_id_to_users.go
- sm-paymentgatewaysubscriptions-plugin/updates/registry.go
- sm-pgstripe-plugin/client.go
- sm-pgstripe-plugin/config/config.yaml
- sm-pgstripe-plugin/console/import_winter.go
- sm-pgstripe-plugin/go.mod
- sm-pgstripe-plugin/go.work
- sm-pgstripe-plugin/models/settings.go
- sm-pgstripe-plugin/models/settings/fields.yaml
- sm-pgstripe-plugin/subscriptions/config/config.yaml
- sm-pgstripe-plugin/subscriptions/decode.go
- sm-pgstripe-plugin/subscriptions/gateway.go
- sm-pgstripe-plugin/subscriptions/job.go
- sm-pgstripe-plugin/subscriptions/plugin.go
- sm-pgstripe-plugin/subscriptions/sync_plan.go
- sm-pgstripe-plugin/subscriptions/webhook.go
- sm-pgstripe-plugin/updates/202610150001_add_subscription_webhook_secrets.go
- vue-paymentgateway-layer/.playground/app/pages/subscriptions/index.vue
- vue-paymentgateway-layer/app/assets/css/pg-base.css
- vue-paymentgateway-layer/app/components/Pg/ManageSubscriptionButton.vue
- vue-paymentgateway-layer/app/components/Pg/MySubscription.vue
- vue-paymentgateway-layer/app/components/Pg/Plans.vue
- vue-paymentgateway-layer/app/components/Pg/SubscribeButton.vue
- vue-paymentgateway-layer/app/composables/usePlans.ts
- vue-paymentgateway-layer/app/composables/useSubscription.ts
- vue-paymentgateway-layer/app/composables/useSubscriptionsApi.ts
- vue-paymentgateway-layer/app/utils/subscribe.ts
- vue-paymentgateway-layer/app/utils/subscriptionsClient.ts
- vue-paymentgateway-layer/nuxt.config.ts
- vue-paymentgateway-layer/package.json
findings:
critical: 2
warning: 7
info: 8
total: 17
status: issues_found
---
# Phase 18.1: Code Review Report
**Reviewed:** 2026-10-14T19:22:09Z
**Depth:** standard
**Files Reviewed:** 96 (four repositories; every path below carries its repository prefix)
**Status:** issues_found
## Summary
The review covered the cabana taglist field and flat reorder (framework and admin SPA), the pgsub core, its admin screens and customer API, the Stripe subscription gateway with its webhook, River job and plan sync, and the Nuxt layer pieces.
The framework half is in good shape. The taglist save path normalises and checks values at every save site before any transaction. The flat reorder writes only the sort column, and a submitted parent is refused. The webhook verifies the signature on the raw body before decoding it, uses a separate secret per endpoint, and claims the processed-event marker inside the transaction that applies the event. The customer API never takes a subscriber id, a subscription id or a redirect URL from the request.
Two money-relevant defects block shipping:
1. **CR-01: stale webhook data can overwrite newer data.** The webhook job applies the subscription snapshot carried in each Stripe event, with nothing to tell an old event from a new one. Stripe delivers events out of order, and River runs and retries jobs concurrently. A paid subscription can drop back to `incomplete` and lose access. A deleted subscription can come back as active.
2. **CR-02: checkout can charge a different currency or amount than the user was shown.** The customer API checks the currency against the plan's local prices. The gateway then silently falls back to `stripe_price_id`, which plan sync sets to the USD price.
The warnings cover:
- Stripe calls made while holding a database transaction and row locks.
- Duplicate Stripe customers and duplicate subscriptions under concurrent or repeated checkout.
- Subscriptions that are dropped after a plan is repriced.
- A grace period inherited from PHP that grants about a full unpaid period.
- A lost-update risk in SyncSubscription.
- Subscribe and manage buttons in the layer that stay disabled after the browser Back button.
Byte compatibility: the webhook's five plain-text answers, their order, the event names, the table, column and constraint names and the permission codes match what the SUMMARYs claim. The documented exception is `text/plain` instead of Laravel's `text/html`. CR-01, CR-02 (the usd/eur/pln loop and its fallback) and WR-05 are defects inherited from the frozen PHP source. They are reported anyway, because the Go port adds concurrent workers and a new customer API that make them easier to reach.
## Narrative Findings (AI reviewer)
## Critical Issues
### CR-01: Webhook job applies stale event snapshots with no ordering guard, so a newer subscription state can be overwritten by an older event
**Files:**
- `sm-pgstripe-plugin/subscriptions/job.go:119-147, 155-170, 187-205`
- `sm-paymentgatewaysubscriptions-plugin/classes/apply.go:28-126` (lookup and lock at 52, unconditional status write at 96, reactivated diff at 138), `apply.go:170-205` (unknown row at 183)
- `sm-paymentgatewaysubscriptions-plugin/classes/service.go:308-330`
**Issue:** `work` maps `data.object` from the Stripe event into `contracts.Update`, and `Apply` writes that status, period and cancel flag onto the row as they are. Neither the event's `created` timestamp nor any version is decoded (the `event` struct in `decode.go:30-36` has only id, type and data), so nothing stops an older event from winning. Stripe does not guarantee delivery order, and River runs several workers and retries failed jobs with backoff. Several realistic sequences break:
1. **Checkout race on a new subscription.** Stripe Checkout emits `customer.subscription.created` (status `incomplete`) and `customer.subscription.updated` (status `active`) within milliseconds.
- Both jobs run `SELECT ... FOR UPDATE`, which finds no row and so locks nothing, and both INSERT.
- If the `updated` job wins the insert, it stores `active` and fires `activated`. The `created` job fails on the unique index on `stripe_subscription_id`, rolls back its marker and is retried.
- On retry it finds the row and writes `incomplete` over `active`. No event fires.
- The customer has paid but `HasAccess` is false until Stripe's next update, typically the next renewal a month later.
2. **Deleted before created.** If the `created` job is retrying (for example "no plan holds price id" until an admin syncs the plan) and `customer.subscription.deleted` arrives:
- `ApplyDeleted` finds no row, logs a warning and returns `nil, nil`, so the deletion marker is committed.
- The `created` retry then inserts the row as `active`.
- The canceled subscription grants access with no end, and `activated` fires.
3. **Spurious `reactivated`.** `Service.Cancel` writes `cancel_at_period_end = true` locally (`service.go:324-325`). An older queued `customer.subscription.updated` carrying `cancel_at_period_end: false` then meets `previousCancel && !sub.CancelAtPeriodEnd && active` (`apply.go:138`) and fires `SubscriptionReactivated` for a cancellation the user never withdrew.
PHP's job had the same snapshot semantics. The Go port adds concurrent workers and retry-with-backoff, which makes the race in sequence 1 likely on every Checkout rather than rare.
**Fix:** Apply authoritative state rather than the snapshot, as Stripe recommends. In the job, outside the transaction, retrieve the subscription from Stripe (the gateway already has the code in `SyncSubscription`) and feed that into `Apply`, keeping the claim-first transaction. Alternatively, decode `event.created` and refuse any update older than the last one applied. That needs a stored per-subscription marker, such as a column or a side table keyed by `stripe_subscription_id`. Also treat an unknown row in `ApplyDeleted` as retryable instead of committing the marker:
```go
// decode.go
type event struct {
ID string `json:"id"`
Type string `json:"type"`
Created int64 `json:"created"`
Data struct{ Object json.RawMessage `json:"object"` } `json:"data"`
}
// job.go: route() for created/updated
fresh, err := p.gateway().Retrieve(ctx, s.ID) // outside tx, before lagoon.Transaction
...
_, err = svc.Apply(ctx, tx, fresh.update())
// apply.go: ApplyDeleted
if len(rows) == 0 {
return nil, fmt.Errorf("golem15.paymentgatewaysubscriptions: apply deleted %s: subscription not stored yet", subscriptionID) // retried
}
```
### CR-02: Checkout can charge a Stripe price in a different currency or amount than the customer API shows and validated
**Files:**
- `sm-paymentgatewaysubscriptions-plugin/api/subscription.go:112-119, 140-149`
- `sm-pgstripe-plugin/subscriptions/gateway.go:128`
- `sm-paymentgatewaysubscriptions-plugin/models/subscription_plan.go:109-119`
- `sm-pgstripe-plugin/subscriptions/sync_plan.go:15, 55-69`
- `sm-paymentgatewaysubscriptions-plugin/api/json.go:62-75`
**Issue:** `Checkout` accepts a currency when `priced(plan, currency)` and `SupportsCurrency(currency)` are both true. `priced` only looks at the local `currency_prices` and `price_cents`/`currency`. The gateway then picks the Stripe price with `StripePriceIDFor(currency)`, which silently falls back to `stripe_price_id` when `stripe_prices` has no entry for that currency.
`SyncPlanToGateway` creates prices only for `usd`, `eur` and `pln` and sets `stripe_price_id` to the USD price. A plan whose `currency` is any other supported code (`gbp`, `cad`, `aud`, `chf`, `jpy`; the admin form accepts any 3-letter text) therefore goes like this:
- `/plans` lists it as, for example, `29.99 GBP`.
- Checkout passes the 422 gate.
- Stripe Checkout runs on the **USD** price created from the same `price_cents`: `$29.99` instead of `£29.99`.
The same happens when `currency_prices` names a currency the plan was never synced in, or when an admin typed a `stripe_price_id` for one currency and the customer asks for another.
JPY is wrong a second way. `money.FromMinor` divides by 100 for every currency, so `/plans` shows `¥5` for `price_cents: 500`, while a JPY price would bill `¥500`.
**Fix:** Before calling the gateway, require a gateway price that belongs to the requested currency, and answer 422 otherwise. Never fall back across currencies:
```go
// api/subscription.go, after the priced/SupportsCurrency check
if !gatewayPriced(plan, currency) {
writeValidation(w, map[string][]string{"currency": {msgCurrencyInvalid}})
return
}
func gatewayPriced(plan *models.SubscriptionPlan, currency string) bool {
if plan.StripePrices.Valid {
if id, ok := plan.StripePrices.Data[strings.ToLower(currency)]; ok && id != "" {
return true
}
return false // synced plan without this currency: never fall back to stripe_price_id
}
// unsynced plan with a hand-entered price id: only its own currency
return plan.StripePriceID != nil && *plan.StripePriceID != "" && strings.EqualFold(plan.Currency, currency)
}
```
Also make `CreateCheckoutSession` refuse the fallback in the same way (`ErrNoPriceForCurrency`), because other callers of `Service.Checkout` hit the same path. Either drop `jpy` from `supportedCurrencies` or handle zero-decimal currencies in `planPrices`.
## Warnings
### WR-01: Plan sync calls Stripe while holding the bulk-action transaction and row locks; a rollback loses the created Stripe ids
**Files:**
- `sm-paymentgatewaysubscriptions-plugin/controllers/subscriptionplans.go:130-153, 155-178`
- `sm-paymentgatewaysubscriptions-plugin/classes/service.go:362-388`
**Issue:** cabana's `BulkAction` opens a transaction, locks the selected plans with `FOR UPDATE` and runs `runSyncSelected` inside it (`summercms.go/modules/cabana/crud.go:369-388`). `syncOne` then makes one product call and up to three price list, create and archive calls per plan, all while the transaction and the row locks are held. This contradicts the `contracts.Gateway` doc ("no transaction is held across a provider call").
Stripe can create a product and its prices and then the transaction can still roll back:
- the admin's request is cancelled or times out (the context is shared);
- a lock wait or statement times out;
- the commit fails.
In each case the saved `stripe_product_id`/`stripe_prices` are lost, and the next sync creates a duplicate product. `runSyncAll` does the same per plan inside `lagoon.Transaction`, discards the transaction error (`_ = lagoon.Transaction(...)`, line 173), and has already counted the plan as `synced`.
**Fix:**
1. Make the bulk action collect only the plan ids.
2. Run each plan's gateway call outside any transaction, then save the ids in a short transaction of its own, the way `ensureCustomer` already does.
3. In `runSyncAll`, check the returned error and count a failed commit as a failure:
```go
if err := lagoon.Transaction(ctx, db, func(ctx context.Context, tx *gorm.DB) error { ... }); err != nil {
tally.failures = append(tally.failures, plan.Code+": save failed")
}
```
### WR-02: Concurrent or retried checkouts create duplicate Stripe customers, and the last write wins
**Files:**
- `sm-paymentgatewaysubscriptions-plugin/classes/service.go:392-411`
- `sm-paymentgatewaysubscriptions-plugin/classes/user_subscribable.go:89`
- `sm-pgstripe-plugin/subscriptions/gateway.go:76-108`
**Issue:** `ensureCustomer` checks `sub.CustomerID()` on a subscriber loaded before the call, creates a Stripe customer without an idempotency key, and then runs `UPDATE users SET stripe_customer_id = ? WHERE id = ?` unconditionally. Two parallel checkout requests (a double click in two tabs, or a client retry after a timeout) each create a customer, and the second UPDATE overwrites the first. If the user completes the session tied to the overwritten customer:
- the subscription lives under a customer the app no longer knows;
- `POST /subscription/portal` opens the other customer's empty portal;
- the user cannot manage or cancel that subscription from the site.
**Fix:** Write the id only when none is stored, and re-read on conflict. Also pass an idempotency key derived from alias and id to `V1Customers.Create`:
```go
res := tx.Exec(`UPDATE users SET stripe_customer_id = ? WHERE id = ? AND (stripe_customer_id IS NULL OR stripe_customer_id = '')`, id, u.id)
if res.RowsAffected == 0 { /* re-read the stored id and use it; optionally delete the orphan customer */ }
```
```go
params.SetIdempotencyKey("pgsub-customer-" + s.MorphAlias() + "-" + strconv.FormatUint(uint64(s.Key()), 10))
```
### WR-03: Checkout's "Already subscribed" guard misses non-active live subscriptions and is racy, so users can be billed twice
**Files:**
- `sm-paymentgatewaysubscriptions-plugin/api/subscription.go:87-93, 198-205`
**Issue:** Checkout refuses with 409 only when `Active()` (active or trialing) finds a row. A user whose subscription is `past_due` (still in grace and still billed by Stripe's retries), `unpaid`, `incomplete` or `paused` can start a second Checkout. A completed second Checkout gives two live Stripe subscriptions on one customer, both charging.
The check is also not atomic. Two parallel requests both pass it and both get a session URL; the layer's busy guard protects only one tab. `POST /subscription/cancel` then cancels only the newest active row, leaving the other one billing.
**Fix:**
- Refuse checkout while any subscription of the subscriber is in a non-terminal status (anything except `canceled` and `incomplete_expired`). Send `past_due`/`unpaid` users to the portal instead.
- Serialize per subscriber, either with `SELECT ... FOR UPDATE` on the users row or `pg_advisory_xact_lock(hashtext('pgsub:user:'||id))` around the check, or with a Stripe idempotency key per user and plan for a short window.
### WR-04: A subscription whose price was archived by a later sync (or whose plan code changed) is never stored, although the customer is billed
**Files:**
- `sm-paymentgatewaysubscriptions-plugin/classes/apply.go:72-85, 300-323`
- `sm-pgstripe-plugin/subscriptions/sync_plan.go:141-151`
**Issue:** `planForPrice` resolves a plan only from the current `stripe_price_id` or the current `stripe_prices` values. Re-syncing a repriced plan archives the old price and replaces the ids in `stripe_prices`. A Checkout session created before the resync (valid for 24 h) and completed after it produces a subscription on the old price. Its `customer.subscription.created`/`updated` events then fail with "no plan holds price id", are retried for conga's default attempts and are discarded. The webhook already answered 200, so Stripe never redelivers. The customer is charged and the app has no subscription row. Changing a plan's `code` (editable in the form) has the same effect for new checkouts on the old lookup key.
**Fix:** Fall back to the `plan_code` metadata the gateway writes on every Checkout subscription (`gateway.go:153`). Carry it in `SubscriptionArgs`, add `PlanCode` to `contracts.Update`, and in `Apply` use `ActivePlanByCode` (or any plan by code) when `planForPrice` returns nil:
```go
if plan == nil && u.PlanCode != "" {
plan, err = planByCode(db, u.PlanCode)
}
```
### WR-05: The grace period is computed from the already advanced period end, so a past_due subscription keeps access for about a full unpaid period plus grace_period_days
**Files:**
- `sm-paymentgatewaysubscriptions-plugin/models/subscription.go:137-142`
**Issue:** `InGracePeriod` grants access while `now < current_period_end + graceDays`. When Stripe renews, it moves `current_period_start`/`current_period_end` to the new period before the renewal invoice is paid. A `past_due` subscription therefore already carries the end of the period that was not paid for. With monthly billing and the default 7 days, `has_access` stays true for about 37 days after the failed payment, not 7. PHP had the same formula (inherited), but `has_access` is now exposed through the new customer API and `Service.HasAccess`.
**Fix:** Base grace on the start of the unpaid period (or on the timestamp of the move to `past_due`):
```go
return now.Before(s.CurrentPeriodStart.AddDate(0, 0, graceDays))
```
If PHP parity must be kept, document the behaviour in the README and the API contract.
### WR-06: SyncSubscription saves a stale, unlocked row in full and never refreshes the plan
**Files:**
- `sm-paymentgatewaysubscriptions-plugin/classes/service.go:335-352`
- `sm-pgstripe-plugin/subscriptions/gateway.go:263-306`
**Issue:** The caller hands in a row loaded earlier without a lock. The gateway changes some fields in memory, and the service then runs `Save(sub)`, which writes every column. Changes that `Apply` made between the load and the save are overwritten with the stale values: `subscription_plan_id` after a plan switch, `subscriber_*`, `stripe_customer_id` and `canceled_at`. The gateway also ignores the retrieved price, so a plan change made in the portal is never reconciled by a sync.
**Fix:**
1. Re-load the row `FOR UPDATE` inside the short transaction.
2. Copy only the gateway-owned fields onto it.
3. Map the retrieved price to a plan with `planForPrice`.
Or `Select(...)` only the columns the gateway sets.
### WR-07: After a redirect the subscribe and manage buttons stay disabled for good, including after the browser Back button
**Files:**
- `vue-paymentgateway-layer/app/utils/subscribe.ts:74, 85, 115, 125, 134`
**Issue:** `state.redirecting` is set to true before `location.assign` and never reset. When the user goes to Stripe Checkout or the portal and presses Back, browsers with a back/forward cache restore the page with its JavaScript state. The button then shows "Subscribing..." and stays disabled, and `cancel()` returns null without a request. The only way out is a full reload. Leaving Checkout with Back is a common path.
**Fix:** Reset the state when the page is shown again:
```ts
if (typeof window !== 'undefined') {
window.addEventListener('pageshow', (e) => { if ((e as PageTransitionEvent).persisted) { state.redirecting = false; state.busy = false } })
}
```
Register the listener in the components' `onMounted` and remove it in `onBeforeUnmount`.
## Info
### IN-01: The per-user throttle key of the customer API never applies
**File:** `sm-paymentgatewaysubscriptions-plugin/routes.go:54-60, 77-82`
**Issue:** `throttle:paymentgatewaysubscriptions-api` is group middleware, and it runs before `pgapi.RequireJWT`. Nothing sets the bouncer principal until the jwt guard runs (`bouncer/jwt.go:63`), so `bouncer.User` is always empty when the bucket key is computed. Every request is keyed by client IP, and the documented "120 per minute per frontend user" is not what happens. The order API has the same pattern.
**Fix:** Run the guard before the throttle on the four Bearer routes. Alternatively, drop the per-user branch and correct the comment and README.
### IN-02: The `metadata_app` filter lets untagged events from other applications through
**File:** `sm-pgstripe-plugin/subscriptions/job.go:136`
**Issue:** `want != "" && got != "" && got != want` skips only events tagged with a different app. On a shared Stripe account, an application that writes no tag (including the PHP apps before cutover) gets every subscription event applied. Each fails on "no plan holds price id" and is retried until it is discarded, which floods the error logs.
**Fix:** When `metadata_app` is set, skip (and mark) untagged events too, or make that behaviour a config option.
### IN-03: The admin sync result shows the raw Stripe error text
**File:** `sm-paymentgatewaysubscriptions-plugin/controllers/subscriptionplans.go:86-92`
**Issue:** `cause.Error()` (the Stripe message) is put into the toast. Elsewhere the gateway deliberately logs only Stripe's type, code and request id, because "the message may echo request data".
**Fix:** Map `*stripe.Error` to a localized line naming the code, or reuse the `failed()` attributes.
### IN-04: The plans table decides "free" differently from the server
**File:** `vue-paymentgateway-layer/app/components/Pg/Plans.vue:35-38, 78-81`
**Issue:** The server treats a plan as free when `price_cents == 0` (checkout answers 422, `api/subscription.go:105-108`). The layer treats it as free when every listed price is zero. A plan with `price_cents: 0` and non-zero `currency_prices` shows a Subscribe button that always fails with "unavailable". A plan with mixed zero and non-zero prices shows `0.00` with a Subscribe button.
**Fix:** Expose a `free` boolean in `PlanJSON`, or apply the same rule on both sides.
### IN-05: TaglistField leaves a typed separator in the input when the draft is empty
**File:** `summercms.go/admin/src/components/form/fields/TaglistField.vue:86-99`
**Issue:** Typing `,` (or a space in space mode) into an empty input leaves `draft` at `''`, which is unchanged, so Vue does not patch the input. The separator stays visible until the next keystroke or blur.
**Fix:** Write the remaining text back to the element (`(event.target as HTMLInputElement).value = rest`) after splitting.
### IN-06: Taglist required message is not localized; `customTags: false` without options is accepted at boot
**File:** `summercms.go/modules/cabana/field_taglist.go:300, 314, 252-263`
**Issue:** The required error is a hard-coded English string, while the other taglist errors use phrase keys. A field with `customTags: false` and no `options` compiles and then refuses every tag at save time.
**Fix:** Use the localized required message. Fail boot when `customTags: false` has no options.
### IN-07: Test-mode and live-mode Stripe customer ids share `users.stripe_customer_id`
**Files:**
- `sm-paymentgatewaysubscriptions-plugin/updates/202610150002_add_stripe_customer_id_to_users.go:18-21`
- `sm-pgstripe-plugin/subscriptions/gateway.go:80-82`
**Issue:** A customer created while `test_mode` is on is stored in the same column and reused after switching to live. Live checkout and portal calls then fail with "No such customer" and the user gets a permanent 502. Nothing recovers from it.
**Fix:** When Stripe answers `resource_missing` for the stored customer, clear it and create a new one. Alternatively, store the customer per mode.
### IN-08: Repository hygiene of the new plugin module
**Files:**
- `sm-paymentgatewaysubscriptions-plugin/go.work:9`
- `sm-paymentgatewaysubscriptions-plugin/go.mod:5-7, 132-134`
**Issue:**
- The committed `go.work` replaces `sm-user-plugin` with `../sm-summercmsio-app/...`, which names a consuming application from a shared core plugin repository.
- `go.mod` requires `sm-paymentgateway-plugin v0.0.0`, which resolves only through the module's own `replace` directives. Consumers ignore those, so every consumer must add its own replace.
- The tagged `summercms v0.1.26` requirement is shadowed by `replace ../summercms.go` in every local build.
**Fix:** Point the workspace at a neutral path or leave `go.work` untracked. Require tagged versions of the sibling plugins once they exist.
---
_Reviewed: 2026-10-14T19:22:09Z_
_Reviewer: Claude (msd-code-reviewer)_
_Depth: standard_