diff --git a/.planning/phases/18.1-payment-gateway-subscriptions/18.1-03-SUMMARY.md b/.planning/phases/18.1-payment-gateway-subscriptions/18.1-03-SUMMARY.md new file mode 100644 index 0000000..11e72d5 --- /dev/null +++ b/.planning/phases/18.1-payment-gateway-subscriptions/18.1-03-SUMMARY.md @@ -0,0 +1,303 @@ +--- +phase: 18.1-payment-gateway-subscriptions +plan: 03 +subsystem: payments +tags: [subscriptions, stripe, stripe-go, webhook, river, conga, postgres, testcontainers] + +# Dependency graph +requires: + - phase: 18-payment-gateway + provides: sm-pgstripe-plugin (ClientFactory, BackendOverride, ModeKeys, VerifySignature, ExpandableID, encrypted settings, import-winter) + - phase: 18.1-02 + provides: sm-paymentgatewaysubscriptions-plugin at aad4647 (contracts.Gateway, HasSubscriptionGateways, classes.Service Apply/ApplyDeleted/ApplyInvoice/ClaimEvent/EventProcessed/SyncPlan, UserSubscribable, six events) +provides: + - "golem15_pgstripe_settings gains subscription_webhook_secret and test_subscription_webhook_secret (migration 202610150001_add_subscription_webhook_secrets, additive), pgstripe.SubscriptionWebhookSecret, two config fallbacks, two write-only form fields" + - "pgstripe:import-winter --webhook-secret-target payments|subscriptions|both (default payments = Phase 18 mapping)" + - "plugin golem15.pgstripesubscriptions (package subscriptions): Stripe gateway incl. SyncPlanToGateway, webhook on /webhook/stripe/subscriptions and /api/subscriptions/webhook, claim-first River job golem15_pgstripesubscriptions_webhook, metadata_app config" + - "SC-4 proof host TestHostSubscriptionFlow (14 subtests incl. a real conga worker) and TestSubscriptionWebhookSmoke (12 subtests)" +affects: [18.1-04, 18.1-05, 18.1-06, sm-pgstripe-plugin, consuming applications' cutover] + +# Actuals (#2632) - chars/4 over the realized diff in sm-pgstripe-plugin (0313820..592b474) +actuals: + tokens: 48666 + tasks: 4 + commits: 3 +plan_head_before: 0313820f8deb221ffb3fa837ed582152f59d72ee +plan_head_after: 592b4741dedec2e7376ba7f2a94f8032cd04c3dc +commits: 3 + +tech-stack: + added: [] + patterns: + - "Sub-plugin in a subpackage of a plugin repository (golem15.pgstripesubscriptions), so payments-only builds link no subscription code" + - "First plugin job on conga: conga.Job over a plugin method, args holding ids/flags/amounts only, claim-first marker in the job transaction" + - "Webhook handler order: body cap, secret resolved before the header, signature on raw bytes, cheap marker check, enqueue only handled types; plain-text PHP bodies" + - "Stripe objects decoded with local structs that read both payload shapes (items then top-level periods; parent.subscription_details then top-level subscription)" + +key-files: + created: + - ../sm-pgstripe-plugin/updates/202610150001_add_subscription_webhook_secrets.go + - ../sm-pgstripe-plugin/subscriptions/plugin.go + - ../sm-pgstripe-plugin/subscriptions/config/config.yaml + - ../sm-pgstripe-plugin/subscriptions/gateway.go + - ../sm-pgstripe-plugin/subscriptions/sync_plan.go + - ../sm-pgstripe-plugin/subscriptions/webhook.go + - ../sm-pgstripe-plugin/subscriptions/decode.go + - ../sm-pgstripe-plugin/subscriptions/job.go + - ../sm-pgstripe-plugin/subscriptions/README.md + - ../sm-pgstripe-plugin/subscriptions/testmain_test.go + - ../sm-pgstripe-plugin/subscriptions/stripefake_test.go + - ../sm-pgstripe-plugin/subscriptions/eventbody_test.go + - ../sm-pgstripe-plugin/subscriptions/fixture_plugin_test.go + - ../sm-pgstripe-plugin/subscriptions/host_test.go + - ../sm-pgstripe-plugin/subscriptions/webhook_smoke_test.go + modified: + - ../sm-pgstripe-plugin/go.mod + - ../sm-pgstripe-plugin/go.work + - ../sm-pgstripe-plugin/client.go + - ../sm-pgstripe-plugin/models/settings.go + - ../sm-pgstripe-plugin/models/settings/fields.yaml + - ../sm-pgstripe-plugin/config/config.yaml + - ../sm-pgstripe-plugin/console/import_winter.go + - ../sm-pgstripe-plugin/lang/en/lang.yaml + - ../sm-pgstripe-plugin/lang/pl/lang.yaml + - ../sm-pgstripe-plugin/settings_smoke_test.go + - ../sm-pgstripe-plugin/settings_security_test.go + - ../sm-pgstripe-plugin/README.md + +key-decisions: + - "Webhook answers keep PHP's plain-text bodies (human reply php-plain-text): text/plain; charset=utf-8, no trailing newline" + - "Price lookup uses Prices List with lookup_keys and active=true instead of PHP's eventually consistent Prices Search" + - "Checkout metadata keeps PHP's keys (subscriber_type, subscriber_id, plan_code, user_email, currency, gateway), adds app only when metadata_app is set, merges CheckoutOptions.Metadata underneath (subscriber and plan keys win), never writes an owner key" + - "The job keeps conga's default three attempts; an app-tag skip and an unknown type keep the marker; any error rolls it back" + - "ReadWinterSettings keeps its signature; the target-aware variant is the additive ReadWinterSettingsFor" + +patterns-established: + - "Package-local Stripe fake per plugin package (D-20); Phase 18's root fake untouched" + - "Real worker in a plugin test: database.dsn set to the fresh database's DSN, conga.StartWorker on the app, poll the row, Stop" + +requirements-completed: [SC-2, SC-4, D-01, D-07, D-10, D-11, D-12, D-15, D-18, D-19, D-20, D-21, D-22] + +coverage: + - id: D1 + description: "Additive root changes: two encrypted subscription secret columns via 202610150001, SecretFallbacks, SubscriptionWebhookSecret (mode-aware, no payment-secret fallback), write-only form fields, ModeKeys and Phase 18 payloads unchanged" + requirement: D-18 + verification: + - kind: integration + ref: "settings_smoke_test.go#TestStripeSettingsSmoke/subscription_secrets_env_and_empty, subscription_secret_stored, subscription_secret_keep_on_empty, subscription_secret_clear_on_null" + status: pass + - kind: unit + ref: "settings_security_test.go#TestStripeDataHandling" + status: pass + - kind: integration + ref: "host_test.go#TestHostStripeFlow (Phase 18 regression)" + status: pass + human_judgment: false + - id: D2 + description: "pgstripe:import-winter --webhook-secret-target payments (default, Phase 18 result), subscriptions, both; unknown value refused before any write; dry run masks" + requirement: D-18 + verification: + - kind: integration + ref: "settings_smoke_test.go#TestStripeImportWinterSmoke/target_default, target_subscriptions, target_both, target_invalid" + status: pass + human_judgment: false + - id: D3 + description: "Stripe gateway: customer, subscription Checkout (session_id placeholder, metadata, app tag), portal, cancel at period end and immediately, sync, SupportsCurrency, SyncPlanToGateway (product, usd/eur/pln prices, reuse, transfer_lookup_key and archive, free plan)" + requirement: SC-2 + verification: + - kind: integration + ref: "subscriptions/host_test.go#TestHostSubscriptionFlow/checkout_activation, cancel_at_period_end, cancel_immediately, plan_sync, portal, sync_subscription, app_tag" + status: pass + human_judgment: false + - id: D4 + description: "Both webhook URLs on one handler with PHP's answers and order, one 60/min/IP bucket, only five types enqueued, handler never writes the marker" + requirement: D-10 + verification: + - kind: integration + ref: "subscriptions/webhook_smoke_test.go#TestSubscriptionWebhookSmoke (12 subtests)" + status: pass + human_judgment: false + - id: D5 + description: "Claim-first job: five event types in both payload shapes, app-tag skip with marker kept, duplicates fire once, PII-free args, a real conga worker processes the job" + requirement: SC-4 + verification: + - kind: integration + ref: "subscriptions/host_test.go#TestHostSubscriptionFlow/invoice_paid_basil, invoice_paid_pre_basil, past_due_grace, reactivation, pre_basil_periods, duplicate_event, app_tag, real_worker" + status: pass + human_judgment: false + - id: D6 + description: "Payments-only builds link no pgsub package (D-07)" + requirement: D-07 + verification: + - kind: other + ref: "go -C ../sm-pgstripe-plugin list -deps . | grep -c sm-paymentgatewaysubscriptions-plugin == 0" + status: pass + human_judgment: false + - id: D7 + description: "Live Stripe test-mode checkout end to end, and whether a billing-portal cancellation needs cancel_at handling beyond cancel_at_period_end (D-21)" + verification: [] + human_judgment: true + rationale: "Needs a real Stripe test-mode account, a public webhook URL and the hosted billing portal; the fake cannot show what Stripe sends for a portal cancellation on current API versions" + +duration: 21min +completed: 2026-10-14 +status: complete +--- + +# Phase 18.1 Plan 03: Stripe subscriptions Summary + +**The new plugin `golem15.pgstripesubscriptions` (package `sm-pgstripe-plugin/subscriptions`) ports pgstripe's subscription half onto the pgsub contracts. It holds the Stripe gateway with the d00f850 plan sync, the shared handler for both webhook URLs with PHP's plain-text answers, and the claim-first River job that reads both API payload shapes. The root plugin gets additive encrypted subscription signing secrets and an import target flag. A Postgres proof host with a package-local Stripe fake and a real conga worker proves all of it.** + +## Performance + +- **Duration:** 21 min (from 2026-10-14T15:32:01Z to 2026-10-14T15:52:44Z) +- **Tasks:** 4 (1 pre-answered decision checkpoint, 1 tracer, 2 auto) +- **Files:** 27 changed in sm-pgstripe-plugin (3838 insertions, 59 deletions) + +## Task 1: webhook contract sheet and reply + +**Reply (verbatim):** "php-plain-text" + +The human gave this reply before dispatch, when the orchestrator showed the two options. Source (frozen, `8b47457`): pgstripe's `http/routes.php` and `http/controllers/StripeWebhookController.php`, and pgsub's `http/routes.php` (the legacy alias). The verify command printed `webhook-sheet-ok`, so the controller has its five literal `response('...')` calls. + +- **URLs, one handler:** `POST /webhook/stripe/subscriptions` (canonical) and `POST /api/subscriptions/webhook` (legacy alias). No CSRF. One bucket, `pgstripesubscriptions-webhook`, allows 60 requests per minute per client IP (PHP `throttle:60,1`). +- **Answers, in check order** (text/plain, no trailing newline): + 1. 500 `Webhook secret not configured` (Error log) + 2. 400 `Invalid signature` (Warn log) + 3. 400 `Invalid payload` (Warn log; also a body over 1 MiB) + 4. 200 `Already processed` (the processed-event marker exists) + 5. 200 `Webhook received` +- **Enqueued types:** only `customer.subscription.created`, `customer.subscription.updated`, `customer.subscription.deleted`, `invoice.paid` and `invoice.payment_failed`. Any other type answers 200 `Webhook received` and queues no job. +- **Job kind:** `golem15_pgstripesubscriptions_webhook`. +- **Plugin:** ID `golem15.pgstripesubscriptions`, import path `git.golem15.com/golem15/sm-pgstripe-plugin/subscriptions`. +- **Migration:** `202610150001_add_subscription_webhook_secrets` adds the columns `subscription_webhook_secret` and `test_subscription_webhook_secret`. +- **Config keys:** `golem15.pgstripe.subscription_webhook_secret`, `golem15.pgstripe.test_subscription_webhook_secret` and `golem15.pgstripesubscriptions.metadata_app`. + +I wrote the sheet and the reply into this SUMMARY on disk before Task 2's first commit. + +## Task Commits + +All code commits are in `../sm-pgstripe-plugin`, on branch `main`. Nothing was pushed or tagged. + +| Task | Name | Commit | +|------|------|--------| +| 1 | Webhook contract (checkpoint:decision, pre-answered) | none (no files) | +| 2 | Tracer: secret columns, the plugin, checkout to `activated` | `e42aafd` | +| 3 | Complete gateway, plan sync, five event types, full proof host | `8f951cb` | +| 4 | Form fields, import target flag, READMEs, extended smokes | `592b474` | + +- **Repositories:** `plan_head_before` is `0313820` and `plan_head_after` is `592b474`; `git rev-list --count` gives 3. +- **pgsub dependency:** `go.mod` requires `sm-paymentgatewaysubscriptions-plugin v0.0.0`, with `replace ../sm-paymentgatewaysubscriptions-plugin` in both go.mod and go.work. MVS raised the framework requirement from v0.1.24 to v0.1.25, but `replace ../summercms.go` still applies, so the held v0.1.26 tag is not involved. + +## Exported identifiers + +- **Root `pgstripe`:** `SubscriptionWebhookSecret(app, settings) (secret, mode string)`. +- **`models`:** fields `Settings.SubscriptionWebhookSecret` and `Settings.TestSubscriptionWebhookSecret`; constants `ConfigSubscriptionWebhookSecret` and `ConfigTestSubscriptionWebhookSecret`. +- **`console`:** + - `WinterOptions.WebhookSecretTarget` + - `WebhookSecretTargetPayments`, `WebhookSecretTargetSubscriptions`, `WebhookSecretTargetBoth` + - `ReadWinterSettingsFor` + - flag `--webhook-secret-target` +- **`subscriptions`:** + - Plugin and routes: `Plugin`, `PluginID`, `WebhookPath`, `LegacyWebhookPath`, `WebhookBucket`, `WebhookBucketMax`, `ConfigMetadataApp`. + - Gateway: `Gateway`, `NewGateway`, `DefaultCurrency`, `ErrNoPriceForCurrency`. + - Webhook: `MaxWebhookBytes`; the five `Body*` constants; `ErrEventMalformed`; `MetadataSubscriberType`, `MetadataSubscriberID`, `MetadataApp`. + - Job: `JobKind`, `WebhookJobArgs`, `SubscriptionArgs`, `InvoiceArgs`, `HandledEventTypes`, and the five `Event*` constants. + - Config: `metadata_app` (env `SUMMER_GOLEM15__PGSTRIPESUBSCRIPTIONS__METADATA_APP`). + +## Tests run + +All Docker tests ran on testcontainers Postgres 16. + +- **Task 2:** + - `go vet ./...` and `go test -short ./...` pass. + - D-07 holds: `go list -deps .` names no pgsub package, and `./subscriptions` imports pgsub `classes`. + - `TestHostSubscriptionFlow/checkout_activation` passes. `TestHostStripeFlow` and `TestStripeSettingsSmoke` (Phase 18) pass, 2 of 2. + - The tracer gate re-ran this verify after the commit and it passed. +- **Task 3:** + - vet and short pass; both path literals are present. + - `TestHostSubscriptionFlow` (14 subtests, including `real_worker`) and `TestSubscriptionWebhookSmoke` (12 subtests) pass, 2 of 2 top-level. + - `TestSDKPin` passes. +- **Task 4:** + - vet and short pass; the form carries both secret fields; `subscriptions/README.md` exists. + - `TestStripeSettingsSmoke` (with 4 new subtests) and `TestStripeImportWinterSmoke` (with 4 target subtests) pass, 2 of 2. +- **Final:** the full `go test -count=1 ./...` passes for both packages. +- **Acceptance greps:** all pass. + - Phase 18's `stripefake_test.go`, `host_test.go`, `webhook.go` and `operator.go` are unchanged since `0313820`. + - The `ModeKeys` signature is unchanged. + - `ADD COLUMN IF NOT EXISTS` appears 2 times. + - `decode.go` matches `customer_email` or `customer_address` 0 times. + - No consuming application is named. + +## Decisions Made + +See `key-decisions` in the frontmatter. The plan's own discretionary choices are all applied: +- Prices List instead of Search. +- The faithful usd/eur/pln loop. +- conga's default attempts. +- No fallback from the subscription secret to the payment secret. +- No check of the operator `enabled` switch. + +## Deviations from Plan + +### Auto-fixed and additive + +1. **[Rule 3 - Blocking] `settings_security_test.go` updated.** + - **Found during:** Task 2. + - **Issue:** Phase 18's `TestStripeDataHandling` asserted exactly four `SecretFallbacks`, so the two additive secrets broke it. + - **Fix:** the test now covers six secrets: both new fields serialize nothing, and both fallbacks map to their keys. + - **Note:** the file is not one of the four Phase 18 files the plan protects. + - **Commit:** `e42aafd`. +2. **The legacy alias was registered in Task 2, not Task 3.** `Routes` registered both paths on the one handler from the start. Task 3's smoke (`legacy_alias`, `shared_bucket`) proves it. +3. **Checkout metadata is broader than the plan's list.** The plan listed subscriber_type, subscriber_id, plan_code and app. The port also keeps PHP's `user_email`, `currency` (upper case), `gateway` and `allow_promotion_codes=true`, to match the frozen source. Merge order: `CheckoutOptions.Metadata` first, then `app`, then the subscriber and plan keys, so a caller cannot override the subscriber identity the job trusts. `billing_owner_id` is not written. `TestHostSubscriptionFlow/app_tag` proves the order. +4. **Additive identifiers beyond the plan's list:** + - `console.ReadWinterSettingsFor`, which keeps the `ReadWinterSettings` signature unchanged (core plugin rule); + - `WebhookBucketMax`, `MaxWebhookBytes`, the `Body*` constants, `JobKind`, `SubscriptionArgs`, `InvoiceArgs`, `ErrEventMalformed`, the `Metadata*` keys and `DefaultCurrency`. +5. **The invoice app tag comes from the invoice's own `metadata.app`,** as PHP read `data.metadata.app`. It is not read from `parent.subscription_details.metadata`. An invoice of another application's subscription finds no row either way. +6. **Content-Type.** Answers are `text/plain; charset=utf-8`, as the decision states. Laravel's `response('...')` would have sent `text/html`. The README notes this. +7. **Extra test coverage:** + - `TestHostSubscriptionFlow/sync_subscription` (Service.SyncSubscription through the gateway, pre-basil periods); + - `TestSubscriptionWebhookSmoke/shared_bucket` (61 requests across both URLs: the 61st gets 429); + - a tampered-body case in `wrong_secret`; + - a payment-secret-signed request in `missing_secret` and `wrong_secret`, which proves there is no payment-secret fallback. +8. **Test harness.** The package `testmain_test.go` adds `freshDBDSN`. The host sets `database.dsn` so a real worker can open its LISTEN pool. + +**Total:** one Rule 3 test fix and several additive choices. None changes scope or a Phase 18 contract. + +## Issues Encountered + +- A Task 4 dry-run assertion expected a `whsec_` prefix on the masked secret. `maskSecret` shows a prefix only when the last `_` is within 8 characters, so I corrected the assertion; the code was right. +- In the first draft of the portal subtest, a plan with only `stripe_price_id` was expected to refuse an `eur` checkout. PHP's `getStripePriceId` falls back to `stripe_price_id`, so the subtest now uses a plan with no price id. + +## Known Stubs + +None. + +## Threat Flags + +None. The two public webhook routes, the job args and the secret columns are the surfaces in the plan's threat register (T-18.1-10 to T-18.1-19). Their mitigations are implemented and tested as listed above. + +## Manual checks left for UAT (/msd-verify-work, 18.1-VALIDATION.md manual-only rows) + +1. A live Stripe test-mode checkout end to end: + - a plan synced with `SyncPlan`; + - Checkout through `Service.Checkout`; + - the dashboard endpoint at `/webhook/stripe/subscriptions` with the test subscription signing secret; + - a worker running; + - result: the row turns active and `activated` fires. +2. A billing-portal cancellation: inspect whether the `customer.subscription.updated` it sends sets `cancel_at` rather than `cancel_at_period_end` on the current API version (D-21). If it does, a follow-up must map `cancel_at`. + +## User Setup Required + +None for this plan. The setup an application needs (dashboard endpoint, signing secret, worker, `metadata_app` at cutover) is in `subscriptions/README.md`. + +## Next Phase Readiness + +- Plan 04 (pgsub admin and customer API) can call `Service.SyncPlan`, which reaches `Gateway.SyncPlanToGateway` once `golem15.pgstripesubscriptions` is activated. The admin "Sync to Stripe" actions need no Stripe import in pgsub. +- Plan 06 can add the parallel duplicate-job test (T-18.1-12) on this job, plus `scripts/check-phase18.1.sh` entries for `TestHostSubscriptionFlow`, `TestSubscriptionWebhookSmoke` and the new settings and import subtests. +- Nothing is pushed. sm-pgstripe-plugin `main` is 3 commits ahead of `0313820`. + +## Self-Check: PASSED + +- All 15 created files exist in `../sm-pgstripe-plugin`, and the tree is clean after `592b474`. +- Commits `e42aafd`, `8f951cb` and `592b474` exist on `main`.