Skip to content

Conversation

@tomerqodo
Copy link

@tomerqodo tomerqodo commented Dec 4, 2025

User description

Benchmark PR discourse#35180

Type: Corrupted (contains bugs)

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 with automated release workflows

  • Replace manual version-bump tasks with new release:* rake tasks

  • Add GitHub Actions workflows for automated release preparation and tagging

  • Remove legacy beta and stable version bump tasks


Diagram Walkthrough

flowchart LR
  A["Manual Version Bumps"] -->|replaced by| B["release:* Rake Tasks"]
  B --> C["GitHub Actions Workflows"]
  C --> D["release-prepare-latest-bump"]
  C --> E["release-handler"]
  C --> F["release-prepare-bump"]
  D -->|creates PR| G["Version Bump on main"]
  G -->|triggers| E
  E -->|tags & branches| H["Release Tags & Branches"]
  H -->|triggers| F
  F -->|creates PR| I["Version Bump on release/*"]
Loading

File Walkthrough

Relevant files
Enhancement
2 files
release.rake
New release automation rake tasks implementation                 
+232/-0 
version_bump.rake
Remove legacy beta and stable version bump tasks                 
+0/-173 
Configuration changes
6 files
version.rb
Update version to date-based format                                           
+1/-1     
action.yml
New GitHub Actions setup for release environment                 
+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
Downgrade checkout action version                                               
+1/-1     
Tests
3 files
release_spec.rb
Add comprehensive tests for new release tasks                       
+203/-0 
version_bump_spec.rb
Remove tests for legacy version bump tasks                             
+0/-134 
theme_spec.rb
Update version number in test fixture                                       
+1/-1     

davidtaylorhq and others added 3 commits November 25, 2025 10:19
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.
@qodo-code-review
Copy link

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
Unverified third-party download

Description: The workflow action downloads and installs the GitHub CLI over apt using a key fetched via
wget over HTTPS without pinning or checksum verification, exposing the supply chain to
MITM or repository key compromise risks during CI setup.
action.yml [9-17]

Referred Code
(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
Command injection risk

Description: The code shells out to the GitHub CLI (system("gh", ...)) with arguments partially derived
from git log output without strict sanitization, which can allow shell/argument injection
via crafted commit messages in untrusted repositories or forks when automation runs in CI.

release.rake [46-53]

Referred Code
  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
Excessive CI permissions

Description: The workflow runs in a privileged container with contents/pull-requests/actions write
permissions and triggers gh workflow run and tag pushes based on a provided SHA,
increasing blast radius if a malicious commit gains control; consider least-privilege
scoping, environment protection, and explicit allowlists for branches.
release-handler.yml [24-49]

Referred Code
name: run
runs-on: ubuntu-latest
container: discourse/discourse_test:slim
timeout-minutes: 10

steps:
  - uses: actions/checkout@v6
    with:
      filter: tree:0
      fetch-depth: 0

  - uses: ./.github/actions/setup-release-environment

  - name: Maybe cut branch
    id: maybe-cut-branch
    if: github.ref == 'refs/heads/main'
    run: bin/rake release:maybe_cut_branch[${{ inputs.sha || github.sha }}]

  - name: Maybe trigger prepare next version
    if: steps.maybe-cut-branch.outputs.new_branch_name
    run: gh workflow run release-prepare-bump.yml -f branch="version-bump/${{ steps.maybe-cut-branch.outputs.new_branch_name }}"


 ... (clipped 5 lines)
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: Robust Error Handling and Edge Case Management

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

Status:
Unhandled Errors: External command and file operations (git, tagging, pushing, PR creation) lack robust
error handling and contextual logging in several paths, leading to possible silent or
generic failures.

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 94 lines)

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:
Missing Auditing: New release automation tasks perform critical actions (tagging, branching, pushing, PR
creation) without emitting structured audit logs that include actor, timestamp, action,
and outcome.

Referred Code
namespace :release do
  desc "Check a commit hash and create a release branch if it's a trigger"
  task "maybe_cut_branch", [:check_ref] do |t, args|
    check_ref = args[:check_ref]

    ReleaseUtils.with_clean_worktree("main") do
      ReleaseUtils.git("checkout", check_ref.to_s)
      new_version = ReleaseUtils.parse_current_version

      ReleaseUtils.git("checkout", "#{check_ref}^1")
      previous_version = ReleaseUtils.parse_current_version

      next "version has not changed" if new_version == previous_version

      raise "Unexpected previous version" if !previous_version.ends_with? "-latest"
      raise "Unexpected new version" if !new_version.ends_with? "-latest"
      if Gem::Version.new(new_version) < Gem::Version.new(previous_version)
        raise "New version is smaller than old version"
      end

      parts = new_version.split(".")


 ... (clipped 141 lines)

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:
Verbose Errors: Errors raised include raw git command arguments and outputs which may expose internal
details; confirm these are not surfaced to end users and only appear in secure CI logs.

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

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:
Token Exposure Risk: GitHub Actions steps use GH_TOKEN to run gh commands; ensure no command echoes tokens or
sensitive outputs to logs and that logs remain free of secrets.

Referred Code
- name: Setup gems
  shell: bash
  run: |
    bundle config --local path vendor/bundle
    bundle config --local deployment true
    bundle config --local without development
    bundle install --jobs 4
    bundle clean

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:
Input Validation: Workflow uses user-supplied input sha and branch-derived refs without explicit validation
or pinning, which could allow unintended refs if misused.

Referred Code
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"

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
The release workflow is fundamentally flawed

The release process is broken. The release-handler workflow passes an incorrect
branch name, and the maybe_cut_branch rake task creates release branches from
the wrong commit.

Examples:

.github/workflows/release-handler.yml [42-46]
      - name: Maybe trigger prepare next version
        if: steps.maybe-cut-branch.outputs.new_branch_name
        run: gh workflow run release-prepare-bump.yml -f branch="version-bump/${{ steps.maybe-cut-branch.outputs.new_branch_name }}"
        env:
          GH_TOKEN: ${{ github.token }}
lib/tasks/release.rake [79-93]
      ReleaseUtils.git("checkout", "#{check_ref}^1")
      previous_version = ReleaseUtils.parse_current_version

      next "version has not changed" if new_version == previous_version

      raise "Unexpected previous version" if !previous_version.ends_with? "-latest"
      raise "Unexpected new version" if !new_version.ends_with? "-latest"
      if Gem::Version.new(new_version) < Gem::Version.new(previous_version)
        raise "New version is smaller than old version"
      end

 ... (clipped 5 lines)

Solution Walkthrough:

Before:

# .github/workflows/release-handler.yml
- name: Maybe trigger prepare next version
  if: steps.maybe-cut-branch.outputs.new_branch_name
  run: gh workflow run release-prepare-bump.yml -f branch="version-bump/${{ steps.maybe-cut-branch.outputs.new_branch_name }}"

# lib/tasks/release.rake
task "maybe_cut_branch", [:check_ref] do |t, args|
  # ...
  ReleaseUtils.git("checkout", "#{check_ref}^1")
  previous_version = ReleaseUtils.parse_current_version
  # ...
  parts = new_version.split(".")
  new_branch_name = "release/#{parts[0]}.#{parts[1]}"
  ReleaseUtils.git("branch", new_branch_name) # Creates branch from check_ref^1
  # ...
end

After:

# .github/workflows/release-handler.yml
- name: Maybe trigger prepare next version
  if: steps.maybe-cut-branch.outputs.new_branch_name
  run: gh workflow run release-prepare-bump.yml -f branch="${{ steps.maybe-cut-branch.outputs.new_branch_name }}"

# lib/tasks/release.rake
task "maybe_cut_branch", [:check_ref] do |t, args|
  # ...
  ReleaseUtils.git("checkout", "#{check_ref}^1")
  previous_version = ReleaseUtils.parse_current_version
  # ...
  parts = new_version.split(".")
  new_branch_name = "release/#{parts[0]}.#{parts[1]}"
  ReleaseUtils.git("branch", new_branch_name, "#{check_ref}^1") # Explicitly create branch from correct commit
  # ...
end
Suggestion importance[1-10]: 10

__

Why: This suggestion correctly identifies two critical, independent flaws in the core logic of the new release process that would cause it to fail or produce incorrect releases, making it non-functional.

High
Possible issue
Add sudo to chown command

Add sudo to the chown root:root . command in the setup-release-environment
action to provide the necessary permissions and prevent the step from failing.

.github/actions/setup-release-environment/action.yml [19-21]

 - name: Set working directory owner
   shell: bash
-  run: chown root:root .
+  run: sudo chown root:root .
  • Apply / Chat
Suggestion importance[1-10]: 9

__

Why: The suggestion correctly identifies that the chown command will fail without sudo because the workflow runs as a non-root user, which is a critical bug that would cause the action to fail.

High
Fix incorrect workflow input parameter

Remove the version-bump/ prefix when calling the release-prepare-bump.yml
workflow, as the new_branch_name variable already contains the correctly
formatted release/* branch name expected by the downstream task.

.github/workflows/release-handler.yml [42-46]

 - name: Maybe trigger prepare next version
   if: steps.maybe-cut-branch.outputs.new_branch_name
-  run: gh workflow run release-prepare-bump.yml -f branch="version-bump/${{ steps.maybe-cut-branch.outputs.new_branch_name }}"
+  run: gh workflow run release-prepare-bump.yml -f branch="${{ steps.maybe-cut-branch.outputs.new_branch_name }}"
   env:
     GH_TOKEN: ${{ github.token }}
  • Apply / Chat
Suggestion importance[1-10]: 9

__

Why: The suggestion correctly identifies a bug where an incorrect branch name is passed to a downstream workflow, which would cause the rake task to fail due to a validation check.

High
Correctly identify the latest stable release

Modify the logic for updating the release tag to filter out pre-release versions
from existing_releases before comparison. This ensures the release tag correctly
points to the latest stable version.

lib/tasks/release.rake [152-159]

-if existing_releases.last && Gem::Version.new(current_version) > existing_releases.last
-  ReleaseUtils.git "tag", "-a", "release", "-m", "latest release"
+latest_stable_release = existing_releases.reject(&:prerelease?).last
+if latest_stable_release && Gem::Version.new(current_version) > latest_stable_release
+  ReleaseUtils.git "tag", "-f", "-a", "release", "-m", "latest release"
   if ReleaseUtils.dry_run?
     puts "[DRY RUN] Skipping pushing 'release' tag to origin"
   else
     ReleaseUtils.git "push", "origin", "-f", "refs/tags/release"
   end
 end
  • Apply / Chat
Suggestion importance[1-10]: 8

__

Why: The suggestion correctly identifies a logic flaw where pre-release versions could prevent the 'release' tag from being updated for a new stable version, and the proposed fix correctly filters for stable releases.

Medium
Fix incorrect version increment logic

Correct the version increment logic to reset the patch version to 0 when the
minor version is incremented, ensuring the new version string is correctly
formatted (e.g., 2025.11.0-latest).

lib/tasks/release.rake [179-184]

 if Gem::Version.new(target_version_number) <= Gem::Version.new(current_version)
-  puts "Target version #{target_version_number} is already >= #{current_version}. Incrementing instead."
-  major, minor, patch_and_pre = current_version.split(".")
+  puts "Target version #{target_version_number} is not newer than #{current_version}. Incrementing instead."
+  major, minor, _ = current_version.split(".")
   minor = (minor.to_i + 1).to_s.rjust(2, "0")
-  target_version_number = "#{major}.#{minor}.#{patch_and_pre}"
+  target_version_number = "#{major}.#{minor}.0-latest"
 end
  • Apply / Chat
Suggestion importance[1-10]: 8

__

Why: The suggestion correctly identifies a bug in the version increment logic where the patch version was not reset to zero, leading to an incorrect next version string.

Medium
  • 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.

3 participants