mirror of
https://github.com/Sea-Haven-Industries/pr-reviewer.git
synced 2026-10-06 19:22:15 +00:00
Group PRs by repo in the sidebar and add auto-merge
Group the review queue into a collapsible section per repo (collapse state persisted in localStorage), with PRs ordered oldest to newest by creation date, so a large multi-repo queue is easier to scan. Add an optional per-PR auto-merge control: a method choice (squash by default per handbook, merge, or rebase) enables GitHub auto-merge via a GraphQL mutation, so the PR merges once required checks pass. It only fires when clicked; nothing merges automatically. Back this with created_at and node_id from the PR search, two new nullable store columns added via an idempotent PRAGMA-guarded migration, and a cheap-gate metadata backfill so already-cached PRs gain node_id without being re-reviewed. New endpoint POST /api/automerge.
This commit is contained in:
parent
c07e4397aa
commit
26f0558589
10 changed files with 401 additions and 18 deletions
|
|
@ -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
|
||||
|
||||
|
|
|
|||
|
|
@ -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 {}
|
||||
|
|
|
|||
19
app/main.py
19
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
|
||||
|
||||
|
|
|
|||
24
app/store.py
24
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)
|
||||
|
|
|
|||
|
|
@ -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 = {
|
||||
|
|
|
|||
|
|
@ -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 = `<span class="badge ${it.status}">${STATUS_LABEL[it.status]||it.status}</span>`;
|
||||
div.innerHTML = `
|
||||
<div class="repo">#${it.number}</div>
|
||||
<div class="title">${escapeHtml(it.title||"(no title)")}</div>
|
||||
<div class="sub">
|
||||
<span>@${escapeHtml(it.author||"")}</span>
|
||||
${mention ? '<span class="badge mention">will @mention</span>':''}
|
||||
${statusBadge}
|
||||
</div>`;
|
||||
div.onclick = ()=>selectPR(k);
|
||||
return div;
|
||||
}
|
||||
|
||||
function renderQueue(){
|
||||
const q = document.getElementById("queue");
|
||||
if(!state.items.length){ q.innerHTML = '<div class="empty">No PRs in the queue yet. The worker reviews new PRs automatically.</div>'; 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 = `<span class="badge ${it.status}">${STATUS_LABEL[it.status]||it.status}</span>`;
|
||||
div.innerHTML = `
|
||||
<div class="repo">${it.owner}/${it.repo} #${it.number}</div>
|
||||
<div class="title">${escapeHtml(it.title||"(no title)")}</div>
|
||||
<div class="sub">
|
||||
<span>@${escapeHtml(it.author||"")}</span>
|
||||
${mention ? '<span class="badge mention">will @mention</span>':''}
|
||||
${statusBadge}
|
||||
</div>`;
|
||||
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 = `<span class="chev">${isCollapsed ? "▸" : "▾"}</span>` +
|
||||
`<span class="repo-name">${escapeHtml(repo)}</span>` +
|
||||
`<span class="repo-count">${items.length}</span>`;
|
||||
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){
|
|||
<option value="REQUEST_CHANGES"${rec==="REQUEST_CHANGES"?" selected":""}>REQUEST_CHANGES</option>
|
||||
</select>
|
||||
<button class="primary" id="postBtn">Post review to GitHub</button>
|
||||
</div>
|
||||
|
||||
<label class="small">Auto-merge (merges automatically once required checks pass)</label>
|
||||
<div class="toolbar">
|
||||
<select id="mergeMethod">
|
||||
<option value="SQUASH">Squash</option>
|
||||
<option value="MERGE">Merge</option>
|
||||
<option value="REBASE">Rebase</option>
|
||||
</select>
|
||||
<button id="automergeBtn">Enable auto-merge</button>
|
||||
</div>`;
|
||||
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){
|
||||
|
|
|
|||
|
|
@ -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:
|
||||
|
|
|
|||
|
|
@ -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")
|
||||
|
|
|
|||
|
|
@ -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")
|
||||
|
|
|
|||
|
|
@ -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()
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue