Skip to content

Commit e7d7bcf

Browse files
mattmilleraiclaude
andauthored
feat(cursor-review): axes_alongside_panel — run cursor-approve's start phase and the gating axes beside the panel (#434)
* feat(cursor-review): axes_alongside_panel — run cursor-approve's start phase and the gating axes beside the panel New opt-in input (default false; every job it adds is `if: inputs.axes_alongside_panel && …`, so nothing changes with it off): - approve-start-early: cursor-approve.yml `start` right after round-cap, approve_gate `pending` (or `capped`), round = round-cap's real next_round. - axis-correctness / axis-conformance: the axis workflows beside the panel, needing a SUCCESSFUL start, secrets passed explicitly, not in panel-integrity's or blocking-gate's needs. - axes-closeout: card.py closeout rewrites a still-pending early card to no_decision/push_then_relabel when the round's approve_gate is not pass. - axes_started output; axes / axis_prior_rounds / axis_direct_model inputs. card.py: a `pending` start card carries no LANE_MARKER (approve_only_on_push must never read an undecided panel as passed), plus the `closeout` subcommand. auto-approve.py: withdraw treats `pending` as `pass`. cursor-approve.yml drops its unused `actions: read`: a nested job cannot exceed its caller's grant, and the documented cursor-review caller grants contents + pull-requests only, so keeping it would fail every caller at startup on the pin bump, input on or off. Nothing in that workflow calls the Actions API; same-run artifact upload/download needs no actions scope. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(cursor-review): harden axes_alongside_panel per the panel's review - axes_ok output: false when an axis job started beside the panel did not succeed. A panel cell can claim the run-global `axis-<name>` artifact first, so the genuine upload fails; the documented decide `if:` now skips on axes_ok false, and axes-closeout rewrites the card on that path. - approve-start-early requires defer_approval, runs only for a round within the diff-size cap with a passing preflight (or the capped hand-off), and is ordered after dismiss-stale-approval; post-review waits for it, so a late pending card cannot overwrite decide's round card. - cursor-approve's start validates `axes` under approve_gate `pending` (a non-empty subset of correctness,conformance) and exports it normalized; the axis jobs' `if:` reads that output. - axes-closeout runs under !cancelled() (not always()) on any start that ran, and card.py closeout only rewrites a card whose run link is this run's. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
1 parent 7fdd9be commit e7d7bcf

14 files changed

Lines changed: 1011 additions & 24 deletions

‎.github/cursor-approve/card.py‎

Lines changed: 121 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,16 @@
99
``start`` (before the axis jobs)
1010
The round heading, every enabled axis as pending, and a link to the run.
1111
When cursor-review reported ``approve_gate == capped`` it writes
12-
"Round limit reached, needs a human" and no table instead.
12+
"Round limit reached, needs a human" and no table instead. Under
13+
cursor-review's ``axes_alongside_panel`` it runs while the panel still
14+
does (``approve_gate == pending``): the same table, a line saying the
15+
panel is still running, and no ``LANE_MARKER``.
16+
17+
``closeout`` (cursor-review's ``axes-closeout``, after its panel decided)
18+
Rewrite that early card to "No decision this round" when the panel did
19+
not pass, or an axis job did not complete — only a ``pass`` card for the
20+
same commit, round and workflow run with an axis still pending, so a card
21+
any decide (or a newer run) already wrote is left alone.
1322
1423
``ensure`` (decide phase, before the review is posted)
1524
Print the card's URL, creating it first when the start phase never ran,
@@ -91,6 +100,7 @@
91100
VERDICT_ICONS = {"green": "🟢 green", "yellow": "🟡 yellow", "red": "🔴 red",
92101
"n/a": "⚪ n/a"}
93102
NO_RESULT = "⚠️ no result"
103+
PENDING_ROW = "⏳ pending"
94104
# A yellow's two meanings (aggregate.py's YELLOW_KINDS), told apart on the card
95105
# so a reader sees which kind gated a round. Both keep the `🟡 yellow` prefix
96106
# and contain no `red`/`green`/`n/a`/`pending`: a consumer that reduces a
@@ -443,18 +453,33 @@ def _reviewed_line(sha: str, run_url: str) -> str:
443453
return f"_Reviewed commit: {reviewed} · {_run_link(run_url)}_"
444454

445455

456+
PANEL_RUNNING_TEXT = "*The review panel is still running; the axes judge the same commit in parallel.*"
457+
458+
446459
def render_start(round_no, max_rounds, sha: str, axes: list, approve_gate: str, run_url: str) -> str:
460+
"""The start phase's card: every axis pending, or the capped hand-off.
461+
462+
`approve_gate == "pending"` is cursor-review's `axes_alongside_panel`
463+
start, written while its panel is still running: the same `pass`/`none`
464+
head and table, a line saying so, and NO `LANE_MARKER`. The lane marker
465+
means "the last panel round passed", which `auto-approve.py
466+
approve_only_blocker` trusts to let a push skip the panel; an early card
467+
carrying it would put a round whose panel has not passed into that lane.
468+
"""
447469
capped = approve_gate == "capped"
470+
early = approve_gate == "pending"
448471
lines = card_head(STATE_CAPPED if capped else STATE_PASS, NEXT_HUMAN if capped else NEXT_NONE)
449472
lines += [heading(round_no, max_rounds, sha), ""]
450473
if capped:
451474
lines.append("**Round limit reached, needs a human.**")
452475
lines += ["", f"**Next step:** {NEXT_HUMAN_CAPPED_TEXT}"]
453476
else:
454477
lines += ["| Axis | Verdict | Why |", "|---|---|---|"]
455-
lines += [f"| {axis} | ⏳ pending | |" for axis in axes]
478+
lines += [f"| {axis} | {PENDING_ROW} | |" for axis in axes]
479+
if early:
480+
lines += ["", PANEL_RUNNING_TEXT]
456481
lines += ["", _reviewed_line(sha, run_url)]
457-
if not capped:
482+
if not capped and not early:
458483
lines += ["", LANE_MARKER]
459484
return "\n".join(lines) + "\n"
460485

@@ -650,6 +675,7 @@ def render_pending(round_no, max_rounds, sha: str, run_url: str) -> str:
650675
REREVIEW_HEADS_KEPT = 20
651676
_HEADING_RE = re.compile(r"^### Cursor approve · round ([0-9?]+) of ([0-9?]+) · ", re.MULTILINE)
652677
_REVIEWED_RE = re.compile(r"^_Reviewed commit: `([0-9a-f]{40})`", re.MULTILINE)
678+
_REVIEWED_RUN_RE = re.compile(r"^_Reviewed commit: .*\[workflow run\]\(([^)\s]+)\)", re.MULTILINE)
653679
_STATE_RE = re.compile(r"\A" + re.escape(CARD_MARKER) + r"\r?\n<!-- cursor-approve-state: ([a-z_]+) -->")
654680

655681
# --- approver-only re-checks (cursor-review's `approve_only_on_push`) --------
@@ -662,7 +688,9 @@ def render_pending(round_no, max_rounds, sha: str, run_url: str) -> str:
662688
# history names no other editor (auto-approve.py `card_foreign_editors`):
663689
#
664690
# LANE_MARKER written by every card that sits past a passing panel: the
665-
# start card (not at the cap) and the decide card (not handed
691+
# start card (not at the cap, nor `pending`: an
692+
# `axes_alongside_panel` start runs before the panel has
693+
# passed) and the decide card (not handed
666694
# to a human), and carried by `render_dismissed`. Absent from
667695
# cursor-review's own round card, so the moment a panel round
668696
# decides — whatever it decided — the PR is out of the lane
@@ -705,6 +733,12 @@ def reviewed_sha(body) -> str:
705733
return match.group(1) if match else ""
706734

707735

736+
def reviewed_run_url(body) -> str:
737+
"""The workflow-run link on the card's "Reviewed commit" line, or ""."""
738+
match = _REVIEWED_RUN_RE.search(body or "")
739+
return match.group(1) if match else ""
740+
741+
708742
def stale_head(body) -> str:
709743
"""The head a dismissal-stale card was written for, or ""."""
710744
match = _STALE_HEAD_RE.search(body or "")
@@ -1279,6 +1313,18 @@ def main(argv=None) -> int:
12791313
# The commit this round's axes judge: an approver-only re-check shares its
12801314
# panel round's number, so a same-round entry for ANOTHER commit is prior.
12811315
p.add_argument("--commit-sha", default="")
1316+
p = sub.add_parser("closeout")
1317+
p.add_argument("--repo", required=True)
1318+
p.add_argument("--pr-number", required=True)
1319+
p.add_argument("--login", required=True)
1320+
p.add_argument("--commit-sha", required=True)
1321+
p.add_argument("--round", default="")
1322+
p.add_argument("--approve-gate", default="")
1323+
p.add_argument("--max-rounds", default="")
1324+
p.add_argument("--review-label", default=DEFAULT_REVIEW_LABEL)
1325+
p.add_argument("--run-url", default="")
1326+
p.add_argument("--axes-ok", default="true")
1327+
p.add_argument("--axis-results", default="")
12821328
for name in ("start", "ensure", "decide"):
12831329
p = sub.add_parser(name)
12841330
p.add_argument("--repo", required=True)
@@ -1303,6 +1349,8 @@ def main(argv=None) -> int:
13031349
return 2
13041350
if args.cmd == "prior":
13051351
return cmd_prior(args)
1352+
if args.cmd == "closeout":
1353+
return cmd_closeout(args)
13061354
axes = parse_axes(args.axes)
13071355
entry, keep = None, None
13081356
if args.cmd == "start":
@@ -1403,5 +1451,74 @@ def cmd_prior(args) -> int:
14031451
return 0
14041452

14051453

1454+
_PENDING_ROW_RE = re.compile(r"^\| [a-z]+ \| " + re.escape(PENDING_ROW) + r" \|", re.MULTILINE)
1455+
1456+
1457+
def closeout_body(body, commit_sha: str, round_no, approve_gate: str, max_rounds="",
1458+
label=DEFAULT_REVIEW_LABEL, run_url: str = "", axes_ok: bool = True,
1459+
axis_results: str = ""):
1460+
"""(new body or None, why): the close-out of an `axes_alongside_panel` round.
1461+
1462+
cursor-review's early start card says every axis is pending while its panel
1463+
runs. When that panel then did NOT pass, nothing else rewrites the card on
1464+
every path (the caller's decide runs only on `pass`), so it would keep
1465+
saying "pending" for a round whose axes' verdicts nobody will read. On
1466+
`pass` the same holds when an axis job did not succeed (`axes_ok` false):
1467+
the caller's decide skips then, since a failed axis upload may mean another
1468+
job claimed its artifact name. Only THAT card is rewritten: state `pass`,
1469+
this commit, this round, written by THIS run (a run a newer one superseded
1470+
must not close out the newer run's card), at least one pending axis row.
1471+
Anything else — a card a decide already wrote, one for another commit,
1472+
round or run — is left alone, and so is every card on a clean `pass`.
1473+
"""
1474+
gate = (approve_gate or "").strip()
1475+
if gate == "pass" and axes_ok:
1476+
return None, "the panel passed; the caller's decide writes the verdicts"
1477+
if card_state(body) != STATE_PASS:
1478+
return None, f"the card's state is {card_state(body) or 'unreadable'!r}, not 'pass'"
1479+
if reviewed_sha(body) != (commit_sha or "").strip().lower():
1480+
return None, "the card is for another commit"
1481+
if card_round(body) != _int_or_q(round_no):
1482+
return None, "the card is for another round"
1483+
if reviewed_run_url(body) != (run_url or "").strip():
1484+
return None, "the card was written by another workflow run"
1485+
if not _PENDING_ROW_RE.search(body or ""):
1486+
return None, "the card has no pending axis"
1487+
if gate == "pass":
1488+
detail = f" ({axis_results.strip()})" if axis_results.strip() else ""
1489+
headline = (f"No decision this round: an axis job did not complete{detail}, "
1490+
"so no axis verdict for this commit is trusted.")
1491+
else:
1492+
headline = (f"No decision this round: the review panel did not pass ({gate or 'no verdict'}), "
1493+
"so the axes' verdicts for this commit were discarded.")
1494+
return render_round(round_no, max_rounds, commit_sha, STATE_NO_DECISION, NEXT_PUSH, headline, [],
1495+
next_push_text(label), run_url), "rewritten"
1496+
1497+
1498+
def cmd_closeout(args) -> int:
1499+
"""Rewrite the early start card when its round's panel did not pass.
1500+
1501+
Never an error past the arguments, like `prior`: a card that cannot be read
1502+
or written is a warning, and the round's outcome is already decided.
1503+
"""
1504+
try:
1505+
found = find_card(list_comments(args.repo, args.pr_number), args.login)
1506+
if found is None:
1507+
print("No cursor-approve card on this PR; nothing to close out.")
1508+
return 0
1509+
body, why = closeout_body(found.get("body") or "", args.commit_sha, args.round, args.approve_gate,
1510+
args.max_rounds, args.review_label, args.run_url,
1511+
axes_ok=(args.axes_ok or "").strip().lower() != "false",
1512+
axis_results=args.axis_results)
1513+
if body is None:
1514+
print(f"Leaving the cursor-approve card alone: {why}.")
1515+
return 0
1516+
upsert(args.repo, args.pr_number, args.login, body, card=found)
1517+
print("Closed out the early cursor-approve card: the panel did not pass, or an axis job did not complete.")
1518+
except Exception as e: # noqa: BLE001 — a report; the round is already decided
1519+
print(f"::warning::Could not close out the cursor-approve card ({e.__class__.__name__}: {e})")
1520+
return 0
1521+
1522+
14061523
if __name__ == "__main__":
14071524
sys.exit(main())

‎.github/cursor-approve/tests/test_card.py‎

Lines changed: 103 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -914,5 +914,108 @@ def test_integral_float_renders_as_integer(self):
914914
self.assertEqual(card._int_or_q("1.5"), "?")
915915

916916

917+
class AxesAlongsidePanel(unittest.TestCase):
918+
"""cursor-review's `axes_alongside_panel`: the early start card and its close-out."""
919+
920+
AXES = ["correctness", "conformance"]
921+
RUN_URL = "https://github.com/o/r/actions/runs/9"
922+
923+
def early(self, sha=SHA, round_no="2"):
924+
return card.render_start(round_no, "5", sha, self.AXES, "pending", self.RUN_URL)
925+
926+
def test_a_pending_start_card_carries_no_lane_marker(self):
927+
# Load-bearing (approve_only_on_push): the lane marker says the panel
928+
# passed, and the panel of this round is still running.
929+
body = self.early()
930+
self.assertNotIn(card.LANE_MARKER, body)
931+
self.assertFalse(card.in_axes_lane(body))
932+
self.assertTrue(card.in_axes_lane(card.render_start("2", "5", SHA, self.AXES, "pass", self.RUN_URL)))
933+
934+
def test_a_pending_start_card_is_pass_none_with_the_table_and_the_panel_line(self):
935+
body = self.early()
936+
self.assertEqual(markers(body), ("pass", "none"))
937+
self.assertIn("| correctness | ⏳ pending |", body)
938+
self.assertIn("| conformance | ⏳ pending |", body)
939+
self.assertIn(card.PANEL_RUNNING_TEXT, body)
940+
self.assertNotIn(card.PANEL_RUNNING_TEXT, card.render_start("2", "5", SHA, self.AXES, "pass", ""))
941+
942+
def test_closeout_leaves_a_decided_card_alone(self):
943+
for body in (card.render_decide("2", "5", SHA, self.AXES, decision(correctness="green", conformance="green"),
944+
"approved", "0", ""),
945+
card.render_round("2", "5", SHA, card.STATE_CHANGES, card.NEXT_RESOLVE, "x", [], "y", ""),
946+
card.render_round("2", "5", SHA, card.STATE_PASS, card.NEXT_NONE, "Approved", [], "", "")):
947+
for gate in ("fail", "untrusted", "off"):
948+
new, _ = card.closeout_body(body, SHA, "2", gate, run_url=self.RUN_URL, axes_ok=False)
949+
self.assertIsNone(new)
950+
951+
def test_closeout_leaves_a_card_for_another_commit_or_round_alone(self):
952+
self.assertIsNone(card.closeout_body(self.early(sha="b" * 40), SHA, "2", "fail", run_url=self.RUN_URL)[0])
953+
self.assertIsNone(card.closeout_body(self.early(round_no="1"), SHA, "2", "fail", run_url=self.RUN_URL)[0])
954+
955+
def test_closeout_leaves_a_card_another_run_wrote_alone(self):
956+
# A run a newer one superseded (same commit, same uncounted round) must
957+
# not close out the card that newer run's start wrote.
958+
other = card.render_start("2", "5", SHA, self.AXES, "pending", "https://github.com/o/r/actions/runs/10")
959+
self.assertIsNone(card.closeout_body(other, SHA, "2", "fail", "5", run_url=self.RUN_URL)[0])
960+
self.assertIsNone(card.closeout_body(self.early(), SHA, "2", "fail", "5", run_url="")[0])
961+
962+
def test_closeout_leaves_the_card_alone_when_the_panel_passed(self):
963+
self.assertIsNone(card.closeout_body(self.early(), SHA, "2", "pass", run_url=self.RUN_URL)[0])
964+
965+
def test_closeout_rewrites_a_passing_round_whose_axis_job_did_not_complete(self):
966+
new, why = card.closeout_body(self.early(), SHA, "2", "pass", "5", run_url=self.RUN_URL, axes_ok=False,
967+
axis_results="correctness failure, conformance success")
968+
self.assertEqual(why, "rewritten")
969+
self.assertEqual(markers(new), ("no_decision", "push_then_relabel"))
970+
self.assertIn("an axis job did not complete (correctness failure, conformance success)", new)
971+
self.assertNotIn("⏳ pending", new)
972+
973+
def test_closeout_rewrites_a_pending_card_when_the_panel_did_not_pass(self):
974+
for gate in ("untrusted", "fail", "off"):
975+
new, why = card.closeout_body(self.early(), SHA, "2", gate, "5", run_url=self.RUN_URL)
976+
self.assertEqual(why, "rewritten")
977+
self.assertEqual(markers(new), ("no_decision", "push_then_relabel"))
978+
self.assertIn(f"the review panel did not pass ({gate})", new)
979+
self.assertNotIn("⏳ pending", new)
980+
self.assertFalse(card.in_axes_lane(new))
981+
self.assertEqual(card.reviewed_sha(new), SHA)
982+
self.assertEqual(card.card_round(new), "2")
983+
984+
def test_closeout_is_idempotent(self):
985+
new, _ = card.closeout_body(self.early(), SHA, "2", "fail", run_url=self.RUN_URL)
986+
self.assertIsNotNone(new)
987+
self.assertIsNone(card.closeout_body(new, SHA, "2", "fail", run_url=self.RUN_URL)[0])
988+
989+
def test_closeout_command_edits_only_the_approvers_card_and_never_fails(self):
990+
planted = {"id": 3, "user": {"login": "someone"}, "body": self.early()}
991+
mine = {"id": 7, "user": {"login": LOGIN}, "body": self.early()}
992+
comments = [planted, mine]
993+
patched = []
994+
995+
def fake_gh(args, payload=None):
996+
if "--paginate" in args:
997+
return json.dumps([comments])
998+
patched.append(args[3])
999+
mine["body"] = payload["body"]
1000+
return json.dumps({"id": 7})
1001+
1002+
argv = ["closeout", "--repo", "o/r", "--pr-number", "1", "--login", LOGIN, "--commit-sha", SHA,
1003+
"--round", "2", "--max-rounds", "5", "--approve-gate", "untrusted", "--run-url", self.RUN_URL]
1004+
with mock.patch.object(card, "gh", fake_gh), contextlib.redirect_stdout(io.StringIO()):
1005+
self.assertEqual(card.main(argv), 0)
1006+
self.assertEqual(card.main(argv), 0) # a second run finds nothing pending
1007+
self.assertEqual(patched, ["repos/o/r/issues/comments/7"])
1008+
self.assertEqual(markers(mine["body"]), ("no_decision", "push_then_relabel"))
1009+
self.assertEqual(planted["body"], self.early())
1010+
1011+
def broken_gh(args, payload=None):
1012+
raise RuntimeError("boom")
1013+
1014+
out = io.StringIO()
1015+
with mock.patch.object(card, "gh", broken_gh), contextlib.redirect_stdout(out):
1016+
self.assertEqual(card.main(argv), 0)
1017+
self.assertIn("::warning::Could not close out", out.getvalue())
1018+
1019+
9171020
if __name__ == "__main__":
9181021
unittest.main()

0 commit comments

Comments
 (0)