From 91b0e620b0468dee748a91b3a2f5ac5d8b76a9f4 Mon Sep 17 00:00:00 2001 From: Samson Akol Date: Mon, 26 Oct 2026 11:37:16 +0300 Subject: [PATCH 1/8] Add the issue validation automations A contributor opens an issue and nothing checks the claim in it. rtibblesbot can now validate an issue it did not open, so ask it on every contributor issue and mark the issue while the check runs. The comment mentions the bot outside backticks. The agent polls its notifications and holds no inbound endpoint, so the mention is the only way to reach it. The label comes off on a comment that carries the marker and comes from rtibblesbot. The marker alone is not enough, because it is plain text that anyone can paste. --- .../automation-group-issue-comment.yml | 42 ++++++ .github/workflows/automation-group-issues.yml | 13 +- .github/workflows/automation.yml | 16 +- .github/workflows/issue-validation-done.yml | 41 +++++ .github/workflows/issue-validation.yml | 54 +++++++ automation-registry.yml | 20 +++ scripts/constants.js | 21 +++ scripts/issue-validation-done.js | 45 ++++++ scripts/issue-validation-done.test.js | 141 ++++++++++++++++++ scripts/issue-validation.js | 31 ++++ scripts/issue-validation.test.js | 109 ++++++++++++++ scripts/utils.js | 44 ++++++ 12 files changed, 567 insertions(+), 10 deletions(-) create mode 100644 .github/workflows/automation-group-issue-comment.yml create mode 100644 .github/workflows/issue-validation-done.yml create mode 100644 .github/workflows/issue-validation.yml create mode 100644 scripts/issue-validation-done.js create mode 100644 scripts/issue-validation-done.test.js create mode 100644 scripts/issue-validation.js create mode 100644 scripts/issue-validation.test.js 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..ea1f4ca --- /dev/null +++ b/.github/workflows/issue-validation-done.yml @@ -0,0 +1,41 @@ +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 + 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/scripts/constants.js b/scripts/constants.js index d92e6e3..b1d2855 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', @@ -205,6 +222,10 @@ module.exports = { RTIBBLESBOT_USERNAME, LABEL_COMMUNITY_REVIEW, BOT_MESSAGE_COMMUNITY_REVIEW, + LABEL_COMMUNITY_ISSUE_REVIEW, + ISSUE_VALIDATION_MARKER, + ISSUE_REVIEW_MAX_DAYS, + BOT_MESSAGE_ISSUE_VALIDATION, TEAMS_WITH_CLOSE_CONTRIBUTORS, HOLIDAY_MESSAGE, PR_STATS_REPOS, 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..a51dc9d --- /dev/null +++ b/scripts/issue-validation-done.test.js @@ -0,0 +1,141 @@ +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, + LE_BOT_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('the bot account is not rtibblesbot either', async () => { + const github = fakeGithub(); + await issueValidationDone({ + github, + context: fakeContext({ author: LE_BOT_USERNAME }), + 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..12794ce --- /dev/null +++ b/scripts/issue-validation.js @@ -0,0 +1,31 @@ +// See docs/community-automations.md + +const { + COMMUNITY_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 (!COMMUNITY_REPOS.includes(repo)) { + core.info(`${repo} is not a community repo, so no validation 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..0d900fd --- /dev/null +++ b/scripts/issue-validation.test.js @@ -0,0 +1,109 @@ +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'); + +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('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/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, From 30c7939fbbda2f93ea492ecade1529de0385142a Mon Sep 17 00:00:00 2001 From: Samson Akol Date: Mon, 26 Oct 2026 11:37:16 +0300 Subject: [PATCH 2/8] Sweep a stuck issue review label If the agent never answers, the label stays on and tells a maintainer nothing. The weekly run now clears a label older than ISSUE_REVIEW_MAX_DAYS. The sweep lives in its own module because unassign-inactive-issues skips every issue with no assignees, and an issue under review normally has none. --- scripts/issue-review-sweep.js | 90 +++++++++++++++ scripts/issue-review-sweep.test.js | 171 ++++++++++++++++++++++++++++ scripts/unassign-inactive-issues.js | 4 + 3 files changed, 265 insertions(+) create mode 100644 scripts/issue-review-sweep.js create mode 100644 scripts/issue-review-sweep.test.js diff --git a/scripts/issue-review-sweep.js b/scripts/issue-review-sweep.js new file mode 100644 index 0000000..51b5429 --- /dev/null +++ b/scripts/issue-review-sweep.js @@ -0,0 +1,90 @@ +// 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..bde42fa --- /dev/null +++ b/scripts/issue-review-sweep.test.js @@ -0,0 +1,171 @@ +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, +} = {}) { + const removed = []; + return { + removed, + paginate: async (fn, params) => fn(params), + rest: { + issues: { + listForRepo: async () => { + if (listFails) { + throw new Error('server error'); + } + return issues.map(number => ({ number })); + }, + listEventsForTimeline: async ({ issue_number }) => { + if (timelineFails) { + throw new Error('server error'); + } + return timelines[issue_number] || []; + }, + removeLabel: async ({ name, issue_number }) => { + 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('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/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`); From 6add3694b40272c603cfed4c8f1b8141ce9ec050 Mon Sep 17 00:00:00 2001 From: Samson Akol Date: Mon, 26 Oct 2026 11:37:16 +0300 Subject: [PATCH 3/8] Document the issue validation automations --- docs/community-automations.md | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/docs/community-automations.md b/docs/community-automations.md index 561c198..c4f9bbf 100644 --- a/docs/community-automations.md +++ b/docs/community-automations.md @@ -102,6 +102,27 @@ 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` + +A pair of automations that asks `rtibblesbot` to check the claim in a contributor's issue, and that marks the issue while the check runs. + +`issue-validation` runs on `issues: [opened]`. It acts when the repo is in `COMMUNITY_REPOS`, the opener is a contributor, and the issue does not already carry `community-issue-review`. It posts one comment and adds that label. The add call creates the label in a repo that does not define it. + +The comment mentions `@rtibblesbot` outside backticks. The mention is load-bearing: the agent polls its GitHub notifications and holds no inbound endpoint, so a mention is the only way to reach it. A label creates no notification. A backticked handle, as in `BOT_MESSAGE_RTIBBLESBOT_REVIEW`, sends none either. + +`issue-validation-done` runs on `issue_comment: [created]`. It removes the label when the comment carries the marker `` and comes from `rtibblesbot`. Both conditions are needed. The marker is plain text that anyone can paste, and `rtibblesbot` comments on issues for other reasons. This cannot live in `contributor-issue-comment`, because that automation drops a bot comment by design. + +The weekly `unassign-inactive-issues` run sweeps the stuck case through `scripts/issue-review-sweep.js`. If the agent never answers, because the service is stopped or every token is capped, the label stays on and tells a maintainer nothing. The sweep removes it once the `labeled` event is older than `ISSUE_REVIEW_MAX_DAYS` days. + +Neither automation closes the issue, and neither acts on what the check finds. The agent's comment names the close it recommends, and a maintainer asks for it in a reply. + +In `scripts/constants.js` set: + +- `LABEL_COMMUNITY_ISSUE_REVIEW`: the label applied while the check runs +- `ISSUE_VALIDATION_MARKER`: the marker that ends the check, shared with learningequality/coding-agent +- `ISSUE_REVIEW_MAX_DAYS`: how long the label may stay on before the sweep clears it +- `BOT_MESSAGE_ISSUE_VALIDATION`: the comment text + # `holiday-message` Sends a holiday message to community pull requests and issue comments. From f46ced75d76236438aea708a0b899f995775e0c8 Mon Sep 17 00:00:00 2001 From: Samson Akol Date: Mon, 26 Oct 2026 11:48:44 +0300 Subject: [PATCH 4/8] Format the issue review sweep files The pre-commit hooks did not run when 30c7939 landed, so prettier wanted these two files reformatted. --- scripts/issue-review-sweep.js | 3 +- scripts/issue-review-sweep.test.js | 92 +++++++++++++++++++++--------- 2 files changed, 67 insertions(+), 28 deletions(-) diff --git a/scripts/issue-review-sweep.js b/scripts/issue-review-sweep.js index 51b5429..2abf681 100644 --- a/scripts/issue-review-sweep.js +++ b/scripts/issue-review-sweep.js @@ -19,8 +19,7 @@ async function labeledAt(name, owner, repo, issueNumber, github, core) { per_page: 100, }); const applied = events.filter( - event => - event.event === 'labeled' && event.label?.name?.toLowerCase() === name.toLowerCase(), + event => event.event === 'labeled' && event.label?.name?.toLowerCase() === name.toLowerCase(), ); if (!applied.length) { return null; diff --git a/scripts/issue-review-sweep.test.js b/scripts/issue-review-sweep.test.js index bde42fa..a2500a2 100644 --- a/scripts/issue-review-sweep.test.js +++ b/scripts/issue-review-sweep.test.js @@ -69,10 +69,15 @@ test('a label older than the limit comes off', async () => { issues: [7], timelines: { 7: [labeled(daysAgo(ISSUE_REVIEW_MAX_DAYS + 1))] }, }); - const cleared = await sweepIssueReviewLabels('learningequality', 'kolibri', { - github, - core: fakeCore(), - }, NOW); + 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 }]); @@ -83,10 +88,15 @@ test('a label younger than the limit stays on', async () => { issues: [7], timelines: { 7: [labeled(daysAgo(ISSUE_REVIEW_MAX_DAYS - 1))] }, }); - const cleared = await sweepIssueReviewLabels('learningequality', 'kolibri', { - github, - core: fakeCore(), - }, NOW); + const cleared = await sweepIssueReviewLabels( + 'learningequality', + 'kolibri', + { + github, + core: fakeCore(), + }, + NOW, + ); assert.deepEqual(cleared, []); assert.deepEqual(github.removed, []); @@ -97,10 +107,15 @@ test('a label at exactly the limit stays on', async () => { issues: [7], timelines: { 7: [labeled(daysAgo(ISSUE_REVIEW_MAX_DAYS))] }, }); - const cleared = await sweepIssueReviewLabels('learningequality', 'kolibri', { - github, - core: fakeCore(), - }, NOW); + const cleared = await sweepIssueReviewLabels( + 'learningequality', + 'kolibri', + { + github, + core: fakeCore(), + }, + NOW, + ); assert.deepEqual(cleared, []); }); @@ -113,10 +128,15 @@ test('the most recent application decides', async () => { 7: [labeled(daysAgo(30)), unlabeled(daysAgo(29)), labeled(daysAgo(0))], }, }); - const cleared = await sweepIssueReviewLabels('learningequality', 'kolibri', { - github, - core: fakeCore(), - }, NOW); + const cleared = await sweepIssueReviewLabels( + 'learningequality', + 'kolibri', + { + github, + core: fakeCore(), + }, + NOW, + ); assert.deepEqual(cleared, []); }); @@ -126,10 +146,15 @@ test('a labeled event for another label is not read as this one', async () => { issues: [7], timelines: { 7: [labeled(daysAgo(30), 'help wanted')] }, }); - const cleared = await sweepIssueReviewLabels('learningequality', 'kolibri', { - github, - core: fakeCore(), - }, NOW); + const cleared = await sweepIssueReviewLabels( + 'learningequality', + 'kolibri', + { + github, + core: fakeCore(), + }, + NOW, + ); assert.deepEqual(cleared, []); }); @@ -142,10 +167,15 @@ test('each issue is judged on its own age', async () => { 8: [labeled(daysAgo(1))], }, }); - const cleared = await sweepIssueReviewLabels('learningequality', 'kolibri', { - github, - core: fakeCore(), - }, NOW); + const cleared = await sweepIssueReviewLabels( + 'learningequality', + 'kolibri', + { + github, + core: fakeCore(), + }, + NOW, + ); assert.deepEqual(cleared, [7]); }); @@ -153,7 +183,12 @@ test('each issue is judged on its own age', async () => { 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); + const cleared = await sweepIssueReviewLabels( + 'learningequality', + 'kolibri', + { github, core }, + NOW, + ); assert.deepEqual(cleared, []); assert.equal(core.warnings.length, 1); @@ -163,7 +198,12 @@ test('a timeline this run cannot read leaves the label on', async () => { 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); + const cleared = await sweepIssueReviewLabels( + 'learningequality', + 'kolibri', + { github, core }, + NOW, + ); assert.deepEqual(cleared, []); assert.equal(core.warnings.length, 1); From a0bebeb6b5a363d9c775d7d186a00d1d9a87023f Mon Sep 17 00:00:00 2001 From: Samson Akol Date: Mon, 26 Oct 2026 11:48:44 +0300 Subject: [PATCH 5/8] Cover the contributor gate with tests The contributor, core member and bot cases decide which issues reach the new validation automation, and nothing tested them. The gate is shared by six leaves, so these tests cover all of them. --- scripts/is-contributor.test.js | 111 +++++++++++++++++++++++++++++++++ 1 file changed, 111 insertions(+) create mode 100644 scripts/is-contributor.test.js 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/); +}); From 30f6647c846d0a283fd7ccefa1f019519b7accd3 Mon Sep 17 00:00:00 2001 From: Samson Akol Date: Mon, 26 Oct 2026 12:00:05 +0300 Subject: [PATCH 6/8] Reword the issue validation automation docs --- docs/community-automations.md | 41 +++++++++++++++++++++++++---------- 1 file changed, 30 insertions(+), 11 deletions(-) diff --git a/docs/community-automations.md b/docs/community-automations.md index c4f9bbf..bc1e7c9 100644 --- a/docs/community-automations.md +++ b/docs/community-automations.md @@ -104,24 +104,43 @@ The failed check is the only report. Nothing is posted on the pull request. # `issue-validation` and `issue-validation-done` -A pair of automations that asks `rtibblesbot` to check the claim in a contributor's issue, and that marks the issue while the check runs. +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 acts when the repo is in `COMMUNITY_REPOS`, the opener is a contributor, and the issue does not already carry `community-issue-review`. It posts one comment and adds that label. The add call creates the label in a repo that does not define it. +## `issue-validation` -The comment mentions `@rtibblesbot` outside backticks. The mention is load-bearing: the agent polls its GitHub notifications and holds no inbound endpoint, so a mention is the only way to reach it. A label creates no notification. A backticked handle, as in `BOT_MESSAGE_RTIBBLESBOT_REVIEW`, sends none either. +Runs on `issues: [opened]`. It proceeds only if the repository is in `COMMUNITY_REPOS`, the issue opener is a contributor, and the issue does not already have the `community-issue-review` label. -`issue-validation-done` runs on `issue_comment: [created]`. It removes the label when the comment carries the marker `` and comes from `rtibblesbot`. Both conditions are needed. The marker is plain text that anyone can paste, and `rtibblesbot` comments on issues for other reasons. This cannot live in `contributor-issue-comment`, because that automation drops a bot comment by design. +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 weekly `unassign-inactive-issues` run sweeps the stuck case through `scripts/issue-review-sweep.js`. If the agent never answers, because the service is stopped or every token is capped, the label stays on and tells a maintainer nothing. The sweep removes it once the `labeled` event is older than `ISSUE_REVIEW_MAX_DAYS` days. +**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. -Neither automation closes the issue, and neither acts on what the check finds. The agent's comment names the close it recommends, and a maintainer asks for it in a reply. +## `issue-validation-done` -In `scripts/constants.js` set: +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. + +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`: -- `LABEL_COMMUNITY_ISSUE_REVIEW`: the label applied while the check runs -- `ISSUE_VALIDATION_MARKER`: the marker that ends the check, shared with learningequality/coding-agent -- `ISSUE_REVIEW_MAX_DAYS`: how long the label may stay on before the sweep clears it -- `BOT_MESSAGE_ISSUE_VALIDATION`: the comment text +- `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` From a2e323066989a8d9ca6f97705f93eb6fa4d1274e Mon Sep 17 00:00:00 2001 From: Samson Akol Date: Mon, 26 Oct 2026 13:29:59 +0300 Subject: [PATCH 7/8] Fix three findings from the local review morango takes contributions from time to time, so validation must reach it. COMMUNITY_REPOS is an alias of PR_STATS_REPOS, so morango also joins the weekly statistics report and the cross-repo assignment count. The gate now reads ISSUE_VALIDATION_REPOS, which adds test-actions. The old gate did not name it, so the automation could not be tested where every other one is tested. The label job now tests the author and the marker in its condition, as resolve-bot-pr-threads does. Every comment in every consumer used to generate a token, check out this repo and install dependencies before the script returned. --- .github/workflows/issue-validation-done.yml | 3 +++ docs/community-automations.md | 5 ++++- scripts/constants.js | 6 ++++++ scripts/issue-validation.js | 6 +++--- scripts/issue-validation.test.js | 10 ++++++++++ 5 files changed, 26 insertions(+), 4 deletions(-) diff --git a/.github/workflows/issue-validation-done.yml b/.github/workflows/issue-validation-done.yml index ea1f4ca..d904fe3 100644 --- a/.github/workflows/issue-validation-done.yml +++ b/.github/workflows/issue-validation-done.yml @@ -11,6 +11,9 @@ on: 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 diff --git a/docs/community-automations.md b/docs/community-automations.md index bc1e7c9..d611342 100644 --- a/docs/community-automations.md +++ b/docs/community-automations.md @@ -108,7 +108,7 @@ These two automations ask `rtibblesbot` to validate claims in contributor-submit ## `issue-validation` -Runs on `issues: [opened]`. It proceeds only if the repository is in `COMMUNITY_REPOS`, the issue opener is a contributor, and the issue does not already have the `community-issue-review` label. +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. @@ -123,6 +123,8 @@ Runs on `issue_comment: [created]`. It removes the `community-issue-review` labe 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 @@ -137,6 +139,7 @@ Neither automation closes issues or acts on the validation findings. The agent's Configure the following constants in `scripts/constants.js`: +- `ISSUE_VALIDATION_REPOS`: The repositories where validation runs, which are the community repositories plus the sandbox the automations are tested in. - `LABEL_COMMUNITY_ISSUE_REVIEW`: The label applied while validation is in progress. - `ISSUE_VALIDATION_MARKER`: The marker that signals validation completion, shared with `learningequality/coding-agent`. - `ISSUE_REVIEW_MAX_DAYS`: The maximum age of the label before the sweep clears it. diff --git a/scripts/constants.js b/scripts/constants.js index b1d2855..88a6400 100644 --- a/scripts/constants.js +++ b/scripts/constants.js @@ -179,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 = [ @@ -223,6 +228,7 @@ module.exports = { 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, diff --git a/scripts/issue-validation.js b/scripts/issue-validation.js index 12794ce..4b441f9 100644 --- a/scripts/issue-validation.js +++ b/scripts/issue-validation.js @@ -1,7 +1,7 @@ // See docs/community-automations.md const { - COMMUNITY_REPOS, + ISSUE_VALIDATION_REPOS, LABEL_COMMUNITY_ISSUE_REVIEW, BOT_MESSAGE_ISSUE_VALIDATION, } = require('./constants'); @@ -13,8 +13,8 @@ module.exports = async ({ github, context, core }) => { const repo = context.repo.repo; const issueNumber = context.payload.issue.number; - if (!COMMUNITY_REPOS.includes(repo)) { - core.info(`${repo} is not a community repo, so no validation is requested.`); + if (!ISSUE_VALIDATION_REPOS.includes(repo)) { + core.info(`Issue validation does not run in ${repo}, so none is requested.`); return; } diff --git a/scripts/issue-validation.test.js b/scripts/issue-validation.test.js index 0d900fd..99c12df 100644 --- a/scripts/issue-validation.test.js +++ b/scripts/issue-validation.test.js @@ -4,6 +4,8 @@ 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 = []; @@ -82,6 +84,14 @@ test('a repo outside the community list gets neither', async () => { 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() }); From 625368c267fe63411a548ffa6f01f98fa3dfcd1a Mon Sep 17 00:00:00 2001 From: Samson Akol Date: Mon, 26 Oct 2026 14:19:01 +0300 Subject: [PATCH 8/8] Close two test gaps in the sweep and drop a duplicate The sweep's fake listing ignored the label filter, so removing that filter failed no test. Without it the sweep reads an old labeled event on an issue whose label is already gone, gets a 404 from the removal, and reports clearing a label the issue never had. The fake removal always succeeded, so a failed removal was untested. The bot-account case took the same branch as the contributor case, so it guarded nothing. --- scripts/issue-review-sweep.test.js | 53 ++++++++++++++++++++++++++- scripts/issue-validation-done.test.js | 12 ------ 2 files changed, 51 insertions(+), 14 deletions(-) diff --git a/scripts/issue-review-sweep.test.js b/scripts/issue-review-sweep.test.js index a2500a2..79dc5df 100644 --- a/scripts/issue-review-sweep.test.js +++ b/scripts/issue-review-sweep.test.js @@ -26,18 +26,25 @@ function fakeGithub({ 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 () => { + listForRepo: async ({ labels }) => { if (listFails) { throw new Error('server error'); } - return issues.map(number => ({ number })); + const wanted = (labels || '').split(',').filter(Boolean); + return all.filter(issue => wanted.every(name => issue.labels.includes(name))); }, listEventsForTimeline: async ({ issue_number }) => { if (timelineFails) { @@ -46,6 +53,11 @@ function fakeGithub({ 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 }); }, }, @@ -180,6 +192,43 @@ test('each issue is judged on its own age', async () => { 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(); diff --git a/scripts/issue-validation-done.test.js b/scripts/issue-validation-done.test.js index a51dc9d..5be44b0 100644 --- a/scripts/issue-validation-done.test.js +++ b/scripts/issue-validation-done.test.js @@ -6,7 +6,6 @@ const { ISSUE_VALIDATION_MARKER, LABEL_COMMUNITY_ISSUE_REVIEW, RTIBBLESBOT_USERNAME, - LE_BOT_USERNAME, } = require('./constants'); const VERDICT = `${ISSUE_VALIDATION_MARKER}\n\n**Verdict:** claim holds`; @@ -79,17 +78,6 @@ test('the same marker from another account leaves the label on', async () => { assert.deepEqual(github.removed, []); }); -test('the bot account is not rtibblesbot either', async () => { - const github = fakeGithub(); - await issueValidationDone({ - github, - context: fakeContext({ author: LE_BOT_USERNAME }), - core: fakeCore(), - }); - - assert.deepEqual(github.removed, []); -}); - test('a comment from rtibblesbot without the marker leaves the label on', async () => { const github = fakeGithub(); await issueValidationDone({