docs: add Codex review throttling protocol#439
Conversation
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR adds review governance docs, updates branch cleanup guidance, revises AGENTS workflow rules, and changes the GitHub workflow that auto-resolves Codex review comments. ChangesReview protocol and branch ledger governance
Estimated code review effort: 3 (Moderate) | ~25 minutes 🔕 Pre-merge checks override appliedThe pre-merge checks have been overridden successfully. You can now proceed with the merge. Overridden by ❌ Failed checks (1 error)
✅ Passed checks (10 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
Comment |
Head branch was pushed to by a user without write access
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/branch-cleanup-guide.md`:
- Around line 44-51: The skip-check in the branch cleanup guide is too loose
because the current lookup uses only BRANCH_NAME, which can incorrectly skip a
required review. Update the instructions around the ledger check to require
matching the branch/ref, the recorded HEAD SHA, and the branch-cleanup scope
together before deciding to skip, using the branch-review-ledger.md lookup flow
and the skip rule wording to make the condition explicit.
In `@docs/codex-review-protocol.md`:
- Around line 23-26: The review protocol currently conflicts by forbidding edits
in pure review runs while also requiring `docs/branch-review-ledger.md` updates
after branch/PR reviews. Update the guidance so the ledger entry requirement
applies only outside pure review mode, or explicitly as a separate post-review
step. Adjust the wording in the protocol section with the review rules so `pure
review`, `branch/PR reviews`, and `docs/branch-review-ledger.md` no longer imply
both behaviors at once.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 8b4acf46-bd16-44ac-a0e0-81bb90bd6eb4
📒 Files selected for processing (4)
AGENTS.mddocs/branch-cleanup-guide.mddocs/branch-review-ledger.mddocs/codex-review-protocol.md
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e426255a57
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Require branch, HEAD, and scope to match before skipping cleanup reviews, and allow ledger appends as the sole edit during pure review runs. Co-authored-by: Cursor <cursoragent@cursor.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6251c0101e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Wrap issues.listComments and issues.createComment in explicit try/catch paths that log a clear notice and fail the job deterministically on auth, rate-limit, and client API errors while preserving skip cases. Co-authored-by: Cursor <cursoragent@cursor.com>
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/codex-autofix-review-comments.yml (1)
112-151: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winAsymmetric 403 handling defeats the "keep permission-skip paths green" intent.
listCommentsruns beforecreateCommentand uses the same token. On a 403,failGitHubApi("issues.listComments", ...)callscore.setFailed(Line 35-38), so the run fails hard before ever reaching the soft-skip 403 branch at Line 145-148. If the token lacks comment permissions, it will typically 403 on the read too, so the green permission-skip path is effectively unreachable. Consider treating a 403 onlistCommentsas the same soft skip.🔧 Suggested soft-skip on read permission failure
} catch (error) { + if (error?.status === 403) { + core.warning("Skipping Codex auto-resolve request because this workflow token cannot read PR comments."); + return; + } core.notice(`Failed to list PR comments for #${issue_number} before posting Codex auto-resolve request.`); failGitHubApi("issues.listComments", error); return; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/codex-autofix-review-comments.yml around lines 112 - 151, The permission-skip flow is asymmetric because Codex auto-resolve currently hard-fails on a 403 from github.rest.issues.listComments before it can reach the existing soft-skip path in github.rest.issues.createComment. Update the try/catch around listComments in the Codex auto-resolve workflow to treat error.status === 403 the same way as createComment by logging a warning/notice and returning early instead of calling failGitHubApi, while keeping failGitHubApi for other GitHub API errors. Use the existing marker check and createComment 403 branch as the reference behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In @.github/workflows/codex-autofix-review-comments.yml:
- Around line 112-151: The permission-skip flow is asymmetric because Codex
auto-resolve currently hard-fails on a 403 from github.rest.issues.listComments
before it can reach the existing soft-skip path in
github.rest.issues.createComment. Update the try/catch around listComments in
the Codex auto-resolve workflow to treat error.status === 403 the same way as
createComment by logging a warning/notice and returning early instead of calling
failGitHubApi, while keeping failGitHubApi for other GitHub API errors. Use the
existing marker check and createComment 403 branch as the reference behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 7701b927-a5ab-4f23-9652-0779d0d71d24
📒 Files selected for processing (14)
.cursor/skills/accessibility-review/SKILL.md.cursor/skills/ai-architecture-review/SKILL.md.cursor/skills/api-review/SKILL.md.cursor/skills/code-quality-review/SKILL.md.cursor/skills/design-review/SKILL.md.cursor/skills/frontend-architecture-review/SKILL.md.cursor/skills/performance-review/SKILL.md.cursor/skills/release-readiness-review/SKILL.md.cursor/skills/repo-auditor/SKILL.md.cursor/skills/security-review/SKILL.md.cursor/skills/supabase-postgres-best-practices/SKILL.md.cursor/skills/testing-review/SKILL.md.cursor/skills/ux-review/SKILL.md.github/workflows/codex-autofix-review-comments.yml
✅ Files skipped from review due to trivial changes (13)
- .cursor/skills/code-quality-review/SKILL.md
- .cursor/skills/ai-architecture-review/SKILL.md
- .cursor/skills/api-review/SKILL.md
- .cursor/skills/testing-review/SKILL.md
- .cursor/skills/security-review/SKILL.md
- .cursor/skills/supabase-postgres-best-practices/SKILL.md
- .cursor/skills/performance-review/SKILL.md
- .cursor/skills/repo-auditor/SKILL.md
- .cursor/skills/release-readiness-review/SKILL.md
- .cursor/skills/accessibility-review/SKILL.md
- .cursor/skills/ux-review/SKILL.md
- .cursor/skills/design-review/SKILL.md
- .cursor/skills/frontend-architecture-review/SKILL.md
Use SHA-scoped PR labels and concurrency to prevent duplicate resolve requests, skip non-actionable reviews, document the auto-resolve confirmation exception, and add CI workflow guard checks. Co-authored-by: Cursor <cursoragent@cursor.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2a99f75281
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Document manual re-trigger, add resolve completion checklist, cover top-level PR comments in auto-resolve, and link the review protocol from README. Co-authored-by: Cursor <cursoragent@cursor.com>
Head branch was pushed to by a user without write access
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d270538513
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
To use Codex here, create a Codex account and connect to github. |
Queue PR/head workflow runs, create SHA labels before applying them, post the resolve comment before the label lock, and skip resolve completion summaries on issue_comment events. Co-authored-by: Cursor <cursoragent@cursor.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4178268a68
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (isResolveCompletionSummary(body)) { | ||
| core.notice( | ||
| "Skipping Codex auto-resolve request because the PR comment looks like a resolve completion summary.", | ||
| ); | ||
| return true; |
There was a problem hiding this comment.
Check severity before summary skip
For Codex top-level PR comments that contain an actionable severity marker but also use one of these common phrases, e.g. P1 ... could not resolve the module, this early completion-summary check returns before hasSeverityFindingMarkers() runs, so the workflow skips the auto-resolve request even though AGENTS.md says actionable top-level findings should trigger it. Move the P0–P3 marker check ahead of the broad summary heuristics, or make the completion-summary detection require the explicit resolve-summary structure.
Useful? React with 👍 / 👎.
|
To use Codex here, create a Codex account and connect to github. |
Treat issues.get and listComments permission failures as warnings so the workflow stays green when the token cannot read PR metadata. Co-authored-by: Cursor <cursoragent@cursor.com>
|
@copilot resolve the merge conflicts in this pull request |
|
Superseded by #444, which merged the minimal PR-level Codex auto-resolve throttle to main. Closing this PR because later auto-agent commits reintroduced SHA-scoped dedupe and kept canceling checks. |
Pull request was closed
Summary
AGENTS.md.github/workflows/codex-autofix-review-comments.ymlto handle Codex PR review submissions and inline review comments, while deduping by PR number and failing deterministically on GitHub API errorsWhy
Codex review behavior was prone to repeated branch reviews and ambiguous auto-fix loops because review rules were split across chat memory, local skills, untracked docs, and a workflow that only covered part of how Codex comments appear on PRs.
Files / areas touched
AGENTS.md.github/workflows/codex-autofix-review-comments.yml.cursor/skills/*review*/SKILL.mdand Supabase Postgres review guidancedocs/branch-review-ledger.mddocs/codex-review-protocol.mddocs/branch-cleanup-guide.mdVerification
git diff --check: passed (no conflict markers or trailing whitespace)rg -n "pull_request_review:|pull_request_review_comment:|codex-autoresolve-pr:|failGitHubApi|issues.listComments|issues.createComment|in_reply_to_id|chatgpt-codex-connector" .github/workflows/codex-autofix-review-comments.yml: passed (matched review triggers, PR-scoped marker, Codex actor guard, reply/auto-request skip paths, and GitHub API error handling)Select-String -Path docs\branch-cleanup-guide.md -Pattern "branch-cleanup|Select-String -Pattern": passed (matched branch+HEAD+scope ledger skip instructions)node "$env:TEMP\verify-codex-autofix-workflow.mjs": passed (10 workflow guard assertions: review triggers, PR-level concurrency, PR-scoped marker, Codex actor guard, reply skip, auto-request skip, listComments/createComment error handling, open-PR guard)python -c "import yaml, pathlib; yaml.safe_load(pathlib.Path('.github/workflows/codex-autofix-review-comments.yml').read_text())": passed (workflow YAML parses)Checks not run
actionlintwas not run because it is not installed in this environment.node_modules/prettieris not installed in this worktree.Risks / notes
C:\Users\joshs\.codex\skillsare outside the repo and are not part of the PR.