diff --git a/CLAUDE.md b/CLAUDE.md index e67af46..8d94df5 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -20,6 +20,27 @@ GitHub commit status: Keep logic in `src/`. Anything added to only one front end will drift; that is the whole reason the split exists. +### The two front ends do not behave the same + +Shared logic does not mean shared behaviour, and advice that is right for one +is routinely wrong for the other. **Establish which front end a repo uses +before saying anything about what it will post** — a repo is on the App if +`.github/circle(ci)?[-_]artifacts.ya?ml` exists and the `on: status` workflow +is gone (see "Where things stand" for who is where). + +| | Action | App | +|---|---|---| +| Pending "Waiting for CircleCI ..." status | posted, unless `post-pending: 'false'` | **never**, unconditionally: `handle` returns on `state: pending` and forces `postPending: false` | +| Options come from | `with:` in the workflow | `.github/circleci-artifacts.yml` on the **default branch** | +| `url` output | yes | no — no step to consume it | +| Private CircleCI projects (`api-token`) | yes | no | +| Duplicate CircleCI deliveries | posted twice | deduped for 5 min | +| Where a run shows up | a workflow run per `status` event | nowhere; the App's Recent Deliveries tab | + +The pending row is the one that bites: any reasoning of the form "a status +already posted earlier will be left behind" is action-only, and every caveat +about `post-pending` is too. + ## Commands ```bash @@ -77,6 +98,13 @@ Things that cost real debugging time. Do not undo these. (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. + `no-artifact-state` (`failure` default / `success` / `skip`) overrides the + "red when they do not" half, for repos where a build that uploads nothing is + expected (scipy's `[skip circle]`). There is deliberately no grey: a commit + status has no neutral state, and `pending` would be a yellow one that never + resolves. Doing it properly would mean posting **check runs** instead, i.e. a + `checks: write` permission for action users and an App permission change that + every installation has to re-approve. - **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 @@ -258,25 +286,37 @@ deploy but needs its own `CLOUDFLARE_ANALYTICS_TOKEN` — a repo secret with Account Analytics: Read **only**, never the deploy token, which can write to production. -## Where things stand (2026-07-28) +## Where things stand (2026-08-05) + +The App is in production, but on a *personal* Cloudflare account and a +personally-owned App registration, not scientific-python infrastructure. + +Who is on which front end — this is the first thing to establish before +answering anything about behaviour, per "The two front ends do not behave the +same": + +| Repo | Front end | Evidence | +|---|---|---| +| `LABSN/expyfun` | App | `.github/circleci-artifacts.yml`, workflow removed | +| `scipy/scipy` | App | `.github/circle-artifacts.yml`, workflow removed | +| `scikit-image/scikit-image` | App | `.github/circleci-artifacts.yaml`, workflow removed | +| `mne-tools/mne-python` | App | `.github/circle-artifacts.yml`, workflow removed | +| `braindecode/braindecode` | App | `.github/circleci-artifacts.yaml`; its `push-artifact-to-circleci.yml` is unrelated (it triggers a CircleCI pipeline) | +| `scikit-learn/scikit-learn` | Action | `.github/workflows/artifact-redirector.yml`, no config file | -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. +Verify from the repo rather than trusting this table, which goes stale: config +file present + `on: status` workflow gone means the App. Installation state +itself is only visible from the App's own settings. 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 +1. Hand over to scientific-python: App ownership transfers preserve installations, and the Worker is stateless, so it is `wrangler deploy` + three Worker secrets + two repo secrets (see "Deploying from CI") + 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 +2. 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 diff --git a/README.md b/README.md index 33d9dea..c111057 100644 --- a/README.md +++ b/README.md @@ -67,6 +67,16 @@ 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)). +- Use `no-artifact-state` to change what a job that uploaded nothing posts, + which is worth doing if that is an expected outcome for your repo — a commit + that deliberately skips the CircleCI docs build, say. It defaults to + `'failure'` (red); `'success'` posts a green status linking to the CircleCI + job instead, and `'skip'` posts no status at all (and sets no `url` output). + There is no grey option because a GitHub commit status has no neutral state: + it is red, green, or yellow, and the yellow one would sit unresolved forever. + With `'skip'`, note that a "Waiting for CircleCI ..." status already posted + earlier stays yellow, so `post-pending: 'false'` usually belongs with it. + (Not a concern under the App, which never posts a pending status at all.) - 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 diff --git a/action.yml b/action.yml index 0a95732..47d4929 100644 --- a/action.yml +++ b/action.yml @@ -38,6 +38,14 @@ inputs: `on: status` trigger produces). required: false default: 'true' + no-artifact-state: + description: | + What to post when the job uploaded no artifacts: 'failure' (red, the + default), 'success' (green, linking to the CircleCI job) or 'skip' to + post no status at all. Useful when a job that uploads nothing is an + expected outcome, e.g. a '[skip circle]' commit. + required: false + default: 'failure' outputs: url: description: 'The full redirect URL' diff --git a/dist/index.js b/dist/index.js index fc7b733..e103694 100644 --- a/dist/index.js +++ b/dist/index.js @@ -36424,6 +36424,19 @@ var external_node_url_ = __nccwpck_require__(3136); const DEFAULT_JOBS = 'build_docs,doc,build' const DEFAULT_DOMAIN = 'output.circle-artifacts.com' +// What to post when the job uploaded nothing. `pending` is not offered on +// purpose: a commit status has no neutral/grey state, and the yellow one would +// sit there unresolved forever. +const NO_ARTIFACT_STATES = ['failure', 'success', 'skip'] + +function noArtifactState(value) { + const state = value.toLowerCase() || 'failure' + if (!NO_ARTIFACT_STATES.includes(state)) { + throw new Error(`no-artifact-state must be one of ${NO_ARTIFACT_STATES.join(', ')}, got '${value}'`) + } + return state +} + // 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 = {}) { @@ -36441,6 +36454,7 @@ function normalizeConfig(raw = {}) { // 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', + noArtifactState: noArtifactState(get('no-artifact-state')), } } @@ -36513,14 +36527,21 @@ function redirectUrl(first, path, domain, fallback) { // 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. -function statusFor(payloadState, hasArtifacts, path) { +// +// Returns null when there are no artifacts and the repo asked for no status at +// all, which is a legitimate outcome rather than a red one for a job that is +// expected to upload nothing (a "[skip circle]" commit, say). +function statusFor(payloadState, hasArtifacts, path, noArtifactState = 'failure') { 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'} + if (noArtifactState === 'skip') { + return null + } + return {state: noArtifactState, description: 'No artifacts found'} } // Fail loudly on a non-2xx response. Without this a 404 or a rate limit @@ -36660,12 +36681,15 @@ async function resolveStatus({payload, config, fetchFn = globalThis.fetch, log = const first = await firstArtifactUrl(fetchFn, artifactsUrl, {headers}) log(`First artifact: ${first}`) - const url = redirectUrl(first, config.path, config.domain, payload.target_url) - const {state, description} = statusFor(payload.state, first != null, config.path) + const status = statusFor(payload.state, first != null, config.path, config.noArtifactState) + if (status === null) { + log('Ignoring: no artifacts, and no-artifact-state is "skip"') + return null + } return { - url, - state, - description, + url: redirectUrl(first, config.path, config.domain, payload.target_url), + state: status.state, + description: status.description, context: config.jobTitle || `${payload.context} artifact`, } } @@ -36715,6 +36739,7 @@ async function run({context = github_context, fetchFn = globalThis.fetch, getOct 'domain': getInput('domain'), 'api-token': getInput('api-token', {required: false}), 'post-pending': getInput('post-pending', {required: false}), + 'no-artifact-state': getInput('no-artifact-state', {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 fd83b6f..0edba95 100644 --- a/index.js +++ b/index.js @@ -42,6 +42,7 @@ export async function run({context = github.context, fetchFn = globalThis.fetch, 'domain': core.getInput('domain'), 'api-token': core.getInput('api-token', {required: false}), 'post-pending': core.getInput('post-pending', {required: false}), + 'no-artifact-state': core.getInput('no-artifact-state', {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 2b8aae9..4ec4713 100644 --- a/index.test.js +++ b/index.test.js @@ -7,7 +7,7 @@ import { debug, 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', 'post-pending'] +const INPUTS = ['artifact-path', 'repo-token', 'api-token', 'circleci-jobs', 'job-title', 'domain', 'post-pending', 'no-artifact-state'] 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'} @@ -237,6 +237,10 @@ test('statusFor', () => { assert.deepEqual(statusFor('pending', false, 'p'), {state: 'pending', description: 'Waiting for CircleCI ...'}) assert.deepEqual(statusFor('failure', true, 'p'), {state: 'success', description: 'Link to p'}) assert.deepEqual(statusFor('success', false, 'p'), {state: 'failure', description: 'No artifacts found'}) + assert.deepEqual(statusFor('success', false, 'p', 'success'), {state: 'success', description: 'No artifacts found'}) + assert.equal(statusFor('success', false, 'p', 'skip'), null) + assert.deepEqual(statusFor('pending', false, 'p', 'skip'), {state: 'pending', description: 'Waiting for CircleCI ...'}, + 'a pending status is unaffected: nothing is known about the artifacts yet') }) // Tier 1 fixes @@ -490,3 +494,41 @@ test('post-pending defaults to on, and only "false" turns it off', async () => { assert.equal(status === null ? null : status.state, expected, `post-pending: ${value}`) } }) + +// A job that uploads nothing is red by default, but it is an expected outcome +// for a repo that skips its CircleCI build on demand (scipy's "[skip circle]") +test('no-artifact-state: success links to the job in green instead', async () => { + const {url, status} = await runAction({ + inputs: {'no-artifact-state': 'success'}, + bodies: [{items: []}], + }) + assert.equal(url, 'https://circleci.com/gh/scientific-python/circleci-artifacts-redirector-action/94') + assert.equal(status.state, 'success') + assert.equal(status.description, 'No artifacts found') + assert.equal(status.target_url, url, 'still points at the job, not a link that 404s') +}) + +test('no-artifact-state: skip posts nothing at all', async () => { + const {url, status} = await runAction({ + inputs: {'no-artifact-state': 'SKIP'}, // and the value is case-insensitive + bodies: [{items: []}], + }) + assert.equal(status, null) + assert.equal(url, undefined, 'and there is no artifact URL to output either') + assert.equal(process.exitCode, 0, 'ignored, not failed') +}) + +test('no-artifact-state only applies when there are no artifacts', async () => { + for (const value of ['success', 'skip']) { + const {status} = await runAction({inputs: {'no-artifact-state': value}, bodies: [{items: [ARTIFACT]}]}) + assert.equal(status.description, 'Link to doc/index.html', `no-artifact-state: ${value}`) + } +}) + +test('an unknown no-artifact-state fails the job rather than guessing', async () => { + const {status, out} = await runAction({inputs: {'no-artifact-state': 'neutral'}, bodies: [{items: []}]}) + assert.equal(status, null) + assert.equal(process.exitCode, 1) // core.setFailed() + process.exitCode = 0 + assert.match(out, /::error::no-artifact-state must be one of failure, success, skip, got 'neutral'/) +}) diff --git a/src/config.js b/src/config.js index 87b9a51..8941ab9 100644 --- a/src/config.js +++ b/src/config.js @@ -5,6 +5,19 @@ export const DEFAULT_JOBS = 'build_docs,doc,build' export const DEFAULT_DOMAIN = 'output.circle-artifacts.com' +// What to post when the job uploaded nothing. `pending` is not offered on +// purpose: a commit status has no neutral/grey state, and the yellow one would +// sit there unresolved forever. +export const NO_ARTIFACT_STATES = ['failure', 'success', 'skip'] + +function noArtifactState(value) { + const state = value.toLowerCase() || 'failure' + if (!NO_ARTIFACT_STATES.includes(state)) { + throw new Error(`no-artifact-state must be one of ${NO_ARTIFACT_STATES.join(', ')}, got '${value}'`) + } + return state +} + // 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 = {}) { @@ -22,6 +35,7 @@ export function normalizeConfig(raw = {}) { // 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', + noArtifactState: noArtifactState(get('no-artifact-state')), } } diff --git a/src/core.js b/src/core.js index 5363b9d..adb09e8 100644 --- a/src/core.js +++ b/src/core.js @@ -37,14 +37,21 @@ export function redirectUrl(first, path, domain, fallback) { // 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) { +// +// Returns null when there are no artifacts and the repo asked for no status at +// all, which is a legitimate outcome rather than a red one for a job that is +// expected to upload nothing (a "[skip circle]" commit, say). +export function statusFor(payloadState, hasArtifacts, path, noArtifactState = 'failure') { 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'} + if (noArtifactState === 'skip') { + return null + } + return {state: noArtifactState, description: 'No artifacts found'} } // Fail loudly on a non-2xx response. Without this a 404 or a rate limit @@ -184,12 +191,15 @@ export async function resolveStatus({payload, config, fetchFn = globalThis.fetch const first = await firstArtifactUrl(fetchFn, artifactsUrl, {headers}) log(`First artifact: ${first}`) - const url = redirectUrl(first, config.path, config.domain, payload.target_url) - const {state, description} = statusFor(payload.state, first != null, config.path) + const status = statusFor(payload.state, first != null, config.path, config.noArtifactState) + if (status === null) { + log('Ignoring: no artifacts, and no-artifact-state is "skip"') + return null + } return { - url, - state, - description, + url: redirectUrl(first, config.path, config.domain, payload.target_url), + state: status.state, + description: status.description, context: config.jobTitle || `${payload.context} artifact`, } } diff --git a/worker/index.js b/worker/index.js index a8511c0..16cd77d 100644 --- a/worker/index.js +++ b/worker/index.js @@ -214,7 +214,7 @@ export async function handle(request, env, {fetchFn = globalThis.fetch, log = () const status = await resolveStatus({payload, config, fetchFn, log}) if (status === null) { - return new Response('ignored: not a watched job', {status: 200}) + return new Response('ignored: not a watched job, or no artifacts with no-artifact-state: skip', {status: 200}) } // The URL is part of the key, so a re-run that produces different artifacts diff --git a/worker/index.test.js b/worker/index.test.js index 6333059..ac4dde3 100644 --- a/worker/index.test.js +++ b/worker/index.test.js @@ -126,6 +126,29 @@ test('ignores repos with no config file, and configs with no artifact-path', asy } }) +test('honours no-artifact-state from the config file', async () => { + const empty = {items: []} + const {fetchFn, seen} = backend({config: `${CONFIG}no-artifact-state: skip\n`, artifacts: empty}) + const response = await handle(webhook(PAYLOAD), ENV, {fetchFn}) + assert.equal(response.status, 200) + assert.ok(!seen.some((r) => r.url.includes('/statuses/')), 'nothing posted') + + clearCache() + const green = backend({config: `${CONFIG}no-artifact-state: success\n`, artifacts: empty}) + await handle(webhook(PAYLOAD), ENV, {fetchFn: green.fetchFn}) + const posted = JSON.parse(green.seen.find((r) => r.url.includes('/statuses/')).body) + assert.equal(posted.state, 'success') + assert.equal(posted.description, 'No artifacts found') + assert.equal(posted.target_url, PAYLOAD.target_url) +}) + +// i.e. the delivery 500s and shows up in Recent Deliveries, rather than the +// repo silently getting whatever we guessed it meant +test('a config with an unusable no-artifact-state throws', async () => { + const {fetchFn} = backend({config: `${CONFIG}no-artifact-state: grey\n`}) + await assert.rejects(() => handle(webhook(PAYLOAD), ENV, {fetchFn}), /no-artifact-state must be one of/) +}) + 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}) @@ -229,6 +252,7 @@ test('normalizeConfig applies the same defaults as the action', () => { assert.equal(config.domain, 'output.circle-artifacts.com') assert.equal(config.path, 'p') assert.equal(config.apiToken, '') + assert.equal(config.noArtifactState, 'failure') assert.deepEqual(normalizeConfig().jobNames, ['build_docs', 'doc', 'build']) assert.deepEqual(normalizeConfig({'circleci-jobs': 'a, ,b '}).jobNames, ['a', 'b']) })