Add actionable merge-gate readiness reporting
swadhinbiswas committed 8 months ago
0a55637
1 parent 6c81bdf
commit 0a55637
4 files changed
+45 -3
Showing 4 changed files+45-3
▼doc/feature_audit.md
+1-1
| @@ -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 and optional `pullNumber` readiness reports); 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`, 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 | | |
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/openapi.ts
+1-1
| @@ -1308,7 +1308,7 @@ export const openApiSpec = { | ||
1308 | 1308 | get: { |
1309 | 1309 | tags: ["CI/CD"], |
1310 | 1310 | summary: "List repository merge gates and required checks", |
1311 | - description: "Returns merge policy config plus a derived report (counts, breakdown, structured warnings with code/severity/message). Optionally evaluates readiness for a specific PR via `pullNumber` query.", | |
1311 | + description: "Returns merge policy config plus a derived report (counts, breakdown, structured warnings with code/severity/message). Optionally evaluates readiness for a specific PR via `pullNumber` query and includes actionable readiness recommendations.", | |
1312 | 1312 | security: [{ bearerAuth: [] }], |
1313 | 1313 | parameters: [ |
1314 | 1314 | { name: "owner", in: "path", required: true, schema: { type: "string" } }, |
1315 | 1315 | |
▼src/pages/api/repos/[owner]/[repo]/merge-gates.ts
+29-0
| @@ -97,6 +97,13 @@ export const GET: APIRoute = withErrorHandler(async ({ params, locals, request } | ||
97 | 97 | } |
98 | 98 | |
99 | 99 | let readiness: Awaited<ReturnType<typeof evaluateGates>> | null = null; |
100 | + let readinessReport: | |
101 | + | { | |
102 | + failedGateCount: number; | |
103 | + failedGateNames: string[]; | |
104 | + recommendations: string[]; | |
105 | + } | |
106 | + | null = null; | |
100 | 107 | if (pullNumberRaw !== null) { |
101 | 108 | const pullNumber = Number.parseInt(pullNumberRaw, 10); |
102 | 109 | if (!Number.isFinite(pullNumber) || pullNumber < 1) { |
| @@ -110,6 +117,27 @@ export const GET: APIRoute = withErrorHandler(async ({ params, locals, request } | ||
110 | 117 | }); |
111 | 118 | if (!pullRequest) return notFound("Pull request not found"); |
112 | 119 | readiness = await evaluateGates(pullRequest.id); |
120 | + const failed = readiness.results.filter((result) => !result.passed); | |
121 | + const recommendations = new Set<string>(); | |
122 | + for (const gate of failed) { | |
123 | + if (gate.gateName.startsWith("Status: ")) { | |
124 | + recommendations.add("Re-run or fix failing required status checks."); | |
125 | + } | |
126 | + if (gate.gateName === "Review" || gate.message.toLowerCase().includes("approval")) { | |
127 | + recommendations.add("Request approvals from required reviewers and resolve outstanding review changes."); | |
128 | + } | |
129 | + if (gate.message.toLowerCase().includes("label")) { | |
130 | + recommendations.add("Add required labels and remove blocked labels before merging."); | |
131 | + } | |
132 | + if (gate.gateName === "Merge Conflicts") { | |
133 | + recommendations.add("Rebase or merge base branch to resolve merge conflicts."); | |
134 | + } | |
135 | + } | |
136 | + readinessReport = { | |
137 | + failedGateCount: failed.length, | |
138 | + failedGateNames: failed.map((gate) => gate.gateName), | |
139 | + recommendations: [...recommendations.values()], | |
140 | + }; | |
113 | 141 | } |
114 | 142 | |
115 | 143 | return success({ |
| @@ -125,6 +153,7 @@ export const GET: APIRoute = withErrorHandler(async ({ params, locals, request } | ||
125 | 153 | warnings, |
126 | 154 | }, |
127 | 155 | readiness, |
156 | + readinessReport, | |
128 | 157 | }); |
129 | 158 | }); |
130 | 159 | |
131 | 160 | |
▼tests/integration/merge-gates-route.test.ts
+14-1
| @@ -117,6 +117,14 @@ describe("merge gates route", () => { | ||
117 | 117 | }); |
118 | 118 | |
119 | 119 | it("returns readiness report when pullNumber is provided", async () => { |
120 | + evaluateGatesMock.mockResolvedValue({ | |
121 | + canMerge: false, | |
122 | + results: [ | |
123 | + { passed: false, gateName: "Status: ci/build", message: "failed" }, | |
124 | + { passed: false, gateName: "Review", message: "Needs 1 more approval(s)" }, | |
125 | + ], | |
126 | + }); | |
127 | + | |
120 | 128 | const response = await mergeGatesGet({ |
121 | 129 | params: { owner: "owner-1", repo: "demo" }, |
122 | 130 | locals: { user: { id: "user-1", isAdmin: false } }, |
| @@ -126,7 +134,12 @@ describe("merge gates route", () => { | ||
126 | 134 | const body = await readJson(response); |
127 | 135 | expect(response.status).toBe(200); |
128 | 136 | expect(evaluateGatesMock).toHaveBeenCalledWith("pr-1"); |
129 | - expect(body?.data?.readiness?.canMerge).toBe(true); | |
137 | + expect(body?.data?.readiness?.canMerge).toBe(false); | |
138 | + expect(body?.data?.readinessReport?.failedGateCount).toBe(2); | |
139 | + expect(body?.data?.readinessReport?.failedGateNames).toEqual(["Status: ci/build", "Review"]); | |
140 | + expect(body?.data?.readinessReport?.recommendations).toContain( | |
141 | + "Re-run or fix failing required status checks." | |
142 | + ); | |
130 | 143 | }); |
131 | 144 | |
132 | 145 | it("validates pullNumber query parameter", async () => { |
133 | 146 | |