Bump to Node 24, upgrade GitHub Actions, fix primaries-required bug, add prettier#1
Merged
Conversation
Update contribution credit for O-Mutt
Adds an .nvmrc as a single source of truth for the Node version and switches the action's JavaScript runtime from node20 to node24. The package.json engines field documents the requirement for local devs.
…eck jobs - actions/checkout v4 -> v6 - actions/setup-node v3.8.1 -> v6, reading Node version from .nvmrc instead of hardcoded 16.x - actions/upload-artifact v3 -> v7 - github/codeql-action/* v2 -> v4 - new lint job runs `npm run lint` and `npm run format:check` - units job now uses setup-node so tests run on the declared Node version
wait.js was leftover from the actions/template-javascript-action starter and was never imported. .eslintrc.json is ignored by ESLint 9 in favor of the flat config in eslint.config.mjs.
…rs, harden API call Behavior fix: - core.getInput returns "" not undefined for an unset input, so the prior `primariesRequired !== undefined` guard always evaluated true. That made the busy-skip path run even when the user did not set primaries-required, silently rejecting busy primaries. The new parsePrimariesRequired() returns undefined for unset/empty/zero/NaN and only treats positive integers as a real requirement. Structure: - Extract parsePrimariesRequired, resolveApiPath, and reportFallback so main() reads top-to-bottom and the duplicated fallback-on-error block becomes a single function call. - Flatten the runner loop with early continues. - Always emit `use-runner` as a JSON-stringified array regardless of which code path produces it. - Await core.summary.write() so the summary cannot be dropped on a fast process exit. Reliability: - 10s socket timeout on the runners API call. - Send the GitHub-recommended `Accept: application/vnd.github+json` and `X-GitHub-Api-Version: 2022-11-28` headers. Tests grow from 5 to 18, covering the primaries-required regression, non-200 responses, label-superset matching, header verification, and unit tests for parsePrimariesRequired and resolveApiPath.
…onfig - Add prettier and eslint-config-prettier devDependencies - .prettierrc.json: single quotes, ES5 trailing commas, semis, LF endings to match the existing codebase style - .prettierignore excludes dist, lockfiles, license/codeowners, and the local-only .claude tooling directories - npm scripts: `format` writes, `format:check` is CI-friendly, and `all` now runs format:check alongside lint/test/build - eslint.config.mjs: append eslint-config-prettier so ESLint stops flagging things Prettier owns; bump ecmaVersion 2018 -> latest to match Node 24; ignore the local .claude dirs - Format README and codeql-config to match the new style
Regenerate the bundled action entrypoint with @vercel/ncc under Node 24.10.0 so check-dist passes on the new runtime.
- Rename action to "Self-Hosted Runner Fallback" so the Marketplace listing and the README title agree - Update author metadata to reflect current ownership - Replace the inaccurate "repo and actions scope" guidance on github-token with the actual scope matrix: classic PAT with repo for user repos, admin:org for organizations, manage_runners:enterprise for enterprises - Document the use-runner output as a JSON-encoded array and remind consumers to wrap it in fromJson(...) README: - Add Marketplace, build, and license badges pointing at the publishing fork (O-Mutt/runner-fallback-action) - Lead with a "what / why" tagline and a 10-line copy-pasteable quick start before the input tables - Split inputs into Required / Scope (mutually exclusive) / Optional with a defaults column - Add a dedicated Outputs section - Add a Permissions section that maps each scope to its required token type and permission - Add a Versioning section that recommends pinning to a major tag or a commit SHA and tracks the Node-runtime lineage of v1 vs v2 - Add a Contributing section pointing at `npm run all` and explaining the dist/ commit policy - Update the example reference from @v1 to @v2 to match the upcoming release
`npm ci` was failing on all GitHub-hosted runners because the previously committed package-lock.json was missing several platform-specific binary entries (@unrs/resolver-binding-*, @napi-rs/wasm-runtime, @emnapi/*) — a known npm 10+ bug where `npm install` does not always record every platform variant of optional dependencies. Regenerated the lockfile from scratch (rm -rf node_modules package-lock.json && npm install) so all platform variants are listed. The fresh resolution also picked up newer compatible patch versions of a few transitive dependencies of @actions/core, which in turn pulled in undici for the http client; rebuilt dist/ accordingly.
The `test` and `test-multiple-labels` jobs ran the action with
`github-token: ${{ secrets.TEST_GITHUB_TOKEN }}` and that secret is not
configured in this fork, so the action correctly failed with
"Input required and not supplied: github-token".
Fall back to the auto-injected `secrets.GITHUB_TOKEN` when the test
secret is unset. That token lacks runner-admin scope so the API call
will 401, but with `fallback-on-error: true` the action emits the
fallback runner instead of failing, which lets these jobs exercise the
fallback-on-error code path even without bespoke secrets. Maintainers
who configure TEST_GITHUB_TOKEN still get the primary-path coverage.
- Rename .github/workflows/test.yml to ci.yml and the workflow display name from 'units-test' to 'CI'. The workflow does lint, format-check, unit tests, and runs the action against the GitHub API — "CI" is a more accurate label for that scope. - Add a concurrency group that cancels superseded runs on the same PR while still running every push to main, so we stop paying for stale PR commits after a force-push. - Update the README badge URL and label.
CODEOWNERS still had the actions/template-action default "@actions/actions-runtime", which is not a real reviewer and silently disabled automatic review assignment. Set the owner to the current maintainer. .gitignore now excludes .claude/, .claude-flow/, .mcp.json, and CLAUDE.md so contributors using Claude Code locally cannot accidentally commit their personal agent configuration.
SECURITY.md documents which majors receive fixes, points reporters at GitHub's private vulnerability reporting flow, and scopes what we consider in vs. out (upstream-dep CVEs route through Dependabot). CONTRIBUTING.md captures the conventions a new contributor needs: .nvmrc as the Node source of truth, the committed dist/ policy enforced by check-dist, the Prettier+ESLint+Jest tooling, and the release flow that re-points the floating @v2 tag for Marketplace consumers.
`@actions/core@1.11.1` ships its own bundled `@actions/http-client@2.2.3` which pinned `undici <=6.23.0`. That undici range is affected by: - GHSA-g9mf-h72j-4rw9 (unbounded decompression chain, high) - GHSA-2mjp-6q6p-2qxm (HTTP request/response smuggling) - GHSA-vrm6-8vpv-qv8q (unbounded memory in WebSocket permessage-deflate) - GHSA-v9p9-hfj2-hcw8 (unhandled exception in WebSocket client) - GHSA-4992-7rv2-5pvq (CRLF injection via `upgrade` option) `npm audit fix` cannot reach the nested undici because it would have to bump `@actions/core` across a major. Pinning `undici` via npm `overrides` at the root forces every transitive resolution onto the patched 6.24 line without dragging in a `@actions/core` major bump. `npm audit` now reports 0 vulnerabilities. The smaller, fixed undici also reduces the bundled dist/ from ~1.93 MB to ~1.10 MB.
Previously the Quick start referenced \`secrets.RUNNER_API_TOKEN\` without explaining where it came from, and required-fields detail lived only in a reference table. A first-time Marketplace user had to assemble the setup process themselves. Added three sections without disturbing the reference material: - **Prerequisites** — what state the repo/org/enterprise must be in before any of this works (runner registered, admin-scoped token, token stored as a secret). - **Setup** — five numbered steps walking a new user from "create token in GitHub UI" through "consume the output with fromJson". Each step names the exact GitHub menu path so users do not have to guess. - **Troubleshooting** — a symptom/cause/fix table covering the failures we have seen most often: empty token secret, 401 from a wrong-scope token, 404 from a misspelled org slug, fallback being picked when primaries appear online (primaries-required tuning), fromJson omissions, and bursty-load races that the concurrency block in the Full example fixes. The required-fields table is unchanged and remains the formal reference. README is 211 lines, still well under the 500-line limit.
…cy signals Three badges (CI / static Marketplace / License) is enough to say "this exists and is MIT", but does not signal whether the action is actively maintained, currently safe to use, or which version is on the Marketplace right now. Reworked the badge row to: - Keep the CI status badge as the primary "is main green?" signal - Add the CodeQL workflow badge so consumers can see we run static security analysis on every push and PR - Replace the static "Marketplace" badge with a dynamic `github/v/release` badge that auto-updates to the current released tag — still links to the Marketplace listing page - Add a Snyk vulnerabilities badge via shields.io so a glance is enough to see whether any known transitive CVE has cropped up - Add a "last commit" badge so users can immediately tell the project is being maintained (or warn them if it goes quiet) - Keep the License badge as the rightmost reassurance
- New \`npm run test:coverage\` script runs \`jest --coverage\` - jest config in package.json collects coverage on index.js and emits lcov + text-summary + json-summary reporters - Coverage thresholds set just below current values (branches 75, functions 70, lines 50, statements 55) so CI fails on regression but passes on the existing baseline. Improving these is a follow-up that needs main()/reportFallback() tests with @actions/core mocked. - CI units job now runs test:coverage and uploads lcov.info via codecov/codecov-action@v5. CODECOV_TOKEN is optional for public repos but supported when present. README badges: add Codecov coverage badge, add GitHub stars badge. Coverage output ignored via existing .gitignore entry.
Adds a release-please workflow so version bumps, CHANGELOG entries, and GitHub Releases are generated from Conventional Commit history instead of being tagged by hand. Pieces: - .github/workflows/release-please.yml runs on every push to main. It invokes googleapis/release-please-action@v4 in manifest mode, then runs a follow-up step that re-points the floating major tag (e.g. v2 -> v2.0.0) after a release is cut. Marketplace consumers pinned to @v2 pick up new releases automatically without action from the maintainer. - .release-please-config.json declares the package as release-type "node", forces the first release to 2.0.0 via "release-as", uses the merge-base with main (dc7732f) as bootstrap-sha so the v2.0.0 changelog includes commits from this PR forward only, and pulls in docs/build/ci/chore sections in addition to feat/fix. - .release-please-manifest.json pins the last released baseline to the current package.json version (1.0.2). release-please updates this file in the release PR when a new version ships. - CONTRIBUTING.md updates the Conventions section to call out that Conventional Commits drive the version bump, and replaces the manual-tag "Releasing" section with the release-please flow plus a short note about the first-time Marketplace tickbox. Requires `contents: write` and `pull-requests: write` on the GITHUB_TOKEN, declared at the workflow level.
…orm wasm fallbacks npm 11 on macOS still records the lockfile with platform-specific optional dependencies missing for variants that the current host does not need. jest@30 -> @jest/core -> jest-resolve -> unrs-resolver pulls @unrs/resolver-binding-wasm32-wasi, whose sub-dependencies @emnapi/core and @emnapi/runtime were therefore absent from the lockfile when it was generated on darwin-arm64. Linux runners then fail \`npm ci\` with "Missing: @emnapi/core@1.10.0 from lock file" because they exercise the wasm fallback path. Regenerated the lockfile inside an official \`node:24\` Linux container (\`docker run --rm -v $PWD:/app -w /app node:24 sh -c "npm install --no-audit --no-fund"\`), which records every cross-platform variant and lands the missing @emnapi/* entries. Verified \`npm ci\` now succeeds both locally on macOS and inside the Linux container. dist/ is byte-identical, so no rebuild needed. If this bug bites again on a future regen, run the same docker one-liner before committing the lockfile.
Welcome to Codecov 🎉Once you merge this PR into your default branch, you're all set! Codecov will compare coverage reports and display results in all future pull requests. Thanks for integrating Codecov - We've got you covered ☂️ |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
node24; a new.nvmrc(containing24) is the single source of truth and workflows source the Node version from it instead of hardcodingactions/checkout@v6,actions/setup-node@v6,actions/upload-artifact@v7,github/codeql-action/*@v4; deprecated v2/v3 references removedcore.getInputreturns""(notundefined) for unset inputs, so the priorprimariesRequired !== undefinedguard always ran the busy-skip path. A busy primary was silently skipped even when the caller didn't request a free primary count. NewparsePrimariesRequired()returnsundefinedfor unset/empty/zero/NaN and only treats positive integers as a real requirementindex.jsrefactor for readability — extractedparsePrimariesRequired,resolveApiPath, andreportFallbackhelpers somain()reads top-to-bottom; the duplicated fallback-on-error block is now one function call; runner loop flattened with early continues;core.summary.write()properly awaited;use-runneroutput is always a JSON-stringified array regardless of code pathAccept: application/vnd.github+jsonandX-GitHub-Api-Version: 2022-11-28headerseslint-config-prettierso ESLint and Prettier stop fighting),npm run format/npm run format:checkscripts, single-quote ES5-trailing-comma config matching the existing codebase style; bumpedecmaVersion: 2018 → latestto match Node 24lintjob runsnpm run lintandnpm run format:check;unitsjob now usessetup-nodeso tests run on the declared Node versionwait.js(dead template leftover, never imported) and.eslintrc.json(legacy config that ESLint 9 ignores in favor ofeslint.config.mjs)Why
The codebase was on Node 16 in workflows / Node 20 in the action runtime, with several deprecated action versions that would soon stop receiving security fixes. While doing that bump, an audit surfaced a real behavior bug in the runner-availability logic plus enough small readability wins to be worth landing together.
Test plan
npm run allpasses locally on Node 24 (lint, format:check, build, tests)check-distworkflow passes on the new runtime (dist/ regenerated against Node 24)github/codeql-action@v4units-testworkflow's newlintjob passesunits-testworkflow'stestandtest-outputjobs still exercise the action against the GitHub APIprimaries-requiredunset (should select the primary, not fall back — the bug fix)Commits
Split into 6 logical commits for easier review:
chore(node): bump runtime to Node 24, manage version via .nvmrcci: upgrade workflow actions to latest majors, add lint and format-check jobschore: remove dead wait.js and legacy .eslintrc.jsonrefactor(index): fix unset primaries-required handling, extract helpers, harden API callchore(tooling): add Prettier with CI format check, modernize ESLint configbuild: rebuild dist/ for Node 24 runtimeFollow-ups (not in this PR)
npm auditreports 9 transitive-dep vulnerabilities (4 moderate, 5 high) — worth a separatenpm audit fixPR.claude/,.claude-flow/,.mcp.json, andCLAUDE.mdare local Claude Code tooling — worth adding to.gitignoreso they can't be accidentally committed