From b94d2481f57f87d117f7446c3f77779be4f4e1f8 Mon Sep 17 00:00:00 2001 From: Suresh Kasipandy Date: Sat, 16 May 2026 20:59:53 -0400 Subject: [PATCH 1/5] Allowlist read-only pnpm typecheck/lint to reduce permission prompts --- .claude/settings.json | 2 ++ 1 file changed, 2 insertions(+) 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:*)", From 1932ffb7d38eb8f4fe121802e3dd4c30407ef3ec Mon Sep 17 00:00:00 2001 From: Suresh Kasipandy Date: Sat, 16 May 2026 20:59:53 -0400 Subject: [PATCH 2/5] Codex review replies must be in-thread and @codex-tagged Fixes the convention bug where a Codex review thread was closed with a top-level PR comment and no @codex tag (Codex never re-evaluated). - scripts/review-gate.sh: new 'reply' subcommand posts via addPullRequestReviewThreadReply (in the thread, not a PR comment) - CLAUDE.md / .codex/DELEGATION.md / /review-gate: mandate in-thread reply + @codex tag, and resolve a substantive thread only after Codex re-evaluates (never unilaterally) --- .claude/commands/review-gate.md | 11 +++-- .codex/DELEGATION.md | 4 +- CLAUDE.md | 20 +++++--- scripts/review-gate.sh | 85 ++++++++++++++++----------------- 4 files changed, 65 insertions(+), 55 deletions(-) diff --git a/.claude/commands/review-gate.md b/.claude/commands/review-gate.md index 266dc8d..b7d536c 100644 --- a/.claude/commands/review-gate.md +++ b/.claude/commands/review-gate.md @@ -20,10 +20,13 @@ 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. + - **Reply IN the thread (not a top-level PR comment), tagging `@codex`**: + `scripts/review-gate.sh reply "Fixed in : . @codex please re-review"`. + - **Resolve only after** Codex re-evaluates (trivial fixes: the in-thread + `@codex` reply is the record): `scripts/review-gate.sh resolve `. +4. Re-trigger when warranted: `gh pr comment $1 --body "@codex review"`, + wait for Codex to respond, then re-evaluate from step 1. Never resolve a + substantive thread without the in-thread `@codex` reply posted. 5. When **all** review threads are resolved, `gh pr checks $1` is green, and `mergeStateStatus` is `CLEAN`: - `gh pr merge $1 --squash --delete-branch`. diff --git a/.codex/DELEGATION.md b/.codex/DELEGATION.md index d336ab4..5fa2295 100644 --- a/.codex/DELEGATION.md +++ b/.codex/DELEGATION.md @@ -128,7 +128,9 @@ 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. Codex review replies go **in-thread, tagged +`@codex`** (never a top-level comment), and threads are resolved only after +Codex re-evaluates. See `CLAUDE.md` for the review-gate mechanics. ## Cost / network policy diff --git a/CLAUDE.md b/CLAUDE.md index c1339ab..4142abe 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -120,18 +120,24 @@ 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. **Reply IN the review thread Codex spawned** — never a + top-level PR comment — citing the fix commit, and **tag `@codex`** so it + re-evaluates: + `scripts/review-gate.sh reply "Fixed in : … @codex please re-review"`. +5. **Resolve only AFTER** Codex has re-evaluated the thread (or, for a + trivial fix, with the in-thread reply as the record — never resolve a + substantive thread unilaterally without the `@codex` reply posted): + `scripts/review-gate.sh resolve `. 6. Confirm CI green: `gh pr checks `. 7. Confirm threads resolved **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. +or a blocked merge state. Never resolve a Codex conversation with only a +top-level PR comment or no `@codex` reply. 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, replied to in-thread with +`@codex`, and resolved. ## Project specifics diff --git a/scripts/review-gate.sh b/scripts/review-gate.sh index 4076cc9..6968dd4 100755 --- a/scripts/review-gate.sh +++ b/scripts/review-gate.sh @@ -6,31 +6,17 @@ # 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}} - } - } - } - } -}' +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 comments(first:1){nodes{author{login} body path}}}}}}}' case "$cmd" in status) @@ -39,51 +25,64 @@ 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 + who=(c.get("author") or {}).get("login","?") + body=" ".join((c.get("body") or "").split())[:140] + print(" [open] %s @%s: %s" % (t["id"], who, 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 + who=(c.get("author") or {}).get("login","?") + path=c.get("path") or "-" + state="resolved" if t["isResolved"] else "OPEN" + print("%s [%s] @%s (%s)" % (t["id"], state, who, path)) + print(" "+" ".join((c.get("body") or "").split())[:400]) +' + ;; + + reply) + # reply — posts IN the review thread Codex spawned + # (not a top-level PR comment) so the conversation is correctly answered. + # Include "@codex" in the body when a re-review is wanted; resolve only + # AFTER Codex has re-evaluated (or for a trivial fix, with the in-thread + # reply as the record). + 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 From 3fcd8b57af7d7c0273f256495bcd38bcde609793 Mon Sep 17 00:00:00 2001 From: Suresh Kasipandy Date: Sat, 16 May 2026 21:05:37 -0400 Subject: [PATCH 3/5] review-gate.sh: read latest thread comments, not just the first Addresses Codex review P2 on PR #2: comments(first:1) only ever showed the original finding, so once a thread had finding -> reply -> Codex re-review, status/threads would resolve on stale data. Now fetches comments(last:20) and surfaces both the finding and the latest reply. --- scripts/review-gate.sh | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/scripts/review-gate.sh b/scripts/review-gate.sh index 6968dd4..4f0f9f4 100755 --- a/scripts/review-gate.sh +++ b/scripts/review-gate.sh @@ -16,7 +16,7 @@ repo_json="$(gh repo view --json owner,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"])')" -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 comments(first:1){nodes{author{login} body path}}}}}}}' +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 comments(last:20){nodes{author{login} body path}}}}}}}' case "$cmd" in status) @@ -35,10 +35,11 @@ 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] + cs=t["comments"]["nodes"] or [{}] + c=cs[-1] # LATEST comment — reflects Codex re-review, not the stale finding who=(c.get("author") or {}).get("login","?") body=" ".join((c.get("body") or "").split())[:140] - print(" [open] %s @%s: %s" % (t["id"], who, body)) + print(" [open] %s (%d msgs, latest @%s): %s" % (t["id"], len(cs), who, body)) clean = mss=="CLEAN" and len(openn)==0 print("\nGATE:", "CLEAN (mergeable once CI green)" if clean else "BLOCKED") ' @@ -53,12 +54,16 @@ th=json.load(sys.stdin)["data"]["repository"]["pullRequest"]["reviewThreads"]["n 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 "-" + cs=t["comments"]["nodes"] or [{}] + first=cs[0]; last=cs[-1] + path=first.get("path") or "-" state="resolved" if t["isResolved"] else "OPEN" - print("%s [%s] @%s (%s)" % (t["id"], state, who, path)) - print(" "+" ".join((c.get("body") or "").split())[:400]) + print("%s [%s] (%s) %d msg(s)" % (t["id"], state, path, len(cs))) + fw=(first.get("author") or {}).get("login","?") + print(" finding @%s: %s" % (fw, " ".join((first.get("body") or "").split())[:300])) + if len(cs) > 1: + lw=(last.get("author") or {}).get("login","?") + print(" latest @%s: %s" % (lw, " ".join((last.get("body") or "").split())[:300])) ' ;; From 29a435c06b82d5598a47a5276b0b7448f5f6dc22 Mon Sep 17 00:00:00 2001 From: Suresh Kasipandy Date: Sat, 16 May 2026 21:12:49 -0400 Subject: [PATCH 4/5] Fix review-gate query + correct the Codex re-review convention Addresses Codex P2 on PR #2 ("fetch the original finding separately"): the GraphQL query now uses aliased finding:comments(first:1) + recent:comments(last:20), so the original finding is never lost no matter how many replies a thread has; renderers updated accordingly. Corrects the convention itself (empirically: Codex re-review lands as a NEW thread, not an in-thread reply): reply in-thread for the audit record then resolve; the gate is 'zero unresolved Codex threads + CI green + mergeStateStatus CLEAN', not waiting for an in-thread re-review. Updated CLAUDE.md, .codex/DELEGATION.md, /review-gate. --- .claude/commands/review-gate.md | 21 ++++++++++++--------- .codex/DELEGATION.md | 7 ++++--- CLAUDE.md | 31 ++++++++++++++++++------------- scripts/review-gate.sh | 29 +++++++++++++++++++---------- 4 files changed, 53 insertions(+), 35 deletions(-) diff --git a/.claude/commands/review-gate.md b/.claude/commands/review-gate.md index b7d536c..6ec968e 100644 --- a/.claude/commands/review-gate.md +++ b/.claude/commands/review-gate.md @@ -20,15 +20,18 @@ Loop until the gate is CLEAN: - Re-run the relevant evidence from `CLAUDE.md` (tsc / vitest / build / `npm run inspect`). - Commit + `git push`. - - **Reply IN the thread (not a top-level PR comment), tagging `@codex`**: - `scripts/review-gate.sh reply "Fixed in : . @codex please re-review"`. - - **Resolve only after** Codex re-evaluates (trivial fixes: the in-thread - `@codex` reply is the record): `scripts/review-gate.sh resolve `. -4. Re-trigger when warranted: `gh pr comment $1 --body "@codex review"`, - wait for Codex to respond, then re-evaluate from step 1. Never resolve a - substantive thread without the in-thread `@codex` reply posted. -5. When **all** review threads are resolved, `gh pr checks $1` is green, and - `mergeStateStatus` is `CLEAN`: + - **Reply IN the thread for the audit record (never a top-level PR + comment)**, then **resolve it** — the fix commit + in-thread reply are + the record: + `scripts/review-gate.sh reply "Fixed in : "` then + `scripts/review-gate.sh resolve `. +4. Re-trigger: `gh pr comment $1 --body "@codex review"`. Codex's re-review + arrives as **NEW threads** (it does not reply in the old 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 in-thread reply 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/.codex/DELEGATION.md b/.codex/DELEGATION.md index 5fa2295..8b11ef8 100644 --- a/.codex/DELEGATION.md +++ b/.codex/DELEGATION.md @@ -128,9 +128,10 @@ 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. Codex review replies go **in-thread, tagged -`@codex`** (never a top-level comment), and threads are resolved only after -Codex re-evaluates. See `CLAUDE.md` for the review-gate mechanics. +feedback, not interruption. Codex review replies go **in-thread** (never a +top-level comment) for the audit record, then the thread is resolved; Codex's +re-review arrives as **new threads**, so 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 4142abe..f642a4c 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -120,22 +120,27 @@ 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. **Reply IN the review thread Codex spawned** — never a - top-level PR comment — citing the fix commit, and **tag `@codex`** so it - re-evaluates: - `scripts/review-gate.sh reply "Fixed in : … @codex please re-review"`. -5. **Resolve only AFTER** Codex has re-evaluated the thread (or, for a - trivial fix, with the in-thread reply as the record — never resolve a - substantive thread unilaterally without the `@codex` reply posted): - `scripts/review-gate.sh resolve `. +4. Push the fix. **Reply IN the original thread for the audit record** — + never a top-level PR comment — citing the fix commit: + `scripts/review-gate.sh reply "Fixed in : …"`. Then + **resolve that thread** (the fix commit + in-thread reply are the record). + Codex does **not** re-evaluate inside the same thread — its re-review + arrives as **brand-new threads**, so do not wait for an in-thread reply. +5. Re-trigger review: `gh pr comment --body "@codex review"`. Codex + posts NEW threads for anything still wrong (or nothing). Loop back to + step 2 and handle those the same way 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. Never resolve a Codex conversation with only a -top-level PR comment or no `@codex` reply. A clean Codex comment like "Didn't -find any major issues" counts as review evidence **only after** any earlier +The gate is **"zero unresolved Codex threads + CI green + mergeStateStatus +CLEAN"**, not "Codex replied in-thread". Do **not** merge while GitHub reports +unresolved conversations, failed checks, or a blocked merge state. Never +resolve with only a top-level PR comment (always the in-thread reply + +fix commit). Claude is final judge: a non-actionable nitpick may be resolved +with a reasoned in-thread reply rather than looped forever. A clean Codex +"Didn't find any major issues" counts **only after** any earlier Codex conversations on the PR have been fixed, replied to in-thread with `@codex`, and resolved. diff --git a/scripts/review-gate.sh b/scripts/review-gate.sh index 4f0f9f4..e8f001d 100755 --- a/scripts/review-gate.sh +++ b/scripts/review-gate.sh @@ -16,7 +16,11 @@ repo_json="$(gh repo view --json owner,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"])')" -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 comments(last:20){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) @@ -35,11 +39,14 @@ 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: - cs=t["comments"]["nodes"] or [{}] - c=cs[-1] # LATEST comment — reflects Codex re-review, not the stale finding - who=(c.get("author") or {}).get("login","?") - body=" ".join((c.get("body") or "").split())[:140] - print(" [open] %s (%d msgs, latest @%s): %s" % (t["id"], len(cs), who, body)) + 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") ' @@ -54,14 +61,16 @@ th=json.load(sys.stdin)["data"]["repository"]["pullRequest"]["reviewThreads"]["n if not th: print("no review threads"); sys.exit() for t in th: - cs=t["comments"]["nodes"] or [{}] - first=cs[0]; last=cs[-1] + 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, len(cs))) + 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 len(cs) > 1: + if n > 1: lw=(last.get("author") or {}).get("login","?") print(" latest @%s: %s" % (lw, " ".join((last.get("body") or "").split())[:300])) ' From 823bfa294f8037793840f38229b262a338640c02 Mon Sep 17 00:00:00 2001 From: Suresh Kasipandy Date: Sat, 16 May 2026 21:15:45 -0400 Subject: [PATCH 5/5] Adopt top-level @codex acknowledgement convention Per direction: when Codex leaves a review comment, acknowledge with a TOP-LEVEL PR comment that @codex-mentions it and highlights the change, resolve the old conversation, then wait for re-review (which always arrives as new threads). Updates CLAUDE.md, .codex/DELEGATION.md, /review-gate; review-gate.sh 'reply' demoted to optional audit note. --- .claude/commands/review-gate.md | 19 +++++++++-------- .codex/DELEGATION.md | 9 +++++---- CLAUDE.md | 36 ++++++++++++++++----------------- scripts/review-gate.sh | 9 ++++----- 4 files changed, 36 insertions(+), 37 deletions(-) diff --git a/.claude/commands/review-gate.md b/.claude/commands/review-gate.md index 6ec968e..8012c21 100644 --- a/.claude/commands/review-gate.md +++ b/.claude/commands/review-gate.md @@ -20,16 +20,15 @@ Loop until the gate is CLEAN: - Re-run the relevant evidence from `CLAUDE.md` (tsc / vitest / build / `npm run inspect`). - Commit + `git push`. - - **Reply IN the thread for the audit record (never a top-level PR - comment)**, then **resolve it** — the fix commit + in-thread reply are - the record: - `scripts/review-gate.sh reply "Fixed in : "` then - `scripts/review-gate.sh resolve `. -4. Re-trigger: `gh pr comment $1 --body "@codex review"`. Codex's re-review - arrives as **NEW threads** (it does not reply in the old 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 in-thread reply rather than looped forever. + - **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`. diff --git a/.codex/DELEGATION.md b/.codex/DELEGATION.md index 8b11ef8..52ff3c8 100644 --- a/.codex/DELEGATION.md +++ b/.codex/DELEGATION.md @@ -128,10 +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. Codex review replies go **in-thread** (never a -top-level comment) for the audit record, then the thread is resolved; Codex's -re-review arrives as **new threads**, so the gate is "zero unresolved Codex -threads + CI green + mergeStateStatus CLEAN". See `CLAUDE.md` for 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 f642a4c..45c1910 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -120,29 +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. **Reply IN the original thread for the audit record** — - never a top-level PR comment — citing the fix commit: - `scripts/review-gate.sh reply "Fixed in : …"`. Then - **resolve that thread** (the fix commit + in-thread reply are the record). - Codex does **not** re-evaluate inside the same thread — its re-review - arrives as **brand-new threads**, so do not wait for an in-thread reply. -5. Re-trigger review: `gh pr comment --body "@codex review"`. Codex - posts NEW threads for anything still wrong (or nothing). Loop back to - step 2 and handle those the same way until **zero unresolved Codex - threads** remain. +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 **zero unresolved threads** and `mergeStateStatus` is `CLEAN` (`scripts/review-gate.sh status `). The gate is **"zero unresolved Codex threads + CI green + mergeStateStatus -CLEAN"**, not "Codex replied in-thread". Do **not** merge while GitHub reports -unresolved conversations, failed checks, or a blocked merge state. Never -resolve with only a top-level PR comment (always the in-thread reply + -fix commit). Claude is final judge: a non-actionable nitpick may be resolved -with a reasoned in-thread reply rather than looped forever. A clean Codex -"Didn't find any major issues" counts **only after** any earlier -Codex conversations on the PR have been fixed, replied to in-thread with -`@codex`, and resolved. +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 e8f001d..fca523c 100755 --- a/scripts/review-gate.sh +++ b/scripts/review-gate.sh @@ -77,11 +77,10 @@ for t in th: ;; reply) - # reply — posts IN the review thread Codex spawned - # (not a top-level PR comment) so the conversation is correctly answered. - # Include "@codex" in the body when a re-review is wanted; resolve only - # AFTER Codex has re-evaluated (or for a trivial fix, with the in-thread - # reply as the record). + # 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; }