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 — 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 — 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 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:

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:

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

Part of ApexYard — multi-project SDLC framework for Claude Code · MIT.