Skip to content

Commit a9cafc4

Browse files
WibiasWibias
committed
feat(ci): add pure PR ancestry and description quality checks
1 parent 3da6f80 commit a9cafc4

3 files changed

Lines changed: 278 additions & 0 deletions

File tree

‎.github/scripts/pr-quality.cjs‎

Lines changed: 114 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,114 @@
1+
"use strict";
2+
3+
const path = require("node:path");
4+
const {
5+
clean,
6+
isPlaceholderOnlyValue,
7+
hasSubstantialStructuredContent,
8+
} = require(path.join(__dirname, "issue-quality.cjs"));
9+
10+
const ANCESTRY_BEHIND_THRESHOLD = 20;
11+
const MIN_SECTION_LEN = 40;
12+
const MIN_RICH_SECTIONS = 2;
13+
const UNSTRUCTURED_MIN_LEN = 120;
14+
const UNSTRUCTURED_MIN_BLOCKS = 2;
15+
16+
function isWrongAncestry({ behindMain, behindBase, threshold = ANCESTRY_BEHIND_THRESHOLD }) {
17+
return behindMain === 0 && behindBase >= threshold;
18+
}
19+
20+
function authorHasPushPermission(permission) {
21+
return permission === "admin" || permission === "maintain" || permission === "write";
22+
}
23+
24+
/**
25+
* True when the body uses literal backslash-n as the dominant line break
26+
* (agent bug seen on #644) rather than real newlines.
27+
*/
28+
function hasEscapedNewlines(text) {
29+
const escaped = (text.match(/\\n/g) || []).length;
30+
if (escaped < 2) return false;
31+
const real = (text.match(/\n/g) || []).length;
32+
return escaped > real;
33+
}
34+
35+
function countContentBlocks(text) {
36+
const blocks = text
37+
.split(/\n\s*\n/)
38+
.map((b) => b.trim())
39+
.filter(Boolean);
40+
if (blocks.length >= 2) return blocks.length;
41+
const bullets = text
42+
.split("\n")
43+
.map((l) => l.trim())
44+
.filter((l) => /^[-*+]\s+\S/.test(l));
45+
return Math.max(blocks.length, bullets.length);
46+
}
47+
48+
function assessPrDescription(body) {
49+
if (typeof body !== "string" || !body.trim()) {
50+
return { ok: false, reason: "empty" };
51+
}
52+
if (hasEscapedNewlines(body)) {
53+
return { ok: false, reason: "escaped_newlines" };
54+
}
55+
const cleaned = clean(body);
56+
if (!cleaned) {
57+
const strippedComments = body.replace(/<!--[\s\S]*?-->/g, "").trim();
58+
if (!strippedComments) return { ok: false, reason: "empty" };
59+
if (isPlaceholderOnlyValue(strippedComments)) {
60+
return { ok: false, reason: "placeholder" };
61+
}
62+
return { ok: false, reason: "empty" };
63+
}
64+
if (isPlaceholderOnlyValue(cleaned)) {
65+
return { ok: false, reason: "placeholder" };
66+
}
67+
if (hasSubstantialStructuredContent(cleaned, MIN_SECTION_LEN, MIN_RICH_SECTIONS)) {
68+
return { ok: true };
69+
}
70+
if (
71+
cleaned.length >= UNSTRUCTURED_MIN_LEN &&
72+
countContentBlocks(cleaned) >= UNSTRUCTURED_MIN_BLOCKS
73+
) {
74+
return { ok: true };
75+
}
76+
return { ok: false, reason: "thin" };
77+
}
78+
79+
function collectPrQualityFailures({
80+
baseRef,
81+
allowedBases,
82+
body,
83+
behindMain,
84+
behindBase,
85+
authorPermission,
86+
permissionLookupFailed = false,
87+
}) {
88+
const failures = [];
89+
const wrongBase = !allowedBases.includes(baseRef);
90+
if (wrongBase) {
91+
failures.push({ code: "wrong_base" });
92+
} else {
93+
const skipAncestry =
94+
!permissionLookupFailed && authorHasPushPermission(authorPermission);
95+
if (!skipAncestry && isWrongAncestry({ behindMain, behindBase })) {
96+
failures.push({ code: "wrong_ancestry" });
97+
}
98+
}
99+
100+
const desc = assessPrDescription(body);
101+
if (!desc.ok) {
102+
failures.push({ code: "bad_description", reason: desc.reason });
103+
}
104+
return failures;
105+
}
106+
107+
module.exports = {
108+
ANCESTRY_BEHIND_THRESHOLD,
109+
isWrongAncestry,
110+
authorHasPushPermission,
111+
assessPrDescription,
112+
collectPrQualityFailures,
113+
hasEscapedNewlines,
114+
};

‎.github/scripts/pr-quality.test.cjs‎

Lines changed: 159 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,159 @@
1+
"use strict";
2+
3+
const { describe, it } = require("node:test");
4+
const assert = require("node:assert/strict");
5+
const {
6+
ANCESTRY_BEHIND_THRESHOLD,
7+
isWrongAncestry,
8+
authorHasPushPermission,
9+
assessPrDescription,
10+
collectPrQualityFailures,
11+
} = require("./pr-quality.cjs");
12+
13+
describe("isWrongAncestry", () => {
14+
it("flags #644-shaped compares (0 behind main, far behind base)", () => {
15+
assert.equal(
16+
isWrongAncestry({ behindMain: 0, behindBase: 44 }),
17+
true,
18+
);
19+
});
20+
21+
it("uses threshold 20 by default", () => {
22+
assert.equal(ANCESTRY_BEHIND_THRESHOLD, 20);
23+
assert.equal(isWrongAncestry({ behindMain: 0, behindBase: 20 }), true);
24+
assert.equal(isWrongAncestry({ behindMain: 0, behindBase: 19 }), false);
25+
});
26+
27+
it("passes when head is behind main (not sitting on main tip)", () => {
28+
assert.equal(isWrongAncestry({ behindMain: 1, behindBase: 44 }), false);
29+
});
30+
});
31+
32+
describe("authorHasPushPermission", () => {
33+
it("accepts write/maintain/admin only", () => {
34+
assert.equal(authorHasPushPermission("admin"), true);
35+
assert.equal(authorHasPushPermission("maintain"), true);
36+
assert.equal(authorHasPushPermission("write"), true);
37+
assert.equal(authorHasPushPermission("triage"), false);
38+
assert.equal(authorHasPushPermission("read"), false);
39+
assert.equal(authorHasPushPermission(null), false);
40+
});
41+
});
42+
43+
describe("assessPrDescription", () => {
44+
it("rejects empty and comment-only bodies", () => {
45+
assert.equal(assessPrDescription("").ok, false);
46+
assert.equal(assessPrDescription(" ").ok, false);
47+
assert.equal(
48+
assessPrDescription("<!-- release notes by coderabbit.ai -->\n\n<!-- end -->").reason,
49+
"empty",
50+
);
51+
});
52+
53+
it("rejects placeholder-only bodies", () => {
54+
assert.equal(assessPrDescription("N/A").reason, "placeholder");
55+
assert.equal(assessPrDescription("TODO").reason, "placeholder");
56+
});
57+
58+
it("rejects literal escaped newlines like #644", () => {
59+
const body =
60+
"## What changed\\n- make the Windows tray launcher resolve Codex home\\n\\n## Validation\\n- git diff --check";
61+
assert.equal(assessPrDescription(body).reason, "escaped_newlines");
62+
});
63+
64+
it("rejects thin real-newline bodies", () => {
65+
assert.equal(assessPrDescription("fix stuff").reason, "thin");
66+
});
67+
68+
it("accepts two rich markdown sections", () => {
69+
const body = [
70+
"## Summary",
71+
"This change updates the Windows tray launcher so it resolves CODEX_HOME through the shared helper instead of a hardcoded path.",
72+
"",
73+
"## Test plan",
74+
"- Launch the tray app after setting CODEX_HOME",
75+
"- Confirm the listener and launcher use the same workspace root",
76+
].join("\n");
77+
assert.equal(assessPrDescription(body).ok, true);
78+
});
79+
80+
it("accepts unstructured bodies that are long enough with multiple blocks", () => {
81+
const p1 =
82+
"Updates the Windows tray launcher to resolve the active Codex home through the shared helper so listener and launcher stay aligned.";
83+
const p2 =
84+
"Validated with git diff --check on the changed tray module; typecheck was not available in that session so CI must cover it.";
85+
assert.equal(assessPrDescription(`${p1}\n\n${p2}`).ok, true);
86+
});
87+
});
88+
89+
describe("collectPrQualityFailures", () => {
90+
const allowed = ["dev", "dev2-go"];
91+
92+
it("reports wrong_base without requiring ancestry inputs", () => {
93+
const failures = collectPrQualityFailures({
94+
baseRef: "main",
95+
allowedBases: allowed,
96+
body: "## Summary\n" + "x".repeat(50) + "\n\n## Test plan\n" + "y".repeat(50),
97+
behindMain: 0,
98+
behindBase: 0,
99+
authorPermission: "read",
100+
});
101+
assert.ok(failures.some((f) => f.code === "wrong_base"));
102+
assert.ok(!failures.some((f) => f.code === "wrong_ancestry"));
103+
});
104+
105+
it("reports wrong_ancestry for contributor on #644-shaped compare", () => {
106+
const failures = collectPrQualityFailures({
107+
baseRef: "dev",
108+
allowedBases: allowed,
109+
body: [
110+
"## Summary",
111+
"This change updates the Windows tray launcher so it resolves CODEX_HOME through the shared helper instead of a hardcoded path.",
112+
"",
113+
"## Test plan",
114+
"- Launch the tray app after setting CODEX_HOME",
115+
"- Confirm the listener and launcher use the same workspace root",
116+
].join("\n"),
117+
behindMain: 0,
118+
behindBase: 44,
119+
authorPermission: "read",
120+
});
121+
assert.deepEqual(
122+
failures.map((f) => f.code),
123+
["wrong_ancestry"],
124+
);
125+
});
126+
127+
it("skips ancestry for push permission but still flags bad description", () => {
128+
const failures = collectPrQualityFailures({
129+
baseRef: "dev",
130+
allowedBases: allowed,
131+
body: "",
132+
behindMain: 0,
133+
behindBase: 44,
134+
authorPermission: "write",
135+
});
136+
assert.ok(!failures.some((f) => f.code === "wrong_ancestry"));
137+
assert.ok(failures.some((f) => f.code === "bad_description"));
138+
});
139+
140+
it("applies ancestry when permission lookup failed (fail closed)", () => {
141+
const failures = collectPrQualityFailures({
142+
baseRef: "dev",
143+
allowedBases: allowed,
144+
body: [
145+
"## Summary",
146+
"This change updates the Windows tray launcher so it resolves CODEX_HOME through the shared helper instead of a hardcoded path.",
147+
"",
148+
"## Test plan",
149+
"- Launch the tray app after setting CODEX_HOME",
150+
"- Confirm the listener and launcher use the same workspace root",
151+
].join("\n"),
152+
behindMain: 0,
153+
behindBase: 44,
154+
authorPermission: null,
155+
permissionLookupFailed: true,
156+
});
157+
assert.ok(failures.some((f) => f.code === "wrong_ancestry"));
158+
});
159+
});

‎.github/workflows/issue-quality-tests.yml‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@ on:
66
- ".github/ISSUE_TEMPLATE/**"
77
- ".github/scripts/issue-quality.cjs"
88
- ".github/scripts/issue-quality.test.cjs"
9+
- ".github/scripts/pr-quality.cjs"
10+
- ".github/scripts/pr-quality.test.cjs"
911
- ".github/scripts/pr-labeler.cjs"
1012
- ".github/scripts/pr-labeler.test.cjs"
1113
- ".github/scripts/enforce-pr-target.test.cjs"
@@ -25,6 +27,8 @@ on:
2527
- ".github/ISSUE_TEMPLATE/**"
2628
- ".github/scripts/issue-quality.cjs"
2729
- ".github/scripts/issue-quality.test.cjs"
30+
- ".github/scripts/pr-quality.cjs"
31+
- ".github/scripts/pr-quality.test.cjs"
2832
- ".github/scripts/pr-labeler.cjs"
2933
- ".github/scripts/pr-labeler.test.cjs"
3034
- ".github/scripts/enforce-pr-target.test.cjs"
@@ -56,6 +60,7 @@ jobs:
5660
- name: Run validator tests
5761
run: |
5862
node --test .github/scripts/issue-quality.test.cjs
63+
node --test .github/scripts/pr-quality.test.cjs
5964
node --test .github/scripts/pr-labeler.test.cjs
6065
node --test .github/scripts/enforce-pr-target.test.cjs
6166
node --test .github/scripts/issue-translation.test.cjs

0 commit comments

Comments
 (0)