From ba7ae51f30b8b3652abc3241b3a05f6ebedac1d8 Mon Sep 17 00:00:00 2001 From: Ritwik G Date: Tue, 27 Oct 2026 11:57:58 +0530 Subject: [PATCH 1/8] feat: CLA check action to replace cla-assistant.io Checks every PR commit author against a signatures file in a private repo, records signatures posted as a PR comment, and reports the result as the license/cla commit status so existing branch protection keeps working. Includes an importer for the cla-assistant.io JSON export. Co-Authored-By: Claude Opus 5.5 --- .github/workflows/ci.yml | 19 ++++ .gitignore | 1 + README.md | 98 ++++++++++++++++++ action.yml | 49 +++++++++ examples/cla.yml | 40 ++++++++ package.json | 12 +++ scripts/import-cla-assistant.js | 72 +++++++++++++ src/cla.js | 137 +++++++++++++++++++++++++ src/github.js | 46 +++++++++ src/index.js | 48 +++++++++ src/main.js | 132 ++++++++++++++++++++++++ src/store.js | 54 ++++++++++ test/cla.test.js | 98 ++++++++++++++++++ test/fake-github.js | 88 ++++++++++++++++ test/main.test.js | 173 ++++++++++++++++++++++++++++++++ 15 files changed, 1067 insertions(+) create mode 100644 .github/workflows/ci.yml create mode 100644 .gitignore create mode 100644 README.md create mode 100644 action.yml create mode 100644 examples/cla.yml create mode 100644 package.json create mode 100644 scripts/import-cla-assistant.js create mode 100644 src/cla.js create mode 100644 src/github.js create mode 100644 src/index.js create mode 100644 src/main.js create mode 100644 src/store.js create mode 100644 test/cla.test.js create mode 100644 test/fake-github.js create mode 100644 test/main.test.js diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml new file mode 100644 index 0000000..cccb49a --- /dev/null +++ b/.github/workflows/ci.yml @@ -0,0 +1,19 @@ +name: CI + +on: + pull_request: + push: + branches: [main] + +permissions: + contents: read + +jobs: + test: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + - uses: actions/setup-node@v4 + with: + node-version: 24 + - run: npm test diff --git a/.gitignore b/.gitignore new file mode 100644 index 0000000..c2658d7 --- /dev/null +++ b/.gitignore @@ -0,0 +1 @@ +node_modules/ diff --git a/README.md b/README.md new file mode 100644 index 0000000..abc90a8 --- /dev/null +++ b/README.md @@ -0,0 +1,98 @@ +# cla-action + +A GitHub Action that enforces the Zipstack Contributor License Agreement (CLA) on pull requests. It replaces cla-assistant.io with something that runs on GitHub Actions and depends on no external service. + +## How it works + +On every pull request, and on every new PR comment: + +1. It lists the PR's commit authors. +2. Each author passes if they have already signed, are a bot or on the allowlist, or are a member of the organization. +3. It sets a commit status (`license/cla` by default) on the PR head: `success` when everyone passes, `failure` otherwise. **Make this status the required check.** +4. When someone still needs to sign, it posts one comment on the PR naming them and giving the exact phrase to post. It edits that same comment as things change, instead of adding new ones. +5. When a commit author posts the phrase, it records their signature and re-runs the check. Commenting `recheck` re-runs the check without signing. + +A signature covers every repository that uses the same signatures file, so a contributor signs once. + +### Where signatures live + +Signatures are stored in one JSON file (`signatures.json`) in a private repository, which the action reads and writes through the GitHub contents API: + +```json +{ + "version": 1, + "signatures": [ + { + "id": 583231, + "login": "octocat", + "signed_at": "2026-10-27T10:00:00.000Z", + "cla_document": "https://gist.github.com/...", + "repository": "Zipstack/unstract", + "pull_request": 2373, + "comment_url": "https://github.com/Zipstack/unstract/pull/2373#issuecomment-..." + } + ] +} +``` + +Signers are matched by numeric GitHub user id, so renaming an account doesn't invalidate a signature. Each write includes the file's current blob sha, so two signatures recorded at the same moment can't overwrite each other: the later write is rejected, re-reads the file and retries. + +## Setup + +1. **Signatures repository.** Create a private repository (e.g. `Zipstack/cla-signatures`). The action creates `signatures.json` on the first signature. +2. **GitHub App.** Create an app owned by the organization with these permissions: + - Repository → Contents: **Read and write**, installed on the signatures repository only. + - Organization → Members: **Read**, needed when `exempt-org-members` is on. + + Store its id as the organization variable `CLA_APP_ID` and its private key as the organization secret `CLA_APP_PRIVATE_KEY`. +3. **Workflow.** Copy [`examples/cla.yml`](examples/cla.yml) to `.github/workflows/cla.yml` in each repository that requires the CLA. +4. **Branch protection.** Require the `license/cla` status. If the repository used cla-assistant.io, it already requires that context. Unlink the repository from cla-assistant.io first, so the two don't both set the status. + +### Why `pull_request_target` + +PRs from forks don't get secrets under `pull_request`, and the action needs a token for the signatures repository. `pull_request_target` runs in the context of the base repository, so it gets the token. That is safe only because the workflow never checks out or runs the PR's code, so **never add a checkout step to this workflow.** + +## Inputs + +| Input | Default | Description | +| --- | --- | --- | +| `cla-document-url` | — (required) | Link to the CLA text. | +| `signatures-repository` | — (required) | `owner/name` of the signatures repository. | +| `signatures-token` | — (required) | Token with contents read/write on the signatures repository, plus organization members read. | +| `signatures-path` | `signatures.json` | Path of the signatures file. | +| `signatures-branch` | `main` | Branch to read and write. | +| `sign-phrase` | `I have read the CLA Document and I hereby sign the CLA` | Comment that signs. Case, whitespace and a trailing full stop are ignored. | +| `allowlist` | `*[bot]` | Comma-separated logins that never need to sign. `*` is a wildcard. | +| `agent-logins` | `claude` | Coding-agent accounts whose commits count as the PR author's (see below). | +| `exempt-org-members` | `true` | Skip members of the organization. | +| `organization` | repository owner | Organization whose members are exempt. | +| `status-context` | `license/cla` | Name of the commit status. | +| `github-token` | `${{ github.token }}` | Token for the calling repository. The workflow must grant `pull-requests: write` and `statuses: write`. | + +## Commits that can't be signed + +A commit whose author email isn't linked to any GitHub account has no GitHub user, so nobody can sign for it. The PR comment lists those commits by short sha and author name (never by email), and the check stays failing until the author links the email to their account or amends the commits. + +## Commits made by coding agents + +Commits made with Claude Code are authored by `noreply@anthropic.com`, which GitHub links to the account `claude`. That account can't sign anything on the contributor's behalf, so commits from any login in `agent-logins` are attributed to the PR author, and the PR author signs for them. Org members stay exempt as usual. Don't put agent accounts on the `allowlist`: that would let anyone skip the CLA by having the agent make every commit. + +## Migrating from cla-assistant.io + +1. Log in at [cla-assistant.io](https://cla-assistant.io) with an account that has push access to the linked repository. Open the repository's signature report and download **Complete History as JSON**. +2. Convert it: + + ```sh + GITHUB_TOKEN=$(gh auth token) node scripts/import-cla-assistant.js cla-assistant.json > signatures.json + ``` + + The script resolves each login to its GitHub user id, skips revoked signatures, keeps the earliest signature per person, and reports any account that no longer exists. To merge into an existing file, pass it as the second argument. +3. Commit `signatures.json` to the signatures repository. + +## Development + +The action has no dependencies; it uses Node's built-in `fetch` and test runner. + +```sh +npm test +``` diff --git a/action.yml b/action.yml new file mode 100644 index 0000000..88139bc --- /dev/null +++ b/action.yml @@ -0,0 +1,49 @@ +name: Zipstack CLA check +description: Requires every commit author on a pull request to have signed the Contributor License Agreement, and records signatures made by PR comment. +author: Zipstack + +inputs: + cla-document-url: + description: Link to the CLA text that contributors agree to. + required: true + signatures-repository: + description: Repository (owner/name) holding the signatures file, normally private. + required: true + signatures-token: + description: Token with contents read/write on the signatures repository, and organization members read when exempt-org-members is on. Use a GitHub App token. + required: true + signatures-path: + description: Path of the signatures file in the signatures repository. + default: signatures.json + signatures-branch: + description: Branch of the signatures repository to read and write. + default: main + sign-phrase: + description: Exact comment a contributor posts to sign. + default: I have read the CLA Document and I hereby sign the CLA + allowlist: + description: Comma-separated logins that never need to sign. `*` is a wildcard. + default: '*[bot]' + agent-logins: + description: Comma-separated logins of coding agents whose commits count as the PR author's, so the PR author signs for them. + default: claude + exempt-org-members: + description: Skip members of the organization (they are covered by their employment terms). + default: 'true' + organization: + description: Organization whose members are exempt. Defaults to the repository owner. + default: '' + status-context: + description: Name of the commit status this action sets. Make this the required check. + default: license/cla + github-token: + description: Token for the calling repository. Needs pull-requests write and statuses write. + default: ${{ github.token }} + +runs: + using: node24 + main: src/index.js + +branding: + icon: edit-3 + color: blue diff --git a/examples/cla.yml b/examples/cla.yml new file mode 100644 index 0000000..944d2f2 --- /dev/null +++ b/examples/cla.yml @@ -0,0 +1,40 @@ +# Copy to .github/workflows/cla.yml in each repository that requires the CLA. +# +# pull_request_target runs in the context of the base repository, so it gets +# secrets even for PRs from forks. That is safe here because this workflow +# never checks out or runs the PR's code. Do not add a checkout step. +name: CLA + +on: + pull_request_target: + types: [opened, synchronize, reopened] + issue_comment: + types: [created] + +permissions: + pull-requests: write + statuses: write + +concurrency: + group: cla-${{ github.event.pull_request.number || github.event.issue.number }} + +jobs: + cla: + # Comments on issues (not PRs) never need a CLA check. + if: github.event_name == 'pull_request_target' || github.event.issue.pull_request + runs-on: ubuntu-latest + steps: + - name: Mint signatures token + id: app-token + uses: actions/create-github-app-token@v2 + with: + app-id: ${{ vars.CLA_APP_ID }} + private-key: ${{ secrets.CLA_APP_PRIVATE_KEY }} + owner: Zipstack + repositories: cla-signatures + + - uses: Zipstack/cla-action@v1 + with: + cla-document-url: https://gist.github.com/arun-venkataswamy/0dec44b8e5155c43461caf62c38ddac3 + signatures-repository: Zipstack/cla-signatures + signatures-token: ${{ steps.app-token.outputs.token }} diff --git a/package.json b/package.json new file mode 100644 index 0000000..aa4c5ae --- /dev/null +++ b/package.json @@ -0,0 +1,12 @@ +{ + "name": "cla-action", + "private": true, + "description": "GitHub Action that enforces the Zipstack Contributor License Agreement on pull requests", + "license": "MIT", + "engines": { + "node": ">=24" + }, + "scripts": { + "test": "node --test test/*.test.js" + } +} diff --git a/scripts/import-cla-assistant.js b/scripts/import-cla-assistant.js new file mode 100644 index 0000000..42c75ed --- /dev/null +++ b/scripts/import-cla-assistant.js @@ -0,0 +1,72 @@ +#!/usr/bin/env node +'use strict' + +// One-off migration from cla-assistant.io. +// +// Export: log in at https://cla-assistant.io with a GitHub account that has +// push access to the linked repository, open the repository's signature +// report and download "Complete History as JSON". +// +// Usage: +// GITHUB_TOKEN=... node scripts/import-cla-assistant.js [existing-signatures.json] > signatures.json +// +// cla-assistant records the GitHub login only, so each login is resolved to +// its numeric user id (which survives renames). Revoked signatures are +// skipped; for a login that signed more than once the earliest signature is +// kept. Logins that no longer exist are reported on stderr and skipped. + +const fs = require('node:fs') +const { createClient } = require('../src/github') + +async function main() { + const [exportPath, existingPath] = process.argv.slice(2) + if (!exportPath) throw new Error('usage: import-cla-assistant.js [existing-signatures.json]') + if (!process.env.GITHUB_TOKEN) throw new Error('GITHUB_TOKEN is required to resolve GitHub user ids') + + const exported = JSON.parse(fs.readFileSync(exportPath, 'utf8')) + const data = existingPath + ? JSON.parse(fs.readFileSync(existingPath, 'utf8')) + : { version: 1, signatures: [] } + const known = new Set(data.signatures.map((signature) => signature.id)) + + const earliest = new Map() + for (const entry of exported) { + if (!entry.user_name || entry.revoked_at) continue + const login = entry.user_name.toLowerCase() + const current = earliest.get(login) + if (!current || new Date(entry.signed_at) < new Date(current.signed_at)) earliest.set(login, entry) + } + + const gh = createClient(process.env.GITHUB_TOKEN) + let added = 0 + for (const entry of [...earliest.values()].sort((a, b) => new Date(a.signed_at) - new Date(b.signed_at))) { + let user + try { + user = await gh.request('GET', `/users/${encodeURIComponent(entry.user_name)}`) + } catch (error) { + if (error.status !== 404) throw error + console.error(`skipped ${entry.user_name}: GitHub account not found`) + continue + } + if (known.has(user.id)) continue + known.add(user.id) + data.signatures.push({ + id: user.id, + login: user.login, + signed_at: new Date(entry.signed_at).toISOString(), + cla_document: entry.gist_url, + cla_version: entry.gist_version, + repository: `${entry.repo_owner}/${entry.repo_name}`, + source: 'cla-assistant.io', + }) + added++ + } + + process.stdout.write(`${JSON.stringify(data, null, 2)}\n`) + console.error(`imported ${added} signature(s); ${data.signatures.length} in total`) +} + +main().catch((error) => { + console.error(error.message) + process.exitCode = 1 +}) diff --git a/src/cla.js b/src/cla.js new file mode 100644 index 0000000..a6ff7ef --- /dev/null +++ b/src/cla.js @@ -0,0 +1,137 @@ +'use strict' + +// Pure logic: deciding who must sign and rendering the PR comment. +// Nothing in this file talks to GitHub. + +const COMMENT_MARKER = '' + +function normalize(text) { + return String(text || '') + .trim() + .replace(/\s+/g, ' ') + .replace(/[.!]+$/, '') + .toLowerCase() +} + +function isSignComment(body, phrase) { + return normalize(body) === normalize(phrase) +} + +function isRecheckComment(body) { + return normalize(body) === 'recheck' +} + +function parseList(value) { + return String(value || '') + .split(/[,\n]/) + .map((item) => item.trim()) + .filter(Boolean) +} + +// Patterns are GitHub logins, optionally with `*` wildcards (e.g. `*[bot]`). +function matchesAllowlist(login, patterns) { + return patterns.some((pattern) => { + const escaped = pattern.toLowerCase().replace(/[.+?^${}()|[\]\\]/g, '\\$&') + return new RegExp(`^${escaped.replace(/\*/g, '.*')}$`).test(login.toLowerCase()) + }) +} + +// Commit authors of a PR. A commit whose author email is not linked to a +// GitHub account has no `author` object; that person cannot sign until they +// link the email, so they are reported separately. +function collectCommitAuthors(commits) { + const users = new Map() + const unlinked = [] + for (const commit of commits) { + if (commit.author && commit.author.id) { + users.set(commit.author.id, { + id: commit.author.id, + login: commit.author.login, + type: commit.author.type, + }) + } else { + unlinked.push({ sha: commit.sha, name: commit.commit.author.name }) + } + } + return { users: [...users.values()], unlinked } +} + +// Commits made by a coding agent (e.g. Claude Code commits as the `claude` +// account) are the PR author's contribution, so the PR author is the one who +// must sign for them. Allowlisting the agent instead would let anyone skip the +// CLA by having the agent make every commit. +function attributeToPrAuthor(users, agentLogins, prAuthor) { + const agents = new Set(agentLogins.map((login) => login.toLowerCase())) + const result = new Map() + for (const user of users) { + const owner = agents.has(user.login.toLowerCase()) + ? { id: prAuthor.id, login: prAuthor.login, type: prAuthor.type } + : user + result.set(owner.id, owner) + } + return [...result.values()] +} + +async function classifyAuthors(users, { signedIds, allowlist, isOrgMember }) { + const result = { signed: [], exempt: [], unsigned: [] } + for (const user of users) { + if (signedIds.has(user.id)) { + result.signed.push(user) + } else if (user.type === 'Bot' || matchesAllowlist(user.login, allowlist)) { + result.exempt.push(user) + } else if (isOrgMember && (await isOrgMember(user.login))) { + result.exempt.push(user) + } else { + result.unsigned.push(user) + } + } + return result +} + +function renderComment({ unsigned, unlinked, claUrl, phrase }) { + if (unsigned.length === 0 && unlinked.length === 0) { + return `${COMMENT_MARKER}\n✅ All committers have signed the [CLA](${claUrl}). Thank you!` + } + + const lines = [ + COMMENT_MARKER, + `Thank you for your contribution! Before we can merge it, every committer needs to sign our [Contributor License Agreement](${claUrl}).`, + '', + ] + if (unsigned.length > 0) { + lines.push( + 'To sign, read the CLA and then post a comment on this pull request with exactly this text:', + '', + '```', + phrase, + '```', + '', + 'Still to sign:', + ...unsigned.map((user) => `- [ ] @${user.login}`), + '', + ) + } + if (unlinked.length > 0) { + lines.push( + 'These commits use an email address that is not linked to any GitHub account, so their author cannot sign. ' + + '[Add the email to your GitHub account](https://github.com/settings/emails), or amend the commits to use a linked email, then comment `recheck`.', + '', + ...unlinked.map((commit) => `- ${commit.sha.slice(0, 7)} (${commit.name})`), + '', + ) + } + lines.push('Already signed? Comment `recheck` to run the check again.') + return lines.join('\n') +} + +module.exports = { + COMMENT_MARKER, + attributeToPrAuthor, + classifyAuthors, + collectCommitAuthors, + isRecheckComment, + isSignComment, + matchesAllowlist, + parseList, + renderComment, +} diff --git a/src/github.js b/src/github.js new file mode 100644 index 0000000..e57abbc --- /dev/null +++ b/src/github.js @@ -0,0 +1,46 @@ +'use strict' + +// Minimal GitHub REST client on top of the built-in fetch, so the action +// ships without a node_modules directory or a bundling step. + +class GitHubError extends Error { + constructor(status, method, path, body) { + super(`${method} ${path} failed with ${status}: ${body}`) + this.status = status + } +} + +function createClient(token, apiUrl = process.env.GITHUB_API_URL || 'https://api.github.com') { + async function request(method, path, body) { + const headers = { + Accept: 'application/vnd.github+json', + Authorization: `Bearer ${token}`, + 'X-GitHub-Api-Version': '2022-11-28', + 'User-Agent': 'zipstack-cla-action', + } + if (body !== undefined) headers['Content-Type'] = 'application/json' + + const res = await fetch(`${apiUrl}${path}`, { + method, + headers, + body: body === undefined ? undefined : JSON.stringify(body), + }) + const text = await res.text() + if (!res.ok) throw new GitHubError(res.status, method, path, text) + return text ? JSON.parse(text) : null + } + + async function paginate(path) { + const items = [] + const sep = path.includes('?') ? '&' : '?' + for (let page = 1; ; page++) { + const batch = await request('GET', `${path}${sep}per_page=100&page=${page}`) + items.push(...batch) + if (batch.length < 100) return items + } + } + + return { request, paginate } +} + +module.exports = { createClient, GitHubError } diff --git a/src/index.js b/src/index.js new file mode 100644 index 0000000..a6b00d1 --- /dev/null +++ b/src/index.js @@ -0,0 +1,48 @@ +'use strict' + +const fs = require('node:fs') +const { parseList } = require('./cla') +const { createClient } = require('./github') +const { handle } = require('./main') + +// Same lookup as @actions/core getInput: INPUT_ with spaces as underscores. +function input(name, { required = false } = {}) { + const value = (process.env[`INPUT_${name.replace(/ /g, '_').toUpperCase()}`] || '').trim() + if (required && !value) throw new Error(`Input required and not supplied: ${name}`) + return value +} + +async function main() { + const repository = process.env.GITHUB_REPOSITORY + const inputs = { + claDocumentUrl: input('cla-document-url', { required: true }), + signaturesRepository: input('signatures-repository', { required: true }), + signaturesPath: input('signatures-path') || 'signatures.json', + signaturesBranch: input('signatures-branch') || 'main', + signPhrase: input('sign-phrase', { required: true }), + allowlist: parseList(input('allowlist')), + agentLogins: parseList(input('agent-logins')), + exemptOrgMembers: input('exempt-org-members') !== 'false', + organization: input('organization') || repository.split('/')[0], + statusContext: input('status-context') || 'license/cla', + } + + const result = await handle({ + eventName: process.env.GITHUB_EVENT_NAME, + event: JSON.parse(fs.readFileSync(process.env.GITHUB_EVENT_PATH, 'utf8')), + repository, + inputs, + gh: createClient(input('github-token', { required: true })), + signaturesGh: createClient(input('signatures-token', { required: true })), + log: console.log, + }) + + // The CLA outcome is reported through the commit status, which is what branch + // protection requires. The job itself only fails when the action breaks. + if (result && !result.passed) console.log('CLA not yet signed by every committer; status set to failure.') +} + +main().catch((error) => { + console.log(`::error::${error.message}`) + process.exitCode = 1 +}) diff --git a/src/main.js b/src/main.js new file mode 100644 index 0000000..74cc1c9 --- /dev/null +++ b/src/main.js @@ -0,0 +1,132 @@ +'use strict' + +const { + COMMENT_MARKER, + attributeToPrAuthor, + classifyAuthors, + collectCommitAuthors, + isRecheckComment, + isSignComment, + renderComment, +} = require('./cla') +const { addSignature, loadSignatures, signedIds } = require('./store') + +// Works out which PR the event is about and whether the commenter is signing. +// Returns null when the event needs no action. +function resolveTrigger(eventName, event, phrase) { + if (eventName === 'pull_request_target' || eventName === 'pull_request') { + return { prNumber: event.pull_request.number, signer: null } + } + if (eventName === 'issue_comment') { + if (event.action !== 'created' || !event.issue.pull_request) return null + const body = event.comment.body + if (isSignComment(body, phrase)) { + return { prNumber: event.issue.number, signer: event.comment.user, comment: event.comment } + } + if (isRecheckComment(body)) return { prNumber: event.issue.number, signer: null } + return null + } + throw new Error(`Unsupported event "${eventName}". Trigger on pull_request_target and issue_comment.`) +} + +async function upsertComment(gh, repository, prNumber, body, { createIfMissing }) { + const comments = await gh.paginate(`/repos/${repository}/issues/${prNumber}/comments`) + const existing = comments.find((comment) => comment.body && comment.body.includes(COMMENT_MARKER)) + if (existing) { + if (existing.body === body) return existing + return gh.request('PATCH', `/repos/${repository}/issues/comments/${existing.id}`, { body }) + } + if (!createIfMissing) return null + return gh.request('POST', `/repos/${repository}/issues/${prNumber}/comments`, { body }) +} + +async function handle({ eventName, event, repository, inputs, gh, signaturesGh, log }) { + const trigger = resolveTrigger(eventName, event, inputs.signPhrase) + if (!trigger) { + log('Nothing to do for this event.') + return null + } + + const pr = await gh.request('GET', `/repos/${repository}/pulls/${trigger.prNumber}`) + if (pr.state !== 'open') { + log(`PR #${pr.number} is ${pr.state}; skipping.`) + return null + } + + const commits = await gh.paginate(`/repos/${repository}/pulls/${pr.number}/commits`) + const authors = collectCommitAuthors(commits) + const users = attributeToPrAuthor(authors.users, inputs.agentLogins, pr.user) + const unlinked = authors.unlinked + const location = { + repository: inputs.signaturesRepository, + path: inputs.signaturesPath, + branch: inputs.signaturesBranch, + } + + let { data } = await loadSignatures(signaturesGh, location) + const signer = trigger.signer + if (signer && !signedIds(data).has(signer.id)) { + if (users.some((user) => user.id === signer.id)) { + data = await addSignature(signaturesGh, location, { + id: signer.id, + login: signer.login, + signed_at: new Date().toISOString(), + cla_document: inputs.claDocumentUrl, + repository, + pull_request: pr.number, + comment_url: trigger.comment.html_url, + }) + log(`Recorded signature for @${signer.login}.`) + } else { + log(`@${signer.login} is not a commit author on PR #${pr.number}; not recording a signature.`) + } + } + + const org = inputs.organization + const isOrgMember = inputs.exemptOrgMembers + ? async (login) => { + try { + await signaturesGh.request('GET', `/orgs/${org}/members/${encodeURIComponent(login)}`) + return true + } catch (error) { + if (error.status === 404) return false + throw error + } + } + : null + + const result = await classifyAuthors(users, { + signedIds: signedIds(data), + allowlist: inputs.allowlist, + isOrgMember, + }) + const passed = result.unsigned.length === 0 && unlinked.length === 0 + log( + `signed: ${result.signed.map((u) => u.login).join(', ') || '-'}; ` + + `exempt: ${result.exempt.map((u) => u.login).join(', ') || '-'}; ` + + `unsigned: ${result.unsigned.map((u) => u.login).join(', ') || '-'}; ` + + `unlinked commits: ${unlinked.length}`, + ) + + const comment = await upsertComment( + gh, + repository, + pr.number, + renderComment({ ...result, unlinked, claUrl: inputs.claDocumentUrl, phrase: inputs.signPhrase }), + { createIfMissing: !passed }, + ) + + const pending = result.unsigned.length + unlinked.length + await gh.request('POST', `/repos/${repository}/statuses/${pr.head.sha}`, { + state: passed ? 'success' : 'failure', + context: inputs.statusContext, + description: passed + ? 'All committers have signed the CLA.' + : `${pending} committer${pending === 1 ? '' : 's'} still need${pending === 1 ? 's' : ''} to sign the CLA.`, + target_url: comment ? comment.html_url : inputs.claDocumentUrl, + }) + + return { passed, ...result, unlinked } +} + +module.exports = { handle, resolveTrigger } diff --git a/src/store.js b/src/store.js new file mode 100644 index 0000000..1fabf97 --- /dev/null +++ b/src/store.js @@ -0,0 +1,54 @@ +'use strict' + +// Signatures live in one JSON file in a (private) repository, read and written +// through the contents API. Writes carry the blob sha, so two PRs signing at +// the same moment cannot overwrite each other: the loser gets 409, re-reads +// and tries again. + +const EMPTY = { version: 1, signatures: [] } +const MAX_WRITE_ATTEMPTS = 5 + +function contentsPath({ repository, path }) { + return `/repos/${repository}/contents/${path.split('/').map(encodeURIComponent).join('/')}` +} + +async function loadSignatures(gh, location) { + try { + const file = await gh.request('GET', `${contentsPath(location)}?ref=${encodeURIComponent(location.branch)}`) + const data = JSON.parse(Buffer.from(file.content, 'base64').toString('utf8')) + return { data, sha: file.sha } + } catch (error) { + if (error.status === 404) return { data: structuredClone(EMPTY), sha: undefined } + throw error + } +} + +function signedIds(data) { + return new Set(data.signatures.map((signature) => signature.id)) +} + +// Returns the signatures after the write. Signing twice is a no-op. +async function addSignature(gh, location, record) { + for (let attempt = 1; ; attempt++) { + const { data, sha } = await loadSignatures(gh, location) + if (signedIds(data).has(record.id)) return data + + data.signatures.push(record) + const body = { + message: `Add CLA signature for @${record.login}`, + content: Buffer.from(`${JSON.stringify(data, null, 2)}\n`).toString('base64'), + branch: location.branch, + } + if (sha) body.sha = sha + + try { + await gh.request('PUT', contentsPath(location), body) + return data + } catch (error) { + const conflict = error.status === 409 || error.status === 422 + if (!conflict || attempt >= MAX_WRITE_ATTEMPTS) throw error + } + } +} + +module.exports = { addSignature, loadSignatures, signedIds } diff --git a/test/cla.test.js b/test/cla.test.js new file mode 100644 index 0000000..feeac5b --- /dev/null +++ b/test/cla.test.js @@ -0,0 +1,98 @@ +'use strict' + +const test = require('node:test') +const assert = require('node:assert/strict') +const { + classifyAuthors, + collectCommitAuthors, + isRecheckComment, + isSignComment, + matchesAllowlist, + parseList, + renderComment, +} = require('../src/cla') +const { commitBy } = require('./fake-github') + +const PHRASE = 'I have read the CLA Document and I hereby sign the CLA' + +test('sign comment matches regardless of case, spacing and a trailing full stop', () => { + assert.ok(isSignComment(PHRASE, PHRASE)) + assert.ok(isSignComment(' i have read the CLA document\nand I hereby sign the CLA. ', PHRASE)) + assert.ok(!isSignComment(`${PHRASE} but not really`, PHRASE)) + assert.ok(!isSignComment('LGTM', PHRASE)) +}) + +test('recheck comment', () => { + assert.ok(isRecheckComment('recheck')) + assert.ok(isRecheckComment(' Recheck! ')) + assert.ok(!isRecheckComment('please recheck')) +}) + +test('allowlist supports wildcards and escapes the rest of the pattern', () => { + const patterns = parseList('*[bot], octocat') + assert.ok(matchesAllowlist('dependabot[bot]', patterns)) + assert.ok(matchesAllowlist('OctoCat', patterns)) + assert.ok(!matchesAllowlist('dependabotXbot', patterns)) + assert.ok(!matchesAllowlist('octocat2', patterns)) +}) + +test('commit authors are de-duplicated and unlinked commits reported separately', () => { + const alice = { id: 1, login: 'alice' } + const { users, unlinked } = collectCommitAuthors([ + commitBy(alice, '1'.repeat(40)), + commitBy(alice, '2'.repeat(40)), + commitBy(null, '3'.repeat(40)), + ]) + assert.deepEqual(users.map((u) => u.login), ['alice']) + assert.deepEqual(unlinked, [{ sha: '3'.repeat(40), name: 'Someone Unlinked' }]) +}) + +test('authors are signed, exempt (bot, allowlist, org member) or unsigned', async () => { + const users = [ + { id: 1, login: 'signed-user', type: 'User' }, + { id: 2, login: 'renovate', type: 'Bot' }, + { id: 3, login: 'allowed', type: 'User' }, + { id: 4, login: 'staff', type: 'User' }, + { id: 5, login: 'stranger', type: 'User' }, + ] + const result = await classifyAuthors(users, { + signedIds: new Set([1]), + allowlist: ['allowed'], + isOrgMember: async (login) => login === 'staff', + }) + assert.deepEqual(result.signed.map((u) => u.login), ['signed-user']) + assert.deepEqual(result.exempt.map((u) => u.login), ['renovate', 'allowed', 'staff']) + assert.deepEqual(result.unsigned.map((u) => u.login), ['stranger']) +}) + +test('org members are not exempt when the check is switched off', async () => { + const result = await classifyAuthors([{ id: 4, login: 'staff', type: 'User' }], { + signedIds: new Set(), + allowlist: [], + isOrgMember: null, + }) + assert.deepEqual(result.unsigned.map((u) => u.login), ['staff']) +}) + +test('comment lists who must sign and the exact phrase', () => { + const body = renderComment({ + unsigned: [{ login: 'stranger' }], + unlinked: [], + claUrl: 'https://example.com/cla', + phrase: PHRASE, + }) + assert.match(body, /zipstack-cla-action/) + assert.match(body, /@stranger/) + assert.ok(body.includes(PHRASE)) +}) + +test('comment explains unlinked commits without exposing the email', () => { + const body = renderComment({ + unsigned: [], + unlinked: [{ sha: 'abcdef1234567', name: 'Someone' }], + claUrl: 'https://example.com/cla', + phrase: PHRASE, + }) + assert.match(body, /abcdef1 \(Someone\)/) + assert.ok(!body.includes(PHRASE)) +}) diff --git a/test/fake-github.js b/test/fake-github.js new file mode 100644 index 0000000..fecc434 --- /dev/null +++ b/test/fake-github.js @@ -0,0 +1,88 @@ +'use strict' + +const { GitHubError } = require('../src/github') + +// In-memory stand-in for the parts of the GitHub API the action uses. +function fakeGitHub({ pr, commits = [], comments = [], members = [], signatures, conflictsBeforeWrite = 0 } = {}) { + const state = { + pr, + commits, + comments: comments.map((comment) => ({ ...comment })), + statuses: [], + members: new Set(members), + file: signatures === undefined ? null : { data: signatures, sha: 'sha-0' }, + writes: 0, + conflictsBeforeWrite, + } + let nextId = 1000 + + function encodeFile() { + return { + sha: state.file.sha, + content: Buffer.from(JSON.stringify(state.file.data)).toString('base64'), + } + } + + async function request(method, path, body) { + let match + if (method === 'GET' && (match = path.match(/^\/repos\/[^/]+\/[^/]+\/pulls\/(\d+)$/))) { + return state.pr + } + if (method === 'GET' && path.startsWith('/repos/Zipstack/cla-signatures/contents/')) { + if (!state.file) throw new GitHubError(404, method, path, 'Not Found') + return encodeFile() + } + if (method === 'PUT' && path.startsWith('/repos/Zipstack/cla-signatures/contents/')) { + if (state.conflictsBeforeWrite > 0) { + state.conflictsBeforeWrite-- + throw new GitHubError(409, method, path, 'sha mismatch') + } + if ((state.file && body.sha !== state.file.sha) || (!state.file && body.sha)) { + throw new GitHubError(409, method, path, 'sha mismatch') + } + state.writes++ + state.file = { + data: JSON.parse(Buffer.from(body.content, 'base64').toString('utf8')), + sha: `sha-${state.writes}`, + } + return {} + } + if (method === 'GET' && (match = path.match(/^\/orgs\/[^/]+\/members\/(.+)$/))) { + if (state.members.has(decodeURIComponent(match[1]))) return null + throw new GitHubError(404, method, path, 'Not Found') + } + if (method === 'POST' && path.match(/\/issues\/\d+\/comments$/)) { + const comment = { id: nextId++, body: body.body, html_url: `https://github.com/comment/${nextId}` } + state.comments.push(comment) + return comment + } + if (method === 'PATCH' && (match = path.match(/\/issues\/comments\/(\d+)$/))) { + const comment = state.comments.find((c) => c.id === Number(match[1])) + comment.body = body.body + return comment + } + if (method === 'POST' && path.match(/\/statuses\/[0-9a-f]+$/)) { + state.statuses.push(body) + return {} + } + throw new Error(`fake GitHub: unexpected ${method} ${path}`) + } + + async function paginate(path) { + if (path.match(/\/pulls\/\d+\/commits$/)) return state.commits + if (path.match(/\/issues\/\d+\/comments$/)) return state.comments + throw new Error(`fake GitHub: unexpected paginate ${path}`) + } + + return { request, paginate, state } +} + +function commitBy(user, sha = 'a'.repeat(40)) { + return { + sha, + author: user ? { id: user.id, login: user.login, type: user.type || 'User' } : null, + commit: { author: { name: user ? user.login : 'Someone Unlinked' } }, + } +} + +module.exports = { commitBy, fakeGitHub } diff --git a/test/main.test.js b/test/main.test.js new file mode 100644 index 0000000..5e9b04f --- /dev/null +++ b/test/main.test.js @@ -0,0 +1,173 @@ +'use strict' + +const test = require('node:test') +const assert = require('node:assert/strict') +const { handle, resolveTrigger } = require('../src/main') +const { addSignature } = require('../src/store') +const { commitBy, fakeGitHub } = require('./fake-github') + +const PHRASE = 'I have read the CLA Document and I hereby sign the CLA' +const HEAD = 'f'.repeat(40) +const alice = { id: 11, login: 'alice', type: 'User' } +const bob = { id: 22, login: 'bob', type: 'User' } +const staff = { id: 33, login: 'staff', type: 'User' } + +const inputs = { + claDocumentUrl: 'https://example.com/cla', + signaturesRepository: 'Zipstack/cla-signatures', + signaturesPath: 'signatures.json', + signaturesBranch: 'main', + signPhrase: PHRASE, + allowlist: ['*[bot]'], + agentLogins: ['claude'], + exemptOrgMembers: true, + organization: 'Zipstack', + statusContext: 'license/cla', +} + +function openPr(author = alice) { + return { number: 7, state: 'open', head: { sha: HEAD }, user: author } +} + +const claudeAgent = { id: 81847, login: 'claude', type: 'User' } + +function run(gh, eventName, event) { + return handle({ + eventName, + event, + repository: 'Zipstack/unstract', + inputs, + gh, + signaturesGh: gh, + log: () => {}, + }) +} + +const prEvent = { pull_request: { number: 7 } } +function commentEvent(user, body) { + return { + action: 'created', + issue: { number: 7, pull_request: {} }, + comment: { user, body, html_url: 'https://github.com/Zipstack/unstract/pull/7#c1' }, + } +} + +test('unsigned author: failure status and a comment asking them to sign', async () => { + const gh = fakeGitHub({ pr: openPr(), commits: [commitBy(alice)] }) + const result = await run(gh, 'pull_request_target', prEvent) + + assert.equal(result.passed, false) + assert.equal(gh.state.statuses.length, 1) + assert.equal(gh.state.statuses[0].state, 'failure') + assert.equal(gh.state.statuses[0].context, 'license/cla') + assert.equal(gh.state.comments.length, 1) + assert.match(gh.state.comments[0].body, /@alice/) +}) + +test('signing by comment records the signature and flips the status to success', async () => { + const gh = fakeGitHub({ pr: openPr(), commits: [commitBy(alice)] }) + await run(gh, 'pull_request_target', prEvent) + const result = await run(gh, 'issue_comment', commentEvent(alice, PHRASE)) + + assert.equal(result.passed, true) + assert.deepEqual(gh.state.file.data.signatures.map((s) => s.id), [alice.id]) + assert.equal(gh.state.file.data.signatures[0].pull_request, 7) + assert.equal(gh.state.statuses.at(-1).state, 'success') + assert.equal(gh.state.comments.length, 1, 'the existing comment is updated, not duplicated') + assert.match(gh.state.comments[0].body, /All committers have signed/) +}) + +test('a commenter who is not a commit author cannot sign on someone else\'s PR', async () => { + const gh = fakeGitHub({ pr: openPr(), commits: [commitBy(alice)] }) + const result = await run(gh, 'issue_comment', commentEvent(bob, PHRASE)) + + assert.equal(result.passed, false) + assert.equal(gh.state.file, null, 'nothing written') + assert.equal(gh.state.statuses.at(-1).state, 'failure') +}) + +test('signing twice writes only once', async () => { + const gh = fakeGitHub({ pr: openPr(), commits: [commitBy(alice)] }) + await run(gh, 'issue_comment', commentEvent(alice, PHRASE)) + await run(gh, 'issue_comment', commentEvent(alice, PHRASE)) + assert.equal(gh.state.writes, 1) +}) + +test('already-signed, org-member and bot authors pass without a comment', async () => { + const gh = fakeGitHub({ + pr: openPr(), + commits: [commitBy(alice), commitBy(staff), commitBy({ id: 99, login: 'dependabot[bot]', type: 'Bot' })], + members: ['staff'], + signatures: { version: 1, signatures: [{ id: alice.id, login: 'alice' }] }, + }) + const result = await run(gh, 'pull_request_target', prEvent) + + assert.equal(result.passed, true) + assert.equal(gh.state.statuses[0].state, 'success') + assert.equal(gh.state.comments.length, 0) +}) + +test('a commit with no linked GitHub account keeps the check failing', async () => { + const gh = fakeGitHub({ pr: openPr(), commits: [commitBy(null)], signatures: { version: 1, signatures: [] } }) + const result = await run(gh, 'pull_request_target', prEvent) + assert.equal(result.passed, false) + assert.match(gh.state.statuses[0].description, /1 committer still needs/) +}) + +test('agent-authored commits require the PR author to sign, not the agent', async () => { + const gh = fakeGitHub({ pr: openPr(bob), commits: [commitBy(claudeAgent)] }) + const result = await run(gh, 'pull_request_target', prEvent) + + assert.equal(result.passed, false) + assert.deepEqual(result.unsigned.map((u) => u.login), ['bob']) + assert.doesNotMatch(gh.state.comments[0].body, /@claude/) + + await run(gh, 'issue_comment', commentEvent(bob, PHRASE)) + assert.deepEqual(gh.state.file.data.signatures.map((s) => s.login), ['bob']) + assert.equal(gh.state.statuses.at(-1).state, 'success') +}) + +test('agent-authored commits by an org member pass through the member exemption', async () => { + const gh = fakeGitHub({ pr: openPr(staff), commits: [commitBy(claudeAgent)], members: ['staff'] }) + const result = await run(gh, 'pull_request_target', prEvent) + assert.equal(result.passed, true) +}) + +test('closed PRs and unrelated comments are ignored', async () => { + const gh = fakeGitHub({ pr: { ...openPr(), state: 'closed' }, commits: [commitBy(alice)] }) + assert.equal(await run(gh, 'pull_request_target', prEvent), null) + assert.equal(await run(gh, 'issue_comment', commentEvent(alice, 'LGTM')), null) + assert.equal(gh.state.statuses.length, 0) +}) + +test('comments on plain issues are ignored', () => { + const event = { action: 'created', issue: { number: 1 }, comment: { body: PHRASE } } + assert.equal(resolveTrigger('issue_comment', event, PHRASE), null) +}) + +test('edited comments do not sign', () => { + const event = { ...commentEvent(alice, PHRASE), action: 'edited' } + assert.equal(resolveTrigger('issue_comment', event, PHRASE), null) +}) + +test('a concurrent write is retried against the latest file', async () => { + const gh = fakeGitHub({ signatures: { version: 1, signatures: [] }, conflictsBeforeWrite: 2 }) + const location = { repository: 'Zipstack/cla-signatures', path: 'signatures.json', branch: 'main' } + const data = await addSignature(gh, location, { id: 1, login: 'alice' }) + assert.deepEqual(data.signatures.map((s) => s.id), [1]) + assert.equal(gh.state.writes, 1) +}) + +test('a signature recorded by a concurrent run is not written again', async () => { + const gh = fakeGitHub({ signatures: { version: 1, signatures: [{ id: 1, login: 'alice' }] } }) + const location = { repository: 'Zipstack/cla-signatures', path: 'signatures.json', branch: 'main' } + const data = await addSignature(gh, location, { id: 1, login: 'alice' }) + assert.equal(data.signatures.length, 1) + assert.equal(gh.state.writes, 0) +}) + +test('a write that keeps conflicting gives up instead of looping', async () => { + const gh = fakeGitHub({ signatures: { version: 1, signatures: [] }, conflictsBeforeWrite: 99 }) + const location = { repository: 'Zipstack/cla-signatures', path: 'signatures.json', branch: 'main' } + await assert.rejects(addSignature(gh, location, { id: 1, login: 'alice' }), /409/) +}) From a1b59f25b7baca6a0e934015d16167d313829914 Mon Sep 17 00:00:00 2001 From: Ritwik G Date: Tue, 27 Oct 2026 12:03:58 +0530 Subject: [PATCH 2/8] chore: add MIT licence Co-Authored-By: Claude Opus 5.5 --- LICENSE | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) create mode 100644 LICENSE diff --git a/LICENSE b/LICENSE new file mode 100644 index 0000000..4c6e1cb --- /dev/null +++ b/LICENSE @@ -0,0 +1,21 @@ +MIT License + +Copyright (c) 2026 Zipstack, Inc. + +Permission is hereby granted, free of charge, to any person obtaining a copy +of this software and associated documentation files (the "Software"), to deal +in the Software without restriction, including without limitation the rights +to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +copies of the Software, and to permit persons to whom the Software is +furnished to do so, subject to the following conditions: + +The above copyright notice and this permission notice shall be included in all +copies or substantial portions of the Software. + +THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE +SOFTWARE. From 392164dfc567e448bac3c5280065e4d60bfdf518 Mon Sep 17 00:00:00 2001 From: Ritwik G Date: Tue, 27 Oct 2026 12:17:46 +0530 Subject: [PATCH 3/8] fix: close CLA bypasses and lost-signature paths from self-review round 1 - Check the PR author as well as commit authors. Commit authors are matched by email, which the committer sets, so attributing every commit to an exempt or signed account no longer passes the check. - Fail a PR whose commit count exceeds what GitHub lists (250) instead of passing on a partial list. - Run the full check on every PR comment and record signatures from all sign comments on the PR, so a sign comment whose run the concurrency group cancelled is recorded by the next run. - Make the fake GitHub behave like the real one on write conflicts (a concurrent writer adds a signature and moves the sha; a missing sha on an existing file gets 422), and test the real pagination loop. - README: describe the above, correct when Claude Code commits as `claude`, and list the known limitations accepted for now. Co-Authored-By: Claude Opus 5.5 --- README.md | 28 ++++++++++-- src/cla.js | 31 +++++++++----- src/main.js | 86 ++++++++++++++++++++++--------------- test/cla.test.js | 11 ++--- test/fake-github.js | 20 ++++++--- test/github.test.js | 38 +++++++++++++++++ test/main.test.js | 101 ++++++++++++++++++++++++++++++++++++++------ 7 files changed, 243 insertions(+), 72 deletions(-) create mode 100644 test/github.test.js diff --git a/README.md b/README.md index abc90a8..52e1158 100644 --- a/README.md +++ b/README.md @@ -6,11 +6,17 @@ A GitHub Action that enforces the Zipstack Contributor License Agreement (CLA) o On every pull request, and on every new PR comment: -1. It lists the PR's commit authors. -2. Each author passes if they have already signed, are a bot or on the allowlist, or are a member of the organization. +1. It lists the PR's commit authors, plus the PR author. +2. Each of them passes if they have already signed, are a bot or on the allowlist, or are a member of the organization. 3. It sets a commit status (`license/cla` by default) on the PR head: `success` when everyone passes, `failure` otherwise. **Make this status the required check.** 4. When someone still needs to sign, it posts one comment on the PR naming them and giving the exact phrase to post. It edits that same comment as things change, instead of adding new ones. -5. When a commit author posts the phrase, it records their signature and re-runs the check. Commenting `recheck` re-runs the check without signing. +5. When a commit author or the PR author has posted the phrase in a comment on the PR, it records their signature. Any comment re-runs the check; the PR comment suggests `recheck`. + +Every run reads all of the PR's comments for signatures, not just the comment that triggered it. The workflow's concurrency group keeps one pending run per PR and cancels the one already pending, so a sign comment's own run may never execute; the next run records it instead. + +The PR author is checked even without commits of their own. Commit authors are matched by the commit's email address, which the committer chooses, so without that a contributor could attribute every commit to an exempt or already-signed account. + +GitHub lists at most 250 commits of a pull request. A PR with more fails the check and asks to be split, rather than passing on a partial list. A signature covers every repository that uses the same signatures file, so a contributor signs once. @@ -75,7 +81,21 @@ A commit whose author email isn't linked to any GitHub account has no GitHub use ## Commits made by coding agents -Commits made with Claude Code are authored by `noreply@anthropic.com`, which GitHub links to the account `claude`. That account can't sign anything on the contributor's behalf, so commits from any login in `agent-logins` are attributed to the PR author, and the PR author signs for them. Org members stay exempt as usual. Don't put agent accounts on the `allowlist`: that would let anyone skip the CLA by having the agent make every commit. +When Claude Code is itself the commit author (for example in a Claude Code cloud session), the commit is authored by `noreply@anthropic.com`, which GitHub links to the account `claude`. A local Claude Code session usually commits as the developer and names Claude only in a `Co-Authored-By` trailer, which the action does not read. The `claude` account can't sign anything on the contributor's behalf, so commits from any login in `agent-logins` are attributed to the PR author, and the PR author signs for them. Org members stay exempt as usual. Don't put agent accounts on the `allowlist`: that would let anyone skip the CLA by having the agent make every commit. + +## Known limitations + +Accepted for now; each one either fails closed or is a policy choice still open. + +- **A run that errors posts no status.** The required check then stays pending, and the error is only in the workflow run. API calls have no timeout or retry beyond the signatures write. +- **A missing signatures file reads as "nobody has signed yet",** and an organization the token can't see reads as "not a member". A misconfigured repository, path, branch or organization therefore asks everyone to sign rather than failing with a configuration error. +- **A sign comment that is not accepted gets no reply on the PR** (for example from someone who is neither a commit author nor the PR author). The reason is only in the workflow log. +- **A signature is not tied to a CLA version.** Any signature in the file counts for every repository and every `cla-document-url`. Records from the importer carry `cla_version`; records from the action don't. +- **The PR comment is found by its marker alone**, whoever wrote it, and an unlinked commit's author name is shown as the committer wrote it. +- **The signatures file must stay under 1 MB** (a few thousand signatures): above that the contents API stops returning the file and every run fails. +- **`exempt-org-members` is off only for the exact value `false`.** +- **The status description counts unlinked commits** as committers, one per commit. +- **The file's `version` field is not checked** when it is read. ## Migrating from cla-assistant.io diff --git a/src/cla.js b/src/cla.js index a6ff7ef..736342f 100644 --- a/src/cla.js +++ b/src/cla.js @@ -17,10 +17,6 @@ function isSignComment(body, phrase) { return normalize(body) === normalize(phrase) } -function isRecheckComment(body) { - return normalize(body) === 'recheck' -} - function parseList(value) { return String(value || '') .split(/[,\n]/) @@ -56,9 +52,9 @@ function collectCommitAuthors(commits) { return { users: [...users.values()], unlinked } } -// Commits made by a coding agent (e.g. Claude Code commits as the `claude` -// account) are the PR author's contribution, so the PR author is the one who -// must sign for them. Allowlisting the agent instead would let anyone skip the +// Commits made by a coding agent (e.g. a Claude Code cloud session commits as +// the `claude` account) are the PR author's contribution, so the PR author is +// the one who must sign for them. Allowlisting the agent instead would let anyone skip the // CLA by having the agent make every commit. function attributeToPrAuthor(users, agentLogins, prAuthor) { const agents = new Set(agentLogins.map((login) => login.toLowerCase())) @@ -72,6 +68,15 @@ function attributeToPrAuthor(users, agentLogins, prAuthor) { return [...result.values()] } +// The PR author must sign (or be exempt) even with no commit of their own. +// Commit authors are matched by the commit's email, which the committer sets +// freely, so without this a contributor could attribute every commit to an +// exempt or already-signed account and never sign. +function addPrAuthor(users, prAuthor) { + if (users.some((user) => user.id === prAuthor.id)) return users + return [...users, { id: prAuthor.id, login: prAuthor.login, type: prAuthor.type }] +} + async function classifyAuthors(users, { signedIds, allowlist, isOrgMember }) { const result = { signed: [], exempt: [], unsigned: [] } for (const user of users) { @@ -88,8 +93,8 @@ async function classifyAuthors(users, { signedIds, allowlist, isOrgMember }) { return result } -function renderComment({ unsigned, unlinked, claUrl, phrase }) { - if (unsigned.length === 0 && unlinked.length === 0) { +function renderComment({ unsigned, unlinked, truncated = false, claUrl, phrase }) { + if (unsigned.length === 0 && unlinked.length === 0 && !truncated) { return `${COMMENT_MARKER}\n✅ All committers have signed the [CLA](${claUrl}). Thank you!` } @@ -98,6 +103,12 @@ function renderComment({ unsigned, unlinked, claUrl, phrase }) { `Thank you for your contribution! Before we can merge it, every committer needs to sign our [Contributor License Agreement](${claUrl}).`, '', ] + if (truncated) { + lines.push( + 'This pull request has more commits than GitHub lists (250), so not every committer can be checked. Split it into smaller pull requests.', + '', + ) + } if (unsigned.length > 0) { lines.push( 'To sign, read the CLA and then post a comment on this pull request with exactly this text:', @@ -126,10 +137,10 @@ function renderComment({ unsigned, unlinked, claUrl, phrase }) { module.exports = { COMMENT_MARKER, + addPrAuthor, attributeToPrAuthor, classifyAuthors, collectCommitAuthors, - isRecheckComment, isSignComment, matchesAllowlist, parseList, diff --git a/src/main.js b/src/main.js index 74cc1c9..5230dce 100644 --- a/src/main.js +++ b/src/main.js @@ -2,35 +2,36 @@ const { COMMENT_MARKER, + addPrAuthor, attributeToPrAuthor, classifyAuthors, collectCommitAuthors, - isRecheckComment, isSignComment, renderComment, } = require('./cla') const { addSignature, loadSignatures, signedIds } = require('./store') -// Works out which PR the event is about and whether the commenter is signing. -// Returns null when the event needs no action. -function resolveTrigger(eventName, event, phrase) { +// Works out which PR the event is about. Returns null when the event needs no +// action. +// +// Every PR comment runs the whole check, not just sign and `recheck` comments. +// The workflow's concurrency group keeps only one pending run per PR and +// cancels the one already pending, so a sign comment's run can be replaced by +// a later comment's run. Each run therefore does all the work, including +// recording sign comments it finds on the PR, so the run that replaced a +// cancelled one does what the cancelled one would have done. +function resolveTrigger(eventName, event) { if (eventName === 'pull_request_target' || eventName === 'pull_request') { - return { prNumber: event.pull_request.number, signer: null } + return { prNumber: event.pull_request.number } } if (eventName === 'issue_comment') { if (event.action !== 'created' || !event.issue.pull_request) return null - const body = event.comment.body - if (isSignComment(body, phrase)) { - return { prNumber: event.issue.number, signer: event.comment.user, comment: event.comment } - } - if (isRecheckComment(body)) return { prNumber: event.issue.number, signer: null } - return null + return { prNumber: event.issue.number } } throw new Error(`Unsupported event "${eventName}". Trigger on pull_request_target and issue_comment.`) } -async function upsertComment(gh, repository, prNumber, body, { createIfMissing }) { - const comments = await gh.paginate(`/repos/${repository}/issues/${prNumber}/comments`) +async function upsertComment(gh, repository, prNumber, comments, body, { createIfMissing }) { const existing = comments.find((comment) => comment.body && comment.body.includes(COMMENT_MARKER)) if (existing) { if (existing.body === body) return existing @@ -41,7 +42,7 @@ async function upsertComment(gh, repository, prNumber, body, { createIfMissing } } async function handle({ eventName, event, repository, inputs, gh, signaturesGh, log }) { - const trigger = resolveTrigger(eventName, event, inputs.signPhrase) + const trigger = resolveTrigger(eventName, event) if (!trigger) { log('Nothing to do for this event.') return null @@ -54,8 +55,12 @@ async function handle({ eventName, event, repository, inputs, gh, signaturesGh, } const commits = await gh.paginate(`/repos/${repository}/pulls/${pr.number}/commits`) + // The PR commits endpoint returns at most 250 commits. Authors beyond that + // are never seen, so a PR longer than that fails rather than passing on a + // partial list. + const truncated = commits.length < pr.commits const authors = collectCommitAuthors(commits) - const users = attributeToPrAuthor(authors.users, inputs.agentLogins, pr.user) + const users = addPrAuthor(attributeToPrAuthor(authors.users, inputs.agentLogins, pr.user), pr.user) const unlinked = authors.unlinked const location = { repository: inputs.signaturesRepository, @@ -63,23 +68,32 @@ async function handle({ eventName, event, repository, inputs, gh, signaturesGh, branch: inputs.signaturesBranch, } + // Signatures come from every sign comment on the PR, not only the one that + // triggered this run (see resolveTrigger). Only a commit author or the PR + // author can sign; the earliest sign comment per person is recorded. + const comments = await gh.paginate(`/repos/${repository}/issues/${pr.number}/comments`) let { data } = await loadSignatures(signaturesGh, location) - const signer = trigger.signer - if (signer && !signedIds(data).has(signer.id)) { - if (users.some((user) => user.id === signer.id)) { - data = await addSignature(signaturesGh, location, { - id: signer.id, - login: signer.login, - signed_at: new Date().toISOString(), - cla_document: inputs.claDocumentUrl, - repository, - pull_request: pr.number, - comment_url: trigger.comment.html_url, - }) - log(`Recorded signature for @${signer.login}.`) - } else { - log(`@${signer.login} is not a commit author on PR #${pr.number}; not recording a signature.`) + const required = new Set(users.map((user) => user.id)) + const handled = new Set() + for (const comment of comments) { + const signer = comment.user + if (!signer || handled.has(signer.id) || !isSignComment(comment.body, inputs.signPhrase)) continue + handled.add(signer.id) + if (signedIds(data).has(signer.id)) continue + if (!required.has(signer.id)) { + log(`@${signer.login} is not a commit author or the author of PR #${pr.number}; not recording a signature.`) + continue } + data = await addSignature(signaturesGh, location, { + id: signer.id, + login: signer.login, + signed_at: new Date().toISOString(), + cla_document: inputs.claDocumentUrl, + repository, + pull_request: pr.number, + comment_url: comment.html_url, + }) + log(`Recorded signature for @${signer.login}.`) } const org = inputs.organization @@ -100,7 +114,8 @@ async function handle({ eventName, event, repository, inputs, gh, signaturesGh, allowlist: inputs.allowlist, isOrgMember, }) - const passed = result.unsigned.length === 0 && unlinked.length === 0 + const passed = result.unsigned.length === 0 && unlinked.length === 0 && !truncated + if (truncated) log(`PR #${pr.number} has ${pr.commits} commits; only the first ${commits.length} can be listed.`) log( `signed: ${result.signed.map((u) => u.login).join(', ') || '-'}; ` + `exempt: ${result.exempt.map((u) => u.login).join(', ') || '-'}; ` + @@ -112,7 +127,8 @@ async function handle({ eventName, event, repository, inputs, gh, signaturesGh, gh, repository, pr.number, - renderComment({ ...result, unlinked, claUrl: inputs.claDocumentUrl, phrase: inputs.signPhrase }), + comments, + renderComment({ ...result, unlinked, truncated, claUrl: inputs.claDocumentUrl, phrase: inputs.signPhrase }), { createIfMissing: !passed }, ) @@ -122,11 +138,13 @@ async function handle({ eventName, event, repository, inputs, gh, signaturesGh, context: inputs.statusContext, description: passed ? 'All committers have signed the CLA.' - : `${pending} committer${pending === 1 ? '' : 's'} still need${pending === 1 ? 's' : ''} to sign the CLA.`, + : truncated + ? `PR has ${pr.commits} commits; only ${commits.length} can be checked. Split it into smaller PRs.` + : `${pending} committer${pending === 1 ? '' : 's'} still need${pending === 1 ? 's' : ''} to sign the CLA.`, target_url: comment ? comment.html_url : inputs.claDocumentUrl, }) - return { passed, ...result, unlinked } + return { passed, ...result, unlinked, truncated } } module.exports = { handle, resolveTrigger } diff --git a/test/cla.test.js b/test/cla.test.js index feeac5b..165c129 100644 --- a/test/cla.test.js +++ b/test/cla.test.js @@ -3,9 +3,9 @@ const test = require('node:test') const assert = require('node:assert/strict') const { + addPrAuthor, classifyAuthors, collectCommitAuthors, - isRecheckComment, isSignComment, matchesAllowlist, parseList, @@ -22,10 +22,11 @@ test('sign comment matches regardless of case, spacing and a trailing full stop' assert.ok(!isSignComment('LGTM', PHRASE)) }) -test('recheck comment', () => { - assert.ok(isRecheckComment('recheck')) - assert.ok(isRecheckComment(' Recheck! ')) - assert.ok(!isRecheckComment('please recheck')) +test('the PR author is added once to the people who must sign', () => { + const alice = { id: 1, login: 'alice', type: 'User' } + const bob = { id: 2, login: 'bob', type: 'User' } + assert.deepEqual(addPrAuthor([alice], bob), [alice, bob]) + assert.deepEqual(addPrAuthor([alice, bob], bob), [alice, bob]) }) test('allowlist supports wildcards and escapes the rest of the pattern', () => { diff --git a/test/fake-github.js b/test/fake-github.js index fecc434..ca66a1c 100644 --- a/test/fake-github.js +++ b/test/fake-github.js @@ -34,12 +34,17 @@ function fakeGitHub({ pr, commits = [], comments = [], members = [], signatures, } if (method === 'PUT' && path.startsWith('/repos/Zipstack/cla-signatures/contents/')) { if (state.conflictsBeforeWrite > 0) { + // Another run signs first: the file gains a signature and a new sha, + // so this write's sha is stale. state.conflictsBeforeWrite-- - throw new GitHubError(409, method, path, 'sha mismatch') - } - if ((state.file && body.sha !== state.file.sha) || (!state.file && body.sha)) { - throw new GitHubError(409, method, path, 'sha mismatch') + const concurrent = { id: 9000 + state.conflictsBeforeWrite, login: 'concurrent-signer' } + const data = state.file ? state.file.data : { version: 1, signatures: [] } + state.file = { data: { ...data, signatures: [...data.signatures, concurrent] }, sha: `sha-c${state.conflictsBeforeWrite}` } } + // GitHub answers 409 to a stale sha and 422 to a missing sha when the + // file already exists. + if (state.file && !body.sha) throw new GitHubError(422, method, path, '"sha" wasn\'t supplied') + if (state.file && body.sha !== state.file.sha) throw new GitHubError(409, method, path, 'sha mismatch') state.writes++ state.file = { data: JSON.parse(Buffer.from(body.content, 'base64').toString('utf8')), @@ -52,7 +57,12 @@ function fakeGitHub({ pr, commits = [], comments = [], members = [], signatures, throw new GitHubError(404, method, path, 'Not Found') } if (method === 'POST' && path.match(/\/issues\/\d+\/comments$/)) { - const comment = { id: nextId++, body: body.body, html_url: `https://github.com/comment/${nextId}` } + const comment = { + id: nextId++, + user: { id: 41898282, login: 'github-actions[bot]', type: 'Bot' }, + body: body.body, + html_url: `https://github.com/comment/${nextId}`, + } state.comments.push(comment) return comment } diff --git a/test/github.test.js b/test/github.test.js new file mode 100644 index 0000000..768313f --- /dev/null +++ b/test/github.test.js @@ -0,0 +1,38 @@ +'use strict' + +const test = require('node:test') +const assert = require('node:assert/strict') +const { createClient } = require('../src/github') + +// Serves `total` numbered items in pages, as the GitHub REST API does. +function stubFetch(t, total) { + const urls = [] + t.mock.method(globalThis, 'fetch', async (url) => { + urls.push(url) + const params = new URL(url).searchParams + const perPage = Number(params.get('per_page')) + const start = (Number(params.get('page')) - 1) * perPage + const items = Array.from({ length: Math.max(0, Math.min(perPage, total - start)) }, (_, i) => start + i) + return new Response(JSON.stringify(items), { status: 200 }) + }) + return urls +} + +test('paginate follows pages until a short one', async (t) => { + const urls = stubFetch(t, 250) + const items = await createClient('token', 'https://api.test').paginate('/repos/o/r/pulls/1/commits') + assert.equal(items.length, 250) + assert.equal(urls.length, 3) +}) + +test('paginate stops after an exactly full last page with one empty request', async (t) => { + const urls = stubFetch(t, 200) + const items = await createClient('token', 'https://api.test').paginate('/x') + assert.equal(items.length, 200) + assert.equal(urls.length, 3) +}) + +test('a failed request throws with the HTTP status', async (t) => { + t.mock.method(globalThis, 'fetch', async () => new Response('Not Found', { status: 404 })) + await assert.rejects(createClient('token', 'https://api.test').request('GET', '/x'), (error) => error.status === 404) +}) diff --git a/test/main.test.js b/test/main.test.js index 5e9b04f..275b17c 100644 --- a/test/main.test.js +++ b/test/main.test.js @@ -4,6 +4,7 @@ const test = require('node:test') const assert = require('node:assert/strict') const { handle, resolveTrigger } = require('../src/main') const { addSignature } = require('../src/store') +const { COMMENT_MARKER } = require('../src/cla') const { commitBy, fakeGitHub } = require('./fake-github') const PHRASE = 'I have read the CLA Document and I hereby sign the CLA' @@ -52,6 +53,14 @@ function commentEvent(user, body) { } } +// Posts a comment on the PR and runs the action for it, as GitHub would. +let nextCommentId = 1 +function comment(gh, user, body) { + const id = nextCommentId++ + gh.state.comments.push({ id, user, body, html_url: `https://github.com/Zipstack/unstract/pull/7#issuecomment-${id}` }) + return run(gh, 'issue_comment', commentEvent(user, body)) +} + test('unsigned author: failure status and a comment asking them to sign', async () => { const gh = fakeGitHub({ pr: openPr(), commits: [commitBy(alice)] }) const result = await run(gh, 'pull_request_target', prEvent) @@ -67,19 +76,20 @@ test('unsigned author: failure status and a comment asking them to sign', async test('signing by comment records the signature and flips the status to success', async () => { const gh = fakeGitHub({ pr: openPr(), commits: [commitBy(alice)] }) await run(gh, 'pull_request_target', prEvent) - const result = await run(gh, 'issue_comment', commentEvent(alice, PHRASE)) + const result = await comment(gh, alice, PHRASE) assert.equal(result.passed, true) assert.deepEqual(gh.state.file.data.signatures.map((s) => s.id), [alice.id]) assert.equal(gh.state.file.data.signatures[0].pull_request, 7) assert.equal(gh.state.statuses.at(-1).state, 'success') - assert.equal(gh.state.comments.length, 1, 'the existing comment is updated, not duplicated') - assert.match(gh.state.comments[0].body, /All committers have signed/) + const botComments = gh.state.comments.filter((c) => c.body.includes(COMMENT_MARKER)) + assert.equal(botComments.length, 1, 'the existing comment is updated, not duplicated') + assert.match(botComments[0].body, /All committers have signed/) }) test('a commenter who is not a commit author cannot sign on someone else\'s PR', async () => { const gh = fakeGitHub({ pr: openPr(), commits: [commitBy(alice)] }) - const result = await run(gh, 'issue_comment', commentEvent(bob, PHRASE)) + const result = await comment(gh, bob, PHRASE) assert.equal(result.passed, false) assert.equal(gh.state.file, null, 'nothing written') @@ -88,8 +98,8 @@ test('a commenter who is not a commit author cannot sign on someone else\'s PR', test('signing twice writes only once', async () => { const gh = fakeGitHub({ pr: openPr(), commits: [commitBy(alice)] }) - await run(gh, 'issue_comment', commentEvent(alice, PHRASE)) - await run(gh, 'issue_comment', commentEvent(alice, PHRASE)) + await comment(gh, alice, PHRASE) + await comment(gh, alice, PHRASE) assert.equal(gh.state.writes, 1) }) @@ -108,7 +118,12 @@ test('already-signed, org-member and bot authors pass without a comment', async }) test('a commit with no linked GitHub account keeps the check failing', async () => { - const gh = fakeGitHub({ pr: openPr(), commits: [commitBy(null)], signatures: { version: 1, signatures: [] } }) + const gh = fakeGitHub({ + pr: openPr(staff), + commits: [commitBy(null)], + members: ['staff'], + signatures: { version: 1, signatures: [] }, + }) const result = await run(gh, 'pull_request_target', prEvent) assert.equal(result.passed, false) assert.match(gh.state.statuses[0].description, /1 committer still needs/) @@ -122,7 +137,7 @@ test('agent-authored commits require the PR author to sign, not the agent', asyn assert.deepEqual(result.unsigned.map((u) => u.login), ['bob']) assert.doesNotMatch(gh.state.comments[0].body, /@claude/) - await run(gh, 'issue_comment', commentEvent(bob, PHRASE)) + await comment(gh, bob, PHRASE) assert.deepEqual(gh.state.file.data.signatures.map((s) => s.login), ['bob']) assert.equal(gh.state.statuses.at(-1).state, 'success') }) @@ -133,31 +148,89 @@ test('agent-authored commits by an org member pass through the member exemption' assert.equal(result.passed, true) }) -test('closed PRs and unrelated comments are ignored', async () => { +test('closed PRs are ignored', async () => { const gh = fakeGitHub({ pr: { ...openPr(), state: 'closed' }, commits: [commitBy(alice)] }) assert.equal(await run(gh, 'pull_request_target', prEvent), null) - assert.equal(await run(gh, 'issue_comment', commentEvent(alice, 'LGTM')), null) + assert.equal(await comment(gh, alice, PHRASE), null) assert.equal(gh.state.statuses.length, 0) + assert.equal(gh.state.file, null) +}) + +test('a sign comment whose own run was cancelled is recorded by the next run', async () => { + // The concurrency group cancels a pending run when another is queued, so the + // sign comment's run may never execute. Any later run must pick it up. + const gh = fakeGitHub({ pr: openPr(), commits: [commitBy(alice)] }) + gh.state.comments.push({ id: 500, user: alice, body: PHRASE, html_url: 'https://github.com/c/500' }) + const result = await comment(gh, bob, 'LGTM') + + assert.equal(result.passed, true) + assert.deepEqual(gh.state.file.data.signatures.map((s) => s.id), [alice.id]) + assert.equal(gh.state.file.data.signatures[0].comment_url, 'https://github.com/c/500') + assert.equal(gh.state.statuses.at(-1).state, 'success') +}) + +test('the PR author must sign even when every commit is attributed to an exempt account', async () => { + // Commit authors are matched by email, which the committer chooses. + const botAccount = { id: 41898282, login: 'github-actions[bot]', type: 'Bot' } + const gh = fakeGitHub({ + pr: openPr(bob), + commits: [commitBy(botAccount), commitBy(staff)], + members: ['staff'], + signatures: { version: 1, signatures: [] }, + }) + const result = await run(gh, 'pull_request_target', prEvent) + + assert.equal(result.passed, false) + assert.deepEqual(result.unsigned.map((u) => u.login), ['bob']) + + await comment(gh, bob, PHRASE) + assert.deepEqual(gh.state.file.data.signatures.map((s) => s.login), ['bob']) + assert.equal(gh.state.statuses.at(-1).state, 'success') +}) + +test('a PR with more commits than GitHub lists fails instead of passing on a partial list', async () => { + const gh = fakeGitHub({ + pr: { ...openPr(), commits: 300 }, + commits: [commitBy(alice)], + signatures: { version: 1, signatures: [{ id: alice.id, login: 'alice' }] }, + }) + const result = await run(gh, 'pull_request_target', prEvent) + + assert.equal(result.passed, false) + assert.equal(gh.state.statuses[0].state, 'failure') + assert.match(gh.state.statuses[0].description, /300 commits/) + assert.match(gh.state.comments[0].body, /more commits than GitHub lists/) }) test('comments on plain issues are ignored', () => { const event = { action: 'created', issue: { number: 1 }, comment: { body: PHRASE } } - assert.equal(resolveTrigger('issue_comment', event, PHRASE), null) + assert.equal(resolveTrigger('issue_comment', event), null) }) test('edited comments do not sign', () => { const event = { ...commentEvent(alice, PHRASE), action: 'edited' } - assert.equal(resolveTrigger('issue_comment', event, PHRASE), null) + assert.equal(resolveTrigger('issue_comment', event), null) }) -test('a concurrent write is retried against the latest file', async () => { +test('a concurrent write is retried against the latest file, keeping the other signatures', async () => { const gh = fakeGitHub({ signatures: { version: 1, signatures: [] }, conflictsBeforeWrite: 2 }) const location = { repository: 'Zipstack/cla-signatures', path: 'signatures.json', branch: 'main' } const data = await addSignature(gh, location, { id: 1, login: 'alice' }) - assert.deepEqual(data.signatures.map((s) => s.id), [1]) + assert.deepEqual(data.signatures.map((s) => s.id), [9001, 9000, 1]) + assert.deepEqual(gh.state.file.data.signatures.map((s) => s.id), [9001, 9000, 1]) assert.equal(gh.state.writes, 1) }) +test('two first signatures at once: the one that finds the file already created retries', async () => { + // Both runs saw no file; the other creates it first, so this write has no + // sha and GitHub answers 422. + const gh = fakeGitHub({ conflictsBeforeWrite: 1 }) + const location = { repository: 'Zipstack/cla-signatures', path: 'signatures.json', branch: 'main' } + const data = await addSignature(gh, location, { id: 1, login: 'alice' }) + assert.deepEqual(data.signatures.map((s) => s.id), [9000, 1]) + assert.deepEqual(gh.state.file.data.signatures.map((s) => s.id), [9000, 1]) +}) + test('a signature recorded by a concurrent run is not written again', async () => { const gh = fakeGitHub({ signatures: { version: 1, signatures: [{ id: 1, login: 'alice' }] } }) const location = { repository: 'Zipstack/cla-signatures', path: 'signatures.json', branch: 'main' } From f336521204be7d48b732c04b1f81951e0c64db58 Mon Sep 17 00:00:00 2001 From: Ritwik G Date: Tue, 27 Oct 2026 12:29:07 +0530 Subject: [PATCH 4/8] fix: ignore edited sign comments and date signatures by the comment Self-review round 2: - Reading every PR comment let a comment edited into the sign phrase record a signature, and anyone with write access can edit others' comments. Edited comments no longer sign; the test now drives that through a later run instead of only checking resolveTrigger. - signed_at is the sign comment's created_at rather than the time of whichever run recorded it. - The fake PR reports its commit count as GitHub does, so the truncation guard is exercised at the boundary. - README known limitations: drop the "each one fails closed" framing, which the exempt-org-members item contradicts; note that a PR opened by an agent-logins account can't pass and that edited comments don't sign; correct which write is retried. Co-Authored-By: Claude Opus 5.5 --- README.md | 8 ++++---- src/main.js | 9 ++++++--- test/fake-github.js | 7 ++++++- test/main.test.js | 41 ++++++++++++++++++++++++++++++++++++----- 4 files changed, 52 insertions(+), 13 deletions(-) diff --git a/README.md b/README.md index 52e1158..556e4f8 100644 --- a/README.md +++ b/README.md @@ -85,15 +85,15 @@ When Claude Code is itself the commit author (for example in a Claude Code cloud ## Known limitations -Accepted for now; each one either fails closed or is a policy choice still open. - -- **A run that errors posts no status.** The required check then stays pending, and the error is only in the workflow run. API calls have no timeout or retry beyond the signatures write. +- **A run that errors posts no status.** The required check then stays pending, and the error is only in the workflow run. API calls have no timeout or retry; only a signatures write that loses a race with another write is retried. - **A missing signatures file reads as "nobody has signed yet",** and an organization the token can't see reads as "not a member". A misconfigured repository, path, branch or organization therefore asks everyone to sign rather than failing with a configuration error. - **A sign comment that is not accepted gets no reply on the PR** (for example from someone who is neither a commit author nor the PR author). The reason is only in the workflow log. - **A signature is not tied to a CLA version.** Any signature in the file counts for every repository and every `cla-document-url`. Records from the importer carry `cla_version`; records from the action don't. - **The PR comment is found by its marker alone**, whoever wrote it, and an unlinked commit's author name is shown as the committer wrote it. - **The signatures file must stay under 1 MB** (a few thousand signatures): above that the contents API stops returning the file and every run fails. -- **`exempt-org-members` is off only for the exact value `false`.** +- **`exempt-org-members` is off only for the exact value `false`.** Any other value, including `False` or `no`, leaves org members exempt. +- **A PR opened by an account in `agent-logins` can't pass**, because that account would have to sign as the PR author and can't. Open the PR from a person's account, or from a GitHub App (`[bot]` accounts are exempt). +- **An edited comment never signs**, even when its author edited it into the phrase; they post the phrase again as a new comment. - **The status description counts unlinked commits** as committers, one per commit. - **The file's `version` field is not checked** when it is read. diff --git a/src/main.js b/src/main.js index 5230dce..33d06bf 100644 --- a/src/main.js +++ b/src/main.js @@ -70,14 +70,17 @@ async function handle({ eventName, event, repository, inputs, gh, signaturesGh, // Signatures come from every sign comment on the PR, not only the one that // triggered this run (see resolveTrigger). Only a commit author or the PR - // author can sign; the earliest sign comment per person is recorded. + // author can sign; the earliest sign comment per person is recorded. An + // edited comment never signs: people with write access can edit anyone's + // comment, so its text may not be what its author wrote. const comments = await gh.paginate(`/repos/${repository}/issues/${pr.number}/comments`) let { data } = await loadSignatures(signaturesGh, location) const required = new Set(users.map((user) => user.id)) const handled = new Set() for (const comment of comments) { const signer = comment.user - if (!signer || handled.has(signer.id) || !isSignComment(comment.body, inputs.signPhrase)) continue + if (!signer || handled.has(signer.id) || comment.updated_at !== comment.created_at) continue + if (!isSignComment(comment.body, inputs.signPhrase)) continue handled.add(signer.id) if (signedIds(data).has(signer.id)) continue if (!required.has(signer.id)) { @@ -87,7 +90,7 @@ async function handle({ eventName, event, repository, inputs, gh, signaturesGh, data = await addSignature(signaturesGh, location, { id: signer.id, login: signer.login, - signed_at: new Date().toISOString(), + signed_at: comment.created_at, cla_document: inputs.claDocumentUrl, repository, pull_request: pr.number, diff --git a/test/fake-github.js b/test/fake-github.js index ca66a1c..aa68675 100644 --- a/test/fake-github.js +++ b/test/fake-github.js @@ -26,7 +26,9 @@ function fakeGitHub({ pr, commits = [], comments = [], members = [], signatures, async function request(method, path, body) { let match if (method === 'GET' && (match = path.match(/^\/repos\/[^/]+\/[^/]+\/pulls\/(\d+)$/))) { - return state.pr + // GitHub reports the PR's full commit count, which the commits list + // can fall short of. + return { commits: state.commits.length, ...state.pr } } if (method === 'GET' && path.startsWith('/repos/Zipstack/cla-signatures/contents/')) { if (!state.file) throw new GitHubError(404, method, path, 'Not Found') @@ -57,8 +59,11 @@ function fakeGitHub({ pr, commits = [], comments = [], members = [], signatures, throw new GitHubError(404, method, path, 'Not Found') } if (method === 'POST' && path.match(/\/issues\/\d+\/comments$/)) { + const now = new Date().toISOString() const comment = { id: nextId++, + created_at: now, + updated_at: now, user: { id: 41898282, login: 'github-actions[bot]', type: 'Bot' }, body: body.body, html_url: `https://github.com/comment/${nextId}`, diff --git a/test/main.test.js b/test/main.test.js index 275b17c..eb3cbc8 100644 --- a/test/main.test.js +++ b/test/main.test.js @@ -55,9 +55,21 @@ function commentEvent(user, body) { // Posts a comment on the PR and runs the action for it, as GitHub would. let nextCommentId = 1 -function comment(gh, user, body) { +function postComment(gh, user, body, { editedAt } = {}) { const id = nextCommentId++ - gh.state.comments.push({ id, user, body, html_url: `https://github.com/Zipstack/unstract/pull/7#issuecomment-${id}` }) + const createdAt = new Date(Date.UTC(2026, 9, 1, 0, id)).toISOString() + gh.state.comments.push({ + id, + user, + body, + created_at: createdAt, + updated_at: editedAt || createdAt, + html_url: `https://github.com/Zipstack/unstract/pull/7#issuecomment-${id}`, + }) + return gh.state.comments.at(-1) +} +function comment(gh, user, body) { + postComment(gh, user, body) return run(gh, 'issue_comment', commentEvent(user, body)) } @@ -160,12 +172,13 @@ test('a sign comment whose own run was cancelled is recorded by the next run', a // The concurrency group cancels a pending run when another is queued, so the // sign comment's run may never execute. Any later run must pick it up. const gh = fakeGitHub({ pr: openPr(), commits: [commitBy(alice)] }) - gh.state.comments.push({ id: 500, user: alice, body: PHRASE, html_url: 'https://github.com/c/500' }) + const signed = postComment(gh, alice, PHRASE) const result = await comment(gh, bob, 'LGTM') assert.equal(result.passed, true) assert.deepEqual(gh.state.file.data.signatures.map((s) => s.id), [alice.id]) - assert.equal(gh.state.file.data.signatures[0].comment_url, 'https://github.com/c/500') + assert.equal(gh.state.file.data.signatures[0].comment_url, signed.html_url) + assert.equal(gh.state.file.data.signatures[0].signed_at, signed.created_at, 'signed when the comment was posted') assert.equal(gh.state.statuses.at(-1).state, 'success') }) @@ -207,9 +220,27 @@ test('comments on plain issues are ignored', () => { assert.equal(resolveTrigger('issue_comment', event), null) }) -test('edited comments do not sign', () => { +test('edited comments do not sign', async () => { const event = { ...commentEvent(alice, PHRASE), action: 'edited' } assert.equal(resolveTrigger('issue_comment', event), null) + + // Nor does a later run pick up a comment that was edited into the phrase. + const gh = fakeGitHub({ pr: openPr(), commits: [commitBy(alice)] }) + postComment(gh, alice, PHRASE, { editedAt: '2026-10-20T00:00:00.000Z' }) + const result = await comment(gh, bob, 'LGTM') + assert.equal(result.passed, false) + assert.equal(gh.state.file, null, 'nothing written') +}) + +test('a PR whose commit count matches the listed commits is not treated as truncated', async () => { + const gh = fakeGitHub({ + pr: { ...openPr(), commits: 2 }, + commits: [commitBy(alice, '1'.repeat(40)), commitBy(alice, '2'.repeat(40))], + signatures: { version: 1, signatures: [{ id: alice.id, login: 'alice' }] }, + }) + const result = await run(gh, 'pull_request_target', prEvent) + assert.equal(result.truncated, false) + assert.equal(result.passed, true) }) test('a concurrent write is retried against the latest file, keeping the other signatures', async () => { From a253251b7ab877e4f050364a710ad0debf7a21a5 Mon Sep 17 00:00:00 2001 From: Ritwik G Date: Tue, 27 Oct 2026 12:37:25 +0530 Subject: [PATCH 5/8] fix: log ignored edited sign comments and keep re-signing possible Self-review round 3: - An edited (or hidden) sign comment is now logged when skipped, and does not use up the signer's slot, so posting the phrase again signs. Tested, including the reordering that would lock a signer out. - signed_at is normalised with toISOString, so records match the importer's format; the test fixture now uses GitHub's timestamp format. - Code comment and README say hidden comments are skipped too, which the updated_at check implies. - README: the agent-logins PR limitation no longer claims the code stops an agent account from signing; the retry note says which responses are retried. Co-Authored-By: Claude Opus 5.5 --- README.md | 6 +++--- src/main.js | 13 +++++++++---- test/main.test.js | 16 ++++++++++++++-- 3 files changed, 26 insertions(+), 9 deletions(-) diff --git a/README.md b/README.md index 556e4f8..7e07117 100644 --- a/README.md +++ b/README.md @@ -85,15 +85,15 @@ When Claude Code is itself the commit author (for example in a Claude Code cloud ## Known limitations -- **A run that errors posts no status.** The required check then stays pending, and the error is only in the workflow run. API calls have no timeout or retry; only a signatures write that loses a race with another write is retried. +- **A run that errors posts no status.** The required check then stays pending, and the error is only in the workflow run. API calls have no timeout or retry; only the signatures write is retried, and only on a 409 or 422 response, up to 5 attempts. - **A missing signatures file reads as "nobody has signed yet",** and an organization the token can't see reads as "not a member". A misconfigured repository, path, branch or organization therefore asks everyone to sign rather than failing with a configuration error. - **A sign comment that is not accepted gets no reply on the PR** (for example from someone who is neither a commit author nor the PR author). The reason is only in the workflow log. - **A signature is not tied to a CLA version.** Any signature in the file counts for every repository and every `cla-document-url`. Records from the importer carry `cla_version`; records from the action don't. - **The PR comment is found by its marker alone**, whoever wrote it, and an unlinked commit's author name is shown as the committer wrote it. - **The signatures file must stay under 1 MB** (a few thousand signatures): above that the contents API stops returning the file and every run fails. - **`exempt-org-members` is off only for the exact value `false`.** Any other value, including `False` or `no`, leaves org members exempt. -- **A PR opened by an account in `agent-logins` can't pass**, because that account would have to sign as the PR author and can't. Open the PR from a person's account, or from a GitHub App (`[bot]` accounts are exempt). -- **An edited comment never signs**, even when its author edited it into the phrase; they post the phrase again as a new comment. +- **A PR opened by an account in `agent-logins` fails the check** unless that account itself signs, because it counts as the PR author. The action does not stop an agent account from signing. Open the PR from a person's account, or from a GitHub App (`[bot]` accounts are exempt). +- **An edited or hidden comment never signs**, even when its author edited it into the phrase or a maintainer only hid it; the person posts the phrase again as a new comment. The workflow log says which comment was ignored. - **The status description counts unlinked commits** as committers, one per commit. - **The file's `version` field is not checked** when it is read. diff --git a/src/main.js b/src/main.js index 33d06bf..900cc80 100644 --- a/src/main.js +++ b/src/main.js @@ -72,15 +72,20 @@ async function handle({ eventName, event, repository, inputs, gh, signaturesGh, // triggered this run (see resolveTrigger). Only a commit author or the PR // author can sign; the earliest sign comment per person is recorded. An // edited comment never signs: people with write access can edit anyone's - // comment, so its text may not be what its author wrote. + // comment, so its text may not be what its author wrote. The check compares + // updated_at with created_at, so a hidden (minimized) comment is skipped too. const comments = await gh.paginate(`/repos/${repository}/issues/${pr.number}/comments`) let { data } = await loadSignatures(signaturesGh, location) const required = new Set(users.map((user) => user.id)) const handled = new Set() for (const comment of comments) { const signer = comment.user - if (!signer || handled.has(signer.id) || comment.updated_at !== comment.created_at) continue - if (!isSignComment(comment.body, inputs.signPhrase)) continue + if (!signer || handled.has(signer.id) || !isSignComment(comment.body, inputs.signPhrase)) continue + if (comment.updated_at !== comment.created_at) { + // Not marked handled, so a later unedited sign comment still counts. + log(`Ignoring edited or hidden sign comment ${comment.html_url} from @${signer.login}; post the phrase again as a new comment.`) + continue + } handled.add(signer.id) if (signedIds(data).has(signer.id)) continue if (!required.has(signer.id)) { @@ -90,7 +95,7 @@ async function handle({ eventName, event, repository, inputs, gh, signaturesGh, data = await addSignature(signaturesGh, location, { id: signer.id, login: signer.login, - signed_at: comment.created_at, + signed_at: new Date(comment.created_at).toISOString(), cla_document: inputs.claDocumentUrl, repository, pull_request: pr.number, diff --git a/test/main.test.js b/test/main.test.js index eb3cbc8..a08c044 100644 --- a/test/main.test.js +++ b/test/main.test.js @@ -57,7 +57,8 @@ function commentEvent(user, body) { let nextCommentId = 1 function postComment(gh, user, body, { editedAt } = {}) { const id = nextCommentId++ - const createdAt = new Date(Date.UTC(2026, 9, 1, 0, id)).toISOString() + // GitHub's timestamps have no milliseconds. + const createdAt = new Date(Date.UTC(2026, 9, 1, 0, id)).toISOString().replace('.000Z', 'Z') gh.state.comments.push({ id, user, @@ -178,7 +179,11 @@ test('a sign comment whose own run was cancelled is recorded by the next run', a assert.equal(result.passed, true) assert.deepEqual(gh.state.file.data.signatures.map((s) => s.id), [alice.id]) assert.equal(gh.state.file.data.signatures[0].comment_url, signed.html_url) - assert.equal(gh.state.file.data.signatures[0].signed_at, signed.created_at, 'signed when the comment was posted') + assert.equal( + gh.state.file.data.signatures[0].signed_at, + new Date(signed.created_at).toISOString(), + 'signed when the comment was posted', + ) assert.equal(gh.state.statuses.at(-1).state, 'success') }) @@ -230,6 +235,13 @@ test('edited comments do not sign', async () => { const result = await comment(gh, bob, 'LGTM') assert.equal(result.passed, false) assert.equal(gh.state.file, null, 'nothing written') + + // Posting the phrase again as a new comment signs. + const fresh = postComment(gh, alice, PHRASE) + const retry = await comment(gh, bob, 'thanks') + assert.equal(retry.passed, true) + assert.deepEqual(gh.state.file.data.signatures.map((s) => s.id), [alice.id]) + assert.equal(gh.state.file.data.signatures[0].signed_at, new Date(fresh.created_at).toISOString()) }) test('a PR whose commit count matches the listed commits is not treated as truncated', async () => { From 01097588391c54720d02392161c6958d8aaf1c7a Mon Sep 17 00:00:00 2001 From: Ritwik G Date: Tue, 27 Oct 2026 12:43:41 +0530 Subject: [PATCH 6/8] fix: only advise re-signing when it would help Self-review round 4: - The signature loop checks "already signed" and "may sign" before the edited-comment check, so the ignored-comment log no longer tells someone who has signed, or cannot sign, to post the phrase again. The log line is now asserted, including that it stops once the person has signed. - README: hiding or editing a comment after its signature was recorded does not remove the signature; an exempt agent account passes as PR author. - Edited-comment fixture uses GitHub's timestamp format. Co-Authored-By: Claude Opus 5.5 --- README.md | 4 ++-- src/main.js | 14 +++++++++----- test/main.test.js | 16 ++++++++++++---- 3 files changed, 23 insertions(+), 11 deletions(-) diff --git a/README.md b/README.md index 7e07117..a3d6ad4 100644 --- a/README.md +++ b/README.md @@ -92,8 +92,8 @@ When Claude Code is itself the commit author (for example in a Claude Code cloud - **The PR comment is found by its marker alone**, whoever wrote it, and an unlinked commit's author name is shown as the committer wrote it. - **The signatures file must stay under 1 MB** (a few thousand signatures): above that the contents API stops returning the file and every run fails. - **`exempt-org-members` is off only for the exact value `false`.** Any other value, including `False` or `no`, leaves org members exempt. -- **A PR opened by an account in `agent-logins` fails the check** unless that account itself signs, because it counts as the PR author. The action does not stop an agent account from signing. Open the PR from a person's account, or from a GitHub App (`[bot]` accounts are exempt). -- **An edited or hidden comment never signs**, even when its author edited it into the phrase or a maintainer only hid it; the person posts the phrase again as a new comment. The workflow log says which comment was ignored. +- **A PR opened by an account in `agent-logins` fails the check** unless that account itself signs or is exempt (an org member, or on the allowlist), because it counts as the PR author. The action does not stop an agent account from signing. Open the PR from a person's account, or from a GitHub App (`[bot]` accounts are exempt). +- **An edited or hidden comment never signs**, even when its author edited it into the phrase or a maintainer only hid it; the person posts the phrase again as a new comment. The workflow log says which comment was ignored. This is checked when the signature is recorded, normally seconds after the comment is posted: editing or hiding a comment afterwards does not remove a signature already recorded, and nothing in the action does. - **The status description counts unlinked commits** as committers, one per commit. - **The file's `version` field is not checked** when it is read. diff --git a/src/main.js b/src/main.js index 900cc80..b15623c 100644 --- a/src/main.js +++ b/src/main.js @@ -81,17 +81,21 @@ async function handle({ eventName, event, repository, inputs, gh, signaturesGh, for (const comment of comments) { const signer = comment.user if (!signer || handled.has(signer.id) || !isSignComment(comment.body, inputs.signPhrase)) continue - if (comment.updated_at !== comment.created_at) { - // Not marked handled, so a later unedited sign comment still counts. - log(`Ignoring edited or hidden sign comment ${comment.html_url} from @${signer.login}; post the phrase again as a new comment.`) + if (signedIds(data).has(signer.id)) { + handled.add(signer.id) continue } - handled.add(signer.id) - if (signedIds(data).has(signer.id)) continue if (!required.has(signer.id)) { + handled.add(signer.id) log(`@${signer.login} is not a commit author or the author of PR #${pr.number}; not recording a signature.`) continue } + if (comment.updated_at !== comment.created_at) { + // Not marked handled, so a later unedited sign comment still counts. + log(`Ignoring edited or hidden sign comment ${comment.html_url} from @${signer.login}; post the phrase again as a new comment.`) + continue + } + handled.add(signer.id) data = await addSignature(signaturesGh, location, { id: signer.id, login: signer.login, diff --git a/test/main.test.js b/test/main.test.js index a08c044..9ce1f40 100644 --- a/test/main.test.js +++ b/test/main.test.js @@ -32,7 +32,7 @@ function openPr(author = alice) { const claudeAgent = { id: 81847, login: 'claude', type: 'User' } -function run(gh, eventName, event) { +function run(gh, eventName, event, log = () => {}) { return handle({ eventName, event, @@ -40,7 +40,7 @@ function run(gh, eventName, event) { inputs, gh, signaturesGh: gh, - log: () => {}, + log, }) } @@ -231,10 +231,13 @@ test('edited comments do not sign', async () => { // Nor does a later run pick up a comment that was edited into the phrase. const gh = fakeGitHub({ pr: openPr(), commits: [commitBy(alice)] }) - postComment(gh, alice, PHRASE, { editedAt: '2026-10-20T00:00:00.000Z' }) - const result = await comment(gh, bob, 'LGTM') + const edited = postComment(gh, alice, PHRASE, { editedAt: '2026-10-20T00:00:00Z' }) + postComment(gh, bob, 'LGTM') + const logs = [] + const result = await run(gh, 'issue_comment', commentEvent(bob, 'LGTM'), (line) => logs.push(line)) assert.equal(result.passed, false) assert.equal(gh.state.file, null, 'nothing written') + assert.ok(logs.some((line) => line.includes(edited.html_url) && line.includes('@alice')), 'the log names the ignored comment') // Posting the phrase again as a new comment signs. const fresh = postComment(gh, alice, PHRASE) @@ -242,6 +245,11 @@ test('edited comments do not sign', async () => { assert.equal(retry.passed, true) assert.deepEqual(gh.state.file.data.signatures.map((s) => s.id), [alice.id]) assert.equal(gh.state.file.data.signatures[0].signed_at, new Date(fresh.created_at).toISOString()) + + // Once signed, the old edited comment no longer tells them to post again. + const later = [] + await run(gh, 'issue_comment', commentEvent(bob, 'ok'), (line) => later.push(line)) + assert.ok(!later.some((line) => line.includes(edited.html_url)), 'no repost advice after signing') }) test('a PR whose commit count matches the listed commits is not treated as truncated', async () => { From 816742149eeb42b64ff113e071edf718b717fba8 Mon Sep 17 00:00:00 2001 From: Ritwik G Date: Tue, 27 Oct 2026 12:48:18 +0530 Subject: [PATCH 7/8] fix: tidy the ignored-comment advice and the agent-logins note Self-review round 5: - Test that someone who may not sign is told so, rather than told to post the phrase again, which pins the order of the checks in the signature loop. - The ignored-comment log advises posting again only "if a signature is still needed", since exempt users reach it too. - README no longer suggests the allowlist for an agent account, which the coding-agents section warns against. Co-Authored-By: Claude Opus 5.5 --- README.md | 2 +- src/main.js | 2 +- test/main.test.js | 7 +++++++ 3 files changed, 9 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index a3d6ad4..f11bcf0 100644 --- a/README.md +++ b/README.md @@ -92,7 +92,7 @@ When Claude Code is itself the commit author (for example in a Claude Code cloud - **The PR comment is found by its marker alone**, whoever wrote it, and an unlinked commit's author name is shown as the committer wrote it. - **The signatures file must stay under 1 MB** (a few thousand signatures): above that the contents API stops returning the file and every run fails. - **`exempt-org-members` is off only for the exact value `false`.** Any other value, including `False` or `no`, leaves org members exempt. -- **A PR opened by an account in `agent-logins` fails the check** unless that account itself signs or is exempt (an org member, or on the allowlist), because it counts as the PR author. The action does not stop an agent account from signing. Open the PR from a person's account, or from a GitHub App (`[bot]` accounts are exempt). +- **A PR opened by an account in `agent-logins` fails the check** unless that account itself signs or is an org member, because it counts as the PR author. The action does not stop an agent account from signing. Open the PR from a person's account, or from a GitHub App (`[bot]` accounts are exempt). - **An edited or hidden comment never signs**, even when its author edited it into the phrase or a maintainer only hid it; the person posts the phrase again as a new comment. The workflow log says which comment was ignored. This is checked when the signature is recorded, normally seconds after the comment is posted: editing or hiding a comment afterwards does not remove a signature already recorded, and nothing in the action does. - **The status description counts unlinked commits** as committers, one per commit. - **The file's `version` field is not checked** when it is read. diff --git a/src/main.js b/src/main.js index b15623c..6b02138 100644 --- a/src/main.js +++ b/src/main.js @@ -92,7 +92,7 @@ async function handle({ eventName, event, repository, inputs, gh, signaturesGh, } if (comment.updated_at !== comment.created_at) { // Not marked handled, so a later unedited sign comment still counts. - log(`Ignoring edited or hidden sign comment ${comment.html_url} from @${signer.login}; post the phrase again as a new comment.`) + log(`Ignoring edited or hidden sign comment ${comment.html_url} from @${signer.login}; if a signature is still needed, post the phrase again as a new comment.`) continue } handled.add(signer.id) diff --git a/test/main.test.js b/test/main.test.js index 9ce1f40..80f2273 100644 --- a/test/main.test.js +++ b/test/main.test.js @@ -246,6 +246,13 @@ test('edited comments do not sign', async () => { assert.deepEqual(gh.state.file.data.signatures.map((s) => s.id), [alice.id]) assert.equal(gh.state.file.data.signatures[0].signed_at, new Date(fresh.created_at).toISOString()) + // Someone who may not sign is told so, not told to post again. + const outsider = postComment(gh, staff, PHRASE, { editedAt: '2026-10-20T00:00:00Z' }) + const outsiderLogs = [] + await run(gh, 'issue_comment', commentEvent(bob, 'hm'), (line) => outsiderLogs.push(line)) + assert.ok(outsiderLogs.some((line) => line.includes('@staff is not a commit author'))) + assert.ok(!outsiderLogs.some((line) => line.includes(outsider.html_url))) + // Once signed, the old edited comment no longer tells them to post again. const later = [] await run(gh, 'issue_comment', commentEvent(bob, 'ok'), (line) => later.push(line)) From 5a5ded5f7fd1a7bb356ffe5763cb58f2d7fd8c3a Mon Sep 17 00:00:00 2001 From: Ritwik G Date: Tue, 27 Oct 2026 12:53:09 +0530 Subject: [PATCH 8/8] docs: point the agent-logins note at the real exemption rules Self-review round 6: the note listed only org membership, which is wrong for an allowlisted agent account and for exempt-org-members: false. It now refers to "How it works" step 2 instead of restating the rules. Co-Authored-By: Claude Opus 5.5 --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index f11bcf0..3586d82 100644 --- a/README.md +++ b/README.md @@ -92,7 +92,7 @@ When Claude Code is itself the commit author (for example in a Claude Code cloud - **The PR comment is found by its marker alone**, whoever wrote it, and an unlinked commit's author name is shown as the committer wrote it. - **The signatures file must stay under 1 MB** (a few thousand signatures): above that the contents API stops returning the file and every run fails. - **`exempt-org-members` is off only for the exact value `false`.** Any other value, including `False` or `no`, leaves org members exempt. -- **A PR opened by an account in `agent-logins` fails the check** unless that account itself signs or is an org member, because it counts as the PR author. The action does not stop an agent account from signing. Open the PR from a person's account, or from a GitHub App (`[bot]` accounts are exempt). +- **A PR opened by an account in `agent-logins` fails the check** unless that account itself signs or is exempt (see step 2 of *How it works*), because it counts as the PR author. The action does not stop an agent account from signing. Open the PR from a person's account, or from a GitHub App (`[bot]` accounts are exempt). - **An edited or hidden comment never signs**, even when its author edited it into the phrase or a maintainer only hid it; the person posts the phrase again as a new comment. The workflow log says which comment was ignored. This is checked when the signature is recorded, normally seconds after the comment is posted: editing or hiding a comment afterwards does not remove a signature already recorded, and nothing in the action does. - **The status description counts unlinked commits** as committers, one per commit. - **The file's `version` field is not checked** when it is read.