diff --git a/modules/gitea/client.ts b/modules/gitea/client.ts index a087d1ec..58cb9469 100644 --- a/modules/gitea/client.ts +++ b/modules/gitea/client.ts @@ -1082,12 +1082,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 @@ -1102,6 +1109,38 @@ export class GiteaAdmin { await this.addCollaborator(owner, repo, user, "write"); } + /** 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); + 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; + } + + /** 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 }) }); diff --git a/modules/gitea/test/tools-account.test.ts b/modules/gitea/test/tools-account.test.ts index 80519429..c69edb77 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,23 @@ 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)] : []); + // 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), keep(await read(req))); + return json(res, 200, userRules.get(key)); + } + 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(?:\/(.*))?$/); if (m) { @@ -205,6 +223,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 +233,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 +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), "write"); + 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 } }]); @@ -636,9 +657,37 @@ 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(); + // 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/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"]); - assert.deepEqual(((await unwritableTool.run({})) as any).unwritable, []); + 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; 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), + /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" }); assert.deepEqual(writes.delivered(), ["jschoubben/car-hunter", "novox/hq"]); diff --git a/modules/gitea/writes.ts b/modules/gitea/writes.ts index ae9d41a6..552f7d58 100644 --- a/modules/gitea/writes.ts +++ b/modules/gitea/writes.ts @@ -18,10 +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 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, 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"; @@ -34,8 +42,35 @@ export interface WriteForge { repository(owner: string, repo: string): Promise; repositoryPermission(owner: string, repo: string, user: string): Promise; 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_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[] { + 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`); + 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; +} + +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; @@ -51,8 +86,13 @@ 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). */ + /** Written already (a collaborator of a user's repository), or written through the organisation's team. */ already: string[]; + /** 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. */ @@ -118,7 +158,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: [], 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) { @@ -140,36 +180,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 writes it, and is put on main's merge whitelist when the rule keeps one. + let changed = false; + if (!WRITES.has(permission)) { + try { + await this.forge.addWriteCollaborator(owner, name, this.account); + } catch (err) { + result.refused.push({ repository: full, why: said(err) }); + continue; + } + let after: string; + try { + after = await this.forge.repositoryPermission(owner, name, this.account); + } catch (err) { + result.refused.push({ repository: full, why: `granted, but its permission could not be read back — ${said(err)}` }); + continue; + } + if (!WRITES.has(after)) { + result.refused.push({ repository: full, why: `the grant was answered, but the forge still gives ${this.account} "${after}"` }); + continue; + } + changed = true; } - let after: string; try { - after = await this.forge.repositoryPermission(owner, name, this.account); + 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: `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} writes it, but could not be put on main's merge whitelist — ${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)}`)); @@ -192,10 +251,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; @@ -205,12 +266,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 }; } } @@ -224,14 +294,19 @@ 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) 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) 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 " + 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..816793b2 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,17 @@ 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. + // 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) } ask := m.askSteps(steps) var load func(string) ([]byte, error) @@ -97,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, 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 91783730..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 @@ -3,6 +3,7 @@ package main import ( "context" "encoding/json" + "fmt" "os" "slices" "strings" @@ -125,12 +126,28 @@ 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) + } + // 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 { + 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..56bb28d4 100644 --- a/modules/mesh-build-go/files/settings.json +++ b/modules/mesh-build-go/files/settings.json @@ -1,3 +1,5 @@ { - "capacity": "shanks=3,ace=2,g14=0,novox=0" + "capacity": "shanks=3,ace=2,g14=0,novox=0", + "cores-per-step": 6, + "build-hours": "" } diff --git a/modules/mesh-build-go/go.mod b/modules/mesh-build-go/go.mod index 613a4e54..d6680c79 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.20261026184717-fe11db929397 + git.novox.be/novox/mesh-sdk/go v0.1.14-0.20261026202658-805b4d9a3536 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 43dd8cc6..f30779ec 100644 --- a/modules/mesh-build-go/go.sum +++ b/modules/mesh-build-go/go.sum @@ -1,9 +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.20261026175935-22bc4dc8ef4f h1:LwefZSIEVjhdhgx22kgL+eJFfD9VVGJblYNPUCh1yZQ= -git.novox.be/novox/mesh-sdk/go v0.1.14-0.20261026175935-22bc4dc8ef4f/go.mod h1:ouERNpZizzwZ0HLEtAypsY9FR20FyUgUYRni6J7kVCY= -git.novox.be/novox/mesh-sdk/go v0.1.14-0.20261026184717-fe11db929397 h1:ozuxlh0fOlq1c3mJ4DLNPgQQ+AIaqd6nWd5CQaEe/r4= -git.novox.be/novox/mesh-sdk/go v0.1.14-0.20261026184717-fe11db929397/go.mod h1:ouERNpZizzwZ0HLEtAypsY9FR20FyUgUYRni6J7kVCY= +git.novox.be/novox/mesh-sdk/go v0.1.14-0.20261026202658-805b4d9a3536 h1:+SoDdDXF7Tgj38WP4kRpUSps6DJVl6P+w5u/KfdtwnQ= +git.novox.be/novox/mesh-sdk/go v0.1.14-0.20261026202658-805b4d9a3536/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..7526d1a0 100644 --- a/modules/mesh-build-go/module.json +++ b/modules/mesh-build-go/module.json @@ -29,6 +29,18 @@ "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" + }, + "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.", + "minimum": 0.1, + "default": 6 } }, "tools": [ @@ -107,7 +119,7 @@ "path": "${dir:state}/settings.json", "mode": "0600", "owner": "mesh-build-go", - "takes-config": ["capacity"], + "takes-config": ["capacity", "cores-per-step", "build-hours"], "merge": "json", "template": "files/settings.json" }, 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/asks.go b/modules/mesh-build-image/cmd/mesh-build-image/asks.go index 45e03207..ad9899bf 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,17 @@ 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. + // 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) } ask := m.askSteps(steps) var load func(string) ([]byte, error) @@ -50,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, 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/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"}}, 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..73ee4fa8 100644 --- a/modules/mesh-build-image/files/settings.json +++ b/modules/mesh-build-image/files/settings.json @@ -1,3 +1,5 @@ { - "capacity": "shanks=3,ace=2,g14=0,novox=0" + "capacity": "shanks=3,ace=2,g14=0,novox=0", + "cores-per-step": 2, + "build-hours": "" } diff --git a/modules/mesh-build-image/go.mod b/modules/mesh-build-image/go.mod index ef648e05..aa7efa34 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.20261026184717-fe11db929397 + git.novox.be/novox/mesh-sdk/go v0.1.14-0.20261026202658-805b4d9a3536 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 d8451089..c6d0c7bf 100644 --- a/modules/mesh-build-image/go.sum +++ b/modules/mesh-build-image/go.sum @@ -1,9 +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.20261026175935-22bc4dc8ef4f h1:LwefZSIEVjhdhgx22kgL+eJFfD9VVGJblYNPUCh1yZQ= -git.novox.be/novox/mesh-sdk/go v0.1.14-0.20261026175935-22bc4dc8ef4f/go.mod h1:ouERNpZizzwZ0HLEtAypsY9FR20FyUgUYRni6J7kVCY= -git.novox.be/novox/mesh-sdk/go v0.1.14-0.20261026184717-fe11db929397 h1:ozuxlh0fOlq1c3mJ4DLNPgQQ+AIaqd6nWd5CQaEe/r4= -git.novox.be/novox/mesh-sdk/go v0.1.14-0.20261026184717-fe11db929397/go.mod h1:ouERNpZizzwZ0HLEtAypsY9FR20FyUgUYRni6J7kVCY= +git.novox.be/novox/mesh-sdk/go v0.1.14-0.20261026202658-805b4d9a3536 h1:+SoDdDXF7Tgj38WP4kRpUSps6DJVl6P+w5u/KfdtwnQ= +git.novox.be/novox/mesh-sdk/go v0.1.14-0.20261026202658-805b4d9a3536/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..c83b9d88 100644 --- a/modules/mesh-build-image/module.json +++ b/modules/mesh-build-image/module.json @@ -24,6 +24,18 @@ "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" + }, + "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.", + "minimum": 0.1, + "default": 2 } }, "tools": [ @@ -78,7 +90,7 @@ "path": "${dir:state}/settings.json", "mode": "0600", "owner": "mesh-build-image", - "takes-config": ["capacity"], + "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 0e1b845d..f2353133 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,17 @@ 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. + // 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) } ask := m.askSteps(steps) var load func(string) ([]byte, error) @@ -58,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, 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 1985c863..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 @@ -3,6 +3,7 @@ package main import ( "context" "encoding/json" + "fmt" "os" "strings" "testing" @@ -83,12 +84,28 @@ 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) + } + // 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 { + 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..73ee4fa8 100644 --- a/modules/mesh-build-node/files/settings.json +++ b/modules/mesh-build-node/files/settings.json @@ -1,3 +1,5 @@ { - "capacity": "shanks=3,ace=2,g14=0,novox=0" + "capacity": "shanks=3,ace=2,g14=0,novox=0", + "cores-per-step": 2, + "build-hours": "" } diff --git a/modules/mesh-build-node/go.mod b/modules/mesh-build-node/go.mod index d3f88ccc..6b89c6fe 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.20261026184717-fe11db929397 + git.novox.be/novox/mesh-sdk/go v0.1.14-0.20261026202658-805b4d9a3536 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 00db2862..51bc0bbb 100644 --- a/modules/mesh-build-node/go.sum +++ b/modules/mesh-build-node/go.sum @@ -1,9 +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.20261026175935-22bc4dc8ef4f h1:LwefZSIEVjhdhgx22kgL+eJFfD9VVGJblYNPUCh1yZQ= -git.novox.be/novox/mesh-sdk/go v0.1.14-0.20261026175935-22bc4dc8ef4f/go.mod h1:ouERNpZizzwZ0HLEtAypsY9FR20FyUgUYRni6J7kVCY= -git.novox.be/novox/mesh-sdk/go v0.1.14-0.20261026184717-fe11db929397 h1:ozuxlh0fOlq1c3mJ4DLNPgQQ+AIaqd6nWd5CQaEe/r4= -git.novox.be/novox/mesh-sdk/go v0.1.14-0.20261026184717-fe11db929397/go.mod h1:ouERNpZizzwZ0HLEtAypsY9FR20FyUgUYRni6J7kVCY= +git.novox.be/novox/mesh-sdk/go v0.1.14-0.20261026202658-805b4d9a3536 h1:+SoDdDXF7Tgj38WP4kRpUSps6DJVl6P+w5u/KfdtwnQ= +git.novox.be/novox/mesh-sdk/go v0.1.14-0.20261026202658-805b4d9a3536/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..baf8b291 100644 --- a/modules/mesh-build-node/module.json +++ b/modules/mesh-build-node/module.json @@ -25,6 +25,18 @@ "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" + }, + "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.", + "minimum": 0.1, + "default": 2 } }, "tools": [ @@ -100,7 +112,7 @@ "path": "${dir:state}/settings.json", "mode": "0600", "owner": "mesh-build-node", - "takes-config": ["capacity"], + "takes-config": ["capacity", "cores-per-step", "build-hours"], "merge": "json", "template": "files/settings.json" }, 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), ":") diff --git a/modules/mesh-knowledge/cmd/mesh-knowledge/progress.go b/modules/mesh-knowledge/cmd/mesh-knowledge/progress.go index ec7b655c..f8882f1d 100644 --- a/modules/mesh-knowledge/cmd/mesh-knowledge/progress.go +++ b/modules/mesh-knowledge/cmd/mesh-knowledge/progress.go @@ -168,6 +168,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 } @@ -835,8 +837,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 0b6cec47..7c26e289 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, "the issue tracker's milestone records of design 47") { 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])