Skip to content

Commit d74a6c9

Browse files
committed
ci: require accepted issues for community pull requests
Publish an acceptance status on PR heads using trusted base code and repository-author permissions. Align contributor guidance and prevent PR body text from bypassing acceptance. Closes #4339 Signed-off-by: Evan Lezar <elezar@nvidia.com>
1 parent 81f712e commit d74a6c9

8 files changed

Lines changed: 258 additions & 3 deletions

File tree

‎.agents/skills/build-from-issue/SKILL.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ Use a single issue comment beginning with `> **🏗️ build-plan**` when a plan
2727
1. Check the current branch and working tree. Preserve unrelated work. Create an issue-specific branch or worktree as needed, following Branch Names in `CONTRIBUTING.md`.
2828
2. Implement the smallest coherent change that fulfills the acceptance criteria. Update relevant skills when behavior or commands change. Keep published documentation minimal: explain exactly what users need, avoid duplication across pages, and omit internal details with no user impact.
2929
3. Add meaningful tests for changed behavior and follow the verification guidance in `CONTRIBUTING.md`. Select format, lint, compile or type checks, and tests for affected components and their dependencies. Run the relevant E2E lane for infrastructure, sandbox, or policy changes. Guidance and template edits need applicable Markdown, YAML, link, and consistency checks. Do not require full Rust, SDK, or repository CI solely because a commit or PR is being created; broaden checks only for a concrete remaining risk or failed check.
30-
4. Review the diff, use a signed-off Conventional Commit, and prepare a PR following `create-github-pr`.
30+
4. Before pushing, apply the contributor access and issue acceptance check in `create-github-pr`. If blocked, preserve local work and report the reason. Review the diff, use a signed-off Conventional Commit, and prepare the PR.
3131

3232
Every PR from this issue-backed workflow must have its own existing issue and use `Closes #<id>` in its Related Issue section. For work needing multiple PRs, split the scope into a closable issue per PR. A high-level issue may track those issues but should not be closed by an incomplete PR. Report the implementation, verification, and any remaining limitation in the PR description, rather than copying earlier issue diagnostics.
3333

‎.agents/skills/create-github-pr/SKILL.md‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,26 @@ Create pull requests on GitHub using the `gh` CLI.
1717

1818
## Before Creating a PR
1919

20+
### Verify Contributor Access and Issue Acceptance
21+
22+
Before pushing or creating a PR (including a draft), verify the author's access
23+
to the target repository, not their fork. For the authenticated `gh` user:
24+
25+
```bash
26+
gh repo view OWNER/REPO --json nameWithOwner,viewerPermission
27+
gh issue view ISSUE_NUMBER --repo OWNER/REPO --json number,url,labels
28+
```
29+
30+
Authors with verified `WRITE`, `MAINTAIN`, or `ADMIN` access may proceed without
31+
`state:accepted`; keep the issue link and note the missing label. Everyone else
32+
must link at least one target-repository issue and have `state:accepted` on each.
33+
Otherwise, preserve local work and report which issue needs acceptance. A direct
34+
request, roadmap placement, other labels, or vouch status does not bypass this gate.
35+
36+
When acting for another author, verify their permissions. Resolve failed lookups
37+
before proceeding; retry sandbox network failures outside the sandbox before
38+
reporting an authentication problem. Vouch requirements still apply separately.
39+
2040
### Check Config Documentation
2141

2242
If the branch changes gateway TOML parsing, `[openshell.gateway]` fields,
@@ -220,6 +240,9 @@ EOF
220240

221241
The command outputs the PR URL and number.
222242

243+
Rerun `PR Issue Acceptance` after linked issue labels change; it does not refresh
244+
automatically on issue-label events.
245+
223246
**Display the URL using markdown link syntax** so it's easily clickable:
224247

225248
```

‎.github/PULL_REQUEST_TEMPLATE.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
<!-- 1-3 sentences: what this PR does and why -->
33

44
## Related Issue
5-
<!-- Every PR must close an existing issue covering its scope, except automated dependency updates. Use Closes #NNN; see CONTRIBUTING.md for the exception. -->
5+
<!-- Every PR must close an existing issue covering its scope, except automated dependency updates. Use Closes #NNN; see CONTRIBUTING.md for the exception. Authors without repository write access must link at least one issue with state:accepted. PR body text cannot bypass this acceptance check. -->
66

77
## Changes
88
<!-- Bullet list of key changes -->
Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
# SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
2+
# SPDX-License-Identifier: Apache-2.0
3+
4+
name: PR Issue Acceptance
5+
6+
on:
7+
pull_request_target:
8+
types: [opened, edited, reopened, synchronize]
9+
10+
permissions:
11+
contents: read
12+
issues: read
13+
pull-requests: read
14+
statuses: write
15+
16+
concurrency:
17+
group: pr-issue-acceptance-${{ github.event.pull_request.number }}
18+
cancel-in-progress: true
19+
20+
jobs:
21+
issue-acceptance:
22+
if: github.repository_owner == 'NVIDIA'
23+
runs-on: ubuntu-latest
24+
steps:
25+
# Load trusted base code only; never check out or execute the PR head.
26+
- uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
27+
with:
28+
ref: ${{ github.sha }}
29+
persist-credentials: false
30+
sparse-checkout: tasks/scripts/check_issue_acceptance.cjs
31+
sparse-checkout-cone-mode: false
32+
33+
- name: Check author access and linked issue acceptance
34+
uses: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 # v9
35+
with:
36+
script: |
37+
const check = require('./tasks/scripts/check_issue_acceptance.cjs');
38+
await check({ github, context, core });

‎AGENTS.md‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -175,6 +175,10 @@ ocsf_emit!(event);
175175

176176
Follow the [branch naming convention in CONTRIBUTING.md](CONTRIBUTING.md#branch-names).
177177

178+
## Pull Requests
179+
180+
Follow `CONTRIBUTING.md` and `create-github-pr`. Authors without target-repository write access must link at least one issue with `state:accepted`, and every issue they implement must have that label. Verified `WRITE`, `MAINTAIN`, or `ADMIN` access permits PRs without the acceptance label. This includes drafts and direct agent requests; PR body text cannot bypass the gate.
181+
178182
## Commits
179183

180184
- Always use [Conventional Commits](https://www.conventionalcommits.org/) format for commit messages

‎CONTRIBUTING.md‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -388,7 +388,11 @@ See [docs/CONTRIBUTING.mdx](docs/CONTRIBUTING.mdx) for the current docs authorin
388388

389389
Every PR except an automated dependency update must close an existing issue. In the PR's **Related Issue** section, use `Closes #NNN` for the issue covering that PR's scope. Split multi-PR work into a closable issue for each PR; a tracking issue can link them. Security fixes follow the private disclosure process in [SECURITY.md](SECURITY.md).
390390

391-
Automated dependency update PRs, including Dependabot PRs, are exempt from the separate issue and closing-reference requirement. Human-authored dependency updates follow the normal issue requirement. The exception does not change applicable verification, review, or private vulnerability disclosure requirements.
391+
Authors without write access to the target repository must link at least one issue with `state:accepted`, and every linked issue must be accepted. This applies to drafts and direct requests to agents. Authors with verified `WRITE`, `MAINTAIN`, or `ADMIN` access may create a PR without the acceptance label; they must still follow the issue-linking requirement above. PR body text, roadmap placement, other labels, and vouch status do not bypass acceptance.
392+
393+
The `PR Issue Acceptance` workflow publishes a status on the PR head commit when a PR is opened, edited, reopened, or updated with new commits. Link accepted issues in this repository using `Fixes #NNN` or `Closes #NNN`; GitHub recognizes closing references when a PR targets the default branch. Rerun the workflow after issue labels change. Making the status required for merging and configuring any required Actions event policy are separate repository settings; merge-queue support is not included.
394+
395+
Automated dependency update PRs, including Dependabot PRs, are exempt from the separate issue and closing-reference requirement. Human-authored dependency updates follow the normal issue requirement. The exception does not bypass `PR Issue Acceptance` for authors without repository write access or change applicable verification, review, or private vulnerability disclosure requirements.
392396

393397
### Branch Names
394398

Lines changed: 95 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,95 @@
1+
// SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
2+
// SPDX-License-Identifier: Apache-2.0
3+
4+
module.exports = async function checkIssueAcceptance({ github, context, core }) {
5+
const { owner, repo } = context.repo;
6+
const { data: pr } = await github.rest.pulls.get({
7+
owner, repo, pull_number: context.payload.pull_request.number,
8+
});
9+
const author = pr.user.login;
10+
11+
async function publish(state, description) {
12+
await github.rest.repos.createCommitStatus({
13+
owner, repo, sha: pr.head.sha,
14+
context: 'PR Issue Acceptance', state, description,
15+
target_url: `${context.serverUrl}/${owner}/${repo}/actions/runs/${context.runId}`,
16+
});
17+
}
18+
19+
await publish('pending', 'Checking author access and issue acceptance');
20+
21+
async function evaluate() {
22+
try {
23+
const { data } = await github.rest.repos.getCollaboratorPermissionLevel({
24+
owner, repo, username: author,
25+
});
26+
// The REST API maps maintain to write and triage to read.
27+
if (['write', 'maintain', 'admin'].includes(data.permission)) {
28+
core.info(`${author} has repository write access; acceptance gate passed.`);
29+
return;
30+
}
31+
} catch (error) {
32+
if (error.status !== 404) throw error;
33+
}
34+
35+
const issues = [];
36+
let cursor = null;
37+
do {
38+
const result = await github.graphql(`
39+
query($owner: String!, $repo: String!, $number: Int!, $cursor: String) {
40+
repository(owner: $owner, name: $repo) {
41+
pullRequest(number: $number) {
42+
closingIssuesReferences(first: 100, after: $cursor) {
43+
nodes { number url repository { name owner { login } } }
44+
pageInfo { hasNextPage endCursor }
45+
}
46+
}
47+
}
48+
}
49+
`, { owner, repo, number: pr.number, cursor });
50+
const linked = result.repository.pullRequest.closingIssuesReferences;
51+
issues.push(...linked.nodes);
52+
cursor = linked.pageInfo.hasNextPage ? linked.pageInfo.endCursor : null;
53+
} while (cursor);
54+
55+
if (issues.length === 0) {
56+
return 'Community PRs must link an issue with state:accepted using '
57+
+ 'Fixes #NNN or Closes #NNN.';
58+
}
59+
60+
const unaccepted = [];
61+
for (const issue of issues) {
62+
if (`${issue.repository.owner.login}/${issue.repository.name}`.toLowerCase()
63+
!== `${owner}/${repo}`.toLowerCase()) {
64+
return `Acceptance must be recorded in ${owner}/${repo}, not ${issue.url}.`;
65+
}
66+
const labels = await github.paginate(github.rest.issues.listLabelsOnIssue, {
67+
owner: issue.repository.owner.login,
68+
repo: issue.repository.name,
69+
issue_number: issue.number,
70+
per_page: 100,
71+
});
72+
if (!labels.some(label => label.name === 'state:accepted')) {
73+
unaccepted.push(issue.url);
74+
}
75+
}
76+
77+
if (unaccepted.length) {
78+
return 'Community PRs require state:accepted on every linked issue. '
79+
+ `Ask a maintainer to accept: ${unaccepted.join(', ')}. `
80+
+ 'After acceptance, rerun this workflow.';
81+
}
82+
core.info(`All ${issues.length} linked issue(s) have state:accepted.`);
83+
}
84+
85+
try {
86+
const failure = await evaluate();
87+
await publish(failure ? 'failure' : 'success', failure
88+
? 'Linked issues require maintainer acceptance'
89+
: 'Author access or issue acceptance verified');
90+
if (failure) core.setFailed(failure);
91+
} catch (error) {
92+
await publish('error', 'Could not verify author access or issue acceptance');
93+
throw error;
94+
}
95+
};
Lines changed: 91 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,91 @@
1+
// SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
2+
// SPDX-License-Identifier: Apache-2.0
3+
4+
const assert = require('node:assert/strict');
5+
const test = require('node:test');
6+
const checkIssueAcceptance = require('./check_issue_acceptance.cjs');
7+
8+
async function check({ permission = 'read', issues = [], body = '', permissionError } = {}) {
9+
const failures = [];
10+
const statuses = [];
11+
const github = {
12+
rest: {
13+
pulls: {
14+
get: async () => ({ data: {
15+
number: 7, user: { login: 'contributor' }, head: { sha: 'pr-head' }, body,
16+
} }),
17+
},
18+
repos: {
19+
getCollaboratorPermissionLevel: async ({ username }) => {
20+
assert.equal(username, 'contributor');
21+
if (permissionError) throw Object.assign(new Error('lookup failed'), { status: permissionError });
22+
return { data: { permission } };
23+
},
24+
createCommitStatus: async status => { statuses.push(status); },
25+
},
26+
issues: { listLabelsOnIssue() {} },
27+
},
28+
graphql: async () => ({ repository: { pullRequest: { closingIssuesReferences: {
29+
nodes: issues.map(issue => ({
30+
number: issue.number,
31+
url: `https://github.com/${issue.owner || 'NVIDIA'}/OpenShell/issues/${issue.number}`,
32+
repository: { name: 'OpenShell', owner: { login: issue.owner || 'NVIDIA' } },
33+
})),
34+
pageInfo: { hasNextPage: false, endCursor: null },
35+
} } } }),
36+
paginate: async (_method, { issue_number }) => issues.find(issue => issue.number === issue_number).labels || [],
37+
};
38+
await checkIssueAcceptance({
39+
github,
40+
context: {
41+
repo: { owner: 'NVIDIA', repo: 'OpenShell' },
42+
payload: { pull_request: { number: 7 } },
43+
actor: 'maintainer', sha: 'base-commit', serverUrl: 'https://github.com', runId: 42,
44+
},
45+
core: { info() {}, setFailed: message => failures.push(message) },
46+
});
47+
assert.deepEqual(statuses.map(status => status.state), ['pending', failures.length ? 'failure' : 'success']);
48+
assert(statuses.every(status => status.sha === 'pr-head' && status.context === 'PR Issue Acceptance'));
49+
return failures;
50+
}
51+
52+
for (const permission of ['write', 'maintain', 'admin']) {
53+
test(`${permission} access allows an issue-less PR`, async () => {
54+
assert.deepEqual(await check({ permission }), []);
55+
});
56+
}
57+
58+
for (const body of ['', 'No issue required: typo fix', '<!-- No issue required: example -->']) {
59+
test(`community PR without linked issues fails for body ${JSON.stringify(body)}`, async () => {
60+
const failures = await check({ body });
61+
assert.equal(failures.length, 1);
62+
assert.match(failures[0], /must link an issue with state:accepted/);
63+
});
64+
}
65+
66+
test('all linked issues must be accepted, regardless of PR body or triggering actor', async () => {
67+
const failures = await check({
68+
body: 'No issue required: small fix',
69+
issues: [{ number: 1, labels: [{ name: 'state:accepted' }] }, { number: 2 }],
70+
});
71+
assert.equal(failures.length, 1);
72+
assert.match(failures[0], /issues\/2/);
73+
});
74+
75+
test('accepted issues in the target repository pass', async () => {
76+
assert.deepEqual(await check({ issues: [{ number: 1, labels: [{ name: 'state:accepted' }] }] }), []);
77+
});
78+
79+
test('accepted issues in another repository cannot bypass the gate', async () => {
80+
const failures = await check({ issues: [{ number: 1, owner: 'contributor', labels: [{ name: 'state:accepted' }] }] });
81+
assert.equal(failures.length, 1);
82+
assert.match(failures[0], /Acceptance must be recorded in NVIDIA\/OpenShell/);
83+
});
84+
85+
test('a non-collaborator still needs an accepted issue', async () => {
86+
assert.equal((await check({ permissionError: 404, body: 'No issue required: typo fix' })).length, 1);
87+
});
88+
89+
test('permission lookup failures do not grant an exemption', async () => {
90+
await assert.rejects(check({ permissionError: 403 }), /lookup failed/);
91+
});

0 commit comments

Comments
 (0)