--- name: architecture-review description: '[Code Quality] Use when reviewing architecture compliance for layers, messaging, service boundaries, CQRS, repos, entity events, and data/consistency/tenancy boundaries. Universal architecture laws, coupling taxonomy and the anti-pattern catalog live in `.claude/docs/architecture-knowledge.md` (project docs always outrank it).' --- > Codex compatibility note: > > - Invoke repository skills with `$skill-name` in Codex; this mirrored copy rewrites legacy Claude `/skill-name` references. > - Task tracker mandate: BEFORE executing any workflow or skill step, create/update task tracking for all steps and keep it synchronized as progress changes. > - User-question prompts mean to ask the user directly in Codex. > - Ignore Claude-specific mode-switch instructions when they appear. > - Strict execution contract: when a user explicitly invokes a skill, execute that skill protocol as written. > - Subagent authorization: when a skill is user-invoked or AI-detected and its protocol requires subagents, that skill activation authorizes use of the required `spawn_agent` subagent(s) for that task. > - Do not skip, reorder, or merge protocol steps unless the user explicitly approves the deviation first. > - For workflow skills, execute each listed child-skill step explicitly and report step-by-step evidence. > - If a required step/tool cannot run in this environment, stop and ask the user before adapting. ## Codex Project-Reference Loading (No Hooks) Codex uses static project-reference loading instead of runtime-injected project docs. When coding, planning, debugging, testing, or reviewing, open project docs explicitly using this routing. **Always read:** - `docs/project-config.json` (project-specific paths, commands, modules, and workflow/test settings) - `docs/project-reference/docs-index-reference.md` (routes to the full `docs/project-reference/*` catalog) - `docs/project-reference/lessons.md` (always-on guardrails and anti-patterns) **Missing/stale context route:** If `docs/project-config.json`, the docs index, `lessons.md`, `CLAUDE.md`, `AGENTS.md`, or any task-required reference doc is missing or stale, auto-run `$project-init` or the narrow setup route (`$project-config`, `$docs-init`, `$scan-all`, `$scan --target=`, `$claude-md-init`) before ordinary project-specific work. If Codex mirrors or `AGENTS.md` are missing/stale, ask the user to run `$sync-codex`; do not auto-run it. **Situation-based docs:** - Project structure/architecture/tech-stack/deployment/setup (any layer — backend, frontend, or infra): `project-structure-reference.md` - Backend/CQRS/API/domain/entity changes: `backend-patterns-reference.md`, `domain-entities-reference.md` - Frontend/UI/styling/design-system: `frontend-patterns-reference.md`, `scss-styling-guide.md`, `design-system/README.md` - Spec authoring, `docs/specs/` pathing, or TC format: `feature-spec-reference.md`, `spec-system-reference.md`, `spec-principles.md` - Behavior/public-contract changes or spec-test-code sync: `workflow-spec-test-code-cycle-reference.md` plus the spec docs above - Derived spec indexes/ERDs/reimplementation guides: `spec-system-reference.md` and source Feature Specs under `docs/specs/` - Integration test implementation/review: `integration-test-reference.md` - E2E test implementation/review: `e2e-test-reference.md` - Code review/audit work: `code-review-rules.md` plus domain docs above based on changed files Do not read all docs blindly. Start from `docs-index-reference.md`, then open only relevant files for the task. > **[BLOCKING]** Execute skill steps in declared order. NEVER skip, reorder, or merge steps without explicit user approval. > **[BLOCKING]** Before each step or sub-skill call, update task tracking: set `in_progress` when step starts, set `completed` when step ends. > **[BLOCKING]** Every completed/skipped step MUST include brief evidence or explicit skip reason. > **[BLOCKING]** If Task tools are unavailable, create and maintain an equivalent step-by-step plan tracker with the same status transitions. ## Quick Summary **Goal:** Ensure changes preserve architecture boundaries, ownership, message flow, and generated artifact integrity before handoff — validating changed code against layers, service boundaries, message flow, CQRS, repositories, entity events, frontend architecture, generated artifacts, recorded architecture decisions (ADRs), and quality tooling. **Summary:** - **Purpose:** validate a changeset against architecture rules the project records in its OWN reference docs; classify every finding PASS/WARN/BLOCKED with `file:line` proof; self-validate before handoff — one reviewer in the `workflow-review-changes` pipeline. - **Main phases — run in order:** Phase 0 load architecture rules → Phase 1 determine scope → Phase 2 blast radius (if `graph.db`) → Phase 3 architecture review (13 categories) → Phase 4 finalize compliance report → Phase 5 `$why-review` self-validation gate → Next Steps ask the user directly. - **The 13 Phase-3 categories — review EVERY applicable one, serially:** 0 quality-tooling baseline · 1 clean-architecture layers · 2 message-bus patterns · 3 CQRS compliance · 4 repository patterns · 5 service-pattern era (legacy vs modern) · 6 entity event handlers · 7 service boundaries · 8 frontend architecture (frontend files only) · 9 ADR / recorded-decision conformance · 10 spec-loop discipline (property-TC + dual-feedback) · 11 scalability & coupling regression (diff-scoped; BLOCKED/WARN) · **12 data, consistency & tenancy boundaries (dual-write, idempotency, isolation level + write skew, unfenced lock, breaking migration, tenant isolation, dataset writer ownership; BLOCKED/WARN)**. Per category: `Think:` derivation → doc rule → source evidence → `file:line` proof + grep 3+ counterexamples → verdict. NEVER scan categories in parallel; codebase convention wins over a suspected violation. — why: skipping a category silently drops the violation class it uniquely covers. - **Workload-first scalability gate:** before judging a scale technique, prove read/write ratio · sustained/peak load · query shapes · data growth · burst/hot-key skew · geography · latency/consistency budgets; then check the reversible ladder (measure/tune → vertical and/or stateless horizontal from headroom + availability → read/write tactics → partition/shard LAST). Missing evidence means INFO/route, never a scale violation. — why: architecture review must catch regressions without penalizing a lean system for scale it does not have. - Phase 0 is non-negotiable and first: load the project architecture docs (`backend-patterns-reference.md`, `project-structure-reference.md`, `frontend-patterns-reference.md`, `code-review-rules.md`) — every rule and base-class/symbol name comes from those docs, NEVER general knowledge; the framework names in Categories 2–8 are illustrative only. - **Universal reasoning comes from `.claude/docs/architecture-knowledge.md`** (coupling taxonomy + four coupling dimensions, distributed-monolith signature, module-design principles §4, isolation levels + coordination primitives §8-§9, ~100-entry anti-pattern catalog, symptom→root-cause triage, judgment checklists §20) — use it to RECOGNIZE a defect class, then prove it with `file:line`. **The project's own reference docs and accepted ADRs OUTRANK that catalog on every conflict — NEVER flag a deviation from the catalog as a project violation.** An anti-pattern match is a HYPOTHESIS until evidence plus the damaged quality attribute are both named. — why: pattern-shape matching without project grounding is exactly the guess-as-fact failure this skill exists to prevent. - Stay in lane: deep-review only what this skill OWNS (layers, messaging/CQRS/repos/service boundaries, entity events, frontend architecture, quality tooling, generated artifacts, ADRs); record a one-line `→ route to {sibling}` pointer for security/performance/DDD/UI/test findings instead of expanding them. — why: duplicated findings across reviewers inflate severity counts and bury issues each reviewer uniquely owns. - Read-only until validated: **self-audit every draft finding against the 11 thinking red flags (`architecture-knowledge.md` §20.3) FIRST** — a finding whose sacrifice/trade-off you cannot name, or that rests on "best practice", is demoted or deleted, never reworded — then run the Phase 5 `$why-review` self-validation gate before handoff; fixes happen only in the validated fix loop, and every fix restarts a full review from Phase 0. Write findings to `plans/reports/arch-review-{date}-{slug}.md`. **Default scope:** All uncommitted changes (staged + unstaged). Override: specify files, directories, services, or full codebase. > **MANDATORY MUST ATTENTION** Plan tasks to READ architecture docs BEFORE reviewing: > > 1. `docs/project-reference/backend-patterns-reference.md` — CQRS, messaging, repos, validation, entity events, layer rules **(READ FIRST — primary rules source)** > 2. `docs/project-reference/project-structure-reference.md` — service map, layer structure, DB ownership > 3. `docs/project-reference/frontend-patterns-reference.md` — component hierarchy, store, API patterns **(frontend files only)** > 4. `docs/project-reference/code-review-rules.md` — anti-patterns, conventions > > Not found → search: "architecture documentation", "service patterns", "messaging patterns". Rules come from docs — NOT general knowledge. **Workflow:** 1. **Phase 0: Load Architecture Rules** — Read project architecture docs (rules come from docs, NEVER general knowledge) 2. **Phase 1: Determine Scope** — Changed files (default) or user-specified scope 3. **Phase 2: Blast Radius** — Run `$graph-blast-radius` if `graph.db` exists 4. **Phase 3: Architecture Review** — Check each file serially against all 13 applicable categories (0 tooling → 12 data, consistency & tenancy) 5. **Phase 4: Finalize** — Generate compliance report with PASS/BLOCKED/WARN verdicts 6. **Phase 5: Why-Review Self-Validation Gate** — Adversarially validate own findings via `$why-review` before handoff (MANDATORY when any finding exists) 7. **Next Steps** — ask the user directly: `$code-simplifier` / `$code-review` / skip **Key Rules (top 3 critical first):** - MUST ATTENTION read project architecture docs in Phase 0 BEFORE reviewing — rules come from docs, NEVER general knowledge. - Every violation needs `file:line` proof + grep 3+ counterexamples before flagging — NEVER speculate. - MUST ATTENTION review one category at a time: doc rule → source evidence → verdict — NEVER scan categories simultaneously. - Write findings to `plans/reports/arch-review-{date}-{slug}.md`. - BLOCKED = must fix before merge | WARN = review and decide | PASS = compliant. - Review is read-only until `$why-review --validate-findings` confirms findings; fixes happen only in the validated fix loop or downstream plan/feature-implement, and every fix restarts a full architecture review from Phase 0 with a fresh task breakdown. ## Your Mission $ARGUMENTS ## First Principle — Easy to Change > **Success metric: future change cost.** DRY, SRP, abstraction, patterns, naming, layering, tests exist to make next change cheaper. Before applying any rule, ask: **does this lower or raise future change cost?** - Reject "best practices" raising cost: premature abstraction, speculative generality, leaky indirection, ceremony without payoff. — why: cost added now with no payoff is debt, not quality. - Name real enemies: **coupling, hidden state, duplicated knowledge, unclear intent, irreversible decisions exposed too early**. - Prefer simple reversible design over sophisticated rigid design. — why: reversible decisions cost less to undo when wrong. - If downstream rule raises change cost, this principle wins. --- ## Quality Tooling Principle — Tech-Stack Adaptive > Architecture review includes automated quality guardrails. Without stack-appropriate linting, formatting, type checks, static analysis, dependency/security-review scanning, CI enforcement, defects depend on reviewer memory. Evaluate detected stacks, not fixed tool list: - Detect stacks from project-reference docs, manifests, lock files, build files, CI before recommending tools. - Per production stack, verify formatter/style config, linter/code analyzer, compiler/type-check strictness, dependency/vulnerability scanning, tests/coverage, CI/pre-commit enforcement. - Prefer official or ecosystem-standard tooling; local docs absent/stale → check current official docs before recommending setup. - MUST ATTENTION recommend enforceable best practice only: installed but unwired tool = WARN; production source with no relevant automated quality gate = BLOCKED. - Identify missing capability first; map to local equivalent before prescribing new tooling. — why: prescribing a tool that duplicates an existing gate adds noise, not coverage. --- ## Review Mindset (NON-NEGOTIABLE) Skeptical. Every claim needs traced proof, confidence >80%. - NEVER flag violations without reading actual code + tracing dependency — READ the code, trace the import chain, then flag. - Every finding MUST include `file:line` evidence. - Before flagging pattern violation: grep 3+ existing examples — codebase convention wins. - Question: "Actually a violation, or an established exception?" ## Ownership & Handoff (own vs delegate) This skill = one reviewer in a multi-reviewer pipeline — `workflow-review-changes` runs it beside the siblings below. Review ONLY what this skill owns; route the rest so findings are not double-reported across reviewers. | This skill OWNS (deep-review here) | Delegate to sibling (one-line pointer only — do NOT deep-review) | | ----------------------------------------------------------------------------------------------------------------------- | -------------------------------------------------------------------------------------------------------------------- | | Layer boundaries, dependency direction, business-logic placement | — | | Messaging patterns, CQRS structure, repository patterns, service-pattern era, entity event handlers, service boundaries | — | | Frontend ARCHITECTURE (base classes, store/effect, API-service base, subscription teardown, CSS-class presence) | Visual/SCSS/responsive/z-index quality → `ui-review` | | Quality-tooling baseline, generated-artifact integrity, ADR / recorded-decision conformance | — | | Architecture-level auth PLACEMENT (a gate exists at the boundary) | OWASP, secrets, dependency/supply-chain, authz-matrix depth → `security-review` | | Structural soundness of a hot path (no obvious N+1 introduced by the diff) | Query plans, indexing depth, latency/throughput budgets → `performance-review` | | — | Domain entity / value-object DDD design quality → `domain-entities-review` | | — | Integration-test assertion quality, coverage, traceability → `integration-test-review` | | — | Runtime production-readiness of service/API changes (observability wiring, rollback) → `production-readiness-review` | When a finding clearly belongs to a sibling, record one-line `→ route to {skill}` pointer and move on — NEVER expand it. — why: duplicated findings across reviewers inflate severity counts and bury issues each reviewer uniquely owns. ## Phase 0: Load Architecture Rules (MANDATORY FIRST) > **MUST ATTENTION:** Read project docs BEFORE reviewing. Rules come from docs, NEVER general knowledge. - read `docs/project-reference/backend-patterns-reference.md` — extract messaging naming, layer rules, CQRS patterns, repo rules, entity event handler patterns, validation patterns - read `docs/project-reference/project-structure-reference.md` — extract service map, layer structure, DB ownership - frontend files in scope → read `docs/project-reference/frontend-patterns-reference.md` - read `docs/project-reference/code-review-rules.md` — extract anti-patterns + review rules directly **Universal reasoning layer (secondary, never authoritative):** consult `.claude/docs/architecture-knowledge.md` for the coupling taxonomy + four coupling dimensions, the distributed-monolith detection signature, the ~100-entry anti-pattern catalog, and the symptom→root-cause triage matrix. Use it to RECOGNIZE a defect class the project docs do not name explicitly. **Honor its provenance markers in §3/§8/§9/§10:** a row or section banner marked `— VERIFY` is an UNVERIFIED assertion — it may seed a hypothesis, but NEVER quote it as the authority for a finding; confirm against the named source (or the project's own docs) first. > **MUST ATTENTION — precedence is absolute.** Project reference docs and accepted ADRs > the knowledge catalog > general knowledge. **NEVER report a deviation from the catalog as a project violation**, and NEVER let a catalog entry override an established, grepped codebase convention. Every catalog-derived observation is a HYPOTHESIS until you have BOTH `file:line`/config/topology evidence AND the named quality attribute it damages — otherwise record it as INFO or drop it. — why: universal patterns applied as project rules generate confident false positives, the most expensive output this skill can produce. ## Phase 1: Determine Scope **Default (no override):** Review all uncommitted changes. ```bash git status # List changed files git diff # Staged + unstaged changes git diff --cached # Staged only ``` - Collect file list to review. - Categorize: backend (.cs), frontend (.ts/.html), config, docs, other. - Filter to architecture-relevant files (skip pure docs, configs, tests unless architecture-relevant). ## Phase 2: Blast Radius (if graph.db exists) - `.code-graph/graph.db` exists → call `$graph-blast-radius` skill. - Record: impacted file count, cross-service impact, risk level. - Prioritize review by highest-impact files first. - Graph unavailable → note "Graph not available — skipping blast radius" and proceed. Per changed file with downstream impact: ```bash python .claude/scripts/code_graph trace --direction downstream --json ``` Flag MESSAGE_BUS consumers or event handlers impacted by changes. ## Phase 3: Architecture Review Create report: `plans/reports/arch-review-{date}-{slug}.md` Per file in scope, evaluate against ALL applicable categories. Skip categories not applicable to file type. MUST ATTENTION review serially. Per applicable category: read docs/source evidence → derive risk with `Think:` → grep 3+ examples/counterexamples → record PASS/WARN/BLOCKED. NEVER scan categories simultaneously —