# Design Review

> 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.

- Skill: `me2resh/design-review` (Agent Skill)
- Install (CLI): `npx skillmds@latest add me2resh/design-review`
- Raw SKILL.md: https://api.skillmd.com/api/skills/me2resh/design-review/raw
- Safety review: pending
- Works with: Claude Code, Claude.ai, OpenAI Codex
- Category: AI & ML
- Author: me2resh (https://skillmd.com/u/me2resh)
- Updated: 2026-09-17
- Page: https://skillmd.com/skills/me2resh/design-review

---


# /design-review — Solution Architecture Review

Read .claude/rules/writing-standard.md before you write a review.
Use the controlled technical writing profile for the architecture review.
If the artifact fails the profile, you must request changes.
State the verdict and next action first. State evidence after the verdict.

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".

### 0a. Never hand the reviewer a marker path (me2resh/apexyard#1144)

**The spawn prompt for Tariq MUST NOT contain a literal marker path.** Say
*"write your approval marker on an APPROVED verdict"*; say nothing about where.

Tariq already resolves the correct path through `review_marker_path` — the
repo-qualified `<owner>__<repo>__<pr>-architecture.approved` form from AgDR-0060,
which is the exact path the gates read. A path in the prompt overrides that
correct resolution: the agent obeys the instruction it was handed, and the
marker lands at the bare-number `<pr>-architecture.approved` instead. **No gate reads
that path** — there is no bare-number fallback on any on-disk marker lookup.

The failure is silent in the dangerous direction. `ls .claude/session/reviews/`
shows a file that reads, to a human, like a valid approval; only the merge
attempt reveals otherwise. And at that moment the obvious repair — moving the
file into place — is marker forging, the behaviour
[`pr-workflow.md`](../../rules/pr-workflow.md) § "Build agents cannot
self-review" exists to prevent. The right recovery is always: delete the
gate-invisible file and re-run a real review.

`warn-unqualified-review-marker.sh` warns (advisory, never blocks) when a
bare-number marker appears, and the merge gates name the near-miss in their
refusal message — but the cheap fix is upstream of both: don't pass a path.

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.*