---
name: design-review
description: Review a technical design / migration AgDR / feature spec for architectural soundness BEFORE the Build phase. Invokes the Solution Architect agent (Tariq) — the non-code analog of /code-review.
disable-model-invocation: false
argument-hint: "<pr-number-or-path> [repo]"
allowed-tools: Bash, Read, Grep, Glob
---

# /design-review — Solution Architecture Review

Review a **design artifact** — a technical design doc, a migration AgDR, or a feature spec / PRD — for architectural soundness before any code is built against it. This is the non-code analog of `/code-review`: where Rex reviews a code PR, **Tariq (the Solution Architect)** reviews the design.

The Tech Lead *authors* the design; Tariq *reviews* it. Authoring and reviewing are deliberately separate — an author reviewing their own design is the gap this role closes.

## Activated agent + role

When `/design-review` runs:

1. **Primary reviewer**: the **Solution Architect agent (Tariq)** at [`.claude/agents/solution-architect.md`](../../agents/solution-architect.md) — reviews the design against the architecture review lens (NFRs, patterns, tech debt, AgDR linkage, risk, trade-offs, traceability, migration safety) and discovers + applies adopter handbooks exactly as Rex does.
2. **Escalation**: the **[Head of Engineering](../../../roles/engineering/head-of-engineering.md)** — for enterprise / cross-project / new-tech-stack concerns that exceed the Solution Architect's remit.
3. **Conditional Security Auditor**: if the design touches auth / crypto / secrets / user data, chain `/security-review` for the deeper pass.

See [`.claude/rules/role-triggers.md`](../../rules/role-triggers.md) for the full activation protocol. Per the role's `isolated-work-class`, the reviewer runs as a spawned sub-agent.

## Usage

```
/design-review 42                         # review the design in PR #42
/design-review 42 your-org/your-repo      # specify the repo
/design-review docs/designs/checkout.md   # doc-only review (no PR yet)
```

## Process

### 0. Write the active-reviewer marker (REQUIRED — me2resh/apexyard#843, when reviewing a PR)

Before spawning the Solution Architect agent (Tariq) for a PR review, write the active-reviewer session marker. It records that this review pass is the sanctioned one and suppresses `warn-review-marker-write.sh`'s advisory warning on the `*-architecture.approved` write (same convention as `/code-review`'s rex marker; that hook warns and never blocks since #1026 — AgDR-0111). Use the SAME resolved `owner/repo` from step 1 (below) — the sibling-repo resolution in split-portfolio v2 matters here too. At skill entry:

```bash
ops_root=$(git rev-parse --show-toplevel)
r="$ops_root"
while [ -n "$r" ] && [ "$r" != "/" ]; do
  [ -f "$r/.apexyard-fork" ] && { ops_root="$r"; break; }
  [ -f "$r/onboarding.yaml" ] && [ -f "$r/apexyard.projects.yaml" ] && { ops_root="$r"; break; }
  r=$(dirname "$r")
done
mkdir -p "$ops_root/.claude/session"
printf '%s\n' "<owner/repo>#<pr>:architecture" > "$ops_root/.claude/session/active-reviewer"
```

On skill exit (after the review is posted, whether or not the marker gets written), clear it:

```bash
rm -f "$ops_root/.claude/session/active-reviewer"
```

Doc-only reviews (no PR yet) never write a marker, so this step is a no-op for them. Nothing mechanically stops a build-class sub-agent writing the same file; what makes this marker legitimate is that a real, independent review happened. See `.claude/hooks/warn-review-marker-write.sh` and `.claude/rules/pr-workflow.md` § "Build agents cannot self-review".

1. Resolve the target — a PR number (preferred: gives a diff + a place to post the verdict + a marker key) or a path to a design artifact. **Also resolve the repo**: the optional second arg (`/design-review 42 owner/repo`), or the `owner/repo#N` form. In split-portfolio v2 the PR lives in a sibling repo, so a bare `gh pr view 42` resolved against the ops-fork cwd hits the WRONG repo — pass the resolved repo as `--repo` to **every** `gh pr view` / `gh pr diff` call, and thread it into Tariq's spawn so it becomes BOTH his `$PR_HOST_REPO` (the base repo `tracker_review_submit` posts the review to — #763) AND the key for his `<owner>__<repo>__<pr>-architecture.approved` marker (and the active-reviewer marker from step 0). This is the slug the `require-architecture-review.sh` gate derives from the merge command's cd-target (me2resh/apexyard#687). If only a bare number is given and `gh pr view <N>` can't resolve the PR from the current cwd, STOP and ask for the `owner/repo#N` form — never write the marker under a guessed qualifier.
2. Fetch PR details and the latest commit SHA (when reviewing a PR).
3. Read the design artifact(s).
4. Review against the architecture review lens (below) plus discovered handbooks.
5. Submit the review through the tracker-agnostic `tracker_review_submit` (gh PR / glab MR / custom host — #763), not a hardcoded `gh pr review` (when reviewing a PR).
6. On APPROVED only: write the sign-off marker so the Design→Build gate passes (see `/approve-architecture` — Tariq writes the marker himself on an APPROVED verdict; `/approve-architecture` is the human/operator path to record the same marker). Clear the active-reviewer marker from step 0 after the review is posted.

## Review Lens

### Quality attributes / NFRs

- NFRs stated and addressed; targets concrete, not vague.

### Design patterns & structure

- Pattern fits the problem; fits the established architecture; dependencies point the right way.

### Technical debt

- Incurred debt is explicit, justified, and has a paydown path — no silent debt.

### Decisions (AgDR linkage) — BLOCKING

- Every significant technical decision (library, framework, storage, integration, pattern) is captured in an AgDR.
- A real decision with no AgDR → REQUEST CHANGES (run `/decide` first).

### Risk

- Failure modes, blast radius, and rollback addressed.

### Trade-off analysis

- Alternatives genuinely considered; trade-offs of the chosen path stated.

### Requirements traceability

- Design satisfies the PRD / acceptance criteria; no scope creep, no uncovered requirement.

### Migration safety (migration AgDRs)

- Data-loss risk, downtime, lock contention, cross-service consumers, observability, reversible cutover.

### Adopter Handbooks

- Discover + apply the public `handbooks/**` tree and the private `custom-handbooks/**` layer (framework defaults unless overridden in the sibling portfolio repo). Blocking handbooks turn a finding into a required change.

## Output

Posts a GitHub review comment with:

- Commit SHA reviewed
- Review-lens results
- Blocking findings + handbook findings
- Verdict: APPROVED / CHANGES REQUESTED / COMMENT

On APPROVED, Tariq writes `<owner>__<repo>__<pr>-architecture.approved` (repo-qualified, see AgDR-0060) so the `require-architecture-review.sh` gate lets the design PR merge.

Invokes: Solution Architect Agent (Tariq)

## Relationship to other review skills

| Skill | Reviewer | Reviews | Gate |
|-------|----------|---------|------|
| `/code-review` | Rex (Code Reviewer) | code PRs | `block-unreviewed-merge.sh` (Rex marker) |
| `/security-review` | Hakim (Security Auditor) | security-sensitive diffs | auto-fire on auth/crypto/secrets |
| **`/design-review`** | **Tariq (Solution Architect)** | **technical designs / migration AgDRs / feature specs** | **`require-architecture-review.sh` (architecture marker)** |

---

*Part of [ApexYard](https://github.com/me2resh/apexyard) — multi-project SDLC framework for Claude Code · MIT.*