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 });