Complete auto-merge rule introspection and PR policy UI
swadhinbiswas committed 8 months ago
38b7cb9
1 parent 73b1fe0
commit 38b7cb9
4 files changed
+227 -8
Showing 4 changed files+227-8
▼doc/feature_audit.md
+3-3
| @@ -8,7 +8,7 @@ | ||
8 | 8 | | Category | Implemented | Partial | Missing | Total | |
9 | 9 | | :--- | :---: | :---: | :---: | :---: | |
10 | 10 | | **Repository & Git** | 9 | 4 | 0 | 13 | |
11 | -| **Pull Requests** | 7 | 8 | 0 | 15 | | |
11 | +| **Pull Requests** | 8 | 7 | 0 | 15 | | |
12 | 12 | | **Code Review** | 8 | 2 | 0 | 10 | |
13 | 13 | | **Issues & Planning** | 10 | 0 | 0 | 10 | |
14 | 14 | | **CI/CD & Automation** | 4 | 4 | 0 | 8 | |
| @@ -19,7 +19,7 @@ | ||
19 | 19 | | **Notifications & Collaboration** | 5 | 3 | 0 | 8 | |
20 | 20 | | **Interfaces & Extensibility** | 5 | 2 | 0 | 7 | |
21 | 21 | | **Self-Hosted & Deployment** | 4 | 3 | 0 | 7 | |
22 | -| **Total** | **72** | **53** | **0** | **125** | | |
22 | +| **Total** | **73** | **52** | **0** | **125** | | |
23 | 23 | |
24 | 24 | --- |
25 | 25 | |
| @@ -53,7 +53,7 @@ | ||
53 | 53 | | Required reviewers per state | ✅ | State reviewer requirements are enforced during transitions, and required-reviewer introspection now supports state-policy preview mode (`pulls/{number}/required-reviewers?stateId=`) including team/user requirement progress; PR state UI surfaces live per-state reviewer policy guidance before applying transitions | |
54 | 54 | | PR merge queues | ✅ | `merge-queue.ts` (590+ lines) | |
55 | 55 | | Conflict detection before merge | ✅ | `mergeable` field in PR schema | |
56 | -| Auto-merge rules | ⚠️ | Auto-merge rules engine plus explicit PR auto-merge control/status APIs now implemented (`pulls/{number}/auto-merge`); richer policy UX and rule introspection remain partial | | |
56 | +| Auto-merge rules | ✅ | Auto-merge rules engine plus explicit PR auto-merge control/status APIs are fully wired (`pulls/{number}/auto-merge`), including policy summary and per-rule introspection in status responses (`policySummary`, `ruleEvaluations`) and PR detail UI controls for enable/disable, method selection, blockers, and live rule evaluation visibility | | |
57 | 57 | | Draft PRs | ✅ | Schema supports `isDraft` field | |
58 | 58 | | PR labels | ✅ | Full implementation with schema | |
59 | 59 | | PR assignees | ✅ | Full implementation with schema | |
60 | 60 | |
▼src/lib/auto-merge.ts
+25-5
| @@ -4,10 +4,13 @@ | ||
4 | 4 | */ |
5 | 5 | |
6 | 6 | import { getDatabase, schema } from "@/db"; |
7 | -import { eq, and } from "drizzle-orm"; | |
7 | +import { eq } from "drizzle-orm"; | |
8 | 8 | import { logger } from "./logger"; |
9 | 9 | import { addToMergeQueue } from "./merge-queue"; |
10 | -import { evaluateAutoMergeRules } from "./auto-merge-rules"; | |
10 | +import { | |
11 | + evaluateAutoMergeRulesDetailed, | |
12 | + type AutoMergeRuleEvaluation, | |
13 | +} from "./auto-merge-rules"; | |
11 | 14 | |
12 | 15 | export interface AutoMergeOptions { |
13 | 16 | mergeMethod: "merge" | "squash" | "rebase"; |
| @@ -22,6 +25,14 @@ export interface AutoMergeStatus { | ||
22 | 25 | blockers: string[]; |
23 | 26 | enabledBy?: string; |
24 | 27 | enabledAt?: Date; |
28 | + policySummary?: { | |
29 | + totalRules: number; | |
30 | + matchedRules: number; | |
31 | + passedRules: number; | |
32 | + failedRules: number; | |
33 | + unmatchedRules: number; | |
34 | + }; | |
35 | + ruleEvaluations?: AutoMergeRuleEvaluation[]; | |
25 | 36 | } |
26 | 37 | |
27 | 38 | /** |
| @@ -155,15 +166,14 @@ export async function checkAutoMergeEligibility(prId: string): Promise<AutoMerge | ||
155 | 166 | |
156 | 167 | // Check reviews |
157 | 168 | const reviews = pr.reviews || []; |
158 | - const approvals = reviews.filter(r => r.state === "approved"); | |
159 | 169 | const changesRequested = reviews.filter(r => r.state === "changes_requested"); |
160 | 170 | |
161 | 171 | if (changesRequested.length > 0) { |
162 | 172 | blockers.push("Changes requested by reviewer(s)"); |
163 | 173 | } |
164 | 174 | |
165 | - const ruleBlockers = await evaluateAutoMergeRules(prId); | |
166 | - blockers.push(...ruleBlockers); | |
175 | + const ruleEvaluation = await evaluateAutoMergeRulesDetailed(prId); | |
176 | + blockers.push(...ruleEvaluation.blockers); | |
167 | 177 | |
168 | 178 | // Get enabler info |
169 | 179 | let enabledBy: string | undefined; |
| @@ -181,6 +191,16 @@ export async function checkAutoMergeEligibility(prId: string): Promise<AutoMerge | ||
181 | 191 | blockers, |
182 | 192 | enabledBy, |
183 | 193 | enabledAt: pr.autoMergeEnabledAt || undefined, |
194 | + policySummary: { | |
195 | + totalRules: ruleEvaluation.evaluations.length, | |
196 | + matchedRules: ruleEvaluation.evaluations.filter((evaluation) => evaluation.matched).length, | |
197 | + passedRules: ruleEvaluation.evaluations.filter((evaluation) => evaluation.passed).length, | |
198 | + failedRules: ruleEvaluation.evaluations.filter( | |
199 | + (evaluation) => evaluation.matched && !evaluation.passed | |
200 | + ).length, | |
201 | + unmatchedRules: ruleEvaluation.evaluations.filter((evaluation) => !evaluation.matched).length, | |
202 | + }, | |
203 | + ruleEvaluations: ruleEvaluation.evaluations, | |
184 | 204 | }; |
185 | 205 | } |
186 | 206 | |
187 | 207 | |
▼src/pages/[owner]/[repo]/pulls/[number].astro
+165-0
| @@ -388,6 +388,40 @@ try { | ||
388 | 388 | </a> |
389 | 389 | </div> |
390 | 390 | )} |
391 | + | |
392 | + {canWrite && ( | |
393 | + <div class="mt-4 rounded-md border border-dashed p-3 space-y-2"> | |
394 | + <div class="flex items-center justify-between gap-2"> | |
395 | + <div class="text-xs font-medium text-muted-foreground">Auto-merge</div> | |
396 | + <div class="flex items-center gap-2"> | |
397 | + <select | |
398 | + id="auto-merge-method" | |
399 | + class="h-8 rounded-md border bg-background px-2 text-xs" | |
400 | + > | |
401 | + <option value="merge">Merge</option> | |
402 | + <option value="squash">Squash</option> | |
403 | + <option value="rebase">Rebase</option> | |
404 | + </select> | |
405 | + <button | |
406 | + id="auto-merge-toggle" | |
407 | + class="h-8 rounded-md border border-primary/30 px-3 text-xs text-primary" | |
408 | + > | |
409 | + Enable auto-merge | |
410 | + </button> | |
411 | + <button | |
412 | + id="auto-merge-refresh" | |
413 | + class="h-8 rounded-md border px-3 text-xs text-muted-foreground" | |
414 | + > | |
415 | + Refresh | |
416 | + </button> | |
417 | + </div> | |
418 | + </div> | |
419 | + <div id="auto-merge-status" class="text-xs text-muted-foreground"></div> | |
420 | + <div id="auto-merge-summary" class="text-xs text-muted-foreground"></div> | |
421 | + <div id="auto-merge-blockers" class="space-y-1 text-xs text-amber-500"></div> | |
422 | + <div id="auto-merge-rules" class="space-y-1 text-xs text-muted-foreground"></div> | |
423 | + </div> | |
424 | + )} | |
391 | 425 | </div> |
392 | 426 | </div> |
393 | 427 | </div> |
| @@ -734,6 +768,13 @@ try { | ||
734 | 768 | const prStateHint = document.getElementById("pr-state-hint"); |
735 | 769 | const prStateReviewers = document.getElementById("pr-state-reviewers"); |
736 | 770 | const prStateMsg = document.getElementById("pr-state-msg"); |
771 | + const autoMergeMethod = document.getElementById("auto-merge-method"); | |
772 | + const autoMergeToggle = document.getElementById("auto-merge-toggle"); | |
773 | + const autoMergeRefresh = document.getElementById("auto-merge-refresh"); | |
774 | + const autoMergeStatus = document.getElementById("auto-merge-status"); | |
775 | + const autoMergeSummary = document.getElementById("auto-merge-summary"); | |
776 | + const autoMergeBlockers = document.getElementById("auto-merge-blockers"); | |
777 | + const autoMergeRules = document.getElementById("auto-merge-rules"); | |
737 | 778 | let stackApprovalRecommendations = []; |
738 | 779 | |
739 | 780 | // Keyboard Shortcuts |
| @@ -896,6 +937,129 @@ try { | ||
896 | 937 | } |
897 | 938 | }); |
898 | 939 | |
940 | + function setAutoMergeControlsBusy(busy) { | |
941 | + if (autoMergeToggle) { | |
942 | + if (busy) autoMergeToggle.setAttribute("disabled", "true"); | |
943 | + else autoMergeToggle.removeAttribute("disabled"); | |
944 | + } | |
945 | + if (autoMergeRefresh) { | |
946 | + if (busy) autoMergeRefresh.setAttribute("disabled", "true"); | |
947 | + else autoMergeRefresh.removeAttribute("disabled"); | |
948 | + } | |
949 | + if (autoMergeMethod) { | |
950 | + if (busy) autoMergeMethod.setAttribute("disabled", "true"); | |
951 | + else autoMergeMethod.removeAttribute("disabled"); | |
952 | + } | |
953 | + } | |
954 | + | |
955 | + function renderAutoMergeStatus(status) { | |
956 | + if (!autoMergeToggle || !autoMergeStatus || !autoMergeSummary) return; | |
957 | + | |
958 | + const enabled = !!status?.enabled; | |
959 | + const eligible = !!status?.eligibleToMerge; | |
960 | + const blockers = status?.blockers || []; | |
961 | + const summary = status?.policySummary; | |
962 | + const evaluations = status?.ruleEvaluations || []; | |
963 | + | |
964 | + autoMergeToggle.textContent = enabled ? "Disable auto-merge" : "Enable auto-merge"; | |
965 | + autoMergeStatus.textContent = enabled | |
966 | + ? eligible | |
967 | + ? "Auto-merge is enabled and currently merge-ready." | |
968 | + : "Auto-merge is enabled but waiting for merge policy." | |
969 | + : "Auto-merge is disabled."; | |
970 | + | |
971 | + if (status?.enabledBy || status?.enabledAt) { | |
972 | + const enabledBy = status?.enabledBy ? `by ${status.enabledBy}` : "by unknown user"; | |
973 | + const enabledAt = status?.enabledAt ? ` at ${new Date(status.enabledAt).toLocaleString()}` : ""; | |
974 | + autoMergeStatus.textContent += ` Enabled ${enabledBy}${enabledAt}.`; | |
975 | + } | |
976 | + | |
977 | + if (!summary || summary.totalRules === 0) { | |
978 | + autoMergeSummary.textContent = "No repository auto-merge rules configured."; | |
979 | + } else { | |
980 | + autoMergeSummary.textContent = `Rules: ${summary.passedRules}/${summary.totalRules} passing • failed ${summary.failedRules} • unmatched ${summary.unmatchedRules}`; | |
981 | + } | |
982 | + | |
983 | + if (autoMergeBlockers) { | |
984 | + autoMergeBlockers.innerHTML = blockers.map((blocker) => `<div>• ${blocker}</div>`).join(""); | |
985 | + } | |
986 | + | |
987 | + if (autoMergeRules) { | |
988 | + autoMergeRules.innerHTML = evaluations.map((rule) => { | |
989 | + const state = !rule.matched ? "not matched" : rule.passed ? "passing" : "failing"; | |
990 | + const reasons = (rule.reasons || []).length > 0 | |
991 | + ? ` (${rule.reasons.join(" | ")})` | |
992 | + : ""; | |
993 | + return `<div>• ${rule.name}: ${state}${reasons}</div>`; | |
994 | + }).join(""); | |
995 | + } | |
996 | + } | |
997 | + | |
998 | + async function loadAutoMergeStatus() { | |
999 | + if (!autoMergeStatus) return; | |
1000 | + autoMergeStatus.textContent = "Loading auto-merge status..."; | |
1001 | + | |
1002 | + try { | |
1003 | + const res = await fetch( | |
1004 | + `/api/repos/${repoOwner}/${repoName}/pulls/${prNumber}/auto-merge`, | |
1005 | + ); | |
1006 | + const payload = await res.json(); | |
1007 | + if (!res.ok) { | |
1008 | + throw new Error(payload?.error?.message || payload?.message || "Failed to load auto-merge status"); | |
1009 | + } | |
1010 | + | |
1011 | + renderAutoMergeStatus(payload?.data || payload); | |
1012 | + } catch (error) { | |
1013 | + autoMergeStatus.textContent = error.message || "Failed to load auto-merge status"; | |
1014 | + if (autoMergeSummary) autoMergeSummary.textContent = ""; | |
1015 | + if (autoMergeBlockers) autoMergeBlockers.innerHTML = ""; | |
1016 | + if (autoMergeRules) autoMergeRules.innerHTML = ""; | |
1017 | + } | |
1018 | + } | |
1019 | + | |
1020 | + autoMergeRefresh?.addEventListener("click", async (event) => { | |
1021 | + event.preventDefault(); | |
1022 | + loadAutoMergeStatus(); | |
1023 | + }); | |
1024 | + | |
1025 | + autoMergeToggle?.addEventListener("click", async (event) => { | |
1026 | + event.preventDefault(); | |
1027 | + if (!autoMergeToggle) return; | |
1028 | + | |
1029 | + const disabling = autoMergeToggle.textContent?.toLowerCase().includes("disable"); | |
1030 | + setAutoMergeControlsBusy(true); | |
1031 | + if (autoMergeStatus) { | |
1032 | + autoMergeStatus.textContent = disabling | |
1033 | + ? "Disabling auto-merge..." | |
1034 | + : "Enabling auto-merge..."; | |
1035 | + } | |
1036 | + | |
1037 | + try { | |
1038 | + const res = await fetch( | |
1039 | + `/api/repos/${repoOwner}/${repoName}/pulls/${prNumber}/auto-merge`, | |
1040 | + disabling | |
1041 | + ? { method: "DELETE" } | |
1042 | + : { | |
1043 | + method: "POST", | |
1044 | + headers: { "Content-Type": "application/json" }, | |
1045 | + body: JSON.stringify({ mergeMethod: autoMergeMethod?.value || "merge" }), | |
1046 | + }, | |
1047 | + ); | |
1048 | + const payload = await res.json(); | |
1049 | + if (!res.ok) { | |
1050 | + throw new Error(payload?.error?.message || payload?.message || "Failed to update auto-merge"); | |
1051 | + } | |
1052 | + | |
1053 | + renderAutoMergeStatus(payload?.data || payload); | |
1054 | + } catch (error) { | |
1055 | + if (autoMergeStatus) { | |
1056 | + autoMergeStatus.textContent = error.message || "Failed to update auto-merge"; | |
1057 | + } | |
1058 | + } finally { | |
1059 | + setAutoMergeControlsBusy(false); | |
1060 | + } | |
1061 | + }); | |
1062 | + | |
899 | 1063 | aiReviewBtn?.addEventListener("click", async () => { |
900 | 1064 | aiReviewBtn.setAttribute("disabled", "true"); |
901 | 1065 | const originalText = aiReviewBtn.innerHTML; |
| @@ -1333,6 +1497,7 @@ try { | ||
1333 | 1497 | refreshStackMaintenanceStatus(); |
1334 | 1498 | refreshStackMergeReadiness(); |
1335 | 1499 | } |
1500 | + loadAutoMergeStatus(); | |
1336 | 1501 | |
1337 | 1502 | mergeBtn?.addEventListener("click", async () => { |
1338 | 1503 | if (!confirm("Are you sure you want to merge this pull request?")) return; |
1339 | 1504 | |
▼tests/integration/pr-auto-merge-route.test.ts
+34-0
| @@ -22,6 +22,22 @@ const { | ||
22 | 22 | enabled: true, |
23 | 23 | eligibleToMerge: false, |
24 | 24 | blockers: ["1 pending check(s)"], |
25 | + policySummary: { | |
26 | + totalRules: 1, | |
27 | + matchedRules: 1, | |
28 | + passedRules: 0, | |
29 | + failedRules: 1, | |
30 | + unmatchedRules: 0, | |
31 | + }, | |
32 | + ruleEvaluations: [ | |
33 | + { | |
34 | + ruleId: "rule-1", | |
35 | + name: "Main branch policy", | |
36 | + matched: true, | |
37 | + passed: false, | |
38 | + reasons: ["Rule \"Main branch policy\" has pending checks"], | |
39 | + }, | |
40 | + ], | |
25 | 41 | })), |
26 | 42 | enableAutoMergeMock: vi.fn(async () => ({ success: true })), |
27 | 43 | disableAutoMergeMock: vi.fn(async () => true), |
| @@ -104,6 +120,22 @@ describe("pull request auto-merge route", () => { | ||
104 | 120 | enabled: true, |
105 | 121 | eligibleToMerge: false, |
106 | 122 | blockers: ["1 pending check(s)"], |
123 | + policySummary: { | |
124 | + totalRules: 1, | |
125 | + matchedRules: 1, | |
126 | + passedRules: 0, | |
127 | + failedRules: 1, | |
128 | + unmatchedRules: 0, | |
129 | + }, | |
130 | + ruleEvaluations: [ | |
131 | + { | |
132 | + ruleId: "rule-1", | |
133 | + name: "Main branch policy", | |
134 | + matched: true, | |
135 | + passed: false, | |
136 | + reasons: ["Rule \"Main branch policy\" has pending checks"], | |
137 | + }, | |
138 | + ], | |
107 | 139 | }); |
108 | 140 | compareBranchesMock.mockResolvedValue({ |
109 | 141 | diffs: [{ file: "src/app.ts", additions: 1, deletions: 0 }], |
| @@ -122,6 +154,8 @@ describe("pull request auto-merge route", () => { | ||
122 | 154 | const body = await readJson(response); |
123 | 155 | expect(response.status).toBe(200); |
124 | 156 | expect(body?.data?.enabled).toBe(true); |
157 | + expect(body?.data?.policySummary?.totalRules).toBe(1); | |
158 | + expect(body?.data?.ruleEvaluations?.[0]?.name).toBe("Main branch policy"); | |
125 | 159 | expect(getAutoMergeStatusMock).toHaveBeenCalledWith("pr-1"); |
126 | 160 | }); |
127 | 161 | |
128 | 162 | |