Skip to content

Conversation

@tomerqodo
Copy link

@tomerqodo tomerqodo commented Dec 4, 2025

User description

Benchmark PR discourse#35180

Type: Clean (correct implementation)

Original PR Title: DEV: Introduce new versioning system
Original PR Description: Implements date-based versioning, based on the RFC at https://meta.discourse.org/t/383536

Manual version-bump tasks are removed, and replaced with new release:* rake tasks. These are run in GitHub actions. Release process will work something like:

  1. Trigger release-prepare-latest-bump via workflow_dispatch. This will create a PR which bumps the in-development version to the next -latest

  2. Merge that PR

  3. release-handler will be triggered automatically, tag the commit with v0000.00.0-latest, and cut a release/xxxx.xx branch from the previous commit

  4. release-prepare-bump will be triggered automatically, and create a PR which bumps the version on release/xxxx.xx branch to remove the -latest suffix

  5. Merge that PR

  6. release-handler will be triggered automatically, and will tag the commit with v0000.00.0

In future, we will add handling for ESR, and new workflows for security fixes.
Original PR URL: discourse#35180


PR Type

Enhancement


Description

  • Implement date-based versioning system replacing manual version bumps

  • Add new release:* rake tasks for automated release workflow

  • Create GitHub Actions workflows for release preparation and tagging

  • Remove legacy beta/stable version bump tasks from version_bump.rake

  • Update version to 2025.11.0-latest following new versioning scheme


Diagram Walkthrough

flowchart LR
  A["Manual Version Bumps"] -->|replaced by| B["Date-Based Versioning"]
  B --> C["release:prepare_next_version"]
  B --> D["release:maybe_cut_branch"]
  B --> E["release:maybe_tag_release"]
  C --> F["GitHub Actions Workflows"]
  D --> F
  E --> F
  F --> G["Automated Release Process"]
Loading

File Walkthrough

Relevant files
Enhancement
2 files
release.rake
New release automation tasks and utilities                             
+232/-0 
version_bump.rake
Remove legacy beta and stable bump tasks                                 
+0/-173 
Configuration changes
6 files
version.rb
Update version to new date-based format                                   
+1/-1     
action.yml
New GitHub Action for release environment setup                   
+43/-0   
release-handler.yml
New workflow for tagging and branch creation                         
+49/-0   
release-prepare-bump.yml
New workflow for release branch version bumps                       
+39/-0   
release-prepare-latest-bump.yml
New workflow for main branch version bumps                             
+29/-0   
tests.yml
Update checkout action version                                                     
+1/-1     
Tests
2 files
release_spec.rb
Add comprehensive tests for new release tasks                       
+203/-0 
version_bump_spec.rb
Remove tests for deprecated version bump tasks                     
+0/-134 
Miscellaneous
1 files
theme_spec.rb
Update version check to match new versioning scheme           
+1/-1     

@qodo-code-review
Copy link

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
Command injection risk

Description: The ReleaseUtils.git wrapper executes arbitrary git commands via Open3 without sanitizing
or constraining arguments, which could allow command misuse if any inputs (e.g.,
branch/ref names from environment/workflow inputs) are attacker-controlled; ensure all
refs/branches are validated against a strict pattern before use and avoid passing
untrusted values.
release.rake [27-34]

Referred Code
def self.git(*args, allow_failure: false, silent: false)
  puts "> git #{args.inspect}" unless silent
  stdout, stderr, status = Open3.capture3({ "LEFTHOOK" => "0" }, "git", *args)
  if !status.success? && !allow_failure
    raise "Command failed: git #{args.inspect}\n#{stdout.indent(2)}\n#{stderr.indent(2)}"
  end
  stdout
end
Supply chain risk

Description: The custom action installs and runs gh and uses sudo-enabled apt commands inside CI; if
the base image or network mirror is compromised this increases supply-chain risk—pin
versions and verify checksums/signatures for downloaded artifacts to reduce exposure.
action.yml [8-17]

Referred Code
# https://github.com/cli/cli/blob/trunk/docs/install_linux.md#debian-ubuntu-linux-raspberry-pi-os-apt
(type -p wget >/dev/null || (sudo apt update && sudo apt install wget -y)) \
  && sudo mkdir -p -m 755 /etc/apt/keyrings \
  && out=$(mktemp) && wget -nv -O$out https://cli.github.com/packages/githubcli-archive-keyring.gpg \
  && cat $out | sudo tee /etc/apt/keyrings/githubcli-archive-keyring.gpg > /dev/null \
  && sudo chmod go+r /etc/apt/keyrings/githubcli-archive-keyring.gpg \
  && sudo mkdir -p -m 755 /etc/apt/sources.list.d \
  && echo "deb [arch=$(dpkg --print-architecture) signed-by=/etc/apt/keyrings/githubcli-archive-keyring.gpg] https://cli.github.com/packages stable main" | sudo tee /etc/apt/sources.list.d/github-cli.list > /dev/null \
  && sudo apt update \
  && sudo apt install gh -y
Excessive CI permissions

Description: The workflow grants contents, pull-requests, and actions write permissions to the job by
default, which is overly broad for tagging and branch creation and could be abused if a
job step is compromised; least-privilege permissions (e.g., only contents:write) should be
applied.
release-handler.yml [17-21]

Referred Code
permissions:
  contents: write
  pull-requests: write
  actions: write
Token misuse in CI

Description: The workflow triggers another workflow using gh workflow run with ${{ github.token }};
depending on repository settings, this token may create untrusted workflow runs with
elevated permissions—ensure the token is scoped minimally and consider using the
workflow_run trigger with restricted permissions.
release-handler.yml [44-47]

Referred Code
run: gh workflow run release-prepare-bump.yml -f branch="${{ steps.maybe-cut-branch.outputs.new_branch_name }}"
env:
  GH_TOKEN: ${{ github.token }}
Ticket Compliance
🎫 No ticket provided
  • Create ticket/issue
Codebase Duplication Compliance
Codebase context is not defined

Follow the guide to enable codebase context checks.

Custom Compliance
🟢
Generic: Meaningful Naming and Self-Documenting Code

Objective: Ensure all identifiers clearly express their purpose and intent, making code
self-documenting

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Secure Error Handling

Objective: To prevent the leakage of sensitive system information through error messages while
providing sufficient detail for internal debugging.

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Secure Logging Practices

Objective: To ensure logs are useful for debugging and auditing without exposing sensitive
information like PII, PHI, or cardholder data.

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Comprehensive Audit Trails

Objective: To create a detailed and reliable record of critical system actions for security analysis
and compliance.

Status:
No audit logs: New release automation tasks perform critical repository actions (branch/tag creation,
pushes, PR creation) without emitting structured audit logs including actor, timestamp,
action, and outcome.

Referred Code
def self.make_pr(base:, branch:)
  return if test_mode?

  args = ["--title", `git log -1 --pretty=%s`.strip, "--body", `git log -1 --pretty=%b`.strip]

  success =
    system("gh", "pr", "create", "--base", base, "--head", branch, *args) ||
      system("gh", "pr", "edit", branch, *args)

  raise "Failed to create or update PR" unless success
end

def self.with_clean_worktree(origin_branch)
  git "fetch", "origin", origin_branch
  path = "#{Rails.root}/tmp/version-bump-worktree-#{SecureRandom.hex}"
  begin
    FileUtils.mkdir_p(path)
    git "worktree", "add", path, "origin/#{origin_branch}"
    Dir.chdir(path) { yield } # rubocop:disable Discourse/NoChdir
  ensure
    puts "Cleaning up temporary worktree..."


 ... (clipped 169 lines)

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Robust Error Handling and Edge Case Management

Objective: Ensure comprehensive error handling that provides meaningful context and graceful
degradation

Status:
Error handling gaps: Several operations raise generic exceptions or rely on system calls without retries or
contextual error messages, which may not gracefully handle edge cases (e.g., network/git
failures, missing env vars).

Referred Code
def self.git(*args, allow_failure: false, silent: false)
  puts "> git #{args.inspect}" unless silent
  stdout, stderr, status = Open3.capture3({ "LEFTHOOK" => "0" }, "git", *args)
  if !status.success? && !allow_failure
    raise "Command failed: git #{args.inspect}\n#{stdout.indent(2)}\n#{stderr.indent(2)}"
  end
  stdout
end

def self.ref_exists?(ref)
  git "rev-parse", "--verify", ref
  true
rescue StandardError
  false
end

def self.make_pr(base:, branch:)
  return if test_mode?

  args = ["--title", `git log -1 --pretty=%s`.strip, "--body", `git log -1 --pretty=%b`.strip]



 ... (clipped 113 lines)

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Security-First Input Validation and Data Handling

Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent
vulnerabilities

Status:
Action inputs trust: Workflow accepts a user-supplied SHA and branch contexts to drive tagging and branch
creation without explicit validation or protection against unexpected values.

Referred Code
name: Release - create tags & branches

on:
  workflow_dispatch:
    inputs:
      sha:
        description: "The commit SHA to prepare the release for"
        required: false
        type: string
  push:
    branches:
      - main
      - release/*
    paths:
      - "lib/version.rb"

permissions:
  contents: write
  pull-requests: write
  actions: write



 ... (clipped 28 lines)

Learn more about managing compliance generic rules or creating your own custom rules

Compliance status legend 🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

@qodo-code-review
Copy link

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
High-level
Rake tasks are too complex

The Rake tasks in lib/tasks/release.rake are overly complex, handling git
operations, API calls, and business logic directly. This logic should be
refactored into dedicated Ruby classes for better testability and
maintainability, with the Rake tasks serving only as simple entry points.

Examples:

lib/tasks/release.rake [3-232]
module ReleaseUtils
  def self.dry_run?
    !!ENV["DRY_RUN"]
  end

  def self.test_mode?
    ENV["RUNNING_RELEASE_IN_RSPEC_TESTS"] == "1"
  end

  def self.read_version_rb

 ... (clipped 220 lines)

Solution Walkthrough:

Before:

# lib/tasks/release.rake

module ReleaseUtils
  def self.git(*args, ...)
    # ... shells out to git command ...
  end

  def self.make_pr(base:, branch:)
    # ... shells out to gh cli ...
  end

  def self.with_clean_worktree(origin_branch)
    # ... creates temp git worktree, chdir's into it ...
  end
end

namespace :release do
  task "maybe_cut_branch", [:check_ref] do |t, args|
    ReleaseUtils.with_clean_worktree("main") do
      # ... complex git operations, version checks, and workflow logic ...
    end
  end
end

After:

# lib/release_manager.rb (new file)
class ReleaseManager
  def initialize(repo_path)
    # ...
  end

  def maybe_cut_branch(check_ref)
    # ... complex logic encapsulated here ...
    # Uses a git library or dedicated git wrapper class
  end

  def make_pr(...)
    # ... uses an API client for GitHub ...
  end
end

# lib/tasks/release.rake (simplified)
namespace :release do
  task "maybe_cut_branch", [:check_ref] do |t, args|
    manager = ReleaseManager.new(Rails.root)
    manager.maybe_cut_branch(args[:check_ref])
  end
end
Suggestion importance[1-10]: 8

__

Why: The suggestion correctly identifies a significant architectural issue where complex, fragile logic is tightly coupled within Rake tasks, making it hard to test and maintain.

Medium
Possible issue
Use safe git command execution

Replace backtick git command execution in make_pr with the ReleaseUtils.git
helper method for improved error handling and code consistency.

lib/tasks/release.rake [43-53]

 def self.make_pr(base:, branch:)
   return if test_mode?
 
-  args = ["--title", `git log -1 --pretty=%s`.strip, "--body", `git log -1 --pretty=%b`.strip]
+  title = git("log", "-1", "--pretty=%s", silent: true).strip
+  body = git("log", "-1", "--pretty=%b", silent: true).strip
+  args = ["--title", title, "--body", body]
 
   success =
     system("gh", "pr", "create", "--base", base, "--head", branch, *args) ||
       system("gh", "pr", "edit", branch, *args)
 
   raise "Failed to create or update PR" unless success
 end
  • Apply / Chat
Suggestion importance[1-10]: 7

__

Why: This is a valid suggestion that improves code consistency and robustness by using the newly introduced git helper method, which provides centralized error handling.

Medium
Ensure original directory is restored

Modify with_clean_worktree to save the original working directory and restore it
in the ensure block, preventing state leakage if an error occurs.

lib/tasks/release.rake [55-67]

 def self.with_clean_worktree(origin_branch)
   git "fetch", "origin", origin_branch
   path = "#{Rails.root}/tmp/version-bump-worktree-#{SecureRandom.hex}"
+  original_dir = Dir.pwd
   begin
     FileUtils.mkdir_p(path)
     git "worktree", "add", path, "origin/#{origin_branch}"
     Dir.chdir(path) { yield } # rubocop:disable Discourse/NoChdir
   ensure
+    Dir.chdir(original_dir)
     puts "Cleaning up temporary worktree..."
     git "worktree", "remove", "--force", path, silent: true, allow_failure: true
     FileUtils.rm_rf(path)
   end
 end
  • Apply / Chat
Suggestion importance[1-10]: 7

__

Why: The suggestion correctly identifies a potential issue where the working directory is not restored after an exception, and provides a correct fix to ensure the process state is cleaned up properly.

Medium
Improve git ref existence check

Refactor ref_exists? to check the git command's exit status instead of using a
broad rescue StandardError, making the check more reliable.

lib/tasks/release.rake [36-41]

 def self.ref_exists?(ref)
-  git "rev-parse", "--verify", ref
-  true
-rescue StandardError
-  false
+  _stdout, status = git("rev-parse", "--verify", "--quiet", ref, allow_failure: true, silent: true)
+  status.success?
 end
  • Apply / Chat
Suggestion importance[1-10]: 6

__

Why: The suggestion correctly identifies that rescuing StandardError is a poor practice for control flow and can mask other errors, proposing a more robust check of the command's exit status.

Low
  • More

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants