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

Treat removed new-form Version/OS headings as missing, soft-fail
ancestry compares, tighten harness require/write allowlists, and
clarify enforce-target is convention until branch protection.

Author: Wibias

.github/scripts/issue-quality.cjs

@@ -704,11 +704,11 @@ function validateIssue(issue) {
     } else if (
       !softPass &&
       isNewBugForm &&
-      version !== null &&
-      (isEmpty(version) || isRawPlaceholder(version))
+      (version === null || isEmpty(version) || isRawPlaceholder(version))
     ) {
-      // New form requires Version. Legacy N/A / No response soft-pass stays
-      // only for bodies without Client or integration.
+      // New form requires Version (including when the heading was removed).
+      // Legacy N/A / No response soft-pass stays only for bodies without
+      // Client or integration.
       reasons.push("Version is missing.");
       guidance.push("Add your OpenCodex version so we can reproduce the environment.");
     }
@@ -719,8 +719,7 @@ function validateIssue(issue) {
     } else if (
       !softPass &&
       isNewBugForm &&
-      os !== null &&
-      (isEmpty(os) || isRawPlaceholder(os))
+      (os === null || isEmpty(os) || isRawPlaceholder(os))
     ) {
       reasons.push("Operating system is missing.");
       guidance.push("Add your OS name and version (for example Windows 11 24H2).");

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

@@ -799,6 +799,46 @@ describe("validateIssue - bug", () => {
     assert.ok(result.reasons.some((r) => /Operating system/i.test(r)));
   });
 
+  it("rejects a new-form bug when the Version heading was removed", () => {
+    const body = [
+      "### Client or integration",
+      "Codex CLI",
+      "### Area",
+      "CLI",
+      "### Summary",
+      "Proxy returns 502 when streaming is enabled on Windows.",
+      "### Reproduction",
+      "1. ocx start",
+      "2. Send a streaming /v1/responses request",
+      "### Operating system",
+      "Windows 11",
+    ].join("\n");
+    const result = validateIssue({ title: "Streaming 502", body, labels: ["bug"] });
+    assert.equal(result.kind, "bug");
+    assert.equal(result.valid, false);
+    assert.ok(result.reasons.some((r) => /Version/i.test(r) && /missing/i.test(r)));
+  });
+
+  it("rejects a new-form bug when the Operating system heading was removed", () => {
+    const body = [
+      "### Client or integration",
+      "Codex CLI",
+      "### Area",
+      "CLI",
+      "### Summary",
+      "Proxy returns 502 when streaming is enabled on Windows.",
+      "### Reproduction",
+      "1. ocx start",
+      "2. Send a streaming /v1/responses request",
+      "### Version",
+      "2.7.42",
+    ].join("\n");
+    const result = validateIssue({ title: "Streaming 502", body, labels: ["bug"] });
+    assert.equal(result.kind, "bug");
+    assert.equal(result.valid, false);
+    assert.ok(result.reasons.some((r) => /Operating system/i.test(r) && /missing/i.test(r)));
+  });
+
   it("rejects a new-form bug whose Reproduction is only a vague phrase", () => {
     const body = [
       "### Client or integration",

.github/scripts/pr-quality.cjs

@@ -84,14 +84,18 @@ function collectPrQualityFailures({
   behindBase,
   authorPermission,
   permissionLookupFailed = false,
+  ancestryLookupFailed = false,
 }) {
   const failures = [];
   const wrongBase = !allowedBases.includes(baseRef);
   if (wrongBase) {
     failures.push({ code: "wrong_base" });
   } else {
+    // Permission lookup fails closed (still evaluate ancestry). Compare API
+    // failures skip ancestry — zeros would falsely pass the #644 heuristic.
     const skipAncestry =
-      !permissionLookupFailed && authorHasPushPermission(authorPermission);
+      ancestryLookupFailed ||
+      (!permissionLookupFailed && authorHasPushPermission(authorPermission));
     if (!skipAncestry && isWrongAncestry({ behindMain, behindBase })) {
       failures.push({ code: "wrong_ancestry" });
     }

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

@@ -170,4 +170,24 @@ describe("collectPrQualityFailures", () => {
     });
     assert.ok(failures.some((f) => f.code === "wrong_ancestry"));
   });
+
+  it("skips ancestry when compare lookup failed (cannot evaluate)", () => {
+    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: 0,
+      authorPermission: "read",
+      ancestryLookupFailed: true,
+    });
+    assert.ok(!failures.some((f) => f.code === "wrong_ancestry"));
+  });
 });

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

@@ -277,25 +277,33 @@ jobs:
 
             let behindMain = 0;
             let behindBase = 0;
+            let ancestryLookupFailed = false;
             const baseAllowed = ALLOWED_BASES.includes(pr.base.ref);
 
             if (baseAllowed) {
               const headSha = pr.head.sha;
-              const { data: mainCompare } =
-                await github.rest.repos.compareCommitsWithBasehead({
-                  owner,
-                  repo,
-                  basehead: `main...${headSha}`
-                });
-              behindMain = mainCompare.behind_by;
-
-              const { data: baseCompare } =
-                await github.rest.repos.compareCommitsWithBasehead({
-                  owner,
-                  repo,
-                  basehead: `${pr.base.ref}...${headSha}`
-                });
-              behindBase = baseCompare.behind_by;
+              try {
+                const { data: mainCompare } =
+                  await github.rest.repos.compareCommitsWithBasehead({
+                    owner,
+                    repo,
+                    basehead: `main...${headSha}`
+                  });
+                behindMain = mainCompare.behind_by;
+
+                const { data: baseCompare } =
+                  await github.rest.repos.compareCommitsWithBasehead({
+                    owner,
+                    repo,
+                    basehead: `${pr.base.ref}...${headSha}`
+                  });
+                behindBase = baseCompare.behind_by;
+              } catch (error) {
+                ancestryLookupFailed = true;
+                core.warning(
+                  `Could not compare commits for ancestry check: ${error.message}`
+                );
+              }
             }
 
             const failures = collectPrQualityFailures({
@@ -305,7 +313,8 @@ jobs:
               behindMain,
               behindBase,
               authorPermission,
-              permissionLookupFailed
+              permissionLookupFailed,
+              ancestryLookupFailed
             });
 
             if (failures.length > 0) {

AGENTS.md

@@ -80,10 +80,12 @@ integration line to another, or rebasing a stale branch onto the current head,
 is ordinary maintenance rather than noise — open it as a normal pull request
 and name the source commits in the description.
 
-The required **`enforce-target`** CI check rejects pull requests whose head
+The **`enforce-target`** CI check rejects pull requests whose head
 ancestry sits on the **`main`** tip while far behind **`dev`** or **`dev2-go`**,
 and rejects empty, thin, or malformed descriptions; authors with repository
-push permission skip the ancestry heuristic only.
+push permission skip the ancestry heuristic only. As with approval requirements
+in [`MAINTAINERS.md`](./MAINTAINERS.md), this is enforced by convention until
+branch protection is configured.
 
 [`MAINTAINERS.md`](./MAINTAINERS.md) is authoritative for review and merge
 policy (approvals, CI requirements, security review, promotion). This file

MAINTAINERS.md

@@ -48,10 +48,12 @@ rule is written out under [Review and merge policy](#review-and-merge-policy).
     `dev2-go` before closing out the `dev` merge, label it `needs-go-port`,
     and name the source commits. That label is the durable signal that a
     deferred port is intentional, not forgotten.
-- The required **`enforce-target`** CI check rejects pull requests whose head
+- The **`enforce-target`** CI check rejects pull requests whose head
   ancestry sits on the **`main`** tip while far behind **`dev`** or **`dev2-go`**,
   and rejects empty, thin, or malformed descriptions; authors with repository
-  push permission skip the ancestry heuristic only.
+  push permission skip the ancestry heuristic only. As with the approval
+  requirement above, this is enforced by convention until branch protection is
+  configured (see the note under the change log).
 - A pull request requires approval from at least one maintainer and successful required CI checks
   before merge.
 - Authors do not approve their own pull requests.

tests/ci-workflows.test.ts

@@ -713,7 +713,13 @@ describe("GitHub Actions hardening", () => {
     // nobody reviewed.
     const restWrites = [...script.matchAll(/github\.rest\.[\w.]+/g)]
       .map(match => match[0])
-      .filter(name => !name.endsWith(".get") && !name.endsWith(".listComments") && !name.includes(".repos."));
+      .filter(
+        name =>
+          !name.endsWith(".get") &&
+          !name.endsWith(".listComments") &&
+          name !== "github.rest.repos.getCollaboratorPermissionLevel" &&
+          name !== "github.rest.repos.compareCommitsWithBasehead",
+      );
     expect([...new Set(restWrites)].sort()).toEqual([
       "github.rest.issues.createComment",
       "github.rest.issues.updateComment",

tests/helpers/enforce-pr-target-harness.ts

@@ -398,6 +398,11 @@ export async function runEnforcePrTarget(
   const outputs: { name: string; value: unknown }[] = [];
   const states = new Map<string, unknown>();
   const failOn = new Set(options.failOn ?? []);
+  if (options.failPermissionLookup) {
+    // Route through `record` so the call appears in the recording even when it
+    // rejects (same semantics as `failOn`).
+    failOn.add("repos.getCollaboratorPermissionLevel");
+  }
   const failStatus = options.failStatus ?? 500;
 
   const pr = {
@@ -452,19 +457,21 @@ export async function runEnforcePrTarget(
 
   const nodeRequire = createRequire(path.join(process.cwd(), "package.json"));
   const scriptsRoot = path.resolve(process.cwd(), ".github", "scripts");
+  /** Bare modules the workflow script may load (see enforce-pr-target.yml). */
+  const ALLOWED_MODULES = new Set(["path", "node:path"]);
 
   function scopedRequire(id: string) {
     calls.push({ method: "require", args: [id] });
-    const isRelative = id.startsWith(".") || id.startsWith("/") || path.isAbsolute(id);
-    if (!isRelative) {
+    const isPathLike = id.startsWith(".") || path.isAbsolute(id);
+    if (!isPathLike) {
+      if (!ALLOWED_MODULES.has(id)) {
+        throw new Error(`the script must not require ${id}`);
+      }
       return nodeRequire(id);
     }
-    const resolved = path.isAbsolute(id) ? id : path.resolve(process.cwd(), id);
+    const resolved = path.isAbsolute(id) ? path.resolve(id) : path.resolve(process.cwd(), id);
     if (!resolved.startsWith(scriptsRoot + path.sep) && resolved !== scriptsRoot) {
-      const norm = resolved.replace(/\\/g, "/");
-      if (!norm.includes("/.github/scripts/")) {
-        throw new Error(`the script must not require ${id}`);
-      }
+      throw new Error(`the script must not require ${id}`);
     }
     return nodeRequire(resolved);
   }
@@ -494,14 +501,10 @@ export async function runEnforcePrTarget(
       updateComment: (args: unknown) => respond("issues.updateComment", args, { id: 7 }),
     },
     repos: {
-      getCollaboratorPermissionLevel: (args: unknown) => {
-        if (options.failPermissionLookup) {
-          return Promise.reject(octokitError("repos.getCollaboratorPermissionLevel", failStatus));
-        }
-        return respond("repos.getCollaboratorPermissionLevel", args, {
+      getCollaboratorPermissionLevel: (args: unknown) =>
+        respond("repos.getCollaboratorPermissionLevel", args, {
           permission: options.authorPermission ?? "read",
-        });
-      },
+        }),
       compareCommitsWithBasehead: (args: unknown) => {
         const basehead = String((args as { basehead?: string })?.basehead ?? "");
         return respond("repos.compareCommitsWithBasehead", args, compareResult(basehead));