Architecture Review Skill

When to Trigger

After
```
writing-plans
```
produces an implementation plan, before
```
executing-plans
```
When a significant refactor is proposed
When adding a new external service or integration
When the team is debating between two approaches
When something "feels off" about a design but you can't articulate why

Step-by-Step Process

1. Load the Plan

Read the implementation plan (typically

implementation_plan.md

or the artifact from

writing-plans

Extract:

Components being built
Data flow between components
External dependencies
Assumptions the plan makes

2. Run the Checklists

Work through each checklist section. For every "No" or "Unclear" answer, flag it as a finding.

A. Complexity Check

Is this the simplest solution that could work?
Are there off-the-shelf solutions (libraries, services) that cover 80%+ of this?
Does each component have a single, clear responsibility?
Would a junior developer understand this in 30 minutes?

B. Coupling & Cohesion

Can each module be tested in isolation?
Are there circular dependencies? (A needs B needs A)
Is data ownership clear? (only one place writes to each piece of state)
Is the public API surface minimal? (don't expose what doesn't need to be exposed)

C. Data & State

Is the source of truth for each piece of data clearly defined?
What happens if the datastore goes down?
Is there any global mutable state? If so, is it justified?
Are database migrations planned for any schema changes?

D. Failure Modes

What happens when each external service is unavailable?
Are there retry mechanisms with backoff for network calls?
Are there timeouts on all I/O operations?
Is there a circuit breaker if a downstream dependency degrades?
What does a partial failure look like to the user?

E. Security

Is all user input validated and sanitized?
Are secrets managed via env vars / secrets manager (not hardcoded)?
Is authentication and authorization clearly separated?
Are there any SQL injection / XSS / CSRF vectors?
Does the principle of least privilege apply to service accounts?

F. Scalability

What is the expected load in 6 months? Does the design handle it?
Are there any obvious N+1 query problems?
Is any caching planned? Is the invalidation strategy defined?
Are background jobs used for anything that could block the request cycle?

G. Observability

Is structured logging planned?
Are there metrics/health endpoints?
Will errors surface in a monitoring tool (not just logs)?
Is there a way to trace a request end-to-end?

H. Operability

Can this be deployed without downtime?
Is there a rollback plan if the deployment goes wrong?
Are environment differences (dev/staging/prod) documented?

3. Classify Findings

For each "No" or "Unclear" found above, classify it:

Class	Meaning	Action
🔴 Blocker	Must fix before implementation	Revise plan
🟡 Warning	Should address, can proceed carefully	Add a task to backlog
🔵 Note	Nice to have, low risk	Document and move on

4. Produce the Review Report

Her zaman ayrı bir dosyaya yaz — plan dosyasını kirletme:

.agent/reviews/architecture-review-<plan-slug>.md

Eğer bu review'dan önemli bir mimari karar çıktıysa, ek olarak

knowledge-base-update

skill'ini kullanarak

decision

tipiyle knowledge base'e de ekle —

project-context-primer

oradan okuyacak.

# Architecture Review — <plan name>
**Date:** <YYYY-MM-DD>
**Reviewer:** Agent

## Verdict
<Approved | Approved with conditions | Revisions required>

## 🔴 Blockers
### 1. <Finding title>
**Checklist item:** Failure Modes — timeout on I/O
**Issue:** The email service call has no timeout. If SendGrid hangs, the entire request thread blocks.
**Recommendation:** Wrap in a 5-second timeout; use a background job queue for non-critical emails.

## 🟡 Warnings
### 1. <Finding title>
...

## 🔵 Notes
### 1. <Finding title>
...

## Summary
<Overall health of the architecture in 2-3 sentences.>

5. Gate: Proceed or Revise?

If there are any 🔴 Blockers: stop. Share the report with the user and revise the plan before any code is written.
If there are only 🟡/🔵: proceed, but ensure warnings are tracked as tasks.

Rules

Be specific, not generic. "You should add error handling" is useless. "The
```
fetchUser()
```
call on line 42 of the plan has no 404 handling" is a review.
Don't block on stylistic preferences. Blockers are objective risks, not taste.
Never approve a plan with an undefined failure mode for a critical path.
If you can't understand part of the plan, that is itself a finding — clarity is a requirement.

Vibe-coder-kit architecture-review