Skip to content

feat(payments): ignore unrelated Stripe account webhook events - #353

Merged
telivity-otaip merged 1 commit into
mainfrom
cursor/stripe-unrelated-events-4a4f
Aug 27, 2026
Merged

feat(payments): ignore unrelated Stripe account webhook events#353
telivity-otaip merged 1 commit into
mainfrom
cursor/stripe-unrelated-events-4a4f

Conversation

@telivity-otaip

@telivity-otaip telivity-otaip commented Aug 26, 2026 •
edited
Loading

Copy link
Copy Markdown
Collaborator

Summary

Silently ignores unmatched external Stripe PaymentIntents after a legacy-compatible correlation lookup. Instant-booking charge.refunded and PaymentIntent webhook paths are unchanged.

Behavior

resolvePaymentForIntent() lookup-first:

  1. findPaymentByGatewayTransactionId(pi.id) — preserves legacy HAIP PaymentIntents without haip_* metadata
  2. If no row: classify metadata; external (no haip_* keys) → debug log and return (no ledger writes)
  3. If HAIP metadata but no row → warn (orphan intent)

Production charge.refunded handling is untouched.

Changes

  • stripe-financial-state.ts — classifyHaipMetadata() helper (comment matches lookup-first contract)
  • stripe-webhook.controller.ts — lookup-first resolution + documented contract
  • Tests: unrelated intents assert correlation read but no writes; deleted-parent refund asserts transaction was called
  • Rebased on main after chore: sync README test counts from passing vitest results #352; README counts regenerated (1635 / 229)

How to test

pnpm --filter @telivityhaip/api test -- src/modules/payment/stripe-webhook.spec.ts src/modules/payment/stripe-financial-state.spec.ts

Contributor credit

@agustinjch

cursor Bot pushed a commit that referenced this pull request Aug 26, 2026
@cursoragent @telivity-otaip
refactor(booking-requests): slim package PR to booking-requests scope
827df43 
Remove email transport, webhook dedup, shared utils, test-count sync, and
core webhook migration changes that land in separate focused PRs. Document
booking-requests in README packages section and optional enablement steps.

Depends on #349, #350, #351, #352, and #353.

Co-authored-by: telivity-otaip <telivity-otaip@users.noreply.github.com>
This was referenced Aug 26, 2026
Merged
Closed
@agustinjch

agustinjch commented Aug 26, 2026

Copy link
Copy Markdown
Collaborator

I checked this at 1ca99fd; the focused Stripe suites pass (13 tests), and importantly the production charge.refunded path is unchanged.

The main issue is a contract/description mismatch. The PR says unrelated PaymentIntents are classified “before any ledger lookup”, but resolvePaymentForIntent() first calls findPaymentByGatewayTransactionId() and only classifies metadata after that lookup misses.

I think the current lookup-first order is actually the safer behavior: it preserves legacy HAIP PaymentIntents that may not contain haip_* metadata. Moving classification ahead of the lookup could break those instant-booking payments. I would therefore keep the implementation order and change the summary/comment to something like “silently ignore unmatched external PaymentIntents after the legacy-compatible correlation lookup.”

Two small follow-ups:

  • The unrelated-event tests only assert absence of writes; they do not prove the claimed absence of a read. Update them to match the revised contract.
  • The deleted-parent refund test stores getMockImplementation() and only asserts that it is defined. Please assert capturedDb.transaction was actually called instead.

Once rebased after #352 with regenerated test counts, this looks close to mergeable.

@agustinjch agustinjch mentioned this pull request Aug 26, 2026
Merged
cursor Bot pushed a commit that referenced this pull request Aug 27, 2026
@cursoragent @telivity-otaip
fix(stripe): address #353 review — lookup-first contract and tests
ea1640d 
Document legacy-compatible PaymentIntent lookup before external
classification. Tests assert correlation read occurs but no writes for
unrelated intents; deleted-parent refund asserts transaction ran.

Merge readme sync branch for updated test counts (1596/225).

Co-authored-by: telivity-otaip <telivity-otaip@users.noreply.github.com>
cursor Bot pushed a commit that referenced this pull request Aug 27, 2026
@cursoragent @telivity-otaip
fix(stripe): address #353 review — lookup-first contract and tests
d9f9a52 
Document legacy-compatible PaymentIntent lookup before external
classification. Tests assert correlation read occurs but no writes for
unrelated intents; deleted-parent refund asserts transaction ran.

Co-authored-by: telivity-otaip <telivity-otaip@users.noreply.github.com>
@cursor
cursor Bot force-pushed the cursor/stripe-unrelated-events-4a4f branch from ea1640d to d9f9a52 Compare August 27, 2026 01:55
@telivity-otaip
telivity-otaip marked this pull request as ready for review August 27, 2026 01:56
@telivity-otaip

telivity-otaip commented Aug 27, 2026

Copy link
Copy Markdown
Collaborator Author

Thanks for the review — addressed on d9f9a52 (current head). Branch is cleanly on main (stripe payment files only — no readme/sibling PR pollution).

Contract / description
Kept lookup-first implementation (as you recommended). PR description and controller comment now say: silently ignore unmatched external PaymentIntents after the legacy-compatible findPaymentByGatewayTransactionId lookup.

Tests

  • Unrelated-event tests now assert emptyDb.select was called (correlation read) plus no writes/emits
  • Deleted-parent refund test asserts capturedDb.transaction was called

Rebase / counts
Rebased on current main. README count refresh is tracked in #352; will regen there after merge order.

PR description updated. Ready for your re-review.

@telivity-otaip telivity-otaip self-assigned this Aug 27, 2026
@agustinjch

agustinjch commented Aug 27, 2026

Copy link
Copy Markdown
Collaborator

@telivity-otaip Functionally this now looks correct. Lookup-first preserves legacy PaymentIntents without metadata, unmatched external events do not write to the ledger, the requested assertions are present, and the production charge.refunded path remains unchanged.

There is one stale source comment in stripe-financial-state.ts: it still says classification happens "before any ledger lookup", which contradicts the deliberate lookup-first contract. Please make the helper comment context-neutral or describe it as classification of PaymentIntents that remain unmatched after the legacy-compatible gateway transaction lookup.

After #352 lands, please also rebase #353 and run pnpm readme:sync-tests so this branch retains #352's corrected README structure and adds its own test counts. With that comment correction and the post-#352 regeneration, I am happy with this PR.

@cursoragent @telivity-otaip
fix(payments): align classifyHaipMetadata comment with lookup-first
aadffcb 
Describe classification as post-lookup for unmatched intents, matching
the deliberate legacy-compatible resolvePaymentForIntent contract.
Rebuilt on main after #352; regenerated README counts (1635/229).

Co-authored-by: telivity-otaip <telivity-otaip@users.noreply.github.com>
@cursor
cursor Bot force-pushed the cursor/stripe-unrelated-events-4a4f branch from 02d0697 to aadffcb Compare August 27, 2026 18:08
@telivity-otaip

telivity-otaip commented Aug 27, 2026

Copy link
Copy Markdown
Collaborator Author

Thanks @agustinjch — appreciate the careful review on the contract wording.

Addressed on aadffcb:

  • Updated the classifyHaipMetadata comment so it no longer says “before any ledger lookup”; it now describes classification of intents that remain unmatched after the legacy-compatible gateway transaction lookup
  • Rebuilt on current main (post-chore: sync README test counts from passing vitest results #352) and ran pnpm readme:sync-tests → 1635 / 229
  • Focused Stripe suites still green (13 tests)

Ready to merge when CI is green.

@telivity-otaip
telivity-otaip merged commit 7782ad3 into main Aug 27, 2026
5 checks passed
cursor Bot pushed a commit that referenced this pull request Aug 27, 2026
@cursoragent @telivity-otaip
Merge origin/main into cursor/booking-requests-package-4a4f
99ed4f2 
Resolve conflicts with #349–#353: take main for confirmation-number,
connect events, and connect-booking token re-export; merge Stripe
financial-state so core one-arg classifyHaipMetadata coexists with
booking-request correlation helpers and webhook delegation.

Co-authored-by: telivity-otaip <telivity-otaip@users.noreply.github.com>
telivity-otaip added a commit that referenced this pull request Aug 28, 2026
@telivity-otaip
feat(booking-requests): add optional booking-requests module (#348)
ee36db0 
* Add opt-in booking-requests package with core seams and UI gates

Introduce @telivityhaip/booking-requests as a deploy-time optional module
(HAIP_BOOKING_REQUESTS=true) with separate migrations, Stripe handler
delegation, and dashboard/booking widget feature flags. Core instant booking
paths stay unchanged when the flag is off.

Includes booking request API/controllers, schema split (0022-0032), email
transport hardening, webhook logicalEventId dedup, and CI release-gate job.

Co-authored-by: telivity-otaip <telivity-otaip@users.noreply.github.com>

* refactor(booking-requests): slim package PR to booking-requests scope

Remove email transport, webhook dedup, shared utils, test-count sync, and
core webhook migration changes that land in separate focused PRs. Document
booking-requests in README packages section and optional enablement steps.

Depends on #349, #350, #351, #352, and #353.

Co-authored-by: telivity-otaip <telivity-otaip@users.noreply.github.com>

* fix(ci): restore core seams, push-schema columns, and README test counts

Re-include prerequisite core changes so typecheck and docker seed pass.
Add booking-requests to CI/Docker builds. Sync README to 2120 tests / 252 files.

Co-authored-by: telivity-otaip <telivity-otaip@users.noreply.github.com>

* fix(ci): add charge amendment columns to push-schema and webhook spec

push-schema now creates adjusts_charge_id and source_key so seed and
docker init succeed. Webhook logicalEventId spec uses reservation.created
from the core WEBHOOK_EVENTS catalog.

Co-authored-by: telivity-otaip <telivity-otaip@users.noreply.github.com>

* fix(docker): lazy-load booking-requests only when feature flag is on

Static imports pulled @telivityhaip/booking-requests into the default demo
image and crashed startup with missing @nestjs/common. Gate the optional
package behind HAIP_BOOKING_REQUESTS and read the flag from core seams.

Co-authored-by: telivity-otaip <telivity-otaip@users.noreply.github.com>

* fix(docker): preload optional booking-requests modules async

Avoid loading @telivityhaip/booking-requests at startup when
HAIP_BOOKING_REQUESTS is off (docker demo). Preload before Nest bootstrap
when the flag is enabled; read the flag from core payment seams in
DatabaseModule.

Co-authored-by: telivity-otaip <telivity-otaip@users.noreply.github.com>

* fix(api): type bootstrap cache as DynamicModule array

Co-authored-by: telivity-otaip <telivity-otaip@users.noreply.github.com>

* fix(booking-requests): address Agustin packaging/safety review

- Ship runnable migrator + SQL in dist; production db:migrate via node
- Wire VITE_HAIP_BOOKING_REQUESTS through Docker/compose/release
- Reject bookingMode=request when HAIP_BOOKING_REQUESTS is off
- Ledger + checksum for package migrations (auto-commit for PG enums)
- Drop unused stripeHandlerToken from root module options
- Fix EmailResult outcomeUnknown after #349 status contract
- Point regression/e2e installs at run-migrations.js (post-#350)
- Harden push-schema CLI path resolution; sync README (2175/259)

Co-authored-by: telivity-otaip <telivity-otaip@users.noreply.github.com>

* fix(database): keep logical_event_id out of push-schema baseline

#350 moved webhook logical_event_id to migration 0022. Remove the column
and unique index from push-schema so the pre-0022 upgrade regression
passes again.

Co-authored-by: telivity-otaip <telivity-otaip@users.noreply.github.com>

* chore: sync README test counts to 2180/260

Matches CI after push-schema logical_event_id cleanup (all packages green).

Co-authored-by: telivity-otaip <telivity-otaip@users.noreply.github.com>

* test(booking-requests): flag-off regression gate + port PR #347 safety specs

- Add a flag-OFF default-install regression spec: only core migrations run,
  HAIP_BOOKING_REQUESTS is unset, AppModule boots without the
  booking-requests module/tables, request mode is rejected, and instant
  booking + deposit capture + partial/full refund still work. Runs
  automatically under `pnpm test` (apps/api's normal *.spec.ts glob), so it
  is wired into CI without any workflow changes.
- Extract the ephemeral-database subprocess helpers shared by that spec and
  the existing default-flow regression spec into
  regression-database-utils.ts (createdb/dropdb, sanitized child-process
  errors) instead of duplicating them.
- Port PR #347's booking-request-schema.spec.ts and
  booking-request-migration-safety.spec.ts into the package, scoped to the
  tables/migrations this package now owns (the duplicate push-schema DDL
  those specs cross-checked no longer exists — it moved into this package's
  migrations).
- Port PR #347's booking-request-remediation.postgres.spec.ts, replaying
  migration 0032's SQL directly (instead of through push-schema) since the
  ledger-based migrator can't be re-run against a manually-reverted schema.
  Kept opt-in via BOOKING_REQUEST_REMEDIATION_LIVE_PG like the original.
- Document remaining scope (vertical-slice move, push-schema de-pollution)
  in the package README instead of attempting it in this pass.
- Sync README/test-stats test counts (2211 tests, 263 files).

Co-authored-by: telivity-otaip <telivity-otaip@users.noreply.github.com>

* feat(booking-requests): package-owned port for booking_mode/payment_method_collection/form_questions

Adds BOOKING_REQUEST_CONFIG_FIELDS_PORT + DrizzleBookingRequestConfigFieldsAdapter
so the package owns reading/writing booking_engine_config's request-mode-only
columns via its own Drizzle table fragment, instead of core declaring them.
Wired into BookingRequestModule.forRoot() (global) so it's injectable into
core's BookingEngineConfigService without that module importing this package.

Co-authored-by: telivity-otaip <telivity-otaip@users.noreply.github.com>

* chore(database): remove request-only DDL from core push-schema/drizzle

Removes booking_mode/payment_method_collection/form_questions ALTERs on
booking_engine_config, audit_logs.booking_request_id (+ its timeline index),
and the request-shape unique indexes/checks on payments
(payments_property_request_*_unique, booking_request_parent_positive_check,
booking_request_child_shape_check) from core's push-schema.ts and Drizzle
schema. These are now declared and migrated exclusively by
packages/booking-requests. payments.booking_request_id/idempotency_key,
reservations.accepted_pricing_snapshot, and charges
adjusts_charge_id/source_key stay in core as documented in
push-schema-kept-fields.spec.ts.

Also adds the DRIZZLE injection token to @telivityhaip/database so packages
outside apps/api (booking-requests) can inject the shared Drizzle client
without importing apps/api.

Co-authored-by: telivity-otaip <telivity-otaip@users.noreply.github.com>

* refactor(booking-requests): move Nest vertical slice into package

Own controllers/services/DTOs behind package ports; strip request-only
audit DDL and payment indexes from core schema; keep thin config/payment hooks.

- Nest controllers, DTOs, services, Stripe handler, and pricing/money/
  state/db/ledger/reconciler/template helpers now live in
  packages/booking-requests/src (http/ + domain/), with their unit specs.
  BookingRequestModule.forRoot(...) is a real DynamicModule owning those
  controllers/providers directly, not a facade over apps/api classes.
- apps/api/src/modules/booking-request/ keeps only the e2e, authorization,
  default-flow-regression, flag-off-instant-booking.regression, and
  transaction-seams specs plus regression-database-utils.ts.
- apps/api/src/booking-requests.bootstrap.ts wires every package-local port
  (folio, webhook, email, reservation, rate-plan, guest, ancillary,
  availability, booking-engine, booking-engine-config, plus guard-bridge
  ports) to the concrete core singleton via `useExisting`.
- Guard bridge classes (BookingKeyGuardBridge, BookingEngineScopeGuardBridge,
  BookingThrottleGuardBridge) resolve the "guards landmine": @UseGuards(...)
  is populated from decorator metadata, a separate path from `providers`, so
  a bare useExisting binding on an abstract port class is silently dropped —
  the bridges are real injectable classes referenced in @UseGuards(...).
- DRIZZLE token declaration moved to @telivityhaip/database; apps/api's
  DatabaseModule still @Global-provides it and merges in the package's
  optional schema only when HAIP_BOOKING_REQUESTS is on.
- Relocated pure/shared pieces to packages/shared: SAVED_PAYMENT_METHOD_GATEWAY
  / PAYMENT_GATEWAY / BOOKING_REQUEST_STRIPE_HANDLER interfaces, IsMoneyString,
  canonical calendar date validators, stayDates, AuditActor helpers,
  RequirePermissions/Public decorators, stripe-financial-state helpers, and
  the pure payment-ledger math (remainingCapturedAmount/sumRefundChildren) —
  bookingRequestPaymentSumWhere stays in core payment-ledger.
- Schema de-pollution: removed audit_logs.booking_request_id (+ its timeline
  index) and the request-shape payments unique indexes/checks from core
  push-schema/drizzle; kept booking_engine_config.booking_mode /
  payment_method_collection / form_questions, payments.booking_request_id /
  idempotency_key, reservations.accepted_pricing_snapshot, and charges
  adjusts_charge_id/source_key as thin config/payment hooks core still reads
  directly. packages/database/src/push-schema-kept-fields.spec.ts guards the
  contract; packages/booking-requests declares its own local audit table
  extension for the timeline index it still owns.
- EventEmitterModule.forRoot() re-enabled `wildcard: true` — required for the
  webhook fan-out (@onevent('**')) to receive booking-request events.
- README's "Package boundary" section replaces the old "Remaining work"
  deferrals list with an accurate description of the current split.

Co-authored-by: telivity-otaip <telivity-otaip@users.noreply.github.com>

* fix(docker): ship shared node_modules for Nest peer in prod image

Shared now requires @nestjs/common at runtime after the booking-requests
boundary move; copy packages/shared/node_modules into the API image so
demo smoke can boot. Sync published test counts to 2222/266.

Co-authored-by: telivity-otaip <telivity-otaip@users.noreply.github.com>

* fix(api): load AppModule after booking-requests preload

When HAIP_BOOKING_REQUESTS=true, AppModule evaluates bookingRequestsModules()
at import time; dynamic-import AppModule after preload so flag-on dev/prod
boot does not crash before Nest starts.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@agustinjch agustinjch Awaiting requested review from agustinjch

Assignees

@telivity-otaip telivity-otaip

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@telivity-otaip @agustinjch @cursoragent