From 46166df4c6b1b461c1a381a5a890b7f3e3d41d34 Mon Sep 17 00:00:00 2001 From: Richard Tibbles Date: Fri, 6 Nov 2026 13:15:59 -0800 Subject: [PATCH 1/4] Share the template section check and match headings at any level - Move section parsing, failure detection and the comment sync out of the pull request check - Parse and mark `###` sections; a section runs to the next heading of the same or higher level - Match a body heading to the template's at either level Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_013PjXpa7vMuHx3PGqwovv5f --- docs/community-automations.md | 2 + scripts/pull-request-template.js | 87 ++---------------------------- scripts/template-check.js | 93 ++++++++++++++++++++++++++++++++ 3 files changed, 99 insertions(+), 83 deletions(-) create mode 100644 scripts/template-check.js diff --git a/docs/community-automations.md b/docs/community-automations.md index 2737448..ca4a097 100644 --- a/docs/community-automations.md +++ b/docs/community-automations.md @@ -91,6 +91,8 @@ A section is required when its heading carries a `` comment in ``` +A `###` heading can be marked too. A section runs to the next heading of the same or higher level, so a `##` section includes its `###` subsections. A body's heading matches the template's at either level and in any case. + A template that marks no section requires every level-two heading, so that a repo is strict rather than silently unchecked. To change what a repo requires, edit the markers in that repo's template. No change is required here. Four states fail, and the log and comment name every one of them rather than stopping at the first: diff --git a/scripts/pull-request-template.js b/scripts/pull-request-template.js index 43d65de..e72d334 100644 --- a/scripts/pull-request-template.js +++ b/scripts/pull-request-template.js @@ -7,65 +7,13 @@ const fs = require('fs'); const path = require('path'); const { isContributor } = require('./utils'); -const { LE_BOT_USERNAME, PR_TEMPLATE_PATHS, PR_TEMPLATE_TARGET } = require('./constants'); +const { requiredSections, failures, syncComment } = require('./template-check'); +const { PR_TEMPLATE_PATHS, PR_TEMPLATE_TARGET } = require('./constants'); const ORG_TEMPLATE_PATH = path.join(__dirname, '..', PR_TEMPLATE_TARGET); -const HEADING = /^## [^#]/; -const REQUIRED_MARKER = /^\s*\s*$/; -const HTML_COMMENT = //g; -const PLACEHOLDER = '…'; const COMMENT_MARKER = ''; -function sections(markdown) { - const out = []; - for (const line of (markdown || '').split(/\r?\n/)) { - if (HEADING.test(line)) out.push({ heading: line.trim(), lines: [] }); - else if (out.length) out[out.length - 1].lines.push(line); - } - return out; -} - -function isMarked(section) { - const first = section.lines.find(line => line.trim() !== ''); - return first !== undefined && REQUIRED_MARKER.test(first); -} - -// A template that marks nothing requires every section, so a repo whose template -// has no markers is strict rather than unchecked. -function requiredSections(template) { - const all = sections(template); - const marked = all.filter(isMarked); - return marked.length ? marked : all; -} - -function content(section) { - return section.lines.join('\n').replace(HTML_COMMENT, '').trim(); -} - -function failures(body, required) { - const found = new Map(sections(body).map(section => [section.heading.toLowerCase(), section])); - const problems = []; - for (const { heading, lines } of required) { - const section = found.get(heading.toLowerCase()); - if (!section) { - problems.push(`\`${heading}\` is missing.`); - continue; - } - const text = content(section); - if (text === '') { - problems.push(`\`${heading}\` is empty.`); - } else if (text.split('\n').some(line => line.trim() === PLACEHOLDER)) { - // The template puts the placeholder on a line of its own, so an ellipsis - // inside a sentence is prose rather than an unfilled section. - problems.push(`\`${heading}\` still holds the \`${PLACEHOLDER}\` placeholder.`); - } else if (text === content({ lines })) { - problems.push(`\`${heading}\` still holds the text from the template.`); - } - } - return problems; -} - /** * The repo's own template, `null` if none exists, or `{ unreadable }` if a * candidate path returns anything other than a 404. An unreadable path may @@ -97,34 +45,6 @@ async function readRepoTemplate({ github, context, core }, ref) { return { template: null }; } -async function syncComment({ github, context, core }, issueNumber, body) { - const { owner, repo } = context.repo; - try { - const comments = await github.paginate(github.rest.issues.listComments, { - owner, - repo, - issue_number: issueNumber, - per_page: 100, - }); - const ours = comments.filter( - comment => comment.user?.login === LE_BOT_USERNAME && comment.body?.includes(COMMENT_MARKER), - ); - if (body === null) { - await Promise.all( - ours.map(comment => - github.rest.issues.deleteComment({ owner, repo, comment_id: comment.id }), - ), - ); - } else if (!ours.length) { - await github.rest.issues.createComment({ owner, repo, issue_number: issueNumber, body }); - } else if (ours[0].body !== body) { - await github.rest.issues.updateComment({ owner, repo, comment_id: ours[0].id, body }); - } - } catch (error) { - core.warning(`Could not update the template comment: ${error.message}`); - } -} - function commentBody(problems, templateUrl) { return [ COMMENT_MARKER, @@ -167,7 +87,7 @@ module.exports = async ({ github, context, core }) => { const problems = failures(pullRequest.body, required); if (!problems.length) { core.info(`The body carries all ${required.length} required sections.`); - await syncComment({ github, context, core }, pullRequest.number, null); + await syncComment({ github, context, core }, pullRequest.number, COMMENT_MARKER, null); return; } @@ -177,6 +97,7 @@ module.exports = async ({ github, context, core }) => { await syncComment( { github, context, core }, pullRequest.number, + COMMENT_MARKER, commentBody(problems, templateUrl), ); diff --git a/scripts/template-check.js b/scripts/template-check.js new file mode 100644 index 0000000..75fee97 --- /dev/null +++ b/scripts/template-check.js @@ -0,0 +1,93 @@ +const { LE_BOT_USERNAME } = require('./constants'); + +const HEADING = /^(#{2,3}) [^#]/; +const REQUIRED_MARKER = /^\s*\s*$/; +const HTML_COMMENT = //g; +const PLACEHOLDER = '…'; + +function headingKey(heading) { + return heading.replace(/^#+\s*/, '').toLowerCase(); +} + +function sections(markdown) { + const lines = (markdown || '').split(/\r?\n/); + const headings = []; + lines.forEach((line, index) => { + const match = HEADING.exec(line); + if (match) headings.push({ heading: line.trim(), level: match[1].length, index }); + }); + return headings.map(({ heading, level, index }, i) => { + const end = headings.slice(i + 1).find(next => next.level <= level); + return { heading, level, lines: lines.slice(index + 1, end ? end.index : lines.length) }; + }); +} + +function isMarked(section) { + const first = section.lines.find(line => line.trim() !== ''); + return first !== undefined && REQUIRED_MARKER.test(first); +} + +// A template that marks nothing requires every top-level section, so a repo +// whose template has no markers is strict rather than unchecked. +function requiredSections(template) { + const all = sections(template); + const marked = all.filter(isMarked); + return marked.length ? marked : all.filter(section => section.level === 2); +} + +function content(section) { + return section.lines.join('\n').replace(HTML_COMMENT, '').trim(); +} + +function failures(body, required) { + const found = new Map(sections(body).map(section => [headingKey(section.heading), section])); + const problems = []; + for (const { heading, lines } of required) { + const section = found.get(headingKey(heading)); + if (!section) { + problems.push(`\`${heading}\` is missing.`); + continue; + } + const text = content(section); + if (text === '') { + problems.push(`\`${heading}\` is empty.`); + } else if (text.split('\n').some(line => line.trim() === PLACEHOLDER)) { + // The template puts the placeholder on a line of its own, so an ellipsis + // inside a sentence is prose rather than an unfilled section. + problems.push(`\`${heading}\` still holds the \`${PLACEHOLDER}\` placeholder.`); + } else if (text === content({ lines })) { + problems.push(`\`${heading}\` still holds the text from the template.`); + } + } + return problems; +} + +async function syncComment({ github, context, core }, issueNumber, marker, body) { + const { owner, repo } = context.repo; + try { + const comments = await github.paginate(github.rest.issues.listComments, { + owner, + repo, + issue_number: issueNumber, + per_page: 100, + }); + const ours = comments.filter( + comment => comment.user?.login === LE_BOT_USERNAME && comment.body?.includes(marker), + ); + if (body === null) { + await Promise.all( + ours.map(comment => + github.rest.issues.deleteComment({ owner, repo, comment_id: comment.id }), + ), + ); + } else if (!ours.length) { + await github.rest.issues.createComment({ owner, repo, issue_number: issueNumber, body }); + } else if (ours[0].body !== body) { + await github.rest.issues.updateComment({ owner, repo, comment_id: ours[0].id, body }); + } + } catch (error) { + core.warning(`Could not update the template comment: ${error.message}`); + } +} + +module.exports = { headingKey, sections, requiredSections, failures, syncComment }; From 904636c2e6148646e8c6afbbef1d86a5555454b7 Mon Sep 17 00:00:00 2001 From: Richard Tibbles Date: Fri, 6 Nov 2026 13:16:07 -0800 Subject: [PATCH 2/4] Mark the sections each issue template requires - Bug: Observed behavior, Expected behavior, Steps to reproduce, Context - Enhancement: Current behavior, Desired behavior - Other: Description - Technical task: Overview, The Change, Acceptance Criteria - Technical tracking: Overview, Scope - Product: every section - Feature: every section except Reference Links Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_013PjXpa7vMuHx3PGqwovv5f --- .github/ISSUE_TEMPLATE/bug_issue.md | 4 ++++ .github/ISSUE_TEMPLATE/enhancement_issue.md | 2 ++ .github/ISSUE_TEMPLATE/feature_issue.md | 7 +++++++ .github/ISSUE_TEMPLATE/other_issue.md | 1 + .github/ISSUE_TEMPLATE/product_issue.md | 4 ++++ .github/ISSUE_TEMPLATE/technical_task_issue.md | 3 +++ .github/ISSUE_TEMPLATE/technical_tracking_issue.md | 2 ++ 7 files changed, 23 insertions(+) diff --git a/.github/ISSUE_TEMPLATE/bug_issue.md b/.github/ISSUE_TEMPLATE/bug_issue.md index 9a5ff41..de51da9 100644 --- a/.github/ISSUE_TEMPLATE/bug_issue.md +++ b/.github/ISSUE_TEMPLATE/bug_issue.md @@ -16,6 +16,7 @@ Instructions: **Target branch:** ## Observed behavior + @@ -39,6 +40,7 @@ Please wrap errors in triple backticks for clean formatting like this: … ## Expected behavior + @@ -53,6 +55,7 @@ Implications and real-world consequences for learners, coaches, admins, and othe … ## Steps to reproduce + @@ -60,6 +63,7 @@ Precise steps that someone else can follow in order to see this behavior … ## Context + ## Current behavior + ## Desired behavior + ## Value add diff --git a/.github/ISSUE_TEMPLATE/feature_issue.md b/.github/ISSUE_TEMPLATE/feature_issue.md index f040360..8a2eb6f 100644 --- a/.github/ISSUE_TEMPLATE/feature_issue.md +++ b/.github/ISSUE_TEMPLATE/feature_issue.md @@ -14,16 +14,21 @@ type: Tracking Issue --> ## Requirements + ### What problem are we solving? + ### Why this? Why now? + ## Outcomes + - [Specific, measurable outcome] ## Constraints + - [Budget, deadline, technical limitation] @@ -48,12 +53,14 @@ type: Tracking Issue - [Explicit exclusion] ## Context + ### Relevant Feedback - [Feedback from partners, users, clearinghouse, teammates] ### Technical Context + - [Relevant systems, prior art, technical considerations] diff --git a/.github/ISSUE_TEMPLATE/other_issue.md b/.github/ISSUE_TEMPLATE/other_issue.md index abde6bf..56c6482 100644 --- a/.github/ISSUE_TEMPLATE/other_issue.md +++ b/.github/ISSUE_TEMPLATE/other_issue.md @@ -10,6 +10,7 @@ about: For issues that don't fit any other category --> ## Description + diff --git a/.github/ISSUE_TEMPLATE/product_issue.md b/.github/ISSUE_TEMPLATE/product_issue.md index c5e2694..4b54d6c 100644 --- a/.github/ISSUE_TEMPLATE/product_issue.md +++ b/.github/ISSUE_TEMPLATE/product_issue.md @@ -13,19 +13,23 @@ type: Tracking Issue _[General guidance: Product issues should describe the feature, but are not actually technically specced for dev work.]_ ## Overview + _[First, the workflow in one sentence]_ ### Description and outcomes + _[Then, more detail. Provide a short summary and/or list key outcomes that the user should be able to accomplish with this feature.]_ ### Resources + - _[Link Gherkin Scenarios]_ - _[Link Figma Scenarios]_ ### Accessibility Requirements + ## AI usage diff --git a/.github/ISSUE_TEMPLATE/technical_task_issue.md b/.github/ISSUE_TEMPLATE/technical_task_issue.md index 00f1bb1..7cd45a2 100644 --- a/.github/ISSUE_TEMPLATE/technical_task_issue.md +++ b/.github/ISSUE_TEMPLATE/technical_task_issue.md @@ -13,6 +13,7 @@ Instructions: --> ## Overview + ### The Change + ### Acceptance Criteria + ## Overview + ### Scope + ` rules of `pull-request-template`. The issue does not record which template it came from, so it passes if it satisfies any one of them. + +- **Fails:** posts one comment naming the template it shares the most headings with and what that template still needs, and adds `needs-template`. No validation is requested. +- **Passes:** deletes that comment, removes `needs-template`, and requests validation, unless the issue already carries `community-issue-review`. + +Validation is requested by a comment mentioning `@rtibblesbot` and the `community-issue-review` label. If a label does not exist in the repository, the add-label call creates it. **The mention is essential.** The agent polls its GitHub notifications and has no inbound endpoint, so a mention is the only way to reach it. Adding a label does not generate a notification, and a handle inside backticks, as in `BOT_MESSAGE_RTIBBLESBOT_REVIEW`, does not trigger one either. @@ -147,9 +152,11 @@ The weekly `unassign-inactive-issues` run also sweeps stalled validations throug If the agent never responds, for example, because the service is down or all available tokens have reached their limits, the label would otherwise remain indefinitely. The sweep removes it when the issue's `labeled` event is older than `ISSUE_REVIEW_MAX_DAYS` days. +In repositories listed in `ISSUE_VALIDATION_REPOS`, the same run closes, as not planned, every open issue labelled `needs-template` for longer than `ISSUE_TEMPLATE_MAX_DAYS` days, with `BOT_MESSAGE_ISSUE_TEMPLATE_CLOSED`. To keep an issue open, a maintainer fixes its description or removes the label. Reopening a closed issue reruns the check and restarts the clock. + ## Scope and configuration -Neither automation closes issues or acts on the validation findings. The agent's comment recommends whether the issue should be closed; a maintainer must request that action in a reply. +Neither automation acts on the validation findings. The agent's comment recommends whether the issue should be closed; a maintainer must request that action in a reply. Configure the following constants in `scripts/constants.js`: @@ -159,6 +166,9 @@ Configure the following constants in `scripts/constants.js`: - `ISSUE_REVIEW_MAX_DAYS`: The maximum age of the label before the sweep clears it. - `BOT_MESSAGE_ISSUE_VALIDATION`: The comment text posted to request validation. - `ISSUE_VALIDATION_FOLLOW_UPS`: The six verdicts and the follow-up each one asks a maintainer for, used in the Slack line. +- `LABEL_NEEDS_TEMPLATE`: The label on an issue that fails the template check. +- `ISSUE_TEMPLATE_MAX_DAYS`: How long that label stays on before the sweep closes the issue. +- `BOT_MESSAGE_ISSUE_TEMPLATE_CLOSED`: The comment posted when the sweep closes an issue. # `holiday-message` diff --git a/scripts/constants.js b/scripts/constants.js index b77e167..f5148f2 100644 --- a/scripts/constants.js +++ b/scripts/constants.js @@ -186,6 +186,15 @@ const BOT_MESSAGE_ISSUE_VALIDATION = `- @rtibblesbot's comments are LLM-generated, so please evaluate them critically.\n\n` + `A maintainer decides what happens next. This check does not close the issue.`; +const LABEL_NEEDS_TEMPLATE = 'needs-template'; +const ISSUE_TEMPLATE_MAX_DAYS = 7; +const ISSUE_TEMPLATE_MARKER = ''; + +const BOT_MESSAGE_ISSUE_TEMPLATE_CLOSED = + `**Closing this issue because its description still doesn't follow an issue template ` + + `after ${ISSUE_TEMPLATE_MAX_DAYS} days.** Once you've edited it to follow one, ` + + `comment here and a maintainer can reopen it.`; + // Repositories to include in PR statistics reports const PR_STATS_REPOS = [ 'kolibri', @@ -252,6 +261,10 @@ module.exports = { ISSUE_VALIDATION_NO_VERDICT_FOLLOW_UP, ISSUE_REVIEW_MAX_DAYS, BOT_MESSAGE_ISSUE_VALIDATION, + LABEL_NEEDS_TEMPLATE, + ISSUE_TEMPLATE_MAX_DAYS, + ISSUE_TEMPLATE_MARKER, + BOT_MESSAGE_ISSUE_TEMPLATE_CLOSED, TEAMS_WITH_CLOSE_CONTRIBUTORS, HOLIDAY_MESSAGE, PR_STATS_REPOS, diff --git a/scripts/issue-review-sweep.js b/scripts/issue-review-sweep.js index 2abf681..9ce40e3 100644 --- a/scripts/issue-review-sweep.js +++ b/scripts/issue-review-sweep.js @@ -1,14 +1,21 @@ // See docs/community-automations.md -const { LABEL_COMMUNITY_ISSUE_REVIEW, ISSUE_REVIEW_MAX_DAYS } = require('./constants'); +const { + ISSUE_VALIDATION_REPOS, + LABEL_COMMUNITY_ISSUE_REVIEW, + ISSUE_REVIEW_MAX_DAYS, + LABEL_NEEDS_TEMPLATE, + ISSUE_TEMPLATE_MAX_DAYS, + BOT_MESSAGE_ISSUE_TEMPLATE_CLOSED, +} = require('./constants'); const { removeLabel } = require('./utils'); const MS_PER_DAY = 24 * 60 * 60 * 1000; /** - * When the label was last applied, or null when the timeline carries no such - * event. The timeline returns events oldest first, so the last match is the - * most recent application. + * When the label was last applied, or the issue last reopened if later, or null + * when the timeline carries no such label event. The timeline returns events + * oldest first. */ async function labeledAt(name, owner, repo, issueNumber, github, core) { try { @@ -18,55 +25,61 @@ async function labeledAt(name, owner, repo, issueNumber, github, core) { issue_number: issueNumber, per_page: 100, }); - const applied = events.filter( + const applied = events.findLastIndex( event => event.event === 'labeled' && event.label?.name?.toLowerCase() === name.toLowerCase(), ); - if (!applied.length) { + if (applied === -1) { return null; } - return new Date(applied[applied.length - 1].created_at); + const reopened = events.slice(applied).findLast(event => event.event === 'reopened'); + return new Date((reopened ?? events[applied]).created_at); } catch (error) { core.warning(`Failed to read the timeline of issue #${issueNumber}: ${error.message}`); return null; } } -/** - * Removes the review label from every open issue that carried it for longer - * than ISSUE_REVIEW_MAX_DAYS, and returns the issue numbers it cleared. A stuck - * label means the agent never answered, so it tells a maintainer nothing. - */ -async function sweepIssueReviewLabels(owner, repo, { github, core }, now = Date.now()) { +async function staleIssues(name, maxDays, owner, repo, { github, core }, now) { let issues = []; try { issues = await github.paginate(github.rest.issues.listForRepo, { owner, repo, state: 'open', - labels: LABEL_COMMUNITY_ISSUE_REVIEW, + labels: name, per_page: 100, }); } catch (error) { - core.warning(`Failed to list issues under review in ${owner}/${repo}: ${error.message}`); + core.warning(`Failed to list issues labelled ${name} in ${owner}/${repo}: ${error.message}`); return []; } - const cleared = []; + const stale = []; for (const issue of issues) { - const applied = await labeledAt( - LABEL_COMMUNITY_ISSUE_REVIEW, - owner, - repo, - issue.number, - github, - core, - ); - if (!applied) { - continue; - } - if (now - applied.getTime() <= ISSUE_REVIEW_MAX_DAYS * MS_PER_DAY) { - continue; + const applied = await labeledAt(name, owner, repo, issue.number, github, core); + if (applied && now - applied.getTime() > maxDays * MS_PER_DAY) { + stale.push({ issue, applied }); } + } + return stale; +} + +/** + * Removes the review label from every open issue that carried it for longer + * than ISSUE_REVIEW_MAX_DAYS, and returns the issue numbers it cleared. A stuck + * label means the agent never answered, so it tells a maintainer nothing. + */ +async function sweepIssueReviewLabels(owner, repo, { github, core }, now = Date.now()) { + const stale = await staleIssues( + LABEL_COMMUNITY_ISSUE_REVIEW, + ISSUE_REVIEW_MAX_DAYS, + owner, + repo, + { github, core }, + now, + ); + const cleared = []; + for (const { issue, applied } of stale) { const removed = await removeLabel( LABEL_COMMUNITY_ISSUE_REVIEW, owner, @@ -86,4 +99,43 @@ async function sweepIssueReviewLabels(owner, repo, { github, core }, now = Date. return cleared; } -module.exports = { sweepIssueReviewLabels }; +async function closeIssuesMissingTemplate(owner, repo, { github, core }, now = Date.now()) { + if (!ISSUE_VALIDATION_REPOS.includes(repo)) { + return []; + } + const stale = await staleIssues( + LABEL_NEEDS_TEMPLATE, + ISSUE_TEMPLATE_MAX_DAYS, + owner, + repo, + { github, core }, + now, + ); + const closed = []; + for (const { issue, applied } of stale.filter(({ issue }) => !issue.pull_request)) { + try { + await github.rest.issues.update({ + owner, + repo, + issue_number: issue.number, + state: 'closed', + state_reason: 'not_planned', + }); + await github.rest.issues.createComment({ + owner, + repo, + issue_number: issue.number, + body: BOT_MESSAGE_ISSUE_TEMPLATE_CLOSED, + }); + closed.push(issue.number); + core.info( + `Closed issue #${issue.number}, labelled ${LABEL_NEEDS_TEMPLATE} ${applied.toISOString()}.`, + ); + } catch (error) { + core.warning(`Failed to close issue #${issue.number}: ${error.message}`); + } + } + return closed; +} + +module.exports = { sweepIssueReviewLabels, closeIssuesMissingTemplate }; diff --git a/scripts/issue-review-sweep.test.js b/scripts/issue-review-sweep.test.js index 79dc5df..bb4d71e 100644 --- a/scripts/issue-review-sweep.test.js +++ b/scripts/issue-review-sweep.test.js @@ -1,8 +1,14 @@ const test = require('node:test'); const assert = require('node:assert/strict'); -const { sweepIssueReviewLabels } = require('./issue-review-sweep'); -const { LABEL_COMMUNITY_ISSUE_REVIEW, ISSUE_REVIEW_MAX_DAYS } = require('./constants'); +const { sweepIssueReviewLabels, closeIssuesMissingTemplate } = require('./issue-review-sweep'); +const { + LABEL_COMMUNITY_ISSUE_REVIEW, + ISSUE_REVIEW_MAX_DAYS, + LABEL_NEEDS_TEMPLATE, + ISSUE_TEMPLATE_MAX_DAYS, + BOT_MESSAGE_ISSUE_TEMPLATE_CLOSED, +} = require('./constants'); const NOW = Date.parse('2026-10-26T12:00:00Z'); const MS_PER_DAY = 24 * 60 * 60 * 1000; @@ -256,5 +262,143 @@ test('a listing this run cannot read sweeps nothing', async () => { assert.deepEqual(cleared, []); assert.equal(core.warnings.length, 1); - assert.match(core.warnings[0], /Failed to list issues under review/); + assert.match(core.warnings[0], /Failed to list issues labelled/); +}); + +const templateIssue = number => ({ number, labels: [LABEL_NEEDS_TEMPLATE] }); + +function closingGithub(options) { + const github = fakeGithub(options); + github.commented = []; + github.closed = []; + github.rest.issues.createComment = async ({ issue_number, body }) => { + github.commented.push({ issue_number, body }); + }; + github.rest.issues.update = async ({ issue_number, state, state_reason }) => { + github.closed.push({ issue_number, state, state_reason }); + }; + return github; +} + +test('an issue missing its template past the limit is closed with a comment', async () => { + const github = closingGithub({ + issues: [templateIssue(7)], + timelines: { 7: [labeled(daysAgo(ISSUE_TEMPLATE_MAX_DAYS + 1), LABEL_NEEDS_TEMPLATE)] }, + }); + const closed = await closeIssuesMissingTemplate( + 'learningequality', + 'kolibri', + { github, core: fakeCore() }, + NOW, + ); + + assert.deepEqual(closed, [7]); + assert.deepEqual(github.commented, [ + { issue_number: 7, body: BOT_MESSAGE_ISSUE_TEMPLATE_CLOSED }, + ]); + assert.deepEqual(github.closed, [ + { issue_number: 7, state: 'closed', state_reason: 'not_planned' }, + ]); +}); + +test('an issue missing its template within the limit stays open', async () => { + const github = closingGithub({ + issues: [templateIssue(7)], + timelines: { 7: [labeled(daysAgo(ISSUE_TEMPLATE_MAX_DAYS - 1), LABEL_NEEDS_TEMPLATE)] }, + }); + const closed = await closeIssuesMissingTemplate( + 'learningequality', + 'kolibri', + { github, core: fakeCore() }, + NOW, + ); + + assert.deepEqual(closed, []); + assert.deepEqual(github.commented, []); +}); + +test('an issue whose template label a maintainer removed stays open', async () => { + const github = closingGithub({ + issues: [{ number: 7, labels: [] }], + timelines: { 7: [labeled(daysAgo(30), LABEL_NEEDS_TEMPLATE)] }, + }); + const closed = await closeIssuesMissingTemplate( + 'learningequality', + 'kolibri', + { github, core: fakeCore() }, + NOW, + ); + + assert.deepEqual(closed, []); +}); + +test('a close this run cannot make warns and is not counted', async () => { + const github = closingGithub({ + issues: [templateIssue(7)], + timelines: { 7: [labeled(daysAgo(30), LABEL_NEEDS_TEMPLATE)] }, + }); + github.rest.issues.update = async () => { + throw new Error('forbidden'); + }; + const core = fakeCore(); + const closed = await closeIssuesMissingTemplate( + 'learningequality', + 'kolibri', + { github, core }, + NOW, + ); + + assert.deepEqual(closed, []); + assert.deepEqual(github.commented, []); + assert.match(core.warnings[0], /Failed to close issue #7/); +}); + +test('a reopened issue still missing its template gets the full limit again', async () => { + const github = closingGithub({ + issues: [templateIssue(7)], + timelines: { + 7: [ + labeled(daysAgo(30), LABEL_NEEDS_TEMPLATE), + { event: 'reopened', created_at: daysAgo(ISSUE_TEMPLATE_MAX_DAYS - 1) }, + ], + }, + }); + const closed = await closeIssuesMissingTemplate( + 'learningequality', + 'kolibri', + { github, core: fakeCore() }, + NOW, + ); + + assert.deepEqual(closed, []); +}); + +test('a pull request carrying the template label is not closed', async () => { + const github = closingGithub({ + issues: [{ ...templateIssue(7), pull_request: {} }], + timelines: { 7: [labeled(daysAgo(30), LABEL_NEEDS_TEMPLATE)] }, + }); + const closed = await closeIssuesMissingTemplate( + 'learningequality', + 'kolibri', + { github, core: fakeCore() }, + NOW, + ); + + assert.deepEqual(closed, []); +}); + +test('a repo outside issue validation has nothing closed', async () => { + const github = closingGithub({ + issues: [templateIssue(7)], + timelines: { 7: [labeled(daysAgo(30), LABEL_NEEDS_TEMPLATE)] }, + }); + const closed = await closeIssuesMissingTemplate( + 'learningequality', + 'internal-tooling', + { github, core: fakeCore() }, + NOW, + ); + + assert.deepEqual(closed, []); }); diff --git a/scripts/issue-template.js b/scripts/issue-template.js new file mode 100644 index 0000000..1db8d99 --- /dev/null +++ b/scripts/issue-template.js @@ -0,0 +1,88 @@ +const fs = require('fs'); +const path = require('path'); + +const { + headingKey, + sections, + requiredSections, + failures, + syncComment, +} = require('./template-check'); +const { + LABEL_NEEDS_TEMPLATE, + ISSUE_TEMPLATE_MARKER, + ISSUE_TEMPLATE_MAX_DAYS, +} = require('./constants'); +const { addLabel } = require('./utils'); + +const TEMPLATE_DIR = path.join(__dirname, '..', '.github', 'ISSUE_TEMPLATE'); +const TEMPLATE_URL = 'https://github.com/learningequality/.github/blob/main/.github/ISSUE_TEMPLATE'; +const FRONT_MATTER_NAME = /^name:\s*["']?(.+?)["']?\s*$/m; + +function readTemplates(dir = TEMPLATE_DIR) { + return fs + .readdirSync(dir) + .filter(file => file.endsWith('.md')) + .sort() + .map(file => { + const text = fs.readFileSync(path.join(dir, file), 'utf8'); + return { + file, + name: FRONT_MATTER_NAME.exec(text)?.[1] ?? file, + headings: sections(text).map(section => headingKey(section.heading)), + required: requiredSections(text), + }; + }); +} + +function closestFailure(body, templates) { + const present = new Set(sections(body).map(section => headingKey(section.heading))); + const scored = templates.map(template => ({ + template, + problems: failures(body, template.required), + shared: template.headings.filter(heading => present.has(heading)).length, + })); + if (scored.some(({ problems }) => !problems.length)) return null; + scored.sort((a, b) => b.shared - a.shared || a.problems.length - b.problems.length); + return scored[0]; +} + +function commentBody({ template, problems }, { owner, repo }) { + return [ + ISSUE_TEMPLATE_MARKER, + `**📝 This issue doesn't follow one of our ` + + `[issue templates](https://github.com/${owner}/${repo}/issues/new/choose) yet.** ` + + `It is closest to [${template.name}](${TEMPLATE_URL}/${template.file}). ` + + 'Please edit the description to fix:', + '', + ...problems.map(problem => `- ${problem}`), + '', + 'The check runs again when you save, and this comment goes away once it passes. ' + + 'We review the issue after that. ' + + `**If it still doesn't follow a template in ${ISSUE_TEMPLATE_MAX_DAYS} days, ` + + 'it will be closed.**', + ].join('\n'); +} + +async function checkIssueTemplate({ github, context, core }, issue, templates = readTemplates()) { + const { owner, repo } = context.repo; + const failure = closestFailure(issue.body, templates); + if (!failure) { + core.info(`Issue #${issue.number} follows a template.`); + await syncComment({ github, context, core }, issue.number, ISSUE_TEMPLATE_MARKER, null); + return true; + } + core.info( + `Issue #${issue.number} is closest to ${failure.template.file}: ` + failure.problems.join(' '), + ); + await syncComment( + { github, context, core }, + issue.number, + ISSUE_TEMPLATE_MARKER, + commentBody(failure, context.repo), + ); + await addLabel(LABEL_NEEDS_TEMPLATE, owner, repo, issue.number, github, core); + return false; +} + +module.exports = { checkIssueTemplate, readTemplates }; diff --git a/scripts/issue-template.test.js b/scripts/issue-template.test.js new file mode 100644 index 0000000..c107bab --- /dev/null +++ b/scripts/issue-template.test.js @@ -0,0 +1,99 @@ +const test = require('node:test'); +const assert = require('node:assert/strict'); +const fs = require('fs'); +const path = require('path'); + +const { checkIssueTemplate, readTemplates } = require('./issue-template'); +const { LABEL_NEEDS_TEMPLATE } = require('./constants'); + +const TEMPLATE_DIR = path.join(__dirname, '..', '.github', 'ISSUE_TEMPLATE'); +const TEMPLATE_FILES = fs.readdirSync(TEMPLATE_DIR).filter(file => file.endsWith('.md')); + +function fakeGithub() { + const comments = []; + const added = []; + return { + comments, + added, + paginate: async () => [], + rest: { + issues: { + listComments: async () => [], + createComment: async ({ body }) => { + comments.push(body); + }, + addLabels: async ({ labels }) => { + added.push(...labels); + }, + }, + }, + }; +} + +const context = { repo: { owner: 'learningequality', repo: 'kolibri' } }; +const core = { info() {}, warning() {} }; + +const check = (body, github = fakeGithub()) => + checkIssueTemplate({ github, context, core }, { number: 7, body }); + +function fillIn(template) { + return template + .split('\n') + .flatMap(line => { + if (line.trim() === '…') return ['Filled in.']; + if (/^\s*\s*$/.test(line)) return [line, 'Filled in.']; + return [line]; + }) + .join('\n'); +} + +for (const file of TEMPLATE_FILES) { + const template = fs.readFileSync(path.join(TEMPLATE_DIR, file), 'utf8'); + + test(`${file} filled in passes`, async () => { + assert.equal(await check(fillIn(template)), true); + }); + + test(`${file} left as written fails`, async () => { + assert.equal(await check(template), false); + }); + + test(`${file} without AI usage fails`, async () => { + const withoutAiUsage = fillIn(template).split('## AI usage')[0]; + assert.equal(await check(withoutAiUsage), false); + }); +} + +test('the templates are read with their names', () => { + const names = readTemplates().map(template => template.name); + assert.ok(names.includes('Report a bug')); +}); + +test('a failure comments, labels, and names the closest template', async () => { + const github = fakeGithub(); + const body = '## Steps to reproduce\n\n1. Sync\n\n## AI usage\n\nNone.\n'; + assert.equal(await check(body, github), false); + + assert.deepEqual(github.added, [LABEL_NEEDS_TEMPLATE]); + assert.match(github.comments[0], /closest to \[Report a bug\]/); + assert.match(github.comments[0], /`## Observed behavior` is missing/); + assert.ok(!github.comments[0].includes('Steps to reproduce')); +}); + +test('a required subsection is checked even when its parent is filled', async () => { + const template = fs.readFileSync(path.join(TEMPLATE_DIR, 'technical_tracking_issue.md'), 'utf8'); + const withoutScope = fillIn(template).replace('### Scope', '### Boundaries'); + const github = fakeGithub(); + assert.equal(await check(withoutScope, github), false); + assert.match(github.comments[0], /`### Scope` is missing/); +}); + +test('a filled template matches whatever heading level the body uses', async () => { + // kolibri#15703 followed the bug template with `###` headings throughout. + const template = fs.readFileSync(path.join(TEMPLATE_DIR, 'bug_issue.md'), 'utf8'); + assert.equal(await check(fillIn(template).replace(/^## /gm, '### ')), true); +}); + +test('an empty body fails rather than throwing', async () => { + assert.equal(await check(null), false); +}); diff --git a/scripts/issue-validation.js b/scripts/issue-validation.js index 4b441f9..9b4c480 100644 --- a/scripts/issue-validation.js +++ b/scripts/issue-validation.js @@ -3,9 +3,11 @@ const { ISSUE_VALIDATION_REPOS, LABEL_COMMUNITY_ISSUE_REVIEW, + LABEL_NEEDS_TEMPLATE, BOT_MESSAGE_ISSUE_VALIDATION, } = require('./constants'); -const { sendBotMessage, hasLabel, addLabel } = require('./utils'); +const { sendBotMessage, addLabel, removeLabel } = require('./utils'); +const { checkIssueTemplate } = require('./issue-template'); module.exports = async ({ github, context, core }) => { try { @@ -18,7 +20,36 @@ module.exports = async ({ github, context, core }) => { return; } - if (await hasLabel(LABEL_COMMUNITY_ISSUE_REVIEW, owner, repo, issueNumber, github, core)) { + const { data: issue } = await github.rest.issues.get({ + owner, + repo, + issue_number: issueNumber, + }); + if (issue.state !== 'open') { + core.info(`Issue #${issueNumber} is ${issue.state}.`); + return; + } + + const labels = (issue.labels || []).map(label => + (typeof label === 'string' ? label : label.name).toLowerCase(), + ); + const flagged = labels.includes(LABEL_NEEDS_TEMPLATE); + if (context.payload.action !== 'opened' && !flagged) { + core.info(`Issue #${issueNumber} is not waiting on the template.`); + return; + } + + if (!(await checkIssueTemplate({ github, context, core }, issue))) { + return; + } + if ( + flagged && + !(await removeLabel(LABEL_NEEDS_TEMPLATE, owner, repo, issueNumber, github, core)) + ) { + return; + } + + if (labels.includes(LABEL_COMMUNITY_ISSUE_REVIEW)) { core.info(`Issue #${issueNumber} already carries ${LABEL_COMMUNITY_ISSUE_REVIEW}.`); return; } diff --git a/scripts/issue-validation.test.js b/scripts/issue-validation.test.js index 99c12df..cc5b1e6 100644 --- a/scripts/issue-validation.test.js +++ b/scripts/issue-validation.test.js @@ -2,39 +2,93 @@ const test = require('node:test'); const assert = require('node:assert/strict'); const issueValidation = require('./issue-validation'); -const { LABEL_COMMUNITY_ISSUE_REVIEW, COMMUNITY_REPOS } = require('./constants'); +const { + LABEL_COMMUNITY_ISSUE_REVIEW, + LABEL_NEEDS_TEMPLATE, + COMMUNITY_REPOS, + BOT_MESSAGE_ISSUE_VALIDATION, + ISSUE_TEMPLATE_MARKER, + LE_BOT_USERNAME, +} = require('./constants'); const SANDBOX = 'test-actions'; -function fakeGithub({ labels = [], addFails = false } = {}) { - const comments = []; +const FOLLOWS_TEMPLATE = '## Description\n\nThe sync fails.\n\n## AI usage\n\nI used no AI.\n'; +// Shaped like kolibri#15779, which an agent wrote without a template. +const FREESTYLED = [ + '## Summary', + '', + 'The sync fails.', + '', + '## Steps to reproduce', + '', + '1. Sync', + '', + '## Suggested fix', + '', + 'Retry.', +].join('\n'); + +function fakeGithub({ + body = FOLLOWS_TEMPLATE, + labels = [], + state = 'open', + comments = [], + addFails = false, + removeFails = false, +} = {}) { + const created = []; + const updated = []; + const deleted = []; const added = []; + const removed = []; + const listComments = async () => comments; return { - comments, + created, + updated, + deleted, added, + removed, paginate: async (fn, params) => fn(params), rest: { issues: { - listLabelsOnIssue: async () => labels.map(name => ({ name })), + get: async () => ({ + data: { number: 7, body, state, labels: labels.map(name => ({ name })) }, + }), + listComments, addLabels: async ({ labels: names }) => { if (addFails) { throw new Error('Resource not accessible by integration'); } added.push(...names); }, - createComment: async ({ body }) => { - comments.push(body); + removeLabel: async ({ name }) => { + if (removeFails) { + const error = new Error('server error'); + error.status = 500; + throw error; + } + removed.push(name); + }, + createComment: async ({ body: text }) => { + created.push(text); return { data: { html_url: 'https://github.com/learningequality/kolibri/issues/7' } }; }, + updateComment: async ({ comment_id, body: text }) => { + updated.push({ comment_id, body: text }); + }, + deleteComment: async ({ comment_id }) => { + deleted.push(comment_id); + }, }, }, }; } -function fakeContext({ repo = COMMUNITY_REPOS[0] } = {}) { +function fakeContext({ repo = COMMUNITY_REPOS[0], action = 'opened' } = {}) { return { repo: { owner: 'learningequality', repo }, - payload: { issue: { number: 7 } }, + payload: { action, issue: { number: 7 } }, }; } @@ -55,12 +109,14 @@ function fakeCore() { }; } -test('a new issue in a community repo gets one comment and the label', async () => { +const templateComment = { id: 3, user: { login: LE_BOT_USERNAME }, body: ISSUE_TEMPLATE_MARKER }; + +test('a new issue following a template gets one comment and the review label', async () => { const github = fakeGithub(); const core = fakeCore(); await issueValidation({ github, context: fakeContext(), core }); - assert.equal(github.comments.length, 1); + assert.deepEqual(github.created, [BOT_MESSAGE_ISSUE_VALIDATION]); assert.deepEqual(github.added, [LABEL_COMMUNITY_ISSUE_REVIEW]); assert.deepEqual(core.failures, []); }); @@ -71,15 +127,89 @@ test('the comment mentions rtibblesbot outside backticks, which is the trigger', // A backticked handle sends no notification, and the mention is the only way // to reach the agent. See learningequality/coding-agent#1229. - assert.match(github.comments[0], /(^|[^`])@rtibblesbot/); + assert.match(github.created[0], /(^|[^`])@rtibblesbot/); +}); + +test('a new freestyled issue is flagged and not sent for validation', async () => { + const github = fakeGithub({ body: FREESTYLED }); + await issueValidation({ github, context: fakeContext(), core: fakeCore() }); + + assert.equal(github.created.length, 1); + assert.ok(github.created[0].includes(ISSUE_TEMPLATE_MARKER)); + assert.match(github.created[0], /closest to \[Report a bug\]/); + assert.match(github.created[0], /`## AI usage` is missing/); + assert.ok( + !github.created[0].includes('@rtibblesbot'), + 'the template comment must not summon the agent', + ); + assert.deepEqual(github.added, [LABEL_NEEDS_TEMPLATE]); +}); + +test('an edit that makes a flagged issue pass clears it and requests validation', async () => { + const github = fakeGithub({ labels: [LABEL_NEEDS_TEMPLATE], comments: [templateComment] }); + await issueValidation({ github, context: fakeContext({ action: 'edited' }), core: fakeCore() }); + + assert.deepEqual(github.deleted, [3]); + assert.deepEqual(github.removed, [LABEL_NEEDS_TEMPLATE]); + assert.deepEqual(github.created, [BOT_MESSAGE_ISSUE_VALIDATION]); + assert.deepEqual(github.added, [LABEL_COMMUNITY_ISSUE_REVIEW]); +}); + +test('a flagged issue whose label cannot come off is not sent for validation', async () => { + const github = fakeGithub({ labels: [LABEL_NEEDS_TEMPLATE], removeFails: true }); + await issueValidation({ github, context: fakeContext({ action: 'edited' }), core: fakeCore() }); + + assert.deepEqual(github.created, []); + assert.deepEqual(github.added, []); +}); + +test('an edit that leaves a flagged issue failing updates the comment', async () => { + const github = fakeGithub({ + body: FREESTYLED, + labels: [LABEL_NEEDS_TEMPLATE], + comments: [templateComment], + }); + await issueValidation({ github, context: fakeContext({ action: 'edited' }), core: fakeCore() }); + + assert.deepEqual(github.created, []); + assert.equal(github.updated.length, 1); + assert.deepEqual(github.removed, []); +}); + +test('an edit to an issue never flagged is not checked', async () => { + const github = fakeGithub({ body: FREESTYLED }); + await issueValidation({ github, context: fakeContext({ action: 'edited' }), core: fakeCore() }); + + assert.deepEqual(github.created, []); + assert.deepEqual(github.added, []); +}); + +test('reopening a flagged issue that now passes requests validation', async () => { + const github = fakeGithub({ labels: [LABEL_NEEDS_TEMPLATE] }); + await issueValidation({ + github, + context: fakeContext({ action: 'reopened' }), + core: fakeCore(), + }); + + assert.deepEqual(github.removed, [LABEL_NEEDS_TEMPLATE]); + assert.deepEqual(github.created, [BOT_MESSAGE_ISSUE_VALIDATION]); +}); + +test('an edit to a closed issue does nothing until it is reopened', async () => { + const github = fakeGithub({ labels: [LABEL_NEEDS_TEMPLATE], state: 'closed' }); + await issueValidation({ github, context: fakeContext({ action: 'edited' }), core: fakeCore() }); + + assert.deepEqual(github.created, []); + assert.deepEqual(github.removed, []); }); test('a repo outside the community list gets neither', async () => { - const github = fakeGithub(); + const github = fakeGithub({ body: FREESTYLED }); const core = fakeCore(); await issueValidation({ github, context: fakeContext({ repo: 'internal-tooling' }), core }); - assert.deepEqual(github.comments, []); + assert.deepEqual(github.created, []); assert.deepEqual(github.added, []); assert.deepEqual(core.failures, []); }); @@ -88,15 +218,15 @@ test('the sandbox is in scope, so the automation can be tested there', async () const github = fakeGithub(); await issueValidation({ github, context: fakeContext({ repo: SANDBOX }), core: fakeCore() }); - assert.equal(github.comments.length, 1); + assert.equal(github.created.length, 1); assert.deepEqual(github.added, [LABEL_COMMUNITY_ISSUE_REVIEW]); }); -test('an issue already under review gets no second comment', async () => { +test('an issue already under review gets no second request', async () => { const github = fakeGithub({ labels: [LABEL_COMMUNITY_ISSUE_REVIEW] }); await issueValidation({ github, context: fakeContext(), core: fakeCore() }); - assert.deepEqual(github.comments, []); + assert.deepEqual(github.created, []); assert.deepEqual(github.added, []); }); @@ -104,7 +234,7 @@ test('the label match ignores case', async () => { const github = fakeGithub({ labels: ['Community-Issue-Review'] }); await issueValidation({ github, context: fakeContext(), core: fakeCore() }); - assert.deepEqual(github.comments, []); + assert.deepEqual(github.created, []); }); test('a failed label add warns and leaves the run passing', async () => { @@ -112,7 +242,7 @@ test('a failed label add warns and leaves the run passing', async () => { const core = fakeCore(); await issueValidation({ github, context: fakeContext(), core }); - assert.equal(github.comments.length, 1); + assert.equal(github.created.length, 1); assert.deepEqual(core.failures, []); assert.equal(core.warnings.length, 1); assert.match(core.warnings[0], /Failed to add/); diff --git a/scripts/unassign-inactive-issues.js b/scripts/unassign-inactive-issues.js index f5f804e..0be01de 100644 --- a/scripts/unassign-inactive-issues.js +++ b/scripts/unassign-inactive-issues.js @@ -1,4 +1,4 @@ -const { sweepIssueReviewLabels } = require('./issue-review-sweep'); +const { sweepIssueReviewLabels, closeIssuesMissingTemplate } = require('./issue-review-sweep'); const formatUnassignments = unassignments => { if (unassignments.length === 0) return ''; @@ -259,6 +259,7 @@ module.exports = async ({ github, context, core }) => { } await sweepIssueReviewLabels(owner, repo, { github, core }); + await closeIssuesMissingTemplate(owner, repo, { github, core }); const issues = await getAllIssues(github, owner, repo); //console.log(`Processing ${issues.length} open issues`); From 60e95d4bc39c44f9d143297db5837c7192c05107 Mon Sep 17 00:00:00 2001 From: Richard Tibbles Date: Fri, 6 Nov 2026 14:19:38 -0800 Subject: [PATCH 4/4] Run issue validation in every public repo with the automation Replace the ISSUE_VALIDATION_REPOS allowlist with a private repo skip, since installing the automation already chooses the repos. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_013PjXpYijZViQWNrFsbNwwV --- docs/community-automations.md | 5 ++--- scripts/constants.js | 5 ----- scripts/issue-review-sweep.js | 4 ---- scripts/issue-review-sweep.test.js | 15 --------------- scripts/issue-validation.js | 5 ++--- scripts/issue-validation.test.js | 21 +++++---------------- 6 files changed, 9 insertions(+), 46 deletions(-) diff --git a/docs/community-automations.md b/docs/community-automations.md index 14e71b2..25860db 100644 --- a/docs/community-automations.md +++ b/docs/community-automations.md @@ -110,7 +110,7 @@ These two automations ask `rtibblesbot` to validate claims in contributor-submit ## `issue-validation` -Runs on `issues: [opened, edited, reopened]` for open issues whose opener is a contributor, in repositories listed in `ISSUE_VALIDATION_REPOS`. `edited` and `reopened` only run while the issue carries `needs-template`, so issues opened before the check existed are never flagged. +Runs on `issues: [opened, edited, reopened]` for open issues whose opener is a contributor, in public repositories. `edited` and `reopened` only run while the issue carries `needs-template`, so issues opened before the check existed are never flagged. It first checks the description against this repo's `.github/ISSUE_TEMPLATE/*.md`, using the `` rules of `pull-request-template`. The issue does not record which template it came from, so it passes if it satisfies any one of them. @@ -152,7 +152,7 @@ The weekly `unassign-inactive-issues` run also sweeps stalled validations throug If the agent never responds, for example, because the service is down or all available tokens have reached their limits, the label would otherwise remain indefinitely. The sweep removes it when the issue's `labeled` event is older than `ISSUE_REVIEW_MAX_DAYS` days. -In repositories listed in `ISSUE_VALIDATION_REPOS`, the same run closes, as not planned, every open issue labelled `needs-template` for longer than `ISSUE_TEMPLATE_MAX_DAYS` days, with `BOT_MESSAGE_ISSUE_TEMPLATE_CLOSED`. To keep an issue open, a maintainer fixes its description or removes the label. Reopening a closed issue reruns the check and restarts the clock. +The same run closes, as not planned, every open issue labelled `needs-template` for longer than `ISSUE_TEMPLATE_MAX_DAYS` days, with `BOT_MESSAGE_ISSUE_TEMPLATE_CLOSED`. To keep an issue open, a maintainer fixes its description or removes the label. Reopening a closed issue reruns the check and restarts the clock. ## Scope and configuration @@ -160,7 +160,6 @@ Neither automation acts on the validation findings. The agent's comment recommen Configure the following constants in `scripts/constants.js`: -- `ISSUE_VALIDATION_REPOS`: The repositories where validation runs, which are the community repositories plus the sandbox the automations are tested in. - `LABEL_COMMUNITY_ISSUE_REVIEW`: The label applied while validation is in progress. - `ISSUE_VALIDATION_MARKER`: The marker that signals validation completion, shared with `learningequality/coding-agent`. - `ISSUE_REVIEW_MAX_DAYS`: The maximum age of the label before the sweep clears it. diff --git a/scripts/constants.js b/scripts/constants.js index f5148f2..58bdb8f 100644 --- a/scripts/constants.js +++ b/scripts/constants.js @@ -209,10 +209,6 @@ const PR_STATS_REPOS = [ // Repositories in which we accept open-source contributions const COMMUNITY_REPOS = [...PR_STATS_REPOS]; -// No other automation gates on the repo, so all of them already run in the -// sandbox. See docs/testing-automations.md. -const ISSUE_VALIDATION_REPOS = [...COMMUNITY_REPOS, 'test-actions']; - // Every place GitHub reads a pull request template from. The contents API is // case sensitive, so both spellings are needed. const PR_TEMPLATE_PATHS = [ @@ -253,7 +249,6 @@ module.exports = { LABEL_COMMUNITY_REVIEW, BOT_MESSAGE_COMMUNITY_REVIEW, LABEL_COMMUNITY_ISSUE_REVIEW, - ISSUE_VALIDATION_REPOS, ISSUE_VALIDATION_MARKER, ISSUE_VALIDATION_DUPLICATE_VERDICT, ISSUE_VALIDATION_FOLLOW_UPS, diff --git a/scripts/issue-review-sweep.js b/scripts/issue-review-sweep.js index 9ce40e3..147df20 100644 --- a/scripts/issue-review-sweep.js +++ b/scripts/issue-review-sweep.js @@ -1,7 +1,6 @@ // See docs/community-automations.md const { - ISSUE_VALIDATION_REPOS, LABEL_COMMUNITY_ISSUE_REVIEW, ISSUE_REVIEW_MAX_DAYS, LABEL_NEEDS_TEMPLATE, @@ -100,9 +99,6 @@ async function sweepIssueReviewLabels(owner, repo, { github, core }, now = Date. } async function closeIssuesMissingTemplate(owner, repo, { github, core }, now = Date.now()) { - if (!ISSUE_VALIDATION_REPOS.includes(repo)) { - return []; - } const stale = await staleIssues( LABEL_NEEDS_TEMPLATE, ISSUE_TEMPLATE_MAX_DAYS, diff --git a/scripts/issue-review-sweep.test.js b/scripts/issue-review-sweep.test.js index bb4d71e..e069e6d 100644 --- a/scripts/issue-review-sweep.test.js +++ b/scripts/issue-review-sweep.test.js @@ -387,18 +387,3 @@ test('a pull request carrying the template label is not closed', async () => { assert.deepEqual(closed, []); }); - -test('a repo outside issue validation has nothing closed', async () => { - const github = closingGithub({ - issues: [templateIssue(7)], - timelines: { 7: [labeled(daysAgo(30), LABEL_NEEDS_TEMPLATE)] }, - }); - const closed = await closeIssuesMissingTemplate( - 'learningequality', - 'internal-tooling', - { github, core: fakeCore() }, - NOW, - ); - - assert.deepEqual(closed, []); -}); diff --git a/scripts/issue-validation.js b/scripts/issue-validation.js index 9b4c480..988607e 100644 --- a/scripts/issue-validation.js +++ b/scripts/issue-validation.js @@ -1,7 +1,6 @@ // See docs/community-automations.md const { - ISSUE_VALIDATION_REPOS, LABEL_COMMUNITY_ISSUE_REVIEW, LABEL_NEEDS_TEMPLATE, BOT_MESSAGE_ISSUE_VALIDATION, @@ -15,8 +14,8 @@ module.exports = async ({ github, context, core }) => { const repo = context.repo.repo; const issueNumber = context.payload.issue.number; - if (!ISSUE_VALIDATION_REPOS.includes(repo)) { - core.info(`Issue validation does not run in ${repo}, so none is requested.`); + if (context.payload.repository?.private) { + core.info(`Issue validation does not run in private repo ${repo}.`); return; } diff --git a/scripts/issue-validation.test.js b/scripts/issue-validation.test.js index cc5b1e6..16ce0fd 100644 --- a/scripts/issue-validation.test.js +++ b/scripts/issue-validation.test.js @@ -5,14 +5,11 @@ const issueValidation = require('./issue-validation'); const { LABEL_COMMUNITY_ISSUE_REVIEW, LABEL_NEEDS_TEMPLATE, - COMMUNITY_REPOS, BOT_MESSAGE_ISSUE_VALIDATION, ISSUE_TEMPLATE_MARKER, LE_BOT_USERNAME, } = require('./constants'); -const SANDBOX = 'test-actions'; - const FOLLOWS_TEMPLATE = '## Description\n\nThe sync fails.\n\n## AI usage\n\nI used no AI.\n'; // Shaped like kolibri#15779, which an agent wrote without a template. const FREESTYLED = [ @@ -85,10 +82,10 @@ function fakeGithub({ }; } -function fakeContext({ repo = COMMUNITY_REPOS[0], action = 'opened' } = {}) { +function fakeContext({ action = 'opened', isPrivate = false } = {}) { return { - repo: { owner: 'learningequality', repo }, - payload: { action, issue: { number: 7 } }, + repo: { owner: 'learningequality', repo: 'kolibri' }, + payload: { action, issue: { number: 7 }, repository: { private: isPrivate } }, }; } @@ -204,24 +201,16 @@ test('an edit to a closed issue does nothing until it is reopened', async () => assert.deepEqual(github.removed, []); }); -test('a repo outside the community list gets neither', async () => { +test('an issue in a private repo gets neither', async () => { const github = fakeGithub({ body: FREESTYLED }); const core = fakeCore(); - await issueValidation({ github, context: fakeContext({ repo: 'internal-tooling' }), core }); + await issueValidation({ github, context: fakeContext({ isPrivate: true }), core }); assert.deepEqual(github.created, []); assert.deepEqual(github.added, []); assert.deepEqual(core.failures, []); }); -test('the sandbox is in scope, so the automation can be tested there', async () => { - const github = fakeGithub(); - await issueValidation({ github, context: fakeContext({ repo: SANDBOX }), core: fakeCore() }); - - assert.equal(github.created.length, 1); - assert.deepEqual(github.added, [LABEL_COMMUNITY_ISSUE_REVIEW]); -}); - test('an issue already under review gets no second request', async () => { const github = fakeGithub({ labels: [LABEL_COMMUNITY_ISSUE_REVIEW] }); await issueValidation({ github, context: fakeContext(), core: fakeCore() });