mirror of
https://github.com/bitsocialnet/5chan.git
synced 2026-08-03 07:41:04 +02:00
chore: add advisory code quality review workflow
This commit is contained in:
Tommaso Casaburi
@@ -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
|
||||
|
||||
+4
@@ -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 "$@"
|
||||
@@ -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.
|
||||
@@ -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
|
||||
|
||||
Executable
+4
@@ -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 "$@"
|
||||
@@ -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.
|
||||
@@ -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
|
||||
|
||||
+4
@@ -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 "$@"
|
||||
@@ -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.
|
||||
@@ -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` |
|
||||
|
||||
@@ -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` |
|
||||
|
||||
+122
@@ -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
|
||||
Reference in New Issue
Block a user