fix(messaging): correct ASB subscription-listener API + emulator/AutoProvision docs (#148)#158
Closed
emeraldleaf wants to merge 2 commits into
Closed
fix(messaging): correct ASB subscription-listener API + emulator/AutoProvision docs (#148)#158emeraldleaf wants to merge 2 commits into
emeraldleaf wants to merge 2 commits into
Conversation
Official Wolverine emulator recipe confirms feat-branch Approach B and contradicts the 'emulator has no management API' rule in CLAUDE.md:230 + architecture.md:399. Tracked on #148.
…Provision docs (#148) The local saga never advanced past Placed because subscription listeners were registered with a combined "{topic}/{sub}" string instead of ListenToAzureServiceBusSubscription("{sub}", c => c.TopicName = "{topic}"), which silently mis-registers the listener so it never attaches — and fails against real Azure too, not just the emulator. Fixed across Order/Payment/Shipping/Notification. Keep Wolverine AutoProvision=false locally. Decompiling Wolverine 6.8.0 shows AzureServiceBusSubscription.InitializeAsync calls the management-API path (SetupAsync -> SubscriptionExistsAsync) ONLY when AutoProvision is true; with it false the listener binds over AMQP with no management call, so the emulator's subscription-admin HTTP 500 is never reached. An earlier "Approach B" attempt on this branch re-enabled AutoProvision + injected a management connection string — reverted here, because it reintroduced the 500. Correct the docs that claimed the emulator has no management API: it does (on the emulatorhealth endpoint, port 5300), but its subscription admin endpoints return 500 while topics/queues return 200. Updated CLAUDE.md, docs/architecture.md, docs/how-it-works.md, docs/full-saga-deployment-plan.md. Live end-to-end verification is currently blocked by an Aspire DCP container-creation timeout on the external-drive-backed Docker (unrelated to this change); the fix rests on the decompiled control-flow proof plus the stubbed-transport integration tests. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
WalkthroughCorrects ChangesAzure Service Bus Subscription Wiring Fix
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@CLAUDE.md`:
- Around line 227-231: CLAUDE.md exceeds the 500-line hard cap which is blocking
the CI build. Refactor by moving the four detailed bullet points about Azure
Aspire configuration (Service Bus subscription naming conventions, Aspire 13+
resource fallbacks, WithReference behavior with WaitFor requirements, and
Wolverine AutoProvision incompatibility with the emulator) out of CLAUDE.md and
into a paired documentation file in the docs/ folder (e.g., docs/aspire-setup.md
or similar). Replace the detailed explanations in CLAUDE.md with concise rule
headlines followed by links to the new paired doc, similar to how the existing
content references docs/architecture.md. This keeps CLAUDE.md lean and scannable
while preserving the detailed rationale in dedicated skill documentation.
In `@NextAurora.AppHost/AppHost.cs`:
- Around line 120-133: The code unconditionally disables AutoProvision across
all environments by setting `Wolverine__AutoProvision=false` at four locations,
contradicting the documented intent that AutoProvision should remain enabled in
publish mode. Modify the four environment variable assignments where
`disableAutoProvision` is used (in the Wolverine service configurations) to only
apply the false value when building for dev/emulator mode. For publish mode
builds, either omit the setting to allow the Wolverine default of true, or
explicitly set `Wolverine__AutoProvision=true` to ensure AutoProvision is
enabled in production against real Azure Service Bus.
🪄 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: 5831fe78-bb5f-45d9-8964-a778d11fa9eb
📒 Files selected for processing (10)
.claude/audits/INDEX.mdCLAUDE.mdNextAurora.AppHost/AppHost.csNotificationService/Program.csOrderService/Program.csPaymentService/Program.csShippingService/Program.csdocs/architecture.mddocs/full-saga-deployment-plan.mddocs/how-it-works.md
Comment on lines
+227
to
231
| - **Service Bus subscription names are globally unique within the namespace** (Aspire 13+). Don't reuse the same subscription name on different topics — `DistributedApplicationException` at AppHost startup. Convention: `{consumer}-{source-events}-sub` (e.g. `notify-orders-sub`, `notify-payments-sub`). When adding a new subscription in `AppHost.cs`, also update the matching `ListenToAzureServiceBusSubscription("{sub}", c => c.TopicName = "{topic}")` call in the consuming service's `Program.cs` — subscription name and topic are SEPARATE arguments, NOT a combined `"{topic}/{sub}"` string (that form silently mis-registers the listener so it never attaches, and fails against real Azure too, not just the emulator). | ||
| - **Aspire 13+ Azure resources need explicit local-dev fallbacks.** `AddAzureServiceBus` requires a chained `.RunAsEmulator()` for local runs (the implicit emulator behavior from Aspire 9 is gone). `AddAzureApplicationInsights` has no local emulator at all — gate it on `builder.ExecutionContext.IsPublishMode` and skip in dev. Without these, AppHost's resource pane shows "Missing subscription configuration" and every service that `WithReference`s the resource fails to start. | ||
| - **`WithReference(x)` ≠ wait-for-healthy in Aspire 13.** `WithReference` only injects connection strings / endpoints; the service starts as soon as its env vars are resolvable, even if the target is still warming up. Containers like the Service Bus emulator take 30-60s to be healthy on first run; services that race past that crash with "connection refused" and exit. **Hard rule: every `WithReference` on a non-trivial dependency (DB, messaging, identity, peer service) gets a matching `.WaitFor(x)`.** Without it, the Aspire dashboard shows services as "Finished" instead of "Running" because they exited before infra was ready. | ||
| - **Wolverine `AutoProvision()` is incompatible with the Service Bus emulator — disable it for local dev.** `AutoProvision()` (default on; `Wolverine:AutoProvision` true) provisions topics/subscriptions at startup via the Service Bus **management API** (`ServiceBusAdministrationClient`). The **emulator implements only the AMQP data plane, not the management API**, so every check fails, retries 4×, and the host dies with `BrokerInitializationException: Unable to initialize the Broker asb in time` — a sub-second, deterministic startup crash hitting *every* Wolverine service (catalog has no ASB, so it survives, which makes it look selective). Locally the topology is already declared in `AppHost.cs` (`AddServiceBusTopic`/`AddServiceBusSubscription` write the emulator's config), so provisioning is impossible *and* redundant. **Fix: AppHost injects `Wolverine__AutoProvision=false` into each Wolverine service** (mirrors the integration-test harnesses); in Publish mode against real Azure it stays on to create entities. See [docs/architecture.md](docs/architecture.md) Infrastructure section. | ||
| - **Wolverine `AutoProvision()` is incompatible with the Service Bus emulator — disable it for local dev.** `AutoProvision()` (default on; `Wolverine:AutoProvision` true) provisions topics/subscriptions at startup via the Service Bus **management API** (`ServiceBusAdministrationClient`). The 2.0.0 emulator *does* implement that API (on its `emulatorhealth` HTTP endpoint, port 5300), but its **subscription admin endpoints return HTTP 500** (topics and queues return 200), so AutoProvision's per-subscription check fails, retries 4×, and the host dies with `BrokerInitializationException: Unable to initialize the Broker asb in time` — a sub-second, deterministic startup crash hitting *every* Wolverine service (catalog has no ASB, so it survives, which makes it look selective). Locally the topology is already declared in `AppHost.cs` (`AddServiceBusTopic`/`AddServiceBusSubscription` write the emulator's `Config.json`, which it provisions at container start), so provisioning is redundant — and with `AutoProvision=false` the subscription **listener binds directly over AMQP with no management call** (decompiled: `AzureServiceBusSubscription.InitializeAsync` only hits the 500-ing path when `AutoProvision` is true). **Fix: AppHost injects `Wolverine__AutoProvision=false` into each Wolverine service** (mirrors the integration-test harnesses); in Publish mode against real Azure it stays on to create entities. See [docs/architecture.md](docs/architecture.md) Infrastructure section. | ||
|
|
There was a problem hiding this comment.
CLAUDE.md size now blocks CI merge.
BUILD_AND_TEST / build is failing because this file crossed the 500-line hard cap. Move detailed rationale to a paired doc/skill and keep only concise rule headlines + links here.
As per coding guidelines, CLAUDE.md must stay lean and CI enforces a hard fail at 500 lines.
🤖 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 `@CLAUDE.md` around lines 227 - 231, CLAUDE.md exceeds the 500-line hard cap
which is blocking the CI build. Refactor by moving the four detailed bullet
points about Azure Aspire configuration (Service Bus subscription naming
conventions, Aspire 13+ resource fallbacks, WithReference behavior with WaitFor
requirements, and Wolverine AutoProvision incompatibility with the emulator) out
of CLAUDE.md and into a paired documentation file in the docs/ folder (e.g.,
docs/aspire-setup.md or similar). Replace the detailed explanations in CLAUDE.md
with concise rule headlines followed by links to the new paired doc, similar to
how the existing content references docs/architecture.md. This keeps CLAUDE.md
lean and scannable while preserving the detailed rationale in dedicated skill
documentation.
Sources: Coding guidelines, Pipeline failures
Comment on lines
+120
to
133
| // to create/verify topics + subscriptions at host startup. The 2.0.0 emulator DOES implement the | ||
| // management API (on its 'emulatorhealth' HTTP endpoint, port 5300) — but its SUBSCRIPTION admin | ||
| // endpoints return HTTP 500 (topics + queues return 200). So AutoProvision's per-subscription | ||
| // SubscriptionExistsAsync/CreateSubscriptionAsync calls fail and the host dies with | ||
| // `BrokerInitializationException: Unable to initialize the Broker asb in time`. Locally the topology | ||
| // is already declared above (AddServiceBusTopic/AddServiceBusSubscription write the emulator's | ||
| // Config.json, which the emulator provisions at container start), and Wolverine's listener attaches | ||
| // over AMQP — which works — so provisioning is both impossible AND redundant. (Decompiled proof: | ||
| // AzureServiceBusSubscription.InitializeAsync only calls the 500-ing SetupAsync when AutoProvision | ||
| // is true; with it false, the listener binds over AMQP with no management call.) Disable it for the | ||
| // four Wolverine services in dev; in Publish mode against real Azure, AutoProvision stays on | ||
| // (Wolverine:AutoProvision defaults true) to create the entities. Mirrors the test harnesses' | ||
| // `Wolverine:AutoProvision=false`. See CLAUDE.md. | ||
| // `Wolverine:AutoProvision=false`. See CLAUDE.md + #148. | ||
| const string disableAutoProvision = "Wolverine__AutoProvision"; |
There was a problem hiding this comment.
Publish-mode AutoProvision behavior is contradicted by current env injection.
Line 130 says AutoProvision stays enabled in publish mode, but Lines 144, 151, 159, and 165 set Wolverine__AutoProvision=false unconditionally. That keeps AutoProvision disabled even for publish runs via AppHost.
Suggested fix
const string disableAutoProvision = "Wolverine__AutoProvision";
+var autoProvisionValue = builder.ExecutionContext.IsPublishMode ? "true" : "false";
@@
.WithReference(realm, configurationPrefix: keycloakConfigPrefix).WaitFor(realm)
.WithEnvironment("Frontend__AllowedOrigins", SpaDevOrigin)
- .WithEnvironment(disableAutoProvision, "false");
+ .WithEnvironment(disableAutoProvision, autoProvisionValue);
@@
.WithReference(realm, configurationPrefix: keycloakConfigPrefix).WaitFor(realm)
- .WithEnvironment(disableAutoProvision, "false");
+ .WithEnvironment(disableAutoProvision, autoProvisionValue);
@@
.WithReference(realm, configurationPrefix: keycloakConfigPrefix).WaitFor(realm)
.WithEnvironment("Frontend__AllowedOrigins", SpaDevOrigin)
- .WithEnvironment(disableAutoProvision, "false");
+ .WithEnvironment(disableAutoProvision, autoProvisionValue);
@@
WithOptionalAppInsights(
builder.AddProject<Projects.NotificationService>("notification-service")
.WithReference(serviceBus).WaitFor(serviceBus), appInsights)
- .WithEnvironment(disableAutoProvision, "false");
+ .WithEnvironment(disableAutoProvision, autoProvisionValue);🤖 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 `@NextAurora.AppHost/AppHost.cs` around lines 120 - 133, The code
unconditionally disables AutoProvision across all environments by setting
`Wolverine__AutoProvision=false` at four locations, contradicting the documented
intent that AutoProvision should remain enabled in publish mode. Modify the four
environment variable assignments where `disableAutoProvision` is used (in the
Wolverine service configurations) to only apply the false value when building
for dev/emulator mode. For publish mode builds, either omit the setting to allow
the Wolverine default of true, or explicitly set `Wolverine__AutoProvision=true`
to ensure AutoProvision is enabled in production against real Azure Service Bus.
Owner
Author
Correction — live testing revealed the fix is necessary but NOT sufficient (and I'd over-claimed)A live end-to-end run (stack actually booted once the Docker VM had enough RAM) showed the order stays at
So the emulator has a dual-bind: Per the official Wolverine guidance for permission-limited brokers, the fix is The subscription-listener API fix and the "emulator has a management API" doc correction remain valid regardless. |
Owner
Author
|
Superseded by #159. The ASB subscription-listener API fix here is carried into #159, but the headline goal shifted: rather than fix the ASB emulator (which has upstream limitations — subscription admin 500 + non-provisionable system queues), #159 makes the transport config-selectable with RabbitMQ as the default, which runs the full saga locally (verified: order → Shipped) and matches the deployed broker. ASB stays opt-in for Azure. Closing this in favor of #159. |
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.
What
Two things, both real and prod-relevant:
Subscription-listener API fix (the actual long-standing local-saga blocker). Listeners were registered with a combined
"{topic}/{sub}"string:which silently mis-registers the listener so it never attaches. Corrected to separate args across Order/Payment/Shipping/Notification:
This would fail against real Azure Service Bus too — not just the emulator.
Doc corrections. CLAUDE.md + architecture.md claimed "the emulator implements only the AMQP data plane, not the management API." That's false. The 2.0.0 emulator does expose the management API (on
emulatorhealth, port 5300); only its subscription admin endpoints return HTTP 500 (topics/queues return 200). Corrected in CLAUDE.md,docs/architecture.md(×3),docs/how-it-works.md,docs/full-saga-deployment-plan.md.Why
AutoProvision=falsestays (and is sufficient)Decompiling Wolverine 6.8.0:
With
AutoProvision=false, no management-API call happens — the listener binds over AMQP to the subscription Aspire'sConfig.jsonalready created, so the subscription-admin 500 is never reached. An earlier "Approach B" commit on this branch re-enabled AutoProvision + injected a management connection string; that reintroduced the 500 and is fully reverted here. Net diff vsmain= the API fix + accurate comments.Test plan
dotnet build— clean (0 warnings,TreatWarningsAsErrorson).CreateRenderedResourcesAsync(container creation) on the external-drive-backed Docker, before any service starts. Unrelated to this change. The fix rests on the decompiled control-flow proof above. Re-run once Docker is on faster storage to observe the subscription listeners attach.Notes
/check-rulesaudit: CLAUDE.md touched (subscription-naming rule + AutoProvision rule). Inline rule-paraphrase comments end withSee CLAUDE.md.🤖 Generated with Claude Code
Summary by CodeRabbit
Bug Fixes
Documentation