Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .agents/skills/build-from-issue/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ Use a single issue comment beginning with `> **🏗️ build-plan**` when a plan
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`.
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.
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.
4. Review the diff, use a signed-off Conventional Commit, and prepare a PR following `create-github-pr`.
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.

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.

Expand Down
23 changes: 23 additions & 0 deletions .agents/skills/create-github-pr/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,26 @@ Create pull requests on GitHub using the `gh` CLI.

## Before Creating a PR

### Verify Contributor Access and Issue Acceptance

Before pushing or creating a PR (including a draft), verify the author's access
to the target repository, not their fork. For the authenticated `gh` user:

```bash
gh repo view OWNER/REPO --json nameWithOwner,viewerPermission
gh issue view ISSUE_NUMBER --repo OWNER/REPO --json number,url,labels
```

Authors with verified `WRITE`, `MAINTAIN`, or `ADMIN` access may proceed without
`state:accepted`; keep the issue link and note the missing label. Everyone else
must link at least one target-repository issue and have `state:accepted` on each.
Otherwise, preserve local work and report which issue needs acceptance. A direct
request, roadmap placement, other labels, or vouch status does not bypass this gate.

When acting for another author, verify their permissions. Resolve failed lookups
before proceeding; retry sandbox network failures outside the sandbox before
reporting an authentication problem. Vouch requirements still apply separately.

### Check Config Documentation

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

The command outputs the PR URL and number.

Rerun `PR Issue Acceptance` after linked issue labels change; it does not refresh
automatically on issue-label events.

**Display the URL using markdown link syntax** so it's easily clickable:

```
Expand Down
19 changes: 18 additions & 1 deletion .agents/skills/review-github-pr/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -39,11 +39,28 @@ gh pr list --head "<branch>" --state open
Retrieve the PR metadata:

```bash
gh pr view <number> --json title,body,state,headRefName,baseRefName,labels,author
gh pr view <number> --json title,body,state,headRefName,baseRefName,labels,author,closingIssuesReferences
```

Record the **title**, **body**, **headRefName**, and **baseRefName** for use in later steps.

### Check Issue Acceptance

Inspect every closing issue reference (paginate as needed) and fetch its current
repository and labels. Verify the PR author's permission in the target repository:

```bash
gh api repos/OWNER/REPO/collaborators/AUTHOR/permission --jq .permission
```

Call out each linked issue missing `state:accepted`, with an issue link. Verified
`write`, `maintain`, or `admin` access permits the exception; report it as context,
not a blocking concern. Other authors need at least one target-repository issue,
and every linked issue must be accepted; report violations under **Potential
Concerns**. PR body text such as `No issue required` does not bypass this gate.
Report failed lookups as unverified and continue the code review. For a branch
without a PR, note that issue acceptance was not verified.

## Step 3: Generate the Diff

### Primary: gh pr diff
Expand Down
6 changes: 5 additions & 1 deletion .github/CODEOWNERS
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@

## Merge enforcement — these paths decide who can merge, so they notify the full
# list of maintainers to raise scrutiny on changes here.
#
#
# PR checks run local to the branch, so unless the check specifically avoids
# using branch-local files, a PR could change its own gates and appear to pass
# them. Covering the required checks here closes that gap.
Expand All @@ -31,5 +31,9 @@
/tasks/scripts/check_maintainer_approval_test.py @NVIDIA/openshell-codeowners @mrunalp @sjenning @derekwaynecarr
/tasks/scripts/alert_maintainer_change_test.py @NVIDIA/openshell-codeowners @mrunalp @sjenning @derekwaynecarr

# PR issue acceptance checks
/tasks/scripts/check_issue_acceptance.cjs @NVIDIA/openshell-codeowners @mrunalp @sjenning @derekwaynecarr
/tasks/scripts/check_issue_acceptance.test.cjs @NVIDIA/openshell-codeowners @mrunalp @sjenning @derekwaynecarr

# Auditable SBOM check
/tasks/scripts/verify-image-sbom.sh @NVIDIA/openshell-codeowners @mrunalp @sjenning @derekwaynecarr
2 changes: 1 addition & 1 deletion .github/PULL_REQUEST_TEMPLATE.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
<!-- 1-3 sentences: what this PR does and why -->

## Related Issue
<!-- Every PR must close an existing issue covering its scope, except automated dependency updates. Use Closes #NNN; see CONTRIBUTING.md for the exception. -->
<!-- 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. -->

## Changes
<!-- Bullet list of key changes -->
Expand Down
40 changes: 40 additions & 0 deletions .github/workflows/pr-issue-acceptance.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
# SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
# SPDX-License-Identifier: Apache-2.0

name: PR Issue Acceptance

on:
pull_request_target:
types: [opened, edited, reopened, synchronize]
Comment thread
github-advanced-security[bot] marked this conversation as resolved.
Fixed

permissions:
contents: read
issues: read
pull-requests: read

concurrency:
group: pr-issue-acceptance-${{ github.event.pull_request.number }}
cancel-in-progress: true

jobs:
issue-acceptance:
name: OpenShell / PR Issue Acceptance
timeout-minutes: 5
if: github.repository_owner == 'NVIDIA'
runs-on: ubuntu-latest
steps:
# pull_request_target supplies the trusted default-branch SHA. Load
# only that helper; PR metadata is data, never executable code.
- uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
with:
ref: ${{ github.sha }}
persist-credentials: false
sparse-checkout: tasks/scripts/check_issue_acceptance.cjs
sparse-checkout-cone-mode: false

- name: Check author access and linked issue acceptance
uses: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 # v9
with:
script: |
const check = require('./tasks/scripts/check_issue_acceptance.cjs');
await check({ github, context, core });
1 change: 1 addition & 0 deletions .github/zizmor.yml
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ rules:
- dco.yml:3
- e2e-label-help.yml:13
- maintainers-change-alert.yml:6
- pr-issue-acceptance.yml:6
- release-canary.yml:3
- required-ci-gates.yml:3
- vouch-check.yml:3
4 changes: 4 additions & 0 deletions AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,10 @@ ocsf_emit!(event);

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

## Pull Requests

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.

## Commits

- Always use [Conventional Commits](https://www.conventionalcommits.org/) format for commit messages
Expand Down
6 changes: 5 additions & 1 deletion CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -394,7 +394,11 @@ See [docs/CONTRIBUTING.mdx](docs/CONTRIBUTING.mdx) for the current docs authorin

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).

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.
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.

The `PR Issue Acceptance` workflow reports the native `OpenShell / PR Issue Acceptance` job result for the triggering PR when it is opened, edited, reopened, or updated with new commits. It does not publish a shared commit status; lookup and execution errors fail the job. 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 job required for merging and configuring any required Actions event policy are separate repository settings; merge-queue support is not included.

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.

### Branch Names

Expand Down
77 changes: 77 additions & 0 deletions tasks/scripts/check_issue_acceptance.cjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
// SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
// SPDX-License-Identifier: Apache-2.0

module.exports = async function checkIssueAcceptance({ github, context, core }) {
const { owner, repo } = context.repo;
const { data: pr } = await github.rest.pulls.get({
Comment thread
elezar marked this conversation as resolved.
owner, repo, pull_number: context.payload.pull_request.number,
});
const author = pr.user.login;

async function evaluate() {
try {
const { data } = await github.rest.repos.getCollaboratorPermissionLevel({
owner, repo, username: author,
});
// The REST API maps maintain to write and triage to read.
if (['write', 'maintain', 'admin'].includes(data.permission)) {
core.info(`${author} has repository write access; acceptance gate passed.`);
return;
}
} catch (error) {
if (error.status !== 404) throw error;
}

const issues = [];
let cursor = null;
do {
const result = await github.graphql(`
query($owner: String!, $repo: String!, $number: Int!, $cursor: String) {
repository(owner: $owner, name: $repo) {
pullRequest(number: $number) {
closingIssuesReferences(first: 100, after: $cursor) {
nodes { number url repository { name owner { login } } }
pageInfo { hasNextPage endCursor }
}
}
}
}
`, { owner, repo, number: pr.number, cursor });
const linked = result.repository.pullRequest.closingIssuesReferences;
issues.push(...linked.nodes);
cursor = linked.pageInfo.hasNextPage ? linked.pageInfo.endCursor : null;
} while (cursor);

if (issues.length === 0) {
return 'Community PRs must link an issue with state:accepted using '
+ 'Fixes #NNN or Closes #NNN.';
}

const unaccepted = [];
for (const issue of issues) {
if (`${issue.repository.owner.login}/${issue.repository.name}`.toLowerCase()
!== `${owner}/${repo}`.toLowerCase()) {
return `Acceptance must be recorded in ${owner}/${repo}, not ${issue.url}.`;
}
const labels = await github.paginate(github.rest.issues.listLabelsOnIssue, {
owner: issue.repository.owner.login,
repo: issue.repository.name,
issue_number: issue.number,
per_page: 100,
});
if (!labels.some(label => label.name === 'state:accepted')) {
unaccepted.push(issue.url);
}
}

if (unaccepted.length) {
return 'Community PRs require state:accepted on every linked issue. '
+ `Ask a maintainer to accept: ${unaccepted.join(', ')}. `
+ 'After acceptance, rerun this workflow.';
}
core.info(`All ${issues.length} linked issue(s) have state:accepted.`);
}

const failure = await evaluate();
if (failure) core.setFailed(failure);
};
131 changes: 131 additions & 0 deletions tasks/scripts/check_issue_acceptance.test.cjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,131 @@
// SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
// SPDX-License-Identifier: Apache-2.0

const assert = require('node:assert/strict');
const test = require('node:test');
const checkIssueAcceptance = require('./check_issue_acceptance.cjs');

async function check({
permission = 'read', issues = [], body = '', permissionError,
number = 7, headSha = 'pr-head', pullError, issueError, labelError,
} = {}) {
const failures = [];
const github = {
rest: {
pulls: {
get: async ({ pull_number }) => {
assert.equal(pull_number, number);
if (pullError) throw new Error('PR lookup failed');
return { data: {
number, user: { login: 'contributor' }, head: { sha: headSha }, body,
} };
},
},
repos: {
getCollaboratorPermissionLevel: async ({ username }) => {
assert.equal(username, 'contributor');
if (permissionError) throw Object.assign(new Error('lookup failed'), { status: permissionError });
return { data: { permission } };
},
createCommitStatus() { assert.fail('PR decisions must not publish commit statuses'); },
},
issues: { listLabelsOnIssue() {} },
},
graphql: async (_query, variables) => {
assert.equal(variables.number, number);
if (issueError) throw new Error('Issue lookup failed');
return { repository: { pullRequest: { closingIssuesReferences: {
nodes: issues.map(issue => ({
number: issue.number,
url: `https://github.com/${issue.owner || 'NVIDIA'}/OpenShell/issues/${issue.number}`,
repository: { name: 'OpenShell', owner: { login: issue.owner || 'NVIDIA' } },
})),
pageInfo: { hasNextPage: false, endCursor: null },
} } } };
},
paginate: async (_method, { issue_number }) => {
if (labelError) throw new Error('Label lookup failed');
return issues.find(issue => issue.number === issue_number).labels || [];
},
};
await checkIssueAcceptance({
github,
context: {
repo: { owner: 'NVIDIA', repo: 'OpenShell' },
payload: { pull_request: { number } },
actor: 'maintainer', sha: 'base-commit', serverUrl: 'https://github.com', runId: 42,
},
core: { info() {}, setFailed: message => failures.push(message) },
});
return failures;
}

for (const permission of ['write', 'maintain', 'admin']) {
test(`${permission} access allows an issue-less PR`, async () => {
assert.deepEqual(await check({ permission }), []);
});
}

for (const body of ['', 'No issue required: typo fix', '<!-- No issue required: example -->']) {
test(`community PR without linked issues fails for body ${JSON.stringify(body)}`, async () => {
const failures = await check({ body });
assert.equal(failures.length, 1);
assert.match(failures[0], /must link an issue with state:accepted/);
});
}

test('all linked issues must be accepted, regardless of PR body or triggering actor', async () => {
const failures = await check({
body: 'No issue required: small fix',
issues: [{ number: 1, labels: [{ name: 'state:accepted' }] }, { number: 2 }],
});
assert.equal(failures.length, 1);
assert.match(failures[0], /issues\/2/);
});

test('accepted issues in the target repository pass', async () => {
assert.deepEqual(await check({ issues: [{ number: 1, labels: [{ name: 'state:accepted' }] }] }), []);
});

test('accepted issues in another repository cannot bypass the gate', async () => {
const failures = await check({ issues: [{ number: 1, owner: 'contributor', labels: [{ name: 'state:accepted' }] }] });
assert.equal(failures.length, 1);
assert.match(failures[0], /Acceptance must be recorded in NVIDIA\/OpenShell/);
});

test('a non-collaborator still needs an accepted issue', async () => {
assert.equal((await check({ permissionError: 404, body: 'No issue required: typo fix' })).length, 1);
});

test('permission lookup failures do not grant an exemption', async () => {
await assert.rejects(check({ permissionError: 403 }), /lookup failed/);
});

for (const [option, message] of [
['pullError', /PR lookup failed/],
['issueError', /Issue lookup failed/],
['labelError', /Label lookup failed/],
]) {
test(`${option} fails the workflow invocation`, async () => {
await assert.rejects(check({ [option]: true, issues: [{ number: 1 }] }), message);
});
}

test('PRs sharing a head SHA evaluate their own issue references in either order', async () => {
const rejected = { number: 7, headSha: 'shared-head', issues: [{ number: 1 }] };
const accepted = {
number: 8, headSha: 'shared-head',
issues: [{ number: 2, labels: [{ name: 'state:accepted' }] }],
};
for (const pair of [[rejected, accepted], [accepted, rejected]]) {
const results = await Promise.all(pair.map(input => check(input)));
pair.forEach((input, index) => {
assert.equal(results[index].length, input.number === 7 ? 1 : 0);
});
}
});

test('a failed refresh rejects after an earlier successful evaluation', async () => {
assert.deepEqual(await check({ permission: 'write' }), []);
await assert.rejects(check({ permission: 'write', pullError: true }), /PR lookup failed/);
});
Loading