Public: anyone can read this; the crew can change it.
Merging a pull refuses with team_review_required while a named team's reviewer has not approved #10096
Check · Review · Land · Live done — Landed and live in production.
scout/forge:scout/10080-merging-a-pull-refuses-with-team-review → main 3 commits fork 1 thread · 1 unresolved
11 files
changed +279
−7
src/actions/pulls.ts +9 −1
| @@ -4,6 +4,7 @@ | ||
| 4 | 4 | * issues — a pull is an issue with a diff. |
| 5 | 5 | */ |
| 6 | 6 | |
| 7 | import { teamsHolding } from "../core/issue_teams.ts"; | |
| 7 | 8 | import { isContributionFork, type GuestRefusal } from "../core/contribute.ts"; |
| 8 | 9 | import { takeGuestPull } from "./guests.ts"; |
| 9 | 10 | import { requireAbility } from "./roles.ts"; |
| @@ -476,7 +477,7 @@ export async function mergePull( | ||
| 476 | 477 | requested: MergeStrategy, |
| 477 | 478 | /** The merge queue's guard: land only onto this base, as this tree (#2036). */ |
| 478 | 479 | expect?: LandingExpect, |
| 479 | /** `pastGate`: the owner's break-glass (`checks/bypass.ts`) skips the checks requirement. */ | |
| 480 | /** `pastGate`: the owner's break-glass (`checks/bypass.ts`) skips the checks and team requirements. */ | |
| 480 | 481 | opts: { readonly pastGate?: boolean } = {}, |
| 481 | 482 | ): Promise< |
| 482 | 483 | Result<{ mergeOid: string; carried: boolean }, AccessError | MergePullError> |
| @@ -550,6 +551,13 @@ export async function mergePull( | ||
| 550 | 551 | db: app.db, |
| 551 | 552 | requireReview: repo.requireReview, |
| 552 | 553 | requireChecks: repo.requireChecks && opts.pastGate !== true, |
| 554 | // A named team with a reviewer here reads this head first (#10080). | |
| 555 | ...(opts.pastGate === true | |
| 556 | ? {} | |
| 557 | : { | |
| 558 | teamsHolding: (headOid: string) => | |
| 559 | teamsHolding(app.db, { ...judged, headOid }).map((v) => v.name), | |
| 560 | }), | |
| 553 | 561 | }, |
| 554 | 562 | strategy, |
| 555 | 563 | ...(expect === undefined ? {} : { expect }), |
src/core/issue_teams.ts +1 −1
| @@ -103,7 +103,7 @@ export const teamsAnswered = (verdicts: readonly TeamVerdict[]): boolean => | ||
| 103 | 103 | * pull's repository and no current approval on its head. A stale approval, |
| 104 | 104 | * or a newer verdict that is not an approval, holds; a team with nobody to |
| 105 | 105 | * ask does not, matching the review's lapse rule (docs/OPERATING.md). |
| 106 | * Nothing gates on it yet (#9930). | |
| 106 | * `mergePull` refuses `team_review_required` while it is non-empty (#10080). | |
| 107 | 107 | */ |
| 108 | 108 | export function teamsHolding( |
| 109 | 109 | db: Database, |
src/core/pulls.ts +11 −1
| @@ -796,6 +796,12 @@ export interface ReviewGate { | ||
| 796 | 796 | * Read live inside the merge like the review verdict. @default false |
| 797 | 797 | */ |
| 798 | 798 | readonly requireChecks?: boolean; |
| 799 | /** | |
| 800 | * The names of the crew teams that still hold this head (#9930, | |
| 801 | * `teamsHolding`), asked with the live head inside the merge. Absent | |
| 802 | * means no team gate: the owner's break-glass passes it. | |
| 803 | */ | |
| 804 | readonly teamsHolding?: (headOid: string) => readonly string[]; | |
| 799 | 805 | } |
| 800 | 806 | |
| 801 | 807 | export interface PullStatus { |
| @@ -1079,7 +1085,9 @@ export type MergePullError = | ||
| 1079 | 1085 | /** The repo requires an approval of this head, or a change was requested. */ |
| 1080 | 1086 | | "review_required" |
| 1081 | 1087 | /** The repo requires a passing deploy of this head, and there is none yet. */ |
| 1082 | | "checks_required"; | |
| 1088 | | "checks_required" | |
| 1089 | /** A team the pull's issue names has a reviewer here and has not approved this head (#10080). */ | |
| 1090 | | { readonly kind: "team_review_required"; readonly teams: readonly string[] }; | |
| 1083 | 1091 | |
| 1084 | 1092 | /** The base a landing must find, and the tree it must write (#2036). */ |
| 1085 | 1093 | export interface LandingExpect { |
| @@ -1175,6 +1183,8 @@ export async function mergePull( | ||
| 1175 | 1183 | } |
| 1176 | 1184 | if (status.reviews !== null && !status.reviews.mergeable) |
| 1177 | 1185 | return err("review_required"); |
| 1186 | const teams = input.gate?.teamsHolding?.(status.headOid) ?? []; | |
| 1187 | if (teams.length > 0) return err({ kind: "team_review_required", teams }); | |
| 1178 | 1188 | // The check is the latest deploy run of *this* head. Read here, not |
| 1179 | 1189 | // handed in, for the same reason as the review verdict above. A landing |
| 1180 | 1190 | // the merge queue gated (`expect`) is not held for a green on an older |
src/core/teams_words.ts +5 −1
| @@ -2,7 +2,7 @@ | ||
| 2 | 2 | * Crew teams' words (#2681): the sentences a person reads about the teams an |
| 3 | 3 | * issue names. One place, so the web merge box, the terminal and the API/MCP |
| 4 | 4 | * say the same thing when a team holds a pull (#9930). Plain text: names are |
| 5 | * escaped where they are rendered, not here. Nothing renders these yet. | |
| 5 | * escaped where they are rendered, not here. | |
| 6 | 6 | */ |
| 7 | 7 | |
| 8 | 8 | /** "Waiting on Design's review", or "…on Design's and Data science's reviews"; "" with no teams. */ |
| @@ -13,3 +13,7 @@ export function waitingOnTeams(names: readonly string[]): string { | ||
| 13 | 13 | const list = `${owned.slice(0, -1).join(", ")} and ${owned[owned.length - 1]}`; |
| 14 | 14 | return `Waiting on ${list} reviews`; |
| 15 | 15 | } |
| 16 | ||
| 17 | /** The merge's refusal while named teams hold a pull (#10080): the web, the API and the MCP say it. */ | |
| 18 | export const teamHoldRefusal = (names: readonly string[]): string => | |
| 19 | `${waitingOnTeams(names)}. Nothing was merged.`; |
src/fleet/verbs.ts +3
| @@ -29,6 +29,7 @@ import { moveLimb, parseSeat, type Seating } from "../core/limb_seats.ts"; | ||
| 29 | 29 | import { getHold } from "../core/holds.ts"; |
| 30 | 30 | import { recordInteraction, removeDismissals, removeMutes } from "../core/interactions.ts"; |
| 31 | 31 | import { getPull } from "../core/pulls.ts"; |
| 32 | import { teamHoldRefusal } from "../core/teams_words.ts"; | |
| 32 | 33 | import { askItemsOf } from "../web/studio/asks.ts"; |
| 33 | 34 | import type { FleetValue } from "../core/fleet_ops.ts"; |
| 34 | 35 | import { findPillar, isStructural, type Pillars } from "../core/pillars.ts"; |
| @@ -438,6 +439,8 @@ export const VERBS: Record<string, Verb> = { | ||
| 438 | 439 | bringUpToDate(app, t.repo, pull, merged.error.files); |
| 439 | 440 | return `it conflicts with main (${merged.error.files.join(", ")}); its agent brings it up to date`; |
| 440 | 441 | } |
| 442 | if (typeof merged.error !== "string" && merged.error.kind === "team_review_required") | |
| 443 | return teamHoldRefusal(merged.error.teams); | |
| 441 | 444 | return typeof merged.error === "string" ? words(merged.error) : merged.error.kind; |
| 442 | 445 | }, |
| 443 | 446 | }, |
src/http/api.ts +4 −1
| @@ -8,6 +8,7 @@ | ||
| 8 | 8 | * back to continue); errors are `{ error, message }`. |
| 9 | 9 | */ |
| 10 | 10 | |
| 11 | import { teamHoldRefusal } from "../core/teams_words.ts"; | |
| 11 | 12 | import { guardedRefusal } from "../crew/stranger.ts"; |
| 12 | 13 | import { MAX_PASS, passOf } from "../actions/box_pass.ts"; |
| 13 | 14 | import { homeTruth } from "../system/home_truth.ts"; |
| @@ -2776,7 +2777,9 @@ export function apiRoutes(router: Router<Handler>): void { | ||
| 2776 | 2777 | if (typeof merged.error !== "string") |
| 2777 | 2778 | return merged.error.kind === "guarded" |
| 2778 | 2779 | ? json({ error: "guarded", message: guardedRefusal(merged.error.paths), paths: merged.error.paths }, 409) |
| 2779 | : json({ error: merged.error.kind }, 409); | |
| 2780 | : merged.error.kind === "team_review_required" | |
| 2781 | ? json({ error: merged.error.kind, message: teamHoldRefusal(merged.error.teams), teams: merged.error.teams }, 409) | |
| 2782 | : json({ error: merged.error.kind }, 409); | |
| 2780 | 2783 | return originAware(merged.error, 409); |
| 2781 | 2784 | } |
| 2782 | 2785 | return json({ merged: true, ...merged.value }); |
src/tui/screens/pull.ts +15 −2
| @@ -26,6 +26,7 @@ | ||
| 26 | 26 | * `n`/`p` walk the unsettled ones while the cursor is inside a thread. |
| 27 | 27 | */ |
| 28 | 28 | |
| 29 | import { teamHoldRefusal } from "../../core/teams_words.ts"; | |
| 29 | 30 | import { crewFilters, guardedRefusal, isStrangerPull, strangerGuard, strangerNext } from "../../crew/stranger.ts"; |
| 30 | 31 | import { RED_ALONE_WAIT, redAloneAt, redMark } from "../../checks/queue.ts"; |
| 31 | 32 | import { |
| @@ -73,7 +74,7 @@ import { resolveOid } from "../../git/merge.ts"; | ||
| 73 | 74 | import { nextOfPull, type Next } from "../../flow/next_actor.ts"; |
| 74 | 75 | import { stepLine, stepOfPull, type PathStep } from "../../flow/path.ts"; |
| 75 | 76 | import { fromKeptFork } from "../../core/landing_kind.ts"; |
| 76 | import { getPull, pullDir, variantSiblings, type Pull } from "../../core/pulls.ts"; | |
| 77 | import { getPull, pullDir, variantSiblings, type MergePullError, type Pull } from "../../core/pulls.ts"; | |
| 77 | 78 | import { voteVariant } from "../../actions/variants.ts"; |
| 78 | 79 | import { getRepoById, type Repo } from "../../core/repos.ts"; |
| 79 | 80 | import { wallOf, wallPlain, type Wall } from "../../contract/policy.ts"; |
| @@ -220,6 +221,18 @@ export function teamLine(v: TeamVerdict): string { | ||
| 220 | 221 | return `${v.name}: ${words} by ${v.by ?? ""}${v.stale ? " (on an older head)" : ""}`; |
| 221 | 222 | } |
| 222 | 223 | |
| 224 | /** | |
| 225 | * Why `m` did not land, in the toast's words: a structured refusal says its | |
| 226 | * own sentence — the same as the page's merge box (#10080) — and any other | |
| 227 | * code is worded by `errorText`. | |
| 228 | */ | |
| 229 | export function mergeRefusal(why: string | MergePullError): string { | |
| 230 | if (typeof why === "string") return errorText(why); | |
| 231 | if (why.kind === "guarded") return guardedRefusal(why.paths); | |
| 232 | if (why.kind === "team_review_required") return teamHoldRefusal(why.teams); | |
| 233 | return errorText(why.kind); | |
| 234 | } | |
| 235 | ||
| 223 | 236 | /** How often a pull's screen re-reads while its check is queued or running. */ |
| 224 | 237 | const CHECK_TICK_MS = 5000; |
| 225 | 238 | |
| @@ -772,7 +785,7 @@ export function pullScreen( | ||
| 772 | 785 | const landed = await mergePull(f.app, f.user, target, row, strategy); |
| 773 | 786 | if (!landed.ok) { |
| 774 | 787 | const why = landed.error; |
| 775 | ctx.toast(typeof why !== "string" && why.kind === "guarded" ? guardedRefusal(why.paths) : errorText(String(why)), "danger"); | |
| 788 | ctx.toast(mergeRefusal(why), "danger"); | |
| 776 | 789 | return; |
| 777 | 790 | } |
| 778 | 791 | await reload(); |
src/web/pages/pull.ts +3
| @@ -85,6 +85,7 @@ import { | ||
| 85 | 85 | type ThreadView, |
| 86 | 86 | } from "../diff-view.ts"; |
| 87 | 87 | import { field, readForm } from "../form.ts"; |
| 88 | import { teamHoldRefusal } from "../../core/teams_words.ts"; | |
| 88 | 89 | import { teamVerdicts, type TeamVerdict } from "../../core/issue_teams.ts"; |
| 89 | 90 | import { html, raw, when, type Html } from "../html.ts"; |
| 90 | 91 | import { flashTo, fragment, page } from "../layout.ts"; |
| @@ -1230,6 +1231,8 @@ export function mergeFailure(error: AccessError | MergePullError): string { | ||
| 1230 | 1231 | return "The base branch moved while merging. Try again."; |
| 1231 | 1232 | if (error.kind === "unsafe") return "That merge was refused as unsafe."; |
| 1232 | 1233 | if (error.kind === "guarded") return guardedRefusal(error.paths); |
| 1234 | if (error.kind === "team_review_required") | |
| 1235 | return teamHoldRefusal(error.teams); | |
| 1233 | 1236 | return "Git refused the merge."; |
| 1234 | 1237 | } |
| 1235 | 1238 | const fromOrigin = originError(error); |
src/web/studio/actions.ts +3
| @@ -25,6 +25,7 @@ import { canRead, getRepo, type Repo } from "../../core/repos.ts"; | ||
| 25 | 25 | import { bringUpToDate } from "../../crew/autonomy.ts"; |
| 26 | 26 | import type { Ctx } from "../../http/context.ts"; |
| 27 | 27 | import { crumbsFor, loadRepo, notFoundPage, pageHead, repoBase, signInFirst } from "../chrome.ts"; |
| 28 | import { teamHoldRefusal } from "../../core/teams_words.ts"; | |
| 28 | 29 | import { guardedRefusal } from "../../crew/stranger.ts"; |
| 29 | 30 | import type { Guarded } from "../../core/pulls.ts"; |
| 30 | 31 | import { backTo, field, readForm, type Form } from "../form.ts"; |
| @@ -40,6 +41,8 @@ function mergeWords(error: unknown): string { | ||
| 40 | 41 | const e = error as { kind: string; message?: string }; |
| 41 | 42 | if (e.kind === "git") return `git said ${e.message ?? "no"}`; |
| 42 | 43 | if (e.kind === "guarded") return guardedRefusal((error as unknown as { paths: readonly Guarded[] }).paths); |
| 44 | if (e.kind === "team_review_required") | |
| 45 | return teamHoldRefusal((error as unknown as { teams: readonly string[] }).teams); | |
| 43 | 46 | return e.kind.replace(/_/g, " "); |
| 44 | 47 | } |
| 45 | 48 | return String(error); |
test/pull-tui-teams.test.ts +55
| @@ -152,3 +152,58 @@ test("a two-team issue shows one team approved and one awaited; no team shows no | ||
| 152 | 152 | tui.stop(); |
| 153 | 153 | } |
| 154 | 154 | }, 60_000); |
| 155 | ||
| 156 | test("m on a pull a team holds toasts the team's sentence, not an object (#10080)", async () => { | |
| 157 | const alice = getUserByName(app.db, "alice")!; | |
| 158 | const r = await createRepo(app, alice, { name: "held", visibility: "public" }); | |
| 159 | if (!r.ok) throw new Error(String(r.error)); | |
| 160 | const repo = r.value; | |
| 161 | const quill = getUserByName(app.db, "quill")!; | |
| 162 | addToCrew(app, alice, repo, "quill"); | |
| 163 | seatAs(app.db, repo.id, quill.id, "reviewer"); | |
| 164 | const seed = await commitToBranch(app, alice, repo, { | |
| 165 | branch: "main", | |
| 166 | message: "main", | |
| 167 | files: [{ path: "code.txt", content: "main" }], | |
| 168 | }); | |
| 169 | if (!seed.ok) throw new Error(JSON.stringify(seed.error)); | |
| 170 | const issue = openIssue(app, alice, repo, { title: "Chart" }); | |
| 171 | if (!issue.ok) throw new Error(issue.error); | |
| 172 | const set = nameTeams(app, alice, repo, issue.value, ["design"]); | |
| 173 | if (!set.ok) throw new Error(set.error); | |
| 174 | const c = await commitToBranch(app, alice, repo, { | |
| 175 | branch: "feat", | |
| 176 | base: "main", | |
| 177 | message: "feat", | |
| 178 | files: [{ path: "feat.md", content: "x\n" }], | |
| 179 | }); | |
| 180 | if (!c.ok) throw new Error(JSON.stringify(c.error)); | |
| 181 | const o = await openPull(app, alice, repo, { | |
| 182 | title: "Pull feat", | |
| 183 | body: `Closes #${issue.value.number}`, | |
| 184 | baseBranch: "main", | |
| 185 | headBranch: "feat", | |
| 186 | }); | |
| 187 | if (!o.ok) throw new Error(String(o.error)); | |
| 188 | ||
| 189 | const { app: tui, session } = createForgeApp( | |
| 190 | app, | |
| 191 | { sshUsername: "alice", remoteAddress: "127.0.0.1", fingerprint: "SHA256:alice", user: alice }, | |
| 192 | { terminal: headlessTerminal(160, 44), exitHooks: false }, | |
| 193 | ); | |
| 194 | tui.start(); | |
| 195 | try { | |
| 196 | tui.ctx.push(pullScreen(session, tui.ctx, local(session, repo), o.value.number)); | |
| 197 | await settleApp(tui); | |
| 198 | for (const key of ["m", "enter"]) { | |
| 199 | tui.dispatch({ type: "key", key, ctrl: false, alt: false, shift: false }); | |
| 200 | await settleApp(tui); | |
| 201 | } | |
| 202 | const text = tui.screen.toText(); | |
| 203 | expect(text).toContain("Waiting on Design's review. Nothing was merged."); | |
| 204 | expect(text).not.toContain("[object Object]"); | |
| 205 | expect(getPull(app.db, repo.id, o.value.number)!.mergedAt).toBeNull(); | |
| 206 | } finally { | |
| 207 | tui.stop(); | |
| 208 | } | |
| 209 | }); |
test/team-merge-gate.test.ts added +170
| @@ -0,0 +1,170 @@ | ||
| 1 | /** | |
| 2 | * A named team's review gates the merge (#10080, #9930): `mergePull` refuses | |
| 3 | * `team_review_required`, with the one sentence, while a team with a crew | |
| 4 | * reviewer here has not approved the head; the owner's break-glass passes. | |
| 5 | */ | |
| 6 | ||
| 7 | import { seatAs } from "./helpers/seat.ts"; | |
| 8 | import { afterAll, beforeAll, describe, expect, test } from "bun:test"; | |
| 9 | import { mkdtemp, rm } from "node:fs/promises"; | |
| 10 | import { tmpdir } from "node:os"; | |
| 11 | import { join } from "node:path"; | |
| 12 | import { addToCrew, createAgent } from "../src/actions/crew.ts"; | |
| 13 | import { nameTeams } from "../src/actions/issue_teams.ts"; | |
| 14 | import { openIssue } from "../src/actions/issues.ts"; | |
| 15 | import { mergePull, openPull } from "../src/actions/pulls.ts"; | |
| 16 | import { commitToBranch, createRepo } from "../src/actions/repos.ts"; | |
| 17 | import { createApp, type App } from "../src/app.ts"; | |
| 18 | import { getPull, type Pull } from "../src/core/pulls.ts"; | |
| 19 | import type { Repo } from "../src/core/repos.ts"; | |
| 20 | import { submitReview } from "../src/core/reviews.ts"; | |
| 21 | import { saveTeam } from "../src/core/teams.ts"; | |
| 22 | import { createUser, getUserByName, type User } from "../src/core/users.ts"; | |
| 23 | import { serve, type Servers } from "../src/http/server.ts"; | |
| 24 | import { mergeFailure } from "../src/web/pages/pull.ts"; | |
| 25 | import { quietLogs } from "./helpers/globals.ts"; | |
| 26 | import { tokenFor } from "./helpers/tokens.ts"; | |
| 27 | ||
| 28 | quietLogs(); | |
| 29 | ||
| 30 | const SENTENCE = "Waiting on Design's review. Nothing was merged."; | |
| 31 | ||
| 32 | describe("a named team's review gates the merge", () => { | |
| 33 | let dataDir = ""; | |
| 34 | let app: App; | |
| 35 | let servers: Servers; | |
| 36 | let alice: User; | |
| 37 | let repo: Repo; | |
| 38 | let n = 0; | |
| 39 | const id = (name: string) => getUserByName(app.db, name)!.id; | |
| 40 | const fresh = (pull: Pull) => getPull(app.db, repo.id, pull.number)!; | |
| 41 | ||
| 42 | const push = async (branch: string) => { | |
| 43 | const c = await commitToBranch(app, alice, repo, { | |
| 44 | branch, | |
| 45 | base: "main", | |
| 46 | message: `${branch} ${++n}`, | |
| 47 | files: [{ path: `${branch}.md`, content: `${n}\n` }], | |
| 48 | }); | |
| 49 | if (!c.ok) throw new Error(JSON.stringify(c.error)); | |
| 50 | }; | |
| 51 | ||
| 52 | /** A pull whose body closes a fresh issue naming `teams`. */ | |
| 53 | const pullNaming = async (teams: string[]): Promise<Pull> => { | |
| 54 | const issue = openIssue(app, alice, repo, { title: "A chart" }); | |
| 55 | if (!issue.ok) throw new Error(issue.error); | |
| 56 | if (teams.length > 0) { | |
| 57 | const set = nameTeams(app, alice, repo, issue.value, teams); | |
| 58 | if (!set.ok) throw new Error(set.error); | |
| 59 | } | |
| 60 | const branch = `g${++n}`; | |
| 61 | await push(branch); | |
| 62 | const o = await openPull(app, alice, repo, { | |
| 63 | title: branch, | |
| 64 | body: `Closes #${issue.value.number}`, | |
| 65 | baseBranch: "main", | |
| 66 | headBranch: branch, | |
| 67 | }); | |
| 68 | if (!o.ok) throw new Error(String(o.error)); | |
| 69 | return o.value; | |
| 70 | }; | |
| 71 | const approve = (pull: Pull, by: string) => | |
| 72 | submitReview(app.db, { issueId: pull.id, authorId: id(by), state: "approved", commitOid: fresh(pull).headOid }); | |
| 73 | const land = (pull: Pull, opts: { pastGate?: boolean } = {}) => | |
| 74 | mergePull(app, alice, repo, fresh(pull), "merge", undefined, opts); | |
| 75 | ||
| 76 | beforeAll(async () => { | |
| 77 | dataDir = await mkdtemp(join(tmpdir(), "forge-team-merge-gate-")); | |
| 78 | app = createApp({ dataDir, port: 0 }); | |
| 79 | const made = await createUser(app.db, { | |
| 80 | origin: "seed", | |
| 81 | trusted: true, | |
| 82 | username: "alice", | |
| 83 | email: "[email protected]", | |
| 84 | password: "a-good-passphrase", | |
| 85 | adminIfFirst: true, | |
| 86 | }); | |
| 87 | if (!made.ok) throw new Error(made.error); | |
| 88 | alice = made.value; | |
| 89 | const r = await createRepo(app, alice, { name: "garden", visibility: "private" }); | |
| 90 | if (!r.ok) throw new Error(String(r.error)); | |
| 91 | repo = r.value; | |
| 92 | const seeded = await commitToBranch(app, alice, repo, { | |
| 93 | message: "seed", | |
| 94 | files: [{ path: "README.md", content: "# garden\n" }], | |
| 95 | }); | |
| 96 | if (!seeded.ok) throw new Error(JSON.stringify(seeded.error)); | |
| 97 | for (const name of ["quill", "moss"]) { | |
| 98 | const agent = await createAgent(app, alice, { username: name }); | |
| 99 | if (!agent.ok) throw new Error(String(agent.error)); | |
| 100 | addToCrew(app, alice, repo, name); | |
| 101 | } | |
| 102 | // quill reviews here; moss is on the crew but not as a reviewer. | |
| 103 | seatAs(app.db, repo.id, id("quill"), "reviewer"); | |
| 104 | saveTeam(app.db, "design", { charter: "", dial: 0, members: [id("quill")] }); | |
| 105 | saveTeam(app.db, "security", { charter: "", dial: 0, members: [id("moss")] }); | |
| 106 | servers = await serve(app); | |
| 107 | }, 120_000); | |
| 108 | ||
| 109 | afterAll(async () => { | |
| 110 | servers?.stop(); | |
| 111 | for (let i = 0; i < 20; i++) await new Promise((done) => setImmediate(done)); | |
| 112 | await Bun.sleep(100); | |
| 113 | app?.close(); | |
| 114 | if (dataDir !== "") await rm(dataDir, { recursive: true, force: true, maxRetries: 5, retryDelay: 50 }); | |
| 115 | }, 30_000); | |
| 116 | ||
| 117 | test("held while the team is awaited, with the one sentence", async () => { | |
| 118 | const pull = await pullNaming(["design"]); | |
| 119 | const held = await land(pull); | |
| 120 | expect(held.ok).toBe(false); | |
| 121 | if (held.ok) return; | |
| 122 | expect(held.error).toEqual({ kind: "team_review_required", teams: ["Design"] }); | |
| 123 | // The MCP merge answers in these words, the page's. | |
| 124 | expect(mergeFailure(held.error)).toBe(SENTENCE); | |
| 125 | expect(fresh(pull).mergedAt).toBeNull(); | |
| 126 | }); | |
| 127 | ||
| 128 | test("the API answers team_review_required and the sentence", async () => { | |
| 129 | const pull = await pullNaming(["design"]); | |
| 130 | const res = await fetch(`${servers.url}/api/v1/repos/alice/garden/pulls/${pull.number}/merge`, { | |
| 131 | method: "POST", | |
| 132 | headers: { authorization: `Bearer ${tokenFor(app, "alice", "rw")}`, "content-type": "application/json" }, | |
| 133 | body: "{}", | |
| 134 | }); | |
| 135 | expect(res.status).toBe(409); | |
| 136 | expect(await res.json()).toEqual({ error: "team_review_required", message: SENTENCE, teams: ["Design"] }); | |
| 137 | expect(fresh(pull).mergedAt).toBeNull(); | |
| 138 | }); | |
| 139 | ||
| 140 | test("lands once the team approves the head", async () => { | |
| 141 | const pull = await pullNaming(["design"]); | |
| 142 | approve(pull, "quill"); | |
| 143 | const landed = await land(pull); | |
| 144 | expect(landed.ok).toBe(true); | |
| 145 | expect(fresh(pull).mergedAt).not.toBeNull(); | |
| 146 | }); | |
| 147 | ||
| 148 | test("an approval of an older head holds", async () => { | |
| 149 | const pull = await pullNaming(["design"]); | |
| 150 | approve(pull, "quill"); | |
| 151 | await push(pull.headBranch); | |
| 152 | const held = await land(pull); | |
| 153 | expect(held.ok).toBe(false); | |
| 154 | if (!held.ok) expect(held.error).toEqual({ kind: "team_review_required", teams: ["Design"] }); | |
| 155 | expect(fresh(pull).mergedAt).toBeNull(); | |
| 156 | }); | |
| 157 | ||
| 158 | test("a team with no reviewer here does not hold", async () => { | |
| 159 | const pull = await pullNaming(["security"]); | |
| 160 | const landed = await land(pull); | |
| 161 | expect(landed.ok).toBe(true); | |
| 162 | }); | |
| 163 | ||
| 164 | test("the owner's break-glass passes", async () => { | |
| 165 | const pull = await pullNaming(["design"]); | |
| 166 | const landed = await land(pull, { pastGate: true }); | |
| 167 | expect(landed.ok).toBe(true); | |
| 168 | expect(fresh(pull).mergedAt).not.toBeNull(); | |
| 169 | }); | |
| 170 | }); |
Same refusal must reach the terminal: src/tui/screens/pull.ts:775 stringifies object errors other than guarded, which gives '[object Object]' for team_review_required. Handle it there with teamHoldRefusal and test it.