From bff850f7983865b21ed708c40ef7008971209895 Mon Sep 17 00:00:00 2001 From: jochen Date: Wed, 4 Nov 2026 10:45:00 +0100 Subject: [PATCH 1/7] nats: nats_consumer_remove reads a push consumer too (issue 1126) js.Consumer refuses a push consumer ("consumer is not a pull consumer"), so the controller's leftovers on EVENTS and CONTROL could not be read and were not removed. The tool now reads a consumer of either kind (CONSUMER.INFO only, which is all its grant gives), before the delete and for the confirmation after it. Quick fix: this change, in the tool, one function, about an hour with its test rows (a pull and a push consumer). Thorough fix: the same tool also needs its grant to cover the KV_* streams' consumers (mesh-controller-take-in, mesh-controller-config-follow), and that grant is composed by mesh-sdk's BusRemovalGrants from a closed set (ControllerLeftoverConsumersOn), not by the manifest, so it is not in this commit: it needs a change in mesh-sdk and a group delivery. Chosen here: the quick fix for the read, since it is one place and blocks the operator's removal now; the grant is reported to the lead. Claude-Session: https://claude.ai/code/session_01JG73KG7AfSdh6FP8VrBqCn --- modules/nats/cmd/nats-tools/removal.go | 24 +++++++++++++++++--- modules/nats/cmd/nats-tools/removal_test.go | 25 ++++++++++++++++++--- 2 files changed, 43 insertions(+), 6 deletions(-) diff --git a/modules/nats/cmd/nats-tools/removal.go b/modules/nats/cmd/nats-tools/removal.go index fd64382c..21888b0f 100644 --- a/modules/nats/cmd/nats-tools/removal.go +++ b/modules/nats/cmd/nats-tools/removal.go @@ -72,6 +72,25 @@ func step(ctx context.Context) (context.Context, context.CancelFunc) { return context.WithTimeout(ctx, answerWithin) } +// readConsumer reads one consumer of any kind. **A push consumer is read as well as a pull consumer**: js.Consumer +// refuses one ("consumer is not a pull consumer", the controller's leftovers on EVENTS and CONTROL were push consumers), +// so the push reading is tried when the pull reading refuses. Both ask only CONSUMER.INFO of that consumer, which is +// all the grant gives (never STREAM.INFO of a stream the tool may not read). +func readConsumer(ctx context.Context, js jetstream.JetStream, stream, consumer string) (*jetstream.ConsumerInfo, error) { + c, err := js.Consumer(ctx, stream, consumer) + if errors.Is(err, jetstream.ErrNotPullConsumer) { + p, perr := js.PushConsumer(ctx, stream, consumer) + if perr != nil { + return nil, perr + } + return p.CachedInfo(), nil + } + if err != nil { + return nil, err + } + return c.CachedInfo(), nil +} + // ConsumerRemoved is what nats_consumer_remove answers and says. type ConsumerRemoved struct { Stream string `json:"stream"` @@ -98,7 +117,7 @@ func RemoveConsumer(ctx context.Context, bus Bus, said Said, a map[string]any) ( } defer closeBus() sctx, cancel := step(ctx) - c, err := js.Consumer(sctx, stream, consumer) + info, err := readConsumer(sctx, js, stream, consumer) cancel() if err != nil { if errors.Is(err, jetstream.ErrConsumerNotFound) || errors.Is(err, jetstream.ErrStreamNotFound) { @@ -106,7 +125,6 @@ func RemoveConsumer(ctx context.Context, bus Bus, said Said, a map[string]any) ( } return nil, unanswered(err, fmt.Sprintf("the consumer %s of %s could not be read, so it was not removed", consumer, stream)) } - info := c.CachedInfo() was := map[string]any{ "durable": info.Config.Durable, "filter_subject": info.Config.FilterSubject, "filter_subjects": info.Config.FilterSubjects, "deliver_subject": info.Config.DeliverSubject, "pending": info.NumPending, "ack_pending": info.NumAckPending, @@ -119,7 +137,7 @@ func RemoveConsumer(ctx context.Context, bus Bus, said Said, a map[string]any) ( return nil, unanswered(err, fmt.Sprintf("consumer %s of %s was not removed", consumer, stream)) } sctx, cancel = step(ctx) - _, err = js.Consumer(sctx, stream, consumer) + _, err = readConsumer(sctx, js, stream, consumer) cancel() // Only the bus saying it is not there is a removal; a read that failed otherwise confirms nothing. if err == nil { diff --git a/modules/nats/cmd/nats-tools/removal_test.go b/modules/nats/cmd/nats-tools/removal_test.go index 3e211b27..b11f0929 100644 --- a/modules/nats/cmd/nats-tools/removal_test.go +++ b/modules/nats/cmd/nats-tools/removal_test.go @@ -189,8 +189,27 @@ accounts { MESH { jetstream: enabled, users: [ if _, err := RemoveConsumer(ctx, bus, say, map[string]any{"stream": "EVENTS", "consumer": "mesh-grants", "why": "x"}); err == nil { t.Fatal("a consumer of EVENTS outside the grant was removed") } - if _, err := RemoveConsumer(ctx, bus, say, map[string]any{"stream": "EVENTS", "consumer": "controller-merges", "why": "the controller ended"}); err != nil { - t.Fatalf("the controller's consumer of EVENTS: %v", err) + // Issue 1126: the controller's leftovers were push consumers too, which js.Consumer refuses ("not a pull consumer"): + // a pull and a push consumer are read, removed and confirmed alike. + for _, row := range []struct{ name, kind, deliver string }{ + {"controller-merges", "pull", ""}, + {"controller", "push", "deliver.controller"}, + } { + if row.deliver != "" { + if _, err := ajs.CreateConsumer(ctx, "EVENTS", jetstream.ConsumerConfig{Durable: row.name, DeliverSubject: row.deliver, AckPolicy: jetstream.AckExplicitPolicy}); err != nil { + t.Fatal(err) + } + } + got, err := RemoveConsumer(ctx, bus, say, map[string]any{"stream": "EVENTS", "consumer": row.name, "why": "the controller ended"}) + if err != nil { + t.Fatalf("the controller's %s consumer %s of EVENTS: %v", row.kind, row.name, err) + } + if r := got.(ConsumerRemoved); !r.Removed || r.Was["deliver_subject"] != row.deliver { + t.Fatalf("the %s consumer %s: %+v", row.kind, row.name, r) + } + if _, err := ajs.Consumer(ctx, "EVENTS", row.name); err == nil { + t.Fatalf("the %s consumer %s is still there", row.kind, row.name) + } } if _, err := ajs.Consumer(ctx, "EVENTS", "mesh-grants"); err != nil { t.Fatalf("the other consumer of EVENTS went: %v", err) @@ -241,7 +260,7 @@ accounts { MESH { jetstream: enabled, users: [ if k := keys(other); k != "g14.1" { t.Fatalf("another bucket changed: %s", k) } - if strings.Join(said, ",") != "consumer-removed,stream-removed,consumer-removed,stream-removed,bucket-keys-removed" { + if strings.Join(said, ",") != "consumer-removed,stream-removed,consumer-removed,consumer-removed,stream-removed,bucket-keys-removed" { t.Fatalf("the audit events said: %v", said) } } From 052fb473d05cb5c060e390fe9b2180ccbddd528a Mon Sep 17 00:00:00 2001 From: jochen Date: Wed, 4 Nov 2026 11:00:53 +0100 Subject: [PATCH 2/7] gitea: remove the code bound to no event since ADR 0571 clause 13: the verdict setter, the controller's checked and plan types, the build seat's started (issue 1126) Claude-Session: https://claude.ai/code/session_01JG73KG7AfSdh6FP8VrBqCn --- modules/gitea/gate.ts | 51 +--- modules/gitea/index.ts | 125 +--------- modules/gitea/pulls.ts | 284 +---------------------- modules/gitea/test/gate.test.ts | 133 +---------- modules/gitea/test/pulls.test.ts | 155 +------------ modules/gitea/test/siblings.test.ts | 38 --- modules/gitea/test/tools-account.test.ts | 11 - modules/gitea/test/trunk.test.ts | 26 --- 8 files changed, 20 insertions(+), 803 deletions(-) diff --git a/modules/gitea/gate.ts b/modules/gitea/gate.ts index ca10290c..067dea26 100644 --- a/modules/gitea/gate.ts +++ b/modules/gitea/gate.ts @@ -1,8 +1,7 @@ // **One writer of a head's mesh/merge-gate, and the verdict lands last** (novox/hq issue 446). // -// A pending check says where it is: "waiting in the build queue since …" when the head is announced, "building on -// since …" when a build machine takes the check — the build seat's `started` (novox/hq ADR 0157) — and the -// verdict when the controller says `checked`. The forge keeps only the newest status of a context, so a pending word +// A pending check says where it is: "waiting in the build queue since …" when the head is announced, and the verdict when +// it is said. The forge keeps only the newest status of a context, so a pending word // written after the verdict hides it: a pass reads pending and the merge waits for ever; a failure hides under // "building". Two writers cannot order their writes, and a writer that reads the forge before writing races the other // between its read and its write. So this module is the one writer of mesh/merge-gate, and every write of one head's @@ -20,16 +19,6 @@ import { CHECK_CONTEXT, pendingWords } from "./pulls.js"; import type { CommitStatus, HeadCheck } from "./client.js"; -/** The build seat's `started`, as mesh-controller's link.BuildStart publishes it. */ -export interface BuildStarted { - id?: string; - repository?: string; - path?: string; - ref?: string; - on?: string; - at?: string; -} - /** What the gate writer needs of the forge: a status set, and a head's gate as the forge holds it. */ export interface GateForge { setCommitStatus(owner: string, repo: string, sha: string, status: CommitStatus): Promise; @@ -77,15 +66,9 @@ export function openedAt(history: GateStatus[]): number | undefined { /** How far a build machine's clock may stand behind this one's when a start is weighed against the pending words. */ const CLOCK_SLACK_MS = 60_000; -/** How the words of a check a machine is running begin. */ +/** How the words of a check a machine was running began: a status the forge still holds from before is no opening of a check. */ const BUILDING = "building on "; -/** The words of a check a machine is running, in the time the operator reads. */ -export function buildingWords(on: string, at: Date): string { - const two = (n: number) => String(n).padStart(2, "0"); - return `${BUILDING}${on} since ${two(at.getHours())}:${two(at.getMinutes())}`; -} - /** Whether a repository as the mesh clones it (`http://forge/novox/hq.git`) is owner/repo. */ export function clonedAs(url: string | undefined, owner: string, repo: string): boolean { const u = (url ?? "").trim().toLowerCase().replace(/\.git$/, ""); @@ -196,34 +179,6 @@ export class GateWriter { }).then(() => written); } - /** - * A build machine took an ask of this head: said while no verdict was heard, and only for the check now pending — - * a start from before the pending words were set (a run before a recheck, the stream's history) is not this check's. - * A head this process did not announce (it restarted since) is weighed against the forge's own gate: said only while - * that is pending, and not for a start older than it. - */ - started(owner: string, repo: string, sha: string, b: BuildStarted): Promise { - const at = b.at ? new Date(b.at) : new Date(NaN); - if (!b.on || Number.isNaN(at.getTime())) return Promise.resolve(false); - const h = this.head(owner, repo, sha); - let said = false; - return this.said(h, `${owner}/${repo}@${sha.slice(0, 8)}: could not say where its check is`, async () => { - if (h.verdict) return; - let since = h.pendingAt; - if (since === undefined) { - const gate = await this.forge.gateOf(owner, repo, sha); - // No gate on the forge: none was written (its base does not require it, issue 1301), and nothing is said. - if (!gate || gate.state !== "pending") return; - since = gate?.updated_at ? Date.parse(gate.updated_at) : undefined; - } - if (since !== undefined && at.getTime() < since - CLOCK_SLACK_MS) return; - if (h.verdict) return; // heard while the forge was read - await this.forge.setCommitStatus(owner, repo, sha, { state: "pending", context: CHECK_CONTEXT, - description: buildingWords(b.on as string, at) }); - said = true; - }).then(() => said); - } - /** * The verdict: marked at once, so no pending word is written after it, then written in its turn — last. A write * that fails fails the caller, so the event is offered again. diff --git a/modules/gitea/index.ts b/modules/gitea/index.ts index 9271f010..fd4380c6 100644 --- a/modules/gitea/index.ts +++ b/modules/gitea/index.ts @@ -9,13 +9,9 @@ // // module.gitea.pull.closed — a pull request closed unmerged: the delivery it was is stopped (novox/hq ADR 0239) // -// Once consumed mesh-controller.checked — a pull request's merge check, judged — and set it as the head -// commit's statuses (setVerdict, kept below for a revert, and bound to no event since ADR 0571 clause 13): `mesh/merge-gate`, the modules of the mesh's graph the change touches, and -// `mesh/repo-check`, the repository's own merge-check.sh (novox/hq ADR 0237 as amended); with a comment -// saying why when the gate is not a pass or the repository's own check failed. Of the two, only those the base -// still requires are written (issue 1301): main requires `mesh/check` — the checker's — everywhere, hq included since -// ADR 0527 §2, and since R4g no controller judges a merge (its merge gate went with build-agent, ADR 0524 §5), so nothing -// publishes a merge check's verdict, this module consumes no event of the controller, and these two are written for no base. +// It consumes no event of the controller: no controller judges a merge (ADR 0524 §5, ADR 0571 clause 13), and the two older +// statuses, `mesh/merge-gate` and `mesh/repo-check`, are written for no base that main requires (issue 1301); main +// requires `mesh/check`, the checker's, everywhere, hq included since ADR 0527 §2. // // Reads mesh-delivery-checker.owed — what the checker owes the forge, mesh/check on each head it judged — by a watch, and // writes it (check.ts; ADR 0521, R4e finding R2-1). No tool writes mesh/check (finding K1), and no event: the watch hands @@ -53,14 +49,13 @@ import { announce, keepWatermark, readWatermark } from "./announce.js"; import { mergeTool } from "./merge.js"; import { GiteaAdmin, GiteaClient, movedSince } from "./client.js"; import { keepMainRules, mainRulesStep } from "./mainrules.js"; -import { CHECK_CONTEXT, CHECK_STATUS, REPO_CHECK_CONTEXT, commentFor, composedGate, groupGateStatus, headKey, maintained, - requiredOf, statusesFor, writesGate, type Announced, type Checked, repoCheckFacts } from "./pulls.js"; +import { CHECK_CONTEXT, CHECK_STATUS, REPO_CHECK_CONTEXT, headKey, requiredOf, writesGate, type Announced } from "./pulls.js"; import { actOn, announceNewHeads, startPass } from "./watch.js"; import { GateWriter } from "./gate.js"; import { HOOK_PATH, HOOK_PORT, Pace, claimHead, ensureSystemHook, eventsTool, keptSecret, receiver, type ForgeEvent } from "./webhook.js"; -import { keepSiblings, noteComposed, noteVerdict, readSiblings, siblingsFile, siblingsOf, weighSiblings } from "./siblings.js"; +import { keepSiblings, readSiblings, siblingsFile, weighSiblings } from "./siblings.js"; import { bindInBackground, bindUntilHeld } from "./bind.js"; -import { announcementOf, keepTook, lateVerdictComment, outOfDateWords, sameCommit, tookFile, weighTrunks } from "./trunk.js"; +import { announcementOf, weighTrunks } from "./trunk.js"; import { ReadAll, readAllTools } from "./readall.js"; import { DeliveredWrites, INDEXER_MODULES, writeTools, type ModulesChange } from "./writes.js"; import { TOOLS_ACCOUNT, TOOLS_ORG, meshAccounts } from "./tools-account.js"; @@ -356,114 +351,6 @@ async function siblingMoves(client: GiteaClient, repository: string, number?: nu } } -// **The verdict, set where the pull request shows it.** Every verdict is the head commit's status; one -// that is not a pass also leaves the check's own account as a comment, so the reason is read where the -// change is reviewed. An error — the check could not run — is the forge's `error`, never a success. -async function setVerdict(client: GiteaClient, event: { body: unknown }): Promise { - const c = (event.body ?? {}) as Checked; - if (!c.group && c.owner && c.repo && c.took) { - // How long it took, for the merge tool to say how long the next one may (novox/hq ADR 0323 §5). - keepTook(tookFile(process.env.MESH_GITEA_STATE_DIR), `${c.owner}/${c.repo}`, - { took: c.took, at: new Date().toISOString(), head: c.commit }); - } - if (c.group) { - // A delivery group's composed check (novox/hq ADR 0239, ADR 0403 §1): its verdict is each member's merge gate, at the - // head it read; the group's own status, mesh/delivery-group, is its owner's to say. - await setGroupVerdict(client, c); - return; - } - if (!c.owner || !c.repo || !c.commit || !c.verdict) { - console.error("[gitea] a merge check's verdict named no repository, commit or verdict; ignored"); - return; - } - // The siblings this verdict read are kept for the member (ADR 0382 §5); a verdict that read none removes the record. - const siblings = siblingsOf(c.siblings); - if (c.number && noteVerdict(siblingNamed, `${c.owner}/${c.repo}#${c.number}`.toLowerCase(), c.commit, siblings)) { - keepSiblings(siblingsRecord, siblingNamed); - } - // A sibling that moved while the check ran is weighed once this verdict has landed, in the head's own turn after it. - if (siblings.length) { - for (const repository of new Set(siblings.map((s) => s.repository))) { - void Promise.resolve().then(() => siblingMoves(client, repository)); - } - } - // Each status links to the pull request, where the delivery's view is kept (novox/hq ADR 0239). - let target: string | undefined; - let base: string | undefined; - if (c.number) { - const pull = await client.getPullRequest(c.owner, c.repo, c.number).catch(() => undefined); - target = pull?.html_url || undefined; - base = pull?.base || undefined; - } - // **A verdict judged against a trunk commit that is no longer the trunk's head is out of date when it lands** - // (novox/hq ADR 0323 §4): a check asked before the trunk moved. It is said so, pending with the reason, in place of - // its pass or failure; a trunk that cannot be read lets the verdict stand, and the merge tool compares the commits. - const trunk = base ?? c.plan?.base; - // Only the statuses the base still requires are written (issue 1301): on hq, mesh/repo-check alone. - const required = trunk ? requiredOf(await client.appliedProtection(c.owner, c.repo, trunk).catch(() => undefined)) : undefined; - const kept = maintained(required); - if (c.trunk && trunk) { - const head = await client.branchHead(c.owner, c.repo, trunk).catch(() => undefined); - if (head && !sameCommit(c.trunk, head)) { - const description = outOfDateWords(c.trunk, head); - for (const context of [...(composedGate(c) ? [] : [CHECK_CONTEXT]), ...(c["repo-check"] ? [REPO_CHECK_CONTEXT] : [])] - .filter((x) => kept.includes(x))) { - await client.setCommitStatus(c.owner, c.repo, c.commit, { state: "pending", context, description, - ...(target ? { target_url: target } : {}) }); - } - // Its account is kept and shown all the same (ADR 0323 §7): what it judged, and that it is out of date. - const said = commentFor(c, await repoCheckFacts(client, c, trunk), required); - if (c.number && !composedGate(c)) await client.addComment(c.owner, c.repo, c.number, lateVerdictComment(c, said, description)); - console.log(`[gitea] ${c.owner}/${c.repo}#${c.number ?? "?"} at ${c.commit.slice(0, 8)}: merge check ${c.verdict}, ` + - `${description}`); - return; - } - } - const facts = await repoCheckFacts(client, c, base ?? c.plan?.base); - const gateNow = composedGate(c) ? (await client.headChecks(c.owner, c.repo, c.commit).catch(() => ({} as Record)))[CHECK_CONTEXT]?.description : undefined; - for (const status of statusesFor(c, facts, gateNow, required)) { - await client.setCommitStatus(c.owner, c.repo, c.commit, target ? { ...status, target_url: target } : status); - } - const comment = composedGate(c) ? null : commentFor(c, facts, required); - if (comment && c.number) await client.addComment(c.owner, c.repo, c.number, comment); - console.log(`[gitea] ${c.owner}/${c.repo}#${c.number ?? "?"} at ${c.commit.slice(0, 8)}: merge check ${c.verdict}`); -} - -/** **The composed verdict sets each member's gate** (novox/hq ADR 0403 §1, issue 978), in each head's own turn of the gate - * writer (gate.ts), judged against the trunk as a member's own verdict is: one that names a trunk commit that is no longer - * the trunk's head is out of date when it lands, and said so, pending. */ -async function setGroupVerdict(client: GiteaClient, c: Checked): Promise { - // The base branch each member merges into, read with its pull request below: the composed verdict names the siblings of - // each member from it (issue 1049). - const bases = new Map(); - for (const m of c.members ?? []) { - if (!m.owner || !m.repo || !m.commit || !c.verdict) { - console.error(`[gitea] a composed verdict of ${c.group} names a member with no repository, commit or verdict; ignored`); - continue; - } - await gates.verdict(m.owner, m.repo, m.commit, async () => { - const pull = m.number ? await client.getPullRequest(m.owner, m.repo, m.number).catch(() => undefined) : undefined; - const target = pull?.html_url || undefined; - if (m.number && pull?.base) bases.set(`${m.owner}/${m.repo}#${m.number}`, pull.base); - let status = groupGateStatus(c, m); - if (m.trunk && pull?.base) { - const head = await client.branchHead(m.owner, m.repo, pull.base).catch(() => undefined); - if (head && !sameCommit(m.trunk, head)) status = { state: "pending", context: CHECK_CONTEXT, description: outOfDateWords(m.trunk, head) }; - } - await client.setCommitStatus(m.owner, m.repo, m.commit, target ? { ...status, target_url: target } : status); - console.log(`[gitea] ${m.owner}/${m.repo}#${m.number ?? "?"} at ${m.commit.slice(0, 8)}: the group ${c.group}'s composed check ${c.verdict}, ` + - `its merge gate ${status.state}`); - }, c.id); - } - // The siblings of each member are the other members at the heads and base commits this verdict composed (issue 1049): a - // sibling's own merge of such a head is no move of the member's base. Kept for what the merge tool and the siblings' - // weighing read; nothing is run. - if (c.verdict && c.members?.length) { - const members = c.members.map((m) => ({ ...m, base: m.number ? bases.get(`${m.owner}/${m.repo}#${m.number}`) : undefined })); - if (noteComposed(siblingNamed, members)) keepSiblings(siblingsRecord, siblingNamed); - } -} - // **This module is the one writer of a head's mesh/merge-gate** (novox/hq issue 446): the announcement's pending words, // where the check is when a build machine takes it, and the verdict — each head's writes in one chain, the verdict last // (gate.ts). A second writer elsewhere raced the verdict and could leave a pass reading pending. diff --git a/modules/gitea/pulls.ts b/modules/gitea/pulls.ts index 957e43da..16a01183 100644 --- a/modules/gitea/pulls.ts +++ b/modules/gitea/pulls.ts @@ -1,17 +1,15 @@ // A pull request's merge check (novox/hq to-be 45 §9): what the forge's announcer says when a pull -// request's head moves, and what it sets as the pull request's status when the mesh says the verdict. +// request's head moves, and the status contexts the check is kept under. // -// **Before merge, never after.** Every check the mesh had ran after a merge, on a machine: a manifest the -// node-engine refuses (issue 236), an identity a real machine's name made too long (263). So each new head -// of an open pull request is announced as `pull.updated`; the controller asks the build seat to check it -// against every machine of the mesh's facts; and the verdict comes back as the controller's `checked`, -// which this sets as the head commit's status — and, when it is not a pass, as a comment saying why. +// **Before merge, never after.** Each new head of an open pull request is announced as `pull.updated`, and the mesh +// checks it against every machine of its facts before it merges. The verdict is the checker's `mesh/check`, which this +// module writes from the checker's owed state (check.ts); the controller's `checked`, which once set the two older +// statuses, is no event any more (ADR 0571 clause 13). // -// Pure functions here, so they are tested without a forge; index.ts does the asking and the setting. +// Pure functions here, so they are tested without a forge. -import type { CommitStatus, GiteaClient, GiteaPull, GiteaRepo } from "./client.js"; +import type { GiteaPull, GiteaRepo } from "./client.js"; import { movedSince } from "./client.js"; -import { UNKNOWN_MARK, besideWords, siblingsOf } from "./siblings.js"; /** The status context a merge check's gate is kept under: one per commit, the newest replacing the last. */ export const CHECK_CONTEXT = "mesh/merge-gate"; @@ -59,88 +57,6 @@ export function pendingWords(announced: Date): string { return `waiting in the build queue since ${two(announced.getHours())}:${two(announced.getMinutes())}`; } -/** One layer of a check, judged. */ -export interface Layer { - verdict: string; - summary: string; - /** The modules a merge of the change would move, as the controller's planner reckons it… */ - modules?: string[]; - /** …and those it would build after them because they stand on them. */ - dependents?: string[]; -} - -/** What the controller says as `checked` (mesh-controller internal/link, Checked). */ -export interface Checked { - owner: string; - repo: string; - number?: number; - commit: string; - verdict: string; - summary: string; - report?: string; - id: string; - on?: string; - /** The gate — the modules of the mesh's graph the change touches — and the repository's own check. A - * controller from before the layers says neither, and its verdict is the gate's. */ - gate?: Layer; - "repo-check"?: Layer; - /** The change plan of the commit checked (novox/hq ADR 0238): what a merge of it would build and send. */ - plan?: ChangePlan; - /** Set on a delivery group's composed check (novox/hq ADR 0239): its verdict is each member's merge gate (ADR 0403 §1, - * issue 978), set in setGroupVerdict, never read as this head's own. */ - group?: string; - /** The members a composed check read, each at its head and the trunk commit it was merged onto (ADR 0323 §6). */ - members?: { owner: string; repo: string; number?: number; commit: string; trunk?: string }[]; - /** The trunk commit the head was merged into and judged as, twelve characters (novox/hq ADR 0323 §3); absent from a - * controller whose build seat judged the head alone. Beside are the other repositories' commits the check read, - * by directory; took how long it ran. */ - trunk?: string; - beside?: Record; - took?: string; - /** The other members of its delivery group the check read beside the head, each at its head merged onto its base - * commit (novox/hq ADR 0382 §4); absent when it read none, and from a controller from before the rule. */ - siblings?: { repository: string; number: number; head: string; base: string; base_commit: string }[]; - /** Why the siblings could not be learned (mesh-delivery did not answer), when that is so. */ - siblings_unknown?: string; - /** Where the judge came from, when it came from the group (ADR 0382 §3): said in the account. */ - judge_from?: string; -} - -/** A change plan, as the controller says it (mesh-controller internal/link, ChangePlan). */ -export interface ChangePlan { - repository: string; - base: string; - head: string; - moved?: string[]; - dependents?: string[]; - new?: string[]; - unread?: string[]; - tiers?: string[][]; - machines?: { machine: string; receives?: string[]; waits?: string[] }[]; - steps?: string[]; - summary: string; -} - -/** Whether a plan builds anything: a change that touches the mesh's graph. */ -function builds(plan?: ChangePlan): boolean { - return !!plan && ((plan.moved?.length ?? 0) > 0 || (plan.new?.length ?? 0) > 0); -} - -/** A delivery plan (the controller's ChangePlan) as a person reads it on the pull request. */ -export function planText(plan: ChangePlan): string { - const lines = [`**Delivery plan** — ${plan.summary}`]; - (plan.tiers ?? []).forEach((tier, i) => lines.push(`- tier ${i}: ${tier.join(", ")}`)); - for (const m of plan.machines ?? []) { - const parts: string[] = []; - if (m.receives?.length) parts.push(`receives ${m.receives.join(", ")}`); - if (m.waits?.length) parts.push(`waits for a person: ${m.waits.join(", ")}`); - lines.push(`- ${m.machine}: ${parts.join("; ")}`); - } - for (const step of plan.steps ?? []) lines.push(`- ${step}`); - if (plan.unread?.length) lines.push(`- read by no module's build: ${plan.unread.join(", ")}`); - return lines.join("\n"); -} - /** The heads already announced, keyed by `headKey`, so a restart announces nothing twice. */ export type Announced = Record; @@ -176,197 +92,11 @@ export function reposToLook(repos: GiteaRepo[], floor: string, pulls: PullSeen[] return repos.filter((r) => moved.has(r.full_name)); } -// **A required status is success or it blocks** (novox/hq issue 293). The forge combines `warning` as a -// failure, and a branch whose protection requires mesh/merge-gate or mesh/repo-check — with no -// administrator override — will not merge past one. So the controller's `warning`, a note and never a -// question, is set as `success` with the note in its description; only what a person must decide is a -// `failure`, with why. The mapping, verdict by verdict: -// -// pass → success -// warning (a note: rebuild width, a problem -// already so on the base, a script's own) → success, "pass, with a note: …" -// repo-check, no merge-check.sh, not required → success, "no repository check defined" -// repo-check, no merge-check.sh, required (or -// the protection unreadable) → failure: a person adds one, or lifts the requirement -// fail → failure -// error, or no verdict → error — never a success - -/** The forge's state for a verdict: an error is the forge's `error`, never a success; a warning is a note. */ -function stateOf(verdict: string): CommitStatus["state"] { - return verdict === "pass" || verdict === "warning" ? "success" : verdict === "fail" ? "failure" : "error"; -} - -/** How a verdict is said in a status's description: a warning as the pass with a note it is. */ -function saidAs(verdict: string): string { - return verdict === "warning" ? "pass, with a note" : verdict || "error"; -} - -function described(verdict: string, summary: string, modules?: string[], dependents?: string[]): string { - // The forge keeps a short description; the rest is the comment's. - let d = `${saidAs(verdict)}: ${summary}`; - if (modules?.length) d += ` [${modules.join(", ")}${dependents?.length ? ` +${dependents.length} dependent(s)` : ""}]`; - return clipped(d); -} - export function clipped(d: string): string { d = d.replace(/\s+/g, " ").trim(); return d.length > 140 ? d.slice(0, 139) + "…" : d; } -/** What the forge says of a repository's own check beyond the controller's verdict, asked by index.ts only - * when the verdict is a warning: whether the head holds a merge-check.sh, and whether the base branch's - * protection requires mesh/repo-check. Unknown is undefined. */ -export interface RepoCheckFacts { - defined?: boolean; - required?: boolean; -} - -// **What only the forge knows of a repository check said as a warning** (novox/hq issue 293): whether the -// head holds a merge-check.sh at all, and — when it does not — whether the base branch's protection -// requires mesh/repo-check. Asked only for a warning; unknown is left undefined, which statusesFor reads -// as possibly required: a failure on a status nothing requires blocks nothing, a success on one that is -// required would let an untested repository merge. The base's protection is the one the forge applies, read -// from the branch: the rules' own list is a repository administrator's, which the tools' account is not -// (novox/hq ADR 0365). -export async function repoCheckFacts( - forge: Pick, c: Checked, base: string | undefined): Promise { - if (c["repo-check"]?.verdict !== "warning") return {}; - const defined = await forge.holdsFile(c.owner, c.repo, c.commit, "merge-check.sh").catch(() => undefined); - if (defined !== false) return { defined }; - const rule = await forge.appliedProtection(c.owner, c.repo, base || "main").catch(() => undefined); - if (rule === undefined) return { defined }; - const required = !!rule?.enable_status_check && (rule.status_check_contexts ?? []).includes(REPO_CHECK_CONTEXT); - return { defined, required }; -} - -/** The description of a required repository check that is not defined. */ -export const UNDEFINED_REQUIRED = "fail: mesh/repo-check is required here and this head defines no merge-check.sh — " + - "a person adds one, or lifts the requirement from the branch's protection"; - -/** The repository check's status: its verdict, unless the repository defines no check at all — then - * success where the check is not required, and a failure for a person where it is (or may be). */ -function repoStatus(repo: Layer, facts: RepoCheckFacts): CommitStatus { - if (repo.verdict === "warning" && facts.defined === false) { - return facts.required === false - ? { state: "success", context: REPO_CHECK_CONTEXT, description: "no repository check defined" } - : { state: "failure", context: REPO_CHECK_CONTEXT, description: clipped(UNDEFINED_REQUIRED) }; - } - return { state: stateOf(repo.verdict), context: REPO_CHECK_CONTEXT, description: described(repo.verdict, repo.summary) }; -} - -/** Whether the repository check is a failure a person must read: failed, could not run, or required and - * not defined. */ -function repoWrong(repo: Layer | undefined, facts: RepoCheckFacts): boolean { - return !!repo && repoStatus(repo, facts).state !== "success"; -} - /** What a group member's gate says while the group's composed check is the one that judges it: the member's own check can say * nothing more of it, so asking that check again brings no verdict that names a trunk (novox/hq issue 1106). */ export const WAITS_FOR_COMPOSED = "waits for its group's composed check"; - -/** The forge's status for the gate. */ -export function statusFor(c: Checked): CommitStatus { - const gate = c.gate ?? { verdict: c.verdict, summary: c.summary }; - // With a plan, the status says what the change does and how it was judged: "pass: builds gitea → anchor; - // no bus step; every machine composes…". - const description = builds(c.plan) - ? described(gate.verdict, `${c.plan!.summary}; ${gate.summary}`) - : described(gate.verdict, gate.summary, gate.modules, gate.dependents); - return judgedFirst(c, { state: stateOf(gate.verdict), context: CHECK_CONTEXT, description }); -} - -/** - * **A member's gate from its group's composed verdict** (novox/hq ADR 0403 §1, issue 978): the composed check, which - * composed every machine with all the members' heads, sets each member's `mesh/merge-gate` at the head it read. The - * description names what it judged first, as a member's own gate does (ADR 0323 §3), and the group. - */ -export function groupGateStatus(c: Checked, m: NonNullable[number]): CommitStatus { - const said = described(c.verdict, `group ${c.group} composed: ${c.summary}`); - const first = m.trunk ? `head ${m.commit.slice(0, 12)} merged into trunk ${m.trunk.slice(0, 12)} — ` : ""; - return { state: stateOf(c.verdict), context: CHECK_CONTEXT, description: clipped(first + said) }; -} - -/** Whether a verdict's gate layer is the group's composed check's (ADR 0403 §1): no gate was run for the head alone. */ -export function composedGate(c: Checked): boolean { - return c.gate?.verdict === "composed"; -} - -/** Every status a verdict sets: the gate's, and the repository's own check's when it was said; of them, only those the - * base requires (`required`, the base's rule: maintained). */ -export function statusesFor(c: Checked, facts: RepoCheckFacts = {}, gateNow?: string, required?: string[]): CommitStatus[] { - const kept = maintained(required); - return allStatusesFor(c, facts, gateNow).filter((s) => kept.includes(s.context)); -} - -function allStatusesFor(c: Checked, facts: RepoCheckFacts = {}, gateNow?: string): CommitStatus[] { - if (composedGate(c)) { - // A member whose gate the group's composed check judges (ADR 0403 §1): only its repository's check is its own to - // set. The gate stays pending, saying it waits for the group, unless a composed verdict already stands on it. - const out: CommitStatus[] = []; - if (!(gateNow ?? "").includes(" composed: ")) { - out.push({ state: "pending", context: CHECK_CONTEXT, description: WAITS_FOR_COMPOSED }); - } - const repo = c["repo-check"]; - if (repo) out.push(judgedFirst(c, repoStatus(repo, facts))); - return out; - } - const out = [statusFor(c)]; - const repo = c["repo-check"]; - if (repo) out.push(judgedFirst(c, repoStatus(repo, facts))); - return out; -} - -/** **A status names what its verdict judged, first** (novox/hq ADR 0323 §3): "head 1a2b3c4d5e6f merged into trunk - * 7a8b9c0d1e2f — pass: …", so the forge's 140 characters never clip it away; the trunk's move is weighed by it. A - * verdict that names no trunk commit is said as before. */ -function judgedFirst(c: Checked, status: CommitStatus): CommitStatus { - // The siblings it read, named after the commits (ADR 0382 §4), so the clip never takes them either. - const siblings = siblingsOf(c.siblings); - // A verdict that could not learn them says so on the status too (clause 1: it is still checked, and merges). - const mark = siblings.length ? besideWords(siblings) : c.siblings_unknown ? UNKNOWN_MARK : ""; - if (!c.trunk) return mark ? { ...status, description: clipped(`${mark} — ${status.description}`) } : status; - return { ...status, description: clipped(`head ${c.commit.slice(0, 12)} merged into trunk ${c.trunk.slice(0, 12)}${mark ? " " + mark : ""} — ${status.description}`) }; -} - -/** The comment a verdict leaves on its pull request, with the check's own account: the change plan of a - * change that builds something (novox/hq ADR 0238), and why, when the gate is not a pass or the - * repository's own check failed or could not run. A repository with no merge-check.sh, touching nothing, - * is said by its statuses alone, not by a comment on every push. */ -export function commentFor(c: Checked, facts: RepoCheckFacts = {}, required?: string[]): string | null { - const gate = c.gate ?? { verdict: c.verdict, summary: c.summary }; - // A base that no longer requires the gate (issue 1301) is told nothing of it: its verdict is the repository's own check. - const withGate = writesGate(required); - const repo = c["repo-check"]; - const wrong = repoWrong(repo, facts); - if ((!withGate || gate.verdict === "pass") && !wrong && !builds(c.plan)) return null; - const lines = [`**Merge check** at \`${c.commit.slice(0, 8)}\``, ""]; - if (c.trunk) { - // What was judged (novox/hq ADR 0323 §3): the head merged into the trunk commit, and what was read beside it. - const beside = Object.entries(c.beside ?? {}).map(([d, sha]) => `${d} \`${sha}\``).join(", "); - lines.splice(1, 0, `Judged: head \`${c.commit.slice(0, 12)}\` merged into trunk \`${c.trunk.slice(0, 12)}\`` + - (beside ? `; read beside it: ${beside}` : "")); - } - const siblings = siblingsOf(c.siblings); - if (siblings.length) { - lines.splice(c.trunk ? 2 : 1, 0, `Siblings read beside it: ` + siblings.map((s) => - `${s.repository}#${s.number} \`${s.head.slice(0, 12)}\` merged onto \`${s.base_commit.slice(0, 12)}\` (${s.base})`).join(", ") + - (c.judge_from ? `; judged by ${c.judge_from}` : "")); - } else if (c.siblings_unknown) { - lines.splice(c.trunk ? 2 : 1, 0, `Siblings could not be learned: ${c.siblings_unknown}`); - } - if (withGate) { - lines.push(`- \`${CHECK_CONTEXT}\`: **${saidAs(gate.verdict).toUpperCase()}** — ${gate.summary}` + - (gate.modules?.length ? ` (modules: ${gate.modules.join(", ")}` + - (gate.dependents?.length ? `; built after them: ${gate.dependents.join(", ")}` : "") + ")" : "")); - } - if (repo) { - const st = repoStatus(repo, facts); - lines.push(`- \`${REPO_CHECK_CONTEXT}\`: ` + (repo.verdict === "warning" && facts.defined === false - ? `**${st.state === "success" ? "PASS" : "FAIL"}** — ${st.description.replace(/^fail: /, "")}` - : `**${saidAs(repo.verdict).toUpperCase()}** — ${repo.summary}`)); - } - if (builds(c.plan)) lines.push("", planText(c.plan!)); - const ran = c.on ? `\n\nRun by the build seat on ${c.on} as \`${c.id}\` (\`builds --log ${c.id}\`).` : ""; - const report = c.report ? `\n\n\`\`\`\n${c.report.replace(/```/g, "'''")}\n\`\`\`` : ""; - return lines.join("\n") + ran + report; -} diff --git a/modules/gitea/test/gate.test.ts b/modules/gitea/test/gate.test.ts index 82cea9ff..590f1c3e 100644 --- a/modules/gitea/test/gate.test.ts +++ b/modules/gitea/test/gate.test.ts @@ -2,18 +2,11 @@ import assert from "node:assert/strict"; import { test } from "node:test"; // **A pending check says where it is, and the verdict lands last** (novox/hq issue 446). This module is the one writer -// of a head's mesh/merge-gate: the announcement's pending words, "building on since …" when the build seat -// says a machine took the check, and the verdict. The forge keeps the newest status of a context, so a pending word +// of a head's mesh/merge-gate: the announcement's pending words and the verdict. The forge keeps the newest status of a context, so a pending word // written after the verdict hides it — a pass reads pending, a failure hides under "building". const SHA = "a6b08de8507c1703528277e5cefc80548dbc9bb3"; -// The build seat's `started` as a machine said it for novox/hq#309's check on 2026-10-11 (its log, read with the -// controller's `builds --log build-1791680941048622697`, opens at 03:09:01 on shanks); only the forge's address -// replaced. The shape is mesh-controller's link.BuildStart. -const startedOnShanks = { id: "build-1791680941048622697", repository: "http://127.0.0.1:20000/novox/hq.git", - ref: SHA, on: "shanks", at: "2026-10-11T01:09:01.204319Z" }; - /** A forge that keeps the newest status per context, as gitea does, and whose writes take as long as a test says. */ function aForge() { const kept: Record = {}; @@ -71,99 +64,8 @@ function aForge() { /** The clock of the test: the head announced at 01:08:50Z. */ const announcedAt = () => new Date("2026-10-11T01:08:50Z"); -test("a build machine taking the check is said on the head: building on which machine, since the local time", async () => { - const { GateWriter, buildingWords } = await import("../gate.ts"); - const forge = aForge(); - const gates = new GateWriter(forge, announcedAt); - await gates.pending("novox", "hq", SHA); - assert.equal(await gates.started("novox", "hq", SHA, startedOnShanks), true); - assert.equal(forge.kept["mesh/merge-gate"].state, "pending"); - assert.equal(forge.kept["mesh/merge-gate"].description, buildingWords("shanks", new Date(startedOnShanks.at))); - assert.equal(buildingWords("shanks", new Date(2026, 9, 11, 3, 9, 1)), "building on shanks since 03:09"); -}); - -test("the verdict heard while a 'building' write is in flight lands last and stays", async () => { - const { GateWriter } = await import("../gate.ts"); - const forge = aForge(); - const gates = new GateWriter(forge, announcedAt); - await gates.pending("novox", "hq", SHA); - forge.holdWrites(); // the 'building' request is in flight when the verdict comes - const building = gates.started("novox", "hq", SHA, startedOnShanks); - await new Promise((r) => setImmediate(r)); - const verdict = gates.verdict("novox", "hq", SHA, () => - forge.setCommitStatus("novox", "hq", SHA, { state: "success", context: "mesh/merge-gate", description: "pass" })); - forge.release(); - await Promise.all([building, verdict]); - assert.equal(`${forge.kept["mesh/merge-gate"].state} ${forge.kept["mesh/merge-gate"].description}`, "success pass", - `the verdict is not the newest status: ${forge.written.join(" | ")}`); -}); - -// The race the review of mesh-catalog#202 reproduced: a writer reads the forge's gate (pending), the verdict is written, -// then the writer writes "building" — and the pass reads pending until the next push. A head this process did not -// announce is the one case where the forge is read before writing. -test("the verdict written while a start's read of the forge is in flight stays the newest status", async () => { - const { GateWriter } = await import("../gate.ts"); - const forge = aForge(); - forge.kept["mesh/merge-gate"] = { state: "pending", description: "waiting", updated_at: "2026-10-11T01:08:50Z" }; - const gates = new GateWriter(forge, announcedAt); - forge.holdReads(); - const building = gates.started("novox", "hq", SHA, startedOnShanks); - await new Promise((r) => setImmediate(r)); - const verdict = gates.verdict("novox", "hq", SHA, () => - forge.setCommitStatus("novox", "hq", SHA, { state: "success", context: "mesh/merge-gate", description: "pass" })); - await new Promise((r) => setImmediate(r)); - forge.releaseReads(); - await Promise.all([building, verdict]); - assert.equal(`${forge.kept["mesh/merge-gate"].state} ${forge.kept["mesh/merge-gate"].description}`, "success pass", - `the verdict is not the newest status: ${forge.written.join(" | ")}`); -}); - -test("the verdict heard while a 'building' write still waits its turn: the 'building' is never written", async () => { - const { GateWriter } = await import("../gate.ts"); - const forge = aForge(); - const gates = new GateWriter(forge, announcedAt); - forge.holdWrites(); // the pending words are still being written - const pending = gates.pending("novox", "hq", SHA); - const building = gates.started("novox", "hq", SHA, startedOnShanks); - const verdict = gates.verdict("novox", "hq", SHA, () => - forge.setCommitStatus("novox", "hq", SHA, { state: "failure", context: "mesh/merge-gate", description: "fail: 0 of 4 compose" })); - forge.release(); - await Promise.all([pending, verdict]); - assert.equal(await building, false); - assert.equal(forge.kept["mesh/merge-gate"].state, "failure"); - assert.ok(!forge.written.some((w) => w.includes("building")), forge.written.join(" | ")); -}); - -test("a start heard after the verdict writes nothing", async () => { - const { GateWriter } = await import("../gate.ts"); - const forge = aForge(); - const gates = new GateWriter(forge, announcedAt); - await gates.pending("novox", "hq", SHA); - await gates.verdict("novox", "hq", SHA, () => - forge.setCommitStatus("novox", "hq", SHA, { state: "success", context: "mesh/merge-gate", description: "pass" })); - assert.equal(await gates.started("novox", "hq", SHA, startedOnShanks), false); - assert.equal(forge.kept["mesh/merge-gate"].state, "success"); -}); - -test("a start from before the check now pending (a run before a recheck, the stream's history) writes nothing", async () => { - const { GateWriter } = await import("../gate.ts"); - const forge = aForge(); - let now = announcedAt(); - const gates = new GateWriter(forge, () => now); - await gates.pending("novox", "hq", SHA); - await gates.verdict("novox", "hq", SHA, () => - forge.setCommitStatus("novox", "hq", SHA, { state: "failure", context: "mesh/merge-gate", description: "error" })); - now = new Date("2026-10-11T01:30:00Z"); // checked again: the recheck's pending, as the tool writes it (issue 456) - await gates.rechecked("novox", "hq", SHA, { state: "pending", context: "mesh/merge-gate", description: "checking again: flaky" }); - assert.equal(await gates.started("novox", "hq", SHA, startedOnShanks), false, "the first run's start"); - assert.equal(forge.kept["mesh/merge-gate"].description, "checking again: flaky"); - assert.equal(await gates.started("novox", "hq", SHA, { ...startedOnShanks, at: "2026-10-11T01:31:00Z" }), true, - "the second run's start"); -}); - // **A recheck's pending goes through the one writer** (novox/hq issue 456). mesh-delivery's recheck puts the gate back -// to pending through the gitea_commit_status tool; before, that write bypassed the head's chain: the first run's -// verdict mark stayed set, so the new run never said "building", and an old run's verdict could land over the pending. +// to pending through the gitea_commit_status tool. /** The ask of a check, as the controller's link.NewBuildID names it: when it was asked, in unix nanoseconds. */ const askOf = (at: string) => `build-${BigInt(Date.parse(at)) * 1_000_000n}`; @@ -209,20 +111,6 @@ test("an old run's verdict in flight when the recheck comes lands before the rec assert.equal(gateOf(forge), "pending checking again: main moved on", forge.written.join(" | ")); }); -test("a rechecked head says where its new run is: building on which machine", async () => { - const { GateWriter, buildingWords } = await import("../gate.ts"); - const forge = aForge(); - let now = announcedAt(); - const gates = new GateWriter(forge, () => now); - await gates.pending("novox", "hq", SHA); - await gates.verdict("novox", "hq", SHA, writeOf(forge, "success", "pass"), firstRun); - now = recheckAt; - await gates.rechecked("novox", "hq", SHA, checkingAgain); - const at = "2026-10-11T01:30:03Z"; - assert.equal(await gates.started("novox", "hq", SHA, { ...startedOnShanks, id: secondRun, at }), true); - assert.equal(forge.kept["mesh/merge-gate"].description, buildingWords("shanks", new Date(at))); -}); - test("the recheck's own verdict is written, even asked a little before the pending by another machine's clock", async () => { const { GateWriter } = await import("../gate.ts"); const forge = aForge(); @@ -306,16 +194,6 @@ test("when a check was asked is read from its id; the opening of the check now p assert.equal(openedAt([]), undefined); }); -test("a head this process did not announce (it restarted) is said only while the forge holds its gate pending", async () => { - const { GateWriter } = await import("../gate.ts"); - const forge = aForge(); - forge.kept["mesh/merge-gate"] = { state: "success", description: "pass", updated_at: "2026-10-11T01:20:00Z" }; - const gates = new GateWriter(forge, announcedAt); - assert.equal(await gates.started("novox", "hq", SHA, startedOnShanks), false); - forge.kept["mesh/merge-gate"] = { state: "pending", description: "waiting", updated_at: "2026-10-11T01:08:50Z" }; - assert.equal(await gates.started("novox", "hq", SHA, startedOnShanks), true); -}); - test("a repository as the mesh clones it is told from another", async () => { const { clonedAs } = await import("../gate.ts"); assert.equal(clonedAs("http://127.0.0.1:20000/novox/hq.git", "novox", "hq"), true); @@ -325,10 +203,3 @@ test("a repository as the mesh clones it is told from another", async () => { // Issue 1301: a build machine's start on a head that holds no mesh/merge-gate (its base requires none, so none was written) // says nothing: no "building on" status appears on a main that does not require the gate. -test("a start on a head with no gate on the forge writes nothing", async () => { - const { GateWriter } = await import("../gate.ts"); - const forge = aForge(); - const gates = new GateWriter(forge, announcedAt); - assert.equal(await gates.started("novox", "hq", SHA, startedOnShanks), false); - assert.equal(forge.kept["mesh/merge-gate"], undefined); -}); diff --git a/modules/gitea/test/pulls.test.ts b/modules/gitea/test/pulls.test.ts index c548ae78..bbc0daf1 100644 --- a/modules/gitea/test/pulls.test.ts +++ b/modules/gitea/test/pulls.test.ts @@ -29,92 +29,6 @@ test("a pull request retargeted keeps its head and is announced again, into its assert.notEqual(headKey("novox/de-spiegel", 6, "main"), headKey("novox/de-spiegel", 6, "old-trunk")); }); -test("a verdict is the head commit's status; an error is the forge's error, never a success", async () => { - const { statusFor, commentFor, CHECK_CONTEXT } = await import("../pulls.ts"); - const base = { owner: "novox", repo: "mesh-catalog", number: 7, commit: "0123456789abcdef", id: "build-1", on: "laptop" }; - assert.equal(statusFor({ ...base, verdict: "pass", summary: "every machine composes" }).state, "success"); - // A warning is a note, never a blocking state: the forge combines `warning` as a failure (issue 293). - const wide = statusFor({ ...base, verdict: "warning", summary: "a merge rebuilds 14 module(s)" }); - assert.equal(wide.state, "success"); - assert.match(wide.description, /^pass, with a note: a merge rebuilds 14 module/); - assert.equal(statusFor({ ...base, verdict: "fail", summary: "x" }).state, "failure"); - assert.equal(statusFor({ ...base, verdict: "error", summary: "the check could not run" }).state, "error"); - assert.equal(statusFor({ ...base, verdict: "", summary: "" }).state, "error", "no verdict is no pass"); - const long = statusFor({ ...base, verdict: "fail", summary: "y".repeat(500) }); - assert.ok(long.description.length <= 140 && long.context === CHECK_CONTEXT); - assert.equal(commentFor({ ...base, verdict: "pass", summary: "fine" }), null, "a pass leaves no comment"); - const said = commentFor({ ...base, verdict: "fail", summary: "lemurs refused", report: "fails:\n - ```x```" }) ?? ""; - assert.match(said, /mesh\/merge-gate`: \*\*FAIL\*\*/); - assert.match(said, /01234567/); - assert.match(said, /builds --log build-1/); - assert.ok(!said.slice(said.indexOf("```") + 3, said.lastIndexOf("```")).includes("```"), "the report cannot close its own block"); -}); - -test("each layer is its own status: the gate with the modules it judged, the repository's own check beside it", async () => { - const { statusesFor, commentFor, CHECK_CONTEXT, REPO_CHECK_CONTEXT } = await import("../pulls.ts"); - const base = { owner: "novox", repo: "mesh-catalog", number: 7, commit: "0123456789abcdef", id: "build-1" }; - const both = statusesFor({ ...base, verdict: "pass", summary: "every machine composes", - gate: { verdict: "pass", summary: "every machine composes", modules: ["gitea", "keycloak"], dependents: ["node-tools"] }, - "repo-check": { verdict: "fail", summary: "its merge-check.sh failed: FAIL x" } }); - assert.deepEqual(both.map((s) => [s.context, s.state]), [[CHECK_CONTEXT, "success"], [REPO_CHECK_CONTEXT, "failure"]]); - assert.match(both[0].description, /gitea, keycloak \+1 dependent/); - assert.match(commentFor({ ...base, verdict: "pass", summary: "", gate: { verdict: "pass", summary: "" }, - "repo-check": { verdict: "fail", summary: "its merge-check.sh failed" } }) ?? "", /mesh\/repo-check`: \*\*FAIL/); - - // Nothing of the graph touched, no script: a pass and a warning, and no comment on every push. - const quiet = { ...base, verdict: "pass", summary: "the change touches no module of the mesh's graph", - gate: { verdict: "pass", summary: "the change touches no module of the mesh's graph" }, - "repo-check": { verdict: "warning", summary: "the repository declares no merge-check.sh" } }; - assert.deepEqual(statusesFor(quiet, { defined: false, required: false }).map((s) => [s.state, s.description]), - [["success", "pass: the change touches no module of the mesh's graph"], ["success", "no repository check defined"]]); - assert.equal(commentFor(quiet, { defined: false, required: false }), null); - - // A repository outside the mesh, touching nothing: the gate alone, a pass. - assert.equal(statusesFor({ ...base, verdict: "pass", summary: "x", gate: { verdict: "pass", summary: "x" } }).length, 1); - - // A controller from before the layers: its verdict is the gate's. - assert.deepEqual(statusesFor({ ...base, verdict: "warning", summary: "wide" }).map((s) => [s.context, s.state]), - [[CHECK_CONTEXT, "success"]]); -}); - -// **A required status is success or it blocks** (novox/hq issue 293): branch protection requires -// mesh/merge-gate (and mesh/repo-check on the core repositories) with no administrator override, and the -// forge combines `warning` as a failure. No verdict ever sets `warning` on either; a note is a success that -// says it; only what a person must decide is a failure, with why. -test("no verdict sets warning on a required check; only a person's decision fails", async () => { - const { statusesFor, commentFor, UNDEFINED_REQUIRED, REPO_CHECK_CONTEXT } = await import("../pulls.ts"); - const base = { owner: "novox", repo: "photos", number: 3, commit: "0123456789abcdef", id: "build-2" }; - const noScript = { verdict: "warning", summary: "the repository declares no merge-check.sh" }; - const quiet = { ...base, verdict: "pass", summary: "x", gate: { verdict: "pass", summary: "x" }, "repo-check": noScript }; - for (const verdict of ["pass", "warning", "fail", "error", ""]) { - for (const facts of [{}, { defined: true }, { defined: false }, { defined: false, required: true }, { defined: false, required: false }]) { - const c = { ...base, verdict, summary: "s", gate: { verdict, summary: "s" }, "repo-check": { verdict, summary: "s" } }; - for (const s of statusesFor(c, facts)) assert.notEqual(s.state, "warning", `${verdict} ${JSON.stringify(facts)} → ${s.context}`); - } - } - // A repository without a merge-check.sh where repo-check is not required: success, said. - assert.deepEqual(statusesFor(quiet, { defined: false, required: false })[1], - { state: "success", context: REPO_CHECK_CONTEXT, description: "no repository check defined" }); - // Where it is required — or the protection could not be read — a person decides: a failure, with why. - for (const facts of [{ defined: false, required: true }, { defined: false }]) { - const s = statusesFor(quiet, facts)[1]; - assert.equal(s.state, "failure"); - assert.ok(s.description.length <= 140 && UNDEFINED_REQUIRED.startsWith(s.description.replace(/…$/, ""))); - assert.match(s.description, /^fail: mesh\/repo-check is required/); - assert.match(commentFor(quiet, facts) ?? "", /mesh\/repo-check`: \*\*FAIL\*\* — mesh\/repo-check is required/); - } - // A script that defines its check and said a warning itself: a note, a success. - const noted = statusesFor({ ...quiet, "repo-check": { verdict: "warning", summary: "2 tests skipped" } }, { defined: true })[1]; - assert.deepEqual([noted.state, noted.description], ["success", "pass, with a note: 2 tests skipped"]); - // The gate's notes — a wide rebuild, a problem already so on the base — are successes that say so. - const already = statusesFor({ ...base, verdict: "warning", summary: "s", - gate: { verdict: "warning", summary: "the module check's problems were all so on main already" } })[0]; - assert.deepEqual([already.state, already.description], - ["success", "pass, with a note: the module check's problems were all so on main already"]); - assert.match(commentFor({ ...base, verdict: "warning", summary: "wide", gate: { verdict: "warning", summary: "wide" } }) ?? "", - /PASS, WITH A NOTE/); -}); - test("a commit status is set on the commit, under the merge check's context", async () => { const { GiteaClient } = await import("../client.ts"); let seen: { path: string; body: any } | null = null; @@ -140,24 +54,6 @@ test("a commit status is set on the commit, under the merge check's context", as assert.equal(seen!.body.context, "mesh/merge-gate"); }); -test("a change plan is the gate's result: said on the status and, when it builds something, as a comment", async () => { - const { statusFor, commentFor } = await import("../pulls.ts"); - const plan = { - repository: "novox/mesh-catalog", base: "main", head: "0123456789abcdef", moved: ["gitea"], - tiers: [["gitea"]], machines: [{ machine: "anchor", receives: ["gitea"] }], steps: [], - summary: "builds gitea → anchor; no bus step", - }; - const c = { owner: "novox", repo: "mesh-catalog", number: 7, commit: "0123456789abcdef", id: "b", verdict: "pass", - summary: "every machine composes", gate: { verdict: "pass", summary: "every machine composes", modules: ["gitea"] }, plan }; - assert.equal(statusFor(c).description, "pass: builds gitea → anchor; no bus step; every machine composes"); - const said = commentFor(c) ?? ""; - assert.match(said, /Delivery plan\*\* — builds gitea → anchor/); - assert.match(said, /- anchor: receives gitea/); - // A plan that builds nothing, passing: the statuses say it, no comment. - const nothing = { ...c, plan: { ...plan, moved: [], tiers: [], machines: [], summary: "builds nothing" } }; - assert.equal(commentFor(nothing), null); -}); - // **A pending check says where it is** (novox/hq issue 446): announced, it waits in the build queue, since the time the // operator reads — not "mapping this head onto its module graph" for the ten minutes of a controller check. test("a head announced is pending in the build queue, since the local time it was announced", async () => { @@ -166,48 +62,10 @@ test("a head announced is pending in the build queue, since the local time it wa assert.equal(pendingWords(new Date(2026, 9, 11, 14, 0, 0)), "waiting in the build queue since 14:00"); }); -// **A group's composed verdict is each member's merge gate** (novox/hq ADR 0403 §1, issue 978): set at the head the -// composed check read, naming the trunk commit it was merged onto, the group, and what the check said. -test("a composed verdict sets every member's gate at its own head, naming its trunk and the group", async () => { - const { groupGateStatus, CHECK_CONTEXT } = await import("../pulls.ts"); - const c = { owner: "", repo: "", commit: "", id: "build-1", group: "feat/x", verdict: "pass", summary: "both compose", - members: [{ owner: "novox", repo: "mesh-controller", number: 1, commit: "c".repeat(40), trunk: "a".repeat(40) }, - { owner: "novox", repo: "mesh-catalog", number: 2, commit: "d".repeat(40) }] }; - const [first, second] = c.members.map((m) => groupGateStatus(c, m)); - assert.equal(first.context, CHECK_CONTEXT); - assert.equal(first.state, "success"); - assert.match(first.description, /^head cccccccccccc merged into trunk aaaaaaaaaaaa — pass: group feat\/x composed: both compose/); - assert.equal(second.state, "success"); - assert.match(second.description, /^pass: group feat\/x composed: both compose/, "no trunk named, none said"); - // A composed check that failed is every member's failure, never a success. - assert.equal(groupGateStatus({ ...c, verdict: "fail", summary: "a and b do not compose" }, c.members[0]).state, "failure"); - assert.equal(groupGateStatus({ ...c, verdict: "error", summary: "could not run" }, c.members[0]).state, "error"); -}); - -// A member's own verdict whose gate layer is the group's composed check's sets its repository check only, and never the -// gate as an error: the gate stays pending until the composed verdict lands, in either order (ADR 0403 §1, issue 978). -test("a member's own verdict with a composed gate layer leaves the gate to the composed verdict, in either order", async () => { - const { statusesFor, composedGate, groupGateStatus, CHECK_CONTEXT, REPO_CHECK_CONTEXT } = await import("../pulls.ts"); - const c = { owner: "novox", repo: "mesh-catalog", number: 2, commit: "d".repeat(40), id: "build-1", verdict: "pass", summary: "x", - gate: { verdict: "composed", summary: "judged by its group" }, "repo-check": { verdict: "pass", summary: "suite passed" } }; - assert.ok(composedGate(c)); - // Before the composed verdict: the repository check, and the gate pending, saying it waits. - const before = statusesFor(c, {}, undefined); - assert.deepEqual(before.map((s) => [s.context, s.state]), [[CHECK_CONTEXT, "pending"], [REPO_CHECK_CONTEXT, "success"]]); - assert.match(before[0].description, /waits for its group's composed check/); - assert.ok(!before.some((s) => s.state === "error")); - // After it: the standing composed gate is left alone. - const standing = groupGateStatus({ ...c, group: "feat/x", members: [] }, { owner: "novox", repo: "mesh-catalog", commit: c.commit }).description; - const after = statusesFor(c, {}, standing); - assert.deepEqual(after.map((s) => [s.context, s.state]), [[REPO_CHECK_CONTEXT, "success"]]); - // A verdict with an ordinary gate is as before. - assert.equal(composedGate({ ...c, gate: { verdict: "pass", summary: "x" } }), false); -}); - // Issue 1301 (ADR 0503 §3, §5): gitea writes the controller's statuses only where the base still requires them. hq's main -// requires mesh/repo-check alone: its verdict sets mesh/repo-check and no mesh/merge-gate, and its comment names no gate. +// requires mesh/repo-check alone, so no mesh/merge-gate is written on its heads. test("a base's required statuses decide which of the controller's statuses are written", async () => { - const { maintained, writesGate, statusesFor, commentFor, CHECK_STATUS } = await import("../pulls.ts"); + const { maintained, writesGate, CHECK_STATUS } = await import("../pulls.ts"); assert.equal(CHECK_STATUS, "mesh/check"); assert.deepEqual(maintained(undefined), ["mesh/merge-gate", "mesh/repo-check"], "a rule not read: as before"); assert.deepEqual(maintained(["mesh/repo-check"]), ["mesh/repo-check"]); @@ -217,15 +75,6 @@ test("a base's required statuses decide which of the controller's statuses are w assert.equal(writesGate(["mesh/repo-check"]), false); assert.equal(writesGate(["mesh/check"]), false); assert.equal(writesGate(["mesh/merge-gate", "mesh/repo-check"]), true); - const c = { owner: "novox", repo: "hq", number: 5, commit: "a".repeat(40), id: "x", verdict: "warning", summary: "no script", - trunk: "b".repeat(12), "repo-check": { verdict: "pass", summary: "ok" } } as any; - const hq = statusesFor(c, {}, undefined, ["mesh/repo-check"]); - assert.deepEqual(hq.map((s: any) => s.context), ["mesh/repo-check"]); - assert.match(hq[0].description, /merged into trunk bbbbbbbbbbbb/); - assert.deepEqual(statusesFor(c).map((s: any) => s.context), ["mesh/merge-gate", "mesh/repo-check"], "unread: as before"); - assert.deepEqual(statusesFor(c, {}, undefined, ["mesh/check"]), []); - const comment = commentFor(c, {}, ["mesh/repo-check"]); - assert.ok(comment === null || !comment.includes("mesh/merge-gate"), comment ?? ""); }); test("a rule the forge gives with no contexts (null) requires nothing, and an unread rule is unknown", async () => { diff --git a/modules/gitea/test/siblings.test.ts b/modules/gitea/test/siblings.test.ts index 92da1119..edadee27 100644 --- a/modules/gitea/test/siblings.test.ts +++ b/modules/gitea/test/siblings.test.ts @@ -13,7 +13,6 @@ import { join } from "node:path"; import { approvedHeadsOf, besideWords, groupRefusal, keepSiblings, namesSiblings, noteVerdict, readSiblings, siblingOutOfDate, siblingsFile, siblingsOf, siblingStatuses, weighSiblings, type SiblingRecord } from "../dist/siblings.js"; import { weighTrunks } from "../dist/trunk.js"; -import { commentFor, statusesFor } from "../dist/pulls.js"; const h = (c: string) => c.repeat(40); const MEMBER_HEAD = h("a"), HOST_HEAD = h("b"), HOST_BASE = h("c"), HOST_NEW = h("d"), HOST_MOVED = h("e"), MERGE = h("f"); @@ -22,43 +21,6 @@ const verdict = { owner: "novox", repo: "mesh-controller", number: 346, commit: trunk: h("1").slice(0, 12), siblings: [sibling], judge_from: "sibling novox/mesh-controller#346 merge 123456789abc", gate: { verdict: "pass", summary: "every machine composes ".repeat(10) }, "repo-check": { verdict: "pass", summary: "ok" } }; -test("both statuses name the siblings the verdict read, keeping the verdict word and the start of its reason within 140 characters", () => { - // A realistic controller summary for a failure, long enough that the forge clips it. - const reason = "2 of 4 machines do not compose: ace: mesh-host.service needs seat node-reads, which no module on ace claims after this change"; - const c = { ...verdict, gate: { verdict: "fail", summary: reason }, "repo-check": { verdict: "fail", summary: "its merge-check.sh failed: internal/link/check.go:145:2: undefined" } }; - const [gate, repo] = statusesFor(c); - for (const s of [gate, repo]) { - assert.ok(s.description.startsWith(`head ${MEMBER_HEAD.slice(0, 12)} merged into trunk 111111111111 beside mesh-host#114@bbbbbbbb — fail: `), s.description); - assert.ok(s.description.length <= 140, `${s.description.length} characters`); - assert.ok(namesSiblings(s.description)); - } - assert.match(gate.description, /fail: 2 of 4 machines do not compose: ace: mesh-host/, gate.description); - assert.match(repo.description, /fail: its merge-check\.sh failed: internal/, repo.description); - assert.equal(besideWords([...siblingsOf([sibling]), ...siblingsOf([{ ...sibling, number: 115 }])]), "beside mesh-host#114@bbbbbbbb +1 more"); - const comment = commentFor({ ...verdict, gate: { verdict: "fail", summary: "no" } })!; - assert.match(comment, /Siblings read beside it: novox\/mesh-host#114 `bbbbbbbbbbbb` merged onto `cccccccccccc` \(main\); judged by sibling novox\/mesh-controller#346 merge/); -}); - -test("a verdict that could not learn its siblings says so on the status and in the comment, and names none", () => { - const { siblings, ...unknown } = { ...verdict, siblings_unknown: "mesh-delivery did not answer" }; - const reason = "2 of 4 machines do not compose: ace: mesh-host.service needs seat node-reads, which no module claims"; - for (const s of statusesFor({ ...unknown, gate: { verdict: "fail", summary: reason } })) { - assert.match(s.description, /^head aaaaaaaaaaaa merged into trunk 111111111111 siblings unknown — (fail|pass): /, s.description); - assert.ok(s.description.length <= 140); - assert.equal(namesSiblings(s.description), false, "it read no siblings, so the merge tool asks for none"); - } - assert.match(commentFor({ ...unknown, gate: { verdict: "fail", summary: "no" } })!, /Siblings could not be learned: mesh-delivery did not answer/); -}); - -test("a verdict without siblings is said exactly as before (an older controller)", () => { - const { siblings, judge_from, ...old } = verdict; - const [gate] = statusesFor(old); - assert.equal(namesSiblings(gate.description), false); - assert.match(gate.description, /^head aaaaaaaaaaaa merged into trunk 111111111111 — pass: /); - assert.deepEqual(siblingsOf(undefined), []); - assert.deepEqual(siblingsOf([{ repository: "novox/x" }]), [], "a sibling named without its head is none"); -}); - test("the siblings a verdict names are kept per member, replaced by its next verdict, and removed by one that read none", () => { const file = siblingsFile(mkdtempSync(join(tmpdir(), "gitea-siblings-"))); const record: SiblingRecord = {}; diff --git a/modules/gitea/test/tools-account.test.ts b/modules/gitea/test/tools-account.test.ts index c69edb77..6bf55535 100644 --- a/modules/gitea/test/tools-account.test.ts +++ b/modules/gitea/test/tools-account.test.ts @@ -17,7 +17,6 @@ import { join } from "node:path"; import { MintedToken } from "../dist/token.js"; import { GiteaClient } from "../dist/client.js"; import { TOOLS_ACCOUNT, TOOLS_TEAM_UNITS, BUILDER_ACCOUNT, AGENT_ACCOUNT, meshAccounts } from "../dist/tools-account.js"; -import { repoCheckFacts } from "../dist/pulls.js"; const ADMIN = "mesh-admin"; const PASSWORD = "the-vault-minted-this"; @@ -364,16 +363,6 @@ test("the manifest hands the tools their account, and the Go forge bundle holds // Review of issue 469: what reads the rules' list is moved to the branch, and nothing leaves the tools without a token. -test("a repository check said as a warning on a head without merge-check.sh reads the base's protection from the branch", async () => { - const client = fresh(await environment()); - const forgeOf = { holdsFile: async () => false, appliedProtection: client.appliedProtection.bind(client) }; - const c = { owner: "novox", repo: "hq", commit: "abc123def4567890abc123def4567890abc12345", "repo-check": { verdict: "warning" } } as any; - // main requires mesh/repo-check: required, so a head that defines no check fails it for a person to see. - assert.deepEqual(await repoCheckFacts(forgeOf, c, "main"), { defined: false, required: true }); - // A base nothing guards: not required, so the head's repository check passes rather than fails. - assert.deepEqual(await repoCheckFacts(forgeOf, c, "feat/unguarded"), { defined: false, required: false }); -}); - test("a mint that fails leaves the administrator's old token in place: the tools are never left without one", async () => { const env = await environment(); const old = { id: 901, name: "mesh-tools", sha1: "sha-admin-kept", user: ADMIN, scopes: ["read:user"] }; diff --git a/modules/gitea/test/trunk.test.ts b/modules/gitea/test/trunk.test.ts index 5df4c788..2e4b242f 100644 --- a/modules/gitea/test/trunk.test.ts +++ b/modules/gitea/test/trunk.test.ts @@ -8,37 +8,11 @@ import assert from "node:assert/strict"; import { checkOnItsWay, lateVerdictComment, outOfDateStatuses, outOfDateWords, trunkNamed, verdictAtTrunk, weighTrunks } from "../dist/trunk.js"; import { GateWriter } from "../dist/gate.js"; -import { statusesFor, commentFor } from "../dist/pulls.js"; const HEAD = "dfb1415aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"; const TRUNK = "a2c2c35bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb"; const MOVED = "d28ca95ccccccccccccccccccccccccccccccccc"; -test("both statuses say the head and the trunk commit the verdict judged, first, whatever the summary's length", () => { - const c = { owner: "novox", repo: "mesh-host", number: 66, commit: HEAD, verdict: "pass", summary: "x", id: "build-1", - trunk: TRUNK.slice(0, 12), beside: { "mesh-catalog": "0123456789ab", "mesh-lab": "ba9876543210" }, - gate: { verdict: "pass", summary: "4 of 4 compose ".repeat(20) }, - "repo-check": { verdict: "fail", summary: "its merge-check.sh failed: internal/store/store.go:145:2: Declared redeclared" } }; - const [gate, repo] = statusesFor(c); - for (const s of [gate, repo]) { - assert.ok(s.description.startsWith("head dfb1415aaaaa merged into trunk a2c2c35bbbbb — "), s.description); - assert.ok(s.description.length <= 140, `${s.description.length} characters`); - assert.equal(trunkNamed(s.description), "a2c2c35bbbbb"); - } - assert.equal(gate.state, "success"); - assert.equal(repo.state, "failure"); - const comment = commentFor(c)!; - assert.match(comment, /Judged: head `dfb1415aaaaa` merged into trunk `a2c2c35bbbbb`; read beside it: mesh-catalog `0123456789ab`, mesh-lab `ba9876543210`/); - - // A verdict of a build seat from before the rule names no trunk, and is said as before. - const [old] = statusesFor({ ...c, trunk: undefined, gate: { verdict: "pass", summary: "every machine composes" } }); - assert.equal(old.description, "pass: every machine composes"); - assert.equal(trunkNamed(old.description), undefined); - // ADR 0548 §8: a carried verdict's line names the head on the new trunk first, so the merge tool merges it on that trunk. - assert.equal(trunkNamed(`pass ${HEAD.slice(0, 12)} on ${MOVED.slice(0, 12)} | carried from ${TRUNK.slice(0, 12)}, mesh-catalog 0123456789ab | unit 0 run, 4 kept`), - MOVED.slice(0, 12)); -}); - test("a trunk's move puts an open head's merge check out of date, naming both commits, unless a check is on its way", () => { const verdict = (trunk: string, state = "success") => ({ "mesh/merge-gate": { state, description: `head ${HEAD.slice(0, 12)} merged into trunk ${trunk.slice(0, 12)} — pass: x` }, From bf4ce7ef7b5eeea835c8fcee5e88476caef63d4c Mon Sep 17 00:00:00 2001 From: jochen Date: Wed, 4 Nov 2026 11:01:11 +0100 Subject: [PATCH 3/7] mesh-conditions: remove the hold's controller special case, bound to a module that is no more (issue 1126) Claude-Session: https://claude.ai/code/session_01JG73KG7AfSdh6FP8VrBqCn --- .../cmd/mesh-conditions/hold.go | 22 ++++--------------- 1 file changed, 4 insertions(+), 18 deletions(-) diff --git a/modules/mesh-conditions/cmd/mesh-conditions/hold.go b/modules/mesh-conditions/cmd/mesh-conditions/hold.go index c95d106f..97be8fed 100644 --- a/modules/mesh-conditions/cmd/mesh-conditions/hold.go +++ b/modules/mesh-conditions/cmd/mesh-conditions/hold.go @@ -23,22 +23,15 @@ import ( "git.novox.be/novox/mesh-sdk/go/health" ) -// The modules whose delivery holds what they serve beyond their own findings (Limits 12). -const ( - controllerModule = "mesh-controller" - runtimeModule = "node-tools" -) - -// controllerServes are the sources of what the controller serves while it is delivered: its listening, its lease, its -// writes and its facts (ADR 0505 §2 (a)), matched to mesh-controller whatever its key names. -var controllerServes = map[string]bool{"S4": true, "S7": true, "S12": true, "S13": true, "S14": true, "S25": true} +// The module whose delivery holds what it serves beyond their own findings (Limits 12). +const runtimeModule = "node-tools" // about is a node and a module a finding or a condition is about, for the hold. type about struct{ Node, Module string } // aboutOf are the modules on nodes a condition's subject is about (Limits 12): a module on a machine, a provider on its -// node, a seat of the same name as its module on a machine, the controller's own work wherever mesh-controller is -// delivered, and every module's tools on a machine whose tool runner is delivered. Never the machine itself. +// node, a seat of the same name as its module on a machine, and every module's tools on a machine whose tool +// runner is delivered. Never the machine itself. func (k *Keeper) aboutOf(kind, source string, s health.Subject) []about { var out []about parts := strings.Split(s.ID, ".") @@ -72,13 +65,6 @@ func (k *Keeper) aboutOf(kind, source string, s health.Subject) []about { } return out } - if controllerServes[source] || (s.Scope == health.ScopeCore && strings.HasPrefix(s.ID, "controller")) { - for hk := range k.wants { - if node, module := splitNodeModule(hk); module == controllerModule { - out = append(out, about{Node: node, Module: module}) - } - } - } if s.Machine != "" && s.Scope != health.ScopeMachine { out = append(out, about{Node: s.Machine, Module: runtimeModule}) } From b0b7dfc5eb9c60faa373fb08c47a760388a68a0a Mon Sep 17 00:00:00 2001 From: jochen Date: Wed, 4 Nov 2026 11:02:12 +0100 Subject: [PATCH 4/7] mesh-issues: the removed controller is no registered owner and no core owner; old issues keep what they name (issue 1126) Claude-Session: https://claude.ai/code/session_01JG73KG7AfSdh6FP8VrBqCn --- modules/mesh-issues/README.md | 2 +- modules/mesh-issues/cmd/mesh-issues/bus_test.go | 10 +++++----- modules/mesh-issues/cmd/mesh-issues/check.go | 4 ++-- modules/mesh-issues/cmd/mesh-issues/core_test.go | 2 +- modules/mesh-issues/cmd/mesh-issues/manifest_test.go | 2 +- modules/mesh-issues/cmd/mesh-issues/tracker_test.go | 2 +- modules/mesh-issues/files/settings.json | 2 -- modules/mesh-issues/module.json | 2 -- 8 files changed, 11 insertions(+), 15 deletions(-) diff --git a/modules/mesh-issues/README.md b/modules/mesh-issues/README.md index 5359b82b..dbd6ff01 100644 --- a/modules/mesh-issues/README.md +++ b/modules/mesh-issues/README.md @@ -364,7 +364,7 @@ said (hq issue 483). ``` { "first-number": 365, - "owners": ["hq", "mesh-catalog", "mesh-catalog modules/nats", "mesh-controller", …] + "owners": ["hq", "mesh-catalog", "mesh-catalog modules/nats", "mesh-host", …] } ``` diff --git a/modules/mesh-issues/cmd/mesh-issues/bus_test.go b/modules/mesh-issues/cmd/mesh-issues/bus_test.go index 531f057a..77b87b1e 100644 --- a/modules/mesh-issues/cmd/mesh-issues/bus_test.go +++ b/modules/mesh-issues/cmd/mesh-issues/bus_test.go @@ -418,7 +418,7 @@ func (i *instance) call(verb string, args map[string]any) (json.RawMessage, erro func writeSettings(t *testing.T, first int) string { t.Helper() path := filepath.Join(t.TempDir(), "settings.json") - raw, _ := json.Marshal(map[string]any{"first-number": first, "owners": []string{"hq", "mesh-controller"}}) + raw, _ := json.Marshal(map[string]any{"first-number": first, "owners": []string{"hq", "mesh-sdk"}}) if err := os.WriteFile(path, raw, 0o600); err != nil { t.Fatal(err) } @@ -647,15 +647,15 @@ func TestAnIssueIsOpenedNotedLinkedMovedAndListedOverTheBus(t *testing.T) { } must(i.call("note", map[string]any{"number": 365, "text": "found in the gate's plan"})) must(i.call("link", map[string]any{"number": 365, "kind": "decision", "target": "0237"})) - if _, err := i.call("move", map[string]any{"number": 365, "status": "resolved", "fixed-by": []string{"mesh-controller#300"}}); err == nil { + if _, err := i.call("move", map[string]any{"number": 365, "status": "resolved", "fixed-by": []string{"mesh-sdk#300"}}); err == nil { t.Fatal("resolved without located-in") } - must(i.call("move", map[string]any{"number": 365, "status": "located", "located-in": []string{"mesh-controller internal/gate"}})) + must(i.call("move", map[string]any{"number": 365, "status": "located", "located-in": []string{"mesh-sdk internal/gate"}})) must(i.call("move", map[string]any{"number": 365, "status": "fixing", "by": "fix-365@g14"})) - if _, err := i.call("move", map[string]any{"number": 365, "status": "resolved", "fixed-by": []string{"mesh-controller#300"}}); err == nil { + if _, err := i.call("move", map[string]any{"number": 365, "status": "resolved", "fixed-by": []string{"mesh-sdk#300"}}); err == nil { t.Fatal("a core issue resolved without a replay") } - must(i.call("move", map[string]any{"number": 365, "status": "resolved", "fixed-by": []string{"mesh-controller#300"}, "replay": "R365"})) + must(i.call("move", map[string]any{"number": 365, "status": "resolved", "fixed-by": []string{"mesh-sdk#300"}, "replay": "R365"})) var is Issue _ = json.Unmarshal(must(i.call("get", map[string]any{"number": 365})), &is) if is.Status != StatusResolved || len(is.Notes) != 1 || len(is.Links) != 1 || is.Replay != "R365" || is.Closed == nil { diff --git a/modules/mesh-issues/cmd/mesh-issues/check.go b/modules/mesh-issues/cmd/mesh-issues/check.go index b16ffe34..c657d52b 100644 --- a/modules/mesh-issues/cmd/mesh-issues/check.go +++ b/modules/mesh-issues/cmd/mesh-issues/check.go @@ -173,10 +173,10 @@ func (o Owners) locatedIn(e string) (string, error) { return v, nil } -// core is an owner whose issues are core issues (hq cycle.py, to-be 45): the controller, the node-engine, +// core is an owner whose issues are core issues (hq cycle.py, to-be 45): the node-engine, // the tool runner and the console, the SDK, the delivery parts, and the catalogue's bus, forge and build modules // (issue 1041; ADR 0524 §5). -var core = regexp.MustCompile(`\b(mesh-controller|mesh-host|mesh-tools|mesh-sdk|node-tools|mesh-delivery|` + +var core = regexp.MustCompile(`\b(mesh-host|mesh-tools|mesh-sdk|node-tools|mesh-delivery|` + `mesh-build-(go|node|image)|mesh-catalog\s+modules/(nats|gitea))\b`) // IsCore says whether an issue located in these owners is a core issue (ADR 0237). diff --git a/modules/mesh-issues/cmd/mesh-issues/core_test.go b/modules/mesh-issues/cmd/mesh-issues/core_test.go index 8057324a..23780147 100644 --- a/modules/mesh-issues/cmd/mesh-issues/core_test.go +++ b/modules/mesh-issues/cmd/mesh-issues/core_test.go @@ -14,7 +14,7 @@ func TestIsCoreNamesTheBuildModulesAndTheDeliveryParts(t *testing.T) { "mesh-catalog modules/mesh-build-go", "mesh-catalog modules/mesh-build-node", "mesh-catalog modules/mesh-build-image", "mesh-catalog modules/mesh-build-go/cmd/mesh-build-go", "mesh-delivery", "mesh-delivery modules/mesh-delivery-checker", "mesh-catalog modules/mesh-delivery-gate", - "mesh-catalog modules/nats", "mesh-catalog modules/gitea", "mesh-controller", + "mesh-catalog modules/nats", "mesh-catalog modules/gitea", "mesh-sdk", } { if !IsCore([]string{e}) { t.Errorf("an issue located in %q is not core", e) diff --git a/modules/mesh-issues/cmd/mesh-issues/manifest_test.go b/modules/mesh-issues/cmd/mesh-issues/manifest_test.go index d0619782..7468d361 100644 --- a/modules/mesh-issues/cmd/mesh-issues/manifest_test.go +++ b/modules/mesh-issues/cmd/mesh-issues/manifest_test.go @@ -166,7 +166,7 @@ func TestTheSettingsStartTheCounterAndRegisterTheOwners(t *testing.T) { t.Error("the owners are not sorted") } // 1133/AC1: the repository mesh-delivery is registered, so an issue can be located in it. - for _, repo := range []string{"hq", "mesh-catalog", "mesh-controller", "mesh-delivery", "mesh-host", "mesh-sdk", "mesh-tools"} { + for _, repo := range []string{"hq", "mesh-catalog", "mesh-delivery", "mesh-host", "mesh-sdk", "mesh-tools"} { if Owners(s.Owners).owns(repo) != repo { t.Errorf("%s is not registered", repo) } diff --git a/modules/mesh-issues/cmd/mesh-issues/tracker_test.go b/modules/mesh-issues/cmd/mesh-issues/tracker_test.go index 49d11876..55c29fc9 100644 --- a/modules/mesh-issues/cmd/mesh-issues/tracker_test.go +++ b/modules/mesh-issues/cmd/mesh-issues/tracker_test.go @@ -294,7 +294,7 @@ func TestTheCyclesRulesHoldAtTheWrite(t *testing.T) { {"resolved from open", []string{"hq"}, "", MoveAsk{Status: StatusResolved, FixedBy: []string{"hq#280"}}, "only from fixing"}, {"resolved from located", []string{"hq"}, StatusLocated, MoveAsk{Status: StatusResolved, FixedBy: []string{"hq#280"}}, "only from fixing"}, {"resolved from diagnosing", []string{"hq"}, StatusDiagnosing, MoveAsk{Status: StatusResolved, FixedBy: []string{"hq#280"}}, "only from fixing"}, - {"a core issue resolved without a replay", []string{"mesh-controller"}, fx, MoveAsk{Status: StatusResolved, FixedBy: []string{"mesh-controller#9"}}, "core issue"}, + {"a core issue resolved without a replay", []string{"mesh-sdk"}, fx, MoveAsk{Status: StatusResolved, FixedBy: []string{"mesh-sdk#9"}}, "core issue"}, {"a core issue in the bus resolved without a replay", []string{"mesh-catalog modules/nats"}, fx, MoveAsk{Status: StatusResolved, FixedBy: []string{"mesh-catalog#9"}}, "core issue"}, {"a core issue resolved with a replay", []string{"mesh-tools node-tools/internal/bus"}, fx, MoveAsk{Status: StatusResolved, FixedBy: []string{"mesh-tools#25"}, Replay: "R361"}, ""}, {"a core issue resolved with replay-none", []string{"mesh-sdk"}, fx, MoveAsk{Status: StatusResolved, FixedBy: []string{"mesh-sdk#16"}, ReplayNone: "it needs two machines"}, ""}, diff --git a/modules/mesh-issues/files/settings.json b/modules/mesh-issues/files/settings.json index 265fe2da..dca5d6cf 100644 --- a/modules/mesh-issues/files/settings.json +++ b/modules/mesh-issues/files/settings.json @@ -144,8 +144,6 @@ "mesh-catalog modules/zsh", "mesh-catalog modules/zsh-autosuggestions", "mesh-catalog modules/zsh-syntax-highlighting", - "mesh-controller", - "mesh-controller modules/route-proxy", "mesh-delivery", "mesh-delivery modules/mesh-delivery-builder", "mesh-delivery modules/mesh-delivery-checker", diff --git a/modules/mesh-issues/module.json b/modules/mesh-issues/module.json index 196c586c..bd6323d2 100644 --- a/modules/mesh-issues/module.json +++ b/modules/mesh-issues/module.json @@ -256,8 +256,6 @@ "mesh-catalog modules/zsh", "mesh-catalog modules/zsh-autosuggestions", "mesh-catalog modules/zsh-syntax-highlighting", - "mesh-controller", - "mesh-controller modules/route-proxy", "mesh-delivery", "mesh-delivery modules/mesh-delivery-builder", "mesh-delivery modules/mesh-delivery-checker", From 1572ab9b25b01379ba5052c653bbf85c7d006f76 Mon Sep 17 00:00:00 2001 From: jochen Date: Wed, 4 Nov 2026 11:02:56 +0100 Subject: [PATCH 5/7] messenger: remove the old controller's walk decoder and its plan-moved names; the view's events are the only walks (issue 1126) Claude-Session: https://claude.ai/code/session_01JG73KG7AfSdh6FP8VrBqCn --- .../messenger/cmd/messenger/adr505_test.go | 6 - modules/messenger/cmd/messenger/walks.go | 119 ++---------------- modules/messenger/cmd/messenger/walks_test.go | 70 ++--------- 3 files changed, 17 insertions(+), 178 deletions(-) diff --git a/modules/messenger/cmd/messenger/adr505_test.go b/modules/messenger/cmd/messenger/adr505_test.go index af6e0284..25e6defc 100644 --- a/modules/messenger/cmd/messenger/adr505_test.go +++ b/modules/messenger/cmd/messenger/adr505_test.go @@ -51,12 +51,6 @@ var shanksDeclared = Touch{Machine: "shanks", Modules: []string{"mesh-build-imag // ADR 0505 §2b: a machine whose declaration a walk's merges change is held like a machine it sends to, and is not // named among the machines the walk reached. func TestAMachineWhoseDeclarationTheWalkChangesIsHeldAndNotNamedAsReached(t *testing.T) { - ev, ok, err := DecodeWalk([]byte(`{"id":"plan-d","walk":"plan-d","phase":"started","budget_class":"leaf",` + - `"bound_seconds":600,"touches":[{"machine":"novox","modules":["gitea"]},` + - `{"machine":"shanks","modules":["mesh-build-image"],"declared":true}]}`)) - if err != nil || !ok || len(ev.Touches) != 2 || !ev.Touches[1].Declared || ev.Touches[0].Declared { - t.Fatalf("a declared touch was not read: %+v %v %v", ev.Touches, ok, err) - } h, tg, dt, c, _ := newHolder(t) walk(t, h, "plan-d", WalkStarted, "", c.now(), 600, novox("gitea"), shanksDeclared) k := cond("machine.shanks.uncomposable", Urgent, "shanks cannot get new instructions") diff --git a/modules/messenger/cmd/messenger/walks.go b/modules/messenger/cmd/messenger/walks.go index bfb40f8c..2dbb4100 100644 --- a/modules/messenger/cmd/messenger/walks.go +++ b/modules/messenger/cmd/messenger/walks.go @@ -3,26 +3,14 @@ package main // While a delivery walks, what it touches is held from the operator and told once, with the walk's result, // when it ends (novox/hq ADR 0308, issue 532). // -// **What this reads.** The controller says each walk's start and end on its `plan-moved` event (the plan -// whole, ADR 0282 §6), and on those two moments the body carries beside the plan (ADR 0308 §1): +// **What this reads.** The delivery view's `moving`, `settled` and `stuck` events (DecodeMotion below): a walk is a module in +// motion on a node, its id "/", and it carries (ADR 0308 §1): // -// - `walk`: the plan's id (`id` is read when it is missing); -// - `phase`: `started` or `ended`; a plan-moved without it is a move inside a walk and is not read here; -// - `touches`: `[{machine, modules}]`, every module the walk moves on every machine it sends it to, and every -// module a moved tool runner or node-engine carries there; -// - `outcome`: `delivered`, `failed` or `stopped`, on `ended`; -// - `bound_seconds`: twice the largest delivery budget among its deliveries, or 900 when none has one (§5), -// on `started`; missing, 900 is taken; -// - `budget_class`: `core`, `leaf` or `none`; -// - `started_at` on the start and `ended_at` on the end (else `at`, else the plan's `updated`): when it -// happened. A start older than its bound is history and opens nothing; an end older than FreshFor with -// nothing held closes quietly. -// -// A touch may name `carried` beside `modules` (what a moved tool runner or node-engine carries); the -// controller lists those in `modules` too, which is what is read. -// -// An event of its own named `walk`, with the same body, is read the same way, should the controller say it -// there instead. +// - `touches`: `[{machine, modules}]`, the module it moves on that node; +// - `outcome`: `delivered`, `failed` or `stopped`, on the end; +// - `bound_seconds`: on the start; missing, 900 is taken (§5); +// - the start's own moment on the start and the end's on the end. A start older than its bound is history and opens +// nothing; an end older than FreshFor with nothing held closes quietly. // // **What is held (§2).** While a walk is open and within its bound, a message whose subject is a machine the // walk touches, or a module it touches, is held instead of sent: kept on the walk, with when it was held and @@ -60,11 +48,8 @@ import ( "time" ) -// The walk events, by their local names under the controller's seat. +// The walk's phases and outcomes. const ( - EventPlanMoved = "plan-moved" - EventWalk = "walk" - WalkStarted = "started" WalkEnded = "ended" @@ -279,94 +264,6 @@ type walkKeeper interface { Passes() (map[string]Pass, error) } -// DecodeWalk reads a plan-moved or walk event's body: false when it says no walk's start or end. -func DecodeWalk(body []byte) (WalkEvent, bool, error) { - var raw map[string]json.RawMessage - if err := json.Unmarshal(body, &raw); err != nil || raw == nil { - return WalkEvent{}, false, fmt.Errorf("a walk event's body is not a JSON object") - } - str := func(names ...string) string { - for _, n := range names { - var s string - if v, ok := raw[n]; ok && json.Unmarshal(v, &s) == nil && strings.TrimSpace(s) != "" { - return strings.TrimSpace(s) - } - } - return "" - } - var ev WalkEvent - ev.Phase = str("phase") - if ev.Phase == "" { - return WalkEvent{}, false, nil - } - ev.Walk = str("walk", "id") - if ev.Walk == "" { - return WalkEvent{}, false, fmt.Errorf("a walk's %s names no walk", ev.Phase) - } - switch ev.Phase { - case WalkStarted, WalkEnded: - default: - return WalkEvent{}, false, fmt.Errorf("walk %s: phase %q is neither started nor ended", ev.Walk, ev.Phase) - } - ev.Outcome = str("outcome") - if ev.Phase == WalkEnded { - switch ev.Outcome { - case WalkDelivered, WalkFailed, WalkStopped: - default: - return WalkEvent{}, false, fmt.Errorf("walk %s ended with outcome %q: neither delivered, failed nor stopped", ev.Walk, ev.Outcome) - } - } - ev.Class = str("budget_class", "budget-class") - for _, n := range []string{"bound_seconds", "bound-seconds"} { - if v, ok := raw[n]; ok && string(v) != "null" { - var f float64 - if json.Unmarshal(v, &f) != nil || f < 0 { - return WalkEvent{}, false, fmt.Errorf("walk %s: %s is not a number of seconds", ev.Walk, n) - } - ev.BoundSeconds = int(f) - break - } - } - if v, ok := raw["touches"]; ok && string(v) != "null" { - var list []struct { - Machine string `json:"machine"` - Node string `json:"node"` - Modules []string `json:"modules"` - Declared bool `json:"declared"` - } - if json.Unmarshal(v, &list) != nil { - return WalkEvent{}, false, fmt.Errorf("walk %s: touches is not a list of {machine, modules}", ev.Walk) - } - for _, t := range list { - m := t.Machine - if m == "" { - m = t.Node - } - if m != "" { - ev.Touches = append(ev.Touches, Touch{Machine: m, Modules: t.Modules, Declared: t.Declared}) - } - } - } - if s := str("started_at"); s != "" { - if t, err := time.Parse(time.RFC3339Nano, s); err == nil { - ev.StartedAt = t - } - } - when := []string{"started_at", "at", "updated"} - if ev.Phase == WalkEnded { - when = []string{"ended_at", "at", "updated"} - } - for _, n := range when { - if s := str(n); s != "" { - if t, err := time.Parse(time.RFC3339Nano, s); err == nil { - ev.At = t - break - } - } - } - return ev, true, nil -} - // mergeTouches is what two sayings of a walk's touches name together. A machine is declared only while no saying // sends to it (ADR 0505 §2b). func mergeTouches(a, b []Touch) []Touch { diff --git a/modules/messenger/cmd/messenger/walks_test.go b/modules/messenger/cmd/messenger/walks_test.go index 84fd2e33..5a58b884 100644 --- a/modules/messenger/cmd/messenger/walks_test.go +++ b/modules/messenger/cmd/messenger/walks_test.go @@ -1,7 +1,6 @@ package main import ( - "encoding/json" "errors" "strings" "testing" @@ -11,35 +10,12 @@ import ( // The tests of novox/hq ADR 0308 "How it is checked" §2–§6, with the controller's walk events given by a fake // source: the body the controller's plan-moved carries at a walk's start and end (ADR 0308 §1). -// walkBody is a plan-moved body as the controller says a walk's start or end (mesh-controller pull request 247). -func walkBody(t *testing.T, id, phase, outcome string, at time.Time, bound int, touches ...Touch) []byte { - t.Helper() - b := map[string]any{"id": id, "state": "walking", "walk": id, "phase": phase, "touches": touches, - "updated": at.Format(time.RFC3339Nano), "budget_class": "leaf"} - if phase == WalkStarted { - b["started_at"] = at.Format(time.RFC3339Nano) - } else { - b["ended_at"] = at.Format(time.RFC3339Nano) - } - if outcome != "" { - b["outcome"] = outcome - } - if bound > 0 { - b["bound_seconds"] = bound - } - raw, err := json.Marshal(b) - if err != nil { - t.Fatal(err) - } - return raw -} - -// walk says a walk's start or end to the router, through the decoder the bus's events go through. +// walk says a walk's start or end to the router, as the delivery view's events are read into it (DecodeMotion). func walk(t *testing.T, h *Holder, id, phase, outcome string, at time.Time, bound int, touches ...Touch) { t.Helper() - ev, ok, err := DecodeWalk(walkBody(t, id, phase, outcome, at, bound, touches...)) - if err != nil || !ok { - t.Fatalf("the walk event was not read: %v %v", ok, err) + ev := WalkEvent{Walk: id, Phase: phase, Outcome: outcome, Class: "leaf", Touches: touches, BoundSeconds: bound, At: at} + if phase == WalkStarted { + ev.StartedAt = at } h.Walked(ev) } @@ -412,27 +388,8 @@ func TestARestartSaysNothingItHasSaid(t *testing.T) { } } -// The controller's walk events are read as ADR 0308 §1 says them; any other plan-moved is not a walk's. -func TestTheWalkEventsAreReadAsTheControllerSaysThem(t *testing.T) { - at := time.Date(2026, 10, 12, 11, 12, 0, 0, time.UTC) - ev, ok, err := DecodeWalk(walkBody(t, "plan-1", WalkStarted, "", at, 1200, novox("mesh-tools", "node-tools"))) - if err != nil || !ok || ev.Walk != "plan-1" || ev.BoundSeconds != 1200 || ev.Class != "leaf" || !ev.At.Equal(at) || - len(ev.Touches) != 1 || ev.Touches[0].Machine != "novox" || len(ev.Touches[0].Modules) != 2 { - t.Fatalf("%+v %v %v", ev, ok, err) - } - // As mesh-controller's walk events (its pull request 247) say an end: ended_at, and carried beside modules. - end, ok, err := DecodeWalk([]byte(`{"id":"plan-1","walk":"plan-1","phase":"ended","outcome":"stopped",` + - `"touches":[{"machine":"novox","modules":["node-tools","messenger"],"carried":["messenger"]}],` + - `"started_at":"2026-10-12T11:12:00Z","ended_at":"2026-10-12T11:19:00Z","updated":"2026-10-12T11:19:01Z"}`)) - if err != nil || !ok || end.Outcome != WalkStopped || !end.At.Equal(at.Add(7*time.Minute)) || len(end.Touches[0].Modules) != 2 { - t.Fatalf("%+v %v %v", end, ok, err) - } - if _, ok, err := DecodeWalk([]byte(`{"id":"plan-1","state":"walking","tier":1}`)); ok || err != nil { - t.Fatalf("a move inside a walk read as its start or end: %v %v", ok, err) - } - if _, _, err := DecodeWalk([]byte(`{"walk":"plan-1","phase":"ended"}`)); err == nil { - t.Fatalf("an end without its outcome was read") - } +// A walk without a bound is bounded at the default, and a start older than its bound is history. +func TestAWalkWithoutABoundIsBoundedAtTheDefault(t *testing.T) { w := Walk{} if w.bound() != DefaultWalkBound { t.Fatalf("a walk without a bound is bounded at %s", w.bound()) @@ -540,11 +497,7 @@ func TestAStartHeardAfterItsEndOpensNothing(t *testing.T) { h, tg, dt, c, st := newHolder(t) started := c.now() c.pass(20 * time.Second) - end, _, err := DecodeWalk([]byte(`{"walk":"plan-e","phase":"ended","outcome":"delivered","touches":[{"machine":"novox","modules":["mesh-tools"]}],` + - `"started_at":"` + started.Format(time.RFC3339Nano) + `","ended_at":"` + c.now().Format(time.RFC3339Nano) + `"}`)) - if err != nil { - t.Fatal(err) - } + end := WalkEvent{Walk: "plan-e", Phase: WalkEnded, Outcome: WalkDelivered, Touches: []Touch{novox("mesh-tools")}, StartedAt: started, At: c.now()} h.Walked(end) c.pass(time.Second) walk(t, h, "plan-e", WalkStarted, "", started, 600, novox("mesh-tools")) @@ -747,14 +700,9 @@ func TestAStartHeardAfterItsEndOpensNothingWhenTheControllersClockIsAhead(t *tes h, _, _, c, _ := newHolder(t) ended := c.now().Add(10 * time.Minute) // by the controller's clock started := ended.Add(-20 * time.Second) - body := `{"walk":"plan-k","phase":"ended","outcome":"delivered","touches":[{"machine":"novox","modules":["mesh-tools"]}],` + - `"ended_at":"` + ended.Format(time.RFC3339Nano) + `"` + end := WalkEvent{Walk: "plan-k", Phase: WalkEnded, Outcome: WalkDelivered, Touches: []Touch{novox("mesh-tools")}, At: ended} if said == "both" { - body += `,"started_at":"` + started.Format(time.RFC3339Nano) + `"` - } - end, _, err := DecodeWalk([]byte(body + `}`)) - if err != nil { - t.Fatal(err) + end.StartedAt = started } h.Walked(end) c.pass(time.Second) From 4f67143a7867ccb3ba5b0be839a0d449c2b854d9 Mon Sep 17 00:00:00 2001 From: jochen Date: Wed, 4 Nov 2026 11:05:50 +0100 Subject: [PATCH 6/7] gitea: remove what only tests called: the gate writer's verdict and its ask and history weighing, the late-verdict comment, the check-duration writer, the sibling record's two writers (issue 1126) Claude-Session: https://claude.ai/code/session_01JG73KG7AfSdh6FP8VrBqCn --- modules/gitea/client.ts | 13 --- modules/gitea/gate.ts | 148 +++------------------------- modules/gitea/index.ts | 2 - modules/gitea/siblings.ts | 42 -------- modules/gitea/test/gate.test.ts | 141 +++----------------------- modules/gitea/test/guard.test.ts | 15 +-- modules/gitea/test/siblings.test.ts | 32 ++---- modules/gitea/test/trunk.test.ts | 31 +----- modules/gitea/trunk.ts | 25 +---- 9 files changed, 45 insertions(+), 404 deletions(-) diff --git a/modules/gitea/client.ts b/modules/gitea/client.ts index 9ccf741c..5f80c6de 100644 --- a/modules/gitea/client.ts +++ b/modules/gitea/client.ts @@ -672,19 +672,6 @@ export class GiteaClient { return out; } - /** Every status of one context on a commit, newest first, as the forge keeps each one it was given: what tells - * when the check now pending on a head was opened, after a restart (novox/hq issue 456). */ - async contextHistory(owner: string, repo: string, sha: string, context: string): Promise { - const raw = await this.request(`${repoPath(owner, repo)}/statuses/${commitSegment(sha)}?sort=highestindex&limit=50`); - const out: HeadCheck[] = []; - for (const s of Array.isArray(raw) ? raw : []) { - if (s?.context !== context) continue; - out.push({ state: String(s.status ?? s.state ?? ""), description: String(s.description ?? ""), - updated_at: String(s.created_at ?? s.updated_at ?? "") }); - } - return out; - } - /** A commit's statuses whole, as the forge combines them for a ref (a sha, or the start of one): the commit * it resolved to and every status with its description, link, times and setter (novox/hq ADR 0239). */ async combinedStatus(owner: string, repo: string, ref: string): Promise<{ sha: string; state: string; statuses: unknown[] }> { diff --git a/modules/gitea/gate.ts b/modules/gitea/gate.ts index 067dea26..e1c36bb5 100644 --- a/modules/gitea/gate.ts +++ b/modules/gitea/gate.ts @@ -1,74 +1,21 @@ -// **One writer of a head's mesh/merge-gate, and the verdict lands last** (novox/hq issue 446). +// **One writer of a head's mesh/merge-gate** (novox/hq issue 446). // -// A pending check says where it is: "waiting in the build queue since …" when the head is announced, and the verdict when -// it is said. The forge keeps only the newest status of a context, so a pending word -// written after the verdict hides it: a pass reads pending and the merge waits for ever; a failure hides under -// "building". Two writers cannot order their writes, and a writer that reads the forge before writing races the other -// between its read and its write. So this module is the one writer of mesh/merge-gate, and every write of one head's -// gate goes through one chain: a verdict is marked the moment it is heard, before it waits its turn, and a "building" -// still waiting behind or taking its turn after that mark is dropped, never written. -// -// **A recheck's pending is written here too** (novox/hq issue 456). mesh-delivery's recheck puts the gate back to -// pending through the gitea_commit_status tool, which this module serves from the watcher's own bundle so that it -// reaches this writer: the pending clears the verdict mark, so the new run's "building" is said, and queues behind an -// old verdict in flight, so that verdict lands first. A verdict is weighed by the ask it answers — the controller's -// checked.id, `build-` of when the check was asked — against when the check now pending was opened: -// an older run's verdict, redelivered after its write failed, is dropped, never written over the pending. After a -// restart, when the check was opened is read from the forge's own history of the gate. +// A pending check says where it is: "waiting in the build queue since …" when the head is announced. The forge keeps only +// the newest status of a context, so two writers cannot order their writes; this module is the one writer of +// mesh/merge-gate, and every write of one head's gate goes through one chain, in the order it was asked. A recheck's +// pending (novox/hq issue 456) and a trunk's move (ADR 0323 §4) take their turns on the same chain. No verdict is written +// here any more: the controller's `checked` is no event (ADR 0571 clause 13), and main requires the checker's mesh/check. import { CHECK_CONTEXT, pendingWords } from "./pulls.js"; import type { CommitStatus, HeadCheck } from "./client.js"; -/** What the gate writer needs of the forge: a status set, and a head's gate as the forge holds it. */ +/** What the gate writer needs of the forge: a status set, and a head's statuses as the forge holds them. */ export interface GateForge { setCommitStatus(owner: string, repo: string, sha: string, status: CommitStatus): Promise; - /** The head's mesh/merge-gate, newest: its state and when it was set; undefined when it has none. */ - gateOf(owner: string, repo: string, sha: string): Promise<{ state: string; updated_at?: string } | undefined>; - /** The head's mesh/merge-gate statuses, every one the forge keeps, newest first (novox/hq issue 456). */ - gateHistory(owner: string, repo: string, sha: string): Promise; /** The head's statuses by context, the newest of each, with their words (novox/hq ADR 0323). */ headChecks(owner: string, repo: string, sha: string): Promise>; } -/** One status of a head's gate as the forge keeps it: its state, its words, and when it was set. */ -export interface GateStatus { - state: string; - description?: string; - updated_at?: string; -} - -/** - * When a check was asked, from its id as mesh-controller's link.NewBuildID writes it: `build-`. - * Undefined for an id of any other shape, whose ask time is not known (novox/hq issue 456). - */ -export function askedAt(id: string | undefined): number | undefined { - const m = /^build-(\d{1,20})$/.exec(String(id ?? "")); - if (!m) return undefined; - const ms = Number(BigInt(m[1]) / 1_000_000n); - return ms > 0 ? ms : undefined; -} - -/** - * When the check now pending on a head was opened, from the gate's history, newest first: the newest status, past the - * "building" words this writer says only inside an open check, when it is pending; undefined when the gate holds a - * verdict, holds nothing, or its time cannot be read (novox/hq issue 456). - */ -export function openedAt(history: GateStatus[]): number | undefined { - for (const s of history) { - if (s.state !== "pending") return undefined; - if ((s.description ?? "").startsWith(BUILDING)) continue; - const t = Date.parse(s.updated_at ?? ""); - return Number.isNaN(t) ? undefined : t; - } - return undefined; -} - -/** How far a build machine's clock may stand behind this one's when a start is weighed against the pending words. */ -const CLOCK_SLACK_MS = 60_000; - -/** How the words of a check a machine was running began: a status the forge still holds from before is no opening of a check. */ -const BUILDING = "building on "; - /** Whether a repository as the mesh clones it (`http://forge/novox/hq.git`) is owner/repo. */ export function clonedAs(url: string | undefined, owner: string, repo: string): boolean { const u = (url ?? "").trim().toLowerCase().replace(/\.git$/, ""); @@ -77,12 +24,6 @@ export function clonedAs(url: string | undefined, owner: string, repo: string): } interface Head { - /** When the pending words were last set by this writer; undefined for a head this process did not announce. */ - pendingAt?: number; - /** A verdict was heard for the head: nothing pending is written after it. */ - verdict: boolean; - /** Which verdict set the mark: one dropped in its turn takes the mark back only while it is still its own. */ - mark?: object; /** The chain the head's writes take their turns on. */ chain: Promise; } @@ -101,7 +42,7 @@ export class GateWriter { const key = `${owner}/${repo}@${sha}`.toLowerCase(); let h = this.heads.get(key); if (!h) { - h = { verdict: false, chain: Promise.resolve() }; + h = { chain: Promise.resolve() }; this.heads.set(key, h); // Kept for the heads still in play; the oldest go first, past what any forge holds open at once. if (this.heads.size > 2000) this.heads.delete(this.heads.keys().next().value as string); @@ -127,36 +68,17 @@ export class GateWriter { pending(owner: string, repo: string, sha: string): Promise { const h = this.head(owner, repo, sha); const at = this.now(); - this.opened(h, at); return this.said(h, `${owner}/${repo}@${sha.slice(0, 8)}: could not say a check is pending`, () => this.forge.setCommitStatus(owner, repo, sha, { state: "pending", context: CHECK_CONTEXT, description: pendingWords(at) })); } - /** A new check of the head opened now: no verdict heard for it yet, and nothing older than it is its own. */ - private opened(h: Head, at: Date): void { - h.pendingAt = at.getTime(); - h.verdict = false; - h.mark = undefined; - } - /** - * A recheck's pending, asked through the gitea_commit_status tool (novox/hq issue 456): on mesh/merge-gate it opens - * a new check of the head — the verdict mark cleared, so the new run's "building" is said — and takes its turn - * after every write already asked for, an old verdict in flight included, so it lands over that verdict. Any - * other status of the merge check (mesh/repo-check, which the verdict also writes) takes its turn on the same - * chain. A write that fails fails the caller, and the head is as it was before it. + * A recheck's pending, asked through the gitea_commit_status tool (novox/hq issue 456): it takes its turn after every + * write already asked for, so it lands over them. Any other status of the merge check takes its turn on the same + * chain. A write that fails fails the caller. */ rechecked(owner: string, repo: string, sha: string, status: CommitStatus): Promise { - const h = this.head(owner, repo, sha); - const write = () => this.forge.setCommitStatus(owner, repo, sha, status); - if (status.context !== CHECK_CONTEXT || status.state !== "pending") return this.turn(h, write); - const before = { pendingAt: h.pendingAt, verdict: h.verdict, mark: h.mark }; - const at = this.now(); - this.opened(h, at); - return this.turn(h, write).catch((err) => { - if (h.pendingAt === at.getTime() && !h.verdict) Object.assign(h, before); - throw err; - }); + return this.turn(this.head(owner, repo, sha), () => this.forge.setCommitStatus(owner, repo, sha, status)); } /** @@ -178,50 +100,4 @@ export class GateWriter { } }).then(() => written); } - - /** - * The verdict: marked at once, so no pending word is written after it, then written in its turn — last. A write - * that fails fails the caller, so the event is offered again. - * - * `id` is the ask it answers (checked.id): a verdict of a check asked before the check now pending was opened is - * an older run's — redelivered after its write failed, or heard late — and is dropped, never written over the - * pending (novox/hq issue 456). When this process opened no check of the head (it restarted since), the opening - * is read from the forge's history of the gate in the verdict's turn; a history that cannot be read writes the - * verdict, as one with no readable id is: a verdict is never lost to what cannot be weighed. - */ - verdict(owner: string, repo: string, sha: string, write: () => Promise, id?: string): Promise { - const h = this.head(owner, repo, sha); - const asked = askedAt(id); - const head = `${owner}/${repo}@${sha.slice(0, 8)}`; - if (asked !== undefined && h.pendingAt !== undefined && older(asked, h.pendingAt)) { - console.log(`[gitea] ${head}: the verdict of ${id} is an older run's than the check now pending; not written`); - return Promise.resolve(); - } - const before = h.verdict; - const mark = {}; - h.verdict = true; - h.mark = mark; - return this.turn(h, async () => { - if (asked !== undefined && h.pendingAt === undefined) { - let opened: number | undefined; - try { - opened = openedAt(await this.forge.gateHistory(owner, repo, sha)); - } catch (err) { - console.error(`[gitea] ${head}: the gate's history could not be read, so the verdict of ${id} is written ` + - `unweighed — ${err instanceof Error ? err.message : err}`); - } - if (opened !== undefined && older(asked, opened)) { - if (h.mark === mark) h.verdict = before; - console.log(`[gitea] ${head}: the verdict of ${id} is an older run's than the check the forge holds pending; not written`); - return; - } - } - await write(); - }); - } -} - -/** Whether a check asked at `asked` was asked before a check opened at `opened`, past what the clocks may differ by. */ -function older(asked: number, opened: number): boolean { - return asked < opened - CLOCK_SLACK_MS; } diff --git a/modules/gitea/index.ts b/modules/gitea/index.ts index fd4380c6..cbf8bedd 100644 --- a/modules/gitea/index.ts +++ b/modules/gitea/index.ts @@ -356,8 +356,6 @@ async function siblingMoves(client: GiteaClient, repository: string, number?: nu // (gate.ts). A second writer elsewhere raced the verdict and could leave a pass reading pending. const gates = new GateWriter({ setCommitStatus: (owner, repo, sha, status) => (gitea as GiteaClient).setCommitStatus(owner, repo, sha, status), - gateOf: async (owner, repo, sha) => (await (gitea as GiteaClient).headChecks(owner, repo, sha))[CHECK_CONTEXT], - gateHistory: (owner, repo, sha) => (gitea as GiteaClient).contextHistory(owner, repo, sha, CHECK_CONTEXT), headChecks: (owner, repo, sha) => (gitea as GiteaClient).headChecks(owner, repo, sha), }); diff --git a/modules/gitea/siblings.ts b/modules/gitea/siblings.ts index ae9b6c58..5dcff478 100644 --- a/modules/gitea/siblings.ts +++ b/modules/gitea/siblings.ts @@ -119,48 +119,6 @@ export function keepSiblings(file: string | null, record: SiblingRecord): void { } } -/** A member's latest verdict, kept: its siblings when it read some, else the entry removed. Answers whether it changed. */ -export function noteVerdict(record: SiblingRecord, key: string, head: string, siblings: SiblingNamed[]): boolean { - if (siblings.length === 0) { - if (!(key in record)) return false; - delete record[key]; - return true; - } - record[key] = { head, siblings }; - return true; -} - -/** One member of a composed verdict, as the group's composed check read it: its pull request, the head it judged, the - * trunk commit that head was merged onto, and the branch the pull request merges into. */ -export interface ComposedMember { - owner: string; - repo: string; - number?: number; - commit: string; - trunk?: string; - base?: string; -} - -/** - * **A group's composed verdict names, for each member, the other members at the heads and base commits it composed** - * (novox/hq ADR 0403 §5, issue 1049): the composed check read every member's head together, so it is the verdict to trust - * for the siblings. Without this, the record held what a member's OWN verdict named, which went stale when a sibling got - * a new head; the sibling's own merge of the head the composed check judged then read as another head's merge, put the - * member's gate back to pending, and the group merged in two passes. A member the verdict cannot name (no pull request - * number, trunk commit or base branch), or a verdict of fewer than two such members, changes nothing. Answers whether - * the record changed. - */ -export function noteComposed(record: SiblingRecord, members: ComposedMember[]): boolean { - const named = members.filter((m) => m.owner && m.repo && m.number && m.commit && m.trunk && m.base); - if (named.length < 2 || named.length !== members.length) return false; - for (const m of named) { - const siblings = named.filter((o) => o !== m).map((o): SiblingNamed => ({ - repository: `${o.owner}/${o.repo}`, number: o.number!, head: o.commit, base: o.base!, base_commit: o.trunk! })); - record[memberKey(m.owner, m.repo, m.number!)] = { head: m.commit, siblings }; - } - return true; -} - // --- what a sibling's move is --------------------------------------------------------------------------------- /** What weighing a sibling reads of the forge. */ diff --git a/modules/gitea/test/gate.test.ts b/modules/gitea/test/gate.test.ts index 590f1c3e..eee5342b 100644 --- a/modules/gitea/test/gate.test.ts +++ b/modules/gitea/test/gate.test.ts @@ -1,9 +1,8 @@ import assert from "node:assert/strict"; import { test } from "node:test"; -// **A pending check says where it is, and the verdict lands last** (novox/hq issue 446). This module is the one writer -// of a head's mesh/merge-gate: the announcement's pending words and the verdict. The forge keeps the newest status of a context, so a pending word -// written after the verdict hides it — a pass reads pending, a failure hides under "building". +// **One writer of a head's mesh/merge-gate** (novox/hq issue 446): the announcement's pending words, a recheck's pending and +// a trunk's move, each write of a head in the order it was asked. const SHA = "a6b08de8507c1703528277e5cefc80548dbc9bb3"; @@ -11,15 +10,11 @@ const SHA = "a6b08de8507c1703528277e5cefc80548dbc9bb3"; function aForge() { const kept: Record = {}; const written: string[] = []; - /** Every mesh/merge-gate status the forge was given, newest first, as gitea keeps each one. */ - const history: { state: string; description: string; updated_at: string }[] = []; let failNext = 0; let hold: Promise | null = null; let release = () => {}; - let readHold: Promise | null = null; - let readRelease = () => {}; return { - kept, written, history, + kept, written, /** The next writes fail: a request the forge did not take. */ failWrites(n = 1) { failNext = n; @@ -32,6 +27,9 @@ function aForge() { hold = null; release(); }, + async headChecks() { + return {}; + }, async setCommitStatus(_o: string, _r: string, _sha: string, s: { state: string; context: string; description: string }) { if (hold) await hold; if (failNext > 0) { @@ -39,121 +37,35 @@ function aForge() { throw new Error("the forge answered 502"); } kept[s.context] = { state: s.state, description: s.description, updated_at: new Date().toISOString() }; - if (s.context === "mesh/merge-gate") history.unshift(kept[s.context]); written.push(`${s.state} ${s.description}`); }, - /** Reads answer what the forge held when asked, as late as the reads are held: the answer of a request in flight. */ - holdReads() { - readHold = new Promise((r) => (readRelease = r)); - }, - releaseReads() { - readHold = null; - readRelease(); - }, - async gateOf(_o: string, _r: string, _sha: string) { - const held = kept["mesh/merge-gate"] ? { ...kept["mesh/merge-gate"] } : undefined; - if (readHold) await readHold; - return held; - }, - async gateHistory(_o: string, _r: string, _sha: string) { - return history.map((h) => ({ ...h })); - }, }; } -/** The clock of the test: the head announced at 01:08:50Z. */ -const announcedAt = () => new Date("2026-10-11T01:08:50Z"); - // **A recheck's pending goes through the one writer** (novox/hq issue 456). mesh-delivery's recheck puts the gate back // to pending through the gitea_commit_status tool. -/** The ask of a check, as the controller's link.NewBuildID names it: when it was asked, in unix nanoseconds. */ -const askOf = (at: string) => `build-${BigInt(Date.parse(at)) * 1_000_000n}`; -const firstRun = askOf("2026-10-11T01:09:01Z"); const recheckAt = new Date("2026-10-11T01:30:00Z"); -const secondRun = askOf("2026-10-11T01:30:02Z"); const checkingAgain = { state: "pending" as const, context: "mesh/merge-gate", description: "checking again: main moved on" }; -const writeOf = (forge: ReturnType, state: string, description: string) => () => - forge.setCommitStatus("novox", "hq", SHA, { state, context: "mesh/merge-gate", description }); const gateOf = (forge: ReturnType) => `${forge.kept["mesh/merge-gate"].state} ${forge.kept["mesh/merge-gate"].description}`; -test("an old run's verdict redelivered after a recheck's pending is not written over it", async () => { - const { GateWriter } = await import("../gate.ts"); - const forge = aForge(); - let now = announcedAt(); - const gates = new GateWriter(forge, () => now); - await gates.pending("novox", "hq", SHA); - forge.failWrites(); // the first run's verdict is not taken: the event is offered again - await assert.rejects(gates.verdict("novox", "hq", SHA, writeOf(forge, "success", "pass"), firstRun)); - now = recheckAt; - await gates.rechecked("novox", "hq", SHA, checkingAgain); - await gates.verdict("novox", "hq", SHA, writeOf(forge, "success", "pass"), firstRun); // redelivered - assert.equal(gateOf(forge), "pending checking again: main moved on", forge.written.join(" | ")); - // The recheck's own verdict is written. - await gates.verdict("novox", "hq", SHA, writeOf(forge, "failure", "fail: 0 of 4 compose"), secondRun); - assert.equal(gateOf(forge), "failure fail: 0 of 4 compose"); +test("a repository as the mesh clones it is told from another", async () => { + const { clonedAs } = await import("../gate.ts"); + assert.equal(clonedAs("http://127.0.0.1:20000/novox/hq.git", "novox", "hq"), true); + assert.equal(clonedAs("http://127.0.0.1:20000/novox/mesh-hq.git", "novox", "hq"), false); + assert.equal(clonedAs("http://127.0.0.1:20000/novox/mesh-controller.git", "novox", "hq"), false); }); -test("an old run's verdict in flight when the recheck comes lands before the recheck's pending", async () => { - const { GateWriter } = await import("../gate.ts"); - const forge = aForge(); - let now = announcedAt(); - const gates = new GateWriter(forge, () => now); - await gates.pending("novox", "hq", SHA); - forge.holdWrites(); // the first run's verdict is being written when the recheck comes - const verdict = gates.verdict("novox", "hq", SHA, writeOf(forge, "success", "pass"), firstRun); - await new Promise((r) => setImmediate(r)); - now = recheckAt; - const pending = gates.rechecked("novox", "hq", SHA, checkingAgain); - forge.release(); - await Promise.all([verdict, pending]); - assert.equal(gateOf(forge), "pending checking again: main moved on", forge.written.join(" | ")); -}); - -test("the recheck's own verdict is written, even asked a little before the pending by another machine's clock", async () => { +test("a recheck the forge does not take fails its caller, and the next write still takes its turn", async () => { const { GateWriter } = await import("../gate.ts"); const forge = aForge(); const gates = new GateWriter(forge, () => recheckAt); - await gates.rechecked("novox", "hq", SHA, checkingAgain); - await gates.verdict("novox", "hq", SHA, writeOf(forge, "success", "pass"), askOf("2026-10-11T01:29:30Z")); - assert.equal(gateOf(forge), "success pass"); - // A verdict whose ask cannot be read is written: a verdict is never lost to what cannot be weighed. - await gates.rechecked("novox", "hq", SHA, checkingAgain); - await gates.verdict("novox", "hq", SHA, writeOf(forge, "failure", "fail"), "an id of another shape"); - assert.equal(gateOf(forge), "failure fail"); -}); - -test("after a restart, an old run's verdict is weighed against the check the forge's history holds open", async () => { - const { GateWriter } = await import("../gate.ts"); - const forge = aForge(); - // What the forge kept before this process started: the announcement, the first run's verdict never written, - // the recheck's pending, and the second run's "building" over it. - forge.history.unshift({ state: "pending", description: "waiting in the build queue since 03:08", updated_at: "2026-10-11T01:08:50Z" }); - forge.history.unshift({ state: "pending", description: "checking again: main moved on", updated_at: "2026-10-11T01:30:00Z" }); - forge.history.unshift({ state: "pending", description: "building on shanks since 03:30", updated_at: "2026-10-11T01:30:05Z" }); - forge.kept["mesh/merge-gate"] = forge.history[0]; - const gates = new GateWriter(forge, announcedAt); - await gates.verdict("novox", "hq", SHA, writeOf(forge, "success", "pass"), firstRun); - assert.equal(forge.kept["mesh/merge-gate"].description, "building on shanks since 03:30", forge.written.join(" | ")); - await gates.verdict("novox", "hq", SHA, writeOf(forge, "failure", "fail"), secondRun); - assert.equal(gateOf(forge), "failure fail"); -}); - -test("a recheck the forge does not take fails its caller, and the head is as it was", async () => { - const { GateWriter } = await import("../gate.ts"); - const forge = aForge(); - let now = announcedAt(); - const gates = new GateWriter(forge, () => now); await gates.pending("novox", "hq", SHA); forge.failWrites(); - await assert.rejects(gates.verdict("novox", "hq", SHA, writeOf(forge, "success", "pass"), firstRun)); - now = recheckAt; - forge.failWrites(); await assert.rejects(gates.rechecked("novox", "hq", SHA, checkingAgain)); - // The recheck was refused, so its check is not asked: the first run's verdict, offered again, is the head's. - await gates.verdict("novox", "hq", SHA, writeOf(forge, "success", "pass"), firstRun); - assert.equal(gateOf(forge), "success pass"); + await gates.rechecked("novox", "hq", SHA, checkingAgain); + assert.equal(gateOf(forge), "pending checking again: main moved on"); }); test("the status tool writes the merge check's pending through the writer, and the mesh's other statuses as they were", async () => { @@ -166,40 +78,15 @@ test("the status tool writes the merge check's pending through the writer, and t setCommitStatus: async (_o, _r, _sha, s) => void direct.push(`${s.context} ${s.state}`), gates, }); - await gates.verdict("novox", "hq", SHA, writeOf(forge, "success", "pass"), firstRun); await tool.run({ owner: "novox", repo: "hq", sha: SHA, context: "mesh/merge-gate", state: "pending", description: "checking again: x" }); await tool.run({ owner: "novox", repo: "hq", sha: SHA, context: "mesh/repo-check", state: "pending", description: "checking again: x" }); await tool.run({ owner: "novox", repo: "hq", sha: SHA, context: "mesh/delivery", state: "success", description: "delivered" }); assert.equal(gateOf(forge), "pending checking again: x"); assert.equal(forge.kept["mesh/repo-check"].state, "pending"); assert.deepEqual(direct, ["mesh/delivery success"]); - // The redelivered first run's verdict is the writer's to drop. - await gates.verdict("novox", "hq", SHA, writeOf(forge, "success", "pass"), firstRun); - assert.equal(gateOf(forge), "pending checking again: x"); // A head named short is another head to the writer: refused, nothing sent. await assert.rejects(tool.run({ owner: "novox", repo: "hq", sha: SHA.slice(0, 12), context: "mesh/merge-gate", state: "pending" }), /whole sha/); await assert.rejects(tool.run({ owner: "novox", repo: "hq", sha: SHA, context: "mesh/merge-gate", state: "success" }), /set by its verdict only/); }); - -test("when a check was asked is read from its id; the opening of the check now pending from the gate's history", async () => { - const { askedAt, openedAt } = await import("../gate.ts"); - assert.equal(askedAt("build-1791680941048622697"), Date.parse("2026-10-11T01:09:01.048Z")); - assert.equal(askedAt("build-mesh-catalog"), undefined); - assert.equal(askedAt(undefined), undefined); - assert.equal(openedAt([{ state: "pending", description: "building on ace since 03:31", updated_at: "2026-10-11T01:31:00Z" }, - { state: "pending", description: "checking again: x", updated_at: "2026-10-11T01:30:00Z" }]), Date.parse("2026-10-11T01:30:00Z")); - assert.equal(openedAt([{ state: "success", description: "pass", updated_at: "2026-10-11T01:30:00Z" }]), undefined); - assert.equal(openedAt([]), undefined); -}); - -test("a repository as the mesh clones it is told from another", async () => { - const { clonedAs } = await import("../gate.ts"); - assert.equal(clonedAs("http://127.0.0.1:20000/novox/hq.git", "novox", "hq"), true); - assert.equal(clonedAs("http://127.0.0.1:20000/novox/mesh-hq.git", "novox", "hq"), false); - assert.equal(clonedAs("http://127.0.0.1:20000/novox/mesh-controller.git", "novox", "hq"), false); -}); - -// Issue 1301: a build machine's start on a head that holds no mesh/merge-gate (its base requires none, so none was written) -// says nothing: no "building on" status appears on a main that does not require the gate. diff --git a/modules/gitea/test/guard.test.ts b/modules/gitea/test/guard.test.ts index f6b1ec9e..8b080200 100644 --- a/modules/gitea/test/guard.test.ts +++ b/modules/gitea/test/guard.test.ts @@ -7,7 +7,7 @@ import { test, after, beforeEach } from "node:test"; import assert from "node:assert/strict"; import { createServer, type IncomingMessage, type ServerResponse } from "node:http"; -import { mkdtempSync, readFileSync } from "node:fs"; +import { mkdtempSync, readFileSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; @@ -179,7 +179,7 @@ const client = new GiteaClient(`http://127.0.0.1:${port}`, new ConfiguredToken(" const stateDir = mkdtempSync(join(tmpdir(), "gitea-tools-")); // The merge check's one writer, as the watcher's bundle holds it: what the merge tool's pending goes through. const writer = new GateWriter({ setCommitStatus: (o, r, sha, s) => client.setCommitStatus(o, r, sha, s), - gateOf: async () => undefined, gateHistory: async () => [], headChecks: async () => ({}) }); + headChecks: async () => ({}) }); const throughTheWriter: string[] = []; const gates = { rechecked: (o: string, r: string, sha: string, s: any) => { throughTheWriter.push(`${sha.slice(0, 12)} ${s.context} ${s.state}`); @@ -302,8 +302,8 @@ test("gitea_merge_pull_request merges only a head whose required statuses all su // the trunk's head and passes, the merge goes through — asked again, never by itself. test("gitea_merge_pull_request refuses a verdict judged against an earlier trunk, and asks the check once", async () => { const merge = () => tool("gitea_merge_pull_request").run({ owner: "novox", repo: "mesh-catalog", number: 7, head: REVIEWED }); - const { keepTook, tookFile } = await import("../dist/trunk.js"); - keepTook(tookFile(stateDir), "novox/mesh-catalog", { took: "4m31s", at: "2026-10-12T14:49:00Z" }); + const { tookFile } = await import("../dist/trunk.js"); + writeFileSync(tookFile(stateDir)!, JSON.stringify({ "novox/mesh-catalog": { took: "4m31s", at: "2026-10-12T14:49:00Z" } })); state.trunk = MOVED; // another merge moved the trunk after the verdict await assert.rejects(merge(), (err: Error) => { @@ -734,7 +734,7 @@ test("gitea_merge_pull_request refuses a head the Approve did not name", async ( // the sibling merged that head, and the member then merges on its first ask: the sibling's own merge is no move of its // base. The records are the module's own (siblings.ts), the weighing is what the sibling's merge announcement does. test("a group of two merges in one pass: the sibling's merge of the composed head does not stop the member's merge (issue 1049)", async () => { - const { keepSiblings, siblingsFile, noteComposed, weighSiblings } = await import("../dist/siblings.js"); + const { keepSiblings, siblingsFile, weighSiblings } = await import("../dist/siblings.js"); const both = ["novox/mesh-catalog@" + REVIEWED, "novox/mesh-host@" + SIBLING_HEAD]; const OLD_SIBLING = "9".repeat(40), HOST_BASE = "c".repeat(40), HOST_MERGE = "f".repeat(40); const beside = `head ${REVIEWED.slice(0, 12)} merged into trunk ${TRUNK.slice(0, 12)} beside mesh-host#114@${OLD_SIBLING.slice(0, 8)}`; @@ -747,8 +747,9 @@ test("a group of two merges in one pass: the sibling's merge of the composed hea const record: Record = { "novox/mesh-catalog#7": { head: REVIEWED, siblings: [{ repository: "novox/mesh-host", number: 114, head: OLD_SIBLING, base: "main", base_commit: HOST_BASE }] } }; // The composed verdict then judged both new heads. - noteComposed(record, [{ owner: "novox", repo: "mesh-catalog", number: 7, commit: REVIEWED, trunk: TRUNK.slice(0, 12), base: "main" }, - { owner: "novox", repo: "mesh-host", number: 114, commit: SIBLING_HEAD, trunk: HOST_BASE.slice(0, 12), base: "main" }]); + Object.assign(record, { + "novox/mesh-catalog#7": { head: REVIEWED, siblings: [{ repository: "novox/mesh-host", number: 114, head: SIBLING_HEAD, base: "main", base_commit: HOST_BASE.slice(0, 12) }] }, + "novox/mesh-host#114": { head: SIBLING_HEAD, siblings: [{ repository: "novox/mesh-catalog", number: 7, head: REVIEWED, base: "main", base_commit: TRUNK.slice(0, 12) }] } }); // The sibling merges its head; its announcement weighs the member's record. The member's gate is written only by what // that weighing decides, through the writer. const pulled = { number: 114, title: "", html_url: "", merged: true, state: "closed", head_sha: SIBLING_HEAD, merge_commit_sha: HOST_MERGE }; diff --git a/modules/gitea/test/siblings.test.ts b/modules/gitea/test/siblings.test.ts index edadee27..0562b635 100644 --- a/modules/gitea/test/siblings.test.ts +++ b/modules/gitea/test/siblings.test.ts @@ -10,7 +10,7 @@ import { mkdtempSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; -import { approvedHeadsOf, besideWords, groupRefusal, keepSiblings, namesSiblings, noteVerdict, readSiblings, siblingOutOfDate, siblingsFile, +import { approvedHeadsOf, besideWords, groupRefusal, keepSiblings, namesSiblings, readSiblings, siblingOutOfDate, siblingsFile, siblingsOf, siblingStatuses, weighSiblings, type SiblingRecord } from "../dist/siblings.js"; import { weighTrunks } from "../dist/trunk.js"; @@ -21,15 +21,11 @@ const verdict = { owner: "novox", repo: "mesh-controller", number: 346, commit: trunk: h("1").slice(0, 12), siblings: [sibling], judge_from: "sibling novox/mesh-controller#346 merge 123456789abc", gate: { verdict: "pass", summary: "every machine composes ".repeat(10) }, "repo-check": { verdict: "pass", summary: "ok" } }; -test("the siblings a verdict names are kept per member, replaced by its next verdict, and removed by one that read none", () => { +test("the siblings a verdict names are kept per member, and read back from where the holder keeps its state", () => { const file = siblingsFile(mkdtempSync(join(tmpdir(), "gitea-siblings-"))); - const record: SiblingRecord = {}; - assert.equal(noteVerdict(record, "novox/mesh-controller#346", MEMBER_HEAD, []), false); - assert.equal(noteVerdict(record, "novox/mesh-controller#346", MEMBER_HEAD, siblingsOf([sibling])), true); + const record: SiblingRecord = { "novox/mesh-controller#346": { head: MEMBER_HEAD, siblings: siblingsOf([sibling]) } }; keepSiblings(file, record); assert.deepEqual(readSiblings(file), record, "kept where the holder keeps its state"); - assert.equal(noteVerdict(record, "novox/mesh-controller#346", MEMBER_HEAD, []), true); - assert.deepEqual(record, {}); assert.deepEqual(readSiblings(null), {}); }); @@ -241,6 +237,10 @@ const composedMembers = () => [ { owner: "novox", repo: "mesh-controller", number: 346, commit: MEMBER_HEAD, trunk: "111111111111", base: "main" }, { owner: "novox", repo: "mesh-host", number: 114, commit: HOST_NEW, trunk: HOST_BASE.slice(0, 12), base: "main" }, ]; +/** The record a group's composed verdict once left: for each member, the other members at the heads and base commits it composed. */ +const composedRecord = (members: ReturnType): SiblingRecord => Object.fromEntries(members.map((m) => [ + `${m.owner}/${m.repo}#${m.number}`.toLowerCase(), { head: m.commit, siblings: members.filter((o) => o !== m).map((o) => ({ + repository: `${o.owner}/${o.repo}`, number: o.number, head: o.commit, base: o.base, base_commit: o.trunk })) }])); const staleRecord = (): SiblingRecord => ({ "novox/mesh-controller#346": { head: MEMBER_HEAD, siblings: siblingsOf([sibling]) } }); test("without the composed verdict's siblings, a sibling's own merge of its new head is a move (the pin of issue 1049)", async () => { @@ -252,9 +252,7 @@ test("without the composed verdict's siblings, a sibling's own merge of its new }); test("a composed verdict names the siblings of each member at the heads and base commits it composed, so a sibling's own merge of its new head is no move (issue 1049)", async () => { - const { noteComposed } = await import("../dist/siblings.js"); - const record = staleRecord(); - assert.equal(noteComposed(record, composedMembers()), true); + const record = composedRecord(composedMembers()); assert.deepEqual(record["novox/mesh-controller#346"], { head: MEMBER_HEAD, siblings: [{ repository: "novox/mesh-host", number: 114, head: HOST_NEW, base: "main", base_commit: HOST_BASE.slice(0, 12) }] }); assert.deepEqual(record["novox/mesh-host#114"], { head: HOST_NEW, @@ -267,21 +265,9 @@ test("a composed verdict names the siblings of each member at the heads and base assert.equal(record["novox/mesh-controller#346"].siblings[0].merged, true); // A merge of any other head, or onto any other base, is still a move. const other = world(); - const again: SiblingRecord = {}; - noteComposed(again, composedMembers()); + const again = composedRecord(composedMembers()); Object.assign(other.host, { merged: true, state: "closed", merge_commit_sha: MERGE, head_sha: HOST_NEW }); other.parents = [HOST_MOVED, HOST_NEW]; await weigh(other, again); assert.equal(other.written.length, 2); }); - -test("a composed verdict keeps nothing for a member it cannot name, and a group of one is none (issue 1049)", async () => { - const { noteComposed } = await import("../dist/siblings.js"); - const record: SiblingRecord = {}; - const [a, b] = composedMembers(); - assert.equal(noteComposed(record, [a]), false); - assert.equal(noteComposed(record, [a, { ...b, number: undefined }]), false, "a member with no pull request number"); - assert.equal(noteComposed(record, [a, { ...b, trunk: undefined }]), false, "a member with no trunk commit"); - assert.equal(noteComposed(record, [a, { ...b, base: undefined }]), false, "a member with no base branch"); - assert.deepEqual(record, {}); -}); diff --git a/modules/gitea/test/trunk.test.ts b/modules/gitea/test/trunk.test.ts index 2e4b242f..2323f0af 100644 --- a/modules/gitea/test/trunk.test.ts +++ b/modules/gitea/test/trunk.test.ts @@ -6,7 +6,7 @@ import { test } from "node:test"; import assert from "node:assert/strict"; -import { checkOnItsWay, lateVerdictComment, outOfDateStatuses, outOfDateWords, trunkNamed, verdictAtTrunk, weighTrunks } from "../dist/trunk.js"; +import { checkOnItsWay, outOfDateStatuses, outOfDateWords, trunkNamed, verdictAtTrunk, weighTrunks } from "../dist/trunk.js"; import { GateWriter } from "../dist/gate.js"; const HEAD = "dfb1415aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"; @@ -60,8 +60,6 @@ function aForge(pulls: { number: number; base: string; head: string }[], heads: writes.push(`${sha} ${s.context} ${s.state}`); statuses.set(sha, { ...statuses.get(sha), [s.context]: { state: s.state, description: s.description } }); }, - gateOf: async () => undefined, - gateHistory: async () => [], headChecks: async (_o: string, _r: string, sha: string) => statuses.get(sha) ?? {}, }; return { forge, writes, statuses }; @@ -111,24 +109,6 @@ test("a trunk that cannot be read is weighed again on the next pass, never recor assert.equal(trunks["novox/mesh-host:main"], TRUNK, "not recorded: the next pass weighs it again"); }); -test("the trunk's move takes its turn after a verdict in flight, and weighs what the forge holds then", async () => { - const heads: Record = { main: TRUNK }; - const { forge, statuses } = aForge([{ number: 1, base: "main", head: HEAD }], heads); - const gates = new GateWriter(forge); - // A verdict judged against the trunk's new head, in flight when the move is weighed: it lands first, and is left. - let release!: () => void; - const held = new Promise((r) => (release = r)); - const verdictLanded = gates.verdict("novox", "mesh-host", HEAD, async () => { - await held; - statuses.set(HEAD, { "mesh/merge-gate": { state: "success", description: `head x merged into trunk ${MOVED.slice(0, 12)} — pass` } }); - }); - const moved = gates.outOfDate("novox", "mesh-host", HEAD, (checks) => outOfDateStatuses(checks, MOVED)); - release(); - await verdictLanded; - assert.deepEqual(await moved, [], "a verdict judged against the trunk's head is not put out of date"); - assert.equal(statuses.get(HEAD)!["mesh/merge-gate"].state, "success"); -}); - test("a verdict judged against a trunk newer than the pass's first read is not put out of date", async () => { const NEWER = "ffffffffffffffffffffffffffffffffffffffff"; const heads: Record = { main: TRUNK }; @@ -142,15 +122,6 @@ test("a verdict judged against a trunk newer than the pass's first read is not p assert.equal(statuses.get(HEAD)!["mesh/merge-gate"].state, "success", "a fresh verdict was put out of date"); }); -test("a verdict that landed out of date keeps its account on the pull request", () => { - const words = outOfDateWords(TRUNK, MOVED); - const own = lateVerdictComment({ commit: HEAD, trunk: TRUNK.slice(0, 12), verdict: "fail" }, "**Merge check** at `dfb1415a`\n\nthe report", words); - assert.match(own, /the report/); - assert.match(own, /\*\*Out of date when it landed\*\*: out of date: judged against trunk a2c2c35bbbbb/); - const bare = lateVerdictComment({ commit: HEAD, trunk: TRUNK.slice(0, 12), verdict: "pass" }, null, words); - assert.match(bare, /head dfb1415aaaaa merged into trunk a2c2c35bbbbb, pass/); -}); - test("a release branch's move puts every open pull request into it out of date, as a trunk's does (ADR 0358 §5)", async () => { // novox/hq ADR 0358 §5: for a pull request into a release branch, the release branch takes the trunk's place in ADR // 0323 — the check merges the head onto the release branch's head and names it as the trunk commit, so the release diff --git a/modules/gitea/trunk.ts b/modules/gitea/trunk.ts index 55866f30..608e5805 100644 --- a/modules/gitea/trunk.ts +++ b/modules/gitea/trunk.ts @@ -19,8 +19,7 @@ // // Pure functions and two small files of the module's own state, so this is tested without a forge. -import { existsSync, mkdirSync, readFileSync, renameSync, writeFileSync } from "node:fs"; -import { dirname } from "node:path"; +import { existsSync, readFileSync } from "node:fs"; import type { CommitStatus, GiteaClient, GiteaPull, HeadCheck } from "./client.js"; import { CHECK_CONTEXT, WAITS_FOR_COMPOSED, maintained, requiredOf } from "./pulls.js"; @@ -112,14 +111,6 @@ export function askedWords(at: Date, trunkHead: string): string { `against trunk ${twelve(trunkHead)}`; } -/** The comment a verdict that landed out of date leaves (ADR 0323 §7: kept and shown): its own account when it has - * one, else what it judged and said, and that it is out of date. */ -export function lateVerdictComment(c: { commit: string; trunk?: string; verdict: string }, account: string | null, - outOfDate: string): string { - const own = account ?? `**Merge check** at \`${c.commit.slice(0, 8)}\`: ${judgedWords(c.commit, c.trunk ?? "")}, ${c.verdict}`; - return `${own}\n\n**Out of date when it landed**: ${outOfDate}`; -} - // --- a trunk's move, weighed -------------------------------------------------------------------------------------- /** What weighing a trunk's move reads of the forge. */ @@ -204,20 +195,6 @@ function readTookAll(file: string | null): Record { } } -/** Keep how long a repository's check took. A record that cannot be written is said, never fatal to the verdict. */ -export function keepTook(file: string | null, repository: string, took: Took): void { - if (!file || !took.took) return; - try { - const all = readTookAll(file); - all[repository.toLowerCase()] = took; - mkdirSync(dirname(file), { recursive: true }); - writeFileSync(file + ".tmp", JSON.stringify(all)); - renameSync(file + ".tmp", file); - } catch (err) { - console.error(`[gitea] how long ${repository}'s check took could not be kept — ${err instanceof Error ? err.message : err}`); - } -} - /** How long a repository's last check took, in words for the merge tool's refusal. */ export function tookWords(file: string | null, repository: string): string { const t = readTookAll(file)[repository.toLowerCase()]; From 080aed68556aab92354f52111f19497737213a92 Mon Sep 17 00:00:00 2001 From: jochen Date: Wed, 4 Nov 2026 11:06:48 +0100 Subject: [PATCH 7/7] gitea: the merge refusal no longer says how long the last check took; nothing writes that record (issue 1126) Claude-Session: https://claude.ai/code/session_01JG73KG7AfSdh6FP8VrBqCn --- modules/gitea/merge.ts | 9 ++++----- modules/gitea/test/guard.test.ts | 9 +++------ modules/gitea/trunk.ts | 33 +------------------------------- 3 files changed, 8 insertions(+), 43 deletions(-) diff --git a/modules/gitea/merge.ts b/modules/gitea/merge.ts index 0505aeb7..96f6d69a 100644 --- a/modules/gitea/merge.ts +++ b/modules/gitea/merge.ts @@ -20,7 +20,7 @@ import { MERGE_METHODS, Refused, checkName } from "./guard.js"; import { CHECK_CONTEXT, CHECK_STATUS, REPO_CHECK_CONTEXT, maintained, requiredOf } from "./pulls.js"; import { approvedHeadsOf, groupRefusal, memberKey, namesSiblings, notApproved, readSiblings, siblingOutOfDate, siblingsFile } from "./siblings.js"; import { checkGatesMerge } from "./mainrules.js"; -import { announcementOf, askedWords, outOfDateWords, sameCommit, tookFile, tookWords, twelve, verdictAtTrunk, waitsForComposed } from "./trunk.js"; +import { announcementOf, askedWords, outOfDateWords, sameCommit, twelve, verdictAtTrunk, waitsForComposed } from "./trunk.js"; /** A head named for a merge: at least 12 hex characters of its sha (novox/hq issue 381), up to a sha-256 commit's 64. */ const REVIEWED_HEAD = /^[0-9a-f]{12,64}$/; @@ -47,7 +47,7 @@ export function mergeTool(gitea: GiteaClient, given: MergeDeps): ToolDefinition "optionally deleting the source branch afterwards. Refused " + "otherwise, with nothing merged: no tool merges past a check, and none merges a head that moved after it was " + "reviewed. A verdict judged against an earlier trunk commit is refused, naming both, and the head's check against " + - "the trunk's head is asked once, with how long the last check of the repository took: ask again at the same head " + + "the trunk's head is asked once: ask again at the same head " + "once it passes — the tool never merges by itself.", input: { owner: { type: "string", description: "the repository owner" }, @@ -163,7 +163,6 @@ export function mergeTool(gitea: GiteaClient, given: MergeDeps): ToolDefinition const siblingWords = siblingOutOfDate(checks[judge]?.description) ? checks[judge]?.description : undefined; const judged = siblingWords ? `was put out of date by a sibling's move (${siblingWords})` : at.judged ? `was judged against trunk ${twelve(at.judged)}` : "names no trunk commit it was judged against"; - const took = tookWords(tookFile(deps.stateDir), `${owner}/${repo}`); const head = twelve(pull.head_sha), now = twelve(trunkHead); // **A member that waits for its group's composed check is not announced again** (novox/hq issue 1106): its own // check can only say "composed" once more, so an ask here brought a verdict naming no trunk and the next merge was @@ -176,7 +175,7 @@ export function mergeTool(gitea: GiteaClient, given: MergeDeps): ToolDefinition if (at.onItsWay) { throw new Refused(`${owner}/${repo}#${number} is not merged: its verdict ${judged}, and ${pull.base} is now ` + `${now}. Its check is already asked and not yet answered (${checks[judge]?.description}), so it is not ` + - `asked again; ${took}. Ask again at ${head} once both statuses pass: this tool never merges by itself ` + + `asked again. Ask again at ${head} once both statuses pass: this tool never merges by itself ` + `(novox/hq ADR 0323). A check that never answers is asked again with mesh-delivery.recheck. Nothing was merged`); } // Pending first, so a second ask finds the check on its way; then the head is announced as a new head is. An @@ -203,7 +202,7 @@ export function mergeTool(gitea: GiteaClient, given: MergeDeps): ToolDefinition `${now}, and its check could not be asked: ${err instanceof Error ? err.message : err}. Nothing was merged`); } throw new Refused(`${owner}/${repo}#${number} is not merged: its verdict ${judged}, and ${pull.base} is now ` + - `${now}. The check of head ${head} merged into trunk ${now} was asked; ${took}. Ask again at ${head} once both ` + + `${now}. The check of head ${head} merged into trunk ${now} was asked. Ask again at ${head} once both ` + `statuses pass: this tool never merges by itself (novox/hq ADR 0323). In a busy repository another merge may ` + `move the trunk again first; this is then refused and asked once more. A check that never answers is asked ` + `again with mesh-delivery.recheck. Nothing was merged`); diff --git a/modules/gitea/test/guard.test.ts b/modules/gitea/test/guard.test.ts index 8b080200..3b93f56e 100644 --- a/modules/gitea/test/guard.test.ts +++ b/modules/gitea/test/guard.test.ts @@ -7,7 +7,7 @@ import { test, after, beforeEach } from "node:test"; import assert from "node:assert/strict"; import { createServer, type IncomingMessage, type ServerResponse } from "node:http"; -import { mkdtempSync, readFileSync, writeFileSync } from "node:fs"; +import { mkdtempSync, readFileSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; @@ -298,18 +298,15 @@ test("gitea_merge_pull_request merges only a head whose required statuses all su // **The merge tool refuses a verdict whose trunk commit is not the trunk's head, and asks the check itself** (novox/hq // ADR 0323 §5, issue 341): node-engine #66 merged at 00:54 on a verdict of 00:49 that judged its head alone, after #65 // had moved the trunk. Both commits are named; one check is asked, the announcement a new head makes, and not a second -// while it is on its way; the refusal says how long the last check of the repository took; and once the verdict judges +// while it is on its way; and once the verdict judges // the trunk's head and passes, the merge goes through — asked again, never by itself. test("gitea_merge_pull_request refuses a verdict judged against an earlier trunk, and asks the check once", async () => { const merge = () => tool("gitea_merge_pull_request").run({ owner: "novox", repo: "mesh-catalog", number: 7, head: REVIEWED }); - const { tookFile } = await import("../dist/trunk.js"); - writeFileSync(tookFile(stateDir)!, JSON.stringify({ "novox/mesh-catalog": { took: "4m31s", at: "2026-10-12T14:49:00Z" } })); state.trunk = MOVED; // another merge moved the trunk after the verdict await assert.rejects(merge(), (err: Error) => { assert.match(err.message, /is not merged: its verdict was judged against trunk 7a8b9c0d1e2f, and main is now 3c4d5e6f7a8b/); assert.match(err.message, /check of head abc1234def56 merged into trunk 3c4d5e6f7a8b was asked/); - assert.match(err.message, /the last check of novox\/mesh-catalog took 4m31s/); assert.match(err.message, /never merges by itself/); return true; }); @@ -329,7 +326,7 @@ test("gitea_merge_pull_request refuses a verdict judged against an earlier trunk assert.equal(state.statuses.find((s) => s.context === "mesh/repo-check")!.state, "pending"); // Asked again while that check is on its way: refused, and nothing is asked a second time. - await assert.rejects(merge(), /already asked and not yet answered .*not asked again; the last check of novox\/mesh-catalog took 4m31s.*mesh-delivery\.recheck/); + await assert.rejects(merge(), /already asked and not yet answered .*not asked again.*mesh-delivery\.recheck/); assert.equal(state.emitted.length, 1, "no second check while one is on its way"); state.statuses = [{ ...gate, description: "building on ace since 15:02" }, ...state.statuses.slice(1)]; await assert.rejects(merge(), /not asked again/); diff --git a/modules/gitea/trunk.ts b/modules/gitea/trunk.ts index 608e5805..c1e2d98d 100644 --- a/modules/gitea/trunk.ts +++ b/modules/gitea/trunk.ts @@ -10,7 +10,7 @@ // Nothing is run for it: a recheck, a new head, or the merge tool asks the check again; // - **the merge tool** (tools/index.ts) compares the trunk commit the head's verdict names with the trunk's head at // the moment it is asked, merges nothing when they differ, asks the head's check once — the same announcement a -// new head makes — and says how long the last check of that repository took. It never merges by itself after. +// new head makes — and never merges by itself after. // // Since issue 1301 (ADR 0447 §4) a main that requires mesh/check is the checker's: its line names the trunk commit it judged // ("pass on "), the checker sets it out of date when the trunk moves, and the merge tool refuses a verdict @@ -19,7 +19,6 @@ // // Pure functions and two small files of the module's own state, so this is tested without a forge. -import { existsSync, readFileSync } from "node:fs"; import type { CommitStatus, GiteaClient, GiteaPull, HeadCheck } from "./client.js"; import { CHECK_CONTEXT, WAITS_FOR_COMPOSED, maintained, requiredOf } from "./pulls.js"; @@ -172,36 +171,6 @@ export async function weighTrunks(forge: TrunkForge, gates: OutOfDateWriter, tru return changed; } -// --- how long the last check of a repository took ------------------------------------------------------------- - -/** How long one repository's last check took, and when it was heard: kept by the watcher, read by the merge tool. */ -export interface Took { - took: string; - at: string; - head?: string; -} - -/** The file the watcher keeps each repository's last check's duration in, beside its other records. */ -export function tookFile(stateDir: string | undefined): string | null { - return stateDir ? `${stateDir.replace(/\/+$/, "")}/checks-took.json` : null; -} - -function readTookAll(file: string | null): Record { - if (!file || !existsSync(file)) return {}; - try { - return JSON.parse(readFileSync(file, "utf8")) as Record; - } catch { - return {}; - } -} - -/** How long a repository's last check took, in words for the merge tool's refusal. */ -export function tookWords(file: string | null, repository: string): string { - const t = readTookAll(file)[repository.toLowerCase()]; - if (!t) return `how long a check of ${repository} takes is not known yet: no verdict of it was heard since this was kept`; - return `the last check of ${repository} took ${t.took} (heard ${t.at})`; -} - // --- the announcement of a head, shared by the watcher and the merge tool --------------------------------------- /** `pull.updated` for one open pull request's head: what the controller asks the build seat to check (novox/hq to-be