From 67bce7e1897ac8dc0098c0bd7fcbc841fd66ef67 Mon Sep 17 00:00:00 2001 From: jochen Date: Fri, 9 Oct 2026 12:55:09 +0200 Subject: [PATCH] Announce a pull request opened after its push, and say one nobody checks (hq issue 347) The announcer read only the repositories whose update time moved since its last pass, less a minute. Opening, reopening or retargeting a pull request does not move a repository's update time, so a pull request opened more than a minute after its push was never announced, never checked, and nothing said so (mesh-sdk #14, 2026-10-09). - Each pass also looks at every repository holding a pull request updated since the floor, by the forge's own search; every ten minutes, or when the search fails, at every repository. - Every five minutes the forge is asked for every open pull request; one on a base requiring mesh/merge-gate whose head has had none for ten minutes is said as the forge failing that pull request's merge check (a provider standing, ADR 0224), which the controller keeps as a condition, and is recovered when a check arrives or it closes. - The client refuses a pull request or issue number that is not one before asking the forge (a misnamed tool argument sent pulls/NaN). --- modules/gitea/README.md | 18 ++++++ modules/gitea/client.ts | 40 +++++++++++++ modules/gitea/index.ts | 84 ++++++++++++++++++++++++++- modules/gitea/pulls.ts | 48 ++++++++++++++- modules/gitea/test/announcing.test.ts | 61 +++++++++++++++++++ 5 files changed, 248 insertions(+), 3 deletions(-) create mode 100644 modules/gitea/test/announcing.test.ts diff --git a/modules/gitea/README.md b/modules/gitea/README.md index 1cf89306..b6f90d2b 100644 --- a/modules/gitea/README.md +++ b/modules/gitea/README.md @@ -4,6 +4,24 @@ The forge, and the mesh's npm package registry: it claims the `git` and `npm-pac Its tools, its events and the provisioner of registry accounts are the TypeScript bundle (`index.ts`, `tools/`, `provisioner/`). This README covers the Go bundle beside it. +## Announcing pull requests to the merge check (novox/hq to-be 45 §9, issue 347) + +Every new head of an open pull request is announced as `pull.updated`, once, and the controller has it checked. + +- **Which repositories a pass looks at.** Each that moved since the last pass, less a minute's margin (issue 250); + each holding a pull request updated since then, found by the forge's own search — opening, reopening or + retargeting a pull request moves its own update time and not its repository's, and a pull request opened more + than a minute after its push was never announced (issue 347); and every repository on a sweep, every ten + minutes, or when the search fails. +- **A pull request nobody checks is said.** Every five minutes the forge itself is asked for every open pull + request; one whose base requires `mesh/merge-gate` and whose head has had no such status for ten minutes is + said as this forge failing *the merge check of `/#`* (`provisioner.failing`, class + `unannounced`), which the controller keeps as a condition (ADR 0224), said again each pass while it lasts, and + recovered when a check arrives or the pull request closes. What is said is kept in `pulls-unannounced.json` + beside the announcer's record, so a restart recovers what it said. +- **A pull request or issue number that is not one is refused** before the forge is asked: a tool called with a + misnamed argument sent `pulls/NaN`. + ## The package registry's retention (novox/hq ADR 0251 §4, to-be 51) Every publish adds a version to the registry, and until this nothing removed one. The Go bundle diff --git a/modules/gitea/client.ts b/modules/gitea/client.ts index 10577d13..4458796b 100644 --- a/modules/gitea/client.ts +++ b/modules/gitea/client.ts @@ -194,6 +194,7 @@ export class GiteaClient { } async getIssue(owner: string, repo: string, index: number): Promise { + indexOf(index); return GiteaClient.mapIssue(await this.request(`/repos/${owner}/${repo}/issues/${index}`)); } @@ -209,6 +210,7 @@ export class GiteaClient { /** Patch an issue's state — the one edit the close tool needs. */ async setIssueState(owner: string, repo: string, index: number, state: "open" | "closed"): Promise { + indexOf(index); return GiteaClient.mapIssue( await this.request(`/repos/${owner}/${repo}/issues/${index}`, { method: "PATCH", @@ -218,6 +220,7 @@ export class GiteaClient { } async addComment(owner: string, repo: string, index: number, body: string): Promise<{ id: number; html_url: string }> { + indexOf(index); const c = await this.request(`/repos/${owner}/${repo}/issues/${index}/comments`, { method: "POST", body: JSON.stringify({ body }), @@ -257,7 +260,28 @@ export class GiteaClient { return (prs ?? []).map(GiteaClient.mapPull); } + /** The open pull requests of every repository the mesh's account sees, updated since a moment when one is given, + * page by page (novox/hq issue 347): each with its repository and its own update time. */ + async searchOpenPulls(since?: string): Promise<{ owner: string; name: string; number: number; updated_at?: string }[]> { + const out: { owner: string; name: string; number: number; updated_at?: string }[] = []; + for (let page = 1; page <= 20; page++) { + const qs = new URLSearchParams({ type: "pulls", state: "open", limit: "50", page: String(page) }); + if (since) qs.set("since", since); + const found = await this.request(`/repos/issues/search?${qs}`); + for (const i of found ?? []) { + const r = i?.repository ?? {}; + const [fo, fn] = String(r.full_name ?? "").split("/"); + const owner = String(r.owner ?? fo ?? ""); + const name = String(r.name ?? fn ?? ""); + if (owner && name && i?.number) out.push({ owner, name, number: Number(i.number), updated_at: i.updated_at }); + } + if (!found || found.length < 50) break; + } + return out; + } + async getPullRequest(owner: string, repo: string, index: number): Promise { + indexOf(index); return GiteaClient.mapPull(await this.request(`/repos/${owner}/${repo}/pulls/${index}`)); } @@ -275,6 +299,7 @@ export class GiteaClient { * deleted is gone from its source, and the mesh forgets it rather than asking its build (novox/hq ADR * 0236). The forge says `deleted`; `removed` is read the same. */ async listPullFiles(owner: string, repo: string, index: number, most = 3000): Promise<{ paths: string[]; removed: string[]; truncated: boolean }> { + indexOf(index); const paths: string[] = []; const removed: string[] = []; let pageSize = 0; @@ -408,6 +433,7 @@ export class GiteaClient { /** Close or reopen a pull request without merging it. A pull request is an issue to the forge's * state machine, and the pulls endpoint takes the same `state`. */ async setPullState(owner: string, repo: string, index: number, state: "open" | "closed"): Promise { + indexOf(index); return GiteaClient.mapPull( await this.request(`/repos/${owner}/${repo}/pulls/${index}`, { method: "PATCH", body: JSON.stringify({ state }) }), ); @@ -415,6 +441,7 @@ export class GiteaClient { /** Change a pull request's title or body; a field left undefined is left alone. */ async updatePullRequest(owner: string, repo: string, index: number, data: { title?: string; body?: string }): Promise { + indexOf(index); return GiteaClient.mapPull( await this.request(`/repos/${owner}/${repo}/pulls/${index}`, { method: "PATCH", body: JSON.stringify(data) }), ); @@ -422,11 +449,13 @@ export class GiteaClient { /** The unified diff of a pull request, as text. */ async pullDiff(owner: string, repo: string, index: number): Promise { + indexOf(index); return this.requestText(`/repos/${owner}/${repo}/pulls/${index}.diff`); } /** Every comment on an issue or pull request, oldest first. */ async listComments(owner: string, repo: string, index: number): Promise { + indexOf(index); const raw = await this.request(`/repos/${owner}/${repo}/issues/${index}/comments`); return (raw ?? []).map((c) => ({ id: Number(c?.id ?? 0), @@ -464,6 +493,7 @@ export class GiteaClient { } async mergePullRequest(owner: string, repo: string, index: number, method = "merge", deleteBranch = false): Promise { + indexOf(index); await this.request(`/repos/${owner}/${repo}/pulls/${index}/merge`, { method: "POST", body: JSON.stringify({ Do: method, delete_branch_after_merge: deleteBranch }), @@ -729,6 +759,16 @@ export class GiteaAdmin { } } +/** A pull request's or issue's number, refused unless it is one: a tool asked with a misnamed argument hands over + * `NaN`, and the forge was asked about `pulls/NaN` (novox/hq issue 347). */ +export function indexOf(index: number): number { + if (!Number.isInteger(index) || index <= 0) { + throw new Error(`a pull request or issue number is a whole number above 0, and ${String(index)} was given — ` + + "is the argument named \`number\`? Nothing was asked of the forge"); + } + return index; +} + /** The repositories that moved at or after a moment: every one when there is no moment yet, and one whose * update time is not known, so a forge that does not say is asked as before (novox/hq issue 250). */ export function movedSince(repos: GiteaRepo[], floor: string): GiteaRepo[] { diff --git a/modules/gitea/index.ts b/modules/gitea/index.ts index 25953001..934960ba 100644 --- a/modules/gitea/index.ts +++ b/modules/gitea/index.ts @@ -28,7 +28,7 @@ import { emit, on } from "@novox/mesh-sdk/events"; import { GiteaClient, movedSince } from "./client.js"; import { CHECK_CONTEXT, REPO_CHECK_CONTEXT, commentFor, headsToAnnounce, statusesFor, type Announced, type Checked, - type RepoCheckFacts } from "./pulls.js"; + type RepoCheckFacts, reposToLook, unannounced, type OpenPull } from "./pulls.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 @@ -213,12 +213,29 @@ function keepHeads(): void { renameSync(tmp, pullsRecord); } let lastPullLook = ""; +// **A sweep now and then looks at every repository** (novox/hq issue 347), whatever moved: the floor is a +// heuristic, and a heuristic that misses is caught by the next sweep rather than never. +const SWEEP_MS = 10 * 60_000; +let lastSweep = 0; async function pollPulls(client: GiteaClient): Promise { const began = new Date().toISOString(); const floor = lastPullLook ? new Date(Date.parse(lastPullLook) - MARGIN_MS).toISOString() : ""; const dayAgo = new Date(Date.now() - 24 * 3600_000).toISOString(); let changed = false; - for (const repo of movedSince(await client.listAllRepos(), floor)) { + // A pull request opened, reopened or retargeted moves its own update time and not its repository's: the + // pull requests updated since the floor are found by their own (issue 347). A search that fails is a sweep. + let sweep = Date.now() - lastSweep >= SWEEP_MS; + let updated: { owner: string; name: string; updated_at?: string }[] = []; + if (floor && !sweep) { + try { + updated = await client.searchOpenPulls(floor); + } catch (err) { + console.error(`[gitea] the pull requests updated lately could not be searched, so every repository is looked at — ${err instanceof Error ? err.message : err}`); + sweep = true; + } + } + if (sweep) lastSweep = Date.now(); + for (const repo of reposToLook(await client.listAllRepos(), floor, updated, sweep)) { const open = await client.listPullRequests(repo.owner, repo.name, { state: "open", sort: "recentupdate", limit: "20" }); for (const pull of headsToAnnounce(repo.full_name, open, heads)) { const key = `${repo.full_name}#${pull.number}`; @@ -273,6 +290,68 @@ async function pollPulls(client: GiteaClient): Promise { lastPullLook = began; } +// **An open pull request nobody checks is said** (novox/hq issue 347). One whose base requires the merge check and +// whose head has had none for ten minutes was announced by nobody — the fault this watcher exists for, which went +// unseen while the pull request could not merge. Said as this forge failing the merge check, the provider standing +// the controller keeps as a condition (ADR 0224) and says to the operator; said again every pass while it lasts, and +// recovered when a check arrives or the pull request is no longer open. Asked of the forge itself, never of what +// the announcer remembers, so a fault in the announcer cannot hide itself. +const unannouncedRecord = process.env.MESH_GITEA_STATE_DIR ? join(process.env.MESH_GITEA_STATE_DIR, "pulls-unannounced.json") : null; +let failing: Record = {}; +if (unannouncedRecord && existsSync(unannouncedRecord)) { + try { + failing = (JSON.parse(readFileSync(unannouncedRecord, "utf8")) as { failing: Record }).failing ?? {}; + } catch { + // Unreadable is no record: a pull request still unchecked is said again at once. + } +} +function keepFailing(): void { + if (!unannouncedRecord) return; + mkdirSync(join(unannouncedRecord, ".."), { recursive: true }); + const tmp = unannouncedRecord + ".tmp"; + writeFileSync(tmp, JSON.stringify({ failing })); + renameSync(tmp, unannouncedRecord); +} +const consumerOf = (key: string) => `the merge check of ${key}`; +async function watchUnannounced(client: GiteaClient): Promise { + const open: OpenPull[] = []; + const rules = new Map> | undefined>(); + for (const found of await client.searchOpenPulls()) { + const full = `${found.owner}/${found.name}`; + const pull = await client.getPullRequest(found.owner, found.name, found.number); + if (pull.state !== "open" || !pull.head_sha) continue; + if (!rules.has(full)) rules.set(full, await client.branchProtections(found.owner, found.name).catch(() => undefined)); + const rule = rules.get(full)?.find((p) => (p.rule_name ?? p.branch_name) === (pull.base || "main")); + const statuses = Object.keys(await client.commitStatuses(found.owner, found.name, pull.head_sha)); + open.push({ key: `${full}#${pull.number}`, base: pull.base ?? "", head_sha: pull.head_sha, updated_at: pull.updated_at, + statuses, required: rule?.enable_status_check ? (rule.status_check_contexts ?? []) : [] }); + } + const now = Date.now(); + const unchecked = new Map(unannounced(open, now).map((p) => [p.key, p] as const)); + const node = process.env.MESH_NODE ?? ""; + for (const [key, p] of unchecked) { + const since = failing[key] ?? p.updated_at ?? new Date(now).toISOString(); + await emit("provisioner.failing", { + provider: "git", "provider-node": node, consumer: consumerOf(key), node: "", class: "unannounced", + error: `${key} is open on ${p.base}, which requires ${CHECK_CONTEXT}, and its head ${p.head_sha.slice(0, 12)} has had no ` + + `merge check for ${Math.round((now - Date.parse(p.updated_at ?? since)) / 60_000)} minutes: the mesh was never asked to check it`, + since, attempts: 1, + }); + if (!failing[key]) console.error(`[gitea] ${key} has had no merge check for ten minutes: said to the mesh`); + failing[key] = since; + } + for (const key of Object.keys(failing)) { + if (unchecked.has(key)) continue; + await emit("provisioner.recovered", { + provider: "git", "provider-node": node, consumer: consumerOf(key), node: "", since: failing[key], attempts: 0, + why: "it has a merge check, or is no longer open", + }); + console.log(`[gitea] ${key} has a merge check now, or is no longer open`); + delete failing[key]; + } + keepFailing(); +} + // **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. @@ -346,6 +425,7 @@ if (gitea) { tick(() => pollRepos(client), 60_000); tick(() => pollMerged(client), 30_000); tick(() => pollPulls(client), 30_000); + tick(() => watchUnannounced(client), 5 * 60_000); await on("mesh-controller.checked", (event) => setVerdict(client, event)); console.log("[gitea] watching for new repositories and merged pull requests"); } diff --git a/modules/gitea/pulls.ts b/modules/gitea/pulls.ts index c5b001d9..7020fa38 100644 --- a/modules/gitea/pulls.ts +++ b/modules/gitea/pulls.ts @@ -9,7 +9,8 @@ // // Pure functions here, so they are tested without a forge; index.ts does the asking and the setting. -import type { CommitStatus, GiteaPull } from "./client.js"; +import type { CommitStatus, GiteaPull, GiteaRepo } from "./client.js"; +import { movedSince } from "./client.js"; /** The status context a merge check's gate is kept under: one per commit, the newest replacing the last. */ export const CHECK_CONTEXT = "mesh/merge-gate"; @@ -92,6 +93,51 @@ export function headsToAnnounce(full: string, pulls: GiteaPull[], announced: Ann return pulls.filter((p) => p.state === "open" && !!p.head_sha && announced[`${full}#${p.number}`] !== p.head_sha); } +/** A pull request the forge found updated, by its repository: what `searchOpenPulls` answers. */ +export interface PullSeen { + owner: string; + name: string; + updated_at?: string; +} + +/** **The repositories a pass looks at** (novox/hq issue 347): every one that moved since the floor, every one + * holding a pull request updated since the floor — opening, reopening or retargeting a pull request moves its own + * update time and not its repository's — and every one on a sweep or the first pass. */ +export function reposToLook(repos: GiteaRepo[], floor: string, pulls: PullSeen[], sweep: boolean): GiteaRepo[] { + if (sweep || !floor) return repos; + const moved = new Set(movedSince(repos, floor).map((r) => r.full_name)); + const at = Date.parse(floor); + for (const p of pulls) { + if (!p.updated_at || !(Date.parse(p.updated_at) < at)) moved.add(`${p.owner}/${p.name}`); + } + return repos.filter((r) => moved.has(r.full_name)); +} + +/** How long a head may go without a merge check before it is said: the announcer's passes are half a minute + * apart and the check is asked at once, so ten minutes is a fault, never a queue. */ +export const UNANNOUNCED_AFTER_MS = 10 * 60_000; + +/** An open pull request as the watcher of announcements reads it. */ +export interface OpenPull { + /** `owner/repo#number`. */ + key: string; + base: string; + head_sha: string; + /** When its head last moved, or it was opened or retargeted. */ + updated_at?: string; + /** The status contexts its head has, any state. */ + statuses: string[]; + /** The contexts its base branch's protection requires. */ + required: string[]; +} + +/** The open pull requests whose base requires the merge check and whose head has had none for ten minutes + * (novox/hq issue 347): announced by nobody, checked by nothing, and blocked from merging with no word why. */ +export function unannounced(pulls: OpenPull[], now: number, after = UNANNOUNCED_AFTER_MS): OpenPull[] { + return pulls.filter((p) => p.required.includes(CHECK_CONTEXT) && !p.statuses.includes(CHECK_CONTEXT) && + !!p.updated_at && now - Date.parse(p.updated_at) >= after); +} + // **A required status is success or it blocks** (novox/hq issue 293). The forge combines `warning` as a // failure, and a branch whose protection requires mesh/merge-gate or mesh/repo-check — with no // administrator override — will not merge past one. So the controller's `warning`, a note and never a diff --git a/modules/gitea/test/announcing.test.ts b/modules/gitea/test/announcing.test.ts new file mode 100644 index 00000000..a6093e16 --- /dev/null +++ b/modules/gitea/test/announcing.test.ts @@ -0,0 +1,61 @@ +import assert from "node:assert/strict"; +import { test } from "node:test"; +import { createServer } from "node:http"; + +// **A pull request opened after its push is still checked** (novox/hq issue 347). The announcer read only the +// repositories whose update time moved since its last pass, less a minute's margin; opening, reopening or +// retargeting a pull request does not move a repository's update time, so a pull request opened more than a +// minute or so after its push was never announced, never checked, and nothing said so. + +const repo = (full: string, updated: string) => { + const [owner, name] = full.split("/"); + return { full_name: full, owner, name, private: false, html_url: "", updated_at: updated }; +}; + +test("a pull request opened after the margin is looked at, by its own update time", async () => { + const { reposToLook } = await import("../pulls.ts"); + const pushed = "2026-10-09T10:00:00Z"; // the branch pushed: the repository's update time + // The pull request opened two minutes later; the last pass was at 10:02:30, so the floor is 10:01:30. + const floor = "2026-10-09T10:01:30Z"; + const repos = [repo("novox/mesh-sdk", pushed), repo("novox/hq", "2026-10-09T09:00:00Z")]; + const opened = [{ owner: "novox", name: "mesh-sdk", updated_at: "2026-10-09T10:02:00Z" }]; + assert.deepEqual(reposToLook(repos, floor, opened, false).map((r) => r.full_name), ["novox/mesh-sdk"], + "the repository of a pull request updated since the floor, though the repository itself did not move"); + assert.deepEqual(reposToLook(repos, floor, [], false).map((r) => r.full_name), [], + "without the pull request's own time nothing is looked at — the fault"); + assert.deepEqual(reposToLook(repos, floor, [], true).map((r) => r.full_name).sort(), ["novox/hq", "novox/mesh-sdk"], + "a sweep looks at every repository"); + assert.deepEqual(reposToLook(repos, "", [], false).length, 2, "the first pass looks at every repository"); +}); + +test("an open pull request on a protected branch with no merge check for ten minutes is said, and nothing else", async () => { + const { unannounced } = await import("../pulls.ts"); + const now = Date.parse("2026-10-09T10:30:00Z"); + const pull = { + key: "novox/mesh-sdk#14", base: "main", head_sha: "bfe82315f42c", updated_at: "2026-10-09T10:12:00Z", + statuses: [] as string[], required: ["mesh/merge-gate", "mesh/repo-check"], + }; + assert.deepEqual(unannounced([pull], now).map((p) => p.key), ["novox/mesh-sdk#14"]); + assert.deepEqual(unannounced([{ ...pull, statuses: ["mesh/merge-gate"] }], now), [], "a head with a merge check"); + assert.deepEqual(unannounced([{ ...pull, required: [] }], now), [], "a branch that requires no merge check"); + assert.deepEqual(unannounced([{ ...pull, updated_at: "2026-10-09T10:25:00Z" }], now), [], "a head five minutes old"); +}); + +test("a pull request or issue number that is not one is refused before the forge is asked", async () => { + const { GiteaClient } = await import("../client.ts"); + let asked = 0; + const server = createServer((_req, res) => { + asked++; + res.end("{}"); + }); + await new Promise((r) => server.listen(0, r)); + const client = new GiteaClient(`http://127.0.0.1:${(server.address() as any).port}`, "t"); + try { + await assert.rejects(client.getPullRequest("novox", "hq", Number(undefined)), /whole number/); + await assert.rejects(client.addComment("novox", "hq", 0, "x"), /whole number/); + await assert.rejects(client.listPullFiles("novox", "hq", 1.5), /whole number/); + assert.equal(asked, 0, "the forge was asked about pulls/NaN"); + } finally { + server.close(); + } +});