From 85352c8d6f8e765b13332568e41c94327fad2ed1 Mon Sep 17 00:00:00 2001 From: jochen Date: Mon, 5 Oct 2026 17:39:01 +0200 Subject: [PATCH 1/3] gitea: announce a merge once, from the poll The merge tool announced pull.merged and so did the poll added for issue 131, so every merge made through the tool reached the controller twice. The poll sees every path and carries the clone url; it is now the only emitter. hq issue 250. --- modules/gitea/index.ts | 10 +++++----- modules/gitea/tools/index.ts | 36 +++++++++--------------------------- 2 files changed, 14 insertions(+), 32 deletions(-) diff --git a/modules/gitea/index.ts b/modules/gitea/index.ts index 947086f..fcb437d 100644 --- a/modules/gitea/index.ts +++ b/modules/gitea/index.ts @@ -3,12 +3,12 @@ // // Emits (novox/hq ADR 0041/0042): // module.gitea.repo.created — a repository appeared, however it was made (push, web UI, or tool) +// module.gitea.pull.merged — a pull request was merged, however it was merged (web UI, API, or tool) // -// issue.opened and pull.merged are emitted from the tools (tools/index.ts), at the instant the mesh -// takes that action — the natural point, and one process only. repo.created belongs here instead: -// a repository is usually born from a `git push` or the web UI, which no tool sees, so polling the -// repo list is the only way to catch every path — and keeping it out of the create-repo tool means -// the fact is never announced twice from two processes. +// issue.opened is emitted from its tool (tools/index.ts). repo.created and pull.merged belong here: a +// repository or a merge is as often made by the web UI or a plain API call, which no tool sees, so +// polling is the only way to catch every path — and the only emitter, so a fact is never announced +// twice. The merge tool announced too until novox/hq issue 250, and every merge it made was heard twice. // // The polling is deliberately unhurried: an event a minute late is still an event, whereas hammering // the forge for an immediacy nobody asked for is not. diff --git a/modules/gitea/tools/index.ts b/modules/gitea/tools/index.ts index 51932f8..5c4cabe 100644 --- a/modules/gitea/tools/index.ts +++ b/modules/gitea/tools/index.ts @@ -1,12 +1,12 @@ // gitea's tools — moved here from the shared sdk (novox/hq ADR 0039), importing gitea's own client. // They return structured data; the mesh serves them through the sdk's tool harness. // -// Two tools emit an event at the natural point of the action they take (novox/hq ADR 0041/0042): -// create-issue emits issue.opened, merge-pull-request emits pull.merged — the mesh's own hand on -// the forge, announced the instant it moves. repo.created is deliberately NOT emitted here: repos -// are far more often born from a `git push` or the web UI than from this tool, so the events -// entrypoint (index.ts) owns that one by polling, which catches every path without this tool and -// the poll double-announcing the same repo from two processes. +// One tool emits an event at the natural point of the action it takes (novox/hq ADR 0041/0042): +// create-issue emits issue.opened. pull.merged and repo.created are deliberately NOT emitted here: a +// merge or a repository is as often made in the web UI or by a plain API call as by these tools, so the +// events entrypoint (index.ts) owns both by polling, which catches every path. Announcing a merge here +// as well announced every merge made through this tool twice — the tool's at once, the poll's moments +// later (novox/hq issue 250). import { registerModuleTools, type ToolDefinition } from "@novox/mesh-sdk/tools"; import { emit } from "@novox/mesh-sdk/events"; @@ -228,29 +228,11 @@ export function getGiteaTools(gitea: GiteaClient): ToolDefinition[] { const number = Number(args.number); const method = args.method ? String(args.method) : "merge"; const deleteBranch = args.delete_branch === undefined ? true : Boolean(args.delete_branch); - // Read the PR first, so the merged event carries a title and branches, not just a number. - const pull = await gitea.getPullRequest(owner, repo, number); await gitea.mergePullRequest(owner, repo, number, method, deleteBranch); - // Read it again: the merge commit only exists now, and it is what a build is made from. + // 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. const merged = await gitea.getPullRequest(owner, repo, number); - // And what it changed, so the mesh rebuilds the modules whose own files moved rather than - // every module built from the repository (novox/hq 04-ISSUES/131). - const changed = await gitea.listPullFiles(owner, repo, number); - await emit("pull.merged", { - owner, - repo, - number, - title: pull.title, - head: pull.head, - base: pull.base, - merge_commit_sha: merged.merge_commit_sha, - merged_at: merged.merged_at, - method, - html_url: pull.html_url, - paths: changed.paths, - paths_truncated: changed.truncated, - }); - return { merged: true, number, method, deleted_branch: deleteBranch }; + return { merged: true, number, method, deleted_branch: deleteBranch, merge_commit_sha: merged.merge_commit_sha }; }, }, From 63e53f4622c2cedb95ce55650a60ba81bf4bffe2 Mon Sep 17 00:00:00 2001 From: jochen Date: Mon, 5 Oct 2026 17:46:54 +0200 Subject: [PATCH 2/3] gitea: read every page of a pull request's changed files The forge caps a page at fifty when asked for a hundred, so a large merge's file list was cut short and reported whole: a module whose own files moved was not rebuilt. Read until a page comes back short. hq issue 252. --- modules/gitea/client.ts | 22 ++++++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-) diff --git a/modules/gitea/client.ts b/modules/gitea/client.ts index c43e6e8..0aafb42 100644 --- a/modules/gitea/client.ts +++ b/modules/gitea/client.ts @@ -249,10 +249,24 @@ export class GiteaClient { * `limit` is what is asked for, and a merge that changed more says so rather than being read * page by page: what the mesh does with a partial list is treat the whole repository as changed, * so more pages would buy nothing. */ - async listPullFiles(owner: string, repo: string, index: number, limit = 100): Promise<{ paths: string[]; truncated: boolean }> { - const files = await this.request(`/repos/${owner}/${repo}/pulls/${index}/files?limit=${limit}`); - const paths = (files ?? []).map((f) => String(f?.filename ?? "")).filter((p) => p !== ""); - return { paths, truncated: paths.length >= limit }; + /** Every file a pull request changed, page by page. **The forge caps a page below what is asked** + * (fifty, asked for a hundred), so one page read as the whole list dropped files silently, and a module + * whose own files moved was not rebuilt (novox/hq issue 252). Read until a page comes back short; past + * `most` files the list is cut and says so, and the mesh then rebuilds everything built from the + * repository, the safe direction. */ + async listPullFiles(owner: string, repo: string, index: number, most = 3000): Promise<{ paths: string[]; truncated: boolean }> { + const paths: string[] = []; + let pageSize = 0; + for (let page = 1; ; page++) { + const files = (await this.request(`/repos/${owner}/${repo}/pulls/${index}/files?limit=50&page=${page}`)) ?? []; + if (page === 1) pageSize = files.length; + for (const f of files) { + const name = String(f?.filename ?? ""); + if (name !== "") paths.push(name); + } + if (files.length === 0 || files.length < pageSize) return { paths, truncated: false }; + if (paths.length >= most) return { paths, truncated: true }; + } } async createPullRequest( From 45507c3d5c65e1a52ac7db83485c321e66c8bb25 Mon Sep 17 00:00:00 2001 From: jochen Date: Mon, 5 Oct 2026 17:47:36 +0200 Subject: [PATCH 3/3] gitea: test that a pull request's files are read past the forge's page cap --- modules/gitea/test/pages.test.ts | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) create mode 100644 modules/gitea/test/pages.test.ts diff --git a/modules/gitea/test/pages.test.ts b/modules/gitea/test/pages.test.ts new file mode 100644 index 0000000..6e6d964 --- /dev/null +++ b/modules/gitea/test/pages.test.ts @@ -0,0 +1,24 @@ +import assert from "node:assert/strict"; +import { test } from "node:test"; +import { createServer } from "node:http"; +import { GiteaClient } from "../client.ts"; + +test("every page of a pull request's files is read, though the forge caps a page at fifty", async () => { + const total = 59; + const server = createServer((req, res) => { + const url = new URL(req.url ?? "", "http://x"); + const page = Number(url.searchParams.get("page") ?? "1"); + const start = (page - 1) * 50; + const files = Array.from({ length: Math.max(0, Math.min(50, total - start)) }, (_, i) => ({ filename: `modules/m${start + i}/x` })); + res.setHeader("content-type", "application/json"); + res.end(JSON.stringify(files)); + }); + await new Promise((r) => server.listen(0, r)); + const port = (server.address() as any).port; + const client = new GiteaClient(`http://127.0.0.1:${port}`, "t"); + const got = await client.listPullFiles("novox", "mesh-catalog", 60); + server.close(); + assert.equal(got.paths.length, total); + assert.equal(got.truncated, false); + assert.equal(new Set(got.paths).size, total); +});