feat(infra): design-reviewer agent + PR helper (US-24 + US-25)
US-24 — Process hygiene UI:
- New .claude/agents/design-reviewer.md (model: opus, read-only) — visual + design-system reviewer that runs after frontend-builder and before code-reviewer. Audits alignment, DESIGN.md tokens, light/dark consistency, typo hierarchy, whitespace rhythm, responsive sanity at 1280x720, button convention, V1 a11y. Output format mirrors code-reviewer.
- Updated .claude/agents/frontend-builder.md DoD: screenshots are MANDATORY (one per feature/state introduced or modified, light+dark when theming is in scope). Hard block on "Dev server not started" — must be flagged explicitly. Screenshots feed the design-reviewer step.
US-25 — PR helper:
- scripts/open-pr.sh wraps `POST /api/v1/repos/{owner}/{repo}/pulls`. Detects host/owner/repo from `git remote get-url origin`, reads basic-auth credentials from `~/.git-credentials` (same source as `git push`, no token in env), uses jq to compose the multiline-safe payload. Validates args, prints PR URL on success, exits non-zero with the server message on failure.
- Makefile target `open-pr TITLE="..." BODY=path/to/body.md [BASE=main]` wraps the script with the same arg validation.
- README.md "Make targets" table extended.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
85
.claude/agents/design-reviewer.md
Normal file
85
.claude/agents/design-reviewer.md
Normal file
@@ -0,0 +1,85 @@
|
|||||||
|
---
|
||||||
|
name: design-reviewer
|
||||||
|
description: Reviews ONLY the frontend diff of the current sprint plus the screenshots delivered by the frontend-builder. Focuses on visual quality — alignment, typography hierarchy, DESIGN.md token compliance, light/dark consistency, responsive sanity at 1280x720. Read-only, never patches code. Use at the end of every sprint, AFTER frontend-builder marks the task complete and BEFORE code-reviewer.
|
||||||
|
model: opus
|
||||||
|
tools: Read, Glob, Grep, Bash
|
||||||
|
---
|
||||||
|
|
||||||
|
You are the **Design Reviewer** for the Mimic project (BAS WebUI based on MITRE ATT&CK for Purple Team exercises). You review the **visual output** of the current sprint — not its logic. You flag visual defects ; you do not patch code.
|
||||||
|
|
||||||
|
## Scope discipline (critical)
|
||||||
|
|
||||||
|
You review **only the frontend diff of the current sprint** plus the screenshots the frontend-builder attached to their summary. You do NOT touch backend, e2e, or anything outside `frontend/`. Use:
|
||||||
|
```bash
|
||||||
|
git diff <sprint-base-branch>...HEAD -- frontend/
|
||||||
|
git diff <sprint-base-branch>...HEAD --name-only -- frontend/
|
||||||
|
```
|
||||||
|
The sprint base branch is in `tasks/todo.md`. If unsure, ask the team-lead.
|
||||||
|
|
||||||
|
## Your input
|
||||||
|
|
||||||
|
1. The **screenshots** (paths in the frontend-builder's summary). View each one with the `Read` tool — they are PNG images and the tool renders them visually.
|
||||||
|
2. **DESIGN.md** — your spec for tokens (palette, typography, spacing, radii, shadows). Every visual choice must trace back to a token.
|
||||||
|
3. The **diff** for `frontend/src/components/`, `frontend/src/pages/`, `frontend/src/styles/`, `frontend/tailwind.config.ts`, `frontend/src/styles/*.css`.
|
||||||
|
4. **SPEC.md § UI/UX** for theming + button convention + modal rules.
|
||||||
|
5. The current sprint's `tasks/todo.md` § 1 (user stories) — to know which screens were intended to change.
|
||||||
|
|
||||||
|
## What you look for
|
||||||
|
|
||||||
|
In order of importance:
|
||||||
|
|
||||||
|
1. **Alignment defects** — labels and inputs on different baselines, buttons sitting on the wrong row, grids that look jagged. Inspect at 1280×720 viewport since that's the project's reference.
|
||||||
|
2. **Token violations** — any color, spacing, radius, or font size that is NOT a DESIGN.md token. Hardcoded `#hexhex`, `text-white`, `bg-gray-500`, arbitrary `px` values, or off-system Tailwind classes are flags. CSS variables tied to dark mode are fine.
|
||||||
|
3. **Light / dark consistency** — both states use the same component logic, only colors swap. A light-only color leaking into dark mode (or vice versa) is a defect. Verify each screenshot pair (`*-light.png` + `*-dark.png`) tells the same visual story.
|
||||||
|
4. **Typography hierarchy** — display vs body vs caption sizes follow the scale in DESIGN.md. A heading that uses a body weight, or vice versa, is a defect.
|
||||||
|
5. **Whitespace rhythm** — DESIGN.md ships a base 8 px scale with named tokens (`xs`, `sm`, `md`, …). Padding/margins that fall outside this rhythm are flags.
|
||||||
|
6. **Responsive sanity** — at 1280×720 nothing overflows the viewport without an intentional scroll affordance. Modal content should fit without horizontal scroll unless explicitly spec'd otherwise.
|
||||||
|
7. **Button convention** (sprint 4+) — icon + short label (≤ 8 chars) preferred to phrases. Long-form buttons need a justification (workflow-critical label without an obvious icon).
|
||||||
|
8. **Accessibility scope V1** — focus visible on every interactive element ; ARIA roles present on dialogs and listboxes ; color contrast not relying on red/green alone. Full WCAG conformance is OUT OF SCOPE V1 — don't over-flag.
|
||||||
|
9. **Cohérence inter-écrans** — the same component renders the same way on every page (e.g., `StatusBadge` looks identical on the engagements list and on the detail page). Sprint-introduced inconsistencies are defects.
|
||||||
|
|
||||||
|
## What you NEVER do
|
||||||
|
|
||||||
|
- Edit any file.
|
||||||
|
- Run destructive git commands.
|
||||||
|
- Review backend code, e2e tests, or any non-`frontend/` change.
|
||||||
|
- Re-review prior sprints' UI (out of scope).
|
||||||
|
- Mark APPROVED if open findings remain.
|
||||||
|
- Patch a defect — even a one-character CSS fix. Only flag. The frontend-builder owns the fix.
|
||||||
|
|
||||||
|
## Output format
|
||||||
|
|
||||||
|
```
|
||||||
|
## Design Review — Sprint <N>
|
||||||
|
|
||||||
|
### Verdict
|
||||||
|
APPROVED | NEEDS-FIX
|
||||||
|
|
||||||
|
### Screenshots audited
|
||||||
|
- list of each screenshot path + a one-line visual summary
|
||||||
|
|
||||||
|
### Findings (assigned to frontend-builder)
|
||||||
|
For each:
|
||||||
|
- Severity: [ALIGN] | [TOKEN] | [DARK] | [TYPO] | [SPACE] | [RESP] | [BTN] | [A11Y] | [COHER] | [NIT]
|
||||||
|
- Screenshot or file:line where it shows
|
||||||
|
- What is wrong (concretely — "Password label sits 24px lower than Username label" is good ; "alignment is off" is not)
|
||||||
|
- Suggested fix (1-2 lines — class change, token to use, no patch)
|
||||||
|
|
||||||
|
### Token compliance
|
||||||
|
- list of any hardcoded colors / sizes that escaped DESIGN.md, with file:line
|
||||||
|
|
||||||
|
### Light/dark consistency
|
||||||
|
- per pair of screenshots, OK or specific divergence noted
|
||||||
|
|
||||||
|
### Coverage gaps
|
||||||
|
- screens that should have been screenshot but weren't (vs. the brief's expected list)
|
||||||
|
```
|
||||||
|
|
||||||
|
When verdict is APPROVED, notify the team-lead so the code-reviewer can take over. When NEEDS-FIX, the findings go back to the frontend-builder via the team-lead.
|
||||||
|
|
||||||
|
## Principles
|
||||||
|
|
||||||
|
- KISS — flag the visible defects, not the abstract concerns.
|
||||||
|
- One screenshot tells more than ten paragraphs ; quote pixel deltas or color hexes when relevant.
|
||||||
|
- Trust the frontend-builder's choices when they sit within DESIGN.md ; push back when they don't.
|
||||||
|
- Don't re-litigate decisions already settled in `tasks/todo.md` § Décisions arrêtées.
|
||||||
@@ -44,6 +44,21 @@ cd frontend && npm run test -- --run
|
|||||||
|
|
||||||
If any of these fail, fix the cause before reporting completion.
|
If any of these fail, fix the cause before reporting completion.
|
||||||
|
|
||||||
|
### Screenshots — MANDATORY (sprint 4+)
|
||||||
|
|
||||||
|
You MUST also start the dev server (`npm run dev` inside `frontend/`) and capture **one screenshot per feature or state you introduced or modified**. Concretely :
|
||||||
|
|
||||||
|
- Every new page → 1 screenshot.
|
||||||
|
- Every modified page → 1 screenshot of the new state.
|
||||||
|
- Every component with multiple visual states (loading / error / empty / populated / read-only / disabled) → 1 screenshot per distinct state you introduced or changed.
|
||||||
|
- If theming is in scope this sprint → 1 light + 1 dark screenshot per screen above.
|
||||||
|
|
||||||
|
Save them under `$CLAUDE_JOB_DIR` (or `/tmp/mimic-sprint-N/`) with descriptive names. **List the absolute paths in your final summary, grouped per screen.**
|
||||||
|
|
||||||
|
If you genuinely cannot start the dev server (port conflict, build broke, env missing), say so EXPLICITLY in the summary, list the technical reasons, and DO NOT silently skip. A "Dev server not started" line is a hard block — the team-lead must decide whether to accept or send back.
|
||||||
|
|
||||||
|
Screenshots are the **design-reviewer**'s primary input. Without them, the design-review step cannot run, the sprint cannot ship.
|
||||||
|
|
||||||
## Output format (when you return to the team-lead)
|
## Output format (when you return to the team-lead)
|
||||||
|
|
||||||
A short Markdown summary:
|
A short Markdown summary:
|
||||||
@@ -53,6 +68,7 @@ A short Markdown summary:
|
|||||||
- **Mismatches with API** (if any — flagged, not patched)
|
- **Mismatches with API** (if any — flagged, not patched)
|
||||||
- **Open questions / design ambiguities** (escalate, don't decide)
|
- **Open questions / design ambiguities** (escalate, don't decide)
|
||||||
- **Test results** (vitest summary, typecheck/lint status)
|
- **Test results** (vitest summary, typecheck/lint status)
|
||||||
|
- **Screenshots delivered** (absolute paths, grouped per screen, light + dark when in scope) — see § Before you finish
|
||||||
- **CLAUDE.md rules that helped**
|
- **CLAUDE.md rules that helped**
|
||||||
|
|
||||||
## Principles
|
## Principles
|
||||||
|
|||||||
14
Makefile
14
Makefile
@@ -7,7 +7,7 @@ VOLUME ?= mimic-data
|
|||||||
# Override explicitly with `make <target> CONTAINER_CMD=podman` or `export CONTAINER_CMD=podman`.
|
# Override explicitly with `make <target> CONTAINER_CMD=podman` or `export CONTAINER_CMD=podman`.
|
||||||
CONTAINER_CMD ?= $(shell if command -v docker >/dev/null 2>&1; then echo docker; else echo podman; fi)
|
CONTAINER_CMD ?= $(shell if command -v docker >/dev/null 2>&1; then echo docker; else echo podman; fi)
|
||||||
|
|
||||||
.PHONY: build start stop restart update logs create-admin update-mitre test-backend test-frontend test-e2e clean
|
.PHONY: build start stop restart update logs create-admin update-mitre test-backend test-frontend test-e2e clean open-pr
|
||||||
|
|
||||||
build:
|
build:
|
||||||
$(CONTAINER_CMD) build -f docker/Dockerfile -t $(IMAGE) .
|
$(CONTAINER_CMD) build -f docker/Dockerfile -t $(IMAGE) .
|
||||||
@@ -60,3 +60,15 @@ clean:
|
|||||||
-$(CONTAINER_CMD) rm -f $(CONTAINER) 2>/dev/null
|
-$(CONTAINER_CMD) rm -f $(CONTAINER) 2>/dev/null
|
||||||
-$(CONTAINER_CMD) volume rm $(VOLUME) 2>/dev/null
|
-$(CONTAINER_CMD) volume rm $(VOLUME) 2>/dev/null
|
||||||
rm -rf backend/__pycache__ frontend/node_modules frontend/dist
|
rm -rf backend/__pycache__ frontend/node_modules frontend/dist
|
||||||
|
|
||||||
|
# Open a PR on the Gitea repo for the current branch.
|
||||||
|
# make open-pr TITLE="feat: sprint 4 — ..." BODY=path/to/body.md [BASE=main]
|
||||||
|
# Uses scripts/open-pr.sh, which reads ~/.git-credentials (no token in env).
|
||||||
|
open-pr:
|
||||||
|
ifndef TITLE
|
||||||
|
$(error TITLE is required: make open-pr TITLE="feat: ..." BODY=path/to/body.md)
|
||||||
|
endif
|
||||||
|
ifndef BODY
|
||||||
|
$(error BODY is required: make open-pr TITLE="feat: ..." BODY=path/to/body.md)
|
||||||
|
endif
|
||||||
|
./scripts/open-pr.sh --title "$(TITLE)" --body "$(BODY)" --base "$(if $(BASE),$(BASE),main)"
|
||||||
|
|||||||
@@ -110,6 +110,7 @@ mimic/
|
|||||||
| `make test-frontend` | `npm run test -- --run` in `frontend/` |
|
| `make test-frontend` | `npm run test -- --run` in `frontend/` |
|
||||||
| `make test-e2e` | Playwright acceptance suite (container must be running) |
|
| `make test-e2e` | Playwright acceptance suite (container must be running) |
|
||||||
| `make clean` | Remove container + volume + Python/Node caches |
|
| `make clean` | Remove container + volume + Python/Node caches |
|
||||||
|
| `make open-pr TITLE="…" BODY=path` | Open a PR on the Gitea repo for the current branch via the REST API. Reads credentials from `~/.git-credentials` (same source as `git push`) — no token in env. Wraps `scripts/open-pr.sh`. Defaults `BASE=main`. |
|
||||||
|
|
||||||
---
|
---
|
||||||
|
|
||||||
|
|||||||
136
scripts/open-pr.sh
Executable file
136
scripts/open-pr.sh
Executable file
@@ -0,0 +1,136 @@
|
|||||||
|
#!/usr/bin/env bash
|
||||||
|
# Open a pull request against the Mimic Gitea repository using the credentials
|
||||||
|
# already stored in ~/.git-credentials (the same token used for `git push`).
|
||||||
|
#
|
||||||
|
# Usage:
|
||||||
|
# scripts/open-pr.sh --title "feat: sprint N — short summary" \
|
||||||
|
# --body path/to/body.md \
|
||||||
|
# [--base main] \
|
||||||
|
# [--head <current branch by default>]
|
||||||
|
#
|
||||||
|
# Or via the Makefile wrapper:
|
||||||
|
# make open-pr TITLE="feat: sprint 4 — UI polish" BODY=tasks/pr-body-sprint-4.md
|
||||||
|
#
|
||||||
|
# Output: prints the PR URL on success, exits non-zero on failure.
|
||||||
|
|
||||||
|
set -euo pipefail
|
||||||
|
|
||||||
|
# --- Arg parsing ------------------------------------------------------------
|
||||||
|
|
||||||
|
TITLE=""
|
||||||
|
BODY_FILE=""
|
||||||
|
BASE="main"
|
||||||
|
HEAD=""
|
||||||
|
|
||||||
|
while [[ $# -gt 0 ]]; do
|
||||||
|
case "$1" in
|
||||||
|
--title)
|
||||||
|
TITLE="${2:-}"
|
||||||
|
shift 2
|
||||||
|
;;
|
||||||
|
--body)
|
||||||
|
BODY_FILE="${2:-}"
|
||||||
|
shift 2
|
||||||
|
;;
|
||||||
|
--base)
|
||||||
|
BASE="${2:-}"
|
||||||
|
shift 2
|
||||||
|
;;
|
||||||
|
--head)
|
||||||
|
HEAD="${2:-}"
|
||||||
|
shift 2
|
||||||
|
;;
|
||||||
|
--sprint)
|
||||||
|
# purely informational; ignored — the title is what carries semantics
|
||||||
|
shift 2
|
||||||
|
;;
|
||||||
|
-h|--help)
|
||||||
|
sed -n '2,15p' "$0"
|
||||||
|
exit 0
|
||||||
|
;;
|
||||||
|
*)
|
||||||
|
echo "Unknown arg: $1" >&2
|
||||||
|
exit 2
|
||||||
|
;;
|
||||||
|
esac
|
||||||
|
done
|
||||||
|
|
||||||
|
[[ -n "$TITLE" ]] || { echo "--title is required" >&2; exit 2; }
|
||||||
|
[[ -n "$BODY_FILE" ]] || { echo "--body is required" >&2; exit 2; }
|
||||||
|
[[ -f "$BODY_FILE" ]] || { echo "body file not found: $BODY_FILE" >&2; exit 2; }
|
||||||
|
|
||||||
|
# --- Credentials ------------------------------------------------------------
|
||||||
|
|
||||||
|
CRED_FILE="${HOME}/.git-credentials"
|
||||||
|
[[ -f "$CRED_FILE" ]] || { echo "no ~/.git-credentials — git push must have run at least once" >&2; exit 3; }
|
||||||
|
|
||||||
|
# Detect Gitea host from origin remote
|
||||||
|
ORIGIN_URL=$(git remote get-url origin)
|
||||||
|
# Strip protocol, .git suffix → host/owner/repo
|
||||||
|
case "$ORIGIN_URL" in
|
||||||
|
https://*)
|
||||||
|
REST="${ORIGIN_URL#https://}"
|
||||||
|
;;
|
||||||
|
*)
|
||||||
|
echo "origin is not https (got: $ORIGIN_URL) — this script supports HTTPS Gitea only" >&2
|
||||||
|
exit 3
|
||||||
|
;;
|
||||||
|
esac
|
||||||
|
REST="${REST%.git}"
|
||||||
|
HOST="${REST%%/*}"
|
||||||
|
PATHPART="${REST#*/}" # owner/repo
|
||||||
|
OWNER="${PATHPART%%/*}"
|
||||||
|
REPO="${PATHPART#*/}"
|
||||||
|
REPO="${REPO%%/*}" # belt + braces in case of trailing slash
|
||||||
|
|
||||||
|
# Match the credential line for this host
|
||||||
|
CRED_LINE=$(grep -E "^https://[^@]+@${HOST}\$" "$CRED_FILE" || true)
|
||||||
|
[[ -n "$CRED_LINE" ]] || { echo "no credential for host ${HOST} in ${CRED_FILE}" >&2; exit 3; }
|
||||||
|
|
||||||
|
USER_PART=$(echo "$CRED_LINE" | sed -E 's|^https://([^:]+):.*|\1|')
|
||||||
|
TOKEN=$(echo "$CRED_LINE" | sed -E 's|^https://[^:]+:([^@]+)@.*$|\1|')
|
||||||
|
|
||||||
|
[[ -n "$USER_PART" && -n "$TOKEN" ]] || { echo "could not parse user/token from credential" >&2; exit 3; }
|
||||||
|
|
||||||
|
# --- Branch -----------------------------------------------------------------
|
||||||
|
|
||||||
|
if [[ -z "$HEAD" ]]; then
|
||||||
|
HEAD=$(git rev-parse --abbrev-ref HEAD)
|
||||||
|
fi
|
||||||
|
[[ "$HEAD" != "HEAD" ]] || { echo "detached HEAD — pass --head explicitly" >&2; exit 3; }
|
||||||
|
|
||||||
|
# --- Compose payload --------------------------------------------------------
|
||||||
|
|
||||||
|
API_URL="https://${HOST}/api/v1/repos/${OWNER}/${REPO}/pulls"
|
||||||
|
|
||||||
|
PAYLOAD=$(jq -n \
|
||||||
|
--arg title "$TITLE" \
|
||||||
|
--rawfile body "$BODY_FILE" \
|
||||||
|
--arg head "$HEAD" \
|
||||||
|
--arg base "$BASE" \
|
||||||
|
'{title:$title, body:$body, head:$head, base:$base}')
|
||||||
|
|
||||||
|
# --- POST -------------------------------------------------------------------
|
||||||
|
|
||||||
|
RESPONSE_FILE=$(mktemp)
|
||||||
|
HTTP_CODE=$(curl -sS -u "${USER_PART}:${TOKEN}" \
|
||||||
|
-H "Content-Type: application/json" \
|
||||||
|
-X POST \
|
||||||
|
-d "$PAYLOAD" \
|
||||||
|
-o "$RESPONSE_FILE" \
|
||||||
|
-w "%{http_code}" \
|
||||||
|
"$API_URL")
|
||||||
|
|
||||||
|
if [[ "$HTTP_CODE" != "201" ]]; then
|
||||||
|
echo "PR creation failed (HTTP $HTTP_CODE):" >&2
|
||||||
|
jq -r '.message // empty' "$RESPONSE_FILE" >&2 2>/dev/null || cat "$RESPONSE_FILE" >&2
|
||||||
|
rm -f "$RESPONSE_FILE"
|
||||||
|
exit 4
|
||||||
|
fi
|
||||||
|
|
||||||
|
PR_URL=$(jq -r '.html_url' "$RESPONSE_FILE")
|
||||||
|
PR_NUMBER=$(jq -r '.number' "$RESPONSE_FILE")
|
||||||
|
rm -f "$RESPONSE_FILE"
|
||||||
|
|
||||||
|
echo "Opened PR #${PR_NUMBER}"
|
||||||
|
echo "$PR_URL"
|
||||||
Reference in New Issue
Block a user