From 4fe0494be604d6c36d02ae30254cd8d70f3f2028 Mon Sep 17 00:00:00 2001 From: Tommaso Casaburi Date: Mon, 27 Apr 2026 12:39:15 +0700 Subject: [PATCH] docs(agent rules): require React best-practices review --- AGENTS.md | 5 +- docs/agent-playbooks/hooks-setup.md | 7 +- scripts/agent-hooks/react-pattern-review.sh | 85 +++++++++++++++++---- src/AGENTS.md | 2 +- 4 files changed, 77 insertions(+), 22 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 1c436c26..699c6136 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -26,7 +26,7 @@ Only record items that are repo-specific, likely to recur, and have a concrete m | Situation | Required action | |---|---| -| React UI logic changed (`src/components`, `src/views`, `src/hooks`, UI stores) | Follow React architecture rules below and run `yarn doctor` | +| React UI logic changed (`src/components`, `src/views`, `src/hooks`, UI stores) | Follow React architecture rules below, review the diff with `vercel-react-best-practices` and `vercel:react-best-practices` when available, fix valid findings, then run `yarn doctor` | | `package.json` changed | Run `corepack yarn install` to keep `yarn.lock` in sync | | Dependencies or import graph changed | Run `yarn knip` as an advisory manifest/import audit | | Translation key/value changed | Use `docs/agent-playbooks/translations.md` | @@ -146,7 +146,8 @@ src/ - When changing shared agent behavior, update the relevant files in `.codex/skills/`, `.cursor/skills/`, `.claude/skills/`, `.codex/agents/`, `.cursor/agents/`, `.claude/agents/`, `.codex/hooks/`, `.cursor/hooks/`, `.claude/hooks/`, and their `hooks.json` or config entry points as needed. - If `AGENTS.md` references a skill, agent, or hook, prefer a tracked file under `.codex/`, `.cursor/`, or `.claude/` rather than an untracked local-only instruction. - Review `.codex/config.toml`, `.cursor/hooks.json`, and `.claude/hooks.json` before changing agent orchestration or hook behavior, because they are the entry points contributors will actually load. -- When a diff adds new `useEffect`, `useLayoutEffect`, `useInsertionEffect`, `useMemo`, `useCallback`, or `memo(...)` usage under `src/`, treat the repo hook reminder as mandatory and reconsider the change with `you-might-not-need-an-effect` and `vercel-react-best-practices` before finishing. +- Before finishing any React UI logic change under `src/components`, `src/views`, `src/hooks`, or UI stores, review the changed diff with `vercel-react-best-practices` and, in Codex/Vercel-plugin sessions, `vercel:react-best-practices`. Fix valid findings before final verification; do not limit this review to diffs that add new hooks or memoization. +- When a diff adds new `useEffect`, `useLayoutEffect`, `useInsertionEffect`, `useMemo`, `useCallback`, or `memo(...)` usage under `src/`, treat the repo hook reminder as mandatory and also reconsider the change with `you-might-not-need-an-effect` before finishing. - Directory-specific auto-loaded rules live under `src/AGENTS.md` and `scripts/AGENTS.md`; read them before editing files in those trees. - For work expected to span multiple sessions, keep explicit task state in a `feature-list.json` plus `progress.md` pair using `docs/agent-playbooks/long-running-agent-workflow.md`. - If more than one human or toolchain needs the same task state, keep it in a tracked location such as `docs/agent-runs//` instead of burying it in a tool-specific hidden directory. diff --git a/docs/agent-playbooks/hooks-setup.md b/docs/agent-playbooks/hooks-setup.md index c5a0ea5b..7713f9fe 100644 --- a/docs/agent-playbooks/hooks-setup.md +++ b/docs/agent-playbooks/hooks-setup.md @@ -8,16 +8,17 @@ If your AI coding assistant supports lifecycle hooks, configure these for this r |---|---|---| | `afterFileEdit` | `scripts/agent-hooks/format.sh` | Auto-format files after AI edits | | `afterFileEdit` | `scripts/agent-hooks/yarn-install.sh` | Run `corepack yarn install` when `package.json` changes | -| `afterFileEdit` | `scripts/agent-hooks/react-pattern-review.sh` | When a diff adds `useEffect`/memo primitives in `src/`, remind the agent to reconsider with the React review skills | +| `afterFileEdit` | `scripts/agent-hooks/react-pattern-review.sh` | When React UI source changes, remind the agent to run the React best-practice review skills; also flag new `useEffect`/memo primitives | | `stop` | `scripts/agent-hooks/sync-git-branches.sh` | Prune stale refs and delete integrated temporary task branches | -| `stop` | `scripts/agent-hooks/react-pattern-review.sh` | Re-scan the current diff for new React effects/memos before the final verify gate | +| `stop` | `scripts/agent-hooks/react-pattern-review.sh` | Re-scan the current diff for React UI source changes and new React effects/memos before the final verify gate | | `stop` | `scripts/agent-hooks/verify.sh` | Hard-gate build, lint, and type-check; keep `yarn audit` informational | ## Why - Consistent formatting - Lockfile stays in sync -- New `useEffect`/memo additions get an explicit second look before the agent finishes +- React UI source changes get an explicit best-practices review reminder before the agent finishes +- New `useEffect`/memo additions get an additional effect-specific second look before the agent finishes - Build/lint/type issues caught early - Security visibility via `corepack yarn npm audit` - One shared hook implementation for Codex, Cursor, and Claude diff --git a/scripts/agent-hooks/react-pattern-review.sh b/scripts/agent-hooks/react-pattern-review.sh index 7826ad94..2e8ebc75 100755 --- a/scripts/agent-hooks/react-pattern-review.sh +++ b/scripts/agent-hooks/react-pattern-review.sh @@ -1,6 +1,6 @@ #!/bin/bash -# afterFileEdit/stop hook: remind agents to review new React effects and memoization +# afterFileEdit/stop hook: remind agents to review React UI source changes set -u @@ -61,6 +61,13 @@ matches_scope() { return 1 } +is_react_ui_source_file() { + case "$1" in + src/components/*|src/views/*|src/hooks/*|src/stores/*) return 0 ;; + *) return 1 ;; + esac +} + parse_matches_from_diff() { awk ' /^\+\+\+ b\// { @@ -109,11 +116,25 @@ append_results() { printf '%s\n%s' "$existing" "$incoming" } +append_file_if_react_ui_source() { + local existing="$1" + local file_path="$2" + + if is_source_file "$file_path" && matches_scope "$file_path" && is_react_ui_source_file "$file_path"; then + append_results "$existing" "$file_path" + return + fi + + printf '%s' "$existing" +} + results="" +react_source_files="" file_path="$(extract_file_path)" if [ -n "$file_path" ]; then if is_source_file "$file_path" && matches_scope "$file_path"; then + react_source_files="$(append_file_if_react_ui_source "$react_source_files" "$file_path")" if git ls-files --others --exclude-standard -- "$file_path" | grep -q '.'; then results="$(scan_untracked_file "$file_path")" else @@ -125,18 +146,25 @@ else diff_output="$(git diff --no-ext-diff --unified=0 --no-color HEAD -- '*.js' '*.jsx' '*.ts' '*.tsx' '*.mjs' '*.cjs' 2>/dev/null || true)" results="$(printf '%s\n' "$diff_output" | parse_matches_from_diff)" + while IFS= read -r changed_file; do + [ -z "$changed_file" ] && continue + react_source_files="$(append_file_if_react_ui_source "$react_source_files" "$changed_file")" + done < <(git diff --name-only --diff-filter=ACMRT HEAD -- 'src/components' 'src/views' 'src/hooks' 'src/stores' 2>/dev/null || true) + while IFS= read -r untracked_file; do [ -z "$untracked_file" ] && continue is_source_file "$untracked_file" || continue matches_scope "$untracked_file" || continue + react_source_files="$(append_file_if_react_ui_source "$react_source_files" "$untracked_file")" file_results="$(scan_untracked_file "$untracked_file")" results="$(append_results "$results" "$file_results")" done < <(git ls-files --others --exclude-standard -- '*.js' '*.jsx' '*.ts' '*.tsx' '*.mjs' '*.cjs') fi results="$(printf '%s\n' "$results" | sed '/^$/d' | awk '!seen[$0]++')" +react_source_files="$(printf '%s\n' "$react_source_files" | sed '/^$/d' | awk '!seen[$0]++')" -if [ -z "$results" ]; then +if [ -z "$results" ] && [ -z "$react_source_files" ]; then exit 0 fi @@ -150,26 +178,51 @@ if [ -n "$skill_dir" ] && [ -f "$repo_root/$skill_dir/vercel-react-best-practice vercel_skill="$repo_root/$skill_dir/vercel-react-best-practices/SKILL.md" fi -echo "=== React Hook Review Reminder ===" -echo "New React effect or memo primitives were added in the current diff:" +echo "=== React Best Practices Review Reminder ===" -match_count=0 -while IFS= read -r match_line; do - [ -z "$match_line" ] && continue - match_count=$((match_count + 1)) - if [ "$match_count" -le 10 ]; then - echo "- $match_line" +if [ -n "$react_source_files" ]; then + echo "React UI source changed in the current diff:" + + file_count=0 + while IFS= read -r changed_file; do + [ -z "$changed_file" ] && continue + file_count=$((file_count + 1)) + if [ "$file_count" -le 10 ]; then + echo "- $changed_file" + fi + done <<< "$react_source_files" + + if [ "$file_count" -gt 10 ]; then + echo "- ... and $((file_count - 10)) more" fi -done <<< "$results" -if [ "$match_count" -gt 10 ]; then - echo "- ... and $((match_count - 10)) more" + echo "Before finishing, review the changed diff with:" + echo "- $vercel_skill" + echo "- vercel:react-best-practices, when available in the current harness" +fi + +if [ -n "$results" ]; then + echo "New React effect or memo primitives were also added in the current diff:" + + match_count=0 + while IFS= read -r match_line; do + [ -z "$match_line" ] && continue + match_count=$((match_count + 1)) + if [ "$match_count" -le 10 ]; then + echo "- $match_line" + fi + done <<< "$results" + + if [ "$match_count" -gt 10 ]; then + echo "- ... and $((match_count - 10)) more" + fi + + echo "Also reconsider effect/memo usage with:" + echo "- $effect_skill" fi -echo "Reconsider this change with:" -echo "- $effect_skill" -echo "- $vercel_skill" echo "Questions to resolve before finishing:" +echo "- Does the TSX avoid inline object/array prop churn and unnecessary component work?" echo "- Can this be derived during render instead of synchronized with an effect?" echo "- Can interaction logic move to an event handler or a key-based reset?" echo "- Is the memoization actually needed, or is simpler render-time code better?" diff --git a/src/AGENTS.md b/src/AGENTS.md index 6d7b969b..df28af1e 100644 --- a/src/AGENTS.md +++ b/src/AGENTS.md @@ -5,5 +5,5 @@ These rules apply to `src/**`. Follow the repo-root `AGENTS.md` first, then use - Keep route composition in `src/views/`, reusable UI in `src/components/`, shared logic in `src/hooks/`, and shared app state in `src/stores/`. - Before adding new state, decide whether it belongs in render, a reusable hook, or a Zustand store. Do not duplicate the same state logic across views. - Use `@bitsocial/bitsocial-react-hooks` for data access. Do not add data-fetching `useEffect` calls or effects that only synchronize derived state. -- When changing React UI logic, run `yarn doctor` in addition to build, lint, and type-check. When changing layout or interaction, verify desktop and mobile behavior with `playwright-cli`. +- When changing React UI logic, review the changed diff with `vercel-react-best-practices` and `vercel:react-best-practices` when available before final verification. Run `yarn doctor` in addition to build, lint, and type-check. When changing layout or interaction, verify desktop and mobile behavior with `playwright-cli`. - Prefer extending nearby tests under `src/**/__tests__/` when touching already-covered behavior.