Skip to content

Commit 6afbdbe

Browse files
Copilotscott-shumway
authored andcommitted
Avoid backtracking in issue spam checks
Copilot-Session: 38cc48ab-6686-41b4-b35e-459fb890f924
1 parent c66e298 commit 6afbdbe

4 files changed

Lines changed: 151 additions & 2 deletions

File tree

Lines changed: 129 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,129 @@
1+
const assert = require('node:assert/strict');
2+
const { readFileSync } = require('node:fs');
3+
const path = require('node:path');
4+
const { spawnSync } = require('node:child_process');
5+
const test = require('node:test');
6+
7+
const workflow = readFileSync(
8+
path.join(__dirname, '../workflows/check-for-spammy-issues.yml'),
9+
'utf8',
10+
);
11+
const script = workflow.split(' script: |\n')[1];
12+
assert.ok(script, 'The workflow must include its github-script body');
13+
14+
const harness = `
15+
const { readFileSync } = require('node:fs');
16+
const input = JSON.parse(readFileSync(0, 'utf8'));
17+
const calls = [];
18+
const context = {
19+
payload: {
20+
issue: { number: 42, title: input.title, body: input.body },
21+
sender: { login: 'reporter' },
22+
},
23+
};
24+
const github = {
25+
teams: {
26+
async getMembershipForUserInOrg() {
27+
if (!input.employee) throw new Error('Not a team member');
28+
},
29+
},
30+
issues: {
31+
async update(params) { calls.push({ method: 'update', params }); },
32+
async createComment(params) { calls.push({ method: 'createComment', params }); },
33+
},
34+
};
35+
const AsyncFunction = Object.getPrototypeOf(async function () {}).constructor;
36+
new AsyncFunction('github', 'context', input.script)(github, context)
37+
.then(() => process.stdout.write(JSON.stringify(calls)))
38+
.catch(error => {
39+
console.error(error);
40+
process.exitCode = 1;
41+
});
42+
`;
43+
44+
function runWorkflow(body, { title = 'Too short', employee = false } = {}) {
45+
const result = spawnSync(process.execPath, ['-e', harness], {
46+
input: JSON.stringify({ script, body, title, employee }),
47+
encoding: 'utf8',
48+
timeout: 2000,
49+
});
50+
assert.ifError(result.error);
51+
assert.equal(result.status, 0, result.stderr);
52+
return JSON.parse(result.stdout);
53+
}
54+
55+
function assertClosed(body, options) {
56+
const calls = runWorkflow(body, options);
57+
assert.deepEqual(calls.map(call => call.method), ['update', 'createComment']);
58+
assert.deepEqual(calls[0].params, {
59+
owner: 'github',
60+
repo: 'rest-api-description',
61+
issue_number: 42,
62+
labels: ['invalid'],
63+
state: 'closed',
64+
});
65+
assert.equal(calls[1].params.issue_number, 42);
66+
}
67+
68+
test('closes issues with short titles and bodies', () => {
69+
assertClosed('Not enough detail');
70+
});
71+
72+
test('accepts a descriptive title', () => {
73+
assert.deepEqual(runWorkflow('', { title: 'A descriptive issue title' }), []);
74+
});
75+
76+
test('accepts a descriptive body', () => {
77+
assert.deepEqual(runWorkflow('This issue contains enough words to explain the problem'), []);
78+
});
79+
80+
test('does not count template comments or indented headings as body words', () => {
81+
assertClosed('<!--- These template words should not count -->\n # This heading should not count either\nToo short');
82+
});
83+
84+
test('preserves descriptive text between headings', () => {
85+
assert.deepEqual(
86+
runWorkflow('# First heading\nThis issue contains enough words to explain the problem\n## Next heading\n'),
87+
[],
88+
);
89+
});
90+
91+
test('ignores a heading without a trailing newline', () => {
92+
assertClosed('# These heading words alone should not keep an issue open');
93+
});
94+
95+
test('handles CRLF headings and body text', () => {
96+
assertClosed(' \t## These heading words should not count\r\nToo short\r\n');
97+
assert.deepEqual(
98+
runWorkflow('# Heading\r\nThis issue contains enough words to explain the problem\r\n'),
99+
[],
100+
);
101+
});
102+
103+
test('keeps hash characters that are not at the start of a line', () => {
104+
assert.deepEqual(runWorkflow('This body refers to #123 and explains the problem'), []);
105+
});
106+
107+
test('handles empty, null, and missing bodies', () => {
108+
for (const body of ['', null, undefined]) {
109+
assertClosed(body);
110+
}
111+
});
112+
113+
test('leaves employee issues open', () => {
114+
assert.deepEqual(runWorkflow(null, { employee: true }), []);
115+
});
116+
117+
test('still rejects the unchanged schema template title', () => {
118+
assertClosed('This issue contains enough words to explain the problem', {
119+
title: '[Schema Inaccuracy] <Describe Problem>',
120+
});
121+
});
122+
123+
test('handles a maximum-length heading followed by whitespace within two seconds', () => {
124+
assertClosed('#' + ' '.repeat(65535));
125+
});
126+
127+
test('handles maximum-length whitespace before a heading within two seconds', () => {
128+
assertClosed(' '.repeat(65534) + '#x');
129+
});

‎.github/workflows/check-for-spammy-issues.yml‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,8 +31,11 @@ jobs:
3131
}
3232
const titleWordCountMin = 3
3333
34-
let body = issue.body.replace(/<\!---.*?-->/g, "");
35-
body = body.replace(/^\s*(#)\s*(.+?)\s*\n/gm, "");
34+
let body = (issue.body ?? '').replace(/<\!---.*?-->/g, "");
35+
body = body
36+
.split('\n')
37+
.filter(line => !line.trimStart().startsWith('#'))
38+
.join('\n');
3639
3740
let bodyWordCount = 0
3841
if (body) {

‎.github/workflows/scripts-tests.yml‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,10 +8,12 @@ on:
88
paths:
99
- '.github/scripts/**'
1010
- '.github/workflows/scripts-tests.yml'
11+
- '.github/workflows/check-for-spammy-issues.yml'
1112
pull_request:
1213
paths:
1314
- '.github/scripts/**'
1415
- '.github/workflows/scripts-tests.yml'
16+
- '.github/workflows/check-for-spammy-issues.yml'
1517
workflow_dispatch:
1618

1719
jobs:
@@ -27,3 +29,9 @@ jobs:
2729
python-version: '3.12'
2830
- run: python3 -m unittest discover --start-directory .github/scripts --verbose
2931
name: Run script tests
32+
- uses: actions/setup-node@49933ea5288caeca8642d1e84afbd3f7d6820020 # v4.4.0
33+
name: Install NodeJS
34+
with:
35+
node-version: '22'
36+
- run: node --test .github/scripts/test_*.js
37+
name: Run JavaScript script tests

‎CONTRIBUTING.md‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,15 @@ This workflow is defined in `.github/workflows/linter-failure-notifier.yml`.
3838
0. Push to your fork and [submit a pull request][pr]
3939
0. Pat your self on the back and wait for your pull request to be reviewed and merged.
4040

41+
## Workflow script tests
42+
43+
Run the workflow script tests with Python 3 and Node.js 22:
44+
45+
```sh
46+
python3 -m unittest discover --start-directory .github/scripts --verbose
47+
node --test .github/scripts/test_*.js
48+
```
49+
4150
## Resources
4251

4352
- [How to Contribute to Open Source](https://opensource.guide/how-to-contribute/)

0 commit comments

Comments
 (0)