From d3f122194596cf3fa60463d5fdcef509d0414d2e Mon Sep 17 00:00:00 2001 From: Zach Vorhies Date: Sat, 24 Oct 2026 20:13:06 -0700 Subject: [PATCH] coordinator: a red master hides that repo's fix-ci tasks Tick 5: #2313 and #2322 were listed as fix-ci while they failed only because obs-studio's master was red. fix-master already covers them, and each fix-ci took a coordinator's time to rediscover the same cause. The skill also says every kind takes a lease, triage included. --- .claude/skills/obs-org-coordinator/SKILL.md | 5 +++-- tests/test_coordinator.py | 20 ++++++++++++++++++-- workflow/coordinator.py | 2 ++ 3 files changed, 23 insertions(+), 4 deletions(-) diff --git a/.claude/skills/obs-org-coordinator/SKILL.md b/.claude/skills/obs-org-coordinator/SKILL.md index c59686c..81dcf12 100644 --- a/.claude/skills/obs-org-coordinator/SKILL.md +++ b/.claude/skills/obs-org-coordinator/SKILL.md @@ -44,7 +44,7 @@ coordinator holds, and orders them: | Kind | What it means | What you do | |---|---|---| -| `fix-master` | a default branch's last push run failed | find the PR that broke it; fix or revert first. Queued PRs fail until it is green. Needs an admin (token, secret, rule)? One `human-review-needed` issue with the log line | +| `fix-master` | a default branch's last push run failed (its PRs' `fix-ci` tasks are hidden until it is green) | find the PR that broke it; fix or revert first. Queued PRs fail until it is green. Needs an admin (token, secret, rule)? One `human-review-needed` issue with the log line | | `requeue` | a green PR now conflicts (ejected from the queue) | rebase, keep both sides of list conflicts, `--force-with-lease`, `gh pr merge --auto` | | `fix-ci` | a PR's checks fail | ยง3 | | `enqueue` | green and mergeable, not queued | `gh pr merge -R --auto`, then confirm (gh pr view has no queue field): `gh api graphql -f query='{repository(owner:"obs-rust",name:""){pullRequest(number:){isInMergeQueue}}}'` | @@ -75,7 +75,8 @@ uv run -m workflow.coordinator done --owner "$COORDINATOR ``` `take` creates a git ref atomically, so if another coordinator runs the same -tick, exactly one of you gets each task and the other skips it. A lease +tick, exactly one of you gets each task and the other skips it. Every kind +takes a lease, `triage` included. A lease lapses after 2 hours if you die mid-task. Actions safe to repeat (`gh pr merge --auto`, labels, lease-protected pushes) need no lease, but take one anyway: it keeps the lists in step. diff --git a/tests/test_coordinator.py b/tests/test_coordinator.py index fc3294a..0ab3cf4 100644 --- a/tests/test_coordinator.py +++ b/tests/test_coordinator.py @@ -337,6 +337,22 @@ def test_red_master_comes_first(self): self.w.master(R, red=False) self.assertNotIn("fix-master", self.kinds()) + def test_red_master_hides_fix_ci_in_that_repo(self): + # Tick 5: #2313 and #2322 failed only because master was red. + n = self.w.open_pr(R, "alice") + self.w.ci(R, n, passed=False) + other = self.w.open_pr("obs-rust/obs-deps", "bob") + self.w.ci("obs-rust/obs-deps", other, passed=False) + self.w.master(R, red=True) + + def at(repo, number): + return [t.kind for t in self.work() if (t.repo, t.number) == (repo, number)] + + self.assertEqual(at(R, n), []) + self.assertEqual(at("obs-rust/obs-deps", other), ["fix-ci"]) + self.w.master(R, red=False) + self.assertEqual(at(R, n), ["fix-ci"]) + def test_full_order(self): self.w.master(R, red=True) self.w.open_issue(R, labels={"human-review-needed"}) @@ -350,8 +366,8 @@ def test_full_order(self): self.w.gh.x("UPDATE items SET merge_state='BEHIND' WHERE id=?", self.w.gh.node(R, reb)) enq = self.w.open_pr(R, "gina") self.w.ci(R, enq, passed=True) - fix = self.w.open_pr(R, "hal") - self.w.ci(R, fix, passed=False) + fix = self.w.open_pr("obs-rust/obs-deps", "hal") # R's master is red + self.w.ci("obs-rust/obs-deps", fix, passed=False) req = self.green_pr(author="ivy") self.w.enqueue(R, req) self.w.eject(R, req) diff --git a/workflow/coordinator.py b/workflow/coordinator.py index 8aaafd9..e5a0e35 100644 --- a/workflow/coordinator.py +++ b/workflow/coordinator.py @@ -183,6 +183,8 @@ def tasks( if item.state != "OPEN": continue kind = pr_task(item, now) if item.kind == "pr" else issue_task(item, now) + if kind == "fix-ci" and item.repo in red: + continue # it fails because master does; fix-master covers it if kind: out.append(task(kind, item, now)) out = [t for t in out if lease.key(t.kind, t.repo, t.number) not in held]