Skip to content

fix(ruby): namespace gem tags - #323

Draft
NikolayS wants to merge 4 commits into
mainfrom
agent/fix-ruby-release-tag
Draft

fix(ruby): namespace gem tags#323
NikolayS wants to merge 4 commits into
mainfrom
agent/fix-ruby-release-tag

Conversation

@NikolayS

Copy link
Copy Markdown
Owner

What changed

  • replace Bundler's generic rake release path with an explicit trusted-publishing workflow;
  • namespace Ruby client tags as ruby/v${VERSION}, leaving v${VERSION} to SQL/server releases;
  • tag the exact workflow-dispatch SHA validated by the build job;
  • pass the exact tested gem artifact to the publish job;
  • validate canonical version, tag, filename, gem name, embedded version, clean tree, and remote tag availability;
  • make dry runs exercise tests/build/install/artifact/tag checks without tagging or publishing;
  • document partial-failure recovery and namespaced retraction;
  • remove the generic Bundler release task.

Why

Ruby and server versions are independent. The old workflow would publish Ruby 0.3.0 by creating v0.3.0 in the monorepo, colliding with the server release tag.

Fixes #312.

Validation

  • actionlint .github/workflows/release-ruby.yml;
  • Ruby YAML parsing;
  • validator positive and negative cases;
  • Ruby 3.3.11 container validation;
  • 76 Ruby test runs, 0 failures/errors (database-dependent tests skipped without a DSN);
  • gem build, metadata inspection, isolated install, require/version/public-class smoke;
  • confirmed no generic release task remains;
  • git diff --check.

The trusted-publishing mechanism was cross-checked against the current official RubyGems actions: configure-rubygems-credentials supports direct RubyGems CLI use with id-token: write, and rubygems-await is the propagation command used by release-gem.

External prerequisite

The repository's rubygems GitHub environment and RubyGems trusted publisher still need to be configured before a real publish. This PR does not create tags, publish a gem, or change those external settings.

This is intentionally a draft and has not been merged.

@NikolayS NikolayS changed the title release(ruby): namespace gem tags fix(ruby): namespace gem tags Jul 11, 2026
@NikolayS

Copy link
Copy Markdown
Owner Author

Real-user verification evidence

The original release-validator RED is retrospective: the initial implementation was committed without its automated validator coverage. Applying the final validator test to base 73af5c6 produced 5/5 failures because script/validate_release.rb did not exist. Head verification:

ruby -Iclients/ruby/lib clients/ruby/test/test_validate_release.rb
actionlint .github/workflows/release-ruby.yml
gem build clients/ruby/pgque.gemspec --output /tmp/pgque-0.3.0.rc.1.gem
ruby clients/ruby/script/validate_release.rb \
  0.3.0.rc.1 ruby/v0.3.0.rc.1 /tmp/pgque-0.3.0.rc.1.gem

Result: workflow/validator suite 7 runs, 31 assertions, 0 failures; exact tag, canonical version, missing artifact, pinned release-tooling, and shell-safety contracts passed. The exact gem artifact validated and installed in an isolated GEM_HOME. Head CI: 17/17 checks passed.

0.3.0.rc.1 is already published and is not a releasable candidate. PR #343 is stacked on this PR, carries the source bump to rc.2 and the successful non-publishing database-backed dry run, and must merge after this PR.

@NikolayS NikolayS left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

REV rubric review

Automated ultrareview quota was unavailable, so I executed the five official
REV rubric prompts manually. SOC2 findings were omitted as instructed. This is
a comment-only review, not an approval.

Blocking findings

No unresolved functional or security defect on the reviewed stack. There is
one integration-order gate: merge #323 before #343, then merge #343 before
operating the Ruby release workflow. Do not publish in the interval between
those merges; #343 supplies the rc.2 bump and published-version guard.

Rubric results

  • Bug hunter: namespaced tags, exact-SHA tagging, exact artifact transfer, and
    tag collision checks are coherent. Partial-failure recovery is documented.
  • Security reviewer: publish privileges are job-scoped; the build checkout
    does not retain credentials; privileged rubygems-await tooling is pinned
    to 0.5.4 for both installation and execution.
  • Docs reviewer: bootstrap, namespaced retraction, tag-versus-gem failure, and
    Trusted Publishing prerequisites are clear. The #323 to #343 order above is
    mandatory because #343 completes the live rc.2 operator contract.
  • Guidelines checker: current hardening commits use allowed types and all CI
    checks pass. Immutable history: test(ruby): cover release validator uses a
    type no longer allowed by CLAUDE.md; do not amend or force-push it.
  • Test analyzer: validator cases, workflow shell/tooling contract, artifact
    checks, actionlint, and the database-backed Ruby job provide appropriate
    coverage. #343's exact-head release dry run validates the combined stack.

Nonblocking findings

Only the immutable commit-type history. No current functional/security change
is requested.

@NikolayS

NikolayS commented Aug 3, 2026

Copy link
Copy Markdown
Owner Author

samorev Code Review Report

Pipeline Coverage
unknown Not reported

REVIEW FINDINGS (14)

HIGH MR/PR state - Review target is draft

The review target is still marked as draft.
Fix: Mark it ready for review before merge.

CRITICAL CI/Pipeline - Pipeline status is unknown

Provider CI reported status unknown.
Fix: Fix failing checks and rerun review.

HIGH [bugs] ruby/setup-ruby with bundler-cache: true runs bundle install before the new clean-tree gate, and Bundler routinely rewrites Gemfile.lock in CI (adding the runner's platform to PLATFORMS, updating BUNDLED WITH). That mutation makes git diff --exit-code fail, so a perfectly valid release aborts with an opaque diff dump instead of a real error.

Fix: Capture the clean-tree assertion before Bundler touches the tree — either move the git diff --exit-code / git diff-index --quiet --cached HEAD pair into a step placed immediately after actions/checkout@v4 and before setup-ruby, or set BUNDLE_FROZEN=1 (bundle config set --local frozen true) so bundle install refuses to modify the lockfile rather than silently dirtying it. If Gemfile.lock is intentionally untracked for this gem, add a comment saying so, because the gate's correctness depends on that fact.

MEDIUM [bugs] Nothing checks whether the version is already published on RubyGems before the tag is pushed. The tag push happens first, so re-releasing an already-published version produces exactly the orphan-tag partial failure this PR is meant to eliminate.

Fix: Add a remote-version pre-check alongside the existing git ls-remote tag check in the build job (fails before any privileged job runs), e.g. gem list -r -e pgque --all or a https://rubygems.org/api/v1/versions/pgque.json fetch, and abort if $VERSION is present. Alternatively push the tag after a successful gem push, so the failure mode is a missing tag (trivially re-creatable) rather than an orphan tag.

MEDIUM [tests] The validator's artifact-inspection branch is almost entirely untested. test_validate_release.rb covers only the missing-file case; the filename check, the gem-name check, the embedded-spec-version check, and the three-argument success path all have zero coverage — and those are precisely the checks the PR adds to guarantee "the exact tested gem artifact" is what gets published.

Fix: Build a real gem once in a Dir.mktmpdir (or check in a tiny fixture .gem) and add: a success case with three arguments asserting the from <path> suffix in the output; a renamed-file case asserting gem filename must be; a case using a gem whose spec name/version differ from the arguments asserting gem name must be and gem version ... != expected. Also add a wrong-arity case asserting the usage: abort.

MEDIUM [tests] test_rejects_missing_artifact asserts a relative filename is missing, but that filename is exactly what gem build pgque.gemspec produces in clients/ruby. Any developer (or CI reordering) that runs rake test after a local gem build makes the file exist, the validator succeeds, and the test fails for reasons unrelated to the code under test.

Fix: Point at a path that cannot exist, e.g. File.join(Dir.mktmpdir, "pgque-#{Pgque::VERSION}.gem"), so the test is independent of the working directory's build state.

MEDIUM [tests] test_release_workflow_shell_contract is self-selecting and therefore vacuous for the case it is supposed to guard. It filters to scripts that already contain set -Eeuo pipefail and then asserts those scripts contain it — a new multi-line run: block added without the prologue passes silently. The psql/seq assertions are dead code borrowed from an unrelated convention: this workflow contains no psql, so the entire branch never executes.

Fix: Assert the contract over every multi-line script instead of the pre-filtered subset — scripts.select { |s| s.include?("\n") }.each { |s| assert_match(...) } — so a missing prologue actually fails. Drop the psql block (or move it to a shared workflow-lint test that runs against workflows that use psql).

MEDIUM [security] Action pinning is inconsistent in the most privileged job. rubygems/configure-rubygems-credentials is correctly SHA-pinned, but actions/checkout@v4, ruby/setup-ruby@v1, and actions/download-artifact@v4 remain on mutable tags in a job that holds contents: write, id-token: write, and live RubyGems trusted-publishing credentials. A compromised or retagged upstream action there can exfiltrate the OIDC token or push arbitrary content.

Fix: SHA-pin every uses: in publish-rubygems (and ideally in build) with a trailing # vX.Y.Z comment, matching the style already applied to configure-rubygems-credentials. Note in a comment that gem install rubygems-await --version 0.5.4 is version-pinned but not integrity-verified, since RubyGems has no checksum-pinning equivalent.

MEDIUM [bugs] retention-days: 1 is the minimum GitHub allows, but publish-rubygems sits behind environment: rubygems, which blocks on manual reviewer approval. If approval lands more than 24 hours after the build job, actions/download-artifact fails and the release must be re-dispatched from scratch.

Fix: Raise retention-days to something that comfortably exceeds realistic approval latency (7 is a reasonable default for a release artifact), or document in RELEASE.md that the rubygems environment must be approved within 24 hours of the build.

LOW [bugs] The build job's remote-tag pre-check depends on unauthenticated git ls-remote, because the same checkout now sets persist-credentials: false. That works only while the repository is public; if it is ever made private, git ls-remote exits 128 and the step reports "failed to check release tag" — a misleading auth failure dressed as a tag-check failure.

Fix: Either keep credentials for this checkout (the job only has contents: read, so the exposure is minimal), or pass an explicit authenticated URL for the check, e.g. git ls-remote --exit-code --tags "https://x-access-token:${GITHUB_TOKEN}@github.com/${GITHUB_REPOSITORY}" ... with GITHUB_TOKEN from secrets.GITHUB_TOKEN. At minimum, distinguish exit code 128 in the error message so an auth failure is diagnosable.

LOW [tests] The central behavioral change of this PR — that the workflow tags ruby/v${VERSION} and never a bare v${VERSION} — has no workflow-level test. The validator tests only prove the script rejects a bad tag argument; nothing catches a regression that changes TAG_NAME in the YAML itself, and the test file already demonstrates the pattern for asserting on workflow contents.

Fix: Add a test that loads the workflow and asserts both jobs' env.TAG_NAME equals ruby/v${{ inputs.version }}, and that no run: block in the workflow matches /git (tag|push)[^\n]*\bv\$\{/ without the ruby/ prefix.

LOW [bugs] Inconsistent nil-safety in the pinning test: install is dereferenced with &. but await_step is not. If the "Wait for release to propagate" step is renamed or removed, the test raises NoMethodError on nil (a test error with a confusing backtrace) instead of producing a clean assertion failure naming the missing step.

Fix: Assert both steps exist first (refute_nil install, "missing 'Install release tooling' step" and likewise for await_step), then compare run values without &..

LOW [docs] RELEASE.md's recovery guidance has a gap for the "gem push reported failure but actually published" case. It tells the reader to check RubyGems first, but the only concrete recovery recipe is "delete the tag, then re-dispatch" — which, for an already-published version, will fail again at gem push and leave a published gem with no tag at all.

Fix: Add an explicit branch: if the version is already live on RubyGems, do not re-dispatch — the tag was pushed before gem push, so it is already correct; verify with git ls-remote --tags origin 'refs/tags/ruby/v*' and manually create/push the tag only if it is genuinely absent.

LOW [docs] Removing require "bundler/gem_tasks" from the Rakefile deletes rake build, rake install, and rake release, but nothing documents that. RELEASE.md was updated to describe the new flow, yet never states that those tasks are intentionally gone, and the new script/validate_release.rb — now the single source of truth for release preconditions — is not mentioned anywhere in the docs.

Fix: Add a short note to RELEASE.md stating that bundler/gem_tasks is deliberately not loaded (so no rake release can bypass the namespaced-tag policy), and document ruby script/validate_release.rb VERSION TAG_NAME [GEM_PATH] as the local pre-flight check maintainers can run before dispatching.


Summary

Area Findings Potential Filtered
CI/Pipeline 1 0 0
Security 0 1 0
Bugs 0 5 0
Tests 0 4 0
Guidelines 0 0 0
Docs 0 2 0
Metadata 1 0 0

Note:

  • Findings: High-confidence issues (8-10/10) - blocking or non-blocking per severity
  • Potential: Medium-confidence issues (4-7/10) - review manually
  • Filtered: Low-confidence issues (0-3/10) - excluded as likely false positives
Review metadata
provider=github
kind=pr
project=NikolayS/PgQue
number=323
target=github:NikolayS/PgQue#323
state=OPEN
draft=true
diff_lines=461
diff_added=267
diff_removed=88
diff_bytes=18041
comments_count=1
commits_count=4
ci_status=unknown
ci_summary=total=18 success=17 failure=0 pending=0 other=1
prompt=.claude/commands/review-mr.md
blocking=false
posted_by=gh
no_comment=false
live_posting=posted

samorev-assisted review (AI analysis by Tanya301/samorev)

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.

release(ruby): prevent gem tag collision with server v0.3.0

1 participant