Skip to content
Merged
Show file tree
Hide file tree
Changes from 5 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
6 changes: 6 additions & 0 deletions .claude/agents/architecture-reviewer.md
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,12 @@ Specific bug-classes that have bitten this repo before. When the target file mat
- **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 + 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 metrics / `Meter` / `Counter` declarations

- **A declared-but-never-incremented instrument is a defect, not inert code (Must-fix).** Every `CreateCounter`/`CreateHistogram` needs at least one `.Add(...)`/`.Record(...)` call site in tracked code, and every metrics *holder class* must actually be injected somewhere. A counter nothing increments reads as a working alarm signal — an operator wires an alert to it and it never fires. This is worse than no counter. (Found the hard way: a `NextAuroraMetrics` class was registered but never injected; all five of its counters were dead while docs presented one as *the* DLQ alarm. Deleted in #171.)
- **Prefer the framework's own instruments over hand-rolled equivalents.** Wolverine already emits `wolverine-dead-letter-queue`, `wolverine-execution-failure`, `wolverine-messages-sent`/`-received`, and inbox/outbox depth. Don't re-implement them.
- **Wolverine's meter is named `Wolverine:{ServiceName}`, so OTel must register it as `AddMeter("Wolverine*")`** — a literal `AddMeter("Wolverine")` silently collects nothing. Flag any "tidying" of that wildcard.

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

- **Topology completeness vs the first-boot race (CRITICAL).** 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.
Expand Down
2 changes: 1 addition & 1 deletion .claude/commands/grid-infographic.md
Original file line number Diff line number Diff line change
Expand Up @@ -307,7 +307,7 @@ the surface taxonomy should be 5 or 6. The grid template accommodates either —
6 surfaces fit cleanly into Section 04 as a single column of 6 items, or split
2×3.

See `docs/post-ideas/two-readers-one-canon.md`
See `docs/decisions/2026-07-17-sixth-surface-okl.md`
for the deeper take on trigger-loaded as a *load-timing* dimension.

## When NOT to use this skill
Expand Down
7 changes: 4 additions & 3 deletions .claude/scripts/check-messaging-topology.sh
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,10 @@ fail=0
# --- 1. Every queue const reaches a .ToQueue( declaration somewhere -------------------
while IFS= read -r name; do
if ! grep -q "\.ToQueue(MessagingQueues\.$name)" "${PROGRAMS[@]}"; then
# Direct queues (no exchange binding) are exempt from .ToQueue() — but their
# declaration IS the ListenToRabbitQueue call, so that must exist instead;
# otherwise removing the sole listener leaves an undeclared queue and a clean audit.
# FORWARD-GUARD: no direct queue exists today (the only one was dead wiring, removed
# in #170). If one is ever re-introduced, it is exempt from .ToQueue() —
# but its declaration IS the ListenToRabbitQueue call, so that must exist instead;
# otherwise removing the sole listener would leave an undeclared queue and a clean audit.
if grep -B2 "public const string $name = " "$TOPOLOGY" | grep -qi "direct queue"; then
if ! grep -q "ListenToRabbitQueue(MessagingQueues\.$name)" "${PROGRAMS[@]}"; then
echo "TOPOLOGY GAP — direct queue MessagingQueues.$name has no ListenToRabbitQueue() declaring it"
Expand Down
94 changes: 67 additions & 27 deletions .claude/scripts/check-tombstones.sh
Original file line number Diff line number Diff line change
@@ -1,17 +1,27 @@
#!/usr/bin/env bash
# Tombstone audit — removed-identifier drift control. See CLAUDE.md.
# Tombstone audit — removed-identifier drift control. See CLAUDE.md "Identifier-move discipline".
#
# 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).
# 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".
# 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)
# GROUP-SCOPED EXEMPTIONS: patterns live in groups ([group-name] headings); an allowlist line is
# `<group> <path>` and exempts that file from that group ONLY. A file allowlisted for a past
# removal is still audited against every future tombstone. (The original file-scoped allowlist
# exempted docs/architecture.md from ALL later tombstones, so the audit passed while it still
# documented deleted artifacts as live — found by architecture review, not by this script.)
#
# KNOWN BLIND SPOT: identifiers split across a line break evade these regexes — in .excalidraw
# JSON a wrapped label is literally "messages.\nabandoned", which `messages\.abandoned` cannot
# match. When a tombstoned name might appear in a DIAGRAM, check the rendered .png/.svg too —
# the render is the only surface that sees wrapped text.
#
# Usage: .claude/scripts/check-tombstones.sh (CI runs it per PR)

set -euo pipefail

Expand All @@ -21,32 +31,62 @@ cd "$ROOT"
TOMBSTONES=".claude/tombstones.txt"
ALLOWLIST=".claude/tombstones-allowlist.txt"

excludes=()
fail=0
group=""

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.
# `[group-name]` starts a new group.
case "$line" in
\[*\])
group="${line#[}"
group="${group%]}"
continue
;;
esac

pattern="$line"
if [ -z "$group" ]; then
echo "TOMBSTONE CONFIG ERROR — pattern '$pattern' appears before any [group] heading."
exit 1
fi

# Build this pattern's exclusions: files allowlisted for THIS group only.
excludes=()
while read -r grp pth; do
[ -z "$grp" ] && continue
case "$grp" in \#*) continue ;; esac
[ -n "$pth" ] || continue
if [ "$grp" = "$group" ]; then
excludes+=(":(exclude)$pth")
fi
done < "$ALLOWLIST"

# git grep exit codes: 0 = matches (violation), 1 = clean, >=2 = error (e.g. invalid regex).
# An invalid tombstone must FAIL the audit, not silently disable itself.
# `-e "$pattern"` so a pattern starting with `-` (e.g. `-sub`) is never parsed as a flag.
# Guard the array expansion: under `set -u`, "${excludes[@]}" on an empty array is an
# "unbound variable" error in bash < 4.4 (macOS default 3.2) — a group with zero
# exemptions would otherwise break local runs.
set +e
hits=$(git grep -inE "$pattern" -- '.' "${excludes[@]}" 2>&1)
if [ "${#excludes[@]}" -gt 0 ]; then
hits=$(git grep -inE -e "$pattern" -- '.' "${excludes[@]}" 2>&1)
else
hits=$(git grep -inE -e "$pattern" -- '.' 2>&1)
fi
status=$?
set -e
Comment thread
coderabbitai[bot] marked this conversation as resolved.

if [ "$status" -ge 2 ]; then
echo "TOMBSTONE AUDIT ERROR — pattern '$pattern' failed to evaluate (git grep exit $status):"
echo "TOMBSTONE AUDIT ERROR — [$group] 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 "TOMBSTONE VIOLATION — [$group] pattern '$pattern':"
# sed -n rather than `| head`: under pipefail, head closing the pipe early would
# SIGPIPE the producer and abort the script mid-audit.
echo "$hits" | sed -n '1,30p' | sed 's/^/ /'
echo ""
fail=1
Expand All @@ -55,8 +95,8 @@ 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."
echo "Fix the references — or, for a genuinely historical/comparative mention, add"
echo "'<group> <path>' to $ALLOWLIST with a justification comment."
exit 1
fi
echo "Tombstone audit clean."
67 changes: 45 additions & 22 deletions .claude/tombstones-allowlist.txt
Original file line number Diff line number Diff line change
@@ -1,23 +1,46 @@
# 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.
# Tombstone-audit exemptions — GROUP-SCOPED.
#
# Historical decision records (past-tense by nature):
docs/project-decisions.md
docs/full-saga-deployment-plan.md
docs/war-story-wolverine6-outbox-atomicity.md
# Retrospective ABOUT the ASB-emulator debugging saga (historical narrative by nature):
docs/the-integration-gap.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
# FORMAT: <group-name> <path>
#
# Each line exempts ONE file from ONE tombstone group (groups are the `[name]` headings in
# tombstones.txt). An entry earns its place by containing LEGITIMATE historical or comparative
# mentions of that group's removed identifiers.
#
# WHY GROUP-SCOPED: a file allowlisted for a past removal must STILL be audited against every
# future tombstone. The earlier file-scoped format silently exempted `docs/architecture.md`
# (allowlisted for the Azure Service Bus removal) from the send-notification and
# messages.abandoned tombstones added months later — so the audit reported "clean" while
# architecture.md still documented both deleted artifacts as live. Never re-introduce a
# whole-file exemption.
#
# Keep this list short: an allowlisted (file, group) pair can re-drift invisibly, so prefer
# fixing the reference over adding an exemption.

# --- azure-service-bus: historical decision records + comparative guides ---
azure-service-bus docs/project-decisions.md
azure-service-bus docs/full-saga-deployment-plan.md
azure-service-bus docs/war-story-wolverine6-outbox-atomicity.md
azure-service-bus docs/messaging-transport-selection.md
azure-service-bus docs/performance-and-data-correctness.md
azure-service-bus docs/the-integration-gap.md
azure-service-bus CLAUDE.md
azure-service-bus docs/architecture.md
azure-service-bus .claude/audits/INDEX.md

# --- dead-observability-artifacts: the decision record showing the pre-removal OTel wiring ---
dead-observability-artifacts docs/project-decisions.md

# --- dead-observability-artifacts: the reviewer checklist cites the deleted class as the
# worked example of the rule it encodes (historical, by construction) ---
dead-observability-artifacts .claude/agents/architecture-reviewer.md

# --- the control's own definition files (they must name every pattern, by construction) ---
azure-service-bus .claude/tombstones.txt
azure-service-bus .claude/tombstones-allowlist.txt
azure-service-bus .claude/scripts/check-tombstones.sh
send-notification-queue .claude/tombstones.txt
send-notification-queue .claude/tombstones-allowlist.txt
send-notification-queue .claude/scripts/check-tombstones.sh
dead-observability-artifacts .claude/tombstones.txt
dead-observability-artifacts .claude/tombstones-allowlist.txt
dead-observability-artifacts .claude/scripts/check-tombstones.sh
41 changes: 34 additions & 7 deletions .claude/tombstones.txt
Original file line number Diff line number Diff line change
@@ -1,16 +1,43 @@
# 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.
# Removed-identifier tombstones. See CLAUDE.md "Identifier-move discipline".
#
# Files where historical/comparative mentions are legitimate are listed in
# tombstones-allowlist.txt. Everything else must be clean.
# When a subsystem/identifier is REMOVED, its names go here so CI fails any doc, comment, or
# config that still presents it as current. This is the identifier-level counterpart of the
# file-move discipline: the compiler catches stale identifiers in code; nothing catches them
# in prose.
#
# --- Azure Service Bus transport (removed in #159; RabbitMQ is the transport) ---
# FORMAT: patterns are organised into GROUPS. A group starts with `[group-name]`; every
# pattern below it belongs to that group until the next `[...]` line. Patterns are
# case-insensitive extended regexes (git grep -iE).
#
# WHY GROUPS: allowlist entries are scoped to ONE group (see tombstones-allowlist.txt). A file
# exempted for a past removal must still be audited against every future tombstone. The
# earlier file-scoped allowlist let `docs/architecture.md` — exempted for the Azure Service Bus
# removal — silently skip the send-notification and messages.abandoned tombstones added months
# later, so the audit reported "clean" on a sweep that was not to zero. Group-scoping closes it.

[azure-service-bus]
# 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

[send-notification-queue]
# Removed in #170 — dead-end wiring. Nothing published SendNotificationCommand and it had no
# handler; the working path is the in-process Wolverine cascade of the internal
# SendNotificationRequest. Documenting the queue as publishable was actively harmful (a
# conforming publisher's message would arrive unhandleable and be discarded).
SendNotificationCommand
send-notification

[dead-observability-artifacts]
# Removed in #171 — declared/registered but never incremented/emitted (the ASB processors that
# fed them died in the Wolverine migration). Wolverine's own meter (Wolverine:{ServiceName},
# registered via AddMeter("Wolverine*")) provides the real instruments in their place.
messages\.abandoned
MessagesAbandoned
NextAuroraMetrics
NextAurora\.Messaging
15 changes: 15 additions & 0 deletions .coderabbit.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -508,6 +508,11 @@ reviews:

- path: "NextAurora.ServiceDefaults/Extensions.cs"
instructions: |
OTEL METER NAMES: Wolverine names its meter "Wolverine:{ServiceName}" (e.g.
"Wolverine:NextAurora.OrderService"), so the registration MUST be the wildcard
AddMeter("Wolverine*") — a literal AddMeter("Wolverine") silently collects
nothing. Flag any change that "tidies" the wildcard to a literal. See CLAUDE.md.

SERVICE-REQUEST-FLOW DOC PAIRING (CLAUDE.md "Doc-and-diagram discipline").
Extensions.cs configures the middleware pipeline ordering (auth →
correlation-id → authorization), JWT validation, and the
Expand Down Expand Up @@ -559,6 +564,16 @@ reviews:
ListenToRabbitQueue call instead), or a renamed const whose old string
should be tombstoned (.claude/tombstones.txt).

- path: "**/Metrics/*.cs"
instructions: |
DEAD INSTRUMENTS ARE A DEFECT (Must-fix). Every CreateCounter/CreateHistogram
needs at least one .Add(...)/.Record(...) call site in tracked code, and a
metrics holder class must actually be injected somewhere. A counter nothing
increments reads as a working alarm signal — an operator alerts on it and it
never fires. Prefer the framework's own instruments (Wolverine already emits
wolverine-dead-letter-queue, -execution-failure, -messages-sent/-received,
inbox/outbox depth) over hand-rolled equivalents. See CLAUDE.md.

- path: "docs/**/*.md"
instructions: |
TOMBSTONED IDENTIFIERS (see CLAUDE.md "Identifier-move discipline"). Flag
Expand Down
Loading
Loading