Wolverine 5→6 upgrade + transactional-outbox atomicity fix#154
Merged
Conversation
… outbox enlistment) Major-version bump of all six WolverineFx.* packages. Build is source-compatible; three runtime breaking changes surfaced and were fixed (all caught by the integration suite): 1. Runtime code generator split out of core (GH-2876) → reference WolverineFx.RuntimeCompilation from ServiceDefaults (flows to every service). Restores TypeLoadMode.Dynamic behavior. 2. ServiceLocationPolicy default flipped to NotAllowed (rejects codegen container-fallback for non-inlinable deps like IProductCache/HybridCache) → AllowHandlerServiceLocation() shared extension, called in all five services. Not the service-locator anti-pattern; a codegen knob. 3. Constructor-injected IMessageBus (the IEventPublisher shim) is no longer enlisted in the handler's outbox transaction under 6.x — a publish through it fires immediately, before commit. This broke PaymentService's Acceptor→Gateway split: the local PaymentProcessingRequested continuation reached the Gateway handler before Payment(Pending) committed, so the Gateway found no row and the payment stuck in Pending. Fix: publish the continuation via a method-injected IMessageContext (enlisted) in ProcessPaymentHandler. Encoding: new always-on rule in CLAUDE.md; full upgrade notes in docs/project-decisions.md; corrected the now-false "stages in the same transaction" claim in all three WolverineEventPublisher XML docs. STATUS.md updated. Open follow-up (tracked in Issues): verify outbox atomicity of OrderService/ShippingService external publishes (same constructor-IMessageBus non-enlistment) and apply IMessageContext if violated. Tests: build clean (0 warnings, TreatWarningsAsErrors on); 116 unit/arch + 38/38 integration green. /check-rules audit: CLAUDE.md changed (added the IMessageContext outbox-enlistment rule); the paired inline comments (ProcessPayment.cs, the 3 WolverineEventPublisher docs) end with the canonical pointer and the detail lives in docs/project-decisions.md per the lean-CLAUDE.md rule. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Warning Review limit reached
More reviews will be available in 39 minutes and 49 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (7)
WalkthroughUpgrades WolverineFx from 5.39.3 to 6.8.0 across all services, addressing three runtime-breaking changes: adds ChangesWolverineFx 5→6 Upgrade and Multi-Service Outbox Refactoring
Codacy CI Tooling Setup
Sequence DiagramsequenceDiagram
participant Handler as OrderService/PaymentService<br/>PlaceOrderHandler
participant MessageContext as IMessageContext<br/>(method param)
participant DbContext as EF Core DbContext
participant Outbox as wolverine_outgoing<br/>_envelopes
participant Dispatcher as Event Dispatcher<br/>(post-commit)
Handler->>MessageContext: PublishAsync(Event)
MessageContext->>Outbox: Stage envelope in transaction
Handler->>DbContext: SaveChangesAsync()
DbContext->>Outbox: Commit staged envelope atomically
DbContext-->>Handler: Transaction complete
Dispatcher->>Outbox: Poll and dispatch
Dispatcher->>Dispatcher: Send Event downstream
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related issues
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
Actionable comments posted: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
PaymentService/Features/ProcessPayment.cs (1)
234-271:⚠️ Potential issue | 🟠 Major | ⚡ Quick winPublish the terminal events through the enlisted context too.
PaymentCompletedEvent/PaymentFailedEventstill go out via the constructor-injected publisher beforeSaveChangesAsync, but this PR documents that constructor-injected Wolverine publishes are no longer transaction-enlisted in 6.x. IfSaveChangesAsyncfails, consumers can observe a terminal payment event for a row that never committed.♻️ Suggested fix
public class PaymentProcessingRequestedHandler( PaymentDbContext context, IPaymentGateway gateway, - IEventPublisher eventPublisher) + ) { - public async Task HandleAsync(PaymentProcessingRequested message, CancellationToken cancellationToken) + public async Task HandleAsync(PaymentProcessingRequested message, IMessageContext messageContext, CancellationToken cancellationToken) { @@ - await eventPublisher.PublishAsync(new PaymentCompletedEvent + await messageContext.PublishAsync(new PaymentCompletedEvent { @@ - }, cancellationToken); + }); @@ - await eventPublisher.PublishAsync(new PaymentFailedEvent + await messageContext.PublishAsync(new PaymentFailedEvent { @@ - }, cancellationToken); + });🤖 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 `@PaymentService/Features/ProcessPayment.cs` around lines 234 - 271, The terminal payment events (PaymentCompletedEvent in the success branch and PaymentFailedEvent in the failure branch) are being published through the constructor-injected eventPublisher before SaveChangesAsync, but in Wolverine 6.x this publisher is not transaction-enlisted, creating a risk where consumers observe these events even if SaveChangesAsync later fails and the database changes don't commit. Replace the eventPublisher.PublishAsync calls in both the success and failure branches with a transaction-enlisted publish mechanism (likely through the context object) to ensure atomicity with the database changes, so the events are only visible to consumers if and when SaveChangesAsync succeeds.
🤖 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 @.codacy/cli.sh:
- Line 40: The local variable declaration combined with command substitution
assignment masks the exit code of the pipeline containing grep and cut. Separate
the local declaration from the assignment of the version variable so that the
return value of the command substitution is properly exposed and can be checked
for errors. This allows detection if grep or cut fails during extraction of the
version string from the version_file.
- Line 58: The local variable declaration and assignment on line 58 using the
pattern `local version=$(...)` masks the exit status of the command
substitution, preventing detection of failures from the echo, grep, or cut
commands. Split the declaration and assignment into two separate statements:
first declare `local version` on its own line, then assign the result of the
command substitution `version=$(echo "$response" | grep -m 1 tag_name | cut
-d'"' -f4)` on the next line so that the exit code from the pipeline is properly
propagated and can be checked.
- Around line 26-28: The conditional checks on lines 26 and 28 are calling
`uname` multiple times when the `os_name` variable was already set on line 10.
Replace the `$(uname)` calls in the conditional expressions with references to
the cached `$os_name` variable to eliminate the redundant system calls and
improve efficiency.
- Around line 49-60: The get_latest_version() function lacks error handling
after the curl calls, meaning network failures, HTTP errors, or invalid
responses will silently result in an empty version string. Add explicit
validation immediately after the curl assignments to check that the response is
not empty and contains valid JSON. If the response is invalid or empty, either
return an error code or log a warning before attempting to extract the version
with the grep command on line 58. This ensures that failures in fetching from
the GitHub API are explicitly detected rather than producing empty version
strings that propagate silently.
- Line 148: Replace the eval command in the cli.sh script that currently uses
`eval "$run_command $*"`. First, change `$*` to `"$@"` to preserve individual
argument boundaries. If `$run_command` contains only a binary path or command
name without dynamic code, remove the eval wrapper entirely and execute the
command directly as `$run_command "$@"`. If eval is truly necessary for some
reason, at minimum replace `$*` with `"$@"` to ensure arguments are passed as
separate tokens rather than concatenated strings, which prevents word-splitting
issues and improves security.
- Line 65: The `fatal` function is called at three locations in .codacy/cli.sh
(lines 65, 78, and 142) but is never defined, causing runtime crashes when those
error conditions are triggered. Define the `fatal` function once near the top of
the script after line 7. This single definition will resolve all three call
sites. The function should output the error message and cleanly exit the script,
ensuring the rate limit error at line 65, the missing curl/wget error at line
78, and the empty binary path error at line 142 all report clean errors instead
of crashing with "command not found".
- Line 4: In `.codacy/cli.sh` at line 4, change `set -e +o pipefail` to `set -e
-o pipefail` (replace the `+o` with `-o`). The `+o pipefail` syntax disables
pipeline failure detection, allowing malformed API responses to be silently
processed. After making this change, add explicit validation checks in the
`get_latest_version()` function (around line 58) to verify that the piped
command result is not empty before using the extracted version value, ensuring
that empty or malformed API responses are properly caught and handled.
- Around line 97-105: Add SHA256 checksum validation to protect against
supply-chain attacks when downloading the Codacy CLI binary. After downloading
the tarball using the download function and before extracting it with the tar
command, download the checksums.txt file from the same GitHub release URL (using
the pattern codacy-cli-v2_${version}_checksums.txt), then validate the
downloaded tarball using sha256sum with the --ignore-missing flag before
proceeding with the tar extraction. This ensures the binary integrity is
verified prior to extraction.
In @.codacy/codacy.yaml:
- Around line 1-15: The runtimes and tools sections in the Codacy configuration
file are pinned to older versions (such as go@1.24.13, dart@3.7.2, java@17.0.10,
node@22.2.0, python@3.11.11, and various analysis tools like eslint, pylint,
trivy, etc.). Review Codacy's supported runtime matrix documentation to identify
the current stable releases for each runtime and tool, then update all version
pins in both the runtimes and tools sections to their latest stable versions to
improve long-term security and feature support.
In `@CLAUDE.md`:
- Around line 240-241: The CLAUDE.md file exceeds its 500-line hard limit due to
an overly detailed Wolverine upgrade explanation added at lines 240-241. Replace
this verbose explanation with a concise one-liner rule stating that handlers
must use method-injected IMessageContext (not constructor-injected
IEventPublisher) for transactional message publishing, and move the full
detailed explanation including the PaymentService example, the rationale about
transaction enlistment, and references to the upgrade notes to
docs/project-decisions.md under the "Wolverine 5→6 upgrade" section where
detailed context belongs.
---
Outside diff comments:
In `@PaymentService/Features/ProcessPayment.cs`:
- Around line 234-271: The terminal payment events (PaymentCompletedEvent in the
success branch and PaymentFailedEvent in the failure branch) are being published
through the constructor-injected eventPublisher before SaveChangesAsync, but in
Wolverine 6.x this publisher is not transaction-enlisted, creating a risk where
consumers observe these events even if SaveChangesAsync later fails and the
database changes don't commit. Replace the eventPublisher.PublishAsync calls in
both the success and failure branches with a transaction-enlisted publish
mechanism (likely through the context object) to ensure atomicity with the
database changes, so the events are only visible to consumers if and when
SaveChangesAsync succeeds.
🪄 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: 5cd3d326-af3f-474b-9c5f-d8e9200409b2
⛔ Files ignored due to path filters (1)
.claude/scheduled_tasks.lockis excluded by!**/*.lock
📒 Files selected for processing (17)
.codacy/cli.sh.codacy/codacy.yamlCLAUDE.mdCatalogService/Program.csDirectory.Packages.propsNextAurora.ServiceDefaults/Extensions.csNextAurora.ServiceDefaults/NextAurora.ServiceDefaults.csprojNotificationService/Program.csOrderService/Infrastructure/WolverineEventPublisher.csOrderService/Program.csPaymentService/Features/ProcessPayment.csPaymentService/Infrastructure/WolverineEventPublisher.csPaymentService/Program.csShippingService/Infrastructure/WolverineEventPublisher.csShippingService/Program.csdocs/STATUS.mddocs/project-decisions.md
| #!/usr/bin/env bash | ||
|
|
||
|
|
||
| set -e +o pipefail |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify if there are pipelines in this script that might fail benignly
rg -n '\|' .codacy/cli.shRepository: emeraldleaf/NextAurora
Length of output: 446
🏁 Script executed:
cat -n .codacy/cli.shRepository: emeraldleaf/NextAurora
Length of output: 4917
Replace +o pipefail with -o pipefail to catch pipeline failures.
The +o pipefail syntax disables pipefail, masking failures in command pipelines. This creates a real issue: at line 58, get_latest_version() uses echo "$response" | grep -m 1 tag_name | cut -d'"' -f4 without validating the result. If the API response is empty or malformed, grep fails but the pipeline returns 0 (from cut), resulting in an empty version being written to version.yaml.
Change line 4 to set -e -o pipefail to properly catch pipeline errors, then add explicit validation checks in get_latest_version() for empty results before using the version.
🤖 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 @.codacy/cli.sh at line 4, In `.codacy/cli.sh` at line 4, change `set -e +o
pipefail` to `set -e -o pipefail` (replace the `+o` with `-o`). The `+o
pipefail` syntax disables pipeline failure detection, allowing malformed API
responses to be silently processed. After making this change, add explicit
validation checks in the `get_latest_version()` function (around line 58) to
verify that the piped command result is not empty before using the extracted
version value, ensuring that empty or malformed API responses are properly
caught and handled.
Comment on lines
+26
to
+28
| if [ "$(uname)" = "Linux" ]; then | ||
| CODACY_CLI_V2_TMP_FOLDER="$HOME/.cache/codacy/codacy-cli-v2" | ||
| elif [ "$(uname)" = "Darwin" ]; then |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | 💤 Low value
Use the cached os_name variable.
Lines 26 and 28 call uname again, but os_name was already set on Line 10. This is a minor inefficiency.
♻️ Proposed refactor
- if [ "$(uname)" = "Linux" ]; then
+ if [ "$os_name" = "Linux" ]; then
CODACY_CLI_V2_TMP_FOLDER="$HOME/.cache/codacy/codacy-cli-v2"
- elif [ "$(uname)" = "Darwin" ]; then
+ elif [ "$os_name" = "Darwin" ]; then
CODACY_CLI_V2_TMP_FOLDER="$HOME/Library/Caches/Codacy/codacy-cli-v2"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if [ "$(uname)" = "Linux" ]; then | |
| CODACY_CLI_V2_TMP_FOLDER="$HOME/.cache/codacy/codacy-cli-v2" | |
| elif [ "$(uname)" = "Darwin" ]; then | |
| if [ "$os_name" = "Linux" ]; then | |
| CODACY_CLI_V2_TMP_FOLDER="$HOME/.cache/codacy/codacy-cli-v2" | |
| elif [ "$os_name" = "Darwin" ]; then |
🤖 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 @.codacy/cli.sh around lines 26 - 28, The conditional checks on lines 26 and
28 are calling `uname` multiple times when the `os_name` variable was already
set on line 10. Replace the `$(uname)` calls in the conditional expressions with
references to the cached `$os_name` variable to eliminate the redundant system
calls and improve efficiency.
Comment on lines
+49
to
+60
| get_latest_version() { | ||
| local response | ||
| if [ -n "$GH_TOKEN" ]; then | ||
| response=$(curl -Lq --header "Authorization: Bearer $GH_TOKEN" "https://api.github.com/repos/codacy/codacy-cli-v2/releases/latest" 2>/dev/null) | ||
| else | ||
| response=$(curl -Lq "https://api.github.com/repos/codacy/codacy-cli-v2/releases/latest" 2>/dev/null) | ||
| fi | ||
|
|
||
| handle_rate_limit "$response" | ||
| local version=$(echo "$response" | grep -m 1 tag_name | cut -d'"' -f4) | ||
| echo "$version" | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | ⚡ Quick win
Add error handling for GitHub API failures.
The function only checks for rate limiting but doesn't handle network errors, 404 responses, or invalid JSON. If curl fails, $response will be empty, and Line 58 will produce an empty version string.
Consider adding explicit error checks after the curl call.
🛡️ Suggested improvement
if [ -n "$GH_TOKEN" ]; then
- response=$(curl -Lq --header "Authorization: Bearer $GH_TOKEN" "https://api.github.com/repos/codacy/codacy-cli-v2/releases/latest" 2>/dev/null)
+ response=$(curl -Lq --fail --header "Authorization: Bearer $GH_TOKEN" "https://api.github.com/repos/codacy/codacy-cli-v2/releases/latest" 2>/dev/null) || {
+ echo "Error: Failed to fetch latest version from GitHub API" >&2
+ return 1
+ }
else
- response=$(curl -Lq "https://api.github.com/repos/codacy/codacy-cli-v2/releases/latest" 2>/dev/null)
+ response=$(curl -Lq --fail "https://api.github.com/repos/codacy/codacy-cli-v2/releases/latest" 2>/dev/null) || {
+ echo "Error: Failed to fetch latest version from GitHub API" >&2
+ return 1
+ }
fi📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| get_latest_version() { | |
| local response | |
| if [ -n "$GH_TOKEN" ]; then | |
| response=$(curl -Lq --header "Authorization: Bearer $GH_TOKEN" "https://api.github.com/repos/codacy/codacy-cli-v2/releases/latest" 2>/dev/null) | |
| else | |
| response=$(curl -Lq "https://api.github.com/repos/codacy/codacy-cli-v2/releases/latest" 2>/dev/null) | |
| fi | |
| handle_rate_limit "$response" | |
| local version=$(echo "$response" | grep -m 1 tag_name | cut -d'"' -f4) | |
| echo "$version" | |
| } | |
| get_latest_version() { | |
| local response | |
| if [ -n "$GH_TOKEN" ]; then | |
| response=$(curl -Lq --fail --header "Authorization: Bearer $GH_TOKEN" "https://api.github.com/repos/codacy/codacy-cli-v2/releases/latest" 2>/dev/null) || { | |
| echo "Error: Failed to fetch latest version from GitHub API" >&2 | |
| return 1 | |
| } | |
| else | |
| response=$(curl -Lq --fail "https://api.github.com/repos/codacy/codacy-cli-v2/releases/latest" 2>/dev/null) || { | |
| echo "Error: Failed to fetch latest version from GitHub API" >&2 | |
| return 1 | |
| } | |
| fi | |
| handle_rate_limit "$response" | |
| local version=$(echo "$response" | grep -m 1 tag_name | cut -d'"' -f4) | |
| echo "$version" | |
| } |
🧰 Tools
🪛 Shellcheck (0.11.0)
[warning] 58-58: Declare and assign separately to avoid masking return values.
(SC2155)
🤖 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 @.codacy/cli.sh around lines 49 - 60, The get_latest_version() function lacks
error handling after the curl calls, meaning network failures, HTTP errors, or
invalid responses will silently result in an empty version string. Add explicit
validation immediately after the curl assignments to check that the response is
not empty and contains valid JSON. If the response is invalid or empty, either
return an error code or log a warning before attempting to extract the version
with the grep command on line 58. This ensures that failures in fetching from
the GitHub API are explicitly detected rather than producing empty version
strings that propagate silently.
3 tasks
…cted IMessageContext Completes the Wolverine 6 outbox-atomicity fix across all services. The constructor-injected IEventPublisher (over IMessageBus) is NOT enlisted in the handler's outbox transaction under 6.x — a publish through it fires inline, before SaveChanges commits, silently defeating the transactional outbox. Proven with a rollback test (force the commit to fail; assert no event dispatched): OrderService's external OrderPlacedEvent survived a rolled-back commit — non-atomic. Fix: publish through the IMessageContext Wolverine injects as a HandleAsync method parameter (the only enlisted context). Applied to every write-then-publish handler: - OrderService PlaceOrderHandler → OrderPlacedEvent - ShippingService CreateShipmentHandler → ShipmentDispatchedEvent - PaymentService Gateway (PaymentProcessingRequestedHandler) → PaymentCompleted/FailedEvent Dead-code removal: the IEventPublisher shim is now unused by OrderService and ShippingService handlers (only consumers) → deleted port + impl + registration in both. Retained in PaymentService for two non-write-then-publish paths: the Acceptor's terminal-event REPUBLISH (already-committed payment, no entity write → inline send is correct) and the PaymentRecoveryJob. Test: new ThrowingSaveChangesInterceptor + OrderSagaTests.PlaceOrder_does_not_dispatch_ OrderPlacedEvent_when_the_commit_rolls_back (RED before fix, GREEN after; first assertion guards that the interceptor actually fired, so no false pass). Docs: full war story in docs/war-story-wolverine6-outbox-atomicity.md; project-decisions.md §13 updated (caveat → proven+fixed). Still open (tracked): PaymentRecoveryJob non-handler enlistment needs IDbContextOutbox + its own rollback test. Tests: build clean (0 warnings); 116 unit/arch + 39 integration green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
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 `@tests/OrderService.Tests.Integration/OrderSagaTests.cs`:
- Around line 121-127: The test currently does not explicitly verify that the
POST request failed as expected before checking DB and event invariants, which
allows false-positive passes if the request fails earlier for a different
reason. Modify the ExecuteAndWaitAsync call to capture the HTTP response from
client.PostAsJsonAsync along with the session, then add an explicit assertion on
the response status code to confirm it contains the expected failure status
(such as an error code indicating SaveChanges failure) before proceeding with
any DB or event invariant checks. This ensures the failure occurred in the
specific SaveChanges path being tested, not in some other path.
🪄 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: 95530f9b-8664-432c-b61b-10fc2b14b753
📒 Files selected for processing (13)
OrderService/Domain/IEventPublisher.csOrderService/Features/PlaceOrder.csOrderService/Infrastructure/DependencyInjection.csOrderService/Infrastructure/WolverineEventPublisher.csPaymentService/Features/ProcessPayment.csShippingService/Domain/IEventPublisher.csShippingService/Features/CreateShipment.csShippingService/Infrastructure/DependencyInjection.csShippingService/Infrastructure/WolverineEventPublisher.csdocs/project-decisions.mddocs/war-story-wolverine6-outbox-atomicity.mdtests/OrderService.Tests.Integration/OrderSagaTests.cstests/OrderService.Tests.Integration/ThrowingSaveChangesInterceptor.cs
💤 Files with no reviewable changes (4)
- ShippingService/Domain/IEventPublisher.cs
- OrderService/Domain/IEventPublisher.cs
- ShippingService/Infrastructure/WolverineEventPublisher.cs
- OrderService/Infrastructure/WolverineEventPublisher.cs
…ses the non-handler case) The recovery sweeper marks timed-out payments Failed and publishes PaymentFailedEvent outside the Wolverine handler pipeline, inside an explicit BeginTransactionAsync. Under 6.x the constructor-injected IEventPublisher (IMessageBus) isn't enlisted in that transaction, so the event published INLINE — a rolled-back recovery commit still dispatched PaymentFailedEvent (a "payment failed" event for a payment whose Failed state never persisted). Same root cause as the handlers, different fix (no method-injected context available in a BackgroundService). Fix: Wolverine's non-handler outbox API — enroll the DbContext in an IDbContextOutbox, publish through it, then SaveChangesAndFlushMessagesAsync() (stages envelope + saves entity + commits, atomically). Replaces the manual BeginTransaction → Publish → SaveChanges → Commit. IDbContextOutbox resolves from the existing AddDbContext + UseEntityFrameworkCoreTransactions() registration — no wiring change. Proven by the "outbox-in-non-handler" rollback test CLAUDE.md requires (and which didn't exist): PaymentRecoveryAtomicityTests.RecoverySweep_does_not_dispatch_PaymentFailedEvent_when_the_commit_rolls_back + ThrowOnPaymentUpdateInterceptor — RED before, GREEN after (first assertion guards the interceptor fired). InternalsVisibleTo added for the integration project to drive internal SweepAsync. IEventPublisher is retained in PaymentService only for the Acceptor's terminal-event republish (an already-committed payment, no entity write → inline send is correct). Docs: project-decisions.md §13 + war-story updated (RecoveryJob caveat → fixed). Closes #153. Tests: build clean (0 warnings); 116 unit/arch + 40 integration green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…eRabbit
CI build was failing on the broken-link audit: docs/code-flows/{order,shipping}service.md still
linked to the OrderService/ShippingService IEventPublisher + WolverineEventPublisher files this PR
deleted. Updated both code-flow docs — the mermaid publish participant is now IMessageContext
(method-injected), and the dead-file table rows are replaced with the framework-type note. Fixed the
matching stale (non-link) reference in docs/observability.md.
Untracked .codacy/ (Codacy CLI artifacts accidentally committed via `git add -A`) and added it to
.gitignore — restores its pre-session untracked state; also clears CodeRabbit's noise on cli.sh.
CodeRabbit feedback:
- CLAUDE.md: trimmed the new Wolverine rule to a headline + one-line summary + pointers (the worked
example/rationale already live in project-decisions.md + the war story). CodeRabbit's "over 500
lines" premise was wrong (file is 319), but the lean-CLAUDE.md convention favors the trim anyway.
- OrderSagaTests rollback test: assert the POST status so it can't false-pass if the request fails
before the forced commit. The interceptor now throws DbUpdateException (a real infra commit failure
→ GlobalExceptionHandler 500), distinct from the 409 it gives business conflicts; test asserts 500.
Verified locally: broken-link audit fail=0, CLAUDE.md 319 lines, OrderService integration green.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
emeraldleaf
dismissed
coderabbitai[bot]’s stale review
June 15, 2026 02:03
addressed in later commits
This was referenced Jun 15, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Major-version bump of all six
WolverineFx.*packages → 6.8.0, plus the fix for a production correctness regression the upgrade introduced. Build is source-compatible; the runtime breaking changes are caught by the integration suite.The three breaking changes
WolverineFx.RuntimeCompilationfrom ServiceDefaults.ServiceLocationPolicydefault flipped toNotAllowed→AllowHandlerServiceLocation()shared extension in all 5 services. (A codegen knob, not the service-locator anti-pattern.)IMessageBusis no longer enlisted in the handler's outbox transaction. This is the serious one — it silently broke the transactional outbox.The outbox regression (the heart of this PR)
Handlers published events via
IEventPublisher(a shim over constructor-injectedIMessageBus). In 5.x that publish was enlisted in the handler transaction (staged → atomic). In 6.x it fires inline, beforeSaveChangescommits. Proven with a rollback test: force the commit to fail after publishing, andOrderPlacedEventwas still dispatched despite the order rolling back.Production impact (transport-agnostic — real ASB, emulator, RabbitMQ alike):
PlaceOrderpublishes → event hits the broker →SaveChangesfails → the order never persists, but PaymentService already consumed the event and charges for an order that doesn't exist. Intermittent, invisible to happy-path tests, money-touching. This is exactly what the outbox exists to prevent — so the upgrade must not merge without this fix.Fix: publish through the
IMessageContextWolverine injects as aHandleAsyncmethod parameter (the only enlisted context), in every write-then-publish handler —PlaceOrder,CreateShipment, PaymentService Gateway, and PaymentService's local Acceptor→Gateway continuation. One parameter each; the four heavier hypotheses we tried first (UseDurableLocalQueues, constructor-IMessageContext, both) were all wrong and reverted.Dead-code removal: the
IEventPublishershim, now unused by OrderService/ShippingService handlers, was deleted (port + impl + registration). Retained only in PaymentService for two non-write-then-publish paths (Acceptor terminal-event republish;PaymentRecoveryJob).New test
OrderSagaTests.PlaceOrder_does_not_dispatch_OrderPlacedEvent_when_the_commit_rolls_back+ThrowingSaveChangesInterceptor— RED before the fix, GREEN after. The first assertion (no order persisted) guards that the interceptor actually fired, so the atomicity assertion can't give a false pass.Encoding
IMessageContext, notIEventPublisher.docs/project-decisions.md§13: full upgrade notes; the cross-service caveat updated from "unverified" → "proven and fixed".docs/war-story-...md: the narrative./check-rules audit: CLAUDE.md changed; paired inline comments end with the canonical pointer; detail lives in
docs/project-decisions.mdper the lean-CLAUDE.md rule.Outbox atomicity — now complete across handler and non-handler code
PaymentRecoveryJob(the non-handler recovery sweeper) had the same latent non-enlistment. Fixed with Wolverine's non-handler outbox API (IDbContextOutbox→Enroll+SaveChangesAndFlushMessagesAsync), proven by a new "outbox-in-non-handler" rollback test (PaymentRecoveryAtomicityTests). Closes #153.Tests
dotnet build— clean, 0 warnings (TreatWarningsAsErrorson).🤖 Generated with Claude Code
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Documentation
Chores
Tests