feat(#471): Solution Architect — independent design-review role/agent (Rex for non-code) by atlas-apex · Pull Request #472 · me2resh/apexyard

atlas-apex · 2026-06-01T06:25:16Z

Summary

Adds the Solution Architect (persona Tariq) — an independent, review-only design reviewer, the non-code analog of the Code Reviewer (Rex). The Tech Lead (Hisham) authors a technical design / migration AgDR / feature spec; Tariq reviews it before Build. Author and reviewer are deliberately split — an author reviewing their own design is the exact gap this closes (the "Rex for the non-code stuff" ask in [Feature] Solution Architect — independent design-review role/agent (Rex for non-code) #471).
One role, not two. A dedicated Enterprise Architect was considered and rejected — its remit (enterprise / strategic / cross-project / new-tech-stack) overlaps the existing Head of Engineering, so that review stays with Khalid. The new role owns solution/technical-design review only. Rationale in AgDR-0054.
Gated before Build (new Gate 3b). A technical design lands as a committed doc that merges before implementation, so gating that merge on Tariq's sign-off is the faithful mechanical realisation of "review the design before Build". require-architecture-review.sh blocks merging a design-artifact PR (both gh pr merge and gh api .../merge shapes) until <pr>-architecture.approved exists at a matching HEAD SHA — the non-code twin of require-design-review-for-ui.sh.
Agent modeled on Rex — read-only tools (disallowedTools: Write, Edit), model: opus, the same public + private custom-handbooks/** discovery (framework defaults unless overridden in the sibling portfolio repo), a HARD STOP requiring gh pr review, and the bare-SHA sign-off marker contract. Isolated-work-class per AgDR-0050 § Axis 6, so it spawns as a sub-agent.
Two new skills — /design-review (invoke Tariq) and /approve-architecture (record the marker, the operator path; analog of /approve-design).
Trigger wiring — detect-role-trigger.sh fires Tariq on design-artifact edits; a migration AgDR fires both Tech Lead (author) and Solution Architect (reviewer) by design. Activation table, workflow gates (Gate 3b), Phase 2 of the SDLC, and settings.json all updated.
Counts refreshed — roles 19→20, agents 23→24, hooks 36→37, skills 55→57 across CLAUDE.md and site/*; test_site_counts.sh green.

Testing

bash .claude/hooks/tests/test_require_architecture_review.sh — 17 pass. Covers the DESIGN_GLOBS matcher (design artifacts match, source/non-migration-AgDR/readme do not), and end-to-end gate behaviour via a self-contained mock gh: design PR + no marker → BLOCK; + matching marker → ALLOW; + stale-SHA marker → BLOCK; non-design PR → no-op; non-merge command → no-op; gh api merge shape → BLOCK.
bash .claude/hooks/tests/test_detect_role_trigger.sh — 34 pass (8 new Tariq cases: design-doc / designs/ / prds/ path triggers, migration-AgDR fires both roles, isolated-work-class spawn banner, prompted activation, source-file negative).
bash .claude/hooks/tests/test_site_counts.sh — green (skills 57, hooks 37, roles 20).
shellcheck -S warning clean on the new + edited hooks and tests.
Pre-existing, unrelated test failures (test_agent_routing_sync_and_drift, test_link_custom_skills) reproduce identically on clean dev (Ollama-not-running / symlink-sandbox env) — not introduced here.

Glossary

Term	Definition
Solution Architect (Tariq)	New review-only role that reviews technical designs / migration AgDRs / feature specs before Build; the non-code analog of the Code Reviewer (Rex).
Design artifact	A committed technical design doc, migration AgDR, or feature spec / PRD — the input Tariq reviews. Default match patterns are configurable via `design_paths` / `design_paths_exclude`.
Gate 3b (architecture review)	The Design→Build merge gate: a design-artifact PR can't merge without a `<pr>-architecture.approved` sign-off marker at a matching HEAD SHA.
Sign-off marker	A gitignored session file (`.claude/session/reviews/<pr>-architecture.approved`) containing the bare 40-char HEAD SHA; written by Tariq on APPROVED or by `/approve-architecture`.
Isolated-work-class	AgDR-0050 § Axis 6 classification: roles whose work benefits from isolated context + tool restriction spawn as sub-agents (like Rex / Hakim / the Tech Lead).
Review lens	Tariq's fixed competency checklist — quality attributes / NFRs, design patterns, technical debt, decision (AgDR) linkage, risk, trade-off analysis, requirements traceability, migration safety.

Closes #471

…Rex for non-code) Adds one new role, the Solution Architect (persona Tariq), as a read-only reviewer of technical designs / migration AgDRs / feature specs — the non-code analog of the Code Reviewer (Rex). The Tech Lead authors the design; Tariq reviews it before Build. Authoring and reviewing are deliberately separate. - roles/architecture/solution-architect.md — new 6th department; persona Tariq, isolated-work-class, structured review lens (NFRs, patterns, tech debt, AgDR linkage, risk, trade-offs, traceability, migration safety) - .claude/agents/solution-architect.md — review agent modeled on Rex: read-only (disallow Write/Edit), opus, handbook discovery (public + private custom), HARD STOP submit-review + sign-off marker on APPROVED - /design-review (invoke Tariq) + /approve-architecture (record the marker) - require-architecture-review.sh — Design→Build merge gate (Gate 3b) on design-artifact PRs, both merge shapes; modeled on require-design-review-for-ui.sh - detect-role-trigger.sh fires Tariq on design-artifact edits (additive to the Tech Lead docs/agdr/ trigger — a migration AgDR fires both) - role-triggers.md, workflow-gates.md (Gate 3b), workflows/sdlc.md Phase 2, settings.json wiring, CLAUDE.md + site/* counts (roles 19→20, agents 23→24, hooks 36→37, skills 55→57), AgDR-0054 - Tests: test_require_architecture_review.sh (17), test_detect_role_trigger.sh (+8 Tariq cases, 34 total); shellcheck clean; site-counts green Closes #471 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

atlas-apex

Code Review: PR #472

Commit: 82eec0a783030530010e8a178512491e31c5e394

Summary

Adds the Solution Architect (persona Tariq) — a read-only, review-only design-review role/agent, the non-code analog of the Code Reviewer (Rex). Tech Lead authors the design; Tariq independently reviews it before Build. Ships a new merge gate (require-architecture-review.sh, Gate 3b), trigger wiring in detect-role-trigger.sh, two skills (/design-review, /approve-architecture), AgDR-0054, and refreshed framework counts. Cleanly scoped to a single commit (82eec0a, +1177/-39, 22 files).

Checklist Results

✅ Architecture & Design: Pass — clean author/reviewer split; one role (EA correctly rejected as overlapping Head of Engineering); review-only enforced mechanically via disallowedTools: Write, Edit.
✅ Code Quality: Pass — hooks are clear, well-commented, shellcheck-clean (-S warning, including the test file).
✅ Testing: Pass — test_require_architecture_review.sh 17/17 and test_detect_role_trigger.sh 34/34 verified locally; new hook + both trigger families (path + migration-AgDR-fires-both + prompted + negative) covered.
✅ Security: Pass — no secrets; marker is gitignored session state; trust model documented and matches the existing marker hooks.
✅ Performance: Pass — N/A (docs/hooks/agent change).
✅ PR Description & Glossary: Pass — Glossary present and substantive; ticket linked (Closes #471); AgDR linked via  marker.
✅ Summary Bullet Narrative: Pass — bullets are narrative (what + why), not label-only.
✅ Technical Decisions (AgDR):Pass — AgDR-0054 present, referenced, and thorough (options table, rejected alternatives, consequences).
✅ Adopter Handbooks: N/A — no handbook trigger patterns in a docs/hooks/agent diff.

Verified against the standards

Shell hook quality — the load-bearing part:

require-architecture-review.sh covers both merge shapes (gh pr merge and gh api .../pulls/<N>/merge) via the shared _lib-extract-pr.sh (is_merge_command / extract_pr_number), closing the #47 API-shape bypass. Confirmed wired into settings.json on both Bash(gh pr merge *) and Bash(gh api *).
Marker-path resolution is correct: resolves the ops-fork root via resolve_ops_root (from _lib-ops-root.sh) rather than git rev-parse --show-toplevel, so markers land at the ops fork even inside a workspace/<project>/ clone (the #229/#230 fix). The agent and both skills resolve the same MARKER_HOME consistently.
SHA-consistency resolves the PR's real HEAD via gh with a documented local-HEAD fallback (#55), matching the other gates.
design_paths (REPLACE) + design_paths_exclude (additive carve-out) mirror the ui_paths / ui_paths_exclude precedent (#275).
Agent marker contract is the bare 40-char SHA + newline (verified via od -c); /approve-architecture writes it with printf '%s\n' — no labels/JSON, matching the gate's strip-then-compare read.

Framework count consistency: on-disk actuals = roles 20, agents 24, hooks 37 (non-_lib), skills 57 — all match the PR's claims and site/*. test_site_counts.sh is green (hooks=37). CLAUDE.md's agents/skills/roles rows (24/57/20) are correct.

Commit SHA: local HEAD == PR headRefOid (82eec0a). Single clean feature commit.

Note on scope: the git clone→suggest-mcp-reindex-after-clone.sh settings hook and the CLAUDE.md SETUP portfolio_onboarding_path change appear in the diff-vs-dev but belong to prior branch commits (#465/#470, not yet merged to dev), not to the Solution Architect commit 82eec0a. Out of scope for this review.

Suggestions

nit: CLAUDE.md "Hooks" row says 25 shell scripts, while site/* and test_site_counts.sh use 37 (the on-disk find ... ! -name '_lib*' count). This is a deliberate dual-count convention — AgDR-0054 line 40 spells it out ("hooks 24→25 (36→37 committed shell scripts)"), and CLAUDE.md has used the curated narrative number since before this PR (it was 24 on dev while site said 35). So it's internally consistent, not a bug. But it's a confusing divergence that test_site_counts.sh doesn't guard (the test only scans site/*). Consider either (a) aligning CLAUDE.md's "Hooks" row to the on-disk count (37) so all surfaces agree, or (b) extending the count test to also assert CLAUDE.md's curated number against its own documented convention. Non-blocking — purely a future-maintenance clarity nit.

Verdict

APPROVED

Excellent work — the gate faithfully mirrors the proven Rex/require-design-review-for-ui.sh pattern, both merge shapes and the ops-root marker path are handled correctly, and the test coverage is real (verified locally, not just claimed). The single nit above is advisory and does not hold up the merge.

🤖 Reviewed by Rex (Code Reviewer Agent)
📌 Reviewed commit: 82eec0a783030530010e8a178512491e31c5e394

…gs + lists CI markdownlint-cli2 flagged the checklist sections in the Solution Architect agent + /design-review skill: `### Heading` sat directly above a `- [ ]` list with no blank line (MD022 blanks-around-headings + MD032 blanks-around-lists). Add the required blank line after each heading. No content change. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

atlas-apex

Code Review (re-review): PR #472

Commit: 848433370f472caaabcaa239ad060389e1e6af8a

Summary

Re-review triggered by a new commit (8484333) pushed after my prior APPROVED review at 82eec0a. Verified the delta 82eec0a..8484333 is a pure markdownlint whitespace fix — no content, logic, or checklist changes.

Delta verification (`82eec0a`..`8484333`)

2 files touched: .claude/agents/solution-architect.md (+8), .claude/skills/design-review/SKILL.md (+9)
17 insertions, 0 deletions — additions only
Every added line is a single blank line inserted after a ### Heading and before the following list (MD022 blanks-around-headings / MD032 blanks-around-lists)
Mechanical check: 0 non-blank added lines in the entire delta
Heading text, checklist items, BLOCKING markers, and AgDR-enforcement language are byte-for-byte identical to the approved 82eec0a version

Checklist Results (carried forward — unchanged from prior approval)

Architecture & Design: Pass
Code Quality: Pass
Testing: Pass (hook + tests unchanged)
Security: Pass
Performance: Pass
PR Description & Glossary: Pass
Technical Decisions (AgDR):Pass (AgDR unchanged)
Adopter Handbooks: N/A

Issues Found

None. The new commit is cosmetic-only and does not affect any prior finding.

Verdict

APPROVED (posted as comment — GitHub blocks self-approval; verdict is unchanged)

Reviewed by Rex (Code Reviewer Agent)
Reviewed commit: 848433370f472caaabcaa239ad060389e1e6af8a

… (Rex for non-code) (#472) * feat: add Solution Architect — independent design-review role/agent (Rex for non-code) Adds one new role, the Solution Architect (persona Tariq), as a read-only reviewer of technical designs / migration AgDRs / feature specs — the non-code analog of the Code Reviewer (Rex). The Tech Lead authors the design; Tariq reviews it before Build. Authoring and reviewing are deliberately separate. - roles/architecture/solution-architect.md — new 6th department; persona Tariq, isolated-work-class, structured review lens (NFRs, patterns, tech debt, AgDR linkage, risk, trade-offs, traceability, migration safety) - .claude/agents/solution-architect.md — review agent modeled on Rex: read-only (disallow Write/Edit), opus, handbook discovery (public + private custom), HARD STOP submit-review + sign-off marker on APPROVED - /design-review (invoke Tariq) + /approve-architecture (record the marker) - require-architecture-review.sh — Design→Build merge gate (Gate 3b) on design-artifact PRs, both merge shapes; modeled on require-design-review-for-ui.sh - detect-role-trigger.sh fires Tariq on design-artifact edits (additive to the Tech Lead docs/agdr/ trigger — a migration AgDR fires both) - role-triggers.md, workflow-gates.md (Gate 3b), workflows/sdlc.md Phase 2, settings.json wiring, CLAUDE.md + site/* counts (roles 19→20, agents 23→24, hooks 36→37, skills 55→57), AgDR-0054 - Tests: test_require_architecture_review.sh (17), test_detect_role_trigger.sh (+8 Tariq cases, 34 total); shellcheck clean; site-counts green Closes #471 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix: markdownlint MD022/MD032 — blank lines around review-lens headings + lists CI markdownlint-cli2 flagged the checklist sections in the Solution Architect agent + /design-review skill: `### Heading` sat directly above a `- [ ]` list with no blank line (MD022 blanks-around-headings + MD032 blanks-around-lists). Add the required blank line after each heading. No content change. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: me2resh <ahmed.abdelaliem@gmail.com> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

atlas-apex commented Jun 1, 2026

View reviewed changes

atlas-apex merged commit cda12b7 into me2resh:dev Jun 1, 2026
5 checks passed

This was referenced Jun 1, 2026

[Task] MCP-first rule doesn't reach sub-agents — they grep instead of search_code #475

Closed

[Feature] Solution Architect — independent design-review role/agent (Rex for non-code) #471

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(#471): Solution Architect — independent design-review role/agent (Rex for non-code)#472

feat(#471): Solution Architect — independent design-review role/agent (Rex for non-code)#472
atlas-apex merged 2 commits into
me2resh:devfrom
atlas-apex:feature/GH-471-solution-architect-role

atlas-apex commented Jun 1, 2026

Uh oh!

atlas-apex left a comment

Uh oh!

atlas-apex left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

atlas-apex commented Jun 1, 2026

Summary

Testing

Glossary

Uh oh!

atlas-apex left a comment

Choose a reason for hiding this comment

Code Review: PR #472

Summary

Checklist Results

Verified against the standards

Suggestions

Verdict

Uh oh!

atlas-apex left a comment

Choose a reason for hiding this comment

Code Review (re-review): PR #472

Summary

Delta verification (82eec0a..8484333)

Checklist Results (carried forward — unchanged from prior approval)

Issues Found

Verdict

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Delta verification (`82eec0a`..`8484333`)