From bd809cdae5f812f47aaf0ff897c482721c9450a5 Mon Sep 17 00:00:00 2001 From: "touchless-bot[bot]" <271340427+touchless-bot[bot]@users.noreply.github.com> Date: Fri, 16 Oct 2026 10:45:02 -0400 Subject: [PATCH] sync from touchless-bot infra/bot-ci@8ae6a0d9ddb1 --- .github/workflows/ci.yml | 21 ++++++++++-- .github/workflows/pr-lint.yml | 2 +- .synced-from | 2 +- README.md | 8 ++--- actions/ci-plan/action.yml | 45 +++++++++++++++++++++++++ actions/ci/planChecks.ts | 23 ++++++++++--- actions/pr-lint/lintPullRequest.ts | 7 ++-- actions/second-look/action.yml | 2 +- actions/second-look/approveHintOf.ts | 11 ++++++ actions/second-look/changedLinesOf.ts | 14 ++++++++ actions/second-look/checkRun.ts | 5 ++- actions/second-look/fetchPull.ts | 6 +++- actions/second-look/notifyUnfinished.ts | 26 +++++++++----- actions/second-look/postReview.ts | 10 +++--- actions/second-look/reviewBodyOf.ts | 9 +++-- actions/second-look/synthesize.ts | 13 +++++-- actions/second-look/types.ts | 2 ++ 17 files changed, 168 insertions(+), 38 deletions(-) create mode 100644 actions/ci-plan/action.yml create mode 100644 actions/second-look/approveHintOf.ts create mode 100644 actions/second-look/changedLinesOf.ts diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index dd3571a..5deaa47 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -1,5 +1,7 @@ -# required on every repo's default-branch PRs by the org ruleset "bot-ci". -# rulesets ignore trigger filters: this always runs on opened / synchronize / reopened. +# Build is for the bot's repos: ones made from app-template, without their own CI. required on default-branch +# PRs by the org ruleset "bot-ci-build", which leaves out every repo setup.sh's scan found isn't one. rulesets +# ignore trigger filters: this always runs on opened / synchronize / reopened. the plan job decides on a light +# runner, so a human repo created since that scan never starts the build runner (a skipped job still passes). # staging previews only, exo-style (wrangler env.feature, -.touchlessapis.com). # the checks job runs the PR's code; its one secret, PACKAGES_READ_TOKEN (read:packages for # @touchlesscode/*), only reaches the scripts-off install after npmrcGuard. it bundles the previews. @@ -22,8 +24,23 @@ concurrency: cancel-in-progress: true jobs: + plan: + name: Plan + runs-on: ${{ vars.BOT_CI_LIGHT_RUNNER || vars.ORG_CI_LIGHT_RUNNER || 'ubicloud-standard-2' }} + timeout-minutes: 5 + outputs: + build: ${{ steps.plan.outputs.build }} + steps: + - id: plan + uses: touchlesscode/bot-ci/actions/ci-plan@main + with: + github-token: ${{ github.token }} + template: touchlesscode/app-template + checks: name: Build + needs: plan + if: needs.plan.outputs.build == 'true' runs-on: ${{ vars.BOT_CI_RUNNER || vars.ORG_CI_RUNNER || 'ubicloud-standard-8' }} timeout-minutes: 30 outputs: diff --git a/.github/workflows/pr-lint.yml b/.github/workflows/pr-lint.yml index f76267b..5b47d9a 100644 --- a/.github/workflows/pr-lint.yml +++ b/.github/workflows/pr-lint.yml @@ -1,4 +1,4 @@ -# required on every repo's default-branch PRs by the org ruleset "bot-ci". +# required on every repo's default-branch PRs by the org ruleset "bot-ci" (Build has its own, "bot-ci-build"). # rulesets ignore `types`, so an edited title or body doesn't re-run this; a push or reopen does. name: PR Lint diff --git a/.synced-from b/.synced-from index 8c4824a..5f6cdce 100644 --- a/.synced-from +++ b/.synced-from @@ -1 +1 @@ -828a939b6052af3a708691fbe161d2df1bf9ef50 +8ae6a0d9ddb1e2ff0f05f766007129720638fc66 diff --git a/README.md b/README.md index 11b4496..4b2fa22 100644 --- a/README.md +++ b/README.md @@ -1,6 +1,6 @@ # bot-ci -The CI and guardrails for bot-built code in touchlesscode. Two org rulesets make it apply to every repo's default branch without anyone adding files to the repo, and everything runs on Ubicloud runners. +The CI and guardrails for bot-built code in touchlesscode. Org rulesets make it apply to every repo's default branch without anyone adding files to the repo, and everything runs on Ubicloud runners. | check | runs on | what it does | |-------|---------|--------------| @@ -11,7 +11,7 @@ The CI and guardrails for bot-built code in touchlesscode. Two org rulesets make A second ruleset, `review-required`, needs one human approval given after the last push, and blocks force-pushes and deleting the default branch. The Touchless Bot App has no bypass, so it can open PRs but never merge one alone. -In practice Build and Previews only do work in repos with no CI of their own, which today means the bot's `touchlesscode/apps` monorepo and repos made from `app-template`. exo and anything else with a `pull_request` workflow skip them. +In practice Build and Previews only run in repos with no CI of their own, which today means the bot's `touchlesscode/apps` monorepo and repos made from `app-template`. exo and anything else with a `pull_request` workflow are left out of the `bot-ci-build` ruleset, so they never see them. ## PR Lint @@ -34,7 +34,7 @@ Dependabot, merge-queue and revert PRs are skipped. **Editing the title or body ## Build -First it decides whether to run at all (`actions/ci/planChecks.ts`). It skips repos that already run a GitHub workflow on `pull_request` or use another CI system (Buildkite, CircleCI, GitLab, and so on), and repos with no root `package.json`. The job summary says which. +First a light **Plan** job decides whether to run at all (`actions/ci/planChecks.ts`, from a checkout of just the CI config and `package.json`). It skips repos that already run a GitHub workflow on `pull_request` or use another CI system (Buildkite, CircleCI, GitLab, and so on), and repos with no root `package.json`; then the Build job doesn't start, which still passes. The job summary says why. Repos known to skip are left out of the ruleset altogether, so they don't even get Plan. - **NX workspaces** get exo's pipeline: `nrwl/nx-set-shas` finds the base, then `nx affected -t build`, `lint`, `typecheck` and `test`, with the local NX cache and the pnpm store cached. - **Other Node repos** install with a frozen lockfile, then run whichever of `lint`, a typecheck script (`typecheck`, `type-check`, `check-types` or `tsc`), `test` and `build` they declare. @@ -88,7 +88,7 @@ The guard doesn't check bindings: a feature env that binds a prod KV namespace, ## Opting a repo out -Add your own `pull_request` workflow and Build and Previews skip the repo. To leave a repo out of the rulesets entirely, an org owner adds it to the excludes (`setup.sh --exclude `). +Add your own `pull_request` workflow and Build and Previews skip the repo (and leave its required checks at the next `setup.sh` run). To leave a repo out of the rulesets entirely, an org owner adds it to the excludes (`setup.sh --exclude `). ## Settings diff --git a/actions/ci-plan/action.yml b/actions/ci-plan/action.yml new file mode 100644 index 0000000..02acf1b --- /dev/null +++ b/actions/ci-plan/action.yml @@ -0,0 +1,45 @@ +name: bot-ci plan +description: Whether Build has anything to do in this repo (actions/ci/planChecks.ts) — only repos made from app-template (the bot's repos), without their own CI, with a package.json — from the repo's template link and a sparse checkout of just its CI config and package.json, so any other repo costs a light runner for a few seconds and never starts Build. Keep the sparse list in step with planChecks' otherCiFiles (a test checks) +inputs: + github-token: + required: true + template: + description: "owner/name of the app template; repos made from it get Build" + required: true +outputs: + build: + description: "'true' when Build should run" + value: ${{ steps.plan.outputs.skip != 'true' }} +runs: + using: composite + steps: + - id: template + shell: bash + env: + GH_TOKEN: ${{ inputs.github-token }} + run: echo "of=$(gh api "repos/$GITHUB_REPOSITORY" --jq '.template_repository.full_name // ""')" >> "$GITHUB_OUTPUT" + - if: steps.template.outputs.of == inputs.template + uses: actions/checkout@v7 + with: + persist-credentials: false + sparse-checkout-cone-mode: false + sparse-checkout: | + /.github/workflows/ + /package.json + /.buildkite/ + /.circleci/ + /codefresh.yml + /.gitlab-ci.yml + /azure-pipelines.yml + /bitbucket-pipelines.yml + /.travis.yml + /Jenkinsfile + - uses: actions/setup-node@v6 + with: + node-version: 24 + - id: plan + shell: bash + env: + BOT_CI_TEMPLATE: ${{ inputs.template }} + BOT_CI_TEMPLATE_OF: ${{ steps.template.outputs.of }} + run: node "$GITHUB_ACTION_PATH/../ci/planChecks.ts" diff --git a/actions/ci/planChecks.ts b/actions/ci/planChecks.ts index c846d7d..f770838 100755 --- a/actions/ci/planChecks.ts +++ b/actions/ci/planChecks.ts @@ -1,7 +1,8 @@ #!/usr/bin/env node /** * org ci planner: decides what the org-wide required ci runs in a checked-out repo. - * nothing when the repo already has its own pull request ci or isn't a node project. + * nothing when the repo wasn't made from app-template (it's for the bot's repos), already has its own + * pull request ci, or isn't a node project. * nx workspaces get exo's pipeline: `nx affected -t build|lint|typecheck|test` against * nx-set-shas' base, with the local nx cache. other node repos get install plus the * lint / typecheck / test / build scripts they declare. @@ -43,7 +44,8 @@ export type RepoSnapshot = { readText: (path: string) => string | undefined } -const otherCiFiles = [".buildkite", ".circleci", "codefresh.yml", ".gitlab-ci.yml", "azure-pipelines.yml", "bitbucket-pipelines.yml", ".travis.yml", "Jenkinsfile"] +/** other ci systems' config at the repo root (actions/ci-plan's sparse checkout lists the same paths) */ +export const otherCiFiles = [".buildkite", ".circleci", "codefresh.yml", ".gitlab-ci.yml", "azure-pipelines.yml", "bitbucket-pipelines.yml", ".travis.yml", "Jenkinsfile"] const typecheckScripts = ["typecheck", "type-check", "check-types", "tsc"] const nxTargets = ["build", "lint", "typecheck", "test"] const placeholderTest = /no test specified/ @@ -113,8 +115,13 @@ export const nodeVersionOf = (repo: RepoSnapshot, packageJson: PackageJson): { n return { nodeVersion: "24" } } -/** the whole plan for one repo */ -export const planChecks = (repo: RepoSnapshot): CheckPlan => { +/** + * the whole plan for one repo. `botBuilt` says whether the repo was made from app-template (the bot's repos and + * `apps`); Build is only for those, so anything else is skipped. undefined means it wasn't checked (the Build job, + * which only starts after the plan job said yes) + */ +export const planChecks = (repo: RepoSnapshot, { botBuilt }: { botBuilt?: boolean } = {}): CheckPlan => { + if (botBuilt === false) return { skip: true, reason: "not made from app-template; Build is only for the bot's repos" } const ownCi = ownCiOf(repo) if (ownCi) return { skip: true, reason: `the repo runs its own CI (${ownCi})` } const text = repo.readText("package.json") @@ -173,8 +180,14 @@ export const outputLinesOf = (plan: CheckPlan) => [ ]), ] +/** BOT_CI_TEMPLATE_OF (the repo's template, blank for none) against BOT_CI_TEMPLATE; undefined when either isn't set */ +export const botBuiltOf = (environment: NodeJS.ProcessEnv) => { + if (!environment.BOT_CI_TEMPLATE || environment.BOT_CI_TEMPLATE_OF === undefined) return undefined + return environment.BOT_CI_TEMPLATE_OF.toLowerCase() === environment.BOT_CI_TEMPLATE.toLowerCase() +} + if (import.meta.main) { - const plan = planChecks(directorySnapshot(process.cwd())) + const plan = planChecks(directorySnapshot(process.cwd()), { botBuilt: botBuiltOf(process.env) }) console.log(`Build: ${plan.skip ? "skipping — " : ""}${plan.reason}`) if (process.env.GITHUB_OUTPUT) appendFileSync(process.env.GITHUB_OUTPUT, `${outputLinesOf(plan).join("\n")}\n`) if (process.env.GITHUB_STEP_SUMMARY) appendFileSync(process.env.GITHUB_STEP_SUMMARY, `### Build\n${plan.skip ? "Skipped" : "Plan"}: ${plan.reason}\n`) diff --git a/actions/pr-lint/lintPullRequest.ts b/actions/pr-lint/lintPullRequest.ts index 210c9d8..a86a1ba 100644 --- a/actions/pr-lint/lintPullRequest.ts +++ b/actions/pr-lint/lintPullRequest.ts @@ -49,9 +49,12 @@ const sections: Section[] = [ export const isLintInScope = ({ scope, author, body, botLogins = defaultBotLogins }: { scope?: string; author?: string; body?: string; botLogins?: string[] }) => scope === "all" || (author !== undefined && botLogins.includes(author)) || harnessMarker.test(body ?? "") -/** the content under a markdown heading, up to the next `##`/`###` heading (html comments after the heading are ignored) */ +/** + * the content under a markdown heading, up to the next `##`/`###` heading (html comments after the heading are ignored). + * the heading must be a line of its own, so a mention of it in prose or backticks doesn't count + */ export const sectionContentOf = (body: string, heading: string) => - body.match(new RegExp(`${heading}\\s*(?:\\s*)*([\\s\\S]*?)(?=^## |^### |\\s*$)`, "m"))?.[1]?.trim() + body.match(new RegExp(`^${heading}[ \\t]*$\\s*(?:\\s*)*([\\s\\S]*?)(?=^## |^### |\\s*$)`, "m"))?.[1]?.trim() /** every title and description problem, or why the pr is exempt (dependabot, merge queue, reverts) */ export const lintPullRequest = ({ title, body, branch }: PullRequestText): LintResult => { diff --git a/actions/second-look/action.yml b/actions/second-look/action.yml index 05fe3ad..15be34d 100644 --- a/actions/second-look/action.yml +++ b/actions/second-look/action.yml @@ -5,7 +5,7 @@ inputs: description: "prepare, synthesize or abandon" required: true github-token: - description: "a Touchless Bot App token for the PR's repo only. prepare: contents + pull-requests read; synthesize: contents read, issues + pull-requests write (posting falls back to it when post-token is blank, and it re-requests clanker-in-chief after the inline review); abandon: issues write, for the didn't-finish reply when post-token is blank" + description: "a Touchless Bot App token for the PR's repo only. prepare: contents + pull-requests read; synthesize: contents read, issues + pull-requests write (posting falls back to it when post-token is blank, and it re-requests clanker-in-chief after the inline review); abandon: issues + pull-requests write, for the didn't-finish comment review when post-token is blank" required: false checks-token: description: "a Touchless Bot App token with checks write on the PR's repo, minted on its own so a missing Checks permission only means no check. blank: no check" diff --git a/actions/second-look/approveHintOf.ts b/actions/second-look/approveHintOf.ts new file mode 100644 index 0000000..3c9a785 --- /dev/null +++ b/actions/second-look/approveHintOf.ts @@ -0,0 +1,11 @@ +/** the comment that forces an approval; the relay (scripts/bot-ci-relay/requestOf.ts) acts on it */ +export const approveCommand = "@clanker-in-chief approve" + +/** + * the line under a review that didn't approve: tags the pr's author (not a bot, which can't read it) and tells them + * they can force an approval for now + */ +export const approveHintOf = (author?: string) => { + const who = author && !author.endsWith("[bot]") ? `@${author}: ` : "" + return `${who}if this shouldn't hold the PR up, comment \`${approveCommand}\` (add why after it) and I'll approve it anyway.` +} diff --git a/actions/second-look/changedLinesOf.ts b/actions/second-look/changedLinesOf.ts new file mode 100644 index 0000000..f256c9f --- /dev/null +++ b/actions/second-look/changedLinesOf.ts @@ -0,0 +1,14 @@ +/** second look reviews a pr only when github's additions plus deletions stay at or under this */ +export const maxChangedLines = 5_000 + +/** lines a pr changes, github's additions plus deletions; undefined when github didn't report both */ +export const changedLinesOf = ({ additions, deletions }: { additions?: number; deletions?: number }) => + typeof additions === "number" && typeof deletions === "number" && Number.isInteger(additions) && Number.isInteger(deletions) && additions >= 0 && deletions >= 0 + ? additions + deletions + : undefined + +/** the line count when it's over the limit; undefined when the pr is reviewable, or when its size isn't known */ +export const overLimitOf = (pull: { additions?: number; deletions?: number }) => { + const changed = changedLinesOf(pull) + return changed !== undefined && changed > maxChangedLines ? changed : undefined +} diff --git a/actions/second-look/checkRun.ts b/actions/second-look/checkRun.ts index d18ddb6..c7df0b5 100644 --- a/actions/second-look/checkRun.ts +++ b/actions/second-look/checkRun.ts @@ -1,7 +1,8 @@ #!/usr/bin/env node /** * second look's check on the pr, in a dispatched run: `start` opens it on the head while the - * review runs, `abandon` closes it when the run ended without a review. synthesize.ts closes it + * review runs, `abandon` closes it when the run ended without a review, unless something already did (the relay's + * forced approval turns it green and cancels the run). synthesize.ts closes it * with the verdict. best effort: without the app's checks permission there's just no check, * and the review comment still posts. * @@ -28,6 +29,8 @@ const main = async () => { if (action === "abandon") { const id = process.env.SECOND_LOOK_CHECK_RUN_ID if (!id) return console.log("second look: no check to close") + const current = await fetch(`https://api.github.com/repos/${repository}/check-runs/${id}`, { headers: { authorization: `Bearer ${token}`, accept: "application/vnd.github+json" } }) + if (current.ok && ((await current.json()) as { status?: string }).status === "completed") return console.log("second look: the check is already closed (a forced approval?), leaving it") await sendCheckRun({ token, repository, id, body: checkAbandonedOf({ result: process.env.SECOND_LOOK_RESULT || "failure", detailsUrl }) }) return console.log(`second look: closed the check on ${repository} (${id})`) } diff --git a/actions/second-look/fetchPull.ts b/actions/second-look/fetchPull.ts index cf4931f..fff3f7c 100644 --- a/actions/second-look/fetchPull.ts +++ b/actions/second-look/fetchPull.ts @@ -2,7 +2,8 @@ /** * second look, in a dispatched run: fetches the pr it was asked to look at and writes it as a * pull_request event (targetOf.ts reads it), so every later step sees the same pr. it refuses - * repos outside the run's own org and prs that aren't open. with SECOND_LOOK_HEAD_SHA (what + * repos outside the run's own org, prs that aren't open, and prs that change over 5,000 lines. + * with SECOND_LOOK_HEAD_SHA (what * prepare checked out), a pr that has moved on since is still reviewed at that commit. * * node fetchPull.ts @@ -13,6 +14,7 @@ */ import { appendFileSync, mkdirSync, writeFileSync } from "node:fs" import { dirname } from "node:path" +import { maxChangedLines, overLimitOf } from "./changedLinesOf.ts" import type { PullRequest } from "./types.ts" /** why second look won't look at this pr; undefined when it will */ @@ -20,6 +22,8 @@ export const refusalOf = ({ repository, owner, pull }: { repository: string; own if (!/^[\w.-]+\/[\w.-]+$/.test(repository)) return `"${repository}" isn't an owner/name repository` if (repository.split("/")[0].toLowerCase() !== owner.toLowerCase()) return `${repository} isn't in ${owner}` if (pull && pull.state !== "open") return `${repository}#${pull.number} is ${pull.state ?? "not open"}` + const over = pull ? overLimitOf(pull) : undefined + if (pull && over !== undefined) return `${repository}#${pull.number} changes ${over} lines, over the ${maxChangedLines} second look will review` return undefined } diff --git a/actions/second-look/notifyUnfinished.ts b/actions/second-look/notifyUnfinished.ts index 4abaa77..6d22ce5 100644 --- a/actions/second-look/notifyUnfinished.ts +++ b/actions/second-look/notifyUnfinished.ts @@ -1,14 +1,16 @@ #!/usr/bin/env node /** * second look's wrap-up when a run ended without a review (and wasn't cancelled): the relay already - * told the pr a review is coming, so this says it isn't, with a link to the run. best effort. + * told the pr a review is coming, so this says it isn't, with a link to the run, as a comment review + * that tags the author with the force-approval hint (a plain pr comment when a review can't be posted). best effort. * * node notifyUnfinished.ts * - * env: SECOND_LOOK_POST_TOKEN (clanker-in-chief's PAT) or GITHUB_TOKEN (the app, issues write), + * env: SECOND_LOOK_POST_TOKEN (clanker-in-chief's PAT) or GITHUB_TOKEN (the app, issues + pull requests write), * SECOND_LOOK_REPOSITORY (validated by prepare), SECOND_LOOK_NUMBER, SECOND_LOOK_RESULT, SECOND_LOOK_STAGE, * GITHUB_SERVER_URL, GITHUB_REPOSITORY, GITHUB_RUN_ID. */ +import { approveHintOf } from "./approveHintOf.ts" import { unfinishedReplyOf } from "./unfinishedReplyOf.ts" const main = async () => { @@ -16,14 +18,20 @@ const main = async () => { const repository = process.env.SECOND_LOOK_REPOSITORY ?? "" const number = process.env.SECOND_LOOK_NUMBER ?? "" if (!token || !repository || !/^\d+$/.test(number)) return console.log("second look: no token or pr to tell, skipping the reply") + const api = (path: string, body?: unknown) => + fetch(`https://api.github.com/repos/${repository}/${path}`, { + method: body === undefined ? "GET" : "POST", + headers: { authorization: `Bearer ${token}`, accept: "application/vnd.github+json", "content-type": "application/json" }, + ...(body === undefined ? {} : { body: JSON.stringify(body) }), + }) const detailsUrl = `${process.env.GITHUB_SERVER_URL || "https://github.com"}/${process.env.GITHUB_REPOSITORY}/actions/runs/${process.env.GITHUB_RUN_ID}` - const body = unfinishedReplyOf({ result: process.env.SECOND_LOOK_RESULT || "failure", stage: process.env.SECOND_LOOK_STAGE || "review", detailsUrl }) - const response = await fetch(`https://api.github.com/repos/${repository}/issues/${number}/comments`, { - method: "POST", - headers: { authorization: `Bearer ${token}`, accept: "application/vnd.github+json", "content-type": "application/json" }, - body: JSON.stringify({ body }), - }) - if (!response.ok) throw new Error(`replying answered ${response.status}`) + const pull = await api(`pulls/${number}`) + const author = pull.ok ? ((await pull.json()) as { user?: { login?: string } }).user?.login : undefined + const body = `${unfinishedReplyOf({ result: process.env.SECOND_LOOK_RESULT || "failure", stage: process.env.SECOND_LOOK_STAGE || "review", detailsUrl })}\n\n${approveHintOf(author)}` + const review = await api(`pulls/${number}/reviews`, { event: "COMMENT", body }) + if (review.ok) return console.log(`second look: told ${repository}#${number} the run didn't finish (comment review)`) + const comment = await api(`issues/${number}/comments`, { body }) + if (!comment.ok) throw new Error(`replying answered ${review.status}, then ${comment.status}`) console.log(`second look: told ${repository}#${number} the run didn't finish`) } diff --git a/actions/second-look/postReview.ts b/actions/second-look/postReview.ts index b8832b5..0dcfedb 100644 --- a/actions/second-look/postReview.ts +++ b/actions/second-look/postReview.ts @@ -2,9 +2,10 @@ import type { InlineComment } from "./inlineCommentsOf.ts" import type { ReviewEvent } from "./reviewEventOf.ts" /** - * submits one review on `commitId` as `event`, carrying the new inline comments. github answers 422 when the poster - * can't approve or request changes (its own pr), so that one goes in as a plain comment review instead. a comment - * review with nothing inline isn't posted. resolves to the review's url and the event it went in as, or undefined + * submits one review on `commitId` as `event`, carrying the new inline comments; every run gets one, so the pr always + * shows an approval, a change request or a comment from second look. github answers 422 when the poster can't approve + * or request changes (its own pr), so that one goes in as a plain comment review instead. resolves to the review's url + * and the event it went in as */ export const postReview = async ({ token, repository, number, commitId, event, comments, body, fetchImpl = fetch }: { token: string @@ -15,8 +16,7 @@ export const postReview = async ({ token, repository, number, commitId, event, c comments: InlineComment[] body: string fetchImpl?: typeof fetch -}): Promise<{ url: string; event: ReviewEvent } | undefined> => { - if (event === "COMMENT" && !comments.length) return undefined +}): Promise<{ url: string; event: ReviewEvent }> => { const response = await fetchImpl(`https://api.github.com/repos/${repository}/pulls/${number}/reviews`, { method: "POST", headers: { authorization: `Bearer ${token}`, accept: "application/vnd.github+json", "content-type": "application/json" }, diff --git a/actions/second-look/reviewBodyOf.ts b/actions/second-look/reviewBodyOf.ts index 4688553..3787df6 100644 --- a/actions/second-look/reviewBodyOf.ts +++ b/actions/second-look/reviewBodyOf.ts @@ -1,6 +1,9 @@ import { cliNameOf } from "./cliNameOf.ts" import type { Family } from "./types.ts" -/** the review's own body: the headline and the reviewer's summary; the testing and architecture detail stays in the summary comment */ -export const reviewBodyOf = ({ headline, summary, reviewer }: { headline: string; summary?: string; reviewer: Family }) => - [headline, ...(summary ? ["", summary] : []), "", `Reviewed via ${cliNameOf(reviewer)} · testing and architecture are in the Second Look comment`].join("\n") +/** + * the review's own body: the headline, the reviewer's summary (or why there isn't one) and, when it didn't approve, + * the force-approval hint; the testing and architecture detail stays in the summary comment + */ +export const reviewBodyOf = ({ headline, summary, hint, reviewer }: { headline: string; summary?: string; hint?: string; reviewer: Family }) => + [headline, ...(summary ? ["", summary] : []), ...(hint ? ["", hint] : []), "", `Reviewed via ${cliNameOf(reviewer)} · testing and architecture are in the Second Look comment`].join("\n") diff --git a/actions/second-look/synthesize.ts b/actions/second-look/synthesize.ts index 168d1b9..ba1c3f2 100644 --- a/actions/second-look/synthesize.ts +++ b/actions/second-look/synthesize.ts @@ -6,8 +6,9 @@ * architecture, reviews the diff against all that and the rules, folds in both * testers' reports, and follows up on its own open threads from earlier runs: a fixed one * gets a reply and is resolved, a changed one gets a reply, and nothing is posted twice. - * then it submits one review as clanker-in-chief: approve on a pass, request changes on a block, with the new - * findings that sit on the diff as inline comments. then one sticky pr comment, and it closes the pr's check with + * then it submits one review as clanker-in-chief: approve on a pass, request changes on a block, a comment when it + * couldn't review, with the new findings that sit on the diff as inline comments; one that doesn't approve tags the + * author with `@clanker-in-chief approve`, which forces an approval for now. then one sticky pr comment, and it closes the pr's check with * the outcome. it never runs pr code. a block turns the pr's check red only with SECOND_LOOK_MODE=enforce. exits 1 only * when it crashes before closing the check, so the workflow's wrap-up closes it instead. * @@ -25,6 +26,7 @@ import { appendFileSync, existsSync, mkdirSync, readFileSync, writeFileSync } from "node:fs" import { homedir } from "node:os" import { join } from "node:path" +import { approveHintOf } from "./approveHintOf.ts" import { architectureDocsOf } from "./architectureDocsOf.ts" import { childEnvironmentOf } from "./childEnvironmentOf.ts" import { cliCommandOf, defaultModels } from "./cliCommandOf.ts" @@ -152,7 +154,12 @@ const main = async () => { const followUps = verdict ? await followUpThreads({ threads, followUps: verdict.earlier, onThread, headSha: pull.head.sha, reviewer, token: postToken, resolveTokens: [postToken, appToken], repository, number: pull.number, warn }) : [] const event = reviewEventOf(verdict) const headline = headlineOf({ verdict, open: followUps.filter(({ status }) => status !== "fixed").length }) - const reviewBody = reviewBodyOf({ headline, summary: verdict ? threadKeysLinkedOf(threads)(verdict.summary) : undefined, reviewer }) + const reviewBody = reviewBodyOf({ + headline, + summary: verdict ? threadKeysLinkedOf(threads)(verdict.summary) : error ? `Couldn't review: ${error}` : undefined, + hint: event === "APPROVE" ? undefined : approveHintOf(pull.user?.login), + reviewer, + }) const posted = await postReview({ token: postToken, repository, number: pull.number, commitId: pull.head.sha, event, comments: fresh.map(({ comment }) => comment), body: reviewBody }) .catch((failure: Error) => { warn(`couldn't post the review, the summary has every finding: ${failure.message}`) diff --git a/actions/second-look/types.ts b/actions/second-look/types.ts index 096e31c..92f02c3 100644 --- a/actions/second-look/types.ts +++ b/actions/second-look/types.ts @@ -14,6 +14,8 @@ export type PullRequest = { base: { sha: string; ref?: string; repo?: { full_name?: string } } head: { sha: string; ref?: string; repo?: { full_name?: string } } user?: { login?: string } + additions?: number + deletions?: number } /** one testing step a tester tried */