From 601e53c7cb55d445bbb5802501a3976b173a6456 Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Wed, 4 Nov 2026 19:56:06 -0800 Subject: [PATCH 1/3] fix(hub): run the portal's Merge through the merge gate (CL-12170) --- .../github-read-access.postgres.test.ts | 29 +++++++++++++++++-- apps/hub/src/github/pr-actions.ts | 17 ++++++++++- .../Corbits Triage User Manual.md | 2 +- packages/github-tool/src/do.ts | 14 ++++++--- .../triage-workflows/src/logic/actions.ts | 8 +++-- 5 files changed, 60 insertions(+), 10 deletions(-) diff --git a/apps/hub/src/github/github-read-access.postgres.test.ts b/apps/hub/src/github/github-read-access.postgres.test.ts index a95a65c34..9474485fe 100644 --- a/apps/hub/src/github/github-read-access.postgres.test.ts +++ b/apps/hub/src/github/github-read-access.postgres.test.ts @@ -35,6 +35,8 @@ const HEAD = "abc1230000000000000000000000000000000000"; const testSchema = `corbits_read_access_${randomBytes(6).toString("hex")}`; const requested: string[] = []; const reviewed: unknown[] = []; +let reviews: unknown[] = []; +let checkRuns: unknown[] = []; const READS: Record = { [`/repos/${REPO}/pulls`]: [ @@ -45,7 +47,6 @@ const READS: Record = { [`/repos/${REPO}/pulls/8/commits`]: [{ sha: HEAD, commit: { message: "fix: the widget" } }], [`/repos/${REPO}/pulls/8/files`]: [{ filename: "src/widget.ts", status: "modified", additions: 1, deletions: 0, patch: "@@ -1 +1 @@\n+fixed" }], [`/repos/${REPO}/issues/8/comments`]: [], - [`/repos/${REPO}/commits/${HEAD}/check-runs`]: { check_runs: [] }, [`/repos/${REPO}/commits/${HEAD}/status`]: { statuses: [] }, }; @@ -55,7 +56,9 @@ async function github(req: Request): Promise { if (pathname === "/app/installations/1/access_tokens") return Response.json({ token: "installation-token", expires_at: new Date(Date.now() + 3_600_000).toISOString() }); if (req.method === "GET" && pathname in READS) return Response.json(READS[pathname]); if (pathname === `/repos/${REPO}/installation`) return Response.json({ id: 1 }); - if (req.method === "GET" && pathname === `/repos/${REPO}/pulls/8/reviews`) return Response.json([]); + if (req.method === "GET" && pathname === `/repos/${REPO}/pulls/8/reviews`) return Response.json(reviews); + if (req.method === "POST" && pathname === "/graphql") return Response.json({ data: { repository: { pullRequest: { reviewThreads: { nodes: [], pageInfo: { hasNextPage: false, endCursor: null } } } } } }); + if (req.method === "GET" && pathname === `/repos/${REPO}/commits/${HEAD}/check-runs`) return Response.json({ total_count: checkRuns.length, check_runs: checkRuns }); if (req.method === "POST" && pathname === `/repos/${REPO}/pulls/8/reviews`) { reviewed.push(await req.json()); return Response.json({ id: 1, state: "APPROVED" }); @@ -125,6 +128,7 @@ beforeAll(async () => { { id: "prn_admin", tenantId: TENANT, kind: "user", refId: "usr_admin", status: "active" }, { id: "prn_approver", tenantId: TENANT, kind: "user", refId: "usr_approver", status: "active" }, { id: "prn_lent", tenantId: TENANT, kind: "user", refId: "usr_lent", status: "active" }, + { id: "prn_merger", tenantId: TENANT, kind: "user", refId: "usr_merger", status: "active" }, ]); await db.insert(schema.role).values([ { id: "rol_member", tenantId: TENANT, name: "member", isSystem: true }, @@ -146,6 +150,8 @@ beforeAll(async () => { { id: "grt_approver_labels", tenantId: TENANT, principalId: "prn_approver", resource: `triage-do:labels:github:${REPO}`, action: "invoke", effect: "allow", origin: "system" }, { id: "grt_approver_comment", tenantId: TENANT, principalId: "prn_approver", resource: `triage-do:comment:github:${REPO}`, action: "invoke", effect: "allow", origin: "system" }, { id: "grt_lent_use", tenantId: TENANT, principalId: "prn_lent", resource: "credential:crd_github", action: "use", effect: "allow", origin: "system" }, + { id: "grt_merger_use", tenantId: TENANT, principalId: "prn_merger", resource: "credential:crd_github", action: "use", effect: "allow", origin: "system" }, + { id: "grt_merger_merge", tenantId: TENANT, principalId: "prn_merger", resource: `triage-do:merge:github:${REPO}`, action: "invoke", effect: "allow", origin: "system" }, { id: "grt_triage_approve", tenantId: TENANT, roleId: "rol_triage", resource: "triage-do:approve:*", action: "invoke", effect: "allow", origin: "role" }, ]); await db.insert(schema.provider).values({ id: "prv_github", tenantId: TENANT, name: "github", plugin: "http", apiBaseUrl: githubApiOrigin }); @@ -206,6 +212,25 @@ test("PostgreSQL: approving needs the approve Do grant on top of use, and pins t expect(reviewed).toHaveLength(1); }); +test("PostgreSQL: the portal's Merge is refused while a reviewer requests changes or CI has failed, and nothing is merged", async () => { + const merge = { action: "merge", repo: REPO, number: 8, headSha: HEAD } as const; + try { + reviews = [{ user: { login: "hubot" }, state: "CHANGES_REQUESTED", commit_id: HEAD }]; + checkRuns = [{ name: "ci", status: "completed", conclusion: "success" }]; + const changes = await prActions(merge, member("usr_merger")); + expect({ status: changes.status, body: await changes.json() }).toMatchObject({ status: 409, body: { error: { code: "merge_refused", message: expect.stringContaining("hubot") } } }); + + reviews = []; + checkRuns = [{ name: "ci", status: "completed", conclusion: "failure" }]; + const failing = await prActions(merge, member("usr_merger")); + expect({ status: failing.status, body: await failing.json() }).toMatchObject({ status: 409, body: { error: { code: "merge_refused", message: expect.stringContaining("CI has failed") } } }); + expect(requested).not.toContain(`PUT /repos/${REPO}/pulls/8/merge`); + } finally { + reviews = []; + checkRuns = []; + } +}); + test("PostgreSQL: every verdict route needs workflow-run read, which holding the GitHub credential does not give", async () => { const unused = {} as never; const app = guardedApp(handle.db, sessionOf, grants, { diff --git a/apps/hub/src/github/pr-actions.ts b/apps/hub/src/github/pr-actions.ts index bada78936..55bec50a4 100644 --- a/apps/hub/src/github/pr-actions.ts +++ b/apps/hub/src/github/pr-actions.ts @@ -15,8 +15,10 @@ import { sendCodeReview, upsertTriageComment, } from "@corbits/github-tool/github"; -import { codeownersForPr, type GithubFetch } from "@corbits/github-reads"; +import { readMergeGate } from "@corbits/github-tool/do"; +import { codeownersForPr, readPr, type GithubFetch } from "@corbits/github-reads"; import { ownedLabels, TRIAGE_LABELS, type ActionKind, type SendCodeReviewInput, type SentThreads } from "@corbits/triage-contracts"; +import { mergesWithoutCi } from "@corbits/triage-workflows/logic/actions"; import { codeReviewSummary } from "@corbits/triage-workflows/logic/render"; import type { CodeReviewRuns, CompletedReview } from "../code-review-runs.js"; import type { PolicyReader } from "./repo-policy.js"; @@ -139,7 +141,20 @@ export function createGithubPrActions(deps: PortalCredentialDeps & { return read.status === "ok" ? ownedLabels(read.pack.labels) : TRIAGE_LABELS; } + /** The portal's Merge is held by the same gate as triage's own; bypassing it is a GitHub admin's call, made on GitHub. */ + async function mergeRefused(gh: GithubFetch, login: string, tenantId: string, { repo, number, headSha }: { repo: string; number: number; headSha?: string }) { + const read = await deps.readCheckPack(tenantId, repo, "github"); + const ciOptional = read.status === "ok" && mergesWithoutCi(read.pack); + const sha = headSha ?? (await readPr(gh, repo, number)).sha; + const { gate } = await readMergeGate(gh, { repo, number, sha, login, ciOptional }); + return gate.allowed ? undefined : failure(409, "merge_refused", `Not merged: ${gate.reason}.`); + } + async function act(gh: GithubFetch, login: string, tenantId: string, body: typeof ActionBody.infer) { + if (body.action === "merge") { + const refused = await mergeRefused(gh, login, tenantId, body); + if (refused) return refused; + } if (body.action === "code-review") { const { completed, carriedTo } = await deps.codeReviews.latest(tenantId, { repo: body.repo, number: body.number, head: body.headSha }); return sendReview(function sendToGithub(input) { diff --git a/docs/Corbits Triage User Manual/Corbits Triage User Manual.md b/docs/Corbits Triage User Manual/Corbits Triage User Manual.md index 5443b7b07..993e76f71 100644 --- a/docs/Corbits Triage User Manual/Corbits Triage User Manual.md +++ b/docs/Corbits Triage User Manual/Corbits Triage User Manual.md @@ -488,7 +488,7 @@ Clicking a pull request opens it on the right. The pane is built to be read top - **Request changes**: **Request changes from @ada: Linked issue**. When no rule failed it says what GitHub or Bitbucket holds it for, such as **Request changes from @ada: behind its base branch**, or just **Request changes from @ada**. - **Approve**: **Review and approve** with every rule met, or **Approve anyway, or ask @ada to fix: Linked issue** when one failed. - **Reply**: **Reply to @ada**, or **Reply to @ada · not met: Linked issue** when a rule failed and Triage drafted no feedback. - - **Merge**: **Ready to merge**, or **Ready to merge · still not met: Linked issue** when a rule failed. A pull request that passed but that GitHub or Bitbucket will not merge yet says why instead, such as **Can't merge yet: waiting for a required review**, **Can't merge yet: behind its base branch**, **Can't merge yet: the branch's rules block the merge** or, on Bitbucket, the reason its merge checks give, and **Merge** stays off until it can. A pull request Triage opened itself is merged rather than approved, so it reads **Ready to merge** too; with no button and a rule failing it reads **Not met: Linked issue**, and when GitHub or Bitbucket holds it with nothing failed, **Blocked:** and why, such as **Blocked: behind its base branch**. + - **Merge**: **Ready to merge**, or **Ready to merge · still not met: Linked issue** when a rule failed. A pull request that passed but that GitHub or Bitbucket will not merge yet says why instead, such as **Can't merge yet: waiting for a required review**, **Can't merge yet: behind its base branch**, **Can't merge yet: the branch's rules block the merge** or, on Bitbucket, the reason its merge checks give, and **Merge** stays off until it can. Pressing **Merge** follows the same rules as Triage's own merge: it is refused, with the reason, while a reviewer requests changes, a person's review thread is open or CI has not passed; a GitHub admin can still merge on GitHub. A pull request Triage opened itself is merged rather than approved, so it reads **Ready to merge** too; with no button and a rule failing it reads **Not met: Linked issue**, and when GitHub or Bitbucket holds it with nothing failed, **Blocked:** and why, such as **Blocked: behind its base branch**. Some decisions come before the button. A pull request the author updated after your review reads **Re-review: @ada updated it**, then what is still open, such as **· still not met: Linked issue** or **· still to decide: Focused**, or **· all rules met** when nothing is. A rule you need to decide reads **Decide: Focused (Triage wasn't sure)** when the model was unsure, **Decide: Linked issue (your policy asks you)** when the policy asks you about it when it fails, or **(see why below)** for both. A rule that could not be checked reads **Couldn't check Merge conflicts · use Triage again in More**. After you approve, it says only what is still open or what holds the merge, such as **You approved · still not met: Linked issue** or **You approved · ready to merge**. When an action waits for your approval on a pull request that is otherwise the author's, the line asks about it, such as **Needs you: Approve this pull request?** or **Needs you: Add label bug?**. - **Done:** what the run ended with: the verdict as its label on the pull request says it (**Ready**, **Not ready** or **Needs human review**), what Triage posted on its own, and whose turn it is, such as **Done: Not ready · added label risk: medium · waiting on @ada to fix: Linked issue**. A merged or closed pull request reads **Done: Merged** or **Done: Closed**, a draft with a verdict **Done: Not ready · draft · waiting on @ada**, and once you approve with nothing left for you, **Done: You approved**. **Reply** stays available if you want to add something. A verdict Triage could not settle reads **Done: Needs human review · Triage could not settle it · Triage again**. When the author pushed commits Triage has not checked, the line adds **newer commits not yet checked**, and **Triage again** when nothing in it is yours. A passing pull request is never merged before Triage checks them, so it reads **Done: Ready to merge · newer commits not yet checked · Triage again**. On the author's turn it reads, for example, **Done: Not ready · waiting on @ada · newer commits not yet checked · Triage again**. diff --git a/packages/github-tool/src/do.ts b/packages/github-tool/src/do.ts index 8f2ca28c3..c3ae25d3d 100644 --- a/packages/github-tool/src/do.ts +++ b/packages/github-tool/src/do.ts @@ -102,6 +102,15 @@ function closedOutcome(kind: ActionKind, merged: boolean): DoOutcome { throw new DoNotRunnableError(NOT_OPEN); } +/** Reads the head's CI, each reviewer's latest review and the open review threads, and asks the merge gate. */ +export async function readMergeGate(gh: GithubFetch, { repo, number, sha, login, ciOptional }: { repo: string; number: number; sha: string; login: string; ciOptional: boolean }) { + const ci = summarizeChecks(await getChecks(gh, repo, sha)); + const reviews = await getReviews(gh, repo, number, login); + const changesRequestedBy = [...latestDecisive(reviews)].filter(([, state]) => state === "CHANGES_REQUESTED").map(([reviewer]) => reviewer); + const gate = mergeGate({ ci, ciOptional, changesRequestedBy, threadsHold: await unresolvedThreadsHold(gh, repo, number, login) }); + return { gate, reviews }; +} + /** Writes one Do unless GitHub already shows it, so a repeat after a lost audit write changes nothing. `login` names the App's bot account, whose reviews are triage's own. */ export async function executeDo(gh: GithubFetch, { request, step }: DoInput, login: string): Promise { const { repo, number } = request; @@ -176,10 +185,7 @@ export async function executeDo(gh: GithubFetch, { request, step }: DoInput, log // CI, reviews and review threads are read again here because a held merge can be approved long after its verdict; branch protection stays the backstop. case "merge": { onHead(pr.sha, request, step); - const ci = summarizeChecks(await getChecks(gh, repo, step.headSha)); - const reviews = await getReviews(gh, repo, number, login); - const changesRequestedBy = [...latestDecisive(reviews)].filter(([, state]) => state === "CHANGES_REQUESTED").map(([reviewer]) => reviewer); - const gate = mergeGate({ ci, ciOptional: "ciOptional" in target, changesRequestedBy, threadsHold: await unresolvedThreadsHold(gh, repo, number, login) }); + const { gate, reviews } = await readMergeGate(gh, { repo, number, sha: step.headSha, login, ciOptional: "ciOptional" in target }); if (!gate.allowed) throw new DoNotRunnableError(gate.reason); // Execute runs before the mirror, and a protected branch refuses the merge while triage's own request on this head stands; one on an earlier head is the mirror's to take back. const ownRequest = reviews.some((review) => review.triageRequest && review.state === "CHANGES_REQUESTED" && review.commitId === step.headSha); diff --git a/packages/triage-workflows/src/logic/actions.ts b/packages/triage-workflows/src/logic/actions.ts index 9ad860095..a8a69f34f 100644 --- a/packages/triage-workflows/src/logic/actions.ts +++ b/packages/triage-workflows/src/logic/actions.ts @@ -52,6 +52,11 @@ export interface ActionInput { repoSource?: RepoSource; } +/** Only a policy that switched CI green off may merge a head with no CI; one that never names CI still requires it. */ +export function mergesWithoutCi(pack: CheckPack): boolean { + return pack.checks["ci-green"]?.enabled === false; +} + /** Catch-up runs the policy's actions whatever woke it. */ export function wakes(pack: CheckPack, facts: PrFacts): boolean { return facts.events.includes("catch-up") || facts.events.some((event) => (pack.actions.when as readonly string[]).includes(event)); @@ -154,8 +159,7 @@ function resolve(step: Exclude, input: ActionInput): Re case "comment": return step.target; case "merge": - // Only a policy that switched CI green off may merge a head with no CI; one that never names CI still requires it. - return input.pack.checks["ci-green"]?.enabled === false ? { ciOptional: true } : {}; + return mergesWithoutCi(input.pack) ? { ciOptional: true } : {}; case "approve": case "close": return {}; From aafff29ccae09e7525617e9e64f353f72012548a Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Wed, 4 Nov 2026 20:00:58 -0800 Subject: [PATCH 2/3] fix(hub): pin the portal's Merge to the gated head and read the policy once (CL-12170) --- .../github-read-access.postgres.test.ts | 13 +++++-- apps/hub/src/github/pr-actions.ts | 38 +++++++++---------- .../triage-workflows/src/logic/actions.ts | 11 ++++-- 3 files changed, 37 insertions(+), 25 deletions(-) diff --git a/apps/hub/src/github/github-read-access.postgres.test.ts b/apps/hub/src/github/github-read-access.postgres.test.ts index 9474485fe..90b3b886d 100644 --- a/apps/hub/src/github/github-read-access.postgres.test.ts +++ b/apps/hub/src/github/github-read-access.postgres.test.ts @@ -179,7 +179,7 @@ test("PostgreSQL: holding read or use on the GitHub credential reads pull reques expect(await details.json()).toMatchObject({ pr: { number: 8 }, files: [{ path: "src/widget.ts" }] }); } - const act = await prActions({ action: "merge", repo: REPO, number: 8 }, member("usr_member")); + const act = await prActions({ action: "merge", repo: REPO, number: 8, headSha: HEAD }, member("usr_member")); expect(act.status).toBe(403); expect(await act.json()).toEqual({ error: { code: "forbidden", message: "You do not have access to the GitHub App credential." } }); expect(requested.some((line) => line.startsWith("PUT"))).toBe(false); @@ -202,7 +202,7 @@ test("PostgreSQL: approving needs the approve Do grant on top of use, and pins t expect(approved.status).toBe(200); expect(reviewed).toEqual([{ body: "", event: "APPROVE", commit_id: HEAD }]); - const merge = await prActions({ action: "merge", repo: REPO, number: 8 }, member("usr_approver")); + const merge = await prActions({ action: "merge", repo: REPO, number: 8, headSha: HEAD }, member("usr_approver")); expect(merge.status).toBe(403); expect(await merge.json()).toEqual({ error: { code: "forbidden", message: "You do not have the grant for this kind of Do in this repository." } }); @@ -212,7 +212,7 @@ test("PostgreSQL: approving needs the approve Do grant on top of use, and pins t expect(reviewed).toHaveLength(1); }); -test("PostgreSQL: the portal's Merge is refused while a reviewer requests changes or CI has failed, and nothing is merged", async () => { +test("PostgreSQL: the portal's Merge is refused while a reviewer requests changes or CI has failed, or without the head it was shown, and nothing is merged", async () => { const merge = { action: "merge", repo: REPO, number: 8, headSha: HEAD } as const; try { reviews = [{ user: { login: "hubot" }, state: "CHANGES_REQUESTED", commit_id: HEAD }]; @@ -224,6 +224,13 @@ test("PostgreSQL: the portal's Merge is refused while a reviewer requests change checkRuns = [{ name: "ci", status: "completed", conclusion: "failure" }]; const failing = await prActions(merge, member("usr_merger")); expect({ status: failing.status, body: await failing.json() }).toMatchObject({ status: 409, body: { error: { code: "merge_refused", message: expect.stringContaining("CI has failed") } } }); + + checkRuns = [{ name: "ci", status: "completed", conclusion: "success" }]; + const app = guardedApp(handle.db, sessionOf, grants, { prActions }); + const unpinned = await app.request(`/api/integrations/github-actions/${TENANT}`, { + method: "POST", headers: { "x-test-user": "usr_merger", authorization: "Bearer test", "content-type": "application/json" }, body: JSON.stringify({ action: "merge", repo: REPO, number: 8 }), + }); + expect({ status: unpinned.status, body: await unpinned.json() }).toMatchObject({ status: 400, body: { error: { code: "invalid_request" } } }); expect(requested).not.toContain(`PUT /repos/${REPO}/pulls/8/merge`); } finally { reviews = []; diff --git a/apps/hub/src/github/pr-actions.ts b/apps/hub/src/github/pr-actions.ts index 55bec50a4..887fb4462 100644 --- a/apps/hub/src/github/pr-actions.ts +++ b/apps/hub/src/github/pr-actions.ts @@ -16,9 +16,9 @@ import { upsertTriageComment, } from "@corbits/github-tool/github"; import { readMergeGate } from "@corbits/github-tool/do"; -import { codeownersForPr, readPr, type GithubFetch } from "@corbits/github-reads"; -import { ownedLabels, TRIAGE_LABELS, type ActionKind, type SendCodeReviewInput, type SentThreads } from "@corbits/triage-contracts"; -import { mergesWithoutCi } from "@corbits/triage-workflows/logic/actions"; +import { codeownersForPr, type GithubFetch } from "@corbits/github-reads"; +import { ownedLabels, TRIAGE_LABELS, type ActionKind, type CheckPack, type SendCodeReviewInput, type SentThreads } from "@corbits/triage-contracts"; +import { mergesWithoutCi, refusalText } from "@corbits/triage-workflows/logic/actions"; import { codeReviewSummary } from "@corbits/triage-workflows/logic/render"; import type { CodeReviewRuns, CompletedReview } from "../code-review-runs.js"; import type { PolicyReader } from "./repo-policy.js"; @@ -53,7 +53,7 @@ export const ActionBody = type({ repo: /^[\w.-]+\/[\w.-]+$/, number: "number.int .or({ action: "'review'", event: "'REQUEST_CHANGES'", body: "string" }) .or({ action: "'review'", event: "'APPROVE'", body: "string", headSha: "string > 0" }) .or({ action: "'code-review'", headSha: "string > 0" }) - .or({ action: "'merge'", "headSha?": "string > 0" }) + .or({ action: "'merge'", headSha: "string > 0" }) .or({ action: "'close'", labels: "string[]", comment: "string" }), ); @@ -101,7 +101,7 @@ async function runAction(gh: GithubFetch, login: string, body: Exclude { - if (body.action !== "close") return TRIAGE_LABELS; + /** Only a close and a merge depend on the repository's policy. */ + async function policyOf(tenantId: string, body: typeof ActionBody.infer): Promise { + if (body.action !== "close" && body.action !== "merge") return undefined; const read = await deps.readCheckPack(tenantId, body.repo, "github"); - return read.status === "ok" ? ownedLabels(read.pack.labels) : TRIAGE_LABELS; + return read.status === "ok" ? read.pack : undefined; } /** The portal's Merge is held by the same gate as triage's own; bypassing it is a GitHub admin's call, made on GitHub. */ - async function mergeRefused(gh: GithubFetch, login: string, tenantId: string, { repo, number, headSha }: { repo: string; number: number; headSha?: string }) { - const read = await deps.readCheckPack(tenantId, repo, "github"); - const ciOptional = read.status === "ok" && mergesWithoutCi(read.pack); - const sha = headSha ?? (await readPr(gh, repo, number)).sha; - const { gate } = await readMergeGate(gh, { repo, number, sha, login, ciOptional }); - return gate.allowed ? undefined : failure(409, "merge_refused", `Not merged: ${gate.reason}.`); + async function mergeRefused(gh: GithubFetch, login: string, pack: CheckPack | undefined, { repo, number, headSha }: { repo: string; number: number; headSha: string }) { + const ciOptional = pack !== undefined && mergesWithoutCi(pack); + const { gate } = await readMergeGate(gh, { repo, number, sha: headSha, login, ciOptional }); + return gate.allowed ? undefined : failure(409, "merge_refused", refusalText(gate)); } async function act(gh: GithubFetch, login: string, tenantId: string, body: typeof ActionBody.infer) { - if (body.action === "merge") { - const refused = await mergeRefused(gh, login, tenantId, body); - if (refused) return refused; - } if (body.action === "code-review") { const { completed, carriedTo } = await deps.codeReviews.latest(tenantId, { repo: body.repo, number: body.number, head: body.headSha }); return sendReview(function sendToGithub(input) { return sendCodeReview(gh, input, login); }, body, completed, carriedTo); } - return runAction(gh, login, body, await ownedBy(tenantId, body)); + const pack = await policyOf(tenantId, body); + if (body.action === "merge") { + const refused = await mergeRefused(gh, login, pack, body); + if (refused) return refused; + } + return runAction(gh, login, body, pack ? ownedLabels(pack.labels) : TRIAGE_LABELS); } return async function handle(body: typeof ActionBody.infer, { tenantId, principalId }: Member): Promise { diff --git a/packages/triage-workflows/src/logic/actions.ts b/packages/triage-workflows/src/logic/actions.ts index a8a69f34f..408fff54f 100644 --- a/packages/triage-workflows/src/logic/actions.ts +++ b/packages/triage-workflows/src/logic/actions.ts @@ -1,7 +1,7 @@ import { sha256 } from "@noble/hashes/sha2.js"; import { bytesToHex, utf8ToBytes } from "@noble/hashes/utils.js"; import { missingFrom } from "@corbits/github-reads"; -import { alwaysHeld, authorGroupsOf, CATALOG_IDS, catalogCheckEnabled, checkName, ciRefusal, classificationSources, labelRules, LEVEL_LABELS, levelLabel, mergeGate, RISK_MARKS, ruleLabel, whenHolds, type ActionKind, type CheckPack, type RepoSource, type ResolvedTarget, type CiState, type Step, type StepSubject, teamSlug, type TriageEvent, type VerdictWord, VERDICTS } from "@corbits/triage-contracts"; +import { alwaysHeld, authorGroupsOf, CATALOG_IDS, catalogCheckEnabled, checkName, ciRefusal, classificationSources, labelRules, LEVEL_LABELS, levelLabel, mergeGate, RISK_MARKS, ruleLabel, whenHolds, type ActionKind, type CheckPack, type MergeRefusal, type RepoSource, type ResolvedTarget, type CiState, type Step, type StepSubject, teamSlug, type TriageEvent, type VerdictWord, VERDICTS } from "@corbits/triage-contracts"; import { issueReference, type CheckResult, type PrFacts } from "./checks.js"; import { authorFailures, bounded, inlineCode, isLabelledFinding } from "./render.js"; import { compare, errorText } from "@corbits/common"; @@ -192,6 +192,11 @@ function sentence(text: string): string { return `${text[0]!.toUpperCase()}${text.slice(1)}.`; } +/** The threads' hold is already a sentence; the gate's other reasons are not. */ +export function refusalText(refusal: MergeRefusal): string { + return refusal.cause === "threads" ? refusal.reason : sentence(refusal.reason); +} + // Cancelled CI waits on a re-run, so a step reads it as not finished. const CI: Record = { success: "passing", failure: "failing", pending: "pending", cancelled: "pending", none: "none" }; @@ -333,8 +338,8 @@ export function heldApprovals(actions: SuggestedAction[], approval: ApprovalHold if (action.kind === "merge") { const gate = mergeGate({ ci, ciOptional: "ciOptional" in action.target, changesRequestedBy: changesRequested, threadsHold: mergeHold }); if (gate.allowed) return action; - if (gate.cause === "ci") return { ...action, held: true, reason: sentence(gate.reason), ci: gate.ci }; - return { list: action.list, index: action.index, kind: action.kind, skipped: true, waits: true, reason: gate.cause === "threads" ? gate.reason : sentence(gate.reason) }; + if (gate.cause === "ci") return { ...action, held: true, reason: refusalText(gate), ci: gate.ci }; + return { list: action.list, index: action.index, kind: action.kind, skipped: true, waits: true, reason: refusalText(gate) }; } if (ci === "pending") return { ...action, held: true, reason: sentence(ciRefusal(ci)), ci }; return action; From dab0d210ec62f47bffedd819469d0c56a0dc373e Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Wed, 4 Nov 2026 20:11:52 -0800 Subject: [PATCH 3/3] docs: say a pull request with no CI merges only when its policy turns CI green off (CL-12170) --- docs/Corbits Triage User Manual/Corbits Triage User Manual.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/Corbits Triage User Manual/Corbits Triage User Manual.md b/docs/Corbits Triage User Manual/Corbits Triage User Manual.md index 993e76f71..7dee9d783 100644 --- a/docs/Corbits Triage User Manual/Corbits Triage User Manual.md +++ b/docs/Corbits Triage User Manual/Corbits Triage User Manual.md @@ -488,7 +488,7 @@ Clicking a pull request opens it on the right. The pane is built to be read top - **Request changes**: **Request changes from @ada: Linked issue**. When no rule failed it says what GitHub or Bitbucket holds it for, such as **Request changes from @ada: behind its base branch**, or just **Request changes from @ada**. - **Approve**: **Review and approve** with every rule met, or **Approve anyway, or ask @ada to fix: Linked issue** when one failed. - **Reply**: **Reply to @ada**, or **Reply to @ada · not met: Linked issue** when a rule failed and Triage drafted no feedback. - - **Merge**: **Ready to merge**, or **Ready to merge · still not met: Linked issue** when a rule failed. A pull request that passed but that GitHub or Bitbucket will not merge yet says why instead, such as **Can't merge yet: waiting for a required review**, **Can't merge yet: behind its base branch**, **Can't merge yet: the branch's rules block the merge** or, on Bitbucket, the reason its merge checks give, and **Merge** stays off until it can. Pressing **Merge** follows the same rules as Triage's own merge: it is refused, with the reason, while a reviewer requests changes, a person's review thread is open or CI has not passed; a GitHub admin can still merge on GitHub. A pull request Triage opened itself is merged rather than approved, so it reads **Ready to merge** too; with no button and a rule failing it reads **Not met: Linked issue**, and when GitHub or Bitbucket holds it with nothing failed, **Blocked:** and why, such as **Blocked: behind its base branch**. + - **Merge**: **Ready to merge**, or **Ready to merge · still not met: Linked issue** when a rule failed. A pull request that passed but that GitHub or Bitbucket will not merge yet says why instead, such as **Can't merge yet: waiting for a required review**, **Can't merge yet: behind its base branch**, **Can't merge yet: the branch's rules block the merge** or, on Bitbucket, the reason its merge checks give, and **Merge** stays off until it can. Pressing **Merge** follows the same rules as Triage's own merge: it is refused, with the reason, while a reviewer requests changes, a person's review thread is open, or CI has not passed (a pull request with no CI merges only when the policy turns **CI green** off); a GitHub admin can still merge on GitHub. A pull request Triage opened itself is merged rather than approved, so it reads **Ready to merge** too; with no button and a rule failing it reads **Not met: Linked issue**, and when GitHub or Bitbucket holds it with nothing failed, **Blocked:** and why, such as **Blocked: behind its base branch**. Some decisions come before the button. A pull request the author updated after your review reads **Re-review: @ada updated it**, then what is still open, such as **· still not met: Linked issue** or **· still to decide: Focused**, or **· all rules met** when nothing is. A rule you need to decide reads **Decide: Focused (Triage wasn't sure)** when the model was unsure, **Decide: Linked issue (your policy asks you)** when the policy asks you about it when it fails, or **(see why below)** for both. A rule that could not be checked reads **Couldn't check Merge conflicts · use Triage again in More**. After you approve, it says only what is still open or what holds the merge, such as **You approved · still not met: Linked issue** or **You approved · ready to merge**. When an action waits for your approval on a pull request that is otherwise the author's, the line asks about it, such as **Needs you: Approve this pull request?** or **Needs you: Add label bug?**. - **Done:** what the run ended with: the verdict as its label on the pull request says it (**Ready**, **Not ready** or **Needs human review**), what Triage posted on its own, and whose turn it is, such as **Done: Not ready · added label risk: medium · waiting on @ada to fix: Linked issue**. A merged or closed pull request reads **Done: Merged** or **Done: Closed**, a draft with a verdict **Done: Not ready · draft · waiting on @ada**, and once you approve with nothing left for you, **Done: You approved**. **Reply** stays available if you want to add something. A verdict Triage could not settle reads **Done: Needs human review · Triage could not settle it · Triage again**. When the author pushed commits Triage has not checked, the line adds **newer commits not yet checked**, and **Triage again** when nothing in it is yours. A passing pull request is never merged before Triage checks them, so it reads **Done: Ready to merge · newer commits not yet checked · Triage again**. On the author's turn it reads, for example, **Done: Not ready · waiting on @ada · newer commits not yet checked · Triage again**.