From 52de4c320ed4d185a3efbe7b58342fc776fc0680 Mon Sep 17 00:00:00 2001 From: jochen Date: Mon, 26 Oct 2026 16:36:50 +0100 Subject: [PATCH 1/2] gitea: the merging account writes every repository that builds a registered module, read from the indexer's modules (issue 1945) car-hunter #2 was approved and its commit status refused 403: mesh-tools only read the repositories of a user. It is now a write collaborator on each repository the indexer's modules state names (watched over the bus: at the start and on each registration), read back and said as gitea.repo.writable; the operator's other repositories stay read-only. gitea_grant_write and gitea_unwritable_repos do and show it by hand; gitea_commit_statuses answers merge.writes. --- modules/gitea/README.md | 18 ++ modules/gitea/client.ts | 17 +- modules/gitea/index.ts | 22 +- modules/gitea/module.json | 4 +- modules/gitea/test/tools-account.test.ts | 69 ++++++- modules/gitea/tools/index.ts | 14 +- modules/gitea/writes.ts | 246 +++++++++++++++++++++++ 7 files changed, 382 insertions(+), 8 deletions(-) create mode 100644 modules/gitea/writes.ts diff --git a/modules/gitea/README.md b/modules/gitea/README.md index 09c8fada..1bfe7723 100644 --- a/modules/gitea/README.md +++ b/modules/gitea/README.md @@ -304,6 +304,24 @@ named and asked nothing; the start's pass is run again ten minutes apart until e grant by hand, now, on every repository or on those named, for every account or those named; each grant read back from the forge, a refusal answered in the forge's own words). +### The merging account writes the repositories the mesh delivers + +`mesh-tools` sets every commit status and merges; outside the organisation it only reads (above), so a pull request of +a user's repository could be checked and asked but never merged (issue 1945: car-hunter #2's status refused 403 after +the operator's Approve). `writes.ts` makes it a collaborator that **writes** on each repository that builds a registered +module, and on no other (the operator, 2026-10-26): the build modules' and the agents' accounts keep read. + +- **Which:** the indexer's state `mesh-delivery-indexer.modules` (in this module's `reads`), one key per registered + module, each naming its repository. A repository of `novox` needs nothing: the team writes it. Every other repository + of a user stays read-only. +- **When:** the state's watch hands every module at the start and each one registered after; a repository not yet + written is granted then, read back from the forge, and said on the bus as `gitea.repo.writable` {owner, repo, + account}, on which mesh-delivery-gate reads that repository again. Nothing polls. +- **Tools:** `gitea_unwritable_repos` (every delivered repository the account cannot write, its permission and why) + and `gitea_grant_write` (the same grant by hand, on every delivered repository or those named; what it granted, what + was written already, what the forge refused in its own words, and the repositories left read-only and why). + `gitea_commit_statuses` answers `merge.writes`, whether the account may write the repository at all. + ## Files it does not know Each directory or archive this module declares says how it treats a file it did not write (ADR 0422, ADR 0416). diff --git a/modules/gitea/client.ts b/modules/gitea/client.ts index ed94c29c..a36e113f 100644 --- a/modules/gitea/client.ts +++ b/modules/gitea/client.ts @@ -50,6 +50,9 @@ export interface GiteaRepo { default_branch?: string; /** When anything last moved in it — a push, and so a merge. */ updated_at?: string; + /** Whether the account asking may write it (the forge's permissions.push or admin): a commit status and a merge need + * it. Absent when the forge did not say. */ + writes?: boolean; } /** An issue, with its labels flattened to names. */ @@ -831,6 +834,8 @@ export class GiteaClient { html_url: r.html_url, default_branch: r.default_branch, updated_at: r.updated_at ?? undefined, + ...(r.permissions && typeof r.permissions === "object" + ? { writes: r.permissions.push === true || r.permissions.admin === true } : {}), }; } @@ -1053,8 +1058,18 @@ export class GiteaAdmin { /** Make an account a collaborator of a repository that reads only. The forge asks a repository's administrator for * it, which on a repository of another user only the site administrator is. Idempotent: the forge answers 204 again. */ async addReadCollaborator(owner: string, repo: string, user: string): Promise { + await this.addCollaborator(owner, repo, user, "read"); + } + + /** Make an account a collaborator of a repository that writes: pushes a branch, sets a commit status, merges (writes.ts: + * only on a repository that builds a registered module). An existing read collaborator is changed to write. */ + async addWriteCollaborator(owner: string, repo: string, user: string): Promise { + await this.addCollaborator(owner, repo, user, "write"); + } + + private async addCollaborator(owner: string, repo: string, user: string, permission: "read" | "write"): Promise { const path = `${repoPath(owner, repo)}/collaborators/${encodeURIComponent(user)}`; - const res = await this.request(path, { method: "PUT", body: JSON.stringify({ permission: "read" }) }); + const res = await this.request(path, { method: "PUT", body: JSON.stringify({ permission }) }); if (res.status === 204 || res.status === 200) return; GiteaAdmin.fail(path, res); } diff --git a/modules/gitea/index.ts b/modules/gitea/index.ts index 70577358..21e62e13 100644 --- a/modules/gitea/index.ts +++ b/modules/gitea/index.ts @@ -58,10 +58,11 @@ import { CHECK_CONTEXT, CHECK_STATUS, REPO_CHECK_CONTEXT, commentFor, composedGa import { GateWriter } from "./gate.js"; import { HOOK_PATH, HOOK_PORT, Pace, claimHead, ensureSystemHook, eventsTool, keptSecret, receiver, type ForgeEvent } from "./webhook.js"; import { keepSiblings, noteComposed, noteVerdict, readSiblings, siblingsFile, siblingsOf, weighSiblings } from "./siblings.js"; -import { bindInBackground } from "./bind.js"; +import { bindInBackground, bindUntilHeld } from "./bind.js"; import { announcementOf, keepTook, lateVerdictComment, outOfDateWords, sameCommit, tookFile, weighTrunks } from "./trunk.js"; import { ReadAll, readAllTools } from "./readall.js"; -import { TOOLS_ACCOUNT, meshAccounts } from "./tools-account.js"; +import { DeliveredWrites, INDEXER_MODULES, writeTools, type ModulesChange } from "./writes.js"; +import { TOOLS_ACCOUNT, TOOLS_ORG, meshAccounts } from "./tools-account.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 @@ -509,8 +510,14 @@ await emit("states", statesAnnouncement(process.env.MESH_MODULE || "gitea", proc let readAllOnce: ReadAll | null = null; const readAll = (): ReadAll => (readAllOnce ??= new ReadAll(GiteaAdmin.fromEnv(), meshAccounts(process.env.MESH_GITEA_TOOLS_USER || TOOLS_ACCOUNT))); +// **The merging account writes every repository that builds a registered module** (writes.ts; the operator, +// 2026-10-26; issue 1945): which those are is the indexer's state, watched below. Made on first use, as readAll. +let writesOnce: DeliveredWrites | null = null; +const writes = (): DeliveredWrites => (writesOnce ??= new DeliveredWrites(GiteaAdmin.fromEnv(), + process.env.MESH_GITEA_TOOLS_USER || TOOLS_ACCOUNT, TOOLS_ORG, (l) => console.log(`[gitea] ${l}`), (type, body) => emit(type, body))); + // gitea_events is served whether or not the forge is watched: it says so (novox/hq issue 729). -registerModuleTools("gitea", () => [eventsTool(pace), ...readAllTools(readAll), ...(gitea ? [commitStatusTool({ +registerModuleTools("gitea", () => [eventsTool(pace), ...readAllTools(readAll), ...writeTools(writes), ...(gitea ? [commitStatusTool({ setCommitStatus: (owner, repo, sha, status) => (gitea as GiteaClient).setCommitStatus(owner, repo, sha, status), gates, }), mergeTool(gitea, { gates })] : [])]); @@ -625,6 +632,15 @@ if (gitea) { } catch (err) { console.error(`[gitea] the mesh's accounts are not given read on every repository: no site administrator's login — ${err instanceof Error ? err.message : err}`); } + // The indexer's modules, watched: the watch hands every key's latest value at its start and then each change, so a + // repository that builds a registered module is made writable at the start and when its first module is registered + // (writes.ts). A read not granted yet is said and asked again until it holds. + try { + const w = writes(); + void bindUntilHeld(INDEXER_MODULES, () => state(INDEXER_MODULES).watch((change) => w.take(change as ModulesChange))); + } catch (err) { + console.error(`[gitea] the merging account is not given write on the delivered repositories: no site administrator's login — ${err instanceof Error ? err.message : err}`); + } // Main takes no push and only the tools' account merges, in every repository, kept and not only when a token is // minted: at the start and every ten minutes after, so a repository made later gets it too (issue 948, ADR 0398 §2). // Only while the setting keep-main-rules is true (mainrules.ts); false, the default, leaves every repository's rule as set. diff --git a/modules/gitea/module.json b/modules/gitea/module.json index 5eb7d147..954a796b 100644 --- a/modules/gitea/module.json +++ b/modules/gitea/module.json @@ -40,6 +40,7 @@ ], "emits": [ "repo.created", + "repo.writable", "issue.opened", "pull.merged", "pull.updated", @@ -567,6 +568,7 @@ "mesh-build-go.keys", "mesh-build-node.keys", "mesh-build-image.keys", - "mesh-delivery-checker.owed" + "mesh-delivery-checker.owed", + "mesh-delivery-indexer.modules" ] } diff --git a/modules/gitea/test/tools-account.test.ts b/modules/gitea/test/tools-account.test.ts index 5d275189..80519429 100644 --- a/modules/gitea/test/tools-account.test.ts +++ b/modules/gitea/test/tools-account.test.ts @@ -206,7 +206,10 @@ function fakeForge() { if (m) { const repo = forgeRepos.find((r) => r.owner === decodeURIComponent(m![1]) && r.name === decodeURIComponent(m![2])); if (!repo || permissionOf(me, repo) === "none") return json(res, 404, { message: "repository does not exist" }); - if (req.method === "GET" && !m[3]) return json(res, 200, repoJSON(repo)); + if (req.method === "GET" && !m[3]) { + const mine = permissionOf(me, repo); + return json(res, 200, { ...repoJSON(repo), permissions: { pull: true, push: ["write", "admin", "owner"].includes(mine), admin: ["admin", "owner"].includes(mine) } }); + } const user = decodeURIComponent(m[3] ?? ""); if (req.method === "GET" && m[4]) { if (repoFaults.permission.has(repo.full)) return json(res, 500, { message: "the forge is not well" }); @@ -576,3 +579,67 @@ test("on the forge's events: a repository made is granted read at once, a known repoFaults.refuse.clear(); repoFaults.permission.clear(); }); + +// The merging account writes every repository that builds a registered module, and no other (writes.ts; the operator, +// 2026-10-26; issue 1945): car-hunter #2 was approved and its commit status refused 403, mesh-tools reading only there. + +test("the merging account is given write on each repository the indexer's modules name, read back and said; the operator's other repositories stay read-only; the tools say both", async () => { + const env = await environment(); + const client = fresh(env); + await client.listRepos(); // the account and its team exist, as after any mint + forgeRepos.splice(0); + addRepo("novox/hq", true); + for (const full of ["jschoubben/car-hunter", "jschoubben/snake", "jschoubben/dotfiles"]) addRepo(full, true).collaborators.set(TOOLS_ACCOUNT, "read"); + const { GiteaAdmin } = await import("../dist/client.js"); + const { DeliveredWrites, writeTools, WRITABLE_EVENT } = await import("../dist/writes.js"); + const lines: string[] = []; + const said: { type: string; body: any }[] = []; + const writes = new DeliveredWrites(GiteaAdmin.fromEnv(env), TOOLS_ACCOUNT, "novox", (l: string) => lines.push(l), + async (type: string, body: unknown) => { said.push({ type, body }); }); + const [unwritableTool, grantTool] = writeTools(() => writes); + await assert.rejects(grantTool.run({}), /not known yet/, "nothing is granted before the indexer's modules were read"); + assert.equal((await client.getRepo("jschoubben", "car-hunter")).writes, false, "the statuses' answer says the account cannot write"); + + // 1945/AC1: the watch hands the indexer's modules; a user's repository a module builds from is made writable, once. + await writes.take({ key: "car-hunter", op: "put", value: { v: 1, repository: "jschoubben/car-hunter", commit: "c" }, current: true }); + await writes.take({ key: "gitea", op: "put", value: { v: 1, repository: "novox/hq", commit: "c" }, current: true }); + await writes.take({ key: "broken", op: "put", value: { v: 1, repository: "no-slash" }, current: true }); + const from = forge.calls.length; + await writes.take({ key: "car-hunter-2", op: "put", value: { v: 1, repository: "jschoubben/car-hunter" } }); + assert.equal(forge.calls.length, from, "a repository already written asks nothing more"); + const repo = (full: string) => forgeRepos.find((x) => x.full === full)!; + assert.equal(repo("jschoubben/car-hunter").collaborators.get(TOOLS_ACCOUNT), "write"); + assert.equal(repo("novox/hq").collaborators.size, 0, "the organisation's repository is written through its team, no collaborator"); + assert.equal((await client.getRepo("jschoubben", "car-hunter")).writes, true); + assert.deepEqual(said, [{ type: WRITABLE_EVENT, body: { owner: "jschoubben", repo: "car-hunter", account: TOOLS_ACCOUNT } }]); + assert.match(lines.join("\n"), /mesh-tools was given write on jschoubben\/car-hunter, which builds the registered module car-hunter/); + assert.match(lines.join("\n"), /broken names no repository/); + + // 1945/AC2: a delivered repository the forge refuses is said, in its words, by the listing and the grant; the operator's + // repository no module builds from is left read-only, named with why, and a grant naming it asks the forge nothing. + repoFaults.refuse.add("jschoubben/snake"); + await writes.take({ key: "snake", op: "put", value: { v: 1, repository: "jschoubben/snake" } }); + assert.match(lines.join("\n"), /could not be given write on jschoubben\/snake, which builds snake — .*422/); + const listed = await unwritableTool.run({}) as any; + assert.deepEqual(listed.unwritable.map((u: any) => [u.repository, u.permission]), [["jschoubben/snake", "read"]]); + assert.match(listed.unwritable[0].why, /builds snake.*gitea_grant_write/); + const all = await grantTool.run({}) as any; + assert.deepEqual(all.delivered, ["jschoubben/car-hunter", "jschoubben/snake", "novox/hq"]); + assert.deepEqual(all.already, ["jschoubben/car-hunter", "novox/hq"]); + assert.deepEqual(all.granted, []); + assert.match(all.refused.find((x: any) => x.repository === "jschoubben/snake").why, /422.*would not have it/); + assert.deepEqual(all.left_read_only.map((x: any) => x.repository), ["jschoubben/dotfiles"]); + assert.match(all.left_read_only[0].why, /builds no registered module/); + const puts = forge.calls.length; + const named = await grantTool.run({ repositories: ["jschoubben/dotfiles"] }) as any; + assert.deepEqual([named.granted, named.left_read_only.map((x: any) => x.repository)], [[], ["jschoubben/dotfiles"]]); + assert.ok(!forge.calls.slice(puts).some((c) => c.startsWith("PUT ")), "no write is asked on a repository no module builds from"); + assert.equal(repo("jschoubben/dotfiles").collaborators.get(TOOLS_ACCOUNT), "read"); + repoFaults.refuse.clear(); + const after = await grantTool.run({ repositories: ["jschoubben/snake"] }) as any; + assert.deepEqual(after.granted, ["jschoubben/snake"]); + assert.deepEqual(((await unwritableTool.run({})) as any).unwritable, []); + // A module gone from the indexer leaves its repository out of the delivered list. + await writes.take({ key: "snake", op: "delete" }); + assert.deepEqual(writes.delivered(), ["jschoubben/car-hunter", "novox/hq"]); +}); diff --git a/modules/gitea/tools/index.ts b/modules/gitea/tools/index.ts index fe33da7e..8f21cdb1 100644 --- a/modules/gitea/tools/index.ts +++ b/modules/gitea/tools/index.ts @@ -492,7 +492,7 @@ function allGiteaTools(gitea: GiteaClient, admin?: () => GiteaAdmin): ToolDefini }, { name: "gitea_commit_statuses", - description: "Read a commit's statuses — or a pull request's head's — as the forge keeps them: the newest of each context (mesh/check, mesh/delivery, …) with its state, description, link, when it was set and by whom; and whether the branch it merges into would let it merge, every status that branch's protection requires being a success; for a pull request, also the forge's own mergeable (false while it is in conflict with its base or the forge is still working that out). Reads only.", + description: "Read a commit's statuses — or a pull request's head's — as the forge keeps them: the newest of each context (mesh/check, mesh/delivery, …) with its state, description, link, when it was set and by whom; and whether the branch it merges into would let it merge, every status that branch's protection requires being a success; for a pull request, also the forge's own mergeable (false while it is in conflict with its base or the forge is still working that out); and merge.writes, whether the mesh's merging account may write the repository at all (false: its statuses and its merge are refused until gitea_grant_write). Reads only.", input: { owner: { type: "string", description: "the repository owner" }, repo: { type: "string", description: "the repository name" }, @@ -528,8 +528,18 @@ function allGiteaTools(gitea: GiteaClient, admin?: () => GiteaAdmin): ToolDefini } catch (err) { unreadable = err instanceof Error ? err.message : String(err); } + // Whether this account may write the repository at all (issue 1945): a commit status and a merge need it, and a + // repository of a user it only reads refuses both 403. Not said when the forge would not answer. + let writes: boolean | undefined; + try { + writes = (await gitea.getRepo(owner, repo)).writes; + } catch { + writes = undefined; + } + const merge = mergeableUnder(base, rule, statuses, unreadable); return { commit: combined.sha || ref, pull, combined: combined.state, statuses, - merge: mergeableUnder(base, rule, statuses, unreadable) }; + merge: writes === undefined ? merge : { ...merge, writes, + ...(writes ? {} : { cannot_write: `the forge account cannot write ${owner}/${repo}: gitea_grant_write` }) } }; }, }, { diff --git a/modules/gitea/writes.ts b/modules/gitea/writes.ts new file mode 100644 index 00000000..ae9d41a6 --- /dev/null +++ b/modules/gitea/writes.ts @@ -0,0 +1,246 @@ +// The account that sets commit statuses and merges writes every repository that builds a registered module, and no other +// (the operator, 2026-10-26: "yes the mesh should get write access", on the repositories the mesh delivers; novox/hq issue +// 1945). car-hunter #2 (a repository of a user, not of the organisation novox) was approved, and mesh-delivery's first +// act, its commit status, was refused 403: the tools' account was only a collaborator that reads there (readall.ts). +// +// **Which account.** The tools' account, `mesh-tools`: the gitea module's own tools set every commit status and merge +// (gitea_commit_status, gitea_merge_pull_request) with its token. The build modules' `nox-builder` and the machines' +// `nox-agent` only fetch, and keep read (readall.ts). +// +// **Which repositories: the indexer's word, read over the bus.** mesh-delivery-indexer keeps every registered module in its +// state `modules` (mesh-sdk go/delivery ModulesBucket), each value naming the repository it builds from. This module reads +// that state (its manifest's `reads`) with a watch: the watch hands the latest value of every key when it starts, and then +// every change, so a repository is granted at the module's start and when a module of it is first registered. Nothing +// polls. A repository of the organisation needs nothing: the organisation's team writes it (tools-account.ts). Every other +// repository of a user (the operator's dotfiles, say) is left as it is: read only. +// +// **Every grant is read back** from the forge (its permission for the account), and said on the bus as +// `repo.writable` {owner, repo, account}: mesh-delivery-gate, which does not ask the operator's Approve of a pull request +// whose merge the forge would refuse, reads that repository again on it. +// +// Not taken away: a repository whose last module goes from the indexer keeps its collaborator until a hand removes it; +// gitea_unwritable_repos and gitea_grant_write say the delivered list as it is now. + +import type { ForgeRepository } from "./client.js"; + +/** The indexer's state this module reads: one key per registered module, its value naming the repository. */ +export const INDEXER_MODULES = "mesh-delivery-indexer.modules"; +/** The event said once an account writes a repository it could not write before. */ +export const WRITABLE_EVENT = "repo.writable"; + +/** What the module asks of the forge for this, with the site administrator's login (GiteaAdmin, client.ts). */ +export interface WriteForge { + everyRepository(): Promise; + repository(owner: string, repo: string): Promise; + repositoryPermission(owner: string, repo: string, user: string): Promise; + addWriteCollaborator(owner: string, repo: string, user: string): Promise; +} + +/** One change of the indexer's modules state, as the SDK's state watch hands it. */ +export interface ModulesChange { + key: string; + op: "put" | "delete"; + value?: unknown; + revision?: number; + current?: boolean; +} + +export interface Written { + account: string; + /** The repositories that build a registered module, as the indexer says now. */ + delivered: string[]; + /** Made writable now, each read back from the forge. */ + granted: string[]; + /** Written already (the organisation's team, or a collaborator that writes). */ + already: string[]; + /** The forge refused, in its own words; or the repository is the organisation's, whose team should write it. */ + refused: { repository: string; why: string }[]; + /** Repositories the account cannot write that are left so, and why: they build no registered module. */ + left_read_only: { repository: string; permission: string; why: string }[]; +} + +const WRITES = new Set(["write", "admin", "owner"]); +const said = (err: unknown): string => (err instanceof Error ? err.message : String(err)); + +function split(full: string): [string, string] { + const [owner, name, ...rest] = String(full ?? "").split("/"); + if (!owner || !name || rest.length) throw new Error(`"${full}" is not a repository: say owner/name`); + return [owner, name]; +} + +export class DeliveredWrites { + /** Module to the repository it builds from, as the indexer's state says. */ + readonly modules = new Map(); + /** Whether the watch has handed the indexer's state at all: until then, which repositories are delivered is not known. */ + heard = false; + /** Repositories seen written by the account in this process: a module registered again asks nothing more. */ + readonly writable = new Set(); + + constructor(private readonly forge: WriteForge, readonly account: string, readonly org: string, + private readonly log: (line: string) => void = (l) => console.log(`[gitea] ${l}`), + private readonly emit: (type: string, body: unknown) => Promise = async () => {}) {} + + /** The repositories that build a registered module, sorted. */ + delivered(): string[] { + return [...new Set(this.modules.values())].sort(); + } + + /** One change of the indexer's modules: a repository newly delivered is made writable now. Never throws. */ + async take(change: ModulesChange): Promise { + this.heard = true; + if (change.op === "delete") { + this.modules.delete(change.key); + return; + } + const repository = String((change.value as { repository?: unknown } | undefined)?.repository ?? "").trim(); + try { + split(repository); + } catch { + this.log(`${INDEXER_MODULES} ${change.key} names no repository (${JSON.stringify(repository)}): no write is granted for it`); + return; + } + this.modules.set(change.key, repository); + if (this.writable.has(repository)) return; + try { + const r = await this.grant([repository]); + for (const g of r.granted) this.log(`${this.account} was given write on ${g}, which builds the registered module ${change.key}`); + for (const x of r.refused) this.log(`${this.account} could not be given write on ${x.repository}, which builds ${change.key} — ${x.why}`); + } catch (err) { + this.log(`whether ${this.account} writes ${repository} (module ${change.key}) could not be settled — ${said(err)}`); + } + } + + /** Grant write on every delivered repository the account cannot write (or on those named, each of which must be + * delivered); with none named, also say which repositories it cannot write are left so, and why. */ + async grant(only?: string[]): Promise { + if (!this.heard) { + throw new Error(`which repositories build a registered module is not known yet: ${INDEXER_MODULES} has not been read ` + + "(the watch is not granted or not answered); nothing is granted"); + } + const delivered = this.delivered(); + const result: Written = { account: this.account, delivered, granted: [], already: [], refused: [], left_read_only: [] }; + const named = only && only.length ? only.map((x) => String(x).trim()) : null; + const targets: string[] = []; + for (const full of named ?? delivered) { + split(full); + if (delivered.includes(full)) targets.push(full); + else result.left_read_only.push({ repository: full, permission: "unchanged", + why: "builds no registered module (the indexer names none from it), so the account is given no write there" }); + } + for (const full of targets) { + const [owner, name] = split(full); + let permission: string; + try { + if (!(await this.forge.repository(owner, name))) { + result.refused.push({ repository: full, why: "the forge holds no repository by that name" }); + continue; + } + permission = await this.forge.repositoryPermission(owner, name, this.account); + } catch (err) { + result.refused.push({ repository: full, why: `its permission could not be read — ${said(err)}` }); + continue; + } + if (WRITES.has(permission)) { + this.writable.add(full); + result.already.push(full); + continue; + } + if (owner === this.org) { + // The organisation's repositories are written through its team (tools-account.ts), never a collaborator. + result.refused.push({ repository: full, why: `a repository of ${this.org}, which ${this.account} writes through the ` + + `team ${this.org}/${this.account}; the forge gives it "${permission}", so the team is not as it should be (it is ` + + "set again at the next mint of the tools' token)" }); + continue; + } + try { + await this.forge.addWriteCollaborator(owner, name, this.account); + } catch (err) { + result.refused.push({ repository: full, why: said(err) }); + continue; + } + let after: string; + try { + after = await this.forge.repositoryPermission(owner, name, this.account); + } catch (err) { + result.refused.push({ repository: full, why: `granted, but its permission could not be read back — ${said(err)}` }); + continue; + } + if (!WRITES.has(after)) { + result.refused.push({ repository: full, why: `the grant was answered, but the forge still gives ${this.account} "${after}"` }); + continue; + } + this.writable.add(full); + result.granted.push(full); + await this.emit(WRITABLE_EVENT, { owner, repo: name, account: this.account }).catch((err) => + this.log(`${this.account} writes ${full} now, and that could not be said on the bus — ${said(err)}`)); + } + if (!named) { + for (const repo of await this.forge.everyRepository()) { + if (repo.owner === this.org || delivered.includes(repo.full_name)) continue; + let permission: string; + try { + permission = await this.forge.repositoryPermission(repo.owner, repo.name, this.account); + } catch (err) { + permission = `unknown — ${said(err)}`; + } + if (WRITES.has(permission)) continue; + result.left_read_only.push({ repository: repo.full_name, permission, + why: "builds no registered module (the indexer names none from it), so the account is given no write there" }); + } + } + return result; + } + + /** Every delivered repository the account cannot write, as the forge says now, and why. */ + async unwritable(): Promise<{ account: string; delivered: number; unwritable: { repository: string; permission: string; why: string }[] }> { + if (!this.heard) throw new Error(`which repositories build a registered module is not known yet: ${INDEXER_MODULES} has not been read`); + const delivered = this.delivered(); + const out: { repository: string; permission: string; why: string }[] = []; + for (const full of delivered) { + const [owner, name] = split(full); + let permission: string; + try { + permission = await this.forge.repositoryPermission(owner, name, this.account); + } catch (err) { + out.push({ repository: full, permission: "unknown", why: `the forge did not answer ${this.account}'s permission on it — ${said(err)}` }); + continue; + } + if (WRITES.has(permission)) continue; + const modules = [...this.modules].filter(([, r]) => r === full).map(([m]) => m).sort(); + out.push({ repository: full, permission, why: `builds ${modules.join(", ")}, and the forge gives ${this.account} ` + + `"${permission}" on it: its commit status and its merge are refused until gitea_grant_write grants it` }); + } + return { account: this.account, delivered: delivered.length, unwritable: out }; + } +} + +type Tool = { name: string; description: string; input: Record; run: (args: Readonly>) => Promise }; + +/** The two tools: which delivered repositories the merging account cannot write, and the grant now. */ +export function writeTools(writes: () => DeliveredWrites): Tool[] { + return [ + { + name: "gitea_unwritable_repos", + description: "Every repository that builds a registered module (the indexer's modules, read over the bus) on which " + + "the mesh's merging forge account (mesh-tools, which sets commit statuses and merges) cannot write, with the " + + "permission the forge gives it there and why: such a pull request can be checked but never merged, and the " + + "delivery gate does not ask its Approve. Empty when it writes every one. Read with the site administrator's login.", + input: {}, + run: async () => writes().unwritable(), + }, + { + name: "gitea_grant_write", + description: "Give the mesh's merging forge account (mesh-tools) write, as a collaborator, on every repository that " + + "builds a registered module and that it cannot write now, or on the repositories named (each must build one). " + + "The module does this itself at its start and when a module of a new repository is registered; this is the same " + + "act by hand, such as when a merge or a commit status was refused 403. A repository of the organisation novox " + + "is written through its team and needs nothing. Answers what was granted (each read back from the forge), what " + + "was written already, what the forge refused in its own words, and which other repositories it left read-only " + + "and why (they build no registered module). Needs the site administrator's login, used for this grant alone.", + input: { + repositories: { type: "array", items: { type: "string" }, description: "owner/name of each repository; empty or absent, every delivered one" }, + }, + run: async (args) => writes().grant(Array.isArray(args.repositories) ? args.repositories.map(String) : undefined), + }, + ]; +} From 24b244bc88dc492e2eba09bb294b281e3421aff0 Mon Sep 17 00:00:00 2001 From: jochen Date: Mon, 26 Oct 2026 16:39:56 +0100 Subject: [PATCH 2/2] mesh-delivery-gate: ask no Approve of a merge the forge account cannot carry out, and say why (issue 1945) car-hunter #2 was approved and its commit status then refused 403. Before the ask the gate reads merge.writes for each repository it covers; where the merging account cannot write, nothing is asked, the delivery waits on 'the forge account cannot write : gitea_grant_write' and a condition is raised. gitea.repo.writable reads it again and asks an approved, refused merge of it again. --- modules/mesh-delivery-gate/README.md | 8 ++ .../cmd/mesh-delivery-gate/approval.go | 13 ++ .../cmd/mesh-delivery-gate/fakes_test.go | 8 +- .../cmd/mesh-delivery-gate/holder.go | 2 + .../cmd/mesh-delivery-gate/main.go | 6 +- .../cmd/mesh-delivery-gate/ports.go | 4 + .../cmd/mesh-delivery-gate/ready.go | 11 ++ .../cmd/mesh-delivery-gate/writes.go | 115 ++++++++++++++++++ .../cmd/mesh-delivery-gate/writes_test.go | 44 +++++++ modules/mesh-delivery-gate/module.json | 3 +- 10 files changed, 211 insertions(+), 3 deletions(-) create mode 100644 modules/mesh-delivery-gate/cmd/mesh-delivery-gate/writes.go create mode 100644 modules/mesh-delivery-gate/cmd/mesh-delivery-gate/writes_test.go diff --git a/modules/mesh-delivery-gate/README.md b/modules/mesh-delivery-gate/README.md index 66299ef3..a7ed14ad 100644 --- a/modules/mesh-delivery-gate/README.md +++ b/modules/mesh-delivery-gate/README.md @@ -36,6 +36,14 @@ names every refusal and rejection at once, and the pull request is told at once; `needs: new-head` and names its author: the branch, and the session a `Pushed-by: ` line in the pull request's description names. +## A merge the forge account cannot carry out is not asked (issue 1945) + +Before the operator's Approve is asked, the gate reads through `gitea_commit_statuses` whether the forge's merging +account writes each repository the ask covers (`merge.writes`). Where it does not, nothing is asked: the delivery waits +on "the forge account cannot write : gitea_grant_write" (`show`), raised at once as a condition. The read holds +until the gitea module's `gitea.repo.writable` (its grant, read back) or ten minutes, then the ask is made again; an +approved merge of that repository the forge refused is asked again on the same event (`writes.go`). + ## The seat's hand-over The gate took the seat `mesh-delivery` over from the module `mesh-delivery` (ADR 0449 §4, ADR 0450 §6, ADR 0454) and carried its diff --git a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/approval.go b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/approval.go index 3229d6d8..3c385094 100644 --- a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/approval.go +++ b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/approval.go @@ -339,6 +339,9 @@ func (h *Holder) approval(d *Delivery, now time.Time) (string, func()) { return "the operator cannot be asked from here", nil } members := h.askMembers(d) + if waits := h.writeWaits(d, members, now); waits != "" { + return waits, nil // asked only once the forge account can carry the merge out (writes.go, issue 1945) + } group := "" if len(members) > 1 { group = d.Group @@ -423,6 +426,16 @@ func (h *Holder) approval(d *Delivery, now time.Time) (string, func()) { h.mu.Unlock() } } + // Nothing is asked that the forge account could not merge (writes.go, issue 1945): said at the next look. + if says := h.cannotWrite(reads); says != "" { + stale() + h.mu.Lock() + defer h.mu.Unlock() + undo() + h.Logf("[mesh-delivery] %s passed its checks and the operator is not asked: %s", d.ID, says) + h.lookAt(h.Now(), "an ask not made: the forge account cannot write a repository it covers") + return + } if readErr != nil || holdSays != "" { stale() h.mu.Lock() diff --git a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/fakes_test.go b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/fakes_test.go index 6bbec874..f1d36f37 100644 --- a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/fakes_test.go +++ b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/fakes_test.go @@ -101,7 +101,9 @@ type fakeForge struct { viewCalls int // What Statuses reads: the pull requests by owner/repo#number, the base's required statuses, whether the // protection cannot be read, and what it was asked. - pulls map[string]ForgePull + pulls map[string]ForgePull + // unwritable are the repositories (owner/repo) the forge's merging account cannot write (issue 1945). + unwritable map[string]bool required []string protectionDown bool asked []string @@ -220,6 +222,10 @@ func (f *fakeForge) Statuses(owner, repo string, number int, commit, base string base = "main" } out.Merge = ForgeMerge{Branch: base, Protected: f.required != nil, Required: f.required} + if f.unwritable[owner+"/"+repo] { + no := false + out.Merge.Writes, out.Merge.CannotWrite = &no, "the forge account cannot write "+owner+"/"+repo+": gitea_grant_write" + } if f.protectionDown { out.Merge.Says = "the protection of " + base + " could not be read" return out, nil diff --git a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/holder.go b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/holder.go index 06b56023..138c7589 100644 --- a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/holder.go +++ b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/holder.go @@ -82,6 +82,8 @@ type Holder struct { // unmergeableSince is when the forge first said a group's member does not merge now, by group and member (issue 1275, // group_merge.go): within forgeMergeGrace that is the forge still working out mergeability, asked again shortly. unmergeableSince map[string]time.Time + // writeReads is whether the forge's merging account writes each repository, as last read (writes.go, issue 1945). + writeReads map[string]*writeRead // Turns is the gate's bucket turns (turns.go, ADR 0548 §3); nil keeps none. turnsHeld are the turns as written there, // by repository, read once at the first look (turnsLoaded). Turns TurnStore diff --git a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/main.go b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/main.go index 58ff5b46..be04c7f1 100644 --- a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/main.go +++ b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/main.go @@ -34,7 +34,7 @@ func (l *listening) get() string { l.mu.Lock(); defer l.mu.Unlock(); return l.n // The events this owner follows: the forge's pull requests. The forge's status events say that a check answered (events.go). var followed = []string{"gitea.pull.updated", "gitea.pull.merged", "gitea.pull.closed", "gitea.forge.status", "gitea.forge.pull-request.edited", "gitea.forge.pull-request.closed", - forgePattern, ConfigChangedEvent} + forgePattern, ConfigChangedEvent, writableEvent} // forgePattern is the forge bridge's messages on FORGE (novox/hq ADR 0437 §8): the gate follows a merge and the refusal of one. const forgePattern = "mesh-delivery-forge-bridge.forge.**" @@ -219,6 +219,10 @@ func take(h *Holder, e stdio.Envelope) error { } h.CommitStatus(s) h.KeepCheckVerdict(s) + case writableEvent: + if err := h.WriteGranted(e.Body); err != nil { + h.Refuse(err) + } case ConfigChangedEvent: var c ConfigChange if err := json.Unmarshal(e.Body, &c); err != nil { diff --git a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/ports.go b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/ports.go index a18d5cab..76b40c28 100644 --- a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/ports.go +++ b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/ports.go @@ -76,6 +76,10 @@ type ForgeMerge struct { } `json:"blocking"` Mergeable *bool `json:"mergeable"` Says string `json:"says"` + // Writes is whether the forge's merging account may write the repository at all (issue 1945): false, its statuses and + // its merge are refused, and CannotWrite says so with the tool that grants it. Nil when the forge holder did not say. + Writes *bool `json:"writes,omitempty"` + CannotWrite string `json:"cannot_write,omitempty"` } // Store keeps deliveries and groups: one key each, in the module's own state (ADR 0201). diff --git a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/ready.go b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/ready.go index cf2d5e5b..4b96bc11 100644 --- a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/ready.go +++ b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/ready.go @@ -128,6 +128,11 @@ type ReadyRun struct { HeldSince *time.Time `json:"held_since,omitempty"` HeldFor string `json:"held_for,omitempty"` HandNow string `json:"hand_now,omitempty"` + + // CannotWrite is why the operator is not asked: the forge's merging account cannot write a repository the ask covers + // (writes.go, issue 1945); CannotWriteSince since when. Said at once as a condition. + CannotWrite string `json:"cannot_write,omitempty"` + CannotWriteSince *time.Time `json:"cannot_write_since,omitempty"` } // MergeBranch is the one branch this owner merges into (novox/hq ADR 0398 §4): a merge into any other is its author's. @@ -520,6 +525,12 @@ func (h *Holder) readyLines(now time.Time) []StalledLine { out = append(out, StalledLine{ID: d.ID, Number: d.Number, State: d.State, For: now.Sub(r.Seen).Round(time.Second).String(), Bound: headHeardBound.String(), H2: "none: a later announcement of the pull request's head (the merge tool announces it again, the forge's announcer restarts, or a new push) or an edit of its description; a recheck does not fill it — the operator's", Says: fmt.Sprintf("%s#%d passed its checks at %s and waits for %s", d.Repository, d.Number, shortOf(r.Head, 12), waitsForHead)}) + case r.CannotWrite != "" && r.Approved == nil && r.CannotWriteSince != nil: + out = append(out, StalledLine{ID: d.ID, Number: d.Number, State: d.State, + For: now.Sub(*r.CannotWriteSince).Round(time.Second).String(), Bound: "0s", + H2: "none: the forge account given write on the repository (gitea_grant_write; the gitea module grants it itself once a module of it is registered), which asks the operator then", + Says: fmt.Sprintf("%s#%d passed its checks at %s and the operator is not asked for its Approve: %s", + d.Repository, d.Number, shortOf(r.Head, 12), r.CannotWrite)}) case r.Failed != "" && d.State == Rejected: // Its cure is its author's new head: listed here at once (`show`), told on the pull request at once, and the // operator's only once no new head came within the author's bound (novox/hq ADR 0505 §5). diff --git a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/writes.go b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/writes.go new file mode 100644 index 00000000..cb142cbe --- /dev/null +++ b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/writes.go @@ -0,0 +1,115 @@ +package main + +import ( + "encoding/json" + "fmt" + "strings" + "time" +) + +// **The operator is asked only for a merge the forge account can carry out** (novox/hq issue 1945). car-hunter #2, a +// repository of a user, was approved, and its first act after the Approve, the commit status, was refused 403: the mesh's +// merging account (the gitea module's tools, mesh-tools) only read that repository. Before the gate asks the operator's +// Approve it reads, through the forge holder's gitea_commit_statuses, whether that account writes each repository the +// ask covers (merge.writes), as the ask is about to be written. Where it does not, nothing is asked: the delivery waits, +// saying "the forge account cannot write : gitea_grant_write", and the gate raises it as a condition at once. +// +// A repository not written is kept so, and the ask is not composed again, until the gitea module's event +// gitea.repo.writable (its grant, read back from the forge) or, as a safety net, writeReadLasts after the read. A forge +// that does not say merge.writes (an older gitea module) is taken as writing: the merge says its own refusal, as before. + +// writeReadLasts is how long a read of whether the forge account writes a repository holds. +const writeReadLasts = 10 * time.Minute + +// writableEvent is the gitea module's event of an account given write on a repository. +const writableEvent = "gitea.repo.writable" + +// writeRead is what the forge last said of the merging account on one repository it cannot write. +type writeRead struct { + says string + at time.Time +} + +// cannotWriteSays is what a delivery waits on when the forge account cannot write its repository. +func cannotWriteSays(repository string) string { + return "the forge account cannot write " + repository + ": gitea_grant_write" +} + +// writeWaits is what an ask about to be made waits on: a repository it covers that the forge account was read, within +// writeReadLasts, not to write. Under the lock. +func (h *Holder) writeWaits(d *Delivery, members []*Delivery, now time.Time) string { + r := d.Ready + for _, m := range members { + repo := strings.ToLower(m.Repository) + w := h.writeReads[repo] + if w == nil { + continue + } + if !now.Before(w.at.Add(writeReadLasts)) { + delete(h.writeReads, repo) // read again at the ask + continue + } + if r.CannotWrite != w.says { + r.CannotWrite, r.CannotWriteSince = w.says, &now + h.keep(d) + } + h.lookAt(w.at.Add(writeReadLasts), "a repository the forge account could not write, read again") + return w.says + } + if r.CannotWrite != "" { + r.CannotWrite, r.CannotWriteSince = "", nil + h.keep(d) + } + return "" +} + +// cannotWrite reads, at the ask, whether the forge account writes each repository the ask covers, and answers what the +// first one it cannot write says; a read that fails says nothing (the merge says its own refusal, as before). Outside the +// lock; a repository not written is kept, under the lock. +func (h *Holder) cannotWrite(reads []memberRead) string { + for _, m := range reads { + fc, err := h.Forge.Statuses(m.owner, m.repo, m.number, m.head, m.base) + if err != nil || fc == nil || fc.Merge.Writes == nil || *fc.Merge.Writes { + continue + } + repo := strings.ToLower(m.owner + "/" + m.repo) + says := fc.Merge.CannotWrite + if says == "" { + says = cannotWriteSays(repo) + } + h.mu.Lock() + if h.writeReads == nil { + h.writeReads = map[string]*writeRead{} + } + h.writeReads[repo] = &writeRead{says: says, at: h.Now()} + h.mu.Unlock() + return says + } + return "" +} + +// WriteGranted takes the gitea module's gitea.repo.writable: the repository's read is dropped, so the next look reads it +// again, and an approved merge of it that the forge refused is asked again, its cause gone. +func (h *Holder) WriteGranted(body json.RawMessage) error { + var e struct { + Owner string `json:"owner"` + Repo string `json:"repo"` + } + if err := json.Unmarshal(body, &e); err != nil || e.Owner == "" || e.Repo == "" { + return fmt.Errorf("%s names no repository: %s", writableEvent, string(body)) + } + repo := strings.ToLower(e.Owner + "/" + e.Repo) + h.mu.Lock() + defer h.mu.Unlock() + delete(h.writeReads, repo) + for _, d := range h.deliveries { + if strings.ToLower(d.Repository) != repo || d.Ready == nil || d.Ready.Approved == nil || d.Ready.Merged != nil { + continue + } + d.Ready.Repeats = 0 + h.mergeAgainNow(d) + h.keep(d) + } + h.Logf("[mesh-delivery] the forge account was given write on %s: it is read again before an ask, and an approved merge of it is asked again", repo) + return nil +} diff --git a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/writes_test.go b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/writes_test.go new file mode 100644 index 00000000..28624230 --- /dev/null +++ b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/writes_test.go @@ -0,0 +1,44 @@ +package main + +import ( + "strings" + "testing" +) + +// Issue 1945: car-hunter #2 was asked, approved, and its commit status refused 403, since the forge's merging account only +// read the repository. 1945/AC3: a pull request whose checks passed, in a repository the merging account cannot write, is +// not asked; it waits saying so, loud as a condition at once. 1945/AC4: the gitea module's gitea.repo.writable asks it then. +func TestAnUnwritableRepositoryIsNotAskedAndSaysSoUntilTheWriteIsGranted(t *testing.T) { + w := announced(t) + conds := hearConditions(w) + w.forge.unwritable = map[string]bool{"novox/app": true} + hear(t, w, shimCheckedKey, judgedAt("novox/app", 7, readyHead, "pass", "t1t1t1t1t1t1")) + w.h.React() + if w.op.count() != 0 || len(w.op.open()) != 0 { + t.Fatalf("the operator was asked %d times for a merge the forge account cannot carry out", w.op.count()) + } + d := w.h.deliveries[IDOf("novox/app", readyHead)] + want := "the forge account cannot write novox/app: gitea_grant_write" + if d.Ready.Waits != want || d.Ready.CannotWrite != want { + t.Fatalf("waits %q, cannot_write %q; wanted %q", d.Ready.Waits, d.Ready.CannotWrite, want) + } + if c := conds.last(d.ID); c == nil || c.State != "raised" || !strings.Contains(c.Summary+c.Explanation+c.Headline, "cannot write novox/app") { + t.Fatalf("not said as a condition: %+v", c) + } + // Looked at again with nothing changed, the forge is not read again: the read holds until the grant or its bound. + asked := len(w.forge.asked) + w.h.React() + if len(w.forge.asked) != asked || w.op.count() != 0 { + t.Fatalf("read again %d times, asked %d", len(w.forge.asked)-asked, w.op.count()) + } + + delete(w.forge.unwritable, "novox/app") + hear(t, w, writableEvent, map[string]string{"owner": "novox", "repo": "app", "account": "mesh-tools"}) + w.h.React() + if w.op.count() != 1 { + t.Fatalf("after the write was granted the operator was asked %d times, wanted once", w.op.count()) + } + if d.Ready.CannotWrite != "" || strings.Contains(d.Ready.Waits, "cannot write") { + t.Fatalf("still says it cannot write: %q / %q", d.Ready.CannotWrite, d.Ready.Waits) + } +} diff --git a/modules/mesh-delivery-gate/module.json b/modules/mesh-delivery-gate/module.json index afe42fc1..6dd019be 100644 --- a/modules/mesh-delivery-gate/module.json +++ b/modules/mesh-delivery-gate/module.json @@ -26,7 +26,8 @@ "gitea.forge.pull-request.edited", "gitea.forge.pull-request.closed", "mesh-delivery-forge-bridge.forge.**", - "mesh-config.config-changed" + "mesh-config.config-changed", + "gitea.repo.writable" ], "emits": [ "transition",