From 8f4cb8c62e61d83383ddfa00637fe0b8650f5384 Mon Sep 17 00:00:00 2001 From: Taylor Addison Date: Sat, 24 Oct 2026 13:58:18 -0400 Subject: [PATCH] cockpit wait: green only once GitHub has every run and job for the head A wait that started 8 s after a push returned "1 of 1 green" at once from the previous head's run: the fleet view learns a new head only from its first run, and the new head's run (nine jobs, six behind `needs:`) was still queued. GitHub's check list had the same gap: a needs-gated job has no check run until its needs finish. The daemon now asks GitHub for the head first (an open PR's branch ref wins over the PR's lagging head.sha) and confirms every fleet green there: every workflow run for the head (actions/runs?head_sha=, newest per workflow) completed, every job of each and every other check passed, and nothing new for 30 s. Until then the answer is pending with GitHub's reason; GitHub not answering is not a yes. Greens carry `verified`. The CLI confirms an older daemon's unverified green itself with gh, and its GitHub fallback uses the same rule. Tests replay the race (a new head's run still queued, a workflow_run run registering after the first run finished). Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 16 ++ cockpit/Sources/CockpitCore/API.swift | 12 + cockpit/Sources/CockpitCore/CheckWait.swift | 32 ++- .../CockpitCore/Fixtures/v1/checks-green.json | 8 +- .../Sources/CockpitCore/GitHubChecks.swift | 180 ++++++++++-- cockpit/Sources/CockpitCore/Rollup.swift | 2 + cockpit/Sources/cockpit/main.swift | 2 +- cockpit/Tests/CockpitCoreTests/APITests.swift | 118 ++++++++ dashboard/fleetd.js | 72 ++++- dashboard/lib/api.js | 5 +- dashboard/lib/checks.js | 262 +++++++++++++++--- dashboard/scripts/make-glance-fixtures.mjs | 19 +- dashboard/test/checks.test.js | 213 +++++++++++++- docs/api.md | 22 +- docs/cockpit/cli.md | 43 ++- 15 files changed, 893 insertions(+), 113 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a0e9db8..3a955fb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -178,6 +178,22 @@ version numbers (`v0.1.0`, `v0.1.1`, etc.). ### Fixed +- **`cockpit wait` and `wait_for_checks` no longer go green before a PR's + checks have all registered.** On acme/stream-app PR #5 (2026-10-24) a + wait that started 8 s after a push returned `1 of 1 green` at once, from the + previous head's run: the fleet view learns a new head only from its first + run, and the new head's run 40116718009 (nine jobs, six of them behind + `needs:`) was still queued. GitHub's own check list had the same gap, since + a `needs:`-gated job has no check run until its needs finish. Now GitHub + names the head (an open PR's branch ref, which moves before the PR's + `head.sha` does), and a green ends the wait only when GitHub has every + workflow run for that head (`actions/runs?head_sha=`, newest per workflow) + completed, every job of each and every other check passed, and nothing new + for 30 s (runs for separate events and `workflow_run` runs register later). + Until then the answer is `pending` with GitHub's reason; GitHub not + answering is not a yes. A green carries `verified` (runs, jobs, checks + covered), and the CLI confirms a green itself with `gh` when an older + daemon sends none. The CLI's GitHub fallback uses the same rule. - **The Hosts tab no longer lists the coordinator as a second, stale host.** `buildHostList` already dropped a heartbeat record under the coordinator's own id, but the `/api/hosts` handler and the `/metrics` stale-host count diff --git a/cockpit/Sources/CockpitCore/API.swift b/cockpit/Sources/CockpitCore/API.swift index a35a56d..ef44439 100644 --- a/cockpit/Sources/CockpitCore/API.swift +++ b/cockpit/Sources/CockpitCore/API.swift @@ -68,6 +68,18 @@ public struct V1Checks: Decodable, Sendable, Equatable { public var fleetSaid: String? public var checks: V1CheckSet? public var orphans: [V1Orphan]? + /// On a green: what GitHub's side of the check covered. A daemon that + /// confirms every green on GitHub always sends it, so a green without it + /// came from an older daemon and is confirmed by the client + /// (stream-app PR #5, 2026-10-24). + public var verified: Verified? + + public struct Verified: Codable, Sendable, Equatable { + public var head: String? + public var runs: Int + public var jobs: Int + public var checks: Int? + } public var finished: Bool { state != "pending" } diff --git a/cockpit/Sources/CockpitCore/CheckWait.swift b/cockpit/Sources/CockpitCore/CheckWait.swift index e961753..7f546e6 100644 --- a/cockpit/Sources/CockpitCore/CheckWait.swift +++ b/cockpit/Sources/CockpitCore/CheckWait.swift @@ -21,7 +21,9 @@ public struct CheckWait: Sendable { /// Wall-clock deadline, epoch ms. public var deadline: Double public var ask: Ask - /// A PR's checks from GitHub, for while the dashboard is unreachable. + /// A PR's checks from GitHub, for while the dashboard is unreachable, and + /// to confirm a green from a daemon too old to confirm it itself (its + /// answer has no `verified`). public var github: (@Sendable () async -> CheckSet?)? /// Recovers orphaned runs; returns why it could not, or nil. public var recover: (@Sendable ([V1Orphan]) async -> String?)? @@ -30,6 +32,8 @@ public struct CheckWait: Sendable { public var now: @Sendable () -> Double = { Format.nowMs() } public var sleep: @Sendable (Double) async -> Void = { s in try? await Task.sleep(nanoseconds: UInt64(s * 1e9)) } public var githubEveryS: Double = 60 + /// How often an unconfirmed green is asked of GitHub again. + public var confirmEveryS: Double = 15 public var retryS: Double = 10 /// After a recovery, a red made only of the recovered runs, cancelled, is /// the force-cancel landing before the re-run starts, not a result. @@ -44,6 +48,7 @@ public struct CheckWait: Sendable { var last: (V1Checks, Data)? var said: String? var askedGitHub = -Double.infinity + var confirmedAt = -Double.infinity var recovered: [Int: Double] = [:] var reachable = false var dropped = false @@ -71,10 +76,28 @@ public struct CheckWait: Sendable { reachable = true dropped = false last = (a, raw) - if let s = a.checks, case let p = s.progress + (s.etaGreenMs.map { ", done in \(Presenter.range($0))" } ?? ""), p != said { + if let s = a.checks, case let p = s.progress + (s.etaGreenMs.map { ", done in \(Presenter.range($0))" } ?? "") + + (a.state == "pending" ? a.reason.map { " — \($0)" } ?? "" : ""), p != said { progress(p) said = p } + // A green from a daemon that does not confirm greens on GitHub + // (stream-app PR #5, 2026-10-24: its "1 of 1 green" was the + // previous head's, 8 s after a push). Not believed until GitHub + // agrees; GitHub not answering is not a yes. + if a.state == "green", a.verified == nil, let github { + if now() - confirmedAt >= confirmEveryS * 1000 { + confirmedAt = now() + let s = await github() + if let s, s.state == .green { return .answer(a, raw) } + if let s, s.state == .red { return .github(s) } + let p = "the dashboard read \(a.checks?.progress ?? "green"), but GitHub " + + (s.map { "has \($0.progress)\($0.why.map { " — \($0)" } ?? "")" } ?? "did not answer") + if p != said { progress(p); said = p } + } + await sleep(min(retryS, max(0.01, (deadline - now()) / 1000))) + continue + } if a.state == "pointless", let recover, let orphans = a.orphans, !orphans.isEmpty, orphans.allSatisfy(\.workPassed) { @@ -114,7 +137,10 @@ public struct CheckWait: Sendable { let progress = a.checks?.progress switch a.state { case "green": - out.append("✔ \(label): \(progress ?? "green")\(note)") + let covered = a.verified.map { v in + " (GitHub: \(v.runs) run\(v.runs == 1 ? "" : "s"), \(v.jobs) job\(v.jobs == 1 ? "" : "s"))" + } ?? "" + out.append("✔ \(label): \(progress ?? "green")\(covered)\(note)") case "red": out.append("✖ \(label): \(progress ?? "red")\(note)") for f in a.checks?.failed ?? [] { diff --git a/cockpit/Sources/CockpitCore/Fixtures/v1/checks-green.json b/cockpit/Sources/CockpitCore/Fixtures/v1/checks-green.json index b027255..6eb554a 100644 --- a/cockpit/Sources/CockpitCore/Fixtures/v1/checks-green.json +++ b/cockpit/Sources/CockpitCore/Fixtures/v1/checks-green.json @@ -43,5 +43,11 @@ }, "reason": null, "hint": null, - "fleetAgeMs": 0 + "fleetAgeMs": 0, + "verified": { + "head": "ec99782000000000000000000000000000000000", + "runs": 1, + "jobs": 1, + "checks": 0 + } } diff --git a/cockpit/Sources/CockpitCore/GitHubChecks.swift b/cockpit/Sources/CockpitCore/GitHubChecks.swift index 6db518d..f05e7c8 100644 --- a/cockpit/Sources/CockpitCore/GitHubChecks.swift +++ b/cockpit/Sources/CockpitCore/GitHubChecks.swift @@ -1,11 +1,18 @@ import Foundation /// GitHub's own view of a PR's checks: `gh pr view --json -/// headRefOid,statusCheckRollup`. Cockpit's rows are a copy of this, made by -/// the daemon's fast loop; when that loop stops finishing ticks the copy -/// freezes, so a wait asks the original as well (shop-ios PR #445 and -/// shop-android PR #280, 2026-10-07: both green on GitHub while cockpit -/// still showed them running). +/// headRefOid,statusCheckRollup`, the branch ref, and the head's workflow +/// runs. Cockpit's rows are a copy of this, made by the daemon's fast loop; +/// when that loop stops finishing ticks the copy freezes, so a wait asks the +/// original as well (shop-ios PR #445 and shop-android PR #280, 2026-10-07: +/// both green on GitHub while cockpit still showed them running). +/// +/// The same green rule as the daemon's (dashboard/lib/checks.js +/// githubVerdict): every workflow run for the head completed, every check +/// completed and passed, and nothing new for `settleMs`. The rollup lists only +/// jobs that exist, and a job behind `needs:` exists only once its needs +/// finish, so a green rollup alone is not the end (stream-app PR #5, +/// 2026-10-24). public struct GitHubChecks: Sendable, Equatable { public struct Check: Sendable, Equatable { public var name: String @@ -14,40 +21,110 @@ public struct GitHubChecks: Sendable, Equatable { public var status: String public var conclusion: String? public var url: String? + public var completedAt: Double? = nil } + /// One workflow run for the head, from `actions/runs?head_sha=`. + public struct Run: Sendable, Equatable { + public var id: Int + public var workflowId: Int? + public var name: String? + public var status: String + public var conclusion: String? + public var createdAt: Double? + public var updatedAt: Double? + public var url: String? + } + + /// How long GitHub must have been quiet about the head before a green. + public static let settleMs: Double = 30_000 + public var head: String? public var checks: [Check] + /// The head's workflow runs; nil when not asked (judged by checks alone). + public var runs: [Run]? = nil + /// When GitHub was asked, epoch ms; the settle window counts from it. + public var fetchedAt: Double? = nil + /// From the PR view, to find where its branch is now. + public var open: Bool = false + public var headRepo: String? = nil + public var headRef: String? = nil + + /// The newest run of each workflow, as in the fleet's rollup. + public var latestRuns: [Run] { + var by: [String: Run] = [:] + for r in runs ?? [] { + let key = r.workflowId.map(String.init) ?? r.name ?? String(r.id) + if let held = by[key], held.id > r.id { continue } + by[key] = r + } + return by.values.sorted { $0.id < $1.id } + } + + var lastChange: Double { + (latestRuns.flatMap { [$0.createdAt, $0.updatedAt] } + (runs ?? []).map(\.createdAt) + checks.map(\.completedAt)) + .compactMap { $0 }.max() ?? 0 + } + + /// Why a finished-looking answer is still pending; nil otherwise. + public var pendingWhy: String? { + let runs = latestRuns + if checks.isEmpty && runs.isEmpty { return "GitHub has no runs or checks for the head yet" } + if let r = runs.first(where: { $0.status != "completed" }) { + return "GitHub still has \(r.name ?? "a run") \(r.status.replacingOccurrences(of: "_", with: " "))" + } + if let c = checks.first(where: { $0.status != "completed" }) { return "GitHub still has \(c.name) running" } + if !failedAny, let fetchedAt, case let quiet = fetchedAt - lastChange, quiet < Self.settleMs { + return "settling: GitHub's newest run or job changed \(Int(max(0, quiet / 1000))) s ago, and a green waits \(Int(Self.settleMs / 1000)) s for any run still to register" + } + return nil + } + + var failedAny: Bool { + !rows.allSatisfy(\.passed) || !latestRuns.allSatisfy { ["success", "skipped", "neutral"].contains($0.conclusion ?? "") } + } - /// Finished only when GitHub lists at least one check and every one is - /// complete. An empty rollup is a head whose workflows have not been - /// created yet, not a pass. public var state: CheckSet.State { - if checks.isEmpty || checks.contains(where: { $0.status != "completed" }) { return .pending } - return rows.allSatisfy(\.passed) ? .green : .red + if pendingWhy != nil { return .pending } + return failedAny ? .red : .green } /// As cockpit rows, so a GitHub answer reports exactly like a cockpit one - /// (same green rule: success/skipped/neutral only; cancelled is red). + /// (same green rule: success/skipped/neutral only; cancelled is red). A + /// run with no job in the rollup yet is one row of its own. var rows: [RunRow] { - checks.enumerated().map { i, c in - RunRow(id: -(i + 1), repo: "", workflow: c.name, status: c.status, conclusion: c.conclusion, + let listed = checks.map { c -> RunRow in + RunRow(id: 0, repo: "", workflow: c.name, status: c.status, conclusion: c.conclusion, branch: nil, sha: head, prNumber: nil, event: nil, title: nil, url: c.url, startedAt: nil, updatedAt: nil, elapsedMs: nil, expectedMs: nil) } + let unlisted = latestRuns.filter { r in !checks.contains { $0.url?.contains("/actions/runs/\(r.id)/") == true } } + .map { r in + RunRow(id: 0, repo: "", workflow: r.name, status: r.status, conclusion: r.status == "completed" ? r.conclusion : nil, + branch: nil, sha: head, prNumber: nil, event: nil, title: nil, url: r.url, + startedAt: nil, updatedAt: nil, elapsedMs: nil, expectedMs: nil) + } + return (listed + unlisted).enumerated().map { i, r in var r = r; r.id = -(i + 1); return r } } public func checkSet(repo: String, pr: Int?) -> CheckSet { let rs = rows.map { r -> RunRow in var r = r; r.repo = repo; r.prNumber = pr; return r } let failed = rs.filter(\.failed) let done = rs.filter { !$0.isActive }.count - return CheckSet(id: "github:\(repo)@\(head ?? "?")", repo: repo, prNumber: pr, branch: nil, - sha: head.map { String($0.prefix(12)) }, title: nil, runs: rs, total: rs.count, done: done, - failed: failed, queued: [], etaGreenMs: nil, state: state) + var set = CheckSet(id: "github:\(repo)@\(head ?? "?")", repo: repo, prNumber: pr, branch: nil, + sha: head.map { String($0.prefix(12)) }, title: nil, runs: rs, total: rs.count, done: done, + failed: failed, queued: [], etaGreenMs: nil, state: state) + set.why = pendingWhy + return set + } + + static func ms(_ s: String?) -> Double? { + s.flatMap { ISO8601DateFormatter().date(from: $0) }.map { $0.timeIntervalSince1970 * 1000 } } - /// Parses `gh pr view --json headRefOid,statusCheckRollup`. Check runs - /// carry status + conclusion; commit statuses carry one `state`. + /// Parses `gh pr view --json headRefOid,headRefName,headRepository, + /// headRepositoryOwner,state,statusCheckRollup`. Check runs carry status + + /// conclusion; commit statuses carry one `state`. public static func parse(prView data: Data) throws -> GitHubChecks { struct Item: Decodable { var __typename: String? @@ -59,8 +136,18 @@ public struct GitHubChecks: Sendable, Equatable { var state: String? var detailsUrl: String? var targetUrl: String? + var completedAt: String? + var startedAt: String? + } + struct Named: Decodable { var name: String?; var login: String? } + struct View: Decodable { + var headRefOid: String? + var headRefName: String? + var headRepository: Named? + var headRepositoryOwner: Named? + var state: String? + var statusCheckRollup: [Item]? } - struct View: Decodable { var headRefOid: String?; var statusCheckRollup: [Item]? } let v = try JSONDecoder().decode(View.self, from: data) let checks = (v.statusCheckRollup ?? []).map { i -> Check in if i.__typename == "StatusContext" || (i.status == nil && i.state != nil) { @@ -73,21 +160,46 @@ public struct GitHubChecks: Sendable, Equatable { } return Check(name: i.context ?? i.name ?? "status", workflow: nil, status: finished ? "completed" : "in_progress", conclusion: conclusion, - url: i.targetUrl) + url: i.targetUrl, completedAt: finished ? ms(i.startedAt) : nil) } let status = (i.status ?? "QUEUED").lowercased() return Check(name: i.name ?? "check", workflow: i.workflowName, status: status == "completed" ? "completed" : status, conclusion: status == "completed" ? (i.conclusion ?? "").lowercased() : nil, - url: i.detailsUrl) + url: i.detailsUrl, completedAt: ms(i.completedAt)) + } + var c = GitHubChecks(head: v.headRefOid, checks: checks) + c.open = v.state?.uppercased() == "OPEN" + if let owner = v.headRepositoryOwner?.login, let name = v.headRepository?.name { c.headRepo = "\(owner)/\(name)" } + c.headRef = v.headRefName + return c + } + + /// Parses `gh api repos//actions/runs?head_sha=`. + public static func parse(runs data: Data) throws -> [Run] { + struct R: Decodable { + var id: Int + var workflow_id: Int? + var name: String? + var status: String? + var conclusion: String? + var created_at: String? + var updated_at: String? + var html_url: String? + } + struct Page: Decodable { var workflow_runs: [R]? } + return (try JSONDecoder().decode(Page.self, from: data).workflow_runs ?? []).map { r in + let status = r.status ?? "queued" + return Run(id: r.id, workflowId: r.workflow_id, name: r.name, status: status == "completed" ? "completed" : status, + conclusion: status == "completed" ? (r.conclusion ?? "") : nil, + createdAt: ms(r.created_at), updatedAt: ms(r.updated_at), url: r.html_url) } - return GitHubChecks(head: v.headRefOid, checks: checks) } } /// Asks GitHub through the `gh` CLI. `gh pr view` is GraphQL, whose budget is /// separate from the REST budget the daemon's token polls with, and a wait -/// asks at most twice a minute — far from the bulk calls that once +/// asks at most a few times a minute — far from the bulk calls that once /// rate-penalized the daemon. public struct GHChecksProbe: Sendable { public var repo: String @@ -96,10 +208,26 @@ public struct GHChecksProbe: Sendable { public init(repo: String, pr: Int) { self.repo = repo; self.pr = pr } + /// nil unless GitHub answered all of it: a part missing is not a yes. public func fetch() async -> GitHubChecks? { - guard let out = await Self.gh(["pr", "view", String(pr), "-R", repo, "--json", "headRefOid,statusCheckRollup"], - timeout: timeout) else { return nil } - return try? GitHubChecks.parse(prView: out) + let asked = Format.nowMs() + guard let out = await Self.gh(["pr", "view", String(pr), "-R", repo, "--json", + "headRefOid,headRefName,headRepository,headRepositoryOwner,state,statusCheckRollup"], + timeout: timeout), + var c = try? GitHubChecks.parse(prView: out) else { return nil } + // The PR's head catches up with a push a moment after the branch + // moves; until it does, its rollup is the previous head's. + if c.open, let from = c.headRepo, let ref = c.headRef, + let b = await Self.gh(["api", "repos/\(from)/git/ref/heads/\(ref)", "--jq", ".object.sha"], timeout: timeout) { + let sha = String(decoding: b, as: UTF8.self).trimmingCharacters(in: .whitespacesAndNewlines) + if sha.count == 40, sha != c.head { c.head = sha; c.checks = [] } + } + guard let head = c.head, + let runs = await Self.gh(["api", "repos/\(repo)/actions/runs?head_sha=\(head)&per_page=100"], timeout: timeout), + let parsed = try? GitHubChecks.parse(runs: runs) else { return nil } + c.runs = parsed + c.fetchedAt = asked + return c } /// `owner/name` for a bare repo name: from the glance when it knows the diff --git a/cockpit/Sources/CockpitCore/Rollup.swift b/cockpit/Sources/CockpitCore/Rollup.swift index 804dfc5..baee5b5 100644 --- a/cockpit/Sources/CockpitCore/Rollup.swift +++ b/cockpit/Sources/CockpitCore/Rollup.swift @@ -22,6 +22,8 @@ public struct CheckSet: Identifiable, Sendable, Equatable { /// moved to another commit after they ran and has since come back, so a /// new run is due (site-web PR #79, 2026-10-03). public var superseded: [RunRow] = [] + /// GitHub's reason a finished-looking set is still pending ("settling: …"). + public var why: String? = nil public var label: String { let repoShort = repo.split(separator: "/").last.map(String.init) ?? repo diff --git a/cockpit/Sources/cockpit/main.swift b/cockpit/Sources/cockpit/main.swift index de65884..c109fe9 100644 --- a/cockpit/Sources/cockpit/main.swift +++ b/cockpit/Sources/cockpit/main.swift @@ -405,7 +405,7 @@ case "wait": fail(" recovery failed: \(why)", code: 2) case .github(let s): let mark = s.state == .green ? "✔" : "✖" - print("\(mark) \(target.label): \(s.progress) (from GitHub; the dashboard did not answer)") + print("\(mark) \(target.label): \(s.progress) (from GitHub)") for f in s.failed { print(" \(f.workflow ?? "?")\(f.cancelled ? " (cancelled)" : "")\(f.url.map { " — \($0)" } ?? "")") } exit(s.state == .green ? 0 : 1) case .timeout(let a, let raw, let reached): diff --git a/cockpit/Tests/CockpitCoreTests/APITests.swift b/cockpit/Tests/CockpitCoreTests/APITests.swift index d07fa1b..cd54eb0 100644 --- a/cockpit/Tests/CockpitCoreTests/APITests.swift +++ b/cockpit/Tests/CockpitCoreTests/APITests.swift @@ -209,3 +209,121 @@ struct CheckWaitTests { #expect(a.exitCode == 2) } } + +/// GitHub's answer for a PR as `gh` gives it, and the green rule over it: +/// stream-app PR #5 (2026-10-24), where every check GitHub listed had passed +/// while run 40116718009 was still creating the six jobs behind `needs:`. +@Suite("GitHub's checks") +struct GitHubChecksTests { + static let view = Data(""" + {"headRefOid":"d42a20fa8be5e24c477004b82a8c2194eacd6493","headRefName":"fix/ci-gate", + "headRepository":{"name":"stream-app"},"headRepositoryOwner":{"login":"acme"},"state":"OPEN", + "statusCheckRollup":[ + {"__typename":"CheckRun","name":"guard","workflowName":"ci","status":"COMPLETED","conclusion":"SUCCESS", + "completedAt":"2026-10-24T17:34:39Z","detailsUrl":"https://github.com/acme/stream-app/actions/runs/40116718009/job/120939404752"}, + {"__typename":"CheckRun","name":"changes","workflowName":"ci","status":"COMPLETED","conclusion":"SUCCESS", + "completedAt":"2026-10-24T17:34:46Z","detailsUrl":"https://github.com/acme/stream-app/actions/runs/40116718009/job/120939404951"}]} + """.utf8) + + static func runs(_ status: String, conclusion: String? = nil, updated: String = "2026-10-24T17:34:46Z") -> Data { + Data(""" + {"total_count":1,"workflow_runs":[{"id":40116718009,"workflow_id":7,"name":"ci","status":"\(status)", + "conclusion":\(conclusion.map { "\"\($0)\"" } ?? "null"),"created_at":"2026-10-24T17:34:12Z","updated_at":"\(updated)", + "html_url":"https://github.com/acme/stream-app/actions/runs/40116718009"}]} + """.utf8) + } + + static func at(_ iso: String) -> Double { ISO8601DateFormatter().date(from: iso)!.timeIntervalSince1970 * 1000 } + + @Test func aGreenRollupWhileTheRunIsInProgressIsPending() throws { + var c = try GitHubChecks.parse(prView: Self.view) + #expect(c.state == .green, "the rollup alone, as before") + #expect(c.headRepo == "acme/stream-app" && c.headRef == "fix/ci-gate" && c.open) + c.runs = try GitHubChecks.parse(runs: Self.runs("in_progress")) + c.fetchedAt = Self.at("2026-10-24T17:34:50Z") + #expect(c.state == .pending) + #expect(c.pendingWhy == "GitHub still has ci in progress") + #expect(c.checkSet(repo: "acme/stream-app", pr: 5).progress == "2 of 2 done") + } + + @Test func aFinishedRunIsGreenOnlyOnceGitHubHasBeenQuiet() throws { + var c = try GitHubChecks.parse(prView: Self.view) + c.runs = try GitHubChecks.parse(runs: Self.runs("completed", conclusion: "success", updated: "2026-10-24T17:36:54Z")) + c.fetchedAt = Self.at("2026-10-24T17:37:00Z") + #expect(c.state == .pending) + #expect(c.pendingWhy?.hasPrefix("settling: ") == true) + c.fetchedAt = Self.at("2026-10-24T17:37:30Z") + #expect(c.state == .green) + } + + @Test func aRunWithNoJobInTheRollupIsARowOfItsOwn() throws { + var c = try GitHubChecks.parse(prView: Self.view) + c.checks = [] + c.runs = try GitHubChecks.parse(runs: Self.runs("queued")) + let set = c.checkSet(repo: "acme/stream-app", pr: 5) + #expect(set.state == .pending) + #expect(set.progress == "0 of 1 done") + } + + @Test func aFailedRunIsRed() throws { + var c = try GitHubChecks.parse(prView: Self.view) + c.runs = try GitHubChecks.parse(runs: Self.runs("completed", conclusion: "cancelled", updated: "2026-10-24T17:36:54Z")) + c.fetchedAt = Self.at("2026-10-24T17:36:55Z") + #expect(c.state == .red) + } +} + +@Suite("cockpit wait confirms an older daemon's green") +struct ConfirmGreenTests { + private func unverifiedGreen() throws -> (V1Checks, Data) { + var (a, raw) = try answer("checks-green") + a.verified = nil + return (a, raw) + } + + @Test func aGreenWithVerifiedIsBelievedWithoutAsking() async throws { + let clock = Clock() + var w = wait(Script([.success(try answer("checks-green"))]), clock) + let asks = Script([.failure(URLError(.unknown))]) + w.github = { _ = try? asks.next(0); return nil } + guard case .answer(let a, _) = await w.run() else { Issue.record("no answer"); return } + #expect(a.state == "green") + #expect(asks.asked.isEmpty) + #expect(CheckWait.text(a, label: "comet-web @ ec99782") == "✔ comet-web @ ec99782: 1 of 1 green (GitHub: 1 run, 1 job)") + } + + @Test func anUnverifiedGreenWaitsForGitHub() async throws { + let clock = Clock() + var w = wait(Script([.success(try unverifiedGreen())]), clock) + var pending = CheckSet(id: "github", repo: "acme/comet-web", prNumber: 7, branch: nil, sha: nil, title: nil, + runs: [], total: 9, done: 2, failed: [], queued: [], etaGreenMs: nil, state: .pending) + pending.why = "GitHub still has ci in progress" + var green = pending + green.state = .green + green.done = 9 + let asks = Script([.failure(URLError(.unknown))]) + let (stillPending, nowGreen) = (pending, green) + w.github = { _ = try? asks.next(0); return asks.asked.count >= 3 ? nowGreen : stillPending } + guard case .answer(let a, _) = await w.run() else { Issue.record("no answer"); return } + #expect(a.state == "green") + #expect(asks.asked.count == 3) + #expect(clock.now - 1_000_000 >= 30_000, "asked every 15 s") + } + + @Test func anUnverifiedGreenThatGitHubCallsRedIsRed() async throws { + let clock = Clock() + var w = wait(Script([.success(try unverifiedGreen())]), clock) + let red = CheckSet(id: "github", repo: "acme/comet-web", prNumber: 7, branch: nil, sha: nil, title: nil, + runs: [], total: 2, done: 2, failed: [], queued: [], etaGreenMs: nil, state: .red) + w.github = { red } + guard case .github(let s) = await w.run() else { Issue.record("not GitHub's red"); return } + #expect(s.state == .red) + } + + @Test func anUnverifiedGreenGitHubCannotConfirmTimesOut() async throws { + let clock = Clock() + var w = wait(Script([.success(try unverifiedGreen())]), clock, deadlineS: 60) + w.github = { nil } + guard case .timeout = await w.run() else { Issue.record("did not time out"); return } + } +} diff --git a/dashboard/fleetd.js b/dashboard/fleetd.js index 12c2473..a3b7e98 100644 --- a/dashboard/fleetd.js +++ b/dashboard/fleetd.js @@ -44,7 +44,7 @@ import { handleMcp } from './lib/mcp.js'; import { createOAuth, parsePublicUrl } from './lib/oauth.js'; import { createPublicHandler } from './lib/public.js'; import { findOrphans, trimJob } from './lib/orphans.js'; -import { waitForChecks, parseGitHubChecks } from './lib/checks.js'; +import { waitForChecks, parseGitHubChecks, latestRuns } from './lib/checks.js'; import { loadOrCreateToken, authorize, bearerToken, tokenMatches } from './lib/auth.js'; import { initAllowedHosts, refreshNetworkIdentity, reachableUrls, ensureHostAllowed, pairingUrl, @@ -472,26 +472,68 @@ function currentGlance(now = Date.now()) { // the steps, and keeping them here costs no extra GitHub call. let activeRunJobs = []; -// GitHub's own checks for a wait's target, asked when the fleet view is stale -// or has nothing for it. Shared for 20 s, so ten agents waiting on one PR -// cost GitHub what one does. +// GitHub's own answer for a wait's target (lib/checks.js `gc`): the head +// commit, every workflow run for it with the jobs of each completed run, and +// the other checks and statuses. Every wait asks before it ends green, and a +// stale or empty fleet view asks more often. Shared for 10 s, so ten agents +// waiting on one PR cost GitHub what one does; the answer keeps its own +// fetchedAt, so a shared copy never looks quieter than it was. const githubChecksCache = new Map(); +const refPath = (ref) => ref.split('/').map(encodeURIComponent).join('/'); + +// The head as of now. A PR's head.sha catches up with a push a moment after +// the branch moves, so an open PR's branch ref is asked as well, and wins. +async function githubHeadFor(target) { + const { repo } = target; + if (target.sha) { + if (/^[0-9a-f]{40}$/i.test(target.sha)) return target.sha; + return (await gh.get(`repos/${repo}/commits/${encodeURIComponent(target.sha)}`)).data?.sha ?? null; + } + if (target.branch) { + return (await gh.get(`repos/${repo}/git/ref/heads/${refPath(target.branch)}`)).data?.object?.sha ?? null; + } + if (target.pr == null) return null; + const pr = (await gh.get(`repos/${repo}/pulls/${target.pr}`)).data; + let head = pr?.head?.sha ?? null; + const from = pr?.head?.repo?.full_name; + if (pr?.state === 'open' && from && pr.head.ref) { + const moved = await gh.get(`repos/${from}/git/ref/heads/${refPath(pr.head.ref)}`).catch(() => null); + head = moved?.data?.object?.sha ?? head; + } + return head; +} + async function githubChecksFor(target) { const key = `${target.repo}|${target.pr ?? ''}|${target.sha ?? ''}|${target.branch ?? ''}`; const hit = githubChecksCache.get(key); - if (hit && Date.now() - hit.at < 20_000) return hit.value; - let head = target.sha ?? null; - if (target.pr != null) head = (await gh.get(`repos/${target.repo}/pulls/${target.pr}`)).data?.head?.sha ?? null; - const ref = head ?? target.branch; - if (!ref) return null; - const enc = encodeURIComponent(ref); - const [runs, status] = await Promise.all([ - gh.get(`repos/${target.repo}/commits/${enc}/check-runs?per_page=100`), - gh.get(`repos/${target.repo}/commits/${enc}/status`), + if (hit && Date.now() - hit.at < 10_000) return hit.value; + const fetchedAt = Date.now(); + const head = await githubHeadFor(target); + if (!head) return null; + const { repo } = target; + const [checkRuns, status, runs] = await Promise.all([ + gh.get(`repos/${repo}/commits/${head}/check-runs?per_page=100`), + gh.get(`repos/${repo}/commits/${head}/status`), + gh.get(`repos/${repo}/actions/runs?head_sha=${head}&per_page=100`), ]); - const value = parseGitHubChecks(head ?? ref, runs.data, status.data); + const workflowRuns = runs.data?.workflow_runs ?? []; + // Jobs only for runs that completed (a running run is pending whatever its + // jobs say), and only for the newest run of each workflow. + const jobs = new Map(); + const done = latestRuns(workflowRuns.map((r) => ({ id: r.id, workflowId: r.workflow_id, workflow: r.name, status: r.status }))) + .filter((r) => r.status === 'completed'); + await Promise.all(done.map(async (r) => { + const all = []; + for (let page = 1; page <= 5; page++) { + const { data } = await gh.get(`repos/${repo}/actions/runs/${r.id}/jobs?filter=latest&per_page=100&page=${page}`); + all.push(...(data?.jobs ?? [])); + if (all.length >= (data?.total_count ?? 0) || !(data?.jobs ?? []).length) break; + } + jobs.set(r.id, all); + })); + const value = parseGitHubChecks(head, checkRuns.data, status.data, { runs: workflowRuns, jobs, fetchedAt }); if (githubChecksCache.size > 200) githubChecksCache.clear(); - githubChecksCache.set(key, { at: Date.now(), value }); + githubChecksCache.set(key, { at: fetchedAt, value }); return value; } diff --git a/dashboard/lib/api.js b/dashboard/lib/api.js index 752ad18..607f0b6 100644 --- a/dashboard/lib/api.js +++ b/dashboard/lib/api.js @@ -150,7 +150,10 @@ export const ROUTES = [ + '(at most 300) until the checks finish; pending means the wait ran out, so ask again. pointless means ' + 'waiting cannot end well: the host is down, the disk floor is holding jobs, GitHub is refusing jobs, ' + 'a queued check will never start, or a job was orphaned; reason says which, and orphans carry their ' - + 'recovery. Cancelled is red, not green. Use this instead of polling gh pr checks. A client that gives ' + + 'recovery. Cancelled is red, not green. Green means GitHub itself has every workflow run for the head ' + + 'completed with all of its jobs passed, and nothing new for 30 s (verified says what it covered): runs ' + + 'and needs-gated jobs register after a push, so a green first wave is not the end. ' + + 'Use this instead of polling gh pr checks. A client that gives ' + 'up on slow calls should use a shorter wait and ask again.', params: [ ...repoPath, diff --git a/dashboard/lib/checks.js b/dashboard/lib/checks.js index 4d64571..b8c5009 100644 --- a/dashboard/lib/checks.js +++ b/dashboard/lib/checks.js @@ -143,11 +143,18 @@ export function checkSets(g) { // is null (still waiting), never an older commit's result: on site-web PR #79 // a wait read "red" from a commit the branch had been force-pushed off. // Without a known head, the commit with the newest run stands in for it. -export function findCheckSet(g, { repo, pr = null, sha = null, branch = null }) { +// +// `head` is the head as GitHub names it now (waitForChecks asks). It beats +// the glance's own idea of the head, which is learned from runs and so is the +// PREVIOUS head until the fast loop sees the new commit's first run: +// stream-app PR #5 (2026-10-24) read the old commit's "1 of 1 green" 8 s +// after a push, while run 40116718009 on the new head was still queued. +export function findCheckSet(g, { repo, pr = null, sha = null, branch = null, head = null }) { const sets = checkSets(g).filter((s) => sameRepo(s.repo, repo) && (pr == null || s.prNumber === pr) && (sha == null || sameCommit(sha, s.sha)) && (branch == null || s.branch === branch)); + if (head) return sets.find((s) => sameCommit(head, s.sha)) ?? null; if (pr == null) return sets[0] ?? null; const withHead = [...(g.runs ?? []), ...(g.recent ?? [])] .filter((r) => sameRepo(r.repo, repo) && r.prNumber === pr && r.prHead) @@ -210,24 +217,100 @@ export function overlay(g, rows) { return { ...g, runs, recent }; } + // ------------------------------------------------------------ GitHub's view -// GitHub's own checks for a ref, from the REST check-runs and combined-status -// endpoints: { head, checks: [{ name, status, conclusion, url }] }. Finished -// only when GitHub lists at least one check and every one is complete: an -// empty list is a head whose workflows have not been created yet, not a pass. -export function githubState(gc) { - if (!gc?.checks?.length || gc.checks.some((c) => c.status !== 'completed')) return 'pending'; - return gc.checks.every(passed) ? 'green' : 'red'; +// GitHub's own answer about a head, as fleetd's githubChecksFor builds it with +// parseGitHubChecks: +// { head, fetchedAt, runs, checks } +// runs every workflow run GitHub has for the head commit +// (actions/runs?head_sha=), each { id, workflowId, workflow, event, +// status, conclusion, createdAt, updatedAt, url, jobs }. jobs are +// the latest attempt's { name, status, conclusion, url, +// completedAt }, or null while unread (fleetd reads them once the +// run has completed). +// checks commit statuses, check runs from other apps, and any Actions +// check run whose run is missing from `runs`. +// An answer without `runs` (an older source) is judged by its checks alone. + +// How long GitHub must have been quiet about a head, as of the moment it was +// asked, before its green is believed. Runs register seconds after a push +// (pull_request and push are separate events), a workflow_run-triggered run +// only after the run it follows completes, and a job behind `needs:` only once +// its needs finish. A finished first wave is not the end. +export const SETTLE_MS = 30_000; + +// The newest run of each workflow, as in the fleet's rollup: a re-trigger or +// a concurrency replacement on the same commit wins over the run it replaced. +export function latestRuns(runs) { + const byWorkflow = new Map(); + for (const r of runs ?? []) { + const key = r.workflowId ?? r.workflow ?? r.id; + if (!byWorkflow.has(key) || byWorkflow.get(key).id < r.id) byWorkflow.set(key, r); + } + return [...byWorkflow.values()].sort((a, b) => a.id - b.id); } -export function parseGitHubChecks(head, checkRuns, combined) { - const runs = (checkRuns?.check_runs ?? []).map((c) => ({ - name: c.name, - status: c.status === 'completed' ? 'completed' : (c.status ?? 'queued'), - conclusion: c.status === 'completed' ? (c.conclusion ?? '') : null, - url: c.html_url ?? c.details_url ?? null, - })); +// When anything about the head last changed on GitHub, epoch ms (0 if unknown). +function lastChange(gc) { + const times = [ + ...(gc.runs ?? []).flatMap((r) => [r.createdAt, r.updatedAt, ...(r.jobs ?? []).map((j) => j.completedAt)]), + ...(gc.checks ?? []).map((c) => c.completedAt), + ].filter(Number.isFinite); + return times.length ? Math.max(...times) : 0; +} + +const names = (rs) => rs.map((r) => r.workflow ?? r.name ?? String(r.id)).join(', '); + +// { state: green | red | pending, why }. Green needs every workflow run for +// the head completed, every one of their jobs completed, every other check +// completed, all of them passed, and nothing new for settleMs. +export function githubVerdict(gc, { settleMs = SETTLE_MS } = {}) { + if (!gc) return { state: 'pending', why: 'GitHub did not answer' }; + const head = gc.head ? String(gc.head).slice(0, 7) : 'the head'; + const runs = latestRuns(gc.runs); + const checks = gc.checks ?? []; + if (!runs.length && !checks.length) return { state: 'pending', why: `GitHub has no runs or checks for ${head} yet` }; + const running = runs.filter(isActive); + if (running.length) return { state: 'pending', why: `GitHub still has ${names(running)} ${String(running[0].status).replace('_', ' ')}` }; + const unread = runs.filter((r) => gc.runs && !Array.isArray(r.jobs)); + if (unread.length) return { state: 'pending', why: `the jobs of ${names(unread)} are not read yet` }; + const jobs = runs.flatMap((r) => r.jobs ?? []); + const open = [...jobs, ...checks].filter(isActive); + if (open.length) return { state: 'pending', why: `GitHub still has ${names(open)} running` }; + if (runs.some(failed) || jobs.some(failed) || checks.some(failed)) return { state: 'red', why: null }; + const quietMs = (gc.fetchedAt ?? Date.now()) - lastChange(gc); + if (quietMs < settleMs) { + return { + state: 'pending', + why: `settling: GitHub's newest run or job for ${head} changed ${Math.max(0, Math.round(quietMs / 1000))} s ago, and a green waits ${Math.round(settleMs / 1000)} s for any run still to register`, + }; + } + return { state: 'green', why: null }; +} + +export const githubState = (gc, opts) => githubVerdict(gc, opts).state; + +const ms = (iso) => { + const t = Date.parse(iso ?? ''); + return Number.isFinite(t) ? t : null; +}; + +// GitHub's REST answers for one head as `gc`. `actions` holds the workflow +// runs (actions/runs?head_sha=) and, by run id, the jobs of each run read so +// far. Without it, this is the older check-runs-and-statuses answer. +export function parseGitHubChecks(head, checkRuns, combined, actions = null) { + const runIds = new Set((actions?.runs ?? []).map((r) => r.id)); + const fromCheckRuns = (checkRuns?.check_runs ?? []) + // An Actions check run is a job of one of `runs`, already counted there. + .filter((c) => !actions || !runIds.has(runIdFromUrl(c.html_url ?? c.details_url))) + .map((c) => ({ + name: c.name, + status: c.status === 'completed' ? 'completed' : (c.status ?? 'queued'), + conclusion: c.status === 'completed' ? (c.conclusion ?? '') : null, + url: c.html_url ?? c.details_url ?? null, + completedAt: ms(c.completed_at), + })); const statuses = (combined?.statuses ?? []).map((s) => { const st = String(s.state ?? 'pending').toLowerCase(); const finished = st !== 'pending' && st !== 'expected'; @@ -236,14 +319,58 @@ export function parseGitHubChecks(head, checkRuns, combined) { status: finished ? 'completed' : 'in_progress', conclusion: !finished ? null : st === 'success' ? 'success' : 'failure', url: s.target_url ?? null, + completedAt: finished ? ms(s.updated_at) : null, }; }); - return { head, checks: [...runs, ...statuses] }; + const gc = { head, checks: [...fromCheckRuns, ...statuses] }; + if (!actions) return gc; + gc.fetchedAt = actions.fetchedAt ?? null; + gc.runs = (actions.runs ?? []).map((r) => { + const jobs = actions.jobs?.get(r.id); + return { + id: r.id, + workflowId: r.workflow_id ?? null, + workflow: r.name ?? null, + event: r.event ?? null, + status: r.status === 'completed' ? 'completed' : (r.status ?? 'queued'), + conclusion: r.status === 'completed' ? (r.conclusion ?? '') : null, + runAttempt: r.run_attempt ?? null, + createdAt: ms(r.created_at), + updatedAt: ms(r.updated_at), + url: r.html_url ?? null, + jobs: !jobs ? null : jobs.map((j) => ({ + name: j.name, + status: j.status === 'completed' ? 'completed' : (j.status ?? 'queued'), + conclusion: j.status === 'completed' ? (j.conclusion ?? '') : null, + url: j.html_url ?? null, + completedAt: ms(j.completed_at), + })), + }; + }); + return gc; +} + +// What a green from GitHub's side of the check carries: how much it covered. +export function confirmation(gc) { + if (!gc?.runs) return null; + const runs = latestRuns(gc.runs); + return { + head: gc.head ?? null, + runs: runs.length, + jobs: runs.reduce((n, r) => n + (r.jobs?.length ?? 0), 0), + checks: gc.checks?.length ?? 0, + }; } -// GitHub's answer as a check set, so it reads exactly like the fleet's. -export function githubSet(gc, target) { - const runs = gc.checks.map((c, i) => ({ +// GitHub's answer as a check set, so it reads exactly like the fleet's: one +// row per job of each run (a run whose jobs are unread is one row), then the +// other checks. +export function githubSet(gc, target, verdict = githubVerdict(gc)) { + const items = [ + ...latestRuns(gc.runs).flatMap((r) => (r.jobs?.length ? r.jobs : [{ name: r.workflow, status: r.status, conclusion: r.conclusion, url: r.url }])), + ...(gc.checks ?? []), + ]; + const runs = items.map((c, i) => ({ id: -(i + 1), repo: target.repo, workflow: c.name, status: c.status, conclusion: c.conclusion, sha: gc.head ? gc.head.slice(0, 12) : null, prNumber: target.pr ?? null, url: c.url, })); @@ -260,7 +387,7 @@ export function githubSet(gc, target) { failed: runs.filter(failed), queued: [], etaGreenMs: null, - state: githubState(gc), + state: verdict.state, superseded: [], }; return { ...set, progress: progress(set) }; @@ -286,15 +413,24 @@ const realSleep = (ms, signal) => new Promise((resolve) => { // source: what the daemon reads each time round — // glance() the fleet view (/api/glance) // orphans(glance) orphaned jobs, lib/orphans.js -// prChecks(target) GitHub's checks for the target (null when GitHub did not answer) +// prChecks(target) GitHub's answer for the target's head, `gc` above +// (null when GitHub did not answer) // run(repo, id) one run's latest attempt as a glance row (null likewise) // // The rules, each from a wait that once went wrong: +// - GitHub names the head. The fleet view learns a new head only from its +// first run, so right after a push it still holds the previous head's +// result (stream-app PR #5, 2026-10-24: "1 of 1 green" from the old +// commit 8 s after the push, while the new head's run was queued). +// - The fleet's green is a rollup of the runs it has seen. It ends the wait +// only once GitHub agrees (githubVerdict: every run and job for the head +// completed and passed, and quiet for SETTLE_MS); until then the wait +// stays pending with GitHub's reason. GitHub not answering is not a yes. // - A stale fleet view (the fast loop stopped finishing ticks, shop-ios PR // #445, 2026-10-07) or one with no runs for the target is not believed: // GitHub is asked every crossStaleMs; otherwise every crossFreshMs, // because a view can freeze while ticks still finish. GitHub's finished -// answer ends the wait; its "pending" never overrides the fleet's result. +// answer ends the wait; its "pending" never overrides the fleet's red. // - Before a red ends the wait, each failed run is asked of GitHub: a // re-run keeps the run id and the view can lag it by a tick (jot-ios PR // #181, 2026-10-11). A run running again is pending. @@ -303,6 +439,7 @@ const realSleep = (ms, signal) => new Promise((resolve) => { // - A pointless verdict from a stale view is not a reason to stop. export async function waitForChecks(target, source, { waitS = 0, signal = null, now = Date.now, sleep = realSleep, tickMs = 2000, crossFreshMs = 120_000, crossStaleMs = 30_000, + confirmMs = 10_000, settleMs = SETTLE_MS, } = {}) { const start = now(); const deadline = start + Math.min(Math.max(0, waitS), MAX_WAIT_S) * 1000; @@ -310,34 +447,71 @@ export async function waitForChecks(target, source, { let nextCross = start + crossFreshMs; let askedAt = null; let lastGitHub = null; + let verdict = null; + let unread = false; // a verdict not yet weighed against the fleet's pending const finish = (state, extra) => ({ state, waitedMs: now() - start, ...extra }); + const askGitHub = async () => { + askedAt = now(); + const gc = await source.prChecks(target).catch(() => null); + if (!gc) return null; + lastGitHub = { ...gc, fetchedAt: gc.fetchedAt ?? askedAt }; + verdict = githubVerdict(lastGitHub, { settleMs }); + unread = true; + return verdict; + }; + const githubAnswer = (v, g, d) => { + const set = githubSet(lastGitHub, target, v); + return finish(v.state, { + source: 'github', checks: set, reason: null, + hint: v.state === 'red' && set.failed.some(cancelled) ? CANCELLED_HINT : null, + fleetAgeMs: g.ageMs ?? null, fleetSaid: d.set?.progress ?? null, + verified: v.state === 'green' ? confirmation(lastGitHub) : null, + }); + }; + + // The head first: a fleet answer about the previous head is no answer. + await askGitHub(); for (;;) { const g = source.glance(); - const view = overlay(g, [...fresh.values()]); - let d = decide(view, target); + const t = lastGitHub?.head ? { ...target, head: lastGitHub.head } : target; + let d = decide(overlay(g, [...fresh.values()]), t); + let held = null; if (d.state === 'red') { for (const r of d.set.failed.filter((x) => x.id > 0)) { const f = await source.run(target.repo, r.id).catch(() => null); if (f) fresh.set(f.id, f); } - d = decide(overlay(g, [...fresh.values()]), target); + d = decide(overlay(g, [...fresh.values()]), t); } - if (d.state === 'green' || d.state === 'red') { - const reran = d.state === 'green' && fresh.size > 0; - return finish(d.state, { - source: reran ? 'github' : 'fleet', checks: d.set, reason: null, - hint: d.state === 'red' && d.set.failed.some(cancelled) ? CANCELLED_HINT : null, + if (d.state === 'red') { + return finish('red', { + source: 'fleet', checks: d.set, reason: null, + hint: d.set.failed.some(cancelled) ? CANCELLED_HINT : null, fleetAgeMs: g.ageMs ?? null, }); } + if (d.state === 'green') { + if (askedAt === null || now() - askedAt >= confirmMs) await askGitHub(); + // The head moved while we looked: judge the new one. + if (lastGitHub?.head && !sameCommit(lastGitHub.head, t.head)) continue; + if (verdict?.state === 'green') { + return finish('green', { + source: fresh.size > 0 ? 'github' : 'fleet', checks: d.set, reason: null, hint: null, + fleetAgeMs: g.ageMs ?? null, verified: confirmation(lastGitHub), + }); + } + if (verdict?.state === 'red') return githubAnswer(verdict, g, d); + held = lastGitHub ? verdict.why : 'GitHub did not answer, and a green is not believed without it'; + } if (d.state === 'pointless' && !g.stale) { return finish('pointless', { source: 'fleet', checks: d.set, reason: d.reason, hint: null, fleetAgeMs: g.ageMs ?? null }); } const runIds = new Set((d.set?.runs ?? []).filter(isActive).map((r) => r.id)); + for (const r of lastGitHub?.runs ?? []) if (isActive(r)) runIds.add(r.id); for (const c of lastGitHub?.checks ?? []) { const id = c.status !== 'completed' ? runIdFromUrl(c.url) : null; if (id) runIds.add(id); @@ -350,30 +524,26 @@ export async function waitForChecks(target, source, { }); } + // A held green asks GitHub above, every confirmMs; this is for the rest. const eager = g.stale || !d.set || [...fresh.values()].some(isActive); if (eager) nextCross = Math.min(nextCross, askedAt === null ? now() : askedAt + crossStaleMs); - if (now() >= nextCross) { - askedAt = now(); - const gc = await source.prChecks(target).catch(() => null); - if (gc) { - lastGitHub = gc; - const state = githubState(gc); - if (state !== 'pending') { - return finish(state, { - source: 'github', checks: githubSet(gc, target), reason: null, hint: null, fleetAgeMs: g.ageMs ?? null, - fleetSaid: d.set?.progress ?? null, - }); - } + if (!held) { + if (now() >= nextCross) { + await askGitHub(); + nextCross = now() + (eager ? crossStaleMs : crossFreshMs); } - nextCross = now() + (eager ? crossStaleMs : crossFreshMs); + // GitHub's finished answer ends a wait the fleet has not finished. + if (unread && verdict && verdict.state !== 'pending') return githubAnswer(verdict, g, d); } + unread = false; if (now() >= deadline || signal?.aborted) { - const useGitHub = (g.stale || !d.set) && lastGitHub; + const useGitHub = lastGitHub && (g.stale || !d.set || held); return finish('pending', { source: useGitHub ? 'github' : 'fleet', - checks: useGitHub ? githubSet(lastGitHub, target) : d.set, - reason: d.state === 'pointless' ? `${d.reason} (from a stale fleet view)` : null, + checks: useGitHub ? githubSet(lastGitHub, target, verdict) : d.set, + reason: held ? `the fleet reads ${d.set.progress}, but ${held}` + : d.state === 'pointless' ? `${d.reason} (from a stale fleet view)` : null, hint: null, fleetAgeMs: g.ageMs ?? null, }); diff --git a/dashboard/scripts/make-glance-fixtures.mjs b/dashboard/scripts/make-glance-fixtures.mjs index c46e88f..2c36a2b 100644 --- a/dashboard/scripts/make-glance-fixtures.mjs +++ b/dashboard/scripts/make-glance-fixtures.mjs @@ -62,11 +62,28 @@ const orphan = classifyJob({ ...recorded, run_id: pendingRun.id }, { repo, runner: { name: recorded.runner_name, state: 'draining' }, now: Date.parse(recorded.steps.at(-1).started_at) + 600_000, }); const drain = { id: 'runner.drain', label: 'Drain runner', danger: 'medium', confirm: 'Stops the runner after its current job.', scope: 'act', args: ACTION_ARGS['runner.drain'] }; +// GitHub's answer for a commit, made from the view's own rows: each run with +// one job, as long ago as the view says it finished. +const githubFor = (view) => async (target) => { + const rows = [...view.runs, ...view.recent].filter((r) => r.repo === target.repo && r.sha === target.sha); + if (!rows.length) return null; + return { + head: `${target.sha}${'0'.repeat(40 - target.sha.length)}`, + fetchedAt: s.now, + checks: [], + runs: rows.map((r) => ({ + id: r.id, workflowId: r.workflow, workflow: r.workflow, event: r.event, status: r.status, conclusion: r.conclusion ?? null, + createdAt: r.startedAt, updatedAt: r.updatedAt, url: r.url, + jobs: r.status !== 'completed' ? null + : [{ name: r.workflow, status: 'completed', conclusion: r.conclusion, url: r.url, completedAt: r.updatedAt }], + })), + }; +}; const ctx = (view, orphans = []) => ({ glance: () => view, orphans: () => orphans, waitForChecks: (target, opts) => waitForChecks(target, { - glance: () => view, orphans: () => orphans, prChecks: async () => null, run: async () => null, + glance: () => view, orphans: () => orphans, prChecks: githubFor(view), run: async () => null, }, { ...opts, now: () => s.now }), readOnly: () => false, actionInfo: (id) => (id === drain.id ? drain : null), diff --git a/dashboard/test/checks.test.js b/dashboard/test/checks.test.js index e7d995f..5e740de 100644 --- a/dashboard/test/checks.test.js +++ b/dashboard/test/checks.test.js @@ -4,7 +4,7 @@ import { readFileSync } from 'node:fs'; import { join, dirname } from 'node:path'; import { fileURLToPath } from 'node:url'; import { - checkSets, findCheckSet, decide, waitForChecks, parseGitHubChecks, githubState, runIdFromUrl, + checkSets, findCheckSet, decide, waitForChecks, parseGitHubChecks, githubState, githubVerdict, githubSet, runIdFromUrl, } from '../lib/checks.js'; // The same cases as cockpit's RollupTests.swift, against the same glance @@ -214,41 +214,225 @@ describe('GitHub\'s checks', () => { }); // A clock the loop advances itself, so a five-minute wait takes no real time. +const T0 = 1_000_000_000; function clock() { - let t = 1_000_000; + let t = T0; return { now: () => t, sleep: async (ms) => { t += ms; } }; } +// One workflow run as GitHub has it (the shape parseGitHubChecks makes): its +// jobs once it has completed, none while it runs. +function ghRun(id, workflow, { + status = 'completed', conclusion = 'success', jobs = ['build'], created = T0 - 600_000, updated = T0 - 300_000, +} = {}) { + const done = status === 'completed'; + return { + id, workflowId: `wf-${workflow}`, workflow, event: 'pull_request', status, conclusion: done ? conclusion : null, + createdAt: created, updatedAt: updated, url: `https://github.com/acme/comet-web/actions/runs/${id}`, + jobs: done ? jobs.map((j) => (typeof j === 'string' + ? { name: j, status: 'completed', conclusion: 'success', url: null, completedAt: updated } + : { url: null, completedAt: updated, ...j })) : null, + }; +} +const ghAnswer = (head, runs, checks = []) => ({ head, runs, checks }); + function source(g, extra = {}) { return { glance: () => g, orphans: () => [], prChecks: async () => null, run: async () => null, ...extra }; } +describe('GitHub\'s verdict', () => { + test('every run completed and passed, quiet for the settle window: green', () => { + const gc = { ...ghAnswer('abc1234', [ghRun(1, 'ci', { jobs: ['guard', 'changes', 'web-e2e'] })]), fetchedAt: T0 }; + assert.deepEqual(githubVerdict(gc), { state: 'green', why: null }); + assert.equal(githubSet(gc, target).progress, '3 of 3 green'); + }); + + // stream-app PR #5, 2026-10-24: guard and changes had passed; the six + // jobs behind `needs: changes` did not exist yet, so every check GitHub + // listed was green while run 40116718009 was still in progress. + test('a run still in progress is pending, however green its first jobs are', () => { + const gc = parseGitHubChecks('d42a20fa8be5e24c477004b82a8c2194eacd6493', { + check_runs: ['guard', 'changes'].map((name, i) => ({ + name, status: 'completed', conclusion: 'success', completed_at: '2026-10-24T17:34:46Z', + html_url: `https://github.com/acme/stream-app/actions/runs/40116718009/job/12093940475${i}`, + })), + }, { statuses: [] }, { + runs: [{ id: 40116718009, workflow_id: 7, name: 'ci', event: 'pull_request', status: 'in_progress', conclusion: null, + created_at: '2026-10-24T17:34:12Z', updated_at: '2026-10-24T17:34:46Z', html_url: 'https://github.com/acme/stream-app/actions/runs/40116718009' }], + jobs: new Map(), + fetchedAt: Date.parse('2026-10-24T17:36:00Z'), + }); + // The check-only rule this replaces read the same answer as green. + assert.equal(githubState({ head: gc.head, checks: [{ name: 'guard', status: 'completed', conclusion: 'success' }, { name: 'changes', status: 'completed', conclusion: 'success' }] }), 'green'); + assert.equal(gc.checks.length, 0, 'the run\'s own check runs are its jobs, not extra checks'); + assert.deepEqual(githubVerdict(gc), { state: 'pending', why: 'GitHub still has ci in progress' }); + assert.equal(githubSet(gc, target).progress, '0 of 1 done'); + }); + + test('a green that just finished waits out the settle window', () => { + const gc = ghAnswer('abc1234', [ghRun(1, 'ci', { updated: T0 - 5_000 })]); + const early = githubVerdict({ ...gc, fetchedAt: T0 }); + assert.equal(early.state, 'pending'); + assert.match(early.why, /settling: .* changed 5 s ago, and a green waits 30 s/); + assert.equal(githubVerdict({ ...gc, fetchedAt: T0 + 25_000 }).state, 'green'); + assert.equal(githubVerdict({ ...gc, fetchedAt: T0 + 25_000 }, { settleMs: 60_000 }).state, 'pending'); + }); + + test('a completed run whose jobs are unread, or a job still running, is pending', () => { + assert.equal(githubVerdict({ ...ghAnswer('abc', [{ ...ghRun(1, 'ci'), jobs: null }]), fetchedAt: T0 }).state, 'pending'); + const stuck = ghRun(1, 'ci', { jobs: ['build', { name: 'deploy', status: 'in_progress', conclusion: null }] }); + assert.deepEqual(githubVerdict({ ...ghAnswer('abc', [stuck]), fetchedAt: T0 }), { state: 'pending', why: 'GitHub still has deploy running' }); + }); + + test('a failed or cancelled job is red; skipped and neutral pass', () => { + const at = (jobs) => githubVerdict({ ...ghAnswer('abc', [ghRun(1, 'ci', { jobs })]), fetchedAt: T0 }).state; + assert.equal(at(['build', { name: 'e2e', status: 'completed', conclusion: 'failure' }]), 'red'); + assert.equal(at([{ name: 'e2e', status: 'completed', conclusion: 'cancelled' }]), 'red'); + assert.equal(at([{ name: 'e2e', status: 'completed', conclusion: 'skipped' }, { name: 'n', status: 'completed', conclusion: 'neutral' }]), 'green'); + assert.equal(githubVerdict({ ...ghAnswer('abc', [ghRun(1, 'ci', { conclusion: 'startup_failure', jobs: [] })]), fetchedAt: T0 }).state, 'red'); + }); + + test('the newest run of a workflow wins over the one it replaced on the same commit', () => { + const replaced = ghRun(1, 'ci', { conclusion: 'cancelled', jobs: [{ name: 'build', status: 'completed', conclusion: 'cancelled' }] }); + assert.equal(githubVerdict({ ...ghAnswer('abc', [replaced, ghRun(2, 'ci')]), fetchedAt: T0 }).state, 'green'); + assert.equal(githubVerdict({ ...ghAnswer('abc', [replaced, ghRun(2, 'ci', { status: 'queued' })]), fetchedAt: T0 }).state, 'pending'); + }); + + test('another app\'s check, a status, and a check of a run GitHub did not list still count', () => { + const gc = parseGitHubChecks('abc', { + check_runs: [ + { name: 'build', status: 'completed', conclusion: 'success', html_url: 'https://github.com/a/b/actions/runs/9/job/1' }, + { name: 'other-run', status: 'in_progress', html_url: 'https://github.com/a/b/actions/runs/10/job/2' }, + { name: 'codecov', status: 'completed', conclusion: 'success', details_url: 'https://codecov.io/x', completed_at: '2026-10-24T17:00:00Z' }, + ], + }, { statuses: [{ context: 'ci/legacy', state: 'success', updated_at: '2026-10-24T17:00:00Z' }] }, { + runs: [{ id: 9, workflow_id: 1, name: 'ci', status: 'completed', conclusion: 'success', created_at: '2026-10-24T16:50:00Z', updated_at: '2026-10-24T17:00:00Z' }], + jobs: new Map([[9, [{ name: 'build', status: 'completed', conclusion: 'success', completed_at: '2026-10-24T17:00:00Z' }]]]), + fetchedAt: Date.parse('2026-10-24T18:00:00Z'), + }); + assert.deepEqual(gc.checks.map((c) => c.name), ['other-run', 'codecov', 'ci/legacy']); + assert.equal(githubVerdict(gc).state, 'pending'); + }); + + test('nothing on GitHub yet, or no answer, is pending', () => { + assert.equal(githubVerdict({ head: 'abc', runs: [], checks: [], fetchedAt: T0 }).state, 'pending'); + assert.equal(githubVerdict(null).state, 'pending'); + }); +}); + describe('waiting', () => { - test('a finished result answers at once, without asking GitHub', async () => { + test('a finished fleet green ends the wait once GitHub agrees, and says what GitHub covered', async () => { let asked = 0; - const out = await waitForChecks(target, source(glance({ recent: [run(1, 'ci')] }), { prChecks: async () => { asked++; return null; } }), { waitS: 60 }); + const out = await waitForChecks(target, source(glance({ recent: [run(1, 'ci')] }), { + prChecks: async () => { asked++; return ghAnswer('abc1234def', [ghRun(1, 'ci', { jobs: ['a', 'b'] })]); }, + }), { waitS: 60 }); assert.equal(out.state, 'green'); assert.equal(out.source, 'fleet'); - assert.equal(asked, 0); + assert.equal(asked, 1); + assert.deepEqual(out.verified, { head: 'abc1234def', runs: 1, jobs: 2, checks: 0 }); }); - test('wait=0 on a fresh, pending view answers pending without asking GitHub', async () => { + test('a fleet green GitHub cannot confirm is not a green', async () => { + const c = clock(); + const out = await waitForChecks(target, source(glance({ recent: [run(1, 'ci')] })), { waitS: 60, now: c.now, sleep: c.sleep }); + assert.equal(out.state, 'pending'); + assert.match(out.reason, /^the fleet reads 1 of 1 green, but GitHub did not answer/); + }); + + // stream-app PR #5, 2026-10-24, replayed: the wait began 8 s after the + // push. The fleet still knew only the old head (8c2d77c, green) and had not + // seen run 40116718009 on the new one (d42a20f, queued), and returned the old + // head's "1 of 1 green" at once. Now GitHub names the head, and the green + // comes after the new run's nine jobs pass and GitHub stays quiet 30 s. + test('right after a push, the old head\'s green does not end the wait (stream-app PR #5)', async () => { + const c = clock(); + const repo = 'acme/stream-app'; + const pr5 = { repo, pr: 5 }; + const oldHead = run(40112490807, 'ci', { repo, pr: 5, sha: '8c2d77c', head: '8c2d77c18879', updated: T0 - 120_000 }); + const jobs = ['guard', 'changes', 'typescript', 'firetv-core', 'roku-offdevice', 'tvos-core', 'sql', 'web-e2e', 'ci-success']; + const finishedAt = T0 + 160_000; + const newHead = 'd42a20fa8be5e24c477004b82a8c2194eacd6493'; + let asks = 0; + const out = await waitForChecks(pr5, { + glance: () => glance({ + // The fleet catches up with the new run 40 s in. + runs: c.now() < T0 + 40_000 ? [] + : c.now() < finishedAt ? [run(40116718009, 'ci', { repo, pr: 5, sha: 'd42a20f', head: 'd42a20fa8be5', status: 'in_progress' })] : [], + recent: [oldHead, ...(c.now() >= finishedAt ? [run(40116718009, 'ci', { repo, pr: 5, sha: 'd42a20f', head: 'd42a20fa8be5', updated: finishedAt })] : [])], + }), + orphans: () => [], + run: async () => null, + prChecks: async () => { + asks++; + const status = c.now() < T0 + 30_000 ? 'queued' : c.now() < finishedAt ? 'in_progress' : 'completed'; + return ghAnswer(newHead, [ghRun(40116718009, 'ci', { status, jobs, created: T0 - 8_000, updated: status === 'completed' ? finishedAt : T0 })]); + }, + }, { waitS: 300, now: c.now, sleep: c.sleep }); + assert.equal(out.state, 'green'); + assert.ok(out.waitedMs >= 190_000, `green after ${out.waitedMs} ms: before the run finished and settled`); + assert.equal(out.checks.sha, 'd42a20f'); + assert.deepEqual(out.verified, { head: newHead, runs: 1, jobs: 9, checks: 0 }); + assert.ok(asks < 40, `asked GitHub ${asks} times`); + }); + + test('wait=0 right after a push reads pending, not the old head\'s green', async () => { + const repo = 'acme/stream-app'; + const oldHead = run(40112490807, 'ci', { repo, pr: 5, sha: '8c2d77c', head: '8c2d77c18879' }); + const out = await waitForChecks({ repo, pr: 5 }, source(glance({ recent: [oldHead] }), { + prChecks: async () => ghAnswer('d42a20fa8be5e24c477004b82a8c2194eacd6493', [ghRun(40116718009, 'ci', { status: 'queued' })]), + }), { waitS: 0 }); + assert.equal(out.state, 'pending'); + assert.equal(out.source, 'github'); + assert.equal(out.checks.progress, '0 of 1 done'); + }); + + // A workflow_run workflow is created only after the run it follows + // completes: the first run's green is not the commit's. + test('a second run registering after the first finished keeps the wait open', async () => { + const c = clock(); + const first = ghRun(11, 'ci', { updated: T0 - 2_000 }); + const out = await waitForChecks(target, source(glance({ recent: [run(11, 'ci')] }), { + prChecks: async () => { + const t = c.now(); + if (t < T0 + 15_000) return ghAnswer('abc1234def', [first]); + const status = t < T0 + 100_000 ? 'in_progress' : 'completed'; + return ghAnswer('abc1234def', [first, ghRun(12, 'deploy-preview', { status, created: T0 + 12_000, updated: status === 'completed' ? T0 + 100_000 : T0 + 12_000 })]); + }, + }), { waitS: 300, now: c.now, sleep: c.sleep }); + assert.equal(out.state, 'green'); + assert.ok(out.waitedMs >= 130_000, `green after ${out.waitedMs} ms`); + assert.equal(out.verified.runs, 2); + }); + + test('a run the fleet has not seen that failed on GitHub turns its green red', async () => { + const out = await waitForChecks(target, source(glance({ recent: [run(11, 'ci')] }), { + prChecks: async () => ghAnswer('abc1234def', [ghRun(11, 'ci'), ghRun(12, 'lint', { conclusion: 'cancelled', jobs: [{ name: 'lint', status: 'completed', conclusion: 'cancelled' }] })]), + }), { waitS: 60 }); + assert.equal(out.state, 'red'); + assert.equal(out.source, 'github'); + assert.deepEqual(out.checks.failed.map((r) => r.workflow), ['lint']); + assert.match(out.hint, /cancelled is not a pass/); + }); + + test('wait=0 on a fresh, pending view answers pending from the fleet', async () => { let asked = 0; const g = glance({ runs: [run(1, 'ci', { status: 'in_progress', elapsed: 1, expected: 10 })] }); const out = await waitForChecks(target, source(g, { prChecks: async () => { asked++; return null; } }), { waitS: 0 }); assert.equal(out.state, 'pending'); + assert.equal(out.source, 'fleet'); assert.equal(out.checks.progress, '0 of 1 done'); - assert.equal(asked, 0); + assert.equal(asked, 1, 'once, for the head'); }); test('a stale view asks GitHub, and GitHub\'s green ends the wait', async () => { const g = { ...glance({ runs: [run(1, 'ci', { status: 'in_progress' })] }), stale: true }; const out = await waitForChecks(target, source(g, { - prChecks: async () => ({ head: 'abc1234def', checks: [{ name: 'ci', status: 'completed', conclusion: 'success', url: null }] }), + prChecks: async () => ghAnswer('abc1234def', [ghRun(1, 'ci', { jobs: ['ci'] })]), }), { waitS: 0 }); assert.equal(out.state, 'green'); assert.equal(out.source, 'github'); assert.equal(out.fleetSaid, '0 of 1 done'); + assert.equal(out.verified.jobs, 1); }); test('GitHub\'s pending never overrides the fleet; the wait times out pending', async () => { @@ -256,11 +440,11 @@ describe('waiting', () => { let asked = 0; const g = { ...glance({ runs: [run(1, 'ci', { status: 'in_progress' })] }), stale: true }; const out = await waitForChecks(target, source(g, { - prChecks: async () => { asked++; return { head: 'abc', checks: [{ name: 'ci', status: 'in_progress', conclusion: null }] }; }, + prChecks: async () => { asked++; return ghAnswer('abc', [ghRun(1, 'ci', { status: 'in_progress' })]); }, }), { waitS: 90, now: c.now, sleep: c.sleep, tickMs: 1000 }); assert.equal(out.state, 'pending'); assert.equal(out.source, 'github'); - assert.ok(asked >= 3 && asked <= 4, `asked ${asked} times in 90 s`); + assert.ok(asked >= 3 && asked <= 5, `asked ${asked} times in 90 s`); }); test('a red is asked of GitHub first, and a re-run in progress keeps it pending', async () => { @@ -282,6 +466,15 @@ describe('waiting', () => { assert.match(out.hint, /cancelled is not a pass/); }); + test('a red of the commit GitHub names as the head, not of the one before it', async () => { + const c = clock(); + const before = run(1, 'ci', { conclusion: 'failure', sha: 'aaaaaaa', head: 'aaaaaaaaaaaa' }); + const out = await waitForChecks(target, source(glance({ recent: [before] }), { + prChecks: async () => ghAnswer('bbbbbbbbbbbbbbbb', [ghRun(2, 'ci', { status: 'queued' })]), + }), { waitS: 5, now: c.now, sleep: c.sleep }); + assert.equal(out.state, 'pending'); + }); + test('an orphan among the target\'s runs ends the wait as pointless, with its recovery', async () => { const g = glance({ runs: [run(38318681067, 'docs', { status: 'in_progress' })] }); const orphan = { repo: 'acme/comet-web', runId: 38318681067, summary: 'build: orphaned by a draining runner — not your code.', recovery: ['x'] }; diff --git a/docs/api.md b/docs/api.md index 2da16b4..ed3dcd4 100644 --- a/docs/api.md +++ b/docs/api.md @@ -131,8 +131,11 @@ itself. open until the PR's checks finish, for at most `wait` seconds (300 at most). It returns one of four states: -- `green`: every check passed. Only `success`, `skipped` and `neutral` count - as a pass. +- `green`: every check passed, and GitHub agrees: every workflow run it has + for the head commit is completed, every job of each run passed, and nothing + new appeared for 30 seconds. Only `success`, `skipped` and `neutral` count + as a pass. `verified` says what GitHub's side covered + (`{ head, runs, jobs, checks }`). - `red`: a check finished without passing. `cancelled` is red, and `hint` says so. - `pending`: the wait ran out. Ask again. @@ -142,8 +145,19 @@ It returns one of four states: runner is dead, or a job was orphaned (`orphans` lists them with their recovery). -A PR's checks are its head commit's checks. An older commit's result never -answers for the PR, even after a force-push. +A PR's checks are its head commit's checks, and GitHub names the head (the +PR's branch ref while it is open, since the PR's own `head.sha` catches up a +moment after a push). An older commit's result never answers for the PR, even +right after a push or a force-push. + +A green is never the fleet view's alone. The view rolls up the runs it has +seen, and right after a push it has not seen them all: runs for `push` and +`pull_request` register separately, a `workflow_run` run only after the run it +follows completes, and a job behind `needs:` only when its needs finish. So +before a green ends the wait the daemon reads `actions/runs?head_sha=` and each +completed run's jobs, and holds the green (`pending`, with `reason`) until all +of them are done and GitHub has been quiet for 30 seconds. If GitHub does not +answer, the wait stays pending rather than trust the view. The answer comes from the fleet view (`source: "fleet"`) unless the view is stale, has no runs for the target, or is behind a re-run. In those cases the diff --git a/docs/cockpit/cli.md b/docs/cockpit/cli.md index 81bd47e..df6dfde 100644 --- a/docs/cockpit/cli.md +++ b/docs/cockpit/cli.md @@ -190,7 +190,7 @@ $ cockpit wait comet-web --pr 96 | Exit | Printed | Meaning | |---|---|---| -| `0` | `✔` | Every check passed | +| `0` | `✔` | Every check passed, and GitHub confirms every run and job for the head finished (see [a green is confirmed on GitHub](#a-green-is-confirmed-on-github)) | | `1` | `✖` and each failed check, with its URL | At least one failed or was cancelled. A cancellation is often an admission hold that ran past `timeout-minutes`, and the output says so | | `2` | `⏹ not waiting — ` | Waiting cannot end well: the host is down, the disk floor is holding jobs, GitHub is refusing jobs for billing, one of the checks is queued with a cause that never clears, or a job was orphaned by a draining runner (the reason carries the recovery commands). Stop and report the reason | | `3` | `…timed out` or `? dashboard unreachable` | The timeout passed, or (for `--sha` and `--branch`) the dashboard could not be reached; try `cockpit sentinel` | @@ -201,11 +201,12 @@ past the deadline whatever else is happening. A dashboard that stops answering does not end the wait. The tunnel is reopened and the request retried every 10 seconds, and with `--pr` cockpit asks GitHub itself meanwhile, once a minute, with your own `gh` login -(`gh pr view --json headRefOid,statusCheckRollup`). GitHub's finished answer -ends the wait and says so: +(`gh pr view --json headRefOid,statusCheckRollup`, the branch ref, and +`actions/runs?head_sha=`). GitHub's finished answer, by the same rule as +[a confirmed green](#a-green-is-confirmed-on-github), ends the wait and says so: ```console -✔ comet-web PR #96: 1 of 1 green (from GitHub; the dashboard did not answer) +✔ comet-web PR #96: 1 of 1 green (from GitHub) ``` A `--sha` or `--branch` wait needs the dashboard, and exits `3` when it cannot @@ -219,6 +220,37 @@ does not end the wait. Until the head has a run, the wait keeps waiting. After a force-push back to an earlier commit, that commit's old result waits for the new run ("waiting for kit-ci to run again on this commit"). +### A green is confirmed on GitHub + +The fleet's view rolls up the runs it has seen, and right after a push it has +not seen them all. On acme/stream-app PR #5 (2026-10-24) a wait that +started 8 seconds after a push printed `1 of 1 green` at once: the view still +held the previous head's run, and the new head's run, with nine jobs, was +queued. Every agent merges on exit `0`, so that could merge untested code. + +Now GitHub names the head (an open PR's branch ref), and a green ends the wait +only when GitHub has, for that head: + +- every workflow run (`actions/runs?head_sha=`) completed, the newest of each + workflow counting; +- every job of those runs, and every other check and status, completed with + `success`, `skipped` or `neutral`; +- nothing new for 30 seconds: runs for `push` and `pull_request` register + separately, a `workflow_run` run appears only after the run it follows, and + a job behind `needs:` only once its needs finish. + +Until then the wait prints GitHub's count and reason: + +```console +… stream-app PR #5: 2 of 3 done +… stream-app PR #5: 9 of 9 done — settling: GitHub's newest run or job for d42a20f changed 4 s ago, and a green waits 30 s for any run still to register +✔ stream-app PR #5: 1 of 1 green (GitHub: 1 run, 9 jobs) +``` + +GitHub not answering is not a yes: the wait stays pending. With a dashboard +older than this rule (no `verified` in its answer), the CLI confirms the green +itself with your `gh` login. + ### A red is checked against the run's latest attempt A re-run (`gh run rerun`, with or without `--failed`) keeps the run id, and the @@ -245,7 +277,8 @@ GitHub's **finished** answer ends the wait, with how far behind the fleet was: ``` That note is a daemon problem worth reporting, not part of the PR's result. -GitHub still *pending* never overrides the fleet's own green or red. +GitHub still *pending* never overrides the fleet's own red, and holds its +green until GitHub finishes. ### A job orphaned by a draining runner