From 2f2dfa9b51e04cc53a8264a5fc31074cc7391fb4 Mon Sep 17 00:00:00 2001 From: Wibias <37517432+Wibias@users.noreply.github.com> Date: Fri, 24 Jul 2026 13:41:09 +0200 Subject: [PATCH 1/5] fix(ci): reject low-effort feature reports in issue-quality bot --- .github/scripts/issue-quality.cjs | 46 ++++++++++++++++++++- .github/scripts/issue-quality.test.cjs | 45 ++++++++++++++++++++ .github/workflows/enforce-issue-quality.yml | 12 +++++- 3 files changed, 100 insertions(+), 3 deletions(-) diff --git a/.github/scripts/issue-quality.cjs b/.github/scripts/issue-quality.cjs index 2caa8a240..169c981d6 100644 --- a/.github/scripts/issue-quality.cjs +++ b/.github/scripts/issue-quality.cjs @@ -13,6 +13,7 @@ function clean(raw) { let s = raw.replace(//g, ""); // Treat GitHub's "No response" placeholder as empty. s = s.replace(/^[\s_*]*No response[\s_*]*$/gim, ""); + s = s.replace(/^[\s_*]*(na|n\/a|not applicable|not available)[\s_*]*$/gim, ""); return s.trim(); } @@ -334,7 +335,10 @@ function isPlaceholder(text) { const lower = c.toLowerCase(); return ( lower === "no response" || + lower === "na" || lower === "n/a" || + lower === "not applicable" || + lower === "not available" || lower === "none" || lower === "todo" || lower === "tbd" || @@ -342,6 +346,32 @@ function isPlaceholder(text) { ); } +function countWords(text) { + const c = clean(text); + if (!c) return 0; + const spaced = (c.match(/\b[\p{L}\p{N}']+\b/gu) || []).length; + if (spaced > 0) return spaced; + return (c.match(/\p{L}/gu) || []).length; +} + +function hasConcreteDetail(text) { + const c = clean(text); + if (!c) return false; + return ( + /\d/.test(c) || + /[`{}\[\]<>/\\]/.test(c) || + /\b(ocx|config|api|cli|dashboard|provider|proxy|route|endpoint|workflow|command)\b/i.test(c) + ); +} + +function isTooTerseFeatureSection(text) { + if (isEmpty(text) || isPlaceholder(text)) return false; + const words = countWords(text); + if (words >= 8) return false; + if (words >= 6 && hasConcreteDetail(text)) return false; + return !hasConcreteDetail(text); +} + /** * Check if raw section text is a GitHub "No response" placeholder variant * without stripping it first. Used to distinguish intentionally blank optional @@ -386,7 +416,12 @@ function validateIssue(issue) { // On the legacy / translated forms these sections may be absent (null). if (blocker !== null && isEmpty(blocker)) emptyCore.push("current limitation"); if (isEmpty(behaviour)) emptyCore.push("expected behaviour"); - if (example !== null && isEmpty(example)) emptyCore.push("example usage"); + if (example !== null && isPlaceholder(example)) { + reasons.push("Example usage or interface contains placeholder text instead of a concrete example."); + guidance.push("Add a real CLI command, config snippet, API exchange, or before/after workflow example."); + } else if (example !== null && isEmpty(example)) { + emptyCore.push("example usage"); + } const mappedHeadingPresent = goal !== null || blocker !== null || behaviour !== null || example !== null; @@ -421,6 +456,15 @@ function validateIssue(issue) { guidance.push("Replace placeholder text with your actual proposal."); } } + + const terseSections = []; + if (goal !== null && isTooTerseFeatureSection(goal)) terseSections.push("goal / problem"); + if (blocker !== null && isTooTerseFeatureSection(blocker)) terseSections.push("current limitation"); + if (behaviour !== null && isTooTerseFeatureSection(behaviour)) terseSections.push("expected behaviour"); + if (terseSections.length > 0) { + reasons.push(`Required sections are too vague to act on: ${terseSections.join(", ")}.`); + guidance.push("Describe the workflow, limitation, and expected behaviour with enough detail for someone to implement or evaluate the request."); + } } if (kind === "bug") { diff --git a/.github/scripts/issue-quality.test.cjs b/.github/scripts/issue-quality.test.cjs index 80d1c2f9a..1322aef79 100644 --- a/.github/scripts/issue-quality.test.cjs +++ b/.github/scripts/issue-quality.test.cjs @@ -163,6 +163,46 @@ describe("validateIssue - feature", () => { assert.equal(result.valid, true); }); + it("rejects issue #401-style low-effort feature with placeholder example", () => { + const body = [ + "### Area", + "Proxy and routing", + "### What are you trying to accomplish?", + "Quota for Chatgpt running out that can no longer use voice mode. Would like to change other model for that", + "### What prevents this today?", + "No usage without codex quota", + "### What should OpenCodex do?", + "Change another voice model", + "### Example usage or interface", + "NA", + ].join("\n"); + const result = validateIssue({ + title: "Change voice chat to different model", + body, + labels: ["enhancement"], + }); + assert.equal(result.kind, "feature"); + assert.equal(result.valid, false); + assert.ok(result.reasons.some((r) => r.includes("placeholder"))); + assert.ok(result.reasons.some((r) => r.includes("expected behaviour"))); + }); + + it("rejects feature reports with placeholder example variants", () => { + const body = [ + "### What are you trying to accomplish?", + "Route voice requests to a configured fallback provider when the primary quota is exhausted.", + "### What prevents this today?", + "Voice mode is hard-wired to the primary Codex quota and cannot switch providers.", + "### What should OpenCodex do?", + "Expose a setting to choose the fallback voice model and provider.", + "### Example usage or interface", + "N/A", + ].join("\n"); + const result = validateIssue({ title: "Voice fallback routing", body, labels: ["enhancement"] }); + assert.equal(result.valid, false); + assert.ok(result.reasons.some((r) => r.includes("placeholder"))); + }); + it("accepts a valid legacy feature request without blocker/example headings", () => { const body = [ "### Problem to solve", @@ -424,6 +464,11 @@ describe("normalisation", () => { assert.equal(clean("_No response_"), ""); }); + it("treats NA and not applicable as placeholders", () => { + assert.equal(clean("NA"), ""); + assert.equal(clean("not applicable"), ""); + }); + it("strips HTML comments", () => { assert.equal(clean("Hello world"), "Hello world"); }); diff --git a/.github/workflows/enforce-issue-quality.yml b/.github/workflows/enforce-issue-quality.yml index 73a1e4b6b..8c1cb2a34 100644 --- a/.github/workflows/enforce-issue-quality.yml +++ b/.github/workflows/enforce-issue-quality.yml @@ -10,13 +10,19 @@ on: - opened - edited - reopened + workflow_dispatch: + inputs: + issue_number: + description: Issue number to validate and enforce + required: true + type: number permissions: contents: read issues: write concurrency: - group: issue-quality-${{ github.event.issue.number }} + group: issue-quality-${{ github.event.issue.number || inputs.issue_number }} cancel-in-progress: true jobs: @@ -45,7 +51,9 @@ jobs: } = require(path.join(process.cwd(), ".github", "scripts", "issue-quality.cjs")); const { owner, repo } = context.repo; - const issue_number = context.payload.issue.number; + const issue_number = context.eventName === "workflow_dispatch" + ? Number(context.payload.inputs.issue_number) + : context.payload.issue.number; const actor = context.actor; const eventType = context.eventName === "issues" ? context.payload.action From 34b3c2de0ce2dcf4f4946c92d87c0b1a044706fd Mon Sep 17 00:00:00 2001 From: Wibias <37517432+Wibias@users.noreply.github.com> Date: Fri, 24 Jul 2026 20:35:59 +0200 Subject: [PATCH 2/5] fix(ci): address issue-quality review feedback - distinguish blank example sections from explicit placeholder tokens - scope workflow issues:write permission to the validate job - validate workflow_dispatch issue_number as a positive safe integer - add regression test for blank example usage --- .github/scripts/issue-quality.cjs | 19 +++++++++----- .github/scripts/issue-quality.test.cjs | 17 +++++++++++++ .github/workflows/enforce-issue-quality.yml | 28 +++++++++++++++------ 3 files changed, 51 insertions(+), 13 deletions(-) diff --git a/.github/scripts/issue-quality.cjs b/.github/scripts/issue-quality.cjs index 169c981d6..d5456f9b7 100644 --- a/.github/scripts/issue-quality.cjs +++ b/.github/scripts/issue-quality.cjs @@ -330,10 +330,11 @@ function allRepeatTitle(sections, title) { } function isPlaceholder(text) { - const c = clean(text); - if (!c) return true; - const lower = c.toLowerCase(); - return ( + if (typeof text !== "string") return false; + const trimmed = text.trim(); + if (!trimmed) return false; + const lower = trimmed.toLowerCase(); + if ( lower === "no response" || lower === "na" || lower === "n/a" || @@ -342,8 +343,14 @@ function isPlaceholder(text) { lower === "none" || lower === "todo" || lower === "tbd" || - /^_no response_$/i.test(c) - ); + /^_no response_$/i.test(trimmed) + ) { + return true; + } + const c = clean(text); + if (!c) return false; + const cleanedLower = c.toLowerCase(); + return cleanedLower === "none" || cleanedLower === "todo" || cleanedLower === "tbd"; } function countWords(text) { diff --git a/.github/scripts/issue-quality.test.cjs b/.github/scripts/issue-quality.test.cjs index 1322aef79..29c1ad4cc 100644 --- a/.github/scripts/issue-quality.test.cjs +++ b/.github/scripts/issue-quality.test.cjs @@ -203,6 +203,23 @@ describe("validateIssue - feature", () => { assert.ok(result.reasons.some((r) => r.includes("placeholder"))); }); + it("reports blank example usage as missing, not placeholder", () => { + const body = [ + "### What are you trying to accomplish?", + "Route voice requests to a configured fallback provider when the primary quota is exhausted.", + "### What prevents this today?", + "Voice mode is hard-wired to the primary Codex quota and cannot switch providers.", + "### What should OpenCodex do?", + "Expose a setting to choose the fallback voice model and provider.", + "### Example usage or interface", + "", + ].join("\n"); + const result = validateIssue({ title: "Voice fallback routing", body, labels: ["enhancement"] }); + assert.equal(result.valid, false); + assert.ok(result.reasons.some((r) => r.includes("example usage"))); + assert.ok(!result.reasons.some((r) => r.includes("placeholder"))); + }); + it("accepts a valid legacy feature request without blocker/example headings", () => { const body = [ "### Problem to solve", diff --git a/.github/workflows/enforce-issue-quality.yml b/.github/workflows/enforce-issue-quality.yml index 8c1cb2a34..f1f56a1cd 100644 --- a/.github/workflows/enforce-issue-quality.yml +++ b/.github/workflows/enforce-issue-quality.yml @@ -17,10 +17,6 @@ on: required: true type: number -permissions: - contents: read - issues: write - concurrency: group: issue-quality-${{ github.event.issue.number || inputs.issue_number }} cancel-in-progress: true @@ -28,6 +24,10 @@ concurrency: jobs: validate: runs-on: ubuntu-latest + permissions: + contents: read + # Required for issue closure, reopen, and bot comments. + issues: write steps: - name: Checkout trusted workflow code @@ -51,9 +51,23 @@ jobs: } = require(path.join(process.cwd(), ".github", "scripts", "issue-quality.cjs")); const { owner, repo } = context.repo; - const issue_number = context.eventName === "workflow_dispatch" - ? Number(context.payload.inputs.issue_number) - : context.payload.issue.number; + let issue_number; + if (context.eventName === "workflow_dispatch") { + const rawIssueNumber = context.payload.inputs.issue_number; + const parsedIssueNumber = Number(rawIssueNumber); + if ( + !Number.isSafeInteger(parsedIssueNumber) || + parsedIssueNumber <= 0 + ) { + core.setFailed( + `Invalid workflow_dispatch issue_number: ${rawIssueNumber}. Expected a positive integer.`, + ); + return; + } + issue_number = parsedIssueNumber; + } else { + issue_number = context.payload.issue.number; + } const actor = context.actor; const eventType = context.eventName === "issues" ? context.payload.action From 70bb34ba0d29cdaf401f797b591e9ca66e2d32e7 Mon Sep 17 00:00:00 2001 From: Wibias <37517432+Wibias@users.noreply.github.com> Date: Fri, 24 Jul 2026 23:51:21 +0200 Subject: [PATCH 3/5] fix(ci): harden issue-quality terseness and workflow_dispatch PR guard --- .github/scripts/issue-quality.cjs | 113 +++++++---- .github/scripts/issue-quality.test.cjs | 205 ++++++++++++++++++-- .github/workflows/enforce-issue-quality.yml | 29 +++ tests/ci-workflows.test.ts | 56 ++++++ 4 files changed, 357 insertions(+), 46 deletions(-) diff --git a/.github/scripts/issue-quality.cjs b/.github/scripts/issue-quality.cjs index d5456f9b7..a3cfceb45 100644 --- a/.github/scripts/issue-quality.cjs +++ b/.github/scripts/issue-quality.cjs @@ -6,14 +6,32 @@ // --------------------------------------------------------------------------- /** - * Strip HTML comments, "No response" placeholders, and trim whitespace. + * True when the entire meaningful value is a placeholder-only token. + * Supports harmless Markdown emphasis/code markers and trailing punctuation. + * Sentences that merely contain a placeholder phrase are not matches. + */ +const PLACEHOLDER_ONLY_RE = + /^[\s_*~`]*(?:no\s+response|n\/?a|not\s+applicable|not\s+available|none|todo|tbd)[\s_*~`]*[.!?]*$/i; + +function isPlaceholderOnlyValue(raw) { + if (typeof raw !== "string") return false; + const withoutComments = raw.replace(//g, "").trim(); + if (!withoutComments) return false; + return PLACEHOLDER_ONLY_RE.test(withoutComments); +} + +/** + * Strip HTML comments, placeholder-only values, and trim whitespace. */ function clean(raw) { if (typeof raw !== "string") return ""; let s = raw.replace(//g, ""); - // Treat GitHub's "No response" placeholder as empty. - s = s.replace(/^[\s_*]*No response[\s_*]*$/gim, ""); - s = s.replace(/^[\s_*]*(na|n\/a|not applicable|not available)[\s_*]*$/gim, ""); + // Treat placeholder-only lines (GitHub "No response", N/A, etc.) as empty. + s = s + .split("\n") + .map((line) => (isPlaceholderOnlyValue(line) ? "" : line)) + .join("\n"); + if (isPlaceholderOnlyValue(s)) return ""; return s.trim(); } @@ -330,35 +348,19 @@ function allRepeatTitle(sections, title) { } function isPlaceholder(text) { - if (typeof text !== "string") return false; - const trimmed = text.trim(); - if (!trimmed) return false; - const lower = trimmed.toLowerCase(); - if ( - lower === "no response" || - lower === "na" || - lower === "n/a" || - lower === "not applicable" || - lower === "not available" || - lower === "none" || - lower === "todo" || - lower === "tbd" || - /^_no response_$/i.test(trimmed) - ) { - return true; - } - const c = clean(text); - if (!c) return false; - const cleanedLower = c.toLowerCase(); - return cleanedLower === "none" || cleanedLower === "todo" || cleanedLower === "tbd"; + return isPlaceholderOnlyValue(text); } function countWords(text) { const c = clean(text); if (!c) return 0; const spaced = (c.match(/\b[\p{L}\p{N}']+\b/gu) || []).length; - if (spaced > 0) return spaced; - return (c.match(/\p{L}/gu) || []).length; + const letters = (c.match(/\p{L}/gu) || []).length; + if (spaced === 0) return letters; + if (letters >= 8 && /[\p{Script=Han}\p{Script=Hiragana}\p{Script=Katakana}\p{Script=Hangul}]/u.test(c) && spaced < letters) { + return letters; + } + return spaced; } function hasConcreteDetail(text) { @@ -376,19 +378,17 @@ function isTooTerseFeatureSection(text) { const words = countWords(text); if (words >= 8) return false; if (words >= 6 && hasConcreteDetail(text)) return false; - return !hasConcreteDetail(text); + return true; } /** - * Check if raw section text is a GitHub "No response" placeholder variant - * without stripping it first. Used to distinguish intentionally blank optional - * fields from actively cleared required fields. + * Check if raw section text is a placeholder-only variant without relying on + * clean() first. Used to distinguish intentionally blank optional fields + * (legacy "No response" / N/A) from actively cleared required fields. */ function isRawPlaceholder(raw) { if (raw === null) return false; - const trimmed = raw.trim(); - if (!trimmed) return false; - return /^[\s_*]*no response[\s_*]*$/i.test(trimmed); + return isPlaceholderOnlyValue(raw); } /** @@ -638,6 +638,47 @@ function shouldReopen(botState, issue, maintainerOverride) { return true; } +/** + * workflow_dispatch accepts a bare issue number, but GitHub reuses the same + * number namespace for issues and pull requests. Reject PR targets before any + * validation or mutation runs. + * + * @param {{ pull_request?: unknown }} issue + * @param {number|string} issueNumber + * @param {string} eventName + * @returns {string|null} + */ +function rejectsWorkflowDispatchPullRequest(issue, issueNumber, eventName) { + if (eventName !== "workflow_dispatch") return null; + if (!issue?.pull_request) return null; + return `#${issueNumber} is a pull request. This workflow only accepts issue numbers.`; +} + +/** + * workflow_dispatch can be started from a selected branch. Reject runs whose + * selected ref is not the repository default branch so untrusted branch code + * cannot drive issue mutations with issues:write. + * + * @param {string} eventName + * @param {string|null|undefined} ref + * @param {string|null|undefined} defaultBranch + * @returns {string|null} + */ +function rejectsWorkflowDispatchNonDefaultBranch(eventName, ref, defaultBranch) { + if (eventName !== "workflow_dispatch") return null; + if (!defaultBranch || typeof defaultBranch !== "string") { + return "workflow_dispatch requires repository.default_branch to be available."; + } + const expected = `refs/heads/${defaultBranch}`; + if (ref !== expected) { + return ( + `workflow_dispatch must run from the default branch (${defaultBranch}); ` + + `selected ref was ${ref || "(empty)"}.` + ); + } + return null; +} + // --------------------------------------------------------------------------- // Exports // --------------------------------------------------------------------------- @@ -652,8 +693,12 @@ module.exports = { validateIssue, shouldReopen, shouldEnforceClosure, + isPlaceholderOnlyValue, + isPlaceholder, isRawPlaceholder, labelForKind, KIND_TO_LABEL, hasSubstantialStructuredContent, + rejectsWorkflowDispatchPullRequest, + rejectsWorkflowDispatchNonDefaultBranch, }; diff --git a/.github/scripts/issue-quality.test.cjs b/.github/scripts/issue-quality.test.cjs index 29c1ad4cc..c32760181 100644 --- a/.github/scripts/issue-quality.test.cjs +++ b/.github/scripts/issue-quality.test.cjs @@ -12,8 +12,28 @@ const { shouldReopen, shouldEnforceClosure, labelForKind, + isPlaceholderOnlyValue, + isPlaceholder, + isRawPlaceholder, + rejectsWorkflowDispatchPullRequest, + rejectsWorkflowDispatchNonDefaultBranch, } = require("./issue-quality.cjs"); +function featureBodyWithGoal(goal) { + return [ + "### Area", + "CLI", + "### What are you trying to accomplish?", + goal, + "### What prevents this today?", + "Port resets to 10100 after every ocx stop command.", + "### What should OpenCodex do?", + "Persist the last used port in config across restarts.", + "### Example usage or interface", + "ocx start --port 8080 && ocx stop && ocx start", + ].join("\n"); +} + // --------------------------------------------------------------------------- // Detection // --------------------------------------------------------------------------- @@ -188,19 +208,35 @@ describe("validateIssue - feature", () => { }); it("rejects feature reports with placeholder example variants", () => { - const body = [ - "### What are you trying to accomplish?", - "Route voice requests to a configured fallback provider when the primary quota is exhausted.", - "### What prevents this today?", - "Voice mode is hard-wired to the primary Codex quota and cannot switch providers.", - "### What should OpenCodex do?", - "Expose a setting to choose the fallback voice model and provider.", - "### Example usage or interface", + const placeholders = [ + "NA", "N/A", - ].join("\n"); - const result = validateIssue({ title: "Voice fallback routing", body, labels: ["enhancement"] }); - assert.equal(result.valid, false); - assert.ok(result.reasons.some((r) => r.includes("placeholder"))); + "_N/A_", + "NA.", + "N/A.", + "Not applicable", + "Not applicable.", + "Not available!", + ]; + for (const example of placeholders) { + const body = [ + "### What are you trying to accomplish?", + "Route voice requests to a configured fallback provider when the primary quota is exhausted.", + "### What prevents this today?", + "Voice mode is hard-wired to the primary Codex quota and cannot switch providers.", + "### What should OpenCodex do?", + "Expose a setting to choose the fallback voice model and provider.", + "### Example usage or interface", + example, + ].join("\n"); + const result = validateIssue({ title: "Voice fallback routing", body, labels: ["enhancement"] }); + assert.equal(result.valid, false, `Expected placeholder example "${example}" to be invalid`); + assert.ok( + result.reasons.some((r) => r.includes("placeholder")), + `Expected placeholder reason for "${example}", got: ${result.reasons.join("; ")}`, + ); + assert.ok(!result.reasons.some((r) => r.includes("example usage"))); + } }); it("reports blank example usage as missing, not placeholder", () => { @@ -249,6 +285,44 @@ describe("validateIssue - feature", () => { assert.equal(result.kind, "feature"); assert.equal(result.valid, true); }); + + it("rejects terse goal sections that only contain a keyword, digit, or punctuation", () => { + const terseGoals = ["API", "provider", "1", "/", "use CLI", "route 1"]; + for (const goal of terseGoals) { + const result = validateIssue({ + title: "Improve feature request quality", + body: featureBodyWithGoal(goal), + labels: ["enhancement"], + }); + assert.equal(result.kind, "feature"); + assert.equal(result.valid, false, `Expected terse goal "${goal}" to be invalid`); + assert.ok( + result.reasons.some((r) => r.includes("too vague")), + `Expected too vague reason for "${goal}", got: ${result.reasons.join(", ")}`, + ); + } + }); + + it("accepts sufficiently detailed goal sections", () => { + const detailedGoal = + "Expose a dashboard setting to choose the fallback voice model and provider."; + const detailedResult = validateIssue({ + title: "Voice fallback routing", + body: featureBodyWithGoal(detailedGoal), + labels: ["enhancement"], + }); + assert.equal(detailedResult.kind, "feature"); + assert.equal(detailedResult.valid, true); + + const concreteGoal = "Route voice requests through the configured fallback API provider."; + const concreteResult = validateIssue({ + title: "Voice fallback routing", + body: featureBodyWithGoal(concreteGoal), + labels: ["enhancement"], + }); + assert.equal(concreteResult.kind, "feature"); + assert.equal(concreteResult.valid, true); + }); }); // --------------------------------------------------------------------------- @@ -328,6 +402,29 @@ describe("validateIssue - bug", () => { assert.equal(result.valid, true, `Expected valid but got: ${result.reasons.join(", ")}`); }); + it("accepts a legacy bug with N/A-style placeholders in Version and Operating system", () => { + for (const placeholder of ["N/A", "NA", "Not applicable.", "Not available!"]) { + const body = [ + "### Summary", + "Proxy crashes on startup when streaming is enabled.", + "### Reproduction", + "Run ocx start and send any streaming request.", + "### Version", + placeholder, + "### Operating system", + placeholder, + ].join("\n"); + const result = validateIssue({ title: "[Bug]: crash", body, labels: ["bug"] }); + assert.equal(result.kind, "bug"); + assert.equal( + result.valid, + true, + `Expected legacy env placeholder "${placeholder}" to remain valid, got: ${result.reasons.join(", ")}`, + ); + assert.ok(!result.reasons.some((r) => r.includes("Version"))); + } + }); + it("rejects a new-form bug where env fields were actively cleared", () => { const body = [ "### Client or integration", @@ -483,7 +580,45 @@ describe("normalisation", () => { it("treats NA and not applicable as placeholders", () => { assert.equal(clean("NA"), ""); + assert.equal(clean("N/A"), ""); + assert.equal(clean("_N/A_"), ""); + assert.equal(clean("NA."), ""); + assert.equal(clean("N/A."), ""); assert.equal(clean("not applicable"), ""); + assert.equal(clean("Not applicable."), ""); + assert.equal(clean("Not available!"), ""); + }); + + it("does not treat sentences containing placeholder phrases as empty", () => { + assert.equal(clean("This is N/A for voice mode today."), "This is N/A for voice mode today."); + assert.equal(clean("Not applicable to Claude Code."), "Not applicable to Claude Code."); + }); + + it("shares one placeholder matcher across clean, isPlaceholder, and isRawPlaceholder", () => { + const placeholders = [ + "No response", + "NA", + "N/A", + "_N/A_", + "NA.", + "N/A.", + "None", + "Todo", + "TBD", + "Not applicable", + "Not applicable.", + "Not available!", + ]; + for (const value of placeholders) { + assert.equal(isPlaceholderOnlyValue(value), true, value); + assert.equal(isPlaceholder(value), true, value); + assert.equal(isRawPlaceholder(value), true, value); + assert.equal(clean(value), "", value); + } + assert.equal(isPlaceholderOnlyValue("Route voice traffic to provider N/A fallback"), false); + assert.equal(isPlaceholder("use CLI"), false); + assert.equal(isRawPlaceholder(""), false); + assert.equal(isRawPlaceholder(null), false); }); it("strips HTML comments", () => { @@ -812,3 +947,49 @@ describe("labelForKind", () => { assert.equal(labelForKind("unknown"), null); }); }); + +// --------------------------------------------------------------------------- +// workflow_dispatch guards +// --------------------------------------------------------------------------- + +describe("rejectsWorkflowDispatchPullRequest", () => { + it("rejects pull request numbers on workflow_dispatch", () => { + assert.equal( + rejectsWorkflowDispatchPullRequest({ pull_request: {} }, 423, "workflow_dispatch"), + "#423 is a pull request. This workflow only accepts issue numbers.", + ); + }); + + it("allows issues and non-dispatch events", () => { + assert.equal(rejectsWorkflowDispatchPullRequest({ pull_request: {} }, 423, "issues"), null); + assert.equal(rejectsWorkflowDispatchPullRequest({}, 42, "workflow_dispatch"), null); + }); +}); + +describe("rejectsWorkflowDispatchNonDefaultBranch", () => { + it("rejects workflow_dispatch runs that are not on the default branch", () => { + assert.equal( + rejectsWorkflowDispatchNonDefaultBranch( + "workflow_dispatch", + "refs/heads/fix/issue-quality-low-effort-reports", + "main", + ), + "workflow_dispatch must run from the default branch (main); selected ref was refs/heads/fix/issue-quality-low-effort-reports.", + ); + }); + + it("allows default-branch dispatches and normal issue events", () => { + assert.equal( + rejectsWorkflowDispatchNonDefaultBranch("workflow_dispatch", "refs/heads/main", "main"), + null, + ); + assert.equal( + rejectsWorkflowDispatchNonDefaultBranch( + "issues", + "refs/heads/fix/issue-quality-low-effort-reports", + "main", + ), + null, + ); + }); +}); \ No newline at end of file diff --git a/.github/workflows/enforce-issue-quality.yml b/.github/workflows/enforce-issue-quality.yml index f1f56a1cd..3cec9fa5b 100644 --- a/.github/workflows/enforce-issue-quality.yml +++ b/.github/workflows/enforce-issue-quality.yml @@ -33,6 +33,10 @@ jobs: - name: Checkout trusted workflow code uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 with: + # Always load validator scripts from the repository default branch so + # a branch-selected workflow_dispatch cannot execute untrusted code + # with issues:write. + ref: ${{ github.event.repository.default_branch }} persist-credentials: false sparse-checkout: .github/scripts @@ -48,6 +52,8 @@ jobs: shouldReopen, shouldEnforceClosure, labelForKind, + rejectsWorkflowDispatchPullRequest, + rejectsWorkflowDispatchNonDefaultBranch, } = require(path.join(process.cwd(), ".github", "scripts", "issue-quality.cjs")); const { owner, repo } = context.repo; @@ -73,6 +79,19 @@ jobs: ? context.payload.action : ""; + // ----------------------------------------------------------------- + // Reject branch-selected manual dispatches before any issue API use + // ----------------------------------------------------------------- + const nonDefaultBranchFailure = rejectsWorkflowDispatchNonDefaultBranch( + context.eventName, + context.ref, + context.payload.repository?.default_branch, + ); + if (nonDefaultBranchFailure) { + core.setFailed(nonDefaultBranchFailure); + return; + } + // ----------------------------------------------------------------- // Fetch live issue state // ----------------------------------------------------------------- @@ -80,6 +99,16 @@ jobs: owner, repo, issue_number, }); + const pullRequestFailure = rejectsWorkflowDispatchPullRequest( + issue, + issue_number, + context.eventName, + ); + if (pullRequestFailure) { + core.setFailed(pullRequestFailure); + return; + } + // ----------------------------------------------------------------- // Trusted-user exemption // ----------------------------------------------------------------- diff --git a/tests/ci-workflows.test.ts b/tests/ci-workflows.test.ts index cb06538c7..c5dfee63e 100644 --- a/tests/ci-workflows.test.ts +++ b/tests/ci-workflows.test.ts @@ -164,6 +164,62 @@ describe("GitHub Actions hardening", () => { expect(workflow).not.toMatch(/uses:\s+\S+@(?:v\d+|main|master)\b/); }); + test("issue-quality workflow rejects workflow_dispatch pull request numbers before mutation", async () => { + const workflow = await readText(".github/workflows/enforce-issue-quality.yml"); + + // Manual dispatch is supported, but only with a positive issue number. + expect(workflow).toContain("workflow_dispatch:"); + expect(workflow).toContain("issue_number:"); + expect(workflow).toContain("Number.isSafeInteger(parsedIssueNumber)"); + expect(workflow).toContain("parsedIssueNumber <= 0"); + + // Job-scoped permissions only (no top-level issues:write). + expect(workflow).toMatch( + /jobs:\s*\n\s*validate:[\s\S]*?permissions:\s*\n\s*contents: read\s*\n\s*#.*\n\s*issues: write/, + ); + const beforeJobs = workflow.split(/jobs:\s*\n/)[0]!; + expect(beforeJobs).not.toMatch(/^\s*permissions:/m); + + // Trusted scripts always come from the repository default branch. + const checkoutStep = workflow + .split("- name: Checkout trusted workflow code")[1]! + .split(/\n {6}- name:/)[0]!; + expect(checkoutStep).toContain("ref: ${{ github.event.repository.default_branch }}"); + expect(checkoutStep).toContain("persist-credentials: false"); + expect(checkoutStep).toContain("sparse-checkout: .github/scripts"); + + const script = workflow + .split("script: |")[1]! + .split(/\n {6}- name:/)[0]!; + + // Invalid issue numbers fail before any issues API call. + const invalidNumberIdx = script.indexOf("Invalid workflow_dispatch issue_number:"); + const firstIssuesGetIdx = script.indexOf("github.rest.issues.get({"); + expect(invalidNumberIdx).toBeGreaterThan(-1); + expect(firstIssuesGetIdx).toBeGreaterThan(-1); + expect(invalidNumberIdx).toBeLessThan(firstIssuesGetIdx); + + // Non-default-branch dispatches fail before any issues API mutation. + const branchGuardIdx = script.indexOf("const nonDefaultBranchFailure = rejectsWorkflowDispatchNonDefaultBranch("); + const firstMutationIdx = script.indexOf("github.rest.issues.update({"); + expect(branchGuardIdx).toBeGreaterThan(-1); + expect(firstMutationIdx).toBeGreaterThan(-1); + expect(branchGuardIdx).toBeLessThan(firstMutationIdx); + expect(branchGuardIdx).toBeLessThan(firstIssuesGetIdx); + + // Pull-request numbers are rejected after issues.get and before mutations. + const prGuardIdx = script.indexOf("const pullRequestFailure = rejectsWorkflowDispatchPullRequest("); + const listCommentsIdx = script.indexOf("github.rest.issues.listComments"); + const addLabelsIdx = script.indexOf("github.rest.issues.addLabels"); + expect(prGuardIdx).toBeGreaterThan(-1); + expect(prGuardIdx).toBeGreaterThan(firstIssuesGetIdx); + expect(prGuardIdx).toBeLessThan(listCommentsIdx); + expect(prGuardIdx).toBeLessThan(addLabelsIdx); + expect(prGuardIdx).toBeLessThan(firstMutationIdx); + expect(script).toContain("if (pullRequestFailure) {"); + expect(script).toContain("core.setFailed(pullRequestFailure);"); + }); + test("React Doctor workflow is SHA-pinned, engine-pinned, advisory, and read-only", async () => { const workflow = await readText(".github/workflows/react-doctor.yml"); From 158b0f31b22f6c36a306e945fac63ed106b76ff0 Mon Sep 17 00:00:00 2001 From: Wibias <37517432+Wibias@users.noreply.github.com> Date: Sat, 25 Jul 2026 01:11:18 +0200 Subject: [PATCH 4/5] fix(ci): tighten issue-quality word count and fenced placeholders Count Cyrillic/Greek/Arabic as real word tokens, reject fenced N/A-only examples, and cover the true 6-7 word concrete-detail boundary. --- .github/scripts/issue-quality.cjs | 47 ++++++++-- .github/scripts/issue-quality.test.cjs | 121 ++++++++++++++++++++++++- 2 files changed, 159 insertions(+), 9 deletions(-) diff --git a/.github/scripts/issue-quality.cjs b/.github/scripts/issue-quality.cjs index a3cfceb45..8b18e6486 100644 --- a/.github/scripts/issue-quality.cjs +++ b/.github/scripts/issue-quality.cjs @@ -13,11 +13,32 @@ const PLACEHOLDER_ONLY_RE = /^[\s_*~`]*(?:no\s+response|n\/?a|not\s+applicable|not\s+available|none|todo|tbd)[\s_*~`]*[.!?]*$/i; +/** + * If `text` is exactly one enclosing fenced code block (``` or ~~~), return the + * inner body; otherwise null. Real multi-statement fences are left alone by + * the placeholder matcher after unwrap. + */ +function unwrapSingleEnclosingFence(text) { + const trimmed = text.trim(); + const match = trimmed.match(/^(```|~~~)[^\n]*\r?\n([\s\S]*?)\r?\n\1[ \t]*$/); + if (!match) return null; + return match[2]; +} + function isPlaceholderOnlyValue(raw) { if (typeof raw !== "string") return false; - const withoutComments = raw.replace(//g, "").trim(); - if (!withoutComments) return false; - return PLACEHOLDER_ONLY_RE.test(withoutComments); + let value = raw.replace(//g, "").trim(); + if (!value) return false; + + // A lone fenced block whose entire body is a placeholder is still placeholder + // text (e.g. ```text\nN/A\n```), not a real example. + const unwrapped = unwrapSingleEnclosingFence(value); + if (unwrapped !== null) { + value = unwrapped.trim(); + if (!value) return false; + } + + return PLACEHOLDER_ONLY_RE.test(value); } /** @@ -26,6 +47,9 @@ function isPlaceholderOnlyValue(raw) { function clean(raw) { if (typeof raw !== "string") return ""; let s = raw.replace(//g, ""); + // Whole-value placeholders first (including a single enclosing fence), so + // line-by-line stripping cannot leave bare fence markers behind. + if (isPlaceholderOnlyValue(s)) return ""; // Treat placeholder-only lines (GitHub "No response", N/A, etc.) as empty. s = s .split("\n") @@ -354,13 +378,20 @@ function isPlaceholder(text) { function countWords(text) { const c = clean(text); if (!c) return 0; - const spaced = (c.match(/\b[\p{L}\p{N}']+\b/gu) || []).length; + + // Use Unicode letter/number tokens so Cyrillic/Greek/Arabic words count as + // one token each. `\b` is ASCII-only and would miss those scripts. + const tokens = c.match(/[\p{L}\p{N}']+/gu) || []; const letters = (c.match(/\p{L}/gu) || []).length; - if (spaced === 0) return letters; - if (letters >= 8 && /[\p{Script=Han}\p{Script=Hiragana}\p{Script=Katakana}\p{Script=Hangul}]/u.test(c) && spaced < letters) { + const containsCjk = + /[\p{Script=Han}\p{Script=Hiragana}\p{Script=Katakana}\p{Script=Hangul}]/u.test(c); + + // CJK often has no whitespace-separated words; letter count is the detail proxy. + if (containsCjk && letters >= 8) { return letters; } - return spaced; + + return tokens.length; } function hasConcreteDetail(text) { @@ -696,6 +727,8 @@ module.exports = { isPlaceholderOnlyValue, isPlaceholder, isRawPlaceholder, + countWords, + hasConcreteDetail, labelForKind, KIND_TO_LABEL, hasSubstantialStructuredContent, diff --git a/.github/scripts/issue-quality.test.cjs b/.github/scripts/issue-quality.test.cjs index c32760181..696dbcb25 100644 --- a/.github/scripts/issue-quality.test.cjs +++ b/.github/scripts/issue-quality.test.cjs @@ -15,6 +15,8 @@ const { isPlaceholderOnlyValue, isPlaceholder, isRawPlaceholder, + countWords, + hasConcreteDetail, rejectsWorkflowDispatchPullRequest, rejectsWorkflowDispatchNonDefaultBranch, } = require("./issue-quality.cjs"); @@ -34,6 +36,19 @@ function featureBodyWithGoal(goal) { ].join("\n"); } +function featureBodyWithExample(example) { + return [ + "### What are you trying to accomplish?", + "Route voice requests to a configured fallback provider when the primary quota is exhausted.", + "### What prevents this today?", + "Voice mode is hard-wired to the primary Codex quota and cannot switch providers.", + "### What should OpenCodex do?", + "Expose a setting to choose the fallback voice model and provider.", + "### Example usage or interface", + example, + ].join("\n"); +} + // --------------------------------------------------------------------------- // Detection // --------------------------------------------------------------------------- @@ -303,6 +318,29 @@ describe("validateIssue - feature", () => { } }); + it("rejects a single long non-CJK word as overly terse", () => { + const terseGoals = [ + "провайдер", + "маршрут", + "πάροχος", + "واجهة", + ]; + for (const goal of terseGoals) { + assert.equal(countWords(goal), 1, `Expected "${goal}" to count as one word`); + const result = validateIssue({ + title: "Improve feature request quality", + body: featureBodyWithGoal(goal), + labels: ["enhancement"], + }); + assert.equal(result.kind, "feature"); + assert.equal(result.valid, false, `Expected terse goal "${goal}" to be invalid`); + assert.ok( + result.reasons.some((r) => r.includes("too vague")), + `Expected too vague reason for "${goal}", got: ${result.reasons.join(", ")}`, + ); + } + }); + it("accepts sufficiently detailed goal sections", () => { const detailedGoal = "Expose a dashboard setting to choose the fallback voice model and provider."; @@ -313,8 +351,16 @@ describe("validateIssue - feature", () => { }); assert.equal(detailedResult.kind, "feature"); assert.equal(detailedResult.valid, true); + assert.ok(countWords(detailedGoal) >= 8); + }); + + it("accepts a 6-7 word goal when it includes concrete technical detail", () => { + const concreteGoal = "Route requests through the configured API provider."; + assert.equal(countWords(concreteGoal), 7); + assert.ok(countWords(concreteGoal) < 8); + assert.ok(countWords(concreteGoal) >= 6); + assert.equal(hasConcreteDetail(concreteGoal), true); - const concreteGoal = "Route voice requests through the configured fallback API provider."; const concreteResult = validateIssue({ title: "Voice fallback routing", body: featureBodyWithGoal(concreteGoal), @@ -323,8 +369,73 @@ describe("validateIssue - feature", () => { assert.equal(concreteResult.kind, "feature"); assert.equal(concreteResult.valid, true); }); -}); + it("rejects a 6-7 word goal that lacks concrete technical detail", () => { + // Avoid keywords that count as concrete detail (api/provider/workflow/…). + const vagueGoal = "Make this process easier for all users."; + assert.equal(countWords(vagueGoal), 7); + assert.equal(hasConcreteDetail(vagueGoal), false); + + const vagueResult = validateIssue({ + title: "Voice fallback routing", + body: featureBodyWithGoal(vagueGoal), + labels: ["enhancement"], + }); + assert.equal(vagueResult.kind, "feature"); + assert.equal(vagueResult.valid, false); + assert.ok(vagueResult.reasons.some((r) => r.includes("too vague"))); + }); + + it("rejects fenced placeholder-only examples", () => { + const fencedPlaceholders = [ + "```\nN/A\n```", + "```text\nN/A\n```", + "```json\nNot applicable.\n```", + "~~~text\nN/A\n~~~", + ]; + for (const example of fencedPlaceholders) { + assert.equal( + isPlaceholderOnlyValue(example), + true, + `Expected fenced placeholder to match: ${JSON.stringify(example)}`, + ); + const result = validateIssue({ + title: "Voice fallback routing", + body: featureBodyWithExample(example), + labels: ["enhancement"], + }); + assert.equal(result.valid, false, `Expected fenced placeholder to be invalid: ${JSON.stringify(example)}`); + assert.ok( + result.reasons.some((r) => r.includes("placeholder")), + `Expected placeholder reason for ${JSON.stringify(example)}, got: ${result.reasons.join("; ")}`, + ); + } + }); + + it("accepts real fenced examples that merely mention N/A", () => { + const realExamples = [ + "```text\nThe API returns N/A when no provider is configured.\n```", + '```json\n{"provider":"N/A","fallback":"anthropic"}\n```', + ]; + for (const example of realExamples) { + assert.equal( + isPlaceholderOnlyValue(example), + false, + `Expected real example not to be placeholder-only: ${JSON.stringify(example)}`, + ); + const result = validateIssue({ + title: "Voice fallback routing", + body: featureBodyWithExample(example), + labels: ["enhancement"], + }); + assert.equal( + result.valid, + true, + `Expected real fenced example to remain valid, got: ${result.reasons.join("; ")}`, + ); + } + }); +}); // --------------------------------------------------------------------------- // Validation: bug // --------------------------------------------------------------------------- @@ -608,6 +719,10 @@ describe("normalisation", () => { "Not applicable", "Not applicable.", "Not available!", + "```\nN/A\n```", + "```text\nN/A\n```", + "```json\nNot applicable.\n```", + "~~~text\nN/A\n~~~", ]; for (const value of placeholders) { assert.equal(isPlaceholderOnlyValue(value), true, value); @@ -616,6 +731,8 @@ describe("normalisation", () => { assert.equal(clean(value), "", value); } assert.equal(isPlaceholderOnlyValue("Route voice traffic to provider N/A fallback"), false); + assert.equal(isPlaceholderOnlyValue("```text\nThe API returns N/A when no provider is configured.\n```"), false); + assert.equal(isPlaceholderOnlyValue('```json\n{"provider":"N/A","fallback":"anthropic"}\n```'), false); assert.equal(isPlaceholder("use CLI"), false); assert.equal(isRawPlaceholder(""), false); assert.equal(isRawPlaceholder(null), false); From af28e37c7495565170f1509de16414f9b1e47e6d Mon Sep 17 00:00:00 2001 From: Wibias <37517432+Wibias@users.noreply.github.com> Date: Sat, 25 Jul 2026 01:48:18 +0200 Subject: [PATCH 5/5] fix(ci): stop mixed-script CJK from inflating word counts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Count CJK glyphs separately from non-CJK tokens so stubs like provider中 cannot bypass the feature terseness check. --- .github/scripts/issue-quality.cjs | 22 ++++++++++------------ .github/scripts/issue-quality.test.cjs | 25 +++++++++++++++++++++++++ 2 files changed, 35 insertions(+), 12 deletions(-) diff --git a/.github/scripts/issue-quality.cjs b/.github/scripts/issue-quality.cjs index 8b18e6486..2f7264cba 100644 --- a/.github/scripts/issue-quality.cjs +++ b/.github/scripts/issue-quality.cjs @@ -375,23 +375,21 @@ function isPlaceholder(text) { return isPlaceholderOnlyValue(text); } +const CJK_RE = + /[\p{Script=Han}\p{Script=Hiragana}\p{Script=Katakana}\p{Script=Hangul}]/gu; + function countWords(text) { const c = clean(text); if (!c) return 0; - // Use Unicode letter/number tokens so Cyrillic/Greek/Arabic words count as - // one token each. `\b` is ASCII-only and would miss those scripts. - const tokens = c.match(/[\p{L}\p{N}']+/gu) || []; - const letters = (c.match(/\p{L}/gu) || []).length; - const containsCjk = - /[\p{Script=Han}\p{Script=Hiragana}\p{Script=Katakana}\p{Script=Hangul}]/u.test(c); - - // CJK often has no whitespace-separated words; letter count is the detail proxy. - if (containsCjk && letters >= 8) { - return letters; - } + // Count each CJK character as one unit, and non-CJK scripts as Unicode + // word tokens. Mixing one CJK glyph into a Latin/Cyrillic word must not + // inflate the count to letter-length. + const cjkChars = c.match(CJK_RE) || []; + const nonCjkText = c.replace(CJK_RE, " "); + const nonCjkTokens = nonCjkText.match(/[\p{L}\p{N}']+/gu) || []; - return tokens.length; + return cjkChars.length + nonCjkTokens.length; } function hasConcreteDetail(text) { diff --git a/.github/scripts/issue-quality.test.cjs b/.github/scripts/issue-quality.test.cjs index 696dbcb25..ee27b34f1 100644 --- a/.github/scripts/issue-quality.test.cjs +++ b/.github/scripts/issue-quality.test.cjs @@ -341,6 +341,31 @@ describe("validateIssue - feature", () => { } }); + it("counts mixed-script CJK text without inflating non-CJK letter length", () => { + assert.equal(countWords("provider中"), 2); + assert.equal(countWords("провайдер中"), 2); + assert.equal(countWords("configuration中"), 2); + assert.equal(countWords("中provider文"), 3); + }); + + it("rejects mixed-script CJK stubs that only inflate letter counts", () => { + const terseGoals = ["provider中", "провайдер中", "configuration中"]; + for (const goal of terseGoals) { + assert.ok(countWords(goal) < 8, `Expected "${goal}" to stay under 8 units`); + const result = validateIssue({ + title: "Improve feature request quality", + body: featureBodyWithGoal(goal), + labels: ["enhancement"], + }); + assert.equal(result.kind, "feature"); + assert.equal(result.valid, false, `Expected terse goal "${goal}" to be invalid`); + assert.ok( + result.reasons.some((r) => r.includes("too vague")), + `Expected too vague reason for "${goal}", got: ${result.reasons.join(", ")}`, + ); + } + }); + it("accepts sufficiently detailed goal sections", () => { const detailedGoal = "Expose a dashboard setting to choose the fallback voice model and provider.";