From 91b0e620b0468dee748a91b3a2f5ac5d8b76a9f4 Mon Sep 17 00:00:00 2001 From: Samson Akol Date: Mon, 26 Oct 2026 11:37:16 +0300 Subject: [PATCH 01/11] 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 02/11] 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 03/11] 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 04/11] 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 05/11] 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 06/11] 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 07/11] 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 08/11] 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({ From e94ca18ba6a01dbc441fcb070d082acf67c71405 Mon Sep 17 00:00:00 2001 From: Samson Akol Date: Thu, 29 Oct 2026 10:25:09 +0300 Subject: [PATCH 09/11] Notify support-dev when a validation verdict lands The script now reads the verdict from the comment and posts one line to #support-dev with the verdict, the links, the reporter and the follow-up the verdict calls for. --- .../automation-group-issue-comment.yml | 3 +- .github/workflows/automation.yml | 2 +- .github/workflows/issue-validation-done.yml | 13 ++ automation-registry.yml | 1 + docs/community-automations.md | 9 ++ scripts/constants.js | 20 +++ scripts/issue-validation-done.js | 35 ++++- scripts/issue-validation-done.test.js | 132 +++++++++++++++++- 8 files changed, 207 insertions(+), 8 deletions(-) diff --git a/.github/workflows/automation-group-issue-comment.yml b/.github/workflows/automation-group-issue-comment.yml index 8837811..c536b6e 100644 --- a/.github/workflows/automation-group-issue-comment.yml +++ b/.github/workflows/automation-group-issue-comment.yml @@ -17,7 +17,7 @@ on: description: "Forwarded to: contributor-issue-comment" required: false SLACK_WEBHOOK_URL: - description: "Forwarded to: contributor-issue-comment" + description: "Forwarded to: issue-validation-done, contributor-issue-comment" required: false jobs: issue-validation-done: @@ -29,6 +29,7 @@ jobs: secrets: LE_BOT_APP_ID: ${{ secrets.LE_BOT_APP_ID }} LE_BOT_PRIVATE_KEY: ${{ secrets.LE_BOT_PRIVATE_KEY }} + SLACK_WEBHOOK_URL: ${{ secrets.SLACK_WEBHOOK_URL }} contributor-issue-comment: name: Contributor Issue Comment if: ${{ github.event_name == 'issue_comment' && github.event.action == 'created' }} diff --git a/.github/workflows/automation.yml b/.github/workflows/automation.yml index b76d3e9..ddc95ec 100644 --- a/.github/workflows/automation.yml +++ b/.github/workflows/automation.yml @@ -27,7 +27,7 @@ on: description: "Forwarded to: contributor-pr-reply, contributor-issue-comment, unassign-inactive-issues" required: false SLACK_WEBHOOK_URL: - description: "Forwarded to: contributor-issue-comment" + description: "Forwarded to: issue-validation-done, contributor-issue-comment" required: false jobs: pull-request-target: diff --git a/.github/workflows/issue-validation-done.yml b/.github/workflows/issue-validation-done.yml index d904fe3..4b97c05 100644 --- a/.github/workflows/issue-validation-done.yml +++ b/.github/workflows/issue-validation-done.yml @@ -8,6 +8,9 @@ on: LE_BOT_PRIVATE_KEY: description: 'GitHub App Private Key for authentication' required: true + SLACK_WEBHOOK_URL: + description: 'Webhook URL for Slack #support-dev channel' + required: true jobs: clear-review-label: name: Clear review label @@ -42,3 +45,13 @@ jobs: script: | const script = require('./scripts/issue-validation-done.js'); return await script({ github, context, core }); + - name: Send Slack notification to support-dev channel + if: ${{ steps.script.outputs.support_dev_message }} + uses: slackapi/slack-github-action@v4.0.0 + with: + webhook-type: incoming-webhook + webhook: ${{ secrets.SLACK_WEBHOOK_URL }} + payload: | + { + "text": "${{ steps.script.outputs.support_dev_message }}" + } diff --git a/automation-registry.yml b/automation-registry.yml index fe1347a..29e7e7d 100644 --- a/automation-registry.yml +++ b/automation-registry.yml @@ -148,6 +148,7 @@ automations: secrets: LE_BOT_APP_ID: required LE_BOT_PRIVATE_KEY: required + SLACK_WEBHOOK_URL: optional - name: contributor-issue-comment enabled: true leaf: contributor-issue-comment.yml diff --git a/docs/community-automations.md b/docs/community-automations.md index d611342..09ab5c9 100644 --- a/docs/community-automations.md +++ b/docs/community-automations.md @@ -127,6 +127,14 @@ The leaf workflow tests both conditions in its job `if:`, as `resolve-bot-pr-thr This logic cannot live in `contributor-issue-comment`, because that automation deliberately ignores bot comments. +It also posts one line to Slack `#support-dev` for every validation comment, so a maintainer knows to follow up. The line carries the verdict, a link to the comment, a link to the issue, the reporter's handle, and the follow-up the verdict calls for: + +``` +*[kolibri] on issue: reported by _octocat_* _(consider closing as duplicate)_ +``` + +`ISSUE_VALIDATION_FOLLOW_UPS` holds the six verdicts and their follow-ups. `learningequality/coding-agent#1229` fixes the vocabulary from the agent's side. A comment that states no verdict, or one outside the six, reads `Validation: no verdict read` and asks a maintainer to check the agent run, because under that contract such a comment means something broke. + ## Handling stalled reviews The weekly `unassign-inactive-issues` run also sweeps stalled validations through `scripts/issue-review-sweep.js`. @@ -144,6 +152,7 @@ Configure the following constants in `scripts/constants.js`: - `ISSUE_VALIDATION_MARKER`: The marker that signals validation completion, shared with `learningequality/coding-agent`. - `ISSUE_REVIEW_MAX_DAYS`: The maximum age of the label before the sweep clears it. - `BOT_MESSAGE_ISSUE_VALIDATION`: The comment text posted to request validation. +- `ISSUE_VALIDATION_FOLLOW_UPS`: The six verdicts and the follow-up each one asks a maintainer for, used in the Slack line. # `holiday-message` diff --git a/scripts/constants.js b/scripts/constants.js index 88a6400..eec6f1b 100644 --- a/scripts/constants.js +++ b/scripts/constants.js @@ -93,6 +93,22 @@ const LABEL_COMMUNITY_ISSUE_REVIEW = 'community-issue-review'; const ISSUE_VALIDATION_MARKER = ''; const ISSUE_REVIEW_MAX_DAYS = 3; +// The same contract fixes these six verdicts, one per validation comment. Each +// value is the follow-up the Slack notification asks a maintainer for. The +// `duplicate of #N` key carries an issue number, so the script matches it on +// its own. +const ISSUE_VALIDATION_DUPLICATE_VERDICT = 'duplicate of #N'; +const ISSUE_VALIDATION_FOLLOW_UPS = { + 'claim holds': 'ready for triage', + 'claim does not hold': 'consider closing', + [ISSUE_VALIDATION_DUPLICATE_VERDICT]: 'consider closing as duplicate', + 'already fixed': 'consider closing as fixed', + 'not enough information': 'needs more from the reporter', + 'out of scope': 'consider closing', +}; +const ISSUE_VALIDATION_NO_VERDICT = 'no verdict read'; +const ISSUE_VALIDATION_NO_VERDICT_FOLLOW_UP = 'check the agent run'; + // Will be attached to bot messages when not empty // const GSOC_NOTE = ''; const GSOC_NOTE = `\n\n**Are you preparing for Google Summer of Code? See our [GSoC guidelines.](https://learningequality.org/contributing-to-our-open-code-base/#google-summer-of-code)**`; @@ -230,6 +246,10 @@ module.exports = { LABEL_COMMUNITY_ISSUE_REVIEW, ISSUE_VALIDATION_REPOS, ISSUE_VALIDATION_MARKER, + ISSUE_VALIDATION_DUPLICATE_VERDICT, + ISSUE_VALIDATION_FOLLOW_UPS, + ISSUE_VALIDATION_NO_VERDICT, + ISSUE_VALIDATION_NO_VERDICT_FOLLOW_UP, ISSUE_REVIEW_MAX_DAYS, BOT_MESSAGE_ISSUE_VALIDATION, TEAMS_WITH_CLOSE_CONTRIBUTORS, diff --git a/scripts/issue-validation-done.js b/scripts/issue-validation-done.js index 142263f..5b021af 100644 --- a/scripts/issue-validation-done.js +++ b/scripts/issue-validation-done.js @@ -2,10 +2,32 @@ const { ISSUE_VALIDATION_MARKER, + ISSUE_VALIDATION_DUPLICATE_VERDICT, + ISSUE_VALIDATION_FOLLOW_UPS, + ISSUE_VALIDATION_NO_VERDICT, + ISSUE_VALIDATION_NO_VERDICT_FOLLOW_UP, LABEL_COMMUNITY_ISSUE_REVIEW, RTIBBLESBOT_USERNAME, } = require('./constants'); -const { removeLabel } = require('./utils'); +const { removeLabel, escapeIssueTitleForSlackMessage } = require('./utils'); + +const VERDICT_PREFIX = '**Verdict:**'; +const DUPLICATE_VERDICT = /^duplicate of #\d+$/; + +function readVerdict(body) { + const line = (body || '').split(/\r?\n/).find(candidate => candidate.startsWith(VERDICT_PREFIX)); + if (!line) { + return null; + } + + const verdict = line.slice(VERDICT_PREFIX.length).trim(); + if (DUPLICATE_VERDICT.test(verdict)) { + return { verdict, followUp: ISSUE_VALIDATION_FOLLOW_UPS[ISSUE_VALIDATION_DUPLICATE_VERDICT] }; + } + + const followUp = ISSUE_VALIDATION_FOLLOW_UPS[verdict]; + return followUp ? { verdict, followUp } : null; +} module.exports = async ({ github, context, core }) => { try { @@ -28,6 +50,17 @@ module.exports = async ({ github, context, core }) => { return; } + const read = readVerdict(comment.body); + const verdict = read ? read.verdict : ISSUE_VALIDATION_NO_VERDICT; + const followUp = read ? read.followUp : ISSUE_VALIDATION_NO_VERDICT_FOLLOW_UP; + const title = escapeIssueTitleForSlackMessage(issue.title || ''); + core.setOutput( + 'support_dev_message', + `*[${context.repo.repo}] <${comment.html_url}|Validation: ${verdict}> ` + + `on issue: <${issue.html_url}|${title}> reported by _${issue.user.login}_* ` + + `_(${followUp})_`, + ); + const removed = await removeLabel( LABEL_COMMUNITY_ISSUE_REVIEW, context.repo.owner, diff --git a/scripts/issue-validation-done.test.js b/scripts/issue-validation-done.test.js index 5be44b0..b195298 100644 --- a/scripts/issue-validation-done.test.js +++ b/scripts/issue-validation-done.test.js @@ -4,11 +4,19 @@ const assert = require('node:assert/strict'); const issueValidationDone = require('./issue-validation-done'); const { ISSUE_VALIDATION_MARKER, + ISSUE_VALIDATION_FOLLOW_UPS, + ISSUE_VALIDATION_DUPLICATE_VERDICT, LABEL_COMMUNITY_ISSUE_REVIEW, RTIBBLESBOT_USERNAME, } = require('./constants'); const VERDICT = `${ISSUE_VALIDATION_MARKER}\n\n**Verdict:** claim holds`; +const COMMENT_URL = 'https://github.com/learningequality/kolibri/issues/7#issuecomment-1'; +const ISSUE_URL = 'https://github.com/learningequality/kolibri/issues/7'; + +function body(verdict) { + return `${ISSUE_VALIDATION_MARKER}\n\n**Verdict:** ${verdict}\n**Evidence:** \`a.py:1\``; +} function fakeGithub({ status = 200 } = {}) { const removed = []; @@ -29,28 +37,42 @@ function fakeGithub({ status = 200 } = {}) { }; } -function fakeContext({ author = RTIBBLESBOT_USERNAME, body = VERDICT, pullRequest = false } = {}) { - const issue = { number: 7 }; +function fakeContext({ + author = RTIBBLESBOT_USERNAME, + commentBody = VERDICT, + pullRequest = false, + title = 'Server PID file kills the test run', + reporter = 'octocat', +} = {}) { + const issue = { number: 7, title, html_url: ISSUE_URL, user: { login: reporter } }; if (pullRequest) { issue.pull_request = { url: 'https://api.github.com/repos/learningequality/kolibri/pulls/7' }; } return { repo: { owner: 'learningequality', repo: 'kolibri' }, - payload: { issue, comment: { body, user: { login: author } } }, + payload: { + issue, + comment: { body: commentBody, html_url: COMMENT_URL, user: { login: author } }, + }, }; } function fakeCore() { const failures = []; const warnings = []; + const outputs = {}; return { failures, warnings, + outputs, info() {}, warning(message) { warnings.push(message); }, error() {}, + setOutput(name, value) { + outputs[name] = value; + }, setFailed(message) { failures.push(message); }, @@ -82,7 +104,7 @@ 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.' }), + context: fakeContext({ commentBody: 'Looks like a duplicate of #42.' }), core: fakeCore(), }); @@ -92,7 +114,7 @@ test('a comment from rtibblesbot without the marker leaves the label on', async test('an empty comment body does not throw', async () => { const github = fakeGithub(); const core = fakeCore(); - await issueValidationDone({ github, context: fakeContext({ body: null }), core }); + await issueValidationDone({ github, context: fakeContext({ commentBody: null }), core }); assert.deepEqual(github.removed, []); assert.deepEqual(core.failures, []); @@ -127,3 +149,103 @@ test('any other remove failure warns and leaves the run passing', async () => { assert.equal(core.warnings.length, 1); assert.match(core.warnings[0], /Failed to remove/); }); + +for (const [verdict, followUp] of Object.entries(ISSUE_VALIDATION_FOLLOW_UPS)) { + const stated = verdict === ISSUE_VALIDATION_DUPLICATE_VERDICT ? 'duplicate of #15001' : verdict; + + test(`the notification carries the ${verdict} verdict and its follow-up`, async () => { + const core = fakeCore(); + await issueValidationDone({ + github: fakeGithub(), + context: fakeContext({ commentBody: body(stated) }), + core, + }); + + assert.equal( + core.outputs.support_dev_message, + `*[kolibri] <${COMMENT_URL}|Validation: ${stated}> on issue: ` + + `<${ISSUE_URL}|Server PID file kills the test run> reported by _octocat_* _(${followUp})_`, + ); + }); +} + +test('a comment with no verdict line still notifies', async () => { + const core = fakeCore(); + await issueValidationDone({ + github: fakeGithub(), + context: fakeContext({ commentBody: `${ISSUE_VALIDATION_MARKER}\n\nNothing to report.` }), + core, + }); + + assert.match(core.outputs.support_dev_message, /Validation: no verdict read/); + assert.match(core.outputs.support_dev_message, /_\(check the agent run\)_$/); +}); + +test('a body with CRLF line endings reads the same verdict', async () => { + const core = fakeCore(); + await issueValidationDone({ + github: fakeGithub(), + context: fakeContext({ commentBody: body('claim holds').replace(/\n/g, '\r\n') }), + core, + }); + + assert.match(core.outputs.support_dev_message, /Validation: claim holds>/); +}); + +test('a verdict outside the vocabulary notifies as unread', async () => { + const core = fakeCore(); + await issueValidationDone({ + github: fakeGithub(), + context: fakeContext({ commentBody: body('probably fine') }), + core, + }); + + assert.match(core.outputs.support_dev_message, /Validation: no verdict read/); +}); + +test('a duplicate verdict with no issue number notifies as unread', async () => { + const core = fakeCore(); + await issueValidationDone({ + github: fakeGithub(), + context: fakeContext({ commentBody: body('duplicate of') }), + core, + }); + + assert.match(core.outputs.support_dev_message, /Validation: no verdict read/); +}); + +test('an issue title holding angle brackets cannot break the link', async () => { + const core = fakeCore(); + await issueValidationDone({ + github: fakeGithub(), + context: fakeContext({ title: 'Fix rendering' }), + core, + }); + + assert.match(core.outputs.support_dev_message, /Fix <ChannelList> rendering/); + assert.doesNotMatch(core.outputs.support_dev_message, //); +}); + +test('a failed label removal still leaves the notification set', async () => { + const core = fakeCore(); + await issueValidationDone({ github: fakeGithub({ status: 500 }), context: fakeContext(), core }); + + assert.match(core.outputs.support_dev_message, /Validation: claim holds/); +}); + +for (const guard of [ + { name: 'a pull request comment', context: { pullRequest: true } }, + { name: 'another account', context: { author: 'contributor' } }, + { name: 'a comment with no marker', context: { commentBody: 'No marker here.' } }, +]) { + test(`${guard.name} sets no notification`, async () => { + const core = fakeCore(); + await issueValidationDone({ + github: fakeGithub(), + context: fakeContext(guard.context), + core, + }); + + assert.equal(core.outputs.support_dev_message, undefined); + }); +} From 5f822c6de999b5b741da17c5d7b2f3be6a355857 Mon Sep 17 00:00:00 2001 From: Samson Akol Date: Thu, 29 Oct 2026 11:09:54 +0300 Subject: [PATCH 10/11] Read a verdict the way the agent writes one The agent's LLM session writes the verdict line, so its shape varies, and the agent's own reader tolerates that. This reader matched one fixed prefix, so eight shapes the agent accepts came through as no verdict read and sent a maintainer to check a healthy run. It now mirrors the agent's reader. A quote or a backslash in a title also made the Slack payload invalid JSON and failed the step, because every leaf interpolates its line into a JSON string literal. escapeIssueTitleForSlackMessage now escapes the backslash before the quote, and contributor-pr-reply calls it, having used the raw title until now. --- docs/community-automations.md | 6 +++- scripts/constants.js | 5 +-- scripts/contributor-pr-reply.js | 6 ++-- scripts/contributor-pr-reply.test.js | 22 +++++++++++-- scripts/issue-validation-done.js | 15 ++++++--- scripts/issue-validation-done.test.js | 42 ++++++++++++++++++++++++ scripts/utils.js | 11 ++++++- scripts/utils.test.js | 47 +++++++++++++++++++++++++++ 8 files changed, 140 insertions(+), 14 deletions(-) create mode 100644 scripts/utils.test.js diff --git a/docs/community-automations.md b/docs/community-automations.md index 09ab5c9..a18b5bc 100644 --- a/docs/community-automations.md +++ b/docs/community-automations.md @@ -133,7 +133,11 @@ It also posts one line to Slack `#support-dev` for every validation comment, so *[kolibri] on issue: reported by _octocat_* _(consider closing as duplicate)_ ``` -`ISSUE_VALIDATION_FOLLOW_UPS` holds the six verdicts and their follow-ups. `learningequality/coding-agent#1229` fixes the vocabulary from the agent's side. A comment that states no verdict, or one outside the six, reads `Validation: no verdict read` and asks a maintainer to check the agent run, because under that contract such a comment means something broke. +`ISSUE_VALIDATION_FOLLOW_UPS` holds the six verdicts and their follow-ups. `learningequality/coding-agent#1229` fixes the vocabulary from the agent's side. + +The agent's LLM session writes the verdict line, so its shape varies. `readVerdict` therefore mirrors the agent's own reader, which tolerates a missing bold, a colon on either side of it, a lowercase label, a list or quote prefix, backticks, and trailing punctuation. Keep the two readers in step. A stricter reader here reports a healthy run as broken. + +A comment outside all of that reads `Validation: no verdict read` and asks a maintainer to check the agent run. The agent posts no comment at all when it states no verdict, so this line means the rendering drifted, not that a verdict is missing. ## Handling stalled reviews diff --git a/scripts/constants.js b/scripts/constants.js index eec6f1b..35a610c 100644 --- a/scripts/constants.js +++ b/scripts/constants.js @@ -93,10 +93,7 @@ const LABEL_COMMUNITY_ISSUE_REVIEW = 'community-issue-review'; const ISSUE_VALIDATION_MARKER = ''; const ISSUE_REVIEW_MAX_DAYS = 3; -// The same contract fixes these six verdicts, one per validation comment. Each -// value is the follow-up the Slack notification asks a maintainer for. The -// `duplicate of #N` key carries an issue number, so the script matches it on -// its own. +// The same contract fixes these six verdicts, one per validation comment. const ISSUE_VALIDATION_DUPLICATE_VERDICT = 'duplicate of #N'; const ISSUE_VALIDATION_FOLLOW_UPS = { 'claim holds': 'ready for triage', diff --git a/scripts/contributor-pr-reply.js b/scripts/contributor-pr-reply.js index 613e707..4048973 100644 --- a/scripts/contributor-pr-reply.js +++ b/scripts/contributor-pr-reply.js @@ -5,7 +5,7 @@ const { LABEL_COMMUNITY_REVIEW, RTIBBLESBOT_USERNAME, } = require('./constants'); -const { sendBotMessage, getLinkedIssues } = require('./utils'); +const { sendBotMessage, getLinkedIssues, escapeIssueTitleForSlackMessage } = require('./utils'); module.exports = async ({ github, context, core }) => { try { @@ -13,7 +13,9 @@ module.exports = async ({ github, context, core }) => { const repo = context.repo.repo; const number = context.payload.pull_request.number; const url = context.payload.pull_request.html_url; - const title = context.payload.pull_request.title; + // Only the Slack lines below read this, and each one interpolates it into a + // JSON string literal in the leaf's payload. + const title = escapeIssueTitleForSlackMessage(context.payload.pull_request.title); const author = context.payload.pull_request.user.login; const lookup = await getLinkedIssues(number, { github, context, core }); diff --git a/scripts/contributor-pr-reply.test.js b/scripts/contributor-pr-reply.test.js index 94593ee..41ebd07 100644 --- a/scripts/contributor-pr-reply.test.js +++ b/scripts/contributor-pr-reply.test.js @@ -6,14 +6,14 @@ const { LABEL_COMMUNITY_REVIEW, RTIBBLESBOT_USERNAME } = require('./constants'); const AUTHOR = 'outsider'; -function fakeContext(author = AUTHOR) { +function fakeContext(author = AUTHOR, title = 'Fix it') { return { repo: { owner: 'learningequality', repo: 'kolibri' }, payload: { pull_request: { number: 42, html_url: 'https://github.com/learningequality/kolibri/pull/42', - title: 'Fix it', + title, user: { login: author }, }, }, @@ -231,6 +231,24 @@ test('a failed lookup falls back to the standard reply', async () => { assert.deepEqual(core.failures, []); }); +// The leaf interpolates the Slack line into a JSON string literal, so a quote or +// a backslash in the title breaks the payload and fails the step. +test('a title holding a quote and a backslash reaches Slack escaped', async () => { + const github = fakeGithub([{ number: 7, assignees: [AUTHOR] }]); + const core = await run(github, fakeContext(AUTHOR, 'Treat "done" as C:\\full')); + + const line = core.outputs.slack_notification; + assert.match(line, /Treat \\"done\\" as C:\\\\full/); + assert.doesNotThrow(() => JSON.parse(`{"text": "${line}"}`)); +}); + +test('a title holding angle brackets cannot end the Slack link early', async () => { + const github = fakeGithub([{ number: 7, assignees: [AUTHOR] }]); + const core = await run(github, fakeContext(AUTHOR, 'Remove ')); + + assert.match(core.outputs.slack_notification, /Remove <KButton>/); +}); + test('a failed lookup does not ask the author to link an issue', async () => { const github = fakeGithub([], { graphqlError: 'API down' }); await run(github); diff --git a/scripts/issue-validation-done.js b/scripts/issue-validation-done.js index 5b021af..ca14bdc 100644 --- a/scripts/issue-validation-done.js +++ b/scripts/issue-validation-done.js @@ -11,16 +11,23 @@ const { } = require('./constants'); const { removeLabel, escapeIssueTitleForSlackMessage } = require('./utils'); -const VERDICT_PREFIX = '**Verdict:**'; +// The verdict line is written by the agent's LLM session, not by a template, so +// its shape varies. These three mirror the agent's own reader, which has to +// tolerate that: _LIST_NUMBER_RE, _VERDICT_RE and the strips in parse_verdict +// and classify_verdict, all in learningequality/coding-agent. Anything stricter +// here reports a healthy run as broken. +const LIST_NUMBER = /^(\s*)\d+[.)][ \t]+/gm; +const VERDICT_LINE = /^[\s>*_-]*verdict[*_\s]*:\s*(.+?)\s*$/im; +const VERDICT_TRIM = /^[`*_\s.!;]+|[`*_\s.!;]+$/g; const DUPLICATE_VERDICT = /^duplicate of #\d+$/; function readVerdict(body) { - const line = (body || '').split(/\r?\n/).find(candidate => candidate.startsWith(VERDICT_PREFIX)); - if (!line) { + const match = VERDICT_LINE.exec((body || '').replace(LIST_NUMBER, '$1')); + if (!match) { return null; } - const verdict = line.slice(VERDICT_PREFIX.length).trim(); + const verdict = match[1].toLowerCase().replace(VERDICT_TRIM, ''); if (DUPLICATE_VERDICT.test(verdict)) { return { verdict, followUp: ISSUE_VALIDATION_FOLLOW_UPS[ISSUE_VALIDATION_DUPLICATE_VERDICT] }; } diff --git a/scripts/issue-validation-done.test.js b/scripts/issue-validation-done.test.js index b195298..59a7776 100644 --- a/scripts/issue-validation-done.test.js +++ b/scripts/issue-validation-done.test.js @@ -181,6 +181,48 @@ test('a comment with no verdict line still notifies', async () => { assert.match(core.outputs.support_dev_message, /_\(check the agent run\)_$/); }); +// The agent's LLM session writes the verdict line, so its shape varies. Each of +// these is accepted by the reader in learningequality/coding-agent, so it must +// be accepted here too. +for (const [line, shape] of [ + ['**Verdict:** claim holds', 'the shape seen live'], + ['**Verdict**: claim holds', 'the colon outside the bold'], + ['Verdict: claim holds', 'no bold'], + ['**verdict:** claim holds', 'a lowercase label'], + ['- **Verdict:** claim holds', 'a list item'], + ['1. **Verdict:** claim holds', 'a numbered list item'], + ['> **Verdict:** claim holds', 'a quote'], + ['**Verdict:** Claim holds.', 'a capital and a full stop'], + ['**Verdict:** `claim holds`', 'the value in backticks'], +]) { + test(`a verdict stated with ${shape} is read`, async () => { + const core = fakeCore(); + await issueValidationDone({ + github: fakeGithub(), + context: fakeContext({ + commentBody: `${ISSUE_VALIDATION_MARKER}\n\n${line}\n**Evidence:** \`a.py:1\``, + }), + core, + }); + + assert.match(core.outputs.support_dev_message, /Validation: claim holds>/); + assert.match(core.outputs.support_dev_message, /_\(ready for triage\)_$/); + }); +} + +test('a duplicate verdict is read with the colon outside the bold', async () => { + const core = fakeCore(); + await issueValidationDone({ + github: fakeGithub(), + context: fakeContext({ + commentBody: `${ISSUE_VALIDATION_MARKER}\n\n**Verdict**: duplicate of #15001\n**Evidence:** x`, + }), + core, + }); + + assert.match(core.outputs.support_dev_message, /Validation: duplicate of #15001>/); +}); + test('a body with CRLF line endings reads the same verdict', async () => { const core = fakeCore(); await issueValidationDone({ diff --git a/scripts/utils.js b/scripts/utils.js index 5c5f661..66be6bb 100644 --- a/scripts/utils.js +++ b/scripts/utils.js @@ -135,8 +135,17 @@ async function sendBotMessage(issueNumber, message, { github, context }) { } } +/** + * Every caller's message is interpolated into a JSON string literal in the + * leaf's Slack payload, so a backslash has to be escaped before the quote. + * The angle brackets are Slack's own link syntax. + */ function escapeIssueTitleForSlackMessage(issueTitle) { - return issueTitle.replace(/"/g, '\\"').replace(//g, '>'); + return issueTitle + .replace(/\\/g, '\\\\') + .replace(/"/g, '\\"') + .replace(//g, '>'); } /** diff --git a/scripts/utils.test.js b/scripts/utils.test.js new file mode 100644 index 0000000..334f41e --- /dev/null +++ b/scripts/utils.test.js @@ -0,0 +1,47 @@ +const test = require('node:test'); +const assert = require('node:assert/strict'); + +const { escapeIssueTitleForSlackMessage } = require('./utils'); + +// Every caller interpolates the result into a JSON string literal in its leaf's +// Slack payload, so the test parses that payload rather than the string alone. +function payload(title) { + return `{\n "text": "on issue: "\n}`; +} + +function rendered(title) { + return JSON.parse(payload(title)).text; +} + +test('a plain title passes through', () => { + assert.equal(rendered('Migrate the device tests'), 'on issue: '); +}); + +test('a backslash survives as one backslash', () => { + assert.equal(rendered('Fix C:\\path handling'), 'on issue: '); +}); + +test('a quote survives as one quote', () => { + assert.equal(rendered('Quote " in title'), 'on issue: '); +}); + +test('a backslash before a quote survives both', () => { + assert.equal(rendered('Odd \\" title'), 'on issue: '); +}); + +test('angle brackets cannot end the Slack link early', () => { + const text = rendered('Fix rendering'); + assert.equal(text, 'on issue: '); + assert.equal(text.match(/>/g).length, 1); +}); + +for (const title of [ + 'Fix C:\\path handling', + 'Quote " in title', + 'Both \\ and " and ', + 'Trailing backslash \\', +]) { + test(`the payload stays valid JSON for ${JSON.stringify(title)}`, () => { + assert.doesNotThrow(() => JSON.parse(payload(title))); + }); +} From b8e72f18ca4d46d88ada39bac967cc1b09253b8c Mon Sep 17 00:00:00 2001 From: Samson Akol Date: Thu, 29 Oct 2026 11:30:20 +0300 Subject: [PATCH 11/11] Drop the comments that restate the code What is left is the pointer to the coding-agent reader these regexes mirror, and the payload constraint that decides the escaping order. --- scripts/contributor-pr-reply.js | 2 -- scripts/contributor-pr-reply.test.js | 2 -- scripts/issue-validation-done.js | 3 +-- scripts/utils.js | 1 - scripts/utils.test.js | 2 -- 5 files changed, 1 insertion(+), 9 deletions(-) diff --git a/scripts/contributor-pr-reply.js b/scripts/contributor-pr-reply.js index 4048973..e16c5c7 100644 --- a/scripts/contributor-pr-reply.js +++ b/scripts/contributor-pr-reply.js @@ -13,8 +13,6 @@ module.exports = async ({ github, context, core }) => { const repo = context.repo.repo; const number = context.payload.pull_request.number; const url = context.payload.pull_request.html_url; - // Only the Slack lines below read this, and each one interpolates it into a - // JSON string literal in the leaf's payload. const title = escapeIssueTitleForSlackMessage(context.payload.pull_request.title); const author = context.payload.pull_request.user.login; diff --git a/scripts/contributor-pr-reply.test.js b/scripts/contributor-pr-reply.test.js index 41ebd07..042524d 100644 --- a/scripts/contributor-pr-reply.test.js +++ b/scripts/contributor-pr-reply.test.js @@ -231,8 +231,6 @@ test('a failed lookup falls back to the standard reply', async () => { assert.deepEqual(core.failures, []); }); -// The leaf interpolates the Slack line into a JSON string literal, so a quote or -// a backslash in the title breaks the payload and fails the step. test('a title holding a quote and a backslash reaches Slack escaped', async () => { const github = fakeGithub([{ number: 7, assignees: [AUTHOR] }]); const core = await run(github, fakeContext(AUTHOR, 'Treat "done" as C:\\full')); diff --git a/scripts/issue-validation-done.js b/scripts/issue-validation-done.js index ca14bdc..b29bb20 100644 --- a/scripts/issue-validation-done.js +++ b/scripts/issue-validation-done.js @@ -14,8 +14,7 @@ const { removeLabel, escapeIssueTitleForSlackMessage } = require('./utils'); // The verdict line is written by the agent's LLM session, not by a template, so // its shape varies. These three mirror the agent's own reader, which has to // tolerate that: _LIST_NUMBER_RE, _VERDICT_RE and the strips in parse_verdict -// and classify_verdict, all in learningequality/coding-agent. Anything stricter -// here reports a healthy run as broken. +// and classify_verdict, all in learningequality/coding-agent. const LIST_NUMBER = /^(\s*)\d+[.)][ \t]+/gm; const VERDICT_LINE = /^[\s>*_-]*verdict[*_\s]*:\s*(.+?)\s*$/im; const VERDICT_TRIM = /^[`*_\s.!;]+|[`*_\s.!;]+$/g; diff --git a/scripts/utils.js b/scripts/utils.js index 66be6bb..df835d3 100644 --- a/scripts/utils.js +++ b/scripts/utils.js @@ -138,7 +138,6 @@ async function sendBotMessage(issueNumber, message, { github, context }) { /** * Every caller's message is interpolated into a JSON string literal in the * leaf's Slack payload, so a backslash has to be escaped before the quote. - * The angle brackets are Slack's own link syntax. */ function escapeIssueTitleForSlackMessage(issueTitle) { return issueTitle diff --git a/scripts/utils.test.js b/scripts/utils.test.js index 334f41e..6355ad4 100644 --- a/scripts/utils.test.js +++ b/scripts/utils.test.js @@ -3,8 +3,6 @@ const assert = require('node:assert/strict'); const { escapeIssueTitleForSlackMessage } = require('./utils'); -// Every caller interpolates the result into a JSON string literal in its leaf's -// Slack payload, so the test parses that payload rather than the string alone. function payload(title) { return `{\n "text": "on issue: "\n}`; }