diff --git a/modules/gitea/client.ts b/modules/gitea/client.ts index 0782397a..bb8de0d2 100644 --- a/modules/gitea/client.ts +++ b/modules/gitea/client.ts @@ -765,6 +765,27 @@ export class GiteaClient { return this.requestText(`${repoPath(owner, repo)}/pulls/${index}.diff`); } + /** **Whether a pull request carries an approving review at a head** (novox/hq ADR 0382 §6): a review of the forge that + * approves, is not dismissed and was made at that very head; or the mesh's ready mark naming that head (issue 722), which is + * how an agent's approval is read where the forge refuses an agent's approve. Reads only. */ + async approvedAt(owner: string, repo: string, index: number, head: string, authors?: string[]): Promise { + indexOf(index); + const want = head.toLowerCase(); + for (let page = 1; page <= 20; page++) { + const raw = await this.request(`${repoPath(owner, repo)}/pulls/${index}/reviews?page=${page}&limit=50`); + for (const r of raw ?? []) { + if (String(r?.state ?? "").toUpperCase() === "APPROVED" && !r?.dismissed && !r?.stale && + String(r?.commit_id ?? "").toLowerCase() === want) return true; + } + if (!raw || raw.length < 50) break; + } + const labels = await this.request(`${repoPath(owner, repo)}/issues/${index}/labels`); + const label = (labels ?? []).find((l) => l?.name === READY_LABEL); + if (!label) return false; + const ready = readyMarkOf(Number(label.id ?? 0), await this.timeline(owner, repo, index), authors ?? readyAuthors()); + return !!ready && ready.head.toLowerCase() === want; + } + /** **Every event of an issue's or pull request's timeline**, oldest first, page by page (novox/hq issue 722): a long * thread hides nothing. Bounded at 100 pages of 50; a longer one is said, and its first 5000 events read. */ async timeline(owner: string, repo: string, index: number): Promise { diff --git a/modules/gitea/index.ts b/modules/gitea/index.ts index c730bc2a..4c444060 100644 --- a/modules/gitea/index.ts +++ b/modules/gitea/index.ts @@ -45,6 +45,7 @@ import { CHECK_CONTEXT, REPO_CHECK_CONTEXT, commentFor, headsToAnnounce, statuse repoCheckFacts, reposToLook } from "./pulls.js"; import { GateWriter, clonedAs, type BuildStarted } from "./gate.js"; import { HOOK_PATH, HOOK_PORT, Pace, claimHead, ensureSystemHook, eventsTool, keptSecret, receiver, type ForgeEvent } from "./webhook.js"; +import { keepSiblings, noteVerdict, readSiblings, siblingsFile, siblingsOf, weighSiblings } from "./siblings.js"; import { announcementOf, keepTook, lateVerdictComment, outOfDateWords, sameCommit, tookFile, weighTrunks } from "./trunk.js"; // Without a way to a token — configured, or mintable with the admin account (token.ts) — there is @@ -136,6 +137,7 @@ async function pollMerged(client: GiteaClient): Promise { await emit("pull.closed", { owner: repo.owner, repo: repo.name, number: pull.number, title: pull.title, head: pull.head, head_sha: pull.head_sha, base: pull.base, html_url: pull.html_url }); console.log(`[gitea] announced ${repo.full_name}#${pull.number} closed unmerged`); + await siblingMoves(client, repo.full_name, pull.number); } announced.add(closedKey); changed = true; @@ -218,6 +220,8 @@ async function announceMerge(client: GiteaClient, repo: RepoOf, pull: PullOf): P // Said, because a trigger that fires silently is indistinguishable from one that did not // fire (novox/hq 04-ISSUES/131) — this line is how an operator knows the mesh was told. console.log(`[gitea] announced merge ${repo.full_name}#${pull.number} (${pull.merge_commit_sha.slice(0, 8)}) into ${pull.base}`); + // Its own merge of the head that was read is no sibling's move; any other is (ADR 0382 §5). + await siblingMoves(client, repo.full_name, pull.number); } catch (err) { announced.delete(pull.merge_commit_sha); throw err; @@ -264,6 +268,8 @@ async function announceHead(client: GiteaClient, repo: RepoOf, pull: OpenOf): Pr throw err; } keepHeads(); + // A sibling's new head puts out of date every verdict that read the sibling at another head (ADR 0382 §5). + await siblingMoves(client, repo.full_name, pull.number); // Said, so an operator knows the mesh was asked to check it. console.log(`[gitea] announced ${key} at ${sha.slice(0, 8)} to be checked before it merges`); // Pending until the verdict comes, so the pull request says a check is running rather than nothing — and @@ -341,7 +347,22 @@ function keepTrunks(): void { /** Whether a trunk of one repository moved, weighed with this module's forge and its one writer (trunk.ts). */ function trunksMoved(client: GiteaClient, owner: string, name: string): Promise { - return weighTrunks(client, gates, trunks, owner, name); + return weighTrunks(client, gates, trunks, owner, name, () => siblingMoves(client, `${owner}/${name}`)); +} + +// **The siblings each open pull request's latest verdict names** (novox/hq ADR 0382 §5, siblings.ts), by member, kept beside +// the other records. A sibling's new head, close unmerged, merge, or a move of its base puts the merge check of every +// member whose verdict names it back to pending, with the reason; nothing is run for it. +const siblingsRecord = siblingsFile(process.env.MESH_GITEA_STATE_DIR); +const siblingNamed = readSiblings(siblingsRecord); + +/** A sibling moved, weighed (ADR 0382 §5): never fatal to what heard it. */ +async function siblingMoves(client: GiteaClient, repository: string, number?: number): Promise { + try { + if (await weighSiblings(client, gates, siblingNamed, repository, number)) keepSiblings(siblingsRecord, siblingNamed); + } catch (err) { + console.error(`[gitea] whether a sibling of ${repository}${number ? "#" + number : ""} moved could not be weighed — ${err instanceof Error ? err.message : err}`); + } } // **The verdict, set where the pull request shows it.** Every verdict is the head commit's status; one @@ -363,6 +384,17 @@ async function setVerdict(client: GiteaClient, event: { body: unknown }): Promis console.error("[gitea] a merge check's verdict named no repository, commit or verdict; ignored"); return; } + // The siblings this verdict read are kept for the member (ADR 0382 §5); a verdict that read none removes the record. + const siblings = siblingsOf(c.siblings); + if (c.number && noteVerdict(siblingNamed, `${c.owner}/${c.repo}#${c.number}`.toLowerCase(), c.commit, siblings)) { + keepSiblings(siblingsRecord, siblingNamed); + } + // A sibling that moved while the check ran is weighed once this verdict has landed, in the head's own turn after it. + if (siblings.length) { + for (const repository of new Set(siblings.map((s) => s.repository))) { + void Promise.resolve().then(() => siblingMoves(client, repository)); + } + } // Each status links to the pull request, where the delivery's view is kept (novox/hq ADR 0239). let target: string | undefined; let base: string | undefined; @@ -445,6 +477,11 @@ async function buildStarted(event: { body: unknown }): Promise { async function actOn(client: GiteaClient, event: ForgeEvent): Promise { const b = event.body as { owner?: string; repo?: string; number?: number }; if (!b.owner || !b.repo || !b.number) return; + if (event.name === "forge.pull-request.closed") { + // Closed unmerged is a sibling's move; a merge is weighed with its announcement below (ADR 0382 §5). + if (!(event.body as { merged?: boolean }).merged) await siblingMoves(client, `${b.owner}/${b.repo}`, b.number); + return; + } const head = ["forge.pull-request.opened", "forge.pull-request.synchronized", "forge.pull-request.reopened"].includes(event.name); const merged = event.name === "forge.pull-request.merged"; if (!head && !merged) return; diff --git a/modules/gitea/merge.ts b/modules/gitea/merge.ts index 9c8e2570..fc70b6ef 100644 --- a/modules/gitea/merge.ts +++ b/modules/gitea/merge.ts @@ -11,7 +11,8 @@ import type { CommitStatus, GiteaClient } from "./client.js"; import { mergeableUnder, newestByContext } from "./checks.js"; import { MERGE_METHODS, Refused, checkName } from "./guard.js"; import { CHECK_CONTEXT, REPO_CHECK_CONTEXT } from "./pulls.js"; -import { announcementOf, askedWords, outOfDateWords, tookFile, tookWords, twelve, verdictAtTrunk } from "./trunk.js"; +import { groupRefusal, memberKey, namesSiblings, readSiblings, siblingOutOfDate, siblingsFile } from "./siblings.js"; +import { announcementOf, askedWords, outOfDateWords, sameCommit, tookFile, tookWords, twelve, verdictAtTrunk } from "./trunk.js"; /** A head named for a merge: at least 12 hex characters of its sha (novox/hq issue 381), up to a sha-256 commit's 64. */ const REVIEWED_HEAD = /^[0-9a-f]{12,64}$/; @@ -97,7 +98,11 @@ export function mergeTool(gitea: GiteaClient, given: MergeDeps): ToolDefinition const checks = await gitea.headChecks(owner, repo, pull.head_sha); const at = verdictAtTrunk(checks, trunkHead); if (!at.current) { - const judged = at.judged ? `was judged against trunk ${twelve(at.judged)}` : "names no trunk commit it was judged against"; + // A verdict put out of date by a sibling's move says so in its own words (ADR 0382 §5); it is asked again as a + // trunk's move is, and a failed ask puts those words back, not the trunk's. + const siblingWords = siblingOutOfDate(checks[CHECK_CONTEXT]?.description) ? checks[CHECK_CONTEXT]?.description : undefined; + const judged = siblingWords ? `was put out of date by a sibling's move (${siblingWords})` + : at.judged ? `was judged against trunk ${twelve(at.judged)}` : "names no trunk commit it was judged against"; const took = tookWords(tookFile(deps.stateDir), `${owner}/${repo}`); const head = twelve(pull.head_sha), now = twelve(trunkHead); if (at.onItsWay) { @@ -123,7 +128,7 @@ export function mergeTool(gitea: GiteaClient, given: MergeDeps): ToolDefinition const cloneURL = (await gitea.getRepo(owner, repo)).clone_url; await deps.emit("pull.updated", await announcementOf(gitea, owner, repo, cloneURL, pull)); } catch (err) { - await say(outOfDateWords(at.judged, trunkHead)).catch(() => {}); + await say(siblingWords ?? outOfDateWords(at.judged, trunkHead)).catch(() => {}); throw new Error(`${owner}/${repo}#${number} is not merged: its verdict ${judged}, and ${pull.base} is now ` + `${now}, and its check could not be asked: ${err instanceof Error ? err.message : err}. Nothing was merged`); } @@ -139,6 +144,25 @@ export function mergeTool(gitea: GiteaClient, given: MergeDeps): ToolDefinition throw new Refused(`${owner}/${repo}#${number} is not merged: ${verdict.says}. A pull request merges once ` + `its checks succeed; no tool merges past them (novox/hq ADR 0266)`); } + // **A member judged beside its siblings merges only with its group passed and reviewed** (novox/hq ADR 0382 §6): the + // siblings its verdict names are the holder's record (siblings.ts); a status that says it read siblings with no record + // of them is no verdict to merge on, never read as none. + const heard = readSiblings(siblingsFile(deps.stateDir))[memberKey(owner, repo, number)]; + const siblings = heard && sameCommit(heard.head, pull.head_sha) ? heard.siblings : []; + if (!siblings.length && namesSiblings(statuses.find((s) => s.context === CHECK_CONTEXT)?.description)) { + throw new Refused(`${owner}/${repo}#${number} is not merged: its verdict read siblings, and which ones is not held ` + + `here, so the group cannot be weighed. Ask its check again at ${twelve(pull.head_sha)}. Nothing was merged`); + } + if (siblings.length) { + const missing = await groupRefusal({ headChecks: (o, r, sha) => gitea.headChecks(o, r, sha), + approvedAt: (o, r, n, h) => gitea.approvedAt(o, r, n, h) }, + { owner, repo, number, head: String(pull.head_sha) }, siblings); + if (missing) { + throw new Refused(`${owner}/${repo}#${number} is not merged: its verdict was judged beside ${siblings.length} ` + + `sibling(s), and ${missing}. A member judged beside its siblings merges once its group passed at the heads that ` + + `verdict read and each of them is approved there (novox/hq ADR 0382). Nothing was merged`); + } + } await gitea.mergePullRequest(owner, repo, number, method, deleteBranch, pull.head_sha); // The merge commit only exists now; answered so the caller can follow what is built from it. // pull.merged is the events entrypoint's to announce (index.ts), once, within its poll. diff --git a/modules/gitea/pulls.ts b/modules/gitea/pulls.ts index 2c3a549d..c6c92667 100644 --- a/modules/gitea/pulls.ts +++ b/modules/gitea/pulls.ts @@ -11,6 +11,7 @@ import type { CommitStatus, GiteaClient, GiteaPull, GiteaRepo } from "./client.js"; import { movedSince } from "./client.js"; +import { UNKNOWN_MARK, besideWords, siblingsOf } from "./siblings.js"; /** The status context a merge check's gate is kept under: one per commit, the newest replacing the last. */ export const CHECK_CONTEXT = "mesh/merge-gate"; @@ -65,6 +66,13 @@ export interface Checked { trunk?: string; beside?: Record; took?: string; + /** The other members of its delivery group the check read beside the head, each at its head merged onto its base + * commit (novox/hq ADR 0382 §4); absent when it read none, and from a controller from before the rule. */ + siblings?: { repository: string; number: number; head: string; base: string; base_commit: string }[]; + /** Why the siblings could not be learned (mesh-delivery did not answer), when that is so. */ + siblings_unknown?: string; + /** Where the judge came from, when it came from the group (ADR 0382 §3): said in the account. */ + judge_from?: string; } /** A change plan, as the controller says it (mesh-controller internal/link, ChangePlan). */ @@ -163,7 +171,7 @@ function described(verdict: string, summary: string, modules?: string[], depende return clipped(d); } -function clipped(d: string): string { +export function clipped(d: string): string { d = d.replace(/\s+/g, " ").trim(); return d.length > 140 ? d.slice(0, 139) + "…" : d; } @@ -238,8 +246,12 @@ export function statusesFor(c: Checked, facts: RepoCheckFacts = {}): CommitStatu * 7a8b9c0d1e2f — pass: …", so the forge's 140 characters never clip it away; the trunk's move is weighed by it. A * verdict that names no trunk commit is said as before. */ function judgedFirst(c: Checked, status: CommitStatus): CommitStatus { - if (!c.trunk) return status; - return { ...status, description: clipped(`head ${c.commit.slice(0, 12)} merged into trunk ${c.trunk.slice(0, 12)} — ${status.description}`) }; + // The siblings it read, named after the commits (ADR 0382 §4), so the clip never takes them either. + const siblings = siblingsOf(c.siblings); + // A verdict that could not learn them says so on the status too (clause 1: it is still checked, and merges). + const mark = siblings.length ? besideWords(siblings) : c.siblings_unknown ? UNKNOWN_MARK : ""; + if (!c.trunk) return mark ? { ...status, description: clipped(`${mark} — ${status.description}`) } : status; + return { ...status, description: clipped(`head ${c.commit.slice(0, 12)} merged into trunk ${c.trunk.slice(0, 12)}${mark ? " " + mark : ""} — ${status.description}`) }; } /** The comment a verdict leaves on its pull request, with the check's own account: the change plan of a @@ -258,6 +270,14 @@ export function commentFor(c: Checked, facts: RepoCheckFacts = {}): string | nul lines.splice(1, 0, `Judged: head \`${c.commit.slice(0, 12)}\` merged into trunk \`${c.trunk.slice(0, 12)}\`` + (beside ? `; read beside it: ${beside}` : "")); } + const siblings = siblingsOf(c.siblings); + if (siblings.length) { + lines.splice(c.trunk ? 2 : 1, 0, `Siblings read beside it: ` + siblings.map((s) => + `${s.repository}#${s.number} \`${s.head.slice(0, 12)}\` merged onto \`${s.base_commit.slice(0, 12)}\` (${s.base})`).join(", ") + + (c.judge_from ? `; judged by ${c.judge_from}` : "")); + } else if (c.siblings_unknown) { + lines.splice(c.trunk ? 2 : 1, 0, `Siblings could not be learned: ${c.siblings_unknown}`); + } lines.push(`- \`${CHECK_CONTEXT}\`: **${saidAs(gate.verdict).toUpperCase()}** — ${gate.summary}` + (gate.modules?.length ? ` (modules: ${gate.modules.join(", ")}` + (gate.dependents?.length ? `; built after them: ${gate.dependents.join(", ")}` : "") + ")" : "")); diff --git a/modules/gitea/siblings.ts b/modules/gitea/siblings.ts new file mode 100644 index 00000000..64d05aad --- /dev/null +++ b/modules/gitea/siblings.ts @@ -0,0 +1,286 @@ +// **A delivery group's member is checked beside the other members' heads, and a sibling's move puts that verdict out of +// date** (novox/hq ADR 0382 §5, §6; issue 867). +// +// The controller's verdict (`checked`) names the siblings its check read: each as the sibling's repository, pull +// request, head, base branch and the base commit the sibling's head was merged onto. This module, the forge's holder, +// keeps them per open pull request, beside its other records, and: +// +// - on its own hearing of a sibling's new head, of its close unmerged, or of a move of its base, sets +// mesh/merge-gate and mesh/repo-check of every member whose verdict names that sibling back to pending, with the +// reason. Nothing is run for it. The one move that does not count is the sibling's own merge of the head that was +// read onto the base commit that was read (in either order of the two merges); +// - lets the merge tool refuse a verdict that read siblings unless mesh/delivery-group passed at the very heads it +// read, and every sibling head it read carries an approving review at that head (merge.ts). +// +// A verdict without siblings (a controller from before ADR 0382) is none of this: nothing is kept and nothing changes. +// Pure functions and one small file of the module's own state, so this is tested without a forge. + +import { existsSync, mkdirSync, readFileSync, renameSync, writeFileSync } from "node:fs"; +import { dirname } from "node:path"; +import type { CommitStatus, GiteaPull, HeadCheck } from "./client.js"; +import { CHECK_CONTEXT, REPO_CHECK_CONTEXT, clipped } from "./pulls.js"; +import { sameCommit, twelve, type OutOfDateWriter } from "./trunk.js"; + +/** The status the delivery group's owner sets on each member's head (novox/hq ADR 0239). */ +export const GROUP_CONTEXT = "mesh/delivery-group"; + +/** One sibling a verdict read, as the controller names it (contract of ADR 0382 §4). */ +export interface SiblingNamed { + repository: string; + number: number; + head: string; + base: string; + base_commit: string; + /** Set here, never by the controller: the sibling merged its head onto its base commit read, so it is no sibling's + * move any more. */ + merged?: boolean; +} + +/** What is kept of one member's latest verdict that read siblings: the head it was of and the siblings it named. */ +export interface SiblingEntry { + head: string; + siblings: SiblingNamed[]; +} + +/** The records, by `owner/repo#number` of the member. */ +export type SiblingRecord = Record; + +/** The member's key. */ +export function memberKey(owner: string, repo: string, number: number): string { + return `${owner}/${repo}#${number}`.toLowerCase(); +} + +/** The siblings a verdict names, trimmed to what is kept; none when it names none or names one badly. */ +export function siblingsOf(named: unknown): SiblingNamed[] { + if (!Array.isArray(named)) return []; + const out: SiblingNamed[] = []; + for (const s of named) { + const r = s as Record; + const repository = String(r?.repository ?? ""), head = String(r?.head ?? ""), base = String(r?.base ?? ""); + const number = Number(r?.number); + if (!repository || !head || !base || !Number.isInteger(number) || number <= 0) continue; + out.push({ repository, number, head, base, base_commit: String(r?.base_commit ?? "") }); + } + return out; +} + +/** The siblings as a status names them: "mesh-host#114@abcdef123456, …". */ +export function siblingWords(siblings: SiblingNamed[]): string { + return siblings.map((s) => `${s.repository.split("/").pop()}#${s.number}@${twelve(s.head)}`).join(", "); +} + +/** What a status of a verdict that read siblings carries, after the commits it judged. */ +export function besideWords(siblings: SiblingNamed[]): string { + // Short, so that the verdict word and the start of its reason survive the forge's 140 characters: the first sibling + // by its pull request and 8 characters of its head, and how many more. The whole list is the holder's record and the comment's. + const first = siblings[0]; + const more = siblings.length > 1 ? ` +${siblings.length - 1} more` : ""; + return `beside ${first.repository.split("/").pop()}#${first.number}@${twelve(first.head).slice(0, 8)}${more}`; +} + +/** The mark a status carries when the siblings could not be learned (ADR 0382 §1): said, never silent. */ +export const UNKNOWN_MARK = "siblings unknown"; + +/** Whether a status names that its verdict read siblings (so that a record lost does not read as none). */ +export function namesSiblings(description: string | undefined): boolean { + return / beside [\w.-]+#\d+@[0-9a-f]{8}\b/.test(description ?? ""); +} + +/** Whether a status is a verdict put out of date by a sibling's move. */ +export function siblingOutOfDate(description: string | undefined): boolean { + return /^out of date: judged beside /.test(description ?? ""); +} + +// --- the record on disk ---------------------------------------------------------------------------------------- + +/** The file the holder keeps the siblings its verdicts name in, beside its other records. */ +export function siblingsFile(stateDir: string | undefined): string | null { + return stateDir ? `${stateDir.replace(/\/+$/, "")}/siblings-named.json` : null; +} + +export function readSiblings(file: string | null): SiblingRecord { + if (!file || !existsSync(file)) return {}; + try { + return (JSON.parse(readFileSync(file, "utf8")) as { members?: SiblingRecord }).members ?? {}; + } catch { + return {}; + } +} + +export function keepSiblings(file: string | null, record: SiblingRecord): void { + if (!file) return; + try { + mkdirSync(dirname(file), { recursive: true }); + writeFileSync(file + ".tmp", JSON.stringify({ members: record })); + renameSync(file + ".tmp", file); + } catch (err) { + console.error(`[gitea] the siblings a verdict named could not be kept — ${err instanceof Error ? err.message : err}`); + } +} + +/** A member's latest verdict, kept: its siblings when it read some, else the entry removed. Answers whether it changed. */ +export function noteVerdict(record: SiblingRecord, key: string, head: string, siblings: SiblingNamed[]): boolean { + if (siblings.length === 0) { + if (!(key in record)) return false; + delete record[key]; + return true; + } + record[key] = { head, siblings }; + return true; +} + +// --- what a sibling's move is --------------------------------------------------------------------------------- + +/** What weighing a sibling reads of the forge. */ +export interface SiblingForge { + getPullRequest(owner: string, repo: string, index: number): Promise; + branchHead(owner: string, repo: string, branch: string): Promise; + commitParents(owner: string, repo: string, sha: string): Promise; +} + +/** The reason a verdict that named this sibling is out of date, from what the forge holds now, or nothing; and whether + * the sibling has merged its head read onto the base commit read, which ends it as a sibling. */ +export async function siblingMoved(forge: SiblingForge, s: SiblingNamed): Promise<{ reason?: string; merged?: boolean }> { + const [owner, repo] = s.repository.split("/"); + const who = `${repo}#${s.number}`; + const pull = await forge.getPullRequest(owner, repo, s.number); + if (pull.merged) { + // Its own merge of the head that was read onto the base commit that was read: the head the merge joined is the head + // read, and the commit it was made onto is the base commit read. Any other merge is a move of the base. + const parents = pull.merge_commit_sha ? await forge.commitParents(owner, repo, pull.merge_commit_sha).catch(() => []) : []; + const own = sameCommit(pull.head_sha, s.head) && parents.length >= 1 && sameCommit(parents[0], s.base_commit) && + (parents.length === 1 || sameCommit(parents[1], s.head)); + if (own) return { merged: true }; + return { reason: `${who} was merged otherwise than its head ${twelve(s.head)} onto ${twelve(s.base_commit)}` }; + } + if (pull.state !== "open") return { reason: `${who} was closed unmerged` }; + if (!sameCommit(pull.head_sha, s.head)) { + return { reason: `${who} at ${twelve(s.head)}, it is now at ${twelve(pull.head_sha)}` }; + } + const base = await forge.branchHead(owner, repo, s.base); + if (!sameCommit(base, s.base_commit)) { + return { reason: `${who} merged onto ${twelve(s.base_commit)}, its ${s.base} is now ${twelve(base)}` }; + } + return {}; +} + +/** A verdict out of date because a sibling moved, as its statuses say it (ADR 0382 §5; the wording of ADR 0323 §4). */ +export function siblingOutOfDateWords(reason: string): string { + return clipped(`out of date: judged beside ${reason}; judged again on a recheck or when it is merged`); +} + +/** The statuses a sibling's move puts back to pending on one member's head, from what the forge holds on it now: none + * when it has no merge check, or its gate is pending (a check on its way, or already out of date). */ +export function siblingStatuses(checks: Record, reason: string): CommitStatus[] { + const gate = checks[CHECK_CONTEXT]; + if (!gate || gate.state === "pending") return []; + const description = siblingOutOfDateWords(reason); + const out: CommitStatus[] = [{ state: "pending", context: CHECK_CONTEXT, description }]; + if (checks[REPO_CHECK_CONTEXT]) out.push({ state: "pending", context: REPO_CHECK_CONTEXT, description }); + return out; +} + +/** What weighing a sibling's move writes with, beside the forge's reads. */ +export interface SiblingWeighing extends SiblingForge {} + +/** + * A sibling moved (a new head, a close, a base move, a merge): every member whose latest verdict names it has its + * merge check put back to pending with the reason. `repository` and, when the event names one, `number` say which + * sibling. A member whose head is no longer the one its verdict was of is dropped from the record: its new head is + * checked afresh. Nothing is run. Answers whether the record changed. A read that fails leaves that member for the next + * move to weigh, and is said. + */ +export async function weighSiblings(forge: SiblingWeighing, gates: OutOfDateWriter, record: SiblingRecord, + repository: string, number?: number): Promise { + let changed = false; + const repo = repository.toLowerCase(); + for (const [key, entry] of Object.entries(record)) { + const [path] = key.split("#"); + const [owner, name] = path.split("/"); + const memberNumber = Number(key.split("#")[1]); + // A member that is merged or no longer open has no verdict to put out of date: its entry goes (the record must not + // grow without bound, and a merged head is never re-pended by a sibling's later push). Weighed when the event is the + // member's own, or names one of its siblings. + const own = path === repo && (number === undefined || number === memberNumber); + if (own || entry.siblings.some((s) => !s.merged && s.repository.toLowerCase() === repo && (number === undefined || s.number === number))) { + try { + const member = await forge.getPullRequest(owner, name, memberNumber); + if (member.merged || member.state !== "open") { + delete record[key]; + changed = true; + continue; + } + } catch (err) { + console.error(`[gitea] ${key}: whether it is still open could not be read — ${err instanceof Error ? err.message : err}`); + } + } + for (const s of entry.siblings) { + if (s.merged || s.repository.toLowerCase() !== repo || (number !== undefined && s.number !== number)) continue; + try { + const moved = await siblingMoved(forge, s); + if (moved.merged) { + s.merged = true; + changed = true; + continue; + } + if (!moved.reason) continue; + const member = await forge.getPullRequest(owner, name, memberNumber); + if (!sameCommit(member.head_sha, entry.head)) { + delete record[key]; + changed = true; + break; + } + const written = await gates.outOfDate(owner, name, entry.head, (checks) => siblingStatuses(checks, moved.reason!), + member.html_url || undefined); + if (written.length) { + console.log(`[gitea] ${key}: its verdict was judged beside ${moved.reason}: out of date, nothing run (novox/hq ADR 0382)`); + } + break; + } catch (err) { + console.error(`[gitea] ${key}: whether ${s.repository}#${s.number} moved could not be weighed, so it is weighed again ` + + `on its next move — ${err instanceof Error ? err.message : err}`); + } + } + } + return changed; +} + +// --- the merge tool's rule (ADR 0382 §6) ---------------------------------------------------------------------- + +/** What the rule reads beside the pull request being merged. */ +export interface GroupForge { + headChecks(owner: string, repo: string, sha: string): Promise>; + /** Whether the sibling's head carries an approving review at that head. */ + approvedAt(owner: string, repo: string, number: number, head: string): Promise; +} + +/** + * What stops a pull request whose verdict read siblings from merging, in words, or nothing: mesh/delivery-group must be + * a success at its own head and at every sibling head the verdict read, and every sibling head read must carry an + * approving review at that head. Every missing thing is named. + */ +export async function groupRefusal(forge: GroupForge, self: { owner: string; repo: string; number: number; head: string }, + siblings: SiblingNamed[]): Promise { + const missing: string[] = []; + const members = [{ repository: `${self.owner}/${self.repo}`, number: self.number, head: self.head, sibling: false }, + ...siblings.map((s) => ({ repository: s.repository, number: s.number, head: s.head, sibling: true }))]; + for (const m of members) { + const [owner, repo] = m.repository.split("/"); + const at = `${repo}#${m.number}@${twelve(m.head)}`; + let group: HeadCheck | undefined; + try { + group = (await forge.headChecks(owner, repo, m.head))[GROUP_CONTEXT]; + } catch (err) { + missing.push(`${GROUP_CONTEXT} at ${at} could not be read (${err instanceof Error ? err.message : err})`); + continue; + } + if (group?.state !== "success") missing.push(`${GROUP_CONTEXT} is ${group?.state ?? "missing"} at ${at}`); + if (!m.sibling) continue; + try { + if (!(await forge.approvedAt(owner, repo, m.number, m.head))) missing.push(`${at} carries no approving review at that head`); + } catch (err) { + missing.push(`whether ${at} carries an approving review could not be read (${err instanceof Error ? err.message : err})`); + } + } + return missing.length ? missing.join("; ") : undefined; +} diff --git a/modules/gitea/test/guard.test.ts b/modules/gitea/test/guard.test.ts index 567ad023..034bf238 100644 --- a/modules/gitea/test/guard.test.ts +++ b/modules/gitea/test/guard.test.ts @@ -44,8 +44,13 @@ const state = { // When set, the merge commit's parents as the forge reports them, or the status it answers them with. parents: null as string[] | null, commitsFail: 0, + // The sibling beside which a verdict was judged (novox/hq ADR 0382 §6): its group status at its head, and its reviews. + groupStatus: "", + siblingGroup: "", + reviews: [] as { state: string; commit_id: string; dismissed?: boolean }[], }; const REVIEWED = "abc1234def5678901234567890abcdef12345678"; +const SIBLING_HEAD = "bbbbbbbbbbbb5678901234567890abcdef12345678".slice(0, 40); const TRUNK = "7a8b9c0d1e2f3a4b5c6d7e8f9a0b1c2d3e4f5a6b"; const MOVED = "3c4d5e6f7a8b9c0d1e2f3a4b5c6d7e8f9a0b1c2d"; // A verdict judged against a trunk commit, as both statuses say it (novox/hq ADR 0323 §3). @@ -69,6 +74,9 @@ beforeEach(() => { state.mergeCommit = "feedface00000000000000000000000000000000"; state.parents = null; state.commitsFail = 0; + state.groupStatus = ""; + state.siblingGroup = ""; + state.reviews = []; }); const json = (res: ServerResponse, status: number, body: unknown): void => { @@ -127,8 +135,17 @@ const server = createServer(async (req, res) => { return json(res, 200, { sha: state.mergeCommit, parents: parents.map((sha) => ({ sha })) }); } if (method === "GET" && path === `${repo}/commits/${state.head}/status`) { - return json(res, 200, { sha: state.head, state: "success", statuses: state.statuses }); + const group = state.groupStatus ? [{ context: "mesh/delivery-group", status: state.groupStatus }] : []; + return json(res, 200, { sha: state.head, state: "success", statuses: [...state.statuses, ...group] }); } + if (method === "GET" && path === `/repos/novox/mesh-host/commits/${SIBLING_HEAD}/status`) { + return json(res, 200, { sha: SIBLING_HEAD, state: "success", + statuses: state.siblingGroup ? [{ context: "mesh/delivery-group", status: state.siblingGroup }] : [] }); + } + if (method === "GET" && path === "/repos/novox/mesh-host/pulls/114/reviews") { + return json(res, 200, url.searchParams.get("page") === "1" ? state.reviews : []); + } + if (method === "GET" && path === "/repos/novox/mesh-host/issues/114/labels") return json(res, 200, []); if (method === "POST" && path === `${repo}/pulls/7/merge`) { state.merged = true; return json(res, 200, null); @@ -529,3 +546,64 @@ test("no tool's caller text becomes another path or another user: `..` and `?sud assert.deepEqual(state.asked, ["DELETE /repos/novox/mesh-catalog/branches/feat/x", "GET /repos/novox/mesh-catalog/contents/modules/gitea/module.json"]); }); + + +// **A member judged beside its siblings merges only with its group passed and reviewed** (novox/hq ADR 0382 §6, issue 867): +// the verdict names the sibling; mesh/delivery-group must be a success at the member's head and the sibling's head that +// verdict read, and the sibling's head must carry an approving review. A verdict that read none merges as before. +test("gitea_merge_pull_request merges a verdict that read siblings only with its group passed and each sibling head reviewed", async () => { + const { keepSiblings, siblingsFile } = await import("../dist/siblings.js"); + const merge = () => tool("gitea_merge_pull_request").run({ owner: "novox", repo: "mesh-catalog", number: 7, head: REVIEWED }); + const beside = `head ${REVIEWED.slice(0, 12)} merged into trunk ${TRUNK.slice(0, 12)} beside mesh-host#114@${SIBLING_HEAD.slice(0, 8)}`; + const named = (siblings: unknown[]) => keepSiblings(siblingsFile(stateDir), + { "novox/mesh-catalog#7": { head: REVIEWED, siblings: siblings as any } }); + state.statuses = [{ context: "mesh/merge-gate", state: "success", description: `${beside} — pass: x` }, + { context: "mesh/repo-check", state: "success", description: `${beside} — pass: y` }]; + named([{ repository: "novox/mesh-host", number: 114, head: SIBLING_HEAD, base: "main", base_commit: "c".repeat(40) }]); + + // Nothing passed: every missing thing is named, nothing merged. + await assert.rejects(merge(), (err: Error) => { + assert.match(err.message, /is not merged: its verdict was judged beside 1 sibling\(s\), and mesh\/delivery-group is missing at mesh-catalog#7@abc1234def56; mesh\/delivery-group is missing at mesh-host#114@bbbbbbbbbbbb; mesh-host#114@bbbbbbbbbbbb carries no approving review at that head/); + return true; + }); + assert.equal(state.merged, false); + + // The group passed at the member's head only: the sibling's head still lacks it. + state.groupStatus = "success"; + await assert.rejects(merge(), /mesh\/delivery-group is missing at mesh-host#114@bbbbbbbbbbbb; mesh-host#114@bbbbbbbbbbbb carries no approving review/); + // Passed at both heads, but the only approval is at another head, or dismissed: not an approval at the head read. + state.siblingGroup = "success"; + state.reviews = [{ state: "APPROVED", commit_id: "d".repeat(40) }, { state: "APPROVED", commit_id: SIBLING_HEAD, dismissed: true }, + { state: "COMMENT", commit_id: SIBLING_HEAD }]; + await assert.rejects(merge(), /judged beside 1 sibling\(s\), and mesh-host#114@bbbbbbbbbbbb carries no approving review at that head\. A member judged beside/); + assert.equal(state.merged, false); + + // Passed and approved at the very heads read: it merges. + state.reviews.push({ state: "APPROVED", commit_id: SIBLING_HEAD }); + assert.equal(((await merge()) as Record).merged, true); + + // A status that says it read siblings, with no record of them, is no verdict to merge on. + state.merged = false; + keepSiblings(siblingsFile(stateDir), {}); + await assert.rejects(merge(), /its verdict read siblings, and which ones is not held here/); + assert.equal(state.merged, false); + + // A verdict that read none (an older controller, or a group of one) merges exactly as before. + state.statuses = judgedAt(TRUNK); + state.groupStatus = ""; + assert.equal(((await merge()) as Record).merged, true); +}); + +test("a verdict put out of date by a sibling's move is refused in its own words, and asked again once", async () => { + const merge = () => tool("gitea_merge_pull_request").run({ owner: "novox", repo: "mesh-catalog", number: 7, head: REVIEWED }); + const words = "out of date: judged beside mesh-host#114 at bbbbbbbbbbbb, it is now at dddddddddddd; judged again on a recheck or when it is merged"; + state.statuses = judgedAt(TRUNK).map((s) => ({ ...s, state: "pending", description: words })); + await assert.rejects(merge(), (err: Error) => { + assert.match(err.message, /its verdict was put out of date by a sibling's move \(out of date: judged beside mesh-host#114 at bbbbbbbbbbbb, it is now at dddddddddddd/); + assert.match(err.message, /was asked/); + assert.match(err.message, /never merges by itself/); + return true; + }); + assert.equal(state.merged, false); + assert.equal(state.emitted.length, 1, "the check is asked, once"); +}); diff --git a/modules/gitea/test/siblings.test.ts b/modules/gitea/test/siblings.test.ts new file mode 100644 index 00000000..209e6c14 --- /dev/null +++ b/modules/gitea/test/siblings.test.ts @@ -0,0 +1,269 @@ +// **A delivery group's member is judged beside its siblings, and a sibling's move puts that verdict out of date** +// (novox/hq ADR 0382 §4-§6, issue 867): the holder keeps the siblings each verdict names; a sibling's new head, close +// unmerged or base move (other than its own merge of the head that was read) sets both statuses of every member whose +// verdict names it back to pending, and runs nothing; a member merges only with its group passed and every sibling head +// read approved. + +import { test } from "node:test"; +import assert from "node:assert/strict"; +import { mkdtempSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; + +import { besideWords, groupRefusal, keepSiblings, namesSiblings, noteVerdict, readSiblings, siblingOutOfDate, siblingsFile, + siblingsOf, siblingStatuses, weighSiblings, type SiblingRecord } from "../dist/siblings.js"; +import { weighTrunks } from "../dist/trunk.js"; +import { commentFor, statusesFor } from "../dist/pulls.js"; + +const h = (c: string) => c.repeat(40); +const MEMBER_HEAD = h("a"), HOST_HEAD = h("b"), HOST_BASE = h("c"), HOST_NEW = h("d"), HOST_MOVED = h("e"), MERGE = h("f"); +const sibling = { repository: "novox/mesh-host", number: 114, head: HOST_HEAD, base: "main", base_commit: HOST_BASE }; +const verdict = { owner: "novox", repo: "mesh-controller", number: 346, commit: MEMBER_HEAD, verdict: "pass", summary: "x", id: "build-1", + trunk: h("1").slice(0, 12), siblings: [sibling], judge_from: "sibling novox/mesh-controller#346 merge 123456789abc", + gate: { verdict: "pass", summary: "every machine composes ".repeat(10) }, "repo-check": { verdict: "pass", summary: "ok" } }; + +test("both statuses name the siblings the verdict read, keeping the verdict word and the start of its reason within 140 characters", () => { + // A realistic controller summary for a failure, long enough that the forge clips it. + const reason = "2 of 4 machines do not compose: ace: mesh-host.service needs seat node-reads, which no module on ace claims after this change"; + const c = { ...verdict, gate: { verdict: "fail", summary: reason }, "repo-check": { verdict: "fail", summary: "its merge-check.sh failed: internal/link/check.go:145:2: undefined" } }; + const [gate, repo] = statusesFor(c); + for (const s of [gate, repo]) { + assert.ok(s.description.startsWith(`head ${MEMBER_HEAD.slice(0, 12)} merged into trunk 111111111111 beside mesh-host#114@bbbbbbbb — fail: `), s.description); + assert.ok(s.description.length <= 140, `${s.description.length} characters`); + assert.ok(namesSiblings(s.description)); + } + assert.match(gate.description, /fail: 2 of 4 machines do not compose: ace: mesh-host/, gate.description); + assert.match(repo.description, /fail: its merge-check\.sh failed: internal/, repo.description); + assert.equal(besideWords([...siblingsOf([sibling]), ...siblingsOf([{ ...sibling, number: 115 }])]), "beside mesh-host#114@bbbbbbbb +1 more"); + const comment = commentFor({ ...verdict, gate: { verdict: "fail", summary: "no" } })!; + assert.match(comment, /Siblings read beside it: novox\/mesh-host#114 `bbbbbbbbbbbb` merged onto `cccccccccccc` \(main\); judged by sibling novox\/mesh-controller#346 merge/); +}); + +test("a verdict that could not learn its siblings says so on the status and in the comment, and names none", () => { + const { siblings, ...unknown } = { ...verdict, siblings_unknown: "mesh-delivery did not answer" }; + const reason = "2 of 4 machines do not compose: ace: mesh-host.service needs seat node-reads, which no module claims"; + for (const s of statusesFor({ ...unknown, gate: { verdict: "fail", summary: reason } })) { + assert.match(s.description, /^head aaaaaaaaaaaa merged into trunk 111111111111 siblings unknown — (fail|pass): /, s.description); + assert.ok(s.description.length <= 140); + assert.equal(namesSiblings(s.description), false, "it read no siblings, so the merge tool asks for none"); + } + assert.match(commentFor({ ...unknown, gate: { verdict: "fail", summary: "no" } })!, /Siblings could not be learned: mesh-delivery did not answer/); +}); + +test("a verdict without siblings is said exactly as before (an older controller)", () => { + const { siblings, judge_from, ...old } = verdict; + const [gate] = statusesFor(old); + assert.equal(namesSiblings(gate.description), false); + assert.match(gate.description, /^head aaaaaaaaaaaa merged into trunk 111111111111 — pass: /); + assert.deepEqual(siblingsOf(undefined), []); + assert.deepEqual(siblingsOf([{ repository: "novox/x" }]), [], "a sibling named without its head is none"); +}); + +test("the siblings a verdict names are kept per member, replaced by its next verdict, and removed by one that read none", () => { + const file = siblingsFile(mkdtempSync(join(tmpdir(), "gitea-siblings-"))); + const record: SiblingRecord = {}; + assert.equal(noteVerdict(record, "novox/mesh-controller#346", MEMBER_HEAD, []), false); + assert.equal(noteVerdict(record, "novox/mesh-controller#346", MEMBER_HEAD, siblingsOf([sibling])), true); + keepSiblings(file, record); + assert.deepEqual(readSiblings(file), record, "kept where the holder keeps its state"); + assert.equal(noteVerdict(record, "novox/mesh-controller#346", MEMBER_HEAD, []), true); + assert.deepEqual(record, {}); + assert.deepEqual(readSiblings(null), {}); +}); + +// --- a sibling's move ----------------------------------------------------------------------------------------- + +interface World { + host: { merged: boolean; state: string; head_sha: string; merge_commit_sha?: string }; + hostBase: string; + parents: string[]; + member: { head_sha: string; state: string; merged: boolean }; + checks: Record; + written: { sha: string; context: string; state: string; description: string }[]; + reads: string[]; +} +const world = (): World => ({ host: { merged: false, state: "open", head_sha: HOST_HEAD }, hostBase: HOST_BASE, parents: [], + member: { head_sha: MEMBER_HEAD, state: "open", merged: false }, + checks: { "mesh/merge-gate": { state: "success", description: "head aaaaaaaaaaaa merged into trunk 111111111111 beside mesh-host#114@bbbbbbbb — pass: x" }, + "mesh/repo-check": { state: "success", description: "pass: y" } }, + written: [], reads: [] }); +const forgeOf = (w: World) => ({ + async getPullRequest(owner: string, repo: string, n: number) { + w.reads.push(`${owner}/${repo}#${n}`); + return repo === "mesh-host" ? { number: n, title: "", html_url: "", ...w.host } : { number: n, title: "", html_url: "", merged: false, ...w.member }; + }, + async branchHead(_o: string, _r: string, _b: string) { return w.hostBase; }, + async commitParents() { return w.parents; }, +}); +const gatesOf = (w: World) => ({ + async outOfDate(_o: string, _r: string, sha: string, decide: (c: any) => any[] | Promise) { + const put = await decide(w.checks); + for (const s of put) { w.written.push({ sha, ...s }); w.checks[s.context] = { state: s.state, description: s.description }; } + return put; + }, +}); +const recordOf = (): SiblingRecord => ({ "novox/mesh-controller#346": { head: MEMBER_HEAD, siblings: siblingsOf([sibling]) } }); +const weigh = (w: World, record: SiblingRecord, repository = "novox/mesh-host", number?: number) => + weighSiblings(forgeOf(w) as any, gatesOf(w) as any, record, repository, number); + +test("a sibling's new head puts both statuses of the member back to pending, with the reason, and runs nothing", async () => { + const w = world(); + w.host.head_sha = HOST_NEW; + await weigh(w, recordOf(), "novox/mesh-host", 114); + assert.deepEqual(w.written.map((s) => [s.sha, s.context, s.state]), + [[MEMBER_HEAD, "mesh/merge-gate", "pending"], [MEMBER_HEAD, "mesh/repo-check", "pending"]]); + assert.equal(w.written[0].description, + "out of date: judged beside mesh-host#114 at bbbbbbbbbbbb, it is now at dddddddddddd; judged again on a recheck or when it is merged"); + assert.ok(siblingOutOfDate(w.written[0].description)); + // Already pending (out of date, or a check on its way): not written again. + w.written.length = 0; + await weigh(w, recordOf(), "novox/mesh-host", 114); + assert.deepEqual(w.written, []); +}); + +test("a sibling closed unmerged puts the member's verdict out of date", async () => { + const w = world(); + w.host.state = "closed"; + await weigh(w, recordOf(), "novox/mesh-host", 114); + assert.equal(w.written.length, 2); + assert.match(w.written[0].description, /^out of date: judged beside mesh-host#114 was closed unmerged/); +}); + +test("a move of the sibling's base puts it out of date, naming both commits", async () => { + const w = world(); + w.hostBase = HOST_MOVED; + await weigh(w, recordOf()); + assert.equal(w.written.length, 2); + assert.match(w.written[0].description, /mesh-host#114 merged onto cccccccccccc, its main is now eeeeeeeeeeee/); +}); + +test("the sibling's own merge of the head read onto the base read does not, in either order of the two merges", async () => { + // The sibling merges first (a merge commit of the base commit read and the head read). + const w = world(); + Object.assign(w.host, { merged: true, state: "closed", merge_commit_sha: MERGE }); + w.hostBase = MERGE; + w.parents = [HOST_BASE, HOST_HEAD]; + const record = recordOf(); + assert.equal(await weigh(w, record), true, "the record changes: the sibling is marked merged"); + assert.deepEqual(w.written, []); + assert.equal(record["novox/mesh-controller#346"].siblings[0].merged, true); + // A move of that base afterwards is no sibling's move: the sibling is merged. + w.hostBase = HOST_MOVED; + await weigh(w, record); + assert.deepEqual(w.written, []); + + // The member merges first: its own repository moves, which no member's verdict names as a sibling. + const m = world(); + m.member.merged = true; + await weigh(m, recordOf(), "novox/mesh-controller"); + assert.deepEqual(m.written, []); + // And the sibling then merges: still not a move. + Object.assign(m.host, { merged: true, state: "closed", merge_commit_sha: MERGE }); + m.parents = [HOST_BASE, HOST_HEAD]; + await weigh(m, recordOf()); + assert.deepEqual(m.written, []); + + // A squash is one parent: the base commit read. + const q = world(); + Object.assign(q.host, { merged: true, state: "closed", merge_commit_sha: MERGE }); + q.parents = [HOST_BASE]; + await weigh(q, recordOf()); + assert.deepEqual(q.written, []); +}); + +test("a merge of another head, or onto another base, is a move", async () => { + const other = world(); + Object.assign(other.host, { merged: true, state: "closed", merge_commit_sha: MERGE, head_sha: HOST_NEW }); + other.parents = [HOST_BASE, HOST_NEW]; + await weigh(other, recordOf()); + assert.equal(other.written.length, 2); + const onto = world(); + Object.assign(onto.host, { merged: true, state: "closed", merge_commit_sha: MERGE }); + onto.parents = [HOST_MOVED, HOST_HEAD]; + await weigh(onto, recordOf()); + assert.equal(onto.written.length, 2); +}); + +test("only the members whose verdict names the sibling are touched, and a member that moved on is dropped", async () => { + // Another sibling's move, or another repository's: nothing read, nothing written. + const w = world(); + w.host.head_sha = HOST_NEW; + await weigh(w, recordOf(), "novox/mesh-lab"); + await weigh(w, recordOf(), "novox/mesh-host", 999); + assert.deepEqual(w.written, []); + // A verdict that is none (a gate not yet judged) is left alone. + const none = world(); + none.host.head_sha = HOST_NEW; + none.checks = {}; + await weigh(none, recordOf()); + assert.deepEqual(none.written, []); + // The member has a new head: its old verdict is no verdict; the record is dropped, nothing written to the old head. + const moved = world(); + moved.host.head_sha = HOST_NEW; + moved.member.head_sha = h("9"); + const record = recordOf(); + assert.equal(await weigh(moved, record), true); + assert.deepEqual(moved.written, []); + assert.deepEqual(record, {}); + // A repository without a repo-check gets no pending one a verdict would never set. + assert.deepEqual(siblingStatuses({ "mesh/merge-gate": { state: "success" } }, "x").map((s) => s.context), ["mesh/merge-gate"]); +}); + +test("a trunk that moved tells the siblings' weighing; one first seen does not", async () => { + const told: string[] = []; + const forge = { listOpenPulls: async () => [{ number: 1, title: "", state: "open", merged: false, head_sha: h("2"), base: "main", html_url: "" }], + branchHead: async () => h("3") }; + const gates = { outOfDate: async () => [] }; + const trunks: Record = {}; + await weighTrunks(forge as any, gates as any, trunks, "novox", "mesh-host", async (b) => { told.push(b); }); + assert.deepEqual(told, [], "first seen"); + trunks["novox/mesh-host:main"] = h("4"); + await weighTrunks(forge as any, gates as any, trunks, "novox", "mesh-host", async (b) => { told.push(b); }); + assert.deepEqual(told, ["main"]); +}); + +// --- the merge rule ------------------------------------------------------------------------------------------- + +test("the group rule names every missing thing: the group status at each head read, and each sibling head's review", async () => { + const self = { owner: "novox", repo: "mesh-controller", number: 346, head: MEMBER_HEAD }; + const statuses: Record = {}; + const approved = new Set(); + const forge = { + headChecks: async (_o: string, r: string, sha: string) => (statuses[`${r}@${sha}`] ? { "mesh/delivery-group": { state: statuses[`${r}@${sha}`] } } : {}), + approvedAt: async (_o: string, r: string, n: number, head: string) => approved.has(`${r}#${n}@${head}`), + }; + const siblings = siblingsOf([sibling]); + const none = await groupRefusal(forge, self, siblings); + assert.match(none!, /mesh\/delivery-group is missing at mesh-controller#346@aaaaaaaaaaaa; mesh\/delivery-group is missing at mesh-host#114@bbbbbbbbbbbb; mesh-host#114@bbbbbbbbbbbb carries no approving review at that head/); + statuses[`mesh-controller@${MEMBER_HEAD}`] = "success"; + statuses[`mesh-host@${HOST_HEAD}`] = "pending"; + assert.match((await groupRefusal(forge, self, siblings))!, /^mesh\/delivery-group is pending at mesh-host#114@bbbbbbbbbbbb; mesh-host#114@bbbbbbbbbbbb carries no approving review/); + statuses[`mesh-host@${HOST_HEAD}`] = "success"; + assert.match((await groupRefusal(forge, self, siblings))!, /^mesh-host#114@bbbbbbbbbbbb carries no approving review at that head$/); + approved.add(`mesh-host#114@${HOST_HEAD}`); + assert.equal(await groupRefusal(forge, self, siblings), undefined); + // A read that fails is named, never read as a pass. + const broken = { ...forge, approvedAt: async () => { throw new Error("the forge is away"); } }; + assert.match((await groupRefusal(broken, self, siblings))!, /could not be read \(the forge is away\)/); + assert.equal(besideWords(siblings), "beside mesh-host#114@bbbbbbbb"); +}); + +test("a merged or closed member's entry is dropped, so a sibling's later push never re-pends its merged head", async () => { + // mesh-host#114 judged beside mesh-controller#346; #114 then merged; a push to #346 follows. + const w = world(); + w.host.merged = true; // the member (mesh-host#114) is merged + w.member.head_sha = h("9"); // the sibling (mesh-controller#346) has a new head + const record: SiblingRecord = { "novox/mesh-host#114": { head: HOST_HEAD, siblings: siblingsOf([ + { repository: "novox/mesh-controller", number: 346, head: MEMBER_HEAD, base: "main", base_commit: h("2") }]) } }; + assert.equal(await weigh(w, record, "novox/mesh-controller", 346), true); + assert.deepEqual(w.written, [], "the merged head is not put back to pending"); + assert.deepEqual(record, {}, "and its entry is gone"); + // On its own close or merge the member's entry goes too, whatever the siblings do. + const own = world(); + own.host.state = "closed"; + const kept = recordOf(); + const mine: SiblingRecord = { "novox/mesh-host#114": { head: HOST_HEAD, siblings: kept["novox/mesh-controller#346"].siblings } }; + assert.equal(await weigh(own, mine, "novox/mesh-host", 114), true); + assert.deepEqual(mine, {}); + assert.deepEqual(own.written, []); +}); diff --git a/modules/gitea/trunk.ts b/modules/gitea/trunk.ts index 1f0b1a6a..1555fab1 100644 --- a/modules/gitea/trunk.ts +++ b/modules/gitea/trunk.ts @@ -128,10 +128,10 @@ export interface OutOfDateWriter { * out of date (ADR 0323 §4). A trunk first seen is recorded as it is and nothing is put out of date. Bounded however * many are open: one listing of the open pull requests (paged), one read of each trunk, then per open pull request into * a trunk that moved one read and at most two writes, one at a time. A trunk that could not be weighed whole is not - * recorded, so the next pass weighs it again. Answers whether the record changed. + * recorded, so the next pass weighs it again. Answers whether the record changed. `moved` is told of each base that moved, for the siblings whose base it is. */ export async function weighTrunks(forge: TrunkForge, gates: OutOfDateWriter, trunks: Record, owner: string, - name: string): Promise { + name: string, moved?: (base: string) => Promise): Promise { let changed = false; try { const open = (await forge.listOpenPulls(owner, name)).filter((p) => p.state === "open" && !!p.head_sha && !!p.base); @@ -151,6 +151,8 @@ export async function weighTrunks(forge: TrunkForge, gates: OutOfDateWriter, tru } console.log(`[gitea] ${owner}/${name}'s ${base} moved from ${twelve(before)} to ${twelve(head)}: ${put} open pull ` + `request(s) into it put out of date, nothing run (novox/hq ADR 0323)`); + // A verdict that read a pull request into this trunk as a sibling is out of date by its move too (ADR 0382 §5). + if (moved) await moved(base); } trunks[key] = head; changed = true; diff --git a/modules/mesh-delivery/cmd/mesh-delivery/holder_test.go b/modules/mesh-delivery/cmd/mesh-delivery/holder_test.go index e2ccccae..5a62afbd 100644 --- a/modules/mesh-delivery/cmd/mesh-delivery/holder_test.go +++ b/modules/mesh-delivery/cmd/mesh-delivery/holder_test.go @@ -1,6 +1,7 @@ package main import ( + "encoding/json" "strings" "testing" "time" @@ -573,3 +574,49 @@ func TestStalledAndCloseWorkFromTheTable(t *testing.T) { t.Fatalf("H2 closed a held delivery: %v", err) } } + +// novox/hq issue 867, ADR 0382 §1 (contract A): each member of a group in `groups` also says its pull +// request number, full head, base branch and whether it is open; `members` keeps its text form. +func TestGroupsNamesEachMembersPullRequestHeadBaseAndOpen(t *testing.T) { + w := newWorld(t) + headA := "a1a1a1a1a1a1a1a1a1a1a1a1a1a1a1a1a1a1a1a1" + headB := "b2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b2" + pb := pr("novox/b", 2, headB, "feat/z") + pb.Base = "release/1" + w.h.PullUpdated(pr("novox/a", 1, headA, "feat/z")) + w.h.PullUpdated(pb) + lines := w.h.Groups(false) + if len(lines) != 1 || len(lines[0].Details) != 2 || len(lines[0].Members) != 2 { + t.Fatalf("groups: %+v", lines) + } + got := map[string]MemberLine{} + for _, m := range lines[0].Details { + got[m.Repository] = m + } + a, b := got["novox/a"], got["novox/b"] + if a.Number != 1 || a.Head != headA || a.Base != "main" || !a.Open || a.ID == "" { + t.Fatalf("member a: %+v", a) + } + if b.Number != 2 || b.Head != headB || b.Base != "release/1" || !b.Open { + t.Fatalf("member b: %+v", b) + } + if !strings.Contains(lines[0].Members[0], "(") { + t.Fatalf("members lost its text form: %v", lines[0].Members) + } + // Closed unmerged: still listed while the group is, but not open. + w.h.PullClosed(pr("novox/b", 2, headB, "feat/z")) + for _, l := range w.h.Groups(true) { + for _, m := range l.Details { + if m.Repository == "novox/b" && m.Open { + t.Fatalf("a closed pull request is open: %+v", m) + } + } + } + // JSON keys are the contract's. + raw, _ := json.Marshal(a) + for _, k := range []string{`"number":1`, `"head":"` + headA + `"`, `"base":"main"`, `"open":true`} { + if !strings.Contains(string(raw), k) { + t.Fatalf("%s lacks %s", raw, k) + } + } +} diff --git a/modules/mesh-delivery/cmd/mesh-delivery/verbs.go b/modules/mesh-delivery/cmd/mesh-delivery/verbs.go index b2dfed42..2b54eb2d 100644 --- a/modules/mesh-delivery/cmd/mesh-delivery/verbs.go +++ b/modules/mesh-delivery/cmd/mesh-delivery/verbs.go @@ -154,6 +154,21 @@ type GroupLine struct { Contradictions []Contradiction `json:"contradictions,omitempty"` Check string `json:"composed,omitempty"` Members []string `json:"members"` + // Details are the members again, one object each, with what a reader needs to find the pull request's + // siblings: its number, full head, base branch and whether it is open (novox/hq ADR 0382 §1). Additive: + // `members` keeps its text form for older readers. + Details []MemberLine `json:"member_details,omitempty"` +} + +// MemberLine is one member of a group, as `groups` lists it beside its text line. +type MemberLine struct { + ID string `json:"id"` + Repository string `json:"repository"` + State State `json:"state"` + Number int `json:"number"` + Head string `json:"head"` + Base string `json:"base"` + Open bool `json:"open"` } // Groups is the `groups` verb. @@ -174,6 +189,9 @@ func (h *Holder) Groups(all bool) []GroupLine { Contradictions: g.Contradictions} for _, d := range members { l.Members = append(l.Members, fmt.Sprintf("%s (%s)", d.ID, d.State)) + l.Details = append(l.Details, MemberLine{ID: d.ID, Repository: d.Repository, State: d.State, + Number: d.Number, Head: d.Commit, Base: d.Base, + Open: !d.State.Final() && d.MergedAs == "" && !d.ClosedUnmerged}) } if g.Check != nil { l.Check = g.Check.Verdict