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( 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/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); +}); 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 }; }, },