Surface PR merge conflicts on the review card
The poller now reads GitHub's mergeable field alongside reviews and checks. A CONFLICTING PR drops any approved-green verdict (as changes-needed-by-you, not a CI failure), wears an alarm-coloured conflicts chip in the card's footer row, and narrates the flip in the ticker. GitHub computes mergeability lazily, so UNKNOWN keeps the previous reading instead of flapping the chip. The poll fold is now a pure function (_fold), so the verdict logic is testable without gh. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
@@ -285,10 +285,18 @@ review and line comment, addresses each point or says why not, commits,
|
||||
pushes so the PR updates, and appends a `## PR update` section to the task
|
||||
file. Then **◔ review PR** again, until it settles.
|
||||
|
||||
The board polls open PRs of review-stage cards (reviews + CI checks, every
|
||||
60s — a plain thread in board.py, no agent involved, silent when review/ is
|
||||
empty) and folds everything into one verdict — any changes-requested review
|
||||
or failing check wins over any approval. Tool chips (CI, copilot, PR, drive)
|
||||
The board polls open PRs of review-stage cards (reviews + CI checks +
|
||||
GitHub's mergeable state, every 60s — a plain thread in board.py, no agent
|
||||
involved, silent when review/ is empty) and folds everything into one
|
||||
verdict — any changes-requested review or failing check wins over any
|
||||
approval. A PR GitHub cannot merge cleanly wears an alarm-coloured
|
||||
`conflicts` chip and counts as changes-needed-by-you (not a CI failure);
|
||||
**↻ act on PR** resolves mechanical conflicts by merging main into the
|
||||
branch — additively, never rebasing or force-pushing — in a dedicated
|
||||
resolution commit, and refuses semantic ones, naming the collision for a
|
||||
human to settle. GitHub computes mergeability lazily, so an UNKNOWN
|
||||
reading keeps the chip's last state rather than flapping.
|
||||
Tool chips (CI, copilot, PR, drive)
|
||||
are destinations, not statuses: they live in the card's footer row, never
|
||||
squeezed into the author row — `CI ✓` (pine), `CI ✕` (terracotta), `◌`
|
||||
while in flight, with hover actions staying in the status pill's slot. The card wears it in the design
|
||||
|
||||
@@ -881,6 +881,10 @@ function cardFor(task) {
|
||||
chips.push({ label: 'CI', glyph: { pass: '✓', fail: '✕', running: '◌' }[prState.ci],
|
||||
cls: { pass: 'ok', fail: 'bad', running: 'accent' }[prState.ci], title: detail });
|
||||
}
|
||||
if (prState && prState.conflicts) {
|
||||
chips.push({ label: 'conflicts', glyph: '✕', cls: 'bad',
|
||||
title: 'GitHub cannot merge this into main — ↻ act on PR can attempt a resolution merge' });
|
||||
}
|
||||
if (prState && prState.copilot) {
|
||||
chips.push({ label: 'copilot',
|
||||
glyph: { asked: '◌', approved: '✓', changes: '✕', commented: '·' }[prState.copilot],
|
||||
|
||||
+51
-18
@@ -191,16 +191,21 @@ def _is_copilot(login) -> bool:
|
||||
return "copilot" in str(login or "").lower()
|
||||
|
||||
|
||||
def _poll_pr(filename: str, url: str) -> None:
|
||||
number = url.rstrip("/").rsplit("/", 1)[-1]
|
||||
result = _run([config.GH_BIN, "pr", "view", number,
|
||||
"--json", "reviews,reviewRequests,statusCheckRollup,state"], timeout=60)
|
||||
if result.returncode != 0:
|
||||
return
|
||||
try:
|
||||
data = json.loads(result.stdout)
|
||||
except json.JSONDecodeError:
|
||||
return
|
||||
def _conflict_state(data: dict, prev: dict) -> bool | None:
|
||||
"""GitHub computes mergeability lazily: UNKNOWN means "not computed
|
||||
yet", never "fine" — keep the previous reading so the chip does not
|
||||
flap while GitHub thinks."""
|
||||
mergeable = str(data.get("mergeable") or "").upper()
|
||||
if mergeable == "CONFLICTING":
|
||||
return True
|
||||
if mergeable == "MERGEABLE":
|
||||
return False
|
||||
return prev.get("conflicts")
|
||||
|
||||
|
||||
def _fold(data: dict, prev: dict) -> dict:
|
||||
"""One gh pr-view payload + the previous snapshot → the new snapshot.
|
||||
Pure fold: fetching, events and broadcasts stay in _poll_pr."""
|
||||
reviews = data.get("reviews") or []
|
||||
checks = data.get("statusCheckRollup") or []
|
||||
changes = any(r.get("state") == "CHANGES_REQUESTED" for r in reviews)
|
||||
@@ -219,15 +224,18 @@ def _poll_pr(filename: str, url: str) -> None:
|
||||
elif cop_requested:
|
||||
copilot = "asked"
|
||||
else:
|
||||
copilot = PR_STATE.get(filename, {}).get("copilot")
|
||||
copilot = "asked" if copilot == "asked" else None
|
||||
copilot = "asked" if prev.get("copilot") == "asked" else None
|
||||
|
||||
states = [_check_state(c) for c in checks]
|
||||
ci = ("fail" if "fail" in states
|
||||
else "running" if "running" in states
|
||||
else "pass" if states else None)
|
||||
|
||||
verdict = ("red" if (changes or ci == "fail")
|
||||
conflicts = _conflict_state(data, prev)
|
||||
|
||||
# A conflict is changes-needed-by-you, not a CI failure: it beats any
|
||||
# approval but leaves the CI chip telling its own story.
|
||||
verdict = ("red" if (changes or ci == "fail" or conflicts)
|
||||
else "green" if approved else "pending")
|
||||
detail_bits = []
|
||||
if reviews:
|
||||
@@ -235,14 +243,32 @@ def _poll_pr(filename: str, url: str) -> None:
|
||||
if ci:
|
||||
detail_bits.append({"fail": "checks failing", "running": "checks running",
|
||||
"pass": "checks ok"}[ci])
|
||||
if conflicts:
|
||||
detail_bits.append("conflicts with main")
|
||||
if copilot:
|
||||
detail_bits.append("copilot " + {"asked": "asked", "approved": "approved",
|
||||
"changes": "asked for changes",
|
||||
"commented": "commented"}[copilot])
|
||||
return {"verdict": verdict, "ci": ci, "copilot": copilot,
|
||||
"conflicts": conflicts, "detail": " · ".join(detail_bits)}
|
||||
|
||||
|
||||
def _poll_pr(filename: str, url: str) -> None:
|
||||
number = url.rstrip("/").rsplit("/", 1)[-1]
|
||||
result = _run([config.GH_BIN, "pr", "view", number,
|
||||
"--json", "reviews,reviewRequests,statusCheckRollup,state,mergeable"],
|
||||
timeout=60)
|
||||
if result.returncode != 0:
|
||||
return
|
||||
try:
|
||||
data = json.loads(result.stdout)
|
||||
except json.JSONDecodeError:
|
||||
return
|
||||
prev = PR_STATE.get(filename, {})
|
||||
PR_STATE[filename] = {"verdict": verdict, "ci": ci, "copilot": copilot,
|
||||
"url": url, "detail": " · ".join(detail_bits),
|
||||
"ts": time.time()}
|
||||
entry = _fold(data, prev)
|
||||
entry.update({"url": url, "ts": time.time()})
|
||||
PR_STATE[filename] = entry
|
||||
verdict, ci, copilot = entry["verdict"], entry["ci"], entry["copilot"]
|
||||
if prev.get("copilot") in (None, "asked") and copilot in ("approved", "changes", "commented"):
|
||||
word = {"approved": "approved it", "changes": "asked for changes",
|
||||
"commented": "commented"}[copilot]
|
||||
@@ -250,7 +276,14 @@ def _poll_pr(filename: str, url: str) -> None:
|
||||
"kind": "agent", "actor": "board", "file": filename,
|
||||
"summary": f"Copilot reviewed {filename}'s PR and {word}"})
|
||||
state.broadcast({"type": "board"})
|
||||
if prev.get("verdict") != verdict and verdict != "pending":
|
||||
if bool(prev.get("conflicts")) != bool(entry["conflicts"]):
|
||||
state.record_board_event({
|
||||
"kind": "agent", "actor": "board", "file": filename,
|
||||
"summary": (f"{filename}'s PR conflicts with main — ↻ act on PR "
|
||||
f"can attempt the resolution" if entry["conflicts"]
|
||||
else f"{filename}'s PR no longer conflicts with main")})
|
||||
state.broadcast({"type": "board"})
|
||||
elif prev.get("verdict") != verdict and verdict != "pending":
|
||||
word = "approved" if verdict == "green" else "changes asked"
|
||||
state.record_board_event({
|
||||
"kind": "agent", "actor": "board", "file": filename,
|
||||
@@ -283,7 +316,7 @@ def poller() -> None:
|
||||
|
||||
|
||||
def public_state() -> dict:
|
||||
return {f: {k: v.get(k) for k in ("verdict", "ci", "copilot", "detail", "url")}
|
||||
return {f: {k: v.get(k) for k in ("verdict", "ci", "copilot", "conflicts", "detail", "url")}
|
||||
for f, v in PR_STATE.items()}
|
||||
|
||||
|
||||
|
||||
@@ -0,0 +1,95 @@
|
||||
"""Conflicted PRs become card state: the poller folds GitHub's
|
||||
mergeable field into the PR snapshot, a conflict drops any
|
||||
approved-green verdict as changes-needed-by-you (not a CI failure),
|
||||
and GitHub's lazily-computed UNKNOWN keeps the previous reading so the
|
||||
chip never flaps. The snapshot's `conflicts` key must reach the UI."""
|
||||
|
||||
import sys
|
||||
import unittest
|
||||
from pathlib import Path
|
||||
|
||||
REPO = Path(__file__).resolve().parents[1]
|
||||
sys.path.insert(0, str(REPO / "manager" / "core"))
|
||||
|
||||
import github # noqa: E402
|
||||
|
||||
|
||||
def payload(**overrides):
|
||||
"""A gh pr-view JSON payload with the fields _fold reads."""
|
||||
data = {"reviews": [], "reviewRequests": [], "statusCheckRollup": []}
|
||||
data.update(overrides)
|
||||
return data
|
||||
|
||||
|
||||
APPROVED = [{"state": "APPROVED"}]
|
||||
|
||||
|
||||
class ConflictFolding(unittest.TestCase):
|
||||
def test_conflict_drops_green_even_when_approved(self):
|
||||
entry = github._fold(payload(reviews=APPROVED,
|
||||
mergeable="CONFLICTING"), {})
|
||||
self.assertTrue(entry["conflicts"])
|
||||
self.assertEqual(entry["verdict"], "red")
|
||||
self.assertIn("conflicts with main", entry["detail"])
|
||||
|
||||
def test_conflict_is_not_a_ci_failure(self):
|
||||
entry = github._fold(payload(mergeable="CONFLICTING"), {})
|
||||
self.assertIsNone(entry["ci"])
|
||||
self.assertEqual(entry["verdict"], "red")
|
||||
|
||||
def test_mergeable_approved_pr_stays_green(self):
|
||||
entry = github._fold(payload(reviews=APPROVED,
|
||||
mergeable="MERGEABLE"), {})
|
||||
self.assertFalse(entry["conflicts"])
|
||||
self.assertEqual(entry["verdict"], "green")
|
||||
|
||||
def test_unknown_keeps_the_previous_reading_both_ways(self):
|
||||
# GitHub computes mergeability lazily after a push: UNKNOWN means
|
||||
# "not yet", never "fine" — the chip must not flap.
|
||||
still = github._fold(payload(mergeable="UNKNOWN"),
|
||||
{"conflicts": True})
|
||||
self.assertTrue(still["conflicts"])
|
||||
self.assertEqual(still["verdict"], "red")
|
||||
clean = github._fold(payload(reviews=APPROVED, mergeable="UNKNOWN"),
|
||||
{"conflicts": False})
|
||||
self.assertFalse(clean["conflicts"])
|
||||
self.assertEqual(clean["verdict"], "green")
|
||||
|
||||
def test_unknown_on_first_sight_alarms_nobody(self):
|
||||
entry = github._fold(payload(mergeable="UNKNOWN"), {})
|
||||
self.assertIsNone(entry["conflicts"])
|
||||
self.assertEqual(entry["verdict"], "pending")
|
||||
self.assertNotIn("conflicts", entry["detail"])
|
||||
|
||||
def test_resolution_lets_green_return(self):
|
||||
entry = github._fold(payload(reviews=APPROVED, mergeable="MERGEABLE"),
|
||||
{"conflicts": True, "verdict": "red"})
|
||||
self.assertFalse(entry["conflicts"])
|
||||
self.assertEqual(entry["verdict"], "green")
|
||||
|
||||
|
||||
class SnapshotReachesTheUI(unittest.TestCase):
|
||||
def test_public_state_carries_conflicts(self):
|
||||
github.PR_STATE["x.md"] = {"verdict": "red", "ci": None,
|
||||
"copilot": None, "conflicts": True,
|
||||
"detail": "conflicts with main",
|
||||
"url": "u", "ts": 1}
|
||||
try:
|
||||
self.assertTrue(github.public_state()["x.md"]["conflicts"])
|
||||
finally:
|
||||
github.PR_STATE.pop("x.md", None)
|
||||
|
||||
def test_the_card_wears_an_alarm_coloured_chip(self):
|
||||
html = (REPO / "manager" / "core" / "board.html").read_text(
|
||||
encoding="utf-8")
|
||||
self.assertIn("prState.conflicts", html)
|
||||
self.assertIn("label: 'conflicts', glyph: '✕', cls: 'bad'", html)
|
||||
|
||||
def test_poller_asks_github_for_mergeable(self):
|
||||
source = (REPO / "manager" / "core" / "github.py").read_text(
|
||||
encoding="utf-8")
|
||||
self.assertIn("statusCheckRollup,state,mergeable", source)
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
unittest.main()
|
||||
Reference in New Issue
Block a user