fix(pay-05): correct UserInvestmentStore PK shape + InvestmentPayment GSI-name default by ff-team-sobrado · Pull Request #11 · functorful/stripe-webhook

ff-team-sobrado · 2026-05-12T19:28:17Z

Why

Companion to infrastructure MR !51 (gitlab). Tomás's PAY-05 Phase 2b Option (c) drill in DEV on 2026-05-12 caught TWO empirically-confirmed deploy-blocking bugs in the PAY-05 Phase 2b cascade:

GSI-name mismatch — IaC env-var set INVESTMENT_PAYMENT_BY_RESERVATION_INDEX_NAME = "investmentPaymentByReservation"; Amplify Gen 2 actually generated gsi-InvestmentReservation.payments. Fixed via MR !51 (IaC env-var bump).
UserInvestmentStore PK-shape mismatch — handler built put item with userId HASH + investmentId#investmentVersion RANGE; deployed schema is id HASH only. TWI cancels at op 3 → 200 to Stripe + zero writes + reconciliation drift. Fixed in this PR.

After bug 1 was fixed via MR !51, a re-drill of Row 2 would have hit bug 2 immediately. This PR is the handler-side companion fix needed to unblock the cutover. Plus a small dormant-tech-debt correction on the handler-default mirror.

What changes

File	Change
`UserInvestmentStore.java`	THE fix: generate `id` UUID HASH key; drop composite from PK construction (keep as non-key item attribute for `UserInvestmentByUserAndInvestment` GSI sort key); condition `attribute_not_exists(id)`; rewrite javadoc to reflect deployed schema
`InvestmentPaymentProperties.java`	Handler-default mirror: `DEFAULT_BY_RESERVATION_INDEX_NAME` from `"investmentPaymentByReservation"` → `"gsi-InvestmentReservation.payments"` (matches MR !51's IaC env-var canonical value)
`InvestmentPaymentStore.java`	Javadoc fix: GSI-name override block references the new canonical default; cross-link to MR !51 + HYG-19 axis-4
`UserInvestmentStoreTest.java` (NEW)	Structural-prevention test class: 6 tests asserting put-item shape against deployed schema (the gap that let the bug slip through PR review)

Verified deployed schema (the source of truth)

$ aws dynamodb describe-table --table-name UserInvestment-2mm6zfbtrngy5jqblv7nna6b2e-NONE --profile ff-dev
{
  "Table": {
    "KeySchema": [{"AttributeName": "id", "KeyType": "HASH"}],
    "AttributeDefinitions": [
      {"AttributeName": "id", "AttributeType": "S"},
      {"AttributeName": "investmentId", "AttributeType": "S"},
      {"AttributeName": "investmentId#investmentVersion", "AttributeType": "S"},
      {"AttributeName": "investmentVersion", "AttributeType": "N"},
      {"AttributeName": "userId", "AttributeType": "S"}
    ],
    "GlobalSecondaryIndexes": [
      {"IndexName": "gsi-Investment.userInvestments",
       "KeySchema": [{"AttributeName": "investmentId", "KeyType": "HASH"},
                     {"AttributeName": "investmentVersion", "KeyType": "RANGE"}]},
      {"IndexName": "UserInvestmentByUserAndInvestment",
       "KeySchema": [{"AttributeName": "userId", "KeyType": "HASH"},
                     {"AttributeName": "investmentId#investmentVersion", "KeyType": "RANGE"}]}
    ]
  }
}

PK is id (S) HASH only — Amplify Gen 2 default UUID identifier. The composite investmentId#investmentVersion attribute IS in AttributeDefinitions, but as the GSI sort key for UserInvestmentByUserAndInvestment, NOT as a base-table key.

Audit findings (no other bugs surfaced)

Per team-lead's "audit before fix" directive: cross-checked every PAY-05 Phase 2b DDB operation against the deployed schemas of all 5 tables (InvestmentReservation, InvestmentPayment, UserInvestment, AuditLog, WebhookEvent). Only UserInvestmentStore was structurally wrong. All other stores (InvestmentReservationStore, InvestmentPaymentStore.findByReservationAndIntent/buildStatusUpdate, AuditLogStore, WebhookIdempotencyStore) verified correct.

Full audit table in Rui→team-lead DM thread 2026-05-12; will copy verbatim into HYG-40 once Tomás files it.

Test plan

6/6 new UserInvestmentStoreTests pass — structural assertions on id HASH key + composite attribute preservation + condition expression + table name + UUID-per-invocation distinctness
82/82 total tests pass (up from 76) — ./gradlew --rerun-tasks test clean run
HYG-19 axis-1 trigger check: no @Inject / @Singleton modifier changes — handler-side store mutation only
HYG-19 axis-2 trigger check: no build.gradle deps / --initialize-at-build-time / native-image buildArgs changes
HYG-19 axis-3 trigger check: no new IAM / KMS / Secrets references
HYG-19 axis-4 trigger check: ✅ THIS IS THE CANONICAL AXIS-4 ANCHOR (env var + bean-construction chain with handler-assumed schema). The structural-prevention test class added here IS the static layer (PR-time put-item.keys assertion vs deployed PK).
HYG-19 axis-5 trigger check: no fluent-builder customization / reflect-config delta
DEV smoke gate (post-merge): Tomás re-drills Row 2 with the seeded fixtures still in DEV DDB. Expect TWI to succeed; all 4 writes land; SES email fires; reservation transitions to executed; UserInvestment row visible via UserInvestmentByUserAndInvestment GSI query

Why this is one PR (not split)

Three changes; all share the same empirical anchor (Tomás's 2026-05-12 drill); all touch the same dynamodb/ package; all needed for PAY-08b cutover to unblock cleanly. Splitting into three PRs would add Miguel-click overhead (3× Owner approval) for no review-quality benefit — the diffs are independent at the file level but causally bound at the cycle level. Bundling matches the "team drives to merge with drilling" discipline (feedback_team_drives_to_merge_with_drilling).

Sequencing

THIS PR merges (Tomás security pair → Miguel Owner approval).
Backend pipeline produces rc0.0.11-<sha> → manual retag to v0.0.12-dd via aws ecr put-image (HYG-12 precedent shape; happens on GHCR pull-through-cache cycle).
Infrastructure IaC bump: stripe_webhook_image_tag v0.0.11-dd → v0.0.12-dd.
DEV apply → Tomás re-drills Row 2 with seeded fixtures → expect 7/7 GREEN on HYG-38 matrix.
Sofia re-engages for PRD cutover approval.

Refs

Infrastructure MR !51 (companion IaC env-var fix): https://gitlab.com/functorful/projects/sobrado/infrastructure/-/merge_requests/51
HYG-19 (DEV-smoke-gate-before-MR + 5-axis trigger surface): https://www.notion.so/35ed485396648140bc0cd6a195c819eb
HYG-40 (Tomás filing — PAY-05 Phase 2b audit + ongoing-drill discipline): forthcoming
Drill empirical anchor: Tomás DM thread 2026-05-12T19:03:45Z
ARCH-12 (verify-pass that confirmed stripe-webhook + revolut-webhook prd-native): https://www.notion.so/35bd4853966481369111c0a4d8a0a2c0

🤖 Generated with Claude Code

@ConfigurationProperties

… GSI-name default Bundles two related PAY-05 Phase 2b corrections + a structural-prevention test class, all surfaced by Tomás's Option (c) drill in DEV on 2026-05-12. ## Bug 1 — UserInvestmentStore PK shape (handler-side companion to MR !51) Pre-fix the store built the put item with `userId` HASH + `investmentId#investmentVersion` RANGE, assuming a composite base-table key analogous to InvestmentReservation. Verified deployed schema via `aws dynamodb describe-table`: KeySchema: id (S, HASH) ← Amplify Gen 2 default UUID identifier only — no sort key on the base table The `investmentId#investmentVersion` attribute is in AttributeDefinitions, but as the GSI sort key for `UserInvestmentByUserAndInvestment`, not as a base-table key. Pre-fix the put was missing the actual `id` HASH key, so the surrounding TransactWriteItems cancelled with a ValidationException-class failure at operation 3 in PaymentIntentSucceededHandler. Bug surface: - Stripe `payment_intent.succeeded` arrives, signature-verifies - Audit-log writer fires (PAY-08; post-verify, correct) - InvestmentReservation load + status guard pass - InvestmentPayment load + status guard pass - TransactWriteItems with 4 ops: payment update, reservation update, UserInvestment put (THIS), AuditLog put - Op 3 cancels (missing HASH key) → atomic rollback → no DDB writes, no SES email, but 200 to Stripe (TransactionCanceledException is in the handler's exception catch per Tomás §10 R4) - Reconciliation drift: Stripe has the charge, Sobrado has nothing. Fix: - Generate `id` (random UUID, same pattern as AuditLogStore). - Drop `investmentId#investmentVersion` from PK construction. KEEP it as a non-key item attribute so the `UserInvestmentByUserAndInvestment` GSI remains queryable (Amplify writes the composite alongside the scalar fields). - Change conditionExpression from `attribute_not_exists(userId)` to `attribute_not_exists(id)`. With a fresh UUID this is structurally a tautology; the load-bearing duplicate-prevention is the surrounding reservation-status guard in the TWI envelope (a replay finds the reservation already `executed`, the conditional check fails on the reservation Update, and the entire TWI rolls back — no duplicate UserInvestment row is possible). Empirical anchor: software.amazon.awssdk.services.dynamodb.model.DynamoDbException: The table does not have the specified index: investmentPaymentByReservation (That message was Bug #2 — the GSI-name mismatch — fixed via infrastructure MR !51. This MR is the handler-side companion fix on the UserInvestment surface that would have been the NEXT failure when re-drilled with the GSI-name fix in place.) ## Bug 2 (handler-default mirror) — InvestmentPaymentProperties Pre-fix: static final String DEFAULT_BY_RESERVATION_INDEX_NAME = "investmentPaymentByReservation"; Post-fix: static final String DEFAULT_BY_RESERVATION_INDEX_NAME = "gsi-InvestmentReservation.payments"; Matches the Amplify Gen 2 auto-generated GSI naming convention (`gsi-<ParentModel>.<relationshipField>`) for `hasMany` ↔ `belongsTo` relations. Dormant tech-debt: the env var set by IaC (MR !51) overrides this default in prd, so the broken default never actually fired at runtime — but keeping the source-side default in sync with the deployed-reality canonical name prevents future drift if a Lambda ever runs without the env var explicitly set. ## Bug 0 (audit-discovered) — javadoc drift Updated javadoc on `UserInvestmentStore`, `InvestmentPaymentStore`, and `InvestmentPaymentProperties` to reflect the deployed reality. Cited HYG-19 axis-4 + MR !51 + the drill anchor. ## Structural-prevention layer — new UserInvestmentStoreTest Added a dedicated store-level test class with 6 tests asserting the put-item shape against the deployed schema: - putItemHasIdHashKeyAsRandomUUID - putItemDoesNotIncludeCompositeAttributeAsBaseTableKey (regression test for THIS bug class) - putItemIncludesAllDomainAttributesAsScalars - putItemHasAttributeNotExistsConditionOnIdHashKey - putTargetsCorrectTable - differentInvocationsProduceDistinctIds This closes the structural gap that let the bug slip through PR review: no store-level test asserted the put-item shape against the deployed table's KeySchema. The handler tests mocked the store entirely (verifying "the store's mock was called" but not "what the store actually produces"). HYG-19 axis-4 static-prevention layer literally points at this: a store- test asserting put-item.keys ⊆ deployed-PK-attribute-set would have caught BOTH bugs (GSI-name AND UserInvestment PK) at PR-time, not at drill-time. Broader-pattern follow-up flagged separately: all PAY-05 Phase 2b stores (UserInvestmentStore + InvestmentReservationStore + AuditLogStore) should have parallel store-level shape-assertion tests. Filing a companion HYG-N ticket post-merge. ## Audit findings (no other bugs surfaced) Cross-checked every PAY-05 Phase 2b DDB operation against deployed schemas. Only the UserInvestment put was structurally wrong. Full audit table in HYG-40 (Tomás filing); my findings in tomas/team-lead DM thread 2026-05-12. ## Tests - 82/82 tests pass (was 76; +6 new UserInvestmentStoreTests). - `./gradlew --rerun-tasks test` clean run. ## HYG-19 axis-4 canonical anchor (post-cascade) This bug pair (MR !51 + this MR) is THE canonical empirical anchor for HYG-19 axis 4 ("new env vars consumed via @ConfigurationProperties or @value" AND "handler-assumed shape silently passes startup, fails at first runtime use of the DDB operation"). Will bank in HYG-19 Notion ticket once PAY-08b cutover stabilizes. ## Coordination - Tomás: security pair-review on the diff shape. No security surface delta — same Lambda, same IAM, same KMS. Only the constructed put-item shape and a properties-bean constant change. - Miguel: Owner-tier approval per the GitHub-side identity-collapse constraint (`feedback_github_bot_identity_collapse_routes_to_owner`). - Tomás re-drills Row 2 with seeded fixtures (still in DEV DDB) post-merge. ## Refs - Infrastructure MR !51 (companion GSI-name fix) - HYG-19 Notion: https://www.notion.so/35ed485396648140bc0cd6a195c819eb - HYG-40 Notion: (Tomás filing — PAY-05 Phase 2b structural audit + ongoing-drill discipline) - Drill empirical anchor: Tomás DM 2026-05-12T19:03:45Z - AuditLogStore — canonical UUID-id pattern reference - WebhookEvent table — single-HASH-key reference pattern 🤖 Generated with [Claude Code](https://claude.com/claude-code)

miguelaferreira approved these changes May 12, 2026

View reviewed changes

ff-team-sobrado merged commit 3ce846b into main May 12, 2026
4 checks passed

ff-team-sobrado deleted the fix/pay-05-userinvestment-pk-shape branch May 12, 2026 19:36

Navigation Menu

Uh oh!

fix(pay-05): correct UserInvestmentStore PK shape + InvestmentPayment GSI-name default - #11

fix(pay-05): correct UserInvestmentStore PK shape + InvestmentPayment GSI-name default#11
ff-team-sobrado merged 1 commit into
mainfrom
fix/pay-05-userinvestment-pk-shape

ff-team-sobrado commented May 12, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

ff-team-sobrado commented May 12, 2026

Why

What changes

Verified deployed schema (the source of truth)

Audit findings (no other bugs surfaced)

Test plan

Why this is one PR (not split)

Sequencing

Refs

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants