Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 31 additions & 2 deletions .github/scripts/enforce-pr-target.test.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,8 @@ describe("enforce-pr-target workflow", () => {
assert.match(workflow, /pull_request_target:/);
assert.doesNotMatch(
workflow,
/actions\/checkout@/,
"wrong-branch enforcer must not check out untrusted PR code",
/ref:\s*\$\{\{\s*github\.event\.pull_request\.head/,
"enforcer must not check out untrusted PR head code",
);
});

Expand Down Expand Up @@ -43,4 +43,33 @@ describe("enforce-pr-target workflow", () => {
assert.match(workflow, /readyConversionFailed/);
assert.match(workflow, /Could not mark pull request ready for review/);
});

it("listens for synchronize so rebase can clear ancestry failures", () => {
assert.match(workflow, /synchronize/);
});

it("checks out trusted default-branch scripts only (never PR head)", () => {
assert.match(workflow, /actions\/checkout@[0-9a-f]{40}/);
assert.match(workflow, /ref:\s*\$\{\{\s*github\.event\.repository\.default_branch\s*\}\}/);
assert.match(workflow, /sparse-checkout:\s*\.github\/scripts/);
assert.match(workflow, /persist-credentials:\s*false/);
assert.doesNotMatch(workflow, /ref:\s*\$\{\{\s*github\.event\.pull_request\.head/);
});

it("loads pr-quality via require from the checked-out scripts", () => {
assert.match(workflow, /pr-quality\.cjs/);
assert.match(workflow, /collectPrQualityFailures/);
});

it("strips stale WRONG BRANCH prefix on failure when base is corrected", () => {
const failureBlock = workflow.match(
/if \(failures\.length > 0\) \{([\s\S]*?)core\.setFailed\(/,
);
assert.ok(failureBlock, "workflow must have a failure path");
const failurePath = failureBlock[1];
assert.match(failurePath, /shouldStripTitlePrefix/);
assert.match(failurePath, /!hasWrongBase/);
assert.match(failurePath, /titlePrefixedByBot = false/);
assert.match(failurePath, /pr\.title\.slice\(TITLE_PREFIX\.length\)/);
});
});
11 changes: 5 additions & 6 deletions .github/scripts/issue-quality.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -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.");
}
Expand All @@ -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).");
Expand Down
40 changes: 40 additions & 0 deletions .github/scripts/issue-quality.test.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
172 changes: 172 additions & 0 deletions .github/scripts/pr-quality.cjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,172 @@
"use strict";

const path = require("node:path");
const {
clean,
isPlaceholderOnlyValue,
hasSubstantialStructuredContent,
} = 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;

/**
* 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) {
return permission === "admin" || permission === "maintain" || permission === "write";
}

/**
* True when the body uses literal backslash-n as the dominant line break
* (agent bug seen on #644) rather than real newlines.
*/
function hasEscapedNewlines(text) {
const escaped = (text.match(/\\n/g) || []).length;
if (escaped < 2) return false;
const real = (text.match(/\n/g) || []).length;
return escaped > real;
}

function countContentBlocks(text) {
const blocks = text
.split(/\n\s*\n/)
.map((b) => b.trim())
.filter(Boolean);
if (blocks.length >= 2) return blocks.length;
const bullets = text
.split("\n")
.map((l) => l.trim())
.filter((l) => /^[-*+]\s+\S/.test(l));
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 withoutTemplate = stripPrTemplateBoilerplate(body);
const cleaned = clean(withoutTemplate);
if (!cleaned) {
const strippedComments = withoutTemplate.replace(/<!--[\s\S]*?-->/g, "").trim();
if (!strippedComments) return { ok: false, reason: "empty" };
if (isPlaceholderOnlyValue(strippedComments)) {
return { ok: false, reason: "placeholder" };
}
return { ok: false, reason: "empty" };
}
if (isPlaceholderOnlyValue(cleaned)) {
return { ok: false, reason: "placeholder" };
}
if (hasSubstantialStructuredContent(cleaned, MIN_SECTION_LEN, MIN_RICH_SECTIONS)) {
return { ok: true };
}
if (
cleaned.length >= UNSTRUCTURED_MIN_LEN &&
countContentBlocks(cleaned) >= UNSTRUCTURED_MIN_BLOCKS
) {
return { ok: true };
}
return { ok: false, reason: "thin" };
}

function collectPrQualityFailures({
baseRef,
allowedBases,
body,
behindMain,
behindBase,
aheadMain = 0,
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 =
ancestryLookupFailed ||
(!permissionLookupFailed && authorHasPushPermission(authorPermission));
if (
!skipAncestry &&
isWrongAncestry({ behindMain, behindBase, aheadMain })
) {
failures.push({ code: "wrong_ancestry" });
}
}

const desc = assessPrDescription(body);
if (!desc.ok) {
failures.push({ code: "bad_description", reason: desc.reason });
}
return failures;
}

module.exports = {
ANCESTRY_BEHIND_THRESHOLD,
ANCESTRY_AHEAD_MAIN_MAX,
isWrongAncestry,
authorHasPushPermission,
assessPrDescription,
collectPrQualityFailures,
hasEscapedNewlines,
stripPrTemplateBoilerplate,
};
Loading
Loading