diff --git a/.github/workflows/automation-group-issue-comment.yml b/.github/workflows/automation-group-issue-comment.yml new file mode 100644 index 0000000..8837811 --- /dev/null +++ b/.github/workflows/automation-group-issue-comment.yml @@ -0,0 +1,42 @@ +# 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: 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 }} + 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..b76d3e9 100644 --- a/.github/workflows/automation.yml +++ b/.github/workflows/automation.yml @@ -18,10 +18,10 @@ 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" @@ -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..d904fe3 --- /dev/null +++ b/.github/workflows/issue-validation-done.yml @@ -0,0 +1,44 @@ +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 +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 }); 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..fe1347a 100644 --- a/automation-registry.yml +++ b/automation-registry.yml @@ -128,6 +128,26 @@ 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 - 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..d611342 100644 --- a/docs/community-automations.md +++ b/docs/community-automations.md @@ -102,6 +102,49 @@ 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. + +## 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. + # `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..88a6400 100644 --- a/scripts/constants.js +++ b/scripts/constants.js @@ -87,6 +87,11 @@ 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; // Will be attached to bot messages when not empty // const GSOC_NOTE = ''; @@ -154,6 +159,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 +179,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 +227,11 @@ module.exports = { RTIBBLESBOT_USERNAME, LABEL_COMMUNITY_REVIEW, BOT_MESSAGE_COMMUNITY_REVIEW, + LABEL_COMMUNITY_ISSUE_REVIEW, + ISSUE_VALIDATION_REPOS, + ISSUE_VALIDATION_MARKER, + ISSUE_REVIEW_MAX_DAYS, + BOT_MESSAGE_ISSUE_VALIDATION, TEAMS_WITH_CLOSE_CONTRIBUTORS, HOLIDAY_MESSAGE, PR_STATS_REPOS, 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..142263f --- /dev/null +++ b/scripts/issue-validation-done.js @@ -0,0 +1,45 @@ +// See docs/community-automations.md + +const { + ISSUE_VALIDATION_MARKER, + LABEL_COMMUNITY_ISSUE_REVIEW, + RTIBBLESBOT_USERNAME, +} = require('./constants'); +const { removeLabel } = require('./utils'); + +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 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..5be44b0 --- /dev/null +++ b/scripts/issue-validation-done.test.js @@ -0,0 +1,129 @@ +const test = require('node:test'); +const assert = require('node:assert/strict'); + +const issueValidationDone = require('./issue-validation-done'); +const { + ISSUE_VALIDATION_MARKER, + LABEL_COMMUNITY_ISSUE_REVIEW, + RTIBBLESBOT_USERNAME, +} = require('./constants'); + +const VERDICT = `${ISSUE_VALIDATION_MARKER}\n\n**Verdict:** claim holds`; + +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, body = VERDICT, pullRequest = false } = {}) { + const issue = { number: 7 }; + 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, user: { login: author } } }, + }; +} + +function fakeCore() { + const failures = []; + const warnings = []; + return { + failures, + warnings, + info() {}, + warning(message) { + warnings.push(message); + }, + error() {}, + 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({ body: '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({ body: 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/); +}); 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..5c5f661 100644 --- a/scripts/utils.js +++ b/scripts/utils.js @@ -195,6 +195,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 +428,8 @@ module.exports = { hasRecentBotComment, getLabels, hasLabel, + addLabel, + removeLabel, getLinkedIssues, getIssues, getPullRequests,