diff --git a/modules/gitea/client.ts b/modules/gitea/client.ts index eb3b8c93..7219b84d 100644 --- a/modules/gitea/client.ts +++ b/modules/gitea/client.ts @@ -341,6 +341,33 @@ export class GiteaClient { return out; } + /** **Every open pull request of a repository**, page by page (novox/hq ADR 0323 §4): what a trunk's move puts out of + * date, however many are open — not the twenty most recently updated a pass announces from. Bounded at 20 pages + * of 50; a repository with more is said, and the rest are left to the merge tool's own comparison. */ + async listOpenPulls(owner: string, repo: string): Promise { + const out: GiteaPull[] = []; + for (let page = 1; page <= 20; page++) { + const batch = await this.listPullRequests(owner, repo, { state: "open", limit: "50", page: String(page) }); + out.push(...batch); + if (batch.length < 50) return out; + } + console.error(`[gitea] ${owner}/${repo} has more than 1000 open pull requests; only the first 1000 are read`); + return out; + } + + /** The commit a branch is at now: a trunk's head (novox/hq ADR 0323). */ + async branchHead(owner: string, repo: string, branch: string): Promise { + const b = await this.request(`${repoPath(owner, repo)}/branches/${pathSegments("branch", branch)}`); + const sha = String(b?.commit?.id ?? ""); + if (!/^[0-9a-f]{40,64}$/i.test(sha)) throw new Error(`the forge named no commit for ${owner}/${repo}'s ${branch}`); + return sha; + } + + /** One repository, as the forge holds it: its clone URL above all, for an announcement. */ + async getRepo(owner: string, repo: string): Promise { + return GiteaClient.mapRepo(await this.request(repoPath(owner, repo))); + } + async getPullRequest(owner: string, repo: string, index: number): Promise { indexOf(index); return GiteaClient.mapPull(await this.request(`${repoPath(owner, repo)}/pulls/${index}`)); diff --git a/modules/gitea/gate.ts b/modules/gitea/gate.ts index 9a19ac72..f92510bf 100644 --- a/modules/gitea/gate.ts +++ b/modules/gitea/gate.ts @@ -18,7 +18,7 @@ // restart, when the check was opened is read from the forge's own history of the gate. import { CHECK_CONTEXT, pendingWords } from "./pulls.js"; -import type { CommitStatus } from "./client.js"; +import type { CommitStatus, HeadCheck } from "./client.js"; /** The build seat's `started`, as mesh-controller's link.BuildStart publishes it. */ export interface BuildStarted { @@ -37,6 +37,8 @@ export interface GateForge { gateOf(owner: string, repo: string, sha: string): Promise<{ state: string; updated_at?: string } | undefined>; /** The head's mesh/merge-gate statuses, every one the forge keeps, newest first (novox/hq issue 456). */ gateHistory(owner: string, repo: string, sha: string): Promise; + /** The head's statuses by context, the newest of each, with their words (novox/hq ADR 0323). */ + headChecks(owner: string, repo: string, sha: string): Promise>; } /** One status of a head's gate as the forge keeps it: its state, its words, and when it was set. */ @@ -174,6 +176,26 @@ export class GateWriter { }); } + /** + * **A trunk's move puts the head's verdict out of date** (novox/hq ADR 0323 §4): its merge check statuses are put back + * to pending, with the reason, in their turn after every write of the head already asked for — a verdict in flight + * lands first. What is written is decided in the turn, from the statuses the forge holds then (`decide`, trunk.ts's + * outOfDateStatuses): a check on its way, or a verdict already judged against the trunk's head, is left as it is. It + * opens no check — nothing is asked — so a verdict of a check asked before it keeps its weight. Answers what it wrote; + * a write that fails fails the caller, and the move is weighed again on the next pass. + */ + outOfDate(owner: string, repo: string, sha: string, + decide: (checks: Record) => CommitStatus[] | Promise, target?: string): Promise { + const h = this.head(owner, repo, sha); + const written: CommitStatus[] = []; + return this.turn(h, async () => { + for (const s of await decide(await this.forge.headChecks(owner, repo, sha))) { + await this.forge.setCommitStatus(owner, repo, sha, target ? { ...s, target_url: target } : s); + written.push(s); + } + }).then(() => written); + } + /** * A build machine took an ask of this head: said while no verdict was heard, and only for the check now pending — * a start from before the pending words were set (a run before a recheck, the stream's history) is not this check's. diff --git a/modules/gitea/index.ts b/modules/gitea/index.ts index 4cbd90fa..ab0cfffe 100644 --- a/modules/gitea/index.ts +++ b/modules/gitea/index.ts @@ -17,8 +17,13 @@ // Consumes node-build-agent.started — a build machine took an ask — and says, on the head a pending check is of, // which machine builds it since when (novox/hq issue 446). This module is the one writer of mesh/merge-gate. // -// Serves gitea_commit_status from this bundle, not the tools', so a recheck's pending reaches that one writer -// (novox/hq issue 456). +// **A trunk's move puts every open verdict into it out of date** (novox/hq ADR 0323 §4, trunk.ts): each pass reads the +// head of every trunk an open pull request merges into, and when one moved, the merge check statuses of every open pull +// request into it go back to pending with the reason. Nothing is run for it. A verdict that lands judged against a trunk +// commit that is no longer the trunk's head is said out of date in place of its pass or failure. +// +// Serves gitea_commit_status and gitea_merge_pull_request from this bundle, not the tools', so a recheck's pending and +// the merge tool's ask of a check reach that one writer (novox/hq issue 456, ADR 0323). // // Every pull request the forge holds is announced, whatever its repository: the controller holds the // module graph and decides what is checked — a repository is never asked to opt in. @@ -34,10 +39,12 @@ import { emit, on } from "@novox/mesh-sdk/events"; import { registerModuleTools } from "@novox/mesh-sdk/tools"; import { commitStatusTool } from "./delivery.js"; +import { mergeTool } from "./merge.js"; import { GiteaClient, movedSince } from "./client.js"; import { CHECK_CONTEXT, REPO_CHECK_CONTEXT, commentFor, headsToAnnounce, statusesFor, type Announced, type Checked, type RepoCheckFacts, reposToLook } from "./pulls.js"; import { GateWriter, clonedAs, type BuildStarted } from "./gate.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 // nothing to watch; log and stay quiet rather than crash the runtime. With one, the first poll mints @@ -250,37 +257,8 @@ async function pollPulls(client: GiteaClient): Promise { const key = `${repo.full_name}#${pull.number}`; const fresh = primedPulls || (!!pull.updated_at && pull.updated_at > dayAgo); if (fresh) { - const files = await client.listPullFiles(repo.owner, repo.name, pull.number); - const head = String(pull.head_sha); - // What the controller maps the change onto the mesh's module graph with (novox/hq ADR 0237 as - // amended): which changed directories hold a module at the head — the merge's rule (issue 278) — - // and whether the head holds the repository's own merge-check.sh. Not said when it could not be - // read; the controller then reads the change as touching everything built from the repository. - let moduleDirs: string[] | null = null; - let mergeCheck: boolean | null = null; - try { - if (!files.truncated) moduleDirs = await client.moduleDirsAt(repo.owner, repo.name, head, files.paths); - mergeCheck = await client.holdsFile(repo.owner, repo.name, head, "merge-check.sh"); - } catch (err) { - console.error(`[gitea] ${key}: what the head holds could not be read — ${err instanceof Error ? err.message : err}`); - } - await emit("pull.updated", { - owner: repo.owner, - repo: repo.name, - number: pull.number, - title: pull.title, - body: pull.body, - base: pull.base, - head: pull.head, - head_sha: pull.head_sha, - clone_url: repo.clone_url, - html_url: pull.html_url, - paths: files.paths, - paths_truncated: files.truncated, - removed: files.removed, - ...(moduleDirs ? { module_dirs: moduleDirs, module_dirs_said: true } : {}), - ...(mergeCheck !== null ? { merge_check: mergeCheck, merge_check_said: true } : {}), - }); + // The announcement the merge tool makes too when it asks a check (trunk.ts, novox/hq ADR 0323 §5). + await emit("pull.updated", await announcementOf(client, repo.owner, repo.name, repo.clone_url, pull)); // Said, so an operator knows the mesh was asked to check it. console.log(`[gitea] announced ${key} at ${String(pull.head_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 @@ -290,17 +268,52 @@ async function pollPulls(client: GiteaClient): Promise { heads[key] = String(pull.head_sha); changed = true; } + if (await trunksMoved(client, repo.owner, repo.name)) changedTrunks = true; } if (!primedPulls || changed) keepHeads(); + if (changedTrunks) keepTrunks(); + changedTrunks = false; primedPulls = true; lastPullLook = began; } +// **The trunks' heads, as this module last read them** (novox/hq ADR 0323 §4), by `owner/repo:branch`: kept beside the +// other records, so a move made while this module was away is weighed on its first pass back. With no record — the +// first pass after this rolls out — a trunk's head is taken as it is and nothing is put out of date: the verdicts +// then standing are weighed on the trunk's next move, and the merge tool compares commits whatever this holds. +const trunksRecord = process.env.MESH_GITEA_STATE_DIR ? join(process.env.MESH_GITEA_STATE_DIR, "trunks-seen.json") : null; +let trunks: Record = {}; +let changedTrunks = false; +if (trunksRecord && existsSync(trunksRecord)) { + try { + trunks = (JSON.parse(readFileSync(trunksRecord, "utf8")) as { trunks: Record }).trunks ?? {}; + } catch { + // An unreadable record is no record: the next pass takes each trunk's head as it is. + } +} +function keepTrunks(): void { + if (!trunksRecord) return; + mkdirSync(join(trunksRecord, ".."), { recursive: true }); + const tmp = trunksRecord + ".tmp"; + writeFileSync(tmp, JSON.stringify({ trunks })); + renameSync(tmp, trunksRecord); +} + +/** 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); +} + // **The verdict, set where the pull request shows it.** Every verdict is the head commit's status; one // that is not a pass also leaves the check's own account as a comment, so the reason is read where the // change is reviewed. An error — the check could not run — is the forge's `error`, never a success. async function setVerdict(client: GiteaClient, event: { body: unknown }): Promise { const c = (event.body ?? {}) as Checked; + if (!c.group && c.owner && c.repo && c.took) { + // How long it took, for the merge tool to say how long the next one may (novox/hq ADR 0323 §5). + keepTook(tookFile(process.env.MESH_GITEA_STATE_DIR), `${c.owner}/${c.repo}`, + { took: c.took, at: new Date().toISOString(), head: c.commit }); + } if (c.group) { // A delivery group's composed check (novox/hq ADR 0239): its verdict is the group owner's to say, on each // member's head, as mesh/delivery-group — never this head's merge gate. @@ -318,6 +331,26 @@ async function setVerdict(client: GiteaClient, event: { body: unknown }): Promis target = pull?.html_url || undefined; base = pull?.base || undefined; } + // **A verdict judged against a trunk commit that is no longer the trunk's head is out of date when it lands** + // (novox/hq ADR 0323 §4): a check asked before the trunk moved. It is said so, pending with the reason, in place of + // its pass or failure; a trunk that cannot be read lets the verdict stand, and the merge tool compares the commits. + const trunk = base ?? c.plan?.base; + if (c.trunk && trunk) { + const head = await client.branchHead(c.owner, c.repo, trunk).catch(() => undefined); + if (head && !sameCommit(c.trunk, head)) { + const description = outOfDateWords(c.trunk, head); + for (const context of [CHECK_CONTEXT, ...(c["repo-check"] ? [REPO_CHECK_CONTEXT] : [])]) { + await client.setCommitStatus(c.owner, c.repo, c.commit, { state: "pending", context, description, + ...(target ? { target_url: target } : {}) }); + } + // Its account is kept and shown all the same (ADR 0323 §7): what it judged, and that it is out of date. + const said = commentFor(c, await repoCheckFacts(client, c, trunk)); + if (c.number) await client.addComment(c.owner, c.repo, c.number, lateVerdictComment(c, said, description)); + console.log(`[gitea] ${c.owner}/${c.repo}#${c.number ?? "?"} at ${c.commit.slice(0, 8)}: merge check ${c.verdict}, ` + + `${description}`); + return; + } + } const facts = await repoCheckFacts(client, c, base ?? c.plan?.base); for (const status of statusesFor(c, facts)) { await client.setCommitStatus(c.owner, c.repo, c.commit, target ? { ...status, target_url: target } : status); @@ -350,6 +383,7 @@ const gates = new GateWriter({ setCommitStatus: (owner, repo, sha, status) => (gitea as GiteaClient).setCommitStatus(owner, repo, sha, status), gateOf: async (owner, repo, sha) => (await (gitea as GiteaClient).headChecks(owner, repo, sha))[CHECK_CONTEXT], gateHistory: (owner, repo, sha) => (gitea as GiteaClient).contextHistory(owner, repo, sha, CHECK_CONTEXT), + headChecks: (owner, repo, sha) => (gitea as GiteaClient).headChecks(owner, repo, sha), }); // **The status tool is served here, beside the writer** (novox/hq issue 456): the runtime starts each bundle as its own @@ -358,7 +392,7 @@ const gates = new GateWriter({ registerModuleTools("gitea", () => (gitea ? [commitStatusTool({ setCommitStatus: (owner, repo, sha, status) => (gitea as GiteaClient).setCommitStatus(owner, repo, sha, status), gates, -})] : [])); +}), mergeTool(gitea, { gates })] : [])); // **A build machine took an ask** (the build seat's `started`, novox/hq ADR 0157): said on the head it checks, when // the head is an open pull request's this module announced. Any other build — a branch, a merge's — says nothing. diff --git a/modules/gitea/merge.ts b/modules/gitea/merge.ts new file mode 100644 index 00000000..05ee77a2 --- /dev/null +++ b/modules/gitea/merge.ts @@ -0,0 +1,183 @@ +// The forge's merge tool, `gitea_merge_pull_request` (novox/hq ADR 0266, issue 381, ADR 0323). +// +// **Served from the watcher's bundle, beside the merge check's one writer** (index.ts, gate.ts; novox/hq issue 456): +// when a verdict is judged against an earlier trunk commit than the trunk's head, the tool asks the head's check and +// puts its statuses back to pending through that writer, so where the check runs is said and no second check is asked. +// Each bundle the runtime serves is its own process; from the tools' bundle the pending bypassed the writer's chain. + +import { emit } from "@novox/mesh-sdk/events"; +import type { ToolDefinition } from "@novox/mesh-sdk/tools"; +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"; + +/** 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}$/; + +/** What the merge tool reaches beside the forge: the merge check's one writer, how an event is said, the module's + * state directory, and the clock. */ +export interface MergeDeps { + gates: { rechecked(owner: string, repo: string, sha: string, status: CommitStatus): Promise }; + emit?: (type: string, body: unknown) => Promise; + stateDir?: string; + now?: () => Date; +} + +export function mergeTool(gitea: GiteaClient, given: MergeDeps): ToolDefinition { + const deps = { ...given, emit: given.emit ?? emit, stateDir: given.stateDir ?? process.env.MESH_GITEA_STATE_DIR, + now: given.now ?? (() => new Date()) }; + return { + name: "gitea_merge_pull_request", + description: "Merge a pull request at the head you name — the commit you reviewed — when that is still its head " + + "and every status its base branch's protection requires succeeded there (mesh/merge-gate, mesh/repo-check), " + + "judged against the trunk's head as it is now, optionally deleting the source branch afterwards. Refused " + + "otherwise, with nothing merged: no tool merges past a check, and none merges a head that moved after it was " + + "reviewed. A verdict judged against an earlier trunk commit is refused, naming both, and the head's check against " + + "the trunk's head is asked once, with how long the last check of the repository took: ask again at the same head " + + "once it passes — the tool never merges by itself.", + input: { + owner: { type: "string", description: "the repository owner" }, + repo: { type: "string", description: "the repository name" }, + number: { type: "number", description: "the PR number" }, + head: { type: "string", description: "the commit you reviewed, as its sha or at least its first 12 characters: " + + "refused unless it is the pull request's head" }, + method: { type: "string", description: "merge (default) | fast-forward-only | squash | rebase | rebase-merge. " + + "Only merge and fast-forward-only can be read back to prove the head you named is what landed" }, + delete_branch: { type: "boolean", description: "delete the source branch after merge (default true)" }, + }, + run: async (args) => { + const owner = checkName("owner", args.owner); + const repo = checkName("name", args.repo); + const number = Number(args.number); + const method = args.method ? String(args.method) : "merge"; + if (!MERGE_METHODS.has(method)) throw new Refused(`${method} is not a merge method: ${[...MERGE_METHODS].join(", ")}`); + const deleteBranch = args.delete_branch === undefined ? true : Boolean(args.delete_branch); + // **Only a head whose required statuses all succeeded** (novox/hq ADR 0266): read here rather than left + // to the forge, whose administrator — the account this tool's token is minted for — may merge past a + // rule that lets it. A rule that cannot be read is no rule passed. + // **Only the head that was reviewed** (novox/hq issue 381): mesh-controller#197 was reviewed at 065cfa0d, + // force-pushed to 3ced96f, checked green there, and merged at 3ced96f, because this tool merged whatever the + // head was and nothing told it which head the review had approved. The caller names it; a head that moved + // since is refused, and the forge is handed the same head, so a push between this read and the merge is + // refused by the forge too. + const named = args.head === undefined ? "" : String(args.head).trim().toLowerCase(); + if (!named) { + throw new Refused(`${owner}/${repo}#${number} is not merged: name the head you reviewed (head), so that only ` + + `that commit can land (novox/hq issue 381). Nothing was merged`); + } + // Twelve characters, not seven: an author who force-pushes can make a commit whose first seven match the + // reviewed one in seconds, and the review would then merge it. Twelve cannot be matched that way. + if (!REVIEWED_HEAD.test(named)) { + throw new Refused(`${owner}/${repo}#${number} is not merged: head ${named} does not name one commit — give its ` + + `sha, or at least its first 12 characters. Nothing was merged`); + } + const pull = await gitea.getPullRequest(owner, repo, number); + if (pull.merged) throw new Refused(`${owner}/${repo}#${number} is already merged`); + if (!pull.head_sha || !pull.base) throw new Refused(`${owner}/${repo}#${number} names no head commit or base branch`); + if (!pull.head_sha.toLowerCase().startsWith(named)) { + throw new Refused(`${owner}/${repo}#${number} is not merged: its head is ${pull.head_sha.slice(0, 12)}, not the ` + + `${named} you named. If you reviewed an earlier head, review the one it has now, then name it. Nothing was merged`); + } + const rule = await gitea.branchProtection(owner, repo, pull.base); + // Only into a branch whose protection requires the checks: a pull request whose base was moved to a + // branch nothing guards would merge past every check (novox/hq ADR 0266). + if (!rule || !rule.enable_status_check || !(rule.status_check_contexts ?? []).filter(Boolean).length) { + throw new Refused(`${owner}/${repo}#${number} merges into ${pull.base}, which no protection rule requiring ` + + `the checks guards: a tool merges only into such a branch (novox/hq ADR 0266). Nothing was merged`); + } + // **The verdict must be judged against the trunk's head** (novox/hq ADR 0323 §5, issue 341): a check judges the + // head merged into a named trunk commit, and a merge of another trunk is not the one that check judged. Compared + // here at the moment the merge is asked, not left to the statuses the trunk's move resets a pass later. When they + // differ nothing is merged, and the head's check against the trunk's head is asked once — the announcement a new + // head makes — never again while one is on its way. The caller asks again at the reviewed head once it passes. + const trunkHead = await gitea.branchHead(owner, repo, pull.base); + 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"; + const took = tookWords(tookFile(deps.stateDir), `${owner}/${repo}`); + const head = twelve(pull.head_sha), now = twelve(trunkHead); + if (at.onItsWay) { + throw new Refused(`${owner}/${repo}#${number} is not merged: its verdict ${judged}, and ${pull.base} is now ` + + `${now}. Its check is already asked and not yet answered (${checks[CHECK_CONTEXT]?.description}), so it is not ` + + `asked again; ${took}. Ask again at ${head} once both statuses pass: this tool never merges by itself ` + + `(novox/hq ADR 0323). A check that never answers is asked again with mesh-delivery.recheck. Nothing was merged`); + } + // Pending first, so a second ask finds the check on its way; then the head is announced as a new head is. An + // announcement that cannot be made puts the statuses back to out of date, so the next ask asks again rather + // than wait for ever on a check nobody asked. + const contexts = [CHECK_CONTEXT, ...(checks[REPO_CHECK_CONTEXT] ? [REPO_CHECK_CONTEXT] : [])]; + // Through the module's one writer of the merge check (gate.ts, novox/hq issue 456): the pending opens a new + // check of the head, so where it runs is said ("building on …") and its verdict is not weighed as older. + const say = async (description: string) => { + for (const context of contexts) { + await deps.gates.rechecked(owner, repo, String(pull.head_sha), { state: "pending", context, description, + ...(pull.html_url ? { target_url: pull.html_url } : {}) }); + } + }; + await say(askedWords(deps.now(), trunkHead)); + try { + 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(() => {}); + 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`); + } + throw new Refused(`${owner}/${repo}#${number} is not merged: its verdict ${judged}, and ${pull.base} is now ` + + `${now}. The check of head ${head} merged into trunk ${now} was asked; ${took}. Ask again at ${head} once both ` + + `statuses pass: this tool never merges by itself (novox/hq ADR 0323). In a busy repository another merge may ` + + `move the trunk again first; this is then refused and asked once more. A check that never answers is asked ` + + `again with mesh-delivery.recheck. Nothing was merged`); + } + const statuses = newestByContext((await gitea.combinedStatus(owner, repo, pull.head_sha)).statuses); + const verdict = mergeableUnder(pull.base, rule, statuses); + if (verdict.mergeable !== true) { + 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)`); + } + 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. + // From here the merge has happened. Nothing below may read as if it had not: a read-back that fails or cannot + // be done is said beside merged: true, and only a proven other head is an error — one that says the merge happened. + const answer: Record = { merged: true, number, method, head: pull.head_sha, deleted_branch: deleteBranch }; + let landed: string | undefined; + try { + const merged = await gitea.getPullRequest(owner, repo, number); + answer.merge_commit_sha = merged.merge_commit_sha; + // What landed, read back from the commit the merge made rather than from the forge's record of the pull + // request, which is the very thing a race would leave behind. A merge commit's second parent is the head it + // joined; a fast-forward lands the head itself. A squash or a rebase makes new commits that name no head. + if (!merged.merge_commit_sha) { + answer.read_back = "the forge named no merge commit, so what landed cannot be read back"; + } else if (method === "merge") { + const parents = await gitea.commitParents(owner, repo, merged.merge_commit_sha); + if (parents.length === 2) landed = parents[1]; + else answer.read_back = `the merge commit has ${parents.length} parent(s), not two: what landed cannot be read back`; + } else if (method === "fast-forward-only") { + // That the forge names the head itself as a fast-forward's merge commit is assumed, not seen on this forge: + // a different sha is said, not raised as another head having landed. + if (merged.merge_commit_sha === pull.head_sha) landed = merged.merge_commit_sha; + else answer.read_back = `the forge named ${merged.merge_commit_sha.slice(0, 12)} as this fast-forward's commit, ` + + `not the head: that the head landed could not be confirmed`; + } else { + answer.read_back = `a ${method} makes new commits that name no head: what landed cannot be read back`; + } + } catch (err) { + answer.read_back = `the merge happened, but reading back what landed failed: ${(err as Error).message}`; + } + // Loud, not a field beside merged: true that a caller reading only that would miss. The merge cannot be taken + // back by this tool; what must not happen is that it reads as the merge that was asked for. + if (landed !== undefined && landed !== pull.head_sha) { + throw new Error(`${owner}/${repo}#${number} WAS merged (${answer.merge_commit_sha}), but the forge merged ` + + `${landed.slice(0, 12)}, not the ${pull.head_sha.slice(0, 12)} it was handed and you reviewed: a commit nobody ` + + `reviewed has landed. Read it before anything is built from it (novox/hq issue 381)`); + } + if (landed !== undefined) answer.landed = landed; + return answer; + }, + }; +} diff --git a/modules/gitea/pulls.ts b/modules/gitea/pulls.ts index 4e0ec815..a51156a1 100644 --- a/modules/gitea/pulls.ts +++ b/modules/gitea/pulls.ts @@ -59,6 +59,12 @@ export interface Checked { plan?: ChangePlan; /** Set on a delivery group's composed check (novox/hq ADR 0239): not this head's merge gate. */ group?: string; + /** The trunk commit the head was merged into and judged as, twelve characters (novox/hq ADR 0323 §3); absent from a + * controller whose build seat judged the head alone. Beside are the other repositories' commits the check read, + * by directory; took how long it ran. */ + trunk?: string; + beside?: Record; + took?: string; } /** A change plan, as the controller says it (mesh-controller internal/link, ChangePlan). */ @@ -199,17 +205,25 @@ export function statusFor(c: Checked): CommitStatus { const description = builds(c.plan) ? described(gate.verdict, `${c.plan!.summary}; ${gate.summary}`) : described(gate.verdict, gate.summary, gate.modules, gate.dependents); - return { state: stateOf(gate.verdict), context: CHECK_CONTEXT, description }; + return judgedFirst(c, { state: stateOf(gate.verdict), context: CHECK_CONTEXT, description }); } /** Every status a verdict sets: the gate's, and the repository's own check's when it was said. */ export function statusesFor(c: Checked, facts: RepoCheckFacts = {}): CommitStatus[] { const out = [statusFor(c)]; const repo = c["repo-check"]; - if (repo) out.push(repoStatus(repo, facts)); + if (repo) out.push(judgedFirst(c, repoStatus(repo, facts))); return out; } +/** **A status names what its verdict judged, first** (novox/hq ADR 0323 §3): "head 1a2b3c4d5e6f merged into trunk + * 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 comment a verdict leaves on its pull request, with the check's own account: the change plan of a * change that builds something (novox/hq ADR 0238), and why, when the gate is not a pass or the * repository's own check failed or could not run. A repository with no merge-check.sh, touching nothing, @@ -220,6 +234,12 @@ export function commentFor(c: Checked, facts: RepoCheckFacts = {}): string | nul const wrong = repoWrong(repo, facts); if (gate.verdict === "pass" && !wrong && !builds(c.plan)) return null; const lines = [`**Merge check** at \`${c.commit.slice(0, 8)}\``, ""]; + if (c.trunk) { + // What was judged (novox/hq ADR 0323 §3): the head merged into the trunk commit, and what was read beside it. + const beside = Object.entries(c.beside ?? {}).map(([d, sha]) => `${d} \`${sha}\``).join(", "); + lines.splice(1, 0, `Judged: head \`${c.commit.slice(0, 12)}\` merged into trunk \`${c.trunk.slice(0, 12)}\`` + + (beside ? `; read beside it: ${beside}` : "")); + } 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/test/guard.test.ts b/modules/gitea/test/guard.test.ts index 3e069b63..f14b50ec 100644 --- a/modules/gitea/test/guard.test.ts +++ b/modules/gitea/test/guard.test.ts @@ -7,11 +7,14 @@ import { test, after, beforeEach } from "node:test"; import assert from "node:assert/strict"; import { createServer, type IncomingMessage, type ServerResponse } from "node:http"; -import { readFileSync } from "node:fs"; +import { mkdtempSync, readFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; import { GiteaClient } from "../dist/client.js"; import { ConfiguredToken } from "../dist/token.js"; import { getGiteaTools } from "../dist/tools/index.js"; +import { mergeTool } from "../dist/merge.js"; import { commitStatusTool } from "../dist/delivery.js"; import { GateWriter } from "../dist/gate.js"; import { judgeApiCall, judgeProtectionChange, ruleCovers, routedPath } from "../dist/guard.js"; @@ -21,7 +24,11 @@ interface Sent { method: string; path: string; body: any } const state = { sent: [] as Sent[], asked: [] as string[], - statuses: [] as { context: string; state: string }[], + statuses: [] as { context: string; state: string; description?: string; updated_at?: string }[], + // The trunk's head (novox/hq ADR 0323): what the merge tool compares the verdict's trunk commit with. + trunk: "", + // What the tools said on the bus. + emitted: [] as { type: string; body: any }[], rules: [] as any[], lastCommit: "2026-01-01T00:00:00Z", // The pull request's head; the head the merge commit joined (the same, unless a test says otherwise) — the forge's @@ -36,13 +43,21 @@ const state = { commitsFail: 0, }; const REVIEWED = "abc1234def5678901234567890abcdef12345678"; +const TRUNK = "7a8b9c0d1e2f3a4b5c6d7e8f9a0b1c2d3e4f5a6b"; +const MOVED = "3c4d5e6f7a8b9c0d1e2f3a4b5c6d7e8f9a0b1c2d"; +// A verdict judged against a trunk commit, as both statuses say it (novox/hq ADR 0323 §3). +const judgedAt = (trunk: string, state = "success") => [ + { context: "mesh/merge-gate", state, description: `head ${REVIEWED.slice(0, 12)} merged into trunk ${trunk.slice(0, 12)} — pass: x` }, + { context: "mesh/repo-check", state, description: `head ${REVIEWED.slice(0, 12)} merged into trunk ${trunk.slice(0, 12)} — pass: y` }]; const protectedMain = () => ({ rule_name: "main", enable_push: false, enable_status_check: true, status_check_contexts: ["mesh/merge-gate", "mesh/repo-check"], block_admin_merge_override: true }); beforeEach(() => { state.sent = []; state.asked = []; - state.statuses = [{ context: "mesh/merge-gate", state: "success" }, { context: "mesh/repo-check", state: "success" }]; + state.statuses = judgedAt(TRUNK); + state.trunk = TRUNK; + state.emitted = []; state.rules = [protectedMain()]; state.head = REVIEWED; state.landed = ""; @@ -73,6 +88,15 @@ const server = createServer(async (req, res) => { const repo = "/repos/novox/mesh-catalog"; if (method === "GET" && path === repo) return json(res, 200, { full_name: "novox/mesh-catalog", default_branch: "main" }); if (method === "GET" && path === `${repo}/branch_protections`) return json(res, 200, state.rules); + if (method === "GET" && path === `${repo}/branches/main`) return json(res, 200, { name: "main", commit: { id: state.trunk } }); + if (method === "GET" && path === `${repo}/pulls/7/files`) { + return json(res, 200, url.searchParams.get("page") === "1" ? [{ filename: "modules/gitea/x.ts", status: "modified" }] : []); + } + if (method === "POST" && path === `${repo}/statuses/${state.head}`) { + // Kept as the forge keeps them: the newest of a context first. + state.statuses = [{ ...body, updated_at: new Date().toISOString() }, ...state.statuses.filter((s) => s.context !== body.context)]; + return json(res, 201, body); + } if (method === "GET" && path === "/user") return json(res, 200, { login: "mesh-admin" }); if (method === "GET" && path === "/repos/mesh-admin/scratch/branch_protections") return json(res, 200, []); if (method === "GET" && /^\/repos\/[^/]+\/[^/]+\/commits$/.test(path)) { @@ -102,11 +126,19 @@ after(() => new Promise((r) => server.close(() => r()))); const { port } = server.address() as { port: number }; const client = new GiteaClient(`http://127.0.0.1:${port}`, new ConfiguredToken("t")); // gitea_commit_status is served from the watcher's bundle, beside the merge check's one writer (novox/hq issue 456). +const stateDir = mkdtempSync(join(tmpdir(), "gitea-tools-")); +// The merge check's one writer, as the watcher's bundle holds it: what the merge tool's pending goes through. +const writer = new GateWriter({ setCommitStatus: (o, r, sha, s) => client.setCommitStatus(o, r, sha, s), + gateOf: async () => undefined, gateHistory: async () => [], headChecks: async () => ({}) }); +const throughTheWriter: string[] = []; +const gates = { rechecked: (o: string, r: string, sha: string, s: any) => { + throughTheWriter.push(`${sha.slice(0, 12)} ${s.context} ${s.state}`); + return writer.rechecked(o, r, sha, s); +} }; const tools = [...getGiteaTools(client), commitStatusTool({ - setCommitStatus: (o, r, sha, s) => client.setCommitStatus(o, r, sha, s), - gates: new GateWriter({ setCommitStatus: (o, r, sha, s) => client.setCommitStatus(o, r, sha, s), - gateOf: async () => undefined, gateHistory: async () => [] }), -})]; + setCommitStatus: (o, r, sha, s) => client.setCommitStatus(o, r, sha, s), gates: writer, +}), mergeTool(client, { gates, stateDir, emit: async (type, body) => { state.emitted.push({ type, body }); }, + now: () => new Date(2026, 9, 12, 15, 1) })]; const tool = (name: string) => tools.find((t) => t.name === name)!; const writes = () => state.sent.map((s) => `${s.method} ${s.path}`); @@ -189,15 +221,15 @@ test("gitea_branch_protection_set only strengthens", async () => { test("gitea_merge_pull_request merges only a head whose required statuses all succeeded, and only that head", async () => { const merge = () => tool("gitea_merge_pull_request").run({ owner: "novox", repo: "mesh-catalog", number: 7, head: REVIEWED }); - state.statuses = [{ context: "mesh/merge-gate", state: "success" }, { context: "mesh/repo-check", state: "failure" }]; + state.statuses = [judgedAt(TRUNK)[0], judgedAt(TRUNK, "failure")[1]]; await assert.rejects(merge(), /not merged: .*mesh\/repo-check \(failure\)/); - state.statuses = [{ context: "mesh/merge-gate", state: "success" }]; + state.statuses = [judgedAt(TRUNK)[0]]; await assert.rejects(merge(), /mesh\/repo-check \(missing\)/); await assert.rejects(tool("gitea_merge_pull_request").run({ owner: "novox", repo: "mesh-catalog", number: 7, head: REVIEWED, method: "manually-merged" }), /not a merge method/); assert.deepEqual(writes(), []); // The operator said yes and the checks passed: the flow that stays. - state.statuses = [{ context: "mesh/merge-gate", state: "success" }, { context: "mesh/repo-check", state: "success" }]; + state.statuses = judgedAt(TRUNK); await merge(); const sent = state.sent.find((s) => s.path.endsWith("/pulls/7/merge"))!; assert.equal(sent.body.force_merge, false); @@ -205,6 +237,75 @@ test("gitea_merge_pull_request merges only a head whose required statuses all su }); +// **The merge tool refuses a verdict whose trunk commit is not the trunk's head, and asks the check itself** (novox/hq +// ADR 0323 §5, issue 341): node-engine #66 merged at 00:54 on a verdict of 00:49 that judged its head alone, after #65 +// had moved the trunk. Both commits are named; one check is asked, the announcement a new head makes, and not a second +// while it is on its way; the refusal says how long the last check of the repository took; and once the verdict judges +// the trunk's head and passes, the merge goes through — asked again, never by itself. +test("gitea_merge_pull_request refuses a verdict judged against an earlier trunk, and asks the check once", async () => { + const merge = () => tool("gitea_merge_pull_request").run({ owner: "novox", repo: "mesh-catalog", number: 7, head: REVIEWED }); + const { keepTook, tookFile } = await import("../dist/trunk.js"); + keepTook(tookFile(stateDir), "novox/mesh-catalog", { took: "4m31s", at: "2026-10-12T14:49:00Z" }); + + state.trunk = MOVED; // another merge moved the trunk after the verdict + await assert.rejects(merge(), (err: Error) => { + assert.match(err.message, /is not merged: its verdict was judged against trunk 7a8b9c0d1e2f, and main is now 3c4d5e6f7a8b/); + assert.match(err.message, /check of head abc1234def56 merged into trunk 3c4d5e6f7a8b was asked/); + assert.match(err.message, /the last check of novox\/mesh-catalog took 4m31s/); + assert.match(err.message, /never merges by itself/); + return true; + }); + assert.equal(state.merged, false); + assert.equal(state.emitted.length, 1, "one check asked"); + assert.equal(state.emitted[0].type, "pull.updated"); + assert.equal(state.emitted[0].body.head_sha, REVIEWED); + assert.equal(state.emitted[0].body.base, "main"); + assert.deepEqual(state.emitted[0].body.paths, ["modules/gitea/x.ts"]); + // The announcement names the trunk's head, for a verdict no build judges (ADR 0323 §3). + assert.equal(state.emitted[0].body.base_sha, MOVED); + // Its pending went through the merge check's one writer (novox/hq issue 456), not around it. + assert.deepEqual(throughTheWriter.splice(0), ["abc1234def56 mesh/merge-gate pending", "abc1234def56 mesh/repo-check pending"]); + const gate = state.statuses.find((s) => s.context === "mesh/merge-gate")!; + assert.equal(gate.state, "pending"); + assert.match(gate.description!, /^waiting in the build queue since 15:01: asked by the merge tool against trunk 3c4d5e6f7a8b/); + assert.equal(state.statuses.find((s) => s.context === "mesh/repo-check")!.state, "pending"); + + // Asked again while that check is on its way: refused, and nothing is asked a second time. + await assert.rejects(merge(), /already asked and not yet answered .*not asked again; the last check of novox\/mesh-catalog took 4m31s.*mesh-delivery\.recheck/); + assert.equal(state.emitted.length, 1, "no second check while one is on its way"); + state.statuses = [{ ...gate, description: "building on ace since 15:02" }, ...state.statuses.slice(1)]; + await assert.rejects(merge(), /not asked again/); + assert.equal(state.emitted.length, 1); + + // A verdict put out of date by the trunk's move is asked too, once. + state.statuses = judgedAt(TRUNK).map((s) => ({ ...s, state: "pending", + description: "out of date: judged against trunk 7a8b9c0d1e2f, the trunk is now 3c4d5e6f7a8b; judged again on a recheck or when it is merged" })); + await assert.rejects(merge(), /was judged against trunk 7a8b9c0d1e2f, and main is now 3c4d5e6f7a8b\. The check .* was asked/); + assert.equal(state.emitted.length, 2); + + // A verdict that names no trunk commit — a build seat from before the rule — is not taken as the trunk's head. + state.statuses = [{ context: "mesh/merge-gate", state: "success", description: "pass: x" }, + { context: "mesh/repo-check", state: "success", description: "pass: y" }]; + await assert.rejects(merge(), /names no trunk commit it was judged against, and main is now 3c4d5e6f7a8b/); + assert.equal(state.emitted.length, 3); + assert.equal(state.merged, false); + + // An ask that cannot be announced puts the statuses back to out of date, so the next ask asks again. + state.statuses = judgedAt(TRUNK); + const failing = mergeTool(client, { gates, stateDir, emit: async () => { throw new Error("the bus is away"); } }); + await assert.rejects(failing.run({ owner: "novox", repo: "mesh-catalog", number: 7, head: REVIEWED }), + /its check could not be asked: the bus is away\. Nothing was merged/); + assert.match(state.statuses.find((s) => s.context === "mesh/merge-gate")!.description!, /^out of date: /); + await assert.rejects(merge(), /was asked/); + assert.equal(state.emitted.length, 4, "asked again after the announcement failed"); + + // The check judged the merge into the trunk's head and passed: asked again at the reviewed head, it merges. + state.statuses = judgedAt(MOVED); + const done = (await merge()) as Record; + assert.equal(done.merged, true); + assert.equal(state.emitted.length, 4, "a merge asks no check"); +}); + // A merge lands only the head that was reviewed (novox/hq issue 381): mesh-controller#197 was reviewed at one head, // force-pushed, checked green at the new one, and merged there, because the tool merged whatever the head was. test("gitea_merge_pull_request merges only the head it is named, and refuses one that is not the head", async () => { diff --git a/modules/gitea/test/token.test.ts b/modules/gitea/test/token.test.ts index 3bbfc5e9..750c8122 100644 --- a/modules/gitea/test/token.test.ts +++ b/modules/gitea/test/token.test.ts @@ -414,7 +414,8 @@ test("the tools register once there is a way to a token, and the first call mint [ "gitea_list_repos", "gitea_create_repo", "gitea_delete_repo", "gitea_list_issues", "gitea_get_issue", "gitea_create_issue", "gitea_close_issue", "gitea_add_comment", - "gitea_list_pull_requests", "gitea_get_pull_request", "gitea_create_pull_request", "gitea_merge_pull_request", + // gitea_merge_pull_request is served from the watcher's bundle, beside the merge check's one writer (ADR 0323). + "gitea_list_pull_requests", "gitea_get_pull_request", "gitea_create_pull_request", "gitea_close_pull_request", "gitea_reopen_pull_request", "gitea_update_pull_request", diff --git a/modules/gitea/test/trunk.test.ts b/modules/gitea/test/trunk.test.ts new file mode 100644 index 00000000..7e7fdd4b --- /dev/null +++ b/modules/gitea/test/trunk.test.ts @@ -0,0 +1,175 @@ +// **A merge check's verdict holds for the trunk commit it judged** (novox/hq ADR 0323, issue 341): both statuses name +// the head and the trunk commit; a trunk's move puts the merge check of every open pull request into that trunk — and +// of none into another — back to pending with the reason, and runs nothing; a verdict that lands judged against a trunk +// that has moved since is said out of date. + +import { test } from "node:test"; +import assert from "node:assert/strict"; + +import { checkOnItsWay, lateVerdictComment, outOfDateStatuses, outOfDateWords, trunkNamed, verdictAtTrunk, weighTrunks } from "../dist/trunk.js"; +import { GateWriter } from "../dist/gate.js"; +import { statusesFor, commentFor } from "../dist/pulls.js"; + +const HEAD = "dfb1415aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"; +const TRUNK = "a2c2c35bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb"; +const MOVED = "d28ca95ccccccccccccccccccccccccccccccccc"; + +test("both statuses say the head and the trunk commit the verdict judged, first, whatever the summary's length", () => { + const c = { owner: "novox", repo: "mesh-host", number: 66, commit: HEAD, verdict: "pass", summary: "x", id: "build-1", + trunk: TRUNK.slice(0, 12), beside: { "mesh-catalog": "0123456789ab", "mesh-lab": "ba9876543210" }, + gate: { verdict: "pass", summary: "4 of 4 compose ".repeat(20) }, + "repo-check": { verdict: "fail", summary: "its merge-check.sh failed: internal/store/store.go:145:2: Declared redeclared" } }; + const [gate, repo] = statusesFor(c); + for (const s of [gate, repo]) { + assert.ok(s.description.startsWith("head dfb1415aaaaa merged into trunk a2c2c35bbbbb — "), s.description); + assert.ok(s.description.length <= 140, `${s.description.length} characters`); + assert.equal(trunkNamed(s.description), "a2c2c35bbbbb"); + } + assert.equal(gate.state, "success"); + assert.equal(repo.state, "failure"); + const comment = commentFor(c)!; + assert.match(comment, /Judged: head `dfb1415aaaaa` merged into trunk `a2c2c35bbbbb`; read beside it: mesh-catalog `0123456789ab`, mesh-lab `ba9876543210`/); + + // A verdict of a build seat from before the rule names no trunk, and is said as before. + const [old] = statusesFor({ ...c, trunk: undefined, gate: { verdict: "pass", summary: "every machine composes" } }); + assert.equal(old.description, "pass: every machine composes"); + assert.equal(trunkNamed(old.description), undefined); +}); + +test("a trunk's move puts an open head's merge check out of date, naming both commits, unless a check is on its way", () => { + const verdict = (trunk: string, state = "success") => ({ + "mesh/merge-gate": { state, description: `head ${HEAD.slice(0, 12)} merged into trunk ${trunk.slice(0, 12)} — pass: x` }, + "mesh/repo-check": { state, description: `head ${HEAD.slice(0, 12)} merged into trunk ${trunk.slice(0, 12)} — pass: y` } }); + const put = outOfDateStatuses(verdict(TRUNK), MOVED); + assert.deepEqual(put.map((s) => [s.context, s.state]), [["mesh/merge-gate", "pending"], ["mesh/repo-check", "pending"]]); + assert.equal(put[0].description, "out of date: judged against trunk a2c2c35bbbbb, the trunk is now d28ca95ccccc; " + + "judged again on a recheck or when it is merged"); + assert.equal(put[0].description, outOfDateWords(TRUNK, MOVED)); + assert.equal(trunkNamed(put[0].description), "a2c2c35bbbbb", "an out-of-date status still names what it judged"); + // A failure is out of date too: it judged another merge. + assert.equal(outOfDateStatuses(verdict(TRUNK, "failure"), MOVED).length, 2); + // Already judged against the trunk's head; a check on its way or already out of date; no check yet: left alone. + assert.deepEqual(outOfDateStatuses(verdict(MOVED), MOVED), []); + assert.deepEqual(outOfDateStatuses({ "mesh/merge-gate": { state: "pending", description: "building on ace since 14:02" } }, MOVED), []); + assert.deepEqual(outOfDateStatuses({ "mesh/merge-gate": { state: "pending", description: put[0].description } }, MOVED), []); + assert.deepEqual(outOfDateStatuses({}, MOVED), []); + // A head with no repository check of its own gets no pending one a verdict would never set. + assert.deepEqual(outOfDateStatuses({ "mesh/merge-gate": verdict(TRUNK)["mesh/merge-gate"] }, MOVED).map((s) => s.context), + ["mesh/merge-gate"]); + // A verdict naming no trunk, of a build seat from before the rule, judged an earlier trunk. + assert.match(outOfDateStatuses({ "mesh/merge-gate": { state: "success", description: "pass: x" } }, MOVED)[0].description, + /judged against a trunk it does not name, the trunk is now d28ca95ccccc/); + + assert.ok(checkOnItsWay({ state: "pending", description: "waiting in the build queue since 14:00" })); + assert.ok(checkOnItsWay({ state: "pending", description: "checking again: a person asked" })); + assert.ok(!checkOnItsWay({ state: "pending", description: put[0].description })); + assert.deepEqual(verdictAtTrunk(verdict(MOVED), MOVED), { current: true, onItsWay: false, judged: "d28ca95ccccc" }); + assert.equal(verdictAtTrunk(verdict(TRUNK), MOVED).current, false); +}); + +/** A forge holding open pull requests into two branches, each head's statuses, and what was written. */ +function aForge(pulls: { number: number; base: string; head: string }[], heads: Record) { + const statuses = new Map>(); + const writes: string[] = []; + const verdict = (trunk: string) => ({ + "mesh/merge-gate": { state: "success", description: `head x merged into trunk ${trunk.slice(0, 12)} — pass` }, + "mesh/repo-check": { state: "success", description: `head x merged into trunk ${trunk.slice(0, 12)} — pass` } }); + for (const p of pulls) statuses.set(p.head, verdict(heads[p.base])); + const forge = { + listOpenPulls: async () => pulls.map((p) => ({ number: p.number, title: "t", state: "open", merged: false, base: p.base, + head: "feat", head_sha: p.head, html_url: `http://forge/pulls/${p.number}` })), + branchHead: async (_o: string, _r: string, branch: string) => heads[branch], + setCommitStatus: async (_o: string, _r: string, sha: string, s: { context: string; state: string; description: string }) => { + writes.push(`${sha} ${s.context} ${s.state}`); + statuses.set(sha, { ...statuses.get(sha), [s.context]: { state: s.state, description: s.description } }); + }, + gateOf: async () => undefined, + gateHistory: async () => [], + headChecks: async (_o: string, _r: string, sha: string) => statuses.get(sha) ?? {}, + }; + return { forge, writes, statuses }; +} + +test("a push to a trunk puts every open pull request into it out of date — twenty-five of them — and none into another", async () => { + const heads: Record = { main: TRUNK, release: "eeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee" }; + const pulls = Array.from({ length: 25 }, (_, i) => ({ number: i + 1, base: "main", head: `${String(i).padStart(2, "0")}`.repeat(20) })); + pulls.push({ number: 99, base: "release", head: "9".repeat(40) }); + const { forge, writes, statuses } = aForge(pulls, heads); + const gates = new GateWriter(forge); + const trunks: Record = {}; + + // First sight: each trunk is taken as it is, and nothing is put out of date. + assert.equal(await weighTrunks(forge, gates, trunks, "novox", "mesh-host"), true); + assert.deepEqual(writes, []); + assert.equal(trunks["novox/mesh-host:main"], TRUNK); + // Nothing moved: nothing read past the trunks, nothing written. + assert.equal(await weighTrunks(forge, gates, trunks, "novox", "mesh-host"), false); + + heads.main = MOVED; + assert.equal(await weighTrunks(forge, gates, trunks, "novox", "mesh-host"), true); + assert.equal(writes.length, 50, "both statuses of each of the 25 pull requests into main"); + for (const p of pulls.filter((p) => p.base === "main")) { + const s = statuses.get(p.head)!; + assert.equal(s["mesh/merge-gate"].state, "pending"); + assert.match(s["mesh/merge-gate"].description!, /judged against trunk a2c2c35bbbbb, the trunk is now d28ca95ccccc/); + assert.equal(s["mesh/repo-check"].state, "pending"); + } + assert.equal(statuses.get("9".repeat(40))!["mesh/merge-gate"].state, "success", "a pull request into another branch is left"); + assert.equal(trunks["novox/mesh-host:main"], MOVED); + + // Weighed again, nothing more: the move was taken once, and the out-of-date statuses are left as they are. + writes.length = 0; + heads.main = "f".repeat(40); + await weighTrunks(forge, gates, trunks, "novox", "mesh-host"); + assert.deepEqual(writes, [], "a check already out of date is not written again"); +}); + +test("a trunk that cannot be read is weighed again on the next pass, never recorded half", async () => { + const heads: Record = { main: TRUNK }; + const { forge } = aForge([{ number: 1, base: "main", head: "1".repeat(40) }], heads); + const trunks: Record = { "novox/mesh-host:main": TRUNK }; + heads.main = MOVED; + const failing = { ...forge, setCommitStatus: async () => { throw new Error("the forge is away"); } }; + assert.equal(await weighTrunks(failing, new GateWriter(failing), trunks, "novox", "mesh-host"), false); + assert.equal(trunks["novox/mesh-host:main"], TRUNK, "not recorded: the next pass weighs it again"); +}); + +test("the trunk's move takes its turn after a verdict in flight, and weighs what the forge holds then", async () => { + const heads: Record = { main: TRUNK }; + const { forge, statuses } = aForge([{ number: 1, base: "main", head: HEAD }], heads); + const gates = new GateWriter(forge); + // A verdict judged against the trunk's new head, in flight when the move is weighed: it lands first, and is left. + let release!: () => void; + const held = new Promise((r) => (release = r)); + const verdictLanded = gates.verdict("novox", "mesh-host", HEAD, async () => { + await held; + statuses.set(HEAD, { "mesh/merge-gate": { state: "success", description: `head x merged into trunk ${MOVED.slice(0, 12)} — pass` } }); + }); + const moved = gates.outOfDate("novox", "mesh-host", HEAD, (checks) => outOfDateStatuses(checks, MOVED)); + release(); + await verdictLanded; + assert.deepEqual(await moved, [], "a verdict judged against the trunk's head is not put out of date"); + assert.equal(statuses.get(HEAD)!["mesh/merge-gate"].state, "success"); +}); + +test("a verdict judged against a trunk newer than the pass's first read is not put out of date", async () => { + const NEWER = "ffffffffffffffffffffffffffffffffffffffff"; + const heads: Record = { main: TRUNK }; + const { forge, statuses } = aForge([{ number: 1, base: "main", head: HEAD }], heads); + const trunks: Record = { "novox/mesh-host:main": TRUNK }; + // The pass reads main at MOVED; by the head's turn the trunk moved again, and the head's check answered for it. + let reads = 0; + const moving = { ...forge, branchHead: async () => (reads++ === 0 ? MOVED : NEWER) }; + statuses.set(HEAD, { "mesh/merge-gate": { state: "success", description: `head x merged into trunk ${NEWER.slice(0, 12)} — pass` } }); + await weighTrunks(moving, new GateWriter(moving), trunks, "novox", "mesh-host"); + assert.equal(statuses.get(HEAD)!["mesh/merge-gate"].state, "success", "a fresh verdict was put out of date"); +}); + +test("a verdict that landed out of date keeps its account on the pull request", () => { + const words = outOfDateWords(TRUNK, MOVED); + const own = lateVerdictComment({ commit: HEAD, trunk: TRUNK.slice(0, 12), verdict: "fail" }, "**Merge check** at `dfb1415a`\n\nthe report", words); + assert.match(own, /the report/); + assert.match(own, /\*\*Out of date when it landed\*\*: out of date: judged against trunk a2c2c35bbbbb/); + const bare = lateVerdictComment({ commit: HEAD, trunk: TRUNK.slice(0, 12), verdict: "pass" }, null, words); + assert.match(bare, /head dfb1415aaaaa merged into trunk a2c2c35bbbbb, pass/); +}); diff --git a/modules/gitea/tools/index.ts b/modules/gitea/tools/index.ts index ede929c7..201b08d0 100644 --- a/modules/gitea/tools/index.ts +++ b/modules/gitea/tools/index.ts @@ -63,6 +63,8 @@ export function getGiteaTools(gitea: GiteaClient): ToolDefinition[] { return namesChecked(allGiteaTools(gitea)); } +// gitea_merge_pull_request is served from the watcher's bundle (merge.ts, index.ts), beside the merge check's one +// writer (novox/hq issue 456, ADR 0323). function allGiteaTools(gitea: GiteaClient): ToolDefinition[] { return [ // ---- Repositories ---- @@ -263,111 +265,6 @@ function allGiteaTools(gitea: GiteaClient): ToolDefinition[] { }), }), }, - { - name: "gitea_merge_pull_request", - description: "Merge a pull request at the head you name — the commit you reviewed — when that is still its head " + - "and every status its base branch's protection requires succeeded there (mesh/merge-gate, mesh/repo-check), " + - "optionally deleting the source branch afterwards. Refused otherwise, with nothing merged: no tool merges past " + - "a check, and none merges a head that moved after it was reviewed.", - input: { - owner: { type: "string", description: "the repository owner" }, - repo: { type: "string", description: "the repository name" }, - number: { type: "number", description: "the PR number" }, - head: { type: "string", description: "the commit you reviewed, as its sha or at least its first 12 characters: " + - "refused unless it is the pull request's head" }, - method: { type: "string", description: "merge (default) | fast-forward-only | squash | rebase | rebase-merge. " + - "Only merge and fast-forward-only can be read back to prove the head you named is what landed" }, - delete_branch: { type: "boolean", description: "delete the source branch after merge (default true)" }, - }, - run: async (args) => { - const owner = String(args.owner); - const repo = String(args.repo); - const number = Number(args.number); - const method = args.method ? String(args.method) : "merge"; - if (!MERGE_METHODS.has(method)) throw new Refused(`${method} is not a merge method: ${[...MERGE_METHODS].join(", ")}`); - const deleteBranch = args.delete_branch === undefined ? true : Boolean(args.delete_branch); - // **Only a head whose required statuses all succeeded** (novox/hq ADR 0266): read here rather than left - // to the forge, whose administrator — the account this tool's token is minted for — may merge past a - // rule that lets it. A rule that cannot be read is no rule passed. - // **Only the head that was reviewed** (novox/hq issue 381): mesh-controller#197 was reviewed at 065cfa0d, - // force-pushed to 3ced96f, checked green there, and merged at 3ced96f, because this tool merged whatever the - // head was and nothing told it which head the review had approved. The caller names it; a head that moved - // since is refused, and the forge is handed the same head, so a push between this read and the merge is - // refused by the forge too. - const named = args.head === undefined ? "" : String(args.head).trim().toLowerCase(); - if (!named) { - throw new Refused(`${owner}/${repo}#${number} is not merged: name the head you reviewed (head), so that only ` + - `that commit can land (novox/hq issue 381). Nothing was merged`); - } - // Twelve characters, not seven: an author who force-pushes can make a commit whose first seven match the - // reviewed one in seconds, and the review would then merge it. Twelve cannot be matched that way. - if (!REVIEWED_HEAD.test(named)) { - throw new Refused(`${owner}/${repo}#${number} is not merged: head ${named} does not name one commit — give its ` + - `sha, or at least its first 12 characters. Nothing was merged`); - } - const pull = await gitea.getPullRequest(owner, repo, number); - if (pull.merged) throw new Refused(`${owner}/${repo}#${number} is already merged`); - if (!pull.head_sha || !pull.base) throw new Refused(`${owner}/${repo}#${number} names no head commit or base branch`); - if (!pull.head_sha.toLowerCase().startsWith(named)) { - throw new Refused(`${owner}/${repo}#${number} is not merged: its head is ${pull.head_sha.slice(0, 12)}, not the ` + - `${named} you named. If you reviewed an earlier head, review the one it has now, then name it. Nothing was merged`); - } - const rule = await gitea.branchProtection(owner, repo, pull.base); - // Only into a branch whose protection requires the checks: a pull request whose base was moved to a - // branch nothing guards would merge past every check (novox/hq ADR 0266). - if (!rule || !rule.enable_status_check || !(rule.status_check_contexts ?? []).filter(Boolean).length) { - throw new Refused(`${owner}/${repo}#${number} merges into ${pull.base}, which no protection rule requiring ` + - `the checks guards: a tool merges only into such a branch (novox/hq ADR 0266). Nothing was merged`); - } - const statuses = newestByContext((await gitea.combinedStatus(owner, repo, pull.head_sha)).statuses); - const verdict = mergeableUnder(pull.base, rule, statuses); - if (verdict.mergeable !== true) { - 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)`); - } - 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. - // From here the merge has happened. Nothing below may read as if it had not: a read-back that fails or cannot - // be done is said beside merged: true, and only a proven other head is an error — one that says the merge happened. - const answer: Record = { merged: true, number, method, head: pull.head_sha, deleted_branch: deleteBranch }; - let landed: string | undefined; - try { - const merged = await gitea.getPullRequest(owner, repo, number); - answer.merge_commit_sha = merged.merge_commit_sha; - // What landed, read back from the commit the merge made rather than from the forge's record of the pull - // request, which is the very thing a race would leave behind. A merge commit's second parent is the head it - // joined; a fast-forward lands the head itself. A squash or a rebase makes new commits that name no head. - if (!merged.merge_commit_sha) { - answer.read_back = "the forge named no merge commit, so what landed cannot be read back"; - } else if (method === "merge") { - const parents = await gitea.commitParents(owner, repo, merged.merge_commit_sha); - if (parents.length === 2) landed = parents[1]; - else answer.read_back = `the merge commit has ${parents.length} parent(s), not two: what landed cannot be read back`; - } else if (method === "fast-forward-only") { - // That the forge names the head itself as a fast-forward's merge commit is assumed, not seen on this forge: - // a different sha is said, not raised as another head having landed. - if (merged.merge_commit_sha === pull.head_sha) landed = merged.merge_commit_sha; - else answer.read_back = `the forge named ${merged.merge_commit_sha.slice(0, 12)} as this fast-forward's commit, ` + - `not the head: that the head landed could not be confirmed`; - } else { - answer.read_back = `a ${method} makes new commits that name no head: what landed cannot be read back`; - } - } catch (err) { - answer.read_back = `the merge happened, but reading back what landed failed: ${(err as Error).message}`; - } - // Loud, not a field beside merged: true that a caller reading only that would miss. The merge cannot be taken - // back by this tool; what must not happen is that it reads as the merge that was asked for. - if (landed !== undefined && landed !== pull.head_sha) { - throw new Error(`${owner}/${repo}#${number} WAS merged (${answer.merge_commit_sha}), but the forge merged ` + - `${landed.slice(0, 12)}, not the ${pull.head_sha.slice(0, 12)} it was handed and you reviewed: a commit nobody ` + - `reviewed has landed. Read it before anything is built from it (novox/hq issue 381)`); - } - if (landed !== undefined) answer.landed = landed; - return answer; - }, - }, - { name: "gitea_close_pull_request", description: "Close a pull request without merging it — one whose work landed elsewhere, or was abandoned.", diff --git a/modules/gitea/trunk.ts b/modules/gitea/trunk.ts new file mode 100644 index 00000000..1f0b1a6a --- /dev/null +++ b/modules/gitea/trunk.ts @@ -0,0 +1,250 @@ +// **A merge check's verdict holds for the trunk commit it judged** (novox/hq ADR 0323, issue 341). +// +// The build seat checks a pull request's head merged into its trunk's head at that moment — the verdict's trunk +// commit — and the verdict names both: "head 1a2b3c4d5e6f merged into trunk 7a8b9c0d1e2f", in both statuses' +// descriptions. When the trunk moves, every open pull request's verdict into it is out of date: +// +// - **the forge's holder** (index.ts) hears the move on its own — it reads each trunk's head on every pass that +// looks at the repository, never through mesh-delivery (ADR 0239 §36) — and sets mesh/merge-gate and +// mesh/repo-check on the head of every open pull request into that trunk back to pending, with the reason. +// Nothing is run for it: a recheck, a new head, or the merge tool asks the check again; +// - **the merge tool** (tools/index.ts) compares the trunk commit the head's verdict names with the trunk's head at +// the moment it is asked, merges nothing when they differ, asks the head's check once — the same announcement a +// new head makes — and says how long the last check of that repository took. It never merges by itself after. +// +// Pure functions and two small files 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, GiteaClient, GiteaPull, HeadCheck } from "./client.js"; +import { CHECK_CONTEXT, REPO_CHECK_CONTEXT } from "./pulls.js"; + +/** A commit as a verdict names it: its first twelve characters. */ +export function twelve(sha: string | undefined): string { + return String(sha ?? "").slice(0, 12).toLowerCase(); +} + +/** Whether two commits are one, however either is abbreviated (seven characters at least). */ +export function sameCommit(a: string | undefined, b: string | undefined): boolean { + const x = String(a ?? "").toLowerCase(), y = String(b ?? "").toLowerCase(); + if (x.length < 7 || y.length < 7) return false; + return x.length <= y.length ? y.startsWith(x) : x.startsWith(y); +} + +/** What a verdict judged, as both statuses' descriptions begin (ADR 0323 §3). */ +export function judgedWords(head: string, trunk: string): string { + return `head ${twelve(head)} merged into trunk ${twelve(trunk)}`; +} + +/** The trunk commit a merge check's status names: the one its verdict judged ("merged into trunk …"), or, for a + * verdict put out of date, the one it had judged ("judged against trunk …"). Undefined when it names none, as a + * verdict of a build seat from before ADR 0323 does. */ +export function trunkNamed(description: string | undefined): string | undefined { + const m = /(?:merged into|judged against) trunk ([0-9a-f]{7,64})\b/i.exec(description ?? ""); + return m ? m[1].toLowerCase() : undefined; +} + +/** A verdict out of date, as its statuses say it (ADR 0323 §4). */ +export function outOfDateWords(judged: string | undefined, now: string): string { + const was = judged ? `judged against trunk ${twelve(judged)}` : "judged against a trunk it does not name"; + return `out of date: ${was}, the trunk is now ${twelve(now)}; judged again on a recheck or when it is merged`; +} + +/** Whether a pending gate says a check of the head is asked and not yet answered: waiting in the build queue, + * building on a machine (gate.ts), or asked again by a recheck (mesh-delivery). Such a check is not asked again. */ +export function checkOnItsWay(gate: HeadCheck | undefined): boolean { + if (!gate || gate.state !== "pending") return false; + const d = gate.description ?? ""; + return d.startsWith("waiting in the build queue") || d.startsWith("building on ") || d.startsWith("checking again"); +} + +/** + * The statuses a trunk's move puts back to pending on one open pull request's head, from what the forge holds on it + * now: none when it has no merge check yet, when a check is pending (on its way, or already out of date — its + * verdict is weighed against the trunk when it lands), or when its verdict judged the trunk's head. Otherwise the + * gate, and the repository's own check when the head has one — a pending repo-check a verdict will not set would + * stand for ever (novox/hq issue 305's rule). + */ +export function outOfDateStatuses(checks: Record, trunkHead: string): CommitStatus[] { + const gate = checks[CHECK_CONTEXT]; + if (!gate || gate.state === "pending") return []; + const judged = trunkNamed(gate.description); + if (judged && sameCommit(judged, trunkHead)) return []; + const description = outOfDateWords(judged, trunkHead); + 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; +} + +/** Where the merge tool stands with a head's verdict and the trunk's head (ADR 0323 §5). */ +export interface VerdictAtTrunk { + /** The verdict judged the trunk's head: the required statuses decide, as before. */ + current: boolean; + /** A check of the head is asked and not yet answered: nothing is asked again. */ + onItsWay: boolean; + /** The trunk commit the verdict names, when it names one. */ + judged?: string; +} + +export function verdictAtTrunk(checks: Record, trunkHead: string): VerdictAtTrunk { + const gate = checks[CHECK_CONTEXT]; + const judged = trunkNamed(gate?.description); + if (checkOnItsWay(gate)) return { current: false, onItsWay: true, judged }; + return { current: !!gate && gate.state !== "pending" && !!judged && sameCommit(judged, trunkHead), onItsWay: false, judged }; +} + +/** The pending words the merge tool's ask of a check is said with: in the build queue from now, as an announced head's. */ +export function askedWords(at: Date, trunkHead: string): string { + const two = (n: number) => String(n).padStart(2, "0"); + return `waiting in the build queue since ${two(at.getHours())}:${two(at.getMinutes())}: asked by the merge tool ` + + `against trunk ${twelve(trunkHead)}`; +} + +/** The comment a verdict that landed out of date leaves (ADR 0323 §7: kept and shown): its own account when it has + * one, else what it judged and said, and that it is out of date. */ +export function lateVerdictComment(c: { commit: string; trunk?: string; verdict: string }, account: string | null, + outOfDate: string): string { + const own = account ?? `**Merge check** at \`${c.commit.slice(0, 8)}\`: ${judgedWords(c.commit, c.trunk ?? "")}, ${c.verdict}`; + return `${own}\n\n**Out of date when it landed**: ${outOfDate}`; +} + +// --- a trunk's move, weighed -------------------------------------------------------------------------------------- + +/** What weighing a trunk's move reads of the forge. */ +export interface TrunkForge { + listOpenPulls(owner: string, repo: string): Promise; + branchHead(owner: string, repo: string, branch: string): Promise; +} + +/** What it writes with: the merge check's one writer (gate.ts), which decides in the head's own turn. */ +export interface OutOfDateWriter { + outOfDate(owner: string, repo: string, sha: string, + decide: (checks: Record) => CommitStatus[] | Promise, target?: string): Promise; +} + +/** + * Every trunk of one repository that an open pull request merges into, read; for one that moved since it was last read + * (`trunks`, by `owner/repo:branch`, which this keeps current), the merge check of every open pull request into it put + * 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. + */ +export async function weighTrunks(forge: TrunkForge, gates: OutOfDateWriter, trunks: Record, owner: string, + name: string): Promise { + let changed = false; + try { + const open = (await forge.listOpenPulls(owner, name)).filter((p) => p.state === "open" && !!p.head_sha && !!p.base); + for (const base of new Set(open.map((p) => String(p.base)))) { + const key = `${owner}/${name}:${base}`.toLowerCase(); + const head = await forge.branchHead(owner, name, base); + const before = trunks[key]; + if (before && sameCommit(before, head)) continue; + if (before) { + let put = 0; + for (const pull of open.filter((p) => p.base === base)) { + // The trunk is read again in the head's own turn: a verdict judged against a head newer than the pass's first + // read — the trunk moved again meanwhile, and a check answered — is not put out of date. + const written = await gates.outOfDate(owner, name, String(pull.head_sha), + async (checks) => outOfDateStatuses(checks, await forge.branchHead(owner, name, base)), pull.html_url || undefined); + if (written.length) put++; + } + 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)`); + } + trunks[key] = head; + changed = true; + } + } catch (err) { + console.error(`[gitea] ${owner}/${name}: whether a trunk moved could not be weighed, so it is weighed again on the next ` + + `pass — ${err instanceof Error ? err.message : err}`); + } + return changed; +} + +// --- how long the last check of a repository took ------------------------------------------------------------- + +/** How long one repository's last check took, and when it was heard: kept by the watcher, read by the merge tool. */ +export interface Took { + took: string; + at: string; + head?: string; +} + +/** The file the watcher keeps each repository's last check's duration in, beside its other records. */ +export function tookFile(stateDir: string | undefined): string | null { + return stateDir ? `${stateDir.replace(/\/+$/, "")}/checks-took.json` : null; +} + +function readTookAll(file: string | null): Record { + if (!file || !existsSync(file)) return {}; + try { + return JSON.parse(readFileSync(file, "utf8")) as Record; + } catch { + return {}; + } +} + +/** Keep how long a repository's check took. A record that cannot be written is said, never fatal to the verdict. */ +export function keepTook(file: string | null, repository: string, took: Took): void { + if (!file || !took.took) return; + try { + const all = readTookAll(file); + all[repository.toLowerCase()] = took; + mkdirSync(dirname(file), { recursive: true }); + writeFileSync(file + ".tmp", JSON.stringify(all)); + renameSync(file + ".tmp", file); + } catch (err) { + console.error(`[gitea] how long ${repository}'s check took could not be kept — ${err instanceof Error ? err.message : err}`); + } +} + +/** How long a repository's last check took, in words for the merge tool's refusal. */ +export function tookWords(file: string | null, repository: string): string { + const t = readTookAll(file)[repository.toLowerCase()]; + if (!t) return `how long a check of ${repository} takes is not known yet: no verdict of it was heard since this was kept`; + return `the last check of ${repository} took ${t.took} (heard ${t.at})`; +} + +// --- the announcement of a head, shared by the watcher and the merge tool --------------------------------------- + +/** `pull.updated` for one open pull request's head: what the controller asks the build seat to check (novox/hq to-be + * 45 §9). The merge tool's ask is this same announcement (ADR 0323 §5). */ +export async function announcementOf(client: GiteaClient, owner: string, name: string, cloneURL: string | undefined, + pull: GiteaPull): Promise> { + const files = await client.listPullFiles(owner, name, pull.number); + const head = String(pull.head_sha); + // What the controller maps the change onto the mesh's module graph with (novox/hq ADR 0237 as amended): which + // changed directories hold a module at the head — the merge's rule (issue 278) — and whether the head holds the + // repository's own merge-check.sh. Not said when it could not be read; the controller then reads the change as + // touching everything built from the repository. + let moduleDirs: string[] | null = null; + let mergeCheck: boolean | null = null; + try { + if (!files.truncated) moduleDirs = await client.moduleDirsAt(owner, name, head, files.paths); + mergeCheck = await client.holdsFile(owner, name, head, "merge-check.sh"); + } catch (err) { + console.error(`[gitea] ${owner}/${name}#${pull.number}: what the head holds could not be read — ${err instanceof Error ? err.message : err}`); + } + // The base branch's head as the head is announced: the trunk commit a verdict no build judges is said against + // (novox/hq ADR 0323 §3). Not said when it cannot be read; that verdict then names none. + const baseSHA = pull.base ? await client.branchHead(owner, name, String(pull.base)).catch(() => undefined) : undefined; + return { + owner, + repo: name, + number: pull.number, + ...(baseSHA ? { base_sha: baseSHA } : {}), + title: pull.title, + body: pull.body, + base: pull.base, + head: pull.head, + head_sha: pull.head_sha, + clone_url: cloneURL, + html_url: pull.html_url, + paths: files.paths, + paths_truncated: files.truncated, + removed: files.removed, + ...(moduleDirs ? { module_dirs: moduleDirs, module_dirs_said: true } : {}), + ...(mergeCheck !== null ? { merge_check: mergeCheck, merge_check_said: true } : {}), + }; +} diff --git a/modules/mesh-delivery/cmd/mesh-delivery/checks.go b/modules/mesh-delivery/cmd/mesh-delivery/checks.go index d8936b2d..a737a311 100644 --- a/modules/mesh-delivery/cmd/mesh-delivery/checks.go +++ b/modules/mesh-delivery/cmd/mesh-delivery/checks.go @@ -51,6 +51,10 @@ type ChecksAnswer struct { Statuses []CheckStatus `json:"statuses"` // Verdict is the merge check's verdict whole, as this owner heard it for this commit. Verdict *Verdict `json:"verdict,omitempty"` + // Judged is what the verdict judged: the head merged into the trunk commit (novox/hq ADR 0323 §3); OutOfDate + // why it no longer counts, when its trunk has moved since. + Judged string `json:"judged,omitempty"` + OutOfDate string `json:"out_of_date,omitempty"` // Merge is whether the branch's protection lets the commit merge; Mergeable nil when it is not known. Merge ForgeMerge `json:"merge"` // Forge is why the forge could not be read, when it could not. @@ -107,6 +111,7 @@ func (h *Holder) Checks(repository string, number int, commit string) (*ChecksAn Group: d.Group} if d.Check != nil && sameCommit(d.Commit, a.Commit) { a.Verdict = d.Check + a.Judged, a.OutOfDate = d.Check.Judged(), d.outOfDate() } } @@ -261,6 +266,12 @@ func (a *ChecksAnswer) says() string { default: merge = "NOT mergeable — " + merge } + if a.Judged != "" { + what += " (" + a.Judged + ")" + } + if a.OutOfDate != "" { + merge += "; its verdict is " + a.OutOfDate + } return what + ": " + statuses + "; " + merge } diff --git a/modules/mesh-delivery/cmd/mesh-delivery/covered_test.go b/modules/mesh-delivery/cmd/mesh-delivery/covered_test.go index ef43241b..ca989ff1 100644 --- a/modules/mesh-delivery/cmd/mesh-delivery/covered_test.go +++ b/modules/mesh-delivery/cmd/mesh-delivery/covered_test.go @@ -451,7 +451,7 @@ func TestAMemberHeldForItsComposedCheckIsLiftedWhenItPasses(t *testing.T) { State: Ready, Since: t0, Created: t0.Add(-time.Hour), Plan: planMoving(b, "b"), Check: &Verdict{Gate: "pass"}} g := &Group{ID: "feat/k", Members: []string{aID, bID}, Order: []string{aID, bID}, Closed: true, OrderOf: map[string]string{aID: aHead, bID: bHead}, - Check: &GroupCheck{Asked: "check-1", At: t0, Commits: c.commits}} + Check: &GroupCheck{Asked: "check-old", At: t0, Commits: c.commits}} for _, d := range []*Delivery{da, db} { _ = store.PutDelivery(d) } @@ -463,7 +463,7 @@ func TestAMemberHeldForItsComposedCheckIsLiftedWhenItPasses(t *testing.T) { if s := w.state(aID); s != Held { t.Fatalf("before its composed verdict it is %s", s) } - w.h.Checked(CheckedEvent{Group: "feat/k", ID: "check-1", Verdict: c.verdict, Summary: "composed: " + c.verdict}) + w.h.Checked(CheckedEvent{Group: "feat/k", ID: "check-old", Verdict: c.verdict, Summary: "composed: " + c.verdict}) w.settleAll() if s := w.state(aID); s != c.want { t.Fatalf("its composed check %s, and it is %s (%s), want %s", c.name, s, w.delivery(aID).HeldWhy, c.want) @@ -471,6 +471,10 @@ func TestAMemberHeldForItsComposedCheckIsLiftedWhenItPasses(t *testing.T) { if c.want == Delivering && !containsString(ctl.delivers, "plan-a") { t.Fatalf("lifted, its walk was not let go: %v", ctl.delivers) } + // A composed check of other heads is asked again for these heads (novox/hq ADR 0323 §6). + if c.name == "passed for other heads" && !containsString(ctl.checks, "feat/k:"+aID+","+bID) { + t.Fatalf("the composed check was not asked again for the heads that merged: %v", ctl.checks) + } }) } } diff --git a/modules/mesh-delivery/cmd/mesh-delivery/holder.go b/modules/mesh-delivery/cmd/mesh-delivery/holder.go index 629103ee..04d6589c 100644 --- a/modules/mesh-delivery/cmd/mesh-delivery/holder.go +++ b/modules/mesh-delivery/cmd/mesh-delivery/holder.go @@ -279,6 +279,8 @@ type PullEvent struct { ModuleDirs []string `json:"module_dirs,omitempty"` ModuleDirsSaid bool `json:"module_dirs_said,omitempty"` HeadChecks map[string]string `json:"head_checks,omitempty"` + // BaseSHA is the base branch's head when the head was announced (novox/hq ADR 0323). + BaseSHA string `json:"base_sha,omitempty"` } func (p PullEvent) repository() string { return strings.ToLower(p.Owner + "/" + p.Repo) } @@ -296,10 +298,22 @@ func (h *Holder) PullUpdated(p PullEvent) { h.mu.Lock() defer h.mu.Unlock() id := IDOf(p.repository(), p.HeadSHA) - if d := h.deliveries[id]; d != nil { - return // announced again: nothing new - } now := h.Now() + if d := h.deliveries[id]; d != nil { + // Announced again: nothing new — but for a delivery whose verdict is out of date, the announcement is its + // check asked again against the trunk's head (the merge tool's ask, novox/hq ADR 0323 §5), and its bound + // runs from now (§7). The base head it was announced with is kept, for a check this owner asks after it. + if p.BaseSHA != "" && d.BaseSHA != p.BaseSHA { + d.BaseSHA = p.BaseSHA + h.keep(d) + } + if d.State == Proposed && d.outOfDate() != "" && d.AskedAgain == nil { + d.AskedAgain = &now + h.keep(d) + } + return + } + replaced := "" for _, old := range h.deliveries { if old.Repository == p.repository() && old.Number == p.Number && !sameCommit(old.Commit, p.HeadSHA) && !old.State.Final() && old.MergedAs == "" { @@ -307,13 +321,14 @@ func (h *Holder) PullUpdated(p PullEvent) { h.settleAndKeep(old) if old.Group != "" { h.leaveGroup(old) + replaced = old.ID } } } d := &Delivery{ID: id, Repository: p.repository(), Commit: p.HeadSHA, Number: p.Number, Title: p.Title, Base: p.Base, Branch: p.Head, HTMLURL: p.HTMLURL, CloneURL: p.CloneURL, Paths: p.Paths, PathsTruncated: p.PathsTruncated, Removed: p.Removed, ModuleDirs: p.ModuleDirs, ModuleDirsSaid: p.ModuleDirsSaid, - After: AfterIn(p.Body), Created: now} + After: AfterIn(p.Body), Created: now, BaseSHA: p.BaseSHA} t, err := Apply(d, EvAnnounced, false, Facts{Now: now, Why: fmt.Sprintf("%s#%d's head announced", d.Repository, d.Number)}) if err != nil { h.refuse(err) @@ -327,7 +342,45 @@ func (h *Holder) PullUpdated(p PullEvent) { ts = append(ts, settle(d, Facts{Now: now})...) } h.moved(d, ts) - h.joinGroup(d) + if !h.takePlaceOf(replaced, d) { + h.joinGroup(d) + } +} + +// takePlaceOf puts a member's new head — a rebase, a push — in its group in the place of the head it superseded, +// when the group is closed: a member of it already reached the trunk, and a closed group takes no new member. The +// new head has its own check, and the group's composed check runs again for the new heads rather than holding the +// group for a person (novox/hq ADR 0323 §6, issue 406). An open group takes it as joinGroup does. +func (h *Holder) takePlaceOf(old string, d *Delivery) bool { + if old == "" { + return false + } + for _, g := range h.groups { + _, ordered := g.OrderOf[old] + if !g.Closed || !ordered && !containsString(g.Order, old) { + continue + } + if ordered { + delete(g.OrderOf, old) + g.OrderOf[d.ID] = d.Commit + } + if !containsString(g.Members, d.ID) { + g.Members = append(g.Members, d.ID) + sort.Strings(g.Members) + } + for i, id := range g.Order { + if id == old { + g.Order[i] = d.ID + } + } + d.Group = g.ID + g.Updated = h.Now() + h.keep(d) + h.keepGroup(g) + h.Logf("[mesh-delivery] group %s: %s takes the place of %s, which it superseded", g.ID, d.ID, old) + return true + } + return false } // decidedOnHead is the verdict the forge's statuses on a head hold, when the gate decided one: passed, warned @@ -468,6 +521,47 @@ func (h *Holder) PullMerged(p PullEvent) { } h.moved(d, ts) h.closeGroupOf(d) + h.trunkMoved(d, p.Base, p.MergeCommit, now) +} + +// trunkMoved takes a merge into a trunk as that trunk's move (novox/hq ADR 0323 §7): every other open delivery into +// it keeps the trunk's new head, and a ready one whose verdict names the commit before goes back to proposed. A +// group's composed check goes out of date when the merge is not one of its members and a member still open merges +// into that trunk (§6); the group then reads checking. Nothing is asked: a recheck or the merge tool judges again. +func (h *Holder) trunkMoved(merged *Delivery, base, commit string, now time.Time) { + if base == "" || commit == "" { + return + } + for _, o := range h.deliveries { + if o == merged || o.Repository != merged.Repository || o.Base != base || o.MergedAs != "" || o.State.Final() || + sameCommit(o.TrunkNow, commit) { + continue + } + // A check asked again against the trunk before this move judged that trunk: it is out of date too. + o.TrunkNow, o.AskedAgain = commit, nil + h.settleAndKeep(o) + } + for _, g := range h.groups { + if g.ID == merged.Group || g.Check == nil || g.Check.Verdict == "" || g.Check.OutOfDate != "" { + continue + } + for _, m := range h.membersOf(g) { + if m.Repository != merged.Repository || m.Base != base || m.MergedAs != "" || m.State.Final() { + continue + } + judged := g.Check.Trunks[m.ID] + if judged == "" { + judged = "?" + } + g.Check.OutOfDate = fmt.Sprintf("composed check out of date: %s was judged against trunk %s, the trunk is now %s "+ + "(%s merged)", m.ID, shortOf(judged, 12), shortOf(commit, 12), merged.ID) + g.Check.OutOfDateAt = now + g.Updated = now + h.keepGroup(g) + h.Logf("[mesh-delivery] group %s: %s", g.ID, g.Check.OutOfDate) + break + } + } } // closeGroupOf closes a delivery's group once it reached the trunk: a delivering group takes no new member. @@ -539,7 +633,13 @@ type CheckedEvent struct { Owner string `json:"owner"` Repo string `json:"repo"` Commit string `json:"commit"` + Trunk string `json:"trunk,omitempty"` } `json:"members,omitempty"` + // Trunk is the trunk commit the head was merged into and judged as, Beside the other repositories' commits the + // check read, and Took how long it ran (novox/hq ADR 0323 §3). + Trunk string `json:"trunk,omitempty"` + Beside map[string]string `json:"beside,omitempty"` + Took string `json:"took,omitempty"` } // Checked takes a verdict: a head's own, or a group's composed check. @@ -554,6 +654,18 @@ func (h *Holder) Checked(c CheckedEvent) { return } g.Check.Verdict, g.Check.Summary = c.Verdict, c.Summary + // Each member's head and the trunk commit it was merged into (novox/hq ADR 0323 §6). + for _, m := range c.Members { + for id := range g.Check.Commits { + if d := h.deliveries[id]; d != nil && strings.EqualFold(d.Repository, m.Owner+"/"+m.Repo) && + sameCommit(d.Commit, m.Commit) && m.Trunk != "" { + if g.Check.Trunks == nil { + g.Check.Trunks = map[string]string{} + } + g.Check.Trunks[id] = m.Trunk + } + } + } g.Updated = now h.keepGroup(g) h.Logf("[mesh-delivery] group %s composed: %s — %s", g.ID, c.Verdict, c.Summary) @@ -574,11 +686,23 @@ func (h *Holder) Checked(c CheckedEvent) { } h.moved(d, []Transition{t}) } + if d.State == Rejected && c.Trunk != "" && d.Check != nil && !sameCommit(d.Check.Trunk, c.Trunk) { + // Judged again against another trunk commit — the merge tool's ask of a verdict out of date, or a check asked + // elsewhere (novox/hq ADR 0323 §5, §7): taken as the recheck it is. + t, err := Apply(d, EvRecheck, true, Facts{Now: now, By: byOwner, Why: fmt.Sprintf("judged again against trunk %s "+ + "(it was rejected against trunk %s)", shortOf(c.Trunk, 12), shortOf(d.Check.Trunk, 12))}) + if err != nil { + h.refuse(err) + return + } + h.moved(d, []Transition{t}) + } if d.State != Proposed { h.Logf("[mesh-delivery] %s is %s: a verdict for it now is history (%s)", d.ID, d.State, c.Verdict) return } - v := &Verdict{ID: c.ID, On: c.On, At: now, Gate: c.Verdict, Summary: c.Summary, Report: c.Report} + v := &Verdict{ID: c.ID, On: c.On, At: now, Gate: c.Verdict, Summary: c.Summary, Report: c.Report, + Head: shortOf(c.Commit, 12), Trunk: c.Trunk, Beside: c.Beside, Took: c.Took} if c.Gate != nil { v.Gate, v.Summary = c.Gate.Verdict, c.Gate.Summary v.Modules, v.Dependents = c.Gate.Modules, c.Gate.Dependents @@ -589,10 +713,14 @@ func (h *Holder) Checked(c CheckedEvent) { if v.Gate == "" { v.Gate = "error" } - d.Check = v + d.Check, d.AskedAgain = v, nil if c.Plan != nil { d.Plan = c.Plan } + if why := d.outOfDate(); why != "" { + // Kept and shown, and it does not count: the delivery stays proposed (ADR 0323 §7). + h.Logf("[mesh-delivery] %s: its verdict %s is %s", d.ID, c.ID, why) + } h.settleAndKeep(d) } @@ -1441,14 +1569,19 @@ func (h *Holder) planGroup(g *Group, members []*Delivery, now time.Time) []func( }) return asks } - // The composed check, once every member is ready and the order holds. + // The composed check, once every member is ready and the order holds. Asked again for new heads, even once the + // group is closed: a member rebased or pushed is a new head, and the group waits for its composed check rather + // than being held for a person (novox/hq ADR 0323 §6, issue 406). One out of date is asked again once a member + // was judged again after it went out of date: a recheck, or the merge tool's ask. allReady := true for _, d := range members { - if d.State != Ready { + // In a closed group a member on the trunk counts — merged and on its way, or already delivered (reached). + if d.State != Ready && !(g.Closed && d.MergedAs != "" && (!d.State.Final() || d.State.Reached())) { allReady = false } } - if allReady && len(g.Cycle) == 0 && !g.Closed && (g.Check == nil || !sameMembers(g.Check.Commits, commits)) { + again := g.Check != nil && (!sameMembers(g.Check.Commits, commits) || g.Check.OutOfDate != "" && rejudgedSince(members, g.Check.OutOfDateAt)) + if allReady && len(g.Cycle) == 0 && (g.Check == nil && !g.Closed || again) { ms := h.inOrder(g, members) id := g.ID asks = append(asks, func() { @@ -1570,7 +1703,7 @@ func (h *Holder) turnsOf(g *Group, members []*Delivery, waits map[string]carryWa } // Composed for these very heads: a verdict on another set of heads is no verdict on these. composed := g.Check != nil && (g.Check.Verdict == "pass" || g.Check.Verdict == "warning") && - sameMembers(g.Check.Commits, commitsOf(members)) + sameMembers(g.Check.Commits, commitsOf(members)) && g.Check.OutOfDate == "" // A member's turn registered, the members after it that the same walk carries take theirs too: one walk is let // go once, for all of them (ADR 0276 decision 5). A member on another walk, or not at its turn, waits. var asks []func() @@ -1656,9 +1789,11 @@ func (h *Holder) turnsOf(g *Group, members []*Delivery, waits map[string]carryWa } return asks case d.State == Published: - // Asked and not answered is not "did not pass": it waits, published, for the answer (issue 316). - if !composed && d.HeldWhy == "" && g.Check != nil && g.Check.Verdict == "" && - sameMembers(g.Check.Commits, commitsOf(members)) { + // Asked and not answered is not "did not pass": it waits, published, for the answer (issue 316). Nor is a + // composed check of other heads — a member rebased or pushed — or one out of date: it runs again for these + // heads, and the member waits for it (novox/hq ADR 0323 §6, issue 406). + if !composed && d.HeldWhy == "" && g.Check != nil && (g.Check.Verdict == "" || g.Check.OutOfDate != "" || + !sameMembers(g.Check.Commits, commitsOf(members))) { return asks } if !composed && d.HeldWhy == "" { @@ -2179,12 +2314,23 @@ func GroupState(g *Group, members []*Delivery) (string, string) { return "delivered", "every member is delivered" case g.Closed && n > 0 && ended == n: return "stopped", supersededWhy + strings.Join(uncovered, ", ") + case g.Closed && g.Check != nil && (g.Check.OutOfDate != "" || g.Check.Verdict == "" || + !sameMembers(g.Check.Commits, commitsOf(members))): + // A member rebased or pushed once the group was delivering, or its composed check out of date: it is composed + // again for these heads, and the group is checking until then, never held (novox/hq ADR 0323 §6, issue 406). + if g.Check.OutOfDate != "" { + return "checking", g.Check.OutOfDate + } + return "checking", "composed again for its members' new heads" case g.Closed: return "delivering", fmt.Sprintf("%d of %d delivered", reached, n) case len(g.Cycle) > 0: return "rejected", cycleSaid(g) case count[Rejected] > 0: return "rejected", "a member's own check failed" + case g.Check != nil && g.Check.OutOfDate != "": + // Out of date counts as outstanding (ADR 0239 §18, novox/hq ADR 0323 §7): checking, saying why. + return "checking", g.Check.OutOfDate case g.Check != nil && g.Check.Verdict != "" && g.Check.Verdict != "pass" && g.Check.Verdict != "warning": return "rejected", "composed together: " + g.Check.Summary case count[Ready] == n && g.Check != nil && sameMembers(g.Check.Commits, commitsOf(members)) && @@ -2250,3 +2396,13 @@ func cycleSaid(g *Group) string { } return why } + +// rejudgedSince is whether a member was judged again after a moment: a verdict of its own heard since. +func rejudgedSince(members []*Delivery, at time.Time) bool { + for _, d := range members { + if d.Check != nil && d.Check.At.After(at) { + return true + } + } + return false +} diff --git a/modules/mesh-delivery/cmd/mesh-delivery/issue341_test.go b/modules/mesh-delivery/cmd/mesh-delivery/issue341_test.go new file mode 100644 index 00000000..1b6e5352 --- /dev/null +++ b/modules/mesh-delivery/cmd/mesh-delivery/issue341_test.go @@ -0,0 +1,463 @@ +package main + +import ( + "strings" + "testing" + "time" +) + +// **A verdict holds for the trunk commit it judged** (novox/hq ADR 0323, issue 341). The check judges the head +// merged into a named trunk commit; when another merge moves that trunk, a ready delivery goes back to proposed, +// a rejected one stays rejected, and nothing is asked. A verdict for an old trunk is kept and shown and does not +// count, so the table reaches a fixed point. A group's composed check goes out of date on a merge that is not one of +// its members, and runs again for new heads instead of holding the group. + +const ( + trunk0 = "a2c2c35000000000000000000000000000000000" + merge1 = "d28ca95000000000000000000000000000000000" +) + +// judged is a verdict of a head merged into a trunk commit. +func judged(repo string, number int, sha, gate, trunk string) CheckedEvent { + c := verdict(repo, number, sha, gate) + c.Trunk = shortOf(trunk, 12) + c.ID = "check-of-" + sha + "-at-" + shortOf(trunk, 8) + c.Took = "4m12s" + c.Beside = map[string]string{"mesh-catalog": "0123456789ab"} + plan := *c.Plan + plan.Head, plan.Trunk = sha, c.Trunk + c.Plan = &plan + return c +} + +func TestATrunkMoveSendsAReadyDeliveryBackToProposed(t *testing.T) { + const host = "novox/mesh-host" + const h64, h66, h67 = "6464646464646464", "dfb1415dfb1415df", "6767676767676767" + w := newWorld(t) + for _, p := range []PullEvent{pr(host, 64, h64, "a"), pr(host, 66, h66, "b"), pr(host, 67, h67, "c")} { + w.h.PullUpdated(p) + } + w.h.Checked(judged(host, 64, h64, "pass", trunk0)) + w.h.Checked(judged(host, 66, h66, "pass", trunk0)) + w.h.Checked(judged(host, 67, h67, "fail", trunk0)) + id66, id67 := IDOf(host, h66), IDOf(host, h67) + if s := w.state(id66); s != Ready { + t.Fatalf("#66 checked green against the trunk is %s", s) + } + asked := len(w.ctl.checks) + + // #64 merges: the trunk moves to its merge commit. + w.h.PullMerged(merged(host, 64, h64, merge1)) + w.settleAll() + d := w.delivery(id66) + if d.State != Proposed { + t.Fatalf("#66's verdict names trunk %s, the trunk is now %s, and it is %s", trunk0[:12], merge1[:12], d.State) + } + last := d.Transitions[len(d.Transitions)-1] + if last.Event != EvTrunkMoved || !strings.Contains(last.Why, trunk0[:12]) || !strings.Contains(last.Why, merge1[:12]) { + t.Fatalf("its transition was %s, %q: not trunk moved with both commits", last.Event, last.Why) + } + if d.Check == nil || d.Check.Trunk != trunk0[:12] { + t.Fatalf("its out-of-date verdict was not kept to be shown: %+v", d.Check) + } + if s := w.state(id67); s != Rejected { + t.Fatalf("a rejected delivery went to %s on the trunk's move", s) + } + if len(w.ctl.checks) != asked { + t.Fatalf("the trunk's move asked a check: %v", w.ctl.checks[asked:]) + } + + // Out of date and not asked: no bound, so stalled does not list it, however long it waits. + w.later(5 * time.Hour) + w.settleAll() + for _, l := range w.h.Stalled() { + if l.ID == id66 { + t.Fatalf("stalled lists a proposed delivery whose check is out of date and not asked: %+v", l) + } + } + if len(w.ctl.checks) != asked { + t.Fatalf("an out-of-date check was asked by this owner: %v", w.ctl.checks[asked:]) + } + + // A late verdict for the old trunk is kept, does not count, and moves nothing: the table's fixed point. + before := len(w.delivery(id66).Transitions) + late := judged(host, 66, h66, "pass", trunk0) + late.ID = "check-late" + w.h.Checked(late) + w.settleAll() + d = w.delivery(id66) + if d.State != Proposed || len(d.Transitions) != before || d.Check == nil || d.Check.ID != "check-late" { + t.Fatalf("a late verdict for an old trunk left it %s after %d transition(s), verdict %+v", d.State, + len(d.Transitions)-before, d.Check) + } + if ts := settle(d, Facts{Now: w.now}); len(ts) != 0 { + t.Fatalf("the table took %d more transition(s) on a verdict that does not count", len(ts)) + } + + // The check asked again judges the head merged into the trunk's head: it counts. + w.h.Checked(judged(host, 66, h66, "pass", merge1)) + w.settleAll() + if s := w.state(id66); s != Ready { + t.Fatalf("a verdict judged against the trunk's head left it %s", s) + } +} + +// A rejected delivery judged again against another trunk — the merge tool's ask — is taken as its recheck. +func TestARejectedDeliveryJudgedAgainstANewTrunkIsRecheckedByTheVerdict(t *testing.T) { + const repo = "novox/mesh-lab" + const head = "1a2b3c4d5e6f7a8b" + w := newWorld(t) + w.h.PullUpdated(pr(repo, 9, head, "x")) + w.h.Checked(judged(repo, 9, head, "fail", trunk0)) + id := IDOf(repo, head) + if s := w.state(id); s != Rejected { + t.Fatalf("it is %s", s) + } + w.h.PullMerged(merged(repo, 8, "8888888888888888", merge1)) + w.h.Checked(judged(repo, 9, head, "pass", merge1)) + w.settleAll() + d := w.delivery(id) + if d.State != Ready || d.Check.Trunk != merge1[:12] { + t.Fatalf("judged green against the trunk's head it is %s with %+v", d.State, d.Check) + } +} + +// **A verdict names the trunk commit it judged** in `checks` and `show`, and the kept plan records it (ADR 0323 §3). +func TestChecksAndShowNameTheHeadAndTheTrunkCommit(t *testing.T) { + const repo = "novox/mesh-lab" + const head = "1a2b3c4d5e6f7a8b" + w := newWorld(t) + w.h.PullUpdated(pr(repo, 9, head, "x")) + w.h.Checked(judged(repo, 9, head, "pass", trunk0)) + id := IDOf(repo, head) + + a, err := w.h.Checks(repo, 0, head) + if err != nil { + t.Fatal(err) + } + want := "head " + head[:12] + " merged into trunk " + trunk0[:12] + if a.Judged != want || a.Verdict == nil || a.Verdict.Head != head[:12] || a.Verdict.Trunk != trunk0[:12] { + t.Fatalf("checks says %q, verdict %+v; want %q", a.Judged, a.Verdict, want) + } + if !strings.Contains(a.Says, want) { + t.Fatalf("checks' words do not say what was judged: %s", a.Says) + } + if a.Verdict.Beside["mesh-catalog"] != "0123456789ab" || a.Verdict.Took != "4m12s" { + t.Fatalf("checks does not name the other repositories' commits or how long it took: %+v", a.Verdict) + } + shown, err := w.h.Show(id) + if err != nil { + t.Fatal(err) + } + d := shown.(map[string]any)["delivery"].(*Delivery) + if d.Check.Head != head[:12] || d.Check.Trunk != trunk0[:12] { + t.Fatalf("show gives the verdict %+v", d.Check) + } + if d.Plan == nil || d.Plan.Trunk != trunk0[:12] || d.Plan.Head != head { + t.Fatalf("the kept delivery plan records %+v", d.Plan) + } + + w.h.PullMerged(merged(repo, 8, "8888888888888888", merge1)) + if a, _ = w.h.Checks(repo, 0, head); !strings.Contains(a.OutOfDate, merge1[:12]) { + t.Fatalf("checks does not say the verdict is out of date: %+v", a) + } +} + +// **A delivery group's composed check follows the same rule** (ADR 0323 §6, §7): a merge that is not one of its +// members into a member's trunk puts it out of date and the group reads checking; its own members merging in its +// order do not; and it runs again once a member is judged again. +func TestAGroupsComposedCheckGoesOutOfDateOnAMergeThatIsNotItsMember(t *testing.T) { + const ctl, cat = "novox/mesh-controller", "novox/mesh-catalog" + const hc, hk = "c1c1c1c1c1c1c1c1", "k1k1k1k1k1k1k1k1" + idc, idk := IDOf(ctl, hc), IDOf(cat, hk) + start := func(t *testing.T) *world { + w := newWorld(t) + w.h.PullUpdated(pr(ctl, 1, hc, "feat/g")) + w.h.PullUpdated(pr(cat, 2, hk, "feat/g")) + w.h.Checked(judged(ctl, 1, hc, "pass", trunk0)) + w.h.Checked(judged(cat, 2, hk, "pass", trunk0)) + w.settleAll() + g := w.h.groups["feat/g"] + if g == nil || g.Check == nil { + t.Fatalf("no composed check was asked: %+v", g) + } + w.h.Checked(CheckedEvent{Group: "feat/g", ID: g.Check.Asked, Verdict: "pass", Summary: "composed", + Members: []struct { + Owner string `json:"owner"` + Repo string `json:"repo"` + Commit string `json:"commit"` + Trunk string `json:"trunk,omitempty"` + }{{Owner: "novox", Repo: "mesh-controller", Commit: hc, Trunk: trunk0[:12]}, + {Owner: "novox", Repo: "mesh-catalog", Commit: hk, Trunk: trunk0[:12]}}}) + w.settleAll() + if state, _ := GroupState(g, w.h.membersOf(g)); state != "ready" { + t.Fatalf("the group composed green is %s", state) + } + if g.Check.Trunks[idc] != trunk0[:12] { + t.Fatalf("the composed check names its members' trunks %v", g.Check.Trunks) + } + return w + } + + t.Run("a merge that is not a member", func(t *testing.T) { + w := start(t) + asked := len(w.ctl.checks) + w.h.PullMerged(merged(ctl, 5, "5555555555555555", merge1)) + w.settleAll() + g := w.h.groups["feat/g"] + state, why := GroupState(g, w.h.membersOf(g)) + if state != "checking" || g.Check.OutOfDate == "" { + t.Fatalf("the group is %s (%s) after a merge that is not its member moved a member's trunk", state, why) + } + for _, head := range []string{ctl + "@" + hc, cat + "@" + hk} { + s := w.forge.statuses[head+" "+ctxGroup] + if !strings.HasPrefix(s, "pending ") || !strings.Contains(s, "out of date") { + t.Fatalf("mesh/delivery-group on %s says %q, not pending with the reason", head, s) + } + } + if len(w.ctl.checks) != asked { + t.Fatalf("the trunk's move asked a check: %v", w.ctl.checks[asked:]) + } + // The controller's member judged again against the trunk's head: the composed check runs again. + w.later(time.Minute) + w.h.Checked(judged(ctl, 1, hc, "pass", merge1)) + w.settleAll() + if len(w.ctl.checks) == asked || !strings.HasPrefix(w.ctl.checks[len(w.ctl.checks)-1], "feat/g:") { + t.Fatalf("the composed check was not asked again once a member was judged again: %v", w.ctl.checks) + } + }) + + t.Run("its own member merging", func(t *testing.T) { + w := start(t) + mc := merged(ctl, 1, hc, merge1) + mc.Head = "feat/g" + w.h.PullMerged(mc) + w.settleAll() + g := w.h.groups["feat/g"] + if g.Check.OutOfDate != "" { + t.Fatalf("the group's own member merging put its composed check out of date: %s", g.Check.OutOfDate) + } + if s := w.state(idk); s != Ready { + t.Fatalf("the catalogue's member, in another repository, is %s", s) + } + }) +} + +// **After a member is rebased or pushed, the composed check runs again for the new heads** and the group reads +// checking, not held (ADR 0323 §6, issue 406's second symptom): its first member merged, closing the group, and then +// its second was rebased just before its own merge. +func TestARebasedMemberIsComposedAgainNotHeld(t *testing.T) { + const ctl, cat = "novox/mesh-controller", "novox/mesh-catalog" + const hc, hk, hc2 = "c1c1c1c1c1c1c1c1", "k1k1k1k1k1k1k1k1", "c2c2c2c2c2c2c2c2" + idc, idk, idc2 := IDOf(ctl, hc), IDOf(cat, hk), IDOf(ctl, hc2) + w := newWorld(t) + w.h.PullUpdated(pr(ctl, 1, hc, "feat/g")) + w.h.PullUpdated(pr(cat, 2, hk, "feat/g")) + w.h.Checked(judged(ctl, 1, hc, "pass", trunk0)) + w.h.Checked(judged(cat, 2, hk, "pass", trunk0)) + w.settleAll() + g := w.h.groups["feat/g"] + if len(g.Order) != 2 || g.Order[0] != idk { + t.Fatalf("the group's order is %v: the catalogue's member first is what this replays", g.Order) + } + w.h.Checked(CheckedEvent{Group: "feat/g", ID: g.Check.Asked, Verdict: "pass", Summary: "composed"}) + w.settleAll() + + // The catalogue's member merges first: the group is closed. + mk := merged(cat, 2, hk, merge1) + mk.Head = "feat/g" + w.h.PullMerged(mk) + w.h.WalkMoved(w.ctl.put(aWalk("plan-cat", cat, merge1, true))) + w.settleAll() + if !g.Closed { + t.Fatal("the group is not closed once a member merged") + } + + // The controller's member is rebased just before its merge: a new head, with its own check. + w.h.PullUpdated(pr(ctl, 1, hc2, "feat/g")) + w.settleAll() + if d := w.delivery(idc2); d.Group != "feat/g" { + t.Fatalf("the rebased head did not take its place in its closed group: %q", d.Group) + } + if containsString(g.Members, idc) || !containsString(g.Members, idc2) || len(g.Order) != 2 || g.Order[1] != idc2 { + t.Fatalf("the group's members are %v in the order %v", g.Members, g.Order) + } + if state, why := GroupState(g, w.h.membersOf(g)); state != "checking" { + t.Fatalf("the group reads %s (%s), not checking", state, why) + } + w.h.Checked(judged(ctl, 1, hc2, "pass", trunk0)) + w.settleAll() + asked := w.ctl.checks[len(w.ctl.checks)-1] + if !strings.HasPrefix(asked, "feat/g:") || !strings.Contains(asked, idc2) || !strings.Contains(asked, idk) { + t.Fatalf("the composed check was not asked again for the new heads: %v", w.ctl.checks) + } + if state, why := GroupState(g, w.h.membersOf(g)); state != "checking" { + t.Fatalf("the group reads %s (%s), not checking", state, why) + } + for _, id := range []string{idk, idc2} { + if d := w.delivery(id); d.State == Held { + t.Fatalf("%s is held for a person: %s", id, d.HeldWhy) + } + } + // Its verdict for the new heads: the rebased member merges and takes its turn. + w.h.Checked(CheckedEvent{Group: "feat/g", ID: g.Check.Asked, Verdict: "pass", Summary: "composed again"}) + mc := merged(ctl, 1, hc2, "e0e0e0e0e0e0e0e0") + mc.Head = "feat/g" + w.h.PullMerged(mc) + w.h.WalkMoved(w.ctl.put(aWalk("plan-ctl", ctl, "e0e0e0e0e0e0e0e0", true))) + w.settleAll() + if d := w.delivery(idc2); d.State == Held { + t.Fatalf("the rebased member, composed green for the new heads, is held: %s", d.HeldWhy) + } +} + +// A head whose merge check the forge's holder put out of date is not a check asked and not answered: `stalled` does +// not list it, however long it waits — rejected, or with no delivery here — until its check is asked (ADR 0323 §7). +func TestAnOutOfDateCheckOnTheForgeIsNotSaidUnanswered(t *testing.T) { + const repo = "novox/mesh-lab" + const head = "1a2b3c4d5e6f7a8b9c0d" + w := newWorld(t) + w.h.PullUpdated(pr(repo, 9, head, "x")) + w.h.Checked(judged(repo, 9, head, "fail", trunk0)) + words := outOfDateWords(trunk0, merge1) + pending := func(description, at string) map[string]HeadCheck { + return map[string]HeadCheck{ctxGate: {State: "pending", Description: description, UpdatedAt: at}, + ctxRepo: {State: "pending", Description: description, UpdatedAt: at}} + } + w.forge.open = []OpenPull{{Owner: "novox", Repo: "mesh-lab", Number: 9, Base: "main", HeadSHA: head, + Required: []string{ctxGate, ctxRepo}, Statuses: []string{ctxGate, ctxRepo}, Checks: pending(words, "2026-10-06T12:01:00Z")}} + w.h.SeeOpenPulls() + w.later(3 * time.Hour) + w.h.SeeOpenPulls() + for _, l := range w.h.Stalled() { + if strings.HasPrefix(l.ID, repo) { + t.Fatalf("an out-of-date check nobody asked is said stalled: %+v", l) + } + } + // Asked by the merge tool: pending anew, and its bound runs from then. + w.forge.open[0].Checks = pending("waiting in the build queue since 15:01: asked by the merge tool", "2026-10-06T15:01:00Z") + w.h.SeeOpenPulls() + w.later(2 * time.Hour) + w.h.SeeOpenPulls() + said := false + for _, l := range w.h.Stalled() { + said = said || l.State == Unanswered + } + if !said { + t.Fatal("a check the merge tool asked and that never answered is not said") + } +} + +// The merge tool's ask announces an out-of-date head again: its one-hour bound runs from then, and a check that never +// answers is said stalled (ADR 0323 §7). +func TestAnOutOfDateDeliveryAskedAgainIsBoundFromTheAsk(t *testing.T) { + const repo = "novox/mesh-lab" + const head = "1a2b3c4d5e6f7a8b" + w := newWorld(t) + w.h.PullUpdated(pr(repo, 9, head, "x")) + w.h.Checked(judged(repo, 9, head, "pass", trunk0)) + w.h.PullMerged(merged(repo, 8, "8888888888888888", merge1)) + id := IDOf(repo, head) + w.later(3 * time.Hour) + w.h.PullUpdated(pr(repo, 9, head, "x")) // the merge tool asked its check + w.later(30 * time.Minute) + for _, l := range w.h.Stalled() { + if l.ID == id { + t.Fatalf("listed half an hour after its check was asked again: %+v", l) + } + } + w.later(time.Hour) + said := false + for _, l := range w.h.Stalled() { + if l.ID == id { + said = true + if !strings.Contains(l.Says, "asked again") || !strings.Contains(l.H2, "operator") { + t.Fatalf("its stalled line says %+v", l) + } + } + } + if !said { + t.Fatal("a check asked again that never answered is not said") + } + // Its verdict judged against the trunk's head: ready, and nothing bounds it as proposed. + w.h.Checked(judged(repo, 9, head, "pass", merge1)) + if d := w.delivery(id); d.State != Ready || d.AskedAgain != nil { + t.Fatalf("judged again it is %s, asked again %v", d.State, d.AskedAgain) + } +} + +// The same, with the first member already delivered when the second is rebased: a delivered member counts for the +// composed check of the new heads, which is asked, and the rebased member is never held (review of ADR 0323 §6). +func TestARebasedMemberAfterAnotherWasDeliveredIsComposedAgain(t *testing.T) { + const ctl, cat = "novox/mesh-controller", "novox/mesh-catalog" + const hc, hk, hc2 = "c1c1c1c1c1c1c1c1", "k1k1k1k1k1k1k1k1", "c2c2c2c2c2c2c2c2" + idk, idc2 := IDOf(cat, hk), IDOf(ctl, hc2) + w := newWorld(t) + w.h.PullUpdated(pr(ctl, 1, hc, "feat/g")) + w.h.PullUpdated(pr(cat, 2, hk, "feat/g")) + w.h.Checked(judged(ctl, 1, hc, "pass", trunk0)) + w.h.Checked(judged(cat, 2, hk, "pass", trunk0)) + w.settleAll() + g := w.h.groups["feat/g"] + w.h.Checked(CheckedEvent{Group: "feat/g", ID: g.Check.Asked, Verdict: "pass", Summary: "composed"}) + w.settleAll() + + // The catalogue's member merges and its walk is done: delivered. + mk := merged(cat, 2, hk, merge1) + mk.Head = "feat/g" + w.h.PullMerged(mk) + walk := w.ctl.put(aWalk("plan-cat", cat, merge1, true)) + w.h.WalkMoved(walk) + w.settleAll() + walk = w.ctl.walk("plan-cat") + walk.State = walkDone + w.h.WalkMoved(w.ctl.put(walk)) + w.settleAll() + if s := w.state(idk); s != Delivered { + t.Fatalf("the catalogue's member is %s, not delivered", s) + } + + // The controller's member is rebased, and judged. + w.h.PullUpdated(pr(ctl, 1, hc2, "feat/g")) + w.h.Checked(judged(ctl, 1, hc2, "pass", trunk0)) + w.settleAll() + asked := w.ctl.checks[len(w.ctl.checks)-1] + if !strings.HasPrefix(asked, "feat/g:") || !strings.Contains(asked, idc2) { + t.Fatalf("the composed check was not asked again for the new heads: %v", w.ctl.checks) + } + if state, why := GroupState(g, w.h.membersOf(g)); state != "checking" { + t.Fatalf("the group reads %s (%s), not checking", state, why) + } + w.h.Checked(CheckedEvent{Group: "feat/g", ID: g.Check.Asked, Verdict: "pass", Summary: "composed again"}) + mc := merged(ctl, 1, hc2, "e0e0e0e0e0e0e0e0") + mc.Head = "feat/g" + w.h.PullMerged(mc) + w.h.WalkMoved(w.ctl.put(aWalk("plan-ctl", ctl, "e0e0e0e0e0e0e0e0", true))) + w.settleAll() + if d := w.delivery(idc2); d.State == Held { + t.Fatalf("the rebased member is held: %s", d.HeldWhy) + } +} + +// A check this owner asks — a recheck — sends the base head the forge last announced the head with, so the +// controller's verdict for a head no build judges names its trunk commit (ADR 0323 §3, review of mesh-controller#271). +func TestARecheckSendsTheAnnouncedBaseHead(t *testing.T) { + const repo = "novox/mesh-tools-go" + const head = "1a2b3c4d5e6f7a8b" + w := newWorld(t) + p := pr(repo, 3, head, "x") + p.BaseSHA = trunk0 + w.h.PullUpdated(p) + id := IDOf(repo, head) + if m := MemberOf(w.delivery(id)); m.BaseSHA != trunk0 { + t.Fatalf("the member sends base head %q", m.BaseSHA) + } + p.BaseSHA = merge1 // announced again, by the merge tool + w.h.PullUpdated(p) + if m := MemberOf(w.delivery(id)); m.BaseSHA != merge1 { + t.Fatalf("announced again, the member sends base head %q", m.BaseSHA) + } + if raw := string(mustJSON(MemberOf(w.delivery(id)))); !strings.Contains(raw, `"base_sha":"`+merge1+`"`) { + t.Fatalf("the member as the controller reads it: %s", raw) + } +} diff --git a/modules/mesh-delivery/cmd/mesh-delivery/model.go b/modules/mesh-delivery/cmd/mesh-delivery/model.go index 8f8a5047..55980cb2 100644 --- a/modules/mesh-delivery/cmd/mesh-delivery/model.go +++ b/modules/mesh-delivery/cmd/mesh-delivery/model.go @@ -49,6 +49,17 @@ type Delivery struct { // CheckAsk is this owner's own ask of its check, when the forge's announcement did not bring one: a head // adopted while it already waited, or one whose announcement the controller missed. CheckAsk *CheckAsk `json:"check_ask,omitempty"` + // TrunkNow is its trunk's head as this owner last heard it move while it was open: the commit the last merge + // into its trunk landed as (novox/hq ADR 0323 §7). A verdict that names another trunk commit is out of date. + // Empty until a merge into its trunk is heard after it was made. + TrunkNow string `json:"trunk_now,omitempty"` + // BaseSHA is the base branch's head the forge last announced its head with (ADR 0323): sent with a check this + // owner asks, for a verdict no build judges to name its trunk commit. + BaseSHA string `json:"base_sha,omitempty"` + // AskedAgain is when its head was announced again while its verdict was out of date — the merge tool's ask of + // its check against the trunk's head (ADR 0323 §5) — and no verdict has come since: its one-hour bound runs from + // then. Nil while nothing is asked, and cleared by a verdict or another move of its trunk. + AskedAgain *time.Time `json:"asked_again,omitempty"` // The facts the table's guards read. NewerHead is the head that superseded it; ClosedUnmerged the pull // request closed; MergedAs the commit it landed on the trunk as; HeldWhy what holds it for a person. @@ -128,6 +139,38 @@ type Verdict struct { Repo string `json:"repo,omitempty"` RepoSaid string `json:"repo_summary,omitempty"` Report string `json:"report,omitempty"` + // Head and Trunk are what it judged (novox/hq ADR 0323 §3): the head merged into the trunk commit, twelve + // characters each. Trunk is empty for a verdict of a build seat that judged the head alone. Beside are the + // commits of the other repositories the check read, by directory; Took how long it ran. + Head string `json:"head,omitempty"` + Trunk string `json:"trunk,omitempty"` + Beside map[string]string `json:"beside,omitempty"` + Took string `json:"took,omitempty"` +} + +// Judged is what a verdict judged, in words: "head 1a2b3c4d5e6f merged into trunk 7a8b9c0d1e2f" (ADR 0323 §3); +// empty for a verdict that names no trunk commit. +func (v *Verdict) Judged() string { + if v == nil || v.Trunk == "" { + return "" + } + return fmt.Sprintf("head %s merged into trunk %s", shortOf(v.Head, 12), shortOf(v.Trunk, 12)) +} + +// outOfDate is why a delivery's verdict no longer counts: it names a trunk commit that is not its trunk's head +// as this owner heard it (novox/hq ADR 0323 §7). Empty when it counts — or names no trunk commit, as a verdict +// of a build seat from before the rule does, which counts as it did. +func (d *Delivery) outOfDate() string { + if d.Check == nil || d.Check.Trunk == "" || d.TrunkNow == "" || sameCommit(d.Check.Trunk, d.TrunkNow) { + return "" + } + return outOfDateWords(d.Check.Trunk, d.TrunkNow) +} + +// outOfDateWords is a verdict out of date, as the forge's holder says it on the pull request (ADR 0323 §4). +func outOfDateWords(judged, now string) string { + return fmt.Sprintf("out of date: judged against trunk %s, the trunk is now %s; judged again on a recheck or when "+ + "it is merged", shortOf(judged, 12), shortOf(now, 12)) } // CheckAsk is a head's own check, asked of the controller by this owner (or re-asked by healer H2). @@ -165,6 +208,8 @@ type DeliveryPlan struct { Machines []MachinePlan `json:"machines,omitempty"` Steps []string `json:"steps,omitempty"` Summary string `json:"summary"` + // Trunk is the trunk commit the head was merged into when its verdict judged it (novox/hq ADR 0323 §3). + Trunk string `json:"trunk,omitempty"` } // receiving are the machines the plan sends a module to, in the plan's order; none when the plan names no @@ -752,6 +797,13 @@ type GroupCheck struct { Commits map[string]string `json:"commits"` Verdict string `json:"verdict,omitempty"` Summary string `json:"summary,omitempty"` + // Trunks are the trunk commit each member's head was merged into, by member id (novox/hq ADR 0323 §6). + Trunks map[string]string `json:"trunks,omitempty"` + // OutOfDate is why its verdict no longer counts: a merge that is not one of the group's members moved a + // member's trunk, at OutOfDateAt. It counts as outstanding (ADR 0239 §18), and runs again once a member is + // judged again after it. + OutOfDate string `json:"out_of_date,omitempty"` + OutOfDateAt time.Time `json:"out_of_date_at"` } // Commits is each member's commit, by id. diff --git a/modules/mesh-delivery/cmd/mesh-delivery/ports.go b/modules/mesh-delivery/cmd/mesh-delivery/ports.go index 8675075e..c0185b1d 100644 --- a/modules/mesh-delivery/cmd/mesh-delivery/ports.go +++ b/modules/mesh-delivery/cmd/mesh-delivery/ports.go @@ -31,13 +31,16 @@ type Member struct { ModuleDirsSaid bool `json:"module_dirs_said,omitempty"` CloneURL string `json:"clone_url,omitempty"` After []string `json:"after,omitempty"` + // BaseSHA is the base branch's head the forge last announced the head with (novox/hq ADR 0323): the trunk commit + // the controller's verdict names when no build judges the head. + BaseSHA string `json:"base_sha,omitempty"` } // MemberOf is a delivery as a member. func MemberOf(d *Delivery) Member { return Member{ID: d.ID, Repository: d.Repository, Base: d.Base, Head: d.Commit, Number: d.Number, Paths: d.Paths, PathsTruncated: d.PathsTruncated, Removed: d.Removed, ModuleDirs: d.ModuleDirs, ModuleDirsSaid: d.ModuleDirsSaid, - CloneURL: d.CloneURL, After: d.After} + CloneURL: d.CloneURL, After: d.After, BaseSHA: d.BaseSHA} } // Order is a group's order as the controller works it out: each pair with the rules it won over by diff --git a/modules/mesh-delivery/cmd/mesh-delivery/table.go b/modules/mesh-delivery/cmd/mesh-delivery/table.go index 5203ee17..04fa603f 100644 --- a/modules/mesh-delivery/cmd/mesh-delivery/table.go +++ b/modules/mesh-delivery/cmd/mesh-delivery/table.go @@ -85,6 +85,7 @@ const ( EvCovered Event = "covered" // a later merge into the same trunk delivered every module it moves EvLifted Event = "lifted" // what held it for a person is no longer so: its group's composed check passed, or a walk now answers its merge EvRewalk Event = "rewalk" // the walk that carried a later commit for it failed, or was stopped for another merge (ADR 0299): it waits for the walk that walks it again + EvTrunkMoved Event = "trunk moved" // another merge moved its trunk, and its verdict names the trunk commit before (novox/hq ADR 0323) ) // Facts are what a guard reads beyond the delivery itself: the moment, and who asks, for an act. @@ -167,9 +168,17 @@ var Table = []Row{ Holds: func(d *Delivery, f Facts) error { return unless(d.MergedAs != "" && (d.Walk != nil || d.MovesNothing()), "not on the trunk yet") }}, + // **A verdict counts only for the trunk's head** (novox/hq ADR 0323 §7): one naming a trunk commit that is no + // longer the trunk's head is kept and shown, and does not count, so the delivery stays proposed and the rows + // cannot loop with `trunk moved`; a recheck or the merge tool's ask judges it again. {From: []State{Proposed}, Event: EvChecked, To: Checked, - Guard: "the verdict names this commit", - Holds: func(d *Delivery, f Facts) error { return unless(d.Check != nil, "no verdict") }}, + Guard: "a verdict names this commit and the trunk's head as its trunk commit", + Holds: func(d *Delivery, f Facts) error { + if d.Check == nil { + return errors.New("no verdict") + } + return unless(d.outOfDate() == "", "its verdict is "+d.outOfDate()) + }}, {From: []State{Checked}, Event: EvAccepted, To: Ready, Guard: "the gate passed or warned, and the repository's own check did not fail", Holds: func(d *Delivery, f Facts) error { return unless(d.Check.Passes(), "the verdict does not pass") }}, @@ -199,6 +208,14 @@ var Table = []Row{ Says: func(d *Delivery) string { return "covered: a later merge that contains it was delivered: " + d.CoveredBy }}, + // **A ready delivery whose verdict went out of date goes back to proposed** (novox/hq ADR 0323 §7): another + // merge moved its trunk. A rejected one stays rejected, its status on the forge saying the verdict is out of date. + {From: []State{Ready}, Event: EvTrunkMoved, To: Proposed, + Guard: "its verdict names a trunk commit that is not its trunk's head", + Holds: func(d *Delivery, f Facts) error { + return unless(d.MergedAs == "" && d.outOfDate() != "", "not merged, and its verdict is not out of date") + }, + Says: func(d *Delivery) string { return d.outOfDate() }}, {From: []State{Proposed, Checked, Rejected}, Event: EvMergedUnchecked, To: Held, Guard: "merged without a passing check — a verdict known with it is taken first: only a person decides that it goes on", Holds: func(d *Delivery, f Facts) error { return unless(d.MergedAs != "", "not merged") }}, @@ -543,6 +560,9 @@ const checkGrace = 15 * time.Minute // runs from its start plus the grace before it is asked: an ask that cannot be made is still listed. func boundSince(d *Delivery) time.Time { if d.State == Proposed { + if d.AskedAgain != nil && d.outOfDate() != "" { + return *d.AskedAgain + } if d.CheckAsk != nil { return d.CheckAsk.At } @@ -562,6 +582,17 @@ var rewalkBound = Bound{For: unwalked, Says: "the walk that carried a later comm // boundOf is a delivery's bound: its state's, or none for a delivery held because no walk answers it. func boundOf(d *Delivery) (Bound, bool) { + if d.State == Proposed && d.outOfDate() != "" { + // Its check is out of date and not asked: nothing is running for it, and it costs nothing (ADR 0323 §7). + // Its bound starts when its check is asked: a recheck puts the verdict away; the merge tool's ask announces the + // head again, and its bound runs from then. + if d.AskedAgain == nil { + return Bound{}, false + } + b := Bounds[Proposed] + b.H2, b.Says = nil, "its verdict is "+d.outOfDate()+"; its check was asked again and not answered: the operator's" + return b, true + } if d.State == Held && d.Unwalked && d.Walk == nil { return heldUnwalked, true } diff --git a/modules/mesh-delivery/cmd/mesh-delivery/table_test.go b/modules/mesh-delivery/cmd/mesh-delivery/table_test.go index 75f5fecc..c1f902cf 100644 --- a/modules/mesh-delivery/cmd/mesh-delivery/table_test.go +++ b/modules/mesh-delivery/cmd/mesh-delivery/table_test.go @@ -82,6 +82,9 @@ func aDeliveryFor(r Row) (*Delivery, Facts) { case EvRewalk: d.MergedAs, d.AnsweredBy = "aaaa1111", "bbbb2222" d.Walk = &WalkSeen{ID: "plan-2", State: walkFailed} + case EvTrunkMoved: + d.Check = &Verdict{Gate: "pass", Head: "c0ffee000000", Trunk: "d28ca95bbbbb"} + d.TrunkNow = "690966dccccccccc" case EvLifted: d.MergedAs, d.Lifted = "aaaa1111", "its group's composed check passed" d.Walk = &WalkSeen{ID: "plan-1", State: walkBuilding, Waits: true, WaitedFor: "mesh-delivery"} @@ -129,7 +132,7 @@ func TestEveryRowOfTheTableIsTakenWhenItsGuardHoldsAndRefusedWhenNot(t *testing. func TestEveryPairTheTableDoesNotHoldIsRefusedByName(t *testing.T) { events := []Event{EvAnnounced, EvAppeared, EvAdopted, EvChecked, EvAccepted, EvRefused, EvRecheck, EvReask, EvNewHead, EvClosed, EvMerged, EvMergedUnchecked, EvGo, EvHold, EvRelease, EvDone, EvFailed, EvSuperseded, EvStop, - EvHistory, EvRetire, EvCovered, EvLifted, EvRewalk} + EvHistory, EvRetire, EvCovered, EvLifted, EvRewalk, EvTrunkMoved} for _, from := range append([]State{None}, AllStates...) { for _, ev := range events { for _, act := range []bool{false, true} { diff --git a/modules/mesh-delivery/cmd/mesh-delivery/unchecked.go b/modules/mesh-delivery/cmd/mesh-delivery/unchecked.go index 0db11ed5..2947b823 100644 --- a/modules/mesh-delivery/cmd/mesh-delivery/unchecked.go +++ b/modules/mesh-delivery/cmd/mesh-delivery/unchecked.go @@ -44,6 +44,10 @@ const Unannounced State = "unannounced" // own for the proposed state's bound: its check was asked and not answered. const Unanswered State = "unanswered" +// outOfDatePrefix begins the words the forge's holder puts a merge check back to pending with when its trunk moved +// (novox/hq ADR 0323 §4): the gitea module's outOfDateWords. +const outOfDatePrefix = "out of date:" + // UncheckedAfter is how long a head may go without a merge check before it is said: the announcer passes every half // minute and the check is asked at once, so ten minutes is a fault, never a queue. const UncheckedAfter = 10 * time.Minute @@ -119,6 +123,9 @@ func (p OpenPull) unanswered() (waiting []string, pending map[string]string, sta // since nothing here says whether the one it has answered. case !ok || got.State == "": waiting = append(waiting, c+" never set") + case got.State == "pending" && strings.HasPrefix(got.Description, outOfDatePrefix): + // Out of date because its trunk moved, and not asked (novox/hq ADR 0323 §4, §7): nothing runs for it and + // it costs nothing, so it is not a check asked and not answered. Asked again, it is pending anew. case got.State == "pending": pending[c] = utcOf(got.UpdatedAt) waiting = append(waiting, c+" pending since "+orUnknown(pending[c])) diff --git a/modules/mesh-delivery/cmd/mesh-delivery/verbs.go b/modules/mesh-delivery/cmd/mesh-delivery/verbs.go index 07f5642d..bcaa8e57 100644 --- a/modules/mesh-delivery/cmd/mesh-delivery/verbs.go +++ b/modules/mesh-delivery/cmd/mesh-delivery/verbs.go @@ -229,6 +229,8 @@ func (h *Holder) Stalled() []StalledLine { b, _ := boundOf(d) h2 := "none: the state is the operator's" switch { + case d.State == Proposed && d.AskedAgain != nil && d.outOfDate() != "": + h2 = "none: its verdict is out of date and its check, asked again, has not answered — the operator's" case d.State == Proposed && d.CheckAsk == nil: h2 = "none: its check could not be asked yet — the controller does not answer delivery-check" case d.State == Proposed && d.CheckAsk.Reasked > 0: