Grill my architecture
Skill NasserAlbusaidi/ship-gate/skills/grill-my-architecture
Brutal staff-engineer review of system architecture — service/module boundaries, data ownership, communication patterns, failure modes, scaling, security trust boundaries, evolvability, cognitive load, cost. Accepts an architecture doc, a set of ADRs, a system design write-up, a diagram pack, a verbal sketch, OR the codebase itself (the skill reconstructs the de-facto architecture from the code when no doc exists). Distinct from `grill-my-code` (file/function craft), `grill-my-backend` (server-side surface within one service), `grill-my-plan` (a specific planned change before code is written), `improve-codebase-architecture` (constructive refactoring opportunities), and `plan-eng-review` / `plan-ceo-review` (constructive walkthroughs). Use when the unit of critique is the *system shape*, not a file or a feature. Triggers on "grill my architecture", "grill the system design", "grill these ADRs", "is this architecture going to survive", "tear apart this design", "review my system design brutally", "stress-test the architecture".From its SKILL.md
npx -y skills add NasserAlbusaidi/ship-gate --skill grill-my-architectureAssembled from the repository path, not quoted from the project. Check it against their README if it does not work.
2 things to look at
- 19 days oldThe repository was created 19 days ago. New is not bad, but a brand new repository carrying a familiar-sounding name is the shape a typosquat arrives in, and there has been no time for anyone else to find a problem with it.
- 0 stars0 stars. Stars are a popularity signal and not a quality one, but at this level it is likely that nobody has read this closely except its author, and you would be relying on your own review.
SKILL.md
39.4 KB, ~8.6k tokens by cl100k_base, as published. Nobody here has run it
Setup — read the shared protocol first
Before doing anything else, read these two files in full:
${CLAUDE_PLUGIN_ROOT}/skills/_grill-shared/tone.md${CLAUDE_PLUGIN_ROOT}/skills/_grill-shared/protocol.md
They contain the tone, hard rules, catalog protocol, the interactive-vs-report fork, and the punch-list/report formats. The rest of this file specifies the architecture-review flavor.
If either Read fails (file not found or permission denied), stop and ask the user where the shared grill files live (they ship in _grill-shared/ beside this skill). Do not improvise the protocol from memory — the whole point of the shared file is that it stays consistent across the family.
This skill stands on its own — it does not layer on grill-my-code the way the backend/frontend skills do. Code-level principles are not what bite at the architecture layer. When you are grilling architecture, reach for the principle library in this file. If a finding is genuinely a code-craft issue (a single function is buggy), that's the wrong skill — tell the user to use grill-my-code or grill-my-backend for that surface and stay focused on the system shape.
What this skill grills, exactly
The system shape:
- Where component / service / module boundaries are drawn and why
- Who owns what data, who is allowed to write it, who reads it through what path
- How components communicate (sync/async, request/response/event, push/pull, broadcast/point-to-point)
- Where state lives (and where it pretends to live but actually doesn't)
- Where trust zones begin and end
- What happens when each component fails, individually and in combination
- Whether the system can scale along the axes the business will demand
- Whether on-call can understand the system at 3am with no warm-up
- Whether the system can be incrementally replaced, or whether it's locked in
- Whether the team holds enough of it in their heads to operate it
NOT what this skill grills:
- A single function's correctness — use
grill-my-code - A single endpoint's handler — use
grill-my-backend - A single component's accessibility — use
grill-my-frontend - A specific planned change — use
grill-my-plan - "How do I make this nicer" — use
improve-codebase-architecture(constructive)
Step 1 — Pick the target
Before reading anything, ask the user exactly this:
What am I grilling?
- An architecture doc / system design write-up / set of ADRs — give me the path(s)
- A diagram pack (C4 / Mermaid / sequence diagrams) — give me the directory; I'll need text descriptions too if it's image-only
- The codebase itself — I'll reconstruct the de-facto architecture from the code
- A verbal sketch — describe the system shape in as much depth as you'd put in a design doc
- Some combination — name what you have and what's missing
And: what's the scope? "The whole system" / "the data layer" / "the service boundaries" / "the failure model" / "the auth/trust boundaries" / "evolution & migration path" / "operational story"?
And: what stage is this at? "Greenfield design (nothing built yet)" / "early build, can still change anything" / "deployed but small" / "deployed at scale, change cost is real" / "legacy, considering rewrite". Severity calibration depends on this.
Wait for the answer. Do not guess.
If they pick (1) — docs/ADRs: read every doc in scope, and any doc they link to that's load-bearing. ADRs reference parent ADRs; the parent is required reading.
If they pick (2) — diagrams: read the descriptions and any accompanying text. If the only artifact is image-only, ask for text descriptions OR for the codebase to ground-truth against. Diagrams alone are insufficient — they describe intent, not behaviour.
If they pick (3) — code only: this is the hardest mode and the most valuable. Read enough of the codebase to build the system map (Step 1.5). You will need to read service entry points, route registries, ORM models, migration history, queue/broker config, infra-as-code (Terraform / Pulumi / CDK / k8s manifests), dependency graphs, and at least two end-to-end flows. State the time/breadth budget you're spending up front — "I'm going to spend ~15 minutes mapping; tell me to stop earlier or push deeper" — so the user can steer.
If they pick (4) — verbal sketch: treat the conversation as the input. Same rule as grill-my-plan — restate the sketch back in Step 1.5 before grilling. Verbal-sketch give-up clause: if after two restatement attempts the user still says "no, you've got it wrong," tell them: "I don't have a clean enough read of the architecture to grill it. Either write it down in 5–10 bullets (components, communication, data ownership) and paste it back, or pick a different input mode." Don't keep grasping.
If they pick (5) — combination: name what you're using and what you'll have to reconstruct vs trust.
Step 1.5 — Build the system map (hard gate)
An architecture review without an explicit map is vibe-checking. This step is the single highest-leverage one in the skill. Most architecture critique fails because the reviewer grilled against a misread system shape.
Build a written map. Six elements, every one named:
- Components. What units exist? Services, modules, packages, lambdas, jobs, scheduled tasks, external systems. One line per component with one phrase of purpose.
- Data ownership. For each significant data category (users, orders, events, sessions, audit log, billing, content, search index, …), name the single owner. If there are two writers, that's already a finding — note it, but capture the de-facto state in the map.
- Communication. For each edge between components: sync vs async, request/response vs event vs queue vs polling, push vs pull, point-to-point vs broadcast. One line per edge.
- State. Where does durable state live (each datastore, each queue, each cache treated as state, each external system that holds state)? Where does implicit state live that the code pretends is stateless (sticky sessions, in-memory caches, local file writes, cron-schedule-based truth)?
- Trust boundaries. Where do trust zones begin and end? What crosses each boundary, and with what authentication / authorization? "Internet → edge → app → DB" is too coarse — name actual zones (public web, authenticated user session, internal service mesh, privileged admin plane, third-party webhook ingress, …) and the auth at each crossing.
- External dependencies. Vendors, SaaS, payment, identity providers, CDNs, observability stacks. Note which are in the critical path of user-facing requests.
Then render the topology as a diagram alongside the prose. The prose carries the precision (sync/async, owner-of-record, auth method, drift annotations); the diagram carries the shape (what touches what, what's inside what trust zone). Each does what the other can't — ship both, not either.
Pick the diagram format based on where the human reads it:
- Interactive grilling (Phase C / Step 1.5 surfacing): ASCII text diagram. You are talking into a terminal. Mermaid renders as code-fenced source that the user has to copy elsewhere to view — that's friction. Use ASCII trust-zone headers, arrows (
──▶,══▶,···▶), and text annotations. Trust zones as square-bracket headers ([ ZONE NAME — qualifier ]), components as plain names, indentation for hierarchy. - Report mode (Phase D, file write): Mermaid flowchart. The report is read in GitHub / Notion / Obsidian / an IDE preview where Mermaid renders as a real diagram. Use
flowchartsyntax withsubgraphper trust zone.
Rules for the diagram (both formats):
- Trust zones are the top-level grouping. Each trust zone from element 5 becomes a containing box (Mermaid
subgraph) or a bracketed header (ASCII[ ZONE NAME ]). The boundary crossings between zones are where the security findings live; making them visually obvious is the point. - Every edge gets a one-phrase label describing the communication kind:
sync HTTP,async event,stdio subprocess,cross-thread callback,push, no authn,fire-and-forget,HEAD resolved by uvx. Bare arrows are decorative — don't ship them. - Distinguish dangerous boundary crossings visually. Mermaid: dotted arrows (
-.->) for unauthenticated / untrusted edges, thick arrows (==>) for high-privilege crossings. ASCII:···▶for dotted / untrusted,══▶for thick / high-privilege,──▶for normal. The reader's eye should land on the trust-boundary findings before they finish reading. - High-privilege crossings include: anything entering a
bypassPermissions-equivalent zone, anything fetching arbitrary code from a remote source, anything carrying long-lived credentials across a trust boundary, anything where the receiver runs in the caller's environment. - Components, not files. One node per component (service, module, subprocess, external system) — not one node per source file. The diagram is at the architecture layer.
- Owner-of-record annotations on the data nodes. If a component owns durable state, say so in its label (Mermaid: in the node text; ASCII: as a
★annotation below the name). If a component appears to own state but actually delegates, say that too (※ doc claims it does, code delegates to Y). The diagram is where doc-vs-code drift in data ownership becomes glaring. - Don't draw the fix. No "current vs proposed" / "before vs after" diagrams. The grill family's rule applies: the user fixes. Drawing the proposed architecture is writing the fix in pictures.
For ASCII diagrams, supply a one-line legend underneath:
Legend:
══▶ thick — high-privilege crossing
───▶ normal edge with auth or trust
···▶ dotted — unauthenticated / unpinned / unsigned
★ load-bearing state ownership
※ doc-vs-code drift
After rendering the diagram, point out what the diagram makes obvious that the prose hid — usually two or three things: a trust zone that turns out to be permeable on entry, a thick high-privilege arrow that crosses a zone boundary, a piece of state that lives outside the component that claims to own it. Three bullets max. This step earns the diagram its tokens — if you can't name what the picture revealed that the prose didn't, you didn't draw a useful diagram.
Skip the diagram only if: the map has fewer than 4 components OR fewer than 4 edges OR the user explicitly asked for prose only. Three boxes and three edges add no signal over the prose.
Then surface the prose map AND the diagram back to the user, in the same message, in this form:
System map (as I understand it):
Components: A (user-facing API), B (auth service), C (jobs worker), D (event bus), E (analytics ETL), …
Data ownership:
- Users, sessions → B
- Orders, line items → A
- Audit log → A (writes), E (reads)
- Search index → C (writes, derived from A)
- Billing → external (Stripe)
Communication:
- A → B: sync HTTP, every authenticated request
- A → D: async event publish on order state change
- C → D: subscribe to order events; writes back to A via sync HTTP
- E → A: nightly pull (read replica)
State:
- Durable: Postgres (A, B), Redis (B sessions, A rate-limit), S3 (uploads), Stripe (billing)
- Implicit: in-memory cache on A for feature flags (TTL 60s, not invalidated on write)
Trust boundaries:
- Public → A: JWT validated at edge
- A → B: shared service token (one secret, rotated quarterly)
- A ↔ DB: connection-string in env, no per-row auth
- Third-party webhook → A: HMAC signature verified
External dependencies in critical path: Stripe (billing call on signup), Auth0 (token validation on every request), CloudFront (assets only — not critical-path).
Did I get this right? Anything wrong, missing, or oversimplified before I start grilling?
Wait for confirmation or correction. If the user corrects you, update the map and re-surface it. Do not start grilling against a wrong map.
If grilling from code only (input mode 3): the map is your reconstruction and may be wrong. Note your low-confidence inferences explicitly — "I think the auth check on the admin endpoints lives in middleware X but I only traced one route end-to-end; verify before I escalate this to a CRITICAL finding."
The hard gate. After the map is confirmed:
- If the map itself reveals 1+ CRITICAL structural issues (two owners for the same data category, no trust boundary at a place that obviously needs one, a component with no clear purpose, a documented async edge that's actually sync in code), surface them in a single message before continuing:
"The map itself surfaces N blocking issues before I even start grilling. Continue to the full review anyway, or pause to discuss these first?
- [CRITICAL] <one-line issue>
- ..."
Wait for the user's choice. Same handling as
grill-my-planStep 1.5.
Step 2 — Catalog, choose mode, grill or report
Now follow protocol.md Phases A → B → (C or D) → E. The map and any issues already surfaced from Step 1.5 are part of the catalog.
Five architecture-flavor adaptations:
-
Severity is dominated by reversibility, not by blast-radius-now. The protocol's tier definitions apply unchanged, but at the architecture layer almost every issue that earns a place in the catalog is HIGH or CRITICAL — because architecture decisions that are easy to reverse aren't really architecture decisions. Calibration: CRITICAL when reversing the decision costs months of work, requires a customer-facing migration, or commits the team to a path it can't walk back without a rewrite. HIGH when reversing costs weeks and one team's full focus, OR when the decision locks in future pain that will compound as load/team-size/scope grows. MEDIUM is for architecture issues that are real but solvable inside a normal sprint with no coordination cost. NITPICK at this layer almost always means "this is a code-review finding pretending to be architecture" — drop it or hand it to
grill-my-code. -
Concrete consequence must name a load scenario AND a horizon. "Won't scale" is mush. "Will become hard to evolve" is mush. Passing: "At ~50k tenants, the per-tenant sharding scheme creates a hot shard holding ~60% of traffic because the top three customers are >40x the median tenant; capacity planning breaks and the choice becomes either re-shard the largest customers out individually (weeks of coordinated migration) or build a per-tenant capacity model the SRE team doesn't have today." Horizon must be anchored to something observable: a roadmap item, a known integration milestone, a contract date, a measurable threshold, a known external dependency event. Reject invented horizons.
-
Web-verification budget is the loosest in the family —
≤10 silent / >10 ask. Override the sub-protocol ceiling. Architecture review cites the most external authorities of any review type (CAP theorem & consensus semantics, database isolation guarantees, vendor SLAs and rate limits, framework lifecycle, queue delivery semantics, RFC details for protocols, cloud-provider service behaviours). Stress-testing architecture without verification produces confident-wrong critique. The sub-protocol's when to verify and what to do with the result rules still apply unchanged — one search per claim, drop or demote when ambiguous. -
Stage-aware severity. Greenfield architecture decisions are cheaper to revisit than deployed-at-scale ones. The same finding (
shared DB across services) is HIGH on a greenfield design (you can still draw the boundary differently) and CRITICAL on a system with 18 months of production data and three teams writing to that DB (the migration cost is now the architecture, not the future). Adjust tier based on the stage answer the user gave in Step 1. -
The "what does this quietly commit us to" rule. Most architecture critique is naming what the design commits the team to that they haven't priced in. When you can't articulate "this commits you to X for Y reason and you'll discover it when Z happens", the issue isn't real architecture criticism — it's taste.
Architecture-review principle library
Use these names. Don't paraphrase them into mush. Architecture review needs sharp principles or it degenerates into adjectives.
Boundaries & decomposition
- Boundary cut along the wrong seam — components split by technical layer (controllers / services / repositories) when they should be split by capability (orders / inventory / pricing), or split by capability when they should be a single module. The wrong seam costs every future change.
- Service boundary not aligned to data ownership — service A is "the truth" for an entity but service B writes to A's tables directly (or via a shared library that bypasses A's API). Boundary is fictional.
- Boundary cuts perpendicular to change frequency — things that always change together live in separate components; things that rarely change together are smushed into one. Every change crosses a seam it shouldn't.
- Microservice for what should be a function call — separate deploy unit, network hop, observability surface, ops overhead — to serve a piece of logic that was always going to be coupled to its caller.
- Monolith for what should be split — one component with two deploy cadences, two scaling profiles, two on-call rotations contending; deploy gates each other's releases.
- Distributed monolith — N separately-deployed services that must all release together because they share a wire format / shared library / data contract that wasn't versioned. Worst of both worlds: ops cost of microservices, coupling of a monolith.
- Shared database across services — two services write to the same tables. Private data is now public; schema changes coordinate across teams; the boundary is a lie.
- Shared cache treated as a contract — services depend on the shape/keys in a Redis they all touch; cache is now an undocumented public API.
- Conway's law violation — architecture doesn't match team boundaries; cross-team coordination tax on every change, or one team owns a service they can't fully understand because half the logic lives in another team's repo.
- Inverse Conway maneuver missing — team layout is fighting the design; the architecture wants teams shaped a way the org isn't.
- Module without a purpose statement — a component exists, but you cannot finish the sentence "this component owns X and only X." That's an integration-by-accident, not a module.
Data architecture & state
- Single source of truth absent — same fact represented in multiple stores with no chosen authority; drift is inevitable and there's no reconciliation path.
- Authority-of-record undefined — for entity X, which system says? If "both" or "depends", that's the finding.
- Data ownership unclear — two services have write access to the same data; conflict resolution is "last writer wins" by accident.
- Shared mutable state across services — global state hiding behind an API; race conditions become a distributed-systems problem.
- Distributed-transaction assumption — code assumes atomicity across two stores / two services; the runtime offers no such guarantee. Either accept eventual consistency explicitly with sagas/compensation, or collapse the boundary.
- Saga / compensation absent for multi-step writes — multi-step write that crosses services has no compensation path; partial failure leaves the system in an undefined state forever.
- CQRS / event-sourcing chosen without need — adopted because "events are scalable", not because read and write models genuinely diverged; complexity bought, no benefit collected.
- CQRS / event-sourcing warranted but absent — read load 100x write load, complex projections needed, audit trail required — and the system still uses a single read-write model that's drowning.
- Storage substrate mismatched to access pattern — OLTP store for OLAP, KV store for relational queries, blob store for queryable data, RDBMS for time-series at scale, document store for highly-relational data. The substrate fights the queries.
- Polyglot persistence by fashion — 5 different stores chosen for resume-driven reasons; each adds ops burden, backup story, monitoring, expertise tax. The justification is "this is the right tool for X" but X is the same shape for all five.
- Schema-evolution strategy absent — no story for "what happens when this shape changes across services"; first cross-service migration discovers the strategy under fire.
- Read model derived without invalidation contract — search index / materialized view / cache derived from primary data; no agreement on freshness, no end-to-end test, no detection of drift.
- Stateless tier with hidden state — service "horizontally scales" but sticky sessions, local file writes, in-memory caches mean a request is bound to a node; horizontal scaling silently doesn't.
Communication & coupling
- Sync where async was right — user-facing request blocks on work that doesn't need to complete in-band (sending an email, building a thumbnail, syncing to analytics). Tail latency owned by the slowest dependency.
- Async where sync was right — request/response interaction dressed up as events because async felt modern; UX now has to invent fake "we got your request" states, debugging requires tracing across queues, the correlation is implicit.
- Shared library that ships data shapes — every consumer must redeploy in lockstep when the shape changes. The shared lib is a hidden distributed monolith.
- Coupling via shared database — see Boundaries.
- Coupling via shared cache — see Boundaries.
- Temporal coupling — component A must run before B with nothing in the system enforcing the order; works in dev because the developer always runs them in order, breaks in prod under concurrency.
- Pull where push was right (or vice versa) — polling where an event would have served (cost + latency), or push notifications where pull would have been simpler and the consumer can tolerate the lag.
- Broadcast vs point-to-point mismatch — event-bus broadcast used for what should be a direct call (every consumer must filter), or direct call used where multiple subscribers are inevitable (each new consumer requires the producer to change).
- RPC over HTTP where event-driven was the right model — call/response chosen because it's familiar; the real semantics are "tell others a thing happened"; producers shouldn't know about consumers.
- Choreography vs orchestration mismatch — complex multi-step workflow done as choreographed events (no one knows the global state) when it needed an orchestrator, or done as a central orchestrator when loose coupling via events would have been simpler.
- Request fan-out without timeout/budget plan — service A makes N parallel downstream calls; no global timeout, no budget for the aggregate; one slow downstream tail-latencies the whole user request.
- Circular service dependency — A → B → C → A; nothing can be deployed independently; reasoning about failure modes becomes intractable.
- Layering violated — lower layer calls higher layer (data layer calls a domain service; domain calls the controller). The "layers" are a lie.
- Wire contract not versioned — message / event / RPC schema with no version field, no compatibility policy. First breaking change becomes a coordinated multi-service deploy.
- Client SDK tightly couples the client to the server — generated SDK that only the server team can change, shipped to many consumers who can't pin or fork.
Failure & resilience
- Single point of failure unnamed — at least one component, when it fails, takes the whole system down. Either the SPOF is acknowledged (and tolerated, with documented RTO) or eliminated. Unacknowledged is the finding.
- Failure mode catalog absent — the design never enumerates "what fails how" for each component. On-call inherits an unbounded blast radius.
- Cascading-failure path open — slow dependency stalls upstream, upstream stalls its upstream, queue backs up, whole system tips. No bulkheading, no shedding, no degradation path.
- Bulkhead absent — one tenant / customer / job class can consume enough of a shared resource (DB connections, thread pool, queue depth) to degrade the system for everyone.
- Backpressure absent — fast producer + slow consumer + unbounded buffer = OOM or queue collapse; nothing in the architecture tells the producer to slow down or shed.
- Retry without budget or backoff — failed call retried with no exponential backoff, no jitter, no per-caller budget; a downstream hiccup becomes a self-inflicted DDoS.
- Circuit breaker absent on flaky external dependency — every call to a known-flaky vendor pays the timeout; no break, no half-open recovery; the vendor's outage is your outage in real time.
- Recovery path manual where automation was warranted — page on a problem the runbook says "run command X"; that command should be a hook, not a page. Or alternatively, automated recovery on a class of failure where a human should be the gate (the script "fixes" by deleting suspicious data).
- State-after-partial-failure undefined — multi-step process; step 3 of 5 dies; system state is "depends"; recovery requires forensic investigation per incident.
- Disaster recovery never rehearsed — DR plan exists on paper; nobody has restored from backup in a year; first real failure discovers the plan doesn't work.
- Quorum / consensus assumed where reality is eventual consistency — code reads-its-own-write through a replica, assumes a write is visible to the next request; works most of the time, fails in a way that's invisible until it's not.
- Split-brain not prevented in leader-election — two leaders can exist transiently; writes diverge; reconciliation undefined.
- Idempotency absent on retry surface — at-least-once delivery semantics + non-idempotent handler = duplicate side effects (charges, emails, notifications) on every retry.
Scaling & capacity
- Scaling axis confused — vertical when the bottleneck is parallelizable, horizontal when the bottleneck is single-node state. Wrong axis means scaling spend buys nothing.
- Capacity headroom undefined — "how much load before X breaks?" has no answer. First incident is a load test.
- Hot key / hot partition risk — sharding scheme has predictable hot keys (sharding by tenant when 3 tenants are 100x the others; sharding by date when today's writes dominate; sharding by hash of a low-cardinality key).
- Read/write ratio ignored in storage choice — storage tuned for write-heavy access but workload is 99% read, or vice versa. Cost and latency both wrong.
- Cold start ignored in serverless choice — function chosen for elastic scale, called from latency-sensitive critical path, cold-start tail latency unacceptable; the architecture choice and the latency budget contradict.
- Spiky workload not buffered — request rate is volatile, no queue or shock absorber between edge and worker; scaling lags spikes; users see errors at every peak.
- 10x growth scenario not considered — design holds at current scale; nothing in the system extends gracefully; 10x users requires architectural surgery, not config.
- 100x cost at 10x users — per-user cost grows superlinearly (per-user S3 bucket, per-user DB schema, per-user lambda); the architecture is financially unsustainable at the next milestone.
- Tenant scale not modeled — multi-tenant system without an explicit answer to "what does the worst-case tenant cost us in storage / compute / blast radius?". The biggest tenant will define the architecture; better to know which constraint binds first.
Observability & operability
- Trace propagation broken across boundaries — async hop drops trace context; cross-service correlation requires reading timestamps and guessing.
- Logs siloed per service with no correlation ID — debugging a single user request requires manual log-stitching; on-call wastes hours per incident.
- Metrics measure the wrong thing — counters that increment on the success path while the failure mode is "function never called"; dashboards stay green during outages. Pick a framework (RED / USE / golden signals) and apply it consistently.
- SLI absent — "what does working mean for this system?" has no answer; you cannot decide whether you're up or down except by gut.
- SLO absent — what's the error budget? Who owns it? What happens when it's blown? Without an SLO, prioritization between feature work and reliability work is political not technical.
- Alert fires on cause when symptom was right (or vice versa) — alerting on "DB CPU > 80%" instead of "p99 latency on the user-facing endpoint > 500ms"; on-call paged for a thing that doesn't matter, or not paged for a thing that does.
- Runbook absent or stale — page fires, runbook says "see Marcus"; Marcus left; on-call invents the response under fire.
- Debugging requires production access by default — every investigation requires SSH to a box or a prod DB query; nothing has been built to let people understand the system from outside it.
- Reproducibility absent — a single failed request cannot be replayed in dev; debugging is "try to make it happen again in staging."
- Chaos / fault injection never run — the failure model is theoretical; nobody has confirmed the system actually behaves the way the docs claim under partial failure.
Security & trust boundaries
- Trust zone undefined — design never says "this is the trust boundary; everything inside trusts everything else inside; everything outside is hostile." Without the line, defense in depth is decoration.
- Edge auth, no service-to-service auth — internal services trust any caller on the private network; one compromised pod = lateral movement everywhere.
- Authorization model unclear — who decides what a user can do? In how many places? The answer "depends on the endpoint" is the finding.
- Tenant isolation enforced only at the app layer — DB connection has full multi-tenant access; one missed
WHERE tenant_id = ?in any query, anywhere, is a cross-tenant data leak. Push isolation to a layer the developer can't accidentally bypass (row-level security, per-tenant connection, per-tenant schema). - Secret management strategy absent or per-service — secrets in env files, in code, in different vaults per service, with different rotation cadences; no audit trail of who/what reads them.
- PII flow unmapped — sensitive data lives in several stores, flows through several services, leaves to several vendors, and there's no map; first compliance audit constructs it under pressure.
- Compliance boundary doesn't match service boundary — PCI/HIPAA/GDPR-scoped data flows through services that didn't sign up to be in scope; the entire system is now in scope by spillover.
- Defense in depth absent — a single failure of one layer (one library CVE, one misconfigured ingress, one leaked token) = full compromise; no second line.
- Audit log strategy not unified — each service writes its own audit log in its own shape; cross-system "who did what" requires log archeology.
- Identity propagation broken across async boundaries — sync request has an authenticated user; the job it enqueues runs as "system"; authorization decisions in the job have no idea who initiated it.
Evolvability & change cost
- Reversibility map absent — design never names which decisions are one-way doors and which are two-way; one-way doors get made casually because nobody flagged them as such.
- Strangler-fig path missing — system can be built but cannot be incrementally replaced; the only migration story is "rewrite from scratch."
- Public API committed before proven — shipped to mobile / third parties / SDKs while the shape was still being figured out; can't change without breaking consumers; consumer coordination cost is now permanent.
- Schema rigidity — every shape change requires a migration; nullable everywhere or rigid everywhere; the design implicitly assumes shapes won't change, but they will.
- Versioning strategy absent for events / APIs / messages — first breaking change becomes a discovery exercise.
- Deprecation strategy absent — how does v1 die? If the answer is "it doesn't", v1 is forever, and every future feature pays the v1 tax.
- Migration plan for "current → desired" architecture absent — the desired state is described; the path from where we are today is not; the design is a destination postcard.
- Build-vs-buy unexamined — built bespoke what an off-the-shelf component does better and is on someone else's on-call rotation. Or alternatively, bought a vendor for what's a 200-line in-house solution and now pays vendor risk + lock-in.
- Vendor lock-in chosen without explicit pricing — committing to one cloud's bespoke service (vendor-specific queue / DB / function runtime / auth) with no exit estimate; the cost is invisible until exit.
- Custom format / protocol where a standard would have served — bespoke serialization, bespoke schema language, bespoke auth scheme; every new hire and every integration pays the tax.
Cognitive load & team alignment
- Architecture exceeds team's cognitive bandwidth — 5-person team operating a 40-microservice estate; nobody holds the whole system; every change requires a cross-component investigation; ops burden eats feature velocity.
- Conway misalignment — services span teams; every change requires cross-team coordination; deployment cadences fight team workflows.
- Bus factor of 1 on a load-bearing component — one person built it, owns it, and is the only one who understands it; the architecture is critically dependent on them not leaving.
- Implicit prerequisites — to work on X you must already know Y, Z, W; none of that is written down; new joiners take months to be productive.
- Onboarding path absent or measured in months — no clear "here's how to make your first change"; the architecture's complexity is inherited tacitly, not taught.
- Local development setup requires production access or complex orchestration — you can't run a meaningful slice of the system locally; iteration loop is slow and inhibits experimentation; subtly biases the team away from changes that would touch many services.
- Architecture's metaphor unclear — you cannot finish the sentence "this system is a ___" in one phrase. Without a shared mental model, every conversation about the system spends 5 minutes establishing common ground.
Cost & sustainability
- Cloud cost model not estimated — no projected cost at 10x scale; surprise bills become the architecture's most pressing pressure.
- Per-request cost not measured — you don't know what a single user-facing request costs in cloud spend; pricing decisions and architecture decisions are made blind.
- Storage cost unbounded — logs / traces / metrics / event store retention undefined or "forever"; storage bill grows linearly with time regardless of value.
- Build / deploy time not in requirements — a system that takes 90 minutes to deploy is not a system that supports incident response; deploy time is an architectural property, not a CI tweak.
- Test cost (CI runtime) ignored — full-system test takes 45 minutes; PRs queue; people start skipping tests; the cost of quality compounds.
- On-call cost ignored — how often will this page someone? At what hour? If the answer is "weekly at 3am", the architecture has an outstanding human-cost bill nobody priced.
Truth-checking & premise validation
- Premise check — is the problem the architecture solves actually the real problem, or is the real problem upstream (a product question), downstream (a UX question), or organizational (a team question)?
- "Best practice" cited without context — "we use microservices because Netflix does"; the context that justified the pattern at Netflix doesn't apply here.
- Hype-driven tech choice — Kubernetes for a 3-VM workload, Kafka for 100 events/day, microservices for a 4-person team, GraphQL for an internal-only CRUD service. The technology is correct in some context; this isn't it.
- Cargo cult — pattern copied from a famous post-mortem or blog post; the post described one company's escape from a specific problem; copying their solution without their problem buys complexity, not safety.
- Simpler alternative not considered — has monolith been ruled out, or just not mentioned? Has "do nothing" been ruled out? Has "buy" been ruled out? If the design says "we chose X" without saying "vs Y, Z", the comparison wasn't made.
- Evidence absent for load/perf/scaling claims — "this won't scale" / "this will scale" — where's the benchmark, the load test, the capacity model? Architecture claims that aren't measured are aesthetic preferences.
- Inversion — what would have to be true for the opposite architecture to be the right call? If the answer is "nothing plausible", the design is right by default. If the answer names a future the team can't rule out, the design is fragile.
Pick the principle that fits. If none fits, the issue probably isn't real — drop it.
A note on grilling architecture
Code bugs cost hours; architecture decisions cost years. A wrong line of code is found by the test suite or the next reader. A wrong architecture decision is found years later, by the team that inherits it, when the migration cost exceeds the original build cost by an order of magnitude.
So architecture critique is rarely about whether something is "incorrect" right now. It's about what the design quietly commits the team to that they haven't priced in. Every architecture finding worth raising answers:
- What does this lock in?
- When will it bite — anchored to what observable event?
- What's the cost of reversing it then, vs reversing it now?
If you have sharp answers to those three, the finding is real. If you don't, the finding is taste — drop it.
The other thing that makes architecture review special: most of the time, the problem is not what's in the design. It's what's missing — the failure mode that wasn't considered, the trust boundary that wasn't named, the migration path that wasn't drawn, the alternative that wasn't compared. Reading the design once tells you what's there. Re-reading it asking "what's absent that should be present?" tells you what bites in two years. Do both passes.
What ships with it
Read from the repository
Just SKILL.md. No reference files, no scripts.