Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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) |
16 changes: 13 additions & 3 deletions .claude/scripts/check-claude-md-refs.sh
Original file line number Diff line number Diff line change
Expand Up @@ -30,11 +30,21 @@ matches=$(grep -rln "See CLAUDE.md" \
| grep -v "^${CANONICAL}\$" \
|| true)

# No matches: silent no-op (nothing to remind about).
if [ -z "$matches" ]; then
# AI-instruction surfaces are ALWAYS on the review list, marker or not — so this list is
# built BEFORE any no-match early-exit. They paraphrase canon for other AI tools (Copilot,
# CodeRabbit), and 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

# Nothing at all to remind about: silent no-op.
if [ -z "$matches" ] && [ -z "$ai_surfaces" ]; then
exit 0
fi

# 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:-(none)}" "$ai_surfaces")
jq -n --arg m "$msg" '{hookSpecificOutput: {hookEventName: "PostToolUse", additionalContext: $m}}'
62 changes: 62 additions & 0 deletions .claude/scripts/check-tombstones.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
#!/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.
# Exit codes: 0 = matches (violation), 1 = clean, >=2 = error (e.g. invalid
# regex) — an invalid tombstone must FAIL the audit, not silently disable it.
set +e
hits=$(git grep -inE "$pattern" -- '.' "${excludes[@]}" 2>&1)
status=$?
set -e
if [ "$status" -ge 2 ]; then
echo "TOMBSTONE AUDIT ERROR — pattern '$pattern' failed to evaluate (git grep exit $status):"
echo "$hits" | sed -n '1,5p'
fail=1
elif [ "$status" -eq 0 ] && [ -n "$hits" ]; then
echo "TOMBSTONE VIOLATION — pattern '$pattern':"
# sed -n '1,30p' rather than piping through head: under pipefail, head
# closing the pipe early would SIGPIPE the producer and abort the script.
echo "$hits" | sed -n '1,30p' | sed 's/^/ /'
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