diff --git a/.changes/source-control/fix-10437-cost-hold-deleted-run-4e6b4dfc.md b/.changes/source-control/fix-10437-cost-hold-deleted-run-4e6b4dfc.md new file mode 100644 index 0000000000..6f5ffdd903 --- /dev/null +++ b/.changes/source-control/fix-10437-cost-hold-deleted-run-4e6b4dfc.md @@ -0,0 +1,7 @@ +--- +bump: patch +--- + +### Fixed + +- `cost_decision` keeps a cost hold when a lane run of a newer head was deleted. Deleting a run left the older run's notice as the newest, so an older head became the decided head. The decided head now stands only while the head branch's push history (the repository activity API, one page, which deleting a run leaves) shows its push before the decision and a run of every review lane, started before the decision, on each head pushed after that push and before the decision (`unran_head`). A full page or an unreadable history carries nothing. diff --git a/.github/scripts/pr-merge/enable-auto-merge.test.mjs b/.github/scripts/pr-merge/enable-auto-merge.test.mjs index bb089e41ee..693100c412 100644 --- a/.github/scripts/pr-merge/enable-auto-merge.test.mjs +++ b/.github/scripts/pr-merge/enable-auto-merge.test.mjs @@ -146,6 +146,10 @@ function world(pulls, { failArm = false, armErrors = {}, filesError = null, note if (method === "GET" && runs) { return { workflow_runs: runs[1] === "pr-review-hosted" ? [{ id: 9, head_sha: HEAD, head_repository: { full_name: REPO }, created_at: "2026-10-22T10:50:00Z" }] : [] }; } + // The head branch's push history: the head, pushed before its run. + if (method === "GET" && path === "/repos/melodic-software/claude-code-plugins/activity?ref=refs%2Fheads%2Ffix%2Fcost&direction=desc&per_page=100") { + return [{ after: HEAD, timestamp: "2026-10-22T10:45:00Z", activity_type: "push", ref: "refs/heads/fix/cost" }]; + } if (method === "GET" && path === `/repos/melodic-software/claude-code-plugins/compare/main...${HEAD}`) { return { files: [{ filename: "infra/main.tf" }] }; } diff --git a/.github/scripts/pr-merge/merge-holds.mjs b/.github/scripts/pr-merge/merge-holds.mjs index ae16f1711f..013cca9ed5 100755 --- a/.github/scripts/pr-merge/merge-holds.mjs +++ b/.github/scripts/pr-merge/merge-holds.mjs @@ -570,6 +570,46 @@ export function decidedHead(notices, at) { return heads.size === 1 ? [...heads][0] : null; } +// Why the head branch's push history shows a lane run missing before a +// decision made at `at`, else null (babysit_merge_policy.unran_head, and +// ci-status's cost-hold step). `activity` is one page of the repository +// activity API for the branch, newest first, which deleting a run leaves; +// `ran` lists, per REVIEW_LANES entry, the heads of its runs started before +// `at`. Every head the branch moved to after its last push of `decided` +// before `at` needs a run of every lane: with one deleted, an older notice +// would become the decided head (#10437). No push of `decided` before `at` +// or a full page, which may not reach it, keeps the hold too. +export function unranHead(activity, ran, decided, at) { + if (!Array.isArray(activity)) { + return "the branch's push history is unreadable"; + } + if (activity.length >= 100) { + return "the branch's push history fills a page"; + } + if (!Array.isArray(ran) || ran.length !== REVIEW_LANES.length || !ran.every(Array.isArray)) { + return "the lane runs do not cover every review lane"; + } + const moves = []; + for (const move of activity) { + const after = String(move?.after ?? ""); + if (/^[0-9a-f]{40}$/.test(after) && !/^0{40}$/.test(after)) { + const made = Date.parse(move.timestamp); + if (!Number.isFinite(made)) { + return "the branch's push history is unreadable"; + } + moves.push({ after, made }); + } + } + const cutoff = Date.parse(at); + const before = moves.filter((m) => m.made < cutoff); + const last = Math.max(...before.filter((m) => m.after === decided).map((m) => m.made)); + if (!Number.isFinite(last)) { + return `the branch shows no push of ${decided} before the decision`; + } + const gap = before.find((m) => m.after !== decided && m.made >= last && !ran.every((heads) => heads.includes(m.after))); + return gap ? `${gap.after} was pushed after the decided head with no run of every review lane before the decision` : null; +} + // Every item of the REST list at `path` whose answer holds it under `key`. async function listed(read, path, key) { const rows = []; @@ -593,9 +633,10 @@ async function listed(read, path, key) { // repository's same-named branch speaks for another pull request), each // run's head vetted against // `base` (laneChange), its status job's cost notices (costNotices) from jobs -// done before `at`, then decidedHead. A run on a head that fails the vet, or -// a read that fails, decides nothing. `read` holds actions: read and -// checks: read. +// done before `at`, then decidedHead, kept only while the branch's push +// history shows no lane run missing since the decided head (unranHead). A +// run on a head that fails the vet, or a read that fails, decides nothing. +// `read` holds contents: read, actions: read and checks: read. async function noticedHead(read, repo, branch, base, at) { const cutoff = Date.parse(at); if (!Number.isFinite(cutoff) || typeof branch !== "string" || !BRANCH.test(branch)) { @@ -603,8 +644,11 @@ async function noticedHead(read, repo, branch, base, at) { } const notices = []; const vetted = new Set(); + const ran = []; for (const [file, check] of REVIEW_LANES) { const runs = await listed(read, `/repos/${repo}/actions/workflows/${file}/runs?event=pull_request&branch=${encodeURIComponent(branch)}`, "workflow_runs"); + const heads = []; + ran.push(heads); for (const run of runs) { if (run?.head_repository?.full_name !== repo || !(Date.parse(run.created_at) < cutoff)) { continue; @@ -613,6 +657,7 @@ async function noticedHead(read, repo, branch, base, at) { if (!/^[0-9a-f]{40}$/.test(head)) { return null; } + heads.push(head); if (!vetted.has(head)) { if (laneChange(await read("GET", `/repos/${repo}/compare/${base}...${head}`)) !== null) { return null; @@ -626,7 +671,12 @@ async function noticedHead(read, repo, branch, base, at) { } } } - return decidedHead(notices, at); + const decided = decidedHead(notices, at); + if (decided === null) { + return null; + } + const activity = await read("GET", `/repos/${repo}/activity?ref=${encodeURIComponent(`refs/heads/${branch}`)}&direction=desc&per_page=100`); + return unranHead(activity, ran, decided, at) === null ? decided : null; } // Whether a COMMENT_EDITED_QUERY answer's node was never edited. diff --git a/.github/scripts/pr-merge/merge-holds.test.mjs b/.github/scripts/pr-merge/merge-holds.test.mjs index b94ef8b150..e54ef1735f 100644 --- a/.github/scripts/pr-merge/merge-holds.test.mjs +++ b/.github/scripts/pr-merge/merge-holds.test.mjs @@ -8,7 +8,9 @@ // request counts as it is, one on its linked issue (closing, else `Refs:`) // only when it names `#`. It covers the head it decided, the sha of the // newest cost hold notice a review lane's status job recorded before it on a -// run vetted not to change a review-lane workflow (#9635), and a later head +// run vetted not to change a review-lane workflow (#9635), while the head +// branch's push history shows a run of every lane on each head pushed since +// it before the decision (#10437), and a later head // only when the net change against the base branch is the same at both (owner // decisions Q17 c and Q18 d on #9436). import assert from "node:assert/strict"; @@ -17,7 +19,7 @@ import { writeFileSync } from "node:fs"; import { join } from "node:path"; import { test } from "node:test"; -import { BODY_WRITERS_QUERY, COMMENT_EDITED_QUERY, HOLD_PATH_CLASSES, HOLD_PATH_FLOOR, PULL_OPENED_QUERY, REVIEW_FLOOR, changeOf, costDecision, costHold, costNotices, decidedHead, evidenceHold, laneChange, linkHold, main, pathHold, sameNetChange } from "./merge-holds.mjs"; +import { BODY_WRITERS_QUERY, COMMENT_EDITED_QUERY, HOLD_PATH_CLASSES, HOLD_PATH_FLOOR, PULL_OPENED_QUERY, REVIEW_FLOOR, changeOf, costDecision, costHold, costNotices, decidedHead, evidenceHold, laneChange, linkHold, main, pathHold, sameNetChange, unranHead } from "./merge-holds.mjs"; import { tempDir } from "../../../lib/test-tmp.mjs"; const REPO = "owner/repo"; @@ -40,6 +42,18 @@ const laneRun = (sha, at = "2026-10-22T11:00:00Z", { criteria = "cost", lane = " created, notes: [{ annotation_level: "notice", message: `claude-lane-hold lane=claude-review sha=${notice} criteria=${criteria} label=needs-human` }], }); +// The head branch's history as the repository activity API lists it, newest +// first: a push of `after` at `timestamp`. +const push = (after, timestamp) => ({ after, timestamp, activity_type: "push", ref: "refs/heads/fix/cost" }); +const newestFirst = (moves) => [...moves].sort((a, b) => Date.parse(b.timestamp) - Date.parse(a.timestamp)); +// By default each run's head was pushed five minutes before its first status +// job finished. +const pushesOf = (runs) => + newestFirst( + Object.entries( + runs.reduce((first, run) => ({ ...first, [run.sha]: Math.min(first[run.sha] ?? Infinity, Date.parse(run.at)) }), {}), + ).map(([sha, at]) => push(sha, new Date(at - 300_000).toISOString())), + ); // A compare answer: the pull request's files against the base. const TF = { filename: "infra/main.tf", status: "modified", patch: "@@ -1 +1 @@\n-a\n+b" }; const net = (...files) => ({ files }); @@ -70,10 +84,18 @@ function world({ runs = [laneRun(HEAD)], compares = {}, base = "main", + activity = pushesOf(runs), } = {}) { const reads = []; const answers = { ...Object.fromEntries(runs.map((run) => [run.sha, net(TF)])), ...compares }; const github = async (method, path, body) => { + if (path === "/repos/owner/repo/activity?ref=refs%2Fheads%2Ffix%2Fcost&direction=desc&per_page=100") { + reads.push("activity"); + if (fail === "activity") { + throw new Error("activity failed"); + } + return activity; + } const compare = /^\/repos\/owner\/repo\/compare\/(.+)\.\.\.([0-9a-f]{40})$/.exec(path); const listing = /^\/repos\/owner\/repo\/actions\/workflows\/([a-z-]+\.yml)\/runs\?event=pull_request&branch=fix%2Fcost&per_page=100&page=1$/.exec(path); const jobs = /^\/repos\/owner\/repo\/actions\/runs\/([0-9]+)\/jobs\?filter=all&per_page=100&page=1$/.exec(path); @@ -153,6 +175,7 @@ test("an owner decision on the closing issue, linked from the body, clears the c "jobs 1", "notes 1", "runs pr-review-security-hosted.yml", + "activity", ]); }); @@ -500,7 +523,7 @@ test("a base merge after the decision carries it, and a pull request commit afte assert.equal((await later({ [OLD]: net(TF), [HEAD]: net(TF) })).link, LINK); const commit = await later({ [OLD]: net(TF), [HEAD]: net(TF, { filename: "scripts/run.sh", status: "modified", patch: "@@ -1 +1 @@\n-x\n+y" }) }); assert.equal(commit.link, null); - assert.deepEqual(commit.reads.slice(-3), ["runs pr-review-security-hosted.yml", `compare ${OLD}`, `compare ${HEAD}`]); + assert.deepEqual(commit.reads.slice(-3), ["activity", `compare ${OLD}`, `compare ${HEAD}`]); }); test("a rebase onto a newer base with the same net change carries, whatever order the files list in", async () => { @@ -647,6 +670,63 @@ test("a lane run on a head that changes a review-lane workflow, or whose compare } }); +// #10437: a deleted lane run of a head pushed after the decided head, before +// the decision, would make an older notice the decided head. The branch's +// push history, which deleting a run leaves, shows that head, and it must +// have a run of every review lane started before the decision. +test("a head pushed between the decided head and the decision with no run of every lane keeps the hold", async () => { + const NEW = "1111111111111111111111111111111111111111"; + const same = { [OLD]: net(TF), [HEAD]: net(TF) }; + const history = (...moves) => newestFirst([push(OLD, "2026-10-22T10:25:00Z"), ...moves]); + const plain = (sha, lane) => laneRun(sha, "2026-10-22T11:20:00Z", { criteria: "trust-gate", lane, created: "2026-10-22T11:12:00Z" }); + const carried = (runs, activity, options = {}) => decide(undefined, { runs: [laneRun(OLD), ...runs], compares: same, activity, ...options }); + // No deletion: both lanes ran on the newer head, which noticed no cost. + const both = [plain(NEW, "pr-review-hosted.yml"), plain(NEW, "pr-review-security-hosted.yml")]; + assert.equal((await carried(both, history(push(NEW, "2026-10-22T11:10:00Z")))).link, LINK); + // A head pushed after the decision needs no run here. + assert.equal((await carried([], history(push(NEW, "2026-10-22T12:10:00Z")))).link, LINK); + for (const [name, runs] of Object.entries({ + "both lanes' runs deleted": [], + "the code lane's run deleted": [plain(NEW, "pr-review-security-hosted.yml")], + "the security lane's run deleted": [plain(NEW, "pr-review-hosted.yml")], + "a run started only after the decision": both.map((run) => ({ ...run, created: "2026-10-22T12:05:00Z" })), + "a run whose head is in another repository": both.map((run) => ({ ...run, repo: "fork/repo" })), + })) { + const kept = await carried(runs, history(push(NEW, "2026-10-22T11:10:00Z"))); + assert.equal(kept.link, null, name); + assert.ok(kept.reads.includes("activity"), name); + } + // Pushing the decided head again after the decision makes it the head: the + // gap before the decision still keeps the hold. + const back = history(push(NEW, "2026-10-22T11:10:00Z"), push(OLD, "2026-10-22T12:30:00Z")); + assert.equal((await carried([], back, { head: OLD })).link, null); + // A history that shows no push of the decided head before the decision, a + // full page that may not reach it, or a read that fails keeps the hold. + assert.equal((await carried(both, [push(NEW, "2026-10-22T11:10:00Z")])).link, null); + assert.equal((await carried(both, [push(OLD, "2026-10-22T12:30:00Z")])).link, null); + const full = newestFirst([...Array.from({ length: 99 }, (_, i) => push(NEW, new Date(Date.parse("2026-10-22T12:10:00Z") + i * 1000).toISOString())), push(OLD, "2026-10-22T10:25:00Z")]); + assert.equal((await carried(both, full)).link, null); + assert.equal((await carried(both, full.slice(1))).link, LINK); + assert.equal((await carried(both, history(), { fail: "activity" })).link, null); + assert.equal((await carried(both, { message: "Not Found" })).link, null); +}); + +test("unranHead names the first head pushed after the decided head with no run of every lane", () => { + const NEW = "1111111111111111111111111111111111111111"; + const at = "2026-10-22T12:00:00Z"; + const moves = [push(NEW, "2026-10-22T11:10:00Z"), { ...push("0".repeat(40), "2026-10-22T11:05:00Z"), activity_type: "branch_deletion" }, push(OLD, "2026-10-22T10:25:00Z")]; + assert.equal(unranHead(moves, [[OLD, NEW], [NEW]], OLD, at), null); + assert.match(unranHead(moves, [[OLD], [NEW]], OLD, at), new RegExp(NEW)); + assert.match(unranHead(moves, [[OLD, NEW]], OLD, at), /every review lane/); + assert.match(unranHead(moves, null, OLD, at), /every review lane/); + assert.match(unranHead(moves, [[NEW], [NEW]], HEAD, at), /no push of/); + assert.match(unranHead(moves, [[NEW], [NEW]], OLD, "not a time"), /no push of/); + assert.match(unranHead([{ after: NEW, timestamp: "soon" }, ...moves], [[NEW], [NEW]], OLD, at), /unreadable/); + assert.match(unranHead("nope", [[], []], OLD, at), /unreadable/); + // Only the moves after the decided head's last push before the decision count. + assert.equal(unranHead([push(OLD, "2026-10-22T11:30:00Z"), ...moves], [[OLD], [OLD]], OLD, at), null); +}); + test("laneChange, costNotices and decidedHead", () => { assert.equal(laneChange(net(TF)), null); assert.equal(laneChange(net({ filename: ".github/workflows/pr-review-hosted.yml" })), "a review-lane workflow"); diff --git a/.github/workflows/pr-require-checks.yml b/.github/workflows/pr-require-checks.yml index 1e9c396842..ce62bd5085 100644 --- a/.github/workflows/pr-require-checks.yml +++ b/.github/workflows/pr-require-checks.yml @@ -3984,7 +3984,14 @@ jobs: # way and only where it names its own run's head; never a hold # comment, which any GITHUB_TOKEN workflow can post as # github-actions[bot]. With none, or two heads at that time, it covers - # no head. The linked issue is the one issue in this repository that + # no head. Nor does it while the head branch's push history (the + # repository activity API, which deleting a run leaves) shows a head + # pushed after the decided head's last push and before the decision + # with no run of every review lane started before the decision, since + # a deleted run of a newer head would make an older notice the decided + # head (the base's merge-holds.mjs unranHead, #10437); a history with + # no push of the decided head before it, or a full page, covers none + # too. The linked issue is the one issue in this repository that # GitHub records it as closing, else, when it closes none, the one issue # its plain `Refs:` lines name (the base's merge-holds.mjs refsIssue), # read to be an issue and not a pull request. A second closing issue @@ -4159,9 +4166,14 @@ jobs: # for another pull request), updated in the window or started before # the decision, as {at, criteria, sha, run}: `sha` as the notice # names it, `run` the head its run ran on. - notices='[]' vetted='' + # `ran` lists, per lane, the heads of its runs from this repository + # started before the decision. + notices='[]' vetted='' ran='[]' while read -r file check; do runs=$(read_api "repos/$repo/actions/workflows/$file/runs?event=pull_request&branch=$ref&per_page=100" --paginate) || keep "the $file runs are unreadable" + ran=$(jq -nc --argjson ran "$ran" --arg repo "$repo" --argjson decided "$decided_at" '$ran + [[inputs.workflow_runs[] + | select(.head_repository.full_name == $repo and (.created_at | fromdateiso8601) < $decided) | .head_sha] | unique]' <<<"$runs") || + keep "the $file runs do not parse" while read -r id sha; do [ -n "$id" ] || continue [[ $sha =~ ^[0-9a-f]{40}$ ]] || keep "run $id names no head commit" @@ -4195,6 +4207,19 @@ jobs: decided=$(jq -r --argjson at "$decided_at" '[.[] | select(.at < $at and .sha == .run and any(.criteria | split(",")[]; . == "cost"))] | (map(.at) | max) as $newest | [.[] | select(.at == $newest) | .sha] | unique | if length == 1 then .[0] else "" end' <<<"$notices") || keep "the hold notices do not parse" + # The decided head stands only while the head branch's push history + # shows a run of every lane on each head pushed since it before the + # decision (the base's unranHead; one page, as merge-holds.mjs reads it). + if [ -n "$decided" ]; then + read_api "repos/$repo/activity?ref=refs%2Fheads%2F$ref&direction=desc&per_page=100" >"$RUNNER_TEMP/cost-hold-activity.json" || + keep "the head branch's push history is unreadable" + printf '%s\n' "$ran" >"$RUNNER_TEMP/cost-hold-ran.json" + jq -n --arg sha "$decided" '$sha' >"$RUNNER_TEMP/cost-hold-decided.json" + jq '.comment.created_at' <<<"$chosen" >"$RUNNER_TEMP/cost-hold-decided-at.json" || keep "the decisions do not parse" + gap=$(holds_js unranHead "$RUNNER_TEMP/cost-hold-activity.json" "$RUNNER_TEMP/cost-hold-ran.json" \ + "$RUNNER_TEMP/cost-hold-decided.json" "$RUNNER_TEMP/cost-hold-decided-at.json") || keep "the head branch's push history could not be checked" + [ -z "$gap" ] || keep "$gap" + fi carries "$decided" || keep "the body cites no recorded owner cost decision covering this head" decision=$(jq -r .link <<<"$chosen") decision_id=$(jq -r .id <<<"$chosen") holds=$(jq -c --argjson since "$since" '[.[] | select(.at > $since)]' <<<"$notices") || keep "the hold notices do not parse" diff --git a/plugins/source-control/skills/babysit-prs/scripts/babysit_merge_policy.py b/plugins/source-control/skills/babysit-prs/scripts/babysit_merge_policy.py index a82111e770..97fc411459 100755 --- a/plugins/source-control/skills/babysit-prs/scripts/babysit_merge_policy.py +++ b/plugins/source-control/skills/babysit-prs/scripts/babysit_merge_policy.py @@ -2310,6 +2310,55 @@ def decided_head(notices: Collection[tuple[str, Any]], at: Any) -> str | None: return heads.pop() if len(heads) == 1 else None +def unran_head(activity: Any, ran: Any, decided: str, at: Any) -> str | None: + """Why the head branch's push history shows a lane run missing before a + decision made at `at`, else None (merge-holds.mjs `unranHead`, and + ci-status's cost-hold step). `activity` is one page of the repository + activity API for the branch, newest first, which deleting a run leaves; + `ran` lists, per REVIEW_LANES entry, the heads of its runs started before + `at`. Every head the branch moved to after its last push of `decided` + before `at` needs a run of every lane: with one deleted, an older notice + would become the decided head (#10437). No push of `decided` before `at` + or a full page, which may not reach it, keeps the hold too.""" + if not is_json_array(activity): + return "the branch's push history is unreadable" + if len(activity) >= 100: + return "the branch's push history fills a page" + if ( + not is_json_array(ran) + or len(ran) != len(REVIEW_LANES) + or not all(is_json_array(heads) for heads in ran) + ): + return "the lane runs do not cover every review lane" + moves: list[tuple[str, datetime]] = [] + for move in activity: + after = str(json_object(move).get("after") or "") + if re.fullmatch(r"[0-9a-f]{40}", after) and after != "0" * 40: + made = parse_timestamp(json_object(move).get("timestamp")) + if made is None: + return "the branch's push history is unreadable" + moves.append((after, made)) + cutoff = parse_timestamp(at) + before = [ + (after, made) for after, made in moves if cutoff is not None and made < cutoff + ] + pushed = [made for after, made in before if after == decided] + if not pushed: + return f"the branch shows no push of {decided} before the decision" + last = max(pushed) + for after, made in before: + if ( + after != decided + and made >= last + and not all(after in heads for heads in ran) + ): + return ( + f"{after} was pushed after the decided head with no run of every" + " review lane before the decision" + ) + return None + + def _noticed_head(owner: str, name: str, branch: Any, base: str, at: Any) -> str | None: """The head a decision made at `at` decided, from the review lanes' check-run annotations, never a comment (any GITHUB_TOKEN workflow posts as @@ -2317,8 +2366,10 @@ def _noticed_head(owner: str, name: str, branch: Any, base: str, at: Any) -> str the head `branch` of `owner`/`name` started before `at` (a run from another repository's same-named branch speaks for another pull request), each run's head vetted against `base` (`lane_change`), its status job's cost notices (`cost_notices`) - from jobs done before `at`, then `decided_head`. A run on a head that fails - the vet decides nothing, and a read that fails raises.""" + from jobs done before `at`, then `decided_head`, kept only while the + branch's push history shows no lane run missing since the decided head + (`unran_head`). A run on a head that fails the vet decides nothing, and a + read that fails raises.""" cutoff = parse_timestamp(at) if cutoff is None or not isinstance(branch, str) or not BASE_BRANCH.fullmatch(branch): return None @@ -2326,7 +2377,10 @@ def _noticed_head(owner: str, name: str, branch: Any, base: str, at: Any) -> str env = {"GH_TOKEN": token} if token else None notices: list[tuple[str, Any]] = [] vetted: set[str] = set() + ran: list[list[str]] = [] for file, check in REVIEW_LANES: + heads: list[str] = [] + ran.append(heads) runs = _paginated( f"repos/{owner}/{name}/actions/workflows/{file}/runs" f"?event=pull_request&branch={quote(branch, safe='')}&per_page=100", @@ -2342,6 +2396,7 @@ def _noticed_head(owner: str, name: str, branch: Any, base: str, at: Any) -> str head = str(run.get("head_sha") or "") if not re.fullmatch(r"[0-9a-f]{40}", head): return None + heads.append(head) if head not in vetted: status, compare = _get(f"repos/{owner}/{name}/compare/{base}...{head}", env) if status != 200: @@ -2368,7 +2423,16 @@ def _noticed_head(owner: str, name: str, branch: Any, base: str, at: Any) -> str head, job.get("completed_at"), ) - return decided_head(notices, at) + decided = decided_head(notices, at) + if decided is None: + return None + ref = quote(f"refs/heads/{branch}", safe="") + status, activity = _get( + f"repos/{owner}/{name}/activity?ref={ref}&direction=desc&per_page=100", env + ) + if status != 200: + raise PolicyReadError("the head branch's push history is unreadable") + return decided if unran_head(activity, ran, decided, at) is None else None def same_net_change(decided: Any, current: Any) -> bool: diff --git a/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_merge_cost.py b/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_merge_cost.py index 0a2f558723..47fd40ae65 100644 --- a/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_merge_cost.py +++ b/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_merge_cost.py @@ -9,6 +9,7 @@ import pathlib import sys import unittest +from datetime import datetime, timedelta from typing import Any from unittest import mock @@ -66,21 +67,72 @@ def _net(*files: dict[str, Any]) -> dict[str, Any]: return {"files": list(files)} +def _push(after: str, timestamp: str) -> dict[str, Any]: + """A push of `after` to the head branch, as the repository activity API + lists it.""" + return { + "after": after, + "timestamp": timestamp, + "activity_type": "push", + "ref": "refs/heads/fix/cost", + } + + +def _newest_first(moves: list[dict[str, Any]]) -> list[dict[str, Any]]: + return sorted(moves, key=lambda m: m["timestamp"], reverse=True) + + +def _pushes(runs: list[dict[str, Any]]) -> list[dict[str, Any]]: + """Each run's head pushed five minutes before its first status job + finished.""" + first: dict[str, str] = {} + for r in runs: + first[r["sha"]] = min(first.get(r["sha"], r["at"]), r["at"]) + return _newest_first( + [ + _push( + sha, + (datetime.fromisoformat(at) - timedelta(minutes=5)).strftime( + "%Y-%m-%dT%H:%M:%SZ" + ), + ) + for sha, at in first.items() + ] + ) + + class Lanes: - """The head reads of `_noticed_head` over `runs`, and the compares: a - run's head answers _net(TF) unless `compares` names it. A read whose label - (or `compare `) is `unreadable` fails. `reads` logs every endpoint and - `envs` the environment each lane read took.""" + """The head reads of `_noticed_head` over `runs`, the compares (a run's + head answers _net(TF) unless `compares` names it) and the head branch's + push history (`activity`, by default `_pushes(runs)`). A read whose label + (or `compare `, or `activity`) is `unreadable` fails. `reads` logs + every endpoint and `envs` the environment each lane read took.""" - def __init__(self, runs: list[dict[str, Any]], compares: dict[str, Any], unreadable: str = "") -> None: + def __init__( + self, + runs: list[dict[str, Any]], + compares: dict[str, Any], + unreadable: str = "", + activity: Any = None, + ) -> None: self.runs = runs self.compares = {r["sha"]: _net(TF) for r in runs} | compares self.unreadable = unreadable + self.activity = _pushes(runs) if activity is None else activity self.reads: list[str] = [] self.envs: list[Any] = [] def get(self, endpoint: str, env: Any = None) -> tuple[int | None, Any]: self.reads.append(endpoint) + if "/activity?" in endpoint: + assert endpoint == ( + "repos/owner/repo/activity?ref=refs%2Fheads%2Ffix%2Fcost" + "&direction=desc&per_page=100" + ), endpoint + self.envs.append(env) + return ( + (404, None) if self.unreadable == "activity" else (200, self.activity) + ) sha = endpoint.rsplit("...", 1)[1] if sha in self.compares and self.unreadable != f"compare {sha}": return 200, self.compares[sha] @@ -149,15 +201,16 @@ def decide( unreadable: str = "", pull: dict[str, Any] | None = None, head: str = HEAD, + activity: Any = None, ) -> str | None: created = PR_CREATED if created is None else created self.lanes = lanes = Lanes( - [_run(HEAD)] if runs is None else runs, compares or {}, unreadable + [_run(HEAD)] if runs is None else runs, compares or {}, unreadable, activity ) self.reads = lanes.reads def get(endpoint: str, env: Any = None) -> tuple[int | None, Any]: - if "/compare/" in endpoint: + if "/compare/" in endpoint or "/activity?" in endpoint: return lanes.get(endpoint, env) self.reads.append(endpoint) return status, comment if comment is not None else _comment() @@ -197,6 +250,8 @@ def test_an_owner_decision_on_the_closing_issue_clears(self) -> None: "repos/owner/repo/check-runs/1/annotations?per_page=100", "repos/owner/repo/actions/workflows/pr-review-security-hosted.yml/runs" "?event=pull_request&branch=fix%2Fcost&per_page=100", + "repos/owner/repo/activity?ref=refs%2Fheads%2Ffix%2Fcost" + "&direction=desc&per_page=100", ], ) @@ -432,13 +487,124 @@ def test_a_lane_run_on_a_head_it_cannot_vet_decides_nothing(self) -> None: runs = [_run(OLD, "2026-10-22T10:40:00Z"), _run(HEAD)] self.assertIsNone(self.decide(runs=runs, compares={OLD: answer})) + # #10437: a deleted lane run of a head pushed after the decided head, + # before the decision, would make an older notice the decided head. The + # branch's push history, which deleting a run leaves, shows that head, and + # it must have a run of every review lane started before the decision. + def test_a_head_pushed_since_the_decided_head_with_no_run_of_every_lane_keeps_the_hold( + self, + ) -> None: + new = "1" * 40 + same = {OLD: _net(TF), HEAD: _net(TF)} + + def plain(sha: str, lane: str) -> dict[str, Any]: + return _run( + sha, + "2026-10-22T11:20:00Z", + criteria="trust-gate", + lane=lane, + created="2026-10-22T11:12:00Z", + ) + + def history(*moves: dict[str, Any]) -> list[dict[str, Any]]: + return _newest_first([_push(OLD, "2026-10-22T10:25:00Z"), *moves]) + + def carried( + runs: list[dict[str, Any]], activity: Any, **change: Any + ) -> str | None: + return self.decide( + runs=[_run(OLD), *runs], compares=same, activity=activity, **change + ) + + both = [ + plain(new, "pr-review-hosted.yml"), + plain(new, "pr-review-security-hosted.yml"), + ] + pushed = history(_push(new, "2026-10-22T11:10:00Z")) + # No deletion: both lanes ran on the newer head, which noticed no cost. + self.assertEqual(carried(both, pushed), LINK) + # A head pushed after the decision needs no run here. + self.assertEqual(carried([], history(_push(new, "2026-10-22T12:10:00Z"))), LINK) + for name, runs in { + "both lanes' runs deleted": [], + "the code lane's run deleted": [ + plain(new, "pr-review-security-hosted.yml") + ], + "the security lane's run deleted": [plain(new, "pr-review-hosted.yml")], + "a run started only after the decision": [ + r | {"created": "2026-10-22T12:05:00Z"} for r in both + ], + "a run whose head is in another repository": [ + r | {"repo": "fork/repo"} for r in both + ], + }.items(): + with self.subTest(name): + self.assertIsNone(carried(runs, pushed)) + self.assertTrue(any("/activity?" in r for r in self.reads)) + # Pushing the decided head again after the decision makes it the head: + # the gap before the decision still keeps the hold. + back = history( + _push(new, "2026-10-22T11:10:00Z"), _push(OLD, "2026-10-22T12:30:00Z") + ) + self.assertIsNone(carried([], back, head=OLD)) + # No push of the decided head before the decision, a full page that may + # not reach it, or a read that fails keeps the hold. + self.assertIsNone(carried(both, [_push(new, "2026-10-22T11:10:00Z")])) + self.assertIsNone(carried(both, [_push(OLD, "2026-10-22T12:30:00Z")])) + full = _newest_first( + [_push(new, f"2026-10-22T12:10:{i:02d}Z") for i in range(59)] + + [_push(new, f"2026-10-22T12:11:{i:02d}Z") for i in range(40)] + + [_push(OLD, "2026-10-22T10:25:00Z")] + ) + self.assertIsNone(carried(both, full)) + self.assertEqual(carried(both, full[1:]), LINK) + self.assertIsNone(carried(both, pushed, unreadable="activity")) + self.assertIsNone(carried(both, {"message": "Not Found"})) + + def test_unran_head_names_the_first_head_pushed_since_with_no_run_of_every_lane( + self, + ) -> None: + new = "1" * 40 + at = "2026-10-22T12:00:00Z" + gone = _push("0" * 40, "2026-10-22T11:05:00Z") | { + "activity_type": "branch_deletion" + } + moves = [ + _push(new, "2026-10-22T11:10:00Z"), + gone, + _push(OLD, "2026-10-22T10:25:00Z"), + ] + self.assertIsNone(policy.unran_head(moves, [[OLD, new], [new]], OLD, at)) + self.assertIn(new, policy.unran_head(moves, [[OLD], [new]], OLD, at) or "") + self.assertIn( + "every review lane", policy.unran_head(moves, [[OLD, new]], OLD, at) or "" + ) + self.assertIn( + "every review lane", policy.unran_head(moves, None, OLD, at) or "" + ) + self.assertIn( + "no push of", policy.unran_head(moves, [[new], [new]], HEAD, at) or "" + ) + self.assertIn( + "no push of", + policy.unran_head(moves, [[new], [new]], OLD, "not a time") or "", + ) + odd = [{"after": new, "timestamp": "soon"}, *moves] + self.assertIn( + "unreadable", policy.unran_head(odd, [[new], [new]], OLD, at) or "" + ) + self.assertIn("unreadable", policy.unran_head("nope", [[], []], OLD, at) or "") + # Only the moves after the decided head's last push before it count. + again = [_push(OLD, "2026-10-22T11:30:00Z"), *moves] + self.assertIsNone(policy.unran_head(again, [[OLD], [OLD]], OLD, at)) + def test_the_lane_reads_take_the_actions_token_when_set(self) -> None: with mock.patch.dict(policy.os.environ, {policy.ACTIONS_TOKEN_ENV: "job"}): self.assertEqual(self.decide(), LINK) - self.assertEqual(self.lanes.envs, [{"GH_TOKEN": "job"}] * 4) + self.assertEqual(self.lanes.envs, [{"GH_TOKEN": "job"}] * 5) with mock.patch.dict(policy.os.environ, {policy.ACTIONS_TOKEN_ENV: ""}): self.assertEqual(self.decide(), LINK) - self.assertEqual(self.lanes.envs, [None] * 4) + self.assertEqual(self.lanes.envs, [None] * 5) def test_the_decided_head_is_never_read_from_a_comment(self) -> None: self.decide() @@ -500,7 +666,7 @@ def pick(body: str, runs: list[dict[str, Any]]) -> str | None: lanes = Lanes(runs, changed) def get(endpoint: str, env: Any = None) -> tuple[int | None, Any]: - if "/compare/" in endpoint: + if "/compare/" in endpoint or "/activity?" in endpoint: return lanes.get(endpoint, env) return 200, rows[endpoint.rsplit("/", 1)[1]] diff --git a/plugins/source-control/skills/babysit-prs/scripts/tests/test_merge_holds_sync.py b/plugins/source-control/skills/babysit-prs/scripts/tests/test_merge_holds_sync.py index 0128b75761..6901c139ed 100644 --- a/plugins/source-control/skills/babysit-prs/scripts/tests/test_merge_holds_sync.py +++ b/plugins/source-control/skills/babysit-prs/scripts/tests/test_merge_holds_sync.py @@ -27,8 +27,10 @@ place of an admin, a linked-issue comment counting only when it names the pull request as `#` (`namesPull`, `names_pull`), and both carry it to the same heads: the decided head, read from the review lanes' check-run - notices (`noticedHead`, `_noticed_head`), or a later one with the same net - change against the base branch (`sameNetChange`). + notices (`noticedHead`, `_noticed_head`) while the head branch's push + history shows a run of every review lane on each head pushed since it + (`unranHead`, `unran_head`), or a later one with the same net change + against the base branch (`sameNetChange`). - The high-path lift: `pathLifters` over `fromRest` and `babysit_merge_policy.path_lifters` name the same trusted approvers of the head on the same REST review lists. @@ -46,6 +48,7 @@ import subprocess import sys import unittest +from datetime import datetime, timedelta from typing import Any from unittest import mock @@ -589,14 +592,38 @@ def _run( } +def _push(after: str, timestamp: str) -> dict[str, Any]: + return {"after": after, "timestamp": timestamp, "activity_type": "push"} + + # The head reads: the review lanes' runs on the head branch (a cost notice at -# the head before the decision by default), their jobs and annotations, and a -# compare per head; a run's head answers {"files": [TF]} unless `compares` -# names it. +# the head before the decision by default), their jobs and annotations, a +# compare per head (a run's head answers {"files": [TF]} unless `compares` +# names it), and the head branch's push history, newest first (`activity`, +# by default each run's head pushed five minutes before its first status job +# finished). DEFAULT_HEADS = {"runs": [_run(HEAD)], "compares": {}} TF = {"filename": "a.tf", "status": "modified", "patch": "@@ -1 +1 @@\n-a\n+b"} MORE = {"filename": "b.sh", "status": "added", "patch": "+c"} LATE = "2026-01-02T18:00:00Z" +NEW = "1" * 40 +# The decided head OLD, then NEW pushed before the decision; HEAD carries +# OLD's net change. +SINCE = [_push(NEW, "2026-01-02T13:00:00Z"), _push(OLD, "2026-01-02T10:00:00Z")] +SAME = {OLD: {"files": [TF]}, HEAD: {"files": [TF]}} + + +def _plain(lane: str) -> dict[str, Any]: + """A run on NEW before the decision whose status job noticed no cost.""" + return _run( + NEW, + "2026-01-02T13:10:00Z", + criteria="trust-gate", + lane=lane, + created="2026-01-02T13:05:00Z", + ) + + # Each case: the heads world for a decision on the pull request (_link(7)). CARRIES = { "a notice at the head": DEFAULT_HEADS, @@ -667,6 +694,34 @@ def _run( "runs": [_run(OLD)], "compares": {OLD: {"files": [TF, TF]}, HEAD: {"files": [TF, TF]}}, }, + "a newer head pushed before it with a run of every lane": { + "runs": [ + _run(OLD), + _plain("pr-review-hosted.yml"), + _plain("pr-review-security-hosted.yml"), + ], + "compares": SAME, + "activity": SINCE, + }, + "a newer head pushed before it whose lane runs were deleted": { + "runs": [_run(OLD)], + "compares": SAME, + "activity": SINCE, + }, + "a newer head pushed before it with one lane's run deleted": { + "runs": [_run(OLD), _plain("pr-review-security-hosted.yml")], + "compares": SAME, + "activity": SINCE, + }, + "a history with no push of the decided head": { + "runs": [ + _run(OLD), + _plain("pr-review-hosted.yml"), + _plain("pr-review-security-hosted.yml"), + ], + "compares": SAME, + "activity": SINCE[:1], + }, "a patchless pure rename": { "runs": [_run(OLD)], "compares": { @@ -685,10 +740,29 @@ def _answers(heads: dict[str, Any]) -> dict[str, Any]: return {r["sha"]: {"files": [TF]} for r in heads["runs"]} | heads["compares"] +def _activity(heads: dict[str, Any]) -> list[dict[str, Any]]: + if "activity" in heads: + return heads["activity"] + first: dict[str, str] = {} + for r in heads["runs"]: + first[r["sha"]] = min(first.get(r["sha"], r["at"]), r["at"]) + pushes = [ + _push( + sha, + (datetime.fromisoformat(at) - timedelta(minutes=5)).strftime( + "%Y-%m-%dT%H:%M:%SZ" + ), + ) + for sha, at in first.items() + ] + return sorted(pushes, key=lambda m: m["timestamp"], reverse=True) + + NODE_DECISION = """ import { costDecision } from %s; -const [body, comment, issue, heads, answers] = JSON.parse(process.argv[1]); +const [body, comment, issue, heads, answers, activity] = JSON.parse(process.argv[1]); const github = async (method, path, data) => { + if (method === "GET" && path === "/repos/o/r/activity?ref=refs%%2Fheads%%2Fb&direction=desc&per_page=100") return activity; if (method === "GET" && path.includes("/compare/")) { const sha = path.split("...")[1]; if (!(sha in answers)) throw new Error("no such commit"); @@ -733,6 +807,7 @@ def js_decision( ISSUE, DEFAULT_HEADS | heads, _answers(DEFAULT_HEADS | heads), + _activity(DEFAULT_HEADS | heads), ] ), ], @@ -756,6 +831,11 @@ def get(endpoint: str, env: Any = None) -> tuple[int | None, Any]: if "/compare/" in endpoint: sha = endpoint.rsplit("...", 1)[1] return (200, answers[sha]) if sha in answers else (404, None) + if "/activity?" in endpoint: + assert endpoint == ( + "repos/o/r/activity?ref=refs%2Fheads%2Fb&direction=desc&per_page=100" + ), endpoint + return 200, _activity(heads) return 200, comment def paginated( @@ -826,6 +906,7 @@ def test_both_carry_a_decision_to_the_same_heads(self) -> None: "a notice at the head", "a base merge since the decided head", "a notice from the security lane", + "a newer head pushed before it with a run of every lane", } for case in carried: self.assertEqual(answers[case], _link(7), case) diff --git a/scripts/ci-hold-cost-decision.test.sh b/scripts/ci-hold-cost-decision.test.sh index d33eea5d32..e06d687ee3 100755 --- a/scripts/ci-hold-cost-decision.test.sh +++ b/scripts/ci-hold-cost-decision.test.sh @@ -13,7 +13,9 @@ # decision, the newest on the pull request else the newest on the issue, # covers the head it decided (the sha of the newest cost hold notice a review # lane's status job recorded before it, naming its own run's head; with none, -# no head) and a later head only when the net change against the base branch +# no head, and none while the head branch's push history shows a head pushed +# since the decided head, before the decision, with no run of every lane) and a +# later head only when the net change against the base branch # is the same at both (owner decisions Q17 c and Q18 d on #9436). The holds # and the decided head are read from the lanes' check-run notices, never from # a hold comment. @@ -86,7 +88,8 @@ fi # STUB_TRACKER (absent when unset) and STUB_ACTORS; a `tracker` failure leaves # BASE_CONFIG empty, as a failed checkout does, and an `actors` failure leaves # the trusted-actor list out of it. Runs, jobs and annotations come from JSON maps -# keyed by workflow file, run id and check-run id. +# keyed by workflow file, run id and check-run id; the head branch's push +# history comes from STUB_ACTIVITY, listed newest first. mkdir -p "$TMP_ROOT/bin" cat >"$TMP_ROOT/bin/gh" <<'EOF' #!/usr/bin/env bash @@ -103,6 +106,11 @@ case "$*" in */issues/6419/comments*) failing comments; printf '%s\n' "$STUB_COMMENTS" ;; */issues/comments/*) failing decision; from "$STUB_DECISIONS" "$(key comments "$@")" ;; */issues/[0-9]*) failing issue; from "$STUB_ISSUES" "$(key issues "$@")" ;; +*/activity\?ref=*) + failing activity + [[ "$*" == *'/activity?ref=refs%2Fheads%2Ffix%2Fcost&direction=desc&per_page=100' ]] || exit 1 + jq -c 'sort_by(.timestamp) | reverse' <<<"$STUB_ACTIVITY" + ;; */actions/workflows/*/runs*) failing runs [[ "$*" == *'branch=fix%2Fcost&'* ]] || exit 1 @@ -153,6 +161,9 @@ TF='{"filename":"infra/main.tf","status":"modified","patch":"@@ -1 +1 @@\n-a\n+b MORE='{"filename":"scripts/run.sh","status":"modified","patch":"+y"}' # nets : the compare answers at OLD and HEAD. nets() { STUB_COMPARE="$(jq -cn --arg h "$HEAD" --arg o "$OLD" --argjson a "$1" --argjson b "$2" '{($o): {files: $a}, ($h): {files: $b}}')"; export STUB_COMPARE; } +# push : a push to the head branch, as the repository +# activity API lists it. +push() { jq -cn --arg a "$1" --arg t "$2" '{after: $a, timestamp: $t, activity_type: "push", ref: "refs/heads/fix/cost"}'; } # job job() { jq -cn --argjson id "$1" --arg name "$2" --arg at "$3" '{id: $id, name: $name, completed_at: $at}'; } # lanes : one run per lane, its status job done @@ -169,7 +180,8 @@ lanes() { } # decided [completed_at] [criteria] [workflow] [job] [notice-sha]: a run # on , started at 13:00, whose job (the code lane's status job unless -# named) recorded a hold notice naming [notice-sha] (default ). +# named) recorded a hold notice naming [notice-sha] (default ), and a +# push of five minutes before that job finished. decided() { local id=$((DECIDED += 1)) at="${2:-2026-10-23T13:10:00Z}" STUB_RUNS="$(jq -c --arg f "${4:-pr-review-hosted.yml}" --argjson id "$id" --arg sha "$1" --arg at "$at" \ @@ -177,16 +189,38 @@ decided() { STUB_JOBS="$(jq -c --arg id "$id" --argjson j "$(job "$id" "${5:-$CODE_CHECK}" "$at")" '. + {($id): {jobs: [$j]}}' <<<"$STUB_JOBS")" STUB_NOTES="$(jq -c --arg id "$id" --argjson n "[$(note "claude-lane-hold lane=claude-review sha=${6:-$1} criteria=${3:-cost} label=needs-human")]" \ '. + {($id): $n}' <<<"$STUB_NOTES")" - export STUB_RUNS STUB_JOBS STUB_NOTES + STUB_ACTIVITY="$(jq -c --arg sha "$1" --arg at "$at" '. + [{after: $sha, timestamp: ($at | fromdateiso8601 - 300 | todateiso8601), activity_type: "push"}]' <<<"$STUB_ACTIVITY")" + export STUB_RUNS STUB_JOBS STUB_NOTES STUB_ACTIVITY } -# nodecided: drops every run decided added. +# nodecided: drops every run and push decided added, leaving the push of +# HEAD that lanes' runs ran on. nodecided() { DECIDED=100 STUB_RUNS="$(jq -c 'map_values(.workflow_runs |= map(select(.id <= 100)))' <<<"$STUB_RUNS")" - export STUB_RUNS + STUB_ACTIVITY="[$(push "$HEAD" 2026-10-23T13:18:30Z)]" + export STUB_RUNS STUB_ACTIVITY +} +# pushed [at]: a push of at [at] (13:08, between the decided +# run's notice and the decision). +pushed() { + STUB_ACTIVITY="$(jq -c --argjson p "$(push "$1" "${2:-2026-10-23T13:08:00Z}")" '. + [$p]' <<<"$STUB_ACTIVITY")" + export STUB_ACTIVITY +} +# ran : a run on started at 13:09, before the +# decision, whose status job noticed nothing; its head changes no file unless +# a compare already names it. +ran() { + local id=$((DECIDED += 1)) + STUB_RUNS="$(jq -c --arg f "$2" --argjson id "$id" --arg sha "$1" \ + '.[$f].workflow_runs += [{id: $id, head_sha: $sha, created_at: "2026-10-23T13:09:00Z", updated_at: "2026-10-23T13:11:00Z", head_repository: {full_name: "o/r"}}]' <<<"$STUB_RUNS")" + STUB_JOBS="$(jq -c --arg id "$id" --argjson j "$(job "$id" "$3" 2026-10-23T13:11:00Z)" '. + {($id): {jobs: [$j]}}' <<<"$STUB_JOBS")" + STUB_NOTES="$(jq -c --arg id "$id" '. + {($id): []}' <<<"$STUB_NOTES")" + STUB_COMPARE="$(jq -c --arg sha "$1" '. + {($sha): (.[$sha] // {files: []})}' <<<"$STUB_COMPARE")" + export STUB_RUNS STUB_JOBS STUB_NOTES STUB_COMPARE } reset() { + export STUB_ACTIVITY='[]' export STUB_FAIL='' STUB_LABELS='needs-human' LANES_APP_ID="$APP_ID" export STUB_ACTORS="{\"actors\":[{\"kind\":\"human\",\"id\":$OWNER_ID},{\"kind\":\"bot\",\"id\":5},{\"kind\":\"bot\",\"id\":$APP_ID,\"login\":\"melodic-automation-lanes[bot]\"}]}" export STUB_PR="{\"number\":6419,\"created_at\":\"2026-10-23T05:00:00Z\",\"head\":{\"ref\":\"fix/cost\",\"sha\":\"0123456789abcdef0123456789abcdef01234567\"},\"base\":{\"ref\":\"main\"},\"body\":\"Closes #9306\\n\\n- Cost decision: [Q50]($LINK)\"}" @@ -401,6 +435,45 @@ expect "a file with no patch carries nothing" no no no "covering this head" nets "[$TF]" "[$TF]" export STUB_FAIL=compare expect "a failed compare read carries nothing" no no no "are unreadable" +# #10437: a deleted lane run of a head pushed after the decided head, before +# the decision, would make an older notice the decided head. The head +# branch's push history, which deleting a run leaves, shows that head, and it +# needs a run of every review lane started before the decision. +NEW=1111111111111111111111111111111111111111 +reset +nodecided +decided "$OLD" +nets "[$TF]" "[$TF]" +pushed "$NEW" +expect "a head pushed after the decided head whose lane runs were deleted keeps the hold" no no no "$NEW was pushed after the decided head" +if grep -q 'activity?ref=refs%2Fheads%2Ffix%2Fcost&direction=desc&per_page=100$' "$TMP_ROOT/log"; then + ok "the push history is read as one page, as merge-holds.mjs reads it" +else + fail "the push history read: '$(grep activity "$TMP_ROOT/log")'" +fi +ran "$NEW" pr-review-hosted.yml "$CODE_CHECK" +expect "a head pushed since with the security lane's run deleted keeps the hold" no no no "$NEW was pushed after the decided head" +ran "$NEW" pr-review-security-hosted.yml "$SEC_CHECK" +expect "a head pushed since with a run of every lane and no cost notice carries the decision" yes yes yes +STUB_FAIL=activity expect "an unreadable push history carries nothing" no no no "push history is unreadable" +nodecided +decided "$OLD" +nets "[$TF]" "[$TF]" +pushed "$NEW" 2026-10-23T13:16:00Z +expect "a head pushed after the decision needs no lane run before it" yes yes yes +nodecided +decided "$OLD" +STUB_ACTIVITY="[$(push "$NEW" 2026-10-23T13:08:00Z)]" +export STUB_ACTIVITY +expect "a push history with no push of the decided head before the decision carries nothing" no no no "no push of $OLD" +# The decided head pushed back after the decision is the current head; the +# deleted run before the decision still keeps the hold. +reset +nodecided +decided "$HEAD" +pushed "$NEW" +expect "the decided head pushed back over a head whose lane runs were deleted keeps the hold" no no no "$NEW was pushed after the decided head" +reset export STUB_FAIL='' nodecided decided "$HEAD" 2026-10-23T13:00:00Z @@ -415,10 +488,14 @@ expect "a later notice before the decision whose criteria include cost decides i nodecided decided "$OLD" decided "$HEAD" 2026-10-23T13:16:00Z +# Every lane ran on HEAD, pushed before the decision, so the push history +# keeps nothing here. +ran "$HEAD" pr-review-security-hosted.yml "$SEC_CHECK" expect "a notice after the decision decides nothing" no no no "covering this head" nodecided decided "$OLD" decided "$HEAD" 2026-10-23T13:12:00Z trust-gate +ran "$HEAD" pr-review-security-hosted.yml "$SEC_CHECK" expect "a later notice without cost decides nothing, so the older cost notice does" no no no "covering this head" nodecided decided "$OLD" 2026-10-23T13:12:00Z cost pr-review-hosted.yml "$CODE_CHECK" "$HEAD" @@ -432,6 +509,8 @@ for head_repo in '{"full_name":"f/r"}' null; do nets "[$TF]" "[$TF,$MORE]" STUB_RUNS="$(jq -c --argjson r "$head_repo" '.["pr-review-hosted.yml"].workflow_runs |= map(if .id == 102 then .head_repository = $r else . end)' <<<"$STUB_RUNS")" export STUB_RUNS + ran "$HEAD" pr-review-hosted.yml "$CODE_CHECK" + ran "$HEAD" pr-review-security-hosted.yml "$SEC_CHECK" expect "a notice from a run whose head repository is $head_repo decides nothing" no no no "covering this head" done nodecided @@ -475,7 +554,7 @@ nodecided decided "$OLD" nets "[$TF]" "[$TF]" export STUB_FAIL=module -expect "an unreadable base merge-holds.mjs carries no closing-issue decision" no no no "covering this head" +expect "an unreadable base merge-holds.mjs carries no closing-issue decision" no no no "push history could not be checked" reset expect "at the decided head the decision carries" yes yes yes if grep -q 'compare/main\.\.\.' "$TMP_ROOT/log"; then fail "a decision at its own head read a compare"; else ok "a decision at its own head reads no compare"; fi