From 850961c5f71dfc102160af883fb3855a6a5e69bb Mon Sep 17 00:00:00 2001 From: Eric Larson Date: Tue, 28 Jul 2026 14:09:11 -0400 Subject: [PATCH 1/7] ENH: Prototype a GitHub App front end (gh-27) Extract the resolution logic into src/core.js and the option handling into src/config.js, then add a second front end: worker/index.js, a Cloudflare Worker that serves a GitHub App reacting to status webhooks directly. A repo using the App has no workflow, so it gets no workflow runs at all, which is the complaint in gh-27. The action keeps working exactly as before; index.js is now a thin wrapper over the same core, and its 22 tests pass unchanged. Config for the App lives in .github/circleci-artifacts.yml, read from the default branch so a forked PR cannot redirect the link it posts. Co-Authored-By: Claude Opus 5 --- .pre-commit-config.yaml | 2 +- README.md | 32 +++++ dist/index.js | 258 +++++++++++++++++++++++++--------------- index.js | 150 ++++------------------- index.test.js | 14 ++- src/config.js | 52 ++++++++ src/core.js | 120 +++++++++++++++++++ worker/index.js | 149 +++++++++++++++++++++++ worker/index.test.js | 218 +++++++++++++++++++++++++++++++++ wrangler.toml | 3 + 10 files changed, 773 insertions(+), 225 deletions(-) create mode 100644 src/config.js create mode 100644 src/core.js create mode 100644 worker/index.js create mode 100644 worker/index.test.js create mode 100644 wrangler.toml diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 6aab15a..c590020 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -12,7 +12,7 @@ repos: rev: 'v10.8.0' hooks: - id: eslint - files: ^index(\.test)?\.js$ + files: ^(index|src/.*|worker/.*)(\.test)?\.js$ additional_dependencies: - globals@17.8.0 - eslint@10.8.0 diff --git a/README.md b/README.md index 696c838..1feda2f 100644 --- a/README.md +++ b/README.md @@ -82,6 +82,38 @@ jobs: > (rather than app) API and that this is always tied to the `master`/default > branch of a given repository. +## GitHub App (prototype, not yet deployed) + +`worker/index.js` is a Cloudflare Worker that does the same job as the action, +but as a GitHub App reacting to `status` webhooks server-side. The point is +[#27](https://github.com/scientific-python/circleci-artifacts-redirector-action/issues/27): +with the App there is no workflow, so there are **no workflow runs at all** — +instead of one run per status event, most of which do nothing. + +Instead of a workflow file, a repo using the App has +`.github/circleci-artifacts.yml`, which is the `with:` block of the old +workflow with the indentation and `repo-token` removed: + +```yaml +artifact-path: 0/doc/index.html +circleci-jobs: build_docs +job-title: Check the rendered docs here! +``` + +The config is always read from the **default branch**, so a pull request +(including one from a fork) cannot change where the link points. + +Both front ends share `src/core.js` and `src/config.js`, so the two cannot +drift apart: the same resolution logic and the same option defaults serve both. + +Differences from the action, by design: + +- No `url` output, because there is no workflow step to consume it. +- Public CircleCI projects only: a private project needs an `api-token`, which + would mean storing each repo's CircleCI token server-side. + +The action is not going away; the App is a second way to run the same code. + ## Limitations Currently has (known) limitations: diff --git a/dist/index.js b/dist/index.js index 8765ff1..9178333 100644 --- a/dist/index.js +++ b/dist/index.js @@ -29149,12 +29149,7 @@ var __webpack_exports__ = {}; // EXPORTS __nccwpck_require__.d(__webpack_exports__, { - x6: () => (/* binding */ fetchJson), - O$: () => (/* binding */ legacyArtifactsUrl), - BH: () => (/* binding */ pickJob), - Qc: () => (/* binding */ redirectUrl), - eF: () => (/* binding */ run), - SR: () => (/* binding */ statusFor) + e: () => (/* binding */ run) }); ;// CONCATENATED MODULE: external "os" @@ -36420,19 +36415,66 @@ function github_getOctokit(token, options, ...additionalPlugins) { //# sourceMappingURL=github.js.map // EXTERNAL MODULE: external "node:url" var external_node_url_ = __nccwpck_require__(3136); -;// CONCATENATED MODULE: ./index.js -// This as annoying because CircleCI does not use the App API. -// Hence we must monitor statuses rather than using the more convenient -// "checks" API. -// -// After changing this file, use `ncc build index.js -o dist` to rebuild to dist/ - -// Refs: -// https://docs.github.com/en/developers/webhooks-and-events/webhooks/webhook-events-and-payloads#status - - - +;// CONCATENATED MODULE: ./src/config.js +// The options accepted by both front ends: the GitHub Action reads them from +// workflow inputs, the GitHub App from .github/circleci-artifacts.yml. Keep the +// defaults here in sync with action.yml (index.test.js checks that they match). + +const DEFAULT_JOBS = 'build_docs,doc,build' +const DEFAULT_DOMAIN = 'output.circle-artifacts.com' + +// Turn raw string options into the shape the resolver wants. Missing values +// fall back to the defaults, so callers can pass whatever they happen to have. +function normalizeConfig(raw = {}) { + const get = (name) => (raw[name] ?? '').toString().trim() + return { + // Tolerate spaces after the commas, e.g. "build_docs, doc" + jobNames: (get('circleci-jobs') || DEFAULT_JOBS) + .split(',') + .map((name) => name.trim()) + .filter((name) => name !== ''), + path: get('artifact-path'), + domain: get('domain') || DEFAULT_DOMAIN, + jobTitle: get('job-title'), + apiToken: get('api-token'), + } +} + +// A deliberately small parser for the flat "key: value" config file. The file +// is the `with:` block of the old workflow, so every value is a scalar; if that +// ever stops being true this should become a real YAML dependency. +function parseConfig(text) { + const config = {} + for (const rawLine of text.split('\n')) { + const line = rawLine.trim() + if (line === '' || line.startsWith('#')) { + continue + } + const colon = line.indexOf(':') + if (colon === -1) { + continue + } + const key = line.slice(0, colon).trim() + let value = line.slice(colon + 1).trim() + const comment = value.indexOf(' #') + if (comment !== -1 && !value.startsWith('"') && !value.startsWith("'")) { + value = value.slice(0, comment).trim() + } + if ((value.startsWith('"') && value.endsWith('"')) || + (value.startsWith("'") && value.endsWith("'"))) { + value = value.slice(1, -1) + } + config[key] = value + } + return config +} +;// CONCATENATED MODULE: ./src/core.js +// Everything both front ends share: given a GitHub `status` event payload and +// the options for a repo, work out which commit status to post. +// +// This module deliberately uses nothing Node-specific -- only the global fetch +// -- so that it also runs in a Cloudflare Worker (see worker/index.js). // Pick the job whose artifacts should be linked. A single-job workflow is // unambiguous; otherwise prefer a job the user asked for, and fall back to the @@ -36488,96 +36530,122 @@ async function fetchJson(fetchFn, url, options) { return response.json() } +// Work out the artifacts endpoint for a status target_url, which comes in two +// flavours depending on how the project is connected to GitHub. +async function artifactsUrlFor(target, jobNames, fetchFn, log) { + if (target.includes('/pipelines/circleci/') || target.includes('app.circleci.com/workflow/')) { + // ───── New GitHub‑App URL ─────────────────────────────────────────── + // .../pipelines/circleci////workflows/ + // OR + // .../workflow/ + const workflowId = target.split('/').at(-1) + log(`workflow: ${workflowId}`) + const jobs = await fetchJson(fetchFn, `https://circleci.com/api/v2/workflow/${workflowId}/job`) + if (!jobs.items.length) { + throw new Error(`No jobs returned for workflow ${workflowId}`) + } + const job = pickJob(jobs.items, jobNames) + log(`Using job ${job.name} of ${jobs.items.map((item) => item.name).join(', ')}`) + return `https://circleci.com/api/v2/project/${job.project_slug}/${job.job_number}/artifacts` + } + // ───── Legacy OAuth URL (…/gh///) ──────────────── + return legacyArtifactsUrl(target) +} + +// The whole job: from a status payload plus config, produce the commit status +// to create, or null when this event is none of our business. Throws when +// CircleCI cannot be reached or returns something unusable. +async function resolveStatus({payload, config, fetchFn = globalThis.fetch, log = () => {}}) { + // Each job reports itself as a "ci/circleci: " status context + const contexts = config.jobNames.map((name) => `ci/circleci: ${name}`) + if (!contexts.includes(payload.context)) { + log(`Ignoring context: ${payload.context}`) + return null + } + if (!payload.target_url) { + // Some status events carry no URL at all, so there is nothing to link to + log('Ignoring status with no target_url') + return null + } + log(`state: ${payload.state}, target_url: ${payload.target_url}`) + + const target = payload.target_url.split('?')[0] // strip any ?utm=… + const artifactsUrl = await artifactsUrlFor(target, config.jobNames, fetchFn, log) + log(`Fetching JSON: ${artifactsUrl}`) + // Only send a token when we have one: CircleCI rejects a bogus token with + // a 401 even for public projects, but is happy with no token at all + const headers = {'accept': 'application/json', 'user-agent': 'curl/7.85.0'} + if (config.apiToken !== '') { + headers['Circle-Token'] = config.apiToken + } + const artifacts = await fetchJson(fetchFn, artifactsUrl, {headers}) + log(`Artifacts JSON: ${JSON.stringify(artifacts)}`) + + const url = redirectUrl(artifacts.items, config.path, config.domain, payload.target_url) + const {state, description} = statusFor(payload.state, artifacts.items.length > 0, config.path) + return { + url, + state, + description, + context: config.jobTitle || `${payload.context} artifact`, + } +} + +;// CONCATENATED MODULE: ./index.js +// This as annoying because CircleCI does not use the App API. +// Hence we must monitor statuses rather than using the more convenient +// "checks" API. +// +// After changing this file, use `ncc build index.js -o dist` to rebuild to dist/ +// +// The logic itself lives in src/core.js, which is shared with the GitHub App +// front end in worker/index.js. + +// Refs: +// https://docs.github.com/en/developers/webhooks-and-events/webhooks/webhook-events-and-payloads#status + + + + + + + // The context/fetch/octokit arguments exist so that tests can inject fakes; // in production the defaults are always used. async function run({context = github_context, fetchFn = globalThis.fetch, getOctokit = github_getOctokit} = {}) { try { core_debug((new Date()).toTimeString()) const payload = context.payload - const path = getInput('artifact-path', {required: true}) const token = getInput('repo-token', {required: true}) - const apiToken = getInput('api-token', {required: false}) - if (apiToken !== '') { + const config = normalizeConfig({ + 'artifact-path': getInput('artifact-path', {required: true}), + 'circleci-jobs': getInput('circleci-jobs', {required: false}), + 'job-title': getInput('job-title', {required: false}), + 'domain': getInput('domain'), + 'api-token': getInput('api-token', {required: false}), + }) + if (config.apiToken !== '') { // Keep the token out of the logs, including any future logging of it - core_setSecret(apiToken) + core_setSecret(config.apiToken) core_debug('Successfully read CircleCI API token') } - // Tolerate spaces after the commas, e.g. "build_docs, doc" - const jobNames = (getInput('circleci-jobs', {required: false}) || 'build_docs,doc,build') - .split(',') - .map((name) => name.trim()) - .filter((name) => name !== '') - - // Each job reports itself as a "ci/circleci: " status context - const contexts = jobNames.map((name) => `ci/circleci: ${name}`) - core_debug(`Considering CircleCI jobs named: ${contexts}`) - if (!contexts.includes(payload.context)) { - core_debug(`Ignoring context: ${payload.context}`) - return - } - core_debug(`context: ${payload.context}`) - core_debug(`state: ${payload.state}`) - core_debug(`target_url: ${payload.target_url}`) - if (!payload.target_url) { - // Some status events carry no URL at all, so there is nothing to link to - core_debug('Ignoring status with no target_url') + const status = await resolveStatus({payload, config, fetchFn, log: core_debug}) + if (status === null) { return } - // e.g., https://circleci.com/gh/mne-tools/mne-python/53315 - // e.g., https://circleci.com/gh/scientific-python/circleci-artifacts-redirector-action/94?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link - const target = payload.target_url.split('?')[0] // strip any ?utm=… - let artifactsUrl = '' - if (target.includes('/pipelines/circleci/') || target.includes('app.circleci.com/workflow/')) { - // ───── New GitHub‑App URL ─────────────────────────────────────────── - // .../pipelines/circleci////workflows/ - // OR - // .../workflow/ - const workflowId = target.split('/').at(-1) - core_debug(`workflow: ${workflowId}`) - - const jobs = await fetchJson(fetchFn, `https://circleci.com/api/v2/workflow/${workflowId}/job`) - if (!jobs.items.length) { - setFailed(`No jobs returned for workflow ${workflowId}`) - return - } - - const job = pickJob(jobs.items, jobNames) - core_debug(`Using job ${job.name} of ${jobs.items.map((item) => item.name).join(', ')}`) - core_debug(`slug: ${job.project_slug}`) // "circleci//" - core_debug(`job#: ${job.job_number}`) - artifactsUrl = `https://circleci.com/api/v2/project/${job.project_slug}/${job.job_number}/artifacts` - } else { - artifactsUrl = legacyArtifactsUrl(target) - } - - core_debug(`Fetching JSON: ${artifactsUrl}`) - // Only send a token when we have one: CircleCI rejects a bogus token with - // a 401 even for public projects, but is happy with no token at all - const headers = {'accept': 'application/json', 'user-agent': 'curl/7.85.0'} - if (apiToken !== '') { - headers['Circle-Token'] = apiToken - } - // e.g., https://circleci.com/api/v2/project/gh/scientific-python/circleci-artifacts-redirector-action/94/artifacts - const artifacts = await fetchJson(fetchFn, artifactsUrl, {headers}) - core_debug(`Artifacts JSON: ${JSON.stringify(artifacts)}`) - // e.g., {"next_page_token":null,"items":[{"path":"test_artifacts/root_artifact.md","node_index":0,"url":"https://output.circle-artifacts.com/output/job/6fdfd148-31da-4a30-8e89-a20595696ca5/artifacts/0/test_artifacts/root_artifact.md"}]} - const url = redirectUrl(artifacts.items, path, getInput('domain'), payload.target_url) - core_debug(`Linking to: ${url}`) - core_debug((new Date()).toTimeString()) - setOutput('url', url) + core_debug(`Linking to: ${status.url}`) + setOutput('url', status.url) - const {state, description} = statusFor(payload.state, artifacts.items.length > 0, path) - const jobTitle = getInput('job-title', {required: false}) || `${payload.context} artifact` const client = getOctokit(token) return client.rest.repos.createCommitStatus({ repo: context.repo.repo, owner: context.repo.owner, sha: payload.sha, - state, - target_url: url, - description, - context: jobTitle + state: status.state, + target_url: status.url, + description: status.description, + context: status.context }) } catch (error) { // Keep the failure itself readable; the stack is there with debug logging @@ -36588,15 +36656,11 @@ async function run({context = github_context, fetchFn = globalThis.fetch, getOct // Run only when invoked as the action entry point, so that index.test.js can // import run() without executing it (this survives the ncc bundling). -/* node:coverage ignore next 3 */ +/* node:coverage disable */ if (import.meta.url === (0,external_node_url_.pathToFileURL)(process.argv[1]).href) { run() } +/* node:coverage enable */ -var __webpack_exports__fetchJson = __webpack_exports__.x6; -var __webpack_exports__legacyArtifactsUrl = __webpack_exports__.O$; -var __webpack_exports__pickJob = __webpack_exports__.BH; -var __webpack_exports__redirectUrl = __webpack_exports__.Qc; -var __webpack_exports__run = __webpack_exports__.eF; -var __webpack_exports__statusFor = __webpack_exports__.SR; -export { __webpack_exports__fetchJson as fetchJson, __webpack_exports__legacyArtifactsUrl as legacyArtifactsUrl, __webpack_exports__pickJob as pickJob, __webpack_exports__redirectUrl as redirectUrl, __webpack_exports__run as run, __webpack_exports__statusFor as statusFor }; +var __webpack_exports__run = __webpack_exports__.e; +export { __webpack_exports__run as run }; diff --git a/index.js b/index.js index 7cad27a..2ee021c 100644 --- a/index.js +++ b/index.js @@ -3,6 +3,9 @@ // "checks" API. // // After changing this file, use `ncc build index.js -o dist` to rebuild to dist/ +// +// The logic itself lives in src/core.js, which is shared with the GitHub App +// front end in worker/index.js. // Refs: // https://docs.github.com/en/developers/webhooks-and-events/webhooks/webhook-events-and-payloads#status @@ -10,60 +13,8 @@ import * as core from '@actions/core' import * as github from '@actions/github' import { pathToFileURL } from 'node:url' - -// Pick the job whose artifacts should be linked. A single-job workflow is -// unambiguous; otherwise prefer a job the user asked for, and fall back to the -// first one. -export function pickJob(items, jobNames) { - if (items.length === 1) { - return items[0] - } - return items.find((job) => jobNames.includes(job.name)) ?? items[0] -} - -// Turn a legacy OAuth target_url (…/gh///) into the v2 -// artifacts endpoint. -export function legacyArtifactsUrl(target) { - const [orgId, repoId, buildId] = new URL(target).pathname.split('/').slice(-3) - return `https://circleci.com/api/v2/project/gh/${orgId}/${repoId}/${buildId}/artifacts` -} - -// Build the URL to link to: the requested artifact if anything was uploaded, -// otherwise the CircleCI job itself (rewriting the domain only makes sense for -// artifact URLs). -export function redirectUrl(items, path, domain, fallback) { - if (!items.length) { - return fallback - } - // e.g., https://output.circle-artifacts.com/output/job//artifacts/0/doc/index.html - const job = items[0].url.split('/output/')[1].split('/artifacts/')[0] - return `https://${domain}/output/${job}/artifacts/${path}` -} - -// The status reports whether the link is usable, not whether the CircleCI job -// passed (gh-57): a job can fail late and still upload good artifacts, and the -// job's own status already reports the failure. -export function statusFor(payloadState, hasArtifacts, path) { - if (payloadState === 'pending') { - return {state: payloadState, description: 'Waiting for CircleCI ...'} - } - if (hasArtifacts) { - return {state: 'success', description: `Link to ${path}`} - } - return {state: 'failure', description: 'No artifacts found'} -} - -// Fetch JSON from the CircleCI API, failing loudly on a non-2xx response. -// Without this a 404 or a rate limit surfaces as a confusing "cannot read -// properties of undefined" from the caller. -export async function fetchJson(fetchFn, url, options) { - const response = await fetchFn(url, options) - if (!response.ok) { - const body = await response.text().catch(() => '') - throw new Error(`CircleCI API returned ${response.status} for ${url}: ${body.slice(0, 200)}`) - } - return response.json() -} +import { normalizeConfig } from './src/config.js' +import { resolveStatus } from './src/core.js' // The context/fetch/octokit arguments exist so that tests can inject fakes; // in production the defaults are always used. @@ -71,90 +22,36 @@ export async function run({context = github.context, fetchFn = globalThis.fetch, try { core.debug((new Date()).toTimeString()) const payload = context.payload - const path = core.getInput('artifact-path', {required: true}) const token = core.getInput('repo-token', {required: true}) - const apiToken = core.getInput('api-token', {required: false}) - if (apiToken !== '') { + const config = normalizeConfig({ + 'artifact-path': core.getInput('artifact-path', {required: true}), + 'circleci-jobs': core.getInput('circleci-jobs', {required: false}), + 'job-title': core.getInput('job-title', {required: false}), + 'domain': core.getInput('domain'), + 'api-token': core.getInput('api-token', {required: false}), + }) + if (config.apiToken !== '') { // Keep the token out of the logs, including any future logging of it - core.setSecret(apiToken) + core.setSecret(config.apiToken) core.debug('Successfully read CircleCI API token') } - // Tolerate spaces after the commas, e.g. "build_docs, doc" - const jobNames = (core.getInput('circleci-jobs', {required: false}) || 'build_docs,doc,build') - .split(',') - .map((name) => name.trim()) - .filter((name) => name !== '') - - // Each job reports itself as a "ci/circleci: " status context - const contexts = jobNames.map((name) => `ci/circleci: ${name}`) - core.debug(`Considering CircleCI jobs named: ${contexts}`) - if (!contexts.includes(payload.context)) { - core.debug(`Ignoring context: ${payload.context}`) - return - } - core.debug(`context: ${payload.context}`) - core.debug(`state: ${payload.state}`) - core.debug(`target_url: ${payload.target_url}`) - if (!payload.target_url) { - // Some status events carry no URL at all, so there is nothing to link to - core.debug('Ignoring status with no target_url') + const status = await resolveStatus({payload, config, fetchFn, log: core.debug}) + if (status === null) { return } - // e.g., https://circleci.com/gh/mne-tools/mne-python/53315 - // e.g., https://circleci.com/gh/scientific-python/circleci-artifacts-redirector-action/94?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link - const target = payload.target_url.split('?')[0] // strip any ?utm=… - let artifactsUrl = '' - if (target.includes('/pipelines/circleci/') || target.includes('app.circleci.com/workflow/')) { - // ───── New GitHub‑App URL ─────────────────────────────────────────── - // .../pipelines/circleci////workflows/ - // OR - // .../workflow/ - const workflowId = target.split('/').at(-1) - core.debug(`workflow: ${workflowId}`) - - const jobs = await fetchJson(fetchFn, `https://circleci.com/api/v2/workflow/${workflowId}/job`) - if (!jobs.items.length) { - core.setFailed(`No jobs returned for workflow ${workflowId}`) - return - } - - const job = pickJob(jobs.items, jobNames) - core.debug(`Using job ${job.name} of ${jobs.items.map((item) => item.name).join(', ')}`) - core.debug(`slug: ${job.project_slug}`) // "circleci//" - core.debug(`job#: ${job.job_number}`) - artifactsUrl = `https://circleci.com/api/v2/project/${job.project_slug}/${job.job_number}/artifacts` - } else { - artifactsUrl = legacyArtifactsUrl(target) - } - - core.debug(`Fetching JSON: ${artifactsUrl}`) - // Only send a token when we have one: CircleCI rejects a bogus token with - // a 401 even for public projects, but is happy with no token at all - const headers = {'accept': 'application/json', 'user-agent': 'curl/7.85.0'} - if (apiToken !== '') { - headers['Circle-Token'] = apiToken - } - // e.g., https://circleci.com/api/v2/project/gh/scientific-python/circleci-artifacts-redirector-action/94/artifacts - const artifacts = await fetchJson(fetchFn, artifactsUrl, {headers}) - core.debug(`Artifacts JSON: ${JSON.stringify(artifacts)}`) - // e.g., {"next_page_token":null,"items":[{"path":"test_artifacts/root_artifact.md","node_index":0,"url":"https://output.circle-artifacts.com/output/job/6fdfd148-31da-4a30-8e89-a20595696ca5/artifacts/0/test_artifacts/root_artifact.md"}]} - const url = redirectUrl(artifacts.items, path, core.getInput('domain'), payload.target_url) - core.debug(`Linking to: ${url}`) - core.debug((new Date()).toTimeString()) - core.setOutput('url', url) + core.debug(`Linking to: ${status.url}`) + core.setOutput('url', status.url) - const {state, description} = statusFor(payload.state, artifacts.items.length > 0, path) - const jobTitle = core.getInput('job-title', {required: false}) || `${payload.context} artifact` const client = getOctokit(token) return client.rest.repos.createCommitStatus({ repo: context.repo.repo, owner: context.repo.owner, sha: payload.sha, - state, - target_url: url, - description, - context: jobTitle + state: status.state, + target_url: status.url, + description: status.description, + context: status.context }) } catch (error) { // Keep the failure itself readable; the stack is there with debug logging @@ -165,7 +62,8 @@ export async function run({context = github.context, fetchFn = globalThis.fetch, // Run only when invoked as the action entry point, so that index.test.js can // import run() without executing it (this survives the ncc bundling). -/* node:coverage ignore next 3 */ +/* node:coverage disable */ if (import.meta.url === pathToFileURL(process.argv[1]).href) { run() } +/* node:coverage enable */ diff --git a/index.test.js b/index.test.js index 4e853ca..2562c08 100644 --- a/index.test.js +++ b/index.test.js @@ -3,7 +3,9 @@ import assert from 'node:assert/strict' import fs from 'node:fs' import os from 'node:os' import path from 'node:path' -import { run, pickJob, legacyArtifactsUrl, redirectUrl, statusFor, fetchJson } from './index.js' +import { run } from './index.js' +import { pickJob, legacyArtifactsUrl, redirectUrl, statusFor, fetchJson, resolveStatus } from './src/core.js' +import { normalizeConfig } from './src/config.js' const INPUTS = ['artifact-path', 'repo-token', 'api-token', 'circleci-jobs', 'job-title', 'domain'] const OUTPUT_FILE = path.join(fs.mkdtempSync(path.join(os.tmpdir(), 'redirector-')), 'output.txt') @@ -289,3 +291,13 @@ test('fetchJson', async () => { const unreadable = {ok: false, status: 500, text: async () => { throw new Error('nope') }} await assert.rejects(() => fetchJson(async () => unreadable, 'https://x'), /returned 500/) }) + +test('resolveStatus works without a logger', async () => { + const fetchFn = async () => ({ok: true, status: 200, json: async () => ({items: [ARTIFACT]})}) + const status = await resolveStatus({ + payload: {context: 'ci/circleci: build', state: 'success', target_url: 'https://circleci.com/gh/o/r/1'}, + config: normalizeConfig({'artifact-path': 'doc/index.html'}), + fetchFn, + }) + assert.equal(status.url, 'https://output.circle-artifacts.com/output/job/abc/artifacts/doc/index.html') +}) diff --git a/src/config.js b/src/config.js new file mode 100644 index 0000000..439e9a8 --- /dev/null +++ b/src/config.js @@ -0,0 +1,52 @@ +// The options accepted by both front ends: the GitHub Action reads them from +// workflow inputs, the GitHub App from .github/circleci-artifacts.yml. Keep the +// defaults here in sync with action.yml (index.test.js checks that they match). + +export const DEFAULT_JOBS = 'build_docs,doc,build' +export const DEFAULT_DOMAIN = 'output.circle-artifacts.com' + +// Turn raw string options into the shape the resolver wants. Missing values +// fall back to the defaults, so callers can pass whatever they happen to have. +export function normalizeConfig(raw = {}) { + const get = (name) => (raw[name] ?? '').toString().trim() + return { + // Tolerate spaces after the commas, e.g. "build_docs, doc" + jobNames: (get('circleci-jobs') || DEFAULT_JOBS) + .split(',') + .map((name) => name.trim()) + .filter((name) => name !== ''), + path: get('artifact-path'), + domain: get('domain') || DEFAULT_DOMAIN, + jobTitle: get('job-title'), + apiToken: get('api-token'), + } +} + +// A deliberately small parser for the flat "key: value" config file. The file +// is the `with:` block of the old workflow, so every value is a scalar; if that +// ever stops being true this should become a real YAML dependency. +export function parseConfig(text) { + const config = {} + for (const rawLine of text.split('\n')) { + const line = rawLine.trim() + if (line === '' || line.startsWith('#')) { + continue + } + const colon = line.indexOf(':') + if (colon === -1) { + continue + } + const key = line.slice(0, colon).trim() + let value = line.slice(colon + 1).trim() + const comment = value.indexOf(' #') + if (comment !== -1 && !value.startsWith('"') && !value.startsWith("'")) { + value = value.slice(0, comment).trim() + } + if ((value.startsWith('"') && value.endsWith('"')) || + (value.startsWith("'") && value.endsWith("'"))) { + value = value.slice(1, -1) + } + config[key] = value + } + return config +} diff --git a/src/core.js b/src/core.js new file mode 100644 index 0000000..d052aa4 --- /dev/null +++ b/src/core.js @@ -0,0 +1,120 @@ +// Everything both front ends share: given a GitHub `status` event payload and +// the options for a repo, work out which commit status to post. +// +// This module deliberately uses nothing Node-specific -- only the global fetch +// -- so that it also runs in a Cloudflare Worker (see worker/index.js). + +// Pick the job whose artifacts should be linked. A single-job workflow is +// unambiguous; otherwise prefer a job the user asked for, and fall back to the +// first one. +export function pickJob(items, jobNames) { + if (items.length === 1) { + return items[0] + } + return items.find((job) => jobNames.includes(job.name)) ?? items[0] +} + +// Turn a legacy OAuth target_url (…/gh///) into the v2 +// artifacts endpoint. +export function legacyArtifactsUrl(target) { + const [orgId, repoId, buildId] = new URL(target).pathname.split('/').slice(-3) + return `https://circleci.com/api/v2/project/gh/${orgId}/${repoId}/${buildId}/artifacts` +} + +// Build the URL to link to: the requested artifact if anything was uploaded, +// otherwise the CircleCI job itself (rewriting the domain only makes sense for +// artifact URLs). +export function redirectUrl(items, path, domain, fallback) { + if (!items.length) { + return fallback + } + // e.g., https://output.circle-artifacts.com/output/job//artifacts/0/doc/index.html + const job = items[0].url.split('/output/')[1].split('/artifacts/')[0] + return `https://${domain}/output/${job}/artifacts/${path}` +} + +// The status reports whether the link is usable, not whether the CircleCI job +// passed (gh-57): a job can fail late and still upload good artifacts, and the +// job's own status already reports the failure. +export function statusFor(payloadState, hasArtifacts, path) { + if (payloadState === 'pending') { + return {state: payloadState, description: 'Waiting for CircleCI ...'} + } + if (hasArtifacts) { + return {state: 'success', description: `Link to ${path}`} + } + return {state: 'failure', description: 'No artifacts found'} +} + +// Fetch JSON from the CircleCI API, failing loudly on a non-2xx response. +// Without this a 404 or a rate limit surfaces as a confusing "cannot read +// properties of undefined" from the caller. +export async function fetchJson(fetchFn, url, options) { + const response = await fetchFn(url, options) + if (!response.ok) { + const body = await response.text().catch(() => '') + throw new Error(`CircleCI API returned ${response.status} for ${url}: ${body.slice(0, 200)}`) + } + return response.json() +} + +// Work out the artifacts endpoint for a status target_url, which comes in two +// flavours depending on how the project is connected to GitHub. +export async function artifactsUrlFor(target, jobNames, fetchFn, log) { + if (target.includes('/pipelines/circleci/') || target.includes('app.circleci.com/workflow/')) { + // ───── New GitHub‑App URL ─────────────────────────────────────────── + // .../pipelines/circleci////workflows/ + // OR + // .../workflow/ + const workflowId = target.split('/').at(-1) + log(`workflow: ${workflowId}`) + const jobs = await fetchJson(fetchFn, `https://circleci.com/api/v2/workflow/${workflowId}/job`) + if (!jobs.items.length) { + throw new Error(`No jobs returned for workflow ${workflowId}`) + } + const job = pickJob(jobs.items, jobNames) + log(`Using job ${job.name} of ${jobs.items.map((item) => item.name).join(', ')}`) + return `https://circleci.com/api/v2/project/${job.project_slug}/${job.job_number}/artifacts` + } + // ───── Legacy OAuth URL (…/gh///) ──────────────── + return legacyArtifactsUrl(target) +} + +// The whole job: from a status payload plus config, produce the commit status +// to create, or null when this event is none of our business. Throws when +// CircleCI cannot be reached or returns something unusable. +export async function resolveStatus({payload, config, fetchFn = globalThis.fetch, log = () => {}}) { + // Each job reports itself as a "ci/circleci: " status context + const contexts = config.jobNames.map((name) => `ci/circleci: ${name}`) + if (!contexts.includes(payload.context)) { + log(`Ignoring context: ${payload.context}`) + return null + } + if (!payload.target_url) { + // Some status events carry no URL at all, so there is nothing to link to + log('Ignoring status with no target_url') + return null + } + log(`state: ${payload.state}, target_url: ${payload.target_url}`) + + const target = payload.target_url.split('?')[0] // strip any ?utm=… + const artifactsUrl = await artifactsUrlFor(target, config.jobNames, fetchFn, log) + log(`Fetching JSON: ${artifactsUrl}`) + // Only send a token when we have one: CircleCI rejects a bogus token with + // a 401 even for public projects, but is happy with no token at all + const headers = {'accept': 'application/json', 'user-agent': 'curl/7.85.0'} + if (config.apiToken !== '') { + headers['Circle-Token'] = config.apiToken + } + const artifacts = await fetchJson(fetchFn, artifactsUrl, {headers}) + log(`Artifacts JSON: ${JSON.stringify(artifacts)}`) + + const url = redirectUrl(artifacts.items, config.path, config.domain, payload.target_url) + const {state, description} = statusFor(payload.state, artifacts.items.length > 0, config.path) + return { + url, + state, + description, + context: config.jobTitle || `${payload.context} artifact`, + } +} diff --git a/worker/index.js b/worker/index.js new file mode 100644 index 0000000..e957f6d --- /dev/null +++ b/worker/index.js @@ -0,0 +1,149 @@ +// GitHub App front end: a Cloudflare Worker that receives `status` webhooks and +// posts the artifact link itself, so repos do not need a workflow at all (and +// therefore do not get a workflow run per status event, gh-27). +// +// The resolution logic is shared with the action; only the plumbing is here. +// +// Deploy: wrangler deploy +// Secrets: wrangler secret put APP_ID / PRIVATE_KEY / WEBHOOK_SECRET +// +// PRIVATE_KEY must be the PKCS#8 form of the App's key, which GitHub does not +// hand you directly: +// openssl pkcs8 -topk8 -inform PEM -outform PEM -nocrypt \ +// -in downloaded.private-key.pem -out pkcs8.pem + +import { normalizeConfig, parseConfig } from '../src/config.js' +import { resolveStatus } from '../src/core.js' + +export const CONFIG_PATH = '.github/circleci-artifacts.yml' +const API = 'https://api.github.com' +const UA = {'user-agent': 'circleci-artifacts-redirector-app', 'accept': 'application/vnd.github+json'} + +// Constant-time-ish comparison of the webhook signature. +export async function verifySignature(secret, body, signature) { + if (!signature || !signature.startsWith('sha256=')) { + return false + } + const key = await crypto.subtle.importKey( + 'raw', new TextEncoder().encode(secret), {name: 'HMAC', hash: 'SHA-256'}, false, ['verify']) + const bytes = signature.slice('sha256='.length) + if (bytes.length !== 64 || !/^[0-9a-f]+$/.test(bytes)) { + return false + } + const provided = Uint8Array.from(bytes.match(/../g).map((h) => parseInt(h, 16))) + return crypto.subtle.verify('HMAC', key, provided, new TextEncoder().encode(body)) +} + +function base64url(bytes) { + return btoa(String.fromCharCode(...new Uint8Array(bytes))) + .replace(/\+/g, '-').replace(/\//g, '_').replace(/=+$/, '') +} + +// Mint an installation access token: sign a JWT with the App key, then trade it +// in for a token scoped to the repo the event came from. +export async function mintToken({appId, privateKey, installationId, fetchFn = globalThis.fetch, now = Date.now}) { + const der = Uint8Array.from( + atob(privateKey.replace(/-----[^-]+-----/g, '').replace(/\s/g, '')), + (c) => c.charCodeAt(0)) + const key = await crypto.subtle.importKey( + 'pkcs8', der, {name: 'RSASSA-PKCS1-v1_5', hash: 'SHA-256'}, false, ['sign']) + const issued = Math.floor(now() / 1000) - 60 + const claims = {iat: issued, exp: issued + 600, iss: appId} + const unsigned = `${base64url(new TextEncoder().encode(JSON.stringify({alg: 'RS256', typ: 'JWT'})))}.` + + `${base64url(new TextEncoder().encode(JSON.stringify(claims)))}` + const signature = await crypto.subtle.sign( + 'RSASSA-PKCS1-v1_5', key, new TextEncoder().encode(unsigned)) + const jwt = `${unsigned}.${base64url(signature)}` + + const response = await fetchFn(`${API}/app/installations/${installationId}/access_tokens`, { + method: 'POST', + headers: {...UA, authorization: `Bearer ${jwt}`}, + }) + if (!response.ok) { + throw new Error(`Could not mint an installation token: ${response.status}`) + } + return (await response.json()).token +} + +// Read the config from the *default branch*, never the PR head: otherwise a +// forked PR could point `domain` at a host it controls and have us post a +// trusted-looking link to it. +export async function readConfig(fetchFn, repo, token) { + const url = `${API}/repos/${repo.full_name}/contents/${CONFIG_PATH}?ref=${repo.default_branch}` + const response = await fetchFn(url, {headers: {...UA, authorization: `Bearer ${token}`}}) + if (response.status === 404) { + return null + } + if (!response.ok) { + throw new Error(`Could not read ${CONFIG_PATH}: ${response.status}`) + } + const {content} = await response.json() + return parseConfig(atob(content.replace(/\s/g, ''))) +} + +export async function handle(request, env, {fetchFn = globalThis.fetch, log = () => {}} = {}) { + if (request.method !== 'POST') { + return new Response('POST only', {status: 405}) + } + const body = await request.text() + if (!await verifySignature(env.WEBHOOK_SECRET, body, request.headers.get('x-hub-signature-256'))) { + return new Response('bad signature', {status: 401}) + } + if (request.headers.get('x-github-event') !== 'status') { + return new Response('ignored: not a status event', {status: 200}) + } + + const payload = JSON.parse(body) + // Cheap filter first: most status events on a busy repo are not ours, and + // this path must not cost an API call + if (!(payload.context ?? '').startsWith('ci/circleci: ')) { + return new Response('ignored: not a CircleCI status', {status: 200}) + } + + const token = await mintToken({ + appId: env.APP_ID, + privateKey: env.PRIVATE_KEY, + installationId: payload.installation.id, + fetchFn, + }) + const raw = await readConfig(fetchFn, payload.repository, token) + if (raw === null) { + return new Response(`ignored: no ${CONFIG_PATH}`, {status: 200}) + } + const config = normalizeConfig(raw) + if (config.path === '') { + return new Response('ignored: no artifact-path configured', {status: 200}) + } + + const status = await resolveStatus({payload, config, fetchFn, log}) + if (status === null) { + return new Response('ignored: not a watched job', {status: 200}) + } + + const response = await fetchFn(`${API}/repos/${payload.repository.full_name}/statuses/${payload.sha}`, { + method: 'POST', + headers: {...UA, authorization: `Bearer ${token}`}, + body: JSON.stringify({ + state: status.state, + target_url: status.url, + description: status.description, + context: status.context, + }), + }) + if (!response.ok) { + throw new Error(`Could not post the status: ${response.status}`) + } + return new Response(`posted ${status.state}: ${status.url}`, {status: 200}) +} + +export default { + async fetch(request, env) { + try { + return await handle(request, env) + } catch (error) { + // A 500 tells GitHub the delivery failed, so it shows up in the App's + // "Recent Deliveries" tab rather than vanishing + return new Response(String(error), {status: 500}) + } + }, +} diff --git a/worker/index.test.js b/worker/index.test.js new file mode 100644 index 0000000..c63f3d6 --- /dev/null +++ b/worker/index.test.js @@ -0,0 +1,218 @@ +import test from 'node:test' +import assert from 'node:assert/strict' +import crypto from 'node:crypto' +import worker, { handle, verifySignature, mintToken, readConfig, CONFIG_PATH } from './index.js' +import { parseConfig, normalizeConfig } from '../src/config.js' + +const SECRET = 'webhook-secret' +// A throwaway key, generated once here, so the JWT path runs for real +const {privateKey} = crypto.generateKeyPairSync('rsa', { + modulusLength: 2048, + privateKeyEncoding: {type: 'pkcs8', format: 'pem'}, + publicKeyEncoding: {type: 'spki', format: 'pem'}, +}) +const ENV = {APP_ID: '123', PRIVATE_KEY: privateKey, WEBHOOK_SECRET: SECRET} + +const CONFIG = 'artifact-path: 0/doc/index.html\njob-title: Docs preview\n' +const ARTIFACT = {url: 'https://output.circle-artifacts.com/output/job/abc/artifacts/0/doc/other.html'} + +const PAYLOAD = { + context: 'ci/circleci: build', + state: 'success', + sha: 'deadbeef', + target_url: 'https://circleci.com/gh/scientific-python/demo/94', + repository: {full_name: 'scientific-python/demo', default_branch: 'main'}, + installation: {id: 42}, +} + +function sign(body, secret = SECRET) { + return 'sha256=' + crypto.createHmac('sha256', secret).update(body).digest('hex') +} + +function webhook(payload, {event = 'status', secret = SECRET, method = 'POST'} = {}) { + const body = JSON.stringify(payload) + return new Request('https://worker.example/', { + method, + body: method === 'POST' ? body : undefined, + headers: {'x-github-event': event, 'x-hub-signature-256': sign(body, secret)}, + }) +} + +// Fake GitHub + CircleCI. Returns the requests it saw so tests can assert on +// what would have been posted. +function backend({config = CONFIG, artifacts = {items: [ARTIFACT]}, statusCode = 201} = {}) { + const seen = [] + const fetchFn = async (url, options = {}) => { + seen.push({url, method: options.method ?? 'GET', body: options.body}) + if (url.endsWith('/access_tokens')) { + return new Response(JSON.stringify({token: 'ghs_installation'}), {status: 201}) + } + if (url.includes(`/contents/${CONFIG_PATH}`)) { + return config === null + ? new Response('{}', {status: 404}) + : new Response(JSON.stringify({content: btoa(config)}), {status: 200}) + } + if (url.includes('/artifacts')) { + return new Response(JSON.stringify(artifacts), {status: 200}) + } + if (url.includes('/statuses/')) { + return new Response('{}', {status: statusCode}) + } + throw new Error(`unexpected request: ${url}`) + } + return {fetchFn, seen} +} + +test('posts a status for a CircleCI event', async () => { + const {fetchFn, seen} = backend() + const response = await handle(webhook(PAYLOAD), ENV, {fetchFn}) + assert.equal(response.status, 200) + + const posted = seen.find((r) => r.url.includes('/statuses/')) + assert.ok(posted, 'a status was posted') + assert.equal(posted.url, 'https://api.github.com/repos/scientific-python/demo/statuses/deadbeef') + assert.deepEqual(JSON.parse(posted.body), { + state: 'success', + target_url: 'https://output.circle-artifacts.com/output/job/abc/artifacts/0/doc/index.html', + description: 'Link to 0/doc/index.html', + context: 'Docs preview', + }) +}) + +test('reads the config from the default branch, not the event', async () => { + const {fetchFn, seen} = backend() + await handle(webhook(PAYLOAD), ENV, {fetchFn}) + const read = seen.find((r) => r.url.includes(`/contents/${CONFIG_PATH}`)) + assert.match(read.url, /\?ref=main$/, 'pinned to the default branch') +}) + +test('rejects a bad signature before doing anything', async () => { + const {fetchFn, seen} = backend() + const response = await handle(webhook(PAYLOAD, {secret: 'wrong'}), ENV, {fetchFn}) + assert.equal(response.status, 401) + assert.deepEqual(seen, [], 'no API calls on an unverified body') +}) + +test('ignores non-status events and non-CircleCI contexts without any API call', async () => { + for (const [request, why] of [ + [webhook(PAYLOAD, {event: 'push'}), 'wrong event'], + [webhook({...PAYLOAD, context: 'codecov/patch'}), 'wrong context'], + [webhook({...PAYLOAD, context: undefined}), 'no context'], + ]) { + const {fetchFn, seen} = backend() + const response = await handle(request, ENV, {fetchFn}) + assert.equal(response.status, 200, why) + assert.deepEqual(seen, [], `no API calls: ${why}`) + } +}) + +test('ignores repos with no config file, and configs with no artifact-path', async () => { + for (const config of [null, 'job-title: incomplete\n']) { + const {fetchFn, seen} = backend({config}) + const response = await handle(webhook(PAYLOAD), ENV, {fetchFn}) + assert.equal(response.status, 200) + assert.ok(!seen.some((r) => r.url.includes('/statuses/')), 'nothing posted') + } +}) + +test('ignores a job the config does not watch', async () => { + const {fetchFn, seen} = backend({config: 'artifact-path: p\ncircleci-jobs: other\n'}) + const response = await handle(webhook(PAYLOAD), ENV, {fetchFn}) + assert.equal(response.status, 200) + assert.ok(!seen.some((r) => r.url.includes('/artifacts')), 'no CircleCI call either') +}) + +test('rejects non-POST', async () => { + const {fetchFn} = backend() + const response = await handle(webhook(PAYLOAD, {method: 'GET'}), ENV, {fetchFn}) + assert.equal(response.status, 405) +}) + +test('surfaces failures to post', async () => { + const {fetchFn} = backend({statusCode: 403}) + await assert.rejects(() => handle(webhook(PAYLOAD), ENV, {fetchFn}), /Could not post the status: 403/) +}) + +test('surfaces failures to read config or mint a token', async () => { + const broken = async (url) => url.endsWith('/access_tokens') + ? new Response('{}', {status: 401}) + : new Response('{}', {status: 500}) + await assert.rejects( + () => handle(webhook(PAYLOAD), ENV, {fetchFn: broken}), /Could not mint an installation token: 401/) + + const badConfig = async (url) => url.endsWith('/access_tokens') + ? new Response(JSON.stringify({token: 't'}), {status: 201}) + : new Response('{}', {status: 500}) + await assert.rejects( + () => handle(webhook(PAYLOAD), ENV, {fetchFn: badConfig}), new RegExp(`Could not read ${CONFIG_PATH}: 500`)) +}) + +test('verifySignature rejects malformed headers', async () => { + for (const header of [null, 'sha1=abc', 'sha256=nothex', 'sha256=' + 'a'.repeat(63)]) { + assert.equal(await verifySignature(SECRET, 'body', header), false, `rejected: ${header}`) + } + assert.equal(await verifySignature(SECRET, 'body', sign('body')), true) +}) + +test('mintToken signs a real RS256 JWT', async () => { + let authorization + const fetchFn = async (url, options) => { + authorization = options.headers.authorization + return new Response(JSON.stringify({token: 'ghs_x'}), {status: 201}) + } + const token = await mintToken({ + appId: '123', privateKey, installationId: 7, fetchFn, now: () => 1_000_000_000_000}) + assert.equal(token, 'ghs_x') + + const [header, claims, signature] = authorization.replace('Bearer ', '').split('.') + assert.deepEqual(JSON.parse(atob(header)), {alg: 'RS256', typ: 'JWT'}) + const parsed = JSON.parse(atob(claims)) + assert.equal(parsed.iss, '123') + assert.equal(parsed.exp - parsed.iat, 600) + // Verify the signature against the public half, i.e. GitHub would accept it + const verified = crypto.createVerify('RSA-SHA256') + .update(`${header}.${claims}`) + .verify(privateKey, Buffer.from(signature.replace(/-/g, '+').replace(/_/g, '/'), 'base64')) + assert.ok(verified, 'the JWT signature checks out') +}) + +test('readConfig returns null when the file is missing', async () => { + const fetchFn = async () => new Response('{}', {status: 404}) + assert.equal(await readConfig(fetchFn, {full_name: 'a/b', default_branch: 'main'}, 't'), null) +}) + +test('parseConfig handles the shapes a migrated workflow produces', () => { + assert.deepEqual(parseConfig([ + '# a comment', + '', + 'artifact-path: 0/doc/index.html', + 'job-title: "Check the rendered docs here!"', + "circleci-jobs: 'build_docs, doc'", + 'domain: circle.scientific-python.dev # via the proxy', + 'not a mapping line', + ].join('\n')), { + 'artifact-path': '0/doc/index.html', + 'job-title': 'Check the rendered docs here!', + 'circleci-jobs': 'build_docs, doc', + 'domain': 'circle.scientific-python.dev', + }) +}) + +test('normalizeConfig applies the same defaults as the action', () => { + const config = normalizeConfig({'artifact-path': ' p '}) + assert.deepEqual(config.jobNames, ['build_docs', 'doc', 'build']) + assert.equal(config.domain, 'output.circle-artifacts.com') + assert.equal(config.path, 'p') + assert.equal(config.apiToken, '') + assert.deepEqual(normalizeConfig().jobNames, ['build_docs', 'doc', 'build']) + assert.deepEqual(normalizeConfig({'circleci-jobs': 'a, ,b '}).jobNames, ['a', 'b']) +}) + +test('the default entry point answers without touching the network', async () => { + // A bad signature is rejected before any fetch happens + assert.equal((await worker.fetch(webhook(PAYLOAD, {secret: 'wrong'}), ENV)).status, 401) + + // An unusable private key throws while importing, i.e. still before any fetch + const response = await worker.fetch(webhook(PAYLOAD), {...ENV, PRIVATE_KEY: 'not-a-key'}) + assert.equal(response.status, 500, 'failures surface to GitHub as a failed delivery') +}) diff --git a/wrangler.toml b/wrangler.toml new file mode 100644 index 0000000..43750db --- /dev/null +++ b/wrangler.toml @@ -0,0 +1,3 @@ +name = "circleci-artifacts-redirector-app" +main = "worker/index.js" +compatibility_date = "2026-07-28" From 83530f0b8fb5078534da55818de79ea9cf2609b3 Mon Sep 17 00:00:00 2001 From: Eric Larson Date: Tue, 28 Jul 2026 14:21:14 -0400 Subject: [PATCH 2/7] ENH: Cache tokens and config per isolate Cloudflare reuses an isolate across many requests, so memoizing the installation token (50 min) and the repo config (10 min) in a plain Map removes most of the GitHub API traffic without needing KV. Failures are evicted rather than cached, and storing the promise means concurrent events for one repo share a single request. Cuts subrequests for scikit-learn + MNE-Python + SciPy from ~14,700/wk to ~6,300/wk when isolates are warm; a cold isolate simply fetches again. Co-Authored-By: Claude Opus 5 --- worker/index.js | 41 +++++++++++++++++++++++++++++---- worker/index.test.js | 54 ++++++++++++++++++++++++++++++++++++++++++-- 2 files changed, 88 insertions(+), 7 deletions(-) diff --git a/worker/index.js b/worker/index.js index e957f6d..a042b3f 100644 --- a/worker/index.js +++ b/worker/index.js @@ -16,6 +16,34 @@ import { normalizeConfig, parseConfig } from '../src/config.js' import { resolveStatus } from '../src/core.js' export const CONFIG_PATH = '.github/circleci-artifacts.yml' +// Cloudflare reuses an isolate across many requests, so a plain Map removes +// most of the token and config traffic without needing KV. Nothing here is +// correctness-critical: a cold isolate simply fetches again. +export const TOKEN_TTL_MS = 50 * 60 * 1000 // installation tokens last an hour +export const CONFIG_TTL_MS = 10 * 60 * 1000 +const cache = new Map() + +export function clearCache() { + cache.clear() +} + +// Memoize a promise, evicting it if it rejects so that a blip is not cached +// for the whole TTL. Storing the promise (not the value) also means concurrent +// events for the same repo share one request. +export function cached(key, ttl, produce, now = Date.now) { + const hit = cache.get(key) + if (hit && hit.expires > now()) { + return hit.value + } + const value = produce() + cache.set(key, {value, expires: now() + ttl}) + value.catch(() => { + if (cache.get(key)?.value === value) { + cache.delete(key) + } + }) + return value +} const API = 'https://api.github.com' const UA = {'user-agent': 'circleci-artifacts-redirector-app', 'accept': 'application/vnd.github+json'} @@ -81,7 +109,7 @@ export async function readConfig(fetchFn, repo, token) { return parseConfig(atob(content.replace(/\s/g, ''))) } -export async function handle(request, env, {fetchFn = globalThis.fetch, log = () => {}} = {}) { +export async function handle(request, env, {fetchFn = globalThis.fetch, log = () => {}, now = Date.now} = {}) { if (request.method !== 'POST') { return new Response('POST only', {status: 405}) } @@ -100,13 +128,16 @@ export async function handle(request, env, {fetchFn = globalThis.fetch, log = () return new Response('ignored: not a CircleCI status', {status: 200}) } - const token = await mintToken({ + const repo = payload.repository + const token = await cached(`token:${payload.installation.id}`, TOKEN_TTL_MS, () => mintToken({ appId: env.APP_ID, privateKey: env.PRIVATE_KEY, installationId: payload.installation.id, fetchFn, - }) - const raw = await readConfig(fetchFn, payload.repository, token) + }), now) + const raw = await cached( + `config:${repo.full_name}@${repo.default_branch}`, CONFIG_TTL_MS, + () => readConfig(fetchFn, repo, token), now) if (raw === null) { return new Response(`ignored: no ${CONFIG_PATH}`, {status: 200}) } @@ -120,7 +151,7 @@ export async function handle(request, env, {fetchFn = globalThis.fetch, log = () return new Response('ignored: not a watched job', {status: 200}) } - const response = await fetchFn(`${API}/repos/${payload.repository.full_name}/statuses/${payload.sha}`, { + const response = await fetchFn(`${API}/repos/${repo.full_name}/statuses/${payload.sha}`, { method: 'POST', headers: {...UA, authorization: `Bearer ${token}`}, body: JSON.stringify({ diff --git a/worker/index.test.js b/worker/index.test.js index c63f3d6..c1206b2 100644 --- a/worker/index.test.js +++ b/worker/index.test.js @@ -1,9 +1,11 @@ -import test from 'node:test' +import test, { beforeEach } from 'node:test' import assert from 'node:assert/strict' import crypto from 'node:crypto' -import worker, { handle, verifySignature, mintToken, readConfig, CONFIG_PATH } from './index.js' +import worker, { handle, verifySignature, mintToken, readConfig, clearCache, CONFIG_PATH, TOKEN_TTL_MS, CONFIG_TTL_MS } from './index.js' import { parseConfig, normalizeConfig } from '../src/config.js' +beforeEach(clearCache) + const SECRET = 'webhook-secret' // A throwaway key, generated once here, so the JWT path runs for real const {privateKey} = crypto.generateKeyPairSync('rsa', { @@ -99,6 +101,7 @@ test('ignores non-status events and non-CircleCI contexts without any API call', [webhook({...PAYLOAD, context: 'codecov/patch'}), 'wrong context'], [webhook({...PAYLOAD, context: undefined}), 'no context'], ]) { + clearCache() const {fetchFn, seen} = backend() const response = await handle(request, ENV, {fetchFn}) assert.equal(response.status, 200, why) @@ -108,6 +111,7 @@ test('ignores non-status events and non-CircleCI contexts without any API call', test('ignores repos with no config file, and configs with no artifact-path', async () => { for (const config of [null, 'job-title: incomplete\n']) { + clearCache() const {fetchFn, seen} = backend({config}) const response = await handle(webhook(PAYLOAD), ENV, {fetchFn}) assert.equal(response.status, 200) @@ -216,3 +220,49 @@ test('the default entry point answers without touching the network', async () => const response = await worker.fetch(webhook(PAYLOAD), {...ENV, PRIVATE_KEY: 'not-a-key'}) assert.equal(response.status, 500, 'failures surface to GitHub as a failed delivery') }) + +test('reuses the token and config across events in the same isolate', async () => { + const {fetchFn, seen} = backend() + await handle(webhook(PAYLOAD), ENV, {fetchFn}) + const first = seen.length + await handle(webhook({...PAYLOAD, sha: 'cafe'}), ENV, {fetchFn}) + + const second = seen.slice(first).map((r) => r.url) + assert.ok(!second.some((u) => u.endsWith('/access_tokens')), 'token reused') + assert.ok(!second.some((u) => u.includes('/contents/')), 'config reused') + assert.ok(second.some((u) => u.includes('/statuses/')), 'but the status is still posted') + assert.equal(second.length, 2, 'only CircleCI + the status POST') +}) + +test('refetches once each cache entry expires', async () => { + const {fetchFn, seen} = backend() + let clock = 1_000_000 + const now = () => clock + await handle(webhook(PAYLOAD), ENV, {fetchFn, now}) + + clock += CONFIG_TTL_MS + 1 + let before = seen.length + await handle(webhook(PAYLOAD), ENV, {fetchFn, now}) + let urls = seen.slice(before).map((r) => r.url) + assert.ok(urls.some((u) => u.includes('/contents/')), 'config refetched') + assert.ok(!urls.some((u) => u.endsWith('/access_tokens')), 'token still valid') + + clock += TOKEN_TTL_MS + 1 + before = seen.length + await handle(webhook(PAYLOAD), ENV, {fetchFn, now}) + urls = seen.slice(before).map((r) => r.url) + assert.ok(urls.some((u) => u.endsWith('/access_tokens')), 'token refetched') +}) + +test('does not cache a failure', async () => { + let fail = true + const {fetchFn} = backend() + const flaky = async (url, options) => (fail && url.endsWith('/access_tokens')) + ? new Response('{}', {status: 500}) + : fetchFn(url, options) + + await assert.rejects(() => handle(webhook(PAYLOAD), ENV, {fetchFn: flaky})) + fail = false + const response = await handle(webhook(PAYLOAD), ENV, {fetchFn: flaky}) + assert.equal(response.status, 200, 'the next event retries instead of serving the failure') +}) From c5498bc01632d6f2d48c83540f1dc8ea61acad89 Mon Sep 17 00:00:00 2001 From: Eric Larson Date: Tue, 28 Jul 2026 14:56:26 -0400 Subject: [PATCH 3/7] FIX: Fail closed when WEBHOOK_SECRET is unset Web Crypto rejects a zero-length HMAC key, so a missing or empty secret threw and surfaced as a 500 instead of rejecting the delivery. Found by a real webhook delivery while wiring up the app: GitHub reported 500 where it should have been a 401. Co-Authored-By: Claude Opus 5 --- worker/index.js | 4 +++- worker/index.test.js | 11 +++++++++++ 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/worker/index.js b/worker/index.js index a042b3f..6816663 100644 --- a/worker/index.js +++ b/worker/index.js @@ -49,7 +49,9 @@ const UA = {'user-agent': 'circleci-artifacts-redirector-app', 'accept': 'applic // Constant-time-ish comparison of the webhook signature. export async function verifySignature(secret, body, signature) { - if (!signature || !signature.startsWith('sha256=')) { + // An unset secret must fail closed rather than throw: Web Crypto rejects a + // zero-length HMAC key, which would otherwise surface as a 500 + if (!secret || !signature || !signature.startsWith('sha256=')) { return false } const key = await crypto.subtle.importKey( diff --git a/worker/index.test.js b/worker/index.test.js index c1206b2..c959a55 100644 --- a/worker/index.test.js +++ b/worker/index.test.js @@ -152,6 +152,8 @@ test('surfaces failures to read config or mint a token', async () => { }) test('verifySignature rejects malformed headers', async () => { + assert.equal(await verifySignature('', 'body', sign('body')), false, 'an empty secret never verifies') + assert.equal(await verifySignature(undefined, 'body', sign('body')), false, 'nor an unset one') for (const header of [null, 'sha1=abc', 'sha256=nothex', 'sha256=' + 'a'.repeat(63)]) { assert.equal(await verifySignature(SECRET, 'body', header), false, `rejected: ${header}`) } @@ -266,3 +268,12 @@ test('does not cache a failure', async () => { const response = await handle(webhook(PAYLOAD), ENV, {fetchFn: flaky}) assert.equal(response.status, 200, 'the next event retries instead of serving the failure') }) + +test('a missing WEBHOOK_SECRET is a 401, not a crash', async () => { + const {fetchFn, seen} = backend() + for (const env of [{...ENV, WEBHOOK_SECRET: undefined}, {...ENV, WEBHOOK_SECRET: ''}]) { + const response = await handle(webhook(PAYLOAD), env, {fetchFn}) + assert.equal(response.status, 401) + assert.deepEqual(seen, []) + } +}) From 629f26ce161735ff1f08309d86032200531287bc Mon Sep 17 00:00:00 2001 From: Eric Larson Date: Tue, 28 Jul 2026 15:19:28 -0400 Subject: [PATCH 4/7] ENH: Add post-pending, and never post pending from the app Every status posted is itself a status event that comes back around (gh-27), so the 'Waiting for CircleCI ...' status doubles the traffic a repo generates. The action gains a post-pending option, defaulting to true so existing setups are unchanged; the app hard-codes it off, since the final status says everything the pending one did. When off, the pending event returns before the CircleCI call, so it costs no API traffic either. Co-Authored-By: Claude Opus 5 --- README.md | 9 +++++++++ action.yml | 8 ++++++++ dist/index.js | 10 ++++++++++ index.js | 1 + index.test.js | 23 ++++++++++++++++++++++- src/config.js | 3 +++ src/core.js | 6 ++++++ worker/index.js | 4 +++- worker/index.test.js | 12 ++++++++++++ 9 files changed, 74 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 1feda2f..a8028a6 100644 --- a/README.md +++ b/README.md @@ -67,6 +67,12 @@ jobs: status for that). So a job that fails after uploading its artifacts still gets a green link, and a job that passes without uploading anything gets a red one (see [#57](https://github.com/scientific-python/circleci-artifacts-redirector-action/issues/57)). +- Set `post-pending: 'false'` to skip the "Waiting for CircleCI ..." status + that is posted while the job is still running. That halves the statuses this + action creates, and since every status is itself a `status` event, it halves + the workflow runs they trigger too (see + [#27](https://github.com/scientific-python/circleci-artifacts-redirector-action/issues/27)). + It defaults to `'true'`, so existing setups are unchanged. - The action has an output `url` that you can use in downstream steps, but this URL will only point to a valid artifact once the job is complete, i.e., `github.event.status` is either `'success'`, `'fail'`, or (maybe) `'error'`, @@ -109,6 +115,9 @@ drift apart: the same resolution logic and the same option defaults serve both. Differences from the action, by design: - No `url` output, because there is no workflow step to consume it. +- No "Waiting for CircleCI ..." status: the app always behaves as though + `post-pending` were `false`, since each status it posts is itself a `status` + event, and the final status says everything the pending one did. - Public CircleCI projects only: a private project needs an `api-token`, which would mean storing each repo's CircleCI token server-side. diff --git a/action.yml b/action.yml index 4ed1299..0a95732 100644 --- a/action.yml +++ b/action.yml @@ -30,6 +30,14 @@ inputs: which addresses some routing issues. required: false default: 'output.circle-artifacts.com' + post-pending: + description: | + Whether to post a "Waiting for CircleCI ..." status while the CircleCI + job is still running. Set to 'false' to halve the number of statuses + this action creates (and therefore the number of workflow runs the + `on: status` trigger produces). + required: false + default: 'true' outputs: url: description: 'The full redirect URL' diff --git a/dist/index.js b/dist/index.js index 9178333..a77372e 100644 --- a/dist/index.js +++ b/dist/index.js @@ -36437,6 +36437,9 @@ function normalizeConfig(raw = {}) { domain: get('domain') || DEFAULT_DOMAIN, jobTitle: get('job-title'), apiToken: get('api-token'), + // Only a literal "false" turns it off, so existing users keep the + // "Waiting for CircleCI ..." status they have always had + postPending: get('post-pending').toLowerCase() !== 'false', } } @@ -36562,6 +36565,12 @@ async function resolveStatus({payload, config, fetchFn = globalThis.fetch, log = log(`Ignoring context: ${payload.context}`) return null } + if (payload.state === 'pending' && !config.postPending) { + // Skipping these halves the statuses posted, and every status posted is + // itself a status event that comes back around (gh-27) + log('Ignoring pending status: post-pending is off') + return null + } if (!payload.target_url) { // Some status events carry no URL at all, so there is nothing to link to log('Ignoring status with no target_url') @@ -36623,6 +36632,7 @@ async function run({context = github_context, fetchFn = globalThis.fetch, getOct 'job-title': getInput('job-title', {required: false}), 'domain': getInput('domain'), 'api-token': getInput('api-token', {required: false}), + 'post-pending': getInput('post-pending', {required: false}), }) if (config.apiToken !== '') { // Keep the token out of the logs, including any future logging of it diff --git a/index.js b/index.js index 2ee021c..be05554 100644 --- a/index.js +++ b/index.js @@ -29,6 +29,7 @@ export async function run({context = github.context, fetchFn = globalThis.fetch, 'job-title': core.getInput('job-title', {required: false}), 'domain': core.getInput('domain'), 'api-token': core.getInput('api-token', {required: false}), + 'post-pending': core.getInput('post-pending', {required: false}), }) if (config.apiToken !== '') { // Keep the token out of the logs, including any future logging of it diff --git a/index.test.js b/index.test.js index 2562c08..4ccd38c 100644 --- a/index.test.js +++ b/index.test.js @@ -7,7 +7,7 @@ import { run } from './index.js' import { pickJob, legacyArtifactsUrl, redirectUrl, statusFor, fetchJson, resolveStatus } from './src/core.js' import { normalizeConfig } from './src/config.js' -const INPUTS = ['artifact-path', 'repo-token', 'api-token', 'circleci-jobs', 'job-title', 'domain'] +const INPUTS = ['artifact-path', 'repo-token', 'api-token', 'circleci-jobs', 'job-title', 'domain', 'post-pending'] const OUTPUT_FILE = path.join(fs.mkdtempSync(path.join(os.tmpdir(), 'redirector-')), 'output.txt') const ARTIFACT = {url: 'https://output.circle-artifacts.com/output/job/abc/artifacts/0/doc/other.html'} @@ -301,3 +301,24 @@ test('resolveStatus works without a logger', async () => { }) assert.equal(status.url, 'https://output.circle-artifacts.com/output/job/abc/artifacts/doc/index.html') }) + +test('post-pending: false skips the pending status entirely', async () => { + const {requests, url, status} = await runAction({ + inputs: {'post-pending': 'false'}, + payload: {state: 'pending'}, + }) + assert.deepEqual(requests, [], 'and costs no CircleCI call') + assert.equal(url, undefined) + assert.equal(status, null) +}) + +test('post-pending defaults to on, and only "false" turns it off', async () => { + for (const [value, expected] of [[undefined, 'pending'], ['true', 'pending'], ['False', null], ['false', null]]) { + const {status} = await runAction({ + inputs: value === undefined ? {} : {'post-pending': value}, + payload: {state: 'pending'}, + bodies: [{items: []}], + }) + assert.equal(status === null ? null : status.state, expected, `post-pending: ${value}`) + } +}) diff --git a/src/config.js b/src/config.js index 439e9a8..87b9a51 100644 --- a/src/config.js +++ b/src/config.js @@ -19,6 +19,9 @@ export function normalizeConfig(raw = {}) { domain: get('domain') || DEFAULT_DOMAIN, jobTitle: get('job-title'), apiToken: get('api-token'), + // Only a literal "false" turns it off, so existing users keep the + // "Waiting for CircleCI ..." status they have always had + postPending: get('post-pending').toLowerCase() !== 'false', } } diff --git a/src/core.js b/src/core.js index d052aa4..fd49c22 100644 --- a/src/core.js +++ b/src/core.js @@ -90,6 +90,12 @@ export async function resolveStatus({payload, config, fetchFn = globalThis.fetch log(`Ignoring context: ${payload.context}`) return null } + if (payload.state === 'pending' && !config.postPending) { + // Skipping these halves the statuses posted, and every status posted is + // itself a status event that comes back around (gh-27) + log('Ignoring pending status: post-pending is off') + return null + } if (!payload.target_url) { // Some status events carry no URL at all, so there is nothing to link to log('Ignoring status with no target_url') diff --git a/worker/index.js b/worker/index.js index 6816663..d478902 100644 --- a/worker/index.js +++ b/worker/index.js @@ -143,7 +143,9 @@ export async function handle(request, env, {fetchFn = globalThis.fetch, log = () if (raw === null) { return new Response(`ignored: no ${CONFIG_PATH}`, {status: 200}) } - const config = normalizeConfig(raw) + // The app never posts a pending status: it would double the webhook traffic + // it generates for no benefit the final status does not already provide + const config = {...normalizeConfig(raw), postPending: false} if (config.path === '') { return new Response('ignored: no artifact-path configured', {status: 200}) } diff --git a/worker/index.test.js b/worker/index.test.js index c959a55..464c0f5 100644 --- a/worker/index.test.js +++ b/worker/index.test.js @@ -277,3 +277,15 @@ test('a missing WEBHOOK_SECRET is a 401, not a crash', async () => { assert.deepEqual(seen, []) } }) + +test('the app never posts a pending status, whatever the config says', async () => { + for (const config of [CONFIG, CONFIG + 'post-pending: true\n']) { + clearCache() + const {fetchFn, seen} = backend({config}) + const response = await handle(webhook({...PAYLOAD, state: 'pending'}), ENV, {fetchFn}) + assert.equal(response.status, 200) + assert.match(await response.text(), /ignored/) + assert.ok(!seen.some((r) => r.url.includes('/statuses/')), 'nothing posted') + assert.ok(!seen.some((r) => r.url.includes('/artifacts')), 'and no CircleCI call') + } +}) From cba19ccea79b413dcdb53bdfd66848e2452038fd Mon Sep 17 00:00:00 2001 From: Eric Larson Date: Tue, 28 Jul 2026 16:00:49 -0400 Subject: [PATCH 5/7] ENH: Drop duplicate status posts in the app CircleCI delivered the same build_docs status twice during live testing on LABSN/expyfun, and the app posted an identical status for each. GitHub only shows the latest per context, so it is invisible in the UI, but it doubles both the posts and the status events they generate (gh-27). Keyed on repo, sha, context, state and the resolved URL, so a re-run that produces different artifacts still posts. Best-effort: two simultaneous duplicates can both miss, and a cold isolate forgets. Co-Authored-By: Claude Opus 5 --- README.md | 3 +++ worker/index.js | 23 +++++++++++++++++++++++ worker/index.test.js | 35 ++++++++++++++++++++++++++++++++++- 3 files changed, 60 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index a8028a6..32e9c07 100644 --- a/README.md +++ b/README.md @@ -120,6 +120,9 @@ Differences from the action, by design: event, and the final status says everything the pending one did. - Public CircleCI projects only: a private project needs an `api-token`, which would mean storing each repo's CircleCI token server-side. +- Duplicate deliveries are dropped: CircleCI sometimes reports the same job + status twice, and posting an identical status twice is invisible in the UI + but doubles the events it generates. The action is not going away; the App is a second way to run the same code. diff --git a/worker/index.js b/worker/index.js index d478902..9270ffa 100644 --- a/worker/index.js +++ b/worker/index.js @@ -21,6 +21,7 @@ export const CONFIG_PATH = '.github/circleci-artifacts.yml' // correctness-critical: a cold isolate simply fetches again. export const TOKEN_TTL_MS = 50 * 60 * 1000 // installation tokens last an hour export const CONFIG_TTL_MS = 10 * 60 * 1000 +export const DEDUPE_TTL_MS = 5 * 60 * 1000 const cache = new Map() export function clearCache() { @@ -44,6 +45,21 @@ export function cached(key, ttl, produce, now = Date.now) { }) return value } +// True if this exact key was seen recently, recording it if not. CircleCI +// sometimes delivers the same status twice, and each delivery would otherwise +// post an identical status: invisible in the UI, since GitHub only shows the +// latest per context, but it doubles both the posts and the status events they +// generate. Best-effort by design -- two simultaneous duplicates can still both +// miss, and a cold isolate forgets everything. +export function seenRecently(key, ttl, now = Date.now) { + const hit = cache.get(key) + if (hit && hit.expires > now()) { + return true + } + cache.set(key, {value: true, expires: now() + ttl}) + return false +} + const API = 'https://api.github.com' const UA = {'user-agent': 'circleci-artifacts-redirector-app', 'accept': 'application/vnd.github+json'} @@ -155,6 +171,13 @@ export async function handle(request, env, {fetchFn = globalThis.fetch, log = () return new Response('ignored: not a watched job', {status: 200}) } + // The URL is part of the key, so a re-run that produces different artifacts + // still posts, while a duplicate delivery of the same event does not + const key = `posted:${repo.full_name}:${payload.sha}:${status.context}:${status.state}:${status.url}` + if (seenRecently(key, DEDUPE_TTL_MS, now)) { + return new Response(`ignored: already posted ${status.state}`, {status: 200}) + } + const response = await fetchFn(`${API}/repos/${repo.full_name}/statuses/${payload.sha}`, { method: 'POST', headers: {...UA, authorization: `Bearer ${token}`}, diff --git a/worker/index.test.js b/worker/index.test.js index 464c0f5..813c250 100644 --- a/worker/index.test.js +++ b/worker/index.test.js @@ -1,7 +1,7 @@ import test, { beforeEach } from 'node:test' import assert from 'node:assert/strict' import crypto from 'node:crypto' -import worker, { handle, verifySignature, mintToken, readConfig, clearCache, CONFIG_PATH, TOKEN_TTL_MS, CONFIG_TTL_MS } from './index.js' +import worker, { handle, verifySignature, mintToken, readConfig, clearCache, CONFIG_PATH, TOKEN_TTL_MS, CONFIG_TTL_MS, DEDUPE_TTL_MS } from './index.js' import { parseConfig, normalizeConfig } from '../src/config.js' beforeEach(clearCache) @@ -289,3 +289,36 @@ test('the app never posts a pending status, whatever the config says', async () assert.ok(!seen.some((r) => r.url.includes('/artifacts')), 'and no CircleCI call') } }) + +test('a duplicate delivery does not post the status twice', async () => { + const {fetchFn, seen} = backend() + const first = await handle(webhook(PAYLOAD), ENV, {fetchFn}) + const second = await handle(webhook(PAYLOAD), ENV, {fetchFn}) + + assert.match(await first.text(), /^posted success/) + assert.match(await second.text(), /already posted/) + assert.equal(seen.filter((r) => r.url.includes('/statuses/')).length, 1, 'posted once') +}) + +test('but a different result for the same commit still posts', async () => { + const {fetchFn: pendingFetch} = backend() + await handle(webhook(PAYLOAD), ENV, {fetchFn: pendingFetch}) + + // same sha and context, different artifacts (e.g. a re-run) -> must post + const other = {url: 'https://output.circle-artifacts.com/output/job/zzz/artifacts/0/doc/other.html'} + const {fetchFn, seen} = backend({artifacts: {items: [other]}}) + const response = await handle(webhook(PAYLOAD), ENV, {fetchFn}) + assert.match(await response.text(), /^posted success/) + assert.equal(seen.filter((r) => r.url.includes('/statuses/')).length, 1) +}) + +test('the dedupe window expires', async () => { + const {fetchFn, seen} = backend() + let clock = 5_000_000 + const now = () => clock + await handle(webhook(PAYLOAD), ENV, {fetchFn, now}) + clock += DEDUPE_TTL_MS + 1 + const response = await handle(webhook(PAYLOAD), ENV, {fetchFn, now}) + assert.match(await response.text(), /^posted success/) + assert.equal(seen.filter((r) => r.url.includes('/statuses/')).length, 2) +}) From c789248bf9361e080de6d4a55ddb87a70e2b2c76 Mon Sep 17 00:00:00 2001 From: Eric Larson Date: Tue, 28 Jul 2026 16:07:11 -0400 Subject: [PATCH 6/7] DOC: Add CLAUDE.md Working notes for agents: the shared-core layout, the commands, and the gotchas that cost real debugging time (the Circle-Token 401, the status semantics from gh-57, why exact artifact-path matching was declined, why the app must read config from the default branch, and the fork-is-a- CircleCI-project trap). Co-Authored-By: Claude Opus 5 --- CLAUDE.md | 130 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 130 insertions(+) create mode 100644 CLAUDE.md diff --git a/CLAUDE.md b/CLAUDE.md new file mode 100644 index 0000000..5d47525 --- /dev/null +++ b/CLAUDE.md @@ -0,0 +1,130 @@ +# CLAUDE.md + +Notes for agents working on this repo. User-facing docs live in `README.md`; +this file is the working knowledge that is easy to get wrong. + +## What this is + +Two front ends over one core, for putting a link to a CircleCI artifact into a +GitHub commit status: + +| File | Role | +|---|---| +| `src/core.js` | all the logic; runtime-neutral (global `fetch` only, nothing from `node:`) | +| `src/config.js` | option names, defaults and the config-file parser, shared by both | +| `index.js` | GitHub Action entry point (`@actions/core`, `@actions/github`) | +| `worker/index.js` | GitHub App entry point: a Cloudflare Worker handling `status` webhooks | +| `dist/index.js` | the bundle the action actually runs; **committed**, built by `ncc` | + +Keep logic in `src/`. Anything added to only one front end will drift; that is +the whole reason the split exists. + +## Commands + +```bash +npm test # eslint + node --test +npm run coverage # the same, with a hard 100% line/branch/function floor +npx ncc build index.js -o dist # after ANY change to index.js or src/ +pre-commit run --all-files # yamllint + eslint, as CI runs them +npx wrangler deploy # after ANY change to worker/ or src/ +``` + +CI enforces 100% coverage. New code needs tests, or `/* node:coverage +disable */` with a reason (see the entry-point guard in `index.js`). + +## Conventions + +- **No semicolons, single quotes**, `eqeqeq` with `{null: 'ignore'}` — enforced + by `eslint.config.mjs`, all autofixable with `npx eslint . --fix`. +- **Rebuild `dist/`** in the same commit as any `index.js`/`src/` change, or + the action ships stale code. autofix.ci also does this on PRs. +- **`.pre-commit-config.yaml` pins ESLint separately from `package.json`** and + dependabot only updates the latter. Bump both together. +- Style-only commits go in `.git-blame-ignore-revs`. +- Node version comes from `.nvmrc` (CircleCI orb and `setup-node` both read it). + +## Testing style + +`node:test`, no framework. The action is tested by setting real `INPUT_*` env +vars and injecting `fetchFn`/`getOctokit`; the Worker by building a real +`Request` and injecting `fetchFn`. Both use the real `@actions/core` and real +Web Crypto — the JWT test signs with a generated key and verifies with +`node:crypto`, so it would be accepted by GitHub. + +**Mutation-test any fix**: revert it alone and confirm the new test fails. This +caught a test that passed with *and* without the fix (an HTTP-error test that +only asserted "the job failed", when the old code also failed, just with a +useless message). + +## Hard-won gotchas + +Things that cost real debugging time. Do not undo these. + +- **Never send `Circle-Token` unless a token was supplied.** CircleCI answers + `401` to a bogus token even on public projects, while no header at all is + `200`. Sending the literal string `"null"` broke every tokenless public repo + (gh-119). +- **The status reports the link, not the build** (gh-57): green when artifacts + exist, red when they do not, regardless of whether CircleCI passed. +- **Do not add exact `artifact-path` matching.** It was proposed and declined: + CircleCI lists only files, so anyone whose path is a directory (`0/dev/`, + relying on an index redirect) would go permanently red. A broken link is the + lesser evil. Revisiting it would also need `next_page_token` paging. +- **The app must read config from the default branch.** Reading it from the + event's ref would let a forked PR point `domain:` at a host it controls and + have us post a trusted-looking link to it. Verified live with a fork PR whose + branch config said `SHOULD NOT APPEAR`. +- **`on: status` cannot be filtered** — no `types`, no branches, and the + workflow must exist on the default branch. Job-level `if` skips the work but + the run entry is still created, which is gh-27. Every status the action posts + is itself a `status` event, so it triggers its own workflow again; that is why + `post-pending` exists. +- **A fork that is itself a followed CircleCI project suppresses upstream + builds.** CircleCI builds it in the fork's project and never creates a + `pull/N` pipeline in the parent, so the upstream PR shows no status while + every setting looks correct. Check + `/api/v1.1/project/github///settings` for `build-fork-prs`. +- **Never suggest installing the CircleCI GitHub App as a fix for forked PRs** + — App pipelines are *never* built on forks, so it makes this strictly worse. + The OAuth integration is the one that supports them. + +## The GitHub App + +Deploy: `npx wrangler deploy`. Secrets: `APP_ID`, `PRIVATE_KEY`, +`WEBHOOK_SECRET` via `wrangler secret put`. + +- `PRIVATE_KEY` must be **PKCS#8** (`openssl pkcs8 -topk8 -nocrypt …`); Web + Crypto cannot import the PKCS#1 file GitHub gives you. +- Upload `WEBHOOK_SECRET` with `printf '%s'`, never `< file` — a trailing + newline makes every delivery `401`. +- Token (50 min), config (10 min) and posted-status dedupe (5 min) are cached in + an isolate-level `Map`. All best-effort: a cold isolate just refetches, and a + duplicate can slip through. Nothing is correctness-critical. +- Repos with no `.github/circleci-artifacts.yml` are inert, so a stale + installation posts nothing. +- Responses are the diagnostic surface: the App's Advanced → Recent Deliveries + tab shows exactly which stage a delivery reached. + +## Where things stand (2026-07-28) + +The App prototype is merged/being merged from `app-prototype`. It is **running +in production for `LABSN/expyfun`**, which removed its workflow — but on a +*personal* Cloudflare account and a personally-owned App registration, not +scientific-python infrastructure. + +Next steps, roughly in order: + +1. More repos: `scikit-image/scikit-image` and `braindecode/braindecode` + already have the App installed (since 2019) and only need a config file. + Then MNE-Python and SciPy. +2. Hand over to scientific-python: App ownership transfers preserve + installations, and the Worker is stateless, so it is `wrangler deploy` + + three secrets + one webhook URL change. Stefan van der Walt (stefanv) runs + the org's existing Cloudflare Worker (`scientific-python/circleci-proxy`); + he and Jarrod Millman are the org owners. +3. Measured load for scikit-learn + MNE-Python + SciPy combined: ~8,200 + deliveries/week, about 1.2% of the Workers free tier. + +Not supported by the App, by design: private CircleCI projects (would need +server-side token storage) and the `url` output (no workflow step to consume +it). The action remains the answer for both, and is not going away. From bcfbdea3e1b2dcd0e111522797bf506928649df2 Mon Sep 17 00:00:00 2001 From: Eric Larson Date: Tue, 28 Jul 2026 16:25:25 -0400 Subject: [PATCH 7/7] ENH: Accept the usual config file spellings Migrating means git mv-ing the old workflow, which is called circle_artifacts.yml in SciPy and circle-artifacts.yml in MNE-Python, so requiring one exact name makes the migration silently no-op. Find the config by listing .github/ and matching circle(ci)?[-_]artifacts.ya?ml, preferring the documented spelling when a repo has several. Costs one extra API call when a config exists, which the 10 minute cache absorbs; a repo with no config now costs one call rather than one per candidate name. Co-Authored-By: Claude Opus 5 --- CLAUDE.md | 8 ++++-- README.md | 5 ++++ worker/index.js | 32 +++++++++++++++++---- worker/index.test.js | 67 ++++++++++++++++++++++++++++++++++++-------- 4 files changed, 92 insertions(+), 20 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 5d47525..9f57b4d 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -100,8 +100,12 @@ Deploy: `npx wrangler deploy`. Secrets: `APP_ID`, `PRIVATE_KEY`, - Token (50 min), config (10 min) and posted-status dedupe (5 min) are cached in an isolate-level `Map`. All best-effort: a cold isolate just refetches, and a duplicate can slip through. Nothing is correctness-critical. -- Repos with no `.github/circleci-artifacts.yml` are inert, so a stale - installation posts nothing. +- Repos with no config file are inert, so a stale installation posts nothing. +- The config file is found by **listing `.github/` and matching + `CONFIG_NAME`** (`circle(ci)?[-_]artifacts.ya?ml`) rather than fetching one + fixed path: people migrate by `git mv`-ing their workflow, which is called + `circle_artifacts.yml` in SciPy and MNE-Python. Costs one extra API call when + a config exists, cached for 10 minutes. - Responses are the diagnostic surface: the App's Advanced → Recent Deliveries tab shows exactly which stage a delivery reached. diff --git a/README.md b/README.md index 32e9c07..33d9dea 100644 --- a/README.md +++ b/README.md @@ -106,6 +106,11 @@ circleci-jobs: build_docs job-title: Check the rendered docs here! ``` +Since migrating usually means `git mv`-ing the old workflow, the underscore +and `circle` spellings are accepted too — `circle-artifacts.yml`, +`circle_artifacts.yml`, `circleci_artifacts.yml`, and the `.yaml` versions of +each all work. + The config is always read from the **default branch**, so a pull request (including one from a fork) cannot change where the link points. diff --git a/worker/index.js b/worker/index.js index 9270ffa..ab857f8 100644 --- a/worker/index.js +++ b/worker/index.js @@ -15,7 +15,12 @@ import { normalizeConfig, parseConfig } from '../src/config.js' import { resolveStatus } from '../src/core.js' -export const CONFIG_PATH = '.github/circleci-artifacts.yml' +export const CONFIG_DIR = '.github' +export const CANONICAL_CONFIG = 'circleci-artifacts.yml' +// People migrate by `git mv`-ing their old workflow, which is variously called +// circle_artifacts.yml (SciPy), circle-artifacts.yml, circleci_artifacts.yml… +// so accept any of those spellings rather than making them rename the file. +export const CONFIG_NAME = /^circle(ci)?[-_]artifacts\.ya?ml$/ // Cloudflare reuses an isolate across many requests, so a plain Map removes // most of the token and config traffic without needing KV. Nothing here is // correctness-critical: a cold isolate simply fetches again. @@ -115,13 +120,28 @@ export async function mintToken({appId, privateKey, installationId, fetchFn = gl // forked PR could point `domain` at a host it controls and have us post a // trusted-looking link to it. export async function readConfig(fetchFn, repo, token) { - const url = `${API}/repos/${repo.full_name}/contents/${CONFIG_PATH}?ref=${repo.default_branch}` - const response = await fetchFn(url, {headers: {...UA, authorization: `Bearer ${token}`}}) - if (response.status === 404) { + const headers = {...UA, authorization: `Bearer ${token}`} + const ref = `?ref=${repo.default_branch}` + const listing = await fetchFn(`${API}/repos/${repo.full_name}/contents/${CONFIG_DIR}${ref}`, {headers}) + if (listing.status === 404) { + return null // no .github directory at all + } + if (!listing.ok) { + throw new Error(`Could not list ${CONFIG_DIR}: ${listing.status}`) + } + const entries = await listing.json() + if (!Array.isArray(entries)) { + return null + } + const files = entries.filter((entry) => entry.type === 'file' && CONFIG_NAME.test(entry.name)) + // Prefer the documented spelling when a repo somehow has several + const file = files.find((entry) => entry.name === CANONICAL_CONFIG) ?? files[0] + if (file === undefined) { return null } + const response = await fetchFn(`${API}/repos/${repo.full_name}/contents/${file.path}${ref}`, {headers}) if (!response.ok) { - throw new Error(`Could not read ${CONFIG_PATH}: ${response.status}`) + throw new Error(`Could not read ${file.path}: ${response.status}`) } const {content} = await response.json() return parseConfig(atob(content.replace(/\s/g, ''))) @@ -157,7 +177,7 @@ export async function handle(request, env, {fetchFn = globalThis.fetch, log = () `config:${repo.full_name}@${repo.default_branch}`, CONFIG_TTL_MS, () => readConfig(fetchFn, repo, token), now) if (raw === null) { - return new Response(`ignored: no ${CONFIG_PATH}`, {status: 200}) + return new Response(`ignored: no ${CONFIG_DIR}/${CANONICAL_CONFIG}`, {status: 200}) } // The app never posts a pending status: it would double the webhook traffic // it generates for no benefit the final status does not already provide diff --git a/worker/index.test.js b/worker/index.test.js index 813c250..16ffb2d 100644 --- a/worker/index.test.js +++ b/worker/index.test.js @@ -1,7 +1,7 @@ import test, { beforeEach } from 'node:test' import assert from 'node:assert/strict' import crypto from 'node:crypto' -import worker, { handle, verifySignature, mintToken, readConfig, clearCache, CONFIG_PATH, TOKEN_TTL_MS, CONFIG_TTL_MS, DEDUPE_TTL_MS } from './index.js' +import worker, { handle, verifySignature, mintToken, readConfig, clearCache, CONFIG_DIR, CANONICAL_CONFIG, CONFIG_NAME, TOKEN_TTL_MS, CONFIG_TTL_MS, DEDUPE_TTL_MS } from './index.js' import { parseConfig, normalizeConfig } from '../src/config.js' beforeEach(clearCache) @@ -42,17 +42,23 @@ function webhook(payload, {event = 'status', secret = SECRET, method = 'POST'} = // Fake GitHub + CircleCI. Returns the requests it saw so tests can assert on // what would have been posted. -function backend({config = CONFIG, artifacts = {items: [ARTIFACT]}, statusCode = 201} = {}) { +function backend({config = CONFIG, names = [CANONICAL_CONFIG], artifacts = {items: [ARTIFACT]}, statusCode = 201} = {}) { const seen = [] const fetchFn = async (url, options = {}) => { seen.push({url, method: options.method ?? 'GET', body: options.body}) if (url.endsWith('/access_tokens')) { return new Response(JSON.stringify({token: 'ghs_installation'}), {status: 201}) } - if (url.includes(`/contents/${CONFIG_PATH}`)) { - return config === null - ? new Response('{}', {status: 404}) - : new Response(JSON.stringify({content: btoa(config)}), {status: 200}) + if (url.includes(`/contents/${CONFIG_DIR}?`)) { + if (config === null) { + return new Response('[]', {status: 200}) // .github exists, no config in it + } + const listing = [{type: 'dir', name: 'workflows', path: '.github/workflows'}] + .concat(names.map((name) => ({type: 'file', name, path: `${CONFIG_DIR}/${name}`}))) + return new Response(JSON.stringify(listing), {status: 200}) + } + if (url.includes(`/contents/${CONFIG_DIR}/`)) { + return new Response(JSON.stringify({content: btoa(config)}), {status: 200}) } if (url.includes('/artifacts')) { return new Response(JSON.stringify(artifacts), {status: 200}) @@ -84,8 +90,9 @@ test('posts a status for a CircleCI event', async () => { test('reads the config from the default branch, not the event', async () => { const {fetchFn, seen} = backend() await handle(webhook(PAYLOAD), ENV, {fetchFn}) - const read = seen.find((r) => r.url.includes(`/contents/${CONFIG_PATH}`)) - assert.match(read.url, /\?ref=main$/, 'pinned to the default branch') + for (const read of seen.filter((r) => r.url.includes('/contents/'))) { + assert.match(read.url, /\?ref=main$/, 'pinned to the default branch') + } }) test('rejects a bad signature before doing anything', async () => { @@ -148,7 +155,7 @@ test('surfaces failures to read config or mint a token', async () => { ? new Response(JSON.stringify({token: 't'}), {status: 201}) : new Response('{}', {status: 500}) await assert.rejects( - () => handle(webhook(PAYLOAD), ENV, {fetchFn: badConfig}), new RegExp(`Could not read ${CONFIG_PATH}: 500`)) + () => handle(webhook(PAYLOAD), ENV, {fetchFn: badConfig}), new RegExp(`Could not list ${CONFIG_DIR}: 500`)) }) test('verifySignature rejects malformed headers', async () => { @@ -182,9 +189,21 @@ test('mintToken signs a real RS256 JWT', async () => { assert.ok(verified, 'the JWT signature checks out') }) -test('readConfig returns null when the file is missing', async () => { - const fetchFn = async () => new Response('{}', {status: 404}) - assert.equal(await readConfig(fetchFn, {full_name: 'a/b', default_branch: 'main'}, 't'), null) +test('readConfig returns null when there is nothing to read', async () => { + const repo = {full_name: 'a/b', default_branch: 'main'} + const missing = async () => new Response('{}', {status: 404}) + assert.equal(await readConfig(missing, repo, 't'), null, 'no .github directory') + + const empty = async () => new Response('[]', {status: 200}) + assert.equal(await readConfig(empty, repo, 't'), null, 'no matching file') + + const notADir = async () => new Response('{"type":"file"}', {status: 200}) + assert.equal(await readConfig(notADir, repo, 't'), null, '.github is somehow a file') + + const unreadable = async (url) => url.includes(`${CONFIG_DIR}?`) + ? new Response(JSON.stringify([{type: 'file', name: CANONICAL_CONFIG, path: `${CONFIG_DIR}/${CANONICAL_CONFIG}`}]), {status: 200}) + : new Response('{}', {status: 500}) + await assert.rejects(() => readConfig(unreadable, repo, 't'), /Could not read .github\/circleci-artifacts.yml: 500/) }) test('parseConfig handles the shapes a migrated workflow produces', () => { @@ -322,3 +341,27 @@ test('the dedupe window expires', async () => { assert.match(await response.text(), /^posted success/) assert.equal(seen.filter((r) => r.url.includes('/statuses/')).length, 2) }) + +test('the config file may use any of the usual spellings', async () => { + for (const name of ['circleci-artifacts.yml', 'circleci_artifacts.yml', 'circle-artifacts.yml', + 'circle_artifacts.yml', 'circle_artifacts.yaml']) { + clearCache() + const {fetchFn, seen} = backend({names: [name]}) + const response = await handle(webhook(PAYLOAD), ENV, {fetchFn}) + assert.match(await response.text(), /^posted success/, name) + assert.ok(seen.some((r) => r.url.includes(`/contents/${CONFIG_DIR}/${name}`)), `read ${name}`) + } +}) + +test('unrelated files in .github are not mistaken for config', () => { + for (const name of ['dependabot.yml', 'release.yml', 'circleci-artifacts.txt', 'my-circle-artifacts.yml']) { + assert.equal(CONFIG_NAME.test(name), false, name) + } +}) + +test('the documented spelling wins when a repo has several', async () => { + const {fetchFn, seen} = backend({names: ['circle_artifacts.yml', CANONICAL_CONFIG]}) + await handle(webhook(PAYLOAD), ENV, {fetchFn}) + assert.ok(seen.some((r) => r.url.includes(`/contents/${CONFIG_DIR}/${CANONICAL_CONFIG}`))) + assert.ok(!seen.some((r) => r.url.includes('circle_artifacts.yml'))) +})