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
182 changes: 182 additions & 0 deletions .claude/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,182 @@
# `.claude/` — repository AI workflow configuration

This directory holds the configuration that shapes how Claude Code interacts with this repository. Everything here is checked into git so the same workflow runs for every developer (and for AI assistants that pick up work in a fresh session).

If you just want the canonical project rules, those live in [`/CLAUDE.md`](../CLAUDE.md), not here. This directory holds the *tooling* — agents, slash commands, skills, hooks, and the architecture map — that enforces and amplifies those rules.

For the visual + narrative of how all this plumbing fits together, see [`docs/dev-loop.md`](../docs/dev-loop.md).

---

## Directory layout

```
.claude/
├── agents/ Specialized subagents (Agent tool)
├── architecture-map.md Repo-wide code-graph for AI orientation
├── commands/ Slash commands (/command-name)
├── scripts/ Hook scripts wired in settings.json
├── settings.json Permissions, hooks, additional dirs
├── settings.local.json Local-only overrides (gitignored)
└── skills/ On-demand specialized knowledge (Skill tool)
```

---

## The three extension surfaces — agents, commands, skills

These three look superficially similar but solve different problems. Picking the right one matters because each has a different invocation cost + cognitive overhead.

### Quick decision tree

```
Is the work a recurring motion you want as a single keystroke?
├── YES → slash command in .claude/commands/
└── NO → Does it need a focused, independent reviewer
that won't share your conversation context?
├── YES → subagent in .claude/agents/
└── NO → Is it specialized knowledge the model
should pull in on demand?
├── YES → skill in .claude/skills/
└── NO → It's probably a CLAUDE.md rule,
not an extension surface.
```

### Agents (`.claude/agents/`)

**What:** Standalone subagents the main conversation can dispatch. They start fresh with no shared context — only the prompt + the files they read. They produce a report; you decide what to do with it.

**When to use:**
- Independent review or audit work (architecture, security, code review)
- Long-running research that would bloat the main conversation
- Parallel work that doesn't depend on the main conversation state
- Anything that benefits from "fresh eyes" — i.e. *not* primed by the conversation that surfaced the question

**When NOT to use:**
- Quick iterative back-and-forth (use the main conversation)
- Anything that needs the conversation's prior context
- Tasks where the user expects to interactively steer

**How to invoke:**
- From Claude Code: the model calls the Agent tool with `subagent_type: <agent-name>` and a self-contained prompt
- Agents you dispatch run in their own context window with the tools listed in their frontmatter

**Authoring:**
- A markdown file in `.claude/agents/` with YAML frontmatter: `name`, `description`, `tools` (the tools the agent can use)
- The body is the system prompt for the subagent — its job, how it works, output format, hard rules
- The `description` field is critical — Claude Code surfaces it when deciding which agent to dispatch

**Current inventory:**
| Agent | Purpose |
|---|---|
| `architecture-reviewer.md` | Reviews a file/diff against this project's SOLID/DDD/VSA-vs-Clean/Performance/Security rules from CLAUDE.md. Produces a categorized report (Must fix / Should consider / Aligned / Rules to encode). Best invoked with a specific file path. |

### Commands (`.claude/commands/`)

**What:** Markdown files that define `/command-name` slash commands. They bake repeated motions into a single keystroke + a consistent procedure. Run in the main conversation (no separate context).

**When to use:**
- Frequently-repeated procedure with multiple steps that should be done the same way each time
- Refusable patterns (e.g. "scaffold a VSA feature slice — but refuse for CatalogService because it uses Clean Architecture")
- Audit motions (`/check-rules`)
- Status / project-state updates (`/sync-status`)

**When NOT to use:**
- One-off tasks (just ask in chat)
- Things that need fresh context (use an agent)
- Procedures so trivial they don't warrant a name

**How to invoke:**
- Type `/command-name` in Claude Code chat
- Optional positional arguments expand into `$ARGUMENTS` in the command body
- The command's markdown body becomes the prompt for the next assistant turn — so you can include instructions, hard rules, file references, etc.

**Authoring:**
- A markdown file in `.claude/commands/` with YAML frontmatter: `description` (one-line) and `argument-hint` (shown in chat when typing `/`)
- The body is the prompt: tell the model what to do, what to read, what to refuse, what to output

**Current inventory:**
| Command | What it does |
|---|---|
| `/new-feature-slice <ServiceName> <FeatureName>` | Scaffolds a VSA feature slice matching the `PlaceOrder.cs` canonical shape. Refuses for CatalogService (Clean Architecture). |
| `/sync-status` | Refreshes `docs/STATUS.md` from `git log` + open issues, diff-style with confirmation. |
| `/check-rules` | Audits every `See CLAUDE.md` paraphrase against the canonical rule. |

### Skills (`.claude/skills/`)

**What:** Specialized knowledge bundles the model loads on demand. Unlike agents (which run in a separate context), a skill's content gets *injected into the main conversation* when triggered — so the model can apply specialized procedures, vocabularies, or decision criteria mid-task.

**When to use:**
- Procedural knowledge that doesn't apply to every conversation (so it shouldn't be in CLAUDE.md, which is always loaded)
- Cross-cutting expertise (debugging discipline, performance tuning, test-driven development)
- Externally-authored bundles you want to install once (community skills from `obra/superpowers`, `trailofbits/skills`, etc.)
- Domain-specific reference material (e.g. dotnet-performance for EF Core hot-path review)

**When NOT to use:**
- Universal project rules (those go in CLAUDE.md)
- One-off procedures (just describe in chat)
- Things narrow enough to be a slash command

**How to invoke:**
- **Manual:** the model calls the Skill tool with `skill: <skill-name>` based on the user's request
- **Auto-trigger:** Claude Code reads each skill's frontmatter `description` field and matches against user-message intent. If the description says "Use when ... debugging ... bug ... test failure ...", the skill auto-loads when the user reports a bug.
- A skill's full contents (`SKILL.md` + referenced files) become part of the model's working context for the rest of the conversation

**Authoring:**
- A directory in `.claude/skills/<skill-name>/` containing at minimum `SKILL.md` with YAML frontmatter
- `name` (kebab-case, matches directory)
- `description` (load-bearing — this is what triggers auto-invocation; include trigger keywords)
- Optional sub-files (`references/`, `scripts/`) referenced from `SKILL.md`

**Current inventory:**
| Skill | When it fires |
|---|---|
| `dotnet-performance` | EF Core queries, async/await, concurrency, GC, caching, middleware, migrations |
| `excalidraw-diagram` | Generating architecture diagrams |
| `skill-security-auditor` | Pre-install security gate for community skills |
| `verification-before-completion` | About to claim "done/fixed/passing" without evidence |
| `systematic-debugging` | Any bug, test failure, unexpected behavior (auto-triggers) |
| `variant-analysis` | Find one bug, search for sibling patterns across the codebase |
| `test-driven-development` | RED-GREEN-REFACTOR enforcement |
| `using-git-worktrees` | Feature work needing workspace isolation |
| `writing-plans` + `executing-plans` | Spec-driven multi-step work with review checkpoints |

---

## Hooks (`.claude/scripts/`)

Hooks are Bash scripts wired in `settings.json` that fire on Claude Code events. They run in the harness (not in the model's context) and can block tool calls, inject context, or audit changes.

**Current hooks:**
| Hook | Event | Script |
|---|---|---|
| Block sync-over-async | `PreToolUse` (Edit/Write on `.cs`) | `scripts/block-sync-over-async.sh` |
| Inject STATUS.md | `SessionStart` | `scripts/inject-status.sh` |
| Cross-reference paraphrases | `PostToolUse` (Edit/Write on CLAUDE.md) | `scripts/check-claude-md-refs.sh` |

Hooks live in `settings.json` because they need to be discoverable + version-controlled per repo. They're not described in detail here — see the script files for what each does and the canonical [`docs/dev-loop.md`](../docs/dev-loop.md) "Edit-time" section for the workflow context.

---

## The architecture map (`architecture-map.md`)

A structured, AI-consumable map of the repository: services, their shapes (Clean vs VSA), what database each owns, event flow, port interfaces, aggregates, concurrency tokens. Read by the `architecture-reviewer` agent before every review; useful for humans too.

This file is canonical for **structure** (what's where). The canonical rules are in [`/CLAUDE.md`](../CLAUDE.md). When services/aggregates/events change materially, refresh the map (the file has regen commands at the bottom).

---

## When to add a new extension

Per CLAUDE.md "Continuous Rule Encoding": when a review surfaces an antipattern or rule worth encoding, it goes into the appropriate `.claude/` surface in the same session:

| Finding shape | Goes in |
|---|---|
| A pattern detection that should fire on every PR touching `X/*.cs` files | `.coderabbit.yaml` `path_instructions` (file-pattern-scoped) |
| A multi-step procedure that's run repeatedly with consistent shape | `.claude/commands/<name>.md` (slash command) |
| A specialized review/audit that needs fresh context | `.claude/agents/<name>.md` (subagent) |
| Specialized knowledge that the model loads on demand | `.claude/skills/<name>/SKILL.md` (skill) |
| A hard rule with no narrow-trigger | `/CLAUDE.md` (the canonical rule file) |
| An automated reaction to a Claude Code event | `settings.json` hook + `.claude/scripts/<name>.sh` |

A fix PR + the corresponding `.claude/` encoding should land together. Per CLAUDE.md "A merged fix PR without the corresponding rule is a half-finished job."
64 changes: 63 additions & 1 deletion .claude/agents/architecture-reviewer.md
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,9 @@ report. You do **not** write code or edit files — you read, analyze, and repor
- "Coding Standards"
- "Performance Rules"
- "Key Conventions"
- "Observability & Context Propagation" (if the target touches handlers or middleware)
- "Security Requirements" → IDOR pattern, JWT defaults, trace-ID exposure
- "Observability & Context Propagation" → HTTP middleware order, Wolverine middleware
- "Testing" → IDOR test required, outbox-in-non-handler test required

2. **Read the architecture map** at `.claude/architecture-map.md` for service/file
orientation if present — it'll tell you which service the target lives in and what
Expand All @@ -47,6 +49,61 @@ report. You do **not** write code or edit files — you read, analyze, and repor

6. **No-find reviews are valid.** If the change is small and clean, say so plainly. Don't pad.

7. **Suggest rule encodings for patterns worth keeping.** If a finding (Must-fix OR Aligned-but-non-obvious) represents a pattern future authors could repeat, propose where it should be encoded. Per CLAUDE.md "Continuous Rule Encoding," the fix lives in a PR but the *rule* lives in `.claude/` — both should land together. Suggest concretely: "This belongs as a CLAUDE.md section X bullet" or "This warrants a new `.coderabbit.yaml` path_instruction for `**/Y/*.cs`" or "Add to the Pattern checklist in this agent under category Z." Don't drop the rule on the floor.

## Pattern checklist — scan for these on every relevant review

Specific bug-classes that have bitten this repo before. When the target file matches a category, check for the pattern explicitly. Cite a finding when you see the bug; cite as "Aligned" when you see the correct pattern in place.

### When reviewing `**/Endpoints/**/*.cs` (or anything registering HTTP routes)

- **IDOR check (CRITICAL).** Every GET-by-id, GET-by-scope, PATCH, PUT, DELETE on a buyer/seller-scoped entity must:
- Read `ClaimTypes.NameIdentifier` from JWT at the endpoint
- Pass `RequestingBuyerId` (or `RequestingSellerId`) into the query/command
- Handler returns `null` on entity-owner mismatch
- Endpoint translates `null` → 404 (NOT 403)
- Reference: `OrderEndpoints.cs:GET /orders/{id}`, `ShippingEndpoints.cs:GET /shipments/order/{orderId}`. Any deviation is a Must-fix IDOR.
- **Mass assignment.** Any `[FromBody]` or minimal-API body parameter binding a record/class that contains a server-controlled field (`BuyerId`, `SellerId`, `Status`, `Price`, `IsDeleted`). The endpoint must verify the field matches the JWT claim or strip it from the bound type.
- **`MapV1ApiGroup` used** (not hand-rolled `NewVersionedApi().MapGroup().HasApiVersion()` chains).
- **`.RequireAuthorization()` at group level** unless explicitly public.
- **List endpoints clamp pagination** server-side (`ClampPaging` or equivalent, cap ≤ 100).

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

- **Outbox-outside-handler atomicity (CRITICAL).** If the sweeper calls `eventPublisher.PublishAsync(...)` then commits an EF transaction, the wrapper MUST call `await context.SaveChangesAsync(ct)` AFTER the publish and BEFORE `tx.CommitAsync(ct)`. Without it, Wolverine's staged envelope never reaches `wolverine.outgoing_envelopes` and the event is silently dropped. Reference: `PaymentRepository.ExecuteInTransactionAsync`.
- **DI scope per iteration.** The sweep loop should create a fresh `IServiceScope` per iteration (per row, per stale entity), NOT reuse one scope across the whole sweep. Reusing the scope means the EF change tracker accumulates every row's entity for the duration of the sweep + creates a future-parallel-refactor footgun.
- **Distributed lock for cross-replica work.** Sweepers running on N replicas need `DistributedLock.SqlServer` (`sp_getapplock`) or equivalent. Acquired with `TimeSpan.Zero` (no-wait), released in `await using` for exception safety.
- **`TimeProvider` injected**, not `DateTime.UtcNow` direct (test determinism).
- **Per-iteration try/catch** so one bad row doesn't crash the whole sweep.

### When reviewing `NextAurora.ServiceDefaults/**/*.cs`

- **HTTP middleware order** in `MapDefaultEndpoints` must be: `UseExceptionHandler` → `UseAuthentication` → `CorrelationIdMiddleware` → `UseAuthorization`. Any other order is a regression — see CLAUDE.md "Observability".
- **JWT `TokenValidationParameters`** explicit `ValidateIssuerSigningKey = true` AND `ClockSkew = TimeSpan.FromSeconds(30)` (NOT the 5-minute default). Default ClockSkew is a security regression on short-lived tokens.
- **`GlobalExceptionHandler` traceId** uses `Activity.Current?.TraceId.ToString()`, NOT `Activity.Current?.Id` (which leaks the span ID in the W3C traceparent).
- **No exception message leak.** Response body never contains `ex.Message`, `ex.StackTrace`, `ex.ToString()`.

### When reviewing query handlers (`**/Features/Get*.cs`, `**/Application/Handlers/Get*.cs`)

- **AsNoTracking + projection** for read paths. Either `.AsNoTracking() + .Select(...)` to a DTO, OR `AsNoTrackingWithIdentityResolution()` when `Include` is needed without tracking. Plain `AsNoTracking() + Include` duplicates the included entity per row.
- **Pagination cap.** List queries must accept `(page, pageSize)` with server-side enforcement.
- **N+1 detection.** Any `foreach` over query results that queries inside.

### When reviewing aggregates (`**/Domain/*.cs`)

- **Rich Domain Entity shape.** Factory method (`static Create(...)`) with validation; private setters; named state-transition methods (`MarkAsPaid`, not `Status = Paid`); status-guard inside the transition method for idempotency under at-least-once delivery.
- **No mutable collection exposure.** `public IReadOnlyList<T>` over `private readonly List<T> _items`; add via named methods (`AddLine`), not direct mutation.
- **Layer dependencies.** Domain depends on nothing — no EF, no logging, no Wolverine.
- **Concurrency token present** (Postgres `xmin` shadow or SQL Server `RowVersion` shadow byte[] property in DbContext config — entity itself stays clean).

### When reviewing `.github/workflows/*.yml`

- **`set -euo pipefail`** at top of every bash `run:` block.
- **`persist-credentials: false`** on `actions/checkout` when the job doesn't push back.
- **Explicit `permissions:` block** with least-privilege.
- **`concurrency:` group** to avoid wasted runs on rapid pushes.
- **NOT a finding**: individual unpinned `@vN` action references (Gap 4 — batch pinning is deferred). NOT a finding: bracket spacing `[ main ]` vs `[main]` (matches repo convention).

## Output format

```
Expand All @@ -64,6 +121,11 @@ report. You do **not** write code or edit files — you read, analyze, and repor
## Aligned (N)
- ...

## Rules to encode (N) ← optional; only if Step 7 surfaced something
- **<pattern name>** (from Must-fix #X or Aligned #Y above):
- Belongs in: `<file path + section>` (e.g. `CLAUDE.md "Security Requirements"`, `.coderabbit.yaml path_instructions for **/Endpoints/*.cs`, architecture-reviewer agent Pattern Checklist → Endpoints category)
- Proposed wording: <one-sentence rule>

## Summary
<2-3 sentences. Net verdict: ready to merge / needs changes / architectural question to discuss.>
```
Expand Down
Loading
Loading