diff --git a/.cursor/skills/accessibility-review/SKILL.md b/.cursor/skills/accessibility-review/SKILL.md index 20f697292..9da952268 100644 --- a/.cursor/skills/accessibility-review/SKILL.md +++ b/.cursor/skills/accessibility-review/SKILL.md @@ -7,6 +7,10 @@ description: Reviews keyboard navigation support, semantic HTML tags, drawer/dia Use this skill when reviewing user interfaces to ensure compliance with a11y standards. +## Repository Review Protocol + +Follow `AGENTS.md` review throttling and `docs/codex-review-protocol.md` before starting. Do not review opportunistically, do not mutate files during pure review, and update `docs/branch-review-ledger.md` after completed branch/PR reviews. + ## Review Checklist ### 1. Keyboard & Focus Control diff --git a/.cursor/skills/ai-architecture-review/SKILL.md b/.cursor/skills/ai-architecture-review/SKILL.md index 74df735d9..53ebfe523 100644 --- a/.cursor/skills/ai-architecture-review/SKILL.md +++ b/.cursor/skills/ai-architecture-review/SKILL.md @@ -7,6 +7,10 @@ description: Reviews RAG flows, context assembly, model routing, structured outp Use this skill when reviewing or modifying the RAG (Retrieval-Augmented Generation) pipeline, document retrieval, and LLM integrations. +## Repository Review Protocol + +Follow `AGENTS.md` review throttling and `docs/codex-review-protocol.md` before starting. Do not review opportunistically, do not mutate files during pure review, and update `docs/branch-review-ledger.md` after completed branch/PR reviews. + ## Review Checklist ### 1. Retrieval Quality & Provenance diff --git a/.cursor/skills/api-review/SKILL.md b/.cursor/skills/api-review/SKILL.md index 92c96971d..4ed102675 100644 --- a/.cursor/skills/api-review/SKILL.md +++ b/.cursor/skills/api-review/SKILL.md @@ -7,6 +7,10 @@ description: Reviews API routes for routing contracts, input schema validation ( Use this skill when reviewing or modifying API routes, endpoints, or network boundaries within this repository (primarily under `src/app/api/`). +## Repository Review Protocol + +Follow `AGENTS.md` review throttling and `docs/codex-review-protocol.md` before starting. Do not review opportunistically, do not mutate files during pure review, and update `docs/branch-review-ledger.md` after completed branch/PR reviews. + ## Review Checklist ### 1. Request Validation diff --git a/.cursor/skills/code-quality-review/SKILL.md b/.cursor/skills/code-quality-review/SKILL.md index add222b68..7aafaf4b5 100644 --- a/.cursor/skills/code-quality-review/SKILL.md +++ b/.cursor/skills/code-quality-review/SKILL.md @@ -7,6 +7,10 @@ description: Reviews code quality, naming structures, control flow patterns, abs Use this skill when auditing source files for maintainability, readability, and structural patterns. +## Repository Review Protocol + +Follow `AGENTS.md` review throttling and `docs/codex-review-protocol.md` before starting. Do not review opportunistically, do not mutate files during pure review, and update `docs/branch-review-ledger.md` after completed branch/PR reviews. + ## Review Checklist ### 1. Maintainability & Naming diff --git a/.cursor/skills/design-review/SKILL.md b/.cursor/skills/design-review/SKILL.md index e52718181..a25810bc9 100644 --- a/.cursor/skills/design-review/SKILL.md +++ b/.cursor/skills/design-review/SKILL.md @@ -7,6 +7,10 @@ description: Reviews layout density, theme adherence, typography, spacing, and v Use this skill when auditing UI styling, colors, layout structures, and spacing rules. +## Repository Review Protocol + +Follow `AGENTS.md` review throttling and `docs/codex-review-protocol.md` before starting. Do not review opportunistically, do not mutate files during pure review, and update `docs/branch-review-ledger.md` after completed branch/PR reviews. + ## Review Checklist ### 1. Style & Spacing Consistency diff --git a/.cursor/skills/frontend-architecture-review/SKILL.md b/.cursor/skills/frontend-architecture-review/SKILL.md index 626c9bef2..b03aecad3 100644 --- a/.cursor/skills/frontend-architecture-review/SKILL.md +++ b/.cursor/skills/frontend-architecture-review/SKILL.md @@ -7,6 +7,10 @@ description: Reviews Next/React boundaries, state ownership, duplicated client s Use this skill when reviewing Next.js App Router structures, component boundaries, state management, and React performance. +## Repository Review Protocol + +Follow `AGENTS.md` review throttling and `docs/codex-review-protocol.md` before starting. Do not review opportunistically, do not mutate files during pure review, and update `docs/branch-review-ledger.md` after completed branch/PR reviews. + ## Review Checklist ### 1. Next.js App Router Boundaries diff --git a/.cursor/skills/performance-review/SKILL.md b/.cursor/skills/performance-review/SKILL.md index af66b5fad..e1cf0235b 100644 --- a/.cursor/skills/performance-review/SKILL.md +++ b/.cursor/skills/performance-review/SKILL.md @@ -7,6 +7,10 @@ description: Reviews model latency/cost, cache policies, duplicate query coalesc Use this skill when reviewing query bottlenecks, database RPC latencies, client bundle sizes, or API cost overhead. +## Repository Review Protocol + +Follow `AGENTS.md` review throttling and `docs/codex-review-protocol.md` before starting. Do not review opportunistically, do not mutate files during pure review, and update `docs/branch-review-ledger.md` after completed branch/PR reviews. + ## Review Checklist ### 1. Database & API Latency diff --git a/.cursor/skills/release-readiness-review/SKILL.md b/.cursor/skills/release-readiness-review/SKILL.md index eac0fb971..c990979e6 100644 --- a/.cursor/skills/release-readiness-review/SKILL.md +++ b/.cursor/skills/release-readiness-review/SKILL.md @@ -7,6 +7,10 @@ description: Reviews build validation, linting/type-checking, target database ch Use this skill when preparing a branch for merging, pull request submission, or production deployment. +## Repository Review Protocol + +Follow `AGENTS.md` review throttling and `docs/codex-review-protocol.md` before starting. Do not review opportunistically, do not mutate files during pure review, and update `docs/branch-review-ledger.md` after completed branch/PR reviews. + ## Review Checklist ### 1. Build & Automation Checks diff --git a/.cursor/skills/repo-auditor/SKILL.md b/.cursor/skills/repo-auditor/SKILL.md index 2b48c5bde..e0b77bd68 100644 --- a/.cursor/skills/repo-auditor/SKILL.md +++ b/.cursor/skills/repo-auditor/SKILL.md @@ -7,6 +7,10 @@ description: Reviews module dependencies, broken imports, unused files, and stru Use this skill when auditing the workspace layout, folder structures, imports, and dependencies. +## Repository Review Protocol + +Follow `AGENTS.md` review throttling and `docs/codex-review-protocol.md` before starting. Do not review opportunistically, do not mutate files during pure review, and update `docs/branch-review-ledger.md` after completed branch/PR reviews. + ## Review Checklist ### 1. Structural Audits diff --git a/.cursor/skills/security-review/SKILL.md b/.cursor/skills/security-review/SKILL.md index 663195611..3e8f5249e 100644 --- a/.cursor/skills/security-review/SKILL.md +++ b/.cursor/skills/security-review/SKILL.md @@ -7,6 +7,10 @@ description: Reviews credentials, token handling, JWT claims, service-role confi Use this skill when reviewing code that touches authentication, database schemas, environment variables, storage, or external API gateways. +## Repository Review Protocol + +Follow `AGENTS.md` review throttling and `docs/codex-review-protocol.md` before starting. Do not review opportunistically, do not mutate files during pure review, and update `docs/branch-review-ledger.md` after completed branch/PR reviews. + ## Review Checklist ### 1. Key Confinement & Environment Safety diff --git a/.cursor/skills/supabase-postgres-best-practices/SKILL.md b/.cursor/skills/supabase-postgres-best-practices/SKILL.md index 07aa1b3a6..f9261d20c 100644 --- a/.cursor/skills/supabase-postgres-best-practices/SKILL.md +++ b/.cursor/skills/supabase-postgres-best-practices/SKILL.md @@ -25,6 +25,8 @@ Reference these guidelines when: - Optimizing for Postgres-specific features - Working with Row-Level Security (RLS) +For review-only use, follow `AGENTS.md` review throttling and `docs/codex-review-protocol.md` first. Do not review opportunistically, do not mutate files during pure review, and update `docs/branch-review-ledger.md` after completed branch/PR reviews. + ## Rule Categories by Priority | Priority | Category | Impact | Prefix | diff --git a/.cursor/skills/testing-review/SKILL.md b/.cursor/skills/testing-review/SKILL.md index 23d9af423..b2b06e014 100644 --- a/.cursor/skills/testing-review/SKILL.md +++ b/.cursor/skills/testing-review/SKILL.md @@ -7,6 +7,10 @@ description: Reviews unit/integration/E2E coverage, checks for fragile/flaky tes Use this skill when reviewing test coverage, testing harnesses, and test execution paths (Vitest + Playwright). +## Repository Review Protocol + +Follow `AGENTS.md` review throttling and `docs/codex-review-protocol.md` before starting. Do not review opportunistically, do not mutate files during pure review, and update `docs/branch-review-ledger.md` after completed branch/PR reviews. + ## Review Checklist ### 1. Test Harness Coverage diff --git a/.cursor/skills/ux-review/SKILL.md b/.cursor/skills/ux-review/SKILL.md index d4de4dae9..564c32cb9 100644 --- a/.cursor/skills/ux-review/SKILL.md +++ b/.cursor/skills/ux-review/SKILL.md @@ -7,6 +7,10 @@ description: Reviews the question-to-answer flow, source-review friction, eviden Use this skill when reviewing user interface flows, search input behaviors, sidebars, modal navigation, and responsiveness. +## Repository Review Protocol + +Follow `AGENTS.md` review throttling and `docs/codex-review-protocol.md` before starting. Do not review opportunistically, do not mutate files during pure review, and update `docs/branch-review-ledger.md` after completed branch/PR reviews. + ## Review Checklist ### 1. Search Composer Integration diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 40f2dc678..7378479b4 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -68,6 +68,9 @@ jobs: - name: Format check run: npm run format:check + - name: Codex auto-resolve workflow guards + run: npm run check:codex-autofix-workflow + - name: Lint run: npm run lint diff --git a/.github/workflows/codex-autofix-review-comments.yml b/.github/workflows/codex-autofix-review-comments.yml index 3d0b6557b..42c499ab0 100644 --- a/.github/workflows/codex-autofix-review-comments.yml +++ b/.github/workflows/codex-autofix-review-comments.yml @@ -1,16 +1,20 @@ name: Codex auto-resolve review comments on: + pull_request_review: + types: [submitted] pull_request_review_comment: types: [created] + issue_comment: + types: [created] permissions: contents: read issues: write - pull-requests: read + pull-requests: write concurrency: - group: codex-autoresolve-${{ github.event.pull_request.number }}-${{ github.event.pull_request.head.sha }} + group: codex-autoresolve-${{ github.event.pull_request.number || github.event.issue.number }}-${{ github.event.pull_request.head.sha || 'issue-comment' }} cancel-in-progress: false jobs: @@ -18,48 +22,347 @@ jobs: name: Request Codex auto-resolve runs-on: ubuntu-latest if: > - github.event.pull_request.state == 'open' && - contains(github.event.comment.user.login, 'chatgpt-codex-connector') + ( + github.event_name != 'issue_comment' && + github.event.pull_request.state == 'open' && + ( + contains(github.event.review.user.login, 'chatgpt-codex-connector') || + contains(github.event.comment.user.login, 'chatgpt-codex-connector') + ) + ) || + ( + github.event_name == 'issue_comment' && + github.event.issue.pull_request && + github.event.issue.state == 'open' && + contains(github.event.comment.user.login, 'chatgpt-codex-connector') + ) steps: - name: Ask Codex to resolve review comments uses: actions/github-script@v7 with: github-token: ${{ github.token }} script: | - const pr = context.payload.pull_request; - const reviewComment = context.payload.comment; + const codexActors = ["chatgpt-codex-connector[bot]", "chatgpt-codex-connector"]; + + function failGitHubApi(operation, error) { + const status = error?.status; + const message = error?.message || String(error); + const details = status ? `status=${status}` : "status=unknown"; + + if (status === 401 || status === 403) { + core.setFailed(`GitHub API auth failure during ${operation}: ${details} ${message}`); + return; + } + + if (status === 429) { + core.setFailed(`GitHub API rate limit during ${operation}: ${details} ${message}`); + return; + } + + if (status && status >= 400 && status < 500) { + core.setFailed(`GitHub API client error during ${operation}: ${details} ${message}`); + return; + } + + core.setFailed(`GitHub API failure during ${operation}: ${details} ${message}`); + } + + function isCodexActor(login) { + const normalized = String(login || ""); + return codexActors.some((actor) => normalized === actor || normalized.includes("chatgpt-codex-connector")); + } + + async function loadPullRequest() { + if (context.payload.pull_request) { + return context.payload.pull_request; + } + + const issue = context.payload.issue; + if (!issue?.pull_request) { + return null; + } + + const { data: pullRequest } = await github.rest.pulls.get({ + owner: context.repo.owner, + repo: context.repo.repo, + pull_number: issue.number, + }); + return pullRequest; + } + + function sourceActor() { + if (context.eventName === "issue_comment") { + return context.payload.comment?.user?.login || ""; + } + if (context.eventName === "pull_request_review") { + return context.payload.review?.user?.login || ""; + } + return context.payload.comment?.user?.login || ""; + } + + function sourceBody() { + if (context.eventName === "issue_comment") { + return context.payload.comment?.body || ""; + } + if (context.eventName === "pull_request_review") { + return context.payload.review?.body || ""; + } + return context.payload.comment?.body || ""; + } + + function sourceId() { + if (context.eventName === "issue_comment") { + return `issue-comment:${context.payload.comment?.id}`; + } + if (context.eventName === "pull_request_review") { + return `review:${context.payload.review?.id}`; + } + return `comment:${context.payload.comment?.id}`; + } + + function hasSeverityFindingMarkers(body) { + return /\bP[0-3]\b/.test(body || ""); + } + + function isResolveCompletionSummary(body) { + const text = body || ""; + return ( + /\bfixed threads?\b/i.test(text) || + /\bdeferred or not-actionable\b/i.test(text) || + /\bcould not resolve\b/i.test(text) || + /\bcompletion summary\b/i.test(text) || + /\bremaining human decisions\b/i.test(text) + ); + } + + async function ensureRequestLabel(github, owner, repo, labelName) { + try { + await github.rest.issues.getLabel({ + owner, + repo, + name: labelName, + }); + } catch (error) { + if (error.status !== 404) { + throw error; + } + + await github.rest.issues.createLabel({ + owner, + repo, + name: labelName, + color: "1d76db", + description: "Codex auto-resolve requested for this PR head SHA.", + }); + } + } + + async function reviewHasActionableInlineComments(github, owner, repo, pullNumber, reviewId) { + const comments = await github.paginate(github.rest.pulls.listReviewComments, { + owner, + repo, + pull_number: pullNumber, + per_page: 100, + }); + + return comments.some((comment) => { + if (comment.pull_request_review_id !== reviewId) { + return false; + } + if (comment.in_reply_to_id) { + return false; + } + return isCodexActor(comment.user?.login); + }); + } + + async function shouldSkipSubmittedReview(github, owner, repo, pullNumber, review) { + const state = review?.state || ""; + const body = review?.body || ""; + + if (state === "APPROVED" || state === "DISMISSED") { + core.notice(`Skipping Codex auto-resolve request because review state is ${state}.`); + return true; + } + + if (state === "CHANGES_REQUESTED") { + return false; + } + + if (hasSeverityFindingMarkers(body)) { + return false; + } + + if (await reviewHasActionableInlineComments(github, owner, repo, pullNumber, review.id)) { + return false; + } + + core.notice("Skipping Codex auto-resolve request because the submitted review has no actionable findings."); + return true; + } - if (reviewComment.in_reply_to_id) { + function shouldSkipIssueComment(body) { + if (isResolveCompletionSummary(body)) { + core.notice( + "Skipping Codex auto-resolve request because the PR comment looks like a resolve completion summary.", + ); + return true; + } + + if (hasSeverityFindingMarkers(body)) { + return false; + } + + if (/\b(blocking|must fix|changes requested)\b/i.test(body || "")) { + return false; + } + + core.notice("Skipping Codex auto-resolve request because the PR comment has no actionable findings."); + return true; + } + + if (!isCodexActor(sourceActor())) { + core.notice(`Skipping because actor ${sourceActor()} is not the Codex review bot.`); + return; + } + + if (context.payload.comment?.in_reply_to_id) { core.notice("Skipping Codex auto-resolve request because the comment is a review-thread reply."); return; } + if ( + sourceBody().includes("codex-autoresolve:") || + sourceBody().includes("codex-autoresolve-pr:") || + sourceBody().includes("@codex resolve all review comments") + ) { + core.notice("Skipping Codex auto-resolve request because the source comment already looks like an auto-resolve request."); + return; + } + + const pr = await loadPullRequest(); + if (!pr) { + core.notice("Skipping Codex auto-resolve request because the pull request payload is missing."); + return; + } + + if (pr.state !== "open") { + core.notice(`Skipping because pull request #${pr.number} is ${pr.state}.`); + return; + } + const owner = context.repo.owner; const repo = context.repo.repo; const issue_number = pr.number; - const marker = ``; + const headSha = pr.head.sha; + const requestLabel = `codex-ar-${headSha}`; + const marker = ``; + + if (context.eventName === "pull_request_review") { + if (await shouldSkipSubmittedReview(github, owner, repo, issue_number, context.payload.review)) { + return; + } + } + + if (context.eventName === "issue_comment" && shouldSkipIssueComment(sourceBody())) { + return; + } + + let existingLabels; + try { + const labeledPullRequest = await github.rest.issues.get({ + owner, + repo, + issue_number, + }); + existingLabels = (labeledPullRequest.data.labels || []).map((label) => label.name); + } catch (error) { + if (error.status === 403) { + core.warning("Skipping Codex auto-resolve request because this workflow token cannot read PR labels."); + return; + } + core.notice(`Failed to list labels on PR #${issue_number} before posting Codex auto-resolve request.`); + failGitHubApi("issues.get", error); + return; + } + + if (existingLabels.includes(requestLabel)) { + core.notice(`Skipping duplicate Codex auto-resolve request for PR #${issue_number} at head ${headSha}.`); + return; + } + + for (const labelName of existingLabels) { + if (labelName.startsWith("codex-ar-") && labelName !== requestLabel) { + try { + await github.rest.issues.removeLabel({ + owner, + repo, + issue_number, + name: labelName, + }); + } catch (error) { + core.warning(`Failed to remove stale Codex auto-resolve label ${labelName}: ${error.message || error}`); + } + } + } - const existingComments = await github.paginate(github.rest.issues.listComments, { - owner, - repo, - issue_number, - per_page: 100, - }); + let existingComments; + try { + existingComments = await github.paginate(github.rest.issues.listComments, { + owner, + repo, + issue_number, + per_page: 100, + }); + } 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; + } if (existingComments.some((comment) => (comment.body || "").includes(marker))) { - core.notice("Skipping duplicate Codex auto-resolve request for this PR head commit."); + core.notice(`Skipping duplicate Codex auto-resolve request comment for PR #${issue_number} at head ${headSha}.`); return; } const prompt = [ marker, - ``, - "@codex resolve all review comments using the repository instructions. Always fix P0 and P1 findings. For P2 and lower findings, decide whether each is worth fixing automatically. Fix clear, scoped, low-risk issues with the best minimal change; otherwise reply explaining why the issue is deferred or not actionable. After each fix or decision, resolve the review conversation if supported. Do not use external APIs, paid services, credentials, dependency changes, or broad refactors unless explicitly authorized. Add targeted tests where behavior changes and run the narrowest relevant validation.", + ``, + `Codex auto-resolve requested for PR #${issue_number} at \`${headSha.slice(0, 7)}\`.`, + "@codex resolve all review comments using AGENTS.md § Codex GitHub review behavior (Primary PR command).", ].join("\n\n"); - await github.rest.issues.createComment({ - owner, - repo, - issue_number, - body: prompt, - }); + try { + await github.rest.issues.createComment({ + owner, + repo, + issue_number, + body: prompt, + }); + } catch (error) { + if (error.status === 403) { + core.warning("Skipping Codex auto-resolve request because this workflow token cannot comment on the pull request."); + return; + } + core.notice(`Failed to create Codex auto-resolve request comment on PR #${issue_number}.`); + failGitHubApi("issues.createComment", error); + return; + } + + try { + await ensureRequestLabel(github, owner, repo, requestLabel); + await github.rest.issues.addLabels({ + owner, + repo, + issue_number, + labels: [requestLabel], + }); + } catch (error) { + core.warning( + `Codex auto-resolve comment posted, but label lock failed for PR #${issue_number}: ${error.message || error}`, + ); + } diff --git a/AGENTS.md b/AGENTS.md index 7bb5cca02..b7c0a513a 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -111,6 +111,49 @@ Scope and safety: + + +## Codex review throttling and routing + +Do not review branches opportunistically. Review the current changed diff, PR, or branch only when the user explicitly asks for review/audit/hunter/cleanup/upload work, when CI/check failures are the task, or when the current change touches high-risk areas that require a targeted review before handoff. + +Use `docs/codex-review-protocol.md` as the shared review protocol for every repo-local review skill, branch/PR review, audit, bug hunt, release-readiness check, and PR/CI review. + +Before reviewing a branch or PR: + +- Check `docs/branch-review-ledger.md` if it exists. +- Resolve the target with `git rev-parse `. +- If the same branch/ref and HEAD SHA were already reviewed for the same scope, summarize the prior ledger outcome and skip the repeat review unless the user explicitly requests a fresh pass. +- If the target HEAD changed, review only the changed scope and update the ledger after the review. + +Ledger scope: + +- `docs/branch-review-ledger.md` is the durable throttle for local agents, branch cleanup, and explicit repo-local review skills. +- GitHub-hosted Codex PR reviews use workflow labels/comments for auto-resolve dedupe; append the ledger when the reviewed branch is checked out locally and a ledger commit is clearly safe. +- Do not treat the ledger as a substitute for GitHub auto-resolve dedupe on PRs, and do not assume GitHub-hosted reviews always update the ledger. + +Before reviewing multiple branches: + +- Build a short branch inventory first: branch, upstream, ahead/behind, last commit, and merged status. +- Skip branches already merged into `main`. +- Skip unchanged branches already recorded in `docs/branch-review-ledger.md`. +- Do not re-review every branch after ordinary coding tasks. +- If a repeated request targets unchanged reviewed branches, summarize the prior result and ask before doing another full pass. + +Review routing: + +- `diff-review`: Use for explicit review of the current diff, PR, or named branch. Findings first, ordered by severity, with file/line evidence. +- `bug-hunter`: Use only for the exact `bug-hunter` shortcut or an explicit defect-hunt request. Prioritize reproducible bugs and smallest proof. +- `repo-auditor`: Use for explicit repo-wide audit/refactor/dead-code/import/dependency-structure requests. Treat outputs as triage, not automatic delete lists. +- `release-readiness`: Use for explicit release, merge, PR readiness, or handoff confidence requests. Do not run provider-backed gates without confirmation. +- `branch-cleanup`: Use only when the prompt explicitly asks for branch cleanup/hygiene or branch deletion candidates. Apply `docs/branch-cleanup-guide.md` and the review ledger before inspecting branch diffs. +- `pr-ci-fix`: Confirmation-required for this repo. GitHub/GitLab API calls, PR comments, CI reruns, commits, and pushes require explicit user approval and must respect the upload/handoff rules. +- Exception: when `.github/workflows/codex-autofix-review-comments.yml` posts the auto-resolve request, Codex may commit to the PR branch, push, reply on review threads, and resolve conversations without another confirmation step. Do not rerun hosted CI or call paid/provider APIs unless explicitly authorized. + +When a branch or PR review completes, record the reviewed branch/ref, HEAD SHA, date, scope, outcome, and checks in `docs/branch-review-ledger.md` when the ledger is in scope for that review path. + + + # Local server safety @@ -148,6 +191,17 @@ Scope and safety: + + +# API and provider confirmation boundary + +- Never run, modify, test, or otherwise interact with OpenAI, Supabase, GitHub/GitLab, hosted CI, production-like services, or provider-backed workflows without explicit user confirmation. +- Treat indirect API usage inside scripts, tests, release checks, PR tooling, and review automation as confirmation-required too. +- Prefer local, static, mocked, or offline checks. If a recommended verification would touch a provider, report the command and ask before running it. +- `npm run check:supabase-project`, live PR/CI tooling, answer-generation checks, ingestion checks against live services, and release gates that call providers are not automatic. + + + # `upload` shortcut @@ -230,9 +284,13 @@ If changes appear unrelated, incomplete, experimental, or WIP, do not commit eve ## Branch cleanup and reference updates -If stale, inappropriate, merged, or unnecessary branches are detected, list cleanup candidates but do not delete or rename branches automatically. +During `upload`, branch cleanup is limited to the current branch and its upstream unless the user explicitly asks for `branch-cleanup`, branch hygiene, deletion candidates, or stale branch review. + +Do not enumerate, diff, or re-review unrelated stale branches during a normal upload/handoff. If the user explicitly asks for branch cleanup, first apply `docs/branch-review-ledger.md` to skip unchanged reviewed branches, then follow `docs/branch-cleanup-guide.md`. + +If stale, inappropriate, merged, or unnecessary current-branch references are detected, list cleanup candidates but do not delete or rename branches automatically. -Before recommending deletion or rename, audit accessible references including `.github/workflows/*`, CI/CD config, deployment config, scripts, package scripts, docs, release notes or release scripts, safe environment/config files, branch-specific config, open PR metadata if accessible, and GitHub branch protection/default branch metadata if safely accessible. +Before recommending deletion or rename for the current branch, audit accessible references including `.github/workflows/*`, CI/CD config, deployment config, scripts, package scripts, docs, release notes or release scripts, safe environment/config files, branch-specific config, open PR metadata if accessible, and GitHub branch protection/default branch metadata if safely accessible. Update repo-tracked references to a renamed or replacement branch only when the old branch reference is clearly found, the replacement is obvious, the change is low-risk, and the user has approved the branch rename or deletion. If the replacement is unclear, report the reference and ask what it should point to. @@ -244,7 +302,7 @@ Run the smallest relevant checks that are available and appropriate, such as tes ## Final report -After completing `upload`, summarize the current branch and worktree state, whether the worktree is clean, what changed, files committed, commit hash and message if created, whether anything was pushed, remote branch and likely PR target, checks run and results, checks not run and why, branch cleanup candidates, branch references found or updated, risky actions skipped, and exact confirmation needed for any recommended follow-up. +After completing `upload`, summarize the current branch and worktree state, whether the worktree is clean, what changed, files committed, commit hash and message if created, whether anything was pushed, remote branch and likely PR target, checks run and results, checks not run and why, current-branch cleanup candidates or why broader branch cleanup was skipped, branch references found or updated, risky actions skipped, and exact confirmation needed for any recommended follow-up. @@ -323,6 +381,10 @@ When explicitly asked to fix or resolve review findings: - Add or update the smallest relevant test when the issue affects behavior. - Run the narrowest relevant validation for the touched surface before broader suites. - Summarize fixed issues, changed files, validation run, and any remaining human decisions. +- End every resolve pass with a completion summary listing: + - fixed threads or findings + - deferred or not-actionable threads with the reason + - any threads Codex could not resolve because of GitHub permissions/tooling limits ### Review comment lifecycle @@ -335,13 +397,16 @@ When explicitly asked to fix or resolve review findings: ### Automatic resolve trigger -Automatic Codex review is review-only by default. This repository includes `.github/workflows/codex-autofix-review-comments.yml`, which requests the resolve task automatically after Codex posts a top-level review comment. +Automatic Codex review is review-only by default. This repository includes `.github/workflows/codex-autofix-review-comments.yml`, which requests the resolve task automatically after Codex posts a PR review, inline review comment, or actionable top-level PR comment. -- The workflow must only trigger from Codex review bot comments on open pull requests. -- The workflow must skip review-thread replies so Codex fix summaries do not re-trigger the workflow. +- The workflow must only trigger from Codex review bot reviews or comments on open pull requests. +- The workflow must skip `APPROVED`/`DISMISSED` reviews and other no-finding review submissions. +- The workflow must skip review-thread replies and auto-resolve request comments so Codex fix summaries do not re-trigger the workflow. - The workflow must ask Codex to resolve all review comments using these repository instructions. -- The workflow must avoid duplicate requests for the same pull request head commit. +- The workflow must avoid duplicate requests for the same pull request head SHA using a PR label plus marker comment, and may request again when the head SHA changes. +- To force another resolve pass on the current head SHA, post the Primary PR command manually on the PR. - The workflow must not run Codex directly with API credentials. +- When auto-resolve is workflow-triggered, Codex may commit, push to the PR branch, reply on review threads, and resolve conversations without another confirmation step. Do not rerun hosted CI or call paid/provider APIs unless explicitly authorized. - P0 and P1 findings should always be fixed. - P2 and lower findings should be fixed only when clear, scoped, low-risk, and testable; otherwise explain the decision and resolve or mark ready for human resolution. diff --git a/README.md b/README.md index 924157799..691837bb4 100644 --- a/README.md +++ b/README.md @@ -167,6 +167,7 @@ hosted Supabase MCP server uses OAuth, not repo secrets. - `docs/reindex-runbook.md` — safe reindex and ingestion recovery - `docs/retrieval-quality-runbook.md` — RAG/retrieval eval gates - `docs/codex-prompt-playbook.md` — copy/paste prompts for common repo work +- `docs/codex-review-protocol.md` — shared Codex review throttling; pairs with `AGENTS.md` and `.github/workflows/codex-autofix-review-comments.yml` for PR review + auto-resolve - `docs/supabase-migration-reconciliation.md` — migration drift and repair policy - `docs/site-map.md` — generated route map (`npm run sitemap:update`) diff --git a/docs/branch-cleanup-guide.md b/docs/branch-cleanup-guide.md index d2b177e4c..f452202b1 100644 --- a/docs/branch-cleanup-guide.md +++ b/docs/branch-cleanup-guide.md @@ -41,7 +41,18 @@ Before deleting anything: git worktree list --porcelain ``` -5. For each candidate branch, check whether it has patch content not on `main`: +5. Filter candidates through the review ledger before inspecting branch diffs. Follow the lookup procedure in `docs/branch-review-ledger.md` and require all three fields to match before skipping: + + ```powershell + $branch = "BRANCH_NAME" + $head = git rev-parse $branch + Get-Content docs\branch-review-ledger.md | + Select-String -Pattern "\|\s*$branch\s*\|\s*$head\s*\|\s*branch-cleanup\s*\|" + ``` + + Skip the branch only when a ledger row matches the same branch/ref, reviewed HEAD, and `branch-cleanup` scope together. A branch-name-only match is not enough. Re-review when the HEAD changed or the user explicitly asks for a fresh pass. + +6. For each remaining candidate branch, check whether it has patch content not on `main`: ```powershell git log --format="%h %s" --right-only --cherry-pick main...BRANCH_NAME @@ -53,9 +64,11 @@ Delete a branch only when the cherry-pick-aware log is empty, or when the branch ## Recommended Cleanup Order 1. Fetch and inspect current branch state with the commands above. -2. For each candidate branch, confirm patch-unique commits and file diffs against `main`. -3. Port, commit, or explicitly reject useful patch content before deleting any branch ref. -4. Remove detached worktrees only when clean, unneeded, and absent from active `git worktree list` output. +2. Resolve each candidate's HEAD and skip unchanged completed reviews recorded in `docs/branch-review-ledger.md`. +3. For each remaining candidate branch, confirm patch-unique commits and file diffs against `main`. +4. Port, commit, or explicitly reject useful patch content before deleting any branch ref. +5. Record completed cleanup reviews in `docs/branch-review-ledger.md`. +6. Remove detached worktrees only when clean, unneeded, and absent from active `git worktree list` output. ## Final Verification diff --git a/docs/branch-review-ledger.md b/docs/branch-review-ledger.md new file mode 100644 index 000000000..031950135 --- /dev/null +++ b/docs/branch-review-ledger.md @@ -0,0 +1,28 @@ +# Branch Review Ledger + +Use this ledger to prevent repeated branch and PR reviews when the reviewed HEAD has not changed. + +## Scope + +- **In scope:** local agents, Cursor/Codex repo-local review skills, explicit branch cleanup, and other reviews where the agent can safely append this file. +- **Out of scope for required updates:** GitHub-hosted Codex PR reviews and auto-resolve. Those use PR labels and marker comments in `.github/workflows/codex-autofix-review-comments.yml`. +- **Optional on GitHub:** append a ledger row when the reviewed PR branch is checked out locally and a ledger-only commit is clearly safe. + +## Lookup Procedure + +1. Identify the target branch or ref from the user request. If no target is named, use the current branch. +2. Resolve the current HEAD: + + ```powershell + git rev-parse + ``` + +3. Search this file for a row with the same branch/ref, reviewed HEAD, and review scope. +4. If the HEAD and scope match an existing completed row, do not repeat the review. Summarize the prior outcome and ask before doing a fresh pass. +5. If the HEAD changed, or the user explicitly requests a fresh review, review the changed scope and append a new row. +6. For branch-cleanup passes, run this lookup before inspecting branch diffs so unchanged reviewed branches are filtered out early. + +## Review Records + +| Date | Branch or ref | Reviewed HEAD | Scope | Outcome | Checks | +| ---- | ------------- | ------------- | ----- | ------- | ------ | diff --git a/docs/codex-review-protocol.md b/docs/codex-review-protocol.md new file mode 100644 index 000000000..567fcfc81 --- /dev/null +++ b/docs/codex-review-protocol.md @@ -0,0 +1,45 @@ +# Codex Review Protocol + +Use this protocol for every Codex review, audit, bug hunt, PR review, release-readiness check, and branch-cleanup review in this repository. + +## Trigger and Scope + +- Do not start a review opportunistically. +- Review only when the user request, `AGENTS.md` routing rules, or an explicit handoff/release workflow calls for it. +- Review the current diff, named PR, named branch, or explicitly requested area. +- Do not expand into stale branches or unrelated modules unless a confirmed defect crosses that boundary. +- Before branch or PR review, check `docs/branch-review-ledger.md` when the review path is local or branch-cleanup scoped: resolve the target with `git rev-parse`, compare the HEAD and scope, and skip unchanged completed reviews unless the user asks for a fresh pass. +- GitHub-hosted Codex PR reviews and auto-resolve dedupe use PR labels and marker comments from `.github/workflows/codex-autofix-review-comments.yml`; ledger updates there are optional when a safe local commit is possible. + +## GitHub auto-resolve and manual re-trigger + +- `.github/workflows/codex-autofix-review-comments.yml` posts one auto-resolve request per pull request **head SHA** after Codex leaves actionable review comments, inline review comments, or top-level PR comments with findings. +- A new commit on the PR branch changes the head SHA, so a later Codex review can auto-trigger resolve again for that new SHA. +- To force another resolve pass on the **current** head SHA, post the Primary PR command from `AGENTS.md` on the PR: + + `@codex resolve all review comments using AGENTS.md § Codex GitHub review behavior (Primary PR command).` + +- Manual re-trigger is useful after a partial auto-resolve pass, when the workflow dedupe label/marker is already present, or when you want another fix pass without waiting for a new Codex review. + +## Review Output + +- Lead with findings, ordered by severity: P0, P1, P2, then P3. +- Each finding must include file/line evidence, trigger or failure path, expected behavior, actual risk, and the smallest proof or check that would catch it. +- If no high-confidence issue is found, say so clearly and name the highest residual risk area. +- Include checks run, checks not run, and whether any check was skipped because it could touch an API/provider. +- Keep summaries secondary to findings. + +## Mutation Rules + +- For a pure review request, do not edit files, stage, commit, push, post PR comments, rerun hosted CI, or call provider-backed services. + - Exception: append the completed review record to `docs/branch-review-ledger.md` so throttling state persists. +- If the user clearly asks to fix confirmed findings, make the smallest safe change and verify with local, static, or mocked checks first. +- Ask before any OpenAI, Supabase, GitHub/GitLab, hosted CI, or provider-backed workflow. +- After any completed local or branch-cleanup review, update `docs/branch-review-ledger.md` with date, branch/ref, HEAD, scope, outcome, and checks. This ledger append is allowed even during a pure review when the ledger is in scope for that path. + +## Severity Guide + +- P0: Data loss, security breach, production outage, or clinical safety issue likely to harm users immediately. +- P1: Broken core workflow, unsafe automation, privacy/auth failure, or repeatable defect that blocks merge/handoff. +- P2: Real defect, missing guardrail, fragile process, or test gap that should be fixed before relying on the work. +- P3: Low-risk cleanup, clarity, documentation, or future-proofing issue. diff --git a/package.json b/package.json index 643d03e0a..3c686b007 100644 --- a/package.json +++ b/package.json @@ -62,6 +62,7 @@ "check:m13-migration": "tsx scripts/check-m13-migration.ts", "check:july8-live-batch": "tsx scripts/check-july8-live-batch.ts", "check:type-scale": "node scripts/check-type-scale.mjs", + "check:codex-autofix-workflow": "node scripts/check-codex-autofix-workflow.mjs", "recover:ingestion": "tsx scripts/recover-ingestion-queue.ts", "registry:seed": "tsx scripts/seed-registry-records.ts", "registry:embed": "tsx scripts/embed-registry-records.ts", diff --git a/scripts/check-codex-autofix-workflow.mjs b/scripts/check-codex-autofix-workflow.mjs new file mode 100644 index 000000000..292d5ae7d --- /dev/null +++ b/scripts/check-codex-autofix-workflow.mjs @@ -0,0 +1,50 @@ +import { readFileSync } from "node:fs"; +import path from "node:path"; + +const workflowPath = path.join(process.cwd(), ".github", "workflows", "codex-autofix-review-comments.yml"); +const workflow = readFileSync(workflowPath, "utf8"); +const problems = []; + +function requirePattern(label, pattern) { + if (!pattern.test(workflow)) { + problems.push(`Missing required workflow guard: ${label}`); + } +} + +requirePattern("pull_request_review submitted trigger", /pull_request_review:\s*\n\s*types:\s*\[submitted\]/); +requirePattern( + "pull_request_review_comment created trigger", + /pull_request_review_comment:\s*\n\s*types:\s*\[created\]/, +); +requirePattern("issue_comment created trigger", /issue_comment:\s*\n\s*types:\s*\[created\]/); +requirePattern("open pull request job guard", /github\.event\.pull_request\.state == 'open'/); +requirePattern("open issue comment PR guard", /github\.event\.issue\.pull_request/); +requirePattern("Codex actor guard", /chatgpt-codex-connector/); +requirePattern("review-thread reply skip", /in_reply_to_id/); +requirePattern("auto-resolve request skip", /@codex resolve all review comments/); +requirePattern("approved review skip", /APPROVED/); +requirePattern("head SHA marker", /codex-autoresolve-pr:\$\{pr\.number\}:\$\{headSha\}/); +requirePattern("head SHA label lock", /codex-ar-\$\{headSha\}/); +requirePattern("pull-requests write permission", /pull-requests:\s*write/); +requirePattern( + "SHA-scoped concurrency queue", + /codex-autoresolve-\$\{\{ github\.event\.pull_request\.number \|\| github\.event\.issue\.number \}\}-\$\{\{ github\.event\.pull_request\.head\.sha \|\| 'issue-comment' \}\}/, +); +requirePattern("queued concurrency", /cancel-in-progress:\s*false/); +requirePattern("resolve completion summary skip", /isResolveCompletionSummary/); +requirePattern("ensure request label before apply", /ensureRequestLabel/); +requirePattern("comment before label lock", /createComment[\s\S]*ensureRequestLabel/); +requirePattern("permission soft-skip on comment read", /cannot read PR comments/); +requirePattern("AGENTS.md primary command reference", /AGENTS\.md § Codex GitHub review behavior/); + +if (workflow.includes("codex-autoresolve-pr:${pr.number} -->") && !workflow.includes("${headSha}")) { + problems.push("Workflow still uses per-PR-only dedupe marker without head SHA."); +} + +if (problems.length > 0) { + console.error("Codex auto-resolve workflow guard check failed:"); + for (const problem of problems) console.error(`- ${problem}`); + process.exit(1); +} + +console.log("Codex auto-resolve workflow guard check passed.");