Skip to content
Merged
Show file tree
Hide file tree
Changes from 8 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 8 additions & 1 deletion .claude/agents/architecture-reviewer.md
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,14 @@ Specific bug-classes that have bitten this repo before. When the target file mat
- **`.RequireAuthorization()` at group level** unless explicitly public.
- **List endpoints clamp pagination** server-side (`ClampPaging` or equivalent, cap ≤ 100).
- **Rate-limiter shape (Should-consider on single-instance, Must-fix when scaled to 2+ instances).** ASP.NET Core's `RequireRateLimiting` + `AddFixedWindowLimiter` / `AddSlidingWindowLimiter` uses an in-memory counter store. Single-instance: correct. 2+ instances: the limit silently multiplies by N — each instance enforces its own counter, a client hitting any instance gets a fresh allowance. NextAurora is single-instance everywhere today (Catalog deployed on one Fly Machine; rest local), so in-memory is right *for now*. Flag: (a) a new `RequireRateLimiting` on an endpoint without a comment at the call site or registration site (`Program.cs` `AddFixedWindowLimiter`) justifying why in-memory is correct + naming the swap-to-Redis trigger, OR (b) a deployment-config PR (`Dockerfile.*`, `fly.toml`, GitHub Actions deploy workflow) that scales a rate-limited service past 1 Machine without a paired swap to a Redis-backed limiter. The fix when scale-out lands: Redis-backed limiter using the existing HybridCache Redis, with the increment + TTL pair wrapped in a Lua `EVAL` so it's atomic under concurrency. See CLAUDE.md "Security Requirements → Rate Limiting".
- **Long-running work shape (Must-fix on minutes-scale handlers, Should-consider on >1s).** If a write endpoint synchronously awaits something that can take more than ~1s — multi-step external API chain (e.g. Stripe + tax calc + fraud check sequentially), aggregation over thousands of rows, bulk import, report generation — it's the wrong shape. The HTTP request holds a thread, a DB connection, and a concurrency-budget slot for the full duration, so a small spike on that one endpoint can take the rest of the API down with it. Reshape as 202 Accepted: validate + persist a tracking row + publish a Wolverine message + return `202` with the job/correlation ID in the body and a `Location` header pointing to a status endpoint. A background handler does the work; the client polls or receives a push (SignalR/SSE/email). The synchronous parts commit in one EF transaction via `AutoApplyTransactions`. NextAurora already has the full machinery (Wolverine + Service Bus + outbox + saga handlers) — the rule is "use it when a handler would otherwise block on minutes-scale work." Reference shape: `POST /api/v1/orders` (place → publish `OrderPlaced` → return OrderId immediately; PaymentService + ShippingService handle the downstream work async via the saga). Same rule applies to Wolverine handlers themselves: if the handler body runs for minutes, the work belongs in a follow-up message handler, not in-line. See CLAUDE.md "Performance Rules → Long-running work belongs on the message bus."
- **Long-running work shape (Must-fix on minutes-scale handlers, Should-consider on >1s).** If a write endpoint synchronously awaits something that can take more than ~1s — multi-step external API chain (e.g. Stripe + tax calc + fraud check sequentially), aggregation over thousands of rows, bulk import, report generation — it's the wrong shape. The HTTP request holds a thread, a DB connection, and a concurrency-budget slot for the full duration, so a small spike on that one endpoint can take the rest of the API down with it. Reshape as 202 Accepted: validate + persist a tracking row + publish a Wolverine message + return `202` with the job/correlation ID in the body and a `Location` header pointing to a status endpoint. A background handler does the work; the client polls or receives a push (SignalR/SSE/email). The synchronous parts commit in one EF transaction via `AutoApplyTransactions`. NextAurora already has the full machinery (Wolverine + RabbitMQ + outbox + saga handlers) — the rule is "use it when a handler would otherwise block on minutes-scale work." Reference shape: `POST /api/v1/orders` (place → publish `OrderPlaced` → return OrderId immediately; PaymentService + ShippingService handle the downstream work async via the saga). Same rule applies to Wolverine handlers themselves: if the handler body runs for minutes, the work belongs in a follow-up message handler, not in-line. See CLAUDE.md "Performance Rules → Long-running work belongs on the message bus."

### When reviewing `**/Program.cs` messaging blocks (Wolverine/RabbitMQ wiring)

- **Topology completeness vs the first-boot race (CRITICAL until #168 lands).** Fanout exchanges silently discard unroutable messages, and AutoProvision declares topology lazily per-service — a consumer's queue+binding exists only after that consumer's first boot. Flag any new publisher whose consumers' queues are not also declared publisher-side (or otherwise guaranteed to exist before first publish). See CLAUDE.md.
- **Durability is per-direction (Must-fix on new listeners until #169 lands).** `UseDurableOutboxOnAllSendingEndpoints()` covers only sends; default listeners are buffered (acked before handlers run). New `ListenToRabbitQueue(...)` endpoints need `UseDurableInboxOnAllListeningEndpoints()` (store-backed services) or `ProcessInline()` (stateless services). See CLAUDE.md.
- **Exchange/queue names as shared constants, not inline literals.** A typo'd `BindExchange("payment_events")` is silently auto-provisioned and the consumer never receives real events — no error anywhere. Names duplicated between `BindExchange(...).ToQueue("x")` and `ListenToRabbitQueue("x")` must be a single const; cross-service names belong in a shared topology constants home (see #168 discussion).
- **Config-key shape consistency.** `Wolverine:AutoProvision` is read colon-form via `UseSetting`/config; environment overrides use `Wolverine__AutoProvision`. Flag a third variant.

### When reviewing `**/*RecoveryJob*.cs` or any `BackgroundService` / cron-style sweeper

Expand Down
10 changes: 5 additions & 5 deletions .claude/architecture-map.md
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ PlaceOrder (HTTP POST → OrderService)
│ 2. Order aggregate saved + OrderPlacedEvent staged in outbox (same tx)
└─→ OrderPlacedEvent (Service Bus topic)
└─→ OrderPlacedEvent (RabbitMQ exchange)
├─→ PaymentService consumes (OrderPlacedHandler → ProcessPayment)
│ │
│ │ Payment aggregate saved + PaymentCompletedEvent staged in outbox
Expand Down Expand Up @@ -79,9 +79,9 @@ All events live in `NextAurora.Contracts/Events/`:
- `PaymentFailedEvent` — published by PaymentService
- `ShipmentDispatchedEvent` — published by ShippingService

Service Bus topology: one topic per event type, one subscription per consumer-source pair.
Subscription names are globally unique within the namespace (Aspire 13+ rule).
Convention: `{consumer}-{source-events}-sub` (e.g. `notify-orders-sub`).
RabbitMQ topology: one fanout exchange per event type, one queue per consumer bound to it.
Convention: `{consumer}-{source-events}` (e.g. `notify-orders` = NotificationService consuming
`order-events`). Wolverine declares + AutoProvisions the exchanges/queues/bindings.

---

Expand Down Expand Up @@ -150,7 +150,7 @@ the failure mode.

- **REST (HTTP)** — frontend ↔ services only. URL-segment versioned `/api/v1/...`.
- **gRPC (sync)** — OrderService → CatalogService for real-time product validation + stock reservation. Versioned via `.proto` `package` declarations.
- **Service Bus (async)** — all workflow events. Wolverine transports + transactional outbox.
- **RabbitMQ (async)** — all workflow events. Wolverine transport + transactional outbox.

---

Expand Down
1 change: 1 addition & 0 deletions .claude/audits/INDEX.md
Original file line number Diff line number Diff line change
Expand Up @@ -38,3 +38,4 @@ Persistent log of every `/article-audit` run. One row per audit; full reasoning
| 2026-06-12 | [The Async/Await Habit That Quietly Slows ASP.NET Core Under Load](2026-06-12-async-await-habits-under-load.md) (Kerim Kara, Activated Thinker) | ✅ All five teaser categories encoded, mostly more rigorously (build-time bans + hooks vs advice); paywall-limited — intro only, re-audit on paste | No action |
| 2026-06-12 | [.NET Microservices Guide — What To Do and What To Avoid](2026-06-12-dotnet-microservices-do-avoid.md) (Kerim Kara, Real World .NET) | ⚙️ Re-audited after user pasted full body (initial pass paywall-limited). All 10 do/avoid items covered; article's own examples violate the canon 3× (publish-after-save dual-write vs outbox; ValidateAudience=false vs JWT rule; IOrderService layer vs VSA). Recurring implicit stance: internal-gRPC trust + no-gateway undocumented (2nd hit today) | Offered small encoding issue (internal-trust + no-gateway triggers); pending user decision |
| 2026-06-12 | [Building Dapr Workflows in .NET With Aspire](2026-06-12-dapr-workflows-aspire.md) (Milan Jovanović, The .NET Weekly) | ✅ Considered + rejected with depth — project-decisions.md §22 ("Dapr — considered, not adopted") + architecture.md:283 (choreography saga, no central orchestrator) + perf doc already names Temporal/Durable Functions/Step Functions as the durable-execution alternatives. 202-Accepted + idempotency + outbox all encoded | No new issue — §22's table maps the classic 5 building blocks but not **Dapr Workflow**; fold a Workflow row + orchestration-vs-choreography trigger into open #86 |
| 2026-06-15 | [Wolverine + Azure Service Bus Emulator](2026-06-15-wolverine-asb-emulator.md) (WolverineFx official docs) | ⚙️ Official recipe (`UseAzureServiceBus` + `transport.ManagementConnectionString` on :5300 + AutoProvision) matches in-flight feat-branch fix and **contradicts two `main` rules** — CLAUDE.md:230 + architecture.md:399 claim "emulator has no management API" (false; it does, only subscription admin returns 500). Also confirms the `ListenToAzureServiceBusSubscription(sub, c => c.TopicName=topic)` API vs our wrong `"{topic}/{sub}"` form | No new issue — tracked on #148; doc corrections land with that branch. Confirms feat-branch "Approach B" is officially correct; subscription-admin-500 limitation is ours to document (upstream omits it) |
11 changes: 10 additions & 1 deletion .claude/scripts/check-claude-md-refs.sh
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,15 @@ if [ -z "$matches" ]; then
exit 0
fi

# AI-instruction surfaces are ALWAYS on the review list, marker or not. They paraphrase
# canon for other AI tools (Copilot, CodeRabbit), so drift there silently regenerates
# retired patterns — .github/copilot-instructions.md still taught the removed transport
# months after the swap because it carried no marker. See CLAUDE.md.
ai_surfaces=""
for f in "$REPO_ROOT/.github/copilot-instructions.md" "$REPO_ROOT/.coderabbit.yaml"; do
[ -f "$f" ] && ai_surfaces="${ai_surfaces}${f}"$'\n'
done

# Emit additionalContext via hookSpecificOutput so the model sees the reminder.
msg=$(printf 'CLAUDE.md was edited. Files containing the "See CLAUDE.md" marker (review each for staleness against the new rule):\n%s' "$matches")
msg=$(printf 'CLAUDE.md was edited. Files containing the "See CLAUDE.md" marker (review each for staleness against the new rule):\n%s\n\nAI-instruction surfaces (ALWAYS check these — they paraphrase canon for other AI tools and drift silently):\n%s' "$matches" "$ai_surfaces")
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated
jq -n --arg m "$msg" '{hookSpecificOutput: {hookEventName: "PostToolUse", additionalContext: $m}}'
50 changes: 50 additions & 0 deletions .claude/scripts/check-tombstones.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
#!/usr/bin/env bash
# Tombstone audit — removed-identifier drift control. See CLAUDE.md.
#
# When a subsystem is removed (a transport, a metric, an API), its identifiers get
# tombstoned in .claude/tombstones.txt. This script fails if any tombstoned pattern
# appears in tracked files outside the allowlist (.claude/tombstones-allowlist.txt).
#
# Why: the compiler catches stale identifiers in code; NOTHING catches them in docs,
# comments, and config. The RabbitMQ swap (#159) left 15+ docs teaching Azure Service
# Bus as current — found by an ultracode review, not by the loop. This closes that gap
# mechanically: the sweep's completion criterion is "this script passes", not "the docs
# someone remembered are updated".
#
# Usage: .claude/scripts/check-tombstones.sh (run from anywhere; CI runs it per PR)

set -euo pipefail

ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/../.." && pwd)"
cd "$ROOT"

TOMBSTONES=".claude/tombstones.txt"
ALLOWLIST=".claude/tombstones-allowlist.txt"

excludes=()
while IFS= read -r line; do
[ -z "$line" ] && continue
case "$line" in \#*) continue ;; esac
excludes+=(":(exclude)$line")
done < "$ALLOWLIST"

fail=0
while IFS= read -r pattern; do
[ -z "$pattern" ] && continue
case "$pattern" in \#*) continue ;; esac
# git grep over tracked files only; -i case-insensitive, -E extended regex.
if hits=$(git grep -inE "$pattern" -- '.' "${excludes[@]}" 2>/dev/null) && [ -n "$hits" ]; then
echo "TOMBSTONE VIOLATION — pattern '$pattern':"
echo "$hits" | sed 's/^/ /' | head -30
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated
echo ""
fail=1
fi
Comment thread
coderabbitai[bot] marked this conversation as resolved.
done < "$TOMBSTONES"

if [ "$fail" -eq 1 ]; then
echo "Removed identifiers are resurfacing (or were never fully swept)."
echo "Fix the references — or, for genuinely historical/comparative mentions,"
echo "add the file to $ALLOWLIST with a justification comment."
exit 1
fi
echo "Tombstone audit clean."
21 changes: 21 additions & 0 deletions .claude/tombstones-allowlist.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
# Files exempt from the tombstone audit — each entry earns its place by containing
# LEGITIMATE historical or comparative mentions of removed identifiers. Keep this list
# short; an allowlisted file can re-drift invisibly, so prefer fixing over allowlisting.
#
# Historical decision records (past-tense by nature):
docs/project-decisions.md
docs/full-saga-deployment-plan.md
docs/war-story-wolverine6-outbox-atomicity.md
# Comparative/portable guides where the removed transport is a subject of comparison:
docs/messaging-transport-selection.md
docs/performance-and-data-correctness.md
# Canon: carries the "evaluated and removed" rationale + durable-pub/sub pattern comparison:
CLAUDE.md
# Past-tense removal note (line ~57) + AWS-alternative comparison section:
docs/architecture.md
# The control's own definition files:
.claude/tombstones.txt
.claude/tombstones-allowlist.txt
.claude/scripts/check-tombstones.sh
# Audit log of external-article reviews (historical verdicts, intentionally tracked):
.claude/audits/INDEX.md
16 changes: 16 additions & 0 deletions .claude/tombstones.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
# Removed-identifier tombstones — one extended-regex pattern per line (case-insensitive).
# When a subsystem/identifier is REMOVED from the codebase, its names go here so CI fails
# any doc/comment/config that still presents it as current. This is the identifier-level
# counterpart of the file-move discipline. See CLAUDE.md.
#
# Files where historical/comparative mentions are legitimate are listed in
# tombstones-allowlist.txt. Everything else must be clean.
#
# --- Azure Service Bus transport (removed in #159; RabbitMQ is the transport) ---
azureservicebus
azure service bus
azure\.messaging\.servicebus
servicebus\.windows\.net
service bus
# Old ASB subscription names (renamed to queue names without the -sub suffix)
(payment-orders|notify-orders|order-payments|shipping-payments|notify-payments|order-shipping|notify-shipping)-sub
36 changes: 35 additions & 1 deletion .coderabbit.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -261,7 +261,7 @@ reviews:
one EF transaction via AutoApplyTransactions.

NextAurora already has the full infrastructure for this — Wolverine +
Service Bus + outbox + saga handlers — so the rule is "use it when a
RabbitMQ + outbox + saga handlers — so the rule is "use it when a
handler would otherwise block on minutes-scale work." Reference shape:
POST /api/v1/orders (place → publish OrderPlaced → return OrderId
immediately; PaymentService + ShippingService handle the downstream
Expand Down Expand Up @@ -523,6 +523,40 @@ reviews:
Same reasoning as the AppHost.cs rule above — reviewers reason against
the .svg picture of the request lifecycle. Flag missing pairs.

TRANSPORT-REMOVAL SWEEP. When a messaging/transport package is removed
(e.g. a Wolverine transport swap), removing the `PackageReference` is not
enough — flag two orphans: (1) a dead OpenTelemetry `AddSource("...")` for
that transport's ActivitySource in the `.WithTracing(...)` block here
(silently drops messaging spans from the dashboard), and (2) an orphaned
`PackageVersion` left in `Directory.Packages.props` with no remaining
`PackageReference`. Both bit the ASB→RabbitMQ swap (PR #159).

- path: "**/Program.cs"
instructions: |
MESSAGING WIRING (Wolverine/RabbitMQ — see CLAUDE.md Package Management +
Communication Patterns traps). (1) Fanout exchanges DISCARD unroutable
messages and AutoProvision declares topology lazily per-service: flag a new
publisher whose consumers' queues aren't guaranteed to exist before first
publish (first-boot loss window — CLAUDE.md trap, tracked #168). (2)
Durability is per-direction: UseDurableOutboxOnAllSendingEndpoints covers
only sends; new ListenToRabbitQueue endpoints without
UseDurableInboxOnAllListeningEndpoints (or ProcessInline for stateless
services) are buffered and lose in-flight messages on crash (tracked #169).
(3) Exchange/queue name literals duplicated between BindExchange().ToQueue()
and ListenToRabbitQueue() must be a single const — a typo'd name is silently
auto-provisioned and the consumer starves with no error.

- path: "docs/**/*.md"
instructions: |
TOMBSTONED IDENTIFIERS (see CLAUDE.md "Identifier-move discipline"). Flag
any PRESENT-TENSE claim that a removed subsystem is current — the tombstone
list lives in .claude/tombstones.txt (currently: the removed cloud-broker
transport's API names and its old *-sub queue names). Historical/comparative mentions are fine in
the allowlisted decision docs (.claude/tombstones-allowlist.txt). Also flag
docs that assert a metric, trace source, or config key exists when the diff
(or repo) shows nothing emits/reads it — a documented-but-dead signal is
worse than none.

- path: "**"
instructions: |
FILE-MOVE DISCIPLINE (CLAUDE.md "File-move discipline"). When this PR's
Expand Down
Loading
Loading