diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 40f2dc678..b087b1203 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -53,6 +53,9 @@ jobs: - name: Runtime alignment run: npm run check:runtime + - name: Codex auto-resolve workflow guard + run: npm run check:codex-autofix-workflow + - name: Dependency vulnerability audit # Non-mutating gate. The document parsers (pdf-parse, pdfjs-dist, mammoth, # exceljs, jszip) process untrusted uploads, so high/critical advisories in diff --git a/.github/workflows/codex-autofix-review-comments.yml b/.github/workflows/codex-autofix-review-comments.yml index b7adedaab..e34f6c537 100644 --- a/.github/workflows/codex-autofix-review-comments.yml +++ b/.github/workflows/codex-autofix-review-comments.yml @@ -49,12 +49,23 @@ jobs: const issue_number = pr.number; const marker = ``; - 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 GitHub denied permission to list issue comments."); + core.notice("Grant issues: read/write permission or allow the workflow token to read issue comments to enable duplicate request checks."); + return; + } + + throw error; + } if (existingComments.some((comment) => (comment.body || "").includes(marker))) { core.notice("Skipping duplicate Codex auto-resolve request for this pull request."); @@ -79,5 +90,6 @@ jobs: core.warning("Skipping Codex auto-resolve request because this workflow token cannot comment on the pull request."); return; } + throw error; } diff --git a/AGENTS.md b/AGENTS.md index ce9ddff27..0cb5083fd 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -335,11 +335,12 @@ 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 one scoped resolve task after Codex posts an inline PR review comment. - The workflow must only trigger from Codex review bot comments on open pull requests. +- Do not add `issue_comment`, `pull_request_review`, scheduled, push, or broad PR triggers to this workflow without explicit human approval. - 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 ask Codex to resolve only actionable Codex review findings for the triggering pull request and current head. - The workflow must avoid duplicate requests for the same pull request, even after follow-up commits change the head SHA. - The workflow must not run Codex directly with API credentials. - P0 and P1 findings should always be fixed. @@ -347,7 +348,7 @@ Automatic Codex review is review-only by default. This repository includes `.git ### Primary PR command -`@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 resolve actionable Codex review findings for this pull request and current head 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. Do not update the branch from main, address unrelated reviews, broaden scope, or create more than one scoped fix commit unless explicitly asked. 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.` ## Cursor Cloud specific instructions diff --git a/docs/branch-review-ledger.md b/docs/branch-review-ledger.md index e4d61b8c5..9226ec5f6 100644 --- a/docs/branch-review-ledger.md +++ b/docs/branch-review-ledger.md @@ -18,6 +18,6 @@ Use this ledger to prevent repeated branch and PR reviews when the reviewed HEAD ## Review Records -| Date | Branch or ref | Reviewed HEAD | Scope | Outcome | Checks | -| ---------- | -------------- | ------------- | -------------- | ------------------------------------------------------------- | ---------------------------------------------------------------------------------------------------------- | -| 2026-07-09 | example/branch | abc1234 | branch-cleanup | Example: already merged into `main`; no unique patch content. | `git log --right-only --cherry-pick main...example/branch`; `git diff --name-status main...example/branch` | +| Date | Branch or ref | Reviewed HEAD | Scope | Outcome | Checks | +| ---------- | -------------- | --------------------------------------- | -------------- | ------------------------------------------------------------- | ---------------------------------------------------------------------------------------------------------- | +| 2026-07-09 | example/branch | abc1234567890abcdef1234567890abcdef1234 | branch-cleanup | Example: already merged into `main`; no unique patch content. | `git log --right-only --cherry-pick main...example/branch`; `git diff --name-status main...example/branch` | diff --git a/docs/codex-review-protocol.md b/docs/codex-review-protocol.md index 41541489c..ef1c4f5ca 100644 --- a/docs/codex-review-protocol.md +++ b/docs/codex-review-protocol.md @@ -21,10 +21,10 @@ Use this protocol for every Codex review, audit, bug hunt, PR review, release-re ## 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 branch/PR 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. +- For pure reviews, do not update `docs/branch-review-ledger.md` unless the user explicitly asks for a committed process update. If a ledger note would be useful, mention the suggested row in the final response instead. +- When the user explicitly asks for a branch/PR review record, update `docs/branch-review-ledger.md` with date, branch/ref, HEAD, scope, outcome, and checks as a scoped documentation change. ## Severity Guide diff --git a/package.json b/package.json index 643d03e0a..fce7d434a 100644 --- a/package.json +++ b/package.json @@ -29,6 +29,7 @@ "sitemap:update": "tsx scripts/generate-site-map.ts", "sitemap:check": "tsx scripts/generate-site-map.ts --check", "check:runtime": "tsx scripts/check-runtime.ts", + "check:codex-autofix-workflow": "node scripts/check-codex-autofix-workflow.mjs", "check:deployment-readiness": "node scripts/deployment-boot-smoke.mjs", "check:edge:functions": "node scripts/check-edge-functions.mjs", "check:production-readiness": "tsx scripts/production-readiness.ts", diff --git a/scripts/check-codex-autofix-workflow.mjs b/scripts/check-codex-autofix-workflow.mjs new file mode 100644 index 000000000..75a6a6e34 --- /dev/null +++ b/scripts/check-codex-autofix-workflow.mjs @@ -0,0 +1,53 @@ +import fs from "node:fs"; + +const workflowPath = ".github/workflows/codex-autofix-review-comments.yml"; +const workflow = fs.readFileSync(workflowPath, "utf8"); + +const failures = []; + +const forbiddenPatterns = [ + { + pattern: /github\.event\.pull_request\.head\.sha/, + message: "Do not key the Codex auto-resolve workflow on github.event.pull_request.head.sha.", + }, + { + pattern: /pr\.head\.sha/, + message: "Do not key the Codex auto-resolve workflow on pr.head.sha.", + }, + { + pattern: /^\s*issue_comment:/m, + message: "Do not trigger Codex auto-resolve from issue_comment events.", + }, + { + pattern: /^\s*pull_request_review:/m, + message: "Do not trigger Codex auto-resolve from whole pull_request_review events.", + }, +]; + +for (const { pattern, message } of forbiddenPatterns) { + if (pattern.test(workflow)) { + failures.push(message); + } +} + +if (!workflow.includes("group: codex-autoresolve-${{ github.event.pull_request.number }}")) { + failures.push("Codex auto-resolve concurrency must be scoped to the pull request number only."); +} + +if (!workflow.includes("codex-autoresolve-pr:${pr.number}")) { + failures.push("Codex auto-resolve marker must be scoped to the pull request, not the head SHA."); +} + +if (!workflow.includes('sourceBody.includes("@codex resolve all review comments")')) { + failures.push("Codex auto-resolve workflow must skip comments that already look like resolve requests."); +} + +if (failures.length > 0) { + console.error(`Codex auto-resolve workflow guard failed for ${workflowPath}:`); + for (const failure of failures) { + console.error(`- ${failure}`); + } + process.exit(1); +} + +console.log("Codex auto-resolve workflow guard passed.");