[emeraldleaf]

Summary

First application of the long-running-work / 202 Accepted rule landed in #39 (CLAUDE.md "Performance Rules"). The Stripe gateway call is the slow part (sub-second in the happy path, seconds-to-30s on degraded states); doing it inline held an HTTP request open (and a Wolverine handler slot + DbContext + Service Bus message lease on the saga path) for the full duration.

The split

Entry points are unchanged — both still go through ProcessPaymentCommand:

POST /api/v1/payments/process  ─┐
                                ├─► ProcessPaymentCommand
OrderPlacedHandler (saga)      ─┘

ProcessPaymentHandler (Acceptor — fast):

• Idempotency check on OrderId
• Re-publish terminal event for Completed/Failed re-entry (defensive)
• Persist Payment(Pending) + publish PaymentProcessingRequested + SaveChanges (atomic: row + staged envelope in one DB tx)
• Return PaymentId
• No gateway call. Sub-second.

PaymentProcessingRequestedHandler (Gateway — slow, runs on Wolverine worker):

• Re-fetch Payment, status-guard for at-least-once redelivery (no-op if already Completed/Failed)
• Call Stripe
• MarkAsCompleted/MarkAsFailed, publish PaymentCompletedEvent/PaymentFailedEvent
• SaveChanges (atomic: state transition + staged event)

PaymentProcessingRequested is an internal Wolverine message — lives in PaymentService/Features/ (not NextAurora.Contracts) because no other service subscribes to it. No explicit routing config → Wolverine defaults to a local in-memory queue, exactly as intended.

What's preserved

• All existing idempotency semantics (OrderId existence check, terminal-state re-publish, unique-index race recovery)
• Outbox atomicity (publish-before-save on both handlers; AutoApplyTransactions + UseEntityFrameworkCoreTransactions commit entity + envelope in one tx)
• The PaymentRecoveryJob sweeper still handles stuck Pendings (no behavior regression in failure-mode recovery)
• OrderPlacedHandler cascade pattern (still returns ProcessPaymentCommand)
• The HTTP endpoint still returns 202 Accepted — but now the response actually reflects work moved to the bus, not just "the downstream saga happens to be async"

What's NOT fixed (separate STATUS.md item)

Gateway-side idempotency keys. The "Stripe charged but process crashed before MarkAsCompleted → redelivery double-charges" race still exists; fixing it requires plumbing payment.Id.ToString() into the gateway as an Idempotency-Key header. Tracked in STATUS.md as a separate open issue (this refactor doesn't make it worse).

Test plan

• dotnet build clean (1 pre-existing benchmark warning, unrelated)
• All 6 unit test projects pass (112 tests total). PaymentService.Tests.Unit (19) confirms no regressions in validator + OrderPlacedHandler tests.
• Integration tests at the EF + Wolverine outbox level are the right tier for verifying this refactor end-to-end. PaymentService.Tests.Integration doesn't exist yet (paired STATUS.md gap with ShippingService.Tests.Integration). Per CLAUDE.md "Testing," mocking DbContext for unit-level handler tests is explicitly out — Testcontainers is the right axis.
• Manual verification via smoke test once the AppHost is running (Acceptor returns 202 with PaymentId immediately; gateway handler publishes Completed/Failed later)

Merge order

This PR is the first application of #39's rule. #39 should merge first so the rule reference in the new endpoint comment (See CLAUDE.md "Performance Rules → Long-running work...") points at a canonical rule that exists in main.

After both merge, STATUS.md's "Open issues → POST /api/v1/payments/process is a partial Should-consider..." entry (added in #39) should be updated to mark resolved by this PR.

🤖 Generated with Claude Code

Summary by CodeRabbit

• New Features

  • Payment processing requests now return immediately with 202 Accepted and a PaymentId to poll for results.

• Improvements

  • Processing split into a fast accept stage and a background gateway stage for better responsiveness.
  • Improved duplicate payment detection and race recovery to avoid concurrent conflicts.
  • Added detailed failure reason tracking and outcome metrics for processed payments.

• Documentation

  • Updated status notes for the payment endpoint to reflect the new two-stage flow.

[coderabbitai]

Walkthrough

Payment processing refactored from single synchronous handler into two-stage async flow: a fast idempotent Acceptor that persists Pending payments and publishes continuation messages, and a slow Gateway handler that consumes those messages and executes external payment gateway calls. Endpoint documentation clarified to reflect 202 Accepted response and asynchronous processing.

Changes

Two-Stage Payment Processing Architecture

Layer / File(s)	Summary
Endpoint Documentation and Async Contract PaymentService/Endpoints/PaymentEndpoints.cs	Endpoint documentation for POST /api/v1/payments/process updated to clarify that the HTTP call returns 202 Accepted with a PaymentId, triggers asynchronous processing on the message bus, and does not perform synchronous gateway calls on this path.
Acceptor Handler and Idempotent Fast Path PaymentService/Features/ProcessPayment.cs	ProcessPaymentHandler rewritten as idempotent acceptor: checks for existing Payment by OrderId, republishes terminal events if found, otherwise creates new Pending payment, publishes PaymentProcessingRequested continuation message before persisting, and recovers from concurrent insert races by detaching and re-fetching. Architecture documentation describes the split flow and outbox/event publishing atomicity.
Gateway Handler and Slow Path Message Processing PaymentService/Features/ProcessPayment.cs	New PaymentProcessingRequestedHandler consumes PaymentProcessingRequested messages: loads Payment by PaymentId, no-ops if missing or not Pending, calls IPaymentGateway.ProcessPaymentAsync, publishes PaymentCompletedEvent or PaymentFailedEvent before SaveChangesAsync in success and failure branches respectively, and increments PaymentsProcessed counter with outcome label.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'refactor(payments): split ProcessPayment into Acceptor + Gateway' accurately and specifically describes the main architectural change in the pull request.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch refactor/payment-process-async-shape

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@PaymentService/Features/ProcessPayment.cs`:
- Around line 88-90: The existing-payment branch returns immediately after
calling RepublishTerminalEventAsync, which can publish a staged outbox envelope
without flushing it; update the logic in ProcessPayment (the branch that calls
RepublishTerminalEventAsync) to call SaveChangesAsync on the DbContext
immediately after RepublishTerminalEventAsync (or after the underlying
PublishAsync) so the staged envelope is persisted/flushed before returning
existing.Id; apply the same SaveChangesAsync flush in the other similar block
around lines 121-156 where PublishAsync/RepublishTerminalEventAsync is used.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 670eb3cf-a80c-4e20-88d4-aa5a7b67632d

📥 Commits

Reviewing files that changed from the base of the PR and between 961a684 and c7d293b.

📒 Files selected for processing (2)

• PaymentService/Endpoints/PaymentEndpoints.cs
• PaymentService/Features/ProcessPayment.cs

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

docs/STATUS.md (1)

5-5: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Update the "Last updated:" date to today (2026-05-27).

The STATUS.md file is being modified in this PR, but the "Last updated:" date still shows 2026-05-26. As per coding guidelines, the date should be updated to today (2026-05-27) when this file changes.

📅 Proposed fix

-**Last updated:** 2026-05-26 (CatalogService Clean Architecture → VSA collapse; carve-out retired — all five services now share one shape)
+**Last updated:** 2026-05-27 (PaymentService process endpoint refactored to Acceptor + Gateway pattern)

Note: You may also want to adjust the parenthetical description to reflect today's primary change (the payment processing refactor) rather than the previous session's work.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@docs/STATUS.md` at line 5, Update the "Last updated:" line in STATUS.md by
changing the date in the existing line "**Last updated:** 2026-05-26
(CatalogService Clean Architecture → VSA collapse; carve-out retired — all five
services now share one shape)" to "**Last updated:** 2026-05-27" and, if
appropriate, revise the parenthetical note to mention today's primary change
(payment processing refactor) instead of the previous description; modify the
same line text shown in the diff to reflect these edits.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Outside diff comments:
In `@docs/STATUS.md`:
- Line 5: Update the "Last updated:" line in STATUS.md by changing the date in
the existing line "**Last updated:** 2026-05-26 (CatalogService Clean
Architecture → VSA collapse; carve-out retired — all five services now share one
shape)" to "**Last updated:** 2026-05-27" and, if appropriate, revise the
parenthetical note to mention today's primary change (payment processing
refactor) instead of the previous description; modify the same line text shown
in the diff to reflect these edits.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 609021c2-d5f8-4594-a05b-0037926c6dfa

📥 Commits

Reviewing files that changed from the base of the PR and between c7d293b and f32f15f.

📒 Files selected for processing (1)

• docs/STATUS.md

[codecov]

Codecov Report

❌ Patch coverage is 0% with 44 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
PaymentService/Features/ProcessPayment.cs	0.00%	44 Missing ⚠️

📢 Thoughts on this report? Let us know!