From 44875a426359c5247debf25afb2bdbff9445f504 Mon Sep 17 00:00:00 2001 From: Punk 6529 <108035228+punk6529@users.noreply.github.com> Date: Sat, 4 Jul 2026 11:44:56 +0000 Subject: [PATCH 1/4] Wire automatic self reviews for this repository Add a self-review caller workflow that reviews this repo's own PRs with the reusable review workflow: general, security, auth-api, deploy-actions, db-lambda, and glm-swarm on initial review, followup on pushed commits, and maintainer /6529bot comment commands parsed with the same parser the App uses. Kind selection lives in bin/self-review-plan.cjs, guarded by check-review-workflow-kinds and smoke tests. Co-Authored-By: Claude Fable 5 --- .github/workflows/self-review.yml | 82 +++++++++++++++++ bin/self-review-plan.cjs | 113 ++++++++++++++++++++++++ docs/review-workflows.md | 23 +++++ scripts/check-review-workflow-kinds.cjs | 68 ++++++++++++++ scripts/smoke-test.cjs | 97 ++++++++++++++++++++ 5 files changed, 383 insertions(+) create mode 100644 .github/workflows/self-review.yml create mode 100644 bin/self-review-plan.cjs diff --git a/.github/workflows/self-review.yml b/.github/workflows/self-review.yml new file mode 100644 index 0000000..13da2b7 --- /dev/null +++ b/.github/workflows/self-review.yml @@ -0,0 +1,82 @@ +name: 6529bot Self Review + +on: + pull_request: + types: [opened, synchronize, reopened, ready_for_review] + issue_comment: + types: [created] + workflow_dispatch: + inputs: + pr_number: + description: Pull request number to review. + required: true + type: string + review_kinds_json: + description: JSON array of review kinds. Empty uses the initial set. + required: false + default: "" + type: string + +permissions: + contents: read + issues: write + pull-requests: read + id-token: write + +jobs: + plan: + name: Plan review kinds + runs-on: ubuntu-latest + if: >- + github.event_name == 'workflow_dispatch' || + (github.event_name == 'pull_request' && + github.event.pull_request.draft == false && + github.event.pull_request.head.repo.full_name == github.repository) || + (github.event_name == 'issue_comment' && + github.event.issue.pull_request && + contains(fromJSON('["OWNER","MEMBER","COLLABORATOR"]'), github.event.comment.author_association) && + (startsWith(github.event.comment.body, '/6529bot') || startsWith(github.event.comment.body, '@6529bot'))) + outputs: + pr_number: ${{ steps.plan.outputs.pr_number }} + head_sha: ${{ steps.plan.outputs.head_sha }} + event_action: ${{ steps.plan.outputs.event_action }} + review_kinds_json: ${{ steps.plan.outputs.review_kinds_json }} + steps: + - name: Checkout bot + uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3 + with: + persist-credentials: false + + - name: Set up Node + uses: actions/setup-node@48b55a011bda9f5d6aeb4c2d9c7362e8dae4041e # v6.4.0 + with: + node-version: 22 + + - name: Plan review kinds + id: plan + env: + GH_TOKEN: ${{ github.token }} + EVENT_NAME: ${{ github.event_name }} + EVENT_ACTION: ${{ github.event.action || 'manual' }} + PR_NUMBER: ${{ github.event.pull_request.number || github.event.issue.number || inputs.pr_number }} + PR_HEAD_SHA: ${{ github.event.pull_request.head.sha || '' }} + COMMENT_BODY: ${{ github.event.comment.body || '' }} + REQUESTED_KINDS_JSON: ${{ inputs.review_kinds_json || '' }} + INITIAL_KINDS_JSON: '["general","security","auth-api","deploy-actions","db-lambda","glm-swarm"]' + SYNCHRONIZE_KINDS_JSON: '["followup"]' + run: node bin/self-review-plan.cjs + + review: + name: Review + needs: plan + if: ${{ needs.plan.outputs.review_kinds_json != '' && needs.plan.outputs.review_kinds_json != '[]' }} + uses: ./.github/workflows/review.yml + with: + pr_number: ${{ needs.plan.outputs.pr_number }} + head_sha: ${{ needs.plan.outputs.head_sha }} + event_action: ${{ needs.plan.outputs.event_action }} + review_kinds_json: ${{ needs.plan.outputs.review_kinds_json }} + secrets: + ANTHROPIC_API_KEY: ${{ secrets.ANTHROPIC_API_KEY }} + OPENAI_API_KEY: ${{ secrets.OPENAI_API_KEY }} + OPENROUTER_API_KEY: ${{ secrets.OPENROUTER_API_KEY }} diff --git a/bin/self-review-plan.cjs b/bin/self-review-plan.cjs new file mode 100644 index 0000000..6d54b2b --- /dev/null +++ b/bin/self-review-plan.cjs @@ -0,0 +1,113 @@ +#!/usr/bin/env node + +"use strict"; + +const fs = require("fs"); +const { execFileSync } = require("child_process"); +const { REVIEW_KINDS, parseReviewCommand } = require("../src/github-webhook.cjs"); + +function planSelfReview(env = process.env, options = {}) { + const eventName = String(env.EVENT_NAME || "").trim(); + const eventAction = String(env.EVENT_ACTION || "manual").trim() || "manual"; + const prNumber = String(env.PR_NUMBER || "").trim(); + if (!prNumber) { + throw new Error("PR_NUMBER is required."); + } + let reviewKinds; + if (eventName === "workflow_dispatch") { + reviewKinds = String(env.REQUESTED_KINDS_JSON || "").trim() + ? parseKindsJson(env.REQUESTED_KINDS_JSON, "review_kinds_json") + : parseKindsJson(env.INITIAL_KINDS_JSON, "INITIAL_KINDS_JSON"); + } else if (eventName === "pull_request") { + reviewKinds = + eventAction === "synchronize" + ? parseKindsJson(env.SYNCHRONIZE_KINDS_JSON, "SYNCHRONIZE_KINDS_JSON") + : parseKindsJson(env.INITIAL_KINDS_JSON, "INITIAL_KINDS_JSON"); + } else if (eventName === "issue_comment") { + const command = parseReviewCommand(String(env.COMMENT_BODY || "")); + reviewKinds = command?.reviewKinds?.length ? command.reviewKinds : []; + } else { + throw new Error(`Unsupported event '${eventName}'.`); + } + let headSha = String(env.PR_HEAD_SHA || "").trim(); + if (!headSha && reviewKinds.length) { + const resolveHeadSha = options.resolveHeadSha || resolveHeadShaWithGh; + headSha = resolveHeadSha(prNumber, env); + } + return { + prNumber, + headSha, + eventAction, + reviewKinds, + }; +} + +function parseKindsJson(value, label) { + let parsed; + try { + parsed = JSON.parse(String(value || "")); + } catch (error) { + throw new Error(`${label} must be a JSON array of review kinds: ${error.message}`); + } + if (!Array.isArray(parsed) || parsed.length === 0) { + throw new Error(`${label} must be a non-empty JSON array of review kinds.`); + } + for (const kind of parsed) { + if (!REVIEW_KINDS.includes(kind)) { + throw new Error(`${label} includes unknown review kind '${kind}'.`); + } + } + return parsed; +} + +function resolveHeadShaWithGh(prNumber, env = process.env) { + const repository = String(env.GITHUB_REPOSITORY || "").trim(); + if (!repository) { + throw new Error("GITHUB_REPOSITORY is required to resolve the PR head SHA."); + } + const output = execFileSync( + "gh", + ["api", `repos/${repository}/pulls/${prNumber}`, "--jq", ".head.sha"], + { encoding: "utf8" } + ); + const headSha = String(output || "").trim(); + if (!headSha) { + throw new Error(`Could not resolve head SHA for PR #${prNumber}.`); + } + return headSha; +} + +function main() { + const plan = planSelfReview(process.env); + const outputPath = process.env.GITHUB_OUTPUT; + if (!outputPath) { + throw new Error("GITHUB_OUTPUT is required."); + } + const lines = [ + `pr_number=${plan.prNumber}`, + `head_sha=${plan.headSha}`, + `event_action=${plan.eventAction}`, + `review_kinds_json=${JSON.stringify(plan.reviewKinds)}`, + ]; + fs.appendFileSync(outputPath, `${lines.join("\n")}\n`); + if (plan.reviewKinds.length === 0) { + console.log(`No review kinds requested for PR #${plan.prNumber}; skipping review.`); + return; + } + console.log( + `Planned self review for PR #${plan.prNumber} @ ${plan.headSha}: ${plan.reviewKinds.join(", ")}` + ); +} + +if (require.main === module) { + try { + main(); + } catch (error) { + console.error(error.message || error); + process.exit(1); + } +} + +module.exports = { + planSelfReview, +}; diff --git a/docs/review-workflows.md b/docs/review-workflows.md index 4d9b1fe..48360e8 100644 --- a/docs/review-workflows.md +++ b/docs/review-workflows.md @@ -11,6 +11,29 @@ lane for GitHub Actions budget accounting. See Maintainer comment triggers are documented in [comment-commands.md](comment-commands.md). +## Self Review + +This repository reviews its own pull requests through GitHub Actions instead +of the central App webhook. `.github/workflows/self-review.yml` triggers on +pull request events, maintainer `/6529bot` comments, and manual dispatch, then +calls the reusable `.github/workflows/review.yml` workflow with review kinds +planned by `bin/self-review-plan.cjs`: + +- Initial reviews (opened, reopened, ready for review): `general`, + `security`, `auth-api`, `deploy-actions`, `db-lambda`, and `glm-swarm` — + the kinds that match this repository's backend, GitHub App auth, GitHub + Actions, and RDS Data API surface. +- Pushed commits (synchronize): `followup`. +- Comment commands: parsed with the same `parseReviewCommand` used by the + App, restricted to owner, member, or collaborator comment authors. + +Draft pull requests are skipped until they are marked ready for review. +Frontend and repo-specific kinds (`wcag`, `i18n`, `responsiveness`, +`media-external`, `stream-contracts`, `safe-write`, `release-deploy`, +`privacy-evidence`, `signer-ux`) stay out of the automatic set but remain +available through comment commands. `scripts/check-review-workflow-kinds.cjs` +enforces that the planned kinds stay valid. + ## General PR Review Entrypoint: diff --git a/scripts/check-review-workflow-kinds.cjs b/scripts/check-review-workflow-kinds.cjs index f9dcf84..0e884a1 100644 --- a/scripts/check-review-workflow-kinds.cjs +++ b/scripts/check-review-workflow-kinds.cjs @@ -20,6 +20,7 @@ const dispatchWorkflowPaths = [ "templates/review-job-workflow.yml", ]; const reusableWorkflowPath = ".github/workflows/review.yml"; +const selfReviewWorkflowPath = ".github/workflows/self-review.yml"; const SYNCHRONIZE_REVIEW_KINDS = ["followup"]; const SPECIAL_WORKFLOW_REVIEW_KINDS = ["responsiveness"]; @@ -71,6 +72,12 @@ function checkReviewWorkflowKinds(options = {}) { findings ); checkReusableWorkflowCaseStatement(reusableText, providerReviewKinds, reviewKindBins, findings); + checkSelfReviewWorkflow( + options.selfReviewWorkflowText || + fs.readFileSync(path.join(root, selfReviewWorkflowPath), "utf8"), + reviewKinds, + findings + ); if (findings.length) { if (!options.quiet) { @@ -153,6 +160,67 @@ function checkResponsivenessArtifactUrlHandoff(workflowPath, workflowText, findi } } +function checkSelfReviewWorkflow(workflowText, reviewKinds, findings) { + try { + YAML.parse(workflowText); + } catch (error) { + findings.push(`${selfReviewWorkflowPath} must be valid YAML: ${error.message}`); + return; + } + const normalized = normalizeWhitespace(workflowText); + for (const snippet of [ + "uses: ./.github/workflows/review.yml", + "node bin/self-review-plan.cjs", + "author_association", + "ANTHROPIC_API_KEY: ${{ secrets.ANTHROPIC_API_KEY }}", + ]) { + if (!normalized.includes(normalizeWhitespace(snippet))) { + findings.push(`${selfReviewWorkflowPath} must include '${snippet}'.`); + } + } + for (const [label, expectNonEmpty] of [ + ["INITIAL_KINDS_JSON", true], + ["SYNCHRONIZE_KINDS_JSON", true], + ]) { + const match = workflowText.match(new RegExp(`${label}:\\s*'([^']+)'`)); + if (!match) { + findings.push(`${selfReviewWorkflowPath} must define ${label} as a quoted JSON array.`); + continue; + } + let kinds; + try { + kinds = JSON.parse(match[1]); + } catch { + kinds = null; + } + if (!Array.isArray(kinds) || (expectNonEmpty && kinds.length === 0)) { + findings.push(`${selfReviewWorkflowPath} ${label} must be a non-empty JSON array.`); + continue; + } + for (const kind of kinds) { + if (!reviewKinds.includes(kind)) { + findings.push(`${selfReviewWorkflowPath} ${label} includes unknown review kind '${kind}'.`); + } + } + } + const synchronizeMatch = workflowText.match(/SYNCHRONIZE_KINDS_JSON:\s*'([^']+)'/); + if (synchronizeMatch) { + let kinds; + try { + kinds = JSON.parse(synchronizeMatch[1]); + } catch { + kinds = null; + } + if (!arraysEqual(kinds || [], SYNCHRONIZE_REVIEW_KINDS)) { + findings.push( + `${selfReviewWorkflowPath} SYNCHRONIZE_KINDS_JSON must be ${JSON.stringify( + SYNCHRONIZE_REVIEW_KINDS + )}.` + ); + } + } +} + function checkReviewKindBins(reviewKinds, reviewKindBins, findings) { const binKinds = Object.keys(reviewKindBins); if (!arraysEqual(binKinds, reviewKinds)) { diff --git a/scripts/smoke-test.cjs b/scripts/smoke-test.cjs index 8f9b410..38de2b0 100644 --- a/scripts/smoke-test.cjs +++ b/scripts/smoke-test.cjs @@ -34,6 +34,7 @@ const dataApi = require("../src/data-api.cjs"); const diagnostics = require("../src/diagnostics.cjs"); const dogfoodTarget = require("../src/dogfood-target.cjs"); const dogfoodTargetCli = require("../bin/dogfood-target.cjs"); +const selfReviewPlanCli = require("../bin/self-review-plan.cjs"); const dogfoodTargetContractCheck = require("./check-dogfood-target-contract.cjs"); const dogfoodReadiness = require("../src/dogfood-readiness.cjs"); const dogfoodReadinessCli = require("../bin/dogfood-readiness.cjs"); @@ -4641,6 +4642,102 @@ assert.throws( }), /review workflow kind check found/ ); +assert.throws( + () => + reviewWorkflowKindsCheck.checkReviewWorkflowKinds({ + quiet: true, + selfReviewWorkflowText: "jobs: {}\n", + }), + /review workflow kind check found/ +); +const selfReviewPlanEnv = { + INITIAL_KINDS_JSON: '["general","security","auth-api"]', + SYNCHRONIZE_KINDS_JSON: '["followup"]', +}; +assert.deepEqual( + selfReviewPlanCli.planSelfReview({ + ...selfReviewPlanEnv, + EVENT_NAME: "pull_request", + EVENT_ACTION: "opened", + PR_NUMBER: "415", + PR_HEAD_SHA: "self-review-head", + }), + { + prNumber: "415", + headSha: "self-review-head", + eventAction: "opened", + reviewKinds: ["general", "security", "auth-api"], + } +); +assert.deepEqual( + selfReviewPlanCli.planSelfReview({ + ...selfReviewPlanEnv, + EVENT_NAME: "pull_request", + EVENT_ACTION: "synchronize", + PR_NUMBER: "415", + PR_HEAD_SHA: "self-review-head", + }).reviewKinds, + ["followup"] +); +assert.deepEqual( + selfReviewPlanCli.planSelfReview( + { + ...selfReviewPlanEnv, + EVENT_NAME: "issue_comment", + EVENT_ACTION: "created", + PR_NUMBER: "415", + COMMENT_BODY: "/6529bot review security db-lambda", + }, + { resolveHeadSha: () => "resolved-head" } + ), + { + prNumber: "415", + headSha: "resolved-head", + eventAction: "created", + reviewKinds: ["security", "db-lambda"], + } +); +assert.deepEqual( + selfReviewPlanCli.planSelfReview({ + ...selfReviewPlanEnv, + EVENT_NAME: "issue_comment", + EVENT_ACTION: "created", + PR_NUMBER: "415", + COMMENT_BODY: "looks good to me", + }).reviewKinds, + [] +); +assert.deepEqual( + selfReviewPlanCli.planSelfReview( + { + ...selfReviewPlanEnv, + EVENT_NAME: "workflow_dispatch", + PR_NUMBER: "415", + REQUESTED_KINDS_JSON: '["deploy-actions"]', + }, + { resolveHeadSha: () => "resolved-head" } + ).reviewKinds, + ["deploy-actions"] +); +assert.throws( + () => + selfReviewPlanCli.planSelfReview({ + ...selfReviewPlanEnv, + EVENT_NAME: "workflow_dispatch", + PR_NUMBER: "415", + REQUESTED_KINDS_JSON: '["not-a-kind"]', + }), + /unknown review kind 'not-a-kind'/ +); +assert.throws( + () => + selfReviewPlanCli.planSelfReview({ + ...selfReviewPlanEnv, + EVENT_NAME: "pull_request", + EVENT_ACTION: "opened", + }), + /PR_NUMBER is required/ +); assert.equal(replayWebhook.inferEventName(JSON.parse(webhookBody.toString("utf8"))), "pull_request"); assert.equal(replayWebhook.parsePayload(Buffer.from(`\uFEFF${webhookBody}`)).action, "opened"); assert.equal(replayWebhook.normalizePayloadBody(Buffer.from(`\uFEFF${webhookBody}`))[0], 123); From a672cf56971eb15f9a513a9f6827efabd645d385 Mon Sep 17 00:00:00 2001 From: Punk 6529 <108035228+punk6529@users.noreply.github.com> Date: Sat, 4 Jul 2026 11:55:01 +0000 Subject: [PATCH 2/4] Post self reviews with installation tokens The first self-review run failed posting comments: github.token cannot comment on PRs with pull-requests read-only, and the workflow permission contract deliberately keeps it that way. Rework the self-review workflow to mirror the production worker instead: the plan step builds full job payloads with createReviewJobs, and each matrix job mints a GitHub App installation token and runs worker:job, posting as the bot with usage ledger accounting like App-dispatched reviews. Co-Authored-By: Claude Fable 5 --- .github/workflows/self-review.yml | 125 ++++++++++++++++++++---- bin/self-review-plan.cjs | 84 +++++++++++++--- docs/review-workflows.md | 9 +- scripts/check-review-workflow-kinds.cjs | 3 +- scripts/smoke-test.cjs | 114 ++++++++++++--------- 5 files changed, 257 insertions(+), 78 deletions(-) diff --git a/.github/workflows/self-review.yml b/.github/workflows/self-review.yml index 13da2b7..75402cf 100644 --- a/.github/workflows/self-review.yml +++ b/.github/workflows/self-review.yml @@ -1,4 +1,5 @@ name: 6529bot Self Review +run-name: "self-review ${{ github.event.pull_request.number || github.event.issue.number || inputs.pr_number }} (${{ github.event_name }})" on: pull_request: @@ -19,8 +20,6 @@ on: permissions: contents: read - issues: write - pull-requests: read id-token: write jobs: @@ -37,10 +36,7 @@ jobs: contains(fromJSON('["OWNER","MEMBER","COLLABORATOR"]'), github.event.comment.author_association) && (startsWith(github.event.comment.body, '/6529bot') || startsWith(github.event.comment.body, '@6529bot'))) outputs: - pr_number: ${{ steps.plan.outputs.pr_number }} - head_sha: ${{ steps.plan.outputs.head_sha }} - event_action: ${{ steps.plan.outputs.event_action }} - review_kinds_json: ${{ steps.plan.outputs.review_kinds_json }} + jobs_json: ${{ steps.plan.outputs.jobs_json }} steps: - name: Checkout bot uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3 @@ -60,23 +56,116 @@ jobs: EVENT_ACTION: ${{ github.event.action || 'manual' }} PR_NUMBER: ${{ github.event.pull_request.number || github.event.issue.number || inputs.pr_number }} PR_HEAD_SHA: ${{ github.event.pull_request.head.sha || '' }} + PR_BASE_SHA: ${{ github.event.pull_request.base.sha || '' }} + PR_HEAD_REF: ${{ github.event.pull_request.head.ref || '' }} COMMENT_BODY: ${{ github.event.comment.body || '' }} REQUESTED_KINDS_JSON: ${{ inputs.review_kinds_json || '' }} INITIAL_KINDS_JSON: '["general","security","auth-api","deploy-actions","db-lambda","glm-swarm"]' SYNCHRONIZE_KINDS_JSON: '["followup"]' + SELF_INSTALLATION_ID: ${{ vars.REVIEWBOT_SELF_INSTALLATION_ID }} run: node bin/self-review-plan.cjs review: - name: Review + name: "${{ matrix.job.reviewKind }} @ ${{ matrix.job.headSha }}" needs: plan - if: ${{ needs.plan.outputs.review_kinds_json != '' && needs.plan.outputs.review_kinds_json != '[]' }} - uses: ./.github/workflows/review.yml - with: - pr_number: ${{ needs.plan.outputs.pr_number }} - head_sha: ${{ needs.plan.outputs.head_sha }} - event_action: ${{ needs.plan.outputs.event_action }} - review_kinds_json: ${{ needs.plan.outputs.review_kinds_json }} - secrets: - ANTHROPIC_API_KEY: ${{ secrets.ANTHROPIC_API_KEY }} - OPENAI_API_KEY: ${{ secrets.OPENAI_API_KEY }} - OPENROUTER_API_KEY: ${{ secrets.OPENROUTER_API_KEY }} + if: ${{ needs.plan.outputs.jobs_json != '' && needs.plan.outputs.jobs_json != '[]' }} + runs-on: ubuntu-latest + timeout-minutes: ${{ fromJSON(vars.REVIEWBOT_WORKER_TIMEOUT_MINUTES || '20') }} + strategy: + fail-fast: false + matrix: + job: ${{ fromJSON(needs.plan.outputs.jobs_json) }} + concurrency: + group: reviewbot-worker-${{ matrix.job.runKey }} + cancel-in-progress: false + steps: + - name: Checkout bot + uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3 + with: + persist-credentials: false + + - name: Set up Node + uses: actions/setup-node@48b55a011bda9f5d6aeb4c2d9c7362e8dae4041e # v6.4.0 + with: + node-version: 22 + cache: npm + + - name: Install bot dependencies + run: npm ci + + - name: Configure AWS credentials for reviewbot ledgers + if: ${{ vars.REVIEW_USAGE_AWS_ROLE_ARN != '' }} + uses: aws-actions/configure-aws-credentials@e7f100cf4c008499ea8adda475de1042d6975c7b # v6.2.0 + with: + aws-region: ${{ vars.REVIEW_USAGE_AWS_REGION || 'us-east-1' }} + role-to-assume: ${{ vars.REVIEW_USAGE_AWS_ROLE_ARN }} + + - name: Mint target installation token + id: installation-token + env: + REVIEWBOT_GITHUB_APP_ID: ${{ secrets.REVIEWBOT_GITHUB_APP_ID }} + REVIEWBOT_GITHUB_APP_PRIVATE_KEY_BASE64: ${{ secrets.REVIEWBOT_GITHUB_APP_PRIVATE_KEY_BASE64 }} + run: | + node bin/github-app-installation-token.cjs \ + --installation-id "${{ matrix.job.installationId }}" \ + --github-actions-output + + - name: Checkout target repository + uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3 + with: + repository: ${{ matrix.job.headRepoFullName }} + ref: ${{ matrix.job.headSha }} + path: target + token: ${{ steps.installation-token.outputs.token }} + persist-credentials: false + + - name: Write job payload + env: + JOB_JSON: ${{ toJSON(matrix.job) }} + run: printf '%s\n' "$JOB_JSON" > job.json + + - name: Run 6529bot review job + env: + GH_TOKEN: ${{ steps.installation-token.outputs.token }} + ANTHROPIC_API_KEY: ${{ secrets.ANTHROPIC_API_KEY }} + OPENAI_API_KEY: ${{ secrets.OPENAI_API_KEY }} + OPENROUTER_API_KEY: ${{ secrets.OPENROUTER_API_KEY }} + REVIEW_WORKSPACE: ${{ github.workspace }}/target + PR_BASE_SHA: ${{ matrix.job.baseSha }} + PR_HEAD_REF: ${{ matrix.job.headRefName }} + PR_HEAD_REPO: ${{ matrix.job.headRepoFullName }} + REVIEW_USAGE_ENABLED: true + REVIEW_USAGE_AWS_REGION: ${{ vars.REVIEW_USAGE_AWS_REGION }} + REVIEW_USAGE_DB_RESOURCE_ARN: ${{ vars.REVIEW_USAGE_DB_RESOURCE_ARN }} + REVIEW_USAGE_DB_SECRET_ARN: ${{ vars.REVIEW_USAGE_DB_SECRET_ARN }} + REVIEW_USAGE_DB_NAME: ${{ vars.REVIEW_USAGE_DB_NAME }} + REVIEW_USAGE_DB_SCHEMA: ${{ vars.REVIEW_USAGE_DB_SCHEMA }} + REVIEWBOT_RUN_CONTROL_LEDGER_ENABLED: ${{ vars.REVIEWBOT_RUN_CONTROL_LEDGER_ENABLED || 'false' }} + REVIEWBOT_RUN_CONTROL_LEDGER_AWS_REGION: ${{ vars.REVIEWBOT_RUN_CONTROL_LEDGER_AWS_REGION || vars.REVIEW_USAGE_AWS_REGION }} + REVIEWBOT_RUN_CONTROL_LEDGER_DB_RESOURCE_ARN: ${{ vars.REVIEWBOT_RUN_CONTROL_LEDGER_DB_RESOURCE_ARN || vars.REVIEW_USAGE_DB_RESOURCE_ARN }} + REVIEWBOT_RUN_CONTROL_LEDGER_DB_SECRET_ARN: ${{ vars.REVIEWBOT_RUN_CONTROL_LEDGER_DB_SECRET_ARN || vars.REVIEW_USAGE_DB_SECRET_ARN }} + REVIEWBOT_RUN_CONTROL_LEDGER_DB_NAME: ${{ vars.REVIEWBOT_RUN_CONTROL_LEDGER_DB_NAME || vars.REVIEW_USAGE_DB_NAME }} + REVIEWBOT_RUN_CONTROL_LEDGER_DB_SCHEMA: ${{ vars.REVIEWBOT_RUN_CONTROL_LEDGER_DB_SCHEMA || vars.REVIEW_USAGE_DB_SCHEMA }} + REVIEWBOT_WORKER_INCLUDE_OUTPUT: ${{ vars.REVIEWBOT_WORKER_INCLUDE_OUTPUT || 'false' }} + REVIEW_DRAFT_PR_MODE: allow + REVIEW_MAX_CHANGED_FILES: ${{ vars.REVIEW_MAX_CHANGED_FILES || '300' }} + REVIEW_MAX_CHANGED_LINES: ${{ vars.REVIEW_MAX_CHANGED_LINES || '30000' }} + REVIEW_LARGE_PR_CHANGED_LINES: ${{ vars.REVIEW_LARGE_PR_CHANGED_LINES || '3500' }} + REVIEW_MAX_DIFF_CHARS: ${{ vars.REVIEW_MAX_DIFF_CHARS || '250000' }} + REVIEW_MAX_CONTEXT_CHARS: ${{ vars.REVIEW_MAX_CONTEXT_CHARS || '100000' }} + REVIEW_MAX_INPUT_CHARS: ${{ vars.REVIEW_MAX_INPUT_CHARS || '350000' }} + REVIEW_MAX_OUTPUT_TOKENS: ${{ vars.REVIEW_MAX_OUTPUT_TOKENS || '4000' }} + REVIEW_OVERSIZE_BEHAVIOR: ${{ vars.REVIEW_OVERSIZE_BEHAVIOR || 'skip' }} + REVIEW_GLM_SWARM_ENABLED: ${{ vars.REVIEW_GLM_SWARM_ENABLED || 'true' }} + REVIEW_GLM_SWARM_MAX_REVIEWERS: ${{ vars.REVIEW_GLM_SWARM_MAX_REVIEWERS || '4' }} + REVIEW_GLM_SWARM_MAX_FILES_PER_REVIEWER: ${{ vars.REVIEW_GLM_SWARM_MAX_FILES_PER_REVIEWER || '12' }} + REVIEW_GLM_SWARM_REVIEWER_MAX_OUTPUT_TOKENS: ${{ vars.REVIEW_GLM_SWARM_REVIEWER_MAX_OUTPUT_TOKENS || '1200' }} + REVIEW_GLM_SWARM_SYNTHESIS_MAX_OUTPUT_TOKENS: ${{ vars.REVIEW_GLM_SWARM_SYNTHESIS_MAX_OUTPUT_TOKENS || '1800' }} + REVIEW_GLM_SWARM_MAX_TOTAL_OUTPUT_TOKENS: ${{ vars.REVIEW_GLM_SWARM_MAX_TOTAL_OUTPUT_TOKENS || '7000' }} + REVIEW_GLM_SWARM_MAX_COST_USD: ${{ vars.REVIEW_GLM_SWARM_MAX_COST_USD || '2' }} + REVIEW_GLM_SWARM_RAW_OUTPUTS_MODE: ${{ vars.REVIEW_GLM_SWARM_RAW_OUTPUTS_MODE || 'off' }} + REVIEW_GLM_SWARM_RAW_OUTPUTS_FAIL_CLOSED: ${{ vars.REVIEW_GLM_SWARM_RAW_OUTPUTS_FAIL_CLOSED || 'false' }} + REVIEW_GLM_SWARM_RAW_OUTPUTS_AWS_REGION: ${{ vars.REVIEW_GLM_SWARM_RAW_OUTPUTS_AWS_REGION || vars.REVIEW_USAGE_AWS_REGION }} + REVIEW_GLM_SWARM_RAW_OUTPUTS_S3_BUCKET: ${{ vars.REVIEW_GLM_SWARM_RAW_OUTPUTS_S3_BUCKET }} + REVIEW_GLM_SWARM_RAW_OUTPUTS_S3_PREFIX: ${{ vars.REVIEW_GLM_SWARM_RAW_OUTPUTS_S3_PREFIX || 'glm-swarm' }} + run: npm run worker:job -- -- --job-file job.json diff --git a/bin/self-review-plan.cjs b/bin/self-review-plan.cjs index 6d54b2b..e4b948d 100644 --- a/bin/self-review-plan.cjs +++ b/bin/self-review-plan.cjs @@ -5,6 +5,10 @@ const fs = require("fs"); const { execFileSync } = require("child_process"); const { REVIEW_KINDS, parseReviewCommand } = require("../src/github-webhook.cjs"); +const { createReviewJobs } = require("../src/review-job.cjs"); + +const SELF_REVIEW_LANES = [{ provider: "anthropic", model: "claude-opus-4-8" }]; +const SELF_REVIEW_MAX_JOBS = 12; function planSelfReview(env = process.env, options = {}) { const eventName = String(env.EVENT_NAME || "").trim(); @@ -29,16 +33,65 @@ function planSelfReview(env = process.env, options = {}) { } else { throw new Error(`Unsupported event '${eventName}'.`); } + if (reviewKinds.length === 0) { + return { + prNumber, + eventAction, + reviewKinds: [], + jobs: [], + headSha: "", + }; + } + const repository = String(env.GITHUB_REPOSITORY || "").trim(); + if (!repository) { + throw new Error("GITHUB_REPOSITORY is required."); + } + const installationId = String(env.SELF_INSTALLATION_ID || "").trim(); + if (!installationId) { + throw new Error( + "SELF_INSTALLATION_ID is required. Set the REVIEWBOT_SELF_INSTALLATION_ID repository variable." + ); + } let headSha = String(env.PR_HEAD_SHA || "").trim(); - if (!headSha && reviewKinds.length) { - const resolveHeadSha = options.resolveHeadSha || resolveHeadShaWithGh; - headSha = resolveHeadSha(prNumber, env); + let baseSha = String(env.PR_BASE_SHA || "").trim(); + let headRefName = String(env.PR_HEAD_REF || "").trim(); + if (!headSha || !baseSha || !headRefName) { + const resolvePullRequest = options.resolvePullRequest || resolvePullRequestWithGh; + const context = resolvePullRequest(prNumber, env); + headSha = headSha || context.headSha; + baseSha = baseSha || context.baseSha; + headRefName = headRefName || context.headRefName; } - return { + if (!headSha) { + throw new Error(`Could not resolve head SHA for PR #${prNumber}.`); + } + const deliveryId = `self-review-${String(env.GITHUB_RUN_ID || "").trim() || "manual"}`; + const event = { + kind: "self_review", + trigger: eventName === "issue_comment" ? "comment" : "self_review", + shouldEnqueue: true, + deliveryId, + repository: { fullName: repository }, + headRepoFullName: repository, prNumber, headSha, + baseSha, + headRefName, + installationId, + reviewKinds, + actor: String(env.GITHUB_ACTOR || "").trim(), + }; + const jobs = createReviewJobs( + event, + {}, + { maxJobsPerDelivery: SELF_REVIEW_MAX_JOBS, lanes: SELF_REVIEW_LANES } + ); + return { + prNumber, eventAction, reviewKinds, + jobs, + headSha, }; } @@ -60,21 +113,28 @@ function parseKindsJson(value, label) { return parsed; } -function resolveHeadShaWithGh(prNumber, env = process.env) { +function resolvePullRequestWithGh(prNumber, env = process.env) { const repository = String(env.GITHUB_REPOSITORY || "").trim(); if (!repository) { - throw new Error("GITHUB_REPOSITORY is required to resolve the PR head SHA."); + throw new Error("GITHUB_REPOSITORY is required to resolve the PR context."); } const output = execFileSync( "gh", - ["api", `repos/${repository}/pulls/${prNumber}`, "--jq", ".head.sha"], + [ + "api", + `repos/${repository}/pulls/${prNumber}`, + "--jq", + "[.head.sha, .base.sha, .head.ref] | join(\"\\n\")", + ], { encoding: "utf8" } ); - const headSha = String(output || "").trim(); + const [headSha = "", baseSha = "", headRefName = ""] = String(output || "") + .split("\n") + .map((line) => line.trim()); if (!headSha) { throw new Error(`Could not resolve head SHA for PR #${prNumber}.`); } - return headSha; + return { headSha, baseSha, headRefName }; } function main() { @@ -85,17 +145,17 @@ function main() { } const lines = [ `pr_number=${plan.prNumber}`, - `head_sha=${plan.headSha}`, `event_action=${plan.eventAction}`, `review_kinds_json=${JSON.stringify(plan.reviewKinds)}`, + `jobs_json=${JSON.stringify(plan.jobs)}`, ]; fs.appendFileSync(outputPath, `${lines.join("\n")}\n`); - if (plan.reviewKinds.length === 0) { + if (plan.jobs.length === 0) { console.log(`No review kinds requested for PR #${plan.prNumber}; skipping review.`); return; } console.log( - `Planned self review for PR #${plan.prNumber} @ ${plan.headSha}: ${plan.reviewKinds.join(", ")}` + `Planned ${plan.jobs.length} self review job(s) for PR #${plan.prNumber} @ ${plan.headSha}: ${plan.reviewKinds.join(", ")}` ); } diff --git a/docs/review-workflows.md b/docs/review-workflows.md index 48360e8..4098a97 100644 --- a/docs/review-workflows.md +++ b/docs/review-workflows.md @@ -15,9 +15,12 @@ Maintainer comment triggers are documented in This repository reviews its own pull requests through GitHub Actions instead of the central App webhook. `.github/workflows/self-review.yml` triggers on -pull request events, maintainer `/6529bot` comments, and manual dispatch, then -calls the reusable `.github/workflows/review.yml` workflow with review kinds -planned by `bin/self-review-plan.cjs`: +pull request events, maintainer `/6529bot` comments, and manual dispatch. +`bin/self-review-plan.cjs` plans review kinds and builds worker job payloads +with the same `createReviewJobs` pipeline the App uses; each job then runs +`npm run worker:job` with a GitHub App installation token (from the +`REVIEWBOT_SELF_INSTALLATION_ID` repository variable), so comments post as +the bot exactly like App-dispatched reviews: - Initial reviews (opened, reopened, ready for review): `general`, `security`, `auth-api`, `deploy-actions`, `db-lambda`, and `glm-swarm` — diff --git a/scripts/check-review-workflow-kinds.cjs b/scripts/check-review-workflow-kinds.cjs index 0e884a1..ca08952 100644 --- a/scripts/check-review-workflow-kinds.cjs +++ b/scripts/check-review-workflow-kinds.cjs @@ -169,8 +169,9 @@ function checkSelfReviewWorkflow(workflowText, reviewKinds, findings) { } const normalized = normalizeWhitespace(workflowText); for (const snippet of [ - "uses: ./.github/workflows/review.yml", "node bin/self-review-plan.cjs", + "node bin/github-app-installation-token.cjs", + "npm run worker:job -- -- --job-file job.json", "author_association", "ANTHROPIC_API_KEY: ${{ secrets.ANTHROPIC_API_KEY }}", ]) { diff --git a/scripts/smoke-test.cjs b/scripts/smoke-test.cjs index 38de2b0..7d9da64 100644 --- a/scripts/smoke-test.cjs +++ b/scripts/smoke-test.cjs @@ -4651,72 +4651,88 @@ assert.throws( /review workflow kind check found/ ); const selfReviewPlanEnv = { - INITIAL_KINDS_JSON: '["general","security","auth-api"]', + INITIAL_KINDS_JSON: '["general","security","glm-swarm"]', SYNCHRONIZE_KINDS_JSON: '["followup"]', + GITHUB_REPOSITORY: "6529-Collections/6529reviewbot", + GITHUB_RUN_ID: "424242", + GITHUB_ACTOR: "punk6529", + SELF_INSTALLATION_ID: "140321060", + PR_NUMBER: "415", + PR_HEAD_SHA: "self-review-head", + PR_BASE_SHA: "self-review-base", + PR_HEAD_REF: "self-review-branch", }; +const selfReviewInitialPlan = selfReviewPlanCli.planSelfReview({ + ...selfReviewPlanEnv, + EVENT_NAME: "pull_request", + EVENT_ACTION: "opened", +}); +assert.deepEqual(selfReviewInitialPlan.reviewKinds, ["general", "security", "glm-swarm"]); +assert.equal(selfReviewInitialPlan.jobs.length, 3); assert.deepEqual( - selfReviewPlanCli.planSelfReview({ - ...selfReviewPlanEnv, - EVENT_NAME: "pull_request", - EVENT_ACTION: "opened", - PR_NUMBER: "415", - PR_HEAD_SHA: "self-review-head", - }), - { - prNumber: "415", - headSha: "self-review-head", - eventAction: "opened", - reviewKinds: ["general", "security", "auth-api"], - } + selfReviewInitialPlan.jobs.map((job) => `${job.reviewKind}:${job.lane}`), + [ + "general:anthropic:claude-opus-4-8", + "security:anthropic:claude-opus-4-8", + "glm-swarm:openrouter:z-ai-glm-5.2", + ] ); +for (const job of selfReviewInitialPlan.jobs) { + assert.equal(job.installationId, "140321060"); + assert.equal(job.repository.fullName, "6529-Collections/6529reviewbot"); + assert.equal(job.headRepoFullName, "6529-Collections/6529reviewbot"); + assert.equal(job.prNumber, "415"); + assert.equal(job.headSha, "self-review-head"); + assert.equal(job.baseSha, "self-review-base"); + assert.equal(job.headRefName, "self-review-branch"); + assert.match(job.id, /^rj_[0-9a-f]{24}$/); + assert.match(job.runKey, /^rk_[0-9a-f]{24}$/); +} assert.deepEqual( selfReviewPlanCli.planSelfReview({ ...selfReviewPlanEnv, EVENT_NAME: "pull_request", EVENT_ACTION: "synchronize", - PR_NUMBER: "415", - PR_HEAD_SHA: "self-review-head", }).reviewKinds, ["followup"] ); -assert.deepEqual( - selfReviewPlanCli.planSelfReview( - { - ...selfReviewPlanEnv, - EVENT_NAME: "issue_comment", - EVENT_ACTION: "created", - PR_NUMBER: "415", - COMMENT_BODY: "/6529bot review security db-lambda", - }, - { resolveHeadSha: () => "resolved-head" } - ), +const selfReviewCommentPlan = selfReviewPlanCli.planSelfReview( + { + ...selfReviewPlanEnv, + EVENT_NAME: "issue_comment", + EVENT_ACTION: "created", + PR_HEAD_SHA: "", + PR_BASE_SHA: "", + PR_HEAD_REF: "", + COMMENT_BODY: "/6529bot review security db-lambda", + }, { - prNumber: "415", - headSha: "resolved-head", - eventAction: "created", - reviewKinds: ["security", "db-lambda"], + resolvePullRequest: () => ({ + headSha: "resolved-head", + baseSha: "resolved-base", + headRefName: "resolved-branch", + }), } ); +assert.deepEqual(selfReviewCommentPlan.reviewKinds, ["security", "db-lambda"]); +assert.equal(selfReviewCommentPlan.headSha, "resolved-head"); +assert.equal(selfReviewCommentPlan.jobs[0].baseSha, "resolved-base"); +assert.equal(selfReviewCommentPlan.jobs[0].headRefName, "resolved-branch"); assert.deepEqual( selfReviewPlanCli.planSelfReview({ ...selfReviewPlanEnv, EVENT_NAME: "issue_comment", EVENT_ACTION: "created", - PR_NUMBER: "415", COMMENT_BODY: "looks good to me", - }).reviewKinds, - [] + }), + { prNumber: "415", eventAction: "created", reviewKinds: [], jobs: [], headSha: "" } ); assert.deepEqual( - selfReviewPlanCli.planSelfReview( - { - ...selfReviewPlanEnv, - EVENT_NAME: "workflow_dispatch", - PR_NUMBER: "415", - REQUESTED_KINDS_JSON: '["deploy-actions"]', - }, - { resolveHeadSha: () => "resolved-head" } - ).reviewKinds, + selfReviewPlanCli.planSelfReview({ + ...selfReviewPlanEnv, + EVENT_NAME: "workflow_dispatch", + REQUESTED_KINDS_JSON: '["deploy-actions"]', + }).reviewKinds, ["deploy-actions"] ); assert.throws( @@ -4724,7 +4740,6 @@ assert.throws( selfReviewPlanCli.planSelfReview({ ...selfReviewPlanEnv, EVENT_NAME: "workflow_dispatch", - PR_NUMBER: "415", REQUESTED_KINDS_JSON: '["not-a-kind"]', }), /unknown review kind 'not-a-kind'/ @@ -4733,11 +4748,22 @@ assert.throws( () => selfReviewPlanCli.planSelfReview({ ...selfReviewPlanEnv, + PR_NUMBER: "", EVENT_NAME: "pull_request", EVENT_ACTION: "opened", }), /PR_NUMBER is required/ ); +assert.throws( + () => + selfReviewPlanCli.planSelfReview({ + ...selfReviewPlanEnv, + SELF_INSTALLATION_ID: "", + EVENT_NAME: "pull_request", + EVENT_ACTION: "opened", + }), + /SELF_INSTALLATION_ID is required/ +); assert.equal(replayWebhook.inferEventName(JSON.parse(webhookBody.toString("utf8"))), "pull_request"); assert.equal(replayWebhook.parsePayload(Buffer.from(`\uFEFF${webhookBody}`)).action, "opened"); assert.equal(replayWebhook.normalizePayloadBody(Buffer.from(`\uFEFF${webhookBody}`))[0], 123); From 71aa76af76dd83c2705490f4639f025ee42be32e Mon Sep 17 00:00:00 2001 From: Punk 6529 <108035228+punk6529@users.noreply.github.com> Date: Sat, 4 Jul 2026 12:01:07 +0000 Subject: [PATCH 3/4] Address followup review findings on self-review plan Pass the pull request action through as the job trigger, resolve PR context as a JSON object instead of positional newline splitting, gate usage-ledger accounting on the AWS role variable so partially configured repos degrade cleanly, and cover the /6529bot help empty-kinds path. Co-Authored-By: Claude Fable 5 --- .github/workflows/self-review.yml | 2 +- bin/self-review-plan.cjs | 20 ++++++++++++++------ scripts/smoke-test.cjs | 9 +++++++++ 3 files changed, 24 insertions(+), 7 deletions(-) diff --git a/.github/workflows/self-review.yml b/.github/workflows/self-review.yml index 75402cf..9e3eb02 100644 --- a/.github/workflows/self-review.yml +++ b/.github/workflows/self-review.yml @@ -134,7 +134,7 @@ jobs: PR_BASE_SHA: ${{ matrix.job.baseSha }} PR_HEAD_REF: ${{ matrix.job.headRefName }} PR_HEAD_REPO: ${{ matrix.job.headRepoFullName }} - REVIEW_USAGE_ENABLED: true + REVIEW_USAGE_ENABLED: ${{ vars.REVIEW_USAGE_AWS_ROLE_ARN != '' }} REVIEW_USAGE_AWS_REGION: ${{ vars.REVIEW_USAGE_AWS_REGION }} REVIEW_USAGE_DB_RESOURCE_ARN: ${{ vars.REVIEW_USAGE_DB_RESOURCE_ARN }} REVIEW_USAGE_DB_SECRET_ARN: ${{ vars.REVIEW_USAGE_DB_SECRET_ARN }} diff --git a/bin/self-review-plan.cjs b/bin/self-review-plan.cjs index e4b948d..3bc2877 100644 --- a/bin/self-review-plan.cjs +++ b/bin/self-review-plan.cjs @@ -68,7 +68,7 @@ function planSelfReview(env = process.env, options = {}) { const deliveryId = `self-review-${String(env.GITHUB_RUN_ID || "").trim() || "manual"}`; const event = { kind: "self_review", - trigger: eventName === "issue_comment" ? "comment" : "self_review", + trigger: eventName === "issue_comment" ? "comment" : eventAction, shouldEnqueue: true, deliveryId, repository: { fullName: repository }, @@ -124,17 +124,25 @@ function resolvePullRequestWithGh(prNumber, env = process.env) { "api", `repos/${repository}/pulls/${prNumber}`, "--jq", - "[.head.sha, .base.sha, .head.ref] | join(\"\\n\")", + "{headSha: .head.sha, baseSha: .base.sha, headRefName: .head.ref}", ], { encoding: "utf8" } ); - const [headSha = "", baseSha = "", headRefName = ""] = String(output || "") - .split("\n") - .map((line) => line.trim()); + let context; + try { + context = JSON.parse(String(output || "")); + } catch (error) { + throw new Error(`Could not parse PR #${prNumber} context: ${error.message}`); + } + const headSha = String(context.headSha || "").trim(); if (!headSha) { throw new Error(`Could not resolve head SHA for PR #${prNumber}.`); } - return { headSha, baseSha, headRefName }; + return { + headSha, + baseSha: String(context.baseSha || "").trim(), + headRefName: String(context.headRefName || "").trim(), + }; } function main() { diff --git a/scripts/smoke-test.cjs b/scripts/smoke-test.cjs index 7d9da64..1c8ad03 100644 --- a/scripts/smoke-test.cjs +++ b/scripts/smoke-test.cjs @@ -4727,6 +4727,15 @@ assert.deepEqual( }), { prNumber: "415", eventAction: "created", reviewKinds: [], jobs: [], headSha: "" } ); +assert.deepEqual( + selfReviewPlanCli.planSelfReview({ + ...selfReviewPlanEnv, + EVENT_NAME: "issue_comment", + EVENT_ACTION: "created", + COMMENT_BODY: "/6529bot help", + }), + { prNumber: "415", eventAction: "created", reviewKinds: [], jobs: [], headSha: "" } +); assert.deepEqual( selfReviewPlanCli.planSelfReview({ ...selfReviewPlanEnv, From 0c1fc9f59fd4d33f653eeded11360970e8b8e25c Mon Sep 17 00:00:00 2001 From: Punk 6529 <108035228+punk6529@users.noreply.github.com> Date: Sat, 4 Jul 2026 12:11:22 +0000 Subject: [PATCH 4/4] Scope self-review permissions and reject fork PR heads Move id-token: write off the workflow root onto the review job, give the plan job explicit pull-requests: read, resolve and validate the PR head repository so /6529bot comments on fork PRs fail closed before checkout, require a resolved base SHA and head ref, and expose the trigger in plan results with smoke coverage for the fork, partial-context, and trigger semantics raised by the self reviews. Co-Authored-By: Claude Fable 5 --- .github/workflows/self-review.yml | 8 +- bin/self-review-plan.cjs | 23 ++++- scripts/check-review-workflow-kinds.cjs | 8 +- scripts/smoke-test.cjs | 109 ++++++++++++++++++++---- 4 files changed, 125 insertions(+), 23 deletions(-) diff --git a/.github/workflows/self-review.yml b/.github/workflows/self-review.yml index 9e3eb02..cf1f7fc 100644 --- a/.github/workflows/self-review.yml +++ b/.github/workflows/self-review.yml @@ -20,12 +20,14 @@ on: permissions: contents: read - id-token: write jobs: plan: name: Plan review kinds runs-on: ubuntu-latest + permissions: + contents: read + pull-requests: read if: >- github.event_name == 'workflow_dispatch' || (github.event_name == 'pull_request' && @@ -58,6 +60,7 @@ jobs: PR_HEAD_SHA: ${{ github.event.pull_request.head.sha || '' }} PR_BASE_SHA: ${{ github.event.pull_request.base.sha || '' }} PR_HEAD_REF: ${{ github.event.pull_request.head.ref || '' }} + PR_HEAD_REPO: ${{ github.event.pull_request.head.repo.full_name || '' }} COMMENT_BODY: ${{ github.event.comment.body || '' }} REQUESTED_KINDS_JSON: ${{ inputs.review_kinds_json || '' }} INITIAL_KINDS_JSON: '["general","security","auth-api","deploy-actions","db-lambda","glm-swarm"]' @@ -70,6 +73,9 @@ jobs: needs: plan if: ${{ needs.plan.outputs.jobs_json != '' && needs.plan.outputs.jobs_json != '[]' }} runs-on: ubuntu-latest + permissions: + contents: read + id-token: write timeout-minutes: ${{ fromJSON(vars.REVIEWBOT_WORKER_TIMEOUT_MINUTES || '20') }} strategy: fail-fast: false diff --git a/bin/self-review-plan.cjs b/bin/self-review-plan.cjs index 3bc2877..5c2ad76 100644 --- a/bin/self-review-plan.cjs +++ b/bin/self-review-plan.cjs @@ -33,10 +33,12 @@ function planSelfReview(env = process.env, options = {}) { } else { throw new Error(`Unsupported event '${eventName}'.`); } + const trigger = eventName === "issue_comment" ? "comment" : eventAction; if (reviewKinds.length === 0) { return { prNumber, eventAction, + trigger, reviewKinds: [], jobs: [], headSha: "", @@ -55,20 +57,33 @@ function planSelfReview(env = process.env, options = {}) { let headSha = String(env.PR_HEAD_SHA || "").trim(); let baseSha = String(env.PR_BASE_SHA || "").trim(); let headRefName = String(env.PR_HEAD_REF || "").trim(); - if (!headSha || !baseSha || !headRefName) { + let headRepoFullName = String(env.PR_HEAD_REPO || "").trim(); + if (!headSha || !baseSha || !headRefName || !headRepoFullName) { const resolvePullRequest = options.resolvePullRequest || resolvePullRequestWithGh; const context = resolvePullRequest(prNumber, env); headSha = headSha || context.headSha; baseSha = baseSha || context.baseSha; headRefName = headRefName || context.headRefName; + headRepoFullName = headRepoFullName || String(context.headRepoFullName || "").trim(); } if (!headSha) { throw new Error(`Could not resolve head SHA for PR #${prNumber}.`); } + if (!baseSha) { + throw new Error(`Could not resolve base SHA for PR #${prNumber}.`); + } + if (!headRefName) { + throw new Error(`Could not resolve head ref for PR #${prNumber}.`); + } + if (headRepoFullName && headRepoFullName !== repository) { + throw new Error( + `Self review only supports branches in ${repository}; PR #${prNumber} head is ${headRepoFullName}.` + ); + } const deliveryId = `self-review-${String(env.GITHUB_RUN_ID || "").trim() || "manual"}`; const event = { kind: "self_review", - trigger: eventName === "issue_comment" ? "comment" : eventAction, + trigger, shouldEnqueue: true, deliveryId, repository: { fullName: repository }, @@ -89,6 +104,7 @@ function planSelfReview(env = process.env, options = {}) { return { prNumber, eventAction, + trigger, reviewKinds, jobs, headSha, @@ -124,7 +140,7 @@ function resolvePullRequestWithGh(prNumber, env = process.env) { "api", `repos/${repository}/pulls/${prNumber}`, "--jq", - "{headSha: .head.sha, baseSha: .base.sha, headRefName: .head.ref}", + "{headSha: .head.sha, baseSha: .base.sha, headRefName: .head.ref, headRepoFullName: .head.repo.full_name}", ], { encoding: "utf8" } ); @@ -142,6 +158,7 @@ function resolvePullRequestWithGh(prNumber, env = process.env) { headSha, baseSha: String(context.baseSha || "").trim(), headRefName: String(context.headRefName || "").trim(), + headRepoFullName: String(context.headRepoFullName || "").trim(), }; } diff --git a/scripts/check-review-workflow-kinds.cjs b/scripts/check-review-workflow-kinds.cjs index ca08952..40aa390 100644 --- a/scripts/check-review-workflow-kinds.cjs +++ b/scripts/check-review-workflow-kinds.cjs @@ -171,7 +171,8 @@ function checkSelfReviewWorkflow(workflowText, reviewKinds, findings) { for (const snippet of [ "node bin/self-review-plan.cjs", "node bin/github-app-installation-token.cjs", - "npm run worker:job -- -- --job-file job.json", + "npm run worker:job", + "--job-file job.json", "author_association", "ANTHROPIC_API_KEY: ${{ secrets.ANTHROPIC_API_KEY }}", ]) { @@ -179,6 +180,11 @@ function checkSelfReviewWorkflow(workflowText, reviewKinds, findings) { findings.push(`${selfReviewWorkflowPath} must include '${snippet}'.`); } } + if (normalized.includes(normalizeWhitespace("uses: ./.github/workflows/review.yml"))) { + findings.push( + `${selfReviewWorkflowPath} must post through the worker with installation tokens, not the reusable review workflow.` + ); + } for (const [label, expectNonEmpty] of [ ["INITIAL_KINDS_JSON", true], ["SYNCHRONIZE_KINDS_JSON", true], diff --git a/scripts/smoke-test.cjs b/scripts/smoke-test.cjs index 1c8ad03..48b512d 100644 --- a/scripts/smoke-test.cjs +++ b/scripts/smoke-test.cjs @@ -4661,12 +4661,14 @@ const selfReviewPlanEnv = { PR_HEAD_SHA: "self-review-head", PR_BASE_SHA: "self-review-base", PR_HEAD_REF: "self-review-branch", + PR_HEAD_REPO: "6529-Collections/6529reviewbot", }; const selfReviewInitialPlan = selfReviewPlanCli.planSelfReview({ ...selfReviewPlanEnv, EVENT_NAME: "pull_request", EVENT_ACTION: "opened", }); +assert.equal(selfReviewInitialPlan.trigger, "opened"); assert.deepEqual(selfReviewInitialPlan.reviewKinds, ["general", "security", "glm-swarm"]); assert.equal(selfReviewInitialPlan.jobs.length, 3); assert.deepEqual( @@ -4688,14 +4690,13 @@ for (const job of selfReviewInitialPlan.jobs) { assert.match(job.id, /^rj_[0-9a-f]{24}$/); assert.match(job.runKey, /^rk_[0-9a-f]{24}$/); } -assert.deepEqual( - selfReviewPlanCli.planSelfReview({ - ...selfReviewPlanEnv, - EVENT_NAME: "pull_request", - EVENT_ACTION: "synchronize", - }).reviewKinds, - ["followup"] -); +const selfReviewSynchronizePlan = selfReviewPlanCli.planSelfReview({ + ...selfReviewPlanEnv, + EVENT_NAME: "pull_request", + EVENT_ACTION: "synchronize", +}); +assert.deepEqual(selfReviewSynchronizePlan.reviewKinds, ["followup"]); +assert.equal(selfReviewSynchronizePlan.trigger, "synchronize"); const selfReviewCommentPlan = selfReviewPlanCli.planSelfReview( { ...selfReviewPlanEnv, @@ -4704,6 +4705,7 @@ const selfReviewCommentPlan = selfReviewPlanCli.planSelfReview( PR_HEAD_SHA: "", PR_BASE_SHA: "", PR_HEAD_REF: "", + PR_HEAD_REPO: "", COMMENT_BODY: "/6529bot review security db-lambda", }, { @@ -4711,13 +4713,85 @@ const selfReviewCommentPlan = selfReviewPlanCli.planSelfReview( headSha: "resolved-head", baseSha: "resolved-base", headRefName: "resolved-branch", + headRepoFullName: "6529-Collections/6529reviewbot", }), } ); assert.deepEqual(selfReviewCommentPlan.reviewKinds, ["security", "db-lambda"]); +assert.equal(selfReviewCommentPlan.trigger, "comment"); assert.equal(selfReviewCommentPlan.headSha, "resolved-head"); assert.equal(selfReviewCommentPlan.jobs[0].baseSha, "resolved-base"); assert.equal(selfReviewCommentPlan.jobs[0].headRefName, "resolved-branch"); +assert.equal(selfReviewCommentPlan.jobs[0].headRepoFullName, "6529-Collections/6529reviewbot"); +const selfReviewPartialPlan = selfReviewPlanCli.planSelfReview( + { + ...selfReviewPlanEnv, + EVENT_NAME: "issue_comment", + EVENT_ACTION: "created", + PR_BASE_SHA: "", + PR_HEAD_REF: "", + PR_HEAD_REPO: "", + COMMENT_BODY: "/6529bot review security", + }, + { + resolvePullRequest: () => ({ + headSha: "resolved-head", + baseSha: "resolved-base", + headRefName: "resolved-branch", + headRepoFullName: "6529-Collections/6529reviewbot", + }), + } +); +assert.equal(selfReviewPartialPlan.headSha, "self-review-head"); +assert.equal(selfReviewPartialPlan.jobs[0].baseSha, "resolved-base"); +assert.throws( + () => + selfReviewPlanCli.planSelfReview( + { + ...selfReviewPlanEnv, + EVENT_NAME: "issue_comment", + EVENT_ACTION: "created", + PR_HEAD_SHA: "", + PR_BASE_SHA: "", + PR_HEAD_REF: "", + PR_HEAD_REPO: "", + COMMENT_BODY: "/6529bot review security", + }, + { + resolvePullRequest: () => ({ + headSha: "fork-head", + baseSha: "fork-base", + headRefName: "fork-branch", + headRepoFullName: "attacker/6529reviewbot", + }), + } + ), + /Self review only supports branches in 6529-Collections\/6529reviewbot/ +); +assert.throws( + () => + selfReviewPlanCli.planSelfReview( + { + ...selfReviewPlanEnv, + EVENT_NAME: "issue_comment", + EVENT_ACTION: "created", + PR_HEAD_SHA: "", + PR_BASE_SHA: "", + PR_HEAD_REF: "", + PR_HEAD_REPO: "", + COMMENT_BODY: "/6529bot review security", + }, + { + resolvePullRequest: () => ({ + headSha: "resolved-head", + baseSha: "", + headRefName: "resolved-branch", + headRepoFullName: "6529-Collections/6529reviewbot", + }), + } + ), + /Could not resolve base SHA/ +); assert.deepEqual( selfReviewPlanCli.planSelfReview({ ...selfReviewPlanEnv, @@ -4725,7 +4799,7 @@ assert.deepEqual( EVENT_ACTION: "created", COMMENT_BODY: "looks good to me", }), - { prNumber: "415", eventAction: "created", reviewKinds: [], jobs: [], headSha: "" } + { prNumber: "415", eventAction: "created", trigger: "comment", reviewKinds: [], jobs: [], headSha: "" } ); assert.deepEqual( selfReviewPlanCli.planSelfReview({ @@ -4734,16 +4808,15 @@ assert.deepEqual( EVENT_ACTION: "created", COMMENT_BODY: "/6529bot help", }), - { prNumber: "415", eventAction: "created", reviewKinds: [], jobs: [], headSha: "" } -); -assert.deepEqual( - selfReviewPlanCli.planSelfReview({ - ...selfReviewPlanEnv, - EVENT_NAME: "workflow_dispatch", - REQUESTED_KINDS_JSON: '["deploy-actions"]', - }).reviewKinds, - ["deploy-actions"] + { prNumber: "415", eventAction: "created", trigger: "comment", reviewKinds: [], jobs: [], headSha: "" } ); +const selfReviewDispatchPlan = selfReviewPlanCli.planSelfReview({ + ...selfReviewPlanEnv, + EVENT_NAME: "workflow_dispatch", + REQUESTED_KINDS_JSON: '["deploy-actions"]', +}); +assert.deepEqual(selfReviewDispatchPlan.reviewKinds, ["deploy-actions"]); +assert.equal(selfReviewDispatchPlan.trigger, "manual"); assert.throws( () => selfReviewPlanCli.planSelfReview({