diff --git a/README.md b/README.md index b51c9bb..fa0b593 100644 --- a/README.md +++ b/README.md @@ -34,10 +34,11 @@ Open http://127.0.0.1:8765 ## How it works -1. **Background worker** polls your filter (`PR_SEARCH_FILTER`) every `POLL_INTERVAL` seconds and pre-reviews any new or changed non-draft PR, caching the result. The queue shows each PR's status: `reviewing`, `ready`, `error`, or `closed`. **Refresh now** forces an immediate poll. +1. **Background worker** polls your filter (`PR_SEARCH_FILTER`) every `POLL_INTERVAL` seconds and pre-reviews any new or changed non-draft PR, caching the result. The queue is grouped into a collapsible section per repo (collapse state persists), with PRs ordered oldest to newest. Each shows its status: `reviewing`, `ready`, `error`, or `closed`. **Refresh now** forces an immediate poll. 2. **Open a PR** — if its review is `ready`, it appears instantly. Otherwise you see its status, and you can **Run review now** on demand. 3. **Request revision** re-runs the review with your notes folded in as a trusted instruction, separate from the untrusted diff. 4. **Post review to GitHub** submits it as a PR review. You confirm the event type (COMMENT / APPROVE / REQUEST_CHANGES) and can edit the body first. +5. **Enable auto-merge** (optional) from the PR detail view: pick a method (squash/merge/rebase, squash default per handbook) and GitHub merges the PR automatically once required checks pass. Nothing merges without you clicking it. ### Auto-review worker diff --git a/app/github_client.py b/app/github_client.py index 66d5bce..a78fde5 100644 --- a/app/github_client.py +++ b/app/github_client.py @@ -19,6 +19,17 @@ import httpx2 from .config import Config API = "https://api.github.com" +GRAPHQL = "https://api.github.com/graphql" + +_ENABLE_AUTOMERGE = """ +mutation($pr: ID!, $method: PullRequestMergeMethod!) { + enablePullRequestAutoMerge(input: {pullRequestId: $pr, mergeMethod: $method}) { + pullRequest { number autoMergeRequest { enabledAt } } + } +} +""" + +_MERGE_METHODS = {"SQUASH", "MERGE", "REBASE"} class GitHubError(RuntimeError): @@ -94,6 +105,8 @@ class GitHubClient: "url": it["html_url"], "author": (it.get("user") or {}).get("login", ""), "updated_at": it["updated_at"], + "created_at": it.get("created_at", ""), + "node_id": it.get("node_id", ""), "body": it.get("body") or "", "draft": it.get("draft", False), } @@ -133,3 +146,23 @@ class GitHubClient: ) _raise_for_status(r) return r.json() + + # --- write: enable auto-merge (GraphQL; side-effectful) --- + def enable_automerge(self, node_id: str, method: str) -> dict[str, Any]: + method = (method or "").upper() + if method not in _MERGE_METHODS: + raise GitHubError(f"Invalid merge method: {method}") + if not node_id: + raise GitHubError("Missing PR node id for auto-merge") + payload = { + "query": _ENABLE_AUTOMERGE, + "variables": {"pr": node_id, "method": method}, + } + with httpx2.Client(timeout=30) as c: + r = c.post(GRAPHQL, headers=self._headers, json=payload) + _raise_for_status(r) # HTTP-level (401/403/5xx) + data = r.json() + if data.get("errors"): # GraphQL-level (e.g. auto-merge not allowed) + msg = "; ".join(e.get("message", "") for e in data["errors"]) + raise GitHubError(msg or "GraphQL error enabling auto-merge") + return (data.get("data") or {}).get("enablePullRequestAutoMerge") or {} diff --git a/app/main.py b/app/main.py index 4979c53..45282b1 100644 --- a/app/main.py +++ b/app/main.py @@ -150,6 +150,8 @@ def _serialize_review(row: dict[str, Any]) -> dict[str, Any]: "url": row.get("pr_url"), "status": row["status"], "updated_at": row.get("pr_updated_at"), + "created_at": row.get("pr_created_at"), + "node_id": row.get("pr_node_id"), "cached_at": row.get("cached_at"), "attempts": row.get("attempts", 0), "error": row.get("error"), @@ -263,6 +265,23 @@ def api_post(req: PostReq) -> dict[str, Any]: raise HTTPException(400, str(e)) +class AutomergeReq(BaseModel): + owner: str + repo: str + number: int + node_id: str + method: str = "SQUASH" # SQUASH | MERGE | REBASE + + +@app.post("/api/automerge") +def api_automerge(req: AutomergeReq) -> dict[str, Any]: + try: + res = gh().enable_automerge(req.node_id, req.method) + return {"ok": True, "result": res} + except GitHubError as e: + raise HTTPException(400, str(e)) + + def main() -> None: import uvicorn diff --git a/app/store.py b/app/store.py index 63a3b04..52f9213 100644 --- a/app/store.py +++ b/app/store.py @@ -36,12 +36,22 @@ CREATE TABLE IF NOT EXISTS reviews ( pr_author TEXT, pr_url TEXT, pr_updated_at TEXT, -- PR's updatedAt on GitHub + pr_created_at TEXT, -- PR's createdAt (for oldest->newest sort) + pr_node_id TEXT, -- PR GraphQL global id (for auto-merge) cached_at REAL NOT NULL, -- when this row was last written closed_at REAL, -- when the PR left the queue PRIMARY KEY (owner, repo, number) ); """ +# Nullable columns added after the initial release. Migrated in via ALTER TABLE +# on an existing db. Only nullable, no-default columns are safe to ALTER-ADD in +# SQLite, so every entry here must be a bare nullable type. +_MIGRATIONS = ( + ("pr_created_at", "TEXT"), + ("pr_node_id", "TEXT"), +) + # Columns a caller may set via upsert(); owner/repo/number are the key and # cached_at is stamped automatically. _UPSERTABLE = ( @@ -54,6 +64,8 @@ _UPSERTABLE = ( "pr_author", "pr_url", "pr_updated_at", + "pr_created_at", + "pr_node_id", "closed_at", ) @@ -63,6 +75,18 @@ class ReviewStore: self.db_path = db_path with _WRITE_LOCK, self._connect() as conn: conn.executescript(_SCHEMA) + self._migrate(conn) + + def _migrate(self, conn: sqlite3.Connection) -> None: + """Add columns introduced after a db was first created. Each ALTER is + idempotent (guarded by a table_info check) and adds only a nullable + column, so it is safe on an existing db and safe to re-run. Errors + propagate so a broken db fails loudly rather than running half-migrated.""" + existing = {row[1] for row in conn.execute("PRAGMA table_info(reviews)")} + for name, coltype in _MIGRATIONS: + if name not in existing: + conn.execute(f"ALTER TABLE reviews ADD COLUMN {name} {coltype}") + conn.commit() def _connect(self) -> sqlite3.Connection: conn = sqlite3.connect(self.db_path, timeout=30) diff --git a/app/worker.py b/app/worker.py index 333e7b3..7481d3a 100644 --- a/app/worker.py +++ b/app/worker.py @@ -77,6 +77,8 @@ def _review_one( "pr_author": pr.get("author"), "pr_url": pr.get("url"), "pr_updated_at": pr.get("updated_at"), + "pr_created_at": pr.get("created_at"), + "pr_node_id": pr.get("node_id"), } try: diff = gh.pr_diff(owner, repo, number) @@ -150,7 +152,22 @@ def run_cycle( if existing: same_update = existing.get("pr_updated_at") == p.get("updated_at") if existing.get("status") == "ready" and same_update: - continue # cheap gate: nothing changed since last ready review + # Cheap gate: no re-review. But backfill lightweight PR metadata + # (e.g. node_id/created_at added in a later release) when it + # actually differs, so features like auto-merge have current data + # without re-running the model. No-op write when nothing differs. + if existing.get("pr_node_id") != p.get("node_id") or existing.get( + "pr_created_at" + ) != p.get("created_at"): + store.upsert( + p["owner"], + p["repo"], + p["number"], + status="ready", + pr_node_id=p.get("node_id"), + pr_created_at=p.get("created_at"), + ) + continue if ( existing.get("status") == "error" and same_update @@ -170,6 +187,8 @@ def run_cycle( pr_author=p.get("author"), pr_url=p.get("url"), pr_updated_at=p.get("updated_at"), + pr_created_at=p.get("created_at"), + pr_node_id=p.get("node_id"), ) results = { diff --git a/static/index.html b/static/index.html index 42a84db..1d20900 100644 --- a/static/index.html +++ b/static/index.html @@ -60,6 +60,21 @@ .badge.closed { color: var(--ink-dim); border-color: var(--line); } .badge.mention { color: var(--accent); border-color: var(--accent); } .queue-item.closed { opacity: .55; } + .repo-group { border-bottom: 1px solid var(--line); } + .repo-header { + display: flex; align-items: center; gap: 8px; width: 100%; + padding: 9px 14px; background: var(--panel-2); border: none; border-radius: 0; + color: var(--ink); font-family: var(--mono); font-size: 12px; cursor: pointer; + text-align: left; border-bottom: 1px solid var(--line); + } + .repo-header:hover { color: var(--accent); } + .repo-header .chev { width: 12px; color: var(--ink-dim); } + .repo-header .repo-name { flex: 1; overflow: hidden; text-overflow: ellipsis; white-space: nowrap; } + .repo-header .repo-count { + color: var(--ink-dim); background: var(--bg); border: 1px solid var(--line); + border-radius: 10px; padding: 0 7px; font-size: 11px; + } + .queue-item { padding-left: 22px; } .detail { overflow-y: auto; padding: 24px 28px; } .detail h2 { font-size: 17px; margin: 0 0 4px; } .detail .prlink { font-family: var(--mono); font-size: 12px; color: var(--accent); text-decoration: none; } @@ -204,26 +219,80 @@ async function poll(){ const STATUS_LABEL = { reviewing: "reviewing", ready: "ready", error: "error", closed: "closed" }; +const COLLAPSE_KEY = "pr-reviewer:collapsed:v1"; +function loadCollapsed(){ + try { return new Set(JSON.parse(localStorage.getItem(COLLAPSE_KEY) || "[]")); } + catch(e){ return new Set(); } +} +function saveCollapsed(set, repoKeys){ + // Prune repos that no longer exist so the stored set can't grow unbounded. + const pruned = [...set].filter(r => repoKeys.has(r)); + localStorage.setItem(COLLAPSE_KEY, JSON.stringify(pruned)); +} + +// PR age for oldest->newest ordering; fall back to updated_at if created_at absent. +function prTime(it){ return Date.parse(it.created_at || it.updated_at || "") || 0; } + +function queueItem(it){ + const k = key(it); + const div = document.createElement("div"); + div.className = "queue-item" + (state.active===k ? " active":"") + (it.status==="closed" ? " closed":""); + const mention = (state.mentions||[]).includes((it.author||"").toLowerCase()); + const statusBadge = `${STATUS_LABEL[it.status]||it.status}`; + div.innerHTML = ` +
#${it.number}
+
${escapeHtml(it.title||"(no title)")}
+
+ @${escapeHtml(it.author||"")} + ${mention ? 'will @mention':''} + ${statusBadge} +
`; + div.onclick = ()=>selectPR(k); + return div; +} + function renderQueue(){ const q = document.getElementById("queue"); if(!state.items.length){ q.innerHTML = '
No PRs in the queue yet. The worker reviews new PRs automatically.
'; return; } - q.innerHTML = ""; + + // Group by owner/repo. + const groups = {}; state.items.forEach(it => { - const k = key(it); - const div = document.createElement("div"); - div.className = "queue-item" + (state.active===k ? " active":"") + (it.status==="closed" ? " closed":""); - const mention = (state.mentions||[]).includes((it.author||"").toLowerCase()); - const statusBadge = `${STATUS_LABEL[it.status]||it.status}`; - div.innerHTML = ` -
${it.owner}/${it.repo} #${it.number}
-
${escapeHtml(it.title||"(no title)")}
-
- @${escapeHtml(it.author||"")} - ${mention ? 'will @mention':''} - ${statusBadge} -
`; - div.onclick = ()=>selectPR(k); - q.appendChild(div); + const g = `${it.owner}/${it.repo}`; + (groups[g] = groups[g] || []).push(it); + }); + const repoKeys = new Set(Object.keys(groups)); + const collapsed = loadCollapsed(); + saveCollapsed(collapsed, repoKeys); + + // Repo groups ordered by their oldest PR; PRs within each group oldest->newest. + const orderedRepos = Object.keys(groups).sort( + (a, b) => Math.min(...groups[a].map(prTime)) - Math.min(...groups[b].map(prTime)) + ); + + q.innerHTML = ""; + orderedRepos.forEach(repo => { + const items = groups[repo].sort((a, b) => prTime(a) - prTime(b)); + const isCollapsed = collapsed.has(repo); + const section = document.createElement("div"); + section.className = "repo-group"; + + const header = document.createElement("button"); + header.className = "repo-header"; + header.setAttribute("aria-expanded", String(!isCollapsed)); + header.innerHTML = `${isCollapsed ? "▸" : "▾"}` + + `${escapeHtml(repo)}` + + `${items.length}`; + header.onclick = () => { + const c = loadCollapsed(); + c.has(repo) ? c.delete(repo) : c.add(repo); + saveCollapsed(c, repoKeys); + renderQueue(); + }; + section.appendChild(header); + + if(!isCollapsed){ items.forEach(it => section.appendChild(queueItem(it))); } + q.appendChild(section); }); } @@ -305,10 +374,37 @@ function renderReview(pr, rv){ + + + +
+ +
`; document.getElementById("rerunBtn").onclick = ()=>runReview(pr); document.getElementById("reviseBtn").onclick = ()=>revise(pr); document.getElementById("postBtn").onclick = ()=>postReview(pr); + document.getElementById("automergeBtn").onclick = ()=>automerge(pr); +} + +async function automerge(pr){ + if(!pr.node_id){ toast("No node id for this PR yet; refresh the queue.", true); return; } + const method = document.getElementById("mergeMethod").value; + if(!confirm(`Enable ${method} auto-merge for ${pr.owner}/${pr.repo}#${pr.number}? It will merge automatically once required checks pass.`)) return; + const btn = document.getElementById("automergeBtn"); + btn.disabled = true; btn.textContent = "Enabling..."; + try { + await j("/api/automerge", { + method:"POST", headers:{"Content-Type":"application/json"}, + body: JSON.stringify({owner:pr.owner, repo:pr.repo, number:pr.number, node_id:pr.node_id, method}) + }); + toast("Auto-merge enabled."); + btn.textContent = "Auto-merge enabled"; + } catch(e){ toast("Auto-merge failed: "+e.message, true); btn.disabled=false; btn.textContent="Enable auto-merge"; } } async function revise(pr){ diff --git a/tests/test_endpoints.py b/tests/test_endpoints.py index 8b051e6..008eb25 100644 --- a/tests/test_endpoints.py +++ b/tests/test_endpoints.py @@ -261,6 +261,49 @@ def test_api_refresh_503_without_worker(client: TestClient, monkeypatch) -> None assert client.post("/api/refresh").status_code == 503 +def test_api_automerge_ok(client: TestClient, monkeypatch) -> None: + class MergeGH(FakeGH): + def enable_automerge(self, node_id, method): + self.merged = {"node_id": node_id, "method": method} + return {"pullRequest": {"number": 1}} + + gh = MergeGH() + _patch(monkeypatch, gh=gh) + r = client.post( + "/api/automerge", + json={ + "owner": "o", + "repo": "r", + "number": 1, + "node_id": "PR_x", + "method": "SQUASH", + }, + ) + assert r.status_code == 200 + assert r.json()["ok"] is True + assert gh.merged == {"node_id": "PR_x", "method": "SQUASH"} + + +def test_api_automerge_error_maps_to_400(client: TestClient, monkeypatch) -> None: + class BoomGH(FakeGH): + def enable_automerge(self, node_id, method): + raise GitHubError("Auto merge is not allowed for this repository") + + _patch(monkeypatch, gh=BoomGH()) + r = client.post( + "/api/automerge", + json={ + "owner": "o", + "repo": "r", + "number": 1, + "node_id": "PR_x", + "method": "SQUASH", + }, + ) + assert r.status_code == 400 + assert "not allowed" in r.json()["detail"] + + def test_api_handbook_disabled_without_provider( client: TestClient, monkeypatch ) -> None: diff --git a/tests/test_github_client.py b/tests/test_github_client.py index ed4bc54..e194bdf 100644 --- a/tests/test_github_client.py +++ b/tests/test_github_client.py @@ -124,6 +124,28 @@ def test_search_prs_parses_owner_repo_number(monkeypatch, make_cfg) -> None: assert calls[0]["params"]["q"] == client.cfg.PR_SEARCH_FILTER +def test_search_prs_includes_created_at_and_node_id(monkeypatch, make_cfg) -> None: + payload = { + "items": [ + { + "repository_url": "https://api.github.com/repos/o/r", + "number": 7, + "title": "t", + "html_url": "u", + "user": {"login": "octocat"}, + "updated_at": "2026-07-01T00:00:00Z", + "created_at": "2026-06-20T00:00:00Z", + "node_id": "PR_kwABC123", + } + ] + } + client = _client(make_cfg) + patch_httpx(monkeypatch, gh_mod, FakeResponse(200, json_data=payload)) + pr = client.search_prs()[0] + assert pr["created_at"] == "2026-06-20T00:00:00Z" + assert pr["node_id"] == "PR_kwABC123" + + def test_search_prs_handles_missing_user(monkeypatch, make_cfg) -> None: payload = { "items": [ @@ -205,3 +227,59 @@ def test_pr_diff_returns_raw_text(monkeypatch, make_cfg) -> None: out = client.pr_diff("o", "r", 5) assert out == diff assert calls[0]["url"].endswith("/repos/o/r/pulls/5") + + +# --------------------------------------------------------------------------- # +# GitHubClient.enable_automerge +# --------------------------------------------------------------------------- # + + +def test_enable_automerge_builds_graphql_mutation(monkeypatch, make_cfg) -> None: + client = _client(make_cfg) + calls = patch_httpx( + monkeypatch, + gh_mod, + FakeResponse(200, json_data={"data": {"enablePullRequestAutoMerge": {}}}), + ) + client.enable_automerge("PR_node1", "squash") # lowercase normalized + assert calls[0]["url"].endswith("/graphql") + body = calls[0]["json"] + assert body["variables"] == {"pr": "PR_node1", "method": "SQUASH"} + assert "enablePullRequestAutoMerge" in body["query"] + + +def test_enable_automerge_rejects_bad_method(make_cfg) -> None: + client = _client(make_cfg) + with pytest.raises(GitHubError, match=r"Invalid merge method"): + client.enable_automerge("PR_node1", "FASTFORWARD") + + +def test_enable_automerge_rejects_missing_node_id(make_cfg) -> None: + client = _client(make_cfg) + with pytest.raises(GitHubError, match=r"Missing PR node id"): + client.enable_automerge("", "SQUASH") + + +def test_enable_automerge_raises_on_graphql_errors(monkeypatch, make_cfg) -> None: + client = _client(make_cfg) + patch_httpx( + monkeypatch, + gh_mod, + FakeResponse( + 200, + json_data={ + "errors": [{"message": "Auto merge is not allowed for this repository"}] + }, + ), + ) + with pytest.raises(GitHubError, match=r"not allowed for this repository"): + client.enable_automerge("PR_node1", "SQUASH") + + +def test_enable_automerge_raises_on_http_error(monkeypatch, make_cfg) -> None: + client = _client(make_cfg) + patch_httpx( + monkeypatch, gh_mod, FakeResponse(401, json_data={"message": "Bad credentials"}) + ) + with pytest.raises(GitHubError, match=r"GitHub 401: Bad credentials"): + client.enable_automerge("PR_node1", "SQUASH") diff --git a/tests/test_store.py b/tests/test_store.py index 4734d01..f95972e 100644 --- a/tests/test_store.py +++ b/tests/test_store.py @@ -3,11 +3,23 @@ from __future__ import annotations import json +import sqlite3 import pytest from app.store import ReviewStore +# The schema as it shipped before pr_created_at / pr_node_id were added. +_OLD_SCHEMA = """ +CREATE TABLE reviews ( + owner TEXT NOT NULL, repo TEXT NOT NULL, number INTEGER NOT NULL, + status TEXT NOT NULL, diff_hash TEXT, review_json TEXT, error TEXT, + attempts INTEGER NOT NULL DEFAULT 0, pr_title TEXT, pr_author TEXT, + pr_url TEXT, pr_updated_at TEXT, cached_at REAL NOT NULL, closed_at REAL, + PRIMARY KEY (owner, repo, number) +); +""" + @pytest.fixture def store(tmp_path) -> ReviewStore: @@ -76,6 +88,45 @@ def test_reset_stale_reviewing(store: ReviewStore) -> None: assert store.get("o", "r", 2)["status"] == "ready" +def test_migration_adds_columns_to_old_schema(tmp_path) -> None: + db = str(tmp_path / "old.db") + con = sqlite3.connect(db) + con.executescript(_OLD_SCHEMA) + con.execute( + "INSERT INTO reviews (owner, repo, number, status, cached_at) " + "VALUES ('o', 'r', 1, 'ready', 1.0)" + ) + con.commit() + con.close() + + store = ReviewStore(db) # __init__ migrates + + con = sqlite3.connect(db) + cols = {row[1] for row in con.execute("PRAGMA table_info(reviews)")} + con.close() + assert "pr_created_at" in cols + assert "pr_node_id" in cols + + # Existing row survived, and the new columns are writable. + assert store.get("o", "r", 1)["status"] == "ready" + store.upsert( + "o", "r", 1, status="ready", pr_created_at="2026-06-01", pr_node_id="PR_x" + ) + row = store.get("o", "r", 1) + assert row["pr_created_at"] == "2026-06-01" + assert row["pr_node_id"] == "PR_x" + + +def test_migration_is_idempotent(tmp_path) -> None: + db = str(tmp_path / "c.db") + ReviewStore(db) + ReviewStore(db) # second open must not fail on already-present columns + con = sqlite3.connect(db) + cols = {row[1] for row in con.execute("PRAGMA table_info(reviews)")} + con.close() + assert {"pr_created_at", "pr_node_id"} <= cols + + def test_mark_missing_closed_and_purge(store: ReviewStore) -> None: store.upsert("o", "r", 1, status="ready") store.upsert("o", "r", 2, status="ready") diff --git a/tests/test_worker.py b/tests/test_worker.py index 4ae0596..8fe469f 100644 --- a/tests/test_worker.py +++ b/tests/test_worker.py @@ -81,6 +81,25 @@ def test_cheap_gate_skips_unchanged_updated_at(store: ReviewStore) -> None: assert rev.calls == 1 # not re-reviewed, and no diff even compared +def test_cheap_gate_backfills_metadata_without_rereview(store: ReviewStore) -> None: + gh = FakeGH([_pr(1, updated_at="v1")], {1: "d1"}) + rev = CountingReviewer() + run_cycle(gh, rev, store, concurrency=1) + assert rev.calls == 1 + + # Same updated_at, but the PR now carries node_id/created_at (e.g. columns + # added in a later release). Backfill them, but do NOT re-review. + gh.prs[0]["node_id"] = "PR_new" + gh.prs[0]["created_at"] = "2026-06-01T00:00:00Z" + res = run_cycle(gh, rev, store, concurrency=1) + + assert rev.calls == 1 # not re-reviewed + assert res["skipped"] == 1 + row = store.get("o", "r", 1) + assert row["pr_node_id"] == "PR_new" + assert row["pr_created_at"] == "2026-06-01T00:00:00Z" + + def test_precise_gate_skips_when_diff_unchanged(store: ReviewStore) -> None: gh = FakeGH([_pr(1, updated_at="v1")], {1: "diff-1"}) rev = CountingReviewer()