fix(ci): address Codex findings on PR quality gates

Reject untouched PR templates, require low ahead-of-main for ancestry, and checkpoint draft/title ownership before mutations.

Author: Wibias

.github/scripts/pr-quality.cjs

@@ -8,13 +8,37 @@ const {
 } = require(path.join(__dirname, "issue-quality.cjs"));
 
 const ANCESTRY_BEHIND_THRESHOLD = 20;
+/** Cap on ahead_by vs main so stale `dev` forks (many commits ahead of main) are not flagged. */
+const ANCESTRY_AHEAD_MAIN_MAX = 5;
 const MIN_SECTION_LEN = 40;
 const MIN_RICH_SECTIONS = 2;
 const UNSTRUCTURED_MIN_LEN = 120;
 const UNSTRUCTURED_MIN_BLOCKS = 2;
 
-function isWrongAncestry({ behindMain, behindBase, threshold = ANCESTRY_BEHIND_THRESHOLD }) {
-  return behindMain === 0 && behindBase >= threshold;
+/**
+ * Exact instruction / checklist lines from `.github/PULL_REQUEST_TEMPLATE.md`.
+ * Untouched templates must not count as substance.
+ */
+const PR_TEMPLATE_BOILERPLATE_LINES = new Set([
+  "explain the user-visible or maintainer-facing change.",
+  "list the commands or checks you ran.",
+  "scope stays focused and avoids unrelated cleanup.",
+  "docs or release notes were updated when needed.",
+  "security-sensitive changes were reviewed for secrets, auth, and unsafe defaults.",
+]);
+
+function isWrongAncestry({
+  behindMain,
+  behindBase,
+  aheadMain = 0,
+  threshold = ANCESTRY_BEHIND_THRESHOLD,
+  aheadMainMax = ANCESTRY_AHEAD_MAIN_MAX,
+}) {
+  return (
+    behindMain === 0 &&
+    behindBase >= threshold &&
+    aheadMain <= aheadMainMax
+  );
 }
 
 function authorHasPushPermission(permission) {
@@ -45,16 +69,40 @@ function countContentBlocks(text) {
   return Math.max(blocks.length, bullets.length);
 }
 
+function normalizeTemplateLine(line) {
+  return line
+    .replace(/^\s*[-*+]\s+/, "")
+    .replace(/^\s*\[[ xX]\]\s+/, "")
+    .replace(/^\s*#{1,6}\s+/, "")
+    .trim()
+    .toLowerCase();
+}
+
+/** Drop stock PR template headings, instructions, and checklist lines. */
+function stripPrTemplateBoilerplate(text) {
+  return text
+    .split("\n")
+    .filter((line) => {
+      const normalized = normalizeTemplateLine(line);
+      if (!normalized) return true;
+      if (PR_TEMPLATE_BOILERPLATE_LINES.has(normalized)) return false;
+      if (/^(summary|verification|checklist)$/.test(normalized)) return false;
+      return true;
+    })
+    .join("\n");
+}
+
 function assessPrDescription(body) {
   if (typeof body !== "string" || !body.trim()) {
     return { ok: false, reason: "empty" };
   }
   if (hasEscapedNewlines(body)) {
     return { ok: false, reason: "escaped_newlines" };
   }
-  const cleaned = clean(body);
+  const withoutTemplate = stripPrTemplateBoilerplate(body);
+  const cleaned = clean(withoutTemplate);
   if (!cleaned) {
-    const strippedComments = body.replace(/<!--[\s\S]*?-->/g, "").trim();
+    const strippedComments = withoutTemplate.replace(/<!--[\s\S]*?-->/g, "").trim();
     if (!strippedComments) return { ok: false, reason: "empty" };
     if (isPlaceholderOnlyValue(strippedComments)) {
       return { ok: false, reason: "placeholder" };
@@ -82,6 +130,7 @@ function collectPrQualityFailures({
   body,
   behindMain,
   behindBase,
+  aheadMain = 0,
   authorPermission,
   permissionLookupFailed = false,
   ancestryLookupFailed = false,
@@ -96,7 +145,10 @@ function collectPrQualityFailures({
     const skipAncestry =
       ancestryLookupFailed ||
       (!permissionLookupFailed && authorHasPushPermission(authorPermission));
-    if (!skipAncestry && isWrongAncestry({ behindMain, behindBase })) {
+    if (
+      !skipAncestry &&
+      isWrongAncestry({ behindMain, behindBase, aheadMain })
+    ) {
       failures.push({ code: "wrong_ancestry" });
     }
   }
@@ -110,9 +162,11 @@ function collectPrQualityFailures({
 
 module.exports = {
   ANCESTRY_BEHIND_THRESHOLD,
+  ANCESTRY_AHEAD_MAIN_MAX,
   isWrongAncestry,
   authorHasPushPermission,
   assessPrDescription,
   collectPrQualityFailures,
   hasEscapedNewlines,
+  stripPrTemplateBoilerplate,
 };

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

@@ -11,21 +11,28 @@ const {
 } = require("./pr-quality.cjs");
 
 describe("isWrongAncestry", () => {
-  it("flags #644-shaped compares (0 behind main, far behind base)", () => {
+  it("flags #644-shaped compares (0 behind main, far behind base, few ahead of main)", () => {
     assert.equal(
-      isWrongAncestry({ behindMain: 0, behindBase: 44 }),
+      isWrongAncestry({ behindMain: 0, behindBase: 44, aheadMain: 1 }),
       true,
     );
   });
 
   it("uses threshold 20 by default", () => {
     assert.equal(ANCESTRY_BEHIND_THRESHOLD, 20);
-    assert.equal(isWrongAncestry({ behindMain: 0, behindBase: 20 }), true);
-    assert.equal(isWrongAncestry({ behindMain: 0, behindBase: 19 }), false);
+    assert.equal(isWrongAncestry({ behindMain: 0, behindBase: 20, aheadMain: 1 }), true);
+    assert.equal(isWrongAncestry({ behindMain: 0, behindBase: 19, aheadMain: 1 }), false);
   });
 
   it("passes when head is behind main (not sitting on main tip)", () => {
-    assert.equal(isWrongAncestry({ behindMain: 1, behindBase: 44 }), false);
+    assert.equal(isWrongAncestry({ behindMain: 1, behindBase: 44, aheadMain: 1 }), false);
+  });
+
+  it("passes stale dev-based branches that are many commits ahead of main", () => {
+    assert.equal(
+      isWrongAncestry({ behindMain: 0, behindBase: 44, aheadMain: 50 }),
+      false,
+    );
   });
 });
 
@@ -65,6 +72,27 @@ describe("assessPrDescription", () => {
     assert.equal(assessPrDescription("fix stuff").reason, "thin");
   });
 
+  it("rejects an untouched GitHub PR template as empty/thin", () => {
+    const body = [
+      "## Summary",
+      "",
+      "- Explain the user-visible or maintainer-facing change.",
+      "",
+      "## Verification",
+      "",
+      "- List the commands or checks you ran.",
+      "",
+      "## Checklist",
+      "",
+      "- [ ] Scope stays focused and avoids unrelated cleanup.",
+      "- [ ] Docs or release notes were updated when needed.",
+      "- [ ] Security-sensitive changes were reviewed for secrets, auth, and unsafe defaults.",
+    ].join("\n");
+    const result = assessPrDescription(body);
+    assert.equal(result.ok, false);
+    assert.ok(result.reason === "empty" || result.reason === "thin");
+  });
+
   it("accepts two rich markdown sections", () => {
     const body = [
       "## Summary",
@@ -130,6 +158,7 @@ describe("collectPrQualityFailures", () => {
       ].join("\n"),
       behindMain: 0,
       behindBase: 44,
+      aheadMain: 1,
       authorPermission: "read",
     });
     assert.deepEqual(
@@ -145,6 +174,7 @@ describe("collectPrQualityFailures", () => {
       body: "",
       behindMain: 0,
       behindBase: 44,
+      aheadMain: 1,
       authorPermission: "write",
     });
     assert.ok(!failures.some((f) => f.code === "wrong_ancestry"));
@@ -165,12 +195,33 @@ describe("collectPrQualityFailures", () => {
       ].join("\n"),
       behindMain: 0,
       behindBase: 44,
+      aheadMain: 1,
       authorPermission: null,
       permissionLookupFailed: true,
     });
     assert.ok(failures.some((f) => f.code === "wrong_ancestry"));
   });
 
+  it("does not flag stale dev-based branches that are far ahead of main", () => {
+    const failures = collectPrQualityFailures({
+      baseRef: "dev",
+      allowedBases: allowed,
+      body: [
+        "## Summary",
+        "This change updates the Windows tray launcher so it resolves CODEX_HOME through the shared helper instead of a hardcoded path.",
+        "",
+        "## Test plan",
+        "- Launch the tray app after setting CODEX_HOME",
+        "- Confirm the listener and launcher use the same workspace root",
+      ].join("\n"),
+      behindMain: 0,
+      behindBase: 44,
+      aheadMain: 50,
+      authorPermission: "read",
+    });
+    assert.ok(!failures.some((f) => f.code === "wrong_ancestry"));
+  });
+
   it("skips ancestry when compare lookup failed (cannot evaluate)", () => {
     const failures = collectPrQualityFailures({
       baseRef: "dev",
@@ -185,6 +236,7 @@ describe("collectPrQualityFailures", () => {
       ].join("\n"),
       behindMain: 0,
       behindBase: 0,
+      aheadMain: 0,
       authorPermission: "read",
       ancestryLookupFailed: true,
     });

.github/workflows/enforce-pr-target.yml

@@ -277,6 +277,7 @@ jobs:
 
             let behindMain = 0;
             let behindBase = 0;
+            let aheadMain = 0;
             let ancestryLookupFailed = false;
             const baseAllowed = ALLOWED_BASES.includes(pr.base.ref);
 
@@ -290,6 +291,7 @@ jobs:
                     basehead: `main...${headSha}`
                   });
                 behindMain = mainCompare.behind_by;
+                aheadMain = mainCompare.ahead_by;
 
                 const { data: baseCompare } =
                   await github.rest.repos.compareCommitsWithBasehead({
@@ -312,6 +314,7 @@ jobs:
               body: pr.body,
               behindMain,
               behindBase,
+              aheadMain,
               authorPermission,
               permissionLookupFailed,
               ancestryLookupFailed
@@ -346,10 +349,10 @@ jobs:
                 state.titlePrefixedByBot &&
                 pr.title.startsWith(TITLE_PREFIX);
 
+              // Claim title ownership before adding a prefix. Do not clear
+              // ownership until strip succeeds — a failed update must retry.
               if (willPrefixTitle) {
                 state.titlePrefixedByBot = true;
-              } else if (shouldStripTitlePrefix) {
-                state.titlePrefixedByBot = false;
               }
 
               let draftConversionFailed = false;
@@ -380,12 +383,35 @@ jobs:
                   pull_number,
                   title: pr.title.slice(TITLE_PREFIX.length)
                 });
+                state.titlePrefixedByBot = false;
+                await upsertComment(
+                  [
+                    COMMENT_MARKER,
+                    stateMarker(state),
+                    "",
+                    ...failureSections,
+                    "",
+                    "Stale title prefix removed; continuing…"
+                  ].join("\n")
+                );
               }
 
               if (!pr.draft) {
+                // Claim draft ownership before the mutation so a successful
+                // convert followed by a failed comment still restores later.
+                state.autoDraftedByBot = true;
+                await upsertComment(
+                  [
+                    COMMENT_MARKER,
+                    stateMarker(state),
+                    "",
+                    ...failureSections,
+                    "",
+                    "Draft conversion pending…"
+                  ].join("\n")
+                );
                 try {
                   await convertToDraft();
-                  state.autoDraftedByBot = true;
                   await upsertComment(
                     [
                       COMMENT_MARKER,