Serve the merge tool beside the one writer, and close the review's gaps
mesh/delivery-group group feat/341-a-merge-check-judges-the-merge rejected: a member's own check failed
mesh/delivery superseded: a newer head of the same pull request
mesh/merge-gate pass: builds gitea, mesh-delivery → novox; no bus step; every machine composes with the change as it did without (4 of 4 compose)
mesh/repo-check pass: its merge-check.sh passed, each part in its toolchain: merge-check.sh (go), merge-check-typescript.sh (typescript)

The merge tool's pending bypassed the merge check's one writer from the tools'
bundle (issue 456); it is now served beside it. A verdict no build judges names
the announced base head, so the merge tool no longer refuses it for ever. A
late verdict keeps its account, a trunk is read again in each head's turn so a
fresh verdict is not put out of date, and a delivered member counts when a
rebased member's group is composed again (novox/hq ADR 0323).
This commit is contained in:
2026-10-12 17:46:39 +02:00
parent 82cca650a2
commit 434506f9b1
10 changed files with 317 additions and 186 deletions
+3 -3
View File
@@ -184,12 +184,12 @@ export class GateWriter {
* 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<string, HeadCheck>) => CommitStatus[],
target?: string): Promise<CommitStatus[]> {
outOfDate(owner: string, repo: string, sha: string,
decide: (checks: Record<string, HeadCheck>) => CommitStatus[] | Promise<CommitStatus[]>, target?: string): Promise<CommitStatus[]> {
const h = this.head(owner, repo, sha);
const written: CommitStatus[] = [];
return this.turn(h, async () => {
for (const s of decide(await this.forge.headChecks(owner, repo, sha))) {
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);
}
+8 -4
View File
@@ -22,8 +22,8 @@
// 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 from this bundle, not the tools', so a recheck's pending reaches that one writer
// (novox/hq issue 456).
// 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.
@@ -39,11 +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, outOfDateWords, sameCommit, tookFile, weighTrunks } from "./trunk.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
@@ -342,6 +343,9 @@ async function setVerdict(client: GiteaClient, event: { body: unknown }): Promis
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;
@@ -388,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.
+183
View File
@@ -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<void> };
emit?: (type: string, body: unknown) => Promise<void>;
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<string, unknown> = { 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;
},
};
}
+19 -9
View File
@@ -14,6 +14,7 @@ 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";
@@ -126,12 +127,18 @@ 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-"));
const tools = [...getGiteaTools(client, { stateDir, emit: async (type, body) => { state.emitted.push({ type, body }); },
now: () => new Date(2026, 9, 12, 15, 1) }), 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 () => [] }),
})];
// 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: 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}`);
@@ -254,13 +261,17 @@ test("gitea_merge_pull_request refuses a verdict judged against an earlier trunk
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/);
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/);
@@ -281,8 +292,7 @@ test("gitea_merge_pull_request refuses a verdict judged against an earlier trunk
// 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 = getGiteaTools(client, { stateDir, emit: async () => { throw new Error("the bus is away"); } })
.find((t) => t.name === "gitea_merge_pull_request")!;
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: /);
+2 -1
View File
@@ -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",
+23 -1
View File
@@ -6,7 +6,7 @@
import { test } from "node:test";
import assert from "node:assert/strict";
import { checkOnItsWay, outOfDateStatuses, outOfDateWords, trunkNamed, verdictAtTrunk, weighTrunks } from "../dist/trunk.js";
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";
@@ -151,3 +151,25 @@ test("the trunk's move takes its turn after a verdict in flight, and weighs what
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<string, string> = { main: TRUNK };
const { forge, statuses } = aForge([{ number: 1, base: "main", head: HEAD }], heads);
const trunks: Record<string, string> = { "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/);
});
+6 -163
View File
@@ -13,8 +13,6 @@ import { emit } from "@novox/mesh-sdk/events";
import { GiteaClient, type BranchProtection } from "../client.js";
import { appendNote, run, viewBody, viewComment } from "../delivery.js";
import { containedFrom, isCommit, mergeableUnder, newestByContext } from "../checks.js";
import { CHECK_CONTEXT, REPO_CHECK_CONTEXT } from "../pulls.js";
import { announcementOf, askedWords, outOfDateWords, tookFile, tookWords, twelve, verdictAtTrunk } from "../trunk.js";
import {
MERGE_METHODS, Refused, checkName, judgeApiCall, judgeContentsWrite, judgeDelete, judgeProtectionChange, pathSegments, routedPath, sentQuery,
ruleCovers,
@@ -61,19 +59,13 @@ function namesChecked(tools: ToolDefinition[]): ToolDefinition[] {
}));
}
/** What the tools reach beside the forge: how an event is said, the module's state directory, and the clock. */
export interface ToolDeps {
emit?: (type: string, body: unknown) => Promise<void>;
stateDir?: string;
now?: () => Date;
export function getGiteaTools(gitea: GiteaClient): ToolDefinition[] {
return namesChecked(allGiteaTools(gitea));
}
export function getGiteaTools(gitea: GiteaClient, deps: ToolDeps = {}): ToolDefinition[] {
return namesChecked(allGiteaTools(gitea, { emit: deps.emit ?? emit, stateDir: deps.stateDir ?? process.env.MESH_GITEA_STATE_DIR,
now: deps.now ?? (() => new Date()) }));
}
function allGiteaTools(gitea: GiteaClient, deps: Required<Pick<ToolDeps, "emit" | "now">> & ToolDeps): ToolDefinition[] {
// 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 ----
{
@@ -273,155 +265,6 @@ function allGiteaTools(gitea: GiteaClient, deps: Required<Pick<ToolDeps, "emit"
}),
}),
},
{
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 = 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`);
}
// **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). 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] : [])];
const say = async (description: string) => {
for (const context of contexts) {
await gitea.setCommitStatus(owner, repo, 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). 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<string, unknown> = { 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.",
@@ -768,7 +611,7 @@ function allGiteaTools(gitea: GiteaClient, deps: Required<Pick<ToolDeps, "emit"
// way, gitea contributes none rather than failing the whole runtime, and says why.
registerModuleTools("gitea", (env) => {
try {
return getGiteaTools(GiteaClient.fromEnv(env), { stateDir: env.MESH_GITEA_STATE_DIR });
return getGiteaTools(GiteaClient.fromEnv(env));
} catch (err) {
console.log(`[gitea] no tools — ${err instanceof Error ? err.message : String(err)}`);
return [];
+18 -4
View File
@@ -100,6 +100,14 @@ export function askedWords(at: Date, trunkHead: string): string {
`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. */
@@ -110,8 +118,8 @@ export interface TrunkForge {
/** 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<string, HeadCheck>) => CommitStatus[],
target?: string): Promise<CommitStatus[]>;
outOfDate(owner: string, repo: string, sha: string,
decide: (checks: Record<string, HeadCheck>) => CommitStatus[] | Promise<CommitStatus[]>, target?: string): Promise<CommitStatus[]>;
}
/**
@@ -135,8 +143,10 @@ export async function weighTrunks(forge: TrunkForge, gates: OutOfDateWriter, tru
if (before) {
let put = 0;
for (const pull of open.filter((p) => p.base === base)) {
const written = await gates.outOfDate(owner, name, String(pull.head_sha), (checks) => outOfDateStatuses(checks, head),
pull.html_url || undefined);
// 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 ` +
@@ -216,10 +226,14 @@ export async function announcementOf(client: GiteaClient, owner: string, name: s
} 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,
@@ -1569,7 +1569,8 @@ func (h *Holder) planGroup(g *Group, members []*Delivery, now time.Time) []func(
// 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 && !(g.Closed && d.MergedAs != "" && !d.State.Final()) {
// 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
}
}
@@ -385,3 +385,56 @@ func TestAnOutOfDateDeliveryAskedAgainIsBoundFromTheAsk(t *testing.T) {
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)
}
}