From f6309444c3692ef031ac591e5801e6dd407c31da Mon Sep 17 00:00:00 2001 From: George Larson Date: Fri, 28 Aug 2026 22:34:24 +0000 Subject: [PATCH] fix(ci): stop the endpoint audit failing every fork pull request A pull_request event raised from a fork runs with a read-only GITHUB_TOKEN, so the workflow's `pull-requests: write` grant cannot be honoured and the report's comment POST returns 403 "Resource not accessible by integration". The step is `if: always()`, so that 403 failed the whole endpoint-audit check on every fork PR regardless of what the audit itself found. Two PRs are sitting on it right now: #320, approved and 27 days old, and #362, where the audit ran clean (0 actionable client-only calls) and the job still went red on the comment step. The report still has to reach a reviewer, so it now always goes to the job summary, and the comment is attempted only when the run's token can write one. That also covers push events, which have no PR to comment on at all. Verified against the report this repo actually produces: driving the real .audit/endpoint-audit.json through a fork pull_request context throws "Resource not accessible by integration" on the old script and completes with the summary written on the new one. Tooling tests cover same-repo, fork, push and missing-report; audit:endpoints, format:check green. --- .../scripts/post-endpoint-audit-report.cjs | 31 ++++- .github/workflows/endpoint-audit.yml | 18 ++- scripts/test-endpoint-audit.mjs | 115 +++++++++++++++++- 3 files changed, 156 insertions(+), 8 deletions(-) diff --git a/.github/scripts/post-endpoint-audit-report.cjs b/.github/scripts/post-endpoint-audit-report.cjs index 4119cb9..bed7934 100644 --- a/.github/scripts/post-endpoint-audit-report.cjs +++ b/.github/scripts/post-endpoint-audit-report.cjs @@ -95,6 +95,20 @@ function renderEndpointAuditReport(report) { ].join('\n'); } +// A pull_request event raised from a fork runs with a read-only GITHUB_TOKEN, +// so `pull-requests: write` in the workflow cannot be granted and the comment +// POST comes back 403 "Resource not accessible by integration". The step is +// `if: always()`, so that 403 failed the whole endpoint-audit check on every +// fork PR regardless of what the audit found: #320 sat on it for 27 days and +// #362 hit the identical wall. Push events have no PR to comment on at all +// (context.issue.number is undefined), and the same guard covers them. +function canCommentOnPullRequest(context) { + if (!context || context.eventName !== 'pull_request') return false; + const head = context.payload?.pull_request?.head; + if (!head?.repo?.full_name) return false; + return head.repo.full_name === `${context.repo.owner}/${context.repo.repo}`; +} + async function postEndpointAuditReport({ github, context, core, reportPath = REPORT_PATH }) { if (!fs.existsSync(reportPath)) { core.warning(`No audit report at ${reportPath}; skipping PR comment.`); @@ -102,6 +116,21 @@ async function postEndpointAuditReport({ github, context, core, reportPath = REP } const body = renderEndpointAuditReport(JSON.parse(fs.readFileSync(reportPath, 'utf8'))); + + // The job summary is the one surface every run can write, so the report + // reaches a reviewer even when the comment is unavailable. + if (core?.summary) { + await core.summary.addRaw(body).write(); + } + + if (!canCommentOnPullRequest(context)) { + core.info( + 'Skipping the PR comment: this run is a push or a fork pull_request, ' + + 'whose token cannot write comments. The report is in the job summary.' + ); + return; + } + const { owner, repo } = context.repo; const issue_number = context.issue.number; const comments = await github.paginate(github.rest.issues.listComments, { @@ -118,4 +147,4 @@ async function postEndpointAuditReport({ github, context, core, reportPath = REP } } -module.exports = { postEndpointAuditReport, renderEndpointAuditReport }; +module.exports = { canCommentOnPullRequest, postEndpointAuditReport, renderEndpointAuditReport }; diff --git a/.github/workflows/endpoint-audit.yml b/.github/workflows/endpoint-audit.yml index 19ad66d..90834ae 100644 --- a/.github/workflows/endpoint-audit.yml +++ b/.github/workflows/endpoint-audit.yml @@ -47,11 +47,19 @@ jobs: # .audit/ is a dot-directory; upload-artifact@v4 skips hidden paths by default. include-hidden-files: true - # Post the report as a single, self-updating comment on the PR. Runs even - # when the audit gate fails (the report is written before the gate), so - # reviewers see the mismatches that failed the check. - - name: Comment audit report on PR - if: always() && github.event_name == 'pull_request' + # Publish the report. It always lands in the job summary, and additionally + # as a single self-updating PR comment when the run's token can write one + # (same-repo pull requests). Runs even when the audit gate fails (the + # report is written before the gate), so reviewers see the mismatches that + # failed the check. + # + # A fork pull_request runs with a read-only token, so the comment POST + # returns 403 "Resource not accessible by integration". This step is + # `if: always()`, so an unguarded POST failed the whole check on every + # fork PR whatever the audit found. The guard lives in + # post-endpoint-audit-report.cjs; see canCommentOnPullRequest. + - name: Publish audit report + if: always() uses: actions/github-script@v7 with: script: | diff --git a/scripts/test-endpoint-audit.mjs b/scripts/test-endpoint-audit.mjs index 05fb5df..b31324d 100644 --- a/scripts/test-endpoint-audit.mjs +++ b/scripts/test-endpoint-audit.mjs @@ -14,6 +14,50 @@ const { postEndpointAuditReport, renderEndpointAuditReport } = require( ); const fixtureRoot = fs.mkdtempSync(path.join(os.tmpdir(), 'endpoint-audit-')); +const summaryWrites = []; + +function fakeCore(overrides = {}) { + let buffer = ''; + return { + warning: assert.fail, + info: () => {}, + ...overrides, + summary: { + addRaw(text) { + buffer += text; + return this; + }, + async write() { + summaryWrites.push(buffer); + buffer = ''; + return this; + }, + }, + }; +} + +function sameRepoContext(number) { + return { + eventName: 'pull_request', + repo: { owner: 'OpenHands', repo: 'typescript-client' }, + issue: { number }, + payload: { + pull_request: { head: { repo: { full_name: 'OpenHands/typescript-client' } } }, + }, + }; +} + +function forkContext(number) { + return { + eventName: 'pull_request', + repo: { owner: 'OpenHands', repo: 'typescript-client' }, + issue: { number }, + payload: { + pull_request: { head: { repo: { full_name: 'georgeglarson/typescript-client' } } }, + }, + }; +} + try { fs.mkdirSync(path.join(fixtureRoot, 'src/client'), { recursive: true }); @@ -112,8 +156,8 @@ try { let updatedComment; await postEndpointAuditReport({ reportPath: path.join(fixtureRoot, '.audit/endpoint-audit.json'), - context: { repo: { owner: 'OpenHands', repo: 'typescript-client' }, issue: { number: 307 } }, - core: { warning: assert.fail }, + context: sameRepoContext(307), + core: fakeCore(), github: { paginate: async () => [{ id: 123, body: 'old' }], rest: { @@ -132,6 +176,73 @@ try { assert.equal(updatedComment.comment_id, 123); assert.equal(updatedComment.body, body); + assert(summaryWrites.length === 1, 'a same-repo PR still writes the job summary'); + assert(summaryWrites[0].includes('Endpoint audit'), 'the summary carries the report'); + + // A fork PR gets a read-only GITHUB_TOKEN, so the comment POST 403s and takes + // the whole job red. Every fork PR fails this check regardless of content: + // OpenHands/typescript-client#320 sat 27 days on it and #362 hit the same + // wall on 2026-08-28. The report still has to reach a reviewer, so it goes to + // the job summary and the comment is skipped rather than attempted. + summaryWrites.length = 0; + const forkInfo = []; + await postEndpointAuditReport({ + reportPath: path.join(fixtureRoot, '.audit/endpoint-audit.json'), + context: forkContext(362), + core: fakeCore({ info: (msg) => forkInfo.push(msg) }), + github: { + paginate: async () => assert.fail('a fork PR must not read comments'), + rest: { + issues: { + listComments: () => assert.fail('a fork PR must not read comments'), + updateComment: async () => assert.fail('a fork PR must not post a comment'), + createComment: async () => assert.fail('a fork PR must not post a comment'), + }, + }, + }, + }); + assert.equal(summaryWrites.length, 1, 'a fork PR still writes the job summary'); + assert(summaryWrites[0].includes('Endpoint audit'), 'the fork summary carries the report'); + assert( + forkInfo.some((msg) => msg.includes('fork')), + 'the skip is announced, not silent' + ); + + // A push event has no PR to comment on at all (context.issue.number is + // undefined there), so the same guard covers it. + summaryWrites.length = 0; + await postEndpointAuditReport({ + reportPath: path.join(fixtureRoot, '.audit/endpoint-audit.json'), + context: { eventName: 'push', repo: { owner: 'OpenHands', repo: 'typescript-client' } }, + core: fakeCore(), + github: { + paginate: async () => assert.fail('a push event must not read comments'), + rest: { + issues: { + listComments: () => assert.fail('a push event must not read comments'), + updateComment: async () => assert.fail('a push event must not post a comment'), + createComment: async () => assert.fail('a push event must not post a comment'), + }, + }, + }, + }); + assert.equal(summaryWrites.length, 1, 'a push run still writes the job summary'); + + // A missing report is still a warning with nothing published. + summaryWrites.length = 0; + const warnings = []; + await postEndpointAuditReport({ + reportPath: path.join(fixtureRoot, '.audit/does-not-exist.json'), + context: sameRepoContext(307), + core: fakeCore({ warning: (msg) => warnings.push(msg) }), + github: { + paginate: async () => assert.fail('a missing report must not read comments'), + rest: { issues: { listComments: () => assert.fail('no report, no comment') } }, + }, + }); + assert.equal(summaryWrites.length, 0, 'no report means no summary'); + assert.equal(warnings.length, 1, 'a missing report warns'); + console.log('endpoint-audit tooling test passed'); } finally { fs.rmSync(fixtureRoot, { recursive: true, force: true });