From 0f6ae857b353ec6789e3396e0d73354ed5d37646 Mon Sep 17 00:00:00 2001 From: Knacky Date: Wed, 27 May 2026 19:41:34 +0200 Subject: [PATCH] feat(infra): design-reviewer agent + PR helper (US-24 + US-25) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- .claude/agents/design-reviewer.md | 85 ++++++++++++++++++ .claude/agents/frontend-builder.md | 16 ++++ Makefile | 14 ++- README.md | 1 + scripts/open-pr.sh | 136 +++++++++++++++++++++++++++++ 5 files changed, 251 insertions(+), 1 deletion(-) create mode 100644 .claude/agents/design-reviewer.md create mode 100755 scripts/open-pr.sh diff --git a/.claude/agents/design-reviewer.md b/.claude/agents/design-reviewer.md new file mode 100644 index 0000000..9afc862 --- /dev/null +++ b/.claude/agents/design-reviewer.md @@ -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 ...HEAD -- frontend/ +git diff ...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 + +### 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. diff --git a/.claude/agents/frontend-builder.md b/.claude/agents/frontend-builder.md index 61cefd5..4c52a72 100644 --- a/.claude/agents/frontend-builder.md +++ b/.claude/agents/frontend-builder.md @@ -44,6 +44,21 @@ cd frontend && npm run test -- --run 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) A short Markdown summary: @@ -53,6 +68,7 @@ A short Markdown summary: - **Mismatches with API** (if any — flagged, not patched) - **Open questions / design ambiguities** (escalate, don't decide) - **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** ## Principles diff --git a/Makefile b/Makefile index e701465..1a623eb 100644 --- a/Makefile +++ b/Makefile @@ -7,7 +7,7 @@ VOLUME ?= mimic-data # Override explicitly with `make 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) -.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: $(CONTAINER_CMD) build -f docker/Dockerfile -t $(IMAGE) . @@ -60,3 +60,15 @@ clean: -$(CONTAINER_CMD) rm -f $(CONTAINER) 2>/dev/null -$(CONTAINER_CMD) volume rm $(VOLUME) 2>/dev/null 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)" diff --git a/README.md b/README.md index 7a35150..900e8fa 100644 --- a/README.md +++ b/README.md @@ -110,6 +110,7 @@ mimic/ | `make test-frontend` | `npm run test -- --run` in `frontend/` | | `make test-e2e` | Playwright acceptance suite (container must be running) | | `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`. | --- diff --git a/scripts/open-pr.sh b/scripts/open-pr.sh new file mode 100755 index 0000000..c140c9a --- /dev/null +++ b/scripts/open-pr.sh @@ -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 ] +# +# 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"