diff --git a/AGENTS.md b/AGENTS.md index 6b7f70f..50a4849 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -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 diff --git a/manager/core/board.html b/manager/core/board.html index 23c3c2c..aba5e90 100644 --- a/manager/core/board.html +++ b/manager/core/board.html @@ -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], diff --git a/manager/core/github.py b/manager/core/github.py index ad44e03..b3c80c9 100644 --- a/manager/core/github.py +++ b/manager/core/github.py @@ -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()} diff --git a/tests/test_pr_conflicts.py b/tests/test_pr_conflicts.py new file mode 100644 index 0000000..17ff960 --- /dev/null +++ b/tests/test_pr_conflicts.py @@ -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()