From 5f32e8e1db0511712daf0a68dcefb268e9cda9da Mon Sep 17 00:00:00 2001 From: Jinjing <6427696+AmethystLiang@users.noreply.github.com> Date: Mon, 7 Sep 2026 10:45:42 -0700 Subject: [PATCH] Fix issue template enforcement for fences and pre-enforcement issues - Respect fence delimiter type and length in parsing; tildes no longer close backtick fences - Handle pre-enforcement issues correctly; distinguish from post-enforcement so editing an old issue doesn't retroactively flag it - Fix workflow shell script to avoid false negatives on comment detection --- .github/scripts/issue-template-compliance.mjs | 33 ++++++++++--- .../workflows/issue-template-compliance.yml | 4 +- .../issue-template-compliance.test.mjs | 47 +++++++++++++++++-- 3 files changed, 72 insertions(+), 12 deletions(-) diff --git a/.github/scripts/issue-template-compliance.mjs b/.github/scripts/issue-template-compliance.mjs index 84719e3978a..a9452d25339 100644 --- a/.github/scripts/issue-template-compliance.mjs +++ b/.github/scripts/issue-template-compliance.mjs @@ -22,13 +22,22 @@ const EMPTY_FIELD_VALUE = '_No response_' export function parseIssueFormSections(body) { const sections = new Map() let current = null - let inFence = false + // Why: a fence closes only on the same character, at least as long, with nothing trailing. + let fence = null for (const rawLine of (body ?? '').split(/\r?\n/)) { const line = rawLine.trimEnd() - if (/^\s*(`{3,}|~{3,})/.test(line)) { - inFence = !inFence + const delimiter = line.match(/^\s*(`{3,}|~{3,})/)?.[1] + if (!fence) { + fence = delimiter ?? null + } else if ( + delimiter && + delimiter[0] === fence[0] && + delimiter.length >= fence.length && + line.trim() === delimiter + ) { + fence = null } - const heading = !inFence && line.match(/^### (.+)$/) + const heading = !fence && line.match(/^### (.+)$/) if (heading) { current = heading[1].trim() if (!sections.has(current)) { @@ -78,6 +87,17 @@ function hasComplianceLabel(issue) { ) } +// Why hardcoded: the day this workflow shipped. Issues filed earlier were written +// before the forms were mandatory, so editing one must not retroactively flag it. +export const ENFORCEMENT_START = '2026-09-07T00:00:00Z' + +// Why created_at, not label presence: `cancel-in-progress` can kill the `opened` +// run mid-flight, leaving a brand-new non-compliant issue unlabeled. +function predatesEnforcement(issue) { + const createdAt = Date.parse(issue.created_at ?? '') + return Number.isFinite(createdAt) && createdAt < Date.parse(ENFORCEMENT_START) +} + // Returns what the workflow should do; performing it stays in the workflow. export function decideIssueTemplateAction(event, templates = ISSUE_TEMPLATES) { const { action, issue } = event @@ -91,9 +111,8 @@ export function decideIssueTemplateAction(event, templates = ISSUE_TEMPLATES) { return { kind: 'skip', reason: `bot author ${issue.user.login}` } } const flagged = hasComplianceLabel(issue) - if (action === 'edited' && !flagged) { - // Why: older issues predate enforcement; only re-check ones this workflow already flagged. - return { kind: 'skip', reason: 'edited issue was never flagged' } + if (action === 'edited' && !flagged && predatesEnforcement(issue)) { + return { kind: 'skip', reason: 'edited issue predates template enforcement' } } if (action !== 'opened' && action !== 'edited') { return { kind: 'skip', reason: `unhandled action ${action}` } diff --git a/.github/workflows/issue-template-compliance.yml b/.github/workflows/issue-template-compliance.yml index 4bec8b5adf6..b93c435c6ff 100644 --- a/.github/workflows/issue-template-compliance.yml +++ b/.github/workflows/issue-template-compliance.yml @@ -48,7 +48,9 @@ jobs: || echo "Label $LABEL already exists." gh issue edit "$ISSUE_URL" --add-label "$LABEL" # Why: an author may edit several times; keep a single bot comment per issue. - if gh issue view "$ISSUE_URL" --json comments --jq '.comments[].body' | grep -qF "$MARKER"; then + # Why the variable: in an `if` pipeline a failed lookup reads as "no marker" and double-comments. + comments="$(gh issue view "$ISSUE_URL" --json comments --jq '.comments[].body')" + if grep -qF "$MARKER" <<<"$comments"; then echo "Compliance comment already present; skipping." else gh issue comment "$ISSUE_URL" --body "$COMMENT" diff --git a/config/scripts/issue-template-compliance.test.mjs b/config/scripts/issue-template-compliance.test.mjs index f9b28d0dd58..29487ccf31b 100644 --- a/config/scripts/issue-template-compliance.test.mjs +++ b/config/scripts/issue-template-compliance.test.mjs @@ -7,6 +7,7 @@ import { parse } from 'yaml' import { COMMENT_MARKER, COMPLIANCE_LABEL, + ENFORCEMENT_START, ISSUE_TEMPLATES, decideIssueTemplateAction, evaluateIssueTemplateCompliance, @@ -96,6 +97,30 @@ describe('parseIssueFormSections', () => { expect(sections.get('Details')).toContain('### Operating system') }) + it('does not let a tilde line close a backtick fence', () => { + const body = [ + '### Details', + '', + '```', + '~~~', + '### Operating system', + 'still inside the block', + '```', + '', + '### Orca version', + '', + '1.0.0' + ].join('\n') + const sections = parseIssueFormSections(body) + expect([...sections.keys()]).toEqual(['Details', 'Orca version']) + expect(sections.get('Details')).toContain('### Operating system') + }) + + it('requires the closing fence to be at least as long as the opening one', () => { + const body = '### Details\n\n````\n```\n### Operating system\n````\n' + expect([...parseIssueFormSections(body).keys()]).toEqual(['Details']) + }) + it('handles null, empty and heading-less bodies', () => { expect(parseIssueFormSections(null).size).toBe(0) expect(parseIssueFormSections('').size).toBe(0) @@ -172,14 +197,26 @@ describe('decideIssueTemplateAction', () => { expect(decideIssueTemplateAction({ action: 'opened' }).kind).toBe('skip') }) - it('leaves edits to older, never-flagged issues alone', () => { + it('leaves edits to pre-enforcement, never-flagged issues alone', () => { // Why: pre-enforcement issues were written by hand; editing one must not retroactively flag it. - expect(decideIssueTemplateAction(issueEvent('edited', 'old hand-written report'))).toEqual({ + const created_at = new Date(Date.parse(ENFORCEMENT_START) - 1000).toISOString() + expect( + decideIssueTemplateAction(issueEvent('edited', 'old hand-written report', { created_at })) + ).toEqual({ kind: 'skip', - reason: 'edited issue was never flagged' + reason: 'edited issue predates template enforcement' }) }) + it('flags an edited post-enforcement issue whose opened run never labelled it', () => { + // Why: `cancel-in-progress` can kill the `opened` run before it adds the label. + const created_at = new Date(Date.parse(ENFORCEMENT_START) + 1000).toISOString() + const event = issueEvent('edited', 'help', { created_at }) + expect(decideIssueTemplateAction(event).kind).toBe('flag') + // A missing created_at must not become an escape hatch either. + expect(decideIssueTemplateAction(issueEvent('edited', 'help')).kind).toBe('flag') + }) + it('clears a flagged issue once its edit makes it compliant, and re-flags otherwise', () => { const flagged = { labels: [{ name: COMPLIANCE_LABEL }, 'bug'] } expect(decideIssueTemplateAction(issueEvent('edited', bugBody, flagged))).toEqual({ @@ -297,6 +334,8 @@ describe('issue-template-compliance workflow contract', () => { } // Why: comment text flows through env, never interpolated into the shell script. expect(actors[0].run).not.toContain('${{') - expect(actors[0].run).toContain('grep -qF "$MARKER"') + // Why: piping the lookup into `grep` would read a failed `gh issue view` as "no marker". + expect(actors[0].run).not.toMatch(/gh issue view[^\n]*\|/) + expect(actors[0].run).toContain('grep -qF "$MARKER" <<<"$comments"') }) })