From d7dfbe0abfedf90b0ee0433082b41e8609f3444c Mon Sep 17 00:00:00 2001 From: Kyle Sexton <153232337+kyle-sexton@users.noreply.github.com> Date: Wed, 28 Oct 2026 21:27:43 -0400 Subject: [PATCH 1/2] perf(pr-merge): carry completed run attempts across post-merge-dispatch ticks Every post-merge check re-read the earlier attempts of each re-run full run in its window (8 attempt reads per tick on the 2026-10-28 runs). A completed attempt never changes, so the check now answers one from a file pr-merge.yml's post-merge-dispatch carries in its kept report, for the same run, attempt and head SHA, and writes back the completed attempts it used. Any unusable file is no file, and every attempt is read as before. Co-Authored-By: Claude Opus 5.5 --- .github/scripts/pr-merge/merge-job.test.mjs | 20 +- .github/workflows/pr-merge.yml | 6 + .../babysit-prs/scripts/babysit_postmerge.py | 128 +++++++++- .../tests/test_babysit_postmerge_attempts.py | 222 ++++++++++++++++++ 4 files changed, 372 insertions(+), 4 deletions(-) create mode 100644 plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_postmerge_attempts.py diff --git a/.github/scripts/pr-merge/merge-job.test.mjs b/.github/scripts/pr-merge/merge-job.test.mjs index a5a46fb765..1559f528bf 100644 --- a/.github/scripts/pr-merge/merge-job.test.mjs +++ b/.github/scripts/pr-merge/merge-job.test.mjs @@ -542,7 +542,7 @@ test("the early check fails red on a rate-limit stop, after its outputs and repo // check. const LAST_SHA = "d".repeat(40); const lastReport = () => ({ readable: true, ok: true, needsDispatch: false, shared: { mainSha: "a".repeat(40) } }); -const runIdleCheck = ({ last, code = "0", settle = 0, exit = 3 } = {}) => { +const runIdleCheck = ({ last, code = "0", settle = 0, exit = 3, attempts } = {}) => { const dir = tempDir("pr-merge-idle-"); try { const bin = join(dir, "bin"); @@ -560,6 +560,7 @@ const runIdleCheck = ({ last, code = "0", settle = 0, exit = 3 } = {}) => { writeFileSync(join(dir, "post-merge-last/post-merge-code"), `${code}\n`); writeFileSync(join(dir, "post-merge-last/post-merge-code-sha"), `${LAST_SHA}\n`); writeFileSync(join(dir, "post-merge-last/lane-records.json"), "the last run's"); + if (attempts !== undefined) writeFileSync(join(dir, "post-merge-last/attempts.json"), attempts); } const output = join(dir, "output"); const result = spawnSync("bash", ["-c", runScript("Check earlier lane merges", stepsOf(jobBlock("post-merge-dispatch")))], { @@ -578,6 +579,8 @@ const runIdleCheck = ({ last, code = "0", settle = 0, exit = 3 } = {}) => { keptCode: kept("post-merge-code"), keptSha: kept("post-merge-code-sha"), records: kept("lane-records.json"), + attempts: kept("attempts.json"), + handed: existsSync(join(dir, "post-merge-attempts.json")) ? readFileSync(join(dir, "post-merge-attempts.json"), "utf8") : null, }; } finally { rmSync(dir, { recursive: true, force: true }); @@ -619,6 +622,21 @@ test("a tick runs the check itself for no report, a refusal, a crash or a check } }); +// The attempt carry (babysit_postmerge.py ATTEMPTS_ENV): the last run's +// attempts file is handed to the check at the path the step names, and kept +// for the next tick whether the check runs or an idle tick reuses the report. +test("post-merge-dispatch hands the last run's attempts to the check and keeps them", () => { + const file = "${{ runner.temp }}/post-merge-attempts.json"; + assert.ok(step("Check earlier lane merges", stepsOf(jobBlock("post-merge-dispatch"))).includes(`BABYSIT_POSTMERGE_ATTEMPTS: ${file}\n`)); + for (const options of [{}, { settle: 1 }]) { + const run = runIdleCheck({ last: lastReport(), attempts: "carried", ...options }); + assert.equal(run.ran, options.settle === 1); + assert.equal(run.handed, "carried"); + assert.equal(run.attempts, "carried"); + } + assert.equal(runIdleCheck({ last: lastReport(), settle: 1 }).attempts, null); +}); + test("post-merge-dispatch reads the last run's report before its check, from a trusted run alone", () => { const list = stepsOf(jobBlock("post-merge-dispatch")); const names = list.map((s) => s.name); diff --git a/.github/workflows/pr-merge.yml b/.github/workflows/pr-merge.yml index 97b4e4af39..5889da931c 100644 --- a/.github/workflows/pr-merge.yml +++ b/.github/workflows/pr-merge.yml @@ -209,12 +209,17 @@ jobs: env: GH_TOKEN: ${{ github.token }} BABYSIT_POSTMERGE_LANE_RECORDS: ${{ runner.temp }}/lane-records.json + BABYSIT_POSTMERGE_ATTEMPTS: ${{ runner.temp }}/post-merge-attempts.json REPO: ${{ github.repository }} run: | set -euo pipefail last=$RUNNER_TEMP/post-merge-last scripts=.base/plugins/source-control/skills/babysit-prs/scripts code='' code_sha='' + # The completed attempts the last check read, which the check + # answers with no read (babysit_postmerge.py ATTEMPTS_ENV); kept + # below whether or not the check runs. + [ ! -f "$last/attempts.json" ] || cp "$last/attempts.json" "$RUNNER_TEMP/post-merge-attempts.json" # An idle tick: the last run's report, from a check that passed # (exit 0 or 5), stands while the lane room's reuse rule keeps it # and nothing else the check reads has moved (settle --idle-tick). @@ -275,6 +280,7 @@ jobs: # A reused report keeps the commit whose code made it. [ -z "$code_sha" ] || echo "$code_sha" >"$kept/post-merge-code-sha" [ ! -f "$RUNNER_TEMP/lane-records.json" ] || cp "$RUNNER_TEMP/lane-records.json" "$kept/lane-records.json" + [ ! -f "$RUNNER_TEMP/post-merge-attempts.json" ] || cp "$RUNNER_TEMP/post-merge-attempts.json" "$kept/attempts.json" # A rate-limit stop fails the job red once the outputs are written. [ "$code" != 75 ] || exit 1 diff --git a/plugins/source-control/skills/babysit-prs/scripts/babysit_postmerge.py b/plugins/source-control/skills/babysit-prs/scripts/babysit_postmerge.py index e66f374211..b0a233a430 100755 --- a/plugins/source-control/skills/babysit-prs/scripts/babysit_postmerge.py +++ b/plugins/source-control/skills/babysit-prs/scripts/babysit_postmerge.py @@ -114,6 +114,15 @@ _LANE_RECORD_NAME = re.compile( r"pr-merge-(?:dispatched-[1-9][0-9]{0,15}|rerun-[1-9][0-9]{0,15}-attempt-2)" ) +# A completed attempt never changes, so pr-merge.yml's post-merge-dispatch +# names a file here holding the earlier attempts of re-run full runs that the +# last check read (`_Context._attempt`), carried in its kept report. An entry +# answers with no read only for the same run, attempt and head SHA, completed; +# the check writes back the completed attempts it used. Any other file is no +# file, and every attempt is read as before. +ATTEMPTS_ENV = "BABYSIT_POSTMERGE_ATTEMPTS" +ATTEMPTS_VERSION = 1 +_ATTEMPT_KEY = re.compile(r"[1-9][0-9]{0,15}:[1-9][0-9]{0,2}") # With `--lane-merges-only`, a red attempt 1 of a lane-dispatched full run # created within red-main.confirm-wait-minutes plus this, with a newly failing # check that demotes (`_Context.regressions`), gets that re-run even when no @@ -804,7 +813,8 @@ def _runs( return [r for r in runs if r.created_at >= since and r.head_branch == branch] -def _attempt(owner: str, name: str, run_id: int, attempt: int) -> Run | None: +def _attempt(owner: str, name: str, run_id: int, attempt: int) -> Any: + """The attempt's payload as the API returns it, or None on a 404.""" args = ["api", f"repos/{owner}/{name}/actions/runs/{run_id}/attempts/{attempt}"] try: proc = gh_capture(args) @@ -817,11 +827,62 @@ def _attempt(owner: str, name: str, run_id: int, attempt: int) -> Run | None: f"run {run_id} attempt {attempt}: {(proc.stderr or '').strip()}" ) try: - return Run.from_api(json.loads(proc.stdout)) + return json.loads(proc.stdout) except ValueError as exc: raise PostMergeReadError(f"run {run_id} attempt {attempt}: {exc}") from exc +def _attempt_fields(payload: Any) -> dict[str, Any]: + """The fields of an attempt payload `Run.from_api` reads, and no others.""" + item = json_object(payload) + return { + **{ + key: item.get(key) + for key in ( + "id", + "run_attempt", + "status", + "conclusion", + "head_sha", + "head_branch", + "created_at", + "html_url", + ) + }, + "actor": {"login": json_object(item.get("actor")).get("login")}, + "triggering_actor": { + "login": json_object(item.get("triggering_actor")).get("login") + }, + } + + +def load_attempts(path: str, repo: str) -> dict[str, dict[str, Any]]: + """The completed attempts in the file at `path`, by `:`. + Raises `ValueError` for any file of another shape, so none of it is used.""" + data = json_object(json.loads(Path(path).read_text(encoding="utf-8"))) + attempts = data.get("attempts") + if ( + data.get("version") != ATTEMPTS_VERSION + or data.get("repo") != repo + or not is_json_object(attempts) + ): + raise ValueError("an unexpected shape") + for key, entry in attempts.items(): + try: + run = Run.from_api(entry) + except PostMergeReadError as exc: + raise ValueError(f"{key!r}: {exc}") from exc + if not ( + _ATTEMPT_KEY.fullmatch(key) + and key == f"{run.id}:{run.attempt}" + and run.status == "completed" + and _FULL_SHA.fullmatch(run.head_sha) + and entry == _attempt_fields(entry) + ): + raise ValueError(f"{key!r}: an entry of another shape") + return attempts + + # --- Judging runs ------------------------------------------------------------------- _FULL_SHA = re.compile(r"[0-9a-f]{40}") @@ -1132,6 +1193,13 @@ class _Context: lane_proved: dict[str, dict[str, Any]] = field(default_factory=dict) # Carried records whose sender failed `_lane_sender`: never written back. lane_rejected: set[str] = field(default_factory=set) + # The attempts file (`ATTEMPTS_ENV`), its attempts once loaded, and the + # completed attempts this check used, which `save_attempts` writes. + attempts_path: str | None = field( + default_factory=lambda: os.environ.get(ATTEMPTS_ENV) or None + ) + attempts_carried: dict[str, dict[str, Any]] | None = None + attempts_used: dict[str, dict[str, Any]] = field(default_factory=dict) def dispatch_runs_through( self, commit: str, merged_at: datetime, deadline: datetime @@ -1940,7 +2008,60 @@ def outcome(self, run: Run, *, attributable: bool) -> tuple[str, str]: return self.outcome_cache[key] def _attempt(self, run: Run, n: int) -> Run | None: - return run if run.attempt == n else _attempt(self.owner, self.name, run.id, n) + """Attempt `n` of `run`: the listed run itself, a completed attempt + the last check carried for the same run and head SHA, or a read.""" + if run.attempt == n: + return run + key = f"{run.id}:{n}" + payload = self._carried_attempts().get(key) + if payload is None or Run.from_api(payload).head_sha != run.head_sha: + payload = _attempt(self.owner, self.name, run.id, n) + if payload is None: + return None + found = Run.from_api(payload) + if ( + found.status == "completed" + and f"{found.id}:{found.attempt}" == key + and found.head_sha == run.head_sha + ): + self.attempts_used[key] = _attempt_fields(payload) + return found + + def _carried_attempts(self) -> dict[str, dict[str, Any]]: + if self.attempts_carried is None: + self.attempts_carried = {} + path = self.attempts_path + if path and Path(path).exists(): + try: + self.attempts_carried = load_attempts( + path, f"{self.owner}/{self.name}" + ) + except Exception as exc: # noqa: BLE001 (any unusable file is no file) + text = re.sub(r"[\r\n%]", " ", f"{type(exc).__name__}: {exc}") + print(f"attempts not used: {text[:300]}", file=sys.stderr) + return self.attempts_carried + + def save_attempts(self) -> None: + """Write the attempts file: the completed attempts this check used. + Untouched when the check looked up no attempt. Best effort: a + failure is logged, and the file is left as it was.""" + path = self.attempts_path + if not path or self.attempts_carried is None: + return + payload = { + "version": ATTEMPTS_VERSION, + "repo": f"{self.owner}/{self.name}", + "attempts": self.attempts_used, + } + target = Path(path) + tmp = target.with_name(f"{target.name}.{os.getpid()}.tmp") + try: + tmp.write_text(json.dumps(payload), encoding="utf-8") + os.replace(tmp, target) + except OSError as exc: + print(f"attempts not written to {path}: {exc}", file=sys.stderr) + with contextlib.suppress(OSError): + tmp.unlink(missing_ok=True) def _outcome(self, run: Run, attributable: bool) -> tuple[str, str]: first = self._attempt(run, 1) @@ -2397,6 +2518,7 @@ def lane_prs_at(sha: str) -> str | None: finally: if ctx is not None: ctx.save_lane_records(merged_since) + ctx.save_attempts() return PostMergeVerdict( True, merges=states, diff --git a/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_postmerge_attempts.py b/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_postmerge_attempts.py new file mode 100644 index 0000000000..de1eb4bfcf --- /dev/null +++ b/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_postmerge_attempts.py @@ -0,0 +1,222 @@ +"""The attempt carry of the post-merge check (`babysit_postmerge`). + +A completed attempt of a workflow run never changes, so a check answers an +earlier attempt of a re-run full run from the file pr-merge.yml names in +`BABYSIT_POSTMERGE_ATTEMPTS` with no read, for the same run, attempt and head +SHA, and writes back the completed attempts it used. The reports are pins: +the check with no file (the code before the carry) is the source, and a +carried check must report the same. Request counts come from the +`babysit-gh-calls` counter (`babysit_gh._run_gh`), with gh stubbed at +`run_command` by the same fake as the check's tests; each run at attempt 2 +costs one read of its attempt 1 (`_Context._outcome`: attempt 1 decides). +""" + +from __future__ import annotations + +import contextlib +import io +import json +import pathlib +import sys +import tempfile +import unittest +from datetime import UTC, datetime +from typing import Any +from unittest import mock + +sys.path.insert(0, str(pathlib.Path(__file__).resolve().parent.parent)) +sys.path.insert(0, str(pathlib.Path(__file__).resolve().parent)) + +import babysit_gh as gh +import babysit_postmerge as pm +from test_babysit_postmerge import ( + IDENTITY, + OTHER_SHA, + REPO, + SINCE, + FakeGh, + by, + canary_run, + dispatch_run, + human, + schedule_run, +) + +NOW = datetime(2026, 10, 12, 11, 0, tzinfo=UTC) +ATTEMPT_READS = "default GET repos/{}/{}/actions/runs/{}/attempts/{}" + + +class AttemptCase(unittest.TestCase): + """Dispatch run 11 failed attempt 1 and passed its re-run (a flake), and + schedule run 31 failed both attempts (a failure): two attempt-1 reads.""" + + def setUp(self) -> None: + self.gh = FakeGh() + tmp = tempfile.TemporaryDirectory(prefix="attempts-") + self.addCleanup(tmp.cleanup) + self.path = pathlib.Path(tmp.name) / "attempts.json" + + def run_command(command: list[str], **kwargs: Any) -> Any: + # The fake answers without headers, so the `-i` the guard adds is dropped. + args = [a for i, a in enumerate(command[1:]) if (i, a) != (1, "-i")] + return self.gh(args, timeout_seconds=kwargs.get("timeout_seconds")) + + patcher = mock.patch.object(gh, "run_command", side_effect=run_command) + patcher.start() + self.addCleanup(patcher.stop) + self.addCleanup(gh.reset_gh_call_counts) + self.gh.add_runs("post-merge-detect-silent-revert.yml", "push", canary_run(21)) + self.gh.add_runs( + "pr-require-checks.yml", + "workflow_dispatch", + dispatch_run(11, run_attempt=2, conclusion="success"), + ) + self.gh.attempts[(11, 1)] = dispatch_run(11, conclusion="failure") + after = "2026-10-12T10:02:00Z" + self.gh.add_runs( + "pr-require-checks.yml", + "schedule", + schedule_run( + 31, run_attempt=2, triggering_actor=by(IDENTITY), created_at=after + ), + ) + self.gh.attempts[(31, 1)] = schedule_run(31, created_at=after) + + def check(self, path: pathlib.Path | str | None = None) -> pm.PostMergeVerdict: + """One check, with the attempts file at `path` (None: no file named, + the check as it was before the carry).""" + gh.reset_gh_call_counts() + env = {pm.ATTEMPTS_ENV: "" if path is None else str(path)} + with ( + mock.patch.dict("os.environ", env), + contextlib.redirect_stderr(io.StringIO()) as self.stderr, + ): + return pm.check( + REPO, + agent_identity=IDENTITY, + is_human=human, + merged_since=SINCE, + now=NOW, + pinned_sha=lambda merge: None, + ) + + def reads(self) -> int: + return gh.gh_call_counts().get(ATTEMPT_READS, 0) + + def written(self) -> dict[str, Any]: + return json.loads(self.path.read_text(encoding="utf-8")) + + def baseline(self) -> dict[str, Any]: + """The report of the check with no file: the pin's source.""" + report = self.check().report() + self.assertEqual(self.reads(), 2) + return report + + +class TheCarrySparesTheAttemptReads(AttemptCase): + def test_a_carried_check_reports_as_one_with_no_file_in_two_fewer_reads( + self, + ) -> None: + expected = self.baseline() + self.assertEqual( + [f.signal for f in self.check(self.path).failures], ["schedule"] + ) + self.assertEqual(self.reads(), 2) + self.assertEqual(set(self.written()["attempts"]), {"11:1", "31:1"}) + carried = self.check(self.path) + self.assertEqual(self.reads(), 0) + self.assertEqual(carried.report(), expected) + # The flake and the failure stand on the carried attempt 1. + self.assertEqual( + carried.flakes, (f"https://github.com/{REPO}/actions/runs/11",) + ) + self.assertEqual( + carried.failures[0].run_url, + f"https://github.com/{REPO}/actions/runs/31/attempts/2", + ) + + def test_a_carried_check_writes_the_same_attempts_back(self) -> None: + self.check(self.path) + first = self.written() + self.check(self.path) + self.assertEqual(self.written(), first) + + def test_with_no_file_named_every_attempt_is_read(self) -> None: + self.check(self.path) + self.check(None) + self.assertEqual(self.reads(), 2) + + +class AnUnusableFileIsNoFile(AttemptCase): + def carry(self) -> dict[str, Any]: + self.check(self.path) + return self.written() + + def assert_read_as_with_no_file(self, expected: dict[str, Any]) -> None: + verdict = self.check(self.path) + self.assertEqual(self.reads(), 2) + self.assertEqual(verdict.report(), expected) + + def test_garbage_is_read_live(self) -> None: + expected = self.baseline() + self.path.write_text("{not json", encoding="utf-8") + self.assert_read_as_with_no_file(expected) + self.assertIn("attempts not used", self.stderr.getvalue()) + + def test_another_repository_or_version_is_read_live(self) -> None: + expected = self.baseline() + for change in ({"repo": "other/repo"}, {"version": 2}): + data = {**self.carry(), **change} + self.path.write_text(json.dumps(data), encoding="utf-8") + self.assert_read_as_with_no_file(expected) + + def test_an_in_progress_or_misfiled_entry_drops_the_whole_file(self) -> None: + expected = self.baseline() + for key, change in ( + ("11:1", {"status": "in_progress", "conclusion": None}), + ("11:1", {"id": 12}), + ("31:1", {"run_attempt": 2}), + ("31:1", {"head_sha": "not-a-sha"}), + ("31:1", {"extra": True}), + ): + data = self.carry() + data["attempts"][key] = {**data["attempts"][key], **change} + self.path.write_text(json.dumps(data), encoding="utf-8") + self.assert_read_as_with_no_file(expected) + + def test_an_entry_for_another_head_sha_is_read_live(self) -> None: + expected = self.baseline() + data = self.carry() + for entry in data["attempts"].values(): + entry["head_sha"] = OTHER_SHA + self.path.write_text(json.dumps(data), encoding="utf-8") + self.assert_read_as_with_no_file(expected) + # The live reads replace the stale entries. + self.assertNotIn( + OTHER_SHA, {e["head_sha"] for e in self.written()["attempts"].values()} + ) + + +class OnlyCompletedAttemptsAreWritten(AttemptCase): + def test_an_in_progress_attempt_is_not_written(self) -> None: + self.gh.attempts[(11, 1)] = dispatch_run( + 11, status="in_progress", conclusion=None + ) + self.check(self.path) + self.assertEqual(set(self.written()["attempts"]), {"31:1"}) + + def test_a_check_that_looks_up_no_attempt_leaves_the_file_as_it_was( + self, + ) -> None: + self.check(self.path) + before = self.path.read_text(encoding="utf-8") + self.gh.runs.clear() + self.gh.add_runs("post-merge-detect-silent-revert.yml", "push", canary_run(21)) + self.gh.add_runs("pr-require-checks.yml", "workflow_dispatch", dispatch_run(11)) + self.check(self.path) + self.assertEqual(self.reads(), 0) + self.assertEqual(self.path.read_text(encoding="utf-8"), before) + + +if __name__ == "__main__": + unittest.main() From 1d2e9f5c0db8efc9685a5732af446e901fdb4553 Mon Sep 17 00:00:00 2001 From: Kyle Sexton <153232337+kyle-sexton@users.noreply.github.com> Date: Wed, 28 Oct 2026 21:29:47 -0400 Subject: [PATCH 2/2] docs(pr-merge): add changelog fragments for the attempt carry Co-Authored-By: Claude Opus 5.5 --- .../pr-pipeline/perf-postmerge-attempt-carry-2b9f6a17.md | 7 +++++++ .../perf-postmerge-attempt-carry-7c41d0e2.md | 7 +++++++ 2 files changed, 14 insertions(+) create mode 100644 .changes/pr-pipeline/perf-postmerge-attempt-carry-2b9f6a17.md create mode 100644 .changes/source-control/perf-postmerge-attempt-carry-7c41d0e2.md diff --git a/.changes/pr-pipeline/perf-postmerge-attempt-carry-2b9f6a17.md b/.changes/pr-pipeline/perf-postmerge-attempt-carry-2b9f6a17.md new file mode 100644 index 0000000000..87bf6ea8e5 --- /dev/null +++ b/.changes/pr-pipeline/perf-postmerge-attempt-carry-2b9f6a17.md @@ -0,0 +1,7 @@ +--- +bump: patch +--- + +### Changed + +- pr-merge's `post-merge-dispatch` carries the completed run attempts its post-merge check read from one tick to the next, as `attempts.json` in the `pr-merge-post-merge-report` artifact it already keeps. The check step copies the last trusted run's file (read by `last-report.mjs`) to the path it names in `BABYSIT_POSTMERGE_ATTEMPTS`, and keeps the file the check leaves, whether the check runs or an idle tick reuses the report. On the 2026-10-28 ticks this spares the 8 attempt reads each check made, of the job's 50 to 57 GITHUB_TOKEN calls. The `merge` job's check names no file and reads as before. Permissions are unchanged. diff --git a/.changes/source-control/perf-postmerge-attempt-carry-7c41d0e2.md b/.changes/source-control/perf-postmerge-attempt-carry-7c41d0e2.md new file mode 100644 index 0000000000..580c59f85f --- /dev/null +++ b/.changes/source-control/perf-postmerge-attempt-carry-7c41d0e2.md @@ -0,0 +1,7 @@ +--- +bump: patch +--- + +### Changed + +- `/source-control:babysit-prs`: the post-merge check (`babysit_postmerge.py`) answers an earlier attempt of a re-run full run from the file named in `BABYSIT_POSTMERGE_ATTEMPTS` before reading it, when the file holds that run's attempt, completed, at the same head SHA. A completed attempt never changes, so the answer decides as the read would. The check writes back the completed attempts it used; a check that looks up no attempt leaves the file as it was. A missing or malformed file (another version or repository, a key that does not match its entry, an attempt not completed, a field the reader does not use) is no file, and every attempt is read as before. Without the variable nothing changes.