Skip to content

Add a create-release action alongside get-version. Closes #21 - #22

Merged
plamber merged 5 commits into
mainfrom
add-create-release-action
Sep 25, 2026
Merged

plamber merged 5 commits into
mainfrom
add-create-release-action

Conversation

@easylife-agents

Copy link
Copy Markdown
Contributor

Summary

  • Adds a second action, create-release, alongside the existing get-version action -- so any repository (starting with EasyLife365/.github, which currently hand-rolls the exact same logic in bash) can cut a tagged release without duplicating validate-tag-release-move-major-tag logic.
  • Talks to the GitHub REST API directly (@actions/github's getOctokit) -- no checkout, no fetch-depth: 0, no git push. Reuses the semver dependency get-version already has instead of a hand-rolled regex.
  • Moves a floating major tag (v1, v2, ...) inline, in the same call, skipped automatically for a prerelease -- fixes a real bug present, unfixed, in this repo's own update-major-tag.yml (now removed): its regex also matched a prerelease tag like v1.2.3-rc.1, which would have force-moved v1 onto an unstable commit.
  • No separated code: this repo's own release.yml now calls its own create-release action instead of a parallel bash implementation (dogfooding), and the now-redundant update-major-tag.yml is deleted.

A real dependency/tooling issue found and fixed along the way

@actions/github v9 is ESM-only ("type": "module", no require export condition) -- already a listed dependency, unused until now. This broke Jest's CJS-based test resolution (Cannot find module '@actions/github') even though esbuild's bundler handles it fine for the actual runtime output. Fixed with a manual __mocks__/@actions/github.js, matching this repo's existing __mocks__/@actions/core.js pattern, rather than fighting Jest's module resolution for a dependency it fundamentally can't require().

Test plan

  • npm run build, npm run lint -- --max-warnings 0, npm run test (57 tests, 100% coverage on the new core logic), npm run package -- all green locally.
  • npm run test:runtime-smoke extended with a new smoke test for the create-release bundle, mirroring the existing one -- proves the ESM-only @actions/github dependency loads correctly under node20 when bundled, not just under esbuild's own resolver.
  • Manually ran the bundled create-release/dist/index.js directly with a real (fake) token against a real repository -- confirmed it reaches a genuine "Bad credentials" GitHub API response rather than a raw ERR_REQUIRE_ESM crash, proving the full Octokit code path works end-to-end in the actual runtime bundle, not just under test mocks.
  • The repo's own precommit hook (lint + build + package + dist-up-to-date check) ran clean on commit.

🤖 Generated with Claude Code

EasyLife365/.github built its own release/tagging convention with a
hand-rolled bash copy of validate-tag-release-move-major-tag logic --
the exact same thing this repo's own release.yml already implements for
itself. Two copies of the same logic in two repos, that will drift.

create-release is a second action in this repo, sharing the semver
dependency get-version already uses instead of a hand-rolled regex.
Talks to the GitHub REST API directly (@actions/github's getOctokit) --
no checkout, no fetch-depth: 0, no git push needed by the caller. Moves
a floating major tag (v1, v2, ...) inline, skipped automatically for a
prerelease so a caller pinning v1 never silently receives unstable code
(a real bug found in review of .github's copy of this logic, and also
present, unfixed, in this repo's own now-removed update-major-tag.yml).

@actions/github v9 is ESM-only (type: module, no require condition),
which broke jest's CJS-based test resolution even though esbuild's
bundler handles it fine for the actual runtime output -- fixed with a
manual __mocks__/@actions/github.js, matching the existing
__mocks__/@actions/core.js pattern, rather than fighting Jest's module
resolution for a real dependency it can't require(). Verified the
bundled runtime for real, not just via the test suite: ran
create-release/dist/index.js directly with a live (fake) token and
confirmed it reaches a genuine "Bad credentials" API response rather
than a raw ERR_REQUIRE_ESM crash.

No separated code: this repo's own release.yml now calls its own
create-release action (dogfooding) instead of a parallel bash
implementation, and update-major-tag.yml is removed as redundant now
that the move happens inline within the same action call.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@easylife-agents
easylife-agents Bot requested a review from plamber September 25, 2026 04:44
@plamber
plamber marked this pull request as ready for review September 25, 2026 04:45
…review

tests.yml and build-dist.yml's dist-drift checks, and the pre-commit
hook's git add, all only ever covered dist/ (the root get-version
bundle) -- a PR touching src/createRelease/*.ts without rebuilding
create-release/dist/index.js would pass every PR-time check and could be
merged with a stale compiled bundle sitting next to the reviewed source.
release.yml already checked both; now tests.yml and build-dist.yml do
too.

Also strengthens the README's Tag protection note: update-major-tag
defaults to true, so every non-prerelease call is a default-on
destructive, history-rewriting operation, not just an available one.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

@plamber plamber left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Clean after fixing a real CI-coverage gap — one open governance question, not resolved here.

Both reviewers confirmed the code itself is sound: prerelease-exclusion, tag construction (always via semver's own normalized output, never raw input), the force: true major-tag move (always targeting v${parsed.major}, never an arbitrary caller-supplied ref), and token handling (never logged) all check out. Test isolation is correct in both files, and no leftover explicit jest.mock('@actions/github', ...) factory conflicts with the new manual mock.

Security review found a real gap, fixed: tests.yml/build-dist.yml's dist-freshness checks and the pre-commit hook only ever covered dist/ (the root bundle) — a PR touching src/createRelease/*.ts without rebuilding create-release/dist/index.js would have passed every PR-time check with a stale compiled bundle next to reviewed source. Now all three cover both. Also strengthened the README's note that update-major-tag defaults to true (a default-on destructive operation, not just available).

One finding not resolved in this review, surfaced for you directly: this repo has zero branch/tag protection (verified live — no rulesets, main returns "Branch not protected", tag protection 404) and no required PR review, per an existing, documented decision made when this repo shipped only a read-only action. create-release changes that risk profile — it's a write-capable action meant for cross-repo adoption starting with .github. Worth a fresh look at whether that acceptance still holds, not something to silently carry forward or that I should decide in a code review.


_Generated by Claude Code

plamber and others added 3 commits September 25, 2026 06:57
Numbered, exact runbook: decide the version (SemVer rules, and note
package.json's own version field is not the source of truth here),
where to run the Release workflow and what each input means, what the
two jobs actually do, how to confirm it landed, and what to do if
either job fails partway through.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Added a repo-level intro table so the README reads as a two-action repo
from the top instead of framing everything around get-version alone.
get-version's section now explains the actual git/semver mechanics
(branch-merged tag listing, commit-count patch bump), and create-release's
section walks through its six steps in order against the real Octokit
calls (git.getRef/createRef/updateRef, repos.createRelease) -- checked
against src/createRelease/createRelease.ts to keep the description
accurate rather than aspirational.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
npm audit found 3 vulnerabilities (1 moderate, 2 high), all in
transitive dev-tooling dependencies -- browserslist/baseline-browser-
mapping under @babel/preset-env, js-yaml under eslint -- never part of
the bundled runtime output. `npm audit fix` resolved all three within
already-declared semver ranges (package.json unchanged, lockfile only).

Also ran `npm update` for the rest: babel, eslint, jest, ts-jest, esbuild
etc. moved to their latest semver-range-compatible versions. Left the
several available MAJOR bumps (Babel 8, TypeScript 7, lint-staged 17,
@vercel/ncc 0.45) alone -- those are a separate, deliberate upgrade
decision, not implied by a security-fix pass, and this repo's actual
runtime dependencies (@actions/core, @actions/github, semver) were
already at their latest compatible versions and are unchanged here.

Rebuilt both bundles (esbuild 0.28.1 -> 0.28.2 changed output slightly).
Re-verified build, lint, 57 tests, both runtime smoke tests, and
`npm audit` (0 vulnerabilities) all green after the bump.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@plamber
plamber merged commit c7899d5 into main Sep 25, 2026
3 checks passed
@plamber
plamber deleted the add-create-release-action branch September 25, 2026 05:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant