Review Feedback Response skill for Codex

A round of feedback is done when every thread has either a code change or a reasoned reply, nothing outside the feedback changed, the reviewer can see exactly what moved since their last look, and the next round is smaller than this one.

1. Collect everything

Top-level reviews and conversation comments:

gh pr view 123 --comments                     # needs network and gh auth
gh pr view 123 --json reviews --jq '.reviews[] | {author: .author.login, state, body}'

Inline threads with resolution state (the REST API does not expose “resolved”):

gh api graphql -F owner='{owner}' -F repo='{repo}' -F pr=123 -f query='
query($owner: String!, $repo: String!, $pr: Int!) {
  repository(owner: $owner, name: $repo) {
    pullRequest(number: $pr) {
      reviewThreads(first: 100) {
        nodes {
          id isResolved isOutdated path line
          comments(first: 20) { nodes { databaseId author { login } body } }
        }
      }
    }
  }
}' --jq '.data.repository.pullRequest.reviewThreads.nodes[] | select(.isResolved | not)'

isOutdated: true means the code moved under the comment; it still needs an answer.

2. Triage before touching code

Build a table and show it to the user before editing. It is the plan for the round.

#	Who	Where	Ask	Decision	Reason / plan
1	alice	retry.py:42	pass idempotency key	accept	real double-charge risk
2	bob	cart.py:10	rename qty	accept	cheap, matches codebase
3	alice	api.py:88	switch to async client	follow-up	out of scope; open issue
4	bob	cart.py:55	use a dict lookup	discuss	measured: list of 4 tiers, no gain
5	carol	models.py	“why not reuse Money?”	question	answer; no change

Decision rules:

Comment type	Default
Blocker or bug with a scenario	Accept. Reproduce it first with a test if possible
Cheap readability improvement	Accept without debate
Suggestion you think is wrong	Discuss: state evidence, offer an alternative, let the reviewer decide or escalate
Valid but out of scope	Follow-up issue, linked in the reply; do not grow the PR
Two reviewers disagree	Do not pick silently. Summarise both positions for the user or the reviewers
Pure preference with no repo convention	Accept if trivial; otherwise reply once, briefly
Unclear ask	Ask a clarifying question; do not guess-implement

Check AGENTS.md and existing code when a comment is about convention; the repo’s established pattern settles most style disputes.

3. Make the changes

• One logical change per comment or theme, as fixup commits against the commit that introduced the code, so history can be cleaned up before merge:

git commit --fixup=<sha-of-original-commit>    # subject becomes "fixup! <original subject>"

If the team does not squash or autosquash, use plain commits with clear subjects (“Pass idempotency key on payment retries”) instead.

• Do not force-push a rewritten history in the middle of review unless the team works that way. It breaks “changes since your last review” and orphans inline comments. Autosquash at the end (see git-history-surgery) once reviewers approve, and only with the user’s go-ahead.
• Touch nothing a reviewer did not ask about. No drive-by reformatting, no opportunistic refactors. Scope creep is the main cause of endless rounds.
• For each accepted bug, add or extend a test that fails without the fix.
• Re-run the full check suite from AGENTS.md before pushing.

4. Reply to every thread

Draft replies alongside the changes and show them to the user before posting. Each reply states what happened and where:

Done in 4e1c9a2: retries now reuse the original idempotency_key, and
test_retry_reuses_idempotency_key covers the timeout case.

I kept the list here: there are 4 tiers and the lookup runs once per order,
so a dict would not be measurable. Happy to switch if you feel strongly.

Agreed this should be async, but it touches every caller of PaymentClient.
Opened #1520 to do it separately so this fix can ship.

Tone rules: thank briefly once per review, not per comment; never defensive; concede quickly when the reviewer is right; disagree with evidence, not authority.

Posting replies (after the user approves the drafts):

gh api repos/{owner}/{repo}/pulls/123/comments/<databaseId>/replies -f body="$(cat "$TMPDIR/reply-1.md")"
gh pr comment 123 --body-file "$TMPDIR/round-summary.md"   # top-level summary of the round

Resolving threads: follow the team norm. Many teams let the reviewer resolve; if the author resolves, use the thread id from step 1:

gh api graphql -f query='mutation($id: ID!) { resolveReviewThread(input: {threadId: $id}) { thread { isResolved } } }' -f id=<thread-node-id>

5. Push and re-request

Pushing is visible to others; confirm with the user first.

git push                                   # plain push; no --force during review
gh pr edit 123 --add-reviewer alice,bob    # re-requests review from them

Post a short round summary so reviewers know where to look:

Round 2 addressed:
- Retry idempotency (#1, 4e1c9a2) with a new test
- Naming (#2, 9b0d311)
- Deferred async client to #1520 (#3)
- Replied on #4 and #5; no code change
Ready for another look.

Avoiding another round

• Answer the question behind the comment. If a reviewer was confused, the code or a comment should change, not just your reply.
• Look for the same issue elsewhere in the diff before pushing. A reviewer who sees the pattern fixed in one place and not the next will comment again.
• If rounds keep coming on design, suggest a short synchronous conversation and write the outcome into the PR.

Verification

• Every unresolved thread from step 1 has a code change, a reply, or both.
• git diff <last-reviewed-sha>..HEAD contains only changes traceable to a comment.
• New tests for accepted bugs fail without their fix.
• Suite, lint and type checks pass on the pushed head.
• Follow-up issues exist for everything deferred, and each is linked in its thread.