feat(pay-05): real handler bodies (Phase 2b — DDB stores + SES + sanitization)

[ff-team-sobrado]

Summary

PAY-05 Phase 2b — full implementation of payment_intent.succeeded and payment_intent.payment_failed handlers per docs/pay-05-handler-design.md §4.1 + §4.2. Stacked on PR #5 (now merged). Closes the BLOCKING amendment Tomás M-NEW-1 raised on infrastructure!14 (degraded-mode short-circuit on the AuditLog SSM-bridge sentinel).

Phase 2b body work (carried forward from the original scope)

Stores + services (raw-DDB discipline, not Enhanced client)

• InvestmentReservationStore, InvestmentPaymentStore, UserInvestmentStore, AuditLogStore — all Update/Put builders so the handler composes a single TransactWriteItems per event.
• SesEmailService — Phase-1 placeholder English copy, URL-connection HTTP client (native-image-friendly), best-effort failure (returns false, never throws). PAY-21 swaps in pt-PT templated copy.
• SesV2ClientFactory — @Replaces(SesV2Client.class) mirroring the Secrets Manager / DynamoDB factory pattern.
• ReservationKey — Stripe-metadata-parsed record; IllegalArgumentException carries field names only (no values, M1 hygiene applied to error paths).

Real handlers

• PaymentIntentSucceededHandler.handle(...) — full §4.1 sequence: extract → load + guard → atomic 4-op TransactWriteItems (payment processed → success, reservation confirmed → executed, idempotent Put UserInvestment, Put AuditLog) → SES best-effort. TransactionCanceledException-only catch per Tomás §10 R4.
• PaymentIntentFailedHandler.handle(...) — full §4.2: same load+guard discipline; sanitises lastPaymentError.message (cap to ≤500 chars + strip control chars per §10 M1; logs Stripe's errorCode only); branches reservation target on expiresAt vs event.created (pending for retry-window-open, expired past-window); 3-op TWI; no email.

M-NEW-1 — degraded-mode short-circuit (BLOCKING amendment from infrastructure!14)

Both handlers now detect the "PENDING_AUDIT_LOG_BRIDGE" sentinel that Tofu writes into AUDIT_LOG_TABLE_NAME when var.audit_log_ssm_bridge_ready=false, and short-circuit before any DDB read or write. Without this:

1. Stripe webhook arrives. PAY-02 HMAC + idempotency record succeed.
2. Handler builds the 4-item TWI; the AuditLog PutItem targets a non-existent table.
3. DDB cancels the entire transaction atomically — payment + reservation + UserInvestment + AuditLog all roll back.
4. Handler returns 200, Stripe stops retrying, and Sobrado has zero record of the payment.

Behavioural contract on degraded mode:

• ERROR log at construction with the canary "DEGRADED MODE" + the sentinel value (one-line ops signal "bridge not ready").
• ERROR log at every dispatch carrying eventId + eventType + the same canary.
• No DDB reads, no DDB writes, no SES, no markProcessed. The WebhookEvent row stays processed=false so the §6 / M3 alarm fires.

Sentinel string identity ("PENDING_AUDIT_LOG_BRIDGE") and canary ("DEGRADED MODE") are structurally coupled to:

• aws-stripe-webhook-lambda.tofu (env-var collapse value, already in infrastructure!14).
• aws-webhook-events-monitoring.tofu log-metric-filter pattern alternation ?"M3 alarm will surface this" ?"DEGRADED MODE" (ships in the follow-up infrastructure MR opened alongside this PR).

Tests — 78 total, 0 failures

Class	Tests
PaymentIntentSucceededHandlerTest	10 (incl. M-NEW-1 degraded mode + M4 case 16)
PaymentIntentFailedHandlerTest	14 (incl. M-NEW-1 degraded mode + M4 case 16 + M1 sanitisation)
WebhookEventDispatcherTest	10
IgnoredEventHandlerTest	5
WebhookIdempotencyStoreTest	2
WebhookEventProcessorTest	15 (incl. M4 case 17 fallback + body-echo regression)
StripeSignatureVerifierTest	14
secret/*	8

./gradlew test: BUILD SUCCESSFUL.

Tomás's M1–M4 / R1–R4 + M-NEW-1 closure

Item	Status	Where
M1 errorMessage cap+sanitise	✅	PaymentIntentFailedHandler.sanitizeErrorMessage
M2 receivedAt in AuditLog.details	✅	both handlers' buildAuditDetails
M3 processed=false > 5min alarm	✅ partial-coverage (markProcessed canary) shipped in infrastructure!14; strict periodic-scan promoted to Phase-1 launch gate (M-NEW-2)
M4 defensive event-shape tests	✅	case 16 (missing data.object.id) in both handler tests; case 17 (missing/bad event.created) in WebhookEventProcessorTest
R1 IAM wildcard scoping comment	✅	aws-stripe-webhook-iam.tofu (infrastructure!14)
R2 SES Condition: ses:FromAddress	✅	aws-stripe-webhook-iam.tofu (infrastructure!14)
R3 lambdaVersion+gitSha in details	✅	both handlers' constructor @Value("${dd.version}") + @Value("${git.sha}")
R4 TransactionCanceledException-specific catch	✅	both handlers (NOT generic Exception)
M-NEW-1 degraded-mode short-circuit	✅ THIS PR	both handlers' degradedMode flag + new unit test in each handler test class

Cross-track impact

• PAY-21 (SES email): this PR ships the email trigger with English placeholder copy. PAY-21 swaps in pt-PT templated copy + investor-email lookup (Stripe's receipt_email is not always populated by PaymentLambda; PAY-21 closes that gap).
• PAY-09 (Flutter PaymentBloc): no change — this PR writes only the existing executed/failed/expired/success enum values the schema already declares. The §9.3 design doc pin (unit test asserting handlers write only documented status values) covers regressions.
• PAY-23 (PII CMK): respected — no InvestorIban access, no PII CMK Decrypt grants.
• PAY-07 (reconciliation): the AuditLog row this handler writes is the in-DDB business-event audit trail. The S3 raw-webhook-body bucket is PAY-08 (separate); the Stripe-vs-DDB daily diff is PAY-07.

Apply order to clear the AuditLog SSM bridge gate

1. Application MR (TBD) adds "AuditLog" to amplify/backend.ts:tablesToExport, deploys.
2. Infrastructure: set var.audit_log_ssm_bridge_ready = true in secrets.auto.tfvars, replan + reapply.
3. Lambda's AUDIT_LOG_TABLE_NAME resolves to the real table name on the next image roll. Degraded-mode auto-clears at cold start.

Until step 3 lands, the Lambda runs in degraded mode end-to-end: handlers short-circuit, the "DEGRADED MODE" ERROR fires per dispatch, the M3 alarm extension catches it. Zero risk of silent payment data loss during the cross-repo deploy window.

Companion infrastructure MR

Follow-up MR adds the M-NEW-1 alarm-pattern alternation (?"M3 alarm will surface this" ?"DEGRADED MODE") to aws-webhook-events-monitoring.tofu. Same review pair, separate scope.

PR review pair

• Rui primary (handler architecture, store discipline, M-NEW-1 wiring, test surface).
• Tomás security review (M-NEW-1 patch verification, M1+M2+M4 verification in code, R1+R4 in code, sanitisation correctness).

References

• Notion ticket: https://www.notion.so/34fd48539664810c9036d028db1b5e39
• Design contract: docs/pay-05-handler-design.md (§9 Tomás review log + §11 M-NEW-1 fold-in)
• Stacked-on PR: feat(pay-05): event handler scaffold + dispatcher refactor (design checkpoint, no behaviour change) #5 (merged)
• Companion infra MRs: infrastructure!14 (merged) + follow-up alarm MR (TBD link)
• Tomás M-NEW-1 review note: https://gitlab.com/functorful/projects/sobrado/infrastructure/-/merge_requests/14#note_3309173324

[ff-team-sobrado]

Posted on behalf of Tomás (Security Champion) — same pattern as the prior PR #5 reviews. Tomás cannot post directly from his sub-agent session (GitHub MCP doesn't surface there). The team bot identity is the GitHub author of this review. Authorship is Tomás's; verbatim below.

Verdict at a glance: APPROVE for PR #6 cleanup commit 0186148. However: a companion BLOCKING amendment on infrastructure!14 (M-NEW-1, silent-payment-data-loss vulnerability in the audit_log_ssm_bridge_ready=false deploy window) requires a small handler change in this same Lambda code repository before the Phase-2b deploy is safe. The M-NEW-1 fix lands as a follow-up commit on this branch (feat/pay-05-phase-2b-handler-bodies) before merge. See /tmp/pay-05-mr14-review.md §M-NEW-1 (also at https://gitlab.com/functorful/projects/sobrado/infrastructure/-/merge_requests/14#note_3309173324) for full code shape + unit test + extended alarm pattern.

---

Tomás re-review — PR #6 cleanup commit 0186148: APPROVE

Verified the two cleanups Rui-backend added on top of PR #6's prior body — markProcessed + Tomás §10 M4 case 17 explicit tests. Both are in the right shape; APPROVE for merge once PR #5 lands and PR #6 rebases onto main.

WebhookIdempotencyStore.markProcessed(eventId) ✅

Implementation in src/main/java/com/functorful/stripewebhook/idempotency/WebhookIdempotencyStore.java:

• attribute_exists(eventId) condition — defends against the unlikely "mark something that doesn't exist" race (won't happen given the recordFirstDelivery → handler order, but belt-and-braces).
• WARN-not-ERROR severity — correct: business state is already in DDB at this point; the M3 alarm is the safety net for any row that lingers.
• Canary string "M3 alarm will surface this if the row stays false for >5min" in the log line — structurally coupled to the infra-side log-metric-filter pattern in aws-webhook-events-monitoring.tofu. Drift between the two would silently break detection; pinning the exact string in code review is the only mitigation. Pinned.
• Logs eventId + errorClass.getSimpleName() only — no PII leakage, no exception message that could carry user-supplied content.
• Catches RuntimeException (broad) — correct for this path. The narrow ConditionalCheckFailedException discrimination my §10 R4 asked for applies to the TWI catch in the handlers, NOT to markProcessed (where the noise/signal trade-off favours catching everything → WARN → M3 alarm).
• @NewSpan parity with the rest of the store. ✅

Both handlers call markProcessed after TWI ✅

PaymentIntentSucceededHandler and PaymentIntentFailedHandler invoke idempotencyStore.markProcessed(event.eventId()) after the TWI returns. Tests assert via verify(idempotencyStore).markProcessed(EVENT_ID).

M4 case 17 explicit tests ✅

WebhookEventProcessorTest:

• missingEventCreatedFallsBackToReceiverClock — orchestrator falls back to Instant.now() when event.created is absent.
• nonNumericEventCreatedFallsBackToReceiverClock — same fallback when the field is present but non-numeric.

The fallback was already present in WebhookEventProcessor.buildEvent(...); these tests pin the contract so a future "clean up the orchestrator" refactor can't silently drop it.

Caveat — does NOT close MR !14's BLOCKING amendment

This APPROVE covers only the PR #6 commit 0186148. The companion infrastructure MR !14 carries one BLOCKING amendment (M-NEW-1: silent-payment-data-loss vulnerability in the audit_log_ssm_bridge_ready=false deploy window) that requires a small handler change in this same Lambda code repository before the Phase-2b deploy is safe.

Specifically: both PaymentIntentSucceededHandler and PaymentIntentFailedHandler need a degraded-mode short-circuit when AUDIT_LOG_TABLE_NAME is the literal sentinel "PENDING_AUDIT_LOG_BRIDGE". Without it, the handler attempts a 4-item TWI against a non-existent AuditLog table, all 4 writes roll back atomically, Stripe gets 200, and Sobrado has zero record of the payment. See /tmp/pay-05-mr14-review.md §M-NEW-1 for the full code shape + unit test + extended alarm pattern.

Recommended merge order

1. PR #5 merges first (squash, per team-lead's plan).
2. M-NEW-1 patch lands as a follow-up commit on feat/pay-05-phase-2b-handler-bodies (degraded-mode handler short-circuit + unit test).
3. PR #6 rebases onto main; security re-review of the M-NEW-1 patch (small surface — should be a fast pass).
4. infrastructure!14 lands the matching alarm-pattern alternation in aws-webhook-events-monitoring.tofu.
5. PR #6 + infrastructure!14 apply together. The first deploy uses audit_log_ssm_bridge_ready=false and the handler is in degraded mode (loud, alarmed); flag flips to true after the application repo's tablesToExport MR lands; degraded mode auto-clears on next Lambda image roll.

— Tomás (Security Champion)

[ff-team-sobrado]

Posted on behalf of Tomás (Security Champion) — same pattern as the prior PR #5 / PR #6 reviews. Tomás cannot post directly from his sub-agent session (GitHub MCP doesn't surface there). The team bot identity is the GitHub author of this review. Authorship is Tomás's; verbatim below.

---

Tomás re-review — M-NEW-1 patch (06e0060) + merge-of-main (528aefc): APPROVE

This is the re-review I committed to in the prior 0186148 APPROVE (which carried the explicit caveat that the M-NEW-1 amendment from infrastructure!14 had to land in this same repository before the Phase-2b deploy could be considered safe). Both commits in scope here:

• 06e0060 — degraded-mode short-circuit on the PENDING_AUDIT_LOG_BRIDGE sentinel.
• 528aefc — merge-of-main resolution after PR #5 squash; "ours" resolution verified correct (HEAD is a strict superset).

Verdict

APPROVE for merge. All nine items on my standing PAY-23-style mandatory checklist verified against the patch. One operational caveat documented at the bottom — coordination, not a code defect.

Verification — code-side checklist (9/9 PASS)

#	Check	Verdict	Evidence
1	Short-circuit fires before any DDB read or write	✅	if (degradedMode) { log.error(...); return; } is the very first statement in both handlers' handle(...) after @NewSpan. Verified in both PaymentIntentSucceededHandler.java and PaymentIntentFailedHandler.java.
2	Short-circuit fires before any SES call	✅	Succeeded handler — sesEmailService is only invoked after the TWI; degraded path returns before reaching it.
3	Short-circuit does not call markProcessed	✅	markProcessed(event.eventId()) is only reached after the TWI succeeds in both handlers. Degraded path returns earlier. The WebhookEvent row stays processed=false so the M3 alarm path remains intact (caveat below on alarm-pattern landing).
4	Sentinel string identity matches Tofu literal exactly	✅	Verified across both sides: AUDIT_LOG_BRIDGE_PENDING_SENTINEL = "PENDING_AUDIT_LOG_BRIDGE" (Java constant, both handlers) ↔ aws-stripe-webhook-lambda.tofu:122 (AUDIT_LOG_TABLE_NAME = var.audit_log_ssm_bridge_ready ? ... : "PENDING_AUDIT_LOG_BRIDGE").
5	Canary string "DEGRADED MODE" lands in the dispatch ERROR log	✅	log.error("PAY-05 handler in {} — AuditLog SSM bridge not ready ...", DEGRADED_MODE_CANARY, event.eventId(), event.eventType()) in both handlers.
6	No PII leakage in degraded-mode log lines	✅	Construction log emits sentinel value (constant) + canary (constant); dispatch log emits eventId (Stripe evt_xxx) + eventType (payment_intent.succeeded / .payment_failed) + canary. No user IDs, no amounts, no email addresses, no last_payment_error.message.
7	Tests verify zero side effects under the sentinel	✅	degradedMode_doesNotTouchDdbAndDoesNotMarkProcessed in both PaymentIntentSucceededHandlerTest and PaymentIntentFailedHandlerTest. Each asserts: verify(reservationStore, never()).load(any()), verify(paymentStore, never()).findByReservationAndIntent(...), verify(dynamoDbClient, never()).transactWriteItems(...), verify(idempotencyStore, never()).markProcessed(any()), plus verify(sesEmailService, never()).sendPaymentConfirmation(...) on the succeeded side.
8	Existing tests' constructors updated to pass a non-sentinel auditLogTableName	✅	Both setUp() blocks now pass "AuditLog-test-table" to the test-seam constructor — confirmed normal-mode tests still exercise the full TWI + markProcessed path. CI: 78/78 GREEN.
9	Bean fail-closed on missing env var	✅	@Value("${audit-log.table-name}") carries no default — Micronaut bean construction throws if the env var is unset, fail-closed. Lambda init aborts rather than silently entering degraded mode on a config bug.

PAY-23 territory respected

Re-confirmed across all 17 changed files: no InvestorIban access, no sobrado-pii-cmk Decrypt grant, no pii-crypto-layer import. PAY-22 lane is uncontaminated.

Devil's-advocate review (none reach blocking)

• Race during config flip / image roll. When var.audit_log_ssm_bridge_ready flips from false → true, old containers in degraded mode race with new containers in normal mode. Both serve traffic during rollover. Old-container events stay processed=false and either get retried by Stripe or are manually replayed; new-container events flow normally. No data corruption — accepted.
• What if the sentinel matches accidentally? "PENDING_AUDIT_LOG_BRIDGE" is a deliberately unusual literal — extremely unlikely to be a legitimate DDB table name. Accepted.
• Construction-time log spam. Single ERROR per cold start (Micronaut @Singleton). Accepted — degraded Lambda should be loud.
• Dispatch-time log spam. Every event in degraded mode emits an ERROR. Accepted: in degraded mode you're already dropping events on the floor; the spam IS the desired signal.
• @Value property mapping. audit-log.table-name ← AUDIT_LOG_TABLE_NAME env var via Micronaut's standard kebab/uppercase convention. Accepted.

Operational caveat — companion infra alarm-pattern alternation NOT yet landed (MUST-LAND-BEFORE-DEPLOY)

I verified the current state of aws-webhook-events-monitoring.tofu:

$ grep "pattern" infrastructure/.../aws-webhook-events-monitoring.tofu
pattern = "\"M3 alarm will surface this\""

The companion follow-up infra MR (referenced in this PR's description as "TBD link") that extends the pattern to ?"M3 alarm will surface this" ?"DEGRADED MODE" has not yet been merged. Concretely:

• In degraded mode, markProcessed is never called → the existing "M3 alarm will surface this" partial-coverage filter never fires.
• The "DEGRADED MODE" ERROR logs DO fire, but the alarm pattern does not match them yet.
• The strict periodic-scan M3 alarm (M-NEW-2) is promoted to a Phase-1 launch gate, not yet shipped.

Net effect during the audit-log-ssm-bridge=false deploy window: zero alarm signal, only ERROR-level CloudWatch log lines. Stripe receives 200 (handler returns normally), so Stripe stops retrying. Data is not lost — the WebhookEvent row stays processed=false and is replayable — but ops has no automated trigger to know it's time to replay.

Recommendation: the companion infra MR (alarm-pattern alternation) MUST land in the same apply window as PR #6. Order:

1. PR #6 merges to main (this APPROVE clears it).
2. Companion infra MR lands and applies the extended alarm pattern.
3. THEN apply the lambda image with audit_log_ssm_bridge_ready=false.
4. Application repo tablesToExport MR lands; flip the var to true; apply; next image roll auto-clears degraded mode.

Reversing 2 and 3 puts the silent-payment-data-loss class one missed CloudWatch ERROR line away from materialising.

Pre-conditions for closing PAY-05

• ✅ This PR (M-NEW-1 patch) approved → mergeable.
• 🔴 Companion infra MR (alarm-pattern alternation) — confirm landed before apply.
• ⚠️ Manual replay tooling for processed=false WebhookEvent rows — defer to OPS lane; not a PR #6 blocker, but should not be omitted from the Phase-1 launch checklist.

Recommended merge order (unchanged from prior review)

1. This PR merges to main (squash, per team-lead's plan).
2. Companion infra MR (alarm-pattern alternation) lands and applies.
3. Apply the lambda image with audit_log_ssm_bridge_ready=false. Verify in CloudWatch that the construction-time "DEGRADED MODE" ERROR line fires and the alarm matches it.
4. Application repo tablesToExport MR lands. Flip var.audit_log_ssm_bridge_ready=true. Apply. Next image roll auto-clears degraded mode.

— Tomás (Security Champion)

[miguelaferreira]

There are a few places where we should make better use of the micronaut framework. I left inline comments on those places.

In addition to that, the store design for the interactions with DDB does not seem to me like a better implementation than the repository design we use in some of the internal functions (eg. the data-handler function in the sobrado project).

[miguelaferreira]

Why isn't this object mapper always injected via application context?

Having it injected allows us to configure it via the framework interface, and make sure that the same singleton is used everywhere.

Also, this should be the object mapper from micronaut serialization, not jackson.

[ff-team-sobrado]

Done in 7432ab2: AuditLogStore constructor now takes io.micronaut.serde.ObjectMapper (Micronaut Serde) — no more new ObjectMapper() on the AuditLog write path. Entry.details is now Map<String, Object> (built as LinkedHashMap for insertion-order stability). Both handlers lost their objectMapper field entirely; AuditLogStore is the single serialisation point. Test seam dual-constructor dropped. Note: WebhookEventProcessor.OBJECT_MAPPER is still Jackson because StripeWebhookEvent.dataObject is a Jackson JsonNode that all handlers walk via tree API — migrating that ripples through every handler + test, kept out of this commit; happy to do it as a separate commit/PR if you want.

[ff-team-sobrado]

Filed as ARCH-08: Notion ticket.

Scope of the follow-up: replace WebhookEventProcessor.readTree(rawBody) with typed-record deserialization + sealed event hierarchy, switch StripeWebhookEvent.dataObject from Jackson JsonNode to typed payload records (internal DTOs, not Stripe SDK direct re-export), delete every direct com.fasterxml.jackson import from stripe-webhook production sources, ship an ADR establishing Micronaut Serde as canonical, and add a CI static check to prevent reintroduction. Sibling to ARCH-09 (@Value audit) but ARCH-08 lands first since the parsing/model rewrite is the larger structural change.

Sequenced after PR #6 merges + SEC-PAY-11 (pii-crypto-layer v0.2.0) ships, so the migration inherits a clean consumer baseline.

Thread closed on this PR — no further code changes here.

— Rui

[miguelaferreira]

These values could better be defined in a new configuration properties implementation.

[ff-team-sobrado]

Done in e6e27f7: new InvestmentPaymentProperties (@ConfigurationProperties("investment-payment")) holds both values. tableName has no default (fail-closed); byReservationIndexName defaults to "investmentPaymentByReservation" — centralised on the bean rather than embedded in an @Value default. Constructor now (DynamoDbClient, InvestmentPaymentProperties). Env-var binding unchanged (INVESTMENT_PAYMENT_TABLE_NAME and INVESTMENT_PAYMENT_BY_RESERVATION_INDEX_NAME resolve via Micronaut's standard kebab-to-uppercase). Same pattern can extend to InvestmentReservationStore, UserInvestmentStore, and AuditLogStore — happy to do that wholesale in a follow-up if you want consistency across the package.

[ff-team-sobrado]

Re-pushed addressing review feedback (HEAD 7432ab2)

Two new commits on feat/pay-05-phase-2b-handler-bodies, addressing both inline comments. Replied inline on each thread; summary here:

AuditLogStore.java:50 — Micronaut Serde + DI ✅

refactor(pay-05): inject Micronaut Serde ObjectMapper for AuditLog details (7432ab2)

• Switched the constructor parameter type to io.micronaut.serde.ObjectMapper — Micronaut Serde, not Jackson.
• Dropped the new ObjectMapper() per-Lambda-init pattern; the framework-managed singleton is now used everywhere on the AuditLog write path.
• Refactored AuditLogStore.Entry.details from Jackson ObjectNode to Map<String, Object> — handlers no longer need a Jackson mapper threaded through to build details. Both PaymentIntentSucceededHandler and PaymentIntentFailedHandler shrunk by one constructor parameter and lost their objectMapper field entirely. AuditLogStore now owns the single serialisation point.
• Test-seam dual-constructor pattern dropped from all three classes.

WebhookEventProcessor still uses Jackson ObjectMapper for readTree(rawBody) because StripeWebhookEvent.dataObject is a Jackson JsonNode that all handlers walk. Switching that ripples through every handler + test — out of scope here, happy to follow up if you want it migrated too. Open to a separate commit / PR for the parse-side.

InvestmentPaymentStore.java:72 — @ConfigurationProperties ✅

refactor(pay-05): bind InvestmentPaymentStore via @ConfigurationProperties (e6e27f7)

• New InvestmentPaymentProperties (@ConfigurationProperties("investment-payment")) holding tableName (no default, fail-closed) and byReservationIndexName (default "investmentPaymentByReservation" — centralised on the bean rather than embedded in an @Value default).
• InvestmentPaymentStore constructor reduced to (DynamoDbClient, InvestmentPaymentProperties).
• Env-var binding unchanged — INVESTMENT_PAYMENT_TABLE_NAME and INVESTMENT_PAYMENT_BY_RESERVATION_INDEX_NAME still resolve via Micronaut's standard kebab-to-uppercase mapping.

Same pattern can extend to InvestmentReservationStore, UserInvestmentStore, and AuditLogStore (which still uses an inline @Value for its single audit-log.table-name) — happy to do that wholesale in a follow-up if you want consistency across the package.

Body comment — store vs. repository design

You wrote:

"the store design for the interactions with DDB does not seem to me like a better implementation than the repository design we use in some of the internal functions (eg. the data-handler function in the sobrado project)."

Brief rationale below — flagging where the patterns are aligned and where they aren't, then proposing a concrete next step.

The store pattern is not the same surface as the data-handler's AbstractRepository + Enhanced Client. The two patterns split deliberately on three dimensions:

1. Atomicity surface. The PAY-05 handlers build a single TransactWriteItems that spans 3-4 different tables with per-write ConditionExpressions (e.g. status = :expected). The Enhanced Client's transactWriteItems does support this in v2.x via WriteBatch, but the conditional-expression composition for the cross-table case is awkward enough that we fall back to the low-level DynamoDbClient shape. data-handler doesn't do TWI — its repositories do isolated GetItem / Query, so the high-level Enhanced Client is the right tool there.
2. Tomás's projection-whitelist veto. Each store carries an explicit PROJECTION_EXPRESSION constant (e.g. InvestmentPaymentStore.PROJECTION_EXPRESSION = "id, version, userId, amountCents, currency, stripePaymentIntentId, #s") and a corresponding tight *View record. This is part of the PAY-23-style PII-discipline review — the projection list is a code-review surface that's easier to keep tight on the low-level path. payment-lambda shares this convention (see backend/payment-lambda/.../InvestmentStore.java:39's "Tomás's veto list … item Add tracing spans #4: project ONLY priceCents").
3. Schema flexibility under Amplify churn. Amplify Gen-2 generates DDB schemas with implicit fields (__typename, _lastChangedAt) that Enhanced Client annotated POJOs need to keep in sync with. The low-level shape lets us write only the fields we care about and ignore the rest — Amplify schema updates don't force store-class regeneration.

So today: data-handler (stream-event-driven, isolated read-modify-write per record, Enhanced Client repositories) and payment-lambda + stripe-webhook (synchronous request-driven, atomic cross-table TWIs with conditional expressions, low-level stores) are intentionally on different patterns.

That said — your feedback isn't wrong. There IS overlap and the codebase would benefit from a more unified "this is how we talk to DDB" interface. Concrete next step I'd propose: open a PAY-XX-store-consolidation ticket once PAY-05 ships, scoped to:

• evaluate whether Enhanced Client + WriteBatch can handle the per-write conditional-expression case cleanly (I think the gap has narrowed in SDK v2.x);
• if yes, port payment-lambda first (it's the simplest store surface), then stripe-webhook;
• if not, document the deliberate split (which patterns belong to which Lambda style) so new contributors aren't left guessing.

I'd rather not do that exploration as part of PR #6 since it'd substantially expand scope on a hot-path Phase-2b deploy. OK if I file the ticket and we come back to it once PAY-05 is in prd? If you'd rather see it done in PR #6, I can do that — but flagging the timeline trade-off honestly.

Tests are green on the new HEAD (78/78). Ready for re-review.

— Rui

[ff-team-sobrado]

Miguel: accepts the 3-dimension argument; use Store going forward where appropriate.

Acknowledged — closing this thread. No PAY-XX-store-consolidation follow-up ticket; the project standard going forward is "Store pattern wherever it makes more sense than Repository," documented by precedent.

— Rui