diff --git a/.claude/hooks.json b/.claude/hooks.json index 1b96c07d..b683cd82 100644 --- a/.claude/hooks.json +++ b/.claude/hooks.json @@ -24,6 +24,10 @@ "command": ".claude/hooks/react-pattern-review.sh", "timeout": 10 }, + { + "command": ".claude/hooks/code-quality-review-reminder.sh", + "timeout": 10 + }, { "command": ".claude/hooks/verify.sh", "timeout": 60 diff --git a/.claude/hooks/code-quality-review-reminder.sh b/.claude/hooks/code-quality-review-reminder.sh new file mode 100755 index 00000000..75cf2ee4 --- /dev/null +++ b/.claude/hooks/code-quality-review-reminder.sh @@ -0,0 +1,4 @@ +#!/bin/bash + +repo_root="$(cd "$(dirname "$0")/../.." && pwd)" +exec "$repo_root/scripts/agent-hooks/code-quality-review-reminder.sh" --skill-dir .claude/skills "$@" diff --git a/.claude/skills/code-quality-review/SKILL.md b/.claude/skills/code-quality-review/SKILL.md new file mode 100644 index 00000000..c3a15c6c --- /dev/null +++ b/.claude/skills/code-quality-review/SKILL.md @@ -0,0 +1,38 @@ +--- +name: code-quality-review +description: Advisory pre-push/pre-PR code quality review for current diffs. Use when asked to review code quality, run a pre-PR or pre-push quality pass, inspect AI-generated changes, check for over-engineering, or before pushing/opening a PR. Reports suggestions only; does not block or edit unless explicitly asked. +--- + +# Code Quality Review + +Run an advisory review of the current diff. This is a suggestion pass for the author before push or PR, not a blocking CI gate. + +## Scope + +- If the user supplies a base, review against that base. +- Otherwise review branch changes against `master` plus any staged, unstaged, or untracked files. +- Read `AGENTS.md` and any nested `AGENTS.md` files that cover changed paths. For UI work, also read the repo design guidance named there. +- Skim nearby source before judging a hunk. Do not review from the patch alone when surrounding patterns matter. +- Do not re-run or duplicate deterministic tools such as build, lint, typecheck, React Doctor, or Knip. Mention them only when the diff suggests they are especially relevant. + +## Review Lenses + +1. **Documented standards**: flag clear drift from repo policy, directory rules, design rules, or known-surprises guidance. +2. **Simplicity**: look for speculative abstractions, wrappers with one caller, new configuration nobody sets, avoidable dependencies, boilerplate, or code that a native platform feature, standard library, or existing dependency already covers. +3. **Structure**: look for wrong-layer logic, conditionals growing into state machines, duplicated helpers, file-size sprawl, unclear ownership, or casts and optionality that blur trust boundaries. +4. **Interface and tests**: prefer deep modules with small useful interfaces over shallow pass-through modules. Tests should exercise public behavior and survive internal refactors. Risky logic should have a fast feedback loop. +5. **Scope control**: identify behavior that was not requested, changes unrelated to the task, or cleanup that should be split out. + +## Output + +- If there are no high-confidence findings, say: `No high-confidence advisory findings. Ship.` +- Otherwise report at most 8 findings, ordered by expected payoff. +- Use this format: `[standard|simplicity|structure|testability|scope] path:line - Finding. Suggestion. Confidence: high|medium.` +- Keep findings specific and actionable. Skip nits, style preferences, and anything the repo tooling already handles. +- End with `Advisory only; not a blocker.` + +## Boundaries + +- Do not suggest deleting tests, accessibility, security checks, trust-boundary validation, data-loss protection, or error handling unless you provide an equally safe simpler replacement. +- Respect documented product constraints and historical decisions. If a suggestion appears to contradict an ADR, known surprise, or explicit repo policy, call that out instead of presenting it as a straightforward cleanup. +- Do not edit files unless the user explicitly asks you to apply the review findings. diff --git a/.codex/hooks.json b/.codex/hooks.json index 483fc302..45e8d465 100644 --- a/.codex/hooks.json +++ b/.codex/hooks.json @@ -24,6 +24,10 @@ "command": ".codex/hooks/react-pattern-review.sh", "timeout": 10 }, + { + "command": ".codex/hooks/code-quality-review-reminder.sh", + "timeout": 10 + }, { "command": ".codex/hooks/verify.sh", "timeout": 60 diff --git a/.codex/hooks/code-quality-review-reminder.sh b/.codex/hooks/code-quality-review-reminder.sh new file mode 100755 index 00000000..55cee6a7 --- /dev/null +++ b/.codex/hooks/code-quality-review-reminder.sh @@ -0,0 +1,4 @@ +#!/bin/bash + +repo_root="$(cd "$(dirname "$0")/../.." && pwd)" +exec "$repo_root/scripts/agent-hooks/code-quality-review-reminder.sh" --skill-dir .codex/skills "$@" diff --git a/.codex/skills/code-quality-review/SKILL.md b/.codex/skills/code-quality-review/SKILL.md new file mode 100644 index 00000000..c3a15c6c --- /dev/null +++ b/.codex/skills/code-quality-review/SKILL.md @@ -0,0 +1,38 @@ +--- +name: code-quality-review +description: Advisory pre-push/pre-PR code quality review for current diffs. Use when asked to review code quality, run a pre-PR or pre-push quality pass, inspect AI-generated changes, check for over-engineering, or before pushing/opening a PR. Reports suggestions only; does not block or edit unless explicitly asked. +--- + +# Code Quality Review + +Run an advisory review of the current diff. This is a suggestion pass for the author before push or PR, not a blocking CI gate. + +## Scope + +- If the user supplies a base, review against that base. +- Otherwise review branch changes against `master` plus any staged, unstaged, or untracked files. +- Read `AGENTS.md` and any nested `AGENTS.md` files that cover changed paths. For UI work, also read the repo design guidance named there. +- Skim nearby source before judging a hunk. Do not review from the patch alone when surrounding patterns matter. +- Do not re-run or duplicate deterministic tools such as build, lint, typecheck, React Doctor, or Knip. Mention them only when the diff suggests they are especially relevant. + +## Review Lenses + +1. **Documented standards**: flag clear drift from repo policy, directory rules, design rules, or known-surprises guidance. +2. **Simplicity**: look for speculative abstractions, wrappers with one caller, new configuration nobody sets, avoidable dependencies, boilerplate, or code that a native platform feature, standard library, or existing dependency already covers. +3. **Structure**: look for wrong-layer logic, conditionals growing into state machines, duplicated helpers, file-size sprawl, unclear ownership, or casts and optionality that blur trust boundaries. +4. **Interface and tests**: prefer deep modules with small useful interfaces over shallow pass-through modules. Tests should exercise public behavior and survive internal refactors. Risky logic should have a fast feedback loop. +5. **Scope control**: identify behavior that was not requested, changes unrelated to the task, or cleanup that should be split out. + +## Output + +- If there are no high-confidence findings, say: `No high-confidence advisory findings. Ship.` +- Otherwise report at most 8 findings, ordered by expected payoff. +- Use this format: `[standard|simplicity|structure|testability|scope] path:line - Finding. Suggestion. Confidence: high|medium.` +- Keep findings specific and actionable. Skip nits, style preferences, and anything the repo tooling already handles. +- End with `Advisory only; not a blocker.` + +## Boundaries + +- Do not suggest deleting tests, accessibility, security checks, trust-boundary validation, data-loss protection, or error handling unless you provide an equally safe simpler replacement. +- Respect documented product constraints and historical decisions. If a suggestion appears to contradict an ADR, known surprise, or explicit repo policy, call that out instead of presenting it as a straightforward cleanup. +- Do not edit files unless the user explicitly asks you to apply the review findings. diff --git a/.cursor/hooks.json b/.cursor/hooks.json index a0b1a9ff..55298aaa 100644 --- a/.cursor/hooks.json +++ b/.cursor/hooks.json @@ -24,6 +24,10 @@ "command": ".cursor/hooks/react-pattern-review.sh", "timeout": 10 }, + { + "command": ".cursor/hooks/code-quality-review-reminder.sh", + "timeout": 10 + }, { "command": ".cursor/hooks/verify.sh", "timeout": 60 diff --git a/.cursor/hooks/code-quality-review-reminder.sh b/.cursor/hooks/code-quality-review-reminder.sh new file mode 100755 index 00000000..7666bd5f --- /dev/null +++ b/.cursor/hooks/code-quality-review-reminder.sh @@ -0,0 +1,4 @@ +#!/bin/bash + +repo_root="$(cd "$(dirname "$0")/../.." && pwd)" +exec "$repo_root/scripts/agent-hooks/code-quality-review-reminder.sh" --skill-dir .cursor/skills "$@" diff --git a/.cursor/skills/code-quality-review/SKILL.md b/.cursor/skills/code-quality-review/SKILL.md new file mode 100644 index 00000000..c3a15c6c --- /dev/null +++ b/.cursor/skills/code-quality-review/SKILL.md @@ -0,0 +1,38 @@ +--- +name: code-quality-review +description: Advisory pre-push/pre-PR code quality review for current diffs. Use when asked to review code quality, run a pre-PR or pre-push quality pass, inspect AI-generated changes, check for over-engineering, or before pushing/opening a PR. Reports suggestions only; does not block or edit unless explicitly asked. +--- + +# Code Quality Review + +Run an advisory review of the current diff. This is a suggestion pass for the author before push or PR, not a blocking CI gate. + +## Scope + +- If the user supplies a base, review against that base. +- Otherwise review branch changes against `master` plus any staged, unstaged, or untracked files. +- Read `AGENTS.md` and any nested `AGENTS.md` files that cover changed paths. For UI work, also read the repo design guidance named there. +- Skim nearby source before judging a hunk. Do not review from the patch alone when surrounding patterns matter. +- Do not re-run or duplicate deterministic tools such as build, lint, typecheck, React Doctor, or Knip. Mention them only when the diff suggests they are especially relevant. + +## Review Lenses + +1. **Documented standards**: flag clear drift from repo policy, directory rules, design rules, or known-surprises guidance. +2. **Simplicity**: look for speculative abstractions, wrappers with one caller, new configuration nobody sets, avoidable dependencies, boilerplate, or code that a native platform feature, standard library, or existing dependency already covers. +3. **Structure**: look for wrong-layer logic, conditionals growing into state machines, duplicated helpers, file-size sprawl, unclear ownership, or casts and optionality that blur trust boundaries. +4. **Interface and tests**: prefer deep modules with small useful interfaces over shallow pass-through modules. Tests should exercise public behavior and survive internal refactors. Risky logic should have a fast feedback loop. +5. **Scope control**: identify behavior that was not requested, changes unrelated to the task, or cleanup that should be split out. + +## Output + +- If there are no high-confidence findings, say: `No high-confidence advisory findings. Ship.` +- Otherwise report at most 8 findings, ordered by expected payoff. +- Use this format: `[standard|simplicity|structure|testability|scope] path:line - Finding. Suggestion. Confidence: high|medium.` +- Keep findings specific and actionable. Skip nits, style preferences, and anything the repo tooling already handles. +- End with `Advisory only; not a blocker.` + +## Boundaries + +- Do not suggest deleting tests, accessibility, security checks, trust-boundary validation, data-loss protection, or error handling unless you provide an equally safe simpler replacement. +- Respect documented product constraints and historical decisions. If a suggestion appears to contradict an ADR, known surprise, or explicit repo policy, call that out instead of presenting it as a straightforward cleanup. +- Do not edit files unless the user explicitly asks you to apply the review findings. diff --git a/AGENTS.md b/AGENTS.md index 73f98c01..64e0ffaf 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -65,6 +65,7 @@ When CodeGraph MCP tools are available and `.codegraph/` exists, prefer them for | New reviewable feature/fix started while on `master` | Create a short-lived `codex/feature/*`, `codex/fix/*`, `codex/docs/*`, or `codex/chore/*` branch from `master` before editing; use a separate worktree only for parallel tasks | | New unrelated task started while another task branch is already checked out or being worked on by another agent | Create a separate worktree from `master`, create a new short-lived task branch there, and keep each agent on its own worktree/branch/PR | | Open PR needs feedback triage or merge readiness check | Use the `review-and-merge-pr` skill to inspect bot/human feedback, fix valid findings, and merge only after verification | +| Before pushing or opening a PR with code, docs, or AI workflow changes | Run the advisory `code-quality-review` skill on the current diff; treat findings as suggestions, not blockers, and address only high-confidence improvements | | Repo AI workflow files changed (`.codex/**`, `.cursor/**`, `.claude/**`) | Keep the Codex, Cursor, and Claude copies aligned when they represent the same workflow; run `yarn ai-workflow:check` to catch parity and drift issues; update `AGENTS.md` if the default agent policy changes | | GitHub operation needed | Use `gh` CLI, not GitHub MCP | | User asks for commit/issue phrasing | Use `docs/agent-playbooks/commit-issue-format.md` | diff --git a/public/llms-full.txt b/public/llms-full.txt index b302c3a3..43800638 100644 --- a/public/llms-full.txt +++ b/public/llms-full.txt @@ -246,6 +246,7 @@ When CodeGraph MCP tools are available and `.codegraph/` exists, prefer them for | New reviewable feature/fix started while on `master` | Create a short-lived `codex/feature/*`, `codex/fix/*`, `codex/docs/*`, or `codex/chore/*` branch from `master` before editing; use a separate worktree only for parallel tasks | | New unrelated task started while another task branch is already checked out or being worked on by another agent | Create a separate worktree from `master`, create a new short-lived task branch there, and keep each agent on its own worktree/branch/PR | | Open PR needs feedback triage or merge readiness check | Use the `review-and-merge-pr` skill to inspect bot/human feedback, fix valid findings, and merge only after verification | +| Before pushing or opening a PR with code, docs, or AI workflow changes | Run the advisory `code-quality-review` skill on the current diff; treat findings as suggestions, not blockers, and address only high-confidence improvements | | Repo AI workflow files changed (`.codex/**`, `.cursor/**`, `.claude/**`) | Keep the Codex, Cursor, and Claude copies aligned when they represent the same workflow; run `yarn ai-workflow:check` to catch parity and drift issues; update `AGENTS.md` if the default agent policy changes | | GitHub operation needed | Use `gh` CLI, not GitHub MCP | | User asks for commit/issue phrasing | Use `docs/agent-playbooks/commit-issue-format.md` | diff --git a/scripts/agent-hooks/code-quality-review-reminder.sh b/scripts/agent-hooks/code-quality-review-reminder.sh new file mode 100755 index 00000000..89013ebe --- /dev/null +++ b/scripts/agent-hooks/code-quality-review-reminder.sh @@ -0,0 +1,122 @@ +#!/bin/bash + +# stop hook: remind agents to run the advisory code-quality review before push/PR + +set -u + +cat > /dev/null + +skill_dir="" + +while [ "$#" -gt 0 ]; do + case "$1" in + --skill-dir) + skill_dir="${2:-}" + shift 2 + ;; + *) + shift + ;; + esac +done + +repo_root="$(git rev-parse --show-toplevel 2>/dev/null || pwd)" +cd "$repo_root" || exit 0 + +default_branch="$(git symbolic-ref --quiet --short refs/remotes/origin/HEAD 2>/dev/null | sed 's#^origin/##')" +if [ -z "$default_branch" ]; then + default_branch="master" +fi + +current_branch="$(git branch --show-current 2>/dev/null || true)" + +is_generated_or_lock_file() { + case "$1" in + yarn.lock|package-lock.json|pnpm-lock.yaml|bun.lockb|dist/*|build/*|coverage/*|node_modules/*|public/llms*.txt|about/public/llms*.txt|docs/static/llms*.txt|*.map) return 0 ;; + *) return 1 ;; + esac +} + +is_reviewable_file() { + case "$1" in + *.js|*.jsx|*.ts|*.tsx|*.mjs|*.cjs|*.css|*.json|*.md|*.yml|*.yaml|*.toml|*.sh|Dockerfile) return 0 ;; + *) return 1 ;; + esac +} + +append_file() { + local existing="$1" + local file_path="$2" + + [ -n "$file_path" ] || { + printf '%s' "$existing" + return + } + + is_generated_or_lock_file "$file_path" && { + printf '%s' "$existing" + return + } + + is_reviewable_file "$file_path" || { + printf '%s' "$existing" + return + } + + if [ -z "$existing" ]; then + printf '%s' "$file_path" + return + fi + + printf '%s\n%s' "$existing" "$file_path" +} + +changed_files="" + +while IFS= read -r changed_file; do + changed_files="$(append_file "$changed_files" "$changed_file")" +done < <(git diff --name-only --diff-filter=ACMRT HEAD -- 2>/dev/null || true) + +if [ -n "$current_branch" ] && [ "$current_branch" != "$default_branch" ] && git rev-parse --verify "$default_branch" >/dev/null 2>&1; then + while IFS= read -r changed_file; do + changed_files="$(append_file "$changed_files" "$changed_file")" + done < <(git diff --name-only --diff-filter=ACMRT "$default_branch"...HEAD -- 2>/dev/null || true) +fi + +while IFS= read -r changed_file; do + changed_files="$(append_file "$changed_files" "$changed_file")" +done < <(git ls-files --others --exclude-standard) + +changed_files="$(printf '%s\n' "$changed_files" | sed '/^$/d' | awk '!seen[$0]++')" + +if [ -z "$changed_files" ]; then + exit 0 +fi + +skill_ref="code-quality-review" +if [ -n "$skill_dir" ] && [ -f "$repo_root/$skill_dir/code-quality-review/SKILL.md" ]; then + skill_ref="$repo_root/$skill_dir/code-quality-review/SKILL.md" +fi + +echo "=== Advisory Code Quality Review Reminder ===" +echo "Reviewable files changed in this branch or working tree:" + +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 <<< "$changed_files" + +if [ "$file_count" -gt 10 ]; then + echo "- ... and $((file_count - 10)) more" +fi + +echo "Before pushing or opening a PR, run the advisory review:" +echo "- $skill_ref" +echo "Use it for documented standards, avoidable complexity, structure, interface/testability, and scope control." +echo "Advisory only; not a blocker." + +exit 0