Modernization, reliability fixes, CI hardening, and new features#69
Open
jefflinse wants to merge 9 commits into
Open
Modernization, reliability fixes, CI hardening, and new features#69jefflinse wants to merge 9 commits into
jefflinse wants to merge 9 commits into
Conversation
- package.json: fix repository/bugs/homepage boilerplate (was pointing at actions/javascript-action template) - .nvmrc, .tool-versions: bump Node 16 -> 20 to match action.yml runtime - workflows: bump actions/checkout v2/v3 -> v4 across all 4 workflows - workflows: bump github/codeql-action v1 -> v3 - README: replace @v1.6.0 references with floating @v1; bump actions/checkout examples to @v4; add new "Pinning a Version" section documenting v1 / v1.x.y / SHA tradeoffs - pr.js: fix "too manu" typo and double-space in error message; update matching test assertion - dist/: rebuild to pick up the typo fix
version.js - paginate listMatchingRefs so repos with >100 tags are handled (#26) - replace `new Map()` + property-access misuse with a plain object keyed by version.version (the previous code worked only by accident; it never used the Map API) - wrap createTag/createRef in try/catch that rethrows 403/404 with a clearer "missing `contents: write` permission" hint pointing at the README (#42, #45) pr.js - searchPRByCommit now tries listPullRequestsAssociatedWithCommit first (no rate limit, more reliable for rebase-merges) and falls back to the search API only if that yields no merged PR - searchPRByCommit returns null on no-match instead of throwing, so callers can treat it as a no-op - getReleaseType "no release label" error now lists the configured release+noop label names so users can see what was expected (#49) index.js - bumpAndTagNewVersion now wraps getReleaseType/getReleaseNotes in try/catch (matches validate mode) so users see the real validation error instead of a generic "unexpected error" - treat searchPRByCommit returning null as a graceful no-op, fixing the "first commit in a fresh repo" failure mode (#27) - coerce `skipped` output to string explicitly tests - new pagination test, no-PR-found test, two permission-error tests, and tests for both the associated-PR primary path and the search fallback. 30/30 pass with ~99% coverage.
Workflows - new dist-check.yml: rebuild dist/ in CI and fail PRs whose dist drifted from source (closes the silent-stale-dist failure mode) - master-ci.yml: declare explicit `permissions:`, add a publish-release job that creates a GitHub Release from the bump output and advances the floating major-version tag (e.g. v1) to the new commit. This removes the manual maintenance burden that left v1 / Releases stale - pr-release-metadata.yml: skip the validate job for dependabot[bot] so dependabot PRs no longer fail a check they cannot satisfy - master-ci test job: pin Node via setup-node + .nvmrc so local and CI use the same Node version Docs - new CONTRIBUTING.md: build/test, dist/ contract, PR labeling, the release pipeline, and the forked-PR caveat (issue #20 / #6) - new SECURITY.md: supported versions and private vulnerability reporting via GitHub Security tab - README: tighten the Contributing section to point at the new files
Adds four backwards-compatible features. All defaults preserve the existing behavior so this is a safe minor bump. action.yml / config.js - new input dry-run (default false): compute the bump and emit outputs without creating any tag or release - new input create-release (default false): also create a GitHub Release alongside the annotated tag - new outputs: major, minor, patch (issue #11), release-url, and a bump-summary JSON aggregate suitable for fromJSON() in downstream steps version.js - createRelease now returns { tag, releaseUrl } and optionally calls repos.createRelease when config.createRelease is true (issue #12). The release-creation call is wrapped in the same permissions-error helper as the tag/ref calls index.js - centralize output emission via emitOutputs() so the two modes stay in sync and bump-summary is always populated - in bump mode, honor dry-run by short-circuiting before createRelease while still emitting computed outputs - emit major/minor/patch parts in both validate and bump modes tests + README - new tests covering: tag-only release shape, GitHub Release path, dry-run config, create-release config - README Inputs/Outputs tables and Full Example refreshed to document the new surface 31/31 tests pass; coverage 98.6%
jefflinse
changed the title
The @actions/* runtime chain pulls vulnerable transitives that ship in dist/: - undici (high): bundled via @actions/http-client (used only for ProxyAgent) and @actions/github. Advisories GHSA-g9mf-h72j-4rw9, GHSA-2mjp-6q6p-2qxm, GHSA-vrm6-8vpv-qv8q, GHSA-v9p9-hfj2-hcw8, GHSA-4992-7rv2-5pvq — all fixed in 6.24.0. - uuid (moderate): bundled via @actions/core (used for ghadelimiter generation). GHSA-w5hq-g745-h8pq, fixed in 11.1.1. A clean fix by bumping @actions/core (1->3) and @actions/github (6->9) is not possible: those majors are ESM-only and this action is CommonJS, so require() would break at runtime. Instead pin patched transitives via npm overrides (undici ^6.24.0, uuid ^11.1.1) and rebuild dist/. Verified: ProxyAgent(undici@6.26.0) and uuid.v4() smoke-tested, dist bundle loads under require(), 31/31 tests pass, lint clean.
npm audit fix resolves the remaining advisories within existing semver ranges (lockfile-only; no manifest changes, nothing bundled into dist/): - minimatch (high) GHSA -> 3.1.3, via eslint/jest toolchain - flatted (high) -> 3.4.2 - picomatch (mod) -> 2.3.2 - semver (high) -> patched, via eslint-plugin-import npm audit now reports 0 vulnerabilities. lint + 31/31 tests pass.
There was a problem hiding this comment.
Pull request overview
This PR modernizes the action’s runtime/tooling, hardens CI/release automation, fixes reliability issues around version/PR discovery, and adds new action inputs/outputs (including optional GitHub Releases).
Changes:
- Modernize runtimes and workflows (Node 20, checkout v4, CodeQL v3), and add CI safeguards (dist drift check, automated release + floating major tag advancement).
- Reliability fixes: paginate tag listing for version detection, improve permissions-related error messaging, and make PR association lookup more robust (with null/no-op behavior instead of hard failures).
- New features:
dry-runandcreate-releaseinputs; new outputs (major/minor/patch,release-url,bump-summary).
Reviewed changes
Copilot reviewed 19 out of 22 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
index.js |
Adds dry-run, create-release, new outputs, and centralizes output emission. |
version.js |
Adds tag pagination, permission-error wrapping, and optional GitHub Release creation. |
pr.js |
Improves PR lookup by commit SHA and refines release-label error messaging. |
config.js |
Adds config parsing for dry-run and create-release. |
action.yml |
Documents new inputs and outputs (version parts, release URL, JSON summary). |
version.test.js |
Updates tests for pagination and createRelease behavior/return shape. |
pr.test.js |
Updates tests for new PR search behavior and improved error messages/typos. |
config.test.js |
Extends config tests to cover new inputs’ defaults and parsing. |
README.md |
Updates usage examples, documents new inputs/outputs, and adds pinning guidance. |
CONTRIBUTING.md |
Adds contributor guidance including required dist/ rebuild workflow. |
SECURITY.md |
Adds a security policy and private vulnerability reporting guidance. |
package.json |
Fixes repository metadata and adds overrides for undici/uuid. |
package-lock.json |
Locks dependency updates/audit fixes and reflects overrides. |
.nvmrc |
Bumps local Node version to 20.18.0. |
.tool-versions |
Bumps asdf Node version to 20.18.0. |
.github/workflows/pr-release-metadata.yml |
Updates checkout and skips validate job for Dependabot. |
.github/workflows/master-ci.yml |
Adds explicit permissions, Node setup, release publishing, and floating major tag advancement. |
.github/workflows/dist-check.yml |
New workflow to fail PRs when dist/ is out of sync with source. |
.github/workflows/dev-ci.yml |
Updates checkout to v4. |
.github/workflows/codeql-analysis.yml |
Updates checkout to v4 and CodeQL action to v3. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+63
to
+75
|
|
||
| const currentVersion = await getCurrentVersion(config) | ||
| const newVersion = semver.inc(currentVersion, releaseType) | ||
| const parts = versionParts(newVersion) |
Owner
Author
There was a problem hiding this comment.
Good catch — fixed in c0c8485. validateActivePR() now mirrors bump mode: when getReleaseType() returns 'skip' it no longer calls getReleaseNotes() or computes a next version, and it emits skipped: 'true' with only old-version (no more vnull). Verified both paths via a mocked smoke harness since index.js self-invokes and isn't unit-tested.
validateActivePR() unconditionally called getReleaseNotes() and semver.inc(currentVersion, releaseType). When a no-op label is present getReleaseType() returns 'skip', so semver.inc(v, 'skip') returned null, producing 'next version: vnull', a 'vnull' version output, and an incorrect skipped: 'false'. Mirror the existing 'bump' mode handling: when releaseType === 'skip', don't require release notes or compute a next version, and emit skipped: 'true' with only old-version. Non-skip validation is unchanged. Reported by Copilot on PR #69. index.js is not unit-tested (it self- invokes run() on load), so both paths were verified with a mocked smoke harness: skip emits skipped:'true' with no version; a minor label still emits v1.8.0 with major/minor/patch parts. 31/31 tests pass.
Comment on lines
+42
to
45
| try { | ||
| const q = `is:merged ${commitSHA}` | ||
| const data = await config.octokit.rest.search.issuesAndPullRequests({ q }) | ||
|
|
Owner
Author
There was a problem hiding this comment.
Fixed in 396de5e. The fallback query is now repo:${owner}/${repo} is:pr is:merged ${commitSHA} — scoped to this repo and to PRs, so it can't match an issue or a same-SHA PR in another repository. Added test assertions on the query string. (The primary listPullRequestsAssociatedWithCommit path was already repo-scoped.)
Comment on lines
31
to
+33
| steps: | ||
| - name: Checkout repository | ||
| uses: actions/checkout@v3 | ||
| uses: actions/checkout@v4 |
Owner
Author
There was a problem hiding this comment.
Fixed in 396de5e. Added explicit job permissions to the analyze job (actions: read, contents: read, security-events: write) so the SARIF upload works on repos with read-only default token permissions — consistent with the explicit permissions: block already added to master-ci.yml.
Two issues reported by Copilot on PR #69: pr.js — searchPRByCommit's search-API fallback used the query 'is:merged <sha>', which is not scoped to the current repository and dropped the original type:pr qualifier. As written it could return a merged PR from a different repo that matches the SHA, or an issue rather than a PR, leading to an incorrect version bump. Scope the query with repo:<owner>/<repo> and is:pr. The primary listPullRequestsAssociatedWithCommit path was already repo-scoped, so this only affects the rare fallback. Added test assertions on the query. codeql-analysis.yml — github/codeql-action@v3's analyze step uploads SARIF, which requires security-events: write. On repos whose default GITHUB_TOKEN is read-only the upload would fail, so declare explicit job permissions (actions: read, contents: read, security-events: write), matching the explicit-permissions approach already used in master-ci.yml. 31/31 tests pass, lint clean, dist rebuilt.
Comment on lines
+53
to
+57
| const e = new Error( | ||
| `${action} failed: ${err.message}. ` | ||
| + 'This is usually caused by a missing `contents: write` permission ' | ||
| + 'on the GITHUB_TOKEN. See README §Permissions.', | ||
| ) |
Owner
Author
There was a problem hiding this comment.
Fixed in b3c9be4. The hint now references the repo-token input and notes that the default GITHUB_TOKEN needs the contents: write workflow permission (a PAT/App token needs equivalent repo write access).
Comment on lines
235
to
+239
| release-notes-suffix: '' | ||
| with-v: false | ||
| base-branch: false | ||
| dry-run: false | ||
| create-release: false |
Owner
Author
There was a problem hiding this comment.
Fixed in b3c9be4. Switched the noop-labels examples to the multiline-string (|) form so core.getMultilineInput can read them. Also fixed a third occurrence earlier in the README that wasn't flagged, and verified all three examples now parse as strings.
Comment on lines
270
to
+274
| release-notes-suffix: '' | ||
| with-v: false | ||
| base-branch: false | ||
| dry-run: false | ||
| create-release: false |
Comment on lines
11
to
14
| steps: | ||
| - uses: actions/checkout@v3 | ||
| - uses: actions/checkout@v4 | ||
| - run: npm ci | ||
| - run: npm test |
Owner
Author
There was a problem hiding this comment.
Fixed in b3c9be4. Added actions/setup-node@v4 with node-version-file: .nvmrc to dev-ci.yml, matching master-ci.yml.
Comment on lines
+53
to
+57
| const e = new Error( | ||
| `${action} failed: ${err.message}. ` | ||
| + 'This is usually caused by a missing `contents: write` permission ' | ||
| + 'on the GITHUB_TOKEN. See README §Permissions.', | ||
| ) |
Comment on lines
11
to
14
| steps: | ||
| - uses: actions/checkout@v3 | ||
| - uses: actions/checkout@v4 | ||
| - run: npm ci | ||
| - run: npm test |
Comment on lines
+53
to
+57
| const e = new Error( | ||
| `${action} failed: ${err.message}. ` | ||
| + 'This is usually caused by a missing `contents: write` permission ' | ||
| + 'on the GITHUB_TOKEN. See README §Permissions.', | ||
| ) |
Comment on lines
235
to
+239
| release-notes-suffix: '' | ||
| with-v: false | ||
| base-branch: false | ||
| dry-run: false | ||
| create-release: false |
Comment on lines
270
to
+274
| release-notes-suffix: '' | ||
| with-v: false | ||
| base-branch: false | ||
| dry-run: false | ||
| create-release: false |
Comment on lines
11
to
14
| steps: | ||
| - uses: actions/checkout@v3 | ||
| - uses: actions/checkout@v4 | ||
| - run: npm ci | ||
| - run: npm test |
…de in dev-ci Three issues reported by Copilot on PR #69: version.js — the permissions-error hint hard-coded GITHUB_TOKEN, but the action uses whatever token is passed via the repo-token input. Reworded to reference repo-token and note that the default GITHUB_TOKEN needs the contents: write workflow permission (a PAT/App token needs equivalent repo write access). Still matches the existing test assertion. README.md — the noop-labels workflow examples used a YAML sequence, but action inputs must be strings, so core.getMultilineInput('noop-labels') can't read them and the workflow fails to parse (issue #49). Switched all three examples to the multiline-string (| ) form. Fixed the third occurrence too, which Copilot did not flag. dev-ci.yml — pinned Node via actions/setup-node + .nvmrc so it no longer depends on ubuntu-latest's default Node, matching master-ci.yml. 31/31 tests pass, lint clean, dist rebuilt; README noop-labels examples verified to parse as strings.
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
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.
RELEASE NOTES
Four distinct groups of improvements:
Cleanup & modernization
package.jsonrepository/bugs/homepage boilerplate (was pointing at theactions/javascript-actiontemplate).nvmrc,.tool-versions) to match the action.yml runtimeactions/checkout→ v4 andgithub/codeql-action→ v3 across all workflows@v1references, new "Pinning a Version" sectiondist/Reliability bug fixes
contents: writepermission errors on 403/404 (Error on version bump: Resource not accessible by integration #42, Reference Update Failed on bump #45)searchPRByCommituseslistPullRequestsAssociatedWithCommitfirst (better for rebase merges) with search-API fallback; returns null instead of throwingProcess & CI hardening
dist-check.yml: fails PRs whosedist/drifted from sourcemaster-ci.yml: explicitpermissions:, auto-create GitHub Release and advance the floating major tag (e.g.v1)dependabot[bot]CONTRIBUTING.mdandSECURITY.mdNew features
dry-runandcreate-release(both default false)major/minor/patch(Outputs should include individual version parts #11),release-url, and abump-summaryJSON aggregatecreateReleaseoptionally creates a GitHub Release (Action should create a full release, not just an annotated tag #12)Phase 5 — dependency security (
85c4bdb,296faff)Resolves all 12 open Dependabot/
npm auditfindings (npm auditnow reports 0 vulnerabilities).Runtime (ships in
dist/):undici(high, 5 advisories) →^6.24.0anduuid(moderate) →^11.1.1, pinned via npmoverrides. A clean fix by bumping@actions/core(1→3) /@actions/github(6→9) was rejected: those majors are ESM-only and this action is CommonJS, sorequire()would break at runtime. Verified the actually-used APIs (undici.ProxyAgent,uuid.v4()) and that the rebuiltdist/bundle loads underrequire().Dev-only (eslint/jest toolchain, never bundled):
minimatch,flatted,picomatch,semver— resolved vianpm audit fixwithin existing semver ranges (lockfile-only).