diff --git a/.claude/commands/review.md b/.claude/commands/review.md new file mode 100644 index 00000000..cabde7be --- /dev/null +++ b/.claude/commands/review.md @@ -0,0 +1,60 @@ +--- +description: PR 을 올리기 전 로컬 리뷰. 기계 검사 + 영역별 리뷰 에이전트 +--- + +PR 을 올리기 전에 **CodeRabbit 이 볼 것을 먼저 본다.** 원격에서 지적받고 고치는 +왕복은 비싸고, 무엇보다 그 사이에 잘못된 코드가 브랜치에 남는다. + +## 1. 기계 검사 + +```bash +.claude/hooks/review-branch.sh ${1:-origin/develop} +``` + +이 스크립트는 `check-java.sh`·`check-lua.sh` 를 **브랜치 변경 전체**에 태운다. +두 훅은 `Write|Edit` 도구로 쓴 파일만 보므로, 힙독이나 스크립트로 만든 파일은 +그냥 지나간다 — 실제로 그렇게 들어간 위반이 CodeRabbit 까지 간 적이 있다. + +위반이 나오면 **고치고 다시 돌린다.** 통과할 때까지 2번으로 넘어가지 않는다. + +## 2. 빌드와 품질 임계 + +```bash +./gradlew build jacocoTestCoverageVerification pitest --no-daemon -q +``` + +도메인 분기 100%, 뮤테이션 생존 ≤10%. 생존 뮤턴트가 나오면 **숫자만 보지 말고 +어디가 살아남았는지** 본다 — 임계를 통과해도 이미 버그가 났던 자리에 몰려 +있으면 그건 통과가 아니다. + +## 3. 영역별 리뷰 에이전트 + +변경 경로에 따라 `.claude/agents/` 에서 고른다. 1번 스크립트가 마지막에 +어느 에이전트를 돌릴지 알려준다. + +| 변경 | 에이전트 | +|---|---| +| `domain/**` | `domain-guardian` | +| `*.lua` · `redis/**` | `redis-cluster-checker` | +| 장애·회복·서킷·리트라이 | `resilience-auditor` | +| `src/test/**` · `src/testFixtures/**` | `test-quality-reviewer` | +| 전 영역 (항상) | `style-enforcer` | + +각 에이전트에게 **변경 범위를 명시**해서 넘긴다 — 저장소 전체를 훑게 하면 +이번 변경과 무관한 지적이 섞여 진짜 지적이 묻힌다. + +## 4. 사람이 볼 것 + +기계도 에이전트도 못 보는 것이 남는다. + +- **판정 순서를 바꿨다면** 계획서의 사다리 표도 함께 고쳤는가 (PK-A4) +- **계획서와 구현이 어긋난다면** 어느 쪽이 목표에 가까운가. 계획서가 스스로 + 모순인 경우가 있다 (AIJ-0012) +- **테스트 이름이 실제로 검증하는 것과 같은가.** 이름만 맞고 내용이 다른 + 테스트는 통과하면서 버그를 덮는다 + +## 5. 그다음 + +- 저널이 필요한 변경인가 — `src/**`·`*.gradle`·`.github/workflows/**` +- 커밋 제목이 50칸 이내 명사형인가, 푸터에 `Refs: CY-###` 가 있는가 +- 푸시 전에 **무엇을 바꿨는지 요약**한다 diff --git a/.claude/hooks/check-java.sh b/.claude/hooks/check-java.sh index fa296562..1162d8e7 100755 --- a/.claude/hooks/check-java.sh +++ b/.claude/hooks/check-java.sh @@ -90,10 +90,30 @@ fi # ── JS-14 중첩 클래스는 static ──────────────────────────────────────────────── # 들여쓰기된 class 선언 = 중첩. 수식어가 없는 경우도 잡는다. -report "JS-14" "중첩 클래스는 static — 바깥 인스턴스를 붙들어 누수를 만든다" \ - "$(scan 'JS-14' \ - '^[0-9]+:[[:space:]]+((public|protected|private|final|abstract)[[:space:]]+)*class[[:space:]]' \ - 'static')" +# +# **JUnit 5 의 @Nested 는 제외한다.** 그쪽은 static 이면 아예 실행되지 않는다 — +# 규칙과 프레임워크가 충돌하는 자리라 규칙이 진다. 여기서 오탐을 내면 사람은 +# 훅을 고치는 대신 우회하고, 그러면 진짜 위반도 같이 지나간다. +# @Nested 이후 **선언까지의 연속 어노테이션 줄**을 전부 건너뛴다. 개수를 못 +# 박으면 @Tag 하나 붙는 순간 오탐이 되고, 오탐이 나면 훅이 우회된다. +nested_class_lines=$(awk ' + # `@Nested class Inner {` 처럼 한 줄에 같이 오면 그 줄이 곧 선언이다. + # pending 을 켠 채 넘어가면 **다음 중첩 클래스가 대신 면제된다.** + /^[[:space:]]*@Nested([[:space:]]|\(|$)/ && /class[[:space:]]/ { print NR; pending=0; next } + /^[[:space:]]*@Nested([[:space:]]|\(|$)/ { pending=1; next } + # 어노테이션 인자가 여러 줄에 걸치면 이어지는 줄은 @ 로 시작하지 않는다. + # 개수나 형태를 못 박지 말고 **선언 줄을 만날 때까지** 건너뛴다. + pending && /class[[:space:]]/ { print NR; pending=0; next } + pending { next } +' "$file") +js14=$(scan 'JS-14' \ + '^[0-9]+:[[:space:]]+((public|protected|private|final|abstract)[[:space:]]+)*class[[:space:]]' \ + 'static') +for n in $nested_class_lines; do + js14=$(printf '%s\n' "$js14" | grep -vE "^[[:space:]]*$n:") +done +js14=$(printf '%s' "$js14" | grep -v '^[[:space:]]*$') +report "JS-14" "중첩 클래스는 static — 바깥 인스턴스를 붙들어 누수를 만든다" "$js14" # ── JS-6 Javadoc 5줄 초과 (원본에서 검사한다) ───────────────────────────────── hits=$(awk ' diff --git a/.claude/hooks/guard-pr.sh b/.claude/hooks/guard-pr.sh new file mode 100755 index 00000000..d166c660 --- /dev/null +++ b/.claude/hooks/guard-pr.sh @@ -0,0 +1,76 @@ +#!/usr/bin/env bash +# PR 을 올리기 전에 로컬 리뷰를 강제한다. +# +# **왜 차단인가.** "올리기 전에 돌려라" 는 규범은 잊힌다. 실제로 잊었고, +# CodeRabbit 이 두 라운드에 걸쳐 14건을 지적했는데 그중 셋은 우리 자신의 +# MUST 규칙 위반이었다 — 로컬에서 1초면 잡히는 것들이다. +# +# PreToolUse(Bash) 훅. `gh pr create` 를 만나면 기계 검사를 돌리고 +# 위반이 있으면 exit 2 로 막는다. + +set -uo pipefail + +input=$(cat) +cmd=$(printf '%s' "$input" | jq -r '.tool_input.command // empty') + +# PR 생성이 아니면 통과 +[[ "$cmd" != *"gh pr create"* ]] && exit 0 + +# **검사를 못 돌리면 막는다.** 통과시키면 게이트가 인프라 오류 한 번에 +# 조용히 사라진다 — 가드는 fail closed 여야 한다. +if ! ROOT=$(git rev-parse --show-toplevel 2>/dev/null); then + echo "git 저장소가 아니라 로컬 리뷰를 돌릴 수 없다. PR 은 저장소 안에서 연다." >&2 + exit 2 +fi +RUNNER="$ROOT/.claude/hooks/review-branch.sh" +if [[ ! -x "$RUNNER" ]]; then + echo "로컬 리뷰 러너를 실행할 수 없다: $RUNNER" >&2 + echo " chmod +x .claude/hooks/*.sh" >&2 + exit 2 +fi + +# base 를 명령에서 뽑는다. 없으면 develop +# **명령 문자열 전체를 훑지 않는다.** `--title "--base release"` 처럼 인용부호 +# 안에 들어간 값을 옵션으로 착각한다. 인자를 토큰으로 쪼갠 뒤 옵션 자리만 본다. +# +# 실행하지 않고 쪼갠다 — `xargs` 는 셸 인용 규칙을 그대로 따르면서 명령을 +# 부르지 않는다. +base="" +mapfile -t args < <(printf '%s' "$cmd" | xargs -n1 printf '%s\n' 2>/dev/null) +for ((i = 0; i < ${#args[@]}; i++)); do + case "${args[i]}" in + --base=*) base="${args[i]#--base=}"; break ;; + -B=*) base="${args[i]#-B=}"; break ;; + --base|-B) + base="${args[i + 1]:-}" + break ;; + esac +done +base="${base:-develop}" +# 이미 접두가 붙어 있으면 겹치지 않게 둔다. origin/origin/develop 이 되면 +# 러너가 폴백을 타고, 폴백마저 없으면 브랜치 커밋을 하나도 안 보고 통과한다. +[[ "$base" != origin/* ]] && base="origin/$base" + +out=$("$RUNNER" "$base" 2>&1) +status=$? + +if ((status != 0)); then + { + echo "PR 을 올리기 전에 로컬 리뷰가 통과해야 한다." + echo + # 전체를 보여 준다. 걸러내면 정작 필요한 줄이 빠진다. + printf '%s\n' "$out" + echo + echo "고친 뒤 다시 시도한다. 수동 실행: .claude/hooks/review-branch.sh $base" + echo "전체 절차: /review" + } >&2 + exit 2 +fi + +# 통과했어도 기계가 못 보는 것이 남는다 — 막지는 않고 알린다. +{ + echo "로컬 기계 검사 통과. 아직 안 한 것이 있는지 본다:" + echo " · ./gradlew build jacocoTestCoverageVerification pitest" + printf '%s\n' "$out" | sed -n '/사람·에이전트가 볼 것/,$p' | sed 's/^/ /' +} >&2 +exit 0 diff --git a/.claude/hooks/review-branch.sh b/.claude/hooks/review-branch.sh new file mode 100755 index 00000000..860a36e6 --- /dev/null +++ b/.claude/hooks/review-branch.sh @@ -0,0 +1,252 @@ +#!/usr/bin/env bash +# 브랜치 전체 로컬 리뷰. PR 을 올리기 전에 돌린다. +# +# **왜 필요한가.** check-java.sh · check-lua.sh 는 PostToolUse(Write|Edit) 훅이라 +# 그 도구로 쓴 파일만 본다. 힙독이나 스크립트로 쓴 파일은 훅을 통째로 지나간다 — +# 실제로 그렇게 들어간 JS-6·JS-12·JS-13 위반이 CodeRabbit 까지 갔다. +# +# 이 스크립트는 **파일을 어떻게 만들었든** 브랜치의 변경 전체를 같은 검사에 태운다. +# 검사 내용을 여기 복사하지 않고 기존 훅을 그대로 호출한다 — 사본이 생기면 갈라진다. +# +# 사용: .claude/hooks/review-branch.sh [base] 기본 base 는 origin/develop +# .claude/hooks/review-branch.sh --self-test 검사가 실제로 무는지 확인 + +set -uo pipefail +cd "$(git rev-parse --show-toplevel)" || exit 1 + +HOOKS=".claude/hooks" +findings=0 + +say() { printf '%s\n' "$*"; } +head2() { printf '\n\033[1m%s\033[0m\n' "$*"; } + +# 기존 훅을 그 훅의 입력 형식으로 호출한다. +run_hook() { + local hook=$1 file=$2 out + # **절대경로로 넘긴다.** 훅은 `*/src/test/*` 로 테스트 여부를 가르는데 + # git 은 선행 슬래시 없는 상대경로를 준다 — 그대로 넘기면 테스트가 + # 프로덕션 규칙으로 검사되어 오탐이 나고, 동시에 테스트 전용 검사는 + # 아예 안 돈다. 이미 절대경로면 그대로 둔다. + [[ "$file" != /* ]] && file="$PWD/$file" + out=$(jq -nc --arg f "$file" '{tool_input:{file_path:$f}}' \ + | "$HOOKS/$hook" 2>&1) + [[ -n "$out" ]] && { printf '%s\n' "$out"; return 1; } + return 0 +} + +# ── .coderabbit.yaml 의 path_instructions 중 기계로 볼 수 있는 것 ───────────── +# 나머지(설계 의도·판정 순서의 타당성)는 .claude/agents/ 가 본다. + +# JS-12 · 값/상태 객체의 public 생성자 (class 한정. record 는 언어 제약) +check_js12() { + local file=$1 + # **주석을 걷어낸 뒤 실제 선언만 본다.** 파일 전문을 훑으면 `// record Service(` + # 같은 주석 한 줄이 진짜 위반을 면제해 버린다. + local code + code=$(awk '{ + line = $0 + sub(/\/\/.*/, "", line) + if (line ~ /^[[:space:]]*\*/) line = "" + if (line ~ /^[[:space:]]*\/\*/) line = "" + printf "%d:%s\n", NR, line + }' "$file") + + printf '%s\n' "$code" \ + | grep -E '^[0-9]+:[[:space:]]*public [A-Z][A-Za-z0-9_]*\(' \ + | while IFS=: read -r n rest; do + local name + name=$(printf '%s' "$rest" | sed -E 's/^[[:space:]]*public[[:space:]]+([A-Za-z0-9_]+).*/\1/') + # 그 이름의 타입이 record 로 선언됐으면 정규 생성자라 막을 수 없다 + if printf '%s\n' "$code" \ + | grep -qE "^[0-9]+:.*(^|[[:space:]])record[[:space:]]+$name([[:space:]]|\()"; then + continue + fi + printf ' JS-12 %s:%s public 생성자 — 정적 팩토리를 쓴다\n' "$file" "$n" + done +} + +# TS-11 · 약한 단언 +check_ts11() { + local file=$1 + grep -nE '\.(isNotEmpty|isNotNull|isNotZero)\(\)\s*;' "$file" 2>/dev/null \ + | sed "s|^| TS-11 $file:|;s|:\s*| |" \ + | sed 's/$/ ← 무엇이 들어 있는지까지 단언한다/' +} + +# TS-4 · 실제 시간 의존 / TS-7 · 비활성 테스트 +check_test_misc() { + local file=$1 + grep -nE 'Thread\.sleep|Instant\.now\(\)|System\.currentTimeMillis' "$file" 2>/dev/null \ + | sed "s|^| TS-4 $file:|" | sed 's/$/ ← 시각·대기를 주입한다/' + grep -nE '@Disabled|@RepeatedTest\(.*\)\s*//.*(불안정|flaky)' "$file" 2>/dev/null \ + | sed "s|^| TS-7 $file:|" | sed 's/$/ ← 불안정 테스트를 덮지 않는다/' +} + +# EX-1 · 정상 실패를 예외로 +check_ex1() { + local file=$1 + grep -nE 'throw new .*(SoldOut|QueueFull|Overload).*Exception' "$file" 2>/dev/null \ + | sed "s|^| EX-1 $file:|" | sed 's/$/ ← 매진·큐 상한은 판정값이지 예외가 아니다/' +} + +# RX-2 · Flux.interval +check_rx2() { + local file=$1 + grep -nE 'Flux\.interval\(' "$file" 2>/dev/null \ + | sed "s|^| RX-2 $file:|" | sed 's/$/ ← repeatWhen 을 쓴다. 회복 시 몰아서 터진다/' +} + +review_files() { + local -a files=("$@") + local java=() lua=() + for f in "${files[@]}"; do + [[ -f "$f" ]] || continue + case "$f" in + *.java) java+=("$f") ;; + *.lua) lua+=("$f") ;; + esac + done + + if ((${#java[@]})); then + head2 "Java — 기존 훅 (JS-1·2·4·6·9·13·14 · DS-1 · RX-1 · LG-5·6)" + local clean=1 + for f in "${java[@]}"; do run_hook check-java.sh "$f" || clean=0; done + ((clean)) && say " 위반 없음" + ((clean)) || findings=$((findings + 1)) + + head2 "Java — 훅이 안 보는 것 (.coderabbit.yaml path_instructions)" + local out="" + for f in "${java[@]}"; do + case "$f" in + src/test/*|src/testFixtures/*|*/src/test/*|*/src/testFixtures/*) + out+=$(check_ts11 "$f")$'\n' + out+=$(check_test_misc "$f")$'\n' ;; + *) + out+=$(check_js12 "$f")$'\n' + out+=$(check_ex1 "$f")$'\n' + out+=$(check_rx2 "$f")$'\n' ;; + esac + done + out=$(printf '%s' "$out" | grep -v '^[[:space:]]*$') + if [[ -n "$out" ]]; then say "$out"; findings=$((findings + 1)); else say " 위반 없음"; fi + fi + + if ((${#lua[@]})); then + head2 "Lua — RD-1·2·10" + local clean=1 + for f in "${lua[@]}"; do run_hook check-lua.sh "$f" || clean=0; done + ((clean)) && say " 위반 없음" || findings=$((findings + 1)) + fi +} + +# ── 자기검증 — 통과만 하는 검사는 검사가 아니다 ────────────────────────────── +self_test() { + local tmp; tmp=$(mktemp -d); local fail=0 + probe() { # 이름, 경로, 내용, 기대규칙 + local name=$1 path=$2 body=$3 want=$4 + mkdir -p "$(dirname "$tmp/$path")"; printf '%s' "$body" > "$tmp/$path" + local got; got=$(review_files "$tmp/$path" 2>&1) + if printf '%s' "$got" | grep -q "$want"; then + printf ' ✓ %s\n' "$name" + else + printf ' ✗ %s — %s 를 못 잡았다\n' "$name" "$want"; fail=1 + fi + } + probe "public 생성자" "src/main/java/A.java" \ + $'class A {\n public A(int x) {}\n}\n' "JS-12" + probe "Javadoc 6줄 (check-java.sh 위임)" "src/main/java/B.java" \ + $'/**\n * 1\n * 2\n * 3\n * 4\n * 5\n * 6\n */\nclass B {}\n' "JS-6" + probe "약한 단언" "src/test/java/CTest.java" \ + $'class CTest { void t() { assertThat(x).isNotEmpty(); } }\n' "TS-11" + probe "실제 시간" "src/test/java/DTest.java" \ + $'class DTest { void t() { Thread.sleep(10); } }\n' "TS-4" + probe "정상 실패를 예외로" "src/main/java/E.java" \ + $'class E { void f() { throw new SoldOutException(); } }\n' "EX-1" + probe "Flux.interval" "src/main/java/F.java" \ + $'class F { void f() { Flux.interval(d).subscribe(); } }\n' "RX-2" + # 주석 한 줄이 진짜 위반을 면제하던 회귀 + probe "주석 속 record 는 면제가 아니다" "src/main/java/G.java" \ + $'// record G(int x)\nclass G {\n public G() {}\n}\n' "JS-12" + # record 와 클래스가 한 파일에 섞여도 클래스만 잡는다 + probe "record 가 있어도 다른 클래스는 검사한다" "src/main/java/H.java" \ + $'public record Marker(int x) {}\n\nclass H {\n public H() {}\n}\n' "JS-12" + # **상대경로 회귀.** git 은 선행 슬래시 없는 경로를 준다. 절대경로로 + # 안 바꾸면 테스트 전용 검사가 아예 안 돌고(미탐), 동시에 테스트가 + # 프로덕션 규칙으로 검사된다(오탐). 둘 다 실제로 났다. + # + # 저장소 안의 빌드 경로에 둔다 — 러너가 git 루트에서 도는 것을 전제하므로 + # 저장소 밖으로 나가면 재현이 안 된다. + local relprobe="build/review-selftest/src/test/java/RelTest.java" + mkdir -p "$(dirname "$relprobe")" + printf 'class RelTest {\n void t() throws Exception {\n Thread.sleep(10);\n }\n}\n' \ + > "$relprobe" + local rel; rel=$(review_files "$relprobe" 2>&1) + rm -rf build/review-selftest + if printf '%s' "$rel" | grep -q "TS-4"; then + printf ' ✓ 상대경로에서도 테스트 검사가 돈다\n' + else + printf ' ✗ 상대경로에서 테스트 검사가 안 돈다 (미탐)\n'; fail=1 + fi + # 헤더 줄에도 규칙 ID 가 적혀 있다. 지적 형식([RX-1])으로만 본다. + if printf '%s' "$rel" | grep -q "\[RX-1\]"; then + printf ' ✗ 테스트를 프로덕션 규칙으로 검사한다 (오탐)\n'; fail=1 + else + printf ' ✓ 테스트를 프로덕션 규칙으로 검사하지 않는다\n' + fi + + rm -rf "$tmp" + findings=0 + return $fail +} + +if [[ "${1:-}" == "--self-test" ]]; then + head2 "자기검증 — 각 검사가 실제로 무는가" + self_test && { say $'\n자기검증 통과'; exit 0; } || { say $'\n자기검증 실패'; exit 1; } +fi + +BASE="${1:-origin/develop}" +# **다른 ref 로 대체하지 않는다.** 요청한 기준이 아닌 것을 보면 검사 결과가 +# 실제 PR 과 어긋나고, 어긋난 통과는 통과가 아니다. +if ! git rev-parse --verify "$BASE^{commit}" >/dev/null 2>&1 \ + || ! git merge-base "$BASE" HEAD >/dev/null 2>&1; then + say "기준을 해석할 수 없다: $BASE" + say " git fetch 하거나 기준을 인자로 준다 — 이 상태로는 무엇이 바뀌었는지 알 수 없다" + exit 1 +fi + +# 커밋된 것만 보면 **아직 안 커밋한 위반을 놓친다.** 개발 중에 돌릴 때가 +# 오히려 더 중요하므로 작업 트리까지 합친다. +mapfile -t CHANGED < <({ + git diff --name-only "$BASE"...HEAD + git diff --name-only HEAD + git diff --name-only --cached + git ls-files --others --exclude-standard +} | sort -u) + +if ((${#CHANGED[@]} == 0)); then + say "$BASE 대비 변경 없음"; exit 0 +fi + +head2 "$BASE 대비 변경 ${#CHANGED[@]}건 (작업 트리 포함)" +printf ' %s\n' "${CHANGED[@]}" + +review_files "${CHANGED[@]}" + +head2 "사람·에이전트가 볼 것" +say " 기계 검사는 형태만 본다. 판정 순서의 타당성, 불변식이 실제로 지켜지는지," +say " 픽스처가 도달 불가능한 상태를 만들 수 있는지는 .claude/agents/ 가 본다." +printf '\n' +# 경로 판정은 선행 슬래시를 요구하지 않는다 — git 이 주는 형식이 상대경로다. +all=" ${CHANGED[*]} " +[[ "$all" == *"/domain/"* ]] && say " → domain-guardian" +[[ "$all" == *".lua"* || "$all" == *"/redis/"* ]] && say " → redis-cluster-checker" +[[ "$all" == *"src/test/"* || "$all" == *"src/testFixtures/"* ]] && say " → test-quality-reviewer" +# 장애·회복 경로는 파일명으로 안 드러난다. 이름에 단서가 있을 때만 권한다. +[[ "$all" == *"esilience"* || "$all" == *"ircuit"* || "$all" == *"etry"* \ + || "$all" == *"ailover"* || "$all" == *"eader"* || "$all" == *"haos"* ]] \ + && say " → resilience-auditor" +say " → style-enforcer (항상)" + +printf '\n' +((findings)) && { say "위반 있음 — 고치고 다시 돌린다"; exit 1; } +say "기계 검사 통과" diff --git a/.claude/hooks/self-test.sh b/.claude/hooks/self-test.sh index 16698f7d..a30a62a7 100755 --- a/.claude/hooks/self-test.sh +++ b/.claude/hooks/self-test.sh @@ -68,6 +68,46 @@ file_case check-java.sh 'class A { class Inner { } }' 'src/main/java/G2.java' block 'JS-14 수식어 없는 중첩 (회귀)' + +file_case check-java.sh '/** 한 줄 javadoc */ +class Z { + void a() {} + void b() {} + void c() {} + void d() {} + void e() {} + void f() {} +}' 'src/main/java/Z.java' allow 'JS-6 한 줄 Javadoc 은 본문 0줄 (회귀) — 사본이 여기서 갈렸다' + +file_case check-java.sh 'class N { + @Nested + class Inner { + } +}' 'src/test/java/NTest.java' allow 'JS-14 @Nested 는 면제 (회귀) — static 이면 실행되지 않는다' + +file_case check-java.sh 'class N2 { + @Nested + @DisplayName("설명") + @Tag("slow") + class Inner { + } +}' 'src/test/java/N2Test.java' allow 'JS-14 @Nested 어노테이션 3개 (회귀) — 개수를 못 박지 않는다' + +file_case check-java.sh 'class N3 { + @Nested class Inner { + } + + class Leaked { + } +}' 'src/test/java/N3Test.java' block 'JS-14 @Nested 가 같은 줄이면 다음 중첩이 새지 않는다 (회귀)' + +# 상대경로로 넘어오면 테스트가 프로덕션 규칙으로 검사된다. 러너가 절대경로로 +# 넘기는지를 여기서 고정한다 — 이 회귀가 실제로 났다. +file_case check-java.sh 'class R { + void t() throws Exception { + Thread.sleep(10); + } +}' 'src/test/java/RTest.java' allow 'RX-1 은 테스트 소스셋에 적용되지 않는다 (회귀)' file_case check-java.sh 'class A { void f() { mono.block(); @@ -254,6 +294,103 @@ git_case 'feat(app): CY-18 진입점 추가 Refs: CY-18' block '제목에 Jira 키' git_case 'Merge branch develop' allow '병합 커밋은 대상 아님' +# ── 브랜치 리뷰 러너 ───────────────────────────────────────────────────────── +# 이 러너가 없으면 힙독·스크립트로 쓴 파일은 어떤 검사도 안 받는다. +# 러너 자신의 자기검증을 여기서 함께 돌린다. +echo +echo '[review-branch.sh]' +if "$ROOT/.claude/hooks/review-branch.sh" --self-test >/dev/null 2>&1; then + printf ' ok 러너 자기검증 (JS-12·JS-6·TS-11·TS-4·EX-1·RX-2)\n'; pass=$((pass + 1)) +else + printf ' FAIL 러너 자기검증 — 검사 중 하나가 위반을 못 잡는다\n'; fail=$((fail + 1)) +fi + +# ── PR 가드 ────────────────────────────────────────────────────────────────── +echo +echo '[guard-pr.sh]' +# **프로덕션 소스 트리에 쓰지 않는다.** 스크립트가 중간에 죽으면 public 생성자를 +# 가진 JS-12 위반 파일이 도메인 패키지에 남는다. 저장소 루트의 임시 디렉터리에 +# 두고 trap 을 건다 — gitignore 에 걸리면 러너의 변경 목록에 안 잡혀 무의미하다. +probe_dir="$ROOT/.selftest-probe" +probe="$probe_dir/Probe.java" +mkdir -p "$probe_dir" +# EXIT 트랩은 하나뿐이라 앞의 것을 덮는다. 둘 다 지우게 합친다. +trap 'rm -rf "$tmp" "$probe_dir"' EXIT +cat > "$probe" <<'PROBE' +class Probe { + public Probe() { + } +} +PROBE + +# 러너가 이 파일을 실제로 본다는 것부터 확인한다. 안 보면 아래 차단 검증이 +# 통과해도 그건 다른 이유로 막힌 것이다. +# 러너는 위반이 있으면 1 을 낸다. pipefail 아래서 파이프로 바로 받으면 +# grep 이 맞아도 파이프라인이 실패로 읽힌다 — 출력을 먼저 담는다. +runner_out=$("$ROOT/.claude/hooks/review-branch.sh" 2>&1 || true) +probe_seen=0 +printf '%s' "$runner_out" | grep -q '.selftest-probe/Probe.java' && probe_seen=1 + +printf '{"tool_input":{"command":"gh pr create --base develop"}}' \ + | "$ROOT/.claude/hooks/guard-pr.sh" >/dev/null 2>&1 +blocked=$? + +rm -rf "$probe_dir" + +# 기준 브랜치가 없으면 러너가 "무엇이 바뀌었는지 알 수 없다" 로 차단한다. +# 그게 맞는 동작이지만 **하네스가 저장소 ref 상태에 의존하면 안 된다** (TS-7). +base_ok=0 +for ref in origin/develop develop; do + git -C "$ROOT" rev-parse --verify "$ref^{commit}" >/dev/null 2>&1 \ + && git -C "$ROOT" merge-base "$ref" HEAD >/dev/null 2>&1 && base_ok=1 && break +done + +if [[ -n "$(git -C "$ROOT" status --porcelain)" || $base_ok -eq 0 ]]; then + clean=0 + skip_clean=1 +else + printf '{"tool_input":{"command":"gh pr create --base develop"}}' \ + | "$ROOT/.claude/hooks/guard-pr.sh" >/dev/null 2>&1 + clean=$? + skip_clean=0 +fi + +printf '{"tool_input":{"command":"git status"}}' \ + | "$ROOT/.claude/hooks/guard-pr.sh" >/dev/null 2>&1 +unrelated=$? + +# 검사를 못 돌리는 상황에서 통과시키면 게이트가 조용히 사라진다 (fail closed) +failclosed=$(cd /tmp && printf '{"tool_input":{"command":"gh pr create"}}' \ + | "$ROOT/.claude/hooks/guard-pr.sh" >/dev/null 2>&1; echo $?) + +if ((probe_seen)); then + printf ' ok 러너가 변경된 프로브 파일을 본다\n'; pass=$((pass + 1)) +else + printf ' FAIL 러너가 프로브를 못 본다 — 차단 검증이 무의미하다\n'; fail=$((fail + 1)) +fi +if ((failclosed == 2)); then + printf ' ok 저장소 밖에서는 막는다 (fail closed)\n'; pass=$((pass + 1)) +else + printf ' FAIL 저장소 밖인데 통과시켰다 (exit %d)\n' "$failclosed"; fail=$((fail + 1)) +fi +if ((blocked == 2)); then + printf ' ok 실제 위반에서 PR 생성을 막는다\n'; pass=$((pass + 1)) +else + printf ' FAIL 위반이 있는데 PR 생성을 통과시켰다 (exit %d)\n' "$blocked"; fail=$((fail + 1)) +fi +if ((skip_clean)); then + printf ' skip 작업 트리가 더럽거나 기준 브랜치가 없어 "깨끗하면 통과" 는 건너뛴다\n' +elif ((clean == 0)); then + printf ' ok 깨끗하면 막지 않는다\n'; pass=$((pass + 1)) +else + printf ' FAIL 깨끗한데 막았다 (exit %d)\n' "$clean"; fail=$((fail + 1)) +fi +if ((unrelated == 0)); then + printf ' ok PR 생성이 아닌 명령은 건드리지 않는다\n'; pass=$((pass + 1)) +else + printf ' FAIL 무관한 명령을 막았다 (exit %d)\n' "$unrelated"; fail=$((fail + 1)) +fi + echo printf '통과 %d · 실패 %d\n' "$pass" "$fail" ((fail == 0)) diff --git a/.claude/settings.json b/.claude/settings.json index 60e9689d..e1503d30 100644 --- a/.claude/settings.json +++ b/.claude/settings.json @@ -20,6 +20,10 @@ { "type": "command", "command": "$CLAUDE_PROJECT_DIR/.claude/hooks/check-commit-msg.sh" + }, + { + "type": "command", + "command": "$CLAUDE_PROJECT_DIR/.claude/hooks/guard-pr.sh" } ] } diff --git a/.github/workflows/_verify-conventions.yml b/.github/workflows/_verify-conventions.yml index b14c25dc..bcb3fe03 100644 --- a/.github/workflows/_verify-conventions.yml +++ b/.github/workflows/_verify-conventions.yml @@ -29,6 +29,9 @@ jobs: - uses: actions/checkout@11d5960a326750d5838078e36cf38b85af677262 # v4.4.0 with: persist-credentials: false + # 자기검증이 브랜치 리뷰 러너를 돌린다. 얕은 체크아웃이면 기준 + # 브랜치가 없어 그 케이스를 건너뛰고, 건너뛴 검사는 없는 것과 같다. + fetch-depth: 0 # 훅이 위반을 잡지 못하면 모든 코드를 통과시킨다 (TS-9) - name: 훅 자기검증 run: | diff --git a/CLAUDE.md b/CLAUDE.md index 03cc0a7d..78534c95 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -131,7 +131,7 @@ waiting/ 4. GREEN — 최소 구현 커밋: feat(scope): ... 5. REFACTOR (선택) 커밋: refactor(scope): ... 6. 작업 로그 기록 /journal 또는 ai/journal/ -7. 리뷰 에이전트 실행 해당 영역 에이전트 +7. 로컬 리뷰 /review ← PR 전에 끝낸다 8. PR → develop squash 금지. TDD 사이클 커밋이 이력의 목적이다 ``` @@ -153,9 +153,25 @@ waiting/ --- -## 8. 리뷰 에이전트 +## 8. 리뷰 — PR 을 올리기 전에 끝낸다 -코드를 쓴 뒤 해당 영역 에이전트를 돌린다. 사람 리뷰의 앞단이지 대체가 아니다. +**`/review` 를 돌린다.** 원격에서 지적받고 고치는 왕복은 비싸고, 그 사이 +잘못된 코드가 브랜치에 남는다. 절차 전문: [.claude/commands/review.md](.claude/commands/review.md) + +```bash +.claude/hooks/review-branch.sh # 기계 검사 — CodeRabbit 이 볼 것을 먼저 본다 +./gradlew build jacocoTestCoverageVerification pitest +``` + +`gh pr create` 는 **기계 검사가 통과해야 실행된다** (`.claude/hooks/guard-pr.sh`). +막히면 우회하지 말고 고친다. + +> **왜 브랜치 전체를 다시 보는가.** `check-java.sh`·`check-lua.sh` 는 +> `Write|Edit` 훅이라 **그 도구로 쓴 파일만** 본다. 힙독이나 스크립트로 만든 +> 파일은 통째로 지나가고, 실제로 그렇게 들어간 위반이 CodeRabbit 까지 갔다. + +기계가 통과했다고 끝이 아니다. 해당 영역 에이전트를 돌린다 — 사람 리뷰의 +앞단이지 대체가 아니다. | 에이전트 | 언제 | |---|---| diff --git a/ai/journal/2026/08/AIJ-0013-local-review-before-pr.md b/ai/journal/2026/08/AIJ-0013-local-review-before-pr.md new file mode 100644 index 00000000..f408c1b1 --- /dev/null +++ b/ai/journal/2026/08/AIJ-0013-local-review-before-pr.md @@ -0,0 +1,86 @@ +--- +id: AIJ-0013 +date: 2026-08-19 +kind: implement +phase: 3 +plan: [] +jira: CY-227 +commits: [81f92c2] +agent: claude-opus-5 +confidence: medium +promoted-to: +--- + +# 훅이 못 본 파일들 — PR 전에 브랜치 전체를 다시 본다 + +이전 맥락: [AIJ-0012](AIJ-0012-allocation-polling-smoothing.md) 에서 리뷰 지적 +14건을 받았고 그중 셋이 우리 자신의 MUST 규칙 위반이었다. 그때는 "새 패키지를 +만들 때마다 같은 자리에서 다시 샌다" 고만 적었는데, **원인이 사람이 아니라 +배선이었다.** + +## 무엇을 + +`review-branch.sh`(브랜치 전체 기계 검사), `guard-pr.sh`(PR 생성 차단), +`/review` 커맨드. `check-java.sh` 의 `@Nested` 오탐 수정. + +## 왜 (근거) + +**`check-java.sh` 는 `Write|Edit` PostToolUse 훅이라 그 도구로 쓴 파일만 본다.** +Phase 2 의 파일은 대부분 힙독과 스크립트로 만들었고, 그것들은 훅을 통째로 +지나갔다. 규칙 위반이 두 라운드에 걸쳐 원격 리뷰까지 간 진짜 이유가 이것이다. + +**검사 내용을 복사하지 않는다.** 러너는 기존 훅을 그 훅의 입력 형식으로 호출할 +뿐이다 — 사본이 생기면 갈라지고, 갈라진 사본은 오탐을 낸다. + +**차단이 리마인더보다 낫다.** "올리기 전에 돌려라" 는 잊힌다. 실제로 잊었다. + +## 자기검증이 세 결함을 덮고 있었다 + +러너를 만들고 자기검증 6건을 붙였는데 **전부 통과했다.** 그런데 리뷰 +에이전트가 셋을 찾았다. + +**① 상대경로.** `git diff --name-only` 는 `src/test/...` 를 준다 — 선행 슬래시가 +없다. `*/src/test/*` 가 안 맞아 **테스트 전용 검사는 한 번도 안 돌았고(미탐), +동시에 테스트가 프로덕션 규칙으로 검사됐다(오탐).** 자기검증은 프로브를 +절대경로 임시 디렉터리에 써서 **실제 호출 경로를 안 탔다.** + +**② JS-6 사본.** 러너가 `check-java.sh` 가 이미 하는 검사를 awk 로 다시 +구현해 뒀고, 한 줄 `/** … */` 처리 분기가 빠져 오탐을 냈다. 자기검증은 +`check-java.sh` 가 먼저 같은 규칙 ID 를 출력해서 **사본을 통째로 지워도 +통과했다** — 약한 단언이다. + +**③ `@Nested` 면제 범위.** 어노테이션 두 줄만 건너뛰어 `@Tag` 하나만 더 붙으면 +오탐이 났다. 그리고 훅 동작을 바꾸면서 **그 동작을 고정하는 회귀 케이스를 안 +넣었다** — 자기검증을 강화하는 커밋에서. + +셋 다 회귀 케이스로 못 박았다. 자기검증 60 → 68 건. + +## 고려했으나 택하지 않은 것 + +- **`Bash` PostToolUse 훅으로 파일 쓰기를 잡기** — 힙독 안의 경로를 파싱해야 + 하는데 형태가 무한하다. 파싱이 틀리면 오탐이고, 오탐은 우회를 부른다. + 변경 집합을 git 에게 묻는 쪽이 확실하다. +- **`gh pr create` 를 막지 않고 경고만** — 경고는 지나친다. 이 저장소가 + 차단으로 얻은 것들(커밋 규약, 경로 보호)이 이미 그 증거다. + +## 확신이 낮은 부분 + +- **`guard-pr.sh` 의 base 추출이 얕다.** `--base origin/develop` 처럼 이미 + 접두가 붙은 입력을 아직 안 다룬다. +- **러너가 보는 규칙은 `.coderabbit.yaml` 의 일부다.** 판정 순서의 타당성, + 불변식이 실제로 지켜지는지는 여전히 에이전트와 사람 몫이다. + +## 검증 + +- 러너 자기검증 8건 · 훅 자기검증 67건 전건 통과 +- 이미 병합된 Phase 2 코드에 돌려 **위반 3건 발견** — FQDN 2곳, + `private static` 2개, `@Nested` 오탐 + +## 다음 사람에게 + +**"자기검증이 통과했다" 가 "검사가 맞다" 를 뜻하지 않는다.** 이 저장소는 그 +전례를 이미 갖고 있었는데(픽스처가 도달 불가 상태를 만들 수 있어 핵심 버그가 +3개월 살아남았다) **하네스 자신에게 같은 일이 났다.** + +프로브를 실제 호출 경로로 태우지 않으면, 하네스는 자기가 시험하는 코드가 아니라 +**자기가 만든 이상적인 입력**을 시험한다. 프로브 경로 하나가 달라서 셋을 놓쳤다. diff --git a/ai/journal/index.md b/ai/journal/index.md index 9e233a14..1feb1324 100644 --- a/ai/journal/index.md +++ b/ai/journal/index.md @@ -8,6 +8,7 @@ | ID | 날짜 | 종류 | 제목 | 확신 | 승격 | |---|---|---|---|---|---| +| [AIJ-0013](2026/08/AIJ-0013-local-review-before-pr.md) | 2026-08-19 | implement | 훅이 못 본 파일들 — PR 전 브랜치 전체 검사 | medium | — | | [AIJ-0012](2026/08/AIJ-0012-allocation-polling-smoothing.md) | 2026-08-19 | implement | 배분·폴링·평활화 — 계획서 예시와 완료 조건의 충돌 | high | — | | [AIJ-0011](2026/08/AIJ-0011-admission-ladder-and-mutation-gaps.md) | 2026-08-19 | implement | 판정 사다리 · 순위 추정 · 뮤테이션이 짚은 경계 | high | — | | [AIJ-0010](2026/08/AIJ-0010-domain-state-and-limiter.md) | 2026-08-19 | implement | 순수 도메인 — 불변식·통과 상한·리미터 | high | — | @@ -53,6 +54,7 @@ | AIJ-0010 | 노드 번호(`nodeIndex`)가 틱마다 안정적이다 | Phase 4 하트비트 설계 시 확인. 바뀌면 배분이 출렁인다 | | AIJ-0011 | 남은 생존 뮤턴트 3건이 정말 등가다 | 논증으로만 확인했다. 도구가 보장하지 않는다 | | AIJ-0011 | 샤드 균등 분포 가정이 쏠린 순간에도 성립한다 | Phase 3 에서 실측 | +| AIJ-0013 | `guard-pr.sh` 의 base 추출이 모든 입력 형태를 다룬다 | `--base origin/develop` 같은 중복 접두 미처리 | | AIJ-0012 | 폴링 밴드 간격 1/3/10 초가 예산 4,000 RPS 와 맞는다 | 30 초만 계획서가 못 박았다. Phase 6 실측 | | AIJ-0012 | ETA 버킷 경계 30/90/450 초가 이탈 판단 구간과 맞는다 | 문구에서 역산했다. 실측 필요 | | AIJ-0012 | 히스테리시스 최소 유지 3틱이 적절하다 | 계획서에 기본값이 없다 | diff --git a/ai/rules/00-index.md b/ai/rules/00-index.md index fa34f541..69da9a1d 100644 --- a/ai/rules/00-index.md +++ b/ai/rules/00-index.md @@ -41,6 +41,16 @@ 아직 없으면 언제 생기는지를 적는다. 나머지는 리뷰 에이전트와 사람이 본다. +| 표기 | 무엇이 막는가 | 언제 | +|---|---|---| +| ✅ | `check-java.sh`·`check-lua.sh` | 파일을 쓰는 순간 | +| **PR** | `review-branch.sh` → `guard-pr.sh` | `gh pr create` 시점 | +| CI | 워크플로 | 푸시 이후 | + +> **✅ 는 `Write`·`Edit` 로 쓴 파일만 본다.** 힙독이나 스크립트로 만든 파일은 +> 그 훅을 지나가므로, 브랜치 전체를 다시 보는 **PR** 단이 필요하다 +> ([AIJ-0013](../journal/2026/08/AIJ-0013-local-review-before-pr.md)). + --- ## MUST 목록 (전체) @@ -53,7 +63,7 @@ | JS-2 | 와일드카드 import 금지 | ✅ | | JS-6 | Javadoc 5줄 이하 | ✅ | | JS-9 | `@Data` 금지 | ✅ | -| JS-12 | 생성자 대신 정적 팩토리 | — | +| JS-12 | 생성자 대신 정적 팩토리 | **PR** | | JS-13 | `private static` 메서드 금지 | ✅ | | JS-14 | 유틸리티·중첩 클래스는 `static` | ✅ | | DS-1 | 도메인은 Spring·Redis·시계를 참조하지 않는다 | ✅ | @@ -73,20 +83,21 @@ | LG-4 | Loki 라벨은 저카디널리티만 | 6.5.2 예정 | | LG-5 | 리액티브에서 MDC 금지 | ✅ | | LG-6 | 개인정보·비밀 로깅 금지 | ✅ | -| EX-1 | 정상 실패는 예외가 아니다 (판정값으로) | — | +| EX-1 | 정상 실패는 예외가 아니다 (판정값으로) | **PR** | | EX-2 | 모든 예외는 `WaitingException` 상속 | — | | EX-7 | 내부 정보를 응답에 담지 않는다 | — | | EX-12 | 전역 처리는 `ErrorWebExceptionHandler` | — | | JS-5 | 필드는 `final` | — | -| RX-2 | 주기 루프는 `repeatWhen`, `Flux.interval` 아님 | — | +| RX-2 | 주기 루프는 `repeatWhen`, `Flux.interval` 아님 | **PR** | | RX-3 | 배경 루프에 스케줄러를 명시한다 | — | | RX-5 | 에러가 루프를 죽이지 않게 한다 | — | | RX-6 | 실패해도 마지막 좋은 상태를 지운다 | — | | RX-7 | 요청 바디를 읽지 않는다 | — | | RX-11 | 공유 가변 상태에는 메모리 가시성을 명시한다 | — | | TS-2 | 테스트 이름은 한글 문장 | — | -| TS-4 | 시계를 고정한다 | — | -| TS-7 | 불안정 테스트는 격리하지 말고 고친다 | — | +| TS-4 | 시계를 고정한다 | **PR** | +| TS-7 | 불안정 테스트는 격리하지 말고 고친다 | **PR** | +| TS-11 | 약한 단언을 쓰지 않는다 | **PR** | | TS-8 | 카오스도 TDD로 쓴다 | — | | TS-9 | 하네스를 자기검증한다 | CI | | RD-2 | 한 스크립트는 한 슬롯만 만진다 | — | diff --git a/ai/rules/10-java-style.md b/ai/rules/10-java-style.md index 102e878d..51d1483a 100644 --- a/ai/rules/10-java-style.md +++ b/ai/rules/10-java-style.md @@ -298,6 +298,10 @@ private class Accumulator { } private static final class Accumulator { } ``` +**예외는 JUnit 5 의 `@Nested` 하나다.** 그쪽은 static 이면 아예 실행되지 +않는다 — 규칙과 프레임워크가 충돌하는 자리라 규칙이 진다. 훅도 면제하므로 +`RULE-EXCEPTION` 주석을 달 필요가 없다. + non-static 내부 클래스는 바깥 인스턴스를 잡고 있어, 그 참조가 배경 루프나 컬렉션에 실려 나가면 **바깥 객체 전체가 GC되지 않는다.** 리액티브 체인처럼 객체가 스레드를 넘나드는 곳에서 특히 위험하다. diff --git a/ai/rules/60-workflow.md b/ai/rules/60-workflow.md index c5a2631e..23b4070b 100644 --- a/ai/rules/60-workflow.md +++ b/ai/rules/60-workflow.md @@ -52,6 +52,7 @@ type·scope 를 영문으로 두는 이유는 Conventional Commits 도구가 그 ``` domain admission allocation snapshot queue capacity token redis routing config health chaos plan rules +hooks ci ``` 여러 패키지에 걸치면 생략한다. diff --git a/src/main/java/com/kafkick/waiting/domain/allocation/FairShareAllocator.java b/src/main/java/com/kafkick/waiting/domain/allocation/FairShareAllocator.java index be345b3a..e15d550e 100644 --- a/src/main/java/com/kafkick/waiting/domain/allocation/FairShareAllocator.java +++ b/src/main/java/com/kafkick/waiting/domain/allocation/FairShareAllocator.java @@ -9,7 +9,20 @@ *
균등하게만 나누면 한산한 쿠폰이 못 쓰고 남긴 몫이 버려지고, 요구량 비례로만 * 나누면 몰리는 쿠폰 하나가 전부 가져가 나머지가 굶는다 (C-1·C-3). */ -public final class FairShareAllocator { +public class FairShareAllocator { + + private FairShareAllocator() { + } + + /** + * 배분기를 만든다. + * + *
상태가 없어 static 으로 둘 수도 있지만 배분은 도메인 규칙이다
+ * (JS-14). 두 번째 정책이 생길 때 호출부를 안 고치려면 인스턴스여야 한다.
+ */
+ public static FairShareAllocator create() {
+ return new FairShareAllocator();
+ }
/**
* 굶주린 쿠폰에게 균등하게 나누고, 못 쓴 몫을 다시 굶주린 쪽으로 돌린다.
@@ -18,7 +31,7 @@ public final class FairShareAllocator {
* 이득이고, 노드마다 다른 쪽을 고르면 총합이 전역 크레딧을 넘는다. 남긴
* 나머지는 다음 틱 배분에 다시 들어간다.
*/
- public static List