swadhin / opencodehub public

Complete merge-gate policy introspection and PR UX

swadhinbiswas committed 8 months ago
df63ee1
1 parent ca0ff7d
commit df63ee1
5 files changed
+184 -8
Showing 5 changed files+184-8
doc/feature_audit.mdsrc/lib/ci-gates.tssrc/pages/[owner]/[repo]/pulls/[number].astrosrc/pages/api/repos/[owner]/[repo]/pulls/[number]/merge-readiness.tstests/integration/pr-merge-readiness-route.test.ts
▼doc/feature_audit.md
+3-3
@@ -11,7 +11,7 @@
11
11
| **Pull Requests** | 9 | 6 | 0 | 15 |
12
12
| **Code Review** | 9 | 1 | 0 | 10 |
13
13
| **Issues & Planning** | 10 | 0 | 0 | 10 |
14
-| **CI/CD & Automation** | 4 | 4 | 0 | 8 |
14
+| **CI/CD & Automation** | 5 | 3 | 0 | 8 |
15
15
| **Third-Party Integrations** | 4 | 18 | 0 | 22 |
16
16
| **Dependency & Impact Awareness** | 1 | 4 | 0 | 5 |
17
17
| **Security** | 9 | 3 | 0 | 12 |
@@ -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** | **75** | **50** | **0** | **125** |
22
+| **Total** | **76** | **49** | **0** | **125** |
23
23
24
24
---
25
25
@@ -94,7 +94,7 @@
94
94
| External CI integration | ⚠️ | Provider-aware integration APIs and normalized status ingestion are implemented, with repo-level integration health/status + token rotate/disable APIs (`external-ci`, `external-ci/checks`); provider-specific setup UX still maturing |
95
95
| Self-hosted runners | ✅ | `runner/` with Docker executor |
96
96
| Secrets management | ✅ | Schema + UI for repo/org secrets |
97
-| Merge checks & gates | ⚠️ | Required status checks + external CI gate readiness are enforced, with explicit PR gate introspection via `pulls/{number}/merge-readiness`, active label/review custom gate evaluation in `ci-gates.ts`, and repository-level gate policy management/reporting APIs (`/repos/{owner}/{repo}/merge-gates`, `/repos/{owner}/{repo}/merge-gates/{id}` including structured policy warnings/breakdowns plus actionable `pullNumber` readiness recommendations); broader policy UX remains |
97
+| Merge checks & gates | ✅ | Required status checks + external CI gate readiness are enforced, with explicit PR gate introspection via `pulls/{number}/merge-readiness` now including policy reports (`failedByType`, recommendations, pass/fail counts), active label/review/custom gate evaluation in `ci-gates.ts` with gate-type metadata, repository-level gate policy management/reporting APIs (`/repos/{owner}/{repo}/merge-gates`, `/repos/{owner}/{repo}/merge-gates/{id}`), and PR detail merge-gate policy UX for live blockers/breakdowns/remediation guidance |
98
98
| Automation rules engine | ⚠️ | Conditions/actions active with retry + dead-letter audit logging; advanced orchestration still maturing |
99
99
| Webhooks | ✅ | Full implementation with UI |
100
100
| Workflow templates | ⚠️ | Template library plus repo-level listing/apply APIs are now implemented (`workflow/templates`); broader UX/workflow adoption still maturing |
101
101
▼src/lib/ci-gates.ts
+28-2
@@ -150,18 +150,21 @@ export async function evaluateGates(prId: string): Promise<{
150
150
passed: false,
151
151
gateName: `Status: ${required.checkName}`,
152
152
message: "Check not found",
153
+ details: { gateType: "status_check", branch: required.branch, strictMode: required.strictMode },
153
154
});
154
155
} else if (check.conclusion !== "success") {
155
156
results.push({
156
157
passed: false,
157
158
gateName: `Status: ${required.checkName}`,
158
159
message: `Check ${check.status}: ${check.conclusion || "pending"}`,
160
+ details: { gateType: "status_check", branch: required.branch, strictMode: required.strictMode },
159
161
});
160
162
} else {
161
163
results.push({
162
164
passed: true,
163
165
gateName: `Status: ${required.checkName}`,
164
166
message: "Check passed",
167
+ details: { gateType: "status_check", branch: required.branch, strictMode: required.strictMode },
165
168
});
166
169
}
167
170
}
@@ -175,18 +178,21 @@ export async function evaluateGates(prId: string): Promise<{
175
178
passed: false,
176
179
gateName: "Review",
177
180
message: "At least one approval required",
181
+ details: { gateType: "review" },
178
182
});
179
183
} else if (changesRequested) {
180
184
results.push({
181
185
passed: false,
182
186
gateName: "Review",
183
187
message: "Changes requested by reviewer",
188
+ details: { gateType: "review" },
184
189
});
185
190
} else {
186
191
results.push({
187
192
passed: true,
188
193
gateName: "Review",
189
194
message: `${approvals.length} approval(s)`,
195
+ details: { gateType: "review", approvals: approvals.length },
190
196
});
191
197
}
192
198
@@ -206,12 +212,14 @@ export async function evaluateGates(prId: string): Promise<{
206
212
passed: false,
207
213
gateName: "Merge Conflicts",
208
214
message: "Branch has conflicts that must be resolved",
215
+ details: { gateType: "conflict" },
209
216
});
210
217
} else {
211
218
results.push({
212
219
passed: true,
213
220
gateName: "Merge Conflicts",
214
221
message: "No conflicts",
222
+ details: { gateType: "conflict" },
215
223
});
216
224
}
217
225
@@ -264,6 +272,7 @@ async function evaluateSingleGate(
264
272
passed: false,
265
273
gateName: gate.name,
266
274
message: `Missing required labels: ${missingRequired.join(", ")}`,
275
+ details: { gateType: "label", missingRequired, blockedLabels: blockedLabels },
267
276
};
268
277
}
269
278
if (presentBlocked.length > 0) {
@@ -271,12 +280,14 @@ async function evaluateSingleGate(
271
280
passed: false,
272
281
gateName: gate.name,
273
282
message: `Blocked labels present: ${presentBlocked.join(", ")}`,
283
+ details: { gateType: "label", presentBlocked, requiredLabels },
274
284
};
275
285
}
276
286
return {
277
287
passed: true,
278
288
gateName: gate.name,
279
289
message: "Label check passed",
290
+ details: { gateType: "label", requiredLabels, blockedLabels },
280
291
};
281
292
}
282
293
@@ -303,6 +314,7 @@ async function evaluateSingleGate(
303
314
passed: false,
304
315
gateName: gate.name,
305
316
message: "Changes requested by reviewer",
317
+ details: { gateType: "review", minReviews, approvalCount },
306
318
};
307
319
}
308
320
if (approvalCount < minReviews) {
@@ -310,12 +322,14 @@ async function evaluateSingleGate(
310
322
passed: false,
311
323
gateName: gate.name,
312
324
message: `Needs ${minReviews - approvalCount} more approval(s)`,
325
+ details: { gateType: "review", minReviews, approvalCount },
313
326
};
314
327
}
315
328
return {
316
329
passed: true,
317
330
gateName: gate.name,
318
331
message: `Review requirements met (${approvalCount}/${minReviews})`,
332
+ details: { gateType: "review", minReviews, approvalCount },
319
333
};
320
334
}
321
335
@@ -330,20 +344,32 @@ async function evaluateSingleGate(
330
344
passed: Boolean(passed),
331
345
gateName: gate.name,
332
346
message: passed ? "Custom gate passed" : "Custom gate failed",
347
+ details: { gateType: "custom" },
333
348
};
334
349
} catch (error) {
335
350
return {
336
351
passed: false,
337
352
gateName: gate.name,
338
353
message: "Error evaluating gate",
354
+ details: { gateType: "custom" },
339
355
};
340
356
}
341
357
}
342
- return { passed: true, gateName: gate.name, message: "No condition defined" };
358
+ return {
359
+ passed: true,
360
+ gateName: gate.name,
361
+ message: "No condition defined",
362
+ details: { gateType: "custom" },
363
+ };
343
364
}
344
365
345
366
default:
346
- return { passed: true, gateName: gate.name, message: "Unknown gate type" };
367
+ return {
368
+ passed: true,
369
+ gateName: gate.name,
370
+ message: "Unknown gate type",
371
+ details: { gateType: gate.gateType || "unknown" },
372
+ };
347
373
}
348
374
}
349
375
350
376
▼src/pages/[owner]/[repo]/pulls/[number].astro
+79-0
@@ -452,6 +452,22 @@ try {
452
452
<div id="codeowner-blockers" class="space-y-1 text-xs text-amber-500"></div>
453
453
<div id="codeowner-files" class="space-y-1 text-xs text-muted-foreground"></div>
454
454
</div>
455
+
456
+ <div class="mt-4 rounded-md border border-dashed p-3 space-y-2">
457
+ <div class="flex items-center justify-between">
458
+ <div class="text-xs font-medium text-muted-foreground">Merge gate policy</div>
459
+ <button
460
+ id="merge-readiness-refresh"
461
+ class="h-8 rounded-md border px-3 text-xs text-muted-foreground"
462
+ >
463
+ Refresh
464
+ </button>
465
+ </div>
466
+ <div id="merge-readiness-summary" class="text-xs text-muted-foreground"></div>
467
+ <div id="merge-readiness-breakdown" class="text-xs text-muted-foreground"></div>
468
+ <div id="merge-readiness-blockers" class="space-y-1 text-xs text-amber-500"></div>
469
+ <div id="merge-readiness-recommendations" class="space-y-1 text-xs text-muted-foreground"></div>
470
+ </div>
455
471
</div>
456
472
</div>
457
473
</div>
@@ -813,6 +829,11 @@ try {
813
829
const codeOwnerPolicy = document.getElementById("codeowner-policy");
814
830
const codeOwnerBlockers = document.getElementById("codeowner-blockers");
815
831
const codeOwnerFiles = document.getElementById("codeowner-files");
832
+ const mergeReadinessRefresh = document.getElementById("merge-readiness-refresh");
833
+ const mergeReadinessSummary = document.getElementById("merge-readiness-summary");
834
+ const mergeReadinessBreakdown = document.getElementById("merge-readiness-breakdown");
835
+ const mergeReadinessBlockers = document.getElementById("merge-readiness-blockers");
836
+ const mergeReadinessRecommendations = document.getElementById("merge-readiness-recommendations");
816
837
let stackApprovalRecommendations = [];
817
838
818
839
// Keyboard Shortcuts
@@ -1237,6 +1258,63 @@ try {
1237
1258
loadCodeOwnerEnforcement();
1238
1259
});
1239
1260
1261
+ async function loadMergeReadiness() {
1262
+ if (!mergeReadinessSummary || !mergeReadinessBreakdown) return;
1263
+ mergeReadinessSummary.textContent = "Loading merge readiness...";
1264
+ mergeReadinessBreakdown.textContent = "";
1265
+ if (mergeReadinessBlockers) mergeReadinessBlockers.innerHTML = "";
1266
+ if (mergeReadinessRecommendations) mergeReadinessRecommendations.innerHTML = "";
1267
+
1268
+ try {
1269
+ const res = await fetch(
1270
+ `/api/repos/${repoOwner}/${repoName}/pulls/${prNumber}/merge-readiness`,
1271
+ );
1272
+ const payload = await res.json();
1273
+ if (!res.ok) {
1274
+ throw new Error(payload?.error?.message || payload?.message || "Failed to load merge readiness");
1275
+ }
1276
+
1277
+ const data = payload?.data || payload;
1278
+ const report = data?.policyReport || {};
1279
+ const canMerge = !!data?.canMerge;
1280
+ const blockers = data?.blockers || [];
1281
+ const failedByType = report?.failedByType || {};
1282
+ const recommendations = report?.recommendations || [];
1283
+
1284
+ mergeReadinessSummary.textContent = canMerge
1285
+ ? "All merge gates are currently satisfied."
1286
+ : "Merge is currently blocked by one or more gates.";
1287
+ mergeReadinessBreakdown.textContent =
1288
+ `Gates: passed ${report?.passedGates || 0}/${report?.totalGates || 0} • failed ${report?.failedGates || 0}`;
1289
+
1290
+ if (mergeReadinessBlockers) {
1291
+ mergeReadinessBlockers.innerHTML = blockers.map((blocker) => `<div>• ${blocker}</div>`).join("");
1292
+ }
1293
+
1294
+ if (mergeReadinessRecommendations) {
1295
+ const typeParts = Object.entries(failedByType).map(([key, count]) => `${key}: ${count}`);
1296
+ const sections = [];
1297
+ if (typeParts.length > 0) {
1298
+ sections.push(`<div>Failed gate types: ${typeParts.join(", ")}</div>`);
1299
+ }
1300
+ if (recommendations.length > 0) {
1301
+ sections.push(...recommendations.map((item) => `<div>• ${item}</div>`));
1302
+ }
1303
+ mergeReadinessRecommendations.innerHTML = sections.join("");
1304
+ }
1305
+ } catch (error) {
1306
+ mergeReadinessSummary.textContent = error.message || "Failed to load merge readiness";
1307
+ mergeReadinessBreakdown.textContent = "";
1308
+ if (mergeReadinessBlockers) mergeReadinessBlockers.innerHTML = "";
1309
+ if (mergeReadinessRecommendations) mergeReadinessRecommendations.innerHTML = "";
1310
+ }
1311
+ }
1312
+
1313
+ mergeReadinessRefresh?.addEventListener("click", async (event) => {
1314
+ event.preventDefault();
1315
+ loadMergeReadiness();
1316
+ });
1317
+
1240
1318
aiReviewBtn?.addEventListener("click", async () => {
1241
1319
aiReviewBtn.setAttribute("disabled", "true");
1242
1320
const originalText = aiReviewBtn.innerHTML;
@@ -1677,6 +1755,7 @@ try {
1677
1755
loadAutoMergeStatus();
1678
1756
loadPrChecks();
1679
1757
loadCodeOwnerEnforcement();
1758
+ loadMergeReadiness();
1680
1759
1681
1760
mergeBtn?.addEventListener("click", async () => {
1682
1761
if (!confirm("Are you sure you want to merge this pull request?")) return;
1683
1762
▼src/pages/api/repos/[owner]/[repo]/pulls/[number]/merge-readiness.ts
+65-3
@@ -7,6 +7,43 @@ import { withErrorHandler } from "@/lib/errors";
7
7
import { canReadRepo } from "@/lib/permissions";
8
8
import { evaluateGates } from "@/lib/ci-gates";
9
9
10
+function inferGateType(gate: { gateName: string; details?: Record<string, unknown> }) {
11
+ const detailsType = typeof gate?.details?.gateType === "string" ? gate.details.gateType : null;
12
+ if (detailsType) return detailsType;
13
+ if (gate.gateName.startsWith("Status:")) return "status_check";
14
+ if (gate.gateName === "Review") return "review";
15
+ if (gate.gateName === "Merge Conflicts") return "conflict";
16
+ return "custom";
17
+}
18
+
19
+function buildRecommendations(
20
+ failed: Array<{ gateName: string; message: string; details?: Record<string, unknown> }>
21
+) {
22
+ const recommendations = new Set<string>();
23
+ for (const gate of failed) {
24
+ const gateType = inferGateType(gate);
25
+ const message = gate.message.toLowerCase();
26
+
27
+ if (gateType === "status_check") {
28
+ recommendations.add("Re-run or fix failing required status checks.");
29
+ }
30
+ if (gateType === "review" || message.includes("approval")) {
31
+ recommendations.add("Request required approvals and resolve review feedback.");
32
+ }
33
+ if (gateType === "label" || message.includes("label")) {
34
+ recommendations.add("Apply required labels and remove blocked labels.");
35
+ }
36
+ if (gateType === "conflict") {
37
+ recommendations.add("Rebase or merge the base branch to resolve conflicts.");
38
+ }
39
+ if (gateType === "custom") {
40
+ recommendations.add("Review custom merge gate configuration and conditions.");
41
+ }
42
+ }
43
+
44
+ return [...recommendations.values()];
45
+}
46
+
10
47
export const GET: APIRoute = withErrorHandler(async ({ params, locals }) => {
11
48
const owner = params.owner;
12
49
const repoName = params.repo;
@@ -59,13 +96,31 @@ export const GET: APIRoute = withErrorHandler(async ({ params, locals }) => {
59
96
gateResults: [],
60
97
mergeable: pr.mergeable,
61
98
mergeableState: pr.mergeableState,
99
+ policyReport: {
100
+ totalGates: 0,
101
+ failedGates: 0,
102
+ passedGates: 0,
103
+ failedByType: {},
104
+ recommendations: ["Re-open the pull request before attempting to merge."],
105
+ },
62
106
});
63
107
}
64
108
65
109
const gateResult = await evaluateGates(pr.id);
66
- const blockers = gateResult.results
67
- .filter((result) => !result.passed)
68
- .map((result) => result.message);
110
+ const failed = gateResult.results.filter((result) => !result.passed);
111
+ const blockers = failed.map((result) => result.message);
112
+ const failedByType = failed.reduce<Record<string, number>>((acc, gate) => {
113
+ const gateType = inferGateType(gate as any);
114
+ acc[gateType] = (acc[gateType] || 0) + 1;
115
+ return acc;
116
+ }, {});
117
+ const recommendations = buildRecommendations(
118
+ failed.map((gate) => ({
119
+ gateName: gate.gateName,
120
+ message: gate.message,
121
+ details: gate.details,
122
+ }))
123
+ );
69
124
70
125
return success({
71
126
canMerge: gateResult.canMerge,
@@ -73,5 +128,12 @@ export const GET: APIRoute = withErrorHandler(async ({ params, locals }) => {
73
128
gateResults: gateResult.results,
74
129
mergeable: pr.mergeable,
75
130
mergeableState: pr.mergeableState,
131
+ policyReport: {
132
+ totalGates: gateResult.results.length,
133
+ failedGates: failed.length,
134
+ passedGates: gateResult.results.length - failed.length,
135
+ failedByType,
136
+ recommendations,
137
+ },
76
138
});
77
139
});
78
140
▼tests/integration/pr-merge-readiness-route.test.ts
+9-0
@@ -87,6 +87,11 @@ describe("pull request merge-readiness route", () => {
87
87
expect(response.status).toBe(200);
88
88
expect(body?.data?.canMerge).toBe(false);
89
89
expect(body?.data?.blockers).toContain("At least one approval required");
90
+ expect(body?.data?.policyReport?.failedGates).toBe(1);
91
+ expect(body?.data?.policyReport?.failedByType?.review).toBe(1);
92
+ expect(body?.data?.policyReport?.recommendations).toContain(
93
+ "Request required approvals and resolve review feedback."
94
+ );
90
95
expect(evaluateGatesMock).toHaveBeenCalledWith("pr-1");
91
96
});
92
97
@@ -102,5 +107,9 @@ describe("pull request merge-readiness route", () => {
102
107
expect(response.status).toBe(200);
103
108
expect(body?.data?.canMerge).toBe(false);
104
109
expect(body?.data?.blockers).toEqual(["Pull request is not open"]);
110
+ expect(body?.data?.policyReport?.failedGates).toBe(0);
111
+ expect(body?.data?.policyReport?.recommendations).toContain(
112
+ "Re-open the pull request before attempting to merge."
113
+ );
105
114
});
106
115
});
107
116