From f0fbe193fa3c36afe633588996f25838b13ad513 Mon Sep 17 00:00:00 2001 From: jochen Date: Mon, 26 Oct 2026 17:59:34 +0100 Subject: [PATCH 1/8] gitea: mesh-tools administers a user's delivered repository and puts main's rule right, permissions read as the account sees them; knowledge-base.progress: minimal answers leave items to the tree, failed calls warn before parse faults (issues 1945, 1947) --- modules/gitea/client.ts | 48 ++++++++++-- modules/gitea/test/tools-account.test.ts | 31 +++++++- modules/gitea/writes.ts | 77 +++++++++++++------ .../cmd/mesh-knowledge/progress.go | 15 ++++ .../cmd/mesh-knowledge/progress_test.go | 11 +++ 5 files changed, 151 insertions(+), 31 deletions(-) diff --git a/modules/gitea/client.ts b/modules/gitea/client.ts index a36e113f..f63a4456 100644 --- a/modules/gitea/client.ts +++ b/modules/gitea/client.ts @@ -6,7 +6,7 @@ import { createHash } from "node:crypto"; import { readFileSync } from "node:fs"; import { ConfiguredToken, MintedToken, type TokenSource } from "./token.js"; -import { MAIN_BRANCH, mainRuleBody, protectionBody, type BranchProtection, type ProtectionWanted } from "./protection.js"; +import { MAIN_BRANCH, mainMergers, mainRuleBody, protectionBody, type BranchProtection, type ProtectionWanted } from "./protection.js"; import { Refused, checkSentPath, ruleCovers, pathSegment, pathSegments } from "./guard.js"; import { isCommit } from "./checks.js"; @@ -1047,12 +1047,19 @@ export class GiteaAdmin { return true; } - /** What the forge itself says an account may do on a repository: none, read, write, admin or owner. */ + /** What the forge gives an account on a repository, effective: none, read, write, admin or owner. It is the repository as + * that account sees it (the site administrator acts as the account with `sudo`), so access through an organisation's team + * counts. The collaborator endpoint is not used: it names only a collaborator's own permission, and says "read" of an + * account that writes through a team (novox/hq issue 1945). */ async repositoryPermission(owner: string, repo: string, user: string): Promise { - const path = `${repoPath(owner, repo)}/collaborators/${encodeURIComponent(user)}/permission`; + const path = `${repoPath(owner, repo)}?sudo=${encodeURIComponent(user)}`; const res = await this.request(path); + if (res.status === 404) return "none"; if (res.status !== 200) GiteaAdmin.fail(path, res); - return String(res.body?.permission ?? "none"); + const p = res.body?.permissions; + if (p?.admin === true) return res.body?.owner?.login === user ? "owner" : "admin"; + if (p?.push === true) return "write"; + return p?.pull === true ? "read" : "none"; } /** Make an account a collaborator of a repository that reads only. The forge asks a repository's administrator for @@ -1067,7 +1074,38 @@ export class GiteaAdmin { await this.addCollaborator(owner, repo, user, "write"); } - private async addCollaborator(owner: string, repo: string, user: string, permission: "read" | "write"): Promise { + /** Make an account an administrator collaborator of a repository (writes.ts: a repository of a user that builds a + * registered module, where the tools' account must also put main's rule right). */ + async addAdminCollaborator(owner: string, repo: string, user: string): Promise { + await this.addCollaborator(owner, repo, user, "admin"); + } + + /** Put main's rule right in one repository: no push, the merge whitelist (the tools' account first), no status required, + * and no administrator override. A rule the operator already set that is stronger is kept, never made weaker (ADR 0266): + * a whitelist that holds the tools' account and only logins the wanted one names, more approvals, a push already refused. + * Answers the rule as the forge holds it after, read back. With the administrator's login. */ + async ensureMainRule(owner: string, repo: string, mergers: string[] = mainMergers()): Promise<{ created: boolean; rule: BranchProtection }> { + const rules = await this.branchProtections(owner, repo); + const own = rules.find((r) => (r.rule_name ?? r.branch_name) === MAIN_BRANCH); + const kept = own?.enable_merge_whitelist === true ? (own.merge_whitelist_usernames ?? []) : []; + const whitelist = kept.length > 0 && kept.includes(mergers[0]) && kept.every((u) => mergers.includes(u)) ? kept : mergers; + const body = { ...mainRuleBody(mergers), merge_whitelist_usernames: whitelist, block_admin_merge_override: true }; + const path = `${repoPath(owner, repo)}/branch_protections`; + if (own) { + const res = await this.request(`${path}/${MAIN_BRANCH}`, { method: "PATCH", body: JSON.stringify(body) }); + if (res.status !== 200) GiteaAdmin.fail(`${path}/${MAIN_BRANCH}`, res); + } else { + const covering = rules.filter((r) => ruleCovers(String(r.rule_name ?? r.branch_name ?? ""), MAIN_BRANCH)); + const approvals = Math.max(0, ...covering.map((r) => Number(r.required_approvals ?? 0))); + const res = await this.request(path, { method: "POST", body: JSON.stringify({ rule_name: MAIN_BRANCH, ...(approvals > 0 ? { required_approvals: approvals } : {}), ...body }) }); + if (res.status !== 201) GiteaAdmin.fail(path, res); + } + const after = (await this.branchProtections(owner, repo)).find((r) => (r.rule_name ?? r.branch_name) === MAIN_BRANCH); + if (!after) throw new Error(`main's rule of ${owner}/${repo} was written, but the forge does not list it`); + return { created: !own, rule: after }; + } + + private async addCollaborator(owner: string, repo: string, user: string, permission: "read" | "write" | "admin"): Promise { const path = `${repoPath(owner, repo)}/collaborators/${encodeURIComponent(user)}`; const res = await this.request(path, { method: "PUT", body: JSON.stringify({ permission }) }); if (res.status === 204 || res.status === 200) return; diff --git a/modules/gitea/test/tools-account.test.ts b/modules/gitea/test/tools-account.test.ts index 80519429..23a30d67 100644 --- a/modules/gitea/test/tools-account.test.ts +++ b/modules/gitea/test/tools-account.test.ts @@ -28,6 +28,7 @@ interface Team { id: number; name: string; units: any; members: Set } interface ForgeRepo { owner: string; name: string; full: string; private: boolean; archived?: boolean; ownerVisibility: string; collaborators: Map } const forgeRepos: ForgeRepo[] = []; +const userRules = new Map(); const repoFaults = { refuse: new Set(), permission: new Set() }; function addRepo(full: string, isPrivate: boolean, ownerVisibility = "public"): ForgeRepo { const [owner, name] = full.split("/"); @@ -84,7 +85,7 @@ function fakeForge() { const server = createServer(async (req, res) => { const url = new URL(req.url ?? "/", "http://fake"); const p = url.pathname.replace(/^\/api\/v1/, ""); - const me = who(req); + const me = url.searchParams.has("sudo") && isAdmin(who(req)) ? url.searchParams.get("sudo") : who(req); calls.push(`${req.method} ${p} as ${me}`); if (me === null) return json(res, 401, { message: "unauthorized" }); @@ -168,6 +169,20 @@ function fakeForge() { if (m[2] === "feat/unguarded") return json(res, 200, { name: m[2], protected: false }); return json(res, 404, { message: "branch does not exist" }); } + // A repository of a user: its rules are its administrator's (a site administrator, or a collaborator with admin). + m = p.match(/^\/repos\/(?!novox\/)([^/]+)\/([^/]+)\/branch_protections(?:\/(.*))?$/); + if (m) { + const key = `${m[1]}/${m[2]}`; + const repo = forgeRepos.find((r) => r.full === key); + if (!repo || !["admin", "owner"].includes(permissionOf(me, repo))) return json(res, 403, { message: "user should be an owner or a collaborator with admin write of a repository" }); + if (req.method === "GET" && m[3] === undefined) return json(res, 200, userRules.has(key) ? [userRules.get(key)] : []); + if (req.method === "PATCH" && m[3] === "main") { + if (!userRules.has(key)) return json(res, 404, { message: "branch protection does not exist" }); + Object.assign(userRules.get(key), await read(req)); + return json(res, 200, userRules.get(key)); + } + if (req.method === "POST") { const b = await read(req); userRules.set(key, b); return json(res, 201, b); } + } // gitea's reqAdmin on the whole group: reading the rules is administration too. m = p.match(/^\/repos\/novox\/([^/]+)\/branch_protections(?:\/(.*))?$/); if (m) { @@ -205,6 +220,7 @@ function fakeForge() { m = p.match(/^\/repos\/([^/]+)\/([^/]+)(?:\/collaborators\/([^/]+)(\/permission)?)?$/); if (m) { const repo = forgeRepos.find((r) => r.owner === decodeURIComponent(m![1]) && r.name === decodeURIComponent(m![2])); + if (repo && req.method === "GET" && !m[3] && url.searchParams.has("sudo") && repoFaults.permission.has(repo.full)) return json(res, 500, { message: "the forge is not well" }); if (!repo || permissionOf(me, repo) === "none") return json(res, 404, { message: "repository does not exist" }); if (req.method === "GET" && !m[3]) { const mine = permissionOf(me, repo); @@ -214,7 +230,8 @@ function fakeForge() { if (req.method === "GET" && m[4]) { if (repoFaults.permission.has(repo.full)) return json(res, 500, { message: "the forge is not well" }); if (!isAdmin(me) && me !== user) return json(res, 403, { message: "only the user or an administrator" }); - return json(res, 200, { permission: permissionOf(user, repo) }); + // As gitea answers: the collaborator's own permission, not what a team gives the account. + return json(res, 200, { permission: repo.collaborators.get(user) ?? "read" }); } if (req.method === "PUT" && !m[4]) { // gitea's reqAdmin: the repository's administration, which on a repository of another user the site administrator alone holds. @@ -608,7 +625,9 @@ test("the merging account is given write on each repository the indexer's module 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("jschoubben/car-hunter").collaborators.get(TOOLS_ACCOUNT), "admin", "a user's repository is administered, not only written"); + assert.deepEqual(userRules.get("jschoubben/car-hunter"), { rule_name: "main", enable_push: false, enable_merge_whitelist: true, + merge_whitelist_usernames: [TOOLS_ACCOUNT], enable_status_check: false, status_check_contexts: [], block_admin_merge_override: true }); 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 } }]); @@ -636,8 +655,14 @@ test("the merging account is given write on each repository the indexer's module 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(); + // A stronger rule the operator set (a whitelist of the tools' account alone, two approvals) is kept, never weakened. + userRules.set("jschoubben/snake", { rule_name: "main", enable_push: true, enable_merge_whitelist: true, + merge_whitelist_usernames: [TOOLS_ACCOUNT], required_approvals: 2, block_admin_merge_override: false }); const after = await grantTool.run({ repositories: ["jschoubben/snake"] }) as any; assert.deepEqual(after.granted, ["jschoubben/snake"]); + assert.deepEqual(after.main_rule.map((x: any) => [x.repository, x.created, x.rule.enable_push, x.rule.required_approvals, x.rule.block_admin_merge_override]), + [["jschoubben/snake", false, false, 2, true]]); + assert.equal(repo("jschoubben/snake").collaborators.get(TOOLS_ACCOUNT), "admin"); 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" }); diff --git a/modules/gitea/writes.ts b/modules/gitea/writes.ts index ae9d41a6..70ef948c 100644 --- a/modules/gitea/writes.ts +++ b/modules/gitea/writes.ts @@ -18,10 +18,14 @@ // `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. // +// **A repository of a user is administered, not only written** (the operator, 2026-10-26: "mesh-tools should get all the +// permissions I give him"): the account is an admin collaborator there and puts main's rule right (client.ts ensureMainRule). +// // 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"; +import type { BranchProtection } from "./protection.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"; @@ -33,7 +37,8 @@ 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; + addAdminCollaborator(owner: string, repo: string, user: string): Promise; + ensureMainRule(owner: string, repo: string): Promise<{ created: boolean; rule: BranchProtection }>; } /** One change of the indexer's modules state, as the SDK's state watch hands it. */ @@ -51,8 +56,12 @@ export interface Written { delivered: string[]; /** Made writable now, each read back from the forge. */ granted: string[]; - /** Written already (the organisation's team, or a collaborator that writes). */ + /** Administered already (a repository of a user), or written through the organisation's team. */ already: string[]; + /** Each repository of a user whose main's rule was put right (no push, the merge whitelist, no status, no administrator + * override), read back: what the forge holds for it now. */ + main_rule: { repository: string; created: boolean; rule: { enable_push?: boolean; merge_whitelist_usernames?: string[] | null; + block_admin_merge_override?: boolean; enable_status_check?: boolean; required_approvals?: number } }[]; /** 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. */ @@ -60,6 +69,7 @@ export interface Written { } const WRITES = new Set(["write", "admin", "owner"]); +const ADMINS = new Set(["admin", "owner"]); const said = (err: unknown): string => (err instanceof Error ? err.message : String(err)); function split(full: string): [string, string] { @@ -118,7 +128,7 @@ export class DeliveredWrites { "(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 result: Written = { account: this.account, delivered, granted: [], already: [], main_rule: [], 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) { @@ -140,36 +150,55 @@ export class DeliveredWrites { 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) { + if (WRITES.has(permission)) { + this.writable.add(full); + result.already.push(full); + continue; + } // 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; + // A repository of a user: the account administers it (the operator, 2026-10-26: "mesh-tools should get all the + // permissions I give him"), so it also puts main's rule right, which only an administrator may. + let changed = false; + if (!ADMINS.has(permission)) { + try { + await this.forge.addAdminCollaborator(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 (!ADMINS.has(after)) { + result.refused.push({ repository: full, why: `the grant was answered, but the forge still gives ${this.account} "${after}"` }); + continue; + } + changed = true; } - let after: string; try { - after = await this.forge.repositoryPermission(owner, name, this.account); + const { created, rule } = await this.forge.ensureMainRule(owner, name); + result.main_rule.push({ repository: full, created, rule: { enable_push: rule.enable_push, + merge_whitelist_usernames: rule.merge_whitelist_usernames, block_admin_merge_override: rule.block_admin_merge_override, + enable_status_check: rule.enable_status_check, required_approvals: rule.required_approvals } }); } 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}"` }); + result.refused.push({ repository: full, why: `${this.account} administers it, but main's rule could not be put right — ${said(err)}` }); continue; } this.writable.add(full); + if (!changed) { + result.already.push(full); + continue; + } 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)}`)); @@ -230,12 +259,14 @@ export function writeTools(writes: () => DeliveredWrites): Tool[] { }, { 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). " + + description: "Give the mesh's merging forge account (mesh-tools) administration, as a collaborator, on every repository " + + "of a user that builds a registered module, and put main's rule right there (no push, the merge whitelist, no status " + + "required, no administrator override; a stronger rule the operator set is kept), or on the repositories named (each " + + "must build one). The answer's main_rule says each rule as the forge holds it after, read back. " + "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 " + + "was administered 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" }, diff --git a/modules/mesh-knowledge/cmd/mesh-knowledge/progress.go b/modules/mesh-knowledge/cmd/mesh-knowledge/progress.go index 5edd5aaf..99afc4ca 100644 --- a/modules/mesh-knowledge/cmd/mesh-knowledge/progress.go +++ b/modules/mesh-knowledge/cmd/mesh-knowledge/progress.go @@ -167,6 +167,8 @@ func (k *KB) Progress(a ProgressAsk) (*Progress, error) { if err != nil { return nil, err } + // A cut takes the warnings from the last: the calls that failed are said before the answers that did not parse. + callsFirst(p.Warnings) fitProgress(p, boundOf(a.Bound)) return p, nil } @@ -822,8 +824,21 @@ func movedSay(t *time.Time) string { return " · moved " + when(t) } +// callsFirst puts the warnings of calls that failed before those of answers that did not parse, in place. +func callsFirst(w []string) { + sort.SliceStable(w, func(i, j int) bool { + return !strings.Contains(w[i], " answered something else") && strings.Contains(w[j], " answered something else") + }) +} + // fitProgress keeps an answer within its bound: the items go first, then the tree's last lines, saying what was cut. +// A minimal answer never carries the items: its tree says the same, one line per milestone, so leaving them out is how +// the minimal form is made (the items of design 47 alone passed 8192 bytes) and no cut. A detailed answer's items, and +// everything else that goes, are said in cut. func fitProgress(p *Progress, bound int) { + if p.Detail == DetailMinimal && p.Tree != "" { + p.Items = nil + } if size(p) <= bound { return } diff --git a/modules/mesh-knowledge/cmd/mesh-knowledge/progress_test.go b/modules/mesh-knowledge/cmd/mesh-knowledge/progress_test.go index b2895846..ad8347af 100644 --- a/modules/mesh-knowledge/cmd/mesh-knowledge/progress_test.go +++ b/modules/mesh-knowledge/cmd/mesh-knowledge/progress_test.go @@ -306,12 +306,23 @@ func TestProgressOfADesignMinimalAndDetailed(t *testing.T) { if len(raw) > DefaultBound || p.Detail != DetailMinimal || !strings.Contains(p.Source, "issue 1781") { t.Fatalf("%d bytes, %s, %s", len(raw), p.Detail, p.Source) } + if p.Cut != "" || len(p.Items) != 0 { + t.Errorf("a minimal answer leaves the items to its tree and is not a cut: cut %q, %d items", p.Cut, len(p.Items)) + } + warns := []string{"mesh-delivery.show a answered something else: x", "issue-tracker.get 1: refused", "the tracker did not name 2", "issue-tracker.get 3 answered something else: y"} + callsFirst(warns) + if strings.Join(warns, "|") != "issue-tracker.get 1: refused|the tracker did not name 2|mesh-delivery.show a answered something else: x|issue-tracker.get 3 answered something else: y" { + t.Errorf("call failures do not come first: %q", warns) + } if seats.asked["seat:issue-tracker.list"] != 1 || seats.asked["seat:mesh-delivery.deliveries"] != 0 || seats.asked["seat:issue-tracker.get"] != 0 { t.Fatalf("minimal asked %v", seats.asked) } if w := strings.Join(p.Warnings, "\n"); !strings.Contains(w, "the tracker neither answered nor named missing") || !strings.Contains(w, "973, 525") { t.Errorf("a row the tracker did not answer is not said: %v", p.Warnings) } + if big, _ := k.Progress(ProgressAsk{Of: "design 47", Bound: 8192}); big.Cut != "" || big.Tree != p.Tree { + t.Errorf("a minimal answer at bound 8192 is cut: %q", big.Cut) + } lines := strings.Split(p.Tree, "\n") if !strings.HasPrefix(lines[0], "✗ design 47 · Delivery: from a commit to delivered — ") { t.Fatalf("first line %q", lines[0]) From c85472285d7156ee0f90d8613a0409ad507fd7ba Mon Sep 17 00:00:00 2001 From: jochen Date: Mon, 26 Oct 2026 18:23:42 +0100 Subject: [PATCH 2/8] gitea: main's rule read back must say what was written, since the forge silently drops a whitelist login that cannot write (issue 1945, review round 1) --- modules/gitea/client.ts | 7 +++++++ modules/gitea/test/tools-account.test.ts | 16 ++++++++++++++-- 2 files changed, 21 insertions(+), 2 deletions(-) diff --git a/modules/gitea/client.ts b/modules/gitea/client.ts index f63a4456..5e9f102e 100644 --- a/modules/gitea/client.ts +++ b/modules/gitea/client.ts @@ -1102,6 +1102,13 @@ export class GiteaAdmin { } const after = (await this.branchProtections(owner, repo)).find((r) => (r.rule_name ?? r.branch_name) === MAIN_BRANCH); if (!after) throw new Error(`main's rule of ${owner}/${repo} was written, but the forge does not list it`); + // The forge answers a write it did not keep: it drops a whitelist login that cannot write the repository, silently. + const listed = after.merge_whitelist_usernames ?? []; + const wrong: string[] = []; + if (after.enable_push !== false) wrong.push(`push is ${after.enable_push === true ? "allowed" : "not refused"}`); + if (after.enable_merge_whitelist !== true || !listed.includes(mergers[0])) wrong.push(`the merge whitelist [${listed.join(", ")}] leaves out ${mergers[0]}`); + if (after.enable_status_check !== false) wrong.push("a status is still required"); + if (wrong.length > 0) throw new Error(`main's rule of ${owner}/${repo} was written, but the forge holds otherwise: ${wrong.join("; ")}`); return { created: !own, rule: after }; } diff --git a/modules/gitea/test/tools-account.test.ts b/modules/gitea/test/tools-account.test.ts index 23a30d67..2f1cd6aa 100644 --- a/modules/gitea/test/tools-account.test.ts +++ b/modules/gitea/test/tools-account.test.ts @@ -176,12 +176,15 @@ function fakeForge() { const repo = forgeRepos.find((r) => r.full === key); if (!repo || !["admin", "owner"].includes(permissionOf(me, repo))) return json(res, 403, { message: "user should be an owner or a collaborator with admin write of a repository" }); if (req.method === "GET" && m[3] === undefined) return json(res, 200, userRules.has(key) ? [userRules.get(key)] : []); + // As gitea does: a whitelist login that cannot write the repository is dropped, and the write still answered. + const keep = (b: any) => (Array.isArray(b.merge_whitelist_usernames) + ? { ...b, merge_whitelist_usernames: b.merge_whitelist_usernames.filter((u: string) => ["write", "admin", "owner"].includes(permissionOf(u, repo))) } : b); if (req.method === "PATCH" && m[3] === "main") { if (!userRules.has(key)) return json(res, 404, { message: "branch protection does not exist" }); - Object.assign(userRules.get(key), await read(req)); + Object.assign(userRules.get(key), keep(await read(req))); return json(res, 200, userRules.get(key)); } - if (req.method === "POST") { const b = await read(req); userRules.set(key, b); return json(res, 201, b); } + if (req.method === "POST") { const b = keep(await read(req)); userRules.set(key, b); return json(res, 201, b); } } // gitea's reqAdmin on the whole group: reading the rules is administration too. m = p.match(/^\/repos\/novox\/([^/]+)\/branch_protections(?:\/(.*))?$/); @@ -664,6 +667,15 @@ test("the merging account is given write on each repository the indexer's module [["jschoubben/snake", false, false, 2, true]]); assert.equal(repo("jschoubben/snake").collaborators.get(TOOLS_ACCOUNT), "admin"); assert.deepEqual(((await unwritableTool.run({})) as any).unwritable, []); + // The forge drops a whitelist login that cannot write and still answers 200: main's rule is read back and that fails loud. + const admin = GiteaAdmin.fromEnv(env); + await assert.rejects(admin.ensureMainRule("jschoubben", "dotfiles"), + /main's rule of jschoubben\/dotfiles was written, but the forge holds otherwise: the merge whitelist \[\] leaves out mesh-tools/); + userRules.get("jschoubben/snake").enable_status_check = true; + userRules.get("jschoubben/snake").enable_push = true; + const held = await admin.ensureMainRule("jschoubben", "snake"); + assert.deepEqual([held.rule.enable_push, held.rule.enable_status_check, held.rule.merge_whitelist_usernames], [false, false, [TOOLS_ACCOUNT]]); + userRules.delete("jschoubben/dotfiles"); // 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"]); From e1e5a8ff0dfc97d19a59250bb7efacf5ea116c44 Mon Sep 17 00:00:00 2001 From: jochen Date: Mon, 26 Oct 2026 18:34:03 +0100 Subject: [PATCH 3/8] gitea: mesh-tools writes a user's delivered repository and main's rule stays the operator's: only its merge whitelist gets mesh-tools, read back; what else blocks the mesh's merge is said (issue 1945, the operator's answer 2026-10-26 18:30) --- modules/gitea/client.ts | 68 +++++++-------- modules/gitea/test/tools-account.test.ts | 40 +++++---- modules/gitea/writes.ts | 101 ++++++++++++++++------- 3 files changed, 124 insertions(+), 85 deletions(-) diff --git a/modules/gitea/client.ts b/modules/gitea/client.ts index 5e9f102e..408c93af 100644 --- a/modules/gitea/client.ts +++ b/modules/gitea/client.ts @@ -6,7 +6,7 @@ import { createHash } from "node:crypto"; import { readFileSync } from "node:fs"; import { ConfiguredToken, MintedToken, type TokenSource } from "./token.js"; -import { MAIN_BRANCH, mainMergers, mainRuleBody, protectionBody, type BranchProtection, type ProtectionWanted } from "./protection.js"; +import { MAIN_BRANCH, mainRuleBody, protectionBody, type BranchProtection, type ProtectionWanted } from "./protection.js"; import { Refused, checkSentPath, ruleCovers, pathSegment, pathSegments } from "./guard.js"; import { isCommit } from "./checks.js"; @@ -1074,45 +1074,39 @@ export class GiteaAdmin { await this.addCollaborator(owner, repo, user, "write"); } - /** Make an account an administrator collaborator of a repository (writes.ts: a repository of a user that builds a - * registered module, where the tools' account must also put main's rule right). */ - async addAdminCollaborator(owner: string, repo: string, user: string): Promise { - await this.addCollaborator(owner, repo, user, "admin"); - } - - /** Put main's rule right in one repository: no push, the merge whitelist (the tools' account first), no status required, - * and no administrator override. A rule the operator already set that is stronger is kept, never made weaker (ADR 0266): - * a whitelist that holds the tools' account and only logins the wanted one names, more approvals, a push already refused. - * Answers the rule as the forge holds it after, read back. With the administrator's login. */ - async ensureMainRule(owner: string, repo: string, mergers: string[] = mainMergers()): Promise<{ created: boolean; rule: BranchProtection }> { + /** The rule that applies to main in a repository, as its administrator reads it: a rule named main, else the first glob + * rule covering main; null when none does. With the administrator's login. */ + async mainRule(owner: string, repo: string): Promise { const rules = await this.branchProtections(owner, repo); - const own = rules.find((r) => (r.rule_name ?? r.branch_name) === MAIN_BRANCH); - const kept = own?.enable_merge_whitelist === true ? (own.merge_whitelist_usernames ?? []) : []; - const whitelist = kept.length > 0 && kept.includes(mergers[0]) && kept.every((u) => mergers.includes(u)) ? kept : mergers; - const body = { ...mainRuleBody(mergers), merge_whitelist_usernames: whitelist, block_admin_merge_override: true }; - const path = `${repoPath(owner, repo)}/branch_protections`; - if (own) { - const res = await this.request(`${path}/${MAIN_BRANCH}`, { method: "PATCH", body: JSON.stringify(body) }); - if (res.status !== 200) GiteaAdmin.fail(`${path}/${MAIN_BRANCH}`, res); - } else { - const covering = rules.filter((r) => ruleCovers(String(r.rule_name ?? r.branch_name ?? ""), MAIN_BRANCH)); - const approvals = Math.max(0, ...covering.map((r) => Number(r.required_approvals ?? 0))); - const res = await this.request(path, { method: "POST", body: JSON.stringify({ rule_name: MAIN_BRANCH, ...(approvals > 0 ? { required_approvals: approvals } : {}), ...body }) }); - if (res.status !== 201) GiteaAdmin.fail(path, res); - } - const after = (await this.branchProtections(owner, repo)).find((r) => (r.rule_name ?? r.branch_name) === MAIN_BRANCH); - if (!after) throw new Error(`main's rule of ${owner}/${repo} was written, but the forge does not list it`); - // The forge answers a write it did not keep: it drops a whitelist login that cannot write the repository, silently. - const listed = after.merge_whitelist_usernames ?? []; - const wrong: string[] = []; - if (after.enable_push !== false) wrong.push(`push is ${after.enable_push === true ? "allowed" : "not refused"}`); - if (after.enable_merge_whitelist !== true || !listed.includes(mergers[0])) wrong.push(`the merge whitelist [${listed.join(", ")}] leaves out ${mergers[0]}`); - if (after.enable_status_check !== false) wrong.push("a status is still required"); - if (wrong.length > 0) throw new Error(`main's rule of ${owner}/${repo} was written, but the forge holds otherwise: ${wrong.join("; ")}`); - return { created: !own, rule: after }; + return rules.find((r) => (r.rule_name ?? r.branch_name) === MAIN_BRANCH) + ?? rules.find((r) => ruleCovers(String(r.rule_name ?? r.branch_name ?? ""), MAIN_BRANCH)) ?? null; } - private async addCollaborator(owner: string, repo: string, user: string, permission: "read" | "write" | "admin"): Promise { + /** Put an account on main's merge whitelist in a repository of a user, and nothing else (the operator, 2026-10-26: + * write, the rule untouched). Only an existing rule whose merge whitelist is on is changed, and only its whitelist: the + * account is added to the logins already there. Push, status checks, approvals and the administrator override stay as + * the operator set them; with no rule, or the whitelist off, nothing is written (whoever writes may merge). Read back: + * the forge drops a whitelist login that cannot write the repository and still answers success, so a whitelist read back + * without the account fails loud. Answers the rule as the forge holds it after. With the administrator's login. */ + async ensureMergerListed(owner: string, repo: string, account: string): Promise<{ changed: boolean; rule: BranchProtection | null }> { + const rule = await this.mainRule(owner, repo); + const listed = rule?.merge_whitelist_usernames ?? []; + if (!rule || rule.enable_merge_whitelist !== true || listed.includes(account)) return { changed: false, rule }; + const name = String(rule.rule_name ?? rule.branch_name); + const path = `${repoPath(owner, repo)}/branch_protections/${encodeURIComponent(name)}`; + const res = await this.request(path, { method: "PATCH", body: JSON.stringify({ merge_whitelist_usernames: [...listed, account] }) }); + if (res.status !== 200) GiteaAdmin.fail(path, res); + const after = (await this.branchProtections(owner, repo)).find((r) => (r.rule_name ?? r.branch_name) === name); + if (!after) throw new Error(`main's rule ${name} of ${owner}/${repo} was written, but the forge does not list it`); + const held = after.merge_whitelist_usernames ?? []; + if (after.enable_merge_whitelist === true && !held.includes(account)) { + throw new Error(`${account} was added to the merge whitelist of main's rule ${name} of ${owner}/${repo}, but the forge ` + + `holds [${held.join(", ")}]: it drops a login that cannot write the repository`); + } + return { changed: true, rule: after }; + } + + 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 }) }); if (res.status === 204 || res.status === 200) return; diff --git a/modules/gitea/test/tools-account.test.ts b/modules/gitea/test/tools-account.test.ts index 2f1cd6aa..02644863 100644 --- a/modules/gitea/test/tools-account.test.ts +++ b/modules/gitea/test/tools-account.test.ts @@ -628,9 +628,8 @@ test("the merging account is given write on each repository the indexer's module 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), "admin", "a user's repository is administered, not only written"); - assert.deepEqual(userRules.get("jschoubben/car-hunter"), { rule_name: "main", enable_push: false, enable_merge_whitelist: true, - merge_whitelist_usernames: [TOOLS_ACCOUNT], enable_status_check: false, status_check_contexts: [], block_admin_merge_override: true }); + assert.equal(repo("jschoubben/car-hunter").collaborators.get(TOOLS_ACCOUNT), "write", "a user's repository is written, not administered"); + assert.equal(userRules.has("jschoubben/car-hunter"), false, "a repository with no rule for main is given none"); 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 } }]); @@ -658,23 +657,28 @@ test("the merging account is given write on each repository the indexer's module 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(); - // A stronger rule the operator set (a whitelist of the tools' account alone, two approvals) is kept, never weakened. - userRules.set("jschoubben/snake", { rule_name: "main", enable_push: true, enable_merge_whitelist: true, - merge_whitelist_usernames: [TOOLS_ACCOUNT], required_approvals: 2, block_admin_merge_override: false }); + // The operator's rule is left as set (the operator, 2026-10-26: "Write, rule untouched"): only mesh-tools is added to its + // merge whitelist; a status the mesh does not set and required approvals are said as blocking the mesh's merge, unchanged. + const operators = { rule_name: "main", enable_push: true, enable_merge_whitelist: true, merge_whitelist_usernames: ["jschoubben"], + enable_status_check: true, status_check_contexts: ["mesh/merge-gate", "ci/external"], required_approvals: 2, block_admin_merge_override: false }; + userRules.set("jschoubben/snake", structuredClone(operators)); const after = await grantTool.run({ repositories: ["jschoubben/snake"] }) as any; assert.deepEqual(after.granted, ["jschoubben/snake"]); - assert.deepEqual(after.main_rule.map((x: any) => [x.repository, x.created, x.rule.enable_push, x.rule.required_approvals, x.rule.block_admin_merge_override]), - [["jschoubben/snake", false, false, 2, true]]); - assert.equal(repo("jschoubben/snake").collaborators.get(TOOLS_ACCOUNT), "admin"); - assert.deepEqual(((await unwritableTool.run({})) as any).unwritable, []); - // The forge drops a whitelist login that cannot write and still answers 200: main's rule is read back and that fails loud. - const admin = GiteaAdmin.fromEnv(env); - await assert.rejects(admin.ensureMainRule("jschoubben", "dotfiles"), - /main's rule of jschoubben\/dotfiles was written, but the forge holds otherwise: the merge whitelist \[\] leaves out mesh-tools/); - userRules.get("jschoubben/snake").enable_status_check = true; - userRules.get("jschoubben/snake").enable_push = true; - const held = await admin.ensureMainRule("jschoubben", "snake"); - assert.deepEqual([held.rule.enable_push, held.rule.enable_status_check, held.rule.merge_whitelist_usernames], [false, false, [TOOLS_ACCOUNT]]); + assert.equal(repo("jschoubben/snake").collaborators.get(TOOLS_ACCOUNT), "write"); + assert.deepEqual(userRules.get("jschoubben/snake"), { ...operators, merge_whitelist_usernames: ["jschoubben", TOOLS_ACCOUNT] }, "only the whitelist changed"); + assert.deepEqual(after.merge_whitelist, [{ repository: "jschoubben/snake", changed: true, listed: ["jschoubben", TOOLS_ACCOUNT] }]); + const blockedWhy = "writable, but main's rule blocks the mesh's merge: it requires the status ci/external, which the mesh does not set; " + + "it requires 2 approving reviews on the forge, which the gate does not give"; + assert.deepEqual(after.merge_blocked, [{ repository: "jschoubben/snake", why: blockedWhy }]); + const again = await unwritableTool.run({}) as any; + assert.deepEqual([again.unwritable, again.merge_blocked], [[], [{ repository: "jschoubben/snake", why: blockedWhy }]]); + const writesBefore = forge.calls.filter((c) => /^PATCH .*branch_protections/.test(c)).length; + assert.deepEqual(((await grantTool.run({ repositories: ["jschoubben/snake"] })) as any).merge_whitelist[0].changed, false); + assert.equal(forge.calls.filter((c) => /^PATCH .*branch_protections/.test(c)).length, writesBefore, "a listed account is not written again"); + // The forge drops a whitelist login that cannot write and still answers 200: the whitelist is read back and that fails loud. + userRules.set("jschoubben/dotfiles", { rule_name: "main", enable_merge_whitelist: true, merge_whitelist_usernames: ["jschoubben"] }); + await assert.rejects(GiteaAdmin.fromEnv(env).ensureMergerListed("jschoubben", "dotfiles", TOOLS_ACCOUNT), + /mesh-tools was added to the merge whitelist of main's rule main of jschoubben\/dotfiles, but the forge holds \[jschoubben\]: it drops a login that cannot write/); userRules.delete("jschoubben/dotfiles"); // A module gone from the indexer leaves its repository out of the delivered list. await writes.take({ key: "snake", op: "delete" }); diff --git a/modules/gitea/writes.ts b/modules/gitea/writes.ts index 70ef948c..5237625c 100644 --- a/modules/gitea/writes.ts +++ b/modules/gitea/writes.ts @@ -18,14 +18,18 @@ // `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. // -// **A repository of a user is administered, not only written** (the operator, 2026-10-26: "mesh-tools should get all the -// permissions I give him"): the account is an admin collaborator there and puts main's rule right (client.ts ensureMainRule). +// **A repository of a user is written, and main's rule is left as the operator set it** (the operator, 2026-10-26, asked +// "Write, rule untouched"): the account is a write collaborator there, and only added to main's merge whitelist when that +// rule keeps one (client.ts ensureMergerListed). What else in main's rule stops the mesh's merge (a status the mesh does not +// set, approvals the gate does not give) is changed by nobody but said: both tools list such a repository as writable, but +// blocked, with why (merge_blocked). // // 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"; import type { BranchProtection } from "./protection.js"; +import { CHECK_CONTEXT, REPO_CHECK_CONTEXT } from "./pulls.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"; @@ -37,10 +41,33 @@ export interface WriteForge { everyRepository(): Promise; repository(owner: string, repo: string): Promise; repositoryPermission(owner: string, repo: string, user: string): Promise; - addAdminCollaborator(owner: string, repo: string, user: string): Promise; - ensureMainRule(owner: string, repo: string): Promise<{ created: boolean; rule: BranchProtection }>; + addWriteCollaborator(owner: string, repo: string, user: string): Promise; + mainRule(owner: string, repo: string): Promise; + ensureMergerListed(owner: string, repo: string, account: string): Promise<{ changed: boolean; rule: BranchProtection | null }>; } +/** The statuses every pull request the mesh delivers carries (mesh-delivery's checks); a rule requiring another blocks. */ +const MESH_CONTEXTS = new Set([CHECK_CONTEXT, REPO_CHECK_CONTEXT]); + +/** What in main's rule stops the mesh's merge, in words; empty when nothing does. Changed by nobody here: the operator's. */ +export function mergeBlocks(rule: BranchProtection | null, account: string): string[] { + if (!rule) return []; + const blocks: string[] = []; + if (rule.enable_merge_whitelist === true && !(rule.merge_whitelist_usernames ?? []).includes(account)) { + blocks.push(`its merge whitelist [${(rule.merge_whitelist_usernames ?? []).join(", ")}] leaves out ${account} (gitea_grant_write adds it)`); + } + if (rule.enable_status_check === true) { + for (const c of rule.status_check_contexts ?? []) { + if (!MESH_CONTEXTS.has(c)) blocks.push(`it requires the status ${c}, which the mesh does not set`); + } + } + const approvals = Number(rule.required_approvals ?? 0); + if (approvals > 0) blocks.push(`it requires ${approvals} approving review${approvals === 1 ? "" : "s"} on the forge, which the gate does not give`); + return blocks; +} + +const blockedWhy = (blocks: string[]): string => `writable, but main's rule blocks the mesh's merge: ${blocks.join("; ")}`; + /** One change of the indexer's modules state, as the SDK's state watch hands it. */ export interface ModulesChange { key: string; @@ -56,12 +83,13 @@ export interface Written { delivered: string[]; /** Made writable now, each read back from the forge. */ granted: string[]; - /** Administered already (a repository of a user), or written through the organisation's team. */ + /** Written already (a collaborator of a user's repository), or written through the organisation's team. */ already: string[]; - /** Each repository of a user whose main's rule was put right (no push, the merge whitelist, no status, no administrator - * override), read back: what the forge holds for it now. */ - main_rule: { repository: string; created: boolean; rule: { enable_push?: boolean; merge_whitelist_usernames?: string[] | null; - block_admin_merge_override?: boolean; enable_status_check?: boolean; required_approvals?: number } }[]; + /** Each repository of a user: whether the account was added to main's merge whitelist now, and the whitelist read back + * (null: main's rule keeps no whitelist, or there is no rule, and whoever writes may merge). */ + merge_whitelist: { repository: string; changed: boolean; listed: string[] | null }[]; + /** Writable, but main's rule (the operator's, left as set) stops the mesh's merge, and why. */ + merge_blocked: { repository: string; why: 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. */ @@ -69,7 +97,6 @@ export interface Written { } const WRITES = new Set(["write", "admin", "owner"]); -const ADMINS = new Set(["admin", "owner"]); const said = (err: unknown): string => (err instanceof Error ? err.message : String(err)); function split(full: string): [string, string] { @@ -128,7 +155,7 @@ export class DeliveredWrites { "(the watch is not granted or not answered); nothing is granted"); } const delivered = this.delivered(); - const result: Written = { account: this.account, delivered, granted: [], already: [], main_rule: [], refused: [], left_read_only: [] }; + const result: Written = { account: this.account, delivered, granted: [], already: [], merge_whitelist: [], merge_blocked: [], 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) { @@ -162,12 +189,11 @@ export class DeliveredWrites { "set again at the next mint of the tools' token)" }); continue; } - // A repository of a user: the account administers it (the operator, 2026-10-26: "mesh-tools should get all the - // permissions I give him"), so it also puts main's rule right, which only an administrator may. + // A repository of a user: the account writes it, and is put on main's merge whitelist when the rule keeps one. let changed = false; - if (!ADMINS.has(permission)) { + if (!WRITES.has(permission)) { try { - await this.forge.addAdminCollaborator(owner, name, this.account); + await this.forge.addWriteCollaborator(owner, name, this.account); } catch (err) { result.refused.push({ repository: full, why: said(err) }); continue; @@ -179,19 +205,20 @@ export class DeliveredWrites { result.refused.push({ repository: full, why: `granted, but its permission could not be read back — ${said(err)}` }); continue; } - if (!ADMINS.has(after)) { + if (!WRITES.has(after)) { result.refused.push({ repository: full, why: `the grant was answered, but the forge still gives ${this.account} "${after}"` }); continue; } changed = true; } try { - const { created, rule } = await this.forge.ensureMainRule(owner, name); - result.main_rule.push({ repository: full, created, rule: { enable_push: rule.enable_push, - merge_whitelist_usernames: rule.merge_whitelist_usernames, block_admin_merge_override: rule.block_admin_merge_override, - enable_status_check: rule.enable_status_check, required_approvals: rule.required_approvals } }); + const { changed: listedNow, rule } = await this.forge.ensureMergerListed(owner, name, this.account); + result.merge_whitelist.push({ repository: full, changed: listedNow, + listed: rule?.enable_merge_whitelist === true ? (rule.merge_whitelist_usernames ?? []) : null }); + const blocks = mergeBlocks(rule, this.account); + if (blocks.length > 0) result.merge_blocked.push({ repository: full, why: blockedWhy(blocks) }); } catch (err) { - result.refused.push({ repository: full, why: `${this.account} administers it, but main's rule could not be put right — ${said(err)}` }); + result.refused.push({ repository: full, why: `${this.account} writes it, but could not be put on main's merge whitelist — ${said(err)}` }); continue; } this.writable.add(full); @@ -221,10 +248,12 @@ export class DeliveredWrites { } /** 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 }[] }> { + async unwritable(): Promise<{ account: string; delivered: number; unwritable: { repository: string; permission: string; why: string }[]; + merge_blocked: { repository: 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 }[] = []; + const blocked: { repository: string; why: string }[] = []; for (const full of delivered) { const [owner, name] = split(full); let permission: string; @@ -234,12 +263,21 @@ export class DeliveredWrites { 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; + if (WRITES.has(permission)) { + if (owner === this.org) continue; // the organisation's main rules are the mesh's own (mainrules.ts) + try { + const blocks = mergeBlocks(await this.forge.mainRule(owner, name), this.account); + if (blocks.length > 0) blocked.push({ repository: full, why: blockedWhy(blocks) }); + } catch (err) { + blocked.push({ repository: full, why: `writable, but main's rule could not be read — ${said(err)}` }); + } + 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 }; + return { account: this.account, delivered: delivered.length, unwritable: out, merge_blocked: blocked }; } } @@ -253,20 +291,23 @@ export function writeTools(writes: () => DeliveredWrites): Tool[] { 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.", + "delivery gate does not ask its Approve. Empty when it writes every one. Its merge_blocked lists each repository of " + + "a user it writes whose main's rule (the operator's, left as set) still stops the mesh's merge: a merge whitelist " + + "without it, a required status the mesh does not set, required approving reviews. 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) administration, as a collaborator, on every repository " + - "of a user that builds a registered module, and put main's rule right there (no push, the merge whitelist, no status " + - "required, no administrator override; a stronger rule the operator set is kept), or on the repositories named (each " + - "must build one). The answer's main_rule says each rule as the forge holds it after, read back. " + + description: "Give the mesh's merging forge account (mesh-tools) write, as a collaborator, on every repository of a " + + "user that builds a registered module, or on the repositories named (each must build one). Main's rule there is left " + + "as the operator set it; only when it keeps a merge whitelist is mesh-tools added to it, read back (merge_whitelist). " + + "What else in that rule stops the mesh's merge (a required status the mesh does not set, required approving reviews) " + + "is changed by nobody and said in merge_blocked. " + "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 administered already, what the forge refused in its own words, and which other repositories it left read-only " + + "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" }, From c584dc57b95e52cfa7dc32bdeff20b0362a81de9 Mon Sep 17 00:00:00 2001 From: jochen Date: Mon, 26 Oct 2026 18:37:02 +0100 Subject: [PATCH 4/8] gitea: mesh/check is the mesh's own status and never blocks; signed commits, an outdated branch and a rejected review are said as blocking the mesh's merge (issue 1945, review round 2) --- modules/gitea/test/tools-account.test.ts | 12 ++++++++++-- modules/gitea/writes.ts | 7 +++++-- 2 files changed, 15 insertions(+), 4 deletions(-) diff --git a/modules/gitea/test/tools-account.test.ts b/modules/gitea/test/tools-account.test.ts index 02644863..c69edb77 100644 --- a/modules/gitea/test/tools-account.test.ts +++ b/modules/gitea/test/tools-account.test.ts @@ -660,7 +660,8 @@ test("the merging account is given write on each repository the indexer's module // The operator's rule is left as set (the operator, 2026-10-26: "Write, rule untouched"): only mesh-tools is added to its // merge whitelist; a status the mesh does not set and required approvals are said as blocking the mesh's merge, unchanged. const operators = { rule_name: "main", enable_push: true, enable_merge_whitelist: true, merge_whitelist_usernames: ["jschoubben"], - enable_status_check: true, status_check_contexts: ["mesh/merge-gate", "ci/external"], required_approvals: 2, block_admin_merge_override: false }; + enable_status_check: true, status_check_contexts: ["mesh/check", "mesh/merge-gate", "ci/external"], required_approvals: 2, + block_admin_merge_override: false, require_signed_commits: true, block_on_outdated_branch: true, block_on_rejected_reviews: true }; userRules.set("jschoubben/snake", structuredClone(operators)); const after = await grantTool.run({ repositories: ["jschoubben/snake"] }) as any; assert.deepEqual(after.granted, ["jschoubben/snake"]); @@ -668,13 +669,20 @@ test("the merging account is given write on each repository the indexer's module assert.deepEqual(userRules.get("jschoubben/snake"), { ...operators, merge_whitelist_usernames: ["jschoubben", TOOLS_ACCOUNT] }, "only the whitelist changed"); assert.deepEqual(after.merge_whitelist, [{ repository: "jschoubben/snake", changed: true, listed: ["jschoubben", TOOLS_ACCOUNT] }]); const blockedWhy = "writable, but main's rule blocks the mesh's merge: it requires the status ci/external, which the mesh does not set; " + - "it requires 2 approving reviews on the forge, which the gate does not give"; + "it requires 2 approving reviews on the forge, which the gate does not give; it requires signed commits, which the mesh does not make; " + + "it refuses a branch behind main, which the mesh does not bring up to date; " + + "it refuses a merge after a review asked for changes, which the gate does not clear"; assert.deepEqual(after.merge_blocked, [{ repository: "jschoubben/snake", why: blockedWhy }]); const again = await unwritableTool.run({}) as any; assert.deepEqual([again.unwritable, again.merge_blocked], [[], [{ repository: "jschoubben/snake", why: blockedWhy }]]); const writesBefore = forge.calls.filter((c) => /^PATCH .*branch_protections/.test(c)).length; assert.deepEqual(((await grantTool.run({ repositories: ["jschoubben/snake"] })) as any).merge_whitelist[0].changed, false); assert.equal(forge.calls.filter((c) => /^PATCH .*branch_protections/.test(c)).length, writesBefore, "a listed account is not written again"); + // The mesh's own statuses never block: a rule requiring only them, with mesh-tools listed, is not reported. + userRules.set("jschoubben/snake", { rule_name: "main", enable_merge_whitelist: true, merge_whitelist_usernames: [TOOLS_ACCOUNT], + enable_status_check: true, status_check_contexts: ["mesh/check", "mesh/merge-gate", "mesh/repo-check"] }); + assert.deepEqual(((await unwritableTool.run({})) as any).merge_blocked, []); + assert.deepEqual(((await grantTool.run({ repositories: ["jschoubben/snake"] })) as any).merge_blocked, []); // The forge drops a whitelist login that cannot write and still answers 200: the whitelist is read back and that fails loud. userRules.set("jschoubben/dotfiles", { rule_name: "main", enable_merge_whitelist: true, merge_whitelist_usernames: ["jschoubben"] }); await assert.rejects(GiteaAdmin.fromEnv(env).ensureMergerListed("jschoubben", "dotfiles", TOOLS_ACCOUNT), diff --git a/modules/gitea/writes.ts b/modules/gitea/writes.ts index 5237625c..552f7d58 100644 --- a/modules/gitea/writes.ts +++ b/modules/gitea/writes.ts @@ -29,7 +29,7 @@ import type { ForgeRepository } from "./client.js"; import type { BranchProtection } from "./protection.js"; -import { CHECK_CONTEXT, REPO_CHECK_CONTEXT } from "./pulls.js"; +import { CHECK_CONTEXT, CHECK_STATUS, REPO_CHECK_CONTEXT } from "./pulls.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"; @@ -47,7 +47,7 @@ export interface WriteForge { } /** The statuses every pull request the mesh delivers carries (mesh-delivery's checks); a rule requiring another blocks. */ -const MESH_CONTEXTS = new Set([CHECK_CONTEXT, REPO_CHECK_CONTEXT]); +const MESH_CONTEXTS = new Set([CHECK_STATUS, CHECK_CONTEXT, REPO_CHECK_CONTEXT]); /** What in main's rule stops the mesh's merge, in words; empty when nothing does. Changed by nobody here: the operator's. */ export function mergeBlocks(rule: BranchProtection | null, account: string): string[] { @@ -63,6 +63,9 @@ export function mergeBlocks(rule: BranchProtection | null, account: string): str } const approvals = Number(rule.required_approvals ?? 0); if (approvals > 0) blocks.push(`it requires ${approvals} approving review${approvals === 1 ? "" : "s"} on the forge, which the gate does not give`); + if (rule.require_signed_commits === true) blocks.push("it requires signed commits, which the mesh does not make"); + if (rule.block_on_outdated_branch === true) blocks.push("it refuses a branch behind main, which the mesh does not bring up to date"); + if (rule.block_on_rejected_reviews === true) blocks.push("it refuses a merge after a review asked for changes, which the gate does not clear"); return blocks; } From 854dbfb3f3f073b235163e0c5d6dc7e9d440deba Mon Sep 17 00:00:00 2001 From: jochen Date: Mon, 26 Oct 2026 19:12:12 +0100 Subject: [PATCH 5/8] mesh-issues: a milestone or commit link moves no issue and keeps no worker current (issue 1781) The seed linked 188 issues at once and every one read as moved at the seed's minute, hiding when each really moved; on a worked issue the link also counted as its worker's activity. These links only place an issue, so last-moved and the worker's activity leave them out. --- .../cmd/mesh-issues/list_progress_test.go | 19 ++++++++++++++++++- .../mesh-issues/cmd/mesh-issues/tracker.go | 18 ++++++++++++++---- 2 files changed, 32 insertions(+), 5 deletions(-) diff --git a/modules/mesh-issues/cmd/mesh-issues/list_progress_test.go b/modules/mesh-issues/cmd/mesh-issues/list_progress_test.go index ef252169..844989f1 100644 --- a/modules/mesh-issues/cmd/mesh-issues/list_progress_test.go +++ b/modules/mesh-issues/cmd/mesh-issues/list_progress_test.go @@ -2,6 +2,7 @@ package main import ( "reflect" + "strings" "testing" "time" ) @@ -26,6 +27,22 @@ func TestListSaysWhenEachIssueMovedAndKeepsTheLinkedOnes(t *testing.T) { if _, err := h.Link(LinkAsk{Number: quiet, Kind: LinkDesign, Target: "03-DESIGN/01-to-be/47-delivery-from-commit-to-delivered.md"}); err != nil { t.Fatal(err) } + // Issue 1781: a milestone or a commit link places the issue and moves nothing, nor keeps its worker current. + if _, err := h.milestoneSet("lead@shanks", MilestoneSetAsk{Design: 47, ID: "R6", Outcome: "x"}); err != nil { + t.Fatal(err) + } + h.fixing(t, noted) + worked, _ := h.Get(noted) + clock = clock.Add(time.Hour) + for _, l := range []LinkAsk{{Number: noted, Kind: LinkMilestone, Target: "47/R6"}, + {Number: noted, Kind: LinkCommit, Target: "mesh-catalog@" + strings.Repeat("a", 40)}} { + if _, err := h.link("seed@shanks", l); err != nil { + t.Fatal(err) + } + } + if after, _ := h.Get(noted); !after.Worker.Active.Equal(worked.Worker.Active) { + t.Fatalf("a milestone link kept the worker current: %v, was %v", after.Worker.Active, worked.Worker.Active) + } start := time.Date(2026, 10, 26, 12, 0, 0, 0, time.UTC) for _, c := range []struct { name string @@ -35,7 +52,7 @@ func TestListSaysWhenEachIssueMovedAndKeepsTheLinkedOnes(t *testing.T) { refuse bool }{ {"numbers say moved", ListAsk{Numbers: []uint64{quiet, noted, linked}}, []uint64{quiet, noted, linked}, - []time.Time{start.Add(2 * time.Hour), start.Add(time.Hour), start.Add(2 * time.Hour)}, false}, + []time.Time{start.Add(2 * time.Hour), start.Add(2 * time.Hour), start.Add(2 * time.Hour)}, false}, {"linked to a decision", ListAsk{Linked: "decision:0493"}, []uint64{linked}, nil, false}, {"linked to a design", ListAsk{Linked: " design:03-DESIGN/01-to-be/47-delivery-from-commit-to-delivered.md"}, []uint64{quiet}, nil, false}, {"linked and numbers together", ListAsk{Linked: "decision:0493", Numbers: []uint64{quiet, noted}}, nil, nil, false}, diff --git a/modules/mesh-issues/cmd/mesh-issues/tracker.go b/modules/mesh-issues/cmd/mesh-issues/tracker.go index 2f06b304..03780639 100644 --- a/modules/mesh-issues/cmd/mesh-issues/tracker.go +++ b/modules/mesh-issues/cmd/mesh-issues/tracker.go @@ -675,7 +675,9 @@ func (t *Tracker) link(by string, a LinkAsk) (*Linked, error) { return fmt.Errorf("issue %d already has the %s link to %s", is.Number, kind, target) } is.Links = append(is.Links[:i:i], is.Links[i+1:]...) - touch(is, by, at) + if !placesOnly(kind) { + touch(is, by, at) + } return nil } } @@ -692,7 +694,9 @@ func (t *Tracker) link(by string, a LinkAsk) (*Linked, error) { return fmt.Errorf("issue %d has %d links, the most one issue keeps", is.Number, MostLinks) } is.Links = append(is.Links, Link{Kind: kind, Target: target, By: by, At: at}) - touch(is, by, at) + if !placesOnly(kind) { + touch(is, by, at) + } return nil }) if err != nil { @@ -768,7 +772,7 @@ type Row struct { } // lastMoved is when an issue last changed, as its own record says: the latest of its opening, its closing, its -// notes, its links, its worker's start and last act, its import and what the delivery last said of each fix. +// notes, its links but those that only place it, its worker's start and last act, its import and what the delivery last said of each fix. func lastMoved(is *Issue) time.Time { at := is.Opened later := func(t time.Time) { @@ -783,7 +787,9 @@ func lastMoved(is *Issue) time.Time { later(n.At) } for _, l := range is.Links { - later(l.At) + if !placesOnly(l.Kind) { + later(l.At) + } } if w := is.Worker; w != nil { later(w.Since) @@ -798,6 +804,10 @@ func lastMoved(is *Issue) time.Time { return at } +// placesOnly says a link of this kind only places the issue (its milestone) or names a commit that fixed it: no work +// on it, so it neither moves the issue nor keeps its worker current (issue 1781: the seed linked 188 issues at once). +func placesOnly(kind string) bool { return kind == LinkMilestone || kind == LinkCommit } + // checkLinked is a list's linked filter, kind:target, said the one way a link of that kind is kept. func checkLinked(s string) (kind, target string, err error) { k, t, ok := strings.Cut(strings.TrimSpace(s), ":") From 245143d948ea9d28c67dc6e6b4b0fa5de990d39d Mon Sep 17 00:00:00 2001 From: jochen Date: Mon, 26 Oct 2026 19:55:31 +0100 Subject: [PATCH 6/8] Build modules run at most as many steps as their node's build CPU gives, and say a check's phases Each of mesh-build-go, -node and -image declares cores-per-step (6, 2, 2) and hands it to the SDK puller, which bounds the configured capacity by floor(slice quota cores / cores-per-step), read live; thread limits and the status tools follow the effective capacity (issue 1806's follow-up: three Go checks on shanks' 12-core slice starved past the 30-minute step bound). mesh-build-go says its lint and test phases with the ask id (issue 1890). --- modules/mesh-build-go/cmd/mesh-build-go/asks.go | 6 +++++- .../mesh-build-go/cmd/mesh-build-go/asks_test.go | 16 +++++++++++++--- modules/mesh-build-go/cmd/mesh-build-go/cpu.go | 8 +++++++- modules/mesh-build-go/cmd/mesh-build-go/lint.go | 4 ++++ modules/mesh-build-go/files/settings.json | 3 ++- modules/mesh-build-go/go.mod | 2 +- modules/mesh-build-go/go.sum | 4 ++-- modules/mesh-build-go/module.json | 8 +++++++- .../cmd/mesh-build-image/asks.go | 6 +++++- .../mesh-build-image/cmd/mesh-build-image/cpu.go | 8 +++++++- modules/mesh-build-image/files/settings.json | 3 ++- modules/mesh-build-image/go.mod | 2 +- modules/mesh-build-image/go.sum | 4 ++-- modules/mesh-build-image/module.json | 8 +++++++- .../mesh-build-node/cmd/mesh-build-node/asks.go | 6 +++++- .../cmd/mesh-build-node/asks_test.go | 16 +++++++++++++--- .../mesh-build-node/cmd/mesh-build-node/cpu.go | 8 +++++++- modules/mesh-build-node/files/settings.json | 3 ++- modules/mesh-build-node/go.mod | 2 +- modules/mesh-build-node/go.sum | 4 ++-- modules/mesh-build-node/module.json | 8 +++++++- 21 files changed, 102 insertions(+), 27 deletions(-) diff --git a/modules/mesh-build-go/cmd/mesh-build-go/asks.go b/modules/mesh-build-go/cmd/mesh-build-go/asks.go index f514673c..722d5b48 100644 --- a/modules/mesh-build-go/cmd/mesh-build-go/asks.go +++ b/modules/mesh-build-go/cmd/mesh-build-go/asks.go @@ -84,9 +84,13 @@ func (m *Module) Puller(steps []StepTool, source buildstep.AskSource) *buildstep // The capacity's own change wakes the puller (issue 1887): the runtime's watch of the config value as mesh-config // resolved it; the settings file is read only when that watch cannot start. A test's Capacity is its own. var watch func(string, func(stdio.ProvisionChange) error) error + // And the CPU bounds it (issue 1806's follow-up): the configured capacity is an upper bound, the node's build slice + // gives floor(its quota's cores / the module's cores-per-step) at once, read live; a test judges by its own Capacity. + var cores func() float64 if capacity == nil { capacity = buildstep.NodeCapacity(m.Settings, buildstep.DefaultCapacities, m.Node, m.Say) watch = buildstep.WatchConfigServed + cores = buildstep.NodeCoresPerStep(m.Settings, buildstep.DefaultCoresPerStep[delivery.AskGo], m.Say) } ask := m.askSteps(steps) var load func(string) ([]byte, error) @@ -97,7 +101,7 @@ func (m *Module) Puller(steps []StepTool, source buildstep.AskSource) *buildstep } } return &buildstep.Puller{Source: source, Language: delivery.AskGo, Node: m.Node, Put: m.Steps, PausePut: m.Pauses, Load: load, - Capacity: capacity, WatchConfig: watch, Logf: m.Say, Has: m.Has, + Capacity: capacity, WatchConfig: watch, CoresPerStep: cores, Logf: m.Say, Has: m.Has, Run: func(ctx context.Context, a delivery.StepAsk) (any, error) { if err := m.Check(ctx); err != nil { return nil, err diff --git a/modules/mesh-build-go/cmd/mesh-build-go/asks_test.go b/modules/mesh-build-go/cmd/mesh-build-go/asks_test.go index 91783730..02f18241 100644 --- a/modules/mesh-build-go/cmd/mesh-build-go/asks_test.go +++ b/modules/mesh-build-go/cmd/mesh-build-go/asks_test.go @@ -3,6 +3,7 @@ package main import ( "context" "encoding/json" + "fmt" "os" "slices" "strings" @@ -125,12 +126,21 @@ func TestManifestClaimsTheGoBuildSeatAndReadsItsCapacity(t *testing.T) { raw, _ := os.ReadFile(moduleDir + "/module.json") var doc struct { Config map[string]struct { - Default string `json:"default"` + Default json.RawMessage `json:"default"` } `json:"config"` } _ = json.Unmarshal(raw, &doc) - if doc.Config[buildstep.CapacitySetting].Default != buildstep.DefaultCapacities { - t.Errorf("the capacity's default = %q, want %q", doc.Config[buildstep.CapacitySetting].Default, buildstep.DefaultCapacities) + // Issue 1806's follow-up: the cores a step needs is a setting of its own, in the settings file, with the SDK's default. + if cores := string(doc.Config[buildstep.CoresPerStepSetting].Default); cores != fmt.Sprint(buildstep.DefaultCoresPerStep[delivery.AskGo]) { + t.Errorf("the cores-per-step default = %s, want %g", cores, buildstep.DefaultCoresPerStep[delivery.AskGo]) + } + if got := buildstep.NodeCoresPerStep(moduleDir+"/files/settings.json", 0, nil)(); got != buildstep.DefaultCoresPerStep[delivery.AskGo] { + t.Errorf("the settings template holds cores-per-step %g", got) + } + var capacity string + _ = json.Unmarshal(doc.Config[buildstep.CapacitySetting].Default, &capacity) + if capacity != buildstep.DefaultCapacities { + t.Errorf("the capacity's default = %q, want %q", capacity, buildstep.DefaultCapacities) } settings := resource(t, m, "settings") if settings["path"] != "${dir:state}/settings.json" { diff --git a/modules/mesh-build-go/cmd/mesh-build-go/cpu.go b/modules/mesh-build-go/cmd/mesh-build-go/cpu.go index 24457cb4..c1c00903 100644 --- a/modules/mesh-build-go/cmd/mesh-build-go/cpu.go +++ b/modules/mesh-build-go/cmd/mesh-build-go/cpu.go @@ -15,6 +15,8 @@ import ( func (m *Module) cpuLimits() buildcpu.Limits { slots := 1 switch { + case m.Held != nil: + slots = m.Held.CapacityNow() // the capacity the puller judges by, bounded by the node's build slice case m.Capacity != nil: slots = m.Capacity() case m.Settings != "": @@ -32,5 +34,9 @@ func (m *Module) threadEnv() []string { // cpuStatus is what the status tool says of it: the limits a step is given, and what the kernel holds this runtime // to (its unit and its slice), read from its cgroup rather than assumed. func (m *Module) cpuStatus() map[string]any { - return map[string]any{"limits": m.cpuLimits(), "in-force": buildcpu.ReadInForce()} + out := map[string]any{"limits": m.cpuLimits(), "in-force": buildcpu.ReadInForce()} + if m.Held != nil { + out["capacity"] = m.Held.CapacityDetail() // the capacity judged by and why (issue 1806's follow-up) + } + return out } diff --git a/modules/mesh-build-go/cmd/mesh-build-go/lint.go b/modules/mesh-build-go/cmd/mesh-build-go/lint.go index 7fb41f5f..23527db1 100644 --- a/modules/mesh-build-go/cmd/mesh-build-go/lint.go +++ b/modules/mesh-build-go/cmd/mesh-build-go/lint.go @@ -14,6 +14,7 @@ import ( "strings" "sync" + "git.novox.be/novox/mesh-sdk/go/buildstep" "git.novox.be/novox/mesh-sdk/go/forgekey" ) @@ -132,6 +133,9 @@ func (m *Module) lint(ctx context.Context, run forgekey.Runner, tree, d string, // lintAll lints each directory, up to m.parallel() at once like the level's tests; the failures join in directory order // and the error is the first directory's, so the answer does not depend on which ended first (issue 1930). func (m *Module) lintAll(ctx context.Context, run forgekey.Runner, tree string, dirs []string, env func(d string) []string) ([]Failure, error) { + // The ask's phases say it (issue 1890): lint of the directories, then their tests. + buildstep.Phase(ctx, fmt.Sprintf("unit: lint of %d directories", len(dirs))) + defer buildstep.Phase(ctx, fmt.Sprintf("unit: tests of %d directories", len(dirs))) type answer struct { failed []Failure err error diff --git a/modules/mesh-build-go/files/settings.json b/modules/mesh-build-go/files/settings.json index 3175fe39..054af9a2 100644 --- a/modules/mesh-build-go/files/settings.json +++ b/modules/mesh-build-go/files/settings.json @@ -1,3 +1,4 @@ { - "capacity": "shanks=3,ace=2,g14=0,novox=0" + "capacity": "shanks=3,ace=2,g14=0,novox=0", + "cores-per-step": 6 } diff --git a/modules/mesh-build-go/go.mod b/modules/mesh-build-go/go.mod index 868b22d2..5e2a7a39 100644 --- a/modules/mesh-build-go/go.mod +++ b/modules/mesh-build-go/go.mod @@ -3,7 +3,7 @@ module meshbuildgo go 1.26.0 require ( - git.novox.be/novox/mesh-sdk/go v0.1.14-0.20261026142629-ff81bdf8cc8f + git.novox.be/novox/mesh-sdk/go v0.1.14-0.20261026185254-02f50bd9d642 github.com/distribution/distribution/v3 v3.1.2 github.com/nats-io/nats-server/v2 v2.11.17 github.com/nats-io/nats.go v1.54.0 diff --git a/modules/mesh-build-go/go.sum b/modules/mesh-build-go/go.sum index c86aa01f..b4a5f723 100644 --- a/modules/mesh-build-go/go.sum +++ b/modules/mesh-build-go/go.sum @@ -1,5 +1,5 @@ -git.novox.be/novox/mesh-sdk/go v0.1.14-0.20261026142629-ff81bdf8cc8f h1:GYLCvz2x+JNUOTbXzyh2QwzX0sTzvRlXR7lGocWjk3A= -git.novox.be/novox/mesh-sdk/go v0.1.14-0.20261026142629-ff81bdf8cc8f/go.mod h1:ouERNpZizzwZ0HLEtAypsY9FR20FyUgUYRni6J7kVCY= +git.novox.be/novox/mesh-sdk/go v0.1.14-0.20261026185254-02f50bd9d642 h1:mSBxF9KgIJyNL0gOKYq5naWewUj2yazU/tGOh12Tze0= +git.novox.be/novox/mesh-sdk/go v0.1.14-0.20261026185254-02f50bd9d642/go.mod h1:ouERNpZizzwZ0HLEtAypsY9FR20FyUgUYRni6J7kVCY= github.com/alicebob/miniredis/v2 v2.35.0 h1:QwLphYqCEAo1eu1TqPRN2jgVMPBweeQcR21jeqDCONI= github.com/alicebob/miniredis/v2 v2.35.0/go.mod h1:TcL7YfarKPGDAthEtl5NBeHZfeUQj6OXMm/+iu5cLMM= github.com/antithesishq/antithesis-sdk-go v0.7.0-default-no-op h1:Z/MZK75wC/NSrkgqeNIa7jexam9uWzhLmFTSCPI/kn0= diff --git a/modules/mesh-build-go/module.json b/modules/mesh-build-go/module.json index 2f566cbf..9530f128 100644 --- a/modules/mesh-build-go/module.json +++ b/modules/mesh-build-go/module.json @@ -29,6 +29,12 @@ "description": "How many asks of the Go build seat this module runs at once on each node, as node=asks pairs separated by commas, such as shanks=3,ace=2 (novox/hq ADR 0525 §5); a node set to 0 pulls nothing, a node not named runs one at a time. The control-node and the laptop build nothing.", "pattern": "|[a-z0-9_-]+=[0-9]+(,[a-z0-9_-]+=[0-9]+)*", "default": "shanks=3,ace=2,g14=0,novox=0" + }, + "cores-per-step": { + "type": "number", + "description": "How many processors one Go step of this module needs (a full check of a large repository takes what it is given: three at about 4 cores each ran past the 30-minute step bound on 2026-10-26). The capacity above is an upper bound: on each node the module runs at most floor(the cores of its build slice's CPU quota / this) steps at once, the quota read live from its cgroup (issue 1806), so a lowered build CPU share never starves its steps. Above 0.", + "minimum": 0.1, + "default": 6 } }, "tools": [ @@ -107,7 +113,7 @@ "path": "${dir:state}/settings.json", "mode": "0600", "owner": "mesh-build-go", - "takes-config": ["capacity"], + "takes-config": ["capacity", "cores-per-step"], "merge": "json", "template": "files/settings.json" }, diff --git a/modules/mesh-build-image/cmd/mesh-build-image/asks.go b/modules/mesh-build-image/cmd/mesh-build-image/asks.go index 45e03207..3775fc7b 100644 --- a/modules/mesh-build-image/cmd/mesh-build-image/asks.go +++ b/modules/mesh-build-image/cmd/mesh-build-image/asks.go @@ -40,9 +40,13 @@ func (m *Module) Puller(steps []StepTool, source buildstep.AskSource) *buildstep // The capacity's own change wakes the puller (issue 1887): the runtime's watch of the config value as mesh-config // resolved it; the settings file is read only when that watch cannot start. A test's Capacity is its own. var watch func(string, func(stdio.ProvisionChange) error) error + // And the CPU bounds it (issue 1806's follow-up): the configured capacity is an upper bound, the node's build slice + // gives floor(its quota's cores / the module's cores-per-step) at once, read live; a test judges by its own Capacity. + var cores func() float64 if capacity == nil { capacity = buildstep.NodeCapacity(m.Settings, buildstep.DefaultCapacities, m.Node, m.Say) watch = buildstep.WatchConfigServed + cores = buildstep.NodeCoresPerStep(m.Settings, buildstep.DefaultCoresPerStep[delivery.AskImage], m.Say) } ask := m.askSteps(steps) var load func(string) ([]byte, error) @@ -50,7 +54,7 @@ func (m *Module) Puller(steps []StepTool, source buildstep.AskSource) *buildstep load = func(key string) ([]byte, error) { return m.ReadPauses.Get(key) } } return &buildstep.Puller{Source: source, Language: delivery.AskImage, Node: m.Node, Put: m.Steps, PausePut: m.Pauses, Load: load, - Capacity: capacity, WatchConfig: watch, Logf: m.Say, + Capacity: capacity, WatchConfig: watch, CoresPerStep: cores, Logf: m.Say, Run: func(ctx context.Context, a delivery.StepAsk) (any, error) { if err := m.Check(ctx); err != nil { return nil, err diff --git a/modules/mesh-build-image/cmd/mesh-build-image/cpu.go b/modules/mesh-build-image/cmd/mesh-build-image/cpu.go index 126b319d..91f1d314 100644 --- a/modules/mesh-build-image/cmd/mesh-build-image/cpu.go +++ b/modules/mesh-build-image/cmd/mesh-build-image/cpu.go @@ -15,6 +15,8 @@ import ( func (m *Module) cpuLimits() buildcpu.Limits { slots := 1 switch { + case m.Held != nil: + slots = m.Held.CapacityNow() // the capacity the puller judges by, bounded by the node's build slice case m.Capacity != nil: slots = m.Capacity() case m.Settings != "": @@ -32,5 +34,9 @@ func (m *Module) threadEnv() []string { // cpuStatus is what the status tool says of it: the limits a step is given, and what the kernel holds this runtime // to (its unit and its slice), read from its cgroup rather than assumed. func (m *Module) cpuStatus() map[string]any { - return map[string]any{"limits": m.cpuLimits(), "in-force": buildcpu.ReadInForce()} + out := map[string]any{"limits": m.cpuLimits(), "in-force": buildcpu.ReadInForce()} + if m.Held != nil { + out["capacity"] = m.Held.CapacityDetail() // the capacity judged by and why (issue 1806's follow-up) + } + return out } diff --git a/modules/mesh-build-image/files/settings.json b/modules/mesh-build-image/files/settings.json index 3175fe39..9acf4e20 100644 --- a/modules/mesh-build-image/files/settings.json +++ b/modules/mesh-build-image/files/settings.json @@ -1,3 +1,4 @@ { - "capacity": "shanks=3,ace=2,g14=0,novox=0" + "capacity": "shanks=3,ace=2,g14=0,novox=0", + "cores-per-step": 2 } diff --git a/modules/mesh-build-image/go.mod b/modules/mesh-build-image/go.mod index 52658707..0e530690 100644 --- a/modules/mesh-build-image/go.mod +++ b/modules/mesh-build-image/go.mod @@ -3,7 +3,7 @@ module meshbuildimage go 1.26.0 require ( - git.novox.be/novox/mesh-sdk/go v0.1.14-0.20261026142629-ff81bdf8cc8f + git.novox.be/novox/mesh-sdk/go v0.1.14-0.20261026185254-02f50bd9d642 github.com/nats-io/nats-server/v2 v2.11.17 github.com/nats-io/nats.go v1.54.0 ) diff --git a/modules/mesh-build-image/go.sum b/modules/mesh-build-image/go.sum index ba65afa3..9e114f39 100644 --- a/modules/mesh-build-image/go.sum +++ b/modules/mesh-build-image/go.sum @@ -1,5 +1,5 @@ -git.novox.be/novox/mesh-sdk/go v0.1.14-0.20261026142629-ff81bdf8cc8f h1:GYLCvz2x+JNUOTbXzyh2QwzX0sTzvRlXR7lGocWjk3A= -git.novox.be/novox/mesh-sdk/go v0.1.14-0.20261026142629-ff81bdf8cc8f/go.mod h1:ouERNpZizzwZ0HLEtAypsY9FR20FyUgUYRni6J7kVCY= +git.novox.be/novox/mesh-sdk/go v0.1.14-0.20261026185254-02f50bd9d642 h1:mSBxF9KgIJyNL0gOKYq5naWewUj2yazU/tGOh12Tze0= +git.novox.be/novox/mesh-sdk/go v0.1.14-0.20261026185254-02f50bd9d642/go.mod h1:ouERNpZizzwZ0HLEtAypsY9FR20FyUgUYRni6J7kVCY= github.com/antithesishq/antithesis-sdk-go v0.7.0-default-no-op h1:Z/MZK75wC/NSrkgqeNIa7jexam9uWzhLmFTSCPI/kn0= github.com/antithesishq/antithesis-sdk-go v0.7.0-default-no-op/go.mod h1:FQyySiasQQM8735Ddel3MRojmy4dA1IqCeyJ5jmPMbI= github.com/google/go-tpm v0.9.8 h1:slArAR9Ft+1ybZu0lBwpSmpwhRXaa85hWtMinMyRAWo= diff --git a/modules/mesh-build-image/module.json b/modules/mesh-build-image/module.json index ae9c5162..3fe1377f 100644 --- a/modules/mesh-build-image/module.json +++ b/modules/mesh-build-image/module.json @@ -24,6 +24,12 @@ "description": "How many asks of the image build seat this module runs at once on each node, as node=asks pairs separated by commas, such as shanks=3,ace=2 (novox/hq ADR 0525 §5); a node set to 0 pulls nothing, a node not named runs one at a time. The control-node and the laptop build nothing.", "pattern": "|[a-z0-9_-]+=[0-9]+(,[a-z0-9_-]+=[0-9]+)*", "default": "shanks=3,ace=2,g14=0,novox=0" + }, + "cores-per-step": { + "type": "number", + "description": "How many processors one image step of this module needs (buildah, whose RUN lines are mostly fetches). The capacity above is an upper bound: on each node the module runs at most floor(the cores of its build slice's CPU quota / this) steps at once, the quota read live from its cgroup (issue 1806), so a lowered build CPU share never starves its steps. Above 0.", + "minimum": 0.1, + "default": 2 } }, "tools": [ @@ -78,7 +84,7 @@ "path": "${dir:state}/settings.json", "mode": "0600", "owner": "mesh-build-image", - "takes-config": ["capacity"], + "takes-config": ["capacity", "cores-per-step"], "merge": "json", "template": "files/settings.json" }, diff --git a/modules/mesh-build-node/cmd/mesh-build-node/asks.go b/modules/mesh-build-node/cmd/mesh-build-node/asks.go index 0e1b845d..25bff14f 100644 --- a/modules/mesh-build-node/cmd/mesh-build-node/asks.go +++ b/modules/mesh-build-node/cmd/mesh-build-node/asks.go @@ -48,9 +48,13 @@ func (m *Module) Puller(steps []StepTool, source buildstep.AskSource) *buildstep // The capacity's own change wakes the puller (issue 1887): the runtime's watch of the config value as mesh-config // resolved it; the settings file is read only when that watch cannot start. A test's Capacity is its own. var watch func(string, func(stdio.ProvisionChange) error) error + // And the CPU bounds it (issue 1806's follow-up): the configured capacity is an upper bound, the node's build slice + // gives floor(its quota's cores / the module's cores-per-step) at once, read live; a test judges by its own Capacity. + var cores func() float64 if capacity == nil { capacity = buildstep.NodeCapacity(m.Settings, buildstep.DefaultCapacities, m.Node, m.Say) watch = buildstep.WatchConfigServed + cores = buildstep.NodeCoresPerStep(m.Settings, buildstep.DefaultCoresPerStep[delivery.AskNode], m.Say) } ask := m.askSteps(steps) var load func(string) ([]byte, error) @@ -58,7 +62,7 @@ func (m *Module) Puller(steps []StepTool, source buildstep.AskSource) *buildstep load = func(key string) ([]byte, error) { return m.ReadPauses.Get(key) } } return &buildstep.Puller{Source: source, Language: delivery.AskNode, Node: m.Node, Put: m.Steps, PausePut: m.Pauses, Load: load, - Capacity: capacity, WatchConfig: watch, Logf: m.Say, + Capacity: capacity, WatchConfig: watch, CoresPerStep: cores, Logf: m.Say, Run: func(ctx context.Context, a delivery.StepAsk) (any, error) { if err := m.Prepare(ctx); err != nil { return nil, err diff --git a/modules/mesh-build-node/cmd/mesh-build-node/asks_test.go b/modules/mesh-build-node/cmd/mesh-build-node/asks_test.go index 1985c863..dcecf188 100644 --- a/modules/mesh-build-node/cmd/mesh-build-node/asks_test.go +++ b/modules/mesh-build-node/cmd/mesh-build-node/asks_test.go @@ -3,6 +3,7 @@ package main import ( "context" "encoding/json" + "fmt" "os" "strings" "testing" @@ -83,12 +84,21 @@ func TestManifestTakesTheBuildCapacity(t *testing.T) { raw, _ := os.ReadFile(moduleDir + "/module.json") var doc struct { Config map[string]struct { - Default string `json:"default"` + Default json.RawMessage `json:"default"` } `json:"config"` } _ = json.Unmarshal(raw, &doc) - if doc.Config[buildstep.CapacitySetting].Default != buildstep.DefaultCapacities { - t.Errorf("the capacity's default = %q", doc.Config[buildstep.CapacitySetting].Default) + // Issue 1806's follow-up: the cores a step needs is a setting of its own, in the settings file, with the SDK's default. + if cores := string(doc.Config[buildstep.CoresPerStepSetting].Default); cores != fmt.Sprint(buildstep.DefaultCoresPerStep[delivery.AskNode]) { + t.Errorf("the cores-per-step default = %s, want %g", cores, buildstep.DefaultCoresPerStep[delivery.AskNode]) + } + if got := buildstep.NodeCoresPerStep(moduleDir+"/files/settings.json", 0, nil)(); got != buildstep.DefaultCoresPerStep[delivery.AskNode] { + t.Errorf("the settings template holds cores-per-step %g", got) + } + var capacity string + _ = json.Unmarshal(doc.Config[buildstep.CapacitySetting].Default, &capacity) + if capacity != buildstep.DefaultCapacities { + t.Errorf("the capacity's default = %q", capacity) } if resource(t, m, "settings")["path"] != "${dir:state}/settings.json" { t.Error("no settings file") diff --git a/modules/mesh-build-node/cmd/mesh-build-node/cpu.go b/modules/mesh-build-node/cmd/mesh-build-node/cpu.go index b40ceda4..5936b615 100644 --- a/modules/mesh-build-node/cmd/mesh-build-node/cpu.go +++ b/modules/mesh-build-node/cmd/mesh-build-node/cpu.go @@ -15,6 +15,8 @@ import ( func (m *Module) cpuLimits() buildcpu.Limits { slots := 1 switch { + case m.Held != nil: + slots = m.Held.CapacityNow() // the capacity the puller judges by, bounded by the node's build slice case m.Capacity != nil: slots = m.Capacity() case m.Settings != "": @@ -32,5 +34,9 @@ func (m *Module) threadEnv() []string { // cpuStatus is what the status tool says of it: the limits a step is given, and what the kernel holds this runtime // to (its unit and its slice), read from its cgroup rather than assumed. func (m *Module) cpuStatus() map[string]any { - return map[string]any{"limits": m.cpuLimits(), "in-force": buildcpu.ReadInForce()} + out := map[string]any{"limits": m.cpuLimits(), "in-force": buildcpu.ReadInForce()} + if m.Held != nil { + out["capacity"] = m.Held.CapacityDetail() // the capacity judged by and why (issue 1806's follow-up) + } + return out } diff --git a/modules/mesh-build-node/files/settings.json b/modules/mesh-build-node/files/settings.json index 3175fe39..9acf4e20 100644 --- a/modules/mesh-build-node/files/settings.json +++ b/modules/mesh-build-node/files/settings.json @@ -1,3 +1,4 @@ { - "capacity": "shanks=3,ace=2,g14=0,novox=0" + "capacity": "shanks=3,ace=2,g14=0,novox=0", + "cores-per-step": 2 } diff --git a/modules/mesh-build-node/go.mod b/modules/mesh-build-node/go.mod index e79f663b..4f5fd29a 100644 --- a/modules/mesh-build-node/go.mod +++ b/modules/mesh-build-node/go.mod @@ -3,7 +3,7 @@ module meshbuildnode go 1.26.0 require ( - git.novox.be/novox/mesh-sdk/go v0.1.14-0.20261026142629-ff81bdf8cc8f + git.novox.be/novox/mesh-sdk/go v0.1.14-0.20261026185254-02f50bd9d642 github.com/distribution/distribution/v3 v3.1.2 github.com/nats-io/nats.go v1.54.0 ) diff --git a/modules/mesh-build-node/go.sum b/modules/mesh-build-node/go.sum index 28122ac9..8b19ae3a 100644 --- a/modules/mesh-build-node/go.sum +++ b/modules/mesh-build-node/go.sum @@ -1,5 +1,5 @@ -git.novox.be/novox/mesh-sdk/go v0.1.14-0.20261026142629-ff81bdf8cc8f h1:GYLCvz2x+JNUOTbXzyh2QwzX0sTzvRlXR7lGocWjk3A= -git.novox.be/novox/mesh-sdk/go v0.1.14-0.20261026142629-ff81bdf8cc8f/go.mod h1:ouERNpZizzwZ0HLEtAypsY9FR20FyUgUYRni6J7kVCY= +git.novox.be/novox/mesh-sdk/go v0.1.14-0.20261026185254-02f50bd9d642 h1:mSBxF9KgIJyNL0gOKYq5naWewUj2yazU/tGOh12Tze0= +git.novox.be/novox/mesh-sdk/go v0.1.14-0.20261026185254-02f50bd9d642/go.mod h1:ouERNpZizzwZ0HLEtAypsY9FR20FyUgUYRni6J7kVCY= github.com/alicebob/miniredis/v2 v2.35.0 h1:QwLphYqCEAo1eu1TqPRN2jgVMPBweeQcR21jeqDCONI= github.com/alicebob/miniredis/v2 v2.35.0/go.mod h1:TcL7YfarKPGDAthEtl5NBeHZfeUQj6OXMm/+iu5cLMM= github.com/antithesishq/antithesis-sdk-go v0.7.0-default-no-op h1:Z/MZK75wC/NSrkgqeNIa7jexam9uWzhLmFTSCPI/kn0= diff --git a/modules/mesh-build-node/module.json b/modules/mesh-build-node/module.json index 701419b2..970cd694 100644 --- a/modules/mesh-build-node/module.json +++ b/modules/mesh-build-node/module.json @@ -25,6 +25,12 @@ "description": "How many asks of the Node build seat this module runs at once on each node, as node=asks pairs separated by commas, such as shanks=3,ace=2 (novox/hq ADR 0525 §5); a node set to 0 pulls nothing, a node not named runs one at a time. The control-node and the laptop build nothing.", "pattern": "|[a-z0-9_-]+=[0-9]+(,[a-z0-9_-]+=[0-9]+)*", "default": "shanks=3,ace=2,g14=0,novox=0" + }, + "cores-per-step": { + "type": "number", + "description": "How many processors one Node step of this module needs (tsc, esbuild and a test runner; a median step takes 15 s). The capacity above is an upper bound: on each node the module runs at most floor(the cores of its build slice's CPU quota / this) steps at once, the quota read live from its cgroup (issue 1806), so a lowered build CPU share never starves its steps. Above 0.", + "minimum": 0.1, + "default": 2 } }, "tools": [ @@ -100,7 +106,7 @@ "path": "${dir:state}/settings.json", "mode": "0600", "owner": "mesh-build-node", - "takes-config": ["capacity"], + "takes-config": ["capacity", "cores-per-step"], "merge": "json", "template": "files/settings.json" }, From 3f8b0b42480b6eef362f2360ec5d9d0c2e088b7d Mon Sep 17 00:00:00 2001 From: jochen Date: Mon, 26 Oct 2026 21:19:59 +0100 Subject: [PATCH 7/8] Build modules take asks only within their node's build hours mesh-build-go, -node and -image declare build-hours (node=HH:MM-HH:MM in the node's local time, empty = any hour) and hand it to the SDK puller (the operator, 2026-10-26: g14 builds only from 18:00 to 06:00); their SDK pin moves to mesh-sdk d00dac1, which has it. --- modules/mesh-build-go/cmd/mesh-build-go/asks.go | 6 +++++- modules/mesh-build-go/cmd/mesh-build-go/asks_test.go | 7 +++++++ modules/mesh-build-go/files/settings.json | 3 ++- modules/mesh-build-go/go.mod | 2 +- modules/mesh-build-go/go.sum | 4 ++-- modules/mesh-build-go/module.json | 8 +++++++- modules/mesh-build-image/cmd/mesh-build-image/asks.go | 6 +++++- modules/mesh-build-image/files/settings.json | 3 ++- modules/mesh-build-image/go.mod | 2 +- modules/mesh-build-image/go.sum | 4 ++-- modules/mesh-build-image/module.json | 8 +++++++- modules/mesh-build-node/cmd/mesh-build-node/asks.go | 6 +++++- modules/mesh-build-node/cmd/mesh-build-node/asks_test.go | 7 +++++++ modules/mesh-build-node/files/settings.json | 3 ++- modules/mesh-build-node/go.mod | 2 +- modules/mesh-build-node/go.sum | 4 ++-- modules/mesh-build-node/module.json | 8 +++++++- 17 files changed, 65 insertions(+), 18 deletions(-) diff --git a/modules/mesh-build-go/cmd/mesh-build-go/asks.go b/modules/mesh-build-go/cmd/mesh-build-go/asks.go index 722d5b48..816793b2 100644 --- a/modules/mesh-build-go/cmd/mesh-build-go/asks.go +++ b/modules/mesh-build-go/cmd/mesh-build-go/asks.go @@ -86,8 +86,12 @@ func (m *Module) Puller(steps []StepTool, source buildstep.AskSource) *buildstep var watch func(string, func(stdio.ProvisionChange) error) error // And the CPU bounds it (issue 1806's follow-up): the configured capacity is an upper bound, the node's build slice // gives floor(its quota's cores / the module's cores-per-step) at once, read live; a test judges by its own Capacity. + // And the node's build hours (the operator, 2026-10-26: g14 builds only from 18:00 to 06:00): outside them it takes + // nothing new. var cores func() float64 + var hours func() (*buildstep.BuildWindow, string) if capacity == nil { + hours = buildstep.NodeBuildHours(m.Settings, m.Node, m.Say) capacity = buildstep.NodeCapacity(m.Settings, buildstep.DefaultCapacities, m.Node, m.Say) watch = buildstep.WatchConfigServed cores = buildstep.NodeCoresPerStep(m.Settings, buildstep.DefaultCoresPerStep[delivery.AskGo], m.Say) @@ -101,7 +105,7 @@ func (m *Module) Puller(steps []StepTool, source buildstep.AskSource) *buildstep } } return &buildstep.Puller{Source: source, Language: delivery.AskGo, Node: m.Node, Put: m.Steps, PausePut: m.Pauses, Load: load, - Capacity: capacity, WatchConfig: watch, CoresPerStep: cores, Logf: m.Say, Has: m.Has, + Capacity: capacity, WatchConfig: watch, CoresPerStep: cores, BuildHours: hours, Logf: m.Say, Has: m.Has, Run: func(ctx context.Context, a delivery.StepAsk) (any, error) { if err := m.Check(ctx); err != nil { return nil, err diff --git a/modules/mesh-build-go/cmd/mesh-build-go/asks_test.go b/modules/mesh-build-go/cmd/mesh-build-go/asks_test.go index 02f18241..20f6d788 100644 --- a/modules/mesh-build-go/cmd/mesh-build-go/asks_test.go +++ b/modules/mesh-build-go/cmd/mesh-build-go/asks_test.go @@ -137,6 +137,13 @@ func TestManifestClaimsTheGoBuildSeatAndReadsItsCapacity(t *testing.T) { if got := buildstep.NodeCoresPerStep(moduleDir+"/files/settings.json", 0, nil)(); got != buildstep.DefaultCoresPerStep[delivery.AskGo] { t.Errorf("the settings template holds cores-per-step %g", got) } + // And the build hours (the operator, 2026-10-26): a setting of their own, empty by default (any hour), in the template. + if hours := string(doc.Config[buildstep.BuildHoursSetting].Default); hours != `""` { + t.Errorf("the build-hours default = %s, want empty", hours) + } + if w, fault := buildstep.NodeBuildHours(moduleDir+"/files/settings.json", "g14", t.Errorf)(); w != nil || fault != "" { + t.Errorf("the settings template holds build hours %v (%s)", w, fault) + } var capacity string _ = json.Unmarshal(doc.Config[buildstep.CapacitySetting].Default, &capacity) if capacity != buildstep.DefaultCapacities { diff --git a/modules/mesh-build-go/files/settings.json b/modules/mesh-build-go/files/settings.json index 054af9a2..56bb28d4 100644 --- a/modules/mesh-build-go/files/settings.json +++ b/modules/mesh-build-go/files/settings.json @@ -1,4 +1,5 @@ { "capacity": "shanks=3,ace=2,g14=0,novox=0", - "cores-per-step": 6 + "cores-per-step": 6, + "build-hours": "" } diff --git a/modules/mesh-build-go/go.mod b/modules/mesh-build-go/go.mod index 5e2a7a39..24e99464 100644 --- a/modules/mesh-build-go/go.mod +++ b/modules/mesh-build-go/go.mod @@ -3,7 +3,7 @@ module meshbuildgo go 1.26.0 require ( - git.novox.be/novox/mesh-sdk/go v0.1.14-0.20261026185254-02f50bd9d642 + git.novox.be/novox/mesh-sdk/go v0.1.14-0.20261026202137-d00dac140b03 github.com/distribution/distribution/v3 v3.1.2 github.com/nats-io/nats-server/v2 v2.11.17 github.com/nats-io/nats.go v1.54.0 diff --git a/modules/mesh-build-go/go.sum b/modules/mesh-build-go/go.sum index b4a5f723..4fd3d8a8 100644 --- a/modules/mesh-build-go/go.sum +++ b/modules/mesh-build-go/go.sum @@ -1,5 +1,5 @@ -git.novox.be/novox/mesh-sdk/go v0.1.14-0.20261026185254-02f50bd9d642 h1:mSBxF9KgIJyNL0gOKYq5naWewUj2yazU/tGOh12Tze0= -git.novox.be/novox/mesh-sdk/go v0.1.14-0.20261026185254-02f50bd9d642/go.mod h1:ouERNpZizzwZ0HLEtAypsY9FR20FyUgUYRni6J7kVCY= +git.novox.be/novox/mesh-sdk/go v0.1.14-0.20261026202137-d00dac140b03 h1:QIcGyV96AHZ6t/4AgBDlUOcs5tq40PppC7pwEpmzZco= +git.novox.be/novox/mesh-sdk/go v0.1.14-0.20261026202137-d00dac140b03/go.mod h1:ouERNpZizzwZ0HLEtAypsY9FR20FyUgUYRni6J7kVCY= github.com/alicebob/miniredis/v2 v2.35.0 h1:QwLphYqCEAo1eu1TqPRN2jgVMPBweeQcR21jeqDCONI= github.com/alicebob/miniredis/v2 v2.35.0/go.mod h1:TcL7YfarKPGDAthEtl5NBeHZfeUQj6OXMm/+iu5cLMM= github.com/antithesishq/antithesis-sdk-go v0.7.0-default-no-op h1:Z/MZK75wC/NSrkgqeNIa7jexam9uWzhLmFTSCPI/kn0= diff --git a/modules/mesh-build-go/module.json b/modules/mesh-build-go/module.json index 9530f128..7526d1a0 100644 --- a/modules/mesh-build-go/module.json +++ b/modules/mesh-build-go/module.json @@ -30,6 +30,12 @@ "pattern": "|[a-z0-9_-]+=[0-9]+(,[a-z0-9_-]+=[0-9]+)*", "default": "shanks=3,ace=2,g14=0,novox=0" }, + "build-hours": { + "type": "string", + "description": "When each node may take this module's build asks, as node=HH:MM-HH:MM pairs in the node's local time, separated by commas, such as g14=18:00-06:00 (an end before its start crosses midnight). Outside its window a node takes no new step and finishes the ones it runs; a node not named, or an empty value, builds at any hour. The operator, 2026-10-26: g14 builds only outside office hours.", + "pattern": "|[a-z0-9_-]+=([01][0-9]|2[0-3]):[0-5][0-9]-([01][0-9]|2[0-3]):[0-5][0-9](,[a-z0-9_-]+=([01][0-9]|2[0-3]):[0-5][0-9]-([01][0-9]|2[0-3]):[0-5][0-9])*", + "default": "" + }, "cores-per-step": { "type": "number", "description": "How many processors one Go step of this module needs (a full check of a large repository takes what it is given: three at about 4 cores each ran past the 30-minute step bound on 2026-10-26). The capacity above is an upper bound: on each node the module runs at most floor(the cores of its build slice's CPU quota / this) steps at once, the quota read live from its cgroup (issue 1806), so a lowered build CPU share never starves its steps. Above 0.", @@ -113,7 +119,7 @@ "path": "${dir:state}/settings.json", "mode": "0600", "owner": "mesh-build-go", - "takes-config": ["capacity", "cores-per-step"], + "takes-config": ["capacity", "cores-per-step", "build-hours"], "merge": "json", "template": "files/settings.json" }, diff --git a/modules/mesh-build-image/cmd/mesh-build-image/asks.go b/modules/mesh-build-image/cmd/mesh-build-image/asks.go index 3775fc7b..ad9899bf 100644 --- a/modules/mesh-build-image/cmd/mesh-build-image/asks.go +++ b/modules/mesh-build-image/cmd/mesh-build-image/asks.go @@ -42,8 +42,12 @@ func (m *Module) Puller(steps []StepTool, source buildstep.AskSource) *buildstep var watch func(string, func(stdio.ProvisionChange) error) error // And the CPU bounds it (issue 1806's follow-up): the configured capacity is an upper bound, the node's build slice // gives floor(its quota's cores / the module's cores-per-step) at once, read live; a test judges by its own Capacity. + // And the node's build hours (the operator, 2026-10-26: g14 builds only from 18:00 to 06:00): outside them it takes + // nothing new. var cores func() float64 + var hours func() (*buildstep.BuildWindow, string) if capacity == nil { + hours = buildstep.NodeBuildHours(m.Settings, m.Node, m.Say) capacity = buildstep.NodeCapacity(m.Settings, buildstep.DefaultCapacities, m.Node, m.Say) watch = buildstep.WatchConfigServed cores = buildstep.NodeCoresPerStep(m.Settings, buildstep.DefaultCoresPerStep[delivery.AskImage], m.Say) @@ -54,7 +58,7 @@ func (m *Module) Puller(steps []StepTool, source buildstep.AskSource) *buildstep load = func(key string) ([]byte, error) { return m.ReadPauses.Get(key) } } return &buildstep.Puller{Source: source, Language: delivery.AskImage, Node: m.Node, Put: m.Steps, PausePut: m.Pauses, Load: load, - Capacity: capacity, WatchConfig: watch, CoresPerStep: cores, Logf: m.Say, + Capacity: capacity, WatchConfig: watch, CoresPerStep: cores, BuildHours: hours, Logf: m.Say, Run: func(ctx context.Context, a delivery.StepAsk) (any, error) { if err := m.Check(ctx); err != nil { return nil, err diff --git a/modules/mesh-build-image/files/settings.json b/modules/mesh-build-image/files/settings.json index 9acf4e20..73ee4fa8 100644 --- a/modules/mesh-build-image/files/settings.json +++ b/modules/mesh-build-image/files/settings.json @@ -1,4 +1,5 @@ { "capacity": "shanks=3,ace=2,g14=0,novox=0", - "cores-per-step": 2 + "cores-per-step": 2, + "build-hours": "" } diff --git a/modules/mesh-build-image/go.mod b/modules/mesh-build-image/go.mod index 0e530690..e931caa0 100644 --- a/modules/mesh-build-image/go.mod +++ b/modules/mesh-build-image/go.mod @@ -3,7 +3,7 @@ module meshbuildimage go 1.26.0 require ( - git.novox.be/novox/mesh-sdk/go v0.1.14-0.20261026185254-02f50bd9d642 + git.novox.be/novox/mesh-sdk/go v0.1.14-0.20261026202137-d00dac140b03 github.com/nats-io/nats-server/v2 v2.11.17 github.com/nats-io/nats.go v1.54.0 ) diff --git a/modules/mesh-build-image/go.sum b/modules/mesh-build-image/go.sum index 9e114f39..cffc5072 100644 --- a/modules/mesh-build-image/go.sum +++ b/modules/mesh-build-image/go.sum @@ -1,5 +1,5 @@ -git.novox.be/novox/mesh-sdk/go v0.1.14-0.20261026185254-02f50bd9d642 h1:mSBxF9KgIJyNL0gOKYq5naWewUj2yazU/tGOh12Tze0= -git.novox.be/novox/mesh-sdk/go v0.1.14-0.20261026185254-02f50bd9d642/go.mod h1:ouERNpZizzwZ0HLEtAypsY9FR20FyUgUYRni6J7kVCY= +git.novox.be/novox/mesh-sdk/go v0.1.14-0.20261026202137-d00dac140b03 h1:QIcGyV96AHZ6t/4AgBDlUOcs5tq40PppC7pwEpmzZco= +git.novox.be/novox/mesh-sdk/go v0.1.14-0.20261026202137-d00dac140b03/go.mod h1:ouERNpZizzwZ0HLEtAypsY9FR20FyUgUYRni6J7kVCY= github.com/antithesishq/antithesis-sdk-go v0.7.0-default-no-op h1:Z/MZK75wC/NSrkgqeNIa7jexam9uWzhLmFTSCPI/kn0= github.com/antithesishq/antithesis-sdk-go v0.7.0-default-no-op/go.mod h1:FQyySiasQQM8735Ddel3MRojmy4dA1IqCeyJ5jmPMbI= github.com/google/go-tpm v0.9.8 h1:slArAR9Ft+1ybZu0lBwpSmpwhRXaa85hWtMinMyRAWo= diff --git a/modules/mesh-build-image/module.json b/modules/mesh-build-image/module.json index 3fe1377f..c83b9d88 100644 --- a/modules/mesh-build-image/module.json +++ b/modules/mesh-build-image/module.json @@ -25,6 +25,12 @@ "pattern": "|[a-z0-9_-]+=[0-9]+(,[a-z0-9_-]+=[0-9]+)*", "default": "shanks=3,ace=2,g14=0,novox=0" }, + "build-hours": { + "type": "string", + "description": "When each node may take this module's build asks, as node=HH:MM-HH:MM pairs in the node's local time, separated by commas, such as g14=18:00-06:00 (an end before its start crosses midnight). Outside its window a node takes no new step and finishes the ones it runs; a node not named, or an empty value, builds at any hour. The operator, 2026-10-26: g14 builds only outside office hours.", + "pattern": "|[a-z0-9_-]+=([01][0-9]|2[0-3]):[0-5][0-9]-([01][0-9]|2[0-3]):[0-5][0-9](,[a-z0-9_-]+=([01][0-9]|2[0-3]):[0-5][0-9]-([01][0-9]|2[0-3]):[0-5][0-9])*", + "default": "" + }, "cores-per-step": { "type": "number", "description": "How many processors one image step of this module needs (buildah, whose RUN lines are mostly fetches). The capacity above is an upper bound: on each node the module runs at most floor(the cores of its build slice's CPU quota / this) steps at once, the quota read live from its cgroup (issue 1806), so a lowered build CPU share never starves its steps. Above 0.", @@ -84,7 +90,7 @@ "path": "${dir:state}/settings.json", "mode": "0600", "owner": "mesh-build-image", - "takes-config": ["capacity", "cores-per-step"], + "takes-config": ["capacity", "cores-per-step", "build-hours"], "merge": "json", "template": "files/settings.json" }, diff --git a/modules/mesh-build-node/cmd/mesh-build-node/asks.go b/modules/mesh-build-node/cmd/mesh-build-node/asks.go index 25bff14f..f2353133 100644 --- a/modules/mesh-build-node/cmd/mesh-build-node/asks.go +++ b/modules/mesh-build-node/cmd/mesh-build-node/asks.go @@ -50,8 +50,12 @@ func (m *Module) Puller(steps []StepTool, source buildstep.AskSource) *buildstep var watch func(string, func(stdio.ProvisionChange) error) error // And the CPU bounds it (issue 1806's follow-up): the configured capacity is an upper bound, the node's build slice // gives floor(its quota's cores / the module's cores-per-step) at once, read live; a test judges by its own Capacity. + // And the node's build hours (the operator, 2026-10-26: g14 builds only from 18:00 to 06:00): outside them it takes + // nothing new. var cores func() float64 + var hours func() (*buildstep.BuildWindow, string) if capacity == nil { + hours = buildstep.NodeBuildHours(m.Settings, m.Node, m.Say) capacity = buildstep.NodeCapacity(m.Settings, buildstep.DefaultCapacities, m.Node, m.Say) watch = buildstep.WatchConfigServed cores = buildstep.NodeCoresPerStep(m.Settings, buildstep.DefaultCoresPerStep[delivery.AskNode], m.Say) @@ -62,7 +66,7 @@ func (m *Module) Puller(steps []StepTool, source buildstep.AskSource) *buildstep load = func(key string) ([]byte, error) { return m.ReadPauses.Get(key) } } return &buildstep.Puller{Source: source, Language: delivery.AskNode, Node: m.Node, Put: m.Steps, PausePut: m.Pauses, Load: load, - Capacity: capacity, WatchConfig: watch, CoresPerStep: cores, Logf: m.Say, + Capacity: capacity, WatchConfig: watch, CoresPerStep: cores, BuildHours: hours, Logf: m.Say, Run: func(ctx context.Context, a delivery.StepAsk) (any, error) { if err := m.Prepare(ctx); err != nil { return nil, err diff --git a/modules/mesh-build-node/cmd/mesh-build-node/asks_test.go b/modules/mesh-build-node/cmd/mesh-build-node/asks_test.go index dcecf188..8ae9a43d 100644 --- a/modules/mesh-build-node/cmd/mesh-build-node/asks_test.go +++ b/modules/mesh-build-node/cmd/mesh-build-node/asks_test.go @@ -95,6 +95,13 @@ func TestManifestTakesTheBuildCapacity(t *testing.T) { if got := buildstep.NodeCoresPerStep(moduleDir+"/files/settings.json", 0, nil)(); got != buildstep.DefaultCoresPerStep[delivery.AskNode] { t.Errorf("the settings template holds cores-per-step %g", got) } + // And the build hours (the operator, 2026-10-26): a setting of their own, empty by default (any hour), in the template. + if hours := string(doc.Config[buildstep.BuildHoursSetting].Default); hours != `""` { + t.Errorf("the build-hours default = %s, want empty", hours) + } + if w, fault := buildstep.NodeBuildHours(moduleDir+"/files/settings.json", "g14", t.Errorf)(); w != nil || fault != "" { + t.Errorf("the settings template holds build hours %v (%s)", w, fault) + } var capacity string _ = json.Unmarshal(doc.Config[buildstep.CapacitySetting].Default, &capacity) if capacity != buildstep.DefaultCapacities { diff --git a/modules/mesh-build-node/files/settings.json b/modules/mesh-build-node/files/settings.json index 9acf4e20..73ee4fa8 100644 --- a/modules/mesh-build-node/files/settings.json +++ b/modules/mesh-build-node/files/settings.json @@ -1,4 +1,5 @@ { "capacity": "shanks=3,ace=2,g14=0,novox=0", - "cores-per-step": 2 + "cores-per-step": 2, + "build-hours": "" } diff --git a/modules/mesh-build-node/go.mod b/modules/mesh-build-node/go.mod index 4f5fd29a..55492ff6 100644 --- a/modules/mesh-build-node/go.mod +++ b/modules/mesh-build-node/go.mod @@ -3,7 +3,7 @@ module meshbuildnode go 1.26.0 require ( - git.novox.be/novox/mesh-sdk/go v0.1.14-0.20261026185254-02f50bd9d642 + git.novox.be/novox/mesh-sdk/go v0.1.14-0.20261026202137-d00dac140b03 github.com/distribution/distribution/v3 v3.1.2 github.com/nats-io/nats.go v1.54.0 ) diff --git a/modules/mesh-build-node/go.sum b/modules/mesh-build-node/go.sum index 8b19ae3a..338460ec 100644 --- a/modules/mesh-build-node/go.sum +++ b/modules/mesh-build-node/go.sum @@ -1,5 +1,5 @@ -git.novox.be/novox/mesh-sdk/go v0.1.14-0.20261026185254-02f50bd9d642 h1:mSBxF9KgIJyNL0gOKYq5naWewUj2yazU/tGOh12Tze0= -git.novox.be/novox/mesh-sdk/go v0.1.14-0.20261026185254-02f50bd9d642/go.mod h1:ouERNpZizzwZ0HLEtAypsY9FR20FyUgUYRni6J7kVCY= +git.novox.be/novox/mesh-sdk/go v0.1.14-0.20261026202137-d00dac140b03 h1:QIcGyV96AHZ6t/4AgBDlUOcs5tq40PppC7pwEpmzZco= +git.novox.be/novox/mesh-sdk/go v0.1.14-0.20261026202137-d00dac140b03/go.mod h1:ouERNpZizzwZ0HLEtAypsY9FR20FyUgUYRni6J7kVCY= github.com/alicebob/miniredis/v2 v2.35.0 h1:QwLphYqCEAo1eu1TqPRN2jgVMPBweeQcR21jeqDCONI= github.com/alicebob/miniredis/v2 v2.35.0/go.mod h1:TcL7YfarKPGDAthEtl5NBeHZfeUQj6OXMm/+iu5cLMM= github.com/antithesishq/antithesis-sdk-go v0.7.0-default-no-op h1:Z/MZK75wC/NSrkgqeNIa7jexam9uWzhLmFTSCPI/kn0= diff --git a/modules/mesh-build-node/module.json b/modules/mesh-build-node/module.json index 970cd694..baf8b291 100644 --- a/modules/mesh-build-node/module.json +++ b/modules/mesh-build-node/module.json @@ -26,6 +26,12 @@ "pattern": "|[a-z0-9_-]+=[0-9]+(,[a-z0-9_-]+=[0-9]+)*", "default": "shanks=3,ace=2,g14=0,novox=0" }, + "build-hours": { + "type": "string", + "description": "When each node may take this module's build asks, as node=HH:MM-HH:MM pairs in the node's local time, separated by commas, such as g14=18:00-06:00 (an end before its start crosses midnight). Outside its window a node takes no new step and finishes the ones it runs; a node not named, or an empty value, builds at any hour. The operator, 2026-10-26: g14 builds only outside office hours.", + "pattern": "|[a-z0-9_-]+=([01][0-9]|2[0-3]):[0-5][0-9]-([01][0-9]|2[0-3]):[0-5][0-9](,[a-z0-9_-]+=([01][0-9]|2[0-3]):[0-5][0-9]-([01][0-9]|2[0-3]):[0-5][0-9])*", + "default": "" + }, "cores-per-step": { "type": "number", "description": "How many processors one Node step of this module needs (tsc, esbuild and a test runner; a median step takes 15 s). The capacity above is an upper bound: on each node the module runs at most floor(the cores of its build slice's CPU quota / this) steps at once, the quota read live from its cgroup (issue 1806), so a lowered build CPU share never starves its steps. Above 0.", @@ -106,7 +112,7 @@ "path": "${dir:state}/settings.json", "mode": "0600", "owner": "mesh-build-node", - "takes-config": ["capacity", "cores-per-step"], + "takes-config": ["capacity", "cores-per-step", "build-hours"], "merge": "json", "template": "files/settings.json" }, From dcf32576a88071eadf7f7f13f37f62d7bfc0dc01 Mon Sep 17 00:00:00 2001 From: jochen Date: Mon, 26 Oct 2026 21:25:24 +0100 Subject: [PATCH 8/8] mesh-build-image: build with the module's directory as context, so a Dockerfile in a subdirectory finds its COPY paths (issue 1703) photos' admin-client, client and server Dockerfiles live in subdirectories and copy paths from the module's root (admin-client/..., client-server/serve.mjs), as build-agent built them and as the fingerprint reads the module's whole directory. Taking the Dockerfile's own directory as context failed every such COPY with copier: stat "/admin-client/package.json": no such file or directory, on shanks and on ace. Quick fix (rejected): move photos' Dockerfiles to its repository root; it changes the operator's photo application and leaves the trap for the next module. Thorough fix (chosen): one context for every image, the module's directory; it never leaves that directory, so the isolation of ADR 0487 section 2 holds. --- modules/mesh-build-image/README.md | 5 ++- .../cmd/mesh-build-image/build.go | 18 +++++--- .../cmd/mesh-build-image/build_test.go | 41 ++++++++++++++++++- 3 files changed, 54 insertions(+), 10 deletions(-) diff --git a/modules/mesh-build-image/README.md b/modules/mesh-build-image/README.md index a6c3b62e..e27eaa30 100644 --- a/modules/mesh-build-image/README.md +++ b/modules/mesh-build-image/README.md @@ -24,8 +24,9 @@ The Dockerfile's steps may use the network (ADR 0487 §6); nothing a step can re - `build_image_fetch` fetches a commit over SSH into a workspace (a base commit makes the merge preview, as the other build modules' fetch). Long: answers a step id and ends on its key in `steps`. -- `build_image_build` builds one image artifact: `buildah bud` with the Dockerfile of the module's directory as file - and context, `--target`, and the images `build.on` pins as `--build-arg`. Answers the image's digest. With +- `build_image_build` builds one image artifact: `buildah bud` with the Dockerfile `from` names (in the module's + directory or a subdirectory of it) as file and the module's own directory as context, so a COPY names paths from the + module's root (issue 1703), `--target`, and the images `build.on` pins as `--build-arg`. Answers the image's digest. With `publish: true` (a merged trunk commit only) the module's own process then pushes it to the registry under `/:` (the first 12 hex digits of the digest unless the call gives one), waiting out a registry held still, and the key carries the digest the registry serves and the reference pinned by it. Without diff --git a/modules/mesh-build-image/cmd/mesh-build-image/build.go b/modules/mesh-build-image/cmd/mesh-build-image/build.go index c3e1d0e4..323cf3a5 100644 --- a/modules/mesh-build-image/cmd/mesh-build-image/build.go +++ b/modules/mesh-build-image/cmd/mesh-build-image/build.go @@ -55,18 +55,18 @@ func (m *Module) buildTool() StepTool { } // unsupportedImage says why this module does not build the artifact, or "" when it does. It is the one place that -// knows which image forms are out: ADR 0487 §1 takes the Dockerfile's directory as the build context and the base +// knows which image forms are out: ADR 0487 §1 builds from the module's own directory (the build context) and the base // from the Dockerfile or an image pinned in `build.on`, so a context that is another repository and a base that is // another module's artifact are not built (the lead's decision for R4c-9, which retires them with build-agent at R4g). func unsupportedImage(manifest catalogue.Manifest, a catalogue.Artifact) string { if a.Context != nil { - return fmt.Sprintf("the image %s of %s builds with the repository %s as its context; mesh-build-image takes the Dockerfile's directory as the build context (ADR 0487 §1) and does not build a whole repository", + return fmt.Sprintf("the image %s of %s builds with the repository %s as its context; mesh-build-image takes the module's own directory as the build context (ADR 0487 §1) and does not build another repository", a.Name, manifest.Module, a.Context.Repository) } if manifest.Build != nil { for _, on := range manifest.Build.On { if on.Module != "" { - return fmt.Sprintf("the image %s of %s stands on the artifact %s of the module %s through the build argument %s; mesh-build-image builds only from the Dockerfile's directory and an image pinned in build.on (ADR 0487 §1)", + return fmt.Sprintf("the image %s of %s stands on the artifact %s of the module %s through the build argument %s; mesh-build-image builds only from the module's own directory and an image pinned in build.on (ADR 0487 §1)", a.Name, manifest.Module, on.Artifact, on.Module, on.Arg) } } @@ -183,8 +183,14 @@ func (m *Module) build(ctx context.Context, args map[string]any) (any, error) { return m.push(ctx, workspace, manifest.Module, local, res, tag) } -// dockerfileOf is the Dockerfile an artifact's `from` names and the build context: the directory it lies in. It must -// be a regular file inside the module's own directory (checks/imageplace holds the catalogue to the same). +// dockerfileOf is the Dockerfile an artifact's `from` names and the build context: the module's own directory, the +// one its manifest lies in, whichever subdirectory the Dockerfile sits in. `from` is a path relative to that +// directory, and so is every COPY of the Dockerfile: a recipe in a subdirectory (photos' admin-client/Dockerfile +// copies admin-client/... and client-server/serve.mjs) builds as build-agent built it, and as the fingerprint reads +// the module's whole directory as its build source. Taking the Dockerfile's own directory instead failed every such +// COPY with `copier: stat ... no such file or directory` (issue 1703, 2026-10-26). The context never leaves the module's +// directory, so a module's build reads no other module's files. The Dockerfile must be a regular file inside that +// directory (checks/imageplace holds the catalogue to the same). func dockerfileOf(moduleDir, from string) (dockerfile, contextDir string, err error) { clean := filepath.Clean(filepath.FromSlash(strings.TrimSpace(from))) if from == "" || filepath.IsAbs(clean) || clean == ".." || strings.HasPrefix(clean, ".."+string(filepath.Separator)) { @@ -195,7 +201,7 @@ func dockerfileOf(moduleDir, from string) (dockerfile, contextDir string, err er if err != nil || !info.Mode().IsRegular() { return "", "", fmt.Errorf("%q is not a file in the module's directory", from) } - return dockerfile, filepath.Dir(dockerfile), nil + return dockerfile, moduleDir, nil } // localName names the image in buildah's store: one name for each artifact of a workspace. diff --git a/modules/mesh-build-image/cmd/mesh-build-image/build_test.go b/modules/mesh-build-image/cmd/mesh-build-image/build_test.go index 3bb92b3a..a8c866ac 100644 --- a/modules/mesh-build-image/cmd/mesh-build-image/build_test.go +++ b/modules/mesh-build-image/cmd/mesh-build-image/build_test.go @@ -94,7 +94,7 @@ func TestBuildRunsBuildahRootlessAndAnswersTheDigest(t *testing.T) { } } if !strings.HasSuffix(bud[0].line, " "+dir) { - t.Errorf("the context is not the Dockerfile's directory:\n%s", bud[0].line) + t.Errorf("the context is not the module's directory:\n%s", bud[0].line) } for _, c := range f.calls { if c.name != "buildah" { @@ -122,6 +122,43 @@ func TestBuildRunsBuildahRootlessAndAnswersTheDigest(t *testing.T) { } } +// Issue 1703, 2026-10-26: a Dockerfile in a subdirectory of the module builds with the module's directory as its +// context, so its COPY of a path from the module's root (photos' admin-client/Dockerfile copies admin-client/... and +// client-server/serve.mjs) finds it; the Dockerfile's own directory as the context failed with `copier: stat +// "/admin-client/package.json": no such file or directory`. +func TestBuildContextIsTheModulesDirectoryForADockerfileInASubdirectory(t *testing.T) { + m, f, states := setup(t, func(s string) string { + return strings.Replace(s, `"from": "Dockerfile"`, `"from": "admin-client/Dockerfile"`, 1) + }) + dir := filepath.Join(m.Work, "ws1", "tree", "modules", "demo") + for name, body := range map[string]string{ + "admin-client/Dockerfile": "ARG DEMO_BASE\nFROM ${DEMO_BASE} AS final\nCOPY admin-client/package.json client-server/serve.mjs /\n", + "admin-client/package.json": "{}\n", + "client-server/serve.mjs": "\n", + } { + if err := os.MkdirAll(filepath.Dir(filepath.Join(dir, name)), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(dir, name), []byte(body), 0o644); err != nil { + t.Fatal(err) + } + } + _, st := runBuild(t, m, states, nil) + if st.State != delivery.StepPassed { + t.Fatalf("step = %+v", st) + } + bud := f.called("bud") + if len(bud) != 1 { + t.Fatalf("buildah bud ran %d times", len(bud)) + } + if !strings.Contains(bud[0].line, "--file "+dir+"/admin-client/Dockerfile ") { + t.Errorf("the Dockerfile is not the subdirectory's:\n%s", bud[0].line) + } + if !strings.HasSuffix(bud[0].line, " "+dir) { + t.Errorf("the context is not the module's directory, so admin-client/ and client-server/ are out of reach:\n%s", bud[0].line) + } +} + // 1703c, issue 1703: a failing Dockerfile step fails the step, its reason saying buildah's error from stderr, never the // last STEP line buildah writes on stdout; nothing is pushed. func TestBuildFailsWithBuildahsError(t *testing.T) { @@ -178,7 +215,7 @@ func TestBuildRefusesAnImageWithARepositoryContextOrAModuleBase(t *testing.T) { }{ "context": {func(s string) string { return strings.Replace(s, `"target": "final"`, `"target": "final", "context": {"seat": "git", "repository": "novox/mesh-controller"}`, 1) - }, []string{"novox/mesh-controller", "Dockerfile's directory", "ADR 0487"}}, + }, []string{"novox/mesh-controller", "module's own directory", "ADR 0487"}}, "module base": {func(s string) string { return strings.Replace(s, `"image": "docker.io/library/busybox@sha256:0000000000000000000000000000000000000000000000000000000000000001"`, `"module": "mesh-sdk", "artifact": "lib"`, 1) }, []string{"artifact lib of the module mesh-sdk", "DEMO_BASE"}},