Skip to content

ci(release): port release-hardening fixes from ctxr/kit PR #1 review loop#2

Merged
meshin-dev merged 2 commits intomainfrom
fix/release-hardening
Apr 18, 2026
Merged

ci(release): port release-hardening fixes from ctxr/kit PR #1 review loop#2
meshin-dev merged 2 commits intomainfrom
fix/release-hardening

Conversation

@meshin-dev
Copy link
Copy Markdown
Contributor

Summary

PR #1 landed the core PR-based release flow. A subsequent 6-round Copilot review loop on the parallel ctxr/kit PR #1 surfaced a stack of defensive gaps that were fixed there. This PR ports the same fixes over so skill-code-review and ctxr/kit stay in lockstep.

publish.yml

  • Refuse non-tag refs first-step guard: closes the workflow_dispatch-on-a-branch untagged-publish hole. workflow_dispatch is retained (operators can re-run publish against an existing tag after transient npm outages) but dispatching against a branch no longer silently calls npm publish on an untagged build.
  • ::error:: annotation on tag/version mismatch so the failure is visible in Actions UI summaries.

release.yml

  • Header comment reflects the GITHUB_TOKEN trigger rule: tags pushed by GITHUB_TOKEN do NOT trigger on: push: tags workflows (GitHub design, to prevent workflow recursion), so publish is a separate manual dispatch until a PAT/App-token is wired into tag-on-main.yml.
  • Drop cache: npm: this job only runs npm version --no-git-tag-version --ignore-scripts and never installs anything.
  • Rebuild PR body via printf '%s\n\n%s\n\n%s' so paragraphs land at column 0 in the final string. The previous quoted-multi-line shape preserved yaml-script indentation and GitHub rendered the body as a <pre> code block. Step-5 wording also updated to match the new manual-dispatch flow.
  • Gate "edit existing PR" on an open PR for the head branch (gh pr list --head --state open ...). gh pr view "$BRANCH" matches closed/merged PRs too, so a previous release attempt that left a closed PR would cause subsequent dispatches to silently edit the closed PR and never open a new one.

tag-on-main.yml

  • Header comment updated to explain the GITHUB_TOKEN trigger rule + the PAT/App-token upgrade path.
  • Drop cache: npm (same reasoning: this step only uses node -p + git show | node, no install).
  • Version-detect step: explicit set -euo pipefail under shell: bash. The comment claimed bash -eo pipefail behaviour but Actions' default shell is -e only, so the git show ... | node -e pipeline could silently swallow upstream failures. Make the claim true.
  • After swallowing an "already exists" push error, re-verify the remote tag's dereferenced SHA matches HEAD. A concurrent creator (another workflow, a user) could still have tagged a different commit between the ls-remote check and the push — the previous code false-greened that orphaned release. Now fail loudly with the orphan-tag error + delete hint.

README.md

  • One-time setup: add the "Allow GitHub Actions to create and approve pull requests" bullet. It was only mentioned in troubleshooting before, so first-time operators would hit the failure without warning.
  • Step 5: describe the manual publish dispatch + GITHUB_TOKEN trigger rule in a block-quote note, plus the PAT/App-token upgrade path.
  • Troubleshooting: align the publish.yml step name to the actual "Verify version matches tag" text (was "Verify tag matches package.json").
  • Troubleshooting: replace "push an empty commit to main" with "merge a trivial no-op PR". Direct pushes to main are blocked by branch protection under the rulesets this flow is designed for.

Test plan

  • Merge this PR.
  • Verify tag-on-main.yml sees no version change on the merge commit, no-ops.
  • Dispatch Release with bump: patch.
  • Verify release/v<new> branch exists and a fresh release PR is open.
  • Review + merge the release PR.
  • Verify tag-on-main.yml creates the v<new> tag.
  • Manually dispatch Publish to npm against v<new> (until a PAT is wired in).
  • Verify @ctxr/skill-code-review@<new> on npm.

Related

  • ctxr/kit#1 — parallel port carrying the same fix set (plus templates).

 review loop

The initial port of the PR-based release flow (PR #1) landed the
core shape. A subsequent 6-round Copilot review loop on the ctxr/kit
port surfaced a stack of defensive gaps that were fixed there. Port
the same fixes here so skill-code-review and ctxr/kit stay in lockstep.

Summary of ported fixes:

publish.yml
- Add first-step "Refuse non-tag refs" guard. `workflow_dispatch`
  is retained (operators can re-run publish against an existing
  tag after transient npm outages) but dispatching against a
  branch no longer silently skips the tag/version check and calls
  `npm publish` on an untagged build.
- Annotate tag/version mismatch with `::error::` and a second
  guidance line so the failure is visible in Actions UI summaries.

release.yml
- Header comment now reflects the GITHUB_TOKEN trigger rule:
  tags pushed by the default `GITHUB_TOKEN` do NOT trigger
  `on: push: tags` workflows, so publish is a separate manual
  dispatch step until a PAT or GitHub App token is wired into
  tag-on-main.yml.
- Drop `cache: npm` on setup-node. This job only runs `npm
  version --no-git-tag-version --ignore-scripts` and never
  installs anything, so the cache step adds setup overhead with
  no install step to benefit from it.
- Rebuild the PR body with `printf '%s\n\n%s\n\n%s'` so each
  paragraph lands at column 0 in the final string. The previous
  quoted-multi-line shape preserved yaml-script indentation on
  every continuation line and GitHub rendered the PR body as a
  <pre> code block. Also reword step 5 to "dispatch publish.yml
  against v<version>" matching the new README wording.
- Gate the "edit existing PR" path on an OPEN PR for this head
  branch. `gh pr view "$BRANCH"` also matches closed/merged PRs,
  so a previous release attempt that left a closed PR on
  `release/v<version>` would cause the next dispatch to silently
  edit the closed PR and never open a new one. Switch to
  `gh pr list --head --state open --json number --jq '.[0].number // empty'`.

tag-on-main.yml
- Header comment updated to drop the "then triggers publish.yml"
  claim and explain the GITHUB_TOKEN trigger rule + the
  PAT/App-token upgrade path.
- Drop `cache: npm` (same reasoning — this step only uses `node -p`
  to read package.json and `git show | node` for the before-SHA).
- Version-detect step: set `-euo pipefail` under `shell: bash`.
  A comment claimed `bash -eo pipefail` behaviour but Actions'
  default shell is `-e` only, so the `git show ... | node -e "..."`
  pipeline could silently swallow upstream failures. Make the
  claim true.
- After swallowing an "already exists" push error, re-read the
  remote tag and compare its dereferenced SHA to HEAD. The
  concurrency group serialises runs on main but a concurrent
  creator (another workflow, a user) could still tag a DIFFERENT
  commit between our ls-remote check and our push — the previous
  code would false-green that orphaned release. Now fail loudly
  with the same orphan-tag error + delete hint the absent-tag
  path uses.

README.md
- One-time setup: add the "Allow GitHub Actions to create and
  approve pull requests" bullet. It was only mentioned in
  troubleshooting before, so first-time operators would hit the
  failure without warning.
- Cutting a release step 5: describe the manual publish dispatch
  and the GITHUB_TOKEN trigger rule in a block-quote note, plus
  the PAT/App-token upgrade path that makes publish auto-chain.
- Troubleshooting: align the `publish.yml` step name to the
  actual "Verify version matches tag" text operators will see in
  Actions logs (was "Verify tag matches package.json").
- Troubleshooting: replace "push an empty commit to main" with
  "merge a trivial no-op PR". Direct pushes to main are blocked
  by branch protection under the rulesets this flow is
  designed for, so the PR path is the reliable retrigger.
Copilot AI review requested due to automatic review settings April 18, 2026 23:05
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Ports CI release-hardening improvements (previously identified in ctxr/kit PR #1 review loop) into this repo’s PR-based release flow, closing a few workflow-dispatch and race-condition gaps and aligning operator docs with the actual release mechanics.

Changes:

  • Harden publish.yml to refuse non-tag refs on workflow_dispatch, preventing accidental untagged publishes.
  • Update release.yml/tag-on-main.yml to remove unnecessary npm caching, improve PR body formatting, ensure pipefail behavior, and re-verify remote tag SHA on “already exists” races.
  • Update README release instructions/troubleshooting to match the manual publish dispatch flow and required repo/org settings.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File Description
README.md Documents the manual publish dispatch (due to GITHUB_TOKEN trigger rules) and adds one-time setup/troubleshooting clarifications.
.github/workflows/tag-on-main.yml Removes unused npm caching, makes shell failure behavior explicit, and adds a post-race remote-tag SHA verification to prevent false-green orphan releases.
.github/workflows/release.yml Removes unused npm caching and fixes PR body formatting; only edits an existing open PR for the head branch to avoid silently updating closed PRs.
.github/workflows/publish.yml Adds an early guard step to fail fast when dispatched on non-tag refs; improves version/tag mismatch visibility via ::error:: annotations.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread .github/workflows/tag-on-main.yml Outdated
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@meshin-dev meshin-dev merged commit 8965212 into main Apr 18, 2026
8 checks passed
@meshin-dev meshin-dev deleted the fix/release-hardening branch April 18, 2026 23:19
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.

2 participants