From 961e57f7e40320a6792044aebb14bd7cdbe514f4 Mon Sep 17 00:00:00 2001 From: jochen Date: Mon, 12 Oct 2026 20:52:27 +0200 Subject: [PATCH] Page the announcer through every open pull request updated since its last look A pass read only the 20 most recently updated open pull requests of a repository, so a new head beyond them went unannounced and its merge check never started (novox/hq issue 579). Pages stop at the floor; a sweep reads all. --- modules/gitea/client.ts | 28 +++++++++- modules/gitea/index.ts | 4 +- modules/gitea/test/paging.test.ts | 92 +++++++++++++++++++++++++++++++ 3 files changed, 121 insertions(+), 3 deletions(-) create mode 100644 modules/gitea/test/paging.test.ts diff --git a/modules/gitea/client.ts b/modules/gitea/client.ts index 7219b84d..323a1e6c 100644 --- a/modules/gitea/client.ts +++ b/modules/gitea/client.ts @@ -342,8 +342,8 @@ export class GiteaClient { } /** **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. */ + * date, however many are open, whatever their update time (a pass announces from listOpenPullsSince). 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++) { @@ -355,6 +355,30 @@ export class GiteaClient { return out; } + /** **The open pull requests of a repository updated since a moment**, newest first, page by page (novox/hq issue + * 579): what a pass announces new heads from. The forge sorts `recentupdate` by update time, newest first, so the + * paging stops at the first page that reaches back past `since`; with no moment — a sweep or the first pass — + * every page is read. Bounded like listOpenPulls at 20 pages of 50. A pull request seen twice, pushed up a page + * while the pages were read, is answered once. */ + async listOpenPullsSince(owner: string, repo: string, since: string): Promise { + const out: GiteaPull[] = []; + const seen = new Set(); + const floor = since ? Date.parse(since) : NaN; + for (let page = 1; page <= 20; page++) { + const batch = await this.listPullRequests(owner, repo, { state: "open", sort: "recentupdate", limit: "50", page: String(page) }); + for (const p of batch) { + if (seen.has(p.number)) continue; + seen.add(p.number); + out.push(p); + } + if (batch.length < 50) return out; + const oldest = batch[batch.length - 1]?.updated_at; + if (!Number.isNaN(floor) && oldest && Date.parse(oldest) < floor) return out; + } + console.error(`[gitea] ${owner}/${repo} has more than 1000 open pull requests updated since ${since || "ever"}; only the newest 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)}`); diff --git a/modules/gitea/index.ts b/modules/gitea/index.ts index ab0cfffe..d64045a7 100644 --- a/modules/gitea/index.ts +++ b/modules/gitea/index.ts @@ -252,7 +252,9 @@ async function pollPulls(client: GiteaClient): Promise { } 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" }); + // Every open pull request updated since the floor, paged (novox/hq issue 579): the newest 20 alone left a head + // beyond them unannounced. A sweep and the first pass read every page. + const open = await client.listOpenPullsSince(repo.owner, repo.name, sweep ? "" : floor); for (const pull of headsToAnnounce(repo.full_name, open, heads)) { const key = `${repo.full_name}#${pull.number}`; const fresh = primedPulls || (!!pull.updated_at && pull.updated_at > dayAgo); diff --git a/modules/gitea/test/paging.test.ts b/modules/gitea/test/paging.test.ts new file mode 100644 index 00000000..a9f40a32 --- /dev/null +++ b/modules/gitea/test/paging.test.ts @@ -0,0 +1,92 @@ +import assert from "node:assert/strict"; +import { test } from "node:test"; +import { createServer } from "node:http"; + +// **Every open pull request's new head is announced, however many are open** (novox/hq issue 579). A pass asked the +// forge for the 20 most recently updated open pull requests of a repository, so a new head on the 21st or later was +// not announced and its merge check never started. A pass now pages through the open pull requests updated since its +// last look — every page on a sweep or the first pass — and stops at the first page that reaches back past the floor. + +const HOUR = 3600_000; +const T0 = Date.parse("2026-10-12T12:00:00Z"); +const sha = (n: number, v: string) => `${v}${String(n).padStart(4, "0")}`.padEnd(40, "0"); + +/** A forge holding `count` open pull requests of novox/hq, #n updated n hours after T0 (#count the newest), paged and + * sorted the way gitea answers `sort=recentupdate`: by update time, newest first. */ +function forge(count: number, heads: Record) { + const asked: URLSearchParams[] = []; + const server = createServer((req, res) => { + const url = new URL(req.url ?? "", "http://x"); + if (url.pathname !== "/api/v1/repos/novox/hq/pulls") { + res.statusCode = 404; + return res.end("{}"); + } + asked.push(url.searchParams); + const limit = Number(url.searchParams.get("limit") ?? 30); + const page = Number(url.searchParams.get("page") ?? 1); + const all = Array.from({ length: count }, (_, i) => count - i).map((n) => ({ + number: n, state: "open", base: { ref: "main" }, head: { ref: `b${n}`, sha: heads[n] ?? sha(n, "a") }, + updated_at: new Date(T0 + n * HOUR).toISOString(), html_url: `h/${n}`, + })); + res.end(JSON.stringify(all.slice((page - 1) * limit, page * limit))); + }); + return { server, asked }; +} + +async function serve(count: number, heads: Record) { + const { GiteaClient } = await import("../client.ts"); + const f = forge(count, heads); + await new Promise((r) => f.server.listen(0, r)); + const client = new GiteaClient(`http://127.0.0.1:${(f.server.address() as any).port}`, "t"); + return { client, ...f }; +} + +test("a new head beyond the newest 20 open pull requests is announced, exactly once", async () => { + const { headsToAnnounce } = await import("../pulls.ts"); + // 25 open, every head announced before; #3 — the 23rd most recently updated — got a new head. + const announced: Record = {}; + for (let n = 1; n <= 25; n++) announced[`novox/hq#${n}`] = sha(n, "a"); + const { client, server, asked } = await serve(25, { 3: sha(3, "b") }); + try { + // The fault: the newest 20 alone do not hold it. + const newest = await client.listPullRequests("novox", "hq", { state: "open", sort: "recentupdate", limit: "20" }); + assert.deepEqual(headsToAnnounce("novox/hq", newest, announced), [], "the newest 20 miss #3's new head"); + asked.length = 0; + const said: string[] = []; + for (let pass = 0; pass < 2; pass++) { + // The sweep, as index.ts's pollPulls asks: no floor, every page. + const open = await client.listOpenPullsSince("novox", "hq", ""); + for (const p of headsToAnnounce("novox/hq", open, announced)) { + said.push(`#${p.number}@${p.head_sha}`); + announced[`novox/hq#${p.number}`] = String(p.head_sha); + } + } + assert.deepEqual(said, [`#3@${sha(3, "b")}`], "the new head past the newest 20, announced once over two passes"); + assert.ok(asked.every((q) => q.get("state") === "open" && q.get("sort") === "recentupdate"), "asked newest first"); + } finally { + server.close(); + } +}); + +test("a pass pages until it reaches back past its floor, and no further", async () => { + // 130 open pull requests, #n updated n hours after T0: page 1 holds #130–#81, page 2 #80–#31, page 3 #30–#1. + const { client, server, asked } = await serve(130, {}); + try { + // A floor between #60 and #61: page 2 reaches back past it, so page 3 is never asked. + const floor = new Date(T0 + 60.5 * HOUR).toISOString(); + const open = await client.listOpenPullsSince("novox", "hq", floor); + assert.deepEqual(asked.map((q) => q.get("page")), ["1", "2"]); + assert.equal(open.length, 100); + assert.ok(open.some((p) => p.number === 61), "every one updated since the floor is read"); + // A floor no page reaches past: the last short page ends it. + asked.length = 0; + assert.equal((await client.listOpenPullsSince("novox", "hq", "")).length, 130); + assert.deepEqual(asked.map((q) => q.get("page")), ["1", "2", "3"]); + // Every page holds fewer than 50 updated since a recent floor: one page. + asked.length = 0; + await client.listOpenPullsSince("novox", "hq", new Date(T0 + 129 * HOUR).toISOString()); + assert.deepEqual(asked.map((q) => q.get("page")), ["1"]); + } finally { + server.close(); + } +});