diff --git a/.github/workflows/automation-group-issue-comment.yml b/.github/workflows/automation-group-issue-comment.yml new file mode 100644 index 0000000..c536b6e --- /dev/null +++ b/.github/workflows/automation-group-issue-comment.yml @@ -0,0 +1,43 @@ +# This file is generated by scripts/generate-automation.js from automation-registry.yml. +# Do not edit it by hand - edit the registry and run: +# node scripts/generate-automation.js +# +# Reusable workflow called from automation.yml: the automations triggered by issue_comment. +name: "Automation: Issue Comment" +on: + workflow_call: + secrets: + LE_BOT_APP_ID: + description: "Forwarded to: issue-validation-done, contributor-issue-comment" + required: true + LE_BOT_PRIVATE_KEY: + description: "Forwarded to: issue-validation-done, contributor-issue-comment" + required: true + SLACK_COMMUNITY_NOTIFICATIONS_WEBHOOK_URL: + description: "Forwarded to: contributor-issue-comment" + required: false + SLACK_WEBHOOK_URL: + description: "Forwarded to: issue-validation-done, contributor-issue-comment" + required: false +jobs: + issue-validation-done: + name: Issue Validation Done + if: ${{ github.event_name == 'issue_comment' && github.event.action == 'created' }} + uses: learningequality/.github/.github/workflows/issue-validation-done.yml@main + permissions: + contents: read + secrets: + LE_BOT_APP_ID: ${{ secrets.LE_BOT_APP_ID }} + LE_BOT_PRIVATE_KEY: ${{ secrets.LE_BOT_PRIVATE_KEY }} + SLACK_WEBHOOK_URL: ${{ secrets.SLACK_WEBHOOK_URL }} + contributor-issue-comment: + name: Contributor Issue Comment + if: ${{ github.event_name == 'issue_comment' && github.event.action == 'created' }} + uses: learningequality/.github/.github/workflows/contributor-issue-comment.yml@main + permissions: + contents: read + secrets: + LE_BOT_APP_ID: ${{ secrets.LE_BOT_APP_ID }} + LE_BOT_PRIVATE_KEY: ${{ secrets.LE_BOT_PRIVATE_KEY }} + SLACK_WEBHOOK_URL: ${{ secrets.SLACK_WEBHOOK_URL }} + SLACK_COMMUNITY_NOTIFICATIONS_WEBHOOK_URL: ${{ secrets.SLACK_COMMUNITY_NOTIFICATIONS_WEBHOOK_URL }} diff --git a/.github/workflows/automation-group-issues.yml b/.github/workflows/automation-group-issues.yml index 4438def..a2d9165 100644 --- a/.github/workflows/automation-group-issues.yml +++ b/.github/workflows/automation-group-issues.yml @@ -8,10 +8,10 @@ on: workflow_call: secrets: LE_BOT_APP_ID: - description: "Forwarded to: issue-open, issue-label-header, good-first-issue-comment, community-contribution-label" + description: "Forwarded to: issue-open, issue-label-header, good-first-issue-comment, community-contribution-label, issue-validation" required: true LE_BOT_PRIVATE_KEY: - description: "Forwarded to: issue-open, issue-label-header, good-first-issue-comment, community-contribution-label" + description: "Forwarded to: issue-open, issue-label-header, good-first-issue-comment, community-contribution-label, issue-validation" required: true jobs: issue-open: @@ -50,3 +50,12 @@ jobs: secrets: LE_BOT_APP_ID: ${{ secrets.LE_BOT_APP_ID }} LE_BOT_PRIVATE_KEY: ${{ secrets.LE_BOT_PRIVATE_KEY }} + issue-validation: + name: Issue Validation + if: ${{ github.event_name == 'issues' && github.event.action == 'opened' }} + uses: learningequality/.github/.github/workflows/issue-validation.yml@main + permissions: + contents: read + secrets: + LE_BOT_APP_ID: ${{ secrets.LE_BOT_APP_ID }} + LE_BOT_PRIVATE_KEY: ${{ secrets.LE_BOT_PRIVATE_KEY }} diff --git a/.github/workflows/automation.yml b/.github/workflows/automation.yml index 18d6ccf..ddc95ec 100644 --- a/.github/workflows/automation.yml +++ b/.github/workflows/automation.yml @@ -18,16 +18,16 @@ on: description: Unused - accepted until every consumer stops forwarding it required: false LE_BOT_APP_ID: - description: "Forwarded to: review-requested, pull-request-label, dependabot-reviewer, contributor-pr-reply, pull-request-template, issue-open, issue-label-header, good-first-issue-comment, community-contribution-label, contributor-issue-comment, unassign-inactive-issues" + description: "Forwarded to: review-requested, pull-request-label, dependabot-reviewer, contributor-pr-reply, pull-request-template, issue-open, issue-label-header, good-first-issue-comment, community-contribution-label, issue-validation, issue-validation-done, contributor-issue-comment, unassign-inactive-issues" required: true LE_BOT_PRIVATE_KEY: - description: "Forwarded to: review-requested, pull-request-label, dependabot-reviewer, contributor-pr-reply, pull-request-template, issue-open, issue-label-header, good-first-issue-comment, community-contribution-label, contributor-issue-comment, unassign-inactive-issues" + description: "Forwarded to: review-requested, pull-request-label, dependabot-reviewer, contributor-pr-reply, pull-request-template, issue-open, issue-label-header, good-first-issue-comment, community-contribution-label, issue-validation, issue-validation-done, contributor-issue-comment, unassign-inactive-issues" required: true SLACK_COMMUNITY_NOTIFICATIONS_WEBHOOK_URL: description: "Forwarded to: contributor-pr-reply, contributor-issue-comment, unassign-inactive-issues" required: false SLACK_WEBHOOK_URL: - description: "Forwarded to: contributor-issue-comment" + description: "Forwarded to: issue-validation-done, contributor-issue-comment" required: false jobs: pull-request-target: @@ -42,24 +42,24 @@ jobs: SLACK_COMMUNITY_NOTIFICATIONS_WEBHOOK_URL: ${{ secrets.SLACK_COMMUNITY_NOTIFICATIONS_WEBHOOK_URL }} issues: name: Issues - if: ${{ (github.event_name == 'issues' && contains(fromJSON('["opened","reopened"]'), github.event.action)) || (github.event_name == 'issues' && contains(fromJSON('["labeled","unlabeled"]'), github.event.action) && github.event.label.name == 'help wanted') || (github.event_name == 'issues' && github.event.action == 'labeled' && (github.event.label.name == 'good first issue' || github.event.label.name == 'help wanted')) || (github.event_name == 'issues' && contains(fromJSON('["assigned","unassigned"]'), github.event.action)) }} + if: ${{ (github.event_name == 'issues' && contains(fromJSON('["opened","reopened"]'), github.event.action)) || (github.event_name == 'issues' && contains(fromJSON('["labeled","unlabeled"]'), github.event.action) && github.event.label.name == 'help wanted') || (github.event_name == 'issues' && github.event.action == 'labeled' && (github.event.label.name == 'good first issue' || github.event.label.name == 'help wanted')) || (github.event_name == 'issues' && contains(fromJSON('["assigned","unassigned"]'), github.event.action)) || (github.event_name == 'issues' && github.event.action == 'opened') }} uses: learningequality/.github/.github/workflows/automation-group-issues.yml@main permissions: contents: read secrets: LE_BOT_APP_ID: ${{ secrets.LE_BOT_APP_ID }} LE_BOT_PRIVATE_KEY: ${{ secrets.LE_BOT_PRIVATE_KEY }} - contributor-issue-comment: - name: Contributor Issue Comment - if: ${{ github.event_name == 'issue_comment' && github.event.action == 'created' }} - uses: learningequality/.github/.github/workflows/contributor-issue-comment.yml@main + issue-comment: + name: Issue Comment + if: ${{ (github.event_name == 'issue_comment' && github.event.action == 'created') || (github.event_name == 'issue_comment' && github.event.action == 'created') }} + uses: learningequality/.github/.github/workflows/automation-group-issue-comment.yml@main permissions: contents: read secrets: LE_BOT_APP_ID: ${{ secrets.LE_BOT_APP_ID }} LE_BOT_PRIVATE_KEY: ${{ secrets.LE_BOT_PRIVATE_KEY }} - SLACK_WEBHOOK_URL: ${{ secrets.SLACK_WEBHOOK_URL }} SLACK_COMMUNITY_NOTIFICATIONS_WEBHOOK_URL: ${{ secrets.SLACK_COMMUNITY_NOTIFICATIONS_WEBHOOK_URL }} + SLACK_WEBHOOK_URL: ${{ secrets.SLACK_WEBHOOK_URL }} resolve-bot-pr-threads: name: Resolve Bot PR Threads if: ${{ github.event_name == 'pull_request_review_comment' }} diff --git a/.github/workflows/issue-validation-done.yml b/.github/workflows/issue-validation-done.yml new file mode 100644 index 0000000..4b97c05 --- /dev/null +++ b/.github/workflows/issue-validation-done.yml @@ -0,0 +1,57 @@ +name: Clear the issue review label when the validation lands +on: + workflow_call: + secrets: + LE_BOT_APP_ID: + description: 'GitHub App ID for authentication' + required: true + LE_BOT_PRIVATE_KEY: + description: 'GitHub App Private Key for authentication' + required: true + SLACK_WEBHOOK_URL: + description: 'Webhook URL for Slack #support-dev channel' + required: true +jobs: + clear-review-label: + name: Clear review label + if: > + github.event.comment.user.login == 'rtibblesbot' && contains(github.event.comment.body, '') + + runs-on: ubuntu-latest + steps: + - name: Generate App Token + id: generate-token + uses: actions/create-github-app-token@v3 + with: + app-id: ${{ secrets.LE_BOT_APP_ID }} + private-key: ${{ secrets.LE_BOT_PRIVATE_KEY }} + permission-issues: write + - name: Checkout .github repository + uses: actions/checkout@v7 + with: + repository: learningequality/.github + ref: main + - name: Setup Node.js + uses: actions/setup-node@v7 + with: + node-version: '22' + - name: Install dependencies + run: yarn install --frozen-lockfile + - name: Run script + id: script + uses: actions/github-script@v9 + with: + github-token: ${{ steps.generate-token.outputs.token }} + script: | + const script = require('./scripts/issue-validation-done.js'); + return await script({ github, context, core }); + - name: Send Slack notification to support-dev channel + if: ${{ steps.script.outputs.support_dev_message }} + uses: slackapi/slack-github-action@v4.0.0 + with: + webhook-type: incoming-webhook + webhook: ${{ secrets.SLACK_WEBHOOK_URL }} + payload: | + { + "text": "${{ steps.script.outputs.support_dev_message }}" + } diff --git a/.github/workflows/issue-validation.yml b/.github/workflows/issue-validation.yml new file mode 100644 index 0000000..b1a533b --- /dev/null +++ b/.github/workflows/issue-validation.yml @@ -0,0 +1,54 @@ +name: Ask rtibblesbot to validate a contributor issue +on: + workflow_call: + secrets: + LE_BOT_APP_ID: + description: 'GitHub App ID for authentication' + required: true + LE_BOT_PRIVATE_KEY: + description: 'GitHub App Private Key for authentication' + required: true +permissions: + contents: read +jobs: + check-if-contributor: + name: Check if author is contributor + uses: learningequality/.github/.github/workflows/is-contributor.yml@main + secrets: + LE_BOT_APP_ID: ${{ secrets.LE_BOT_APP_ID }} + LE_BOT_PRIVATE_KEY: ${{ secrets.LE_BOT_PRIVATE_KEY }} + with: + username: ${{ github.event.issue.user.login }} + author_association: ${{ github.event.issue.author_association }} + request-validation: + name: Request validation + needs: [check-if-contributor] + if: ${{ needs.check-if-contributor.outputs.is_contributor == 'true' }} + runs-on: ubuntu-latest + steps: + - name: Generate App Token + id: generate-token + uses: actions/create-github-app-token@v3 + with: + app-id: ${{ secrets.LE_BOT_APP_ID }} + private-key: ${{ secrets.LE_BOT_PRIVATE_KEY }} + permission-issues: write + - name: Checkout .github repository + uses: actions/checkout@v7 + with: + repository: learningequality/.github + ref: main + - name: Setup Node.js + uses: actions/setup-node@v7 + with: + node-version: '22' + - name: Install dependencies + run: yarn install --frozen-lockfile + - name: Run script + id: script + uses: actions/github-script@v9 + with: + github-token: ${{ steps.generate-token.outputs.token }} + script: | + const script = require('./scripts/issue-validation.js'); + return await script({ github, context, core }); diff --git a/automation-registry.yml b/automation-registry.yml index 74dd2a6..29e7e7d 100644 --- a/automation-registry.yml +++ b/automation-registry.yml @@ -128,6 +128,27 @@ automations: secrets: LE_BOT_APP_ID: required LE_BOT_PRIVATE_KEY: required + - name: issue-validation + enabled: true + leaf: issue-validation.yml + on: + issues: + types: [opened] + if: "github.event_name == 'issues' && github.event.action == 'opened'" + secrets: + LE_BOT_APP_ID: required + LE_BOT_PRIVATE_KEY: required + - name: issue-validation-done + enabled: true + leaf: issue-validation-done.yml + on: + issue_comment: + types: [created] + if: "github.event_name == 'issue_comment' && github.event.action == 'created'" + secrets: + LE_BOT_APP_ID: required + LE_BOT_PRIVATE_KEY: required + SLACK_WEBHOOK_URL: optional - name: contributor-issue-comment enabled: true leaf: contributor-issue-comment.yml diff --git a/docs/community-automations.md b/docs/community-automations.md index 561c198..a18b5bc 100644 --- a/docs/community-automations.md +++ b/docs/community-automations.md @@ -102,6 +102,62 @@ Four states fail, and the log names every one of them rather than stopping at th The failed check is the only report. Nothing is posted on the pull request. +# `issue-validation` and `issue-validation-done` + +These two automations ask `rtibblesbot` to validate claims in contributor-submitted issues and track the review while it is in progress. + +## `issue-validation` + +Runs on `issues: [opened]`. It proceeds only if the repository is in `ISSUE_VALIDATION_REPOS`, the issue opener is a contributor, and the issue does not already have the `community-issue-review` label. + +It posts a comment mentioning `@rtibblesbot` and adds the label. If the 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. + +## `issue-validation-done` + +Runs on `issue_comment: [created]`. It removes the `community-issue-review` label only when both conditions are met: + +- The comment contains the `` marker. +- The comment author is `rtibblesbot`. + +Both checks are necessary. Anyone can include the marker in a comment, and `rtibblesbot` also comments on issues for other reasons. + +The leaf workflow tests both conditions in its job `if:`, as `resolve-bot-pr-threads` does, so an ordinary comment starts no runner. The script tests them again. + +This logic cannot live in `contributor-issue-comment`, because that automation deliberately ignores bot comments. + +It also posts one line to Slack `#support-dev` for every validation comment, so a maintainer knows to follow up. The line carries the verdict, a link to the comment, a link to the issue, the reporter's handle, and the follow-up the verdict calls for: + +``` +*[kolibri] on issue: reported by _octocat_* _(consider closing as duplicate)_ +``` + +`ISSUE_VALIDATION_FOLLOW_UPS` holds the six verdicts and their follow-ups. `learningequality/coding-agent#1229` fixes the vocabulary from the agent's side. + +The agent's LLM session writes the verdict line, so its shape varies. `readVerdict` therefore mirrors the agent's own reader, which tolerates a missing bold, a colon on either side of it, a lowercase label, a list or quote prefix, backticks, and trailing punctuation. Keep the two readers in step. A stricter reader here reports a healthy run as broken. + +A comment outside all of that reads `Validation: no verdict read` and asks a maintainer to check the agent run. The agent posts no comment at all when it states no verdict, so this line means the rendering drifted, not that a verdict is missing. + +## Handling stalled reviews + +The weekly `unassign-inactive-issues` run also sweeps stalled validations through `scripts/issue-review-sweep.js`. + +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. + +## 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. + +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. +- `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. + # `holiday-message` Sends a holiday message to community pull requests and issue comments. diff --git a/scripts/constants.js b/scripts/constants.js index d92e6e3..35a610c 100644 --- a/scripts/constants.js +++ b/scripts/constants.js @@ -87,6 +87,24 @@ const MAX_ASSIGNED_ISSUES = 2; const ASSIGN_COOLDOWN_DAYS = 7; const ASSIGN_GUIDANCE_MARKER = ''; const LABEL_COMMUNITY_REVIEW = 'community-review'; +const LABEL_COMMUNITY_ISSUE_REVIEW = 'community-issue-review'; +// rtibblesbot opens its validation comment with this marker. The contract is +// stated from the agent's side in learningequality/coding-agent#1229. +const ISSUE_VALIDATION_MARKER = ''; +const ISSUE_REVIEW_MAX_DAYS = 3; + +// The same contract fixes these six verdicts, one per validation comment. +const ISSUE_VALIDATION_DUPLICATE_VERDICT = 'duplicate of #N'; +const ISSUE_VALIDATION_FOLLOW_UPS = { + 'claim holds': 'ready for triage', + 'claim does not hold': 'consider closing', + [ISSUE_VALIDATION_DUPLICATE_VERDICT]: 'consider closing as duplicate', + 'already fixed': 'consider closing as fixed', + 'not enough information': 'needs more from the reporter', + 'out of scope': 'consider closing', +}; +const ISSUE_VALIDATION_NO_VERDICT = 'no verdict read'; +const ISSUE_VALIDATION_NO_VERDICT_FOLLOW_UP = 'check the agent run'; // Will be attached to bot messages when not empty // const GSOC_NOTE = ''; @@ -154,6 +172,18 @@ const BOT_MESSAGE_RTIBBLESBOT_REVIEW = `📢✨ **Before we assign a reviewer, w const BOT_MESSAGE_COMMUNITY_REVIEW = `📢✨ **Before we assign a reviewer, we'll invite community pre-review. See the [community review guidance](https://github.com/learningequality/.github/blob/main/docs/community-review.md) for both authors and reviewers.**`; +const BOT_MESSAGE_ISSUE_VALIDATION = + `🔍 **Thanks for opening this issue!** @rtibblesbot will check it against the code ` + + `and comment with its findings.\n\n` + + `**What happens next:**\n\n` + + `- The \`${LABEL_COMMUNITY_ISSUE_REVIEW}\` label stays on until the review comment is posted.\n` + + `- The comment identifies the files, lines, or commits examined so you can follow the ` + + `reasoning and respond if you disagree.\n` + + `- If the check confirms the issue, @rtibblesbot may also correct the title and ` + + `description. GitHub preserves the edit history, so the original text remains available.\n` + + `- @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.`; + // Repositories to include in PR statistics reports const PR_STATS_REPOS = [ 'kolibri', @@ -162,11 +192,16 @@ const PR_STATS_REPOS = [ 'le-utils', '.github', 'ricecooker', + 'morango', ]; // 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 = [ @@ -205,6 +240,15 @@ module.exports = { RTIBBLESBOT_USERNAME, 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, + ISSUE_VALIDATION_NO_VERDICT, + ISSUE_VALIDATION_NO_VERDICT_FOLLOW_UP, + ISSUE_REVIEW_MAX_DAYS, + BOT_MESSAGE_ISSUE_VALIDATION, TEAMS_WITH_CLOSE_CONTRIBUTORS, HOLIDAY_MESSAGE, PR_STATS_REPOS, diff --git a/scripts/contributor-pr-reply.js b/scripts/contributor-pr-reply.js index 613e707..e16c5c7 100644 --- a/scripts/contributor-pr-reply.js +++ b/scripts/contributor-pr-reply.js @@ -5,7 +5,7 @@ const { LABEL_COMMUNITY_REVIEW, RTIBBLESBOT_USERNAME, } = require('./constants'); -const { sendBotMessage, getLinkedIssues } = require('./utils'); +const { sendBotMessage, getLinkedIssues, escapeIssueTitleForSlackMessage } = require('./utils'); module.exports = async ({ github, context, core }) => { try { @@ -13,7 +13,7 @@ module.exports = async ({ github, context, core }) => { const repo = context.repo.repo; const number = context.payload.pull_request.number; const url = context.payload.pull_request.html_url; - const title = context.payload.pull_request.title; + const title = escapeIssueTitleForSlackMessage(context.payload.pull_request.title); const author = context.payload.pull_request.user.login; const lookup = await getLinkedIssues(number, { github, context, core }); diff --git a/scripts/contributor-pr-reply.test.js b/scripts/contributor-pr-reply.test.js index 94593ee..042524d 100644 --- a/scripts/contributor-pr-reply.test.js +++ b/scripts/contributor-pr-reply.test.js @@ -6,14 +6,14 @@ const { LABEL_COMMUNITY_REVIEW, RTIBBLESBOT_USERNAME } = require('./constants'); const AUTHOR = 'outsider'; -function fakeContext(author = AUTHOR) { +function fakeContext(author = AUTHOR, title = 'Fix it') { return { repo: { owner: 'learningequality', repo: 'kolibri' }, payload: { pull_request: { number: 42, html_url: 'https://github.com/learningequality/kolibri/pull/42', - title: 'Fix it', + title, user: { login: author }, }, }, @@ -231,6 +231,22 @@ test('a failed lookup falls back to the standard reply', async () => { assert.deepEqual(core.failures, []); }); +test('a title holding a quote and a backslash reaches Slack escaped', async () => { + const github = fakeGithub([{ number: 7, assignees: [AUTHOR] }]); + const core = await run(github, fakeContext(AUTHOR, 'Treat "done" as C:\\full')); + + const line = core.outputs.slack_notification; + assert.match(line, /Treat \\"done\\" as C:\\\\full/); + assert.doesNotThrow(() => JSON.parse(`{"text": "${line}"}`)); +}); + +test('a title holding angle brackets cannot end the Slack link early', async () => { + const github = fakeGithub([{ number: 7, assignees: [AUTHOR] }]); + const core = await run(github, fakeContext(AUTHOR, 'Remove ')); + + assert.match(core.outputs.slack_notification, /Remove <KButton>/); +}); + test('a failed lookup does not ask the author to link an issue', async () => { const github = fakeGithub([], { graphqlError: 'API down' }); await run(github); diff --git a/scripts/is-contributor.test.js b/scripts/is-contributor.test.js new file mode 100644 index 0000000..76380b9 --- /dev/null +++ b/scripts/is-contributor.test.js @@ -0,0 +1,111 @@ +const test = require('node:test'); +const assert = require('node:assert/strict'); + +const isContributorScript = require('./is-contributor'); +const { CLOSE_CONTRIBUTORS, LE_BOT_USERNAME } = require('./constants'); + +const CLOSE_CONTRIBUTOR = CLOSE_CONTRIBUTORS[0]; + +function fakeCore() { + const outputs = {}; + const failures = []; + return { + outputs, + failures, + info() {}, + warning() {}, + error() {}, + setOutput(name, value) { + outputs[name] = value; + }, + setFailed(message) { + failures.push(message); + }, + }; +} + +async function run(username, authorAssociation) { + const before = { + USERNAME: process.env.USERNAME, + AUTHOR_ASSOCIATION: process.env.AUTHOR_ASSOCIATION, + }; + const core = fakeCore(); + try { + if (username === undefined) delete process.env.USERNAME; + else process.env.USERNAME = username; + if (authorAssociation === undefined) delete process.env.AUTHOR_ASSOCIATION; + else process.env.AUTHOR_ASSOCIATION = authorAssociation; + + await isContributorScript({ + core, + github: {}, + context: { repo: { owner: 'learningequality', repo: 'kolibri' } }, + }); + } finally { + for (const [name, value] of Object.entries(before)) { + if (value === undefined) delete process.env[name]; + else process.env[name] = value; + } + } + return core; +} + +test('a first time contributor is a contributor', async () => { + const core = await run('outsider', 'FIRST_TIME_CONTRIBUTOR'); + + assert.equal(core.outputs.is_contributor, true); + assert.deepEqual(core.failures, []); +}); + +test('an account with no association to the repo is a contributor', async () => { + const core = await run('outsider', 'NONE'); + + assert.equal(core.outputs.is_contributor, true); +}); + +test('an organization owner is not a contributor', async () => { + const core = await run('owner', 'OWNER'); + + assert.equal(core.outputs.is_contributor, false); +}); + +test('a bot is not a contributor', async () => { + const core = await run(LE_BOT_USERNAME, 'NONE'); + + assert.equal(core.outputs.is_contributor, false); +}); + +test('a team member who is not a close contributor is not a contributor', async () => { + const core = await run('coreteam', 'MEMBER'); + + assert.equal(core.outputs.is_contributor, false); +}); + +test('a close contributor stays a contributor even as a team member', async () => { + // GSoC and other team memberships make some close contributors 'MEMBER'. + const core = await run(CLOSE_CONTRIBUTOR, 'MEMBER'); + + assert.equal(core.outputs.is_contributor, true); +}); + +test('a close contributor is matched whatever case the login arrives in', async () => { + const core = await run(CLOSE_CONTRIBUTOR.toUpperCase(), 'MEMBER'); + + assert.equal(core.outputs.is_contributor, true); +}); + +test('a missing username fails the run', async () => { + const core = await run(undefined, 'NONE'); + + assert.equal(core.outputs.is_contributor, false); + assert.equal(core.failures.length, 1); + assert.match(core.failures[0], /Missing username/); +}); + +test('a missing author association fails the run', async () => { + const core = await run('outsider', undefined); + + assert.equal(core.outputs.is_contributor, false); + assert.equal(core.failures.length, 1); + assert.match(core.failures[0], /Missing authorAssociation/); +}); diff --git a/scripts/issue-review-sweep.js b/scripts/issue-review-sweep.js new file mode 100644 index 0000000..2abf681 --- /dev/null +++ b/scripts/issue-review-sweep.js @@ -0,0 +1,89 @@ +// See docs/community-automations.md + +const { LABEL_COMMUNITY_ISSUE_REVIEW, ISSUE_REVIEW_MAX_DAYS } = 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. + */ +async function labeledAt(name, owner, repo, issueNumber, github, core) { + try { + const events = await github.paginate(github.rest.issues.listEventsForTimeline, { + owner, + repo, + issue_number: issueNumber, + per_page: 100, + }); + const applied = events.filter( + event => event.event === 'labeled' && event.label?.name?.toLowerCase() === name.toLowerCase(), + ); + if (!applied.length) { + return null; + } + return new Date(applied[applied.length - 1].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()) { + let issues = []; + try { + issues = await github.paginate(github.rest.issues.listForRepo, { + owner, + repo, + state: 'open', + labels: LABEL_COMMUNITY_ISSUE_REVIEW, + per_page: 100, + }); + } catch (error) { + core.warning(`Failed to list issues under review in ${owner}/${repo}: ${error.message}`); + return []; + } + + const cleared = []; + 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 removed = await removeLabel( + LABEL_COMMUNITY_ISSUE_REVIEW, + owner, + repo, + issue.number, + github, + core, + ); + if (removed) { + cleared.push(issue.number); + core.info( + `Removed ${LABEL_COMMUNITY_ISSUE_REVIEW} from issue #${issue.number}, ` + + `applied ${applied.toISOString()}.`, + ); + } + } + return cleared; +} + +module.exports = { sweepIssueReviewLabels }; diff --git a/scripts/issue-review-sweep.test.js b/scripts/issue-review-sweep.test.js new file mode 100644 index 0000000..79dc5df --- /dev/null +++ b/scripts/issue-review-sweep.test.js @@ -0,0 +1,260 @@ +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 NOW = Date.parse('2026-10-26T12:00:00Z'); +const MS_PER_DAY = 24 * 60 * 60 * 1000; + +const daysAgo = days => new Date(NOW - days * MS_PER_DAY).toISOString(); + +const labeled = (at, name = LABEL_COMMUNITY_ISSUE_REVIEW) => ({ + event: 'labeled', + label: { name }, + created_at: at, +}); + +const unlabeled = (at, name = LABEL_COMMUNITY_ISSUE_REVIEW) => ({ + event: 'unlabeled', + label: { name }, + created_at: at, +}); + +function fakeGithub({ + issues = [], + timelines = {}, + listFails = false, + timelineFails = false, + removeFails = false, +} = {}) { + const removed = []; + // An entry is a number, which carries the review label, or { number, labels } + // to model an issue carrying other labels. + const all = issues.map(issue => + typeof issue === 'number' ? { number: issue, labels: [LABEL_COMMUNITY_ISSUE_REVIEW] } : issue, + ); + return { + removed, + paginate: async (fn, params) => fn(params), + rest: { + issues: { + listForRepo: async ({ labels }) => { + if (listFails) { + throw new Error('server error'); + } + const wanted = (labels || '').split(',').filter(Boolean); + return all.filter(issue => wanted.every(name => issue.labels.includes(name))); + }, + listEventsForTimeline: async ({ issue_number }) => { + if (timelineFails) { + throw new Error('server error'); + } + return timelines[issue_number] || []; + }, + removeLabel: async ({ name, issue_number }) => { + if (removeFails) { + const error = new Error('server error'); + error.status = 500; + throw error; + } + removed.push({ name, issue_number }); + }, + }, + }, + }; +} + +function fakeCore() { + const warnings = []; + return { + warnings, + info() {}, + warning(message) { + warnings.push(message); + }, + }; +} + +test('a label older than the limit comes off', async () => { + const github = fakeGithub({ + issues: [7], + timelines: { 7: [labeled(daysAgo(ISSUE_REVIEW_MAX_DAYS + 1))] }, + }); + const cleared = await sweepIssueReviewLabels( + 'learningequality', + 'kolibri', + { + github, + core: fakeCore(), + }, + NOW, + ); + + assert.deepEqual(cleared, [7]); + assert.deepEqual(github.removed, [{ name: LABEL_COMMUNITY_ISSUE_REVIEW, issue_number: 7 }]); +}); + +test('a label younger than the limit stays on', async () => { + const github = fakeGithub({ + issues: [7], + timelines: { 7: [labeled(daysAgo(ISSUE_REVIEW_MAX_DAYS - 1))] }, + }); + const cleared = await sweepIssueReviewLabels( + 'learningequality', + 'kolibri', + { + github, + core: fakeCore(), + }, + NOW, + ); + + assert.deepEqual(cleared, []); + assert.deepEqual(github.removed, []); +}); + +test('a label at exactly the limit stays on', async () => { + const github = fakeGithub({ + issues: [7], + timelines: { 7: [labeled(daysAgo(ISSUE_REVIEW_MAX_DAYS))] }, + }); + const cleared = await sweepIssueReviewLabels( + 'learningequality', + 'kolibri', + { + github, + core: fakeCore(), + }, + NOW, + ); + + assert.deepEqual(cleared, []); +}); + +test('the most recent application decides', async () => { + // Labelled long ago, cleared, then labelled again today. + const github = fakeGithub({ + issues: [7], + timelines: { + 7: [labeled(daysAgo(30)), unlabeled(daysAgo(29)), labeled(daysAgo(0))], + }, + }); + const cleared = await sweepIssueReviewLabels( + 'learningequality', + 'kolibri', + { + github, + core: fakeCore(), + }, + NOW, + ); + + assert.deepEqual(cleared, []); +}); + +test('a labeled event for another label is not read as this one', async () => { + const github = fakeGithub({ + issues: [7], + timelines: { 7: [labeled(daysAgo(30), 'help wanted')] }, + }); + const cleared = await sweepIssueReviewLabels( + 'learningequality', + 'kolibri', + { + github, + core: fakeCore(), + }, + NOW, + ); + + assert.deepEqual(cleared, []); +}); + +test('each issue is judged on its own age', async () => { + const github = fakeGithub({ + issues: [7, 8], + timelines: { + 7: [labeled(daysAgo(ISSUE_REVIEW_MAX_DAYS + 1))], + 8: [labeled(daysAgo(1))], + }, + }); + const cleared = await sweepIssueReviewLabels( + 'learningequality', + 'kolibri', + { + github, + core: fakeCore(), + }, + NOW, + ); + + assert.deepEqual(cleared, [7]); +}); + +test('an issue that no longer carries the label is left alone', async () => { + // The timeline keeps the old `labeled` event, so only the label filter on the + // listing keeps this issue out of the sweep. + const github = fakeGithub({ + issues: [{ number: 7, labels: ['help wanted'] }], + timelines: { 7: [labeled(daysAgo(30)), unlabeled(daysAgo(29))] }, + }); + const cleared = await sweepIssueReviewLabels( + 'learningequality', + 'kolibri', + { github, core: fakeCore() }, + NOW, + ); + + assert.deepEqual(cleared, []); + assert.deepEqual(github.removed, []); +}); + +test('a removal this run cannot make is not counted as cleared', async () => { + const github = fakeGithub({ + issues: [7], + timelines: { 7: [labeled(daysAgo(ISSUE_REVIEW_MAX_DAYS + 1))] }, + removeFails: true, + }); + const core = fakeCore(); + const cleared = await sweepIssueReviewLabels( + 'learningequality', + 'kolibri', + { github, core }, + NOW, + ); + + assert.deepEqual(cleared, []); + assert.equal(core.warnings.length, 1); + assert.match(core.warnings[0], /Failed to remove/); +}); + +test('a timeline this run cannot read leaves the label on', async () => { + const github = fakeGithub({ issues: [7], timelineFails: true }); + const core = fakeCore(); + const cleared = await sweepIssueReviewLabels( + 'learningequality', + 'kolibri', + { github, core }, + NOW, + ); + + assert.deepEqual(cleared, []); + assert.equal(core.warnings.length, 1); + assert.match(core.warnings[0], /Failed to read the timeline/); +}); + +test('a listing this run cannot read sweeps nothing', async () => { + const github = fakeGithub({ listFails: true }); + const core = fakeCore(); + const cleared = await sweepIssueReviewLabels( + 'learningequality', + 'kolibri', + { github, core }, + NOW, + ); + + assert.deepEqual(cleared, []); + assert.equal(core.warnings.length, 1); + assert.match(core.warnings[0], /Failed to list issues under review/); +}); diff --git a/scripts/issue-validation-done.js b/scripts/issue-validation-done.js new file mode 100644 index 0000000..b29bb20 --- /dev/null +++ b/scripts/issue-validation-done.js @@ -0,0 +1,84 @@ +// See docs/community-automations.md + +const { + ISSUE_VALIDATION_MARKER, + ISSUE_VALIDATION_DUPLICATE_VERDICT, + ISSUE_VALIDATION_FOLLOW_UPS, + ISSUE_VALIDATION_NO_VERDICT, + ISSUE_VALIDATION_NO_VERDICT_FOLLOW_UP, + LABEL_COMMUNITY_ISSUE_REVIEW, + RTIBBLESBOT_USERNAME, +} = require('./constants'); +const { removeLabel, escapeIssueTitleForSlackMessage } = require('./utils'); + +// The verdict line is written by the agent's LLM session, not by a template, so +// its shape varies. These three mirror the agent's own reader, which has to +// tolerate that: _LIST_NUMBER_RE, _VERDICT_RE and the strips in parse_verdict +// and classify_verdict, all in learningequality/coding-agent. +const LIST_NUMBER = /^(\s*)\d+[.)][ \t]+/gm; +const VERDICT_LINE = /^[\s>*_-]*verdict[*_\s]*:\s*(.+?)\s*$/im; +const VERDICT_TRIM = /^[`*_\s.!;]+|[`*_\s.!;]+$/g; +const DUPLICATE_VERDICT = /^duplicate of #\d+$/; + +function readVerdict(body) { + const match = VERDICT_LINE.exec((body || '').replace(LIST_NUMBER, '$1')); + if (!match) { + return null; + } + + const verdict = match[1].toLowerCase().replace(VERDICT_TRIM, ''); + if (DUPLICATE_VERDICT.test(verdict)) { + return { verdict, followUp: ISSUE_VALIDATION_FOLLOW_UPS[ISSUE_VALIDATION_DUPLICATE_VERDICT] }; + } + + const followUp = ISSUE_VALIDATION_FOLLOW_UPS[verdict]; + return followUp ? { verdict, followUp } : null; +} + +module.exports = async ({ github, context, core }) => { + try { + const issue = context.payload.issue; + const comment = context.payload.comment; + const author = comment.user.login; + + if (issue.pull_request) { + core.info('The comment is on a pull request, so there is no review label to clear.'); + return; + } + + if (author !== RTIBBLESBOT_USERNAME) { + core.info(`${author} is not ${RTIBBLESBOT_USERNAME}, so the label stays on.`); + return; + } + + if (!(comment.body || '').includes(ISSUE_VALIDATION_MARKER)) { + core.info('The comment carries no validation marker, so the label stays on.'); + return; + } + + const read = readVerdict(comment.body); + const verdict = read ? read.verdict : ISSUE_VALIDATION_NO_VERDICT; + const followUp = read ? read.followUp : ISSUE_VALIDATION_NO_VERDICT_FOLLOW_UP; + const title = escapeIssueTitleForSlackMessage(issue.title || ''); + core.setOutput( + 'support_dev_message', + `*[${context.repo.repo}] <${comment.html_url}|Validation: ${verdict}> ` + + `on issue: <${issue.html_url}|${title}> reported by _${issue.user.login}_* ` + + `_(${followUp})_`, + ); + + const removed = await removeLabel( + LABEL_COMMUNITY_ISSUE_REVIEW, + context.repo.owner, + context.repo.repo, + issue.number, + github, + core, + ); + if (removed) { + core.info(`Removed ${LABEL_COMMUNITY_ISSUE_REVIEW} from issue #${issue.number}.`); + } + } catch (error) { + core.setFailed(`Action failed with error: ${error.message}`); + } +}; diff --git a/scripts/issue-validation-done.test.js b/scripts/issue-validation-done.test.js new file mode 100644 index 0000000..59a7776 --- /dev/null +++ b/scripts/issue-validation-done.test.js @@ -0,0 +1,293 @@ +const test = require('node:test'); +const assert = require('node:assert/strict'); + +const issueValidationDone = require('./issue-validation-done'); +const { + ISSUE_VALIDATION_MARKER, + ISSUE_VALIDATION_FOLLOW_UPS, + ISSUE_VALIDATION_DUPLICATE_VERDICT, + LABEL_COMMUNITY_ISSUE_REVIEW, + RTIBBLESBOT_USERNAME, +} = require('./constants'); + +const VERDICT = `${ISSUE_VALIDATION_MARKER}\n\n**Verdict:** claim holds`; +const COMMENT_URL = 'https://github.com/learningequality/kolibri/issues/7#issuecomment-1'; +const ISSUE_URL = 'https://github.com/learningequality/kolibri/issues/7'; + +function body(verdict) { + return `${ISSUE_VALIDATION_MARKER}\n\n**Verdict:** ${verdict}\n**Evidence:** \`a.py:1\``; +} + +function fakeGithub({ status = 200 } = {}) { + const removed = []; + return { + removed, + rest: { + issues: { + removeLabel: async ({ name, issue_number }) => { + if (status !== 200) { + const error = new Error(status === 404 ? 'Label does not exist' : 'server error'); + error.status = status; + throw error; + } + removed.push({ name, issue_number }); + }, + }, + }, + }; +} + +function fakeContext({ + author = RTIBBLESBOT_USERNAME, + commentBody = VERDICT, + pullRequest = false, + title = 'Server PID file kills the test run', + reporter = 'octocat', +} = {}) { + const issue = { number: 7, title, html_url: ISSUE_URL, user: { login: reporter } }; + if (pullRequest) { + issue.pull_request = { url: 'https://api.github.com/repos/learningequality/kolibri/pulls/7' }; + } + return { + repo: { owner: 'learningequality', repo: 'kolibri' }, + payload: { + issue, + comment: { body: commentBody, html_url: COMMENT_URL, user: { login: author } }, + }, + }; +} + +function fakeCore() { + const failures = []; + const warnings = []; + const outputs = {}; + return { + failures, + warnings, + outputs, + info() {}, + warning(message) { + warnings.push(message); + }, + error() {}, + setOutput(name, value) { + outputs[name] = value; + }, + setFailed(message) { + failures.push(message); + }, + }; +} + +test('a marker comment from rtibblesbot removes the label', async () => { + const github = fakeGithub(); + const core = fakeCore(); + await issueValidationDone({ github, context: fakeContext(), core }); + + assert.deepEqual(github.removed, [{ name: LABEL_COMMUNITY_ISSUE_REVIEW, issue_number: 7 }]); + assert.deepEqual(core.failures, []); +}); + +test('the same marker from another account leaves the label on', async () => { + const github = fakeGithub(); + // The marker is plain text that anyone can paste, so the author decides too. + await issueValidationDone({ + github, + context: fakeContext({ author: 'contributor' }), + core: fakeCore(), + }); + + assert.deepEqual(github.removed, []); +}); + +test('a comment from rtibblesbot without the marker leaves the label on', async () => { + const github = fakeGithub(); + await issueValidationDone({ + github, + context: fakeContext({ commentBody: 'Looks like a duplicate of #42.' }), + core: fakeCore(), + }); + + assert.deepEqual(github.removed, []); +}); + +test('an empty comment body does not throw', async () => { + const github = fakeGithub(); + const core = fakeCore(); + await issueValidationDone({ github, context: fakeContext({ commentBody: null }), core }); + + assert.deepEqual(github.removed, []); + assert.deepEqual(core.failures, []); +}); + +test('a pull request comment is ignored', async () => { + const github = fakeGithub(); + await issueValidationDone({ + github, + context: fakeContext({ pullRequest: true }), + core: fakeCore(), + }); + + assert.deepEqual(github.removed, []); +}); + +test('a label that is already off does not fail the run', async () => { + const github = fakeGithub({ status: 404 }); + const core = fakeCore(); + await issueValidationDone({ github, context: fakeContext(), core }); + + assert.deepEqual(core.failures, []); + assert.deepEqual(core.warnings, []); +}); + +test('any other remove failure warns and leaves the run passing', async () => { + const github = fakeGithub({ status: 500 }); + const core = fakeCore(); + await issueValidationDone({ github, context: fakeContext(), core }); + + assert.deepEqual(core.failures, []); + assert.equal(core.warnings.length, 1); + assert.match(core.warnings[0], /Failed to remove/); +}); + +for (const [verdict, followUp] of Object.entries(ISSUE_VALIDATION_FOLLOW_UPS)) { + const stated = verdict === ISSUE_VALIDATION_DUPLICATE_VERDICT ? 'duplicate of #15001' : verdict; + + test(`the notification carries the ${verdict} verdict and its follow-up`, async () => { + const core = fakeCore(); + await issueValidationDone({ + github: fakeGithub(), + context: fakeContext({ commentBody: body(stated) }), + core, + }); + + assert.equal( + core.outputs.support_dev_message, + `*[kolibri] <${COMMENT_URL}|Validation: ${stated}> on issue: ` + + `<${ISSUE_URL}|Server PID file kills the test run> reported by _octocat_* _(${followUp})_`, + ); + }); +} + +test('a comment with no verdict line still notifies', async () => { + const core = fakeCore(); + await issueValidationDone({ + github: fakeGithub(), + context: fakeContext({ commentBody: `${ISSUE_VALIDATION_MARKER}\n\nNothing to report.` }), + core, + }); + + assert.match(core.outputs.support_dev_message, /Validation: no verdict read/); + assert.match(core.outputs.support_dev_message, /_\(check the agent run\)_$/); +}); + +// The agent's LLM session writes the verdict line, so its shape varies. Each of +// these is accepted by the reader in learningequality/coding-agent, so it must +// be accepted here too. +for (const [line, shape] of [ + ['**Verdict:** claim holds', 'the shape seen live'], + ['**Verdict**: claim holds', 'the colon outside the bold'], + ['Verdict: claim holds', 'no bold'], + ['**verdict:** claim holds', 'a lowercase label'], + ['- **Verdict:** claim holds', 'a list item'], + ['1. **Verdict:** claim holds', 'a numbered list item'], + ['> **Verdict:** claim holds', 'a quote'], + ['**Verdict:** Claim holds.', 'a capital and a full stop'], + ['**Verdict:** `claim holds`', 'the value in backticks'], +]) { + test(`a verdict stated with ${shape} is read`, async () => { + const core = fakeCore(); + await issueValidationDone({ + github: fakeGithub(), + context: fakeContext({ + commentBody: `${ISSUE_VALIDATION_MARKER}\n\n${line}\n**Evidence:** \`a.py:1\``, + }), + core, + }); + + assert.match(core.outputs.support_dev_message, /Validation: claim holds>/); + assert.match(core.outputs.support_dev_message, /_\(ready for triage\)_$/); + }); +} + +test('a duplicate verdict is read with the colon outside the bold', async () => { + const core = fakeCore(); + await issueValidationDone({ + github: fakeGithub(), + context: fakeContext({ + commentBody: `${ISSUE_VALIDATION_MARKER}\n\n**Verdict**: duplicate of #15001\n**Evidence:** x`, + }), + core, + }); + + assert.match(core.outputs.support_dev_message, /Validation: duplicate of #15001>/); +}); + +test('a body with CRLF line endings reads the same verdict', async () => { + const core = fakeCore(); + await issueValidationDone({ + github: fakeGithub(), + context: fakeContext({ commentBody: body('claim holds').replace(/\n/g, '\r\n') }), + core, + }); + + assert.match(core.outputs.support_dev_message, /Validation: claim holds>/); +}); + +test('a verdict outside the vocabulary notifies as unread', async () => { + const core = fakeCore(); + await issueValidationDone({ + github: fakeGithub(), + context: fakeContext({ commentBody: body('probably fine') }), + core, + }); + + assert.match(core.outputs.support_dev_message, /Validation: no verdict read/); +}); + +test('a duplicate verdict with no issue number notifies as unread', async () => { + const core = fakeCore(); + await issueValidationDone({ + github: fakeGithub(), + context: fakeContext({ commentBody: body('duplicate of') }), + core, + }); + + assert.match(core.outputs.support_dev_message, /Validation: no verdict read/); +}); + +test('an issue title holding angle brackets cannot break the link', async () => { + const core = fakeCore(); + await issueValidationDone({ + github: fakeGithub(), + context: fakeContext({ title: 'Fix rendering' }), + core, + }); + + assert.match(core.outputs.support_dev_message, /Fix <ChannelList> rendering/); + assert.doesNotMatch(core.outputs.support_dev_message, //); +}); + +test('a failed label removal still leaves the notification set', async () => { + const core = fakeCore(); + await issueValidationDone({ github: fakeGithub({ status: 500 }), context: fakeContext(), core }); + + assert.match(core.outputs.support_dev_message, /Validation: claim holds/); +}); + +for (const guard of [ + { name: 'a pull request comment', context: { pullRequest: true } }, + { name: 'another account', context: { author: 'contributor' } }, + { name: 'a comment with no marker', context: { commentBody: 'No marker here.' } }, +]) { + test(`${guard.name} sets no notification`, async () => { + const core = fakeCore(); + await issueValidationDone({ + github: fakeGithub(), + context: fakeContext(guard.context), + core, + }); + + assert.equal(core.outputs.support_dev_message, undefined); + }); +} diff --git a/scripts/issue-validation.js b/scripts/issue-validation.js new file mode 100644 index 0000000..4b441f9 --- /dev/null +++ b/scripts/issue-validation.js @@ -0,0 +1,31 @@ +// See docs/community-automations.md + +const { + ISSUE_VALIDATION_REPOS, + LABEL_COMMUNITY_ISSUE_REVIEW, + BOT_MESSAGE_ISSUE_VALIDATION, +} = require('./constants'); +const { sendBotMessage, hasLabel, addLabel } = require('./utils'); + +module.exports = async ({ github, context, core }) => { + try { + const owner = context.repo.owner; + 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.`); + return; + } + + if (await hasLabel(LABEL_COMMUNITY_ISSUE_REVIEW, owner, repo, issueNumber, github, core)) { + core.info(`Issue #${issueNumber} already carries ${LABEL_COMMUNITY_ISSUE_REVIEW}.`); + return; + } + + await sendBotMessage(issueNumber, BOT_MESSAGE_ISSUE_VALIDATION, { github, context }); + await addLabel(LABEL_COMMUNITY_ISSUE_REVIEW, owner, repo, issueNumber, github, core); + } catch (error) { + core.setFailed(`Action failed with error: ${error.message}`); + } +}; diff --git a/scripts/issue-validation.test.js b/scripts/issue-validation.test.js new file mode 100644 index 0000000..99c12df --- /dev/null +++ b/scripts/issue-validation.test.js @@ -0,0 +1,119 @@ +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 SANDBOX = 'test-actions'; + +function fakeGithub({ labels = [], addFails = false } = {}) { + const comments = []; + const added = []; + return { + comments, + added, + paginate: async (fn, params) => fn(params), + rest: { + issues: { + listLabelsOnIssue: async () => labels.map(name => ({ name })), + addLabels: async ({ labels: names }) => { + if (addFails) { + throw new Error('Resource not accessible by integration'); + } + added.push(...names); + }, + createComment: async ({ body }) => { + comments.push(body); + return { data: { html_url: 'https://github.com/learningequality/kolibri/issues/7' } }; + }, + }, + }, + }; +} + +function fakeContext({ repo = COMMUNITY_REPOS[0] } = {}) { + return { + repo: { owner: 'learningequality', repo }, + payload: { issue: { number: 7 } }, + }; +} + +function fakeCore() { + const failures = []; + const warnings = []; + return { + failures, + warnings, + info() {}, + warning(message) { + warnings.push(message); + }, + error() {}, + setFailed(message) { + failures.push(message); + }, + }; +} + +test('a new issue in a community repo gets one comment and the label', async () => { + const github = fakeGithub(); + const core = fakeCore(); + await issueValidation({ github, context: fakeContext(), core }); + + assert.equal(github.comments.length, 1); + assert.deepEqual(github.added, [LABEL_COMMUNITY_ISSUE_REVIEW]); + assert.deepEqual(core.failures, []); +}); + +test('the comment mentions rtibblesbot outside backticks, which is the trigger', async () => { + const github = fakeGithub(); + await issueValidation({ github, context: fakeContext(), core: fakeCore() }); + + // 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/); +}); + +test('a repo outside the community list gets neither', async () => { + const github = fakeGithub(); + const core = fakeCore(); + await issueValidation({ github, context: fakeContext({ repo: 'internal-tooling' }), core }); + + assert.deepEqual(github.comments, []); + 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.comments.length, 1); + assert.deepEqual(github.added, [LABEL_COMMUNITY_ISSUE_REVIEW]); +}); + +test('an issue already under review gets no second comment', async () => { + const github = fakeGithub({ labels: [LABEL_COMMUNITY_ISSUE_REVIEW] }); + await issueValidation({ github, context: fakeContext(), core: fakeCore() }); + + assert.deepEqual(github.comments, []); + assert.deepEqual(github.added, []); +}); + +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, []); +}); + +test('a failed label add warns and leaves the run passing', async () => { + const github = fakeGithub({ addFails: true }); + const core = fakeCore(); + await issueValidation({ github, context: fakeContext(), core }); + + assert.equal(github.comments.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 741d5f6..f5f804e 100644 --- a/scripts/unassign-inactive-issues.js +++ b/scripts/unassign-inactive-issues.js @@ -1,3 +1,5 @@ +const { sweepIssueReviewLabels } = require('./issue-review-sweep'); + const formatUnassignments = unassignments => { if (unassignments.length === 0) return ''; @@ -256,6 +258,8 @@ module.exports = async ({ github, context, core }) => { ); } + await sweepIssueReviewLabels(owner, repo, { github, core }); + const issues = await getAllIssues(github, owner, repo); //console.log(`Processing ${issues.length} open issues`); diff --git a/scripts/utils.js b/scripts/utils.js index 33b9011..df835d3 100644 --- a/scripts/utils.js +++ b/scripts/utils.js @@ -135,8 +135,16 @@ async function sendBotMessage(issueNumber, message, { github, context }) { } } +/** + * Every caller's message is interpolated into a JSON string literal in the + * leaf's Slack payload, so a backslash has to be escaped before the quote. + */ function escapeIssueTitleForSlackMessage(issueTitle) { - return issueTitle.replace(/"/g, '\\"').replace(//g, '>'); + return issueTitle + .replace(/\\/g, '\\\\') + .replace(/"/g, '\\"') + .replace(//g, '>'); } /** @@ -195,6 +203,48 @@ async function hasLabel(name, owner, repo, issueNumber, github, core) { return labels.includes(name.toLowerCase()); } +/** + * Adds a label to an issue, and returns whether the call succeeded. The call + * creates the label in a repository that does not define it. + */ +async function addLabel(name, owner, repo, issueNumber, github, core) { + try { + await github.rest.issues.addLabels({ + owner, + repo, + issue_number: issueNumber, + labels: [name], + }); + return true; + } catch (error) { + core.warning(`Failed to add ${name} to issue #${issueNumber}: ${error.message}`); + return false; + } +} + +/** + * Removes a label from an issue, and returns whether the call succeeded. GitHub + * answers 404 when the label is not on the issue, which counts as removed. + */ +async function removeLabel(name, owner, repo, issueNumber, github, core) { + try { + await github.rest.issues.removeLabel({ + owner, + repo, + issue_number: issueNumber, + name, + }); + return true; + } catch (error) { + if (error.status === 404) { + core.info(`${name} is not on issue #${issueNumber}.`); + return true; + } + core.warning(`Failed to remove ${name} from issue #${issueNumber}: ${error.message}`); + return false; + } +} + /** * Fetches the issues a pull request is linked to, or null when the lookup fails. * GitHub records a link from a closing keyword in the description, or from the @@ -386,6 +436,8 @@ module.exports = { hasRecentBotComment, getLabels, hasLabel, + addLabel, + removeLabel, getLinkedIssues, getIssues, getPullRequests, diff --git a/scripts/utils.test.js b/scripts/utils.test.js new file mode 100644 index 0000000..6355ad4 --- /dev/null +++ b/scripts/utils.test.js @@ -0,0 +1,45 @@ +const test = require('node:test'); +const assert = require('node:assert/strict'); + +const { escapeIssueTitleForSlackMessage } = require('./utils'); + +function payload(title) { + return `{\n "text": "on issue: "\n}`; +} + +function rendered(title) { + return JSON.parse(payload(title)).text; +} + +test('a plain title passes through', () => { + assert.equal(rendered('Migrate the device tests'), 'on issue: '); +}); + +test('a backslash survives as one backslash', () => { + assert.equal(rendered('Fix C:\\path handling'), 'on issue: '); +}); + +test('a quote survives as one quote', () => { + assert.equal(rendered('Quote " in title'), 'on issue: '); +}); + +test('a backslash before a quote survives both', () => { + assert.equal(rendered('Odd \\" title'), 'on issue: '); +}); + +test('angle brackets cannot end the Slack link early', () => { + const text = rendered('Fix rendering'); + assert.equal(text, 'on issue: '); + assert.equal(text.match(/>/g).length, 1); +}); + +for (const title of [ + 'Fix C:\\path handling', + 'Quote " in title', + 'Both \\ and " and ', + 'Trailing backslash \\', +]) { + test(`the payload stays valid JSON for ${JSON.stringify(title)}`, () => { + assert.doesNotThrow(() => JSON.parse(payload(title))); + }); +}