diff --git a/.claude/commands/review-gate.md b/.claude/commands/review-gate.md index 266dc8d..8012c21 100644 --- a/.claude/commands/review-gate.md +++ b/.claude/commands/review-gate.md @@ -20,12 +20,17 @@ Loop until the gate is CLEAN: - Re-run the relevant evidence from `CLAUDE.md` (tsc / vitest / build / `npm run inspect`). - Commit + `git push`. - - Reply on the thread referencing the fix commit, then - `scripts/review-gate.sh resolve `. -4. If changes were non-trivial, re-trigger: `gh pr comment $1 --body - "@codex review"`, wait for it to respond, then re-evaluate from step 1. -5. When **all** review threads are resolved, `gh pr checks $1` is green, and - `mergeStateStatus` is `CLEAN`: + - **Post a top-level PR comment that `@codex`-mentions the finding and + highlights the change**, then **resolve the old thread**: + `gh pr comment $1 --body "@codex addressed in : . Please re-review."` + then `scripts/review-gate.sh resolve `. +4. **Wait for Codex's re-review** — it arrives as **NEW threads** (never a + reply in the resolved one). Loop back to step 1 and handle those the same + way until **zero unresolved Codex threads**. As final judge, a + non-actionable nitpick may be resolved with a reasoned top-level `@codex` + note rather than looped forever. +5. When **zero Codex threads are unresolved**, `gh pr checks $1` is green, + and `mergeStateStatus` is `CLEAN`: - `gh pr merge $1 --squash --delete-branch`. - Linear MCP: move the linked issue to **Done** and add a comment with the merged PR URL. diff --git a/.claude/settings.json b/.claude/settings.json index 6eba4ef..0c97af0 100644 --- a/.claude/settings.json +++ b/.claude/settings.json @@ -9,6 +9,8 @@ "Bash(npm run sim:headless:*)", "Bash(npm test:*)", "Bash(npm run build:*)", + "Bash(pnpm typecheck:*)", + "Bash(pnpm lint:*)", "Bash(git status:*)", "Bash(git diff:*)", "Bash(git log:*)", diff --git a/.codex/DELEGATION.md b/.codex/DELEGATION.md index d336ab4..52ff3c8 100644 --- a/.codex/DELEGATION.md +++ b/.codex/DELEGATION.md @@ -128,7 +128,11 @@ Linear issue = intent → Claude decomposes → Codex implements bounded slice Claude verifies locally → PR opened (`Fixes `) → CI + GitHub Codex review → Claude polls feedback → Codex fixes bounded feedback → Claude verifies/integrates → Linear updated with evidence. Failed CI/review is -feedback, not interruption. See `CLAUDE.md` for the review-gate mechanics. +feedback, not interruption. Acknowledge each finding with a **top-level +`@codex` PR comment highlighting the change**, then **resolve the old +thread** and wait for re-review (which arrives as **new threads**). The gate +is "zero unresolved Codex threads + CI green + mergeStateStatus CLEAN". See +`CLAUDE.md` for mechanics. ## Cost / network policy diff --git a/CLAUDE.md b/CLAUDE.md index c1339ab..45c1910 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -120,18 +120,29 @@ Before merging: 2. Read every Codex comment and inline review thread (`scripts/review-gate.sh threads `). 3. Fix actionable feedback on the PR branch locally (this part is autonomous). -4. Push the fix; request re-review with `@codex review` when the change is - non-trivial. -5. Resolve each fixed Codex conversation - (`scripts/review-gate.sh resolve `). +4. Push the fix. For each addressed finding, **post a top-level PR comment + that `@codex`-mentions it and highlights the change**, citing the fix + commit and which thread/finding it addresses: + `gh pr comment --body "@codex addressed in : . Please re-review."` +5. **Resolve the old conversation** + (`scripts/review-gate.sh resolve `), then **wait for Codex's + re-review** — it arrives as **brand-new threads**, never as a reply in + the resolved one. Loop back to step 2 for any new threads until **zero + unresolved Codex threads** remain. 6. Confirm CI green: `gh pr checks `. -7. Confirm threads resolved **and** `mergeStateStatus` is `CLEAN` +7. Confirm **zero unresolved threads** and `mergeStateStatus` is `CLEAN` (`scripts/review-gate.sh status `). -Do **not** merge while GitHub reports unresolved conversations, failed checks, -or a blocked merge state. A clean Codex comment like "Didn't find any major -issues" counts as review evidence **only after** any earlier Codex -conversations on the PR have been fixed and resolved. +The gate is **"zero unresolved Codex threads + CI green + mergeStateStatus +CLEAN"**. Do **not** merge while GitHub reports unresolved conversations, +failed checks, or a blocked merge state. The acknowledgement is a top-level +`@codex` comment highlighting the change (not an in-thread reply — Codex +re-reviews via new threads regardless); then resolve the old thread. Claude +is final judge: a non-actionable nitpick may be resolved with a reasoned +top-level `@codex` note rather than looped forever. A clean Codex "Didn't +find any major issues" counts **only after** every earlier Codex +conversation has been acknowledged (top-level `@codex`) and resolved. ## Project specifics diff --git a/scripts/review-gate.sh b/scripts/review-gate.sh index 4076cc9..fca523c 100755 --- a/scripts/review-gate.sh +++ b/scripts/review-gate.sh @@ -6,31 +6,21 @@ # scripts/review-gate.sh resolve resolve one review conversation # # Read subcommands (status, threads) are side-effect free. `resolve` performs a -# GraphQL mutation — it is part of the normal delivery flow, not destructive. -set -euo pipefail +# GraphQL mutation — part of the normal delivery flow, not destructive. +set -uo pipefail cmd="${1:-}" arg="${2:-}" repo_json="$(gh repo view --json owner,name)" -OWNER="$(echo "$repo_json" | python3 -c 'import json,sys;print(json.load(sys.stdin)["owner"]["login"])')" -REPO="$(echo "$repo_json" | python3 -c 'import json,sys;print(json.load(sys.stdin)["name"])')" +OWNER="$(printf '%s' "$repo_json" | python3 -c 'import json,sys;print(json.load(sys.stdin)["owner"]["login"])')" +REPO="$(printf '%s' "$repo_json" | python3 -c 'import json,sys;print(json.load(sys.stdin)["name"])')" -threads_query=' -query($owner:String!,$repo:String!,$pr:Int!){ - repository(owner:$owner,name:$repo){ - pullRequest(number:$pr){ - mergeable - mergeStateStatus - reviewThreads(first:100){ - nodes{ - id isResolved isOutdated - comments(first:1){nodes{author{login} body path}} - } - } - } - } -}' +# `finding` = the original review comment (first), fetched separately so it is +# never lost no matter how many replies a thread accrues; `recent` = the tail +# (latest state, e.g. a fix reply). Codex re-reviews land as NEW threads, so +# the gate is "zero unresolved Codex threads", not an in-thread re-review. +Q='query($owner:String!,$repo:String!,$pr:Int!){repository(owner:$owner,name:$repo){pullRequest(number:$pr){mergeable mergeStateStatus reviewThreads(first:100){nodes{id isResolved isOutdated finding:comments(first:1){nodes{author{login} body path}} recent:comments(last:20){totalCount nodes{author{login} body}}}}}}}' case "$cmd" in status) @@ -39,51 +29,73 @@ case "$cmd" in gh pr checks "$arg" || true echo echo "== merge state ==" - gh api graphql -F owner="$OWNER" -F repo="$REPO" -F pr="$arg" -f query="$threads_query" \ - | python3 - <<'PY' + resp="$(gh api graphql -F owner="$OWNER" -F repo="$REPO" -F pr="$arg" -f query="$Q")" + printf '%s' "$resp" | python3 -c ' import json,sys d=json.load(sys.stdin)["data"]["repository"]["pullRequest"] th=d["reviewThreads"]["nodes"] openn=[t for t in th if not t["isResolved"]] -print(f"mergeable={d['mergeable']} mergeStateStatus={d['mergeStateStatus']}") -print(f"review threads: {len(th)} total, {len(openn)} UNRESOLVED") +mss=d["mergeStateStatus"] +print("mergeable=%s mergeStateStatus=%s" % (d["mergeable"], mss)) +print("review threads: %d total, %d UNRESOLVED" % (len(th), len(openn))) for t in openn: - c=(t["comments"]["nodes"] or [{}])[0] - who=(c.get('author') or {}).get('login','?') - body=' '.join((c.get('body') or '').split())[:120] - print(f" [open] {t['id']} @{who}: {body}") -clean = d['mergeStateStatus']=='CLEAN' and len(openn)==0 -print("\nGATE:", "CLEAN ✅ (safe to merge once CI green)" if clean else "BLOCKED ❌") -PY + f=((t["finding"]["nodes"] or [{}])[0]) + rec=t["recent"]["nodes"] or [{}] + last=rec[-1] + fw=(f.get("author") or {}).get("login","?") + lw=(last.get("author") or {}).get("login","?") + body=" ".join((last.get("body") or f.get("body") or "").split())[:140] + print(" [open] %s (%d msgs, finding @%s, latest @%s): %s" + % (t["id"], t["recent"]["totalCount"], fw, lw, body)) +clean = mss=="CLEAN" and len(openn)==0 +print("\nGATE:", "CLEAN (mergeable once CI green)" if clean else "BLOCKED") +' ;; threads) [ -n "$arg" ] || { echo "usage: review-gate.sh threads " >&2; exit 2; } - gh api graphql -F owner="$OWNER" -F repo="$REPO" -F pr="$arg" -f query="$threads_query" \ - | python3 - <<'PY' + resp="$(gh api graphql -F owner="$OWNER" -F repo="$REPO" -F pr="$arg" -f query="$Q")" + printf '%s' "$resp" | python3 -c ' import json,sys th=json.load(sys.stdin)["data"]["repository"]["pullRequest"]["reviewThreads"]["nodes"] -if not th: print("no review threads"); raise SystemExit +if not th: + print("no review threads"); sys.exit() for t in th: - c=(t["comments"]["nodes"] or [{}])[0] - who=(c.get('author') or {}).get('login','?') - path=c.get('path') or '-' - state='resolved' if t['isResolved'] else 'OPEN' - print(f"{t['id']} [{state}] @{who} ({path})") - print(" "+' '.join((c.get('body') or '').split())[:300]) -PY + first=(t["finding"]["nodes"] or [{}])[0] + rec=t["recent"]["nodes"] or [{}] + last=rec[-1] + n=t["recent"]["totalCount"] + path=first.get("path") or "-" + state="resolved" if t["isResolved"] else "OPEN" + print("%s [%s] (%s) %d msg(s)" % (t["id"], state, path, n)) + fw=(first.get("author") or {}).get("login","?") + print(" finding @%s: %s" % (fw, " ".join((first.get("body") or "").split())[:300])) + if n > 1: + lw=(last.get("author") or {}).get("login","?") + print(" latest @%s: %s" % (lw, " ".join((last.get("body") or "").split())[:300])) +' + ;; + + reply) + # reply — OPTIONAL in-thread note (audit only). + # The convention is to acknowledge via a TOP-LEVEL `@codex` PR comment + # highlighting the change, then `resolve` the old thread, then wait for + # Codex's re-review (which arrives as NEW threads). See CLAUDE.md. + body="${*:3}" + { [ -n "$arg" ] && [ -n "$body" ]; } || { + echo 'usage: review-gate.sh reply ' >&2; exit 2; } + resp="$(gh api graphql -F tid="$arg" -F body="$body" -f query='mutation($tid:ID!,$body:String!){addPullRequestReviewThreadReply(input:{pullRequestReviewThreadId:$tid,body:$body}){comment{url}}}')" + printf '%s' "$resp" | python3 -c 'import json,sys;print("replied:",json.load(sys.stdin)["data"]["addPullRequestReviewThreadReply"]["comment"]["url"])' ;; resolve) [ -n "$arg" ] || { echo "usage: review-gate.sh resolve " >&2; exit 2; } - gh api graphql -F threadId="$arg" -f query=' - mutation($threadId:ID!){ - resolveReviewThread(input:{threadId:$threadId}){thread{id isResolved}} - }' | python3 -c 'import json,sys;t=json.load(sys.stdin)["data"]["resolveReviewThread"]["thread"];print("resolved",t["id"],t["isResolved"])' + resp="$(gh api graphql -F threadId="$arg" -f query='mutation($threadId:ID!){resolveReviewThread(input:{threadId:$threadId}){thread{id isResolved}}}')" + printf '%s' "$resp" | python3 -c 'import json,sys;t=json.load(sys.stdin)["data"]["resolveReviewThread"]["thread"];print("resolved",t["id"],t["isResolved"])' ;; *) - echo "usage: review-gate.sh {status|threads|resolve} " >&2 + echo "usage: review-gate.sh {status|threads|reply|resolve} [body]" >&2 exit 2 ;; esac