diff --git a/modules/gitea/README.md b/modules/gitea/README.md index 3031e848..bf775099 100644 --- a/modules/gitea/README.md +++ b/modules/gitea/README.md @@ -13,12 +13,11 @@ Every new head of an open pull request is announced as `pull.updated`, once, and retargeting a pull request moves its own update time and not its repository's, and a pull request opened more than a minute after its push was never announced (issue 347); and every repository on a sweep, every ten minutes, or when the search fails. -- **A pull request nobody checks is said.** Every five minutes the forge itself is asked for every open pull - request; one whose base requires `mesh/merge-gate` and whose head has had no such status for ten minutes is - said as this forge failing *the merge check of `/#`* (`provisioner.failing`, class - `unannounced`), which the controller keeps as a condition (ADR 0224), said again each pass while it lasts, and - recovered when a check arrives or the pull request closes. What is said is kept in `pulls-unannounced.json` - beside the announcer's record, so a restart recovers what it said. +- **A pull request nobody checks is said — by the delivery's owner, not here.** `gitea_open_pulls` answers every + open pull request from the forge itself, with the statuses its base requires and the ones its head has. The + `mesh-delivery` module asks it every five minutes, and a head on a base requiring `mesh/merge-gate` or + `mesh/repo-check` with neither for ten minutes since it first saw it so is a delivery held past its bound + (`delivery..stalled`, state `unannounced`), which the controller raises like any other (issue 347). - **How to see it work.** A pull request opened more than a minute after its branch was pushed — this fix's own was one, and the announcer it fixes never saw it — has `mesh/merge-gate` pending within a pass of the next sweep at the latest, and `mesh-controller.conditions` names none for it. diff --git a/modules/gitea/client.ts b/modules/gitea/client.ts index 4458796b..8c926dab 100644 --- a/modules/gitea/client.ts +++ b/modules/gitea/client.ts @@ -54,6 +54,20 @@ export interface GiteaPull { body?: string; } +/** An open pull request with what its base requires and what its head has: what gitea_open_pulls answers. */ +export interface OpenPull { + owner: string; + repo: string; + number: number; + base: string; + head_sha: string; + html_url?: string; + /** The contexts the base branch's protection requires; none when it requires none, or cannot be read. */ + required: string[]; + /** The contexts the head has a status of, in any state. */ + statuses: string[]; +} + /** A commit status, as the forge keeps it: what a pull request shows beside its head commit. */ export interface CommitStatus { state: "pending" | "success" | "error" | "failure" | "warning"; @@ -280,6 +294,26 @@ export class GiteaClient { return out; } + /** **Every open pull request, from the forge itself** (novox/hq issue 347): with what its base's protection + * requires and the contexts its head has a status of, in any state. What mesh-delivery reads to say a pull + * request the mesh was never asked to check; asked of the forge, never of what the announcer remembers, so a + * fault in the announcer cannot hide itself. Reads only. */ + async openPulls(): Promise { + const out: OpenPull[] = []; + const rules = new Map(); + for (const found of await this.searchOpenPulls()) { + const full = `${found.owner}/${found.name}`; + const pull = await this.getPullRequest(found.owner, found.name, found.number); + if (pull.state !== "open" || !pull.head_sha) continue; + if (!rules.has(full)) rules.set(full, await this.branchProtections(found.owner, found.name).catch(() => undefined)); + const rule = rules.get(full)?.find((p) => (p.rule_name ?? p.branch_name) === (pull.base || "main")); + const statuses = Object.keys(await this.commitStatuses(found.owner, found.name, pull.head_sha)); + out.push({ owner: found.owner, repo: found.name, number: pull.number, base: pull.base ?? "", head_sha: pull.head_sha, + html_url: pull.html_url, required: rule?.enable_status_check ? (rule.status_check_contexts ?? []) : [], statuses }); + } + return out; + } + async getPullRequest(owner: string, repo: string, index: number): Promise { indexOf(index); return GiteaClient.mapPull(await this.request(`/repos/${owner}/${repo}/pulls/${index}`)); diff --git a/modules/gitea/index.ts b/modules/gitea/index.ts index 934960ba..560198f7 100644 --- a/modules/gitea/index.ts +++ b/modules/gitea/index.ts @@ -28,7 +28,7 @@ import { emit, on } from "@novox/mesh-sdk/events"; import { GiteaClient, movedSince } from "./client.js"; import { CHECK_CONTEXT, REPO_CHECK_CONTEXT, commentFor, headsToAnnounce, statusesFor, type Announced, type Checked, - type RepoCheckFacts, reposToLook, unannounced, type OpenPull } from "./pulls.js"; + type RepoCheckFacts, reposToLook } from "./pulls.js"; // Without a way to a token — configured, or mintable with the admin account (token.ts) — there is // nothing to watch; log and stay quiet rather than crash the runtime. With one, the first poll mints @@ -290,68 +290,6 @@ async function pollPulls(client: GiteaClient): Promise { lastPullLook = began; } -// **An open pull request nobody checks is said** (novox/hq issue 347). One whose base requires the merge check and -// whose head has had none for ten minutes was announced by nobody — the fault this watcher exists for, which went -// unseen while the pull request could not merge. Said as this forge failing the merge check, the provider standing -// the controller keeps as a condition (ADR 0224) and says to the operator; said again every pass while it lasts, and -// recovered when a check arrives or the pull request is no longer open. Asked of the forge itself, never of what -// the announcer remembers, so a fault in the announcer cannot hide itself. -const unannouncedRecord = process.env.MESH_GITEA_STATE_DIR ? join(process.env.MESH_GITEA_STATE_DIR, "pulls-unannounced.json") : null; -let failing: Record = {}; -if (unannouncedRecord && existsSync(unannouncedRecord)) { - try { - failing = (JSON.parse(readFileSync(unannouncedRecord, "utf8")) as { failing: Record }).failing ?? {}; - } catch { - // Unreadable is no record: a pull request still unchecked is said again at once. - } -} -function keepFailing(): void { - if (!unannouncedRecord) return; - mkdirSync(join(unannouncedRecord, ".."), { recursive: true }); - const tmp = unannouncedRecord + ".tmp"; - writeFileSync(tmp, JSON.stringify({ failing })); - renameSync(tmp, unannouncedRecord); -} -const consumerOf = (key: string) => `the merge check of ${key}`; -async function watchUnannounced(client: GiteaClient): Promise { - const open: OpenPull[] = []; - const rules = new Map> | undefined>(); - for (const found of await client.searchOpenPulls()) { - const full = `${found.owner}/${found.name}`; - const pull = await client.getPullRequest(found.owner, found.name, found.number); - if (pull.state !== "open" || !pull.head_sha) continue; - if (!rules.has(full)) rules.set(full, await client.branchProtections(found.owner, found.name).catch(() => undefined)); - const rule = rules.get(full)?.find((p) => (p.rule_name ?? p.branch_name) === (pull.base || "main")); - const statuses = Object.keys(await client.commitStatuses(found.owner, found.name, pull.head_sha)); - open.push({ key: `${full}#${pull.number}`, base: pull.base ?? "", head_sha: pull.head_sha, updated_at: pull.updated_at, - statuses, required: rule?.enable_status_check ? (rule.status_check_contexts ?? []) : [] }); - } - const now = Date.now(); - const unchecked = new Map(unannounced(open, now).map((p) => [p.key, p] as const)); - const node = process.env.MESH_NODE ?? ""; - for (const [key, p] of unchecked) { - const since = failing[key] ?? p.updated_at ?? new Date(now).toISOString(); - await emit("provisioner.failing", { - provider: "git", "provider-node": node, consumer: consumerOf(key), node: "", class: "unannounced", - error: `${key} is open on ${p.base}, which requires ${CHECK_CONTEXT}, and its head ${p.head_sha.slice(0, 12)} has had no ` + - `merge check for ${Math.round((now - Date.parse(p.updated_at ?? since)) / 60_000)} minutes: the mesh was never asked to check it`, - since, attempts: 1, - }); - if (!failing[key]) console.error(`[gitea] ${key} has had no merge check for ten minutes: said to the mesh`); - failing[key] = since; - } - for (const key of Object.keys(failing)) { - if (unchecked.has(key)) continue; - await emit("provisioner.recovered", { - provider: "git", "provider-node": node, consumer: consumerOf(key), node: "", since: failing[key], attempts: 0, - why: "it has a merge check, or is no longer open", - }); - console.log(`[gitea] ${key} has a merge check now, or is no longer open`); - delete failing[key]; - } - keepFailing(); -} - // **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. @@ -425,7 +363,6 @@ if (gitea) { tick(() => pollRepos(client), 60_000); tick(() => pollMerged(client), 30_000); tick(() => pollPulls(client), 30_000); - tick(() => watchUnannounced(client), 5 * 60_000); await on("mesh-controller.checked", (event) => setVerdict(client, event)); console.log("[gitea] watching for new repositories and merged pull requests"); } diff --git a/modules/gitea/pulls.ts b/modules/gitea/pulls.ts index 7020fa38..3f45f08f 100644 --- a/modules/gitea/pulls.ts +++ b/modules/gitea/pulls.ts @@ -113,31 +113,6 @@ export function reposToLook(repos: GiteaRepo[], floor: string, pulls: PullSeen[] return repos.filter((r) => moved.has(r.full_name)); } -/** How long a head may go without a merge check before it is said: the announcer's passes are half a minute - * apart and the check is asked at once, so ten minutes is a fault, never a queue. */ -export const UNANNOUNCED_AFTER_MS = 10 * 60_000; - -/** An open pull request as the watcher of announcements reads it. */ -export interface OpenPull { - /** `owner/repo#number`. */ - key: string; - base: string; - head_sha: string; - /** When its head last moved, or it was opened or retargeted. */ - updated_at?: string; - /** The status contexts its head has, any state. */ - statuses: string[]; - /** The contexts its base branch's protection requires. */ - required: string[]; -} - -/** The open pull requests whose base requires the merge check and whose head has had none for ten minutes - * (novox/hq issue 347): announced by nobody, checked by nothing, and blocked from merging with no word why. */ -export function unannounced(pulls: OpenPull[], now: number, after = UNANNOUNCED_AFTER_MS): OpenPull[] { - return pulls.filter((p) => p.required.includes(CHECK_CONTEXT) && !p.statuses.includes(CHECK_CONTEXT) && - !!p.updated_at && now - Date.parse(p.updated_at) >= after); -} - // **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 diff --git a/modules/gitea/test/announcing.test.ts b/modules/gitea/test/announcing.test.ts index a6093e16..ac607bb8 100644 --- a/modules/gitea/test/announcing.test.ts +++ b/modules/gitea/test/announcing.test.ts @@ -28,17 +28,47 @@ test("a pull request opened after the margin is looked at, by its own update tim assert.deepEqual(reposToLook(repos, "", [], false).length, 2, "the first pass looks at every repository"); }); -test("an open pull request on a protected branch with no merge check for ten minutes is said, and nothing else", async () => { - const { unannounced } = await import("../pulls.ts"); - const now = Date.parse("2026-10-09T10:30:00Z"); - const pull = { - key: "novox/mesh-sdk#14", base: "main", head_sha: "bfe82315f42c", updated_at: "2026-10-09T10:12:00Z", - statuses: [] as string[], required: ["mesh/merge-gate", "mesh/repo-check"], - }; - assert.deepEqual(unannounced([pull], now).map((p) => p.key), ["novox/mesh-sdk#14"]); - assert.deepEqual(unannounced([{ ...pull, statuses: ["mesh/merge-gate"] }], now), [], "a head with a merge check"); - assert.deepEqual(unannounced([{ ...pull, required: [] }], now), [], "a branch that requires no merge check"); - assert.deepEqual(unannounced([{ ...pull, updated_at: "2026-10-09T10:25:00Z" }], now), [], "a head five minutes old"); +test("every open pull request is read with what its base requires and what its head has (gitea_open_pulls)", async () => { + const { GiteaClient } = await import("../client.ts"); + const asked: string[] = []; + const server = createServer((req, res) => { + asked.push(req.url ?? ""); + const url = req.url ?? ""; + const send = (v: unknown) => res.end(JSON.stringify(v)); + if (url.startsWith("/api/v1/repos/issues/search")) { + return send(url.includes("page=1") ? [ + { number: 243, repository: { owner: "novox", name: "hq", full_name: "novox/hq" }, updated_at: "2026-10-09T11:00:00Z" }, + { number: 14, repository: { owner: "novox", name: "mesh-sdk", full_name: "novox/mesh-sdk" } }, + ] : []); + } + if (url.startsWith("/api/v1/repos/novox/hq/pulls/243")) { + return send({ number: 243, state: "open", base: { ref: "main" }, head: { ref: "x", sha: "aaa111" }, html_url: "h" }); + } + if (url.startsWith("/api/v1/repos/novox/mesh-sdk/pulls/14")) { + return send({ number: 14, state: "open", base: { ref: "main" }, head: { ref: "y", sha: "bbb222" }, html_url: "s" }); + } + if (url.startsWith("/api/v1/repos/novox/hq/branch_protections")) { + return send([{ rule_name: "main", enable_status_check: true, status_check_contexts: ["mesh/repo-check"] }]); + } + if (url.startsWith("/api/v1/repos/novox/mesh-sdk/branch_protections")) { + return send([{ rule_name: "main", enable_status_check: true, status_check_contexts: ["mesh/merge-gate", "mesh/repo-check"] }]); + } + if (url.includes("/commits/aaa111/status")) return send({ statuses: [] }); + if (url.includes("/commits/bbb222/status")) return send({ statuses: [{ context: "mesh/merge-gate", status: "pending" }] }); + res.statusCode = 404; + res.end("{}"); + }); + await new Promise((r) => server.listen(0, r)); + const client = new GiteaClient(`http://127.0.0.1:${(server.address() as any).port}`, "t"); + try { + const open = await client.openPulls(); + assert.deepEqual(open.map((p) => [p.owner, p.repo, p.number, p.base, p.head_sha, p.required, p.statuses]), [ + ["novox", "hq", 243, "main", "aaa111", ["mesh/repo-check"], []], + ["novox", "mesh-sdk", 14, "main", "bbb222", ["mesh/merge-gate", "mesh/repo-check"], ["mesh/merge-gate"]], + ]); + } finally { + server.close(); + } }); test("a pull request or issue number that is not one is refused before the forge is asked", async () => { diff --git a/modules/gitea/test/token.test.ts b/modules/gitea/test/token.test.ts index c6f3b4a7..aa76e7f4 100644 --- a/modules/gitea/test/token.test.ts +++ b/modules/gitea/test/token.test.ts @@ -411,7 +411,7 @@ test("the tools register once there is a way to a token, and the first call mint "gitea_list_branches", "gitea_delete_branch", "gitea_branch_protection_get", "gitea_branch_protection_set", - "gitea_note_append", "gitea_delivery_view", "gitea_commit_statuses", "gitea_contains", "gitea_commit_status", + "gitea_note_append", "gitea_delivery_view", "gitea_commit_statuses", "gitea_open_pulls", "gitea_contains", "gitea_commit_status", "gitea_list_labels", "gitea_create_label", "gitea_api", ], diff --git a/modules/gitea/tools/index.ts b/modules/gitea/tools/index.ts index 33b70d41..9bb1d68c 100644 --- a/modules/gitea/tools/index.ts +++ b/modules/gitea/tools/index.ts @@ -492,6 +492,12 @@ export function getGiteaTools(gitea: GiteaClient): ToolDefinition[] { merge: mergeableUnder(base, rule, statuses, unreadable) }; }, }, + { + name: "gitea_open_pulls", + description: "Every open pull request of every repository, from the forge itself: its head, its base, the statuses the base's protection requires and the ones its head has, in any state. What mesh-delivery reads to say a pull request the mesh was never asked to check (novox/hq issue 347). Reads only.", + input: {}, + run: async () => ({ pulls: await gitea.openPulls() }), + }, { name: "gitea_contains", description: "Whether a commit is contained in another — an ancestor of it, or the same — from the forge's compare of the two: what mesh-delivery reads before it says a held merge is covered by a later one (novox/hq ADR 0270). Commits only, by sha or the start of one. Reads only.", diff --git a/modules/mesh-delivery/cmd/mesh-delivery/fakes_test.go b/modules/mesh-delivery/cmd/mesh-delivery/fakes_test.go index 26a02a39..c110d819 100644 --- a/modules/mesh-delivery/cmd/mesh-delivery/fakes_test.go +++ b/modules/mesh-delivery/cmd/mesh-delivery/fakes_test.go @@ -239,6 +239,18 @@ type fakeForge struct { contains map[string]bool containsDown bool containsAsks []string + // open is what OpenPulls answers; openDown a forge that cannot say. + open []OpenPull + openDown bool +} + +func (f *fakeForge) OpenPulls() ([]OpenPull, error) { + f.mu.Lock() + defer f.mu.Unlock() + if f.openDown { + return nil, errors.New("the forge is away") + } + return append([]OpenPull(nil), f.open...), nil } func newFakeForge() *fakeForge { diff --git a/modules/mesh-delivery/cmd/mesh-delivery/holder.go b/modules/mesh-delivery/cmd/mesh-delivery/holder.go index 060c3c2a..0ed12ca6 100644 --- a/modules/mesh-delivery/cmd/mesh-delivery/holder.go +++ b/modules/mesh-delivery/cmd/mesh-delivery/holder.go @@ -41,6 +41,8 @@ type Holder struct { // askTried is when a check this owner asks was last tried and could not be asked: tried again a minute on. askTried map[string]time.Time refusals []string + // unchecked are the open pull requests' heads seen without a merge check (unchecked.go). + unchecked map[string]unchecked } // unmatched is a walk no delivery is known for yet: its merge not heard, or merged before this owner held. @@ -80,6 +82,7 @@ func (h *Holder) init() { h.dirtyGroup = map[string]bool{} h.unmatched = map[string]unmatched{} h.askTried = map[string]time.Time{} + h.unchecked = map[string]unchecked{} h.started = h.Now() } diff --git a/modules/mesh-delivery/cmd/mesh-delivery/main.go b/modules/mesh-delivery/cmd/mesh-delivery/main.go index 20f3513b..be88762a 100644 --- a/modules/mesh-delivery/cmd/mesh-delivery/main.go +++ b/modules/mesh-delivery/cmd/mesh-delivery/main.go @@ -68,7 +68,7 @@ func run(h *Holder, l *listening) { go func() { tick := time.NewTicker(10 * time.Second) defer tick.Stop() - last := time.Now() + last, looked := time.Now(), time.Time{} for now := range tick.C { if now.Sub(last) >= 30*time.Second { last = now @@ -76,6 +76,11 @@ func run(h *Holder, l *listening) { logf("[mesh-delivery] the controller's walks cannot be read: %v", err) } } + // Every open pull request, from the forge itself, every five minutes (novox/hq issue 347). + if now.Sub(looked) >= 5*time.Minute { + looked = now + h.SeeOpenPulls() + } h.Tick() h.Flush() } diff --git a/modules/mesh-delivery/cmd/mesh-delivery/manifest_test.go b/modules/mesh-delivery/cmd/mesh-delivery/manifest_test.go index 43a2c526..d5d819f0 100644 --- a/modules/mesh-delivery/cmd/mesh-delivery/manifest_test.go +++ b/modules/mesh-delivery/cmd/mesh-delivery/manifest_test.go @@ -65,7 +65,7 @@ func TestItClaimsTheDeliverySeatAndFollowsWhatItHandles(t *testing.T) { want = append(want, "seat:"+ControllerSeat+"."+v) } want = append(want, "gitea.gitea_note_append", "gitea.gitea_delivery_view", "gitea.gitea_commit_status", - "gitea.gitea_commit_statuses", "gitea.gitea_contains") + "gitea.gitea_commit_statuses", "gitea.gitea_contains", "gitea.gitea_open_pulls") if !reflect.DeepEqual(m.Invokes, want) { t.Fatalf("invokes %v, wanted %v", m.Invokes, want) } diff --git a/modules/mesh-delivery/cmd/mesh-delivery/ports.go b/modules/mesh-delivery/cmd/mesh-delivery/ports.go index be2bb752..a9819fc7 100644 --- a/modules/mesh-delivery/cmd/mesh-delivery/ports.go +++ b/modules/mesh-delivery/cmd/mesh-delivery/ports.go @@ -78,6 +78,9 @@ type Forge interface { Statuses(owner, repo string, number int, commit, base string) (*ForgeChecks, error) // Contains is whether a commit is contained in another — an ancestor of it, or it. Reads only. Contains(owner, repo, commit, in string) (bool, error) + // OpenPulls is every open pull request, with what its base requires and what its head has (novox/hq issue + // 347). Reads only. + OpenPulls() ([]OpenPull, error) } // ForgeChecks is what the forge says of a commit's checks (the gitea module's gitea_commit_statuses). @@ -339,6 +342,24 @@ func (f toolForge) Statuses(owner, repo string, number int, commit, base string) return &c, nil } +func (f toolForge) OpenPulls() ([]OpenPull, error) { + raw, err := f.ask("gitea.gitea_open_pulls", map[string]any{}) + if err != nil { + return nil, err + } + answer, output, ok := answerOf(raw) + if !ok { + return nil, fmt.Errorf("the forge refused gitea_open_pulls: %s", lastLine(output)) + } + var open struct { + Pulls []OpenPull `json:"pulls"` + } + if err := json.Unmarshal(answer, &open); err != nil || open.Pulls == nil { + return nil, fmt.Errorf("the forge's gitea_open_pulls is not readable: %s", clip(string(answer), 200)) + } + return open.Pulls, nil +} + func (f toolForge) Contains(owner, repo, commit, in string) (bool, error) { raw, err := f.ask("gitea.gitea_contains", map[string]any{"owner": owner, "repo": repo, "commit": commit, "in": in}) if err != nil { diff --git a/modules/mesh-delivery/cmd/mesh-delivery/unchecked.go b/modules/mesh-delivery/cmd/mesh-delivery/unchecked.go new file mode 100644 index 00000000..dad89f81 --- /dev/null +++ b/modules/mesh-delivery/cmd/mesh-delivery/unchecked.go @@ -0,0 +1,112 @@ +package main + +import ( + "fmt" + "slices" + "time" +) + +// A pull request the mesh was never asked to check (novox/hq issue 347). +// +// The forge's announcer is the only way the mesh hears of a pull request's head, and on 2026-10-09 it missed one: a +// pull request opened after its push moved nothing the announcer read, and waited unchecked, unable to merge, with +// nothing saying so. This owner asks the forge itself for every open pull request every few minutes, and one whose +// base requires the merge check — mesh/merge-gate or mesh/repo-check — with neither on its head for UncheckedAfter +// since this owner first saw it so is said as a delivery held: a `stalled` line in the state Unannounced, which the +// controller raises as `delivery..stalled` (D14) like any other delivery past its bound. The delivery domain's own +// word, so no second condition and no second judge: the id is the one its delivery will have once announced. + +// Unannounced is a pull request's head the forge never announced: not a state of the table — no delivery exists — +// only what a stalled line says. +const Unannounced State = "unannounced" + +// UncheckedAfter is how long a head may go without a merge check before it is said: the announcer passes every half +// minute and the check is asked at once, so ten minutes is a fault, never a queue. +const UncheckedAfter = 10 * time.Minute + +// mergeChecks are the statuses that are the mesh's merge check (novox/hq ADR 0237): a base requiring either is +// one the mesh is meant to check. +var mergeChecks = []string{"mesh/merge-gate", "mesh/repo-check"} + +// OpenPull is an open pull request as the forge says it (the gitea module's gitea_open_pulls): its head, its base, +// the contexts the base's protection requires, and the contexts its head has a status of, in any state. +type OpenPull struct { + Owner string `json:"owner"` + Repo string `json:"repo"` + Number int `json:"number"` + Base string `json:"base"` + HeadSHA string `json:"head_sha"` + HTMLURL string `json:"html_url,omitempty"` + Required []string `json:"required"` + Statuses []string `json:"statuses"` +} + +// unchecked is a head seen without a merge check, and when this owner first saw it so — its own time, since the +// forge's update time moves on every comment. +type unchecked struct { + pull OpenPull + first time.Time +} + +// wantsACheck is whether the base requires the mesh's merge check and the head has none of it. +func (p OpenPull) wantsACheck() bool { + required := slices.ContainsFunc(mergeChecks, func(c string) bool { return slices.Contains(p.Required, c) }) + checked := slices.ContainsFunc(mergeChecks, func(c string) bool { return slices.Contains(p.Statuses, c) }) + return required && !checked && p.HeadSHA != "" +} + +// SeeOpenPulls asks the forge for every open pull request and keeps the heads that want a check and have none. +// A forge that cannot be asked changes nothing: what was seen stands, and nothing is guessed. +func (h *Holder) SeeOpenPulls() { + open, err := h.Forge.OpenPulls() + if err != nil { + h.Logf("[mesh-delivery] the forge's open pull requests cannot be read: %v", err) + return + } + h.mu.Lock() + defer h.mu.Unlock() + now := h.Now() + seen := map[string]unchecked{} + for _, p := range open { + if !p.wantsACheck() { + continue + } + id := IDOf(p.Owner+"/"+p.Repo, p.HeadSHA) + if h.deliveries[id] != nil { + continue // announced: a delivery with its own bound + } + first := now + if was, ok := h.unchecked[id]; ok { + first = was.first + } else { + h.Logf("[mesh-delivery] %s/%s#%d's head %s has no merge check; said if it has none in %s", p.Owner, p.Repo, + p.Number, short(p.HeadSHA), UncheckedAfter) + } + seen[id] = unchecked{pull: p, first: first} + } + h.unchecked = seen +} + +// uncheckedLines are the heads unchecked past UncheckedAfter, as stalled lines; under the lock. +func (h *Holder) uncheckedLines(now time.Time) []StalledLine { + var out []StalledLine + for id, u := range h.unchecked { + if h.deliveries[id] != nil || now.Sub(u.first) < UncheckedAfter { + continue + } + out = append(out, StalledLine{ID: id, State: Unannounced, For: now.Sub(u.first).Round(time.Second).String(), + Bound: UncheckedAfter.String(), + H2: "none: the forge never announced it, so there is no delivery to close — the operator's", + Says: fmt.Sprintf("%s/%s#%d is open on %s, which requires the merge check, and its head has had no merge "+ + "check: the mesh was never asked to check it. A new commit on its branch announces it", + u.pull.Owner, u.pull.Repo, u.pull.Number, u.pull.Base)}) + } + return out +} + +func short(sha string) string { + if len(sha) > 12 { + return sha[:12] + } + return sha +} diff --git a/modules/mesh-delivery/cmd/mesh-delivery/unchecked_test.go b/modules/mesh-delivery/cmd/mesh-delivery/unchecked_test.go new file mode 100644 index 00000000..992224df --- /dev/null +++ b/modules/mesh-delivery/cmd/mesh-delivery/unchecked_test.go @@ -0,0 +1,77 @@ +package main + +import ( + "strings" + "testing" + "time" +) + +// **A pull request the mesh was never asked to check is a delivery held, and said** (novox/hq issue 347): one open +// on a base that requires the merge check — mesh/merge-gate or mesh/repo-check, as hq requires only the second — +// whose head has had neither for ten minutes since this owner first saw it so, is a stalled line: the controller +// raises it as `delivery..stalled` (D14), the delivery domain's own word for a delivery held past its bound. +func TestAPullRequestNobodyCheckedIsAStalledDelivery(t *testing.T) { + w := newWorld(t) + head := "bfe82315f42c8f4509d0598c32569a202d5eada6" + w.forge.open = []OpenPull{ + {Owner: "novox", Repo: "hq", Number: 243, Base: "main", HeadSHA: head, Required: []string{"mesh/repo-check"}}, + {Owner: "novox", Repo: "sdk", Number: 14, Base: "main", HeadSHA: "aaaa", Required: []string{"mesh/merge-gate", "mesh/repo-check"}, + Statuses: []string{"mesh/merge-gate"}}, + {Owner: "novox", Repo: "free", Number: 1, Base: "main", HeadSHA: "bbbb", Required: nil}, + } + w.h.SeeOpenPulls() + if lines := w.h.Stalled(); len(lines) != 0 { + t.Fatalf("a head just seen is stalled already: %+v", lines) + } + w.later(6 * time.Minute) + w.h.SeeOpenPulls() // seen again: its own first-seen time stands, whatever the forge's update time says + w.later(5 * time.Minute) + lines := w.h.Stalled() + if len(lines) != 1 || lines[0].ID != IDOf("novox/hq", head) || lines[0].State != Unannounced || + !strings.Contains(lines[0].Says, "no merge check") || !strings.HasPrefix(lines[0].H2, "none") { + t.Fatalf("an unchecked pull request on a base requiring only mesh/repo-check reads %+v", lines) + } + if lines[0].For != "11m0s" { + t.Fatalf("its time is not counted from when this owner first saw it: %s", lines[0].For) + } + + // Announced at last: a delivery of its own, with its own bound — no longer this line. + w.h.PullUpdated(pr("novox/hq", 243, head, "decision/x")) + w.h.SeeOpenPulls() + for _, l := range w.h.Stalled() { + if l.State == Unannounced { + t.Fatalf("an announced pull request is still said unannounced: %+v", l) + } + } +} + +// One that gets a check, or closes, is dropped; one the forge cannot be asked about is not guessed. +func TestAnUncheckedPullRequestThatGetsACheckOrClosesIsDropped(t *testing.T) { + w := newWorld(t) + w.forge.open = []OpenPull{{Owner: "novox", Repo: "hq", Number: 1, Base: "main", HeadSHA: "cccc", + Required: []string{"mesh/merge-gate"}}} + w.h.SeeOpenPulls() + w.later(11 * time.Minute) + if len(w.h.Stalled()) != 1 { + t.Fatal("not said") + } + w.forge.open[0].Statuses = []string{"mesh/merge-gate"} + w.h.SeeOpenPulls() + if len(w.h.Stalled()) != 0 { + t.Fatal("a pull request with a check is still said") + } + w.forge.open[0].Statuses = nil + w.h.SeeOpenPulls() + w.later(11 * time.Minute) + w.forge.openDown = true + w.h.SeeOpenPulls() // the forge away: what was seen stands, nothing new is guessed + if len(w.h.Stalled()) != 1 { + t.Fatal("a forge that could not be asked dropped or invented a line") + } + w.forge.openDown = false + w.forge.open = nil + w.h.SeeOpenPulls() + if len(w.h.Stalled()) != 0 { + t.Fatal("a closed pull request is still said") + } +} diff --git a/modules/mesh-delivery/cmd/mesh-delivery/verbs.go b/modules/mesh-delivery/cmd/mesh-delivery/verbs.go index 3edab64d..f161d41d 100644 --- a/modules/mesh-delivery/cmd/mesh-delivery/verbs.go +++ b/modules/mesh-delivery/cmd/mesh-delivery/verbs.go @@ -189,6 +189,7 @@ func (h *Holder) Stalled() []StalledLine { out = append(out, StalledLine{ID: d.ID, State: d.State, For: now.Sub(boundSince(d)).Round(time.Second).String(), Bound: b.For.String(), H2: h2, Says: b.Says}) } + out = append(out, h.uncheckedLines(now)...) sort.Slice(out, func(i, j int) bool { return out[i].ID < out[j].ID }) return out } diff --git a/modules/mesh-delivery/module.json b/modules/mesh-delivery/module.json index a165b473..9c7d8d93 100644 --- a/modules/mesh-delivery/module.json +++ b/modules/mesh-delivery/module.json @@ -44,7 +44,8 @@ "gitea.gitea_delivery_view", "gitea.gitea_commit_status", "gitea.gitea_commit_statuses", - "gitea.gitea_contains" + "gitea.gitea_contains", + "gitea.gitea_open_pulls" ], "state": [ "deliveries",